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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Initialization failure on ACPI/DT platforms due to missing core clock fallback in dw_i3c_master_set_speed().
--

--- Patch [1]: [PATCH v4] i3c: master: dw-i3c-master: fix OD timing for first broadcast ---
commit c4596518222ed00cb6c15e2963a846694b7f5d55
Author: Tze Yee Ng <tze.yee.ng@altera.com>

i3c: master: dw-i3c-master: fix OD timing for first broadcast

This commit implements the ->set_speed() callback to allow the I3C core to
switch open-drain timings for the first broadcast address. It caches the
normal OD register value during bus initialization and computes a separate
od_hcnt for the slow path.

> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 3816a50a52cc5..11ae517cf2e60 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c

[ ... ]

> @@ -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 = clk_get_rate(master->core_clk);
> +		if (!core_rate)
> +			return -EINVAL;

[Severity: High]
Will this cause platforms without a core clock provider to fail here?

The rest of the driver uses dw_i3c_master_get_core_rate() to fall back to
the "clock-frequency" device property if master->core_clk is NULL (such as
on ACPI/DT platforms). If clk_get_rate() is called directly here on those
platforms, it appears core_rate will be 0 and the callback will abort with
-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;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/c2d80c4fabd36aa00dc1fc507ff20211bba10c3f.1785471627.git.tze.yee.ng@altera.com?part=1

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c