Re: [PATCH v3] gpio: Add Intel USBIO GPIO driver

[Hans de Goede <hansg@kernel.org>]

Hi,

On 10-Sep-25 3:20 PM, Bartosz Golaszewski wrote:
> On Wed, Sep 10, 2025 at 2:17 PM Hans de Goede <hansg@kernel.org> wrote:
>>
>> From: Israel Cepeda <israel.a.cepeda.lopez@intel.com>
>>
>> Add a a driver for the GPIO auxbus child device of the Intel USBIO USB
>> IO-expander used by the MIPI cameras on various new (Meteor Lake and
>> later) Intel laptops.
>>
>> Co-developed-by: Hans de Goede <hansg@kernel.org>
>> Signed-off-by: Hans de Goede <hansg@kernel.org>
>> Signed-off-by: Israel Cepeda <israel.a.cepeda.lopez@intel.com>
>> ---
>> Changes in v3:
>> - Drop (offset >= gc->ngpio) check and make usbio_gpio_get_bank_and_pin()
>>   return void
>> - Propagate usbio_gpio_set() ret val in usbio_gpio_direction_output()
>> - Use devm_gpiochip_add_data() and drop auxiliary_driver remove() callback
>>
>> Changes in v2:
>> - Add a config_mutex protect usbio_gpio_update_config() calls, which
>>   read-modify-write banks[x].config, racing with each other
>> - Adjust usbio_gpio_get() to have an int return value and propagate the
>>   usbio_control_msg() return value
>> - Use __le16, __le32 type + cpu_to_le16() and friends for on wire words
>> - Some small style fixes from Sakari's review
>> ---
>>  MAINTAINERS               |   1 +
>>  drivers/gpio/Kconfig      |  11 ++
>>  drivers/gpio/Makefile     |   1 +
>>  drivers/gpio/gpio-usbio.c | 250 ++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 263 insertions(+)
>>  create mode 100644 drivers/gpio/gpio-usbio.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 3410699ad0b2..53694bd91861 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -12699,6 +12699,7 @@ M:      Israel Cepeda <israel.a.cepeda.lopez@intel.com>
>>  M:     Hans de Goede <hansg@kernel.org>
>>  R:     Sakari Ailus <sakari.ailus@linux.intel.com>
>>  S:     Maintained
>> +F:     drivers/gpio/gpio-usbio.c
>>  F:     drivers/usb/misc/usbio.c
>>  F:     include/linux/usb/usbio.h
>>
>> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
>> index e43abb322fa6..5d3ca3dd2687 100644
>> --- a/drivers/gpio/Kconfig
>> +++ b/drivers/gpio/Kconfig
>> @@ -1448,6 +1448,17 @@ config GPIO_LJCA
>>           This driver can also be built as a module. If so, the module
>>           will be called gpio-ljca.
>>
>> +config GPIO_USBIO
>> +       tristate "Intel USBIO GPIO support"
>> +       depends on USB_USBIO
>> +       default USB_USBIO
>> +       help
>> +         Select this option to enable GPIO driver for the INTEL
>> +         USBIO driver stack.
>> +
>> +         This driver can also be built as a module. If so, the module
>> +         will be called gpio_usbio.
>> +
>>  config GPIO_LP3943
>>         tristate "TI/National Semiconductor LP3943 GPIO expander"
>>         depends on MFD_LP3943
>> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
>> index 379f55e9ed1e..8c55e2d5de42 100644
>> --- a/drivers/gpio/Makefile
>> +++ b/drivers/gpio/Makefile
>> @@ -90,6 +90,7 @@ obj-$(CONFIG_GPIO_JANZ_TTL)           += gpio-janz-ttl.o
>>  obj-$(CONFIG_GPIO_KEMPLD)              += gpio-kempld.o
>>  obj-$(CONFIG_GPIO_LATCH)               += gpio-latch.o
>>  obj-$(CONFIG_GPIO_LJCA)                += gpio-ljca.o
>> +obj-$(CONFIG_GPIO_USBIO)               += gpio-usbio.o
>>  obj-$(CONFIG_GPIO_LOGICVC)             += gpio-logicvc.o
>>  obj-$(CONFIG_GPIO_LOONGSON1)           += gpio-loongson1.o
>>  obj-$(CONFIG_GPIO_LOONGSON)            += gpio-loongson.o
>> diff --git a/drivers/gpio/gpio-usbio.c b/drivers/gpio/gpio-usbio.c
>> new file mode 100644
>> index 000000000000..31a376e91877
>> --- /dev/null
>> +++ b/drivers/gpio/gpio-usbio.c
>> @@ -0,0 +1,250 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2025 Intel Corporation.
>> + * Copyright (c) 2025 Red Hat, Inc.
>> + */
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/auxiliary_bus.h>
>> +#include <linux/cleanup.h>
>> +#include <linux/dev_printk.h>
> 
> No need to pull this if you include device.h.

Ok, I'll fix this for v4, which I will delay till
I've a full v4 series to send.

> 
>> +       usbio_acpi_bind(gpio->adev, usbio_gpio_acpi_hids);
>> +
>> +       for (bank = 0; bank < USBIO_MAX_GPIOBANKS && bank_desc[bank].bmap; bank++)
>> +               gpio->banks[bank].bitmap = le32_to_cpu(bank_desc[bank].bmap);
>> +
>> +       gpio->gc.label = ACPI_COMPANION(dev) ?
>> +                                       acpi_dev_name(ACPI_COMPANION(dev)) : dev_name(dev);
>> +       gpio->gc.parent = dev;
>> +       gpio->gc.owner = THIS_MODULE;
>> +       gpio->gc.get_direction = usbio_gpio_get_direction;
>> +       gpio->gc.direction_input = usbio_gpio_direction_input;
>> +       gpio->gc.direction_output = usbio_gpio_direction_output;
>> +       gpio->gc.get = usbio_gpio_get;
>> +       gpio->gc.set = usbio_gpio_set;
>> +       gpio->gc.set_config = usbio_gpio_set_config;
>> +       gpio->gc.base = -1;
>> +       gpio->gc.ngpio = bank * USBIO_GPIOSPERBANK;
>> +       gpio->gc.can_sleep = true;
>> +
>> +       auxiliary_set_drvdata(adev, gpio);
>> +
> 
> Now you no longer need this as there's no remove() anymore.

Ack, I'll fix this for v4.

Regards,

Hans