linux-kernel - Re: [PATCH 1/2] fbdev/offb: Update expected device name

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 20 Jun 2023 10:04:34 +0200
From:   Michal Suchánek <msuchanek@...e.de>
To:     Helge Deller <deller@....de>
Cc:     Cyril Brulebois <cyril@...amax.com>, Rob Herring <robh@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Linux regressions mailing list <regressions@...ts.linux.dev>,
        Salvatore Bonaccorso <carnil@...ian.org>,
        linux-fbdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, stable@...r.kernel.org
Subject: Re: [PATCH 1/2] fbdev/offb: Update expected device name

On Tue, Jun 20, 2023 at 08:24:34AM +0200, Helge Deller wrote:
> On 6/15/23 23:19, Cyril Brulebois wrote:
> > Hi Rob,
> > 
> > Rob Herring <robh@...nel.org> (2023-06-15):
> > > On Thu, Jun 15, 2023 at 03:21:07PM +0200, Michal Suchánek wrote:
> > > > At the time this was proposed it was said that "of-display", is wrong,
> > > > and that "of-display.0" must be used for the first device instead, and
> > > > if something breaks an alias can be provided.
> > > > 
> > > > So how does one provide an alias so that offb can find "of-display.0"
> > > > as "of-display"?
> > > 
> > > I'm not aware of any way. There isn't because device names and paths are
> > > not considered ABI. There are mechanisms for getting stable class device
> > > indices (e.g. i2c0, mmcblk0, fb0, fb1, etc.) though not implemented for
> > > fbN (and please don't add it).
> > > 
> > > In any case, this should be an easy fix. Though if "linux,opened" or
> > > "linux,boot-display" is not set, then you'd still get "of-display.0":
> > > 
> > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > > index 78ae84187449..e46482cef9c7 100644
> > > --- a/drivers/of/platform.c
> > > +++ b/drivers/of/platform.c
> > > @@ -553,7 +553,7 @@ static int __init of_platform_default_populate_init(void)
> > >                          if (!of_get_property(node, "linux,opened", NULL) ||
> > >                              !of_get_property(node, "linux,boot-display", NULL))
> > >                                  continue;
> > > -                       dev = of_platform_device_create(node, "of-display.0", NULL);
> > > +                       dev = of_platform_device_create(node, "of-display", NULL);
> > >                          of_node_put(node);
> > >                          if (WARN_ON(!dev))
> > >                                  return -ENOMEM;
> 
> Michal, does that patch look correct?
> I don't have that hardware, but that way the boot-display gets named
> "of-display" while all others get "of-display.x"....

Yes, that's inconsistent, and IIRC it's what I originally proposed to
preserve compatibility as much as possible.

The patch went through multiple iteration by me and Rob and the final
version that was merged uses the consistent but incompatible naming.

The other option would be to adapt the offb driver, presumably.

I don't understand how the device name figures in attaching the driver
to the device, the only other occurence of of-display is the name of the
offb and ofdrm drivers.

> > I've just replaced my clueless workaround with this patch on top of the
> > kernel found in Debian 12 (Bookworm), i.e. 6.1.27 at this point, and it
> > indeed fixes the black screen problem in the installer's context.
> 
> ... at least it fixes the issue.

I would expect as much. I don't have the hardware either but keeping
what was there before, and only adding on top is kind of the least
disruptive way of adding new stuff.

Thanks

Michal