On 2/3/21 8:41 AM, Dan Carpenter wrote:
> Hello Samuel Holland,
>
> The patch e5c02cf54154: "i2c: mv64xxx: Add runtime PM support" from
> Jan 3, 2021, leads to the following static checker warning:
>
> drivers/i2c/busses/i2c-mv64xxx.c:816 mv64xxx_of_config()
> warn: 'drv_data->clk' isn't an ERR_PTR
>
> drivers/i2c/busses/i2c-mv64xxx.c
> 803 static int
> 804 mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
> 805 struct device *dev)
> 806 {
> 807 const struct of_device_id *device;
> 808 struct device_node *np = dev->of_node;
> 809 u32 bus_freq, tclk;
> 810 int rc = 0;
> 811
> 812 /* CLK is mandatory when using DT to describe the i2c bus. We
> 813 * need to know tclk in order to calculate bus clock
> 814 * factors.
> 815 */
> 816 if (IS_ERR(drv_data->clk)) {
>
> This check used to be correct, but now if it's an error pointer in probe
> then we set it to NULL.
>
> 817 rc = -ENODEV;
> 818 goto out;
> 819 }
> 820 tclk = clk_get_rate(drv_data->clk);
>
> The result is that "tclk" is zero. So probably the correct fix is to
> change the IS_ERR() check to a NULL check? zero rate seems useless.
Yes, that looks like the correct fix. I sent a patch.
Thank you!
Samuel
> 821
> 822 if (of_property_read_u32(np, "clock-frequency", &bus_freq))
> 823 bus_freq = I2C_MAX_STANDARD_MODE_FREQ; /* 100kHz by default */
> 824
> 825 if (of_device_is_compatible(np, "allwinner,sun4i-a10-i2c") ||
> 826 of_device_is_compatible(np, "allwinner,sun6i-a31-i2c"))
> 827 drv_data->clk_n_base_0 = true;
> 828
> 829 if (!mv64xxx_find_baud_factors(drv_data, bus_freq, tclk)) {
> 830 rc = -EINVAL;
> 831 goto out;
> 832 }
> 833
>
> regards,
> dan carpenter
>
