linux-kernel - Re: [PATCH] ARM: at91: remove unnecessary of_platform_default

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: <CAL_JsqKzv4=Vpej3kbjMBkFFOpJE_KQRgYr=Svftd_xs-Bj3-g@mail.gmail.com>
Date:   Wed, 11 Jul 2018 12:13:17 -0600
From:   Rob Herring <robh@...nel.org>
To:     Alexandre Belloni <alexandre.belloni@...tlin.com>
Cc:     Nicolas Ferre <nicolas.ferre@...rochip.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH] ARM: at91: remove unnecessary of_platform_default_populate
 calls

On Wed, Jul 11, 2018 at 10:15 AM Alexandre Belloni
<alexandre.belloni@...tlin.com> wrote:
>
> Hi,
>
> On 09/07/2018 09:50:47-0600, Rob Herring wrote:
> > On Tue, Jun 19, 2018 at 3:40 PM Rob Herring <robh@...nel.org> wrote:
> > >
> > > The DT core will call of_platform_default_populate, so it is not
> > > necessary for machine specific code to call it unless there are custom
> > > match entries, auxdata or parent device. Neither of those apply here, so
> > > remove the call.
> > >
> > > Cc: Nicolas Ferre <nicolas.ferre@...rochip.com>
> > > Cc: Alexandre Belloni <alexandre.belloni@...tlin.com>
> > > Cc: linux-arm-kernel@...ts.infradead.org
> > > Signed-off-by: Rob Herring <robh@...nel.org>
> > > ---
> > >  arch/arm/mach-at91/at91rm9200.c | 5 -----
> > >  arch/arm/mach-at91/at91sam9.c   | 5 -----
> > >  arch/arm/mach-at91/sama5.c      | 5 -----
> > >  3 files changed, 15 deletions(-)
> >
> > Ping?
> >
>
> This breaks the platform in two different ways:
>  - PM is not working anymore because of the missing SRAM node
>  - the pinctrl driver fails to probe and so many drivers are
>    deferring the probe forever
>
> Relevant messages (once the earlycon crap is removed to let earlyprintk
> do its job):
>
> at91_pm_sram_init: failed to find sram device!
> AT91: PM not supported, due to no SRAM allocated

So the at91_pm_sram_init function tries to get SRAM platform device,
but it doesn't exist yet. Of course, that is fragile because while the
device may exist, it's just luck that it's driver has probed already.
Would using .init_late hook instead of .init_machine work for you?

Ideally, couldn't much of this code be converted to a driver? It's a
bit strange for initcall code to have a driver dependency.

>
> pinctrl-at91 ahb:apb:pinctrl@...6a000: you need to specify at least one gpio-controller
> pinctrl-at91: probe of ahb:apb:pinctrl@...6a000 failed with error -22

So this one has the strange dependency that the child nodes probe
before the parent node. That's backwards. Probe order is probably
changing from link order (all the devices are created before drivers
register) to device creation order. I think the fix is the pinctrl
driver should just count the gpio child nodes rather than relying on
aliases (which I'm not a fan of either). I can write a patch to do
that.

Rob