[RFC PATCH] arm64: defconfig: Set MFD_TPS6594_I2C as built-in

[RFC PATCH] arm64: defconfig: Set MFD_TPS6594_I2C as built-in

[Judith Mendez]

SK-AM62A-LP is a device targeting automotive front-camera applications
among other use-cases. It utilizes the TPS6593x PMIC (interfaced over I2C)
to power the SoC and various other peripherals on the board [1].

MMCSD requires the PMIC to be setup correctly before setting the bus
pins to 1.8V using the TPS6594 driver interfaced over i2c.

Currently, the following could be seen when booting the am62ax platform:

"platform fa00000.mmc: deferred probe pending: platform: supplier regulator-5 not ready"
"vdd_mmc1: disabling"

and a failure to boot the SK-AM62A-LP.

One solution is to use initramfs [2], but using initramfs increases the
boot time for this automotive solution which requires faster boot time
parameters.

Another solution is to change MFD_TPS6594_I2C to built-in, that way the
PMIC is setup and the regulators are ready before MMCSD switches to UHS
mode, this is the preferred solution since it does not increase boot time
like the initramfs solution does.

[1] https://www.ti.com/lit/zip/sprr459
[2] https://lore.kernel.org/linux-devicetree/5f03207b-c29b-4d16-92b0-d14eef77bf17@linaro.org/
Fixes: f9010eb938be ("arm64: defconfig: Enable TPS6593 PMIC for SK-AM62A")

Signed-off-by: Judith Mendez <jm@ti.com>
---
 arch/arm64/configs/defconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 7d32fca649965..61f767246d3a5 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -756,7 +756,7 @@ CONFIG_RZ_MTU3=y
 CONFIG_MFD_TI_AM335X_TSCADC=m
 CONFIG_MFD_TI_LP873X=m
 CONFIG_MFD_TPS65219=y
-CONFIG_MFD_TPS6594_I2C=m
+CONFIG_MFD_TPS6594_I2C=y
 CONFIG_MFD_ROHM_BD718XX=y
 CONFIG_MFD_WCD934X=m
 CONFIG_MFD_KHADAS_MCU=m

base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b
-- 
2.46.0

Re: [RFC PATCH] arm64: defconfig: Set MFD_TPS6594_I2C as built-in

[Krzysztof Kozlowski]

On 19/08/2024 22:43, Judith Mendez wrote:
> SK-AM62A-LP is a device targeting automotive front-camera applications
> among other use-cases. It utilizes the TPS6593x PMIC (interfaced over I2C)
> to power the SoC and various other peripherals on the board [1].
> 
> MMCSD requires the PMIC to be setup correctly before setting the bus
> pins to 1.8V using the TPS6594 driver interfaced over i2c.
> 
> Currently, the following could be seen when booting the am62ax platform:
> 
> "platform fa00000.mmc: deferred probe pending: platform: supplier regulator-5 not ready"
> "vdd_mmc1: disabling"
> 
> and a failure to boot the SK-AM62A-LP.
> 
> One solution is to use initramfs [2], but using initramfs increases the
> boot time for this automotive solution which requires faster boot time
> parameters.

This is a defconfig, not a distro/product config, so your product
constraints are not really relevant. You are supposed to boot defconfig
with proper initramfs with necessary modules.

I don't get why people mistake defconfig with their product stuff...

> 
> Another solution is to change MFD_TPS6594_I2C to built-in, that way the
> PMIC is setup and the regulators are ready before MMCSD switches to UHS
> mode, this is the preferred solution since it does not increase boot time
> like the initramfs solution does.

Use initramfs. Several devices, e.g. most Android ones, have fixed size
of boot partition, so size of kernel is important.

> 
> [1] https://www.ti.com/lit/zip/sprr459
> [2] https://lore.kernel.org/linux-devicetree/5f03207b-c29b-4d16-92b0-d14eef77bf17@linaro.org/
> Fixes: f9010eb938be ("arm64: defconfig: Enable TPS6593 PMIC for SK-AM62A")

No bug here to be fixed. You miss initramfs.

> 
> Signed-off-by: Judith Mendez <jm@ti.com>


Best regards,
Krzysztof

Re: [RFC PATCH] arm64: defconfig: Set MFD_TPS6594_I2C as built-in

