On Thu, May 18, 2023 at 6:32 PM Hans de Goede <hdegoede@xxxxxxxxxx> 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@xxxxxxxxx> > 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