[PATCH] mmc: meson-gx: increase power-up delay

[PATCH] mmc: meson-gx: increase power-up delay

[Marc Gonzalez]

With the default power-up delay, on small kernels, the host probes
too soon, and mmc_send_io_op_cond() times out.

Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
---
Stress-tested with 80 cold boots, checking every time
mmc2: new ultra high speed SDR50 SDIO card at address 0001
IIUC, this will also slow down SD & MMC probing,
but an additional 20 ms seems acceptable?
---
 drivers/mmc/host/meson-gx-mmc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 6e5ea0213b477..73ecbcf588c65 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -1182,6 +1182,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	mmc = mmc_alloc_host(sizeof(struct meson_host), &pdev->dev);
 	if (!mmc)
 		return -ENOMEM;
+	mmc->ios.power_delay_ms = 20;
 	host = mmc_priv(mmc);
 	host->mmc = mmc;
 	host->dev = &pdev->dev;
-- 
2.25.1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH] mmc: meson-gx: increase power-up delay

[Heiner Kallweit]

On 14.03.2023 18:24, Marc Gonzalez wrote:
> With the default power-up delay, on small kernels, the host probes
> too soon, and mmc_send_io_op_cond() times out.
> 
Looking at mmc_power_up() and how power_delay_ms is used
I wonder what you mean with "host probes too soon".
Are you sure that the additional delay is needed for the Amlogic MMC
block IP in general? Or could it be that your issue is caused by
a specific regulator and you need to add a delay there?

> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
> ---
> Stress-tested with 80 cold boots, checking every time
> mmc2: new ultra high speed SDR50 SDIO card at address 0001
> IIUC, this will also slow down SD & MMC probing,
> but an additional 20 ms seems acceptable?
> ---
>  drivers/mmc/host/meson-gx-mmc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 6e5ea0213b477..73ecbcf588c65 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -1182,6 +1182,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  	mmc = mmc_alloc_host(sizeof(struct meson_host), &pdev->dev);
>  	if (!mmc)
>  		return -ENOMEM;
> +	mmc->ios.power_delay_ms = 20;
>  	host = mmc_priv(mmc);
>  	host->mmc = mmc;
>  	host->dev = &pdev->dev;


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH] mmc: meson-gx: increase power-up delay

[Marc Gonzalez]

On 14/03/2023 20:45, Heiner Kallweit wrote:

> On 14.03.2023 18:24, Marc Gonzalez wrote:
> 
>> With the default power-up delay, on small kernels, the host probes
>> too soon, and mmc_send_io_op_cond() times out.
> 
> Looking at mmc_power_up() and how power_delay_ms is used
> I wonder what you mean with "host probes too soon".

Hello Heiner,

Thanks for your interest in my patch! :)

I should have added a link to the thread that led to the patch.
https://patchwork.kernel.org/project/linux-wireless/patch/c1a215cf-94be-871b-2a8a-3cc381588f83@free.fr/
Start at "I have run into another issue."

Basically, I have an S905X2-based board.
I built a small kernel for it (with only a few drivers), that boots really fast.

mmc2 (SDIO controller hooked to WiFi chip) would not probe at all,
unless I added lots of printks.
Basically, calling mmc_send_io_op_cond() too soon after the controller
has been reset leads to the CMD5 request timing out.


> Are you sure that the additional delay is needed for the Amlogic MMC
> block IP in general? Or could it be that your issue is caused by
> a specific regulator and you need to add a delay there?

The eternal question...

I have only one type of board. (Actually, I have a reference design
that is slightly different, so I should test on that one as well.)

In vendor kernels, they add delays to the WiFi drivers.
Maybe they have run into the issue, and they're just fixing the symptom?

Default value for ios.power_delay_ms is 10 ms.
msleep(ios.power_delay_ms) is called twice in mmc_power_up().
So raising the delay from 10 to 20 adds 20 ms
to the latency of initializing SDIO/SD/MMC controllers.

Would you be willing to test if the problem manifests on your board?

Regards



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH] mmc: meson-gx: increase power-up delay

[Jerome Brunet]

On Wed 15 Mar 2023 at 11:20, Marc Gonzalez <marc.w.gonzalez@free.fr> wrote:

> On 14/03/2023 20:45, Heiner Kallweit wrote:
>
>> On 14.03.2023 18:24, Marc Gonzalez wrote:
>> 
>>> With the default power-up delay, on small kernels, the host probes
>>> too soon, and mmc_send_io_op_cond() times out.
>> 
>> Looking at mmc_power_up() and how power_delay_ms is used
>> I wonder what you mean with "host probes too soon".
>
> Hello Heiner,
>
> Thanks for your interest in my patch! :)
>
> I should have added a link to the thread that led to the patch.
> https://patchwork.kernel.org/project/linux-wireless/patch/c1a215cf-94be-871b-2a8a-3cc381588f83@free.fr/
> Start at "I have run into another issue."
>
> Basically, I have an S905X2-based board.
> I built a small kernel for it (with only a few drivers), that boots really fast.
>
> mmc2 (SDIO controller hooked to WiFi chip) would not probe at all,
> unless I added lots of printks.
> Basically, calling mmc_send_io_op_cond() too soon after the controller
> has been reset leads to the CMD5 request timing out.
>
>

I tend to agree with Heiner here.
This patch is backing a contraint only reported on your design in the
driver of every AML SoC supported, for every MMC controller.

I think you should look first in your vmmc and vqmmc regulators and
their setup times.

"fixed-regulator" have properties which might be interesting to you,
like
 * startup-delay-us
 * off-on-delay-us

>> Are you sure that the additional delay is needed for the Amlogic MMC
>> block IP in general? Or could it be that your issue is caused by
>> a specific regulator and you need to add a delay there?
>
> The eternal question...
>
> I have only one type of board. (Actually, I have a reference design
> that is slightly different, so I should test on that one as well.)
>
> In vendor kernels, they add delays to the WiFi drivers.
> Maybe they have run into the issue, and they're just fixing the symptom?
>
> Default value for ios.power_delay_ms is 10 ms.
> msleep(ios.power_delay_ms) is called twice in mmc_power_up().
> So raising the delay from 10 to 20 adds 20 ms
> to the latency of initializing SDIO/SD/MMC controllers.
>
> Would you be willing to test if the problem manifests on your board?
>
> Regards


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH] mmc: meson-gx: increase power-up delay

[Heiner Kallweit]

On 15.03.2023 16:27, Jerome Brunet wrote:
> 
> On Wed 15 Mar 2023 at 11:20, Marc Gonzalez <marc.w.gonzalez@free.fr> wrote:
> 
>> On 14/03/2023 20:45, Heiner Kallweit wrote:
>>
>>> On 14.03.2023 18:24, Marc Gonzalez wrote:
>>>
>>>> With the default power-up delay, on small kernels, the host probes
>>>> too soon, and mmc_send_io_op_cond() times out.
>>>
>>> Looking at mmc_power_up() and how power_delay_ms is used
>>> I wonder what you mean with "host probes too soon".
>>
>> Hello Heiner,
>>
>> Thanks for your interest in my patch! :)
>>
>> I should have added a link to the thread that led to the patch.
>> https://patchwork.kernel.org/project/linux-wireless/patch/c1a215cf-94be-871b-2a8a-3cc381588f83@free.fr/
>> Start at "I have run into another issue."
>>
>> Basically, I have an S905X2-based board.
>> I built a small kernel for it (with only a few drivers), that boots really fast.
>>
>> mmc2 (SDIO controller hooked to WiFi chip) would not probe at all,
>> unless I added lots of printks.
>> Basically, calling mmc_send_io_op_cond() too soon after the controller
>> has been reset leads to the CMD5 request timing out.
>>
>>
> 
> I tend to agree with Heiner here.
> This patch is backing a contraint only reported on your design in the
> driver of every AML SoC supported, for every MMC controller.
> 
> I think you should look first in your vmmc and vqmmc regulators and
> their setup times.
> 
> "fixed-regulator" have properties which might be interesting to you,
> like
>  * startup-delay-us
>  * off-on-delay-us
> 
>>> Are you sure that the additional delay is needed for the Amlogic MMC
>>> block IP in general? Or could it be that your issue is caused by
>>> a specific regulator and you need to add a delay there?
>>
>> The eternal question...
>>
>> I have only one type of board. (Actually, I have a reference design
>> that is slightly different, so I should test on that one as well.)
>>

Let me ask few more questions:

You said that the issue is with a SDIO card. How about eMMC and sdcard,
does the issue occur for them too?

Then you mention "too soon after reset", but add a delay to power-up.
If the delay would be needed after reset, then shouldn't it be in
meson_mmc_probe() after the call to device_reset_optional()?

>> In vendor kernels, they add delays to the WiFi drivers.
>> Maybe they have run into the issue, and they're just fixing the symptom?
>>
>> Default value for ios.power_delay_ms is 10 ms.
>> msleep(ios.power_delay_ms) is called twice in mmc_power_up().
>> So raising the delay from 10 to 20 adds 20 ms
>> to the latency of initializing SDIO/SD/MMC controllers.
>>
>> Would you be willing to test if the problem manifests on your board?
>>
>> Regards
> 


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH] mmc: meson-gx: increase power-up delay

[Ulf Hansson]

On Wed, 15 Mar 2023 at 16:31, Jerome Brunet <jbrunet@baylibre.com> wrote:
>
>
> On Wed 15 Mar 2023 at 11:20, Marc Gonzalez <marc.w.gonzalez@free.fr> wrote:
>
> > On 14/03/2023 20:45, Heiner Kallweit wrote:
> >
> >> On 14.03.2023 18:24, Marc Gonzalez wrote:
> >>
> >>> With the default power-up delay, on small kernels, the host probes
> >>> too soon, and mmc_send_io_op_cond() times out.
> >>
> >> Looking at mmc_power_up() and how power_delay_ms is used
> >> I wonder what you mean with "host probes too soon".
> >
> > Hello Heiner,
> >
> > Thanks for your interest in my patch! :)
> >
> > I should have added a link to the thread that led to the patch.
> > https://patchwork.kernel.org/project/linux-wireless/patch/c1a215cf-94be-871b-2a8a-3cc381588f83@free.fr/
> > Start at "I have run into another issue."
> >
> > Basically, I have an S905X2-based board.
> > I built a small kernel for it (with only a few drivers), that boots really fast.
> >
> > mmc2 (SDIO controller hooked to WiFi chip) would not probe at all,
> > unless I added lots of printks.
> > Basically, calling mmc_send_io_op_cond() too soon after the controller
> > has been reset leads to the CMD5 request timing out.
> >
> >
>
> I tend to agree with Heiner here.
> This patch is backing a contraint only reported on your design in the
> driver of every AML SoC supported, for every MMC controller.
>
> I think you should look first in your vmmc and vqmmc regulators and
> their setup times.
>
> "fixed-regulator" have properties which might be interesting to you,
> like
>  * startup-delay-us
>  * off-on-delay-us

If the problem is regulator specific, this would be the correct thing to do.

Although, if the problem is pwrseq specific, like that we need a delay
after enabling the clock and asserting the GPIO enable pin for the
WiFi chip, then we have the "post-power-on-delay-ms" of the pwrseq
node to play with instead.

>
> >> Are you sure that the additional delay is needed for the Amlogic MMC
> >> block IP in general? Or could it be that your issue is caused by
> >> a specific regulator and you need to add a delay there?
> >
> > The eternal question...
> >
> > I have only one type of board. (Actually, I have a reference design
> > that is slightly different, so I should test on that one as well.)
> >
> > In vendor kernels, they add delays to the WiFi drivers.
> > Maybe they have run into the issue, and they're just fixing the symptom?
> >
> > Default value for ios.power_delay_ms is 10 ms.
> > msleep(ios.power_delay_ms) is called twice in mmc_power_up().
> > So raising the delay from 10 to 20 adds 20 ms
> > to the latency of initializing SDIO/SD/MMC controllers.
> >
> > Would you be willing to test if the problem manifests on your board?
> >
> > Regards
>

Kind regards
Uffe

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH] mmc: meson-gx: increase power-up delay

[Marc Gonzalez]

On 15/03/2023 22:14, Heiner Kallweit wrote:

> Let me ask few more questions:
> 
> You said that the issue is with a SDIO card. How about eMMC and sdcard,
> does the issue occur for them too?

I have never seen an issue with eMMC not probing on this board.
(I have not tested the SD card slot, as I'm using it for serial.)

I did a (probably silly) test by setting mmc->ios.power_delay_ms = 0
in the driver, and logging relevant events (for timing).


Here's a boot log where both mmc1 & mmc2 probe correctly:

[    0.879968] >ffe03000.sd
[    0.880320] meson-gx-mmc ffe03000.sd: allocated mmc-pwrseq
[    0.891604] +mmc2
[    0.895179] -mmc2
[    0.896873] +mmc2
[    0.898829] <ffe03000.sd
[    0.951755] Q
[    1.049162] >ffe07000.mmc
[    1.049602] meson-gx-mmc ffe07000.mmc: allocated mmc-pwrseq
[    1.061378] +mmc1
[    1.061666] -mmc1
[    1.062841] +mmc1
[    1.063532] <ffe07000.mmc
[    1.072056] Q
[    1.079669] R
[    1.123210] mmc2: new ultra high speed SDR50 SDIO card at address 0001
[    1.193650] mmc1: new HS200 MMC card at address 0001
[    1.198553] mmcblk1: mmc1:0001 SCA16G 14.7 GiB 
[    1.205777] brcmf_sdio_probe_attach:  AFTER
[    1.209853] mmcblk1boot0: mmc1:0001 SCA16G 4.00 MiB 
[    1.211218] mmcblk1boot1: mmc1:0001 SCA16G 4.00 MiB 
[    1.212254] mmcblk1rpmb: mmc1:0001 SCA16G 4.00 MiB, chardev (246:0)


Analysis for mmc2

[    0.879968] ENTER meson_mmc_probe()
[    0.880320] meson-gx-mmc ffe03000.sd: allocated mmc-pwrseq
[    0.891604] ENTER mmc_power_up()
[    0.895179]  EXIT mmc_power_up()
[    0.896873] ENTER mmc_power_up() => premature exit
[    0.898829]  EXIT meson_mmc_probe()
[    0.951755] ENTER mmc_attach_sdio()
[    1.123210] mmc2: new ultra high speed SDR50 SDIO card at address 0001

mmc_attach_sdio() is called 60 ms after entering mmc_power_up()


Analysis for mmc1

[    1.049162] ENTER meson_mmc_probe()
[    1.049602] meson-gx-mmc ffe07000.mmc: allocated mmc-pwrseq
[    1.061378] ENTER mmc_power_up()
[    1.061666]  EXIT mmc_power_up()
[    1.062841] ENTER mmc_power_up()
[    1.063532]  EXIT meson_mmc_probe() => premature exit
[    1.072056] ENTER mmc_attach_sdio()
[    1.079669] ENTER mmc_attach_mmc()
[    1.193650] mmc1: new HS200 MMC card at address 0001

mmc_attach_mmc() is called 18 ms after entering mmc_power_up()



Here's a boot log where mmc2 *DOES NOT* probe correctly:

[    0.875868] >ffe03000.sd
[    0.876196] meson-gx-mmc ffe03000.sd: allocated mmc-pwrseq
[    0.881407] +mmc2
[    0.886832] -mmc2
[    0.888544] +mmc2
[    0.894688] <ffe03000.sd
[    0.901587] Q
[    0.908937] R
[    0.913466] mmc2: YO no device WTF
[    1.042609] >ffe07000.mmc
[    1.042910] meson-gx-mmc ffe07000.mmc: allocated mmc-pwrseq
[    1.053460] +mmc1
[    1.053694] -mmc1
[    1.055039] +mmc1
[    1.056956] <ffe07000.mmc
[    1.064126] Q
[    1.075205] R
[    1.197676] mmc1: new HS200 MMC card at address 0001
[    1.198337] mmcblk1: mmc1:0001 SCA16G 14.7 GiB 
[    1.203947] mmcblk1boot0: mmc1:0001 SCA16G 4.00 MiB 
[    1.207858] mmcblk1boot1: mmc1:0001 SCA16G 4.00 MiB 
[    1.212593] mmcblk1rpmb: mmc1:0001 SCA16G 4.00 MiB, chardev (246:0)

In this instance, for mmc2, mmc_attach_sdio() is called 20 ms after
entering mmc_power_up() and CMD5 times out (probe failure).


> Then you mention "too soon after reset", but add a delay to power-up.
> If the delay would be needed after reset, then shouldn't it be in
> meson_mmc_probe() after the call to device_reset_optional()?


The first mmc_power_up() is called from:

[    0.916446]  mmc_power_up+0x2c/0xe4
[    0.926186]  mmc_start_host+0x94/0xa0
[    0.926197]  mmc_add_host+0x84/0xf0
[    0.926205]  meson_mmc_probe+0x374/0x3f4
[    0.933086]  platform_probe+0x68/0xe0
[    0.940159]  really_probe+0xbc/0x2f0
[    0.948008]  __driver_probe_device+0x78/0xe0
[    0.955080]  driver_probe_device+0xd8/0x160
[    0.955087]  __driver_attach_async_helper+0x4c/0xc0
[    0.965689]  async_run_entry_fn+0x34/0xe0
[    0.965699]  process_one_work+0x1cc/0x320
[    0.972847]  worker_thread+0x14c/0x450
[    0.972855]  kthread+0x10c/0x110
[    1.018906]  ret_from_fork+0x10/0x20


The second mmc_power_up() (prematurely exited) is called from:

[    1.061686]  mmc_power_up+0x2c/0xe4
[    1.065136]  mmc_rescan+0x18c/0x310
[    1.068586]  process_one_work+0x1cc/0x320
[    1.072553]  worker_thread+0x14c/0x450
[    1.076262]  kthread+0x10c/0x110
[    1.086528]  ret_from_fork+0x10/0x20


I'm confused about device_reset_optional() vs mmc_power_up().
Do they both reset the controller?

Regards


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH] mmc: meson-gx: increase power-up delay

