[PATCH v6 0/5] cpsw: support for control module register

[PATCH v6 0/5] cpsw: support for control module register

[Daniel Mack]

Hi,

this is the 6th version of my patch set, the version history is below.

Note that for personal reasons, I won't be able to work on that patch
set for two weeks, starting from a few hours from now. If there are any
more objections or comments, I'll catch up after that period. Or if
anyone wants to make minor changes to my patches and resubmit them,
I'd also be fine with that :)

The compatibility discussion is still ongoing, but I'd rather submit
what I got until now, and I'm also still convinced that having
ti,am3352-cpsw is the best we can do.


Thanks,
Daniel

v5 -> v6:
	* re-add code to fail if 3rd memory region is given but invalid
	* added Mugunthan's ack on all patches except the one that adds
	  ti,am3352-cpsw
	* remove parens around integer constants

v4 -> v5:
	* fix return value assignment in devm_ioremap_resource() error
	  paths (reported by Sergei Shtylyov)
	* add more RGMII modes (reported by Mugunthan V N)
	* check for IS_ERR(priv->gmii_sel_reg) in
	  cpsw_set_phy_interface_mode() (reported by Mugunthan V N)
	* move SoC specific definitions to cpsw.h
	* further simplify the probe logic, as devm_ioremap_resource()
	  will check for NULL resources (Sergei Shtylyov)

v3 -> v4:
	* use IS_ERR() to check for failed devm_ioremap_resource()
	  calls (reported by Sergei Shtylyov)

v2 -> v3:
	* swap "ti,am3352-cpsw" and "ti,cpsw" to work around a matching
	  bug (reported by Sekhar)

v1 -> v2:
	* combine devm_request_mem_region() and devm_ioremap() and use
	  devm_ioremap_resource() (reported by Sergei Shtylyov)
	* fix multi-line comment style (reported by Sergei Shtylyov)
	* fix ti,rmii-clock-ext property name (reported by Sekhar)
	* rebased to net-next (reported by Mugunthan V N, David Miller)
	* add a new compatible type, and handle AM33xx specific
	  registers that way (reported by Sekhar)
	* move gmii_sel_reg modifications to the open routine
	  (reported by Mugunthan V N)


Daniel Mack (5):
  net: ethernet: cpsw: switch to devres allocations
  net: ethernet: cpsw: add optional third memory region for CONTROL
    module
  net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
  net: ethernet: cpsw: add support for hardware interface mode config
  ARM: dts: am33xx: adopt to cpsw changes

 Documentation/devicetree/bindings/net/cpsw.txt |  10 +-
 arch/arm/boot/dts/am33xx.dtsi                  |   5 +-
 drivers/net/ethernet/ti/cpsw.c                 | 252 +++++++++++++------------
 drivers/net/ethernet/ti/cpsw.h                 |   9 +
 4 files changed, 156 insertions(+), 120 deletions(-)

-- 
1.8.3.1

[PATCH v6 1/5] net: ethernet: cpsw: switch to devres allocations

[Daniel Mack]

This patch cleans up the allocation and error unwind paths, which
allows us to carry less information in struct cpsw_priv and reduce the
amount of jump labels in the probe functions.

Signed-off-by: Daniel Mack <zonque@gmail.com>
Acked-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 drivers/net/ethernet/ti/cpsw.c | 153 ++++++++++++-----------------------------
 1 file changed, 43 insertions(+), 110 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 79974e3..fc3263f 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -367,8 +367,6 @@ struct cpsw_priv {
 	spinlock_t			lock;
 	struct platform_device		*pdev;
 	struct net_device		*ndev;
-	struct resource			*cpsw_res;
-	struct resource			*cpsw_wr_res;
 	struct napi_struct		napi;
 	struct device			*dev;
 	struct cpsw_platform_data	data;
@@ -1712,62 +1710,55 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
 
 	if (of_property_read_u32(node, "active_slave", &prop)) {
 		pr_err("Missing active_slave property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
+		return -EINVAL;
 	}
 	data->active_slave = prop;
 
 	if (of_property_read_u32(node, "cpts_clock_mult", &prop)) {
 		pr_err("Missing cpts_clock_mult property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
+		return -EINVAL;
 	}
 	data->cpts_clock_mult = prop;
 
 	if (of_property_read_u32(node, "cpts_clock_shift", &prop)) {
 		pr_err("Missing cpts_clock_shift property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
+		return -EINVAL;
 	}
 	data->cpts_clock_shift = prop;
 
-	data->slave_data = kcalloc(data->slaves, sizeof(struct cpsw_slave_data),
-				   GFP_KERNEL);
+	data->slave_data = devm_kzalloc(&pdev->dev, data->slaves
+					* sizeof(struct cpsw_slave_data),
+					GFP_KERNEL);
 	if (!data->slave_data)
-		return -EINVAL;
+		return -ENOMEM;
 
 	if (of_property_read_u32(node, "cpdma_channels", &prop)) {
 		pr_err("Missing cpdma_channels property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
+		return -EINVAL;
 	}
 	data->channels = prop;
 
 	if (of_property_read_u32(node, "ale_entries", &prop)) {
 		pr_err("Missing ale_entries property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
+		return -EINVAL;
 	}
 	data->ale_entries = prop;
 
 	if (of_property_read_u32(node, "bd_ram_size", &prop)) {
 		pr_err("Missing bd_ram_size property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
+		return -EINVAL;
 	}
 	data->bd_ram_size = prop;
 
 	if (of_property_read_u32(node, "rx_descs", &prop)) {
 		pr_err("Missing rx_descs property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
+		return -EINVAL;
 	}
 	data->rx_descs = prop;
 
 	if (of_property_read_u32(node, "mac_control", &prop)) {
 		pr_err("Missing mac_control property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
+		return -EINVAL;
 	}
 	data->mac_control = prop;
 
@@ -1794,8 +1785,7 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
 		parp = of_get_property(slave_node, "phy_id", &lenp);
 		if ((parp == NULL) || (lenp != (sizeof(void *) * 2))) {
 			pr_err("Missing slave[%d] phy_id property\n", i);
-			ret = -EINVAL;
-			goto error_ret;
+			return -EINVAL;
 		}
 		mdio_node = of_find_node_by_phandle(be32_to_cpup(parp));
 		phyid = be32_to_cpup(parp+1);
@@ -1825,10 +1815,6 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
 	}
 
 	return 0;
-
-error_ret:
-	kfree(data->slave_data);
-	return ret;
 }
 
 static int cpsw_probe_dual_emac(struct platform_device *pdev,
@@ -1870,7 +1856,6 @@ static int cpsw_probe_dual_emac(struct platform_device *pdev,
 	priv_sl2->coal_intvl = 0;
 	priv_sl2->bus_freq_mhz = priv->bus_freq_mhz;
 
-	priv_sl2->cpsw_res = priv->cpsw_res;
 	priv_sl2->regs = priv->regs;
 	priv_sl2->host_port = priv->host_port;
 	priv_sl2->host_port_regs = priv->host_port_regs;
@@ -1914,8 +1899,8 @@ static int cpsw_probe(struct platform_device *pdev)
 	struct cpsw_priv		*priv;
 	struct cpdma_params		dma_params;
 	struct cpsw_ale_params		ale_params;
-	void __iomem			*ss_regs, *wr_regs;
-	struct resource			*res;
+	void __iomem			*ss_regs;
+	struct resource			*res, *ss_res;
 	u32 slave_offset, sliver_offset, slave_size;
 	int ret = 0, i, k = 0;
 
@@ -1951,7 +1936,7 @@ static int cpsw_probe(struct platform_device *pdev)
 	if (cpsw_probe_dt(&priv->data, pdev)) {
 		pr_err("cpsw: platform data missing\n");
 		ret = -ENODEV;
-		goto clean_ndev_ret;
+		goto clean_runtime_disable_ret;
 	}
 	data = &priv->data;
 
@@ -1965,11 +1950,12 @@ static int cpsw_probe(struct platform_device *pdev)
 
 	memcpy(ndev->dev_addr, priv->mac_addr, ETH_ALEN);
 
-	priv->slaves = kzalloc(sizeof(struct cpsw_slave) * data->slaves,
-			       GFP_KERNEL);
+	priv->slaves =
+		devm_kzalloc(&pdev->dev, sizeof(struct cpsw_slave) * data->slaves,
+			     GFP_KERNEL);
 	if (!priv->slaves) {
-		ret = -EBUSY;
-		goto clean_ndev_ret;
+		ret = -ENOMEM;
+		goto clean_runtime_disable_ret;
 	}
 	for (i = 0; i < data->slaves; i++)
 		priv->slaves[i].slave_num = i;
@@ -1977,55 +1963,31 @@ static int cpsw_probe(struct platform_device *pdev)
 	priv->slaves[0].ndev = ndev;
 	priv->emac_port = 0;
 
-	priv->clk = clk_get(&pdev->dev, "fck");
+	priv->clk = devm_clk_get(&pdev->dev, "fck");
 	if (IS_ERR(priv->clk)) {
-		dev_err(&pdev->dev, "fck is not found\n");
+		dev_err(priv->dev, "fck is not found\n");
 		ret = -ENODEV;
-		goto clean_slave_ret;
+		goto clean_runtime_disable_ret;
 	}
 	priv->coal_intvl = 0;
 	priv->bus_freq_mhz = clk_get_rate(priv->clk) / 1000000;
 
-	priv->cpsw_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!priv->cpsw_res) {
-		dev_err(priv->dev, "error getting i/o resource\n");
-		ret = -ENOENT;
-		goto clean_clk_ret;
-	}
-	if (!request_mem_region(priv->cpsw_res->start,
-				resource_size(priv->cpsw_res), ndev->name)) {
-		dev_err(priv->dev, "failed request i/o region\n");
-		ret = -ENXIO;
-		goto clean_clk_ret;
-	}
-	ss_regs = ioremap(priv->cpsw_res->start, resource_size(priv->cpsw_res));
-	if (!ss_regs) {
-		dev_err(priv->dev, "unable to map i/o region\n");
-		goto clean_cpsw_iores_ret;
+	ss_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	ss_regs = devm_ioremap_resource(&pdev->dev, ss_res);
+	if (IS_ERR(ss_regs)) {
+		ret = PTR_ERR(ss_regs);
+		goto clean_runtime_disable_ret;
 	}
 	priv->regs = ss_regs;
 	priv->version = __raw_readl(&priv->regs->id_ver);
 	priv->host_port = HOST_PORT_NUM;
 
-	priv->cpsw_wr_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	if (!priv->cpsw_wr_res) {
-		dev_err(priv->dev, "error getting i/o resource\n");
-		ret = -ENOENT;
-		goto clean_iomap_ret;
-	}
-	if (!request_mem_region(priv->cpsw_wr_res->start,
-			resource_size(priv->cpsw_wr_res), ndev->name)) {
-		dev_err(priv->dev, "failed request i/o region\n");
-		ret = -ENXIO;
-		goto clean_iomap_ret;
-	}
-	wr_regs = ioremap(priv->cpsw_wr_res->start,
-				resource_size(priv->cpsw_wr_res));
-	if (!wr_regs) {
-		dev_err(priv->dev, "unable to map i/o region\n");
-		goto clean_cpsw_wr_iores_ret;
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	priv->wr_regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(priv->wr_regs)) {
+		ret = PTR_ERR(priv->wr_regs);
+		goto clean_runtime_disable_ret;
 	}
-	priv->wr_regs = wr_regs;
 
 	memset(&dma_params, 0, sizeof(dma_params));
 	memset(&ale_params, 0, sizeof(ale_params));
@@ -2056,12 +2018,12 @@ static int cpsw_probe(struct platform_device *pdev)
 		slave_size           = CPSW2_SLAVE_SIZE;
 		sliver_offset        = CPSW2_SLIVER_OFFSET;
 		dma_params.desc_mem_phys =
-			(u32 __force) priv->cpsw_res->start + CPSW2_BD_OFFSET;
+			(u32 __force) ss_res->start + CPSW2_BD_OFFSET;
 		break;
 	default:
 		dev_err(priv->dev, "unknown version 0x%08x\n", priv->version);
 		ret = -ENODEV;
-		goto clean_cpsw_wr_iores_ret;
+		goto clean_runtime_disable_ret;
 	}
 	for (i = 0; i < priv->data.slaves; i++) {
 		struct cpsw_slave *slave = &priv->slaves[i];
@@ -2089,7 +2051,7 @@ static int cpsw_probe(struct platform_device *pdev)
 	if (!priv->dma) {
 		dev_err(priv->dev, "error initializing dma\n");
 		ret = -ENOMEM;
-		goto clean_wr_iomap_ret;
+		goto clean_runtime_disable_ret;
 	}
 
 	priv->txch = cpdma_chan_create(priv->dma, tx_chan_num(0),
@@ -2124,8 +2086,8 @@ static int cpsw_probe(struct platform_device *pdev)
 
 	while ((res = platform_get_resource(priv->pdev, IORESOURCE_IRQ, k))) {
 		for (i = res->start; i <= res->end; i++) {
-			if (request_irq(i, cpsw_interrupt, 0,
-					dev_name(&pdev->dev), priv)) {
+			if (devm_request_irq(&pdev->dev, i, cpsw_interrupt, 0,
+					     dev_name(priv->dev), priv)) {
 				dev_err(priv->dev, "error attaching irq\n");
 				goto clean_ale_ret;
 			}
@@ -2147,7 +2109,7 @@ static int cpsw_probe(struct platform_device *pdev)
 	if (ret) {
 		dev_err(priv->dev, "error registering net device\n");
 		ret = -ENODEV;
-		goto clean_irq_ret;
+		goto clean_ale_ret;
 	}
 
 	if (cpts_register(&pdev->dev, priv->cpts,
@@ -2155,44 +2117,27 @@ static int cpsw_probe(struct platform_device *pdev)
 		dev_err(priv->dev, "error registering cpts device\n");
 
 	cpsw_notice(priv, probe, "initialized device (regs %x, irq %d)\n",
-		  priv->cpsw_res->start, ndev->irq);
+		    ss_res->start, ndev->irq);
 
 	if (priv->data.dual_emac) {
 		ret = cpsw_probe_dual_emac(pdev, priv);
 		if (ret) {
 			cpsw_err(priv, probe, "error probe slave 2 emac interface\n");
-			goto clean_irq_ret;
+			goto clean_ale_ret;
 		}
 	}
 
 	return 0;
 
-clean_irq_ret:
-	for (i = 0; i < priv->num_irqs; i++)
-		free_irq(priv->irqs_table[i], priv);
 clean_ale_ret:
 	cpsw_ale_destroy(priv->ale);
 clean_dma_ret:
 	cpdma_chan_destroy(priv->txch);
 	cpdma_chan_destroy(priv->rxch);
 	cpdma_ctlr_destroy(priv->dma);
-clean_wr_iomap_ret:
-	iounmap(priv->wr_regs);
-clean_cpsw_wr_iores_ret:
-	release_mem_region(priv->cpsw_wr_res->start,
-			   resource_size(priv->cpsw_wr_res));
-clean_iomap_ret:
-	iounmap(priv->regs);
-clean_cpsw_iores_ret:
-	release_mem_region(priv->cpsw_res->start,
-			   resource_size(priv->cpsw_res));
-clean_clk_ret:
-	clk_put(priv->clk);
-clean_slave_ret:
+clean_runtime_disable_ret:
 	pm_runtime_disable(&pdev->dev);
-	kfree(priv->slaves);
 clean_ndev_ret:
-	kfree(priv->data.slave_data);
 	free_netdev(priv->ndev);
 	return ret;
 }
@@ -2201,30 +2146,18 @@ static int cpsw_remove(struct platform_device *pdev)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct cpsw_priv *priv = netdev_priv(ndev);
-	int i;
 
 	if (priv->data.dual_emac)
 		unregister_netdev(cpsw_get_slave_ndev(priv, 1));
 	unregister_netdev(ndev);
 
 	cpts_unregister(priv->cpts);
-	for (i = 0; i < priv->num_irqs; i++)
-		free_irq(priv->irqs_table[i], priv);
 
 	cpsw_ale_destroy(priv->ale);
 	cpdma_chan_destroy(priv->txch);
 	cpdma_chan_destroy(priv->rxch);
 	cpdma_ctlr_destroy(priv->dma);
-	iounmap(priv->regs);
-	release_mem_region(priv->cpsw_res->start,
-			   resource_size(priv->cpsw_res));
-	iounmap(priv->wr_regs);
-	release_mem_region(priv->cpsw_wr_res->start,
-			   resource_size(priv->cpsw_wr_res));
 	pm_runtime_disable(&pdev->dev);
-	clk_put(priv->clk);
-	kfree(priv->slaves);
-	kfree(priv->data.slave_data);
 	if (priv->data.dual_emac)
 		free_netdev(cpsw_get_slave_ndev(priv, 1));
 	free_netdev(ndev);
-- 
1.8.3.1

[PATCH v6 2/5] net: ethernet: cpsw: add optional third memory region for CONTROL module

[Daniel Mack]

At least the AM33xx SoC has a control module register to configure
details such as the hardware ethernet interface mode.

I'm not sure whether all SoCs which feature the cpsw block have such a
register, so that third memory region is considered optional for now.

Signed-off-by: Daniel Mack <zonque@gmail.com>
Acked-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 Documentation/devicetree/bindings/net/cpsw.txt |  5 ++++-
 drivers/net/ethernet/ti/cpsw.c                 | 11 +++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index 05d660e..4e5ca54 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -4,7 +4,10 @@ TI SoC Ethernet Switch Controller Device Tree Bindings
 Required properties:
 - compatible		: Should be "ti,cpsw"
 - reg			: physical base address and size of the cpsw
-			  registers map
+			  registers map.
+			  An optional third memory region can be supplied if
+			  the platform has a control module register to
+			  configure phy interface details
 - interrupts		: property with a value describing the interrupt
 			  number
 - interrupt-parent	: The parent interrupt controller
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index fc3263f..485df80 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -372,6 +372,7 @@ struct cpsw_priv {
 	struct cpsw_platform_data	data;
 	struct cpsw_ss_regs __iomem	*regs;
 	struct cpsw_wr_regs __iomem	*wr_regs;
+	u32 __iomem			*gmii_sel_reg;
 	u8 __iomem			*hw_stats;
 	struct cpsw_host_regs __iomem	*host_port_regs;
 	u32				msg_enable;
@@ -1989,6 +1990,16 @@ static int cpsw_probe(struct platform_device *pdev)
 		goto clean_runtime_disable_ret;
 	}
 
+	/* Don't fail hard if the optional control memory region is missing */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	if (res) {
+		priv->gmii_sel_reg = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(priv->gmii_sel_reg)) {
+			ret = PTR_ERR(priv->gmii_sel_reg);
+			goto clean_runtime_disable_ret;
+		}
+	}
+
 	memset(&dma_params, 0, sizeof(dma_params));
 	memset(&ale_params, 0, sizeof(ale_params));
 
-- 
1.8.3.1

Re: [PATCH v6 2/5] net: ethernet: cpsw: add optional third memory region for CONTROL module

[Tony Lindgren]

* Daniel Mack <zonque@gmail.com> [130823 12:39]:
> At least the AM33xx SoC has a control module register to configure
> details such as the hardware ethernet interface mode.
> 
> I'm not sure whether all SoCs which feature the cpsw block have such a
> register, so that third memory region is considered optional for now.

Assuming you're talking about omap SCM registers here..

This should be in a separate driver module so the control module
parts can eventually be children of the SCM driver as they are
really separate devices on the bus. See how the USB PHY parts were done
for example.

What do these control module registers do? If it's just multiplexing
and pinconf, then you can use pinctrl-single,bits most likely for it and
access it using the named modes.

However, if the register also contains comparators and control for
regulators, you should only use pinctrl-single for the multiplexing
and pinconf parts.

Regards,

Tony

Re: [PATCH v6 2/5] net: ethernet: cpsw: add optional third memory region for CONTROL module

[Mugunthan V N]

On Monday 26 August 2013 02:34 PM, Tony Lindgren wrote:
> * Daniel Mack <zonque@gmail.com> [130823 12:39]:
>> At least the AM33xx SoC has a control module register to configure
>> details such as the hardware ethernet interface mode.
>>
>> I'm not sure whether all SoCs which feature the cpsw block have such a
>> register, so that third memory region is considered optional for now.
> Assuming you're talking about omap SCM registers here..
>
> This should be in a separate driver module so the control module
> parts can eventually be children of the SCM driver as they are
> really separate devices on the bus. See how the USB PHY parts were done
> for example.
>
> What do these control module registers do? If it's just multiplexing
> and pinconf, then you can use pinctrl-single,bits most likely for it and
> access it using the named modes.
>
> However, if the register also contains comparators and control for
> regulators, you should only use pinctrl-single for the multiplexing
> and pinconf parts.
>
I will take a look into usb control module driver and will try to adopt
the driver here also.

Regards
Mugunthan V N

[PATCH v6 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string

[Daniel Mack]

In order to support features that are specific to the AM335x IP, we have
to add hardware types and another compatible string.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 Documentation/devicetree/bindings/net/cpsw.txt |  3 ++-
 drivers/net/ethernet/ti/cpsw.c                 | 32 ++++++++++++++++++++------
 drivers/net/ethernet/ti/cpsw.h                 |  1 +
 3 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index 4e5ca54..b717458 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -2,7 +2,8 @@ TI SoC Ethernet Switch Controller Device Tree Bindings
 ------------------------------------------------------
 
 Required properties:
-- compatible		: Should be "ti,cpsw"
+- compatible		: Should be "ti,cpsw" for generic cpsw support, or
+			  "ti,am3352-cpsw" for AM3352 SoCs
 - reg			: physical base address and size of the cpsw
 			  registers map.
 			  An optional third memory region can be supplied if
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 485df80..8d023ab 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -155,6 +155,11 @@ do {								\
 		((priv->data.dual_emac) ? priv->emac_port :	\
 		priv->data.active_slave)
 
+enum {
+	CPSW_TYPE_GENERIC,
+	CPSW_TYPE_AM33XX
+};
+
 static int debug_level;
 module_param(debug_level, int, 0);
 MODULE_PARM_DESC(debug_level, "cpsw debug level (NETIF_MSG bits)");
@@ -1692,17 +1697,36 @@ static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv,
 	slave->port_vlan = data->dual_emac_res_vlan;
 }
 
+static const struct of_device_id cpsw_of_mtable[] = {
+	{
+		.compatible	= "ti,am3352-cpsw",
+		.data		= (void *) CPSW_TYPE_AM33XX
+	}, {
+		.compatible	= "ti,cpsw",
+		.data		= (void *) CPSW_TYPE_GENERIC
+	},
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, cpsw_of_mtable);
+
 static int cpsw_probe_dt(struct cpsw_platform_data *data,
 			 struct platform_device *pdev)
 {
 	struct device_node *node = pdev->dev.of_node;
+	const struct of_device_id *match;
 	struct device_node *slave_node;
+	unsigned long match_data;
 	int i = 0, ret;
 	u32 prop;
 
-	if (!node)
+	match = of_match_device(cpsw_of_mtable, &pdev->dev);
+
+	if (!node || !match)
 		return -EINVAL;
 
+	match_data = (unsigned long) match->data;
+	data->hw_type = match_data;
+
 	if (of_property_read_u32(node, "slaves", &prop)) {
 		pr_err("Missing slaves property in the DT.\n");
 		return -EINVAL;
@@ -2213,12 +2237,6 @@ static const struct dev_pm_ops cpsw_pm_ops = {
 	.resume		= cpsw_resume,
 };
 
-static const struct of_device_id cpsw_of_mtable[] = {
-	{ .compatible = "ti,cpsw", },
-	{ /* sentinel */ },
-};
-MODULE_DEVICE_TABLE(of, cpsw_of_mtable);
-
 static struct platform_driver cpsw_driver = {
 	.driver = {
 		.name	 = "cpsw",
diff --git a/drivers/net/ethernet/ti/cpsw.h b/drivers/net/ethernet/ti/cpsw.h
index eb3e101..96c374a 100644
--- a/drivers/net/ethernet/ti/cpsw.h
+++ b/drivers/net/ethernet/ti/cpsw.h
@@ -37,6 +37,7 @@ struct cpsw_platform_data {
 	u32	mac_control;	/* Mac control register */
 	u16	default_vlan;	/* Def VLAN for ALE lookup in VLAN aware mode*/
 	bool	dual_emac;	/* Enable Dual EMAC mode */
+	u32	hw_type;	/* hardware type as specified in 'compatible' */
 };
 
 #endif /* __CPSW_H__ */
-- 
1.8.3.1

[PATCH v6 4/5] net: ethernet: cpsw: add support for hardware interface mode config

[Daniel Mack]

The cpsw currently lacks code to properly set up the hardware interface
mode on AM33xx. Other platforms might be equally affected.

Usually, the bootloader will configure the control module register, so
probably that's why such support wasn't needed in the past. In suspend
mode though, this register is modified, and so it needs reprogramming
after resume.

This patch adds code that makes use of the previously added and optional
support for passing the control mode register, and configures the
correct register bits when the slave is opened.

The AM33xx also has a bit for each slave to configure the RMII reference
clock direction. Setting it is now supported by a per-slave DT property.

This code path introducted by this patch is currently exclusive for
am33xx.

Signed-off-by: Daniel Mack <zonque@gmail.com>
Acked-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 Documentation/devicetree/bindings/net/cpsw.txt |  2 +
 drivers/net/ethernet/ti/cpsw.c                 | 58 ++++++++++++++++++++++++++
 drivers/net/ethernet/ti/cpsw.h                 |  8 ++++
 3 files changed, 68 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index b717458..0895a51 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -34,6 +34,8 @@ Required properties:
 - phy_id		: Specifies slave phy id
 - phy-mode		: The interface between the SoC and the PHY (a string
 			  that of_get_phy_mode() can understand)
+- ti,rmii-clock-ext	: If present, the driver will configure the RMII
+			  interface to external clock usage
 - mac-address		: Specifies slave MAC address
 
 Optional properties:
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 8d023ab..6b47e78 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -980,6 +980,60 @@ static inline void cpsw_add_dual_emac_def_ale_entries(
 		priv->host_port, ALE_VLAN, slave->port_vlan);
 }
 
+static void cpsw_set_phy_interface_mode(struct cpsw_slave *slave,
+					struct cpsw_priv *priv)
+{
+	u32 reg;
+	u32 mask;
+	u32 mode = 0;
+
+	switch (priv->data.hw_type) {
+	case CPSW_TYPE_AM33XX:
+		if (IS_ERR_OR_NULL(priv->gmii_sel_reg))
+			break;
+
+		reg = readl(priv->gmii_sel_reg);
+
+		if (slave->phy) {
+			switch (slave->phy->interface) {
+			case PHY_INTERFACE_MODE_MII:
+			default:
+				mode = AM33XX_GMII_SEL_MODE_MII;
+				break;
+			case PHY_INTERFACE_MODE_RMII:
+				mode = AM33XX_GMII_SEL_MODE_RMII;
+				break;
+			case PHY_INTERFACE_MODE_RGMII:
+			case PHY_INTERFACE_MODE_RGMII_ID:
+			case PHY_INTERFACE_MODE_RGMII_RXID:
+			case PHY_INTERFACE_MODE_RGMII_TXID:
+				mode = AM33XX_GMII_SEL_MODE_RGMII;
+				break;
+			};
+		}
+
+		mask = 0x3 << (slave->slave_num * 2) |
+		       BIT(slave->slave_num + 6);
+		mode <<= slave->slave_num * 2;
+
+		if (slave->data->rmii_clock_external) {
+			if (slave->slave_num == 0)
+				mode |= AM33XX_GMII_SEL_RMII1_IO_CLK_EN;
+			else
+				mode |= AM33XX_GMII_SEL_RMII2_IO_CLK_EN;
+		}
+
+		reg &= ~mask;
+		reg |= mode;
+
+		writel(reg, priv->gmii_sel_reg);
+		break;
+
+	default:
+		break;
+	}
+}
+
 static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
 {
 	char name[32];
@@ -1028,6 +1082,8 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
 			 slave->phy->phy_id);
 		phy_start(slave->phy);
 	}
+
+	cpsw_set_phy_interface_mode(slave, priv);
 }
 
 static inline void cpsw_add_default_vlan(struct cpsw_priv *priv)
@@ -1823,6 +1879,8 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
 			memcpy(slave_data->mac_addr, mac_addr, ETH_ALEN);
 
 		slave_data->phy_if = of_get_phy_mode(slave_node);
+		if (of_find_property(slave_node, "ti,rmii-clock-ext", NULL))
+			slave_data->rmii_clock_external = true;
 
 		if (data->dual_emac) {
 			if (of_property_read_u32(slave_node, "dual_emac_res_vlan",
diff --git a/drivers/net/ethernet/ti/cpsw.h b/drivers/net/ethernet/ti/cpsw.h
index 96c374a..3a6cb16 100644
--- a/drivers/net/ethernet/ti/cpsw.h
+++ b/drivers/net/ethernet/ti/cpsw.h
@@ -19,6 +19,7 @@
 struct cpsw_slave_data {
 	char		phy_id[MII_BUS_ID_SIZE];
 	int		phy_if;
+	bool		rmii_clock_external;
 	u8		mac_addr[ETH_ALEN];
 	u16		dual_emac_res_vlan;	/* Reserved VLAN for DualEMAC */
 };
@@ -40,4 +41,11 @@ struct cpsw_platform_data {
 	u32	hw_type;	/* hardware type as specified in 'compatible' */
 };
 
+/* SoC specific definitions for the CONTROL port */
+#define AM33XX_GMII_SEL_MODE_MII	0
+#define AM33XX_GMII_SEL_MODE_RMII	1
+#define AM33XX_GMII_SEL_MODE_RGMII	2
+
+#define AM33XX_GMII_SEL_RMII2_IO_CLK_EN	BIT(7)
+#define AM33XX_GMII_SEL_RMII1_IO_CLK_EN	BIT(6)
 #endif /* __CPSW_H__ */
-- 
1.8.3.1

[PATCH v6 5/5] ARM: dts: am33xx: adopt to cpsw changes

[Daniel Mack]

This third memory region just denotes one single register in the CONTROL
module block. The driver uses that in order to set the correct physical
ethernet interface modes.

Also update the compatible string to make use of the am335x specific
features of the cpsw driver.

Signed-off-by: Daniel Mack <zonque@gmail.com>
Acked-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 arch/arm/boot/dts/am33xx.dtsi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index a785b95..c7b41ae 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -446,7 +446,7 @@
 		};
 
 		mac: ethernet@4a100000 {
-			compatible = "ti,cpsw";
+			compatible = "ti,am3352-cpsw", "ti,cpsw";
 			ti,hwmods = "cpgmac0";
 			cpdma_channels = <8>;
 			ale_entries = <1024>;
@@ -459,7 +459,8 @@
 			cpts_clock_mult = <0x80000000>;
 			cpts_clock_shift = <29>;
 			reg = <0x4a100000 0x800
-			       0x4a101200 0x100>;
+			       0x4a101200 0x100
+			       0x44e10650 0x4>;
 			#address-cells = <1>;
 			#size-cells = <1>;
 			interrupt-parent = <&intc>;
-- 
1.8.3.1

Re: [PATCH v6 0/5] cpsw: support for control module register

[Mugunthan V N]

On Saturday 24 August 2013 01:02 AM, Daniel Mack wrote:
> Hi,
>
> this is the 6th version of my patch set, the version history is below.
>
> Note that for personal reasons, I won't be able to work on that patch
> set for two weeks, starting from a few hours from now. If there are any
> more objections or comments, I'll catch up after that period. Or if
> anyone wants to make minor changes to my patches and resubmit them,
> I'd also be fine with that :)
>
> The compatibility discussion is still ongoing, but I'd rather submit
> what I got until now, and I'm also still convinced that having
> ti,am3352-cpsw is the best we can do.
>
>
> Thanks,
> Daniel
>
> v5 -> v6:
> 	* re-add code to fail if 3rd memory region is given but invalid
> 	* added Mugunthan's ack on all patches except the one that adds
> 	  ti,am3352-cpsw
> 	* remove parens around integer constants
I am also in your favour of adding ti,am3352-cpsw, but lets get other
comment. You can add my ack to that patch also if you are submitting
another version.

Regards
Mugunthan V N

Re: [PATCH v6 0/5] cpsw: support for control module register

[Daniel Mack]

On 23.08.2013 21:38, Mugunthan V N wrote:
> On Saturday 24 August 2013 01:02 AM, Daniel Mack wrote:

>> v5 -> v6:
>> 	* re-add code to fail if 3rd memory region is given but invalid
>> 	* added Mugunthan's ack on all patches except the one that adds
>> 	  ti,am3352-cpsw
>> 	* remove parens around integer constants
> I am also in your favour of adding ti,am3352-cpsw, but lets get other
> comment. You can add my ack to that patch also if you are submitting
> another version.

I don't think I'll have a chance to do that before I'm AFK for a while :)

That's also the only reason why I sent a v6 already. I would have
normally waited for that discussion to settle of course.

But as I said: I'd be fine if anyone adds another patch on top or amends
mine.


Thanks for the reviews again, everyone!


Daniel