All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 1/2] mfd: intel_soc_pmic: Select designware i2c-bus driver
@ 2017-05-15 18:23 Hans de Goede
  2017-05-15 18:23 ` [PATCH v4 2/2] mfd: axp20x-i2c: Document that this must be builtin on x86 Hans de Goede
  2017-05-22 10:57 ` [PATCH v4 1/2] mfd: intel_soc_pmic: Select designware i2c-bus driver Lee Jones
  0 siblings, 2 replies; 8+ messages in thread
From: Hans de Goede @ 2017-05-15 18:23 UTC (permalink / raw)
  To: Lee Jones; +Cc: Hans de Goede, linux-kernel, Andy Shevchenko

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.

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

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.
---
Changes in v2:
-Fix Kconfig depends and selects to fix warning reported by kbuild test robot
-Improve commit msg (add example of ACPI errors this avoids)
Changes in v3:
-No changes
Changes in v4:
-Add explanation why this is a bool and why it selects i2c-designwaree
 to the help text rather then as comments in the Kconfig
---
 drivers/mfd/Kconfig | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 1b6a83a4a114..61ebda81e134 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -448,17 +448,24 @@ config LPC_SCH
 
 config INTEL_SOC_PMIC
 	bool "Support for Crystal Cove PMIC"
-	depends on GPIOLIB
-	depends on I2C=y
+	depends on HAS_IOMEM
+	select GPIOLIB
+	select I2C
 	select MFD_CORE
 	select REGMAP_I2C
 	select REGMAP_IRQ
+	select COMMON_CLK
+	select I2C_DESIGNWARE_PLATFORM
 	help
 	  Select this option to enable support for Crystal Cove PMIC
 	  on some Intel SoC systems. The PMIC provides ADC, GPIO,
 	  thermal, charger and related power management functions
 	  on these systems.
 
+	  This option is a bool as it provides an ACPI Opregion which must be
+	  available before any devices using it are probed. This option also
+	  causes the designware-i2c driver to be builtin for the same reason.
+
 config INTEL_SOC_PMIC_BXTWC
 	tristate "Support for Intel Broxton Whiskey Cove PMIC"
 	depends on INTEL_PMC_IPC
-- 
2.12.2

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

* [PATCH v4 2/2] mfd: axp20x-i2c: Document that this must be builtin on x86
  2017-05-15 18:23 [PATCH v4 1/2] mfd: intel_soc_pmic: Select designware i2c-bus driver Hans de Goede
@ 2017-05-15 18:23 ` Hans de Goede
  2017-05-22 10:56   ` Lee Jones
  2017-05-22 10:57 ` [PATCH v4 1/2] mfd: intel_soc_pmic: Select designware i2c-bus driver Lee Jones
  1 sibling, 1 reply; 8+ messages in thread
From: Hans de Goede @ 2017-05-15 18:23 UTC (permalink / raw)
  To: Lee Jones; +Cc: Hans de Goede, linux-kernel

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, OpRegionr and i2c-bus drivers are built in.

Since the axp20x mfd code is used on non X86 too we cannot simply change
this into a bool, I've tried some Kconfig magic with if x86 but I could
not get this working correctly, so this commit just documents that this
should be builtin on x86, which fixes errors like these during boot:

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

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Fix Kconfig depends and selects to fix warning reported by kbuild test robot
-Improve commit msg (add example of ACPI errors this avoids)
Changes in v3:
-Since the axp20x mfd code is used on ARM too where it does not necessarily
 need to be builtin settle for simply documenting the need to have this
 builtin on x86
---
 drivers/mfd/Kconfig | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 61ebda81e134..cfac1e8a0a20 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -160,6 +160,11 @@ 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.
+
 config MFD_AXP20X_RSB
 	tristate "X-Powers AXP series PMICs with RSB"
 	select MFD_AXP20X
-- 
2.12.2

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

* Re: [PATCH v4 2/2] mfd: axp20x-i2c: Document that this must be builtin on x86
  2017-05-15 18:23 ` [PATCH v4 2/2] mfd: axp20x-i2c: Document that this must be builtin on x86 Hans de Goede
