Skip to contentSkip to conversation

cpsw: support for control module register

PATCH Superseded Daniel Mack 4 participants 13 messages Aug 23, 2013

Subsystems (4)

v2 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v2 cover cpsw: support for control module register

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                 | 254 ++++++++++++++-----------
 drivers/net/ethernet/ti/cpsw.h                 |   2 +
 4 files changed, 159 insertions(+), 112 deletions(-)
Signature · 1 line
1.8.3.1
#1 lore raw

PATCH v2 1/5 net: ethernet: cpsw: switch to devres allocations

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.

---
 drivers/net/ethernet/ti/cpsw.c | 145 +++++++++++++----------------------------
 1 file changed, 44 insertions(+), 101 deletions(-)
drivers/net/ethernet/ti/cpsw.c +44 -101
index 79974e3..1a91aac 100644
@@ -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,41 @@ 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) {
+	ss_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!ss_res) {
 		dev_err(priv->dev, "error getting i/o resource\n");
 		ret = -ENOENT;
-		goto clean_clk_ret;
+		goto clean_runtime_disable_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));
+	ss_regs = devm_ioremap_resource(&pdev->dev, ss_res);
 	if (!ss_regs) {
 		dev_err(priv->dev, "unable to map i/o region\n");
-		goto clean_cpsw_iores_ret;
+		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) {
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!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;
+		goto clean_runtime_disable_ret;
 	}
-	wr_regs = ioremap(priv->cpsw_wr_res->start,
-				resource_size(priv->cpsw_wr_res));
-	if (!wr_regs) {
+	priv->wr_regs = devm_ioremap_resource(&pdev->dev, res);
+	if (!priv->wr_regs) {
 		dev_err(priv->dev, "unable to map i/o region\n");
-		goto clean_cpsw_wr_iores_ret;
+		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 +2028,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 +2061,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 +2096,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 +2119,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 +2127,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 +2156,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);
Signature · 1 line
1.8.3.1
#2 lore raw

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

In reply to 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.

---
 Documentation/devicetree/bindings/net/cpsw.txt |  5 ++++-
 drivers/net/ethernet/ti/cpsw.c                 | 16 ++++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/net/cpsw.txt +4 -1
index 05d660e..4e5ca54 100644
@@ -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
drivers/net/ethernet/ti/cpsw.c +16 -0
index 1a91aac..bd0b664 100644
@@ -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;
@@ -1999,6 +2000,21 @@ static int cpsw_probe(struct platform_device *pdev)
 		goto clean_runtime_disable_ret;
 	}
 
+	/* If the control memory region is unspecified, continue without it.
+	 * If it is specified, but we're unable to reserve it, bail.
+	 */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	if (!res) {
+		dev_err(priv->dev, "error getting control i/o resource\n");
+		goto no_gmii_sel;
+	}
+	priv->gmii_sel_reg = devm_ioremap_resource(&pdev->dev, res);
+	if (!priv->gmii_sel_reg) {
+		dev_err(priv->dev, "unable to map control i/o region\n");
+		goto clean_runtime_disable_ret;
+	}
+
+no_gmii_sel:
 	memset(&dma_params, 0, sizeof(dma_params));
 	memset(&ale_params, 0, sizeof(ale_params));
 
Signature · 1 line
1.8.3.1
#3 lore raw

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

Hello.

Quoted 7 lines · 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.

[...]

Quoted 4 lines
drivers/net/ethernet/ti/cpsw.c +0 -0
index 1a91aac..bd0b664 100644

[...]

drivers/net/ethernet/ti/cpsw.c static int cpsw_probe(struct platform_device *pdev) · line 2008

Quoted 10 lines
drivers/net/ethernet/ti/cpsw.c +6 -0
@@ -1999,6 +2000,21 @@ static int cpsw_probe(struct platform_device *pdev)
  		goto clean_runtime_disable_ret;
  	}
 
+	/* If the control memory region is unspecified, continue without it.
+	 * If it is specified, but we're unable to reserve it, bail.
+	 */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	if (!res) {
+		dev_err(priv->dev, "error getting control i/o resource\n");

If the resource is optional, why use dev_err()?

drivers/net/ethernet/ti/cpsw.c static int cpsw_probe(struct platform_device *pdev) · line 2012

Quoted 4 lines
drivers/net/ethernet/ti/cpsw.c +4 -0
+		goto no_gmii_sel;
+	}
+	priv->gmii_sel_reg = devm_ioremap_resource(&pdev->dev, res);
+	if (!priv->gmii_sel_reg) {

devm_ioremap_resource() doesn't return NULL, it returns error, so you
should check with IS_ERR() and propagate the error to the caller with PTR_ERR().

drivers/net/ethernet/ti/cpsw.c static int cpsw_probe(struct platform_device *pdev) · line 2013

Quoted 1 line
drivers/net/ethernet/ti/cpsw.c +1 -0
+		dev_err(priv->dev, "unable to map control i/o region\n");

devm_ioremap_resource() prints out the error messages itself, so you don't
have to.

drivers/net/ethernet/ti/cpsw.c static int cpsw_probe(struct platform_device *pdev) · line 2019

Quoted 6 lines
drivers/net/ethernet/ti/cpsw.c +4 -0
+		goto clean_runtime_disable_ret;
+	}
+
+no_gmii_sel:
  	memset(&dma_params, 0, sizeof(dma_params));
  	memset(&ale_params, 0, sizeof(ale_params));

