RE: [PATCH] dma: zynqmp_dma: Initialize descriptor list after freeing during reset

[Harini Katakam]

Hi Vinod,

> -Original Message-
> From: Vinod Koul [mailto:vk...@kernel.org]
> Sent: Monday, May 4, 2020 10:46 AM
> To: Rafał Hibner 
> Cc: Appana Durga Kedareswara Rao ; Radhey Shyam
> Pandey ; Harini Katakam ; Dan
> Williams ; Michal Simek ; open
> list:DMA GENERIC OFFLOAD ENGINE SUBSYSTEM
> ; moderated list:ARM/ZYNQ ARCHITECTURE
> ; open list  ker...@vger.kernel.org>
> Subject: Re: [PATCH] dma: zynqmp_dma: Initialize descriptor list after freeing
> during reset
> 
> On 02-05-20, 15:00, Rafał Hibner wrote:
> > Hello Vinod,
> >
> > On 02.05.2020 14:32, Vinod Koul wrote:
> > > Would it not be better to use list_del_init() where we delete it
> > > rather than do the init here?
> > >
> >
> > It is not a problem of list element itself not being initialized.
> > The problem is that during fault conditions (zynqmp_dma_reset) all
> > elements are moved to free list. List head however is not reinitialized.
> >
> > In normal flow elements are removed by list_del and resubmitted to
> > free list with zynqmp_dma_free_descriptor.
> >
> > static void zynqmp_dma_chan_desc_cleanup(struct zynqmp_dma_chan *chan)
> > {
> >     ...
> >     list_for_each_entry_safe(desc, next, >done_list, node) {
> >     ...
> >     list_del(>node);
> >     ...
> >     zynqmp_dma_free_descriptor(chan, desc);
> >     }
> > }
> >
> > The zynqmp_dma_free_descriptor does not delete elements from the list
> > by itself.
> > I am not he author of this driver so I fixed it by doing non intrusive
> > changes.
> >
> > Anyways, I do not see how using list_del_init would fix the bug.
> 
> Looking at this, i think it would make sense to do list_splice_init() before 
> we
> send the list to be freed.
> 
> Radhey/Appana are cced, they should test this.

I tested this patch (I took over from Appana Kedar).
Once this discussion concludes and Rafal sends a v2, I'll test again and add my 
tag.

Regards,
Harini

Re: [PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure

[Harini Katakam]

Hi Andrew,

On Tue, Aug 13, 2019 at 9:40 PM Andrew Lunn  wrote:
>
> > > The kernel does have a few helper, spi_get_drvdata, pci_get_drvdata,
> > > hci_get_drvdata. So maybe had add phydev_get_drvdata(struct phy_device
> > > *phydev)?
> >
> > Maybe phydev_mdio_get_drvdata? Because the driver data member available is
> > phydev->mdio.dev.driver_data.
>
> I still prefer phydev_get_drvdata(). It fits with the X_get_drvdata()
> pattern, where X is the type of parameter passed to the call, spi,
> pci, hci.
>
> We can also add mdiodev_get_drvdata(mdiodev). A few DSA drivers could
> use that.

Sorry for the late reply. I just sent a v2 adding
mdiodev_get/set_drvdata helpers
and using them in gmii2rgmii driver.
I did not add a corresponding phydev helper because there is no "struct dev" in
"struct phy_device" and I dint know if there were any users to add the member
and then a helper for driver data. Also,
strutct phy_device { struct mdio_device { struct device }}
is already available and it seemed logical to use that field to
set/get driver data
for gmii2rgmii. Please let me know if v2 is okay.

Regards,
Harini

[PATCH v2 2/2] net: phy: gmii2rgmii: Dont use priv field in phy device

[Harini Katakam]

Use set/get drv data in phydev's mdio device instead. Phy device priv
field maybe used by the external phy driver and should not be
overwritten.

Signed-off-by: Harini Katakam 
---
v2:
Use mdio driver data helper.

 drivers/net/phy/xilinx_gmii2rgmii.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/xilinx_gmii2rgmii.c 
b/drivers/net/phy/xilinx_gmii2rgmii.c
index 2d14493..151c2a3 100644
--- a/drivers/net/phy/xilinx_gmii2rgmii.c
+++ b/drivers/net/phy/xilinx_gmii2rgmii.c
@@ -29,7 +29,7 @@ struct gmii2rgmii {
 
 static int xgmiitorgmii_read_status(struct phy_device *phydev)
 {
-   struct gmii2rgmii *priv = phydev->priv;
+   struct gmii2rgmii *priv = mdiodev_get_drvdata(>mdio);
struct mii_bus *bus = priv->mdio->bus;
int addr = priv->mdio->addr;
u16 val = 0;
@@ -90,7 +90,7 @@ static int xgmiitorgmii_probe(struct mdio_device *mdiodev)
memcpy(>conv_phy_drv, priv->phy_dev->drv,
   sizeof(struct phy_driver));
priv->conv_phy_drv.read_status = xgmiitorgmii_read_status;
-   priv->phy_dev->priv = priv;
+   mdiodev_set_drvdata(>phy_dev->mdio, priv);
priv->phy_dev->drv = >conv_phy_drv;
 
return 0;
-- 
2.7.4

[PATCH v2 1/2] include: mdio: Add driver data helpers

[Harini Katakam]

Add set/get drv_data helpers for mdio device.

Signed-off-by: Harini Katakam 
---
v2:
Added this patch driver data helpers in mdio instead of priv field.

 include/linux/mdio.h | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index e8242ad8..a760424 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -68,6 +68,17 @@ struct mdio_driver {
 #define to_mdio_driver(d)  \
container_of(to_mdio_common_driver(d), struct mdio_driver, mdiodrv)
 
+/* device driver data */
+static inline void mdiodev_set_drvdata(struct mdio_device *mdio, void *data)
+{
+   dev_set_drvdata(>dev, data);
+}
+
+static inline void *mdiodev_get_drvdata(struct mdio_device *mdio)
+{
+   return dev_get_drvdata(>dev);
+}
+
 void mdio_device_free(struct mdio_device *mdiodev);
 struct mdio_device *mdio_device_create(struct mii_bus *bus, int addr);
 int mdio_device_register(struct mdio_device *mdiodev);
-- 
2.7.4

[PATCH v2 0/2] Fix GMII2RGMII private field

[Harini Katakam]

Fix the usage of external phy's priv field by gmii2rgmii driver.

Based on net-next.

Harini Katakam (2):
  include: mdio: Add driver data helpers
  net: phy: gmii2rgmii: Dont use priv field in phy device

 drivers/net/phy/xilinx_gmii2rgmii.c |  4 ++--
 include/linux/mdio.h| 11 +++
 2 files changed, 13 insertions(+), 2 deletions(-)

-- 
2.7.4

Re: [PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure

[Harini Katakam]

Hi Andrew,

On Tue, Aug 13, 2019 at 6:54 PM Andrew Lunn  wrote:
>
> On Tue, Aug 13, 2019 at 04:46:40PM +0530, Harini Katakam wrote:
> > Hi Andrew,
> >
> > On Thu, Aug 1, 2019 at 9:36 AM Andrew Lunn  wrote:
> > >
> > > On Wed, Jul 31, 2019 at 03:06:19PM +0530, Harini Katakam wrote:
> > > > Use the priv field in mdio device structure instead of the one in
> > > > phy device structure. The phy device priv field may be used by the
> > > > external phy driver and should not be overwritten.
> > >
> > > Hi Harini
> > >
> > > I _think_ you could use dev_set_drvdata(>dev) in 
> > > xgmiitorgmii_probe() and
> > > dev_get_drvdata(>mdiomdio.dev) in _read_status()
> >
> > Thanks for the review. This works if I do:
> > dev_set_drvdata(>phy_dev->mdio.dev->dev) in probe
> > and then
> > dev_get_drvdata(>mdio.dev) in _read_status()
> >
> > i.e mdiodev in gmii2rgmii probe and priv->phy_dev->mdio are not the same.
> >
> > If this is acceptable, I can send a v2.
>
> Hi Harini
>
> I think this is better, making use of the central driver
> infrastructure, rather than inventing something new.

Ok sure.

>
> The kernel does have a few helper, spi_get_drvdata, pci_get_drvdata,
> hci_get_drvdata. So maybe had add phydev_get_drvdata(struct phy_device
> *phydev)?

Maybe phydev_mdio_get_drvdata? Because the driver data member available is
phydev->mdio.dev.driver_data.

Regards,
Harini

Re: [PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure

[Harini Katakam]

Hi Andrew,

On Thu, Aug 1, 2019 at 9:36 AM Andrew Lunn  wrote:
>
> On Wed, Jul 31, 2019 at 03:06:19PM +0530, Harini Katakam wrote:
> > Use the priv field in mdio device structure instead of the one in
> > phy device structure. The phy device priv field may be used by the
> > external phy driver and should not be overwritten.
>
> Hi Harini
>
> I _think_ you could use dev_set_drvdata(>dev) in 
> xgmiitorgmii_probe() and
> dev_get_drvdata(>mdiomdio.dev) in _read_status()

Thanks for the review. This works if I do:
dev_set_drvdata(>phy_dev->mdio.dev->dev) in probe
and then
dev_get_drvdata(>mdio.dev) in _read_status()

i.e mdiodev in gmii2rgmii probe and priv->phy_dev->mdio are not the same.

If this is acceptable, I can send a v2.

Regards,
Harini

Re: [RFC PATCH 1/2] dt-bindings: net: macb: Add new property for PS SGMII only

[Harini Katakam]

Hi Andrew,

On Mon, Aug 5, 2019 at 10:47 PM Andrew Lunn  wrote:
>
> > Even with the use of this interrupt, the link status actions (link print and
> > netif ops) will still be required. And also the need for macb_open to
> > proceed without phydev. Could you please let me know if that is acceptable
> > to patch or if there's a cleaner way to
> > report this link status?
>
> It sounds like you need to convert to phylink, so you get full sfp
> support. phylib does not handle hotplug of PHYs.
>
> Please look at the comments Russell gave the last time this was
> attempted.

Yes, I looked at the comments from Russell and wasn't sure if this
case qualified for phylink.

Regards,
Harini

Re: [RFC PATCH 1/2] dt-bindings: net: macb: Add new property for PS SGMII only

[Harini Katakam]

Hi Andrew,

On Mon, Aug 5, 2019 at 7:00 PM Andrew Lunn  wrote:
>
> On Mon, Aug 05, 2019 at 11:45:05AM +0530, Harini Katakam wrote:
> > Hi Andrew,
> >
> > On Sun, Aug 4, 2019 at 8:26 PM Andrew Lunn  wrote:
> > >
> > > On Wed, Jul 31, 2019 at 03:10:32PM +0530, Harini Katakam wrote:
> > > > Add a new property to indicate when PS SGMII is used with NO
> > > > external PHY on board.
> > >
> > > Hi Harini
> > >
> > > What exactly is you use case? Are you connecting to a Ethernet switch?
> > > To an SFP cage with a copper module?
> >
> > Yes, an SFP cage is the common HW target for this patch.
>
> Hi Harini
>
> So you have a copper PHY in the SFP cage. It will talk SGMII
> signalling to your PS SGMII. When that signalling is complete i would
> expect the MAC to raise an interrupt, just as if the SGMII PHY was
> soldered on the board. So i don't see why you need this polling?
>

Thanks. Sorry, I overlooked this interrupt. Let me try that.

Regards,
Harini

Re: [RFC PATCH 1/2] dt-bindings: net: macb: Add new property for PS SGMII only

[Harini Katakam]

Hi Andrew,

On Sun, Aug 4, 2019 at 8:26 PM Andrew Lunn  wrote:
>
> On Wed, Jul 31, 2019 at 03:10:32PM +0530, Harini Katakam wrote:
> > Add a new property to indicate when PS SGMII is used with NO
> > external PHY on board.
>
> Hi Harini
>
> What exactly is you use case? Are you connecting to a Ethernet switch?
> To an SFP cage with a copper module?

Yes, an SFP cage is the common HW target for this patch. Essentially, there
is no external PHY driver that the macb can "connect" to; if there was an
external PHY on board, its link status would also indicate when the GEM(PCS)
to external PHY link was down. But in the absence of an external PHY on
HW, PCS link status needs to be monitored and reported to users.

Regards,
Harini

[RFC PATCH 1/2] dt-bindings: net: macb: Add new property for PS SGMII only

[Harini Katakam]

Add a new property to indicate when PS SGMII is used with NO
external PHY on board.

Signed-off-by: Harini Katakam 
---
 Documentation/devicetree/bindings/net/macb.txt | 4 
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/macb.txt 
b/Documentation/devicetree/bindings/net/macb.txt
index 63c73fa..ae1b109 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -38,6 +38,10 @@ Optional properties for PHY child node:
   up via magic packet.
 - phy-handle : see ethernet.txt file in the same directory
 
+Optional properties:
+- is-internal-pcspma: Add when GEM's internal SGMII support is used without
+  any external SGMII PHY.
+
 Examples:
 
macb0: ethernet@fffc4000 {
-- 
2.7.4

[RFC PATCH 2/2] net: macb: Add SGMII poll thread

[Harini Katakam]

The internal SGMII mode in PS GEM on ZynqMP can be used without any
external PHY on board. In this case, the phy framework doesn't kick
in to monitor the link status. Hence do the same in macb driver.

Signed-off-by: Harini Katakam 
Signed-off-by: Kester Aernoudt 
---
 drivers/net/ethernet/cadence/macb.h  |  8 
 drivers/net/ethernet/cadence/macb_main.c | 65 ++--
 2 files changed, 69 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index 03983bd..b07284a 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -155,6 +155,7 @@
 #define GEM_PEFTN  0x01f4 /* PTP Peer Event Frame Tx Ns */
 #define GEM_PEFRSL 0x01f8 /* PTP Peer Event Frame Rx Sec Low */
 #define GEM_PEFRN  0x01fc /* PTP Peer Event Frame Rx Ns */
+#define GEM_PCSSTATUS  0x0204 /* PCS Status */
 #define GEM_DCFG1  0x0280 /* Design Config 1 */
 #define GEM_DCFG2  0x0284 /* Design Config 2 */
 #define GEM_DCFG3  0x0288 /* Design Config 3 */
@@ -455,6 +456,10 @@
 #define MACB_REV_OFFSET0
 #define MACB_REV_SIZE  16
 
+/* Bitfields in PCSSTATUS */
+#define GEM_PCSLINK_OFFSET 2
+#define GEM_PCSLINK_SIZE   1
+
 /* Bitfields in DCFG1. */
 #define GEM_IRQCOR_OFFSET  23
 #define GEM_IRQCOR_SIZE1
@@ -1232,6 +1237,9 @@ struct macb {
u32 rx_intr_mask;
 
struct macb_pm_data pm_data;
+
+   int internal_pcspma;
+   struct task_struct *sgmii_poll_task;
 };
 
 #ifdef CONFIG_MACB_USE_HWSTAMP
diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index 5ca17e6..ae1f18d 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -36,6 +36,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "macb.h"
 
 /* This structure is only used for MACB on SiFive FU540 devices */
@@ -2418,8 +2419,10 @@ static int macb_open(struct net_device *dev)
/* carrier starts down */
netif_carrier_off(dev);
 
-   /* if the phy is not yet register, retry later*/
-   if (!dev->phydev) {
+   /* if the phy is not yet registered and internal SGMII is not used,
+* retry later
+*/
+   if (!bp->internal_pcspma && !dev->phydev) {
err = -EAGAIN;
goto pm_exit;
}
@@ -2441,7 +2444,8 @@ static int macb_open(struct net_device *dev)
macb_init_hw(bp);
 
/* schedule a link state check */
-   phy_start(dev->phydev);
+   if (!bp->internal_pcspma)
+   phy_start(dev->phydev);
 
netif_tx_start_all_queues(dev);
 
@@ -2468,7 +2472,7 @@ static int macb_close(struct net_device *dev)
for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue)
napi_disable(>napi);
 
-   if (dev->phydev)
+   if (!bp->internal_pcspma && dev->phydev)
phy_stop(dev->phydev);
 
spin_lock_irqsave(>lock, flags);
@@ -3187,6 +3191,49 @@ static const struct ethtool_ops gem_ethtool_ops = {
.set_rxnfc  = gem_set_rxnfc,
 };
 
+int gem_sgmii_status_poll(void *data)
+{
+   struct net_device *dev = data;
+   struct macb *bp = netdev_priv(dev);
+   int status, prev_status = 0;
+   u32 reg;
+
+   while (!kthread_should_stop()) {
+   status = gem_readl(bp, PCSSTATUS) & GEM_BIT(PCSLINK);
+   reg = macb_readl(bp, NCR);
+   if (!(reg & MACB_BIT(RE)) || !(reg & MACB_BIT(TE)) ||
+   (!netif_carrier_ok(dev) && prev_status))
+   status = 0;
+
+   if (status != prev_status) {
+   if (status) {
+   reg = macb_readl(bp, NCFGR);
+   reg |= MACB_BIT(FD);
+   reg |= GEM_BIT(GBE);
+
+   macb_or_gem_writel(bp, NCFGR, reg);
+
+   bp->speed = SPEED_1000;
+   bp->duplex = DUPLEX_FULL;
+   bp->link = 1;
+   macb_set_tx_clk(bp->tx_clk, SPEED_1000, dev);
+
+   netif_carrier_on(dev);
+   netdev_info(dev, "link up (%d/%s)\n",
+   SPEED_1000, "Full");
+   } else {
+   netif_carrier_off(dev);
+   netdev_info(dev, "link down\n");
+   }
+
+   prev_status = status;
+   }
+   sch

[RFC PATCH 0/2] Macb SGMII status poll thread

[Harini Katakam]

When PS GEM is used with SGMII mode without an external PHY
on board, add a link status reporting mechanism.

Harini Katakam (2):
  dt-bindings: net: macb: Add new property for PS SGMII only
  net: macb: Add SGMII poll thread

 Documentation/devicetree/bindings/net/macb.txt |  4 ++
 drivers/net/ethernet/cadence/macb.h|  8 
 drivers/net/ethernet/cadence/macb_main.c   | 65 --
 3 files changed, 73 insertions(+), 4 deletions(-)

-- 
2.7.4

[PATCH 0/2] Fix GMII2RGMII private field

[Harini Katakam]

Fix the usage of external phy's priv field by gmii2rgmii driver.

Based on net-next.

Harini Katakam (2):
  include: mdio: Add private field to mdio structure
  net: gmii2rgmii: Switch priv field in mdio device structure

 drivers/net/phy/xilinx_gmii2rgmii.c | 4 ++--
 include/linux/mdio.h| 3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

-- 
2.7.4

[PATCH 1/2] include: mdio: Add private field to mdio structure

[Harini Katakam]

Add a priv pointer to mdio structure to be used by mdio devices if
required.

This priv field will be used by gmii2rgmii driver. As this IP has
no capability to read status on the MDIO bus, the driver currently
snoops the same and needs the instance information is some private
field. Since phy device "priv" can be used by external phy drivers,
it is not appropriate. Hence this addition to mdio device. This is
a temporary solution before the IP can be improved. The need for
this priv field can be re-evaluated later based on other mdio devices.

Signed-off-by: Harini Katakam 
Reviewed-by: Radhey Shyam Pandey 
---
 include/linux/mdio.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index e8242ad8..3399de7 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -40,6 +40,9 @@ struct mdio_device {
struct reset_control *reset_ctrl;
unsigned int reset_assert_delay;
unsigned int reset_deassert_delay;
+
+   /* private data pointer for use by MDIO devices */
+   void *priv;
 };
 #define to_mdio_device(d) container_of(d, struct mdio_device, dev)
 
-- 
2.7.4

[PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure

[Harini Katakam]

Use the priv field in mdio device structure instead of the one in
phy device structure. The phy device priv field may be used by the
external phy driver and should not be overwritten.

Signed-off-by: Harini Katakam 
Reviewed-by: Radhey Shyam Pandey 
---
 drivers/net/phy/xilinx_gmii2rgmii.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/xilinx_gmii2rgmii.c 
b/drivers/net/phy/xilinx_gmii2rgmii.c
index 2d14493..ba31b5c3 100644
--- a/drivers/net/phy/xilinx_gmii2rgmii.c
+++ b/drivers/net/phy/xilinx_gmii2rgmii.c
@@ -29,7 +29,7 @@ struct gmii2rgmii {
 
 static int xgmiitorgmii_read_status(struct phy_device *phydev)
 {
-   struct gmii2rgmii *priv = phydev->priv;
+   struct gmii2rgmii *priv = phydev->mdio.priv;
struct mii_bus *bus = priv->mdio->bus;
int addr = priv->mdio->addr;
u16 val = 0;
@@ -90,7 +90,7 @@ static int xgmiitorgmii_probe(struct mdio_device *mdiodev)
memcpy(>conv_phy_drv, priv->phy_dev->drv,
   sizeof(struct phy_driver));
priv->conv_phy_drv.read_status = xgmiitorgmii_read_status;
-   priv->phy_dev->priv = priv;
+   priv->phy_dev->mdio.priv = priv;
priv->phy_dev->drv = >conv_phy_drv;
 
return 0;
-- 
2.7.4

[PATCH 2/2] net: macb: Fix SUBNS increment and increase resolution

[Harini Katakam]

The subns increment register has 24 bits as follows:
RegBit[15:0] = Subns[23:8]; RegBit[31:24] = Subns[7:0]

Fix the same in the driver and increase sub ns resolution to the
best capable, 24 bits. This should be the case on all GEM versions
that this PTP driver supports.

Signed-off-by: Harini Katakam 
---
 drivers/net/ethernet/cadence/macb.h | 6 +-
 drivers/net/ethernet/cadence/macb_ptp.c | 5 -
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index 90bc70b..03983bd 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -496,7 +496,11 @@
 
 /* Bitfields in TISUBN */
 #define GEM_SUBNSINCR_OFFSET   0
-#define GEM_SUBNSINCR_SIZE 16
+#define GEM_SUBNSINCRL_OFFSET  24
+#define GEM_SUBNSINCRL_SIZE8
+#define GEM_SUBNSINCRH_OFFSET  0
+#define GEM_SUBNSINCRH_SIZE16
+#define GEM_SUBNSINCR_SIZE 24
 
 /* Bitfields in TI */
 #define GEM_NSINCR_OFFSET  0
diff --git a/drivers/net/ethernet/cadence/macb_ptp.c 
b/drivers/net/ethernet/cadence/macb_ptp.c
index 6276eac..43a3f0d 100644
--- a/drivers/net/ethernet/cadence/macb_ptp.c
+++ b/drivers/net/ethernet/cadence/macb_ptp.c
@@ -104,7 +104,10 @@ static int gem_tsu_incr_set(struct macb *bp, struct 
tsu_incr *incr_spec)
 * to take effect.
 */
spin_lock_irqsave(>tsu_clk_lock, flags);
-   gem_writel(bp, TISUBN, GEM_BF(SUBNSINCR, incr_spec->sub_ns));
+   /* RegBit[15:0] = Subns[23:8]; RegBit[31:24] = Subns[7:0] */
+   gem_writel(bp, TISUBN, GEM_BF(SUBNSINCRL, incr_spec->sub_ns) |
+  GEM_BF(SUBNSINCRH, (incr_spec->sub_ns >>
+ GEM_SUBNSINCRL_SIZE)));
gem_writel(bp, TI, GEM_BF(NSINCR, incr_spec->ns));
spin_unlock_irqrestore(>tsu_clk_lock, flags);
 
-- 
2.7.4

[PATCH 1/2] net: macb: Add separate definition for PPM fraction

[Harini Katakam]

The scaled ppm parameter passed to _adjfine() contains a 16 bit
fraction. This just happens to be the same as SUBNSINCR_SIZE now.
Hence define this separately.

Signed-off-by: Harini Katakam 
---
 drivers/net/ethernet/cadence/macb.h | 3 +++
 drivers/net/ethernet/cadence/macb_ptp.c | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index 515bfd2..90bc70b 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -834,6 +834,9 @@ struct gem_tx_ts {
 /* limit RX checksum offload to TCP and UDP packets */
 #define GEM_RX_CSUM_CHECKED_MASK   2
 
+/* Scaled PPM fraction */
+#define PPM_FRACTION   16
+
 /* struct macb_tx_skb - data about an skb which is being transmitted
  * @skb: skb currently being transmitted, only set for the last buffer
  *   of the frame
diff --git a/drivers/net/ethernet/cadence/macb_ptp.c 
b/drivers/net/ethernet/cadence/macb_ptp.c
index 0a8aca8..6276eac 100644
--- a/drivers/net/ethernet/cadence/macb_ptp.c
+++ b/drivers/net/ethernet/cadence/macb_ptp.c
@@ -135,7 +135,7 @@ static int gem_ptp_adjfine(struct ptp_clock_info *ptp, long 
scaled_ppm)
 * (temp / USEC_PER_SEC) + 0.5
 */
adj += (USEC_PER_SEC >> 1);
-   adj >>= GEM_SUBNSINCR_SIZE; /* remove fractions */
+   adj >>= PPM_FRACTION; /* remove fractions */
adj = div_u64(adj, USEC_PER_SEC);
adj = neg_adj ? (word - adj) : (word + adj);
 
-- 
2.7.4

[PATCH 0/2] Sub ns increment fixes in Macb PTP

[Harini Katakam]

The subns increment register fields are not captured correctly in the
driver. Fix the same and also increase the subns incr resolution.

Sub ns resolution was increased to 24 bits in r1p06f2 version. To my
knowledge, this PTP driver, with its current BD time stamp
implementation, is only useful to that version or above. So, I have
increased the resolution unconditionally. Please let me know if there
is any IP versions incompatible with this - there is no register to
obtain this information from.

Changes from RFC:
None

Harini Katakam (2):
  net: macb: Add separate definition for PPM fraction
  net: macb: Fix SUBNS increment and increase resolution

 drivers/net/ethernet/cadence/macb.h | 9 -
 drivers/net/ethernet/cadence/macb_ptp.c | 7 +--
 2 files changed, 13 insertions(+), 3 deletions(-)

-- 
2.7.4

Re: [PATCH 1/2] net: macb: Fix compilation on systems without COMMON_CLK

[Harini Katakam]

On Tue, Jun 25, 2019 at 4:17 AM  wrote:
>
> On 24/06/2019 at 11:57, Palmer Dabbelt wrote:
> > External E-Mail
> >
> >
> > On Mon, 24 Jun 2019 02:40:21 PDT (-0700), nicolas.fe...@microchip.com wrote:
> >> On 24/06/2019 at 08:16, Palmer Dabbelt wrote:
> >>> External E-Mail
> >>>
> >>>
> >>> The patch to add support for the FU540-C000 added a dependency on
> >>> COMMON_CLK, but didn't express that via Kconfig.  This fixes the build
> >>> failure by adding CONFIG_MACB_FU540, which depends on COMMON_CLK and
> >>> conditionally enables the FU540-C000 support.
> >>
> >> Let's try to limit the use of  #ifdef's throughout the code. We are
> >> using them in this driver but only for the hot paths and things that
> >> have an impact on performance. I don't think it's the case here: so
> >> please find another option => NACK.
> >
> > OK.  Would you accept adding a Kconfig dependency of the generic MACB 
> > driver on
> > COMMON_CLK, as suggested in the cover letter?
>
> Yes: all users of this peripheral have COMMON_CLK set.
> You can remove it from the PCI wrapper then.
>

Yes, +1, both Zynq and ZynqMP have COMMON_CLK set, thanks.

Regards,
Harini

Re: [PATCH RESEND net] net: phy: xgmiitorgmii: Support generic PHY status read

[Harini Katakam]

Hi Andrew,
On Thu, Feb 28, 2019 at 1:03 PM Harini Katakam  wrote:
>
> Hi,
> On Wed, Feb 27, 2019 at 2:35 PM Harini Katakam  wrote:
> >
> > Hi Andrew, Paul,
> >
> > On Wed, Feb 27, 2019 at 2:15 PM Michal Simek  
> > wrote:
> > >
> > > On 21. 02. 19 12:03, Michal Simek wrote:
> > > > On 21. 02. 19 11:24, Paul Kocialkowski wrote:
> > > >> Hi,
> > > >>
> > > >> On Wed, 2019-02-20 at 07:58 +0100, Michal Simek wrote:
> > > >>> Hi,
> > > >>>
> > > >>> On 19. 02. 19 18:25, Andrew Lunn wrote:
> 
> > > >> Understood. I think we need to start a discussion about how the general
> > > >> design of this driver can be improved.
> > > >>
> > > >> In particular, I wonder if it could work better to make this driver a
> > > >> PHY driver that just redirects all its ops to the actual PHY driver,
> > > >> except for read_status where it should also add some code.
> >
> > Thanks, I'm looking into this option and also a way to expose the correct
> > interface mode setting as you mentioned below. I'll get back before the
> > end of the week. Please do let me know if you have any further suggestions.
> >
> This IP does not have a PHY identifier or status register that can be accessed
> from the phy framework. We could discuss with our design team to add these
> in the future. But that would take sometime and this version should be still 
> be
> supported. Also, if this IP has a PHY driver, then two phy drivers would have
> to be probed which are connected in a serial manner and I believe I'll have to
> update the framework to support that. Could you please let me know if you have
> any inputs on this?
> OR since this is just a bridge IP, is it acceptable to address the error 
> cases?
> -> Module loading/unloading
> -> Spinlocks for protection
> -> Correct phy mode information to the driver.
> -> Any other concerns
> I could do a respin of this patch after addressing Andrew's comments:
> https://patchwork.kernel.org/patch/9290231/

Related to this, I have a query on how the DT node for gmii2rgmii should look.
One of the users of gmii2rgmii is Cadence macb driver. In Xilinx tree, we use
this piece of code to register this mdiobus:
+ mdio_np = of_get_child_by_name(np, "mdio");
+ if (mdio_np) {
+ of_node_put(mdio_np);
+ err = of_mdiobus_register(bp->mii_bus, mdio_np);
+ if (err)
+ goto err_out_unregister_bus;

And the DT node looks like this:
ethernet {
phy-mode = "gmii";
phy-handle = <>;

mdio {
extphy {
reg = ;
};
gmii_to_rgmii{
compatible = "xlnx,gmii-to-rgmii-1.0";
phy-handle = <>;
reg = ;
};
};
};

Could you please clarify if phy-handle in ethernet should point to
external PHY or gmii2rgmii?

Regards,
Harini

[PATCH] arm64: zynqmp: dt: Add TI PHY quirk

[Harini Katakam]

Add TI PHY strap ctrl quirk for all the HW where applicable.

Signed-off-by: Harini Katakam 
---
 arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm016-dc2.dts | 1 +
 arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts  | 1 +
 arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revB.dts  | 1 +
 arch/arm64/boot/dts/xilinx/zynqmp-zcu104-revA.dts  | 1 +
 arch/arm64/boot/dts/xilinx/zynqmp-zcu106-revA.dts  | 1 +
 arch/arm64/boot/dts/xilinx/zynqmp-zcu111-revA.dts  | 1 +
 6 files changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm016-dc2.dts 
b/arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm016-dc2.dts
index 11cc671..2421ec7 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm016-dc2.dts
+++ b/arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm016-dc2.dts
@@ -89,6 +89,7 @@
ti,rx-internal-delay = <0x8>;
ti,tx-internal-delay = <0xa>;
ti,fifo-depth = <0x1>;
+   ti,dp83867-rxctrl-strap-quirk;
};
 };
 
diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts 
b/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts
index cef8167..2a3b665 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts
+++ b/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts
@@ -110,6 +110,7 @@
ti,rx-internal-delay = <0x8>;
ti,tx-internal-delay = <0xa>;
ti,fifo-depth = <0x1>;
+   ti,dp83867-rxctrl-strap-quirk;
};
 };
 
diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revB.dts 
b/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revB.dts
index af4d868..1780ed2 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revB.dts
+++ b/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revB.dts
@@ -21,6 +21,7 @@
ti,rx-internal-delay = <0x8>;
ti,tx-internal-delay = <0xa>;
ti,fifo-depth = <0x1>;
+   ti,dp83867-rxctrl-strap-quirk;
};
/* Cleanup from RevA */
/delete-node/ phy@21;
diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zcu104-revA.dts 
b/arch/arm64/boot/dts/xilinx/zynqmp-zcu104-revA.dts
index d4ad19a..8f45614 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp-zcu104-revA.dts
+++ b/arch/arm64/boot/dts/xilinx/zynqmp-zcu104-revA.dts
@@ -55,6 +55,7 @@
ti,rx-internal-delay = <0x8>;
ti,tx-internal-delay = <0xa>;
ti,fifo-depth = <0x1>;
+   ti,dp83867-rxctrl-strap-quirk;
};
 };
 
diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zcu106-revA.dts 
b/arch/arm64/boot/dts/xilinx/zynqmp-zcu106-revA.dts
index 94cf509..93ce7eb 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp-zcu106-revA.dts
+++ b/arch/arm64/boot/dts/xilinx/zynqmp-zcu106-revA.dts
@@ -111,6 +111,7 @@
ti,rx-internal-delay = <0x8>;
ti,tx-internal-delay = <0xa>;
ti,fifo-depth = <0x1>;
+   ti,dp83867-rxctrl-strap-quirk;
};
 };
 
diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zcu111-revA.dts 
b/arch/arm64/boot/dts/xilinx/zynqmp-zcu111-revA.dts
index 460adc3..8bb0001 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp-zcu111-revA.dts
+++ b/arch/arm64/boot/dts/xilinx/zynqmp-zcu111-revA.dts
@@ -106,6 +106,7 @@
ti,rx-internal-delay = <0x8>;
ti,tx-internal-delay = <0xa>;
ti,fifo-depth = <0x1>;
+   ti,dp83867-rxctrl-strap-quirk;
};
 };
 
-- 
2.7.4

[PATCH v3 3/4] net: macb: Add pm runtime support

[Harini Katakam]

Add runtime pm functions and move clock handling there.
Add runtime PM calls to mdio functions to allow for active mdio bus.

Signed-off-by: Shubhrajyoti Datta 
Signed-off-by: Harini Katakam 
---
v3 changes:
Fix exit path using goto

v2 changes:
Allow for mdio bus to be active

Changes from RFC:
Updated pm get sync/put sync calls.
Removed unecessary clk up in mdio helpers.

 drivers/net/ethernet/cadence/macb_main.c | 148 +++
 1 file changed, 112 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index be10ee4..5173c02 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -37,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "macb.h"
 
 #define MACB_RX_BUFFER_SIZE128
@@ -80,6 +81,8 @@
  */
 #define MACB_HALT_TIMEOUT  1230
 
+#define MACB_PM_TIMEOUT  100 /* ms */
+
 #define MACB_MDIO_TIMEOUT  100 /* in usecs */
 
 /* DMA buffer descriptor might be different size
@@ -332,12 +335,15 @@ static int macb_mdio_wait_for_idle(struct macb *bp)
 static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
 {
struct macb *bp = bus->priv;
-   int value;
-   int err;
+   int status;
 
-   err = macb_mdio_wait_for_idle(bp);
-   if (err < 0)
-   return err;
+   status = pm_runtime_get_sync(>pdev->dev);
+   if (status < 0)
+   goto mdio_pm_exit;
+
+   status = macb_mdio_wait_for_idle(bp);
+   if (status < 0)
+   goto mdio_read_exit;
 
macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
  | MACB_BF(RW, MACB_MAN_READ)
@@ -345,24 +351,32 @@ static int macb_mdio_read(struct mii_bus *bus, int 
mii_id, int regnum)
  | MACB_BF(REGA, regnum)
  | MACB_BF(CODE, MACB_MAN_CODE)));
 
-   err = macb_mdio_wait_for_idle(bp);
-   if (err < 0)
-   return err;
+   status = macb_mdio_wait_for_idle(bp);
+   if (status < 0)
+   goto mdio_read_exit;
 
-   value = MACB_BFEXT(DATA, macb_readl(bp, MAN));
+   status = MACB_BFEXT(DATA, macb_readl(bp, MAN));
 
-   return value;
+mdio_read_exit:
+   pm_runtime_mark_last_busy(>pdev->dev);
+   pm_runtime_put_autosuspend(>pdev->dev);
+mdio_pm_exit:
+   return status;
 }
 
 static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
   u16 value)
 {
struct macb *bp = bus->priv;
-   int err;
+   int status;
 
-   err = macb_mdio_wait_for_idle(bp);
-   if (err < 0)
-   return err;
+   status = pm_runtime_get_sync(>pdev->dev);
+   if (status < 0)
+   goto mdio_pm_exit;
+
+   status = macb_mdio_wait_for_idle(bp);
+   if (status < 0)
+   goto mdio_write_exit;
 
macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
  | MACB_BF(RW, MACB_MAN_WRITE)
@@ -371,11 +385,15 @@ static int macb_mdio_write(struct mii_bus *bus, int 
mii_id, int regnum,
  | MACB_BF(CODE, MACB_MAN_CODE)
  | MACB_BF(DATA, value)));
 
-   err = macb_mdio_wait_for_idle(bp);
-   if (err < 0)
-   return err;
+   status = macb_mdio_wait_for_idle(bp);
+   if (status < 0)
+   goto mdio_write_exit;
 
-   return 0;
+mdio_write_exit:
+   pm_runtime_mark_last_busy(>pdev->dev);
+   pm_runtime_put_autosuspend(>pdev->dev);
+mdio_pm_exit:
+   return status;
 }
 
 /**
@@ -2418,12 +2436,18 @@ static int macb_open(struct net_device *dev)
 
netdev_dbg(bp->dev, "open\n");
 
+   err = pm_runtime_get_sync(>pdev->dev);
+   if (err < 0)
+   goto pm_exit;
+
/* carrier starts down */
netif_carrier_off(dev);
 
/* if the phy is not yet register, retry later*/
-   if (!dev->phydev)
-   return -EAGAIN;
+   if (!dev->phydev) {
+   err = -EAGAIN;
+   goto pm_exit;
+   }
 
/* RX buffers initialization */
macb_init_rx_buffer_size(bp, bufsz);
@@ -2432,7 +2456,7 @@ static int macb_open(struct net_device *dev)
if (err) {
netdev_err(dev, "Unable to allocate DMA memory (error %d)\n",
   err);
-   return err;
+   goto pm_exit;
}
 
bp->macbgem_ops.mog_init_rings(bp);
@@ -2449,6 +2473,11 @@ static int macb_open(struct net_device *dev)
if (bp->ptp_info)
bp->ptp_info->ptp_init(dev);
 
+pm_exit:
+   if (err) {
+   pm_runtime_put_sync(>pdev->dev);
+   return err;
+   }
return 0;
 }
 
@@ -2477,6 +2506,8 @@ static int 

[PATCH v3 2/4] net: macb: Support clock management for tsu_clk

[Harini Katakam]

TSU clock needs to be enabled/disabled as per support in devicetree
and it should also be controlled during suspend/resume (WOL has no
dependency on this clock).

Signed-off-by: Harini Katakam 
---
v3 and v2:
No changes

 drivers/net/ethernet/cadence/macb.h  |  3 ++-
 drivers/net/ethernet/cadence/macb_main.c | 30 +-
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index f51040f..acc66a7 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -1085,7 +1085,7 @@ struct macb_config {
unsigned intdma_burst_length;
int (*clk_init)(struct platform_device *pdev, struct clk **pclk,
struct clk **hclk, struct clk **tx_clk,
-   struct clk **rx_clk);
+   struct clk **rx_clk, struct clk **tsu_clk);
int (*init)(struct platform_device *pdev);
int jumbo_max_len;
 };
@@ -1165,6 +1165,7 @@ struct macb {
struct clk  *hclk;
struct clk  *tx_clk;
struct clk  *rx_clk;
+   struct clk  *tsu_clk;
struct net_device   *dev;
union {
struct macb_stats   macb;
diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index a19ed2b..be10ee4 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -3325,7 +3325,7 @@ static void macb_probe_queues(void __iomem *mem,
 
 static int macb_clk_init(struct platform_device *pdev, struct clk **pclk,
 struct clk **hclk, struct clk **tx_clk,
-struct clk **rx_clk)
+struct clk **rx_clk, struct clk **tsu_clk)
 {
struct macb_platform_data *pdata;
int err;
@@ -3359,6 +3359,10 @@ static int macb_clk_init(struct platform_device *pdev, 
struct clk **pclk,
if (IS_ERR(*rx_clk))
*rx_clk = NULL;
 
+   *tsu_clk = devm_clk_get(>dev, "tsu_clk");
+   if (IS_ERR(*tsu_clk))
+   *tsu_clk = NULL;
+
err = clk_prepare_enable(*pclk);
if (err) {
dev_err(>dev, "failed to enable pclk (%u)\n", err);
@@ -3383,8 +3387,17 @@ static int macb_clk_init(struct platform_device *pdev, 
struct clk **pclk,
goto err_disable_txclk;
}
 
+   err = clk_prepare_enable(*tsu_clk);
+   if (err) {
+   dev_err(>dev, "failed to enable tsu_clk (%u)\n", err);
+   goto err_disable_rxclk;
+   }
+
return 0;
 
+err_disable_rxclk:
+   clk_disable_unprepare(*rx_clk);
+
 err_disable_txclk:
clk_disable_unprepare(*tx_clk);
 
@@ -3835,13 +3848,14 @@ static const struct net_device_ops at91ether_netdev_ops 
= {
 
 static int at91ether_clk_init(struct platform_device *pdev, struct clk **pclk,
  struct clk **hclk, struct clk **tx_clk,
- struct clk **rx_clk)
+ struct clk **rx_clk, struct clk **tsu_clk)
 {
int err;
 
*hclk = NULL;
*tx_clk = NULL;
*rx_clk = NULL;
+   *tsu_clk = NULL;
 
*pclk = devm_clk_get(>dev, "ether_clk");
if (IS_ERR(*pclk))
@@ -3992,11 +4006,12 @@ static int macb_probe(struct platform_device *pdev)
 {
const struct macb_config *macb_config = _gem_config;
int (*clk_init)(struct platform_device *, struct clk **,
-   struct clk **, struct clk **,  struct clk **)
- = macb_config->clk_init;
+   struct clk **, struct clk **,  struct clk **,
+   struct clk **) = macb_config->clk_init;
int (*init)(struct platform_device *) = macb_config->init;
struct device_node *np = pdev->dev.of_node;
struct clk *pclk, *hclk = NULL, *tx_clk = NULL, *rx_clk = NULL;
+   struct clk *tsu_clk = NULL;
unsigned int queue_mask, num_queues;
struct macb_platform_data *pdata;
bool native_io;
@@ -4024,7 +4039,7 @@ static int macb_probe(struct platform_device *pdev)
}
}
 
-   err = clk_init(pdev, , , _clk, _clk);
+   err = clk_init(pdev, , , _clk, _clk, _clk);
if (err)
return err;
 
@@ -4061,6 +4076,7 @@ static int macb_probe(struct platform_device *pdev)
bp->hclk = hclk;
bp->tx_clk = tx_clk;
bp->rx_clk = rx_clk;
+   bp->tsu_clk = tsu_clk;
if (macb_config)
bp->jumbo_max_len = macb_config->jumbo_max_len;
 
@@ -4180,6 +4196,7 @@ static int macb_probe(struct platform_device *pdev)
clk_disable_unprepare(hclk);
clk_disable_unprepare(pclk);
clk_disab

[PATCH v3 1/4] net: macb: Check MDIO state before read/write and use timeouts

[Harini Katakam]

Replace the while loop in MDIO read/write functions with a timeout.
In addition, add a check for MDIO bus busy before initiating a new
operation as well to make sure there is no ongoing MDIO operation.

Signed-off-by: Shubhrajyoti Datta 
Signed-off-by: Sai Pavan Boddu 
Signed-off-by: Harini Katakam 
Reviewed-by: Andrew Lunn 
---
v3 changes:
Used MACB_BIT for IDLE field

v2 changes:
Use readx_poll_timeout

Changes form RFC:
Cleaned up timeout implementation and moved it to a helper.

 drivers/net/ethernet/cadence/macb.h  |  2 ++
 drivers/net/ethernet/cadence/macb_main.c | 33 ++--
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index 9bbaad9..f51040f 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -715,6 +715,8 @@
__v; \
})
 
+#define MACB_READ_NSR(bp)  macb_readl(bp, NSR)
+
 /* struct macb_dma_desc - Hardware DMA descriptor
  * @addr: DMA address of data buffer
  * @ctrl: Control and status bits
diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index f2915f2..a19ed2b 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -36,6 +36,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "macb.h"
 
 #define MACB_RX_BUFFER_SIZE128
@@ -79,6 +80,8 @@
  */
 #define MACB_HALT_TIMEOUT  1230
 
+#define MACB_MDIO_TIMEOUT  100 /* in usecs */
+
 /* DMA buffer descriptor might be different size
  * depends on hardware configuration:
  *
@@ -318,10 +321,23 @@ static void macb_get_hwaddr(struct macb *bp)
eth_hw_addr_random(bp->dev);
 }
 
+static int macb_mdio_wait_for_idle(struct macb *bp)
+{
+   u32 val;
+
+   return readx_poll_timeout(MACB_READ_NSR, bp, val, val & MACB_BIT(IDLE),
+ 1, MACB_MDIO_TIMEOUT);
+}
+
 static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
 {
struct macb *bp = bus->priv;
int value;
+   int err;
+
+   err = macb_mdio_wait_for_idle(bp);
+   if (err < 0)
+   return err;
 
macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
  | MACB_BF(RW, MACB_MAN_READ)
@@ -329,9 +345,9 @@ static int macb_mdio_read(struct mii_bus *bus, int mii_id, 
int regnum)
  | MACB_BF(REGA, regnum)
  | MACB_BF(CODE, MACB_MAN_CODE)));
 
-   /* wait for end of transfer */
-   while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
-   cpu_relax();
+   err = macb_mdio_wait_for_idle(bp);
+   if (err < 0)
+   return err;
 
value = MACB_BFEXT(DATA, macb_readl(bp, MAN));
 
@@ -342,6 +358,11 @@ static int macb_mdio_write(struct mii_bus *bus, int 
mii_id, int regnum,
   u16 value)
 {
struct macb *bp = bus->priv;
+   int err;
+
+   err = macb_mdio_wait_for_idle(bp);
+   if (err < 0)
+   return err;
 
macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
  | MACB_BF(RW, MACB_MAN_WRITE)
@@ -350,9 +371,9 @@ static int macb_mdio_write(struct mii_bus *bus, int mii_id, 
int regnum,
  | MACB_BF(CODE, MACB_MAN_CODE)
  | MACB_BF(DATA, value)));
 
-   /* wait for end of transfer */
-   while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
-   cpu_relax();
+   err = macb_mdio_wait_for_idle(bp);
+   if (err < 0)
+   return err;
 
return 0;
 }
-- 
2.7.4

[PATCH v2 0/4] Macb power management support for ZynqMP

[Harini Katakam]

This series adds support for macb suspend/resume with system power down.
In relation to the above, this series also updates mdio_read/write
function for PM and adds tsu clock management.

Harini Katakam (4):
  net: macb: Check MDIO state before read/write and use timeouts
  net: macb: Support clock management for tsu_clk
  net: macb: Add pm runtime support
  net: macb: Add support for suspend/resume with full power down

 drivers/net/ethernet/cadence/macb.h  |   5 +-
 drivers/net/ethernet/cadence/macb_main.c | 215 ++-
 2 files changed, 188 insertions(+), 32 deletions(-)

-- 
2.7.4

[PATCH v3 4/4] net: macb: Add support for suspend/resume with full power down

[Harini Katakam]

When macb device is suspended and system is powered down, the clocks
are removed and hence macb should be closed gracefully and restored
upon resume. This patch does the same by switching off the net device,
suspending phy and performing necessary cleanup of interrupts and BDs.
Upon resume, all these are reinitialized again.

Reset of macb device is done only when GEM is not a wake device.
Even when gem is a wake device, tx queues can be stopped and ptp device
can be closed (tsu clock will be disabled in pm_runtime_suspend) as
wake event detection has no dependency on this.

Signed-off-by: Kedareswara rao Appana 
Signed-off-by: Harini Katakam 
---
v3:
Fix >80 char lines

v2 changes:
Fixed parameter passed to phy calls.

 drivers/net/ethernet/cadence/macb_main.c | 40 ++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index 5173c02..ad099fd 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4283,16 +4283,34 @@ static int __maybe_unused macb_suspend(struct device 
*dev)
 {
struct net_device *netdev = dev_get_drvdata(dev);
struct macb *bp = netdev_priv(netdev);
+   struct macb_queue *queue = bp->queues;
+   unsigned long flags;
+   unsigned int q;
+
+   if (!netif_running(netdev))
+   return 0;
 
-   netif_carrier_off(netdev);
-   netif_device_detach(netdev);
 
if (bp->wol & MACB_WOL_ENABLED) {
macb_writel(bp, IER, MACB_BIT(WOL));
macb_writel(bp, WOL, MACB_BIT(MAG));
enable_irq_wake(bp->queues[0].irq);
+   netif_device_detach(netdev);
+   } else {
+   netif_device_detach(netdev);
+   for (q = 0, queue = bp->queues; q < bp->num_queues;
+++q, ++queue)
+   napi_disable(>napi);
+   phy_stop(netdev->phydev);
+   phy_suspend(netdev->phydev);
+   spin_lock_irqsave(>lock, flags);
+   macb_reset_hw(bp);
+   spin_unlock_irqrestore(>lock, flags);
}
 
+   netif_carrier_off(netdev);
+   if (bp->ptp_info)
+   bp->ptp_info->ptp_remove(netdev);
pm_runtime_force_suspend(dev);
 
return 0;
@@ -4302,6 +4320,11 @@ static int __maybe_unused macb_resume(struct device *dev)
 {
struct net_device *netdev = dev_get_drvdata(dev);
struct macb *bp = netdev_priv(netdev);
+   struct macb_queue *queue = bp->queues;
+   unsigned int q;
+
+   if (!netif_running(netdev))
+   return 0;
 
pm_runtime_force_resume(dev);
 
@@ -4309,9 +4332,22 @@ static int __maybe_unused macb_resume(struct device *dev)
macb_writel(bp, IDR, MACB_BIT(WOL));
macb_writel(bp, WOL, 0);
disable_irq_wake(bp->queues[0].irq);
+   } else {
+   macb_writel(bp, NCR, MACB_BIT(MPE));
+   for (q = 0, queue = bp->queues; q < bp->num_queues;
+++q, ++queue)
+   napi_enable(>napi);
+   phy_resume(netdev->phydev);
+   phy_init_hw(netdev->phydev);
+   phy_start(netdev->phydev);
}
 
+   bp->macbgem_ops.mog_init_rings(bp);
+   macb_init_hw(bp);
+   macb_set_rx_mode(netdev);
netif_device_attach(netdev);
+   if (bp->ptp_info)
+   bp->ptp_info->ptp_init(netdev);
 
return 0;
 }
-- 
2.7.4

Re: [PATCH RESEND net] net: phy: xgmiitorgmii: Support generic PHY status read

[Harini Katakam]

Hi,
On Wed, Feb 27, 2019 at 2:35 PM Harini Katakam  wrote:
>
> Hi Andrew, Paul,
>
> On Wed, Feb 27, 2019 at 2:15 PM Michal Simek  wrote:
> >
> > On 21. 02. 19 12:03, Michal Simek wrote:
> > > On 21. 02. 19 11:24, Paul Kocialkowski wrote:
> > >> Hi,
> > >>
> > >> On Wed, 2019-02-20 at 07:58 +0100, Michal Simek wrote:
> > >>> Hi,
> > >>>
> > >>> On 19. 02. 19 18:25, Andrew Lunn wrote:

> > >> Understood. I think we need to start a discussion about how the general
> > >> design of this driver can be improved.
> > >>
> > >> In particular, I wonder if it could work better to make this driver a
> > >> PHY driver that just redirects all its ops to the actual PHY driver,
> > >> except for read_status where it should also add some code.
>
> Thanks, I'm looking into this option and also a way to expose the correct
> interface mode setting as you mentioned below. I'll get back before the
> end of the week. Please do let me know if you have any further suggestions.
>
This IP does not have a PHY identifier or status register that can be accessed
from the phy framework. We could discuss with our design team to add these
in the future. But that would take sometime and this version should be still be
supported. Also, if this IP has a PHY driver, then two phy drivers would have
to be probed which are connected in a serial manner and I believe I'll have to
update the framework to support that. Could you please let me know if you have
any inputs on this?
OR since this is just a bridge IP, is it acceptable to address the error cases?
-> Module loading/unloading
-> Spinlocks for protection
-> Correct phy mode information to the driver.
-> Any other concerns
I could do a respin of this patch after addressing Andrew's comments:
https://patchwork.kernel.org/patch/9290231/

Regards,
Harini

Re: Request for suggestion on net device re-naming/re-ordering based on DT alias

[Harini Katakam]

On Thu, Feb 28, 2019 at 12:10 AM Stephen Hemminger
 wrote:
>
> On Wed, 27 Feb 2019 17:24:03 +0530
> Harini Katakam  wrote:
>

>
> Device naming is a hard problem, and there is no perfect solution.
>
> Device tree should be providing hints to userspace policy for naming, not
> trying to do it in the kernel.
> See: 
> https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/

Thanks Stephen - I'll try this.

Regards,
Harini

Request for suggestion on net device re-naming/re-ordering based on DT alias

[Harini Katakam]

Hi,

We've had some users requesting control over net device name order
when multiple ethernet devices are present on a system. I've tried a
few solutions to this and looked it up on forums. But I apologize if
I have missed something.

I know that the current system allocates eth as per probe order
but that is obviously not stably controlled by user (tried DT
re-ordering and defer probe). One solution is to use DT alias names
to write to (net_device)->name as follows:
Devicetree:
aliases {
   ethernet0 = 
   ethernet1 = 
};
Driver probe:
+   /* Read ethernet DT alias id and assign to right device name*/
+   id = of_alias_get_id(np, "ethernet");
+   if (id < 0) {
+   dev_warn(>dev, "failed to get alias id (%u)\n", id);
+   return id;
+   }
+   snprintf(dev->name, sizeof(dev->name), "eth%d", id);
+

These three drivers seem to have something similar for mdio/phy bus IDs:
drivers/net/ethernet/broadcom/genet/bcmmii.c:409: id =
of_alias_get_id(dn, "eth");
drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c:43: plat->bus_id =
of_alias_get_id(np, "ethernet");
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c:404:
plat->bus_id = of_alias_get_id(np, "ethernet");

Drawback: This approach will break if alias is not provided for one
of the interfaces on board. Not to mention, there could be systems
with multiple ethernet makes (for ex. Cadence macb and Xilinx axienet)
If one of the drivers does not have an alias read mechanism, it is
possible to have clashing ID assignments. Is there any way this
solution can be changed to be stable/acceptable?

One other alternative I've tried is netdev kernel bootargs but this
device name was not being picked by the kernel:
https://www.kernel.org/doc/html/v4.19/admin-guide/kernel-parameters.html
netdev= , , eth0
netdev=, , eth1

Could you please suggest any alternatives?
Thanks!

Regards,
Harini

Re: [PATCH RESEND net] net: phy: xgmiitorgmii: Support generic PHY status read

[Harini Katakam]

Hi Andrew, Paul,

On Wed, Feb 27, 2019 at 2:15 PM Michal Simek  wrote:
>
> On 21. 02. 19 12:03, Michal Simek wrote:
> > On 21. 02. 19 11:24, Paul Kocialkowski wrote:
> >> Hi,
> >>
> >> On Wed, 2019-02-20 at 07:58 +0100, Michal Simek wrote:
> >>> Hi,
> >>>
> >>> On 19. 02. 19 18:25, Andrew Lunn wrote:
> > Thanks for the suggestion! So I had a closer look at that driver to try
> > and see what could go wrong and it looks like I found a few things
> > there.
> 
>  Hi Paul
> 
>  Yes, this driver has issues. If i remember correctly, it got merged
>  while i was on vacation. I pointed out a few issues, but the authors
>  never responded. Feel free to fix it up.
> >>>

Sorry for this - I've synced up with the author and got the comments from the
time this driver was upstreamed. I'll try to address those and Paul's
suggestions
going forward.

> >>> Will be good to know who was that person.
> >>>
> >>> I can't do much this week with this because responsible person for this
> >>> driver is out of office this week. That's why please give us some time
> >>> to get back to this.
> >>
> >> Understood. I think we need to start a discussion about how the general
> >> design of this driver can be improved.
> >>
> >> In particular, I wonder if it could work better to make this driver a
> >> PHY driver that just redirects all its ops to the actual PHY driver,
> >> except for read_status where it should also add some code.

Thanks, I'm looking into this option and also a way to expose the correct
interface mode setting as you mentioned below. I'll get back before the
end of the week. Please do let me know if you have any further suggestions.

Regards,
Harini

> >
> > I didn't take a look at Linux driver but it should work in a way that it
> > checks description (more below) and then wait for attached phy to do its
> > work and on the way back just setup this bridge based on that.
> >
> >> Maybe we could also manage to expose a RGMII PHY mode to the actual PHY
> >> this way. Currently, the PHY mode has to be set to GMII for the MAC to
> >> be configured correctly, but the PHY also gets this information while
> >> it should be told that RGMII is in use. This doesn't seem to play a big
> >> role in PHY configuration though, but it's still inadequate.
> >>
> >> What do you think?

[PATCH v2] net: macb: Apply RXUBR workaround only to versions with errata

[Harini Katakam]

The interrupt handler contains a workaround for RX hang applicable
to Zynq and AT91RM9200 only. Subsequent versions do not need this
workaround. This workaround unnecessarily resets RX whenever RX used
bit read is observed, which can be often under heavy traffic. There
is no other action performed on RX UBR interrupt. Hence introduce a
CAPS mask; enable this interrupt and workaround only on affected
versions.

Signed-off-by: Harini Katakam 
---
v2 changes:
- Added caps mask in correct AT91RM9200 config
- Disabled RXUBR for devices other than AT91RM9200 and Zynq and
kept the interrupt and reset workaround for these two.
- Updated comment in interrupt handler.

Changes from RFC:
- Use CAPS mask instead introducing and errata field.
- Use check only on RX reset part; ISR should still be cleared.

 drivers/net/ethernet/cadence/macb.h  |  3 +++
 drivers/net/ethernet/cadence/macb_main.c | 28 +---
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index 3d45f4c..9bbaad9 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -643,6 +643,7 @@
 #define MACB_CAPS_JUMBO0x0020
 #define MACB_CAPS_GEM_HAS_PTP  0x0040
 #define MACB_CAPS_BD_RD_PREFETCH   0x0080
+#define MACB_CAPS_NEEDS_RSTONUBR   0x0100
 #define MACB_CAPS_FIFO_MODE0x1000
 #define MACB_CAPS_GIGABIT_MODE_AVAILABLE   0x2000
 #define MACB_CAPS_SG_DISABLED  0x4000
@@ -1214,6 +1215,8 @@ struct macb {
 
int rx_bd_rd_prefetch;
int tx_bd_rd_prefetch;
+
+   u32 rx_intr_mask;
 };
 
 #ifdef CONFIG_MACB_USE_HWSTAMP
diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index 66cc792..2b28826 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -56,8 +56,7 @@
 /* level of occupied TX descriptors under which we wake up TX process */
 #define MACB_TX_WAKEUP_THRESH(bp)  (3 * (bp)->tx_ring_size / 4)
 
-#define MACB_RX_INT_FLAGS  (MACB_BIT(RCOMP) | MACB_BIT(RXUBR)  \
-| MACB_BIT(ISR_ROVR))
+#define MACB_RX_INT_FLAGS  (MACB_BIT(RCOMP) | MACB_BIT(ISR_ROVR))
 #define MACB_TX_ERR_FLAGS  (MACB_BIT(ISR_TUND) \
| MACB_BIT(ISR_RLE) \
| MACB_BIT(TXERR))
@@ -1270,7 +1269,7 @@ static int macb_poll(struct napi_struct *napi, int budget)
queue_writel(queue, ISR, MACB_BIT(RCOMP));
napi_reschedule(napi);
} else {
-   queue_writel(queue, IER, MACB_RX_INT_FLAGS);
+   queue_writel(queue, IER, bp->rx_intr_mask);
}
}
 
