Re: [PATCH v5 2/2] phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy

[Heiko Stübner]

Am Dienstag, 14. Juni 2016, 07:52:13 schrieb Guenter Roeck:
> On 06/12/2016 07:10 PM, Frank Wang wrote:
> > The newer SoCs (rk3366, rk3399) take a different usb-phy IP block
> > than rk3288 and before, and most of phy-related registers are also
> > different from the past, so a new phy driver is required necessarily.
> > 
> > Signed-off-by: Frank Wang <frank.wang@xxxxxxxxxxxxxx>
> > ---
> 
> [ ... ]
> 
> > +
> > +static int rockchip_usb2phy_resume(struct phy *phy)
> > +{
> > +	struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy);
> > +	struct rockchip_usb2phy *rphy = dev_get_drvdata(phy->dev.parent);
> > +	int ret;
> > +
> > +	dev_dbg(&rport->phy->dev, "port resume\n");
> > +
> > +	ret = clk_prepare_enable(rphy->clk480m);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = property_enable(rphy, &rport->port_cfg->phy_sus, false);
> > +	if (ret)
> > +		return ret;
> > +
> > +	rport->suspended = false;
> > +	return 0;
> > +}
> > +
> > +static int rockchip_usb2phy_suspend(struct phy *phy)
> > +{
> > +	struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy);
> > +	struct rockchip_usb2phy *rphy = dev_get_drvdata(phy->dev.parent);
> > +	int ret;
> > +
> > +	dev_dbg(&rport->phy->dev, "port suspend\n");
> > +
> > +	ret = property_enable(rphy, &rport->port_cfg->phy_sus, true);
> > +	if (ret)
> > +		return ret;
> > +
> > +	rport->suspended = true;
> > +	clk_disable_unprepare(rphy->clk480m);
> > +	return 0;
> > +}
> > +
> 
> I am still quite confused by the clock handling.
> 
> The above will be called for each instantiated phy (user, otg).
> Each time, clk_disable_unprepare() will be called. Yet, there
> is no matching clk_prepare_enable() call during initialization.
> 
> How does this work ?

the created clock gets the supplying clock as parent, see 

+       rphy->clk = of_clk_get_by_name(node, "phyclk");
+       if (IS_ERR(rphy->clk)) {
+               rphy->clk = NULL;
+               init.parent_names = NULL;
+               init.num_parents = 0;
+       } else {
+               clk_name = __clk_get_name(rphy->clk);
+               init.parent_names = &clk_name;
+               init.num_parents = 1;
+       }

that way when you enable the 480m clock from the phy, its parent will 
automatically get enabled as well. And of course due to refcounting in the 
clock framework, the 480m clock (and thus also its parent) will only get 
disabled once both the host and otg port have disabled it.