All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Shilimkar, Santosh" <santosh.shilimkar@ti.com>
To: Paul Walmsley <paul@pwsan.com>
Cc: Jean Pihet <j-pihet@ti.com>,
	Jean Pihet <jean.pihet@newoldbits.com>,
	linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	khilman@ti.com, Rajendra Nayak <rnayak@ti.com>,
	Nishanth Menon <nm@ti.com>
Subject: Re: [PATCH v5 0/8] ARM: OMAP2+: PM: introduce the power domains functional states
Date: Thu, 16 Aug 2012 11:20:07 +0530	[thread overview]
Message-ID: <CAMQu2gyV6gX=KFjMZ1r_BwsJXmgjuLMAWHrrLCYLgKEBRne0Ow@mail.gmail.com> (raw)
In-Reply-To: <alpine.DEB.2.00.1208151842380.8833@utopia.booyaka.com>

Paul,

On Thu, Aug 16, 2012 at 6:18 AM, Paul Walmsley <paul@pwsan.com> wrote:
>
> Hi Santosh,
>
> On Wed, 15 Aug 2012, Shilimkar, Santosh wrote:
>
> > On Wed, Aug 15, 2012 at 3:32 PM, Jean Pihet <jean.pihet@newoldbits.com>
> > wrote:
> >
> > I didn't find any mention here about why are we going in this path and
> > not
> > in the direction proposed in another RFC [1]
> > I have already given my comments[2] against the introduction of another
> > PD
> > layer which can be avoided easily as demonstrated by the RFC[1]. The
> > comments
> > are still applicable for this series too.
> >
> > We really need to reduce OMAP specific framework overhead and
> > move towards more generic PM frameworks. For me, this series is
> > a step back-ward from that direction. Am really sorry for being critical
> > again but I remain unconvinced about this series and the problem it
> > is trying to solve.
> >
> > May be you have valid reasons not to follow the approach in [1] and in
> > that case, it will be good to clarify that so that some of us get
> > to know your rationale.
>
> I've asked Jean to handle the work of evaluating and/or integrating any
> feedback from you and Rajendra into this series.  Jean, has this latest
> series fully considered those issues?  Or are there still some areas of
> misalignment / lack of clarity?
>
Thanks for the information. The main objection to this series was to
not add un-necessary glue layer which still remains.

From our discussion in past on and off list, your main intention for such
a series was -

1. Need a way to support OSWR.
- OSWR by definition is a RET with configurable logic and memory states.
Its a true power state from PD point of view and its not a logical state.
Now since we have agreed to make the OSWR as a static definition
(in all products so far OSWR is used as a static definition with logic
lost, memory retained kind of configuration.)

- The above requirement can be easily fixed by adding the OSWR
as an additional basic power state as demonstrated in RFC.

- There is no need to add another glue layer for above.

2. Locking so that the low level APIs don't race and henec abstracting the
exported API to 1 or 2 and making rest as private functions.

-- Even before this series, except low level PM code, only one
common API was used to set the PD low power state.
int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 pwrst)

-- Once we make OSWR as basic power state, we also avoid usage of
pwrdm_set_logic_retst() API.

-- We implement lock at this API and export only above API +
may be omap_get_pwrdm_state() kind of API based on need.

-- This solves the second requirement too.

Even if we have more requirement, they can be addressed
too without need of another layer.

If you look at the diffstat alone between two approaches, it is
evident how small piece of code is needed to support above.
Am not too much into the lines of code but basic objection we
have is not to add another glue layer.

Thinking bit loud, for the logical layer for power domain
we should move towards common device power domain
APIs and if needed add/enhance them to support OMAP.
   drivers/base/power/domain.c
May be this though is bit premature but the intetion is
to move towards generic linux framework.

> Anyway.  If there's a problem with this process, it sounds like you,
> Rajendra, Jean, Benoît and I should schedule some time to talk over the
> same issues that you discussed with me on the phone.  Perhaps next week?
>
We can surely do a call if needed. But the comments given so far and the
RFC makes things more or less clear the contention point against the
$subject series.

Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: santosh.shilimkar@ti.com (Shilimkar, Santosh)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v5 0/8] ARM: OMAP2+: PM: introduce the power domains functional states
Date: Thu, 16 Aug 2012 11:20:07 +0530	[thread overview]
Message-ID: <CAMQu2gyV6gX=KFjMZ1r_BwsJXmgjuLMAWHrrLCYLgKEBRne0Ow@mail.gmail.com> (raw)
In-Reply-To: <alpine.DEB.2.00.1208151842380.8833@utopia.booyaka.com>

Paul,

On Thu, Aug 16, 2012 at 6:18 AM, Paul Walmsley <paul@pwsan.com> wrote:
>
> Hi Santosh,
>
> On Wed, 15 Aug 2012, Shilimkar, Santosh wrote:
>
> > On Wed, Aug 15, 2012 at 3:32 PM, Jean Pihet <jean.pihet@newoldbits.com>
> > wrote:
> >
> > I didn't find any mention here about why are we going in this path and
> > not
> > in the direction proposed in another RFC [1]
> > I have already given my comments[2] against the introduction of another
> > PD
> > layer which can be avoided easily as demonstrated by the RFC[1]. The
> > comments
> > are still applicable for this series too.
> >
> > We really need to reduce OMAP specific framework overhead and
> > move towards more generic PM frameworks. For me, this series is
> > a step back-ward from that direction. Am really sorry for being critical
> > again but I remain unconvinced about this series and the problem it
> > is trying to solve.
> >
> > May be you have valid reasons not to follow the approach in [1] and in
> > that case, it will be good to clarify that so that some of us get
> > to know your rationale.
>
> I've asked Jean to handle the work of evaluating and/or integrating any
> feedback from you and Rajendra into this series.  Jean, has this latest
> series fully considered those issues?  Or are there still some areas of
> misalignment / lack of clarity?
>
Thanks for the information. The main objection to this series was to
not add un-necessary glue layer which still remains.

>From our discussion in past on and off list, your main intention for such
a series was -

1. Need a way to support OSWR.
- OSWR by definition is a RET with configurable logic and memory states.
Its a true power state from PD point of view and its not a logical state.
Now since we have agreed to make the OSWR as a static definition
(in all products so far OSWR is used as a static definition with logic
lost, memory retained kind of configuration.)

- The above requirement can be easily fixed by adding the OSWR
as an additional basic power state as demonstrated in RFC.

- There is no need to add another glue layer for above.

2. Locking so that the low level APIs don't race and henec abstracting the
exported API to 1 or 2 and making rest as private functions.

-- Even before this series, except low level PM code, only one
common API was used to set the PD low power state.
int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 pwrst)

-- Once we make OSWR as basic power state, we also avoid usage of
pwrdm_set_logic_retst() API.

-- We implement lock at this API and export only above API +
may be omap_get_pwrdm_state() kind of API based on need.

-- This solves the second requirement too.

Even if we have more requirement, they can be addressed
too without need of another layer.

If you look at the diffstat alone between two approaches, it is
evident how small piece of code is needed to support above.
Am not too much into the lines of code but basic objection we
have is not to add another glue layer.

Thinking bit loud, for the logical layer for power domain
we should move towards common device power domain
APIs and if needed add/enhance them to support OMAP.
   drivers/base/power/domain.c
May be this though is bit premature but the intetion is
to move towards generic linux framework.

> Anyway.  If there's a problem with this process, it sounds like you,
> Rajendra, Jean, Beno?t and I should schedule some time to talk over the
> same issues that you discussed with me on the phone.  Perhaps next week?
>
We can surely do a call if needed. But the comments given so far and the
RFC makes things more or less clear the contention point against the
$subject series.

Regards
Santosh

  reply	other threads:[~2012-08-16  5:50 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-15 10:02 [PATCH v5 0/8] ARM: OMAP2+: PM: introduce the power domains functional states Jean Pihet
2012-08-15 10:02 ` Jean Pihet
2012-08-15 10:02 ` [PATCH 1/8] ARM: OMAP2+: PM: introduce " Jean Pihet
2012-08-15 10:02   ` Jean Pihet
2012-08-15 10:02 ` [PATCH 2/8] ARM: OMAP2+: PM: introduce power domains achievable " Jean Pihet
2012-08-15 10:02   ` Jean Pihet
2012-08-15 10:02 ` [PATCH 3/8] ARM: OMAP2+: PM: add a lock to protect the powerdomains next state Jean Pihet
2012-08-15 10:02   ` Jean Pihet
2012-08-15 10:02 ` [PATCH 4/8] ARM: OMAP2+: PM: use the functional power states API Jean Pihet
2012-08-15 10:02   ` Jean Pihet
2012-08-15 10:02 ` [PATCH 5/8] ARM: OMAP2+: PM: use power domain functional state in stats counters Jean Pihet
2012-08-15 10:02   ` Jean Pihet
2012-08-15 10:02 ` [PATCH 6/8] ARM: OMAP2+: PM debug: trace the functional power domains states Jean Pihet
2012-08-15 10:02   ` Jean Pihet
2012-08-15 10:02 ` [PATCH 7/8] ARM: OMAP2+: powerdomain: add error logs Jean Pihet
2012-08-15 10:02   ` Jean Pihet
2012-08-15 10:02 ` [PATCH 8/8] ARM: OMAP2+: PM: reorganize the powerdomain API in public and private parts Jean Pihet
2012-08-15 10:02   ` Jean Pihet
2012-08-15 17:05 ` [PATCH v5 0/8] ARM: OMAP2+: PM: introduce the power domains functional states Shilimkar, Santosh
2012-08-15 17:05   ` Shilimkar, Santosh
2012-08-16  0:48   ` Paul Walmsley
2012-08-16  0:48     ` Paul Walmsley
2012-08-16  5:50     ` Shilimkar, Santosh [this message]
2012-08-16  5:50       ` Shilimkar, Santosh
2012-09-10 15:09       ` Tero Kristo
2012-09-10 15:09         ` Tero Kristo
2012-09-11  7:50         ` Pihet-XID, Jean
2012-09-11  7:50           ` Pihet-XID, Jean

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='CAMQu2gyV6gX=KFjMZ1r_BwsJXmgjuLMAWHrrLCYLgKEBRne0Ow@mail.gmail.com' \
    --to=santosh.shilimkar@ti.com \
    --cc=j-pihet@ti.com \
    --cc=jean.pihet@newoldbits.com \
    --cc=khilman@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=nm@ti.com \
    --cc=paul@pwsan.com \
    --cc=rnayak@ti.com \
    /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.