diff mbox series

Disable HS-DDR mode for Olimex A64-OLinuXino variants with eMMC

Message ID 0d590f91-5d7a-697a-5644-710d14e3ea75@inventati.org (mailing list archive)
State New, archived
Headers show
Series Disable HS-DDR mode for Olimex A64-OLinuXino variants with eMMC | expand

Commit Message

Philip Rinn Oct. 21, 2020, 10:10 p.m. UTC
Hi,

the Olimex A64-OLinuXino board comes in various variants, three with eMMC.
While MMC HS-DDR mode works fine on one of them (A64-OLinuXino-1Ge4GW) it
doesn't work on the A64-OLinuXino-2Ge8G-IND variant (I don't have the
third variant so I can't check if it works there).

Disabling MMC HS-DDR mode fixes the problem. This is also what Olimex does
for their kernel:

https://github.com/OLIMEX/linux-olimex/commit/eef0e814e74f.patch

Mainline kernel doesn't differentiate between the different variants with
eMMC so I'm disabling HS-DDR mode for all of them as done for the H5 eMMC
controller.

Disable HS-DDR mode for Olimex A64-OLinuXino variants with eMMC

Signed-off-by: Philip Rinn <rinni@inventati.org>

---
drivers/mmc/host/sunxi-mmc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

--
2.28.0

Comments

Clément Péron Oct. 22, 2020, 7:52 a.m. UTC | #1
Hi Philipp,

On Thu, 22 Oct 2020 at 00:10, Philip Rinn <rinni@inventati.org> wrote:
>
> Hi,
>
> the Olimex A64-OLinuXino board comes in various variants, three with eMMC.
> While MMC HS-DDR mode works fine on one of them (A64-OLinuXino-1Ge4GW) it
> doesn't work on the A64-OLinuXino-2Ge8G-IND variant (I don't have the
> third variant so I can't check if it works there).

We got similar situations for Allwinner H6 (Beelink GS1 and Tanix TX6).

There is definitely an issue in the MMC sunxi driver with HS-DDR mode
and it's not limited to "olimex,a64-olinuxino-emmc".

Have a look at this this message posted by Jernej:
https://www.spinics.net/lists/arm-kernel/msg846093.html

I think we should disable the HS-DDR mode for A64 / H6 like it's done
for H5 and not disable board per board until the issue is resolved.
If you want to disable only for your board you can set in your device-tree:
mmc-hs200-1_8v;

This will switch to HS200 mode without using the HS-DDR mode.

Regards,
Clement

>
> Disabling MMC HS-DDR mode fixes the problem. This is also what Olimex does
> for their kernel:
>
> https://github.com/OLIMEX/linux-olimex/commit/eef0e814e74f.patch
>
> Mainline kernel doesn't differentiate between the different variants with
> eMMC so I'm disabling HS-DDR mode for all of them as done for the H5 eMMC
> controller.
>
> Disable HS-DDR mode for Olimex A64-OLinuXino variants with eMMC
>
> Signed-off-by: Philip Rinn <rinni@inventati.org>
>
> ---
> drivers/mmc/host/sunxi-mmc.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> index fc62773602ec..240d3803b8b9 100644
> --- a/drivers/mmc/host/sunxi-mmc.c
> +++ b/drivers/mmc/host/sunxi-mmc.c
> @@ -1397,7 +1397,7 @@ static int sunxi_mmc_probe(struct platform_device *pdev)
>                                   MMC_CAP_SDIO_IRQ;
>
>         /*
> -        * Some H5 devices do not have signal traces precise enough to
> +        * Some devices do not have signal traces precise enough to
>          * use HS DDR mode for their eMMC chips.
>          *
>          * We still enable HS DDR modes for all the other controller
> @@ -1405,7 +1405,8 @@ static int sunxi_mmc_probe(struct platform_device *pdev)
>          */
>         if ((host->cfg->clk_delays || host->use_new_timings) &&
>             !of_device_is_compatible(pdev->dev.of_node,
> -                                    "allwinner,sun50i-h5-emmc"))
> +                                    "allwinner,sun50i-h5-emmc") &&
> +                       !of_machine_is_compatible("olimex,a64-olinuxino-emmc"))
>                 mmc->caps      |= MMC_CAP_1_8V_DDR | MMC_CAP_3_3V_DDR;
>
>         ret = mmc_of_parse(mmc);
> --
> 2.28.0
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/0d590f91-5d7a-697a-5644-710d14e3ea75%40inventati.org.
Maxime Ripard Oct. 22, 2020, 9:09 a.m. UTC | #2
Hi,

On Thu, Oct 22, 2020 at 12:10:45AM +0200, Philip Rinn wrote:
> Hi,

The commit log is going to end up in the git history, so the hi should
be dropped :)

> the Olimex A64-OLinuXino board comes in various variants, three with eMMC.
> While MMC HS-DDR mode works fine on one of them (A64-OLinuXino-1Ge4GW) it
> doesn't work on the A64-OLinuXino-2Ge8G-IND variant (I don't have the
> third variant so I can't check if it works there).

Why is it not working? That flag is just setting what the controller is
capable of, if the eMMC wasn't supporting that mode it wouldn't even
try.

> Disabling MMC HS-DDR mode fixes the problem. This is also what Olimex does
> for their kernel:
> 
> https://github.com/OLIMEX/linux-olimex/commit/eef0e814e74f.patch

Again, there's no context there

Maxime
Maxime Ripard Oct. 22, 2020, 9:13 a.m. UTC | #3
Hi,

On Thu, Oct 22, 2020 at 09:52:29AM +0200, Clément Péron wrote:
> Hi Philipp,
> 
> On Thu, 22 Oct 2020 at 00:10, Philip Rinn <rinni@inventati.org> wrote:
> >
> > Hi,
> >
> > the Olimex A64-OLinuXino board comes in various variants, three with eMMC.
> > While MMC HS-DDR mode works fine on one of them (A64-OLinuXino-1Ge4GW) it
> > doesn't work on the A64-OLinuXino-2Ge8G-IND variant (I don't have the
> > third variant so I can't check if it works there).
> 
> We got similar situations for Allwinner H6 (Beelink GS1 and Tanix TX6).
> 
> There is definitely an issue in the MMC sunxi driver with HS-DDR mode
> and it's not limited to "olimex,a64-olinuxino-emmc".
> 
> Have a look at this this message posted by Jernej:
> https://www.spinics.net/lists/arm-kernel/msg846093.html
> 
> I think we should disable the HS-DDR mode for A64 / H6 like it's done
> for H5 and not disable board per board until the issue is resolved.

I'm not opposed to that on principle, but "it doesn't work on some
board" is not a proper justification to do so. It does on some others,
so surely there's more to it and this would just paper over whatever
issue we have instead of properly fixing it.

> If you want to disable only for your board you can set in your
> device-tree: mmc-hs200-1_8v; This will switch to HS200 mode without
> using the HS-DDR mode.

Sigh. Can we at least work on a fix instead of providing terrible
workarounds?

Maxime
Clément Péron Oct. 22, 2020, 3:39 p.m. UTC | #4
Hi Maxime,

On Thu, 22 Oct 2020 at 11:13, Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi,
>
> On Thu, Oct 22, 2020 at 09:52:29AM +0200, Clément Péron wrote:
> > Hi Philipp,
> >
> > On Thu, 22 Oct 2020 at 00:10, Philip Rinn <rinni@inventati.org> wrote:
> > >
> > > Hi,
> > >
> > > the Olimex A64-OLinuXino board comes in various variants, three with eMMC.
> > > While MMC HS-DDR mode works fine on one of them (A64-OLinuXino-1Ge4GW) it
> > > doesn't work on the A64-OLinuXino-2Ge8G-IND variant (I don't have the
> > > third variant so I can't check if it works there).
> >
> > We got similar situations for Allwinner H6 (Beelink GS1 and Tanix TX6).
> >
> > There is definitely an issue in the MMC sunxi driver with HS-DDR mode
> > and it's not limited to "olimex,a64-olinuxino-emmc".
> >
> > Have a look at this this message posted by Jernej:
> > https://www.spinics.net/lists/arm-kernel/msg846093.html
> >
> > I think we should disable the HS-DDR mode for A64 / H6 like it's done
> > for H5 and not disable board per board until the issue is resolved.
>
> I'm not opposed to that on principle, but "it doesn't work on some
> board" is not a proper justification to do so. It does on some others,
> so surely there's more to it and this would just paper over whatever
> issue we have instead of properly fixing it.

Maybe the term board is not correct, maybe it's the eMMC chip which is
different.
I don't know if all Beelink GS1/Tanix TX6 use the same eMMC chip for
all the boards.

>
> > If you want to disable only for your board you can set in your
> > device-tree: mmc-hs200-1_8v; This will switch to HS200 mode without
> > using the HS-DDR mode.
>
> Sigh. Can we at least work on a fix instead of providing terrible
> workarounds?

I agree with you on this, but my board doesn't have this issue and I
tried to read the vendor kernel and spot a difference but didn't find
anything relevant.

Regards,
Clement

>
> Maxime
Clément Péron Oct. 23, 2020, 8:45 a.m. UTC | #5
Hi Philip and Chen-Yu

(+Chen-Yu added as He also have the issue for H5)

On Thu, 22 Oct 2020 at 17:39, Clément Péron <peron.clem@gmail.com> wrote:
>
> Hi Maxime,
>
> On Thu, 22 Oct 2020 at 11:13, Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > Hi,
> >
> > On Thu, Oct 22, 2020 at 09:52:29AM +0200, Clément Péron wrote:
> > > Hi Philipp,
> > >
> > > On Thu, 22 Oct 2020 at 00:10, Philip Rinn <rinni@inventati.org> wrote:
> > > >
> > > > Hi,
> > > >
> > > > the Olimex A64-OLinuXino board comes in various variants, three with eMMC.
> > > > While MMC HS-DDR mode works fine on one of them (A64-OLinuXino-1Ge4GW) it
> > > > doesn't work on the A64-OLinuXino-2Ge8G-IND variant (I don't have the
> > > > third variant so I can't check if it works there).
> > >
> > > We got similar situations for Allwinner H6 (Beelink GS1 and Tanix TX6).
> > >
> > > There is definitely an issue in the MMC sunxi driver with HS-DDR mode
> > > and it's not limited to "olimex,a64-olinuxino-emmc".
> > >
> > > Have a look at this this message posted by Jernej:
> > > https://www.spinics.net/lists/arm-kernel/msg846093.html
> > >
> > > I think we should disable the HS-DDR mode for A64 / H6 like it's done
> > > for H5 and not disable board per board until the issue is resolved.
> >
> > I'm not opposed to that on principle, but "it doesn't work on some
> > board" is not a proper justification to do so. It does on some others,
> > so surely there's more to it and this would just paper over whatever
> > issue we have instead of properly fixing it.
>
> Maybe the term board is not correct, maybe it's the eMMC chip which is
> different.
> I don't know if all Beelink GS1/Tanix TX6 use the same eMMC chip for
> all the boards.
>
> >
> > > If you want to disable only for your board you can set in your
> > > device-tree: mmc-hs200-1_8v; This will switch to HS200 mode without
> > > using the HS-DDR mode.
> >
> > Sigh. Can we at least work on a fix instead of providing terrible
> > workarounds?
>
> I agree with you on this, but my board doesn't have this issue and I
> tried to read the vendor kernel and spot a difference but didn't find
> anything relevant.

After reviewing the correct file this time :P

I catched a slight difference between the mainline and vendor driver
in DDR mode.

the phase is set to 1
case MMC_TIMING_UHS_DDR50:
case MMC_TIMING_MMC_DDR52:
dat_drv_ph = 1;
speed_mod = SM2_HSDDR52_DDR50;
break;

Could you add a hack in DDR mode and see if it's working better ?

Maybe line 738:

#define SDXC_REG_DRV_DL (0x0140)
#define SDXC_DAT_DRV_PH_SEL BIT(17)

rval = mmc_readl(host, REG_DRV_DL);
rval |= SDXC_DAT_DRV_PH_SEL;
mmc_writel(host, REG_DRV_DL, rval);

Regards,
Clement

>
> Regards,
> Clement
>
> >
> > Maxime
Philip Rinn Oct. 23, 2020, 10:59 a.m. UTC | #6
Hi Clément,

Am 23.10.20 um 10:45 schrieb Clément Péron:
> Hi Philip and Chen-Yu
> 
> (+Chen-Yu added as He also have the issue for H5)
> 
> On Thu, 22 Oct 2020 at 17:39, Clément Péron <peron.clem@gmail.com> wrote:
>>
>> Hi Maxime,
>>
>> On Thu, 22 Oct 2020 at 11:13, Maxime Ripard <maxime@cerno.tech> wrote:
>>>
>>> Hi,
>>>
>>> On Thu, Oct 22, 2020 at 09:52:29AM +0200, Clément Péron wrote:
>>>> Hi Philipp,
>>>>
>>>> On Thu, 22 Oct 2020 at 00:10, Philip Rinn <rinni@inventati.org> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> the Olimex A64-OLinuXino board comes in various variants, three with eMMC.
>>>>> While MMC HS-DDR mode works fine on one of them (A64-OLinuXino-1Ge4GW) it
>>>>> doesn't work on the A64-OLinuXino-2Ge8G-IND variant (I don't have the
>>>>> third variant so I can't check if it works there).
>>>>
>>>> We got similar situations for Allwinner H6 (Beelink GS1 and Tanix TX6).
>>>>
>>>> There is definitely an issue in the MMC sunxi driver with HS-DDR mode
>>>> and it's not limited to "olimex,a64-olinuxino-emmc".
>>>>
>>>> Have a look at this this message posted by Jernej:
>>>> https://www.spinics.net/lists/arm-kernel/msg846093.html
>>>>
>>>> I think we should disable the HS-DDR mode for A64 / H6 like it's done
>>>> for H5 and not disable board per board until the issue is resolved.
>>>
>>> I'm not opposed to that on principle, but "it doesn't work on some
>>> board" is not a proper justification to do so. It does on some others,
>>> so surely there's more to it and this would just paper over whatever
>>> issue we have instead of properly fixing it.
>>
>> Maybe the term board is not correct, maybe it's the eMMC chip which is
>> different.
>> I don't know if all Beelink GS1/Tanix TX6 use the same eMMC chip for
>> all the boards.
>>
>>>
>>>> If you want to disable only for your board you can set in your
>>>> device-tree: mmc-hs200-1_8v; This will switch to HS200 mode without
>>>> using the HS-DDR mode.
>>>
>>> Sigh. Can we at least work on a fix instead of providing terrible
>>> workarounds?
>>
>> I agree with you on this, but my board doesn't have this issue and I
>> tried to read the vendor kernel and spot a difference but didn't find
>> anything relevant.
> 
> After reviewing the correct file this time :P
> 
> I catched a slight difference between the mainline and vendor driver
> in DDR mode.
> 
> the phase is set to 1
> case MMC_TIMING_UHS_DDR50:
> case MMC_TIMING_MMC_DDR52:
> dat_drv_ph = 1;
> speed_mod = SM2_HSDDR52_DDR50;
> break;
> 
> Could you add a hack in DDR mode and see if it's working better ?
> 
> Maybe line 738:
> 
> #define SDXC_REG_DRV_DL (0x0140)
> #define SDXC_DAT_DRV_PH_SEL BIT(17)
> 
> rval = mmc_readl(host, REG_DRV_DL);
> rval |= SDXC_DAT_DRV_PH_SEL;
> mmc_writel(host, REG_DRV_DL, rval);

I tried with this patch but no luck, see log below

--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -230,6 +230,8 @@

 #define SDXC_CAL_TIMEOUT	3	/* in seconds, 3s is enough*/

