On 04/08/2023 13:10, Mark Brown wrote:

On Fri, Aug 04, 2023 at 09:52:16AM +0100, James Clark wrote:

...

TRFCR_EL2_CX needs to become TRFCR_ELx_CX to avoid unnecessary duplication and make the SysregFields block re-usable.

That field is only present in the EL2 version. I would tend to leave the registers split for that reason, there's some minor potential for confusion if people refer to the sysreg file rather than the docs, or potentially confuse some future automation. However that's not a super strongly held opinion.

True, the potential for confusion is a good reason to not try to avoid duplication. Probably helps if it is ever auto generated or validated as well.

I could update it on the next version. But do I leave all the existing _ELx usages in the code, or change them all to _EL1 (Except CX_EL2)? To leave them as _ELx sysreg would look like this, even though _EL1 would probably be more accurate:

SysregFields TRFCR_EL2 Res0 63:7 UnsignedEnum 6:5 TS 0b0001 VIRTUAL 0b0010 GUEST_PHYSICAL 0b0011 PHYSICAL EndEnum Res0 4 Field 3 CX Res0 2 Field 1 E2TRE Field 0 E0TRE EndSysregFields

SysregFields TRFCR_ELx Res0 63:7 UnsignedEnum 6:5 TS 0b0001 VIRTUAL 0b0010 GUEST_PHYSICAL 0b0011 PHYSICAL EndEnum Res0 4:2 Field 1 ExTRE Field 0 E0TRE EndSysregFields

Sysreg TRFCR_EL1 3 0 1 2 1 Fields TRFCR_ELx EndSysreg

Sysreg TRFCR_EL2 3 4 1 2 1 Fields TRFCR_EL2 EndSysreg

Sysreg TRFCR_EL12 3 5 1 2 1 Fields TRFCR_ELx EndSysreg

Otherwise this checks out against DDI0601 2023-06:

Reviewed-by: Mark Brown broonie@kernel.org

Thanks for the review