Re: [PATCH v2 2/2] gpio: fxl6408: add I2C GPIO expander driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 13, 2023 at 02:24:47PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 13, 2023 at 1:33 PM Francesco Dolcini <francesco@xxxxxxxxxx> wrote:
> > +config GPIO_FXL6408
> > +       tristate "FXL6408 I2C GPIO expander"
> > +       select GPIO_REGMAP
> 
> > +       select REGMAP_I2C
> 
> Somebody pointed out that this might require
> 
>     depends on I2C
> 
> being added as well.

This is already there, I would prefer to not duplicate it neither do it
differently from all the other I2C GPIO drivers.

menu "I2C GPIO expanders"
	depends on I2C

> > +#define FXL6408_MAX_REGISTER           0x13
> 
> This is used as a range, but why? If we can have a proper name for
> this register, why bother dumping all this or even having access to?

So, I see 2 options:
1. Use FXL6408_REG_INPUT_STATUS instead of FXL6408_MAX_REGISTER
2. #define FXL6408_REG_INT_STS 0x13 and use it instead

I think that there are trivial benefits on both, with option 2 being the
correct hardware description and that would not need to be changed when adding
additional functionalities to the driver.

Which solution do we prefer for v3?
My preference would be to define FXL6408_REG_INT_STS and use it instead.

Francesco




[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux