mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Marco Felsch <m.felsch@pengutronix.de>
To: Ahmad Fatoum <a.fatoum@pengutronix.de>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH 3/7] net: phy: sync phy_{write,read,modify}_mmd_indirect with linux
Date: Mon, 10 Jul 2023 13:01:10 +0200	[thread overview]
Message-ID: <20230710110110.6r2y2v4kxvjgsaeu@pengutronix.de> (raw)
In-Reply-To: <8839814f-de7c-a70b-8573-55dd2c38c766@pengutronix.de>

On 23-07-10, Ahmad Fatoum wrote:
> Hello Marco,
> 
> On 10.07.23 12:49, Marco Felsch wrote:
> > On 23-07-10, Ahmad Fatoum wrote:
> >> 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.
> > 
> > Do we really need to care about out all-of-tree drivers/boards? If so,
> > we need to ensure to not break any public function.
> 
> Out-of-tree drivers not too much probably, but out-of-tree boards
> are unfortunately the norm. Breaking the build can be justified,
> but silent breaking by swapping arguments is not nice.

And how do you ensure that a public API is only used by drivers? Don't
get me wrong I got your point but if you update an out-of-tree board to
a newer barebox version it may happen that something break. Therefore
manufacturers should upstream there code to reduce the maintenance
effort to a minimum :) I know we don't live in an ideal world :/

Regards,
  Marco

> >> 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.
> > 
> > I was think about such approach, then I checked that the Linux API does
> > handle C45 as well, so we would need least at least $some minimal
> > support like:
> > 
> > if (c45)
> > 	pr_err("C45 phys are not supported yet\n");
> > 	return -EINVAL;
> 
> Sounds good IMO. Maybe return -ENOSYS instead.
> 
> > 
> > and therefore I stuck with the _indirect functions.
> > 
> > Regards,
> >   Marco
> > 
> >>
> >>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> >>> ---
> >>>  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 |
> >>
> >>
> > 
> 
> -- 
> 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 |
> 
> 



  reply	other threads:[~2023-07-10 11:02 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-10  6:36 [PATCH 1/7] ARM: boards: make use of MDIO_MMD register defines Marco Felsch
2023-07-10  6:36 ` [PATCH 2/7] net: phy: micrel: " Marco Felsch
2023-07-10  9:56   ` Ahmad Fatoum
2023-07-10 10:37     ` Marco Felsch
2023-07-10  6:36 ` [PATCH 3/7] net: phy: sync phy_{write,read,modify}_mmd_indirect with linux Marco Felsch
2023-07-10  9:59   ` Ahmad Fatoum
2023-07-10 10:49     ` Marco Felsch
2023-07-10 10:53       ` Ahmad Fatoum
2023-07-10 11:01         ` Marco Felsch [this message]
2023-07-10 11:03           ` Ahmad Fatoum
2023-07-10 12:43             ` Marco Felsch
2023-07-10 12:52               ` Ahmad Fatoum
2023-07-10  6:36 ` [PATCH 4/7] net: phy: add phydev_{err,err_probe,info,warn,dbg} macros Marco Felsch
2023-07-10 10:01   ` Ahmad Fatoum
2023-07-10  6:36 ` [PATCH 5/7] net: phy: at803x: add support for configuring SmartEEE Marco Felsch
2023-07-10 10:10   ` Ahmad Fatoum
2023-07-10 12:10     ` Oleksij Rempel
2023-07-10  6:36 ` [PATCH 6/7] net: phy: at803x: add disable hibernation mode support Marco Felsch
2023-07-10 10:07   ` Ahmad Fatoum
2023-07-10 10:53     ` Marco Felsch
2023-07-10 11:07       ` Ahmad Fatoum
2023-07-10  6:36 ` [PATCH 7/7] net: phy: at803x: disable extended next page bit Marco Felsch
2023-07-10 10:03   ` Ahmad Fatoum
2023-07-10  9:55 ` [PATCH 1/7] ARM: boards: make use of MDIO_MMD register defines Ahmad Fatoum

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230710110110.6r2y2v4kxvjgsaeu@pengutronix.de \
    --to=m.felsch@pengutronix.de \
    --cc=a.fatoum@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox