On 04/07/16 14:17, Rafael J. Wysocki wrote:

On Monday, July 04, 2016 02:00:03 PM Sudeep Holla wrote:

quoted

On 01/07/16 14:07, Daniel Lezcano wrote:

quoted

On 06/28/2016 03:55 PM, Sudeep Holla wrote:

quoted

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" <redacted>
Signed-off-by: Sudeep Holla <redacted>
---

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).

quoted

quoted

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.

I am not sure if Daniel was referring to anything specific. I have
cleaned up in patch 1/6 for cstate. More cleanups can be done there but
needs better understanding and reasoning for the current code which I
don't have as they are mostly x86 related.

Unless someone points me what they would like to change and how, I don't
have much in my mind to do here. Yes it may not look as clean as other
code in the kernel relatively, but without complete understanding of the
history/reasoning for the current state of code I wouldn't touch
anything I don't understand.

I am open to make changes if there's something specific. Sorry I can't
go ahead making changes the way I think based on some vague idea that
the current code is not clean.

-- 
Regards,
Sudeep