| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAPDyKFpxfn+kh8r39Fwoseh=k7rtwWYJr-A9FqMLK4OSiGBGZw@mail.gmail.com>
Date: Thu, 3 Jul 2025 13:29:26 +0200
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Benoît Monin <benoit.monin@...tlin.com>
Cc: Adrian Hunter <adrian.hunter@...el.com>, linux-mmc@...r.kernel.org,
linux-kernel@...r.kernel.org,
Vladimir Kondratiev <vladimir.kondratiev@...ileye.com>,
Tawfik Bayouk <tawfik.bayouk@...ileye.com>, Gregory CLEMENT <gregory.clement@...tlin.com>,
Théo Lebrun <theo.lebrun@...tlin.com>,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
Vladimir Kondratiev <vladimir.kondratiev@...el.com>
Subject: Re: [PATCH 2/2] mmc: sdhci-cadence: tune multi-block read gap
On Thu, 26 Jun 2025 at 16:44, Benoît Monin <benoit.monin@...tlin.com> wrote:
>
> From: Vladimir Kondratiev <vladimir.kondratiev@...el.com>
>
> Additional tuning required for multi-block read command.
> Implement it accordingly to Cadence recommendation.
>
> 2 registers used: HRS37 and HRS38. To HRS37, SD interface mode programmed,
> this selects HRS38 slot - there is separate slot for every SD interface
> mode. HRS38 contains gap parameter,
> it is selected by starting with gap=0 and sending multi-block read command.
> gap incremented until multi-block read succeeds.
>
> As of now, this tuning executed for HS200 only
>
> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@...el.com>
> Signed-off-by: Benoît Monin <benoit.monin@...tlin.com>
> ---
> drivers/mmc/host/sdhci-cadence.c | 142 ++++++++++++++++++++++++++++++-
> 1 file changed, 141 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
> index 27bd2eb29948..6dcf7ae0c99d 100644
> --- a/drivers/mmc/host/sdhci-cadence.c
> +++ b/drivers/mmc/host/sdhci-cadence.c
> @@ -36,6 +36,23 @@
> #define SDHCI_CDNS_HRS06_MODE_MMC_HS400 0x5
> #define SDHCI_CDNS_HRS06_MODE_MMC_HS400ES 0x6
>
> +/* Read block gap */
> +#define SDHCI_CDNS_HRS37 0x94 /* interface mode select */
> +#define SDHCI_CDNS_HRS37_MODE_DS 0x0
> +#define SDHCI_CDNS_HRS37_MODE_HS 0x1
> +#define SDHCI_CDNS_HRS37_MODE_UDS_SDR12 0x8
> +#define SDHCI_CDNS_HRS37_MODE_UDS_SDR25 0x9
> +#define SDHCI_CDNS_HRS37_MODE_UDS_SDR50 0xa
> +#define SDHCI_CDNS_HRS37_MODE_UDS_SDR104 0xb
> +#define SDHCI_CDNS_HRS37_MODE_UDS_DDR50 0xc
> +#define SDHCI_CDNS_HRS37_MODE_MMC_LEGACY 0x20
> +#define SDHCI_CDNS_HRS37_MODE_MMC_SDR 0x21
> +#define SDHCI_CDNS_HRS37_MODE_MMC_DDR 0x22
> +#define SDHCI_CDNS_HRS37_MODE_MMC_HS200 0x23
> +#define SDHCI_CDNS_HRS37_MODE_MMC_HS400 0x24
> +#define SDHCI_CDNS_HRS37_MODE_MMC_HS400ES 0x25
> +#define SDHCI_CDNS_HRS38 0x98 /* Read block gap coefficient */
> +#define SDHCI_CDNS_HRS38_BLKGAP_MAX 0xf
> /* SRS - Slot Register Set (SDHCI-compatible) */
> #define SDHCI_CDNS_SRS_BASE 0x200
>
> @@ -251,6 +268,123 @@ static int sdhci_cdns_set_tune_val(struct sdhci_host *host, unsigned int val)
> return 0;
> }
>
> +/**
> + * mmc_send_mb_read() - send multi-block read command
> + * @host: MMC host
> + *
> + * Sends multi-block read command, CMD23/CMD18/CMD12, ignore read data
> + *
> + * Return: error code
> + */
> +static int mmc_send_mb_read(struct mmc_host *host)
> +{
> + const int blksz = 512;
> + const int blocks = 32;
> + struct scatterlist sg;
> + struct mmc_command sbc = {
> + .opcode = MMC_SET_BLOCK_COUNT,
> + .arg = blocks,
> + .flags = MMC_RSP_R1 | MMC_CMD_AC,
> + };
> + struct mmc_command cmd = {
> + .opcode = MMC_READ_MULTIPLE_BLOCK,
> + .arg = 0,
> + .flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC,
> + };
> + struct mmc_command stop = {
> + .opcode = MMC_STOP_TRANSMISSION,
> + .arg = 0,
> + .flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC,
> + };
> + struct mmc_data data = {
> + .flags = MMC_DATA_READ,
> + .blksz = blksz,
> + .blocks = blocks,
> + .blk_addr = 0,
> + .timeout_ns = 800000000, /* 800ms */
> + .timeout_clks = 1000,
> + .sg = &sg,
> + .sg_len = 1,
> + };
> + struct mmc_request mrq = {
> + .sbc = &sbc,
> + .cmd = &cmd,
> + .data = &data,
> + .stop = &stop,
> + };
> + int size = blksz * blocks, err = 0;
> + u8 *data_buf;
> +
> + data_buf = kzalloc(size, GFP_KERNEL);
> + if (!data_buf)
> + return -ENOMEM;
> +
> + sg_init_one(&sg, data_buf, size);
> +
> + mmc_wait_for_req(host, &mrq);
> +
> + if (sbc.error) {
> + err = sbc.error;
> + goto out;
> + }
> +
> + if (cmd.error) {
> + err = cmd.error;
> + goto out;
> + }
> +
> + if (data.error) {
> + err = data.error;
> + goto out;
> + }
> +
> +out:
> + kfree(data_buf);
> + return err;
> +}
The above function does not belong in a host driver. It's the core's
responsibility to create and manage requests/commands.
That said, I wonder if we really need a multiple-block-read - or if we
can just read the ext-CSD from the eMMC, as it also allows us to
exercise the DATA lines?
If reading ext CSD is okay, we already have mmc_get_ext_csd() being
exported and available for host drivers to use. In fact mtk-sd is
already using it for the similar reason.
[...]
Kind regards
Uffe
Powered by blists - more mailing lists