* Re: [PATCH] net: ethernet: arc: Fix a potential memory leak if an optional regulator is deferred
2018-03-14 21:09 [PATCH] net: ethernet: arc: Fix a potential memory leak if an optional regulator is deferred Christophe JAILLET
@ 2018-03-16 19:27 ` David Miller
2018-03-16 20:51 ` Christophe JAILLET
2018-03-16 20:52 ` Christophe JAILLET
2018-03-16 23:42 ` kbuild test robot
2018-03-17 1:39 ` kbuild test robot
2 siblings, 2 replies; 6+ messages in thread
From: David Miller @ 2018-03-16 19:27 UTC (permalink / raw)
To: christophe.jaillet
Cc: heiko, branislav, netdev, linux-arm-kernel, linux-rockchip,
linux-kernel, kernel-janitors
From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
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'.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: ethernet: arc: Fix a potential memory leak if an optional regulator is deferred
2018-03-16 19:27 ` David Miller
@ 2018-03-16 20:51 ` Christophe JAILLET
2018-03-16 20:52 ` Christophe JAILLET
1 sibling, 0 replies; 6+ messages in thread
From: Christophe JAILLET @ 2018-03-16 20:51 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, linux-arm-kernel, linux-rockchip, kernel-janitors
Le 16/03/2018 à 20:27, David Miller a écrit :
> From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: ethernet: arc: Fix a potential memory leak if an optional regulator is deferred
2018-03-16 19:27 ` David Miller
2018-03-16 20:51 ` Christophe JAILLET
@ 2018-03-16 20:52 ` Christophe JAILLET
1 sibling, 0 replies; 6+ messages in thread
From: Christophe JAILLET @ 2018-03-16 20:52 UTC (permalink / raw)
To: David Miller
Cc: heiko, branislav, netdev, linux-arm-kernel, linux-rockchip,
linux-kernel, kernel-janitors
Le 16/03/2018 à 20:27, David Miller a écrit :
> From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: ethernet: arc: Fix a potential memory leak if an optional regulator is deferred
2018-03-14 21:09 [PATCH] net: ethernet: arc: Fix a potential memory leak if an optional regulator is deferred Christophe JAILLET
2018-03-16 19:27 ` David Miller
@ 2018-03-16 23:42 ` kbuild test robot
2018-03-17 1:39 ` kbuild test robot
2 siblings, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2018-03-16 23:42 UTC (permalink / raw)
To: Christophe JAILLET
Cc: kbuild-all, heiko, branislav, davem, netdev, linux-arm-kernel,
linux-rockchip, linux-kernel, kernel-janitors,
Christophe JAILLET
[-- Attachment #1: Type: text/plain, Size: 6251 bytes --]
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 = &pdev->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 data |= (1 << (priv->soc_data->grf_mode_offset + 16)) |
193 (0 << priv->soc_data->grf_mode_offset);
194
195 err = regmap_write(priv->grf, priv->soc_data->grf_offset, data);
196 if (err) {
197 dev_err(dev, "unable to apply initial settings to grf (%d)\n",
198 err);
199 goto out_regulator_disable;
200 }
201
202 /* RMII interface needs always a rate of 50MHz */
203 err = clk_set_rate(priv->refclk, 50000000);
204 if (err) {
205 dev_err(dev,
206 "failed to change reference clock rate (%d)\n", err);
207 goto out_regulator_disable;
208 }
209
210 if (priv->soc_data->need_div_macclk) {
211 priv->macclk = devm_clk_get(dev, "macclk");
212 if (IS_ERR(priv->macclk)) {
213 dev_err(dev, "failed to retrieve mac clock (%ld)\n",
214 PTR_ERR(priv->macclk));
215 err = PTR_ERR(priv->macclk);
216 goto out_regulator_disable;
217 }
218
219 err = clk_prepare_enable(priv->macclk);
220 if (err) {
221 dev_err(dev, "failed to enable mac clock (%d)\n", err);
222 goto out_regulator_disable;
223 }
224
225 /* RMII TX/RX needs always a rate of 25MHz */
226 err = clk_set_rate(priv->macclk, 25000000);
227 if (err) {
228 dev_err(dev,
229 "failed to change mac clock rate (%d)\n", err);
230 goto out_clk_disable_macclk;
231 }
232 }
233
234 err = arc_emac_probe(ndev, interface);
235 if (err) {
236 dev_err(dev, "failed to probe arc emac (%d)\n", err);
237 goto out_clk_disable_macclk;
238 }
239
240 return 0;
241
242 out_clk_disable_macclk:
243 if (priv->soc_data->need_div_macclk)
244 clk_disable_unprepare(priv->macclk);
245 out_regulator_disable:
246 if (priv->regulator)
247 regulator_disable(priv->regulator);
248 out_clk_disable:
249 clk_disable_unprepare(priv->refclk);
250 out_netdev:
251 free_netdev(ndev);
252 return err;
253 }
254
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 63084 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: ethernet: arc: Fix a potential memory leak if an optional regulator is deferred
2018-03-14 21:09 [PATCH] net: ethernet: arc: Fix a potential memory leak if an optional regulator is deferred Christophe JAILLET
2018-03-16 19:27 ` David Miller
2018-03-16 23:42 ` kbuild test robot
@ 2018-03-17 1:39 ` kbuild test robot
2 siblings, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2018-03-17 1:39 UTC (permalink / raw)
To: Christophe JAILLET
Cc: kbuild-all, heiko, branislav, davem, netdev, linux-arm-kernel,
linux-rockchip, linux-kernel, kernel-janitors,
Christophe JAILLET
[-- Attachment #1: Type: text/plain, Size: 6401 bytes --]
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 = &pdev->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 data |= (1 << (priv->soc_data->grf_mode_offset + 16)) |
193 (0 << priv->soc_data->grf_mode_offset);
194
195 err = regmap_write(priv->grf, priv->soc_data->grf_offset, data);
196 if (err) {
197 dev_err(dev, "unable to apply initial settings to grf (%d)\n",
198 err);
199 goto out_regulator_disable;
200 }
201
202 /* RMII interface needs always a rate of 50MHz */
203 err = clk_set_rate(priv->refclk, 50000000);
204 if (err) {
205 dev_err(dev,
206 "failed to change reference clock rate (%d)\n", err);
207 goto out_regulator_disable;
208 }
209
210 if (priv->soc_data->need_div_macclk) {
211 priv->macclk = devm_clk_get(dev, "macclk");
212 if (IS_ERR(priv->macclk)) {
213 dev_err(dev, "failed to retrieve mac clock (%ld)\n",
214 PTR_ERR(priv->macclk));
215 err = PTR_ERR(priv->macclk);
216 goto out_regulator_disable;
217 }
218
219 err = clk_prepare_enable(priv->macclk);
220 if (err) {
221 dev_err(dev, "failed to enable mac clock (%d)\n", err);
222 goto out_regulator_disable;
223 }
224
225 /* RMII TX/RX needs always a rate of 25MHz */
226 err = clk_set_rate(priv->macclk, 25000000);
227 if (err) {
228 dev_err(dev,
229 "failed to change mac clock rate (%d)\n", err);
230 goto out_clk_disable_macclk;
231 }
232 }
233
234 err = arc_emac_probe(ndev, interface);
235 if (err) {
236 dev_err(dev, "failed to probe arc emac (%d)\n", err);
237 goto out_clk_disable_macclk;
238 }
239
240 return 0;
241
242 out_clk_disable_macclk:
243 if (priv->soc_data->need_div_macclk)
244 clk_disable_unprepare(priv->macclk);
245 out_regulator_disable:
246 if (priv->regulator)
247 regulator_disable(priv->regulator);
248 out_clk_disable:
249 clk_disable_unprepare(priv->refclk);
250 out_netdev:
251 free_netdev(ndev);
252 return err;
253 }
254
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 44804 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread