| Message ID | 20241024152448.102-9-paul.barker.ct@bp.renesas.com |
|---|---|
| State | New |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | Add support for Ethernet interfaces on RZ/G2L | expand |
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.ct@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.
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.ct@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). Thanks,
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.ct@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.
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); } /* Silicon Errata DS80000693B
We can call phy_modify_mmd() instead of manually calling drv->readext() and drv->writeext(). Signed-off-by: Paul Barker <paul.barker.ct@bp.renesas.com> --- drivers/net/phy/micrel_ksz90x1.c | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-)