2024-02-20 22:44:08

by Vasileios Amoiridis

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

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

Signed-off-by: Vasileios Amoiridis <[email protected]>
---
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



2024-02-20 22:44:22

by Vasileios Amoiridis

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

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 <[email protected]>
---
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


2024-02-21 16:10:19

by Andy Shevchenko

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

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 <[email protected]>

--
With Best Regards,
Andy Shevchenko



2024-02-21 16:23:35

by Andy Shevchenko

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

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 <[email protected]>

(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



2024-02-22 07:54:58

by Angel Iglesias

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

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 <[email protected]>

Kind regards,
Angel

2024-02-22 07:58:18

by Angel Iglesias

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

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 <[email protected]>

Kind regards,
Angel

2024-02-24 18:01:26

by Jonathan Cameron

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

On Wed, 21 Feb 2024 15:21:43 +0200
Andy Shevchenko <[email protected]> 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 <[email protected]>
>
> (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.

2024-02-24 18:04:35

by Jonathan Cameron

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

On Tue, 20 Feb 2024 23:43:28 +0100
Vasileios Amoiridis <[email protected]> wrote:

> Alphabetical sorting and separation of headers for the BMPxxx
> SPI driver.
>
> Signed-off-by: Vasileios Amoiridis <[email protected]>

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,


2024-02-24 22:09:57

by Vasileios Amoiridis

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

On Sat, Feb 24, 2024 at 06:04:12PM +0000, Jonathan Cameron wrote:
> On Tue, 20 Feb 2024 23:43:28 +0100
> Vasileios Amoiridis <[email protected]> wrote:
>
> > Alphabetical sorting and separation of headers for the BMPxxx
> > SPI driver.
> >
> > Signed-off-by: Vasileios Amoiridis <[email protected]>
>
> 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,
>