Hi Daniel, On 3/11/23 23:30, Daniel Scally wrote: > ACPI _HID INT347E represents the OmniVision 7251 camera sensor. The > driver for this sensor expects a single pin named "enable", but on > some Microsoft Surface platforms the sensor is assigned a single > GPIO who's type flag is INT3472_GPIO_TYPE_RESET. > > Remap the GPIO pin's function from "reset" to "enable". This is done > outside of the existing remap table since it is a more widespread > discrepancy than that method is designed for. Additionally swap the > polarity of the pin to match the driver's expectation. > > Signed-off-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx> I have recently been looking into GPIO mappings on various devices using the atomisp2 ISP. These use the same _DSM as the IPU3 but then directly on the sensor ACPI device node. What I came up with is this (I still need to submit this upstream): https://github.com/jwrdegoede/linux-sunxi/commit/e2287979db43d46fa7d354c1bde92eb6219b613d One thing which I learned while working on this is that in the value returned by the _DSM the byte with mask 0xff00 is the pin-number on the GPIO controller; and (and this is the important bit!) unlike the assumption in the IPU3/INT3472 code the order in which the GPIOs are listed in the "79234640-9e10-4fea-a5c1-b5aa8b19756f" _DSM is *NOT* always the order in which they are listed in the GPIO resources! So as you can see in the linked commit I'm finding the GPIO resource to go with the _DSM value by matching the pin-numbers. I'm wondering if we need to do the same thing on IPU3 and if this is maybe why we need to do any remapping at all ? Can you please check if there is not something like the above going on before we add a remap quirk for this ? Regards, Hans > --- > drivers/platform/x86/intel/int3472/discrete.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c > index f064da74f50a..2064b3bbe530 100644 > --- a/drivers/platform/x86/intel/int3472/discrete.c > +++ b/drivers/platform/x86/intel/int3472/discrete.c > @@ -98,6 +98,9 @@ static int skl_int3472_map_gpio_to_sensor(struct int3472_discrete_device *int347 > { > const struct int3472_sensor_config *sensor_config; > char *path = agpio->resource_source.string_ptr; > + const struct acpi_device_id ov7251_ids[] = { > + { "INT347E" }, > + }; > struct gpiod_lookup *table_entry; > struct acpi_device *adev; > acpi_handle handle; > @@ -120,6 +123,17 @@ static int skl_int3472_map_gpio_to_sensor(struct int3472_discrete_device *int347 > } > } > > + /* > + * In addition to the function remap table we need to bulk remap the > + * "reset" GPIO for the OmniVision 7251 sensor, as the driver for that > + * expects its only GPIO pin to be called "enable" (and to have the > + * opposite polarity). > + */ > + if (!strcmp(func, "reset") && !acpi_match_device_ids(int3472->sensor, ov7251_ids)) { > + func = "enable"; > + polarity = GPIO_ACTIVE_HIGH; > + } > + > /* Functions mapped to NULL should not be mapped to the sensor */ > if (!func) > return 0;
