Re: [PATCH v3 3/3] drm/ssd130x: Add SSD135X_FAMILY and SSD1351 support

Amit Barzilai <[email protected]> writes:

Hello Amit,

> Hey Javier,

[...]

>
> So for v4 I propose:
>
> - Keep ssd130x_write_cmds() writing SSD13XX_COMMAND per byte as it does
>   today, so there is no change to I2C or SPI wire behaviour anywhere in
>   the series.
>
> - Still do the ssd130x_write_cmd()/ssd130x_write_cmds() unification as
>   its own prep patch, as you asked. It removes the duplicated variadic
>   loop and gives a single place for the check below.
>
> - Add a bool cmd_params_are_data to ssd130x_deviceinfo, checked after
>   the first byte is sent to decide how to send the parameters. If it is
>   set, the parameters go through ssd130x_write_data(); otherwise the
>   existing byte-by-byte loop is used.
>
> - Drop the separate SPI transport patch entirely.
>
> The comment in the core will be phrased in the core's own terms this
> time: parameters go on the data path rather than as further command
> bytes, without mentioning D/C or any other transport-specific
> information.
>

Thanks for the explanations. Since you have given a lot of thought of this
I trust your judgement and agree with your plan. Let's see how the patches
look once you post them.

> Apologies for going back on something I already agreed to. If you still
> prefer it in the SPI layer I will do it that way and fold in the 0x00
> control byte change, with the I2C framing change called out in the
> commit message, but I wanted to flag the breakage before building on it.
> I have already implemented the fixes for most of the other comments
> locally. Once we settle this, I will submit the finished v4.
>

No need to apologize, I appreciate the deep thought and willing to not
break the I2C devices already supported.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat