Messages in this thread | | | Date | Fri, 11 Jun 2021 05:35:22 +0200 | From | Andrew Lunn <> | Subject | Re: [PATCH net-next] net: mdio: mscc-miim: Use devm_platform_get_and_ioremap_resource() |
| |
On Fri, Jun 11, 2021 at 09:35:04AM +0800, Yang Yingliang wrote: > Hi, > > On 2021/6/11 0:01, Andrew Lunn wrote: > > > - dev->regs = devm_ioremap_resource(&pdev->dev, res); > > > + dev->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &res); > > > if (IS_ERR(dev->regs)) { > > Here, only dev->regs is considered. > > > > > dev_err(&pdev->dev, "Unable to map MIIM registers\n"); > > > return PTR_ERR(dev->regs); > > > } > > > > > > > + dev->phy_regs = devm_platform_get_and_ioremap_resource(pdev, 1, &res); > > > + if (res && IS_ERR(dev->phy_regs)) { > > Here you look at both res and dev->phy_regs. > > > > This seems inconsistent. Can devm_platform_get_and_ioremap_resource() > > return success despite res being NULL? > No, if res is NULL, devm_platform_get_and_ioremap_resource() returns failed. > But, before this patch, if the internal phy res is NULL, it doesn't return > error > code, so I checked the res to make sure it doesn't change the origin code > logic.
O.K, so IORESOURCE_MEM, 1 is optional. By making this change, i think you have made this less clear. So i would say it is O.K. to change the first platform_get_resource(pdev, IORESOURCE_MEM, 0) and devm_ioremap_resource(&pdev->dev, res) to one call, but i would leave the second pair alone.
Andrew
| |