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

[NG, TZE YEE]

On 22/7/2026 12:58 am, Frank Li wrote:
> On Tue, Jul 21, 2026 at 03:47:50AM +0000, sashiko-bot at kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>> - [Medium] The calculation for `od_hcnt` can silently overflow its 8-bit capacity if the core clock rate exceeds ~1.28 GHz, resulting in an inappropriately fast open-drain speed.
>> - [Low] Using DIV_ROUND_UP() with a 64-bit numerator or divisor causes a linker error (__udivdi3 undefined) on 32-bit platforms.
>> --
>>
>> --- Patch [1]: [PATCH v2] i3c: master: dw-i3c-master: fix OD timing for first broadcast ---
>> commit b82f29a59ae98bd18cbb544ded9a3bf8f8d60fc6
>> Author: Tze Yee Ng <tze.yee.ng at altera.com>
>>
>> i3c: master: dw-i3c-master: fix OD timing for first broadcast
>>
>> This commit implements ->set_speed() to switch open-drain timing for the
>> first broadcast address according to the I3C specification. It caches the
>> normal OD register value during init and uses a separate od_hcnt for the
>> slow path so SDR extended timing remains derived from the normal PP hcnt.
>>
>>> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
>>> index 84ffcb1893389..077f2bf4b0b3f 100644
>>> --- a/drivers/i3c/master/dw-i3c-master.c
>>> +++ b/drivers/i3c/master/dw-i3c-master.c
>>
>> [ ... ]
>>
>>> @@ -1513,6 +1519,48 @@ 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;
>>> +   u8 od_hcnt, 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;
>>> +
>>> +           lcnt = SCL_I3C_TIMING_LCNT(master->i3c_od_timing_normal);
>>> +           od_hcnt = DIV_ROUND_UP((u64)I3C_BUS_THIGH_INIT_OD_MIN_NS * core_rate,
>>> +                                  1000000000ULL) - 1;
>>
>> [Severity: Low]
>> Will this cause a build failure on 32-bit platforms?
>>
>> Using DIV_ROUND_UP() with a 64-bit divisor (1000000000ULL) will likely
>> result in a linker error for __udivdi3 on 32-bit architectures like ARM32
>> or i386. Should this use DIV_ROUND_UP_ULL() instead?
>>
>
> make sense,
>
>> [Severity: Medium]
>> Can od_hcnt silently overflow here?
>>
>> Because od_hcnt is declared as a u8 at the top of the function, if the
>> core clock rate exceeds ~1.28 GHz, the calculation will evaluate to a
>> value greater than 255.
>
> less possible, but suggest fix it.
>
> Frank
>

Hi Frank,

Thanks for the review.

Agreed on both points. Per the DWC MIPI I3C databook
(SCL_I3C_OD_TIMING), I3C_OD_HCNT is an 8-bit field (bits 23:16), so
values above 255 cannot be programmed in hardware. In v3 I will:

1. Switch to DIV_ROUND_UP_ULL(..., NSEC_PER_SEC) for 32-bit safety

2. Compute od_hcnt in a wider type and clamp it to U8_MAX so it cannot
    silently truncate

At core clocks above ~1.28 GHz the controller cannot meet tHIGH_INIT
(200 ns) anyway; clamping to 0xff gives the longest programmable high time.

Thanks,
Tze Yee

>>
>> This would truncate when assigned to od_hcnt, bypassing the minimum 200ns
>> requirement and potentially causing spike filters on devices to drop the
>> broadcast.
>>
>>> +           if (od_hcnt < SCL_I3C_TIMING_CNT_MIN)
>>> +                   od_hcnt = SCL_I3C_TIMING_CNT_MIN;
>>> +           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;
>>
>> --
>> Sashiko AI review · https://sashiko.dev/#/patchset/20260721033159.11789-1-tze.yee.ng@altera.com?part=1
>>
>> --
>> linux-i3c mailing list
>> linux-i3c at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-i3c