| Message ID | 20230426063541.15378-3-samin.guo@starfivetech.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add motorcomm phy pad-driver-strength-cfg support | expand |
On 2023/4/26 14:35, Samin Guo wrote: > The motorcomm phy (YT8531) supports the ability to adjust the drive > strength of the rx_clk/rx_data, and the default strength may not be > suitable for all boards. So add configurable options to better match > the boards.(e.g. StarFive VisionFive 2) > > Signed-off-by: Samin Guo <samin.guo@starfivetech.com> > --- > drivers/net/phy/motorcomm.c | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c > index 2fa5a90e073b..08f28ed83e60 100644 > --- a/drivers/net/phy/motorcomm.c > +++ b/drivers/net/phy/motorcomm.c > @@ -236,6 +236,11 @@ > */ > #define YTPHY_WCR_TYPE_PULSE BIT(0) > > +#define YTPHY_PAD_DRIVE_STRENGTH_REG 0xA010 > +#define YTPHY_RGMII_RXC_DS GENMASK(15, 13) > +#define YTPHY_RGMII_RXD_DS GENMASK(5, 4) /* Bit 1 and 0 of rgmii_rxd_ds */ > +#define YTPHY_RGMII_RXD_DS2 BIT(12) /* Bit 2 of rgmii_rxd_ds */ > + Please change YTPHY_RGMII_XXX to YT8531_RGMII_XXX. YT8521's reg (0xA010) is not same as this. Keep bit order. > #define YTPHY_SYNCE_CFG_REG 0xA012 > #define YT8521_SCR_SYNCE_ENABLE BIT(5) > /* 1b0 output 25m clock > @@ -1495,6 +1500,7 @@ static int yt8531_config_init(struct phy_device *phydev) > { > struct device_node *node = phydev->mdio.dev.of_node; > int ret; > + u32 val; > > ret = ytphy_rgmii_clk_delay_config_with_lock(phydev); > if (ret < 0) > @@ -1518,6 +1524,32 @@ static int yt8531_config_init(struct phy_device *phydev) > return ret; > } > > + if (!of_property_read_u32(node, "rx-clk-driver-strength", &val)) { Please check the val of "val", add the handle of default value. > + ret = ytphy_modify_ext_with_lock(phydev, > + YTPHY_PAD_DRIVE_STRENGTH_REG, > + YTPHY_RGMII_RXC_DS, > + FIELD_PREP(YTPHY_RGMII_RXC_DS, val)); > + if (ret < 0) > + return ret; > + } > + > + if (!of_property_read_u32(node, "rx-data-driver-strength", &val)) { > + if (val > FIELD_MAX(YTPHY_RGMII_RXD_DS)) { > + val &= FIELD_MAX(YTPHY_RGMII_RXD_DS); > + val = FIELD_PREP(YTPHY_RGMII_RXD_DS, val); > + val |= YTPHY_RGMII_RXD_DS2; > + } else { > + val = FIELD_PREP(YTPHY_RGMII_RXD_DS, val); > + } > + > + ret = ytphy_modify_ext_with_lock(phydev, > + YTPHY_PAD_DRIVE_STRENGTH_REG, > + YTPHY_RGMII_RXD_DS | YTPHY_RGMII_RXD_DS2, > + val); > + if (ret < 0) > + return ret; > + } > + > return 0; > } >
> > + u32 val; > > > > ret = ytphy_rgmii_clk_delay_config_with_lock(phydev); > > if (ret < 0) > > @@ -1518,6 +1524,32 @@ static int yt8531_config_init(struct phy_device *phydev) > > return ret; > > } > > > > + if (!of_property_read_u32(node, "rx-clk-driver-strength", &val)) { > > Please check the val of "val", add the handle of default value. You can assign val to 3, or better still some #define, before calling of_property_read_u32(). If the property is not found, val will retain that value, and you can then write it to the register. But please do add range checks for when val is in DT. We don't want anybody using 42. -EINVAL should be returned. Andrew
Re: [PATCH v1 2/2] net: phy: motorcomm: Add pad drive strength cfg support From: Frank Sae <Frank.Sae@motor-comm.com> to: Samin Guo <samin.guo@starfivetech.com>, <linux-kernel@vger.kernel.org>, <devicetree@vger.kernel.org>, <netdev@vger.kernel.org>, Peter Geis <pgwipeout@gmail.com> data: 2023/4/26 > > > On 2023/4/26 14:35, Samin Guo wrote: >> The motorcomm phy (YT8531) supports the ability to adjust the drive >> strength of the rx_clk/rx_data, and the default strength may not be >> suitable for all boards. So add configurable options to better match >> the boards.(e.g. StarFive VisionFive 2) >> >> Signed-off-by: Samin Guo <samin.guo@starfivetech.com> >> --- >> drivers/net/phy/motorcomm.c | 32 ++++++++++++++++++++++++++++++++ >> 1 file changed, 32 insertions(+) >> >> diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c >> index 2fa5a90e073b..08f28ed83e60 100644 >> --- a/drivers/net/phy/motorcomm.c >> +++ b/drivers/net/phy/motorcomm.c >> @@ -236,6 +236,11 @@ >> */ >> #define YTPHY_WCR_TYPE_PULSE BIT(0) >> >> +#define YTPHY_PAD_DRIVE_STRENGTH_REG 0xA010 >> +#define YTPHY_RGMII_RXC_DS GENMASK(15, 13) >> +#define YTPHY_RGMII_RXD_DS GENMASK(5, 4) /* Bit 1 and 0 of rgmii_rxd_ds */ >> +#define YTPHY_RGMII_RXD_DS2 BIT(12) /* Bit 2 of rgmii_rxd_ds */ >> + > > Please change YTPHY_RGMII_XXX to YT8531_RGMII_XXX. YT8521's reg (0xA010) is not same as this. > Keep bit order. will fix. > >> #define YTPHY_SYNCE_CFG_REG 0xA012 >> #define YT8521_SCR_SYNCE_ENABLE BIT(5) >> /* 1b0 output 25m clock >> @@ -1495,6 +1500,7 @@ static int yt8531_config_init(struct phy_device *phydev) >> { >> struct device_node *node = phydev->mdio.dev.of_node; >> int ret; >> + u32 val; >> >> ret = ytphy_rgmii_clk_delay_config_with_lock(phydev); >> if (ret < 0) >> @@ -1518,6 +1524,32 @@ static int yt8531_config_init(struct phy_device *phydev) >> return ret; >> } >> >> + if (!of_property_read_u32(node, "rx-clk-driver-strength", &val)) { > > Please check the val of "val", add the handle of default value. > Will fix it in the next version, thanks. >> + ret = ytphy_modify_ext_with_lock(phydev, >> + YTPHY_PAD_DRIVE_STRENGTH_REG, >> + YTPHY_RGMII_RXC_DS, >> + FIELD_PREP(YTPHY_RGMII_RXC_DS, val)); >> + if (ret < 0) >> + return ret; >> + } >> + >> + if (!of_property_read_u32(node, "rx-data-driver-strength", &val)) { >> + if (val > FIELD_MAX(YTPHY_RGMII_RXD_DS)) { >> + val &= FIELD_MAX(YTPHY_RGMII_RXD_DS); >> + val = FIELD_PREP(YTPHY_RGMII_RXD_DS, val); >> + val |= YTPHY_RGMII_RXD_DS2; >> + } else { >> + val = FIELD_PREP(YTPHY_RGMII_RXD_DS, val); >> + } >> + >> + ret = ytphy_modify_ext_with_lock(phydev, >> + YTPHY_PAD_DRIVE_STRENGTH_REG, >> + YTPHY_RGMII_RXD_DS | YTPHY_RGMII_RXD_DS2, >> + val); >> + if (ret < 0) >> + return ret; >> + } >> + >> return 0; >> } >>
Re: [PATCH v1 2/2] net: phy: motorcomm: Add pad drive strength cfg support From: Andrew Lunn <andrew@lunn.ch> to: Frank Sae <Frank.Sae@motor-comm.com> data: 2023/4/26 >>> + u32 val; >>> >>> ret = ytphy_rgmii_clk_delay_config_with_lock(phydev); >>> if (ret < 0) >>> @@ -1518,6 +1524,32 @@ static int yt8531_config_init(struct phy_device *phydev) >>> return ret; >>> } >>> >>> + if (!of_property_read_u32(node, "rx-clk-driver-strength", &val)) { >> >> Please check the val of "val", add the handle of default value. > > You can assign val to 3, or better still some #define, before calling > of_property_read_u32(). If the property is not found, val will retain > that value, and you can then write it to the register. > > But please do add range checks for when val is in DT. We don't want > anybody using 42. -EINVAL should be returned. > > Andrew Thanks, good advice. Best regards, Samin
diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c index 2fa5a90e073b..08f28ed83e60 100644 --- a/drivers/net/phy/motorcomm.c +++ b/drivers/net/phy/motorcomm.c @@ -236,6 +236,11 @@ */ #define YTPHY_WCR_TYPE_PULSE BIT(0) +#define YTPHY_PAD_DRIVE_STRENGTH_REG 0xA010 +#define YTPHY_RGMII_RXC_DS GENMASK(15, 13) +#define YTPHY_RGMII_RXD_DS GENMASK(5, 4) /* Bit 1 and 0 of rgmii_rxd_ds */ +#define YTPHY_RGMII_RXD_DS2 BIT(12) /* Bit 2 of rgmii_rxd_ds */ + #define YTPHY_SYNCE_CFG_REG 0xA012 #define YT8521_SCR_SYNCE_ENABLE BIT(5) /* 1b0 output 25m clock @@ -1495,6 +1500,7 @@ static int yt8531_config_init(struct phy_device *phydev) { struct device_node *node = phydev->mdio.dev.of_node; int ret; + u32 val; ret = ytphy_rgmii_clk_delay_config_with_lock(phydev); if (ret < 0) @@ -1518,6 +1524,32 @@ static int yt8531_config_init(struct phy_device *phydev) return ret; } + if (!of_property_read_u32(node, "rx-clk-driver-strength", &val)) { + ret = ytphy_modify_ext_with_lock(phydev, + YTPHY_PAD_DRIVE_STRENGTH_REG, + YTPHY_RGMII_RXC_DS, + FIELD_PREP(YTPHY_RGMII_RXC_DS, val)); + if (ret < 0) + return ret; + } + + if (!of_property_read_u32(node, "rx-data-driver-strength", &val)) { + if (val > FIELD_MAX(YTPHY_RGMII_RXD_DS)) { + val &= FIELD_MAX(YTPHY_RGMII_RXD_DS); + val = FIELD_PREP(YTPHY_RGMII_RXD_DS, val); + val |= YTPHY_RGMII_RXD_DS2; + } else { + val = FIELD_PREP(YTPHY_RGMII_RXD_DS, val); + } + + ret = ytphy_modify_ext_with_lock(phydev, + YTPHY_PAD_DRIVE_STRENGTH_REG, + YTPHY_RGMII_RXD_DS | YTPHY_RGMII_RXD_DS2, + val); + if (ret < 0) + return ret; + } + return 0; }
The motorcomm phy (YT8531) supports the ability to adjust the drive strength of the rx_clk/rx_data, and the default strength may not be suitable for all boards. So add configurable options to better match the boards.(e.g. StarFive VisionFive 2) Signed-off-by: Samin Guo <samin.guo@starfivetech.com> --- drivers/net/phy/motorcomm.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)