[PATCH v2 12/17] clk: shmobile: mstp: Fix the is_enabled() operation

[PATCH v2 12/17] clk: shmobile: mstp: Fix the is_enabled() operation

[Geert Uytterhoeven]

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

The MSTP[SC]R registers have clock stop bits, not clock enable bits. The
bit value should thus be inverted in the is_enabled() operation.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Mike Turquette <mturquette@linaro.org>
---
This depends on the actual fixes, so either we need Mike's ack, or this has
to wait until the fixes have landed in mainline.

v2:
  - New

 drivers/clk/shmobile/clk-mstp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/shmobile/clk-mstp.c b/drivers/clk/shmobile/clk-mstp.c
index 2e5810c88d11..4f5e2c6ec853 100644
--- a/drivers/clk/shmobile/clk-mstp.c
+++ b/drivers/clk/shmobile/clk-mstp.c
@@ -112,7 +112,7 @@ static int cpg_mstp_clock_is_enabled(struct clk_hw *hw)
 	else
 		value = clk_readl(group->smstpcr);
 
-	return !!(value & BIT(clock->bit_index));
+	return !(value & BIT(clock->bit_index));
 }
 
 static const struct clk_ops cpg_mstp_clock_ops = {
-- 
1.7.9.5

Re: [PATCH v2 12/17] clk: shmobile: mstp: Fix the is_enabled() operation

[Laurent Pinchart]

Hi Geert,

On Tuesday 15 April 2014 14:34:08 Geert Uytterhoeven wrote:
> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> The MSTP[SC]R registers have clock stop bits, not clock enable bits. The
> bit value should thus be inverted in the is_enabled() operation.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Mike Turquette <mturquette@linaro.org>
> ---
> This depends on the actual fixes, so either we need Mike's ack, or this has
> to wait until the fixes have landed in mainline.

What's the status of this patch ? Do you still carry it in your tree and plan 
to push it upstream at some point ?

> v2:
>   - New
> 
>  drivers/clk/shmobile/clk-mstp.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/shmobile/clk-mstp.c
> b/drivers/clk/shmobile/clk-mstp.c index 2e5810c88d11..4f5e2c6ec853 100644
> --- a/drivers/clk/shmobile/clk-mstp.c
> +++ b/drivers/clk/shmobile/clk-mstp.c
> @@ -112,7 +112,7 @@ static int cpg_mstp_clock_is_enabled(struct clk_hw *hw)
>  	else
>  		value = clk_readl(group->smstpcr);
> 
> -	return !!(value & BIT(clock->bit_index));
> +	return !(value & BIT(clock->bit_index));
>  }
> 
>  static const struct clk_ops cpg_mstp_clock_ops = {

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 12/17] clk: shmobile: mstp: Fix the is_enabled() operation

[Geert Uytterhoeven]

Hi Laurent,

On Tue, May 13, 2014 at 12:20 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Tuesday 15 April 2014 14:34:08 Geert Uytterhoeven wrote:
>> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>>
>> The MSTP[SC]R registers have clock stop bits, not clock enable bits. The
>> bit value should thus be inverted in the is_enabled() operation.
>>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> Cc: Mike Turquette <mturquette@linaro.org>
>> ---
>> This depends on the actual fixes, so either we need Mike's ack, or this has
>> to wait until the fixes have landed in mainline.
>
> What's the status of this patch ? Do you still carry it in your tree and plan
> to push it upstream at some point ?

Yes, I still have it locally.

It depends on "[PATCH v4] drivers: sh: compile drivers/sh/pm_runtime.c
if ARCH_SHMOBILE_MULTI", so we have to wait until that one hits
mainline.

I think the only ill effects of not having it is that some clocks may not be
disabled properly?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 12/17] clk: shmobile: mstp: Fix the is_enabled() operation

[Laurent Pinchart]

On Tuesday 13 May 2014 08:47:29 Geert Uytterhoeven wrote:
> On Tue, May 13, 2014 at 12:20 AM, Laurent Pinchart wrote:
> > On Tuesday 15 April 2014 14:34:08 Geert Uytterhoeven wrote:
> >> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> >> 
> >> The MSTP[SC]R registers have clock stop bits, not clock enable bits. The
> >> bit value should thus be inverted in the is_enabled() operation.
> >> 
> >> Signed-off-by: Laurent Pinchart
> >> <laurent.pinchart+renesas@ideasonboard.com>
> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >> Cc: Mike Turquette <mturquette@linaro.org>
> >> ---
> >> This depends on the actual fixes, so either we need Mike's ack, or this
> >> has to wait until the fixes have landed in mainline.
> > 
> > What's the status of this patch ? Do you still carry it in your tree and
> > plan to push it upstream at some point ?
> 
> Yes, I still have it locally.
> 
> It depends on "[PATCH v4] drivers: sh: compile drivers/sh/pm_runtime.c
> if ARCH_SHMOBILE_MULTI", so we have to wait until that one hits
> mainline.
> 
> I think the only ill effects of not having it is that some clocks may not be
> disabled properly?

Correct. It's not so urgent, I was just going through my pending patches 
yesterday to make sure none got forgotten.

-- 
Regards,

Laurent Pinchart