Re: [PATCH 1/3] hw/gpio: add PCA6414 i2c GPIO expander

[Philippe Mathieu-Daudé]

On 8/2/23 23:40, Titus Rwantare wrote:
> On Mon, 6 Feb 2023 at 13:38, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> > Hi Titus,
> >
> > On 6/2/23 20:49, Titus Rwantare wrote:
> > > This is a simple i2c device that allows i2c capable devices to have
> > > GPIOs.
> > >
> > > Reviewed-by: Hao Wu <wuhaotsh@google.com>
> > > Signed-off-by: Titus Rwantare <titusr@google.com>
> > > ---
> > >    hw/arm/Kconfig                  |   1 +
> > >    hw/gpio/meson.build             |   1 +
> > >    hw/gpio/pca_i2c_gpio.c          | 362 ++++++++++++++++++++++++++++++++
> > >    hw/gpio/trace-events            |   5 +
> > >    hw/i2c/Kconfig                  |   4 +
> > >    include/hw/gpio/pca_i2c_gpio.h  |  72 +++++++
> > >    tests/qtest/meson.build         |   1 +
> > >    tests/qtest/pca_i2c_gpio-test.c | 169 +++++++++++++++
> > >    8 files changed, 615 insertions(+)
> > >    create mode 100644 hw/gpio/pca_i2c_gpio.c
> > >    create mode 100644 include/hw/gpio/pca_i2c_gpio.h
> > >    create mode 100644 tests/qtest/pca_i2c_gpio-test.c

> > > +#define PCA6416_INPUT_PORT_0                 0x00 /* read */
> > > +#define PCA6416_INPUT_PORT_1                 0x01 /* read */
> > > +#define PCA6416_OUTPUT_PORT_0                0x02 /* read/write */
> > > +#define PCA6416_OUTPUT_PORT_1                0x03 /* read/write */
> > > +#define PCA6416_POLARITY_INVERSION_PORT_0    0x04 /* read/write */
> > > +#define PCA6416_POLARITY_INVERSION_PORT_1    0x05 /* read/write */
> > > +#define PCA6416_CONFIGURATION_PORT_0         0x06 /* read/write */
> > > +#define PCA6416_CONFIGURATION_PORT_1         0x07 /* read/write */
> > > +
> > > +#define PCA6416_OUTPUT_DEFAULT               0xFFFF
> > > +#define PCA6416_CONFIG_DEFAULT               0xFFFF
> > > +
> > > +#define PCA_I2C_OUTPUT_DEFAULT               0xFFFF
> > > +#define PCA_I2C_CONFIG_DEFAULT               0xFFFF
> >
> > (These register definitions could be kept internal in pca_i2c_gpio.c).
>
> I put these here to use them in the qtests.

Oh right, I missed that.

> > > diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> > > index e97616d327..49f406af6b 100644
> > > --- a/tests/qtest/meson.build
> > > +++ b/tests/qtest/meson.build
> > > @@ -241,6 +241,7 @@ qos_test_ss.add(
> > >      'ne2000-test.c',
> > >      'tulip-test.c',
> > >      'nvme-test.c',
> > > +  'pca_i2c_gpio-test.c',
> >
> > Should this be conditional to
> > config_all_devices.has_key('CONFIG_PCA_I2C_GPIO')?
>
> Is that the guidance for qos tests? All these tests would also need to
> be separated out.
This is not clear to me, adding Thomas.