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()

+};

[...]

Reply via email to