| Message ID | 20191111090357.13903-3-u.kleine-koenig@pengutronix.de |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/4] pwm: omap-dmtimer: remove pwmchip in .remove before making it unfunctional | expand |
> This was found by coccicheck: > > drivers/pwm/pwm-omap-dmtimer.c:304:2-8: ERROR: missing put_device; > call of_find_device_by_node on line 255, but without a corresponding > object release within this function. How do you think about to add a wording according to “imperative mood” for your change description? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=31f4f5b495a62c9a8b15b1c3581acd5efeb9af8c#n151 … > +++ b/drivers/pwm/pwm-omap-dmtimer.c … > @@ -352,7 +352,14 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev) … > pdata->free(dm_timer); > -put: > +err_request_timer: > + > +err_timer_property: > +err_platdata: > + > + put_device(&timer_pdev->dev); Would the use of the label “put_device” be more appropriate? > +err_find_timer_pdev: > + > of_node_put(timer); … Would the use of the label “put_node” be better here? > @@ -372,6 +379,8 @@ static int pwm_omap_dmtimer_remove(struct platform_device *pdev) > > omap->pdata->free(omap->dm_timer); > > + put_device(&omap->dm_timer_pdev->dev); > + > mutex_destroy(&omap->mutex); > > return 0; I suggest to omit a few blank lines. Regards, Markus
On Mon, Nov 11, 2019 at 02:41:58PM +0100, Markus Elfring wrote: > > This was found by coccicheck: > > > > drivers/pwm/pwm-omap-dmtimer.c:304:2-8: ERROR: missing put_device; > > call of_find_device_by_node on line 255, but without a corresponding > > object release within this function. > > How do you think about to add a wording according to “imperative mood” > for your change description? > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=31f4f5b495a62c9a8b15b1c3581acd5efeb9af8c#n151 Are you a bot? > > +++ b/drivers/pwm/pwm-omap-dmtimer.c > … > > @@ -352,7 +352,14 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev) > … > > pdata->free(dm_timer); > > -put: > > +err_request_timer: > > + > > +err_timer_property: > > +err_platdata: > > + > > + put_device(&timer_pdev->dev); > > Would the use of the label “put_device” be more appropriate? > > > > +err_find_timer_pdev: > > + > > of_node_put(timer); > … > > Would the use of the label “put_node” be better here? > > > > @@ -372,6 +379,8 @@ static int pwm_omap_dmtimer_remove(struct platform_device *pdev) > > > > omap->pdata->free(omap->dm_timer); > > > > + put_device(&omap->dm_timer_pdev->dev); > > + > > mutex_destroy(&omap->mutex); > > > > return 0; > > I suggest to omit a few blank lines. And I like it the way it is. Best regards Uwe
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=31f4f5b495a62c9a8b15b1c3581acd5efeb9af8c#n151 > > Are you a bot? I hope not. But I got used to the need to point specific suggestions out several times. Would you like to mention any actions in the commit message explicitly? Regards, Markus
Hello, On Mon, Nov 11, 2019 at 10:38:38PM +0100, Markus Elfring wrote: > >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=31f4f5b495a62c9a8b15b1c3581acd5efeb9af8c#n151 > > > > Are you a bot? > > I hope not. > > But I got used to the need to point specific suggestions out several times. And are you also used to people ignore at least n-1 of n of these repetitions? I don't feel that several near-identical mails that just include a link to some documentation is very constructive. Also I got some of your mails twice which doesn't improve the reception of your comments. > Would you like to mention any actions in the commit message explicitly? I don't understand your question, but I assume the answer is "No, I want to keep the commit log as is". Best regards Uwe
>> Would you like to mention any actions in the commit message explicitly? > > I don't understand your question, I hope that remaining communication difficulties will be resolved better. > but I assume the answer is "No, I want to keep the commit log as is". I suggest to take another look at the relevance of the corresponding Linux development documentation then. Regards, Markus
diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c index e36fcad668a6..88a3c5690fea 100644 --- a/drivers/pwm/pwm-omap-dmtimer.c +++ b/drivers/pwm/pwm-omap-dmtimer.c @@ -256,7 +256,7 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev) if (!timer_pdev) { dev_err(&pdev->dev, "Unable to find Timer pdev\n"); ret = -ENODEV; - goto put; + goto err_find_timer_pdev; } timer_pdata = dev_get_platdata(&timer_pdev->dev); @@ -264,7 +264,7 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev) dev_dbg(&pdev->dev, "dmtimer pdata structure NULL, deferring probe\n"); ret = -EPROBE_DEFER; - goto put; + goto err_platdata; } pdata = timer_pdata->timer_ops; @@ -283,19 +283,19 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev) !pdata->write_counter) { dev_err(&pdev->dev, "Incomplete dmtimer pdata structure\n"); ret = -EINVAL; - goto put; + goto err_platdata; } if (!of_get_property(timer, "ti,timer-pwm", NULL)) { dev_err(&pdev->dev, "Missing ti,timer-pwm capability\n"); ret = -ENODEV; - goto put; + goto err_timer_property; } dm_timer = pdata->request_by_node(timer); if (!dm_timer) { ret = -EPROBE_DEFER; - goto put; + goto err_request_timer; } omap = devm_kzalloc(&pdev->dev, sizeof(*omap), GFP_KERNEL); @@ -352,7 +352,14 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev) err_alloc_omap: pdata->free(dm_timer); -put: +err_request_timer: + +err_timer_property: +err_platdata: + + put_device(&timer_pdev->dev); +err_find_timer_pdev: + of_node_put(timer); return ret; @@ -372,6 +379,8 @@ static int pwm_omap_dmtimer_remove(struct platform_device *pdev) omap->pdata->free(omap->dm_timer); + put_device(&omap->dm_timer_pdev->dev); + mutex_destroy(&omap->mutex); return 0;
This was found by coccicheck: drivers/pwm/pwm-omap-dmtimer.c:304:2-8: ERROR: missing put_device; call of_find_device_by_node on line 255, but without a corresponding object release within this function. Reported-by: Markus Elfring <elfring@users.sourceforge.net> Fixes: 6604c6556db9 ("pwm: Add PWM driver for OMAP using dual-mode timers") Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/pwm/pwm-omap-dmtimer.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)