* [bug report] drm: rcar-du: Add RZ/G2L DSI driver
@ 2022-11-18 15:01 ` Dan Carpenter
0 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2022-11-18 15:01 UTC (permalink / raw)
To: biju.das.jz; +Cc: dri-devel, linux-renesas-soc
Hello Biju Das,
The patch 7a043f978ed1: "drm: rcar-du: Add RZ/G2L DSI driver" from
Sep 20, 2022, leads to the following Smatch static checker warning:
drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c:372 rzg2l_mipi_dsi_set_display_timing()
warn: uninitialized special assign 'vich1ppsetr |= (1 << 15)'
drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
347 static void rzg2l_mipi_dsi_set_display_timing(struct rzg2l_mipi_dsi *dsi,
348 const struct drm_display_mode *mode)
349 {
350 u32 vich1ppsetr;
351 u32 vich1vssetr;
352 u32 vich1vpsetr;
353 u32 vich1hssetr;
354 u32 vich1hpsetr;
355 int dsi_format;
356 u32 delay[2];
357 u8 index;
358
359 /* Configuration for Pixel Packet */
360 dsi_format = mipi_dsi_pixel_format_to_bpp(dsi->format);
361 switch (dsi_format) {
362 case 24:
363 vich1ppsetr = VICH1PPSETR_DT_RGB24;
364 break;
365 case 18:
366 vich1ppsetr = VICH1PPSETR_DT_RGB18;
367 break;
What if mipi_dsi_pixel_format_to_bpp() returns 16?
368 }
369
370 if ((dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) &&
371 !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST))
--> 372 vich1ppsetr |= VICH1PPSETR_TXESYNC_PULSE;
^^^^^^^^^^^
Uninitialized.
373
374 rzg2l_mipi_dsi_link_write(dsi, VICH1PPSETR, vich1ppsetr);
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* [bug report] drm: rcar-du: Add RZ/G2L DSI driver
@ 2022-11-18 15:01 ` Dan Carpenter
0 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2022-11-18 15:01 UTC (permalink / raw)
To: biju.das.jz; +Cc: linux-renesas-soc, dri-devel
Hello Biju Das,
The patch 7a043f978ed1: "drm: rcar-du: Add RZ/G2L DSI driver" from
Sep 20, 2022, leads to the following Smatch static checker warning:
drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c:372 rzg2l_mipi_dsi_set_display_timing()
warn: uninitialized special assign 'vich1ppsetr |= (1 << 15)'
drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
347 static void rzg2l_mipi_dsi_set_display_timing(struct rzg2l_mipi_dsi *dsi,
348 const struct drm_display_mode *mode)
349 {
350 u32 vich1ppsetr;
351 u32 vich1vssetr;
352 u32 vich1vpsetr;
353 u32 vich1hssetr;
354 u32 vich1hpsetr;
355 int dsi_format;
356 u32 delay[2];
357 u8 index;
358
359 /* Configuration for Pixel Packet */
360 dsi_format = mipi_dsi_pixel_format_to_bpp(dsi->format);
361 switch (dsi_format) {
362 case 24:
363 vich1ppsetr = VICH1PPSETR_DT_RGB24;
364 break;
365 case 18:
366 vich1ppsetr = VICH1PPSETR_DT_RGB18;
367 break;
What if mipi_dsi_pixel_format_to_bpp() returns 16?
368 }
369
370 if ((dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) &&
371 !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST))
--> 372 vich1ppsetr |= VICH1PPSETR_TXESYNC_PULSE;
^^^^^^^^^^^
Uninitialized.
373
374 rzg2l_mipi_dsi_link_write(dsi, VICH1PPSETR, vich1ppsetr);
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [bug report] drm: rcar-du: Add RZ/G2L DSI driver
2022-11-18 15:01 ` Dan Carpenter
@ 2022-11-18 15:20 ` Biju Das
-1 siblings, 0 replies; 8+ messages in thread
From: Biju Das @ 2022-11-18 15:20 UTC (permalink / raw)
To: Dan Carpenter
Cc: dri-devel@lists.freedesktop.org,
linux-renesas-soc@vger.kernel.org, Laurent Pinchart
Hi Dan Carpenter,
Thanks for the feedback.
> Subject: [bug report] drm: rcar-du: Add RZ/G2L DSI driver
>
> Hello Biju Das,
>
> The patch 7a043f978ed1: "drm: rcar-du: Add RZ/G2L DSI driver" from Sep
> 20, 2022, leads to the following Smatch static checker warning:
>
> drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c:372
> rzg2l_mipi_dsi_set_display_timing()
> warn: uninitialized special assign 'vich1ppsetr |= (1 << 15)'
>
> drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
> 347 static void rzg2l_mipi_dsi_set_display_timing(struct
> rzg2l_mipi_dsi *dsi,
> 348 const struct
> drm_display_mode *mode)
> 349 {
> 350 u32 vich1ppsetr;
> 351 u32 vich1vssetr;
> 352 u32 vich1vpsetr;
> 353 u32 vich1hssetr;
> 354 u32 vich1hpsetr;
> 355 int dsi_format;
> 356 u32 delay[2];
> 357 u8 index;
> 358
> 359 /* Configuration for Pixel Packet */
> 360 dsi_format = mipi_dsi_pixel_format_to_bpp(dsi->format);
> 361 switch (dsi_format) {
> 362 case 24:
> 363 vich1ppsetr = VICH1PPSETR_DT_RGB24;
> 364 break;
> 365 case 18:
> 366 vich1ppsetr = VICH1PPSETR_DT_RGB18;
> 367 break;
>
> What if mipi_dsi_pixel_format_to_bpp() returns 16?
This condition is already validated in rzg2l_mipi_dsi_host_attach(). For val of 16,
It returns error. See line 623.
default:
dev_err(dsi->dev, "Unsupported format 0x%04x\n", device->format);
return -EINVAL;
Cheers,
Biju
>
> 368 }
> 369
> 370 if ((dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
> &&
> 371 !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST))
> --> 372 vich1ppsetr |= VICH1PPSETR_TXESYNC_PULSE;
> ^^^^^^^^^^^
> Uninitialized.
>
> 373
> 374 rzg2l_mipi_dsi_link_write(dsi, VICH1PPSETR,
> vich1ppsetr);
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [bug report] drm: rcar-du: Add RZ/G2L DSI driver
@ 2022-11-18 15:20 ` Biju Das
0 siblings, 0 replies; 8+ messages in thread
From: Biju Das @ 2022-11-18 15:20 UTC (permalink / raw)
To: Dan Carpenter
Cc: linux-renesas-soc@vger.kernel.org, Laurent Pinchart,
dri-devel@lists.freedesktop.org
Hi Dan Carpenter,
Thanks for the feedback.
> Subject: [bug report] drm: rcar-du: Add RZ/G2L DSI driver
>
> Hello Biju Das,
>
> The patch 7a043f978ed1: "drm: rcar-du: Add RZ/G2L DSI driver" from Sep
> 20, 2022, leads to the following Smatch static checker warning:
>
> drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c:372
> rzg2l_mipi_dsi_set_display_timing()
> warn: uninitialized special assign 'vich1ppsetr |= (1 << 15)'
>
> drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
> 347 static void rzg2l_mipi_dsi_set_display_timing(struct
> rzg2l_mipi_dsi *dsi,
> 348 const struct
> drm_display_mode *mode)
> 349 {
> 350 u32 vich1ppsetr;
> 351 u32 vich1vssetr;
> 352 u32 vich1vpsetr;
> 353 u32 vich1hssetr;
> 354 u32 vich1hpsetr;
> 355 int dsi_format;
> 356 u32 delay[2];
> 357 u8 index;
> 358
> 359 /* Configuration for Pixel Packet */
> 360 dsi_format = mipi_dsi_pixel_format_to_bpp(dsi->format);
> 361 switch (dsi_format) {
> 362 case 24:
> 363 vich1ppsetr = VICH1PPSETR_DT_RGB24;
> 364 break;
> 365 case 18:
> 366 vich1ppsetr = VICH1PPSETR_DT_RGB18;
> 367 break;
>
> What if mipi_dsi_pixel_format_to_bpp() returns 16?
This condition is already validated in rzg2l_mipi_dsi_host_attach(). For val of 16,
It returns error. See line 623.
default:
dev_err(dsi->dev, "Unsupported format 0x%04x\n", device->format);
return -EINVAL;
Cheers,
Biju
>
> 368 }
> 369
> 370 if ((dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
> &&
> 371 !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST))
> --> 372 vich1ppsetr |= VICH1PPSETR_TXESYNC_PULSE;
> ^^^^^^^^^^^
> Uninitialized.
>
> 373
> 374 rzg2l_mipi_dsi_link_write(dsi, VICH1PPSETR,
> vich1ppsetr);
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* [bug report] drm: rcar-du: Add RZ/G2L DSI driver
@ 2023-06-15 6:14 Dan Carpenter
2023-06-15 6:22 ` Biju Das
0 siblings, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2023-06-15 6:14 UTC (permalink / raw)
To: biju.das.jz; +Cc: linux-renesas-soc
Hello Biju Das,
The patch 7a043f978ed1: "drm: rcar-du: Add RZ/G2L DSI driver" from
Sep 20, 2022, leads to the following Smatch static checker warning:
drivers/gpu/drm/renesas/rcar-du/rzg2l_mipi_dsi.c:374 rzg2l_mipi_dsi_set_display_timing()
error: uninitialized symbol 'vich1ppsetr'.
drivers/gpu/drm/renesas/rcar-du/rzg2l_mipi_dsi.c
347 static void rzg2l_mipi_dsi_set_display_timing(struct rzg2l_mipi_dsi *dsi,
348 const struct drm_display_mode *mode)
349 {
350 u32 vich1ppsetr;
351 u32 vich1vssetr;
352 u32 vich1vpsetr;
353 u32 vich1hssetr;
354 u32 vich1hpsetr;
355 int dsi_format;
356 u32 delay[2];
357 u8 index;
358
359 /* Configuration for Pixel Packet */
360 dsi_format = mipi_dsi_pixel_format_to_bpp(dsi->format);
361 switch (dsi_format) {
362 case 24:
363 vich1ppsetr = VICH1PPSETR_DT_RGB24;
364 break;
365 case 18:
366 vich1ppsetr = VICH1PPSETR_DT_RGB18;
367 break;
mipi_dsi_pixel_format_to_bpp() also returns 16 and -EIVNAL.
368 }
369
370 if ((dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) &&
371 !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST))
372 vich1ppsetr |= VICH1PPSETR_TXESYNC_PULSE;
373
--> 374 rzg2l_mipi_dsi_link_write(dsi, VICH1PPSETR, vich1ppsetr);
^^^^^^^^^^^
Static checker uninitialized variable warning.
375
376 /* Configuration for Video Parameters */
377 vich1vssetr = VICH1VSSETR_VACTIVE(mode->vdisplay) |
378 VICH1VSSETR_VSA(mode->vsync_end - mode->vsync_start);
379 vich1vssetr |= (mode->flags & DRM_MODE_FLAG_PVSYNC) ?
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [bug report] drm: rcar-du: Add RZ/G2L DSI driver
2023-06-15 6:14 [bug report] drm: rcar-du: Add RZ/G2L DSI driver Dan Carpenter
@ 2023-06-15 6:22 ` Biju Das
2023-06-15 12:37 ` Laurent Pinchart
0 siblings, 1 reply; 8+ messages in thread
From: Biju Das @ 2023-06-15 6:22 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-renesas-soc@vger.kernel.org, Laurent Pinchart
Hi Dan Carpenter,
Thanks for the feedback.
+ Laurent.
It is already validated in [1]
[1] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c#L619
So we won't get wrong formats in rzg2l_mipi_dsi_set_display_timing().
Laurent, Please correct me if I am wrong.
Cheers,
Biju
> -----Original Message-----
> From: Dan Carpenter <dan.carpenter@linaro.org>
> Sent: Thursday, June 15, 2023 7:14 AM
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Cc: linux-renesas-soc@vger.kernel.org
> Subject: [bug report] drm: rcar-du: Add RZ/G2L DSI driver
>
> Hello Biju Das,
>
> The patch 7a043f978ed1: "drm: rcar-du: Add RZ/G2L DSI driver" from Sep 20,
> 2022, leads to the following Smatch static checker warning:
>
> drivers/gpu/drm/renesas/rcar-du/rzg2l_mipi_dsi.c:374
> rzg2l_mipi_dsi_set_display_timing()
> error: uninitialized symbol 'vich1ppsetr'.
>
> drivers/gpu/drm/renesas/rcar-du/rzg2l_mipi_dsi.c
> 347 static void rzg2l_mipi_dsi_set_display_timing(struct
> rzg2l_mipi_dsi *dsi,
> 348 const struct
> drm_display_mode *mode)
> 349 {
> 350 u32 vich1ppsetr;
> 351 u32 vich1vssetr;
> 352 u32 vich1vpsetr;
> 353 u32 vich1hssetr;
> 354 u32 vich1hpsetr;
> 355 int dsi_format;
> 356 u32 delay[2];
> 357 u8 index;
> 358
> 359 /* Configuration for Pixel Packet */
> 360 dsi_format = mipi_dsi_pixel_format_to_bpp(dsi->format);
> 361 switch (dsi_format) {
> 362 case 24:
> 363 vich1ppsetr = VICH1PPSETR_DT_RGB24;
> 364 break;
> 365 case 18:
> 366 vich1ppsetr = VICH1PPSETR_DT_RGB18;
> 367 break;
>
> mipi_dsi_pixel_format_to_bpp() also returns 16 and -EIVNAL.
>
> 368 }
> 369
> 370 if ((dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) &&
> 371 !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST))
> 372 vich1ppsetr |= VICH1PPSETR_TXESYNC_PULSE;
> 373
> --> 374 rzg2l_mipi_dsi_link_write(dsi, VICH1PPSETR, vich1ppsetr);
> ^^^^^^^^^^^
> Static checker uninitialized variable warning.
>
> 375
> 376 /* Configuration for Video Parameters */
> 377 vich1vssetr = VICH1VSSETR_VACTIVE(mode->vdisplay) |
> 378 VICH1VSSETR_VSA(mode->vsync_end - mode-
> >vsync_start);
> 379 vich1vssetr |= (mode->flags & DRM_MODE_FLAG_PVSYNC) ?
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [bug report] drm: rcar-du: Add RZ/G2L DSI driver
2023-06-15 6:22 ` Biju Das
@ 2023-06-15 12:37 ` Laurent Pinchart
2023-06-15 14:10 ` Dan Carpenter
0 siblings, 1 reply; 8+ messages in thread
From: Laurent Pinchart @ 2023-06-15 12:37 UTC (permalink / raw)
To: Biju Das; +Cc: Dan Carpenter, linux-renesas-soc@vger.kernel.org
Hello,
On Thu, Jun 15, 2023 at 06:22:45AM +0000, Biju Das wrote:
> Hi Dan Carpenter,
>
> Thanks for the feedback.
>
> + Laurent.
>
> It is already validated in [1]
>
> [1] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c#L619
>
> So we won't get wrong formats in rzg2l_mipi_dsi_set_display_timing().
>
> Laurent, Please correct me if I am wrong.
The issue is that static checkers can't know that. You should add a
default statement to the switch, possibly with a comment to explain why.
> > -----Original Message-----
> > From: Dan Carpenter <dan.carpenter@linaro.org>
> > Sent: Thursday, June 15, 2023 7:14 AM
> > To: Biju Das <biju.das.jz@bp.renesas.com>
> > Cc: linux-renesas-soc@vger.kernel.org
> > Subject: [bug report] drm: rcar-du: Add RZ/G2L DSI driver
> >
> > Hello Biju Das,
> >
> > The patch 7a043f978ed1: "drm: rcar-du: Add RZ/G2L DSI driver" from Sep 20,
> > 2022, leads to the following Smatch static checker warning:
> >
> > drivers/gpu/drm/renesas/rcar-du/rzg2l_mipi_dsi.c:374 rzg2l_mipi_dsi_set_display_timing()
> > error: uninitialized symbol 'vich1ppsetr'.
> >
> > drivers/gpu/drm/renesas/rcar-du/rzg2l_mipi_dsi.c
> > 347 static void rzg2l_mipi_dsi_set_display_timing(struct rzg2l_mipi_dsi *dsi,
> > 348 const struct drm_display_mode *mode)
> > 349 {
> > 350 u32 vich1ppsetr;
> > 351 u32 vich1vssetr;
> > 352 u32 vich1vpsetr;
> > 353 u32 vich1hssetr;
> > 354 u32 vich1hpsetr;
> > 355 int dsi_format;
> > 356 u32 delay[2];
> > 357 u8 index;
> > 358
> > 359 /* Configuration for Pixel Packet */
> > 360 dsi_format = mipi_dsi_pixel_format_to_bpp(dsi->format);
> > 361 switch (dsi_format) {
> > 362 case 24:
> > 363 vich1ppsetr = VICH1PPSETR_DT_RGB24;
> > 364 break;
> > 365 case 18:
> > 366 vich1ppsetr = VICH1PPSETR_DT_RGB18;
> > 367 break;
> >
> > mipi_dsi_pixel_format_to_bpp() also returns 16 and -EIVNAL.
> >
> > 368 }
> > 369
> > 370 if ((dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) &&
> > 371 !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST))
> > 372 vich1ppsetr |= VICH1PPSETR_TXESYNC_PULSE;
> > 373
> > --> 374 rzg2l_mipi_dsi_link_write(dsi, VICH1PPSETR, vich1ppsetr);
> > ^^^^^^^^^^^
> > Static checker uninitialized variable warning.
> >
> > 375
> > 376 /* Configuration for Video Parameters */
> > 377 vich1vssetr = VICH1VSSETR_VACTIVE(mode->vdisplay) |
> > 378 VICH1VSSETR_VSA(mode->vsync_end - mode->vsync_start);
> > 379 vich1vssetr |= (mode->flags & DRM_MODE_FLAG_PVSYNC) ?
> >
> > regards,
> > dan carpenter
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [bug report] drm: rcar-du: Add RZ/G2L DSI driver
2023-06-15 12:37 ` Laurent Pinchart
@ 2023-06-15 14:10 ` Dan Carpenter
0 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2023-06-15 14:10 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: Biju Das, linux-renesas-soc@vger.kernel.org
On Thu, Jun 15, 2023 at 03:37:49PM +0300, Laurent Pinchart wrote:
> Hello,
>
> On Thu, Jun 15, 2023 at 06:22:45AM +0000, Biju Das wrote:
> > Hi Dan Carpenter,
> >
> > Thanks for the feedback.
> >
> > + Laurent.
> >
> > It is already validated in [1]
> >
> > [1] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c#L619
> >
> > So we won't get wrong formats in rzg2l_mipi_dsi_set_display_timing().
> >
> > Laurent, Please correct me if I am wrong.
>
> The issue is that static checkers can't know that. You should add a
> default statement to the switch, possibly with a comment to explain why.
>
Yeah. Smatch is bad at tracking things which happen in separate system
calls. I hope eventually to be able to silence this warning in Smatch
but that's still some time away.
I don't have strong feelings that we need to silence every static
checker warning. Generally the rule of thumb is that we should always
fix the checker instead.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-06-15 14:10 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-15 6:14 [bug report] drm: rcar-du: Add RZ/G2L DSI driver Dan Carpenter
2023-06-15 6:22 ` Biju Das
2023-06-15 12:37 ` Laurent Pinchart
2023-06-15 14:10 ` Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2022-11-18 15:01 Dan Carpenter
2022-11-18 15:01 ` Dan Carpenter
2022-11-18 15:20 ` Biju Das
2022-11-18 15:20 ` Biju Das
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.