| Message ID | 1605614877-5670-1-git-send-email-radhey.shyam.pandey@xilinx.com |
|---|---|
| State | New |
| Headers | show |
| Series | [net,v2] net: emaclite: Add error handling for of_address_ and _mdio_setup functions | expand |
On 11/17/20, Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com> wrote: > From: Shravya Kumbham <shravya.kumbham@xilinx.com> > > Add ret variable, condition to check the return value and error > path for the of_address_to_resource() function. It also adds error > handling for mdio setup and decrement refcount of phy node. > > Addresses-Coverity: Event check_return value. > Signed-off-by: Shravya Kumbham <shravya.kumbham@xilinx.com> > Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com> > --- > Changes for v2: > > - Change subject_prefix to target net tree. > - Add error handling for mdio_setup and remove phy_read changes. > Error checking of phy_read will be added along with phy_write > in a followup patch. Document the changes in commit description. > --- > drivers/net/ethernet/xilinx/xilinx_emaclite.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c > b/drivers/net/ethernet/xilinx/xilinx_emaclite.c > index 0c26f5bcc523..4e0005164785 100644 > --- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c > +++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c > @@ -820,7 +820,7 @@ static int xemaclite_mdio_write(struct mii_bus *bus, int > phy_id, int reg, > static int xemaclite_mdio_setup(struct net_local *lp, struct device *dev) > { > struct mii_bus *bus; > - int rc; > + int rc, ret; > struct resource res; > struct device_node *np = of_get_parent(lp->phy_node); > struct device_node *npp; > @@ -834,7 +834,12 @@ static int xemaclite_mdio_setup(struct net_local *lp, > struct device *dev) > } > npp = of_get_parent(np); > > - of_address_to_resource(npp, 0, &res); > + ret = of_address_to_resource(npp, 0, &res); > + if (ret) { > + dev_err(dev, "%s resource error!\n", > + dev->of_node->full_name); > + return ret; > + } npp is not returned to of_node_put() in the error case below > if (lp->ndev->mem_start != res.start) { > struct phy_device *phydev; > phydev = of_phy_find_device(lp->phy_node); > @@ -1173,7 +1178,11 @@ static int xemaclite_of_probe(struct platform_device > *ofdev) > xemaclite_update_address(lp, ndev->dev_addr); > > lp->phy_node = of_parse_phandle(ofdev->dev.of_node, "phy-handle", 0); > - xemaclite_mdio_setup(lp, &ofdev->dev); > + rc = xemaclite_mdio_setup(lp, &ofdev->dev); > + if (rc) { > + dev_warn(dev, "error registering MDIO bus: %d\n", rc); > + goto error; > + } > > dev_info(dev, "MAC address is now %pM\n", ndev->dev_addr); > > @@ -1197,6 +1206,7 @@ static int xemaclite_of_probe(struct platform_device > *ofdev) > return 0; > > error: > + of_node_put(lp->phy_node); > free_netdev(ndev); > return rc; > } > -- > 2.7.4 > >
> -----Original Message----- > From: Denis Kirjanov <kda@linux-powerpc.org> > Sent: Tuesday, November 17, 2020 6:43 PM > To: Radhey Shyam Pandey <radheys@xilinx.com> > Cc: davem@davemloft.net; netdev@vger.kernel.org; kuba@kernel.org; > Michal Simek <michals@xilinx.com>; mchehab+samsung@kernel.org; > gregkh@linuxfoundation.org; nicolas.ferre@microchip.com; linux-arm- > kernel@lists.infradead.org; linux-kernel@vger.kernel.org; git > <git@xilinx.com>; Shravya Kumbham <shravyak@xilinx.com> > Subject: Re: [PATCH net v2] net: emaclite: Add error handling for > of_address_ and _mdio_setup functions > > On 11/17/20, Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com> > wrote: > > From: Shravya Kumbham <shravya.kumbham@xilinx.com> > > > > Add ret variable, condition to check the return value and error path > > for the of_address_to_resource() function. It also adds error handling > > for mdio setup and decrement refcount of phy node. > > > > Addresses-Coverity: Event check_return value. > > Signed-off-by: Shravya Kumbham <shravya.kumbham@xilinx.com> > > Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com> > > --- > > Changes for v2: > > > > - Change subject_prefix to target net tree. > > - Add error handling for mdio_setup and remove phy_read changes. > > Error checking of phy_read will be added along with phy_write > > in a followup patch. Document the changes in commit description. > > --- > > drivers/net/ethernet/xilinx/xilinx_emaclite.c | 16 +++++++++++++--- > > 1 file changed, 13 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c > > b/drivers/net/ethernet/xilinx/xilinx_emaclite.c > > index 0c26f5bcc523..4e0005164785 100644 > > --- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c > > +++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c > > @@ -820,7 +820,7 @@ static int xemaclite_mdio_write(struct mii_bus > > *bus, int phy_id, int reg, static int xemaclite_mdio_setup(struct > > net_local *lp, struct device *dev) { > > struct mii_bus *bus; > > - int rc; > > + int rc, ret; > > struct resource res; > > struct device_node *np = of_get_parent(lp->phy_node); > > struct device_node *npp; > > @@ -834,7 +834,12 @@ static int xemaclite_mdio_setup(struct net_local > > *lp, struct device *dev) > > } > > npp = of_get_parent(np); > > > > - of_address_to_resource(npp, 0, &res); > > + ret = of_address_to_resource(npp, 0, &res); > > + if (ret) { > > + dev_err(dev, "%s resource error!\n", > > + dev->of_node->full_name); > > + return ret; > > + } > > npp is not returned to of_node_put() in the error case below Yes, agree. The ref counting of npp device node is broken. I am planning to send v3 with a separate patch to simplify this. [1/2] net: emaclite: Simplify device node ref counting [2/2] net: emaclite: Add error handling for of_address_ to_resource and xemaclite_mdio_setup functions > > > if (lp->ndev->mem_start != res.start) { > > struct phy_device *phydev; > > phydev = of_phy_find_device(lp->phy_node); @@ -1173,7 > +1178,11 @@ > > static int xemaclite_of_probe(struct platform_device > > *ofdev) > > xemaclite_update_address(lp, ndev->dev_addr); > > > > lp->phy_node = of_parse_phandle(ofdev->dev.of_node, "phy- > handle", 0); > > - xemaclite_mdio_setup(lp, &ofdev->dev); > > + rc = xemaclite_mdio_setup(lp, &ofdev->dev); > > + if (rc) { > > + dev_warn(dev, "error registering MDIO bus: %d\n", rc); > > + goto error; > > + } > > > > dev_info(dev, "MAC address is now %pM\n", ndev->dev_addr); > > > > @@ -1197,6 +1206,7 @@ static int xemaclite_of_probe(struct > > platform_device > > *ofdev) > > return 0; > > > > error: > > + of_node_put(lp->phy_node); > > free_netdev(ndev); > > return rc; > > } > > -- > > 2.7.4 > > > >
diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c index 0c26f5bcc523..4e0005164785 100644 --- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c +++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c @@ -820,7 +820,7 @@ static int xemaclite_mdio_write(struct mii_bus *bus, int phy_id, int reg, static int xemaclite_mdio_setup(struct net_local *lp, struct device *dev) { struct mii_bus *bus; - int rc; + int rc, ret; struct resource res; struct device_node *np = of_get_parent(lp->phy_node); struct device_node *npp; @@ -834,7 +834,12 @@ static int xemaclite_mdio_setup(struct net_local *lp, struct device *dev) } npp = of_get_parent(np); - of_address_to_resource(npp, 0, &res); + ret = of_address_to_resource(npp, 0, &res); + if (ret) { + dev_err(dev, "%s resource error!\n", + dev->of_node->full_name); + return ret; + } if (lp->ndev->mem_start != res.start) { struct phy_device *phydev; phydev = of_phy_find_device(lp->phy_node); @@ -1173,7 +1178,11 @@ static int xemaclite_of_probe(struct platform_device *ofdev) xemaclite_update_address(lp, ndev->dev_addr); lp->phy_node = of_parse_phandle(ofdev->dev.of_node, "phy-handle", 0); - xemaclite_mdio_setup(lp, &ofdev->dev); + rc = xemaclite_mdio_setup(lp, &ofdev->dev); + if (rc) { + dev_warn(dev, "error registering MDIO bus: %d\n", rc); + goto error; + } dev_info(dev, "MAC address is now %pM\n", ndev->dev_addr); @@ -1197,6 +1206,7 @@ static int xemaclite_of_probe(struct platform_device *ofdev) return 0; error: + of_node_put(lp->phy_node); free_netdev(ndev); return rc; }