On 5/8/26 1:19 PM, Shameer Kolothum Thodi wrote:
>
>> -----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.
OK thanks ;-)
Eric
>
>>> + 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
