Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1280061 > unrolled thread

Re: [RFC PATCH] clocksource: ti-32k: convert to platform device

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH] clocksource: ti-32k: convert to platform device Tony Lindgren <tony@atomide.com> - 2015-11-30 17:30 +0100
    Re: [RFC PATCH] clocksource: ti-32k: convert to platform device Grygorii Strashko <grygorii.strashko@ti.com> - 2015-12-01 16:10 +0100
      Re: [RFC PATCH] clocksource: ti-32k: convert to platform device Tony Lindgren <tony@atomide.com> - 2015-12-01 17:10 +0100
        Re: [RFC PATCH] clocksource: ti-32k: convert to platform device Grygorii Strashko <grygorii.strashko@ti.com> - 2015-12-01 18:20 +0100
          Re: [RFC PATCH] clocksource: ti-32k: convert to platform device Tony Lindgren <tony@atomide.com> - 2015-12-01 18:30 +0100

#1280061 — Re: [RFC PATCH] clocksource: ti-32k: convert to platform device

* Grygorii Strashko <grygorii.strashko@ti.com> [151127 12:11]:
> Hi Felipe,
> 
> On 11/20/2015 08:21 PM, Felipe Balbi wrote:
> > Grygorii Strashko <grygorii.strashko@ti.com> writes:
> >> Since system clocksource is finally selected by Clocksource core at
> >> fs_initcall stage during boot there are no reasons to initialize
> >> ti_32k_timer at early boot stages. Hence, ti_32k_timer can be
> >> converted to use platform device/driver model and its PM can be
> >> implemented using PM runtime which is common for OMAP devices.
> >>
> >> Platform specific initialization code has to be disabled once as
> >> ti_32k_timer is converted to platform device - otherwise OMAP platform
> >> code will generate boot warnings.
> >>
> >> After this change, all counter_32k's platform code can be removed
> >> once all OMAP boards will be converted to DT.
> >>
> >> Cc: Tony Lindgren <tony@atomide.com>
> >> Cc: Felipe Balbi <balbi@ti.com>
> >> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> >> ---
> 
> [...]
> 
> >> +
> >> +static struct platform_driver ti_32k_driver __initdata = {
> >> +	.probe		= ti_32k_probe,
> >> +	.driver		= {
> >> +		.name	= "ti_32k_timer",
> >> +		.of_match_table = of_match_ptr(ti_32k_of_table),
> >> +	}
> >> +};
> >> +
> >> +static int __init ti_32k_init(void)
> >> +{
> >> +	return platform_driver_register(&ti_32k_driver);
> >>   }
> >> -CLOCKSOURCE_OF_DECLARE(ti_32k_timer, "ti,omap-counter32k",
> >> -		ti_32k_timer_init);
> >> +
> >> +subsys_initcall(ti_32k_init);
> >> +
> >> +MODULE_AUTHOR("Paul Mundt");
> >> +MODULE_AUTHOR("Juha Yrjölä");
> >> +MODULE_DESCRIPTION("OMAP2 32k Timer");
> >> +MODULE_ALIAS("platform:ti_32k_timer");
> >> +MODULE_LICENSE("GPL v2");
> > 
> > this will break clksource_of_init(), right ? Eventually, we want that to
> > be the only thing called by our .init_time method. I'll leave it to Tony
> > to decide, but IMO this is not a good path forward for timers.
> > 
> 
> Yeh :(.  I did additional tests, and, unfortunately, this can't be used as is.
> But not because of clocksource_of_init() which will just produce boot warning.
> It can't be done because of sched_clock_register() which is expected to be
> called during early boot time only and with disabled IRQs.
> 
> It was so tempting to try :)

We should be able to make this into an early_platform_device and just
have it depend on the source clock muxes. See the omap initcall changes
patches I just posted.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1280937

Hi Tony,

On 11/30/2015 06:28 PM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [151127 12:11]:
>> On 11/20/2015 08:21 PM, Felipe Balbi wrote:
>>> Grygorii Strashko <grygorii.strashko@ti.com> writes:
>>>> Since system clocksource is finally selected by Clocksource core at
>>>> fs_initcall stage during boot there are no reasons to initialize
>>>> ti_32k_timer at early boot stages. Hence, ti_32k_timer can be
>>>> converted to use platform device/driver model and its PM can be
>>>> implemented using PM runtime which is common for OMAP devices.
>>>>
>>>> Platform specific initialization code has to be disabled once as
>>>> ti_32k_timer is converted to platform device - otherwise OMAP platform
>>>> code will generate boot warnings.
>>>>
>>>> After this change, all counter_32k's platform code can be removed
>>>> once all OMAP boards will be converted to DT.
>>>>
>>>> Cc: Tony Lindgren <tony@atomide.com>
>>>> Cc: Felipe Balbi <balbi@ti.com>
>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>>> ---
>>
>> [...]
>>
>>>> +
>>>> +static struct platform_driver ti_32k_driver __initdata = {
>>>> +	.probe		= ti_32k_probe,
>>>> +	.driver		= {
>>>> +		.name	= "ti_32k_timer",
>>>> +		.of_match_table = of_match_ptr(ti_32k_of_table),
>>>> +	}
>>>> +};
>>>> +
>>>> +static int __init ti_32k_init(void)
>>>> +{
>>>> +	return platform_driver_register(&ti_32k_driver);
>>>>    }
>>>> -CLOCKSOURCE_OF_DECLARE(ti_32k_timer, "ti,omap-counter32k",
>>>> -		ti_32k_timer_init);
>>>> +
>>>> +subsys_initcall(ti_32k_init);
>>>> +
>>>> +MODULE_AUTHOR("Paul Mundt");
>>>> +MODULE_AUTHOR("Juha Yrjölä");
>>>> +MODULE_DESCRIPTION("OMAP2 32k Timer");
>>>> +MODULE_ALIAS("platform:ti_32k_timer");
>>>> +MODULE_LICENSE("GPL v2");
>>>
>>> this will break clksource_of_init(), right ? Eventually, we want that to
>>> be the only thing called by our .init_time method. I'll leave it to Tony
>>> to decide, but IMO this is not a good path forward for timers.
>>>
>>
>> Yeh :(.  I did additional tests, and, unfortunately, this can't be used as is.
>> But not because of clocksource_of_init() which will just produce boot warning.
>> It can't be done because of sched_clock_register() which is expected to be
>> called during early boot time only and with disabled IRQs.
>>
>> It was so tempting to try :)
> 
> We should be able to make this into an early_platform_device and just
> have it depend on the source clock muxes. See the omap initcall changes
> patches I just posted.
> 

Sry, may be I've missed smth, but how early_platform_device will help us
to get rid of platform code - We'd still need to power on manually 
early_platform_device's from platform code :( through hwmod.

The main reason why I've tried this is because clocksource will be really selected
only at fs_initcall time - and at that time we have no restriction for using platform
devices, Pm runtime APIs, etc. (exception/blocker is sched_clock :().

-- 
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1280990

* Grygorii Strashko <grygorii.strashko@ti.com> [151201 07:09]:
> On 11/30/2015 06:28 PM, Tony Lindgren wrote:
> > 
> > We should be able to make this into an early_platform_device and just
> > have it depend on the source clock muxes. See the omap initcall changes
> > patches I just posted.
> > 
> 
> Sry, may be I've missed smth, but how early_platform_device will help us
> to get rid of platform code - We'd still need to power on manually 
> early_platform_device's from platform code :( through hwmod.

Having minimal platform code early is not a problem. The problem is that
our early code is not minimal.

For the system timers, we should only initialize the mux clocks needed
early to select between 32k and hf oscillator source. This needs to be done
using the clock framework, but we don't need the other clocks initialized
early.

The system timers we're using should be in the alwon power domain, if they
are not, then we should change the timers around so we're using only timers
in the alwon domain for system timers. Typically at least gpt1 and 12 are
always powered. That allows us to leave out the hwmod dependency for system
timers.

Or am I forgetting some other dependency with our system timers?

> The main reason why I've tried this is because clocksource will be really selected
> only at fs_initcall time - and at that time we have no restriction for using platform
> devices, Pm runtime APIs, etc. (exception/blocker is sched_clock :().

Right. But it seems we can leave out quite a bit of the dependencies
for system timers. We already have gptimer probe not touching the system
timers later on and can use shared gptimer functions after the clock
muxing is done.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1281059

On 12/01/2015 06:07 PM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [151201 07:09]:
>> On 11/30/2015 06:28 PM, Tony Lindgren wrote:
>>>
>>> We should be able to make this into an early_platform_device and just
>>> have it depend on the source clock muxes. See the omap initcall changes
>>> patches I just posted.
>>>
>>
>> Sry, may be I've missed smth, but how early_platform_device will help us
>> to get rid of platform code - We'd still need to power on manually
>> early_platform_device's from platform code :( through hwmod.
>
> Having minimal platform code early is not a problem. The problem is that
> our early code is not minimal.
>
> For the system timers, we should only initialize the mux clocks needed
> early to select between 32k and hf oscillator source. This needs to be done
> using the clock framework, but we don't need the other clocks initialized
> early.
>
> The system timers we're using should be in the alwon power domain, if they
> are not, then we should change the timers around so we're using only timers
> in the alwon domain for system timers. Typically at least gpt1 and 12 are
> always powered. That allows us to leave out the hwmod dependency for system
> timers.
>
> Or am I forgetting some other dependency with our system timers?

both counter32 and GP timer have to be enabled through sysc registers.
They are in "Force idle" state after reset.

>
>> The main reason why I've tried this is because clocksource will be really selected
>> only at fs_initcall time - and at that time we have no restriction for using platform
>> devices, Pm runtime APIs, etc. (exception/blocker is sched_clock :().
>
> Right. But it seems we can leave out quite a bit of the dependencies
> for system timers. We already have gptimer probe not touching the system
> timers later on and can use shared gptimer functions after the clock
> muxing is done.


-- 
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1281066

* Grygorii Strashko <grygorii.strashko@ti.com> [151201 09:13]:
> On 12/01/2015 06:07 PM, Tony Lindgren wrote:
> >
> >Or am I forgetting some other dependency with our system timers?
> 
> both counter32 and GP timer have to be enabled through sysc registers.
> They are in "Force idle" state after reset.

That's fine, those are within the timer address space. I'd rather
leave out the hwmod dependency from the system timers at the cost
of a system timer specific minimal init function.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web