Re: [PATCH] MIPS: NI 169445 board support

2017-01-05 Thread Nathan Sullivan 
On Thu, Jan 05, 2017 at 06:33:53PM +, Joao Pinto wrote:
> Hi,
> 
> Às 6:28 PM de 1/5/2017, Niklas Cassel escreveu:
> > On 01/04/2017 05:38 PM, Nathan Sullivan wrote:
> >> On Tue, Dec 20, 2016 at 05:34:34PM +0100, Ralf Baechle wrote:
> >>> On Fri, Dec 02, 2016 at 09:42:09AM -0600, Nathan Sullivan wrote:
> >>>> Date:   Fri, 2 Dec 2016 09:42:09 -0600
> >>>> From: Nathan Sullivan <nathan.sulli...@ni.com>
> >>>> To: r...@linux-mips.org, mark.rutl...@arm.com, robh...@kernel.org
> >>>> CC: linux-m...@linux-mips.org, devicet...@vger.kernel.org,
> >>>>  linux-ker...@vger.kernel.org, Nathan Sullivan <nathan.sulli...@ni.com>
> >>>> Subject: [PATCH] MIPS: NI 169445 board support
> >>>> Content-Type: text/plain
> >>>>
> >>>> Support the National Instruments 169445 board.
> >>> Nathan,
> >>>
> >>> I assume you're going to repost the changes Rob asked for in
> >>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.linux-2Dmips.org_patch_14641_-2326924=DgICaQ=DPL6_X_6JkXFx7AXWqB0tg=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0=5p7f9dIkvVVK4UFHimMpezq5NwIJfUpd08c-Zk4_c6c=_JwSwe4VFYtxV1tcYt6Z8r4hJX0xfoGhCixygUxlg5s=
> >>>   and resubmit?
> >>>
> >>> Thanks,
> >>>
> >>>   Ralf
> >> Hmm, I found the issue with the generic MIPS config and dwc_eth_qos.  The 
> >> NIC
> >> driver attempts to cache align a descriptor ring using the 
> >> ___cacheline_aligned
> >> attribute on the descriptor struct, in combination with a "skip" feature in
> >> hardware.  However, the skip feature only has a three bit field, and the 
> >> generic
> >> MIPS config selects MIPS_L1_CACHE_SHIFT_7.  So, the line size is 128, and 
> >> with a
> >> 64-bit bus, that means the NIC descriptor skip field would need to be set 
> >> to
> >> 14 to align the 16-byte descriptors...
> >>
> >> I guess it makes sense for a generic MIPS kernel to align everything for 
> >> 128 byte
> >> cache lines, and for me to fix the dwc_eth_qos driver to handle cases 
> >> where the
> >> line size is too big for the hardware skip feature, right?
> > 
> > I don't know if you've been following the discussion regarding
> > dwc_eth_qos on netdev, but Joao Pinto from Synopsys is
> > planning on removing the driver (since the stmmac driver
> > now supports the same version of the IP, together with older
> > versions of the IP).
> > 
> > Since device tree bindings are treated as an ABI,
> > Joao has implemented a glue layer for stmmac that parses
> > the dwc_eth_qos binding, but uses stmmac under the hood.
> > 
> > You can use any of the bindings, but since the dwc_eth_qos
> > binding will be marked as deprecated, you might want to
> > consider moving to the stmmac binding.
> 
> A patch set to port dwc_eth_qos to stmmac is at this moment under review:
> 
> http://patchwork.ozlabs.org/patch/711428/
> http://patchwork.ozlabs.org/patch/711438/
> http://patchwork.ozlabs.org/patch/711439/
> 
> Niklas has tested it and it works well, so after the patches are upstreamed 
> the
> dwc_eth_qos will be removed as agreed with Lars.
> 
> Thanks.
>

Thanks for the heads up, I'll wait, adjust my bindings and retest then.

   Nathan

> > 
> >>
> >> Thanks,
> >>
> >>Nathan
> >>
> >>
> > 
>

Re: [PATCH] MIPS: NI 169445 board support

2017-01-04 Thread Nathan Sullivan 
On Tue, Dec 20, 2016 at 05:34:34PM +0100, Ralf Baechle wrote:
> On Fri, Dec 02, 2016 at 09:42:09AM -0600, Nathan Sullivan wrote:
> > Date:   Fri, 2 Dec 2016 09:42:09 -0600
> > From: Nathan Sullivan <nathan.sulli...@ni.com>
> > To: r...@linux-mips.org, mark.rutl...@arm.com, robh...@kernel.org
> > CC: linux-m...@linux-mips.org, devicet...@vger.kernel.org,
> >  linux-ker...@vger.kernel.org, Nathan Sullivan <nathan.sulli...@ni.com>
> > Subject: [PATCH] MIPS: NI 169445 board support
> > Content-Type: text/plain
> > 
> > Support the National Instruments 169445 board.
> 
> Nathan,
> 
> I assume you're going to repost the changes Rob asked for in
> https://patchwork.linux-mips.org/patch/14641/#26924 and resubmit?
> 
> Thanks,
> 
>   Ralf

