[alsa-devel] [PATCH v2] ASoC: RT5677-SPI: Disable 16Bit SPI Transfers

[Ben Zhang]

On Fri, May 3, 2019 at 3:32 PM Curtis Malainey
<cujomalainey at chromium.org> wrote:
>
> The current algorithm allows 3 types of transfers, 16bit, 32bit and
> burst. According to Realtek, 16bit transfers have a special restriction
> in that it is restricted to the memory region of
> 0x18020000 ~ 0x18021000. This region is the memory location of the I2C
> registers. The current algorithm does not uphold this restriction and
> therefore fails to complete writes.
>
> Since this has been broken for some time it likely no one is using it.
> Better to simply disable the 16 bit writes. This will allow users to
> properly load firmware over SPI without data corruption.
>
> CC: Ben Zhang <benzh at chromium.org>
> CC: Oder Chiou <oder_chiou at realtek.com>
> Signed-off-by: Curtis Malainey <cujomalainey at chromium.org>

Reviewed-by: Ben Zhang <benzh at chromium.org>
Looks good to me. Thanks for the fix.

> ---
>  sound/soc/codecs/rt5677-spi.c | 35 ++++++++++++++++-------------------
>  1 file changed, 16 insertions(+), 19 deletions(-)
>
> diff --git a/sound/soc/codecs/rt5677-spi.c b/sound/soc/codecs/rt5677-spi.c
> index 167a02773a0b..84b6bd8b50e1 100644
> --- a/sound/soc/codecs/rt5677-spi.c
> +++ b/sound/soc/codecs/rt5677-spi.c
> @@ -58,13 +58,15 @@ static DEFINE_MUTEX(spi_mutex);
>   * RT5677_SPI_READ/WRITE_32:   Transfer 4 bytes
>   * RT5677_SPI_READ/WRITE_BURST:        Transfer any multiples of 8 bytes
>   *
> - * For example, reading 260 bytes at 0x60030002 uses the following commands:
> - * 0x60030002 RT5677_SPI_READ_16       2 bytes
> + * Note:
> + * 16 Bit writes and reads are restricted to the address range
> + * 0x18020000 ~ 0x18021000
> + *
> + * For example, reading 256 bytes at 0x60030004 uses the following commands:
>   * 0x60030004 RT5677_SPI_READ_32       4 bytes
>   * 0x60030008 RT5677_SPI_READ_BURST    240 bytes
>   * 0x600300F8 RT5677_SPI_READ_BURST    8 bytes
>   * 0x60030100 RT5677_SPI_READ_32       4 bytes
> - * 0x60030104 RT5677_SPI_READ_16       2 bytes
>   *
>   * Input:
>   * @read: true for read commands; false for write commands
> @@ -79,15 +81,13 @@ static u8 rt5677_spi_select_cmd(bool read, u32 align, u32 remain, u32 *len)
>  {
>         u8 cmd;
>
> -       if (align == 2 || align == 6 || remain == 2) {
> -               cmd = RT5677_SPI_READ_16;
> -               *len = 2;
> -       } else if (align == 4 || remain <= 6) {
> +       if (align == 4 || remain <= 4) {
>                 cmd = RT5677_SPI_READ_32;
>                 *len = 4;
>         } else {
>                 cmd = RT5677_SPI_READ_BURST;
> -               *len = min_t(u32, remain & ~7, RT5677_SPI_BURST_LEN);
> +               *len = (((remain - 1) >> 3) + 1) << 3;
> +               *len = min_t(u32, *len, RT5677_SPI_BURST_LEN);
>         }
>         return read ? cmd : cmd + 1;
>  }
> @@ -108,7 +108,7 @@ static void rt5677_spi_reverse(u8 *dst, u32 dstlen, const u8 *src, u32 srclen)
>         }
>  }
>
> -/* Read DSP address space using SPI. addr and len have to be 2-byte aligned. */
> +/* Read DSP address space using SPI. addr and len have to be 4-byte aligned. */
>  int rt5677_spi_read(u32 addr, void *rxbuf, size_t len)
>  {
>         u32 offset;
> @@ -124,7 +124,7 @@ int rt5677_spi_read(u32 addr, void *rxbuf, size_t len)
>         if (!g_spi)
>                 return -ENODEV;
>
> -       if ((addr & 1) || (len & 1)) {
> +       if ((addr & 3) || (len & 3)) {
>                 dev_err(&g_spi->dev, "Bad read align 0x%x(%zu)\n", addr, len);
>                 return -EACCES;
>         }
> @@ -159,13 +159,13 @@ int rt5677_spi_read(u32 addr, void *rxbuf, size_t len)
>  }
>  EXPORT_SYMBOL_GPL(rt5677_spi_read);
>
> -/* Write DSP address space using SPI. addr has to be 2-byte aligned.
> - * If len is not 2-byte aligned, an extra byte of zero is written at the end
> +/* Write DSP address space using SPI. addr has to be 4-byte aligned.
> + * If len is not 4-byte aligned, then extra zeros are written at the end
>   * as padding.
>   */
>  int rt5677_spi_write(u32 addr, const void *txbuf, size_t len)
>  {
> -       u32 offset, len_with_pad = len;
> +       u32 offset;
>         int status = 0;
>         struct spi_transfer t;
>         struct spi_message m;
> @@ -178,22 +178,19 @@ int rt5677_spi_write(u32 addr, const void *txbuf, size_t len)
>         if (!g_spi)
>                 return -ENODEV;
>
> -       if (addr & 1) {
> +       if (addr & 3) {
>                 dev_err(&g_spi->dev, "Bad write align 0x%x(%zu)\n", addr, len);
>                 return -EACCES;
>         }
>
> -       if (len & 1)
> -               len_with_pad = len + 1;
> -
>         memset(&t, 0, sizeof(t));
>         t.tx_buf = buf;
>         t.speed_hz = RT5677_SPI_FREQ;
>         spi_message_init_with_transfers(&m, &t, 1);
>
> -       for (offset = 0; offset < len_with_pad;) {
> +       for (offset = 0; offset < len;) {
>                 spi_cmd = rt5677_spi_select_cmd(false, (addr + offset) & 7,
> -                               len_with_pad - offset, &t.len);
> +                               len - offset, &t.len);
>
>                 /* Construct SPI message header */
>                 buf[0] = spi_cmd;
> --
> 2.21.0.1020.gf2820cf01a-goog
>