@@ -1288,7 +1287,7 @@ static void macb_hresp_error_task(unsigned long data)
u32 ctrl;
 
for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
-   queue_writel(queue, IDR, MACB_RX_INT_FLAGS |
+   queue_writel(queue, IDR, bp->rx_intr_mask |
 MACB_TX_INT_FLAGS |
 MACB_BIT(HRESP));
}
@@ -1318,7 +1317,7 @@ static void macb_hresp_error_task(unsigned long data)
 
/* Enable interrupts */
queue_writel(queue, IER,
-MACB_RX_INT_FLAGS |
+bp->rx_intr_mask |
 MACB_TX_INT_FLAGS |
 MACB_BIT(HRESP));
}
@@ -1372,14 +1371,14 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
(unsigned int)(queue - bp->queues),
(unsigned long)status);
 
-   if (status & MACB_RX_INT_FLAGS) {
+   if (status & bp->rx_intr_mask) {
/* There's no point taking any more interrupts
 * until we have processed the buffers. The
 * scheduling call may fail if the poll routine
 * is already scheduled, so disable interrupts
 * now.
 */
-   queue_writel(queue, IDR, MACB_RX_INT_FLAGS);
+   queue_writel(queue, IDR, bp->rx_intr_mask);
if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
queue_writel(queue, ISR, MACB_BIT(RCOMP));
 
@@ -1412,8 +1411,9 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
/* There is a hardware issue under heavy load where DMA can
 * stop, this causes endless "used buffer descriptor read"
   

Re: [PATCH] net: macb: Apply RXUBR workaround only to versions with errata

[Harini Katakam]

Hi Nicolas,

On Fri, Jan 25, 2019 at 3:58 PM  wrote:
>
> On 24/01/2019 at 14:38, Harini Katakam wrote:
> > The interrupt handler contains a workaround for RX hang applicable
> > to Zynq and AT91 only. Subsequent versions do not need this
>
> AT91RM9200 only. It's not the case for other AT91 SoCs (reading errata
> list for them).
>
> That being said I have to add a patch for making this perfectly clear in
> the comment just above the flag's test.
>
> > workaround. This workaround unecessarily resets RX whenever RX used
>
> Typo: unnecessarily
>
> > bit read is observed, which can be often under heavy traffic. Hence
> > introduce an CAPS mask and a check to enable this workaround.
>
> Nack for this one, see below...

Thanks for the review. I dint realize it was AT91RM9200, hence I edited
the wrong config structure. Will fix.



> >   if (status & MACB_BIT(RXUBR)) {
> > - ctrl = macb_readl(bp, NCR);
> > - macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
> > - wmb();
> > - macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
> > + if (bp->caps & MACB_CAPS_NEEDS_RSTONUBR) {
> > + ctrl = macb_readl(bp, NCR);
> > + macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
> > + wmb();
> > + macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
> > + }
>
> As this interrupt routine is not doing anything else than just being
> called now, what about just removing the use of this status bit for
> platforms which don't need this RX reset?

OK, sure I'll try that. I left the interrupt enabled just in case there were
other users performing an action/using this interrupt.

Regards,
Harini

[PATCH] net: macb: Apply RXUBR workaround only to versions with errata

[Harini Katakam]

The interrupt handler contains a workaround for RX hang applicable
to Zynq and AT91 only. Subsequent versions do not need this
workaround. This workaround unecessarily resets RX whenever RX used
bit read is observed, which can be often under heavy traffic. Hence
introduce an CAPS mask and a check to enable this workaround.

Signed-off-by: Harini Katakam 
---
Changes from RFC:
- Use CAPS mask instead introducing and errata field.
- Use check only on RX reset part; ISR should still be cleared.

 drivers/net/ethernet/cadence/macb.h  |  1 +
 drivers/net/ethernet/cadence/macb_main.c | 16 ++--
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index 3d45f4c..2b412fa 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -643,6 +643,7 @@
 #define MACB_CAPS_JUMBO0x0020
 #define MACB_CAPS_GEM_HAS_PTP  0x0040
 #define MACB_CAPS_BD_RD_PREFETCH   0x0080
+#define MACB_CAPS_NEEDS_RSTONUBR   0x0100
 #define MACB_CAPS_FIFO_MODE0x1000
 #define MACB_CAPS_GIGABIT_MODE_AVAILABLE   0x2000
 #define MACB_CAPS_SG_DISABLED  0x4000
diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index 66cc792..0bda1cd 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1416,10 +1416,12 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 * section 16.7.4 for details.
 */
if (status & MACB_BIT(RXUBR)) {
-   ctrl = macb_readl(bp, NCR);
-   macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
-   wmb();
-   macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
+   if (bp->caps & MACB_CAPS_NEEDS_RSTONUBR) {
+   ctrl = macb_readl(bp, NCR);
+   macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
+   wmb();
+   macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
+   }
 
if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
queue_writel(queue, ISR, MACB_BIT(RXUBR));
@@ -3864,7 +3866,8 @@ static int at91ether_init(struct platform_device *pdev)
 }
 
 static const struct macb_config at91sam9260_config = {
-   .caps = MACB_CAPS_USRIO_HAS_CLKEN | MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII,
+   .caps = MACB_CAPS_USRIO_HAS_CLKEN |
+   MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII | MACB_CAPS_NEEDS_RSTONUBR,
.clk_init = macb_clk_init,
.init = macb_init,
 };
@@ -3928,7 +3931,8 @@ static const struct macb_config zynqmp_config = {
 };
 
 static const struct macb_config zynq_config = {
-   .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_NO_GIGABIT_HALF,
+   .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_NO_GIGABIT_HALF |
+   MACB_CAPS_NEEDS_RSTONUBR,
.dma_burst_length = 16,
.clk_init = macb_clk_init,
.init = macb_init,
-- 
2.7.4

[PATCH v2 4/4] net: macb: Add support for suspend/resume with full power down

[Harini Katakam]

When macb device is suspended and system is powered down, the clocks
are removed and hence macb should be closed gracefully and restored
upon resume. This patch does the same by switching off the net device,
suspending phy and performing necessary cleanup of interrupts and BDs.
Upon resume, all these are reinitialized again.

Reset of macb device is done only when GEM is not a wake device.
Even when gem is a wake device, tx queues can be stopped and ptp device
can be closed (tsu clock will be disabled in pm_runtime_suspend) as
wake event detection has no dependency on this.

Signed-off-by: Kedareswara rao Appana 
Signed-off-by: Harini Katakam 
---
v2 changes:
Fixed parameter passed to phy calls.

 drivers/net/ethernet/cadence/macb_main.c | 38 ++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index 4b85ad7..dcb0194 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4249,16 +4249,33 @@ static int __maybe_unused macb_suspend(struct device 
*dev)
 {
struct net_device *netdev = dev_get_drvdata(dev);
struct macb *bp = netdev_priv(netdev);
+   struct macb_queue *queue = bp->queues;
+   unsigned long flags;
+   unsigned int q;
+
+   if (!netif_running(netdev))
+   return 0;
 
-   netif_carrier_off(netdev);
-   netif_device_detach(netdev);
 
if (bp->wol & MACB_WOL_ENABLED) {
macb_writel(bp, IER, MACB_BIT(WOL));
macb_writel(bp, WOL, MACB_BIT(MAG));
enable_irq_wake(bp->queues[0].irq);
+   netif_device_detach(netdev);
+   } else {
+   netif_device_detach(netdev);
+   for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, 
++queue)
+   napi_disable(>napi);
+   phy_stop(netdev->phydev);
+   phy_suspend(netdev->phydev);
+   spin_lock_irqsave(>lock, flags);
+   macb_reset_hw(bp);
+   spin_unlock_irqrestore(>lock, flags);
}
 
+   netif_carrier_off(netdev);
+   if (bp->ptp_info)
+   bp->ptp_info->ptp_remove(netdev);
pm_runtime_force_suspend(dev);
 
return 0;
@@ -4268,6 +4285,11 @@ static int __maybe_unused macb_resume(struct device *dev)
 {
struct net_device *netdev = dev_get_drvdata(dev);
struct macb *bp = netdev_priv(netdev);
+   struct macb_queue *queue = bp->queues;
+   unsigned int q;
+
+   if (!netif_running(netdev))
+   return 0;
 
pm_runtime_force_resume(dev);
 
@@ -4275,9 +4297,21 @@ static int __maybe_unused macb_resume(struct device *dev)
macb_writel(bp, IDR, MACB_BIT(WOL));
macb_writel(bp, WOL, 0);
disable_irq_wake(bp->queues[0].irq);
+   } else {
+   macb_writel(bp, NCR, MACB_BIT(MPE));
+   for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, 
++queue)
+   napi_enable(>napi);
+   phy_resume(netdev->phydev);
+   phy_init_hw(netdev->phydev);
+   phy_start(netdev->phydev);
}
 
+   bp->macbgem_ops.mog_init_rings(bp);
+   macb_init_hw(bp);
+   macb_set_rx_mode(netdev);
netif_device_attach(netdev);
+   if (bp->ptp_info)
+   bp->ptp_info->ptp_init(netdev);
 
return 0;
 }
-- 
2.7.4

[PATCH v2 4/4] net: macb: Add support for suspend/resume with full power down

[Harini Katakam]

When macb device is suspended and system is powered down, the clocks
are removed and hence macb should be closed gracefully and restored
upon resume. This patch does the same by switching off the net device,
suspending phy and performing necessary cleanup of interrupts and BDs.
Upon resume, all these are reinitialized again.

Reset of macb device is done only when GEM is not a wake device.
Even when gem is a wake device, tx queues can be stopped and ptp device
can be closed (tsu clock will be disabled in pm_runtime_suspend) as
wake event detection has no dependency on this.

Signed-off-by: Kedareswara rao Appana 
Signed-off-by: Harini Katakam 
---
v2 changes:
Fixed parameter passed to phy calls.

 drivers/net/ethernet/cadence/macb_main.c | 38 ++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index 4b85ad7..dcb0194 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4249,16 +4249,33 @@ static int __maybe_unused macb_suspend(struct device 
*dev)
 {
struct net_device *netdev = dev_get_drvdata(dev);
struct macb *bp = netdev_priv(netdev);
+   struct macb_queue *queue = bp->queues;
+   unsigned long flags;
+   unsigned int q;
+
+   if (!netif_running(netdev))
+   return 0;
 
-   netif_carrier_off(netdev);
-   netif_device_detach(netdev);
 
if (bp->wol & MACB_WOL_ENABLED) {
macb_writel(bp, IER, MACB_BIT(WOL));
macb_writel(bp, WOL, MACB_BIT(MAG));
enable_irq_wake(bp->queues[0].irq);
+   netif_device_detach(netdev);
+   } else {
+   netif_device_detach(netdev);
+   for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, 
++queue)
+   napi_disable(>napi);
+   phy_stop(netdev->phydev);
+   phy_suspend(netdev->phydev);
+   spin_lock_irqsave(>lock, flags);
+   macb_reset_hw(bp);
+   spin_unlock_irqrestore(>lock, flags);
}
 
+   netif_carrier_off(netdev);
+   if (bp->ptp_info)
+   bp->ptp_info->ptp_remove(netdev);
pm_runtime_force_suspend(dev);
 
return 0;
@@ -4268,6 +4285,11 @@ static int __maybe_unused macb_resume(struct device *dev)
 {
struct net_device *netdev = dev_get_drvdata(dev);
struct macb *bp = netdev_priv(netdev);
+   struct macb_queue *queue = bp->queues;
+   unsigned int q;
+
+   if (!netif_running(netdev))
+   return 0;
 
pm_runtime_force_resume(dev);
 
@@ -4275,9 +4297,21 @@ static int __maybe_unused macb_resume(struct device *dev)
macb_writel(bp, IDR, MACB_BIT(WOL));
macb_writel(bp, WOL, 0);
disable_irq_wake(bp->queues[0].irq);
+   } else {
+   macb_writel(bp, NCR, MACB_BIT(MPE));
+   for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, 
++queue)
+   napi_enable(>napi);
+   phy_resume(netdev->phydev);
+   phy_init_hw(netdev->phydev);
+   phy_start(netdev->phydev);
}
 
+   bp->macbgem_ops.mog_init_rings(bp);
+   macb_init_hw(bp);
+   macb_set_rx_mode(netdev);
netif_device_attach(netdev);
+   if (bp->ptp_info)
+   bp->ptp_info->ptp_init(netdev);
 
return 0;
 }
-- 
2.7.4

[RFC PATCH] net: macb: Apply RXUBR workaround only to versions with errata

[Harini Katakam]

The interrupt handler contains a workaround for RX hang applicable
to Zynq and AT91 only. Subsequent versions do not need this
workaround. This workaround unecessarily reset RX whenever RX used
bit read is observed, which can be often under heavy traffic.Hence
introduce an errata field and a check to enable this workaround.

Signed-off-by: Harini Katakam 
---
Note: Enabled the errata in zynq and at91 configs only.
Please advise if any other versions are affected by this errata.

 drivers/net/ethernet/cadence/macb.h  | 6 ++
 drivers/net/ethernet/cadence/macb_main.c | 9 +++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index 3d45f4c..f6903d6 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -648,6 +648,9 @@
 #define MACB_CAPS_SG_DISABLED  0x4000
 #define MACB_CAPS_MACB_IS_GEM  0x8000
 
+/* Errata mask bits */
+#define MACB_ERRATA_RXLOCKUP   0x0001
+
 /* LSO settings */
 #define MACB_LSO_UFO_ENABLE0x01
 #define MACB_LSO_TSO_ENABLE0x02
@@ -1085,6 +1088,7 @@ struct macb_config {
struct clk **rx_clk);
int (*init)(struct platform_device *pdev);
int jumbo_max_len;
+   u32 errata;
 };
 
 struct tsu_incr {
@@ -1214,6 +1218,8 @@ struct macb {
 
int rx_bd_rd_prefetch;
int tx_bd_rd_prefetch;
+
+   u32 errata;
 };
 
 #ifdef CONFIG_MACB_USE_HWSTAMP
diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index 1d86b4d..f0bb8a4 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1379,7 +1379,8 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 * the at91 manual, section 41.3.1 or the Zynq manual
 * section 16.7.4 for details.
 */
-   if (status & MACB_BIT(RXUBR)) {
+   if ((bp->errata & MACB_ERRATA_RXLOCKUP) &&
+   (status & MACB_BIT(RXUBR))) {
ctrl = macb_readl(bp, NCR);
macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
wmb();
@@ -3835,6 +3836,7 @@ static const struct macb_config at91sam9260_config = {
.caps = MACB_CAPS_USRIO_HAS_CLKEN | MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII,
.clk_init = macb_clk_init,
.init = macb_init,
+   .errata = MACB_ERRATA_RXLOCKUP,
 };
 
 static const struct macb_config sama5d3macb_config = {
@@ -3900,6 +3902,7 @@ static const struct macb_config zynq_config = {
.dma_burst_length = 16,
.clk_init = macb_clk_init,
.init = macb_init,
+   .errata = MACB_ERRATA_RXLOCKUP,
 };
 
 static const struct of_device_id macb_dt_ids[] = {
@@ -4005,8 +4008,10 @@ static int macb_probe(struct platform_device *pdev)
bp->hclk = hclk;
bp->tx_clk = tx_clk;
bp->rx_clk = rx_clk;
-   if (macb_config)
+   if (macb_config) {
bp->jumbo_max_len = macb_config->jumbo_max_len;
+   bp->errata = macb_config->errata;
+   }
 
bp->wol = 0;
if (of_get_property(np, "magic-packet", NULL))
-- 
2.7.4

[RFC PATCH] net: macb: Apply RXUBR workaround only to versions with errata

[Harini Katakam]

The interrupt handler contains a workaround for RX hang applicable
to Zynq and AT91 only. Subsequent versions do not need this
workaround. This workaround unecessarily reset RX whenever RX used
bit read is observed, which can be often under heavy traffic.Hence
introduce an errata field and a check to enable this workaround.

Signed-off-by: Harini Katakam 
---
Note: Enabled the errata in zynq and at91 configs only.
Please advise if any other versions are affected by this errata.

 drivers/net/ethernet/cadence/macb.h  | 6 ++
 drivers/net/ethernet/cadence/macb_main.c | 9 +++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index 3d45f4c..f6903d6 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -648,6 +648,9 @@
 #define MACB_CAPS_SG_DISABLED  0x4000
 #define MACB_CAPS_MACB_IS_GEM  0x8000
 
+/* Errata mask bits */
+#define MACB_ERRATA_RXLOCKUP   0x0001
+
 /* LSO settings */
 #define MACB_LSO_UFO_ENABLE0x01
 #define MACB_LSO_TSO_ENABLE0x02
@@ -1085,6 +1088,7 @@ struct macb_config {
struct clk **rx_clk);
int (*init)(struct platform_device *pdev);
int jumbo_max_len;
+   u32 errata;
 };
 
 struct tsu_incr {
@@ -1214,6 +1218,8 @@ struct macb {
 
int rx_bd_rd_prefetch;
int tx_bd_rd_prefetch;
+
+   u32 errata;
 };
 
 #ifdef CONFIG_MACB_USE_HWSTAMP
diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index 1d86b4d..f0bb8a4 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1379,7 +1379,8 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 * the at91 manual, section 41.3.1 or the Zynq manual
 * section 16.7.4 for details.
 */
-   if (status & MACB_BIT(RXUBR)) {
+   if ((bp->errata & MACB_ERRATA_RXLOCKUP) &&
+   (status & MACB_BIT(RXUBR))) {
ctrl = macb_readl(bp, NCR);
macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
wmb();
@@ -3835,6 +3836,7 @@ static const struct macb_config at91sam9260_config = {
.caps = MACB_CAPS_USRIO_HAS_CLKEN | MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII,
.clk_init = macb_clk_init,
.init = macb_init,
+   .errata = MACB_ERRATA_RXLOCKUP,
 };
 
 static const struct macb_config sama5d3macb_config = {
@@ -3900,6 +3902,7 @@ static const struct macb_config zynq_config = {
.dma_burst_length = 16,
.clk_init = macb_clk_init,
.init = macb_init,
+   .errata = MACB_ERRATA_RXLOCKUP,
 };
 
 static const struct of_device_id macb_dt_ids[] = {
@@ -4005,8 +4008,10 @@ static int macb_probe(struct platform_device *pdev)
bp->hclk = hclk;
bp->tx_clk = tx_clk;
bp->rx_clk = rx_clk;
-   if (macb_config)
+   if (macb_config) {
bp->jumbo_max_len = macb_config->jumbo_max_len;
+   bp->errata = macb_config->errata;
+   }
 
bp->wol = 0;
if (of_get_property(np, "magic-packet", NULL))
-- 
2.7.4

Re: [RFC PATCH 5/5] net: macb: Add WOL support with ARP

[Harini Katakam]

Hi Claudiu,

On Fri, May 4, 2018 at 5:47 PM, Claudiu Beznea
<claudiu.bez...@microchip.com> wrote:
>
>
> On 22.03.2018 15:51, harinikatakamli...@gmail.com wrote:
>> From: Harini Katakam <hari...@xilinx.com>
>>
>> This patch enables ARP wake event support in GEM through the following:
>>
>> -> WOL capability can be selected based on the SoC/GEM IP version rather
>> than a devictree property alone. Hence add a new capability property and
>> set device as "wakeup capable" in probe in this case.
>> -> Wake source selection can be done via ethtool or by enabling wakeup
>> in /sys/devices/platform//ethx/power/
>> This patch adds default wake source as ARP and the existing selection of
>> WOL using magic packet remains unchanged.
>> -> When GEM is the wake device with ARP as the wake event, the current
>> IP address to match is written to WOL register along with other
>> necessary confuguration required for MAC to recognize an ARP event.
>> -> While RX needs to remain enabled, there is no need to process the
>> actual wake packet - hence tie off all RX queues to avoid unnecessary
>> processing by DMA in the background.
>
> Why is this different for magic packet vs ARP packet?

This should ideally be the same whether it is a magic packet or ARP on
the version of the IP we use (more details in my comment below).
I simply did not alter the magic packet code for now to avoid breaking
others' flow.


>> +#define MACB_CAPS_WOL0x0080
>
> I think would be better to have this as part of bp->wol and use it properly
> in suspend/resume hooks.

I think a capability flag as part of config structure is better
because this is clearly an SoC related feature and there is no need
to have a devicetree property.


> Wouldn't it work if you will change it in something like this:
>
> u32 wolmask, arpipmask = 0;
>
> if (bp->wol & MACB_WOL_ENABLED) {
> macb_writel(bp, IER, MACB_BIT(WOL));
>
> if (bp->wol & MACB_WOL_HAS_ARP_PACKET) {
> /* Enable broadcast. */
> gem_writel(bp, NCFGR, gem_readl(bp, NCFGR) & 
> ~MACB_BIT(NBC));
> arpipmask = 
> cpu_to_be32p(>dev->ip_ptr->ifa_list->ifa_local) & 0x;
> wolmask = arpipmask | MACB_BIT(ARP);
> } else {
> wolmask = MACB_BIT(MAG);
> }
>
> macb_writel(bp, WOL, wolmask);
> enable_irq_wake(bp->queues[0].irq);

The above would work. But I'd still have to add the RX BD changes
and then stop the phy, disable interrupt etc., for most optimal power
down state - the idea is to keep only is essential to detect a wake event.

> netif_device_detach(netdev);
> }
>
> I cannot find anything particular for ARP WOL events in datasheet. Also,
> I cannot find something related to DMA activity while WOL is active

Can you please let me know which version you are referring to?
ZynqMP uses the IP version r1p06 or 07.

There is a clear set of rules for ARP wake event to be recognized.

About the DMA activity, it is not explicitly mentioned but we have
observed that the DMA will continue to process incoming packets
and try to write them to the memory and Cadence has confirmed
the same. Later versions of the IP may have some provision to
stop DMA activity on RX channel but unfortunately in this version,
using a dummy RX buffer descriptor is the only way.

I'm looking into your other suggestions.
Thanks, will get back to you.

Regards,
Harini

Re: [RFC PATCH 5/5] net: macb: Add WOL support with ARP

[Harini Katakam]

Hi Claudiu,

On Fri, May 4, 2018 at 5:47 PM, Claudiu Beznea
 wrote:
>
>
> On 22.03.2018 15:51, harinikatakamli...@gmail.com wrote:
>> From: Harini Katakam 
>>
>> This patch enables ARP wake event support in GEM through the following:
>>
>> -> WOL capability can be selected based on the SoC/GEM IP version rather
>> than a devictree property alone. Hence add a new capability property and
>> set device as "wakeup capable" in probe in this case.
>> -> Wake source selection can be done via ethtool or by enabling wakeup
>> in /sys/devices/platform//ethx/power/
>> This patch adds default wake source as ARP and the existing selection of
>> WOL using magic packet remains unchanged.
>> -> When GEM is the wake device with ARP as the wake event, the current
>> IP address to match is written to WOL register along with other
>> necessary confuguration required for MAC to recognize an ARP event.
>> -> While RX needs to remain enabled, there is no need to process the
>> actual wake packet - hence tie off all RX queues to avoid unnecessary
>> processing by DMA in the background.
>
> Why is this different for magic packet vs ARP packet?

This should ideally be the same whether it is a magic packet or ARP on
the version of the IP we use (more details in my comment below).
I simply did not alter the magic packet code for now to avoid breaking
others' flow.


>> +#define MACB_CAPS_WOL0x0080
>
> I think would be better to have this as part of bp->wol and use it properly
> in suspend/resume hooks.

I think a capability flag as part of config structure is better
because this is clearly an SoC related feature and there is no need
to have a devicetree property.


> Wouldn't it work if you will change it in something like this:
>
> u32 wolmask, arpipmask = 0;
>
> if (bp->wol & MACB_WOL_ENABLED) {
> macb_writel(bp, IER, MACB_BIT(WOL));
>
> if (bp->wol & MACB_WOL_HAS_ARP_PACKET) {
> /* Enable broadcast. */
> gem_writel(bp, NCFGR, gem_readl(bp, NCFGR) & 
> ~MACB_BIT(NBC));
> arpipmask = 
> cpu_to_be32p(>dev->ip_ptr->ifa_list->ifa_local) & 0x;
> wolmask = arpipmask | MACB_BIT(ARP);
> } else {
> wolmask = MACB_BIT(MAG);
> }
>
> macb_writel(bp, WOL, wolmask);
> enable_irq_wake(bp->queues[0].irq);

The above would work. But I'd still have to add the RX BD changes
and then stop the phy, disable interrupt etc., for most optimal power
down state - the idea is to keep only is essential to detect a wake event.

> netif_device_detach(netdev);
> }
>
> I cannot find anything particular for ARP WOL events in datasheet. Also,
> I cannot find something related to DMA activity while WOL is active

Can you please let me know which version you are referring to?
ZynqMP uses the IP version r1p06 or 07.

There is a clear set of rules for ARP wake event to be recognized.

About the DMA activity, it is not explicitly mentioned but we have
observed that the DMA will continue to process incoming packets
and try to write them to the memory and Cadence has confirmed
the same. Later versions of the IP may have some provision to
stop DMA activity on RX channel but unfortunately in this version,
using a dummy RX buffer descriptor is the only way.

I'm looking into your other suggestions.
Thanks, will get back to you.

Regards,
Harini

Re: [RFC PATCH 4/5] net: macb: Add support for suspend/resume with full power down

[Harini Katakam]

Hi Claudiu,

On Thu, May 3, 2018 at 3:39 PM, Claudiu Beznea
<claudiu.bez...@microchip.com> wrote:
>
>
> On 22.03.2018 15:51, harinikatakamli...@gmail.com wrote:
>> From: Harini Katakam <hari...@xilinx.com>
>>
>> When macb device is suspended and system is powered down, the clocks
>> are removed and hence macb should be closed gracefully and restored
>> upon resume.
>
> Is this a power saving mode which shut down the core?

The Ethernet IP is suspended and a majority of the SoC is shut down, yes.


>> + netif_device_detach(netdev);
>> + } else {
>> + netif_device_detach(netdev);
>> + for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, 
>> ++queue)
>> + napi_disable(>napi);
>> + phy_stop(netdev->phydev);
>> + phy_suspend(netdev->phydev);
>> + spin_lock_irqsave(>lock, flags);
>> + macb_reset_hw(bp);
>> + spin_unlock_irqrestore(>lock, flags);
>
> Wouldn't be simple to just call macb_close() here?


>
> Wouln't be simpler to call macb_open() here?

No, I think that would be excessive for suspend. This does just
enough to put the IP into suspend and cut off clocks.
For ex., the RX and TX buffers are not freed and allocated again
in this cycle, just the buffer descriptors.

Regards,
Harini

Re: [RFC PATCH 4/5] net: macb: Add support for suspend/resume with full power down

[Harini Katakam]

Hi Claudiu,

On Thu, May 3, 2018 at 3:39 PM, Claudiu Beznea
 wrote:
>
>
> On 22.03.2018 15:51, harinikatakamli...@gmail.com wrote:
>> From: Harini Katakam 
>>
>> When macb device is suspended and system is powered down, the clocks
>> are removed and hence macb should be closed gracefully and restored
>> upon resume.
>
> Is this a power saving mode which shut down the core?

The Ethernet IP is suspended and a majority of the SoC is shut down, yes.


>> + netif_device_detach(netdev);
>> + } else {
>> + netif_device_detach(netdev);
>> + for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, 
>> ++queue)
>> + napi_disable(>napi);
>> + phy_stop(netdev->phydev);
>> + phy_suspend(netdev->phydev);
>> + spin_lock_irqsave(>lock, flags);
>> + macb_reset_hw(bp);
>> + spin_unlock_irqrestore(>lock, flags);
>
> Wouldn't be simple to just call macb_close() here?


>
> Wouln't be simpler to call macb_open() here?

No, I think that would be excessive for suspend. This does just
enough to put the IP into suspend and cut off clocks.
For ex., the RX and TX buffers are not freed and allocated again
in this cycle, just the buffer descriptors.

Regards,
Harini

Re: [RFC PATCH 3/5] net: macb: Add pm runtime support

[Harini Katakam]

Hi Claudiu,

On Thu, May 3, 2018 at 3:39 PM, Claudiu Beznea
<claudiu.bez...@microchip.com> wrote:
>
>
> On 22.03.2018 15:51, harinikatakamli...@gmail.com wrote:
>> From: Harini Katakam <hari...@xilinx.com>

> I would use a "goto" instruction, e.g.:
> value = -ETIMEDOUT;
> goto out;
>

Will do

>
> Below, in macb_open() you have a return err; case:
> err = macb_alloc_consistent(bp);
> if (err) {
> netdev_err(dev, "Unable to allocate DMA memory (error %d)\n",
>err);
> return err;
> }
>
> You have to undo pm_runtime_get_sync() with pm_runtime_put_sync() or something
> similar to decrement dev->power.usage_count.

Will do


>> @@ -4104,11 +4145,16 @@ static int macb_remove(struct platform_device *pdev)
>>   mdiobus_free(bp->mii_bus);
>>
>>   unregister_netdev(dev);
>> - clk_disable_unprepare(bp->tx_clk);
>> - clk_disable_unprepare(bp->hclk);
>> - clk_disable_unprepare(bp->pclk);
>> - clk_disable_unprepare(bp->rx_clk);
>> - clk_disable_unprepare(bp->tsu_clk);
>> + pm_runtime_disable(>dev);
>> + pm_runtime_dont_use_autosuspend(>dev);
>> + if (!pm_runtime_suspended(>dev)) {
>> + clk_disable_unprepare(bp->tx_clk);
>> + clk_disable_unprepare(bp->hclk);
>> + clk_disable_unprepare(bp->pclk);
>> + clk_disable_unprepare(bp->rx_clk);
>> + clk_disable_unprepare(bp->tsu_clk);
>> + pm_runtime_set_suspended(>dev);
>
> This is driver remove function. Shouldn't clocks be removed?

clk_disable_unprepare IS being done here.
The check for !pm_runtime_suspended is just to make sure the
clocks are not already removed (in runtime_suspend).

Regards,
Harini

Re: [RFC PATCH 3/5] net: macb: Add pm runtime support

[Harini Katakam]

Hi Claudiu,

On Thu, May 3, 2018 at 3:39 PM, Claudiu Beznea
 wrote:
>
>
> On 22.03.2018 15:51, harinikatakamli...@gmail.com wrote:
>> From: Harini Katakam 

> I would use a "goto" instruction, e.g.:
> value = -ETIMEDOUT;
> goto out;
>

Will do

>
> Below, in macb_open() you have a return err; case:
> err = macb_alloc_consistent(bp);
> if (err) {
> netdev_err(dev, "Unable to allocate DMA memory (error %d)\n",
>err);
> return err;
> }
>
> You have to undo pm_runtime_get_sync() with pm_runtime_put_sync() or something
> similar to decrement dev->power.usage_count.

Will do


>> @@ -4104,11 +4145,16 @@ static int macb_remove(struct platform_device *pdev)
>>   mdiobus_free(bp->mii_bus);
>>
>>   unregister_netdev(dev);
>> - clk_disable_unprepare(bp->tx_clk);
>> - clk_disable_unprepare(bp->hclk);
>> - clk_disable_unprepare(bp->pclk);
>> - clk_disable_unprepare(bp->rx_clk);
>> - clk_disable_unprepare(bp->tsu_clk);
>> + pm_runtime_disable(>dev);
>> + pm_runtime_dont_use_autosuspend(>dev);
>> + if (!pm_runtime_suspended(>dev)) {
>> + clk_disable_unprepare(bp->tx_clk);
>> + clk_disable_unprepare(bp->hclk);
>> + clk_disable_unprepare(bp->pclk);
>> + clk_disable_unprepare(bp->rx_clk);
>> + clk_disable_unprepare(bp->tsu_clk);
>> + pm_runtime_set_suspended(>dev);
>
> This is driver remove function. Shouldn't clocks be removed?

clk_disable_unprepare IS being done here.
The check for !pm_runtime_suspended is just to make sure the
clocks are not already removed (in runtime_suspend).

Regards,
Harini

Re: [RFC PATCH 1/5] net: macb: Check MDIO state before read/write and use timeouts

[Harini Katakam]

Hi Claudiu,

On Thu, May 3, 2018 at 3:38 PM, Claudiu Beznea
<claudiu.bez...@microchip.com> wrote:
>
>
> On 22.03.2018 15:51, harinikatakamli...@gmail.com wrote:
>> From: Harini Katakam <hari...@xilinx.com>
>>

>> + ulong timeout;
>> +
>> + timeout = jiffies + msecs_to_jiffies(1000);
>> + /* wait for end of transfer */
>> + do {
>> + if (MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
>> + break;
>> +
>> + cpu_relax();
>> + } while (!time_after_eq(jiffies, timeout));
>> +
>> + if (time_after_eq(jiffies, timeout)) {
>> + netdev_err(bp->dev, "wait for end of transfer timed out\n");
>> + return -ETIMEDOUT;
>> + }
>
> Wouldn't be cleaner to keep it in this way:
>
> while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR))) {
> if (time_after_eq(jiffies, timeout) {
> netdev_err(bp->dev, "wait for end of transfer timed 
> out\n");
> return -ETIMEDOUT;
> }
> cpu_relax();
> }
>

Thanks for the review.
Sure, will update in next version.

Regards,
Harini

Re: [RFC PATCH 1/5] net: macb: Check MDIO state before read/write and use timeouts

[Harini Katakam]

Hi Claudiu,

On Thu, May 3, 2018 at 3:38 PM, Claudiu Beznea
 wrote:
>
>
> On 22.03.2018 15:51, harinikatakamli...@gmail.com wrote:
>> From: Harini Katakam 
>>

>> + ulong timeout;
>> +
>> + timeout = jiffies + msecs_to_jiffies(1000);
>> + /* wait for end of transfer */
>> + do {
>> + if (MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
>> + break;
>> +
>> + cpu_relax();
>> + } while (!time_after_eq(jiffies, timeout));
>> +
>> + if (time_after_eq(jiffies, timeout)) {
>> + netdev_err(bp->dev, "wait for end of transfer timed out\n");
>> + return -ETIMEDOUT;
>> + }
>
> Wouldn't be cleaner to keep it in this way:
>
> while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR))) {
> if (time_after_eq(jiffies, timeout) {
> netdev_err(bp->dev, "wait for end of transfer timed 
> out\n");
> return -ETIMEDOUT;
> }
> cpu_relax();
> }
>

Thanks for the review.
Sure, will update in next version.

Regards,
Harini

Re: [PATCH v3] net: macb: Handle HRESP error

[Harini Katakam]

Hi David,

On Sat, Jan 27, 2018 at 12:09 PM,  <harinikatakamli...@gmail.com> wrote:
> From: Harini Katakam <harini.kata...@xilinx.com>
>
> Handle HRESP error by doing a SW reset of RX and TX and
> re-initializing the descriptors, RX and TX queue pointers.
>
> Signed-off-by: Harini Katakam <hari...@xilinx.com>
> Signed-off-by: Michal Simek <michal.si...@xilinx.com>
> ---
> v3 and v2 changes:
> Fixed patch formatting errors
> Rebased on latest net-next and reinitialized
> multiple rx queues in error handling.
>
>  drivers/net/ethernet/cadence/macb.h  |  3 ++
>  drivers/net/ethernet/cadence/macb_main.c | 59 
> +---
>  2 files changed, 58 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.h 
> b/drivers/net/ethernet/cadence/macb.h
> index c50c5ec..8665982 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #if defined(CONFIG_ARCH_DMA_ADDR_T_64BIT) || defined(CONFIG_MACB_USE_HWSTAMP)
>  #define MACB_EXT_DESC
> @@ -1200,6 +1201,8 @@ struct macb {
> struct ethtool_rx_fs_list rx_fs_list;
> spinlock_t rx_fs_lock;
> unsigned int max_tuples;
> +
> +   struct tasklet_struct   hresp_err_tasklet;
>  };
>
>  #ifdef CONFIG_MACB_USE_HWSTAMP
> diff --git a/drivers/net/ethernet/cadence/macb_main.c 
> b/drivers/net/ethernet/cadence/macb_main.c
> index 234667e..e84afcf 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -1258,6 +1258,57 @@ static int macb_poll(struct napi_struct *napi, int 
> budget)
> return work_done;
>  }
>
> +static void macb_hresp_error_task(unsigned long data)
> +{
> +   struct macb *bp = (struct macb *)data;
> +   struct net_device *dev = bp->dev;
> +   struct macb_queue *queue = bp->queues;
> +   unsigned int q;
> +   u32 ctrl;
> +
> +   for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
> +   queue_writel(queue, IDR, MACB_RX_INT_FLAGS |
> +MACB_TX_INT_FLAGS |
> +MACB_BIT(HRESP));
> +   }
> +   ctrl = macb_readl(bp, NCR);
> +   ctrl &= ~(MACB_BIT(RE) | MACB_BIT(TE));
> +   macb_writel(bp, NCR, ctrl);
> +
> +   netif_tx_stop_all_queues(dev);
> +   netif_carrier_off(dev);
> +
> +   bp->macbgem_ops.mog_init_rings(bp);
> +
> +   /* Initialize TX and RX buffers */
> +   for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
> +   queue_writel(queue, RBQP, lower_32_bits(queue->rx_ring_dma));
> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> +   if (bp->hw_dma_cap & HW_DMA_CAP_64B)
> +   queue_writel(queue, RBQPH,
> +upper_32_bits(queue->rx_ring_dma));
> +#endif
> +   queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> +   if (bp->hw_dma_cap & HW_DMA_CAP_64B)
> +   queue_writel(queue, TBQPH,
> +upper_32_bits(queue->tx_ring_dma));
> +#endif
> +
> +   /* Enable interrupts */
> +   queue_writel(queue, IER,
> +MACB_RX_INT_FLAGS |
> +MACB_TX_INT_FLAGS |
> +MACB_BIT(HRESP));
> +   }
> +
> +   ctrl |= MACB_BIT(RE) | MACB_BIT(TE);
> +   macb_writel(bp, NCR, ctrl);
> +
> +   netif_carrier_on(dev);
> +   netif_tx_start_all_queues(dev);
> +}
> +
>  static irqreturn_t macb_interrupt(int irq, void *dev_id)
>  {
> struct macb_queue *queue = dev_id;
> @@ -1347,10 +1398,7 @@ static irqreturn_t macb_interrupt(int irq, void 
> *dev_id)
> }
>
> if (status & MACB_BIT(HRESP)) {
> -   /* TODO: Reset the hardware, and maybe move the
> -* netdev_err to a lower-priority context as well
> -* (work queue?)
> -*/
> +   tasklet_schedule(>hresp_err_tasklet);
> netdev_err(dev, "DMA bus error: HRESP not OK\n");
>
> if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> @@ -3937,6 +3985,9 @@ static int macb_probe(struct platform_device *pdev)
> goto err_out_unregister_mdio;
> }
>
> +   tasklet_init(>hresp_err_tasklet, macb_hresp_error_task,
> +(unsigned long)bp);
> +
> phy_attached_info(phydev);
>
> netdev_info(dev, "Cadence %s rev 0x%08x at 0x%08lx irq %d (%pM)\n",
> --
> 2.7.4
>

Apologies for the spam.
This is the v3. Please do let me know in case you still see any corruption.

Regards,
Harini

Re: [PATCH v3] net: macb: Handle HRESP error

[Harini Katakam]

Hi David,

On Sat, Jan 27, 2018 at 12:09 PM,   wrote:
> From: Harini Katakam 
>
> Handle HRESP error by doing a SW reset of RX and TX and
> re-initializing the descriptors, RX and TX queue pointers.
>
> Signed-off-by: Harini Katakam 
> Signed-off-by: Michal Simek 
> ---
> v3 and v2 changes:
> Fixed patch formatting errors
> Rebased on latest net-next and reinitialized
> multiple rx queues in error handling.
>
>  drivers/net/ethernet/cadence/macb.h  |  3 ++
>  drivers/net/ethernet/cadence/macb_main.c | 59 
> +---
>  2 files changed, 58 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.h 
> b/drivers/net/ethernet/cadence/macb.h
> index c50c5ec..8665982 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #if defined(CONFIG_ARCH_DMA_ADDR_T_64BIT) || defined(CONFIG_MACB_USE_HWSTAMP)
>  #define MACB_EXT_DESC
> @@ -1200,6 +1201,8 @@ struct macb {
> struct ethtool_rx_fs_list rx_fs_list;
> spinlock_t rx_fs_lock;
> unsigned int max_tuples;
> +
> +   struct tasklet_struct   hresp_err_tasklet;
>  };
>
>  #ifdef CONFIG_MACB_USE_HWSTAMP
> diff --git a/drivers/net/ethernet/cadence/macb_main.c 
> b/drivers/net/ethernet/cadence/macb_main.c
> index 234667e..e84afcf 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -1258,6 +1258,57 @@ static int macb_poll(struct napi_struct *napi, int 
> budget)
> return work_done;
>  }
>
> +static void macb_hresp_error_task(unsigned long data)
> +{
> +   struct macb *bp = (struct macb *)data;
> +   struct net_device *dev = bp->dev;
> +   struct macb_queue *queue = bp->queues;
> +   unsigned int q;
> +   u32 ctrl;
> +
> +   for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
> +   queue_writel(queue, IDR, MACB_RX_INT_FLAGS |
> +MACB_TX_INT_FLAGS |
> +MACB_BIT(HRESP));
> +   }
> +   ctrl = macb_readl(bp, NCR);
> +   ctrl &= ~(MACB_BIT(RE) | MACB_BIT(TE));
> +   macb_writel(bp, NCR, ctrl);
> +
> +   netif_tx_stop_all_queues(dev);
> +   netif_carrier_off(dev);
> +
> +   bp->macbgem_ops.mog_init_rings(bp);
> +
> +   /* Initialize TX and RX buffers */
> +   for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
> +   queue_writel(queue, RBQP, lower_32_bits(queue->rx_ring_dma));
> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> +   if (bp->hw_dma_cap & HW_DMA_CAP_64B)
> +   queue_writel(queue, RBQPH,
> +upper_32_bits(queue->rx_ring_dma));
> +#endif
> +   queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> +   if (bp->hw_dma_cap & HW_DMA_CAP_64B)
> +   queue_writel(queue, TBQPH,
> +upper_32_bits(queue->tx_ring_dma));
> +#endif
> +
> +   /* Enable interrupts */
> +   queue_writel(queue, IER,
> +MACB_RX_INT_FLAGS |
> +MACB_TX_INT_FLAGS |
> +MACB_BIT(HRESP));
> +   }
> +
> +   ctrl |= MACB_BIT(RE) | MACB_BIT(TE);
> +   macb_writel(bp, NCR, ctrl);
> +
> +   netif_carrier_on(dev);
> +   netif_tx_start_all_queues(dev);
> +}
> +
>  static irqreturn_t macb_interrupt(int irq, void *dev_id)
>  {
> struct macb_queue *queue = dev_id;
> @@ -1347,10 +1398,7 @@ static irqreturn_t macb_interrupt(int irq, void 
> *dev_id)
> }
>
> if (status & MACB_BIT(HRESP)) {
> -   /* TODO: Reset the hardware, and maybe move the
> -* netdev_err to a lower-priority context as well
> -* (work queue?)
> -*/
> +   tasklet_schedule(>hresp_err_tasklet);
> netdev_err(dev, "DMA bus error: HRESP not OK\n");
>
> if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> @@ -3937,6 +3985,9 @@ static int macb_probe(struct platform_device *pdev)
> goto err_out_unregister_mdio;
> }
>
> +   tasklet_init(>hresp_err_tasklet, macb_hresp_error_task,
> +(unsigned long)bp);
> +
> phy_attached_info(phydev);
>
> netdev_info(dev, "Cadence %s rev 0x%08x at 0x%08lx irq %d (%pM)\n",
> --
> 2.7.4
>

Apologies for the spam.
This is the v3. Please do let me know in case you still see any corruption.

Regards,
Harini

Re: [PATCH v2] net: macb: Handle HRESP error

[Harini Katakam]

Hi David,

On Fri, Jan 26, 2018 at 9:25 PM, David Miller <da...@davemloft.net> wrote:
> From: Harini Katakam <harini.kata...@xilinx.com>
> Date: Fri, 26 Jan 2018 16:12:11 +0530
>
>> From: Harini Katakam <harini.kata...@xilinx.com>
>>
>> Handle HRESP error by doing a SW reset of RX and TX and
>> re-initializing the descriptors, RX and TX queue pointers.
>>
>> Signed-off-by: Harini Katakam <hari...@xilinx.com>
>> Signed-off-by: Michal Simek <michal.si...@xilinx.com>
>> ---
>> v2:
>> Rebased on top of latest net-next and reinitialized
>> all rx queues.
>
> Your patch was corrupted by your email client, it changed TAB characters
> into sequences of SPACES amongst other things.  We cannot integrate your
> changes until you fix this.

I'll fix this.

>
>> This email and any attachments are intended for the sole use of the named 
>> recipient(s) and contain(s) confidential information that may be 
>> proprietary, privileged or copyrighted under applicable law. If you are not 
>> the intended recipient, do not read, copy, or forward this email message or 
>> any attachments. Delete this email message and any attachments immediately.
>
> This is also completely inappropriate for a public development list and
> you must eliminate this signature on future postings or we will have to
> ignore them.

Sorry, I usually remove this, missed it this time.
Will resend.

Regards,
Harini

Re: [PATCH v2] net: macb: Handle HRESP error

[Harini Katakam]

Hi David,

On Fri, Jan 26, 2018 at 9:25 PM, David Miller  wrote:
> From: Harini Katakam 
> Date: Fri, 26 Jan 2018 16:12:11 +0530
>
>> From: Harini Katakam 
>>
>> Handle HRESP error by doing a SW reset of RX and TX and
>> re-initializing the descriptors, RX and TX queue pointers.
>>
>> Signed-off-by: Harini Katakam 
>> Signed-off-by: Michal Simek 
>> ---
>> v2:
>> Rebased on top of latest net-next and reinitialized
>> all rx queues.
>
> Your patch was corrupted by your email client, it changed TAB characters
> into sequences of SPACES amongst other things.  We cannot integrate your
> changes until you fix this.

I'll fix this.

>
>> This email and any attachments are intended for the sole use of the named 
>> recipient(s) and contain(s) confidential information that may be 
>> proprietary, privileged or copyrighted under applicable law. If you are not 
>> the intended recipient, do not read, copy, or forward this email message or 
>> any attachments. Delete this email message and any attachments immediately.
>
> This is also completely inappropriate for a public development list and
> you must eliminate this signature on future postings or we will have to
> ignore them.

Sorry, I usually remove this, missed it this time.
Will resend.

Regards,
Harini

[PATCH v2] net: macb: Handle HRESP error

[Harini Katakam]

From: Harini Katakam <harini.kata...@xilinx.com>

Handle HRESP error by doing a SW reset of RX and TX and
re-initializing the descriptors, RX and TX queue pointers.

Signed-off-by: Harini Katakam <hari...@xilinx.com>
Signed-off-by: Michal Simek <michal.si...@xilinx.com>
---
v2:
Rebased on top of latest net-next and reinitialized
all rx queues.

 drivers/net/ethernet/cadence/macb.h  |  3 ++
 drivers/net/ethernet/cadence/macb_main.c | 59 +---
 2 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index c50c5ec..8665982 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 

 #if defined(CONFIG_ARCH_DMA_ADDR_T_64BIT) || defined(CONFIG_MACB_USE_HWSTAMP)
 #define MACB_EXT_DESC
@@ -1200,6 +1201,8 @@ struct macb {
struct ethtool_rx_fs_list rx_fs_list;
spinlock_t rx_fs_lock;
unsigned int max_tuples;
+
+   struct tasklet_struct   hresp_err_tasklet;
 };

 #ifdef CONFIG_MACB_USE_HWSTAMP
diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index 234667e..e84afcf 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1258,6 +1258,57 @@ static int macb_poll(struct napi_struct *napi, int 
budget)
return work_done;
 }

+static void macb_hresp_error_task(unsigned long data)
+{
+   struct macb *bp = (struct macb *)data;
+   struct net_device *dev = bp->dev;
+   struct macb_queue *queue = bp->queues;
+   unsigned int q;
+   u32 ctrl;
+
+   for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
+   queue_writel(queue, IDR, MACB_RX_INT_FLAGS |
+MACB_TX_INT_FLAGS |
+MACB_BIT(HRESP));
+   }
+   ctrl = macb_readl(bp, NCR);
+   ctrl &= ~(MACB_BIT(RE) | MACB_BIT(TE));
+   macb_writel(bp, NCR, ctrl);
+
+   netif_tx_stop_all_queues(dev);
+   netif_carrier_off(dev);
+
+   bp->macbgem_ops.mog_init_rings(bp);
+
+   /* Initialize TX and RX buffers */
+   for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
+   queue_writel(queue, RBQP, lower_32_bits(queue->rx_ring_dma));
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+   if (bp->hw_dma_cap & HW_DMA_CAP_64B)
+   queue_writel(queue, RBQPH,
+upper_32_bits(queue->rx_ring_dma));
+#endif
+   queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+   if (bp->hw_dma_cap & HW_DMA_CAP_64B)
+   queue_writel(queue, TBQPH,
+upper_32_bits(queue->tx_ring_dma));
+#endif
+
+   /* Enable interrupts */
+   queue_writel(queue, IER,
+MACB_RX_INT_FLAGS |
+MACB_TX_INT_FLAGS |
+MACB_BIT(HRESP));
+   }
+
+   ctrl |= MACB_BIT(RE) | MACB_BIT(TE);
+   macb_writel(bp, NCR, ctrl);
+
+   netif_carrier_on(dev);
+   netif_tx_start_all_queues(dev);
+}
+
 static irqreturn_t macb_interrupt(int irq, void *dev_id)
 {
struct macb_queue *queue = dev_id;
@@ -1347,10 +1398,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
}

if (status & MACB_BIT(HRESP)) {
-   /* TODO: Reset the hardware, and maybe move the
-* netdev_err to a lower-priority context as well
-* (work queue?)
-*/
+   tasklet_schedule(>hresp_err_tasklet);
netdev_err(dev, "DMA bus error: HRESP not OK\n");

if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
@@ -3937,6 +3985,9 @@ static int macb_probe(struct platform_device *pdev)
goto err_out_unregister_mdio;
}

+   tasklet_init(>hresp_err_tasklet, macb_hresp_error_task,
+(unsigned long)bp);
+
phy_attached_info(phydev);

netdev_info(dev, "Cadence %s rev 0x%08x at 0x%08lx irq %d (%pM)\n",
--
2.7.4

This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.

[PATCH v2] net: macb: Handle HRESP error

[Harini Katakam]

From: Harini Katakam 

Handle HRESP error by doing a SW reset of RX and TX and
re-initializing the descriptors, RX and TX queue pointers.

Signed-off-by: Harini Katakam 
Signed-off-by: Michal Simek 
---
v2:
Rebased on top of latest net-next and reinitialized
all rx queues.

 drivers/net/ethernet/cadence/macb.h  |  3 ++
 drivers/net/ethernet/cadence/macb_main.c | 59 +---
 2 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index c50c5ec..8665982 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 

 #if defined(CONFIG_ARCH_DMA_ADDR_T_64BIT) || defined(CONFIG_MACB_USE_HWSTAMP)
 #define MACB_EXT_DESC
@@ -1200,6 +1201,8 @@ struct macb {
struct ethtool_rx_fs_list rx_fs_list;
spinlock_t rx_fs_lock;
unsigned int max_tuples;
+
+   struct tasklet_struct   hresp_err_tasklet;
 };

 #ifdef CONFIG_MACB_USE_HWSTAMP
diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index 234667e..e84afcf 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1258,6 +1258,57 @@ static int macb_poll(struct napi_struct *napi, int 
budget)
return work_done;
 }

+static void macb_hresp_error_task(unsigned long data)
+{
+   struct macb *bp = (struct macb *)data;
+   struct net_device *dev = bp->dev;
+   struct macb_queue *queue = bp->queues;
+   unsigned int q;
+   u32 ctrl;
+
+   for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
+   queue_writel(queue, IDR, MACB_RX_INT_FLAGS |
+MACB_TX_INT_FLAGS |
+MACB_BIT(HRESP));
+   }
+   ctrl = macb_readl(bp, NCR);
+   ctrl &= ~(MACB_BIT(RE) | MACB_BIT(TE));
+   macb_writel(bp, NCR, ctrl);
+
+   netif_tx_stop_all_queues(dev);
+   netif_carrier_off(dev);
+
+   bp->macbgem_ops.mog_init_rings(bp);
+
+   /* Initialize TX and RX buffers */
+   for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
+   queue_writel(queue, RBQP, lower_32_bits(queue->rx_ring_dma));
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+   if (bp->hw_dma_cap & HW_DMA_CAP_64B)
+   queue_writel(queue, RBQPH,
+upper_32_bits(queue->rx_ring_dma));
+#endif
+   queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+   if (bp->hw_dma_cap & HW_DMA_CAP_64B)
+   queue_writel(queue, TBQPH,
+upper_32_bits(queue->tx_ring_dma));
+#endif
+
+   /* Enable interrupts */
+   queue_writel(queue, IER,
+MACB_RX_INT_FLAGS |
+MACB_TX_INT_FLAGS |
+MACB_BIT(HRESP));
+   }
+
+   ctrl |= MACB_BIT(RE) | MACB_BIT(TE);
+   macb_writel(bp, NCR, ctrl);
+
+   netif_carrier_on(dev);
+   netif_tx_start_all_queues(dev);
+}
+
 static irqreturn_t macb_interrupt(int irq, void *dev_id)
 {
struct macb_queue *queue = dev_id;
@@ -1347,10 +1398,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
}

