linux-kernel - Re: [PATCH] iio: hi8435: Drop hi8435

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sun, 11 Aug 2019 10:00:22 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     Andrey Smirnov <andrew.smirnov@...il.com>
Cc:     linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
        Chris Healy <cphealy@...il.com>,
        Vladimir Barinov <vladimir.barinov@...entembedded.com>,
        Nikita Yushchenko <nikita.yoush@...entembedded.com>
Subject: Re: [PATCH] iio: hi8435: Drop hi8435_remove()

On Sat, 10 Aug 2019 22:45:45 -0700
Andrey Smirnov <andrew.smirnov@...il.com> wrote:

> Convert the remainder of hi8435_probe() to use devres and get rid of
> hi8435_remove().
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@...il.com>
> Cc: linux-kernel@...r.kernel.org
> Cc: linux-iio@...r.kernel.org
> Cc: Jonathan Cameron <jic23@...nel.org>
> Cc: Chris Healy <cphealy@...il.com>
+CC as before.

I've tweaked things a little bit (see below)

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Still time for others to comment btw!

Thanks,

Jonathan

> ---
>  drivers/iio/adc/hi8435.c | 30 +++++++++++++-----------------
>  1 file changed, 13 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/iio/adc/hi8435.c b/drivers/iio/adc/hi8435.c
> index c15f0e154e4d..fe1341383d30 100644
> --- a/drivers/iio/adc/hi8435.c
> +++ b/drivers/iio/adc/hi8435.c
> @@ -456,6 +456,11 @@ static irqreturn_t hi8435_trigger_handler(int irq, void *private)
>  	return IRQ_HANDLED;
>  }
>  
> +static void hi8435_triggered_event_cleanup(void *data)
> +{
> +	iio_triggered_event_cleanup(data);
> +}

Hmm. I wonder if we should do a full devm_iio_triggered_event_setup()
but given we have very few callers that can be a job for another day.

> +
>  static int hi8435_probe(struct spi_device *spi)
>  {
>  	struct iio_dev *idev;
> @@ -513,26 +518,18 @@ static int hi8435_probe(struct spi_device *spi)
>  	if (ret)
>  		return ret;
>  
> -	ret = iio_device_register(idev);
> +	ret = devm_add_action_or_reset(&spi->dev,
> +				       hi8435_triggered_event_cleanup,
> +				       idev);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_iio_device_register(&spi->dev, idev);
>  	if (ret < 0) {
>  		dev_err(&spi->dev, "unable to register device\n");
> -		goto unregister_triggered_event;
> +		return ret;

We could tidy up a little more and do
	if (ret)
		dev_err...

	return ret;

Or, given all the likely paths in iio_device_register() have 
error prints anyway, can just do
	return devm_iio_device_register();

I'll change it to this last one whilst applying.

>  	}
>  
> -	return 0;
> -
> -unregister_triggered_event:
> -	iio_triggered_event_cleanup(idev);
> -	return ret;
> -}
> -
> -static int hi8435_remove(struct spi_device *spi)
> -{
> -	struct iio_dev *idev = spi_get_drvdata(spi);
> -
> -	iio_device_unregister(idev);
> -	iio_triggered_event_cleanup(idev);
> -
>  	return 0;
>  }
>  
> @@ -554,7 +551,6 @@ static struct spi_driver hi8435_driver = {
>  		.of_match_table	= of_match_ptr(hi8435_dt_ids),
>  	},
>  	.probe		= hi8435_probe,
> -	.remove		= hi8435_remove,
>  	.id_table	= hi8435_id,
>  };
>  module_spi_driver(hi8435_driver);