Skip to contentSkip to conversation

Support ROHM BM1390 pressure sensor

PATCH Applied Matti Vaittinen 3 participants 12 messages Sep 27, 2023 – Oct 2, 2023

Subsystems (3)

v4 of 6 Applied Latest is v6 →

lore
Share & tools
lore

Matti Vaittinen

author maintainer
#0 lore raw

v4 cover Support ROHM BM1390 pressure sensor

ROHM BM1390 Pressure sensor (BM1390GLV-Z) can measure pressures ranging from 300 hPa to 1300 hPa with configurable measurement averaging and an internal FIFO. The sensor does also provide temperature measurements although, according to the data sheet, sensor performs internal temperature compensation for the MEMS.

Sensor does also contain IIR filter implemented in HW. The data-sheet says the IIR filter can be configured to be "weak", "middle" or "strong". Some RMS noise figures are provided in data sheet but no accurate maths for the filter configurations is provided.

I actually asked if we can define 3db frequencies corresponding to these IIR filter settings - and I received values 0.452Hz, 0.167Hz, and 0.047Hz but I am not at all sure we understood each others with the HW colleagues... Hence, the IIR filter configuration is not supported by this driver and the filter is just configured to the "middle" setting. (at least for now)

It would also be possible to not use IIR filter but just do some simple averaging. I wonder if it would make sense to implement the OVERSAMPLING value setting so that if this value is written, IIR filter is disabled and number of samples to be averaged is set to value requested by OVERSAMPLING. The data-sheet has a mention that if IIR is used, the number of averaged samples must be set to a fixed value.

The FIFO measurement mode (in sensor hardware) is only measuring the pressure and not the temperature. The driver measures temperature when FIFO is flushed and simply uses the same measured temperature value to all reported temperatures. This should not be a problem when temperature is not changing very rapidly (several degrees C / second) but allows users to get the temperature measurements from sensor without any additional logic.

Revision history:
Major changes here, please see the head room of individual patches for more detailed list.
v3 => v4:
rebased back on v6.6-rc1
dropped patch implementing the exact match search for available_scan_mask
tools: iio_generic_buffer: comment on aligning logic bm1390 driver:
- cleanups and fixes
- own info struct for case where IRQ is omitted and FIFO not supported
- fix support for using other triggers. (not really tested but should work)

v2 => v3:
rebased on v6.6-rc2
added three IIO fixup patches so numbering of patches changed dt-bindings/MAINTAINERS: No changes
bm1390 driver:
- various cleanups and fixes
- do not disable IRQ
- fix temperature reading when FIFO is used
- separate buffer and trigger initialization

v1 => v2:
rebased on v6.6-rc1
dt-bindings:
- fix compatible in the example
sensor driver:
- drop unnecessary write_raw callback
- plenty of small improvements and fixes
MAINTAINERS:
- No changes

Matti Vaittinen (5):
tools: iio: iio_generic_buffer ensure alignment
iio: improve doc for available_scan_mask
dt-bindings: Add ROHM BM1390 pressure sensor
iio: pressure: Support ROHM BU1390
MAINTAINERS: Add ROHM BM1390

 .../bindings/iio/pressure/rohm,bm1390.yaml    |  52 +
 MAINTAINERS                                   |   6 +
 drivers/iio/pressure/Kconfig                  |   9 +
 drivers/iio/pressure/Makefile                 |   1 +
 drivers/iio/pressure/rohm-bm1390.c            | 934 ++++++++++++++++++
 include/linux/iio/iio.h                       |   4 +-
 tools/iio/iio_generic_buffer.c                |  18 +-
 7 files changed, 1022 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/pressure/rohm,bm1390.yaml
 create mode 100644 drivers/iio/pressure/rohm-bm1390.c

base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d

Signature · 14 lines
2.41.0


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 
author maintainer
#1 lore raw

PATCH v4 1/5 tools: iio: iio_generic_buffer ensure alignment

The iio_generic_buffer can return garbage values when the total size of scan data is not a multiple of the largest element in the scan. This can be demonstrated by reading a scan, consisting, for example of one 4-byte and one 2-byte element, where the 4-byte element is first in the buffer.

The IIO generic buffer code does not take into account the last two padding bytes that are needed to ensure that the 4-byte data for next scan is correctly aligned.

Add the padding bytes required to align the next sample with the scan size.

---
I think the whole alignment code could be revised here, but I am unsure what kind of alignment is expected, and if it actually depends on the architecture. Anyways, I'll quote myself from another mail to explain how this patch handles things:

Quoted 7 lines

For non power of2 sizes, the alignment code will result strange alignments. For example, scan consisting of two 6-byte elements would be packed - meaning the second element would probably break the alignment rules by starting from address '6'. I think that on most architectures the proper access would require 2 padding bytes to be added at the end of the first sample. Current code wouldn't do that.

Quoted 7 lines

If we allow only power of 2 sizes - I would expect a scan consisting of a 8 byte element followed by a 16 byte element to be tightly packed. I'd assume that for the 16 byte data, it'd be enough to ensure 8 byte alignment. Current code would however add 8 bytes of padding at the end of the first 8 byte element to make the 16 byte scan element to be aligned at 16 byte address. To my uneducated mind this is not needed - but maybe I just don't know what I am writing about :)

Revision history
v3 => v4:
- drop extra print and TODO coment
- add comment clarifying alignment sizes

---
 tools/iio/iio_generic_buffer.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)
