Re: [PATCH] drm/dbi: Fix SPI Type 1 (9-bit) transfer

[Paul Cercueil <paul@crapouillou.net>]

Hi Noralf,

Le dim. 5 juil. 2020 à 17:58, Noralf Trønnes <noralf@tronnes.org> a 
écrit :
> 
> 
> Den 03.07.2020 16.13, skrev Paul Cercueil:
>>  The function mipi_dbi_spi1_transfer() will transfer its payload as 
>> 9-bit
>>  data, the 9th (MSB) bit being the data/command bit. In order to do 
>> that,
>>  it unpacks the 8-bit values into 16-bit values, then sets the 9th 
>> bit if
>>  the byte corresponds to data, clears it otherwise. The 7 MSB are
>>  padding. The array of now 16-bit values is then passed to the SPI 
>> core
>>  for transfer.
>> 
>>  This function was broken since its introduction, as the length of 
>> the
>>  SPI transfer was set to the payload size before its conversion, but 
>> the
>>  payload doubled in size due to the 8-bit -> 16-bit conversion.
>> 
>>  Fixes: 02dd95fe3169 ("drm/tinydrm: Add MIPI DBI support")
>>  Cc: <stable@vger.kernel.org> # 4.10
> 
> The code was moved to drm_mipi_dbi.c in 5.4 so this patch won't apply
> before that.

I believe I can submit a patch for pre-5.4 too.

>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  ---
> 
> Thanks for fixing this, clearly I didn't test this. Probably because 
> the
> aux spi ip block on the Raspberry Pi that can do 9 bit didn't have a
> driver at the time. Did you actually test this or was it spotted 
> reading
> the code?

I did test it on hardware, yes - that's how I spotted the bug.

-Paul

> Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
> 
>>   drivers/gpu/drm/drm_mipi_dbi.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>>  diff --git a/drivers/gpu/drm/drm_mipi_dbi.c 
>> b/drivers/gpu/drm/drm_mipi_dbi.c
>>  index bb27c82757f1..bf7888ad9ad4 100644
>>  --- a/drivers/gpu/drm/drm_mipi_dbi.c
>>  +++ b/drivers/gpu/drm/drm_mipi_dbi.c
>>  @@ -923,7 +923,7 @@ static int mipi_dbi_spi1_transfer(struct 
>> mipi_dbi *dbi, int dc,
>>   			}
>>   		}
>> 
>>  -		tr.len = chunk;
>>  +		tr.len = chunk * 2;
>>   		len -= chunk;
>> 
>>   		ret = spi_sync(spi, &m);
>> 


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel