ARM: SPEAr13xx: Minor fixes and updation

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 <[email protected]>

---
 drivers/clk/spear/spear1340_clock.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

drivers/clk/spear/spear1340_clock.c +0 -0

index 2cd5520..f927f90 100644

Use AUXDATA to pass platform data for Ethernet controller.

arch/arm/mach-spear13xx/spear1340.c +16 -0

 
+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)
 {

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.

Quoted 16 lines · viresh kumar

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.

Quoted 8 lines

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
.

Hi Arnd,

Quoted 27 lines · Arnd Bergmann

Quoted 16 lines · viresh kumar

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.

Quoted 8 lines

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.

Quoted 7 lines

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.

Quoted 1 line · Vipul Kumar Samar

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.

--
viresh
.

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.

Quoted 6 lines · deepaksi

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

Quoted 8 lines

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.

Quoted 7 lines

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
.

Quoted 18 lines

Quoted 1 line

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

Quoted 8 lines

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.

*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 ?

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.

Quoted 3 lines · Arnd Bergmann

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.

* 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

Quoted 13 lines · Shiraz Hashim

Quoted 3 lines · Arnd Bergmann

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.

Quoted 4 lines

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.

Quoted 9 lines

* 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.

Quoted 2 lines

* 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 ;-)

Hi Arnd,

Quoted 19 lines · Arnd Bergmann

Quoted 13 lines · Shiraz Hashim

Quoted 3 lines · Arnd Bergmann

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.

Quoted 4 lines

This is the case in Ethernet, audio, etc.

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

Quoted 8 lines

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.

Quoted 9 lines

Quoted 9 lines

* 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.

Quoted 6 lines

Quoted 2 lines

* 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);
        }

        -- 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);
        }

Use AUXDATA to pass platform data for Ethernet controller.

Signed-off-by Vipul Kumar Samar <[email protected]>

---
 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(-)

arch/arm/mach-spear13xx/include/mach/generic.h +2 -0

index dac57fd..8c8fbaa 100644
@@ -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);
 

arch/arm/mach-spear13xx/include/mach/spear.h +1 -0

index 65f27de..b0b6f91 100644
@@ -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

arch/arm/mach-spear13xx/spear1340.c +20 -0

index 81e4ed7..ab282ed 100644
@@ -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,

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 <[email protected]>

---
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +0 -0

index 680d2b8..4651579 100644

Quoted 8 lines · 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<[email protected]>

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.

Quoted 9 lines

---
  .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |    5 +++--
  1 files changed, 3 insertions(+), 2 deletions(-)

drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +0 -0

index 680d2b8..4651579 100644

drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c static int stmmac_pltfr_probe(struct platform_device *pdev) · line 121

Quoted 10 lines

drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +3 -2

@@ -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??