Re: [PATCH] net/ncsi: Add NCSI OEM command for FB Tiogapass

[Joel Stanley]

On Thu, 27 Sep 2018 at 00:39,  wrote:
>
> > >  As I understand Justin's version adds a generic handler, using the NCSI
> > >  Netlink interface to pass OEM commands and responses to and from
> > >  userspace, which does the actual packet handling.
> > Thanks for the direction Sam! Justin, if you don't mind, can you share the 
> > patches you have to add the support? This actually would solve a few other 
> > things we are trying to accomplish.
>
>
> Basically, I add a new flag to indicate the request is coming from the 
> Netlink interface to allow the command handler and response handler to react.
> #define NCSI_REQ_FLAG_NETLINK_DRIVEN2
>
> The work flow is as below.
>
> Request:
> User space application -> Netlink interface (msg) -> new Netlink handler - 
> ncsi_send_cmd_nl() - ncsi_xmit_cmd()
>
> Response:
> Response received - ncsi_rcv_rsp() -> internal response handler - 
> ncsi_rsp_handler_xxx() -> ncsi_send_netlink_rsp () -> Netlink interface (msg) 
> -> user space application
> Command timeout - ncsi_request_timeout() -> ncsi_send_netlink_timeout () -> 
> Netlink interface (msg with zero data length) -> user space application
>
> Error:
> Detected error -> ncsi_send_netlink_err () -> Netlink interface (err msg) -> 
> user space application
>
> I will clean up some code and send out the patch.

Thanks for the overview. We look forward to your patches; please
include the same cc list as this series.

I think it makes sense to have some OEM NCSI handing purely in the
kenrel. This would allow eg. the MAC address of an interface to be
correct at boot, without requiring userspace to come up first.

I have also heard stories of temperature sensors over NCSI. Those
would make sense to be hwmon drivers, which again would mean handling
them in the kernel.

Justin, Vijay, can you please list the known NCSI OEM
commands/extensions that we plan on implementing?

Cheers,

Joel

[PATCH v2 2/4] net/ncsi: Drop no more channels message

[Joel Stanley]

This does not provide useful information. As the ncsi maintainer said:

 > either we get a channel or broadcom has gone out to lunch

Acked-by: Samuel Mendoza-Jonas 
Signed-off-by: Joel Stanley 
---
 net/ncsi/ncsi-manage.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 616441c2b54f..716493a61ba6 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -1226,8 +1226,6 @@ int ncsi_process_next_channel(struct ncsi_dev_priv *ndp)
return ncsi_choose_active_channel(ndp);
}
 
-   netdev_printk(KERN_DEBUG, ndp->ndev.dev,
- "NCSI: No more channels to process\n");
ncsi_report_link(ndp, false);
return -ENODEV;
 }
-- 
2.17.1

[PATCH v2 3/4] net/ncsi: Use netdev_dbg for debug messages

[Joel Stanley]

This moves all of the netdev_printk(KERN_DEBUG, ...) messages over to
netdev_dbg.

As Joe explains:

> netdev_dbg is not included in object code unless
> DEBUG is defined or CONFIG_DYNAMIC_DEBUG is set.
> And then, it is not emitted into the log unless
> DEBUG is set or this specific netdev_dbg is enabled
> via the dynamic debug control file.

Which is what we're after in this case.

Acked-by: Samuel Mendoza-Jonas 
Signed-off-by: Joel Stanley 
---
v2: update commit message

 net/ncsi/ncsi-aen.c|  6 +++---
 net/ncsi/ncsi-manage.c | 33 +++--
 2 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
index f899ed61bb57..25e483e8278b 100644
--- a/net/ncsi/ncsi-aen.c
+++ b/net/ncsi/ncsi-aen.c
@@ -148,9 +148,9 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv 
*ndp,
hncdsc = (struct ncsi_aen_hncdsc_pkt *)h;
ncm->data[3] = ntohl(hncdsc->status);
spin_unlock_irqrestore(>lock, flags);
-   netdev_printk(KERN_DEBUG, ndp->ndev.dev,
- "NCSI: host driver %srunning on channel %u\n",
- ncm->data[3] & 0x1 ? "" : "not ", nc->id);
+   netdev_dbg(ndp->ndev.dev,
+  "NCSI: host driver %srunning on channel %u\n",
+  ncm->data[3] & 0x1 ? "" : "not ", nc->id);
 