Hmm, I found the issue with the generic MIPS config and dwc_eth_qos.  The NIC
driver attempts to cache align a descriptor ring using the ___cacheline_aligned
attribute on the descriptor struct, in combination with a "skip" feature in
hardware.  However, the skip feature only has a three bit field, and the generic
MIPS config selects MIPS_L1_CACHE_SHIFT_7.  So, the line size is 128, and with a
64-bit bus, that means the NIC descriptor skip field would need to be set to
14 to align the 16-byte descriptors...

I guess it makes sense for a generic MIPS kernel to align everything for 128 
byte
cache lines, and for me to fix the dwc_eth_qos driver to handle cases where the
line size is too big for the hardware skip feature, right?

Thanks,

   Nathan

[PATCH] igb: adjust ptp timestamps for tx/rx latency

2016-05-03 Thread Nathan Sullivan 
Table 7-62 on page 338 of the i210 datasheet lists TX and RX latencies
for the various speeds the chip supports.  To give better ptp timestamp
accuracy, adjust the timestamps by the amounts Intel gives based on
current link speed.

Signed-off-by: Nathan Sullivan <nathan.sulli...@ni.com>
---
 drivers/net/ethernet/intel/igb/igb.h |8 +++
 drivers/net/ethernet/intel/igb/igb_ptp.c |   36 ++
 2 files changed, 44 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/igb.h 
b/drivers/net/ethernet/intel/igb/igb.h
index 9413fa6..7cee61f9 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -91,6 +91,14 @@ struct igb_adapter;
 #define NVM_COMB_VER_OFF   0x0083
 #define NVM_COMB_VER_PTR   0x003d
 
