[PATCH v4 0/9] ARM: Berlin: Ethernet support

[PATCH v4 0/9] ARM: Berlin: Ethernet support

[Antoine Tenart]

Hi all,

This series introduce support for the Ethernet controller on Berlin SoCs,
using the existing pxa168 Ethernet driver. In order to do this, DT
support is added to the driver alongside some other modifications and
fixes.

This has been tested on a Berlin BG2Q DMP board.

Changes since v3:
	- moved the addition of pxa168_eth_get_mac_address() to the patch
	  using it first

Changes since v2:
        - reworked how the MAC address is configured
        - made the clock anonymous

Changes since v1:
        - removed custom Berlin Ethernet driver
        - used the pxa168 Ethernet driver instead
        - made modifications to the pxa168 driver (DT support, fixes)

Antoine Tenart (9):
  net: pxa168_eth: clean up
  net: pxa168_eth: add device tree support
  Documentation: bindings: net: add the Marvell PXA168 Ethernet
    controller
  net: pxa168_eth: fix Ethernet flow control status
  net: pxa168_eth: set the mac address on the Ethernet controller
  net: pxa168_eth: rework the MAC address setup
  net: pxa168_eth: allow Berlin SoCs to use the pxa168_eth driver
  ARM: dts: berlin: add the Ethernet node
  ARM: dts: berlin: enable the Ethernet port on the BG2Q DMP

 .../devicetree/bindings/net/marvell-pxa168.txt     |  21 ++
 arch/arm/boot/dts/berlin2q-marvell-dmp.dts         |   4 +
 arch/arm/boot/dts/berlin2q.dtsi                    |  10 +
 drivers/net/ethernet/marvell/Kconfig               |   2 +-
 drivers/net/ethernet/marvell/pxa168_eth.c          | 216 +++++++++++++--------
 5 files changed, 174 insertions(+), 79 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/marvell-pxa168.txt

-- 
1.9.1

[PATCH v4 1/9] net: pxa168_eth: clean up

[Antoine Tenart]

Clean up a bit the pxa168_eth driver before adding the device tree
support.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/pxa168_eth.c | 102 +++++++++++++++---------------
 1 file changed, 50 insertions(+), 52 deletions(-)

diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
index 8f5aa7c62b18..b370162dbe02 100644
--- a/drivers/net/ethernet/marvell/pxa168_eth.c
+++ b/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -22,27 +22,29 @@
  * along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <linux/dma-mapping.h>
-#include <linux/in.h>
-#include <linux/ip.h>
-#include <linux/tcp.h>
-#include <linux/udp.h>
-#include <linux/etherdevice.h>
 #include <linux/bitops.h>
+#include <linux/clk.h>
 #include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/etherdevice.h>
 #include <linux/ethtool.h>
-#include <linux/platform_device.h>
-#include <linux/module.h>
+#include <linux/in.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/ip.h>
 #include <linux/kernel.h>
-#include <linux/workqueue.h>
-#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/of.h>
 #include <linux/phy.h>
-#include <linux/io.h>
-#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/pxa168_eth.h>
+#include <linux/tcp.h>
 #include <linux/types.h>
+#include <linux/udp.h>
+#include <linux/workqueue.h>
+
 #include <asm/pgtable.h>
 #include <asm/cacheflush.h>
-#include <linux/pxa168_eth.h>
 
 #define DRIVER_NAME	"pxa168-eth"
 #define DRIVER_VERSION	"0.3"
@@ -296,7 +298,7 @@ static void abort_dma(struct pxa168_eth_private *pep)
 	} while (max_retries-- > 0 && delay <= 0);
 
 	if (max_retries <= 0)
-		printk(KERN_ERR "%s : DMA Stuck\n", __func__);
+		netdev_err(pep->dev, "%s : DMA Stuck\n", __func__);
 }
 
 static int ethernet_phy_get(struct pxa168_eth_private *pep)
@@ -507,9 +509,10 @@ static int add_del_hash_entry(struct pxa168_eth_private *pep,
 
 	if (i == HOP_NUMBER) {
 		if (!del) {
-			printk(KERN_INFO "%s: table section is full, need to "
-					"move to 16kB implementation?\n",
-					 __FILE__);
+			netdev_info(pep->dev,
+				    "%s: table section is full, need to "
+				    "move to 16kB implementation?\n",
+				    __FILE__);
 			return -ENOSPC;
 		} else
 			return 0;
@@ -726,7 +729,7 @@ static int txq_reclaim(struct net_device *dev, int force)
 
 		if (cmd_sts & TX_ERROR) {
 			if (net_ratelimit())
-				printk(KERN_ERR "%s: Error in TX\n", dev->name);
+				netdev_err(dev, "Error in TX\n");
 			dev->stats.tx_errors++;
 		}
 		dma_unmap_single(NULL, addr, count, DMA_TO_DEVICE);
@@ -743,8 +746,7 @@ static void pxa168_eth_tx_timeout(struct net_device *dev)
 {
 	struct pxa168_eth_private *pep = netdev_priv(dev);
 
-	printk(KERN_INFO "%s: TX timeout  desc_count %d\n",
-	       dev->name, pep->tx_desc_count);
+	netdev_info(dev, "TX timeout  desc_count %d\n", pep->tx_desc_count);
 
 	schedule_work(&pep->tx_timeout_task);
 }
@@ -814,9 +816,8 @@ static int rxq_process(struct net_device *dev, int budget)
 			if ((cmd_sts & (RX_FIRST_DESC | RX_LAST_DESC)) !=
 			    (RX_FIRST_DESC | RX_LAST_DESC)) {
 				if (net_ratelimit())
-					printk(KERN_ERR
-					       "%s: Rx pkt on multiple desc\n",
-					       dev->name);
+					netdev_err(dev,
+						   "Rx pkt on multiple desc\n");
 			}
 			if (cmd_sts & RX_ERROR)
 				stats->rx_errors++;
@@ -871,7 +872,7 @@ static void handle_link_event(struct pxa168_eth_private *pep)
 	port_status = rdl(pep, PORT_STATUS);
 	if (!(port_status & LINK_UP)) {
 		if (netif_carrier_ok(dev)) {
-			printk(KERN_INFO "%s: link down\n", dev->name);
+			netdev_info(dev, "link down\n");
 			netif_carrier_off(dev);
 			txq_reclaim(dev, 1);
 		}
@@ -884,9 +885,8 @@ static void handle_link_event(struct pxa168_eth_private *pep)
 
 	duplex = (port_status & FULL_DUPLEX) ? 1 : 0;
 	fc = (port_status & FLOW_CONTROL_ENABLED) ? 1 : 0;
-	printk(KERN_INFO "%s: link up, %d Mb/s, %s duplex, "
-	       "flow control %sabled\n", dev->name,
-	       speed, duplex ? "full" : "half", fc ? "en" : "dis");
+	netdev_info(dev, "link up, %d Mb/s, %s duplex, flow control %sabled\n",
+		    speed, duplex ? "full" : "half", fc ? "en" : "dis");
 	if (!netif_carrier_ok(dev))
 		netif_carrier_on(dev);
 }
@@ -1039,9 +1039,8 @@ static void rxq_deinit(struct net_device *dev)
 		}
 	}
 	if (pep->rx_desc_count)
-		printk(KERN_ERR
-		       "Error in freeing Rx Ring. %d skb's still\n",
-		       pep->rx_desc_count);
+		netdev_err(dev, "Error in freeing Rx Ring. %d skb's still\n",
+			   pep->rx_desc_count);
 	/* Free RX ring */
 	if (pep->p_rx_desc_area)
 		dma_free_coherent(pep->dev->dev.parent, pep->rx_desc_area_size,
@@ -1280,15 +1279,15 @@ static int pxa168_smi_read(struct mii_bus *bus, int phy_addr, int regnum)
 	int val;
 
 	if (smi_wait_ready(pep)) {
-		printk(KERN_WARNING "pxa168_eth: SMI bus busy timeout\n");
+		netdev_warn(pep->dev, "pxa168_eth: SMI bus busy timeout\n");
 		return -ETIMEDOUT;
 	}
 	wrl(pep, SMI, (phy_addr << 16) | (regnum << 21) | SMI_OP_R);
 	/* now wait for the data to be valid */
 	for (i = 0; !((val = rdl(pep, SMI)) & SMI_R_VALID); i++) {
 		if (i == PHY_WAIT_ITERATIONS) {
-			printk(KERN_WARNING
-				"pxa168_eth: SMI bus read not valid\n");
+			netdev_warn(pep->dev,
+				    "pxa168_eth: SMI bus read not valid\n");
 			return -ENODEV;
 		}
 		msleep(10);
@@ -1303,7 +1302,7 @@ static int pxa168_smi_write(struct mii_bus *bus, int phy_addr, int regnum,
 	struct pxa168_eth_private *pep = bus->priv;
 
 	if (smi_wait_ready(pep)) {
-		printk(KERN_WARNING "pxa168_eth: SMI bus busy timeout\n");
+		netdev_warn(pep->dev, "pxa168_eth: SMI bus busy timeout\n");
 		return -ETIMEDOUT;
 	}
 
@@ -1311,7 +1310,7 @@ static int pxa168_smi_write(struct mii_bus *bus, int phy_addr, int regnum,
 	    SMI_OP_W | (value & 0xffff));
 
 	if (smi_wait_ready(pep)) {
-		printk(KERN_ERR "pxa168_eth: SMI bus busy timeout\n");
+		netdev_err(pep->dev, "pxa168_eth: SMI bus busy timeout\n");
 		return -ETIMEDOUT;
 	}
 
@@ -1425,23 +1424,23 @@ static void pxa168_get_drvinfo(struct net_device *dev,
 }
 
 static const struct ethtool_ops pxa168_ethtool_ops = {
-	.get_settings = pxa168_get_settings,
-	.set_settings = pxa168_set_settings,
-	.get_drvinfo = pxa168_get_drvinfo,
-	.get_link = ethtool_op_get_link,
-	.get_ts_info = ethtool_op_get_ts_info,
+	.get_settings	= pxa168_get_settings,
+	.set_settings	= pxa168_set_settings,
+	.get_drvinfo	= pxa168_get_drvinfo,
+	.get_link	= ethtool_op_get_link,
+	.get_ts_info	= ethtool_op_get_ts_info,
 };
 
 static const struct net_device_ops pxa168_eth_netdev_ops = {
-	.ndo_open = pxa168_eth_open,
-	.ndo_stop = pxa168_eth_stop,
-	.ndo_start_xmit = pxa168_eth_start_xmit,
-	.ndo_set_rx_mode = pxa168_eth_set_rx_mode,
-	.ndo_set_mac_address = pxa168_eth_set_mac_address,
-	.ndo_validate_addr = eth_validate_addr,
-	.ndo_do_ioctl = pxa168_eth_do_ioctl,
-	.ndo_change_mtu = pxa168_eth_change_mtu,
-	.ndo_tx_timeout = pxa168_eth_tx_timeout,
+	.ndo_open		= pxa168_eth_open,
+	.ndo_stop		= pxa168_eth_stop,
+	.ndo_start_xmit		= pxa168_eth_start_xmit,
+	.ndo_set_rx_mode	= pxa168_eth_set_rx_mode,
+	.ndo_set_mac_address	= pxa168_eth_set_mac_address,
+	.ndo_validate_addr	= eth_validate_addr,
+	.ndo_do_ioctl		= pxa168_eth_do_ioctl,
+	.ndo_change_mtu		= pxa168_eth_change_mtu,
+	.ndo_tx_timeout		= pxa168_eth_tx_timeout,
 };
 
 static int pxa168_eth_probe(struct platform_device *pdev)
@@ -1456,8 +1455,7 @@ static int pxa168_eth_probe(struct platform_device *pdev)
 
 	clk = clk_get(&pdev->dev, "MFUCLK");
 	if (IS_ERR(clk)) {
-		printk(KERN_ERR "%s: Fast Ethernet failed to get clock\n",
-			DRIVER_NAME);
+		dev_err(&pdev->dev, "Fast Ethernet failed to get clock\n");
 		return -ENODEV;
 	}
 	clk_enable(clk);
@@ -1492,7 +1490,7 @@ static int pxa168_eth_probe(struct platform_device *pdev)
 
 	INIT_WORK(&pep->tx_timeout_task, pxa168_eth_tx_timeout_task);
 
-	printk(KERN_INFO "%s:Using random mac address\n", DRIVER_NAME);
+	dev_info(&pdev->dev, "Using random mac address\n");
 	eth_hw_addr_random(dev);
 
 	pep->pd = dev_get_platdata(&pdev->dev);
-- 
1.9.1

[PATCH v4 2/9] net: pxa168_eth: add device tree support

[Antoine Tenart]

Add the device tree support to the pxa168_eth driver.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/pxa168_eth.c | 67 ++++++++++++++++++++-----------
 1 file changed, 44 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
index b370162dbe02..a27cd85c030d 100644
--- a/drivers/net/ethernet/marvell/pxa168_eth.c
+++ b/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -193,6 +193,7 @@ struct tx_desc {
 
 struct pxa168_eth_private {
 	int port_num;		/* User Ethernet port number    */
+	int phy_addr;
 
 	int rx_resource_err;	/* Rx ring resource error flag */
 
@@ -1360,24 +1361,25 @@ static struct phy_device *phy_scan(struct pxa168_eth_private *pep, int phy_addr)
 	return phydev;
 }
 
-static void phy_init(struct pxa168_eth_private *pep, int speed, int duplex)
+static void phy_init(struct pxa168_eth_private *pep)
 {
 	struct phy_device *phy = pep->phy;
 
 	phy_attach(pep->dev, dev_name(&phy->dev), PHY_INTERFACE_MODE_MII);
 
-	if (speed == 0) {
+	if (pep->pd && pep->pd->speed != 0) {
+		phy->autoneg = AUTONEG_DISABLE;
+		phy->advertising = 0;
+		phy->speed = pep->pd->speed;
+		phy->duplex = pep->pd->duplex;
+	} else {
 		phy->autoneg = AUTONEG_ENABLE;
 		phy->speed = 0;
 		phy->duplex = 0;
 		phy->supported &= PHY_BASIC_FEATURES;
 		phy->advertising = phy->supported | ADVERTISED_Autoneg;
-	} else {
-		phy->autoneg = AUTONEG_DISABLE;
-		phy->advertising = 0;
-		phy->speed = speed;
-		phy->duplex = duplex;
 	}
+
 	phy_start_aneg(phy);
 }
 
@@ -1385,11 +1387,13 @@ static int ethernet_phy_setup(struct net_device *dev)
 {
 	struct pxa168_eth_private *pep = netdev_priv(dev);
 
-	if (pep->pd->init)
+	if (pep->pd && pep->pd->init)
 		pep->pd->init();
-	pep->phy = phy_scan(pep, pep->pd->phy_addr & 0x1f);
+
+	pep->phy = phy_scan(pep, pep->phy_addr & 0x1f);
 	if (pep->phy != NULL)
-		phy_init(pep, pep->pd->speed, pep->pd->duplex);
+		phy_init(pep);
+
 	update_hash_table_mac_address(pep, NULL, dev->dev_addr);
 
 	return 0;
@@ -1453,12 +1457,12 @@ static int pxa168_eth_probe(struct platform_device *pdev)
 
 	printk(KERN_NOTICE "PXA168 10/100 Ethernet Driver\n");
 
-	clk = clk_get(&pdev->dev, "MFUCLK");
+	clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(clk)) {
 		dev_err(&pdev->dev, "Fast Ethernet failed to get clock\n");
 		return -ENODEV;
 	}
-	clk_enable(clk);
+	clk_prepare_enable(clk);
 
 	dev = alloc_etherdev(sizeof(struct pxa168_eth_private));
 	if (!dev) {
@@ -1475,8 +1479,8 @@ static int pxa168_eth_probe(struct platform_device *pdev)
 		err = -ENODEV;
 		goto err_netdev;
 	}
-	pep->base = ioremap(res->start, resource_size(res));
-	if (pep->base == NULL) {
+	pep->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(pep->base)) {
 		err = -ENOMEM;
 		goto err_netdev;
 	}
@@ -1493,16 +1497,26 @@ static int pxa168_eth_probe(struct platform_device *pdev)
 	dev_info(&pdev->dev, "Using random mac address\n");
 	eth_hw_addr_random(dev);
 
-	pep->pd = dev_get_platdata(&pdev->dev);
 	pep->rx_ring_size = NUM_RX_DESCS;
-	if (pep->pd->rx_queue_size)
-		pep->rx_ring_size = pep->pd->rx_queue_size;
-
 	pep->tx_ring_size = NUM_TX_DESCS;
-	if (pep->pd->tx_queue_size)
-		pep->tx_ring_size = pep->pd->tx_queue_size;
 
-	pep->port_num = pep->pd->port_number;
+	pep->pd = dev_get_platdata(&pdev->dev);
+	if (pep->pd) {
+		if (pep->pd->rx_queue_size)
+			pep->rx_ring_size = pep->pd->rx_queue_size;
+
+		if (pep->pd->tx_queue_size)
+			pep->tx_ring_size = pep->pd->tx_queue_size;
+
+		pep->port_num = pep->pd->port_number;
+		pep->phy_addr = pep->pd->phy_addr;
+	} else if (pdev->dev.of_node) {
+		of_property_read_u32(pdev->dev.of_node, "port-id",
+				     &pep->port_num);
+		of_property_read_u32(pdev->dev.of_node, "phy-addr",
+				     &pep->phy_addr);
+	}
+
 	/* Hardware supports only 3 ports */
 	BUG_ON(pep->port_num > 2);
 	netif_napi_add(dev, &pep->napi, pxa168_rx_poll, pep->rx_ring_size);
@@ -1603,6 +1617,12 @@ static int pxa168_eth_suspend(struct platform_device *pdev, pm_message_t state)
 #define pxa168_eth_suspend NULL
 #endif
 
+static const struct of_device_id pxa168_eth_of_match[] = {
+	{ .compatible = "marvell,pxa168-eth" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, pxa168_eth_of_match)
+
 static struct platform_driver pxa168_eth_driver = {
 	.probe = pxa168_eth_probe,
 	.remove = pxa168_eth_remove,
@@ -1610,8 +1630,9 @@ static struct platform_driver pxa168_eth_driver = {
 	.resume = pxa168_eth_resume,
 	.suspend = pxa168_eth_suspend,
 	.driver = {
-		   .name = DRIVER_NAME,
-		   },
+		.name		= DRIVER_NAME,
+		.of_match_table	= of_match_ptr(pxa168_eth_of_match),
+	},
 };
 
 module_platform_driver(pxa168_eth_driver);
-- 
1.9.1

[PATCH v4 3/9] Documentation: bindings: net: add the Marvell PXA168 Ethernet controller

[Antoine Tenart]

This adds the binding documentation for the Marvell PXA168 Ethernet
controller, following its DT support.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 .../devicetree/bindings/net/marvell-pxa168.txt      | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/marvell-pxa168.txt

diff --git a/Documentation/devicetree/bindings/net/marvell-pxa168.txt b/Documentation/devicetree/bindings/net/marvell-pxa168.txt
new file mode 100644
index 000000000000..4177abf3dac3
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/marvell-pxa168.txt
@@ -0,0 +1,21 @@
+* Marvell PXA168 Ethernet Controller
+
+Required properties:
+- compatible: should be "marvell,pxa168-eth".
+- reg: address and length of the register set for the device.
+- interrupts: interrupt for the device.
+- clocks: pointer to the clock for the device.
+
+Optional properties:
+- port-id: should be '0','1' or '2'.
+- phy-addr: MDIO address of the PHY.
+- local-mac-address: see ethernet.txt file in the same directory.
+
+Example:
+
+	eth0: ethernet@f7b90000 {
+		compatible = "marvell,pxa168-eth";
+		reg = <0xf7b90000 0x10000>;
+		clocks = <&chip CLKID_GETH0>;
+		interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
+	};
-- 
1.9.1

Re: [PATCH v4 3/9] Documentation: bindings: net: add the Marvell PXA168 Ethernet controller

[Arnd Bergmann]

On Tuesday 23 September 2014 14:15:30 Antoine Tenart wrote:
> +Optional properties:
> +- port-id: should be '0','1' or '2'.
> +- phy-addr: MDIO address of the PHY.
> +- local-mac-address: see ethernet.txt file in the same directory.
> +
> 

I believe new bindings should not use "phy-addr" properties, but
instead use a phy-handle property pointing to a phy device instead.

The port-id property description could be a little more verbose.
What do those numbers actually mean?

	Arnd

Re: [PATCH v4 3/9] Documentation: bindings: net: add the Marvell PXA168 Ethernet controller

[Antoine Tenart]

Arnd,

On Tue, Sep 23, 2014 at 02:38:39PM +0200, Arnd Bergmann wrote:
> On Tuesday 23 September 2014 14:15:30 Antoine Tenart wrote:
> > +Optional properties:
> > +- port-id: should be '0','1' or '2'.
> > +- phy-addr: MDIO address of the PHY.
> > +- local-mac-address: see ethernet.txt file in the same directory.
> > +
> > 
> 
> I believe new bindings should not use "phy-addr" properties, but
> instead use a phy-handle property pointing to a phy device instead.

That's the MDIO address of the PHY. It's a bit difficult to use a phy
device as the driver uses the libphy. And that would break the platform
using it.

Or maybe you have something in mind?

> 
> The port-id property description could be a little more verbose.
> What do those numbers actually mean?

I'll add some description. That's the Ethernet port number. I reused a
property (port-id) already used by other drivers.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/9] Documentation: bindings: net: add the Marvell PXA168 Ethernet controller

[Antoine Tenart]

On Tue, Sep 23, 2014 at 04:01:13PM +0200, Antoine Tenart wrote:
> Arnd,
> 
> On Tue, Sep 23, 2014 at 02:38:39PM +0200, Arnd Bergmann wrote:
> > On Tuesday 23 September 2014 14:15:30 Antoine Tenart wrote:
> > > +Optional properties:
> > > +- port-id: should be '0','1' or '2'.
> > > +- phy-addr: MDIO address of the PHY.
> > > +- local-mac-address: see ethernet.txt file in the same directory.
> > > +
> > > 
> > 
> > I believe new bindings should not use "phy-addr" properties, but
> > instead use a phy-handle property pointing to a phy device instead.
> 

So I had a look on other Ethernet bindings. Would you agree with
something like the following?

	eth0: ethernet@f7b90000 {
		...
		#address-cells = <1>;
		#size-cells = <0>;
		phy-handle = <&ethphy0>;

		ethphy0: ethernet-phy@0 {
			reg = <0>;
		};
	};

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH v4 3/9] Documentation: bindings: net: add the Marvell PXA168 Ethernet controller

[Arnd Bergmann]

On Tuesday 23 September 2014 16:29:22 Antoine Tenart wrote:
> 
> So I had a look on other Ethernet bindings. Would you agree with
> something like the following?
> 
>         eth0: ethernet@f7b90000 {
>                 ...
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>                 phy-handle = <&ethphy0>;
> 
>                 ethphy0: ethernet-phy@0 {
>                         reg = <0>;
>                 };
>         };

Yes, that looks good.

	Arnd

Re: [PATCH v4 3/9] Documentation: bindings: net: add the Marvell PXA168 Ethernet controller

[Sebastian Hesselbarth]

On 09/23/2014 04:37 PM, Arnd Bergmann wrote:
> On Tuesday 23 September 2014 16:29:22 Antoine Tenart wrote:
>>
>> So I had a look on other Ethernet bindings. Would you agree with
>> something like the following?
>>
>>          eth0: ethernet@f7b90000 {
>>                  ...
>>                  #address-cells = <1>;
>>                  #size-cells = <0>;
>>                  phy-handle = <&ethphy0>;
>>
>>                  ethphy0: ethernet-phy@0 {
>>                          reg = <0>;
>>                  };
>>          };
>
> Yes, that looks good.

nit: ethernet-phy@0 should not be part of the ethernet controller
but either part of a separate mdio-ctrl node or a separate node itself.

AFAIKS, Berlin SoCs have internal PHYs, i.e. no MII/MDIO-pins exposed
but only MLT-3 differential pairs. At least BG2CD also can connect its
ethernet IP to a PHY capable of HDMI Ethernet Channel (HEC).

So for BG2CD this has to look something like:

eth0: ethernet@f7b90000 {
	...
	phy-handle = <&hec>;
};

cec: cec@f7f00baa {
	compatible = "marvell,berlin-cec";
	reg = <0xf7f00baa 0x1234>;

	/* CEC unit contains HEC PHY */
	hec: ethernet-phy {
		bla;
	};
}

For reference, this is what we have for MVEBU SoCs with multiple ports
per controller:

eth: ethernet-ctrl@72000 {
	compatible = "marvell,orion-eth";
	#address-cells = <1>;
	#size-cells = <0>;
	reg = <0x72000 0x4000>;
	clocks = <&gate_clk 2>;
	marvell,tx-checksum-limit = <1600>;
	status = "disabled";

	ethernet-port@0 {
		compatible = "marvell,orion-eth-port";
		reg = <0>;
		interrupts = <29>;
		/* overwrite MAC address in bootloader */
		local-mac-address = [00 00 00 00 00 00];
		phy-handle = <&ethphy>;
	};
};

mdio: mdio-bus@72004 {
	compatible = "marvell,orion-mdio";
	#address-cells = <1>;
	#size-cells = <0>;
	reg = <0x72004 0x84>;
	interrupts = <30>;
	clocks = <&gate_clk 2>;
	status = "disabled";
	ethphy: ethernet-phy {
		/* set phy address in board file */
	};
};

Sebastian

Re: [PATCH v4 3/9] Documentation: bindings: net: add the Marvell PXA168 Ethernet controller

[Arnd Bergmann]

On Tuesday 23 September 2014 17:45:52 Sebastian Hesselbarth wrote:
> For reference, this is what we have for MVEBU SoCs with multiple ports
> per controller:
> 
> eth: ethernet-ctrl@72000 {
>         compatible = "marvell,orion-eth";
>         #address-cells = <1>;
>         #size-cells = <0>;
>         reg = <0x72000 0x4000>;
>         clocks = <&gate_clk 2>;
>         marvell,tx-checksum-limit = <1600>;
>         status = "disabled";
> 
>         ethernet-port@0 {
>                 compatible = "marvell,orion-eth-port";
>                 reg = <0>;
>                 interrupts = <29>;
>                 /* overwrite MAC address in bootloader */
>                 local-mac-address = [00 00 00 00 00 00];
>                 phy-handle = <&ethphy>;
>         };
> };
> 
> mdio: mdio-bus@72004 {
>         compatible = "marvell,orion-mdio";
>         #address-cells = <1>;
>         #size-cells = <0>;
>         reg = <0x72004 0x84>;
>         interrupts = <30>;
>         clocks = <&gate_clk 2>;
>         status = "disabled";
>         ethphy: ethernet-phy {
>                 /* set phy address in board file */
>         };
> };
> 

But in this example, you have the same registers and the same
clocks in two nodes, which are even used by the same device driver
at the moment. It's not a big issue, but my feeling is that Antoine's
approach was actually better because it more closely reflects
the way that the hardware is built.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/9] Documentation: bindings: net: add the Marvell PXA168 Ethernet controller

[Sebastian Hesselbarth]

On 09/23/2014 06:29 PM, Arnd Bergmann wrote:
> On Tuesday 23 September 2014 17:45:52 Sebastian Hesselbarth wrote:
>> For reference, this is what we have for MVEBU SoCs with multiple ports
>> per controller:
>>
>> eth: ethernet-ctrl@72000 {
>>          compatible = "marvell,orion-eth";
...
>>          reg = <0x72000 0x4000>;
...
>>
>>          ethernet-port@0 {
>>                  compatible = "marvell,orion-eth-port";
...
>>                  phy-handle = <&ethphy>;
>>          };
>> };
>>
>> mdio: mdio-bus@72004 {
>>          compatible = "marvell,orion-mdio";
...
>>          reg = <0x72004 0x84>;
..
>>          ethphy: ethernet-phy {
>>                  /* set phy address in board file */
>>          };
>> };

> But in this example, you have the same registers and the same
> clocks in two nodes, which are even used by the same device driver
> at the moment. It's not a big issue, but my feeling is that Antoine's
> approach was actually better because it more closely reflects
> the way that the hardware is built.

I was not referring to the separate mdio bus node, but putting the
ethernet-phy node as a child of ethernet-ctrl.

Anyway, I can live with the ethernet-phy being a child of the controller
node until we discover where it is hooked up.

For the internal MII PHY the controller node maybe is the only sane
place to put it in. The HEC PHY will reside within the CEC IP node but
that is compatible with Antoine's proposal.

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/9] Documentation: bindings: net: add the Marvell PXA168 Ethernet controller

[Arnd Bergmann]

On Tuesday 23 September 2014 18:40:46 Sebastian Hesselbarth wrote:
> On 09/23/2014 06:29 PM, Arnd Bergmann wrote:
> > On Tuesday 23 September 2014 17:45:52 Sebastian Hesselbarth wrote:
> >> For reference, this is what we have for MVEBU SoCs with multiple ports
> >> per controller:
> >>
> >> eth: ethernet-ctrl@72000 {
> >>          compatible = "marvell,orion-eth";
> ...
> >>          reg = <0x72000 0x4000>;
> ...
> >>
> >>          ethernet-port@0 {
> >>                  compatible = "marvell,orion-eth-port";
> ...
> >>                  phy-handle = <&ethphy>;
> >>          };
> >> };
> >>
> >> mdio: mdio-bus@72004 {
> >>          compatible = "marvell,orion-mdio";
> ...
> >>          reg = <0x72004 0x84>;
> ..
> >>          ethphy: ethernet-phy {
> >>                  /* set phy address in board file */
> >>          };
> >> };
> 
> > But in this example, you have the same registers and the same
> > clocks in two nodes, which are even used by the same device driver
> > at the moment. It's not a big issue, but my feeling is that Antoine's
> > approach was actually better because it more closely reflects
> > the way that the hardware is built.
> 
> I was not referring to the separate mdio bus node, but putting the
> ethernet-phy node as a child of ethernet-ctrl.

Ah, got it (I think). Yes, that makes sense.

The part I don't understand yet is how one uses multiple ports. pxa168_eth.c
seems to be written with the assumption that only one port is ever used at
a time, while mv643xx_eth.c can actually use multiple ports simultaneously.

Do you think that is that a hardware limitation of pxa168_eth or a feature
that nobody so far has needed from the driver?

If there is only one port and we just have to know which one that is,
I don't think we need the child nodes, but if one can have multiple
ports operate independently then the driver will need a rework 
to actually be usable with that configuration.


	Arnd

Re: [PATCH v4 3/9] Documentation: bindings: net: add the Marvell PXA168 Ethernet controller

[Sebastian Hesselbarth]

On 09/23/2014 07:02 PM, Arnd Bergmann wrote:
> On Tuesday 23 September 2014 18:40:46 Sebastian Hesselbarth wrote:
>> On 09/23/2014 06:29 PM, Arnd Bergmann wrote:
>>> On Tuesday 23 September 2014 17:45:52 Sebastian Hesselbarth wrote:
>>>> For reference, this is what we have for MVEBU SoCs with multiple ports
>>>> per controller:
>>>>
>>>> eth: ethernet-ctrl@72000 {
>>>>           compatible = "marvell,orion-eth";
>> ...
>>>>           reg = <0x72000 0x4000>;
>> ...
>>>>
>>>>           ethernet-port@0 {
>>>>                   compatible = "marvell,orion-eth-port";
>> ...
>>>>                   phy-handle = <&ethphy>;
>>>>           };
>>>> };
>>>>
>>>> mdio: mdio-bus@72004 {
>>>>           compatible = "marvell,orion-mdio";
>> ...
>>>>           reg = <0x72004 0x84>;
>> ..
>>>>           ethphy: ethernet-phy {
>>>>                   /* set phy address in board file */
>>>>           };
>>>> };
>>
>>> But in this example, you have the same registers and the same
>>> clocks in two nodes, which are even used by the same device driver
>>> at the moment. It's not a big issue, but my feeling is that Antoine's
>>> approach was actually better because it more closely reflects
>>> the way that the hardware is built.
>>
>> I was not referring to the separate mdio bus node, but putting the
>> ethernet-phy node as a child of ethernet-ctrl.
>
> Ah, got it (I think). Yes, that makes sense.

> The part I don't understand yet is how one uses multiple ports. pxa168_eth.c
> seems to be written with the assumption that only one port is ever used at
> a time, while mv643xx_eth.c can actually use multiple ports simultaneously.
>
> Do you think that is that a hardware limitation of pxa168_eth or a feature
> that nobody so far has needed from the driver?

It has been a while I looked it up in the pxa168 datasheet, but IIRC
there is only one port per controller. FWIW, there actually is also
just one port per controller for Orion SoCs. The multiple ports per
controller comes from the PPC system controllers, i.e. mv643xx hence
the name. We just made the binding look like it is more ports available
to make it backwards compatible.. although I doubt anyone is still using
mv643xx anywhere.

> If there is only one port and we just have to know which one that is,
> I don't think we need the child nodes, but if one can have multiple
> ports operate independently then the driver will need a rework
> to actually be usable with that configuration.

I doubt pxa168 needs port nodes at all, i.e. we have the phy-handle
directly as property of the controller node.

The HEC PHY node itself will be a sub-node of some future CEC node,
while the (internal) MII PHY node can stay as a sub-node of the
controller, e.g.

(one final example to make sure we agree on the same)

eth0: ethernet@f7b90000 {
	compatible = "marvell,pxa168-eth";
	reg = <...>;
	#address-cells = <1>;
	#size-cells = <0>;
	phy-handle = <&ethphy0>;

	ethphy0: ethernet-phy@0 {
		reg = <0>;
	};
};

cec0: cec@f7f00baa {
	compatible = "marvell,berlin-cec";
	reg = <...>;
	#address-cells = <1>;
	#size-cells = <0>;
	
	hecphy0: hdmi-ethernet-phy@0 {
		reg = <0>;
	};
};

With berlin2cd-google-chromecast.dts overwriting

&eth0 { phy-handle = <&hecphy0>; };

Sebastian

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/9] Documentation: bindings: net: add the Marvell PXA168 Ethernet controller

[Arnd Bergmann]

On Tuesday 23 September 2014 19:31:40 Sebastian Hesselbarth wrote:
> It has been a while I looked it up in the pxa168 datasheet, but IIRC
> there is only one port per controller. FWIW, there actually is also
> just one port per controller for Orion SoCs. The multiple ports per
> controller comes from the PPC system controllers, i.e. mv643xx hence
> the name. We just made the binding look like it is more ports available
> to make it backwards compatible.. although I doubt anyone is still using
> mv643xx anywhere.

Ok, I see.

> > If there is only one port and we just have to know which one that is,
> > I don't think we need the child nodes, but if one can have multiple
> > ports operate independently then the driver will need a rework
> > to actually be usable with that configuration.
> 
> I doubt pxa168 needs port nodes at all, i.e. we have the phy-handle
> directly as property of the controller node.

Ah, good.

> The HEC PHY node itself will be a sub-node of some future CEC node,
> while the (internal) MII PHY node can stay as a sub-node of the
> controller, e.g.
> 
> (one final example to make sure we agree on the same)
> 
> eth0: ethernet@f7b90000 {
>         compatible = "marvell,pxa168-eth";
>         reg = <...>;
>         #address-cells = <1>;
>         #size-cells = <0>;
>         phy-handle = <&ethphy0>;
> 
>         ethphy0: ethernet-phy@0 {
>                 reg = <0>;
>         };
> };
> 
> cec0: cec@f7f00baa {
>         compatible = "marvell,berlin-cec";
>         reg = <...>;
>         #address-cells = <1>;
>         #size-cells = <0>;
>         
>         hecphy0: hdmi-ethernet-phy@0 {
>                 reg = <0>;
>         };
> };
> 
> With berlin2cd-google-chromecast.dts overwriting
> 
> &eth0 { phy-handle = <&hecphy0>; };

Ok, now I also understood how the cec fits into this. Yes, I think this
looks very good.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/9] Documentation: bindings: net: add the Marvell PXA168 Ethernet controller

[Arnd Bergmann]

On Tuesday 23 September 2014 16:01:13 Antoine Tenart wrote:
> 
> On Tue, Sep 23, 2014 at 02:38:39PM +0200, Arnd Bergmann wrote:
> > On Tuesday 23 September 2014 14:15:30 Antoine Tenart wrote:
> > > +Optional properties:
> > > +- port-id: should be '0','1' or '2'.
> > > +- phy-addr: MDIO address of the PHY.
> > > +- local-mac-address: see ethernet.txt file in the same directory.
> > > +
> > > 
> > 
> > I believe new bindings should not use "phy-addr" properties, but
> > instead use a phy-handle property pointing to a phy device instead.
> 
> That's the MDIO address of the PHY. It's a bit difficult to use a phy
> device as the driver uses the libphy. And that would break the platform
> using it.
> 
> Or maybe you have something in mind?

I don't understand what you mean. Doesn't libphy imply using a phy_device?
What I meant was that you should be able to just call the standard
of_phy_connect() and other interfaces to deal with phylib.

	Arnd

Re: [PATCH v4 3/9] Documentation: bindings: net: add the Marvell PXA168 Ethernet controller

[Antoine Tenart]

On Tue, Sep 23, 2014 at 04:33:48PM +0200, Arnd Bergmann wrote:
> On Tuesday 23 September 2014 16:01:13 Antoine Tenart wrote:
> > 
> > On Tue, Sep 23, 2014 at 02:38:39PM +0200, Arnd Bergmann wrote:
> > > On Tuesday 23 September 2014 14:15:30 Antoine Tenart wrote:
> > > > +Optional properties:
> > > > +- port-id: should be '0','1' or '2'.
> > > > +- phy-addr: MDIO address of the PHY.
> > > > +- local-mac-address: see ethernet.txt file in the same directory.
> > > > +
> > > > 
> > > 
> > > I believe new bindings should not use "phy-addr" properties, but
> > > instead use a phy-handle property pointing to a phy device instead.
> > 
> > That's the MDIO address of the PHY. It's a bit difficult to use a phy
> > device as the driver uses the libphy. And that would break the platform
> > using it.
> > 
> > Or maybe you have something in mind?
> 
> I don't understand what you mean. Doesn't libphy imply using a phy_device?
> What I meant was that you should be able to just call the standard
> of_phy_connect() and other interfaces to deal with phylib.
> 

Yes sure, I read again my answer and... well I should have had a better
look on the matter.

Anyway, as the proposed binding seems to be OK with you, I'll update the
series accordingly.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH v4 4/9] net: pxa168_eth: fix Ethernet flow control status

[Antoine Tenart]

IEEE 802.3x Ethernet flow control is disabled when bit (1 << 2) is set
in the port status register. Fix the flow control detection in the link
event handling function which was relying on the opposite assumption.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/pxa168_eth.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
index a27cd85c030d..5aa2ce2625b4 100644
--- a/drivers/net/ethernet/marvell/pxa168_eth.c
+++ b/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -163,7 +163,7 @@
 /* Bit definitions for Port status */
 #define PORT_SPEED_100		(1 << 0)
 #define FULL_DUPLEX		(1 << 1)
-#define FLOW_CONTROL_ENABLED	(1 << 2)
+#define FLOW_CONTROL_DISABLED	(1 << 2)
 #define LINK_UP			(1 << 3)
 
 /* Bit definitions for work to be done */
@@ -885,7 +885,7 @@ static void handle_link_event(struct pxa168_eth_private *pep)
 		speed = 10;
 
 	duplex = (port_status & FULL_DUPLEX) ? 1 : 0;
-	fc = (port_status & FLOW_CONTROL_ENABLED) ? 1 : 0;
+	fc = (port_status & FLOW_CONTROL_DISABLED) ? 0 : 1;
 	netdev_info(dev, "link up, %d Mb/s, %s duplex, flow control %sabled\n",
 		    speed, duplex ? "full" : "half", fc ? "en" : "dis");
 	if (!netif_carrier_ok(dev))
-- 
1.9.1

[PATCH v4 5/9] net: pxa168_eth: set the mac address on the Ethernet controller

[Antoine Tenart]

When changing the MAC address, in addition to updating the dev_addr in
the net_device structure, this patch also update the MAC address
registers (high and low) of the Ethernet controller with the new MAC.
The address stored in these registers is used for IEEE 802.3x Ethernet
flow control, which is already enabled.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/pxa168_eth.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
index 5aa2ce2625b4..897589455186 100644
--- a/drivers/net/ethernet/marvell/pxa168_eth.c
+++ b/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -60,6 +60,8 @@
 #define PORT_COMMAND		0x0410
 #define PORT_STATUS		0x0418
 #define HTPR			0x0428
+#define MAC_ADDR_LOW		0x0430
+#define MAC_ADDR_HIGH		0x0438
 #define SDMA_CONFIG		0x0440
 #define SDMA_CMD		0x0448
 #define INT_CAUSE		0x0450
@@ -609,11 +611,22 @@ static int pxa168_eth_set_mac_address(struct net_device *dev, void *addr)
 	struct sockaddr *sa = addr;
 	struct pxa168_eth_private *pep = netdev_priv(dev);
 	unsigned char oldMac[ETH_ALEN];
+	u32 mac_h, mac_l;
 
 	if (!is_valid_ether_addr(sa->sa_data))
 		return -EADDRNOTAVAIL;
 	memcpy(oldMac, dev->dev_addr, ETH_ALEN);
 	memcpy(dev->dev_addr, sa->sa_data, ETH_ALEN);
+
+	mac_h = sa->sa_data[0] << 24;
+	mac_h |= sa->sa_data[1] << 16;
+	mac_h |= sa->sa_data[2] << 8;
+	mac_h |= sa->sa_data[3];
+	mac_l = sa->sa_data[4] << 8;
+	mac_l |= sa->sa_data[5];
+	wrl(pep, MAC_ADDR_HIGH, mac_h);
+	wrl(pep, MAC_ADDR_LOW, mac_l);
+
 	netif_addr_lock_bh(dev);
 	update_hash_table_mac_address(pep, oldMac, dev->dev_addr);
 	netif_addr_unlock_bh(dev);
-- 
1.9.1

[PATCH v4 6/9] net: pxa168_eth: rework the MAC address setup

[Antoine Tenart]

This patch rework the way the MAC address is retrieved. The MAC address
can now, in addition to being random, be set in the device tree or
retrieved from the Ethernet controller MAC address registers. The
probing function will try to get a MAC address in the following order:
- From the device tree.
- From the Ethernet controller MAC address registers.
- Generate a random one.

This patch also adds a function to read the MAC address from the
Ethernet Controller registers.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/pxa168_eth.c | 32 +++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
index 897589455186..4a57f640b424 100644
--- a/drivers/net/ethernet/marvell/pxa168_eth.c
+++ b/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -35,6 +35,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_net.h>
 #include <linux/phy.h>
 #include <linux/platform_device.h>
 #include <linux/pxa168_eth.h>
@@ -606,6 +607,21 @@ static void pxa168_eth_set_rx_mode(struct net_device *dev)
 		update_hash_table_mac_address(pep, NULL, ha->addr);
 }
 
+static void pxa168_eth_get_mac_address(struct net_device *dev,
+				       unsigned char *addr)
+{
+	struct pxa168_eth_private *pep = netdev_priv(dev);
+	unsigned int mac_h = rdl(pep, MAC_ADDR_HIGH);
+	unsigned int mac_l = rdl(pep, MAC_ADDR_LOW);
+
+	addr[0] = (mac_h >> 24) & 0xff;
+	addr[1] = (mac_h >> 16) & 0xff;
+	addr[2] = (mac_h >> 8) & 0xff;
+	addr[3] = mac_h & 0xff;
+	addr[4] = (mac_l >> 8) & 0xff;
+	addr[5] = mac_l & 0xff;
+}
+
 static int pxa168_eth_set_mac_address(struct net_device *dev, void *addr)
 {
 	struct sockaddr *sa = addr;
@@ -1466,6 +1482,7 @@ static int pxa168_eth_probe(struct platform_device *pdev)
 	struct net_device *dev = NULL;
 	struct resource *res;
 	struct clk *clk;
+	const unsigned char *mac_addr = NULL;
 	int err;
 
 	printk(KERN_NOTICE "PXA168 10/100 Ethernet Driver\n");
@@ -1507,8 +1524,19 @@ static int pxa168_eth_probe(struct platform_device *pdev)
 
 	INIT_WORK(&pep->tx_timeout_task, pxa168_eth_tx_timeout_task);
 
-	dev_info(&pdev->dev, "Using random mac address\n");
-	eth_hw_addr_random(dev);
+	if (pdev->dev.of_node)
+		mac_addr = of_get_mac_address(pdev->dev.of_node);
+
+	if (mac_addr && is_valid_ether_addr(mac_addr)) {
+		ether_addr_copy(dev->dev_addr, mac_addr);
+	} else {
+		/* try reading the mac address, if set by the bootloader */
+		pxa168_eth_get_mac_address(dev, dev->dev_addr);
+		if (!is_valid_ether_addr(dev->dev_addr)) {
+			dev_info(&pdev->dev, "Using random mac address\n");
+			eth_hw_addr_random(dev);
+		}
+	}
 
 	pep->rx_ring_size = NUM_RX_DESCS;
 	pep->tx_ring_size = NUM_TX_DESCS;
-- 
1.9.1

[PATCH v4 7/9] net: pxa168_eth: allow Berlin SoCs to use the pxa168_eth driver

[Antoine Tenart]

Berlin SoCs have an Ethernet controller compatible with the pxa168.
Allow these SoCs to use the pxa168_eth driver.

Signed-off-by: Antoine Tenart <antoine.tenart-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/net/ethernet/marvell/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
index 1b4fc7c639e6..48b9466d1781 100644
--- a/drivers/net/ethernet/marvell/Kconfig
+++ b/drivers/net/ethernet/marvell/Kconfig
@@ -64,7 +64,7 @@ config MVPP2
 
 config PXA168_ETH
 	tristate "Marvell pxa168 ethernet support"
-	depends on CPU_PXA168
+	depends on CPU_PXA168 || ARCH_BERLIN
 	select PHYLIB
 	---help---
 	  This driver supports the pxa168 Ethernet ports.
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 7/9] net: pxa168_eth: allow Berlin SoCs to use the pxa168_eth driver

