[PATCH v6 03/19] clk: Unconditionally recursively en-/dis-able clocks

Rick Chen
2020-03-10T06:51:05Z

Hi Sean

> > For clocks not in the CCF, their parents will not have UCLASS_CLK, so we
> > just enable them as normal. The enable count is local to the struct clk,
> > but this will never result in the actual en-/dis-able op being called
> > (unless the same struct clk is enabled twice).
> >
> > For clocks in the CCF, we always traverse up the tree when enabling.
> > Previously, CCF clocks without id set would be skipped, stopping the
> > traversal too early.
> >
> > Signed-off-by: Sean Anderson <seanga2@gmail.com>
> > Acked-by: Lukasz Majewski <lukma@denx.de>
> > ---
> >
> > Changes in v5:
> > - Clear enable_count on request
> >
> > Changes in v4:
> > - Lint
> >
> > Changes in v3:
> > - New
> >
> > drivers/clk/clk-uclass.c | 60 ++++++++++++++++++----------------------
> > 1 file changed, 27 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> > index d497737298..3b711395b8 100644
> > --- a/drivers/clk/clk-uclass.c
> > +++ b/drivers/clk/clk-uclass.c
> > @@ -410,6 +410,7 @@ int clk_request(struct udevice *dev, struct clk *clk)
> > ops = clk_dev_ops(dev);
> >
> > clk->dev = dev;
> > + clk->enable_count = 0;
>
> Lukasz has taged Acked-by in v3.
> Re: [PATCH v3 03/12] clk: Unconditionally recursively en-/dis-able clocks
>
> This patch shall be fixed and freeze.
> But you add a new modification (Clear enable_count on request)in v5 and v6.
> Although it seem to look fine and nothing big deal.
> I just highlight it here, and if Lukasz have no other comment further.
> I will pull it via riscv tree later.
>

I have told you in v5, that this patch have conflict with
u-boot/master please rebase it.
But it still conflict in v6.

https://patchwork.ozlabs.org/patch/1246836/

Applying: clk: Always use the supplied struct clk
Applying: clk: Check that ops of composite clock components exist before calling
Applying: clk: Unconditionally recursively en-/dis-able clocks
error: patch failed: drivers/clk/clk-uclass.c:522
error: drivers/clk/clk-uclass.c: patch does not apply
Patch failed at 0003 clk: Unconditionally recursively en-/dis-able clocks
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

> Thanks,
> Rick
>
>
>
> >
> > if (!ops->request)
> > return 0;
> > @@ -522,7 +523,6 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
> > int clk_enable(struct clk *clk)
> > {
> > const struct clk_ops *ops;
> > - struct clk *clkp = NULL;
> > int ret;
> >
> > debug("%s(clk=%p \"%s\")\n", __func__, clk, clk->dev->name);
> > @@ -531,32 +531,29 @@ int clk_enable(struct clk *clk)
> > ops = clk_dev_ops(clk->dev);
> >
> > if (CONFIG_IS_ENABLED(CLK_CCF)) {
> > - /* Take id 0 as a non-valid clk, such as dummy */
> > - if (clk->id && !clk_get_by_id(clk->id, &clkp)) {
> > - if (clkp->enable_count) {
> > - clkp->enable_count++;
> > - return 0;
> > - }
> > - if (clkp->dev->parent &&
> > - device_get_uclass_id(clkp->dev) == UCLASS_CLK) {
> > - ret = clk_enable(dev_get_clk_ptr(clkp->dev->parent));
> > - if (ret) {
> > - printf("Enable %s failed\n",
> > - clkp->dev->parent->name);
> > - return ret;
> > - }
> > + if (clk->enable_count) {
> > + clk->enable_count++;
> > + return 0;
> > + }
> > + if (clk->dev->parent &&
> > + device_get_uclass_id(clk->dev->parent) == UCLASS_CLK) {
> > + ret = clk_enable(dev_get_clk_ptr(clk->dev->parent));
> > + if (ret) {
> > + printf("Enable %s failed\n",
> > + clk->dev->parent->name);
> > + return ret;
> > }
> > }
> >
> > if (ops->enable) {
> > ret = ops->enable(clk);
> > if (ret) {
> > - printf("Enable %s failed\n", clk->dev->name);
> > + printf("Enable %s failed (error %d)\n",
> > + clk->dev->name, ret);
> > return ret;
> > }
> > }
> > - if (clkp)
> > - clkp->enable_count++;
> > + clk->enable_count++;
> > } else {
> > if (!ops->enable)
> > return -ENOSYS;
> > @@ -582,7 +579,6 @@ int clk_enable_bulk(struct clk_bulk *bulk)
> > int clk_disable(struct clk *clk)
> > {
> > const struct clk_ops *ops;
> > - struct clk *clkp = NULL;
> > int ret;
> >
> > debug("%s(clk=%p)\n", __func__, clk);
> > @@ -591,29 +587,27 @@ int clk_disable(struct clk *clk)
> > ops = clk_dev_ops(clk->dev);
> >
> > if (CONFIG_IS_ENABLED(CLK_CCF)) {
> > - if (clk->id && !clk_get_by_id(clk->id, &clkp)) {
> > - if (clkp->enable_count == 0) {
> > - printf("clk %s already disabled\n",
> > - clkp->dev->name);
> > - return 0;
> > - }
> > -
> > - if (--clkp->enable_count > 0)
> > - return 0;
> > + if (clk->enable_count == 0) {
> > + printf("clk %s already disabled\n",
> > + clk->dev->name);
> > + return 0;
> > }
> >
> > + if (--clk->enable_count > 0)
> > + return 0;
> > +
> > if (ops->disable) {
> > ret = ops->disable(clk);
> > if (ret)
> > return ret;
> > }
> >
> > - if (clkp && clkp->dev->parent &&
> > - device_get_uclass_id(clkp->dev) == UCLASS_CLK) {
> > - ret = clk_disable(dev_get_clk_ptr(clkp->dev->parent));
> > + if (clk->dev->parent &&
> > + device_get_uclass_id(clk->dev) == UCLASS_CLK) {
> > + ret = clk_disable(dev_get_clk_ptr(clk->dev->parent));
> > if (ret) {
> > - printf("Disable %s failed\n",
> > - clkp->dev->parent->name);
> > + printf("Disable %s failed (error %d)\n",
> > + clk->dev->parent->name, ret);
> > return ret;
> > }
> > }
> > --
> > 2.25.0
> >