This fixes pcs_request_gpio() in the pinctrl-single driver when
bits_per_mux != 0. It appears this was overlooked when the multiple
pins per register feature was added.
Fixes: 4e7e8017a80e ("pinctrl: pinctrl-single: enhance to configure multiple pins of different modules")
Signed-off-by: David Lechner <[email protected]>
---
v2 changes:
- don't wrap Fixes: line in commit message since it is a special machine-
readable line.
There was some discussion in v1 about using DIV_ROUND_UP(), etc. macros, but
the consensus was to leave it as-is since it matches existing code and that
macros can be introduced in another patch.
drivers/pinctrl/pinctrl-single.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index cec7537..a7c5eb3 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -391,9 +391,25 @@ static int pcs_request_gpio(struct pinctrl_dev *pctldev,
|| pin < frange->offset)
continue;
mux_bytes = pcs->width / BITS_PER_BYTE;
- data = pcs->read(pcs->base + pin * mux_bytes) & ~pcs->fmask;
- data |= frange->gpiofunc;
- pcs->write(data, pcs->base + pin * mux_bytes);
+
+ if (pcs->bits_per_mux) {
+ int byte_num, offset, pin_shift;
+
+ byte_num = (pcs->bits_per_pin * pin) / BITS_PER_BYTE;
+ offset = (byte_num / mux_bytes) * mux_bytes;
+ pin_shift = pin % (pcs->width / pcs->bits_per_pin) *
+ pcs->bits_per_pin;
+
+ data = pcs->read(pcs->base + offset);
+ data &= ~(pcs->fmask << pin_shift);
+ data |= frange->gpiofunc << pin_shift;
+ pcs->write(data, pcs->base + offset);
+ } else {
+ data = pcs->read(pcs->base + pin * mux_bytes);
+ data &= ~pcs->fmask;
+ data |= frange->gpiofunc;
+ pcs->write(data, pcs->base + pin * mux_bytes);
+ }
break;
}
return 0;
--
2.7.4
On Mon, Feb 19, 2018 at 11:57 PM, David Lechner <[email protected]> wrote:
> This fixes pcs_request_gpio() in the pinctrl-single driver when
> bits_per_mux != 0. It appears this was overlooked when the multiple
> pins per register feature was added.
>
> Fixes: 4e7e8017a80e ("pinctrl: pinctrl-single: enhance to configure multiple pins of different modules")
> Signed-off-by: David Lechner <[email protected]>
> ---
>
> v2 changes:
> - don't wrap Fixes: line in commit message since it is a special machine-
> readable line.
>
> There was some discussion in v1 about using DIV_ROUND_UP(), etc. macros, but
> the consensus was to leave it as-is since it matches existing code and that
> macros can be introduced in another patch.
>
> drivers/pinctrl/pinctrl-single.c | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
> index cec7537..a7c5eb3 100644
> --- a/drivers/pinctrl/pinctrl-single.c
> +++ b/drivers/pinctrl/pinctrl-single.c
> @@ -391,9 +391,25 @@ static int pcs_request_gpio(struct pinctrl_dev *pctldev,
> || pin < frange->offset)
> continue;
> mux_bytes = pcs->width / BITS_PER_BYTE;
> - data = pcs->read(pcs->base + pin * mux_bytes) & ~pcs->fmask;
> - data |= frange->gpiofunc;
> - pcs->write(data, pcs->base + pin * mux_bytes);
> +
> + if (pcs->bits_per_mux) {
> + int byte_num, offset, pin_shift;
> +
> + byte_num = (pcs->bits_per_pin * pin) / BITS_PER_BYTE;
> + offset = (byte_num / mux_bytes) * mux_bytes;
> + pin_shift = pin % (pcs->width / pcs->bits_per_pin) *
> + pcs->bits_per_pin;
> +
> + data = pcs->read(pcs->base + offset);
> + data &= ~(pcs->fmask << pin_shift);
> + data |= frange->gpiofunc << pin_shift;
> + pcs->write(data, pcs->base + offset);
> + } else {
> + data = pcs->read(pcs->base + pin * mux_bytes);
> + data &= ~pcs->fmask;
> + data |= frange->gpiofunc;
> + pcs->write(data, pcs->base + pin * mux_bytes);
Just an idea, you may leave this almost untouched and do calculate
pin_shift and offset in condition, like
if (...) {
pin_shift = ...
offset = ...
} else {
pin_shift = 0;
offset = pin * mux_bytes;
}
data = pcs->read(pcs->base + offset);
data &= ~(pcs->fmask << pin_shift);
data |= frange->gpiofunc << pin_shift;
pcs->write(data, pcs->base + offset);
It's also possible to split to two changes, where first introduces
that variables and their default values (see 'else' branch) and second
one introduces an if branch override.
> + }
> break;
--
With Best Regards,
Andy Shevchenko
On 02/20/2018 06:56 AM, Andy Shevchenko wrote:
> On Mon, Feb 19, 2018 at 11:57 PM, David Lechner <[email protected]> wrote:
>> This fixes pcs_request_gpio() in the pinctrl-single driver when
>> bits_per_mux != 0. It appears this was overlooked when the multiple
>> pins per register feature was added.
>>
>> Fixes: 4e7e8017a80e ("pinctrl: pinctrl-single: enhance to configure multiple pins of different modules")
>> Signed-off-by: David Lechner <[email protected]>
>> ---
>>
>> v2 changes:
>> - don't wrap Fixes: line in commit message since it is a special machine-
>> readable line.
>>
>> There was some discussion in v1 about using DIV_ROUND_UP(), etc. macros, but
>> the consensus was to leave it as-is since it matches existing code and that
>> macros can be introduced in another patch.
>>
>> drivers/pinctrl/pinctrl-single.c | 22 +++++++++++++++++++---
>> 1 file changed, 19 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
>> index cec7537..a7c5eb3 100644
>> --- a/drivers/pinctrl/pinctrl-single.c
>> +++ b/drivers/pinctrl/pinctrl-single.c
>> @@ -391,9 +391,25 @@ static int pcs_request_gpio(struct pinctrl_dev *pctldev,
>> || pin < frange->offset)
>> continue;
>
>> mux_bytes = pcs->width / BITS_PER_BYTE;
>> - data = pcs->read(pcs->base + pin * mux_bytes) & ~pcs->fmask;
>> - data |= frange->gpiofunc;
>> - pcs->write(data, pcs->base + pin * mux_bytes);
>> +
>> + if (pcs->bits_per_mux) {
>> + int byte_num, offset, pin_shift;
>> +
>> + byte_num = (pcs->bits_per_pin * pin) / BITS_PER_BYTE;
>> + offset = (byte_num / mux_bytes) * mux_bytes;
>> + pin_shift = pin % (pcs->width / pcs->bits_per_pin) *
>> + pcs->bits_per_pin;
>> +
>> + data = pcs->read(pcs->base + offset);
>> + data &= ~(pcs->fmask << pin_shift);
>> + data |= frange->gpiofunc << pin_shift;
>> + pcs->write(data, pcs->base + offset);
>> + } else {
>
>> + data = pcs->read(pcs->base + pin * mux_bytes);
>> + data &= ~pcs->fmask;
>> + data |= frange->gpiofunc;
>> + pcs->write(data, pcs->base + pin * mux_bytes);
>
> Just an idea, you may leave this almost untouched and do calculate
> pin_shift and offset in condition, like
>
> if (...) {
> pin_shift = ...
> offset = ...
> } else {
> pin_shift = 0;
> offset = pin * mux_bytes;
> }
>
> data = pcs->read(pcs->base + offset);
> data &= ~(pcs->fmask << pin_shift);
> data |= frange->gpiofunc << pin_shift;
> pcs->write(data, pcs->base + offset);
>
> It's also possible to split to two changes, where first introduces
> that variables and their default values (see 'else' branch) and second
> one introduces an if branch override.
>
>> + }
>> break;
>
Yes, there are many ways this could be done. However, I would like
to just leave it as it is since it matches the patterns used
elsewhere in this file.
On Mon, Feb 19, 2018 at 10:57 PM, David Lechner <[email protected]> wrote:
> This fixes pcs_request_gpio() in the pinctrl-single driver when
> bits_per_mux != 0. It appears this was overlooked when the multiple
> pins per register feature was added.
>
> Fixes: 4e7e8017a80e ("pinctrl: pinctrl-single: enhance to configure multiple pins of different modules")
> Signed-off-by: David Lechner <[email protected]>
> ---
>
> v2 changes:
> - don't wrap Fixes: line in commit message since it is a special machine-
> readable line.
Tony/Haojian: are you OK with this change?
Yours,
Linus Walleij
* Linus Walleij <[email protected]> [180323 03:02]:
> On Mon, Feb 19, 2018 at 10:57 PM, David Lechner <[email protected]> wrote:
>
> > This fixes pcs_request_gpio() in the pinctrl-single driver when
> > bits_per_mux != 0. It appears this was overlooked when the multiple
> > pins per register feature was added.
> >
> > Fixes: 4e7e8017a80e ("pinctrl: pinctrl-single: enhance to configure multiple pins of different modules")
> > Signed-off-by: David Lechner <[email protected]>
> > ---
> >
> > v2 changes:
> > - don't wrap Fixes: line in commit message since it is a special machine-
> > readable line.
>
> Tony/Haojian: are you OK with this change?
No objections from me, would be good if Haojian could
test it with his test cases though.
Regards,
Tony
On Mon, Feb 19, 2018 at 10:57 PM, David Lechner <[email protected]> wrote:
> This fixes pcs_request_gpio() in the pinctrl-single driver when
> bits_per_mux != 0. It appears this was overlooked when the multiple
> pins per register feature was added.
>
> Fixes: 4e7e8017a80e ("pinctrl: pinctrl-single: enhance to configure multiple pins of different modules")
> Signed-off-by: David Lechner <[email protected]>
Patch applied for v4.17 with Tony's ACK.
Yours,
Linus Walleij
On Fri, Mar 23, 2018 at 3:22 PM, Tony Lindgren <[email protected]> wrote:
> * Linus Walleij <[email protected]> [180323 03:02]:
>> On Mon, Feb 19, 2018 at 10:57 PM, David Lechner <[email protected]> wrote:
>>
>> > This fixes pcs_request_gpio() in the pinctrl-single driver when
>> > bits_per_mux != 0. It appears this was overlooked when the multiple
>> > pins per register feature was added.
>> >
>> > Fixes: 4e7e8017a80e ("pinctrl: pinctrl-single: enhance to configure multiple pins of different modules")
>> > Signed-off-by: David Lechner <[email protected]>
>> > ---
>> >
>> > v2 changes:
>> > - don't wrap Fixes: line in commit message since it is a special machine-
>> > readable line.
>>
>> Tony/Haojian: are you OK with this change?
>
> No objections from me, would be good if Haojian could
> test it with his test cases though.
I applied it for v4.17 recording this as an ACK :)
If there are problems I bet we will notice in the -rc phase.
Yours,
Linus Walleij