[PATCH 0/5] Intel PMIC: Intel PMIC mfd and OPRegion drivers must be built in

[PATCH 0/5] Intel PMIC: Intel PMIC mfd and OPRegion drivers must be built in

[Hans de Goede]

Hi All,

Here is a series enforcing Intel PMIC mfd and OPRegion drivers to be built
in, this is necessary so that the OPRegion handlers are registered before
other drivers try to enter D0 which may require these OPRegion handlers.

Note patches 4 & 5 are for drivers which have been submitted but not
yet accepted in -next, so theoretically they could be squashed into
the original patches, but I believe it is good to keep them as separate
commits to have a git history which documents why these are builtin.

The patches span both the mfd and ACPI subsystems but can me merged
independently of each other I'm posting this as a single series as
both sides are part of fixing the same issue.

Regards,

Hans

[PATCH 1/5] ACPI / PMIC: Stop xpower OPRegion handler relying on IIO

[Hans de Goede]

The intel_pmic_xpower code provides an OPRegion handler, which must be
available before other drivers using it are loaded, which can only be
ensured if both the mfd and opregion drivers are built in, which is why
the Kconfig option for intel_pmic_xpower is a bool.

The use of IIO is causing trouble for generic distro configs here as
distros will typically want to build IIO drivers as modules and there
really is no reason to use IIO here. The reading of the ADC value is a
single regmap_bulk_read, which is already protected against races by
the regmap-lock.

This commit removes the use of IIO, allowing distros to enable the
driver without needing to built IIO in and also actually simplifies
the code.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/Kconfig                  |  2 +-
 drivers/acpi/pmic/intel_pmic_xpower.c | 21 ++++-----------------
 2 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index e3c4ef0..ec09fa2 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -506,7 +506,7 @@ config CRC_PMIC_OPREGION
 
 config XPOWER_PMIC_OPREGION
 	bool "ACPI operation region support for XPower AXP288 PMIC"
-	depends on AXP288_ADC = y
+	depends on MFD_AXP20X_I2C
 	help
 	  This config adds ACPI operation region support for XPower AXP288 PMIC.
 
diff --git a/drivers/acpi/pmic/intel_pmic_xpower.c b/drivers/acpi/pmic/intel_pmic_xpower.c
index e6e991a..55f5111 100644
--- a/drivers/acpi/pmic/intel_pmic_xpower.c
+++ b/drivers/acpi/pmic/intel_pmic_xpower.c
@@ -18,7 +18,6 @@
 #include <linux/mfd/axp20x.h>
 #include <linux/regmap.h>
 #include <linux/platform_device.h>
-#include <linux/iio/consumer.h>
 #include "intel_pmic.h"
 
 #define XPOWER_GPADC_LOW	0x5b
