[PATCH net-next v6 4/4] powerpc: tqm8548/tqm8xx: add and update CAN device nodes
This patch enables or updates support for the CC770 and AN82527 CAN controller on the TQM8548 and TQM8xx boards. CC: devicetree-disc...@lists.ozlabs.org CC: linuxppc-...@ozlabs.org CC: Kumar Gala ga...@kernel.crashing.org Signed-off-by: Wolfgang Grandegger w...@grandegger.com --- arch/powerpc/boot/dts/tqm8548-bigflash.dts | 19 ++- arch/powerpc/boot/dts/tqm8548.dts | 19 ++- arch/powerpc/boot/dts/tqm8xx.dts | 25 + 3 files changed, 53 insertions(+), 10 deletions(-) diff --git a/arch/powerpc/boot/dts/tqm8548-bigflash.dts b/arch/powerpc/boot/dts/tqm8548-bigflash.dts index 9452c3c..d918752 100644 --- a/arch/powerpc/boot/dts/tqm8548-bigflash.dts +++ b/arch/powerpc/boot/dts/tqm8548-bigflash.dts @@ -352,7 +352,7 @@ ranges = 0 0x0 0xfc00 0x0400 // NOR FLASH bank 1 1 0x0 0xf800 0x0800 // NOR FLASH bank 0 - 2 0x0 0xa300 0x8000 // CAN (2 x i82527) + 2 0x0 0xa300 0x8000 // CAN (2 x CC770) 3 0x0 0xa301 0x8000 // NAND FLASH ; @@ -393,18 +393,27 @@ }; /* Note: CAN support needs be enabled in U-Boot */ - can0@2,0 { - compatible = intel,82527; // Bosch CC770 + can@2,0 { + compatible = bosch,cc770; // Bosch CC770 reg = 2 0x0 0x100; interrupts = 4 1; interrupt-parent = mpic; + bosch,external-clock-frequency = 1600; + bosch,disconnect-rx1-input; + bosch,disconnect-tx1-output; + bosch,iso-low-speed-mux; + bosch,clock-out-frequency = 1600; }; - can1@2,100 { - compatible = intel,82527; // Bosch CC770 + can@2,100 { + compatible = bosch,cc770; // Bosch CC770 reg = 2 0x100 0x100; interrupts = 4 1; interrupt-parent = mpic; + bosch,external-clock-frequency = 1600; + bosch,disconnect-rx1-input; + bosch,disconnect-tx1-output; + bosch,iso-low-speed-mux; }; /* Note: NAND support needs to be enabled in U-Boot */ diff --git a/arch/powerpc/boot/dts/tqm8548.dts b/arch/powerpc/boot/dts/tqm8548.dts index 619776f..988d887 100644 --- a/arch/powerpc/boot/dts/tqm8548.dts +++ b/arch/powerpc/boot/dts/tqm8548.dts @@ -352,7 +352,7 @@ ranges = 0 0x0 0xfc00 0x0400 // NOR FLASH bank 1 1 0x0 0xf800 0x0800 // NOR FLASH bank 0 - 2 0x0 0xe300 0x8000 // CAN (2 x i82527) + 2 0x0 0xe300 0x8000 // CAN (2 x CC770) 3 0x0 0xe301 0x8000 // NAND FLASH ; @@ -393,18 +393,27 @@ }; /* Note: CAN support needs be enabled in U-Boot */ - can0@2,0 { - compatible = intel,82527; // Bosch CC770 + can@2,0 { + compatible = bosch,cc770; // Bosch CC770 reg = 2 0x0 0x100; interrupts = 4 1; interrupt-parent = mpic; + bosch,external-clock-frequency = 1600; + bosch,disconnect-rx1-input; + bosch,disconnect-tx1-output; + bosch,iso-low-speed-mux; + bosch,clock-out-frequency = 1600; }; - can1@2,100 { - compatible = intel,82527; // Bosch CC770 + can@2,100 { + compatible = bosch,cc770; // Bosch CC770 reg = 2 0x100 0x100; interrupts = 4 1; interrupt-parent = mpic; + bosch,external-clock-frequency = 1600; + bosch,disconnect-rx1-input; + bosch,disconnect-tx1-output; + bosch,iso-low-speed-mux; }; /* Note: NAND support needs to be enabled in U-Boot */ diff --git a/arch/powerpc/boot/dts/tqm8xx.dts b/arch/powerpc/boot/dts/tqm8xx.dts index f6da7ec..c3dba25 100644 --- a/arch/powerpc/boot/dts/tqm8xx.dts +++ b/arch/powerpc/boot/dts/tqm8xx.dts @@ -57,6 +57,7 @@ ranges = 0x0 0x0 0x4000 0x80 + 0x3 0x0 0xc000 0x200 ; flash@0,0 { @@ -67,6 +68,30 @@
[PATCH net-next v6 3/4] can: cc770: add platform bus driver for the CC770 and AN82527
This driver works with both, static platform data and device tree bindings. It has been tested on a TQM855L board with two AN82527 CAN controllers on the local bus. CC: devicetree-disc...@lists.ozlabs.org CC: linuxppc-...@ozlabs.org CC: Kumar Gala ga...@kernel.crashing.org Signed-off-by: Wolfgang Grandegger w...@grandegger.com Acked-by: Marc Kleine-Budde m...@pengutronix.de --- .../devicetree/bindings/net/can/cc770.txt | 53 drivers/net/can/cc770/Kconfig |7 + drivers/net/can/cc770/Makefile |1 + drivers/net/can/cc770/cc770_platform.c | 272 4 files changed, 333 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/can/cc770.txt create mode 100644 drivers/net/can/cc770/cc770_platform.c diff --git a/Documentation/devicetree/bindings/net/can/cc770.txt b/Documentation/devicetree/bindings/net/can/cc770.txt new file mode 100644 index 000..77027bf --- /dev/null +++ b/Documentation/devicetree/bindings/net/can/cc770.txt @@ -0,0 +1,53 @@ +Memory mapped Bosch CC770 and Intel AN82527 CAN controller + +Note: The CC770 is a CAN controller from Bosch, which is 100% +compatible with the old AN82527 from Intel, but with bugs being fixed. + +Required properties: + +- compatible : should be bosch,cc770 for the CC770 and intc,82527 + for the AN82527. + +- reg : should specify the chip select, address offset and size required + to map the registers of the controller. The size is usually 0x80. + +- interrupts : property with a value describing the interrupt source + (number and sensitivity) required for the controller. + +Optional properties: + +- bosch,external-clock-frequency : frequency of the external oscillator + clock in Hz. Note that the internal clock frequency used by the + controller is half of that value. If not specified, a default + value of 1600 (16 MHz) is used. + +- bosch,clock-out-frequency : slock frequency in Hz on the CLKOUT pin. + If not specified or if the specified value is 0, the CLKOUT pin + will be disabled. + +- bosch,slew-rate : slew rate of the CLKOUT signal. If not specified, + a resonable value will be calculated. + +- bosch,disconnect-rx0-input : see data sheet. + +- bosch,disconnect-rx1-input : see data sheet. + +- bosch,disconnect-tx1-output : see data sheet. + +- bosch,polarity-dominant : see data sheet. + +- bosch,divide-memory-clock : see data sheet. + +- bosch,iso-low-speed-mux : see data sheet. + +For further information, please have a look to the CC770 or AN82527. + +Examples: + +can@3,100 { + compatible = bosch,cc770; + reg = 3 0x100 0x80; + interrupts = 2 0; + interrupt-parent = mpic; + bosch,external-clock-frequency = 1600; +}; diff --git a/drivers/net/can/cc770/Kconfig b/drivers/net/can/cc770/Kconfig index 28e4d48..22c07a8 100644 --- a/drivers/net/can/cc770/Kconfig +++ b/drivers/net/can/cc770/Kconfig @@ -11,4 +11,11 @@ config CAN_CC770_ISA connected to the ISA bus using I/O port, memory mapped or indirect access. +config CAN_CC770_PLATFORM + tristate Generic Platform Bus based CC770 driver + ---help--- + This driver adds support for the CC770 and AN82527 chips + connected to the platform bus (Linux abstraction for directly + to the processor attached devices). + endif diff --git a/drivers/net/can/cc770/Makefile b/drivers/net/can/cc770/Makefile index 872ecff..9fb8321 100644 --- a/drivers/net/can/cc770/Makefile +++ b/drivers/net/can/cc770/Makefile @@ -4,5 +4,6 @@ obj-$(CONFIG_CAN_CC770) += cc770.o obj-$(CONFIG_CAN_CC770_ISA) += cc770_isa.o +obj-$(CONFIG_CAN_CC770_PLATFORM) += cc770_platform.o ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG diff --git a/drivers/net/can/cc770/cc770_platform.c b/drivers/net/can/cc770/cc770_platform.c new file mode 100644 index 000..53115ee --- /dev/null +++ b/drivers/net/can/cc770/cc770_platform.c @@ -0,0 +1,272 @@ +/* + * Driver for CC770 and AN82527 CAN controllers on the platform bus + * + * Copyright (C) 2009, 2011 Wolfgang Grandegger w...@grandegger.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the version 2 of the GNU General Public License + * as published by the Free Software Foundation + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +/* + * If platform data are used you should have similar definitions + * in your board-specific code: + * + * static struct cc770_platform_data myboard_cc770_pdata = { + * .osc_freq = 1600, + * .cir = 0x41, + * .cor = 0x20, + * .bcr = 0x40, + * }; + * + * Please see include/linux/can/platform/cc770.h
Different behaviour when using nosmp parameter on SMP and UP
Hi, I have a P2020 CPU (powerpc) and I compiled it with two different defconfigs. The first one is a SMP, 2 cores, launched with the nosmp kernel parameter, the other one is an UP kernel. My driver behaviour is not the same whether launching one or the other. It is hard to explain more precisely, as it deals only with ioctl which only does ioread32/iowrite32 on a PCIe device. But I can tell that it never works the same way when UP (not working correctly), or SMP nosmp (working) or even SMP (not working). AFAIK, the nosmp parameter should tell the kernel to act the same is if it is an UP kernel, and it disables IO APIC, which is not an issue in my case. Can you think about anything that would explain it, or would help me debugging it ? Thanks in advance for your help. Regards, JM ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] [hw-breakpoint] Use generic hw-breakpoint interfaces for new PPC ptrace flags
On Mon, Nov 28, 2011 at 02:11:11PM +1100, David Gibson wrote: [snip] On Wed, Oct 12, 2011 at 11:09:48PM +0530, K.Prasad wrote: + if (bp) { + attr = bp-attr; + attr.bp_addr = (unsigned long)bp_info-addr ~HW_BREAKPOINT_ALIGN; + arch_bp_generic_fields(dabr + (DABR_DATA_WRITE | DABR_DATA_READ), + attr.bp_type); + attr.bp_len = len; If gdb is using the new breakpoint interface, surely it should just use it, rather than doing this bit frobbing as in the old SET_DABR call. I understand that you wanted to avoid this duplication of effort in terms of encoding and decoding the breakpoint type from PPC_BREAKPOINT_TRIGGER_READ to DABR_DATA_READ to HW_BREAKPOINT_R. However HW_BREAKPOINT_R is a generic definition used across architectures, DABR_DATA_READ is used in the !CONFIG_HAVE_HW_BREAKPOINT case while PPC_BREAKPOINT_TRIGGER_READ is used in CONFIG_PPC_ADV_DEBUG_REGS case. While we could define PPC_BREAKPOINT_TRIGGER_READ and DABR_DATA_READ to the same value it may not result in any code savings (since the bit translation is done for !CONFIG_HAVE_HW_BREAKPOINT case anyway). So, I think it is best left the way it is. That's not what I'm suggesting. What I'm saying is that ig userspace is using the new generic interface, then it should just set the bp_type field, and it should not use the DABR_DATA_{READ,WRITE} bits. The DABR_DATA bits should *only* be processed in the legacy interface, never in the generic interface. The DABR_DATA_{READ,WRITE} bits are neither set by the user, nor expected by the hw-breakpoint interface. It is an intermediate code used to re-use the arch_bp_generic_fields function. We could convert directly from PPC_BREAKPOINT_TRIGGER_READ to HW_BREAKPOINT_R (using a switch-case) but that may not result in any code savings. DABR_DATA_{READ,WRITE} is indeed legacy and cannot be set by user-space for a PPC_PTRACE_SETHWDEBUG + CONFIG_HAVE_HW_BREAKPOINT combination. [snipped] diff --git a/Documentation/powerpc/ptrace.txt b/Documentation/powerpc/ptrace.txt index f4a5499..f2a7a39 100644 --- a/Documentation/powerpc/ptrace.txt +++ b/Documentation/powerpc/ptrace.txt @@ -127,6 +127,22 @@ Some examples of using the structure to: p.addr2 = (uint64_t) end_range; p.condition_value = 0; +- set a watchpoint in server processors (BookS) + + p.version = 1; + p.trigger_type= PPC_BREAKPOINT_TRIGGER_RW; + p.addr_mode = PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE; + or + p.addr_mode = PPC_BREAKPOINT_MODE_EXACT; + + p.condition_mode = PPC_BREAKPOINT_CONDITION_NONE; + p.addr= (uint64_t) begin_range; You should probably document the alignment constraint on the address here, too. Alignment constraints will be learnt by the user-space during runtime. We provide that as part of 'struct ppc_debug_info' in 'data_bp_alignment' field. While the alignment is always 8-bytes for BookS, I think userspace should be left to learn it through PTRACE_PPC_GETHWDEBUGINFO. + /* For PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE addr2 needs to be specified, where + * addr2 - addr = 8 Bytes. + */ + p.addr2 = (uint64_t) end_range; + p.condition_value = 0; + 3. PTRACE_DELHWDEBUG Takes an integer which identifies an existing breakpoint or watchpoint diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index 05b7dd2..be5dc57 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -1339,6 +1339,12 @@ static int set_dac_range(struct task_struct *child, static long ppc_set_hwdebug(struct task_struct *child, struct ppc_hw_breakpoint *bp_info) { +#ifdef CONFIG_HAVE_HW_BREAKPOINT + int ret, len = 0; + struct thread_struct *thread = (child-thread); + struct perf_event *bp; + struct perf_event_attr attr; +#endif /* CONFIG_HAVE_HW_BREAKPOINT */ #ifndef CONFIG_PPC_ADV_DEBUG_REGS unsigned long dabr; #endif @@ -1382,13 +1388,9 @@ static long ppc_set_hwdebug(struct task_struct *child, */ if ((bp_info-trigger_type PPC_BREAKPOINT_TRIGGER_RW) == 0 || (bp_info-trigger_type ~PPC_BREAKPOINT_TRIGGER_RW) != 0 || - bp_info-addr_mode != PPC_BREAKPOINT_MODE_EXACT || bp_info-condition_mode != PPC_BREAKPOINT_CONDITION_NONE) return -EINVAL; - if (child-thread.dabr) - return -ENOSPC; - if ((unsigned long)bp_info-addr = TASK_SIZE) return -EIO; @@ -1398,15 +1400,75 @@ static long ppc_set_hwdebug(struct task_struct *child, dabr |= DABR_DATA_READ; if (bp_info-trigger_type PPC_BREAKPOINT_TRIGGER_WRITE) dabr |= DABR_DATA_WRITE; +#ifdef
Re: [BUG?]3.0-rc4+ftrace+kprobe: set kprobe at instruction 'stwu' lead to system crash/freeze
Benjamin Herrenschmidt wrote: On Wed, 2011-11-30 at 19:06 +0800, tiejun.chen wrote: - Copy the exception frame we're about to unwind to just -below- the new r1 value we want to write to. Then perform the write, and change r1 to point to that copy of the frame. - Branch to restore: which will unwind everything from that copy of the frame, and eventually set r1 to GPR(1) in the copy which contains the new value of r1. We still can't restore this there. Yes, we can since we have copied the pt_regs down to -below- where we are going to write to. That's the whole trick. We copy the pt_regs somewhere safe and restore from there. I mean we have to do that real restore here. So I'm really not sure if its a good way to add such a codes including check TIF/copy-get new r1/restore operation here since this is so deep for the exception return code. No. Re-read my explanation. In the do_work case (so when things are still easy), we copy the pt_regs of the return frame to a safe place (right below where we want to write to typically), and change r1 to point to this new frame which we use to restore from. Then we do the store which is now safe and go to an unmodified restore exit path. Do you mean we should push the original pt_regs (or that whole exception stack) downwards the location the new r1 point? Then its safe to perform this real emulated stw instruction. At last we will reroute r1 to that copied exception frame to restore as normal. Right? Here I suppose so, I implement this for PPC32 based on the above understanding. I take a validation for kprobing do_fork()/show_interrupts(), now looks fine. Tomorrow I will go PPC64, and hope its fine as well. If everything is good I'll send these patches to linuxppc-dev next week. Cheers Tiejun This is the less intrusive approach and should work just fine, it's also more robust than anything I've been able to think of and the approach would work for 32 and 64-bit similarily. (***) Above comment about a bug: If you look at entry_64.S version of ret_from_except_lite you'll notice that in the !preempt case, after we've checked MSR_PR we test for any TIF flag in _TIF_USER_WORK_MASK to decide whether to go to do_work or not. However, in the preempt case, we do a convoluted trick to test SIGPENDING only if PR was set and always test NEED_RESCHED ... but we forget to test any other bit of _TIF_USER_WORK_MASK !!! So that means that with preempt, we completely fail to test for things like single step, syscall tracing, etc... This is another problem we should address. I think this should be fixed at the same time, by simplifying the code by doing: - Test PR. If set, go to test_work_user, else continue (or the other way around and call it test_work_kernel) - In test_work_user, always test for _TIF_USER_WORK_MASK to decide to go to do_work, maybe call it do_user_work - In test_work_kernel, test for _TIF_KERNEL_WORK_MASK which is set to our new flag along with NEED_RESCHED if preempt is enabled and branch to do_kernel_work. do_user_work is basically the same as today's user_work do_kernel_work is basically the same as today preempt block with added code to handle the new flag as described above. Is anybody volunteering for fixing that ? I don't have the bandwidth I always use one specific kprobe stack to fix this for BOOKE and work well in my local tree :) Do you remember my v3 patch? I think its possible to extend this for all PPC variants. Anyway, I'd like to be this volunteer with our last solution. So the second problem I exposed, for which you just volunteered (hint :-) is an orthogonal issue not related to kprobe or stacks which happen to be something I discovered yesterday while looking at the code. As for the solution to the emulation problem, re-read my explanation. The whole trick is that we can avoid a separate stack (which I really want to avoid) and we can avoid touching the low level restore code path. Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: ppc4xx simple vs SoC's
On Thu, Dec 1, 2011 at 12:39 AM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: Hi Josh ! I was helping Tony with some of the Currituck stuff when I noticed an oddity... So we have various SoC config symbols such as 440EP, 460SX, etc... that in turn select various bits pieces that enable support for that SoC (such as EMAC4 support or FPU support). Those only act as enables for compiling of the code, there is still going to be a runtime check of course. Those SoC config symbols are not user selectable, they are meant to be themselves select'ed by the board Kconfig entries. Yep, and they are. However, the entry for ppc44x_simple doesn't select any of these, meaning for example, AFAIK, that you don't get EMAC4 etc... I'm surprised things work at all ! What am I missing ? CONFIG_PPC44x_SIMPLE is selected by the board Kconfig values, which also select all of the SoC Kconfig options they need as well. ppc44x_simple started as a way to basically avoid duplicating a bunch of board.c files, but it's still driven from the board options. So hopefully that explains how things are working today. The question now is whether you would like something different? josh ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Different behaviour when using nosmp parameter on SMP and UP
On Dec 1, 2011, at 3:57 AM, Jean-Michel Hautbois wrote: Hi, I have a P2020 CPU (powerpc) and I compiled it with two different defconfigs. The first one is a SMP, 2 cores, launched with the nosmp kernel parameter, the other one is an UP kernel. My driver behaviour is not the same whether launching one or the other. It is hard to explain more precisely, as it deals only with ioctl which only does ioread32/iowrite32 on a PCIe device. But I can tell that it never works the same way when UP (not working correctly), or SMP nosmp (working) or even SMP (not working). AFAIK, the nosmp parameter should tell the kernel to act the same is if it is an UP kernel, and it disables IO APIC, which is not an issue in my case. Can you think about anything that would explain it, or would help me debugging it ? This is a bit odd, hard to say w/o more details on what exactly your code is trying to do and wait the failure is. The larger differences between SMP UP build would be setting of memory attribute for cache coherency. In UP case we don't set it. Between SMP 'nosmp' and SMP 'on' cases you seem like you're hitting some locking condition. Guessing your running into a timing locking issue that you're getting lucky on the SMP 'nosmp' case such that it just happens to work. - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Different behaviour when using nosmp parameter on SMP and UP
2011/12/1 Kumar Gala ga...@kernel.crashing.org: On Dec 1, 2011, at 3:57 AM, Jean-Michel Hautbois wrote: Hi, I have a P2020 CPU (powerpc) and I compiled it with two different defconfigs. The first one is a SMP, 2 cores, launched with the nosmp kernel parameter, the other one is an UP kernel. My driver behaviour is not the same whether launching one or the other. It is hard to explain more precisely, as it deals only with ioctl which only does ioread32/iowrite32 on a PCIe device. But I can tell that it never works the same way when UP (not working correctly), or SMP nosmp (working) or even SMP (not working). AFAIK, the nosmp parameter should tell the kernel to act the same is if it is an UP kernel, and it disables IO APIC, which is not an issue in my case. Can you think about anything that would explain it, or would help me debugging it ? This is a bit odd, hard to say w/o more details on what exactly your code is trying to do and wait the failure is. I understand that it is difficult to give more details, but the driver is not mine, and I can't give all the code associated. The larger differences between SMP UP build would be setting of memory attribute for cache coherency. In UP case we don't set it. Between SMP 'nosmp' and SMP 'on' cases you seem like you're hitting some locking condition. Guessing your running into a timing locking issue that you're getting lucky on the SMP 'nosmp' case such that it just happens to work. Any idea on how to debug this ? I am using a 2.6.35 kernel. Thanks, JM ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Different behaviour when using nosmp parameter on SMP and UP
On Thu, Dec 1, 2011 at 9:04 AM, Jean-Michel Hautbois jhautb...@gmail.com wrote: Any idea on how to debug this ? I am using a 2.6.35 kernel. There are a ton of Kconfig options for debugging various locking bugs. Try turning them on. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Different behaviour when using nosmp parameter on SMP and UP
2011/12/1 Tabi Timur-B04825 b04...@freescale.com: On Thu, Dec 1, 2011 at 9:04 AM, Jean-Michel Hautbois jhautb...@gmail.com wrote: Any idea on how to debug this ? I am using a 2.6.35 kernel. There are a ton of Kconfig options for debugging various locking bugs. Try turning them on. And when I do that, it works, even in SMP... :). JM ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
An I2C device tree node can contain a 'cell-index' property that can be used to enumerate the I2C devices. If such a property exists, use it to specify the I2C adapter number. This feature is necessary for the Freescale PowerPC audio drivers (e.g. on the P1022DS). The machine driver needs to know the adapter number for each I2C adapter, but it only has access to the device tree. Previously, the I2C nodes always appeared in cell-index order, so the dynamic numbering coincided with the cell-index property. With commit ab827d97 (powerpc/85xx: Rework P1022DS device tree), the I2C nodes are unintentionally reversed in the device tree, and so the machine driver guesses the wrong I2C adapter number. Signed-off-by: Timur Tabi ti...@freescale.com --- drivers/i2c/busses/i2c-mpc.c | 14 +- 1 files changed, 13 insertions(+), 1 deletions(-) diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c index 107397a..8551c34 100644 --- a/drivers/i2c/busses/i2c-mpc.c +++ b/drivers/i2c/busses/i2c-mpc.c @@ -632,7 +632,19 @@ static int __devinit fsl_i2c_probe(struct platform_device *op) i2c-adap.dev.parent = op-dev; i2c-adap.dev.of_node = of_node_get(op-dev.of_node); - result = i2c_add_adapter(i2c-adap); + /* +* If the I2C node has a cell-index property, use it to enumerate +* the I2C adapter. +*/ + prop = of_get_property(i2c-adap.dev.of_node, cell-index, plen); + if (prop plen == sizeof(u32)) { + dev_dbg(i2c-dev, using cell-index property %u, *prop); + i2c-adap.nr = *prop; + result = i2c_add_numbered_adapter(i2c-adap); + } else { + result = i2c_add_adapter(i2c-adap); + } + if (result 0) { dev_err(i2c-dev, failed to add adapter\n); goto fail_add; -- 1.7.3.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Different behaviour when using nosmp parameter on SMP and UP
2011/12/1 Jean-Michel Hautbois jhautb...@gmail.com: 2011/12/1 Tabi Timur-B04825 b04...@freescale.com: On Thu, Dec 1, 2011 at 9:04 AM, Jean-Michel Hautbois jhautb...@gmail.com wrote: Any idea on how to debug this ? I am using a 2.6.35 kernel. There are a ton of Kconfig options for debugging various locking bugs. Try turning them on. All Kconfig options do not help me with cache coherency problems, though. Or, I didn't find any option related. Any idea ? JM ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
On 12/01/2011 11:33 AM, Timur Tabi wrote: An I2C device tree node can contain a 'cell-index' property that can be used to enumerate the I2C devices. If such a property exists, use it to specify the I2C adapter number. Didn't we decide a long time ago that this was a bad idea? This feature is necessary for the Freescale PowerPC audio drivers (e.g. on the P1022DS). The machine driver needs to know the adapter number for each I2C adapter, but it only has access to the device tree. Previously, the I2C nodes always appeared in cell-index order, so the dynamic numbering coincided with the cell-index property. With commit ab827d97 (powerpc/85xx: Rework P1022DS device tree), the I2C nodes are unintentionally reversed in the device tree, and so the machine driver guesses the wrong I2C adapter number. What specifically do you need this number for? What does it represent? -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 6/8 v2] powerpc/ps3: Fix PS3 repository build warnings
Fix some PS3 repository.c build warnings when DEBUG is defined. Also change most pr_debug calls to pr_devel calls. Fixes warnings like these: format '%lx' expects type 'long unsigned int', but argument 7 has type 'u64' Signed-off-by: Geoff Levand ge...@infradead.org --- Hi Ben, I rebased my for-powerpc branch to include this update. -Geoff v2: - Format u64 is using the ll length modifier (thanks Geert). - Convert pr_debug to pr_devel. arch/powerpc/platforms/ps3/repository.c | 135 --- 1 files changed, 68 insertions(+), 67 deletions(-) diff --git a/arch/powerpc/platforms/ps3/repository.c b/arch/powerpc/platforms/ps3/repository.c index cb68729..7bdfea3 100644 --- a/arch/powerpc/platforms/ps3/repository.c +++ b/arch/powerpc/platforms/ps3/repository.c @@ -44,7 +44,7 @@ static void _dump_field(const char *hdr, u64 n, const char *func, int line) s[i] = (in[i] = 126 in[i] = 32) ? in[i] : '.'; s[i] = 0; - pr_debug(%s:%d: %s%016llx : %s\n, func, line, hdr, n, s); + pr_devel(%s:%d: %s%016llx : %s\n, func, line, hdr, n, s); #endif } @@ -53,7 +53,7 @@ static void _dump_field(const char *hdr, u64 n, const char *func, int line) static void _dump_node_name(unsigned int lpar_id, u64 n1, u64 n2, u64 n3, u64 n4, const char *func, int line) { - pr_debug(%s:%d: lpar: %u\n, func, line, lpar_id); + pr_devel(%s:%d: lpar: %u\n, func, line, lpar_id); _dump_field(n1: , n1, func, line); _dump_field(n2: , n2, func, line); _dump_field(n3: , n3, func, line); @@ -65,13 +65,13 @@ static void _dump_node_name(unsigned int lpar_id, u64 n1, u64 n2, u64 n3, static void _dump_node(unsigned int lpar_id, u64 n1, u64 n2, u64 n3, u64 n4, u64 v1, u64 v2, const char *func, int line) { - pr_debug(%s:%d: lpar: %u\n, func, line, lpar_id); + pr_devel(%s:%d: lpar: %u\n, func, line, lpar_id); _dump_field(n1: , n1, func, line); _dump_field(n2: , n2, func, line); _dump_field(n3: , n3, func, line); _dump_field(n4: , n4, func, line); - pr_debug(%s:%d: v1: %016llx\n, func, line, v1); - pr_debug(%s:%d: v2: %016llx\n, func, line, v2); + pr_devel(%s:%d: v1: %016llx\n, func, line, v1); + pr_devel(%s:%d: v2: %016llx\n, func, line, v2); } /** @@ -135,7 +135,7 @@ static int read_node(unsigned int lpar_id, u64 n1, u64 n2, u64 n3, u64 n4, v2); if (result) { - pr_debug(%s:%d: lv1_read_repository_node failed: %s\n, + pr_warn(%s:%d: lv1_read_repository_node failed: %s\n, __func__, __LINE__, ps3_result(result)); dump_node_name(lpar_id, n1, n2, n3, n4); return -ENOENT; @@ -149,10 +149,10 @@ static int read_node(unsigned int lpar_id, u64 n1, u64 n2, u64 n3, u64 n4, *_v2 = v2; if (v1 !_v1) - pr_debug(%s:%d: warning: discarding non-zero v1: %016llx\n, + pr_devel(%s:%d: warning: discarding non-zero v1: %016llx\n, __func__, __LINE__, v1); if (v2 !_v2) - pr_debug(%s:%d: warning: discarding non-zero v2: %016llx\n, + pr_devel(%s:%d: warning: discarding non-zero v2: %016llx\n, __func__, __LINE__, v2); return 0; @@ -323,16 +323,16 @@ int ps3_repository_find_device(struct ps3_repository_device *repo) result = ps3_repository_read_bus_num_dev(tmp.bus_index, num_dev); if (result) { - pr_debug(%s:%d read_bus_num_dev failed\n, __func__, __LINE__); + pr_devel(%s:%d read_bus_num_dev failed\n, __func__, __LINE__); return result; } - pr_debug(%s:%d: bus_type %u, bus_index %u, bus_id %llu, num_dev %u\n, + pr_devel(%s:%d: bus_type %u, bus_index %u, bus_id %llu, num_dev %u\n, __func__, __LINE__, tmp.bus_type, tmp.bus_index, tmp.bus_id, num_dev); if (tmp.dev_index = num_dev) { - pr_debug(%s:%d: no device found\n, __func__, __LINE__); + pr_devel(%s:%d: no device found\n, __func__, __LINE__); return -ENODEV; } @@ -340,7 +340,7 @@ int ps3_repository_find_device(struct ps3_repository_device *repo) tmp.dev_type); if (result) { - pr_debug(%s:%d read_dev_type failed\n, __func__, __LINE__); + pr_devel(%s:%d read_dev_type failed\n, __func__, __LINE__); return result; } @@ -348,12 +348,12 @@ int ps3_repository_find_device(struct ps3_repository_device *repo) tmp.dev_id); if (result) { - pr_debug(%s:%d ps3_repository_read_dev_id failed\n, __func__, + pr_devel(%s:%d ps3_repository_read_dev_id failed\n, __func__, __LINE__); return result; } - pr_debug(%s:%d: found: dev_type %u,
Re: Different behaviour when using nosmp parameter on SMP and UP
On Dec 1, 2011, at 11:52 AM, Jean-Michel Hautbois wrote: 2011/12/1 Jean-Michel Hautbois jhautb...@gmail.com: 2011/12/1 Tabi Timur-B04825 b04...@freescale.com: On Thu, Dec 1, 2011 at 9:04 AM, Jean-Michel Hautbois jhautb...@gmail.com wrote: Any idea on how to debug this ? I am using a 2.6.35 kernel. There are a ton of Kconfig options for debugging various locking bugs. Try turning them on. All Kconfig options do not help me with cache coherency problems, though. Or, I didn't find any option related. Any idea ? JM Nope, if you can't describe the code in any more detail we can't help. - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Different behaviour when using nosmp parameter on SMP and UP
2011/12/1 Kumar Gala ga...@kernel.crashing.org: On Dec 1, 2011, at 11:52 AM, Jean-Michel Hautbois wrote: 2011/12/1 Jean-Michel Hautbois jhautb...@gmail.com: 2011/12/1 Tabi Timur-B04825 b04...@freescale.com: On Thu, Dec 1, 2011 at 9:04 AM, Jean-Michel Hautbois jhautb...@gmail.com wrote: Any idea on how to debug this ? I am using a 2.6.35 kernel. There are a ton of Kconfig options for debugging various locking bugs. Try turning them on. All Kconfig options do not help me with cache coherency problems, though. Or, I didn't find any option related. Any idea ? JM Nope, if you can't describe the code in any more detail we can't help. Here is what I can tell, I hope this will help you. I have a userland process, which reads and writes data to a character device thanks to ioctl. Reads and Writes are performed with this scheme : Lock Access - ioctl - down_interruptible(semaphore) Read (or Write) - ioctl - ioread32 Unlock Access - ioctl - up(semaphore) It reads (or writes) 4 bytes by 4 bytes, in a loop in the process which will eventually get several bytes from the PCIe device. The read (write) ioctl is really simple (and in fact, may be rewritten) : - kmalloc a structure, then copy_from_user - ioread32 using adress and offset from the structure copied previously - copy_to_user and free the structure. The userland process is currently doing a loop in order to read or writes with a 4bytes step. I am neither the writer of the driver nor the userland process, this is why I can't give you the code. Here is what I would do, at least : 1/ rewrite the read/write part of the ioctl in order to user a per_cpu structure and not a dynamically allocated at each call structure 2/ rewrite the userland part in order to ask the driver for n bytes, leaving the loop on ioread32/iowrite32 inside the ioctl Thanks for your help. JM ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
Scott Wood wrote: On 12/01/2011 11:33 AM, Timur Tabi wrote: An I2C device tree node can contain a 'cell-index' property that can be used to enumerate the I2C devices. If such a property exists, use it to specify the I2C adapter number. Didn't we decide a long time ago that this was a bad idea? I don't see what's wrong with it. This feature is necessary for the Freescale PowerPC audio drivers (e.g. on the P1022DS). The machine driver needs to know the adapter number for each I2C adapter, but it only has access to the device tree. Previously, the I2C nodes always appeared in cell-index order, so the dynamic numbering coincided with the cell-index property. With commit ab827d97 (powerpc/85xx: Rework P1022DS device tree), the I2C nodes are unintentionally reversed in the device tree, and so the machine driver guesses the wrong I2C adapter number. What specifically do you need this number for? What does it represent? Well, I thought the above paragraph explained it pretty well. Audio drivers come in sets of four -- machine, ssi, dma, and codec. The machine driver needs to know which ssi is connected to which codec and which DMA channel(s) it needs to use. So I extract all this information from the device tree. The individual drivers, however, register only with their own subsystem. So the codec driver, for example, doesn't know anything about device trees -- it just registers as an i2c driver. That means that the machine driver has to guess what name the codec driver will use when it registers. In this case, that's the i2c device name, which include the I2C adapter number (adap-nr). That means that given a device tree node, I need to know what the actual bus number is. An alternative approach is to create a function like this: struct i2c_adapter *i2c_adapter_from_node(struct device_node *np); I could then just use adap-nr directly. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
On 12/01/2011 02:54 PM, Timur Tabi wrote: Scott Wood wrote: On 12/01/2011 11:33 AM, Timur Tabi wrote: An I2C device tree node can contain a 'cell-index' property that can be used to enumerate the I2C devices. If such a property exists, use it to specify the I2C adapter number. Didn't we decide a long time ago that this was a bad idea? I don't see what's wrong with it. Obviously, since you're suggesting doing it. :-P Did you search for the old discussions? How is this going to interact with other i2c buses (e.g. on a board FPGA) that might have a conflicting static numbering scheme? Have you ensured that no dynamic bus registrations (e.g. an i2c bus on a PCI device) can happen before the static SoC i2c buses are added? Global numberspaces of this sort are usually the wrong answer. Look at the mess it made with IRQ numbers, and the gymnastics we go through to virtualize it. This feature is necessary for the Freescale PowerPC audio drivers (e.g. on the P1022DS). The machine driver needs to know the adapter number for each I2C adapter, but it only has access to the device tree. Previously, the I2C nodes always appeared in cell-index order, so the dynamic numbering coincided with the cell-index property. With commit ab827d97 (powerpc/85xx: Rework P1022DS device tree), the I2C nodes are unintentionally reversed in the device tree, and so the machine driver guesses the wrong I2C adapter number. What specifically do you need this number for? What does it represent? Well, I thought the above paragraph explained it pretty well. All you said was needs to know the adapter number, nothing about why. Audio drivers come in sets of four -- machine, ssi, dma, and codec. The machine driver needs to know which ssi is connected to which codec and which DMA channel(s) it needs to use. So I extract all this information from the device tree. The individual drivers, however, register only with their own subsystem. So the codec driver, for example, doesn't know anything about device trees -- it just registers as an i2c driver. That means that the machine driver has to guess what name the codec driver will use when it registers. In this case, that's the i2c device name, which include the I2C adapter number (adap-nr). That means that given a device tree node, I need to know what the actual bus number is. An alternative approach is to create a function like this: struct i2c_adapter *i2c_adapter_from_node(struct device_node *np); I could then just use adap-nr directly. If there isn't a way to get a struct device from struct device_node, we should add it. From that you should be able to look up the sysfs name -- no need to mess around with adap-nr as an intermediary. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [BUG?]3.0-rc4+ftrace+kprobe: set kprobe at instruction 'stwu' lead to system crash/freeze
On Thu, 2011-12-01 at 18:44 +0800, tiejun.chen wrote: Do you mean we should push the original pt_regs (or that whole exception stack) downwards the location the new r1 point? Then its safe to perform this real emulated stw instruction. At last we will reroute r1 to that copied exception frame to restore as normal. Right? Right. That way we don't have to modify the (sensitive) restore path, we only hook around the do_work branch which is a lot easier and less risky. Here I suppose so, I implement this for PPC32 based on the above understanding. I take a validation for kprobing do_fork()/show_interrupts(), now looks fine. Tomorrow I will go PPC64, and hope its fine as well. If everything is good I'll send these patches to linuxppc-dev next week. Excellent, thanks ! Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: ppc4xx simple vs SoC's
On Thu, 2011-12-01 at 06:44 -0500, Josh Boyer wrote: However, the entry for ppc44x_simple doesn't select any of these, meaning for example, AFAIK, that you don't get EMAC4 etc... I'm surprised things work at all ! What am I missing ? CONFIG_PPC44x_SIMPLE is selected by the board Kconfig values, which also select all of the SoC Kconfig options they need as well. ppc44x_simple started as a way to basically avoid duplicating a bunch of board.c files, but it's still driven from the board options. So hopefully that explains how things are working today. The question now is whether you would like something different? Ok, just getting my head around that stuff, I haven't really looked since you and Grant I believe came up with the scheme :-) (Was wondering what to do for currituck, but for now we'll keep it a separate board, at least until I can decide how to deal with the interrupt quirk). I just hadn't realized that while we had ppc4xx_simple, we still had board Kconfig's to enable it. Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
Scott Wood wrote: How is this going to interact with other i2c buses (e.g. on a board FPGA) that might have a conflicting static numbering scheme? Have you ensured that no dynamic bus registrations (e.g. an i2c bus on a PCI device) can happen before the static SoC i2c buses are added? Hmm You have a point there. An alternative approach is to create a function like this: struct i2c_adapter *i2c_adapter_from_node(struct device_node *np); I could then just use adap-nr directly. If there isn't a way to get a struct device from struct device_node, we should add it. How do I do that? Scan all the struct devices until I find one where dev-of_node == np? That seems really inefficient. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
On 12/01/2011 03:46 PM, Timur Tabi wrote: Scott Wood wrote: How is this going to interact with other i2c buses (e.g. on a board FPGA) that might have a conflicting static numbering scheme? Have you ensured that no dynamic bus registrations (e.g. an i2c bus on a PCI device) can happen before the static SoC i2c buses are added? Hmm You have a point there. An alternative approach is to create a function like this: struct i2c_adapter *i2c_adapter_from_node(struct device_node *np); I could then just use adap-nr directly. If there isn't a way to get a struct device from struct device_node, we should add it. How do I do that? Scan all the struct devices until I find one where dev-of_node == np? That seems really inefficient. Ideally we would have a field in struct device_node that points to struct device. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
On Thu, Dec 1, 2011 at 2:52 PM, Scott Wood scottw...@freescale.com wrote: On 12/01/2011 03:46 PM, Timur Tabi wrote: Scott Wood wrote: How is this going to interact with other i2c buses (e.g. on a board FPGA) that might have a conflicting static numbering scheme? Have you ensured that no dynamic bus registrations (e.g. an i2c bus on a PCI device) can happen before the static SoC i2c buses are added? Hmm You have a point there. An alternative approach is to create a function like this: struct i2c_adapter *i2c_adapter_from_node(struct device_node *np); I could then just use adap-nr directly. If there isn't a way to get a struct device from struct device_node, we should add it. How do I do that? Scan all the struct devices until I find one where dev-of_node == np? That seems really inefficient. Ideally we would have a field in struct device_node that points to struct device. No. There are cases where multiple devices may reference the same node. It is better to walk the list of i2c adapters and look for one that has the matching node pointer. It really isn't an expensive operation to do it that way. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
Grant Likely wrote: It is better to walk the list of i2c adapters and look for one that has the matching node pointer. It really isn't an expensive operation to do it that way. That's what I was thinking. I can't figure out how to walk the list, though. i2c_get_adapter() takes an adapter number, but I don't think this is going to work: unsigned int i = 0; struct i2c_adapter adap = i2c_get_adapter(0); while (adap) { if (adap-nr == nr) break; adap = i2c_get_adapter(++i); } -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
Scott Wood wrote: Ideally we would have a field in struct device_node that points to struct device. I don't think there's a single place in the code where the connection between the device and device_node is made. In fact, I think dev-of_node needs to be manually initialized by the driver during the OF probe. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
On 12/01/2011 03:59 PM, Timur Tabi wrote: Scott Wood wrote: Ideally we would have a field in struct device_node that points to struct device. I don't think there's a single place in the code where the connection between the device and device_node is made. In fact, I think dev-of_node needs to be manually initialized by the driver during the OF probe. The i2c core does this. Does of_find_i2c_device_by_node() do what you want? -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
Scott Wood wrote: Does of_find_i2c_device_by_node() do what you want? Yeah, that'll work. I wasn't thinking about looking for an i2c device. Thanks. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 5/6] powerpc/boot: Add mfdcrx
+#define mfdcrx(rn) \ + ({ \ + unsigned long rval; \ + asm volatile(mfdcrx %0,%1 : =r(rval) : g(rn)); \ + rval; \ + }) g is never correct on PowerPC, you want r here. You can write this as a static inline btw, you only need the #define stuff when there is an i constraint involved. I think you can still use static inlines even when a constraint is one that requires a compile time constant. Marking a function inline does not guarantee the compiler will perform inline substitution on it, which you need here. I don't think it's a problem with recent GCC in the context of the Linux kernel though, if you use __always_inline that is. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/3] serial: allow passing in hardware bug info via platform device
The normal arch hook into the 8250 serial world is via passing in a plat_serial8250_port struct. However, this struct does not have a bugs field, so there is no way to have the arch code pass in info about known uart issues. Add a bug field to the plat_serial8250_port struct, so that the arch can pass in this information. Also don't do a blanket overwrite of the bugs setting in the 8250.c driver. Finally, relocate the known bug #define list to a globally visible header so that the arch can assign any appropriate values from the list. Signed-off-by: Paul Gortmaker paul.gortma...@windriver.com --- drivers/tty/serial/8250.c |3 ++- drivers/tty/serial/8250.h |5 - include/linux/serial_8250.h |6 ++ 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c index 7c94dbc..f99f27c 100644 --- a/drivers/tty/serial/8250.c +++ b/drivers/tty/serial/8250.c @@ -1101,7 +1101,6 @@ static void autoconfig(struct uart_8250_port *up, unsigned int probeflags) spin_lock_irqsave(up-port.lock, flags); up-capabilities = 0; - up-port.bugs = 0; if (!(up-port.flags UPF_BUGGY_UART)) { /* @@ -3075,6 +3074,7 @@ static int __devinit serial8250_probe(struct platform_device *dev) port.serial_out = p-serial_out; port.handle_irq = p-handle_irq; port.set_termios= p-set_termios; + port.bugs = p-bugs; port.pm = p-pm; port.dev= dev-dev; port.irqflags |= irqflag; @@ -3225,6 +3225,7 @@ int serial8250_register_port(struct uart_port *port) uart-port.regshift = port-regshift; uart-port.iotype = port-iotype; uart-port.flags= port-flags | UPF_BOOT_AUTOCONF; + uart-port.bugs = port-bugs; uart-port.mapbase = port-mapbase; uart-port.private_data = port-private_data; if (port-dev) diff --git a/drivers/tty/serial/8250.h b/drivers/tty/serial/8250.h index 6edf4a6..caefe00 100644 --- a/drivers/tty/serial/8250.h +++ b/drivers/tty/serial/8250.h @@ -44,11 +44,6 @@ struct serial8250_config { #define UART_CAP_UUE (1 12) /* UART needs IER bit 6 set (Xscale) */ #define UART_CAP_RTOIE (1 13) /* UART needs IER bit 4 set (Xscale, Tegra) */ -#define UART_BUG_QUOT (1 0)/* UART has buggy quot LSB */ -#define UART_BUG_TXEN (1 1)/* UART has buggy TX IIR status */ -#define UART_BUG_NOMSR (1 2)/* UART has buggy MSR status bits (Au1x00) */ -#define UART_BUG_THRE (1 3)/* UART has buggy THRE reassertion */ - #define PROBE_RSA (1 0) #define PROBE_ANY (~0) diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h index 1f05bbe..8c660af 100644 --- a/include/linux/serial_8250.h +++ b/include/linux/serial_8250.h @@ -14,6 +14,11 @@ #include linux/serial_core.h #include linux/platform_device.h +#define UART_BUG_QUOT (1 0)/* buggy quot LSB */ +#define UART_BUG_TXEN (1 1)/* buggy TX IIR status */ +#define UART_BUG_NOMSR (1 2)/* buggy MSR status bits (Au1x00) */ +#define UART_BUG_THRE (1 3)/* buggy THRE reassertion */ + /* * This is the platform device platform_data structure */ @@ -30,6 +35,7 @@ struct plat_serial8250_port { unsigned char hub6; upf_t flags; /* UPF_* flags */ unsigned inttype; /* If UPF_FIXED_TYPE */ + unsigned short bugs; /* hardware specific bugs */ unsigned int(*serial_in)(struct uart_port *, int); void(*serial_out)(struct uart_port *, int, int); void(*set_termios)(struct uart_port *, -- 1.7.7 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 0/3] RFC Fix Fsl 8250 BRK bug via letting plat code set bugs
This is a respin of an earlier patch[1] that enabled a workaround via Kconfig for an errata issue when using BRK on FSL 16550 UARTs. In this version, the 8250 specific bug field is moved to the generic uart_port struct, since hardware bugs aren't the unique domain of just the 8250 class of uarts. Then the ability to set a known bugs value via the platform_device entry point is added, so that it can be set externally vs. requiring some sort of autodetection from the actual driver(s). Then, the FSL errata fix is added to 8250.c by using the above support. An entry in a DTS file is used to flag boards/platforms with the issue. It is also added in a way that is optimized away for other architectures. Kumar has a patch pending[2] that updates all the dts files. I've not included it here, so that review focus can be on the implementation. I've re-tested it on an sbc8641D, where sysrq was useless before; with this fix, it works as expected. Thanks, Paul. [1] http://patchwork.ozlabs.org/patch/46609/ [2] http://patchwork.ozlabs.org/patch/128070/ Paul Gortmaker (3): serial: make bugs field not specific to 8250 type uarts. serial: allow passing in hardware bug info via platform device 8250: add workaround for MPC8[356]xx UART break IRQ storm arch/powerpc/kernel/legacy_serial.c | 11 ++ drivers/tty/serial/8250.c | 37 +- drivers/tty/serial/8250.h |5 include/linux/serial_8250.h | 11 ++ include/linux/serial_core.h |1 + 5 files changed, 46 insertions(+), 19 deletions(-) -- 1.7.7 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/3] serial: make bugs field not specific to 8250 type uarts.
There is a struct uart_8250_port that is private to 8250.c itself, and it had a bugs field. But there is no reason to assume that hardware bugs are unique to 8250 type UARTS. Make the bugs field part of the globally visible struct uart_port and remove the 8250 specific one. The value in doing so is that it helps pave the way for allowing arch or platform specific code to pass in information to the specific uart drivers about uart bugs known to impact certain platforms that would otherwise be hard to detect from within the context of the driver itself. Signed-off-by: Paul Gortmaker paul.gortma...@windriver.com --- drivers/tty/serial/8250.c | 25 - include/linux/serial_core.h |1 + 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c index eeadf1b..7c94dbc 100644 --- a/drivers/tty/serial/8250.c +++ b/drivers/tty/serial/8250.c @@ -134,7 +134,6 @@ struct uart_8250_port { struct timer_list timer; /* no irq timer */ struct list_headlist; /* ports on this IRQ */ unsigned short capabilities; /* port capabilities */ - unsigned short bugs; /* port bugs */ unsigned inttx_loadsz; /* transmit fifo load size */ unsigned char acr; unsigned char ier; @@ -828,7 +827,7 @@ static void autoconfig_has_efr(struct uart_8250_port *up) * when DLL is 0. */ if (id3 == 0x52 rev == 0x01) - up-bugs |= UART_BUG_QUOT; + up-port.bugs |= UART_BUG_QUOT; return; } @@ -1102,7 +1101,7 @@ static void autoconfig(struct uart_8250_port *up, unsigned int probeflags) spin_lock_irqsave(up-port.lock, flags); up-capabilities = 0; - up-bugs = 0; + up-port.bugs = 0; if (!(up-port.flags UPF_BUGGY_UART)) { /* @@ -1337,7 +1336,7 @@ static void serial8250_start_tx(struct uart_port *port) up-ier |= UART_IER_THRI; serial_out(up, UART_IER, up-ier); - if (up-bugs UART_BUG_TXEN) { + if (port-bugs UART_BUG_TXEN) { unsigned char lsr; lsr = serial_in(up, UART_LSR); up-lsr_saved_flags |= lsr LSR_SAVE_FLAGS; @@ -1373,7 +1372,7 @@ static void serial8250_enable_ms(struct uart_port *port) container_of(port, struct uart_8250_port, port); /* no MSR capabilities */ - if (up-bugs UART_BUG_NOMSR) + if (port-bugs UART_BUG_NOMSR) return; up-ier |= UART_IER_MSI; @@ -2089,7 +2088,7 @@ static int serial8250_startup(struct uart_port *port) * kick the UART on a regular basis. */ if (!(iir1 UART_IIR_NO_INT) (iir UART_IIR_NO_INT)) { - up-bugs |= UART_BUG_THRE; + port-bugs |= UART_BUG_THRE; pr_debug(ttyS%d - using backup timer\n, serial_index(port)); } @@ -2099,7 +2098,7 @@ static int serial8250_startup(struct uart_port *port) * The above check will only give an accurate result the first time * the port is opened so this value needs to be preserved. */ - if (up-bugs UART_BUG_THRE) { + if (port-bugs UART_BUG_THRE) { up-timer.function = serial8250_backup_timeout; up-timer.data = (unsigned long)up; mod_timer(up-timer, jiffies + @@ -2162,13 +2161,13 @@ static int serial8250_startup(struct uart_port *port) serial_outp(up, UART_IER, 0); if (lsr UART_LSR_TEMT iir UART_IIR_NO_INT) { - if (!(up-bugs UART_BUG_TXEN)) { - up-bugs |= UART_BUG_TXEN; + if (!(port-bugs UART_BUG_TXEN)) { + port-bugs |= UART_BUG_TXEN; pr_debug(ttyS%d - enabling bad tx status workarounds\n, serial_index(port)); } } else { - up-bugs = ~UART_BUG_TXEN; + port-bugs = ~UART_BUG_TXEN; } dont_test_tx_en: @@ -2323,7 +2322,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios, /* * Oxford Semi 952 rev B workaround */ - if (up-bugs UART_BUG_QUOT (quot 0xff) == 0) + if (port-bugs UART_BUG_QUOT (quot 0xff) == 0) quot++; if (up-capabilities UART_CAP_FIFO up-port.fifosize 1) { @@ -2390,7 +2389,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios, * CTS flow control flag and modem status interrupts */ up-ier = ~UART_IER_MSI; - if (!(up-bugs UART_BUG_NOMSR) + if (!(port-bugs
[PATCH 3/3] 8250: add workaround for MPC8[356]xx UART break IRQ storm
Sending a break on the SOC UARTs found in some MPC83xx/85xx/86xx chips seems to cause a short lived IRQ storm (/proc/interrupts typically shows somewhere between 300 and 1500 events). Unfortunately this renders SysRQ over the serial console completely inoperable. The suggested workaround in the errata is to read the Rx register, wait one character period, and then read the Rx register again. We achieve this by tracking the old LSR value, and on the subsequent interrupt event after a break, we don't read LSR, instead we just read the RBR again and return immediately. The fsl,ns16550 is used in the compatible field of the serial device to mark UARTs known to have this issue. Thanks to Scott Wood for providing the errata data which led to a much cleaner fix. Signed-off-by: Paul Gortmaker paul.gortma...@windriver.com --- arch/powerpc/kernel/legacy_serial.c | 11 +++ drivers/tty/serial/8250.c | 11 ++- include/linux/serial_8250.h |5 + 3 files changed, 26 insertions(+), 1 deletions(-) diff --git a/arch/powerpc/kernel/legacy_serial.c b/arch/powerpc/kernel/legacy_serial.c index c7b5afe..dd232ca 100644 --- a/arch/powerpc/kernel/legacy_serial.c +++ b/arch/powerpc/kernel/legacy_serial.c @@ -476,6 +476,15 @@ static void __init fixup_port_mmio(int index, port-membase = ioremap(port-mapbase, 0x100); } +static void __init fixup_port_bugs(int index, + struct device_node *np, + struct plat_serial8250_port *port) +{ + DBG(fixup_port_bugs(%d)\n, index); + + if (of_device_is_compatible(np, fsl,ns16550)) + port-bugs = UART_BUG_FSLBK; +} /* * This is called as an arch initcall, hopefully before the PCI bus is * probed and/or the 8250 driver loaded since we need to register our @@ -512,6 +521,8 @@ static int __init serial_dev_init(void) fixup_port_pio(i, np, port); if ((port-iotype == UPIO_MEM) || (port-iotype == UPIO_TSI)) fixup_port_mmio(i, np, port); + + fixup_port_bugs(i, np, port); } DBG(Registering platform serial ports\n); diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c index f99f27c..32e9821 100644 --- a/drivers/tty/serial/8250.c +++ b/drivers/tty/serial/8250.c @@ -142,6 +142,7 @@ struct uart_8250_port { unsigned char mcr_mask; /* mask of user bits */ unsigned char mcr_force; /* mask of forced bits */ unsigned char cur_iotype; /* Running I/O type */ + unsigned char lsr_last; /* LSR of last IRQ event */ /* * Some bits in registers are cleared on a read, so they must @@ -1553,7 +1554,15 @@ static void serial8250_handle_port(struct uart_8250_port *up) spin_lock_irqsave(up-port.lock, flags); - status = serial_inp(up, UART_LSR); + /* Workaround for IRQ storm errata on break with Freescale 16550 */ + if (UART_BUG_FSLBK up-port.bugs up-lsr_last UART_LSR_BI) { + up-lsr_last = ~UART_LSR_BI; + serial_inp(up, UART_RX); + spin_unlock_irqrestore(up-port.lock, flags); + return; + } + + status = up-lsr_last = serial_inp(up, UART_LSR); DEBUG_INTR(status = %x..., status); diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h index 8c660af..b0f4042 100644 --- a/include/linux/serial_8250.h +++ b/include/linux/serial_8250.h @@ -18,6 +18,11 @@ #define UART_BUG_TXEN (1 1)/* buggy TX IIR status */ #define UART_BUG_NOMSR (1 2)/* buggy MSR status bits (Au1x00) */ #define UART_BUG_THRE (1 3)/* buggy THRE reassertion */ +#ifdef CONFIG_PPC32 +#define UART_BUG_FSLBK (1 4)/* buggy FSL break IRQ storm */ +#else /* help GCC optimize away IRQ handler errata code for ARCH != PPC32 */ +#define UART_BUG_FSLBK 0 +#endif /* * This is the platform device platform_data structure -- 1.7.7 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3] 8250: add workaround for MPC8[356]xx UART break IRQ storm
On 12/01/2011 05:47 PM, Paul Gortmaker wrote: diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h index 8c660af..b0f4042 100644 --- a/include/linux/serial_8250.h +++ b/include/linux/serial_8250.h @@ -18,6 +18,11 @@ #define UART_BUG_TXEN(1 1)/* buggy TX IIR status */ #define UART_BUG_NOMSR (1 2)/* buggy MSR status bits (Au1x00) */ #define UART_BUG_THRE(1 3)/* buggy THRE reassertion */ +#ifdef CONFIG_PPC32 +#define UART_BUG_FSLBK (1 4)/* buggy FSL break IRQ storm */ +#else/* help GCC optimize away IRQ handler errata code for ARCH != PPC32 */ +#define UART_BUG_FSLBK 0 +#endif I believe this bug still exists on our 64-bit chips. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3] 8250: add workaround for MPC8[356]xx UART break IRQ storm
On 11-12-01 06:51 PM, Scott Wood wrote: On 12/01/2011 05:47 PM, Paul Gortmaker wrote: diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h index 8c660af..b0f4042 100644 --- a/include/linux/serial_8250.h +++ b/include/linux/serial_8250.h @@ -18,6 +18,11 @@ #define UART_BUG_TXEN (1 1)/* buggy TX IIR status */ #define UART_BUG_NOMSR (1 2)/* buggy MSR status bits (Au1x00) */ #define UART_BUG_THRE (1 3)/* buggy THRE reassertion */ +#ifdef CONFIG_PPC32 +#define UART_BUG_FSLBK (1 4)/* buggy FSL break IRQ storm */ +#else /* help GCC optimize away IRQ handler errata code for ARCH != PPC32 */ +#define UART_BUG_FSLBK 0 +#endif I believe this bug still exists on our 64-bit chips. OK, I'll simply change the above to CONFIG_PPC then. Thanks, Paul. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH][v2] Enable CONFIG_STRICT_DEVMEM support for Powerpc
From aebed2e9fb9fba7dd0a34595780b828d7a545ba2 Mon Sep 17 00:00:00 2001 From: Sukadev Bhattiprolu suka...@linux.vnet.ibm.com Date: Mon, 29 Aug 2011 14:12:08 -0700 Subject: [PATCH 1/1][v2] Enable CONFIG_STRICT_DEVMEM support for Powerpc. As described in the help text in the patch, this token restricts general access to /dev/mem as a way of increasing security. Specifically, access to exclusive IOMEM and kernel RAM is denied unless CONFIG_STRICT_DEVMEM is set to 'n'. Implement the 'devmem_is_allowed()' interface for POWER. It will be called from range_is_allowed() when userpsace attempts to access /dev/mem. This patch is based on an earlier patch from Steve Best and with input from Paul Mackerras. A test for this patch: # cat /proc/rtas/rmo_buffer 0f19 1 # python -c print 0x0f19 / 0x1 3865 # dd if=/dev/mem of=/tmp/foo count=1 bs=64k skip=3865 1+0 records in 1+0 records out 65536 bytes (66 kB) copied, 0.000205235 s, 319 MB/s # dd if=/dev/mem of=/tmp/foo count=1 bs=64k skip=3864 dd: reading `/dev/mem': Operation not permitted 0+0 records in 0+0 records out 0 bytes (0 B) copied, 0.000226844 s, 0.0 kB/s # dd if=/dev/mem of=/tmp/foo count=1 bs=64k skip=3866 dd: reading `/dev/mem': Operation not permitted 0+0 records in 0+0 records out 0 bytes (0 B) copied, 0.00023542 s, 0.0 kB/s # dd if=/dev/mem of=/tmp/foo dd: reading `/dev/mem': Operation not permitted 0+0 records in 0+0 records out 0 bytes (0 B) copied, 0.00022519 s, 0.0 kB/s Changelog[v2]: - [Anton Blanchard] Punch a hole in devmem to allow librtas/dlpar tools access to /dev/mem. Signed-off-by: Sukadev Bhattiprolu suka...@linux.vnet.ibm.com --- arch/powerpc/Kconfig.debug | 12 arch/powerpc/include/asm/page.h |1 + arch/powerpc/kernel/rtas.c | 10 ++ arch/powerpc/mm/mem.c | 20 drivers/char/mem.c |5 +++-- 5 files changed, 46 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug index 067cb84..1cf1b00 100644 --- a/arch/powerpc/Kconfig.debug +++ b/arch/powerpc/Kconfig.debug @@ -298,4 +298,16 @@ config PPC_EARLY_DEBUG_CPM_ADDR platform probing is done, all platforms selected must share the same address. +config STRICT_DEVMEM + def_bool y + prompt Filter access to /dev/mem + help + This option restricts access to /dev/mem. If this option is + disabled, you allow userspace access to all memory, including + kernel and userspace memory. Accidental memory access is likely + to be disastrous. + Memory access is required for experts who want to debug the kernel. + + If you are unsure, say Y. + endmenu diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h index 2cd664e..dc2ec96 100644 --- a/arch/powerpc/include/asm/page.h +++ b/arch/powerpc/include/asm/page.h @@ -261,6 +261,7 @@ extern void clear_user_page(void *page, unsigned long vaddr, struct page *pg); extern void copy_user_page(void *to, void *from, unsigned long vaddr, struct page *p); extern int page_is_ram(unsigned long pfn); +extern int devmem_is_allowed(unsigned long pfn); #ifdef CONFIG_PPC_SMLPAR void arch_free_page(struct page *page, int order); diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c index d5ca823..07cf2cf 100644 --- a/arch/powerpc/kernel/rtas.c +++ b/arch/powerpc/kernel/rtas.c @@ -937,6 +937,16 @@ asmlinkage int ppc_rtas(struct rtas_args __user *uargs) return 0; } +int page_is_rtas(unsigned long pfn) +{ + unsigned long paddr = (pfn PAGE_SHIFT); + + if (paddr = rtas_rmo_buf paddr (rtas_rmo_buf + RTAS_RMOBUF_MAX)) + return 1; + + return 0; +} + /* * Call early during boot, before mem init or bootmem, to retrieve the RTAS * informations from the device-tree and allocate the RMO buffer for userland diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c index c781bbc..d21dbc6 100644 --- a/arch/powerpc/mm/mem.c +++ b/arch/powerpc/mm/mem.c @@ -549,3 +549,23 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long address, hash_preload(vma-vm_mm, address, access, trap); #endif /* CONFIG_PPC_STD_MMU */ } + +extern int page_is_rtas(unsigned long pfn); + +/* + * devmem_is_allowed(): check to see if /dev/mem access to a certain address + * is valid. The argument is a physical page number. + * + * Access has to be given to non-kernel-ram areas as well, these contain the + * PCI mmio resources as well as potential bios/acpi data regions. + */ +int devmem_is_allowed(unsigned long pfn) +{ + if (iomem_is_exclusive(pfn PAGE_SHIFT)) + return 0; + if (!page_is_ram(pfn)) +
Re: [PATCH 3/3] 8250: add workaround for MPC8[356]xx UART break IRQ storm
On Dec 1, 2011, at 6:05 PM, Paul Gortmaker wrote: On 11-12-01 06:51 PM, Scott Wood wrote: On 12/01/2011 05:47 PM, Paul Gortmaker wrote: diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h index 8c660af..b0f4042 100644 --- a/include/linux/serial_8250.h +++ b/include/linux/serial_8250.h @@ -18,6 +18,11 @@ #define UART_BUG_TXEN (1 1)/* buggy TX IIR status */ #define UART_BUG_NOMSR (1 2)/* buggy MSR status bits (Au1x00) */ #define UART_BUG_THRE (1 3)/* buggy THRE reassertion */ +#ifdef CONFIG_PPC32 +#define UART_BUG_FSLBK (1 4)/* buggy FSL break IRQ storm */ +#else /* help GCC optimize away IRQ handler errata code for ARCH != PPC32 */ +#define UART_BUG_FSLBK 0 +#endif I believe this bug still exists on our 64-bit chips. OK, I'll simply change the above to CONFIG_PPC then. It does, the bug is in the uart IP which I don't think we ever plan on fixing, so 32 or 64-bit parts will have it for ever and ever ;) - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC][PATCH] update FSL 16550 nodes to have...
On Mon, Nov 28, 2011 at 3:02 PM, Kumar Gala ga...@kernel.crashing.org wrote: Signed-off-by: Kumar Gala ga...@kernel.crashing.org --- * Need to fixup the commit message I had this written when I was thinking of re-sending the dts with the other three, that is before I realized the dts patch would then overwhelm the other patches completely. :) P. ppc: update FSL 16550 nodes to have fsl,ns16550 compat entry There is an errata relating to break handling on some of the Freescale 16550 implementations. By marking the nodes with a fsl prefix, we can use that to know when to enable the errata handling. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/3] serial: make bugs field not specific to 8250 type uarts.
Make the bugs field part of the globally visible struct uart_port and remove the 8250 specific one. Except all the bits in it are 8250 specific things or names that are meaningless in generic form - no. I also don't want to encourage flags and bug bits. We already have too many and its making the code a mess. So right now we want less not more. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3] 8250: add workaround for MPC8[356]xx UART break IRQ storm
@@ -1553,7 +1554,15 @@ static void serial8250_handle_port(struct uart_8250_port *up) spin_lock_irqsave(up-port.lock, flags); - status = serial_inp(up, UART_LSR); + /* Workaround for IRQ storm errata on break with Freescale 16550 */ + if (UART_BUG_FSLBK up-port.bugs up-lsr_last UART_LSR_BI) { + up-lsr_last = ~UART_LSR_BI; + serial_inp(up, UART_RX); + spin_unlock_irqrestore(up-port.lock, flags); + return; + } + + status = up-lsr_last = serial_inp(up, UART_LSR); We've now had a recent pile of IRQ function changes adding more quirk bits and special casing. This doesn't scale. We either need to make handle_port a method the specific drivers can override or you could hide the mess in your serial_inp implementation and not touch any core code. I really don't mind which but I suspect dealing with it in your serial_inp handler so that when you read UART_LSR you do the fixup might be simplest providing you can just do that. Sorting out a -handle_port override is probably ultimately the right thing to do and then we can push some of the other funnies out further. At this point it's becoming increasingly clear that 8250 UART cloners are both very bad at cloning the things accurately, and very busy adding extra useful features so we need to start to treat 8250.c as a library you can wrap. Alan ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH][v2] Enable CONFIG_STRICT_DEVMEM support for Powerpc
On Thu, 2011-12-01 at 16:11 -0800, Sukadev Bhattiprolu wrote: From aebed2e9fb9fba7dd0a34595780b828d7a545ba2 Mon Sep 17 00:00:00 2001 From: Sukadev Bhattiprolu suka...@linux.vnet.ibm.com Date: Mon, 29 Aug 2011 14:12:08 -0700 Subject: [PATCH 1/1][v2] Enable CONFIG_STRICT_DEVMEM support for Powerpc. As described in the help text in the patch, this token restricts general access to /dev/mem as a way of increasing security. Specifically, access to exclusive IOMEM and kernel RAM is denied unless CONFIG_STRICT_DEVMEM is set to 'n'. Implement the 'devmem_is_allowed()' interface for POWER. It will be called from range_is_allowed() when userpsace attempts to access /dev/mem. This patch is based on an earlier patch from Steve Best and with input from Paul Mackerras. A slightly different version of that patch is already in my -next branch since earlier this week. Please send a followup patch adding the missing rtas bit. Note that I've dropped the generic printk change which has nothing to do in that patch and can/should be submitted separately if it's really needed. Cheers, Ben. A test for this patch: # cat /proc/rtas/rmo_buffer 0f19 1 # python -c print 0x0f19 / 0x1 3865 # dd if=/dev/mem of=/tmp/foo count=1 bs=64k skip=3865 1+0 records in 1+0 records out 65536 bytes (66 kB) copied, 0.000205235 s, 319 MB/s # dd if=/dev/mem of=/tmp/foo count=1 bs=64k skip=3864 dd: reading `/dev/mem': Operation not permitted 0+0 records in 0+0 records out 0 bytes (0 B) copied, 0.000226844 s, 0.0 kB/s # dd if=/dev/mem of=/tmp/foo count=1 bs=64k skip=3866 dd: reading `/dev/mem': Operation not permitted 0+0 records in 0+0 records out 0 bytes (0 B) copied, 0.00023542 s, 0.0 kB/s # dd if=/dev/mem of=/tmp/foo dd: reading `/dev/mem': Operation not permitted 0+0 records in 0+0 records out 0 bytes (0 B) copied, 0.00022519 s, 0.0 kB/s Changelog[v2]: - [Anton Blanchard] Punch a hole in devmem to allow librtas/dlpar tools access to /dev/mem. Signed-off-by: Sukadev Bhattiprolu suka...@linux.vnet.ibm.com --- arch/powerpc/Kconfig.debug | 12 arch/powerpc/include/asm/page.h |1 + arch/powerpc/kernel/rtas.c | 10 ++ arch/powerpc/mm/mem.c | 20 drivers/char/mem.c |5 +++-- 5 files changed, 46 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug index 067cb84..1cf1b00 100644 --- a/arch/powerpc/Kconfig.debug +++ b/arch/powerpc/Kconfig.debug @@ -298,4 +298,16 @@ config PPC_EARLY_DEBUG_CPM_ADDR platform probing is done, all platforms selected must share the same address. +config STRICT_DEVMEM + def_bool y + prompt Filter access to /dev/mem + help + This option restricts access to /dev/mem. If this option is + disabled, you allow userspace access to all memory, including + kernel and userspace memory. Accidental memory access is likely + to be disastrous. + Memory access is required for experts who want to debug the kernel. + + If you are unsure, say Y. + endmenu diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h index 2cd664e..dc2ec96 100644 --- a/arch/powerpc/include/asm/page.h +++ b/arch/powerpc/include/asm/page.h @@ -261,6 +261,7 @@ extern void clear_user_page(void *page, unsigned long vaddr, struct page *pg); extern void copy_user_page(void *to, void *from, unsigned long vaddr, struct page *p); extern int page_is_ram(unsigned long pfn); +extern int devmem_is_allowed(unsigned long pfn); #ifdef CONFIG_PPC_SMLPAR void arch_free_page(struct page *page, int order); diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c index d5ca823..07cf2cf 100644 --- a/arch/powerpc/kernel/rtas.c +++ b/arch/powerpc/kernel/rtas.c @@ -937,6 +937,16 @@ asmlinkage int ppc_rtas(struct rtas_args __user *uargs) return 0; } +int page_is_rtas(unsigned long pfn) +{ + unsigned long paddr = (pfn PAGE_SHIFT); + + if (paddr = rtas_rmo_buf paddr (rtas_rmo_buf + RTAS_RMOBUF_MAX)) + return 1; + + return 0; +} + /* * Call early during boot, before mem init or bootmem, to retrieve the RTAS * informations from the device-tree and allocate the RMO buffer for userland diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c index c781bbc..d21dbc6 100644 --- a/arch/powerpc/mm/mem.c +++ b/arch/powerpc/mm/mem.c @@ -549,3 +549,23 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long address, hash_preload(vma-vm_mm, address, access, trap); #endif /* CONFIG_PPC_STD_MMU */ } + +extern int page_is_rtas(unsigned long pfn); + +/* + * devmem_is_allowed():
Re: [PATCH][v2] Enable CONFIG_STRICT_DEVMEM support for Powerpc
And an additional comment regarding the rtas bit: #ifdef CONFIG_PPC_SMLPAR void arch_free_page(struct page *page, int order); diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c index d5ca823..07cf2cf 100644 --- a/arch/powerpc/kernel/rtas.c +++ b/arch/powerpc/kernel/rtas.c @@ -937,6 +937,16 @@ asmlinkage int ppc_rtas(struct rtas_args __user *uargs) return 0; } +int page_is_rtas(unsigned long pfn) +{ + unsigned long paddr = (pfn PAGE_SHIFT); + + if (paddr = rtas_rmo_buf paddr (rtas_rmo_buf + RTAS_RMOBUF_MAX)) + return 1; + + return 0; +} So this only exist whenever rtas.c is compiled... but ... + /* * Call early during boot, before mem init or bootmem, to retrieve the RTAS * informations from the device-tree and allocate the RMO buffer for userland diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c index c781bbc..d21dbc6 100644 --- a/arch/powerpc/mm/mem.c +++ b/arch/powerpc/mm/mem.c @@ -549,3 +549,23 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long address, hash_preload(vma-vm_mm, address, access, trap); #endif /* CONFIG_PPC_STD_MMU */ } + +extern int page_is_rtas(unsigned long pfn); + +/* + * devmem_is_allowed(): check to see if /dev/mem access to a certain address + * is valid. The argument is a physical page number. + * + * Access has to be given to non-kernel-ram areas as well, these contain the + * PCI mmio resources as well as potential bios/acpi data regions. + */ +int devmem_is_allowed(unsigned long pfn) +{ + if (iomem_is_exclusive(pfn PAGE_SHIFT)) + return 0; + if (!page_is_ram(pfn)) + return 1; + if (page_is_rtas(pfn)) + return 1; + return 0; +} This calls it unconditionally... you just broke the build of all !rtas platforms. Additionally, putting an extern definition like that in a .c file is gross at best Please instead, put in a header something like #ifdef CONFIG_PPC_RTAS extern int page_is_rtas(unsigned long pfn); #else static inline int page_is_rtas(unsigned long pfn) { } #endif And while at it, call it page_is_rtas_user_buf(); to make it clear what we are talking about, ie, not RTAS core per-se but specifically the RMO buffer. Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/3] serial: make bugs field not specific to 8250 type uarts.
On Thu, Dec 1, 2011 at 7:51 PM, Alan Cox a...@linux.intel.com wrote: Make the bugs field part of the globally visible struct uart_port and remove the 8250 specific one. Except all the bits in it are 8250 specific things or names that are meaningless in generic form - no. I also don't want to encourage flags and bug bits. We already have too many and its making the code a mess. So right now we want less not more. The bits in bugs are only passed through -- the idea is that there is no interpretation of them by any generic layer -- only that they prop from the arch down to the driver which knows what they mean. I can understand the want less not more mentality -- I was thinking the same thing when I was looking at the replication of fields between uart_port and uart_8250_port, and toying with the idea of helping clean that up (separate topic, to be sure.) If you have an idea in mind how arch/platform code should cleanly pass data about known uart bugs to the uart driver, then let me know what you have in mind. I've no real attachment to what I proposed here -- it just happened to be the solution I thought would be the least offensive. If there is a better idea floating around, I'll go ahead and try to implement it. Thanks, Paul. -- To unsubscribe from this list: send the line unsubscribe linux-serial in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/nvram: Add spinlock to oops_to_nvram to prevent oops in compression code.
On Thu, 2011-12-01 at 12:46 +1100, Anton Blanchard wrote: When issuing a system reset we almost always oops in the oops_to_nvram code because multiple CPUs are using the deflate work area. Add a spinlock to protect it. To play it safe I'm using trylock to avoid locking up if the NVRAM code oopses. This means we might miss multiple CPUs oopsing at exactly the same time but I think it's best to play it safe for now. Once we are happy with the reliability we can change it to a full spinlock. Signed-off-by: Anton Blanchard an...@samba.org Acked-by: Jim Keniston jkeni...@us.ibm.com --- Index: linux-build/arch/powerpc/platforms/pseries/nvram.c === --- linux-build.orig/arch/powerpc/platforms/pseries/nvram.c 2011-12-01 09:44:27.205568463 +1100 +++ linux-build/arch/powerpc/platforms/pseries/nvram.c2011-12-01 12:36:49.334478156 +1100 @@ -634,6 +634,8 @@ static void oops_to_nvram(struct kmsg_du { static unsigned int oops_count = 0; static bool panicking = false; + static DEFINE_SPINLOCK(lock); + unsigned long flags; size_t text_len; unsigned int err_type = ERR_TYPE_KERNEL_PANIC_GZ; int rc = -1; @@ -664,6 +666,9 @@ static void oops_to_nvram(struct kmsg_du if (clobbering_unread_rtas_event()) return; + if (!spin_trylock_irqsave(lock, flags)) + return; + if (big_oops_buf) { text_len = capture_last_msgs(old_msgs, old_len, new_msgs, new_len, big_oops_buf, big_oops_buf_sz); @@ -679,4 +684,6 @@ static void oops_to_nvram(struct kmsg_du (void) nvram_write_os_partition(oops_log_partition, oops_buf, (int) (sizeof(*oops_len) + *oops_len), err_type, ++oops_count); + + spin_unlock_irqrestore(lock, flags); } ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3] 8250: add workaround for MPC8[356]xx UART break IRQ storm
On Thu, Dec 1, 2011 at 7:57 PM, Alan Cox a...@linux.intel.com wrote: @@ -1553,7 +1554,15 @@ static void serial8250_handle_port(struct uart_8250_port *up) spin_lock_irqsave(up-port.lock, flags); - status = serial_inp(up, UART_LSR); + /* Workaround for IRQ storm errata on break with Freescale 16550 */ + if (UART_BUG_FSLBK up-port.bugs up-lsr_last UART_LSR_BI) { + up-lsr_last = ~UART_LSR_BI; + serial_inp(up, UART_RX); + spin_unlock_irqrestore(up-port.lock, flags); + return; + } + + status = up-lsr_last = serial_inp(up, UART_LSR); We've now had a recent pile of IRQ function changes adding more quirk bits and special casing. This doesn't scale. We either need to make handle_port a method the specific drivers can override or you could hide the mess in your serial_inp implementation and not touch any core code. To be fair, this one is zero cost for !PPC, but I understand your point, and the idea of hiding it somehow in serial_inp was something that never crossed my mind. I'll look into seeing if I can abuse that. I really don't mind which but I suspect dealing with it in your serial_inp handler so that when you read UART_LSR you do the fixup might be simplest providing you can just do that. Sorting out a -handle_port override is probably ultimately the right thing to do and then we can push some of the other funnies out further. At this point it's becoming increasingly clear that 8250 UART cloners are both very bad at cloning the things accurately, and very busy adding extra useful features so we need to start to treat 8250.c as a library you can wrap. Sad but true. It is like a time warp back to lib8390.c --- whee. P. Alan -- To unsubscribe from this list: send the line unsubscribe linux-serial in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 10/10] powerpc/mpic: Add in-core support for cascaded MPICs
Ben, I fixed the 3 issues that Paul and Michael reported and I can provide them to you two different ways, however you would prefer. I can also send the patches via email if that's more convenient. Option 1: Squashed into the the original patches for bisectability: git://opensource.exmeritus.com/hww-1u-1a/linux.git mpc85xx/mpic-and-stuff Option 2: As a fixup patch on the end: git://opensource.exmeritus.com/hww-1u-1a/linux.git mpc85xx/mpic-and-stuff-fixup You can also use HTTP if you would prefer: http://opensource.exmeritus.com/git/hww-1u-1a/linux.git Cheers, Kyle Moffett -- Curious about my work on the Debian powerpcspe port? I'm keeping a blog here: http://pureperl.blogspot.com/ ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[powerpc] boot up problem
Hi I just found that the 'next' branch you mentioned have problem to boot up. I test it in p1022ds and p1010rdb boards and the result are both the same. Note that for p1022ds I use make p1022ds.dtb to make the dtb file(36bit) with 36bit-uboot. And for p1010rdb I use all 32bit image. The problem list below: scsi0 : sata_fsl ata1: SATA max UDMA/133 irq 74 fsl-sata fffe19000.sata: Sata FSL Platform/CSB Driver init scsi1 : sata_fsl ata2: SATA max UDMA/133 irq 41 Fixed MDIO Bus: probed Unable to handle kernel paging request for data at address 0x Faulting instruction address: 0xc0451630 Oops: Kernel access of bad area, sig: 11 [#1] SMP NR_CPUS=2 P1022 DS Modules linked in: NIP: c0451630 LR: c0451618 CTR: 0007 REGS: ef03fce0 TRAP: 0300 Not tainted (3.2.0-rc3-00099-g883381d) MSR: 00029000 EE,ME,CE CR: 24042022 XER: DEAR: , ESR: 0080 TASK = ef04[1] 'swapper' THREAD: ef03e000 CPU: 0 GPR00: ef03fd98 ef03fd90 ef04 ef1ab22c 0002 ffeb fffe GPR08: b0541215 24042028 23c406c2 GPR16: ca00 0014 3fff 03ff9000 0015 7ff3a760 f1044030 fff4 GPR24: c053e128 ef1ab230 c056a3a8 ef03e000 ef04 ef1ab22c ef1ab228 NIP [c0451630] __mutex_lock_slowpath+0xb4/0x190 LR [c0451618] __mutex_lock_slowpath+0x9c/0x190 Call Trace: [ef03fdd0] [c0451758] mutex_lock+0x4c/0x50 [ef03fde0] [c02b5124] mdiobus_read+0x38/0x74 [ef03fe00] [c02b41f4] get_phy_id+0x24/0x80 [ef03fe20] [c02b9de4] fsl_pq_mdio_probe+0x3b4/0x580 [ef03feb0] [c0266120] platform_drv_probe+0x20/0x30 [ef03fec0] [c0264bbc] driver_probe_device+0xa4/0x1d4 [ef03fee0] [c0264da8] __driver_attach+0xbc/0xc0 [ef03ff00] [c0263ac0] bus_for_each_dev+0x60/0x9c [ef03ff30] [c02647f4] driver_attach+0x24/0x34 [ef03ff40] [c026] bus_add_driver+0x1ac/0x274 [ef03ff70] [c02651b0] driver_register+0x88/0x154 [ef03ff90] [c0266450] platform_driver_register+0x68/0x78 [ef03ffa0] [c05d93b8] fsl_pq_mdio_init+0x18/0x28 [ef03ffb0] [c0001eb8] do_one_initcall+0x34/0x1a8 [ef03ffe0] [c05bb82c] kernel_init+0xa0/0x13c [ef03fff0] [c000d878] kernel_thread+0x4c/0x68 Instruction dump: 801c0020 2f800063 419dffe8 3bbf0004 7fa3eb78 48001aad 813f000c 38010008 3b3f0008 901f000c 93210008 9121000c 3800 93810010 7c0004ac ---[ end trace 1643a9a9c5097f8f ]--- Kernel panic - not syncing: Attempted to kill init! Call Trace: [ef03fbc0] [c0008044] show_stack+0x44/0x154 (unreliable) [ef03fc00] [c04532c8] panic+0xa4/0x1d8 [ef03fc50] [c0049a00] do_exit+0x5dc/0x684 [ef03fca0] [c000a6f0] die+0xdc/0x1b4 [ef03fcc0] [c00128d0] bad_page_fault+0xb4/0xfc [ef03fcd0] [c000ebe4] handle_page_fault+0x7c/0x80 --- Exception: 300 at __mutex_lock_slowpath+0xb4/0x190 LR = __mutex_lock_slowpath+0x9c/0x190 [ef03fd90] [] (null) (unreliable) [ef03fdd0] [c0451758] mutex_lock+0x4c/0x50 [ef03fde0] [c02b5124] mdiobus_read+0x38/0x74 [ef03fe00] [c02b41f4] get_phy_id+0x24/0x80 [ef03fe20] [c02b9de4] fsl_pq_mdio_probe+0x3b4/0x580 [ef03feb0] [c0266120] platform_drv_probe+0x20/0x30 [ef03fec0] [c0264bbc] driver_probe_device+0xa4/0x1d4 [ef03fee0] [c0264da8] __driver_attach+0xbc/0xc0 [ef03ff00] [c0263ac0] bus_for_each_dev+0x60/0x9c [ef03ff30] [c02647f4] driver_attach+0x24/0x34 [ef03ff40] [c026] bus_add_driver+0x1ac/0x274 [ef03ff70] [c02651b0] driver_register+0x88/0x154 [ef03ff90] [c0266450] platform_driver_register+0x68/0x78 [ef03ffa0] [c05d93b8] fsl_pq_mdio_init+0x18/0x28 [ef03ffb0] [c0001eb8] do_one_initcall+0x34/0x1a8 [ef03ffe0] [c05bb82c] kernel_init+0xa0/0x13c [ef03fff0] [c000d878] kernel_thread+0x4c/0x68 Rebooting in 180 seconds.. Do you or anyone else have any idea about this? Thanks. -Original Message- From: Kumar Gala [mailto:ga...@kernel.crashing.org] Sent: Thursday, November 24, 2011 3:10 PM To: Jia Hongtao-B38951 Cc: Li Yang-R58472 Subject: Re: [PATCH 1/2] Unify pci/pcie initialization code When you do this please do it against my latest upstream 'next' branch on: http://git.kernel.org/?p=linux/kernel/git/galak/powerpc.git git://git.kernel.org/pub/scm/linux/kernel/git/galak/powerpc.git - k On Nov 24, 2011, at 12:27 AM, Jia Hongtao-B38951 wrote: Ok, I got it. - Hongtao -Original Message- From: Li Yang-R58472 Sent: Thursday, November 24, 2011 2:06 PM To: Jia Hongtao-B38951; Kumar Gala Subject: RE: [PATCH 1/2] Unify pci/pcie initialization code Hongtao, Please update all the boards currently using fsl_add_bridge(). - Leo -Original Message- From: Kumar Gala [mailto:ga...@kernel.crashing.org] Sent: Thursday, November 24, 2011 12:51 PM To: Jia Hongtao-B38951 Cc: linuxppc-dev@lists.ozlabs.org; Gala Kumar-B11780; Li Yang-R58472 Subject: Re: [PATCH 1/2] Unify pci/pcie initialization code On Nov 22, 2011, at 12:20 AM, Jia Hongtao-B38951 wrote: Hi Kumar, We want more comments on this series of patches ([1/2] [2/2]) to speed up the pushing-to-kernel progress. Thanks. I think the code is fine, but you need to update it for all the
Re: [PATCH 10/10] powerpc/mpic: Add in-core support for cascaded MPICs
On Thu, 2011-12-01 at 19:48 -0600, Moffett, Kyle D wrote: Ben, I fixed the 3 issues that Paul and Michael reported and I can provide them to you two different ways, however you would prefer. I can also send the patches via email if that's more convenient. Option 1: Squashed into the the original patches for bisectability: git://opensource.exmeritus.com/hww-1u-1a/linux.git mpc85xx/mpic-and-stuff This option, please just re-send to the list. Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH][v2] Enable CONFIG_STRICT_DEVMEM support for Powerpc
Benjamin Herrenschmidt [b...@kernel.crashing.org] wrote: | And an additional comment regarding the rtas bit: | +int devmem_is_allowed(unsigned long pfn) | +{ | + if (iomem_is_exclusive(pfn PAGE_SHIFT)) | + return 0; | + if (!page_is_ram(pfn)) | + return 1; | + if (page_is_rtas(pfn)) | + return 1; | + return 0; | +} | | This calls it unconditionally... you just broke the build of all !rtas | platforms. Additionally, putting an extern definition like that in a .c | file is gross at best Oh, Sorry. | | Please instead, put in a header something like | | #ifdef CONFIG_PPC_RTAS | extern int page_is_rtas(unsigned long pfn); | #else | static inline int page_is_rtas(unsigned long pfn) { } | #endif | | And while at it, call it page_is_rtas_user_buf(); to make it clear what | we are talking about, ie, not RTAS core per-se but specifically the RMO | buffer. Ok. I will rename, move the declaration to asm/rtas.h and resend the incremental patch. Sukadev ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH][RFC] fsldma: fix performance degradation by optimizing spinlock use.
Hi Iris, I'm convinced that smp_rmb() is needed when removing the spinlock. As noted, Documentation/memory-barriers.txt says that stores on one CPU can be observed by another CPU in a different order. Previously, there was an UNLOCK (in fsl_dma_tx_submit) followed by a LOCK (in fsl_tx_status). This provided a full barrier, forcing the operations to complete correctly when viewed by the second CPU. I do not agree this smp_rmb() works here. Because when this smp_rmb() executed and begin to read chan-common.cookie, you still cannot avoid the order issue. Something like one is reading old value, but another CPU is updating the new value. My point is here the order is not important for the DMA decision. Completed DMA tx is decided as not complete is not a big deal, because next time it will be OK. I believe there is no case that could cause uncompleted DMA tx is decided as completed, because the fsl_tx_status is called after fsl_dma_tx_submit for a specific cookie. If you can give me an example here, I will agree with you. Thanks, Forrest -Original Message- From: Ira W. Snyder [mailto:i...@ovro.caltech.edu] Sent: 2011年12月1日 1:08 To: Shi Xuelin-B29237 Cc: vinod.k...@intel.com; dan.j.willi...@intel.com; linuxppc-dev@lists.ozlabs.org; linux-ker...@vger.kernel.org; Li Yang-R58472 Subject: Re: [PATCH][RFC] fsldma: fix performance degradation by optimizing spinlock use. On Wed, Nov 30, 2011 at 09:57:47AM +, Shi Xuelin-B29237 wrote: Hello Ira, In drivers/dma/dmaengine.c, we have below tight loop to check DMA completion in mainline Linux: do { status = dma_async_is_tx_complete(chan, cookie, NULL, NULL); if (time_after_eq(jiffies, dma_sync_wait_timeout)) { printk(KERN_ERR dma_sync_wait_timeout!\n); return DMA_ERROR; } } while (status == DMA_IN_PROGRESS); That is the body of dma_sync_wait(). It is mostly used in the raid code. I understand that you don't want to change the raid code to use callbacks. In any case, I think we've strayed from the topic under consideration, which is: can we remove this spinlock without introducing a bug. I'm convinced that smp_rmb() is needed when removing the spinlock. As noted, Documentation/memory-barriers.txt says that stores on one CPU can be observed by another CPU in a different order. Previously, there was an UNLOCK (in fsl_dma_tx_submit) followed by a LOCK (in fsl_tx_status). This provided a full barrier, forcing the operations to complete correctly when viewed by the second CPU. From the text: Therefore, from (1), (2) and (4) an UNLOCK followed by an unconditional LOCK is equivalent to a full barrier, but a LOCK followed by an UNLOCK is not. Also, please read EXAMPLES OF MEMORY BARRIER SEQUENCES and INTER-CPU LOCKING BARRIER EFFECTS. Particularly, in EXAMPLES OF MEMORY BARRIER SEQUENCES, the text notes: Without intervention, CPU 2 may perceive the events on CPU 1 in some effectively random order, despite the write barrier issued by CPU 1: [snip diagram] And thirdly, a read barrier acts as a partial order on loads. Consider the following sequence of events: [snip diagram] Without intervention, CPU 2 may then choose to perceive the events on CPU 1 in some effectively random order, despite the write barrier issued by CPU 1: [snip diagram] And so on. Please read this entire section in the document. I can't give you an ACK on the proposed patch. To the best of my understanding, I believe it introduces a bug. I've tried to provide as much evidence for this belief as I can, in the form of documentation in the kernel source tree. If you can cite some documentation that shows I am wrong, I will happily change my mind! Ira -Original Message- From: Ira W. Snyder [mailto:i...@ovro.caltech.edu] Sent: 2011年11月30日 1:26 To: Li Yang-R58472 Cc: Shi Xuelin-B29237; vinod.k...@intel.com; dan.j.willi...@intel.com; linuxppc-dev@lists.ozlabs.org; linux-ker...@vger.kernel.org Subject: Re: [PATCH][RFC] fsldma: fix performance degradation by optimizing spinlock use. On Tue, Nov 29, 2011 at 03:19:05AM +, Li Yang-R58472 wrote: Subject: Re: [PATCH][RFC] fsldma: fix performance degradation by optimizing spinlock use. On Thu, Nov 24, 2011 at 08:12:25AM +, Shi Xuelin-B29237 wrote: Hi Ira, Thanks for your review. After second thought, I think your scenario may not occur. Because the cookie 20 we query must be returned by fsl_dma_tx_submit(...) in practice. We never query a cookie not returned by fsl_dma_tx_submit(...). I agree about this part. When we call fsl_tx_status(20), the chan-common.cookie is definitely wrote as 20 and cpu2 could not read as 19. This is what I don't agree about. However, I'm not an expert on CPU cache vs. memory accesses in an multi-processor system. The section
Re: [linuxppc-release] [powerpc] boot up problem
Jia Hongtao-B38951 wrote: Hi I just found that the 'next' branch you mentioned have problem to boot up. I test it in p1022ds and p1010rdb boards and the result are both the same. Note that for p1022ds I use make p1022ds.dtb to make the dtb file(36bit) with 36bit-uboot. And for p1010rdb I use all 32bit image. The problem list below: scsi0 : sata_fsl ata1: SATA max UDMA/133 irq 74 fsl-sata fffe19000.sata: Sata FSL Platform/CSB Driver init scsi1 : sata_fsl ata2: SATA max UDMA/133 irq 41 Fixed MDIO Bus: probed Unable to handle kernel paging request for data at address 0x Faulting instruction address: 0xc0451630 Oops: Kernel access of bad area, sig: 11 [#1] Andy has phy driver patch that fixes this. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [linuxppc-release] [powerpc] boot up problem
On Dec 1, 2011, at 8:21 PM, Jia Hongtao-B38951 wrote: Hi I just found that the 'next' branch you mentioned have problem to boot up. I test it in p1022ds and p1010rdb boards and the result are both the same. Note that for p1022ds I use make p1022ds.dtb to make the dtb file(36bit) with 36bit-uboot. And for p1010rdb I use all 32bit image. The problem list below: Can you try Linus's tree and v3.2-rc4 http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=summary and see if you have same issue. scsi0 : sata_fsl ata1: SATA max UDMA/133 irq 74 fsl-sata fffe19000.sata: Sata FSL Platform/CSB Driver init scsi1 : sata_fsl ata2: SATA max UDMA/133 irq 41 Fixed MDIO Bus: probed Unable to handle kernel paging request for data at address 0x Faulting instruction address: 0xc0451630 Oops: Kernel access of bad area, sig: 11 [#1] SMP NR_CPUS=2 P1022 DS Modules linked in: NIP: c0451630 LR: c0451618 CTR: 0007 REGS: ef03fce0 TRAP: 0300 Not tainted (3.2.0-rc3-00099-g883381d) MSR: 00029000 EE,ME,CE CR: 24042022 XER: DEAR: , ESR: 0080 TASK = ef04[1] 'swapper' THREAD: ef03e000 CPU: 0 GPR00: ef03fd98 ef03fd90 ef04 ef1ab22c 0002 ffeb fffe GPR08: b0541215 24042028 23c406c2 GPR16: ca00 0014 3fff 03ff9000 0015 7ff3a760 f1044030 fff4 GPR24: c053e128 ef1ab230 c056a3a8 ef03e000 ef04 ef1ab22c ef1ab228 NIP [c0451630] __mutex_lock_slowpath+0xb4/0x190 LR [c0451618] __mutex_lock_slowpath+0x9c/0x190 Call Trace: [ef03fdd0] [c0451758] mutex_lock+0x4c/0x50 [ef03fde0] [c02b5124] mdiobus_read+0x38/0x74 [ef03fe00] [c02b41f4] get_phy_id+0x24/0x80 [ef03fe20] [c02b9de4] fsl_pq_mdio_probe+0x3b4/0x580 [ef03feb0] [c0266120] platform_drv_probe+0x20/0x30 [ef03fec0] [c0264bbc] driver_probe_device+0xa4/0x1d4 [ef03fee0] [c0264da8] __driver_attach+0xbc/0xc0 [ef03ff00] [c0263ac0] bus_for_each_dev+0x60/0x9c [ef03ff30] [c02647f4] driver_attach+0x24/0x34 [ef03ff40] [c026] bus_add_driver+0x1ac/0x274 [ef03ff70] [c02651b0] driver_register+0x88/0x154 [ef03ff90] [c0266450] platform_driver_register+0x68/0x78 [ef03ffa0] [c05d93b8] fsl_pq_mdio_init+0x18/0x28 [ef03ffb0] [c0001eb8] do_one_initcall+0x34/0x1a8 [ef03ffe0] [c05bb82c] kernel_init+0xa0/0x13c [ef03fff0] [c000d878] kernel_thread+0x4c/0x68 Instruction dump: 801c0020 2f800063 419dffe8 3bbf0004 7fa3eb78 48001aad 813f000c 38010008 3b3f0008 901f000c 93210008 9121000c 3800 93810010 7c0004ac ---[ end trace 1643a9a9c5097f8f ]--- Kernel panic - not syncing: Attempted to kill init! Call Trace: [ef03fbc0] [c0008044] show_stack+0x44/0x154 (unreliable) [ef03fc00] [c04532c8] panic+0xa4/0x1d8 [ef03fc50] [c0049a00] do_exit+0x5dc/0x684 [ef03fca0] [c000a6f0] die+0xdc/0x1b4 [ef03fcc0] [c00128d0] bad_page_fault+0xb4/0xfc [ef03fcd0] [c000ebe4] handle_page_fault+0x7c/0x80 --- Exception: 300 at __mutex_lock_slowpath+0xb4/0x190 LR = __mutex_lock_slowpath+0x9c/0x190 [ef03fd90] [] (null) (unreliable) [ef03fdd0] [c0451758] mutex_lock+0x4c/0x50 [ef03fde0] [c02b5124] mdiobus_read+0x38/0x74 [ef03fe00] [c02b41f4] get_phy_id+0x24/0x80 [ef03fe20] [c02b9de4] fsl_pq_mdio_probe+0x3b4/0x580 [ef03feb0] [c0266120] platform_drv_probe+0x20/0x30 [ef03fec0] [c0264bbc] driver_probe_device+0xa4/0x1d4 [ef03fee0] [c0264da8] __driver_attach+0xbc/0xc0 [ef03ff00] [c0263ac0] bus_for_each_dev+0x60/0x9c [ef03ff30] [c02647f4] driver_attach+0x24/0x34 [ef03ff40] [c026] bus_add_driver+0x1ac/0x274 [ef03ff70] [c02651b0] driver_register+0x88/0x154 [ef03ff90] [c0266450] platform_driver_register+0x68/0x78 [ef03ffa0] [c05d93b8] fsl_pq_mdio_init+0x18/0x28 [ef03ffb0] [c0001eb8] do_one_initcall+0x34/0x1a8 [ef03ffe0] [c05bb82c] kernel_init+0xa0/0x13c [ef03fff0] [c000d878] kernel_thread+0x4c/0x68 Rebooting in 180 seconds.. Do you or anyone else have any idea about this? Thanks. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [linuxppc-release] [powerpc] boot up problem
On Dec 1, 2011, at 9:55 PM, Tabi Timur-B04825 wrote: Jia Hongtao-B38951 wrote: Hi I just found that the 'next' branch you mentioned have problem to boot up. I test it in p1022ds and p1010rdb boards and the result are both the same. Note that for p1022ds I use make p1022ds.dtb to make the dtb file(36bit) with 36bit-uboot. And for p1010rdb I use all 32bit image. The problem list below: scsi0 : sata_fsl ata1: SATA max UDMA/133 irq 74 fsl-sata fffe19000.sata: Sata FSL Platform/CSB Driver init scsi1 : sata_fsl ata2: SATA max UDMA/133 irq 41 Fixed MDIO Bus: probed Unable to handle kernel paging request for data at address 0x Faulting instruction address: 0xc0451630 Oops: Kernel access of bad area, sig: 11 [#1] Andy has phy driver patch that fixes this. Andy, What's going on here? Is there a fix in v3.2-rc4? - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] powerpc: Add support for OpenBlockS 600
So I've had one of these for a while and it looks like the vendor never bothered submitting the support upstream. This adds it using ppc40x_simple and provides a device-tree. There are some changes to the boot wrapper because the way u-boot works on this thing, it seems to expect a multipart image with the kernel, initrd and dtb in it. The USB support is missing as it needs the yet unmerged driver for the DWC OTG part and the GPIOs may need further definition in the dts. Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org --- arch/powerpc/Makefile |2 +- arch/powerpc/boot/Makefile |7 + arch/powerpc/boot/dts/obs600.dts | 314 arch/powerpc/boot/wrapper | 22 ++- arch/powerpc/configs/40x/obs600_defconfig | 83 arch/powerpc/platforms/40x/Kconfig | 10 + arch/powerpc/platforms/40x/ppc40x_simple.c |3 +- 7 files changed, 438 insertions(+), 3 deletions(-) create mode 100644 arch/powerpc/boot/dts/obs600.dts create mode 100644 arch/powerpc/configs/40x/obs600_defconfig diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile index 59f462b..ffe4d88 100644 --- a/arch/powerpc/Makefile +++ b/arch/powerpc/Makefile @@ -165,7 +165,7 @@ all: zImage # With make 3.82 we cannot mix normal and wildcard targets BOOT_TARGETS1 := zImage zImage.initrd uImage -BOOT_TARGETS2 := zImage% dtbImage% treeImage.% cuImage.% simpleImage.% +BOOT_TARGETS2 := zImage% dtbImage% treeImage.% cuImage.% simpleImage.% uImage.% PHONY += $(BOOT_TARGETS1) $(BOOT_TARGETS2) diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile index 72ee8c1..80c133c 100644 --- a/arch/powerpc/boot/Makefile +++ b/arch/powerpc/boot/Makefile @@ -199,6 +199,7 @@ image-$(CONFIG_EP405) += dtbImage.ep405 image-$(CONFIG_HOTFOOT)+= cuImage.hotfoot image-$(CONFIG_WALNUT) += treeImage.walnut image-$(CONFIG_ACADIA) += cuImage.acadia +image-$(CONFIG_OBS600) += uImage.obs600 # Board ports in arch/powerpc/platform/44x/Kconfig image-$(CONFIG_EBONY) += treeImage.ebony cuImage.ebony @@ -316,6 +317,12 @@ $(obj)/zImage.iseries: vmlinux $(obj)/uImage: vmlinux $(wrapperbits) $(call if_changed,wrap,uboot) +$(obj)/uImage.initrd.%: vmlinux $(obj)/%.dtb $(wrapperbits) + $(call if_changed,wrap,uboot-$*,,$(obj)/$*.dtb,$(obj)/ramdisk.image.gz) + +$(obj)/uImage.%: vmlinux $(obj)/%.dtb $(wrapperbits) + $(call if_changed,wrap,uboot-$*,,$(obj)/$*.dtb) + $(obj)/cuImage.initrd.%: vmlinux $(obj)/%.dtb $(wrapperbits) $(call if_changed,wrap,cuboot-$*,,$(obj)/$*.dtb,$(obj)/ramdisk.image.gz) diff --git a/arch/powerpc/boot/dts/obs600.dts b/arch/powerpc/boot/dts/obs600.dts new file mode 100644 index 000..daad350 --- /dev/null +++ b/arch/powerpc/boot/dts/obs600.dts @@ -0,0 +1,314 @@ +/* + * Device Tree Source for PlatHome OpenBlockS 600 (405EX) + * + * Copyright 2011 Ben Herrenschmidt, IBM Corp. + * + * Based on Kilauea by: + * + * Copyright 2007-2009 DENX Software Engineering, Stefan Roese s...@denx.de + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed as is without + * any warranty of any kind, whether express or implied. + */ + +/dts-v1/; + +/ { + #address-cells = 1; + #size-cells = 1; + model = PlatHome,OpenBlockS 600; + compatible = plathome,obs600; + dcr-parent = {/cpus/cpu@0}; + + aliases { + ethernet0 = EMAC0; + ethernet1 = EMAC1; + serial0 = UART0; + serial1 = UART1; + }; + + cpus { + #address-cells = 1; + #size-cells = 0; + + cpu@0 { + device_type = cpu; + model = PowerPC,405EX; + reg = 0x; + clock-frequency = 0; /* Filled in by U-Boot */ + timebase-frequency = 0; /* Filled in by U-Boot */ + i-cache-line-size = 32; + d-cache-line-size = 32; + i-cache-size = 16384; /* 16 kB */ + d-cache-size = 16384; /* 16 kB */ + dcr-controller; + dcr-access-method = native; + }; + }; + + memory { + device_type = memory; + reg = 0x 0x; /* Filled in by U-Boot */ + }; + + UIC0: interrupt-controller { + compatible = ibm,uic-405ex, ibm,uic; + interrupt-controller; + cell-index = 0; + dcr-reg = 0x0c0 0x009; + #address-cells = 0; + #size-cells = 0; + #interrupt-cells = 2; + }; + + UIC1: interrupt-controller1 { + compatible =
Re: [PATCH] powerpc: Add support for OpenBlockS 600
Hi, On Thu, Dec 1, 2011 at 9:35 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: arch/powerpc/boot/dts/obs600.dts | 314 It'd be nice to have dtsi files for the SoCs, there's a lot of boiler plate in there. That shouldn't hold up this specific patch though. arch/powerpc/boot/wrapper | 22 ++- arch/powerpc/configs/40x/obs600_defconfig | 83 A new single-board defconfig? On arm that is a strong NACK, I'd say it's a bad idea on powerpc as well. -Olof ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [linuxppc-release] [powerpc] boot up problem
Yes, both v3.2-rc3 and v3.2-rc4 in Linus's tree have the same issue. - Hongtao. -Original Message- From: Kumar Gala [mailto:ga...@kernel.crashing.org] Sent: Friday, December 02, 2011 12:12 PM To: Jia Hongtao-B38951 Cc: LinuxPPC-dev list; Li Yang-R58472 Subject: Re: [linuxppc-release] [powerpc] boot up problem On Dec 1, 2011, at 8:21 PM, Jia Hongtao-B38951 wrote: Hi I just found that the 'next' branch you mentioned have problem to boot up. I test it in p1022ds and p1010rdb boards and the result are both the same. Note that for p1022ds I use make p1022ds.dtb to make the dtb file(36bit) with 36bit-uboot. And for p1010rdb I use all 32bit image. The problem list below: Can you try Linus's tree and v3.2-rc4 http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=summary and see if you have same issue. scsi0 : sata_fsl ata1: SATA max UDMA/133 irq 74 fsl-sata fffe19000.sata: Sata FSL Platform/CSB Driver init scsi1 : sata_fsl ata2: SATA max UDMA/133 irq 41 Fixed MDIO Bus: probed Unable to handle kernel paging request for data at address 0x Faulting instruction address: 0xc0451630 Oops: Kernel access of bad area, sig: 11 [#1] SMP NR_CPUS=2 P1022 DS Modules linked in: NIP: c0451630 LR: c0451618 CTR: 0007 REGS: ef03fce0 TRAP: 0300 Not tainted (3.2.0-rc3-00099-g883381d) MSR: 00029000 EE,ME,CE CR: 24042022 XER: DEAR: , ESR: 0080 TASK = ef04[1] 'swapper' THREAD: ef03e000 CPU: 0 GPR00: ef03fd98 ef03fd90 ef04 ef1ab22c 0002 ffeb fffe GPR08: b0541215 24042028 23c406c2 GPR16: ca00 0014 3fff 03ff9000 0015 7ff3a760 f1044030 fff4 GPR24: c053e128 ef1ab230 c056a3a8 ef03e000 ef04 ef1ab22c ef1ab228 NIP [c0451630] __mutex_lock_slowpath+0xb4/0x190 LR [c0451618] __mutex_lock_slowpath+0x9c/0x190 Call Trace: [ef03fdd0] [c0451758] mutex_lock+0x4c/0x50 [ef03fde0] [c02b5124] mdiobus_read+0x38/0x74 [ef03fe00] [c02b41f4] get_phy_id+0x24/0x80 [ef03fe20] [c02b9de4] fsl_pq_mdio_probe+0x3b4/0x580 [ef03feb0] [c0266120] platform_drv_probe+0x20/0x30 [ef03fec0] [c0264bbc] driver_probe_device+0xa4/0x1d4 [ef03fee0] [c0264da8] __driver_attach+0xbc/0xc0 [ef03ff00] [c0263ac0] bus_for_each_dev+0x60/0x9c [ef03ff30] [c02647f4] driver_attach+0x24/0x34 [ef03ff40] [c026] bus_add_driver+0x1ac/0x274 [ef03ff70] [c02651b0] driver_register+0x88/0x154 [ef03ff90] [c0266450] platform_driver_register+0x68/0x78 [ef03ffa0] [c05d93b8] fsl_pq_mdio_init+0x18/0x28 [ef03ffb0] [c0001eb8] do_one_initcall+0x34/0x1a8 [ef03ffe0] [c05bb82c] kernel_init+0xa0/0x13c [ef03fff0] [c000d878] kernel_thread+0x4c/0x68 Instruction dump: 801c0020 2f800063 419dffe8 3bbf0004 7fa3eb78 48001aad 813f000c 38010008 3b3f0008 901f000c 93210008 9121000c 3800 93810010 7c0004ac ---[ end trace 1643a9a9c5097f8f ]--- Kernel panic - not syncing: Attempted to kill init! Call Trace: [ef03fbc0] [c0008044] show_stack+0x44/0x154 (unreliable) [ef03fc00] [c04532c8] panic+0xa4/0x1d8 [ef03fc50] [c0049a00] do_exit+0x5dc/0x684 [ef03fca0] [c000a6f0] die+0xdc/0x1b4 [ef03fcc0] [c00128d0] bad_page_fault+0xb4/0xfc [ef03fcd0] [c000ebe4] handle_page_fault+0x7c/0x80 --- Exception: 300 at __mutex_lock_slowpath+0xb4/0x190 LR = __mutex_lock_slowpath+0x9c/0x190 [ef03fd90] [] (null) (unreliable) [ef03fdd0] [c0451758] mutex_lock+0x4c/0x50 [ef03fde0] [c02b5124] mdiobus_read+0x38/0x74 [ef03fe00] [c02b41f4] get_phy_id+0x24/0x80 [ef03fe20] [c02b9de4] fsl_pq_mdio_probe+0x3b4/0x580 [ef03feb0] [c0266120] platform_drv_probe+0x20/0x30 [ef03fec0] [c0264bbc] driver_probe_device+0xa4/0x1d4 [ef03fee0] [c0264da8] __driver_attach+0xbc/0xc0 [ef03ff00] [c0263ac0] bus_for_each_dev+0x60/0x9c [ef03ff30] [c02647f4] driver_attach+0x24/0x34 [ef03ff40] [c026] bus_add_driver+0x1ac/0x274 [ef03ff70] [c02651b0] driver_register+0x88/0x154 [ef03ff90] [c0266450] platform_driver_register+0x68/0x78 [ef03ffa0] [c05d93b8] fsl_pq_mdio_init+0x18/0x28 [ef03ffb0] [c0001eb8] do_one_initcall+0x34/0x1a8 [ef03ffe0] [c05bb82c] kernel_init+0xa0/0x13c [ef03fff0] [c000d878] kernel_thread+0x4c/0x68 Rebooting in 180 seconds.. Do you or anyone else have any idea about this? Thanks. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] pq3: Add default tbi address
The MDIO driver has been changed so that it no longer supports scanning the MDIO bus for a free address for the TBI PHY. This feature was fragile, and required scanning the bus before the bus was fully up and registered. The intended way for FSL devices to specify the TBI PHY's address is via a tbi node in the device tree. All of the device trees had such a node, except for the recent p1/p2 trees. Rather than hand-fixing all of those boards, set a default value (most boards were using 0x11, anyway), and any board which wants to change it from that default can then override it in its board dts file. This fixes an issue where p1/p2 boards would fail to bring up Ethernet, due to not finding a tbi node. Signed-off-by: Andy Fleming aflem...@freescale.com --- arch/powerpc/boot/dts/fsl/pq3-etsec1-0.dtsi |5 + arch/powerpc/boot/dts/fsl/pq3-etsec2-0.dtsi |5 + 2 files changed, 10 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/boot/dts/fsl/pq3-etsec1-0.dtsi b/arch/powerpc/boot/dts/fsl/pq3-etsec1-0.dtsi index a1979ae..0a42e21 100644 --- a/arch/powerpc/boot/dts/fsl/pq3-etsec1-0.dtsi +++ b/arch/powerpc/boot/dts/fsl/pq3-etsec1-0.dtsi @@ -50,4 +50,9 @@ mdio@24520 { #size-cells = 0; compatible = fsl,gianfar-mdio; reg = 0x24520 0x20; + + tbi-phy@11 { + device-type = tbi-phy; + reg = 0x11; + }; }; diff --git a/arch/powerpc/boot/dts/fsl/pq3-etsec2-0.dtsi b/arch/powerpc/boot/dts/fsl/pq3-etsec2-0.dtsi index 1382fec..964670a 100644 --- a/arch/powerpc/boot/dts/fsl/pq3-etsec2-0.dtsi +++ b/arch/powerpc/boot/dts/fsl/pq3-etsec2-0.dtsi @@ -38,6 +38,11 @@ mdio@24000 { #size-cells = 0; compatible = fsl,etsec2-mdio; reg = 0x24000 0x1000 0xb0030 0x4; + + tbi-phy@11 { + device-type = tbi-phy; + reg = 0x11; + }; }; ethernet@b { -- 1.7.3.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] Revert net: fsl_pq_mdio: fix non tbi phy access
This reverts commit c3e072f8a6c5625028531c40ec65f7e301531be2. The TBI PHY Address must always be set to something sensible. If not, the value currently in the register may interfere with MDIO transactions to that address. The architected solution is to have a TBI node in the device tree, which corresponds to the desired address (and which should be chosen so that it does not conflict with other PHYs on the bus). If that node is not there, it is incorrect to just continue. We must return an error, so that developers have a chance to realize they've mis-configured their device trees. A separate patch has been submitted to add such a node to the device trees for boards which were missing that node. Signed-off-by: Andy Fleming aflem...@freescale.com --- drivers/net/ethernet/freescale/fsl_pq_mdio.c | 12 ++-- 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/freescale/fsl_pq_mdio.c b/drivers/net/ethernet/freescale/fsl_pq_mdio.c index f109602..6ff124c 100644 --- a/drivers/net/ethernet/freescale/fsl_pq_mdio.c +++ b/drivers/net/ethernet/freescale/fsl_pq_mdio.c @@ -356,16 +356,16 @@ static int fsl_pq_mdio_probe(struct platform_device *ofdev) if (prop) tbiaddr = *prop; + } - if (tbiaddr == -1) { - err = -EBUSY; + if (tbiaddr == -1) { + err = -EBUSY; - goto err_free_irqs; - } else { - out_be32(tbipa, tbiaddr); - } + goto err_free_irqs; } + out_be32(tbipa, tbiaddr); + err = of_mdiobus_register(new_bus, np); if (err) { printk (KERN_ERR %s: Cannot register as MDIO bus\n, -- 1.7.3.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] pq3: Add default tbi address
On Dec 2, 2011, at 1:05 AM, Andy Fleming wrote: The MDIO driver has been changed so that it no longer supports scanning the MDIO bus for a free address for the TBI PHY. This feature was fragile, and required scanning the bus before the bus was fully up and registered. The intended way for FSL devices to specify the TBI PHY's address is via a tbi node in the device tree. All of the device trees had such a node, except for the recent p1/p2 trees. Rather than hand-fixing all of those boards, set a default value (most boards were using 0x11, anyway), and any board which wants to change it from that default can then override it in its board dts file. This fixes an issue where p1/p2 boards would fail to bring up Ethernet, due to not finding a tbi node. Is this only needed on 1st controller because its what has external PHY control? Signed-off-by: Andy Fleming aflem...@freescale.com --- arch/powerpc/boot/dts/fsl/pq3-etsec1-0.dtsi |5 + arch/powerpc/boot/dts/fsl/pq3-etsec2-0.dtsi |5 + 2 files changed, 10 insertions(+), 0 deletions(-) This doesn't seem correct, meaning this should really be in the board .dts not in the IP. I think the driver should check and warn if this property doesn't exist. diff --git a/arch/powerpc/boot/dts/fsl/pq3-etsec1-0.dtsi b/arch/powerpc/boot/dts/fsl/pq3-etsec1-0.dtsi index a1979ae..0a42e21 100644 --- a/arch/powerpc/boot/dts/fsl/pq3-etsec1-0.dtsi +++ b/arch/powerpc/boot/dts/fsl/pq3-etsec1-0.dtsi @@ -50,4 +50,9 @@ mdio@24520 { #size-cells = 0; compatible = fsl,gianfar-mdio; reg = 0x24520 0x20; + + tbi-phy@11 { + device-type = tbi-phy; + reg = 0x11; + }; }; diff --git a/arch/powerpc/boot/dts/fsl/pq3-etsec2-0.dtsi b/arch/powerpc/boot/dts/fsl/pq3-etsec2-0.dtsi index 1382fec..964670a 100644 --- a/arch/powerpc/boot/dts/fsl/pq3-etsec2-0.dtsi +++ b/arch/powerpc/boot/dts/fsl/pq3-etsec2-0.dtsi @@ -38,6 +38,11 @@ mdio@24000 { #size-cells = 0; compatible = fsl,etsec2-mdio; reg = 0x24000 0x1000 0xb0030 0x4; + + tbi-phy@11 { + device-type = tbi-phy; + reg = 0x11; + }; }; ethernet@b { -- 1.7.3.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev