[PATCH 1/5] net: MOXA ART: clear TX descriptor length bits between sends

[PATCH 1/5] net: MOXA ART: clear TX descriptor length bits between sends

[Jonas Jensen]

Add TX_DESC1_BUF_SIZE_MASK to bits that are cleared, before the TX buffer
length is set. Failing to do so can cause the controller to drop dead
i.e. all TX interrupts stop, resulting in complete communication failure.

Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---

Notes:
    Applies to next-20131122

 drivers/net/ethernet/moxa/moxart_ether.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c
index cbd0133..3c14afd 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.c
+++ b/drivers/net/ethernet/moxa/moxart_ether.c
@@ -349,7 +349,8 @@ static int moxart_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 
 	txdes1 = readl(desc + TX_REG_OFFSET_DESC1);
 	txdes1 |= TX_DESC1_LTS | TX_DESC1_FTS;
-	txdes1 &= ~(TX_DESC1_FIFO_COMPLETE | TX_DESC1_INTR_COMPLETE);
+	txdes1 &= ~(TX_DESC1_FIFO_COMPLETE | TX_DESC1_INTR_COMPLETE |
+		    TX_DESC1_BUF_SIZE_MASK);
 	txdes1 |= (len & TX_DESC1_BUF_SIZE_MASK);
 	writel(txdes1, desc + TX_REG_OFFSET_DESC1);
 	writel(TX_DESC0_DMA_OWN, desc + TX_REG_OFFSET_DESC0);
-- 
1.8.2.1

[PATCH 2/5] net: MOXA ART: connect to PHY and add ethtool support

[Jonas Jensen]

The kernel now has a MDIO bus driver and a phy_driver (RTL8201CP),
connect to this PHY using OF and add ethtool support.

Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---

Notes:
    Applies to next-20131122

 drivers/net/ethernet/moxa/moxart_ether.c | 179 ++++++++++++++++++++++++++++++-
 drivers/net/ethernet/moxa/moxart_ether.h |   1 +
 2 files changed, 179 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c
index 3c14afd..bcc6005 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.c
+++ b/drivers/net/ethernet/moxa/moxart_ether.c
@@ -26,9 +26,15 @@
 #include <linux/of_irq.h>
 #include <linux/crc32.h>
 #include <linux/crc32c.h>
+#include <linux/phy.h>
+#include <linux/of_mdio.h>
 
 #include "moxart_ether.h"
 
+#define DRV_NAME                "moxart-ethernet"
+#define DRV_VERSION             "0.2"
+#define MOXART_NUM_STATS        16
+
 static inline void moxart_emac_write(struct net_device *ndev,
 				     unsigned int reg, unsigned long value)
 {
@@ -61,6 +67,145 @@ static int moxart_set_mac_address(struct net_device *ndev, void *addr)
 	return 0;
 }
 
+static struct {
+	const char str[ETH_GSTRING_LEN];
+} ethtool_stats_keys[] = {
+	{ "tx_ok_mcol_2to15" },
+	{ "tx_ok_1col" },
+	{ "rx_frame_pause" },
+	{ "frame_align_err" },
+	{ "err_col_late_16" },
+	{ "err_col_16" },
+	{ "rx_runt" },
+	{ "late_col" },
+	{ "crc_err" },
+	{ "rx_ftl" },
+	{ "rx_fifo_full" },
+	{ "rx_col" },
+	{ "rx_bcast" },
+	{ "rx_mcast" },
+	{ "rx_ok" },
+	{ "tx_ok" },
+};
+
+static void moxart_get_drvinfo(struct net_device *ndev,
+			       struct ethtool_drvinfo *info)
+{
+	strlcpy(info->driver, DRV_NAME, sizeof(info->driver));
+	strlcpy(info->version, DRV_VERSION, sizeof(info->version));
+	strlcpy(info->bus_info, dev_name(&ndev->dev), sizeof(info->bus_info));
+	info->n_stats = MOXART_NUM_STATS;
+}
+
+static int moxart_get_settings(struct net_device *ndev, struct ethtool_cmd *cmd)
+{
+	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+
+	if (!priv->phy_dev)
+		return -ENODEV;
+
+	return phy_ethtool_gset(priv->phy_dev, cmd);
+}
+
+static int moxart_set_settings(struct net_device *ndev, struct ethtool_cmd *cmd)
+{
+	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+
+	if (!priv->phy_dev)
+		return -ENODEV;
+
+	return phy_ethtool_sset(priv->phy_dev, cmd);
+}
+
+static int moxart_nway_reset(struct net_device *ndev)
+{
+	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+
+	if (!priv->phy_dev)
+		return -EINVAL;
+
+	return genphy_restart_aneg(priv->phy_dev);
+}
+
+static void moxart_get_ethtool_stats(struct net_device *ndev,
+				     struct ethtool_stats *estats,
+				     u64 *tmp_stats)
+{
+	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+	u32 s;
+	int i = 0;
+
+	s = readl(priv->base + REG_TX_COL_COUNTER);
+	tmp_stats[i++] = s & 0xffff0000;
+	tmp_stats[i++] = s & 0x0000ffff;
+	s = readl(priv->base + REG_RPF_AEP_COUNTER);
+	tmp_stats[i++] = s & 0xffff0000;
+	tmp_stats[i++] = s & 0x0000ffff;
+	s = readl(priv->base + REG_XM_PG_COUNTER);
+	tmp_stats[i++] = s & 0xffff0000;
+	tmp_stats[i++] = s & 0x0000ffff;
+	s = readl(priv->base + REG_RUNT_TLC_COUNTER);
+	tmp_stats[i++] = s & 0xffff0000;
+	tmp_stats[i++] = s & 0x0000ffff;
+	s = readl(priv->base + REG_CRC_FTL_COUNTER);
+	tmp_stats[i++] = s & 0xffff0000;
+	tmp_stats[i++] = s & 0x0000ffff;
+	s = readl(priv->base + REG_RLC_RCC_COUNTER);
+	tmp_stats[i++] = s & 0xffff0000;
+	tmp_stats[i++] = s & 0x0000ffff;
+	tmp_stats[i++] = readl(priv->base + REG_BROC_COUNTER);
+	tmp_stats[i++] = readl(priv->base + REG_MULCA_COUNTER);
+	tmp_stats[i++] = readl(priv->base + REG_XP_COUNTER);
+	tmp_stats[i++] = readl(priv->base + REG_RP_COUNTER);
+}
+
+static int moxart_get_sset_count(struct net_device *netdev,
+					int string_set)
+{
+	switch (string_set) {
+	case ETH_SS_STATS:
+		return MOXART_NUM_STATS;
+	default:
+		return -EINVAL;
+	}
+}
+
+static void moxart_get_strings(struct net_device *dev, u32 stringset, u8 *data)
+{
+	switch (stringset) {
+	case ETH_SS_STATS:
+		memcpy(data, &ethtool_stats_keys, sizeof(ethtool_stats_keys));
+		break;
+	default:
+		WARN_ON(1);
+		break;
+	}
+}
+
+static const struct ethtool_ops moxart_ethtool_ops = {
+	.set_settings           = moxart_set_settings,
+	.get_settings           = moxart_get_settings,
+	.get_drvinfo            = moxart_get_drvinfo,
+	.nway_reset             = moxart_nway_reset,
+	.get_link               = ethtool_op_get_link,
+	.get_ethtool_stats      = moxart_get_ethtool_stats,
+	.get_sset_count         = moxart_get_sset_count,
+	.get_strings            = moxart_get_strings,
+};
+
+static int moxart_do_ioctl(struct net_device *ndev, struct ifreq *ir, int cmd)
+{
+	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+
+	if (!netif_running(ndev))
+		return -EINVAL;
+
+	if (!priv->phy_dev)
+		return -ENODEV;
+
+	return phy_mii_ioctl(priv->phy_dev, ir, cmd);
+}
+
 static void moxart_mac_free_memory(struct net_device *ndev)
 {
 	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
@@ -169,6 +314,10 @@ static int moxart_mac_open(struct net_device *ndev)
 	moxart_update_mac_address(ndev);
 	moxart_mac_setup_desc_ring(ndev);
 	moxart_mac_enable(ndev);
+
+	if (priv->phy_dev)
+		phy_start(priv->phy_dev);
+
 	netif_start_queue(ndev);
 
 	netdev_dbg(ndev, "%s: IMR=0x%x, MACCR=0x%x\n",
@@ -184,6 +333,9 @@ static int moxart_mac_stop(struct net_device *ndev)
 
 	napi_disable(&priv->napi);
 
+	if (priv->phy_dev)
+		phy_stop(priv->phy_dev);
+
 	netif_stop_queue(ndev);
 
 	/* disable all interrupts */
@@ -429,12 +581,22 @@ static struct net_device_ops moxart_netdev_ops = {
 	.ndo_set_mac_address	= moxart_set_mac_address,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_change_mtu		= eth_change_mtu,
+	.ndo_do_ioctl           = moxart_do_ioctl,
 };
 
+static void moxart_adjust_link(struct net_device *ndev)
+{
+#ifdef DEBUG
+	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+
+	phy_print_status(priv->phy_dev);
+#endif
+}
+
 static int moxart_mac_probe(struct platform_device *pdev)
 {
 	struct device *p_dev = &pdev->dev;
-	struct device_node *node = p_dev->of_node;
+	struct device_node *node = p_dev->of_node, *phy_node;
 	struct net_device *ndev;
 	struct moxart_mac_priv_t *priv;
 	struct resource *res;
@@ -455,6 +617,20 @@ static int moxart_mac_probe(struct platform_device *pdev)
 	priv = netdev_priv(ndev);
 	priv->ndev = ndev;
 
+	phy_node = of_parse_phandle(node, "phy-handle", 0);
+	if (!phy_node)
+		dev_err(p_dev, "of_parse_phandle failed\n");
+
+	if (phy_node) {
+		priv->phy_dev = of_phy_connect(priv->ndev, phy_node,
+					       &moxart_adjust_link,
+					       0, PHY_INTERFACE_MODE_MII);
+		if (!priv->phy_dev) {
+			dev_err(p_dev, "of_phy_connect failed\n");
+			ret = -ENODEV;
+		}
+	}
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	ndev->base_addr = res->start;
 	priv->base = devm_ioremap_resource(p_dev, res);
@@ -516,6 +692,7 @@ static int moxart_mac_probe(struct platform_device *pdev)
 	ndev->irq = irq;
 
 	SET_NETDEV_DEV(ndev, &pdev->dev);
+	SET_ETHTOOL_OPS(ndev, &moxart_ethtool_ops);
 
 	ret = register_netdev(ndev);
 	if (ret) {
diff --git a/drivers/net/ethernet/moxa/moxart_ether.h b/drivers/net/ethernet/moxa/moxart_ether.h
index 2be9280..193618e 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.h
+++ b/drivers/net/ethernet/moxa/moxart_ether.h
@@ -297,6 +297,7 @@ struct moxart_mac_priv_t {
 	unsigned int reg_imr;
 	struct napi_struct napi;
 	struct net_device *ndev;
+	struct phy_device *phy_dev;
 
 	dma_addr_t rx_base;
 	dma_addr_t rx_mapping[RX_DESC_NUM];
-- 
1.8.2.1

[PATCH 3/5] net: MOXA ART: add IFF_LIVE_ADDR_CHANGE flag

[Jonas Jensen]

.ndo_set_mac_address hook callback already supports IFF_LIVE_ADDR_CHANGE
so add it to our flags.

Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---

Notes:
    Applies to next-20131122

 drivers/net/ethernet/moxa/moxart_ether.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c
index bcc6005..08c3905 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.c
+++ b/drivers/net/ethernet/moxa/moxart_ether.c
@@ -688,7 +688,7 @@ static int moxart_mac_probe(struct platform_device *pdev)
 	ether_setup(ndev);
 	ndev->netdev_ops = &moxart_netdev_ops;
 	netif_napi_add(ndev, &priv->napi, moxart_rx_poll, RX_DESC_NUM);
-	ndev->priv_flags |= IFF_UNICAST_FLT;
+	ndev->priv_flags |= IFF_UNICAST_FLT | IFF_LIVE_ADDR_CHANGE;
 	ndev->irq = irq;
 
 	SET_NETDEV_DEV(ndev, &pdev->dev);
-- 
1.8.2.1

[PATCH 4/5] net: MOXA ART: generate random address

[Jonas Jensen]

The address register is zero:ed on boot, fill it with a randomly generated
address on probe.

Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---

Notes:
    Applies to next-20131122

 drivers/net/ethernet/moxa/moxart_ether.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c
index 08c3905..c02075a 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.c
+++ b/drivers/net/ethernet/moxa/moxart_ether.c
@@ -700,6 +700,12 @@ static int moxart_mac_probe(struct platform_device *pdev)
 		goto init_fail;
 	}
 
+	if (!is_valid_ether_addr(ndev->dev_addr)) {
+		eth_hw_addr_random(ndev);
+		netdev_info(ndev, "generated random MAC address %pM\n",
+			    ndev->dev_addr);
+	}
+
 	netdev_dbg(ndev, "%s: IRQ=%d address=%pM\n",
 		   __func__, ndev->irq, ndev->dev_addr);
 
-- 
1.8.2.1

[PATCH 5/5] net: MOXA ART: use eth_mac_addr()

[Jonas Jensen]

Replace boilerplate in moxart_set_mac_address() with eth_mac_addr().

Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---

Notes:
    Applies to next-20131122

 drivers/net/ethernet/moxa/moxart_ether.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c
index c02075a..9a11d34 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.c
+++ b/drivers/net/ethernet/moxa/moxart_ether.c
@@ -56,12 +56,7 @@ static void moxart_update_mac_address(struct net_device *ndev)
 
 static int moxart_set_mac_address(struct net_device *ndev, void *addr)
 {
-	struct sockaddr *address = addr;
-
-	if (!is_valid_ether_addr(address->sa_data))
-		return -EADDRNOTAVAIL;
-
-	memcpy(ndev->dev_addr, address->sa_data, ndev->addr_len);
+	eth_mac_addr(ndev, addr);
 	moxart_update_mac_address(ndev);
 
 	return 0;
-- 
1.8.2.1

Re: [PATCH 2/5] net: MOXA ART: connect to PHY and add ethtool support

[Jonas Jensen]

On 22 November 2013 15:57, Jonas Jensen <jonas.jensen@gmail.com> wrote:
>  drivers/net/ethernet/moxa/moxart_ether.c | 179 ++++++++++++++++++++++++++++++-
>  drivers/net/ethernet/moxa/moxart_ether.h |   1 +
>  2 files changed, 179 insertions(+), 1 deletion(-)

I see now I forgot to include the devicetree binding document.

I'll wait for comments.


Thanks,
Jonas

Re: [PATCH 2/5] net: MOXA ART: connect to PHY and add ethtool support

[Florian Fainelli]

2013/11/22 Jonas Jensen <jonas.jensen@gmail.com>:
> On 22 November 2013 15:57, Jonas Jensen <jonas.jensen@gmail.com> wrote:
>>  drivers/net/ethernet/moxa/moxart_ether.c | 179 ++++++++++++++++++++++++++++++-
>>  drivers/net/ethernet/moxa/moxart_ether.h |   1 +
>>  2 files changed, 179 insertions(+), 1 deletion(-)
>
> I see now I forgot to include the devicetree binding document.
>
> I'll wait for comments.

You are doing two things in your patch, so that means two separate patches.
-- 
Florian

Re: [PATCH 2/5] net: MOXA ART: connect to PHY and add ethtool support

[Florian Fainelli]

Hello Jonas,

> +       if (!priv->phy_dev)
> +               return -ENODEV;

This should not be required since you will fail probing the interface
if you cannot connect to the PHY device.

> +
> +       return phy_ethtool_gset(priv->phy_dev, cmd);
> +}
> +
> +static int moxart_set_settings(struct net_device *ndev, struct ethtool_cmd *cmd)
> +{
> +       struct moxart_mac_priv_t *priv = netdev_priv(ndev);
> +
> +       if (!priv->phy_dev)
> +               return -ENODEV;

Same here

> +
> +       return phy_ethtool_sset(priv->phy_dev, cmd);
> +}
> +
> +static int moxart_nway_reset(struct net_device *ndev)
> +{
> +       struct moxart_mac_priv_t *priv = netdev_priv(ndev);
> +
> +       if (!priv->phy_dev)
> +               return -EINVAL;

And here

[snip]

> +       if (!priv->phy_dev)
> +               return -ENODEV;

And here

> +
> +       return phy_mii_ioctl(priv->phy_dev, ir, cmd);
> +}
> +
>  static void moxart_mac_free_memory(struct net_device *ndev)
>  {
>         struct moxart_mac_priv_t *priv = netdev_priv(ndev);
> @@ -169,6 +314,10 @@ static int moxart_mac_open(struct net_device *ndev)
>         moxart_update_mac_address(ndev);
>         moxart_mac_setup_desc_ring(ndev);
>         moxart_mac_enable(ndev);
> +
> +       if (priv->phy_dev)
> +               phy_start(priv->phy_dev);
> +
>         netif_start_queue(ndev);
>
>         netdev_dbg(ndev, "%s: IMR=0x%x, MACCR=0x%x\n",
> @@ -184,6 +333,9 @@ static int moxart_mac_stop(struct net_device *ndev)
>
>         napi_disable(&priv->napi);
>
> +       if (priv->phy_dev)
> +               phy_stop(priv->phy_dev);
> +
>         netif_stop_queue(ndev);
>
>         /* disable all interrupts */
> @@ -429,12 +581,22 @@ static struct net_device_ops moxart_netdev_ops = {
>         .ndo_set_mac_address    = moxart_set_mac_address,
>         .ndo_validate_addr      = eth_validate_addr,
>         .ndo_change_mtu         = eth_change_mtu,
> +       .ndo_do_ioctl           = moxart_do_ioctl,
>  };
>
> +static void moxart_adjust_link(struct net_device *ndev)
> +{
> +#ifdef DEBUG
> +       struct moxart_mac_priv_t *priv = netdev_priv(ndev);
> +
> +       phy_print_status(priv->phy_dev);
> +#endif

This is too simplistic, you should at least do the following:

- check for an update in the PHY duplex setting and update the
Ethernet MAC with this new setting
- check for an update in the PHY speed settings and update relevant
MAC registers (RX/TX clock speed, PHY speed etc...)

Also, it is a good practice to retain the previous link
state/duplex/speed, compare them against the phydev values and call
phy_print_status() if there is any change.

> +}
> +
>  static int moxart_mac_probe(struct platform_device *pdev)
>  {
>         struct device *p_dev = &pdev->dev;
> -       struct device_node *node = p_dev->of_node;
> +       struct device_node *node = p_dev->of_node, *phy_node;
>         struct net_device *ndev;
>         struct moxart_mac_priv_t *priv;
>         struct resource *res;
> @@ -455,6 +617,20 @@ static int moxart_mac_probe(struct platform_device *pdev)
>         priv = netdev_priv(ndev);
>         priv->ndev = ndev;
>
> +       phy_node = of_parse_phandle(node, "phy-handle", 0);
> +       if (!phy_node)
> +               dev_err(p_dev, "of_parse_phandle failed\n");
> +
> +       if (phy_node) {
> +               priv->phy_dev = of_phy_connect(priv->ndev, phy_node,
> +                                              &moxart_adjust_link,
> +                                              0, PHY_INTERFACE_MODE_MII);

Unless the hardware supports anything but MII, this is fine. A nicer
binding would include a "phy-mode" or "phy-connection-type" property
which describes how the Ethernet MAC and PHY are connected to each
other. You can then retrieve that property using of_get_phy_mode().
-- 
Florian

Re: [PATCH 2/5] net: MOXA ART: connect to PHY and add ethtool support

[Ben Hutchings]

On Fri, 2013-11-22 at 15:57 +0100, Jonas Jensen wrote:
> The kernel now has a MDIO bus driver and a phy_driver (RTL8201CP),
> connect to this PHY using OF and add ethtool support.
> 
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
> ---
> 
> Notes:
>     Applies to next-20131122
> 
>  drivers/net/ethernet/moxa/moxart_ether.c | 179 ++++++++++++++++++++++++++++++-
>  drivers/net/ethernet/moxa/moxart_ether.h |   1 +
>  2 files changed, 179 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c
> index 3c14afd..bcc6005 100644
> --- a/drivers/net/ethernet/moxa/moxart_ether.c
> +++ b/drivers/net/ethernet/moxa/moxart_ether.c
> @@ -26,9 +26,15 @@
>  #include <linux/of_irq.h>
>  #include <linux/crc32.h>
>  #include <linux/crc32c.h>
> +#include <linux/phy.h>
> +#include <linux/of_mdio.h>
>  
>  #include "moxart_ether.h"
>  
> +#define DRV_NAME                "moxart-ethernet"
> +#define DRV_VERSION             "0.2"
> +#define MOXART_NUM_STATS        16

MOXART_NUM_STATS should be defined as ARRAY_SIZE(ethtool_stats_keys).

[...]
> +static void moxart_get_drvinfo(struct net_device *ndev,
> +			       struct ethtool_drvinfo *info)
> +{
> +	strlcpy(info->driver, DRV_NAME, sizeof(info->driver));
> +	strlcpy(info->version, DRV_VERSION, sizeof(info->version));
> +	strlcpy(info->bus_info, dev_name(&ndev->dev), sizeof(info->bus_info));
> +	info->n_stats = MOXART_NUM_STATS;
[...]

Don't initialise n_stats here; the core will initialise it using your
get_sset_count implementation.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.