| Message ID | 20240823035116.21590-5-rongqianfeng@vivo.com |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: Use devm_clk_get_enabled() helpers | expand |
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.
在 2024/8/23 23:36, Andy Shevchenko 写道: > [Some people who received this message don't often get email from andriy.shevchenko@intel.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > 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. Thanks very much for the detailed comments, I will modify all your suggestions in the next version. Best Regards, Qianfeng > > -- > With Best Regards, > Andy Shevchenko > >
diff --git a/drivers/i2c/busses/i2c-jz4780.c b/drivers/i2c/busses/i2c-jz4780.c index f5362c5dfb50..0cb52a6d05b5 100644 --- a/drivers/i2c/busses/i2c-jz4780.c +++ b/drivers/i2c/busses/i2c-jz4780.c @@ -798,17 +798,15 @@ static int jz4780_i2c_probe(struct platform_device *pdev) 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"); 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"); + jz4780_i2c_set_speed(i2c); dev_info(&pdev->dev, "Bus frequency is %d KHz\n", i2c->speed);
No more special handling needed here, so use dev_err_probe() to simplify the code. Signed-off-by: Rong Qianfeng <rongqianfeng@vivo.com> --- drivers/i2c/busses/i2c-jz4780.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)