@@ -186,28 +185,16 @@ static int intel_xpower_pmic_update_power(struct regmap *regmap, int reg,
  * @regmap: regmap of the PMIC device
  * @reg: register to get the reading
  *
- * We could get the sensor value by manipulating the HW regs here, but since
- * the axp288 IIO driver may also access the same regs at the same time, the
- * APIs provided by IIO subsystem are used here instead to avoid problems. As
- * a result, the two passed in params are of no actual use.
- *
  * Return a positive value on success, errno on failure.
  */
 static int intel_xpower_pmic_get_raw_temp(struct regmap *regmap, int reg)
 {
-	struct iio_channel *gpadc_chan;
-	int ret, val;
-
-	gpadc_chan = iio_channel_get(NULL, "axp288-system-temp");
-	if (IS_ERR_OR_NULL(gpadc_chan))
-		return -EACCES;
+	u8 buf[2];
 
-	ret = iio_read_channel_raw(gpadc_chan, &val);
-	if (ret < 0)
-		val = ret;
+	if (regmap_bulk_read(regmap, AXP288_GP_ADC_H, buf, 2))
+		return -EIO;
 
-	iio_channel_release(gpadc_chan);
-	return val;
+	return (buf[0] << 4) + ((buf[1] >> 4) & 0x0F);
 }
 
 static struct intel_pmic_opregion_data intel_xpower_pmic_opregion_data = {
-- 
2.9.3

[PATCH 2/5] mfd: intel_soc_pmic: Select designware i2c-bus driver

[Hans de Goede]

The Crystal Cove PMIC provides an ACPI OPRegion handler, which must be
available before other drivers using it are loaded, which is why
INTEL_SOC_PMIC is a bool.

Just having the driver is not enough, the driver for the i2c-bus must
also be built in, to ensure this, this patch adds a select for it.

While at it this patch also changes the human readable name of the Kconfig
option to make clear the INTEL_SOC_PMIC option selects support for the
Intel Crystal Cove PMIC and documents why this is a bool.

Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Note this patch will partially conflicts with (contains the same changes as)
a patch in Andy Shevchenko's tree.
---
 drivers/mfd/Kconfig | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index d427a10..0c6a967 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -425,14 +425,17 @@ config LPC_SCH
 	  System Management Bus and General Purpose I/O.
 
 config INTEL_SOC_PMIC
-	bool "Support for Intel Atom SoC PMIC"
+	# This is a bool as it provides an ACPI Opregion which must be
+	# available as soon as possible
+	bool "Support for Intel Crystal Cove PMIC"
 	depends on GPIOLIB
 	depends on I2C=y
 	select MFD_CORE
 	select REGMAP_I2C
 	select REGMAP_IRQ
+	select I2C_DESIGNWARE_PLATFORM
 	help
-	  Select this option to enable support for the PMIC device
+	  Select this option to enable support for the Crystal Cove PMIC
 	  on some Intel SoC systems. The PMIC provides ADC, GPIO,
 	  thermal, charger and related power management functions
 	  on these systems.
-- 
2.9.3

[PATCH 3/5] mfd: axp20c-i2c: Select designware i2c-bus driver on x86

[Hans de Goede]

On x86 the axp288 PMIC provides an ACPI OPRegion handler, which must be
available before other drivers using it are loaded, which can only be
ensured if the mfd, opregion driver and i2c-bus drivers are built in.

The opregion driver is a bool depending on MFD_AXP20X_I2C ensuring that
it will be builtin if the opregion driver is to be enabled, add a
select for the designware-options to ensure those get built in too.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mfd/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 0c6a967..08f8dcb 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -148,6 +148,8 @@ config MFD_AXP20X_I2C
 	tristate "X-Powers AXP series PMICs with I2C"
 	select MFD_AXP20X
 	select REGMAP_I2C
+	select I2C_DESIGNWARE_PLATFORM if X86
+	select I2C_DESIGNWARE_BAYTRAIL if X86
 	depends on I2C
 	help
 	  If you say Y here you get support for the X-Powers AXP series power
-- 
2.9.3

[PATCH 4/5] mfd: intel_soc_pmic_chtwc: This driver must always be builtin when enabled

[Hans de Goede]

The chtwc PMIC provides an ACPI OPRegion handler, which must be available
before other drivers using it are loaded, which can only be ensured if
the mfd, opregion and i2c-bus drivers are built in.

This commit changes the option for the mfd driver to bool and adds
a select for the i2c-bus driver, ensuring both are built-in if enabled.

This fixes errors like these during boot:

mmc0: SDHCI controller on ACPI [80860F14:00] using ADMA
ACPI Error: No handler for Region [REGS] (ffff93543b0cc3a8) [UserDefinedRegion] (20170119/evregion-166)
ACPI Error: Region UserDefinedRegion (ID=143) has no handler (20170119/exfldio-299)
ACPI Error: Method parse/execution failed [\_SB.PCI0.I2C7.PMI5.GET] (Node ffff93543b0cde10), AE_NOT_EXIST (20170119/psparse-543)
ACPI Error: Method parse/execution failed [\_SB.PCI0.SHC1._PS0] (Node ffff93543b0b5cd0), AE_NOT_EXIST (20170119/psparse-543)
acpi 80860F14:02: Failed to change power state to D0

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mfd/Kconfig | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 08f8dcb..396245d 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -443,12 +443,15 @@ config INTEL_SOC_PMIC
 	  on these systems.
 
 config INTEL_SOC_PMIC_CHTWC
-	tristate "Support for Intel Cherry Trail Whiskey Cove PMIC"
+	# This is a bool as it provides an ACPI Opregion which must be
+	# available as soon as possible
+	bool "Support for Intel Cherry Trail Whiskey Cove PMIC"
 	depends on ACPI
 	depends on I2C
 	select MFD_CORE
 	select REGMAP_I2C
 	select REGMAP_IRQ
+	select I2C_DESIGNWARE_PLATFORM
 	help
 	  Select this option to enable support for the Intel Cherry Trail
 	  Whiskey Cove PMIC found on some Intel Cherry Trail systems.
-- 
2.9.3

[PATCH 5/5] ACPI / PMIC: mfd: intel_pmic_chtwc: This driver must always be builtin

[Hans de Goede]

The chtwc PMIC provides an ACPI OPRegion handler, which must be available
before other drivers using it are loaded, which can only be ensured if
both the mfd and opregion drivers are built in.

This fixes errors like these during boot:

mmc0: SDHCI controller on ACPI [80860F14:00] using ADMA
ACPI Error: No handler for Region [REGS] (ffff93543b0cc3a8) [UserDefinedRegion] (20170119/evregion-166)
ACPI Error: Region UserDefinedRegion (ID=143) has no handler (20170119/exfldio-299)
ACPI Error: Method parse/execution failed [\_SB.PCI0.I2C7.PMI5.GET] (Node ffff93543b0cde10), AE_NOT_EXIST (20170119/psparse-543)
ACPI Error: Method parse/execution failed [\_SB.PCI0.SHC1._PS0] (Node ffff93543b0b5cd0), AE_NOT_EXIST (20170119/psparse-543)
acpi 80860F14:02: Failed to change power state to D0

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index ec09fa2..842530f 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -517,7 +517,7 @@ config BXT_WC_PMIC_OPREGION
 	  This config adds ACPI operation region support for BXT WhiskeyCove PMIC.
 
 config CHT_WC_PMIC_OPREGION
-	tristate "ACPI operation region support for CHT Whiskey Cove PMIC"
+	bool "ACPI operation region support for CHT Whiskey Cove PMIC"
 	depends on INTEL_SOC_PMIC_CHTWC
 	help
 	  This config adds ACPI operation region support for CHT Whiskey Cove PMIC.
-- 
2.9.3

Re: [PATCH 2/5] mfd: intel_soc_pmic: Select designware i2c-bus driver

[Andy Shevchenko]

On Wed, 2017-03-22 at 12:29 +0100, Hans de Goede wrote:
> The Crystal Cove PMIC provides an ACPI OPRegion handler, which must be
> available before other drivers using it are loaded, which is why
> INTEL_SOC_PMIC is a bool.
> 
> Just having the driver is not enough, the driver for the i2c-bus must
> also be built in, to ensure this, this patch adds a select for it.
> 

> While at it this patch also changes the human readable name of the
> Kconfig
> option to make clear the INTEL_SOC_PMIC option selects support for the
> Intel Crystal Cove PMIC and documents why this is a bool.

The above is what my patch does, I'm okay if the change is going with
this series as long as Lee is on the same side. Otherwise I would prefer
to go my changes first.

> 
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Note this patch will partially conflicts with (contains the same
> changes as)
> a patch in Andy Shevchenko's tree.
> ---
>  drivers/mfd/Kconfig | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index d427a10..0c6a967 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -425,14 +425,17 @@ config LPC_SCH
>  	  System Management Bus and General Purpose I/O.
>  
>  config INTEL_SOC_PMIC
> -	bool "Support for Intel Atom SoC PMIC"
> +	# This is a bool as it provides an ACPI Opregion which must
> be
> +	# available as soon as possible
> +	bool "Support for Intel Crystal Cove PMIC"
>  	depends on GPIOLIB
>  	depends on I2C=y
>  	select MFD_CORE
>  	select REGMAP_I2C
>  	select REGMAP_IRQ
> +	select I2C_DESIGNWARE_PLATFORM
>  	help
> -	  Select this option to enable support for the PMIC device
> +	  Select this option to enable support for the Crystal Cove
> PMIC
>  	  on some Intel SoC systems. The PMIC provides ADC, GPIO,
>  	  thermal, charger and related power management functions
>  	  on these systems.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH 2/5] mfd: intel_soc_pmic: Select designware i2c-bus driver

[Lee Jones]

On Wed, 22 Mar 2017, Andy Shevchenko wrote:

> On Wed, 2017-03-22 at 12:29 +0100, Hans de Goede wrote:
> > The Crystal Cove PMIC provides an ACPI OPRegion handler, which must be
> > available before other drivers using it are loaded, which is why
> > INTEL_SOC_PMIC is a bool.
> > 
> > Just having the driver is not enough, the driver for the i2c-bus must
> > also be built in, to ensure this, this patch adds a select for it.
> > 
> 
> > While at it this patch also changes the human readable name of the
> > Kconfig
> > option to make clear the INTEL_SOC_PMIC option selects support for the
> > Intel Crystal Cove PMIC and documents why this is a bool.
> 
> The above is what my patch does, I'm okay if the change is going with
> this series as long as Lee is on the same side. Otherwise I would prefer
> to go my changes first.

I think this set is going to need to be rebase on top of MFD.

> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > ---
> > Note this patch will partially conflicts with (contains the same
> > changes as)
> > a patch in Andy Shevchenko's tree.
> > ---
> >  drivers/mfd/Kconfig | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index d427a10..0c6a967 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -425,14 +425,17 @@ config LPC_SCH
> >  	  System Management Bus and General Purpose I/O.
> >  
> >  config INTEL_SOC_PMIC
> > -	bool "Support for Intel Atom SoC PMIC"
> > +	# This is a bool as it provides an ACPI Opregion which must
> > be
> > +	# available as soon as possible
> > +	bool "Support for Intel Crystal Cove PMIC"
> >  	depends on GPIOLIB
> >  	depends on I2C=y
> >  	select MFD_CORE
> >  	select REGMAP_I2C
> >  	select REGMAP_IRQ
> > +	select I2C_DESIGNWARE_PLATFORM
> >  	help
> > -	  Select this option to enable support for the PMIC device
> > +	  Select this option to enable support for the Crystal Cove
> > PMIC
> >  	  on some Intel SoC systems. The PMIC provides ADC, GPIO,
> >  	  thermal, charger and related power management functions
> >  	  on these systems.
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/5] mfd: intel_soc_pmic: Select designware i2c-bus driver

