From: Ahmad Fatoum <a.fatoum@pengutronix.de>
To: Marco Felsch <m.felsch@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:03:39 +0200 [thread overview]
Message-ID: <e96d425b-3f38-1cd7-fc3f-be4b12d09a75@pengutronix.de> (raw)
In-Reply-To: <20230710110110.6r2y2v4kxvjgsaeu@pengutronix.de>
On 10.07.23 13:01, Marco Felsch wrote:
> 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 :/
Some breakage is ultimately unavoidable, but we can try to not needlessly
introduce silent breakage. Here we have a nice way out: We import the
Linux function with the Linux name and use that for upstream code. The old
function is marked deprecated and removed later on. Do you see an issue
with that?
Cheers,
Ahmad
>
> 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 |
>>
>>
>
--
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 |
next prev parent reply other threads:[~2023-07-10 11:05 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
2023-07-10 11:03 ` Ahmad Fatoum [this message]
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=e96d425b-3f38-1cd7-fc3f-be4b12d09a75@pengutronix.de \
--to=a.fatoum@pengutronix.de \
--cc=barebox@lists.infradead.org \
--cc=m.felsch@pengutronix.de \
/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