RE: [PATCH V4 3/3] powerpc/85xx: Add TWR-P1025 board support

2013-09-26 Thread Xie Xiaobo-R63061
 -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

2013-09-25 Thread Xie Xiaobo-R63061
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

2013-09-25 Thread Xie Xiaobo-R63061
 -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

2013-09-10 Thread Xie Xiaobo-R63061
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

2013-09-10 Thread Xie Xiaobo-R63061
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

2013-09-06 Thread Xie Xiaobo-R63061
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

2013-09-06 Thread Xie Xiaobo-R63061
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

2011-12-13 Thread Xie Xiaobo-R63061
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