Re: [PATCH v2] ppc/pnv: Add QME region for P10

[Daniel Henrique Barboza <danielhb413@gmail.com>]

On 7/7/23 22:16, Nicholas Piggin wrote:
> On Fri Jul 7, 2023 at 5:12 PM AEST, Joel Stanley wrote:
>> The Quad Management Engine (QME) manages power related settings for its
>> quad. The xscom region is separate from the quad xscoms, therefore a new
>> region is added. The xscoms in a QME select a given core by selecting
>> the forth nibble.
>>
>> Implement dummy reads for the stop state history (SSH) and special
>> wakeup (SPWU) registers. This quietens some sxcom errors when skiboot
>> boots on p10.
>>
>> Power9 does not have a QME.
>>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
> 
> Nice, already merged but looks good to me. Just one thing...
> 
>> diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
>> index a4c9d95dc5d3..9bc64635471e 100644
>> --- a/include/hw/ppc/pnv_xscom.h
>> +++ b/include/hw/ppc/pnv_xscom.h
>> @@ -127,6 +127,17 @@ struct PnvXScomInterfaceClass {
>>   #define PNV10_XSCOM_EC(proc)                    \
>>       ((0x2 << 16) | ((1 << (3 - (proc))) << 12))
>>   
>> +#define PNV10_XSCOM_QME(chiplet) \
>> +        (PNV10_XSCOM_EQ(chiplet) | (0xE << 16))
>> +
>> +/*
>> + * Make the region larger by 0x1000 (instead of starting at an offset) so the
>> + * modelled addresses start from 0
>> + */
>> +#define PNV10_XSCOM_QME_BASE(core)     \
>> +    ((uint64_t) PNV10_XSCOM_QME(PNV10_XSCOM_EQ_CHIPLET(core)))
>> +#define PNV10_XSCOM_QME_SIZE        (0x8000 + 0x1000)
> 
> I couldn't work out this comment.

FWIW if a comment is inaccurate/wrong it is OK to fix it during the code
freeze window. No need to hold it until after 8.1 is released. Thanks,


Daniel

> 
> Thanks,
> Nick
>