Re: [PATCH] amba: Retry adding deferred devices at late_initcall

From: Marek Szyprowski <m.szyprowski@samsung.com>
To: Linus Walleij <linus.walleij@linaro.org>, Rob Herring <robh@kernel.org>
Cc: Saravana Kannan <saravanak@google.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Russell King <linux@armlinux.org.uk>,
	John Stultz <john.stultz@linaro.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Subject: Re: [PATCH] amba: Retry adding deferred devices at late_initcall
Date: Wed, 29 Apr 2020 08:06:01 +0200	[thread overview]
Message-ID: <733e20b1-9592-6941-766b-9f321ad2ace5@samsung.com> (raw)
In-Reply-To: <CACRpkdZSPb8FxTSt8F3F1VcsTM4qG=6gxz1pBPjTZ0Dk2iVfSQ@mail.gmail.com>

Hi Linus,

On 28.04.2020 22:39, Linus Walleij wrote:
> On Mon, Apr 27, 2020 at 11:25 PM Rob Herring <robh@kernel.org> wrote:
>> If amba bus devices defer when adding, the amba bus code simply retries
>> adding the devices every 5 seconds. This doesn't work well as it
>> completely unsynchronized with starting the init process which can
>> happen in less than 5 secs. Add a retry during late_initcall. If the
>> amba devices are added, then deferred probe takes over. If the
>> dependencies have not probed at this point, then there's no improvement
>> over previous behavior. To completely solve this, we'd need to retry
>> after every successful probe as deferred probe does.
>>
>> The list_empty() check now happens outside the mutex, but the mutex
>> wasn't necessary in the first place.
>>
>> This needed to use deferred probe instead of fragile initcall ordering
>> on 32-bit VExpress systems where the apb_pclk has a number of probe
>> dependencies (vexpress-sysregs, vexpress-config).
>>
>> Cc: John Stultz <john.stultz@linaro.org>
>> Cc: Saravana Kannan <saravanak@google.com>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Sudeep Holla <sudeep.holla@arm.com>
>> Cc: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>> Cc: Russell King <linux@armlinux.org.uk>
>> Signed-off-by: Rob Herring <robh@kernel.org>
> Makes sense to me, and the same approach is found
> in the generic code in drivers/base/dd.c so
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> The timer-based re-probe was added by Marek Szyprowski
> in commit a41980f2a3eb33ed7a2636e83498b47e95ceb05b
> do deal with power domains. I guess it mimics dd.c
> deferred probe at this point?
>
> There are a bit of other differences that have piled up,
> should we take a quick look at dd.c so there is not something
> else we're missing? I see some PM code for example.

Well, late initcall based approach is what earlier version of my patch did:

https://lkml.org/lkml/2016/4/12/414

but then it has been requested to solve the issue 'properly':

https://lkml.org/lkml/2016/4/12/455

https://lkml.org/lkml/2016/4/14/875

For me it is fine to get back to late initcall based solution, though.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel