Re: [PATCH] net: ethernet: arc: Fix a potential memory leak if an optional regulator is deferred
Hi Christophe, Thank you for the patch! Yet something to improve: [auto build test ERROR on v4.16-rc4] [also build test ERROR on next-20180316] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Christophe-JAILLET/net-ethernet-arc-Fix-a-potential-memory-leak-if-an-optional-regulator-is-deferred/20180317-042849 config: openrisc-allyesconfig (attached as .config) compiler: or1k-linux-gcc (GCC) 6.0.0 20160327 (experimental) reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=openrisc All errors (new ones prefixed by >>): drivers/net/ethernet/arc/emac_rockchip.c: In function 'emac_rockchip_probe': >> drivers/net/ethernet/arc/emac_rockchip.c:173:4: error: 'ret' undeclared >> (first use in this function) ret = -EPROBE_DEFER; ^~~ drivers/net/ethernet/arc/emac_rockchip.c:173:4: note: each undeclared identifier is reported only once for each function it appears in vim +/ret +173 drivers/net/ethernet/arc/emac_rockchip.c 102 103 static int emac_rockchip_probe(struct platform_device *pdev) 104 { 105 struct device *dev = >dev; 106 struct net_device *ndev; 107 struct rockchip_priv_data *priv; 108 const struct of_device_id *match; 109 u32 data; 110 int err, interface; 111 112 if (!pdev->dev.of_node) 113 return -ENODEV; 114 115 ndev = alloc_etherdev(sizeof(struct rockchip_priv_data)); 116 if (!ndev) 117 return -ENOMEM; 118 platform_set_drvdata(pdev, ndev); 119 SET_NETDEV_DEV(ndev, dev); 120 121 priv = netdev_priv(ndev); 122 priv->emac.drv_name = DRV_NAME; 123 priv->emac.drv_version = DRV_VERSION; 124 priv->emac.set_mac_speed = emac_rockchip_set_mac_speed; 125 126 interface = of_get_phy_mode(dev->of_node); 127 128 /* RK3036/RK3066/RK3188 SoCs only support RMII */ 129 if (interface != PHY_INTERFACE_MODE_RMII) { 130 dev_err(dev, "unsupported phy interface mode %d\n", interface); 131 err = -ENOTSUPP; 132 goto out_netdev; 133 } 134 135 priv->grf = syscon_regmap_lookup_by_phandle(dev->of_node, 136 "rockchip,grf"); 137 if (IS_ERR(priv->grf)) { 138 dev_err(dev, "failed to retrieve global register file (%ld)\n", 139 PTR_ERR(priv->grf)); 140 err = PTR_ERR(priv->grf); 141 goto out_netdev; 142 } 143 144 match = of_match_node(emac_rockchip_dt_ids, dev->of_node); 145 priv->soc_data = match->data; 146 147 priv->emac.clk = devm_clk_get(dev, "hclk"); 148 if (IS_ERR(priv->emac.clk)) { 149 dev_err(dev, "failed to retrieve host clock (%ld)\n", 150 PTR_ERR(priv->emac.clk)); 151 err = PTR_ERR(priv->emac.clk); 152 goto out_netdev; 153 } 154 155 priv->refclk = devm_clk_get(dev, "macref"); 156 if (IS_ERR(priv->refclk)) { 157 dev_err(dev, "failed to retrieve reference clock (%ld)\n", 158 PTR_ERR(priv->refclk)); 159 err = PTR_ERR(priv->refclk); 160 goto out_netdev; 161 } 162 163 err = clk_prepare_enable(priv->refclk); 164 if (err) { 165 dev_err(dev, "failed to enable reference clock (%d)\n", err); 166 goto out_netdev; 167 } 168 169 /* Optional regulator for PHY */ 170 priv->regulator = devm_regulator_get_optional(dev, "phy"); 171 if (IS_ERR(priv->regulator)) { 172 if (PTR_ERR(priv->regulator) == -EPROBE_DEFER) { > 173 ret = -EPROBE_DEFER; 174 goto out_clk_disable; 175 } 176 dev_err(dev, "no regulator found\n"); 177 priv->regulator = NULL; 178 } 179 180 if (priv->regulator) { 181 err = regulator_enable(priv->regulator); 182 if (err) { 183 dev_err(dev, "failed to enable phy-supply (%d)\n", err); 184 goto out_clk_disable; 185 } 186 } 187 188 /* Set speed 100M */ 189 data = (1 <<
Re: [PATCH] net: ethernet: arc: Fix a potential memory leak if an optional regulator is deferred
Hi Christophe, Thank you for the patch! Yet something to improve: [auto build test ERROR on v4.16-rc4] [also build test ERROR on next-20180316] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Christophe-JAILLET/net-ethernet-arc-Fix-a-potential-memory-leak-if-an-optional-regulator-is-deferred/20180317-042849 config: i386-allmodconfig (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): drivers/net/ethernet/arc/emac_rockchip.c: In function 'emac_rockchip_probe': >> drivers/net/ethernet/arc/emac_rockchip.c:173:4: error: 'ret' undeclared >> (first use in this function); did you mean 'net'? ret = -EPROBE_DEFER; ^~~ net drivers/net/ethernet/arc/emac_rockchip.c:173:4: note: each undeclared identifier is reported only once for each function it appears in vim +173 drivers/net/ethernet/arc/emac_rockchip.c 102 103 static int emac_rockchip_probe(struct platform_device *pdev) 104 { 105 struct device *dev = >dev; 106 struct net_device *ndev; 107 struct rockchip_priv_data *priv; 108 const struct of_device_id *match; 109 u32 data; 110 int err, interface; 111 112 if (!pdev->dev.of_node) 113 return -ENODEV; 114 115 ndev = alloc_etherdev(sizeof(struct rockchip_priv_data)); 116 if (!ndev) 117 return -ENOMEM; 118 platform_set_drvdata(pdev, ndev); 119 SET_NETDEV_DEV(ndev, dev); 120 121 priv = netdev_priv(ndev); 122 priv->emac.drv_name = DRV_NAME; 123 priv->emac.drv_version = DRV_VERSION; 124 priv->emac.set_mac_speed = emac_rockchip_set_mac_speed; 125 126 interface = of_get_phy_mode(dev->of_node); 127 128 /* RK3036/RK3066/RK3188 SoCs only support RMII */ 129 if (interface != PHY_INTERFACE_MODE_RMII) { 130 dev_err(dev, "unsupported phy interface mode %d\n", interface); 131 err = -ENOTSUPP; 132 goto out_netdev; 133 } 134 135 priv->grf = syscon_regmap_lookup_by_phandle(dev->of_node, 136 "rockchip,grf"); 137 if (IS_ERR(priv->grf)) { 138 dev_err(dev, "failed to retrieve global register file (%ld)\n", 139 PTR_ERR(priv->grf)); 140 err = PTR_ERR(priv->grf); 141 goto out_netdev; 142 } 143 144 match = of_match_node(emac_rockchip_dt_ids, dev->of_node); 145 priv->soc_data = match->data; 146 147 priv->emac.clk = devm_clk_get(dev, "hclk"); 148 if (IS_ERR(priv->emac.clk)) { 149 dev_err(dev, "failed to retrieve host clock (%ld)\n", 150 PTR_ERR(priv->emac.clk)); 151 err = PTR_ERR(priv->emac.clk); 152 goto out_netdev; 153 } 154 155 priv->refclk = devm_clk_get(dev, "macref"); 156 if (IS_ERR(priv->refclk)) { 157 dev_err(dev, "failed to retrieve reference clock (%ld)\n", 158 PTR_ERR(priv->refclk)); 159 err = PTR_ERR(priv->refclk); 160 goto out_netdev; 161 } 162 163 err = clk_prepare_enable(priv->refclk); 164 if (err) { 165 dev_err(dev, "failed to enable reference clock (%d)\n", err); 166 goto out_netdev; 167 } 168 169 /* Optional regulator for PHY */ 170 priv->regulator = devm_regulator_get_optional(dev, "phy"); 171 if (IS_ERR(priv->regulator)) { 172 if (PTR_ERR(priv->regulator) == -EPROBE_DEFER) { > 173 ret = -EPROBE_DEFER; 174 goto out_clk_disable; 175 } 176 dev_err(dev, "no regulator found\n"); 177 priv->regulator = NULL; 178 } 179 180 if (priv->regulator) { 181 err = regulator_enable(priv->regulator); 182 if (err) { 183 dev_err(dev, "failed to enable phy-supply (%d)\n", err); 184 goto out_clk_disable; 185 } 186 } 187 188 /* Set speed 100M */ 189 data = (1 << (priv->soc_data->grf_speed_offset + 16)) | 190 (1 << priv->soc_data->grf_speed_offset); 191 /* Set RMII mode */ 192
Re: [PATCH] net: ethernet: arc: Fix a potential memory leak if an optional regulator is deferred
Le 16/03/2018 à 20:27, David Miller a écrit : From: Christophe JAILLETDate: Wed, 14 Mar 2018 22:09:34 +0100 diff --git a/drivers/net/ethernet/arc/emac_rockchip.c b/drivers/net/ethernet/arc/emac_rockchip.c index 16f9bee992fe..8ee9dfd0e363 100644 --- a/drivers/net/ethernet/arc/emac_rockchip.c +++ b/drivers/net/ethernet/arc/emac_rockchip.c @@ -169,8 +169,10 @@ static int emac_rockchip_probe(struct platform_device *pdev) /* Optional regulator for PHY */ priv->regulator = devm_regulator_get_optional(dev, "phy"); if (IS_ERR(priv->regulator)) { - if (PTR_ERR(priv->regulator) == -EPROBE_DEFER) - return -EPROBE_DEFER; + if (PTR_ERR(priv->regulator) == -EPROBE_DEFER) { + ret = -EPROBE_DEFER; + goto out_clk_disable; + } Please build test your changes. There is no 'ret' variable in this function, perhaps you meant 'err'. Yes, obviously, this is 'err'. I apologize. I usually build-test all my patches before submission. This one seems to have gone out of my normal process. Can you fix it yourself or do you want me to re-submit? CJ
Re: [PATCH] net: ethernet: arc: Fix a potential memory leak if an optional regulator is deferred
From: Christophe JAILLETDate: Wed, 14 Mar 2018 22:09:34 +0100 > diff --git a/drivers/net/ethernet/arc/emac_rockchip.c > b/drivers/net/ethernet/arc/emac_rockchip.c > index 16f9bee992fe..8ee9dfd0e363 100644 > --- a/drivers/net/ethernet/arc/emac_rockchip.c > +++ b/drivers/net/ethernet/arc/emac_rockchip.c > @@ -169,8 +169,10 @@ static int emac_rockchip_probe(struct platform_device > *pdev) > /* Optional regulator for PHY */ > priv->regulator = devm_regulator_get_optional(dev, "phy"); > if (IS_ERR(priv->regulator)) { > - if (PTR_ERR(priv->regulator) == -EPROBE_DEFER) > - return -EPROBE_DEFER; > + if (PTR_ERR(priv->regulator) == -EPROBE_DEFER) { > + ret = -EPROBE_DEFER; > + goto out_clk_disable; > + } Please build test your changes. There is no 'ret' variable in this function, perhaps you meant 'err'.
[PATCH] net: ethernet: arc: Fix a potential memory leak if an optional regulator is deferred
If the optional regulator is deferrred, we must release some resources. They will be re-allocated when the probe function will be called again. Fixes: 6eacf31139bf ("ethernet: arc: Add support for Rockchip SoC layer device tree bindings") Signed-off-by: Christophe JAILLET--- drivers/net/ethernet/arc/emac_rockchip.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/arc/emac_rockchip.c b/drivers/net/ethernet/arc/emac_rockchip.c index 16f9bee992fe..8ee9dfd0e363 100644 --- a/drivers/net/ethernet/arc/emac_rockchip.c +++ b/drivers/net/ethernet/arc/emac_rockchip.c @@ -169,8 +169,10 @@ static int emac_rockchip_probe(struct platform_device *pdev) /* Optional regulator for PHY */ priv->regulator = devm_regulator_get_optional(dev, "phy"); if (IS_ERR(priv->regulator)) { - if (PTR_ERR(priv->regulator) == -EPROBE_DEFER) - return -EPROBE_DEFER; + if (PTR_ERR(priv->regulator) == -EPROBE_DEFER) { + ret = -EPROBE_DEFER; + goto out_clk_disable; + } dev_err(dev, "no regulator found\n"); priv->regulator = NULL; } -- 2.14.1