All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lina Iyer <lina.iyer@linaro.org>
To: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Kevin Hilman <khilman@kernel.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Andy Gross <andy.gross@linaro.org>,
	Stephen Boyd <sboyd@codeaurora.org>,
	"linux-arm-msm@vger.kernel.org" <linux-arm-msm@vger.kernel.org>,
	Brendan Jackman <brendan.jackman@arm.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Juri Lelli <Juri.Lelli@arm.com>,
	Axel Haslam <ahaslam+renesas@baylibre.com>
Subject: Re: [PATCH 1/8] PM / Domains: Make genpd state allocation dynamic
Date: Thu, 6 Oct 2016 09:40:52 -0600	[thread overview]
Message-ID: <20161006154052.GB28930@linaro.org> (raw)
In-Reply-To: <CAPDyKFpNnJxueNNk9g7KzHPAsJVQ8xeiaw8OHMevBZ3myhd-xA@mail.gmail.com>

On Thu, Oct 06 2016 at 02:37 -0600, Ulf Hansson wrote:
>On 5 October 2016 at 22:31, Lina Iyer <lina.iyer@linaro.org> wrote:
>> Allow PM Domain states to be defined dynamically by the drivers. This
>> removes the limitation on the maximum number of states possible for a
>> domain.
>>
>> Cc: Axel Haslam <ahaslam+renesas@baylibre.com>
>> Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
<...>
>> -#define GENPD_MAX_NUM_STATES   8 /* Number of possible low power states */
>> -
>>  enum gpd_status {
>>         GPD_STATE_ACTIVE = 0,   /* PM domain is active */
>>         GPD_STATE_POWER_OFF,    /* PM domain is off */
>> @@ -70,7 +68,7 @@ struct generic_pm_domain {
>>         void (*detach_dev)(struct generic_pm_domain *domain,
>>                            struct device *dev);
>>         unsigned int flags;             /* Bit field of configs for genpd */
>> -       struct genpd_power_state states[GENPD_MAX_NUM_STATES];
>> +       struct genpd_power_state *states;
>>         unsigned int state_count; /* number of states */
>>         unsigned int state_idx; /* state that genpd will go to when off */
>>
>> --
>> 2.7.4
>>
>
>In general I like the improvement, but..
>
>This change implies that ->states may very well be NULL. This isn't
>validated by genpd's internal logic when power off/on the domain
>(genpd_power_on|off(), __default_power_down_ok()). You need to fix
>this, somehow.
>
Good point.

>Perhaps the easiest solutions is, when pm_genpd_init() finds that
>->state is NULL, that we allocate a struct genpd_power_state with
>array size of 1 and assign it to ->states. Although, doing this also
>means you need to track that genpd was responsible for the the
>allocation, so it must also free the data from within genpd_remove().
>
>Unless you have other ideas!?
>
I can think of some hacks, but they are uglier than the problem we are
trying to solve. We could drop this patch. Real world situations would
not have more than 8 states and if there is one, we can think about it
then.

Thanks,
Lina

WARNING: multiple messages have this Message-ID (diff)
From: lina.iyer@linaro.org (Lina Iyer)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/8] PM / Domains: Make genpd state allocation dynamic
Date: Thu, 6 Oct 2016 09:40:52 -0600	[thread overview]
Message-ID: <20161006154052.GB28930@linaro.org> (raw)
In-Reply-To: <CAPDyKFpNnJxueNNk9g7KzHPAsJVQ8xeiaw8OHMevBZ3myhd-xA@mail.gmail.com>

On Thu, Oct 06 2016 at 02:37 -0600, Ulf Hansson wrote:
>On 5 October 2016 at 22:31, Lina Iyer <lina.iyer@linaro.org> wrote:
>> Allow PM Domain states to be defined dynamically by the drivers. This
>> removes the limitation on the maximum number of states possible for a
>> domain.
>>
>> Cc: Axel Haslam <ahaslam+renesas@baylibre.com>
>> Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
<...>
>> -#define GENPD_MAX_NUM_STATES   8 /* Number of possible low power states */
>> -
>>  enum gpd_status {
>>         GPD_STATE_ACTIVE = 0,   /* PM domain is active */
>>         GPD_STATE_POWER_OFF,    /* PM domain is off */
>> @@ -70,7 +68,7 @@ struct generic_pm_domain {
>>         void (*detach_dev)(struct generic_pm_domain *domain,
>>                            struct device *dev);
>>         unsigned int flags;             /* Bit field of configs for genpd */
>> -       struct genpd_power_state states[GENPD_MAX_NUM_STATES];
>> +       struct genpd_power_state *states;
>>         unsigned int state_count; /* number of states */
>>         unsigned int state_idx; /* state that genpd will go to when off */
>>
>> --
>> 2.7.4
>>
>
>In general I like the improvement, but..
>
>This change implies that ->states may very well be NULL. This isn't
>validated by genpd's internal logic when power off/on the domain
>(genpd_power_on|off(), __default_power_down_ok()). You need to fix
>this, somehow.
>
Good point.

>Perhaps the easiest solutions is, when pm_genpd_init() finds that
>->state is NULL, that we allocate a struct genpd_power_state with
>array size of 1 and assign it to ->states. Although, doing this also
>means you need to track that genpd was responsible for the the
>allocation, so it must also free the data from within genpd_remove().
>
>Unless you have other ideas!?
>
I can think of some hacks, but they are uglier than the problem we are
trying to solve. We could drop this patch. Real world situations would
not have more than 8 states and if there is one, we can think about it
then.

Thanks,
Lina

  reply	other threads:[~2016-10-06 15:40 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-05 20:31 [PATCH 0/8] PM / Domains: DT support for domain idle states & atomic PM domains Lina Iyer
2016-10-05 20:31 ` Lina Iyer
2016-10-05 20:31 ` [PATCH 1/8] PM / Domains: Make genpd state allocation dynamic Lina Iyer
2016-10-05 20:31   ` Lina Iyer
2016-10-06  8:36   ` Ulf Hansson
2016-10-06  8:36     ` Ulf Hansson
2016-10-06 15:40     ` Lina Iyer [this message]
2016-10-06 15:40       ` Lina Iyer
2016-10-06 19:45       ` Ulf Hansson
2016-10-06 19:45         ` Ulf Hansson
2016-10-06 20:57         ` Lina Iyer
2016-10-06 20:57           ` Lina Iyer
2016-10-07  8:39           ` Ulf Hansson
2016-10-07  8:39             ` Ulf Hansson
2016-10-05 20:31 ` [PATCH 2/8] PM / Domain: Add residency property to genpd states Lina Iyer
2016-10-05 20:31   ` Lina Iyer
2016-10-06  8:38   ` Ulf Hansson
2016-10-06  8:38     ` Ulf Hansson
2016-10-05 20:31 ` [PATCH 3/8] PM / Domains: Allow domain power states to be read from DT Lina Iyer
2016-10-05 20:31   ` Lina Iyer
2016-10-06  8:04   ` Geert Uytterhoeven
2016-10-06  8:04     ` Geert Uytterhoeven
2016-10-06 15:44     ` Lina Iyer
2016-10-06 15:44       ` Lina Iyer
2016-10-06  9:47   ` Ulf Hansson
2016-10-06  9:47     ` Ulf Hansson
2016-10-06 15:53     ` Lina Iyer
2016-10-06 15:53       ` Lina Iyer
2016-10-06 15:54     ` Lina Iyer
2016-10-06 15:54       ` Lina Iyer
2016-10-05 20:31 ` [PATCH 4/8] PM / Domains: Add fwnode provider to genpd states Lina Iyer
2016-10-05 20:31   ` Lina Iyer
2016-10-06 12:01   ` Ulf Hansson
2016-10-06 12:01     ` Ulf Hansson
2016-10-06 15:55     ` Lina Iyer
2016-10-06 15:55       ` Lina Iyer
2016-10-05 20:31 ` [PATCH 5/8] dt/bindings: Update binding for PM domain idle states Lina Iyer
2016-10-05 20:31   ` Lina Iyer
     [not found]   ` <1475699519-109623-6-git-send-email-lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2016-10-06  8:06     ` Geert Uytterhoeven
2016-10-06  8:06       ` Geert Uytterhoeven
     [not found]       ` <CAMuHMdWxft8o493XN2zg1wj2By7P+qBkrvyQOt7WdBnhPAvSXQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-10-06  8:09         ` Geert Uytterhoeven
2016-10-06  8:09           ` Geert Uytterhoeven
     [not found]           ` <CAMuHMdUKbJ1Mt3viEkbCDmZGHQjGi_E=bxEBSHOB1MMY+qfSYQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-10-06 15:56             ` Lina Iyer
2016-10-06 15:56               ` Lina Iyer
2016-10-05 20:31 ` [PATCH 6/8] PM / Domains: Abstract genpd locking Lina Iyer
2016-10-05 20:31   ` Lina Iyer
2016-10-06 10:56   ` Ulf Hansson
2016-10-06 10:56     ` Ulf Hansson
2016-10-06 15:56     ` Lina Iyer
2016-10-06 15:56       ` Lina Iyer
2016-10-05 20:31 ` [PATCH 7/8] PM / Domains: Support IRQ safe PM domains Lina Iyer
2016-10-05 20:31   ` Lina Iyer
2016-10-06 11:34   ` Ulf Hansson
2016-10-06 11:34     ` Ulf Hansson
2016-10-05 20:31 ` [PATCH 8/8] PM / doc: Update device documentation for devices in " Lina Iyer
2016-10-05 20:31   ` Lina Iyer
2016-10-06 11:48   ` Ulf Hansson
2016-10-06 11:48     ` Ulf Hansson
2016-10-05 20:38 ` [PATCH 0/8] PM / Domains: DT support for domain idle states & atomic " Lina Iyer
2016-10-05 20:38   ` Lina Iyer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161006154052.GB28930@linaro.org \
    --to=lina.iyer@linaro.org \
    --cc=Juri.Lelli@arm.com \
    --cc=ahaslam+renesas@baylibre.com \
    --cc=andy.gross@linaro.org \
    --cc=brendan.jackman@arm.com \
    --cc=khilman@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=rjw@rjwysocki.net \
    --cc=sboyd@codeaurora.org \
    --cc=sudeep.holla@arm.com \
    --cc=ulf.hansson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.