2015-04-16 00:52:28

by Florian Fainelli

[permalink] [raw]
Subject: Allowing reset controllers before SMP initialization (on ARM)?

Hi,

In order to support initialization of the secondary core on BCM63138
SoCs, I would want to utilize a reset controller to release the
secondary CPU from reset [1].

Here are multiple options:

- expose a custom function which registers the reset controller platform
driver as early as possible, which is probably acceptable, but also
requires the DT machine descriptor to populate the platform bus earlier,
which we could completely avoid

- have a OF_DECLARE_RESET_CONTROLLER() which is running fairly early
during boot, such that we can utilize reset controllers are early as
possible, before any initcall level, and before SMP initialization is
kicking in

- since the code that boots secondary CPUs is relatively unique, even
within the scope of the reset controllers (sequence involves touching
multiple registers), pulling it outside of the reset controller might be
acceptable (there is still some level of sharing though for low-level
indirect read/write operations)

At this point I am leaning towards option 1) since it still allows the
reset controller abstraction to be used, however, option 3) might
wind-up being the cleanest approach to keep the reset controller small.
At any rate, here is the WIP implementation:

[1]:
https://github.com/ffainelli/linux/commit/133214be43b94b90fd580aa9467a5974ffd989ca

Thank you!
--
Florian


2015-04-16 08:05:10

by Arnd Bergmann

[permalink] [raw]
Subject: Re: Allowing reset controllers before SMP initialization (on ARM)?

On Wednesday 15 April 2015 17:51:18 Florian Fainelli wrote:
> Hi,
>
> In order to support initialization of the secondary core on BCM63138
> SoCs, I would want to utilize a reset controller to release the
> secondary CPU from reset [1].
>
> Here are multiple options:
>
> - expose a custom function which registers the reset controller platform
> driver as early as possible, which is probably acceptable, but also
> requires the DT machine descriptor to populate the platform bus earlier,
> which we could completely avoid

I think populating the platform bus earlier is not realistic, that
would break lots of existing dependencies. In particular, we can't
do it much earlier because it has to be done after the platform bus
itself is instantiated.

> - have a OF_DECLARE_RESET_CONTROLLER() which is running fairly early
> during boot, such that we can utilize reset controllers are early as
> possible, before any initcall level, and before SMP initialization is
> kicking in

We've added a couple of those, and it could be done here, but putting
them in the right order is a bit tricky, and I think we can avoid it.

> - since the code that boots secondary CPUs is relatively unique, even
> within the scope of the reset controllers (sequence involves touching
> multiple registers), pulling it outside of the reset controller might be
> acceptable (there is still some level of sharing though for low-level
> indirect read/write operations)

Yes, it's a hack, but the SMP code is rather special and a lot of
other platforms do similar hacks already. What I'd like to see
happen in the long run is more along the lines of:

- Avoid dependencies on the early SMP code, and just set up the
data structures for possible CPUs early, which can be done without
any hardware interaction. Then move the actual CPU enable path
much later in the boot process, possibly combined with the cpuidle
driver.

Arnd

2015-04-16 09:43:05

by Maxime Coquelin

[permalink] [raw]
Subject: Re: Allowing reset controllers before SMP initialization (on ARM)?

Hi Florian, Arnd,

On 04/16/2015 10:04 AM, Arnd Bergmann wrote:
> On Wednesday 15 April 2015 17:51:18 Florian Fainelli wrote:
>> Hi,
>>
>> In order to support initialization of the secondary core on BCM63138
>> SoCs, I would want to utilize a reset controller to release the
>> secondary CPU from reset [1].
>>
>> Here are multiple options:
>>
>> - expose a custom function which registers the reset controller platform
>> driver as early as possible, which is probably acceptable, but also
>> requires the DT machine descriptor to populate the platform bus earlier,
>> which we could completely avoid
> I think populating the platform bus earlier is not realistic, that
> would break lots of existing dependencies. In particular, we can't
> do it much earlier because it has to be done after the platform bus
> itself is instantiated.
>
>> - have a OF_DECLARE_RESET_CONTROLLER() which is running fairly early
>> during boot, such that we can utilize reset controllers are early as
>> possible, before any initcall level, and before SMP initialization is
>> kicking in
> We've added a couple of those, and it could be done here, but putting
> them in the right order is a bit tricky, and I think we can avoid it.

I have already proposed a OF_DECLARE_RESET_CONTROLLER() implementation:
https://lkml.org/lkml/2015/2/20/395

I needed it for the STM32 timers, but it was not accepted.
Now, I perform the timers reset in the bootloader, but it shouldn't work
in your case.

Kind regards,
Maxime

2015-04-16 18:05:01

by Florian Fainelli

[permalink] [raw]
Subject: Re: Allowing reset controllers before SMP initialization (on ARM)?

2015-04-16 1:04 GMT-07:00 Arnd Bergmann <[email protected]>:
> On Wednesday 15 April 2015 17:51:18 Florian Fainelli wrote:
>> Hi,
>>
>> In order to support initialization of the secondary core on BCM63138
>> SoCs, I would want to utilize a reset controller to release the
>> secondary CPU from reset [1].
>>
>> Here are multiple options:
>>
>> - expose a custom function which registers the reset controller platform
>> driver as early as possible, which is probably acceptable, but also
>> requires the DT machine descriptor to populate the platform bus earlier,
>> which we could completely avoid
>
> I think populating the platform bus earlier is not realistic, that
> would break lots of existing dependencies. In particular, we can't
> do it much earlier because it has to be done after the platform bus
> itself is instantiated.

Agreed, I don't quite like that either.

>
>> - have a OF_DECLARE_RESET_CONTROLLER() which is running fairly early
>> during boot, such that we can utilize reset controllers are early as
>> possible, before any initcall level, and before SMP initialization is
>> kicking in
>
> We've added a couple of those, and it could be done here, but putting
> them in the right order is a bit tricky, and I think we can avoid it.

Right, not only that, but it appears that the reset controller binding
has not standardized a "reset-controller" property, so there is any
good way to scan for all these kinds of controllers in a given Device
Tree today, other than looking at a #reset-cells property
presence/absence.

>
>> - since the code that boots secondary CPUs is relatively unique, even
>> within the scope of the reset controllers (sequence involves touching
>> multiple registers), pulling it outside of the reset controller might be
>> acceptable (there is still some level of sharing though for low-level
>> indirect read/write operations)
>
> Yes, it's a hack, but the SMP code is rather special and a lot of
> other platforms do similar hacks already. What I'd like to see
> happen in the long run is more along the lines of:

Right, and even within the context of 63138, bringing-up the secondary
CPU is about 10 times more lines of code, specific to the CPU complex,
that are not even shared with the reset of the on-chip peripherals, so
I can probably stick the low-level routines in a header file and share
them with not too much gymnastic.

>
> - Avoid dependencies on the early SMP code, and just set up the
> data structures for possible CPUs early, which can be done without
> any hardware interaction. Then move the actual CPU enable path
> much later in the boot process, possibly combined with the cpuidle
> driver.

That sounds like a nice goal, I suppose that as we start standardizing
on the firmware interface to bring-up secondary cores, that might
become easier as well.

Thanks for your feedback!
--
Florian

2015-04-18 19:39:27

by Arnd Bergmann

[permalink] [raw]
Subject: Re: Allowing reset controllers before SMP initialization (on ARM)?

On Thursday 16 April 2015 11:42:08 Maxime Coquelin wrote:
> Hi Florian, Arnd,
>
> On 04/16/2015 10:04 AM, Arnd Bergmann wrote:
> > On Wednesday 15 April 2015 17:51:18 Florian Fainelli wrote:
> >> Hi,
> >>
> >> In order to support initialization of the secondary core on BCM63138
> >> SoCs, I would want to utilize a reset controller to release the
> >> secondary CPU from reset [1].
> >>
> >> Here are multiple options:
> >>
> >> - expose a custom function which registers the reset controller platform
> >> driver as early as possible, which is probably acceptable, but also
> >> requires the DT machine descriptor to populate the platform bus earlier,
> >> which we could completely avoid
> > I think populating the platform bus earlier is not realistic, that
> > would break lots of existing dependencies. In particular, we can't
> > do it much earlier because it has to be done after the platform bus
> > itself is instantiated.
> >
> >> - have a OF_DECLARE_RESET_CONTROLLER() which is running fairly early
> >> during boot, such that we can utilize reset controllers are early as
> >> possible, before any initcall level, and before SMP initialization is
> >> kicking in
> > We've added a couple of those, and it could be done here, but putting
> > them in the right order is a bit tricky, and I think we can avoid it.
>
> I have already proposed a OF_DECLARE_RESET_CONTROLLER() implementation:
> https://lkml.org/lkml/2015/2/20/395
>
> I needed it for the STM32 timers, but it was not accepted.
> Now, I perform the timers reset in the bootloader, but it shouldn't work
> in your case.

I guess if there is enough interest in this, we will eventually do it.
This is something we can change at any time without user-visible changes
or DT binding changes. The tradeoff is mainly that without
OF_DECLARE_RESET_CONTROLLER(), we need a couple of hacks in early init
code, while adding too many of those OF_DECLARE_*() macros has the risk
that we won't be able to order them in a sane way that works on
all platforms, so I'm always reluctant about adding an extra one.

Arnd