Re: [PATCH v31 2/5] i2c: aspeed: Read clock-frequency via i2c_parse_fw_timings()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Kernel panic via division by zero when bus frequency is configured to 0 in device tree.
--

commit 7def856c61d40b684f656602f5cf38ffd5745272
Author: Ryan Chen <ryan_chen@aspeedtech.com>

i2c: aspeed: Read clock-frequency via i2c_parse_fw_timings()

This commit transitions the driver to use i2c_parse_fw_timings() for parsing
the standard clock-frequency property. It maintains backward compatibility by
falling back to the legacy bus-frequency property if the standard one is
missing.

> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> --- a/drivers/i2c/busses/i2c-aspeed.c
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -1025,12 +1026,18 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
>  	}
>  	reset_control_deassert(bus->rst);
>  
> -	ret = of_property_read_u32(pdev->dev.of_node,
> -				   "bus-frequency", &bus->bus_frequency);
> -	if (ret < 0) {
> -		dev_err(&pdev->dev,
> -			"Could not read bus-frequency property\n");
> -		bus->bus_frequency = I2C_MAX_STANDARD_MODE_FREQ;
> +	i2c_parse_fw_timings(&pdev->dev, &timings, false);
> +	if (timings.bus_freq_hz) {
> +		bus->bus_frequency = timings.bus_freq_hz;
> +	} else {
> +		ret = of_property_read_u32(pdev->dev.of_node,
> +					   "bus-frequency",
> +					   &bus->bus_frequency);

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for a malformed
device tree to cause a kernel panic here?

If the device tree specifies bus-frequency = <0> and clock-frequency is
absent or also 0, of_property_read_u32() successfully reads 0 into
bus->bus_frequency.

Since the frequency is not validated to be strictly positive, wouldn't this
later trigger a division by zero exception in aspeed_i2c_init_clk() when
calculating the divisor?

drivers/i2c/busses/i2c-aspeed.c:aspeed_i2c_init_clk() {
    ...
    divisor = DIV_ROUND_UP(bus->parent_clk_frequency, bus->bus_frequency);
    ...
}

Should we add a check to handle a 0 hz configuration gracefully rather than
crashing during device probe?

> +		if (ret < 0) {
> +			dev_err(&pdev->dev,
> +				"Could not read clock-frequency or bus-frequency property\n");
> +			bus->bus_frequency = I2C_MAX_STANDARD_MODE_FREQ;
> +		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603-upstream_i2c-v31-0-ba7a02714f22@aspeedtech.com?part=2