[Marc Gonzalez]

On 16/03/2023 13:32, Marc Gonzalez wrote:

> On 15/03/2023 22:14, Heiner Kallweit wrote:
> 
>> Then you mention "too soon after reset", but add a delay to power-up.
>> If the delay would be needed after reset, then shouldn't it be in
>> meson_mmc_probe() after the call to device_reset_optional()?
> 
> The first mmc_power_up() is called from:
> 
> [    0.916446]  mmc_power_up+0x2c/0xe4
> [    0.926186]  mmc_start_host+0x94/0xa0
> [    0.926197]  mmc_add_host+0x84/0xf0
> [    0.926205]  meson_mmc_probe+0x374/0x3f4
> [    0.933086]  platform_probe+0x68/0xe0
> [    0.940159]  really_probe+0xbc/0x2f0
> [    0.948008]  __driver_probe_device+0x78/0xe0
> [    0.955080]  driver_probe_device+0xd8/0x160
> [    0.955087]  __driver_attach_async_helper+0x4c/0xc0
> [    0.965689]  async_run_entry_fn+0x34/0xe0
> [    0.965699]  process_one_work+0x1cc/0x320
> [    0.972847]  worker_thread+0x14c/0x450
> [    0.972855]  kthread+0x10c/0x110
> [    1.018906]  ret_from_fork+0x10/0x20
> 
> 
> The second mmc_power_up() (prematurely exited) is called from:
> 
> [    1.061686]  mmc_power_up+0x2c/0xe4
> [    1.065136]  mmc_rescan+0x18c/0x310
> [    1.068586]  process_one_work+0x1cc/0x320
> [    1.072553]  worker_thread+0x14c/0x450
> [    1.076262]  kthread+0x10c/0x110
> [    1.086528]  ret_from_fork+0x10/0x20
> 
> 
> I'm confused about device_reset_optional() vs mmc_power_up().
> Do they both reset the controller?

NB: adding 500 ms delay after device_reset_optional() DOES NOT
fix the issue:

[    0.875494] >ffe03000.sd
[    0.875823] meson-gx-mmc ffe03000.sd: allocated mmc-pwrseq
[    1.027111] >ffe07000.mmc
[    1.027430] meson-gx-mmc ffe07000.mmc: allocated mmc-pwrseq
[    1.405170] +mmc2
[    1.405389] -mmc2
[    1.405413] <ffe03000.sd
[    1.405500] +mmc2
[    1.412957] Q
[    1.420317] R
[    1.422590] mmc2: YO no device WTF

