RE: [PATCH V4 3/3] powerpc/85xx: Add TWR-P1025 board support
Hi Scott,
-Original Message-
From: Wood Scott-B07421
Sent: Friday, September 27, 2013 5:27 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
+ /* 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,
The device tree describes the hardware, not the current state of Linux
drivers. Especially drivers that aren't yet in Linux. :-)
OK, I will remain the display node.
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.
Ideally all devices (and bindings) should be described when the device
tree is initally added, regardless of whether you have a driver yet.
I will add a binding document for the ssd1289 device.
-Scott
- Xiaobo
___
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
On Wed, 2013-09-25 at 04:50 -0500, Xie Xiaobo-R63061 wrote: +#if defined(CONFIG_SERIAL_QE) + /* On P1025TWR board, the UCC7 acted as UART port. + * However, The UCC7's CTS pin is low level in default, + * it will impact the transmission in full duplex + * communication. So disable the Flow control pin PA18. + * The UCC7 UART just can use RXD and TXD pins. + */ + par_io_config_pin(0, 18, 0, 0, 0, 0); #endif Any reason not to do this unconditionally? This is a board issue, the code already have a condition - defined SERIAL_QE, and I will add a condition if (machine_is(twr_p1025)). My point was, is there any harm in doing this without regard to CONFIG_SERIAL_QE? -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
-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
On Thu, 2013-09-26 at 04:27 -0500, Xie Xiaobo-R63061 wrote:
-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,
The device tree describes the hardware, not the current state of Linux
drivers. Especially drivers that aren't yet in Linux. :-)
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.
Ideally all devices (and bindings) should be described when the device
tree is initally added, regardless of whether you have a driver yet.
-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 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?
-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
