[PATCH 1/2] drivers: iio: pressure: Sort headers of BMPxxx SPI driver

[PATCH 1/2] drivers: iio: pressure: Sort headers of BMPxxx SPI driver

[Vasileios Amoiridis]

Alphabetical sorting and separation of headers for the BMPxxx
SPI driver.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/pressure/bmp280-spi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c
index e8a5fed07e88..fd7ec6a5bca3 100644
--- a/drivers/iio/pressure/bmp280-spi.c
+++ b/drivers/iio/pressure/bmp280-spi.c
@@ -4,11 +4,12 @@
  *
  * Inspired by the older BMP085 driver drivers/misc/bmp085-spi.c
  */
-#include <linux/module.h>
-#include <linux/spi/spi.h>
 #include <linux/err.h>
+#include <linux/module.h>
 #include <linux/regmap.h>
 
+#include <linux/spi/spi.h>
+
 #include "bmp280.h"
 
 static int bmp280_regmap_spi_write(void *context, const void *data,
-- 
2.25.1

[PATCH 2/2] drivers: iio: pressure: Fixes BMP38x and BMP390 SPI support

[Vasileios Amoiridis]

According to the datasheet of BMP38x and BMP390 devices, for an SPI
read operation the first byte that is returned needs to be dropped,
and the rest of the bytes are the actual data returned from the
sensor.

Fixes: 8d329309184d ("iio: pressure: bmp280: Add support for BMP380 sensor family")
Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/pressure/bmp280-spi.c | 50 ++++++++++++++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c
index fd7ec6a5bca3..7ea79fe57cfd 100644
--- a/drivers/iio/pressure/bmp280-spi.c
+++ b/drivers/iio/pressure/bmp280-spi.c
@@ -4,6 +4,7 @@
  *
  * Inspired by the older BMP085 driver drivers/misc/bmp085-spi.c
  */
+#include <linux/bits.h>
 #include <linux/err.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
@@ -36,6 +37,34 @@ static int bmp280_regmap_spi_read(void *context, const void *reg,
 	return spi_write_then_read(spi, reg, reg_size, val, val_size);
 }
 
+static int bmp380_regmap_spi_read(void *context, const void *reg,
+				  size_t reg_size, void *val, size_t val_size)
+{
+	struct spi_device *spi = to_spi_device(context);
+	u8 rx_buf[4];
+	ssize_t status;
+
+	/*
+	 * Maximum number of consecutive bytes read for a temperature or
+	 * pressure measurement is 3.
+	 */
+	if (val_size > 3)
+		return -EINVAL;
+
+	/*
+	 * According to the BMP3xx datasheets, for a basic SPI read opertion,
+	 * the first byte needs to be dropped and the rest are the requested
+	 * data.
+	 */
+	status = spi_write_then_read(spi, reg, 1, rx_buf, val_size + 1);
+	if (status)
+		return status;
+
+	memcpy(val, rx_buf + 1, val_size);
+
+	return 0;
+}
+
 static struct regmap_bus bmp280_regmap_bus = {
 	.write = bmp280_regmap_spi_write,
 	.read = bmp280_regmap_spi_read,
@@ -43,10 +72,19 @@ static struct regmap_bus bmp280_regmap_bus = {
 	.val_format_endian_default = REGMAP_ENDIAN_BIG,
 };
 
+static struct regmap_bus bmp380_regmap_bus = {
+	.write = bmp280_regmap_spi_write,
+	.read = bmp380_regmap_spi_read,
+	.read_flag_mask = BIT(7),
+	.reg_format_endian_default = REGMAP_ENDIAN_BIG,
+	.val_format_endian_default = REGMAP_ENDIAN_BIG,
+};
+
 static int bmp280_spi_probe(struct spi_device *spi)
 {
 	const struct spi_device_id *id = spi_get_device_id(spi);
 	const struct bmp280_chip_info *chip_info;
+	struct regmap_bus *bmp_regmap_bus;
 	struct regmap *regmap;
 	int ret;
 
@@ -59,8 +97,18 @@ static int bmp280_spi_probe(struct spi_device *spi)
 
 	chip_info = spi_get_device_match_data(spi);
 
+	switch (chip_info->chip_id[0]) {
+	case BMP380_CHIP_ID:
+	case BMP390_CHIP_ID:
+		bmp_regmap_bus = &bmp380_regmap_bus;
+		break;
+	default:
+		bmp_regmap_bus = &bmp280_regmap_bus;
+		break;
+	}
+
 	regmap = devm_regmap_init(&spi->dev,
-				  &bmp280_regmap_bus,
+				  bmp_regmap_bus,
 				  &spi->dev,
 				  chip_info->regmap_config);
 	if (IS_ERR(regmap)) {
-- 
2.25.1

Re: [PATCH 1/2] drivers: iio: pressure: Sort headers of BMPxxx SPI driver

[Andy Shevchenko]

On Tue, Feb 20, 2024 at 11:43:28PM +0100, Vasileios Amoiridis wrote:
> Alphabetical sorting and separation of headers for the BMPxxx
> SPI driver.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] drivers: iio: pressure: Fixes BMP38x and BMP390 SPI support

[Andy Shevchenko]

On Tue, Feb 20, 2024 at 11:43:29PM +0100, Vasileios Amoiridis wrote:
> According to the datasheet of BMP38x and BMP390 devices, for an SPI
> read operation the first byte that is returned needs to be dropped,
> and the rest of the bytes are the actual data returned from the
> sensor.

LGTM,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

(If you got tags in the previous rounds of review, it's your responsibility
 to carry them, in case code is not _drastically_ changed.)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] drivers: iio: pressure: Sort headers of BMPxxx SPI driver

[Angel Iglesias]

On Tue, 2024-02-20 at 23:43 +0100, Vasileios Amoiridis wrote:
> Alphabetical sorting and separation of headers for the BMPxxx
> SPI driver.
> 
LGTM, so:

Acked-by: Angel Iglesias <ang.iglesiasg@gmail.com>

Kind regards,
Angel

Re: [PATCH 2/2] drivers: iio: pressure: Fixes BMP38x and BMP390 SPI support

[Angel Iglesias]

On Tue, 2024-02-20 at 23:43 +0100, Vasileios Amoiridis wrote:
> According to the datasheet of BMP38x and BMP390 devices, for an SPI
> read operation the first byte that is returned needs to be dropped,
> and the rest of the bytes are the actual data returned from the
> sensor.
> 

My tag from the previous round:

Acked-by: Angel Iglesias <ang.iglesiasg@gmail.com>

Kind regards,
Angel

Re: [PATCH 2/2] drivers: iio: pressure: Fixes BMP38x and BMP390 SPI support

[Jonathan Cameron]

On Wed, 21 Feb 2024 15:21:43 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Tue, Feb 20, 2024 at 11:43:29PM +0100, Vasileios Amoiridis wrote:
> > According to the datasheet of BMP38x and BMP390 devices, for an SPI
> > read operation the first byte that is returned needs to be dropped,
> > and the rest of the bytes are the actual data returned from the
> > sensor.  
> 
> LGTM,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> (If you got tags in the previous rounds of review, it's your responsibility
>  to carry them, in case code is not _drastically_ changed.)
> 

Version numbers! Otherwise poor maintainers like me have no
idea which one to pick up.

Anyhow, I got the much earlier one so will ignore newer versions.

Re: [PATCH 1/2] drivers: iio: pressure: Sort headers of BMPxxx SPI driver

[Jonathan Cameron]

On Tue, 20 Feb 2024 23:43:28 +0100
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> Alphabetical sorting and separation of headers for the BMPxxx
> SPI driver.
> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>

Hi Vasileios,

No. This ends up as a dependency of the fix which will want backporting.
This would make it a non minimal change. As such the reordering comes
after the other patch (and not until it is upstream).

All that needed to happen was bits.h being first of the includes in the
fixes patch so that a sort later was minimal.  Doing that had no impact
on the complexity of the fix to backport so was just good code hygiene
in that would simplify the resort to follow.

Anyhow, dropping this for now.  Please send an update version
next cycle once the fix is upstream

Thanks,

Jonathan

> ---
>  drivers/iio/pressure/bmp280-spi.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c
> index e8a5fed07e88..fd7ec6a5bca3 100644
> --- a/drivers/iio/pressure/bmp280-spi.c
> +++ b/drivers/iio/pressure/bmp280-spi.c
> @@ -4,11 +4,12 @@
>   *
>   * Inspired by the older BMP085 driver drivers/misc/bmp085-spi.c
>   */
> -#include <linux/module.h>
> -#include <linux/spi/spi.h>
>  #include <linux/err.h>
> +#include <linux/module.h>
>  #include <linux/regmap.h>
>  
> +#include <linux/spi/spi.h>
> +
>  #include "bmp280.h"
>  
>  static int bmp280_regmap_spi_write(void *context, const void *data,

Re: [PATCH 1/2] drivers: iio: pressure: Sort headers of BMPxxx SPI driver

[Vasileios Amoiridis]

On Sat, Feb 24, 2024 at 06:04:12PM +0000, Jonathan Cameron wrote:
> On Tue, 20 Feb 2024 23:43:28 +0100
> Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> 
> > Alphabetical sorting and separation of headers for the BMPxxx
> > SPI driver.
> > 
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> 
> Hi Vasileios,
> 
> No. This ends up as a dependency of the fix which will want backporting.
> This would make it a non minimal change. As such the reordering comes
> after the other patch (and not until it is upstream).
> 
> All that needed to happen was bits.h being first of the includes in the
> fixes patch so that a sort later was minimal.  Doing that had no impact
> on the complexity of the fix to backport so was just good code hygiene
> in that would simplify the resort to follow.
> 
> Anyhow, dropping this for now.  Please send an update version
> next cycle once the fix is upstream
> 
> Thanks,
> 
> Jonathan
> 
Hi Jonathan,

Thank you very much again for the comments and I am sorry for not versioning
them properly, it won't happen again. I also understand what you mean in the
previous message and I will definitely submit it again, first thing after the
changes are upstreamed.

Thanks,
Vasileios

> > ---
> >  drivers/iio/pressure/bmp280-spi.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c
> > index e8a5fed07e88..fd7ec6a5bca3 100644
> > --- a/drivers/iio/pressure/bmp280-spi.c
> > +++ b/drivers/iio/pressure/bmp280-spi.c
> > @@ -4,11 +4,12 @@
> >   *
> >   * Inspired by the older BMP085 driver drivers/misc/bmp085-spi.c
> >   */
> > -#include <linux/module.h>
> > -#include <linux/spi/spi.h>
> >  #include <linux/err.h>
> > +#include <linux/module.h>
> >  #include <linux/regmap.h>
> >  
> > +#include <linux/spi/spi.h>
> > +
> >  #include "bmp280.h"
> >  
> >  static int bmp280_regmap_spi_write(void *context, const void *data,
>