mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Sudeep Holla <sudeep.holla@arm.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>,
	linux-acpi@vger.kernel.org,
	Vikas Sajjan <vikas.cha.sajjan@hpe.com>, Sunil <sunil.vl@hpe.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	PrashanthPrakash <pprakash@codeaurora.org>,
	Al Stone <al.stone@linaro.org>,
	Ashwin Chaugule <ashwin.chaugule@linaro.org>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Vincent Guittot <vincent.guittot@linaro.org>
Subject: Re: [PATCH v7 2/6] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
Date: Mon, 04 Jul 2016 15:17:18 +0200	[thread overview]
Message-ID: <6955030.D47pi9uhTn@vostro.rjw.lan> (raw)
In-Reply-To: <577A5DD3.4050901@arm.com>

On Monday, July 04, 2016 02:00:03 PM Sudeep Holla wrote:
> 
> On 01/07/16 14:07, Daniel Lezcano wrote:
> > On 06/28/2016 03:55 PM, Sudeep Holla wrote:
> >> ACPI 6.0 introduced an optional object _LPI that provides an alternate
> >> method to describe Low Power Idle states. It defines the local power
> >> states for each node in a hierarchical processor topology. The OSPM can
> >> use _LPI object to select a local power state for each level of processor
> >> hierarchy in the system. They used to produce a composite power state
> >> request that is presented to the platform by the OSPM.
> >>
> >> Since multiple processors affect the idle state for any non-leaf
> >> hierarchy
> >> node, coordination of idle state requests between the processors is
> >> required. ACPI supports two different coordination schemes: Platform
> >> coordinated and  OS initiated.
> >>
> >> This patch adds initial support for Platform coordination scheme of LPI.
> >>
> >> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> >> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> >> ---
> >
> > Hi Sudeep,
> >
> > I looked at the acpi processor idle code sometime ago and from my POV,
> > it was awful, unnecessary complex and very difficult to maintain. The
> > usage of flags all over the places is significant of the lack of control
> > of the written code.
> >
> 
> So you have any specific things in mind ? That's too broad and I know
> it's not so clean, but it's so for legacy reasons. I would leave that
> to Rafael to decide as it takes lots of testing to clean up these code.

The cleanup needs to be done at one point.

Question is when to do it, before adding LPI support or after doing that
(and each option has its pros and cons IMO).

> > Even if you are not responsible of this implementation, the current
> > situation forces you to add more awful code on top of that, which is
> > clearly against "making Linux better".
> >
> 
> OK

So if there are cases in which you need to make the code more complex
because of the legacy stuff in there, I'd say it's better to clean it up
first.

Thanks,
Rafael

  reply	other threads:[~2016-07-04 13:12 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-28 13:55 [PATCH v7 0/6] ACPI / processor_idle: Add ACPI v6.0 LPI support Sudeep Holla
2016-06-28 13:55 ` [PATCH v7 1/6] ACPI / processor_idle: introduce ACPI_PROCESSOR_CSTATE Sudeep Holla
2016-06-28 13:55 ` [PATCH v7 2/6] ACPI / processor_idle: Add support for Low Power Idle(LPI) states Sudeep Holla
2016-07-01 13:07   ` Daniel Lezcano
2016-07-04 13:00     ` Sudeep Holla
2016-07-04 13:17       ` Rafael J. Wysocki [this message]
2016-07-04 13:42         ` Sudeep Holla
2016-07-04 14:11           ` Rafael J. Wysocki
2016-06-28 13:55 ` [PATCH v7 3/6] arm64: cpuidle: drop __init section marker to arm_cpuidle_init Sudeep Holla
2016-06-28 13:55 ` [PATCH v7 4/6] cpuidle: introduce HAVE_GENERIC_CPUIDLE_ENTER for ARM{32,64} platforms Sudeep Holla
2016-07-07 13:21   ` Rafael J. Wysocki
2016-07-07 13:34     ` Sudeep Holla
2016-07-07 14:49       ` Rafael J. Wysocki
2016-07-07 15:48         ` Sudeep Holla
2016-06-28 13:55 ` [PATCH v7 5/6] arm64: add support for ACPI Low Power Idle(LPI) Sudeep Holla
2016-06-28 13:55 ` [PATCH v7 6/6] ACPI : enable ACPI_PROCESSOR_IDLE on ARM64 Sudeep Holla

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=6955030.D47pi9uhTn@vostro.rjw.lan \
    --to=rjw@rjwysocki.net \
    --cc=al.stone@linaro.org \
    --cc=ashwin.chaugule@linaro.org \
    --cc=daniel.lezcano@linaro.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=pprakash@codeaurora.org \
    --cc=sudeep.holla@arm.com \
    --cc=sunil.vl@hpe.com \
    --cc=vikas.cha.sajjan@hpe.com \
    --cc=vincent.guittot@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome