User Details
- User Since
- Feb 26 2021, 3:47 PM (280 w, 3 d)
Yesterday
(And, unless you're in a case where the fix is not simple or you fear about possible complications, no need to upload a review for this, just commit the fix.)
Haven't tried to build it myself, but if that fixes the build, just go ahead. As a side note, I'll post a review removing this code relatively soon.
Fri, Jul 10
Two more things to consider. First, you finally reduce items expected to be read-only to an integer (field value in struct acpi_cppc_rdonly_item), but some of these are dynamic and may change during runtime, in which case we have to keep the full register specification. Second, in the _CPC example I sent you offline for an AMD machine where processors do not report the CPPC extended feature flag, most present items have the PCC access method, so won't be covered by the code here. In the same example, there are only two FFH, for the ReferencePerformanceCounterRegister and DeliveredPerformanceCounterRegister, and the addresses seem to correspond to the MPERF and APERF MSR numbers respectively.
So, as I feared, the CPC parsing library needs significant work.
Disappearance of SYS_RES_FFH (see other revisions in the stack) makes this obsolete.
As noted on previous revisions, SYS_RES_FFH should not exist, nor should acpi_ffh_write() and acpi_ffh_read(). So, basically, this needs a full rework.
acpi_get_rman() should always return NULL on FFH, and it's absolutely inconceivable to match devices on FFH values.
I do not think that this change makes sense, and probably would just drop it.
Looks good. One suggestion in an inline comment. Only thing now is the dependency on the CPPC lib, I'll review the other revisions and come back to here.
Thu, Jul 9
Wed, Jul 8
Be consistent and declare all small helpers as inline.
More compact tests on the max LVT slot, based on D58110.
Tue, Jul 7
Edits from gperciva@
New D58086 for you to check.
Going to commit this. Will apply a few additional minor style (mostly, names, and a declaration moved to the corresponding block) changes in a separate commit.
Looks good (see also inline comment). Could you update the Phab revision's description with the new commit message?
Looks much better like that. :-) "Suggested by: olce" appreciated.
Cater to gpercival's comments
Mon, Jul 6
However, if you feel that the renaming is necessary and local variables should be named runq, I'm learning towards runq_global over global_runq since it has similar naming to runq_pcpu although the name is not grammatically correct.
- Add mention to D58000's work
- 4BSD: Switch from MAXCPU fixed-length array to DPCPU for runq_pcpu (D58000 please review)
The commit message is not completely clear on first read, in particular with respect to why the change here saves memory. Also, there are typos. Suggestions:
Fix typo (missing space).
Fix item lists.
Sun, Jul 5
Removing the hyphen in the first line is fine, but then that should be mentioned in the commit message. I can add a sentence to that effect myself. Which mail do you want to use? The one that can be inferred from your Phabricator login? I also see a mail at "zohomail" in the tree that could match. If you have to disclose the full name and don't want to do that here, then please mail it to me (olce@).
Fri, Jul 3
Thanks, will do some followup next week.
Thu, Jul 2
Simplify.
Wed, Jul 1
Looks good modulo some minor style (in a broad sense) improvements. To ease things, I could apply these changes myself on commit. Please tell me if you object to that.