Re: [PATCH 2/4] platform/x86/intel/pmc: Dump raw SLP_Sx_DBG registers and distinguish between them

[Xi Pardee <xi.pardee@linux.intel.com>]

On 10/13/2025 11:05 AM, Guilherme G. Piccoli wrote:
> Hi Xi, thanks for you feedback. Comments inline:
>
> On 13/10/2025 14:39, Xi Pardee wrote:
>> [...]
>> I am still uncertain about the added value of this patch. Could you
>> please elaborate on how displaying the register name would assist with
>> debugging?
>>
>> The registers in slps0_dbg_maps are intended for diagnosing slps0
>> related issues, and the register names should follow the format
>> SLP_S0_DBG_(1,2,3).
>>
> The names do not help with debugging issues, definitely. It's just a
> detail, an improvement - or at least, I consider the output a bit better
> with this patch. But that part is superfluous, if you prefer not to
> change it, I'm totally OK. And ofc you / Ilpo are right, I changed the
> naming in a wrong way, I'd fix that in next iteration (thanks for noticing).
>
>
>> Additionally, the full 32-bit register data should not be displayed
>> here, as some of the bits are reserved and must not be exposed to users.
>>
> Now, this one I respectfully disagree - these raw values have absolutely
> no meaning for the end-user, exactly due to the reason you said: there
> are many reserved bits. So, for a normal user, a piece of hex values,
> junk...
>
> But for Intel, it might be the case you do have internal specs detailing
> these reserved bits, this could be meaningful. That's exactly the
> purpose of this patch: when debugging rare to reproduce failures, having
> this raw register could help Intel to determine the cause of odd
> failures and eventually fix it. No harm (or meaning) for end-users though.
>
> Of course the driver is from Intel, and as maintainers you could decide
> to not show the raw debug data from these registers - fine as well, let
> me know what you prefer.
>
> Thanks,
>
>
> Guilherme

Hi,

We would prefer not to show reserved bits as they don't provide values 
to the users.

Thanks!

Xi