[Hans de Goede]

Hi,

On 23-03-17 17:46, Lee Jones wrote:
> On Wed, 22 Mar 2017, Andy Shevchenko wrote:
>
>> On Wed, 2017-03-22 at 12:29 +0100, Hans de Goede wrote:
>>> The Crystal Cove PMIC provides an ACPI OPRegion handler, which must be
>>> available before other drivers using it are loaded, which is why
>>> INTEL_SOC_PMIC is a bool.
>>>
>>> Just having the driver is not enough, the driver for the i2c-bus must
>>> also be built in, to ensure this, this patch adds a select for it.
>>>
>>
>>> While at it this patch also changes the human readable name of the
>>> Kconfig
>>> option to make clear the INTEL_SOC_PMIC option selects support for the
>>> Intel Crystal Cove PMIC and documents why this is a bool.
>>
>> The above is what my patch does, I'm okay if the change is going with
>> this series as long as Lee is on the same side. Otherwise I would prefer
>> to go my changes first.
>
> I think this set is going to need to be rebase on top of MFD.

This patch (2/5) and 3/5 should apply cleanly as they only
touch drivers/mfd/Kconfig and mfd-for-next has only 1 commit
touching Kconfig which is no where near the areas these 2
patches are making changes. So I believe they can be applied as is.

