All of lore.kernel.org
 help / color / mirror / Atom feed
From: Drew Fustini <drew@pdp7.com>
To: Maxim Kiselev <bigunclemax@gmail.com>
Cc: xry111@xry111.site, Albert Ou <aou@eecs.berkeley.edu>,
	Conor Dooley <conor@kernel.org>,
	devicetree@vger.kernel.org, dfustini@baylibre.com,
	guoren@kernel.org, jkridner@beagleboard.org, jszhang@kernel.org,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	open list <linux-kernel@vger.kernel.org>,
	linux-riscv@lists.infradead.org,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	robertcnelson@beagleboard.org, Rob Herring <robh+dt@kernel.org>,
	wefu@redhat.com
Subject: Re: [PATCH v8 4/4] riscv: dts: thead: Enable LicheePi 4A eMMC and microSD
Date: Sat, 23 Mar 2024 18:25:00 -0700	[thread overview]
Message-ID: <Zf+A7KEYL/tZb9/N@x1> (raw)
In-Reply-To: <CALHCpMhc1F5Ue7U_gsDXREHUZRVQJNYRCJxYxoNqbN=-39jf7A@mail.gmail.com>

On Wed, Mar 20, 2024 at 03:28:19PM +0300, Maxim Kiselev wrote:
> Hi Xi, Drew
> 
> I have the same problem with SD on my LicheePi 4A.
> 
> After some investigations I found how to fix this tuning error.
> Here is the patch that increases tuning loop count from
> 40(MAX_TUNING_LOOP at sdhci.c) to 128.
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c
> b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 8d6cfb648096..da8f5820fb69 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -706,6 +706,7 @@ static int th1520_execute_tuning(struct sdhci_host
> *host, u32 opcode)
> 
>         /* perform tuning */
>         sdhci_start_tuning(host);
> +       host->tuning_loop_count = 128:
>         host->tuning_err = __sdhci_execute_tuning(host, opcode);
>         if (host->tuning_err) {
>                 /* disable auto-tuning upon tuning error */
> 
> After that change tuning works fine. The same value of loop count is
> used in RevyOS BSP
> https://github.com/revyos/thead-kernel/blob/c6d4e5df18a17903d012ffd89e67d0ee5ce6cf2d/drivers/mmc/host/sdhci-of-dwcmshc.c#L185
> 
> Honestly, it looks a little bit strange for me.
> 
> It seems that the tuning algorithm requires to move through
> all the taps of delay line(128 taps?) even if we use THRESHOLD_MODE
> instend LARGEST_WIN_MODE (I mean bit 2 in AT_CTRL_R(0x540) register).
> 
> Xi, could you also test my fix on your board?

Thanks for figuring this out!

When I was upstreaming support, I noticed __sdhci_execute_tuning() in
T-Head's version of sdhci-of-dwcmshc.c seemed to duplicate what already
existed in drivers/mmc/host/sdhci.c. I had thought T-Head copied it
because it was a static function.

9cc811a342be ("mmc: sdhci: add __sdhci_execute_tuning() to header")
allowed me to remove __sdhci_execute_tuning() from sdhci-of-dwcmshc.
However, I overlooked this resulted in changing the tuning loop from
128 back to the upstream default of 40.

Before this change, the microSD did work for me on the lpi4 but I would
see the following:

[    4.182483] mmc1: Tuning failed, falling back to fixed sampling clock
[    4.189022] sdhci-dwcmshc ffe7090000.mmc: tuning failed: -11
[    4.194734] mmc1: tuning execution failed: -5
[    4.287899] mmc1: new high speed SDHC card at address aaaa
[    4.299763] mmcblk1: mmc1:aaaa SD32G 29.7 GiB
[    4.316963]  mmcblk1: p1 p2

root@lpi4amain:~# cat /sys/kernel/debug/mmc1/ios
clock:		50000000 Hz
actual clock:	49500000 Hz
vdd:		21 (3.3 ~ 3.4 V)
bus mode:	2 (push-pull)
chip select:	0 (don't care)
power mode:	2 (on)
bus width:	2 (4 bits)
timing spec:	2 (sd high-speed)
signal voltage:	0 (3.30 V)
driver type:	0 (driver type B)

With the change to 128, I no longer see the tuning failure and the
microSD continues to work okay:

[    4.307040] mmc1: new ultra high speed SDR104 SDHC card at address aaaa
[    4.320462] mmcblk1: mmc1:aaaa SD32G 29.7 GiB
[    4.338646]  mmcblk1: p1 p2

root@lpi4amain:/sys/kernel/debug/mmc1# cat ios
clock:		198000000 Hz
actual clock:	198000000 Hz
vdd:		21 (3.3 ~ 3.4 V)
bus mode:	2 (push-pull)
chip select:	0 (don't care)
power mode:	2 (on)
bus width:	2 (4 bits)
timing spec:	6 (sd uhs SDR104)
signal voltage:	1 (1.80 V)
driver type:	0 (driver type B)

This has the benefit of the card now works at 198 MHz in SDR104 mode
instead of 50 MHz when tuning failed.

Tested-by: Drew Fustini <drew@pdp7.com>

thanks,
drew

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

WARNING: multiple messages have this Message-ID (diff)
From: Drew Fustini <drew@pdp7.com>
To: Maxim Kiselev <bigunclemax@gmail.com>
Cc: xry111@xry111.site, Albert Ou <aou@eecs.berkeley.edu>,
	Conor Dooley <conor@kernel.org>,
	devicetree@vger.kernel.org, dfustini@baylibre.com,
	guoren@kernel.org, jkridner@beagleboard.org, jszhang@kernel.org,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	open list <linux-kernel@vger.kernel.org>,
	linux-riscv@lists.infradead.org,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	robertcnelson@beagleboard.org, Rob Herring <robh+dt@kernel.org>,
	wefu@redhat.com
Subject: Re: [PATCH v8 4/4] riscv: dts: thead: Enable LicheePi 4A eMMC and microSD
Date: Sat, 23 Mar 2024 18:25:00 -0700	[thread overview]
Message-ID: <Zf+A7KEYL/tZb9/N@x1> (raw)
In-Reply-To: <CALHCpMhc1F5Ue7U_gsDXREHUZRVQJNYRCJxYxoNqbN=-39jf7A@mail.gmail.com>

On Wed, Mar 20, 2024 at 03:28:19PM +0300, Maxim Kiselev wrote:
> Hi Xi, Drew
> 
> I have the same problem with SD on my LicheePi 4A.
> 
> After some investigations I found how to fix this tuning error.
> Here is the patch that increases tuning loop count from
> 40(MAX_TUNING_LOOP at sdhci.c) to 128.
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c
> b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 8d6cfb648096..da8f5820fb69 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -706,6 +706,7 @@ static int th1520_execute_tuning(struct sdhci_host
> *host, u32 opcode)
> 
>         /* perform tuning */
>         sdhci_start_tuning(host);
> +       host->tuning_loop_count = 128:
>         host->tuning_err = __sdhci_execute_tuning(host, opcode);
>         if (host->tuning_err) {
>                 /* disable auto-tuning upon tuning error */
> 
> After that change tuning works fine. The same value of loop count is
> used in RevyOS BSP
> https://github.com/revyos/thead-kernel/blob/c6d4e5df18a17903d012ffd89e67d0ee5ce6cf2d/drivers/mmc/host/sdhci-of-dwcmshc.c#L185
> 
> Honestly, it looks a little bit strange for me.
> 
> It seems that the tuning algorithm requires to move through
> all the taps of delay line(128 taps?) even if we use THRESHOLD_MODE
> instend LARGEST_WIN_MODE (I mean bit 2 in AT_CTRL_R(0x540) register).
> 
> Xi, could you also test my fix on your board?

Thanks for figuring this out!

When I was upstreaming support, I noticed __sdhci_execute_tuning() in
T-Head's version of sdhci-of-dwcmshc.c seemed to duplicate what already
existed in drivers/mmc/host/sdhci.c. I had thought T-Head copied it
because it was a static function.

9cc811a342be ("mmc: sdhci: add __sdhci_execute_tuning() to header")
allowed me to remove __sdhci_execute_tuning() from sdhci-of-dwcmshc.
However, I overlooked this resulted in changing the tuning loop from
128 back to the upstream default of 40.

Before this change, the microSD did work for me on the lpi4 but I would
see the following:

[    4.182483] mmc1: Tuning failed, falling back to fixed sampling clock
[    4.189022] sdhci-dwcmshc ffe7090000.mmc: tuning failed: -11
[    4.194734] mmc1: tuning execution failed: -5
[    4.287899] mmc1: new high speed SDHC card at address aaaa
[    4.299763] mmcblk1: mmc1:aaaa SD32G 29.7 GiB
[    4.316963]  mmcblk1: p1 p2

root@lpi4amain:~# cat /sys/kernel/debug/mmc1/ios
clock:		50000000 Hz
actual clock:	49500000 Hz
vdd:		21 (3.3 ~ 3.4 V)
bus mode:	2 (push-pull)
chip select:	0 (don't care)
power mode:	2 (on)
bus width:	2 (4 bits)
timing spec:	2 (sd high-speed)
signal voltage:	0 (3.30 V)
driver type:	0 (driver type B)

With the change to 128, I no longer see the tuning failure and the
microSD continues to work okay:

[    4.307040] mmc1: new ultra high speed SDR104 SDHC card at address aaaa
[    4.320462] mmcblk1: mmc1:aaaa SD32G 29.7 GiB
[    4.338646]  mmcblk1: p1 p2

root@lpi4amain:/sys/kernel/debug/mmc1# cat ios
clock:		198000000 Hz
actual clock:	198000000 Hz
vdd:		21 (3.3 ~ 3.4 V)
bus mode:	2 (push-pull)
chip select:	0 (don't care)
power mode:	2 (on)
bus width:	2 (4 bits)
timing spec:	6 (sd uhs SDR104)
signal voltage:	1 (1.80 V)
driver type:	0 (driver type B)

This has the benefit of the card now works at 198 MHz in SDR104 mode
instead of 50 MHz when tuning failed.

Tested-by: Drew Fustini <drew@pdp7.com>

thanks,
drew

  parent reply	other threads:[~2024-03-24  1:25 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-20 12:28 [PATCH v8 4/4] riscv: dts: thead: Enable LicheePi 4A eMMC and microSD Maxim Kiselev
2024-03-20 12:28 ` Maxim Kiselev
2024-03-20 12:52 ` Xi Ruoyao
2024-03-20 12:52   ` Xi Ruoyao
2024-03-24  1:25 ` Drew Fustini [this message]
2024-03-24  1:25   ` Drew Fustini
2024-03-24 14:38   ` Xi Ruoyao
2024-03-24 14:38     ` Xi Ruoyao
  -- strict thread matches above, loose matches on Subject: below --
2023-12-06  8:09 [PATCH v8 0/4] RISC-V: Add MMC support for TH1520 boards Drew Fustini
2023-12-06  8:09 ` [PATCH v8 4/4] riscv: dts: thead: Enable LicheePi 4A eMMC and microSD Drew Fustini
2023-12-06  8:09   ` Drew Fustini
2023-12-07  8:25   ` Guo Ren
2023-12-07  8:25     ` Guo Ren
2024-03-02 14:13   ` Xi Ruoyao
2024-03-02 14:13     ` Xi Ruoyao
2024-03-02 16:25     ` Drew Fustini
2024-03-02 16:25       ` Drew Fustini
2024-03-03  8:48       ` Xi Ruoyao
2024-03-03  8:48         ` Xi Ruoyao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zf+A7KEYL/tZb9/N@x1 \
    --to=drew@pdp7.com \
    --cc=aou@eecs.berkeley.edu \
    --cc=bigunclemax@gmail.com \
    --cc=conor@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dfustini@baylibre.com \
    --cc=guoren@kernel.org \
    --cc=jkridner@beagleboard.org \
    --cc=jszhang@kernel.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=robertcnelson@beagleboard.org \
    --cc=robh+dt@kernel.org \
    --cc=wefu@redhat.com \
    --cc=xry111@xry111.site \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.