linux-kernel - Re: [PATCH] iio: adc: Nothing in ADC should be a bool CONFIG

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <5358288C.6030205@kernel.org>
Date:	Wed, 23 Apr 2014 21:54:36 +0100
From:	Jonathan Cameron <jic23@...nel.org>
To:	Doug Anderson <dianders@...omium.org>,
	Lars-Peter Clausen <lars@...afoo.de>
CC:	Milo.Kim@...com, Naveen Krishna Chatradhi <ch.naveen@...sung.com>,
	lee.jones@...aro.org, gregkh@...uxfoundation.org, Russ.Dill@...com,
	alexandre.belloni@...e-electrons.com, oleksandr.kozaruk@...com,
	B38611@...escale.com, johannes.thumshirn@....de, sre@...ian.org,
	sachin.kamat@...aro.org, linux-iio@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iio: adc: Nothing in ADC should be a bool CONFIG

On 22/04/14 01:03, Doug Anderson wrote:
> The whole IIO subsystem can be moved to a module.  If you make it a
> module then stuff marked as "Y" in the adc directory simply won't be
> linked in properly.
>
> The two configs that were wrong were EXYNOS_ADC and LP8788_ADC.  I
> know for a fact that EXYNOS_ADC will work as a module (though it
> appears to crash when you unload it--that needs to be addressed
> separately).
I'd really like to see this pinned down before taking this patch.
I can see you argument that the current approach is clearly wrong,
but swapping one issue for another is not an approach I'd particularly
like to take...

I can't immediately spot the cause of the crash, but there are certainly
some interesting order issues in this driver.  Not enabling the vdd
regulator until after the userspace interfaces are exposed (by the
iio_device_register call) is interesting for a start.

The remove doesn't run in the reverse of the probe order (see clocks
vs regulators for example.)

Gah, my reviewing for one clearly missed some things in this driver.

> I assume LP8788_ADC will also be fine..
>
> Signed-off-by: Doug Anderson <dianders@...omium.org>
> ---
>   drivers/iio/adc/Kconfig | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index d86196c..24c28e3 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -106,7 +106,7 @@ config AT91_ADC
>   	  Say yes here to build support for Atmel AT91 ADC.
>
>   config EXYNOS_ADC
> -	bool "Exynos ADC driver support"
> +	tristate "Exynos ADC driver support"
>   	depends on OF
>   	help
>   	  Core support for the ADC block found in the Samsung EXYNOS series
> @@ -114,7 +114,7 @@ config EXYNOS_ADC
>   	  this resource.
>
>   config LP8788_ADC
> -	bool "LP8788 ADC driver"
> +	tristate "LP8788 ADC driver"
>   	depends on MFD_LP8788
>   	help
>   	  Say yes here to build support for TI LP8788 ADC.
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/