Re: [PATCH 1/3] ACPI: PCC: Add PCC shared memory region command and status bitfields

[lihuisong (C)]

在 2023/9/27 21:59, Sudeep Holla 写道:
> On Wed, Sep 27, 2023 at 10:07:15AM +0800, lihuisong (C) wrote:
> > Hi Sudeep,
> >
> > 在 2023/9/26 20:28, Sudeep Holla 写道:
> > > Define the common macros to use when referring to various bitfields in
> > > the PCC generic communications channel command and status fields.
> > Can you define the bit0 macros in the "flags" for Extended PCC Subspace
> > Shared Memory Region?
> Sure I will take a look and include it in v2 if applicable.
Thanks
> > > Currently different drivers that need to use these bitfields have defined
> > > these locally. This common macro is intended to consolidate and replace
> > > those.
> > >
> > > Cc: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > > Signed-off-by: Sudeep Holla <sudeep.holla@xxxxxxx>
> > > ---
> > >    include/acpi/pcc.h | 11 +++++++++++
> > >    1 file changed, 11 insertions(+)
> > >
> > > diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
> > > index 73e806fe7ce7..66d9934c2ee4 100644
> > > --- a/include/acpi/pcc.h
> > > +++ b/include/acpi/pcc.h
> > > @@ -18,7 +18,18 @@ struct pcc_mbox_chan {
> > >    	u16 min_turnaround_time;
> > >    };
> > > +/* Generic Communications Channel Shared Memory Region */
> > > +#define PCC_SIGNATURE			0x50424300
> > Why is this signature 0x50424300?
> It is as per the specification.
>
> > In ACPI spec, this signature is all 0x50434303.
> No, not exactly. It is just an example.
> The PCC signature - The signature of a subspace is computed by a bitwise-or
> of the value 0x50434300 with the subspace ID. For example, subspace 3 has
> signature 0x50434303
Sorry for my mistake. I know this.
I mean, why doesn't the following macro follow spec and define this 
signature as 0x504*3*430.
"#define PCC_SIGNATURE **0x504*2*4300*"*
Because it seems that all version of ACPI spec is 0x5043430.
> And I see the driver you mentioned(drivers/soc/hisilicon/kunpeng_hccs.c)
> is doing the right thing. I am bit confused as why you being the author
> of the driver are now confused.
I used 0x50424300 instead of 0x50424300 according to the spec.
>