Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Geert Uytterhoeven]

On Fri, Apr 27, 2018 at 2:40 PM, Arnd Bergmann  wrote:
> On Fri, Apr 27, 2018 at 1:53 PM, Bartosz Golaszewski  wrote:
>> 2018-04-27 12:18 GMT+02:00 Arnd Bergmann :
>>> On Fri, Apr 27, 2018 at 10:57 AM, Bartosz Golaszewski
>>>  wrote:
 2018-04-27 9:52 GMT+02:00 Arnd Bergmann :
> On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  
> wrote:
 My patch tries to address exactly the use cases we're facing - for
 example by providing means to probe devices twice (early and late) and
 to check the state we're at in order for users to be able to just do
 the critical initialization early on and then continue with regular
 stuff later.
>>>
>>> Maybe the problem is reusing the name and some of the code from
>>> an existing functionality that we've been trying to get rid of.
>>>
>>
>> I'm not reusing the name - in fact I set the prefix to earlydev_
>> exactly in order to not confuse anyone. I'm also not reusing any code
>> in the second series.
>
> Ok.
>
>>> If what you want to do is completely different from the existing
>>> early_platform implementation, how about starting by moving that
>>> out of drivers/base/platform.c into something under arch/sh/
>>> and renaming it to something with an sh_ prefix.
>>>
>>
>> Yes, this is a good idea, but what about the sh-specific drivers that
>> rely on it? Is including headers from arch/ in driver code still an
>> accepted practice?
>
> I think it's fine here, since we're just move it out of the way and
> there are only very few drivers using it:
>
> drivers/clocksource/sh_cmt.c:early_platform_init("earlytimer",
> _cmt_device_driver);
> drivers/clocksource/sh_mtu2.c:early_platform_init("earlytimer",
> _mtu2_device_driver);
> drivers/clocksource/sh_tmu.c:early_platform_init("earlytimer",
> _tmu_device_driver);
> drivers/clocksource/timer-ti-dm.c:early_platform_init("earlytimer",
> _dm_timer_driver);
> drivers/tty/serial/sh-sci.c:early_platform_init_buffer("earlyprintk",
> _driver,
>
> For timer-ti-dm, it seems like a leftover from old times that can
> be removed. The other four are shared between arch/sh and
> arch/arm/mach-shmobile and already have some #ifdef
> to handle those two cases.

Sh-sci is also used on arm64, for R-Car Gen3 SoCs.
While the latter has CMT and TMU hardware blocks, so far we haven't enabled
support for it yet (just using the ARM arch timer is too convenient? ;-)

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Geert Uytterhoeven]

On Fri, Apr 27, 2018 at 2:40 PM, Arnd Bergmann  wrote:
> On Fri, Apr 27, 2018 at 1:53 PM, Bartosz Golaszewski  wrote:
>> 2018-04-27 12:18 GMT+02:00 Arnd Bergmann :
>>> On Fri, Apr 27, 2018 at 10:57 AM, Bartosz Golaszewski
>>>  wrote:
 2018-04-27 9:52 GMT+02:00 Arnd Bergmann :
> On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  
> wrote:
 My patch tries to address exactly the use cases we're facing - for
 example by providing means to probe devices twice (early and late) and
 to check the state we're at in order for users to be able to just do
 the critical initialization early on and then continue with regular
 stuff later.
>>>
>>> Maybe the problem is reusing the name and some of the code from
>>> an existing functionality that we've been trying to get rid of.
>>>
>>
>> I'm not reusing the name - in fact I set the prefix to earlydev_
>> exactly in order to not confuse anyone. I'm also not reusing any code
>> in the second series.
>
> Ok.
>
>>> If what you want to do is completely different from the existing
>>> early_platform implementation, how about starting by moving that
>>> out of drivers/base/platform.c into something under arch/sh/
>>> and renaming it to something with an sh_ prefix.
>>>
>>
>> Yes, this is a good idea, but what about the sh-specific drivers that
>> rely on it? Is including headers from arch/ in driver code still an
>> accepted practice?
>
> I think it's fine here, since we're just move it out of the way and
> there are only very few drivers using it:
>
> drivers/clocksource/sh_cmt.c:early_platform_init("earlytimer",
> _cmt_device_driver);
> drivers/clocksource/sh_mtu2.c:early_platform_init("earlytimer",
> _mtu2_device_driver);
> drivers/clocksource/sh_tmu.c:early_platform_init("earlytimer",
> _tmu_device_driver);
> drivers/clocksource/timer-ti-dm.c:early_platform_init("earlytimer",
> _dm_timer_driver);
> drivers/tty/serial/sh-sci.c:early_platform_init_buffer("earlyprintk",
> _driver,
>
> For timer-ti-dm, it seems like a leftover from old times that can
> be removed. The other four are shared between arch/sh and
> arch/arm/mach-shmobile and already have some #ifdef
> to handle those two cases.

Sh-sci is also used on arm64, for R-Car Gen3 SoCs.
While the latter has CMT and TMU hardware blocks, so far we haven't enabled
support for it yet (just using the ARM arch timer is too convenient? ;-)

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Arnd Bergmann]

On Fri, Apr 27, 2018 at 6:05 PM, Bartosz Golaszewski
 wrote:
> 2018-04-27 16:48 GMT+02:00 Arnd Bergmann :
>> On Fri, Apr 27, 2018 at 4:05 PM, Bartosz Golaszewski
>
> So speaking in pseudo-C we basically have two ways for an imaginary
> future timer driver:
>
> int foo_probe(struct platform_device *pdev)
> {
> struct clk *clk;
>
> if (probing_early(pdev)) {
> clk = devm_clk_get(dev, "earlyclock");
>
>/* Do early stuff. */
> return 0;
> }
>
> /* Do late stuff. */
>
> return 0;
> }
>
> --- vs ---
>
> int foo_probe(struct platform_device *pdev)
> {
> /* Do late stuff. */
>
> return 0;
> }
>
> static int foo_init(struct device_node *np)
> {
> struct clk *clk;
> struct device *dev = device_from_device_node(np);
>
> /* Do early stuff. */
> clk = devm_clk_get(dev, "earlyclock");
>
> return 0;
> }
>
> TIMER_OF_DECLARE(foo, "bar,foo", foo_init);
>
> I still believe the first approach is easier to implement and has the
> added benefit of supporting board files.

Right. I still like the second approach better, since it avoids
multiplexing two very different code paths into a single
function, and because it's closer to what everyone is used
to at the moment.

Prototyping both is probably helpful to get a better idea
of the actual complexity this introduces.

> I'll give it a thought and will be back at it next week.

Ok. I'll be on vacation for three weeks so I wont' be able
to reply on the new patches.

   Arnd

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Arnd Bergmann]

On Fri, Apr 27, 2018 at 6:05 PM, Bartosz Golaszewski
 wrote:
> 2018-04-27 16:48 GMT+02:00 Arnd Bergmann :
>> On Fri, Apr 27, 2018 at 4:05 PM, Bartosz Golaszewski
>
> So speaking in pseudo-C we basically have two ways for an imaginary
> future timer driver:
>
> int foo_probe(struct platform_device *pdev)
> {
> struct clk *clk;
>
> if (probing_early(pdev)) {
> clk = devm_clk_get(dev, "earlyclock");
>
>/* Do early stuff. */
> return 0;
> }
>
> /* Do late stuff. */
>
> return 0;
> }
>
> --- vs ---
>
> int foo_probe(struct platform_device *pdev)
> {
> /* Do late stuff. */
>
> return 0;
> }
>
> static int foo_init(struct device_node *np)
> {
> struct clk *clk;
> struct device *dev = device_from_device_node(np);
>
> /* Do early stuff. */
> clk = devm_clk_get(dev, "earlyclock");
>
> return 0;
> }
>
> TIMER_OF_DECLARE(foo, "bar,foo", foo_init);
>
> I still believe the first approach is easier to implement and has the
> added benefit of supporting board files.

Right. I still like the second approach better, since it avoids
multiplexing two very different code paths into a single
function, and because it's closer to what everyone is used
to at the moment.

Prototyping both is probably helpful to get a better idea
of the actual complexity this introduces.

> I'll give it a thought and will be back at it next week.

Ok. I'll be on vacation for three weeks so I wont' be able
to reply on the new patches.

   Arnd

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Bartosz Golaszewski]

2018-04-27 16:48 GMT+02:00 Arnd Bergmann :
> On Fri, Apr 27, 2018 at 4:05 PM, Bartosz Golaszewski
>  wrote:
>> 2018-04-27 14:40 GMT+02:00 Arnd Bergmann :
>
>>> For timer-ti-dm, it seems like a leftover from old times that can
>>> be removed. The other four are shared between arch/sh and
>>> arch/arm/mach-shmobile and already have some #ifdef
>>> to handle those two cases.
>>>
>>
>> I'm also seeing that we also call early_platform_cleanup() from
>> platform_bus_init(). Any ideas for this one?
>
> My first idea would be to call it immediately after registering all
> devices and drivers. It looks like it's only needed to make all
> devm_ allocations persistent by removing them from the list,
> so we have to call early_platform_cleanup() before getting
> to the real platform code, but it could be done much earlier
> if we want to, at least after both setup_arch() and sh_late_time_init()
> are complete.
>
>>> I'd rather keep those separate and would prefer not to have
>>> that many different ways of getting there instead. DT and
>>> board files can already share most of the code through the
>>> use of platform_device, especially when you start using
>>> device properties instead of platform_data, and the other
>>> two are rare corner cases and ideally left that way.
>>>
>>> The early boot code is always special and instead of making
>>> it easier to use, we should focus on using it as little as
>>> possible: every line of code that we call before even
>>> initializing timers and consoles means it gets harder to
>>> debug when something goes wrong.
>>>
>>
>> I'm afraid I don't quite understand your reasoning. I fully agree that
>> devices that need to be initialized that early are a rare corner case.
>> We should limit any such uses to the absolute minimum. But when we do
>> need to go this way, we should do it right. Having a unified mechanism
>> for early devices will allow maintainers to enforce good practices
>> (using resources for register mapping, devres, reusing driver code for
>> reading/writing to registers). Having initialization code in machine
>> code will make everybody use different APIs and duplicate solutions. I
>> normally assume that code consolidation is always good.
>>
>> If we add a way for DT-based platform devices to be probed early - it
>> would be based on platform device/driver structures anyway. Why would
>> we even want to not convert the board code into a simple call to
>> early_platform_device_register() if we'll already offer this API for
>> device tree?
>
> I think we first need to define what we really want to achieve here.
> It sounds like you still want to recreate a lot of what early_platform
> devices do, but it seems more important to me to add the missing
> functionality to the OF_DECLARE infrastructure. The most
> important pieces that seem to be missing are solved by finding
> a way to provide a platform_device pointer with the following
> properties:
>
> - allow being passed into dev_print()
> - allow using the pointer as a token for devres unwinding
> - access to device_private data that remains persistent
>   until real a platform_driver gets loaded
>
> That can probably be done as an extension to the current
> infrastructure.
>
> However, I'd be very cautious about the resource portion:
> filling the platform resources (registers, irqs, ...) the way
> we do for regular devices is much harder and can introduce
> additional (or circular) dependencies on other devices.
> OTOH, not using those resources means you have a hard
> time passing information from board files.
>
>   Arnd

So speaking in pseudo-C we basically have two ways for an imaginary
future timer driver:

int foo_probe(struct platform_device *pdev)
{
struct clk *clk;

if (probing_early(pdev)) {
clk = devm_clk_get(dev, "earlyclock");

   /* Do early stuff. */
return 0;
}

/* Do late stuff. */

return 0;
}

--- vs ---

int foo_probe(struct platform_device *pdev)
{
/* Do late stuff. */

return 0;
}

static int foo_init(struct device_node *np)
{
struct clk *clk;
struct device *dev = device_from_device_node(np);

/* Do early stuff. */
clk = devm_clk_get(dev, "earlyclock");

return 0;
}

TIMER_OF_DECLARE(foo, "bar,foo", foo_init);

I still believe the first approach is easier to implement and has the
added benefit of supporting board files.

I'll give it a thought and will be back at it next week.

Best regards,
Bartosz Golaszewski

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Bartosz Golaszewski]

2018-04-27 16:48 GMT+02:00 Arnd Bergmann :
> On Fri, Apr 27, 2018 at 4:05 PM, Bartosz Golaszewski
>  wrote:
>> 2018-04-27 14:40 GMT+02:00 Arnd Bergmann :
>
>>> For timer-ti-dm, it seems like a leftover from old times that can
>>> be removed. The other four are shared between arch/sh and
>>> arch/arm/mach-shmobile and already have some #ifdef
>>> to handle those two cases.
>>>
>>
>> I'm also seeing that we also call early_platform_cleanup() from
>> platform_bus_init(). Any ideas for this one?
>
> My first idea would be to call it immediately after registering all
> devices and drivers. It looks like it's only needed to make all
> devm_ allocations persistent by removing them from the list,
> so we have to call early_platform_cleanup() before getting
> to the real platform code, but it could be done much earlier
> if we want to, at least after both setup_arch() and sh_late_time_init()
> are complete.
>
>>> I'd rather keep those separate and would prefer not to have
>>> that many different ways of getting there instead. DT and
>>> board files can already share most of the code through the
>>> use of platform_device, especially when you start using
>>> device properties instead of platform_data, and the other
>>> two are rare corner cases and ideally left that way.
>>>
>>> The early boot code is always special and instead of making
>>> it easier to use, we should focus on using it as little as
>>> possible: every line of code that we call before even
>>> initializing timers and consoles means it gets harder to
>>> debug when something goes wrong.
>>>
>>
>> I'm afraid I don't quite understand your reasoning. I fully agree that
>> devices that need to be initialized that early are a rare corner case.
>> We should limit any such uses to the absolute minimum. But when we do
>> need to go this way, we should do it right. Having a unified mechanism
>> for early devices will allow maintainers to enforce good practices
>> (using resources for register mapping, devres, reusing driver code for
>> reading/writing to registers). Having initialization code in machine
>> code will make everybody use different APIs and duplicate solutions. I
>> normally assume that code consolidation is always good.
>>
>> If we add a way for DT-based platform devices to be probed early - it
>> would be based on platform device/driver structures anyway. Why would
>> we even want to not convert the board code into a simple call to
>> early_platform_device_register() if we'll already offer this API for
>> device tree?
>
> I think we first need to define what we really want to achieve here.
> It sounds like you still want to recreate a lot of what early_platform
> devices do, but it seems more important to me to add the missing
> functionality to the OF_DECLARE infrastructure. The most
> important pieces that seem to be missing are solved by finding
> a way to provide a platform_device pointer with the following
> properties:
>
> - allow being passed into dev_print()
> - allow using the pointer as a token for devres unwinding
> - access to device_private data that remains persistent
>   until real a platform_driver gets loaded
>
> That can probably be done as an extension to the current
> infrastructure.
>
> However, I'd be very cautious about the resource portion:
> filling the platform resources (registers, irqs, ...) the way
> we do for regular devices is much harder and can introduce
> additional (or circular) dependencies on other devices.
> OTOH, not using those resources means you have a hard
> time passing information from board files.
>
>   Arnd

So speaking in pseudo-C we basically have two ways for an imaginary
future timer driver:

int foo_probe(struct platform_device *pdev)
{
struct clk *clk;

if (probing_early(pdev)) {
clk = devm_clk_get(dev, "earlyclock");

   /* Do early stuff. */
return 0;
}

/* Do late stuff. */

return 0;
}

--- vs ---

int foo_probe(struct platform_device *pdev)
{
/* Do late stuff. */

return 0;
}

static int foo_init(struct device_node *np)
{
struct clk *clk;
struct device *dev = device_from_device_node(np);

/* Do early stuff. */
clk = devm_clk_get(dev, "earlyclock");

return 0;
}

TIMER_OF_DECLARE(foo, "bar,foo", foo_init);

I still believe the first approach is easier to implement and has the
added benefit of supporting board files.

I'll give it a thought and will be back at it next week.

Best regards,
Bartosz Golaszewski

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Rich Felker]

On Fri, Apr 27, 2018 at 02:40:34PM +0200, Arnd Bergmann wrote:
> On Fri, Apr 27, 2018 at 1:53 PM, Bartosz Golaszewski  wrote:
> > 2018-04-27 12:18 GMT+02:00 Arnd Bergmann :
> >> On Fri, Apr 27, 2018 at 10:57 AM, Bartosz Golaszewski
> >>  wrote:
> >>> 2018-04-27 9:52 GMT+02:00 Arnd Bergmann :
>  On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  
>  wrote:
> >>> My patch tries to address exactly the use cases we're facing - for
> >>> example by providing means to probe devices twice (early and late) and
> >>> to check the state we're at in order for users to be able to just do
> >>> the critical initialization early on and then continue with regular
> >>> stuff later.
> >>
> >> Maybe the problem is reusing the name and some of the code from
> >> an existing functionality that we've been trying to get rid of.
> >>
> >
> > I'm not reusing the name - in fact I set the prefix to earlydev_
> > exactly in order to not confuse anyone. I'm also not reusing any code
> > in the second series.
> 
> Ok.
> 
> >> If what you want to do is completely different from the existing
> >> early_platform implementation, how about starting by moving that
> >> out of drivers/base/platform.c into something under arch/sh/
> >> and renaming it to something with an sh_ prefix.
> >>
> >
> > Yes, this is a good idea, but what about the sh-specific drivers that
> > rely on it? Is including headers from arch/ in driver code still an
> > accepted practice?
> 
> I think it's fine here, since we're just move it out of the way and
> there are only very few drivers using it:
> 
> drivers/clocksource/sh_cmt.c:early_platform_init("earlytimer",
> _cmt_device_driver);
> drivers/clocksource/sh_mtu2.c:early_platform_init("earlytimer",
> _mtu2_device_driver);
> drivers/clocksource/sh_tmu.c:early_platform_init("earlytimer",
> _tmu_device_driver);
> drivers/clocksource/timer-ti-dm.c:early_platform_init("earlytimer",
> _dm_timer_driver);
> drivers/tty/serial/sh-sci.c:early_platform_init_buffer("earlyprintk",
> _driver,
> 
> For timer-ti-dm, it seems like a leftover from old times that can
> be removed. The other four are shared between arch/sh and
> arch/arm/mach-shmobile and already have some #ifdef
> to handle those two cases.

FWIW if arch/sh is the only remaining user of the earlyprintk part,
please don't go to any trouble to preserve it. I want to move to
earlycon exclusively. Not sure if that is possible before moving the
boards that are using it to device tree; if so and it's easy, please
consider sending a patch or a sketch of what it should look like so I
can do it.

Rich

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Rich Felker]

On Fri, Apr 27, 2018 at 02:40:34PM +0200, Arnd Bergmann wrote:
> On Fri, Apr 27, 2018 at 1:53 PM, Bartosz Golaszewski  wrote:
> > 2018-04-27 12:18 GMT+02:00 Arnd Bergmann :
> >> On Fri, Apr 27, 2018 at 10:57 AM, Bartosz Golaszewski
> >>  wrote:
> >>> 2018-04-27 9:52 GMT+02:00 Arnd Bergmann :
>  On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  
>  wrote:
> >>> My patch tries to address exactly the use cases we're facing - for
> >>> example by providing means to probe devices twice (early and late) and
> >>> to check the state we're at in order for users to be able to just do
> >>> the critical initialization early on and then continue with regular
> >>> stuff later.
> >>
> >> Maybe the problem is reusing the name and some of the code from
> >> an existing functionality that we've been trying to get rid of.
> >>
> >
> > I'm not reusing the name - in fact I set the prefix to earlydev_
> > exactly in order to not confuse anyone. I'm also not reusing any code
> > in the second series.
> 
> Ok.
> 
> >> If what you want to do is completely different from the existing
> >> early_platform implementation, how about starting by moving that
> >> out of drivers/base/platform.c into something under arch/sh/
> >> and renaming it to something with an sh_ prefix.
> >>
> >
> > Yes, this is a good idea, but what about the sh-specific drivers that
> > rely on it? Is including headers from arch/ in driver code still an
> > accepted practice?
> 
> I think it's fine here, since we're just move it out of the way and
> there are only very few drivers using it:
> 
> drivers/clocksource/sh_cmt.c:early_platform_init("earlytimer",
> _cmt_device_driver);
> drivers/clocksource/sh_mtu2.c:early_platform_init("earlytimer",
> _mtu2_device_driver);
> drivers/clocksource/sh_tmu.c:early_platform_init("earlytimer",
> _tmu_device_driver);
> drivers/clocksource/timer-ti-dm.c:early_platform_init("earlytimer",
> _dm_timer_driver);
> drivers/tty/serial/sh-sci.c:early_platform_init_buffer("earlyprintk",
> _driver,
> 
> For timer-ti-dm, it seems like a leftover from old times that can
> be removed. The other four are shared between arch/sh and
> arch/arm/mach-shmobile and already have some #ifdef
> to handle those two cases.

FWIW if arch/sh is the only remaining user of the earlyprintk part,
please don't go to any trouble to preserve it. I want to move to
earlycon exclusively. Not sure if that is possible before moving the
boards that are using it to device tree; if so and it's easy, please
consider sending a patch or a sketch of what it should look like so I
can do it.

Rich

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Bartosz Golaszewski]

2018-04-27 14:40 GMT+02:00 Arnd Bergmann :
> On Fri, Apr 27, 2018 at 1:53 PM, Bartosz Golaszewski  wrote:
>> 2018-04-27 12:18 GMT+02:00 Arnd Bergmann :
>>> On Fri, Apr 27, 2018 at 10:57 AM, Bartosz Golaszewski
>>>  wrote:
 2018-04-27 9:52 GMT+02:00 Arnd Bergmann :
> On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  
> wrote:
 My patch tries to address exactly the use cases we're facing - for
 example by providing means to probe devices twice (early and late) and
 to check the state we're at in order for users to be able to just do
 the critical initialization early on and then continue with regular
 stuff later.
>>>
>>> Maybe the problem is reusing the name and some of the code from
>>> an existing functionality that we've been trying to get rid of.
>>>
>>
>> I'm not reusing the name - in fact I set the prefix to earlydev_
>> exactly in order to not confuse anyone. I'm also not reusing any code
>> in the second series.
>
> Ok.
>
>>> If what you want to do is completely different from the existing
>>> early_platform implementation, how about starting by moving that
>>> out of drivers/base/platform.c into something under arch/sh/
>>> and renaming it to something with an sh_ prefix.
>>>
>>
>> Yes, this is a good idea, but what about the sh-specific drivers that
>> rely on it? Is including headers from arch/ in driver code still an
>> accepted practice?
>
> I think it's fine here, since we're just move it out of the way and
> there are only very few drivers using it:
>
> drivers/clocksource/sh_cmt.c:early_platform_init("earlytimer",
> _cmt_device_driver);
> drivers/clocksource/sh_mtu2.c:early_platform_init("earlytimer",
> _mtu2_device_driver);
> drivers/clocksource/sh_tmu.c:early_platform_init("earlytimer",
> _tmu_device_driver);
> drivers/clocksource/timer-ti-dm.c:early_platform_init("earlytimer",
> _dm_timer_driver);
> drivers/tty/serial/sh-sci.c:early_platform_init_buffer("earlyprintk",
> _driver,
>
> For timer-ti-dm, it seems like a leftover from old times that can
> be removed. The other four are shared between arch/sh and
> arch/arm/mach-shmobile and already have some #ifdef
> to handle those two cases.
>

I'm also seeing that we also call early_platform_cleanup() from
platform_bus_init(). Any ideas for this one?

>>> Let's just leave the non-DT part out of it by making it sh specific.
>>> Then we can come up with improvements to the current
>>> platform_device handling for DT based platforms that you can
>>> use on DT-based davinci to replace what currently happens on
>>> board-file based davinci systems, without mixing up those
>>> two code paths too much in the base driver support.
>>
>> I don't see why we wouldn't want to unify these two cases. The best
>> solution to me seems having only one entry point into the driver code
>> - the probe() function stored in platform_driver - whether we're
>> probing it from DT, platform data, ACPI or early boot code.
>
> I'd rather keep those separate and would prefer not to have
> that many different ways of getting there instead. DT and
> board files can already share most of the code through the
> use of platform_device, especially when you start using
> device properties instead of platform_data, and the other
> two are rare corner cases and ideally left that way.
>
> The early boot code is always special and instead of making
> it easier to use, we should focus on using it as little as
> possible: every line of code that we call before even
> initializing timers and consoles means it gets harder to
> debug when something goes wrong.
>

I'm afraid I don't quite understand your reasoning. I fully agree that
devices that need to be initialized that early are a rare corner case.
We should limit any such uses to the absolute minimum. But when we do
need to go this way, we should do it right. Having a unified mechanism
for early devices will allow maintainers to enforce good practices
(using resources for register mapping, devres, reusing driver code for
reading/writing to registers). Having initialization code in machine
code will make everybody use different APIs and duplicate solutions. I
normally assume that code consolidation is always good.

If we add a way for DT-based platform devices to be probed early - it
would be based on platform device/driver structures anyway. Why would
we even want to not convert the board code into a simple call to
early_platform_device_register() if we'll already offer this API for
device tree?

Best regards,
Bartosz Golaszewski

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Bartosz Golaszewski]

2018-04-27 14:40 GMT+02:00 Arnd Bergmann :
> On Fri, Apr 27, 2018 at 1:53 PM, Bartosz Golaszewski  wrote:
>> 2018-04-27 12:18 GMT+02:00 Arnd Bergmann :
>>> On Fri, Apr 27, 2018 at 10:57 AM, Bartosz Golaszewski
>>>  wrote:
 2018-04-27 9:52 GMT+02:00 Arnd Bergmann :
> On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  
> wrote:
 My patch tries to address exactly the use cases we're facing - for
 example by providing means to probe devices twice (early and late) and
 to check the state we're at in order for users to be able to just do
 the critical initialization early on and then continue with regular
 stuff later.
>>>
>>> Maybe the problem is reusing the name and some of the code from
>>> an existing functionality that we've been trying to get rid of.
>>>
>>
>> I'm not reusing the name - in fact I set the prefix to earlydev_
>> exactly in order to not confuse anyone. I'm also not reusing any code
>> in the second series.
>
> Ok.
>
>>> If what you want to do is completely different from the existing
>>> early_platform implementation, how about starting by moving that
>>> out of drivers/base/platform.c into something under arch/sh/
>>> and renaming it to something with an sh_ prefix.
>>>
>>
>> Yes, this is a good idea, but what about the sh-specific drivers that
>> rely on it? Is including headers from arch/ in driver code still an
>> accepted practice?
>
> I think it's fine here, since we're just move it out of the way and
> there are only very few drivers using it:
>
> drivers/clocksource/sh_cmt.c:early_platform_init("earlytimer",
> _cmt_device_driver);
> drivers/clocksource/sh_mtu2.c:early_platform_init("earlytimer",
> _mtu2_device_driver);
> drivers/clocksource/sh_tmu.c:early_platform_init("earlytimer",
> _tmu_device_driver);
> drivers/clocksource/timer-ti-dm.c:early_platform_init("earlytimer",
> _dm_timer_driver);
> drivers/tty/serial/sh-sci.c:early_platform_init_buffer("earlyprintk",
> _driver,
>
> For timer-ti-dm, it seems like a leftover from old times that can
> be removed. The other four are shared between arch/sh and
> arch/arm/mach-shmobile and already have some #ifdef
> to handle those two cases.
>

I'm also seeing that we also call early_platform_cleanup() from
platform_bus_init(). Any ideas for this one?

>>> Let's just leave the non-DT part out of it by making it sh specific.
>>> Then we can come up with improvements to the current
>>> platform_device handling for DT based platforms that you can
>>> use on DT-based davinci to replace what currently happens on
>>> board-file based davinci systems, without mixing up those
>>> two code paths too much in the base driver support.
>>
>> I don't see why we wouldn't want to unify these two cases. The best
>> solution to me seems having only one entry point into the driver code
>> - the probe() function stored in platform_driver - whether we're
>> probing it from DT, platform data, ACPI or early boot code.
>
> I'd rather keep those separate and would prefer not to have
> that many different ways of getting there instead. DT and
> board files can already share most of the code through the
> use of platform_device, especially when you start using
> device properties instead of platform_data, and the other
> two are rare corner cases and ideally left that way.
>
> The early boot code is always special and instead of making
> it easier to use, we should focus on using it as little as
> possible: every line of code that we call before even
> initializing timers and consoles means it gets harder to
> debug when something goes wrong.
>

I'm afraid I don't quite understand your reasoning. I fully agree that
devices that need to be initialized that early are a rare corner case.
We should limit any such uses to the absolute minimum. But when we do
need to go this way, we should do it right. Having a unified mechanism
for early devices will allow maintainers to enforce good practices
(using resources for register mapping, devres, reusing driver code for
reading/writing to registers). Having initialization code in machine
code will make everybody use different APIs and duplicate solutions. I
normally assume that code consolidation is always good.

If we add a way for DT-based platform devices to be probed early - it
would be based on platform device/driver structures anyway. Why would
we even want to not convert the board code into a simple call to
early_platform_device_register() if we'll already offer this API for
device tree?

Best regards,
Bartosz Golaszewski

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Arnd Bergmann]

On Fri, Apr 27, 2018 at 4:05 PM, Bartosz Golaszewski
 wrote:
> 2018-04-27 14:40 GMT+02:00 Arnd Bergmann :

>> For timer-ti-dm, it seems like a leftover from old times that can
>> be removed. The other four are shared between arch/sh and
>> arch/arm/mach-shmobile and already have some #ifdef
>> to handle those two cases.
>>
>
> I'm also seeing that we also call early_platform_cleanup() from
> platform_bus_init(). Any ideas for this one?

My first idea would be to call it immediately after registering all
devices and drivers. It looks like it's only needed to make all
devm_ allocations persistent by removing them from the list,
so we have to call early_platform_cleanup() before getting
to the real platform code, but it could be done much earlier
if we want to, at least after both setup_arch() and sh_late_time_init()
are complete.

>> I'd rather keep those separate and would prefer not to have
>> that many different ways of getting there instead. DT and
>> board files can already share most of the code through the
>> use of platform_device, especially when you start using
>> device properties instead of platform_data, and the other
>> two are rare corner cases and ideally left that way.
>>
>> The early boot code is always special and instead of making
>> it easier to use, we should focus on using it as little as
>> possible: every line of code that we call before even
>> initializing timers and consoles means it gets harder to
>> debug when something goes wrong.
>>
>
> I'm afraid I don't quite understand your reasoning. I fully agree that
> devices that need to be initialized that early are a rare corner case.
> We should limit any such uses to the absolute minimum. But when we do
> need to go this way, we should do it right. Having a unified mechanism
> for early devices will allow maintainers to enforce good practices
> (using resources for register mapping, devres, reusing driver code for
> reading/writing to registers). Having initialization code in machine
> code will make everybody use different APIs and duplicate solutions. I
> normally assume that code consolidation is always good.
>
> If we add a way for DT-based platform devices to be probed early - it
> would be based on platform device/driver structures anyway. Why would
> we even want to not convert the board code into a simple call to
> early_platform_device_register() if we'll already offer this API for
> device tree?

I think we first need to define what we really want to achieve here.
It sounds like you still want to recreate a lot of what early_platform
devices do, but it seems more important to me to add the missing
functionality to the OF_DECLARE infrastructure. The most
important pieces that seem to be missing are solved by finding
a way to provide a platform_device pointer with the following
properties:

- allow being passed into dev_print()
- allow using the pointer as a token for devres unwinding
- access to device_private data that remains persistent
  until real a platform_driver gets loaded

That can probably be done as an extension to the current
infrastructure.

However, I'd be very cautious about the resource portion:
filling the platform resources (registers, irqs, ...) the way
we do for regular devices is much harder and can introduce
additional (or circular) dependencies on other devices.
OTOH, not using those resources means you have a hard
time passing information from board files.

  Arnd

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Arnd Bergmann]

On Fri, Apr 27, 2018 at 4:05 PM, Bartosz Golaszewski
 wrote:
> 2018-04-27 14:40 GMT+02:00 Arnd Bergmann :

>> For timer-ti-dm, it seems like a leftover from old times that can
>> be removed. The other four are shared between arch/sh and
>> arch/arm/mach-shmobile and already have some #ifdef
>> to handle those two cases.
>>
>
> I'm also seeing that we also call early_platform_cleanup() from
> platform_bus_init(). Any ideas for this one?

My first idea would be to call it immediately after registering all
devices and drivers. It looks like it's only needed to make all
devm_ allocations persistent by removing them from the list,
so we have to call early_platform_cleanup() before getting
to the real platform code, but it could be done much earlier
if we want to, at least after both setup_arch() and sh_late_time_init()
are complete.

>> I'd rather keep those separate and would prefer not to have
>> that many different ways of getting there instead. DT and
>> board files can already share most of the code through the
>> use of platform_device, especially when you start using
>> device properties instead of platform_data, and the other
>> two are rare corner cases and ideally left that way.
>>
>> The early boot code is always special and instead of making
>> it easier to use, we should focus on using it as little as
>> possible: every line of code that we call before even
>> initializing timers and consoles means it gets harder to
>> debug when something goes wrong.
>>
>
> I'm afraid I don't quite understand your reasoning. I fully agree that
> devices that need to be initialized that early are a rare corner case.
> We should limit any such uses to the absolute minimum. But when we do
> need to go this way, we should do it right. Having a unified mechanism
> for early devices will allow maintainers to enforce good practices
> (using resources for register mapping, devres, reusing driver code for
> reading/writing to registers). Having initialization code in machine
> code will make everybody use different APIs and duplicate solutions. I
> normally assume that code consolidation is always good.
>
> If we add a way for DT-based platform devices to be probed early - it
> would be based on platform device/driver structures anyway. Why would
> we even want to not convert the board code into a simple call to
> early_platform_device_register() if we'll already offer this API for
> device tree?

I think we first need to define what we really want to achieve here.
It sounds like you still want to recreate a lot of what early_platform
devices do, but it seems more important to me to add the missing
functionality to the OF_DECLARE infrastructure. The most
important pieces that seem to be missing are solved by finding
a way to provide a platform_device pointer with the following
properties:

- allow being passed into dev_print()
- allow using the pointer as a token for devres unwinding
- access to device_private data that remains persistent
  until real a platform_driver gets loaded

That can probably be done as an extension to the current
infrastructure.

However, I'd be very cautious about the resource portion:
filling the platform resources (registers, irqs, ...) the way
we do for regular devices is much harder and can introduce
additional (or circular) dependencies on other devices.
OTOH, not using those resources means you have a hard
time passing information from board files.

  Arnd

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Arnd Bergmann]

On Fri, Apr 27, 2018 at 1:53 PM, Bartosz Golaszewski  wrote:
> 2018-04-27 12:18 GMT+02:00 Arnd Bergmann :
>> On Fri, Apr 27, 2018 at 10:57 AM, Bartosz Golaszewski
>>  wrote:
>>> 2018-04-27 9:52 GMT+02:00 Arnd Bergmann :
 On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  
 wrote:
>>> My patch tries to address exactly the use cases we're facing - for
>>> example by providing means to probe devices twice (early and late) and
>>> to check the state we're at in order for users to be able to just do
>>> the critical initialization early on and then continue with regular
>>> stuff later.
>>
>> Maybe the problem is reusing the name and some of the code from
>> an existing functionality that we've been trying to get rid of.
>>
>
> I'm not reusing the name - in fact I set the prefix to earlydev_
> exactly in order to not confuse anyone. I'm also not reusing any code
> in the second series.

Ok.

>> If what you want to do is completely different from the existing
>> early_platform implementation, how about starting by moving that
>> out of drivers/base/platform.c into something under arch/sh/
>> and renaming it to something with an sh_ prefix.
>>
>
> Yes, this is a good idea, but what about the sh-specific drivers that
> rely on it? Is including headers from arch/ in driver code still an
> accepted practice?

I think it's fine here, since we're just move it out of the way and
there are only very few drivers using it:

drivers/clocksource/sh_cmt.c:early_platform_init("earlytimer",
_cmt_device_driver);
drivers/clocksource/sh_mtu2.c:early_platform_init("earlytimer",
_mtu2_device_driver);
drivers/clocksource/sh_tmu.c:early_platform_init("earlytimer",
_tmu_device_driver);
drivers/clocksource/timer-ti-dm.c:early_platform_init("earlytimer",
_dm_timer_driver);
drivers/tty/serial/sh-sci.c:early_platform_init_buffer("earlyprintk",
_driver,

For timer-ti-dm, it seems like a leftover from old times that can
be removed. The other four are shared between arch/sh and
arch/arm/mach-shmobile and already have some #ifdef
to handle those two cases.

>> Let's just leave the non-DT part out of it by making it sh specific.
>> Then we can come up with improvements to the current
>> platform_device handling for DT based platforms that you can
>> use on DT-based davinci to replace what currently happens on
>> board-file based davinci systems, without mixing up those
>> two code paths too much in the base driver support.
>
> I don't see why we wouldn't want to unify these two cases. The best
> solution to me seems having only one entry point into the driver code
> - the probe() function stored in platform_driver - whether we're
> probing it from DT, platform data, ACPI or early boot code.

I'd rather keep those separate and would prefer not to have
that many different ways of getting there instead. DT and
board files can already share most of the code through the
use of platform_device, especially when you start using
device properties instead of platform_data, and the other
two are rare corner cases and ideally left that way.

The early boot code is always special and instead of making
it easier to use, we should focus on using it as little as
possible: every line of code that we call before even
initializing timers and consoles means it gets harder to
debug when something goes wrong.

  Arnd

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Arnd Bergmann]

On Fri, Apr 27, 2018 at 1:53 PM, Bartosz Golaszewski  wrote:
> 2018-04-27 12:18 GMT+02:00 Arnd Bergmann :
>> On Fri, Apr 27, 2018 at 10:57 AM, Bartosz Golaszewski
>>  wrote:
>>> 2018-04-27 9:52 GMT+02:00 Arnd Bergmann :
 On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  
 wrote:
>>> My patch tries to address exactly the use cases we're facing - for
>>> example by providing means to probe devices twice (early and late) and
>>> to check the state we're at in order for users to be able to just do
>>> the critical initialization early on and then continue with regular
>>> stuff later.
>>
>> Maybe the problem is reusing the name and some of the code from
>> an existing functionality that we've been trying to get rid of.
>>
>
> I'm not reusing the name - in fact I set the prefix to earlydev_
> exactly in order to not confuse anyone. I'm also not reusing any code
> in the second series.

Ok.

>> If what you want to do is completely different from the existing
>> early_platform implementation, how about starting by moving that
>> out of drivers/base/platform.c into something under arch/sh/
>> and renaming it to something with an sh_ prefix.
>>
>
> Yes, this is a good idea, but what about the sh-specific drivers that
> rely on it? Is including headers from arch/ in driver code still an
> accepted practice?

I think it's fine here, since we're just move it out of the way and
there are only very few drivers using it:

drivers/clocksource/sh_cmt.c:early_platform_init("earlytimer",
_cmt_device_driver);
drivers/clocksource/sh_mtu2.c:early_platform_init("earlytimer",
_mtu2_device_driver);
drivers/clocksource/sh_tmu.c:early_platform_init("earlytimer",
_tmu_device_driver);
drivers/clocksource/timer-ti-dm.c:early_platform_init("earlytimer",
_dm_timer_driver);
drivers/tty/serial/sh-sci.c:early_platform_init_buffer("earlyprintk",
_driver,

For timer-ti-dm, it seems like a leftover from old times that can
be removed. The other four are shared between arch/sh and
arch/arm/mach-shmobile and already have some #ifdef
to handle those two cases.

>> Let's just leave the non-DT part out of it by making it sh specific.
>> Then we can come up with improvements to the current
>> platform_device handling for DT based platforms that you can
>> use on DT-based davinci to replace what currently happens on
>> board-file based davinci systems, without mixing up those
>> two code paths too much in the base driver support.
>
> I don't see why we wouldn't want to unify these two cases. The best
> solution to me seems having only one entry point into the driver code
> - the probe() function stored in platform_driver - whether we're
> probing it from DT, platform data, ACPI or early boot code.

I'd rather keep those separate and would prefer not to have
that many different ways of getting there instead. DT and
board files can already share most of the code through the
use of platform_device, especially when you start using
device properties instead of platform_data, and the other
two are rare corner cases and ideally left that way.

The early boot code is always special and instead of making
it easier to use, we should focus on using it as little as
possible: every line of code that we call before even
initializing timers and consoles means it gets harder to
debug when something goes wrong.

  Arnd

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Bartosz Golaszewski]

2018-04-27 12:18 GMT+02:00 Arnd Bergmann :
> On Fri, Apr 27, 2018 at 10:57 AM, Bartosz Golaszewski
>  wrote:
>> 2018-04-27 9:52 GMT+02:00 Arnd Bergmann :
>>> On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  wrote:
 On 04/26/2018 12:31 PM, Rich Felker wrote:
>>
>> We have platforms out there which still use both board files and
>> device tree. They are still comercially supported and are not going
>> anywhere anytime soon. Some of these platforms are being actively
>> maintained and cleaned-up. An example is the DaVinci platform: David
>> has recently converted all the SoCs and boards to using the common
>> clock framework. I'm cleaning up some other parts too.
>>
>> The problem with the legacy board code is that a lot of things that
>> should be platform drivers ended up in arch/arm/mach-*. We're now
>> slowly moving this code to drivers/ but some initialization code
>> (timers, critical clocks, irqs) needs to be called early in the boot
>> sequence.
>
> Right, and that's very good work.
>
>> When you're saying that we already have all the OF_DECLARE macros, it
>> seems to me that you're forgetting that we also want to keep
>> supporting the board files. So without the early platform drivers we
>> need to use a mix of OF_DECLARE and handcrafted initialization in
>> arch/arm/mach-* since we can't call platform_device_register() that
>> early. This blocks us from completely moving the should-be-driver code
>> to drivers/, because these drivers *need* to support both cases.
>
> The OF_DECLARE_* functions were initially added as a way to
> remove board files that just consist of callbacks into early
> initialization for some subsystems. As long as you still have
> board files and you are looking for a way to reuse code between
> OF_DECLARE_* functions and board files, why not just leave
> those functions globally visible and call them from the non-DT
> board files?
>
>> The main problem with OF_DECLARE is that although we have
>> corresponding device nodes, we never actually register any real linux
>> devices. If we add to this the fact that current early platform
>> drivers implementation is broken (for reasons I mentioned in the cover
>> letter) the support gets really messy, since we can have up to three
>> entry points to the driver's code. Other issues come to mind as well:
>> if we're using OF_DECLARE we can't benefit from devm* routines.
>
> Right, the devm_* problem has come up before.
>
>> My aim is to provide a clean, robust and generic way of probing
>> certain devices early and then converting them to actual platform
>> devices when we're advanced enough into the boot sequence. If we
>> merged such a framework, we could work towards removing both the
>> previous early platform devices (in favor of the new mechanism) and
>> maybe even deprecating and replacing OF_DECLARE(), since we could
>> simply early probe the DT drivers. Personally I see OF_DECLARE as a
>> bigger hack than early devices.
>>
>> My patch tries to address exactly the use cases we're facing - for
>> example by providing means to probe devices twice (early and late) and
>> to check the state we're at in order for users to be able to just do
>> the critical initialization early on and then continue with regular
>> stuff later.
>
> Maybe the problem is reusing the name and some of the code from
> an existing functionality that we've been trying to get rid of.
>

I'm not reusing the name - in fact I set the prefix to earlydev_
exactly in order to not confuse anyone. I'm also not reusing any code
in the second series.

> If what you want to do is completely different from the existing
> early_platform implementation, how about starting by moving that
> out of drivers/base/platform.c into something under arch/sh/
> and renaming it to something with an sh_ prefix.
>

Yes, this is a good idea, but what about the sh-specific drivers that
rely on it? Is including headers from arch/ in driver code still an
accepted practice?

> Let's just leave the non-DT part out of it by making it sh specific.
> Then we can come up with improvements to the current
> platform_device handling for DT based platforms that you can
> use on DT-based davinci to replace what currently happens on
> board-file based davinci systems, without mixing up those
> two code paths too much in the base driver support.
>

I don't see why we wouldn't want to unify these two cases. The best
solution to me seems having only one entry point into the driver code
- the probe() function stored in platform_driver - whether we're
probing it from DT, platform data, ACPI or early boot code.

Best regards,
Bartosz Golaszewski

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Bartosz Golaszewski]

2018-04-27 12:18 GMT+02:00 Arnd Bergmann :
> On Fri, Apr 27, 2018 at 10:57 AM, Bartosz Golaszewski
>  wrote:
>> 2018-04-27 9:52 GMT+02:00 Arnd Bergmann :
>>> On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  wrote:
 On 04/26/2018 12:31 PM, Rich Felker wrote:
