[PATCH v2 0/6] ARM: davinci: da850: add ethernet driver DT support

[PATCH v2 0/6] ARM: davinci: da850: add ethernet driver DT support

[Prabhakar Lad]

From: Lad, Prabhakar <prabhakar.lad@ti.com>

This patch set enables Ethernet support through device tree model.
This patch set enables mii interface only and is being tested to boot via
rootfs. The rmii phy is present on the i2c gpio expander chip (UI board)
for which yet support needs to be added, once the DT support for the chip
is enabled, enabling rmii will be subsequnet patch.

Changes for v2:
1: Enabled mdio device.
2: Fixed clock lookup.

Lad, Prabhakar (6):
  ARM: davinci: da850: fix clock lookup for mdio device
  ARM: davinci: da850: add DT node for mdio device
  ARM: davinci: da850: add OF_DEV_AUXDATA entry for mdio.
  ARM: davinci: da850: add DT node for eth0.
  ARM: davinci: da850: add OF_DEV_AUXDATA entry for eth0.
  ARM: davinci: da850: configure system configuration chip(CFGCHIP3)
    for emac

 arch/arm/boot/dts/da850-evm.dts       |    8 ++++++
 arch/arm/boot/dts/da850.dtsi          |   42 +++++++++++++++++++++++++++++++++
 arch/arm/mach-davinci/da850.c         |    1 +
 arch/arm/mach-davinci/da8xx-dt.c      |   38 +++++++++++++++++++++++++++++-
 arch/arm/mach-davinci/devices-da8xx.c |    8 +----
 5 files changed, 90 insertions(+), 7 deletions(-)

-- 
1.7.4.1

[PATCH v2 1/6] ARM: davinci: da850: fix clock lookup for mdio device

[Prabhakar Lad]

From: Lad, Prabhakar <prabhakar.lad@ti.com>

This patch removes the clock alias for mdio device and adds a entry
in clock lookup table, this entry can now be used by both DT and NON
DT case.

Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: davinci-linux-open-source@linux.davincidsp.com
Cc: netdev@vger.kernel.org
Cc: devicetree-discuss@lists.ozlabs.org
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Heiko Schocher <hs@denx.de>
---
 arch/arm/mach-davinci/da850.c         |    1 +
 arch/arm/mach-davinci/devices-da8xx.c |    8 ++------
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 86056ca..f74bfb6 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -402,6 +402,7 @@ static struct clk_lookup da850_clks[] = {
 	CLK(NULL,		"arm",		&arm_clk),
 	CLK(NULL,		"rmii",		&rmii_clk),
 	CLK("davinci_emac.1",	NULL,		&emac_clk),
+	CLK("davinci_mdio.0",	"fck",		&emac_clk),
 	CLK("davinci-mcasp.0",	NULL,		&mcasp_clk),
 	CLK("da8xx_lcdc.0",	"fck",		&lcdc_clk),
 	CLK("davinci_mmc.0",	NULL,		&mmcsd0_clk),
diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index 2d5502d..52faa05 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -444,12 +444,8 @@ int __init da8xx_register_emac(void)
 	ret = platform_device_register(&da8xx_mdio_device);
 	if (ret < 0)
 		return ret;
-	ret = platform_device_register(&da8xx_emac_device);
-	if (ret < 0)
-		return ret;
-	ret = clk_add_alias(NULL, dev_name(&da8xx_mdio_device.dev),
-			    NULL, &da8xx_emac_device.dev);
-	return ret;
+
+	return platform_device_register(&da8xx_emac_device);
 }
 
 static struct resource da830_mcasp1_resources[] = {
-- 
1.7.4.1

[PATCH v2 2/6] ARM: davinci: da850: add DT node for mdio device

[Prabhakar Lad]

From: Lad, Prabhakar <prabhakar.lad@ti.com>

Add mdio device tree node information to da850 by
providing register details and bus frequency of mdio.

Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: davinci-linux-open-source@linux.davincidsp.com
Cc: netdev@vger.kernel.org
Cc: devicetree-discuss@lists.ozlabs.org
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Heiko Schocher <hs@denx.de>
---
 arch/arm/boot/dts/da850-evm.dts |    3 +++
 arch/arm/boot/dts/da850.dtsi    |    7 +++++++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
index 98c1a48..a319491 100644
--- a/arch/arm/boot/dts/da850-evm.dts
+++ b/arch/arm/boot/dts/da850-evm.dts
@@ -27,6 +27,9 @@
 		serial2: serial@1d0d000 {
 			status = "okay";
 		};
+		mdio: davinci_mdio@1e24000 {
+			status = "okay";
+		};
 	};
 	nand_cs3@62000000 {
 		status = "okay";
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 7c84822..ba28f2d 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -81,6 +81,13 @@
 			interrupts = <61>;
 			status = "disabled";
 		};
+		mdio: davinci_mdio@1e24000 {
+			compatible = "ti,davinci_mdio";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x224000 0x1000>;
+			bus_freq = <2200000>;
+		};
 	};
 	nand_cs3@62000000 {
 		compatible = "ti,davinci-nand";
-- 
1.7.4.1

[PATCH v2 3/6] ARM: davinci: da850: add OF_DEV_AUXDATA entry for mdio.

[Prabhakar Lad]

From: Lad, Prabhakar <prabhakar.lad@ti.com>

Add OF_DEV_AUXDATA for mdio driver in da850 board dt
file to use mdio clock.

Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: davinci-linux-open-source@linux.davincidsp.com
Cc: netdev@vger.kernel.org
Cc: devicetree-discuss@lists.ozlabs.org
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Heiko Schocher <hs@denx.de>
---
 arch/arm/mach-davinci/da8xx-dt.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index 37c27af..bd00042 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -39,9 +39,15 @@ static void __init da8xx_init_irq(void)
 
 #ifdef CONFIG_ARCH_DAVINCI_DA850
 
+struct of_dev_auxdata da8xx_auxdata[] __initdata = {
+	OF_DEV_AUXDATA("ti,davinci_mdio", 0x01e24000, "davinci_mdio.0", NULL),
+	{},
+};
+
 static void __init da850_init_machine(void)
 {
-	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+	of_platform_populate(NULL, of_default_bus_match_table,
+			     da8xx_auxdata, NULL);
 
 	da8xx_uart_clk_enable();
 }
-- 
1.7.4.1

[PATCH v2 4/6] ARM: davinci: da850: add DT node for eth0.

[Prabhakar Lad]

From: Lad, Prabhakar <prabhakar.lad@ti.com>

Add eth0 device tree node information and pinmux for mii to da850 by
providing interrupt details and local mac address of eth0.

Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: davinci-linux-open-source@linux.davincidsp.com
Cc: netdev@vger.kernel.org
Cc: devicetree-discuss@lists.ozlabs.org
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Heiko Schocher <hs@denx.de>
---
 arch/arm/boot/dts/da850-evm.dts |    5 +++++
 arch/arm/boot/dts/da850.dtsi    |   35 +++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
index a319491..19aa2b3 100644
--- a/arch/arm/boot/dts/da850-evm.dts
+++ b/arch/arm/boot/dts/da850-evm.dts
@@ -30,6 +30,11 @@
 		mdio: davinci_mdio@1e24000 {
 			status = "okay";
 		};
+		eth0: emac@1e20000 {
+			status = "okay";
+			pinctrl-names = "default";
+			pinctrl-0 = <&mii_pins>;
+		};
 	};
 	nand_cs3@62000000 {
 		status = "okay";
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index ba28f2d..76905f3 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -56,6 +56,26 @@
 					0x30 0x01100000  0x0ff00000
 				>;
 			};
