Re: [PATCH] phy: stih407-usb: Use syscon_regmap_lookup_by_phandle_args

[Patrice CHOTARD <patrice.chotard@foss.st.com>]

On 1/11/25 19:54, Krzysztof Kozlowski wrote:
> Use syscon_regmap_lookup_by_phandle_args() which is a wrapper over
> syscon_regmap_lookup_by_phandle() combined with getting the syscon
> argument.  Except simpler code this annotates within one line that given
> phandle has arguments, so grepping for code would be easier.
> 
> There is also no real benefit in printing errors on missing syscon
> argument, because this is done just too late: runtime check on
> static/build-time data.  Dtschema and Devicetree bindings offer the
> static/build-time check for this already.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/phy/st/phy-stih407-usb.c | 24 +++++++-----------------
>  1 file changed, 7 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/phy/st/phy-stih407-usb.c b/drivers/phy/st/phy-stih407-usb.c
> index a4ae2cca7f63..ebb1d0858aa3 100644
> --- a/drivers/phy/st/phy-stih407-usb.c
> +++ b/drivers/phy/st/phy-stih407-usb.c
> @@ -18,8 +18,8 @@
>  #include <linux/mfd/syscon.h>
>  #include <linux/phy/phy.h>
>  
> -#define PHYPARAM_REG	1
> -#define PHYCTRL_REG	2
> +#define PHYPARAM_REG	0
> +#define PHYCTRL_REG	1
>  
>  /* Default PHY_SEL and REFCLKSEL configuration */
>  #define STIH407_USB_PICOPHY_CTRL_PORT_CONF	0x6
> @@ -91,8 +91,8 @@ static int stih407_usb2_picophy_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct device_node *np = dev->of_node;
>  	struct phy_provider *phy_provider;
> +	unsigned int syscon_args[2];
>  	struct phy *phy;
> -	int ret;
>  
>  	phy_dev = devm_kzalloc(dev, sizeof(*phy_dev), GFP_KERNEL);
>  	if (!phy_dev)
> @@ -116,25 +116,15 @@ static int stih407_usb2_picophy_probe(struct platform_device *pdev)
>  	/* Reset port by default: only deassert it in phy init */
>  	reset_control_assert(phy_dev->rstport);
>  
> -	phy_dev->regmap = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
> +	phy_dev->regmap = syscon_regmap_lookup_by_phandle_args(np, "st,syscfg",
> +							       2, syscon_args);
>  	if (IS_ERR(phy_dev->regmap)) {
>  		dev_err(dev, "No syscfg phandle specified\n");
>  		return PTR_ERR(phy_dev->regmap);
>  	}
>  
> -	ret = of_property_read_u32_index(np, "st,syscfg", PHYPARAM_REG,
> -					&phy_dev->param);
> -	if (ret) {
> -		dev_err(dev, "can't get phyparam offset (%d)\n", ret);
> -		return ret;
> -	}
> -
> -	ret = of_property_read_u32_index(np, "st,syscfg", PHYCTRL_REG,
> -					&phy_dev->ctrl);
> -	if (ret) {
> -		dev_err(dev, "can't get phyctrl offset (%d)\n", ret);
> -		return ret;
> -	}
> +	phy_dev->param = syscon_args[PHYPARAM_REG];
> +	phy_dev->ctrl = syscon_args[PHYCTRL_REG];
>  
>  	phy = devm_phy_create(dev, NULL, &stih407_usb2_picophy_data);
>  	if (IS_ERR(phy)) {

Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>

Thanks
Patrice