>>
>> We have platforms out there which still use both board files and
>> device tree. They are still comercially supported and are not going
>> anywhere anytime soon. Some of these platforms are being actively
>> maintained and cleaned-up. An example is the DaVinci platform: David
>> has recently converted all the SoCs and boards to using the common
>> clock framework. I'm cleaning up some other parts too.
>>
>> The problem with the legacy board code is that a lot of things that
>> should be platform drivers ended up in arch/arm/mach-*. We're now
>> slowly moving this code to drivers/ but some initialization code
>> (timers, critical clocks, irqs) needs to be called early in the boot
>> sequence.
>
> Right, and that's very good work.
>
>> When you're saying that we already have all the OF_DECLARE macros, it
>> seems to me that you're forgetting that we also want to keep
>> supporting the board files. So without the early platform drivers we
>> need to use a mix of OF_DECLARE and handcrafted initialization in
>> arch/arm/mach-* since we can't call platform_device_register() that
>> early. This blocks us from completely moving the should-be-driver code
>> to drivers/, because these drivers *need* to support both cases.
>
> The OF_DECLARE_* functions were initially added as a way to
> remove board files that just consist of callbacks into early
> initialization for some subsystems. As long as you still have
> board files and you are looking for a way to reuse code between
> OF_DECLARE_* functions and board files, why not just leave
> those functions globally visible and call them from the non-DT
> board files?
>
>> The main problem with OF_DECLARE is that although we have
>> corresponding device nodes, we never actually register any real linux
>> devices. If we add to this the fact that current early platform
>> drivers implementation is broken (for reasons I mentioned in the cover
>> letter) the support gets really messy, since we can have up to three
>> entry points to the driver's code. Other issues come to mind as well:
>> if we're using OF_DECLARE we can't benefit from devm* routines.
>
> Right, the devm_* problem has come up before.
>
>> My aim is to provide a clean, robust and generic way of probing
>> certain devices early and then converting them to actual platform
>> devices when we're advanced enough into the boot sequence. If we
>> merged such a framework, we could work towards removing both the
>> previous early platform devices (in favor of the new mechanism) and
>> maybe even deprecating and replacing OF_DECLARE(), since we could
>> simply early probe the DT drivers. Personally I see OF_DECLARE as a
>> bigger hack than early devices.
>>
>> My patch tries to address exactly the use cases we're facing - for
>> example by providing means to probe devices twice (early and late) and
>> to check the state we're at in order for users to be able to just do
>> the critical initialization early on and then continue with regular
>> stuff later.
>
> Maybe the problem is reusing the name and some of the code from
> an existing functionality that we've been trying to get rid of.
>

I'm not reusing the name - in fact I set the prefix to earlydev_
exactly in order to not confuse anyone. I'm also not reusing any code
in the second series.

> If what you want to do is completely different from the existing
> early_platform implementation, how about starting by moving that
> out of drivers/base/platform.c into something under arch/sh/
> and renaming it to something with an sh_ prefix.
>

Yes, this is a good idea, but what about the sh-specific drivers that
rely on it? Is including headers from arch/ in driver code still an
accepted practice?

> Let's just leave the non-DT part out of it by making it sh specific.
> Then we can come up with improvements to the current
> platform_device handling for DT based platforms that you can
> use on DT-based davinci to replace what currently happens on
> board-file based davinci systems, without mixing up those
> two code paths too much in the base driver support.
>

I don't see why we wouldn't want to unify these two cases. The best
solution to me seems having only one entry point into the driver code
- the probe() function stored in platform_driver - whether we're
probing it from DT, platform data, ACPI or early boot code.

Best regards,
Bartosz Golaszewski

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Arnd Bergmann]

On Fri, Apr 27, 2018 at 10:57 AM, Bartosz Golaszewski
 wrote:
> 2018-04-27 9:52 GMT+02:00 Arnd Bergmann :
>> On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  wrote:
>>> On 04/26/2018 12:31 PM, Rich Felker wrote:
>
> We have platforms out there which still use both board files and
> device tree. They are still comercially supported and are not going
> anywhere anytime soon. Some of these platforms are being actively
> maintained and cleaned-up. An example is the DaVinci platform: David
> has recently converted all the SoCs and boards to using the common
> clock framework. I'm cleaning up some other parts too.
>
> The problem with the legacy board code is that a lot of things that
> should be platform drivers ended up in arch/arm/mach-*. We're now
> slowly moving this code to drivers/ but some initialization code
> (timers, critical clocks, irqs) needs to be called early in the boot
> sequence.

Right, and that's very good work.

> When you're saying that we already have all the OF_DECLARE macros, it
> seems to me that you're forgetting that we also want to keep
> supporting the board files. So without the early platform drivers we
> need to use a mix of OF_DECLARE and handcrafted initialization in
> arch/arm/mach-* since we can't call platform_device_register() that
> early. This blocks us from completely moving the should-be-driver code
> to drivers/, because these drivers *need* to support both cases.

The OF_DECLARE_* functions were initially added as a way to
remove board files that just consist of callbacks into early
initialization for some subsystems. As long as you still have
board files and you are looking for a way to reuse code between
OF_DECLARE_* functions and board files, why not just leave
those functions globally visible and call them from the non-DT
board files?

> The main problem with OF_DECLARE is that although we have
> corresponding device nodes, we never actually register any real linux
> devices. If we add to this the fact that current early platform
> drivers implementation is broken (for reasons I mentioned in the cover
> letter) the support gets really messy, since we can have up to three
> entry points to the driver's code. Other issues come to mind as well:
> if we're using OF_DECLARE we can't benefit from devm* routines.

Right, the devm_* problem has come up before.

> My aim is to provide a clean, robust and generic way of probing
> certain devices early and then converting them to actual platform
> devices when we're advanced enough into the boot sequence. If we
> merged such a framework, we could work towards removing both the
> previous early platform devices (in favor of the new mechanism) and
> maybe even deprecating and replacing OF_DECLARE(), since we could
> simply early probe the DT drivers. Personally I see OF_DECLARE as a
> bigger hack than early devices.
>
> My patch tries to address exactly the use cases we're facing - for
> example by providing means to probe devices twice (early and late) and
> to check the state we're at in order for users to be able to just do
> the critical initialization early on and then continue with regular
> stuff later.

Maybe the problem is reusing the name and some of the code from
an existing functionality that we've been trying to get rid of.

If what you want to do is completely different from the existing
early_platform implementation, how about starting by moving that
out of drivers/base/platform.c into something under arch/sh/
and renaming it to something with an sh_ prefix.

Let's just leave the non-DT part out of it by making it sh specific.
Then we can come up with improvements to the current
platform_device handling for DT based platforms that you can
use on DT-based davinci to replace what currently happens on
board-file based davinci systems, without mixing up those
two code paths too much in the base driver support.

   Arnd

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Arnd Bergmann]

On Fri, Apr 27, 2018 at 10:57 AM, Bartosz Golaszewski
 wrote:
> 2018-04-27 9:52 GMT+02:00 Arnd Bergmann :
>> On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  wrote:
>>> On 04/26/2018 12:31 PM, Rich Felker wrote:
>
> We have platforms out there which still use both board files and
> device tree. They are still comercially supported and are not going
> anywhere anytime soon. Some of these platforms are being actively
> maintained and cleaned-up. An example is the DaVinci platform: David
> has recently converted all the SoCs and boards to using the common
> clock framework. I'm cleaning up some other parts too.
>
> The problem with the legacy board code is that a lot of things that
> should be platform drivers ended up in arch/arm/mach-*. We're now
> slowly moving this code to drivers/ but some initialization code
> (timers, critical clocks, irqs) needs to be called early in the boot
> sequence.

Right, and that's very good work.

> When you're saying that we already have all the OF_DECLARE macros, it
> seems to me that you're forgetting that we also want to keep
> supporting the board files. So without the early platform drivers we
> need to use a mix of OF_DECLARE and handcrafted initialization in
> arch/arm/mach-* since we can't call platform_device_register() that
> early. This blocks us from completely moving the should-be-driver code
> to drivers/, because these drivers *need* to support both cases.

The OF_DECLARE_* functions were initially added as a way to
remove board files that just consist of callbacks into early
initialization for some subsystems. As long as you still have
board files and you are looking for a way to reuse code between
OF_DECLARE_* functions and board files, why not just leave
those functions globally visible and call them from the non-DT
board files?

> The main problem with OF_DECLARE is that although we have
> corresponding device nodes, we never actually register any real linux
> devices. If we add to this the fact that current early platform
> drivers implementation is broken (for reasons I mentioned in the cover
> letter) the support gets really messy, since we can have up to three
> entry points to the driver's code. Other issues come to mind as well:
> if we're using OF_DECLARE we can't benefit from devm* routines.

Right, the devm_* problem has come up before.

> My aim is to provide a clean, robust and generic way of probing
> certain devices early and then converting them to actual platform
> devices when we're advanced enough into the boot sequence. If we
> merged such a framework, we could work towards removing both the
> previous early platform devices (in favor of the new mechanism) and
> maybe even deprecating and replacing OF_DECLARE(), since we could
> simply early probe the DT drivers. Personally I see OF_DECLARE as a
> bigger hack than early devices.
>
> My patch tries to address exactly the use cases we're facing - for
> example by providing means to probe devices twice (early and late) and
> to check the state we're at in order for users to be able to just do
> the critical initialization early on and then continue with regular
> stuff later.

Maybe the problem is reusing the name and some of the code from
an existing functionality that we've been trying to get rid of.

If what you want to do is completely different from the existing
early_platform implementation, how about starting by moving that
out of drivers/base/platform.c into something under arch/sh/
and renaming it to something with an sh_ prefix.

Let's just leave the non-DT part out of it by making it sh specific.
Then we can come up with improvements to the current
platform_device handling for DT based platforms that you can
use on DT-based davinci to replace what currently happens on
board-file based davinci systems, without mixing up those
two code paths too much in the base driver support.

   Arnd

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Bartosz Golaszewski]

2018-04-27 10:55 GMT+02:00 Arnd Bergmann :
> On Fri, Apr 27, 2018 at 10:29 AM, Sekhar Nori  wrote:
>> On Friday 27 April 2018 01:22 PM, Arnd Bergmann wrote:
>>> On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  wrote:
 On 04/26/2018 12:31 PM, Rich Felker wrote:
>
>>>
>>> I haven't seen the discussion about your clock drivers, but I know that
>>> usually only a very small subset of the clocks on an SoC are needed
>>> that 'early', and you should use a regular platform driver for the rest.
>>
>> Its true that the subset is small, but they are either PLL bypass clocks
>> or clocks derived out of the main clock gate controller on the Soc
>> (DaVinci PSC). So we need some non-platform-device based initialization
>> support in the two main clock drivers used on mach-davinci anyway.
>>
>>> Can you elaborate on which devices need to access your clocks before
>>> you are able to initialize the clk driver through the regular 
>>> platform_driver
>>> framework? Do any of these need complex interactions with the clk
>>> subsystem, or do you just need to ensure they are turned on?
>>
>> Its just the timer IP. There is no complex interaction, just need to
>> ensure that the clock is registered with the framework and also switched
>> on when there is a gate clock.
>>
>> The latest attempt by David for this was posted last night here:
>>
>> https://lkml.org/lkml/2018/4/26/1093
>
> Ok, so the workaround in that series is to set up the timer clk manually
> in the SoC specific code (dm365_init_time etc) and register it to the
> clk subsystem, right?
>
> That seems to be a much more appropriate workaround than adding
> back early_platform devices. We can always try to do something better
> later, but for now I'm happy with that as a workaround.
>

Just to clarify: this is not bringing back the hacky early platform
devices from before. It's a new approach that actually uses the linux
platform driver model (unlike the current mechanism which only uses
the same data structures).

Thanks,
Bart

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Bartosz Golaszewski]

2018-04-27 10:55 GMT+02:00 Arnd Bergmann :
> On Fri, Apr 27, 2018 at 10:29 AM, Sekhar Nori  wrote:
>> On Friday 27 April 2018 01:22 PM, Arnd Bergmann wrote:
>>> On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  wrote:
 On 04/26/2018 12:31 PM, Rich Felker wrote:
>
>>>
>>> I haven't seen the discussion about your clock drivers, but I know that
>>> usually only a very small subset of the clocks on an SoC are needed
>>> that 'early', and you should use a regular platform driver for the rest.
>>
>> Its true that the subset is small, but they are either PLL bypass clocks
>> or clocks derived out of the main clock gate controller on the Soc
>> (DaVinci PSC). So we need some non-platform-device based initialization
>> support in the two main clock drivers used on mach-davinci anyway.
>>
>>> Can you elaborate on which devices need to access your clocks before
>>> you are able to initialize the clk driver through the regular 
>>> platform_driver
>>> framework? Do any of these need complex interactions with the clk
>>> subsystem, or do you just need to ensure they are turned on?
>>
>> Its just the timer IP. There is no complex interaction, just need to
>> ensure that the clock is registered with the framework and also switched
>> on when there is a gate clock.
>>
>> The latest attempt by David for this was posted last night here:
>>
>> https://lkml.org/lkml/2018/4/26/1093
>
> Ok, so the workaround in that series is to set up the timer clk manually
> in the SoC specific code (dm365_init_time etc) and register it to the
> clk subsystem, right?
>
> That seems to be a much more appropriate workaround than adding
> back early_platform devices. We can always try to do something better
> later, but for now I'm happy with that as a workaround.
>

Just to clarify: this is not bringing back the hacky early platform
devices from before. It's a new approach that actually uses the linux
platform driver model (unlike the current mechanism which only uses
the same data structures).

Thanks,
Bart

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Bartosz Golaszewski]

2018-04-27 9:52 GMT+02:00 Arnd Bergmann :
> On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  wrote:
>> On 04/26/2018 12:31 PM, Rich Felker wrote:
>>>
>>> On Thu, Apr 26, 2018 at 05:29:18PM +0200, Bartosz Golaszewski wrote:

 From: Bartosz Golaszewski 

 This is a follow to my series[1] the aim of which was to introduce device
 tree
 support for early platform devices.

 It was received rather negatively. Aside from using device tree to pass
 implementation specific details to the system, two important concerns
 were
 raised: no probe deferral support and the fact that currently the early
 devices
 never get converted to actual platform drivers. This series is a
 proof-of-concept that's trying to address those issues.

 The only user of the current version of early platform drivers is the
 SuperH
 architecture. If this series eventually gets merged, we could simply
 replace
 the other solution.
>>>
>>>
>>> Looking at a quick output of:
>>>
>>> grep -r -A10 early_devices[[] arch/sh/kernel/
>>>
>>> it looks like all of the existing early platform devices are serial
>>> ports, clocks, and clocksources. The switch to device tree should pick
>>> them all up from CLK_OF_DECLARE, TIMER_OF_DECLARE, and
>>> EARLYCON_DECLARE. Until that's complete, the existing code works
>>> as-is. I don't see what problem you're trying to solve.
>>
>>
>> The problem for us is that clk maintainers don't want new drivers to use
>> CLK_OF_DECLARE and instead use platform devices. I have just written such
>> a new driver that is shared by 6 different SoCs. For some combinations of
>> SoCs and clocks, using a platform device is fine but on others we need to
>> register early, so the drivers now have to handle both cases, which is
>> kind of messy and fragile. If there is a generic way to register platform
>> devices early, then the code is simplified because we only have to handle
>> one method of registering the clocks instead of two.
>
> The early_platform code is certainly not a way to make things simpler,
> it just adds one more way of doing the same thing that OF_CLK_DECLARE
> already does. We removed the last early_platform users on ARM a few
> years ago, and I would hope to leave it like that.
>
> I haven't seen the discussion about your clock drivers, but I know that
> usually only a very small subset of the clocks on an SoC are needed
> that 'early', and you should use a regular platform driver for the rest.
>
> Can you elaborate on which devices need to access your clocks before
> you are able to initialize the clk driver through the regular platform_driver
> framework? Do any of these need complex interactions with the clk
> subsystem, or do you just need to ensure they are turned on?
>
>  Arnd

The problem I'm trying to solve is the following:

We have platforms out there which still use both board files and
device tree. They are still comercially supported and are not going
anywhere anytime soon. Some of these platforms are being actively
maintained and cleaned-up. An example is the DaVinci platform: David
has recently converted all the SoCs and boards to using the common
clock framework. I'm cleaning up some other parts too.

The problem with the legacy board code is that a lot of things that
should be platform drivers ended up in arch/arm/mach-*. We're now
slowly moving this code to drivers/ but some initialization code
(timers, critical clocks, irqs) needs to be called early in the boot
sequence.

When you're saying that we already have all the OF_DECLARE macros, it
seems to me that you're forgetting that we also want to keep
supporting the board files. So without the early platform drivers we
need to use a mix of OF_DECLARE and handcrafted initialization in
arch/arm/mach-* since we can't call platform_device_register() that
early. This blocks us from completely moving the should-be-driver code
to drivers/, because these drivers *need* to support both cases.

The main problem with OF_DECLARE is that although we have
corresponding device nodes, we never actually register any real linux
devices. If we add to this the fact that current early platform
drivers implementation is broken (for reasons I mentioned in the cover
letter) the support gets really messy, since we can have up to three
entry points to the driver's code. Other issues come to mind as well:
if we're using OF_DECLARE we can't benefit from devm* routines.

My aim is to provide a clean, robust and generic way of probing
certain devices early and then converting them to actual platform
devices when we're advanced enough into the boot sequence. If we
merged such a framework, we could work towards removing both the
previous early platform devices (in favor of the new mechanism) and
maybe even deprecating and replacing OF_DECLARE(), since we could
simply early probe the DT drivers. Personally I see OF_DECLARE as 

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Bartosz Golaszewski]

2018-04-27 9:52 GMT+02:00 Arnd Bergmann :
> On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  wrote:
>> On 04/26/2018 12:31 PM, Rich Felker wrote:
>>>
>>> On Thu, Apr 26, 2018 at 05:29:18PM +0200, Bartosz Golaszewski wrote:

 From: Bartosz Golaszewski 

 This is a follow to my series[1] the aim of which was to introduce device
 tree
 support for early platform devices.

 It was received rather negatively. Aside from using device tree to pass
 implementation specific details to the system, two important concerns
 were
 raised: no probe deferral support and the fact that currently the early
 devices
 never get converted to actual platform drivers. This series is a
 proof-of-concept that's trying to address those issues.

 The only user of the current version of early platform drivers is the
 SuperH
 architecture. If this series eventually gets merged, we could simply
 replace
 the other solution.
>>>
>>>
>>> Looking at a quick output of:
>>>
>>> grep -r -A10 early_devices[[] arch/sh/kernel/
>>>
>>> it looks like all of the existing early platform devices are serial
>>> ports, clocks, and clocksources. The switch to device tree should pick
>>> them all up from CLK_OF_DECLARE, TIMER_OF_DECLARE, and
>>> EARLYCON_DECLARE. Until that's complete, the existing code works
>>> as-is. I don't see what problem you're trying to solve.
>>
>>
>> The problem for us is that clk maintainers don't want new drivers to use
>> CLK_OF_DECLARE and instead use platform devices. I have just written such
>> a new driver that is shared by 6 different SoCs. For some combinations of
>> SoCs and clocks, using a platform device is fine but on others we need to
>> register early, so the drivers now have to handle both cases, which is
>> kind of messy and fragile. If there is a generic way to register platform
>> devices early, then the code is simplified because we only have to handle
>> one method of registering the clocks instead of two.
>
> The early_platform code is certainly not a way to make things simpler,
> it just adds one more way of doing the same thing that OF_CLK_DECLARE
> already does. We removed the last early_platform users on ARM a few
> years ago, and I would hope to leave it like that.
>
> I haven't seen the discussion about your clock drivers, but I know that
> usually only a very small subset of the clocks on an SoC are needed
> that 'early', and you should use a regular platform driver for the rest.
>
> Can you elaborate on which devices need to access your clocks before
> you are able to initialize the clk driver through the regular platform_driver
> framework? Do any of these need complex interactions with the clk
> subsystem, or do you just need to ensure they are turned on?
>
>  Arnd

The problem I'm trying to solve is the following:

We have platforms out there which still use both board files and
device tree. They are still comercially supported and are not going
anywhere anytime soon. Some of these platforms are being actively
maintained and cleaned-up. An example is the DaVinci platform: David
has recently converted all the SoCs and boards to using the common
clock framework. I'm cleaning up some other parts too.

The problem with the legacy board code is that a lot of things that
should be platform drivers ended up in arch/arm/mach-*. We're now
slowly moving this code to drivers/ but some initialization code
(timers, critical clocks, irqs) needs to be called early in the boot
sequence.

When you're saying that we already have all the OF_DECLARE macros, it
seems to me that you're forgetting that we also want to keep
supporting the board files. So without the early platform drivers we
need to use a mix of OF_DECLARE and handcrafted initialization in
arch/arm/mach-* since we can't call platform_device_register() that
early. This blocks us from completely moving the should-be-driver code
to drivers/, because these drivers *need* to support both cases.

The main problem with OF_DECLARE is that although we have
corresponding device nodes, we never actually register any real linux
devices. If we add to this the fact that current early platform
drivers implementation is broken (for reasons I mentioned in the cover
letter) the support gets really messy, since we can have up to three
entry points to the driver's code. Other issues come to mind as well:
if we're using OF_DECLARE we can't benefit from devm* routines.

My aim is to provide a clean, robust and generic way of probing
certain devices early and then converting them to actual platform
devices when we're advanced enough into the boot sequence. If we
merged such a framework, we could work towards removing both the
previous early platform devices (in favor of the new mechanism) and
maybe even deprecating and replacing OF_DECLARE(), since we could
simply early probe the DT drivers. Personally I see OF_DECLARE as a
bigger hack than early devices.

My patch tries to address 

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Arnd Bergmann]

On Fri, Apr 27, 2018 at 10:29 AM, Sekhar Nori  wrote:
> On Friday 27 April 2018 01:22 PM, Arnd Bergmann wrote:
>> On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  wrote:
>>> On 04/26/2018 12:31 PM, Rich Felker wrote:

>>
>> I haven't seen the discussion about your clock drivers, but I know that
>> usually only a very small subset of the clocks on an SoC are needed
>> that 'early', and you should use a regular platform driver for the rest.
>
> Its true that the subset is small, but they are either PLL bypass clocks
> or clocks derived out of the main clock gate controller on the Soc
> (DaVinci PSC). So we need some non-platform-device based initialization
> support in the two main clock drivers used on mach-davinci anyway.
>
>> Can you elaborate on which devices need to access your clocks before
>> you are able to initialize the clk driver through the regular platform_driver
>> framework? Do any of these need complex interactions with the clk
>> subsystem, or do you just need to ensure they are turned on?
>
> Its just the timer IP. There is no complex interaction, just need to
> ensure that the clock is registered with the framework and also switched
> on when there is a gate clock.
>
> The latest attempt by David for this was posted last night here:
>
> https://lkml.org/lkml/2018/4/26/1093

