| Message ID | 20200211060425.1619471-4-seanga2@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | riscv: Add Sipeed Maix support | expand |
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 at gmail.com> > Acked-by: Lukasz Majewski <lukma at denx.de> > --- > > Changes in v4: > - Lint > > Changes in v3: > - New > > drivers/clk/clk-uclass.c | 59 ++++++++++++++++++---------------------- > 1 file changed, 26 insertions(+), 33 deletions(-) > When I tried to apply this patch-set, there occur a conflict as below: 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:491 error: drivers/clk/clk-uclass.c: patch does not apply Patch failed at 0003 clk: Unconditionally recursively en-/dis-able clocks Thanks Rick > diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c > index 246b9c0ab8..c5f87fee72 100644 > --- a/drivers/clk/clk-uclass.c > +++ b/drivers/clk/clk-uclass.c > @@ -491,7 +491,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); > @@ -500,32 +499,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; > @@ -551,7 +547,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); > @@ -560,29 +555,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 >
On 2/18/20 2:20 AM, Rick Chen wrote: > 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 at gmail.com> >> Acked-by: Lukasz Majewski <lukma at denx.de> >> --- >> >> Changes in v4: >> - Lint >> >> Changes in v3: >> - New >> >> drivers/clk/clk-uclass.c | 59 ++++++++++++++++++---------------------- >> 1 file changed, 26 insertions(+), 33 deletions(-) >> > > When I tried to apply this patch-set, there occur a conflict as below: > > 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:491 > error: drivers/clk/clk-uclass.c: patch does not apply > Patch failed at 0003 clk: Unconditionally recursively en-/dis-able clocks > > Thanks > Rick Hm, perhaps you are applying it on a different commit. My series is based off of c00bd81ae0 Merge branch 'next' of https://gitlab.denx.de/u-boot/custodians/u-boot-mpc83xx What are you applying to? --Sean
Hi Sean > On 2/18/20 2:20 AM, Rick Chen wrote: > > 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 at gmail.com> > >> Acked-by: Lukasz Majewski <lukma at denx.de> > >> --- > >> > >> Changes in v4: > >> - Lint > >> > >> Changes in v3: > >> - New > >> > >> drivers/clk/clk-uclass.c | 59 ++++++++++++++++++---------------------- > >> 1 file changed, 26 insertions(+), 33 deletions(-) > >> > > > > When I tried to apply this patch-set, there occur a conflict as below: > > > > 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:491 > > error: drivers/clk/clk-uclass.c: patch does not apply > > Patch failed at 0003 clk: Unconditionally recursively en-/dis-able clocks > > > > Thanks > > Rick > > Hm, perhaps you are applying it on a different commit. My series is > based off of > > c00bd81ae0 Merge branch 'next' of https://gitlab.denx.de/u-boot/custodians/u-boot-mpc83xx > > What are you applying to? I applied to u-boot/master Thanks Rick > > --Sean
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index 246b9c0ab8..c5f87fee72 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -491,7 +491,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); @@ -500,32 +499,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; @@ -551,7 +547,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); @@ -560,29 +555,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; } }