WBR, Sergei

#4 lore raw

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

Quoted 6 lines · Sergei Shtylyov

drivers/net/ethernet/ti/cpsw.c static int cpsw_probe(struct platform_device *pdev) · line 2013

Quoted 1 line · Daniel Mack
drivers/net/ethernet/ti/cpsw.c +1 -0
+		dev_err(priv->dev, "unable to map control i/o region\n");

devm_ioremap_resource() prints out the error messages itself, so you don't
have to.

Right. Thanks. Will send a v4.

Best,
Daniel

#5 lore raw

PATCH v2 3/5 net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string

In reply to Daniel Mack

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

---
 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(-)
Documentation/devicetree/bindings/net/cpsw.txt +2 -1
index 4e5ca54..b717458 100644
@@ -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
drivers/net/ethernet/ti/cpsw.c +25 -7
index bd0b664..2e19de0 100644
@@ -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,cpsw",
+		.data		= (void *) CPSW_TYPE_GENERIC
+	}, {
+		.compatible	= "ti,am3352-cpsw",
+		.data		= (void *) CPSW_TYPE_AM33XX
+	},
+	{ /* 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;
@@ -2228,12 +2252,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",
drivers/net/ethernet/ti/cpsw.h +1 -0
index eb3e101..96c374a 100644
@@ -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__ */
Signature · 1 line
1.8.3.1
#6 lore raw

PATCH v2 4/5 net: ethernet: cpsw: add support for hardware interface mode config

In reply to 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.

---
 Documentation/devicetree/bindings/net/cpsw.txt |  2 +
 drivers/net/ethernet/ti/cpsw.c                 | 61 ++++++++++++++++++++++++++
 drivers/net/ethernet/ti/cpsw.h                 |  1 +
 3 files changed, 64 insertions(+)
Documentation/devicetree/bindings/net/cpsw.txt +2 -0
index b717458..0895a51 100644
@@ -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:
drivers/net/ethernet/ti/cpsw.c +61 -0

85 lines not on this page · raw message

drivers/net/ethernet/ti/cpsw.h +1 -0
index 96c374a..3baa2350 100644
@@ -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 */
 };
Signature · 1 line
1.8.3.1
#7 lore raw

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

Quoted 43 lines · 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.

---
 Documentation/devicetree/bindings/net/cpsw.txt |  2 +
 drivers/net/ethernet/ti/cpsw.c                 | 61 ++++++++++++++++++++++++++
 drivers/net/ethernet/ti/cpsw.h                 |  1 +
 3 files changed, 64 insertions(+)
Documentation/devicetree/bindings/net/cpsw.txt +2 -0
index b717458..0895a51 100644
@@ -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:
drivers/net/ethernet/ti/cpsw.c +0 -0
index 2e19de0..6e36f49 100644

drivers/net/ethernet/ti/cpsw.c do { \ · line 146

Quoted 9 lines
drivers/net/ethernet/ti/cpsw.c +6 -0
@@ -138,6 +138,13 @@ do {								\
 #define CPSW_CMINTMAX_INTVL	(1000 / CPSW_CMINTMIN_CNT)
 #define CPSW_CMINTMIN_INTVL	((1000 / CPSW_CMINTMAX_CNT) + 1)
 
+#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)

These defined can go into cpsw.h so that we can maintain platform defines in one file.

drivers/net/ethernet/ti/cpsw.c static inline void cpsw_add_dual_emac_def_ale_entries( · line 994

Quoted 13 lines
drivers/net/ethernet/ti/cpsw.c +6 -0
+
 #define cpsw_enable_irq(priv)	\
 	do {			\
 		u32 i;		\
@@ -980,6 +987,56 @@ 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, mask, mode = 0;
+

Please define each variable in separate line to make it clean. Please check for IS_ERR(gmii_sel_reg) and return if gmii register is not specified in DT then the below code will crash when trying to access gmii_sel_reg.

Quoted 7 lines
drivers/net/ethernet/ti/cpsw.c +7 -0
+	switch (priv->data.hw_type) {
+	case CPSW_TYPE_AM33XX:
+		if (!priv->gmii_sel_reg)
+			break;
+
+		reg = readl(priv->gmii_sel_reg);
+

drivers/net/ethernet/ti/cpsw.c static inline void cpsw_add_dual_emac_def_ale_entries( · line 1011

Quoted 10 lines
drivers/net/ethernet/ti/cpsw.c +10 -0
+		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:

You need to take care of other RGMII modes as well, for your info AM335xevm phy mode is "rgmii-txid"

Regards
Mugunthan V N

#8 lore raw

PATCH v2 5/5 ARM: dts: am33xx: adopt to cpsw changes

In reply to 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.

---
 arch/arm/boot/dts/am33xx.dtsi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
arch/arm/boot/dts/am33xx.dtsi +3 -2
index a785b95..1a1e9dd 100644
@@ -446,7 +446,7 @@
 		};
 
 		mac: ethernet at 4a100000 {
-			compatible = "ti,cpsw";
+			compatible = "ti,am3352-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>;
Signature · 1 line
1.8.3.1
#9 lore raw

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

Quoted 17 lines · 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.

---
 arch/arm/boot/dts/am33xx.dtsi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
arch/arm/boot/dts/am33xx.dtsi +0 -0
index a785b95..1a1e9dd 100644

arch/arm/boot/dts/am33xx.dtsi line 449

Quoted 5 lines
arch/arm/boot/dts/am33xx.dtsi +1 -1
@@ -446,7 +446,7 @@
 		};
 
 		mac: ethernet at 4a100000 {
-			compatible = "ti,cpsw";
+			compatible = "ti,am3352-cpsw";

Please keep this as

	compatible = "ti,am3352-cpsw", "ti,cpsw";

so users can upgrade the DTB or kernel independently. Apart from this the series looks good to me.

Thanks,
Sekhar

#10 lore raw

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

Quoted 26 lines · Sekhar Nori
Quoted 17 lines · 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.

---
 arch/arm/boot/dts/am33xx.dtsi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
arch/arm/boot/dts/am33xx.dtsi +0 -0
index a785b95..1a1e9dd 100644

arch/arm/boot/dts/am33xx.dtsi line 449

Quoted 5 lines
arch/arm/boot/dts/am33xx.dtsi +1 -1
@@ -446,7 +446,7 @@
 		};
 
 		mac: ethernet at 4a100000 {
-			compatible = "ti,cpsw";
+			compatible = "ti,am3352-cpsw";

Please keep this as

Quoted 4 lines
	compatible = "ti,am3352-cpsw", "ti,cpsw";

so users can upgrade the DTB or kernel independently. Apart from this the series looks good to me.

Ok, thanks. Given that this last patch will be merged through another tree, can I just resend it separately, while David takes the rest as it is?

Thanks,
Daniel

#11 lore raw

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

Quoted 27 lines · Daniel Mack
Quoted 26 lines · Sekhar Nori
Quoted 17 lines · 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.

---
 arch/arm/boot/dts/am33xx.dtsi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
arch/arm/boot/dts/am33xx.dtsi +0 -0
index a785b95..1a1e9dd 100644

arch/arm/boot/dts/am33xx.dtsi line 449

Quoted 5 lines
arch/arm/boot/dts/am33xx.dtsi +1 -1
@@ -446,7 +446,7 @@
 		};
 
 		mac: ethernet at 4a100000 {
-			compatible = "ti,cpsw";
+			compatible = "ti,am3352-cpsw";

Please keep this as

Quoted 7 lines
Quoted 4 lines
	compatible = "ti,am3352-cpsw", "ti,cpsw";

so users can upgrade the DTB or kernel independently. Apart from this the series looks good to me.

Ok, thanks. Given that this last patch will be merged through another tree, can I just resend it separately, while David takes the rest as it is?

Ideally you could, but there are some bugs in the way compatible string in handled and due to this I suspect you need to reorder cpsw_of_mtable[] to match correctly. Not sure if the bug has been fixed in recent -rcs. Please check if modifying just this patch works and then go ahead and post.

Please Cc Benoit Cousson <[email protected]> on your next submission as he is the one who picks up DT patches for OMAP.

Thanks,
Sekhar

#12 lore raw

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

Quoted 11 lines · Sekhar Nori
Quoted 4 lines · Daniel Mack

On 23.08.2013 11:28, Sekhar Nori wrote:

Ok, thanks. Given that this last patch will be merged through another tree, can I just resend it separately, while David takes the rest as it is?

Ideally you could, but there are some bugs in the way compatible string in handled and due to this I suspect you need to reorder cpsw_of_mtable[] to match correctly. Not sure if the bug has been fixed in recent -rcs. Please check if modifying just this patch works and then go ahead and post.

No, you're right. I had to swap the two entries. Thanks for mentioning it.

Quoted 2 lines

Please Cc Benoit Cousson <[email protected]> on your next submission as he is the one who picks up DT patches for OMAP.

Will do.

Daniel