LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Andrew Jeffery <andrew@aj.id.au>
Cc: "linux-leds@vger.kernel.org" <linux-leds@vger.kernel.org>,
	"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	"Cédric Le Goater" <clg@kaod.org>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Joel Stanley" <joel@jms.id.au>, "Pavel Machek" <pavel@ucw.cz>,
	"Linus Walleij" <linus.walleij@linaro.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-aspeed@lists.ozlabs.org" <linux-aspeed@lists.ozlabs.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH 0/6] leds: Fix pca955x GPIO pin mappings
Date: Thu, 29 Jul 2021 10:40:09 +0300	[thread overview]
Message-ID: <CAHp75Vdx9QA7dmSWK8GHxBBxP0uYjrz=Gm=75yqaWbBX6k3v=w@mail.gmail.com> (raw)
In-Reply-To: <6cc64039-f82a-4c1e-ad2c-16fad7aa3178@www.fastmail.com>

On Thu, Jul 29, 2021 at 3:39 AM Andrew Jeffery <andrew@aj.id.au> wrote:
> On Wed, 28 Jul 2021, at 18:43, Andy Shevchenko wrote:
> > On Wed, Jul 28, 2021 at 8:43 AM Andrew Jeffery <andrew@aj.id.au> wrote:
> > > On Fri, 23 Jul 2021, at 17:45, Andy Shevchenko wrote:
> > >
> > > > I was briefly looking into patches 1-4 and suddenly
> > > > realized that the fix can be similar as in PCA9685 (PWM), I.e. we
> > > > always have chips for the entire pin space and one may map them
> > > > accordingly, requested in one realm (LED) in the other (GPIO)
> > > > automatically is BUSY. Or I missed the point?
> > >
> > > No, you haven't missed the point. I will look at the PCA9685 driver.
> > >
> > > That said, my goal was to implement the behaviour intended by the
> > > existing binding (i.e. fix a bug).
> >
> > Okay, so it implies that this used to work at some point.
>
> I don't think this is true. It only "works" if the lines specified as
> GPIO in the devicetree are contiguous from line 0. That way the pin and
> GPIO number spaces align. I suspect that's all that's been tested up
> until this point.
>
> We now have a board with a PCA9552 where the first 8 pins are LED and
> the last 8 pins are GPIO, and if you specify this in the devicetree
> according to the binding you hit the failure to map between the two
> number spaces.
>
> > What has
> > changed from that point? Why can't we simply fix the culprit commit?
>
> As such nothing has changed, I think it's always been broken, just we
> haven't had hardware configurations that demonstrated the failure.
>
> >
> > > However, userspace would never have
> > > got the results it expected with the existing driver implementation, so
> > > I guess you could argue that no such (useful) userspace exists. Given
> > > that, we could adopt the strategy of always defining a gpiochip
> > > covering the whole pin space, and parts of the devicetree binding just
> > > become redundant.
> >
> > I'm lost now. GPIO has its own userspace ABI, how does it work right
> > now in application to this chip?
>
> As above, it "works" if the GPIOs specified in the devicetree are
> contiguous from line 0. It's broken if they're not.

So, "it never works" means there is no bug. Now, what we need is to
keep the same enumeration scheme, but if you wish to be used half/half
(or any other ratio), the driver should do like the above mentioned
PWM, i.e. register entire space and depending on the requestor either
proceed with a line or mark it as BUSY.

Ideally, looking into what the chip can do, this should be indeed
converted to some like pin control + PWM + LED + GPIO drivers. Then
the function in pin mux configuration can show what exactly is enabled
on the certain line(s).


-- 
With Best Regards,
Andy Shevchenko

  reply	other threads:[~2021-07-29  7:40 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-23  7:58 Andrew Jeffery
2021-07-23  7:58 ` [RFC PATCH 1/6] pinctrl: Add pinctrl_gpio_as_pin() Andrew Jeffery
2021-08-10 13:34   ` Linus Walleij
2021-08-11  0:24     ` Andrew Jeffery
2021-07-23  7:58 ` [RFC PATCH 2/6] pinctrl: Add hook for device-specific map parsing Andrew Jeffery
2021-07-23  7:58 ` [RFC PATCH 3/6] leds: pca955x: Relocate chipdef-related descriptors Andrew Jeffery
2021-07-23  7:58 ` [RFC PATCH 4/6] leds: pca955x: Use pinctrl to map GPIOs to pins Andrew Jeffery
2021-08-10 13:54   ` Linus Walleij
2021-08-11  0:19     ` Andrew Jeffery
2021-07-23  7:58 ` [RFC PATCH 5/6] ARM: dts: rainier: Add presence-detect and fault indictor GPIO expander Andrew Jeffery
2021-07-23  7:58 ` [RFC PATCH 6/6] pinctrl: Check get_group_pins callback on init Andrew Jeffery
     [not found] ` <CAHp75VeQML7njMZ6x8kC-ZJVexC1xJ6n1cB3JneVMAVfuOJgWw@mail.gmail.com>
2021-07-28  5:43   ` [RFC PATCH 0/6] leds: Fix pca955x GPIO pin mappings Andrew Jeffery
2021-07-28  9:13     ` Andy Shevchenko
2021-07-29  0:38       ` Andrew Jeffery
2021-07-29  7:40         ` Andy Shevchenko [this message]
2021-08-03  4:07           ` Andrew Jeffery
2021-08-03 10:33             ` Andy Shevchenko
2021-08-04  4:55               ` Andrew Jeffery

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='CAHp75Vdx9QA7dmSWK8GHxBBxP0uYjrz=Gm=75yqaWbBX6k3v=w@mail.gmail.com' \
    --to=andy.shevchenko@gmail.com \
    --cc=andrew@aj.id.au \
    --cc=clg@kaod.org \
    --cc=devicetree@vger.kernel.org \
    --cc=joel@jms.id.au \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-aspeed@lists.ozlabs.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=pavel@ucw.cz \
    --cc=robh+dt@kernel.org \
    --subject='Re: [RFC PATCH 0/6] leds: Fix pca955x GPIO pin mappings' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).