> -----Original Message----- > From: Eric Auger <[email protected]> > Sent: 20 May 2026 06:49 > To: Shameer Kolothum Thodi <[email protected]>; qemu- > [email protected]; [email protected] > Cc: [email protected]; [email protected]; [email protected]; Nicolin > Chen <[email protected]>; Nathan Chen <[email protected]>; Matt > Ochs <[email protected]>; Jiandi An <[email protected]>; Jason Gunthorpe > <[email protected]>; [email protected]; Krishnakant Jaju > <[email protected]>; [email protected] > Subject: Re: [PATCH v5 15/32] hw/arm/tegra241-cmdqv: Emulate VCMDQ > register reads >
[...] > > +#define SMMU_CMDQV_VCMDQi_GERRORN_(i) \ > > + REG32(VCMDQ##i##_GERRORN, \ > > + CMDQV_VCMDQ_PAGE0_BASE + 0x14 + i * CMDQV_VCMDQ_STRIDE) > \ > > + FIELD(VCMDQ##i##_GERRORN, CMDQ_ERR, 0, 1) \ > > + FIELD(VCMDQ##i##_GERRORN, CONS_DRAM_WR_ABT_ERR, 1, 1) \ > > + FIELD(VCMDQ##i##_GERRORN, CMDQ_INIT_ERR, 2, 1) > > + > > +SMMU_CMDQV_VCMDQi_GERRORN_(0) > > +SMMU_CMDQV_VCMDQi_GERRORN_(1) > > > nit, what I meant in my v4 comments is I would prefer we group the > eventual register definition to represent the actual layout of each > page, instead of interleaving them with actual macros: > This would look like: > > /* page 0 */ > > /* vcmdq0 */ > > +SMMU_CMDQV_VCMDQi_CONS_INDX_(0) > +SMMU_CMDQV_VCMDQi_PROD_INDX_(0) > +SMMU_CMDQV_VCMDQi_CONFIG_(0) > +SMMU_CMDQV_VCMDQi_STATUS_(0) > +SMMU_CMDQV_VCMDQi_GERROR_(0) > +SMMU_CMDQV_VCMDQi_GERRORN_(0) > > /* vcmdq1 */ > > +SMMU_CMDQV_VCMDQi_CONS_INDX_(1) > +SMMU_CMDQV_VCMDQi_PROD_INDX_(1) > +SMMU_CMDQV_VCMDQi_CONFIG_(1) > +SMMU_CMDQV_VCMDQi_STATUS_(1) > +SMMU_CMDQV_VCMDQi_GERROR_(1) > +SMMU_CMDQV_VCMDQi_GERRORN_(1) > > same for page 1 > > To me it looks easier to remember the layout without looking at the spec. > > > Taste & colors, what do you prefer? Looks like I misunderstood your v4 comment. Sure, I will switch to this layout. Thanks, Shameer
