[PATCH 1/2] drm/dbi: Support DBI typec1 read operations

[Doug Anderson]

Hi,

On Fri, Jun 11, 2021 at 2:29 PM Linus Walleij <linus.walleij at linaro.org> wrote:
>
> +static int mipi_dbi_typec1_command_read(struct mipi_dbi *dbi, u8 *cmd,
> +                                       u8 *data, size_t len)
> +{
> +       struct spi_device *spi = dbi->spi;
> +       u32 speed_hz = min_t(u32, MIPI_DBI_MAX_SPI_READ_SPEED,
> +                            spi->max_speed_hz / 2);

I'm kinda curious why "max_speed_hz / 2", but you match the "typec3"
version of this so I guess it's right?


> +       struct spi_transfer tr[2] = {
> +               {
> +                       .speed_hz = speed_hz,
> +                       .bits_per_word = 9,
> +                       .tx_buf = dbi->tx_buf9,
> +                       .len = 2,
> +               }, {
> +                       .speed_hz = speed_hz,
> +                       .bits_per_word = 8,
> +                       .len = len,
> +                       .rx_buf = data,
> +               },
> +       };
> +       struct spi_message m;
> +       u16 *dst16;
> +       int ret;
> +
> +       if (!len)
> +               return -EINVAL;
> +
> +       if (!spi_is_bpw_supported(spi, 9)) {
> +               /*
> +                * FIXME: implement something like mipi_dbi_spi1e_transfer() but
> +                * for reads using emulation.
> +                */
> +               dev_err(&spi->dev,
> +                       "reading on host not supporting 9 bpw not yet implemented\n");
> +               return -EOPNOTSUPP;
> +       }
> +
> +       /*
> +        * Turn the 8bit command into a 16bit version of the command in the
> +        * buffer. Only 9 bits of this will be used when executing the actual
> +        * transfer.
> +        */
> +       dst16 = dbi->tx_buf9;
> +       dst16[0] = *cmd;
> +
> +       spi_message_init_with_transfers(&m, tr, ARRAY_SIZE(tr));
> +       ret = spi_sync(spi, &m);
> +
> +       MIPI_DBI_DEBUG_COMMAND(*cmd, data, len);

nit: Only call MIPI_DBI_DEBUG_COMMAND() if !ret? Otherwise on an error
you're just going to output whatever random data was already in the
buffer that the user passed you. I suppose that same bug is present in
mipi_dbi_typec3_command_read().

Other than that, this seems OK based on my feeble understanding of
MIPI DBI (I couldn't even find the docs that I dug up before, sadly).
Luckily Noralf is also here to review that part! :-) I'm happy with:

Reviewed-by: Douglas Anderson <dianders at chromium.org>