MIME-Version: 1.0
References: <20211117145829.204360-1-marcan@marcan.st> <20211117145829.204360-2-marcan@marcan.st>
 <f3582c00-925d-91ec-c829-0aaa8f0157c0@suse.de> <36f3cf18-6654-e1bf-1fa6-a5797751ee86@marcan.st>
In-Reply-To: <36f3cf18-6654-e1bf-1fa6-a5797751ee86@marcan.st>
From:   Rob Herring <robh+dt@kernel.org>
Date:   Mon, 29 Nov 2021 13:29:34 -0600
Message-ID: <CAL_JsqLd=NrZgkTw+N2+Ka4zqRVpZMRNSisUDV9MhBQA-0TZQg@mail.gmail.com>
Subject: Re: [PATCH 1/3] drm/simpledrm: Bind to OF framebuffers in /chosen
To:     Hector Martin <marcan@marcan.st>
Cc:     Thomas Zimmermann <tzimmermann@suse.de>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Maxime Ripard <mripard@kernel.org>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>,
        dri-devel <dri-devel@lists.freedesktop.org>,
        Alyssa Rosenzweig <alyssa@rosenzweig.io>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Nov 19, 2021 at 9:24 PM Hector Martin <marcan@marcan.st> wrote:
>
> 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?)

of_platform_default_populate_init() should work.