[PATCH v2 0/3] iio: pressure: bmp280: Minor cleanup

[PATCH v2 0/3] iio: pressure: bmp280: Minor cleanup

[Vasileios Amoiridis]

Changes in v2:

Patch 1/3:
	- Switch if case for better readability

Patch 2/3:
	- Reword commit message

---
v1: https://lore.kernel.org/linux-iio/20241128232450.313862-1-vassilisamir@gmail.com/

This series adds the SPI interface description on the device-tree file
of the sensor, adds proper self-described sized variables and performs
a minor optimization in time variable names.

Vasileios Amoiridis (3):
  dt-bindings: iio: pressure: bmp085: Add SPI interface
  iio: pressure: bmp280: Use sizeof() for denominator
  iio: pressure: bmp280: Make time vars intuitive and move to fsleep

 .../bindings/iio/pressure/bmp085.yaml         | 32 +++++++++++++++
 drivers/iio/pressure/bmp280-core.c            | 39 ++++++++++---------
 drivers/iio/pressure/bmp280.h                 |  8 ++--
 3 files changed, 56 insertions(+), 23 deletions(-)


base-commit: a61ff7eac77e86de828fe28c4e42b8ae9ec2b195
-- 
2.43.0

[PATCH v1 1/3] dt-bindings: iio: pressure: bmp085: Add SPI interface

[Vasileios Amoiridis]

The BMP{2,3,5}80 and BME280 devices have an SPI interface, so include it
in the device-tree.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 .../bindings/iio/pressure/bmp085.yaml         | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
index cb201cecfa1a..43af400a9939 100644
--- a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
+++ b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
@@ -55,12 +55,16 @@ properties:
       If not set, defaults to push-pull configuration.
     type: boolean
 
+  spi-max-frequency:
+    maximum: 10000000
+
 required:
   - compatible
   - vddd-supply
   - vdda-supply
 
 allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
   - if:
       properties:
         compatible:
@@ -73,6 +77,16 @@ allOf:
     then:
       properties:
         interrupts: false
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - bosch,bmp085
+              - bosch,bmp180
+    then:
+      properties:
+        spi-max-frequency: false
 
 additionalProperties: false
 
@@ -93,3 +107,21 @@ examples:
             vdda-supply = <&bar>;
         };
     };
+  - |
+    # include <dt-bindings/gpio/gpio.h>
+    # include <dt-bindings/interrupt-controller/irq.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        pressure@0 {
+            compatible = "bosch,bmp280";
+            reg = <0>;
+            spi-max-frequency = <10000000>;
+            interrupt-parent = <&gpio0>;
+            interrupts = <25 IRQ_TYPE_EDGE_RISING>;
+            reset-gpios = <&gpio0 26 GPIO_ACTIVE_LOW>;
+            vddd-supply = <&foo>;
+            vdda-supply = <&bar>;
+        };
+    };
+
-- 
2.43.0

Re: [PATCH v1 1/3] dt-bindings: iio: pressure: bmp085: Add SPI interface

[Rob Herring (Arm)]

On Mon, 02 Dec 2024 19:19:05 +0100, Vasileios Amoiridis wrote:
> The BMP{2,3,5}80 and BME280 devices have an SPI interface, so include it
> in the device-tree.
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> ---
>  .../bindings/iio/pressure/bmp085.yaml         | 32 +++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/pressure/bmp085.example.dtb: pressure@0: interrupts: False schema does not allow [[25, 1]]
	from schema $id: http://devicetree.org/schemas/iio/pressure/bmp085.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241202181907.21471-2-vassilisamir@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v1 1/3] dt-bindings: iio: pressure: bmp085: Add SPI interface

[Vasileios Amoiridis]

On Mon, Dec 02, 2024 at 02:46:13PM -0600, Rob Herring (Arm) wrote:
> 
> On Mon, 02 Dec 2024 19:19:05 +0100, Vasileios Amoiridis wrote:
> > The BMP{2,3,5}80 and BME280 devices have an SPI interface, so include it
> > in the device-tree.
> > 
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> > ---
> >  .../bindings/iio/pressure/bmp085.yaml         | 32 +++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> > 
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/pressure/bmp085.example.dtb: pressure@0: interrupts: False schema does not allow [[25, 1]]

Hi Rob, Krzysztof,

The error is in the example, I put the tree from the I2C example to SPI
but I used bmp280 which is not supporting interrupts. Will be fixed.

Cheers,
Vasilis

> 	from schema $id: http://devicetree.org/schemas/iio/pressure/bmp085.yaml#
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241202181907.21471-2-vassilisamir@gmail.com
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
> 

Re: [PATCH v1 1/3] dt-bindings: iio: pressure: bmp085: Add SPI interface

[Krzysztof Kozlowski]

On 02/12/2024 23:04, Vasileios Amoiridis wrote:
> On Mon, Dec 02, 2024 at 02:46:13PM -0600, Rob Herring (Arm) wrote:
>>
>> On Mon, 02 Dec 2024 19:19:05 +0100, Vasileios Amoiridis wrote:
>>> The BMP{2,3,5}80 and BME280 devices have an SPI interface, so include it
>>> in the device-tree.
>>>
>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
>>> ---
>>>  .../bindings/iio/pressure/bmp085.yaml         | 32 +++++++++++++++++++
>>>  1 file changed, 32 insertions(+)
>>>
>>
>> My bot found errors running 'make dt_binding_check' on your patch:
>>
>> yamllint warnings/errors:
>>
>> dtschema/dtc warnings/errors:
>> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/pressure/bmp085.example.dtb: pressure@0: interrupts: False schema does not allow [[25, 1]]
> 
> Hi Rob, Krzysztof,
> 
> The error is in the example, I put the tree from the I2C example to SPI
> but I used bmp280 which is not supporting interrupts. Will be fixed.


So you sent a patch with a known bug or just did not test?

Best regards,
Krzysztof

Re: [PATCH v1 1/3] dt-bindings: iio: pressure: bmp085: Add SPI interface

[Vasileios Amoiridis]

On Tue, Dec 03, 2024 at 08:23:35AM +0100, Krzysztof Kozlowski wrote:
> On 02/12/2024 23:04, Vasileios Amoiridis wrote:
> > On Mon, Dec 02, 2024 at 02:46:13PM -0600, Rob Herring (Arm) wrote:
> >>
> >> On Mon, 02 Dec 2024 19:19:05 +0100, Vasileios Amoiridis wrote:
> >>> The BMP{2,3,5}80 and BME280 devices have an SPI interface, so include it
> >>> in the device-tree.
> >>>
> >>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >>> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> >>> ---
> >>>  .../bindings/iio/pressure/bmp085.yaml         | 32 +++++++++++++++++++
> >>>  1 file changed, 32 insertions(+)
> >>>
> >>
> >> My bot found errors running 'make dt_binding_check' on your patch:
> >>
> >> yamllint warnings/errors:
> >>
> >> dtschema/dtc warnings/errors:
> >> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/pressure/bmp085.example.dtb: pressure@0: interrupts: False schema does not allow [[25, 1]]
> > 
> > Hi Rob, Krzysztof,
> > 
> > The error is in the example, I put the tree from the I2C example to SPI
> > but I used bmp280 which is not supporting interrupts. Will be fixed.
> 
> 
> So you sent a patch with a known bug or just did not test?
> 
> Best regards,
> Krzysztof

Hi Krzysztof,

Nothing of the two. I didn't test properly. I am trying to automate my
testing+sending patches process (Andy mentioned something similar in
another e-mail in this series). Do you have any advice/tool to propose
on how to automate this? It would be very welcome!

Sorry for the inconvenience in any case.

Cheers,
Vasilis

[PATCH v1 2/3] iio: pressure: bmp280: Use sizeof() for denominator

[Vasileios Amoiridis]

Instead of using magic number 2 as a denominator, make it intuitive by
using sizeof().

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/pressure/bmp280.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
index 2df1175b6b85..a3631bc0e188 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -470,8 +470,8 @@ struct bmp280_data {
 		/* Sensor data buffer */
 		u8 buf[BME280_BURST_READ_BYTES];
 		/* Calibration data buffers */
-		__le16 bmp280_cal_buf[BMP280_CONTIGUOUS_CALIB_REGS / 2];
-		__be16 bmp180_cal_buf[BMP180_REG_CALIB_COUNT / 2];
+		__le16 bmp280_cal_buf[BMP280_CONTIGUOUS_CALIB_REGS / sizeof(__le16)];
+		__be16 bmp180_cal_buf[BMP180_REG_CALIB_COUNT / sizeof(__be16)];
 		u8 bme280_humid_cal_buf[BME280_CONTIGUOUS_CALIB_REGS];
 		u8 bmp380_cal_buf[BMP380_CALIB_REG_COUNT];
 		/* Miscellaneous, endianness-aware data buffers */
-- 
2.43.0

Re: [PATCH v1 2/3] iio: pressure: bmp280: Use sizeof() for denominator

[Jonathan Cameron]

On Mon,  2 Dec 2024 19:19:06 +0100
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> Instead of using magic number 2 as a denominator, make it intuitive by
> using sizeof().
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
Applied this patch.

Thanks,

Jonathan

> ---
>  drivers/iio/pressure/bmp280.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
> index 2df1175b6b85..a3631bc0e188 100644
> --- a/drivers/iio/pressure/bmp280.h
> +++ b/drivers/iio/pressure/bmp280.h
> @@ -470,8 +470,8 @@ struct bmp280_data {
>  		/* Sensor data buffer */
>  		u8 buf[BME280_BURST_READ_BYTES];
>  		/* Calibration data buffers */
> -		__le16 bmp280_cal_buf[BMP280_CONTIGUOUS_CALIB_REGS / 2];
> -		__be16 bmp180_cal_buf[BMP180_REG_CALIB_COUNT / 2];
> +		__le16 bmp280_cal_buf[BMP280_CONTIGUOUS_CALIB_REGS / sizeof(__le16)];
> +		__be16 bmp180_cal_buf[BMP180_REG_CALIB_COUNT / sizeof(__be16)];
>  		u8 bme280_humid_cal_buf[BME280_CONTIGUOUS_CALIB_REGS];
>  		u8 bmp380_cal_buf[BMP380_CALIB_REG_COUNT];
>  		/* Miscellaneous, endianness-aware data buffers */

[PATCH v1 3/3] iio: pressure: bmp280: Make time vars intuitive and move to fsleep

[Vasileios Amoiridis]

Move sleep functions to the new fsleep() implementation. While at it,
add time unit abbreviation as a suffix of time describing variables to
make them more intuitive.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/pressure/bmp280-core.c | 39 +++++++++++++++---------------
 drivers/iio/pressure/bmp280.h      |  4 +--
 2 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index e5ec8137961f..b39ef30f8eda 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -1002,7 +1002,7 @@ static int bmp280_preinit(struct bmp280_data *data)
 	 * after resetting, the device uses the complete power-on sequence so
 	 * it needs to wait for the defined start-up time.
 	 */
-	fsleep(data->start_up_time);
+	fsleep(data->start_up_time_us);
 
 	ret = regmap_read(data->regmap, BMP280_REG_STATUS, &reg);
 	if (ret)
@@ -1161,7 +1161,7 @@ const struct bmp280_chip_info bmp280_chip_info = {
 	.chip_id = bmp280_chip_ids,
 	.num_chip_id = ARRAY_SIZE(bmp280_chip_ids),
 	.regmap_config = &bmp280_regmap_config,
-	.start_up_time = 2000,
+	.start_up_time_us = 2000,
 	.channels = bmp280_channels,
 	.num_channels = ARRAY_SIZE(bmp280_channels),
 	.avail_scan_masks = bmp280_avail_scan_masks,
@@ -1347,7 +1347,7 @@ const struct bmp280_chip_info bme280_chip_info = {
 	.chip_id = bme280_chip_ids,
 	.num_chip_id = ARRAY_SIZE(bme280_chip_ids),
 	.regmap_config = &bme280_regmap_config,
-	.start_up_time = 2000,
+	.start_up_time_us = 2000,
 	.channels = bme280_channels,
 	.num_channels = ARRAY_SIZE(bme280_channels),
 	.avail_scan_masks = bme280_avail_scan_masks,
@@ -1414,7 +1414,7 @@ static int bmp380_cmd(struct bmp280_data *data, u8 cmd)
 		return ret;
 	}
 	/* Wait for 2ms for command to be processed */
-	usleep_range(data->start_up_time, data->start_up_time + 100);
+	fsleep(data->start_up_time_us);
 	/* Check for command processing error */
 	ret = regmap_read(data->regmap, BMP380_REG_ERROR, &reg);
 	if (ret) {
@@ -1806,7 +1806,7 @@ static int bmp380_chip_config(struct bmp280_data *data)
 		 * formula in datasheet section 3.9.2 with an offset of ~+15%
 		 * as it seen as well in table 3.9.1.
 		 */
-		msleep(150);
+		fsleep(150 * USEC_PER_MSEC);
 
 		/* Check config error flag */
 		ret = regmap_read(data->regmap, BMP380_REG_ERROR, &tmp);
@@ -1957,7 +1957,7 @@ const struct bmp280_chip_info bmp380_chip_info = {
 	.num_chip_id = ARRAY_SIZE(bmp380_chip_ids),
 	.regmap_config = &bmp380_regmap_config,
 	.spi_read_extra_byte = true,
-	.start_up_time = 2000,
+	.start_up_time_us = 2000,
 	.channels = bmp380_channels,
 	.num_channels = ARRAY_SIZE(bmp380_channels),
 	.avail_scan_masks = bmp280_avail_scan_masks,
@@ -2006,7 +2006,8 @@ static int bmp580_soft_reset(struct bmp280_data *data)
 		dev_err(data->dev, "failed to send reset command to device\n");
 		return ret;
 	}
-	usleep_range(2000, 2500);
+	/* From datasheet's table 4: electrical characteristics */
+	fsleep(2000);
 
 	/* Dummy read of chip_id */
 	ret = regmap_read(data->regmap, BMP580_REG_CHIP_ID, &reg);
@@ -2208,7 +2209,7 @@ static int bmp580_nvmem_read_impl(void *priv, unsigned int offset, void *val,
 		goto exit;
 	}
 	/* Wait standby transition time */
-	usleep_range(2500, 3000);
+	fsleep(2500);
 
 	while (bytes >= sizeof(*dst)) {
 		addr = bmp580_nvmem_addrs[offset / sizeof(*dst)];
@@ -2274,7 +2275,7 @@ static int bmp580_nvmem_write_impl(void *priv, unsigned int offset, void *val,
 		goto exit;
 	}
 	/* Wait standby transition time */
-	usleep_range(2500, 3000);
+	fsleep(2500);
 
 	while (bytes >= sizeof(*buf)) {
 		addr = bmp580_nvmem_addrs[offset / sizeof(*buf)];
@@ -2458,7 +2459,7 @@ static int bmp580_chip_config(struct bmp280_data *data)
 		return ret;
 	}
 	/* From datasheet's table 4: electrical characteristics */
-	usleep_range(2500, 3000);
+	fsleep(2500);
 
 	/* Set default DSP mode settings */
 	reg_val = FIELD_PREP(BMP580_DSP_COMP_MASK, BMP580_DSP_PRESS_TEMP_COMP_EN) |
@@ -2649,7 +2650,7 @@ const struct bmp280_chip_info bmp580_chip_info = {
 	.chip_id = bmp580_chip_ids,
 	.num_chip_id = ARRAY_SIZE(bmp580_chip_ids),
 	.regmap_config = &bmp580_regmap_config,
-	.start_up_time = 2000,
+	.start_up_time_us = 2000,
 	.channels = bmp580_channels,
 	.num_channels = ARRAY_SIZE(bmp580_channels),
 	.avail_scan_masks = bmp280_avail_scan_masks,
@@ -2720,7 +2721,7 @@ static int bmp180_wait_for_eoc(struct bmp280_data *data, u8 ctrl_meas)
 			delay_us =
 				conversion_time_max[data->oversampling_press];
 
-		usleep_range(delay_us, delay_us + 1000);
+		fsleep(delay_us);
 	}
 
 	ret = regmap_read(data->regmap, BMP280_REG_CTRL_MEAS, &ctrl);
@@ -2988,7 +2989,7 @@ const struct bmp280_chip_info bmp180_chip_info = {
 	.chip_id = bmp180_chip_ids,
 	.num_chip_id = ARRAY_SIZE(bmp180_chip_ids),
 	.regmap_config = &bmp180_regmap_config,
-	.start_up_time = 2000,
+	.start_up_time_us = 2000,
 	.channels = bmp280_channels,
 	.num_channels = ARRAY_SIZE(bmp280_channels),
 	.avail_scan_masks = bmp280_avail_scan_masks,
@@ -3066,7 +3067,7 @@ const struct bmp280_chip_info bmp085_chip_info = {
 	.chip_id = bmp180_chip_ids,
 	.num_chip_id = ARRAY_SIZE(bmp180_chip_ids),
 	.regmap_config = &bmp180_regmap_config,
-	.start_up_time = 2000,
+	.start_up_time_us = 2000,
 	.channels = bmp280_channels,
 	.num_channels = ARRAY_SIZE(bmp280_channels),
 	.avail_scan_masks = bmp280_avail_scan_masks,
@@ -3175,7 +3176,7 @@ int bmp280_common_probe(struct device *dev,
 	data->oversampling_temp = chip_info->oversampling_temp_default;
 	data->iir_filter_coeff = chip_info->iir_filter_coeff_default;
 	data->sampling_freq = chip_info->sampling_freq_default;
-	data->start_up_time = chip_info->start_up_time;
+	data->start_up_time_us = chip_info->start_up_time_us;
 
 	/* Bring up regulators */
 	regulator_bulk_set_supply_names(data->supplies,
@@ -3201,7 +3202,7 @@ int bmp280_common_probe(struct device *dev,
 		return ret;
 
 	/* Wait to make sure we started up properly */
-	usleep_range(data->start_up_time, data->start_up_time + 100);
+	fsleep(data->start_up_time_us);
 
 	/* Bring chip out of reset if there is an assigned GPIO line */
 	gpiod = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
@@ -3287,7 +3288,7 @@ int bmp280_common_probe(struct device *dev,
 	 * Set autosuspend to two orders of magnitude larger than the
 	 * start-up time.
 	 */
-	pm_runtime_set_autosuspend_delay(dev, data->start_up_time / 10);
+	pm_runtime_set_autosuspend_delay(dev, data->start_up_time_us / 10);
 	pm_runtime_use_autosuspend(dev);
 	pm_runtime_put(dev);
 
@@ -3306,7 +3307,7 @@ static int bmp280_runtime_suspend(struct device *dev)
 
 	data->chip_info->set_mode(data, BMP280_SLEEP);
 
-	fsleep(data->start_up_time);
+	fsleep(data->start_up_time_us);
 	return regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
 }
 
@@ -3320,7 +3321,7 @@ static int bmp280_runtime_resume(struct device *dev)
 	if (ret)
 		return ret;
 
-	usleep_range(data->start_up_time, data->start_up_time + 100);
+	fsleep(data->start_up_time_us);
 
 	ret = data->chip_info->chip_config(data);
 	if (ret)
diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
index a3631bc0e188..5b2ee1d0ee46 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -434,7 +434,7 @@ struct bmp280_data {
 		struct bmp380_calib bmp380;
 	} calib;
 	struct regulator_bulk_data supplies[BMP280_NUM_SUPPLIES];
-	unsigned int start_up_time; /* in microseconds */
+	unsigned int start_up_time_us;
 
 	/* log of base 2 of oversampling rate */
 	u8 oversampling_press;
@@ -490,7 +490,7 @@ struct bmp280_chip_info {
 
 	const struct iio_chan_spec *channels;
 	int num_channels;
-	unsigned int start_up_time;
+	unsigned int start_up_time_us;
 	const unsigned long *avail_scan_masks;
 
 	const int *oversampling_temp_avail;
-- 
2.43.0

Re: [PATCH v1 3/3] iio: pressure: bmp280: Make time vars intuitive and move to fsleep

[Jonathan Cameron]

On Mon,  2 Dec 2024 19:19:07 +0100
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> Move sleep functions to the new fsleep() implementation. While at it,
> add time unit abbreviation as a suffix of time describing variables to
> make them more intuitive.
> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
Applied this one.  So only patch 1 needs an update.

Thanks,

Jonathan

> ---
>  drivers/iio/pressure/bmp280-core.c | 39 +++++++++++++++---------------
>  drivers/iio/pressure/bmp280.h      |  4 +--
>  2 files changed, 22 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index e5ec8137961f..b39ef30f8eda 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -1002,7 +1002,7 @@ static int bmp280_preinit(struct bmp280_data *data)
>  	 * after resetting, the device uses the complete power-on sequence so
>  	 * it needs to wait for the defined start-up time.
>  	 */
> -	fsleep(data->start_up_time);
> +	fsleep(data->start_up_time_us);
>  
>  	ret = regmap_read(data->regmap, BMP280_REG_STATUS, &reg);
>  	if (ret)
> @@ -1161,7 +1161,7 @@ const struct bmp280_chip_info bmp280_chip_info = {
>  	.chip_id = bmp280_chip_ids,
>  	.num_chip_id = ARRAY_SIZE(bmp280_chip_ids),
>  	.regmap_config = &bmp280_regmap_config,
> -	.start_up_time = 2000,
> +	.start_up_time_us = 2000,
>  	.channels = bmp280_channels,
>  	.num_channels = ARRAY_SIZE(bmp280_channels),
>  	.avail_scan_masks = bmp280_avail_scan_masks,
> @@ -1347,7 +1347,7 @@ const struct bmp280_chip_info bme280_chip_info = {
>  	.chip_id = bme280_chip_ids,
>  	.num_chip_id = ARRAY_SIZE(bme280_chip_ids),
>  	.regmap_config = &bme280_regmap_config,
> -	.start_up_time = 2000,
> +	.start_up_time_us = 2000,
>  	.channels = bme280_channels,
>  	.num_channels = ARRAY_SIZE(bme280_channels),
>  	.avail_scan_masks = bme280_avail_scan_masks,
> @@ -1414,7 +1414,7 @@ static int bmp380_cmd(struct bmp280_data *data, u8 cmd)
>  		return ret;
>  	}
>  	/* Wait for 2ms for command to be processed */
> -	usleep_range(data->start_up_time, data->start_up_time + 100);
> +	fsleep(data->start_up_time_us);
>  	/* Check for command processing error */
>  	ret = regmap_read(data->regmap, BMP380_REG_ERROR, &reg);
>  	if (ret) {
> @@ -1806,7 +1806,7 @@ static int bmp380_chip_config(struct bmp280_data *data)
>  		 * formula in datasheet section 3.9.2 with an offset of ~+15%
>  		 * as it seen as well in table 3.9.1.
>  		 */
> -		msleep(150);
> +		fsleep(150 * USEC_PER_MSEC);
>  
>  		/* Check config error flag */
>  		ret = regmap_read(data->regmap, BMP380_REG_ERROR, &tmp);
> @@ -1957,7 +1957,7 @@ const struct bmp280_chip_info bmp380_chip_info = {
>  	.num_chip_id = ARRAY_SIZE(bmp380_chip_ids),
>  	.regmap_config = &bmp380_regmap_config,
>  	.spi_read_extra_byte = true,
> -	.start_up_time = 2000,
> +	.start_up_time_us = 2000,
>  	.channels = bmp380_channels,
>  	.num_channels = ARRAY_SIZE(bmp380_channels),
>  	.avail_scan_masks = bmp280_avail_scan_masks,
> @@ -2006,7 +2006,8 @@ static int bmp580_soft_reset(struct bmp280_data *data)
>  		dev_err(data->dev, "failed to send reset command to device\n");
>  		return ret;
>  	}
> -	usleep_range(2000, 2500);
> +	/* From datasheet's table 4: electrical characteristics */
> +	fsleep(2000);
>  
>  	/* Dummy read of chip_id */
>  	ret = regmap_read(data->regmap, BMP580_REG_CHIP_ID, &reg);
> @@ -2208,7 +2209,7 @@ static int bmp580_nvmem_read_impl(void *priv, unsigned int offset, void *val,
>  		goto exit;
>  	}
>  	/* Wait standby transition time */
> -	usleep_range(2500, 3000);
> +	fsleep(2500);
>  
>  	while (bytes >= sizeof(*dst)) {
>  		addr = bmp580_nvmem_addrs[offset / sizeof(*dst)];
> @@ -2274,7 +2275,7 @@ static int bmp580_nvmem_write_impl(void *priv, unsigned int offset, void *val,
>  		goto exit;
>  	}
>  	/* Wait standby transition time */
> -	usleep_range(2500, 3000);
> +	fsleep(2500);
>  
>  	while (bytes >= sizeof(*buf)) {
>  		addr = bmp580_nvmem_addrs[offset / sizeof(*buf)];
> @@ -2458,7 +2459,7 @@ static int bmp580_chip_config(struct bmp280_data *data)
>  		return ret;
>  	}
>  	/* From datasheet's table 4: electrical characteristics */
> -	usleep_range(2500, 3000);
> +	fsleep(2500);
>  
>  	/* Set default DSP mode settings */
>  	reg_val = FIELD_PREP(BMP580_DSP_COMP_MASK, BMP580_DSP_PRESS_TEMP_COMP_EN) |
> @@ -2649,7 +2650,7 @@ const struct bmp280_chip_info bmp580_chip_info = {
>  	.chip_id = bmp580_chip_ids,
>  	.num_chip_id = ARRAY_SIZE(bmp580_chip_ids),
>  	.regmap_config = &bmp580_regmap_config,
> -	.start_up_time = 2000,
> +	.start_up_time_us = 2000,
>  	.channels = bmp580_channels,
>  	.num_channels = ARRAY_SIZE(bmp580_channels),
>  	.avail_scan_masks = bmp280_avail_scan_masks,
> @@ -2720,7 +2721,7 @@ static int bmp180_wait_for_eoc(struct bmp280_data *data, u8 ctrl_meas)
>  			delay_us =
>  				conversion_time_max[data->oversampling_press];
>  
> -		usleep_range(delay_us, delay_us + 1000);
> +		fsleep(delay_us);
>  	}
>  
>  	ret = regmap_read(data->regmap, BMP280_REG_CTRL_MEAS, &ctrl);
> @@ -2988,7 +2989,7 @@ const struct bmp280_chip_info bmp180_chip_info = {
>  	.chip_id = bmp180_chip_ids,
>  	.num_chip_id = ARRAY_SIZE(bmp180_chip_ids),
>  	.regmap_config = &bmp180_regmap_config,
> -	.start_up_time = 2000,
> +	.start_up_time_us = 2000,
>  	.channels = bmp280_channels,
>  	.num_channels = ARRAY_SIZE(bmp280_channels),
>  	.avail_scan_masks = bmp280_avail_scan_masks,
> @@ -3066,7 +3067,7 @@ const struct bmp280_chip_info bmp085_chip_info = {
>  	.chip_id = bmp180_chip_ids,
>  	.num_chip_id = ARRAY_SIZE(bmp180_chip_ids),
>  	.regmap_config = &bmp180_regmap_config,
> -	.start_up_time = 2000,
> +	.start_up_time_us = 2000,
>  	.channels = bmp280_channels,
>  	.num_channels = ARRAY_SIZE(bmp280_channels),
>  	.avail_scan_masks = bmp280_avail_scan_masks,
> @@ -3175,7 +3176,7 @@ int bmp280_common_probe(struct device *dev,
>  	data->oversampling_temp = chip_info->oversampling_temp_default;
>  	data->iir_filter_coeff = chip_info->iir_filter_coeff_default;
>  	data->sampling_freq = chip_info->sampling_freq_default;
> -	data->start_up_time = chip_info->start_up_time;
> +	data->start_up_time_us = chip_info->start_up_time_us;
>  
>  	/* Bring up regulators */
>  	regulator_bulk_set_supply_names(data->supplies,
> @@ -3201,7 +3202,7 @@ int bmp280_common_probe(struct device *dev,
>  		return ret;
>  
>  	/* Wait to make sure we started up properly */
> -	usleep_range(data->start_up_time, data->start_up_time + 100);
> +	fsleep(data->start_up_time_us);
>  
>  	/* Bring chip out of reset if there is an assigned GPIO line */
>  	gpiod = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> @@ -3287,7 +3288,7 @@ int bmp280_common_probe(struct device *dev,
>  	 * Set autosuspend to two orders of magnitude larger than the
>  	 * start-up time.
>  	 */
> -	pm_runtime_set_autosuspend_delay(dev, data->start_up_time / 10);
> +	pm_runtime_set_autosuspend_delay(dev, data->start_up_time_us / 10);
>  	pm_runtime_use_autosuspend(dev);
>  	pm_runtime_put(dev);
>  
> @@ -3306,7 +3307,7 @@ static int bmp280_runtime_suspend(struct device *dev)
>  
>  	data->chip_info->set_mode(data, BMP280_SLEEP);
>  
> -	fsleep(data->start_up_time);
> +	fsleep(data->start_up_time_us);
>  	return regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
>  }
>  
> @@ -3320,7 +3321,7 @@ static int bmp280_runtime_resume(struct device *dev)
>  	if (ret)
>  		return ret;
>  
> -	usleep_range(data->start_up_time, data->start_up_time + 100);
> +	fsleep(data->start_up_time_us);
>  
>  	ret = data->chip_info->chip_config(data);
>  	if (ret)
> diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
> index a3631bc0e188..5b2ee1d0ee46 100644
> --- a/drivers/iio/pressure/bmp280.h
> +++ b/drivers/iio/pressure/bmp280.h
> @@ -434,7 +434,7 @@ struct bmp280_data {
>  		struct bmp380_calib bmp380;
>  	} calib;
>  	struct regulator_bulk_data supplies[BMP280_NUM_SUPPLIES];
> -	unsigned int start_up_time; /* in microseconds */
> +	unsigned int start_up_time_us;
>  
>  	/* log of base 2 of oversampling rate */
>  	u8 oversampling_press;
> @@ -490,7 +490,7 @@ struct bmp280_chip_info {
>  
>  	const struct iio_chan_spec *channels;
>  	int num_channels;
> -	unsigned int start_up_time;
> +	unsigned int start_up_time_us;
>  	const unsigned long *avail_scan_masks;
>  
>  	const int *oversampling_temp_avail;

Re: [PATCH v2 0/3] iio: pressure: bmp280: Minor cleanup

[Andy Shevchenko]

On Mon, Dec 02, 2024 at 07:19:04PM +0100, Vasileios Amoiridis wrote:
> Changes in v2:
> 
> Patch 1/3:
> 	- Switch if case for better readability
> 
> Patch 2/3:
> 	- Reword commit message
> 
> ---
> v1: https://lore.kernel.org/linux-iio/20241128232450.313862-1-vassilisamir@gmail.com/
> 
> This series adds the SPI interface description on the device-tree file
> of the sensor, adds proper self-described sized variables and performs
> a minor optimization in time variable names.

For some reason your patches still have v1 in them. I dunno how you prepare
your series but I recommend one of the two options:
1) b4 relay
2) my script: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/3] iio: pressure: bmp280: Minor cleanup

[Vasileios Amoiridis]

On Mon, Dec 02, 2024 at 08:42:04PM +0200, Andy Shevchenko wrote:
> On Mon, Dec 02, 2024 at 07:19:04PM +0100, Vasileios Amoiridis wrote:
> > Changes in v2:
> > 
> > Patch 1/3:
> > 	- Switch if case for better readability
> > 
> > Patch 2/3:
> > 	- Reword commit message
> > 
> > ---
> > v1: https://lore.kernel.org/linux-iio/20241128232450.313862-1-vassilisamir@gmail.com/
> > 
> > This series adds the SPI interface description on the device-tree file
> > of the sensor, adds proper self-described sized variables and performs
> > a minor optimization in time variable names.
> 
> For some reason your patches still have v1 in them. I dunno how you prepare
> your series but I recommend one of the two options:
> 1) b4 relay
> 2) my script: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>

Hi Andy,

Ah, my mistake! I didn't pay close attention. I use the --reroll-count
from git format-patch. I was not aware of those automated ways, I will
definitely use them. Thanks for pointing it out.

The patches are correct, the versioning is wrong, I can resend them if it
is necessary. 

Cheers,
Vasilis

Re: [PATCH v2 0/3] iio: pressure: bmp280: Minor cleanup

[Andy Shevchenko]

On Mon, Dec 02, 2024 at 08:24:48PM +0100, Vasileios Amoiridis wrote:
> On Mon, Dec 02, 2024 at 08:42:04PM +0200, Andy Shevchenko wrote:
> > On Mon, Dec 02, 2024 at 07:19:04PM +0100, Vasileios Amoiridis wrote:

...

> > > Changes in v2:
> > > 
> > > Patch 1/3:
> > > 	- Switch if case for better readability
> > > 
> > > Patch 2/3:
> > > 	- Reword commit message
> > > 
> > > ---
> > > v1: https://lore.kernel.org/linux-iio/20241128232450.313862-1-vassilisamir@gmail.com/
> > > 
> > > This series adds the SPI interface description on the device-tree file
> > > of the sensor, adds proper self-described sized variables and performs
> > > a minor optimization in time variable names.
> > 
> > For some reason your patches still have v1 in them. I dunno how you prepare
> > your series but I recommend one of the two options:
> > 1) b4 relay
> > 2) my script: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh

> Ah, my mistake! I didn't pay close attention. I use the --reroll-count
> from git format-patch. I was not aware of those automated ways, I will
> definitely use them. Thanks for pointing it out.
> 
> The patches are correct, the versioning is wrong, I can resend them if it
> is necessary. 

Up to Jonathan, if the patches are correct, I don't see the necessity of
resending until there are some issues being noted.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v1 0/3] iio: pressure: bmp280: Minor cleanup

[Vasileios Amoiridis]

This series adds the SPI interface description on the device-tree file
of the sensor, adds proper self-described sized variables and performs
a minor optimization in time variable names.

Vasileios Amoiridis (3):
  dt-bindings: iio: pressure: bmp085: Add SPI interface
  iio: pressure: bmp280: Use sizeof() for denominator
  iio: pressure: bmp280: Make time vars intuitive and move to fsleep

 .../bindings/iio/pressure/bmp085.yaml         | 35 +++++++++++++++++
 drivers/iio/pressure/bmp280-core.c            | 39 ++++++++++---------
 drivers/iio/pressure/bmp280.h                 |  8 ++--
 3 files changed, 59 insertions(+), 23 deletions(-)


base-commit: a61ff7eac77e86de828fe28c4e42b8ae9ec2b195
-- 
2.43.0

[PATCH v1 1/3] dt-bindings: iio: pressure: bmp085: Add SPI interface

[Vasileios Amoiridis]

The BMP{2,3,5}80 and BME280 devices have an SPI interface, so include it
in the device-tree.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 .../bindings/iio/pressure/bmp085.yaml         | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
index cb201cecfa1a..cbddc7bb26a6 100644
--- a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
+++ b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
@@ -55,12 +55,16 @@ properties:
       If not set, defaults to push-pull configuration.
     type: boolean
 
+  spi-max-frequency:
+    maximum: 10000000
+
 required:
   - compatible
   - vddd-supply
   - vdda-supply
 
 allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
   - if:
       properties:
         compatible:
@@ -73,6 +77,19 @@ allOf:
     then:
       properties:
         interrupts: false
+  - if:
+      properties:
+        compatible:
+          not:
+            contains:
+              enum:
+                - bosch,bmp280
+                - bosch,bme280
+                - bosch,bmp380
+                - bosch,bmp580
+    then:
+      properties:
+        spi-max-frequency: false
 
 additionalProperties: false
 
@@ -93,3 +110,21 @@ examples:
             vdda-supply = <&bar>;
         };
     };
+  - |
+    # include <dt-bindings/gpio/gpio.h>
+    # include <dt-bindings/interrupt-controller/irq.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        pressure@0 {
+            compatible = "bosch,bmp280";
+            reg = <0>;
+            spi-max-frequency = <10000000>;
+            interrupt-parent = <&gpio0>;
+            interrupts = <25 IRQ_TYPE_EDGE_RISING>;
+            reset-gpios = <&gpio0 26 GPIO_ACTIVE_LOW>;
+            vddd-supply = <&foo>;
+            vdda-supply = <&bar>;
+        };
+    };
+
-- 
2.43.0

Re: [PATCH v1 1/3] dt-bindings: iio: pressure: bmp085: Add SPI interface

[Krzysztof Kozlowski]

On Fri, Nov 29, 2024 at 12:24:48AM +0100, Vasileios Amoiridis wrote:
> @@ -73,6 +77,19 @@ allOf:
>      then:
>        properties:
>          interrupts: false
> +  - if:
> +      properties:
> +        compatible:
> +          not:
> +            contains:
> +              enum:

You have just two variants in this "not-containts", so invert your
clause. Easier code.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v1 1/3] dt-bindings: iio: pressure: bmp085: Add SPI interface

[Vasileios Amoiridis]

On Fri, Nov 29, 2024 at 08:22:00AM +0100, Krzysztof Kozlowski wrote:
> On Fri, Nov 29, 2024 at 12:24:48AM +0100, Vasileios Amoiridis wrote:
> > @@ -73,6 +77,19 @@ allOf:
> >      then:
> >        properties:
> >          interrupts: false
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          not:
> > +            contains:
> > +              enum:
> 
> You have just two variants in this "not-containts", so invert your
> clause. Easier code.
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> Best regards,
> Krzysztof
>

Hi Krzysztof,

Thank you very much for the review! I will fix it in the next version.

Cheers,
Vasilis