Re: [PATCH] ARM: imx6: pm: declare pm domain latency on power_state struct.

[Lina Iyer <lina.iyer@linaro.org>]

On Mon, Nov 23 2015 at 06:42 -0700, Lucas Stach wrote:
>Am Montag, den 23.11.2015, 14:31 +0100 schrieb Lucas Stach:
>> Am Mittwoch, den 18.11.2015, 07:57 -0700 schrieb Lina Iyer:
>> > From: Axel Haslam <ahaslam+renesas@baylibre.com>
>> >
>> > The generic_pm_domain structure uses an array of latencies to be able to
>> > declare multiple intermediate states.
>> >
>> > Declare a single "OFF" state with the default latencies So that the
>> > power_off_latency_ns and power_on_latency_ns fields of generic_pm_domain
>> > structure can be eventualy removed.
>> >
>> > Signed-off-by: Axel Haslam <ahaslam+renesas@baylibre.com>
>> > Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>> > [Lina: pm_genpd_init() argument change]
>> > ---
>> >  arch/arm/mach-imx/gpc.c | 14 ++++++++++++--
>> >  1 file changed, 12 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c
>> > index 8e7976a..b070e06 100644
>> > --- a/arch/arm/mach-imx/gpc.c
>> > +++ b/arch/arm/mach-imx/gpc.c
>> > @@ -368,13 +368,23 @@ static struct generic_pm_domain imx6q_arm_domain = {
>> >  	.name = "ARM",
>> >  };
>> >
>> > +static struct genpd_power_state imx6q_arm_domain_states[] = {
>>
>> Those states are for the PU, not the ARM domain.
>>
>> > +	{
>> > +		.name = "OFF",
>> > +		.power_off_latency_ns = 25000,
>> > +		.power_on_latency_ns = 2000000,
>> > +	},
>> > +};
>> > +
>> >  static struct pu_domain imx6q_pu_domain = {
>> >  	.base = {
>> >  		.name = "PU",
>> >  		.power_off = imx6q_pm_pu_power_off,
>> >  		.power_on = imx6q_pm_pu_power_on,
>> > -		.power_off_latency_ns = 25000,
>> > -		.power_on_latency_ns = 2000000,
>> > +		.gov = NULL,
>> > +		.status = GPD_STATE_POWER_OFF,
>>
>> The above 2 lines should not be needed. This is a static struct, so .gov
>> will already be NULL. status is set by pm_genpd_init() and the above
>> value is actively wrong.
>>
True.

>> > +		.states = imx6q_arm_domain_states,
>> > +		.state_count = ARRAY_SIZE(imx6q_arm_domain_states),
>> >  	},
>> >  };
>> >
>>
>> How urgent is this patch? I have a series to rework the GPC driver
>> almost ready and I could fold this change in directly if it's okay for
>> this to sit through a review of the rework.
>
>And I just noticed this depends on changes that are not already in
>v4.4-rc. Why is this sent as a single patch and not as part of the
>series adding multiple state support to genpd?
>
Sorry, since this is ARM 32, I had not comple tested it when I sent out
the series. I sent this out after I realized it.

Thanks,
Lina

>Regards,
>Lucas
>
>-- 
>Pengutronix e.K.             | Lucas Stach                 |
>Industrial Linux Solutions   | http://www.pengutronix.de/  |
>