As for the other patches, patch 1/5 is for the ACPI subsys, since
you've requested changes to the Cherry Trail Whiskey Cove MFD driver
I will squash 4/5 into the new version of that and 5/5 again is an
ACPI patch.

Regards,

Hans



>
>>> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>> Note this patch will partially conflicts with (contains the same
>>> changes as)
>>> a patch in Andy Shevchenko's tree.
>>> ---
>>>  drivers/mfd/Kconfig | 7 +++++--
>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>>> index d427a10..0c6a967 100644
>>> --- a/drivers/mfd/Kconfig
>>> +++ b/drivers/mfd/Kconfig
>>> @@ -425,14 +425,17 @@ config LPC_SCH
>>>  	  System Management Bus and General Purpose I/O.
>>>
>>>  config INTEL_SOC_PMIC
>>> -	bool "Support for Intel Atom SoC PMIC"
>>> +	# This is a bool as it provides an ACPI Opregion which must
>>> be
>>> +	# available as soon as possible
>>> +	bool "Support for Intel Crystal Cove PMIC"
>>>  	depends on GPIOLIB
>>>  	depends on I2C=y
>>>  	select MFD_CORE
>>>  	select REGMAP_I2C
>>>  	select REGMAP_IRQ
>>> +	select I2C_DESIGNWARE_PLATFORM
>>>  	help
>>> -	  Select this option to enable support for the PMIC device
>>> +	  Select this option to enable support for the Crystal Cove
>>> PMIC
>>>  	  on some Intel SoC systems. The PMIC provides ADC, GPIO,
>>>  	  thermal, charger and related power management functions
>>>  	  on these systems.
>>
>