Re: [PATCH v3] i2c: designware: Enable SAR in slave mode for Multiple SAR controllers

On Wed, Aug 12, 2026 at 05:15:14PM +0530, Umesh Kumar wrote:
> DesignWare controllers synthesised with the Multiple SAR feature (e.g.
> Marvell CN20K) gate the primary slave address behind IC_ENABLE.SAR_EN
> (bit 19). Per the Synopsys DW_apb_i2c DataBook (Multiple SAR feature),

Also provide a version of the DataBook. Something like v2.20c (hypothetical
example).

> when this bit is set the core acknowledges the address matching IC_SAR;
> otherwise the address is NACKed. The slave enable path writes only
> IC_ENABLE.ENABLE (bit 0), so on such controllers the target never ACKs
> its address and a remote master cannot reach it.
> 
> Set SAR_EN together with ENABLE when entering slave mode. The bit is
> reserved (and ignored) on single-SAR controllers, so existing users are
> unaffected.
> 
> Signed-off-by: Umesh Kumar <[email protected]>
> Acked-by: Mika Westerberg <[email protected]>
> ---

This is a v3 and no changelog here... What's going on?

Assuming this will be provided
Reviewed-by: Andy Shevchenko <[email protected]>

...

> +		/*
> +		 * Per the Synopsys DW_apb_i2c DataBook (Multiple SAR feature),

Ditto (mind the version).

> +		 * controllers synthesised with multiple SARs only acknowledge
> +		 * traffic to IC_SAR when IC_ENABLE.SAR_EN is set; a plain enable
> +		 * is not enough. The bit is reserved (ignored) on single-SAR
> +		 * controllers.
> +		 */

...

> +/*
> + * IC_ENABLE.SAR_EN, per the Synopsys DW_apb_i2c DataBook (Multiple SAR
> + * feature): on controllers synthesised with multiple SARs (e.g. Marvell
> + * CN20K) the primary slave address (IC_SAR) is only acknowledged when this
> + * bit is set. Must be programmed while IC_ENABLE.ENABLE is 0. Reserved on
> + * single-SAR controllers.
> + */

Not sure if we need a dup comment as it's also in the code. Perhaps leave only
in the C code where it makes more sense?

> +#define DW_IC_ENABLE_SAR_EN			BIT(19)

-- 
With Best Regards,
Andy Shevchenko