+#define SDXC_DAT_DRV_PH_SEL BIT(17)
+
 struct sunxi_mmc_clk_delay {
 	u32 output;
 	u32 sample;
@@ -718,6 +720,7 @@
 				   struct mmc_ios *ios, u32 rate)
 {
 	int index;
+	u32 rval;

 	/* clk controller delays not used under new timings mode */
 	if (host->use_new_timings)
@@ -736,6 +739,10 @@
 		if (ios->timing != MMC_TIMING_UHS_DDR50 &&
 		    ios->timing != MMC_TIMING_MMC_DDR52) {
 			index = SDXC_CLK_50M;
+
+			rval = mmc_readl(host, REG_DRV_DL);
+			rval |= SDXC_DAT_DRV_PH_SEL;
+			mmc_writel(host, REG_DRV_DL, rval);
 		} else if (ios->bus_width == MMC_BUS_WIDTH_8) {
 			index = SDXC_CLK_50M_DDR_8BIT;
 		} else {



log:

[...]
[    3.526269] sunxi-mmc 1c0f000.mmc: Error applying setting, reverse things back
[    3.526488] sun50i-a64-pinctrl 1c20800.pinctrl: Couldn't get bank PG regulator
[    3.526495] sun50i-a64-pinctrl 1c20800.pinctrl: request() failed for pin 192
[    3.526502] sun50i-a64-pinctrl 1c20800.pinctrl: pin-192 (1c10000.mmc) status -517
[    3.526509] sun50i-a64-pinctrl 1c20800.pinctrl: could not request pin 192 (PG0)
from group PG0  on device 1c20800.pinctrl
[    3.526514] sunxi-mmc 1c10000.mmc: Error applying setting, reverse things back
[    3.526707] sun50i-a64-pinctrl 1c20800.pinctrl: Couldn't get bank PC regulator
[    3.526713] sun50i-a64-pinctrl 1c20800.pinctrl: request() failed for pin 69
[    3.526720] sun50i-a64-pinctrl 1c20800.pinctrl: pin-69 (1c11000.mmc) status -517
[    3.526727] sun50i-a64-pinctrl 1c20800.pinctrl: could not request pin 69 (PC5)
from group PC5  on device 1c20800.pinctrl
[    3.526732] sunxi-mmc 1c11000.mmc: Error applying setting, reverse things back
[    3.571178] sun50i-a64-pinctrl 1c20800.pinctrl: Couldn't get bank PH regulator
[    3.571191] sun50i-a64-pinctrl 1c20800.pinctrl: request() failed for pin 233
[    3.571199] sun50i-a64-pinctrl 1c20800.pinctrl: pin-233 (1c20800.pinctrl:233)
status -517
[...]
[    3.899303] sunxi-mmc 1c11000.mmc: initialized, max. request size: 2048 KB,
uses new timings mode
[    3.984611] mmc2: new DDR MMC card at address 0001
[    3.986242] mmcblk2: mmc2:0001 Q2J55L 7.09 GiB
[    3.987147] mmcblk2boot0: mmc2:0001 Q2J55L partition 1 16.0 MiB
[    3.988071] mmcblk2boot1: mmc2:0001 Q2J55L partition 2 16.0 MiB
[    3.995437] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    3.995452] sunxi-mmc 1c11000.mmc: send stop command failed
[    3.995898] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    3.996262] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    3.996275] sunxi-mmc 1c11000.mmc: send stop command failed
[    3.996503] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    3.996827] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    3.997149] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    3.997162] sunxi-mmc 1c11000.mmc: send stop command failed
[    3.998239] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    3.998527] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    3.998733] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    3.998746] sunxi-mmc 1c11000.mmc: send stop command failed
[    3.999305] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    3.999323] sunxi-mmc 1c11000.mmc: send stop command failed
[    3.999545] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    3.999752] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    3.999765] sunxi-mmc 1c11000.mmc: send stop command failed
[    4.081911] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    4.133171] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    4.133184] sunxi-mmc 1c11000.mmc: send stop command failed
[    4.133252] blk_update_request: I/O error, dev mmcblk2, sector 0 op 0x0:(READ)
flags 0x0 phys_seg 1 prio class 0
[    4.133260] Buffer I/O error on dev mmcblk2, logical block 0, async page read
[    4.133384] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    4.133464] blk_update_request: I/O error, dev mmcblk2, sector 2 op 0x0:(READ)
flags 0x0 phys_seg 4 prio class 0
[    4.133470] Buffer I/O error on dev mmcblk2, logical block 2, async page read
[    4.133477] Buffer I/O error on dev mmcblk2, logical block 3, async page read
[    4.133483] Buffer I/O error on dev mmcblk2, logical block 4, async page read
[    4.133489] Buffer I/O error on dev mmcblk2, logical block 5, async page read
[    4.133762] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    4.133774] sunxi-mmc 1c11000.mmc: send stop command failed
[    4.134285] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    4.134297] sunxi-mmc 1c11000.mmc: send stop command failed
[    4.134519] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    4.134531] sunxi-mmc 1c11000.mmc: send stop command failed
[    4.134752] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    4.134764] sunxi-mmc 1c11000.mmc: send stop command failed
[    4.134976] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    4.134988] sunxi-mmc 1c11000.mmc: send stop command failed
[    4.186318] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    4.186330] sunxi-mmc 1c11000.mmc: send stop command failed
[    4.186384] blk_update_request: I/O error, dev mmcblk2, sector 0 op 0x0:(READ)
flags 0x0 phys_seg 1 prio class 0
[...]

