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")
Cc: Tony Lindgren <[email protected]>
Cc: Haojian Zhuang <[email protected]>
Cc: Linus Walleij <[email protected]>
Signed-off-by: David Lechner <[email protected]>
---
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 3501491..7a5e2f1 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, Jan 22, 2018 at 1:03 AM, 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")
One line?
> + 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;
Sounds like playing around pretty well defined macro and functions,
e.g. DIV_ROUND_UP(), round_up().
--
With Best Regards,
Andy Shevchenko
On 01/22/2018 08:49 AM, Andy Shevchenko wrote:
> On Mon, Jan 22, 2018 at 1:03 AM, 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")
>
> One line?
One line is more important that wrapping to 75 chars?
>
>> + 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;
>
> Sounds like playing around pretty well defined macro and functions,
> e.g. DIV_ROUND_UP(), round_up().
>
I admit, I just copied existing code (which may be a reason to leave this the
way it is). But, I only see once place to do this:
offset = round_down(byte_num, mux_bytes);
Did I miss another?
On Mon, Jan 22, 2018 at 7:06 PM, David Lechner <[email protected]> wrote:
> On 01/22/2018 08:49 AM, Andy Shevchenko wrote:
>> On Mon, Jan 22, 2018 at 1:03 AM, David Lechner <[email protected]>
>> wrote:
>>> Fixes: 4e7e8017a80e ("pinctrl: pinctrl-single: enhance to configure
>>> multiple pins of different modules")
>> One line?
> One line is more important that wrapping to 75 chars?
It's *special* line, i.e. tag. Tag per line is a rule I know, did I
miss any new change to that?
>>> + 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;
>> Sounds like playing around pretty well defined macro and functions,
>> e.g. DIV_ROUND_UP(), round_up().
> I admit, I just copied existing code (which may be a reason to leave this
> the
> way it is).
Ah, fair enough.
> But, I only see once place to do this:
>
> offset = round_down(byte_num, mux_bytes);
>
> Did I miss another?
I meant that you may use macros to make code cleaner. Though, taking
above into consideration, it would be done as a separate patch later.
--
With Best Regards,
Andy Shevchenko
* Andy Shevchenko <[email protected]> [180123 09:25]:
> On Mon, Jan 22, 2018 at 7:06 PM, David Lechner <[email protected]> wrote:
> > On 01/22/2018 08:49 AM, Andy Shevchenko wrote:
> >> On Mon, Jan 22, 2018 at 1:03 AM, David Lechner <[email protected]>
> >> wrote:
>
> >>> Fixes: 4e7e8017a80e ("pinctrl: pinctrl-single: enhance to configure
> >>> multiple pins of different modules")
>
> >> One line?
>
> > One line is more important that wrapping to 75 chars?
>
> It's *special* line, i.e. tag. Tag per line is a rule I know, did I
> miss any new change to that?
I think the rule for merge commits is to try to limit them to 75
characters for git log.
> >>> + 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;
>
> >> Sounds like playing around pretty well defined macro and functions,
> >> e.g. DIV_ROUND_UP(), round_up().
>
> > I admit, I just copied existing code (which may be a reason to leave this
> > the
> > way it is).
>
> Ah, fair enough.
>
> > But, I only see once place to do this:
> >
> > offset = round_down(byte_num, mux_bytes);
> >
> > Did I miss another?
>
> I meant that you may use macros to make code cleaner. Though, taking
> above into consideration, it would be done as a separate patch later.
Yeah makes sense to me to do them separately. It would be good to
get Haojian's ack or tested-by on this one, I don't think I have
hardware where I can test the GPIO features with this driver.
Regards,
Tony