All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Walleij <linus.walleij@linaro.org>
To: "Bedel, Alban" <alban.bedel@aerq.com>
Cc: "linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	"bgolaszewski@baylibre.com" <bgolaszewski@baylibre.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] gpio: pca953x: add support for open drain pins on PCAL6524
Date: Tue, 2 Feb 2021 20:21:34 +0100	[thread overview]
Message-ID: <CACRpkdZhOWxbOdfC3sh9BXj9dWTcm3gPuGjzz+T96==+G2-i2g@mail.gmail.com> (raw)
In-Reply-To: <93036ef781d20df4c6017178cc545702bd0f42bc.camel@aerq.com>

On Tue, Feb 2, 2021 at 6:45 PM Bedel, Alban <alban.bedel@aerq.com> wrote:
> On Tue, 2021-02-02 at 14:57 +0100, Linus Walleij wrote:

> > > +       if (out_conf & BIT(offset / BANK_SZ))
> >
> > I suppose this could be written if (out_conf & mask)?
>
> No, the mask in ODENn is per bank (group of 8 pins) and not per pin.

Ah I see it now (confused % for / ...)

> > The datasheet says:
> >
> >   "If the ODENx bit is set at logic 0 (push-pull), any bit set to
> > logic 1
> >   in the IOCRx register will reverse the output state of that pin
> > only
> >   to open-drain. When ODENx bit is set at logic 1 (open-drain), a
> >   logic 1 in IOCRx will set that pin to push-pull."
> >
> > So your logic is accounting for the fact that someone go and set
> > one of the bits in ODENx to 1, but aren't they all by default set
> > to zero (or should be programmed by the driver to zero)
> > so that you can control open drain individually here by simply
> > setting the corresponding bit to 1 for open drain and 0 for
> > push-pull?
>
> Yes the ODENx bits are 0 by default, but the bootloader might have
> changed them for example. Currently the driver doesn't do any reset so
> I think it make sense to correctly handle this case as it doesn't bring
> that much extra complexity.

Hm. I guess you're right if that is the style of the driver overall
(don't touch bootloader/reset defaults).

We don't use that style for interrupt registers because we don't
want interrupts to randomly fire, instead we usually disable them
all so the driver and Linux keeps track on what is enabled. But for
bias etc, I guess it is OK. But consider the option of just writing
0 into all the ODENx registers at probe(). I'm not gonna complain
if you really want it like this though.

Yours,
Linus Walleij

  reply	other threads:[~2021-02-02 19:25 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-28 15:36 [PATCH] gpio: pca953x: add support for open drain pins on PCAL6524 Alban Bedel
2021-02-02 11:42 ` Bartosz Golaszewski
2021-02-02 17:45   ` Bedel, Alban
2021-02-03 10:12   ` Andy Shevchenko
2021-02-02 13:57 ` Linus Walleij
2021-02-02 17:45   ` Bedel, Alban
2021-02-02 19:21     ` Linus Walleij [this message]
2021-02-03 10:10 ` Andy Shevchenko

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='CACRpkdZhOWxbOdfC3sh9BXj9dWTcm3gPuGjzz+T96==+G2-i2g@mail.gmail.com' \
    --to=linus.walleij@linaro.org \
    --cc=alban.bedel@aerq.com \
    --cc=bgolaszewski@baylibre.com \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /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.