Ok, so the workaround in that series is to set up the timer clk manually
in the SoC specific code (dm365_init_time etc) and register it to the
clk subsystem, right?

That seems to be a much more appropriate workaround than adding
back early_platform devices. We can always try to do something better
later, but for now I'm happy with that as a workaround.

Please clarify: do we have to set up the clk registers for the timer
here just because we can't rely on the bootloader to have it set up
initially, or is there some other reason? I think what some other
platforms do is to treat the timer clock as a fixed-rate clock that
is not managed by the actual clk driver but is set up by uboot
before we enter the kernel, and then the clk driver just makes sure
it doesn't turn that clk off later.

Arnd

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Arnd Bergmann]

On Fri, Apr 27, 2018 at 10:29 AM, Sekhar Nori  wrote:
> On Friday 27 April 2018 01:22 PM, Arnd Bergmann wrote:
>> On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  wrote:
>>> On 04/26/2018 12:31 PM, Rich Felker wrote:

>>
>> I haven't seen the discussion about your clock drivers, but I know that
>> usually only a very small subset of the clocks on an SoC are needed
>> that 'early', and you should use a regular platform driver for the rest.
>
> Its true that the subset is small, but they are either PLL bypass clocks
> or clocks derived out of the main clock gate controller on the Soc
> (DaVinci PSC). So we need some non-platform-device based initialization
> support in the two main clock drivers used on mach-davinci anyway.
>
>> Can you elaborate on which devices need to access your clocks before
>> you are able to initialize the clk driver through the regular platform_driver
>> framework? Do any of these need complex interactions with the clk
>> subsystem, or do you just need to ensure they are turned on?
>
> Its just the timer IP. There is no complex interaction, just need to
> ensure that the clock is registered with the framework and also switched
> on when there is a gate clock.
>
> The latest attempt by David for this was posted last night here:
>
> https://lkml.org/lkml/2018/4/26/1093

Ok, so the workaround in that series is to set up the timer clk manually
in the SoC specific code (dm365_init_time etc) and register it to the
clk subsystem, right?

That seems to be a much more appropriate workaround than adding
back early_platform devices. We can always try to do something better
later, but for now I'm happy with that as a workaround.

Please clarify: do we have to set up the clk registers for the timer
here just because we can't rely on the bootloader to have it set up
initially, or is there some other reason? I think what some other
platforms do is to treat the timer clock as a fixed-rate clock that
is not managed by the actual clk driver but is set up by uboot
before we enter the kernel, and then the clk driver just makes sure
it doesn't turn that clk off later.

Arnd

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Sekhar Nori]

Hi Arnd,

On Friday 27 April 2018 01:22 PM, Arnd Bergmann wrote:
> On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  wrote:
>> On 04/26/2018 12:31 PM, Rich Felker wrote:
>>>
>>> On Thu, Apr 26, 2018 at 05:29:18PM +0200, Bartosz Golaszewski wrote:

 From: Bartosz Golaszewski 

 This is a follow to my series[1] the aim of which was to introduce device
 tree
 support for early platform devices.

 It was received rather negatively. Aside from using device tree to pass
 implementation specific details to the system, two important concerns
 were
 raised: no probe deferral support and the fact that currently the early
 devices
 never get converted to actual platform drivers. This series is a
 proof-of-concept that's trying to address those issues.

 The only user of the current version of early platform drivers is the
 SuperH
 architecture. If this series eventually gets merged, we could simply
 replace
 the other solution.
>>>
>>>
>>> Looking at a quick output of:
>>>
>>> grep -r -A10 early_devices[[] arch/sh/kernel/
>>>
>>> it looks like all of the existing early platform devices are serial
>>> ports, clocks, and clocksources. The switch to device tree should pick
>>> them all up from CLK_OF_DECLARE, TIMER_OF_DECLARE, and
>>> EARLYCON_DECLARE. Until that's complete, the existing code works
>>> as-is. I don't see what problem you're trying to solve.
>>
>>
>> The problem for us is that clk maintainers don't want new drivers to use
>> CLK_OF_DECLARE and instead use platform devices. I have just written such
>> a new driver that is shared by 6 different SoCs. For some combinations of
>> SoCs and clocks, using a platform device is fine but on others we need to
>> register early, so the drivers now have to handle both cases, which is
>> kind of messy and fragile. If there is a generic way to register platform
>> devices early, then the code is simplified because we only have to handle
>> one method of registering the clocks instead of two.
> 
> The early_platform code is certainly not a way to make things simpler,
> it just adds one more way of doing the same thing that OF_CLK_DECLARE
> already does. We removed the last early_platform users on ARM a few
> years ago, and I would hope to leave it like that.
> 
> I haven't seen the discussion about your clock drivers, but I know that
> usually only a very small subset of the clocks on an SoC are needed
> that 'early', and you should use a regular platform driver for the rest.

Its true that the subset is small, but they are either PLL bypass clocks
or clocks derived out of the main clock gate controller on the Soc
(DaVinci PSC). So we need some non-platform-device based initialization
support in the two main clock drivers used on mach-davinci anyway.

> Can you elaborate on which devices need to access your clocks before
> you are able to initialize the clk driver through the regular platform_driver
> framework? Do any of these need complex interactions with the clk
> subsystem, or do you just need to ensure they are turned on?

Its just the timer IP. There is no complex interaction, just need to
ensure that the clock is registered with the framework and also switched
on when there is a gate clock.

The latest attempt by David for this was posted last night here:

https://lkml.org/lkml/2018/4/26/1093

Thanks,
Sekhar

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Sekhar Nori]

Hi Arnd,

On Friday 27 April 2018 01:22 PM, Arnd Bergmann wrote:
> On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  wrote:
>> On 04/26/2018 12:31 PM, Rich Felker wrote:
>>>
>>> On Thu, Apr 26, 2018 at 05:29:18PM +0200, Bartosz Golaszewski wrote:

 From: Bartosz Golaszewski 

 This is a follow to my series[1] the aim of which was to introduce device
 tree
 support for early platform devices.

 It was received rather negatively. Aside from using device tree to pass
 implementation specific details to the system, two important concerns
 were
 raised: no probe deferral support and the fact that currently the early
 devices
 never get converted to actual platform drivers. This series is a
 proof-of-concept that's trying to address those issues.

 The only user of the current version of early platform drivers is the
 SuperH
 architecture. If this series eventually gets merged, we could simply
 replace
 the other solution.
>>>
>>>
>>> Looking at a quick output of:
>>>
>>> grep -r -A10 early_devices[[] arch/sh/kernel/
>>>
>>> it looks like all of the existing early platform devices are serial
>>> ports, clocks, and clocksources. The switch to device tree should pick
>>> them all up from CLK_OF_DECLARE, TIMER_OF_DECLARE, and
>>> EARLYCON_DECLARE. Until that's complete, the existing code works
>>> as-is. I don't see what problem you're trying to solve.
>>
>>
>> The problem for us is that clk maintainers don't want new drivers to use
>> CLK_OF_DECLARE and instead use platform devices. I have just written such
>> a new driver that is shared by 6 different SoCs. For some combinations of
>> SoCs and clocks, using a platform device is fine but on others we need to
>> register early, so the drivers now have to handle both cases, which is
>> kind of messy and fragile. If there is a generic way to register platform
>> devices early, then the code is simplified because we only have to handle
>> one method of registering the clocks instead of two.
> 
> The early_platform code is certainly not a way to make things simpler,
> it just adds one more way of doing the same thing that OF_CLK_DECLARE
> already does. We removed the last early_platform users on ARM a few
> years ago, and I would hope to leave it like that.
> 
> I haven't seen the discussion about your clock drivers, but I know that
> usually only a very small subset of the clocks on an SoC are needed
> that 'early', and you should use a regular platform driver for the rest.

Its true that the subset is small, but they are either PLL bypass clocks
or clocks derived out of the main clock gate controller on the Soc
(DaVinci PSC). So we need some non-platform-device based initialization
support in the two main clock drivers used on mach-davinci anyway.

> Can you elaborate on which devices need to access your clocks before
> you are able to initialize the clk driver through the regular platform_driver
> framework? Do any of these need complex interactions with the clk
> subsystem, or do you just need to ensure they are turned on?

Its just the timer IP. There is no complex interaction, just need to
ensure that the clock is registered with the framework and also switched
on when there is a gate clock.

The latest attempt by David for this was posted last night here:

https://lkml.org/lkml/2018/4/26/1093

Thanks,
Sekhar

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Arnd Bergmann]

On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  wrote:
> On 04/26/2018 12:31 PM, Rich Felker wrote:
>>
>> On Thu, Apr 26, 2018 at 05:29:18PM +0200, Bartosz Golaszewski wrote:
>>>
>>> From: Bartosz Golaszewski 
>>>
>>> This is a follow to my series[1] the aim of which was to introduce device
>>> tree
>>> support for early platform devices.
>>>
>>> It was received rather negatively. Aside from using device tree to pass
>>> implementation specific details to the system, two important concerns
>>> were
>>> raised: no probe deferral support and the fact that currently the early
>>> devices
>>> never get converted to actual platform drivers. This series is a
>>> proof-of-concept that's trying to address those issues.
>>>
>>> The only user of the current version of early platform drivers is the
>>> SuperH
>>> architecture. If this series eventually gets merged, we could simply
>>> replace
>>> the other solution.
>>
>>
>> Looking at a quick output of:
>>
>> grep -r -A10 early_devices[[] arch/sh/kernel/
>>
>> it looks like all of the existing early platform devices are serial
>> ports, clocks, and clocksources. The switch to device tree should pick
>> them all up from CLK_OF_DECLARE, TIMER_OF_DECLARE, and
>> EARLYCON_DECLARE. Until that's complete, the existing code works
>> as-is. I don't see what problem you're trying to solve.
>
>
> The problem for us is that clk maintainers don't want new drivers to use
> CLK_OF_DECLARE and instead use platform devices. I have just written such
> a new driver that is shared by 6 different SoCs. For some combinations of
> SoCs and clocks, using a platform device is fine but on others we need to
> register early, so the drivers now have to handle both cases, which is
> kind of messy and fragile. If there is a generic way to register platform
> devices early, then the code is simplified because we only have to handle
> one method of registering the clocks instead of two.

The early_platform code is certainly not a way to make things simpler,
it just adds one more way of doing the same thing that OF_CLK_DECLARE
already does. We removed the last early_platform users on ARM a few
years ago, and I would hope to leave it like that.

I haven't seen the discussion about your clock drivers, but I know that
usually only a very small subset of the clocks on an SoC are needed
that 'early', and you should use a regular platform driver for the rest.

Can you elaborate on which devices need to access your clocks before
you are able to initialize the clk driver through the regular platform_driver
framework? Do any of these need complex interactions with the clk
subsystem, or do you just need to ensure they are turned on?

 Arnd

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Arnd Bergmann]

On Fri, Apr 27, 2018 at 4:28 AM, David Lechner  wrote:
> On 04/26/2018 12:31 PM, Rich Felker wrote:
>>
>> On Thu, Apr 26, 2018 at 05:29:18PM +0200, Bartosz Golaszewski wrote:
>>>
>>> From: Bartosz Golaszewski 
>>>
>>> This is a follow to my series[1] the aim of which was to introduce device
>>> tree
>>> support for early platform devices.
>>>
>>> It was received rather negatively. Aside from using device tree to pass
>>> implementation specific details to the system, two important concerns
>>> were
>>> raised: no probe deferral support and the fact that currently the early
>>> devices
>>> never get converted to actual platform drivers. This series is a
>>> proof-of-concept that's trying to address those issues.
>>>
>>> The only user of the current version of early platform drivers is the
>>> SuperH
>>> architecture. If this series eventually gets merged, we could simply
>>> replace
>>> the other solution.
>>
>>
>> Looking at a quick output of:
>>
>> grep -r -A10 early_devices[[] arch/sh/kernel/
>>
>> it looks like all of the existing early platform devices are serial
>> ports, clocks, and clocksources. The switch to device tree should pick
>> them all up from CLK_OF_DECLARE, TIMER_OF_DECLARE, and
>> EARLYCON_DECLARE. Until that's complete, the existing code works
>> as-is. I don't see what problem you're trying to solve.
>
>
> The problem for us is that clk maintainers don't want new drivers to use
> CLK_OF_DECLARE and instead use platform devices. I have just written such
> a new driver that is shared by 6 different SoCs. For some combinations of
> SoCs and clocks, using a platform device is fine but on others we need to
> register early, so the drivers now have to handle both cases, which is
> kind of messy and fragile. If there is a generic way to register platform
> devices early, then the code is simplified because we only have to handle
> one method of registering the clocks instead of two.

The early_platform code is certainly not a way to make things simpler,
it just adds one more way of doing the same thing that OF_CLK_DECLARE
already does. We removed the last early_platform users on ARM a few
years ago, and I would hope to leave it like that.

I haven't seen the discussion about your clock drivers, but I know that
usually only a very small subset of the clocks on an SoC are needed
that 'early', and you should use a regular platform driver for the rest.

Can you elaborate on which devices need to access your clocks before
you are able to initialize the clk driver through the regular platform_driver
framework? Do any of these need complex interactions with the clk
subsystem, or do you just need to ensure they are turned on?

 Arnd

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Rich Felker]

On Thu, Apr 26, 2018 at 09:28:39PM -0500, David Lechner wrote:
> On 04/26/2018 12:31 PM, Rich Felker wrote:
> >On Thu, Apr 26, 2018 at 05:29:18PM +0200, Bartosz Golaszewski wrote:
> >>From: Bartosz Golaszewski 
> >>
> >>This is a follow to my series[1] the aim of which was to introduce device 
> >>tree
> >>support for early platform devices.
> >>
> >>It was received rather negatively. Aside from using device tree to pass
> >>implementation specific details to the system, two important concerns were
> >>raised: no probe deferral support and the fact that currently the early 
> >>devices
> >>never get converted to actual platform drivers. This series is a
> >>proof-of-concept that's trying to address those issues.
> >>
> >>The only user of the current version of early platform drivers is the SuperH
> >>architecture. If this series eventually gets merged, we could simply replace
> >>the other solution.
> >
> >Looking at a quick output of:
> >
> > grep -r -A10 early_devices[[] arch/sh/kernel/
> >
> >it looks like all of the existing early platform devices are serial
> >ports, clocks, and clocksources. The switch to device tree should pick
> >them all up from CLK_OF_DECLARE, TIMER_OF_DECLARE, and
> >EARLYCON_DECLARE. Until that's complete, the existing code works
> >as-is. I don't see what problem you're trying to solve.
> 
> The problem for us is that clk maintainers don't want new drivers to use
> CLK_OF_DECLARE and instead use platform devices. I have just written such
> a new driver that is shared by 6 different SoCs. For some combinations of
> SoCs and clocks, using a platform device is fine but on others we need to
> register early, so the drivers now have to handle both cases, which is
> kind of messy and fragile. If there is a generic way to register platform
> devices early, then the code is simplified because we only have to handle
> one method of registering the clocks instead of two.

Can you get them to explain why? This sounds wrong.

Rich

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Rich Felker]

On Thu, Apr 26, 2018 at 09:28:39PM -0500, David Lechner wrote:
> On 04/26/2018 12:31 PM, Rich Felker wrote:
> >On Thu, Apr 26, 2018 at 05:29:18PM +0200, Bartosz Golaszewski wrote:
> >>From: Bartosz Golaszewski 
> >>
> >>This is a follow to my series[1] the aim of which was to introduce device 
> >>tree
> >>support for early platform devices.
> >>
> >>It was received rather negatively. Aside from using device tree to pass
> >>implementation specific details to the system, two important concerns were
> >>raised: no probe deferral support and the fact that currently the early 
> >>devices
> >>never get converted to actual platform drivers. This series is a
> >>proof-of-concept that's trying to address those issues.
> >>
> >>The only user of the current version of early platform drivers is the SuperH
> >>architecture. If this series eventually gets merged, we could simply replace
> >>the other solution.
> >
> >Looking at a quick output of:
> >
> > grep -r -A10 early_devices[[] arch/sh/kernel/
> >
> >it looks like all of the existing early platform devices are serial
> >ports, clocks, and clocksources. The switch to device tree should pick
> >them all up from CLK_OF_DECLARE, TIMER_OF_DECLARE, and
> >EARLYCON_DECLARE. Until that's complete, the existing code works
> >as-is. I don't see what problem you're trying to solve.
> 
> The problem for us is that clk maintainers don't want new drivers to use
> CLK_OF_DECLARE and instead use platform devices. I have just written such
> a new driver that is shared by 6 different SoCs. For some combinations of
> SoCs and clocks, using a platform device is fine but on others we need to
> register early, so the drivers now have to handle both cases, which is
> kind of messy and fragile. If there is a generic way to register platform
> devices early, then the code is simplified because we only have to handle
> one method of registering the clocks instead of two.

Can you get them to explain why? This sounds wrong.

Rich

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[David Lechner]

On 04/26/2018 12:31 PM, Rich Felker wrote:

On Thu, Apr 26, 2018 at 05:29:18PM +0200, Bartosz Golaszewski wrote:

From: Bartosz Golaszewski 

This is a follow to my series[1] the aim of which was to introduce device tree
support for early platform devices.

It was received rather negatively. Aside from using device tree to pass
implementation specific details to the system, two important concerns were
raised: no probe deferral support and the fact that currently the early devices
never get converted to actual platform drivers. This series is a
proof-of-concept that's trying to address those issues.

The only user of the current version of early platform drivers is the SuperH
architecture. If this series eventually gets merged, we could simply replace
the other solution.


Looking at a quick output of:

grep -r -A10 early_devices[[] arch/sh/kernel/

it looks like all of the existing early platform devices are serial
ports, clocks, and clocksources. The switch to device tree should pick
them all up from CLK_OF_DECLARE, TIMER_OF_DECLARE, and
EARLYCON_DECLARE. Until that's complete, the existing code works
as-is. I don't see what problem you're trying to solve.


The problem for us is that clk maintainers don't want new drivers to use
CLK_OF_DECLARE and instead use platform devices. I have just written such
a new driver that is shared by 6 different SoCs. For some combinations of
SoCs and clocks, using a platform device is fine but on others we need to
register early, so the drivers now have to handle both cases, which is
kind of messy and fragile. If there is a generic way to register platform
devices early, then the code is simplified because we only have to handle
one method of registering the clocks instead of two.



FYI I'm (sometimes-somewhat-absent) arch/sh co-maintainer.

Rich

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[David Lechner]

On 04/26/2018 12:31 PM, Rich Felker wrote:

On Thu, Apr 26, 2018 at 05:29:18PM +0200, Bartosz Golaszewski wrote:

From: Bartosz Golaszewski 

This is a follow to my series[1] the aim of which was to introduce device tree
support for early platform devices.

It was received rather negatively. Aside from using device tree to pass
implementation specific details to the system, two important concerns were
raised: no probe deferral support and the fact that currently the early devices
never get converted to actual platform drivers. This series is a
proof-of-concept that's trying to address those issues.

The only user of the current version of early platform drivers is the SuperH
architecture. If this series eventually gets merged, we could simply replace
the other solution.


Looking at a quick output of:

grep -r -A10 early_devices[[] arch/sh/kernel/

it looks like all of the existing early platform devices are serial
ports, clocks, and clocksources. The switch to device tree should pick
them all up from CLK_OF_DECLARE, TIMER_OF_DECLARE, and
EARLYCON_DECLARE. Until that's complete, the existing code works
as-is. I don't see what problem you're trying to solve.


The problem for us is that clk maintainers don't want new drivers to use
CLK_OF_DECLARE and instead use platform devices. I have just written such
a new driver that is shared by 6 different SoCs. For some combinations of
SoCs and clocks, using a platform device is fine but on others we need to
register early, so the drivers now have to handle both cases, which is
kind of messy and fragile. If there is a generic way to register platform
devices early, then the code is simplified because we only have to handle
one method of registering the clocks instead of two.



FYI I'm (sometimes-somewhat-absent) arch/sh co-maintainer.

Rich

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Rich Felker]

On Thu, Apr 26, 2018 at 05:29:18PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski 
> 
> This is a follow to my series[1] the aim of which was to introduce device tree
> support for early platform devices.
> 
> It was received rather negatively. Aside from using device tree to pass
> implementation specific details to the system, two important concerns were
> raised: no probe deferral support and the fact that currently the early 
> devices
> never get converted to actual platform drivers. This series is a
> proof-of-concept that's trying to address those issues.
> 
> The only user of the current version of early platform drivers is the SuperH
> architecture. If this series eventually gets merged, we could simply replace
> the other solution.

Looking at a quick output of:

grep -r -A10 early_devices[[] arch/sh/kernel/

it looks like all of the existing early platform devices are serial
ports, clocks, and clocksources. The switch to device tree should pick
them all up from CLK_OF_DECLARE, TIMER_OF_DECLARE, and
EARLYCON_DECLARE. Until that's complete, the existing code works
as-is. I don't see what problem you're trying to solve.

FYI I'm (sometimes-somewhat-absent) arch/sh co-maintainer.

Rich

Re: [PATCH RFC PoC 0/2] platform: different approach to early platform drivers

[Rich Felker]

On Thu, Apr 26, 2018 at 05:29:18PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski 
> 
> This is a follow to my series[1] the aim of which was to introduce device tree
> support for early platform devices.
> 
> It was received rather negatively. Aside from using device tree to pass
> implementation specific details to the system, two important concerns were
> raised: no probe deferral support and the fact that currently the early 
> devices
> never get converted to actual platform drivers. This series is a
> proof-of-concept that's trying to address those issues.
> 
> The only user of the current version of early platform drivers is the SuperH
> architecture. If this series eventually gets merged, we could simply replace
> the other solution.

Looking at a quick output of:

grep -r -A10 early_devices[[] arch/sh/kernel/

it looks like all of the existing early platform devices are serial
ports, clocks, and clocksources. The switch to device tree should pick
them all up from CLK_OF_DECLARE, TIMER_OF_DECLARE, and
EARLYCON_DECLARE. Until that's complete, the existing code works
as-is. I don't see what problem you're trying to solve.

FYI I'm (sometimes-somewhat-absent) arch/sh co-maintainer.

Rich