Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 19 Aug 2022 18:44:59 +0100
From:   Jonathan Cameron <jic23@kernel.org>
To:     Artur Rojek <contact@artur-rojek.eu>
Cc:     Paul Cercueil <paul@crapouillou.net>,
        Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        Chris Morgan <macromorgan@hotmail.com>,
        linux-mips@vger.kernel.org, linux-iio@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-input@vger.kernel.org
Subject: Re: [PATCH 2/4] iio: add iio_channel_cb_get_iio_buffer helper
Message-ID: <20220819184459.34a12f3f@jic23-huawei>
In-Reply-To: <20220817105643.95710-3-contact@artur-rojek.eu>
References: <20220817105643.95710-1-contact@artur-rojek.eu>
        <20220817105643.95710-3-contact@artur-rojek.eu>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Wed, 17 Aug 2022 12:56:41 +0200
Artur Rojek <contact@artur-rojek.eu> wrote:

> Introduce a helper function to retrieve an iio_buffer from
> iio_cb_buffer.
> 
> This is useful for consumers that need to extract metadata about
> the buffer, e.g. get the channel offsets.
> 
> Tested-by: Paul Cercueil <paul@crapouillou.net>
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>

Hmm. I'm not keen on breaking this boundary between
exposed interface and implementation like this.

The intent was always that the consumer knew what it was
requesting and had access to all the channel information
so should know what the buffer alignment is.

In this driver there is a call to devm_iio_channel_get_all()
which returns the channels.

The buffer offsets can be calculated from that information
as the alignement in a buffer a consumer sees is entirely
controlled by that information.

It might be helpful to provide some helper functions to allow
the consumer to establish where particular channels are though.
(which will look very like what userspace code has to do as the
information available is much the same).

Perhaps I'm missing some information that is missing from what
is exposed to consumers?

Jonathan


> ---
>  drivers/iio/buffer/industrialio-buffer-cb.c |  7 +++++++
>  include/linux/iio/consumer.h                | 12 ++++++++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/iio/buffer/industrialio-buffer-cb.c b/drivers/iio/buffer/industrialio-buffer-cb.c
> index 4c12b7a94af5..47d6e28b4d36 100644
> --- a/drivers/iio/buffer/industrialio-buffer-cb.c
> +++ b/drivers/iio/buffer/industrialio-buffer-cb.c
> @@ -151,6 +151,13 @@ struct iio_dev
>  }
>  EXPORT_SYMBOL_GPL(iio_channel_cb_get_iio_dev);
>  
> +struct iio_buffer
> +*iio_channel_cb_get_iio_buffer(struct iio_cb_buffer *cb_buffer)
> +{
> +	return &cb_buffer->buffer;
> +}
> +EXPORT_SYMBOL_GPL(iio_channel_cb_get_iio_buffer);
> +
>  MODULE_AUTHOR("Jonathan Cameron <jic23@kernel.org>");
>  MODULE_DESCRIPTION("Industrial I/O callback buffer");
>  MODULE_LICENSE("GPL");
> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
> index 6802596b017c..c28925d5b69c 100644
> --- a/include/linux/iio/consumer.h
> +++ b/include/linux/iio/consumer.h
> @@ -196,6 +196,18 @@ struct iio_channel
>  struct iio_dev
>  *iio_channel_cb_get_iio_dev(const struct iio_cb_buffer *cb_buffer);
>  
> +/**
> + * iio_channel_cb_get_iio_buffer() - get access to the underlying buffer.
> + * @cb_buffer:		The callback buffer from whom we want the buffer
> + *			information.
> + *
> + * This function allows one to obtain information about the buffer.
> + * The primary aim is to allow drivers that are consuming a buffer to query
> + * things like channel offsets in the buffer.
> + */
> +struct iio_buffer
> +*iio_channel_cb_get_iio_buffer(struct iio_cb_buffer *cb_buffer);
> +
>  /**
>   * iio_read_channel_raw() - read from a given channel
>   * @chan:		The channel being queried.