tools/iio/iio_generic_buffer.c +17 -1
index 44bbf80f0cfd..c07c49397b19 100644
@@ -54,9 +54,12 @@ enum autochan {
 static unsigned int size_from_channelarray(struct iio_channel_info *channels, int num_channels)
 {
 	unsigned int bytes = 0;
-	int i = 0;
+	int i = 0, max = 0;
+	unsigned int misalignment;
 
 	while (i < num_channels) {
+		if (channels[i].bytes > max)
+			max = channels[i].bytes;
 		if (bytes % channels[i].bytes == 0)
 			channels[i].location = bytes;
 		else
@@ -66,6 +69,19 @@ static unsigned int size_from_channelarray(struct iio_channel_info *channels, in
 		bytes = channels[i].location + channels[i].bytes;
 		i++;
 	}
+	/*
+	 * We wan't the data in next sample to also be properly aligned so
+	 * we'll add padding at the end if needed.
+	 *
+	 * Please note, this code does ensure alignment to maximum channel
+	 * size. It works only as long as the channel sizes are 1, 2, 4 or 8
+	 * bytes. Also, on 32 bit platforms it might be enough to align also
+	 * the 8 byte elements to 4 byte boundary - which this code is not
+	 * doing.
+	 */
+	misalignment = bytes % max;
+	if (misalignment)
+		bytes += max - misalignment;
 
 	return bytes;
 }
Signature · 14 lines
2.41.0


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 
#2 lore raw

Re: [PATCH v4 1/5] tools: iio: iio_generic_buffer ensure alignment

Quoted 10 lines · Matti Vaittinen

The iio_generic_buffer can return garbage values when the total size of scan data is not a multiple of the largest element in the scan. This can be demonstrated by reading a scan, consisting, for example of one 4-byte and one 2-byte element, where the 4-byte element is first in the buffer.

The IIO generic buffer code does not take into account the last two padding bytes that are needed to ensure that the 4-byte data for next scan is correctly aligned.

Add the padding bytes required to align the next sample with the scan size.

...

tools/iio/iio_generic_buffer.c static unsigned int size_from_channelarray(struct iio_channel_info *channels, in · line 73

Quoted 2 lines
tools/iio/iio_generic_buffer.c +2 -0
+	/*
+	 * We wan't the data in next sample to also be properly aligned so

Pardon me, won't or want, I didn't get?..

tools/iio/iio_generic_buffer.c static unsigned int size_from_channelarray(struct iio_channel_info *channels, in · line 78

Quoted 5 lines
tools/iio/iio_generic_buffer.c +5 -0
+	 * we'll add padding at the end if needed.
+	 *
+	 * Please note, this code does ensure alignment to maximum channel
+	 * size. It works only as long as the channel sizes are 1, 2, 4 or 8
+	 * bytes. Also, on 32 bit platforms it might be enough to align also

32-bit

tools/iio/iio_generic_buffer.c static unsigned int size_from_channelarray(struct iio_channel_info *channels, in · line 79

Quoted 1 line
tools/iio/iio_generic_buffer.c +1 -0
+	 * the 8 byte elements to 4 byte boundary - which this code is not

8-byte
4-byte

Quoted 2 lines
tools/iio/iio_generic_buffer.c +2 -0
+	 * doing.
+	 */
Signature · 2 lines
With Best Regards,
Andy Shevchenko
author maintainer
#3 lore raw

Re: [PATCH v4 1/5] tools: iio: iio_generic_buffer ensure alignment

Quoted 10 lines · Andy Shevchenko
Quoted 9 lines · Matti Vaittinen

The iio_generic_buffer can return garbage values when the total size of scan data is not a multiple of the largest element in the scan. This can be demonstrated by reading a scan, consisting, for example of one 4-byte and one 2-byte element, where the 4-byte element is first in the buffer.

The IIO generic buffer code does not take into account the last two padding bytes that are needed to ensure that the 4-byte data for next scan is correctly aligned.

Quoted 8 lines
Quoted 1 line

Add the padding bytes required to align the next sample with the scan size.

...

tools/iio/iio_generic_buffer.c static unsigned int size_from_channelarray(struct iio_channel_info *channels, in · line 73

Quoted 2 lines
tools/iio/iio_generic_buffer.c +2 -0
+	/*
+	 * We wan't the data in next sample to also be properly aligned so

Pardon me, won't or want, I didn't get?..

Both :D Well, purpose was to say want, but as I try to explain we get what we want only in some case - in other cases we won't ;) Anyways, this is something that should be fixed - thanks :)

Quoted 9 lines

tools/iio/iio_generic_buffer.c static unsigned int size_from_channelarray(struct iio_channel_info *channels, in · line 78

Quoted 6 lines
tools/iio/iio_generic_buffer.c +5 -0
+	 * we'll add padding at the end if needed.
+	 *
+	 * Please note, this code does ensure alignment to maximum channel
+	 * size. It works only as long as the channel sizes are 1, 2, 4 or 8
+	 * bytes. Also, on 32 bit platforms it might be enough to align also

32-bit

Quoted 8 lines

tools/iio/iio_generic_buffer.c static unsigned int size_from_channelarray(struct iio_channel_info *channels, in · line 79

Quoted 1 line
tools/iio/iio_generic_buffer.c +1 -0
+	 * the 8 byte elements to 4 byte boundary - which this code is not

8-byte
4-byte

tools/iio/iio_generic_buffer.c static unsigned int size_from_channelarray(struct iio_channel_info *channels, in · line 81

Quoted 2 lines
tools/iio/iio_generic_buffer.c +2 -0
+	 * doing.
+	 */

Thanks!

Yours,
-- Matti

Signature · 5 lines
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~
maintainer
#4 lore raw

Re: [PATCH v4 1/5] tools: iio: iio_generic_buffer ensure alignment

In reply to Matti Vaittinen
Quoted 63 lines · On Wed, 27 Sep 2023 11:26:07 +0300 Matti Vaittinen <[email protected]>…

The iio_generic_buffer can return garbage values when the total size of scan data is not a multiple of the largest element in the scan. This can be demonstrated by reading a scan, consisting, for example of one 4-byte and one 2-byte element, where the 4-byte element is first in the buffer.

The IIO generic buffer code does not take into account the last two padding bytes that are needed to ensure that the 4-byte data for next scan is correctly aligned.

Add the padding bytes required to align the next sample with the scan size.

---
I think the whole alignment code could be revised here, but I am unsure what kind of alignment is expected, and if it actually depends on the architecture. Anyways, I'll quote myself from another mail to explain how this patch handles things:

Quoted 7 lines

For non power of2 sizes, the alignment code will result strange alignments. For example, scan consisting of two 6-byte elements would be packed - meaning the second element would probably break the alignment rules by starting from address '6'. I think that on most architectures the proper access would require 2 padding bytes to be added at the end of the first sample. Current code wouldn't do that.

Quoted 7 lines

If we allow only power of 2 sizes - I would expect a scan consisting of a 8 byte element followed by a 16 byte element to be tightly packed. I'd assume that for the 16 byte data, it'd be enough to ensure 8 byte alignment. Current code would however add 8 bytes of padding at the end of the first 8 byte element to make the 16 byte scan element to be aligned at 16 byte address. To my uneducated mind this is not needed - but maybe I just don't know what I am writing about :)

Revision history
v3 => v4:
- drop extra print and TODO coment
- add comment clarifying alignment sizes

---
 tools/iio/iio_generic_buffer.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)
tools/iio/iio_generic_buffer.c +4 -1
index 44bbf80f0cfd..c07c49397b19 100644
@@ -54,9 +54,12 @@ enum autochan {
 static unsigned int size_from_channelarray(struct iio_channel_info *channels, int num_channels)
 {
 	unsigned int bytes = 0;
-	int i = 0;
+	int i = 0, max = 0;
+	unsigned int misalignment;
 
 	while (i < num_channels) {
+		if (channels[i].bytes > max)
+			max = channels[i].bytes;
 		if (bytes % channels[i].bytes == 0)
 			channels[i].location = bytes;
 		else
@@ -66,6 +69,19 @@ static unsigned int size_from_channelarray(struct iio_channel_info *channels, in
 		bytes = channels[i].location + channels[i].bytes;
 		i++;

tools/iio/iio_generic_buffer.c static unsigned int size_from_channelarray(struct iio_channel_info *channels, in · line 80

Quoted 10 lines
tools/iio/iio_generic_buffer.c +9 -0
 	}
+	/*
+	 * We wan't the data in next sample to also be properly aligned so
+	 * we'll add padding at the end if needed.
+	 *
+	 * Please note, this code does ensure alignment to maximum channel
+	 * size. It works only as long as the channel sizes are 1, 2, 4 or 8
+	 * bytes. Also, on 32 bit platforms it might be enough to align also
+	 * the 8 byte elements to 4 byte boundary - which this code is not
+	 * doing.

Very much not! We need to present same data alignment to userspace indpendent of what architecture is running.

It's annoyingly inconsistent how 8 byte elements are handled on 32 bit architectures as some have optimized aligned access routines and others will read as 2 32 bit fields. Hence we just stick to 8 byte value is 8 byte aligned which is always fine but wastes a bit of space on x86 32 bit - which I don't care about ;)

Please drop this last bit of the comment as we should just say what it does, not conjecture what it might do!

Quoted 7 lines
tools/iio/iio_generic_buffer.c +4 -0
+	 */
+	misalignment = bytes % max;
+	if (misalignment)
+		bytes += max - misalignment;
 
 	return bytes;
 }
author maintainer
#5 lore raw

Re: [PATCH v4 1/5] tools: iio: iio_generic_buffer ensure alignment

Quoted 79 lines · Jonathan Cameron
Quoted 63 lines · On Wed, 27 Sep 2023 11:26:07 +0300 Matti Vaittinen <[email protected]>…

The iio_generic_buffer can return garbage values when the total size of scan data is not a multiple of the largest element in the scan. This can be demonstrated by reading a scan, consisting, for example of one 4-byte and one 2-byte element, where the 4-byte element is first in the buffer.

The IIO generic buffer code does not take into account the last two padding bytes that are needed to ensure that the 4-byte data for next scan is correctly aligned.

Add the padding bytes required to align the next sample with the scan size.

---
I think the whole alignment code could be revised here, but I am unsure what kind of alignment is expected, and if it actually depends on the architecture. Anyways, I'll quote myself from another mail to explain how this patch handles things:

Quoted 7 lines

For non power of2 sizes, the alignment code will result strange alignments. For example, scan consisting of two 6-byte elements would be packed - meaning the second element would probably break the alignment rules by starting from address '6'. I think that on most architectures the proper access would require 2 padding bytes to be added at the end of the first sample. Current code wouldn't do that.

Quoted 7 lines

If we allow only power of 2 sizes - I would expect a scan consisting of a 8 byte element followed by a 16 byte element to be tightly packed. I'd assume that for the 16 byte data, it'd be enough to ensure 8 byte alignment. Current code would however add 8 bytes of padding at the end of the first 8 byte element to make the 16 byte scan element to be aligned at 16 byte address. To my uneducated mind this is not needed - but maybe I just don't know what I am writing about :)

Revision history
v3 => v4:
- drop extra print and TODO coment
- add comment clarifying alignment sizes

---
  tools/iio/iio_generic_buffer.c | 18 +++++++++++++++++-
  1 file changed, 17 insertions(+), 1 deletion(-)
tools/iio/iio_generic_buffer.c +4 -1
index 44bbf80f0cfd..c07c49397b19 100644
@@ -54,9 +54,12 @@ enum autochan {
  static unsigned int size_from_channelarray(struct iio_channel_info *channels, int num_channels)
  {
  	unsigned int bytes = 0;
-	int i = 0;
+	int i = 0, max = 0;
+	unsigned int misalignment;
 
  	while (i < num_channels) {
+		if (channels[i].bytes > max)
+			max = channels[i].bytes;
  		if (bytes % channels[i].bytes == 0)
  			channels[i].location = bytes;
  		else
@@ -66,6 +69,19 @@ static unsigned int size_from_channelarray(struct iio_channel_info *channels, in
  		bytes = channels[i].location + channels[i].bytes;
  		i++;

tools/iio/iio_generic_buffer.c static unsigned int size_from_channelarray(struct iio_channel_info *channels, in · line 80

Quoted 10 lines
tools/iio/iio_generic_buffer.c +9 -0
  	}
+	/*
+	 * We wan't the data in next sample to also be properly aligned so
+	 * we'll add padding at the end if needed.
+	 *
+	 * Please note, this code does ensure alignment to maximum channel
+	 * size. It works only as long as the channel sizes are 1, 2, 4 or 8
+	 * bytes. Also, on 32 bit platforms it might be enough to align also
+	 * the 8 byte elements to 4 byte boundary - which this code is not
+	 * doing.

Very much not! We need to present same data alignment to userspace indpendent of what architecture is running.

Quoted 8 lines

It's annoyingly inconsistent how 8 byte elements are handled on 32 bit architectures as some have optimized aligned access routines and others will read as 2 32 bit fields. Hence we just stick to 8 byte value is 8 byte aligned which is always fine but wastes a bit of space on x86 32 bit - which I don't care about ;)

Please drop this last bit of the comment as we should just say what it does, not conjecture what it might do!

Ok. The comment was more to catch the reviewers' attention ;) I'll just note the alignment works for power of 2 sample sizes and aligns according to the max sized sample, even if it was bigger than 8.

Thanks!

-- Matti

Signature · 5 lines
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~
author maintainer
#6 lore raw

PATCH v4 2/5 iio: improve doc for available_scan_mask

In reply to Matti Vaittinen

The available_scan_mask is an array of bitmaps representing the channels which can be simultaneously enabled by the driver. In many cases, the hardware can offer more channels than what the user is interested in obtaining. In such cases, it may be preferred that only a subset of channels are enabled, and the driver reads only a subset of the channels from the hardware.

Some devices can't support all channel combinations. For example, the BM1390 pressure sensor must always read the pressure data in order to acknowledge the watermark IRQ, while reading temperature can be omitted. So, the available scan masks would be 'pressure and temperature' and 'pressure only'.

When IIO searches for the scan mask it asks the driver to use, it will pick the first suitable one from the 'available_scan_mask' array. Hence, ordering the masks in the array makes a difference. We should 'prefer' reading just the pressure from the hardware (as it is a cheaper operation than reading both pressure and temperature) over reading both pressure and temperature. Hence, we should set the 'only pressure' as the first scan mask in available_scan_mask array. If we set the 'pressure and temperature' as first in the array, then the 'only temperature' will never get used as 'pressure and temperature' can always serve the user's needs.

Add (minimal) kerneldoc to the 'available_scan_mask' to hint the user that the ordering of masks matters.

---
 include/linux/iio/iio.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
include/linux/iio/iio.h +3 -1
index 202e55b0a28b..7bfa1b9bc8a2 100644
@@ -556,7 +556,9 @@ struct iio_buffer_setup_ops {
  *			and owner
  * @buffer:		[DRIVER] any buffer present
  * @scan_bytes:		[INTERN] num bytes captured to be fed to buffer demux
- * @available_scan_masks: [DRIVER] optional array of allowed bitmasks
+ * @available_scan_masks: [DRIVER] optional array of allowed bitmasks. Sort the
+ *			   array in order of preference, the most preferred
+ *			   masks first.
  * @masklength:		[INTERN] the length of the mask established from
  *			channels
  * @active_scan_mask:	[INTERN] union of all scan masks requested by buffers
Signature · 14 lines
2.41.0


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 
maintainer
#7 lore raw

Re: [PATCH v4 2/5] iio: improve doc for available_scan_mask

Quoted 24 lines · On Wed, 27 Sep 2023 11:27:41 +0300 Matti Vaittinen <[email protected]>…

The available_scan_mask is an array of bitmaps representing the channels which can be simultaneously enabled by the driver. In many cases, the hardware can offer more channels than what the user is interested in obtaining. In such cases, it may be preferred that only a subset of channels are enabled, and the driver reads only a subset of the channels from the hardware.

Some devices can't support all channel combinations. For example, the BM1390 pressure sensor must always read the pressure data in order to acknowledge the watermark IRQ, while reading temperature can be omitted. So, the available scan masks would be 'pressure and temperature' and 'pressure only'.

When IIO searches for the scan mask it asks the driver to use, it will pick the first suitable one from the 'available_scan_mask' array. Hence, ordering the masks in the array makes a difference. We should 'prefer' reading just the pressure from the hardware (as it is a cheaper operation than reading both pressure and temperature) over reading both pressure and temperature. Hence, we should set the 'only pressure' as the first scan mask in available_scan_mask array. If we set the 'pressure and temperature' as first in the array, then the 'only temperature' will never get used as 'pressure and temperature' can always serve the user's needs.

Quoted 4 lines

Add (minimal) kerneldoc to the 'available_scan_mask' to hint the user that the ordering of masks matters.

Applied this patch to the togreg branch of iio.git
If you fancy turning the patch description into some more formal documentation under Documentation/... that might be a good thing in the long run. I always mean to spend some time working on that, but too manyt things to do :(

Thanks,

Jonathan

Quoted 19 lines
---
 include/linux/iio/iio.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
include/linux/iio/iio.h +3 -1
index 202e55b0a28b..7bfa1b9bc8a2 100644
@@ -556,7 +556,9 @@ struct iio_buffer_setup_ops {
  *			and owner
  * @buffer:		[DRIVER] any buffer present
  * @scan_bytes:		[INTERN] num bytes captured to be fed to buffer demux
- * @available_scan_masks: [DRIVER] optional array of allowed bitmasks
+ * @available_scan_masks: [DRIVER] optional array of allowed bitmasks. Sort the
+ *			   array in order of preference, the most preferred
+ *			   masks first.
  * @masklength:		[INTERN] the length of the mask established from
  *			channels
  * @active_scan_mask:	[INTERN] union of all scan masks requested by buffers
author maintainer
#8 lore raw

PATCH v4 3/5 dt-bindings: Add ROHM BM1390 pressure sensor

In reply to Matti Vaittinen

BM1390GLV-Z is a pressure sensor which performs internal temperature compensation for the MEMS. Pressure range is from 300 hPa to 1300 hPa and sample averaging and IIR filtering is built in sensor. Temperature measurement is also supported.

Add dt-bindings for the sensor.

---
Revision history:
v1 => v2:
- Improve spelling in description
- Drop unnecessary '|'
- Use correct example (fix compatible)

---
 .../bindings/iio/pressure/rohm,bm1390.yaml    | 52 +++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/pressure/rohm,bm1390.yaml
Documentation/devicetree/bindings/iio/pressure/rohm,bm1390.yaml new file +52 -0
new file mode 100644
index 000000000000..7c4ca6322bf8
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/pressure/rohm,bm1390.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ROHM BM1390 pressure sensor
+
+maintainers:
+  - Matti Vaittinen <[email protected]>
+
+description:
+  BM1390GLV-Z is a pressure sensor which performs internal temperature
+  compensation for the MEMS. Pressure range is from 300 hPa to 1300 hPa
+  and sample averaging and IIR filtering is built in. Temperature
+  measurement is also supported.
+
+properties:
+  compatible:
+    const: rohm,bm1390glv-z
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  vdd-supply: true
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        pressure-sensor@5d {
+            compatible = "rohm,bm1390glv-z";
+            reg = <0x5d>;
+
+            interrupt-parent = <&gpio1>;
+            interrupts = <29 IRQ_TYPE_LEVEL_LOW>;
+
+            vdd-supply = <&vdd>;
+        };
+    };
Signature · 14 lines
2.41.0


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 
author maintainer
#9 lore raw

PATCH v4 4/5 iio: pressure: Support ROHM BU1390

In reply to Matti Vaittinen

Support for the ROHM BM1390 pressure sensor. The BM1390GLV-Z can measure pressures ranging from 300 hPa to 1300 hPa with configurable measurement averaging and internal FIFO. The sensor does also provide temperature measurements.

Sensor does also contain IIR filter implemented in HW. The data-sheet says the IIR filter can be configured to be "weak", "middle" or "strong". Some RMS noise figures are provided in data sheet but no accurate maths for the filter configurations is provided. Hence, the IIR filter configuration is not supported by this driver and the filter is configured to the "middle" setting (at least not for now).

The FIFO measurement mode is only measuring the pressure and not the temperature. The driver measures temperature when FIFO is flushed and simply uses the same measured temperature value to all reported temperatures. This should not be a problem when temperature is not changing very rapidly (several degrees C / second) but allows users to get the temperature measurements from sensor without any additional logic.

This driver allows the sensor to be used in two muitually exclusive ways,

1. With trigger (data-ready IRQ).
In this case the FIFO is not used as we get data ready for each collected sample. Instead, for each data-ready IRQ we read the sample from sensor and push it to the IIO buffer.

2. With hardware FIFO and watermark IRQ.
In this case the data-ready is not used but we enable watermark IRQ. At each watermark IRQ we go and read all samples in FIFO and push them to the IIO buffer.

---
Revision history:
v3 => v4:
- fix some typos
- drop useless print
- use iio_push_to_buffers() instead of iio_push_to_buffers_with_timestamp()
- provide own iio_info struct for use without IRQ
- allow other triggers
- fix ordering of the FIFO disabling
- don't set INDIO_BUFFER_SOFTWARE when there's no SW buffer

v2 => v3:
- Read temperature only after FIFO is read to overcome a HW quirck
- Drop unused defines
- Allow scanning the pressure only
- Some clarifying comments added, some made less verbose
- warn if measurement stp fails
- use IIO_VAL_FRACTIONAL for pressure scale
- don't disable IRQ but use timestamp from stack
- fix amount of samples to read
- minor styling
- better separate buffer and trigger parts
- allow buffer even when there is no IRQ
with external trigger to be supported.
- add completely, utterly useless NULL check because we have the cycles to waste (grumbles)

v1 => v2:
- prefer s64 over int64_t
- drop not needed handling of 2's complements
- plenty of styling changes
- drop dead code (write_raw)
- fix typos in comments
- explain trigger and FIFO usage in commit message
- do better job at cheking the return values
- ensure there's no race when checking if triggered buffer is used before enabling the FIFO
- print warning if register read fails at IRQ handler
- drop unnecessary warning if IRQ is not given
- explain why we prefer asynchronous probing

---
 drivers/iio/pressure/Kconfig       |   9 +
 drivers/iio/pressure/Makefile      |   1 +
 drivers/iio/pressure/rohm-bm1390.c | 934 +++++++++++++++++++++++++++++
 3 files changed, 944 insertions(+)
 create mode 100644 drivers/iio/pressure/rohm-bm1390.c
drivers/iio/pressure/Kconfig +9 -0
index 7b4c2af32852..95efa32e4289 100644
@@ -16,6 +16,15 @@ config ABP060MG
 	  To compile this driver as a module, choose M here: the module
 	  will be called abp060mg.
 
+config ROHM_BM1390
+	tristate "ROHM BM1390GLV-Z pressure sensor driver"
+	depends on I2C
+	help
+	  Support for the ROHM BM1390 pressure sensor. The BM1390GLV-Z
+	  can measure pressures ranging from 300 hPa to 1300 hPa with
+	  configurable measurement averaging and internal FIFO. The
+	  sensor does also provide temperature measurements.
+
 config BMP280
 	tristate "Bosch Sensortec BMP180/BMP280/BMP380/BMP580 pressure sensor driver"
 	depends on (I2C || SPI_MASTER)
drivers/iio/pressure/Makefile +1 -0
index c90f77210e94..436aec7e65f3 100644
@@ -5,6 +5,7 @@
 
 # When adding new entries keep the list in alphabetical order
 obj-$(CONFIG_ABP060MG) += abp060mg.o
+obj-$(CONFIG_ROHM_BM1390) += rohm-bm1390.o
 obj-$(CONFIG_BMP280) += bmp280.o
 bmp280-objs := bmp280-core.o bmp280-regmap.o
 obj-$(CONFIG_BMP280_I2C) += bmp280-i2c.o
drivers/iio/pressure/rohm-bm1390.c new file +934 -0

934 lines not on this page · raw message

Signature · 14 lines
2.41.0


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 
author maintainer
#10 lore raw

PATCH v4 5/5 MAINTAINERS: Add ROHM BM1390

In reply to Matti Vaittinen

Add myself as a maintainer for ROHM BM1390 pressure sensor driver.

---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)
MAINTAINERS +6 -0
index 90f13281d297..91390f571a05 100644
@@ -18563,6 +18563,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/iio/light/bh1750.yaml
 F:	drivers/iio/light/bh1750.c
 
+ROHM BM1390 PRESSURE SENSOR DRIVER
+M:	Matti Vaittinen <[email protected]>
+L:	[email protected]
+S:	Supported
+F:	drivers/iio/pressure/rohm-bm1390.c
+
 ROHM BU270xx LIGHT SENSOR DRIVERs
 M:	Matti Vaittinen <[email protected]>
 L:	[email protected]
Signature · 14 lines
2.41.0


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 
maintainer Applied
#11 lore raw

Re: [PATCH v4 5/5] MAINTAINERS: Add ROHM BM1390

Quoted 3 lines · On Wed, 27 Sep 2023 11:28:29 +0300 Matti Vaittinen <[email protected]>…

Add myself as a maintainer for ROHM BM1390 pressure sensor driver.

Applied, 3-5 (as well as 2 that I replied to separately).

Nice driver. Thanks,

Jonathan

Quoted 21 lines
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)
MAINTAINERS +6 -0
index 90f13281d297..91390f571a05 100644
@@ -18563,6 +18563,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/iio/light/bh1750.yaml
 F:	drivers/iio/light/bh1750.c
 
+ROHM BM1390 PRESSURE SENSOR DRIVER
+M:	Matti Vaittinen <[email protected]>
+L:	[email protected]
+S:	Supported
+F:	drivers/iio/pressure/rohm-bm1390.c
+
 ROHM BU270xx LIGHT SENSOR DRIVERs
 M:	Matti Vaittinen <[email protected]>
 L:	[email protected]