[v1,1/2] mmc: meson-mx-sdhc: Set MANUAL_STOP for multi-block SDIO commands

The vendor driver implements special handling for multi-block
SD_IO_RW_DIRECT and SD_IO_RW_EXTENDED commands. It sets the MANUAL_STOP
bit in the MESON_SDHC_MISC register for these commands. In All other
cases this bit is cleared.

This fixes SDIO wifi using the brcmfmac driver which reported the
following error without this change on a Netxeon S82 board using a
Meson8 (S802) SoC:
  brcmf_fw_alloc_request: using brcm/brcmfmac43362-sdio for chip
                          BCM43362/1
  brcmf_sdiod_ramrw: membytes transfer failed
  brcmf_sdio_download_code_file: error -110 on writing 219557 membytes
                                 at 0x00000000
  brcmf_sdio_download_firmware: dongle image file download failed

And with this change:
  brcmf_fw_alloc_request: using brcm/brcmfmac43362-sdio for chip
                          BCM43362/1
  brcmf_c_process_clm_blob: no clm_blob available (err=-2), device may
                            have limited channels available
  brcmf_c_preinit_dcmds: Firmware: BCM43362/1 wl0: Apr 22 2013 14:50:00
                         version 5.90.195.89.6 FWID 01-b30a427d

Fixes: e4bf1b0970ef96 ("mmc: host: meson-mx-sdhc: new driver for the Amlogic Meson SDHC host")
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/mmc/host/meson-mx-sdhc-mmc.c | 9 +++++++++
 1 file changed, 9 insertions(+)

On Sun, 12 Dec 2021 at 16:00, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> The vendor driver implements special handling for multi-block
> SD_IO_RW_DIRECT and SD_IO_RW_EXTENDED commands. It sets the MANUAL_STOP
> bit in the MESON_SDHC_MISC register for these commands. In All other
> cases this bit is cleared.
>
> This fixes SDIO wifi using the brcmfmac driver which reported the
> following error without this change on a Netxeon S82 board using a
> Meson8 (S802) SoC:
>   brcmf_fw_alloc_request: using brcm/brcmfmac43362-sdio for chip
>                           BCM43362/1
>   brcmf_sdiod_ramrw: membytes transfer failed
>   brcmf_sdio_download_code_file: error -110 on writing 219557 membytes
>                                  at 0x00000000
>   brcmf_sdio_download_firmware: dongle image file download failed
>
> And with this change:
>   brcmf_fw_alloc_request: using brcm/brcmfmac43362-sdio for chip
>                           BCM43362/1
>   brcmf_c_process_clm_blob: no clm_blob available (err=-2), device may
>                             have limited channels available
>   brcmf_c_preinit_dcmds: Firmware: BCM43362/1 wl0: Apr 22 2013 14:50:00
>                          version 5.90.195.89.6 FWID 01-b30a427d
>
> Fixes: e4bf1b0970ef96 ("mmc: host: meson-mx-sdhc: new driver for the Amlogic Meson SDHC host")
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  drivers/mmc/host/meson-mx-sdhc-mmc.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/mmc/host/meson-mx-sdhc-mmc.c b/drivers/mmc/host/meson-mx-sdhc-mmc.c
> index 7cd9c0ec2fcf..a89190d479cf 100644
> --- a/drivers/mmc/host/meson-mx-sdhc-mmc.c
> +++ b/drivers/mmc/host/meson-mx-sdhc-mmc.c
> @@ -135,6 +135,7 @@ static void meson_mx_sdhc_start_cmd(struct mmc_host *mmc,
>                                     struct mmc_command *cmd)
>  {
>         struct meson_mx_sdhc_host *host = mmc_priv(mmc);
> +       bool manual_stop = false;
>         u32 ictl, send;
>         int pack_len;
>
> @@ -172,12 +173,20 @@ static void meson_mx_sdhc_start_cmd(struct mmc_host *mmc,
>                 else
>                         /* software flush: */
>                         ictl |= MESON_SDHC_ICTL_DATA_XFER_OK;
> +

Maybe add a comment to explain a bit about this workaround here?

> +               manual_stop = cmd->data->blocks > 1 &&
> +                             (cmd->opcode == SD_IO_RW_DIRECT ||

SD_IO_RW_DIRECT doesn't have cmd->data, so checking for that command
doesn't make sense.

> +                              cmd->opcode == SD_IO_RW_EXTENDED);
>         } else {
>                 pack_len = 0;
>
>                 ictl |= MESON_SDHC_ICTL_RESP_OK;
>         }
>
> +       regmap_update_bits(host->regmap, MESON_SDHC_MISC,
> +                          MESON_SDHC_MISC_MANUAL_STOP,
> +                          manual_stop ? MESON_SDHC_MISC_MANUAL_STOP : 0);
> +
>         if (cmd->opcode == MMC_STOP_TRANSMISSION)
>                 send |= MESON_SDHC_SEND_DATA_STOP;
>
> --
> 2.34.1
>

Kind regards
Uffe

Hi Ulf,

On Tue, Dec 14, 2021 at 2:21 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
[...]
> > +
>
> Maybe add a comment to explain a bit about this workaround here?
sure, I'll add a paragraph for v2 because this workaround/fix is not obvious

> > +               manual_stop = cmd->data->blocks > 1 &&
> > +                             (cmd->opcode == SD_IO_RW_DIRECT ||
>
> SD_IO_RW_DIRECT doesn't have cmd->data, so checking for that command
> doesn't make sense.
This also means that you found a bug in the vendor driver :-)
I'll drop SD_IO_RW_DIRECT, do another round of testing and then send
an updated version.

Thank you for taking a closer look!


Best regards,
Martin