Re: [PATCH v6 2/9] reset: eyeq: Add EyeQ7H compatibles

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `valid_mask` definitions for most EyeQ7H sparse domains are incorrectly calculated as contiguous bitmasks, rejecting valid device resets.
- [High] Severe contradiction between the documented domain type and the implemented domain type for the EyeQ7H DDR reset domain.
--

commit 0720cebbbbe1a20186ece402d8172a4c6a7bfdca
Author: Benoît Monin <benoit.monin@bootlin.com>

reset: eyeq: Add EyeQ7H compatibles

This commit adds support for the reset controllers found in the EyeQ7H OLB,
introducing three new reset domain types (ACRP, CFG, ACC) to handle the
different register layouts and behaviors.

diff --git a/drivers/reset/reset-eyeq.c b/drivers/reset/reset-eyeq.c
index 1a38579838970..ce758a1757fce 100644
--- a/drivers/reset/reset-eyeq.c
+++ b/drivers/reset/reset-eyeq.c
[ ... ]
> + * Known resets in EyeQ7H ddr (type EQR_EYEQ7H_CFG)
> + *  0. APB	2. DMI		3. DFI		4. PHY_SMS
> + *  5. CTL_SMS
> + *
> + * Known resets in EyeQ7H east (type EQR_EYEQ7H_CFG)
> + *  0. ISP	2. VEU		3. LBIST
> + *
> + * Known resets in EyeQ7H periph (type EQR_EYEQ6H_SARCR)
> + *  0. gpio	1.EXT TIMER	3.UART		4. SPI
> + *  5. I2C0	6. I2C1		7.I2C2		8. I2S
> + *
> + * Known resets in EyeQ7H south (type EQR_EYEQ7H_CFG)
> + *  0. PCI_PHY	1. PCI_CTL	2. S_NOC	3. GBE_PHY
> + *  4. GBE_CTL	5. XSPI		6. UFS		7. VDIO
> + *
> + * Known resets in EyeQ7H west (type EQR_EYEQ7H_CFG)
> + *  0. GPU	2. CAU		3. LBIST	4. GPU_LBIST
[ ... ]
> +static const struct eqr_domain_descriptor eqr_eyeq7h_ddr_domains[] = {
> +	{
> +		.type = EQR_EYEQ7H_ACRP,

The block comment above documents the ddr domain as type EQR_EYEQ7H_CFG, but
it is initialized here as EQR_EYEQ7H_ACRP.

Since these two types use incompatible register access patterns (CFG modifies
two bits per device in a shared register, while ACRP accesses separate
registers), could this cause out-of-bounds MMIO accesses or fail to reset
the peripherals if the hardware expects the CFG layout?

> +		.valid_mask = 0x1F,

For the ddr domain, the documented indices are 0, 2, 3, 4, and 5. Should
this valid_mask be 0x3D instead of 0x1F?

A mask of 0x1F only covers bits 0-4, which would cause requests for offset 5
(CTL_SMS) to be rejected by the validation in eqr_of_xlate_internal(). This
pattern of using contiguous bitmasks for sparse domains seems to appear in
several places.

[ ... ]
> +static const struct eqr_domain_descriptor eqr_eyeq7h_west_domains[] = {
> +	{
> +		.type = EQR_EYEQ7H_CFG,
> +		.valid_mask = 0xf,

Similarly, the west domain lists indices 0, 2, 3, and 4. A valid_mask of 0xF
(bits 0-3) will reject reset requests for offset 4 (GPU_LBIST) because BIT(4)
falls outside the mask.

Could this valid_mask be updated to 0x1D to include BIT(4)?

This also appears to affect the east, periph, xnn0, and xnn1 domains, which
all have sparse indices but contiguous valid_masks that will reject resets for
the higher offsets.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512-clk-eyeq7-v6-0-0540cce18fb2@bootlin.com?part=2