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

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

 



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





[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux