[PATCH V2 0/7] ARM: SPEAr13xx: Minor fixes and updation

[PATCH V2 0/7] ARM: SPEAr13xx: Minor fixes and updation

[Vipul Kumar Samar]

-v2:
-Fix check retrun value from clk_set_rate and clk_set_parent.
-Fix sys_clk mask value.
-Additional patch for overwrite platform data passed from auxdata_lookup

-v1:
-Fix interrupt bindings for ethernet, i2c,.
-Fix clk enable register for uart1 and i2c1.
-Fix register con_id for ethernet.
-Update sys_clk parent.
-Pass ethernet platform data from auxdata lookup.
-Add local timer clock.

Vipul Kumar Samar (7):
  ARM: SPEAr13xx: Fix Interrupt bindings
  clk: SPEAr1340: Fix clk enable register for uart1 and i2c1.
  clk: SPEAr13xx: Add localtimer (twd) clock support
  Clk : SPEAr13xx: Register con_id for Ethernet phy clks
  Clk: SPEAr1340: Update sys clock parent array
  ARM: SPEAr13xx: Add auxdata for Ethernet controller.
  net: stmmac: Overwrite platform data if passed from auxdata

 arch/arm/boot/dts/spear13xx.dtsi                   |   11 +-
 arch/arm/mach-spear13xx/include/mach/generic.h     |    2 +
 arch/arm/mach-spear13xx/include/mach/spear.h       |    1 +
 arch/arm/mach-spear13xx/spear1340.c                |   32 ++++++
 arch/arm/mach-spear13xx/spear13xx.c                |  104 ++++++++++++++++++++
 drivers/clk/spear/spear1310_clock.c                |   14 ++-
 drivers/clk/spear/spear1340_clock.c                |   14 ++-
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |    5 +-
 8 files changed, 166 insertions(+), 17 deletions(-)

-- 
1.7.2.2

[PATCH V2 1/7] ARM: SPEAr13xx: Fix Interrupt bindings

[Vipul Kumar Samar]

-Correct interrupt bindings for uart, ethernet and pmu.
-Added interrupt binding for keyboard.

Signed-off-by: Vipul Kumar Samar <vipulkumar.samar@st.com>
Acked-by: Viiresh Kumar <viresh.kumar@linaro.org>
---
 arch/arm/boot/dts/spear13xx.dtsi |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/spear13xx.dtsi b/arch/arm/boot/dts/spear13xx.dtsi
index 10dcec7..f7b84ac 100644
--- a/arch/arm/boot/dts/spear13xx.dtsi
+++ b/arch/arm/boot/dts/spear13xx.dtsi
@@ -43,8 +43,8 @@
 
 	pmu {
 		compatible = "arm,cortex-a9-pmu";
-		interrupts = <0 8 0x04
-			      0 9 0x04>;
+		interrupts = <0 6 0x04
+			      0 7 0x04>;
 	};
 
 	L2: l2-cache {
@@ -119,8 +119,8 @@
 		gmac0: eth at e2000000 {
 			compatible = "st,spear600-gmac";
 			reg = <0xe2000000 0x8000>;
-			interrupts = <0 23 0x4
-				      0 24 0x4>;
+			interrupts = <0 33 0x4
+				      0 34 0x4>;
 			interrupt-names = "macirq", "eth_wake_irq";
 			status = "disabled";
 		};
@@ -202,6 +202,7 @@
 			kbd at e0300000 {
 				compatible = "st,spear300-kbd";
 				reg = <0xe0300000 0x1000>;
+				interrupts = <0 52 0x4>;
 				status = "disabled";
 			};
 
@@ -224,7 +225,7 @@
 			serial at e0000000 {
 				compatible = "arm,pl011", "arm,primecell";
 				reg = <0xe0000000 0x1000>;
-				interrupts = <0 36 0x4>;
+				interrupts = <0 35 0x4>;
 				status = "disabled";
 			};
 
-- 
1.7.2.2

[PATCH V2 2/7] clk: SPEAr1340: Fix clk enable register for uart1 and i2c1.

[Vipul Kumar Samar]

This patch is to fix typing mistake of clk enable register of i2c1 and uart1.

Signed-off-by: Vipul Kumar Samar <vipulkumar.samar@st.com>
Acked-by: Viiresh Kumar <viresh.kumar@linaro.org>
---
 drivers/clk/spear/spear1340_clock.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/spear/spear1340_clock.c b/drivers/clk/spear/spear1340_clock.c
index 0f2324b..4484698 100644
--- a/drivers/clk/spear/spear1340_clock.c
+++ b/drivers/clk/spear/spear1340_clock.c
@@ -617,7 +617,7 @@ void __init spear1340_clk_init(void)
 	clk_register_clkdev(clk, "uart1_mclk", NULL);
 
 	clk = clk_register_gate(NULL, "uart1_clk", "uart1_mclk", 0,
-			SPEAR1340_PERIP1_CLK_ENB, SPEAR1340_UART1_CLK_ENB, 0,
+			SPEAR1340_PERIP3_CLK_ENB, SPEAR1340_UART1_CLK_ENB, 0,
 			&_lock);
 	clk_register_clkdev(clk, NULL, "b4100000.serial");
 
@@ -741,7 +741,7 @@ void __init spear1340_clk_init(void)
 	clk_register_clkdev(clk, NULL, "e0280000.i2c");
 
 	clk = clk_register_gate(NULL, "i2c1_clk", "ahb_clk", 0,
-			SPEAR1340_PERIP1_CLK_ENB, SPEAR1340_I2C1_CLK_ENB, 0,
+			SPEAR1340_PERIP3_CLK_ENB, SPEAR1340_I2C1_CLK_ENB, 0,
 			&_lock);
 	clk_register_clkdev(clk, NULL, "b4000000.i2c");
 
-- 
1.7.2.2

[PATCH V2 3/7] clk: SPEAr13xx: Add localtimer (twd) clock support

[Vipul Kumar Samar]

Signed-off-by: Vipul Kumar Samar <vipulkumar.samar@st.com>
Acked-by: Viiresh Kumar <viresh.kumar@linaro.org>
---
 drivers/clk/spear/spear1310_clock.c |    4 ++++
 drivers/clk/spear/spear1340_clock.c |    4 ++++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/clk/spear/spear1310_clock.c b/drivers/clk/spear/spear1310_clock.c
index 0fcec2a..3d83ffc 100644
--- a/drivers/clk/spear/spear1310_clock.c
+++ b/drivers/clk/spear/spear1310_clock.c
@@ -490,6 +490,10 @@ void __init spear1310_clk_init(void)
 			2);
 	clk_register_clkdev(clk, NULL, "ec800620.wdt");
 
+	clk = clk_register_fixed_factor(NULL, "smp_twd_clk", "cpu_clk", 0, 1,
+			2);
+	clk_register_clkdev(clk, NULL, "smp_twd");
+
 	clk = clk_register_fixed_factor(NULL, "ahb_clk", "pll1_clk", 0, 1,
 			6);
 	clk_register_clkdev(clk, "ahb_clk", NULL);
diff --git a/drivers/clk/spear/spear1340_clock.c b/drivers/clk/spear/spear1340_clock.c
index 4484698..7815cbd 100644
--- a/drivers/clk/spear/spear1340_clock.c
+++ b/drivers/clk/spear/spear1340_clock.c
@@ -535,6 +535,10 @@ void __init spear1340_clk_init(void)
 			2);
 	clk_register_clkdev(clk, NULL, "ec800620.wdt");
 
+	clk = clk_register_fixed_factor(NULL, "smp_twd_clk", "cpu_clk", 0, 1,
+			2);
+	clk_register_clkdev(clk, NULL, "smp_twd");
+
 	clk = clk_register_mux(NULL, "ahb_clk", ahb_parents,
 			ARRAY_SIZE(ahb_parents), 0, SPEAR1340_SYS_CLK_CTRL,
 			SPEAR1340_HCLK_SRC_SEL_SHIFT,
-- 
1.7.2.2

[PATCH V2 4/7] Clk : SPEAr13xx: Register con_id for Ethernet phy clks

[Vipul Kumar Samar]

On kernel-3.5 stmmac Ethernet phy is not register as a separate device. So this
patch is to register con_id for phy clk.

Signed-off-by: Vipul Kumar Samar <vipulkumar.samar@st.com>
Acked-by: Viiresh Kumar <viresh.kumar@linaro.org>
---
 drivers/clk/spear/spear1310_clock.c |   10 +++++-----
 drivers/clk/spear/spear1340_clock.c |    2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/spear/spear1310_clock.c b/drivers/clk/spear/spear1310_clock.c
index 3d83ffc..a711662 100644
--- a/drivers/clk/spear/spear1310_clock.c
+++ b/drivers/clk/spear/spear1310_clock.c
@@ -619,7 +619,7 @@ void __init spear1310_clk_init(void)
 			ARRAY_SIZE(gmac_phy_parents), 0,
 			SPEAR1310_PERIP_CLK_CFG, SPEAR1310_GMAC_PHY_CLK_SHIFT,
 			SPEAR1310_GMAC_PHY_CLK_MASK, 0, &_lock);
-	clk_register_clkdev(clk, NULL, "stmmacphy.0");
+	clk_register_clkdev(clk, "stmmacphy.0", NULL);
 
 	/* clcd */
 	clk = clk_register_mux(NULL, "clcd_syn_mclk", clcd_synth_parents,
@@ -920,15 +920,15 @@ void __init spear1310_clk_init(void)
 			SPEAR1310_RAS_CTRL_REG1,
 			SPEAR1310_SMII_RGMII_PHY_CLK_SHIFT,
 			SPEAR1310_PHY_CLK_MASK, 0, &_lock);
-	clk_register_clkdev(clk, NULL, "stmmacphy.1");
-	clk_register_clkdev(clk, NULL, "stmmacphy.2");
-	clk_register_clkdev(clk, NULL, "stmmacphy.4");
+	clk_register_clkdev(clk, "stmmacphy.1", NULL);
+	clk_register_clkdev(clk, "stmmacphy.2", NULL);
+	clk_register_clkdev(clk, "stmmacphy.4", NULL);
 
 	clk = clk_register_mux(NULL, "rmii_phy_mclk", rmii_phy_parents,
 			ARRAY_SIZE(rmii_phy_parents), 0,
 			SPEAR1310_RAS_CTRL_REG1, SPEAR1310_RMII_PHY_CLK_SHIFT,
 			SPEAR1310_PHY_CLK_MASK, 0, &_lock);
-	clk_register_clkdev(clk, NULL, "stmmacphy.3");
+	clk_register_clkdev(clk, "stmmacphy.3", NULL);
 
 	clk = clk_register_mux(NULL, "uart1_mclk", uart_parents,
 			ARRAY_SIZE(uart_parents), 0, SPEAR1310_RAS_CTRL_REG0,
diff --git a/drivers/clk/spear/spear1340_clock.c b/drivers/clk/spear/spear1340_clock.c
index 7815cbd..2cd5520 100644
--- a/drivers/clk/spear/spear1340_clock.c
+++ b/drivers/clk/spear/spear1340_clock.c
@@ -683,7 +683,7 @@ void __init spear1340_clk_init(void)
 			ARRAY_SIZE(gmac_phy_parents), 0,
 			SPEAR1340_PERIP_CLK_CFG, SPEAR1340_GMAC_PHY_CLK_SHIFT,
 			SPEAR1340_GMAC_PHY_CLK_MASK, 0, &_lock);
