Because right now, those clk_* functions are looking pretty similar
but behave differently. And looking at the function, the enable-variant
also just uses clk_bulk_get_all, just looses the number of clocks
because "ret" is reused by the enable call.
But if you don't want to touch the clk API that's also ok, but
you should then add a devm_add_action_or_reset call, to disable
the clocks on driver removal, before they are freed.
> >> if (rphy->phy_cfg->phy_tuning) {
> >> ret = rphy->phy_cfg->phy_tuning(rphy);
> >> if (ret)
> >> - return ret;
> >> + goto disable_clks;
> >> }
> >>
> >> index = 0;
> >> @@ -1484,6 +1502,8 @@ static int rockchip_usb2phy_probe(struct platform_device *pdev)
> >>
> >> put_child:
> >> of_node_put(child_np);
> >> +disable_clks:
> >> + clk_bulk_disable_unprepare(rphy->num_clks, rphy->clks);
> >> return ret;
> >> }
> >>
> >> @@ -1495,6 +1515,30 @@ static int rk3128_usb2phy_tuning(struct rockchip_usb2phy *rphy)
> >> BIT(2) << BIT_WRITEABLE_SHIFT | 0);
> >> }
> >
> > I still maintain, that this maybe should be two separate patches.
> > And you even seem to have nice "cut-here" in it ;-)
> >
> > The code above converts the driver to use clk_bulk to handle
> > multiple clocks for controller variants needing it.
> >
> >
> > And the code below adds the rk3576-specific data to the driver.
> >
>
> Make sense, I shall split two separate patches in the next version.
> BTW, should I send the clk_bulk process patch as a separate series? I
> mean I shouldn't send it together with this series, right?
No, just one series, convert the driver to clk_bulk*, add rk3576 binding,
add rk3576-specific data :-) .
Because people will want to see where you're going with your change
and having "this series depends on this separate patch" creates confusion
and you're really only touching this one driver.
Heiko