We enter mmc_power_up at 1.405170
We enter mmc_attach_sdio at 1.412957
Empirically, 8 ms is not enough time for the SDIO bus to be ready.



Reverting to default power_delay_ms.

[    0.879842] >ffe03000.sd
[    0.880189] meson-gx-mmc ffe03000.sd: allocated mmc-pwrseq
[    1.031635] >ffe07000.mmc
[    1.031958] meson-gx-mmc ffe07000.mmc: allocated mmc-pwrseq
[    1.405284] +mmc2
[    1.430499] -mmc2
[    1.430525] <ffe03000.sd
[    1.430616] +mmc2
[    1.438065] Q
[    1.445404] R
[    1.447677] mmc2: YO no device WTF

We enter mmc_power_up at 1.405284
We enter mmc_attach_sdio at 1.438065
Empirically, 33 ms is not enough time for the SDIO bus to be ready.



Increasing power_delay_ms to 15.

[    0.879788] >ffe03000.sd
[    0.880092] meson-gx-mmc ffe03000.sd: allocated mmc-pwrseq
[    1.031437] >ffe07000.mmc
[    1.031767] meson-gx-mmc ffe07000.mmc: allocated mmc-pwrseq
[    1.405262] +mmc2
[    1.442981] -mmc2
[    1.443007] <ffe03000.sd
[    1.443099] +mmc2
[    1.450549] Q
[    1.565950] +mmc1
[    1.584306] mmc2: new ultra high speed SDR50 SDIO card at address 0001

We enter mmc_power_up at 1.405262
We enter mmc_attach_sdio at 1.450549
Empirically, 45 ms *is* enough time for the SDIO bus to be ready.


My gut feeling is that the problem with SDIO devices exists on all boards.
(I am well aware that I may be wrong.)

Would you be willing to test a small kernel?
I can provide a kernel defconfig and/or a buildroot defconfig.


Regards


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH] mmc: meson-gx: increase power-up delay

[Marc Gonzalez]

On 16/03/2023 12:59, Ulf Hansson wrote:

> If the problem is regulator specific, this would be the correct thing to do.
> 
> Although, if the problem is pwrseq specific, like that we need a delay
> after enabling the clock and asserting the GPIO enable pin for the
> WiFi chip, then we have the "post-power-on-delay-ms" of the pwrseq
> node to play with instead.

Heiner, Jerome, Uffe, Amlogic DT maintainers,

Perhaps tweaking the driver for every user is too intrusive.

Another solution would be to factorize all boards that implement
the reference design SDIO-based RF (WiFi + BT using brcm).

Then we can add post-power-on-delay-ms = 20 in sdio_pwrseq.

NB: there's vddio_ao1v8 vs vddao_1v8 in sei510.
Is my addition OK?

RFC prototype:

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-radxa-zero.dts b/arch/arm64/boot/dts/amlogic/meson-g12a-radxa-zero.dts
index e3bb6df42ff3e..42b5dcf358912 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12a-radxa-zero.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a-radxa-zero.dts
@@ -6,6 +6,7 @@
 /dts-v1/;
 
 #include "meson-g12a.dtsi"
+#include "meson-g12a-ref-design-brcm-rf.dtsi"
 #include <dt-bindings/gpio/meson-g12a-gpio.h>
 #include <dt-bindings/sound/meson-g12a-tohdmitx.h>
 
@@ -53,13 +54,6 @@ emmc_pwrseq: emmc-pwrseq {
 		reset-gpios = <&gpio BOOT_12 GPIO_ACTIVE_LOW>;
 	};
 
-	sdio_pwrseq: sdio-pwrseq {
-		compatible = "mmc-pwrseq-simple";
-		reset-gpios = <&gpio GPIOX_6 GPIO_ACTIVE_LOW>;
-		clocks = <&wifi32k>;
-		clock-names = "ext_clock";
-	};
-
 	ao_5v: regulator-ao_5v {
 		compatible = "regulator-fixed";
 		regulator-name = "AO_5V";
@@ -182,13 +176,6 @@ codec {
 			};
 		};
 	};
