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

[Hector Martin]

On 18/11/2021 18.19, Thomas Zimmermann wrote:
> Hi
>
> Am 17.11.21 um 15:58 schrieb Hector Martin:
> > @@ -897,5 +898,21 @@ static struct platform_driver simpledrm_platform_driver = {
> >    
> >    module_platform_driver(simpledrm_platform_driver);
> >    
> > +static int __init simpledrm_init(void)
> > +{
> > +	struct device_node *np;
> > +
> > +	if (IS_ENABLED(CONFIG_OF_ADDRESS) && of_chosen) {
> > +		for_each_child_of_node(of_chosen, np) {
> > +			if (of_device_is_compatible(np, "simple-framebuffer"))
> > +				of_platform_device_create(np, NULL, NULL);
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +fs_initcall(simpledrm_init);
> > +
>
> Simpledrm is just a driver, but this is platform setup code. Why is this
> code located here and not under arch/ or drivers/firmware/?
>
> I know that other drivers do similar things, it doesn't seem to belong here.

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?)

[1] Documentation/devicetree/bindings/display/simple-framebuffer.yaml

--
Hector Martin (marcan@xxxxxxxxx)
Public Key: https://mrcn.st/pub