All of lore.kernel.org
 help / color / mirror / Atom feed
From: mturquette@linaro.org (Mike Turquette)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3] clk_mux: Fix set_parent doing the wrong thing when INDEX_BIT && index >= 3
Date: Wed, 19 Nov 2014 14:04:49 -0800	[thread overview]
Message-ID: <20141119220449.25314.56163@quantum> (raw)
In-Reply-To: <1416404939-609-1-git-send-email-hdegoede@redhat.com>

Quoting Hans de Goede (2014-11-19 05:48:59)
> If CLK_MUX_INDEX_BIT is set, then each bit turns on / off a single parent,
> so theoretically multiple parents could be enabled at the same time, but in
> practice only one bit should ever be 1. So to select parent 0, set
> the register (*) to 0x01, to select parent 1 set it 0x02, parent 2, 0x04,
> parent 3, 0x08, etc.
> 
> But the current code does:
> 
>                 if (mux->flags & CLK_MUX_INDEX_BIT)
>                         index = (1 << ffs(index));
> 
> Which means that:
> 
> For an input index of 0, ffs returns 0, so we set the register
> to 0x01, ok.
> 
> For an input index of 1, ffs returns 1, so we set the register
> to 0x02, ok.
> 
> For an input index of 2, ffs returns 2, so we set the register
> to 0x04, ok.
> 
> For an input index of 3, ffs returns 1, so we set the register
> to 0x02, not good!
> 
> The code should simply be:
> 
>                 if (mux->flags & CLK_MUX_INDEX_BIT)
>                         index = 1 << index;
> 
> Which always does the right thing, this commit fixes this.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Applied to clk-next towards 3.19. I don't think any merged platforms are
hitting this bug in the field so I've not put it into -fixes.

Thanks,
Mike

> ---
>  drivers/clk/clk-mux.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
> index 4f96ff3..6e1ecf9 100644
> --- a/drivers/clk/clk-mux.c
> +++ b/drivers/clk/clk-mux.c
> @@ -77,7 +77,7 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
>  
>         else {
>                 if (mux->flags & CLK_MUX_INDEX_BIT)
> -                       index = (1 << ffs(index));
> +                       index = 1 << index;
>  
>                 if (mux->flags & CLK_MUX_INDEX_ONE)
>                         index++;
> -- 
> 2.1.0
> 

WARNING: multiple messages have this Message-ID (diff)
From: Mike Turquette <mturquette-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Philipp Zabel
	<philipp.zabel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Grant Likely
	<grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org>,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org,
	Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Subject: Re: [PATCH v3] clk_mux: Fix set_parent doing the wrong thing when INDEX_BIT && index >= 3
Date: Wed, 19 Nov 2014 14:04:49 -0800	[thread overview]
Message-ID: <20141119220449.25314.56163@quantum> (raw)
In-Reply-To: <1416404939-609-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Quoting Hans de Goede (2014-11-19 05:48:59)
> If CLK_MUX_INDEX_BIT is set, then each bit turns on / off a single parent,
> so theoretically multiple parents could be enabled at the same time, but in
> practice only one bit should ever be 1. So to select parent 0, set
> the register (*) to 0x01, to select parent 1 set it 0x02, parent 2, 0x04,
> parent 3, 0x08, etc.
> 
> But the current code does:
> 
>                 if (mux->flags & CLK_MUX_INDEX_BIT)
>                         index = (1 << ffs(index));
> 
> Which means that:
> 
> For an input index of 0, ffs returns 0, so we set the register
> to 0x01, ok.
> 
> For an input index of 1, ffs returns 1, so we set the register
> to 0x02, ok.
> 
> For an input index of 2, ffs returns 2, so we set the register
> to 0x04, ok.
> 
> For an input index of 3, ffs returns 1, so we set the register
> to 0x02, not good!
> 
> The code should simply be:
> 
>                 if (mux->flags & CLK_MUX_INDEX_BIT)
>                         index = 1 << index;
> 
> Which always does the right thing, this commit fixes this.
> 
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Applied to clk-next towards 3.19. I don't think any merged platforms are
hitting this bug in the field so I've not put it into -fixes.

Thanks,
Mike

> ---
>  drivers/clk/clk-mux.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
> index 4f96ff3..6e1ecf9 100644
> --- a/drivers/clk/clk-mux.c
> +++ b/drivers/clk/clk-mux.c
> @@ -77,7 +77,7 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
>  
>         else {
>                 if (mux->flags & CLK_MUX_INDEX_BIT)
> -                       index = (1 << ffs(index));
> +                       index = 1 << index;
>  
>                 if (mux->flags & CLK_MUX_INDEX_ONE)
>                         index++;
> -- 
> 2.1.0
> 

  reply	other threads:[~2014-11-19 22:04 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-19 13:48 [PATCH v3] clk_mux: Fix set_parent doing the wrong thing when INDEX_BIT && index >= 3 Hans de Goede
2014-11-19 13:48 ` Hans de Goede
2014-11-19 22:04 ` Mike Turquette [this message]
2014-11-19 22:04   ` Mike Turquette

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141119220449.25314.56163@quantum \
    --to=mturquette@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.