* [PATCH 0/4] Add a struct mci_cmd::busy_timeout member @ 2024-10-31 9:27 Sebastien Bourdelin 2024-10-31 9:27 ` [PATCH 1/4] mci: add a busy_timeout member to the struct mci_cmd Sebastien Bourdelin ` (3 more replies) 0 siblings, 4 replies; 11+ messages in thread From: Sebastien Bourdelin @ 2024-10-31 9:27 UTC (permalink / raw) To: barebox; +Cc: oss-contrib, sebastien.bourdelin, Sebastien Bourdelin From: Sebastien Bourdelin <sebastien.bourdelin@rtone.fr> This patch series introduce the busy_timeout member as part of the struct mci_cmd and fix the SDHCI timeout value for the BCM2711 SoC. It comes as a first step to [1] and allow sdhci drivers to define their timeout value. [1] https://github.com/barebox/barebox/blob/master/drivers/mci/mci-core.c#L2109 Sebastien Bourdelin (4): mci: add a busy_timeout member to the struct mci_cmd mci: zeroed all structs mci_cmd instances mci: sdhci: use the busy_timeout value in the sdhci_wait_idle functions mci: bcm2835: set timeout value to 100ms arch/arm/mach-socfpga/arria10-xload-emmc.c | 2 +- drivers/mci/dwcmshc-sdhci.c | 2 +- drivers/mci/imx-esdhc-pbl.c | 4 +-- drivers/mci/mci-bcm2835.c | 4 +++ drivers/mci/mci-core.c | 34 +++++++++++----------- drivers/mci/sdhci.c | 14 +++++++-- drivers/mci/sdhci.h | 2 ++ drivers/mci/stm32_sdmmc2.c | 2 +- include/mci.h | 1 + 9 files changed, 41 insertions(+), 24 deletions(-) -- 2.46.2 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/4] mci: add a busy_timeout member to the struct mci_cmd 2024-10-31 9:27 [PATCH 0/4] Add a struct mci_cmd::busy_timeout member Sebastien Bourdelin @ 2024-10-31 9:27 ` Sebastien Bourdelin 2024-11-06 13:57 ` Ahmad Fatoum 2024-10-31 9:27 ` [PATCH 2/4] mci: zeroed all structs mci_cmd instances Sebastien Bourdelin ` (2 subsequent siblings) 3 siblings, 1 reply; 11+ messages in thread From: Sebastien Bourdelin @ 2024-10-31 9:27 UTC (permalink / raw) To: barebox; +Cc: oss-contrib, sebastien.bourdelin, Sebastien Bourdelin From: Sebastien Bourdelin <sebastien.bourdelin@rtone.fr> This commit extends the struct mci_cmd to add a new busy_timeout member. Signed-off-by: Sebastien Bourdelin <sebastien.bourdelin@rtone.fr> --- include/mci.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/mci.h b/include/mci.h index cd01e2f992..f607ef1b74 100644 --- a/include/mci.h +++ b/include/mci.h @@ -471,6 +471,7 @@ struct mci_cmd { unsigned cmdidx; /**< Command to be sent to the SD/MMC card */ unsigned resp_type; /**< Type of expected response, refer MMC_RSP_* macros */ unsigned cmdarg; /**< Command's arguments */ + unsigned busy_timeout; /**< Busy timeout in ms */ unsigned response[4]; /**< card's response */ }; -- 2.46.2 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] mci: add a busy_timeout member to the struct mci_cmd 2024-10-31 9:27 ` [PATCH 1/4] mci: add a busy_timeout member to the struct mci_cmd Sebastien Bourdelin @ 2024-11-06 13:57 ` Ahmad Fatoum 0 siblings, 0 replies; 11+ messages in thread From: Ahmad Fatoum @ 2024-11-06 13:57 UTC (permalink / raw) To: Sebastien Bourdelin, barebox; +Cc: oss-contrib, Sebastien Bourdelin On 31.10.24 10:27, Sebastien Bourdelin wrote: > From: Sebastien Bourdelin <sebastien.bourdelin@rtone.fr> > > This commit extends the struct mci_cmd to add a new busy_timeout member. > > Signed-off-by: Sebastien Bourdelin <sebastien.bourdelin@rtone.fr> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > --- > include/mci.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/mci.h b/include/mci.h > index cd01e2f992..f607ef1b74 100644 > --- a/include/mci.h > +++ b/include/mci.h > @@ -471,6 +471,7 @@ struct mci_cmd { > unsigned cmdidx; /**< Command to be sent to the SD/MMC card */ > unsigned resp_type; /**< Type of expected response, refer MMC_RSP_* macros */ > unsigned cmdarg; /**< Command's arguments */ > + unsigned busy_timeout; /**< Busy timeout in ms */ > unsigned response[4]; /**< card's response */ > }; > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/4] mci: zeroed all structs mci_cmd instances 2024-10-31 9:27 [PATCH 0/4] Add a struct mci_cmd::busy_timeout member Sebastien Bourdelin 2024-10-31 9:27 ` [PATCH 1/4] mci: add a busy_timeout member to the struct mci_cmd Sebastien Bourdelin @ 2024-10-31 9:27 ` Sebastien Bourdelin 2024-11-06 13:57 ` Ahmad Fatoum 2024-10-31 9:27 ` [PATCH 3/4] mci: sdhci: use the busy_timeout value in the sdhci_wait_idle functions Sebastien Bourdelin 2024-10-31 9:27 ` [PATCH 4/4] mci: bcm2835: set timeout value to 100ms Sebastien Bourdelin 3 siblings, 1 reply; 11+ messages in thread From: Sebastien Bourdelin @ 2024-10-31 9:27 UTC (permalink / raw) To: barebox; +Cc: oss-contrib, sebastien.bourdelin, Sebastien Bourdelin From: Sebastien Bourdelin <sebastien.bourdelin@rtone.fr> All structs mci_cmd should be init to zero to avoid error while using the busy_timeout value. Signed-off-by: Sebastien Bourdelin <sebastien.bourdelin@rtone.fr> --- arch/arm/mach-socfpga/arria10-xload-emmc.c | 2 +- drivers/mci/dwcmshc-sdhci.c | 2 +- drivers/mci/imx-esdhc-pbl.c | 4 +-- drivers/mci/mci-core.c | 34 +++++++++++----------- drivers/mci/stm32_sdmmc2.c | 2 +- 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/arch/arm/mach-socfpga/arria10-xload-emmc.c b/arch/arm/mach-socfpga/arria10-xload-emmc.c index ed24faf9bf..61774c6174 100644 --- a/arch/arm/mach-socfpga/arria10-xload-emmc.c +++ b/arch/arm/mach-socfpga/arria10-xload-emmc.c @@ -178,7 +178,7 @@ static int dwmci_cmd(struct mci_cmd *cmd, struct mci_data *data) int arria10_read_blocks(void *dst, int blocknum, size_t len) { - struct mci_cmd cmd; + struct mci_cmd cmd = {}; struct mci_data data; int ret; int blocks; diff --git a/drivers/mci/dwcmshc-sdhci.c b/drivers/mci/dwcmshc-sdhci.c index 010d376421..a7306fdb44 100644 --- a/drivers/mci/dwcmshc-sdhci.c +++ b/drivers/mci/dwcmshc-sdhci.c @@ -52,7 +52,7 @@ static int do_abort_sequence(struct mci_host *mci, struct mci_cmd *current_cmd) { int ret = 0; struct dwcmshc_host *host = priv_from_mci_host(mci); - struct mci_cmd cmd; + struct mci_cmd cmd = {}; u64 start; mci_setup_cmd(&cmd, MMC_CMD_STOP_TRANSMISSION, 0, MMC_RSP_R1b); diff --git a/drivers/mci/imx-esdhc-pbl.c b/drivers/mci/imx-esdhc-pbl.c index 5b1d9a3cf4..0e4f96d2de 100644 --- a/drivers/mci/imx-esdhc-pbl.c +++ b/drivers/mci/imx-esdhc-pbl.c @@ -35,7 +35,7 @@ static u8 ext_csd[512] __aligned(64); static int esdhc_send_ext_csd(struct fsl_esdhc_host *host) { - struct mci_cmd cmd; + struct mci_cmd cmd = {}; struct mci_data data; cmd.cmdidx = MMC_CMD_SEND_EXT_CSD; @@ -67,7 +67,7 @@ static bool __maybe_unused esdhc_bootpart_active(struct fsl_esdhc_host *host) static int esdhc_read_blocks(struct fsl_esdhc_host *host, void *dst, size_t len) { - struct mci_cmd cmd; + struct mci_cmd cmd = {}; struct mci_data data; u32 val; int ret; diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c index 48a3df9ec9..a6c304c1bd 100644 --- a/drivers/mci/mci-core.c +++ b/drivers/mci/mci-core.c @@ -119,7 +119,7 @@ static void mci_setup_cmd(struct mci_cmd *p, unsigned cmd, unsigned arg, unsigne */ static int mci_set_dsr(struct mci *mci) { - struct mci_cmd cmd; + struct mci_cmd cmd = {}; mci_setup_cmd(&cmd, MMC_CMD_SET_DSR, (mci->host->dsr_val >> 16) | 0xffff, MMC_RSP_NONE); @@ -134,7 +134,7 @@ static int mci_set_dsr(struct mci *mci) */ static int mci_set_blocklen(struct mci *mci, unsigned len) { - struct mci_cmd cmd; + struct mci_cmd cmd = {}; if (mci->host->timing == MMC_TIMING_MMC_DDR52) return 0; @@ -148,7 +148,7 @@ static void *sector_buf; static int mci_send_status(struct mci *mci, unsigned int *status) { struct mci_host *host = mci->host; - struct mci_cmd cmd; + struct mci_cmd cmd = {}; int ret; /* @@ -172,7 +172,7 @@ static int mci_send_status(struct mci *mci, unsigned int *status) static int mci_app_sd_status(struct mci *mci, __be32 *ssr) { int err; - struct mci_cmd cmd; + struct mci_cmd cmd = {}; struct mci_data data; cmd.cmdidx = MMC_CMD_APP_CMD; @@ -272,7 +272,7 @@ static int mci_poll_until_ready(struct mci *mci, int timeout_ms) static int mci_block_write(struct mci *mci, const void *src, int blocknum, int blocks) { - struct mci_cmd cmd; + struct mci_cmd cmd = {}; struct mci_data data; unsigned mmccmd; int ret; @@ -374,7 +374,7 @@ static int mci_block_erase(struct mci *card, unsigned int from, static int mci_read_block(struct mci *mci, void *dst, int blocknum, int blocks) { - struct mci_cmd cmd; + struct mci_cmd cmd = {}; struct mci_data data; int ret; unsigned mmccmd; @@ -411,7 +411,7 @@ static int mci_read_block(struct mci *mci, void *dst, int blocknum, */ static int mci_go_idle(struct mci *mci) { - struct mci_cmd cmd; + struct mci_cmd cmd = {}; int err; udelay(1000); @@ -431,7 +431,7 @@ static int mci_go_idle(struct mci *mci) static int sdio_send_op_cond(struct mci *mci) { - struct mci_cmd cmd; + struct mci_cmd cmd = {}; mci_setup_cmd(&cmd, SD_IO_SEND_OP_COND, 0, MMC_RSP_SPI_R4 | MMC_RSP_R4 | MMC_CMD_BCR); @@ -446,7 +446,7 @@ static int sdio_send_op_cond(struct mci *mci) static int sd_send_op_cond(struct mci *mci) { struct mci_host *host = mci->host; - struct mci_cmd cmd; + struct mci_cmd cmd = {}; int timeout = 1000; int err; unsigned voltages; @@ -521,7 +521,7 @@ static int sd_send_op_cond(struct mci *mci) static int mmc_send_op_cond(struct mci *mci) { struct mci_host *host = mci->host; - struct mci_cmd cmd; + struct mci_cmd cmd = {}; int timeout = 1000; int err; @@ -566,7 +566,7 @@ static int mmc_send_op_cond(struct mci *mci) */ int mci_send_ext_csd(struct mci *mci, char *ext_csd) { - struct mci_cmd cmd; + struct mci_cmd cmd = {}; struct mci_data data; /* Get the Card Status Register */ @@ -595,7 +595,7 @@ int mci_send_ext_csd(struct mci *mci, char *ext_csd) int mci_switch(struct mci *mci, unsigned index, unsigned value) { unsigned int status; - struct mci_cmd cmd; + struct mci_cmd cmd = {}; int ret; mci_setup_cmd(&cmd, MMC_CMD_SWITCH, @@ -840,7 +840,7 @@ static int mmc_change_freq(struct mci *mci) static int sd_switch(struct mci *mci, unsigned mode, unsigned group, unsigned value, uint8_t *resp) { - struct mci_cmd cmd; + struct mci_cmd cmd = {}; struct mci_data data; unsigned arg; @@ -909,7 +909,7 @@ static int sd_read_ssr(struct mci *mci) */ static int sd_change_freq(struct mci *mci) { - struct mci_cmd cmd; + struct mci_cmd cmd = {}; struct mci_data data; struct mci_host *host = mci->host; uint32_t *switch_status = sector_buf; @@ -1387,7 +1387,7 @@ static char *mci_version_string(struct mci *mci) static int mci_startup_sd(struct mci *mci) { - struct mci_cmd cmd; + struct mci_cmd cmd = {}; int err; if (mci_caps(mci) & MMC_CAP_4_BIT_DATA) { @@ -1782,7 +1782,7 @@ static int mci_startup_mmc(struct mci *mci) static int mci_startup(struct mci *mci) { struct mci_host *host = mci->host; - struct mci_cmd cmd; + struct mci_cmd cmd = {}; int err; if (IS_ENABLED(CONFIG_MMC_SPI_CRC_ON) && mmc_host_is_spi(host)) { /* enable CRC check for spi */ @@ -1924,7 +1924,7 @@ static int mci_startup(struct mci *mci) static int sd_send_if_cond(struct mci *mci) { struct mci_host *host = mci->host; - struct mci_cmd cmd; + struct mci_cmd cmd = {}; int err; mci_setup_cmd(&cmd, SD_CMD_SEND_IF_COND, diff --git a/drivers/mci/stm32_sdmmc2.c b/drivers/mci/stm32_sdmmc2.c index 822416c457..64a7171d2c 100644 --- a/drivers/mci/stm32_sdmmc2.c +++ b/drivers/mci/stm32_sdmmc2.c @@ -512,7 +512,7 @@ static int stm32_sdmmc2_send_cmd(struct mci_host *mci, struct mci_cmd *cmd, * shall be send on cmd or data errors. */ if (ret && cmd->cmdidx != MMC_CMD_STOP_TRANSMISSION) { - struct mci_cmd stop_cmd; + struct mci_cmd stop_cmd = {}; stop_cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION; stop_cmd.cmdarg = 0; -- 2.46.2 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/4] mci: zeroed all structs mci_cmd instances 2024-10-31 9:27 ` [PATCH 2/4] mci: zeroed all structs mci_cmd instances Sebastien Bourdelin @ 2024-11-06 13:57 ` Ahmad Fatoum 0 siblings, 0 replies; 11+ messages in thread From: Ahmad Fatoum @ 2024-11-06 13:57 UTC (permalink / raw) To: Sebastien Bourdelin, barebox; +Cc: oss-contrib, Sebastien Bourdelin On 31.10.24 10:27, Sebastien Bourdelin wrote: > From: Sebastien Bourdelin <sebastien.bourdelin@rtone.fr> > > All structs mci_cmd should be init to zero to avoid error while using > the busy_timeout value. Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > > Signed-off-by: Sebastien Bourdelin <sebastien.bourdelin@rtone.fr> > --- > arch/arm/mach-socfpga/arria10-xload-emmc.c | 2 +- > drivers/mci/dwcmshc-sdhci.c | 2 +- > drivers/mci/imx-esdhc-pbl.c | 4 +-- > drivers/mci/mci-core.c | 34 +++++++++++----------- > drivers/mci/stm32_sdmmc2.c | 2 +- > 5 files changed, 22 insertions(+), 22 deletions(-) > > diff --git a/arch/arm/mach-socfpga/arria10-xload-emmc.c b/arch/arm/mach-socfpga/arria10-xload-emmc.c > index ed24faf9bf..61774c6174 100644 > --- a/arch/arm/mach-socfpga/arria10-xload-emmc.c > +++ b/arch/arm/mach-socfpga/arria10-xload-emmc.c > @@ -178,7 +178,7 @@ static int dwmci_cmd(struct mci_cmd *cmd, struct mci_data *data) > > int arria10_read_blocks(void *dst, int blocknum, size_t len) > { > - struct mci_cmd cmd; > + struct mci_cmd cmd = {}; > struct mci_data data; > int ret; > int blocks; > diff --git a/drivers/mci/dwcmshc-sdhci.c b/drivers/mci/dwcmshc-sdhci.c > index 010d376421..a7306fdb44 100644 > --- a/drivers/mci/dwcmshc-sdhci.c > +++ b/drivers/mci/dwcmshc-sdhci.c > @@ -52,7 +52,7 @@ static int do_abort_sequence(struct mci_host *mci, struct mci_cmd *current_cmd) > { > int ret = 0; > struct dwcmshc_host *host = priv_from_mci_host(mci); > - struct mci_cmd cmd; > + struct mci_cmd cmd = {}; > u64 start; > > mci_setup_cmd(&cmd, MMC_CMD_STOP_TRANSMISSION, 0, MMC_RSP_R1b); > diff --git a/drivers/mci/imx-esdhc-pbl.c b/drivers/mci/imx-esdhc-pbl.c > index 5b1d9a3cf4..0e4f96d2de 100644 > --- a/drivers/mci/imx-esdhc-pbl.c > +++ b/drivers/mci/imx-esdhc-pbl.c > @@ -35,7 +35,7 @@ static u8 ext_csd[512] __aligned(64); > > static int esdhc_send_ext_csd(struct fsl_esdhc_host *host) > { > - struct mci_cmd cmd; > + struct mci_cmd cmd = {}; > struct mci_data data; > > cmd.cmdidx = MMC_CMD_SEND_EXT_CSD; > @@ -67,7 +67,7 @@ static bool __maybe_unused esdhc_bootpart_active(struct fsl_esdhc_host *host) > > static int esdhc_read_blocks(struct fsl_esdhc_host *host, void *dst, size_t len) > { > - struct mci_cmd cmd; > + struct mci_cmd cmd = {}; > struct mci_data data; > u32 val; > int ret; > diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c > index 48a3df9ec9..a6c304c1bd 100644 > --- a/drivers/mci/mci-core.c > +++ b/drivers/mci/mci-core.c > @@ -119,7 +119,7 @@ static void mci_setup_cmd(struct mci_cmd *p, unsigned cmd, unsigned arg, unsigne > */ > static int mci_set_dsr(struct mci *mci) > { > - struct mci_cmd cmd; > + struct mci_cmd cmd = {}; > > mci_setup_cmd(&cmd, MMC_CMD_SET_DSR, > (mci->host->dsr_val >> 16) | 0xffff, MMC_RSP_NONE); > @@ -134,7 +134,7 @@ static int mci_set_dsr(struct mci *mci) > */ > static int mci_set_blocklen(struct mci *mci, unsigned len) > { > - struct mci_cmd cmd; > + struct mci_cmd cmd = {}; > > if (mci->host->timing == MMC_TIMING_MMC_DDR52) > return 0; > @@ -148,7 +148,7 @@ static void *sector_buf; > static int mci_send_status(struct mci *mci, unsigned int *status) > { > struct mci_host *host = mci->host; > - struct mci_cmd cmd; > + struct mci_cmd cmd = {}; > int ret; > > /* > @@ -172,7 +172,7 @@ static int mci_send_status(struct mci *mci, unsigned int *status) > static int mci_app_sd_status(struct mci *mci, __be32 *ssr) > { > int err; > - struct mci_cmd cmd; > + struct mci_cmd cmd = {}; > struct mci_data data; > > cmd.cmdidx = MMC_CMD_APP_CMD; > @@ -272,7 +272,7 @@ static int mci_poll_until_ready(struct mci *mci, int timeout_ms) > static int mci_block_write(struct mci *mci, const void *src, int blocknum, > int blocks) > { > - struct mci_cmd cmd; > + struct mci_cmd cmd = {}; > struct mci_data data; > unsigned mmccmd; > int ret; > @@ -374,7 +374,7 @@ static int mci_block_erase(struct mci *card, unsigned int from, > static int mci_read_block(struct mci *mci, void *dst, int blocknum, > int blocks) > { > - struct mci_cmd cmd; > + struct mci_cmd cmd = {}; > struct mci_data data; > int ret; > unsigned mmccmd; > @@ -411,7 +411,7 @@ static int mci_read_block(struct mci *mci, void *dst, int blocknum, > */ > static int mci_go_idle(struct mci *mci) > { > - struct mci_cmd cmd; > + struct mci_cmd cmd = {}; > int err; > > udelay(1000); > @@ -431,7 +431,7 @@ static int mci_go_idle(struct mci *mci) > > static int sdio_send_op_cond(struct mci *mci) > { > - struct mci_cmd cmd; > + struct mci_cmd cmd = {}; > > mci_setup_cmd(&cmd, SD_IO_SEND_OP_COND, 0, MMC_RSP_SPI_R4 | MMC_RSP_R4 | MMC_CMD_BCR); > > @@ -446,7 +446,7 @@ static int sdio_send_op_cond(struct mci *mci) > static int sd_send_op_cond(struct mci *mci) > { > struct mci_host *host = mci->host; > - struct mci_cmd cmd; > + struct mci_cmd cmd = {}; > int timeout = 1000; > int err; > unsigned voltages; > @@ -521,7 +521,7 @@ static int sd_send_op_cond(struct mci *mci) > static int mmc_send_op_cond(struct mci *mci) > { > struct mci_host *host = mci->host; > - struct mci_cmd cmd; > + struct mci_cmd cmd = {}; > int timeout = 1000; > int err; > > @@ -566,7 +566,7 @@ static int mmc_send_op_cond(struct mci *mci) > */ > int mci_send_ext_csd(struct mci *mci, char *ext_csd) > { > - struct mci_cmd cmd; > + struct mci_cmd cmd = {}; > struct mci_data data; > > /* Get the Card Status Register */ > @@ -595,7 +595,7 @@ int mci_send_ext_csd(struct mci *mci, char *ext_csd) > int mci_switch(struct mci *mci, unsigned index, unsigned value) > { > unsigned int status; > - struct mci_cmd cmd; > + struct mci_cmd cmd = {}; > int ret; > > mci_setup_cmd(&cmd, MMC_CMD_SWITCH, > @@ -840,7 +840,7 @@ static int mmc_change_freq(struct mci *mci) > static int sd_switch(struct mci *mci, unsigned mode, unsigned group, > unsigned value, uint8_t *resp) > { > - struct mci_cmd cmd; > + struct mci_cmd cmd = {}; > struct mci_data data; > unsigned arg; > > @@ -909,7 +909,7 @@ static int sd_read_ssr(struct mci *mci) > */ > static int sd_change_freq(struct mci *mci) > { > - struct mci_cmd cmd; > + struct mci_cmd cmd = {}; > struct mci_data data; > struct mci_host *host = mci->host; > uint32_t *switch_status = sector_buf; > @@ -1387,7 +1387,7 @@ static char *mci_version_string(struct mci *mci) > > static int mci_startup_sd(struct mci *mci) > { > - struct mci_cmd cmd; > + struct mci_cmd cmd = {}; > int err; > > if (mci_caps(mci) & MMC_CAP_4_BIT_DATA) { > @@ -1782,7 +1782,7 @@ static int mci_startup_mmc(struct mci *mci) > static int mci_startup(struct mci *mci) > { > struct mci_host *host = mci->host; > - struct mci_cmd cmd; > + struct mci_cmd cmd = {}; > int err; > > if (IS_ENABLED(CONFIG_MMC_SPI_CRC_ON) && mmc_host_is_spi(host)) { /* enable CRC check for spi */ > @@ -1924,7 +1924,7 @@ static int mci_startup(struct mci *mci) > static int sd_send_if_cond(struct mci *mci) > { > struct mci_host *host = mci->host; > - struct mci_cmd cmd; > + struct mci_cmd cmd = {}; > int err; > > mci_setup_cmd(&cmd, SD_CMD_SEND_IF_COND, > diff --git a/drivers/mci/stm32_sdmmc2.c b/drivers/mci/stm32_sdmmc2.c > index 822416c457..64a7171d2c 100644 > --- a/drivers/mci/stm32_sdmmc2.c > +++ b/drivers/mci/stm32_sdmmc2.c > @@ -512,7 +512,7 @@ static int stm32_sdmmc2_send_cmd(struct mci_host *mci, struct mci_cmd *cmd, > * shall be send on cmd or data errors. > */ > if (ret && cmd->cmdidx != MMC_CMD_STOP_TRANSMISSION) { > - struct mci_cmd stop_cmd; > + struct mci_cmd stop_cmd = {}; > > stop_cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION; > stop_cmd.cmdarg = 0; -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/4] mci: sdhci: use the busy_timeout value in the sdhci_wait_idle functions 2024-10-31 9:27 [PATCH 0/4] Add a struct mci_cmd::busy_timeout member Sebastien Bourdelin 2024-10-31 9:27 ` [PATCH 1/4] mci: add a busy_timeout member to the struct mci_cmd Sebastien Bourdelin 2024-10-31 9:27 ` [PATCH 2/4] mci: zeroed all structs mci_cmd instances Sebastien Bourdelin @ 2024-10-31 9:27 ` Sebastien Bourdelin 2024-11-06 14:00 ` Ahmad Fatoum 2024-10-31 9:27 ` [PATCH 4/4] mci: bcm2835: set timeout value to 100ms Sebastien Bourdelin 3 siblings, 1 reply; 11+ messages in thread From: Sebastien Bourdelin @ 2024-10-31 9:27 UTC (permalink / raw) To: barebox; +Cc: oss-contrib, sebastien.bourdelin, Sebastien Bourdelin From: Sebastien Bourdelin <sebastien.bourdelin@rtone.fr> The busy_timeout value is used by the sdhci_wait_idle and sdhci_wait_idle_data functions to define the timeout to wait on when send a command to the controller. The default value remains unchanged and is set to 10ms. Signed-off-by: Sebastien Bourdelin <sebastien.bourdelin@rtone.fr> --- drivers/mci/sdhci.c | 14 ++++++++++++-- drivers/mci/sdhci.h | 2 ++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/mci/sdhci.c b/drivers/mci/sdhci.c index fe033583ad..1425c72ef5 100644 --- a/drivers/mci/sdhci.c +++ b/drivers/mci/sdhci.c @@ -796,9 +796,11 @@ void sdhci_enable_clk(struct sdhci *host, u16 clk) int sdhci_wait_idle(struct sdhci *host, struct mci_cmd *cmd, struct mci_data *data) { u32 mask; + unsigned timeout_ms; int ret; mask = SDHCI_CMD_INHIBIT_CMD; + timeout_ms = SDHCI_CMD_DEFAULT_BUSY_TIMEOUT; if (data || (cmd && (cmd->resp_type & MMC_RSP_BUSY))) mask |= SDHCI_CMD_INHIBIT_DATA; @@ -806,7 +808,10 @@ int sdhci_wait_idle(struct sdhci *host, struct mci_cmd *cmd, struct mci_data *da if (cmd && cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION) mask &= ~SDHCI_CMD_INHIBIT_DATA; - ret = wait_on_timeout(10 * MSECOND, + if (cmd && cmd->busy_timeout != 0) + timeout_ms = cmd->busy_timeout; + + ret = wait_on_timeout(timeout_ms * MSECOND, !(sdhci_read32(host, SDHCI_PRESENT_STATE) & mask)); if (ret) { @@ -821,14 +826,19 @@ int sdhci_wait_idle(struct sdhci *host, struct mci_cmd *cmd, struct mci_data *da int sdhci_wait_idle_data(struct sdhci *host, struct mci_cmd *cmd) { u32 mask; + unsigned timeout_ms; int ret; mask = SDHCI_CMD_INHIBIT_CMD | SDHCI_CMD_INHIBIT_DATA; + timeout_ms = SDHCI_CMD_DEFAULT_BUSY_TIMEOUT; if (cmd && cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION) mask &= ~SDHCI_CMD_INHIBIT_DATA; - ret = wait_on_timeout(10 * MSECOND, + if (cmd && cmd->busy_timeout != 0) + timeout_ms = cmd->busy_timeout; + + ret = wait_on_timeout(timeout_ms * MSECOND, !(sdhci_read32(host, SDHCI_PRESENT_STATE) & mask)); if (ret) { diff --git a/drivers/mci/sdhci.h b/drivers/mci/sdhci.h index 5de85239b1..757840acc7 100644 --- a/drivers/mci/sdhci.h +++ b/drivers/mci/sdhci.h @@ -200,6 +200,8 @@ #define SDHCI_MAX_DIV_SPEC_200 256 #define SDHCI_MAX_DIV_SPEC_300 2046 +#define SDHCI_CMD_DEFAULT_BUSY_TIMEOUT 10 + struct sdhci { u32 (*read32)(struct sdhci *host, int reg); u16 (*read16)(struct sdhci *host, int reg); -- 2.46.2 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] mci: sdhci: use the busy_timeout value in the sdhci_wait_idle functions 2024-10-31 9:27 ` [PATCH 3/4] mci: sdhci: use the busy_timeout value in the sdhci_wait_idle functions Sebastien Bourdelin @ 2024-11-06 14:00 ` Ahmad Fatoum 0 siblings, 0 replies; 11+ messages in thread From: Ahmad Fatoum @ 2024-11-06 14:00 UTC (permalink / raw) To: Sebastien Bourdelin, barebox; +Cc: oss-contrib, Sebastien Bourdelin On 31.10.24 10:27, Sebastien Bourdelin wrote: > From: Sebastien Bourdelin <sebastien.bourdelin@rtone.fr> > > The busy_timeout value is used by the sdhci_wait_idle and > sdhci_wait_idle_data functions to define the timeout to wait on when > send a command to the controller. > > The default value remains unchanged and is set to 10ms. > > Signed-off-by: Sebastien Bourdelin <sebastien.bourdelin@rtone.fr> With below point addressed: Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > diff --git a/drivers/mci/sdhci.h b/drivers/mci/sdhci.h > index 5de85239b1..757840acc7 100644 > --- a/drivers/mci/sdhci.h > +++ b/drivers/mci/sdhci.h > @@ -200,6 +200,8 @@ > #define SDHCI_MAX_DIV_SPEC_200 256 > #define SDHCI_MAX_DIV_SPEC_300 2046 > > +#define SDHCI_CMD_DEFAULT_BUSY_TIMEOUT 10 Please rename to SDHCI_CMD_DEFAULT_BUSY_TIMEOUT_MS > + > struct sdhci { > u32 (*read32)(struct sdhci *host, int reg); > u16 (*read16)(struct sdhci *host, int reg); -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/4] mci: bcm2835: set timeout value to 100ms 2024-10-31 9:27 [PATCH 0/4] Add a struct mci_cmd::busy_timeout member Sebastien Bourdelin ` (2 preceding siblings ...) 2024-10-31 9:27 ` [PATCH 3/4] mci: sdhci: use the busy_timeout value in the sdhci_wait_idle functions Sebastien Bourdelin @ 2024-10-31 9:27 ` Sebastien Bourdelin 2024-11-04 12:27 ` Sascha Hauer 2024-11-06 14:01 ` Ahmad Fatoum 3 siblings, 2 replies; 11+ messages in thread From: Sebastien Bourdelin @ 2024-10-31 9:27 UTC (permalink / raw) To: barebox; +Cc: oss-contrib, sebastien.bourdelin, Sebastien Bourdelin From: Sebastien Bourdelin <sebastien.bourdelin@rtone.fr> As mentionned in the Raspberry Pi4 bootrom Changelog [1]: "Increase timeout of early SD/EMMC commands to 100ms". The BCM2711 SDHCI can take up to 100ms to complete a command. Without this change, we had noticed that Barebox sometime stay stucked on a timeout error while trying to boot Linux from the eMMC. [1] https://github.com/raspberrypi/rpi-eeprom/blob/master/firmware-2711/release-notes.md#2022-03-10---promote-the-2022-03-10-beta-release-to-lateststable Signed-off-by: Sebastien Bourdelin <sebastien.bourdelin@rtone.fr> --- drivers/mci/mci-bcm2835.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/mci/mci-bcm2835.c b/drivers/mci/mci-bcm2835.c index cbf99ee7ca..3b086ed22c 100644 --- a/drivers/mci/mci-bcm2835.c +++ b/drivers/mci/mci-bcm2835.c @@ -128,6 +128,10 @@ static int bcm2835_mci_request(struct mci_host *mci, struct mci_cmd *cmd, block_data |= data->blocksize; } + /* BCM2xxx SDHCI might take up to 100ms to complete a command */ + if (cmd != NULL) + cmd->busy_timeout = 100; + ret = sdhci_wait_idle_data(&host->sdhci, cmd); if (ret) return ret; -- 2.46.2 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/4] mci: bcm2835: set timeout value to 100ms 2024-10-31 9:27 ` [PATCH 4/4] mci: bcm2835: set timeout value to 100ms Sebastien Bourdelin @ 2024-11-04 12:27 ` Sascha Hauer 2024-11-06 12:57 ` Sebastien Bourdelin 2024-11-06 14:01 ` Ahmad Fatoum 1 sibling, 1 reply; 11+ messages in thread From: Sascha Hauer @ 2024-11-04 12:27 UTC (permalink / raw) To: Sebastien Bourdelin; +Cc: barebox, oss-contrib, Sebastien Bourdelin On Thu, Oct 31, 2024 at 10:27:21AM +0100, Sebastien Bourdelin wrote: > From: Sebastien Bourdelin <sebastien.bourdelin@rtone.fr> > > As mentionned in the Raspberry Pi4 bootrom Changelog [1]: > "Increase timeout of early SD/EMMC commands to 100ms". > The BCM2711 SDHCI can take up to 100ms to complete a command. > > Without this change, we had noticed that Barebox sometime stay stucked > on a timeout error while trying to boot Linux from the eMMC. > > [1] https://github.com/raspberrypi/rpi-eeprom/blob/master/firmware-2711/release-notes.md#2022-03-10---promote-the-2022-03-10-beta-release-to-lateststable > > Signed-off-by: Sebastien Bourdelin <sebastien.bourdelin@rtone.fr> > --- > drivers/mci/mci-bcm2835.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/mci/mci-bcm2835.c b/drivers/mci/mci-bcm2835.c > index cbf99ee7ca..3b086ed22c 100644 > --- a/drivers/mci/mci-bcm2835.c > +++ b/drivers/mci/mci-bcm2835.c > @@ -128,6 +128,10 @@ static int bcm2835_mci_request(struct mci_host *mci, struct mci_cmd *cmd, > block_data |= data->blocksize; > } > > + /* BCM2xxx SDHCI might take up to 100ms to complete a command */ > + if (cmd != NULL) > + cmd->busy_timeout = 100; Do we need this for all commands or just some specific ones? cmd is always non NULL, no need to check. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/4] mci: bcm2835: set timeout value to 100ms 2024-11-04 12:27 ` Sascha Hauer @ 2024-11-06 12:57 ` Sebastien Bourdelin 0 siblings, 0 replies; 11+ messages in thread From: Sebastien Bourdelin @ 2024-11-06 12:57 UTC (permalink / raw) To: Sascha Hauer; +Cc: barebox, oss-contrib, Sebastien Bourdelin On Mon Nov 4, 2024 at 1:27 PM CET, Sascha Hauer wrote: > On Thu, Oct 31, 2024 at 10:27:21AM +0100, Sebastien Bourdelin wrote: > > From: Sebastien Bourdelin <sebastien.bourdelin@rtone.fr> > > > > As mentionned in the Raspberry Pi4 bootrom Changelog [1]: > > "Increase timeout of early SD/EMMC commands to 100ms". > > The BCM2711 SDHCI can take up to 100ms to complete a command. > > > > Without this change, we had noticed that Barebox sometime stay stucked > > on a timeout error while trying to boot Linux from the eMMC. > > > > [1] https://github.com/raspberrypi/rpi-eeprom/blob/master/firmware-2711/release-notes.md#2022-03-10---promote-the-2022-03-10-beta-release-to-lateststable > > > > Signed-off-by: Sebastien Bourdelin <sebastien.bourdelin@rtone.fr> > > --- > > drivers/mci/mci-bcm2835.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/mci/mci-bcm2835.c b/drivers/mci/mci-bcm2835.c > > index cbf99ee7ca..3b086ed22c 100644 > > --- a/drivers/mci/mci-bcm2835.c > > +++ b/drivers/mci/mci-bcm2835.c > > @@ -128,6 +128,10 @@ static int bcm2835_mci_request(struct mci_host *mci, struct mci_cmd *cmd, > > block_data |= data->blocksize; > > } > > > > + /* BCM2xxx SDHCI might take up to 100ms to complete a command */ > > + if (cmd != NULL) > > + cmd->busy_timeout = 100; > > Do we need this for all commands or just some specific ones? I actually have no idea, the CM4s datasheet doesn't give much informations regarding the eMMC controller and we just found that information in the rpi-eeprom changelog, and saw it was also the defaut timeout value used in U-boot [1]. After this change, we haven't been able to reproduce the issue. We know for sure this issue happens at least on a read command. [1] https://github.com/u-boot/u-boot/blob/master/drivers/mmc/sdhci.c#L195 > > cmd is always non NULL, no need to check. Ok, i can send a v2 for that, just let me know if my explanation for your first question is enough. > > Sascha ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/4] mci: bcm2835: set timeout value to 100ms 2024-10-31 9:27 ` [PATCH 4/4] mci: bcm2835: set timeout value to 100ms Sebastien Bourdelin 2024-11-04 12:27 ` Sascha Hauer @ 2024-11-06 14:01 ` Ahmad Fatoum 1 sibling, 0 replies; 11+ messages in thread From: Ahmad Fatoum @ 2024-11-06 14:01 UTC (permalink / raw) To: Sebastien Bourdelin, barebox; +Cc: oss-contrib, Sebastien Bourdelin Hello Sebastien, Thanks for your fix. On 31.10.24 10:27, Sebastien Bourdelin wrote: > From: Sebastien Bourdelin <sebastien.bourdelin@rtone.fr> > > As mentionned in the Raspberry Pi4 bootrom Changelog [1]: > "Increase timeout of early SD/EMMC commands to 100ms". > The BCM2711 SDHCI can take up to 100ms to complete a command. > > Without this change, we had noticed that Barebox sometime stay stucked > on a timeout error while trying to boot Linux from the eMMC. > > [1] https://github.com/raspberrypi/rpi-eeprom/blob/master/firmware-2711/release-notes.md#2022-03-10---promote-the-2022-03-10-beta-release-to-lateststable > > Signed-off-by: Sebastien Bourdelin <sebastien.bourdelin@rtone.fr> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de> Cheers, Ahmad > --- > drivers/mci/mci-bcm2835.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/mci/mci-bcm2835.c b/drivers/mci/mci-bcm2835.c > index cbf99ee7ca..3b086ed22c 100644 > --- a/drivers/mci/mci-bcm2835.c > +++ b/drivers/mci/mci-bcm2835.c > @@ -128,6 +128,10 @@ static int bcm2835_mci_request(struct mci_host *mci, struct mci_cmd *cmd, > block_data |= data->blocksize; > } > > + /* BCM2xxx SDHCI might take up to 100ms to complete a command */ > + if (cmd != NULL) > + cmd->busy_timeout = 100; > + > ret = sdhci_wait_idle_data(&host->sdhci, cmd); > if (ret) > return ret; -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-11-06 14:14 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-10-31 9:27 [PATCH 0/4] Add a struct mci_cmd::busy_timeout member Sebastien Bourdelin 2024-10-31 9:27 ` [PATCH 1/4] mci: add a busy_timeout member to the struct mci_cmd Sebastien Bourdelin 2024-11-06 13:57 ` Ahmad Fatoum 2024-10-31 9:27 ` [PATCH 2/4] mci: zeroed all structs mci_cmd instances Sebastien Bourdelin 2024-11-06 13:57 ` Ahmad Fatoum 2024-10-31 9:27 ` [PATCH 3/4] mci: sdhci: use the busy_timeout value in the sdhci_wait_idle functions Sebastien Bourdelin 2024-11-06 14:00 ` Ahmad Fatoum 2024-10-31 9:27 ` [PATCH 4/4] mci: bcm2835: set timeout value to 100ms Sebastien Bourdelin 2024-11-04 12:27 ` Sascha Hauer 2024-11-06 12:57 ` Sebastien Bourdelin 2024-11-06 14:01 ` Ahmad Fatoum
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox