[RFC PATCH 0/3] gpio: pca953x: Redesign handling of chip types

[RFC PATCH 0/3] gpio: pca953x: Redesign handling of chip types

[Levente Révész]

Hi All,

The pca953x driver supports many chips. These all have the basic 4
registers: input, output, invert and direction. Most of them have
additional registers for various functions, like pull-up and
pull-down control, interrupt status and mask, hardware debounce.

Different chip types have various register layouts. These layouts differ
even in the basic 4 registers.

This patch series

    1. Cleans up chip type handling, and
    2. Replaces register address calculations. The proposed scheme
       works with every register of every chip type.

These changes make it possible to add support for extended functions for
more chip types, including:

    * Interrupt mask for PCA950X, PCA9698, PCA957X and XRA120X
    * Interrupt status for PCA957X and XRA120X
    * Bias for PCA957X and XRA120X
    * Debounce for PCAL65XX and XRA120X

References:
1. Previous discussion about the chip types
   https://lore.kernel.org/linux-gpio/Y1q52efyv93%2Fz8BC@smile.fi.intel.com/
2. An overview of pca953x chip types
   https://lore.kernel.org/linux-gpio/67bba210-09ac-32fb-bb97-8bfc40c2c200@eilabs.com/


Best Regards,
Levente Révész

Levente Révész (3):
  gpio: pca953x: Replace chip type flags with a type enum
  gpio: pca953x: Describe register maps with enums
  gpio: pca953x: Redesign register address calculation

 drivers/gpio/gpio-pca953x.c | 1196 ++++++++++++++++++++++++++---------
 1 file changed, 894 insertions(+), 302 deletions(-)

-- 
2.38.1

Re: [RFC PATCH 0/3] gpio: pca953x: Redesign handling of chip types

[Andy Shevchenko]

On Mon, Jan 30, 2023 at 09:59:30PM +0100, Levente Révész wrote:
> Hi All,
> 
> The pca953x driver supports many chips. These all have the basic 4
> registers: input, output, invert and direction. Most of them have
> additional registers for various functions, like pull-up and
> pull-down control, interrupt status and mask, hardware debounce.
> 
> Different chip types have various register layouts. These layouts differ
> even in the basic 4 registers.
> 
> This patch series
> 
>     1. Cleans up chip type handling, and
>     2. Replaces register address calculations. The proposed scheme
>        works with every register of every chip type.
> 
> These changes make it possible to add support for extended functions for
> more chip types, including:
> 
>     * Interrupt mask for PCA950X, PCA9698, PCA957X and XRA120X
>     * Interrupt status for PCA957X and XRA120X
>     * Bias for PCA957X and XRA120X
>     * Debounce for PCAL65XX and XRA120X
> 
> References:
> 1. Previous discussion about the chip types
>    https://lore.kernel.org/linux-gpio/Y1q52efyv93%2Fz8BC@smile.fi.intel.com/
> 2. An overview of pca953x chip types
>    https://lore.kernel.org/linux-gpio/67bba210-09ac-32fb-bb97-8bfc40c2c200@eilabs.com/

Thank you for the update, I'll look into the series later on.
Just to tell you that your cover letter is from something else according to the
email chain (the 1/3 isn't linked to 0/3).

-- 
With Best Regards,
Andy Shevchenko

Re: [RFC PATCH 0/3] gpio: pca953x: Redesign handling of chip types

[Andy Shevchenko]

On Mon, Jan 30, 2023 at 09:59:30PM +0100, Levente Révész wrote:
> Hi All,
> 
> The pca953x driver supports many chips. These all have the basic 4
> registers: input, output, invert and direction. Most of them have
> additional registers for various functions, like pull-up and
> pull-down control, interrupt status and mask, hardware debounce.
> 
> Different chip types have various register layouts. These layouts differ
> even in the basic 4 registers.
> 
> This patch series
> 
>     1. Cleans up chip type handling, and
>     2. Replaces register address calculations. The proposed scheme
>        works with every register of every chip type.
> 
> These changes make it possible to add support for extended functions for
> more chip types, including:
> 
>     * Interrupt mask for PCA950X, PCA9698, PCA957X and XRA120X
>     * Interrupt status for PCA957X and XRA120X
>     * Bias for PCA957X and XRA120X
>     * Debounce for PCAL65XX and XRA120X
> 
> References:
> 1. Previous discussion about the chip types
>    https://lore.kernel.org/linux-gpio/Y1q52efyv93%2Fz8BC@smile.fi.intel.com/
> 2. An overview of pca953x chip types
>    https://lore.kernel.org/linux-gpio/67bba210-09ac-32fb-bb97-8bfc40c2c200@eilabs.com/

Now as I'm thinking more of your nice job, it may be less effort to everybody
if you start from moving the driver to be a real pin control driver united with
GPIO handling.

That said, start drivers/pinctrl/pinctrl-pca953x.c from scratch with all your
ideas in it. To make it better for review and testing, split by series in which
you add the main chip family, i.e. PCA953x one (without interrupt?) and then
adding each type one-by-one.

Also prepare some documentation file in Documentation/...gpio.../pca953x or
start a brand new pinctrl folder (we do not have it yet) where you describe
all your research.

Note, this will be quite useful to enable PWM support later on for some chips.

-- 
With Best Regards,
Andy Shevchenko

Re: [RFC PATCH 0/3] gpio: pca953x: Redesign handling of chip types

[Andy Shevchenko]

On Thu, Feb 02, 2023 at 05:12:22PM +0200, Andy Shevchenko wrote:
> On Mon, Jan 30, 2023 at 09:59:30PM +0100, Levente Révész wrote:
> > Hi All,
> > 
> > The pca953x driver supports many chips. These all have the basic 4
> > registers: input, output, invert and direction. Most of them have
> > additional registers for various functions, like pull-up and
> > pull-down control, interrupt status and mask, hardware debounce.
> > 
> > Different chip types have various register layouts. These layouts differ
> > even in the basic 4 registers.
> > 
> > This patch series
> > 
> >     1. Cleans up chip type handling, and
> >     2. Replaces register address calculations. The proposed scheme
> >        works with every register of every chip type.
> > 
> > These changes make it possible to add support for extended functions for
> > more chip types, including:
> > 
> >     * Interrupt mask for PCA950X, PCA9698, PCA957X and XRA120X
> >     * Interrupt status for PCA957X and XRA120X
> >     * Bias for PCA957X and XRA120X
> >     * Debounce for PCAL65XX and XRA120X
> > 
> > References:
> > 1. Previous discussion about the chip types
> >    https://lore.kernel.org/linux-gpio/Y1q52efyv93%2Fz8BC@smile.fi.intel.com/
> > 2. An overview of pca953x chip types
> >    https://lore.kernel.org/linux-gpio/67bba210-09ac-32fb-bb97-8bfc40c2c200@eilabs.com/
> 
> Now as I'm thinking more of your nice job, it may be less effort to everybody
> if you start from moving the driver to be a real pin control driver united with
> GPIO handling.
> 
> That said, start drivers/pinctrl/pinctrl-pca953x.c from scratch with all your
> ideas in it. To make it better for review and testing, split by series in which
> you add the main chip family, i.e. PCA953x one (without interrupt?) and then
> adding each type one-by-one.
> 
> Also prepare some documentation file in Documentation/...gpio.../pca953x or
> start a brand new pinctrl folder (we do not have it yet) where you describe
> all your research.
> 
> Note, this will be quite useful to enable PWM support later on for some chips.

And to avoid messing up with the existing driver, add Kconfig part at last.

-- 
With Best Regards,
Andy Shevchenko

Re: [RFC PATCH 0/3] gpio: pca953x: Redesign handling of chip types

[Levente Révész]

On 02/02/2023 16.14, Andy Shevchenko wrote:
> On Thu, Feb 02, 2023 at 05:12:22PM +0200, Andy Shevchenko wrote:
>> Now as I'm thinking more of your nice job, it may be less effort to everybody
>> if you start from moving the driver to be a real pin control driver united with
>> GPIO handling.

I will look into this, it will probably take a few weeks to come up with something presentable.

Thank you for your review so far!

Best Regards,
Levente Révész

Re: [RFC PATCH 0/3] gpio: pca953x: Redesign handling of chip types

[Andy Shevchenko]

On Fri, Feb 03, 2023 at 11:40:50AM +0100, Levente Révész wrote:
> On 02/02/2023 16.14, Andy Shevchenko wrote:
> > On Thu, Feb 02, 2023 at 05:12:22PM +0200, Andy Shevchenko wrote:
> >> Now as I'm thinking more of your nice job, it may be less effort to everybody
> >> if you start from moving the driver to be a real pin control driver united with
> >> GPIO handling.
> 
> I will look into this, it will probably take a few weeks to come up with
> something presentable.

You have that time because the current series marked as RFC and even
if it had been ready it would have not been applied due to release schedule
(we are now at almost v6.2-rc7 and in two and half weeks we expect
 the release to come, so it means you have ~6 weeks of time).

> Thank you for your review so far!

You are welcome!

-- 
With Best Regards,
Andy Shevchenko

Re: [RFC PATCH 0/3] gpio: pca953x: Redesign handling of chip types

[Linus Walleij]

On Thu, Feb 2, 2023 at 4:12 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Now as I'm thinking more of your nice job, it may be less effort to everybody
> if you start from moving the driver to be a real pin control driver united with
> GPIO handling.

Agreed, the more complex these expanders get the closer they move to becoming
pin controllers with GPIO.

drivers/pinctrl/pinctrl-sx150x.c
drivers/pinctrl/pinctrl-mcp23s08.c
drivers/pinctrl/pinctrl-stmfx.c

Come to mind especially.

So if it makes more sense - go for it!

Yours,
Linus Walleij