linux-kernel - Re: [PATCH 1/3] drm/simpledrm: Bind to OF framebuffers in /chosen

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]

Message-ID: <CABxcv=mkuJLrXr_nbELg39qJvUvV2y69FAisFKURR9bqa3FzKg@mail.gmail.com>
Date:   Tue, 30 Nov 2021 07:44:53 +0100
From:   Javier Martinez Canillas <javier@...hile0.org>
To:     Rob Herring <robh+dt@...nel.org>
Cc:     Hector Martin <marcan@...can.st>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        David Airlie <airlied@...ux.ie>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Alyssa Rosenzweig <alyssa@...enzweig.io>
Subject: Re: [PATCH 1/3] drm/simpledrm: Bind to OF framebuffers in /chosen

> > >
> > > Simpledrm is just a driver, but this is platform setup code. Why is this
> > > code located here and not under arch/ or drivers/firmware/?
> > >

Agreed. Creating platform devices is something for platform code and
not really a DRM driver.

> > > I know that other drivers do similar things, it doesn't seem to belong here.
> >

Yeah, the simplefb driver does this but that seems like something that
should be changed.

> > This definitely doesn't belong in either of those, since it is not arch-
> > or firmware-specific. It is implementing support for the standard
> > simple-framebuffer OF binding, which specifies that it must be located
> > within the /chosen node (and thus the default OF setup code won't do the
> > matching for you); this applies to all OF platforms [1]
> >
> > Adding Rob; do you think this should move from simplefb/simpledrm to
> > common OF code? (where?)
>
> of_platform_default_populate_init() should work.

That should work but I still wonder if it is the correct place to add
this logic.

I think that instead it could be done in the sysfb_create_simplefb()
function [0], which already creates the "simple-framebuffer" device
for x86 legacy BIOS and x86/arm64/riscv EFI so it makes sense to do
the same for OF. That way the simplefb platform device registration
code could also be dropped from the driver and users would just need
to enable CONFIG_SYSFB and CONFIG_SYSFB_SIMPLEFB to have the same.

I have a couple of boards with a bootloader that populates a
"simple-framebuffer" in the /chosen node so I could attempt to write
the patches. But probably won't happen until next week.

[0]: https://elixir.bootlin.com/linux/v5.16-rc3/source/drivers/firmware/sysfb_simplefb.c#L60

Best regards,
Javier