return 0;
 }
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 716493a61ba6..091284760d21 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -788,8 +788,8 @@ static void ncsi_configure_channel(struct ncsi_dev_priv 
*ndp)
}
break;
case ncsi_dev_state_config_done:
-   netdev_printk(KERN_DEBUG, ndp->ndev.dev,
- "NCSI: channel %u config done\n", nc->id);
+   netdev_dbg(ndp->ndev.dev, "NCSI: channel %u config done\n",
+  nc->id);
spin_lock_irqsave(>lock, flags);
if (nc->reconfigure_needed) {
/* This channel's configuration has been updated
@@ -804,8 +804,7 @@ static void ncsi_configure_channel(struct ncsi_dev_priv 
*ndp)
list_add_tail_rcu(>link, >channel_queue);
spin_unlock_irqrestore(>lock, flags);
 
-   netdev_printk(KERN_DEBUG, dev,
- "Dirty NCSI channel state reset\n");
+   netdev_dbg(dev, "Dirty NCSI channel state reset\n");
ncsi_process_next_channel(ndp);
break;
}
@@ -908,9 +907,9 @@ static int ncsi_choose_active_channel(struct ncsi_dev_priv 
*ndp)
}
 
ncm = >modes[NCSI_MODE_LINK];
-   netdev_printk(KERN_DEBUG, ndp->ndev.dev,
- "NCSI: Channel %u added to queue (link %s)\n",
- found->id, ncm->data[2] & 0x1 ? "up" : "down");
+   netdev_dbg(ndp->ndev.dev,
+  "NCSI: Channel %u added to queue (link %s)\n",
+  found->id, ncm->data[2] & 0x1 ? "up" : "down");
 
 out:
spin_lock_irqsave(>lock, flags);
@@ -1316,9 +1315,9 @@ static int ncsi_kick_channels(struct ncsi_dev_priv *ndp)
if ((ndp->ndev.state & 0xff00) ==
ncsi_dev_state_config ||
!list_empty(>link)) {
-   netdev_printk(KERN_DEBUG, nd->dev,
- "NCSI: channel %p marked 
dirty\n",
- nc);
+   netdev_dbg(nd->dev,
+  "NCSI: channel %p marked 
dirty\n",
+  nc);
nc->reconfigure_needed = true;
}
spin_unlock_irqrestore(>lock, flags);
@@ -1336,8 +1335,7 @@ static int ncsi_kick_channels(struct ncsi_dev_priv *ndp)
list_add_tail_rcu(>link, >channel_queue);
spin_unlock_irqrestore(>lock, flags);
 
-   netdev_printk(KERN_DEBUG, nd->dev,
- "NCSI: kicked channel %p\n", nc);
+   netdev_dbg(nd->dev, "NCSI: kicked channel %p\n", nc);
n++;
}
}
@@ -1368,8 +1366,8 @@ int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 
prot

[PATCH v2 4/4] MAINTAINERS: Add Sam as the maintainer for NCSI

[Joel Stanley]

Sam has been handing the maintenance of NCSI for a number release cycles
now.

Acked-by: Samuel Mendoza-Jonas 
Signed-off-by: Joel Stanley 
---
 MAINTAINERS | 5 +
 1 file changed, 5 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9d5eeff51b5f..44851f7c46fc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9756,6 +9756,11 @@ L:   linux-s...@vger.kernel.org
 S: Maintained
 F: drivers/scsi/NCR_D700.*
 
+NCSI LIBRARY:
+M: Samuel Mendoza-Jonas 
+S: Maintained
+F: net/ncsi/
+
 NCT6775 HARDWARE MONITOR DRIVER
 M: Guenter Roeck 
 L: linux-hw...@vger.kernel.org
-- 
2.17.1

[PATCH v2 1/4] net/ncsi: Silence debug messages

[Joel Stanley]

In normal operation we see this series of messages as the host drives
the network device:

 ftgmac100 1e66.ethernet eth0: NCSI: LSC AEN - channel 0 state down
 ftgmac100 1e66.ethernet eth0: NCSI: suspending channel 0
 ftgmac100 1e66.ethernet eth0: NCSI: configuring channel 0
 ftgmac100 1e66.ethernet eth0: NCSI: channel 0 link down after config
 ftgmac100 1e66.ethernet eth0: NCSI interface down
 ftgmac100 1e66.ethernet eth0: NCSI: LSC AEN - channel 0 state up
 ftgmac100 1e66.ethernet eth0: NCSI: configuring channel 0
 ftgmac100 1e66.ethernet eth0: NCSI interface up
 ftgmac100 1e66.ethernet eth0: NCSI: LSC AEN - channel 0 state down
 ftgmac100 1e66.ethernet eth0: NCSI: suspending channel 0
 ftgmac100 1e66.ethernet eth0: NCSI: configuring channel 0
 ftgmac100 1e66.ethernet eth0: NCSI: channel 0 link down after config
 ftgmac100 1e66.ethernet eth0: NCSI interface down
 ftgmac100 1e66.ethernet eth0: NCSI: LSC AEN - channel 0 state up
 ftgmac100 1e66.ethernet eth0: NCSI: configuring channel 0
 ftgmac100 1e66.ethernet eth0: NCSI interface up

This makes all of these messages netdev_dbg. They are still useful to
debug eg. misbehaving network device firmware, but we do not need them
filling up the kernel logs in normal operation.

Acked-by: Samuel Mendoza-Jonas 
Signed-off-by: Joel Stanley 
---
v2: Fix alignment in ftgmac100 change
---
 drivers/net/ethernet/faraday/ftgmac100.c |  4 ++--
 net/ncsi/ncsi-aen.c  |  4 ++--
 net/ncsi/ncsi-manage.c   | 14 +++---
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 78db8e62a83f..ed6c76d20b45 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1735,8 +1735,8 @@ static void ftgmac100_ncsi_handler(struct ncsi_dev *nd)
if (unlikely(nd->state != ncsi_dev_state_functional))
return;
 
-   netdev_info(nd->dev, "NCSI interface %s\n",
-   nd->link_up ? "up" : "down");
+   netdev_dbg(nd->dev, "NCSI interface %s\n",
+  nd->link_up ? "up" : "down");
 }
 
 static void ftgmac100_setup_clk(struct ftgmac100 *priv)
diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
index e7b05de1e6d1..f899ed61bb57 100644
--- a/net/ncsi/ncsi-aen.c
+++ b/net/ncsi/ncsi-aen.c
@@ -73,8 +73,8 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp,
ncm->data[2] = data;
ncm->data[4] = ntohl(lsc->oem_status);
 
-   netdev_info(ndp->ndev.dev, "NCSI: LSC AEN - channel %u state %s\n",
-   nc->id, data & 0x1 ? "up" : "down");
+   netdev_dbg(ndp->ndev.dev, "NCSI: LSC AEN - channel %u state %s\n",
+  nc->id, data & 0x1 ? "up" : "down");
 
chained = !list_empty(>link);
state = nc->state;
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 5561e221b71f..616441c2b54f 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -816,9 +816,9 @@ static void ncsi_configure_channel(struct ncsi_dev_priv 
*ndp)
} else {
hot_nc = NULL;
nc->state = NCSI_CHANNEL_INACTIVE;
-   netdev_warn(ndp->ndev.dev,
-   "NCSI: channel %u link down after config\n",
-   nc->id);
+   netdev_dbg(ndp->ndev.dev,
+  "NCSI: channel %u link down after config\n",
+  nc->id);
}
spin_unlock_irqrestore(>lock, flags);
 
@@ -1199,14 +1199,14 @@ int ncsi_process_next_channel(struct ncsi_dev_priv *ndp)
switch (old_state) {
case NCSI_CHANNEL_INACTIVE:
ndp->ndev.state = ncsi_dev_state_config;
-   netdev_info(ndp->ndev.dev, "NCSI: configuring channel %u\n",
-   nc->id);
+   netdev_dbg(ndp->ndev.dev, "NCSI: configuring channel %u\n",
+  nc->id);
ncsi_configure_channel(ndp);
break;
case NCSI_CHANNEL_ACTIVE:
ndp->ndev.state = ncsi_dev_state_suspend;
-   netdev_info(ndp->ndev.dev, "NCSI: suspending channel %u\n",
-   nc->id);
+   netdev_dbg(ndp->ndev.dev, "NCSI: suspending channel %u\n",
+  nc->id);
ncsi_suspend_channel(ndp);
break;
default:
-- 
2.17.1

[PATCH v2 0/4] Slience NCSI logging

[Joel Stanley]

v2:
  Fix indent issue and commit message based on Joe's feedback
  Add Sam's acks

Here are three changes to silence unnecessary warnings in the ncsi code.

The final patch adds Sam as the maintainer for NCSI.

Joel Stanley (4):
  net/ncsi: Silence debug messages
  net/ncsi: Drop no more channels message
  net/ncsi: Use netdev_dbg for debug messages
  MAINTAINERS: Add Sam as the maintainer for NCSI

 MAINTAINERS  |  5 +++
 drivers/net/ethernet/faraday/ftgmac100.c |  4 +-
 net/ncsi/ncsi-aen.c  | 10 ++---
 net/ncsi/ncsi-manage.c   | 49 +++-
 4 files changed, 34 insertions(+), 34 deletions(-)

-- 
2.17.1

[PATCH 0/4] Slience NCSI logging

[Joel Stanley]

Here are three changes to silence unnecessary warnings in the ncsi code.

The final patch adds Sam as the maintainer for NCSI.

Joel Stanley (4):
  net/ncsi: Silence debug messages
  net/ncsi: Drop no more channels message
  net/ncsi: Use netdev_dbg for debug messages
  MAINTAINERS: Add Sam as the maintainer for NCSI

 MAINTAINERS  |  5 +++
 drivers/net/ethernet/faraday/ftgmac100.c |  2 +-
 net/ncsi/ncsi-aen.c  | 10 ++---
 net/ncsi/ncsi-manage.c   | 49 +++-
 4 files changed, 33 insertions(+), 33 deletions(-)

-- 
2.17.1

[PATCH 3/4] net/ncsi: Use netdev_dbg for debug messages

[Joel Stanley]

This moves all of the netdev_printk(KERN_DEBUG, ...) messages over to
netdev_dbg. There is no change in behaviour.

Signed-off-by: Joel Stanley 
---
 net/ncsi/ncsi-aen.c|  6 +++---
 net/ncsi/ncsi-manage.c | 33 +++--
 2 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
index f899ed61bb57..25e483e8278b 100644
--- a/net/ncsi/ncsi-aen.c
+++ b/net/ncsi/ncsi-aen.c
@@ -148,9 +148,9 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv 
*ndp,
hncdsc = (struct ncsi_aen_hncdsc_pkt *)h;
ncm->data[3] = ntohl(hncdsc->status);
spin_unlock_irqrestore(>lock, flags);
-   netdev_printk(KERN_DEBUG, ndp->ndev.dev,
- "NCSI: host driver %srunning on channel %u\n",
- ncm->data[3] & 0x1 ? "" : "not ", nc->id);
+   netdev_dbg(ndp->ndev.dev,
+  "NCSI: host driver %srunning on channel %u\n",
+  ncm->data[3] & 0x1 ? "" : "not ", nc->id);
 
return 0;
 }
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 716493a61ba6..091284760d21 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -788,8 +788,8 @@ static void ncsi_configure_channel(struct ncsi_dev_priv 
*ndp)
}
break;
case ncsi_dev_state_config_done:
-   netdev_printk(KERN_DEBUG, ndp->ndev.dev,
- "NCSI: channel %u config done\n", nc->id);
+   netdev_dbg(ndp->ndev.dev, "NCSI: channel %u config done\n",
+  nc->id);
spin_lock_irqsave(>lock, flags);
if (nc->reconfigure_needed) {
/* This channel's configuration has been updated
@@ -804,8 +804,7 @@ static void ncsi_configure_channel(struct ncsi_dev_priv 
*ndp)
list_add_tail_rcu(>link, >channel_queue);
spin_unlock_irqrestore(>lock, flags);
 
-   netdev_printk(KERN_DEBUG, dev,
- "Dirty NCSI channel state reset\n");
+   netdev_dbg(dev, "Dirty NCSI channel state reset\n");
ncsi_process_next_channel(ndp);
break;
}
@@ -908,9 +907,9 @@ static int ncsi_choose_active_channel(struct ncsi_dev_priv 
*ndp)
}
 
ncm = >modes[NCSI_MODE_LINK];
-   netdev_printk(KERN_DEBUG, ndp->ndev.dev,
- "NCSI: Channel %u added to queue (link %s)\n",
- found->id, ncm->data[2] & 0x1 ? "up" : "down");
+   netdev_dbg(ndp->ndev.dev,
+  "NCSI: Channel %u added to queue (link %s)\n",
+  found->id, ncm->data[2] & 0x1 ? "up" : "down");
 
 out:
spin_lock_irqsave(>lock, flags);
@@ -1316,9 +1315,9 @@ static int ncsi_kick_channels(struct ncsi_dev_priv *ndp)
if ((ndp->ndev.state & 0xff00) ==
ncsi_dev_state_config ||
!list_empty(>link)) {
-   netdev_printk(KERN_DEBUG, nd->dev,
- "NCSI: channel %p marked 
dirty\n",
- nc);
+   netdev_dbg(nd->dev,
+  "NCSI: channel %p marked 
dirty\n",
+  nc);
nc->reconfigure_needed = true;
}
spin_unlock_irqrestore(>lock, flags);
@@ -1336,8 +1335,7 @@ static int ncsi_kick_channels(struct ncsi_dev_priv *ndp)
list_add_tail_rcu(>link, >channel_queue);
spin_unlock_irqrestore(>lock, flags);
 
-   netdev_printk(KERN_DEBUG, nd->dev,
- "NCSI: kicked channel %p\n", nc);
+   netdev_dbg(nd->dev, "NCSI: kicked channel %p\n", nc);
n++;
}
}
@@ -1368,8 +1366,8 @@ int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 
proto, u16 vid)
list_for_each_entry_rcu(vlan, >vlan_vids, list) {
n_vids++;
if (vlan->vid == vid) {
-   netdev_printk(KERN_DEBUG, dev,
- "NCSI: vid %u already registered\n", vid);
+   netdev_dbg(dev, "NCSI: vid %u already registered\n",
+   

[PATCH 4/4] MAINTAINERS: Add Sam as the maintainer for NCSI

[Joel Stanley]

Sam has been handing the maintenance of NCSI for a number release cycles
now.

Signed-off-by: Joel Stanley 
---
 MAINTAINERS | 5 +
 1 file changed, 5 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9d5eeff51b5f..44851f7c46fc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9756,6 +9756,11 @@ L:   linux-s...@vger.kernel.org
 S: Maintained
 F: drivers/scsi/NCR_D700.*
 
+NCSI LIBRARY:
+M: Samuel Mendoza-Jonas 
+S: Maintained
+F: net/ncsi/
+
 NCT6775 HARDWARE MONITOR DRIVER
 M: Guenter Roeck 
 L: linux-hw...@vger.kernel.org
-- 
2.17.1

[PATCH 2/4] net/ncsi: Drop no more channels message

[Joel Stanley]

This does not provide useful information. As the ncsi maintainer said:

 > either we get a channel or broadcom has gone out to lunch

Signed-off-by: Joel Stanley 
---
 net/ncsi/ncsi-manage.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 616441c2b54f..716493a61ba6 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -1226,8 +1226,6 @@ int ncsi_process_next_channel(struct ncsi_dev_priv *ndp)
return ncsi_choose_active_channel(ndp);
}
 
-   netdev_printk(KERN_DEBUG, ndp->ndev.dev,
- "NCSI: No more channels to process\n");
ncsi_report_link(ndp, false);
return -ENODEV;
 }
-- 
2.17.1

[PATCH 1/4] net/ncsi: Silence debug messages

[Joel Stanley]

In normal operation we see this series of messages as the host drives
the network device:

 ftgmac100 1e66.ethernet eth0: NCSI: LSC AEN - channel 0 state down
 ftgmac100 1e66.ethernet eth0: NCSI: suspending channel 0
 ftgmac100 1e66.ethernet eth0: NCSI: configuring channel 0
 ftgmac100 1e66.ethernet eth0: NCSI: channel 0 link down after config
 ftgmac100 1e66.ethernet eth0: NCSI interface down
 ftgmac100 1e66.ethernet eth0: NCSI: LSC AEN - channel 0 state up
 ftgmac100 1e66.ethernet eth0: NCSI: configuring channel 0
 ftgmac100 1e66.ethernet eth0: NCSI interface up
 ftgmac100 1e66.ethernet eth0: NCSI: LSC AEN - channel 0 state down
 ftgmac100 1e66.ethernet eth0: NCSI: suspending channel 0
 ftgmac100 1e66.ethernet eth0: NCSI: configuring channel 0
 ftgmac100 1e66.ethernet eth0: NCSI: channel 0 link down after config
 ftgmac100 1e66.ethernet eth0: NCSI interface down
 ftgmac100 1e66.ethernet eth0: NCSI: LSC AEN - channel 0 state up
 ftgmac100 1e66.ethernet eth0: NCSI: configuring channel 0
 ftgmac100 1e66.ethernet eth0: NCSI interface up

This makes all of these messages netdev_dbg. They are still useful to
debug eg. misbehaving network device firmware, but we do not need them
filling up the kernel logs in normal operation.

Signed-off-by: Joel Stanley 
---
 drivers/net/ethernet/faraday/ftgmac100.c |  2 +-
 net/ncsi/ncsi-aen.c  |  4 ++--
 net/ncsi/ncsi-manage.c   | 14 +++---
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 78db8e62a83f..a78413d5bfde 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1735,7 +1735,7 @@ static void ftgmac100_ncsi_handler(struct ncsi_dev *nd)
if (unlikely(nd->state != ncsi_dev_state_functional))
return;
 
-   netdev_info(nd->dev, "NCSI interface %s\n",
+   netdev_dbg(nd->dev, "NCSI interface %s\n",
nd->link_up ? "up" : "down");
 }
 
diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
index e7b05de1e6d1..f899ed61bb57 100644
--- a/net/ncsi/ncsi-aen.c
+++ b/net/ncsi/ncsi-aen.c
@@ -73,8 +73,8 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp,
ncm->data[2] = data;
ncm->data[4] = ntohl(lsc->oem_status);
 
-   netdev_info(ndp->ndev.dev, "NCSI: LSC AEN - channel %u state %s\n",
-   nc->id, data & 0x1 ? "up" : "down");
+   netdev_dbg(ndp->ndev.dev, "NCSI: LSC AEN - channel %u state %s\n",
+  nc->id, data & 0x1 ? "up" : "down");
 
chained = !list_empty(>link);
state = nc->state;
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 5561e221b71f..616441c2b54f 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -816,9 +816,9 @@ static void ncsi_configure_channel(struct ncsi_dev_priv 
*ndp)
} else {
hot_nc = NULL;
nc->state = NCSI_CHANNEL_INACTIVE;
-   netdev_warn(ndp->ndev.dev,
-   "NCSI: channel %u link down after config\n",
-   nc->id);
+   netdev_dbg(ndp->ndev.dev,
+  "NCSI: channel %u link down after config\n",
+  nc->id);
}
spin_unlock_irqrestore(>lock, flags);
 
@@ -1199,14 +1199,14 @@ int ncsi_process_next_channel(struct ncsi_dev_priv *ndp)
switch (old_state) {
case NCSI_CHANNEL_INACTIVE:
ndp->ndev.state = ncsi_dev_state_config;
-   netdev_info(ndp->ndev.dev, "NCSI: configuring channel %u\n",
-   nc->id);
+   netdev_dbg(ndp->ndev.dev, "NCSI: configuring channel %u\n",
+  nc->id);
ncsi_configure_channel(ndp);
break;
case NCSI_CHANNEL_ACTIVE:
ndp->ndev.state = ncsi_dev_state_suspend;
-   netdev_info(ndp->ndev.dev, "NCSI: suspending channel %u\n",
-   nc->id);
+   netdev_dbg(ndp->ndev.dev, "NCSI: suspending channel %u\n",
+  nc->id);
ncsi_suspend_channel(ndp);
break;
default:
-- 
2.17.1

[PATCH] virto_net: remove empty file 'virtio_net.'

[Joel Stanley]

Looks like this was mistakenly added to the tree as part of
commit 186b3c998c50 ("virtio-net: support XDP_REDIRECT").

Signed-off-by: Joel Stanley <j...@jms.id.au>
---
 drivers/net/virtio_net. | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 delete mode 100644 drivers/net/virtio_net.

diff --git a/drivers/net/virtio_net. b/drivers/net/virtio_net.
deleted file mode 100644
index e69de29bb2d1..
-- 
2.14.1

[PATCH v3] net: ftgmac100: Request clock and set speed

[Joel Stanley]

According to the ASPEED datasheet, gigabit speeds require a clock of
100MHz or higher. Other speeds require 25MHz or higher. This patch
configures a 100MHz clock if the system has a direct-attached
PHY, or 25MHz if the system is running NC-SI which is limited to 100MHz.

There appear to be no other upstream users of the FTGMAC100 driver it is
hard to know the clocking requirements of other platforms. Therefore a
conservative approach was taken with enabling clocks. If the platform is
not ASPEED, both requesting the clock and configuring the speed is
skipped.

Signed-off-by: Joel Stanley <j...@jms.id.au>
---
Andrew, can you please give this one a spin on hardware?

v3:
 - Fix errors from v2
v2:
 - only touch the clocks on Aspeed platforms
 - unconditionally call clk_unprepare_disable

 drivers/net/ethernet/faraday/ftgmac100.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 9ed8e4b81530..78db8e62a83f 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -21,6 +21,7 @@
 
 #define pr_fmt(fmt)KBUILD_MODNAME ": " fmt
 
+#include 
 #include 
 #include 
 #include 
@@ -59,6 +60,9 @@
 /* Min number of tx ring entries before stopping queue */
 #define TX_THRESHOLD   (MAX_SKB_FRAGS + 1)
 
+#define FTGMAC_100MHZ  1
+#define FTGMAC_25MHZ   2500
+
 struct ftgmac100 {
/* Registers */
struct resource *res;
@@ -96,6 +100,7 @@ struct ftgmac100 {
struct napi_struct napi;
struct work_struct reset_task;
struct mii_bus *mii_bus;
+   struct clk *clk;
 
/* Link management */
int cur_speed;
@@ -1734,6 +1739,22 @@ static void ftgmac100_ncsi_handler(struct ncsi_dev *nd)
nd->link_up ? "up" : "down");
 }
 
+static void ftgmac100_setup_clk(struct ftgmac100 *priv)
+{
+   priv->clk = devm_clk_get(priv->dev, NULL);
+   if (IS_ERR(priv->clk))
+   return;
+
+   clk_prepare_enable(priv->clk);
+
+   /* Aspeed specifies a 100MHz clock is required for up to
+* 1000Mbit link speeds. As NCSI is limited to 100Mbit, 25MHz
+* is sufficient
+*/
+   clk_set_rate(priv->clk, priv->use_ncsi ? FTGMAC_25MHZ :
+   FTGMAC_100MHZ);
+}
+
 static int ftgmac100_probe(struct platform_device *pdev)
 {
struct resource *res;
@@ -1830,6 +1851,9 @@ static int ftgmac100_probe(struct platform_device *pdev)
goto err_setup_mdio;
}
 
+   if (priv->is_aspeed)
+   ftgmac100_setup_clk(priv);
+
/* Default ring sizes */
priv->rx_q_entries = priv->new_rx_q_entries = DEF_RX_QUEUE_ENTRIES;
priv->tx_q_entries = priv->new_tx_q_entries = DEF_TX_QUEUE_ENTRIES;
@@ -1883,6 +1907,8 @@ static int ftgmac100_remove(struct platform_device *pdev)
 
unregister_netdev(netdev);
 
+   clk_disable_unprepare(priv->clk);
+
/* There's a small chance the reset task will have been re-queued,
 * during stop, make sure it's gone before we free the structure.
 */
-- 
2.14.1

[PATCH v2] net: ftgmac100: Request clock and set speed

[Joel Stanley]

According to the ASPEED datasheet, gigabit speeds require a clock of
100MHz or higher. Other speeds require 25MHz or higher. This patch
configures a 100MHz clock if the system has a direct-attached
PHY, or 25MHz if the system is running NC-SI which is limited to 100MHz.

There appear to be no other upstream users of the FTGMAC100 driver so it
is hard to know the clocking requirements of other platforms. Therefore
a conservative approach was taken with enabling clocks. If the platform
is not ASPEED, both requesting the clock and configuring the speed is
skipped.

Signed-off-by: Joel Stanley <j...@jms.id.au>
---
Andrew, as I'm travelling can you please test this on the evb and a
palmetto? Use my wip/aspeed-v4.14-clk branch, or OpenBMC's dev-4.13.

David, please wait for Andrew's tested-by before applying.

Cheers!

v2:
 - only touch the clocks on Aspeed platforms
 - unconditionally call clk_unprepare_disable

 drivers/net/ethernet/faraday/ftgmac100.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 9ed8e4b81530..cd352bf41da1 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -21,6 +21,7 @@
 
 #define pr_fmt(fmt)KBUILD_MODNAME ": " fmt
 
+#include 
 #include 
 #include 
 #include 
@@ -59,6 +60,9 @@
 /* Min number of tx ring entries before stopping queue */
 #define TX_THRESHOLD   (MAX_SKB_FRAGS + 1)
 
+#define FTGMAC_100MHZ  1
+#define FTGMAC_25MHZ   2500
+
 struct ftgmac100 {
/* Registers */
struct resource *res;
@@ -96,6 +100,7 @@ struct ftgmac100 {
struct napi_struct napi;
struct work_struct reset_task;
struct mii_bus *mii_bus;
+   struct clk *clk;
 
/* Link management */
int cur_speed;
@@ -1734,6 +1739,22 @@ static void ftgmac100_ncsi_handler(struct ncsi_dev *nd)
nd->link_up ? "up" : "down");
 }
 
+static void ftgmac100_setup_clk(struct ftgmac100_priv *priv)
+{
+   priv->clk = devm_clk_get(>dev, NULL);
+   if (IS_ERR(priv->clk))
+   return;
+
+   clk_prepare_enable(priv->clk);
+
+   /* Aspeed specifies a 100MHz clock is required for up to
+* 1000Mbit link speeds. As NCSI is limited to 100Mbit, 25MHz
+* is sufficient
+*/
+   clk_set_rate(priv->clk, priv->is_ncsi ? FTGMAC_25MHZ :
+   FTGMAC_100MHZ);
+}
+
 static int ftgmac100_probe(struct platform_device *pdev)
 {
struct resource *res;
@@ -1830,6 +1851,9 @@ static int ftgmac100_probe(struct platform_device *pdev)
goto err_setup_mdio;
}
 
+   if (priv->is_aspeed)
+   ftgmac100_setup_clk(priv);
+
/* Default ring sizes */
priv->rx_q_entries = priv->new_rx_q_entries = DEF_RX_QUEUE_ENTRIES;
priv->tx_q_entries = priv->new_tx_q_entries = DEF_TX_QUEUE_ENTRIES;
@@ -1883,6 +1907,8 @@ static int ftgmac100_remove(struct platform_device *pdev)
 
unregister_netdev(netdev);
 
+   clk_disable_unprepare(priv->clk);
+
/* There's a small chance the reset task will have been re-queued,
 * during stop, make sure it's gone before we free the structure.
 */
-- 
2.14.1

Re: [PATCH] net: ftgmac100: Request clock and set speed

[Joel Stanley]

On Tue, Oct 10, 2017 at 4:14 PM, Benjamin Herrenschmidt
<b...@kernel.crashing.org> wrote:
> On Tue, 2017-10-10 at 15:19 +1030, Joel Stanley wrote:
>> According to the ASPEED datasheet, gigabit speeds require a clock of
>> 100MHz or higher. Other speeds require 25MHz or higher.
>
> Did you try "live" changing by either using ethtool or plugging into
> switches/hubs at different speed ?
>
> Also this is aspeed'isms, we should probably keep that under an
> is_aspeed test.
>
> My assumption is that we wouldn't bother, and just leave the freq
> set based on whether there's a physical gigabit capable connection or
> not (ie, real gigabit PHY vs. NC-SI really). But if it can help save a
> few milliwatts..

I didn't try changing the link speed at runtime. I don't have a setup
that lets me precisely measure power consumption, so it's hard to know
what the benefits are. In the future I can revisit this and do those
measurements.

I'll change it to be as you suggest; 100MHz for PHY and 50MHz for NC-SI.

Cheers,

Joel

Re: [PATCH] net: ftgmac100: Request clock and set speed

[Joel Stanley]

On Tue, Oct 10, 2017 at 2:34 PM, Florian Fainelli <f.faine...@gmail.com> wrote:
>
>
> On 10/09/2017 09:49 PM, Joel Stanley wrote:
>> According to the ASPEED datasheet, gigabit speeds require a clock of
>> 100MHz or higher. Other speeds require 25MHz or higher.
>>
>> Signed-off-by: Joel Stanley <j...@jms.id.au>
>> ---
>
>> @@ -161,6 +170,9 @@ static int ftgmac100_reset_and_config_mac(struct 
>> ftgmac100 *priv)
>>   break;
>>   }
>>
>> + if (freq && priv->clk)
>> + clk_set_rate(priv->clk, freq);
>
> Checking for priv->clk should not be necessary all public clk APIs can
> deal with a NULL clock pointer.

The intention was to set ->clk to NULL to indicate that there was no
clk, and therefore there is no reason to attempt to set the rate or
call prepare/unprepare.

If the we prefer to call the clk apis unconditionally I will send a v2
with the checks removed.

Thanks for the review.

Cheers,

Joel

[PATCH] net: ftgmac100: Request clock and set speed

[Joel Stanley]

According to the ASPEED datasheet, gigabit speeds require a clock of
100MHz or higher. Other speeds require 25MHz or higher.

Signed-off-by: Joel Stanley <j...@jms.id.au>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 9ed8e4b81530..870ebd857978 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -21,6 +21,7 @@
 
 #define pr_fmt(fmt)KBUILD_MODNAME ": " fmt
 
+#include 
 #include 
 #include 
 #include 
@@ -59,6 +60,9 @@
 /* Min number of tx ring entries before stopping queue */
 #define TX_THRESHOLD   (MAX_SKB_FRAGS + 1)
 
+#define FTGMAC_100MHZ  1
+#define FTGMAC_25MHZ   2500
+
 struct ftgmac100 {
/* Registers */
struct resource *res;
@@ -96,6 +100,7 @@ struct ftgmac100 {
struct napi_struct napi;
struct work_struct reset_task;
struct mii_bus *mii_bus;
+   struct clk *clk;
 
/* Link management */
int cur_speed;
@@ -142,18 +147,22 @@ static int ftgmac100_reset_mac(struct ftgmac100 *priv, 
u32 maccr)
 static int ftgmac100_reset_and_config_mac(struct ftgmac100 *priv)
 {
u32 maccr = 0;
+   int freq = 0;
 
switch (priv->cur_speed) {
case SPEED_10:
case 0: /* no link */
+   freq = FTGMAC_25MHZ;
break;
 
case SPEED_100:
maccr |= FTGMAC100_MACCR_FAST_MODE;
+   freq = FTGMAC_25MHZ;
break;
 
case SPEED_1000:
maccr |= FTGMAC100_MACCR_GIGA_MODE;
+   freq = FTGMAC_100MHZ;
break;
default:
netdev_err(priv->netdev, "Unknown speed %d !\n",
@@ -161,6 +170,9 @@ static int ftgmac100_reset_and_config_mac(struct ftgmac100 
*priv)
break;
}
 
+   if (freq && priv->clk)
+   clk_set_rate(priv->clk, freq);
+
/* (Re)initialize the queue pointers */
priv->rx_pointer = 0;
priv->tx_clean_pointer = 0;
@@ -1775,6 +1787,13 @@ static int ftgmac100_probe(struct platform_device *pdev)
priv->dev = >dev;
INIT_WORK(>reset_task, ftgmac100_reset_task);
 
+   /* Enable clock if present */
+   priv->clk = devm_clk_get(>dev, NULL);
+   if (!IS_ERR(priv->clk))
+   clk_prepare_enable(priv->clk);
+   else
+   priv->clk = NULL;
+
/* map io memory */
priv->res = request_mem_region(res->start, resource_size(res),
   dev_name(>dev));
@@ -1883,6 +1902,9 @@ static int ftgmac100_remove(struct platform_device *pdev)
 
unregister_netdev(netdev);
 
+   if (priv->clk)
+   clk_disable_unprepare(priv->clk);
+
/* There's a small chance the reset task will have been re-queued,
 * during stop, make sure it's gone before we free the structure.
 */
-- 
2.14.1

Re: [PATCH net-next v2 3/3] ftgmac100: Support NCSI VLAN filtering when available

[Joel Stanley]

On Mon, Aug 14, 2017 at 10:59 AM, Samuel Mendoza-Jonas
<s...@mendozajonas.com> wrote:
> Register the ndo_vlan_rx_{add,kill}_vid callbacks and set the
> NETIF_F_HW_VLAN_CTAG_FILTER if NCSI is available.
> This allows the VLAN core to notify the NCSI driver when changes occur
> so that the remote NCSI channel can be properly configured to filter on
> the set VLAN tags.
>
> Signed-off-by: Samuel Mendoza-Jonas <s...@mendozajonas.com>

I'm not a vlan expert, so I asked why this was only being set when
doing NCSI. The answer was:

> There is no point setting it when not doing ncsi. The HW doesn't have a
> filter in that case (we do have HW vlan tag extraction and injection,
> which my driver supports, but that's different flags).

Reviewed-by: Joel Stanley <j...@jms.id.au>

> ---
> v2: Moved ftgmac100 change into same patch and reordered
>
>  drivers/net/ethernet/faraday/ftgmac100.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
> b/drivers/net/ethernet/faraday/ftgmac100.c
> index 34dae51effd4..05fe7123d5ae 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -1623,6 +1623,8 @@ static const struct net_device_ops ftgmac100_netdev_ops 
> = {
>  #ifdef CONFIG_NET_POLL_CONTROLLER
> .ndo_poll_controller= ftgmac100_poll_controller,
>  #endif
> +   .ndo_vlan_rx_add_vid= ncsi_vlan_rx_add_vid,
> +   .ndo_vlan_rx_kill_vid   = ncsi_vlan_rx_kill_vid,
>  };
>
>  static int ftgmac100_setup_mdio(struct net_device *netdev)
> @@ -1837,6 +1839,9 @@ static int ftgmac100_probe(struct platform_device *pdev)
> NETIF_F_GRO | NETIF_F_SG | NETIF_F_HW_VLAN_CTAG_RX |
> NETIF_F_HW_VLAN_CTAG_TX;
>
> +   if (priv->use_ncsi)
> +   netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
> +
> /* AST2400  doesn't have working HW checksum generation */
> if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac")))
> netdev->hw_features &= ~NETIF_F_HW_CSUM;
> --
> 2.14.0
>

Re: [PATCH net-next v2 1/3] net/ncsi: Fix several packet definitions

[Joel Stanley]

On Mon, Aug 14, 2017 at 10:59 AM, Samuel Mendoza-Jonas
<s...@mendozajonas.com> wrote:

I asked Sam if these should be backported to stable and he said:

> These are straight up bugs except... without my changes we never call
> this code. As Ben says as time provides a lot of the current definitions
> need to be gone over, there's a few command/response code paths that are
> never triggered and could be broken in similar ways.

So we're okay here.

> Signed-off-by: Samuel Mendoza-Jonas <s...@mendozajonas.com>

Reviewed-by: Joel Stanley <j...@jms.id.au>

Cheers,

Joel

> ---
> v2: Rebased on latest net-next
>
>  net/ncsi/ncsi-cmd.c | 10 +-
>  net/ncsi/ncsi-pkt.h |  2 +-
>  net/ncsi/ncsi-rsp.c |  3 ++-
>  3 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
> index 5e03ed190e18..7567ca63aae2 100644
> --- a/net/ncsi/ncsi-cmd.c
> +++ b/net/ncsi/ncsi-cmd.c
> @@ -139,9 +139,9 @@ static int ncsi_cmd_handler_svf(struct sk_buff *skb,
> struct ncsi_cmd_svf_pkt *cmd;
>
> cmd = skb_put_zero(skb, sizeof(*cmd));
> -   cmd->vlan = htons(nca->words[0]);
> -   cmd->index = nca->bytes[2];
> -   cmd->enable = nca->bytes[3];
> +   cmd->vlan = htons(nca->words[1]);
> +   cmd->index = nca->bytes[6];
> +   cmd->enable = nca->bytes[7];
> ncsi_cmd_build_header(>cmd.common, nca);
>
> return 0;
> @@ -153,7 +153,7 @@ static int ncsi_cmd_handler_ev(struct sk_buff *skb,
> struct ncsi_cmd_ev_pkt *cmd;
>
> cmd = skb_put_zero(skb, sizeof(*cmd));
> -   cmd->mode = nca->bytes[0];
> +   cmd->mode = nca->bytes[3];
> ncsi_cmd_build_header(>cmd.common, nca);
>
> return 0;
> @@ -228,7 +228,7 @@ static struct ncsi_cmd_handler {
> { NCSI_PKT_CMD_AE, 8, ncsi_cmd_handler_ae  },
> { NCSI_PKT_CMD_SL, 8, ncsi_cmd_handler_sl  },
> { NCSI_PKT_CMD_GLS,0, ncsi_cmd_handler_default },
> -   { NCSI_PKT_CMD_SVF,4, ncsi_cmd_handler_svf },
> +   { NCSI_PKT_CMD_SVF,8, ncsi_cmd_handler_svf },
> { NCSI_PKT_CMD_EV, 4, ncsi_cmd_handler_ev  },
> { NCSI_PKT_CMD_DV, 0, ncsi_cmd_handler_default },
> { NCSI_PKT_CMD_SMA,8, ncsi_cmd_handler_sma },
> diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
> index 3ea49ed0a935..91b4b66438df 100644
> --- a/net/ncsi/ncsi-pkt.h
> +++ b/net/ncsi/ncsi-pkt.h
> @@ -104,7 +104,7 @@ struct ncsi_cmd_svf_pkt {
> unsigned char   index; /* VLAN table index  */
> unsigned char   enable;/* Enable or disable */
> __be32  checksum;  /* Checksum  */
> -   unsigned char   pad[14];
> +   unsigned char   pad[18];
>  };
>
>  /* Enable VLAN */
> diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
> index 087db775b3dc..c1a191d790e2 100644
> --- a/net/ncsi/ncsi-rsp.c
> +++ b/net/ncsi/ncsi-rsp.c
> @@ -354,7 +354,8 @@ static int ncsi_rsp_handler_svf(struct ncsi_request *nr)
>
> /* Add or remove the VLAN filter */
> if (!(cmd->enable & 0x1)) {
> -   ret = ncsi_remove_filter(nc, NCSI_FILTER_VLAN, cmd->index);
> +   /* HW indexes from 1 */
> +   ret = ncsi_remove_filter(nc, NCSI_FILTER_VLAN, cmd->index - 
> 1);
> } else {
> vlan = ntohs(cmd->vlan);
> ret = ncsi_add_filter(nc, NCSI_FILTER_VLAN, );
> --
> 2.14.0
>

Re: [PATCH net-next v2 2/3] net/ncsi: Configure VLAN tag filter

[Joel Stanley]

On Mon, Aug 14, 2017 at 10:59 AM, Samuel Mendoza-Jonas
 wrote:
> Make use of the ndo_vlan_rx_{add,kill}_vid callbacks to have the NCSI
> stack process new VLAN tags and configure the channel VLAN filter
> appropriately.
> Several VLAN tags can be set and a "Set VLAN Filter" packet must be sent
> for each one, meaning the ncsi_dev_state_config_svf state must be
> repeated. An internal list of VLAN tags is maintained, and compared
> against the current channel's ncsi_channel_filter in order to keep track
> within the state. VLAN filters are removed in a similar manner, with the
> introduction of the ncsi_dev_state_config_clear_vids state. The maximum
> number of VLAN tag filters is determined by the "Get Capabilities"
> response from the channel.
>
> Signed-off-by: Samuel Mendoza-Jonas 

I've given this some testing, but there are a few things I saw below
that we should sort out.

> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -38,6 +38,22 @@ static inline int ncsi_filter_size(int table)
> return sizes[table];
>  }
>
> +u32 *ncsi_get_filter(struct ncsi_channel *nc, int table, int index)
> +{
> +   struct ncsi_channel_filter *ncf;
> +   int size;
> +
> +   ncf = nc->filters[table];
> +   if (!ncf)
> +   return NULL;
> +
> +   size = ncsi_filter_size(table);
> +   if (size < 0)
> +   return NULL;
> +
> +   return ncf->data + size * index;
> +}
> +
>  int ncsi_find_filter(struct ncsi_channel *nc, int table, void *data)
>  {
> struct ncsi_channel_filter *ncf;
> @@ -58,7 +74,7 @@ int ncsi_find_filter(struct ncsi_channel *nc, int table, 
> void *data)
> index = -1;
> while ((index = find_next_bit(bitmap, ncf->total, index + 1))
>< ncf->total) {
> -   if (!memcmp(ncf->data + size * index, data, size)) {
> +   if (!data || !memcmp(ncf->data + size * index, data, size)) {

Not clear why this check is required?

> spin_unlock_irqrestore(>lock, flags);
> return index;
> }
> @@ -639,6 +655,83 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv 
> *ndp)
> nd->state = ncsi_dev_state_functional;
>  }
>
> +/* Check the VLAN filter bitmap for a set filter, and construct a
> + * "Set VLAN Filter - Disable" packet if found.
> + */
> +static int clear_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
> +struct ncsi_cmd_arg *nca)
> +{
> +   int index;
> +   u16 vid;
> +
> +   index = ncsi_find_filter(nc, NCSI_FILTER_VLAN, NULL);
> +   if (index < 0) {
> +   /* Filter table empty */
> +   return -1;
> +   }
> +
> +   vid = *(u16 *)ncsi_get_filter(nc, NCSI_FILTER_VLAN, index);

You just added this function that returns a pointer to a u32. It's
strange to see the only call site then throw half of it away.

Also, ncsi_get_filter can return NULL.

> +   netdev_printk(KERN_DEBUG, ndp->ndev.dev,
> + "ncsi: removed vlan tag %u at index %d\n",
> + vid, index + 1);
> +   ncsi_remove_filter(nc, NCSI_FILTER_VLAN, index);
> +
> +   nca->type = NCSI_PKT_CMD_SVF;
> +   nca->words[1] = vid;
> +   /* HW filter index starts at 1 */
> +   nca->bytes[6] = index + 1;
> +   nca->bytes[7] = 0x00;
> +   return 0;
> +}

> @@ -751,6 +876,26 @@ static void ncsi_configure_channel(struct ncsi_dev_priv 
> *ndp)
> break;
> case ncsi_dev_state_config_done:
> spin_lock_irqsave(>lock, flags);
> +   if (nc->reconfigure_needed) {
> +   /* This channel's configuration has been updated
> +* part-way during the config state - start the
> +* channel configuration over
> +*/
> +   nc->reconfigure_needed = false;
> +   nc->state = NCSI_CHANNEL_INVISIBLE;
> +   spin_unlock_irqrestore(>lock, flags);
> +
> +   spin_lock_irqsave(>lock, flags);
> +   nc->state = NCSI_CHANNEL_INACTIVE;

This looks strange. What's nc->state up to? Does setting it to
NCSI_CHANNEL_INVISIBLE have any affect?

The locking is confusing too.

> +   list_add_tail_rcu(>link, >channel_queue);
> +   spin_unlock_irqrestore(>lock, flags);
> +
> +   netdev_printk(KERN_DEBUG, dev,
> + "Dirty NCSI channel state reset\n");
> +   ncsi_process_next_channel(ndp);
> +   break;
> +   }
> +
> if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1) {
> hot_nc = nc;
> nc->state = NCSI_CHANNEL_ACTIVE;
> @@ -1191,6 +1336,149 @@ static struct notifier_block 

[PATCH] ftgmac100: return error in ftgmac100_alloc_rx_buf

[Joel Stanley]

The error paths set err, but it's not returned.

I wondered if we should fix all of the callers to check the returned
value, but Ben explains why the code is this way:

> Most call sites ignore it on purpose. There's nothing we can do if
> we fail to get a buffer at interrupt time, so we point the buffer to
> the scratch page so the HW doesn't DMA into lalaland and lose the
> packet.
>
> The one call site that tests and can fail is the one used when brining
> the interface up. If we fail to allocate at that point, we fail the
> ifup. But as you noticed, I do have a bug not returning the error.

Acked-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
Signed-off-by: Joel Stanley <j...@jms.id.au>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index fce0aa4f78b7..33b5c8eb9961 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -392,7 +392,7 @@ static int ftgmac100_alloc_rx_buf(struct ftgmac100 *priv, 
unsigned int entry,
struct net_device *netdev = priv->netdev;
struct sk_buff *skb;
dma_addr_t map;
-   int err;
+   int err = 0;
 
skb = netdev_alloc_skb_ip_align(netdev, RX_BUF_SIZE);
if (unlikely(!skb)) {
@@ -428,7 +428,7 @@ static int ftgmac100_alloc_rx_buf(struct ftgmac100 *priv, 
unsigned int entry,
else
rxdes->rxdes0 = 0;
 
-   return 0;
+   return err;
 }
 
 static unsigned int ftgmac100_next_rx_pointer(struct ftgmac100 *priv,
-- 
2.13.3

Re: [PATCH 2/2] ftgmac100: Make the MDIO bus a child of the ethernet device

[Joel Stanley]

On Mon, Jul 24, 2017 at 4:29 PM, Benjamin Herrenschmidt
<b...@kernel.crashing.org> wrote:
> Populate mii_bus->parent with our own platform device before
> registering, which makes it easier to locate the MDIO bus
> in sysfs when trying to diagnose problems.
>
> Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>

Acked-by: Joel Stanley <j...@jms.id.au>

> ---
>  drivers/net/ethernet/faraday/ftgmac100.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
> b/drivers/net/ethernet/faraday/ftgmac100.c
> index 2b17f7023d91..fce0aa4f78b7 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -1682,6 +1682,7 @@ static int ftgmac100_setup_mdio(struct net_device 
> *netdev)
> priv->mii_bus->name = "ftgmac100_mdio";
> snprintf(priv->mii_bus->id, MII_BUS_ID_SIZE, "%s-%d",
>  pdev->name, pdev->id);
> +   priv->mii_bus->parent = priv->dev;
> priv->mii_bus->priv = priv->netdev;
> priv->mii_bus->read = ftgmac100_mdiobus_read;
> priv->mii_bus->write = ftgmac100_mdiobus_write;
>

Re: [PATCH 1/2] ftgmac100: Increase reset timeout

[Joel Stanley]

On Mon, Jul 24, 2017 at 4:29 PM, Benjamin Herrenschmidt
<b...@kernel.crashing.org> wrote:
> We had reports of 50us not being sufficient to reset the MAC,
> thus hitting the "Hardware reset failed" error bringing the
> interface up on some AST2400 based machines.
>
> This bumps the timeout to 200us.
>
> Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>

I gave this a spin on the problematic hardware, as well as a few other
machines. Looks good, thanks Ben.

Tested-by: Joel Stanley <j...@jms.id.au>

Cheers,

Joel


> ---
>  drivers/net/ethernet/faraday/ftgmac100.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
> b/drivers/net/ethernet/faraday/ftgmac100.c
> index b16128d3..2b17f7023d91 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -125,7 +125,7 @@ static int ftgmac100_reset_mac(struct ftgmac100 *priv, 
> u32 maccr)
> iowrite32(maccr, priv->base + FTGMAC100_OFFSET_MACCR);
> iowrite32(maccr | FTGMAC100_MACCR_SW_RST,
>   priv->base + FTGMAC100_OFFSET_MACCR);
> -   for (i = 0; i < 50; i++) {
> +   for (i = 0; i < 200; i++) {
> unsigned int maccr;
>
> maccr = ioread32(priv->base + FTGMAC100_OFFSET_MACCR);

Re: [PATCH] net: phy: broadcom: Add support for the BCM54210E

[Joel Stanley]

On Wed, Apr 5, 2017 at 3:17 PM, Florian Fainelli <f.faine...@gmail.com> wrote:
>
>
> On 04/04/2017 10:33 PM, Joel Stanley wrote:
>> This device is a single-port RGMII 10/100/1000BASE-T PHY with EEE & WOL.
>
> This looks good, although Rafal did beat you to it:
>
> 0fc9ae107669760c2a8658cb5b5876dbe525e08d ("net: phy: broadcom: add
> support for BCM54210E")

Even better! Thank you.

Cheers,

Joel

[PATCH] net: phy: broadcom: Add support for the BCM54210E

[Joel Stanley]

This device is a single-port RGMII 10/100/1000BASE-T PHY with EEE & WOL.

Signed-off-by: Joel Stanley <j...@jms.id.au>
---
 drivers/net/phy/broadcom.c | 13 +
 include/linux/brcmphy.h|  2 ++
 2 files changed, 15 insertions(+)

diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index 9cd8b27d1292..3df826323129 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -703,6 +703,18 @@ static struct phy_driver broadcom_drivers[] = {
.read_status= genphy_read_status,
.ack_interrupt  = brcm_fet_ack_interrupt,
.config_intr= brcm_fet_config_intr,
+}, {
+   .phy_id = PHY_ID_BCM54210E,
+   .phy_id_mask= 0xfff0,
+   .name   = "Broadcom BCM54210E",
+   .features   = PHY_GBIT_FEATURES |
+   SUPPORTED_Pause | SUPPORTED_Asym_Pause,
+   .flags  = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+   .config_init= bcm54xx_config_init,
+   .config_aneg= genphy_config_aneg,
+   .read_status= genphy_read_status,
+   .ack_interrupt  = bcm_phy_ack_intr,
+   .config_intr= bcm_phy_config_intr,
 } };
 
 module_phy_driver(broadcom_drivers);
@@ -723,6 +735,7 @@ static struct mdio_device_id __maybe_unused broadcom_tbl[] 
= {
{ PHY_ID_BCM57780, 0xfff0 },
{ PHY_ID_BCMAC131, 0xfff0 },
{ PHY_ID_BCM5241, 0xfff0 },
+   { PHY_ID_BCM54210E, 0xfff0},
{ }
 };
 
diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
index 55e517130311..53106b9c89f1 100644
--- a/include/linux/brcmphy.h
+++ b/include/linux/brcmphy.h
@@ -40,6 +40,8 @@
 
 #define PHY_ID_BCM_CYGNUS  0xae025200
 
+#define PHY_ID_BCM54210E   0x600d84a0
+
 #define PHY_BCM_OUI_MASK   0xfc00
 #define PHY_BCM_OUI_1  0x00206000
 #define PHY_BCM_OUI_2  0x0143bc00
-- 
2.11.0

Re: [PATCH] net/faraday: Explicitly include linux/of.h and linux/property.h

[Joel Stanley]

On Fri, Mar 31, 2017 at 2:30 AM, Mark Brown <broo...@kernel.org> wrote:
> This driver uses interfaces from linux/of.h and linux/property.h but
> relies on implict inclusion of those headers which means that changes in
> other headers could break the build, as happened in -next for arm today.
> Add a explicit includes.
>
> Signed-off-by: Mark Brown <broo...@kernel.org>

Acked-by: Joel Stanley <j...@jms.id.au>

Thank you for fixing this Mark.

Cheers,

Joel

> ---
>  drivers/net/ethernet/faraday/ftgmac100.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
> b/drivers/net/ethernet/faraday/ftgmac100.c
> index 928b0df2b8e0..ade6b3e4ed13 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -28,8 +28,10 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>
> --
> 2.11.0
>

Re: [PATCH 1/2] net: phy: broadcom: Update Auxiliary Control Register macros

[Joel Stanley]

On Sat, Oct 22, 2016 at 3:50 AM, Xo Wang <x...@google.com> wrote:
> Add the RXD-to-RXC skew (delay) time bit in the Miscellaneous Control
> shadow register and a mask for the shadow selector field.
>
> Remove a re-definition of MII_BCM54XX_AUXCTL_SHDWSEL_AUXCTL.
>
> Signed-off-by: Xo Wang <x...@google.com>

Reviewed-by: Joel Stanley <j...@jms.id.au>

Cheers,

Joel

> ---
>  include/linux/brcmphy.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Re: [PATCH 2/2] net: phy: broadcom: Add support for BCM54612E

[Joel Stanley]

On Sat, Oct 22, 2016 at 3:50 AM, Xo Wang <x...@google.com> wrote:
> This PHY has internal delays enabled after reset. This clears the
> internal delay enables unless the interface specifically requests them.
>
> Signed-off-by: Xo Wang <x...@google.com>

Reviewed-by: Joel Stanley <j...@jms.id.au>

Cheers,

Joel

> ---
>  drivers/net/phy/broadcom.c | 48 
> ++
>  include/linux/brcmphy.h|  1 +
>  2 files changed, 49 insertions(+)

Re: [PATCH net 2/5] net/ncsi: Split out logic for ncsi_dev_state_suspend_select

[Joel Stanley]

Hi Gavin,

On Fri, Oct 14, 2016 at 1:23 PM, Gavin Shan  wrote:
> This splits out the code that handles ncsi_dev_state_suspend_select
> so that we can add more code to the handler in subsequent patch.
> Apart from adding a error tag to reuse the code in error path,
> no logical changes introduced.
>
> Signed-off-by: Gavin Shan 
> ---
>  net/ncsi/ncsi-manage.c | 38 +-
>  1 file changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> index 1bc96dc..5758a26 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -540,21 +540,30 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv 
> *ndp)
> nd->state = ncsi_dev_state_suspend_select;
> /* Fall through */
> case ncsi_dev_state_suspend_select:
> +   ndp->pending_req_num = 1;
> +
> +   nca.type = NCSI_PKT_CMD_SP;
> +   nca.package = np->id;
> +   nca.channel = NCSI_RESERVED_CHANNEL;
> +   if (ndp->flags & NCSI_DEV_HWA)
> +   nca.bytes[0] = 0;
> +   else
> +   nca.bytes[0] = 1;
> +
> +   nd->state = ncsi_dev_state_suspend_dcnt;
> +
> +   ret = ncsi_xmit_cmd();
> +   if (ret)
> +   goto error;
> +
> +   break;
> case ncsi_dev_state_suspend_dcnt:
> case ncsi_dev_state_suspend_dc:
> case ncsi_dev_state_suspend_deselect:
> ndp->pending_req_num = 1;
>
> nca.package = np->id;
> -   if (nd->state == ncsi_dev_state_suspend_select) {
> -   nca.type = NCSI_PKT_CMD_SP;
> -   nca.channel = NCSI_RESERVED_CHANNEL;
> -   if (ndp->flags & NCSI_DEV_HWA)
> -   nca.bytes[0] = 0;
> -   else
> -   nca.bytes[0] = 1;
> -   nd->state = ncsi_dev_state_suspend_dcnt;
> -   } else if (nd->state == ncsi_dev_state_suspend_dcnt) {
> +   if (nd->state == ncsi_dev_state_suspend_dcnt) {
> nca.type = NCSI_PKT_CMD_DCNT;
> nca.channel = nc->id;
> nd->state = ncsi_dev_state_suspend_dc;

This is a messy switch statement. How about break out out all of the
states as you've done with suspend_select, instead of grouping them
and then doing if ... else if .. else if. I realise there might be one
or two lines duplicated for each state, but I think that's okay at the
expense of readability.

Also, patch 1 could also be merged into this when making this cleanup.

What do you think?

> @@ -570,10 +579,8 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv 
> *ndp)
> }
>
> ret = ncsi_xmit_cmd();
> -   if (ret) {
> -   nd->state = ncsi_dev_state_functional;
> -   return;
> -   }
> +   if (ret)
> +   goto error;
>
> break;
> case ncsi_dev_state_suspend_done:
> @@ -587,6 +594,11 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv 
> *ndp)
> netdev_warn(nd->dev, "Wrong NCSI state 0x%x in suspend\n",
> nd->state);
> }
> +
> +   return;
> +
> +error:
> +   nd->state = ncsi_dev_state_functional;
>  }
>
>  static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> --
> 2.1.0
>

Re: [PATCH net 3/5] net/ncsi: Fix stale link state of inactive channels on failover

[Joel Stanley]

On Fri, Oct 14, 2016 at 1:23 PM, Gavin Shan  wrote:
> The issue was found on BCM5718 which has two NCSI channels in one
> package: C0 and C1. Both of them are connected to different LANs,
> means they are in link-up state and C0 is chosen  as the active
> one until resetting BCM5718 happens as below.
>
> Resetting BCM5718 results in LSC (Link State Change) AEN packet
> received on C0, meaning LSC AEN is missed on C1. When LSC AEN packet
> received on C0 to report link-down, it fails over to C1 because C1
> is in link-up state as software can see. However, C1 is in link-down
> state in hardware. It means the link state is out of synchronization
> between hardware and software, resulting in inappropriate channel (C1)
> selected as active one.
>
> This resolves the issue by sending separate GLS (Get Link Status)
> commands to all channels in the package before trying to do failover.
> The last link state on all channels in the package is retrieved. With
> it, C0 is selected as active one as expected.

I follow this, and can see that happening in the
ncsi_dev_state_suspend_gls state. However, what is

> -   nd->state = ncsi_dev_state_suspend_dcnt;
> +   if (ndp->flags & NCSI_DEV_RESHUFFLE)
> +   nd->state = ncsi_dev_state_suspend_gls;
> +   else
> +   nd->state = ncsi_dev_state_suspend_dcnt;

However, what is this doing? I'm not quite sure what
NCSI_DEV_RESHUFFLE is and why we enable it?

>
> ret = ncsi_xmit_cmd();
> if (ret)
> goto error;
>
> break;
> +   case ncsi_dev_state_suspend_gls:
> +   ndp->pending_req_num = np->channel_num;
> +
> +   nca.type = NCSI_PKT_CMD_GLS;
> +   nca.package = np->id;
> +   nd->state = ncsi_dev_state_suspend_dcnt;
> +
> +   NCSI_FOR_EACH_CHANNEL(np, nc) {
> +   nca.channel = nc->id;
> +   ret = ncsi_xmit_cmd();
> +   if (ret)
> +   goto error;
> +   }
> +
> +   break;
> case ncsi_dev_state_suspend_dcnt:
> case ncsi_dev_state_suspend_dc:
> case ncsi_dev_state_suspend_deselect:
> --
> 2.1.0
>

Re: [PATCH net 4/5] net/ncsi: Choose hot channel as active one if necessary

[Joel Stanley]

On Fri, Oct 14, 2016 at 1:23 PM, Gavin Shan  wrote:
> The issue was found on BCM5718 which has two NCSI channels in one
> package: C0 and C1. C0 is in link-up state while C1 is in link-down
> state. C0 is chosen as active channel until unplugging and plugging
> C0's cable:  On unplugging C0's cable, LSC (Link State Change) AEN
> packet received on C0 to report link-down event. After that, C1 is
> chosen as active channel. LSC AEN for link-up event is lost on C0
> when plugging C0's cable back. We lose the network even C0 is usable.

Why do we lose the LCS AEN packet?

Is this a bug in the BCM5718? If so, we shouldn't put it in the common
ncsi code without adding a quirk for that hardware.

>
> This resolves the issue by recording the (hot) channel that was ever
> chosen as active one. The hot channel is chosen to be active one
> if none of available channels in link-up state. With this, C0 is still
> the active one after unplugging C0's cable. LSC AEN packet received
> on C0 when plugging its cable back.
>
> Signed-off-by: Gavin Shan 
> ---
>  net/ncsi/internal.h|  1 +
>  net/ncsi/ncsi-manage.c | 22 +++---
>  2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
> index eac4858..1308a56 100644
> --- a/net/ncsi/internal.h
> +++ b/net/ncsi/internal.h
> @@ -265,6 +265,7 @@ struct ncsi_dev_priv {
>  #endif
> unsigned intpackage_num; /* Number of packages */
> struct list_headpackages;/* List of packages   */
> +   struct ncsi_channel *hot_channel;/* Channel was ever active*/
> struct ncsi_request requests[256];   /* Request table  */
> unsigned intrequest_id;  /* Last used request ID   */
>  #define NCSI_REQ_START_IDX 1
> diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> index e959979..cccedcf 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -625,6 +625,7 @@ static void ncsi_configure_channel(struct ncsi_dev_priv 
> *ndp)
> struct net_device *dev = nd->dev;
> struct ncsi_package *np = ndp->active_package;
> struct ncsi_channel *nc = ndp->active_channel;
> +   struct ncsi_channel *hot_nc = NULL;
> struct ncsi_cmd_arg nca;
> unsigned char index;
> unsigned long flags;
> @@ -730,12 +731,20 @@ static void ncsi_configure_channel(struct ncsi_dev_priv 
> *ndp)
> break;
> case ncsi_dev_state_config_done:
> spin_lock_irqsave(>lock, flags);
> -   if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1)
> +   if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1) {
> +   hot_nc = nc;
> nc->state = NCSI_CHANNEL_ACTIVE;
> -   else
> +   } else {
> +   hot_nc = NULL;
> nc->state = NCSI_CHANNEL_INACTIVE;
> +   }
> spin_unlock_irqrestore(>lock, flags);
>
> +   /* Update the hot channel */
> +   spin_lock_irqsave(>lock, flags);
> +   ndp->hot_channel = hot_nc;
> +   spin_unlock_irqrestore(>lock, flags);
> +
> ncsi_start_channel_monitor(nc);
> ncsi_process_next_channel(ndp);
> break;
> @@ -753,10 +762,14 @@ static void ncsi_configure_channel(struct ncsi_dev_priv 
> *ndp)
>  static int ncsi_choose_active_channel(struct ncsi_dev_priv *ndp)
>  {
> struct ncsi_package *np;
> -   struct ncsi_channel *nc, *found;
> +   struct ncsi_channel *nc, *found, *hot_nc;
> struct ncsi_channel_mode *ncm;
> unsigned long flags;
>
> +   spin_lock_irqsave(>lock, flags);
> +   hot_nc = ndp->hot_channel;
> +   spin_unlock_irqrestore(>lock, flags);
> +
> /* The search is done once an inactive channel with up
>  * link is found.
>  */
> @@ -774,6 +787,9 @@ static int ncsi_choose_active_channel(struct 
> ncsi_dev_priv *ndp)
> if (!found)
> found = nc;
>
> +   if (nc == hot_nc)
> +   found = nc;
> +
> ncm = >modes[NCSI_MODE_LINK];
> if (ncm->data[2] & 0x1) {
> spin_unlock_irqrestore(>lock, flags);
> --
> 2.1.0
>

[PATCH net-next v2 3/6] net/faraday: Adapt for Aspeed SoCs

[Joel Stanley]

The RXDES and TXDES registers bits in the ftgmac100 indicates EDO{R,T}R
at bit position 15 for the Faraday Tech IP. However, the version of this
IP present in the Aspeed SoCs has these bits at position 30 in the
registers.

It appers that ast2400 SoCs support both positions, with the 15th bit
marked as reserved but still functional. In the ast2500 this bit is
reused for another function, so we need a work around.

This was confirmed with engineers from Aspeed that using bit 30 is
correct for both the ast2400 and ast2500 SoCs.

Signed-off-by: Joel Stanley <j...@jms.id.au>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 62a88d1a1f99..47f512224b57 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1345,9 +1345,6 @@ static int ftgmac100_probe(struct platform_device *pdev)
priv->netdev = netdev;
priv->dev = >dev;
 
-   priv->rxdes0_edorr_mask = BIT(15);
-   priv->txdes0_edotr_mask = BIT(15);
-
spin_lock_init(>tx_lock);
 
/* initialize NAPI */
@@ -1381,6 +1378,16 @@ static int ftgmac100_probe(struct platform_device *pdev)
  FTGMAC100_INT_PHYSTS_CHG |
  FTGMAC100_INT_RPKT_BUF |
  FTGMAC100_INT_NO_RXBUF);
+
+   if (of_machine_is_compatible("aspeed,ast2400") ||
+   of_machine_is_compatible("aspeed,ast2500")) {
+   priv->rxdes0_edorr_mask = BIT(30);
+   priv->txdes0_edotr_mask = BIT(30);
+   } else {
+   priv->rxdes0_edorr_mask = BIT(15);
+   priv->txdes0_edotr_mask = BIT(15);
+   }
+
if (pdev->dev.of_node &&
of_get_property(pdev->dev.of_node, "use-ncsi", NULL)) {
if (!IS_ENABLED(CONFIG_NET_NCSI)) {
-- 
2.9.3

[PATCH net-next v2 6/6] net/faraday: Mask out PHYSTS_CHG interrupt

[Joel Stanley]

The PHYSTS_CHG (the ftgmac100's PHY IRQ) is telling the system to go
look at the PHY registers for a link status change.

The interrupt was causing issues on Aspeed SoC where some board designs
had an active high configuration, some active low, and in some cases
repurposed for other functions. When misconfigured Linux would chew 100%
of CPU cycles servicing interrupts:

 [   20.28] ftgmac100 1e66.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
 [   20.28] ftgmac100 1e66.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
 [   20.28] ftgmac100 1e66.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
 [   20.30] ftgmac100 1e66.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG

While in the ftgmac100 IP can be configured for high, low and edge
sensitivity the current driver always polls the PHY, so we chose to mask
out the interrupt.

See https://patchwork.ozlabs.org/patch/672099/ for more discussion.

Signed-off-by: Joel Stanley <j...@jms.id.au>
---
v2:
 - Reworked to mask out PHYSTS_CHG instead of trying to determine the IRQ line
   level

 drivers/net/ethernet/faraday/ftgmac100.c | 10 +++---
 drivers/net/ethernet/faraday/ftgmac100.h |  1 +
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index e3653b14008a..90f9c5481290 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1075,14 +1075,12 @@ static int ftgmac100_poll(struct napi_struct *napi, int 
budget)
}
 
if (status & priv->int_mask_all & (FTGMAC100_INT_NO_RXBUF |
-   FTGMAC100_INT_RPKT_LOST | FTGMAC100_INT_AHB_ERR |
-   FTGMAC100_INT_PHYSTS_CHG)) {
+   FTGMAC100_INT_RPKT_LOST | FTGMAC100_INT_AHB_ERR)) {
if (net_ratelimit())
-   netdev_info(netdev, "[ISR] = 0x%x: %s%s%s%s\n", status,
+   netdev_info(netdev, "[ISR] = 0x%x: %s%s%s\n", status,
status & FTGMAC100_INT_NO_RXBUF ? "NO_RXBUF 
" : "",
status & FTGMAC100_INT_RPKT_LOST ? 
"RPKT_LOST " : "",
-   status & FTGMAC100_INT_AHB_ERR ? "AHB_ERR " 
: "",
-   status & FTGMAC100_INT_PHYSTS_CHG ? 
"PHYSTS_CHG" : "");
+   status & FTGMAC100_INT_AHB_ERR ? "AHB_ERR " 
: "");
 
if (status & FTGMAC100_INT_NO_RXBUF) {
/* RX buffer unavailable */
@@ -1390,7 +1388,6 @@ static int ftgmac100_probe(struct platform_device *pdev)
  FTGMAC100_INT_XPKT_ETH |
  FTGMAC100_INT_XPKT_LOST |
  FTGMAC100_INT_AHB_ERR |
- FTGMAC100_INT_PHYSTS_CHG |
  FTGMAC100_INT_RPKT_BUF |
  FTGMAC100_INT_NO_RXBUF);
 
@@ -1412,7 +1409,6 @@ static int ftgmac100_probe(struct platform_device *pdev)
 
dev_info(>dev, "Using NCSI interface\n");
priv->use_ncsi = true;
-   priv->int_mask_all &= ~FTGMAC100_INT_PHYSTS_CHG;
priv->ndev = ncsi_register_dev(netdev, ftgmac100_ncsi_handler);
if (!priv->ndev)
goto err_ncsi_dev;
diff --git a/drivers/net/ethernet/faraday/ftgmac100.h 
b/drivers/net/ethernet/faraday/ftgmac100.h
index 8a377ab1d127..a7ce0ac8858a 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.h
+++ b/drivers/net/ethernet/faraday/ftgmac100.h
@@ -157,6 +157,7 @@
 #define FTGMAC100_MACCR_FULLDUP(1 << 8)
 #define FTGMAC100_MACCR_GIGA_MODE  (1 << 9)
 #define FTGMAC100_MACCR_CRC_APD(1 << 10)
+#define FTGMAC100_MACCR_PHY_LINK_LEVEL (1 << 11)
 #define FTGMAC100_MACCR_RX_RUNT(1 << 12)
 #define FTGMAC100_MACCR_JUMBO_LF   (1 << 13)
 #define FTGMAC100_MACCR_RX_ALL (1 << 14)
-- 
2.9.3

[PATCH net-next v2 1/6] net/faraday: Separate rx page storage from rxdesc

[Joel Stanley]

From: Andrew Jeffery <and...@aj.id.au>

The ftgmac100 hardware revision in e.g. the Aspeed AST2500 no longer
reserves all bits in RXDES#2 but instead uses the bottom 16 bits to
store MAC frame metadata. Avoid corruption by shifting struct page
pointers out to their own member in struct ftgmac100.

Signed-off-by: Andrew Jeffery <and...@aj.id.au>
Signed-off-by: Joel Stanley <j...@jms.id.au>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 25 ++---
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 36361f8bf894..40622567159a 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -60,6 +60,8 @@ struct ftgmac100 {
struct ftgmac100_descs *descs;
dma_addr_t descs_dma_addr;
 
+   struct page *rx_pages[RX_QUEUE_ENTRIES];
+
unsigned int rx_pointer;
unsigned int tx_clean_pointer;
unsigned int tx_pointer;
@@ -341,18 +343,27 @@ static bool ftgmac100_rxdes_ipcs_err(struct 
ftgmac100_rxdes *rxdes)
return rxdes->rxdes1 & cpu_to_le32(FTGMAC100_RXDES1_IP_CHKSUM_ERR);
 }
 
+static inline struct page **ftgmac100_rxdes_page_slot(struct ftgmac100 *priv,
+ struct ftgmac100_rxdes 
*rxdes)
+{
+   return >rx_pages[rxdes - priv->descs->rxdes];
+}
+
 /*
  * rxdes2 is not used by hardware. We use it to keep track of page.
  * Since hardware does not touch it, we can skip cpu_to_le32()/le32_to_cpu().
  */
-static void ftgmac100_rxdes_set_page(struct ftgmac100_rxdes *rxdes, struct 
page *page)
+static void ftgmac100_rxdes_set_page(struct ftgmac100 *priv,
+struct ftgmac100_rxdes *rxdes,
+struct page *page)
 {
-   rxdes->rxdes2 = (unsigned int)page;
+   *ftgmac100_rxdes_page_slot(priv, rxdes) = page;
 }
 
-static struct page *ftgmac100_rxdes_get_page(struct ftgmac100_rxdes *rxdes)
+static struct page *ftgmac100_rxdes_get_page(struct ftgmac100 *priv,
+struct ftgmac100_rxdes *rxdes)
 {
-   return (struct page *)rxdes->rxdes2;
+   return *ftgmac100_rxdes_page_slot(priv, rxdes);
 }
 
 /**
@@ -501,7 +512,7 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int 
*processed)
 
do {
dma_addr_t map = ftgmac100_rxdes_get_dma_addr(rxdes);
-   struct page *page = ftgmac100_rxdes_get_page(rxdes);
+   struct page *page = ftgmac100_rxdes_get_page(priv, rxdes);
unsigned int size;
 
dma_unmap_page(priv->dev, map, RX_BUF_SIZE, DMA_FROM_DEVICE);
@@ -779,7 +790,7 @@ static int ftgmac100_alloc_rx_page(struct ftgmac100 *priv,
return -ENOMEM;
}
 
-   ftgmac100_rxdes_set_page(rxdes, page);
+   ftgmac100_rxdes_set_page(priv, rxdes, page);
ftgmac100_rxdes_set_dma_addr(rxdes, map);
ftgmac100_rxdes_set_dma_own(rxdes);
return 0;
@@ -791,7 +802,7 @@ static void ftgmac100_free_buffers(struct ftgmac100 *priv)
 
for (i = 0; i < RX_QUEUE_ENTRIES; i++) {
struct ftgmac100_rxdes *rxdes = >descs->rxdes[i];
-   struct page *page = ftgmac100_rxdes_get_page(rxdes);
+   struct page *page = ftgmac100_rxdes_get_page(priv, rxdes);
dma_addr_t map = ftgmac100_rxdes_get_dma_addr(rxdes);
 
if (!page)
-- 
2.9.3

Re: [PATCH net-next 0/7] ftgmac100 support for ast2500

[Joel Stanley]

Please ignore this one.

On Thu, Sep 22, 2016 at 8:33 AM, Joel Stanley <j...@jms.id.au> wrote:
> Hello Dave,
>
> This series adds support to the ftgmac100 driver for the Aspeed ast2400 and
> ast2500 SoCs. In particular, they ensure the driver works correctly on the
> ast2500 where the MAC block has seen some changes in register layout.
>
> They have been tested on ast2400 and ast2500 systems with the NCSI stack and
> with a directly attached PHY.
>
> Cheers,
>
> Joel
>
> Andrew Jeffery (2):
>   net/ftgmac100: Separate rx page storage from rxdesc
>   net/ftgmac100: Make EDO{R,T}R bits configurable
>
> Gavin Shan (2):
>   net/faraday: Avoid PHYSTS_CHG interrupt
>   net/faraday: Clear stale interrupts
>
> Joel Stanley (3):
>   net/ftgmac100: Adapt for Aspeed SoCs
>   net/faraday: Fix phy link irq on Aspeed G5 SoCs
>   net/faraday: Configure old MDIO interface on Aspeed SoCs
>
>  drivers/net/ethernet/faraday/ftgmac100.c | 92 
> 
>  drivers/net/ethernet/faraday/ftgmac100.h |  8 ++-
>  2 files changed, 77 insertions(+), 23 deletions(-)
>
> --
> 2.9.3
>

[PATCH net-next v2 4/6] net/faraday: Clear stale interrupts

[Joel Stanley]

From: Gavin Shan <gws...@linux.vnet.ibm.com>

There is stale interrupt (PHYSTS_CHG in ISR, bit#6 in 0x0) from
the bootloader (uboot) when enabling the MAC. The stale interrupts
aren't part of kernel and should be cleared.

This clears the stale interrupts in ISR (0x0) when enabling the MAC.

Signed-off-by: Gavin Shan <gws...@linux.vnet.ibm.com>
Signed-off-by: Joel Stanley <j...@jms.id.au>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 47f512224b57..189373743ddf 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1112,6 +1112,7 @@ static int ftgmac100_poll(struct napi_struct *napi, int 
budget)
 static int ftgmac100_open(struct net_device *netdev)
 {
struct ftgmac100 *priv = netdev_priv(netdev);
+   unsigned int status;
int err;
 
err = ftgmac100_alloc_buffers(priv);
@@ -1137,6 +1138,11 @@ static int ftgmac100_open(struct net_device *netdev)
 
ftgmac100_init_hw(priv);
ftgmac100_start_hw(priv, priv->use_ncsi ? 100 : 10);
+
+   /* Clear stale interrupts */
+   status = ioread32(priv->base + FTGMAC100_OFFSET_ISR);
+   iowrite32(status, priv->base + FTGMAC100_OFFSET_ISR);
+
if (netdev->phydev)
phy_start(netdev->phydev);
else if (priv->use_ncsi)
-- 
2.9.3

[PATCH net-next v2 5/6] net/faraday: Configure old MDIO interface on Aspeed SoCs

[Joel Stanley]

The Aspeed SoCs have a new MDIO interface as an option in the G4 and G5
SoCs. The old one is still available, so select it in order to remain
compatible with the ftgmac100 driver.

Signed-off-by: Joel Stanley <j...@jms.id.au>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 9 +
 drivers/net/ethernet/faraday/ftgmac100.h | 5 +
 2 files changed, 14 insertions(+)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 189373743ddf..e3653b14008a 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1252,12 +1252,21 @@ static int ftgmac100_setup_mdio(struct net_device 
*netdev)
struct ftgmac100 *priv = netdev_priv(netdev);
struct platform_device *pdev = to_platform_device(priv->dev);
int i, err = 0;
+   u32 reg;
 
/* initialize mdio bus */
priv->mii_bus = mdiobus_alloc();
if (!priv->mii_bus)
return -EIO;
 
+   if (of_machine_is_compatible("aspeed,ast2400") ||
+   of_machine_is_compatible("aspeed,ast2500")) {
+   /* This driver supports the old MDIO interface */
+   reg = ioread32(priv->base + FTGMAC100_OFFSET_REVR);
+   reg &= ~FTGMAC100_REVR_NEW_MDIO_INTERFACE;
+   iowrite32(reg, priv->base + FTGMAC100_OFFSET_REVR);
+   };
+
priv->mii_bus->name = "ftgmac100_mdio";
snprintf(priv->mii_bus->id, MII_BUS_ID_SIZE, "%s-%d",
 pdev->name, pdev->id);
diff --git a/drivers/net/ethernet/faraday/ftgmac100.h 
b/drivers/net/ethernet/faraday/ftgmac100.h
index c258586ce4a4..8a377ab1d127 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.h
+++ b/drivers/net/ethernet/faraday/ftgmac100.h
@@ -134,6 +134,11 @@
 #define FTGMAC100_DMAFIFOS_TXDMA_REQ   (1 << 31)
 
 /*
+ * Feature Register
+ */
+#define FTGMAC100_REVR_NEW_MDIO_INTERFACE  BIT(31)
+
+/*
  * Receive buffer size register
  */
 #define FTGMAC100_RBSR_SIZE(x) ((x) & 0x3fff)
-- 
2.9.3

[PATCH net-next v2 0/6] ftgmac100 support for ast2500

[Joel Stanley]

Hello Dave,

This series adds support to the ftgmac100 driver for the Aspeed ast2400 and
ast2500 SoCs. In particular, they ensure the driver works correctly on the
ast2500 where the MAC block has seen some changes in register layout.

They have been tested on ast2400 and ast2500 systems with the NCSI stack and
with a directly attached PHY.

V2 reworks the two patches relating to PHYSTS_CHG into the one patch that
disables the interrupt instead of playing with interrupt sensitivity. I kept
patch 4 'net/faraday: Clear stale interrupts' which was first introduced to
clear the stale PHYSTS_CHG interrupt, as it helps keep us safe from unhygienic
(vendor) bootloaders.

Cheers,

Joel

Andrew Jeffery (2):
  net/faraday: Separate rx page storage from rxdesc
  net/faraday: Make EDO{R,T}R bits configurable

Gavin Shan (1):
  net/faraday: Clear stale interrupts

Joel Stanley (3):
  net/faraday: Adapt for Aspeed SoCs
  net/faraday: Configure old MDIO interface on Aspeed SoCs
  net/faraday: Mask out PHYSTS_CHG interrupt

 drivers/net/ethernet/faraday/ftgmac100.c | 97 +++-
 drivers/net/ethernet/faraday/ftgmac100.h |  8 ++-
 2 files changed, 75 insertions(+), 30 deletions(-)

-- 
2.9.3

[PATCH net-next v2 2/6] net/faraday: Make EDO{R,T}R bits configurable

[Joel Stanley]

From: Andrew Jeffery <and...@aj.id.au>

These bits are #defined at a fixed location. In order to support future
hardware that has chosen to move these bits around move the bits into a
member of the struct ftgmac100.

Signed-off-by: Andrew Jeffery <and...@aj.id.au>
Signed-off-by: Joel Stanley <j...@jms.id.au>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 40 +---
 drivers/net/ethernet/faraday/ftgmac100.h |  2 --
 2 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 40622567159a..62a88d1a1f99 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -79,6 +79,9 @@ struct ftgmac100 {
int int_mask_all;
bool use_ncsi;
bool enabled;
+
+   u32 rxdes0_edorr_mask;
+   u32 txdes0_edotr_mask;
 };
 
 static int ftgmac100_alloc_rx_page(struct ftgmac100 *priv,
@@ -259,10 +262,11 @@ static bool ftgmac100_rxdes_packet_ready(struct 
ftgmac100_rxdes *rxdes)
return rxdes->rxdes0 & cpu_to_le32(FTGMAC100_RXDES0_RXPKT_RDY);
 }
 
-static void ftgmac100_rxdes_set_dma_own(struct ftgmac100_rxdes *rxdes)
+static void ftgmac100_rxdes_set_dma_own(const struct ftgmac100 *priv,
+   struct ftgmac100_rxdes *rxdes)
 {
/* clear status bits */
-   rxdes->rxdes0 &= cpu_to_le32(FTGMAC100_RXDES0_EDORR);
+   rxdes->rxdes0 &= cpu_to_le32(priv->rxdes0_edorr_mask);
 }
 
 static bool ftgmac100_rxdes_rx_error(struct ftgmac100_rxdes *rxdes)
@@ -300,9 +304,10 @@ static bool ftgmac100_rxdes_multicast(struct 
ftgmac100_rxdes *rxdes)
return rxdes->rxdes0 & cpu_to_le32(FTGMAC100_RXDES0_MULTICAST);
 }
 
-static void ftgmac100_rxdes_set_end_of_ring(struct ftgmac100_rxdes *rxdes)
+static void ftgmac100_rxdes_set_end_of_ring(const struct ftgmac100 *priv,
+   struct ftgmac100_rxdes *rxdes)
 {
-   rxdes->rxdes0 |= cpu_to_le32(FTGMAC100_RXDES0_EDORR);
+   rxdes->rxdes0 |= cpu_to_le32(priv->rxdes0_edorr_mask);
 }
 
 static void ftgmac100_rxdes_set_dma_addr(struct ftgmac100_rxdes *rxdes,
@@ -393,7 +398,7 @@ ftgmac100_rx_locate_first_segment(struct ftgmac100 *priv)
if (ftgmac100_rxdes_first_segment(rxdes))
return rxdes;
 
-   ftgmac100_rxdes_set_dma_own(rxdes);
+   ftgmac100_rxdes_set_dma_own(priv, rxdes);
ftgmac100_rx_pointer_advance(priv);
rxdes = ftgmac100_current_rxdes(priv);
}
@@ -464,7 +469,7 @@ static void ftgmac100_rx_drop_packet(struct ftgmac100 *priv)
if (ftgmac100_rxdes_last_segment(rxdes))
done = true;
 
-   ftgmac100_rxdes_set_dma_own(rxdes);
+   ftgmac100_rxdes_set_dma_own(priv, rxdes);
ftgmac100_rx_pointer_advance(priv);
rxdes = ftgmac100_current_rxdes(priv);
} while (!done && ftgmac100_rxdes_packet_ready(rxdes));
@@ -556,10 +561,11 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, 
int *processed)
 /**
  * internal functions (transmit descriptor)
  */
-static void ftgmac100_txdes_reset(struct ftgmac100_txdes *txdes)
+static void ftgmac100_txdes_reset(const struct ftgmac100 *priv,
+ struct ftgmac100_txdes *txdes)
 {
/* clear all except end of ring bit */
-   txdes->txdes0 &= cpu_to_le32(FTGMAC100_TXDES0_EDOTR);
+   txdes->txdes0 &= cpu_to_le32(priv->txdes0_edotr_mask);
txdes->txdes1 = 0;
txdes->txdes2 = 0;
txdes->txdes3 = 0;
@@ -580,9 +586,10 @@ static void ftgmac100_txdes_set_dma_own(struct 
ftgmac100_txdes *txdes)
txdes->txdes0 |= cpu_to_le32(FTGMAC100_TXDES0_TXDMA_OWN);
 }
 
-static void ftgmac100_txdes_set_end_of_ring(struct ftgmac100_txdes *txdes)
+static void ftgmac100_txdes_set_end_of_ring(const struct ftgmac100 *priv,
+   struct ftgmac100_txdes *txdes)
 {
-   txdes->txdes0 |= cpu_to_le32(FTGMAC100_TXDES0_EDOTR);
+   txdes->txdes0 |= cpu_to_le32(priv->txdes0_edotr_mask);
 }
 
 static void ftgmac100_txdes_set_first_segment(struct ftgmac100_txdes *txdes)
@@ -701,7 +708,7 @@ static bool ftgmac100_tx_complete_packet(struct ftgmac100 
*priv)
 
dev_kfree_skb(skb);
 
-   ftgmac100_txdes_reset(txdes);
+   ftgmac100_txdes_reset(priv, txdes);
 
ftgmac100_tx_clean_pointer_advance(priv);
 
@@ -792,7 +799,7 @@ static int ftgmac100_alloc_rx_page(struct ftgmac100 *priv,
 
ftgmac100_rxdes_set_page(priv, rxdes, page);
ftgmac100_rxdes_set_dma_addr(rxdes, map);
-   ftgmac100_rxdes_set_dma_own(rxdes);

[PATCH net-next 0/7] ftgmac100 support for ast2500

[Joel Stanley]

Hello Dave,

This series adds support to the ftgmac100 driver for the Aspeed ast2400 and
ast2500 SoCs. In particular, they ensure the driver works correctly on the
ast2500 where the MAC block has seen some changes in register layout.

They have been tested on ast2400 and ast2500 systems with the NCSI stack and
with a directly attached PHY.

Cheers,

Joel

Andrew Jeffery (2):
  net/ftgmac100: Separate rx page storage from rxdesc
  net/ftgmac100: Make EDO{R,T}R bits configurable

Gavin Shan (2):
  net/faraday: Avoid PHYSTS_CHG interrupt
  net/faraday: Clear stale interrupts

Joel Stanley (3):
  net/ftgmac100: Adapt for Aspeed SoCs
  net/faraday: Fix phy link irq on Aspeed G5 SoCs
  net/faraday: Configure old MDIO interface on Aspeed SoCs

 drivers/net/ethernet/faraday/ftgmac100.c | 92 
 drivers/net/ethernet/faraday/ftgmac100.h |  8 ++-
 2 files changed, 77 insertions(+), 23 deletions(-)

-- 
2.9.3

Re: [PATCH net-next 6/7] net/faraday: Fix phy link irq on Aspeed G5 SoCs

[Joel Stanley]

On Wed, Sep 21, 2016 at 6:33 PM, Benjamin Herrenschmidt
<b...@kernel.crashing.org> wrote:
> On Wed, 2016-09-21 at 11:32 +0930, Joel Stanley wrote:
>> I had a look at the eval board schematic and it appears that the line
>> has pull down resistors on it, explaining why the IRQ fires when it's
>> configured to active low. Other machines re-use the pin pin as a GPIO.
>> So yes, I will change this to a dt property in v2. That will mean
>> dropping 4/7 "net/faraday: Avoid PHYSTS_CHG interrupt" as well.
>
> What line is it out of the PHY ? The PHY IRQ ? If yes then it's meant
> to be telling you to go look at the PHY registers for a link status
> change, but only works if the PHY has also been configured
> appropriately...

Yep, PHY IRQ.

> Mostly we ignore those things in Linux and just poll the PHY.

That's simpler. It's what we're doing on Aspeed systems when using NSCI already.

The driver is already polling the PHY, I propose we mask out this
interrupt for all systems. I gave that a run on my ast2500evb and it
behaved itself.

Cheers,

Joe

Re: [PATCH net-next 6/7] net/faraday: Fix phy link irq on Aspeed G5 SoCs

[Joel Stanley]

On Wed, Sep 21, 2016 at 12:59 AM, Andrew Lunn <and...@lunn.ch> wrote:
> On Tue, Sep 20, 2016 at 10:13:14PM +1000, Benjamin Herrenschmidt wrote:
>> On Tue, 2016-09-20 at 16:00 +0930, Joel Stanley wrote:
>> > On Aspeed SoC with a direct PHY connection (non-NSCI), we receive
>> > continual PHYSTS interrupts:
>> >
>> >  [   20.28] ftgmac100 1e66.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
>> >  [   20.28] ftgmac100 1e66.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
>> >  [   20.28] ftgmac100 1e66.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
>> >  [   20.30] ftgmac100 1e66.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
>> >
>> > This is because the driver was enabling low-level sensitive interrupt
>> > generation where the systems are wired for high-level. All CPU cycles
>> > are spent servicing this interrupt.
>>
>> If this is a system wiring issue, should it be represented by a DT
>> property ?
>
> Is there a device tree binding document somewhere?
>
> Is it possible just to put ACTIVE_HIGH in the right place in the
> binding?

I wrote "wired for high level" wrt the SoC internals. To be honest I
wondered the same thing but it's hard with only one (non-NSCI) system
to test on.

I had a look at the eval board schematic and it appears that the line
has pull down resistors on it, explaining why the IRQ fires when it's
configured to active low. Other machines re-use the pin pin as a GPIO.
So yes, I will change this to a dt property in v2. That will mean
dropping 4/7 "net/faraday: Avoid PHYSTS_CHG interrupt" as well.

Cheers,

Joel

[PATCH net-next 1/7] net/faraday: Separate rx page storage from rxdesc

[Joel Stanley]

From: Andrew Jeffery <and...@aj.id.au>

The ftgmac100 hardware revision in e.g. the Aspeed AST2500 no longer
reserves all bits in RXDES#2 but instead uses the bottom 16 bits to
store MAC frame metadata. Avoid corruption by shifting struct page
pointers out to their own member in struct ftgmac100.

Signed-off-by: Andrew Jeffery <and...@aj.id.au>
Signed-off-by: Joel Stanley <j...@jms.id.au>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 25 ++---
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 36361f8bf894..40622567159a 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -60,6 +60,8 @@ struct ftgmac100 {
struct ftgmac100_descs *descs;
dma_addr_t descs_dma_addr;
 
+   struct page *rx_pages[RX_QUEUE_ENTRIES];
+
unsigned int rx_pointer;
unsigned int tx_clean_pointer;
unsigned int tx_pointer;
@@ -341,18 +343,27 @@ static bool ftgmac100_rxdes_ipcs_err(struct 
ftgmac100_rxdes *rxdes)
return rxdes->rxdes1 & cpu_to_le32(FTGMAC100_RXDES1_IP_CHKSUM_ERR);
 }
 
+static inline struct page **ftgmac100_rxdes_page_slot(struct ftgmac100 *priv,
+ struct ftgmac100_rxdes 
*rxdes)
+{
+   return >rx_pages[rxdes - priv->descs->rxdes];
+}
+
 /*
  * rxdes2 is not used by hardware. We use it to keep track of page.
  * Since hardware does not touch it, we can skip cpu_to_le32()/le32_to_cpu().
  */
-static void ftgmac100_rxdes_set_page(struct ftgmac100_rxdes *rxdes, struct 
page *page)
+static void ftgmac100_rxdes_set_page(struct ftgmac100 *priv,
+struct ftgmac100_rxdes *rxdes,
+struct page *page)
 {
-   rxdes->rxdes2 = (unsigned int)page;
+   *ftgmac100_rxdes_page_slot(priv, rxdes) = page;
 }
 
-static struct page *ftgmac100_rxdes_get_page(struct ftgmac100_rxdes *rxdes)
+static struct page *ftgmac100_rxdes_get_page(struct ftgmac100 *priv,
+struct ftgmac100_rxdes *rxdes)
 {
-   return (struct page *)rxdes->rxdes2;
+   return *ftgmac100_rxdes_page_slot(priv, rxdes);
 }
 
 /**
@@ -501,7 +512,7 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int 
*processed)
 
do {
dma_addr_t map = ftgmac100_rxdes_get_dma_addr(rxdes);
-   struct page *page = ftgmac100_rxdes_get_page(rxdes);
+   struct page *page = ftgmac100_rxdes_get_page(priv, rxdes);
unsigned int size;
 
dma_unmap_page(priv->dev, map, RX_BUF_SIZE, DMA_FROM_DEVICE);
@@ -779,7 +790,7 @@ static int ftgmac100_alloc_rx_page(struct ftgmac100 *priv,
return -ENOMEM;
}
 
-   ftgmac100_rxdes_set_page(rxdes, page);
+   ftgmac100_rxdes_set_page(priv, rxdes, page);
ftgmac100_rxdes_set_dma_addr(rxdes, map);
ftgmac100_rxdes_set_dma_own(rxdes);
return 0;
@@ -791,7 +802,7 @@ static void ftgmac100_free_buffers(struct ftgmac100 *priv)
 
for (i = 0; i < RX_QUEUE_ENTRIES; i++) {
struct ftgmac100_rxdes *rxdes = >descs->rxdes[i];
-   struct page *page = ftgmac100_rxdes_get_page(rxdes);
+   struct page *page = ftgmac100_rxdes_get_page(priv, rxdes);
dma_addr_t map = ftgmac100_rxdes_get_dma_addr(rxdes);
 
if (!page)
-- 
2.9.3

[PATCH net-next 2/7] net/faraday: Make EDO{R,T}R bits configurable

[Joel Stanley]

From: Andrew Jeffery <and...@aj.id.au>

These bits are #defined at a fixed location. In order to support future
hardware that has chosen to move these bits around move the bits into a
member of the struct ftgmac100.

Signed-off-by: Andrew Jeffery <and...@aj.id.au>
Signed-off-by: Joel Stanley <j...@jms.id.au>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 40 +---
 drivers/net/ethernet/faraday/ftgmac100.h |  2 --
 2 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 40622567159a..62a88d1a1f99 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -79,6 +79,9 @@ struct ftgmac100 {
int int_mask_all;
bool use_ncsi;
bool enabled;
+
+   u32 rxdes0_edorr_mask;
+   u32 txdes0_edotr_mask;
 };
 
 static int ftgmac100_alloc_rx_page(struct ftgmac100 *priv,
@@ -259,10 +262,11 @@ static bool ftgmac100_rxdes_packet_ready(struct 
ftgmac100_rxdes *rxdes)
return rxdes->rxdes0 & cpu_to_le32(FTGMAC100_RXDES0_RXPKT_RDY);
 }
 
-static void ftgmac100_rxdes_set_dma_own(struct ftgmac100_rxdes *rxdes)
+static void ftgmac100_rxdes_set_dma_own(const struct ftgmac100 *priv,
+   struct ftgmac100_rxdes *rxdes)
 {
/* clear status bits */
-   rxdes->rxdes0 &= cpu_to_le32(FTGMAC100_RXDES0_EDORR);
+   rxdes->rxdes0 &= cpu_to_le32(priv->rxdes0_edorr_mask);
 }
 
 static bool ftgmac100_rxdes_rx_error(struct ftgmac100_rxdes *rxdes)
@@ -300,9 +304,10 @@ static bool ftgmac100_rxdes_multicast(struct 
ftgmac100_rxdes *rxdes)
return rxdes->rxdes0 & cpu_to_le32(FTGMAC100_RXDES0_MULTICAST);
 }
 
-static void ftgmac100_rxdes_set_end_of_ring(struct ftgmac100_rxdes *rxdes)
+static void ftgmac100_rxdes_set_end_of_ring(const struct ftgmac100 *priv,
+   struct ftgmac100_rxdes *rxdes)
 {
-   rxdes->rxdes0 |= cpu_to_le32(FTGMAC100_RXDES0_EDORR);
+   rxdes->rxdes0 |= cpu_to_le32(priv->rxdes0_edorr_mask);
 }
 
 static void ftgmac100_rxdes_set_dma_addr(struct ftgmac100_rxdes *rxdes,
@@ -393,7 +398,7 @@ ftgmac100_rx_locate_first_segment(struct ftgmac100 *priv)
if (ftgmac100_rxdes_first_segment(rxdes))
return rxdes;
 
-   ftgmac100_rxdes_set_dma_own(rxdes);
+   ftgmac100_rxdes_set_dma_own(priv, rxdes);
ftgmac100_rx_pointer_advance(priv);
rxdes = ftgmac100_current_rxdes(priv);
}
@@ -464,7 +469,7 @@ static void ftgmac100_rx_drop_packet(struct ftgmac100 *priv)
if (ftgmac100_rxdes_last_segment(rxdes))
done = true;
 
-   ftgmac100_rxdes_set_dma_own(rxdes);
+   ftgmac100_rxdes_set_dma_own(priv, rxdes);
ftgmac100_rx_pointer_advance(priv);
rxdes = ftgmac100_current_rxdes(priv);
} while (!done && ftgmac100_rxdes_packet_ready(rxdes));
@@ -556,10 +561,11 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, 
int *processed)
 /**
  * internal functions (transmit descriptor)
  */
-static void ftgmac100_txdes_reset(struct ftgmac100_txdes *txdes)
+static void ftgmac100_txdes_reset(const struct ftgmac100 *priv,
+ struct ftgmac100_txdes *txdes)
 {
/* clear all except end of ring bit */
-   txdes->txdes0 &= cpu_to_le32(FTGMAC100_TXDES0_EDOTR);
+   txdes->txdes0 &= cpu_to_le32(priv->txdes0_edotr_mask);
txdes->txdes1 = 0;
txdes->txdes2 = 0;
txdes->txdes3 = 0;
@@ -580,9 +586,10 @@ static void ftgmac100_txdes_set_dma_own(struct 
ftgmac100_txdes *txdes)
txdes->txdes0 |= cpu_to_le32(FTGMAC100_TXDES0_TXDMA_OWN);
 }
 
-static void ftgmac100_txdes_set_end_of_ring(struct ftgmac100_txdes *txdes)
+static void ftgmac100_txdes_set_end_of_ring(const struct ftgmac100 *priv,
+   struct ftgmac100_txdes *txdes)
 {
-   txdes->txdes0 |= cpu_to_le32(FTGMAC100_TXDES0_EDOTR);
+   txdes->txdes0 |= cpu_to_le32(priv->txdes0_edotr_mask);
 }
 
 static void ftgmac100_txdes_set_first_segment(struct ftgmac100_txdes *txdes)
@@ -701,7 +708,7 @@ static bool ftgmac100_tx_complete_packet(struct ftgmac100 
*priv)
 
dev_kfree_skb(skb);
 
-   ftgmac100_txdes_reset(txdes);
+   ftgmac100_txdes_reset(priv, txdes);
 
ftgmac100_tx_clean_pointer_advance(priv);
 
@@ -792,7 +799,7 @@ static int ftgmac100_alloc_rx_page(struct ftgmac100 *priv,
 
ftgmac100_rxdes_set_page(priv, rxdes, page);
ftgmac100_rxdes_set_dma_addr(rxdes, map);
-   ftgmac100_rxdes_set_dma_own(rxdes);

[PATCH net-next 0/7] ftgmac100 support for ast2500

[Joel Stanley]

Hello Dave,

This series adds support to the ftgmac100 driver for the Aspeed ast2400 and
ast2500 SoCs. In particular, they ensure the driver works correctly on the
ast2500 where the MAC block has seen some changes in register layout.

They have been tested on ast2400 and ast2500 systems with the NCSI stack and
with a directly attached PHY.

Cheers,

Joel

Andrew Jeffery (2):
  net/ftgmac100: Separate rx page storage from rxdesc
  net/ftgmac100: Make EDO{R,T}R bits configurable

Gavin Shan (2):
  net/faraday: Avoid PHYSTS_CHG interrupt
  net/faraday: Clear stale interrupts

Joel Stanley (3):
  net/ftgmac100: Adapt for Aspeed SoCs
  net/faraday: Fix phy link irq on Aspeed G5 SoCs
  net/faraday: Configure old MDIO interface on Aspeed SoCs

 drivers/net/ethernet/faraday/ftgmac100.c | 92 
 drivers/net/ethernet/faraday/ftgmac100.h |  8 ++-
 2 files changed, 77 insertions(+), 23 deletions(-)

-- 
2.9.3

[PATCH net-next 3/7] net/faraday: Adapt for Aspeed SoCs

[Joel Stanley]

The RXDES and TXDES registers bits in the ftgmac100 indicates EDO{R,T}R
at bit position 15 for the Faraday Tech IP. However, the version of this
IP present in the Aspeed SoCs has these bits at position 30 in the
registers.

It appers that ast2400 SoCs support both positions, with the 15th bit
marked as reserved but still functional. In the ast2500 this bit is
reused for another function, so we need a work around.

This was confirmed with engineers from Aspeed that using bit 30 is
correct for both the ast2400 and ast2500 SoCs.

Signed-off-by: Joel Stanley <j...@jms.id.au>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 62a88d1a1f99..47f512224b57 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1345,9 +1345,6 @@ static int ftgmac100_probe(struct platform_device *pdev)
priv->netdev = netdev;
priv->dev = >dev;
 
-   priv->rxdes0_edorr_mask = BIT(15);
-   priv->txdes0_edotr_mask = BIT(15);
-
spin_lock_init(>tx_lock);
 
/* initialize NAPI */
@@ -1381,6 +1378,16 @@ static int ftgmac100_probe(struct platform_device *pdev)
  FTGMAC100_INT_PHYSTS_CHG |
  FTGMAC100_INT_RPKT_BUF |
  FTGMAC100_INT_NO_RXBUF);
+
+   if (of_machine_is_compatible("aspeed,ast2400") ||
+   of_machine_is_compatible("aspeed,ast2500")) {
+   priv->rxdes0_edorr_mask = BIT(30);
+   priv->txdes0_edotr_mask = BIT(30);
+   } else {
+   priv->rxdes0_edorr_mask = BIT(15);
+   priv->txdes0_edotr_mask = BIT(15);
+   }
+
if (pdev->dev.of_node &&
of_get_property(pdev->dev.of_node, "use-ncsi", NULL)) {
if (!IS_ENABLED(CONFIG_NET_NCSI)) {
-- 
2.9.3

[PATCH net-next 5/7] net/faraday: Clear stale interrupts

[Joel Stanley]

From: Gavin Shan <gws...@linux.vnet.ibm.com>

There is stale interrupt (PHYSTS_CHG in ISR, bit#6 in 0x0) from
the bootloader (uboot) when enabling the MAC. The stale interrupts
aren't part of kernel and should be cleared.

This clears the stale interrupts in ISR (0x0) when enabling the MAC.

Signed-off-by: Gavin Shan <gws...@linux.vnet.ibm.com>
Signed-off-by: Joel Stanley <j...@jms.id.au>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index f2ea6c2f1fbd..7ba0f2d58a8b 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1113,6 +1113,7 @@ static int ftgmac100_poll(struct napi_struct *napi, int 
budget)
 static int ftgmac100_open(struct net_device *netdev)
 {
struct ftgmac100 *priv = netdev_priv(netdev);
+   unsigned int status;
int err;
 
err = ftgmac100_alloc_buffers(priv);
@@ -1138,6 +1139,11 @@ static int ftgmac100_open(struct net_device *netdev)
 
ftgmac100_init_hw(priv);
ftgmac100_start_hw(priv, priv->use_ncsi ? 100 : 10);
+
+   /* Clear stale interrupts */
+   status = ioread32(priv->base + FTGMAC100_OFFSET_ISR);
+   iowrite32(status, priv->base + FTGMAC100_OFFSET_ISR);
+
if (netdev->phydev)
phy_start(netdev->phydev);
else if (priv->use_ncsi)
-- 
2.9.3

[PATCH net-next 4/7] net/faraday: Avoid PHYSTS_CHG interrupt

[Joel Stanley]

From: Gavin Shan <gws...@linux.vnet.ibm.com>

Bit#11 in MACCR (0x50) designates the signal level for PHY link
status change. It's cleared, meaning high level enabled, by default.
However, we can see continuous interrupt (bit#6) in ISR (0x0) for it
and it's obviously a false alarm. The side effect is CPU cycles wasted
to process the false alarm.

This sets bit#11 in MACCR (0x50) to avoid the bogus interrupt.

Signed-off-by: Gavin Shan <gws...@linux.vnet.ibm.com>
Signed-off-by: Joel Stanley <j...@jms.id.au>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 1 +
 drivers/net/ethernet/faraday/ftgmac100.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 47f512224b57..f2ea6c2f1fbd 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -215,6 +215,7 @@ static void ftgmac100_init_hw(struct ftgmac100 *priv)
 FTGMAC100_MACCR_RXMAC_EN   | \
 FTGMAC100_MACCR_FULLDUP| \
 FTGMAC100_MACCR_CRC_APD| \
+FTGMAC100_MACCR_PHY_LINK_LEVEL | \
 FTGMAC100_MACCR_RX_RUNT| \
 FTGMAC100_MACCR_RX_BROADPKT)
 
diff --git a/drivers/net/ethernet/faraday/ftgmac100.h 
b/drivers/net/ethernet/faraday/ftgmac100.h
index c258586ce4a4..d07b6ea5d1b5 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.h
+++ b/drivers/net/ethernet/faraday/ftgmac100.h
@@ -152,6 +152,7 @@
 #define FTGMAC100_MACCR_FULLDUP(1 << 8)
 #define FTGMAC100_MACCR_GIGA_MODE  (1 << 9)
 #define FTGMAC100_MACCR_CRC_APD(1 << 10)
+#define FTGMAC100_MACCR_PHY_LINK_LEVEL (1 << 11)
 #define FTGMAC100_MACCR_RX_RUNT(1 << 12)
 #define FTGMAC100_MACCR_JUMBO_LF   (1 << 13)
 #define FTGMAC100_MACCR_RX_ALL (1 << 14)
-- 
2.9.3

[PATCH net-next 7/7] net/faraday: Configure old MDIO interface on Aspeed SoCs

[Joel Stanley]

The Aspeed SoCs have a new MDIO interface as an option in the G4 and G5
SoCs. The old one is still available, so select it in order to remain
compatible with the ftgmac100 driver.

Signed-off-by: Joel Stanley <j...@jms.id.au>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 9 +
 drivers/net/ethernet/faraday/ftgmac100.h | 5 +
 2 files changed, 14 insertions(+)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 5466df028381..54c6ba3632a3 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1257,12 +1257,21 @@ static int ftgmac100_setup_mdio(struct net_device 
*netdev)
struct ftgmac100 *priv = netdev_priv(netdev);
struct platform_device *pdev = to_platform_device(priv->dev);
int i, err = 0;
+   u32 reg;
 
/* initialize mdio bus */
priv->mii_bus = mdiobus_alloc();
if (!priv->mii_bus)
return -EIO;
 
+   if (of_machine_is_compatible("aspeed,ast2400") ||
+   of_machine_is_compatible("aspeed,ast2500")) {
+   /* This driver supports the old MDIO interface */
+   reg = ioread32(priv->base + FTGMAC100_OFFSET_REVR);
+   reg &= ~FTGMAC100_REVR_NEW_MDIO_INTERFACE;
+   iowrite32(reg, priv->base + FTGMAC100_OFFSET_REVR);
+   };
+
priv->mii_bus->name = "ftgmac100_mdio";
snprintf(priv->mii_bus->id, MII_BUS_ID_SIZE, "%s-%d",
 pdev->name, pdev->id);
diff --git a/drivers/net/ethernet/faraday/ftgmac100.h 
b/drivers/net/ethernet/faraday/ftgmac100.h
index d07b6ea5d1b5..a7ce0ac8858a 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.h
+++ b/drivers/net/ethernet/faraday/ftgmac100.h
@@ -134,6 +134,11 @@
 #define FTGMAC100_DMAFIFOS_TXDMA_REQ   (1 << 31)
 
 /*
+ * Feature Register
+ */
+#define FTGMAC100_REVR_NEW_MDIO_INTERFACE  BIT(31)
+
+/*
  * Receive buffer size register
  */
 #define FTGMAC100_RBSR_SIZE(x) ((x) & 0x3fff)
-- 
2.9.3

[PATCH net-next 6/7] net/faraday: Fix phy link irq on Aspeed G5 SoCs

[Joel Stanley]

On Aspeed SoC with a direct PHY connection (non-NSCI), we receive
continual PHYSTS interrupts:

 [   20.28] ftgmac100 1e66.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
 [   20.28] ftgmac100 1e66.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
 [   20.28] ftgmac100 1e66.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
 [   20.30] ftgmac100 1e66.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG

This is because the driver was enabling low-level sensitive interrupt
generation where the systems are wired for high-level. All CPU cycles
are spent servicing this interrupt.

Signed-off-by: Joel Stanley <j...@jms.id.au>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 7ba0f2d58a8b..5466df028381 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -223,6 +223,10 @@ static void ftgmac100_start_hw(struct ftgmac100 *priv, int 
speed)
 {
int maccr = MACCR_ENABLE_ALL;
 
+   if (of_machine_is_compatible("aspeed,ast2500")) {
+   maccr &= ~FTGMAC100_MACCR_PHY_LINK_LEVEL;
+   }
+
switch (speed) {
default:
case 10:
-- 
2.9.3