Message ID | 1519077427-30165-1-git-send-email-david@lechnology.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Mon, Feb 19, 2018 at 11:57 PM, David Lechner <david@lechnology.com> 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 <david@lechnology.com> > --- > > 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;
On 02/20/2018 06:56 AM, Andy Shevchenko wrote: > On Mon, Feb 19, 2018 at 11:57 PM, David Lechner <david@lechnology.com> 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 <david@lechnology.com> >> --- >> >> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 19, 2018 at 10:57 PM, David Lechner <david@lechnology.com> 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 <david@lechnology.com> > --- > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Linus Walleij <linus.walleij@linaro.org> [180323 03:02]: > On Mon, Feb 19, 2018 at 10:57 PM, David Lechner <david@lechnology.com> 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 <david@lechnology.com> > > --- > > > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 19, 2018 at 10:57 PM, David Lechner <david@lechnology.com> 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 <david@lechnology.com> Patch applied for v4.17 with Tony's ACK. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Mar 23, 2018 at 3:22 PM, Tony Lindgren <tony@atomide.com> wrote: > * Linus Walleij <linus.walleij@linaro.org> [180323 03:02]: >> On Mon, Feb 19, 2018 at 10:57 PM, David Lechner <david@lechnology.com> 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 <david@lechnology.com> >> > --- >> > >> > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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;
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 <david@lechnology.com> --- 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(-)