[PATCH v3 08/10] phy: socionext: Add UniPhier USB3 PHY driver

[Marek Vasut]

On 2/8/23 10:15, Kunihiko Hayashi wrote:

[...]

> +static int uniphier_usb3phy_init(struct phy *phy)
> +{
> +	struct uniphier_usb3phy_priv *priv = dev_get_priv(phy->dev);
> +	int ret;
> +
> +	ret = clk_enable_bulk(&priv->clks);
> +	if (ret)
> +		goto out_clk;

This should be just 'return ret;'

> +	ret = reset_deassert_bulk(&priv->rsts);
> +	if (ret)
> +		goto out_rst;

This should be goto out_rst, however ...

> +	return 0;
> +
> +out_rst:
> +	reset_release_bulk(&priv->rsts);
> +out_clk:
> +	clk_release_bulk(&priv->clks);

... the out_rst: should only do:

out_rst:
   clk_disable_bulk();
   return ret

out_clk part can be removed.

> +	return ret;
> +}
> +
> +static int uniphier_usb3phy_probe(struct udevice *dev)
> +{
> +	struct uniphier_usb3phy_priv *priv = dev_get_priv(dev);
> +	int ret;
> +
> +	ret = clk_get_bulk(dev, &priv->clks);
> +	if (ret) {
> +		if (ret != -ENOSYS && ret != -ENOENT) {

This can be single line:

if (ret && ret != -ENOSYS && ret != -ENOENT)
   return ret;

> +			printf("Failed to get clocks\n");
> +			return ret;
> +		}
> +	}
> +
> +	ret = reset_get_bulk(dev, &priv->rsts);
> +	if (ret) {
> +		if (ret != -ENOSYS && ret != -ENOENT) {

Same here.

> +			printf("Failed to get resets\n");
> +			clk_release_bulk(&priv->clks);

Better use goto out_clk fail path.

> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static struct phy_ops uniphier_usb3phy_ops = {
> +	.init = uniphier_usb3phy_init,

You should implement .exit callback too, that one should do these two steps:

reset_assert_bulk()
clk_disable_bulk()

> +};

[...]