Re: [PATCH v3 3/3] ARM: OMAP2+: Add GPMC DT support for Ethernet child nodes
Hi Javier!
On Thursday 14 March 2013 at 22:54:11, Javier Martinez Canillas wrote:
Besides being used to interface with external memory devices,
the General-Purpose Memory Controller can be used to connect
Pseudo-SRAM devices such as ethernet controllers to OMAP2+
processors using the TI GPMC as a data bus.
This patch allows an ethernet chip to be defined as an GPMC
child device node.
Signed-off-by: Javier Martinez Canillas
javier.marti...@collabora.co.uk
---
Changes since v2:
- remove optional #address-cells and #size-cells since are not
relevant for ethernet chips; suggested by Jon Hunter.
Changes since v1:
- Improve the DT binding documentation explaining that even when the
GPMC maximum bus address width is 16-bit, it supports devices with
32-bit registers address width and the device property especifying this
has to be set accordingly; suggested by Jon Hunter.
Documentation/devicetree/bindings/net/gpmc-eth.txt | 97
arch/arm/mach-omap2/gpmc.c
|8 ++
2 files changed, 105 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/gpmc-eth.txt
diff --git a/Documentation/devicetree/bindings/net/gpmc-eth.txt
b/Documentation/devicetree/bindings/net/gpmc-eth.txt new file mode
100644
index 000..24cb4e4
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/gpmc-eth.txt
@@ -0,0 +1,97 @@
+Device tree bindings for Ethernet chip connected to TI GPMC
+
+Besides being used to interface with external memory devices, the
+General-Purpose Memory Controller can be used to connect Pseudo-SRAM
devices +such as ethernet controllers to processors using the TI GPMC
as a data bus. +
+Ethernet controllers connected to TI GPMC are represented as child
nodes of +the GPMC controller with an ethernet name.
+
+All timing relevant properties as well as generic GPMC child properties
are +explained in a separate documents. Please refer to
+Documentation/devicetree/bindings/bus/ti-gpmc.txt
+
+For the properties relevant to the ethernet controller connected to the
GPMC +refer to the binding documentation of the device. For example,
the documentation +for the SMSC 911x is
Documentation/devicetree/bindings/net/smsc911x.txt +
+Child nodes need to specify the GPMC bus address width using the
bank-width +property but is possible that an ethernet controller also
has a property to +specify the I/O registers address width. Even when
the GPMC has a maximum 16-bit +address width, it supports devices with
32-bit word registers. +For example with an SMSC LAN911x/912x
controller connected to the TI GPMC on an +OMAP2+ board, bank-width =
2; and reg-io-width = 4;.
+
+Required properties:
+- bank-width:Address width of the device in bytes. GPMC
supports
8-bit + and 16-bit devices and so must be either 1 or 2
bytes.
+- compatible:Compatible string property for the ethernet
child
device. +- gpmc,cs-on:Chip-select assertion time
+- gpmc,cs-rd-off:Chip-select de-assertion time for reads
+- gpmc,cs-wr-off:Chip-select de-assertion time for writes
+- gpmc,oe-on:Output-enable assertion time
+- gpmc,oe-offOutput-enable de-assertion time
+- gpmc,we-on:Write-enable assertion time
+- gpmc,we-off: Write-enable de-assertion time
+- gpmc,access: Start cycle to first data capture (read access)
+- gpmc,rd-cycle: Total read cycle time
+- gpmc,wr-cycle: Total write cycle time
+- reg: Chip-select, base address (relative to
chip-select)
+ and size of the memory mapped for the device.
+ Note that base address will be typically 0 as this
+ is the start of the chip-select.
+
+Optional properties:
+- gpmc,XXX Additional GPMC timings and settings parameters. See
+ Documentation/devicetree/bindings/bus/ti-gpmc.txt
+
+Example:
+
+gpmc: gpmc@6e00 {
+ compatible = ti,omap3430-gpmc;
+ ti,hwmods = gpmc;
+ reg = 0x6e00 0x1000;
+ interrupts = 20;
+ gpmc,num-cs = 8;
+ gpmc,num-waitpins = 4;
+ #address-cells = 2;
+ #size-cells = 1;
+
+ ranges = 5 0 0x2c00 0x100;
+
+ ethernet@5,0 {
+ compatible = smsc,lan9221, smsc,lan9115;
+ reg = 5 0 0xff;
+ bank-width = 2;
+
+ gpmc,mux-add-data;
+ gpmc,cs-on = 0;
+ gpmc,cs-rd-off = 186;
+ gpmc,cs-wr-off = 186;
+ gpmc,adv-on = 12;
+ gpmc,adv-rd-off = 48;
+ gpmc,adv-wr-off = 48;
+ gpmc,oe-on = 54;
+ gpmc,oe-off = 168;
+ gpmc,we-on = 54;
+ gpmc,we-off = 168;
+ gpmc,rd-cycle = 186;
+ gpmc,wr-cycle = 186;
+
Re: [PATCH v3 3/3] ARM: OMAP2+: Add GPMC DT support for Ethernet child nodes
On 04/17/2013 11:40 AM, Lars Poeschel wrote:
Hi Javier!
On Thursday 14 March 2013 at 22:54:11, Javier Martinez Canillas wrote:
Besides being used to interface with external memory devices,
the General-Purpose Memory Controller can be used to connect
Pseudo-SRAM devices such as ethernet controllers to OMAP2+
processors using the TI GPMC as a data bus.
This patch allows an ethernet chip to be defined as an GPMC
child device node.
Signed-off-by: Javier Martinez Canillas
javier.marti...@collabora.co.uk
---
Changes since v2:
- remove optional #address-cells and #size-cells since are not
relevant for ethernet chips; suggested by Jon Hunter.
Changes since v1:
- Improve the DT binding documentation explaining that even when the
GPMC maximum bus address width is 16-bit, it supports devices with
32-bit registers address width and the device property especifying this
has to be set accordingly; suggested by Jon Hunter.
Documentation/devicetree/bindings/net/gpmc-eth.txt | 97
arch/arm/mach-omap2/gpmc.c
|8 ++
2 files changed, 105 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/gpmc-eth.txt
diff --git a/Documentation/devicetree/bindings/net/gpmc-eth.txt
b/Documentation/devicetree/bindings/net/gpmc-eth.txt new file mode
100644
index 000..24cb4e4
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/gpmc-eth.txt
@@ -0,0 +1,97 @@
+Device tree bindings for Ethernet chip connected to TI GPMC
+
+Besides being used to interface with external memory devices, the
+General-Purpose Memory Controller can be used to connect Pseudo-SRAM
devices +such as ethernet controllers to processors using the TI GPMC
as a data bus. +
+Ethernet controllers connected to TI GPMC are represented as child
nodes of +the GPMC controller with an ethernet name.
+
+All timing relevant properties as well as generic GPMC child properties
are +explained in a separate documents. Please refer to
+Documentation/devicetree/bindings/bus/ti-gpmc.txt
+
+For the properties relevant to the ethernet controller connected to the
GPMC +refer to the binding documentation of the device. For example,
the documentation +for the SMSC 911x is
Documentation/devicetree/bindings/net/smsc911x.txt +
+Child nodes need to specify the GPMC bus address width using the
bank-width +property but is possible that an ethernet controller also
has a property to +specify the I/O registers address width. Even when
the GPMC has a maximum 16-bit +address width, it supports devices with
32-bit word registers. +For example with an SMSC LAN911x/912x
controller connected to the TI GPMC on an +OMAP2+ board, bank-width =
2; and reg-io-width = 4;.
+
+Required properties:
+- bank-width: Address width of the device in bytes. GPMC
supports
8-bit + and 16-bit devices and so must be either 1 or 2
bytes.
+- compatible: Compatible string property for the ethernet
child
device. +- gpmc,cs-on: Chip-select assertion time
+- gpmc,cs-rd-off: Chip-select de-assertion time for reads
+- gpmc,cs-wr-off: Chip-select de-assertion time for writes
+- gpmc,oe-on: Output-enable assertion time
+- gpmc,oe-off Output-enable de-assertion time
+- gpmc,we-on: Write-enable assertion time
+- gpmc,we-off: Write-enable de-assertion time
+- gpmc,access: Start cycle to first data capture (read access)
+- gpmc,rd-cycle:Total read cycle time
+- gpmc,wr-cycle:Total write cycle time
+- reg: Chip-select, base address (relative to
chip-select)
+and size of the memory mapped for the device.
+Note that base address will be typically 0 as this
+is the start of the chip-select.
+
+Optional properties:
+- gpmc,XXX Additional GPMC timings and settings parameters. See
+Documentation/devicetree/bindings/bus/ti-gpmc.txt
+
+Example:
+
+gpmc: gpmc@6e00 {
+compatible = ti,omap3430-gpmc;
+ti,hwmods = gpmc;
+reg = 0x6e00 0x1000;
+interrupts = 20;
+gpmc,num-cs = 8;
+gpmc,num-waitpins = 4;
+#address-cells = 2;
+#size-cells = 1;
+
+ranges = 5 0 0x2c00 0x100;
+
+ethernet@5,0 {
+compatible = smsc,lan9221, smsc,lan9115;
+reg = 5 0 0xff;
+bank-width = 2;
+
+gpmc,mux-add-data;
+gpmc,cs-on = 0;
+gpmc,cs-rd-off = 186;
+gpmc,cs-wr-off = 186;
+gpmc,adv-on = 12;
+gpmc,adv-rd-off = 48;
+gpmc,adv-wr-off = 48;
+gpmc,oe-on = 54;
+gpmc,oe-off = 168;
+gpmc,we-on = 54;
+gpmc,we-off = 168;
+gpmc,rd-cycle = 186;
+gpmc,wr-cycle = 186;
+
Re: [PATCH v3 3/3] ARM: OMAP2+: Add GPMC DT support for Ethernet child nodes
On 04/17/2013 07:05 AM, Javier Martinez Canillas wrote:
...
Yes, in fact I just realized that for_each_node_by_name() expand to:
#define for_each_node_by_name(dn, name) \
for (dn = of_find_node_by_name(NULL, name); dn; \
dn = of_find_node_by_name(dn, name))
which means that it will search for a node with name on the complete
DeviceTree and this is wrong. It should only search on GPMC childs.
Good catch. I guess we could have flash ethernet devices connected to
other interfaces such as SPI.
Could you please test the following patch? If it works for you I'll add a
proper
description and submit it as a PATCH.
Thanks a lot and best regards,
Javier
From d8dab9ae9a0284f17553875c2fddd806d9f6ab2e Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas javier.marti...@collabora.co.uk
Date: Wed, 17 Apr 2013 13:50:30 +0200
Subject: [PATCH] ARM: OMAP2+: only search for GPMC DT child nodes on
probe
Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
---
arch/arm/mach-omap2/gpmc.c | 51 ---
1 files changed, 24 insertions(+), 27 deletions(-)
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index ed946df..58e2415 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -1520,35 +1520,32 @@ static int gpmc_probe_dt(struct platform_device *pdev)
return ret;
}
- for_each_node_by_name(child, nand) {
- ret = gpmc_probe_nand_child(pdev, child);
- if (ret 0) {
- of_node_put(child);
- return ret;
- }
- }
-
- for_each_node_by_name(child, onenand) {
- ret = gpmc_probe_onenand_child(pdev, child);
- if (ret 0) {
- of_node_put(child);
- return ret;
- }
- }
+ for_each_child_of_node(pdev-dev.of_node, child) {
+ if (child-name) {
Minor nit ... how about ...
+ if (!child-name)
continue;
+ if (of_node_cmp(child-name, nand) == 0) {
+ ret = gpmc_probe_nand_child(pdev, child);
+ if (ret 0) {
+ of_node_put(child);
+ return ret;
+ }
+ }
- for_each_node_by_name(child, nor) {
- ret = gpmc_probe_generic_child(pdev, child);
- if (ret 0) {
- of_node_put(child);
- return ret;
- }
- }
+ if (of_node_cmp(child-name, onenand) == 0) {
+ ret = gpmc_probe_onenand_child(pdev, child);
+ if (ret 0) {
+ of_node_put(child);
+ return ret;
+ }
+ }
- for_each_node_by_name(child, ethernet) {
- ret = gpmc_probe_generic_child(pdev, child);
- if (ret 0) {
- of_node_put(child);
- return ret;
+ if (of_node_cmp(child-name, ethernet) == 0 ||
+ of_node_cmp(child-name, nor) == 0) {
+ ret = gpmc_probe_generic_child(pdev, child);
+ if (ret 0) {
+ of_node_put(child);
+ return ret;
+ }
+ }
}
}
I am also wondering if the probe should fail if one of its children
fails. The panic that Lars reported occurred because the nand
successfully probed and so the nand device was registered, but because
the ethernet device probe failed, the gpmc probe fails, and then when
the nand device is probed by the mtd driver the kernel panics. I wonder
if it would be better to WARN on child devices that fail to probe but
not return error from the gpmc probe.
Cheers
Jon
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/3] ARM: OMAP2+: Add GPMC DT support for Ethernet child nodes
On 04/17/2013 03:48 PM, Jon Hunter wrote:
On 04/17/2013 07:05 AM, Javier Martinez Canillas wrote:
...
Yes, in fact I just realized that for_each_node_by_name() expand to:
#define for_each_node_by_name(dn, name) \
for (dn = of_find_node_by_name(NULL, name); dn; \
dn = of_find_node_by_name(dn, name))
which means that it will search for a node with name on the complete
DeviceTree and this is wrong. It should only search on GPMC childs.
Good catch. I guess we could have flash ethernet devices connected to
other interfaces such as SPI.
Yes, in fact this is the case for omap4-sdp.dts which has an ethernet controller
connected through SPI:
mcspi1 {
eth@0 {
compatible = ks8851;
spi-max-frequency = 2400;
reg = 0;
interrupt-parent = gpio2;
interrupts = 2; /* gpio line 34 */
vdd-supply = vdd_eth;
};
};
it just didn't fail because the device node name is eth and not ethernet.
which makes me wonder if is OK to rely on a device node name or we should use
compatible properties instead such as ti,gpmc-{eth,nand,nor,onenand} and do
something like:
for_each_compatible_node(pdev-dev.of_node, NULL, ti,gpmc-eth) and so on.
Could you please test the following patch? If it works for you I'll add a
proper
description and submit it as a PATCH.
Thanks a lot and best regards,
Javier
From d8dab9ae9a0284f17553875c2fddd806d9f6ab2e Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas javier.marti...@collabora.co.uk
Date: Wed, 17 Apr 2013 13:50:30 +0200
Subject: [PATCH] ARM: OMAP2+: only search for GPMC DT child nodes on
probe
Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
---
arch/arm/mach-omap2/gpmc.c | 51
---
1 files changed, 24 insertions(+), 27 deletions(-)
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index ed946df..58e2415 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -1520,35 +1520,32 @@ static int gpmc_probe_dt(struct platform_device
*pdev)
return ret;
}
-for_each_node_by_name(child, nand) {
-ret = gpmc_probe_nand_child(pdev, child);
-if (ret 0) {
-of_node_put(child);
-return ret;
-}
-}
-
-for_each_node_by_name(child, onenand) {
-ret = gpmc_probe_onenand_child(pdev, child);
-if (ret 0) {
-of_node_put(child);
-return ret;
-}
-}
+for_each_child_of_node(pdev-dev.of_node, child) {
+if (child-name) {
Minor nit ... how about ...
+ if (!child-name)
continue;
Yes, much better. I just cooked a quick patch so Lars could test it ;-)
+if (of_node_cmp(child-name, nand) == 0) {
+ret = gpmc_probe_nand_child(pdev, child);
+if (ret 0) {
+of_node_put(child);
+return ret;
+}
+}
-for_each_node_by_name(child, nor) {
-ret = gpmc_probe_generic_child(pdev, child);
-if (ret 0) {
-of_node_put(child);
-return ret;
-}
-}
+if (of_node_cmp(child-name, onenand) == 0) {
+ret = gpmc_probe_onenand_child(pdev, child);
+if (ret 0) {
+of_node_put(child);
+return ret;
+}
+}
-for_each_node_by_name(child, ethernet) {
-ret = gpmc_probe_generic_child(pdev, child);
-if (ret 0) {
-of_node_put(child);
-return ret;
+if (of_node_cmp(child-name, ethernet) == 0 ||
+of_node_cmp(child-name, nor) == 0) {
+ret = gpmc_probe_generic_child(pdev, child);
+if (ret 0) {
+of_node_put(child);
+return ret;
+}
+}
}
}
I am also wondering if the probe should fail if one of its children
fails. The panic that Lars reported occurred because the nand
successfully probed and so the nand device was registered, but because
the ethernet device probe failed, the gpmc probe fails, and then when
the nand device is probed by the mtd driver the kernel panics. I wonder
if it would be better to WARN on child devices that fail to probe but
not return error from the gpmc probe.
I wonder
Re: [PATCH v3 3/3] ARM: OMAP2+: Add GPMC DT support for Ethernet child nodes
On Wednesday 17 April 2013 at 14:05:58, Javier Martinez Canillas wrote:
...
Yes, in fact I just realized that for_each_node_by_name() expand to:
#define for_each_node_by_name(dn, name) \
for (dn = of_find_node_by_name(NULL, name); dn; \
dn = of_find_node_by_name(dn, name))
which means that it will search for a node with name on the complete
DeviceTree and this is wrong. It should only search on GPMC childs.
Could you please test the following patch? If it works for you I'll add
a proper description and submit it as a PATCH.
I see discussion is already going on, I just want to report:
I tested the patch and it work as intended. Probing of the gpmc childs
succeeds on am335x with another ethernet node in device tree.
Lars
From d8dab9ae9a0284f17553875c2fddd806d9f6ab2e Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas javier.marti...@collabora.co.uk
Date: Wed, 17 Apr 2013 13:50:30 +0200
Subject: [PATCH] ARM: OMAP2+: only search for GPMC DT child nodes on
probe
Signed-off-by: Javier Martinez Canillas
javier.marti...@collabora.co.uk ---
arch/arm/mach-omap2/gpmc.c | 51
--- 1 files changed, 24
insertions(+), 27 deletions(-)
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index ed946df..58e2415 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -1520,35 +1520,32 @@ static int gpmc_probe_dt(struct platform_device
*pdev) return ret;
}
- for_each_node_by_name(child, nand) {
- ret = gpmc_probe_nand_child(pdev, child);
- if (ret 0) {
- of_node_put(child);
- return ret;
- }
- }
-
- for_each_node_by_name(child, onenand) {
- ret = gpmc_probe_onenand_child(pdev, child);
- if (ret 0) {
- of_node_put(child);
- return ret;
- }
- }
+ for_each_child_of_node(pdev-dev.of_node, child) {
+ if (child-name) {
+ if (of_node_cmp(child-name, nand) == 0) {
+ ret = gpmc_probe_nand_child(pdev, child);
+ if (ret 0) {
+ of_node_put(child);
+ return ret;
+ }
+ }
- for_each_node_by_name(child, nor) {
- ret = gpmc_probe_generic_child(pdev, child);
- if (ret 0) {
- of_node_put(child);
- return ret;
- }
- }
+ if (of_node_cmp(child-name, onenand) == 0) {
+ ret = gpmc_probe_onenand_child(pdev, child);
+ if (ret 0) {
+ of_node_put(child);
+ return ret;
+ }
+ }
- for_each_node_by_name(child, ethernet) {
- ret = gpmc_probe_generic_child(pdev, child);
- if (ret 0) {
- of_node_put(child);
- return ret;
+ if (of_node_cmp(child-name, ethernet) == 0 ||
+ of_node_cmp(child-name, nor) == 0) {
+ ret = gpmc_probe_generic_child(pdev, child);
+ if (ret 0) {
+ of_node_put(child);
+ return ret;
+ }
+ }
}
}
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/3] ARM: OMAP2+: Add GPMC DT support for Ethernet child nodes
On 04/17/2013 09:07 AM, Javier Martinez Canillas wrote:
On 04/17/2013 03:48 PM, Jon Hunter wrote:
On 04/17/2013 07:05 AM, Javier Martinez Canillas wrote:
...
Yes, in fact I just realized that for_each_node_by_name() expand to:
#define for_each_node_by_name(dn, name) \
for (dn = of_find_node_by_name(NULL, name); dn; \
dn = of_find_node_by_name(dn, name))
which means that it will search for a node with name on the complete
DeviceTree and this is wrong. It should only search on GPMC childs.
Good catch. I guess we could have flash ethernet devices connected to
other interfaces such as SPI.
Yes, in fact this is the case for omap4-sdp.dts which has an ethernet
controller
connected through SPI:
mcspi1 {
eth@0 {
compatible = ks8851;
spi-max-frequency = 2400;
reg = 0;
interrupt-parent = gpio2;
interrupts = 2; /* gpio line 34 */
vdd-supply = vdd_eth;
};
};
it just didn't fail because the device node name is eth and not ethernet.
Yes we got lucky here or unlucky as this would have tripped us up
earlier ;-)
which makes me wonder if is OK to rely on a device node name or we should use
compatible properties instead such as ti,gpmc-{eth,nand,nor,onenand} and do
something like:
for_each_compatible_node(pdev-dev.of_node, NULL, ti,gpmc-eth) and so on.
Yes I was wondering about that too. However, it does seem simpler to
just search through the child devices and that would be an easier fix.
Could you please test the following patch? If it works for you I'll add a
proper
description and submit it as a PATCH.
Thanks a lot and best regards,
Javier
From d8dab9ae9a0284f17553875c2fddd806d9f6ab2e Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas javier.marti...@collabora.co.uk
Date: Wed, 17 Apr 2013 13:50:30 +0200
Subject: [PATCH] ARM: OMAP2+: only search for GPMC DT child nodes on
probe
Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
---
arch/arm/mach-omap2/gpmc.c | 51
---
1 files changed, 24 insertions(+), 27 deletions(-)
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index ed946df..58e2415 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -1520,35 +1520,32 @@ static int gpmc_probe_dt(struct platform_device
*pdev)
return ret;
}
- for_each_node_by_name(child, nand) {
- ret = gpmc_probe_nand_child(pdev, child);
- if (ret 0) {
- of_node_put(child);
- return ret;
- }
- }
-
- for_each_node_by_name(child, onenand) {
- ret = gpmc_probe_onenand_child(pdev, child);
- if (ret 0) {
- of_node_put(child);
- return ret;
- }
- }
+ for_each_child_of_node(pdev-dev.of_node, child) {
+ if (child-name) {
Minor nit ... how about ...
+if (!child-name)
continue;
Yes, much better. I just cooked a quick patch so Lars could test it ;-)
+ if (of_node_cmp(child-name, nand) == 0) {
+ ret = gpmc_probe_nand_child(pdev, child);
+ if (ret 0) {
+ of_node_put(child);
+ return ret;
+ }
+ }
- for_each_node_by_name(child, nor) {
- ret = gpmc_probe_generic_child(pdev, child);
- if (ret 0) {
- of_node_put(child);
- return ret;
- }
- }
+ if (of_node_cmp(child-name, onenand) == 0) {
+ ret = gpmc_probe_onenand_child(pdev, child);
+ if (ret 0) {
+ of_node_put(child);
+ return ret;
+ }
+ }
- for_each_node_by_name(child, ethernet) {
- ret = gpmc_probe_generic_child(pdev, child);
- if (ret 0) {
- of_node_put(child);
- return ret;
+ if (of_node_cmp(child-name, ethernet) == 0 ||
+ of_node_cmp(child-name, nor) == 0) {
+ ret = gpmc_probe_generic_child(pdev, child);
+ if (ret 0) {
+ of_node_put(child);
+ return ret;
+ }
+ }
}
}
I am also wondering if the probe should fail if one of its children
fails. The panic that Lars reported occurred because the nand
successfully probed and so the nand device was registered, but because
the ethernet device probe failed,
Re: [PATCH v3 3/3] ARM: OMAP2+: Add GPMC DT support for Ethernet child nodes
On 04/17/2013 08:33 PM, Jon Hunter wrote:
On 04/17/2013 09:07 AM, Javier Martinez Canillas wrote:
On 04/17/2013 03:48 PM, Jon Hunter wrote:
On 04/17/2013 07:05 AM, Javier Martinez Canillas wrote:
...
Yes, in fact I just realized that for_each_node_by_name() expand to:
#define for_each_node_by_name(dn, name) \
for (dn = of_find_node_by_name(NULL, name); dn; \
dn = of_find_node_by_name(dn, name))
which means that it will search for a node with name on the complete
DeviceTree and this is wrong. It should only search on GPMC childs.
Good catch. I guess we could have flash ethernet devices connected to
other interfaces such as SPI.
Yes, in fact this is the case for omap4-sdp.dts which has an ethernet
controller
connected through SPI:
mcspi1 {
eth@0 {
compatible = ks8851;
spi-max-frequency = 2400;
reg = 0;
interrupt-parent = gpio2;
interrupts = 2; /* gpio line 34 */
vdd-supply = vdd_eth;
};
};
it just didn't fail because the device node name is eth and not ethernet.
Yes we got lucky here or unlucky as this would have tripped us up
earlier ;-)
indeed :)
which makes me wonder if is OK to rely on a device node name or we should use
compatible properties instead such as ti,gpmc-{eth,nand,nor,onenand} and do
something like:
for_each_compatible_node(pdev-dev.of_node, NULL, ti,gpmc-eth) and so on.
Yes I was wondering about that too. However, it does seem simpler to
just search through the child devices and that would be an easier fix.
agreed
Could you please test the following patch? If it works for you I'll add a
proper
description and submit it as a PATCH.
Thanks a lot and best regards,
Javier
From d8dab9ae9a0284f17553875c2fddd806d9f6ab2e Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas javier.marti...@collabora.co.uk
Date: Wed, 17 Apr 2013 13:50:30 +0200
Subject: [PATCH] ARM: OMAP2+: only search for GPMC DT child nodes on
probe
Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
---
arch/arm/mach-omap2/gpmc.c | 51
---
1 files changed, 24 insertions(+), 27 deletions(-)
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index ed946df..58e2415 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -1520,35 +1520,32 @@ static int gpmc_probe_dt(struct platform_device
*pdev)
return ret;
}
- for_each_node_by_name(child, nand) {
- ret = gpmc_probe_nand_child(pdev, child);
- if (ret 0) {
- of_node_put(child);
- return ret;
- }
- }
-
- for_each_node_by_name(child, onenand) {
- ret = gpmc_probe_onenand_child(pdev, child);
- if (ret 0) {
- of_node_put(child);
- return ret;
- }
- }
+ for_each_child_of_node(pdev-dev.of_node, child) {
+ if (child-name) {
Minor nit ... how about ...
+ if (!child-name)
continue;
Yes, much better. I just cooked a quick patch so Lars could test it ;-)
+ if (of_node_cmp(child-name, nand) == 0) {
+ ret = gpmc_probe_nand_child(pdev, child);
+ if (ret 0) {
+ of_node_put(child);
+ return ret;
+ }
+ }
- for_each_node_by_name(child, nor) {
- ret = gpmc_probe_generic_child(pdev, child);
- if (ret 0) {
- of_node_put(child);
- return ret;
- }
- }
+ if (of_node_cmp(child-name, onenand) == 0) {
+ ret = gpmc_probe_onenand_child(pdev, child);
+ if (ret 0) {
+ of_node_put(child);
+ return ret;
+ }
+ }
- for_each_node_by_name(child, ethernet) {
- ret = gpmc_probe_generic_child(pdev, child);
- if (ret 0) {
- of_node_put(child);
- return ret;
+ if (of_node_cmp(child-name, ethernet) == 0 ||
+ of_node_cmp(child-name, nor) == 0) {
+ ret = gpmc_probe_generic_child(pdev, child);
+ if (ret 0) {
+ of_node_put(child);
+ return ret;
+ }
+ }
}
}
I am also wondering if the probe should fail if one of its children
fails. The panic that Lars reported occurred because the nand
successfully probed and so the nand device was registered, but because
the ethernet device
Re: [PATCH v3 3/3] ARM: OMAP2+: Add GPMC DT support for Ethernet child nodes
On 03/14/2013 04:54 PM, Javier Martinez Canillas wrote:
Besides being used to interface with external memory devices,
the General-Purpose Memory Controller can be used to connect
Pseudo-SRAM devices such as ethernet controllers to OMAP2+
processors using the TI GPMC as a data bus.
This patch allows an ethernet chip to be defined as an GPMC
child device node.
Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
---
Changes since v2:
- remove optional #address-cells and #size-cells since are not relevant for
ethernet chips; suggested by Jon Hunter.
Changes since v1:
- Improve the DT binding documentation explaining that even when the GPMC
maximum bus address width is 16-bit, it supports devices with 32-bit
registers address width and the device property especifying this has to
be set accordingly; suggested by Jon Hunter.
Documentation/devicetree/bindings/net/gpmc-eth.txt | 97
arch/arm/mach-omap2/gpmc.c |8 ++
2 files changed, 105 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/gpmc-eth.txt
diff --git a/Documentation/devicetree/bindings/net/gpmc-eth.txt
b/Documentation/devicetree/bindings/net/gpmc-eth.txt
new file mode 100644
index 000..24cb4e4
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/gpmc-eth.txt
@@ -0,0 +1,97 @@
+Device tree bindings for Ethernet chip connected to TI GPMC
+
+Besides being used to interface with external memory devices, the
+General-Purpose Memory Controller can be used to connect Pseudo-SRAM devices
+such as ethernet controllers to processors using the TI GPMC as a data bus.
+
+Ethernet controllers connected to TI GPMC are represented as child nodes of
+the GPMC controller with an ethernet name.
+
+All timing relevant properties as well as generic GPMC child properties are
+explained in a separate documents. Please refer to
+Documentation/devicetree/bindings/bus/ti-gpmc.txt
+
+For the properties relevant to the ethernet controller connected to the GPMC
+refer to the binding documentation of the device. For example, the
documentation
+for the SMSC 911x is Documentation/devicetree/bindings/net/smsc911x.txt
+
+Child nodes need to specify the GPMC bus address width using the bank-width
+property but is possible that an ethernet controller also has a property to
+specify the I/O registers address width. Even when the GPMC has a maximum
16-bit
+address width, it supports devices with 32-bit word registers.
+For example with an SMSC LAN911x/912x controller connected to the TI GPMC on
an
+OMAP2+ board, bank-width = 2; and reg-io-width = 4;.
+
+Required properties:
+- bank-width:Address width of the device in bytes. GPMC
supports 8-bit
+ and 16-bit devices and so must be either 1 or 2 bytes.
+- compatible:Compatible string property for the ethernet
child device.
+- gpmc,cs-on:Chip-select assertion time
+- gpmc,cs-rd-off:Chip-select de-assertion time for reads
+- gpmc,cs-wr-off:Chip-select de-assertion time for writes
+- gpmc,oe-on:Output-enable assertion time
+- gpmc,oe-offOutput-enable de-assertion time
+- gpmc,we-on:Write-enable assertion time
+- gpmc,we-off: Write-enable de-assertion time
+- gpmc,access: Start cycle to first data capture (read access)
+- gpmc,rd-cycle: Total read cycle time
+- gpmc,wr-cycle: Total write cycle time
+- reg: Chip-select, base address (relative to
chip-select)
+ and size of the memory mapped for the device.
+ Note that base address will be typically 0 as this
+ is the start of the chip-select.
+
+Optional properties:
+- gpmc,XXX Additional GPMC timings and settings parameters. See
+ Documentation/devicetree/bindings/bus/ti-gpmc.txt
+
+Example:
+
+gpmc: gpmc@6e00 {
+ compatible = ti,omap3430-gpmc;
+ ti,hwmods = gpmc;
+ reg = 0x6e00 0x1000;
+ interrupts = 20;
+ gpmc,num-cs = 8;
+ gpmc,num-waitpins = 4;
+ #address-cells = 2;
+ #size-cells = 1;
+
+ ranges = 5 0 0x2c00 0x100;
+
+ ethernet@5,0 {
+ compatible = smsc,lan9221, smsc,lan9115;
+ reg = 5 0 0xff;
+ bank-width = 2;
+
+ gpmc,mux-add-data;
+ gpmc,cs-on = 0;
+ gpmc,cs-rd-off = 186;
+ gpmc,cs-wr-off = 186;
+ gpmc,adv-on = 12;
+ gpmc,adv-rd-off = 48;
+ gpmc,adv-wr-off = 48;
+ gpmc,oe-on = 54;
+ gpmc,oe-off = 168;
+ gpmc,we-on = 54;
+ gpmc,we-off = 168;
+ gpmc,rd-cycle = 186;
+ gpmc,wr-cycle = 186;
+ gpmc,access =
Re: [PATCH v3 3/3] ARM: OMAP2+: Add GPMC DT support for Ethernet child nodes
Rob, Grant,
On 03/15/2013 10:24 AM, Jon Hunter wrote:
On 03/14/2013 04:54 PM, Javier Martinez Canillas wrote:
Besides being used to interface with external memory devices,
the General-Purpose Memory Controller can be used to connect
Pseudo-SRAM devices such as ethernet controllers to OMAP2+
processors using the TI GPMC as a data bus.
This patch allows an ethernet chip to be defined as an GPMC
child device node.
Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
---
Changes since v2:
- remove optional #address-cells and #size-cells since are not relevant for
ethernet chips; suggested by Jon Hunter.
Changes since v1:
- Improve the DT binding documentation explaining that even when the GPMC
maximum bus address width is 16-bit, it supports devices with 32-bit
registers address width and the device property especifying this has to
be set accordingly; suggested by Jon Hunter.
Documentation/devicetree/bindings/net/gpmc-eth.txt | 97
arch/arm/mach-omap2/gpmc.c |8 ++
2 files changed, 105 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/gpmc-eth.txt
diff --git a/Documentation/devicetree/bindings/net/gpmc-eth.txt
b/Documentation/devicetree/bindings/net/gpmc-eth.txt
new file mode 100644
index 000..24cb4e4
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/gpmc-eth.txt
@@ -0,0 +1,97 @@
+Device tree bindings for Ethernet chip connected to TI GPMC
+
+Besides being used to interface with external memory devices, the
+General-Purpose Memory Controller can be used to connect Pseudo-SRAM devices
+such as ethernet controllers to processors using the TI GPMC as a data bus.
+
+Ethernet controllers connected to TI GPMC are represented as child nodes of
+the GPMC controller with an ethernet name.
+
+All timing relevant properties as well as generic GPMC child properties are
+explained in a separate documents. Please refer to
+Documentation/devicetree/bindings/bus/ti-gpmc.txt
+
+For the properties relevant to the ethernet controller connected to the GPMC
+refer to the binding documentation of the device. For example, the
documentation
+for the SMSC 911x is Documentation/devicetree/bindings/net/smsc911x.txt
+
+Child nodes need to specify the GPMC bus address width using the
bank-width
+property but is possible that an ethernet controller also has a property to
+specify the I/O registers address width. Even when the GPMC has a maximum
16-bit
+address width, it supports devices with 32-bit word registers.
+For example with an SMSC LAN911x/912x controller connected to the TI GPMC
on an
+OMAP2+ board, bank-width = 2; and reg-io-width = 4;.
+
+Required properties:
+- bank-width: Address width of the device in bytes. GPMC
supports 8-bit
+and 16-bit devices and so must be either 1 or 2 bytes.
+- compatible: Compatible string property for the ethernet
child device.
+- gpmc,cs-on: Chip-select assertion time
+- gpmc,cs-rd-off: Chip-select de-assertion time for reads
+- gpmc,cs-wr-off: Chip-select de-assertion time for writes
+- gpmc,oe-on: Output-enable assertion time
+- gpmc,oe-off Output-enable de-assertion time
+- gpmc,we-on: Write-enable assertion time
+- gpmc,we-off: Write-enable de-assertion time
+- gpmc,access: Start cycle to first data capture (read access)
+- gpmc,rd-cycle:Total read cycle time
+- gpmc,wr-cycle:Total write cycle time
+- reg: Chip-select, base address (relative to
chip-select)
+and size of the memory mapped for the device.
+Note that base address will be typically 0 as this
+is the start of the chip-select.
+
+Optional properties:
+- gpmc,XXX Additional GPMC timings and settings parameters. See
+Documentation/devicetree/bindings/bus/ti-gpmc.txt
+
+Example:
+
+gpmc: gpmc@6e00 {
+compatible = ti,omap3430-gpmc;
+ti,hwmods = gpmc;
+reg = 0x6e00 0x1000;
+interrupts = 20;
+gpmc,num-cs = 8;
+gpmc,num-waitpins = 4;
+#address-cells = 2;
+#size-cells = 1;
+
+ranges = 5 0 0x2c00 0x100;
+
+ethernet@5,0 {
+compatible = smsc,lan9221, smsc,lan9115;
+reg = 5 0 0xff;
+bank-width = 2;
+
+gpmc,mux-add-data;
+gpmc,cs-on = 0;
+gpmc,cs-rd-off = 186;
+gpmc,cs-wr-off = 186;
+gpmc,adv-on = 12;
+gpmc,adv-rd-off = 48;
+gpmc,adv-wr-off = 48;
+gpmc,oe-on = 54;
+gpmc,oe-off = 168;
+gpmc,we-on = 54;
+gpmc,we-off = 168;
+gpmc,rd-cycle = 186;
+gpmc,wr-cycle = 186;
+
