Re: [PATCH v2 7/8] ARM: stm32mp: dk2: add barebox SD-Card update handler
On 10/28/19 12:31 PM, Sascha Hauer wrote: > On Mon, Oct 28, 2019 at 12:18:31AM +0100, Ahmad Fatoum wrote: >> Now with the SD/MMC controller supported, lets add a bbu handler, so we >> can use it to update the second stage boot loader partition. >> >> While doing this, I noticed that making use of the bus-width = <4> >> property in the device tree now makes mmc usage fail when reading the >> environment: >> >> ERROR: error SDMMC_STA_DCRCFAIL (0x81042) for cmd 18 >> ERROR: stm32_sdmmc2_end_cmd: error SDMMC_STA_CTIMEOUT (0x4) for cmd 12 >> WARNING: stm32_sdmmc2_send_cmd: cmd 12 failed, retrying ... >> ERROR: stm32_sdmmc2_end_cmd: error SDMMC_STA_CTIMEOUT (0x4) for cmd 12 >> WARNING: stm32_sdmmc2_send_cmd: cmd 12 failed, retrying ... >> ERROR: stm32_sdmmc2_end_cmd: error SDMMC_STA_CTIMEOUT (0x4) for cmd 12 >> WARNING: stm32_sdmmc2_send_cmd: cmd 12 failed, retrying ... >> ERROR: stm32_sdmmc2_end_cmd: error SDMMC_STA_CTIMEOUT (0x4) for cmd 12 > > Could you make this an extra patch? Can do. > >> >> We'll want to fix this eventually, but for now force the bus width to 1 >> and print a notice to the console that we've done so. This is not >> enough, however because it then fails at loading the kernel from MMC: >> >> ERROR: Time out on waiting for SDMMC_STA. cmd 18 >> ERROR: stm32_sdmmc2_end_cmd: error SDMMC_STA_CTIMEOUT (0x804) for cmd 12 >> WARNING: stm32_sdmmc2_send_cmd: cmd 18 failed, retrying ... >> >> Fixing the max frequency at 208 MHz fixes this. So do that and print a >> notice for it as well. >> >> @@ -627,11 +627,22 @@ static int stm32_sdmmc2_probe(struct amba_device *adev, >> if (IS_ERR(priv->reset_ctl)) >> priv->reset_ctl = NULL; >> >> +mci_of_parse(&priv->mci); >> + >> mci->f_min = 40; >> -/* f_max is taken from kernel v5.3 variant_stm32_sdmmc */ >> -mci->f_max = 20800; >> mci->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195; >> >> +if (mci->f_max != 20800) { >> +/* f_max is taken from kernel v5.3 variant_stm32_sdmmc */ >> +dev_notice(dev, "Fixing max-frequency to 208 MHz due to driver >> limitation\n"); >> +mci->f_max = 20800; >> +} > > f_max previously was hardcoded to 208MHz. It still is now, so this can't > fix anything. > Looking at the device tree we have max-frequency = <12000>, so you > probably mean that it doesn't work with the 120MHz specified here, > right? If it doesn't work with a lower frequency it really looks fishy. Exactly. > > Could you have another look what is going on here? I intend to, but not sure if it'll be before the cut-off for v2019.12.0. I'd like for that release to have full STM32MP support. I'll send a v2. > > Sascha > > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v2 7/8] ARM: stm32mp: dk2: add barebox SD-Card update handler
On Mon, Oct 28, 2019 at 12:18:31AM +0100, Ahmad Fatoum wrote: > Now with the SD/MMC controller supported, lets add a bbu handler, so we > can use it to update the second stage boot loader partition. > > While doing this, I noticed that making use of the bus-width = <4> > property in the device tree now makes mmc usage fail when reading the > environment: > > ERROR: error SDMMC_STA_DCRCFAIL (0x81042) for cmd 18 > ERROR: stm32_sdmmc2_end_cmd: error SDMMC_STA_CTIMEOUT (0x4) for cmd 12 > WARNING: stm32_sdmmc2_send_cmd: cmd 12 failed, retrying ... > ERROR: stm32_sdmmc2_end_cmd: error SDMMC_STA_CTIMEOUT (0x4) for cmd 12 > WARNING: stm32_sdmmc2_send_cmd: cmd 12 failed, retrying ... > ERROR: stm32_sdmmc2_end_cmd: error SDMMC_STA_CTIMEOUT (0x4) for cmd 12 > WARNING: stm32_sdmmc2_send_cmd: cmd 12 failed, retrying ... > ERROR: stm32_sdmmc2_end_cmd: error SDMMC_STA_CTIMEOUT (0x4) for cmd 12 Could you make this an extra patch? > > We'll want to fix this eventually, but for now force the bus width to 1 > and print a notice to the console that we've done so. This is not > enough, however because it then fails at loading the kernel from MMC: > > ERROR: Time out on waiting for SDMMC_STA. cmd 18 > ERROR: stm32_sdmmc2_end_cmd: error SDMMC_STA_CTIMEOUT (0x804) for cmd 12 > WARNING: stm32_sdmmc2_send_cmd: cmd 18 failed, retrying ... > > Fixing the max frequency at 208 MHz fixes this. So do that and print a > notice for it as well. > > @@ -627,11 +627,22 @@ static int stm32_sdmmc2_probe(struct amba_device *adev, > if (IS_ERR(priv->reset_ctl)) > priv->reset_ctl = NULL; > > + mci_of_parse(&priv->mci); > + > mci->f_min = 40; > - /* f_max is taken from kernel v5.3 variant_stm32_sdmmc */ > - mci->f_max = 20800; > mci->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195; > > + if (mci->f_max != 20800) { > + /* f_max is taken from kernel v5.3 variant_stm32_sdmmc */ > + dev_notice(dev, "Fixing max-frequency to 208 MHz due to driver > limitation\n"); > + mci->f_max = 20800; > + } f_max previously was hardcoded to 208MHz. It still is now, so this can't fix anything. Looking at the device tree we have max-frequency = <12000>, so you probably mean that it doesn't work with the 120MHz specified here, right? If it doesn't work with a lower frequency it really looks fishy. Could you have another look what is going on here? Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH v2 7/8] ARM: stm32mp: dk2: add barebox SD-Card update handler
Now with the SD/MMC controller supported, lets add a bbu handler, so we can use it to update the second stage boot loader partition. While doing this, I noticed that making use of the bus-width = <4> property in the device tree now makes mmc usage fail when reading the environment: ERROR: error SDMMC_STA_DCRCFAIL (0x81042) for cmd 18 ERROR: stm32_sdmmc2_end_cmd: error SDMMC_STA_CTIMEOUT (0x4) for cmd 12 WARNING: stm32_sdmmc2_send_cmd: cmd 12 failed, retrying ... ERROR: stm32_sdmmc2_end_cmd: error SDMMC_STA_CTIMEOUT (0x4) for cmd 12 WARNING: stm32_sdmmc2_send_cmd: cmd 12 failed, retrying ... ERROR: stm32_sdmmc2_end_cmd: error SDMMC_STA_CTIMEOUT (0x4) for cmd 12 WARNING: stm32_sdmmc2_send_cmd: cmd 12 failed, retrying ... ERROR: stm32_sdmmc2_end_cmd: error SDMMC_STA_CTIMEOUT (0x4) for cmd 12 We'll want to fix this eventually, but for now force the bus width to 1 and print a notice to the console that we've done so. This is not enough, however because it then fails at loading the kernel from MMC: ERROR: Time out on waiting for SDMMC_STA. cmd 18 ERROR: stm32_sdmmc2_end_cmd: error SDMMC_STA_CTIMEOUT (0x804) for cmd 12 WARNING: stm32_sdmmc2_send_cmd: cmd 18 failed, retrying ... Fixing the max frequency at 208 MHz fixes this. So do that and print a notice for it as well. Signed-off-by: Ahmad Fatoum --- arch/arm/boards/stm32mp157c-dk2/board.c | 14 ++ arch/arm/dts/stm32mp157a-dk1.dtsi| 4 arch/arm/mach-stm32mp/include/mach/bbu.h | 14 ++ drivers/mci/stm32_sdmmc2.c | 15 +-- 4 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 arch/arm/mach-stm32mp/include/mach/bbu.h diff --git a/arch/arm/boards/stm32mp157c-dk2/board.c b/arch/arm/boards/stm32mp157c-dk2/board.c index 9cb861af85d8..5bc88781c7ba 100644 --- a/arch/arm/boards/stm32mp157c-dk2/board.c +++ b/arch/arm/boards/stm32mp157c-dk2/board.c @@ -4,6 +4,7 @@ #include #include #include +#include static int dk2_mem_init(void) { @@ -15,3 +16,16 @@ static int dk2_mem_init(void) return 0; } mem_initcall(dk2_mem_init); + +static int dk2_postcore_init(void) +{ + if (!of_machine_is_compatible("st,stm32mp157c-dk2")) + return 0; + + stm32mp_bbu_mmc_register_handler("sd", "/dev/mmc0.ssbl", +BBU_HANDLER_FLAG_DEFAULT); + + return 0; +} + +postcore_initcall(dk2_postcore_init); diff --git a/arch/arm/dts/stm32mp157a-dk1.dtsi b/arch/arm/dts/stm32mp157a-dk1.dtsi index 7f3b6fcf55ae..05a39d32e2fa 100644 --- a/arch/arm/dts/stm32mp157a-dk1.dtsi +++ b/arch/arm/dts/stm32mp157a-dk1.dtsi @@ -8,6 +8,10 @@ #include / { + aliases { + mmc0 = &sdmmc1; + }; + chosen { environment { compatible = "barebox,environment"; diff --git a/arch/arm/mach-stm32mp/include/mach/bbu.h b/arch/arm/mach-stm32mp/include/mach/bbu.h new file mode 100644 index ..8b9504400e9e --- /dev/null +++ b/arch/arm/mach-stm32mp/include/mach/bbu.h @@ -0,0 +1,14 @@ +#ifndef MACH_STM32MP_BBU_H_ +#define MACH_STM32MP_BBU_H_ + +#include + +static inline int stm32mp_bbu_mmc_register_handler(const char *name, + const char *devicefile, + unsigned long flags) +{ + return bbu_register_std_file_update(name, flags, devicefile, + filetype_stm32_image_v1); +} + +#endif /* MACH_STM32MP_BBU_H_ */ diff --git a/drivers/mci/stm32_sdmmc2.c b/drivers/mci/stm32_sdmmc2.c index 7346c8a3f5d6..3f0fff1258c0 100644 --- a/drivers/mci/stm32_sdmmc2.c +++ b/drivers/mci/stm32_sdmmc2.c @@ -627,11 +627,22 @@ static int stm32_sdmmc2_probe(struct amba_device *adev, if (IS_ERR(priv->reset_ctl)) priv->reset_ctl = NULL; + mci_of_parse(&priv->mci); + mci->f_min = 40; - /* f_max is taken from kernel v5.3 variant_stm32_sdmmc */ - mci->f_max = 20800; mci->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195; + if (mci->f_max != 20800) { + /* f_max is taken from kernel v5.3 variant_stm32_sdmmc */ + dev_notice(dev, "Fixing max-frequency to 208 MHz due to driver limitation\n"); + mci->f_max = 20800; + } + + if (mci->host_caps & (MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA)) { + dev_notice(dev, "Fixing bus-width to 1 due to driver limitation\n"); + mci->host_caps &= ~(MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA); + } + return mci_register(&priv->mci); priv_free: -- 2.23.0 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox