Re: [PATCH 0/9] media: v4l: Add v4l2_acpi_parse_sensor_gpios() helper + gc0310 sensor driver

Andy Shevchenko
2023-05-19T12:15:03Z

On Thu, May 18, 2023 at 6:32 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi All,
>
> Now that the atomisp driver supports v4l2-async sensor driver registration
> (I'll post this as a separate series), there is no longer a need to have
> atomisp specific sensor drivers and after cleanup atomisp sensor drivers
> can now be moved to drivers/media/i2c as regular v4l2 sensor drivers!
>
> There is one small bit of ugliness involved though. On atomisp hw
> the sensor drivers need to do some work to get GPIO mappings from ACPI,
> requiring 1 line per sensor driver to call an ACPI specific sensor driver
> helper. The commit msg of patch 1/9 explains why:
>
> """
> On x86/ACPI platforms the GPIO resources do not provide information
> about which GPIO resource maps to which connection-id. So e.g.
> gpiod_get(devg, "reset") does not work.
>
> On devices with an Intel IPU3 or newer ISP there is a special ACPI
> INT3472 device describing the GPIOs and instantiating of the i2c_client
> for a sensor is deferred until the INT3472 driver has been bound based
> on the sensor ACPI device having a _DEP on the INT3472 ACPI device.
>
> This allows the INT3472 driver to add the necessary GPIO lookups
> without needing any special ACPI handling in the sensor driver.
>
> Unfortunately this does not work on devices with an atomisp2 ISP,
> there the _DSM describing the GPIOs is part of the sensor ACPI device
> itself, rather then being part of a separate ACPI device.
>
> IOW there is no separate firmware-node to which we can bind to register
> the GPIO lookups (and also no way to defer creating the sensor i2c_client).
>
> This unfortunately means that all sensor drivers which may be used on
> BYT or CHT hw need some code to deal with ACPI integration.
>
> This patch adds a new v4l2_acpi_parse_sensor_gpios() helper function
> for this, which does all the necessary work. This minimizes the
> (unavoidable) change to sensor drivers for ACPI integration to just
> adding a single line calling this void function to probe().
>
> There also is a no-op stub provided for non ACPI platforms so that
> no #ifdef-s are necessary in the driver.
> """
>
> This series adds the v4l2_acpi_parse_sensor_gpios() helper and
> then after some small cleanups moves the atomisp-gc0310 sensor
> driver (which now is a generic v4l2 sensor driver) to
> drivers/media/i2c.
>
> Since this touches drivers/media the intention is for this
> series to go upstream through the normal drivers/media process
> rather then through my media-atomisp branch.
>
> Note to v4l2-core maintainers: if you are ok with this series
> and would prefer for me to send it upstream through my
> media-atomisp branch, then I can do that too.

For the non-commented or in case you address comments my way
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Regards,
>
> Hans
>
>
> Hans de Goede (9):
> media: v4l: Add v4l2_acpi_parse_sensor_gpios() helper function
> media: atomisp: Switch to v4l2_acpi_parse_sensor_gpios()
> media: atomisp: gc0310: Turn into standard v4l2 sensor driver
> media: atomisp: gc0310: Drop XXGC0310 ACPI hardware-id
> media: atomisp: gc0310: Fix double free in gc0310_remove()
> media: atomisp: gc0310: Cleanup includes
> media: atomisp: gc0310: Remove gc0310_s_config() function
> media: atomisp: gc0310: Remove gc0310.h
> media: Move gc0310 sensor drivers to drivers/media/i2c/
>
> Documentation/driver-api/media/v4l2-acpi.rst | 5 +
> Documentation/driver-api/media/v4l2-core.rst | 1 +
> drivers/media/i2c/Kconfig | 10 +
> drivers/media/i2c/Makefile | 1 +
> .../atomisp-gc0310.c => media/i2c/gc0310.c} | 296 ++++++++++++++---
> drivers/media/v4l2-core/Makefile | 1 +
> drivers/media/v4l2-core/v4l2-acpi.c | 227 +++++++++++++
> drivers/staging/media/atomisp/i2c/Kconfig | 8 -
> drivers/staging/media/atomisp/i2c/Makefile | 1 -
> .../media/atomisp/i2c/atomisp-ov2680.c | 5 +-
> drivers/staging/media/atomisp/i2c/gc0310.h | 309 ------------------
> .../media/atomisp/pci/atomisp_gmin_platform.c | 240 --------------
> include/media/v4l2-acpi.h | 37 +++
> 13 files changed, 540 insertions(+), 601 deletions(-)
> create mode 100644 Documentation/driver-api/media/v4l2-acpi.rst
> rename drivers/{staging/media/atomisp/i2c/atomisp-gc0310.c => media/i2c/gc0310.c} (64%)
> create mode 100644 drivers/media/v4l2-core/v4l2-acpi.c
> delete mode 100644 drivers/staging/media/atomisp/i2c/gc0310.h
> create mode 100644 include/media/v4l2-acpi.h
>
> --
> 2.40.1
>

--
With Best Regards,
Andy Shevchenko