ope, thanks for catching that. I also forgot to update the acpi(4) manpage and a couple other comments in other files but I can commit a fix for those separately

@mhorne just committed f814650aaf788323b3d485d96996fce6cd7b2d7f, does it work with that? I'm surprised it was hanging considering if the suspend stype is unknown it should just not do anything. If it's still suspending I'll send you a patch to try out to exit out of acpi_EnterSleepState() early and see what's going on there.

Strangely, acpiconf -s 3 succeeds in both cases. I believe this is what I was using before, based on the previous implementation of zzz.

fix path ordering

In D56918#1309166, @mhorne wrote:

I tried it out. The machine didn't suspend, instead it just froze :/

I haven't looked at the pathways for this new power device yet, but I will try to familiarize myself with it. That's all I can report today.

respond to mhorne@'s suggestions

check enum overflow

Respond to olce@'s comments:

(note that we will wait for olce@ to commit his acpi_spmc stack to make things simpler)

logic here seems sound

oops, completely screwed up the diff here!

rebase

rebase

rebase to after D56953

rebase to before D55508

Ok. But after having read some AML, I'm leaning towards the opposite thinking at the moment. E.g., if vendors would deactivate some function for a particular model (because the base firmware is shared between models, and that specific model does not support it or something doesn't work or is not relevant), they would do it primarily through the enum function, but would not necessarily remove the actual code. Testing the series on an AMD Framework Laptop is important to confirm (or infirm) all this.

In D56877#1304358, @olce wrote:

In D56877#1304244, @obiwac wrote:

i mean from my pov I agree with you, but if I was new to this code and was reading it for the first time I feel like this might confuse me

Yes, I was thinking also about that in the meantime. I'll add a comment before acpi_spmc_run(), that should be enough.

Oh yes? Do you remember some details about that? Was it specifically linked with the revision used? In that latter case, perhaps D56882 fixes that.

I do not have strong feelings, it's just that I find acpi_spmc_run() much simpler and not really ambiguous in that context (what could acpi_spmc(4) run except a DSM function?).

In D56807#1304184, @olce wrote:

In D56807#1303792, @obiwac wrote:

Are there any guidelines w.r.t. capitalization here? I couldn't find anything in style(9), and it seems to me like the prevailing convention in the code is to not capitalize these

AFAIK, there are none, and it's more or less up to authors. So that's my preference. :-)

But I really do not understand why some people insist on not capitalizing (which is not the same as, they don't do it because they are mimicking things they have seen or because of lazyness or because they simply don't care). Capitalizing helps delineating messages/sentences quickly, even after, e.g., panic:. Maybe the drawback for some is that they find it catching too much attention? I don't know.

In D56806#1304179, @olce wrote:

In D56806#1303791, @obiwac wrote:

I mean, technically these functions aren't just parsing the constraints, they're also getting them... I don't know if I agree with this change

Maybe it's a question of words, but these functions do not "get" them in the sense they query the firmware, that is done only at start of acpi_spmc_get_constraints(), they just interpret/parse the result, building our own internal structures.

fine with this conceptually, but just bear in mind that there are some situations where enum functions report the wrong functions, and Linux just ignores this (have seen this with the AMD DSMs e.g.). So this might cause problems on some machines.

yeah, this logic is much better

not sure I like how generic the function name acpi_spmc_run() is. Technically it is actually running a DSM, just with a bunch of other options so the AML interpreter knows how to dispatch the function. How about acpI_spmc_run_dsm_fn() or something like that?

could we just roll this change into D56756?

Are there any guidelines w.r.t. capitalization here? I couldn't find anything in style(9), and it seems to me like the prevailing convention in the code is to not capitalize these

I mean, technically these functions aren't just parsing the constraints, they're also getting them... I don't know if I agree with this change

i haven't entirely finished reviewing this, but here's a few comments already. Overall I do really like these changes; makes things a bit saner

respond to review comments

Rebase onto D56920

Forgot to change this in all the places it's used

also, TODO, update manpage