Linux-ACPI Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v3 0/3] Fix dependencies for ACPI drivers with OpRegion settings
@ 2020-05-18 11:42 Mauro Carvalho Chehab
  2020-05-18 11:42 ` [PATCH v3 3/3] mfd: ensure that AXP20X_I2C will have the right deps on X86 Mauro Carvalho Chehab
  0 siblings, 1 reply; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2020-05-18 11:42 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Hans de Goede, Stefan Roese, Lee Jones,
	Alexandre Belloni, Jarkko Nikula, Randy Dunlap,
	Krzysztof Kozlowski, Len Brown, Rafael J. Wysocki, linux-acpi,
	Jean Delvare, linux-i2c, Max Staudt, Wolfram Sang

In order for drivers to work properly with OpRegion, there are some
requirements:

	- The I2C core should be builtin;
	- The I2C designware driver should be builtin;
	- The driver itself should also be buildin;
	- The I2C core should be build with support for ACPI OpRegion.

-

v3:
    - Added Hans reviewed-by for the first two patches
    - Added a third patch fixing dependencies for axp20x driver

Mauro Carvalho Chehab (3):
  mfd: Kconfig: change INTEL_SOC_PMIC_CHTDC_TI to bool
  mfd: Kconfig: fix help texts for drivers that depends on I2C=y
  mfd: ensure that AXP20X_I2C will have the right deps on X86

 drivers/acpi/Kconfig       |  3 ++-
 drivers/i2c/busses/Kconfig |  4 ++--
 drivers/mfd/Kconfig        | 40 +++++++++++++++++++++++++-------------
 drivers/mfd/Makefile       |  8 +++++---
 4 files changed, 36 insertions(+), 19 deletions(-)

-- 
2.26.2



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 3/3] mfd: ensure that AXP20X_I2C will have the right deps on X86
  2020-05-18 11:42 [PATCH v3 0/3] Fix dependencies for ACPI drivers with OpRegion settings Mauro Carvalho Chehab
@ 2020-05-18 11:42 ` Mauro Carvalho Chehab
  2020-05-18 12:17   ` Hans de Goede
  0 siblings, 1 reply; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2020-05-18 11:42 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Hans de Goede, Rafael J. Wysocki,
	Len Brown, Lee Jones, Wolfram Sang, Jean Delvare, Jarkko Nikula,
	Krzysztof Kozlowski, Alexandre Belloni, Max Staudt, Stefan Roese,
	linux-acpi, linux-i2c

The axp20x I2C driver can be used on X86, but also on ARM
platforms.

Yet, for X86, it has to be builtin and need ACPI OpRegion
support enabled.

So, the dependency chain is diferent for X86 and for other
archs.

Change the dependency chain to take this into consideration,
ensuring that everything will be set as it should.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/acpi/Kconfig       |  3 ++-
 drivers/i2c/busses/Kconfig |  4 ++--
 drivers/mfd/Kconfig        | 21 ++++++++++++++++-----
 drivers/mfd/Makefile       |  8 +++++---
 4 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index ce2730d61a8f..4490200ef134 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -547,7 +547,8 @@ config CHTCRC_PMIC_OPREGION
 
 config XPOWER_PMIC_OPREGION
 	bool "ACPI operation region support for XPower AXP288 PMIC"
-	depends on MFD_AXP20X_I2C && IOSF_MBI=y
+	depends on MFD_AXP20X_I2C
+	select IOSF_MBI
 	help
 	  This config adds ACPI operation region support for XPower AXP288 PMIC.
 
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 2ddca08f8a76..202e743c8e3e 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -563,8 +563,8 @@ config I2C_DESIGNWARE_PCI
 config I2C_DESIGNWARE_BAYTRAIL
 	bool "Intel Baytrail I2C semaphore support"
 	depends on ACPI
-	depends on (I2C_DESIGNWARE_PLATFORM=m && IOSF_MBI) || \
-		   (I2C_DESIGNWARE_PLATFORM=y && IOSF_MBI=y)
+	depends on I2C_DESIGNWARE_PLATFORM=y
+	select IOSF_MBI
 	help
 	  This driver enables managed host access to the PMIC I2C bus on select
 	  Intel BayTrail platforms using the X-Powers AXP288 PMIC. It allows
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 167f117ff015..11bbbed30098 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -187,7 +187,7 @@ config MFD_AXP20X_I2C
 	tristate "X-Powers AXP series PMICs with I2C"
 	select MFD_AXP20X
 	select REGMAP_I2C
-	depends on I2C
+	depends on !X86 || (X86 && I2C_DESIGNWARE_PLATFORM=y && I2C_DESIGNWARE_BAYTRAIL=y && ACPI_I2C_OPREGION)
 	help
 	  If you say Y here you get support for the X-Powers AXP series power
 	  management ICs (PMICs) controlled with I2C.
@@ -195,10 +195,21 @@ config MFD_AXP20X_I2C
 	  components like regulators or the PEK (Power Enable Key) under the
 	  corresponding menus.
 
-	  Note on x86 this provides an ACPI OpRegion, so this must be 'y'
-	  (builtin) and not a module, as the OpRegion must be available as
-	  soon as possible. For the same reason the I2C bus driver options
-	  I2C_DESIGNWARE_PLATFORM and I2C_DESIGNWARE_BAYTRAIL must be 'y' too.
+	  Note on x86 this provides an ACPI OpRegion, so enabling it
+	  requires that I2C_DESIGNWARE_PLATFORM and I2C_DESIGNWARE_BAYTRAIL
+	  to be set as 'y'. On such architecture, the driver, if enabled,
+	  will always be (builtin) and not a module, as the OpRegion must be
+	  available as soon as possible.
+
+config MFD_AXP20X_X86
+	bool
+	depends on X86
+	default MFD_AXP20X_I2C
+
+config MFD_AXP20X_NOT_X86
+	tristate
+	depends on !X86
+	default MFD_AXP20X_I2C
 
 config MFD_AXP20X_RSB
 	tristate "X-Powers AXP series PMICs with RSB"
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f935d10cbf0f..dd902b5c31ff 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -142,9 +142,11 @@ obj-$(CONFIG_MFD_DA9052_SPI)	+= da9052-spi.o
 obj-$(CONFIG_MFD_DA9052_I2C)	+= da9052-i2c.o
 
 obj-$(CONFIG_MFD_AC100)		+= ac100.o
-obj-$(CONFIG_MFD_AXP20X)	+= axp20x.o
-obj-$(CONFIG_MFD_AXP20X_I2C)	+= axp20x-i2c.o
-obj-$(CONFIG_MFD_AXP20X_RSB)	+= axp20x-rsb.o
+
+obj-$(CONFIG_MFD_AXP20X)		+= axp20x.o
+obj-$(CONFIG_MFD_AXP20X_I2C_X86)	+= axp20x-i2c.o
+obj-$(CONFIG_MFD_AXP20X_I2C_NOT_X86)	+= axp20x-i2c.o
+obj-$(CONFIG_MFD_AXP20X_RSB)		+= axp20x-rsb.o
 
 obj-$(CONFIG_MFD_LP3943)	+= lp3943.o
 obj-$(CONFIG_MFD_LP8788)	+= lp8788.o lp8788-irq.o
-- 
2.26.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 3/3] mfd: ensure that AXP20X_I2C will have the right deps on X86
  2020-05-18 11:42 ` [PATCH v3 3/3] mfd: ensure that AXP20X_I2C will have the right deps on X86 Mauro Carvalho Chehab
@ 2020-05-18 12:17   ` Hans de Goede
  2020-05-18 12:53     ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2020-05-18 12:17 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Rafael J. Wysocki, Len Brown, Lee Jones, Wolfram Sang,
	Jean Delvare, Jarkko Nikula, Krzysztof Kozlowski,
	Alexandre Belloni, Max Staudt, Stefan Roese, linux-acpi,
	linux-i2c

Hi,

On 5/18/20 1:42 PM, Mauro Carvalho Chehab wrote:
> The axp20x I2C driver can be used on X86, but also on ARM
> platforms.
> 
> Yet, for X86, it has to be builtin and need ACPI OpRegion
> support enabled.
> 
> So, the dependency chain is diferent for X86 and for other
> archs.
> 
> Change the dependency chain to take this into consideration,
> ensuring that everything will be set as it should.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Hmm, last time we tried something like this (it was tried before,
but in a bit different way) we ran into all kind of dependency /
select cycles / issues.

With that said I'm fine with giving this another try, maybe let
the test builders / rand config builds play with it for a while
and see what happens?

Regards,

Hans




> ---
>   drivers/acpi/Kconfig       |  3 ++-
>   drivers/i2c/busses/Kconfig |  4 ++--
>   drivers/mfd/Kconfig        | 21 ++++++++++++++++-----
>   drivers/mfd/Makefile       |  8 +++++---
>   4 files changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index ce2730d61a8f..4490200ef134 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -547,7 +547,8 @@ config CHTCRC_PMIC_OPREGION
>   
>   config XPOWER_PMIC_OPREGION
>   	bool "ACPI operation region support for XPower AXP288 PMIC"
> -	depends on MFD_AXP20X_I2C && IOSF_MBI=y
> +	depends on MFD_AXP20X_I2C
> +	select IOSF_MBI
>   	help
>   	  This config adds ACPI operation region support for XPower AXP288 PMIC.
>   
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 2ddca08f8a76..202e743c8e3e 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -563,8 +563,8 @@ config I2C_DESIGNWARE_PCI
>   config I2C_DESIGNWARE_BAYTRAIL
>   	bool "Intel Baytrail I2C semaphore support"
>   	depends on ACPI
> -	depends on (I2C_DESIGNWARE_PLATFORM=m && IOSF_MBI) || \
> -		   (I2C_DESIGNWARE_PLATFORM=y && IOSF_MBI=y)
> +	depends on I2C_DESIGNWARE_PLATFORM=y
> +	select IOSF_MBI
>   	help
>   	  This driver enables managed host access to the PMIC I2C bus on select
>   	  Intel BayTrail platforms using the X-Powers AXP288 PMIC. It allows
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 167f117ff015..11bbbed30098 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -187,7 +187,7 @@ config MFD_AXP20X_I2C
>   	tristate "X-Powers AXP series PMICs with I2C"
>   	select MFD_AXP20X
>   	select REGMAP_I2C
> -	depends on I2C
> +	depends on !X86 || (X86 && I2C_DESIGNWARE_PLATFORM=y && I2C_DESIGNWARE_BAYTRAIL=y && ACPI_I2C_OPREGION)
>   	help
>   	  If you say Y here you get support for the X-Powers AXP series power
>   	  management ICs (PMICs) controlled with I2C.
> @@ -195,10 +195,21 @@ config MFD_AXP20X_I2C
>   	  components like regulators or the PEK (Power Enable Key) under the
>   	  corresponding menus.
>   
> -	  Note on x86 this provides an ACPI OpRegion, so this must be 'y'
> -	  (builtin) and not a module, as the OpRegion must be available as
> -	  soon as possible. For the same reason the I2C bus driver options
> -	  I2C_DESIGNWARE_PLATFORM and I2C_DESIGNWARE_BAYTRAIL must be 'y' too.
> +	  Note on x86 this provides an ACPI OpRegion, so enabling it
> +	  requires that I2C_DESIGNWARE_PLATFORM and I2C_DESIGNWARE_BAYTRAIL
> +	  to be set as 'y'. On such architecture, the driver, if enabled,
> +	  will always be (builtin) and not a module, as the OpRegion must be
> +	  available as soon as possible.
> +
> +config MFD_AXP20X_X86
> +	bool
> +	depends on X86
> +	default MFD_AXP20X_I2C
> +
> +config MFD_AXP20X_NOT_X86
> +	tristate
> +	depends on !X86
> +	default MFD_AXP20X_I2C
>   
>   config MFD_AXP20X_RSB
>   	tristate "X-Powers AXP series PMICs with RSB"
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index f935d10cbf0f..dd902b5c31ff 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -142,9 +142,11 @@ obj-$(CONFIG_MFD_DA9052_SPI)	+= da9052-spi.o
>   obj-$(CONFIG_MFD_DA9052_I2C)	+= da9052-i2c.o
>   
>   obj-$(CONFIG_MFD_AC100)		+= ac100.o
> -obj-$(CONFIG_MFD_AXP20X)	+= axp20x.o
> -obj-$(CONFIG_MFD_AXP20X_I2C)	+= axp20x-i2c.o
> -obj-$(CONFIG_MFD_AXP20X_RSB)	+= axp20x-rsb.o
> +
> +obj-$(CONFIG_MFD_AXP20X)		+= axp20x.o
> +obj-$(CONFIG_MFD_AXP20X_I2C_X86)	+= axp20x-i2c.o
> +obj-$(CONFIG_MFD_AXP20X_I2C_NOT_X86)	+= axp20x-i2c.o
> +obj-$(CONFIG_MFD_AXP20X_RSB)		+= axp20x-rsb.o
>   
>   obj-$(CONFIG_MFD_LP3943)	+= lp3943.o
>   obj-$(CONFIG_MFD_LP8788)	+= lp8788.o lp8788-irq.o
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 3/3] mfd: ensure that AXP20X_I2C will have the right deps on X86
  2020-05-18 12:17   ` Hans de Goede
@ 2020-05-18 12:53     ` Mauro Carvalho Chehab
  2020-05-20 13:34       ` Wolfram Sang
  0 siblings, 1 reply; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2020-05-18 12:53 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Rafael J. Wysocki, Len Brown, Lee Jones, Wolfram Sang,
	Jean Delvare, Jarkko Nikula, Krzysztof Kozlowski,
	Alexandre Belloni, Max Staudt, Stefan Roese, linux-acpi,
	linux-i2c

Em Mon, 18 May 2020 14:17:06 +0200
Hans de Goede <hdegoede@redhat.com> escreveu:

> Hi,
> 
> On 5/18/20 1:42 PM, Mauro Carvalho Chehab wrote:
> > The axp20x I2C driver can be used on X86, but also on ARM
> > platforms.
> > 
> > Yet, for X86, it has to be builtin and need ACPI OpRegion
> > support enabled.
> > 
> > So, the dependency chain is diferent for X86 and for other
> > archs.
> > 
> > Change the dependency chain to take this into consideration,
> > ensuring that everything will be set as it should.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>  
> 
> Hmm, last time we tried something like this (it was tried before,
> but in a bit different way) we ran into all kind of dependency /
> select cycles / issues.

Yeah, changes like that could cause troubles, specially where
select is used. With the approach I took, there's just one
new select for "IOSF_MBI".