-
-	wifi32k: wifi32k {
-		compatible = "pwm-clock";
-		#clock-cells = <0>;
-		clock-frequency = <32768>;
-		pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
-	};
 };
 
 &arb {
@@ -299,37 +286,6 @@ &saradc {
 	vref-supply = <&vddao_1v8>;
 };
 
-/* SDIO */
-&sd_emmc_a {
-	status = "okay";
-	pinctrl-0 = <&sdio_pins>;
-	pinctrl-1 = <&sdio_clk_gate_pins>;
-	pinctrl-names = "default", "clk-gate";
-	#address-cells = <1>;
-	#size-cells = <0>;
-
-	bus-width = <4>;
-	cap-sd-highspeed;
-	sd-uhs-sdr50;
-	max-frequency = <100000000>;
-
-	non-removable;
-	disable-wp;
-
-	/* WiFi firmware requires power to be kept while in suspend */
-	keep-power-in-suspend;
-
-	mmc-pwrseq = <&sdio_pwrseq>;
-
-	vmmc-supply = <&vddao_3v3>;
-	vqmmc-supply = <&vddao_1v8>;
-
-	brcmf: wifi@1 {
-		reg = <1>;
-		compatible = "brcm,bcm4329-fmac";
-	};
-};
-
 /* SD card */
 &sd_emmc_b {
 	status = "okay";
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-ref-design-brcm-rf.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12a-ref-design-brcm-rf.dtsi
new file mode 100644
index 0000000000000..e462324596964
--- /dev/null
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a-ref-design-brcm-rf.dtsi
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
+ */
+
+#include <dt-bindings/gpio/meson-g12a-gpio.h>
+
+/ {
+	sdio_pwrseq: sdio-pwrseq {
+		compatible = "mmc-pwrseq-simple";
+		reset-gpios = <&gpio GPIOX_6 GPIO_ACTIVE_LOW>;
+		clocks = <&wifi32k>;
+		clock-names = "ext_clock";
+	};
+
+	wifi32k: wifi32k {
+		compatible = "pwm-clock";
+		#clock-cells = <0>;
+		clock-frequency = <32768>;
+		pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
+	};
+};
+
+/* SDIO */
+&sd_emmc_a {
+	status = "okay";
+	pinctrl-0 = <&sdio_pins>;
+	pinctrl-1 = <&sdio_clk_gate_pins>;
+	pinctrl-names = "default", "clk-gate";
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	bus-width = <4>;
+	cap-sd-highspeed;
+	sd-uhs-sdr50;
+	max-frequency = <100000000>;
+
+	non-removable;
+	disable-wp;
+
+	/* WiFi firmware requires power to be kept while in suspend */
+	keep-power-in-suspend;
+
+	mmc-pwrseq = <&sdio_pwrseq>;
+
+	vmmc-supply = <&vddao_3v3>;
+	vqmmc-supply = <&vddao_1v8>;
+
+	brcmf: wifi@1 {
+		reg = <1>;
+		compatible = "brcm,bcm4329-fmac";
+	};
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts b/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
index 23b790c6469d3..e12aeb956b7d7 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
@@ -6,6 +6,7 @@
 /dts-v1/;
 
 #include "meson-g12a.dtsi"
+#include "meson-g12a-ref-design-brcm-rf.dtsi"
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/gpio/meson-g12a-gpio.h>
@@ -96,6 +97,15 @@ emmc_1v8: regulator-emmc_1v8 {
 		regulator-always-on;
 	};
 
+	vddao_1v8: regulator-vddao_1v8 {
+		compatible = "regulator-fixed";
+		regulator-name = "VDDAO_1V8";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		vin-supply = <&vddao_3v3>;
+		regulator-always-on;
+	};
+
 	vddao_3v3: regulator-vddao_3v3 {
 		compatible = "regulator-fixed";
 		regulator-name = "VDDAO_3V3";
@@ -143,20 +153,6 @@ vddio_ao1v8: regulator-vddio_ao1v8 {
 		regulator-always-on;
 	};
 
-	sdio_pwrseq: sdio-pwrseq {
-		compatible = "mmc-pwrseq-simple";
-		reset-gpios = <&gpio GPIOX_6 GPIO_ACTIVE_LOW>;
-		clocks = <&wifi32k>;
-		clock-names = "ext_clock";
-	};
-
-	wifi32k: wifi32k {
-		compatible = "pwm-clock";
-		#clock-cells = <0>;
-		clock-frequency = <32768>;
-		pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
-	};
-
 	sound {
 		compatible = "amlogic,axg-sound-card";
 		model = "SEI510";
@@ -375,37 +371,6 @@ &saradc {
 	vref-supply = <&vddio_ao1v8>;
 };
 
-/* SDIO */
-&sd_emmc_a {
-	status = "okay";
-	pinctrl-0 = <&sdio_pins>;
-	pinctrl-1 = <&sdio_clk_gate_pins>;
-	pinctrl-names = "default", "clk-gate";
-	#address-cells = <1>;
-	#size-cells = <0>;
-
-	bus-width = <4>;
-	cap-sd-highspeed;
-	sd-uhs-sdr50;
-	max-frequency = <100000000>;
-
-	non-removable;
-	disable-wp;
-
-	/* WiFi firmware requires power to be kept while in suspend */
-	keep-power-in-suspend;
-
-	mmc-pwrseq = <&sdio_pwrseq>;
-
-	vmmc-supply = <&vddao_3v3>;
-	vqmmc-supply = <&vddio_ao1v8>;
-
-	brcmf: wifi@1 {
-		reg = <1>;
-		compatible = "brcm,bcm4329-fmac";
-	};
-};
-
 /* SD card */
 &sd_emmc_b {
 	status = "okay";
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
index b2bb94981838f..68a8876386115 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
@@ -6,6 +6,7 @@
 /dts-v1/;
 
 #include "meson-g12a.dtsi"
+#include "meson-g12a-ref-design-brcm-rf.dtsi"
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/gpio/meson-g12a-gpio.h>
 #include <dt-bindings/sound/meson-g12a-tohdmitx.h>
@@ -60,13 +61,6 @@ emmc_pwrseq: emmc-pwrseq {
 		reset-gpios = <&gpio BOOT_12 GPIO_ACTIVE_LOW>;
 	};
 
-	sdio_pwrseq: sdio-pwrseq {
-		compatible = "mmc-pwrseq-simple";
-		reset-gpios = <&gpio GPIOX_6 GPIO_ACTIVE_LOW>;
-		clocks = <&wifi32k>;
-		clock-names = "ext_clock";
-	};
-
 	flash_1v8: regulator-flash_1v8 {
 		compatible = "regulator-fixed";
 		regulator-name = "FLASH_1V8";
@@ -226,13 +220,6 @@ codec {
 			};
 		};
 	};
-
-	wifi32k: wifi32k {
-		compatible = "pwm-clock";
-		#clock-cells = <0>;
-		clock-frequency = <32768>;
-		pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
-	};
 };
 
 &arb {
@@ -391,37 +378,6 @@ &usb {
 	dr_mode = "host";
 };
 
-/* SDIO */
-&sd_emmc_a {
-	status = "okay";
-	pinctrl-0 = <&sdio_pins>;
-	pinctrl-1 = <&sdio_clk_gate_pins>;
-	pinctrl-names = "default", "clk-gate";
-	#address-cells = <1>;
-	#size-cells = <0>;
-
-	bus-width = <4>;
-	cap-sd-highspeed;
-	sd-uhs-sdr50;
-	max-frequency = <100000000>;
-
-	non-removable;
-	disable-wp;
-
-	/* WiFi firmware requires power to be kept while in suspend */
-	keep-power-in-suspend;
-
-	mmc-pwrseq = <&sdio_pwrseq>;
-
-	vmmc-supply = <&vddao_3v3>;
-	vqmmc-supply = <&vddao_1v8>;
-
-	brcmf: wifi@1 {
-		reg = <1>;
-		compatible = "brcm,bcm4329-fmac";
-	};
-};
-
 /* SD card */
 &sd_emmc_b {
 	status = "okay";


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic