* [PATCH 1/3] iio: imu: adis: add doc-string for `adis` struct
@ 2019-10-08 8:02 Alexandru Ardelean
2019-10-08 8:02 ` [PATCH 2/3] iio: imu: adis: update `adis_data` struct doc-string Alexandru Ardelean
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Alexandru Ardelean @ 2019-10-08 8:02 UTC (permalink / raw)
To: linux-iio; +Cc: jic23, Alexandru Ardelean
This change adds a doc-string for the `adis` struct. It details the fields
and their roles.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
include/linux/iio/imu/adis.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index 48444f28b003..dbdf421d35dd 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -54,6 +54,20 @@ struct adis_data {
bool has_paging;
};
+/**
+ * struct adis - ADIS device instance data
+ * @spi: Reference to SPI device which owns this ADIS IIO device
+ * @trig: IIO trigger object data
+ * @data: ADIS chip variant specific data
+ * @burst: ADIS burst transfer information
+ * @state_lock: Lock used by the device to protect state
+ * @msg: SPI message object
+ * @xfer: SPI transfer objects to be used for a @msg
+ * @current_page: Some ADIS devices have registers, this selects current page
+ * @buffer: Data buffer for information read from the device
+ * @tx: Cacheline aligned TX buffer for SPI transfers
+ * @rx: Cacheline aligned RX buffer for SPI transfers
+ */
struct adis {
struct spi_device *spi;
struct iio_trigger *trig;
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] iio: imu: adis: update `adis_data` struct doc-string
2019-10-08 8:02 [PATCH 1/3] iio: imu: adis: add doc-string for `adis` struct Alexandru Ardelean
@ 2019-10-08 8:02 ` Alexandru Ardelean
2019-10-08 8:02 ` [PATCH 3/3] iio: imu: adis: add a note better explaining state_lock Alexandru Ardelean
2019-10-12 11:42 ` [PATCH 1/3] iio: imu: adis: add doc-string for `adis` struct Jonathan Cameron
2 siblings, 0 replies; 7+ messages in thread
From: Alexandru Ardelean @ 2019-10-08 8:02 UTC (permalink / raw)
To: linux-iio; +Cc: jic23, Alexandru Ardelean
The doc-string has been neglected over time.
This change updates it with all the missing info.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
include/linux/iio/imu/adis.h | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index dbdf421d35dd..27ebd740f626 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -30,8 +30,13 @@ struct adis_burst;
* @glob_cmd_reg: Register address of the GLOB_CMD register
* @msc_ctrl_reg: Register address of the MSC_CTRL register
* @diag_stat_reg: Register address of the DIAG_STAT register
- * @status_error_msgs: Array of error messgaes
- * @status_error_mask:
+ * @self_test_mask: Bitmask of supported self-test operations
+ * @self_test_no_autoclear: True if device's self-test needs clear of ctrl reg
+ * @startup_delay: Delay in us to be used during ADIS init routines
+ * @status_error_msgs: Array of error message strings
+ * @status_error_mask: Bitmask of errors supported by the device
+ * @enable_irq: Hook for ADIS devices that have a special IRQ enable/disable
+ * @has_paging: True if ADIS device has paged registers
*/
struct adis_data {
unsigned int read_delay;
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] iio: imu: adis: add a note better explaining state_lock
2019-10-08 8:02 [PATCH 1/3] iio: imu: adis: add doc-string for `adis` struct Alexandru Ardelean
2019-10-08 8:02 ` [PATCH 2/3] iio: imu: adis: update `adis_data` struct doc-string Alexandru Ardelean
@ 2019-10-08 8:02 ` Alexandru Ardelean
2019-10-12 11:50 ` Jonathan Cameron
2019-10-12 11:42 ` [PATCH 1/3] iio: imu: adis: add doc-string for `adis` struct Jonathan Cameron
2 siblings, 1 reply; 7+ messages in thread
From: Alexandru Ardelean @ 2019-10-08 8:02 UTC (permalink / raw)
To: linux-iio; +Cc: jic23, Alexandru Ardelean
The `state_lock` mutex was renamed from `txrx_lock` in a previous patch and
is intended to be used by ADIS drivers to protect the state of devices
during consecutive R/W ops.
The initial patch that introduced this change did not do a good [well, any]
job at explaining this. This patch adds a comment to the `state_lock`
better explaining it's use.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
include/linux/iio/imu/adis.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index 27ebd740f626..abd4bd07e960 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -80,6 +80,17 @@ struct adis {
const struct adis_data *data;
struct adis_burst *burst;
+ /**
+ * The state_lock is meant to be used during operations that require
+ * a sequence of SPI R/W in order to protect the SPI transfer
+ * information (fields 'xfer', 'msg' & 'current_page') between
+ * potential concurrent accesses.
+ * This lock is used by all "adis_{functions}" that have to read/write
+ * registers. These functions also have unlocked variants
+ * (see "__adis_{functions}"), which don't hold this lock.
+ * This allows users of the ADIS library to group SPI R/W into
+ * the drivers, but they also must manage this lock themselves.
+ */
struct mutex state_lock;
struct spi_message msg;
struct spi_transfer *xfer;
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] iio: imu: adis: add a note better explaining state_lock
2019-10-08 8:02 ` [PATCH 3/3] iio: imu: adis: add a note better explaining state_lock Alexandru Ardelean
@ 2019-10-12 11:50 ` Jonathan Cameron
0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2019-10-12 11:50 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: linux-iio
On Tue, 8 Oct 2019 11:02:39 +0300
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> The `state_lock` mutex was renamed from `txrx_lock` in a previous patch and
> is intended to be used by ADIS drivers to protect the state of devices
> during consecutive R/W ops.
> The initial patch that introduced this change did not do a good [well, any]
> job at explaining this. This patch adds a comment to the `state_lock`
> better explaining it's use.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Tiny tweak. See below.
Applied to the togreg branch of iio.git and pushed out as testing.
One thing to note though. I don't normally run kernel-doc on random
headers, and my eyes missed it, but there are errors in the docs
for read and write functions that could do with fixing.
[jic23@archlinux iio]$ ./scripts/kernel-doc include/linux/iio/imu/adis.h > temp.html
include/linux/iio/imu/adis.h:138: warning: Function parameter or member 'val' not described in '__adis_write_reg_8'
include/linux/iio/imu/adis.h:138: warning: Excess function parameter 'value' description in '__adis_write_reg_8'
include/linux/iio/imu/adis.h:150: warning: Function parameter or member 'val' not described in '__adis_write_reg_16'
include/linux/iio/imu/adis.h:150: warning: Excess function parameter 'value' description in '__adis_write_reg_16'
include/linux/iio/imu/adis.h:162: warning: Function parameter or member 'val' not described in '__adis_write_reg_32'
include/linux/iio/imu/adis.h:162: warning: Excess function parameter 'value' description in '__adis_write_reg_32'
include/linux/iio/imu/adis.h:217: warning: Function parameter or member 'val' not described in 'adis_write_reg'
include/linux/iio/imu/adis.h:217: warning: Excess function parameter 'value' description in 'adis_write_reg'
include/linux/iio/imu/adis.h:254: warning: Function parameter or member 'val' not described in 'adis_write_reg_8'
include/linux/iio/imu/adis.h:254: warning: Excess function parameter 'value' description in 'adis_write_reg_8'
include/linux/iio/imu/adis.h:266: warning: Function parameter or member 'val' not described in 'adis_write_reg_16'
include/linux/iio/imu/adis.h:266: warning: Excess function parameter 'value' description in 'adis_write_reg_16'
include/linux/iio/imu/adis.h:278: warning: Function parameter or member 'val' not described in 'adis_write_reg_32'
include/linux/iio/imu/adis.h:278: warning: Excess function parameter 'value' description in 'adis_write_reg_32'
If you could role a patch for those, that would be great. Thanks!
Thanks,
Jonathan
> ---
> include/linux/iio/imu/adis.h | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
> index 27ebd740f626..abd4bd07e960 100644
> --- a/include/linux/iio/imu/adis.h
> +++ b/include/linux/iio/imu/adis.h
> @@ -80,6 +80,17 @@ struct adis {
> const struct adis_data *data;
> struct adis_burst *burst;
>
> + /**
This isn't kernel doc so
/*
I'll fix.
> + * The state_lock is meant to be used during operations that require
> + * a sequence of SPI R/W in order to protect the SPI transfer
> + * information (fields 'xfer', 'msg' & 'current_page') between
> + * potential concurrent accesses.
> + * This lock is used by all "adis_{functions}" that have to read/write
> + * registers. These functions also have unlocked variants
> + * (see "__adis_{functions}"), which don't hold this lock.
> + * This allows users of the ADIS library to group SPI R/W into
> + * the drivers, but they also must manage this lock themselves.
> + */
> struct mutex state_lock;
> struct spi_message msg;
> struct spi_transfer *xfer;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] iio: imu: adis: add doc-string for `adis` struct
2019-10-08 8:02 [PATCH 1/3] iio: imu: adis: add doc-string for `adis` struct Alexandru Ardelean
2019-10-08 8:02 ` [PATCH 2/3] iio: imu: adis: update `adis_data` struct doc-string Alexandru Ardelean
2019-10-08 8:02 ` [PATCH 3/3] iio: imu: adis: add a note better explaining state_lock Alexandru Ardelean
@ 2019-10-12 11:42 ` Jonathan Cameron
2 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2019-10-12 11:42 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: linux-iio
On Tue, 8 Oct 2019 11:02:37 +0300
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> This change adds a doc-string for the `adis` struct. It details the fields
> and their roles.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
> include/linux/iio/imu/adis.h | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
> index 48444f28b003..dbdf421d35dd 100644
> --- a/include/linux/iio/imu/adis.h
> +++ b/include/linux/iio/imu/adis.h
> @@ -54,6 +54,20 @@ struct adis_data {
> bool has_paging;
> };
>
> +/**
> + * struct adis - ADIS device instance data
> + * @spi: Reference to SPI device which owns this ADIS IIO device
> + * @trig: IIO trigger object data
> + * @data: ADIS chip variant specific data
> + * @burst: ADIS burst transfer information
> + * @state_lock: Lock used by the device to protect state
> + * @msg: SPI message object
> + * @xfer: SPI transfer objects to be used for a @msg
> + * @current_page: Some ADIS devices have registers, this selects current page
> + * @buffer: Data buffer for information read from the device
> + * @tx: Cacheline aligned TX buffer for SPI transfers
> + * @rx: Cacheline aligned RX buffer for SPI transfers
That one is ever so slightly untrue. It's in the same cacheline as the
tx and so safe (device transfers can only cause trouble for themselves).
I'll tweak to just...
RX buffer for SPI transfers. May share cacheline with tx.
Hope you don't mind as don't want to waste your time on a v2 for just that.
Applied to the togreg branch of iio.git and pushed out as testing for the
autobuilders to play with it.
Thanks,
Jonathan
> + */
> struct adis {
> struct spi_device *spi;
> struct iio_trigger *trig;
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] iio: imu: adis: add doc-string for 'adis' struct
@ 2020-02-21 11:49 Alexandru Ardelean
2020-02-21 11:49 ` [PATCH 3/3] iio: imu: adis: add a note better explaining state_lock Alexandru Ardelean
0 siblings, 1 reply; 7+ messages in thread
From: Alexandru Ardelean @ 2020-02-21 11:49 UTC (permalink / raw)
To: linux-iio, linux-kernel; +Cc: jic23, Alexandru Ardelean
This change adds a doc-string for the 'adis' struct. It details the fields
and their roles.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
include/linux/iio/imu/adis.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index ac7cfd073804..0787a3aabd05 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -73,6 +73,20 @@ struct adis_data {
bool has_paging;
};
+/**
+ * struct adis - ADIS device instance data
+ * @spi: Reference to SPI device which owns this ADIS IIO device
+ * @trig: IIO trigger object data
+ * @data: ADIS chip variant specific data
+ * @burst: ADIS burst transfer information
+ * @state_lock: Lock used by the device to protect state
+ * @msg: SPI message object
+ * @xfer: SPI transfer objects to be used for a @msg
+ * @current_page: Some ADIS devices have registers, this selects current page
+ * @buffer: Data buffer for information read from the device
+ * @tx: Cacheline aligned TX buffer for SPI transfers
+ * @rx: Cacheline aligned RX buffer for SPI transfers
+ */
struct adis {
struct spi_device *spi;
struct iio_trigger *trig;
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] iio: imu: adis: add a note better explaining state_lock
2020-02-21 11:49 [PATCH 1/3] iio: imu: adis: add doc-string for 'adis' struct Alexandru Ardelean
@ 2020-02-21 11:49 ` Alexandru Ardelean
2020-02-21 13:14 ` Jonathan Cameron
0 siblings, 1 reply; 7+ messages in thread
From: Alexandru Ardelean @ 2020-02-21 11:49 UTC (permalink / raw)
To: linux-iio, linux-kernel; +Cc: jic23, Alexandru Ardelean
The 'state_lock' mutex was renamed from 'txrx_lock' in a previous patch and
is intended to be used by ADIS drivers to protect the state of devices
during consecutive R/W ops.
The initial patch that introduced this change did not do a good [well, any]
job at explaining this. This patch adds a comment to the 'state_lock'
better explaining it's use.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
include/linux/iio/imu/adis.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index 11c915b61ae2..a3110c097fa1 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -98,6 +98,17 @@ struct adis {
const struct adis_data *data;
struct adis_burst *burst;
+ /**
+ * The state_lock is meant to be used during operations that require
+ * a sequence of SPI R/W in order to protect the SPI transfer
+ * information (fields 'xfer', 'msg' & 'current_page') between
+ * potential concurrent accesses.
+ * This lock is used by all "adis_{functions}" that have to read/write
+ * registers. These functions also have unlocked variants
+ * (see "__adis_{functions}"), which don't hold this lock.
+ * This allows users of the ADIS library to group SPI R/W into
+ * the drivers, but they also must manage this lock themselves.
+ */
struct mutex state_lock;
struct spi_message msg;
struct spi_transfer *xfer;
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] iio: imu: adis: add a note better explaining state_lock
2020-02-21 11:49 ` [PATCH 3/3] iio: imu: adis: add a note better explaining state_lock Alexandru Ardelean
@ 2020-02-21 13:14 ` Jonathan Cameron
0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2020-02-21 13:14 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel
On Fri, 21 Feb 2020 13:49:43 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> The 'state_lock' mutex was renamed from 'txrx_lock' in a previous patch and
> is intended to be used by ADIS drivers to protect the state of devices
> during consecutive R/W ops.
> The initial patch that introduced this change did not do a good [well, any]
> job at explaining this. This patch adds a comment to the 'state_lock'
> better explaining it's use.
Nice work.
Will pick up in v2.
Thanks,
Jonathan
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
> include/linux/iio/imu/adis.h | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
> index 11c915b61ae2..a3110c097fa1 100644
> --- a/include/linux/iio/imu/adis.h
> +++ b/include/linux/iio/imu/adis.h
> @@ -98,6 +98,17 @@ struct adis {
> const struct adis_data *data;
> struct adis_burst *burst;
>
> + /**
> + * The state_lock is meant to be used during operations that require
> + * a sequence of SPI R/W in order to protect the SPI transfer
> + * information (fields 'xfer', 'msg' & 'current_page') between
> + * potential concurrent accesses.
> + * This lock is used by all "adis_{functions}" that have to read/write
> + * registers. These functions also have unlocked variants
> + * (see "__adis_{functions}"), which don't hold this lock.
> + * This allows users of the ADIS library to group SPI R/W into
> + * the drivers, but they also must manage this lock themselves.
> + */
> struct mutex state_lock;
> struct spi_message msg;
> struct spi_transfer *xfer;
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-02-21 13:14 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-08 8:02 [PATCH 1/3] iio: imu: adis: add doc-string for `adis` struct Alexandru Ardelean
2019-10-08 8:02 ` [PATCH 2/3] iio: imu: adis: update `adis_data` struct doc-string Alexandru Ardelean
2019-10-08 8:02 ` [PATCH 3/3] iio: imu: adis: add a note better explaining state_lock Alexandru Ardelean
2019-10-12 11:50 ` Jonathan Cameron
2019-10-12 11:42 ` [PATCH 1/3] iio: imu: adis: add doc-string for `adis` struct Jonathan Cameron
2020-02-21 11:49 [PATCH 1/3] iio: imu: adis: add doc-string for 'adis' struct Alexandru Ardelean
2020-02-21 11:49 ` [PATCH 3/3] iio: imu: adis: add a note better explaining state_lock Alexandru Ardelean
2020-02-21 13:14 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).