Re: [PATCH] fbdev: omapfb: Fix __be32 sparse warning in panel_enabled()

Re: [PATCH] fbdev: omapfb: Fix __be32 sparse warning in panel_enabled()

[Helge Deller]

CC'ing the linux-omap mailing list, in the hope someone with omap
knowledge can judge if the sashiko AI bot is right with his analysis that
acx565akm_transfer() might be wrong on big-endian systems?

On 8/22/26 11:58, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Hardcoded little-endian assumption when transmitting the command byte breaks the driver on big-endian architectures.
> --
> 
> --- Patch [1]: [PATCH] fbdev: omapfb: Fix __be32 sparse warning in panel_enabled() ---
> commit a9e06be56fc1b74d7c6e76133e9ea1c25675a615
> Author: Helge Deller <deller@gmx.de>
> Subject: fbdev: omapfb: Fix __be32 sparse warning in panel_enabled()
> 
> This commit resolves a sparse warning in panel_enabled() by explicitly
> reading the display status into a __be32 variable. It then converts this
> value to CPU endianness using __be32_to_cpu() before checking the bits.
> 
> Closes: https://lore.kernel.org/oe-kbuild-all/202608211811.lps93xao-lkp@intel.com/
> 
>> diff --git a/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c b/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c
>> index 8f430d9e80549..0202ca8cbfc23 100644
>> --- a/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c
>> +++ b/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c
>> @@ -210,12 +210,13 @@ static void set_display_state(struct panel_drv_data *ddata, int enabled)
>>   
>>   static int panel_enabled(struct panel_drv_data *ddata)
>>   {
>> +	__be32 disp_status_be;
>>   	u32 disp_status;
>>   	int enabled;
>>   
>>   	acx565akm_read(ddata, MIPID_CMD_READ_DISP_STATUS,
>> -			(u8 *)&disp_status, 4);
> 
> [Severity: High]
> This is a pre-existing issue, but while looking at endianness fixes in this
> file, does acx565akm_transfer() have a similar issue on big-endian
> architectures?
> 
> When acx565akm_read() calls acx565akm_transfer(), it passes the command as
> an int:
> 
> drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c:acx565akm_transfer() {
>      ...
>      cmd &=  0xff;
>      x->tx_buf = &cmd;
>      x->bits_per_word = 9;
>      x->len = 2;
>      ...
> }
> 
> Because bits_per_word > 8, the SPI controller reads the buffer as a 16-bit
> integer. Taking the address of a 32-bit int on a little-endian system yields
> the command byte in the lowest bits. However, on big-endian systems, &cmd
> points to the most significant bytes (which are 0x00).
> 
> Will the SPI controller end up transmitting 0x0000 instead of the requested
> command on big-endian architectures?
> 
>> -	disp_status = __be32_to_cpu(disp_status);
>> +			(u8 *)&disp_status_be, 4);
>> +	disp_status = __be32_to_cpu(disp_status_be);
>>   	enabled = (disp_status & (1 << 17)) && (disp_status & (1 << 10));
>>   	dev_dbg(&ddata->spi->dev,
>>   		"LCD panel %senabled by bootloader (status 0x%04x)\n",
>>   		enabled ? "" : "not ", disp_status);
> 

Re: [PATCH] fbdev: omapfb: Fix __be32 sparse warning in panel_enabled()

[Andreas Kemnade]

On Sat, 22 Aug 2026 12:23:35 +0200
Helge Deller <deller@gmx.de> wrote:

> CC'ing the linux-omap mailing list, in the hope someone with omap
> knowledge can judge if the sashiko AI bot is right with his analysis that
> acx565akm_transfer() might be wrong on big-endian systems?
> 

well, it sits behind the spi interface, which should abstract hw oddities.
So wie should not ask about omap specific stuff.

BTW: same issue in drm variant of the driver.

> On 8/22/26 11:58, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > 
> > Pre-existing issues:
> > - [High] Hardcoded little-endian assumption when transmitting the command byte breaks the driver on big-endian architectures.
> > --
> > 
> > --- Patch [1]: [PATCH] fbdev: omapfb: Fix __be32 sparse warning in panel_enabled() ---
> > commit a9e06be56fc1b74d7c6e76133e9ea1c25675a615
> > Author: Helge Deller <deller@gmx.de>
> > Subject: fbdev: omapfb: Fix __be32 sparse warning in panel_enabled()
> > 
> > This commit resolves a sparse warning in panel_enabled() by explicitly
> > reading the display status into a __be32 variable. It then converts this
> > value to CPU endianness using __be32_to_cpu() before checking the bits.
> > 
> > Closes: https://lore.kernel.org/oe-kbuild-all/202608211811.lps93xao-lkp@intel.com/
> >   
> >> diff --git a/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c b/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c
> >> index 8f430d9e80549..0202ca8cbfc23 100644
> >> --- a/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c
> >> +++ b/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c
> >> @@ -210,12 +210,13 @@ static void set_display_state(struct panel_drv_data *ddata, int enabled)
> >>   
> >>   static int panel_enabled(struct panel_drv_data *ddata)
> >>   {
> >> +	__be32 disp_status_be;
> >>   	u32 disp_status;
> >>   	int enabled;
> >>   
> >>   	acx565akm_read(ddata, MIPID_CMD_READ_DISP_STATUS,
> >> -			(u8 *)&disp_status, 4);  
> > 
> > [Severity: High]
> > This is a pre-existing issue, but while looking at endianness fixes in this
> > file, does acx565akm_transfer() have a similar issue on big-endian
> > architectures?
> > 
> > When acx565akm_read() calls acx565akm_transfer(), it passes the command as
> > an int:
> > 
> > drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c:acx565akm_transfer() {
> >      ...
> >      cmd &=  0xff;
> >      x->tx_buf = &cmd;
> >      x->bits_per_word = 9;
> >      x->len = 2;
> >      ...
> > }
> > 
> > Because bits_per_word > 8, the SPI controller reads the buffer as a 16-bit
> > integer. Taking the address of a 32-bit int on a little-endian system yields
> > the command byte in the lowest bits. However, on big-endian systems, &cmd
> > points to the most significant bytes (which are 0x00).
> > 
> > Will the SPI controller end up transmitting 0x0000 instead of the requested
> > command on big-endian architectures?
> > 
Probably yes, at least if I do not see uint32 * as a pointer in the tx_buf there.
The more interesting thing would be the 9th bit. There would be some requirement
neded and documented in the spi_transfer struct to put things into 32bit words
if bits per word is > 8. Nobody would forbid using u16.

Regards,
Andreas