On Mon, Jun 01, 2026 at 12:42:09PM +0100, Shameer Kolothum wrote:
> +/*
> + * Return a pointer into the mmap'd VINTF page0 for the VCMDQ Page 0
> + * register at @offset0 in VCMDQ slot @index, or NULL when the VCMDQ
> + * has no hw_queue allocated or the VINTF page0 is not mmap'd.
> + */
> +static inline uint32_t *tegra241_cmdqv_vintf_ptr(Tegra241CMDQV *cmdqv,
> + int index, hwaddr offset0)
I'd do:
tegra241_cmdqv_vintf_lvcmdq_ptr()
or just:
tegra241_vintf_lvcmdq_ptr()
> static void tegra241_cmdqv_write_vcmdq_page0(Tegra241CMDQV *cmdqv,
> hwaddr offset0, int index,
> uint32_t value, bool direct)
> {
> + uint32_t *ptr = tegra241_cmdqv_vintf_ptr(cmdqv, index, offset0);
> +
> + if (ptr) {
> + switch (offset0) {
> + case A_VCMDQ0_CONS_INDX:
> + case A_VCMDQ0_PROD_INDX:
> + case A_VCMDQ0_CONFIG:
> + case A_VCMDQ0_GERRORN:
> + *ptr = value;
> + goto out;
> + default:
> + break;
> + }
> + }
Could stuff this into the helper? Return a valid ptr only when the
offset0 is one of these four registers; otherwise, return NULL.
Then, here if would be:
if (ptr) {
*ptr = value;
goto out;
}
Doing so inside the helper could also sanitize the other callers.
Otherwise, looks good to me.
Reviewed-by: Nicolin Chen <[email protected]>