+/* Transmit and receive latency (for PTP timestamps) */
+#define IGB_I210_TX_LATENCY_10 9542
+#define IGB_I210_TX_LATENCY_1001024
+#define IGB_I210_TX_LATENCY_1000   178
+#define IGB_I210_RX_LATENCY_10 20662
+#define IGB_I210_RX_LATENCY_1002213
+#define IGB_I210_RX_LATENCY_1000   448
+
 struct vf_data_storage {
unsigned char vf_mac_addresses[ETH_ALEN];
u16 vf_mc_hashes[IGB_MAX_VF_MC_ENTRIES];
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c 
b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 22a8a29..76a896d 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -722,11 +722,29 @@ static void igb_ptp_tx_hwtstamp(struct igb_adapter 
*adapter)
struct e1000_hw *hw = >hw;
struct skb_shared_hwtstamps shhwtstamps;
u64 regval;
+   int adjust = 0;
 
regval = rd32(E1000_TXSTMPL);
regval |= (u64)rd32(E1000_TXSTMPH) << 32;
 
igb_ptp_systim_to_hwtstamp(adapter, , regval);
+   /* adjust timestamp for the TX latency based on link speed */
+   if (adapter->hw.mac.type == e1000_i210) {
+   switch (adapter->link_speed) {
+   case SPEED_10:
+   adjust = IGB_I210_TX_LATENCY_10;
+   break;
+   case SPEED_100:
+   adjust = IGB_I210_TX_LATENCY_100;
+   break;
+   case SPEED_1000:
+   adjust = IGB_I210_TX_LATENCY_1000;
+   break;
+   }
+   }
+
+   shhwtstamps.hwtstamp = ktime_sub_ns(shhwtstamps.hwtstamp, adjust);
+
skb_tstamp_tx(adapter->ptp_tx_skb, );
dev_kfree_skb_any(adapter->ptp_tx_skb);
adapter->ptp_tx_skb = NULL;
@@ -771,6 +789,7 @@ void igb_ptp_rx_rgtstamp(struct igb_q_vector *q_vector,
struct igb_adapter *adapter = q_vector->adapter;
struct e1000_hw *hw = >hw;
u64 regval;
+   int adjust = 0;
 
/* If this bit is set, then the RX registers contain the time stamp. No
 * other packet will be time stamped until we read these registers, so
@@ -790,6 +809,23 @@ void igb_ptp_rx_rgtstamp(struct igb_q_vector *q_vector,
 
igb_ptp_systim_to_hwtstamp(adapter, skb_hwtstamps(skb), regval);
 
+   /* adjust timestamp for the RX latency based on link speed */
+   if (adapter->hw.mac.type == e1000_i210) {
+   switch (adapter->link_speed) {
+   case SPEED_10:
+   adjust = IGB_I210_RX_LATENCY_10;
+   break;
+   case SPEED_100:
+   adjust = IGB_I210_RX_LATENCY_100;
+   break;
+   case SPEED_1000:
+   adjust = IGB_I210_RX_LATENCY_1000;
+   break;
+   }
+   }
+   skb_hwtstamps(skb)->hwtstamp =
+   ktime_add_ns(skb_hwtstamps(skb)->hwtstamp, adjust);
+
/* Update the last_rx_timestamp timer in order to enable watchdog check
 * for error case of latched timestamp on a dropped packet.
 */
-- 
1.7.10.4

Re: [PATCH v2] net: macb: do not scan PHYs manually

2016-04-28 Thread Nathan Sullivan 
On Thu, Apr 28, 2016 at 08:43:03PM +0200, Andrew Lunn wrote:
> > I agree that is a valid fix for AT91, however it won't solve our problem, 
> > since
> > we have no children on the second ethernet MAC in our devices' device 
> > trees. I'm
> > starting to feel like our second MAC shouldn't even really register the 
> > MDIO bus
> > since it isn't being used - maybe adding a DT property to not have a bus is 
> > a
> > better option?
> 
> status = "disabled"
> 
> would be the unusual way.
> 
>   Andrew

Oh, sorry, I meant we use both MACs on Zynq, however the PHYs are on the MDIO
bus of the first MAC.  So, the second MAC is used for ethernet but not for MDIO,
and so it does not have any PHYs under its DT node.  It would be nice if there
were a way to tell macb not to bother with MDIO for the second MAC, since that's
handled by the first MAC.

I guess a good longer-term solution to all these problems would be to treat the
MAC and MDIO as seperate devices, like davinci seems to be doing.

Re: [PATCH v2] net: macb: do not scan PHYs manually

2016-04-28 Thread Nathan Sullivan 
On Thu, Apr 28, 2016 at 06:32:07PM +0200, Andrew Lunn wrote:
> > Hmm, are AT91 platforms special in this regard? As far as I can tell, this
> > driver (macb) and Marvell PXA are the only ethernet drivers that call
> > mdiobus_scan directly, and PXA does it on a known address. I do see that 
> > there
> > are trees that use macb and don't have a phy listed, which is unfortunate.
> 
> How it is supposed to work is that you do one of two things:
> 
> 1) Your device tree does not have an mdio node. In this case, you call
> mdiobus_register() and it will perform a scan of the bus, and find the
> phys.
> 
> 2) Your device tree does have an MDIO node, and you list your PHYs.
> 
> Having an MDIO node and not listing the PHYs is broken...
> 
> There are however, a few broken device trees around, and a few drivers
> have workarounds. e.g. davinci_mdio.c
> 
>/* register the mii bus
>  * Create PHYs from DT only in case if PHY child nodes are explicitly
>  * defined to support backward compatibility with DTs which assume 
> that
>  * Davinci MDIO will always scan the bus for PHYs detection.
>  */
> if (dev->of_node && of_get_child_count(dev->of_node)) {
> data->skip_scan = true;
> ret = of_mdiobus_register(data->bus, dev->of_node);
> } else {
> ret = mdiobus_register(data->bus);
> }
> 
> You probably need to do the same for AT91, count the number of
> children, and it if it zero, fall back to the non-DT way. It would
> also be good to print a warning to get people to fix their device
> tree.
> 
> Andrew

I agree that is a valid fix for AT91, however it won't solve our problem, since
we have no children on the second ethernet MAC in our devices' device trees. I'm
starting to feel like our second MAC shouldn't even really register the MDIO bus
since it isn't being used - maybe adding a DT property to not have a bus is a
better option?

Re: [PATCH v2] net: macb: do not scan PHYs manually

2016-04-28 Thread Nathan Sullivan 
On Thu, Apr 28, 2016 at 05:44:14PM +0200, Nicolas Ferre wrote:
> Le 28/04/2016 16:46, Nathan Sullivan a écrit :
> > Since of_mdiobus_register and mdiobus_register will scan automatically,
> > do not manually scan for PHY devices in the macb ethernet driver. Doing
> > so will result in many nonexistent PHYs on the MDIO bus if the MDIO
> > lines are floating or grounded, such as when they are not used.
> > 
> > Signed-off-by: Nathan Sullivan <nathan.sulli...@ni.com>
> 
> Well, as explained in the commit message that added this feature and in
> the comment, if no phy is specified in the DT we end up without phy...
> 
> There are AT91 platforms which lack specification for the phy node in
> the DT. So, I don't know if there is a better way to deal with this case
> but I see this removal as risky.
> 
> Bye,
> 
> Nicolas Ferre

Hmm, are AT91 platforms special in this regard? As far as I can tell, this
driver (macb) and Marvell PXA are the only ethernet drivers that call
mdiobus_scan directly, and PXA does it on a known address. I do see that there
are trees that use macb and don't have a phy listed, which is unfortunate.

Another way to fix our issue would be to consider all 0x0s a bad ID in
mdiobus_scan, so grounded MDIO lines do not get PHYs scanned.  Or we could add a
DT property to disable the manual scan.  I'm not sure what the correct solution
is, do you have a preference?

[PATCH v2] net: macb: do not scan PHYs manually

2016-04-28 Thread Nathan Sullivan 
Since of_mdiobus_register and mdiobus_register will scan automatically,
do not manually scan for PHY devices in the macb ethernet driver. Doing
so will result in many nonexistent PHYs on the MDIO bus if the MDIO
lines are floating or grounded, such as when they are not used.

Signed-off-by: Nathan Sullivan <nathan.sulli...@ni.com>
---
 drivers/net/ethernet/cadence/macb.c |   19 +--
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c 
b/drivers/net/ethernet/cadence/macb.c
index 48a7d7d..6506b4e 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -424,7 +424,7 @@ static int macb_mii_init(struct macb *bp)
 {
struct macb_platform_data *pdata;
struct device_node *np;
-   int err = -ENXIO, i;
+   int err = -ENXIO;
 
/* Enable management port */
macb_writel(bp, NCR, MACB_BIT(MPE));
@@ -450,23 +450,6 @@ static int macb_mii_init(struct macb *bp)
if (np) {
/* try dt phy registration */
err = of_mdiobus_register(bp->mii_bus, np);
-
-   /* fallback to standard phy registration if no phy were
-  found during dt phy registration */
-   if (!err && !phy_find_first(bp->mii_bus)) {
-   for (i = 0; i < PHY_MAX_ADDR; i++) {
-   struct phy_device *phydev;
-
-   phydev = mdiobus_scan(bp->mii_bus, i);
-   if (IS_ERR(phydev)) {
-   err = PTR_ERR(phydev);
-   break;
-   }
-   }
-
-   if (err)
-   goto err_out_unregister_bus;
-   }
} else {
if (pdata)
bp->mii_bus->phy_mask = pdata->phy_mask;
-- 
1.7.10.4

[PATCH] net: macb: do not scan PHYs manually

2016-04-28 Thread Nathan Sullivan 
Since of_mdiobus_register and mdiobus_register will scan automatically,
do not manually scan for PHY devices in the macb ethernet driver. Doing
so will result in many nonexistent PHYs on the MDIO bus if the MDIO
lines are floating or grounded, such as when they are not used.

Signed-off-by: Nathan Sullivan <nathan.sulli...@ni.com>
---
 drivers/net/ethernet/cadence/macb.c |   17 -
 1 file changed, 17 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c 
b/drivers/net/ethernet/cadence/macb.c
index 48a7d7d..e80e487 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -450,23 +450,6 @@ static int macb_mii_init(struct macb *bp)
if (np) {
/* try dt phy registration */
err = of_mdiobus_register(bp->mii_bus, np);
-
-   /* fallback to standard phy registration if no phy were
-  found during dt phy registration */
-   if (!err && !phy_find_first(bp->mii_bus)) {
-   for (i = 0; i < PHY_MAX_ADDR; i++) {
-   struct phy_device *phydev;
-
-   phydev = mdiobus_scan(bp->mii_bus, i);
-   if (IS_ERR(phydev)) {
-   err = PTR_ERR(phydev);
-   break;
-   }
-   }
-
-   if (err)
-   goto err_out_unregister_bus;
-   }
} else {
if (pdata)
bp->mii_bus->phy_mask = pdata->phy_mask;
-- 
1.7.10.4

[PATCH] net/phy: micrel: Add workaround for bad autoneg

2015-10-21 Thread Nathan Sullivan 
Very rarely, the KSZ9031 will appear to complete autonegotiation, but
will drop all traffic afterwards.  When this happens, the idle error
count will read 0xFF after autonegotiation completes.  Reset the PHY
when in that state.

Signed-off-by: Nathan Sullivan <nathan.sulli...@ni.com>
---
 drivers/net/phy/micrel.c |   23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 499185e..cf6312f 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -514,6 +514,27 @@ static int ksz8873mll_read_status(struct phy_device 
*phydev)
return 0;
 }
 
+static int ksz9031_read_status(struct phy_device *phydev)
+{
+   int err;
+   int regval;
+
+   err = genphy_read_status(phydev);
+   if (err)
+   return err;
+
+   /* Make sure the PHY is not broken. Read idle error count,
+* and reset the PHY if it is maxed out.
+*/
+   regval = phy_read(phydev, MII_STAT1000);
+   if ((regval & 0xFF) == 0xFF) {
+   phy_init_hw(phydev);
+   phydev->link = 0;
+   }
+
+   return 0;
+}
+
 static int ksz8873mll_config_aneg(struct phy_device *phydev)
 {
return 0;
@@ -772,7 +793,7 @@ static struct phy_driver ksphy_driver[] = {
.driver_data= _type,
.config_init= ksz9031_config_init,
.config_aneg= genphy_config_aneg,
-   .read_status= genphy_read_status,
+   .read_status= ksz9031_read_status,
.ack_interrupt  = kszphy_ack_interrupt,
.config_intr= kszphy_config_intr,
.suspend= genphy_suspend,
-- 
1.7.10.4

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

Re: [PATCH] net/phy: micrel: Add workaround for bad autoneg

2015-10-21 Thread Nathan Sullivan 
On Wed, Oct 21, 2015 at 12:20:21PM -0700, Florian Fainelli wrote:
> 2015-10-21 12:17 GMT-07:00 Nathan Sullivan <nathan.sulli...@ni.com>:
> > Very rarely, the KSZ9031 will appear to complete autonegotiation, but
> > will drop all traffic afterwards.  When this happens, the idle error
> > count will read 0xFF after autonegotiation completes.  Reset the PHY
> > when in that state.
> >
> > Signed-off-by: Nathan Sullivan <nathan.sulli...@ni.com>
> > ---
> >  drivers/net/phy/micrel.c |   23 ++-
> >  1 file changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> > index 499185e..cf6312f 100644
> > --- a/drivers/net/phy/micrel.c
> > +++ b/drivers/net/phy/micrel.c
> > @@ -514,6 +514,27 @@ static int ksz8873mll_read_status(struct phy_device 
> > *phydev)
> > return 0;
> >  }
> >
> > +static int ksz9031_read_status(struct phy_device *phydev)
> > +{
> > +   int err;
> > +   int regval;
> > +
> > +   err = genphy_read_status(phydev);
> > +   if (err)
> > +   return err;
> > +
> > +   /* Make sure the PHY is not broken. Read idle error count,
> > +* and reset the PHY if it is maxed out.
> > +*/
> > +   regval = phy_read(phydev, MII_STAT1000);
> > +   if ((regval & 0xFF) == 0xFF) {
> 
> Don't you also need to set phydev->state to PHY_READY here to force
> the state machine to restart?
>

I don't think so, we stay in the PHY_AN state since phy_init_hw will restart
autonegotiation.  Setting link=0 will go through PHY_NOLINK then back to
PHY_AN, so we wait for autoneg to complete again.

> > +   phy_init_hw(phydev);
> > +   phydev->link = 0;
> > +   }
> > +
> > +   return 0;
> > +}
> > +
> >  static int ksz8873mll_config_aneg(struct phy_device *phydev)
> >  {
> > return 0;
> > @@ -772,7 +793,7 @@ static struct phy_driver ksphy_driver[] = {
> > .driver_data= _type,
> > .config_init= ksz9031_config_init,
> > .config_aneg= genphy_config_aneg,
> > -   .read_status= genphy_read_status,
> > +   .read_status= ksz9031_read_status,
> > .ack_interrupt  = kszphy_ack_interrupt,
> > .config_intr= kszphy_config_intr,
> > .suspend= genphy_suspend,
> > --
> > 1.7.10.4
> >
> 
> 
> 
> -- 
> Florian
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] net/phy: micrel: Reenable interrupts during resume

2015-07-31 Thread Nathan Sullivan 
On Fri, Jul 31, 2015 at 12:22:04AM -0700, David Miller wrote:
 From: Nathan Sullivan nathan.sulli...@ni.com
 Date: Thu, 30 Jul 2015 18:09:05 -0500
 
  On Thu, Jul 30, 2015 at 10:00:34AM -0700, David Miller wrote:
  From: Nathan Sullivan nathan.sulli...@ni.com
  Date: Thu, 30 Jul 2015 10:15:48 -0500
  
   Changes for V2: Actually make sure it compiles this time.
  
  If V1 didn't compile, even for you, then I have a big problem.
  
  And that problem is that you didn't test this change at all.
  
  Sorry about that, I have tested it against 3.14, which is why I had
  the older interrupt function in v1.  On HEAD, the phy no longer
  suspends when ethernet goes down on our hardware - I'm still working
  on figuring out why.  I'm also surprised no one noticed this behavior
  before I did, but if the phy never goes into suspend you wouldn't.
 
 I think you should sort out the PHY suspending issue before we move
 forward with this patch.

I believe I found the issue, we are using this PHY with cadence macb as
the MAC.  The driver currently turns off the management port in
macb_reset_hw, which we have stopped with a local change since our hardware
typically has multiple phys on one mdio bus.  That also prevents phy suspend
from working correctly, since the bus goes down before the phy state machine
can stop the phy.

In our local patch, we have macb_reset_hw keep the mdio bus on if it's on
already.  Does that sound like an acceptable fix to you?
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices

2015-07-31 Thread Nathan Sullivan 
On Tue, Jul 28, 2015 at 03:34:51AM +, Punnaiah Choudary Kalluri wrote:
 
 Ok. I will send you updated patch for mdio support soon and we will finalize 
 next
 Course of actions if it doesn't break the existing flow. 
 
 Thanks,
 Punnaiah

When you submit this patch and mdio is seperate from the cadence macb driver,
it will likely cause problems with the ksz9031 phy due to it turning interrupts
off when suspended.  Currently, this is not a problem because taking macb down
turns the management port off.  I have a patch to correct this here:

https://patchwork.ozlabs.org/patch/502189/

Would you mind including this patch in your set?
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices

2015-07-31 Thread Nathan Sullivan 
On Tue, Jul 28, 2015 at 03:34:51AM +, Punnaiah Choudary Kalluri wrote:
 Ok. I will send you updated patch for mdio support soon and we will finalize 
 next
 Course of actions if it doesn't break the existing flow. 
 
 Thanks,
 Punnaiah

Just a heads up, when mdio no longer turns off when macb goes down, the micrel
9031 phy will have an issue with interrupts getting disabling during phy
suspend.  I have a patch to correct this issue here:

https://patchwork.ozlabs.org/patch/502189/

Would you mind including this patch in your set?
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] net/phy: micrel: Reenable interrupts during resume

2015-07-30 Thread Nathan Sullivan 
The ksz9031 has a behavior where it will clear the interrupt enable bits
when leaving power down.  To work around this, make sure the interrupt
bits are in the state they are expected to be when resuming.

Signed-off-by: Nathan Sullivan nathan.sulli...@ni.com
---

Changes for V2: Actually make sure it compiles this time.

---
 drivers/net/phy/micrel.c |   18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 499185e..9774582 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -447,6 +447,22 @@ static int ksz9031_center_flp_timing(struct phy_device 
*phydev)
return genphy_restart_aneg(phydev);
 }
 
+static int ksz9031_resume(struct phy_device *phydev)
+{
+   int result;
+
+   result = genphy_resume(phydev);
+
+   if (result)
+   return result;
+
+   /* This phy will reset interrupt enables when leaving power down */
+   if (PHY_INTERRUPT_ENABLED  phydev-interrupts)
+   result = kszphy_config_intr(phydev);
+
+   return result;
+}
+
 static int ksz9031_config_init(struct phy_device *phydev)
 {
const struct device *dev = phydev-dev;
@@ -776,7 +792,7 @@ static struct phy_driver ksphy_driver[] = {
.ack_interrupt  = kszphy_ack_interrupt,
.config_intr= kszphy_config_intr,
.suspend= genphy_suspend,
-   .resume = genphy_resume,
+   .resume = ksz9031_resume,
.driver = { .owner = THIS_MODULE, },
 }, {
.phy_id = PHY_ID_KSZ8873MLL,
-- 
1.7.10.4

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

Re: [PATCH v2] net/phy: micrel: Reenable interrupts during resume

2015-07-30 Thread Nathan Sullivan 
On Thu, Jul 30, 2015 at 10:00:34AM -0700, David Miller wrote:
 From: Nathan Sullivan nathan.sulli...@ni.com
 Date: Thu, 30 Jul 2015 10:15:48 -0500
 
  Changes for V2: Actually make sure it compiles this time.
 
 If V1 didn't compile, even for you, then I have a big problem.
 
 And that problem is that you didn't test this change at all.

Sorry about that, I have tested it against 3.14, which is why I had
the older interrupt function in v1.  On HEAD, the phy no longer
suspends when ethernet goes down on our hardware - I'm still working
on figuring out why.  I'm also surprised no one noticed this behavior
before I did, but if the phy never goes into suspend you wouldn't.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] net/phy: micrel: Reenable interrupts during resume

2015-07-29 Thread Nathan Sullivan 
The ksz9031 has a behavior where it will clear the interrupt enable bits
when leaving power down.  To work around this, make sure the interrupt
bits are in the state they are expected to be when resuming.

Signed-off-by: Nathan Sullivan nathan.sulli...@ni.com
---
 drivers/net/phy/micrel.c |   18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 499185e..7a93af6 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -447,6 +447,22 @@ static int ksz9031_center_flp_timing(struct phy_device 
*phydev)
return genphy_restart_aneg(phydev);
 }
 
+static int ksz9031_resume(struct phy_device *phydev)
+{
+   int result;
+
+   result = genphy_resume(phydev);
+
+   if (result)
+   return result;
+
+   /* This phy will reset interrupt enables when leaving power down */
+   if (PHY_INTERRUPT_ENABLED  phydev-interrupts)
+   result = kszphy_set_interrupt(phydev);
+
+   return result;
+}
+
 static int ksz9031_config_init(struct phy_device *phydev)
 {
const struct device *dev = phydev-dev;
@@ -776,7 +792,7 @@ static struct phy_driver ksphy_driver[] = {
.ack_interrupt  = kszphy_ack_interrupt,
.config_intr= kszphy_config_intr,
.suspend= genphy_suspend,
-   .resume = genphy_resume,
+   .resume = ksz9031_resume,
.driver = { .owner = THIS_MODULE, },
 }, {
.phy_id = PHY_ID_KSZ8873MLL,
-- 
1.7.10.4

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

[PATCH v2 3/3] ARM: zynq: DT: Use the zynq binding with macb

2015-05-22 Thread Nathan Sullivan 
Use the new zynq binding for macb ethernet, since it will disable half
duplex gigabit like the Zynq TRM says to do.

Signed-off-by: Nathan Sullivan nathan.sulli...@ni.com
---
 arch/arm/boot/dts/zynq-7000.dtsi |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index a5cd2ed..9ea54b3 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -193,7 +193,7 @@
};
 
gem0: ethernet@e000b000 {
-   compatible = cdns,gem;
+   compatible = cdns,zynq-gem;
reg = 0xe000b000 0x1000;
status = disabled;
interrupts = 0 22 4;
@@ -204,7 +204,7 @@
};
 
gem1: ethernet@e000c000 {
-   compatible = cdns,gem;
+   compatible = cdns,zynq-gem;
reg = 0xe000c000 0x1000;
status = disabled;
interrupts = 0 45 4;
-- 
1.7.10.4

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

[PATCH v2 1/3] net: macb: Document zynq gem dt binding

2015-05-22 Thread Nathan Sullivan 
Signed-off-by: Nathan Sullivan nathan.sulli...@ni.com
---
 Documentation/devicetree/bindings/net/cdns-emac.txt |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/cdns-emac.txt 
b/Documentation/devicetree/bindings/net/cdns-emac.txt
index abd67c1..4451ee9 100644
--- a/Documentation/devicetree/bindings/net/cdns-emac.txt
+++ b/Documentation/devicetree/bindings/net/cdns-emac.txt
@@ -3,7 +3,8 @@
 Required properties:
 - compatible: Should be cdns,[chip-]{emac}
   Use cdns,at91rm9200-emac Atmel at91rm9200 SoC.
-  or the generic form: cdns,emac.
+  Use cdns,zynq-gem Xilinx Zynq-7xxx SoC.
+  Or the generic form: cdns,emac.
 - reg: Address and length of the register set for the device
 - interrupts: Should contain macb interrupt
 - phy-mode: see ethernet.txt file in the same directory.
-- 
1.7.10.4

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

[PATCH v2 2/3] net: macb: Disable half duplex gigabit on Zynq

2015-05-22 Thread Nathan Sullivan 
According to the Zynq TRM, gigabit half duplex is not supported.  Add a
new cap and compatible string so Zynq can avoid advertising that mode.

Signed-off-by: Nathan Sullivan nathan.sulli...@ni.com
---
 drivers/net/ethernet/cadence/macb.c |   12 
 drivers/net/ethernet/cadence/macb.h |1 +
 2 files changed, 13 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.c 
b/drivers/net/ethernet/cadence/macb.c
index 61aa570..e7c0ef6 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -350,6 +350,9 @@ static int macb_mii_probe(struct net_device *dev)
else
phydev-supported = PHY_BASIC_FEATURES;
 
+   if (bp-caps  MACB_CAPS_NO_GIGABIT_HALF)
+   phydev-supported = ~SUPPORTED_1000baseT_Half;
+
phydev-advertising = phydev-supported;
 
bp-link = 0;
@@ -2693,6 +2696,14 @@ static const struct macb_config emac_config = {
.init = at91ether_init,
 };
 
+static const struct macb_config zynq_config = {
+   .caps = MACB_CAPS_SG_DISABLED | MACB_CAPS_GIGABIT_MODE_AVAILABLE |
+   MACB_CAPS_NO_GIGABIT_HALF,
+   .dma_burst_length = 16,
+   .clk_init = macb_clk_init,
+   .init = macb_init,
+};
+
 static const struct of_device_id macb_dt_ids[] = {
{ .compatible = cdns,at32ap7000-macb },
{ .compatible = cdns,at91sam9260-macb, .data = at91sam9260_config },
@@ -2703,6 +2714,7 @@ static const struct of_device_id macb_dt_ids[] = {
{ .compatible = atmel,sama5d4-gem, .data = sama5d4_config },
{ .compatible = cdns,at91rm9200-emac, .data = emac_config },
{ .compatible = cdns,emac, .data = emac_config },
+   { .compatible = cdns,zynq-gem, .data = zynq_config },
{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, macb_dt_ids);
diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index eb7d76f..24b1d9b 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -393,6 +393,7 @@
 #define MACB_CAPS_ISR_CLEAR_ON_WRITE   0x0001
 #define MACB_CAPS_USRIO_HAS_CLKEN  0x0002
 #define MACB_CAPS_USRIO_DEFAULT_IS_MII 0x0004
+#define MACB_CAPS_NO_GIGABIT_HALF  0x0008
 #define MACB_CAPS_FIFO_MODE0x1000
 #define MACB_CAPS_GIGABIT_MODE_AVAILABLE   0x2000
 #define MACB_CAPS_SG_DISABLED  0x4000
-- 
1.7.10.4

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

[PATCH] net: macb: Disable half duplex gigabit on Zynq

2015-05-21 Thread Nathan Sullivan 
According to the Zynq TRM, gigabit half duplex is not supported.  Add a
new cap and compatible string so Zynq can avoid advertising that mode.

Signed-off-by: Nathan Sullivan nathan.sulli...@ni.com
---
 drivers/net/ethernet/cadence/macb.c |   12 
 drivers/net/ethernet/cadence/macb.h |1 +
 2 files changed, 13 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.c 
b/drivers/net/ethernet/cadence/macb.c
index 61aa570..e7c0ef6 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -350,6 +350,9 @@ static int macb_mii_probe(struct net_device *dev)
else
phydev-supported = PHY_BASIC_FEATURES;
 
+   if (bp-caps  MACB_CAPS_NO_GIGABIT_HALF)
+   phydev-supported = ~SUPPORTED_1000baseT_Half;
+
phydev-advertising = phydev-supported;
 
bp-link = 0;
@@ -2693,6 +2696,14 @@ static const struct macb_config emac_config = {
.init = at91ether_init,
 };
 
+static const struct macb_config zynq_config = {
+   .caps = MACB_CAPS_SG_DISABLED | MACB_CAPS_GIGABIT_MODE_AVAILABLE |
+   MACB_CAPS_NO_GIGABIT_HALF,
+   .dma_burst_length = 16,
+   .clk_init = macb_clk_init,
+   .init = macb_init,
+};
+
 static const struct of_device_id macb_dt_ids[] = {
{ .compatible = cdns,at32ap7000-macb },
{ .compatible = cdns,at91sam9260-macb, .data = at91sam9260_config },
@@ -2703,6 +2714,7 @@ static const struct of_device_id macb_dt_ids[] = {
{ .compatible = atmel,sama5d4-gem, .data = sama5d4_config },
{ .compatible = cdns,at91rm9200-emac, .data = emac_config },
{ .compatible = cdns,emac, .data = emac_config },
+   { .compatible = cdns,zynq-gem, .data = zynq_config },
{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, macb_dt_ids);
diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index eb7d76f..24b1d9b 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -393,6 +393,7 @@
 #define MACB_CAPS_ISR_CLEAR_ON_WRITE   0x0001
 #define MACB_CAPS_USRIO_HAS_CLKEN  0x0002
 #define MACB_CAPS_USRIO_DEFAULT_IS_MII 0x0004
+#define MACB_CAPS_NO_GIGABIT_HALF  0x0008
 #define MACB_CAPS_FIFO_MODE0x1000
 #define MACB_CAPS_GIGABIT_MODE_AVAILABLE   0x2000
 #define MACB_CAPS_SG_DISABLED  0x4000
-- 
1.7.10.4

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

[PATCH] net: macb: Handle the RXUBR interrupt on all devices

2015-05-05 Thread Nathan Sullivan 
The same hardware issue the at91 must work around applies to at least the
Zynq ethernet, and possibly more devices.  The driver also needs to handle
the RXUBR interrupt since it turns it on with MACB_RX_INT_FLAGS anyway.

Signed-off-by: Nathan Sullivan nathan.sulli...@ni.com
---
 drivers/net/ethernet/cadence/macb.c |   11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cadence/macb.c 
b/drivers/net/ethernet/cadence/macb.c
index 4104d49..61aa570 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -981,7 +981,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
struct macb_queue *queue = dev_id;
struct macb *bp = queue-bp;
struct net_device *dev = bp-dev;
-   u32 status;
+   u32 status, ctrl;
 
status = queue_readl(queue, ISR);
 
@@ -1037,6 +1037,15 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 * add that if/when we get our hands on a full-blown MII PHY.
 */
 
+   if (status  MACB_BIT(RXUBR)) {
+   ctrl = macb_readl(bp, NCR);
+   macb_writel(bp, NCR, ctrl  ~MACB_BIT(RE));
+   macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
+
+   if (bp-caps  MACB_CAPS_ISR_CLEAR_ON_WRITE)
+   macb_writel(bp, ISR, MACB_BIT(RXUBR));
+   }
+
if (status  MACB_BIT(ISR_ROVR)) {
/* We missed at least one packet */
if (macb_is_gem(bp))
-- 
1.7.10.4

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