Re: [PATCH v3 1/8] arm64/sysreg: Add definitions for immediate versions of MSR ALLINT

[Liao, Chang]

Hi, Mark

在 2024/6/4 21:29, Mark Brown 写道:
> On Mon, Jun 03, 2024 at 11:26:39AM +0800, Liao, Chang wrote:
> 
>> Mark, Is your concern is that the series of pstate related macro name in
>> sysregs.h are lack of self-explanatory nature, which make it diffuclt to
>> understand their functionality and purpose? If so, I daft some alternative
>> macro names in the code below, looking forward to your feedback, or if you
>> have any proposal for making these helpers discoverable.
> 
> ...
> 
>> -#define SET_PSTATE(x, r)               __emit_inst(0xd500401f | PSTATE_ ## r | ((!!x) << PSTATE_Imm_shift))
>> +#define MSR_PSTATE_ENCODE(x, r)                __emit_inst(0xd500401f | PSTATE_ ## r | ((!!x) << PSTATE_Imm_shift))
> 
> Possibly, yes?  TBH I was thinking of a comment but that does have "MSR"
> in it so might come up in greps.  Not sure what others would prefer.

I am going to push revision v4 of the interrupt masking patchset, this revision
involves several improvements based on feedback and further testing:

- Addressed feedback comes form Mark Brown.

- Enrich the commit messages of patch includes major changes to provide more
  detailed explanations of the implemenations, purpose and potential result.
  This aims to improve developer understanding and reviewing efficiency.

- Revising the function names of logical interrupt masking to better reflect
  their purpose.

- Fixed bugs during local testing on platform with FEAT_NMI support.

Additionally, i note you left a feedback as below.

>...I've started looking at this in the series. There are some subtleties here, and
>I don't think the helpers in this series are quite right as-is. I will try to
get back to you next week with a description of those; it'll take a short while
to write that up correctly and clearly...

I appreciate your time and look forward your further feedback at your convenience.

Thanks.

-- 
BR
Liao, Chang