| Message ID | 4452cb7e-1a2f-4213-b49f-9de196be9204@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | net: phy: remove calls to devm_hwmon_sanitize_name | expand |
Hello Heiner, On Thu, 13 Mar 2025 20:45:06 +0100 Heiner Kallweit <hkallweit1@gmail.com> wrote: > Since c909e68f8127 ("hwmon: (core) Use device name as a fallback in > devm_hwmon_device_register_with_info") we can simply provide NULL > as name argument. > > Note that neither priv->hwmon_name nor priv->hwmon_dev are used > outside tja11xx_hwmon_register. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/net/phy/nxp-tja11xx.c | 19 +++++-------------- > 1 file changed, 5 insertions(+), 14 deletions(-) > > diff --git a/drivers/net/phy/nxp-tja11xx.c b/drivers/net/phy/nxp-tja11xx.c > index 601094fe2..07e94a247 100644 > --- a/drivers/net/phy/nxp-tja11xx.c > +++ b/drivers/net/phy/nxp-tja11xx.c > @@ -87,8 +87,6 @@ > #define TJA110X_RMII_MODE_REFCLK_IN BIT(0) > > struct tja11xx_priv { > - char *hwmon_name; > - struct device *hwmon_dev; > struct phy_device *phydev; > struct work_struct phy_register_work; > u32 flags; > @@ -508,19 +506,12 @@ static const struct hwmon_chip_info tja11xx_hwmon_chip_info = { > static int tja11xx_hwmon_register(struct phy_device *phydev, > struct tja11xx_priv *priv) > { > - struct device *dev = &phydev->mdio.dev; > - > - priv->hwmon_name = devm_hwmon_sanitize_name(dev, dev_name(dev)); > - if (IS_ERR(priv->hwmon_name)) > - return PTR_ERR(priv->hwmon_name); > - > - priv->hwmon_dev = > - devm_hwmon_device_register_with_info(dev, priv->hwmon_name, > - phydev, > - &tja11xx_hwmon_chip_info, > - NULL); > + struct device *hdev, *dev = &phydev->mdio.dev; > > - return PTR_ERR_OR_ZERO(priv->hwmon_dev); > + hdev = devm_hwmon_device_register_with_info(dev, NULL, phydev, > + &tja11xx_hwmon_chip_info, > + NULL); > + return PTR_ERR_OR_ZERO(hdev); > } The change look correct to me, however I think you can go one step further and remove the field tja11xx_priv.hwmon_name as well as hwmon_dev. One could argue that we can even remove tja11xx_hwmon_register() entirely Thanks, Maxime
On 14.03.2025 08:45, Maxime Chevallier wrote: > Hello Heiner, > > On Thu, 13 Mar 2025 20:45:06 +0100 > Heiner Kallweit <hkallweit1@gmail.com> wrote: > >> Since c909e68f8127 ("hwmon: (core) Use device name as a fallback in >> devm_hwmon_device_register_with_info") we can simply provide NULL >> as name argument. >> >> Note that neither priv->hwmon_name nor priv->hwmon_dev are used >> outside tja11xx_hwmon_register. >> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> --- >> drivers/net/phy/nxp-tja11xx.c | 19 +++++-------------- >> 1 file changed, 5 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/net/phy/nxp-tja11xx.c b/drivers/net/phy/nxp-tja11xx.c >> index 601094fe2..07e94a247 100644 >> --- a/drivers/net/phy/nxp-tja11xx.c >> +++ b/drivers/net/phy/nxp-tja11xx.c >> @@ -87,8 +87,6 @@ >> #define TJA110X_RMII_MODE_REFCLK_IN BIT(0) >> >> struct tja11xx_priv { >> - char *hwmon_name; >> - struct device *hwmon_dev; >> struct phy_device *phydev; >> struct work_struct phy_register_work; >> u32 flags; >> @@ -508,19 +506,12 @@ static const struct hwmon_chip_info tja11xx_hwmon_chip_info = { >> static int tja11xx_hwmon_register(struct phy_device *phydev, >> struct tja11xx_priv *priv) >> { >> - struct device *dev = &phydev->mdio.dev; >> - >> - priv->hwmon_name = devm_hwmon_sanitize_name(dev, dev_name(dev)); >> - if (IS_ERR(priv->hwmon_name)) >> - return PTR_ERR(priv->hwmon_name); >> - >> - priv->hwmon_dev = >> - devm_hwmon_device_register_with_info(dev, priv->hwmon_name, >> - phydev, >> - &tja11xx_hwmon_chip_info, >> - NULL); >> + struct device *hdev, *dev = &phydev->mdio.dev; >> >> - return PTR_ERR_OR_ZERO(priv->hwmon_dev); >> + hdev = devm_hwmon_device_register_with_info(dev, NULL, phydev, >> + &tja11xx_hwmon_chip_info, >> + NULL); >> + return PTR_ERR_OR_ZERO(hdev); >> } > > The change look correct to me, however I think you can go one step > further and remove the field tja11xx_priv.hwmon_name as well as > hwmon_dev. > This is part of the patch. Or what do you mean? > One could argue that we can even remove tja11xx_hwmon_register() > entirely > It's called from two places, and we would have to duplicate some things like IS_ERR(). I think it's ok to leave this function in. > Thanks, > > Maxime Heiner
On Fri, 14 Mar 2025 12:26:33 +0100 Heiner Kallweit <hkallweit1@gmail.com> wrote: > On 14.03.2025 08:45, Maxime Chevallier wrote: > > Hello Heiner, > > > > On Thu, 13 Mar 2025 20:45:06 +0100 > > Heiner Kallweit <hkallweit1@gmail.com> wrote: > > > >> Since c909e68f8127 ("hwmon: (core) Use device name as a fallback in > >> devm_hwmon_device_register_with_info") we can simply provide NULL > >> as name argument. > >> > >> Note that neither priv->hwmon_name nor priv->hwmon_dev are used > >> outside tja11xx_hwmon_register. > >> > >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > >> --- > >> drivers/net/phy/nxp-tja11xx.c | 19 +++++-------------- > >> 1 file changed, 5 insertions(+), 14 deletions(-) > >> > >> diff --git a/drivers/net/phy/nxp-tja11xx.c b/drivers/net/phy/nxp-tja11xx.c > >> index 601094fe2..07e94a247 100644 > >> --- a/drivers/net/phy/nxp-tja11xx.c > >> +++ b/drivers/net/phy/nxp-tja11xx.c > >> @@ -87,8 +87,6 @@ > >> #define TJA110X_RMII_MODE_REFCLK_IN BIT(0) > >> > >> struct tja11xx_priv { > >> - char *hwmon_name; > >> - struct device *hwmon_dev; > >> struct phy_device *phydev; > >> struct work_struct phy_register_work; > >> u32 flags; > >> @@ -508,19 +506,12 @@ static const struct hwmon_chip_info tja11xx_hwmon_chip_info = { > >> static int tja11xx_hwmon_register(struct phy_device *phydev, > >> struct tja11xx_priv *priv) > >> { > >> - struct device *dev = &phydev->mdio.dev; > >> - > >> - priv->hwmon_name = devm_hwmon_sanitize_name(dev, dev_name(dev)); > >> - if (IS_ERR(priv->hwmon_name)) > >> - return PTR_ERR(priv->hwmon_name); > >> - > >> - priv->hwmon_dev = > >> - devm_hwmon_device_register_with_info(dev, priv->hwmon_name, > >> - phydev, > >> - &tja11xx_hwmon_chip_info, > >> - NULL); > >> + struct device *hdev, *dev = &phydev->mdio.dev; > >> > >> - return PTR_ERR_OR_ZERO(priv->hwmon_dev); > >> + hdev = devm_hwmon_device_register_with_info(dev, NULL, phydev, > >> + &tja11xx_hwmon_chip_info, > >> + NULL); > >> + return PTR_ERR_OR_ZERO(hdev); > >> } > > > > The change look correct to me, however I think you can go one step > > further and remove the field tja11xx_priv.hwmon_name as well as > > hwmon_dev. > > > This is part of the patch. Or what do you mean? Uh you are correct :( meh OK sory for the noise then, morning coffee didn't go through entirely this morning it seems. Maxime
diff --git a/drivers/net/phy/nxp-tja11xx.c b/drivers/net/phy/nxp-tja11xx.c index 601094fe2..07e94a247 100644 --- a/drivers/net/phy/nxp-tja11xx.c +++ b/drivers/net/phy/nxp-tja11xx.c @@ -87,8 +87,6 @@ #define TJA110X_RMII_MODE_REFCLK_IN BIT(0) struct tja11xx_priv { - char *hwmon_name; - struct device *hwmon_dev; struct phy_device *phydev; struct work_struct phy_register_work; u32 flags; @@ -508,19 +506,12 @@ static const struct hwmon_chip_info tja11xx_hwmon_chip_info = { static int tja11xx_hwmon_register(struct phy_device *phydev, struct tja11xx_priv *priv) { - struct device *dev = &phydev->mdio.dev; - - priv->hwmon_name = devm_hwmon_sanitize_name(dev, dev_name(dev)); - if (IS_ERR(priv->hwmon_name)) - return PTR_ERR(priv->hwmon_name); - - priv->hwmon_dev = - devm_hwmon_device_register_with_info(dev, priv->hwmon_name, - phydev, - &tja11xx_hwmon_chip_info, - NULL); + struct device *hdev, *dev = &phydev->mdio.dev; - return PTR_ERR_OR_ZERO(priv->hwmon_dev); + hdev = devm_hwmon_device_register_with_info(dev, NULL, phydev, + &tja11xx_hwmon_chip_info, + NULL); + return PTR_ERR_OR_ZERO(hdev); } static int tja11xx_parse_dt(struct phy_device *phydev)
Since c909e68f8127 ("hwmon: (core) Use device name as a fallback in devm_hwmon_device_register_with_info") we can simply provide NULL as name argument. Note that neither priv->hwmon_name nor priv->hwmon_dev are used outside tja11xx_hwmon_register. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/net/phy/nxp-tja11xx.c | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-)