> -----Original Message-----
> From: Eric Auger <[email protected]>
> Sent: 07 May 2026 17:52
> 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];
> [email protected]; Krishnakant Jaju <[email protected]>;
> [email protected]
> Subject: Re: [PATCH v4 25/31] hw/arm/tegra241-cmdqv: Add reset handler
>
> External email: Use caution opening links or attachments
>
>
> On 4/15/26 12:55 PM, Shameer Kolothum wrote:
> > From: Nicolin Chen <[email protected]>
> >
> > Introduce a reset handler for the Tegra241 CMDQV and initialize its
> > register state.
> >
> > Signed-off-by: Nicolin Chen <[email protected]>
> > Signed-off-by: Shameer Kolothum <[email protected]>
> > ---
> > hw/arm/tegra241-cmdqv.h | 2 ++
> > hw/arm/tegra241-cmdqv.c | 50
> +++++++++++++++++++++++++++++++++++++++++
> > hw/arm/trace-events | 1 +
> > 3 files changed, 53 insertions(+)
> >
> > diff --git a/hw/arm/tegra241-cmdqv.h b/hw/arm/tegra241-cmdqv.h
> > index 2befa6205e..b2a444daef 100644
> > --- a/hw/arm/tegra241-cmdqv.h
> > +++ b/hw/arm/tegra241-cmdqv.h
> > @@ -79,6 +79,8 @@ FIELD(CONFIG, CMDQ_MAX_CLK_BATCH, 4, 8)
> > FIELD(CONFIG, CMDQ_MAX_CMD_BATCH, 12, 8)
> > FIELD(CONFIG, CONS_DRAM_EN, 20, 1)
> >
> > +#define V_CONFIG_RESET 0x00020403
> > +
> > REG32(PARAM, 0x4)
> > FIELD(PARAM, CMDQV_VER, 0, 4)
> > FIELD(PARAM, CMDQV_NUM_CMDQ_LOG2, 4, 4)
> > diff --git a/hw/arm/tegra241-cmdqv.c b/hw/arm/tegra241-cmdqv.c
> > index 9c2fc02b92..af68add2f0 100644
> > --- a/hw/arm/tegra241-cmdqv.c
> > +++ b/hw/arm/tegra241-cmdqv.c
> > @@ -8,6 +8,7 @@
> > */
> >
> > #include "qemu/osdep.h"
> > +#include "qemu/error-report.h"
> > #include "qemu/log.h"
> >
> > #include "hw/arm/smmuv3.h"
> > @@ -645,8 +646,57 @@ free_viommu:
> > return false;
> > }
> >
> > +static void tegra241_cmdqv_init_regs(SMMUv3State *s, Tegra241CMDQV
> *cmdqv)
> > +{
> > + int i;
> > +
> > + cmdqv->config = V_CONFIG_RESET;
> > + cmdqv->param = FIELD_DP32(0, PARAM, CMDQV_VER, CMDQV_VER);
> hw/core/registerfields.h:#define FIELD_DP32(storage, reg, field, val)
> what does this 0?
I think it is to make sure we have no garbage set for non-field bits.
Starting from 0 guarantees non-field bits are clean.
> > + cmdqv->param = FIELD_DP32(cmdqv->param, PARAM,
> CMDQV_NUM_CMDQ_LOG2,
> > + CMDQV_NUM_CMDQ_LOG2);
> I was a bit puzzled by the FIELD name being the value name but well.
> > + cmdqv->param = FIELD_DP32(cmdqv->param, PARAM,
> CMDQV_NUM_SID_PER_VI_LOG2,
> > + CMDQV_NUM_SID_PER_VI_LOG2);
> > + trace_tegra241_cmdqv_init_regs(cmdqv->param);
> > + cmdqv->status = R_STATUS_CMDQV_ENABLED_MASK;
> > + for (i = 0; i < 2; i++) {
> > + cmdqv->vi_err_map[i] = 0;
> > + cmdqv->vi_int_mask[i] = 0;
> > + cmdqv->cmdq_err_map[i] = 0;
> > + }
> > + cmdqv->cmdq_err_map[2] = 0;
> > + cmdqv->cmdq_err_map[3] = 0;
> the split looks pretty strange, ie. loop of 2 + continue with individual
> setting. why don't you do the init of
>
> cmdqv->cmdq_err_map in the x4 loop below
😊. True, that does look strange. I will change.
Thanks,
Shameer
