lkml.org 
[lkml]   [2019]   [Jan]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH v10 01/27] PM / Domains: Add generic data pointer to genpd_power_state struct
    Date
    On Tuesday, December 18, 2018 12:53:28 PM CET Ulf Hansson wrote:
    > On Tue, 18 Dec 2018 at 11:39, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
    > >
    > > On 29/11/2018 18:46, Ulf Hansson wrote:
    > > > Let's add a data pointer to the genpd_power_state struct, to allow a genpd
    > > > backend driver to store per state specific data. In order to introduce the
    > > > pointer, we also need to adopt how genpd frees the allocated data for the
    > > > default genpd_power_state struct, that it may allocate at pm_genpd_init().
    > > >
    > > > More precisely, let's use an internal genpd flag to understand when the
    > > > states needs to be freed by genpd. When freeing the states data in
    > > > genpd_remove(), let's also clear the corresponding genpd->states pointer
    > > > and reset the genpd->state_count. In this way, a genpd backend driver
    > > > becomes aware of when there is state specific data for it to free.
    > > >
    > > > Cc: Lina Iyer <ilina@codeaurora.org>
    > > > Co-developed-by: Lina Iyer <lina.iyer@linaro.org>
    > > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
    > > > ---
    > > >
    > > > Changes in v10:
    > > > - Update the patch allow backend drivers to free the states specific
    > > > data during genpd removal. Due to this added complexity, I decided to
    > > > keep the patch separate, rather than fold it into the patch that makes
    > > > use of the new void pointer, which was suggested by Rafael.
    > > > - Claim authorship of the patch as lots of changes has been done since
    > > > the original pick up from Lina Iyer.
    > > >
    > > > ---
    > > > drivers/base/power/domain.c | 8 ++++++--
    > > > include/linux/pm_domain.h | 3 ++-
    > > > 2 files changed, 8 insertions(+), 3 deletions(-)
    > > >
    > > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
    > > > index 7f38a92b444a..e27b91d36a2a 100644
    > > > --- a/drivers/base/power/domain.c
    > > > +++ b/drivers/base/power/domain.c
    > > > @@ -1620,7 +1620,7 @@ static int genpd_set_default_power_state(struct generic_pm_domain *genpd)
    > > >
    > > > genpd->states = state;
    > > > genpd->state_count = 1;
    > > > - genpd->free = state;
    > > > + genpd->free_state = true;
    > > >
    > > > return 0;
    > > > }
    > > > @@ -1736,7 +1736,11 @@ static int genpd_remove(struct generic_pm_domain *genpd)
    > > > list_del(&genpd->gpd_list_node);
    > > > genpd_unlock(genpd);
    > > > cancel_work_sync(&genpd->power_off_work);
    > > > - kfree(genpd->free);
    > > > + if (genpd->free_state) {
    > > > + kfree(genpd->states);
    > > > + genpd->states = NULL;
    > > > + genpd->state_count = 0;
    > >
    > > Why these two initializations? After genpd_remove, this structure
    > > shouldn't be used anymore, no ?
    >
    > Correct.
    >
    > >
    > > > + }
    > >
    > > Instead of a flag, replacing the 'free' pointer to a 'free' callback
    > > will allow to keep the free path self-encapsulated in domain.c
    > >
    > > genpd->free(genpd->states);
    >
    > Right, I get your idea and it makes sense. Let me convert to that.

    OK, so I'm expecting an update here.

    \
     
     \ /
      Last update: 2019-01-11 11:55    [W:6.360 / U:0.084 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site