[PATCH v6 03/19] clk: Unconditionally recursively en-/dis-able clocks
Sean Anderson
seanga2 at gmail.com
Tue Mar 10 14:14:13 CET 2020
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 <seanga2 at gmail.com>
>>> Acked-by: Lukasz Majewski <lukma at 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 <trini at 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
>>>
More information about the U-Boot
mailing list