+			mii_pins: pinmux_mii_pins {
+				pinctrl-single,bits = <
+					/*
+					 * MII_TXEN, MII_TXCLK, MII_COL
+					 * MII_TXD_3, MII_TXD_2, MII_TXD_1
+					 * MII_TXD_0
+					 */
+					0x8 0x88888880 0xfffffff0
+					/*
+					 * MII_RXER, MII_CRS, MII_RXCLK
+					 * MII_RXDV, MII_RXD_3, MII_RXD_2
+					 * MII_RXD_1, MII_RXD_0
+					 */
+					0xc 0x88888888 0xffffffff
+					/* MDIO_CLK, MDIO_D */
+					0x10 0x00222288 0x00ffffff
+					/* GPIO2_6 */
+					0x18 0x00000080 0x000000f0
+				>;
+			};
 		};
 		serial0: serial@1c42000 {
 			compatible = "ns16550a";
@@ -88,6 +108,21 @@
 			reg = <0x224000 0x1000>;
 			bus_freq = <2200000>;
 		};
+		eth0: emac@1e20000 {
+			compatible = "ti,davinci-dm6467-emac";
+			reg = <0x220000 0x4000>;
+			ti,davinci-ctrl-reg-offset = <0x3000>;
+			ti,davinci-ctrl-mod-reg-offset = <0x2000>;
+			ti,davinci-ctrl-ram-offset = <0>;
+			ti,davinci-ctrl-ram-size = <0x2000>;
+			local-mac-address = [ 00 00 00 00 00 00 ];
+			interrupts = <33
+					34
+					35
+					36
+					>;
+			phy-handle = <&mdio>;
+		};
 	};
 	nand_cs3@62000000 {
 		compatible = "ti,davinci-nand";
-- 
1.7.4.1

[PATCH v2 5/6] ARM: davinci: da850: add OF_DEV_AUXDATA entry for eth0.

[Prabhakar Lad]

From: Lad, Prabhakar <prabhakar.lad@ti.com>

Add OF_DEV_AUXDATA for eth0  driver in da850 board dt
file to use emac clock.

Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: davinci-linux-open-source@linux.davincidsp.com
Cc: netdev@vger.kernel.org
Cc: devicetree-discuss@lists.ozlabs.org
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Heiko Schocher <hs@denx.de>
---
 arch/arm/mach-davinci/da8xx-dt.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index bd00042..e533a0a 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -41,6 +41,8 @@ static void __init da8xx_init_irq(void)
 
 struct of_dev_auxdata da8xx_auxdata[] __initdata = {
 	OF_DEV_AUXDATA("ti,davinci_mdio", 0x01e24000, "davinci_mdio.0", NULL),
+	OF_DEV_AUXDATA("ti,davinci-dm6467-emac", 0x01e20000, "davinci_emac.1",
+		       NULL),
 	{},
 };
 
-- 
1.7.4.1

[PATCH v2 6/6] ARM: davinci: da850: configure system configuration chip(CFGCHIP3) for emac

[Prabhakar Lad]

From: Lad, Prabhakar <prabhakar.lad@ti.com>

The system configuration chip CFGCHIP3, controls the emac module.
This patch appropriately configures this register for emac and
sets DA850_MII_MDIO_CLKEN_PIN GPIO pin appropriately.

Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: davinci-linux-open-source@linux.davincidsp.com
Cc: netdev@vger.kernel.org
Cc: devicetree-discuss@lists.ozlabs.org
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Heiko Schocher <hs@denx.de>
---
 arch/arm/mach-davinci/da8xx-dt.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index e533a0a..4a096e3 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -8,6 +8,7 @@
  * published by the Free Software Foundation.
  */
 #include <linux/io.h>
+#include <linux/gpio.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/irqdomain.h>
@@ -39,6 +40,32 @@ static void __init da8xx_init_irq(void)
 
 #ifdef CONFIG_ARCH_DAVINCI_DA850
 
+static void __init da8xx_config_emac(void)
+{
+#define DA850_MII_MDIO_CLKEN_PIN	GPIO_TO_PIN(2, 6)
+#define DA850_EMAC_MODE_SELECT		BIT(8)
+	void __iomem *cfg_chip3_base;
+	int ret;
+	u32 val;
+
+	cfg_chip3_base = DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP3_REG);
+
+	val = __raw_readl(cfg_chip3_base);
+	val &= ~DA850_EMAC_MODE_SELECT;
+	/* configure the CFGCHIP3 register for MII */
+	__raw_writel(val, cfg_chip3_base);
+	pr_info("EMAC: MII PHY configured\n");
+
+	ret = gpio_request(DA850_MII_MDIO_CLKEN_PIN, "mdio_clk_en");
+	if (ret) {
+		pr_warn("Cannot open GPIO %d\n",
+					DA850_MII_MDIO_CLKEN_PIN);
+		return;
+	}
+	/* Enable/Disable MII MDIO clock */
+	gpio_direction_output(DA850_MII_MDIO_CLKEN_PIN, 0);
+}
+
 struct of_dev_auxdata da8xx_auxdata[] __initdata = {
 	OF_DEV_AUXDATA("ti,davinci_mdio", 0x01e24000, "davinci_mdio.0", NULL),
 	OF_DEV_AUXDATA("ti,davinci-dm6467-emac", 0x01e20000, "davinci_emac.1",
@@ -52,6 +79,7 @@ static void __init da850_init_machine(void)
 			     da8xx_auxdata, NULL);
 
 	da8xx_uart_clk_enable();
+	da8xx_config_emac();
 }
 
 static const char *da850_boards_compat[] __initdata = {
-- 
1.7.4.1

Re: [PATCH v2 1/6] ARM: davinci: da850: fix clock lookup for mdio device

[Sekhar Nori]

On 1/28/2013 7:17 PM, Prabhakar Lad wrote:
> From: Lad, Prabhakar <prabhakar.lad@ti.com>
> 
> This patch removes the clock alias for mdio device and adds a entry
> in clock lookup table, this entry can now be used by both DT and NON
> DT case.
> 
> Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: davinci-linux-open-source@linux.davincidsp.com
> Cc: netdev@vger.kernel.org
> Cc: devicetree-discuss@lists.ozlabs.org
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Heiko Schocher <hs@denx.de>
> ---
>  arch/arm/mach-davinci/da850.c         |    1 +
>  arch/arm/mach-davinci/devices-da8xx.c |    8 ++------
>  2 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
> index 86056ca..f74bfb6 100644
> --- a/arch/arm/mach-davinci/da850.c
> +++ b/arch/arm/mach-davinci/da850.c
> @@ -402,6 +402,7 @@ static struct clk_lookup da850_clks[] = {
>  	CLK(NULL,		"arm",		&arm_clk),
>  	CLK(NULL,		"rmii",		&rmii_clk),
>  	CLK("davinci_emac.1",	NULL,		&emac_clk),
> +	CLK("davinci_mdio.0",	"fck",		&emac_clk),
>  	CLK("davinci-mcasp.0",	NULL,		&mcasp_clk),
>  	CLK("da8xx_lcdc.0",	"fck",		&lcdc_clk),
>  	CLK("davinci_mmc.0",	NULL,		&mmcsd0_clk),
> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> index 2d5502d..52faa05 100644
> --- a/arch/arm/mach-davinci/devices-da8xx.c
> +++ b/arch/arm/mach-davinci/devices-da8xx.c
> @@ -444,12 +444,8 @@ int __init da8xx_register_emac(void)
>  	ret = platform_device_register(&da8xx_mdio_device);
>  	if (ret < 0)
>  		return ret;
> -	ret = platform_device_register(&da8xx_emac_device);
> -	if (ret < 0)
> -		return ret;
> -	ret = clk_add_alias(NULL, dev_name(&da8xx_mdio_device.dev),
> -			    NULL, &da8xx_emac_device.dev);
> -	return ret;
> +
> +	return platform_device_register(&da8xx_emac_device);

You added the look-up only for DA850 which means after this patch da830
network will be broken. Did you test on DA830?

Thanks,
Sekhar

Re: [PATCH v2 1/6] ARM: davinci: da850: fix clock lookup for mdio device

[Prabhakar Lad]

Sekhar,

On Sun, Feb 3, 2013 at 4:44 PM, Sekhar Nori <nsekhar@ti.com> wrote:
> On 1/28/2013 7:17 PM, Prabhakar Lad wrote:
>> From: Lad, Prabhakar <prabhakar.lad@ti.com>
>>
>> This patch removes the clock alias for mdio device and adds a entry
>> in clock lookup table, this entry can now be used by both DT and NON
>> DT case.
>>
>> Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: davinci-linux-open-source@linux.davincidsp.com
>> Cc: netdev@vger.kernel.org
>> Cc: devicetree-discuss@lists.ozlabs.org
>> Cc: Sekhar Nori <nsekhar@ti.com>
>> Cc: Heiko Schocher <hs@denx.de>
>> ---
>>  arch/arm/mach-davinci/da850.c         |    1 +
>>  arch/arm/mach-davinci/devices-da8xx.c |    8 ++------
>>  2 files changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
>> index 86056ca..f74bfb6 100644
>> --- a/arch/arm/mach-davinci/da850.c
>> +++ b/arch/arm/mach-davinci/da850.c
>> @@ -402,6 +402,7 @@ static struct clk_lookup da850_clks[] = {
>>       CLK(NULL,               "arm",          &arm_clk),
>>       CLK(NULL,               "rmii",         &rmii_clk),
>>       CLK("davinci_emac.1",   NULL,           &emac_clk),
>> +     CLK("davinci_mdio.0",   "fck",          &emac_clk),
>>       CLK("davinci-mcasp.0",  NULL,           &mcasp_clk),
>>       CLK("da8xx_lcdc.0",     "fck",          &lcdc_clk),
>>       CLK("davinci_mmc.0",    NULL,           &mmcsd0_clk),
>> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
>> index 2d5502d..52faa05 100644
>> --- a/arch/arm/mach-davinci/devices-da8xx.c
>> +++ b/arch/arm/mach-davinci/devices-da8xx.c
>> @@ -444,12 +444,8 @@ int __init da8xx_register_emac(void)
>>       ret = platform_device_register(&da8xx_mdio_device);
>>       if (ret < 0)
>>               return ret;
>> -     ret = platform_device_register(&da8xx_emac_device);
>> -     if (ret < 0)
>> -             return ret;
>> -     ret = clk_add_alias(NULL, dev_name(&da8xx_mdio_device.dev),
>> -                         NULL, &da8xx_emac_device.dev);
>> -     return ret;
>> +
>> +     return platform_device_register(&da8xx_emac_device);
>
> You added the look-up only for DA850 which means after this patch da830
> network will be broken. Did you test on DA830?

Ah yes my bad this will break network on da830, I'll fix it and respin
this patch
alone only, if there aren't any comments for the rest of the series.

Regards,
--Prabhakar


> Thanks,
> Sekhar

Re: [PATCH v2 2/6] ARM: davinci: da850: add DT node for mdio device

[Sekhar Nori]

On 1/28/2013 7:17 PM, Prabhakar Lad wrote:
> From: Lad, Prabhakar <prabhakar.lad@ti.com>
> 
> Add mdio device tree node information to da850 by
> providing register details and bus frequency of mdio.
> 
> Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: davinci-linux-open-source@linux.davincidsp.com
> Cc: netdev@vger.kernel.org
> Cc: devicetree-discuss@lists.ozlabs.org
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Heiko Schocher <hs@denx.de>
> ---
>  arch/arm/boot/dts/da850-evm.dts |    3 +++
>  arch/arm/boot/dts/da850.dtsi    |    7 +++++++
>  2 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
> index 98c1a48..a319491 100644
> --- a/arch/arm/boot/dts/da850-evm.dts
> +++ b/arch/arm/boot/dts/da850-evm.dts
> @@ -27,6 +27,9 @@
>  		serial2: serial@1d0d000 {
>  			status = "okay";
>  		};
> +		mdio: davinci_mdio@1e24000 {

This should just be:

mdio: mdio@1e24000

inline with how we avoid adding davinci prefix to each node name.

> +			status = "okay";
> +		};
>  	};
>  	nand_cs3@62000000 {
>  		status = "okay";
> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> index 7c84822..ba28f2d 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -81,6 +81,13 @@
>  			interrupts = <61>;
>  			status = "disabled";
>  		};
> +		mdio: davinci_mdio@1e24000 {
> +			compatible = "ti,davinci_mdio";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <0x224000 0x1000>;
> +			bus_freq = <2200000>;

The bus frequency is board specific and it is better this is defined in
da850-evm.dts

Thanks,
Sekhar

Re: [PATCH v2 4/6] ARM: davinci: da850: add DT node for eth0.

[Sekhar Nori]

On 1/28/2013 7:17 PM, Prabhakar Lad wrote:
> From: Lad, Prabhakar <prabhakar.lad@ti.com>
> 
> Add eth0 device tree node information and pinmux for mii to da850 by
> providing interrupt details and local mac address of eth0.
> 
> Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: davinci-linux-open-source@linux.davincidsp.com
> Cc: netdev@vger.kernel.org
> Cc: devicetree-discuss@lists.ozlabs.org
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Heiko Schocher <hs@denx.de>
> ---
>  arch/arm/boot/dts/da850-evm.dts |    5 +++++
>  arch/arm/boot/dts/da850.dtsi    |   35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 40 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
> index a319491..19aa2b3 100644
> --- a/arch/arm/boot/dts/da850-evm.dts
> +++ b/arch/arm/boot/dts/da850-evm.dts
> @@ -30,6 +30,11 @@
>  		mdio: davinci_mdio@1e24000 {
>  			status = "okay";
>  		};
> +		eth0: emac@1e20000 {
> +			status = "okay";
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&mii_pins>;
> +		};
>  	};
>  	nand_cs3@62000000 {
>  		status = "okay";
> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> index ba28f2d..76905f3 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -56,6 +56,26 @@
>  					0x30 0x01100000  0x0ff00000
>  				>;
>  			};
> +			mii_pins: pinmux_mii_pins {
> +				pinctrl-single,bits = <
> +					/*
> +					 * MII_TXEN, MII_TXCLK, MII_COL
> +					 * MII_TXD_3, MII_TXD_2, MII_TXD_1
> +					 * MII_TXD_0
> +					 */
> +					0x8 0x88888880 0xfffffff0
> +					/*
> +					 * MII_RXER, MII_CRS, MII_RXCLK
> +					 * MII_RXDV, MII_RXD_3, MII_RXD_2
> +					 * MII_RXD_1, MII_RXD_0
> +					 */
> +					0xc 0x88888888 0xffffffff
> +					/* MDIO_CLK, MDIO_D */

You call this mii_pins, but include mdio pins in there as well. Can you
separate them out? Then some board which uses rmii can simply reuse the
entry.

> +					0x10 0x00222288 0x00ffffff
> +					/* GPIO2_6 */
> +					0x18 0x00000080 0x000000f0

This is SoC specific pin list. Such board specific pins should not make
it here.

> +				>;
> +			};
>  		};
>  		serial0: serial@1c42000 {
>  			compatible = "ns16550a";
> @@ -88,6 +108,21 @@
>  			reg = <0x224000 0x1000>;
>  			bus_freq = <2200000>;
>  		};
> +		eth0: emac@1e20000 {
> +			compatible = "ti,davinci-dm6467-emac";
> +			reg = <0x220000 0x4000>;
> +			ti,davinci-ctrl-reg-offset = <0x3000>;
> +			ti,davinci-ctrl-mod-reg-offset = <0x2000>;
> +			ti,davinci-ctrl-ram-offset = <0>;
> +			ti,davinci-ctrl-ram-size = <0x2000>;
> +			local-mac-address = [ 00 00 00 00 00 00 ];
> +			interrupts = <33
> +					34
> +					35
> +					36
> +					>;
> +			phy-handle = <&mdio>;

I doubt this is required. This property is to pass a handle to the phy,
not mdio bus.

Thanks,
Sekhar

Re: [PATCH v2 2/6] ARM: davinci: da850: add DT node for mdio device

[Prabhakar Lad]

Sekhar,

On Sun, Feb 3, 2013 at 5:23 PM, Sekhar Nori <nsekhar@ti.com> wrote:
> On 1/28/2013 7:17 PM, Prabhakar Lad wrote:
>> From: Lad, Prabhakar <prabhakar.lad@ti.com>
>>
>> Add mdio device tree node information to da850 by
>> providing register details and bus frequency of mdio.
>>
>> Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: davinci-linux-open-source@linux.davincidsp.com
>> Cc: netdev@vger.kernel.org
>> Cc: devicetree-discuss@lists.ozlabs.org
>> Cc: Sekhar Nori <nsekhar@ti.com>
>> Cc: Heiko Schocher <hs@denx.de>
>> ---
>>  arch/arm/boot/dts/da850-evm.dts |    3 +++
>>  arch/arm/boot/dts/da850.dtsi    |    7 +++++++
>>  2 files changed, 10 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
>> index 98c1a48..a319491 100644
>> --- a/arch/arm/boot/dts/da850-evm.dts
>> +++ b/arch/arm/boot/dts/da850-evm.dts
>> @@ -27,6 +27,9 @@
>>               serial2: serial@1d0d000 {
>>                       status = "okay";
>>               };
>> +             mdio: davinci_mdio@1e24000 {
>
> This should just be:
>
> mdio: mdio@1e24000
>
> inline with how we avoid adding davinci prefix to each node name.
>
Ok

>> +                     status = "okay";
>> +             };
>>       };
>>       nand_cs3@62000000 {
>>               status = "okay";
>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>> index 7c84822..ba28f2d 100644
>> --- a/arch/arm/boot/dts/da850.dtsi
>> +++ b/arch/arm/boot/dts/da850.dtsi
>> @@ -81,6 +81,13 @@
>>                       interrupts = <61>;
>>                       status = "disabled";
>>               };
>> +             mdio: davinci_mdio@1e24000 {
>> +                     compatible = "ti,davinci_mdio";
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     reg = <0x224000 0x1000>;
>> +                     bus_freq = <2200000>;
>
> The bus frequency is board specific and it is better this is defined in
> da850-evm.dts
>
Ok I'll move this to dts file.

Regards,
--Prabhakar

> Thanks,
> Sekhar

Re: [PATCH v2 6/6] ARM: davinci: da850: configure system configuration chip(CFGCHIP3) for emac

[Sekhar Nori]

On 1/28/2013 7:17 PM, Prabhakar Lad wrote:
> From: Lad, Prabhakar <prabhakar.lad@ti.com>
> 
> The system configuration chip CFGCHIP3, controls the emac module.
> This patch appropriately configures this register for emac and
> sets DA850_MII_MDIO_CLKEN_PIN GPIO pin appropriately.
> 
> Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: davinci-linux-open-source@linux.davincidsp.com
> Cc: netdev@vger.kernel.org
> Cc: devicetree-discuss@lists.ozlabs.org
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Heiko Schocher <hs@denx.de>
> ---
>  arch/arm/mach-davinci/da8xx-dt.c |   28 ++++++++++++++++++++++++++++
>  1 files changed, 28 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
> index e533a0a..4a096e3 100644
> --- a/arch/arm/mach-davinci/da8xx-dt.c
> +++ b/arch/arm/mach-davinci/da8xx-dt.c
> @@ -8,6 +8,7 @@
>   * published by the Free Software Foundation.
>   */
>  #include <linux/io.h>
> +#include <linux/gpio.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_platform.h>
>  #include <linux/irqdomain.h>
> @@ -39,6 +40,32 @@ static void __init da8xx_init_irq(void)
>  
>  #ifdef CONFIG_ARCH_DAVINCI_DA850
>  
> +static void __init da8xx_config_emac(void)
> +{
> +#define DA850_MII_MDIO_CLKEN_PIN	GPIO_TO_PIN(2, 6)
> +#define DA850_EMAC_MODE_SELECT		BIT(8)
> +	void __iomem *cfg_chip3_base;
> +	int ret;
> +	u32 val;
> +
> +	cfg_chip3_base = DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP3_REG);
> +
> +	val = __raw_readl(cfg_chip3_base);
> +	val &= ~DA850_EMAC_MODE_SELECT;
> +	/* configure the CFGCHIP3 register for MII */
> +	__raw_writel(val, cfg_chip3_base);

Use readl/writel instead.

> +	pr_info("EMAC: MII PHY configured\n");
> +
> +	ret = gpio_request(DA850_MII_MDIO_CLKEN_PIN, "mdio_clk_en");
> +	if (ret) {
> +		pr_warn("Cannot open GPIO %d\n",
> +					DA850_MII_MDIO_CLKEN_PIN);
> +		return;
> +	}
> +	/* Enable/Disable MII MDIO clock */
> +	gpio_direction_output(DA850_MII_MDIO_CLKEN_PIN, 0);
> +}
> +
>  struct of_dev_auxdata da8xx_auxdata[] __initdata = {
>  	OF_DEV_AUXDATA("ti,davinci_mdio", 0x01e24000, "davinci_mdio.0", NULL),
>  	OF_DEV_AUXDATA("ti,davinci-dm6467-emac", 0x01e20000, "davinci_emac.1",
> @@ -52,6 +79,7 @@ static void __init da850_init_machine(void)
>  			     da8xx_auxdata, NULL);
>  
>  	da8xx_uart_clk_enable();
> +	da8xx_config_emac();

There are couple of issues with this implementation.

1) da8xx_config_emac() is specific to DA850 EVM, but masquerades as
   generic for da8xx. Looks like you need two functions, one for soc
   specific configuration and one board specific.
2) da8xx_config_emac() goes through all the time, whether the
   particular board has emac module or not. Shouldn't
   da8xx_config_emac() check if emac is actually enabled in the passed
   dtb and only the do the configuration?
3) The function assumes mii is used always, you can use the rmii_en dt
   property to check if rmii/mii is enabled and configure the soc/board
   accordingly.
4) If the same function can work both for da850 and da830, then it can
   be implemented outside of CONFIG_ARCH_DAVINCI_DA850.

Thanks,
Sekhar

Re: [PATCH v2 6/6] ARM: davinci: da850: configure system configuration chip(CFGCHIP3) for emac

[Prabhakar Lad]

Sekhar ,

On Sun, Feb 3, 2013 at 6:15 PM, Sekhar Nori <nsekhar@ti.com> wrote:
> On 1/28/2013 7:17 PM, Prabhakar Lad wrote:
>> From: Lad, Prabhakar <prabhakar.lad@ti.com>
>>
>> The system configuration chip CFGCHIP3, controls the emac module.
>> This patch appropriately configures this register for emac and
>> sets DA850_MII_MDIO_CLKEN_PIN GPIO pin appropriately.
>>
>> Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: davinci-linux-open-source@linux.davincidsp.com
>> Cc: netdev@vger.kernel.org
>> Cc: devicetree-discuss@lists.ozlabs.org
>> Cc: Sekhar Nori <nsekhar@ti.com>
>> Cc: Heiko Schocher <hs@denx.de>
>> ---
>>  arch/arm/mach-davinci/da8xx-dt.c |   28 ++++++++++++++++++++++++++++
>>  1 files changed, 28 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
>> index e533a0a..4a096e3 100644
>> --- a/arch/arm/mach-davinci/da8xx-dt.c
>> +++ b/arch/arm/mach-davinci/da8xx-dt.c
>> @@ -8,6 +8,7 @@
>>   * published by the Free Software Foundation.
>>   */
>>  #include <linux/io.h>
>> +#include <linux/gpio.h>
>>  #include <linux/of_irq.h>
>>  #include <linux/of_platform.h>
>>  #include <linux/irqdomain.h>
>> @@ -39,6 +40,32 @@ static void __init da8xx_init_irq(void)
>>
>>  #ifdef CONFIG_ARCH_DAVINCI_DA850
>>
>> +static void __init da8xx_config_emac(void)
>> +{
>> +#define DA850_MII_MDIO_CLKEN_PIN     GPIO_TO_PIN(2, 6)
>> +#define DA850_EMAC_MODE_SELECT               BIT(8)
>> +     void __iomem *cfg_chip3_base;
>> +     int ret;
>> +     u32 val;
>> +
>> +     cfg_chip3_base = DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP3_REG);
>> +
>> +     val = __raw_readl(cfg_chip3_base);
>> +     val &= ~DA850_EMAC_MODE_SELECT;
>> +     /* configure the CFGCHIP3 register for MII */
>> +     __raw_writel(val, cfg_chip3_base);
>
> Use readl/writel instead.
>
Ok.

>> +     pr_info("EMAC: MII PHY configured\n");
>> +
>> +     ret = gpio_request(DA850_MII_MDIO_CLKEN_PIN, "mdio_clk_en");
>> +     if (ret) {
>> +             pr_warn("Cannot open GPIO %d\n",
>> +                                     DA850_MII_MDIO_CLKEN_PIN);
>> +             return;
>> +     }
>> +     /* Enable/Disable MII MDIO clock */
>> +     gpio_direction_output(DA850_MII_MDIO_CLKEN_PIN, 0);
>> +}
>> +
>>  struct of_dev_auxdata da8xx_auxdata[] __initdata = {
>>       OF_DEV_AUXDATA("ti,davinci_mdio", 0x01e24000, "davinci_mdio.0", NULL),
>>       OF_DEV_AUXDATA("ti,davinci-dm6467-emac", 0x01e20000, "davinci_emac.1",
>> @@ -52,6 +79,7 @@ static void __init da850_init_machine(void)
>>                            da8xx_auxdata, NULL);
>>
>>       da8xx_uart_clk_enable();
>> +     da8xx_config_emac();
>
> There are couple of issues with this implementation.
>
> 1) da8xx_config_emac() is specific to DA850 EVM, but masquerades as
>    generic for da8xx. Looks like you need two functions, one for soc
>    specific configuration and one board specific.
Ok.

> 2) da8xx_config_emac() goes through all the time, whether the
>    particular board has emac module or not. Shouldn't
>    da8xx_config_emac() check if emac is actually enabled in the passed
>    dtb and only the do the configuration?

Ok. Can you give some pointers how we can access the dtb in board files.

> 3) The function assumes mii is used always, you can use the rmii_en dt
>    property to check if rmii/mii is enabled and configure the soc/board
>    accordingly.

Ok.

> 4) If the same function can work both for da850 and da830, then it can
>    be implemented outside of CONFIG_ARCH_DAVINCI_DA850.
>
Ok.

Regards,
--Prabhakar

> Thanks,
> Sekhar

Re: [PATCH v2 4/6] ARM: davinci: da850: add DT node for eth0.

[Prabhakar Lad]

Sekhar ,

On Sun, Feb 3, 2013 at 5:33 PM, Sekhar Nori <nsekhar@ti.com> wrote:
> On 1/28/2013 7:17 PM, Prabhakar Lad wrote:
>> From: Lad, Prabhakar <prabhakar.lad@ti.com>
>>
>> Add eth0 device tree node information and pinmux for mii to da850 by
>> providing interrupt details and local mac address of eth0.
>>
>> Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: davinci-linux-open-source@linux.davincidsp.com
>> Cc: netdev@vger.kernel.org
>> Cc: devicetree-discuss@lists.ozlabs.org
>> Cc: Sekhar Nori <nsekhar@ti.com>
>> Cc: Heiko Schocher <hs@denx.de>
>> ---
>>  arch/arm/boot/dts/da850-evm.dts |    5 +++++
>>  arch/arm/boot/dts/da850.dtsi    |   35 +++++++++++++++++++++++++++++++++++
>>  2 files changed, 40 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
>> index a319491..19aa2b3 100644
>> --- a/arch/arm/boot/dts/da850-evm.dts
>> +++ b/arch/arm/boot/dts/da850-evm.dts
>> @@ -30,6 +30,11 @@
>>               mdio: davinci_mdio@1e24000 {
>>                       status = "okay";
>>               };
>> +             eth0: emac@1e20000 {
>> +                     status = "okay";
>> +                     pinctrl-names = "default";
>> +                     pinctrl-0 = <&mii_pins>;
>> +             };
>>       };
>>       nand_cs3@62000000 {
>>               status = "okay";
>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>> index ba28f2d..76905f3 100644
>> --- a/arch/arm/boot/dts/da850.dtsi
>> +++ b/arch/arm/boot/dts/da850.dtsi
>> @@ -56,6 +56,26 @@
>>                                       0x30 0x01100000  0x0ff00000
>>                               >;
>>                       };
>> +                     mii_pins: pinmux_mii_pins {
>> +                             pinctrl-single,bits = <
>> +                                     /*
>> +                                      * MII_TXEN, MII_TXCLK, MII_COL
>> +                                      * MII_TXD_3, MII_TXD_2, MII_TXD_1
>> +                                      * MII_TXD_0
>> +                                      */
>> +                                     0x8 0x88888880 0xfffffff0
>> +                                     /*
>> +                                      * MII_RXER, MII_CRS, MII_RXCLK
>> +                                      * MII_RXDV, MII_RXD_3, MII_RXD_2
>> +                                      * MII_RXD_1, MII_RXD_0
>> +                                      */
>> +                                     0xc 0x88888888 0xffffffff
>> +                                     /* MDIO_CLK, MDIO_D */
>
> You call this mii_pins, but include mdio pins in there as well. Can you
> separate them out? Then some board which uses rmii can simply reuse the
> entry.
>
Ok makes sense.

>> +                                     0x10 0x00222288 0x00ffffff
>> +                                     /* GPIO2_6 */
>> +                                     0x18 0x00000080 0x000000f0
>
> This is SoC specific pin list. Such board specific pins should not make
> it here.
>
Ok, so this should be set up using GPIO API's ?

>> +                             >;
>> +                     };
>>               };
>>               serial0: serial@1c42000 {
>>                       compatible = "ns16550a";
>> @@ -88,6 +108,21 @@
>>                       reg = <0x224000 0x1000>;
>>                       bus_freq = <2200000>;
>>               };
>> +             eth0: emac@1e20000 {
>> +                     compatible = "ti,davinci-dm6467-emac";
>> +                     reg = <0x220000 0x4000>;
>> +                     ti,davinci-ctrl-reg-offset = <0x3000>;
>> +                     ti,davinci-ctrl-mod-reg-offset = <0x2000>;
>> +                     ti,davinci-ctrl-ram-offset = <0>;
>> +                     ti,davinci-ctrl-ram-size = <0x2000>;
>> +                     local-mac-address = [ 00 00 00 00 00 00 ];
>> +                     interrupts = <33
>> +                                     34
>> +                                     35
>> +                                     36
>> +                                     >;
>> +                     phy-handle = <&mdio>;
>
> I doubt this is required. This property is to pass a handle to the phy,
> not mdio bus.
>
Ok I'll check on this.

Regards,
--Prabhakar


> Thanks,
> Sekhar

Re: [PATCH v2 6/6] ARM: davinci: da850: configure system configuration chip(CFGCHIP3) for emac

[Sekhar Nori]

On 2/3/2013 6:15 PM, Sekhar Nori wrote:
> On 1/28/2013 7:17 PM, Prabhakar Lad wrote:
>> From: Lad, Prabhakar <prabhakar.lad@ti.com>

> There are couple of issues with this implementation.
> 
> 1) da8xx_config_emac() is specific to DA850 EVM, but masquerades as
>    generic for da8xx. Looks like you need two functions, one for soc
>    specific configuration and one board specific.
> 2) da8xx_config_emac() goes through all the time, whether the
>    particular board has emac module or not. Shouldn't
>    da8xx_config_emac() check if emac is actually enabled in the passed
>    dtb and only the do the configuration?
> 3) The function assumes mii is used always, you can use the rmii_en dt
>    property to check if rmii/mii is enabled and configure the soc/board
>    accordingly.
> 4) If the same function can work both for da850 and da830, then it can
>    be implemented outside of CONFIG_ARCH_DAVINCI_DA850.

