All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tony Lindgren <tony@atomide.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Matthijs Kooijman <matthijs@stdin.nl>,
	Haojian Zhuang <haojian.zhuang@linaro.org>,
	linux-gpio@vger.kernel.org, linux-omap@vger.kernel.org
Subject: Re: [PATCH] pinctrl: single: Fix PIN_CONFIG_BIAS_DISABLE handling
Date: Wed, 3 Apr 2024 11:59:47 +0300	[thread overview]
Message-ID: <20240403085947.GL5132@atomide.com> (raw)
In-Reply-To: <CACRpkdaRM-uqp_Y=gT=FMC48=835dQnB0YomYkxR9ag-SW5cOg@mail.gmail.com>

* Linus Walleij <linus.walleij@linaro.org> [240328 21:02]:
> On Tue, Mar 19, 2024 at 12:07 PM Matthijs Kooijman <matthijs@stdin.nl> wrote:
> 
> > The pinctrl-single driver handles pin_config_set by looking up the
> > requested setting in a DT-defined lookup table, which defines what bits
> > correspond to each setting. There is no way to add
> > PIN_CONFIG_BIAS_DISABLE entries to the table, since there is instead
> > code to disable the bias by applying the disable values of both the
> > pullup and pulldown entries in the table.
> >
> > However, this code is inside the table-lookup loop, so it would only
> > execute if there is an entry for PIN_CONFIG_BIAS_DISABLE in the table,
> > which can never exist, so this code never runs.
> >
> > This commit lifts the offending code out of the loop, so it just
> > executes directly whenever PIN_CONFIG_BIAS_DISABLE is requested,
> > skippipng the table lookup loop.
> >
> > This also introduces a new `param` variable to make the code slightly
> > more readable.
> >
> > This bug seems to have existed when this code was first merged in commit
> > 9dddb4df90d13 ("pinctrl: single: support generic pinconf"). Earlier
> > versions of this patch did have an entry for PIN_CONFIG_BIAS_DISABLE in
> > the lookup table, but that was removed, which is probably how this bug
> > was introduced.
> >
> > Signed-off-by: Matthijs Kooijman <matthijs@stdin.nl>
> 
> This looks reasonable to me, but I need Tony to review it before applying.

Looks good to me:

Reviewed-by: Tony Lindgren <tony@atomide.com>

  reply	other threads:[~2024-04-03  9:00 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-28 12:04 pinctrl: single: Cannot disable bias - PIN_CONFIG_BIAS_DISABLE not handled Matthijs Kooijman
2024-02-29  7:32 ` Tony Lindgren
2024-02-29 18:31   ` Nathan Chancellor
2024-02-29 19:07     ` Matthijs Kooijman
2024-03-01 10:04     ` Tony Lindgren
2024-03-19 11:05   ` Matthijs Kooijman
2024-03-19 11:06     ` [PATCH] pinctrl: single: Fix PIN_CONFIG_BIAS_DISABLE handling Matthijs Kooijman
2024-03-28 21:02       ` Linus Walleij
2024-04-03  8:59         ` Tony Lindgren [this message]
2024-03-29  2:05       ` Haojian Zhuang
2024-04-04 12:00       ` Linus Walleij
2024-03-22  6:24     ` pinctrl: single: Cannot disable bias - PIN_CONFIG_BIAS_DISABLE not handled Tony Lindgren
2024-03-29  2:06       ` Haojian Zhuang

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=20240403085947.GL5132@atomide.com \
    --to=tony@atomide.com \
    --cc=haojian.zhuang@linaro.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=matthijs@stdin.nl \
    /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.