Re: [PATCH 08/14] net: phy: ksz90x1: Simplify ksz9131_config_rgmii_delay

[Marek Vasut]

On 10/28/24 9:50 AM, Paul Barker wrote:

On 27/10/2024 16:20, Marek Vasut wrote:

On 10/24/24 5:24 PM, Paul Barker wrote:

We can call phy_modify_mmd() instead of manually calling drv->readext()
and drv->writeext().

Signed-off-by: Paul Barker <paul.barker...@bp.renesas.com>
---
   drivers/net/phy/micrel_ksz90x1.c | 26 ++++++++------------------
   1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/drivers/net/phy/micrel_ksz90x1.c b/drivers/net/phy/micrel_ksz90x1.c
index b64046e0bc72..6515d8feb9be 100644
--- a/drivers/net/phy/micrel_ksz90x1.c
+++ b/drivers/net/phy/micrel_ksz90x1.c
@@ -502,8 +502,7 @@ static int ksz9131_of_load_all_skew_values(struct 
phy_device *phydev)

static int ksz9131_config_rgmii_delay(struct phy_device *phydev)

   {
-       struct phy_driver *drv = phydev->drv;
-       u16 rxcdll_val, txcdll_val, val;
+       u16 rxcdll_val, txcdll_val;
        int ret;

switch (phydev->interface) {

@@ -527,24 +526,15 @@ static int ksz9131_config_rgmii_delay(struct phy_device 
*phydev)
                return 0;
        }

- val = drv->readext(phydev, 0, KSZ9131RN_MMD_COMMON_CTRL_REG,

-                          KSZ9131RN_RXC_DLL_CTRL);
-       val &= ~KSZ9131RN_DLL_CTRL_BYPASS;
-       val |= rxcdll_val;
-       ret = drv->writeext(phydev, 0, KSZ9131RN_MMD_COMMON_CTRL_REG,
-                           KSZ9131RN_RXC_DLL_CTRL, val);
-       if (ret)
+       ret = phy_modify_mmd(phydev, KSZ9131RN_MMD_COMMON_CTRL_REG,
+                            KSZ9131RN_RXC_DLL_CTRL, KSZ9131RN_DLL_CTRL_BYPASS,
+                            rxcdll_val);
+       if (ret < 0)
                return ret;

- val = drv->readext(phydev, 0, KSZ9131RN_MMD_COMMON_CTRL_REG,

-                          KSZ9131RN_TXC_DLL_CTRL);
-
-       val &= ~KSZ9131RN_DLL_CTRL_BYPASS;
-       val |= txcdll_val;
-       ret = drv->writeext(phydev, 0, KSZ9131RN_MMD_COMMON_CTRL_REG,
-                           KSZ9131RN_TXC_DLL_CTRL, val);
-
-       return ret;
+       return phy_modify_mmd(phydev, KSZ9131RN_MMD_COMMON_CTRL_REG,
+                             KSZ9131RN_TXC_DLL_CTRL, KSZ9131RN_DLL_CTRL_BYPASS,
+                             txcdll_val);
   }

Can't you set both bitfields at the same time ? They seem to be in the
same register.

These writes are to two different registers (KSZ9131RN_RXC_DLL_CTRL and
KSZ9131RN_TXC_DLL_CTRL), using the same bitmask in both cases
(KSZ9131RN_DLL_CTRL_BYPASS).

Uh, right, thanks for clarifying.