On Tue, Nov 30, 2021 at 12:45 AM Javier Martinez Canillas <javier@xxxxxxxxxxxx> wrote: > > > > > > > > > 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. It is because that is where most of the other devices are created unless the bus handles it. > 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. Doesn't look like that would share anything with anything else (BIOS/EFI/ACPI). Rob
