Re: [PATCH v2 00/23] ata: sata_dwc_460ex: make it working again
On Mon, May 09, 2016 at 10:59:32PM +0300, Andy Shevchenko wrote: > On Mon, May 9, 2016 at 10:05 PM, Tejun Heo wrote: > > On Mon, May 09, 2016 at 10:13:59AM +0100, Måns Rullgård wrote: > >> Andy Shevchenko writes: > >> > >> > On Mon, May 9, 2016 at 4:09 AM, Tejun Heo wrote: > >> >> On Sun, May 08, 2016 at 04:00:08PM -0400, Tejun Heo wrote: > >> >>> Hello, Andy. > >> >>> > >> >>> On Wed, May 04, 2016 at 03:22:51PM +0300, Andy Shevchenko wrote: > >> >>> > Tejun, since Vinod applied all necessary patches into his tree, the > >> >>> > series now has just a dependency to whatever branch / tag he marks > >> >>> > for > >> >>> > it. > >> >>> > Do we have a chance to see the SATA series be applied in your tree? > >> >>> > >> >>> Applied 1-22 to libata/for-4.7. There was a couple trivial conflicts > >> >>> which I resolved while applying but it'd be great if you can check > >> >>> whether everything looks okay. > >> >>> > >> >>> > >> >>> https://git.kernel.org/cgit/linux/kernel/git/tj/libata.git/log/?h=for-4.7 > >> >> > >> >> Oops, build failure. Reverted for now. > >> > > >> > I suppose you have to pull material from Vinod. > >> > >> The failure the build bot reported is due to the DMA patches not being > >> in the tree. > > > > Which tree should I pull? > > slave-dma [1], branch topic/dw. But I think Vinod can tell us which > tag/branch will be immutable. Vinod? Please use branch topic/dw. I will not rebase this before sending to Linus. Also if you prefer a signed tag, then please use dmaengine-topic-dw-4.7-rc1 > > [1] http://git.infradead.org/users/vkoul/slave-dma.git Thanks -- ~Vinod ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v3 2/2] KVM: PPC: hypervisor large decrementer support
Power ISAv3 extends the width of the decrementer register from 32 bits. The enlarged register width is implementation dependent, but reads from these registers are automatically sign extended to produce a 64 bit output when operating in large mode. The HDEC always operates in large mode while the DEC register can be operated in 32bit mode or large mode depending on the setting of the LPCR.LD bit. Currently the hypervisor assumes that reads from the DEC and HDEC register produce a 32 bit result which it sign extends to 64 bits using the extsw instruction. This behaviour can result in the guest DEC register value being corrupted by the hypervisor when the guest is operating in LD mode since the results of the extsw instruction only depends on the value of bit 31 in the register to be sign extended. This patch adds the GET_DEC() and GET_HDEC() assembly macros for reading from the decrementer registers. These macros will return the current decrementer value as a 64 bit quantity regardless of the Host CPU or guest decrementer operating mode. Additionally this patch corrects several uses of decrementer values that assume a 32 bit register width. Signed-off-by: Oliver O'Halloran Cc: Paul Mackerras Cc: Michael Neuling --- arch/powerpc/include/asm/exception-64s.h | 29 arch/powerpc/include/asm/kvm_host.h | 2 +- arch/powerpc/include/asm/kvm_ppc.h | 2 +- arch/powerpc/include/uapi/asm/kvm.h | 2 +- arch/powerpc/kernel/time.c | 2 +- arch/powerpc/kvm/book3s_hv_interrupts.S | 3 +-- arch/powerpc/kvm/book3s_hv_rmhandlers.S | 38 ++-- arch/powerpc/kvm/emulate.c | 6 ++--- 8 files changed, 58 insertions(+), 26 deletions(-) diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h index 93ae809fe5ea..4fa303bf6d5b 100644 --- a/arch/powerpc/include/asm/exception-64s.h +++ b/arch/powerpc/include/asm/exception-64s.h @@ -545,4 +545,33 @@ END_FTR_SECTION_IFSET(CPU_FTR_CAN_NAP) #define FINISH_NAP #endif +/* + * On ISAv3 processors the DEC register can be extended from 32 bits to 64 by + * setting the LD flag the LPCR. The decrementer value is a signed quantity so + * sign exension is required when operating in 32 bit mode. The GET_DEC() and + * GET_HDEC() handle this sign extension and yield a 64 bit result independent + * of the LD mode. + * + * NB: It's possible run with LD mode disabled on ISAv3 so GET_DEC() does not + * use a CPU_FEATURE section. A feature section is used for GET_HDEC because + * it has no mode bit. It is always 64 bits for ISAv3 processors. + */ + +#define IS_LD_ENABLED(reg) \ + mfspr reg,SPRN_LPCR; \ + andis. reg,reg,(LPCR_LD >> 16); + +#define GET_DEC(reg) \ + IS_LD_ENABLED(reg);\ + mfspr reg, SPRN_DEC; \ + bne 99f; \ + extsw reg, reg;\ +99: + +#define GET_HDEC(reg) \ + mfspr reg, SPRN_HDEC; \ +BEGIN_FTR_SECTION \ + extsw reg, reg; \ +END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300) + #endif /* _ASM_POWERPC_EXCEPTION_H */ diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index d7b343170453..6330d3fca083 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -516,7 +516,7 @@ struct kvm_vcpu_arch { ulong mcsrr0; ulong mcsrr1; ulong mcsr; - u32 dec; + u64 dec; #ifdef CONFIG_BOOKE u32 decar; #endif diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h index 2544edabe7f3..4de0102930e9 100644 --- a/arch/powerpc/include/asm/kvm_ppc.h +++ b/arch/powerpc/include/asm/kvm_ppc.h @@ -94,7 +94,7 @@ extern int kvmppc_emulate_instruction(struct kvm_run *run, extern int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu); extern int kvmppc_emulate_mmio(struct kvm_run *run, struct kvm_vcpu *vcpu); extern void kvmppc_emulate_dec(struct kvm_vcpu *vcpu); -extern u32 kvmppc_get_dec(struct kvm_vcpu *vcpu, u64 tb); +extern u64 kvmppc_get_dec(struct kvm_vcpu *vcpu, u64 tb); extern void kvmppc_decrementer_func(struct kvm_vcpu *vcpu); extern int kvmppc_sanity_check(struct kvm_vcpu *vcpu); extern int kvmppc_subarch_vcpu_init(struct kvm_vcpu *vcpu); diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h index c93cf35ce379..2dd92e841127 100644 --- a/arch/powerpc/include/uapi/asm/kvm.h +++ b/arch/powerpc/include/uapi/asm/kvm.h @@ -215,7 +215,7 @@ struct kvm_sregs { __u32 tsr; /* KVM_SREGS_E_UPDATE_TSR */ __u32 tcr; __u32 decar; - __u32 dec; /* KVM_SREGS_E_UPDATE_DEC */ + __u64 dec; /* KVM_SREGS_E_UPDATE_DEC */ /*
[PATCH v3 1/2] powerpc/timer - large decrementer support
POWER ISA v3 adds large decrementer (LD) mode of operation which increases the size of the decrementer register from 32 bits to an implementation defined with of up to 64 bits. This patch adds support for the LD on processors with the CPU_FTR_ARCH_300 cpu feature flag set. For CPUs with this feature LD mode is enabled when when the ibm,dec-bits devicetree property is supplied for the boot CPU. The decrementer value is a signed quantity (with negative values indicating a pending exception) and this property is required to find the maximum positive decrementer value. If this property is not supplied then the traditional decrementer width of 32 bits is assumed and LD mode is disabled. This patch was based on initial work by Jack Miller. Signed-off-by: Oliver O'Halloran Cc: Michael Neuling Cc: Balbir Singh Cc: Jack Miller --- arch/powerpc/include/asm/reg.h | 1 + arch/powerpc/include/asm/time.h | 6 +-- arch/powerpc/kernel/time.c | 92 + 3 files changed, 89 insertions(+), 10 deletions(-) diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h index f5f4c66bbbc9..ff581ed1ab9d 100644 --- a/arch/powerpc/include/asm/reg.h +++ b/arch/powerpc/include/asm/reg.h @@ -332,6 +332,7 @@ #define LPCR_AIL_0 0x /* MMU off exception offset 0x0 */ #define LPCR_AIL_3 0x0180 /* MMU on exception offset 0xc00...4xxx */ #define LPCR_ONL 0x0004 /* online - PURR/SPURR count */ +#define LPCR_LD 0x0002 /* large decremeter */ #define LPCR_PECE0x0001f000 /* powersave exit cause enable */ #define LPCR_PECEDP0x0001 /* directed priv dbells cause exit */ #define LPCR_PECEDH0x8000 /* directed hyp dbells cause exit */ diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h index 1092fdd7e737..09211640a0e0 100644 --- a/arch/powerpc/include/asm/time.h +++ b/arch/powerpc/include/asm/time.h @@ -146,7 +146,7 @@ static inline void set_tb(unsigned int upper, unsigned int lower) * in auto-reload mode. The problem is PIT stops counting when it * hits zero. If it would wrap, we could use it just like a decrementer. */ -static inline unsigned int get_dec(void) +static inline u64 get_dec(void) { #if defined(CONFIG_40x) return (mfspr(SPRN_PIT)); @@ -160,10 +160,10 @@ static inline unsigned int get_dec(void) * in when the decrementer generates its interrupt: on the 1 to 0 * transition for Book E/4xx, but on the 0 to -1 transition for others. */ -static inline void set_dec(int val) +static inline void set_dec(u64 val) { #if defined(CONFIG_40x) - mtspr(SPRN_PIT, val); + mtspr(SPRN_PIT, (u32) val); #else #ifndef CONFIG_BOOKE --val; diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c index 81b0900a39ee..0656e80cadbf 100644 --- a/arch/powerpc/kernel/time.c +++ b/arch/powerpc/kernel/time.c @@ -95,7 +95,8 @@ static struct clocksource clocksource_timebase = { .read = timebase_read, }; -#define DECREMENTER_MAX0x7fff +#define DECREMENTER_DEFAULT_MAX 0x7FFF +u64 decrementer_max = DECREMENTER_DEFAULT_MAX; static int decrementer_set_next_event(unsigned long evt, struct clock_event_device *dev); @@ -503,7 +504,7 @@ static void __timer_interrupt(void) __this_cpu_inc(irq_stat.timer_irqs_event); } else { now = *next_tb - now; - if (now <= DECREMENTER_MAX) + if (now <= decrementer_max) set_dec((int)now); /* We may have raced with new irq work */ if (test_irq_work_pending()) @@ -534,7 +535,7 @@ void timer_interrupt(struct pt_regs * regs) /* Ensure a positive value is written to the decrementer, or else * some CPUs will continue to take decrementer exceptions. */ - set_dec(DECREMENTER_MAX); + set_dec(decrementer_max); /* Some implementations of hotplug will get timer interrupts while * offline, just ignore these and we also need to set @@ -582,9 +583,9 @@ static void generic_suspend_disable_irqs(void) * with suspending. */ - set_dec(DECREMENTER_MAX); + set_dec(decrementer_max); local_irq_disable(); - set_dec(DECREMENTER_MAX); + set_dec(decrementer_max); } static void generic_suspend_enable_irqs(void) @@ -865,7 +866,7 @@ static int decrementer_set_next_event(unsigned long evt, static int decrementer_shutdown(struct clock_event_device *dev) { - decrementer_set_next_event(DECREMENTER_MAX, dev); + decrementer_set_next_event(decrementer_max, dev); return 0; } @@ -891,6 +892,76 @@ static void register_decrementer_clockevent(int cpu) clockevents_register_device(dec); } +static inline bool cpu_has_large_dec(void) +{ + return cpu_has_feature(CPU_FTR_AR
[PATCH V2] lib/test-hexdump: Changed to work on BE
test-hexdump.c doesn't work on BE because the test data is in LE format. So add in data for BE. Signed-off-by: Rashmica Gupta --- V2: Checking __BYTE_ORDER__. lib/test_hexdump.c | 35 +++ 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c index 3f415d8101f3..9de91a1b98bf 100644 --- a/lib/test_hexdump.c +++ b/lib/test_hexdump.c @@ -18,29 +18,48 @@ static const unsigned char data_b[] = { static const unsigned char data_a[] = ".2.{p..$}.4...1.L...C..."; -static const char * const test_data_1_le[] __initconst = { +static const char * const test_data_1[] __initconst = { "be", "32", "db", "7b", "0a", "18", "93", "b2", "70", "ba", "c4", "24", "7d", "83", "34", "9b", "a6", "9c", "31", "ad", "9c", "0f", "ac", "e9", "4c", "d1", "19", "99", "43", "b1", "af", "0c", }; -static const char * const test_data_2_le[] __initconst = { +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ +static const char * const test_data_2[] __initconst = { "32be", "7bdb", "180a", "b293", "ba70", "24c4", "837d", "9b34", "9ca6", "ad31", "0f9c", "e9ac", "d14c", "9919", "b143", "0caf", }; -static const char * const test_data_4_le[] __initconst = { +static const char * const test_data_4[] __initconst = { "7bdb32be", "b293180a", "24c4ba70", "9b34837d", "ad319ca6", "e9ac0f9c", "9919d14c", "0cafb143", }; -static const char * const test_data_8_le[] __initconst = { +static const char * const test_data_8[] __initconst = { "b293180a7bdb32be", "9b34837d24c4ba70", "e9ac0f9cad319ca6", "0cafb1439919d14c", }; +#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ +static const char * const test_data_2[] __initconst = { + "be32", "db7b", "0a18", "93b2", + "70ba", "c424", "7d83", "349b", + "a69c", "31ad", "9c0f", "ace9", + "4cd1", "1999", "43b1", "af0c", +}; + +static const char * const test_data_4[] __initconst = { + "be32db7b", "0a1893b2", "70bac424", "7d83349b", + "a69c31ad", "9c0face9", "4cd11999", "43b1af0c", +}; + +static const char * const test_data_8[] __initconst = { + "be32db7b0a1893b2", "70bac4247d83349b", + "a69c31ad9c0face9", "4cd1199943b1af0c", +}; +#endif #define FILL_CHAR '#' @@ -67,13 +86,13 @@ static void __init test_hexdump_prepare_test(size_t len, int rowsize, gs = 1; if (gs == 8) - result = test_data_8_le; + result = test_data_8; else if (gs == 4) - result = test_data_4_le; + result = test_data_4; else if (gs == 2) - result = test_data_2_le; + result = test_data_2; else - result = test_data_1_le; + result = test_data_1; /* hex dump */ p = test; -- 2.5.0 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [1/2] powerpc/mm: Remove duplicated check in do_page_fault()
On Mon, May 09, 2016 at 08:03:50PM +1000, Michael Ellerman wrote: >On Fri, 2016-26-02 at 00:26:25 UTC, Gavin Shan wrote: >> When the page fault happened in user space, we need check it's >> caused by stack frame pointer update instruction and update >> local variable @flag with FAULT_FLAG_USER. Currently, the code >> has two separate check for the same condition. That's unnecessary. >> >> This removes one of the duplicated check. No functinal changes >> introduced. > >It's possible though that store_updates_sp() changes regs, and causes >user_mode(regs) to change, which would mean the second check is necessary. >That's not true with the current code, but you should mention that you >confirmed >that in the change log. > Thanks for review. Yeah, store_updates_sp() checks the failing instruction is the one updating stack frame pointer (stdu and the variable). The info is used to expand the stack later. All of it should have been documented in the commit log. >> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c >> index a67c6d7..935f386 100644 >> --- a/arch/powerpc/mm/fault.c >> +++ b/arch/powerpc/mm/fault.c >> @@ -294,11 +294,10 @@ int __kprobes do_page_fault(struct pt_regs *regs, >> unsigned long address, >> * can result in fault, which will cause a deadlock when called with >> * mmap_sem held >> */ >> -if (user_mode(regs)) >> -store_update_sp = store_updates_sp(regs); >> - >> -if (user_mode(regs)) >> +if (user_mode(regs)) { >> flags |= FAULT_FLAG_USER; >> +store_update_sp = store_updates_sp(regs); >> +} > >It doesn't really matter in this case, but it would be better to keep the >ordering of the two statements the same as before. > Yep, It should have two checks in order as before here if store_updates_sp() can alter MSR[PR] bit in future as you explained above :-) Thanks, Gavin >cheers > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [v2,2/2] powerpc/mm: Ensure "special" zones are empty
On Thu, 2016-05-05 at 07:54:09 UTC, Oliver O'Halloran wrote: > The mm zone mechanism was traditionally used by arch specific code to > partition memory into allocation zones. However there are several zones > that are managed by the mm subsystem rather than the architecture. Most > architectures set the max PFN of these special zones to zero, however on > powerpc we set them to ~0ul. This, in conjunction with a bug in > free_area_init_nodes() results in all of system memory being placed in > ZONE_DEVICE when enabled. Device memory cannot be used for regular kernel > memory allocations so this will cause a kernel panic at boot. This is breaking my freescale machine: Sorting __ex_table... Unable to handle kernel paging request for data at address 0xc00101e28020 Faulting instruction address: 0xc09ab698 cpu 0x0: Vector: 300 (Data Access) at [c0acbb30] pc: c09ab698: .reserve_bootmem_region+0x64/0x8c lr: c09883d0: .free_all_bootmem+0x70/0x200 sp: c0acbdb0 msr: 80021000 dar: c00101e28020 dsisr: 80 current = 0xc0a07640 paca= 0xc0003fff5000 softe: 0irq_happened: 0x01 pid = 0, comm = swapper Linux version 4.6.0-rc3-00160-gc09920947f23 (michael@ka1) (gcc version 5.3.0 (GCC) ) #5 SMP Tue May 10 09:44:11 AEST 2016 enter ? for help [link register ] c09883d0 .free_all_bootmem+0x70/0x200 [c0acbdb0] c0988398 .free_all_bootmem+0x38/0x200 (unreliable) [c0acbe80] c097b700 .mem_init+0x5c/0x7c [c0acbef0] c0971a0c .start_kernel+0x28c/0x4e4 [c0acbf90] c544 start_here_common+0x20/0x5c 0:mon> ? I can give you access some time if you need to debug it. cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4
On Mon, 2016-05-09 at 21:06 +0200, Christian Lamparter via Linuxppc-dev wrote: > > I ran into the following issues: > - gadget.c uses ioread32_rep [0] & iowrite32_rep [1]. > This is interesting because both of these functions actually use > the __raw_io* on powerpc. This is because powerpc uses the default > defines of include/asm-generic/io.h [2]. > > Ideally, this should be done by sth like a writesl_be or writesl(e) > function. But I found none so for now: Let's make a ugly hack: > to_correct_endian that will work for testing, but will be replaced. No. The _rep variants always must use native endian. If for some reason your device requires something different then the device is more broken than I thought. IE. even with a BE core and an LE device, we use non- byeswap _rep versions, this isn't just because we use "defaults" on powerpc for example, it's necessary to work. Here too, I could suggest you google for my talk on the subject :-) But if you think closely about it, you'll figure out that FIFOs don't have an endian per-se, thus what matters is which byte is first in ascending address order, and that byte must land in memory in the first address as well. Interpreting the resulting data might require endian swaps, but actually transferring to/from the fifo doesn't. > - dwc2_readl, dwc2_writel. I have no insight in the craziness that's > going on with MIPS and the memory barrier. But it got me thinking > rather than CONFIG_MIPS, isn't there a umbrella > "ARCH_HAS_NEED_MEMORY_BARRIER_FOR_MMIO" config symbol? > > - is_little_endian (do we want a separate is_big_endian?) > Also, do we want to be able to overwrite the detection code > if the endian setting was set in the device tree?. For now > it always does auto detection (see dwc2_detect_endiannes() ). If auto-detect always work, no need to bother with the device-tree. A single flag is enough, either is_big or is_little, doesn't matter. "readl" should always be little, readl_be should always be big, though the latter isn't supported on all archs. However the new accessors in iomap.h should provide a "be" variant on all archs so switching to these might be a good idea. > ( - 80 character per line issues, is it possible to drop the > hsotg->reg + REGISTER from the dwc2_readl/writel since we > pass the hsotg now anyway and do the reg + REGISTER > calculation in the accessor? I played around with macros > as most functions calling the accessors have the hsotg > variable anyway ) Or just use a temp variable, the compiler should deal with it the same way. Ben. > Regards, > Christian ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4
On Mon, 2016-05-09 at 17:08 +0200, Arnd Bergmann wrote: > > Unfortunately, I don't see any way this could be done in MIPS specific > code: There is typically a byteswap between the internal bus and the PCI > bus on big-endian MIPS systems, so the PCI MMIO ends up being little-endian, Ugh ... not exactly, re-watch my talk on the matter :-) While there is a specific lane wiring to preserve byte addresss, in the end it's the end device itself that is either BE or LE. Regardless of any "bus endianness". > which matches the expected behavior of readl/writel. However, drivers > for non-PCI devices often use the same readl/writel accessors because > that is how it's done on ARMv6/ARMv7. Even then, you can have on-SoC (non-PCI) devices that also have a different endianness from the main CPU. How does it work on ARM for example ? The device endianness should be fixed, regardless of the endianness of the core, no ? > Doing it hardcoded by architecture is just the simplest way to deal > with it, working on the assumption that nothing actually needs the > runtime detection that Ben suggested. No, it's not an archicture problem. It's a problem specific to that one SoC that the device was synthetized to be a certain endian while it was synthetized differently on another SoC... that also happens to be a different architecture. But doesn't have to. For example, we had in the past cases of both LE and BE EHCI implementations on the same architecture (PowerPC). > Detecting the endianess of the > device is probably the best future-proof solution, but it's also > considerably more work to do in the driver, and comes with a > tiny runtime overhead. The runtime overhead is probably non-measurable compared with the cost of the actual MMIOs. Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4
On Mon, 2016-05-09 at 13:39 +0300, Felipe Balbi wrote: > and patch all drivers similarly? Shouldn't arch/mips itself deal with > it and hide it from drivers ? Not sure what you mean, but we never had "endian neutral" accessors. It would be a bit of an endeavour and we already have so many accessors that adding more need a very strong justification. Most IP blocks have a fixed endian... Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4
On Monday 09 May 2016 14:11:23 John Youn wrote: > On 5/9/2016 1:39 PM, Arnd Bergmann wrote: > > > The systems are not a particular endianess, only the current state > > of the CPU is, and that may change independent of the way the > > hardware block got synthesized. We don't support swapping endianess > > at runtime in Linux, but the system normally doesn't care what we > > run. > > > > The normal behavior is for the register contents to be read as > > little-endian, and then swapped on big-endian kernel builds to > > match what the kernel expects. > > > > MIPS is a special case here, because the endianess of the CPU > > core is fixed in hardware (or using a strapping pin) and is often > > tied to the endianess of all the IP blocks. There are a couple > > of other architectures like this (e.g. ARM ixp4xx, but none of the > > modern ARM systems). > > Ok thanks. What you're saying is clear now. > > Is there a standard way to handle this? Must all drivers either check > some endianness configuration or do a runtime check? There are four common approaches: 1 hardcode a particular endianess because an IP block is always used the same way 2 pick the right endianess at compile-time because we know what we are building for (typically by CPU architecture, but there are some other ways) 3 do runtime configuration based on a DT property or platform data 4 do automatic runtime configuration based on well-known register contents Approach 1 is the most common, in particular as most IP blocks are not configurable and are not used on big-endian MIPS machines. Approach 4 as implemented by Christian Lamparter's patch is the most reliable way, and probably makes most sense for DesignWare IP blocks in general, because they are configurable and may get used in all kinds of systems. Approaches 2 and 3 are somewhat inferior because you have to rely on everyone else doing it the way you planned, but 2 is particularly easy and 3 is used when there is no obvious way to detect endianess of the device. The patch I suggested would be approach 2, and as Felipe correctly mentioned, it is not ideal, but it may be more appropriate for backports to the 4.4-longterm, 4.5-stable and 4.6-stable kernels than the more elaborate patch we are now discussing. Arnd ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4
On 5/9/2016 1:39 PM, Arnd Bergmann wrote: > On Monday 09 May 2016 13:22:48 John Youn wrote: >> On 5/9/2016 3:36 AM, Arnd Bergmann wrote: >>> On Monday 09 May 2016 10:23:22 Benjamin Herrenschmidt wrote: On Sun, 2016-05-08 at 13:44 +0200, Christian Lamparter wrote: > On Sunday, May 08, 2016 08:40:55 PM Benjamin Herrenschmidt wrote: >> >> On Sun, 2016-05-08 at 00:54 +0200, Christian Lamparter via Linuxppc-dev >> wrote: >>> >>> I've been looking in getting the MyBook Live Duo's USB OTG port >>> to function. The SoC is a APM82181. Which has a PowerPC 464 core >>> and related to the supported canyonlands architecture in >>> arch/powerpc/. >>> >>> Currently in -next the dwc2 module doesn't load: >> Smells like the APM implementation is little endian. You might need to >> use a flag to indicate what endian to use instead and set it >> appropriately based on some DT properties. > I tried. As per common-properties[0], I added little-endian; but it has no > effect. I looked in dwc2_driver_probe and found no way of specifying the > endian of the device. It all comes down to the dwc2_readl & dwc2_writel > accessors. These - sadly - have been hardwired to use __raw_readl and > __raw_writel. So, it's always "native-endian". While common-properties > says little-endian should be preferred. Right, I meant, you should produce a patch adding a runtime test inside those functions based on a device-tree property, a bit like we do for some of the HCDs like OHCI, EHCI etc... >>> >>> The patch that caused the problem had multiple issues: >>> >>> - it broke big-endian ARM kernels: any machine that was working >>> correctly with a little-endian kernel is no longer using byteswaps >>> on big-endian kernels, which clearly breaks them. >> >> >> I'm a bit confused about how this is supposed to work. My >> understanding was that the readl() and writel() are defined as little >> endian. So byte-swapping was performed if the architecture is big >> endian. And the raw versions never swapped, always using the "native" >> endianness. >> >> dwc2 is always treating the result of readl/writel as if it was read >> in native endian. So it needs to read the registers in big-endian on >> big-endian systems. > > The hardware has no idea of what endianess the CPU uses at any > given time, it's fixed by the SoC design, so there is no such > thing as "native" endianess for a random IP block. > > The readl/writel accessors accomodate for that by swapping the > data on big-endian kernels, because most SoC designers tend to > pick little-endian device registers by default. > >> This was the premise on which this patch was made. >> >> So for big endian systems, isn't what we want is to read in big-endian >> without any byteswapping to little-endian? But your saying this breaks >> big-endian ARM systems as well. Am I missing something? > > The systems are not a particular endianess, only the current state > of the CPU is, and that may change independent of the way the > hardware block got synthesized. We don't support swapping endianess > at runtime in Linux, but the system normally doesn't care what we > run. > > The normal behavior is for the register contents to be read as > little-endian, and then swapped on big-endian kernel builds to > match what the kernel expects. > > MIPS is a special case here, because the endianess of the CPU > core is fixed in hardware (or using a strapping pin) and is often > tied to the endianess of all the IP blocks. There are a couple > of other architectures like this (e.g. ARM ixp4xx, but none of the > modern ARM systems). Ok thanks. What you're saying is clear now. Is there a standard way to handle this? Must all drivers either check some endianness configuration or do a runtime check? Regards, John ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4
On Monday 09 May 2016 13:22:48 John Youn wrote: > On 5/9/2016 3:36 AM, Arnd Bergmann wrote: > > On Monday 09 May 2016 10:23:22 Benjamin Herrenschmidt wrote: > >> On Sun, 2016-05-08 at 13:44 +0200, Christian Lamparter wrote: > >>> On Sunday, May 08, 2016 08:40:55 PM Benjamin Herrenschmidt wrote: > > On Sun, 2016-05-08 at 00:54 +0200, Christian Lamparter via Linuxppc-dev > wrote: > > > > I've been looking in getting the MyBook Live Duo's USB OTG port > > to function. The SoC is a APM82181. Which has a PowerPC 464 core > > and related to the supported canyonlands architecture in > > arch/powerpc/. > > > > Currently in -next the dwc2 module doesn't load: > Smells like the APM implementation is little endian. You might need to > use a flag to indicate what endian to use instead and set it > appropriately based on some DT properties. > >>> I tried. As per common-properties[0], I added little-endian; but it has no > >>> effect. I looked in dwc2_driver_probe and found no way of specifying the > >>> endian of the device. It all comes down to the dwc2_readl & dwc2_writel > >>> accessors. These - sadly - have been hardwired to use __raw_readl and > >>> __raw_writel. So, it's always "native-endian". While common-properties > >>> says little-endian should be preferred. > >> > >> Right, I meant, you should produce a patch adding a runtime test inside > >> those functions based on a device-tree property, a bit like we do for > >> some of the HCDs like OHCI, EHCI etc... > >> > >> > > > > The patch that caused the problem had multiple issues: > > > > - it broke big-endian ARM kernels: any machine that was working > > correctly with a little-endian kernel is no longer using byteswaps > > on big-endian kernels, which clearly breaks them. > > > I'm a bit confused about how this is supposed to work. My > understanding was that the readl() and writel() are defined as little > endian. So byte-swapping was performed if the architecture is big > endian. And the raw versions never swapped, always using the "native" > endianness. > > dwc2 is always treating the result of readl/writel as if it was read > in native endian. So it needs to read the registers in big-endian on > big-endian systems. The hardware has no idea of what endianess the CPU uses at any given time, it's fixed by the SoC design, so there is no such thing as "native" endianess for a random IP block. The readl/writel accessors accomodate for that by swapping the data on big-endian kernels, because most SoC designers tend to pick little-endian device registers by default. > This was the premise on which this patch was made. > > So for big endian systems, isn't what we want is to read in big-endian > without any byteswapping to little-endian? But your saying this breaks > big-endian ARM systems as well. Am I missing something? The systems are not a particular endianess, only the current state of the CPU is, and that may change independent of the way the hardware block got synthesized. We don't support swapping endianess at runtime in Linux, but the system normally doesn't care what we run. The normal behavior is for the register contents to be read as little-endian, and then swapped on big-endian kernel builds to match what the kernel expects. MIPS is a special case here, because the endianess of the CPU core is fixed in hardware (or using a strapping pin) and is often tied to the endianess of all the IP blocks. There are a couple of other architectures like this (e.g. ARM ixp4xx, but none of the modern ARM systems). Arnd ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4
On 5/9/2016 3:36 AM, Arnd Bergmann wrote: > On Monday 09 May 2016 10:23:22 Benjamin Herrenschmidt wrote: >> On Sun, 2016-05-08 at 13:44 +0200, Christian Lamparter wrote: >>> On Sunday, May 08, 2016 08:40:55 PM Benjamin Herrenschmidt wrote: On Sun, 2016-05-08 at 00:54 +0200, Christian Lamparter via Linuxppc-dev wrote: > > I've been looking in getting the MyBook Live Duo's USB OTG port > to function. The SoC is a APM82181. Which has a PowerPC 464 core > and related to the supported canyonlands architecture in > arch/powerpc/. > > Currently in -next the dwc2 module doesn't load: Smells like the APM implementation is little endian. You might need to use a flag to indicate what endian to use instead and set it appropriately based on some DT properties. >>> I tried. As per common-properties[0], I added little-endian; but it has no >>> effect. I looked in dwc2_driver_probe and found no way of specifying the >>> endian of the device. It all comes down to the dwc2_readl & dwc2_writel >>> accessors. These - sadly - have been hardwired to use __raw_readl and >>> __raw_writel. So, it's always "native-endian". While common-properties >>> says little-endian should be preferred. >> >> Right, I meant, you should produce a patch adding a runtime test inside >> those functions based on a device-tree property, a bit like we do for >> some of the HCDs like OHCI, EHCI etc... >> >> > > The patch that caused the problem had multiple issues: > > - it broke big-endian ARM kernels: any machine that was working > correctly with a little-endian kernel is no longer using byteswaps > on big-endian kernels, which clearly breaks them. I'm a bit confused about how this is supposed to work. My understanding was that the readl() and writel() are defined as little endian. So byte-swapping was performed if the architecture is big endian. And the raw versions never swapped, always using the "native" endianness. dwc2 is always treating the result of readl/writel as if it was read in native endian. So it needs to read the registers in big-endian on big-endian systems. This was the premise on which this patch was made. So for big endian systems, isn't what we want is to read in big-endian without any byteswapping to little-endian? But your saying this breaks big-endian ARM systems as well. Am I missing something? The rest of the feedback makes sense. Just confused about this bit. Regards, John ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: perf tools: add support for generating bpf prologue on powerpc
Em Sat, May 07, 2016 at 02:15:36PM +1000, Michael Ellerman escreveu: > On Thu, 2016-05-05 at 15:23:19 UTC, "Naveen N. Rao" wrote: > > Generalize existing macros to serve the purpose. > > > > Cc: Wang Nan > > Cc: Arnaldo Carvalho de Melo > > Cc: Masami Hiramatsu > > Cc: Ian Munsie > > Cc: Michael Ellerman > > Signed-off-by: Naveen N. Rao > > --- > > With this patch: > > # ./perf test 37 > > 37: Test BPF filter : > > 37.1: Test basic BPF filtering : Ok > > 37.2: Test BPF prologue generation : Ok > > 37.3: Test BPF relocation checker: Ok > > > > tools/perf/arch/powerpc/Makefile | 1 + > > tools/perf/arch/powerpc/util/dwarf-regs.c | 40 > > +-- > > 2 files changed, 29 insertions(+), 12 deletions(-) > > Looks feasible, and is in powerpc only code, should I take this or acme? Its upstream already :-) - Arnaldo ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4
On Monday 09 May 2016 21:06:07 Christian Lamparter wrote: > Uh, Thanks for the participation! > > On Monday, May 09, 2016 05:08:48 PM Arnd Bergmann wrote: > > On Monday 09 May 2016 13:39:50 Felipe Balbi wrote: > > > Arnd Bergmann writes: > > > > On Monday 09 May 2016 10:23:22 Benjamin Herrenschmidt wrote: > > > >> On Sun, 2016-05-08 at 13:44 +0200, Christian Lamparter wrote: > > > > > > > > The patch that caused the problem had multiple issues: > > > > > > > > - it broke big-endian ARM kernels: any machine that was working > > > > correctly with a little-endian kernel is no longer using byteswaps > > > > on big-endian kernels, which clearly breaks them. > > > > - On PowerPC the same thing must be true: if it was working before, > > > > using big-endian kernels is now broken. Unlike ARM, 32-bit PowerPC > > > > usually uses big-endian kernels, so they are likely all broken. > > > > - The barrier for dwc2_writel is on the wrong side of the > > > > __raw_writel(), > > > > so the MMIO no longer synchronizes with DMA operations. > > > > - On architectures that require specific CPU instructions for MMIO > > > > access, using the __raw_ variant may turn this into a pointer > > > > dereference that does not have the same effect as the readl/writel. > > > > > > > > I think we can simply make this set of accessors architecture-dependent > > > > (MIPS vs. the rest of the world) to revert ARM and PowerPC back to > > > > the working version. > > > > > > and patch all drivers similarly? Shouldn't arch/mips itself deal with it > > > and hide it from drivers ? > > > > > > > Unfortunately, I don't see any way this could be done in MIPS specific > > code: There is typically a byteswap between the internal bus and the PCI > > bus on big-endian MIPS systems, so the PCI MMIO ends up being little-endian, > > which matches the expected behavior of readl/writel. However, drivers > > for non-PCI devices often use the same readl/writel accessors because > > that is how it's done on ARMv6/ARMv7. > > > > Doing it hardcoded by architecture is just the simplest way to deal > > with it, working on the assumption that nothing actually needs the > > runtime detection that Ben suggested. Detecting the endianess of the > > device is probably the best future-proof solution, but it's also > > considerably more work to do in the driver, and comes with a > > tiny runtime overhead. > > > Ok, just to have it on the table. I went ahead and implemented the > "Detect Endian". > > I looked in the DWC USB OTG's Databook documents v3.30a (If someone wants > them too, PM me). If I read the Application Interface Feature list on > page 30 correctly. The endianess is selectable by a "pin" That said > I don't know which one is it in the APM82181 or any other arch. I looked > around for configuration registers and stuff but unlike DesignWare's AHB > DMA Controller, there's no Bit in the "User HW Config Registers" that > would tell us if it was configured as big-endian or little-endian at > the moment. > > One way out would be to detect the endianess automatically by looking at > the values in the GSNPSID register. This is a read-only register containing > the release number of the core being used. The "upper" 16-bits of it are > hardcoded to 0x4f45 (The comment in dwc2_get_hwparams [1] has it backwards > but not the code below). Good, that should work. > I ran into the following issues: > - gadget.c uses ioread32_rep [0] & iowrite32_rep [1]. > This is interesting because both of these functions actually use > the __raw_io* on powerpc. This is because powerpc uses the default > defines of include/asm-generic/io.h [2]. > > Ideally, this should be done by sth like a writesl_be or writesl(e) > function. But I found none so for now: Let's make a ugly hack: > to_correct_endian that will work for testing, but will be replaced. You must have gotten this wrong: writesl() and the other variants are used to copy byte streams, which are always in the correct endianess, i.e. copying them one byte at a time should result in the same in-memory representation as copying them four bytes at a time. You should never need an additional swap in here. > - is_little_endian (do we want a separate is_big_endian?) > Also, do we want to be able to overwrite the detection code > if the endian setting was set in the device tree?. For now > it always does auto detection (see dwc2_detect_endiannes() ). I'd say that detecting endianess from the register is the safest approach. We can also parse the standard DT properties to do the same, but they can easily be stale, especially when the driver has already gotten this wrong for some users. > ( - 80 character per line issues, is it possible to drop the >hsotg->reg + REGISTER from the dwc2_readl/writel since we >pass the hsotg now anyway and do the reg + REGISTER >calculat
Re: [PATCH v2 00/23] ata: sata_dwc_460ex: make it working again
On Mon, May 9, 2016 at 10:05 PM, Tejun Heo wrote: > On Mon, May 09, 2016 at 10:13:59AM +0100, Måns Rullgård wrote: >> Andy Shevchenko writes: >> >> > On Mon, May 9, 2016 at 4:09 AM, Tejun Heo wrote: >> >> On Sun, May 08, 2016 at 04:00:08PM -0400, Tejun Heo wrote: >> >>> Hello, Andy. >> >>> >> >>> On Wed, May 04, 2016 at 03:22:51PM +0300, Andy Shevchenko wrote: >> >>> > Tejun, since Vinod applied all necessary patches into his tree, the >> >>> > series now has just a dependency to whatever branch / tag he marks for >> >>> > it. >> >>> > Do we have a chance to see the SATA series be applied in your tree? >> >>> >> >>> Applied 1-22 to libata/for-4.7. There was a couple trivial conflicts >> >>> which I resolved while applying but it'd be great if you can check >> >>> whether everything looks okay. >> >>> >> >>> >> >>> https://git.kernel.org/cgit/linux/kernel/git/tj/libata.git/log/?h=for-4.7 >> >> >> >> Oops, build failure. Reverted for now. >> > >> > I suppose you have to pull material from Vinod. >> >> The failure the build bot reported is due to the DMA patches not being >> in the tree. > > Which tree should I pull? slave-dma [1], branch topic/dw. But I think Vinod can tell us which tag/branch will be immutable. Vinod? [1] http://git.infradead.org/users/vkoul/slave-dma.git -- With Best Regards, Andy Shevchenko ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 00/23] ata: sata_dwc_460ex: make it working again
On Mon, May 09, 2016 at 10:13:59AM +0100, Måns Rullgård wrote: > Andy Shevchenko writes: > > > On Mon, May 9, 2016 at 4:09 AM, Tejun Heo wrote: > >> On Sun, May 08, 2016 at 04:00:08PM -0400, Tejun Heo wrote: > >>> Hello, Andy. > >>> > >>> On Wed, May 04, 2016 at 03:22:51PM +0300, Andy Shevchenko wrote: > >>> > Tejun, since Vinod applied all necessary patches into his tree, the > >>> > series now has just a dependency to whatever branch / tag he marks for > >>> > it. > >>> > Do we have a chance to see the SATA series be applied in your tree? > >>> > >>> Applied 1-22 to libata/for-4.7. There was a couple trivial conflicts > >>> which I resolved while applying but it'd be great if you can check > >>> whether everything looks okay. > >>> > >>> https://git.kernel.org/cgit/linux/kernel/git/tj/libata.git/log/?h=for-4.7 > >> > >> Oops, build failure. Reverted for now. > > > > I suppose you have to pull material from Vinod. > > The failure the build bot reported is due to the DMA patches not being > in the tree. Which tree should I pull? Thanks. -- tejun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: barriers: was: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model
On Wed, 4 May 2016, Josh Poimboeuf wrote: > On Wed, May 04, 2016 at 04:12:05PM +0200, Petr Mladek wrote: > > On Wed 2016-05-04 14:39:40, Petr Mladek wrote: > > >* > > >* Note that the task must never be migrated to the target > > >* state when being inside this ftrace handler. > > >*/ > > > > > > We might want to move the second paragraph on top of the function. > > > It is a basic and important fact. It actually explains why the first > > > read barrier is not needed when the patch is being disabled. > > > > I wrote the statement partly intuitively. I think that it is really > > somehow important. And I am slightly in doubts if we are on the safe side. > > > > First, why is it important that the task->patch_state is not switched > > when being inside the ftrace handler? > > > > If we are inside the handler, we are kind-of inside the called > > function. And the basic idea of this consistency model is that > > we must not switch a task when it is inside a patched function. > > This is normally decided by the stack. > > > > The handler is a bit special because it is called right before the > > function. If it was the only patched function on the stack, it would > > not matter if we choose the new or old code. Both decisions would > > be safe for the moment. > > > > The fun starts when the function calls another patched function. > > The other patched function must be called consistently with > > the first one. If the first function was from the patch, > > the other must be from the patch as well and vice versa. > > > > This is why we must not switch task->patch_state dangerously > > when being inside the ftrace handler. > > > > Now I am not sure if this condition is fulfilled. The ftrace handler > > is called as the very first instruction of the function. Does not > > it break the stack validity? Could we sleep inside the ftrace > > handler? Will the patched function be detected on the stack? > > > > Or is my brain already too far in the fantasy world? > > I think this isn't a possibility. > > In today's code base, this can't happen because task patch states are > only switched when sleeping or when exiting the kernel. The ftrace > handler doesn't sleep directly. > > If it were preempted, it couldn't be switched there either because we > consider preempted stacks to be unreliable. And IIRC ftrace handlers cannot sleep and are called with preemption disabled as of now. The code is a bit obscure, but see __ftrace_ops_list_func for example. This is "main" ftrace handler that calls all the registered ones in case FTRACE_OPS_FL_DYNAMIC is set (which is always true for handlers coming from modules) and CONFIG_PREEMPT is on. If it is off and there is only one handler registered for a function dynamic trampoline is used. See commit 12cce594fa8f ("ftrace/x86: Allow !CONFIG_PREEMPT dynamic ops to use allocated trampolines"). I think Steven had a plan to implement dynamic trampolines even for CONFIG_PREEMPT case but he still hasn't done it. It should use RCU_TASKS infrastructure. The reason for all the mess is that ftrace needs to be sure that no task is in the handler when the handler/trampoline is freed. So we should be safe for now even from this side. Miroslav ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH-next v2 0/2] cpufreq: powernv: Fixes for Global pstate management
On 03-05-16, 20:49, Akshay Adiga wrote: > Fixes are based on patch https://patchwork.ozlabs.org/patch/612058/ which > is in Rafael's linux-next. > > - Patch [1] fixes WARN_ON in powernv_target_index() > - Patch [2] Deleting any pending timer to saves an unnecessary irq call > in powernv_target_index() > > Akshay Adiga (2): > cpufreq: powernv: Move smp_call_function_any() out of irq safe block > cpufreq: powernv: del_timer_sync when global and local pstate are > equal Acked-by: Viresh Kumar -- viresh ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4
On Monday 09 May 2016 13:39:50 Felipe Balbi wrote: > Arnd Bergmann writes: > > On Monday 09 May 2016 10:23:22 Benjamin Herrenschmidt wrote: > >> On Sun, 2016-05-08 at 13:44 +0200, Christian Lamparter wrote: > > > > The patch that caused the problem had multiple issues: > > > > - it broke big-endian ARM kernels: any machine that was working > > correctly with a little-endian kernel is no longer using byteswaps > > on big-endian kernels, which clearly breaks them. > > - On PowerPC the same thing must be true: if it was working before, > > using big-endian kernels is now broken. Unlike ARM, 32-bit PowerPC > > usually uses big-endian kernels, so they are likely all broken. > > - The barrier for dwc2_writel is on the wrong side of the __raw_writel(), > > so the MMIO no longer synchronizes with DMA operations. > > - On architectures that require specific CPU instructions for MMIO > > access, using the __raw_ variant may turn this into a pointer > > dereference that does not have the same effect as the readl/writel. > > > > I think we can simply make this set of accessors architecture-dependent > > (MIPS vs. the rest of the world) to revert ARM and PowerPC back to > > the working version. > > and patch all drivers similarly? Shouldn't arch/mips itself deal with it > and hide it from drivers ? > Unfortunately, I don't see any way this could be done in MIPS specific code: There is typically a byteswap between the internal bus and the PCI bus on big-endian MIPS systems, so the PCI MMIO ends up being little-endian, which matches the expected behavior of readl/writel. However, drivers for non-PCI devices often use the same readl/writel accessors because that is how it's done on ARMv6/ARMv7. Doing it hardcoded by architecture is just the simplest way to deal with it, working on the assumption that nothing actually needs the runtime detection that Ben suggested. Detecting the endianess of the device is probably the best future-proof solution, but it's also considerably more work to do in the driver, and comes with a tiny runtime overhead. Arnd ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4
On Mon, 2016-05-09 at 12:36 +0200, Arnd Bergmann wrote: > > I think we can simply make this set of accessors architecture- > dependent > (MIPS vs. the rest of the world) to revert ARM and PowerPC back to > the working version. Or use writel_be which mips seems to support... Really, make it a BE vs. LE device test is a much better solution. For now, since dwc2_readl() and writel don't take the device as an argument, you can make it a function of a compile time #define, or maybe a driver global, but the right way is really something like if (device_is_be()) return readl_be(...) else return readl(...) With the device_is_be() being temporarily set to true for MIPS for example, and later, a second pass, add the device argument and make it a device-flag initialized from the probe routine, possibly from the DT. Cheers, Ben. > Signed-off-by: Arnd Bergmann > > diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h > index 3c58d633ce80..1f8ed149a40f 100644 > --- a/drivers/usb/dwc2/core.h > +++ b/drivers/usb/dwc2/core.h > @@ -64,12 +64,24 @@ > DWC2_TRACE_SCHEDULER_VB(pr_fmt("%s: SCH: " fmt), > \ > dev_name(hsotg->dev), ##__VA_ARGS__) > > + > +#ifdef CONFIG_MIPS > +/* > + * There are some MIPS machines that can run in either big-endian > + * or little-endian mode and that use the dwc2 register without > + * a byteswap in both ways. > + * Unlike other architectures, MIPS does not require a barrier > + * before the __raw_writel() to synchronize with DMA but does > + * require the barrier after the writel() to serialize a series > + * of writes. This set of operations was added specifically for > + * MIPS and should only be used there. > + */ > static inline u32 dwc2_readl(const void __iomem *addr) > { > u32 value = __raw_readl(addr); > > - /* In order to preserve endianness __raw_* operation is > used. Therefore > - * a barrier is needed to ensure IO access is not re-ordered > across > + /* in order to preserve endianness __raw_* operation is > used. therefore > + * a barrier is needed to ensure io access is not re-ordered > across > * reads or writes > */ > mb(); > @@ -81,15 +93,32 @@ static inline void dwc2_writel(u32 value, void > __iomem *addr) > __raw_writel(value, addr); > > /* > - * In order to preserve endianness __raw_* operation is > used. Therefore > - * a barrier is needed to ensure IO access is not re-ordered > across > + * in order to preserve endianness __raw_* operation is > used. therefore > + * a barrier is needed to ensure io access is not re-ordered > across > * reads or writes > */ > mb(); > -#ifdef DWC2_LOG_WRITES > - pr_info("INFO:: wrote %08x to %p\n", value, addr); > +#ifdef dwc2_log_writes > + pr_info("info:: wrote %08x to %p\n", value, addr); > #endif > } > +#else > +/* Normal architectures just use readl/write */ > +static inline u32 dwc2_readl(const void __iomem *addr) > +{ > + u32 value = readl(addr); > + return value; > +} > + > +static inline void dwc2_writel(u32 value, void __iomem *addr) > +{ > + writel(value, addr); > + > +#ifdef dwc2_log_writes > + pr_info("info:: wrote %08x to %p\n", value, addr); > +#endif > +} > +#endif > > /* Maximum number of Endpoints/HostChannels */ > #define MAX_EPS_CHANNELS 16 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V10 00/28] Add new powerpc specific ELF core notes
On 05/06/2016 05:19 PM, Michael Ellerman wrote: > On Tue, 2016-02-16 at 14:29 +0530, Anshuman Khandual wrote: > >> >This patch series adds twelve new ELF core note sections which can >> > be used with existing ptrace request PTRACE_GETREGSET-SETREGSET for >> > accessing >> > various transactional memory and other miscellaneous debug register sets on >> > powerpc platform. >> > >> > Test Result (All tests pass on both BE and LE) >> > -- >> > ptrace-ebb PASS >> > ptrace-gpr PASS >> > ptrace-tm-gpr PASS >> > ptrace-tm-spd-gpr PASS >> > ptrace-tar PASS >> > ptrace-tm-tar PASS >> > ptrace-tm-spd-tar PASS >> > ptrace-vsx PASS >> > ptrace-tm-vsx PASS >> > ptrace-tm-spd-vsx PASS >> > ptrace-tm-spr PASS > How are you building the tests? On BE I get: Yeah, it fails to compile on 32 bit BE. Will make it explicit -m64 next time around. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH-next v2 0/2] cpufreq: powernv: Fixes for Global pstate management
On 05/03/2016 08:49 PM, Akshay Adiga wrote: Fixes are based on patch https://patchwork.ozlabs.org/patch/612058/ which is in Rafael's linux-next. - Patch [1] fixes WARN_ON in powernv_target_index() - Patch [2] Deleting any pending timer to saves an unnecessary irq call in powernv_target_index() Akshay Adiga (2): cpufreq: powernv: Move smp_call_function_any() out of irq safe block cpufreq: powernv: del_timer_sync when global and local pstate are equal drivers/cpufreq/powernv-cpufreq.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) Does this look good ? Anything i should do ? Regards Akshay Adiga ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: klp_task_patch: was: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model
On Fri 2016-05-06 07:38:55, Josh Poimboeuf wrote: > On Thu, May 05, 2016 at 01:57:01PM +0200, Petr Mladek wrote: > > I have missed that the two commands are called with preemption > > disabled. So, I had the following crazy scenario in mind: > > > > > > CPU0CPU1 > > > > klp_enable_patch() > > > > klp_target_state = KLP_PATCHED; > > > > for_each_task() > > set TIF_PENDING_PATCH > > > > # task 123 > > > > if (klp_patch_pending(current) > > klp_patch_task(current) > > > > clear TIF_PENDING_PATCH > > > > smp_rmb(); > > > > # switch to assembly of > > # klp_patch_task() > > > > mov klp_target_state, %r12 > > > > # interrupt and schedule > > # another task > > > > > > klp_reverse_transition(); > > > > klp_target_state = KLP_UNPATCHED; > > > > klt_try_to_complete_transition() > > > > task = 123; > > if (task->patch_state == klp_target_state; > > return 0; > > > > => task 123 is in target state and does > > not block conversion > > > > klp_complete_transition() > > > > > > # disable previous patch on the stack > > klp_disable_patch(); > > > > klp_target_state = KLP_UNPATCHED; > > > > > > # task 123 gets scheduled again > > lea %r12, task->patch_state > > > > => it happily stores an outdated > > state > > > > Thanks for the clear explanation, this helps a lot. > > > This is why the two functions should get called with preemption > > disabled. We should document it at least. I imagine that we will > > use them later also in another context and nobody will remember > > this crazy scenario. > > > > Well, even disabled preemption does not help. The process on > > CPU1 might be also interrupted by an NMI and do some long > > printk in it. > > > > IMHO, the only safe approach is to call klp_patch_task() > > only for "current" on a safe place. Then this race is harmless. > > The switch happen on a safe place, so that it does not matter > > into which state the process is switched. > > I'm not sure about this solution. When klp_complete_transition() is > called, we need all tasks to be patched, for good. We don't want any of > them to randomly switch to the wrong state at some later time in the > middle of a future patch operation. How would changing klp_patch_task() > to only use "current" prevent that? You are right that it is pity but it really should be safe because it is not entirely random. If the race happens and assign an outdated value, there are two situations: 1. It is assigned when there is not transition in the progress. Then it is OK because it will be ignored by the ftrace handler. The right state will be set before the next transition starts. 2. It is assigned when some other transition is in progress. Then it is OK as long as the function is called from "current". The "wrong" state will be used consistently. It will switch to the right state on another safe state. > > By other words, the task state might be updated only > > > >+ by the task itself on a safe place > >+ by other task when the updated on is sleeping on a safe place > > > > This should be well documented and the API should help to avoid > > a misuse. > > I think we could fix it to be safe for future callers who might not have > preemption disabled with a couple of changes to klp_patch_task(): > disabling preemption and testing/clearing the TIF_PATCH_PENDING flag > before changing the patch state: > > void klp_patch_task(struct task_struct *task) > { > preempt_disable(); > > if (test_and_clear_tsk_thread_flag(task, TIF_PATCH_PENDING)) > task->patch_state = READ_ONCE(klp_target_state); > > preempt_enable(); > } It reduces the race window a bit but it is still there. For example, NMI still might add a huge delay between reading klp_target_state and assigning task->patch state. What about the following? /* * This function might assign an outdated value if the transaction `* is reverted and finalized in parallel. But it is safe. If the value * is assigned outside of a transaction, it is ignored and the next * transaction will set the right one. Or if it gets assigned * inside another transaction, it will repeat the cycle and * set the right state. */ void klp_update_current_patch_state() { while (test_and_clear_tsk_thread_flag(current, TIF_PATCH_PENDING)) current->patch_state = READ_ONCE(klp_target_state); } Note that the disabled preemption helped only partially, so I think that
Re: [PATCH v2 1/2] powerpc/timer - large decrementer support
> > > +static inline bool large_dec_supp(void) > > > +{ > > > + return cpu_has_feature(CPU_FTR_ARCH_300); > > > +} > > > + > > Can we rename this to is_large_dec()? > Honestly, I don't like either. How about have_large_dec() ? How about cpu_has_large_dec() like the underlying call? Mikey ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4
Hi, Arnd Bergmann writes: > On Monday 09 May 2016 10:23:22 Benjamin Herrenschmidt wrote: >> On Sun, 2016-05-08 at 13:44 +0200, Christian Lamparter wrote: >> > On Sunday, May 08, 2016 08:40:55 PM Benjamin Herrenschmidt wrote: >> > > >> > > On Sun, 2016-05-08 at 00:54 +0200, Christian Lamparter via Linuxppc-dev >> > > wrote: >> > > > >> > > > I've been looking in getting the MyBook Live Duo's USB OTG port >> > > > to function. The SoC is a APM82181. Which has a PowerPC 464 core >> > > > and related to the supported canyonlands architecture in >> > > > arch/powerpc/. >> > > > >> > > > Currently in -next the dwc2 module doesn't load: >> > > Smells like the APM implementation is little endian. You might need to >> > > use a flag to indicate what endian to use instead and set it >> > > appropriately based on some DT properties. >> > I tried. As per common-properties[0], I added little-endian; but it has no >> > effect. I looked in dwc2_driver_probe and found no way of specifying the >> > endian of the device. It all comes down to the dwc2_readl & dwc2_writel >> > accessors. These - sadly - have been hardwired to use __raw_readl and >> > __raw_writel. So, it's always "native-endian". While common-properties >> > says little-endian should be preferred. >> >> Right, I meant, you should produce a patch adding a runtime test inside >> those functions based on a device-tree property, a bit like we do for >> some of the HCDs like OHCI, EHCI etc... >> >> > > The patch that caused the problem had multiple issues: > > - it broke big-endian ARM kernels: any machine that was working > correctly with a little-endian kernel is no longer using byteswaps > on big-endian kernels, which clearly breaks them. > - On PowerPC the same thing must be true: if it was working before, > using big-endian kernels is now broken. Unlike ARM, 32-bit PowerPC > usually uses big-endian kernels, so they are likely all broken. > - The barrier for dwc2_writel is on the wrong side of the __raw_writel(), > so the MMIO no longer synchronizes with DMA operations. > - On architectures that require specific CPU instructions for MMIO > access, using the __raw_ variant may turn this into a pointer > dereference that does not have the same effect as the readl/writel. > > I think we can simply make this set of accessors architecture-dependent > (MIPS vs. the rest of the world) to revert ARM and PowerPC back to > the working version. and patch all drivers similarly? Shouldn't arch/mips itself deal with it and hide it from drivers ? -- balbi signature.asc Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 4/8] powerpc: add io{read,write}64 accessors
On 5/5/2016 6:37 PM, Horia Geantă wrote: > This will allow device drivers to consistently use io{read,write}XX > also for 64-bit accesses. > > Signed-off-by: Horia Geantă It would be great if PPC maintainers could Ack this patch. As stated in the cover letter: https://lkml.org/lkml/2016/5/5/340 I'd like to go with the whole patch set via cryptodev-2.6 tree. Thanks, Horia > --- > arch/powerpc/kernel/iomap.c | 24 > 1 file changed, 24 insertions(+) > > diff --git a/arch/powerpc/kernel/iomap.c b/arch/powerpc/kernel/iomap.c > index 12e48d56f771..3963f0b68d52 100644 > --- a/arch/powerpc/kernel/iomap.c > +++ b/arch/powerpc/kernel/iomap.c > @@ -38,6 +38,18 @@ EXPORT_SYMBOL(ioread16); > EXPORT_SYMBOL(ioread16be); > EXPORT_SYMBOL(ioread32); > EXPORT_SYMBOL(ioread32be); > +#ifdef __powerpc64__ > +u64 ioread64(void __iomem *addr) > +{ > + return readq(addr); > +} > +u64 ioread64be(void __iomem *addr) > +{ > + return readq_be(addr); > +} > +EXPORT_SYMBOL(ioread64); > +EXPORT_SYMBOL(ioread64be); > +#endif /* __powerpc64__ */ > > void iowrite8(u8 val, void __iomem *addr) > { > @@ -64,6 +76,18 @@ EXPORT_SYMBOL(iowrite16); > EXPORT_SYMBOL(iowrite16be); > EXPORT_SYMBOL(iowrite32); > EXPORT_SYMBOL(iowrite32be); > +#ifdef __powerpc64__ > +void iowrite64(u64 val, void __iomem *addr) > +{ > + writeq(val, addr); > +} > +void iowrite64be(u64 val, void __iomem *addr) > +{ > + writeq_be(val, addr); > +} > +EXPORT_SYMBOL(iowrite64); > +EXPORT_SYMBOL(iowrite64be); > +#endif /* __powerpc64__ */ > > /* > * These are the "repeat read/write" functions. Note the > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4
On Monday 09 May 2016 10:23:22 Benjamin Herrenschmidt wrote: > On Sun, 2016-05-08 at 13:44 +0200, Christian Lamparter wrote: > > On Sunday, May 08, 2016 08:40:55 PM Benjamin Herrenschmidt wrote: > > > > > > On Sun, 2016-05-08 at 00:54 +0200, Christian Lamparter via Linuxppc-dev > > > wrote: > > > > > > > > I've been looking in getting the MyBook Live Duo's USB OTG port > > > > to function. The SoC is a APM82181. Which has a PowerPC 464 core > > > > and related to the supported canyonlands architecture in > > > > arch/powerpc/. > > > > > > > > Currently in -next the dwc2 module doesn't load: > > > Smells like the APM implementation is little endian. You might need to > > > use a flag to indicate what endian to use instead and set it > > > appropriately based on some DT properties. > > I tried. As per common-properties[0], I added little-endian; but it has no > > effect. I looked in dwc2_driver_probe and found no way of specifying the > > endian of the device. It all comes down to the dwc2_readl & dwc2_writel > > accessors. These - sadly - have been hardwired to use __raw_readl and > > __raw_writel. So, it's always "native-endian". While common-properties > > says little-endian should be preferred. > > Right, I meant, you should produce a patch adding a runtime test inside > those functions based on a device-tree property, a bit like we do for > some of the HCDs like OHCI, EHCI etc... > > The patch that caused the problem had multiple issues: - it broke big-endian ARM kernels: any machine that was working correctly with a little-endian kernel is no longer using byteswaps on big-endian kernels, which clearly breaks them. - On PowerPC the same thing must be true: if it was working before, using big-endian kernels is now broken. Unlike ARM, 32-bit PowerPC usually uses big-endian kernels, so they are likely all broken. - The barrier for dwc2_writel is on the wrong side of the __raw_writel(), so the MMIO no longer synchronizes with DMA operations. - On architectures that require specific CPU instructions for MMIO access, using the __raw_ variant may turn this into a pointer dereference that does not have the same effect as the readl/writel. I think we can simply make this set of accessors architecture-dependent (MIPS vs. the rest of the world) to revert ARM and PowerPC back to the working version. Signed-off-by: Arnd Bergmann diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index 3c58d633ce80..1f8ed149a40f 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -64,12 +64,24 @@ DWC2_TRACE_SCHEDULER_VB(pr_fmt("%s: SCH: " fmt),\ dev_name(hsotg->dev), ##__VA_ARGS__) + +#ifdef CONFIG_MIPS +/* + * There are some MIPS machines that can run in either big-endian + * or little-endian mode and that use the dwc2 register without + * a byteswap in both ways. + * Unlike other architectures, MIPS does not require a barrier + * before the __raw_writel() to synchronize with DMA but does + * require the barrier after the writel() to serialize a series + * of writes. This set of operations was added specifically for + * MIPS and should only be used there. + */ static inline u32 dwc2_readl(const void __iomem *addr) { u32 value = __raw_readl(addr); - /* In order to preserve endianness __raw_* operation is used. Therefore -* a barrier is needed to ensure IO access is not re-ordered across + /* in order to preserve endianness __raw_* operation is used. therefore +* a barrier is needed to ensure io access is not re-ordered across * reads or writes */ mb(); @@ -81,15 +93,32 @@ static inline void dwc2_writel(u32 value, void __iomem *addr) __raw_writel(value, addr); /* -* In order to preserve endianness __raw_* operation is used. Therefore -* a barrier is needed to ensure IO access is not re-ordered across +* in order to preserve endianness __raw_* operation is used. therefore +* a barrier is needed to ensure io access is not re-ordered across * reads or writes */ mb(); -#ifdef DWC2_LOG_WRITES - pr_info("INFO:: wrote %08x to %p\n", value, addr); +#ifdef dwc2_log_writes + pr_info("info:: wrote %08x to %p\n", value, addr); #endif } +#else +/* Normal architectures just use readl/write */ +static inline u32 dwc2_readl(const void __iomem *addr) +{ + u32 value = readl(addr); + return value; +} + +static inline void dwc2_writel(u32 value, void __iomem *addr) +{ + writel(value, addr); + +#ifdef dwc2_log_writes + pr_info("info:: wrote %08x to %p\n", value, addr); +#endif +} +#endif /* Maximum number of Endpoints/HostChannels */ #define MAX_EPS_CHANNELS 16 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [1/2] powerpc/mm: Remove duplicated check in do_page_fault()
On Fri, 2016-26-02 at 00:26:25 UTC, Gavin Shan wrote: > When the page fault happened in user space, we need check it's > caused by stack frame pointer update instruction and update > local variable @flag with FAULT_FLAG_USER. Currently, the code > has two separate check for the same condition. That's unnecessary. > > This removes one of the duplicated check. No functinal changes > introduced. It's possible though that store_updates_sp() changes regs, and causes user_mode(regs) to change, which would mean the second check is necessary. That's not true with the current code, but you should mention that you confirmed that in the change log. > diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c > index a67c6d7..935f386 100644 > --- a/arch/powerpc/mm/fault.c > +++ b/arch/powerpc/mm/fault.c > @@ -294,11 +294,10 @@ int __kprobes do_page_fault(struct pt_regs *regs, > unsigned long address, >* can result in fault, which will cause a deadlock when called with >* mmap_sem held >*/ > - if (user_mode(regs)) > - store_update_sp = store_updates_sp(regs); > - > - if (user_mode(regs)) > + if (user_mode(regs)) { > flags |= FAULT_FLAG_USER; > + store_update_sp = store_updates_sp(regs); > + } It doesn't really matter in this case, but it would be better to keep the ordering of the two statements the same as before. cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc: Update of_remove_property() call sites to remove null checking
On Fri, 2016-05-06 at 13:00 +1000, Suraj Jitindar Singh wrote: > On 05/05/16 16:50, Michael Ellerman wrote: > > On Tue, 2016-05-03 at 15:32 -0700, Tyrel Datwyler wrote: > > > On 04/27/2016 10:34 PM, Suraj Jitindar Singh wrote: > > > > diff --git a/arch/powerpc/platforms/pseries/mobility.c > > > > b/arch/powerpc/platforms/pseries/mobility.c > > > > index ceb18d3..a560a98 100644 > > > > --- a/arch/powerpc/platforms/pseries/mobility.c > > > > +++ b/arch/powerpc/platforms/pseries/mobility.c > > > > @@ -191,8 +191,8 @@ static int update_dt_node(__be32 phandle, s32 scope) > > > > break; > > > > > > > > case 0x8000: > > > > - prop = of_find_property(dn, prop_name, > > > > NULL); > > > > - of_remove_property(dn, prop); > > > > + of_remove_property(dn, > > > > of_find_property(dn, > > > > + prop_name, > > > > NULL)); > > > > prop = NULL; > > > > break; > > > > > > > You haven't removed a NULL check here, as suggested by the changelog, > > > but instead made a cosmetic change to the code that still leaves behind > > > a now unnecessary "prop = NULL;" to bit rot. > > Yeah I think you're right. Though it's not very clear how prop is used in > > that > > function. > > I didn't delete the prop = NULL; initially as I didn't fully understand > how it was used in the rest of the function and the effect of deleting > it. Yeah, it's pretty convoluted. I don't think you can actually prove it's safe to remove the prop = NULL for arbitrary inputs. cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model
[...] > +static int klp_target_state; [...] > +void klp_init_transition(struct klp_patch *patch, int state) > +{ > + struct task_struct *g, *task; > + unsigned int cpu; > + struct klp_object *obj; > + struct klp_func *func; > + int initial_state = !state; > + > + klp_transition_patch = patch; > + > + /* > + * If the patch can be applied or reverted immediately, skip the > + * per-task transitions. > + */ > + if (patch->immediate) > + return; > + > + /* > + * Initialize all tasks to the initial patch state to prepare them for > + * switching to the target state. > + */ > + read_lock(&tasklist_lock); > + for_each_process_thread(g, task) > + task->patch_state = initial_state; > + read_unlock(&tasklist_lock); > + > + /* > + * Ditto for the idle "swapper" tasks. > + */ > + get_online_cpus(); > + for_each_online_cpu(cpu) > + idle_task(cpu)->patch_state = initial_state; > + put_online_cpus(); > + > + /* > + * Ensure klp_ftrace_handler() sees the task->patch_state updates > + * before the func->transition updates. Otherwise it could read an > + * out-of-date task state and pick the wrong function. > + */ > + smp_wmb(); > + > + /* > + * Set the func transition states so klp_ftrace_handler() will know to > + * switch to the transition logic. > + * > + * When patching, the funcs aren't yet in the func_stack and will be > + * made visible to the ftrace handler shortly by the calls to > + * klp_patch_object(). > + * > + * When unpatching, the funcs are already in the func_stack and so are > + * already visible to the ftrace handler. > + */ > + klp_for_each_object(patch, obj) > + klp_for_each_func(obj, func) > + func->transition = true; > + > + /* > + * Set the global target patch state which tasks will switch to. This > + * has no effect until the TIF_PATCH_PENDING flags get set later. > + */ > + klp_target_state = state; I am afraid there is a problem for (patch->immediate == true) patches. klp_target_state is not set for those and the comment is not entirely true, because klp_target_state has an effect in several places. [...] > +void klp_start_transition(void) > +{ > + struct task_struct *g, *task; > + unsigned int cpu; > + > + pr_notice("'%s': %s...\n", klp_transition_patch->mod->name, > + klp_target_state == KLP_PATCHED ? "patching" : "unpatching"); Here... > + > + /* > + * If the patch can be applied or reverted immediately, skip the > + * per-task transitions. > + */ > + if (klp_transition_patch->immediate) > + return; > + [...] > +bool klp_try_complete_transition(void) > +{ > + unsigned int cpu; > + struct task_struct *g, *task; > + bool complete = true; > + > + /* > + * If the patch can be applied or reverted immediately, skip the > + * per-task transitions. > + */ > + if (klp_transition_patch->immediate) > + goto success; > + > + /* > + * Try to switch the tasks to the target patch state by walking their > + * stacks and looking for any to-be-patched or to-be-unpatched > + * functions. If such functions are found on a stack, or if the stack > + * is deemed unreliable, the task can't be switched yet. > + * > + * Usually this will transition most (or all) of the tasks on a system > + * unless the patch includes changes to a very common function. > + */ > + read_lock(&tasklist_lock); > + for_each_process_thread(g, task) > + if (!klp_try_switch_task(task)) > + complete = false; > + read_unlock(&tasklist_lock); > + > + /* > + * Ditto for the idle "swapper" tasks. > + */ > + get_online_cpus(); > + for_each_online_cpu(cpu) > + if (!klp_try_switch_task(idle_task(cpu))) > + complete = false; > + put_online_cpus(); > + > + /* > + * Some tasks weren't able to be switched over. Try again later and/or > + * wait for other methods like syscall barrier switching. > + */ > + if (!complete) > + return false; > + > +success: > + /* > + * When unpatching, all tasks have transitioned to KLP_UNPATCHED so we > + * can now remove the new functions from the func_stack. > + */ > + if (klp_target_state == KLP_UNPATCHED) { Here (this is the most important one I think). > + klp_unpatch_objects(klp_transition_patch); > + > + /* > + * Don't allow any existing instances of ftrace handlers to > + * access any obsolete funcs before we reset the func > + * transition states to false. Otherwise the handler may see > + * the deleted "new" func, see that it's not in tra
Re: [2/3] powerpc/fadump: add support to specify memory range based size
On 05/07/2016 09:41 AM, Michael Ellerman wrote: On Fri, 2016-06-05 at 11:50:37 UTC, Hari Bathini wrote: Currently, memory for fadump can be specified with fadump_reserve_mem=size, where only a fixed size can be specified. This patch tries to extend this syntax to support conditional reservation based on memory size, with the below syntax: fadump_reserve_mem=:[,:,...] This syntax helps using the same commandline parameter for different system memory sizes. This is basically using the crashkernel= syntax right? Yep. One of the typical crashkernel syntax.. So can we please reuse the crashkernel= parsing code? but crashkernel has a few other variants which don't make sense for fadump. To reuse the crashkernel parsing code for fadump, it needs little bit of refactoring. Will try to do that and respin.. cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [3/3] powerpc/fadump: add support for fadump_nr_cpus= parameter
On 05/07/2016 09:42 AM, Michael Ellerman wrote: On Fri, 2016-06-05 at 11:51:08 UTC, Hari Bathini wrote: Kernel parameter 'nr_cpus' can be used to limit the maximum number of processors that an SMP kernel could support. This patch extends this to fadump by introducing 'fadump_nr_cpus' parameter that can help in booting fadump kernel on a lower memory footprint. Is there really no other way to do this? I really hate adding new, single use only command line parameters. Hmmm.. only alternative I can think about is enforcing a certain nr_cpu_ids value whenever fadump is active, but that doesn't sound right.. Any suggestions? Thanks Hari cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 00/23] ata: sata_dwc_460ex: make it working again
Andy Shevchenko writes: > On Mon, May 9, 2016 at 4:09 AM, Tejun Heo wrote: >> On Sun, May 08, 2016 at 04:00:08PM -0400, Tejun Heo wrote: >>> Hello, Andy. >>> >>> On Wed, May 04, 2016 at 03:22:51PM +0300, Andy Shevchenko wrote: >>> > Tejun, since Vinod applied all necessary patches into his tree, the >>> > series now has just a dependency to whatever branch / tag he marks for >>> > it. >>> > Do we have a chance to see the SATA series be applied in your tree? >>> >>> Applied 1-22 to libata/for-4.7. There was a couple trivial conflicts >>> which I resolved while applying but it'd be great if you can check >>> whether everything looks okay. >>> >>> https://git.kernel.org/cgit/linux/kernel/git/tj/libata.git/log/?h=for-4.7 >> >> Oops, build failure. Reverted for now. > > I suppose you have to pull material from Vinod. The failure the build bot reported is due to the DMA patches not being in the tree. -- Måns Rullgård ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 00/23] ata: sata_dwc_460ex: make it working again
On Mon, May 9, 2016 at 4:09 AM, Tejun Heo wrote: > On Sun, May 08, 2016 at 04:00:08PM -0400, Tejun Heo wrote: >> Hello, Andy. >> >> On Wed, May 04, 2016 at 03:22:51PM +0300, Andy Shevchenko wrote: >> > Tejun, since Vinod applied all necessary patches into his tree, the >> > series now has just a dependency to whatever branch / tag he marks for >> > it. >> > Do we have a chance to see the SATA series be applied in your tree? >> >> Applied 1-22 to libata/for-4.7. There was a couple trivial conflicts >> which I resolved while applying but it'd be great if you can check >> whether everything looks okay. >> >> https://git.kernel.org/cgit/linux/kernel/git/tj/libata.git/log/?h=for-4.7 > > Oops, build failure. Reverted for now. I suppose you have to pull material from Vinod. -- With Best Regards, Andy Shevchenko ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 1/2] powerpc/timer - large decrementer support
On Mon, May 9, 2016 at 4:28 PM, Balbir Singh wrote: > > > On 04/05/16 17:37, Oliver O'Halloran wrote: >> POWER ISA v3 adds large decrementer (LD) mode of operation which increases >> the size of the decrementer register from 32 bits to an implementation >> defined with of up to 64 bits. >> >> This patch adds support for the LD on processors with the CPU_FTR_ARCH_300 >> cpu feature flag set. Even for CPUs with this feature LD mode is only >> enabled when the property ibm,dec-bits devicetree property is supplied >> for the boot CPU. The decrementer value is a signed quantity (with >> negative values indicating a pending exception) and this property is >> required to find the maximum positive decrementer value. If this property >> is not supplied then the traditional decrementer width of 32 bits is >> assumed and LD mode is disabled. >> >> This patch was based on initial work by Jack Miller. >> >> Signed-off-by: Oliver O'Halloran >> Cc: Jack Miller >> Cc: Balbir Singh >> --- >> arch/powerpc/include/asm/reg.h | 1 + >> arch/powerpc/include/asm/time.h | 6 +-- >> arch/powerpc/kernel/time.c | 89 >> + >> 3 files changed, 86 insertions(+), 10 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h >> index f5f4c66bbbc9..ff581ed1ab9d 100644 >> --- a/arch/powerpc/include/asm/reg.h >> +++ b/arch/powerpc/include/asm/reg.h >> @@ -332,6 +332,7 @@ >> #define LPCR_AIL_0 0x /* MMU off exception offset 0x0 */ >> #define LPCR_AIL_3 0x0180 /* MMU on exception offset >> 0xc00...4xxx */ >> #define LPCR_ONL 0x0004 /* online - PURR/SPURR count */ >> +#define LPCR_LD0x0002 /* large decremeter */ >> #define LPCR_PECE 0x0001f000 /* powersave exit cause enable */ >> #define LPCR_PECEDP 0x0001 /* directed priv dbells cause >> exit */ >> #define LPCR_PECEDH 0x8000 /* directed hyp dbells cause >> exit */ >> diff --git a/arch/powerpc/include/asm/time.h >> b/arch/powerpc/include/asm/time.h >> index 1092fdd7e737..09211640a0e0 100644 >> --- a/arch/powerpc/include/asm/time.h >> +++ b/arch/powerpc/include/asm/time.h >> @@ -146,7 +146,7 @@ static inline void set_tb(unsigned int upper, unsigned >> int lower) >> * in auto-reload mode. The problem is PIT stops counting when it >> * hits zero. If it would wrap, we could use it just like a decrementer. >> */ >> -static inline unsigned int get_dec(void) >> +static inline u64 get_dec(void) >> { >> #if defined(CONFIG_40x) >> return (mfspr(SPRN_PIT)); >> @@ -160,10 +160,10 @@ static inline unsigned int get_dec(void) >> * in when the decrementer generates its interrupt: on the 1 to 0 >> * transition for Book E/4xx, but on the 0 to -1 transition for others. >> */ >> -static inline void set_dec(int val) >> +static inline void set_dec(u64 val) >> { >> #if defined(CONFIG_40x) >> - mtspr(SPRN_PIT, val); >> + mtspr(SPRN_PIT, (u32) val); >> #else >> #ifndef CONFIG_BOOKE >> --val; >> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c >> index 81b0900a39ee..fab34abfb4cd 100644 >> --- a/arch/powerpc/kernel/time.c >> +++ b/arch/powerpc/kernel/time.c >> @@ -95,7 +95,8 @@ static struct clocksource clocksource_timebase = { >> .read = timebase_read, >> }; >> >> -#define DECREMENTER_MAX 0x7fff >> +#define DECREMENTER_DEFAULT_MAX 0x7FFF >> +u64 decrementer_max = DECREMENTER_DEFAULT_MAX; > > Should this be signed, given that the decrementer is signed? Maybe, but there's no particular reason to prefer signed since we (almost) never look at the decrementer value directly. I used u64 since the value to be loaded into the decrementer is calculated using a u64 and the explicit cast to int when calling set_dec() in __timer_interrupt() was the only place I could find where an int was expected. >> +static inline bool large_dec_supp(void) >> +{ >> + return cpu_has_feature(CPU_FTR_ARCH_300); >> +} >> + > > Can we rename this to is_large_dec()? Honestly, I don't like either. How about have_large_dec() ? >> +/* enables the large decrementer for the current CPU */ >> +static void enable_large_decrementer(void) >> +{ >> + /* do we have a large decrementer? */ >> + if (!large_dec_supp()) >> + return; >> + >> + /* do we need a large decrementer? */ >> + if (decrementer_max <= DECREMENTER_DEFAULT_MAX) >> + return; >> + >> + mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) | LPCR_LD); >> + >> + if (!large_dec_on()) { >> + decrementer_max = DECREMENTER_DEFAULT_MAX; >> + >> + pr_warn("time_init: Failed to enable LD mode on CPU %d\n", > > I think stating "Failed to enable Large Decrementer" might be easier to > understand in the logs You're right, I was just trying to keep it under 80 cols :) > >> + smp_processor_id()); >> + } >> +} >> + >> +static void __init set_de
Status of DPAA integration for NXP QoriQ?
Hello, the "fman" Ethernet driver was integrated in mainline Linux Dezember 2015 ("drivers/net/ethernet/freescale/fman"). It seems that the other parts, e.g. BMan, QMan ("drivers/soc/fsl/qbman") and basic DPAA Ethernet support ("drivers/net/ethernet/freescale/dpaa") are still missing. Are there any plans to integrate them? I didn't notice any activity on the mailing list in the last couple of months in this respect. -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.hu...@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Build regressions/improvements in v4.6-rc7
On Mon, May 9, 2016 at 10:24 AM, Geert Uytterhoeven wrote: > JFYI, when comparing v4.6-rc7[1] to v4.6-rc6[3], the summaries are: > - build errors: +188/-7 For a quiet -rc7, the results are devastating: + /home/kisskb/slave/src/arch/sh/kernel/setup.c: error: implicit declaration of function 'early_init_dt_scan' [-Werror=implicit-function-declaration]: => 256:2 sh-randconfig + /home/kisskb/slave/src/drivers/net/ethernet/3com/typhoon.c: error: case label does not reduce to an integer constant: => 1007:2, 1016:2, 1010:2, 1013:2, 1019:2 + /home/kisskb/slave/src/drivers/staging/rtl8192e/rtl8192e/rtl_core.c: error: (near initialization for 'def_qos_parameters.cw_max[0]'): => 372:2 + /home/kisskb/slave/src/drivers/staging/rtl8192e/rtl8192e/rtl_core.c: error: (near initialization for 'def_qos_parameters.cw_max[1]'): => 372:2 + /home/kisskb/slave/src/drivers/staging/rtl8192e/rtl8192e/rtl_core.c: error: (near initialization for 'def_qos_parameters.cw_max[2]'): => 372:2 + /home/kisskb/slave/src/drivers/staging/rtl8192e/rtl8192e/rtl_core.c: error: (near initialization for 'def_qos_parameters.cw_max[3]'): => 372:2 + /home/kisskb/slave/src/drivers/staging/rtl8192e/rtl8192e/rtl_core.c: error: (near initialization for 'def_qos_parameters.cw_min[0]'): => 371:2 + /home/kisskb/slave/src/drivers/staging/rtl8192e/rtl8192e/rtl_core.c: error: (near initialization for 'def_qos_parameters.cw_min[1]'): => 371:2 + /home/kisskb/slave/src/drivers/staging/rtl8192e/rtl8192e/rtl_core.c: error: (near initialization for 'def_qos_parameters.cw_min[2]'): => 371:2 + /home/kisskb/slave/src/drivers/staging/rtl8192e/rtl8192e/rtl_core.c: error: (near initialization for 'def_qos_parameters.cw_min[3]'): => 371:2 + /home/kisskb/slave/src/drivers/staging/rtl8192e/rtl8192e/rtl_core.c: error: initializer element is not constant: => 371:2, 372:2 + /home/kisskb/slave/src/drivers/staging/rtl8192u/r8192U_core.c: error: (near initialization for 'def_qos_parameters.cw_max[0]'): => 1870:2 + /home/kisskb/slave/src/drivers/staging/rtl8192u/r8192U_core.c: error: (near initialization for 'def_qos_parameters.cw_max[1]'): => 1870:2 + /home/kisskb/slave/src/drivers/staging/rtl8192u/r8192U_core.c: error: (near initialization for 'def_qos_parameters.cw_max[2]'): => 1870:2 + /home/kisskb/slave/src/drivers/staging/rtl8192u/r8192U_core.c: error: (near initialization for 'def_qos_parameters.cw_max[3]'): => 1870:2 + /home/kisskb/slave/src/drivers/staging/rtl8192u/r8192U_core.c: error: (near initialization for 'def_qos_parameters.cw_min[0]'): => 1869:2 + /home/kisskb/slave/src/drivers/staging/rtl8192u/r8192U_core.c: error: (near initialization for 'def_qos_parameters.cw_min[1]'): => 1869:2 + /home/kisskb/slave/src/drivers/staging/rtl8192u/r8192U_core.c: error: (near initialization for 'def_qos_parameters.cw_min[2]'): => 1869:2 + /home/kisskb/slave/src/drivers/staging/rtl8192u/r8192U_core.c: error: (near initialization for 'def_qos_parameters.cw_min[3]'): => 1869:2 + /home/kisskb/slave/src/drivers/staging/rtl8192u/r8192U_core.c: error: initializer element is not constant: => 1869:2, 1870:2 + /home/kisskb/slave/src/drivers/usb/chipidea/udc.c: error: (near initialization for 'ctrl_endpt_in_desc.wMaxPacketSize'): => 50:2 + /home/kisskb/slave/src/drivers/usb/chipidea/udc.c: error: (near initialization for 'ctrl_endpt_out_desc.wMaxPacketSize'): => 40:2 + /home/kisskb/slave/src/drivers/usb/chipidea/udc.c: error: initializer element is not constant: => 50:2, 40:2 + /home/kisskb/slave/src/drivers/usb/image/mdc800.c: error: (near initialization for 'mdc800_ed[0].wMaxPacketSize'): => 191:3 + /home/kisskb/slave/src/drivers/usb/image/mdc800.c: error: (near initialization for 'mdc800_ed[1].wMaxPacketSize'): => 201:3 + /home/kisskb/slave/src/drivers/usb/image/mdc800.c: error: (near initialization for 'mdc800_ed[2].wMaxPacketSize'): => 211:3 + /home/kisskb/slave/src/drivers/usb/image/mdc800.c: error: (near initialization for 'mdc800_ed[3].wMaxPacketSize'): => 221:3 + /home/kisskb/slave/src/drivers/usb/image/mdc800.c: error: initializer element is not constant: => 191:3, 211:3, 221:3, 201:3 + /home/kisskb/slave/src/fs/cifs/smb1ops.c: error: (near initialization for 'smb1_values.lock_cmd'): => 1110:2 + /home/kisskb/slave/src/fs/cifs/smb1ops.c: error: initializer element is not constant LD security/built-in.o: => 1110:2 + /home/kisskb/slave/src/fs/cifs/smb1ops.c: error: initializer element is not constant: => 1110:2 + /home/kisskb/slave/src/fs/hpfs/hpfs.h: error: enumerator value for 'FNODE_anode' is not an integer constant: => 435:7 + /home/kisskb/slave/src/fs/hpfs/hpfs.h: error: enumerator value for 'FNODE_dir' is not an integer constant: => 435:37 + /home/kisskb/slave/src/fs/ntfs/dir.c: error: (near initialization for 'I30[0]'): => 36:1 + /home/kisskb/slave/src/fs/ntfs/dir.c: error: (near initialization for 'I30[1]'): => 36:1 + /home/kisskb/slave/src/fs/ntfs/dir.c: error:
Re: [PATCH] kvm-pr: manage illegal instructions
On 21.04.2016 11:25, Thomas Huth wrote: > On 15.03.2016 21:18, Laurent Vivier wrote: >> While writing some instruction tests for kvm-unit-tests for powerpc, >> I've found that illegal instructions are not managed correctly with kvm-pr, >> while it is fine with kvm-hv. >> >> When an illegal instruction (like ".long 0") is processed by kvm-pr, >> the kernel logs are filled with: >> >> Couldn't emulate instruction 0x (op 0 xop 0) >> kvmppc_handle_exit_pr: emulation at 700 failed () >> >> While the exception handler receives an interrupt for each instruction >> executed after the illegal instruction. >> >> Signed-off-by: Laurent Vivier >> --- >> arch/powerpc/kvm/book3s_emulate.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/arch/powerpc/kvm/book3s_emulate.c >> b/arch/powerpc/kvm/book3s_emulate.c >> index 2afdb9c..4ee969d 100644 >> --- a/arch/powerpc/kvm/book3s_emulate.c >> +++ b/arch/powerpc/kvm/book3s_emulate.c >> @@ -99,7 +99,6 @@ int kvmppc_core_emulate_op_pr(struct kvm_run *run, struct >> kvm_vcpu *vcpu, >> >> switch (get_op(inst)) { >> case 0: >> -emulated = EMULATE_FAIL; >> if ((kvmppc_get_msr(vcpu) & MSR_LE) && >> (inst == swab32(inst_sc))) { >> /* >> @@ -112,6 +111,9 @@ int kvmppc_core_emulate_op_pr(struct kvm_run *run, >> struct kvm_vcpu *vcpu, >> kvmppc_set_gpr(vcpu, 3, EV_UNIMPLEMENTED); >> kvmppc_set_pc(vcpu, kvmppc_get_pc(vcpu) + 4); >> emulated = EMULATE_DONE; >> +} else { >> +kvmppc_core_queue_program(vcpu, SRR1_PROGILL); >> +emulated = EMULATE_AGAIN; >> } >> break; >> case 19: >> > > Tested-by: Thomas Huth Ping! Alex, Paul, could you please pick up this patch? This patch is required to get the kvm-unit-tests working properly with kvm-pr, so I'd be glad if we could get this included finally... Thanks, Thomas ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: perf tools: add support for generating bpf prologue on powerpc
On Sat, 2016-05-07 at 16:43 +0530, Naveen N. Rao wrote: > On 2016/05/07 02:15PM, Michael Ellerman wrote: > > On Thu, 2016-05-05 at 15:23:19 UTC, "Naveen N. Rao" wrote: > > > Generalize existing macros to serve the purpose. > > > > > > Cc: Wang Nan > > > Cc: Arnaldo Carvalho de Melo > > > Cc: Masami Hiramatsu > > > Cc: Ian Munsie > > > Cc: Michael Ellerman > > > Signed-off-by: Naveen N. Rao > > > --- > > > With this patch: > > > # ./perf test 37 > > > 37: Test BPF filter : > > > 37.1: Test basic BPF filtering : Ok > > > 37.2: Test BPF prologue generation : Ok > > > 37.3: Test BPF relocation checker: Ok > > > > > > tools/perf/arch/powerpc/Makefile | 1 + > > > tools/perf/arch/powerpc/util/dwarf-regs.c | 40 > > > +-- > > > 2 files changed, 29 insertions(+), 12 deletions(-) > > > > Looks feasible, and is in powerpc only code, should I take this or acme? > > Hi Michael, > Arnaldo has already pulled this in: > http://article.gmane.org/gmane.linux.kernel/2216051 Ah sorry. > It would be good if you can consider user stackdump as that depends on > perf regs support which you have added to powerpc-next: > http://thread.gmane.org/gmane.linux.kernel/2210299/focus=2210749 Yep I did, it's in next. https://git.kernel.org/cgit/linux/kernel/git/powerpc/linux.git/commit/?h=next&id=826dccfd238c3eeac379f5f637e5a3ddc7a4bc22 cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2] TTY: serial, handle platform_get_irq retval properly
platform_get_irq can fail, so we should handle negative value when returned. [v2] platform_get_irq can actually return zero on some platforms. So do not remove checks for irq == 0 there. Signed-off-by: Jiri Slaby Cc: Russell King Cc: "Uwe Kleine-König" Cc: Russell King Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Michael Ellerman Cc: Laxman Dewangan Cc: Stephen Warren Cc: Thierry Reding Cc: Alexandre Courbot Cc: linux-ser...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-te...@vger.kernel.org --- drivers/tty/serial/amba-pl011.c | 8 +++- drivers/tty/serial/fsl_lpuart.c | 8 +++- drivers/tty/serial/pmac_zilog.c | 2 +- drivers/tty/serial/serial-tegra.c | 7 ++- 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c index a2aa655f56c4..c70bb41800f1 100644 --- a/drivers/tty/serial/amba-pl011.c +++ b/drivers/tty/serial/amba-pl011.c @@ -2553,11 +2553,17 @@ static int sbsa_uart_probe(struct platform_device *pdev) if (!uap) return -ENOMEM; + ret = platform_get_irq(pdev, 0); + if (ret < 0) { + dev_err(&pdev->dev, "cannot obtain irq\n"); + return ret; + } + uap->port.irq = ret; + uap->reg_offset = vendor_sbsa.reg_offset; uap->vendor = &vendor_sbsa; uap->fifosize = 32; uap->port.iotype = vendor_sbsa.access_32b ? UPIO_MEM32 : UPIO_MEM; - uap->port.irq = platform_get_irq(pdev, 0); uap->port.ops = &sbsa_uart_pops; uap->fixed_baud = baudrate; diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c index 3d790033744e..7f95f782a485 100644 --- a/drivers/tty/serial/fsl_lpuart.c +++ b/drivers/tty/serial/fsl_lpuart.c @@ -1830,7 +1830,13 @@ static int lpuart_probe(struct platform_device *pdev) sport->port.dev = &pdev->dev; sport->port.type = PORT_LPUART; sport->port.iotype = UPIO_MEM; - sport->port.irq = platform_get_irq(pdev, 0); + ret = platform_get_irq(pdev, 0); + if (ret < 0) { + dev_err(&pdev->dev, "cannot obtain irq\n"); + return ret; + } + sport->port.irq = ret; + if (sport->lpuart32) sport->port.ops = &lpuart32_pops; else diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c index e156e39d620c..b24b0556f5a8 100644 --- a/drivers/tty/serial/pmac_zilog.c +++ b/drivers/tty/serial/pmac_zilog.c @@ -1720,7 +1720,7 @@ static int __init pmz_init_port(struct uart_pmac_port *uap) r_ports = platform_get_resource(uap->pdev, IORESOURCE_MEM, 0); irq = platform_get_irq(uap->pdev, 0); - if (!r_ports || !irq) + if (!r_ports || irq <= 0) return -ENODEV; uap->port.mapbase = r_ports->start; diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c index bee1e5867426..4c4674b51db9 100644 --- a/drivers/tty/serial/serial-tegra.c +++ b/drivers/tty/serial/serial-tegra.c @@ -1310,7 +1310,12 @@ static int tegra_uart_probe(struct platform_device *pdev) } u->iotype = UPIO_MEM32; - u->irq = platform_get_irq(pdev, 0); + ret = platform_get_irq(pdev, 0); + if (ret < 0) { + dev_err(&pdev->dev, "Couldn't get IRQ\n"); + return ret; + } + u->irq = ret; u->regshift = 2; ret = uart_add_one_port(&tegra_uart_driver, u); if (ret < 0) { -- 2.8.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev