From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jose Abreu Subject: Re: [PATCH v2 net-next 5/9] net: stmmac: Add MDIO related functions for XGMAC2 Date: Fri, 3 Aug 2018 16:25:27 +0100 Message-ID: <21eb5877-ebdd-7710-e445-a60cb6e31ad6@synopsys.com> References: <7eba8468b6e69fab66b1bf01f7a2b02b6c81b126.1533307909.git.joabreu@synopsys.com> <20180803152059.GC15029@lunn.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Cc: , "David S. Miller" , "Joao Pinto" , Giuseppe Cavallaro , Alexandre Torgue To: Andrew Lunn , Jose Abreu Return-path: Received: from us01smtprelay-2.synopsys.com ([198.182.47.9]:47124 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727246AbeHCRWV (ORCPT ); Fri, 3 Aug 2018 13:22:21 -0400 In-Reply-To: <20180803152059.GC15029@lunn.ch> Sender: netdev-owner@vger.kernel.org List-ID: Hi Andrew, On 03-08-2018 16:20, Andrew Lunn wrote: > On Fri, Aug 03, 2018 at 03:56:07PM +0100, Jose Abreu wrote: >> Add the MDIO related funcionalities for the new IP block XGMAC2. >> >> Signed-off-by: Jose Abreu >> Cc: David S. Miller >> Cc: Joao Pinto >> Cc: Giuseppe Cavallaro >> Cc: Alexandre Torgue >> Cc: Andrew Lunn >> --- >> Changes from v1: >> - Remove C45 support (Andrew) >> - Add define for bits (Andrew) >> - Remove uneeded cast (Andrew) >> - Use different callbacks instead of if's (Andrew) >> --- >> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 101 +++++++++++++++++++++- >> 1 file changed, 99 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c >> index 5df1a608e566..9bbdb78d3315 100644 >> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c >> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c >> @@ -29,6 +29,7 @@ >> #include >> #include >> >> +#include "dwxgmac2.h" >> #include "stmmac.h" >> >> #define MII_BUSY 0x00000001 >> @@ -39,6 +40,96 @@ >> #define MII_GMAC4_WRITE (1 << MII_GMAC4_GOC_SHIFT) >> #define MII_GMAC4_READ (3 << MII_GMAC4_GOC_SHIFT) >> >> +/* XGMAC defines */ >> +#define MII_XGMAC_SADDR BIT(18) >> +#define MII_XGMAC_CMD_SHIFT 16 >> +#define MII_XGMAC_WRITE (1 << MII_XGMAC_CMD_SHIFT) >> +#define MII_XGMAC_READ (3 << MII_XGMAC_CMD_SHIFT) >> +#define MII_XGMAC_BUSY BIT(22) >> + >> +static int stmmac_xgmac2_mdio_read(struct mii_bus *bus, int phyaddr, int phyreg) >> +{ >> + struct net_device *ndev = bus->priv; >> + struct stmmac_priv *priv = netdev_priv(ndev); >> + unsigned int mii_address = priv->hw->mii.addr; >> + unsigned int mii_data = priv->hw->mii.data; >> + u32 tmp, addr, value = MII_XGMAC_BUSY; >> + >> + if (phyreg & MII_ADDR_C45) { >> + return -EOPNOTSUPP; >> + } else { >> + if (phyaddr >= 4) >> + return -ENODEV; >> + >> + /* Set port as Clause 22 */ >> + tmp = readl(priv->ioaddr + XGMAC_MDIO_C22P); >> + tmp |= BIT(phyaddr); >> + writel(tmp, priv->ioaddr + XGMAC_MDIO_C22P); > Hi Jose > > Maybe put this into a helper? You do repeat it twice. Yes, makes sense. > >> + >> + addr = (phyaddr << 16) | (phyreg & 0x1f); > You could use GENMASK(4, 0) here. That was the point i was trying to > make earlier. But i actually find 0x1f, and 0xffff easier to read. Less typing :D > >> + } >> + >> + value |= (priv->clk_csr << priv->hw->mii.clk_csr_shift) >> + & priv->hw->mii.clk_csr_mask; >> + value |= MII_XGMAC_SADDR | MII_XGMAC_READ; >> + >> + if (readl_poll_timeout(priv->ioaddr + mii_data, tmp, >> + !(tmp & MII_XGMAC_BUSY), 100, 10000)) >> + return -EBUSY; > Probably you want to wait for the bus to be idle before you change the > mode to C22. Some PHYs can do both C22 and C45, e.g. EEE registers can > be in C45 space, while the rest are in C22. Ok but I can't test C45 right now so maybe leave that change to when I can test it ? Thanks and Best Regards, Jose Miguel Abreu > >> + >> + writel(addr, priv->ioaddr + mii_address); >> + writel(value, priv->ioaddr + mii_data); >> + >> + if (readl_poll_timeout(priv->ioaddr + mii_data, tmp, >> + !(tmp & MII_XGMAC_BUSY), 100, 10000)) >> + return -EBUSY; >> + >> + /* Read the data from the MII data register */ >> + return readl(priv->ioaddr + mii_data) & GENMASK(15, 0); >> +} > Andrew