I double-checked that, on most places, this feature is selected.
After this patch, there will be only three "depends on IOSF_MBI",
that could likely be also converted to "select":

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 0ad7ad8cf8e1..d99ad532e17a 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -740,7 +740,8 @@ config THINKPAD_ACPI_HOTKEY_POLL
 
 config INTEL_ATOMISP2_PM
 	tristate "Intel AtomISP2 dummy / power-management driver"
-	depends on PCI && IOSF_MBI && PM
+	depends on PCI && PM
+	select IOSF_MBI
 	help
 	  Power-management driver for Intel's Image Signal Processor found on
 	  Bay Trail and Cherry Trail devices. This dummy driver's sole purpose
@@ -1185,7 +1186,8 @@ config TOUCHSCREEN_DMI
 
 config INTEL_IMR
 	bool "Intel Isolated Memory Region support"
-	depends on X86_INTEL_QUARK && IOSF_MBI
+	depends on X86_INTEL_QUARK
+	select IOSF_MBI
 	---help---
 	  This option provides a means to manipulate Isolated Memory Regions.
 	  IMRs are a set of registers that define read and write access masks
diff --git a/drivers/powercap/Kconfig b/drivers/powercap/Kconfig
index dc1c1381d7fa..f4a3f110c720 100644
--- a/drivers/powercap/Kconfig
+++ b/drivers/powercap/Kconfig
@@ -21,7 +21,8 @@ config INTEL_RAPL_CORE
 
 config INTEL_RAPL
 	tristate "Intel RAPL Support via MSR Interface"
