Re: [PATCH v2 3/3] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84

[Luca Ceresoli]

Hi,

On Thu Jul 16, 2026 at 6:03 PM CEST, Esben Haabendal wrote:
> On Thursday, 16 July 2026 at 17:22, Luca Ceresoli <luca.ceresoli@xxxxxxxxxxx> wrote:
>> On Wed, 15 Jul 2026 15:10:33 +0200, Esben Haabendal <esben@xxxxxxxxxx> wrote:
>>
>> > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
>> > index 7e73035d7798..02ae2b8179b0 100644
>> > --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
>> > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
>> > @@ -893,6 +895,7 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
>> >  {
>> >  	struct drm_bridge *panel_bridge;
>> >  	struct device *dev = ctx->dev;
>> > +	u32 panel_port = 2;
>>
>> Thanks for having applied the changes I had suggested.
>>
>> I'm afraid I realized right now the "panel_port" name is not ideal because
>> there could be another bridge at the output port and not directly a
>> panel. Should you have a reason to send a v3 I'd rename to output_port (and
>> keep my R-by if that's the only change). Otherwise I can do it while
>> applying the patch.
>
> What about the panel_bridge member of struct sn65dsi83? We use
> panel_port/output_port as argument to devm_drm_of_get_bridge() and assign the
> bridge we get from that to ctx->panel_bridge. So are you sure that it makes
> sense to rename panel_port to output_port? Or do you want to rename the
> panel_bridge member also?

Ah, I understand. Well, the panel_bridge name is confusing, perhaps doe to
historical legacy. Perhaps the name was given when the next thing after the
sn65dsi8x could only be a panel. Now if you look at the
devm_drm_of_get_bridge() code it is one of:

  1. a panel_bridge created on the fly to wrap a panel
     (a panel is connected at the sn65dsi8x output)
  2. a pre-existing drm_bridge that does not wrap a panel
     (a LVDS-to-something-else-bridge is connected at the sn65dsi8x output)

In the latter case the panel_bridge does not make sense. But it works
because int he end it is a struct drm_bridge *, not a panel-specific thing.

I am about to change all that and eventually have:

 - a panel_bridge created for every drm_panel
 - all interactions with the next element done accessing the next_bridge,
   no more accessing a drm_panel directly
 - drm_of_find_panel_or_bridge() disappear, because there will be no need
   to access the "next panel"
 - devm_drm_of_get_bridge() just return the next bridge, not bothering
   about panels

I hope this clarifies.

That said, I wouldn't bother renaming existing variables, but for new
variables being added I'd try to use the best possible name. Here you are
storing the number of the output port, so I suggested output_port.

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com