On 2/13/23 04:08, Kunihiko Hayashi wrote:
Hi Marek,

Hello Hayashi-san,

On 2023/02/10 23:09, Marek Vasut wrote:
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.

I see. These operations are unpaired.
I'll fix them.

Thank you

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

I think so, however, when I added .exit() and executed "usb stop;usb start",
unfortunately the command got stuck.

Currently uniphier clk doesn't support CLK_CCF and can't be nested.
I think more changes are needed.

Do you know where exactly the system hangs ?

Do you expect to add the CCF support ?

If so, then add at least a FIXME code comment that the exit callback must be implemented, so that this is not forgotten once CCF is in place. I don't want this series to grow much further into some "rework everything at once" thing.

Reply via email to