-	clk_register_clkdev(clk, NULL, "stmmacphy.0");
+	clk_register_clkdev(clk, "stmmacphy.0", NULL);
 
 	/* clcd */
 	clk = clk_register_mux(NULL, "clcd_syn_mclk", clcd_synth_parents,
-- 
1.7.2.2

[PATCH V2 5/7] Clk: SPEAr1340: Update sys clock parent array

[Vipul Kumar Samar]

SYS_CLK have multiple parents and selection of parent depends on sys_clk_ctrl
register bit no. 23:25.
 0XX: pll1_clk
 10X: sys_synth_clk
 110: pll2_clk
 111: pll3_clk

Update sys_clk parent array accordingly (ex. 0:3-pll1_clk)

Signed-off-by: Vipul Kumar Samar <vipulkumar.samar@st.com>
---
 drivers/clk/spear/spear1340_clock.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/spear/spear1340_clock.c b/drivers/clk/spear/spear1340_clock.c
index 2cd5520..f927f90 100644
--- a/drivers/clk/spear/spear1340_clock.c
+++ b/drivers/clk/spear/spear1340_clock.c
@@ -369,8 +369,8 @@ static struct frac_rate_tbl gen_rtbl[] = {
 
 /* clock parents */
 static const char *vco_parents[] = { "osc_24m_clk", "osc_25m_clk", };
-static const char *sys_parents[] = { "none", "pll1_clk", "none", "none",
-	"sys_syn_clk", "none", "pll2_clk", "pll3_clk", };
+static const char *sys_parents[] = { "pll1_clk", "pll1_clk", "pll1_clk",
+	"pll1_clk", "sys_synth_clk", "sys_synth_clk", "pll2_clk", "pll3_clk", };
 static const char *ahb_parents[] = { "cpu_div3_clk", "amba_syn_clk", };
 static const char *gpt_parents[] = { "osc_24m_clk", "apb_clk", };
 static const char *uart0_parents[] = { "pll5_clk", "osc_24m_clk",
-- 
1.7.2.2

[PATCH V2 6/7] ARM: SPEAr13xx: Add auxdata for Ethernet controller.

[Vipul Kumar Samar]

Use AUXDATA to pass platform data for Ethernet controller.

Signed-off-by: Vipul Kumar Samar <vipulkumar.samar@st.com>
---
 arch/arm/mach-spear13xx/include/mach/generic.h |    2 +
 arch/arm/mach-spear13xx/include/mach/spear.h   |    1 +
 arch/arm/mach-spear13xx/spear1340.c            |   32 +++++++
 arch/arm/mach-spear13xx/spear13xx.c            |  104 ++++++++++++++++++++++++
 4 files changed, 139 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-spear13xx/include/mach/generic.h b/arch/arm/mach-spear13xx/include/mach/generic.h
index dac57fd..8c8fbaa 100644
--- a/arch/arm/mach-spear13xx/include/mach/generic.h
+++ b/arch/arm/mach-spear13xx/include/mach/generic.h
@@ -15,6 +15,7 @@
 #define __MACH_GENERIC_H
 
 #include <linux/dmaengine.h>
+#include <linux/platform_device.h>
 #include <asm/mach/time.h>
 
 /* Add spear13xx structure declarations here */
@@ -31,6 +32,7 @@ void __init spear13xx_map_io(void);
 void __init spear13xx_dt_init_irq(void);
 void __init spear13xx_l2x0_init(void);
 bool dw_dma_filter(struct dma_chan *chan, void *slave);
+int spear13xx_eth_phy_clk_cfg(struct platform_device *pdev);
 void spear_restart(char, const char *);
 void spear13xx_secondary_startup(void);
 
diff --git a/arch/arm/mach-spear13xx/include/mach/spear.h b/arch/arm/mach-spear13xx/include/mach/spear.h
index 65f27de..b0b6f91 100644
--- a/arch/arm/mach-spear13xx/include/mach/spear.h
+++ b/arch/arm/mach-spear13xx/include/mach/spear.h
@@ -46,6 +46,7 @@
 #define DMAC0_BASE				UL(0xEA800000)
 #define DMAC1_BASE				UL(0xEB000000)
 #define MCIF_CF_BASE				UL(0xB2800000)
+#define SPEAR13XX_GETH_BASE			UL(0xE2000000)
 
 /* Devices present in SPEAr1310 */
 #ifdef CONFIG_MACH_SPEAR1310
diff --git a/arch/arm/mach-spear13xx/spear1340.c b/arch/arm/mach-spear13xx/spear1340.c
index 81e4ed7..ab282ed 100644
--- a/arch/arm/mach-spear13xx/spear1340.c
+++ b/arch/arm/mach-spear13xx/spear1340.c
@@ -18,6 +18,9 @@
 #include <linux/delay.h>
 #include <linux/dw_dmac.h>
 #include <linux/of_platform.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+#include <linux/stmmac.h>
 #include <asm/hardware/gic.h>
 #include <asm/mach/arch.h>
 #include <mach/dma.h>
@@ -100,6 +103,34 @@ static struct amba_pl011_data uart1_data = {
 	.dma_rx_param = &uart1_dma_param[1],
 };
 
+/* Ethernet platform data */
+static struct stmmac_mdio_bus_data mdio0_private_data = {
+	.bus_id = 0,
+	.phy_mask = 0,
+};
+
+static struct stmmac_dma_cfg dma0_private_data = {
+	.pbl = 16,
+	.fixed_burst = 1,
+	.burst_len = DMA_AXI_BLEN_ALL,
+};
+
+static struct plat_stmmacenet_data eth_data = {
+	.bus_id = 0,
+	.phy_addr = -1,
+	.interface = PHY_INTERFACE_MODE_RGMII,
+	.has_gmac = 1,
+	.enh_desc = 1,
+	.tx_coe = 1,
+	.dma_cfg = &dma0_private_data,
+	.rx_coe = STMMAC_RX_COE_TYPE2,
+	.bugged_jumbo = 1,
+	.pmt = 1,
+	.mdio_bus_data = &mdio0_private_data,
+	.init = spear13xx_eth_phy_clk_cfg,
+	.clk_csr = STMMAC_CSR_150_250M,
+};
+
 /* SATA device registration */
 static int sata_miphy_init(struct device *dev, void __iomem *addr)
 {
@@ -166,6 +197,7 @@ static struct of_dev_auxdata spear1340_auxdata_lookup[] __initdata = {
 	OF_DEV_AUXDATA("snps,spear-ahci", SPEAR1340_SATA_BASE, NULL,
 			&sata_pdata),
 	OF_DEV_AUXDATA("arm,pl011", SPEAR1340_UART1_BASE, NULL, &uart1_data),
+	OF_DEV_AUXDATA("st,spear600-gmac", SPEAR13XX_GETH_BASE, NULL, &eth_data),
 	{}
 };
 
diff --git a/arch/arm/mach-spear13xx/spear13xx.c b/arch/arm/mach-spear13xx/spear13xx.c
index cf936b1..9ab3b47 100644
--- a/arch/arm/mach-spear13xx/spear13xx.c
+++ b/arch/arm/mach-spear13xx/spear13xx.c
@@ -18,6 +18,9 @@
 #include <linux/dw_dmac.h>
 #include <linux/err.h>
 #include <linux/of_irq.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+#include <linux/stmmac.h>
 #include <asm/hardware/cache-l2x0.h>
 #include <asm/hardware/gic.h>
 #include <asm/mach/map.h>
@@ -80,6 +83,107 @@ struct dw_dma_platform_data dmac_plat_data = {
 	.chan_priority = CHAN_PRIORITY_DESCENDING,
 };
 
+/* Ethernet clock initialization */
+int spear13xx_eth_phy_clk_cfg(struct platform_device *pdev)
+{
+	int ret;
+	struct clk *input_clk, *input_pclk, *phy_pclk, *phy_clk;
+	struct plat_stmmacenet_data *pdata = dev_get_platdata(&pdev->dev);
+	const char *phy_clk_src_name[] = {
+		"phy_input_mclk",
+		"phy_syn_gclk",
+	};
+	const char *input_clk_src_name[] = {
+		"pll2_clk",
+		"gmii_pad_clk",
+		"osc_25m_clk",
+	};
+	const char *phy_clk_name[] = {
+		"stmmacphy.0"
+	};
+
+	if (!pdata)
+		return -EINVAL;
+
+	/* Get the Pll-2 Clock as parent for PHY Input Clock Source */
+	input_pclk = clk_get(NULL, input_clk_src_name[0]);
+	if (IS_ERR(input_pclk)) {
+		ret = PTR_ERR(input_pclk);
+		goto fail_get_input_pclk;
+	}
+
+	/*
+	 * Get the Phy Input clock source as parent for Phy clock. Default
+	 * selection is gmac_phy_input_clk. This selection would be driving both
+	 * the synthesizer and phy clock.
+	 */
+	input_clk = clk_get(NULL, phy_clk_src_name[0]);
+	if (IS_ERR(input_clk)) {
+		ret = PTR_ERR(input_clk);
+		goto fail_get_input_clk;
+	}
+
+	/* Fetch the phy clock */
+	phy_clk = clk_get(NULL, phy_clk_name[pdata->bus_id]);
+	if (IS_ERR(phy_clk)) {
+		ret = PTR_ERR(phy_clk);
+		goto fail_get_phy_clk;
+	}
+
+	/* Set the pll-2 to 125 MHz */
+	ret = clk_set_rate(input_pclk, 125000000);
+	if (IS_ERR_VALUE(ret)) {
+		pr_err("%s:couldn't set rate for input phy clk\n", __func__);
+		goto fail_set_rate;
+	}
+
+	/* Set the Pll-2 as parent for gmac_phy_input_clk */
+	ret = clk_set_parent(input_clk, input_pclk);
+	if (IS_ERR_VALUE(ret)) {
+		pr_err("%s:couldn't set parent for inout phy clk \n", __func__);
+		goto fail_set_rate;
+	}
+	if (pdata->interface == PHY_INTERFACE_MODE_RMII) {
+		/*
+		 * For the rmii interface select gmac_phy_synth_clk
+		 * as the parent and set the clock to 50 Mhz
+		 */
+		phy_pclk = clk_get(NULL, phy_clk_src_name[1]);
+		ret = clk_set_rate(phy_pclk, 50000000);
+		if (IS_ERR_VALUE(ret)) {
+			pr_err("%s:couldn't set rate for phy synth clk\n",
+					__func__);
+			goto fail_set_rate;
+		}
+	} else {
+		/*
+		 * Set the gmac_phy_input_clk as the parent,
+		 * and pll-2 is already running as parent of
+		 * gmac_phy_input_clk at 125 Mhz
+		 */
+		phy_pclk = input_clk;
+	}
+
+	/* Select the parent for phy clock */
+	ret = clk_set_parent(phy_clk, phy_pclk);
+	if (IS_ERR_VALUE(ret)) {
+		pr_err("%s:couldn't set parent for phy clk \n", __func__);
+		goto fail_set_rate;
+	}
+
+	ret = clk_prepare_enable(phy_clk);
+
+	return ret;
+fail_set_rate:
+	clk_put(phy_clk);
+fail_get_phy_clk:
+	clk_put(input_clk);
+fail_get_input_clk:
+	clk_put(input_pclk);
+fail_get_input_pclk:
+	return ret;
+}
+
 void __init spear13xx_l2x0_init(void)
 {
 	/*
-- 
1.7.2.2

[PATCH V2 7/7] net: stmmac: Overwrite platform data if passed from auxdata

[Vipul Kumar Samar]

Platform data can be passed through either device tree or
auxdata_lookup. Device tree still donot have any means to provide
facility for passing platform callbacks to the driver.

If any platform data is available through auxdata_lookup then overwrite
the platform data passed through device tree.

Signed-off-by: Vipul Kumar Samar <vipulkumar.samar@st.com>
---
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 680d2b8..4651579 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -114,10 +114,11 @@ static int stmmac_pltfr_probe(struct platform_device *pdev)
 			pr_err("%s: main dt probe failed", __func__);
 			goto out_unmap;
 		}
-	} else {
-		plat_dat = pdev->dev.platform_data;
 	}
 
+	if (pdev->dev.platform_data)
+		plat_dat = pdev->dev.platform_data;
+
 	/* Custom initialisation (if needed)*/
 	if (plat_dat->init) {
 		ret = plat_dat->init(pdev);
-- 
1.7.2.2

[PATCH V2 7/7] net: stmmac: Overwrite platform data if passed from auxdata

[viresh kumar]

On Fri, Jul 13, 2012 at 10:23 AM, Vipul Kumar Samar
<vipulkumar.samar@st.com> wrote:
> Platform data can be passed through either device tree or
> auxdata_lookup. Device tree still donot have any means to provide
> facility for passing platform callbacks to the driver.
>
> If any platform data is available through auxdata_lookup then overwrite
> the platform data passed through device tree.
>
> Signed-off-by: Vipul Kumar Samar <vipulkumar.samar@st.com>

This one is part of wrong patchset, remove it from this one, when you send a
pull request to Arnd/Olof.

Send it to net dev list separately. Keep giuseppe and stefan Roese in CC.

> ---
>  .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 680d2b8..4651579 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -114,10 +114,11 @@ static int stmmac_pltfr_probe(struct platform_device *pdev)
>                         pr_err("%s: main dt probe failed", __func__);
>                         goto out_unmap;
>                 }
> -       } else {
> -               plat_dat = pdev->dev.platform_data;
>         }
>
> +       if (pdev->dev.platform_data)
> +               plat_dat = pdev->dev.platform_data;
> +

Even this looks wrong. You can't simply override plat_dat like this.
Better to add properties to stmmac driver, that you can pass via DT.

That's what Stefan also mentioned in his original patch:

static int __devinit stmmac_probe_config_dt(struct platform_device *pdev,
					    struct plat_stmmacenet_data *plat,
					    const char **mac)
{
	/*
	 * Currently only the properties needed on SPEAr600
	 * are provided. All other properties should be added
	 * once needed on other platforms.
	 */

--
viresh

[PATCH V2 5/7] Clk: SPEAr1340: Update sys clock parent array

[viresh kumar]

On Fri, Jul 13, 2012 at 10:23 AM, Vipul Kumar Samar
<vipulkumar.samar@st.com> wrote:
> SYS_CLK have multiple parents and selection of parent depends on sys_clk_ctrl
> register bit no. 23:25.
>  0XX: pll1_clk
>  10X: sys_synth_clk
>  110: pll2_clk
>  111: pll3_clk
>
> Update sys_clk parent array accordingly (ex. 0:3-pll1_clk)
>
> Signed-off-by: Vipul Kumar Samar <vipulkumar.samar@st.com>
> ---
>  drivers/clk/spear/spear1340_clock.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/spear/spear1340_clock.c b/drivers/clk/spear/spear1340_clock.c
> index 2cd5520..f927f90 100644
> --- a/drivers/clk/spear/spear1340_clock.c
> +++ b/drivers/clk/spear/spear1340_clock.c
> @@ -369,8 +369,8 @@ static struct frac_rate_tbl gen_rtbl[] = {
>
>  /* clock parents */
>  static const char *vco_parents[] = { "osc_24m_clk", "osc_25m_clk", };
> -static const char *sys_parents[] = { "none", "pll1_clk", "none", "none",
> -       "sys_syn_clk", "none", "pll2_clk", "pll3_clk", };
> +static const char *sys_parents[] = { "pll1_clk", "pll1_clk", "pll1_clk",
> +       "pll1_clk", "sys_synth_clk", "sys_synth_clk", "pll2_clk", "pll3_clk", };

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

[PATCH V2 6/7] ARM: SPEAr13xx: Add auxdata for Ethernet controller.

[viresh kumar]

On Fri, Jul 13, 2012 at 10:23 AM, Vipul Kumar Samar
<vipulkumar.samar@st.com> wrote:
> Use AUXDATA to pass platform data for Ethernet controller.

> diff --git a/arch/arm/mach-spear13xx/spear1340.c b/arch/arm/mach-spear13xx/spear1340.c

> +static struct plat_stmmacenet_data eth_data = {
> +       .bus_id = 0,
> +       .phy_addr = -1,
> +       .interface = PHY_INTERFACE_MODE_RGMII,
> +       .has_gmac = 1,
> +       .enh_desc = 1,
> +       .tx_coe = 1,
> +       .dma_cfg = &dma0_private_data,
> +       .rx_coe = STMMAC_RX_COE_TYPE2,
> +       .bugged_jumbo = 1,
> +       .pmt = 1,
> +       .mdio_bus_data = &mdio0_private_data,
> +       .init = spear13xx_eth_phy_clk_cfg,
> +       .clk_csr = STMMAC_CSR_150_250M,
> +};
> +
>  /* SATA device registration */
>  static int sata_miphy_init(struct device *dev, void __iomem *addr)
>  {
> @@ -166,6 +197,7 @@ static struct of_dev_auxdata spear1340_auxdata_lookup[] __initdata = {
>         OF_DEV_AUXDATA("snps,spear-ahci", SPEAR1340_SATA_BASE, NULL,
>                         &sata_pdata),
>         OF_DEV_AUXDATA("arm,pl011", SPEAR1340_UART1_BASE, NULL, &uart1_data),
> +       OF_DEV_AUXDATA("st,spear600-gmac", SPEAR13XX_GETH_BASE, NULL, &eth_data),
>         {}
>  };

Adding Stefan and Peppe.

I understand why you can't send all platform data from DT.
Let me elaborate the problem statement

stmmac is used by platforms with and without DT.
- Without DT will pass platform data directly, without any issues.
- With DT have to pass all data, some of that via DT and other without
DT, like routines
  (atleast for now)

For now what I suggest is, update DT support for whatever we can..
i.e. support Maximum
properties there. As finally we will support everything via DT, no
platform data.

Whatever is left, that can't be passed via DT, like routine, pass it
from platform data
and merge both these versions of platform data in driver, keeping DT
ones in priority.

i.e. Whatever is defined in DT properties must come from there and
left outs from
platform data.

--
viresh

[PATCH V2 7/7] net: stmmac: Overwrite platform data if passed from auxdata

[vipul kumar samar]

On 7/13/2012 3:47 PM, viresh kumar wrote:
> On Fri, Jul 13, 2012 at 10:23 AM, Vipul Kumar Samar
> <vipulkumar.samar@st.com>  wrote:
>> Platform data can be passed through either device tree or
>> auxdata_lookup. Device tree still donot have any means to provide
>> facility for passing platform callbacks to the driver.
>>
>> If any platform data is available through auxdata_lookup then overwrite
>> the platform data passed through device tree.
>>
>> Signed-off-by: Vipul Kumar Samar<vipulkumar.samar@st.com>
>
> This one is part of wrong patchset, remove it from this one, when you send a
> pull request to Arnd/Olof.
>
> Send it to net dev list separately. Keep giuseppe and stefan Roese in CC.
>
>> ---
>>   .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |    5 +++--
>>   1 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> index 680d2b8..4651579 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> @@ -114,10 +114,11 @@ static int stmmac_pltfr_probe(struct platform_device *pdev)
>>                          pr_err("%s: main dt probe failed", __func__);
>>                          goto out_unmap;
>>                  }
>> -       } else {
>> -               plat_dat = pdev->dev.platform_data;
>>          }
>>
>> +       if (pdev->dev.platform_data)
>> +               plat_dat = pdev->dev.platform_data;
>> +
>
> Even this looks wrong. You can't simply override plat_dat like this.
> Better to add properties to stmmac driver, that you can pass via DT.
>
> That's what Stefan also mentioned in his original patch:
>
> static int __devinit stmmac_probe_config_dt(struct platform_device *pdev,
> 					    struct plat_stmmacenet_data *plat,
> 					    const char **mac)
> {
> 	/*
> 	 * Currently only the properties needed on SPEAr600
> 	 * are provided. All other properties should be added
> 	 * once needed on other platforms.
> 	 */
>

Correct me if i am wrong, We cant pass function pointer from device 
tree. For that we need to pass platdata it from auxdata_lookup.

In case of stmmac, phy is configure through init function passed in 
platform data. Any other way to pass function pointer in device tree??

Regards
Vipul Samar

[PATCH V2 7/7] net: stmmac: Overwrite platform data if passed from auxdata

[viresh kumar]

On Fri, Jul 13, 2012 at 11:33 AM, vipul kumar samar
<vipulkumar.samar@st.com> wrote:
> Correct me if i am wrong, We cant pass function pointer from device tree.
> For that we need to pass platdata it from auxdata_lookup.
>
> In case of stmmac, phy is configure through init function passed in platform
> data. Any other way to pass function pointer in device tree??

Hope, you have already gone through my comments for 6/7.

[PATCH V2 6/7] ARM: SPEAr13xx: Add auxdata for Ethernet controller.

[Arnd Bergmann]

On Friday 13 July 2012, viresh kumar wrote:
> Adding Stefan and Peppe.
> 
> I understand why you can't send all platform data from DT.
> Let me elaborate the problem statement
> 
> stmmac is used by platforms with and without DT.
> - Without DT will pass platform data directly, without any issues.
> - With DT have to pass all data, some of that via DT and other without
> DT, like routines
>   (atleast for now)
> 
> For now what I suggest is, update DT support for whatever we can..
> i.e. support Maximum
> properties there. As finally we will support everything via DT, no
> platform data.
> 
> Whatever is left, that can't be passed via DT, like routine, pass it
> from platform data
> and merge both these versions of platform data in driver, keeping DT
> ones in priority.
> 
> i.e. Whatever is defined in DT properties must come from there and
> left outs from
> platform data.

Absolutely agreed.

The one part I think you both have wrong is the idea that the
spear13xx_eth_phy_clk_cfg function is needed.

I believe the correct answer for this is to introduce a driver for
the phy in drivers/net/phy/, and have the phy be described correctly
in the device tree and referenced found using the of_phy_find_device()
function.

	Arnd

[PATCH V2 6/7] ARM: SPEAr13xx: Add auxdata for Ethernet controller.

[Jean-Christophe PLAGNIOL-VILLARD]

On 14:53 Fri 13 Jul     , Vipul Kumar Samar wrote:
> Use AUXDATA to pass platform data for Ethernet controller.
> 
> Signed-off-by: Vipul Kumar Samar <vipulkumar.samar@st.com>
> ---
>  arch/arm/mach-spear13xx/include/mach/generic.h |    2 +
>  arch/arm/mach-spear13xx/include/mach/spear.h   |    1 +
>  arch/arm/mach-spear13xx/spear1340.c            |   32 +++++++
>  arch/arm/mach-spear13xx/spear13xx.c            |  104 ++++++++++++++++++++++++
>  4 files changed, 139 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-spear13xx/include/mach/generic.h b/arch/arm/mach-spear13xx/include/mach/generic.h
> index dac57fd..8c8fbaa 100644
> --- a/arch/arm/mach-spear13xx/include/mach/generic.h
> +++ b/arch/arm/mach-spear13xx/include/mach/generic.h
> @@ -15,6 +15,7 @@
>  #define __MACH_GENERIC_H
>  
>  #include <linux/dmaengine.h>
> +#include <linux/platform_device.h>
>  #include <asm/mach/time.h>
>  
>  /* Add spear13xx structure declarations here */
> @@ -31,6 +32,7 @@ void __init spear13xx_map_io(void);
>  void __init spear13xx_dt_init_irq(void);
>  void __init spear13xx_l2x0_init(void);
>  bool dw_dma_filter(struct dma_chan *chan, void *slave);
> +int spear13xx_eth_phy_clk_cfg(struct platform_device *pdev);
>  void spear_restart(char, const char *);
>  void spear13xx_secondary_startup(void);
>  
> diff --git a/arch/arm/mach-spear13xx/include/mach/spear.h b/arch/arm/mach-spear13xx/include/mach/spear.h
> index 65f27de..b0b6f91 100644
> --- a/arch/arm/mach-spear13xx/include/mach/spear.h
> +++ b/arch/arm/mach-spear13xx/include/mach/spear.h
> @@ -46,6 +46,7 @@
>  #define DMAC0_BASE				UL(0xEA800000)
>  #define DMAC1_BASE				UL(0xEB000000)
>  #define MCIF_CF_BASE				UL(0xB2800000)
> +#define SPEAR13XX_GETH_BASE			UL(0xE2000000)
>  
>  /* Devices present in SPEAr1310 */
>  #ifdef CONFIG_MACH_SPEAR1310
> diff --git a/arch/arm/mach-spear13xx/spear1340.c b/arch/arm/mach-spear13xx/spear1340.c
> index 81e4ed7..ab282ed 100644
> --- a/arch/arm/mach-spear13xx/spear1340.c
> +++ b/arch/arm/mach-spear13xx/spear1340.c
> @@ -18,6 +18,9 @@
>  #include <linux/delay.h>
>  #include <linux/dw_dmac.h>
>  #include <linux/of_platform.h>
> +#include <linux/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/stmmac.h>
>  #include <asm/hardware/gic.h>
>  #include <asm/mach/arch.h>
>  #include <mach/dma.h>
> @@ -100,6 +103,34 @@ static struct amba_pl011_data uart1_data = {
>  	.dma_rx_param = &uart1_dma_param[1],
>  };
>  
> +/* Ethernet platform data */
> +static struct stmmac_mdio_bus_data mdio0_private_data = {
> +	.bus_id = 0,
> +	.phy_mask = 0,
> +};
> +
> +static struct stmmac_dma_cfg dma0_private_data = {
> +	.pbl = 16,
> +	.fixed_burst = 1,
> +	.burst_len = DMA_AXI_BLEN_ALL,
> +};
> +
> +static struct plat_stmmacenet_data eth_data = {
> +	.bus_id = 0,
> +	.phy_addr = -1,
> +	.interface = PHY_INTERFACE_MODE_RGMII,
> +	.has_gmac = 1,
> +	.enh_desc = 1,
> +	.tx_coe = 1,
> +	.dma_cfg = &dma0_private_data,
> +	.rx_coe = STMMAC_RX_COE_TYPE2,
> +	.bugged_jumbo = 1,
> +	.pmt = 1,
> +	.mdio_bus_data = &mdio0_private_data,
> +	.init = spear13xx_eth_phy_clk_cfg,
> +	.clk_csr = STMMAC_CSR_150_250M,
> +};
sorry I see no reason to do not pass this via DT

I agreed that the pbl is quite sensitive but you do need to pass them via DT

Best Regards,
J.

[PATCH V2 6/7] ARM: SPEAr13xx: Add auxdata for Ethernet controller.

[deepaksi]

Viresh,



On 7/13/2012 4:00 PM, viresh kumar wrote:
> On Fri, Jul 13, 2012 at 10:23 AM, Vipul Kumar Samar
> <vipulkumar.samar@st.com>  wrote:
>> Use AUXDATA to pass platform data for Ethernet controller.
> Adding Stefan and Peppe.
>
> I understand why you can't send all platform data from DT.
> Let me elaborate the problem statement
>
> stmmac is used by platforms with and without DT.
> - Without DT will pass platform data directly, without any issues.
> - With DT have to pass all data, some of that via DT and other without
> DT, like routines
>    (atleast for now)
>
> For now what I suggest is, update DT support for whatever we can..
> i.e. support Maximum
> properties there. As finally we will support everything via DT, no
> platform data.
>
> Whatever is left, that can't be passed via DT, like routine, pass it
> from platform data
> and merge both these versions of platform data in driver, keeping DT
> ones in priority.
>
> i.e. Whatever is defined in DT properties must come from there and
> left outs from
> platform data.

I do differ on the point over here. I do believe that the code for now 
should be left as into mutually
exclusive sections.
As you said, without DT, the platform data would exist without any 
problem, Thats fine

But with DT also, as of now since the DT is still evolving we should not 
merge the data and keep it at
two places. The reasons being.
The stmmac driver is being used by multiple platforms, lets say within 
spear we have different variants
requiring different configurations and dividing those configurations at 
two different places will require larger
maintenance.Any more changes in driver that has dependency on the 
platform data will require updates,
and more such conflicts will arrive related to maintenance.

Lets keep the platform data as a part of AUXDATA for now, till the tree 
evolves fully specifically if DT is being
used.

Deepak




> --
> viresh
> .
>

[PATCH V2 6/7] ARM: SPEAr13xx: Add auxdata for Ethernet controller.

[deepaksi]

Hi Arnd,


On 7/13/2012 7:52 PM, Arnd Bergmann wrote:
> On Friday 13 July 2012, viresh kumar wrote:
>> Adding Stefan and Peppe.
>>
>> I understand why you can't send all platform data from DT.
>> Let me elaborate the problem statement
>>
>> stmmac is used by platforms with and without DT.
>> - Without DT will pass platform data directly, without any issues.
>> - With DT have to pass all data, some of that via DT and other without
>> DT, like routines
>>    (atleast for now)
>>
>> For now what I suggest is, update DT support for whatever we can..
>> i.e. support Maximum
>> properties there. As finally we will support everything via DT, no
>> platform data.
>>
>> Whatever is left, that can't be passed via DT, like routine, pass it
>> from platform data
>> and merge both these versions of platform data in driver, keeping DT
>> ones in priority.
>>
>> i.e. Whatever is defined in DT properties must come from there and
>> left outs from
>> platform data.
> Absolutely agreed.
>
> The one part I think you both have wrong is the idea that the
> spear13xx_eth_phy_clk_cfg function is needed.
>
> I believe the correct answer for this is to introduce a driver for
> the phy in drivers/net/phy/, and have the phy be described correctly
> in the device tree and referenced found using the of_phy_find_device()
> function.

SPEAr platform has been using the generic phy framework, and it just 
requires a register into mdio
bus and provide the necessary callback w.r.t mdio read and write. These 
mdio read and write are more
into the GMAC registers, and hence a part of stmmac driver.

As far as my understanding is concerned, addition to the phy framework 
could have been done had it been
some phy from stmicro, which requires some special addressing to be done.
The function we are talking about is more related to handling the 
interface that phy has. The stmmac core has
been configured as rmii/smii/rgmii and requires the system to generate 
corresponding clock to support data
transfers over PHY.
This was specifically the reason of aligning the phy clock configuration 
function with stmmac driver, and could
well be taken as functional interface clock for mac core.
I do think that we should not add a new driver for aligning the clock 
settings of the interfaces.


Regards
Deepak



>
> 	Arnd
> .
>

[PATCH V2 6/7] ARM: SPEAr13xx: Add auxdata for Ethernet controller.

[Jean-Christophe PLAGNIOL-VILLARD]

On 15:55 Tue 17 Jul     , deepaksi wrote:
> Hi Arnd,
> 
> 
> On 7/13/2012 7:52 PM, Arnd Bergmann wrote:
> >On Friday 13 July 2012, viresh kumar wrote:
> >>Adding Stefan and Peppe.
> >>
> >>I understand why you can't send all platform data from DT.
> >>Let me elaborate the problem statement
> >>
> >>stmmac is used by platforms with and without DT.
> >>- Without DT will pass platform data directly, without any issues.
> >>- With DT have to pass all data, some of that via DT and other without
> >>DT, like routines
> >>   (atleast for now)
> >>
> >>For now what I suggest is, update DT support for whatever we can..
> >>i.e. support Maximum
> >>properties there. As finally we will support everything via DT, no
> >>platform data.
> >>
> >>Whatever is left, that can't be passed via DT, like routine, pass it
> >>from platform data
> >>and merge both these versions of platform data in driver, keeping DT
> >>ones in priority.
> >>
> >>i.e. Whatever is defined in DT properties must come from there and
> >>left outs from
> >>platform data.
> >Absolutely agreed.
> >
> >The one part I think you both have wrong is the idea that the
> >spear13xx_eth_phy_clk_cfg function is needed.
> >
> >I believe the correct answer for this is to introduce a driver for
> >the phy in drivers/net/phy/, and have the phy be described correctly
> >in the device tree and referenced found using the of_phy_find_device()
> >function.
> 
> SPEAr platform has been using the generic phy framework, and it just
> requires a register into mdio
> bus and provide the necessary callback w.r.t mdio read and write.
> These mdio read and write are more
> into the GMAC registers, and hence a part of stmmac driver.
> 
> As far as my understanding is concerned, addition to the phy
> framework could have been done had it been
> some phy from stmicro, which requires some special addressing to be done.
> The function we are talking about is more related to handling the
> interface that phy has. The stmmac core has
> been configured as rmii/smii/rgmii and requires the system to
> generate corresponding clock to support data
> transfers over PHY.
> This was specifically the reason of aligning the phy clock
> configuration function with stmmac driver, and could
> well be taken as functional interface clock for mac core.
> I do think that we should not add a new driver for aligning the
> clock settings of the interfaces.
this is not spear related but IP related this need to move the stmmac

IIRC I see it on other ST SoC

Best Regards,
J.

[PATCH V2 6/7] ARM: SPEAr13xx: Add auxdata for Ethernet controller.

[Arnd Bergmann]

On Tuesday 17 July 2012, deepaksi wrote:
> I do differ on the point over here. I do believe that the code for now 
> should be left as into mutually
> exclusive sections.
> As you said, without DT, the platform data would exist without any 
> problem, Thats fine
> 
> But with DT also, as of now since the DT is still evolving we should not 
> merge the data and keep it at
> two places. The reasons being.
> The stmmac driver is being used by multiple platforms, lets say within 
> spear we have different variants
> requiring different configurations and dividing those configurations at 
> two different places will require larger
> maintenance.

I don't think that the STMMAC_PLATFORM part is used by any other
platform in the mainline kernel, so we are definitely free to
rip out (parts of) the platform_data and replace them with DT
properties.
There is also no platform defining plat_stmmacenet_data
yet, which means that the code is completely untested at
the moment.

Don't worry about any out-of-tree platforms, they can keep
their out of tree patches to add back the platform data if
they don't want to move to DT booting.

Since all the data you are adding to spear1340.c is constant
anyway, I suppose that means this data is specific to
the spear1340 soc, not to a particular board or configuration.
I would suggest you add a preset like

static const struct of_device_id stmmac_dt_ids[] = {
        { .compatible = "st,spear600-gmac", .data = &spear600_data},
        { .compatible = "st,spear1340-gmac", .data = &spear1340_data}
};

that contains all the soc-specific data. You can probably make
that "const" and just kill off the platform_data path in the
driver.

> Any more changes in driver that has dependency on the 
> platform data will require updates,
> and more such conflicts will arrive related to maintenance.
> 
> Lets keep the platform data as a part of AUXDATA for now, till the tree 
> evolves fully specifically if DT is being
> used.

SPEAr is already fully using DT for everything except DMA channels.
Please don't add any more such exceptions.

	Arnd

[PATCH V2 6/7] ARM: SPEAr13xx: Add auxdata for Ethernet controller.

[deepaksi]

Hi Arnd,

On 7/17/2012 10:23 PM, Arnd Bergmann wrote:
> On Tuesday 17 July 2012, deepaksi wrote:
>> I do differ on the point over here. I do believe that the code for now
>> should be left as into mutually
>> exclusive sections.
>> As you said, without DT, the platform data would exist without any
>> problem, Thats fine
>>
>> But with DT also, as of now since the DT is still evolving we should not
>> merge the data and keep it at
>> two places. The reasons being.
>> The stmmac driver is being used by multiple platforms, lets say within
>> spear we have different variants
>> requiring different configurations and dividing those configurations at
>> two different places will require larger
>> maintenance.
> I don't think that the STMMAC_PLATFORM part is used by any other
> platform in the mainline kernel, so we are definitely free to
> rip out (parts of) the platform_data and replace them with DT
> properties.
> There is also no platform defining plat_stmmacenet_data
> yet, which means that the code is completely untested at
> the moment.
>
> Don't worry about any out-of-tree platforms, they can keep
> their out of tree patches to add back the platform data if
> they don't want to move to DT booting.
>
> Since all the data you are adding to spear1340.c is constant
> anyway, I suppose that means this data is specific to
> the spear1340 soc, not to a particular board or configuration.
> I would suggest you add a preset like
>
> static const struct of_device_id stmmac_dt_ids[] = {
>          { .compatible = "st,spear600-gmac", .data =&spear600_data},
>          { .compatible = "st,spear1340-gmac", .data =&spear1340_data}
> };
>
> that contains all the soc-specific data. You can probably make
> that "const" and just kill off the platform_data path in the
> driver.

Sure, we will do it as per the suggestions.

Regards
Deepak

>> Any more changes in driver that has dependency on the
>> platform data will require updates,
>> and more such conflicts will arrive related to maintenance.
>>
>> Lets keep the platform data as a part of AUXDATA for now, till the tree
>> evolves fully specifically if DT is being
>> used.
> SPEAr is already fully using DT for everything except DMA channels.
> Please don't add any more such exceptions.
>
> 	Arnd
> .
>

[PATCH V2 6/7] ARM: SPEAr13xx: Add auxdata for Ethernet controller.

[deepaksi]

Hi Arnd,

>>>
>> I don't think that the STMMAC_PLATFORM part is used by any other
>> platform in the mainline kernel, so we are definitely free to
>> rip out (parts of) the platform_data and replace them with DT
>> properties.
>> There is also no platform defining plat_stmmacenet_data
>> yet, which means that the code is completely untested at
>> the moment.
>>
>> Don't worry about any out-of-tree platforms, they can keep
>> their out of tree patches to add back the platform data if
>> they don't want to move to DT booting.
>>
>> Since all the data you are adding to spear1340.c is constant
>> anyway, I suppose that means this data is specific to
>> the spear1340 soc, not to a particular board or configuration.
>> I would suggest you add a preset like
>>
>> static const struct of_device_id stmmac_dt_ids[] = {
>>          { .compatible = "st,spear600-gmac", .data =&spear600_data},
>>          { .compatible = "st,spear1340-gmac", .data =&spear1340_data}
>> };
>>
>> that contains all the soc-specific data. You can probably make
>> that "const" and just kill off the platform_data path in the
>> driver.
>

I am sorry but bringing alive this discussion once again, as we have 
some implementation
specific open points.

1. In case we go by the above proposal, it kind of solves a few things 
for us, such as

- Through the DT blob it was difficult to plug in some of the platform 
specific callbacks
which kind of did few things that driver needed before it could be 
operational. In SPEAr
architecture we have a miscellaneous space which provides SOC specific
initializations used by devices which can be handled by these callbacks.

Now with the approach suggested, we have the flexibility to add on these 
callbacks,
where DT is not sufficient and move further.

*But there are few catches/ query I have with me.*

1. Within the driver space now, even if it is a stmmac_platform.c file 
we are kind of adding
a non driver related miscellaneous space (SoC specific) to do some 
initializations.
Will that be fine to do all such initializations in driver ?

2. If yes, then it is kind of moving all the platform related 
initializations into driver, and these
could be varying across machines, for example with SPEAr, 6xx /3xx/13xx 
may all have
different callbacks; and then take it to next level when the stmmac 
driver would be shared
across the platforms, and all initializations will plug into this file.
It sounds more of collaborating of all the ethernet driver information 
which previously existed
across platforms into one area, and making the driver space untidy.
I would seek your opinion and suggestions on this.

Regards
Deepak

[PATCH V2 6/7] ARM: SPEAr13xx: Add auxdata for Ethernet controller.

[Arnd Bergmann]

On Wednesday 25 July 2012, deepaksi wrote:

> *But there are few catches/ query I have with me.*
> 
> 1. Within the driver space now, even if it is a stmmac_platform.c file 
> we are kind of adding
> a non driver related miscellaneous space (SoC specific) to do some 
> initializations.
> Will that be fine to do all such initializations in driver ?

It depends a lot on what you want to do with those callbacks.
We generally try hard to make those callbacks unnecessary, but
I agree that this is not always easy or obvious how to do it.

There are cases where you have one SoC doing something very
obscure with an otherwise generic piece of hardware, and in
that case, it may be better to keep a callback in platform
code and use some method (auxdata or a notifier) to hand a
pointer to the driver.

In other cases, you have a number of platforms that essentially
want to do the same thing (set up clocks or voltages, pick a
PHY, ...) and in that case you should not be using callback
functions at all but instead describe the differences in the
device tree using the bindings for whatever you want to do
(adding bindings if necessary).

You can also have a case where the callback is just doing
something that is part of that driver in different ways,
and in that case using the "compatible" property to determine
which way is right is the best option.

Can you post here the complete set of callback implementations
that you have in your development trees at the moment? That should
help us determine which case we're talking about here.

	Arnd

[PATCH V2 6/7] ARM: SPEAr13xx: Add auxdata for Ethernet controller.

[Shiraz Hashim]

Hi Arnd,

On Wed, Jul 25, 2012 at 06:31:53AM +0000, Arnd Bergmann wrote:
> Can you post here the complete set of callback implementations
> that you have in your development trees at the moment? That should
> help us determine which case we're talking about here.

There are following types of callbacks (involving SoC/platform) seen
generally in our case,

  * parent clk selection -
        driver is not aware about various clock sources (which also
        varies from platform to platform) and hence can only be
        programmed by corresponding platform.

        This is the case in Ethernet, audio, etc.

  * spi controller
        This is little hack in the h/w. As part of s/w controlled chip
        select/deselect in spi-pl022, we need to write to some system
        specific register.

  * OTG controller
        phy clock initialization which involves series of steps
        including powering down, etc.

  * NAND controller
        bank selection on runtime by writing to system registers

  * sata controller
        Similar to OTG case which involves clock initialization.

--
regards
Shiraz

[PATCH V2 6/7] ARM: SPEAr13xx: Add auxdata for Ethernet controller.

[Arnd Bergmann]

On Wednesday 25 July 2012, Shiraz Hashim wrote:
> 
> On Wed, Jul 25, 2012 at 06:31:53AM +0000, Arnd Bergmann wrote:
> > Can you post here the complete set of callback implementations
> > that you have in your development trees at the moment? That should
> > help us determine which case we're talking about here.
> 
> There are following types of callbacks (involving SoC/platform) seen
> generally in our case,
> 
>   * parent clk selection -
>         driver is not aware about various clock sources (which also
>         varies from platform to platform) and hence can only be
>         programmed by corresponding platform.
> 
>         This is the case in Ethernet, audio, etc.
>
>   * sata controller
>         Similar to OTG case which involves clock initialization.
 
This should definitely be moved into the drivers. A lot of drivers
enable and program clocks using the generic clock interfaces, so
there is no point using a callback.

Note that since stmmac is an architecture independent driver, this
depends on Mark Brown's patch to provide empty stubs for
the clock management functions on architectures that don't yet
use it.

>   * spi controller
>         This is little hack in the h/w. As part of s/w controlled chip
>         select/deselect in spi-pl022, we need to write to some system
>         specific register.
> 
>   * OTG controller
>         phy clock initialization which involves series of steps
>         including powering down, etc.
> 
>   * NAND controller
>         bank selection on runtime by writing to system registers

I don't understand any of these, so unless you post the code
that you want help with as I asked above, I'll assume that you find
the solution yourself and don't need a callback ;-)

	Arnd

[PATCH V2 6/7] ARM: SPEAr13xx: Add auxdata for Ethernet controller.

[Shiraz Hashim]

Hi Arnd,

On Wed, Jul 25, 2012 at 05:10:53PM +0000, Arnd Bergmann wrote:
> On Wednesday 25 July 2012, Shiraz Hashim wrote:
> > On Wed, Jul 25, 2012 at 06:31:53AM +0000, Arnd Bergmann wrote:
> > > Can you post here the complete set of callback implementations
> > > that you have in your development trees at the moment? That should
> > > help us determine which case we're talking about here.
> > 
> > There are following types of callbacks (involving SoC/platform) seen
> > generally in our case,
> > 
> >   * parent clk selection -
> >         driver is not aware about various clock sources (which also
> >         varies from platform to platform) and hence can only be
> >         programmed by corresponding platform.
> > 
> >         This is the case in Ethernet, audio, etc.
> >
> >   * sata controller
> >         Similar to OTG case which involves clock initialization.
>  
> This should definitely be moved into the drivers. A lot of drivers
> enable and program clocks using the generic clock interfaces, so
> there is no point using a callback.
>
> Note that since stmmac is an architecture independent driver, this
> depends on Mark Brown's patch to provide empty stubs for
> the clock management functions on architectures that don't yet
> use it.

Yes, this is true for clocks associated with devices and we do handle
that in this way.

I was talking more of cases where, we need to

   * select clock sources (parents) about which driver has no
     knowledge and may vary across boards.
   * perform initializations which are more than clock, like phy
     initialization/programming etc. This is SoC dependant.

> >   * spi controller
> >         This is little hack in the h/w. As part of s/w controlled chip
> >         select/deselect in spi-pl022, we need to write to some system
> >         specific register.
> > 
> >   * OTG controller
> >         phy clock initialization which involves series of steps
> >         including powering down, etc.
> > 
> >   * NAND controller
> >         bank selection on runtime by writing to system registers
> 
> I don't understand any of these, so unless you post the code
> that you want help with as I asked above, I'll assume that you find
> the solution yourself and don't need a callback ;-)

Some of them are,

- fsmc NAND controller 

        -- include/linux/mtd/fsmc.h

        struct fsmc_nand_platform_data {
                ...

                void (*select_bank)(uint32_t bank, uint32_t busw);

                ...

        };


        -- arch/arm/mach-spear13xx/spear13xx.c

        void nand_select_bank(u32 bank, u32 busw)
        {
                u32 fsmc_cfg;

                if (cpu_is_spear1340()) {
        #ifdef CONFIG_CPU_SPEAR1340
                        fsmc_cfg = readl(VA_SPEAR1340_FSMC_CFG);
        #endif
                } else 
                        fsmc_cfg = readl(VA_FSMC_CFG);

                fsmc_cfg &= ~(NAND_BANK_MASK << NAND_BANK_SHIFT);
                fsmc_cfg |= (bank << NAND_BANK_SHIFT);

                if (busw)
                        fsmc_cfg |= 1 << NAND_DEV_WIDTH16;
                else
                        fsmc_cfg &= ~(1 << NAND_DEV_WIDTH16);

                if (cpu_is_spear1340()) {
        #ifdef CONFIG_CPU_SPEAR1340
                        writel(fsmc_cfg, VA_SPEAR1340_FSMC_CFG);
        #endif
                } else
                        writel(fsmc_cfg, VA_FSMC_CFG);
        }


- SATA Controller

        -- include/linux/ahci_platform.h

        struct ahci_platform_data {
                int (*init)(struct device *dev, void __iomem *addr);
                void (*exit)(struct device *dev);
                const struct ata_port_info *ata_port_info;
                unsigned int force_port_map;
                unsigned int mask_port_map;
        };


        -- arch/arm/mach-spear13xx/spear1340.c

        void sata_miphy_exit(struct device *dev)
        {
                writel(0, VA_SPEAR1340_PCIE_SATA_CFG);
                writel(0, VA_SPEAR1340_PCIE_MIPHY_CFG);

                /* Enable PCIE SATA Controller reset */
                writel((readl(VA_SPEAR1340_PERIP1_SW_RST) | (0x1000)),
                                VA_SPEAR1340_PERIP1_SW_RST);
                msleep(20);

                /* Switch off sata power domain */
                writel((readl(VA_SPEAR1340_PCM_CFG) & (~0x800)),
                                VA_SPEAR1340_PCM_CFG);
                msleep(20);
        }

        static int sata_miphy_init(struct device *dev, void __iomem *addr)
        {
                writel(SPEAR1340_SATA_CFG_VAL, VA_SPEAR1340_PCIE_SATA_CFG);
                writel(SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA_25M_CRYSTAL_CLK,
                                VA_SPEAR1340_PCIE_MIPHY_CFG);
                /* Switch on sata power domain */
                writel((readl(VA_SPEAR1340_PCM_CFG) | (0x800)),
                                VA_SPEAR1340_PCM_CFG);
                msleep(20);
                /* Disable PCIE SATA Controller reset */
                writel((readl(VA_SPEAR1340_PERIP1_SW_RST) & (~0x1000)),
                                VA_SPEAR1340_PERIP1_SW_RST);
                msleep(20);

                return 0;
        }

        static struct ahci_platform_data sata_pdata = {
                .init = sata_miphy_init,
                .exit = sata_miphy_exit,
        };

--
regards
Shiraz

[PATCH V2 6/7] ARM: SPEAr13xx: Add auxdata for Ethernet controller.

[Arnd Bergmann]

On Thursday 26 July 2012, Shiraz Hashim wrote:
> Hi Arnd,
> 
> On Wed, Jul 25, 2012 at 05:10:53PM +0000, Arnd Bergmann wrote:
> > On Wednesday 25 July 2012, Shiraz Hashim wrote:
> > > On Wed, Jul 25, 2012 at 06:31:53AM +0000, Arnd Bergmann wrote:
> > > > Can you post here the complete set of callback implementations
> > > > that you have in your development trees at the moment? That should
> > > > help us determine which case we're talking about here.
> > > 
> > > There are following types of callbacks (involving SoC/platform) seen
> > > generally in our case,
> > > 
> > >   * parent clk selection -
> > >         driver is not aware about various clock sources (which also
> > >         varies from platform to platform) and hence can only be
> > >         programmed by corresponding platform.
> > > 
> > >         This is the case in Ethernet, audio, etc.
> > >
> > >   * sata controller
> > >         Similar to OTG case which involves clock initialization.
> >  
> > This should definitely be moved into the drivers. A lot of drivers
> > enable and program clocks using the generic clock interfaces, so
> > there is no point using a callback.
> >
> > Note that since stmmac is an architecture independent driver, this
> > depends on Mark Brown's patch to provide empty stubs for
> > the clock management functions on architectures that don't yet
> > use it.
> 
> Yes, this is true for clocks associated with devices and we do handle
> that in this way.
> 
> I was talking more of cases where, we need to
> 
>    * select clock sources (parents) about which driver has no
>      knowledge and may vary across boards.

I think we should be able to describe this now with the clock bindings
for device tree. If not, then we need to update those bindings rather
than work around the shortcomings.

>    * perform initializations which are more than clock, like phy
>      initialization/programming etc. This is SoC dependant.

Setting up the phy is different, but again I think there is a better
way than using callbacks if you put the initialization for the
phy into the driver that deals with that phy.

> > >   * spi controller
> > >         This is little hack in the h/w. As part of s/w controlled chip
> > >         select/deselect in spi-pl022, we need to write to some system
> > >         specific register.
> > > 
> > >   * OTG controller
> > >         phy clock initialization which involves series of steps
> > >         including powering down, etc.
> > > 
> > >   * NAND controller
> > >         bank selection on runtime by writing to system registers
> > 
> > I don't understand any of these, so unless you post the code
> > that you want help with as I asked above, I'll assume that you find
> > the solution yourself and don't need a callback ;-)
> 
> Some of them are,
> 
> - fsmc NAND controller 
> 
>         -- include/linux/mtd/fsmc.h
> 
>         struct fsmc_nand_platform_data {
>                 ...
> 
>                 void (*select_bank)(uint32_t bank, uint32_t busw);
> 
>                 ...
> 
>         };
> 
> 
>         -- arch/arm/mach-spear13xx/spear13xx.c
> 
>         void nand_select_bank(u32 bank, u32 busw)
>         {
>                 u32 fsmc_cfg;
> 
>                 if (cpu_is_spear1340()) {
>         #ifdef CONFIG_CPU_SPEAR1340
>                         fsmc_cfg = readl(VA_SPEAR1340_FSMC_CFG);
>         #endif
>                 } else 
>                         fsmc_cfg = readl(VA_FSMC_CFG);
> 
>                 fsmc_cfg &= ~(NAND_BANK_MASK << NAND_BANK_SHIFT);
>                 fsmc_cfg |= (bank << NAND_BANK_SHIFT);
> 
>                 if (busw)
>                         fsmc_cfg |= 1 << NAND_DEV_WIDTH16;
>                 else
>                         fsmc_cfg &= ~(1 << NAND_DEV_WIDTH16);
> 
>                 if (cpu_is_spear1340()) {
>         #ifdef CONFIG_CPU_SPEAR1340
>                         writel(fsmc_cfg, VA_SPEAR1340_FSMC_CFG);
>         #endif
>                 } else
>                         writel(fsmc_cfg, VA_FSMC_CFG);
>         }

The FSMC driver is ST Microelectronics specific, and this function looks fairly
straightforward. I think this can easily go into the driver, with a few
modifications:

* Find the FSMC_CFG address using the device tree. You should never have
any hardcoded virtual addresses like the one in this file, but instead
ioremap a resource that gets passed from the device tree. Where is this
register located? I assume that it's not part of the normal fsmc registers,
but how to get to this address depends a bit on where it's located.

* Remove the cpu_is_spear1340() hacks. If you have to tell the difference
between variations, make it depend on different "compatible" properties
on the presence or absence of some other property or register range.


> 
> 
>         -- arch/arm/mach-spear13xx/spear1340.c
> 
>         void sata_miphy_exit(struct device *dev)
>         {
>                 writel(0, VA_SPEAR1340_PCIE_SATA_CFG);
>                 writel(0, VA_SPEAR1340_PCIE_MIPHY_CFG);
> 
>                 /* Enable PCIE SATA Controller reset */
>                 writel((readl(VA_SPEAR1340_PERIP1_SW_RST) | (0x1000)),
>                                 VA_SPEAR1340_PERIP1_SW_RST);
>                 msleep(20);
> 
>                 /* Switch off sata power domain */
>                 writel((readl(VA_SPEAR1340_PCM_CFG) & (~0x800)),
>                                 VA_SPEAR1340_PCM_CFG);
>                 msleep(20);
>         }

Again, you should move the accesses to VA_SPEAR1340_PERIP1_SW_RST etc to some
device driver that deals with the specific register range. This can be
the driver for the actual peripheral itself, or it can be some kind of
"system controller" that exports an interface.
In the case of power domains and getting stuff in and out of reset,
the subsystems that you most likely should be using do do this are the
regulator and the powerdomain framework. Regulators already have device
tree bindings, while the power domains still need them.

	Arnd