| Message ID | 20190825150558.15173-1-alejandro.gonzalez.correo@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: sunxi: fix unusuable eMMC on some H6 boards by disabling DDR | expand |
On Sun, 25 Aug 2019 at 17:06, Alejandro González <alejandro.gonzalez.correo@gmail.com> wrote: > > Some Allwinner H6 boards have timing problems when dealing with > DDR-capable eMMC cards. These boards include the Pine H64 and Tanix TX6. > > These timing problems result in out of sync communication between the > driver and the eMMC, which renders the memory unsuable for every > operation but some basic commmands, like reading the status register. > > The cause of these timing problems is not yet well known, but they go > away by disabling DDR mode operation in the driver. Like on some H5 > boards, it might be that the traces are not precise enough to support > these speeds. However, Jernej Skrabec compared the BSP driver with this > driver, and found that the BSP driver configures pinctrl to operate at > 1.8 V when entering DDR mode (although 3.3 V operation is supported), while > the mainline kernel lacks any mechanism to switch voltages dynamically. > Finally, other possible cause might be some timing parameter that is > different on the H6 with respect to other SoCs. > > Therefore, as this fix works reliably, the kernel lacks the required > dynamic pinctrl control for now and a slow eMMC is better than a not > working eMMC, just disable DDR operation for now on H6-compatible > devices. > > Signed-off-by: Alejandro González <alejandro.gonzalez.correo@gmail.com> Assuming this should go stable as well? Perhaps you can find a relevant commit that we can put as a fixes tag as well? Kind regards Uffe > --- > drivers/mmc/host/sunxi-mmc.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c > index d577a6b0ceae..dac57d76d009 100644 > --- a/drivers/mmc/host/sunxi-mmc.c > +++ b/drivers/mmc/host/sunxi-mmc.c > @@ -1395,14 +1395,17 @@ static int sunxi_mmc_probe(struct platform_device *pdev) > > /* > * Some H5 devices do not have signal traces precise enough to > - * use HS DDR mode for their eMMC chips. > + * use HS DDR mode for their eMMC chips. Other H6 devices operate > + * unreliably on HS DDR mode, too. > * > * We still enable HS DDR modes for all the other controller > - * variants that support them. > + * variants that support them properly. > */ > 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_device_is_compatible(pdev->dev.of_node, > + "allwinner,sun50i-h6-emmc")) > mmc->caps |= MMC_CAP_1_8V_DDR | MMC_CAP_3_3V_DDR; > > ret = mmc_of_parse(mmc); > -- > 2.20.1 >
On Sun, Aug 25, 2019 at 05:05:58PM +0200, Alejandro González wrote: > Some Allwinner H6 boards have timing problems when dealing with > DDR-capable eMMC cards. These boards include the Pine H64 and Tanix TX6. > > These timing problems result in out of sync communication between the > driver and the eMMC, which renders the memory unsuable for every > operation but some basic commmands, like reading the status register. > > The cause of these timing problems is not yet well known, but they go > away by disabling DDR mode operation in the driver. Like on some H5 > boards, it might be that the traces are not precise enough to support > these speeds. However, Jernej Skrabec compared the BSP driver with this > driver, and found that the BSP driver configures pinctrl to operate at > 1.8 V when entering DDR mode (although 3.3 V operation is supported), while > the mainline kernel lacks any mechanism to switch voltages dynamically. > Finally, other possible cause might be some timing parameter that is > different on the H6 with respect to other SoCs. This should be a comment in the driver where this is disabled. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
El 27/8/19 a las 15:24, Ulf Hansson escribió:> Assuming this should go stable as well? Perhaps you can find a > relevant commit that we can put as a fixes tag as well? > > Kind regards > Uffe The most relevant commit I've found that is related to enabling DDR speeds on H6 boards is this one: https://github.com/torvalds/linux/commit/07bafc1e3536a4e3c422dbd13341688b54f159bb . But it doesn't address the H6 SoC specifically, so I doubted whether it would be appropiate to mark this patch as fixing it, and opted to not do it. I don't mind adding that tag if it's appropiate, though :-) On the other hand, I'm not sure that I understood correctly what do you mean by this patch going stable, but I might say the changes themselves are stable and work. The only downside I can think of to them is that they are a kind of workaround that reduces performance on H6 boards and/or eMMC not affected by this problem (are there any?), unless device trees are changed. El 27/8/19 a las 15:32, Maxime Ripard escribió: > On Sun, Aug 25, 2019 at 05:05:58PM +0200, Alejandro González wrote: >> Some Allwinner H6 boards have timing problems when dealing with >> DDR-capable eMMC cards. These boards include the Pine H64 and Tanix TX6. >> >> These timing problems result in out of sync communication between the >> driver and the eMMC, which renders the memory unsuable for every >> operation but some basic commmands, like reading the status register. >> >> The cause of these timing problems is not yet well known, but they go >> away by disabling DDR mode operation in the driver. Like on some H5 >> boards, it might be that the traces are not precise enough to support >> these speeds. However, Jernej Skrabec compared the BSP driver with this >> driver, and found that the BSP driver configures pinctrl to operate at >> 1.8 V when entering DDR mode (although 3.3 V operation is supported), while >> the mainline kernel lacks any mechanism to switch voltages dynamically. >> Finally, other possible cause might be some timing parameter that is >> different on the H6 with respect to other SoCs. > > This should be a comment in the driver where this is disabled. > > Maxime Thank you for your review. I'll mention that briefly in a comment in the code for the next revision of this patch. Regards.
On Sun, Aug 25, 2019 at 5:06 PM Alejandro González <alejandro.gonzalez.correo@gmail.com> wrote: > Jernej Skrabec compared the BSP driver with this > driver, and found that the BSP driver configures pinctrl to operate at > 1.8 V when entering DDR mode (although 3.3 V operation is supported), while > the mainline kernel lacks any mechanism to switch voltages dynamically. (...) > the kernel lacks the required > dynamic pinctrl control for now This is not a pin control thing, the I/O voltage level is usually controlled by a regulator called VCCQ, if the selection of the voltage rails is inside the pin control registers, see the solution in drivers/pinctrl/sh-pfc/pfc-sh73a0.c where we simply provide a regulator from inside the pinctrl driver to make things easy for the MMC core. Do this thing! If you don't have time to fix it up properly right now I would slap in a big FIXME in the code so people know this needs to be fixed properly. Yours, Linus Walleij
On Wed, Aug 28, 2019 at 8:52 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > On Sun, Aug 25, 2019 at 5:06 PM Alejandro González > <alejandro.gonzalez.correo@gmail.com> wrote: > > > Jernej Skrabec compared the BSP driver with this > > driver, and found that the BSP driver configures pinctrl to operate at > > 1.8 V when entering DDR mode (although 3.3 V operation is supported), while > > the mainline kernel lacks any mechanism to switch voltages dynamically. AFAIK The Pine H64 does not have the ability to switch I/O voltages. It is fixed to either 1.8V (the default based on the schematics) or 3.3V. > (...) > > the kernel lacks the required > > dynamic pinctrl control for now > > This is not a pin control thing, the I/O voltage level is usually > controlled by a regulator called VCCQ, if the selection of the > voltage rails is inside the pin control registers, see the solution > in drivers/pinctrl/sh-pfc/pfc-sh73a0.c where we simply provide > a regulator from inside the pinctrl driver to make things easy > for the MMC core. Do this thing! Or if it's simply voltage trimming for input, the a80 pinctrl driver has something similar. Basically it registers a notifier on the voltage rail supplying a set of pins, and toggles the register to match the external voltage provided. Unfortunately the user manual is quite vague on what it actually is. > If you don't have time to fix it up properly right now I would slap > in a big FIXME in the code so people know this needs > to be fixed properly. +1 ChenYu
On Wed, Aug 28, 2019 at 09:29:32PM +0800, Chen-Yu Tsai wrote: > On Wed, Aug 28, 2019 at 8:52 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > > > On Sun, Aug 25, 2019 at 5:06 PM Alejandro González > > <alejandro.gonzalez.correo@gmail.com> wrote: > > > > > Jernej Skrabec compared the BSP driver with this > > > driver, and found that the BSP driver configures pinctrl to operate at > > > 1.8 V when entering DDR mode (although 3.3 V operation is supported), while > > > the mainline kernel lacks any mechanism to switch voltages dynamically. > > AFAIK The Pine H64 does not have the ability to switch I/O voltages. It is > fixed to either 1.8V (the default based on the schematics) or 3.3V. Should that be handled at the board level then maybe? Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Wed, Aug 28, 2019 at 9:43 PM Maxime Ripard <mripard@kernel.org> wrote: > > On Wed, Aug 28, 2019 at 09:29:32PM +0800, Chen-Yu Tsai wrote: > > On Wed, Aug 28, 2019 at 8:52 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > > > > > On Sun, Aug 25, 2019 at 5:06 PM Alejandro González > > > <alejandro.gonzalez.correo@gmail.com> wrote: > > > > > > > Jernej Skrabec compared the BSP driver with this > > > > driver, and found that the BSP driver configures pinctrl to operate at > > > > 1.8 V when entering DDR mode (although 3.3 V operation is supported), while > > > > the mainline kernel lacks any mechanism to switch voltages dynamically. > > > > AFAIK The Pine H64 does not have the ability to switch I/O voltages. It is > > fixed to either 1.8V (the default based on the schematics) or 3.3V. > > Should that be handled at the board level then maybe? Yeah. You'd specify which one using vqmmc-supply in the mmc node and vcc-pc-supply in the pinctrl node. ChenYu
On Wed, 28 Aug 2019 at 12:52, Alejandro González <alejandro.gonzalez.correo@gmail.com> wrote: > > El 27/8/19 a las 15:24, Ulf Hansson escribió:> Assuming this should go stable as well? Perhaps you can find a > > relevant commit that we can put as a fixes tag as well? > > > > Kind regards > > Uffe > > The most relevant commit I've found that is related to enabling DDR speeds > on H6 boards is this one: https://github.com/torvalds/linux/commit/07bafc1e3536a4e3c422dbd13341688b54f159bb . > But it doesn't address the H6 SoC specifically, so I doubted whether it would > be appropiate to mark this patch as fixing it, and opted to not do it. I don't > mind adding that tag if it's appropiate, though :-) Hard to say what makes sense here, but how about picking this below instead? Fixes: 0a23f1ad88fc ("dt-binding: mmc: sunxi: add H6 compatible (with A64 fallback)") > > On the other hand, I'm not sure that I understood correctly what do you mean by > this patch going stable, but I might say the changes themselves are stable and work. > The only downside I can think of to them is that they are a kind of workaround that > reduces performance on H6 boards and/or eMMC not affected by this problem (are there > any?), unless device trees are changed. Adding a stable tag and a fixes tag for the commit, makes maintainers of stable kernels to try to backport this commit and fix the problem for "older" kernels. Kind regards Uffe
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c index d577a6b0ceae..dac57d76d009 100644 --- a/drivers/mmc/host/sunxi-mmc.c +++ b/drivers/mmc/host/sunxi-mmc.c @@ -1395,14 +1395,17 @@ static int sunxi_mmc_probe(struct platform_device *pdev) /* * Some H5 devices do not have signal traces precise enough to - * use HS DDR mode for their eMMC chips. + * use HS DDR mode for their eMMC chips. Other H6 devices operate + * unreliably on HS DDR mode, too. * * We still enable HS DDR modes for all the other controller - * variants that support them. + * variants that support them properly. */ 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_device_is_compatible(pdev->dev.of_node, + "allwinner,sun50i-h6-emmc")) mmc->caps |= MMC_CAP_1_8V_DDR | MMC_CAP_3_3V_DDR; ret = mmc_of_parse(mmc);
Some Allwinner H6 boards have timing problems when dealing with DDR-capable eMMC cards. These boards include the Pine H64 and Tanix TX6. These timing problems result in out of sync communication between the driver and the eMMC, which renders the memory unsuable for every operation but some basic commmands, like reading the status register. The cause of these timing problems is not yet well known, but they go away by disabling DDR mode operation in the driver. Like on some H5 boards, it might be that the traces are not precise enough to support these speeds. However, Jernej Skrabec compared the BSP driver with this driver, and found that the BSP driver configures pinctrl to operate at 1.8 V when entering DDR mode (although 3.3 V operation is supported), while the mainline kernel lacks any mechanism to switch voltages dynamically. Finally, other possible cause might be some timing parameter that is different on the H6 with respect to other SoCs. Therefore, as this fix works reliably, the kernel lacks the required dynamic pinctrl control for now and a slow eMMC is better than a not working eMMC, just disable DDR operation for now on H6-compatible devices. Signed-off-by: Alejandro González <alejandro.gonzalez.correo@gmail.com> --- drivers/mmc/host/sunxi-mmc.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)