Re: [PATCH v5] i3c: master: dw-i3c-master: fix OD timing for first broadcast

On Fri, Jul 31, 2026 at 01:01:39AM -0700, [email protected] wrote:
> From: Tze Yee Ng <[email protected]>
>
> Implement ->set_speed() so the I3C core can switch open-drain timing for
> the first broadcast address per spec: I3C_OPEN_DRAIN_SLOW_SPEED programs
> tHIGH_INIT (200 ns) before RSTDAA, and I3C_OPEN_DRAIN_NORMAL_SPEED restores
> normal OD timing afterward. Cache the normal OD register value during bus
> init and use a separate od_hcnt for the slow path so SDR extended timing
> remains derived from the normal PP hcnt.
>
> For AMD_I3C_OD_PP_TIMING, cache AMD_I3C_OD_TIMING as the normal OD
> baseline and stop rewriting OD timing in send_ccc_cmd()/runtime resume so
> I3C_OPEN_DRAIN_SLOW_SPEED is preserved through RSTDAA.
>
> Use PM_RUNTIME_ACQUIRE_AUTOSUSPEND() in set_speed(). Compute od_hcnt with
> DIV_ROUND_UP_ULL() for 32-bit safety and clamp it to U8_MAX to match the
> 8-bit I3C_OD_HCNT field.
>
> Fixes I2C devices with spike filters not being detected on mixed buses.
>
> Signed-off-by: Tze Yee Ng <[email protected]>
> ---

Reviewed-by: Frank Li <[email protected]>

> Changes in v5:
> - Switch set_speed() to use dw_i3c_master_get_core_rate() (Sashiko)
>
> Changes in v4:
> - Rebase on i3c/next.
> - No functional changes.
>
> Changes in v3:
> - Use DIV_ROUND_UP_ULL(..., NSEC_PER_SEC) for od_hcnt to avoid
>   __udivdi3 linker failures on 32-bit platforms (Sashiko/Frank)
> - Compute od_hcnt as u32 and clamp to U8_MAX to match the 8-bit
>   I3C_OD_HCNT field and avoid silent truncation (Sashiko/Frank)
>
> Changes in v2:
> - Recalculate od_hcnt with Zephyr-style formula (Sashiko)
> - Fix AMD_I3C_OD_PP_TIMING interaction with set_speed() (Sashiko)
> - Switch set_speed() to PM_RUNTIME_ACQUIRE_AUTOSUSPEND() (Frank)
> ---
>  drivers/i3c/master/dw-i3c-master.c | 82 +++++++++++++++++++++++-------
>  drivers/i3c/master/dw-i3c-master.h |  1 +
>  include/linux/i3c/master.h         |  1 +
>  3 files changed, 67 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 3816a50a52cc..405089f181c4 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -572,6 +572,13 @@ static unsigned long dw_i3c_master_get_core_rate(struct dw_i3c_master *master)
>  	return core_rate_prop;
>  }
>
> +static void amd_configure_od_pp_quirk(struct dw_i3c_master *master)
> +{
> +	master->i3c_od_timing = AMD_I3C_OD_TIMING;
> +	master->i3c_od_timing_normal = AMD_I3C_OD_TIMING;
> +	master->i3c_pp_timing = AMD_I3C_PP_TIMING;
> +}
> +
>  static int dw_i3c_clk_cfg(struct dw_i3c_master *master)
>  {
>  	unsigned long core_rate, core_period;
> @@ -610,6 +617,18 @@ static int dw_i3c_clk_cfg(struct dw_i3c_master *master)
>  	scl_timing = SCL_I3C_TIMING_HCNT(hcnt) | SCL_I3C_TIMING_LCNT(lcnt);
>  	writel(scl_timing, master->regs + SCL_I3C_OD_TIMING);
>  	master->i3c_od_timing = scl_timing;
> +	master->i3c_od_timing_normal = scl_timing;
> +
> +	/*
> +	 * AMD legacy platforms need fixed OD/PP timings. Cache them as the
> +	 * normal OD baseline so set_speed(NORMAL) restores AMD values, and
> +	 * set_speed(SLOW) can stretch HCNT while keeping the AMD LCNT.
> +	 */
> +	if (master->quirks & AMD_I3C_OD_PP_TIMING) {
> +		amd_configure_od_pp_quirk(master);
> +		writel(master->i3c_pp_timing, master->regs + SCL_I3C_PP_TIMING);
> +		writel(master->i3c_od_timing, master->regs + SCL_I3C_OD_TIMING);
> +	}
>
>  	lcnt = DIV_ROUND_UP(core_rate, I3C_BUS_SDR1_SCL_RATE) - hcnt;
>  	scl_timing = SCL_EXT_LCNT_1(lcnt);
> @@ -825,12 +844,6 @@ static int dw_i3c_ccc_get(struct dw_i3c_master *master, struct i3c_ccc_cmd *ccc)
>  	return ret;
>  }
>
> -static void amd_configure_od_pp_quirk(struct dw_i3c_master *master)
> -{
> -	master->i3c_od_timing = AMD_I3C_OD_TIMING;
> -	master->i3c_pp_timing = AMD_I3C_PP_TIMING;
> -}
> -
>  static int dw_i3c_master_send_ccc_cmd(struct i3c_master_controller *m,
>  				      struct i3c_ccc_cmd *ccc)
>  {
> @@ -840,13 +853,6 @@ static int dw_i3c_master_send_ccc_cmd(struct i3c_master_controller *m,
>  	if (ccc->id == I3C_CCC_ENTDAA)
>  		return -EINVAL;
>
> -	/* AMD platform specific OD and PP timings */
> -	if (master->quirks & AMD_I3C_OD_PP_TIMING) {
> -		amd_configure_od_pp_quirk(master);
> -		writel(master->i3c_pp_timing, master->regs + SCL_I3C_PP_TIMING);
> -		writel(master->i3c_od_timing, master->regs + SCL_I3C_OD_TIMING);
> -	}
> -
>  	ret = pm_runtime_resume_and_get(master->dev);
>  	if (ret < 0) {
>  		dev_err(master->dev,
> @@ -1531,6 +1537,50 @@ static irqreturn_t dw_i3c_master_irq_handler(int irq, void *dev_id)
>  	return IRQ_HANDLED;
>  }
>
> +static int dw_i3c_master_set_speed(struct i3c_master_controller *m,
> +				   enum i3c_open_drain_speed speed)
> +{
> +	struct dw_i3c_master *master = to_dw_i3c_master(m);
> +	unsigned long core_rate;
> +	u32 scl_timing, od_hcnt;
> +	u8 lcnt;
> +
> +	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(master->dev, pm);
> +	if (PM_RUNTIME_ACQUIRE_ERR(&pm))
> +		return -ENXIO;
> +
> +	switch (speed) {
> +	case I3C_OPEN_DRAIN_SLOW_SPEED:
> +		core_rate = dw_i3c_master_get_core_rate(master);
> +		if (!core_rate)
> +			return -EINVAL;
> +
> +		lcnt = SCL_I3C_TIMING_LCNT(master->i3c_od_timing_normal);
> +		od_hcnt = DIV_ROUND_UP_ULL((u64)I3C_BUS_THIGH_INIT_OD_MIN_NS *
> +					  core_rate, NSEC_PER_SEC) - 1;
> +		if (od_hcnt < SCL_I3C_TIMING_CNT_MIN)
> +			od_hcnt = SCL_I3C_TIMING_CNT_MIN;
> +		else if (od_hcnt > U8_MAX)
> +			od_hcnt = U8_MAX;
> +		scl_timing = SCL_I3C_TIMING_HCNT(od_hcnt) |
> +			     SCL_I3C_TIMING_LCNT(lcnt);
> +		writel(scl_timing, master->regs + SCL_I3C_OD_TIMING);
> +		master->i3c_od_timing = scl_timing;
> +		break;
> +
> +	case I3C_OPEN_DRAIN_NORMAL_SPEED:
> +		writel(master->i3c_od_timing_normal,
> +		       master->regs + SCL_I3C_OD_TIMING);
> +		master->i3c_od_timing = master->i3c_od_timing_normal;
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static int dw_i3c_master_set_dev_nack_retry(struct i3c_master_controller *m,
>  					    unsigned int dev_nack_retry_cnt)
>  {
> @@ -1585,6 +1635,7 @@ static const struct i3c_master_controller_ops dw_mipi_i3c_ops = {
>  	.recycle_ibi_slot = dw_i3c_master_recycle_ibi_slot,
>  	.enable_hotjoin = dw_i3c_master_enable_hotjoin,
>  	.disable_hotjoin = dw_i3c_master_disable_hotjoin,
> +	.set_speed = dw_i3c_master_set_speed,
>  	.set_dev_nack_retry = dw_i3c_master_set_dev_nack_retry,
>  };
>
> @@ -1780,10 +1831,7 @@ static void dw_i3c_master_restore_addrs(struct dw_i3c_master *master)
>
>  static void dw_i3c_master_restore_timing_regs(struct dw_i3c_master *master)
>  {
> -	/* AMD platform specific OD and PP timings */
> -	if (master->quirks & AMD_I3C_OD_PP_TIMING)
> -		amd_configure_od_pp_quirk(master);
> -
> +	/* Preserve cached OD timing; it may be the SLOW setting from set_speed(). */
>  	writel(master->i3c_pp_timing, master->regs + SCL_I3C_PP_TIMING);
>  	writel(master->bus_free_timing, master->regs + BUS_FREE_TIMING);
>  	writel(master->i3c_od_timing, master->regs + SCL_I3C_OD_TIMING);
> diff --git a/drivers/i3c/master/dw-i3c-master.h b/drivers/i3c/master/dw-i3c-master.h
> index 28e9348f2153..17ad817d1f8e 100644
> --- a/drivers/i3c/master/dw-i3c-master.h
> +++ b/drivers/i3c/master/dw-i3c-master.h
> @@ -46,6 +46,7 @@ struct dw_i3c_master {
>  	u32 dev_addr;
>  	u32 i3c_pp_timing;
>  	u32 i3c_od_timing;
> +	u32 i3c_od_timing_normal;
>  	u32 ext_lcnt_timing;
>  	u32 bus_free_timing;
>  	u32 i2c_fm_timing;
> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
> index 2dc139a217bf..c3df1c187050 100644
> --- a/include/linux/i3c/master.h
> +++ b/include/linux/i3c/master.h
> @@ -269,6 +269,7 @@ struct i3c_device {
>  #define I3C_BUS_THIGH_MIXED_MAX_NS	41
>  #define I3C_BUS_TIDLE_MIN_NS		200000
>  #define I3C_BUS_TLOW_OD_MIN_NS		200
> +#define I3C_BUS_THIGH_INIT_OD_MIN_NS	200
>
>  /**
>   * enum i3c_bus_mode - I3C bus mode
> --
> 2.43.7
>

-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c