On Tue, 23 Sept 2025 at 10:57, Laurent Pinchart
<laurent.pinch...@ideasonboard.com> wrote:
> On Tue, Sep 23, 2025 at 10:55:20AM +0200, Marek Vasut wrote:
> > On 9/23/25 8:47 AM, Geert Uytterhoeven wrote:
> > >> @@ -457,11 +458,11 @@ static void 
> > >> rcar_mipi_dsi_set_display_timing(struct rcar_mipi_dsi *dsi,
> > >>          u32 vprmset4r;
> > >>
> > >>          /* Configuration for Pixel Stream and Packet Header */
> > >> -       if (mipi_dsi_pixel_format_to_bpp(dsi->format) == 24)
> > >> +       if (dsibpp == 24)
> > >>                  rcar_mipi_dsi_write(dsi, TXVMPSPHSETR, 
> > >> TXVMPSPHSETR_DT_RGB24);
> > >> -       else if (mipi_dsi_pixel_format_to_bpp(dsi->format) == 18)
> > >> +       else if (dsibpp == 18)
> > >>                  rcar_mipi_dsi_write(dsi, TXVMPSPHSETR, 
> > >> TXVMPSPHSETR_DT_RGB18);
> > >> -       else if (mipi_dsi_pixel_format_to_bpp(dsi->format) == 16)
> > >> +       else if (dsibpp == 16)
> > >
> > > What about using the switch() statement instead?
> >
> > Not for single-line bodies in the conditionals. The switch {} statement
> > would require additional break; in each case and that's not worth it
> > here, it would only add noise into the code.
>
> I'm a bit surprised. I don't mind much as I don't work on this driver
> myself, but for what it's worth I would find a switch statement to be
> more readable too. Coding style is of course a matter of personal
> preference in many cases.

Exactly.
And you would no longer need the dsibpp local variable.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Reply via email to