All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Eugen Hristev <eugen.hristev@microchip.com>
Cc: nicolas.ferre@microchip.com, ludovic.desroches@microchip.com,
	alexandre.belloni@free-electrons.com, linux-iio@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-input@vger.kernel.org,
	dmitry.torokhov@gmail.com
Subject: Re: [PATCH 07/14] iio: triggers: on pollfunc attach, complete iio_dev if NULL
Date: Fri, 29 Dec 2017 17:23:37 +0000	[thread overview]
Message-ID: <20171229172337.6226de69@archlinux> (raw)
In-Reply-To: <1513955241-10985-8-git-send-email-eugen.hristev@microchip.com>

On Fri, 22 Dec 2017 17:07:14 +0200
Eugen Hristev <eugen.hristev@microchip.com> wrote:

> When attaching a pollfunc to a trigger, if the pollfunc does not
> have an associated iio_dev pointer, just use the private data
> iio_dev pointer from the trigger to fill in the poll func required
> iio_dev reference.
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>

I'm yet to be convinced this is necessary rather than using a callback
buffer. It's also decidedly unsafe as there is no particular reason
in general to assume the private data is an iio_dev.

> ---
>  drivers/iio/industrialio-trigger.c   | 9 +++++++++
>  include/linux/iio/trigger_consumer.h | 2 ++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
> index 8565c92..ab180bd 100644
> --- a/drivers/iio/industrialio-trigger.c
> +++ b/drivers/iio/industrialio-trigger.c
> @@ -272,6 +272,15 @@ int iio_trigger_attach_poll_func(struct iio_trigger *trig,
>  	bool notinuse
>  		= bitmap_empty(trig->pool, CONFIG_IIO_CONSUMERS_PER_TRIGGER);
>  
> +	/*
> +	 * If we did not get a iio_dev in the poll func, attempt to
> +	 * obtain the trigger's owner's device struct
> +	 */
> +	if (!pf->indio_dev)
> +		pf->indio_dev = iio_trigger_get_drvdata(trig);

That isn't always valid. Triggers don't always have a iio_dev associated
with them at all.
> +	if (!pf->indio_dev)
> +		return -EINVAL;
> +
>  	/* Prevent the module from being removed whilst attached to a trigger */
>  	__module_get(pf->indio_dev->driver_module);
>  
> diff --git a/include/linux/iio/trigger_consumer.h b/include/linux/iio/trigger_consumer.h
> index aeefcdb..36e2a02 100644
> --- a/include/linux/iio/trigger_consumer.h
> +++ b/include/linux/iio/trigger_consumer.h
> @@ -63,6 +63,8 @@ int iio_triggered_buffer_predisable(struct iio_dev *indio_dev);
>  /*
>   * Two functions for the uncommon case when we need to attach or detach
>   * a specific pollfunc to and from a trigger
> + * If the pollfunc has a NULL iio_dev pointer, it will be filled from the
> + * trigger struct.
>   */
>  int iio_trigger_attach_poll_func(struct iio_trigger *trig,
>  				 struct iio_poll_func *pf);


WARNING: multiple messages have this Message-ID (diff)
From: Jonathan Cameron <jic23@kernel.org>
To: Eugen Hristev <eugen.hristev@microchip.com>
Cc: <nicolas.ferre@microchip.com>, <ludovic.desroches@microchip.com>,
	<alexandre.belloni@free-electrons.com>,
	<linux-iio@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-input@vger.kernel.org>, <dmitry.torokhov@gmail.com>
Subject: Re: [PATCH 07/14] iio: triggers: on pollfunc attach, complete iio_dev if NULL
Date: Fri, 29 Dec 2017 17:23:37 +0000	[thread overview]
Message-ID: <20171229172337.6226de69@archlinux> (raw)
In-Reply-To: <1513955241-10985-8-git-send-email-eugen.hristev@microchip.com>

On Fri, 22 Dec 2017 17:07:14 +0200
Eugen Hristev <eugen.hristev@microchip.com> wrote:

> When attaching a pollfunc to a trigger, if the pollfunc does not
> have an associated iio_dev pointer, just use the private data
> iio_dev pointer from the trigger to fill in the poll func required
> iio_dev reference.
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>

I'm yet to be convinced this is necessary rather than using a callback
buffer. It's also decidedly unsafe as there is no particular reason
in general to assume the private data is an iio_dev.

> ---
>  drivers/iio/industrialio-trigger.c   | 9 +++++++++
>  include/linux/iio/trigger_consumer.h | 2 ++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
> index 8565c92..ab180bd 100644
> --- a/drivers/iio/industrialio-trigger.c
> +++ b/drivers/iio/industrialio-trigger.c
> @@ -272,6 +272,15 @@ int iio_trigger_attach_poll_func(struct iio_trigger *trig,
>  	bool notinuse
>  		= bitmap_empty(trig->pool, CONFIG_IIO_CONSUMERS_PER_TRIGGER);
>  
> +	/*
> +	 * If we did not get a iio_dev in the poll func, attempt to
> +	 * obtain the trigger's owner's device struct
> +	 */
> +	if (!pf->indio_dev)
> +		pf->indio_dev = iio_trigger_get_drvdata(trig);

That isn't always valid. Triggers don't always have a iio_dev associated
with them at all.
> +	if (!pf->indio_dev)
> +		return -EINVAL;
> +
>  	/* Prevent the module from being removed whilst attached to a trigger */
>  	__module_get(pf->indio_dev->driver_module);
>  
> diff --git a/include/linux/iio/trigger_consumer.h b/include/linux/iio/trigger_consumer.h
> index aeefcdb..36e2a02 100644
> --- a/include/linux/iio/trigger_consumer.h
> +++ b/include/linux/iio/trigger_consumer.h
> @@ -63,6 +63,8 @@ int iio_triggered_buffer_predisable(struct iio_dev *indio_dev);
>  /*
>   * Two functions for the uncommon case when we need to attach or detach
>   * a specific pollfunc to and from a trigger
> + * If the pollfunc has a NULL iio_dev pointer, it will be filled from the
> + * trigger struct.
>   */
>  int iio_trigger_attach_poll_func(struct iio_trigger *trig,
>  				 struct iio_poll_func *pf);


WARNING: multiple messages have this Message-ID (diff)
From: jic23@kernel.org (Jonathan Cameron)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 07/14] iio: triggers: on pollfunc attach, complete iio_dev if NULL
Date: Fri, 29 Dec 2017 17:23:37 +0000	[thread overview]
Message-ID: <20171229172337.6226de69@archlinux> (raw)
In-Reply-To: <1513955241-10985-8-git-send-email-eugen.hristev@microchip.com>

On Fri, 22 Dec 2017 17:07:14 +0200
Eugen Hristev <eugen.hristev@microchip.com> wrote:

> When attaching a pollfunc to a trigger, if the pollfunc does not
> have an associated iio_dev pointer, just use the private data
> iio_dev pointer from the trigger to fill in the poll func required
> iio_dev reference.
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>

I'm yet to be convinced this is necessary rather than using a callback
buffer. It's also decidedly unsafe as there is no particular reason
in general to assume the private data is an iio_dev.

> ---
>  drivers/iio/industrialio-trigger.c   | 9 +++++++++
>  include/linux/iio/trigger_consumer.h | 2 ++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
> index 8565c92..ab180bd 100644
> --- a/drivers/iio/industrialio-trigger.c
> +++ b/drivers/iio/industrialio-trigger.c
> @@ -272,6 +272,15 @@ int iio_trigger_attach_poll_func(struct iio_trigger *trig,
>  	bool notinuse
>  		= bitmap_empty(trig->pool, CONFIG_IIO_CONSUMERS_PER_TRIGGER);
>  
> +	/*
> +	 * If we did not get a iio_dev in the poll func, attempt to
> +	 * obtain the trigger's owner's device struct
> +	 */
> +	if (!pf->indio_dev)
> +		pf->indio_dev = iio_trigger_get_drvdata(trig);

That isn't always valid. Triggers don't always have a iio_dev associated
with them at all.
> +	if (!pf->indio_dev)
> +		return -EINVAL;
> +
>  	/* Prevent the module from being removed whilst attached to a trigger */
>  	__module_get(pf->indio_dev->driver_module);
>  
> diff --git a/include/linux/iio/trigger_consumer.h b/include/linux/iio/trigger_consumer.h
> index aeefcdb..36e2a02 100644
> --- a/include/linux/iio/trigger_consumer.h
> +++ b/include/linux/iio/trigger_consumer.h
> @@ -63,6 +63,8 @@ int iio_triggered_buffer_predisable(struct iio_dev *indio_dev);
>  /*
>   * Two functions for the uncommon case when we need to attach or detach
>   * a specific pollfunc to and from a trigger
> + * If the pollfunc has a NULL iio_dev pointer, it will be filled from the
> + * trigger struct.
>   */
>  int iio_trigger_attach_poll_func(struct iio_trigger *trig,
>  				 struct iio_poll_func *pf);

  reply	other threads:[~2017-12-29 17:23 UTC|newest]

Thread overview: 92+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-22 15:07 [PATCH 00/14] iio: triggers: add consumer support Eugen Hristev
2017-12-22 15:07 ` Eugen Hristev
2017-12-22 15:07 ` Eugen Hristev
2017-12-22 15:07 ` [PATCH 01/14] MAINTAINERS: add sama5d2 resistive touchscreen Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
2017-12-22 15:07 ` [PATCH 02/14] iio: Add channel for Position Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
     [not found]   ` <1513955241-10985-3-git-send-email-eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
2017-12-29 16:09     ` Jonathan Cameron
2017-12-29 16:09       ` Jonathan Cameron
2017-12-29 16:09       ` Jonathan Cameron
2017-12-22 15:07 ` [PATCH 03/14] dt-bindings: iio: add binding support for iio trigger provider/consumer Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
     [not found]   ` <1513955241-10985-4-git-send-email-eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
2017-12-26 22:35     ` Rob Herring
2017-12-26 22:35       ` Rob Herring
2017-12-26 22:35       ` Rob Herring
2017-12-29 17:24       ` Jonathan Cameron
2017-12-29 17:24         ` Jonathan Cameron
2017-12-22 15:07 ` [PATCH 04/14] dt-bindings: input: touchscreen: sama5d2_rts: create bindings Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
2017-12-22 15:07 ` [PATCH 05/14] iio: triggers: add helper function to retrieve trigger Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
2017-12-22 15:07 ` [PATCH 06/14] iio: triggers: expose attach and detach helpers for pollfuncs Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
2017-12-22 15:07 ` [PATCH 07/14] iio: triggers: on pollfunc attach, complete iio_dev if NULL Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
2017-12-29 17:23   ` Jonathan Cameron [this message]
2017-12-29 17:23     ` Jonathan Cameron
2017-12-29 17:23     ` Jonathan Cameron
2017-12-22 15:07 ` [PATCH 08/14] iio: triggers: add private data to pollfuncs Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
2017-12-22 15:07 ` [PATCH 09/14] iio: inkern: triggers: create helpers for OF trigger retrieval Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
     [not found]   ` <1513955241-10985-10-git-send-email-eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
2017-12-29 17:20     ` Jonathan Cameron
2017-12-29 17:20       ` Jonathan Cameron
2017-12-29 17:20       ` Jonathan Cameron
     [not found] ` <1513955241-10985-1-git-send-email-eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
2017-12-22 15:07   ` [PATCH 10/14] iio: adc: at91-sama5d2_adc: force trigger removal on module remove Eugen Hristev
2017-12-22 15:07     ` Eugen Hristev
2017-12-22 15:07     ` Eugen Hristev
     [not found]     ` <1513955241-10985-11-git-send-email-eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
2017-12-29 16:22       ` Jonathan Cameron
2017-12-29 16:22         ` Jonathan Cameron
2017-12-29 16:22         ` Jonathan Cameron
2017-12-22 15:07 ` [PATCH 11/14] iio: adc: at91-sama5d2_adc: optimize scan index for diff channels Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
     [not found]   ` <1513955241-10985-12-git-send-email-eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
2017-12-29 16:24     ` Jonathan Cameron
2017-12-29 16:24       ` Jonathan Cameron
2017-12-29 16:24       ` Jonathan Cameron
2017-12-22 15:07 ` [PATCH 12/14] iio: adc: at91-sama5d2_adc: support for position and pressure channels Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
     [not found]   ` <1513955241-10985-13-git-send-email-eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
2017-12-29 17:02     ` Jonathan Cameron
2017-12-29 17:02       ` Jonathan Cameron
2017-12-29 17:02       ` Jonathan Cameron
2018-01-04 15:17       ` Eugen Hristev
2018-01-04 15:17         ` Eugen Hristev
2018-01-04 15:17         ` Eugen Hristev
2018-01-06 15:05         ` Jonathan Cameron
2018-01-06 15:05           ` Jonathan Cameron
2018-01-06 15:05           ` Jonathan Cameron
2018-01-08 14:12           ` Ludovic Desroches
2018-01-08 14:12             ` Ludovic Desroches
2018-01-08 14:12             ` Ludovic Desroches
2018-01-14 10:47             ` Jonathan Cameron
2018-01-14 10:47               ` Jonathan Cameron
2018-01-14 10:47               ` Jonathan Cameron
2017-12-22 15:07 ` [PATCH 13/14] input: touchscreen: sama5d2_rts: SAMA5D2 Resistive touchscreen driver Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
     [not found]   ` <1513955241-10985-14-git-send-email-eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
2017-12-22 16:29     ` Philippe Ombredanne
2017-12-22 16:29       ` Philippe Ombredanne
2017-12-22 16:29       ` Philippe Ombredanne
2017-12-26 22:41     ` Rob Herring
2017-12-26 22:41       ` Rob Herring
2017-12-26 22:41       ` Rob Herring
2017-12-29 17:16   ` Jonathan Cameron
2017-12-29 17:16     ` Jonathan Cameron
2017-12-29 17:16     ` Jonathan Cameron
2017-12-22 15:07 ` [PATCH 14/14] ARM: dts: at91: sama5d2: Add resistive touch device Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
2017-12-22 15:07   ` Eugen Hristev
     [not found]   ` <1513955241-10985-15-git-send-email-eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
2017-12-22 16:10     ` Alexandre Belloni
2017-12-22 16:10       ` Alexandre Belloni
2017-12-22 16:10       ` Alexandre Belloni

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=20171229172337.6226de69@archlinux \
    --to=jic23@kernel.org \
    --cc=alexandre.belloni@free-electrons.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=eugen.hristev@microchip.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ludovic.desroches@microchip.com \
    --cc=nicolas.ferre@microchip.com \
    /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.