Re: [PATCH v2 4/7] dt-bindings: reset: Add reset controller constants for mt8167

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect reset ID mapping for MMSYS SW1 reset constants.
--

--- Patch [4]: [PATCH v2 4/7] dt-bindings: reset: Add reset controller constants for mt8167 ---
commit 3e501b19a4f1986647da6f341a25af00a37a8d1d
Author: Luca Leonardo Scorcia <[email protected]>

dt-bindings: reset: Add reset controller constants for mt8167

This commit adds identifying bits for watchdog and mmsys resets on
the mt8167 platform.

> diff --git a/include/dt-bindings/reset/mediatek,mt8167-resets.h b/include/dt-bindings/reset/mediatek,mt8167-resets.h
> new file mode 100644
> index 0000000000000..85d2d0e99c689
> --- /dev/null
> +++ b/include/dt-bindings/reset/mediatek,mt8167-resets.h

[ ... ]

> +#define MT8167_MMSYS_SW0_RST_B_DPI0		23
> +#define MT8167_MMSYS_SW0_RST_B_MIPI_TX_CONFIG	24
> +#define MT8167_MMSYS_SW1_RST_B_LVDS_ENCODER	25
> +#define MT8167_MMSYS_SW1_RST_B_DPI1		26
> +#define MT8167_MMSYS_SW1_RST_B_HDMI		27

[Severity: High]
Will these reset IDs correctly map to the SW1 register?

In drivers/soc/mediatek/mtk-mmsys.c, mtk_mmsys_reset_update() dynamically
calculates the register offset by dividing the ID by 32.

Since MT8167 does not provide an rst_tb mapping array, the raw ID is used for
this division. For IDs 25, 26, and 27, this calculates to an offset of 0,
which targets the SW0_RST_B register instead of SW1_RST_B.

Should these IDs be greater than or equal to 32 if they belong to SW1, or is
the SW1 naming a typo for bits that actually reside in the SW0 register?

> +
> +#endif  /* _DT_BINDINGS_RESET_CONTROLLER_MT8167 */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4