LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Markus Elfring <Markus.Elfring@web.de>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	linux-pwm@vger.kernel.org
Cc: Thierry Reding <thierry.reding@gmail.com>,
	Neil Brown <neilb@suse.de>,
	Neil Armstrong <narmstrong@baylibre.com>,
	kernel@pengutronix.de, kernel-janitors@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/4] pwm: omap-dmtimer: simplify error handling
Date: Mon, 11 Nov 2019 14:32:30 +0100
Message-ID: <a7a4656d-98b4-6bbb-e389-fe7028a38f97@web.de> (raw)
In-Reply-To: <20191111090357.13903-2-u.kleine-koenig@pengutronix.de>

> Implementation note: The put: label was never reached without a goto and
> ret being unequal to 0, so the removed return statement is fine.

This can look fine (in principle) because the label was repositioned here.
Do you really want to call the function “of_node_put” at two places now?


> +++ b/drivers/pwm/pwm-omap-dmtimer.c
>  	omap = devm_kzalloc(&pdev->dev, sizeof(*omap), GFP_KERNEL);
>  	if (!omap) {
> -		pdata->free(dm_timer);
> -		return -ENOMEM;
> +		ret = -ENOMEM;
> +		goto err_alloc_omap;
>  	}
…

I suggest to reconsider your label name selection according to
the Linux coding style.


> @@ -339,13 +334,28 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev)
> +err_pwmchip_add:
> +
> +	/*
> +	 * *omap is allocated using devm_kzalloc,
> +	 * so no free necessary here
> +	 */
> +err_alloc_omap:
> +
> +	pdata->free(dm_timer);

Would the use of the label “free_dm_timer” be more appropriate?


> +put:
> +	of_node_put(timer);
…

Can the label “put_node” be nicer?

Regards,
Markus

  parent reply index

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-09 12:26 [PATCH] pwm: omap-dmtimer: Add missing put_device() call in pwm_omap_dmtimer_probe() Markus Elfring
2019-11-11  7:19 ` Uwe Kleine-König
     [not found]   ` <20191111090357.13903-1-u.kleine-koenig@pengutronix.de>
     [not found]     ` <20191111091606.qf7gj5gln3khfnsl@pengutronix.de>
2019-11-11 13:28       ` [0/4] pwm: omap-dmtimer: Software improvements Markus Elfring
2019-11-11 13:28       ` Markus Elfring
2019-11-11 19:57         ` Uwe Kleine-König
2019-11-11 13:30     ` [PATCH 1/4] pwm: omap-dmtimer: remove pwmchip in .remove before making it unfunctional Markus Elfring
2019-11-11 20:00       ` Uwe Kleine-König
2019-11-11 21:00         ` [1/4] " Markus Elfring
     [not found]     ` <20191111090357.13903-2-u.kleine-koenig@pengutronix.de>
2019-11-11 13:32       ` Markus Elfring [this message]
2019-11-11 20:08         ` [PATCH 2/4] pwm: omap-dmtimer: simplify error handling Uwe Kleine-König
2019-11-11 21:30           ` Markus Elfring
     [not found]     ` <20191111090357.13903-3-u.kleine-koenig@pengutronix.de>
2019-11-11 13:41       ` [PATCH 3/4] pwm: omap-dmtimer: put_device() after of_find_device_by_node() Markus Elfring
2019-11-11 20:09         ` Uwe Kleine-König
2019-11-11 21:38           ` [3/4] " Markus Elfring
2019-11-11 21:46             ` Uwe Kleine-König
2019-11-12  7:40               ` Markus Elfring
     [not found]     ` <20191111090357.13903-4-u.kleine-koenig@pengutronix.de>
2019-11-11 13:47       ` [PATCH 4/4] pwm: omap-dmtimer: Allow compiling with COMPILE_TEST Markus Elfring
2019-11-11 13:47       ` Markus Elfring

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a7a4656d-98b4-6bbb-e389-fe7028a38f97@web.de \
    --to=markus.elfring@web.de \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=narmstrong@baylibre.com \
    --cc=neilb@suse.de \
    --cc=thierry.reding@gmail.com \
    --cc=u.kleine-koenig@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git