Another thing I forgot to mention:

5) This patch replicates a lot of code form board-da850-evm.c. This
needs to be avoided. May be create function to configure emac in
da850.c. This can then be called in board-da850-evm.c as well as in the
dt case.

Thanks,
Sekhar

Re: [PATCH v2 4/6] ARM: davinci: da850: add DT node for eth0.

[Sekhar Nori]

On 2/4/2013 10:37 AM, Prabhakar Lad wrote:
> Sekhar ,
> 
> On Sun, Feb 3, 2013 at 5:33 PM, Sekhar Nori <nsekhar@ti.com> wrote:
>> On 1/28/2013 7:17 PM, Prabhakar Lad wrote:
>>> From: Lad, Prabhakar <prabhakar.lad@ti.com>
>>>
>>> Add eth0 device tree node information and pinmux for mii to da850 by
>>> providing interrupt details and local mac address of eth0.
>>>
>>> Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Cc: davinci-linux-open-source@linux.davincidsp.com
>>> Cc: netdev@vger.kernel.org
>>> Cc: devicetree-discuss@lists.ozlabs.org
>>> Cc: Sekhar Nori <nsekhar@ti.com>
>>> Cc: Heiko Schocher <hs@denx.de>
>>> ---
>>>  arch/arm/boot/dts/da850-evm.dts |    5 +++++
>>>  arch/arm/boot/dts/da850.dtsi    |   35 +++++++++++++++++++++++++++++++++++
>>>  2 files changed, 40 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
>>> index a319491..19aa2b3 100644
>>> --- a/arch/arm/boot/dts/da850-evm.dts
>>> +++ b/arch/arm/boot/dts/da850-evm.dts
>>> @@ -30,6 +30,11 @@
>>>               mdio: davinci_mdio@1e24000 {
>>>                       status = "okay";
>>>               };
>>> +             eth0: emac@1e20000 {
>>> +                     status = "okay";
>>> +                     pinctrl-names = "default";
>>> +                     pinctrl-0 = <&mii_pins>;
>>> +             };
>>>       };
>>>       nand_cs3@62000000 {
>>>               status = "okay";
>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>> index ba28f2d..76905f3 100644
>>> --- a/arch/arm/boot/dts/da850.dtsi
>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>> @@ -56,6 +56,26 @@
>>>                                       0x30 0x01100000  0x0ff00000
>>>                               >;
>>>                       };
>>> +                     mii_pins: pinmux_mii_pins {
>>> +                             pinctrl-single,bits = <
>>> +                                     /*
>>> +                                      * MII_TXEN, MII_TXCLK, MII_COL
>>> +                                      * MII_TXD_3, MII_TXD_2, MII_TXD_1
>>> +                                      * MII_TXD_0
>>> +                                      */
>>> +                                     0x8 0x88888880 0xfffffff0
>>> +                                     /*
>>> +                                      * MII_RXER, MII_CRS, MII_RXCLK
>>> +                                      * MII_RXDV, MII_RXD_3, MII_RXD_2
>>> +                                      * MII_RXD_1, MII_RXD_0
>>> +                                      */
>>> +                                     0xc 0x88888888 0xffffffff
>>> +                                     /* MDIO_CLK, MDIO_D */
>>
>> You call this mii_pins, but include mdio pins in there as well. Can you
>> separate them out? Then some board which uses rmii can simply reuse the
>> entry.
>>
> Ok makes sense.
> 
>>> +                                     0x10 0x00222288 0x00ffffff
>>> +                                     /* GPIO2_6 */
>>> +                                     0x18 0x00000080 0x000000f0
>>
>> This is SoC specific pin list. Such board specific pins should not make
>> it here.
>>
> Ok, so this should be set up using GPIO API's ?

Yes, the pins ultimately will be controlled by gpiolib APIs, but the
pins should be defined under the eth node in .dts

Thanks,
Sekhar

Re: [PATCH v2 4/6] ARM: davinci: da850: add DT node for eth0.

[Prabhakar Lad]

Sekhar,

On Mon, Feb 4, 2013 at 11:20 PM, Sekhar Nori <nsekhar@ti.com> wrote:
> On 2/4/2013 10:37 AM, Prabhakar Lad wrote:
>> Sekhar ,
>>
>> On Sun, Feb 3, 2013 at 5:33 PM, Sekhar Nori <nsekhar@ti.com> wrote:
>>> On 1/28/2013 7:17 PM, Prabhakar Lad wrote:
>>>> From: Lad, Prabhakar <prabhakar.lad@ti.com>
>>>>
>>>> Add eth0 device tree node information and pinmux for mii to da850 by
>>>> providing interrupt details and local mac address of eth0.
>>>>
>>>> Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>> Cc: linux-kernel@vger.kernel.org
>>>> Cc: davinci-linux-open-source@linux.davincidsp.com
>>>> Cc: netdev@vger.kernel.org
>>>> Cc: devicetree-discuss@lists.ozlabs.org
>>>> Cc: Sekhar Nori <nsekhar@ti.com>
>>>> Cc: Heiko Schocher <hs@denx.de>
>>>> ---
>>>>  arch/arm/boot/dts/da850-evm.dts |    5 +++++
>>>>  arch/arm/boot/dts/da850.dtsi    |   35 +++++++++++++++++++++++++++++++++++
>>>>  2 files changed, 40 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
>>>> index a319491..19aa2b3 100644
>>>> --- a/arch/arm/boot/dts/da850-evm.dts
>>>> +++ b/arch/arm/boot/dts/da850-evm.dts
>>>> @@ -30,6 +30,11 @@
>>>>               mdio: davinci_mdio@1e24000 {
>>>>                       status = "okay";
>>>>               };
>>>> +             eth0: emac@1e20000 {
>>>> +                     status = "okay";
>>>> +                     pinctrl-names = "default";
>>>> +                     pinctrl-0 = <&mii_pins>;
>>>> +             };
>>>>       };
>>>>       nand_cs3@62000000 {
>>>>               status = "okay";
>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>>> index ba28f2d..76905f3 100644
>>>> --- a/arch/arm/boot/dts/da850.dtsi
>>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>>> @@ -56,6 +56,26 @@
>>>>                                       0x30 0x01100000  0x0ff00000
>>>>                               >;
>>>>                       };
>>>> +                     mii_pins: pinmux_mii_pins {
>>>> +                             pinctrl-single,bits = <
>>>> +                                     /*
>>>> +                                      * MII_TXEN, MII_TXCLK, MII_COL
>>>> +                                      * MII_TXD_3, MII_TXD_2, MII_TXD_1
>>>> +                                      * MII_TXD_0
>>>> +                                      */
>>>> +                                     0x8 0x88888880 0xfffffff0
>>>> +                                     /*
>>>> +                                      * MII_RXER, MII_CRS, MII_RXCLK
>>>> +                                      * MII_RXDV, MII_RXD_3, MII_RXD_2
>>>> +                                      * MII_RXD_1, MII_RXD_0
>>>> +                                      */
>>>> +                                     0xc 0x88888888 0xffffffff
>>>> +                                     /* MDIO_CLK, MDIO_D */
>>>
>>> You call this mii_pins, but include mdio pins in there as well. Can you
>>> separate them out? Then some board which uses rmii can simply reuse the
>>> entry.
>>>
>> Ok makes sense.
>>
>>>> +                                     0x10 0x00222288 0x00ffffff
>>>> +                                     /* GPIO2_6 */
>>>> +                                     0x18 0x00000080 0x000000f0
>>>
>>> This is SoC specific pin list. Such board specific pins should not make
>>> it here.
>>>
>> Ok, so this should be set up using GPIO API's ?
>
> Yes, the pins ultimately will be controlled by gpiolib APIs, but the
> pins should be defined under the eth node in .dts
>
You mean '0x18 0x00000080 0x000000f0'  still needs to defined in eth0 node ?
Can you elaborate and give some pointers.

Regards,
--Prabhakar

> Thanks,
> Sekhar

Re: [PATCH v2 4/6] ARM: davinci: da850: add DT node for eth0.

[Sekhar Nori]

On 3/8/2013 3:15 PM, Prabhakar Lad wrote:
> Sekhar,
> 
> On Mon, Feb 4, 2013 at 11:20 PM, Sekhar Nori <nsekhar@ti.com> wrote:
>> On 2/4/2013 10:37 AM, Prabhakar Lad wrote:
>>> Sekhar ,
>>>
>>> On Sun, Feb 3, 2013 at 5:33 PM, Sekhar Nori <nsekhar@ti.com> wrote:
>>>> On 1/28/2013 7:17 PM, Prabhakar Lad wrote:
>>>>> From: Lad, Prabhakar <prabhakar.lad@ti.com>
>>>>>
>>>>> Add eth0 device tree node information and pinmux for mii to da850 by
>>>>> providing interrupt details and local mac address of eth0.
>>>>>
>>>>> Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
>>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>>> Cc: linux-kernel@vger.kernel.org
>>>>> Cc: davinci-linux-open-source@linux.davincidsp.com
>>>>> Cc: netdev@vger.kernel.org
>>>>> Cc: devicetree-discuss@lists.ozlabs.org
>>>>> Cc: Sekhar Nori <nsekhar@ti.com>
>>>>> Cc: Heiko Schocher <hs@denx.de>
>>>>> ---
>>>>>  arch/arm/boot/dts/da850-evm.dts |    5 +++++
>>>>>  arch/arm/boot/dts/da850.dtsi    |   35 +++++++++++++++++++++++++++++++++++
>>>>>  2 files changed, 40 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
>>>>> index a319491..19aa2b3 100644
>>>>> --- a/arch/arm/boot/dts/da850-evm.dts
>>>>> +++ b/arch/arm/boot/dts/da850-evm.dts
>>>>> @@ -30,6 +30,11 @@
>>>>>               mdio: davinci_mdio@1e24000 {
>>>>>                       status = "okay";
>>>>>               };
>>>>> +             eth0: emac@1e20000 {
>>>>> +                     status = "okay";
>>>>> +                     pinctrl-names = "default";
>>>>> +                     pinctrl-0 = <&mii_pins>;
>>>>> +             };
>>>>>       };
>>>>>       nand_cs3@62000000 {
>>>>>               status = "okay";
>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>>>> index ba28f2d..76905f3 100644
>>>>> --- a/arch/arm/boot/dts/da850.dtsi
>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>>>> @@ -56,6 +56,26 @@
>>>>>                                       0x30 0x01100000  0x0ff00000
>>>>>                               >;
>>>>>                       };
>>>>> +                     mii_pins: pinmux_mii_pins {
>>>>> +                             pinctrl-single,bits = <
>>>>> +                                     /*
>>>>> +                                      * MII_TXEN, MII_TXCLK, MII_COL
>>>>> +                                      * MII_TXD_3, MII_TXD_2, MII_TXD_1
>>>>> +                                      * MII_TXD_0
>>>>> +                                      */
>>>>> +                                     0x8 0x88888880 0xfffffff0
>>>>> +                                     /*
>>>>> +                                      * MII_RXER, MII_CRS, MII_RXCLK
>>>>> +                                      * MII_RXDV, MII_RXD_3, MII_RXD_2
>>>>> +                                      * MII_RXD_1, MII_RXD_0
>>>>> +                                      */
>>>>> +                                     0xc 0x88888888 0xffffffff
>>>>> +                                     /* MDIO_CLK, MDIO_D */
>>>>
>>>> You call this mii_pins, but include mdio pins in there as well. Can you
>>>> separate them out? Then some board which uses rmii can simply reuse the
>>>> entry.
>>>>
>>> Ok makes sense.
>>>
>>>>> +                                     0x10 0x00222288 0x00ffffff
>>>>> +                                     /* GPIO2_6 */
>>>>> +                                     0x18 0x00000080 0x000000f0
>>>>
>>>> This is SoC specific pin list. Such board specific pins should not make
>>>> it here.
>>>>
>>> Ok, so this should be set up using GPIO API's ?
>>
>> Yes, the pins ultimately will be controlled by gpiolib APIs, but the
>> pins should be defined under the eth node in .dts
>>
> You mean '0x18 0x00000080 0x000000f0'  still needs to defined in eth0 node ?
> Can you elaborate and give some pointers.

Since the GPIO usage for ethernet is board specific, it cannot be in
.dtsi which is SoC generic. I am suggesting do in the .dts file which is
board specific.

Thanks,
Sekhar

Re: [PATCH v2 6/6] ARM: davinci: da850: configure system configuration chip(CFGCHIP3) for emac

[Prabhakar Lad]

Sekhar,

On Mon, Feb 4, 2013 at 10:33 AM, Prabhakar Lad
<prabhakar.csengg@gmail.com> wrote:
> Sekhar ,
>
> On Sun, Feb 3, 2013 at 6:15 PM, Sekhar Nori <nsekhar@ti.com> wrote:
>> On 1/28/2013 7:17 PM, Prabhakar Lad wrote:
>>> From: Lad, Prabhakar <prabhakar.lad@ti.com>
>>>
>>> The system configuration chip CFGCHIP3, controls the emac module.
>>> This patch appropriately configures this register for emac and
>>> sets DA850_MII_MDIO_CLKEN_PIN GPIO pin appropriately.
>>>
>>> Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Cc: davinci-linux-open-source@linux.davincidsp.com
>>> Cc: netdev@vger.kernel.org
>>> Cc: devicetree-discuss@lists.ozlabs.org
>>> Cc: Sekhar Nori <nsekhar@ti.com>
>>> Cc: Heiko Schocher <hs@denx.de>
>>> ---
>>>  arch/arm/mach-davinci/da8xx-dt.c |   28 ++++++++++++++++++++++++++++
>>>  1 files changed, 28 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
>>> index e533a0a..4a096e3 100644
>>> --- a/arch/arm/mach-davinci/da8xx-dt.c
>>> +++ b/arch/arm/mach-davinci/da8xx-dt.c
>>> @@ -8,6 +8,7 @@
>>>   * published by the Free Software Foundation.
>>>   */
>>>  #include <linux/io.h>
>>> +#include <linux/gpio.h>
>>>  #include <linux/of_irq.h>
>>>  #include <linux/of_platform.h>
>>>  #include <linux/irqdomain.h>
>>> @@ -39,6 +40,32 @@ static void __init da8xx_init_irq(void)
>>>
>>>  #ifdef CONFIG_ARCH_DAVINCI_DA850
>>>
>>> +static void __init da8xx_config_emac(void)
>>> +{
>>> +#define DA850_MII_MDIO_CLKEN_PIN     GPIO_TO_PIN(2, 6)
>>> +#define DA850_EMAC_MODE_SELECT               BIT(8)
>>> +     void __iomem *cfg_chip3_base;
>>> +     int ret;
>>> +     u32 val;
>>> +
>>> +     cfg_chip3_base = DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP3_REG);
>>> +
>>> +     val = __raw_readl(cfg_chip3_base);
>>> +     val &= ~DA850_EMAC_MODE_SELECT;
>>> +     /* configure the CFGCHIP3 register for MII */
>>> +     __raw_writel(val, cfg_chip3_base);
>>
>> Use readl/writel instead.
>>
> Ok.
>
>>> +     pr_info("EMAC: MII PHY configured\n");
>>> +
>>> +     ret = gpio_request(DA850_MII_MDIO_CLKEN_PIN, "mdio_clk_en");
>>> +     if (ret) {
>>> +             pr_warn("Cannot open GPIO %d\n",
>>> +                                     DA850_MII_MDIO_CLKEN_PIN);
>>> +             return;
>>> +     }
>>> +     /* Enable/Disable MII MDIO clock */
>>> +     gpio_direction_output(DA850_MII_MDIO_CLKEN_PIN, 0);
>>> +}
>>> +
>>>  struct of_dev_auxdata da8xx_auxdata[] __initdata = {
>>>       OF_DEV_AUXDATA("ti,davinci_mdio", 0x01e24000, "davinci_mdio.0", NULL),
>>>       OF_DEV_AUXDATA("ti,davinci-dm6467-emac", 0x01e20000, "davinci_emac.1",
>>> @@ -52,6 +79,7 @@ static void __init da850_init_machine(void)
>>>                            da8xx_auxdata, NULL);
>>>
>>>       da8xx_uart_clk_enable();
>>> +     da8xx_config_emac();
>>
>> There are couple of issues with this implementation.
>>
>> 1) da8xx_config_emac() is specific to DA850 EVM, but masquerades as
>>    generic for da8xx. Looks like you need two functions, one for soc
>>    specific configuration and one board specific.
> Ok.
>
>> 2) da8xx_config_emac() goes through all the time, whether the
>>    particular board has emac module or not. Shouldn't
>>    da8xx_config_emac() check if emac is actually enabled in the passed
>>    dtb and only the do the configuration?
>
> Ok. Can you give some pointers how we can access the dtb in board files.
>
Any pointers on this could be helpful.

Regards,
--Prabhakar

>> 3) The function assumes mii is used always, you can use the rmii_en dt
>>    property to check if rmii/mii is enabled and configure the soc/board
>>    accordingly.
>
> Ok.
>
>> 4) If the same function can work both for da850 and da830, then it can
>>    be implemented outside of CONFIG_ARCH_DAVINCI_DA850.
>>
> Ok.
>
> Regards,
> --Prabhakar
>
>> Thanks,
>> Sekhar