-	depends on X86 && IOSF_MBI
+	depends on X86
+	select IOSF_MBI
 	select INTEL_RAPL_CORE
 	---help---
 	  This enables support for the Intel Running Average Power Limit (RAPL)

The one for INTEL_IMR could even be dropped, as config 
X86_INTEL_QUARK already selects it too.

> With that said I'm fine with giving this another try, maybe let
> the test builders / rand config builds play with it for a while
> and see what happens?

Yeah, it makes sense to me.

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 3/3] mfd: ensure that AXP20X_I2C will have the right deps on X86
  2020-05-18 12:53     ` Mauro Carvalho Chehab
@ 2020-05-20 13:34       ` Wolfram Sang
  2020-05-20 13:46         ` Jarkko Nikula
  0 siblings, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2020-05-20 13:34 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Hans de Goede, Rafael J. Wysocki, Len Brown, Lee Jones,
	Jean Delvare, Jarkko Nikula, Krzysztof Kozlowski,
	Alexandre Belloni, Max Staudt, Stefan Roese, linux-acpi,
	linux-i2c


[-- Attachment #1: Type: text/plain, Size: 325 bytes --]


> > With that said I'm fine with giving this another try, maybe let
> > the test builders / rand config builds play with it for a while
> > and see what happens?
> 
> Yeah, it makes sense to me.

I'd like to see an ACK from the designware maintainers before sending
this upstream. Testing is all fine, of course.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 3/3] mfd: ensure that AXP20X_I2C will have the right deps on X86
  2020-05-20 13:34       ` Wolfram Sang
@ 2020-05-20 13:46         ` Jarkko Nikula
  0 siblings, 0 replies; 6+ messages in thread
From: Jarkko Nikula @ 2020-05-20 13:46 UTC (permalink / raw)
  To: Wolfram Sang, Mauro Carvalho Chehab
  Cc: Hans de Goede, Rafael J. Wysocki, Len Brown, Lee Jones,
	Jean Delvare, Krzysztof Kozlowski, Alexandre Belloni, Max Staudt,
	Stefan Roese, linux-acpi, linux-i2c

On 5/20/20 4:34 PM, Wolfram Sang wrote:
> 
>>> With that said I'm fine with giving this another try, maybe let
>>> the test builders / rand config builds play with it for a while
>>> and see what happens?
>>
>> Yeah, it makes sense to me.
> 
> I'd like to see an ACK from the designware maintainers before sending
> this upstream. Testing is all fine, of course.
> 
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-18 11:42 [PATCH v3 0/3] Fix dependencies for ACPI drivers with OpRegion settings Mauro Carvalho Chehab
2020-05-18 11:42 ` [PATCH v3 3/3] mfd: ensure that AXP20X_I2C will have the right deps on X86 Mauro Carvalho Chehab
2020-05-18 12:17   ` Hans de Goede
2020-05-18 12:53     ` Mauro Carvalho Chehab
2020-05-20 13:34       ` Wolfram Sang
2020-05-20 13:46         ` Jarkko Nikula

Linux-ACPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-acpi/0 linux-acpi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-acpi linux-acpi/ https://lore.kernel.org/linux-acpi \
		linux-acpi@vger.kernel.org
	public-inbox-index linux-acpi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-acpi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git