[PATCH] mfd: Kconfig: change INTEL_SOC_PMIC_CHTDC_TI to bool

[PATCH] mfd: Kconfig: change INTEL_SOC_PMIC_CHTDC_TI to bool

[Mauro Carvalho Chehab]

The INTEL_SOC_PMIC_CHTDC_TI should be initialized early, before
loading the fbcon driver, as otherwise the i915 driver will
fail to configure pwm:

[   13.674287] fb0: switching to inteldrmfb from EFI VGA
[   13.682380] Console: switching to colour dummy device 80x25
[   13.682468] i915 0000:00:02.0: vgaarb: deactivate vga console
[   13.682686] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[   13.685773] i915 0000:00:02.0: vgaarb: changed VGA decodes: olddecodes=io+mem,decodes=io+mem:owns=io+mem
[   13.686219] i915 0000:00:02.0: [drm] *ERROR* Failed to configure the pwm chip
[   13.699572] [drm] Initialized i915 1.6.0 20200313 for 0000:00:02.0 on minor 0
[   13.739044] fbcon: i915drmfb (fb0) is primary device
[   14.037792] intel_soc_pmic_exec_mipi_pmic_seq_element: No PMIC registered
...
[   24.621403] intel_pmic_install_opregion_handler: Ask to register OpRegion for bus ID=PMI2, HID=INT33F5
[   24.630540] intel_pmic_install_opregion_handler: OpRegion registered

(some extra debug printk's were added to the above)

As suggested by Hans, this patch also addresses an issue with
the dependencies, as, for this driver to be a bool, it also
need the I2C core and the I2C_DESIGNWARE driver to be builtin.

Suggested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/mfd/Kconfig | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index ca0edab91aeb..f9092c79c4e8 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -632,7 +632,7 @@ config INTEL_SOC_PMIC_CHTWC
 config INTEL_SOC_PMIC_CHTDC_TI
 	tristate "Support for Intel Cherry Trail Dollar Cove TI PMIC"
 	depends on GPIOLIB
-	depends on I2C
+	depends on I2C = y && I2C_DESIGNWARE_PLATFORM=y
 	depends on ACPI
 	depends on X86
 	select MFD_CORE
@@ -642,6 +642,10 @@ config INTEL_SOC_PMIC_CHTDC_TI
 	  Select this option for supporting Dollar Cove (TI version) PMIC
 	  device that is found on some Intel Cherry Trail 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
+	  needs the designware-i2c driver to be builtin for the same reason.
+
 config INTEL_SOC_PMIC_MRFLD
 	tristate "Support for Intel Merrifield Basin Cove PMIC"
 	depends on GPIOLIB
-- 
2.31.1

Re: [PATCH] mfd: Kconfig: change INTEL_SOC_PMIC_CHTDC_TI to bool

[Hans de Goede]

Hi,

On 10/29/21 09:02, Mauro Carvalho Chehab wrote:
> The INTEL_SOC_PMIC_CHTDC_TI should be initialized early, before
> loading the fbcon driver, as otherwise the i915 driver will
> fail to configure pwm:
> 
> [   13.674287] fb0: switching to inteldrmfb from EFI VGA
> [   13.682380] Console: switching to colour dummy device 80x25
> [   13.682468] i915 0000:00:02.0: vgaarb: deactivate vga console
> [   13.682686] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [   13.685773] i915 0000:00:02.0: vgaarb: changed VGA decodes: olddecodes=io+mem,decodes=io+mem:owns=io+mem
> [   13.686219] i915 0000:00:02.0: [drm] *ERROR* Failed to configure the pwm chip
> [   13.699572] [drm] Initialized i915 1.6.0 20200313 for 0000:00:02.0 on minor 0
> [   13.739044] fbcon: i915drmfb (fb0) is primary device
> [   14.037792] intel_soc_pmic_exec_mipi_pmic_seq_element: No PMIC registered
> ...
> [   24.621403] intel_pmic_install_opregion_handler: Ask to register OpRegion for bus ID=PMI2, HID=INT33F5
> [   24.630540] intel_pmic_install_opregion_handler: OpRegion registered
> 
> (some extra debug printk's were added to the above)
> 
> As suggested by Hans, this patch also addresses an issue with
> the dependencies, as, for this driver to be a bool, it also
> need the I2C core and the I2C_DESIGNWARE driver to be builtin.
> 
> Suggested-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

> ---
>  drivers/mfd/Kconfig | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index ca0edab91aeb..f9092c79c4e8 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -632,7 +632,7 @@ config INTEL_SOC_PMIC_CHTWC
>  config INTEL_SOC_PMIC_CHTDC_TI
>  	tristate "Support for Intel Cherry Trail Dollar Cove TI PMIC"
>  	depends on GPIOLIB
> -	depends on I2C
> +	depends on I2C = y && I2C_DESIGNWARE_PLATFORM=y
>  	depends on ACPI
>  	depends on X86
>  	select MFD_CORE
> @@ -642,6 +642,10 @@ config INTEL_SOC_PMIC_CHTDC_TI
>  	  Select this option for supporting Dollar Cove (TI version) PMIC
>  	  device that is found on some Intel Cherry Trail 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
> +	  needs the designware-i2c driver to be builtin for the same reason.
> +
>  config INTEL_SOC_PMIC_MRFLD
>  	tristate "Support for Intel Merrifield Basin Cove PMIC"
>  	depends on GPIOLIB
> 

Re: [PATCH] mfd: Kconfig: change INTEL_SOC_PMIC_CHTDC_TI to bool

[Lee Jones]

On Fri, 29 Oct 2021, Mauro Carvalho Chehab wrote:

> The INTEL_SOC_PMIC_CHTDC_TI should be initialized early, before
> loading the fbcon driver, as otherwise the i915 driver will
> fail to configure pwm:
> 
> [   13.674287] fb0: switching to inteldrmfb from EFI VGA
> [   13.682380] Console: switching to colour dummy device 80x25
> [   13.682468] i915 0000:00:02.0: vgaarb: deactivate vga console
> [   13.682686] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [   13.685773] i915 0000:00:02.0: vgaarb: changed VGA decodes: olddecodes=io+mem,decodes=io+mem:owns=io+mem
> [   13.686219] i915 0000:00:02.0: [drm] *ERROR* Failed to configure the pwm chip
> [   13.699572] [drm] Initialized i915 1.6.0 20200313 for 0000:00:02.0 on minor 0
> [   13.739044] fbcon: i915drmfb (fb0) is primary device
> [   14.037792] intel_soc_pmic_exec_mipi_pmic_seq_element: No PMIC registered
> ...
> [   24.621403] intel_pmic_install_opregion_handler: Ask to register OpRegion for bus ID=PMI2, HID=INT33F5
> [   24.630540] intel_pmic_install_opregion_handler: OpRegion registered
> 
> (some extra debug printk's were added to the above)
> 
> As suggested by Hans, this patch also addresses an issue with
> the dependencies, as, for this driver to be a bool, it also
> need the I2C core and the I2C_DESIGNWARE driver to be builtin.
> 
> Suggested-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  drivers/mfd/Kconfig | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index ca0edab91aeb..f9092c79c4e8 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -632,7 +632,7 @@ config INTEL_SOC_PMIC_CHTWC
>  config INTEL_SOC_PMIC_CHTDC_TI
>  	tristate "Support for Intel Cherry Trail Dollar Cove TI PMIC"
>  	depends on GPIOLIB
> -	depends on I2C
> +	depends on I2C = y && I2C_DESIGNWARE_PLATFORM=y

The lack of formatting consistency here is eating me up inside!

>  	depends on ACPI
>  	depends on X86
>  	select MFD_CORE
> @@ -642,6 +642,10 @@ config INTEL_SOC_PMIC_CHTDC_TI
>  	  Select this option for supporting Dollar Cove (TI version) PMIC
>  	  device that is found on some Intel Cherry Trail 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
> +	  needs the designware-i2c driver to be builtin for the same reason.

Is there no way around this?

We do have early module loading available.

>  config INTEL_SOC_PMIC_MRFLD
>  	tristate "Support for Intel Merrifield Basin Cove PMIC"
>  	depends on GPIOLIB

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] mfd: Kconfig: change INTEL_SOC_PMIC_CHTDC_TI to bool

[Hans de Goede]

Hi,

On 11/24/21 16:40, Lee Jones wrote:
> On Fri, 29 Oct 2021, Mauro Carvalho Chehab wrote:
> 
>> The INTEL_SOC_PMIC_CHTDC_TI should be initialized early, before
>> loading the fbcon driver, as otherwise the i915 driver will
>> fail to configure pwm:
>>
>> [   13.674287] fb0: switching to inteldrmfb from EFI VGA
>> [   13.682380] Console: switching to colour dummy device 80x25
>> [   13.682468] i915 0000:00:02.0: vgaarb: deactivate vga console
>> [   13.682686] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
>> [   13.685773] i915 0000:00:02.0: vgaarb: changed VGA decodes: olddecodes=io+mem,decodes=io+mem:owns=io+mem
>> [   13.686219] i915 0000:00:02.0: [drm] *ERROR* Failed to configure the pwm chip
>> [   13.699572] [drm] Initialized i915 1.6.0 20200313 for 0000:00:02.0 on minor 0
>> [   13.739044] fbcon: i915drmfb (fb0) is primary device
>> [   14.037792] intel_soc_pmic_exec_mipi_pmic_seq_element: No PMIC registered
>> ...
>> [   24.621403] intel_pmic_install_opregion_handler: Ask to register OpRegion for bus ID=PMI2, HID=INT33F5
>> [   24.630540] intel_pmic_install_opregion_handler: OpRegion registered
>>
>> (some extra debug printk's were added to the above)
>>
>> As suggested by Hans, this patch also addresses an issue with
>> the dependencies, as, for this driver to be a bool, it also
>> need the I2C core and the I2C_DESIGNWARE driver to be builtin.
>>
>> Suggested-by: Hans de Goede <hdegoede@redhat.com>
>> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
>> ---
>>  drivers/mfd/Kconfig | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index ca0edab91aeb..f9092c79c4e8 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -632,7 +632,7 @@ config INTEL_SOC_PMIC_CHTWC
>>  config INTEL_SOC_PMIC_CHTDC_TI
>>  	tristate "Support for Intel Cherry Trail Dollar Cove TI PMIC"
>>  	depends on GPIOLIB
>> -	depends on I2C
>> +	depends on I2C = y && I2C_DESIGNWARE_PLATFORM=y
> 
> The lack of formatting consistency here is eating me up inside!
> 
>>  	depends on ACPI
>>  	depends on X86
>>  	select MFD_CORE
>> @@ -642,6 +642,10 @@ config INTEL_SOC_PMIC_CHTDC_TI
>>  	  Select this option for supporting Dollar Cove (TI version) PMIC
>>  	  device that is found on some Intel Cherry Trail 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
>> +	  needs the designware-i2c driver to be builtin for the same reason.
> 
> Is there no way around this?

No unfortunately not, the ACPI device-scan is done really early,
and in ACPI everything is a function, including e.g. _HID,
so I've seen _HID method-s reading e.g. GPIOs. So while the
initial ACPI scan is figuring out what sort of devices it
is dealing with, we already need working GPIOs (for example).

Various early ACPI code (AML / the DSDT) tends to access the
PMICs OpRegion and various problems happen when it is not
available. For the same reason the other BYT/CHT related
INTEL_SOC_PMIC_FOO options are already bools.

I guess that the DSDTs for other Intel SoCs then the BYT/CHT
SoCs might be a bit cleaner. BYT/CHT was Intel's first serious
attempt at standard x86 tablet SoC and this shows in various
places.

Regards,

Hans

Re: [PATCH] mfd: Kconfig: change INTEL_SOC_PMIC_CHTDC_TI to bool

[Lee Jones]

On Wed, 24 Nov 2021, Hans de Goede wrote:

> Hi,
> 
> On 11/24/21 16:40, Lee Jones wrote:
> > On Fri, 29 Oct 2021, Mauro Carvalho Chehab wrote:
> > 
> >> The INTEL_SOC_PMIC_CHTDC_TI should be initialized early, before
> >> loading the fbcon driver, as otherwise the i915 driver will
> >> fail to configure pwm:
> >>
> >> [   13.674287] fb0: switching to inteldrmfb from EFI VGA
> >> [   13.682380] Console: switching to colour dummy device 80x25
> >> [   13.682468] i915 0000:00:02.0: vgaarb: deactivate vga console
> >> [   13.682686] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> >> [   13.685773] i915 0000:00:02.0: vgaarb: changed VGA decodes: olddecodes=io+mem,decodes=io+mem:owns=io+mem
> >> [   13.686219] i915 0000:00:02.0: [drm] *ERROR* Failed to configure the pwm chip
> >> [   13.699572] [drm] Initialized i915 1.6.0 20200313 for 0000:00:02.0 on minor 0
> >> [   13.739044] fbcon: i915drmfb (fb0) is primary device
> >> [   14.037792] intel_soc_pmic_exec_mipi_pmic_seq_element: No PMIC registered
> >> ...
> >> [   24.621403] intel_pmic_install_opregion_handler: Ask to register OpRegion for bus ID=PMI2, HID=INT33F5
> >> [   24.630540] intel_pmic_install_opregion_handler: OpRegion registered
> >>
> >> (some extra debug printk's were added to the above)
> >>
> >> As suggested by Hans, this patch also addresses an issue with
> >> the dependencies, as, for this driver to be a bool, it also
> >> need the I2C core and the I2C_DESIGNWARE driver to be builtin.
> >>
> >> Suggested-by: Hans de Goede <hdegoede@redhat.com>
> >> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> >> ---
> >>  drivers/mfd/Kconfig | 6 +++++-
> >>  1 file changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> >> index ca0edab91aeb..f9092c79c4e8 100644
> >> --- a/drivers/mfd/Kconfig
> >> +++ b/drivers/mfd/Kconfig
> >> @@ -632,7 +632,7 @@ config INTEL_SOC_PMIC_CHTWC
> >>  config INTEL_SOC_PMIC_CHTDC_TI
> >>  	tristate "Support for Intel Cherry Trail Dollar Cove TI PMIC"
> >>  	depends on GPIOLIB
> >> -	depends on I2C
> >> +	depends on I2C = y && I2C_DESIGNWARE_PLATFORM=y
> > 
> > The lack of formatting consistency here is eating me up inside!
> > 
> >>  	depends on ACPI
> >>  	depends on X86
> >>  	select MFD_CORE
> >> @@ -642,6 +642,10 @@ config INTEL_SOC_PMIC_CHTDC_TI
> >>  	  Select this option for supporting Dollar Cove (TI version) PMIC
> >>  	  device that is found on some Intel Cherry Trail 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
> >> +	  needs the designware-i2c driver to be builtin for the same reason.
> > 
> > Is there no way around this?
> 
> No unfortunately not, the ACPI device-scan is done really early,
> and in ACPI everything is a function, including e.g. _HID,
> so I've seen _HID method-s reading e.g. GPIOs. So while the
> initial ACPI scan is figuring out what sort of devices it
> is dealing with, we already need working GPIOs (for example).
> 
> Various early ACPI code (AML / the DSDT) tends to access the
> PMICs OpRegion and various problems happen when it is not
> available. For the same reason the other BYT/CHT related
> INTEL_SOC_PMIC_FOO options are already bools.
> 
> I guess that the DSDTs for other Intel SoCs then the BYT/CHT
> SoCs might be a bit cleaner. BYT/CHT was Intel's first serious
> attempt at standard x86 tablet SoC and this shows in various
> places.

Thanks for the explanation.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] mfd: Kconfig: change INTEL_SOC_PMIC_CHTDC_TI to bool

[Mauro Carvalho Chehab]

Em Wed, 24 Nov 2021 15:40:00 +0000
Lee Jones <lee.jones@linaro.org> escreveu:

> > @@ -632,7 +632,7 @@ config INTEL_SOC_PMIC_CHTWC
> >  config INTEL_SOC_PMIC_CHTDC_TI
> >  	tristate "Support for Intel Cherry Trail Dollar Cove TI PMIC"
> >  	depends on GPIOLIB
> > -	depends on I2C
> > +	depends on I2C = y && I2C_DESIGNWARE_PLATFORM=y  
> 
> The lack of formatting consistency here is eating me up inside!

Just sent a v2 removing the extra spaces.

Thanks,
Mauro