Re: [PATCH v4 4/6] drm/ssd130x: Replace positional ssd130x_spi_id[] initialization with C99

[Amit Barzilai]

On Tue, Aug 18, 2026 at 10:25 PM Amit Barzilai
<amit.barzilai22@xxxxxxxxx> wrote:
>
> On Tue, Aug 18, 2026 at 11:41 AM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxx> wrote:
>
> > >  static const struct spi_device_id ssd130x_spi_id[] = {
> > >       /* ssd130x family */
> > > +     { .name = "sh1106", .driver_data = SH1106_ID },
> >
> > Hmm... Is the comment correct for sh1106?
> >
> > >       /* ssd132x family */
> > >       /* ssd133x family */
> >
> > Overall, what do these comment bring us? Are those families have something in
> > common? Because the driver data is unique for each device, so I assume it's not
> > that compatible inside even the same families.
> >
> > TL;DR: Perhaps simply drop those comments for once?
>
> Agreed, I'll remove the comments in v5.

Having looked closer, I'd rather keep the comments in v5.

The same grouping comments are used throughout ssd130x.c,
ssd130x-spi.c, ssd130x.h and ssd130x-i2c.c. Dropping them from
ssd130x_spi_id[] alone would leave the driver inconsistent, and
dropping them everywhere is a cleanup of its own rather than
something this series should carry.

The comment is correct for sh1106: ssd130x_variants[SH1106_ID] sets
.family_id = SSD130X_FAMILY.

And the families do mean something concrete here. family_id is what
indexes the plane, CRTC and encoder helper vtables and the backlight
ops.