On Tue, Aug 08, 2023 at 10:07:21AM +0100, Jonathan Cameron wrote: > On Tue, 8 Aug 2023 15:25:01 +0800 > Wenhua Lin <Wenhua.Lin@xxxxxxxxxx> wrote: ... > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + data->base = devm_ioremap_resource(&pdev->dev, res); > > devm_platform_get_and_ioremap_resource) Is res used anywhere else? > > + if (IS_ERR(data->base)) { > > + dev_err(&pdev->dev, "ioremap resource failed.\n"); > > + ret = PTR_ERR(data->base); > > + goto err_free; > > Read up on what devm calls do - there is no need to manually free > things that were allocated with them in the error paths or remove. > So this should be a direct return. Also use > return dev_err_probe(&pdev->dev, PTR_ERR(data->base), > "....") Btw, with struct device *dev = &pdev->dev; all these become neater. > It both creates neater code and for cases where deferred probing > is possible you will get a nice message on 'why' registered for > debug purposes. > > > + } -- With Best Regards, Andy Shevchenko