On 10.07.23 08:36, Marco Felsch wrote:
> Sync the barebox phy_{write,read,modify}_mmd_indirect API with the Linux
> phy_{write,read,modify}_mmd API to make it easier and less error prone
> to port phy drivers. This also fixes the r8169 driver since this driver
> did not adapt the parameters while porting from Linux.
>
> The sync also aligns the function parameter `prtad` as well as the
> function documentation.
This breaks out of tree board code in a subtle and annoying manner.
We are in luck that the kernel function has a different name than
the barebox function, so I'd rather suggest:
- Add phy_read_mmd/phy_write_mmd functions with same argument
order as Linux
- Switch all users in barebox to the new functions
- Mark the old _indirect function deprecated with a suggestion
to use phy_read_mmd/phy_write_mmd and swap address arguments.
> Signed-off-by: Marco Felsch <m.felsch@xxxxxxxxxxxxxx>
> ---
> arch/arm/boards/datamodul-edm-qmx6/board.c | 6 +--
> arch/arm/boards/embest-marsboard/board.c | 6 +--
> arch/arm/boards/terasic-de0-nano-soc/board.c | 6 +--
> arch/arm/boards/terasic-de10-nano/board.c | 6 +--
> arch/arm/boards/tqma6x/board.c | 6 +--
> drivers/net/phy/at803x.c | 4 +-
> drivers/net/phy/dp83867.c | 40 +++++++-------
> drivers/net/phy/micrel.c | 24 ++++-----
> drivers/net/phy/phy.c | 57 ++++++++++----------
> include/linux/phy.h | 10 ++--
> 10 files changed, 83 insertions(+), 82 deletions(-)
>
> diff --git a/arch/arm/boards/datamodul-edm-qmx6/board.c b/arch/arm/boards/datamodul-edm-qmx6/board.c
> index 366b64d35a..0dc71f2aca 100644
> --- a/arch/arm/boards/datamodul-edm-qmx6/board.c
> +++ b/arch/arm/boards/datamodul-edm-qmx6/board.c
> @@ -49,9 +49,9 @@ static int ksz9031rn_phy_fixup(struct phy_device *dev)
> * min rx data delay, max rx/tx clock delay,
> * min rx/tx control delay
> */
> - phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
> - phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
> - phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x03ff);
> + phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
> + phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
> + phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x03ff);
>
> return 0;
> }
> diff --git a/arch/arm/boards/embest-marsboard/board.c b/arch/arm/boards/embest-marsboard/board.c
> index 7274595e2a..e70fefec5e 100644
> --- a/arch/arm/boards/embest-marsboard/board.c
> +++ b/arch/arm/boards/embest-marsboard/board.c
> @@ -20,13 +20,13 @@ static int ar8035_phy_fixup(struct phy_device *dev)
> /* Ar803x phy SmartEEE feature cause link status generates glitch,
> * which cause ethernet link down/up issue, so disable SmartEEE
> */
> - val = phy_read_mmd_indirect(dev, 0x805d, MDIO_MMD_PCS);
> + val = phy_read_mmd_indirect(dev, MDIO_MMD_PCS, 0x805d);
> phy_write(dev, MII_MMD_DATA, val & ~(1 << 8));
>
> - val = phy_read_mmd_indirect(dev, 0x4003, MDIO_MMD_PCS);
> + val = phy_read_mmd_indirect(dev, MDIO_MMD_PCS, 0x4003);
> phy_write(dev, MII_MMD_DATA, val & ~(1 << 8));
>
> - val = phy_read_mmd_indirect(dev, 0x4007, MDIO_MMD_PCS);
> + val = phy_read_mmd_indirect(dev, MDIO_MMD_PCS, 0x4007);
> val &= 0xffe3;
> val |= 0x18;
> phy_write(dev, MII_MMD_DATA, val);
> diff --git a/arch/arm/boards/terasic-de0-nano-soc/board.c b/arch/arm/boards/terasic-de0-nano-soc/board.c
> index 832160c595..fa83498f46 100644
> --- a/arch/arm/boards/terasic-de0-nano-soc/board.c
> +++ b/arch/arm/boards/terasic-de0-nano-soc/board.c
> @@ -19,9 +19,9 @@ static int phy_fixup(struct phy_device *dev)
> * min rx data delay, max rx/tx clock delay,
> * min rx/tx control delay
> */
> - phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
> - phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
> - phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
> + phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
> + phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
> + phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x003ff);
> return 0;
> }
>
> diff --git a/arch/arm/boards/terasic-de10-nano/board.c b/arch/arm/boards/terasic-de10-nano/board.c
> index e47d9ac841..7ceb691f71 100644
> --- a/arch/arm/boards/terasic-de10-nano/board.c
> +++ b/arch/arm/boards/terasic-de10-nano/board.c
> @@ -19,9 +19,9 @@ static int phy_fixup(struct phy_device *dev)
> * min rx data delay, max rx/tx clock delay,
> * min rx/tx control delay
> */
> - phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
> - phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
> - phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
> + phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
> + phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
> + phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x003ff);
> return 0;
> }
>
> diff --git a/arch/arm/boards/tqma6x/board.c b/arch/arm/boards/tqma6x/board.c
> index 8a91ad652a..3fd2e1db2c 100644
> --- a/arch/arm/boards/tqma6x/board.c
> +++ b/arch/arm/boards/tqma6x/board.c
> @@ -47,9 +47,9 @@ static int ksz9031rn_phy_fixup(struct phy_device *dev)
> * min rx data delay, max rx/tx clock delay,
> * min rx/tx control delay
> */
> - phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
> - phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
> - phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
> + phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
> + phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
> + phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x003ff);
>
> return 0;
> }
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index 18182bffc2..41010c58ad 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -229,14 +229,14 @@ static int at803x_clk_out_config(struct phy_device *phydev)
> if (!priv->clk_25m_mask)
> return 0;
>
> - val = phy_read_mmd_indirect(phydev, AT803X_MMD7_CLK25M, MDIO_MMD_AN);
> + val = phy_read_mmd_indirect(phydev, MDIO_MMD_AN, AT803X_MMD7_CLK25M);
> if (val < 0)
> return val;
>
> val &= ~priv->clk_25m_mask;
> val |= priv->clk_25m_reg;
>
> - phy_write_mmd_indirect(phydev, AT803X_MMD7_CLK25M, MDIO_MMD_AN, val);
> + phy_write_mmd_indirect(phydev, MDIO_MMD_AN, AT803X_MMD7_CLK25M, val);
>
> return 0;
> }
> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
> index d8109172df..85fe5107da 100644
> --- a/drivers/net/phy/dp83867.c
> +++ b/drivers/net/phy/dp83867.c
> @@ -154,14 +154,14 @@ static int dp83867_config_port_mirroring(struct phy_device *phydev)
> struct dp83867_private *dp83867 = phydev->priv;
> u16 val;
>
> - val = phy_read_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR);
> + val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR, DP83867_CFG4);
>
> if (dp83867->port_mirroring == DP83867_PORT_MIRROING_EN)
> val |= DP83867_CFG4_PORT_MIRROR_EN;
> else
> val &= ~DP83867_CFG4_PORT_MIRROR_EN;
>
> - phy_write_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR, val);
> + phy_write_mmd_indirect(phydev, DP83867_DEVADDR, DP83867_CFG4, val);
>
> return 0;
> }
> @@ -256,11 +256,11 @@ static int dp83867_config_init(struct phy_device *phydev)
> phy_write(phydev, DP83867_CTRL, val | DP83867_SW_RESTART);
>
> if (dp83867->rxctrl_strap_quirk) {
> - val = phy_read_mmd_indirect(phydev, DP83867_CFG4,
> - DP83867_DEVADDR);
> + val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR,
> + DP83867_CFG4);
> val &= ~BIT(7);
> - phy_write_mmd_indirect(phydev, DP83867_CFG4,
> - DP83867_DEVADDR, val);
> + phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
> + DP83867_CFG4, val);
> }
>
> if (phy_interface_is_rgmii(phydev)) {
> @@ -270,8 +270,8 @@ static int dp83867_config_init(struct phy_device *phydev)
> if (ret)
> return ret;
>
> - val = phy_read_mmd_indirect(phydev, DP83867_RGMIICTL,
> - DP83867_DEVADDR);
> + val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR,
> + DP83867_RGMIICTL);
>
> switch (phydev->interface) {
> case PHY_INTERFACE_MODE_RGMII_ID:
> @@ -287,31 +287,31 @@ static int dp83867_config_init(struct phy_device *phydev)
> default:
> break;
> }
> - phy_write_mmd_indirect(phydev, DP83867_RGMIICTL,
> - DP83867_DEVADDR, val);
> + phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
> + DP83867_RGMIICTL, val);
>
> delay = (dp83867->rx_id_delay |
> (dp83867->tx_id_delay << DP83867_RGMII_TX_CLK_DELAY_SHIFT));
>
> - phy_write_mmd_indirect(phydev, DP83867_RGMIIDCTL,
> - DP83867_DEVADDR, delay);
> + phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
> + DP83867_RGMIIDCTL, delay);
>
> if (dp83867->io_impedance >= 0) {
> - val = phy_read_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
> - DP83867_DEVADDR);
> + val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR,
> + DP83867_IO_MUX_CFG);
> val &= ~DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL;
> val |= (dp83867->io_impedance &
> DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL);
>
> - phy_write_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
> - DP83867_DEVADDR, val);
> + phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
> + DP83867_IO_MUX_CFG, val);
> }
> } else if (phy_interface_is_sgmii(phydev)) {
> phy_write(phydev, MII_BMCR,
> BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000);
>
> - phy_write_mmd_indirect(phydev, DP83867_RGMIICTL,
> - DP83867_DEVADDR, 0x0);
> + phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
> + DP83867_RGMIICTL, 0x0);
>
> val = DP83867_PHYCTRL_SGMIIEN |
> DP83867_MDI_CROSSOVER_MDIX << DP83867_MDI_CROSSOVER |
> @@ -341,8 +341,8 @@ static int dp83867_config_init(struct phy_device *phydev)
> DP83867_IO_MUX_CFG_CLK_O_SEL_SHIFT;
> }
>
> - phy_modify_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
> - DP83867_DEVADDR, mask, val);
> + phy_modify_mmd_indirect(phydev, DP83867_DEVADDR,
> + DP83867_IO_MUX_CFG, mask, val);
> }
>
> return 0;
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 02d474c442..892df01d2e 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -387,7 +387,7 @@ static int ksz9031_of_load_skew_values(struct phy_device *phydev,
> return 0;
>
> if (matches < numfields)
> - newval = phy_read_mmd_indirect(phydev, reg, MDIO_MMD_WIS);
> + newval = phy_read_mmd_indirect(phydev, MDIO_MMD_WIS, reg);
> else
> newval = 0;
>
> @@ -401,15 +401,15 @@ static int ksz9031_of_load_skew_values(struct phy_device *phydev,
> << (field_sz * i));
> }
>
> - phy_write_mmd_indirect(phydev, reg, MDIO_MMD_WIS, newval);
> + phy_write_mmd_indirect(phydev, MDIO_MMD_WIS, reg, newval);
> return 0;
> }
>
> static int ksz9031_center_flp_timing(struct phy_device *phydev)
> {
> /* Center KSZ9031RNX FLP timing at 16ms. */
> - phy_write_mmd_indirect(phydev, MII_KSZ9031RN_FLP_BURST_TX_HI, 0, 0x0006);
> - phy_write_mmd_indirect(phydev, MII_KSZ9031RN_FLP_BURST_TX_LO, 0, 0x1a80);
> + phy_write_mmd_indirect(phydev, 0, MII_KSZ9031RN_FLP_BURST_TX_HI, 0x0006);
> + phy_write_mmd_indirect(phydev, 0, MII_KSZ9031RN_FLP_BURST_TX_LO, 0x1a80);
>
> return genphy_restart_aneg(phydev);
> }
> @@ -447,27 +447,27 @@ static int ksz9031_config_rgmii_delay(struct phy_device *phydev)
> return 0;
> }
>
> - phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CONTROL_PAD_SKEW,
> - MDIO_MMD_WIS,
> + phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
> + MII_KSZ9031RN_CONTROL_PAD_SKEW,
> FIELD_PREP(MII_KSZ9031RN_RX_CTL_M, rx) |
> FIELD_PREP(MII_KSZ9031RN_TX_CTL_M, tx));
>
> - phy_write_mmd_indirect(phydev, MII_KSZ9031RN_RX_DATA_PAD_SKEW,
> - MDIO_MMD_WIS,
> + phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
> + MII_KSZ9031RN_RX_DATA_PAD_SKEW,
> FIELD_PREP(MII_KSZ9031RN_RXD3, rx) |
> FIELD_PREP(MII_KSZ9031RN_RXD2, rx) |
> FIELD_PREP(MII_KSZ9031RN_RXD1, rx) |
> FIELD_PREP(MII_KSZ9031RN_RXD0, rx));
>
> - phy_write_mmd_indirect(phydev, MII_KSZ9031RN_TX_DATA_PAD_SKEW,
> - MDIO_MMD_WIS,
> + phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
> + MII_KSZ9031RN_TX_DATA_PAD_SKEW,
> FIELD_PREP(MII_KSZ9031RN_TXD3, tx) |
> FIELD_PREP(MII_KSZ9031RN_TXD2, tx) |
> FIELD_PREP(MII_KSZ9031RN_TXD1, tx) |
> FIELD_PREP(MII_KSZ9031RN_TXD0, tx));
>
> - phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CLK_PAD_SKEW,
> - MDIO_MMD_WIS,
> + phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
> + MII_KSZ9031RN_CLK_PAD_SKEW,
> FIELD_PREP(MII_KSZ9031RN_GTX_CLK, tx_clk) |
> FIELD_PREP(MII_KSZ9031RN_RX_CLK, rx_clk));
> return 0;
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 74381949b4..283e1a3d79 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -819,24 +819,25 @@ int genphy_read_status(struct phy_device *phydev)
> return 0;
> }
>
> -static inline void mmd_phy_indirect(struct phy_device *phydev, int prtad,
> - int devad)
> +static inline void mmd_phy_indirect(struct phy_device *phydev, int devad,
> + u16 regnum)
> {
> /* Write the desired MMD Devad */
> phy_write(phydev, MII_MMD_CTRL, devad);
>
> /* Write the desired MMD register address */
> - phy_write(phydev, MII_MMD_DATA, prtad);
> + phy_write(phydev, MII_MMD_DATA, regnum);
>
> /* Select the Function : DATA with no post increment */
> phy_write(phydev, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
> }
>
> /**
> - * phy_read_mmd_indirect - reads data from the MMD registers
> - * @phy_device: phy device
> - * @prtad: MMD Address
> - * @devad: MMD DEVAD
> + * phy_read_mmd_indirect - Convenience function for reading a register
> + * from an MMD on a given PHY.
> + * @phydev: The phy_device struct
> + * @devad: The MMD to read from
> + * @regnum: The register on the MMD to read
> *
> * Description: it reads data from the MMD registers (clause 22 to access to
> * clause 45) of the specified phy address.
> @@ -846,11 +847,11 @@ static inline void mmd_phy_indirect(struct phy_device *phydev, int prtad,
> * 3) Write reg 13 // MMD Data Command for MMD DEVAD
> * 3) Read reg 14 // Read MMD data
> */
> -int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
> +int phy_read_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum)
> {
> u32 ret;
>
> - mmd_phy_indirect(phydev, prtad, devad);
> + mmd_phy_indirect(phydev, devad, regnum);
>
> /* Read the content of the MMD's selected register */
> ret = phy_read(phydev, MII_MMD_DATA);
> @@ -859,11 +860,12 @@ int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
> }
>
> /**
> - * phy_write_mmd_indirect - writes data to the MMD registers
> - * @phy_device: phy device
> - * @prtad: MMD Address
> - * @devad: MMD DEVAD
> - * @data: data to write in the MMD register
> + * phy_write_mmd_indirect - Convenience function for writing a register
> + * on an MMD on a given PHY.
> + * @phydev: The phy_device struct
> + * @devad: The MMD to read from
> + * @regnum: The register on the MMD to read
> + * @val: value to write to @regnum
> *
> * Description: Write data from the MMD registers of the specified
> * phy address.
> @@ -873,34 +875,33 @@ int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
> * 3) Write reg 13 // MMD Data Command for MMD DEVAD
> * 3) Write reg 14 // Write MMD data
> */
> -void phy_write_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
> - u16 data)
> +void phy_write_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
> + u16 val)
> {
> - mmd_phy_indirect(phydev, prtad, devad);
> + mmd_phy_indirect(phydev, devad, regnum);
>
> /* Write the data into MMD's selected register */
> - phy_write(phydev, MII_MMD_DATA, data);
> + phy_write(phydev, MII_MMD_DATA, val);
> }
>
> /**
> - * phy_modify_mmd_indirect - Convenience function for modifying a MMD register
> - * @phydev: phy device
> - * @prtad: MMD Address
> - * @devad: MMD DEVAD
> + * phy_modify_mmd_indirect - Convenience function for modifying a register on MMD
> + * @phydev: the phy_device struct
> + * @devad: the MMD containing register to modify
> + * @regnum: register number to modify
> * @mask: bit mask of bits to clear
> - * @set: new value of bits set in @mask
> - *
> + * @set: new value of bits set in mask to write to @regnum
> */
> -int phy_modify_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
> - u16 mask, u16 set)
> +int phy_modify_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
> + u16 mask, u16 set)
> {
> int ret;
>
> - ret = phy_read_mmd_indirect(phydev, prtad, devad);
> + ret = phy_read_mmd_indirect(phydev, devad, regnum);
> if (ret < 0)
> return ret;
>
> - phy_write_mmd_indirect(phydev, prtad, devad, (ret & ~mask) | set);
> + phy_write_mmd_indirect(phydev, devad, regnum, (ret & ~mask) | set);
>
> return 0;
> }
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 509bf72de9..d96c4e97ab 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -406,11 +406,11 @@ int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask,
> int phy_register_fixup_for_id(const char *bus_id,
> int (*run)(struct phy_device *));
> int phy_scan_fixups(struct phy_device *phydev);
> -int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad);
> -void phy_write_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
> - u16 data);
> -int phy_modify_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
> - u16 mask, u16 set);
> +int phy_read_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum);
> +void phy_write_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
> + u16 val);
> +int phy_modify_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
> + u16 mask, u16 set);
>
> static inline bool phy_acquired(struct phy_device *phydev)
> {
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