@ 2017-05-22 10:56   ` Lee Jones
  2017-05-26 11:14     ` Hans de Goede
  0 siblings, 1 reply; 8+ messages in thread
From: Lee Jones @ 2017-05-22 10:56 UTC (permalink / raw)
  To: Hans de Goede; +Cc: linux-kernel

On Mon, 15 May 2017, Hans de Goede wrote:

> 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, OpRegionr and i2c-bus drivers are built in.
> 
> Since the axp20x mfd code is used on non X86 too we cannot simply change
> this into a bool, I've tried some Kconfig magic with if x86 but I could
> not get this working correctly, so this commit just documents that this
> should be builtin on x86, which fixes errors like these during boot:
> 
> mmc0: SDHCI controller on ACPI [80860F14:00] using ADMA
> ACPI Error: No handler for Region [REGS] (ffff93543b0cc3a8) [UserDefinedRegion]
> ACPI Error: Region UserDefinedRegion (ID=143) has no handler (20170119/exfldio-2
> ACPI Error: Method parse/execution failed [\_SB.PCI0.I2C7.PMI5.GET] (Node ffff93
> ACPI Error: Method parse/execution failed [\_SB.PCI0.SHC1._PS0] (Node ffff93543b
> acpi 80860F14:02: Failed to change power state to D0
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Would be good to get an Ack by the users/maintainers of this driver.

> ---
> Changes in v2:
> -Fix Kconfig depends and selects to fix warning reported by kbuild test robot
> -Improve commit msg (add example of ACPI errors this avoids)
> Changes in v3:
> -Since the axp20x mfd code is used on ARM too where it does not necessarily
>  need to be builtin settle for simply documenting the need to have this
>  builtin on x86
> ---
>  drivers/mfd/Kconfig | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 61ebda81e134..cfac1e8a0a20 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -160,6 +160,11 @@ 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

Nit: s/i2c/I2C/

> +	  I2C_DESIGNWARE_PLATFORM and I2C_DESIGNWARE_BAYTRAIL must be 'y' too.
> +
>  config MFD_AXP20X_RSB
>  	tristate "X-Powers AXP series PMICs with RSB"
>  	select MFD_AXP20X

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

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

* Re: [PATCH v4 1/2] mfd: intel_soc_pmic: Select designware i2c-bus driver
  2017-05-15 18:23 [PATCH v4 1/2] mfd: intel_soc_pmic: Select designware i2c-bus driver Hans de Goede
  2017-05-15 18:23 ` [PATCH v4 2/2] mfd: axp20x-i2c: Document that this must be builtin on x86 Hans de Goede
@ 2017-05-22 10:57 ` Lee Jones
  2017-05-22 19:02   ` Andy Shevchenko
  2017-05-26 11:11   ` Hans de Goede
  1 sibling, 2 replies; 8+ messages in thread
From: Lee Jones @ 2017-05-22 10:57 UTC (permalink / raw)
  To: Hans de Goede; +Cc: linux-kernel, Andy Shevchenko

On Mon, 15 May 2017, 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.
> 
> 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
> 
> 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>

Would be good to get an Ack by the users/maintainers of this driver.

> ---
> Note this patch will partially conflicts with (contains the same changes as)
> a patch in Andy Shevchenko's tree.

Has this been sorted now?

> ---
> Changes in v2:
> -Fix Kconfig depends and selects to fix warning reported by kbuild test robot
> -Improve commit msg (add example of ACPI errors this avoids)
> Changes in v3:
> -No changes
> Changes in v4:
> -Add explanation why this is a bool and why it selects i2c-designwaree
>  to the help text rather then as comments in the Kconfig
> ---
>  drivers/mfd/Kconfig | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 1b6a83a4a114..61ebda81e134 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -448,17 +448,24 @@ config LPC_SCH
>  
>  config INTEL_SOC_PMIC
>  	bool "Support for Crystal Cove PMIC"
> -	depends on GPIOLIB
> -	depends on I2C=y
> +	depends on HAS_IOMEM
> +	select GPIOLIB
> +	select I2C
>  	select MFD_CORE
>  	select REGMAP_I2C
>  	select REGMAP_IRQ
> +	select COMMON_CLK
> +	select I2C_DESIGNWARE_PLATFORM
>  	help
>  	  Select this option to enable support for Crystal Cove PMIC
>  	  on some Intel SoC systems. The PMIC provides ADC, GPIO,
>  	  thermal, charger and related power management functions
>  	  on these systems.
>  
> +	  This option is a bool as it provides an ACPI Opregion which must be

Nit: s/Opregion/OpRegion/

> +	  available before any devices using it are probed. This option also
> +	  causes the designware-i2c driver to be builtin for the same reason.
> +
>  config INTEL_SOC_PMIC_BXTWC
>  	tristate "Support for Intel Broxton Whiskey Cove PMIC"
>  	depends on INTEL_PMC_IPC

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

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

* Re: [PATCH v4 1/2] mfd: intel_soc_pmic: Select designware i2c-bus driver
  2017-05-22 10:57 ` [PATCH v4 1/2] mfd: intel_soc_pmic: Select designware i2c-bus driver Lee Jones
@ 2017-05-22 19:02   ` Andy Shevchenko
  2017-05-26 11:11   ` Hans de Goede
  1 sibling, 0 replies; 8+ messages in thread
From: Andy Shevchenko @ 2017-05-22 19:02 UTC (permalink / raw)
  To: Lee Jones, Hans de Goede; +Cc: linux-kernel

On Mon, 2017-05-22 at 11:57 +0100, Lee Jones wrote:
> On Mon, 15 May 2017, 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.
> > 
> > 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
> > 
> > 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>
> 
> Would be good to get an Ack by the users/maintainers of this driver.

I'm not sure you will get (active) one.

>From my prospective, FWIW:
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>


> 
> > ---
> > Note this patch will partially conflicts with (contains the same
> > changes as)
> > a patch in Andy Shevchenko's tree.
> 
> Has this been sorted now?

>From code it looks like (renaming is already done ---vvv).
 
> >  config INTEL_SOC_PMIC
> >  	bool "Support for Crystal Cove PMIC"


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

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

* Re: [PATCH v4 1/2] mfd: intel_soc_pmic: Select designware i2c-bus driver
  2017-05-22 10:57 ` [PATCH v4 1/2] mfd: intel_soc_pmic: Select designware i2c-bus driver Lee Jones
  2017-05-22 19:02   ` Andy Shevchenko
@ 2017-05-26 11:11   ` Hans de Goede
  1 sibling, 0 replies; 8+ messages in thread
From: Hans de Goede @ 2017-05-26 11:11 UTC (permalink / raw)
  To: Lee Jones; +Cc: linux-kernel, Andy Shevchenko

Hi,

On 22-05-17 12:57, Lee Jones wrote:
> On Mon, 15 May 2017, 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.
>>
>> 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
>>
>> 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>
> 
> Would be good to get an Ack by the users/maintainers of this driver.
> 
>> ---
>> Note this patch will partially conflicts with (contains the same changes as)
>> a patch in Andy Shevchenko's tree.
> 
> Has this been sorted now?

Yes, sorry I should have dropped this bit.

> 
>> ---
>> Changes in v2:
>> -Fix Kconfig depends and selects to fix warning reported by kbuild test robot
>> -Improve commit msg (add example of ACPI errors this avoids)
>> Changes in v3:
>> -No changes
>> Changes in v4:
>> -Add explanation why this is a bool and why it selects i2c-designwaree
>>   to the help text rather then as comments in the Kconfig
>> ---
>>   drivers/mfd/Kconfig | 11 +++++++++--
>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index 1b6a83a4a114..61ebda81e134 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -448,17 +448,24 @@ config LPC_SCH
>>   
>>   config INTEL_SOC_PMIC
>>   	bool "Support for Crystal Cove PMIC"
>> -	depends on GPIOLIB
>> -	depends on I2C=y
>> +	depends on HAS_IOMEM
>> +	select GPIOLIB
>> +	select I2C
>>   	select MFD_CORE
>>   	select REGMAP_I2C
>>   	select REGMAP_IRQ
>> +	select COMMON_CLK
>> +	select I2C_DESIGNWARE_PLATFORM
>>   	help
>>   	  Select this option to enable support for Crystal Cove PMIC
>>   	  on some Intel SoC systems. The PMIC provides ADC, GPIO,
>>   	  thermal, charger and related power management functions
>>   	  on these systems.
>>   
>> +	  This option is a bool as it provides an ACPI Opregion which must be
> 
> Nit: s/Opregion/OpRegion/

Fixed for v5 (which I will send in a minute).

Regards,

Hans

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

* Re: [PATCH v4 2/2] mfd: axp20x-i2c: Document that this must be builtin on x86
  2017-05-22 10:56   ` Lee Jones
@ 2017-05-26 11:14     ` Hans de Goede
  2017-05-30  7:39       ` Lee Jones
  0 siblings, 1 reply; 8+ messages in thread
From: Hans de Goede @ 2017-05-26 11:14 UTC (permalink / raw)
  To: Lee Jones; +Cc: linux-kernel, Chen-Yu Tsai

Hi,

On 22-05-17 12:56, Lee Jones wrote:
> On Mon, 15 May 2017, Hans de Goede wrote:
> 
>> 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, OpRegionr and i2c-bus drivers are built in.
>>
>> Since the axp20x mfd code is used on non X86 too we cannot simply change
>> this into a bool, I've tried some Kconfig magic with if x86 but I could
>> not get this working correctly, so this commit just documents that this
>> should be builtin on x86, which fixes errors like these during boot:
>>
>> mmc0: SDHCI controller on ACPI [80860F14:00] using ADMA
>> ACPI Error: No handler for Region [REGS] (ffff93543b0cc3a8) [UserDefinedRegion]
>> ACPI Error: Region UserDefinedRegion (ID=143) has no handler (20170119/exfldio-2
>> ACPI Error: Method parse/execution failed [\_SB.PCI0.I2C7.PMI5.GET] (Node ffff93
>> ACPI Error: Method parse/execution failed [\_SB.PCI0.SHC1._PS0] (Node ffff93543b
>> acpi 80860F14:02: Failed to change power state to D0
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> Would be good to get an Ack by the users/maintainers of this driver.

If you look at recent commits to drivers for the mfd-subdevices for the AXP288
(which is the only AXP PMIC found on x86 systems) you will see that I've
been the only one touch them / caring about them and in many cases fixing
them from being non-functional to actually working.

I do have quite a few people who are using the AXP288 code on Bay and Cherry
Trail devices with this PMIC and who have send me thank you's and test
success reports.

So I guess I'm the closest thing to a MAINTAINER for the axp288 bits of
this driver.

>> ---
>> Changes in v2:
>> -Fix Kconfig depends and selects to fix warning reported by kbuild test robot
>> -Improve commit msg (add example of ACPI errors this avoids)
>> Changes in v3:
>> -Since the axp20x mfd code is used on ARM too where it does not necessarily
>>   need to be builtin settle for simply documenting the need to have this
>>   builtin on x86
>> ---
>>   drivers/mfd/Kconfig | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index 61ebda81e134..cfac1e8a0a20 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -160,6 +160,11 @@ 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
> 
> Nit: s/i2c/I2C/

Fixed for v5 (which I will send in a minute).

Regards,

Hans

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

* Re: [PATCH v4 2/2] mfd: axp20x-i2c: Document that this must be builtin on x86
  2017-05-26 11:14     ` Hans de Goede
@ 2017-05-30  7:39       ` Lee Jones
  0 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2017-05-30  7:39 UTC (permalink / raw)
  To: Hans de Goede; +Cc: linux-kernel, Chen-Yu Tsai

On Fri, 26 May 2017, Hans de Goede wrote:

> Hi,
> 
> On 22-05-17 12:56, Lee Jones wrote:
> > On Mon, 15 May 2017, Hans de Goede wrote:
> > 
> > > 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, OpRegionr and i2c-bus drivers are built in.
> > > 
> > > Since the axp20x mfd code is used on non X86 too we cannot simply change
> > > this into a bool, I've tried some Kconfig magic with if x86 but I could
> > > not get this working correctly, so this commit just documents that this
> > > should be builtin on x86, which fixes errors like these during boot:
> > > 
> > > mmc0: SDHCI controller on ACPI [80860F14:00] using ADMA
> > > ACPI Error: No handler for Region [REGS] (ffff93543b0cc3a8) [UserDefinedRegion]
> > > ACPI Error: Region UserDefinedRegion (ID=143) has no handler (20170119/exfldio-2
> > > ACPI Error: Method parse/execution failed [\_SB.PCI0.I2C7.PMI5.GET] (Node ffff93
> > > ACPI Error: Method parse/execution failed [\_SB.PCI0.SHC1._PS0] (Node ffff93543b
> > > acpi 80860F14:02: Failed to change power state to D0
> > > 
> > > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > 
> > Would be good to get an Ack by the users/maintainers of this driver.
> 
> If you look at recent commits to drivers for the mfd-subdevices for the AXP288
> (which is the only AXP PMIC found on x86 systems) you will see that I've
> been the only one touch them / caring about them and in many cases fixing
> them from being non-functional to actually working.
> 
> I do have quite a few people who are using the AXP288 code on Bay and Cherry
> Trail devices with this PMIC and who have send me thank you's and test
> success reports.

Would any of them be prepared to submit a Tested-by?

You can forward them (with permission) if you like.

> So I guess I'm the closest thing to a MAINTAINER for the axp288 bits of
> this driver.
> 
> > > ---
> > > Changes in v2:
> > > -Fix Kconfig depends and selects to fix warning reported by kbuild test robot
> > > -Improve commit msg (add example of ACPI errors this avoids)
> > > Changes in v3:
> > > -Since the axp20x mfd code is used on ARM too where it does not necessarily
> > >   need to be builtin settle for simply documenting the need to have this
> > >   builtin on x86
> > > ---
> > >   drivers/mfd/Kconfig | 5 +++++
> > >   1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > > index 61ebda81e134..cfac1e8a0a20 100644
> > > --- a/drivers/mfd/Kconfig
> > > +++ b/drivers/mfd/Kconfig
> > > @@ -160,6 +160,11 @@ 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
> > 
> > Nit: s/i2c/I2C/
> 
> Fixed for v5 (which I will send in a minute).
> 
> Regards,
> 
> Hans

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

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

end of thread, other threads:[~2017-05-30  7:40 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-15 18:23 [PATCH v4 1/2] mfd: intel_soc_pmic: Select designware i2c-bus driver Hans de Goede
2017-05-15 18:23 ` [PATCH v4 2/2] mfd: axp20x-i2c: Document that this must be builtin on x86 Hans de Goede
2017-05-22 10:56   ` Lee Jones
2017-05-26 11:14     ` Hans de Goede
2017-05-30  7:39       ` Lee Jones
2017-05-22 10:57 ` [PATCH v4 1/2] mfd: intel_soc_pmic: Select designware i2c-bus driver Lee Jones
2017-05-22 19:02   ` Andy Shevchenko
2017-05-26 11:11   ` Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.