Re: [PATCH v4 2/4] drm: mxsfb: Add optional DPI output bus-width configuration

[Francesco Dolcini <francesco@dolcini.it>]

On Wed, Sep 16, 2026 at 01:19:08PM +0200, Luca Ceresoli wrote:
> On Wed Sep 16, 2026 at 11:15 AM CEST, Francesco Dolcini wrote:
> > Hello Luca,
> >
> > On Wed, Sep 16, 2026 at 10:42:13AM +0200, Luca Ceresoli wrote:
> >> > LCDIF programs LCD_DATABUS_WIDTH from the selected media bus format. The
> >> > format reported by the downstream panel or bridge describes the display
> >> > input, but it does not describe how the LCDIF data pins are physically
> >> > wired on the board.
> >> >
> >> > These can differ. For example, a 16-bit LCDIF bus can be connected to a
> >> > 24-bit display by wiring the available color bits to the corresponding
> >> > display inputs. In that case, using the display's 24-bit format to
> >> > configure LCDIF selects the wrong data-bus mode and changes the assignment
> >> > of color bits on the LCD_DATA pins.
> >> >
> >> > Read the optional bus-width endpoint property from the LCDIF output port
> >> > and use it to select the media bus format used to configure LCDIF. This
> >> > allows the LCDIF bus mode to describe the physical interface
> >> > independently of the downstream display format.
> >> >
> >> > When the optional property is absent, or it has an invalid value, continue
> >> > using the format reported by the downstream display device, preserving
> >> > the existing behavior. The code is not validating the DT for
> >> > correctness and just fall back to the previous behavior in case of
> >> > errors.
> >>
> >> Why? Generally speaking errors are better spotted immediately, not ignored
> >> silently.
> >
> > My understanding is that the C driver code should not validate the DT
> > and that this is the general recommendation when parsing properties from
> > the DT.
> >
> > The DT checker is going to spot errors in the DT even earlier and in a
> > static way.
> 
> Drivers typically check for invalid values.
> 
> And people writing their private dts can do mistakes and not run the static
> checks. Bad on their side, sure, but having a runtime error would be useful
> there.

Understood. I would personally not do it. I can add the required error
checking code. No other DRM maintainer/reviewer commented so far, so I
am taking this as a request.

What should I do in case there is an invalid value? Fail or have a
warning message?

> Also consider potential regressions. Say someone has an invalid value in
> their product, say bus-width = <15>, which works because the default 18
> which gets used as a fallback happens to be the correct value for their
> hardware. Some years later we implement the new bus-width = <15>, and then
> that user upgrades to a newer kernel and their display won't work because
> it will start using that incorrect bus-witch = <15>. Better stopping this
> before it starts, IMO.

I do not see this situation honestly, if you have a wrong value, e.g. 15
bits, before this patch is ignored, and after this patch is also
ignored ...

The default in the code is equivalent to the bus-width property not
present, and therefore we use whatever is coming from the panel aka the
next bridge.

Francesco