[0/6] ov5693/int3472: Privacy LED handling changes + IPU6 compatibility

  Hi All,

The out of tree IPU6 driver has moved to using the in kernel INT3472
code for doing power-ctrl rather then doing their own thing (good!).

Some of the IPU6 devices with a discrete INT3472 ACPI device have a
privacy-led GPIO. but no clk-enable GPIO. To make this work this series
moves the privacy LED control from being integrated with the clk-provider
to modelling the privacy LED as a separate GPIO. This also brings the
discrete INT3472 ACPI device privacy LED handling inline with the privacy
LED handling for INT3472 TPS68470 PMIC devices which I posted here:

https://lore.kernel.org/platform-driver-x86/20221128214408.165726-1-hdegoede@redhat.com/

This obsoletes my previous "[PATCH 0/3] platform/x86: int3472/discrete:
Make it work with IPU6" series:

https://lore.kernel.org/platform-driver-x86/20221124200007.390901-1-hdegoede@redhat.com/

Mauro since laptops with IPU6 cameras are becoming more and more
popular I would like to get this merged for 6.2 so that with 6.2
users will be able to build the out of tree IPU6 driver without
requiring patching their main kernel. I realize we are a bit
late in the cycle, but can you please still take the ov5693 patch
for 6.2 ? It is quite small / straight-forward and since it used
gpiod_get_optional() it is a no-op without the rest of this series.

This series has been tested on:

- Lenovo ThinkPad X1 Yoga gen 7, IPU6, front: ov2740 with privacy LED
- Dell Latitude 9420, IPU 6 with privacy LED on front
- Mirosoft Surface Go, IPU3, front: ov5693 with privacy LED,
                              back: ov8865 with privacy LED

Regards,

Hans


Hans de Goede (6):
  media: ov5693: Add support for a privacy-led GPIO
  platform/x86: int3472/discrete: Refactor GPIO to sensor mapping
  platform/x86: int3472/discrete: Treat privacy LED as regular GPIO
  platform/x86: int3472/discrete: Move GPIO request to
    skl_int3472_register_clock()
  platform/x86: int3472/discrete: Ensure the clk/power enable pins are
    in output mode
  platform/x86: int3472/discrete: Get the polarity from the _DSM entry

 drivers/media/i2c/ov5693.c                    | 10 ++
 .../x86/intel/int3472/clk_and_regulator.c     | 35 +++++--
 drivers/platform/x86/intel/int3472/common.h   |  4 +-
 drivers/platform/x86/intel/int3472/discrete.c | 95 ++++++++-----------
 4 files changed, 80 insertions(+), 64 deletions(-)
  

On Wed, Nov 30, 2022 at 1:12 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi All,
>
> The out of tree IPU6 driver has moved to using the in kernel INT3472
> code for doing power-ctrl rather then doing their own thing (good!).

than

> Some of the IPU6 devices with a discrete INT3472 ACPI device have a
> privacy-led GPIO. but no clk-enable GPIO. To make this work this series
> moves the privacy LED control from being integrated with the clk-provider
> to modelling the privacy LED as a separate GPIO. This also brings the
> discrete INT3472 ACPI device privacy LED handling inline with the privacy
> LED handling for INT3472 TPS68470 PMIC devices which I posted here:
>
> https://lore.kernel.org/platform-driver-x86/20221128214408.165726-1-hdegoede@redhat.com/
>
> This obsoletes my previous "[PATCH 0/3] platform/x86: int3472/discrete:
> Make it work with IPU6" series:
>
> https://lore.kernel.org/platform-driver-x86/20221124200007.390901-1-hdegoede@redhat.com/
>
> Mauro since laptops with IPU6 cameras are becoming more and more
> popular I would like to get this merged for 6.2 so that with 6.2
> users will be able to build the out of tree IPU6 driver without
> requiring patching their main kernel. I realize we are a bit
> late in the cycle, but can you please still take the ov5693 patch
> for 6.2 ? It is quite small / straight-forward and since it used
> gpiod_get_optional() it is a no-op without the rest of this series.
>
> This series has been tested on:
>
> - Lenovo ThinkPad X1 Yoga gen 7, IPU6, front: ov2740 with privacy LED
> - Dell Latitude 9420, IPU 6 with privacy LED on front
> - Mirosoft Surface Go, IPU3, front: ov5693 with privacy LED,

Microsoft?

>                               back: ov8865 with privacy LED

I like this series! Minimum invasion and code.
  

Hi,

On 11/30/22 11:03, Andy Shevchenko wrote:
> On Wed, Nov 30, 2022 at 1:12 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi All,
>>
>> The out of tree IPU6 driver has moved to using the in kernel INT3472
>> code for doing power-ctrl rather then doing their own thing (good!).
> 
> than
> 
>> Some of the IPU6 devices with a discrete INT3472 ACPI device have a
>> privacy-led GPIO. but no clk-enable GPIO. To make this work this series
>> moves the privacy LED control from being integrated with the clk-provider
>> to modelling the privacy LED as a separate GPIO. This also brings the
>> discrete INT3472 ACPI device privacy LED handling inline with the privacy
>> LED handling for INT3472 TPS68470 PMIC devices which I posted here:
>>
>> https://lore.kernel.org/platform-driver-x86/20221128214408.165726-1-hdegoede@redhat.com/
>>
>> This obsoletes my previous "[PATCH 0/3] platform/x86: int3472/discrete:
>> Make it work with IPU6" series:
>>
>> https://lore.kernel.org/platform-driver-x86/20221124200007.390901-1-hdegoede@redhat.com/
>>
>> Mauro since laptops with IPU6 cameras are becoming more and more
>> popular I would like to get this merged for 6.2 so that with 6.2
>> users will be able to build the out of tree IPU6 driver without
>> requiring patching their main kernel. I realize we are a bit
>> late in the cycle, but can you please still take the ov5693 patch
>> for 6.2 ? It is quite small / straight-forward and since it used
>> gpiod_get_optional() it is a no-op without the rest of this series.
>>
>> This series has been tested on:
>>
>> - Lenovo ThinkPad X1 Yoga gen 7, IPU6, front: ov2740 with privacy LED
>> - Dell Latitude 9420, IPU 6 with privacy LED on front
>> - Mirosoft Surface Go, IPU3, front: ov5693 with privacy LED,
> 
> Microsoft?

Ack.

>>                               back: ov8865 with privacy LED
> 
> I like this series! Minimum invasion and code.

I'm glad you like it and thank you for the review.

Regards,

Hans
  

On Wed, Nov 30, 2022 at 12:11:43AM +0100, Hans de Goede wrote:
> Hi All,
> 
> The out of tree IPU6 driver has moved to using the in kernel INT3472
> code for doing power-ctrl rather then doing their own thing (good!).
> 
> Some of the IPU6 devices with a discrete INT3472 ACPI device have a
> privacy-led GPIO. but no clk-enable GPIO. To make this work this series
> moves the privacy LED control from being integrated with the clk-provider
> to modelling the privacy LED as a separate GPIO. This also brings the
> discrete INT3472 ACPI device privacy LED handling inline with the privacy
> LED handling for INT3472 TPS68470 PMIC devices which I posted here:
> 
> https://lore.kernel.org/platform-driver-x86/20221128214408.165726-1-hdegoede@redhat.com/
> 
> This obsoletes my previous "[PATCH 0/3] platform/x86: int3472/discrete:
> Make it work with IPU6" series:
> 
> https://lore.kernel.org/platform-driver-x86/20221124200007.390901-1-hdegoede@redhat.com/
> 
> Mauro since laptops with IPU6 cameras are becoming more and more
> popular I would like to get this merged for 6.2 so that with 6.2
> users will be able to build the out of tree IPU6 driver without
> requiring patching their main kernel. I realize we are a bit
> late in the cycle, but can you please still take the ov5693 patch
> for 6.2 ? It is quite small / straight-forward and since it used
> gpiod_get_optional() it is a no-op without the rest of this series.
> 
> This series has been tested on:
> 
> - Lenovo ThinkPad X1 Yoga gen 7, IPU6, front: ov2740 with privacy LED
> - Dell Latitude 9420, IPU 6 with privacy LED on front
> - Mirosoft Surface Go, IPU3, front: ov5693 with privacy LED,
>                               back: ov8865 with privacy LED

FWIW,
Reviewed-by: Andy Shevchenko <andy@kernel.org>
assuming nit-picks will be addressed as agreed.
  

Hi Hans,

On Wed, Nov 30, 2022 at 12:11:43AM +0100, Hans de Goede wrote:
> Hi All,
> 
> The out of tree IPU6 driver has moved to using the in kernel INT3472
> code for doing power-ctrl rather then doing their own thing (good!).

For the set, with comments addressed:

Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
  

Hi All,

On 11/30/22 00:11, Hans de Goede wrote:
> Hi All,
> 
> The out of tree IPU6 driver has moved to using the in kernel INT3472
> code for doing power-ctrl rather then doing their own thing (good!).
> 
> Some of the IPU6 devices with a discrete INT3472 ACPI device have a
> privacy-led GPIO. but no clk-enable GPIO. To make this work this series
> moves the privacy LED control from being integrated with the clk-provider
> to modelling the privacy LED as a separate GPIO. This also brings the
> discrete INT3472 ACPI device privacy LED handling inline with the privacy
> LED handling for INT3472 TPS68470 PMIC devices which I posted here:
> 
> https://lore.kernel.org/platform-driver-x86/20221128214408.165726-1-hdegoede@redhat.com/
> 
> This obsoletes my previous "[PATCH 0/3] platform/x86: int3472/discrete:
> Make it work with IPU6" series:
> 
> https://lore.kernel.org/platform-driver-x86/20221124200007.390901-1-hdegoede@redhat.com/
> 
> Mauro since laptops with IPU6 cameras are becoming more and more
> popular I would like to get this merged for 6.2 so that with 6.2
> users will be able to build the out of tree IPU6 driver without
> requiring patching their main kernel. I realize we are a bit
> late in the cycle, but can you please still take the ov5693 patch
> for 6.2 ? It is quite small / straight-forward and since it used
> gpiod_get_optional() it is a no-op without the rest of this series.
> 
> This series has been tested on:
> 
> - Lenovo ThinkPad X1 Yoga gen 7, IPU6, front: ov2740 with privacy LED
> - Dell Latitude 9420, IPU 6 with privacy LED on front
> - Mirosoft Surface Go, IPU3, front: ov5693 with privacy LED,
>                               back: ov8865 with privacy LED

There has once again been push-back against the concept using
plain GPIOs for the privacy LED controls rather then wrapping
this in a LED class device. This time in the related series
adding support for the privacy LED on the back of Surface Go
devices:

https://lore.kernel.org/platform-driver-x86/20221128214408.165726-1-hdegoede@redhat.com/

Given all the comments / requests to use the LED class for this
I'm going to attempt to do that, see the above thread for some
challenges which I already encountered while exploring LED class
usage for this + proposed solution for those (adding a lookup
table mechanism to the LED class code similar to the existing
GPIO lookup table support).

This will result in a partial rewrite of this series, so self
NACK for this version of the series.

Andy this also means that I will not be using your new str_high_low()
helper function. The code which could use this will likely stay
around, but given that I need to do a rewrite and then get ne
reviews, it would IMHO be better to just get your series starting with:

[PATCH v1 1/3] lib/string_helpers: Add missing header files to MAINTAINERS database

upstream independently and then later my code can be moved over
to the helper (or if the helper lands first maybe use it from
day one), either way it seems best to decouple the merging
of these 2 series from each other.

Regards,

Hans












> Hans de Goede (6):
>   media: ov5693: Add support for a privacy-led GPIO
>   platform/x86: int3472/discrete: Refactor GPIO to sensor mapping
>   platform/x86: int3472/discrete: Treat privacy LED as regular GPIO
>   platform/x86: int3472/discrete: Move GPIO request to
>     skl_int3472_register_clock()
>   platform/x86: int3472/discrete: Ensure the clk/power enable pins are
>     in output mode
>   platform/x86: int3472/discrete: Get the polarity from the _DSM entry
> 
>  drivers/media/i2c/ov5693.c                    | 10 ++
>  .../x86/intel/int3472/clk_and_regulator.c     | 35 +++++--
>  drivers/platform/x86/intel/int3472/common.h   |  4 +-
>  drivers/platform/x86/intel/int3472/discrete.c | 95 ++++++++-----------
>  4 files changed, 80 insertions(+), 64 deletions(-)
>
  

On Wed, Dec 7, 2022 at 7:34 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 11/30/22 00:11, Hans de Goede wrote:

...

> Andy this also means that I will not be using your new str_high_low()
> helper function. The code which could use this will likely stay
> around, but given that I need to do a rewrite and then get ne
> reviews, it would IMHO be better to just get your series starting with:
>
> [PATCH v1 1/3] lib/string_helpers: Add missing header files to MAINTAINERS database
>
> upstream independently and then later my code can be moved over
> to the helper (or if the helper lands first maybe use it from
> day one), either way it seems best to decouple the merging
> of these 2 series from each other.

Sure, no problem and thank you for the information!