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 <joab...@synopsys.com>
>> Cc: David S. Miller <da...@davemloft.net>
>> Cc: Joao Pinto <jpi...@synopsys.com>
>> Cc: Giuseppe Cavallaro <peppe.cavall...@st.com>
>> Cc: Alexandre Torgue <alexandre.tor...@st.com>
>> Cc: Andrew Lunn <and...@lunn.ch>
>> ---
>> 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 <linux/phy.h>
>> #include <linux/slab.h>
>>
>> +#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
