On Mon Aug 17, 2026 at 10:25 PM +08, Junhui Liu wrote:
> Hi Troy,
> Thanks for the review.
>
> On Mon Aug 17, 2026 at 5:12 PM CST, Troy Mitchell wrote:
>>> The controller bus mode should be selected according to the requested
>>> I2C bus speed. However, the driver currently passes the functional clock
>>> rate to k1_i2c_set_bus_speed(), so the selected mode does not reflect
>>> the requested bus speed.
>>>
>>> Fix this by reading the clock-frequency property from the Device Tree,
>>> defaulting to standard speed, and drop the unused clk_rate field.
>>
>> `clock-frequency` is the requested SCL rate, but this patch only changes the
>> value passed to `k1_i2c_set_bus_speed()`.
>>
>>> @@ -496,10 +496,13 @@ static int k1_i2c_probe(struct udevice *bus)
>>> debug("%s: failed to enable clock\n", __func__);
>>> return ret;
>>> }
>>> - priv->clk_rate = clk_get_rate(&priv->clk);
>>>
>>> priv->base = (void *)devfdt_get_addr_ptr(bus);
>>> - k1_i2c_set_bus_speed(bus, priv->clk_rate);
>>> +
>>> + speed = dev_read_u32_default(bus, "clock-frequency",
>>> + I2C_SPEED_STANDARD_RATE);
>>> + k1_i2c_set_bus_speed(bus, speed);
>>
>> `k1_i2c_set_bus_speed()` only changes `ICR_MODE_MASK`. It does not program
>> the ILCR divider or initialize IWCR, so the actual SCL rate still depends on
>> the register reset values and may not match the Device Tree.
>
> Yes. This patch only aims to fix the ICR_MODE selection. On the K1 board
> I tested, the register reset defaults are enough for both the PMIC and
> the EEPROM to work under U-Boot.
>
>>
>> Please calculate and program ILCR from the functional clock rate and the
>> requested SCL rate, initialize IWCR as required by the hardware, and reject
>> unsupported rates. The functional clock handle or rate therefore needs to
>> remain available to `k1_i2c_set_bus_speed()`.
>
> That's a fair point. The current driver is aligned with the mainline
> Linux K1 I2C driver. It does not include your later work to program
> ILCR/IWCR:
> https://lore.kernel.org/linux-riscv/[email protected]/
>
> Porting that change to U-Boot is not a small amount of work. I'd prefer
> to keep that as a follow-up series rather than mix it into this ICR_MODE
> fix.
Then:Reviewed-by: Troy Mitchell <[email protected]> -- Troy Mitchell
signature.asc
Description: PGP signature
