Re: [PATCH v3] iio: trigger: warn about non-registered iio trigger getting attempt

[Dmitry Rokosov]

Hello Jonathan,

I notice the patchset from 
https://lore.kernel.org/all/20220524181150.9240-1-ddrokosov@xxxxxxxxxxxxxx/
is not merged to stable yet.
I think if this WARN() patch is okay for you, maybe it's better to merge
it together with the previous one. It will notify developers about this
problem as you suggested before, and the previous patchset resolves the issue
in the all IIO drivers.

What do you think about it?

On Tue, Jun 07, 2022 at 06:39:18PM +0000, Dmitry Rokosov wrote:
> As a part of patch series about wrong trigger register() and get()
> calls order in the some IIO drivers trigger initialization path:
> 
> https://lore.kernel.org/all/20220524181150.9240-1-ddrokosov@xxxxxxxxxxxxxx/
> 
> runtime WARN_ONCE() is added to alarm IIO driver authors who make such
> a mistake.
> 
> When an IIO driver allocates a new IIO trigger, it should register it
> before calling the get() operation. In other words, each IIO driver
> must abide by IIO trigger alloc()/register()/get() calls order.
> 
> Signed-off-by: Dmitry Rokosov <ddrokosov@xxxxxxxxxxxxxx>
> ---
> Changes:
> v1 -> v2: totally reworked the patch, used trig->list entry instead of
>           trig->owner as driver registration indicator.
>           It works perfectly for both builtin and built as a module
>           drivers.
> 
> v2 -> v3: changed WARN() call to WARN_ONCE() to avoid warn spamming
>           during deferred probe() as Andy suggested.
> ---
>  drivers/iio/industrialio-trigger.c | 2 ++
>  include/linux/iio/trigger.h        | 5 +++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
> index f504ed351b3e..d6277e72d515 100644
> --- a/drivers/iio/industrialio-trigger.c
> +++ b/drivers/iio/industrialio-trigger.c
> @@ -581,6 +581,8 @@ struct iio_trigger *viio_trigger_alloc(struct device *parent,
>  	if (trig->name == NULL)
>  		goto free_descs;
>  
> +	INIT_LIST_HEAD(&trig->list);
> +
>  	trig->subirq_chip.name = trig->name;
>  	trig->subirq_chip.irq_mask = &iio_trig_subirqmask;
>  	trig->subirq_chip.irq_unmask = &iio_trig_subirqunmask;
> diff --git a/include/linux/iio/trigger.h b/include/linux/iio/trigger.h
> index 4c69b144677b..03b1d6863436 100644
> --- a/include/linux/iio/trigger.h
> +++ b/include/linux/iio/trigger.h
> @@ -93,6 +93,11 @@ static inline void iio_trigger_put(struct iio_trigger *trig)
>  static inline struct iio_trigger *iio_trigger_get(struct iio_trigger *trig)
>  {
>  	get_device(&trig->dev);
> +
> +	WARN_ONCE(list_empty(&trig->list),
> +		  "Getting non-registered iio trigger %s is prohibited\n",
> +		  trig->name);
> +
>  	__module_get(trig->owner);
>  
>  	return trig;
> -- 
> 2.36.0

-- 
Thank you,
Dmitry