On Thu, 5 Dec 2019 13:17:22 +0100
Laszlo Ersek <ler...@redhat.com> wrote:
> On 12/04/19 18:05, Igor Mammedov wrote:
> > Correct returned value description in case 'Command field' == 0x0,
> > it's in not PXM but CPU selector value with pending event
>
> (1) s/in not/not/
>
> >
> > In addition describe 0 blanket value in case of not supported
> > 'Command field' value.
> >
> > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> > ---
> > docs/specs/acpi_cpu_hotplug.txt | 11 +++++------
> > 1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/docs/specs/acpi_cpu_hotplug.txt
> > b/docs/specs/acpi_cpu_hotplug.txt
> > index 4e65286..19c508f 100644
> > --- a/docs/specs/acpi_cpu_hotplug.txt
> > +++ b/docs/specs/acpi_cpu_hotplug.txt
> > @@ -56,9 +56,8 @@ read access:
> > 3-7: reserved and should be ignored by OSPM
> > [0x5-0x7] reserved
> > [0x8] Command data: (DWORD access)
> > - in case of error or unsupported command reads is 0xFFFFFFFF
> > - current 'Command field' value:
> > - 0: returns PXM value corresponding to device
> > + contains 0 unless last stored in 'Command field' value is one of:
> > + 0: contains 'CPU selector' value of a CPU with pending
> > event[s]
>
> (2) I think we can improve the word order:
>
> last stored in 'Command field' value
> ->
> value last stored in 'Command field'
>
> >
> > write access:
> > offset:
> > @@ -81,9 +80,9 @@ write access:
> > value:
> > 0: selects a CPU device with inserting/removing events and
> > following reads from 'Command data' register return
> > - selected CPU (CPU selector value). If no CPU with events
> > - found, the current CPU selector doesn't change and
> > - corresponding insert/remove event flags are not set.
> > + selected CPU ('CPU selector' value).
> > + If no CPU with events found, the current 'CPU selector'
> > doesn't
> > + change and corresponding insert/remove event flags are not
> > set.
>
> (3) AFAICT this is only a -- useful! -- re-wrapping.
Not sure what you are trying to say here ...
> But, since we are
> modifying this section anyway, can we replace "flags are not set" with
> "flags are left unchanged" or "flags are not modified"?
sure
> "set" is ambiguous with bit fields: it can mean "rewritten", and it can
> mean "set to 1".
>
> > 1: following writes to 'Command data' register set OST event
> > register in QEMU
> > 2: following writes to 'Command data' register set OST status
> >
>
> Anyway, these are all superficial comments. Pick up whatever you agree
> with. Either way:
>
> Reviewed-by: Laszlo Ersek <ler...@redhat.com>
>
> Thanks!
> Laszlo