[Nishanth Menon]

On 23:01-20240819, Krzysztof Kozlowski wrote:
> On 19/08/2024 22:43, Judith Mendez wrote:
> > SK-AM62A-LP is a device targeting automotive front-camera applications
> > among other use-cases. It utilizes the TPS6593x PMIC (interfaced over I2C)
> > to power the SoC and various other peripherals on the board [1].
> > 
> > MMCSD requires the PMIC to be setup correctly before setting the bus
> > pins to 1.8V using the TPS6594 driver interfaced over i2c.
> > 
> > Currently, the following could be seen when booting the am62ax platform:
> > 
> > "platform fa00000.mmc: deferred probe pending: platform: supplier regulator-5 not ready"
> > "vdd_mmc1: disabling"
> > 
> > and a failure to boot the SK-AM62A-LP.
> > 
> > One solution is to use initramfs [2], but using initramfs increases the
> > boot time for this automotive solution which requires faster boot time
> > parameters.
> 
> This is a defconfig, not a distro/product config, so your product
> constraints are not really relevant. You are supposed to boot defconfig
> with proper initramfs with necessary modules.
> 
> I don't get why people mistake defconfig with their product stuff...
> 
> > 
> > Another solution is to change MFD_TPS6594_I2C to built-in, that way the
> > PMIC is setup and the regulators are ready before MMCSD switches to UHS
> > mode, this is the preferred solution since it does not increase boot time
> > like the initramfs solution does.
> 
> Use initramfs. Several devices, e.g. most Android ones, have fixed size
> of boot partition, so size of kernel is important.

am62a products do not use android in general. Standard distros such
as debian etc usage are limited as well. These products tend to have
limited resources just sufficient for the normal operations.

While I understand that we do keep the product usage model separate
from what upstream defconfig looks like, we have been very careful
to only enable the basic configurations necessary for default system
startup. During the initial days of K3, we had considered going down
the initramfs route, but realized that this was not a practical
option for developers to sustain and iterate quickly for triage or
development. Till date, we have maintained nfs and sd card boot as
default to allow for automated testing of upstream kernel.

I understand that you have provided similar comments for other
platforms[1] as well, but, I am now wondering if this is a new rule
we are taking in aarch64 platforms to allow just initramfs and
force all drivers to be modules (I understand that is the default
preference in android, but that is not true in other ecosystems). I am
curious if this was some sort of conclusion in the list (my search of
public-inbox seems to fail me here).

[1] https://lore.kernel.org/linux-arm-kernel/e08e6325-4b2b-c1ce-b33a-877de2c0babe@linaro.org/
-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [RFC PATCH] arm64: defconfig: Set MFD_TPS6594_I2C as built-in

[Krzysztof Kozlowski]

On 20/08/2024 13:53, Nishanth Menon wrote:
> On 23:01-20240819, Krzysztof Kozlowski wrote:
>> On 19/08/2024 22:43, Judith Mendez wrote:
>>> SK-AM62A-LP is a device targeting automotive front-camera applications
>>> among other use-cases. It utilizes the TPS6593x PMIC (interfaced over I2C)
>>> to power the SoC and various other peripherals on the board [1].
>>>
>>> MMCSD requires the PMIC to be setup correctly before setting the bus
>>> pins to 1.8V using the TPS6594 driver interfaced over i2c.
>>>
>>> Currently, the following could be seen when booting the am62ax platform:
>>>
>>> "platform fa00000.mmc: deferred probe pending: platform: supplier regulator-5 not ready"
>>> "vdd_mmc1: disabling"
>>>
>>> and a failure to boot the SK-AM62A-LP.
>>>
>>> One solution is to use initramfs [2], but using initramfs increases the
>>> boot time for this automotive solution which requires faster boot time
>>> parameters.
>>
>> This is a defconfig, not a distro/product config, so your product
>> constraints are not really relevant. You are supposed to boot defconfig
>> with proper initramfs with necessary modules.
>>
>> I don't get why people mistake defconfig with their product stuff...
>>
>>>
>>> Another solution is to change MFD_TPS6594_I2C to built-in, that way the
>>> PMIC is setup and the regulators are ready before MMCSD switches to UHS
>>> mode, this is the preferred solution since it does not increase boot time
>>> like the initramfs solution does.
>>
>> Use initramfs. Several devices, e.g. most Android ones, have fixed size
>> of boot partition, so size of kernel is important.
> 
> am62a products do not use android in general. Standard distros such

