On 3/10/20 2:51 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 <sean...@gmail.com> >>> Acked-by: Lukasz Majewski <lu...@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". > >
This was just rebased against u-boot/master. Should I be rebasing against a different branch? The commit I am working off is $ git show upstream/master commit 1efb9796f80e1394f080be1b4f3173ff108ad1f2 (upstream/master, upstream/WIP/03Mar2020, upstream/HEAD) Merge: 8aad16916d 9aa886cc0b Author: Tom Rini <tr...@konsulko.com> Date: Tue Mar 3 21:48:49 2020 -0500 Merge tag 'dm-pull-3mar20' of git://git.denx.de/u-boot-dm Fixes for power domain on device removal >> 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 >>>