x86-power-control for ARM CPU based system.

[Ed Tanous]

On Thu, Apr 29, 2021 at 8:47 AM Mohaimen Alsamarai
<Mohaimen.Alsamarai at fii-na.com> wrote:
>
> Adding openbmc mail list
>
> -----Original Message-----
> From: Bills, Jason M <jason.m.bills at linux.intel.com>
> Sent: Tuesday, March 23, 2021 4:08 PM
> To: Brandon Ong <Brandon.Ong at fii-na.com>
> Cc: Lancelot Kao <lancelot.cy.kao at fii-na.com>; Mohaimen Alsamarai <Mohaimen.Alsamarai at fii-na.com>
> Subject: Re: x86-power-control for ARM CPU based system.
>
> Hi Brandon,
> On 3/22/2021 3:43 PM, Brandon Ong wrote:
> > Hi Jason,
> >
> > I am currently working on the implementation of the x86-power-control
> > for an ARM CPU based system.
> >
> >
> > Is there a way to add a compile option to x86-power-control in order
> > to change the behavior to support the ARM power control logic if it
> > were to be integrated into x86-power-control?
> x86-power-control was created to solve specific timing issues with our platforms.  It wasn't designed to be a flexible solution for the community to use.

And OpenBMC was initially designed for POWER platforms.  Things change :)

Clearly x86-power-control seems to solve more problems, as a lot of
new platforms seem to be preferring it.  If the code being changed is
messy, unmaintainable, or isn't well abstracted, that's a different
discussion, but outright saying nobody else can make use of
x86-power-control seems problematic, and would lead to a power control
daemon per-platform, which seems hard to maintain, and in looking at
the amd patches, an amd specific daemon would largely just copy-paste
95% of x86-power-control today into something like amd-power-control
if we took this to the logical conclusion.

>
> phosphor-state-manager
> (https://github.com/openbmc/phosphor-state-manager) is the OpenBMC community power state manager.  It is designed for flexibility in how different systems change power state.
>
> Rather than add build modifications to x86-power-control for your needs, I'd recommend that you look at phosphor-state-manager which was designed to be customizable for different systems.

phosphor-state-manager has all the problems that you found when you
went to use it, and found it lacking.  Clearly Brandon has found the
same and is looking to make some (hopefully minor) mods to make
x86-power-control more useful in more contexts.  If it's a matter of
code cleanliness or separation, there's certainly a discussion to be
had here, but effectively saying that everyone should go build their
own version of x86-power-control seems wasteful, as a lot of platforms
share similar properties to what x86-power-control does.

The things I see in the patch are:
1. The ability to invert polarities of the inputs.
2. The ability to disable at compile time some of the watchdogs that
don't make sense on certain platforms.
3. disabling the beeper (which I'm not sure is needed so long as you
handle errors silently).
4. A couple of platform-name-specific hardcodes, that I suspect aren't
needed or can be abstracted.

Is there a way we can avoid the duplication of code in this case?

>
> Thanks,
> -Jason
>
> >
> > Thanks,
> > Brandon
> >