But I (and others) use sometimes devices with Android partitioning so
the size of vmlinux is important. I want to be able to enable KASAN. If
each person brings their modules into built-in, I won't be.

> as debian etc usage are limited as well. These products tend to have
> limited resources just sufficient for the normal operations.

So defconfig is not suitable for them in the first place.

> 
> While I understand that we do keep the product usage model separate
> from what upstream defconfig looks like, we have been very careful
> to only enable the basic configurations necessary for default system
> startup. During the initial days of K3, we had considered going down
> the initramfs route, but realized that this was not a practical
> option for developers to sustain and iterate quickly for triage or
> development. Till date, we have maintained nfs and sd card boot as
> default to allow for automated testing of upstream kernel.

I don't understand what is here not practical. Entire Qualcomm Landing
team for all products uses initramfs, my Exynos development uses
initramfs. There is no difference in building the kernel, no practical
impact, the same effort (after setting up the scripts, but we all are
scripting). I know, that 10 years ago many of developers, including
myself, did not want to switch to initramfs, but things change.

> 
> I understand that you have provided similar comments for other
> platforms[1] as well, but, I am now wondering if this is a new rule
> we are taking in aarch64 platforms to allow just initramfs and
> force all drivers to be modules (I understand that is the default

It's not particularly new. The use of modules for multi_v7 and
arm64/defconfig was since years.

> preference in android, but that is not true in other ecosystems). I am
> curious if this was some sort of conclusion in the list (my search of
> public-inbox seems to fail me here).

There are different point of views. I am presenting here mine and I back
it with arguments, that such changes accumulate and have impact on
developers workflow.

Defconfig is for developers and as starting point for distros or
products. Not as final product defconfig.

>  
> [1] https://lore.kernel.org/linux-arm-kernel/e08e6325-4b2b-c1ce-b33a-877de2c0babe@linaro.org/

Best regards,
Krzysztof

Re: [RFC PATCH] arm64: defconfig: Set MFD_TPS6594_I2C as built-in

[Catalin Marinas]

On Wed, Aug 21, 2024 at 08:33:49AM +0200, Krzysztof Kozlowski wrote:
> On 20/08/2024 13:53, Nishanth Menon wrote:
> > On 23:01-20240819, Krzysztof Kozlowski wrote:
> >> On 19/08/2024 22:43, Judith Mendez wrote:
> >>> SK-AM62A-LP is a device targeting automotive front-camera applications
> >>> among other use-cases. It utilizes the TPS6593x PMIC (interfaced over I2C)
> >>> to power the SoC and various other peripherals on the board [1].
> >>>
> >>> MMCSD requires the PMIC to be setup correctly before setting the bus
> >>> pins to 1.8V using the TPS6594 driver interfaced over i2c.
> >>>
> >>> Currently, the following could be seen when booting the am62ax platform:
> >>>
> >>> "platform fa00000.mmc: deferred probe pending: platform: supplier regulator-5 not ready"
> >>> "vdd_mmc1: disabling"
> >>>
> >>> and a failure to boot the SK-AM62A-LP.
> >>>
> >>> One solution is to use initramfs [2], but using initramfs increases the
> >>> boot time for this automotive solution which requires faster boot time
> >>> parameters.
> >>
> >> This is a defconfig, not a distro/product config, so your product
> >> constraints are not really relevant. You are supposed to boot defconfig
> >> with proper initramfs with necessary modules.
[...]
> > I understand that you have provided similar comments for other
> > platforms[1] as well, but, I am now wondering if this is a new rule
> > we are taking in aarch64 platforms to allow just initramfs and
> > force all drivers to be modules (I understand that is the default
[...]
> There are different point of views. I am presenting here mine and I back
> it with arguments, that such changes accumulate and have impact on
> developers workflow.
> 
> Defconfig is for developers and as starting point for distros or
> products. Not as final product defconfig.

Drive-by comment here (defconfig is mostly an arm-soc thing): in general
I agree with this position. Just because a product cannot (efficiently)
use initramfs, we shouldn't pollute the default built-ins with random
drivers. If you want something optimal for your product, just tweak the
defconfig or take it up with the distros. I'm fine with built-ins,
however, if they are required before reaching the initramfs stage (e.g.
interrupt controllers) but that's not the case here.

-- 
Catalin

Re: [RFC PATCH] arm64: defconfig: Set MFD_TPS6594_I2C as built-in

[Nishanth Menon]

Krzysztof,

First: I appreciate all you do for the community and this discussion.
Thank you.

On 08:33-20240821, Krzysztof Kozlowski wrote:
> On 20/08/2024 13:53, Nishanth Menon wrote:
> > On 23:01-20240819, Krzysztof Kozlowski wrote:
> >> On 19/08/2024 22:43, Judith Mendez wrote:
> >>> SK-AM62A-LP is a device targeting automotive front-camera applications
> >>> among other use-cases. It utilizes the TPS6593x PMIC (interfaced over I2C)
> >>> to power the SoC and various other peripherals on the board [1].
> >>>
> >>> MMCSD requires the PMIC to be setup correctly before setting the bus
> >>> pins to 1.8V using the TPS6594 driver interfaced over i2c.
> >>>
> >>> Currently, the following could be seen when booting the am62ax platform:
> >>>
> >>> "platform fa00000.mmc: deferred probe pending: platform: supplier regulator-5 not ready"
> >>> "vdd_mmc1: disabling"
> >>>
> >>> and a failure to boot the SK-AM62A-LP.
> >>>
> >>> One solution is to use initramfs [2], but using initramfs increases the
> >>> boot time for this automotive solution which requires faster boot time
> >>> parameters.
> >>
> >> This is a defconfig, not a distro/product config, so your product
> >> constraints are not really relevant. You are supposed to boot defconfig
> >> with proper initramfs with necessary modules.
> >>
> >> I don't get why people mistake defconfig with their product stuff...
> >>
> >>>
> >>> Another solution is to change MFD_TPS6594_I2C to built-in, that way the
> >>> PMIC is setup and the regulators are ready before MMCSD switches to UHS
> >>> mode, this is the preferred solution since it does not increase boot time
> >>> like the initramfs solution does.
> >>
> >> Use initramfs. Several devices, e.g. most Android ones, have fixed size
> >> of boot partition, so size of kernel is important.
> > 
> > am62a products do not use android in general. Standard distros such
> 
> But I (and others) use sometimes devices with Android partitioning so
> the size of vmlinux is important. I want to be able to enable KASAN. If
> each person brings their modules into built-in, I won't be.

The scope is restricted to be sd and nfs boot capability only in Image
file.

> 
> > as debian etc usage are limited as well. These products tend to have
> > limited resources just sufficient for the normal operations.
> 
> So defconfig is not suitable for them in the first place.
> 
> > 
> > While I understand that we do keep the product usage model separate
> > from what upstream defconfig looks like, we have been very careful
> > to only enable the basic configurations necessary for default system
> > startup. During the initial days of K3, we had considered going down
> > the initramfs route, but realized that this was not a practical
> > option for developers to sustain and iterate quickly for triage or
> > development. Till date, we have maintained nfs and sd card boot as
> > default to allow for automated testing of upstream kernel.
> 
> I don't understand what is here not practical. Entire Qualcomm Landing
> team for all products uses initramfs, my Exynos development uses
> initramfs. There is no difference in building the kernel, no practical
> impact, the same effort (after setting up the scripts, but we all are
> scripting). I know, that 10 years ago many of developers, including
> myself, did not want to switch to initramfs, but things change.

I understand initramfs is not new and it is not rocket science. The
ecosystem however hasn't adopted it for few reasons that I mentioned.

As of commit v6.11-rc4-19-gb311c1b497e5 blindly doing a make
modules_install with the default defconfig is 1.1G loaded via
initramfs Vs putting it in a filesystem - those extra load times are
significant for iterative development - you can see what is
practical - unless I deal with each of the engineers having custom
scripts selectively setting up modules for their initramfs environment
and dealing with that consequence - I really don't want to get into
the business of distro build rules with each engineer (and this goes
to the entire ecosystem I deal with).

> 
> > 
> > I understand that you have provided similar comments for other
> > platforms[1] as well, but, I am now wondering if this is a new rule
> > we are taking in aarch64 platforms to allow just initramfs and
> > force all drivers to be modules (I understand that is the default
> 
> It's not particularly new. The use of modules for multi_v7 and
> arm64/defconfig was since years.

The insistence of initramfs as the only way the default defconfig will
boot is a bit new.

> 
> > preference in android, but that is not true in other ecosystems). I am
> > curious if this was some sort of conclusion in the list (my search of
> > public-inbox seems to fail me here).
> 
> There are different point of views. I am presenting here mine and I back
> it with arguments, that such changes accumulate and have impact on
> developers workflow.
> 
> Defconfig is for developers and as starting point for distros or
> products. Not as final product defconfig.

This is a bit of a duality thing here - the argument you have
presented in the thread indicates that android based product platforms
you work with uses fixed boot partition sizes for kernel Image. And as
a result increasing Image size has side effects of preventing boot.
K3 architecture devices had already considered the possibility that
Image files will grow given newer SoC support being added. We picked a
balance of what was practical for our ecosystem. From where I stand, I
see insistence of "your product" usecase to be more critical than my
ecosystem needs of consistency and flexibility.

To the implied statement that we are insisting on product defconfig
in upstream: NO, we have not. We maintain config fragments in our s/w
products as necessary already.

For upstream, we already have insisted on a line in our ecosystem that
is sd and ethernet/nfs boot as the minimal configuration, if there is
an effort to force just one k3 product to do initramfs Vs all other
products in the family to have capability of sd/nfs boot - that, in my
opinion, is not treating the ecosystem fairly and equally.

Further, I am not a fan of arbitrary new "it should be better done
this way" rules introduced just for a few products, but not for
others.

I think you already know this, but the right way, IMHO, to get this
kind of general recommendation is to introduce documentation into
Documentation/arch/arm64/ or equivalent with reasonable guidance to
developers how to manage the initramfs. I would however suggest doing
this with the 2025 kernel as the first transition point and we can all
work together to reduce the size of Image. Do send a patch introducing
the recommended content for arm64/defconfig and we can debate the pros
and cons in that context.

That said, there are things to improve in this patch, I will comment
separately on it. As such, enabling SD/ethernet nfs boot is inline
with the base policy we had set for k3 ecosystem. Unless Arnd or SoC
maintainers insist we start arbitrarily break parts of the ecosystem,
I don't see a reason to reject this support (If we have a problem
adding a few K for a regulator driver, I suppose we have already
reached a limit where adding a new platform arch with common defconfig
may be impractical and we have much bigger problems and needs to be
addressed via reducing the scope of defconfig generated Image overall -
but lets do that across arm64 platforms consistently).

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [RFC PATCH] arm64: defconfig: Set MFD_TPS6594_I2C as built-in

[Bjorn Andersson]

On Mon, Aug 19, 2024 at 03:43:52PM -0500, Judith Mendez wrote:
> SK-AM62A-LP is a device targeting automotive front-camera applications
> among other use-cases. It utilizes the TPS6593x PMIC (interfaced over I2C)
> to power the SoC and various other peripherals on the board [1].
> 
> MMCSD requires the PMIC to be setup correctly before setting the bus
> pins to 1.8V using the TPS6594 driver interfaced over i2c.
> 
> Currently, the following could be seen when booting the am62ax platform:
> 
> "platform fa00000.mmc: deferred probe pending: platform: supplier regulator-5 not ready"
> "vdd_mmc1: disabling"

Is this the regulator framework disabling the "unused" vdd_mmc1 while
you still have a probe deferred client?

That's not right.

> 
> and a failure to boot the SK-AM62A-LP.
> 
> One solution is to use initramfs [2], but using initramfs increases the
> boot time for this automotive solution which requires faster boot time
> parameters.
> 
> Another solution is to change MFD_TPS6594_I2C to built-in, that way the
> PMIC is setup and the regulators are ready before MMCSD switches to UHS
> mode, this is the preferred solution since it does not increase boot time
> like the initramfs solution does.
> 
> [1] https://www.ti.com/lit/zip/sprr459
> [2] https://lore.kernel.org/linux-devicetree/5f03207b-c29b-4d16-92b0-d14eef77bf17@linaro.org/
> Fixes: f9010eb938be ("arm64: defconfig: Enable TPS6593 PMIC for SK-AM62A")
> 
> Signed-off-by: Judith Mendez <jm@ti.com>
> ---
>  arch/arm64/configs/defconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index 7d32fca649965..61f767246d3a5 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -756,7 +756,7 @@ CONFIG_RZ_MTU3=y
>  CONFIG_MFD_TI_AM335X_TSCADC=m
>  CONFIG_MFD_TI_LP873X=m
>  CONFIG_MFD_TPS65219=y
> -CONFIG_MFD_TPS6594_I2C=m
> +CONFIG_MFD_TPS6594_I2C=y

These things should work with =m, and then you can make them =y in your
product config to avoid the probe deferral.

Regards,
Bjorn

>  CONFIG_MFD_ROHM_BD718XX=y
>  CONFIG_MFD_WCD934X=m
>  CONFIG_MFD_KHADAS_MCU=m
> 
> base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b
> -- 
> 2.46.0
> 

Re: [RFC PATCH] arm64: defconfig: Set MFD_TPS6594_I2C as built-in

[Nishanth Menon]

On 09:42-20240820, Bjorn Andersson wrote:
> On Mon, Aug 19, 2024 at 03:43:52PM -0500, Judith Mendez wrote:
> > SK-AM62A-LP is a device targeting automotive front-camera applications
> > among other use-cases. It utilizes the TPS6593x PMIC (interfaced over I2C)
> > to power the SoC and various other peripherals on the board [1].
> > 
> > MMCSD requires the PMIC to be setup correctly before setting the bus
> > pins to 1.8V using the TPS6594 driver interfaced over i2c.
> > 
> > Currently, the following could be seen when booting the am62ax platform:
> > 
> > "platform fa00000.mmc: deferred probe pending: platform: supplier regulator-5 not ready"
> > "vdd_mmc1: disabling"
> 
> Is this the regulator framework disabling the "unused" vdd_mmc1 while
> you still have a probe deferred client?
> 
> That's not right.

I think context of the full log might be missing, hence misleading - but
if this is correct, regulator framework might have a problem as well -
it does not change the fact that we will need regulator driver to
support switch from 3.3v to 1.8v to enable UHS (which is another
topic as well - do not trust the bootloader either for the default
voltage enabled in the system).

Judith: This is why providing links to previous versions of patches AND
providing links to the full before and after logs in the diffstat
section helps reviewers - please do that in the next revision.

> 
> > 
> > and a failure to boot the SK-AM62A-LP.
> > 
> > One solution is to use initramfs [2], but using initramfs increases the
> > boot time for this automotive solution which requires faster boot time
> > parameters.
> > 
> > Another solution is to change MFD_TPS6594_I2C to built-in, that way the
> > PMIC is setup and the regulators are ready before MMCSD switches to UHS
> > mode, this is the preferred solution since it does not increase boot time
> > like the initramfs solution does.

What is the adder in cost? bloat-o-meter report summary should have
been part of commits that increase vmlinux

> > 
> > [1] https://www.ti.com/lit/zip/sprr459
> > [2] https://lore.kernel.org/linux-devicetree/5f03207b-c29b-4d16-92b0-d14eef77bf17@linaro.org/
> > Fixes: f9010eb938be ("arm64: defconfig: Enable TPS6593 PMIC for SK-AM62A")
> > 
> > Signed-off-by: Judith Mendez <jm@ti.com>
> > ---
> >  arch/arm64/configs/defconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> > index 7d32fca649965..61f767246d3a5 100644
> > --- a/arch/arm64/configs/defconfig
> > +++ b/arch/arm64/configs/defconfig
> > @@ -756,7 +756,7 @@ CONFIG_RZ_MTU3=y
> >  CONFIG_MFD_TI_AM335X_TSCADC=m
> >  CONFIG_MFD_TI_LP873X=m
> >  CONFIG_MFD_TPS65219=y
> > -CONFIG_MFD_TPS6594_I2C=m
> > +CONFIG_MFD_TPS6594_I2C=y
> 
> These things should work with =m, and then you can make them =y in your
> product config to avoid the probe deferral.

I think this is inline with the discussion with Krystoff on the topic as
well


Judith: you just need the regulator, for sd/mmc to work. Why is there
half a dozen unrelated stuff coming with it [1] report from
bloat-o-meter? Please fix the driver dependencies before adding
un-necessary stuff to the kernel Image file.

[1] https://gist.github.com/nmenon/0caff032a430bbf649ad1107e6f12f9a

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [RFC PATCH] arm64: defconfig: Set MFD_TPS6594_I2C as built-in

[Judith Mendez]

Hello Nishanth,

Apologies for the late reply,

On 8/21/24 6:09 AM, Nishanth Menon wrote:
> On 09:42-20240820, Bjorn Andersson wrote:
>> On Mon, Aug 19, 2024 at 03:43:52PM -0500, Judith Mendez wrote:
>>> SK-AM62A-LP is a device targeting automotive front-camera applications
>>> among other use-cases. It utilizes the TPS6593x PMIC (interfaced over I2C)
>>> to power the SoC and various other peripherals on the board [1].
>>>
>>> MMCSD requires the PMIC to be setup correctly before setting the bus
>>> pins to 1.8V using the TPS6594 driver interfaced over i2c.
>>>
>>> Currently, the following could be seen when booting the am62ax platform:
>>>
>>> "platform fa00000.mmc: deferred probe pending: platform: supplier regulator-5 not ready"
>>> "vdd_mmc1: disabling"
>>
>> Is this the regulator framework disabling the "unused" vdd_mmc1 while
>> you still have a probe deferred client?
>>
>> That's not right.
> 
> I think context of the full log might be missing, hence misleading - but
> if this is correct, regulator framework might have a problem as well -
> it does not change the fact that we will need regulator driver to
> support switch from 3.3v to 1.8v to enable UHS (which is another
> topic as well - do not trust the bootloader either for the default
> voltage enabled in the system).
> 
> Judith: This is why providing links to previous versions of patches AND
> providing links to the full before and after logs in the diffstat
> section helps reviewers - please do that in the next revision.

Understood, will fix for v2.

> 
>>
>>>
>>> and a failure to boot the SK-AM62A-LP.
>>>
>>> One solution is to use initramfs [2], but using initramfs increases the
>>> boot time for this automotive solution which requires faster boot time
>>> parameters.
>>>
>>> Another solution is to change MFD_TPS6594_I2C to built-in, that way the
>>> PMIC is setup and the regulators are ready before MMCSD switches to UHS
>>> mode, this is the preferred solution since it does not increase boot time
>>> like the initramfs solution does.
> 
> What is the adder in cost? bloat-o-meter report summary should have
> been part of commits that increase vmlinux

Will include in v2.

> 
>>>
>>> [1] https://www.ti.com/lit/zip/sprr459
>>> [2] https://lore.kernel.org/linux-devicetree/5f03207b-c29b-4d16-92b0-d14eef77bf17@linaro.org/
>>> Fixes: f9010eb938be ("arm64: defconfig: Enable TPS6593 PMIC for SK-AM62A")
>>>
>>> Signed-off-by: Judith Mendez <jm@ti.com>
>>> ---
>>>   arch/arm64/configs/defconfig | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
>>> index 7d32fca649965..61f767246d3a5 100644
>>> --- a/arch/arm64/configs/defconfig
>>> +++ b/arch/arm64/configs/defconfig
>>> @@ -756,7 +756,7 @@ CONFIG_RZ_MTU3=y
>>>   CONFIG_MFD_TI_AM335X_TSCADC=m
>>>   CONFIG_MFD_TI_LP873X=m
>>>   CONFIG_MFD_TPS65219=y
>>> -CONFIG_MFD_TPS6594_I2C=m
>>> +CONFIG_MFD_TPS6594_I2C=y
>>
>> These things should work with =m, and then you can make them =y in your
>> product config to avoid the probe deferral.
> 
> I think this is inline with the discussion with Krystoff on the topic as
> well
> 
> 
> Judith: you just need the regulator, for sd/mmc to work. Why is there
> half a dozen unrelated stuff coming with it [1] report from
> bloat-o-meter? Please fix the driver dependencies before adding
> un-necessary stuff to the kernel Image file.
> 
> [1] https://gist.github.com/nmenon/0caff032a430bbf649ad1107e6f12f9a

Ok, will check this out, thanks for reviewing. (:

~ Judith

> 

Re: [RFC PATCH] arm64: defconfig: Set MFD_TPS6594_I2C as built-in

[Judith Mendez]

Hi Nishanth,

On 8/21/24 6:09 AM, Nishanth Menon wrote:
> On 09:42-20240820, Bjorn Andersson wrote:
>> On Mon, Aug 19, 2024 at 03:43:52PM -0500, Judith Mendez wrote:
>>> SK-AM62A-LP is a device targeting automotive front-camera applications
>>> among other use-cases. It utilizes the TPS6593x PMIC (interfaced over I2C)
>>> to power the SoC and various other peripherals on the board [1].
>>>
>>> MMCSD requires the PMIC to be setup correctly before setting the bus
>>> pins to 1.8V using the TPS6594 driver interfaced over i2c.
>>>
>>> Currently, the following could be seen when booting the am62ax platform:
>>>
>>> "platform fa00000.mmc: deferred probe pending: platform: supplier regulator-5 not ready"
>>> "vdd_mmc1: disabling"
>>
>> Is this the regulator framework disabling the "unused" vdd_mmc1 while
>> you still have a probe deferred client?
>>
>> That's not right.
> 
> I think context of the full log might be missing, hence misleading - but
> if this is correct, regulator framework might have a problem as well -
> it does not change the fact that we will need regulator driver to
> support switch from 3.3v to 1.8v to enable UHS (which is another
> topic as well - do not trust the bootloader either for the default
> voltage enabled in the system).
> 
> Judith: This is why providing links to previous versions of patches AND
> providing links to the full before and after logs in the diffstat
> section helps reviewers - please do that in the next revision.
> 
>>
>>>
>>> and a failure to boot the SK-AM62A-LP.
>>>
>>> One solution is to use initramfs [2], but using initramfs increases the
>>> boot time for this automotive solution which requires faster boot time
>>> parameters.
>>>
>>> Another solution is to change MFD_TPS6594_I2C to built-in, that way the
>>> PMIC is setup and the regulators are ready before MMCSD switches to UHS
>>> mode, this is the preferred solution since it does not increase boot time
>>> like the initramfs solution does.
> 
> What is the adder in cost? bloat-o-meter report summary should have
> been part of commits that increase vmlinux
> 
>>>
>>> [1] https://www.ti.com/lit/zip/sprr459
>>> [2] https://lore.kernel.org/linux-devicetree/5f03207b-c29b-4d16-92b0-d14eef77bf17@linaro.org/
>>> Fixes: f9010eb938be ("arm64: defconfig: Enable TPS6593 PMIC for SK-AM62A")
>>>
>>> Signed-off-by: Judith Mendez <jm@ti.com>
>>> ---
>>>   arch/arm64/configs/defconfig | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
>>> index 7d32fca649965..61f767246d3a5 100644
>>> --- a/arch/arm64/configs/defconfig
>>> +++ b/arch/arm64/configs/defconfig
>>> @@ -756,7 +756,7 @@ CONFIG_RZ_MTU3=y
>>>   CONFIG_MFD_TI_AM335X_TSCADC=m
>>>   CONFIG_MFD_TI_LP873X=m
>>>   CONFIG_MFD_TPS65219=y
>>> -CONFIG_MFD_TPS6594_I2C=m
>>> +CONFIG_MFD_TPS6594_I2C=y
>>
>> These things should work with =m, and then you can make them =y in your
>> product config to avoid the probe deferral.
> 
> I think this is inline with the discussion with Krystoff on the topic as
> well
> 
> 
> Judith: you just need the regulator, for sd/mmc to work. Why is there
> half a dozen unrelated stuff coming with it [1] report from
> bloat-o-meter? Please fix the driver dependencies before adding
> un-necessary stuff to the kernel Image file.
> 
> [1] https://gist.github.com/nmenon/0caff032a430bbf649ad1107e6f12f9a


It seems like TPS6594_I2C driver has a few dependencies:

select MFD_TPS6594
select REGMAP_I2C
select CRC8
depends on I2C

I am not sure what could be done here to change the driver to built
in without increasing the size of the kernel this much:

Total: Before=19064141, After=19108349, chg +0.23%

Please advice.
Thanks,

~ Judith