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

["Guilherme G. Piccoli" <gpiccoli@igalia.com>]

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