On Wed, Jul 28, 2021 at 8:43 AM Andrew Jeffery [off-list ref] wrote:

On Fri, 23 Jul 2021, at 17:45, Andy Shevchenko wrote:

quoted

On Friday, July 23, 2021, Andrew Jeffery [off-list ref] wrote:

quoted

quoted

This series does a bunch of crimes, so it's an RFC. I'm cross-posting to the
pinctrl/GPIO and LEDs lists because the PCA955x devices impact all of them. What
needs fixing is the leds-pca955x driver's failure to map the GPIO numberspace to
the pin numberspace of the PCA955x devices. The series solves that by
implementing pinctrl and pinmux in the leds-pca955x driver.

Things I'm unsure about:

1. Patch 1: The pinctrl_gpio_as_pin() API feels a bit dirty, not sure what
   others thoughts are on that (Linus?).

2. Patch 2: I've added a new callback to hook the entirety of the pinctrl map
   parsing rather than supplying a subnode-specific callback. This was necessary
   to handle the PCA955x devicetree binding in a backwards compatible way.

3. Patch 4: The PCA955x devices don't actually have any pinmux hardware, but the
   properties of the pinctrl/pinmux subsystems in the kernel map nicely onto the
   problem we have. But it's quite a bit of code...

4. Patch 6: I also lost a bunch of time to overlooking the get_group_pins()
   callback for pinctrl, and it seems odd to me that it isn't required.

Please review!

Sounds like a hack.

Yes, possibly. Feedback like this is why I sent the series as an RFC.

quoted

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. What has
changed from that point? Why can't we simply fix the culprit commit?

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?

-- 
With Best Regards,
Andy Shevchenko

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel