All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Javier Carrasco <javier.carrasco.cruz@gmail.com>
Cc: Pavel Machek <pavel@ucw.cz>, Lee Jones <lee@kernel.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	AngeloGioacchino Del Regno
	<angelogioacchino.delregno@collabora.com>,
	Gene Chen <gene_chen@richtek.com>,
	Jacek Anaszewski <jacek.anaszewski@gmail.com>,
	Bartosz Golaszewski <brgl@bgdev.pl>, Chen-Yu Tsai <wens@csie.org>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	Samuel Holland <samuel@sholland.org>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, linux-sunxi@lists.linux.dev,
	stable@vger.kernel.org
Subject: Re: [PATCH 00/18] leds: switch to device_for_each_child_node_scoped()
Date: Sat, 23 Nov 2024 21:29:37 +0200	[thread overview]
Message-ID: <Z0ItIQWHOGL-GTi8@surfacebook.localdomain> (raw)
In-Reply-To: <20240927-leds_device_for_each_child_node_scoped-v1-0-95c0614b38c8@gmail.com>

Fri, Sep 27, 2024 at 01:20:51AM +0200, Javier Carrasco kirjoitti:
> This series switches from the device_for_each_child_node() macro to its
> scoped variant, which in general makes the code more robust if new early
> exits are added to the loops, because there is no need for explicit
> calls to fwnode_handle_put(). Depending on the complexity of the loop
> and its error handling, the code gets simplified and it gets easier to
> follow.
> 
> The non-scoped variant of the macro is error-prone, and it has been the
> source of multiple bugs where the child node refcount was not
> decremented accordingly in error paths within the loops. The first patch
> of this series is a good example, which fixes that kind of bug that is
> regularly found in node iterators.
> 
> The uses of device_for_each_child_node() with no early exits have been
> left untouched because their simpilicty justifies the non-scoped
> variant.
> 
> Note that the child node is now declared in the macro, and therefore the
> explicit declaration is no longer required.
> 
> The general functionality should not be affected by this modification.
> If functional changes are found, please report them back as errors.


Thank you for this series. It may now benefit from the next steps:

1) converting to return dev_err_probe() or dev_warn_probe() directly since
   the goto had been replaced by direct return, hence saving even more LoCs;

2) dropping or refactoring the complex conditions due to your nice series
   being applied.

I'll comment individually on some to show what I meant by the above.

-- 
With Best Regards,
Andy Shevchenko




      parent reply	other threads:[~2024-11-23 19:31 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-26 23:20 [PATCH 00/18] leds: switch to device_for_each_child_node_scoped() Javier Carrasco
2024-09-26 23:20 ` [PATCH 01/18] leds: flash: mt6360: fix device_for_each_child_node() refcounting in error paths Javier Carrasco
2024-09-26 23:20 ` [PATCH 02/18] leds: flash: mt6370: switch to device_for_each_child_node_scoped() Javier Carrasco
2024-09-26 23:20 ` [PATCH 03/18] leds: flash: leds-qcom-flash: " Javier Carrasco
2024-09-26 23:20 ` [PATCH 04/18] leds: aw200xx: " Javier Carrasco
2024-09-26 23:20 ` [PATCH 05/18] leds: cr0014114: " Javier Carrasco
2024-11-23 19:31   ` Andy Shevchenko
2024-09-26 23:20 ` [PATCH 06/18] leds: el15203000: " Javier Carrasco
2024-11-23 19:34   ` Andy Shevchenko
2024-09-26 23:20 ` [PATCH 07/18] leds: gpio: " Javier Carrasco
2024-11-23 19:35   ` Andy Shevchenko
2024-09-26 23:20 ` [PATCH 08/18] leds: lm3532: " Javier Carrasco
2024-11-23 19:37   ` Andy Shevchenko
2024-09-26 23:21 ` [PATCH 09/18] leds: lm3697: " Javier Carrasco
2024-11-23 19:37   ` Andy Shevchenko
2024-09-26 23:21 ` [PATCH 10/18] leds: lp50xx: " Javier Carrasco
2024-09-26 23:21 ` [PATCH 11/18] leds: max77650: " Javier Carrasco
2024-09-27  6:36   ` Bartosz Golaszewski
2024-11-23 19:47   ` Andy Shevchenko
2024-09-26 23:21 ` [PATCH 12/18] leds: ns2: " Javier Carrasco
2024-09-26 23:21 ` [PATCH 13/18] leds: pca963x: " Javier Carrasco
2024-11-23 19:45   ` Andy Shevchenko
2024-09-26 23:21 ` [PATCH 14/18] leds: pwm: " Javier Carrasco
2024-09-26 23:21 ` [PATCH 15/18] leds: sun50i-a100: " Javier Carrasco
2024-11-23 19:41   ` Andy Shevchenko
2024-09-26 23:21 ` [PATCH 16/18] leds: tca6507: " Javier Carrasco
2024-11-23 19:40   ` Andy Shevchenko
2024-09-26 23:21 ` [PATCH 17/18] leds: rgb: ktd202x: " Javier Carrasco
2024-09-26 23:21 ` [PATCH 18/18] leds: rgb: mt6370: " Javier Carrasco
2024-10-09 14:20 ` [PATCH 00/18] leds: " Lee Jones
2024-11-23 19:29 ` Andy Shevchenko [this message]

Reply instructions:

You may reply publicly 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=Z0ItIQWHOGL-GTi8@surfacebook.localdomain \
    --to=andy.shevchenko@gmail.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=brgl@bgdev.pl \
    --cc=gene_chen@richtek.com \
    --cc=jacek.anaszewski@gmail.com \
    --cc=javier.carrasco.cruz@gmail.com \
    --cc=jernej.skrabec@gmail.com \
    --cc=lee@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-sunxi@lists.linux.dev \
    --cc=matthias.bgg@gmail.com \
    --cc=pavel@ucw.cz \
    --cc=samuel@sholland.org \
    --cc=stable@vger.kernel.org \
    --cc=wens@csie.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.