Re: TPS68470 PMIC config option

Jean Delvare
2021-09-02T09:57:41Z

Hi Sakari,

On Wed, 1 Sep 2021 22:32:51 +0300, Sakari Ailus wrote:
> On Wed, Sep 01, 2021 at 08:39:19PM +0300, Andy Shevchenko wrote:
> > On Wed, Sep 01, 2021 at 04:02:34PM +0200, Jean Delvare wrote:
> > > Is there a reason why config TPS68470_PMIC_OPREGION is not under "if
> > > PMIC_OPREGION" where all other *_PMIC_OPREGION driver options are?
> >
> > It was originally like that.
> >
> > Sakari, do you know?
>
> The answer can be found in Makefile:
>
> obj-$(CONFIG_PMIC_OPREGION) += intel_pmic.o
>
> intel_pmic.c seems to contain common functionality for PMICs in Intel SoCs
> whereas the TPS68470 is an external chip. The two codebases are distinct.
>
> Perhaps it could make sense to either rename this as
> CONFIG_PMIC_INTEL_OPREGION, or move the TPS68470 driver in and change the
> Kconfig+Makefile to have the common code compiled if at least one of the
> drivers is enabled.

OK, thanks for the explanation I get it now. Yes, the fact that the
menu looks vendor-neutral while it is about Intel drivers only is
confusing. Renaming it would help. I'm not sure about your alternative
proposal as I can't actually see any common code or dependency between
intel_pmic and tps68470_pmic.

What about the following?

From: Jean Delvare <jdelvare@suse.de>
Subject: ACPI / PMIC: Rename CONFIG_PMIC_OPREGION

Rename the intel_pmic driver's Kconfig option to make it clear it's
about the Intel chipset family.

Signed-off-by: Jean Delvare <jdelvare@suse.de>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <lenb@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
---
drivers/acpi/pmic/Kconfig | 10 +++++-----
drivers/acpi/pmic/Makefile | 2 +-
drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 4 ++--
drivers/staging/media/atomisp/Kconfig | 2 +-
4 files changed, 9 insertions(+), 9 deletions(-)

--- linux-5.14.orig/drivers/acpi/pmic/Kconfig 2021-08-30 00:04:50.000000000 +0200
+++ linux-5.14/drivers/acpi/pmic/Kconfig 2021-09-02 11:51:14.146662112 +0200
@@ -1,14 +1,14 @@
# SPDX-License-Identifier: GPL-2.0

-menuconfig PMIC_OPREGION
- bool "PMIC (Power Management Integrated Circuit) operation region support"
+menuconfig INTEL_PMIC_OPREGION
+ bool "Intel PMIC (Power Management Integrated Circuit) operation region support"
help
Select this option to enable support for ACPI operation
- region of the PMIC chip. The operation region can be used
+ region of the Intel PMIC chip. The operation region can be used
to control power rails and sensor reading/writing on the
PMIC chip.

-if PMIC_OPREGION
+if INTEL_PMIC_OPREGION

config BYTCRC_PMIC_OPREGION
bool "ACPI operation region support for Bay Trail Crystal Cove PMIC"
@@ -48,7 +48,7 @@ config CHT_DC_TI_PMIC_OPREGION
help
This config adds ACPI operation region support for Dollar Cove TI PMIC.

-endif # PMIC_OPREGION
+endif # INTEL_PMIC_OPREGION

config TPS68470_PMIC_OPREGION
bool "ACPI operation region support for TPS68470 PMIC"
--- linux-5.14.orig/drivers/acpi/pmic/Makefile 2021-08-30 00:04:50.000000000 +0200
+++ linux-5.14/drivers/acpi/pmic/Makefile 2021-09-02 11:21:34.527694178 +0200
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0

-obj-$(CONFIG_PMIC_OPREGION) += intel_pmic.o
+obj-$(CONFIG_INTEL_PMIC_OPREGION) += intel_pmic.o
obj-$(CONFIG_BYTCRC_PMIC_OPREGION) += intel_pmic_bytcrc.o
obj-$(CONFIG_CHTCRC_PMIC_OPREGION) += intel_pmic_chtcrc.o
obj-$(CONFIG_XPOWER_PMIC_OPREGION) += intel_pmic_xpower.o
--- linux-5.14.orig/drivers/staging/media/atomisp/Kconfig 2021-08-30 00:04:50.000000000 +0200
+++ linux-5.14/drivers/staging/media/atomisp/Kconfig 2021-09-02 11:51:39.792007892 +0200
@@ -12,7 +12,7 @@ menuconfig INTEL_ATOMISP
config VIDEO_ATOMISP
tristate "Intel Atom Image Signal Processor Driver"
depends on VIDEO_V4L2 && INTEL_ATOMISP
- depends on PMIC_OPREGION
+ depends on INTEL_PMIC_OPREGION
select IOSF_MBI
select VIDEOBUF_VMALLOC
select VIDEO_V4L2_SUBDEV_API
--- linux-5.14.orig/drivers/gpu/drm/i915/display/intel_dsi_vbt.c 2021-08-30 00:04:50.000000000 +0200
+++ linux-5.14/drivers/gpu/drm/i915/display/intel_dsi_vbt.c 2021-09-02 11:52:38.230795493 +0200
@@ -511,7 +511,7 @@ static const u8 *mipi_exec_spi(struct in
static const u8 *mipi_exec_pmic(struct intel_dsi *intel_dsi, const u8 *data)
{
struct drm_i915_private *i915 = to_i915(intel_dsi->base.base.dev);
-#ifdef CONFIG_PMIC_OPREGION
+#ifdef CONFIG_INTEL_PMIC_OPREGION
u32 value, mask, reg_address;
u16 i2c_address;
int ret;
@@ -529,7 +529,7 @@ static const u8 *mipi_exec_pmic(struct i
drm_err(&i915->drm, "%s failed, error: %d\n", __func__, ret);
#else
drm_err(&i915->drm,
- "Your hardware requires CONFIG_PMIC_OPREGION and it is not set\n");
+ "Your hardware requires CONFIG_INTEL_PMIC_OPREGION and it is not set\n");
#endif

return data + 15;

--
Jean Delvare
SUSE L3 Support