[lkml]   [2021]   [Feb]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH v3 5/6] platform/x86: Add intel_skl_int3472 driver
Hi all

On 22/02/2021 13:07, Daniel Scally wrote:
> diff --git a/drivers/platform/x86/intel-int3472/Kconfig b/drivers/platform/x86/intel-int3472/Kconfig
> new file mode 100644
> index 000000000000..b94622245c21
> --- /dev/null
> +++ b/drivers/platform/x86/intel-int3472/Kconfig
> @@ -0,0 +1,31 @@
> +config INTEL_SKL_INT3472
> + tristate "Intel SkyLake ACPI INT3472 Driver"
> + depends on ACPI
> + depends on REGULATOR
> + depends on GPIOLIB
> + depends on COMMON_CLK && CLKDEV_LOOKUP
> + depends on I2C
> + select MFD_CORE
> + select REGMAP_I2C
> + help
> + This driver adds support for the INT3472 ACPI devices found on some
> + Intel SkyLake devices.
> +
> + The INT3472 is an Intel camera power controller, a logical device
> + found on some Skylake-based systems that can map to different
> + hardware devices depending on the platform. On machines
> + designed for Chrome OS, it maps to a TPS68470 camera PMIC. On
> + machines designed for Windows, it maps to either a TP68470
> + camera PMIC, a uP6641Q sensor PMIC, or a set of discrete GPIOs
> + and power gates.
> +
> + If your device was designed for Chrome OS, this driver will provide
> + an ACPI OpRegion, which must be available before any of the devices
> + using it are probed. For this reason, you should select Y if your
> + device was designed for ChromeOS. For the same reason the
> + I2C_DESIGNWARE_PLATFORM option must be set to Y too.
> +
> + Say Y or M here if you have a SkyLake device designed for use
> + with Windows or ChromeOS. Say N here if you are not sure.
> +
> + The module will be named "intel-skl-int3472"
The Kconfig option for the existing tps68470 driver is a bool which
depends on I2C_DESIGNWARE_PLATFORM=y, giving the following reason:

This option is a bool as it provides an ACPI operation
region, which must be available before any of the devices
using this are probed. This option also configures the
designware-i2c driver to be built-in, for the same reason.

One problem I've faced is that that scenario only applies to some
devices that this new driver can support, so hard-coding it as built in
didn't make much sense. For that reason I opted to set it tristate, but
of course that issue still exists for ChromeOS devices where the
OpRegion will be registered. I opted for simply documenting that
requirement, as is done in aaac4a2eadaa6: "mfd: axp20x-i2c: Document
that this must be builtin on x86", but that's not entirely satisfactory.
Possible alternatives might be setting "depends on
though of course the User would still have to realise they need to
build-in the INTEL_SKL_INT3472 Kconfig option too.

Feedback around this issue would be particularly welcome, as I'm not
sure what the best approach might be.

 \ /
  Last update: 2021-02-22 15:09    [W:0.161 / U:3.184 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site