if (status & MACB_BIT(HRESP)) {
-   /* TODO: Reset the hardware, and maybe move the
-* netdev_err to a lower-priority context as well
-* (work queue?)
-*/
+   tasklet_schedule(>hresp_err_tasklet);
netdev_err(dev, "DMA bus error: HRESP not OK\n");

if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
@@ -3937,6 +3985,9 @@ static int macb_probe(struct platform_device *pdev)
goto err_out_unregister_mdio;
}

+   tasklet_init(>hresp_err_tasklet, macb_hresp_error_task,
+(unsigned long)bp);
+
phy_attached_info(phydev);

netdev_info(dev, "Cadence %s rev 0x%08x at 0x%08lx irq %d (%pM)\n",
--
2.7.4

This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.

[RFC PATCH 2/3] net: macb: Tie-off unused RX queues

[Harini Katakam]

From: "Edgar E. Iglesias" <edgar.igles...@xilinx.com>

Currently, we only use the first receive queue and leave the
remaining DMA descriptor pointers pointing at 0.

Disable unused queues by connecting them to a looped descriptor
chain without free slots.

Signed-off-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
Signed-off-by: Harini Katakam <hari...@xilinx.com>
Signed-off-by: Michal Simek <michal.si...@xilinx.com>
---
 drivers/net/ethernet/cadence/macb.h  |  2 ++
 drivers/net/ethernet/cadence/macb_main.c | 42 
 2 files changed, 44 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index acb6578..974c801 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -994,6 +994,7 @@ struct macb {
unsigned intrx_tail;
unsigned intrx_prepared_head;
struct macb_dma_desc*rx_ring;
+   struct macb_dma_desc*rx_ring_tieoff;
struct sk_buff  **rx_skbuff;
void*rx_buffers;
size_t  rx_buffer_size;
@@ -1019,6 +1020,7 @@ struct macb {
}   hw_stats;
 
dma_addr_t  rx_ring_dma;
+   dma_addr_t  rx_ring_tieoff_dma;
dma_addr_t  rx_buffers_dma;
 
struct macb_or_gem_ops  macbgem_ops;
diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index 623ae9c..b14a04d 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1755,6 +1755,12 @@ static void macb_free_consistent(struct macb *bp)
bp->rx_ring = NULL;
}
 
+   if (bp->rx_ring_tieoff) {
+   dma_free_coherent(>pdev->dev, sizeof(bp->rx_ring_tieoff[0]),
+ bp->rx_ring_tieoff, bp->rx_ring_tieoff_dma);
+   bp->rx_ring_tieoff = NULL;
+   }
+
for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
kfree(queue->tx_skb);
queue->tx_skb = NULL;
@@ -1826,6 +1832,19 @@ static int macb_alloc_consistent(struct macb *bp)
 >rx_ring_dma, GFP_KERNEL);
if (!bp->rx_ring)
goto out_err;
+
+   /* If we have more than one queue, allocate a tie off descriptor
+* that will be used to disable unused RX queues.
+*/
+   if (bp->num_queues > 1) {
+   bp->rx_ring_tieoff = dma_alloc_coherent(>pdev->dev,
+   sizeof(bp->rx_ring_tieoff[0]),
+   >rx_ring_tieoff_dma,
+   GFP_KERNEL);
+   if (!bp->rx_ring_tieoff)
+   goto out_err;
+   }
+
netdev_dbg(bp->dev,
   "Allocated RX ring of %d bytes at %08lx (mapped %p)\n",
   size, (unsigned long)bp->rx_ring_dma, bp->rx_ring);
@@ -1840,6 +1859,19 @@ static int macb_alloc_consistent(struct macb *bp)
return -ENOMEM;
 }
 
+static void macb_init_tieoff(struct macb *bp)
+{
+   struct macb_dma_desc *d = bp->rx_ring_tieoff;
+
+   if (bp->num_queues > 1) {
+   /* Setup a wrapping descriptor with no free slots
+* (WRAP and USED) to tie off/disable unused RX queues.
+*/
+   d->addr = MACB_BIT(RX_WRAP) | MACB_BIT(RX_USED);
+   d->ctrl = 0;
+   }
+}
+
 static void gem_init_rings(struct macb *bp)
 {
struct macb_queue *queue;
@@ -1862,6 +1894,7 @@ static void gem_init_rings(struct macb *bp)
bp->rx_prepared_head = 0;
 
gem_rx_refill(bp);
+   macb_init_tieoff(bp);
 }
 
 static void macb_init_rings(struct macb *bp)
@@ -1879,6 +1912,7 @@ static void macb_init_rings(struct macb *bp)
bp->queues[0].tx_head = 0;
bp->queues[0].tx_tail = 0;
desc->ctrl |= MACB_BIT(TX_WRAP);
+   macb_init_tieoff(bp);
 }
 
 static void macb_reset_hw(struct macb *bp)
@@ -2063,6 +2097,14 @@ static void macb_init_hw(struct macb *bp)
queue_writel(queue, TBQPH, 
upper_32_bits(queue->tx_ring_dma));
 #endif
 
+   /* We only use the first queue at the moment. Remaining
+* queues must be tied-off before we enable the receiver.
+*
+* See the documentation for receive_q1_ptr for more info.
+*/
+   if (q)
+   queue_writel(queue, RBQP, bp->rx_ring_tieoff_dma);
+
/* Enable interrupts */
queue_writel(queue, IER,
 MACB_RX_INT_FLAGS |
-- 
2.7.4

[RFC PATCH 2/3] net: macb: Tie-off unused RX queues

[Harini Katakam]

From: "Edgar E. Iglesias" 

Currently, we only use the first receive queue and leave the
remaining DMA descriptor pointers pointing at 0.

Disable unused queues by connecting them to a looped descriptor
chain without free slots.

Signed-off-by: Edgar E. Iglesias 
Signed-off-by: Harini Katakam 
Signed-off-by: Michal Simek 
---
 drivers/net/ethernet/cadence/macb.h  |  2 ++
 drivers/net/ethernet/cadence/macb_main.c | 42 
 2 files changed, 44 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index acb6578..974c801 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -994,6 +994,7 @@ struct macb {
unsigned intrx_tail;
unsigned intrx_prepared_head;
struct macb_dma_desc*rx_ring;
+   struct macb_dma_desc*rx_ring_tieoff;
struct sk_buff  **rx_skbuff;
void*rx_buffers;
size_t  rx_buffer_size;
@@ -1019,6 +1020,7 @@ struct macb {
}   hw_stats;
 
dma_addr_t  rx_ring_dma;
+   dma_addr_t  rx_ring_tieoff_dma;
dma_addr_t  rx_buffers_dma;
 
struct macb_or_gem_ops  macbgem_ops;
diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index 623ae9c..b14a04d 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1755,6 +1755,12 @@ static void macb_free_consistent(struct macb *bp)
bp->rx_ring = NULL;
}
 
+   if (bp->rx_ring_tieoff) {
+   dma_free_coherent(>pdev->dev, sizeof(bp->rx_ring_tieoff[0]),
+ bp->rx_ring_tieoff, bp->rx_ring_tieoff_dma);
+   bp->rx_ring_tieoff = NULL;
+   }
+
for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
kfree(queue->tx_skb);
queue->tx_skb = NULL;
@@ -1826,6 +1832,19 @@ static int macb_alloc_consistent(struct macb *bp)
 >rx_ring_dma, GFP_KERNEL);
if (!bp->rx_ring)
goto out_err;
+
+   /* If we have more than one queue, allocate a tie off descriptor
+* that will be used to disable unused RX queues.
+*/
+   if (bp->num_queues > 1) {
+   bp->rx_ring_tieoff = dma_alloc_coherent(>pdev->dev,
+   sizeof(bp->rx_ring_tieoff[0]),
+   >rx_ring_tieoff_dma,
+   GFP_KERNEL);
+   if (!bp->rx_ring_tieoff)
+   goto out_err;
+   }
+
netdev_dbg(bp->dev,
   "Allocated RX ring of %d bytes at %08lx (mapped %p)\n",
   size, (unsigned long)bp->rx_ring_dma, bp->rx_ring);
@@ -1840,6 +1859,19 @@ static int macb_alloc_consistent(struct macb *bp)
return -ENOMEM;
 }
 
+static void macb_init_tieoff(struct macb *bp)
+{
+   struct macb_dma_desc *d = bp->rx_ring_tieoff;
+
+   if (bp->num_queues > 1) {
+   /* Setup a wrapping descriptor with no free slots
+* (WRAP and USED) to tie off/disable unused RX queues.
+*/
+   d->addr = MACB_BIT(RX_WRAP) | MACB_BIT(RX_USED);
+   d->ctrl = 0;
+   }
+}
+
 static void gem_init_rings(struct macb *bp)
 {
struct macb_queue *queue;
@@ -1862,6 +1894,7 @@ static void gem_init_rings(struct macb *bp)
bp->rx_prepared_head = 0;
 
gem_rx_refill(bp);
+   macb_init_tieoff(bp);
 }
 
 static void macb_init_rings(struct macb *bp)
@@ -1879,6 +1912,7 @@ static void macb_init_rings(struct macb *bp)
bp->queues[0].tx_head = 0;
bp->queues[0].tx_tail = 0;
desc->ctrl |= MACB_BIT(TX_WRAP);
+   macb_init_tieoff(bp);
 }
 
 static void macb_reset_hw(struct macb *bp)
@@ -2063,6 +2097,14 @@ static void macb_init_hw(struct macb *bp)
queue_writel(queue, TBQPH, 
upper_32_bits(queue->tx_ring_dma));
 #endif
 
+   /* We only use the first queue at the moment. Remaining
+* queues must be tied-off before we enable the receiver.
+*
+* See the documentation for receive_q1_ptr for more info.
+*/
+   if (q)
+   queue_writel(queue, RBQP, bp->rx_ring_tieoff_dma);
+
/* Enable interrupts */
queue_writel(queue, IER,
 MACB_RX_INT_FLAGS |
-- 
2.7.4

[RFC PATCH 1/3] net: macb: Add RBQP to the macb queues

[Harini Katakam]

From: "Edgar E. Iglesias" <edgar.igles...@xilinx.com>

Add RX queue pointer to macb queues to make it accessible for the
multiple queues available. Currently the first RX queue is used.

Signed-off-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
Signed-off-by: Harini Katakam <hari...@xilinx.com>
Signed-off-by: Michal Simek <michal.si...@xilinx.com>
---
 drivers/net/ethernet/cadence/macb.h  | 1 +
 drivers/net/ethernet/cadence/macb_main.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index c93f3a2..acb6578 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -968,6 +968,7 @@ struct macb_queue {
unsigned intIMR;
unsigned intTBQP;
unsigned intTBQPH;
+   unsigned intRBQP;
 
unsigned inttx_head, tx_tail;
struct macb_dma_desc*tx_ring;
diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index 72a67f7..623ae9c 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2875,6 +2875,7 @@ static int macb_init(struct platform_device *pdev)
if (bp->hw_dma_cap & HW_DMA_CAP_64B)
queue->TBQPH = GEM_TBQPH(hw_q - 1);
 #endif
+   queue->RBQP = GEM_RBQP(hw_q - 1);
} else {
/* queue0 uses legacy registers */
queue->ISR  = MACB_ISR;
@@ -2886,6 +2887,7 @@ static int macb_init(struct platform_device *pdev)
if (bp->hw_dma_cap & HW_DMA_CAP_64B)
queue->TBQPH = MACB_TBQPH;
 #endif
+   queue->RBQP = MACB_RBQP;
}
 
/* get irq: here we use the linux queue index, not the hardware
-- 
2.7.4

[RFC PATCH 3/3] net: macb: Handle HRESP error

[Harini Katakam]

From: Harini Katakam <harini.kata...@xilinx.com>

Handle HRESP error by doing a SW reset of RX and TX and
re-initializing the descriptors, RX and TX queue pointers.

Signed-off-by: Harini Katakam <hari...@xilinx.com>
Signed-off-by: Michal Simek <michal.si...@xilinx.com>
---
 drivers/net/ethernet/cadence/macb.h  |  2 +
 drivers/net/ethernet/cadence/macb_main.c | 65 ++--
 2 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index 974c801..5246ee1 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -1060,6 +1060,8 @@ struct macb {
struct ptp_clock_info ptp_clock_info;
struct tsu_incr tsu_incr;
struct hwtstamp_config tstamp_config;
+
+   struct tasklet_struct   hresp_err_tasklet;
 };
 
 #ifdef CONFIG_MACB_USE_HWSTAMP
diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index b14a04d..d76e04f 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1244,6 +1244,63 @@ static int macb_poll(struct napi_struct *napi, int 
budget)
return work_done;
 }
 
+static void macb_hresp_error_task(unsigned long data)
+{
+   struct macb *bp = (struct macb *)data;
+   struct net_device *dev = bp->dev;
+   struct macb_queue *queue = bp->queues;
+   unsigned int q;
+   u32 ctrl;
+
+   for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
+   queue_writel(queue, IDR, MACB_RX_INT_FLAGS |
+MACB_TX_INT_FLAGS |
+MACB_BIT(HRESP));
+   }
+   ctrl = macb_readl(bp, NCR);
+   ctrl &= ~(MACB_BIT(RE) | MACB_BIT(TE));
+   macb_writel(bp, NCR, ctrl);
+
+   netif_tx_stop_all_queues(dev);
+   netif_carrier_off(dev);
+
+   bp->macbgem_ops.mog_init_rings(bp);
+
+   /* Initialize TX and RX buffers */
+   macb_writel(bp, RBQP, lower_32_bits(bp->rx_ring_dma));
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+   if (bp->hw_dma_cap & HW_DMA_CAP_64B)
+   macb_writel(bp, RBQPH, upper_32_bits(bp->rx_ring_dma));
+#endif
+   for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
+   queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+   if (bp->hw_dma_cap & HW_DMA_CAP_64B)
+   queue_writel(queue, TBQPH, 
upper_32_bits(queue->tx_ring_dma));
+#endif
+
+   /* We only use the first queue at the moment. Remaining
+* queues must be tied-off before we enable the receiver.
+*
+* See the documentation for receive_q1_ptr for more info.
+*/
+   if (q)
+   queue_writel(queue, RBQP, bp->rx_ring_tieoff_dma);
+
+   /* Enable interrupts */
+   queue_writel(queue, IER,
+MACB_RX_INT_FLAGS |
+MACB_TX_INT_FLAGS |
+MACB_BIT(HRESP));
+   }
+
+   ctrl |= MACB_BIT(RE) | MACB_BIT(TE);
+   macb_writel(bp, NCR, ctrl);
+
+   netif_carrier_on(dev);
+   netif_tx_start_all_queues(dev);
+}
+
 static irqreturn_t macb_interrupt(int irq, void *dev_id)
 {
struct macb_queue *queue = dev_id;
@@ -1333,10 +1390,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
}
 
if (status & MACB_BIT(HRESP)) {
-   /* TODO: Reset the hardware, and maybe move the
-* netdev_err to a lower-priority context as well
-* (work queue?)
-*/
+   tasklet_schedule(>hresp_err_tasklet);
netdev_err(dev, "DMA bus error: HRESP not OK\n");
 
if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
@@ -3586,6 +3640,9 @@ static int macb_probe(struct platform_device *pdev)
goto err_out_unregister_mdio;
}
 
+   tasklet_init(>hresp_err_tasklet, macb_hresp_error_task,
+(unsigned long)bp);
+
phy_attached_info(phydev);
 
netdev_info(dev, "Cadence %s rev 0x%08x at 0x%08lx irq %d (%pM)\n",
-- 
2.7.4

[RFC PATCH 1/3] net: macb: Add RBQP to the macb queues

[Harini Katakam]

From: "Edgar E. Iglesias" 

Add RX queue pointer to macb queues to make it accessible for the
multiple queues available. Currently the first RX queue is used.

Signed-off-by: Edgar E. Iglesias 
Signed-off-by: Harini Katakam 
Signed-off-by: Michal Simek 
---
 drivers/net/ethernet/cadence/macb.h  | 1 +
 drivers/net/ethernet/cadence/macb_main.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index c93f3a2..acb6578 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -968,6 +968,7 @@ struct macb_queue {
unsigned intIMR;
unsigned intTBQP;
unsigned intTBQPH;
+   unsigned intRBQP;
 
unsigned inttx_head, tx_tail;
struct macb_dma_desc*tx_ring;
diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index 72a67f7..623ae9c 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2875,6 +2875,7 @@ static int macb_init(struct platform_device *pdev)
if (bp->hw_dma_cap & HW_DMA_CAP_64B)
queue->TBQPH = GEM_TBQPH(hw_q - 1);
 #endif
+   queue->RBQP = GEM_RBQP(hw_q - 1);
} else {
/* queue0 uses legacy registers */
queue->ISR  = MACB_ISR;
@@ -2886,6 +2887,7 @@ static int macb_init(struct platform_device *pdev)
if (bp->hw_dma_cap & HW_DMA_CAP_64B)
queue->TBQPH = MACB_TBQPH;
 #endif
+   queue->RBQP = MACB_RBQP;
}
 
/* get irq: here we use the linux queue index, not the hardware
-- 
2.7.4

[RFC PATCH 3/3] net: macb: Handle HRESP error

[Harini Katakam]

From: Harini Katakam 

Handle HRESP error by doing a SW reset of RX and TX and
re-initializing the descriptors, RX and TX queue pointers.

Signed-off-by: Harini Katakam 
Signed-off-by: Michal Simek 
---
 drivers/net/ethernet/cadence/macb.h  |  2 +
 drivers/net/ethernet/cadence/macb_main.c | 65 ++--
 2 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index 974c801..5246ee1 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -1060,6 +1060,8 @@ struct macb {
struct ptp_clock_info ptp_clock_info;
struct tsu_incr tsu_incr;
struct hwtstamp_config tstamp_config;
+
+   struct tasklet_struct   hresp_err_tasklet;
 };
 
 #ifdef CONFIG_MACB_USE_HWSTAMP
diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index b14a04d..d76e04f 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1244,6 +1244,63 @@ static int macb_poll(struct napi_struct *napi, int 
budget)
return work_done;
 }
 
+static void macb_hresp_error_task(unsigned long data)
+{
+   struct macb *bp = (struct macb *)data;
+   struct net_device *dev = bp->dev;
+   struct macb_queue *queue = bp->queues;
+   unsigned int q;
+   u32 ctrl;
+
+   for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
+   queue_writel(queue, IDR, MACB_RX_INT_FLAGS |
+MACB_TX_INT_FLAGS |
+MACB_BIT(HRESP));
+   }
+   ctrl = macb_readl(bp, NCR);
+   ctrl &= ~(MACB_BIT(RE) | MACB_BIT(TE));
+   macb_writel(bp, NCR, ctrl);
+
+   netif_tx_stop_all_queues(dev);
+   netif_carrier_off(dev);
+
+   bp->macbgem_ops.mog_init_rings(bp);
+
+   /* Initialize TX and RX buffers */
+   macb_writel(bp, RBQP, lower_32_bits(bp->rx_ring_dma));
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+   if (bp->hw_dma_cap & HW_DMA_CAP_64B)
+   macb_writel(bp, RBQPH, upper_32_bits(bp->rx_ring_dma));
+#endif
+   for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
+   queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+   if (bp->hw_dma_cap & HW_DMA_CAP_64B)
+   queue_writel(queue, TBQPH, 
upper_32_bits(queue->tx_ring_dma));
+#endif
+
+   /* We only use the first queue at the moment. Remaining
+* queues must be tied-off before we enable the receiver.
+*
+* See the documentation for receive_q1_ptr for more info.
+*/
+   if (q)
+   queue_writel(queue, RBQP, bp->rx_ring_tieoff_dma);
+
+   /* Enable interrupts */
+   queue_writel(queue, IER,
+MACB_RX_INT_FLAGS |
+MACB_TX_INT_FLAGS |
+MACB_BIT(HRESP));
+   }
+
+   ctrl |= MACB_BIT(RE) | MACB_BIT(TE);
+   macb_writel(bp, NCR, ctrl);
+
+   netif_carrier_on(dev);
+   netif_tx_start_all_queues(dev);
+}
+
 static irqreturn_t macb_interrupt(int irq, void *dev_id)
 {
struct macb_queue *queue = dev_id;
@@ -1333,10 +1390,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
}
 
if (status & MACB_BIT(HRESP)) {
-   /* TODO: Reset the hardware, and maybe move the
-* netdev_err to a lower-priority context as well
-* (work queue?)
-*/
+   tasklet_schedule(>hresp_err_tasklet);
netdev_err(dev, "DMA bus error: HRESP not OK\n");
 
if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
@@ -3586,6 +3640,9 @@ static int macb_probe(struct platform_device *pdev)
goto err_out_unregister_mdio;
}
 
+   tasklet_init(>hresp_err_tasklet, macb_hresp_error_task,
+(unsigned long)bp);
+
phy_attached_info(phydev);
 
netdev_info(dev, "Cadence %s rev 0x%08x at 0x%08lx irq %d (%pM)\n",
-- 
2.7.4

[RFC PATCH 0/3] Miscellaneous fixes in macb driver

[Harini Katakam]

This series fixes the following:
-> Ties off unused RX queues
-> Handles RX HRESP error

Edgar E. Iglesias (2):
  net: macb: Add RBQP to the macb queues
  net: macb: Tie-off unused RX queues

Harini Katakam (1):
  net: macb: Handle HRESP error

 drivers/net/ethernet/cadence/macb.h  |   5 ++
 drivers/net/ethernet/cadence/macb_main.c | 109 +--
 2 files changed, 110 insertions(+), 4 deletions(-)

-- 
2.7.4

[RFC PATCH 0/3] Miscellaneous fixes in macb driver

[Harini Katakam]

This series fixes the following:
-> Ties off unused RX queues
-> Handles RX HRESP error

Edgar E. Iglesias (2):
  net: macb: Add RBQP to the macb queues
  net: macb: Tie-off unused RX queues

Harini Katakam (1):
  net: macb: Handle HRESP error

 drivers/net/ethernet/cadence/macb.h  |   5 ++
 drivers/net/ethernet/cadence/macb_main.c | 109 +--
 2 files changed, 110 insertions(+), 4 deletions(-)

-- 
2.7.4

Re: [RFC PATCH 1/2] net: macb: Add RBQP to the macb queues

[Harini Katakam]

Hi,

Please ignore this series.
I'm sending another updated one.
Sorry for the inconvenience.

Regards,
Harini

On Mon, Nov 27, 2017 at 12:33 PM, Harini Katakam
<harini.kata...@xilinx.com> wrote:
> From: "Edgar E. Iglesias" <edgar.igles...@xilinx.com>
>
> Add RX queue pointer to macb queues to make it accessible for the
> multiple queues available. Currently the first RX queue is used.
>
> Signed-off-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
> Signed-off-by: Harini Katakam <hari...@xilinx.com>
> Signed-off-by: Michal Simek <michal.si...@xilinx.com>
> ---
>  drivers/net/ethernet/cadence/macb.h  | 1 +
>  drivers/net/ethernet/cadence/macb_main.c | 2 ++
>  2 files changed, 3 insertions(+)
>
> diff --git a/drivers/net/ethernet/cadence/macb.h 
> b/drivers/net/ethernet/cadence/macb.h
> index c93f3a2..acb6578 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -968,6 +968,7 @@ struct macb_queue {
> unsigned intIMR;
> unsigned intTBQP;
> unsigned intTBQPH;
> +   unsigned intRBQP;
>
> unsigned inttx_head, tx_tail;
> struct macb_dma_desc*tx_ring;
> diff --git a/drivers/net/ethernet/cadence/macb_main.c 
> b/drivers/net/ethernet/cadence/macb_main.c
> index 72a67f7..623ae9c 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -2875,6 +2875,7 @@ static int macb_init(struct platform_device *pdev)
> if (bp->hw_dma_cap & HW_DMA_CAP_64B)
> queue->TBQPH = GEM_TBQPH(hw_q - 1);
>  #endif
> +   queue->RBQP = GEM_RBQP(hw_q - 1);
> } else {
> /* queue0 uses legacy registers */
> queue->ISR  = MACB_ISR;
> @@ -2886,6 +2887,7 @@ static int macb_init(struct platform_device *pdev)
> if (bp->hw_dma_cap & HW_DMA_CAP_64B)
> queue->TBQPH = MACB_TBQPH;
>  #endif
> +   queue->RBQP = MACB_RBQP;
> }
>
> /* get irq: here we use the linux queue index, not the 
> hardware
> --
> 2.7.4
>

Re: [RFC PATCH 1/2] net: macb: Add RBQP to the macb queues

[Harini Katakam]

Hi,

Please ignore this series.
I'm sending another updated one.
Sorry for the inconvenience.

Regards,
Harini

On Mon, Nov 27, 2017 at 12:33 PM, Harini Katakam
 wrote:
> From: "Edgar E. Iglesias" 
>
> Add RX queue pointer to macb queues to make it accessible for the
> multiple queues available. Currently the first RX queue is used.
>
> Signed-off-by: Edgar E. Iglesias 
> Signed-off-by: Harini Katakam 
> Signed-off-by: Michal Simek 
> ---
>  drivers/net/ethernet/cadence/macb.h  | 1 +
>  drivers/net/ethernet/cadence/macb_main.c | 2 ++
>  2 files changed, 3 insertions(+)
>
> diff --git a/drivers/net/ethernet/cadence/macb.h 
> b/drivers/net/ethernet/cadence/macb.h
> index c93f3a2..acb6578 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -968,6 +968,7 @@ struct macb_queue {
> unsigned intIMR;
> unsigned intTBQP;
> unsigned intTBQPH;
> +   unsigned intRBQP;
>
> unsigned inttx_head, tx_tail;
> struct macb_dma_desc*tx_ring;
> diff --git a/drivers/net/ethernet/cadence/macb_main.c 
> b/drivers/net/ethernet/cadence/macb_main.c
> index 72a67f7..623ae9c 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -2875,6 +2875,7 @@ static int macb_init(struct platform_device *pdev)
> if (bp->hw_dma_cap & HW_DMA_CAP_64B)
> queue->TBQPH = GEM_TBQPH(hw_q - 1);
>  #endif
> +   queue->RBQP = GEM_RBQP(hw_q - 1);
> } else {
> /* queue0 uses legacy registers */
> queue->ISR  = MACB_ISR;
> @@ -2886,6 +2887,7 @@ static int macb_init(struct platform_device *pdev)
> if (bp->hw_dma_cap & HW_DMA_CAP_64B)
> queue->TBQPH = MACB_TBQPH;
>  #endif
> +   queue->RBQP = MACB_RBQP;
> }
>
> /* get irq: here we use the linux queue index, not the 
> hardware
> --
> 2.7.4
>

[RFC PATCH 2/2] net: macb: Tie-off unused RX queues

[Harini Katakam]

From: "Edgar E. Iglesias" <edgar.igles...@xilinx.com>

Currently, we only use the first receive queue and leave the
remaining DMA descriptor pointers pointing at 0.

Disable unused queues by connecting them to a looped descriptor
chain without free slots.

Signed-off-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
Signed-off-by: Harini Katakam <hari...@xilinx.com>
Signed-off-by: Michal Simek <michal.si...@xilinx.com>
---
 drivers/net/ethernet/cadence/macb.h  |  2 ++
 drivers/net/ethernet/cadence/macb_main.c | 42 
 2 files changed, 44 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index acb6578..974c801 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -994,6 +994,7 @@ struct macb {
unsigned intrx_tail;
unsigned intrx_prepared_head;
struct macb_dma_desc*rx_ring;
+   struct macb_dma_desc*rx_ring_tieoff;
struct sk_buff  **rx_skbuff;
void*rx_buffers;
size_t  rx_buffer_size;
@@ -1019,6 +1020,7 @@ struct macb {
}   hw_stats;
 
dma_addr_t  rx_ring_dma;
+   dma_addr_t  rx_ring_tieoff_dma;
dma_addr_t  rx_buffers_dma;
 
struct macb_or_gem_ops  macbgem_ops;
diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index 623ae9c..b14a04d 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1755,6 +1755,12 @@ static void macb_free_consistent(struct macb *bp)
bp->rx_ring = NULL;
}
 
+   if (bp->rx_ring_tieoff) {
+   dma_free_coherent(>pdev->dev, sizeof(bp->rx_ring_tieoff[0]),
+ bp->rx_ring_tieoff, bp->rx_ring_tieoff_dma);
+   bp->rx_ring_tieoff = NULL;
+   }
+
for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
kfree(queue->tx_skb);
queue->tx_skb = NULL;
@@ -1826,6 +1832,19 @@ static int macb_alloc_consistent(struct macb *bp)
 >rx_ring_dma, GFP_KERNEL);
if (!bp->rx_ring)
goto out_err;
+
+   /* If we have more than one queue, allocate a tie off descriptor
+* that will be used to disable unused RX queues.
+*/
+   if (bp->num_queues > 1) {
+   bp->rx_ring_tieoff = dma_alloc_coherent(>pdev->dev,
+   sizeof(bp->rx_ring_tieoff[0]),
+   >rx_ring_tieoff_dma,
+   GFP_KERNEL);
+   if (!bp->rx_ring_tieoff)
+   goto out_err;
+   }
+
netdev_dbg(bp->dev,
   "Allocated RX ring of %d bytes at %08lx (mapped %p)\n",
   size, (unsigned long)bp->rx_ring_dma, bp->rx_ring);
@@ -1840,6 +1859,19 @@ static int macb_alloc_consistent(struct macb *bp)
return -ENOMEM;
 }
 
+static void macb_init_tieoff(struct macb *bp)
+{
+   struct macb_dma_desc *d = bp->rx_ring_tieoff;
+
+   if (bp->num_queues > 1) {
+   /* Setup a wrapping descriptor with no free slots
+* (WRAP and USED) to tie off/disable unused RX queues.
+*/
+   d->addr = MACB_BIT(RX_WRAP) | MACB_BIT(RX_USED);
+   d->ctrl = 0;
+   }
+}
+
 static void gem_init_rings(struct macb *bp)
 {
struct macb_queue *queue;
@@ -1862,6 +1894,7 @@ static void gem_init_rings(struct macb *bp)
bp->rx_prepared_head = 0;
 
gem_rx_refill(bp);
+   macb_init_tieoff(bp);
 }
 
 static void macb_init_rings(struct macb *bp)
@@ -1879,6 +1912,7 @@ static void macb_init_rings(struct macb *bp)
bp->queues[0].tx_head = 0;
bp->queues[0].tx_tail = 0;
desc->ctrl |= MACB_BIT(TX_WRAP);
+   macb_init_tieoff(bp);
 }
 
 static void macb_reset_hw(struct macb *bp)
@@ -2063,6 +2097,14 @@ static void macb_init_hw(struct macb *bp)
queue_writel(queue, TBQPH, 
upper_32_bits(queue->tx_ring_dma));
 #endif
 
+   /* We only use the first queue at the moment. Remaining
+* queues must be tied-off before we enable the receiver.
+*
+* See the documentation for receive_q1_ptr for more info.
+*/
+   if (q)
+   queue_writel(queue, RBQP, bp->rx_ring_tieoff_dma);
+
/* Enable interrupts */
queue_writel(queue, IER,
 MACB_RX_INT_FLAGS |
-- 
2.7.4

[RFC PATCH 2/2] net: macb: Tie-off unused RX queues

[Harini Katakam]

From: "Edgar E. Iglesias" 

Currently, we only use the first receive queue and leave the
remaining DMA descriptor pointers pointing at 0.

Disable unused queues by connecting them to a looped descriptor
chain without free slots.

Signed-off-by: Edgar E. Iglesias 
Signed-off-by: Harini Katakam 
Signed-off-by: Michal Simek 
---
 drivers/net/ethernet/cadence/macb.h  |  2 ++
 drivers/net/ethernet/cadence/macb_main.c | 42 
 2 files changed, 44 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index acb6578..974c801 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -994,6 +994,7 @@ struct macb {
unsigned intrx_tail;
unsigned intrx_prepared_head;
struct macb_dma_desc*rx_ring;
+   struct macb_dma_desc*rx_ring_tieoff;
struct sk_buff  **rx_skbuff;
void*rx_buffers;
size_t  rx_buffer_size;
@@ -1019,6 +1020,7 @@ struct macb {
}   hw_stats;
 
dma_addr_t  rx_ring_dma;
+   dma_addr_t  rx_ring_tieoff_dma;
dma_addr_t  rx_buffers_dma;
 
struct macb_or_gem_ops  macbgem_ops;
diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index 623ae9c..b14a04d 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1755,6 +1755,12 @@ static void macb_free_consistent(struct macb *bp)
bp->rx_ring = NULL;
}
 
+   if (bp->rx_ring_tieoff) {
+   dma_free_coherent(>pdev->dev, sizeof(bp->rx_ring_tieoff[0]),
+ bp->rx_ring_tieoff, bp->rx_ring_tieoff_dma);
+   bp->rx_ring_tieoff = NULL;
+   }
+
for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
kfree(queue->tx_skb);
queue->tx_skb = NULL;
@@ -1826,6 +1832,19 @@ static int macb_alloc_consistent(struct macb *bp)
 >rx_ring_dma, GFP_KERNEL);
if (!bp->rx_ring)
goto out_err;
+
+   /* If we have more than one queue, allocate a tie off descriptor
+* that will be used to disable unused RX queues.
+*/
+   if (bp->num_queues > 1) {
+   bp->rx_ring_tieoff = dma_alloc_coherent(>pdev->dev,
+   sizeof(bp->rx_ring_tieoff[0]),
+   >rx_ring_tieoff_dma,
+   GFP_KERNEL);
+   if (!bp->rx_ring_tieoff)
+   goto out_err;
+   }
+
netdev_dbg(bp->dev,
   "Allocated RX ring of %d bytes at %08lx (mapped %p)\n",
   size, (unsigned long)bp->rx_ring_dma, bp->rx_ring);
@@ -1840,6 +1859,19 @@ static int macb_alloc_consistent(struct macb *bp)
return -ENOMEM;
 }
 
+static void macb_init_tieoff(struct macb *bp)
+{
+   struct macb_dma_desc *d = bp->rx_ring_tieoff;
+
+   if (bp->num_queues > 1) {
+   /* Setup a wrapping descriptor with no free slots
+* (WRAP and USED) to tie off/disable unused RX queues.
+*/
+   d->addr = MACB_BIT(RX_WRAP) | MACB_BIT(RX_USED);
+   d->ctrl = 0;
+   }
+}
+
 static void gem_init_rings(struct macb *bp)
 {
struct macb_queue *queue;
@@ -1862,6 +1894,7 @@ static void gem_init_rings(struct macb *bp)
bp->rx_prepared_head = 0;
 
gem_rx_refill(bp);
+   macb_init_tieoff(bp);
 }
 
 static void macb_init_rings(struct macb *bp)
@@ -1879,6 +1912,7 @@ static void macb_init_rings(struct macb *bp)
bp->queues[0].tx_head = 0;
bp->queues[0].tx_tail = 0;
desc->ctrl |= MACB_BIT(TX_WRAP);
+   macb_init_tieoff(bp);
 }
 
 static void macb_reset_hw(struct macb *bp)
@@ -2063,6 +2097,14 @@ static void macb_init_hw(struct macb *bp)
queue_writel(queue, TBQPH, 
upper_32_bits(queue->tx_ring_dma));
 #endif
 
+   /* We only use the first queue at the moment. Remaining
+* queues must be tied-off before we enable the receiver.
+*
+* See the documentation for receive_q1_ptr for more info.
+*/
+   if (q)
+   queue_writel(queue, RBQP, bp->rx_ring_tieoff_dma);
+
/* Enable interrupts */
queue_writel(queue, IER,
 MACB_RX_INT_FLAGS |
-- 
2.7.4

[RFC PATCH 1/2] net: macb: Add RBQP to the macb queues

[Harini Katakam]

From: "Edgar E. Iglesias" <edgar.igles...@xilinx.com>

Add RX queue pointer to macb queues to make it accessible for the
multiple queues available. Currently the first RX queue is used.

Signed-off-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
Signed-off-by: Harini Katakam <hari...@xilinx.com>
Signed-off-by: Michal Simek <michal.si...@xilinx.com>
---
 drivers/net/ethernet/cadence/macb.h  | 1 +
 drivers/net/ethernet/cadence/macb_main.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index c93f3a2..acb6578 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -968,6 +968,7 @@ struct macb_queue {
unsigned intIMR;
unsigned intTBQP;
unsigned intTBQPH;
+   unsigned intRBQP;
 
unsigned inttx_head, tx_tail;
struct macb_dma_desc*tx_ring;
diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index 72a67f7..623ae9c 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2875,6 +2875,7 @@ static int macb_init(struct platform_device *pdev)
if (bp->hw_dma_cap & HW_DMA_CAP_64B)
queue->TBQPH = GEM_TBQPH(hw_q - 1);
 #endif
+   queue->RBQP = GEM_RBQP(hw_q - 1);
} else {
/* queue0 uses legacy registers */
queue->ISR  = MACB_ISR;
@@ -2886,6 +2887,7 @@ static int macb_init(struct platform_device *pdev)
if (bp->hw_dma_cap & HW_DMA_CAP_64B)
queue->TBQPH = MACB_TBQPH;
 #endif
+   queue->RBQP = MACB_RBQP;
}
 
/* get irq: here we use the linux queue index, not the hardware
-- 
2.7.4

[RFC PATCH 1/2] net: macb: Add RBQP to the macb queues

[Harini Katakam]

From: "Edgar E. Iglesias" 

Add RX queue pointer to macb queues to make it accessible for the
multiple queues available. Currently the first RX queue is used.

Signed-off-by: Edgar E. Iglesias 
Signed-off-by: Harini Katakam 
Signed-off-by: Michal Simek 
---
 drivers/net/ethernet/cadence/macb.h  | 1 +
 drivers/net/ethernet/cadence/macb_main.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index c93f3a2..acb6578 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -968,6 +968,7 @@ struct macb_queue {
unsigned intIMR;
unsigned intTBQP;
unsigned intTBQPH;
+   unsigned intRBQP;
 
unsigned inttx_head, tx_tail;
struct macb_dma_desc*tx_ring;
diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index 72a67f7..623ae9c 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2875,6 +2875,7 @@ static int macb_init(struct platform_device *pdev)
if (bp->hw_dma_cap & HW_DMA_CAP_64B)
queue->TBQPH = GEM_TBQPH(hw_q - 1);
 #endif
+   queue->RBQP = GEM_RBQP(hw_q - 1);
} else {
/* queue0 uses legacy registers */
queue->ISR  = MACB_ISR;
@@ -2886,6 +2887,7 @@ static int macb_init(struct platform_device *pdev)
if (bp->hw_dma_cap & HW_DMA_CAP_64B)
queue->TBQPH = MACB_TBQPH;
 #endif
+   queue->RBQP = MACB_RBQP;
}
 
/* get irq: here we use the linux queue index, not the hardware
-- 
2.7.4

Re: [PATCH net-next v2] macb: Common code to enable ptp support for MACB/GEM

[Harini Katakam]

Hi Rafal,

On Thu, Jan 26, 2017 at 8:45 PM, Rafal Ozieblo  wrote:
>> -Original Message-
>> From: Andrei Pistirica [mailto:andrei.pistir...@microchip.com]
>> Sent: 19 stycznia 2017 16:56
>> Subject: [PATCH net-next v2] macb: Common code to enable ptp support for 
>> MACB/GEM
>>
>>
>> +static inline bool gem_has_ptp(struct macb *bp)
>> +{
>> + return !!(bp->caps & MACB_CAPS_GEM_HAS_PTP);
>> +}
> Why don't you use hardware capabilities here? Would it be better to read it 
> from hardware instead adding it to many configuration?

If you are referring to TSU bit in DCFG5, then we will be relying on
Cadence IP's information irrespective of the SoC capability
and whether the PTP support was adequate.
I think the capability approach gives better control and
it is not really much to add.

Regards,
Harini

Re: [PATCH net-next v2] macb: Common code to enable ptp support for MACB/GEM

[Harini Katakam]

Hi Rafal,

On Thu, Jan 26, 2017 at 8:45 PM, Rafal Ozieblo  wrote:
>> -Original Message-
>> From: Andrei Pistirica [mailto:andrei.pistir...@microchip.com]
>> Sent: 19 stycznia 2017 16:56
>> Subject: [PATCH net-next v2] macb: Common code to enable ptp support for 
>> MACB/GEM
>>
>>
>> +static inline bool gem_has_ptp(struct macb *bp)
>> +{
>> + return !!(bp->caps & MACB_CAPS_GEM_HAS_PTP);
>> +}
> Why don't you use hardware capabilities here? Would it be better to read it 
> from hardware instead adding it to many configuration?

If you are referring to TSU bit in DCFG5, then we will be relying on
Cadence IP's information irrespective of the SoC capability
and whether the PTP support was adequate.
I think the capability approach gives better control and
it is not really much to add.

Regards,
Harini

Re: [PATCH net-next v2] macb: Common code to enable ptp support for MACB/GEM

[Harini Katakam]

On Wed, Jan 25, 2017 at 2:56 PM, Nicolas Ferre <nicolas.fe...@atmel.com> wrote:
> Le 19/01/2017 à 17:07, Nicolas Ferre a écrit :
>> Le 19/01/2017 à 08:56, Andrei Pistirica a écrit :
>>> This patch does the following:
>>> - MACB/GEM-PTP interface
>>> - registers and bitfields for TSU
>>> - capability flags to enable PTP per platform basis
>>>
>>> Signed-off-by: Andrei Pistirica <andrei.pistir...@microchip.com>
>>
>> Acked-by: Nicolas Ferre <nicolas.fe...@atmel.com>

Reviewed-by: Harini Katakam <hari...@xilinx.com>

>
> Harini or Rafal, do you plan to review this patch and add your
> "Reviewed-by" tags? It can be useful to make this support move forward.

Sure, reviewed and working with it, meant to add tag :)

Regards,
Harini

Re: [PATCH net-next v2] macb: Common code to enable ptp support for MACB/GEM

[Harini Katakam]

On Wed, Jan 25, 2017 at 2:56 PM, Nicolas Ferre  wrote:
> Le 19/01/2017 à 17:07, Nicolas Ferre a écrit :
>> Le 19/01/2017 à 08:56, Andrei Pistirica a écrit :
>>> This patch does the following:
>>> - MACB/GEM-PTP interface
>>> - registers and bitfields for TSU
>>> - capability flags to enable PTP per platform basis
>>>
>>> Signed-off-by: Andrei Pistirica 
>>
>> Acked-by: Nicolas Ferre 

Reviewed-by: Harini Katakam 

>
> Harini or Rafal, do you plan to review this patch and add your
> "Reviewed-by" tags? It can be useful to make this support move forward.

Sure, reviewed and working with it, meant to add tag :)

Regards,
Harini

Re: [PATCH net-next] macb: Common code to enable ptp support for SAMA5Dx platforms.

[Harini Katakam]

On Wed, Jan 18, 2017 at 11:02 PM, Nicolas Ferre  wrote:
> Le 18/01/2017 à 09:57, Andrei Pistirica a écrit :
>> This patch does the following:
>> - add GEM-PTP interface
>> - registers and bitfields for TSU are named according to SAMA5Dx data sheet
>> - PTP support based on platform capability
>
> The $subject will certainly never match reality, sadly "enable ptp
> support for SAMA5Dx platforms". So, you'd better change it.
> (no "." at the end BTW).
>
>> Signed-off-by: Andrei Pistirica 
>> ---
>> This is just the common code for GEM-PTP support. Code is based on the 
>> comments
>> related to the following patch series:
>> - [RFC PATCH net-next v1-to-4 1/2] macb: Add 1588 support in Cadence GEM.
>> - [RFC PATCH net-next v1-to-4 2/2] macb: Enable 1588 support in SAMA5Dx 
>> platforms.
>>
>> Note: Patch on net-next: January 18.
>>
>> Rafal/Harini, you can continue the work for GME-GXL.

Thanks Andrei.


>
> Otherwise, I'm okay with the rest.
>
> I suggest to people that will keep the ball rolling on this topic to
> take advantage of the chunks of code that Andrei developed with the help
> of Richard and the best practices discussed. I think particularly, if it
> makes sense with HW, about:
> - gem_ptp_do_[rt]xstamp(bp, skb) dereference scheme
> - gem_ptp_adjfine() rationale
> - gem_get_ptp_peer() if needed
>

Sure, will patch on top of this.

Regards,
Harini

> Regards,
> --
> Nicolas Ferre

Re: [PATCH net-next] macb: Common code to enable ptp support for SAMA5Dx platforms.

[Harini Katakam]

On Wed, Jan 18, 2017 at 11:02 PM, Nicolas Ferre  wrote:
> Le 18/01/2017 à 09:57, Andrei Pistirica a écrit :
>> This patch does the following:
>> - add GEM-PTP interface
>> - registers and bitfields for TSU are named according to SAMA5Dx data sheet
>> - PTP support based on platform capability
>
> The $subject will certainly never match reality, sadly "enable ptp
> support for SAMA5Dx platforms". So, you'd better change it.
> (no "." at the end BTW).
>
>> Signed-off-by: Andrei Pistirica 
>> ---
>> This is just the common code for GEM-PTP support. Code is based on the 
>> comments
>> related to the following patch series:
>> - [RFC PATCH net-next v1-to-4 1/2] macb: Add 1588 support in Cadence GEM.
>> - [RFC PATCH net-next v1-to-4 2/2] macb: Enable 1588 support in SAMA5Dx 
>> platforms.
>>
>> Note: Patch on net-next: January 18.
>>
>> Rafal/Harini, you can continue the work for GME-GXL.

Thanks Andrei.


>
> Otherwise, I'm okay with the rest.
>
> I suggest to people that will keep the ball rolling on this topic to
> take advantage of the chunks of code that Andrei developed with the help
> of Richard and the best practices discussed. I think particularly, if it
> makes sense with HW, about:
> - gem_ptp_do_[rt]xstamp(bp, skb) dereference scheme
> - gem_ptp_adjfine() rationale
> - gem_get_ptp_peer() if needed
>

Sure, will patch on top of this.

Regards,
Harini

> Regards,
> --
> Nicolas Ferre

Re: [RFC PATCH net-next v4 1/2] macb: Add 1588 support in Cadence GEM.

[Harini Katakam]

Hi Richard,

On Tue, Jan 3, 2017 at 3:59 PM, Richard Cochran
<richardcoch...@gmail.com> wrote:
> On Tue, Jan 03, 2017 at 10:36:11AM +0530, Harini Katakam wrote:
>> I understand that it is not accurate - it is an initial version.
>
> Why do you say, "it is an initial version?"
>
> The Atmel device has this IP core burned in.  The core is hopelessly
> broken, and it cannot be fixed in SW either, so what is your point?
>

I'm sorry - I just meant that this was before many necessary
enhancements and fixes.
Newer SoCs including ZynqMP (for which the original series was sent)
have the descriptor based approach which is reliable.

Regards,
Harini

Re: [RFC PATCH net-next v4 1/2] macb: Add 1588 support in Cadence GEM.

[Harini Katakam]

Hi Richard,

On Tue, Jan 3, 2017 at 3:59 PM, Richard Cochran
 wrote:
> On Tue, Jan 03, 2017 at 10:36:11AM +0530, Harini Katakam wrote:
>> I understand that it is not accurate - it is an initial version.
>
> Why do you say, "it is an initial version?"
>
> The Atmel device has this IP core burned in.  The core is hopelessly
> broken, and it cannot be fixed in SW either, so what is your point?
>

I'm sorry - I just meant that this was before many necessary
enhancements and fixes.
Newer SoCs including ZynqMP (for which the original series was sent)
have the descriptor based approach which is reliable.

Regards,
Harini

Re: [RFC PATCH net-next v4 1/2] macb: Add 1588 support in Cadence GEM.

[Harini Katakam]

Hi Richard,

On Mon, Jan 2, 2017 at 9:43 PM, Richard Cochran
 wrote:
> On Mon, Jan 02, 2017 at 03:47:07PM +0100, Nicolas Ferre wrote:
>> Le 02/01/2017 à 12:31, Richard Cochran a écrit :
>> > This Cadence IP core is a complete disaster.
>>
>> Well, it evolved and propose several options to different SoC
>> integrators. This is not something unusual...
>> I suspect as well that some other network adapters have the same
>> weakness concerning PTP timestamp in single register as the early
>> revisions of this IP.
>
> It appears that this core can neither latch the time on read or write,
> or even latch time stamps.  I have worked with many different PTP HW
> implementations, even early ones like on the ixp4xx, and it is no
> exaggeration to say that this one is uniquely broken.
>
>> I suspect that Rafal tend to jump too quickly to the latest IP revisions
>> and add more options to this series: let's not try to pour too much
>> things into this code right now.
>
> Why can't you check the IP version in the driver?

There is an IP revision register but it would be probably be better
to rely on "caps" from the compatibility strings - to cover SoC
specific implementations. Also, when this extended BD is
added (with timestamp), additional words will need to be added
statically which will be consistent with Andrei's CONFIG_
checks.

>
> And is it really true that the registers don't latch the time stamps,
> as Rafal said?  If so, then we cannot accept the non-descriptor driver
> version, since it cannot possibly work correctly.
>

AFAIK, the two sets of registers only hold the timestamp till the next
event (or peer event) packet comes in.
I understand that it is not accurate - it is an initial version.

Regards,
Harini

Re: [RFC PATCH net-next v4 1/2] macb: Add 1588 support in Cadence GEM.

[Harini Katakam]

Hi Richard,

On Mon, Jan 2, 2017 at 9:43 PM, Richard Cochran
 wrote:
> On Mon, Jan 02, 2017 at 03:47:07PM +0100, Nicolas Ferre wrote:
>> Le 02/01/2017 à 12:31, Richard Cochran a écrit :
>> > This Cadence IP core is a complete disaster.
>>
>> Well, it evolved and propose several options to different SoC
>> integrators. This is not something unusual...
>> I suspect as well that some other network adapters have the same
>> weakness concerning PTP timestamp in single register as the early
>> revisions of this IP.
>
> It appears that this core can neither latch the time on read or write,
> or even latch time stamps.  I have worked with many different PTP HW
> implementations, even early ones like on the ixp4xx, and it is no
> exaggeration to say that this one is uniquely broken.
>
>> I suspect that Rafal tend to jump too quickly to the latest IP revisions
>> and add more options to this series: let's not try to pour too much
>> things into this code right now.
>
> Why can't you check the IP version in the driver?

There is an IP revision register but it would be probably be better
to rely on "caps" from the compatibility strings - to cover SoC
specific implementations. Also, when this extended BD is
added (with timestamp), additional words will need to be added
statically which will be consistent with Andrei's CONFIG_
checks.

>
> And is it really true that the registers don't latch the time stamps,
> as Rafal said?  If so, then we cannot accept the non-descriptor driver
> version, since it cannot possibly work correctly.
>

AFAIK, the two sets of registers only hold the timestamp till the next
event (or peer event) packet comes in.
I understand that it is not accurate - it is an initial version.

Regards,
Harini

Re: [RFC PATCH net-next v4 1/2] macb: Add 1588 support in Cadence GEM.

[Harini Katakam]

Hi Richard,

On Mon, Jan 2, 2017 at 5:01 PM, Richard Cochran
 wrote:
> On Mon, Jan 02, 2017 at 09:36:10AM +, Rafal Ozieblo wrote:
>> According Cadence Hardware team:
>> "It is just that some customers prefer to have the time in the descriptors 
>> as that is provided per frame.
>> The registers are simply overwritten when a new event frame is 
>> transmitted/received and so software could miss it."
>> The question is are you sure that you read timestamp for current frame? (not 
>> for the next frame).
>
> AFAICT, having the time stamp in the descriptor is not universally
> supported.  Looking at the Xilinx Zynq 7000 TRM, I can't find any
> mention of this.
>
> This Cadence IP core is a complete disaster.
>
> Unless someone can tell us how this IP works in all of its
> incarnations, this series is going nowhere.

>From the revision history of Cadence spec, all versions starting
r1p02 have ability to include timestamp in descriptors.
For previous versions the event register is the only option.
But yes, there have been multiple enhancements and
bug fixes in this IP w.r.t PTP making each implementation
different.

Regards,
Harini

Re: [RFC PATCH net-next v4 1/2] macb: Add 1588 support in Cadence GEM.

[Harini Katakam]

Hi Richard,

On Mon, Jan 2, 2017 at 5:01 PM, Richard Cochran
 wrote:
> On Mon, Jan 02, 2017 at 09:36:10AM +, Rafal Ozieblo wrote:
>> According Cadence Hardware team:
>> "It is just that some customers prefer to have the time in the descriptors 
>> as that is provided per frame.
>> The registers are simply overwritten when a new event frame is 
>> transmitted/received and so software could miss it."
>> The question is are you sure that you read timestamp for current frame? (not 
>> for the next frame).
>
> AFAICT, having the time stamp in the descriptor is not universally
> supported.  Looking at the Xilinx Zynq 7000 TRM, I can't find any
> mention of this.
>
> This Cadence IP core is a complete disaster.
>
> Unless someone can tell us how this IP works in all of its
> incarnations, this series is going nowhere.

>From the revision history of Cadence spec, all versions starting
r1p02 have ability to include timestamp in descriptors.
For previous versions the event register is the only option.
But yes, there have been multiple enhancements and
bug fixes in this IP w.r.t PTP making each implementation
different.

Regards,
Harini

Re: [RFC PATCH net-next v3 1/2] macb: Add 1588 support in Cadence GEM.

[Harini Katakam]

Hi Andrei,

On Mon, Dec 12, 2016 at 3:52 PM,   wrote:
>
>
>> -Original Message-
>> From: Rafal Ozieblo [mailto:raf...@cadence.com]
>> Sent: Friday, December 09, 2016 11:20 AM
>> To: Andrei Pistirica - M16132; richardcoch...@gmail.com
>> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
>> ker...@lists.infradead.org; da...@davemloft.net;
>> nicolas.fe...@atmel.com; harinikatakamli...@gmail.com;
>> harini.kata...@xilinx.com; punn...@xilinx.com; mich...@xilinx.com;
>> anir...@xilinx.com; boris.brezil...@free-electrons.com;
>> alexandre.bell...@free-electrons.com; tbul...@pixelsurmer.com
>> Subject: RE: [RFC PATCH net-next v3 1/2] macb: Add 1588 support in Cadence
>> GEM.
>>
>> -Original Message-
>> > From: andrei.pistir...@microchip.com
>> > [mailto:andrei.pistir...@microchip.com]
>> > Sent: 8 grudnia 2016 15:42
>> > To: richardcoch...@gmail.com
>> > Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org;
>> > linux-arm-ker...@lists.infradead.org; da...@davemloft.net;
>> > nicolas.fe...@atmel.com; harinikatakamli...@gmail.com;
>> > harini.kata...@xilinx.com; punn...@xilinx.com; mich...@xilinx.com;
>> > anir...@xilinx.com; boris.brezil...@free-electrons.com;
>> > alexandre.bell...@free-electrons.com; tbul...@pixelsurmer.com; Rafal
>> > Ozieblo
>> > Subject: RE: [RFC PATCH net-next v3 1/2] macb: Add 1588 support in
>> Cadence GEM.
>> >
>> >
>> >
>> > > -Original Message-
>> > > From: Richard Cochran [mailto:richardcoch...@gmail.com]
>> > > Sent: Wednesday, December 07, 2016 11:04 PM
>> > > To: Andrei Pistirica - M16132
>> > > Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
>> > > ker...@lists.infradead.org; da...@davemloft.net;
>> > > nicolas.fe...@atmel.com; harinikatakamli...@gmail.com;
>> > > harini.kata...@xilinx.com; punn...@xilinx.com; mich...@xilinx.com;
>> > > anir...@xilinx.com; boris.brezil...@free-electrons.com;
>> > > alexandre.bell...@free-electrons.com; tbul...@pixelsurmer.com;
>> > > raf...@cadence.com
>> > > Subject: Re: [RFC PATCH net-next v3 1/2] macb: Add 1588 support in
>> > > Cadence GEM.
>> > >
>> > > On Wed, Dec 07, 2016 at 08:39:09PM +0100, Richard Cochran wrote:
>> > > > > +static s32 gem_ptp_max_adj(unsigned int f_nom) {
>> > > > > +   u64 adj;
>> > > > > +
>> > > > > +   /* The 48 bits of seconds for the GEM overflows every:
>> > > > > +* 2^48/(365.25 * 24 * 60 *60) =~ 8 925 512 years (~= 9 mil 
>> > > > > years),
>> > > > > +* thus the maximum adjust frequency must not overflow
>> > > > > + CNS
>> > > register:
>> > > > > +*
>> > > > > +* addend  = 10^9/nominal_freq
>> > > > > +* adj_max = +/- addend*ppb_max/10^9
>> > > > > +* max_ppb = (2^8-1)*nominal_freq-10^9
>> > > > > +*/
>> > > > > +   adj = f_nom;
>> > > > > +   adj *= 0x;
>> > > > > +   adj -= 10ULL;
>> > > >
>> > > > What is this computation, and how does it relate to the comment?
>> >
>> > I considered the following simple equation: increment value at nominal
>> frequency (which is 10^9/nominal frequency nsecs) + the maximum drift
>> value (nsecs) <= maximum increment value at nominal frequency (which is
>> 8bit:0x).
>> > If maximum drift is written as function of nominal frequency and
>> maximum ppb, then the equation above yields that the maximum ppb is:
>> (2^8 - 1) *nominal_frequency - 10^9. The equation is also simplified by the
>> fact that the drift is written as ppm + 16bit_fractions and the increment
>> value is written as nsec + 16bit_fractions.
>> >
>> > Rafal said that this value is hardcoded: 0x64E6, while Harini said:
>> 25000.
>>
>> To clarify a little bit. In my reference code this value (0x64E6) was taken
>> from our legacy code. It was used for testing only. I know it should be
>> change to something more accurate. This is the reason why I asked how did
>> you count it (25000). According to our calculations this value depends
>> on actual set period (incr_ns and incr_sub_ns) and min and max value we
>> can set. The calculation were a little bit intricate, so we decided to leave 
>> it
>> as it was.
>>
>> >
>> > I need to dig into this...
>> >
>> > >
>> > > I am not sure what you meant, but it sounds like you are on the wrong
>> track.
>> > > Let me explain...
>> >
>> > Thanks.
>> >
>> > >
>> > > The max_adj has nothing at all to do with the width of the time register.
>> > > Rather, it should reflect the maximum possible change in the tuning
>> word.
>> > >
>> > > For example, with a nominal 8 ns period, the tuning word is 0x8.
>> > > Looking at running the clock more slowly, the slowest possible word
>> > > is 0x1, meaning a difference of 0x7.  This implies an
>> > > adjustment of
>> > > 0x7/0x8 or 98092 ppb.  Running more quickly, we can
>> > > already have 0x10, twice as fast, or just under 2 billion ppb.
>> > >
>> > > You should consider the extreme cases to determine the most 

Re: [RFC PATCH net-next v3 1/2] macb: Add 1588 support in Cadence GEM.

[Harini Katakam]

Hi Andrei,

On Mon, Dec 12, 2016 at 3:52 PM,   wrote:
>
>
>> -Original Message-
>> From: Rafal Ozieblo [mailto:raf...@cadence.com]
>> Sent: Friday, December 09, 2016 11:20 AM
>> To: Andrei Pistirica - M16132; richardcoch...@gmail.com
>> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
>> ker...@lists.infradead.org; da...@davemloft.net;
>> nicolas.fe...@atmel.com; harinikatakamli...@gmail.com;
>> harini.kata...@xilinx.com; punn...@xilinx.com; mich...@xilinx.com;
>> anir...@xilinx.com; boris.brezil...@free-electrons.com;
>> alexandre.bell...@free-electrons.com; tbul...@pixelsurmer.com
>> Subject: RE: [RFC PATCH net-next v3 1/2] macb: Add 1588 support in Cadence
>> GEM.
>>
>> -Original Message-
>> > From: andrei.pistir...@microchip.com
>> > [mailto:andrei.pistir...@microchip.com]
>> > Sent: 8 grudnia 2016 15:42
>> > To: richardcoch...@gmail.com
>> > Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org;
>> > linux-arm-ker...@lists.infradead.org; da...@davemloft.net;
>> > nicolas.fe...@atmel.com; harinikatakamli...@gmail.com;
>> > harini.kata...@xilinx.com; punn...@xilinx.com; mich...@xilinx.com;
>> > anir...@xilinx.com; boris.brezil...@free-electrons.com;
>> > alexandre.bell...@free-electrons.com; tbul...@pixelsurmer.com; Rafal
>> > Ozieblo
>> > Subject: RE: [RFC PATCH net-next v3 1/2] macb: Add 1588 support in
>> Cadence GEM.
>> >
>> >
>> >
>> > > -Original Message-
>> > > From: Richard Cochran [mailto:richardcoch...@gmail.com]
>> > > Sent: Wednesday, December 07, 2016 11:04 PM
>> > > To: Andrei Pistirica - M16132
>> > > Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
>> > > ker...@lists.infradead.org; da...@davemloft.net;
>> > > nicolas.fe...@atmel.com; harinikatakamli...@gmail.com;
>> > > harini.kata...@xilinx.com; punn...@xilinx.com; mich...@xilinx.com;
>> > > anir...@xilinx.com; boris.brezil...@free-electrons.com;
>> > > alexandre.bell...@free-electrons.com; tbul...@pixelsurmer.com;
>> > > raf...@cadence.com
>> > > Subject: Re: [RFC PATCH net-next v3 1/2] macb: Add 1588 support in
>> > > Cadence GEM.
>> > >
>> > > On Wed, Dec 07, 2016 at 08:39:09PM +0100, Richard Cochran wrote:
>> > > > > +static s32 gem_ptp_max_adj(unsigned int f_nom) {
>> > > > > +   u64 adj;
>> > > > > +
>> > > > > +   /* The 48 bits of seconds for the GEM overflows every:
>> > > > > +* 2^48/(365.25 * 24 * 60 *60) =~ 8 925 512 years (~= 9 mil 
>> > > > > years),
>> > > > > +* thus the maximum adjust frequency must not overflow
>> > > > > + CNS
>> > > register:
>> > > > > +*
>> > > > > +* addend  = 10^9/nominal_freq
>> > > > > +* adj_max = +/- addend*ppb_max/10^9
>> > > > > +* max_ppb = (2^8-1)*nominal_freq-10^9
>> > > > > +*/
>> > > > > +   adj = f_nom;
>> > > > > +   adj *= 0x;
>> > > > > +   adj -= 10ULL;
>> > > >
>> > > > What is this computation, and how does it relate to the comment?
>> >
>> > I considered the following simple equation: increment value at nominal
>> frequency (which is 10^9/nominal frequency nsecs) + the maximum drift
>> value (nsecs) <= maximum increment value at nominal frequency (which is
>> 8bit:0x).
>> > If maximum drift is written as function of nominal frequency and
>> maximum ppb, then the equation above yields that the maximum ppb is:
>> (2^8 - 1) *nominal_frequency - 10^9. The equation is also simplified by the
>> fact that the drift is written as ppm + 16bit_fractions and the increment
>> value is written as nsec + 16bit_fractions.
>> >
>> > Rafal said that this value is hardcoded: 0x64E6, while Harini said:
>> 25000.
>>
>> To clarify a little bit. In my reference code this value (0x64E6) was taken
>> from our legacy code. It was used for testing only. I know it should be
>> change to something more accurate. This is the reason why I asked how did
>> you count it (25000). According to our calculations this value depends
>> on actual set period (incr_ns and incr_sub_ns) and min and max value we
>> can set. The calculation were a little bit intricate, so we decided to leave 
>> it
>> as it was.
>>
>> >
>> > I need to dig into this...
>> >
>> > >
>> > > I am not sure what you meant, but it sounds like you are on the wrong
>> track.
>> > > Let me explain...
>> >
>> > Thanks.
>> >
>> > >
>> > > The max_adj has nothing at all to do with the width of the time register.
>> > > Rather, it should reflect the maximum possible change in the tuning
>> word.
>> > >
>> > > For example, with a nominal 8 ns period, the tuning word is 0x8.
>> > > Looking at running the clock more slowly, the slowest possible word
>> > > is 0x1, meaning a difference of 0x7.  This implies an
>> > > adjustment of
>> > > 0x7/0x8 or 98092 ppb.  Running more quickly, we can
>> > > already have 0x10, twice as fast, or just under 2 billion ppb.
>> > >
>> > > You should consider the extreme cases to determine the most limited
>> > > (smallest) max_adj 

Re: [RFC PATCH net-next v3 1/2] macb: Add 1588 support in Cadence GEM.

[Harini Katakam]

Hi,

On Thu, Dec 8, 2016 at 8:11 PM,   wrote:
>
>
>> -Original Message-
>> From: Richard Cochran [mailto:richardcoch...@gmail.com]
>> Sent: Wednesday, December 07, 2016 11:04 PM
>> To: Andrei Pistirica - M16132
>> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
>> ker...@lists.infradead.org; da...@davemloft.net;
>> nicolas.fe...@atmel.com; harinikatakamli...@gmail.com;
>> harini.kata...@xilinx.com; punn...@xilinx.com; mich...@xilinx.com;
>> anir...@xilinx.com; boris.brezil...@free-electrons.com;
>> alexandre.bell...@free-electrons.com; tbul...@pixelsurmer.com;
>> raf...@cadence.com
>> Subject: Re: [RFC PATCH net-next v3 1/2] macb: Add 1588 support in
>> Cadence GEM.
>>
>> On Wed, Dec 07, 2016 at 08:39:09PM +0100, Richard Cochran wrote:
>> > > +static s32 gem_ptp_max_adj(unsigned int f_nom) {
>> > > + u64 adj;
>> > > +
>> > > + /* The 48 bits of seconds for the GEM overflows every:
>> > > +  * 2^48/(365.25 * 24 * 60 *60) =~ 8 925 512 years (~= 9 mil years),
>> > > +  * thus the maximum adjust frequency must not overflow CNS
>> register:
>> > > +  *
>> > > +  * addend  = 10^9/nominal_freq
>> > > +  * adj_max = +/- addend*ppb_max/10^9
>> > > +  * max_ppb = (2^8-1)*nominal_freq-10^9
>> > > +  */
>> > > + adj = f_nom;
>> > > + adj *= 0x;
>> > > + adj -= 10ULL;
>> >
>> > What is this computation, and how does it relate to the comment?
>
> I considered the following simple equation: increment value at nominal 
> frequency (which is 10^9/nominal frequency nsecs) + the maximum drift value 
> (nsecs) <= maximum increment value at nominal frequency (which is 
> 8bit:0x).
> If maximum drift is written as function of nominal frequency and maximum ppb, 
> then the equation above yields that the maximum ppb is: (2^8 - 1) 
> *nominal_frequency - 10^9. The equation is also simplified by the fact that 
> the drift is written as ppm + 16bit_fractions and the increment value is 
> written as nsec + 16bit_fractions.
>
> Rafal said that this value is hardcoded: 0x64E6, while Harini said: 25000.

@ Andrei, I may have equated max ppb to max tsu frequency allowed on
the system and set that.
That will be wrong.

>
> I need to dig into this...
>
>>
>> I am not sure what you meant, but it sounds like you are on the wrong track.
>> Let me explain...
>
> Thanks.
>
>>
>> The max_adj has nothing at all to do with the width of the time register.
>> Rather, it should reflect the maximum possible change in the tuning word.
>>
>> For example, with a nominal 8 ns period, the tuning word is 0x8.
>> Looking at running the clock more slowly, the slowest possible word is
>> 0x1, meaning a difference of 0x7.  This implies an adjustment of
>> 0x7/0x8 or 98092 ppb.  Running more quickly, we can already
>> have 0x10, twice as fast, or just under 2 billion ppb.
>>
>> You should consider the extreme cases to determine the most limited
>> (smallest) max_adj value:
>>
>> Case 1 - high frequency
>> ~~~
>>
>> With a nominal 1 ns period, we have the nominal tuning word 0x1.
>> The smallest is 0x1 for a difference of 0x.  This corresponds to an
>> adjustment of 0x/0x1 = .847412109375 or 84741 ppb.
>>
>> Case 2 - low frequency
>> ~~
>>
>> With a nominal 255 ns period, the nominal word is 0xFF, the largest
>> 0xFF, and the difference is 0x.  This corresponds to and adjustment
>> of 0x/0xFF = .0039215087890625 or 3921508 ppb.
>>
>> Since 3921508 ppb is a huge adjustment, you can simply use that as a safe
>> maximum, ignoring the actual input clock.
>>

Thanks Richard.
So, if I understand right, this is theoretically limited by the
maximum input clock:
So if the highest frequency allowed (also commonly sourced in my case)
is 200MHz,
then with a 5ns time period, considering the adjustment to slowest
possible word,
0x4/0x5 will be 96948 ppb.
Shouldn't this be the max_adj?
I'm afraid I don't get why we are choosing the most limited max adj..
Sorry if I'm missing something - could you please help me understand?

Regards,
Harini

Re: [RFC PATCH net-next v3 1/2] macb: Add 1588 support in Cadence GEM.

[Harini Katakam]

Hi,

On Thu, Dec 8, 2016 at 8:11 PM,   wrote:
>
>
>> -Original Message-
>> From: Richard Cochran [mailto:richardcoch...@gmail.com]
>> Sent: Wednesday, December 07, 2016 11:04 PM
>> To: Andrei Pistirica - M16132
>> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
>> ker...@lists.infradead.org; da...@davemloft.net;
>> nicolas.fe...@atmel.com; harinikatakamli...@gmail.com;
>> harini.kata...@xilinx.com; punn...@xilinx.com; mich...@xilinx.com;
>> anir...@xilinx.com; boris.brezil...@free-electrons.com;
>> alexandre.bell...@free-electrons.com; tbul...@pixelsurmer.com;
>> raf...@cadence.com
>> Subject: Re: [RFC PATCH net-next v3 1/2] macb: Add 1588 support in
>> Cadence GEM.
>>
>> On Wed, Dec 07, 2016 at 08:39:09PM +0100, Richard Cochran wrote:
>> > > +static s32 gem_ptp_max_adj(unsigned int f_nom) {
>> > > + u64 adj;
>> > > +
>> > > + /* The 48 bits of seconds for the GEM overflows every:
>> > > +  * 2^48/(365.25 * 24 * 60 *60) =~ 8 925 512 years (~= 9 mil years),
>> > > +  * thus the maximum adjust frequency must not overflow CNS
>> register:
>> > > +  *
>> > > +  * addend  = 10^9/nominal_freq
>> > > +  * adj_max = +/- addend*ppb_max/10^9
>> > > +  * max_ppb = (2^8-1)*nominal_freq-10^9
>> > > +  */
>> > > + adj = f_nom;
>> > > + adj *= 0x;
>> > > + adj -= 10ULL;
>> >
>> > What is this computation, and how does it relate to the comment?
>
> I considered the following simple equation: increment value at nominal 
> frequency (which is 10^9/nominal frequency nsecs) + the maximum drift value 
> (nsecs) <= maximum increment value at nominal frequency (which is 
> 8bit:0x).
> If maximum drift is written as function of nominal frequency and maximum ppb, 
> then the equation above yields that the maximum ppb is: (2^8 - 1) 
> *nominal_frequency - 10^9. The equation is also simplified by the fact that 
> the drift is written as ppm + 16bit_fractions and the increment value is 
> written as nsec + 16bit_fractions.
>
> Rafal said that this value is hardcoded: 0x64E6, while Harini said: 25000.

@ Andrei, I may have equated max ppb to max tsu frequency allowed on
the system and set that.
That will be wrong.

>
> I need to dig into this...
>
>>
>> I am not sure what you meant, but it sounds like you are on the wrong track.
>> Let me explain...
>
> Thanks.
>
>>
>> The max_adj has nothing at all to do with the width of the time register.
>> Rather, it should reflect the maximum possible change in the tuning word.
>>
>> For example, with a nominal 8 ns period, the tuning word is 0x8.
>> Looking at running the clock more slowly, the slowest possible word is
>> 0x1, meaning a difference of 0x7.  This implies an adjustment of
>> 0x7/0x8 or 98092 ppb.  Running more quickly, we can already
>> have 0x10, twice as fast, or just under 2 billion ppb.
>>
>> You should consider the extreme cases to determine the most limited
>> (smallest) max_adj value:
>>
>> Case 1 - high frequency
>> ~~~
>>
>> With a nominal 1 ns period, we have the nominal tuning word 0x1.
>> The smallest is 0x1 for a difference of 0x.  This corresponds to an
>> adjustment of 0x/0x1 = .847412109375 or 84741 ppb.
>>
>> Case 2 - low frequency
>> ~~
>>
>> With a nominal 255 ns period, the nominal word is 0xFF, the largest
>> 0xFF, and the difference is 0x.  This corresponds to and adjustment
>> of 0x/0xFF = .0039215087890625 or 3921508 ppb.
>>
>> Since 3921508 ppb is a huge adjustment, you can simply use that as a safe
>> maximum, ignoring the actual input clock.
>>

Thanks Richard.
So, if I understand right, this is theoretically limited by the
maximum input clock:
So if the highest frequency allowed (also commonly sourced in my case)
is 200MHz,
then with a 5ns time period, considering the adjustment to slowest
possible word,
0x4/0x5 will be 96948 ppb.
Shouldn't this be the max_adj?
I'm afraid I don't get why we are choosing the most limited max adj..
Sorry if I'm missing something - could you please help me understand?

Regards,
Harini

Re: [RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings

[Harini Katakam]

Hi Florian,

On Sun, Dec 4, 2016 at 4:10 AM, Florian Fainelli <f.faine...@gmail.com> wrote:
> Le 12/03/16 à 13:35, Rob Herring a écrit :
>> On Mon, Nov 28, 2016 at 03:19:27PM +0530, Harini Katakam wrote:
>>> +- reg: Address and length of the register set of MAC to be used
>>> +- clock-names: Tuple listing input clock names.
>>> +Required elements: 'pclk', 'hclk'
>>> +Optional elements: 'tx_clk'
>>> +- clocks: Phandles to input clocks.
>
> You are also missing mandatory properties:
>
> #address-cells = <1> and #size-cells = <0>
>
> Where is patch 1? Can you make sure you have the same recipient list for
> both patches in this series so we can review both the binding and driver?
>

Thanks for review, I'll update.

I did send the cover letter, patch 1 and 2 to the same recipient list.
I can see them on the mailing list. The first patch is called:
[RFC PATCH 1/2] net: macb: Add MDIO driver for accessing multiple PHY devices
I hope you can find it.

Regards,
Harini

Re: [RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings

[Harini Katakam]

Hi Florian,

On Sun, Dec 4, 2016 at 4:10 AM, Florian Fainelli  wrote:
> Le 12/03/16 à 13:35, Rob Herring a écrit :
>> On Mon, Nov 28, 2016 at 03:19:27PM +0530, Harini Katakam wrote:
>>> +- reg: Address and length of the register set of MAC to be used
>>> +- clock-names: Tuple listing input clock names.
>>> +Required elements: 'pclk', 'hclk'
>>> +Optional elements: 'tx_clk'
>>> +- clocks: Phandles to input clocks.
>
> You are also missing mandatory properties:
>
> #address-cells = <1> and #size-cells = <0>
>
> Where is patch 1? Can you make sure you have the same recipient list for
> both patches in this series so we can review both the binding and driver?
>

Thanks for review, I'll update.

I did send the cover letter, patch 1 and 2 to the same recipient list.
I can see them on the mailing list. The first patch is called:
[RFC PATCH 1/2] net: macb: Add MDIO driver for accessing multiple PHY devices
I hope you can find it.

Regards,
Harini

Re: [RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings

[Harini Katakam]

Hi Rob,


Thanks for the review.
On Sun, Dec 4, 2016 at 3:05 AM, Rob Herring <r...@kernel.org> wrote:
> On Mon, Nov 28, 2016 at 03:19:27PM +0530, Harini Katakam wrote:

>> +Required properties:
>> +- compatible: Should be "cdns,macb-mdio"
>
> Only one version ever? This needs more specific compatible strings.
>

This is part of the Cadence MAC in a way.
I can use revision number from the Cadence spec I was working with.
But it is not necessarily specific that version.

I'll take care of the other comments in the next version.

Regards,
Harini

Re: [RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings

[Harini Katakam]

Hi Rob,


Thanks for the review.
On Sun, Dec 4, 2016 at 3:05 AM, Rob Herring  wrote:
> On Mon, Nov 28, 2016 at 03:19:27PM +0530, Harini Katakam wrote:

>> +Required properties:
>> +- compatible: Should be "cdns,macb-mdio"
>
> Only one version ever? This needs more specific compatible strings.
>

This is part of the Cadence MAC in a way.
I can use revision number from the Cadence spec I was working with.
But it is not necessarily specific that version.

I'll take care of the other comments in the next version.

Regards,
Harini

Re: [PATCH] net: macb: Write only necessary bits in NCR in macb reset

[Harini Katakam]

Hi David,

On Wed, Nov 30, 2016 at 5:35 AM, David Miller <da...@davemloft.net> wrote:
> From: Harini Katakam <harini.kata...@xilinx.com>
> Date: Mon, 28 Nov 2016 14:53:49 +0530
>
>> In macb_reset_hw, use read-modify-write to disable RX and TX.
>> This way exiting settings and reserved bits wont be disturbed.
>> Use the same method for clearing statistics as well.
>>
>> Signed-off-by: Harini Katakam <hari...@xilinx.com>
>
> This doesn't make much sense to me.
>
> Consider the two callers of this function.
>
> macb_init_hw() is going to do a non-masking write to the NCR
> register:
>
> /* Enable TX and RX */
> macb_writel(bp, NCR, MACB_BIT(RE) | MACB_BIT(TE) | MACB_BIT(MPE));
>
> So obviously no other writable fields matter at all for programming
> the chip properly, otherwise macb_init_hw() would "or" in the bits
> after a read of NCR.  But that's not what this code does, it
> writes "RE | TE | MPE" directly.
>
> And the other caller is macb_close() which is shutting down the
> chip so can zero out all the other bits and it can't possibly
> matter, also due to the assertion above about macb_init_hw()
> showing that only the RE, TE, and MPE bits matter for proper
> functioning of the chip.
>
> You haven't shown a issue caused by the way the code works now, so
> this patch isn't fixing a bug.  In fact, the "bit preserving" would
> even be misleading to someone reading the code.  They will ask
> themselves what bits need to be preserved, and as shown above none of
> them need to be.
>
> I'm not applying this, sorry.
>

Thanks for the detailed analysis.
I noticed an issue with respect to management port enable bit
when working on the patch series for macb mdio driver for
multi MAC - multi PHY access via same mdio bus.
MPE bit of the MAC (whose phy management register
is used) was being reset. But that is a separate
series still in review.
You are right, there is no existing bug that this
patch addresses. I will come back to this later if
required.

Regards,
Harini

Re: [PATCH] net: macb: Write only necessary bits in NCR in macb reset

[Harini Katakam]

Hi David,

On Wed, Nov 30, 2016 at 5:35 AM, David Miller  wrote:
> From: Harini Katakam 
> Date: Mon, 28 Nov 2016 14:53:49 +0530
>
>> In macb_reset_hw, use read-modify-write to disable RX and TX.
>> This way exiting settings and reserved bits wont be disturbed.
>> Use the same method for clearing statistics as well.
>>
>> Signed-off-by: Harini Katakam 
>
> This doesn't make much sense to me.
>
> Consider the two callers of this function.
>
> macb_init_hw() is going to do a non-masking write to the NCR
> register:
>
> /* Enable TX and RX */
> macb_writel(bp, NCR, MACB_BIT(RE) | MACB_BIT(TE) | MACB_BIT(MPE));
>
> So obviously no other writable fields matter at all for programming
> the chip properly, otherwise macb_init_hw() would "or" in the bits
> after a read of NCR.  But that's not what this code does, it
> writes "RE | TE | MPE" directly.
>
> And the other caller is macb_close() which is shutting down the
> chip so can zero out all the other bits and it can't possibly
> matter, also due to the assertion above about macb_init_hw()
> showing that only the RE, TE, and MPE bits matter for proper
> functioning of the chip.
>
> You haven't shown a issue caused by the way the code works now, so
> this patch isn't fixing a bug.  In fact, the "bit preserving" would
> even be misleading to someone reading the code.  They will ask
> themselves what bits need to be preserved, and as shown above none of
> them need to be.
>
> I'm not applying this, sorry.
>

Thanks for the detailed analysis.
I noticed an issue with respect to management port enable bit
when working on the patch series for macb mdio driver for
multi MAC - multi PHY access via same mdio bus.
MPE bit of the MAC (whose phy management register
is used) was being reset. But that is a separate
series still in review.
You are right, there is no existing bug that this
patch addresses. I will come back to this later if
required.

Regards,
Harini

[PATCH v2] net: macb: Write only necessary bits in NCR in macb reset

[Harini Katakam]

In macb_reset_hw, use read-modify-write to disable RX and TX.
Existing settings, for ex. management port enable,
are being cleared in the current implementation.
Also certain reserved bits are read only.
Hence it is better to use read-modify-write.
Use the same method for clearing statistics as well.

Signed-off-by: Harini Katakam <hari...@xilinx.com>
---

v2:
Make ctrl type as u32
Improve commit description

---
 drivers/net/ethernet/cadence/macb.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c 
b/drivers/net/ethernet/cadence/macb.c
index 0e489bb..2ce3407 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -1744,14 +1744,18 @@ static void macb_reset_hw(struct macb *bp)
 {
struct macb_queue *queue;
unsigned int q;
+   u32 ctrl;
 
/* Disable RX and TX (XXX: Should we halt the transmission
 * more gracefully?)
 */
-   macb_writel(bp, NCR, 0);
+   ctrl = macb_readl(bp, NCR);
+   ctrl &= ~(MACB_BIT(RE) | MACB_BIT(TE));
+   macb_writel(bp, NCR, ctrl);
 
/* Clear the stats registers (XXX: Update stats first?) */
-   macb_writel(bp, NCR, MACB_BIT(CLRSTAT));
+   ctrl |= MACB_BIT(CLRSTAT);
+   macb_writel(bp, NCR, ctrl);
 
/* Clear all status flags */
macb_writel(bp, TSR, -1);
-- 
2.7.4

[PATCH v2] net: macb: Write only necessary bits in NCR in macb reset

[Harini Katakam]

In macb_reset_hw, use read-modify-write to disable RX and TX.
Existing settings, for ex. management port enable,
are being cleared in the current implementation.
Also certain reserved bits are read only.
Hence it is better to use read-modify-write.
Use the same method for clearing statistics as well.

Signed-off-by: Harini Katakam 
---

v2:
Make ctrl type as u32
Improve commit description

---
 drivers/net/ethernet/cadence/macb.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c 
b/drivers/net/ethernet/cadence/macb.c
index 0e489bb..2ce3407 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -1744,14 +1744,18 @@ static void macb_reset_hw(struct macb *bp)
 {
struct macb_queue *queue;
unsigned int q;
+   u32 ctrl;
 
/* Disable RX and TX (XXX: Should we halt the transmission
 * more gracefully?)
 */
-   macb_writel(bp, NCR, 0);
+   ctrl = macb_readl(bp, NCR);
+   ctrl &= ~(MACB_BIT(RE) | MACB_BIT(TE));
+   macb_writel(bp, NCR, ctrl);
 
/* Clear the stats registers (XXX: Update stats first?) */
-   macb_writel(bp, NCR, MACB_BIT(CLRSTAT));
+   ctrl |= MACB_BIT(CLRSTAT);
+   macb_writel(bp, NCR, ctrl);
 
/* Clear all status flags */
macb_writel(bp, TSR, -1);
-- 
2.7.4

Re: [RFC PATCH 1/2] net: macb: Add MDIO driver for accessing multiple PHY devices

[Harini Katakam]

Hi Andrew,

On Mon, Nov 28, 2016 at 10:03 PM, Andrew Lunn <and...@lunn.ch> wrote:
> On Mon, Nov 28, 2016 at 03:19:14PM +0530, Harini Katakam wrote:
>> This patch is to add support for the hardware with multiple ethernet
>> MAC controllers and a single MDIO bus connected to multiple PHY devices.
>> MDIO lines are connected to any one of the ethernet MAC controllers and
>> all the PHY devices will be accessed using the PHY maintenance interface
>> in that MAC controller. This handling along with PHY functionality is
>> moved to macb_mdio.c
>>
>> Signed-off-by: Punnaiah Choudary Kalluri <punn...@xilinx.com>
>> Signed-off-by: Harini Katakam <hari...@xilinx.com>
>> ---
>>  drivers/net/ethernet/cadence/Makefile|   2 +-
>>  drivers/net/ethernet/cadence/macb.c  | 169 +++-
>>  drivers/net/ethernet/cadence/macb.h  |   2 +
>>  drivers/net/ethernet/cadence/macb_mdio.c | 266 
>> +++
>>  4 files changed, 294 insertions(+), 145 deletions(-)
>>  create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c
>>

>> + bus->irq = devm_kzalloc(>dev, sizeof(int) * PHY_MAX_ADDR,
>> + GFP_KERNEL);
>
> This looks wrong, or at least old. It used to be a pointer to an array,
> but it is now an actual array.

Sorry, this was a mistake.
I changed this after rebase, will update in next version.

>
>> +static const struct of_device_id macb_mdio_dt_ids[] = {
>> + { .compatible = "cdns,macb-mdio" },
>> +
>> +};
>
>
> I've not looked hard enough to know, but can you keep backwards
> compatibility? Won't old device tree's assume the mdio bus is always
> present? Now you need an explicit node otherwise there will not be an
> mdio bus?

Yes, an explicit MDIO bus is required. But I'm not sure
how to maintain backward compatibility (without using this separate
macb_mdio) and have different MACs use the same MDIO bus
with separate PHYs.

Regards,
Harini

Re: [RFC PATCH 1/2] net: macb: Add MDIO driver for accessing multiple PHY devices

[Harini Katakam]

Hi Andrew,

On Mon, Nov 28, 2016 at 10:03 PM, Andrew Lunn  wrote:
> On Mon, Nov 28, 2016 at 03:19:14PM +0530, Harini Katakam wrote:
>> This patch is to add support for the hardware with multiple ethernet
>> MAC controllers and a single MDIO bus connected to multiple PHY devices.
>> MDIO lines are connected to any one of the ethernet MAC controllers and
>> all the PHY devices will be accessed using the PHY maintenance interface
>> in that MAC controller. This handling along with PHY functionality is
>> moved to macb_mdio.c
>>
>> Signed-off-by: Punnaiah Choudary Kalluri 
>> Signed-off-by: Harini Katakam 
>> ---
>>  drivers/net/ethernet/cadence/Makefile|   2 +-
>>  drivers/net/ethernet/cadence/macb.c  | 169 +++-
>>  drivers/net/ethernet/cadence/macb.h  |   2 +
>>  drivers/net/ethernet/cadence/macb_mdio.c | 266 
>> +++
>>  4 files changed, 294 insertions(+), 145 deletions(-)
>>  create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c
>>

>> + bus->irq = devm_kzalloc(>dev, sizeof(int) * PHY_MAX_ADDR,
>> + GFP_KERNEL);
>
> This looks wrong, or at least old. It used to be a pointer to an array,
> but it is now an actual array.

Sorry, this was a mistake.
I changed this after rebase, will update in next version.

>
>> +static const struct of_device_id macb_mdio_dt_ids[] = {
>> + { .compatible = "cdns,macb-mdio" },
>> +
>> +};
>
>
> I've not looked hard enough to know, but can you keep backwards
> compatibility? Won't old device tree's assume the mdio bus is always
> present? Now you need an explicit node otherwise there will not be an
> mdio bus?

Yes, an explicit MDIO bus is required. But I'm not sure
how to maintain backward compatibility (without using this separate
macb_mdio) and have different MACs use the same MDIO bus
with separate PHYs.

Regards,
Harini

[PATCH] net: macb: Write only necessary bits in NCR in macb reset

[Harini Katakam]

In macb_reset_hw, use read-modify-write to disable RX and TX.
This way exiting settings and reserved bits wont be disturbed.
Use the same method for clearing statistics as well.

Signed-off-by: Harini Katakam <hari...@xilinx.com>
---
 drivers/net/ethernet/cadence/macb.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c 
b/drivers/net/ethernet/cadence/macb.c
index 0e489bb..80ccfc4 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -1743,15 +1743,18 @@ static void macb_init_rings(struct macb *bp)
 static void macb_reset_hw(struct macb *bp)
 {
struct macb_queue *queue;
-   unsigned int q;
+   unsigned int q, ctrl;
 
/* Disable RX and TX (XXX: Should we halt the transmission
 * more gracefully?)
 */
-   macb_writel(bp, NCR, 0);
+   ctrl = macb_readl(bp, NCR);
+   ctrl &= ~(MACB_BIT(RE) | MACB_BIT(TE));
+   macb_writel(bp, NCR, ctrl);
 
/* Clear the stats registers (XXX: Update stats first?) */
-   macb_writel(bp, NCR, MACB_BIT(CLRSTAT));
+   ctrl |= MACB_BIT(CLRSTAT);
+   macb_writel(bp, NCR, ctrl);
 
/* Clear all status flags */
macb_writel(bp, TSR, -1);
-- 
2.7.4

[PATCH] net: macb: Write only necessary bits in NCR in macb reset

[Harini Katakam]

In macb_reset_hw, use read-modify-write to disable RX and TX.
This way exiting settings and reserved bits wont be disturbed.
Use the same method for clearing statistics as well.

Signed-off-by: Harini Katakam 
---
 drivers/net/ethernet/cadence/macb.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c 
b/drivers/net/ethernet/cadence/macb.c
index 0e489bb..80ccfc4 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -1743,15 +1743,18 @@ static void macb_init_rings(struct macb *bp)
 static void macb_reset_hw(struct macb *bp)
 {
struct macb_queue *queue;
-   unsigned int q;
+   unsigned int q, ctrl;
 
/* Disable RX and TX (XXX: Should we halt the transmission
 * more gracefully?)
 */
-   macb_writel(bp, NCR, 0);
+   ctrl = macb_readl(bp, NCR);
+   ctrl &= ~(MACB_BIT(RE) | MACB_BIT(TE));
+   macb_writel(bp, NCR, ctrl);
 
/* Clear the stats registers (XXX: Update stats first?) */
-   macb_writel(bp, NCR, MACB_BIT(CLRSTAT));
+   ctrl |= MACB_BIT(CLRSTAT);
+   macb_writel(bp, NCR, ctrl);
 
/* Clear all status flags */
macb_writel(bp, TSR, -1);
-- 
2.7.4

[RFC PATCH 0/2] Multi phy management

[Harini Katakam]

This series is to add support for the hardware with multiple ethernet
MAC controllers and a single MDIO bus connected to multiple PHY devices.
MDIO lines are connected to any one of the ethernet MAC controllers and
all the PHY devices will be accessed using the PHY maintenance interface
in that MAC controller.
This handling along with PHY functionality is moved to macb_mdio.c
within the macb driver space.
This is because of the phy maintenance register is within the MAC.
Please advise on how to proceed with handling such a requirement.

Harini Katakam (2):
  net: macb: Add MDIO driver for accessing multiple PHY devices
  Documentation: devictree: Add macb mdio bindings

 .../devicetree/bindings/net/macb-mdio.txt  |  31 +++
 drivers/net/ethernet/cadence/Makefile  |   2 +-
 drivers/net/ethernet/cadence/macb.c| 169 ++---
 drivers/net/ethernet/cadence/macb.h|   2 +
 drivers/net/ethernet/cadence/macb_mdio.c   | 266 +
 5 files changed, 325 insertions(+), 145 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/macb-mdio.txt
 create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c

-- 
2.7.4

[RFC PATCH 0/2] Multi phy management

[Harini Katakam]

This series is to add support for the hardware with multiple ethernet
MAC controllers and a single MDIO bus connected to multiple PHY devices.
MDIO lines are connected to any one of the ethernet MAC controllers and
all the PHY devices will be accessed using the PHY maintenance interface
in that MAC controller.
This handling along with PHY functionality is moved to macb_mdio.c
within the macb driver space.
This is because of the phy maintenance register is within the MAC.
Please advise on how to proceed with handling such a requirement.

Harini Katakam (2):
  net: macb: Add MDIO driver for accessing multiple PHY devices
  Documentation: devictree: Add macb mdio bindings

 .../devicetree/bindings/net/macb-mdio.txt  |  31 +++
 drivers/net/ethernet/cadence/Makefile  |   2 +-
 drivers/net/ethernet/cadence/macb.c| 169 ++---
 drivers/net/ethernet/cadence/macb.h|   2 +
 drivers/net/ethernet/cadence/macb_mdio.c   | 266 +
 5 files changed, 325 insertions(+), 145 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/macb-mdio.txt
 create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c

-- 
2.7.4

[RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings

[Harini Katakam]

Add documentations for macb mdio driver.

Signed-off-by: Harini Katakam <hari...@xilinx.com>
---
 .../devicetree/bindings/net/macb-mdio.txt  | 31 ++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/macb-mdio.txt

diff --git a/Documentation/devicetree/bindings/net/macb-mdio.txt 
b/Documentation/devicetree/bindings/net/macb-mdio.txt
new file mode 100644
index 000..014cedf
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/macb-mdio.txt
@@ -0,0 +1,31 @@
+* Cadence MACB MDIO controller
+
+Required properties:
+- compatible: Should be "cdns,macb-mdio"
+- reg: Address and length of the register set of MAC to be used
+- clock-names: Tuple listing input clock names.
+   Required elements: 'pclk', 'hclk'
+   Optional elements: 'tx_clk'
+- clocks: Phandles to input clocks.
+
+Examples:
+
+   mdio {
+   compatible = "cdns,macb-mdio";
+   reg = <0x0 0xff0b 0x0 0x1000>;
+   clocks = <>, <>, <>;
+   clock-names = "pclk", "hclk", "tx_clk";
+   ethernet_phyC: ethernet-phy@C {
+   reg = <0xC>;
+   };
+   ethernet_phy7: ethernet-phy@7 {
+   reg = <0x7>;
+   };
+   ethernet_phy3: ethernet-phy@3 {
+   reg = <0x3>;
+   };
+   ethernet_phy8: ethernet-phy@8 {
+   reg = <0x8>;
+   };
+   };
+
-- 
2.7.4

[RFC PATCH 1/2] net: macb: Add MDIO driver for accessing multiple PHY devices

[Harini Katakam]

This patch is to add support for the hardware with multiple ethernet
MAC controllers and a single MDIO bus connected to multiple PHY devices.
MDIO lines are connected to any one of the ethernet MAC controllers and
all the PHY devices will be accessed using the PHY maintenance interface
in that MAC controller. This handling along with PHY functionality is
moved to macb_mdio.c

Signed-off-by: Punnaiah Choudary Kalluri <punn...@xilinx.com>
Signed-off-by: Harini Katakam <hari...@xilinx.com>
---
 drivers/net/ethernet/cadence/Makefile|   2 +-
 drivers/net/ethernet/cadence/macb.c  | 169 +++-
 drivers/net/ethernet/cadence/macb.h  |   2 +
 drivers/net/ethernet/cadence/macb_mdio.c | 266 +++
 4 files changed, 294 insertions(+), 145 deletions(-)
 create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c

diff --git a/drivers/net/ethernet/cadence/Makefile 
b/drivers/net/ethernet/cadence/Makefile
index 91f79b1..75c3d84 100644
--- a/drivers/net/ethernet/cadence/Makefile
+++ b/drivers/net/ethernet/cadence/Makefile
@@ -2,4 +2,4 @@
 # Makefile for the Atmel network device drivers.
 #
 
-obj-$(CONFIG_MACB) += macb.o
+obj-$(CONFIG_MACB) += macb.o macb_mdio.o
diff --git a/drivers/net/ethernet/cadence/macb.c 
b/drivers/net/ethernet/cadence/macb.c
index 80ccfc4..ae2a797 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -232,45 +232,6 @@ static void macb_get_hwaddr(struct macb *bp)
eth_hw_addr_random(bp->dev);
 }
 
-static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
-{
-   struct macb *bp = bus->priv;
-   int value;
-
-   macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
- | MACB_BF(RW, MACB_MAN_READ)
- | MACB_BF(PHYA, mii_id)
- | MACB_BF(REGA, regnum)
- | MACB_BF(CODE, MACB_MAN_CODE)));
-
-   /* wait for end of transfer */
-   while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
-   cpu_relax();
-
-   value = MACB_BFEXT(DATA, macb_readl(bp, MAN));
-
-   return value;
-}
-
-static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
-  u16 value)
-{
-   struct macb *bp = bus->priv;
-
-   macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
- | MACB_BF(RW, MACB_MAN_WRITE)
- | MACB_BF(PHYA, mii_id)
- | MACB_BF(REGA, regnum)
- | MACB_BF(CODE, MACB_MAN_CODE)
- | MACB_BF(DATA, value)));
-
-   /* wait for end of transfer */
-   while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
-   cpu_relax();
-
-   return 0;
-}
-
 /**
  * macb_set_tx_clk() - Set a clock to a new frequency
  * @clkPointer to the clock to change
@@ -385,27 +346,19 @@ static void macb_handle_link_change(struct net_device 
*dev)
 static int macb_mii_probe(struct net_device *dev)
 {
struct macb *bp = netdev_priv(dev);
-   struct macb_platform_data *pdata;
struct phy_device *phydev;
-   int phy_irq;
int ret;
 
-   phydev = phy_find_first(bp->mii_bus);
+   if (dev->phydev)
+   return 0;
+
+   phydev = of_phy_find_device(bp->phy_node);
if (!phydev) {
netdev_err(dev, "no PHY found\n");
return -ENXIO;
}
-
-   pdata = dev_get_platdata(>pdev->dev);
-   if (pdata && gpio_is_valid(pdata->phy_irq_pin)) {
-   ret = devm_gpio_request(>pdev->dev, pdata->phy_irq_pin,
-   "phy int");
-   if (!ret) {
-   phy_irq = gpio_to_irq(pdata->phy_irq_pin);
-   phydev->irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
-   }
-   }
-
+   if (bp->phy_irq)
+   phydev->irq = bp->phy_irq;
/* attach the mac to the phy */
ret = phy_connect_direct(dev, phydev, _handle_link_change,
 bp->phy_interface);
@@ -429,80 +382,9 @@ static int macb_mii_probe(struct net_device *dev)
bp->speed = 0;
bp->duplex = -1;
 
-   return 0;
-}
-
-static int macb_mii_init(struct macb *bp)
-{
-   struct macb_platform_data *pdata;
-   struct device_node *np;
-   int err = -ENXIO, i;
-
-   /* Enable management port */
-   macb_writel(bp, NCR, MACB_BIT(MPE));
-
-   bp->mii_bus = mdiobus_alloc();
-   if (!bp->mii_bus) {
-   err = -ENOMEM;
-   goto err_out;
-   }
-
-   bp->mii_bus->name = "MACB_mii_bus";
-   bp->mii_bus->read = _mdio_read;
-   bp->mii_bus->write = _mdio_write;
-   snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
-

[RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings

[Harini Katakam]

Add documentations for macb mdio driver.

Signed-off-by: Harini Katakam 
---
 .../devicetree/bindings/net/macb-mdio.txt  | 31 ++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/macb-mdio.txt

diff --git a/Documentation/devicetree/bindings/net/macb-mdio.txt 
b/Documentation/devicetree/bindings/net/macb-mdio.txt
new file mode 100644
index 000..014cedf
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/macb-mdio.txt
@@ -0,0 +1,31 @@
+* Cadence MACB MDIO controller
+
+Required properties:
+- compatible: Should be "cdns,macb-mdio"
+- reg: Address and length of the register set of MAC to be used
+- clock-names: Tuple listing input clock names.
+   Required elements: 'pclk', 'hclk'
+   Optional elements: 'tx_clk'
+- clocks: Phandles to input clocks.
+
+Examples:
+
+   mdio {
+   compatible = "cdns,macb-mdio";
+   reg = <0x0 0xff0b 0x0 0x1000>;
+   clocks = <>, <>, <>;
+   clock-names = "pclk", "hclk", "tx_clk";
+   ethernet_phyC: ethernet-phy@C {
+   reg = <0xC>;
+   };
+   ethernet_phy7: ethernet-phy@7 {
+   reg = <0x7>;
+   };
+   ethernet_phy3: ethernet-phy@3 {
+   reg = <0x3>;
+   };
+   ethernet_phy8: ethernet-phy@8 {
+   reg = <0x8>;
+   };
+   };
+
-- 
2.7.4

[RFC PATCH 1/2] net: macb: Add MDIO driver for accessing multiple PHY devices

[Harini Katakam]

This patch is to add support for the hardware with multiple ethernet
MAC controllers and a single MDIO bus connected to multiple PHY devices.
MDIO lines are connected to any one of the ethernet MAC controllers and
all the PHY devices will be accessed using the PHY maintenance interface
in that MAC controller. This handling along with PHY functionality is
moved to macb_mdio.c

Signed-off-by: Punnaiah Choudary Kalluri 
Signed-off-by: Harini Katakam 
---
 drivers/net/ethernet/cadence/Makefile|   2 +-
 drivers/net/ethernet/cadence/macb.c  | 169 +++-
 drivers/net/ethernet/cadence/macb.h  |   2 +
 drivers/net/ethernet/cadence/macb_mdio.c | 266 +++
 4 files changed, 294 insertions(+), 145 deletions(-)
 create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c

diff --git a/drivers/net/ethernet/cadence/Makefile 
b/drivers/net/ethernet/cadence/Makefile
index 91f79b1..75c3d84 100644
--- a/drivers/net/ethernet/cadence/Makefile
+++ b/drivers/net/ethernet/cadence/Makefile
@@ -2,4 +2,4 @@
 # Makefile for the Atmel network device drivers.
 #
 
-obj-$(CONFIG_MACB) += macb.o
+obj-$(CONFIG_MACB) += macb.o macb_mdio.o
diff --git a/drivers/net/ethernet/cadence/macb.c 
b/drivers/net/ethernet/cadence/macb.c
index 80ccfc4..ae2a797 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -232,45 +232,6 @@ static void macb_get_hwaddr(struct macb *bp)
eth_hw_addr_random(bp->dev);
 }
 
-static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
-{
-   struct macb *bp = bus->priv;
-   int value;
-
-   macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
- | MACB_BF(RW, MACB_MAN_READ)
- | MACB_BF(PHYA, mii_id)
- | MACB_BF(REGA, regnum)
- | MACB_BF(CODE, MACB_MAN_CODE)));
-
-   /* wait for end of transfer */
-   while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
-   cpu_relax();
-
-   value = MACB_BFEXT(DATA, macb_readl(bp, MAN));
-
-   return value;
-}
-
-static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
-  u16 value)
-{
-   struct macb *bp = bus->priv;
-
-   macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
- | MACB_BF(RW, MACB_MAN_WRITE)
- | MACB_BF(PHYA, mii_id)
- | MACB_BF(REGA, regnum)
- | MACB_BF(CODE, MACB_MAN_CODE)
- | MACB_BF(DATA, value)));
-
-   /* wait for end of transfer */
-   while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
-   cpu_relax();
-
-   return 0;
-}
-
 /**
  * macb_set_tx_clk() - Set a clock to a new frequency
  * @clkPointer to the clock to change
@@ -385,27 +346,19 @@ static void macb_handle_link_change(struct net_device 
*dev)
 static int macb_mii_probe(struct net_device *dev)
 {
struct macb *bp = netdev_priv(dev);
-   struct macb_platform_data *pdata;
struct phy_device *phydev;
-   int phy_irq;
int ret;
 
-   phydev = phy_find_first(bp->mii_bus);
+   if (dev->phydev)
+   return 0;
+
+   phydev = of_phy_find_device(bp->phy_node);
if (!phydev) {
netdev_err(dev, "no PHY found\n");
return -ENXIO;
}
-
-   pdata = dev_get_platdata(>pdev->dev);
-   if (pdata && gpio_is_valid(pdata->phy_irq_pin)) {
-   ret = devm_gpio_request(>pdev->dev, pdata->phy_irq_pin,
-   "phy int");
-   if (!ret) {
-   phy_irq = gpio_to_irq(pdata->phy_irq_pin);
-   phydev->irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
-   }
-   }
-
+   if (bp->phy_irq)
+   phydev->irq = bp->phy_irq;
/* attach the mac to the phy */
ret = phy_connect_direct(dev, phydev, _handle_link_change,
 bp->phy_interface);
@@ -429,80 +382,9 @@ static int macb_mii_probe(struct net_device *dev)
bp->speed = 0;
bp->duplex = -1;
 
-   return 0;
-}
-
-static int macb_mii_init(struct macb *bp)
-{
-   struct macb_platform_data *pdata;
-   struct device_node *np;
-   int err = -ENXIO, i;
-
-   /* Enable management port */
-   macb_writel(bp, NCR, MACB_BIT(MPE));
-
-   bp->mii_bus = mdiobus_alloc();
-   if (!bp->mii_bus) {
-   err = -ENOMEM;
-   goto err_out;
-   }
-
-   bp->mii_bus->name = "MACB_mii_bus";
-   bp->mii_bus->read = _mdio_read;
-   bp->mii_bus->write = _mdio_write;
-   snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
-bp->pdev->name, bp->pdev-&