* [PATCH v2] pinctrl: pinctrl-single: Fix pcs_parse_bits_in_pinctrl_entry to use __ffs than ffs
@ 2016-04-14 4:59 Keerthy
2016-04-14 16:02 ` Tony Lindgren
2016-04-15 9:27 ` Linus Walleij
0 siblings, 2 replies; 6+ messages in thread
From: Keerthy @ 2016-04-14 4:59 UTC (permalink / raw)
To: tony
Cc: linus.walleij, linux-arm-kernel, linux-omap, linux-gpio,
linux-kernel, j-keerthy, t-kristo, nsekhar
pcs_parse_bits_in_pinctrl_entry uses ffs which gives bit indices
ranging from 1 to MAX. This leads to a corner case where we try to request
the pin number = MAX and fails.
bit_pos value is being calculted using ffs. pin_num_from_lsb uses
bit_pos value. pins array is populated with:
pin + pin_num_from_lsb.
The above is 1 more than usual bit indices as bit_pos uses ffs to compute
first set bit. Hence the last of the pins array is populated with the MAX
value and not MAX - 1 which causes error when we call pin_request.
mask_pos is rightly calculated as ((pcs->fmask) << (bit_pos - 1))
Consequently val_pos and submask are correct.
Hence use __ffs which gives (ffs(x) - 1) as the first bit set.
fixes: 4e7e8017a8 ("pinctrl: pinctrl-single: enhance to configure multiple pins of different modules")
Signed-off-by: Keerthy <j-keerthy@ti.com>
---
Changes in v2:
* Changed pcs->fshift to use __ffs instead of ffs to be consistent.
Boot tesed on da850-evm and checked the pinctrl sysfs nodes.
drivers/pinctrl/pinctrl-single.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index fb126d5..cf9bafa 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -1280,9 +1280,9 @@ static int pcs_parse_bits_in_pinctrl_entry(struct pcs_device *pcs,
/* Parse pins in each row from LSB */
while (mask) {
- bit_pos = ffs(mask);
+ bit_pos = __ffs(mask);
pin_num_from_lsb = bit_pos / pcs->bits_per_pin;
- mask_pos = ((pcs->fmask) << (bit_pos - 1));
+ mask_pos = ((pcs->fmask) << bit_pos);
val_pos = val & mask_pos;
submask = mask & mask_pos;
@@ -1852,7 +1852,7 @@ static int pcs_probe(struct platform_device *pdev)
ret = of_property_read_u32(np, "pinctrl-single,function-mask",
&pcs->fmask);
if (!ret) {
- pcs->fshift = ffs(pcs->fmask) - 1;
+ pcs->fshift = __ffs(pcs->fmask);
pcs->fmax = pcs->fmask >> pcs->fshift;
} else {
/* If mask property doesn't exist, function mux is invalid. */
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] pinctrl: pinctrl-single: Fix pcs_parse_bits_in_pinctrl_entry to use __ffs than ffs
2016-04-14 4:59 [PATCH v2] pinctrl: pinctrl-single: Fix pcs_parse_bits_in_pinctrl_entry to use __ffs than ffs Keerthy
@ 2016-04-14 16:02 ` Tony Lindgren
2016-04-15 9:27 ` Linus Walleij
1 sibling, 0 replies; 6+ messages in thread
From: Tony Lindgren @ 2016-04-14 16:02 UTC (permalink / raw)
To: Keerthy
Cc: linus.walleij, linux-arm-kernel, linux-omap, linux-gpio,
linux-kernel, t-kristo, nsekhar
* Keerthy <j-keerthy@ti.com> [160413 22:00]:
> pcs_parse_bits_in_pinctrl_entry uses ffs which gives bit indices
> ranging from 1 to MAX. This leads to a corner case where we try to request
> the pin number = MAX and fails.
>
> bit_pos value is being calculted using ffs. pin_num_from_lsb uses
> bit_pos value. pins array is populated with:
>
> pin + pin_num_from_lsb.
>
> The above is 1 more than usual bit indices as bit_pos uses ffs to compute
> first set bit. Hence the last of the pins array is populated with the MAX
> value and not MAX - 1 which causes error when we call pin_request.
>
> mask_pos is rightly calculated as ((pcs->fmask) << (bit_pos - 1))
> Consequently val_pos and submask are correct.
>
> Hence use __ffs which gives (ffs(x) - 1) as the first bit set.
>
> fixes: 4e7e8017a8 ("pinctrl: pinctrl-single: enhance to configure multiple pins of different modules")
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
>
> Changes in v2:
>
> * Changed pcs->fshift to use __ffs instead of ffs to be consistent.
Thanks for updating it, looks good to me and still works here:
Acked-by: Tony Lindgren <tony@atomide.com>
> Boot tesed on da850-evm and checked the pinctrl sysfs nodes.
>
> drivers/pinctrl/pinctrl-single.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
> index fb126d5..cf9bafa 100644
> --- a/drivers/pinctrl/pinctrl-single.c
> +++ b/drivers/pinctrl/pinctrl-single.c
> @@ -1280,9 +1280,9 @@ static int pcs_parse_bits_in_pinctrl_entry(struct pcs_device *pcs,
>
> /* Parse pins in each row from LSB */
> while (mask) {
> - bit_pos = ffs(mask);
> + bit_pos = __ffs(mask);
> pin_num_from_lsb = bit_pos / pcs->bits_per_pin;
> - mask_pos = ((pcs->fmask) << (bit_pos - 1));
> + mask_pos = ((pcs->fmask) << bit_pos);
> val_pos = val & mask_pos;
> submask = mask & mask_pos;
>
> @@ -1852,7 +1852,7 @@ static int pcs_probe(struct platform_device *pdev)
> ret = of_property_read_u32(np, "pinctrl-single,function-mask",
> &pcs->fmask);
> if (!ret) {
> - pcs->fshift = ffs(pcs->fmask) - 1;
> + pcs->fshift = __ffs(pcs->fmask);
> pcs->fmax = pcs->fmask >> pcs->fshift;
> } else {
> /* If mask property doesn't exist, function mux is invalid. */
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] pinctrl: pinctrl-single: Fix pcs_parse_bits_in_pinctrl_entry to use __ffs than ffs
2016-04-14 4:59 [PATCH v2] pinctrl: pinctrl-single: Fix pcs_parse_bits_in_pinctrl_entry to use __ffs than ffs Keerthy
2016-04-14 16:02 ` Tony Lindgren
@ 2016-04-15 9:27 ` Linus Walleij
2016-04-15 15:22 ` Tony Lindgren
1 sibling, 1 reply; 6+ messages in thread
From: Linus Walleij @ 2016-04-15 9:27 UTC (permalink / raw)
To: Keerthy
Cc: Tony Lindgren, linux-arm-kernel@lists.infradead.org, Linux-OMAP,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
Tero Kristo, Sekhar Nori
On Thu, Apr 14, 2016 at 6:59 AM, Keerthy <j-keerthy@ti.com> wrote:
> pcs_parse_bits_in_pinctrl_entry uses ffs which gives bit indices
> ranging from 1 to MAX. This leads to a corner case where we try to request
> the pin number = MAX and fails.
>
> bit_pos value is being calculted using ffs. pin_num_from_lsb uses
> bit_pos value. pins array is populated with:
>
> pin + pin_num_from_lsb.
>
> The above is 1 more than usual bit indices as bit_pos uses ffs to compute
> first set bit. Hence the last of the pins array is populated with the MAX
> value and not MAX - 1 which causes error when we call pin_request.
>
> mask_pos is rightly calculated as ((pcs->fmask) << (bit_pos - 1))
> Consequently val_pos and submask are correct.
>
> Hence use __ffs which gives (ffs(x) - 1) as the first bit set.
>
> fixes: 4e7e8017a8 ("pinctrl: pinctrl-single: enhance to configure multiple pins of different modules")
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
>
> Changes in v2:
>
> * Changed pcs->fshift to use __ffs instead of ffs to be consistent.
>
> Boot tesed on da850-evm and checked the pinctrl sysfs nodes.
Patch applied for fixes with Tony's ACK.
Should it also be tagged for stable?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] pinctrl: pinctrl-single: Fix pcs_parse_bits_in_pinctrl_entry to use __ffs than ffs
2016-04-15 9:27 ` Linus Walleij
@ 2016-04-15 15:22 ` Tony Lindgren
2016-04-23 9:45 ` Linus Walleij
0 siblings, 1 reply; 6+ messages in thread
From: Tony Lindgren @ 2016-04-15 15:22 UTC (permalink / raw)
To: Linus Walleij
Cc: Keerthy, linux-arm-kernel@lists.infradead.org, Linux-OMAP,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
Tero Kristo, Sekhar Nori
* Linus Walleij <linus.walleij@linaro.org> [160415 02:29]:
> On Thu, Apr 14, 2016 at 6:59 AM, Keerthy <j-keerthy@ti.com> wrote:
>
> > pcs_parse_bits_in_pinctrl_entry uses ffs which gives bit indices
> > ranging from 1 to MAX. This leads to a corner case where we try to request
> > the pin number = MAX and fails.
> >
> > bit_pos value is being calculted using ffs. pin_num_from_lsb uses
> > bit_pos value. pins array is populated with:
> >
> > pin + pin_num_from_lsb.
> >
> > The above is 1 more than usual bit indices as bit_pos uses ffs to compute
> > first set bit. Hence the last of the pins array is populated with the MAX
> > value and not MAX - 1 which causes error when we call pin_request.
> >
> > mask_pos is rightly calculated as ((pcs->fmask) << (bit_pos - 1))
> > Consequently val_pos and submask are correct.
> >
> > Hence use __ffs which gives (ffs(x) - 1) as the first bit set.
> >
> > fixes: 4e7e8017a8 ("pinctrl: pinctrl-single: enhance to configure multiple pins of different modules")
> > Signed-off-by: Keerthy <j-keerthy@ti.com>
> > ---
> >
> > Changes in v2:
> >
> > * Changed pcs->fshift to use __ffs instead of ffs to be consistent.
> >
> > Boot tesed on da850-evm and checked the pinctrl sysfs nodes.
>
> Patch applied for fixes with Tony's ACK.
>
> Should it also be tagged for stable?
Probably a good idea, I can see somebody pulling hair out because
of this in various product trees.
Regards,
Tony
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] pinctrl: pinctrl-single: Fix pcs_parse_bits_in_pinctrl_entry to use __ffs than ffs
2016-04-15 15:22 ` Tony Lindgren
@ 2016-04-23 9:45 ` Linus Walleij
2016-04-26 16:17 ` Tony Lindgren
0 siblings, 1 reply; 6+ messages in thread
From: Linus Walleij @ 2016-04-23 9:45 UTC (permalink / raw)
To: Tony Lindgren
Cc: Keerthy, linux-arm-kernel@lists.infradead.org, Linux-OMAP,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
Tero Kristo, Sekhar Nori
On Fri, Apr 15, 2016 at 5:22 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Linus Walleij <linus.walleij@linaro.org> [160415 02:29]:
>> On Thu, Apr 14, 2016 at 6:59 AM, Keerthy <j-keerthy@ti.com> wrote:
>>
>> > pcs_parse_bits_in_pinctrl_entry uses ffs which gives bit indices
>> > ranging from 1 to MAX. This leads to a corner case where we try to request
>> > the pin number = MAX and fails.
>> >
>> > bit_pos value is being calculted using ffs. pin_num_from_lsb uses
>> > bit_pos value. pins array is populated with:
>> >
>> > pin + pin_num_from_lsb.
>> >
>> > The above is 1 more than usual bit indices as bit_pos uses ffs to compute
>> > first set bit. Hence the last of the pins array is populated with the MAX
>> > value and not MAX - 1 which causes error when we call pin_request.
>> >
>> > mask_pos is rightly calculated as ((pcs->fmask) << (bit_pos - 1))
>> > Consequently val_pos and submask are correct.
>> >
>> > Hence use __ffs which gives (ffs(x) - 1) as the first bit set.
>> >
>> > fixes: 4e7e8017a8 ("pinctrl: pinctrl-single: enhance to configure multiple pins of different modules")
>> > Signed-off-by: Keerthy <j-keerthy@ti.com>
>> > ---
>> >
>> > Changes in v2:
>> >
>> > * Changed pcs->fshift to use __ffs instead of ffs to be consistent.
>> >
>> > Boot tesed on da850-evm and checked the pinctrl sysfs nodes.
>>
>> Patch applied for fixes with Tony's ACK.
>>
>> Should it also be tagged for stable?
>
> Probably a good idea, I can see somebody pulling hair out because
> of this in various product trees.
Ooops sorry I totally missed to add that :(
Please ask Greg to take it as a selected stable patch.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] pinctrl: pinctrl-single: Fix pcs_parse_bits_in_pinctrl_entry to use __ffs than ffs
2016-04-23 9:45 ` Linus Walleij
@ 2016-04-26 16:17 ` Tony Lindgren
0 siblings, 0 replies; 6+ messages in thread
From: Tony Lindgren @ 2016-04-26 16:17 UTC (permalink / raw)
To: Linus Walleij
Cc: Keerthy, linux-arm-kernel@lists.infradead.org, Linux-OMAP,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
Tero Kristo, Sekhar Nori
* Linus Walleij <linus.walleij@linaro.org> [160423 02:46]:
> On Fri, Apr 15, 2016 at 5:22 PM, Tony Lindgren <tony@atomide.com> wrote:
> > * Linus Walleij <linus.walleij@linaro.org> [160415 02:29]:
> >> Patch applied for fixes with Tony's ACK.
> >>
> >> Should it also be tagged for stable?
> >
> > Probably a good idea, I can see somebody pulling hair out because
> > of this in various product trees.
>
> Ooops sorry I totally missed to add that :(
>
> Please ask Greg to take it as a selected stable patch.
Well it has the fixes tag, let's see if the stable people will
find it automatically now :) If not, Keerthy can send email
requesting adding it manually to stable trees.
Tony
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-04-26 16:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-14 4:59 [PATCH v2] pinctrl: pinctrl-single: Fix pcs_parse_bits_in_pinctrl_entry to use __ffs than ffs Keerthy
2016-04-14 16:02 ` Tony Lindgren
2016-04-15 9:27 ` Linus Walleij
2016-04-15 15:22 ` Tony Lindgren
2016-04-23 9:45 ` Linus Walleij
2016-04-26 16:17 ` Tony Lindgren
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).