RE: [PATCH V4 3/3] powerpc/85xx: Add TWR-P1025 board support
-Original Message- From: Wood Scott-B07421 Sent: Thursday, September 26, 2013 7:10 AM To: Xie Xiaobo-R63061 Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org; Johnston Michael- R49610 Subject: Re: [PATCH V4 3/3] powerpc/85xx: Add TWR-P1025 board support On Wed, 2013-09-25 at 04:50 -0500, Xie Xiaobo-R63061 wrote: Hi Scott, See the reply inline. -Original Message- From: Wood Scott-B07421 Sent: Wednesday, September 25, 2013 7:22 AM To: Xie Xiaobo-R63061 Cc: linuxppc-dev@lists.ozlabs.org; Johnston Michael-R49610 Subject: Re: [PATCH V4 3/3] powerpc/85xx: Add TWR-P1025 board support On Tue, 2013-09-24 at 18:48 +0800, Xie Xiaobo wrote: + partition@8 { + /* 3.5 MB for Linux Kernel Image */ + reg = 0x0008 0x0038; + label = NOR Linux Kernel Image; + }; Is this enough? I will enlarge it to 6MB. + partition@40 { + /* 58.75MB for JFFS2 based Root file System */ + reg = 0x0040 0x03ac; + label = NOR Root File System; + }; Don't specify jffs2. OK, I will remove jffs2 + /* CS2 for Display */ + ssd1289@2,0 { + #address-cells = 1; + #size-cells = 1; + compatible = ssd1289; + reg = 0x2 0x 0x0002 + 0x2 0x0002 0x0002; + }; Node names should be generic. What does ssd1289 do? If this is actually the display device, then it should be called display@2,0. OK. The ssd1289 is a LCD controller. How about a vendor prefix on that compatible? Why #address-cells/#size- cells despite no child nodes? Where is a binding that says what each of those two reg resources mean? I will add the vendor prefix. I review the ssd1289 driver, and the #address-cells/#size-cells were un-used. I will remove them. And a binding? Why do you need two separate reg resources rather than just 2 0 4? Will they ever be discontiguous? [Xie] I review the ssd1289 driver code, and found the driver need two reg resources, if change the dts, the driver also should be modified accordingly. So I remove the ssd1289 node from this patch. I will submit new patch include the dts modification, ssd1289 driver and the binding. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH V4 3/3] powerpc/85xx: Add TWR-P1025 board support
Hi Scott, See the reply inline. -Original Message- From: Wood Scott-B07421 Sent: Wednesday, September 25, 2013 7:22 AM To: Xie Xiaobo-R63061 Cc: linuxppc-dev@lists.ozlabs.org; Johnston Michael-R49610 Subject: Re: [PATCH V4 3/3] powerpc/85xx: Add TWR-P1025 board support On Tue, 2013-09-24 at 18:48 +0800, Xie Xiaobo wrote: + partition@8 { + /* 3.5 MB for Linux Kernel Image */ + reg = 0x0008 0x0038; + label = NOR Linux Kernel Image; + }; Is this enough? I will enlarge it to 6MB. + partition@40 { + /* 58.75MB for JFFS2 based Root file System */ + reg = 0x0040 0x03ac; + label = NOR Root File System; + }; Don't specify jffs2. OK, I will remove jffs2 + /* CS2 for Display */ + ssd1289@2,0 { + #address-cells = 1; + #size-cells = 1; + compatible = ssd1289; + reg = 0x2 0x 0x0002 + 0x2 0x0002 0x0002; + }; Node names should be generic. What does ssd1289 do? If this is actually the display device, then it should be called display@2,0. OK. The ssd1289 is a LCD controller. How about a vendor prefix on that compatible? Why #address-cells/#size- cells despite no child nodes? Where is a binding that says what each of those two reg resources mean? I will add the vendor prefix. I review the ssd1289 driver, and the #address-cells/#size-cells were un-used. I will remove them. diff --git a/arch/powerpc/boot/dts/p1025twr_32b.dts b/arch/powerpc/boot/dts/p1025twr_32b.dts new file mode 100644 index 000..ccb173f --- /dev/null +++ b/arch/powerpc/boot/dts/p1025twr_32b.dts @@ -0,0 +1,135 @@ +/* + * P1025 TWR Device Tree Source (32-bit address map) + * + * Copyright 2013 Freescale Semiconductor Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * * Neither the name of Freescale Semiconductor nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * + * ALTERNATIVELY, this software may be distributed under the terms of +the + * GNU General Public License (GPL) as published by the Free +Software + * Foundation, either version 2 of that License or (at your option) +any + * later version. + * + * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND +ANY + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +ARE + * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE +FOR ANY + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE +USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +/include/ fsl/p1021si-pre.dtsi +/ { + model = fsl,P1025; + compatible = fsl,TWR-P1025; + + memory { + device_type = memory; + }; + + lbc: localbus@ffe05000 { + reg = 0 0xffe05000 0 0x1000; + + /* NOR Flash and SSD1289 */ + ranges = 0x0 0x0 0x0 0xec00 0x0400 + 0x2 0x0 0x0 0xe000 0x0002; + }; + + soc: soc@ffe0 { + ranges = 0x0 0x0 0xffe0 0x10; + }; + + pci0: pcie@ffe09000 { + ranges = 0x200 0x0 0xa000 0 0xa000 0x0 0x2000 + 0x100 0x0 0x 0 0xffc1 0x0 0x1; + reg = 0 0xffe09000 0 0x1000; + pcie@0 { + ranges = 0x200 0x0 0xa000 + 0x200 0x0 0xa000 + 0x0 0x2000 + + 0x100 0x0 0x0 + 0x100 0x0 0x0 + 0x0 0x10; + }; + }; + + pci1: pcie@ffe0a000 { + reg = 0 0xffe0a000 0 0x1000
RE: [PATCH V4 1/3] powerpc/85xx: Add QE common init functions
-Original Message- From: Wood Scott-B07421 Sent: Wednesday, September 25, 2013 7:13 AM To: Xie Xiaobo-R63061 Cc: linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH V4 1/3] powerpc/85xx: Add QE common init functions On Tue, 2013-09-24 at 18:48 +0800, Xie Xiaobo wrote: Define two QE init functions in common file, and avoid the same codes being duplicated in board files. Signed-off-by: Xie Xiaobo x@freescale.com --- V4 - V3: Nochange arch/powerpc/platforms/85xx/common.c | 51 +++ arch/powerpc/platforms/85xx/mpc85xx.h | 8 ++ 2 files changed, 59 insertions(+) diff --git a/arch/powerpc/platforms/85xx/common.c b/arch/powerpc/platforms/85xx/common.c index d0861a0..08fff48 100644 --- a/arch/powerpc/platforms/85xx/common.c +++ b/arch/powerpc/platforms/85xx/common.c @@ -7,6 +7,9 @@ */ #include linux/of_platform.h +#include asm/machdep.h +#include asm/qe.h +#include asm/qe_ic.h #include sysdev/cpm2_pic.h #include mpc85xx.h @@ -80,3 +83,51 @@ void __init mpc85xx_cpm2_pic_init(void) irq_set_chained_handler(irq, cpm2_cascade); } #endif + +#ifdef CONFIG_QUICC_ENGINE +void __init mpc85xx_qe_pic_init(void) { + struct device_node *np; + + np = of_find_compatible_node(NULL, NULL, fsl,qe-ic); + if (np) { + if (machine_is(mpc8568_mds) || machine_is(mpc8569_mds)) + qe_ic_init(np, 0, qe_ic_cascade_muxed_mpic, NULL); + else + qe_ic_init(np, 0, qe_ic_cascade_low_mpic, + qe_ic_cascade_high_mpic); + of_node_put(np); + } else + pr_err(%s: Could not find qe-ic node\n, __func__); } Have the caller pass in a flag indicating the type of cascade. Or, perhaps this function isn't worth factoring out. Where is the check for p1021_mds? Where did 8568/9 MDS come from? I don't see those checks removed in patch 2. [Xie] The qe_pic_init just call one function qe_ic_init(), So I just need factor out the qe_init function, Is it feasible? BTW, when you move code from one place to another, do it in one patch. Don't add it in one patch and then remove it in another. A more useful split would have been one patch handling qe_init and another handling qe_pic_init. [Xie] I will place these change into a patch. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH V3 1/2] powerpc/85xx: Add QE common init functions
Hi, Thank you very much. I will submit updated patch soon. Best Regards Xie Xiaobo -Original Message- From: Wood Scott-B07421 Sent: Friday, September 06, 2013 11:25 PM To: Xie Xiaobo-R63061 Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org; ga...@kernel.crashing.org Subject: Re: [PATCH V3 1/2] powerpc/85xx: Add QE common init functions On Fri, 2013-09-06 at 04:52 -0500, Xie Xiaobo-R63061 wrote: Hi Scott, I already remove these code from the P1025TWR platform file(see the 2/2 patch). Do you means I also need to remove these codes from the others platforms and use the common call instead? Thank you. Yes. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH V3 2/2] powerpc/85xx: Add TWR-P1025 board support
Hi Scott, I discuss it with Liu Shengzhou again. I tried interrupts = 1 1 0 0 instead of interrupts = 1 1 on P1025TWR, and it's OK. So I will add 2 cells for the interrupts property. Thank you. Best Regards Xie Xiaobo -Original Message- From: Wood Scott-B07421 Sent: Friday, September 06, 2013 11:26 PM To: Xie Xiaobo-R63061 Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org; ga...@kernel.crashing.org; Johnston Michael-R49610 Subject: Re: [PATCH V3 2/2] powerpc/85xx: Add TWR-P1025 board support On Fri, 2013-09-06 at 05:01 -0500, Xie Xiaobo-R63061 wrote: Hi Scott, Thanks for your reminding and advice. I discuss this with Liu Shengzhou(the first person that remind me #interrupt-cells is 4), he advised removing the interrupts property from the phy node, because the mdio used the poll way preferentially. I don't follow... if the PHYs have interrupts, why would we prefer to poll? In any case, the device tree describes the hardware, not how you'd prefer to use it. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH V3 1/2] powerpc/85xx: Add QE common init functions
Hi Scott, I already remove these code from the P1025TWR platform file(see the 2/2 patch). Do you means I also need to remove these codes from the others platforms and use the common call instead? Thank you. Best Regards Xie Xiaobo -Original Message- From: Wood Scott-B07421 Sent: Thursday, September 05, 2013 12:27 AM To: Xie Xiaobo-R63061 Cc: linuxppc-dev@lists.ozlabs.org; ga...@kernel.crashing.org Subject: Re: [PATCH V3 1/2] powerpc/85xx: Add QE common init functions On Mon, 2013-09-02 at 18:11 +0800, Xie Xiaobo wrote: Define two QE init functions in common file, and avoid the same codes being duplicated in board files. Signed-off-by: Xie Xiaobo x@freescale.com --- V3 - V2: Nochange arch/powerpc/platforms/85xx/common.c | 47 +++ arch/powerpc/platforms/85xx/mpc85xx.h | 8 ++ 2 files changed, 55 insertions(+) Don't just copy it; remove it from the place you copied from and have that code call the common version. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH V3 2/2] powerpc/85xx: Add TWR-P1025 board support
Hi Scott, Thanks for your reminding and advice. I discuss this with Liu Shengzhou(the first person that remind me #interrupt-cells is 4), he advised removing the interrupts property from the phy node, because the mdio used the poll way preferentially. Best Regards Xie Xiaobo -Original Message- From: Wood Scott-B07421 Sent: Thursday, September 05, 2013 12:26 AM To: Xie Xiaobo-R63061 Cc: linuxppc-dev@lists.ozlabs.org; ga...@kernel.crashing.org; Johnston Michael-R49610 Subject: Re: [PATCH V3 2/2] powerpc/85xx: Add TWR-P1025 board support On Mon, 2013-09-02 at 18:11 +0800, Xie Xiaobo wrote: +soc { + usb@22000 { + phy_type = ulpi; + }; + + mdio@24000 { + phy0: ethernet-phy@2 { + interrupt-parent = mpic; + interrupts = 1 1; + reg = 0x2; + }; + + phy1: ethernet-phy@1 { + interrupt-parent = mpic; + interrupts = 2 1; + reg = 0x1; + }; Again, #interrupt-cells is 4. Please respond to feedback rather than ignoring it and reposting the same thing without comment. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] mmc: sdhci-pltfm: Added sdhci-adjust-timeout quirk
Hi Changming, OK, you can merge my patch into your patches. Hi all, Please ignore this patch. Changming will send the similar patch. BRs Xie Xiaobo -Original Message- From: Huang Changming-R66093 Sent: 2011年12月13日 16:00 To: Xie Xiaobo-R63061; linuxppc-dev@lists.ozlabs.org Cc: avoront...@ru.mvista.com; linux-...@vger.kernel.org; Xie Xiaobo-R63061 Subject: RE: [PATCH] mmc: sdhci-pltfm: Added sdhci-adjust-timeout quirk Xiaobo, I have one other similar patch, but the property is 'sdhci,adjust-timeout'. Maybe I can repost it with add your signed-off-by? -Original Message- From: linuxppc-dev-bounces+r66093=freescale@lists.ozlabs.org [mailto:linuxppc-dev-bounces+r66093=freescale@lists.ozlabs.org] On Behalf Of Xie Xiaobo Sent: Monday, December 05, 2011 4:55 PM To: linuxppc-dev@lists.ozlabs.org Cc: avoront...@ru.mvista.com; linux-...@vger.kernel.org; Xie Xiaobo- R63061 Subject: [PATCH] mmc: sdhci-pltfm: Added sdhci-adjust-timeout quirk Some controller provides an incorrect timeout value for transfers, So it need the quirk to adjust timeout value to 0xE. E.g. eSDHC of MPC8536, P1010, and P2020. Signed-off-by: Xie Xiaobo x@freescale.com --- drivers/mmc/host/sdhci-pltfm.c |5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci- pltfm.c index a9e12ea..b5d6b3f 100644 --- a/drivers/mmc/host/sdhci-pltfm.c +++ b/drivers/mmc/host/sdhci-pltfm.c @@ -2,7 +2,7 @@ * sdhci-pltfm.c Support for SDHCI platform devices * Copyright (c) 2009 Intel Corporation * - * Copyright (c) 2007 Freescale Semiconductor, Inc. + * Copyright (c) 2007, 2011 Freescale Semiconductor, Inc. * Copyright (c) 2009 MontaVista Software, Inc. * * Authors: Xiaobo Xie x@freescale.com @@ -68,6 +68,9 @@ void sdhci_get_of_property(struct platform_device *pdev) if (of_get_property(np, sdhci,1-bit-only, NULL)) host-quirks |= SDHCI_QUIRK_FORCE_1_BIT_DATA; + if (of_get_property(np, sdhci,sdhci-adjust-timeout, NULL)) + host-quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL; + if (sdhci_of_wp_inverted(np)) host-quirks |= SDHCI_QUIRK_INVERTED_WRITE_PROTECT; -- 1.6.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev