On Fri, Aug 23, 2024 at 11:51:16AM +0800, Rong Qianfeng wrote: > No more special handling needed here, so use dev_err_probe() > to simplify the code. Ah, okay. But see below. ... > ret = of_property_read_u32(pdev->dev.of_node, "clock-frequency", > &clk_freq); > - if (ret) { > - dev_err(&pdev->dev, "clock-frequency not specified in DT\n"); > - return ret; > - } > + if (ret) > + return dev_err_probe(&pdev->dev, ret, > + "clock-frequency not specified in DT\n"); Besides converting to the i2c_timings and respective APIs... > i2c->speed = clk_freq / 1000; > - if (i2c->speed == 0) { > - ret = -EINVAL; > - dev_err(&pdev->dev, "clock-frequency minimum is 1000\n"); > - return ret; > - } > + if (i2c->speed == 0) > + return dev_err_probe(&pdev->dev, -EINVAL, > + "clock-frequency minimum is 1000\n"); ...this makes sense to do with struct device *dev = &pdev->dev; ... return dev_err_probe(dev, ...); And continue with a patch to replace all those &pdev->dev with dev. -- With Best Regards, Andy Shevchenko