mbox series

[RFC,00/13] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling.

Message ID 20240101172611.694830-1-jic23@kernel.org
Headers show
Series device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling. | expand

Message

Jonathan Cameron Jan. 1, 2024, 5:25 p.m. UTC
From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

RFC mainly because it's untested. I have none of the relevant hardware and
haven't yet emulated the firmware descriptions to test this.
I have tested the device tree only version:
https://lore.kernel.org/linux-iio/20231217184648.185236-1-jic23@kernel.org/
which is very similar.

Failing to release the references on early exit from loops over child nodes
and similar are a fairly common source of bugs. The need to explicitly
release the references via fwnode_handle_put() also complicate the code.

The first patch enables

	struct fwnode_handle *child __free(fwnode_handle) = NULL;

	device_for_each_child_node(dev, child) {
		if (err)
			/*
			 * Previously needed a fwnode_handle_put() here,
			 * will now be called automatically as well leave
			 * the scope within which the cleanup is registered
			 */
			return err;
	}

/*
 * fwnode_handle_put() no automatically called here - but child == NULL so
 * that call is a noop
 */
}

As can be seen by the examples from IIO that follow this can save
a reasonable amount of complexity and boiler plate code, often enabling
additional cleanups in related code such as use of
return dev_err_probe().

Jonathan Cameron (13):
  device property: Add cleanup.h based fwnode_handle_put() scope based
    cleanup.
  iio: adc: max11410: Use __free(fwnode_handle) to replace
    fwnode_handle_put() calls
  iio: adc: mcp3564: Use __free(fwnode_handle) to replace
    fwnode_handle_put() calls
  iio: adc: qcom-spmi-adc5: Use __free(fwnode_handle) to replace
    fwnode_handle_put() calls
  iio: adc: rzg2l_adc: Use __free(fwnode_handle) to replace
    fwnode_handle_put() calls
  iio: adc: stm32: Use __free(fwnode_handle) to replace
    fwnode_handle_put() calls
  iio: adc: ti-ads1015: Use __free(fwnode_handle) to replace
    fwnode_handle_put() calls
  iio: adc: ti-ads131e08: Use __free(fwnode_handle) to replace
    fwnode_handle_put() calls
  iio: addac: ad74413r: Use __free(fwnode_handle) to replace
    fwnode_handle_put() calls
  iio: dac: ad3552: Use __free(fwnode_handle) to replace
    fwnode_handle_put() calls
  iio: dac: ad5770r: Use __free(fwnode_handle) to replace
    fwnode_handle_put() calls
  iio: dac: ltc2688: Use __free(fwnode_handle) to replace
    fwnode_handle_put() calls
  iio: temp: ltc2983: Use __free(fwnode_handle) to replace
    fwnode_handle_put() calls

 drivers/iio/adc/max11410.c        | 26 ++++--------
 drivers/iio/adc/mcp3564.c         | 15 ++++---
 drivers/iio/adc/qcom-spmi-adc5.c  |  6 +--
 drivers/iio/adc/rzg2l_adc.c       | 10 ++---
 drivers/iio/adc/stm32-adc.c       | 62 +++++++++++----------------
 drivers/iio/adc/ti-ads1015.c      |  4 +-
 drivers/iio/adc/ti-ads131e08.c    | 12 ++----
 drivers/iio/addac/ad74413r.c      |  9 +---
 drivers/iio/dac/ad3552r.c         | 50 +++++++++-------------
 drivers/iio/dac/ad5770r.c         | 18 +++-----
 drivers/iio/dac/ltc2688.c         | 23 +++-------
 drivers/iio/temperature/ltc2983.c | 70 ++++++++++---------------------
 include/linux/property.h          |  2 +
 13 files changed, 104 insertions(+), 203 deletions(-)

Comments

Andy Shevchenko Jan. 6, 2024, 3:19 p.m. UTC | #1
On Mon, Jan 01, 2024 at 05:25:58PM +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> RFC mainly because it's untested. I have none of the relevant hardware and
> haven't yet emulated the firmware descriptions to test this.
> I have tested the device tree only version:
> https://lore.kernel.org/linux-iio/20231217184648.185236-1-jic23@kernel.org/
> which is very similar.
> 
> Failing to release the references on early exit from loops over child nodes
> and similar are a fairly common source of bugs. The need to explicitly
> release the references via fwnode_handle_put() also complicate the code.
> 
> The first patch enables
> 
> 	struct fwnode_handle *child __free(fwnode_handle) = NULL;
> 
> 	device_for_each_child_node(dev, child) {
> 		if (err)
> 			/*
> 			 * Previously needed a fwnode_handle_put() here,
> 			 * will now be called automatically as well leave
> 			 * the scope within which the cleanup is registered
> 			 */
> 			return err;
> 	}
> 
> /*
>  * fwnode_handle_put() no automatically called here - but child == NULL so
>  * that call is a noop
>  */
> }
> 
> As can be seen by the examples from IIO that follow this can save
> a reasonable amount of complexity and boiler plate code, often enabling
> additional cleanups in related code such as use of
> return dev_err_probe().

Important comment on the first patch, otherwise LGTM.