[Arnd Bergmann]

On Tuesday 23 September 2014 14:15:34 Antoine Tenart wrote:
> diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
> index 1b4fc7c639e6..48b9466d1781 100644
> --- a/drivers/net/ethernet/marvell/Kconfig
> +++ b/drivers/net/ethernet/marvell/Kconfig
> @@ -64,7 +64,7 @@ config MVPP2
>  
>  config PXA168_ETH
>         tristate "Marvell pxa168 ethernet support"
> -       depends on CPU_PXA168
> +       depends on CPU_PXA168 || ARCH_BERLIN
>         select PHYLIB
>         ---help---
>           This driver supports the pxa168 Ethernet ports.
> 

Myabe you can also add '|| COMPILE_TEST' here, to allow building it
on other architectures. Check if you need 'depends on OF' though.

	Arnd

Re: [PATCH v4 7/9] net: pxa168_eth: allow Berlin SoCs to use the pxa168_eth driver

[Antoine Tenart]

Arnd,

On Tue, Sep 23, 2014 at 02:40:01PM +0200, Arnd Bergmann wrote:
> On Tuesday 23 September 2014 14:15:34 Antoine Tenart wrote:
> > diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
> > index 1b4fc7c639e6..48b9466d1781 100644
> > --- a/drivers/net/ethernet/marvell/Kconfig
> > +++ b/drivers/net/ethernet/marvell/Kconfig
> > @@ -64,7 +64,7 @@ config MVPP2
> >  
> >  config PXA168_ETH
> >         tristate "Marvell pxa168 ethernet support"
> > -       depends on CPU_PXA168
> > +       depends on CPU_PXA168 || ARCH_BERLIN
> >         select PHYLIB
> >         ---help---
> >           This driver supports the pxa168 Ethernet ports.
> > 
> 
> Myabe you can also add '|| COMPILE_TEST' here, to allow building it

We can add this.

> on other architectures. Check if you need 'depends on OF' though.

I don't think that's needed, the driver still supports non-dt cases.
I'll check.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 8/9] ARM: dts: berlin: add the Ethernet node

[Antoine Tenart]

This patch adds the Ethernet node, enabling the network unit on Berlin
BG2Q SoCs.

Signed-off-by: Antoine Tenart <antoine.tenart-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/boot/dts/berlin2q.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
index 902eddb19cd8..a88db4097b47 100644
--- a/arch/arm/boot/dts/berlin2q.dtsi
+++ b/arch/arm/boot/dts/berlin2q.dtsi
@@ -114,6 +114,16 @@
 			#interrupt-cells = <3>;
 		};
 
+		eth0: ethernet@b90000 {
+			compatible = "marvell,pxa168-eth";
+			reg = <0xb90000 0x10000>;
+			clocks = <&chip CLKID_GETH0>;
+			interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
+			/* set by bootloader */
+			local-mac-address = [00 00 00 00 00 00];
+			status = "disabled";
+		};
+
 		cpu-ctrl@dd0000 {
 			compatible = "marvell,berlin-cpu-ctrl";
 			reg = <0xdd0000 0x10000>;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 9/9] ARM: dts: berlin: enable the Ethernet port on the BG2Q DMP

[Antoine Tenart]

This patch enables the Ethernet port on the Marvell Berlin2Q DMP board.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 arch/arm/boot/dts/berlin2q-marvell-dmp.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2q-marvell-dmp.dts b/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
index a357ce02a64e..ea1f99b8eed6 100644
--- a/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
+++ b/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
@@ -45,3 +45,7 @@
 &uart0 {
 	status = "okay";
 };
+
+&eth0 {
+	status = "okay";
+};
-- 
1.9.1

Re: [PATCH v4 0/9] ARM: Berlin: Ethernet support

[Arnd Bergmann]

On Tuesday 23 September 2014 14:15:27 Antoine Tenart wrote:
> 
> This series introduce support for the Ethernet controller on Berlin SoCs,
> using the existing pxa168 Ethernet driver. In order to do this, DT
> support is added to the driver alongside some other modifications and
> fixes.
> 

Looks great overall, I've commented on two issues I've found, but only
the phy-addr one really needs to be addressed. Once that is out of the
way, please add

Acked-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH v4 0/9] ARM: Berlin: Ethernet support

[Antoine Tenart]

On Tue, Sep 23, 2014 at 02:41:47PM +0200, Arnd Bergmann wrote:
> On Tuesday 23 September 2014 14:15:27 Antoine Tenart wrote:
> > 
> > This series introduce support for the Ethernet controller on Berlin SoCs,
> > using the existing pxa168 Ethernet driver. In order to do this, DT
> > support is added to the driver alongside some other modifications and
> > fixes.
> > 
> 
> Looks great overall, I've commented on two issues I've found, but only
> the phy-addr one really needs to be addressed. Once that is out of the
> way, please add
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>

Thanks!

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com