| Message ID | 20221129231149.697154-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | ov5693/int3472: Privacy LED handling changes + IPU6 compatibility | expand |
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!
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(-)