Regards,
Philip

> Regards,
> Clement
> 
>>
>> Regards,
>> Clement
>>
>>>
>>> Maxime
Clément Péron Oct. 24, 2020, 1 p.m. UTC | #7
Hi Philip

<snip>

> >
> > I catched a slight difference between the mainline and vendor driver
> > in DDR mode.
> >
> > the phase is set to 1
> > case MMC_TIMING_UHS_DDR50:
> > case MMC_TIMING_MMC_DDR52:
> > dat_drv_ph = 1;
> > speed_mod = SM2_HSDDR52_DDR50;
> > break;
> >
> > Could you add a hack in DDR mode and see if it's working better ?
> >
> > Maybe line 738:
> >
> > #define SDXC_REG_DRV_DL (0x0140)
> > #define SDXC_DAT_DRV_PH_SEL BIT(17)
> >
> > rval = mmc_readl(host, REG_DRV_DL);
> > rval |= SDXC_DAT_DRV_PH_SEL;
> > mmc_writel(host, REG_DRV_DL, rval);
>
> I tried with this patch but no luck, see log below
>
> --- a/drivers/mmc/host/sunxi-mmc.c
> +++ b/drivers/mmc/host/sunxi-mmc.c
> @@ -230,6 +230,8 @@
>
>  #define SDXC_CAL_TIMEOUT       3       /* in seconds, 3s is enough*/
>
> +#define SDXC_DAT_DRV_PH_SEL BIT(17)
> +
>  struct sunxi_mmc_clk_delay {
>         u32 output;
>         u32 sample;
> @@ -718,6 +720,7 @@
>                                    struct mmc_ios *ios, u32 rate)
>  {
>         int index;
> +       u32 rval;
>
>         /* clk controller delays not used under new timings mode */
>         if (host->use_new_timings)
> @@ -736,6 +739,10 @@
>                 if (ios->timing != MMC_TIMING_UHS_DDR50 &&
>                     ios->timing != MMC_TIMING_MMC_DDR52) {
>                         index = SDXC_CLK_50M;
> +
> +                       rval = mmc_readl(host, REG_DRV_DL);
> +                       rval |= SDXC_DAT_DRV_PH_SEL;
> +                       mmc_writel(host, REG_DRV_DL, rval);
>                 } else if (ios->bus_width == MMC_BUS_WIDTH_8) {
>                         index = SDXC_CLK_50M_DDR_8BIT;
>                 } else {

Sorry this will not work, as H6 uses new timing mode the function will
return before setting the DRV_PHASE.

Could you retry with a patch like this :
https://github.com/clementperon/linux/commit/287a62a6e9ccda3ade9b407ce12ee7db0865b41b.patch

Thanks for your test,
Clement

<snip>


>
> Regards,
> Philip
>
> > Regards,
> > Clement
> >
> >>
> >> Regards,
> >> Clement
> >>
> >>>
> >>> Maxime
>
Philip Rinn Oct. 24, 2020, 9:54 p.m. UTC | #8
Hi Clément,

On 24.10.20 at 15:00, Clément Péron wrote:
> Hi Philip
> 
> <snip>
> 
>>>
>>> I catched a slight difference between the mainline and vendor driver
>>> in DDR mode.
>>>
>>> the phase is set to 1
>>> case MMC_TIMING_UHS_DDR50:
>>> case MMC_TIMING_MMC_DDR52:
>>> dat_drv_ph = 1;
>>> speed_mod = SM2_HSDDR52_DDR50;
>>> break;
>>>
>>> Could you add a hack in DDR mode and see if it's working better ?
>>>
>>> Maybe line 738:
>>>
>>> #define SDXC_REG_DRV_DL (0x0140)
>>> #define SDXC_DAT_DRV_PH_SEL BIT(17)
>>>
>>> rval = mmc_readl(host, REG_DRV_DL);
>>> rval |= SDXC_DAT_DRV_PH_SEL;
>>> mmc_writel(host, REG_DRV_DL, rval);
>>
>> I tried with this patch but no luck, see log below

<snip>

> Sorry this will not work, as H6 uses new timing mode the function will
> return before setting the DRV_PHASE.

I'm actually testing on an A64 - but that shouldn't matter, right?

> Could you retry with a patch like this :
> https://github.com/clementperon/linux/commit/287a62a6e9ccda3ade9b407ce12ee7db0865b41b.patch

Unfortunately that didn't work either:

[...]
[    3.755418] sunxi-mmc 1c0f000.mmc: initialized, max. request size: 16384 KB,
uses new timings mode
[    3.755432] sunxi-mmc 1c0f000.mmc: MMC version: 0.0.0
[    3.758685] sunxi-mmc 1c10000.mmc: allocated mmc-pwrseq
[    3.783369] sunxi-mmc 1c10000.mmc: initialized, max. request size: 16384 KB,
uses new timings mode
[    3.783382] sunxi-mmc 1c10000.mmc: MMC version: 0.0.0
[    3.798267] mmc0: new high speed SDHC card at address aaaa
[    3.799776] mmcblk0: mmc0:aaaa SC16G 14.8 GiB
[    3.804248]  mmcblk0: p1
[    3.811371] sunxi-mmc 1c11000.mmc: initialized, max. request size: 2048 KB,
uses new timings mode
[    3.811385] sunxi-mmc 1c11000.mmc: MMC version: 0.0.0
[...]
[    3.936574] sunxi-mmc 1c11000.mmc: Setting Phase delay
[    3.936890] mmc2: new DDR MMC card at address 0001
[    3.938617] mmcblk2: mmc2:0001 Q2J55L 7.09 GiB
[    3.939692] mmcblk2boot0: mmc2:0001 Q2J55L partition 1 16.0 MiB
[    3.940519] mmcblk2boot1: mmc2:0001 Q2J55L partition 2 16.0 MiB
[    3.943067] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    3.943083] sunxi-mmc 1c11000.mmc: send stop command failed
[...]
[    4.009680] blk_update_request: I/O error, dev mmcblk2, sector 0 op 0x0:(READ)
flags 0x0 phys_seg 1 prio class 0
[    4.009692] Buffer I/O error on dev mmcblk2, logical block 0, async page read
[    4.009817] sunxi-mmc 1c11000.mmc: data error, sending stop command
[    4.009899] blk_update_request: I/O error, dev mmcblk2, sector 2 op 0x0:(READ)
flags 0x0 phys_seg 6 prio class 0
[    4.009906] Buffer I/O error on dev mmcblk2, logical block 2, async page read
[    4.009914] Buffer I/O error on dev mmcblk2, logical block 3, async page read
[    4.009920] Buffer I/O error on dev mmcblk2, logical block 4, async page read
[    4.009926] Buffer I/O error on dev mmcblk2, logical block 5, async page read
[    4.009931] Buffer I/O error on dev mmcblk2, logical block 6, async page read
[    4.009937] Buffer I/O error on dev mmcblk2, logical block 7, async page read
[...]

> Thanks for your test,
> Clement

Thanks for trying to fix that :-)
Philip
diff mbox series

Patch

diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index fc62773602ec..240d3803b8b9 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -1397,7 +1397,7 @@  static int sunxi_mmc_probe(struct platform_device *pdev)
 				  MMC_CAP_SDIO_IRQ;

 	/*
-	 * Some H5 devices do not have signal traces precise enough to
+	 * Some devices do not have signal traces precise enough to
 	 * use HS DDR mode for their eMMC chips.
 	 *
 	 * We still enable HS DDR modes for all the other controller
@@ -1405,7 +1405,8 @@  static int sunxi_mmc_probe(struct platform_device *pdev)
 	 */
 	if ((host->cfg->clk_delays || host->use_new_timings) &&
 	    !of_device_is_compatible(pdev->dev.of_node,
-				     "allwinner,sun50i-h5-emmc"))
+				     "allwinner,sun50i-h5-emmc") &&
+			!of_machine_is_compatible("olimex,a64-olinuxino-emmc"))
 		mmc->caps      |= MMC_CAP_1_8V_DDR | MMC_CAP_3_3V_DDR;

 	ret = mmc_of_parse(mmc);