Re: [PATCH] net: ethernet: arc: Fix a potential memory leak if an optional regulator is deferred

2018-03-16 Thread kbuild test robot
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

2018-03-16 Thread kbuild test robot
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

2018-03-16 Thread Christophe JAILLET

Le 16/03/2018 à 20:27, David Miller a écrit :

From: Christophe JAILLET 
Date: 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

2018-03-16 Thread David Miller
From: Christophe JAILLET 
Date: 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

2018-03-14 Thread Christophe JAILLET
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