netdev - Re: [PATCH net-next v5 5/6] net: tn40xx: add mdio bus support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <1f28bc3c-3489-4fc7-b5de-20824631e5df@gmx.net>
Date: Wed, 8 May 2024 20:08:57 +0200
From: Hans-Frieder Vogt <hfdevel@....net>
To: FUJITA Tomonori <fujita.tomonori@...il.com>
Cc: netdev@...r.kernel.org, andrew@...n.ch, horms@...nel.org,
 kuba@...nel.org, jiri@...nulli.us, pabeni@...hat.com
Subject: Re: [PATCH net-next v5 5/6] net: tn40xx: add mdio bus support

 > +static int tn40_mdio_read(struct tn40_priv *priv, int port, int device,
 > +              u16 regnum)
 > +{
 > +    void __iomem *regs = priv->regs;
 > +    u32 tmp_reg, i;
 > +
 > +    /* wait until MDIO is not busy */
 > +    if (tn40_mdio_get(priv, NULL))
 > +        return -EIO;
 > +
 > +    i = ((device & 0x1F) | ((port & 0x1F) << 5));

instead of using numbers for the masks, you may use here the constants
defined in uapi/linux/mdio.h, to make the code more understandable

i = (device & MDIO_PHY_ID_DEVAD) | ((port << 5) & MDIO_PHY_ID_PRTAD);

 > +    writel(i, regs + TN40_REG_MDIO_CMD);
 > +    writel((u32)regnum, regs + TN40_REG_MDIO_ADDR);
 > +    if (tn40_mdio_get(priv, NULL))
 > +        return -EIO;
 > +
 > +    writel(((1 << 15) | i), regs + TN40_REG_MDIO_CMD);

similarly here:

writel((MDIO_PHY_ID_C45 | i), regs + TN40_REG_MDIO_CMD);

 > +    /* read CMD_STAT until not busy */
 > +    if (tn40_mdio_get(priv, NULL))
 > +        return -EIO;
 > +
 > +    tmp_reg = readl(regs + TN40_REG_MDIO_DATA);
 > +    return lower_16_bits(tmp_reg);
 > +}
 > +
 > +static int tn40_mdio_write(struct tn40_priv *priv, int port, int device,
 > +               u16 regnum, u16 data)
 > +{
 > +    void __iomem *regs = priv->regs;
 > +    u32 tmp_reg = 0;
 > +    int ret;
 > +
 > +    /* wait until MDIO is not busy */
 > +    if (tn40_mdio_get(priv, NULL))
 > +        return -EIO;
 > +    writel(((device & 0x1F) | ((port & 0x1F) << 5)),

and also here, similarly:

writel((device & MDIO_PHY_ID_DEVAD) | ((port << 5) & MDIO_PHY_ID_PRTAD),

 > +           regs + TN40_REG_MDIO_CMD);
 > +    writel((u32)regnum, regs + TN40_REG_MDIO_ADDR);
 > +    if (tn40_mdio_get(priv, NULL))
 > +        return -EIO;
 > +    writel((u32)data, regs + TN40_REG_MDIO_DATA);
 > +    /* read CMD_STAT until not busy */
 > +    ret = tn40_mdio_get(priv, &tmp_reg);
 > +    if (ret)
 > +        return -EIO;
 > +
 > +    if (TN40_GET_MDIO_RD_ERR(tmp_reg)) {
 > +        dev_err(&priv->pdev->dev, "MDIO error after write command\n");
 > +        return -EIO;
 > +    }
 > +    return 0;
 > +}

--
Cheers,
Hans