[PATCH v2 0/2] iio: pressure: ms5637: Add variant specific

[PATCH v2 0/2] iio: pressure: ms5637: Add variant specific

[Louis Adamian]

Hi all,

This series adds temperature compensation and pressure scaling for the
MS5637 series (02B, 30BA), MS5803 series (01BA, 02BA, 05BA, 14BA, 30BA),
MS5805, MS5837 series (02BA, 30BA), and MS8607. These all share the same
temperature compensation formula, but the constants differ between
devices. This series adds a ms_tp_comp_consts structure to store these
per-device differences and applies the compensation.

---
Changes in v2:
- Rebased on top of the togreg branch in iio.git.

Patch 1:
- Add SPI bindings for the MS5803 family.
- Add compatible strings for the ms5637-02ba and ms5837-02ba.
- Add vdd-supply as a requirement

Patch 2:
- Fix the sens2 calculation in the 2nd order temperature compensation.
- Fix the pressure scaling in ms5637_read_raw.
- Add datasheet citations to each compensation constant structure
initialization.
- Addressed automated review feedback from Sashiko
- Formatting
 - Remove unrelated formatting changes.
 - Change ms_tp_data struct initializations to match the existing
   format.
 - Sort i2c_device_id alphabetically.

Link to v1: https://lore.kernel.org/linux-iio/20260526185612.35048-2-adamianlouis@gmail.com/

Thank you, Jonathan, for the comments on v1.

Best regards,
Louis

Louis Adamian (2):
  dt-bindings: iio: pressure: Add MS5637
  iio: pressure: ms5637: Add variant specific temperature compensation

 .../bindings/iio/pressure/meas,ms5637.yaml    |  87 ++++
 .../devicetree/bindings/trivial-devices.yaml  |  10 -
 .../iio/common/ms_sensors/ms_sensors_i2c.c    |  62 ++-
 .../iio/common/ms_sensors/ms_sensors_i2c.h    |  48 +++
 drivers/iio/pressure/ms5637.c                 | 388 +++++++++++++++++-
 5 files changed, 552 insertions(+), 43 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/pressure/meas,ms5637.yaml

-- 
2.54.0

[PATCH v2 1/2] dt-bindings: iio: pressure: Add MS5637

[Louis Adamian]

Create meas,ms5637 devicetree binding and move existing devices from
trivial-devices.yaml. Explicit compatible strings are added for the
different sub-variants because each requires different constants for the
second-order temperature compensation formulas. Previously, the driver
exposed the generic compatibles for the ms5637, MS5803, MS5805
and MS5837 families and applied the MS5637-02BA compensation to all of
them.To preserve the DT ABI the existing compatibles are retained and
mapped to the -02BA variant of each family (ms5637-02ba, ms5803-02ba,
ms5837-02ba).

Signed-off-by: Louis Adamian <adamianlouis@gmail.com>
---
 .../bindings/iio/pressure/meas,ms5637.yaml    | 87 +++++++++++++++++++
 .../devicetree/bindings/trivial-devices.yaml  | 10 ---
 2 files changed, 87 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/pressure/meas,ms5637.yaml

diff --git a/Documentation/devicetree/bindings/iio/pressure/meas,ms5637.yaml b/Documentation/devicetree/bindings/iio/pressure/meas,ms5637.yaml
new file mode 100644
index 000000000000..f17ac3ffefc9
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/pressure/meas,ms5637.yaml
@@ -0,0 +1,87 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/pressure/meas,ms5637.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+title: Measurement Specialties ms5637 and similar pressure sensors
+
+maintainers:
+  - Louis Adamian <adamianlouis@gmail.com>
+
+description:
+  Measurement Specialties pressure and temperature sensors. The MS5803
+  family supports both I2C and SPI interfaces. All other variants are I2C only.
+
+properties:
+  compatible:
+    enum:
+      - meas,ms5637
+      - meas,ms5637-02ba
+      - meas,ms5637-30ba
+      - meas,ms5803
+      - meas,ms5803-01ba
+      - meas,ms5803-02ba
+      - meas,ms5803-05ba
+      - meas,ms5803-14ba
+      - meas,ms5803-30ba
+      - meas,ms5805
+      - meas,ms5837
+      - meas,ms5837-02ba
+      - meas,ms5837-30ba
+      - meas,ms8607-temppressure
+
+  reg:
+    maxItems: 1
+
+  vdd-supply:
+    description: Regulator that provides power to the sensor
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - meas,ms5803
+              - meas,ms5803-01ba
+              - meas,ms5803-02ba
+              - meas,ms5803-05ba
+              - meas,ms5803-14ba
+              - meas,ms5803-30ba
+    then:
+      $ref: /schemas/spi/spi-peripheral-props.yaml#
+      properties:
+        spi-max-frequency:
+          maximum: 20000000
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pressure@76 {
+            compatible = "meas,ms5637-02ba";
+            reg = <0x76>;
+            vdd-supply = <&vcc_3v3>;
+        };
+    };
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pressure@0 {
+            compatible = "meas,ms5803-14ba";
+            reg = <0>;
+            vdd-supply = <&vcc_3v3>;
+            spi-max-frequency = <20000000>;
+        };
+    };
diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index 23fd4513933a..a1516ab50a40 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -232,18 +232,8 @@ properties:
           - meas,htu21
             # Measurement Specialities I2C temperature and humidity sensor
           - meas,htu31
-            # Measurement Specialities I2C pressure and temperature sensor
-          - meas,ms5637
-            # Measurement Specialities I2C pressure and temperature sensor
-          - meas,ms5803
-            # Measurement Specialities I2C pressure and temperature sensor
-          - meas,ms5805
-            # Measurement Specialities I2C pressure and temperature sensor
-          - meas,ms5837
             # Measurement Specialities temp and humidity part of ms8607 device
           - meas,ms8607-humidity
-            # Measurement Specialities temp and pressure part of ms8607 device
-          - meas,ms8607-temppressure
             # Measurement Specialties temperature sensor
           - meas,tsys01
             # MEMSIC magnetometer
-- 
2.54.0

[PATCH v2 2/2] iio: pressure: ms5637: Add variant specific temperature compensation

[Louis Adamian]

Add correct temperature compensation for ms5637-30BA, MS5803-01BA,02BA,
05BA, 14BA, 30BA, MS5837-30BA. The temperature compensation formula is
shared across these sensors but with different constants. Add
ms_tp_comp_consts to capture these per-device differences. Add pressure
variant specific pressure scale variable.

Signed-off-by: Louis Adamian <adamianlouis@gmail.com>
---
 .../iio/common/ms_sensors/ms_sensors_i2c.c    |  62 ++-
 .../iio/common/ms_sensors/ms_sensors_i2c.h    |  48 +++
 drivers/iio/pressure/ms5637.c                 | 388 +++++++++++++++++-
 3 files changed, 465 insertions(+), 33 deletions(-)

diff --git a/drivers/iio/common/ms_sensors/ms_sensors_i2c.c b/drivers/iio/common/ms_sensors/ms_sensors_i2c.c
index 1960a2ce82a8..d5e217d7ba0b 100644
--- a/drivers/iio/common/ms_sensors/ms_sensors_i2c.c
+++ b/drivers/iio/common/ms_sensors/ms_sensors_i2c.c
@@ -604,6 +604,39 @@ int ms_sensors_tp_read_prom(struct ms_tp_dev *dev_data)
 }
 EXPORT_SYMBOL_NS(ms_sensors_tp_read_prom, "IIO_MEAS_SPEC_SENSORS");
 
+/* apply second order temperature compensation */
+static void ms_tp_compensate(const struct ms_tp_comp_consts *c,
+			     s32 temp, s32 dt, s64 *t2, s64 *off2, s64 *sens2)
+{
+	if (temp < 2000) {
+		s64 tmp = (s64)temp - 2000;
+
+		*t2 = (c->low_t2_multiplier * ((s64)dt * (s64)dt)) >>
+		      c->low_t2_shift;
+		*off2 = (c->low_off2_multiplier * tmp * tmp) >>
+			c->low_off2_shift;
+		*sens2 = (c->low_sens2_multiplier * tmp * tmp) >>
+			 c->low_sens2_shift;
+
+		if (temp < -1500) {
+			s64 tmp_vlow = (s64)temp + 1500;
+			*off2 += c->vlow_off2_multiplier * tmp_vlow * tmp_vlow;
+			*sens2 +=
+				c->vlow_sens2_multiplier * tmp_vlow * tmp_vlow;
+		}
+	} else {
+		*sens2 = 0;
+		if (c->has_vhigh_temp && temp > 4500)
+			*sens2 -= (((s64)temp - 4500) * ((s64)temp - 4500)) >> 3;
+
+		*t2 = (c->high_t2_multiplier * ((s64)dt * (s64)dt)) >>
+		      c->high_t2_shift;
+		*off2 = (c->high_off2_multiplier *
+			 ((s64)temp - 2000) * ((s64)temp - 2000)) >>
+			c->high_off2_shift;
+	}
+}
+
 /**
  * ms_sensors_read_temp_and_pressure() - read temp and pressure
  * @dev_data:	pointer to temperature/pressure device data
@@ -624,6 +657,7 @@ int ms_sensors_read_temp_and_pressure(struct ms_tp_dev *dev_data,
 	s32 dt, temp;
 	s64 off, sens, t2, off2, sens2;
 	u16 *prom = dev_data->prom, delay;
+	const struct ms_tp_comp_consts *c = dev_data->comp_consts;
 
 	mutex_lock(&dev_data->lock);
 	delay = ms_sensors_tp_conversion_time[dev_data->res_index];
@@ -654,37 +688,21 @@ int ms_sensors_read_temp_and_pressure(struct ms_tp_dev *dev_data,
 	/* Actual temperature = 2000 + dT * TEMPSENS */
 	temp = 2000 + (((s64)dt * prom[6]) >> 23);
 
-	/* Second order temperature compensation */
-	if (temp < 2000) {
-		s64 tmp = (s64)temp - 2000;
-
-		t2 = (3 * ((s64)dt * (s64)dt)) >> 33;
-		off2 = (61 * tmp * tmp) >> 4;
-		sens2 = (29 * tmp * tmp) >> 4;
-
-		if (temp < -1500) {
-			s64 tmp = (s64)temp + 1500;
-
-			off2 += 17 * tmp * tmp;
-			sens2 += 9 * tmp * tmp;
-		}
-	} else {
-		t2 = (5 * ((s64)dt * (s64)dt)) >> 38;
-		off2 = 0;
-		sens2 = 0;
-	}
+	ms_tp_compensate(c, temp, dt, &t2, &off2, &sens2);
 
 	/* OFF = OFF_T1 + TCO * dT */
-	off = (((s64)prom[2]) << 17) + ((((s64)prom[4]) * (s64)dt) >> 6);
+	off = (((s64)prom[2]) << c->off_t1_shift) +
+	      ((((s64)prom[4]) * (s64)dt) >> c->off_shift);
 	off -= off2;
 
 	/* Sensitivity at actual temperature = SENS_T1 + TCS * dT */
-	sens = (((s64)prom[1]) << 16) + (((s64)prom[3] * dt) >> 7);
+	sens = (((s64)prom[1]) << c->sens_t1_shift) +
+	       (((s64)prom[3] * dt) >> c->sens_shift);
 	sens -= sens2;
 
 	/* Temperature compensated pressure = D1 * SENS - OFF */
 	*temperature = (temp - t2) * 10;
-	*pressure = (u32)(((((s64)p_adc * sens) >> 21) - off) >> 15);
+	*pressure = (u32)(((((s64)p_adc * sens) >> c->press_sens_shift) - off) >> c->press_shift);
 
 	return 0;
 }
diff --git a/drivers/iio/common/ms_sensors/ms_sensors_i2c.h b/drivers/iio/common/ms_sensors/ms_sensors_i2c.h
index f15b973f27c6..0d3b68e03c8f 100644
--- a/drivers/iio/common/ms_sensors/ms_sensors_i2c.h
+++ b/drivers/iio/common/ms_sensors/ms_sensors_i2c.h
@@ -35,6 +35,52 @@ struct ms_tp_hw_data {
 	u8 max_res_index;
 };
 
+/**
+ * struct ms_tp_comp_consts - Temperature compensation constants
+ * @press_scale: pressure scale
+ * @high_t2_multiplier: multiplier for t2 in high temperature state
+ * @high_t2_shift: bit shift for t2 in high temperature state
+ * @high_off2_multiplier: multiplier for off2 in high temperature state
+ * @high_off2_shift: bit shift for off2 in high temperature state
+ * @low_t2_multiplier: multiplier for t2 in low temperature state
+ * @low_t2_shift: bit shift for t2 in low temperature state
+ * @low_off2_multiplier: multiplier for off2 in low temperature state
+ * @low_off2_shift: bit shift for off2 in low temperature state
+ * @low_sens2_multiplier: multiplier for sens2 in low temperature state
+ * @low_sens2_shift: bit shift for sens2 in low temperature state
+ * @vlow_off2_multiplier: multiplier for value added to off2 in very low temperature state
+ * @vlow_sens2_multiplier: multiplier for value added to sens2 in very low temperature state
+ * @has_vhigh_temp: has very high temperature compensation logic
+ * @off_t1_shift: temperature offset t1 bit shift
+ * @off_shift: temperature offset shift
+ * @sens_t1_shift: temperature sensitivity t1 shift
+ * @sens_shift: temperature sensitivity shift
+ * @press_sens_shift: pressure sensitivity shift
+ * @press_shift: pressure shift
+ */
+struct ms_tp_comp_consts {
+	u32 press_scale;
+	u8 high_t2_multiplier;
+	u8 high_t2_shift;
+	u8 high_off2_multiplier;
+	u8 high_off2_shift;
+	u8 low_t2_multiplier;
+	u8 low_t2_shift;
+	u8 low_off2_multiplier;
+	u8 low_off2_shift;
+	u8 low_sens2_multiplier;
+	u8 low_sens2_shift;
+	u8 vlow_off2_multiplier;
+	u8 vlow_sens2_multiplier;
+	bool has_vhigh_temp;
+	u8 off_t1_shift;
+	u8 off_shift;
+	u8 sens_t1_shift;
+	u8 sens_shift;
+	u8 press_sens_shift;
+	u8 press_shift;
+};
+
 /**
  * struct ms_tp_dev - Temperature/Pressure sensor device structure
  * @client:	i2c client
@@ -42,6 +88,7 @@ struct ms_tp_hw_data {
  * @prom:	array of PROM coefficients used for conversion. Added element
  *              for CRC computation
  * @res_index:	index to selected sensor resolution
+ * @comp_consts: temperature compensation constants
  */
 struct ms_tp_dev {
 	struct i2c_client *client;
@@ -49,6 +96,7 @@ struct ms_tp_dev {
 	const struct ms_tp_hw_data *hw;
 	u16 prom[MS_SENSORS_TP_PROM_WORDS_NB];
 	u8 res_index;
+	const struct ms_tp_comp_consts *comp_consts;
 };
 
 int ms_sensors_reset(void *cli, u8 cmd, unsigned int delay);
diff --git a/drivers/iio/pressure/ms5637.c b/drivers/iio/pressure/ms5637.c
index 03945a4fc718..36bda9694b4c 100644
--- a/drivers/iio/pressure/ms5637.c
+++ b/drivers/iio/pressure/ms5637.c
@@ -33,6 +33,7 @@
 struct ms_tp_data {
 	const char *name;
 	const struct ms_tp_hw_data *hw;
+	const struct ms_tp_comp_consts *comp_consts;
 };
 
 static const int ms5637_samp_freq[6] = { 960, 480, 240, 120, 60, 30 };
@@ -73,8 +74,10 @@ static int ms5637_read_raw(struct iio_dev *indio_dev,
 
 			return IIO_VAL_INT;
 		case IIO_PRESSURE:	/* in kPa */
-			*val = pressure / 1000;
-			*val2 = (pressure % 1000) * 1000;
+			*val = pressure / dev_data->comp_consts->press_scale;
+			*val2 = (pressure %
+				 (s64)dev_data->comp_consts->press_scale) *
+				(1000000 / dev_data->comp_consts->press_scale);
 
 			return IIO_VAL_INT_PLUS_MICRO;
 		default:
@@ -171,6 +174,7 @@ static int ms5637_probe(struct i2c_client *client)
 	dev_data->res_index = data->hw->max_res_index;
 	dev_data->hw = data->hw;
 	mutex_init(&dev_data->lock);
+	dev_data->comp_consts = data->comp_consts;
 
 	indio_dev->info = &ms5637_info;
 	indio_dev->name = data->name;
@@ -201,33 +205,395 @@ static const struct ms_tp_hw_data ms5803_hw_data  = {
 	.max_res_index = 4
 };
 
-static const struct ms_tp_data ms5637_data = { .name = "ms5637", .hw = &ms5637_hw_data };
+/*
+ * MS5637-02BA03 compensation constants
+ * Datasheet: https://www.te.com/commerce/DocumentDelivery/DDEController?Action=showdoc&DocId=Data+Sheet%7FMS5637-02BA03%7FB1%7Fpdf%7FEnglish%7FENG_DS_MS5637-02BA03_B1.pdf
+ * Pages: 8-9
+ * Sections: Pressure and Temperature Calculation, Second Order Temperature Compensation
+ */
+static const struct ms_tp_comp_consts ms5637_02_consts = {
+	.press_scale = 1000,
+	.high_t2_multiplier = 5,
+	.high_t2_shift = 38,
+	.high_off2_multiplier = 0,
+	.high_off2_shift = 0,
+	.low_t2_multiplier = 3,
+	.low_t2_shift = 33,
+	.low_off2_multiplier = 61,
+	.low_off2_shift = 4,
+	.low_sens2_multiplier = 29,
+	.low_sens2_shift = 4,
+	.vlow_off2_multiplier = 17,
+	.vlow_sens2_multiplier = 9,
+	.has_vhigh_temp = false,
+	.off_t1_shift = 17,
+	.off_shift = 6,
+	.sens_t1_shift = 16,
+	.sens_shift = 7,
+	.press_sens_shift = 21,
+	.press_shift = 15,
+};
+
+static const struct ms_tp_data ms5637_02_data = {
+	.name = "ms5637-02ba",
+	.hw = &ms5637_hw_data,
+	.comp_consts = &ms5637_02_consts,
+};
+
+/*
+ * MS5637-30BA compensation constants
+ * Datasheet: https://www.te.com/commerce/DocumentDelivery/DDEController?Action=showdoc&DocId=Data+Sheet%7FMS5637-30BA%7FA3%7Fpdf%7FEnglish%7FENG_DS_MS5637-30BA_A3.pdf
+ * Pages: 8-9
+ * Sections: Pressure and Temperature Calculation, Second Order Temperature Compensation
+ */
+static const struct ms_tp_comp_consts ms5637_30_consts = {
+	.press_scale = 100,
+	.high_t2_multiplier = 2,
+	.high_t2_shift = 37,
+	.high_off2_multiplier = 1,
+	.high_off2_shift = 4,
+	.low_t2_multiplier = 3,
+	.low_t2_shift = 33,
+	.low_off2_multiplier = 3,
+	.low_off2_shift = 1,
+	.low_sens2_multiplier = 5,
+	.low_sens2_shift = 3,
+	.vlow_off2_multiplier = 7,
+	.vlow_sens2_multiplier = 4,
+	.has_vhigh_temp = false,
+	.off_t1_shift = 16,
+	.off_shift = 7,
+	.sens_t1_shift = 15,
+	.sens_shift = 8,
+	.press_sens_shift = 21,
+	.press_shift = 13,
+};
 
-static const struct ms_tp_data ms5803_data = { .name = "ms5803", .hw = &ms5803_hw_data };
+static const struct ms_tp_data ms5637_30_data = {
+	.name = "ms5637-30ba",
+	.hw = &ms5637_hw_data,
+	.comp_consts = &ms5637_30_consts,
+};
 
-static const struct ms_tp_data ms5805_data = { .name = "ms5805", .hw = &ms5637_hw_data };
+/*
+ * MS5803-01BA compensation Constants
+ * Datasheet: https://www.te.com/commerce/DocumentDelivery/DDEController?Action=srchrtrv&DocNm=MS5803-01BA&DocType=Data%20Sheet&DocLang=English&DocFormat=pdf
+ * Pages: 13-14
+ * Sections: Pressure and Temperature Calculation, Second Order Temperature Compensation
+ */
+static const struct ms_tp_comp_consts ms5803_01_consts = {
+	.press_scale = 1000,
+	.high_t2_multiplier = 0,
+	.high_t2_shift = 0,
+	.high_off2_multiplier = 0,
+	.high_off2_shift = 0,
+	.low_t2_multiplier = 1,
+	.low_t2_shift = 31,
+	.low_off2_multiplier = 3,
+	.low_off2_shift = 0,
+	.low_sens2_multiplier = 7,
+	.low_sens2_shift = 3,
+	.vlow_off2_multiplier = 0,
+	.vlow_sens2_multiplier = 2,
+	.has_vhigh_temp = true,
+	.off_t1_shift = 16,
+	.off_shift = 7,
+	.sens_t1_shift = 15,
+	.sens_shift = 8,
+	.press_sens_shift = 21,
+	.press_shift = 15,
+};
 
-static const struct ms_tp_data ms5837_data = { .name = "ms5837", .hw = &ms5637_hw_data };
+static const struct ms_tp_data ms5803_01_data = {
+	.name = "ms5803-01ba",
+	.hw = &ms5803_hw_data,
+	.comp_consts = &ms5803_01_consts,
+};
+
+/*
+ * MS5803-02BA compensation constants
+ * Datasheet: https://www.te.com/commerce/DocumentDelivery/DDEController?Action=srchrtrv&DocNm=MS5803-02BA&DocType=Data%20Sheet&DocLang=English&DocFormat=pdf
+ * Pages: 13-14
+ * Sections: Pressure and Temperature Calculation, Second Order Temperature Compensation
+ */
+static const struct ms_tp_comp_consts ms5803_02_consts = {
+	.press_scale = 1000,
+	.high_t2_multiplier = 0,
+	.high_t2_shift = 0,
+	.high_off2_multiplier = 0,
+	.high_off2_shift = 0,
+	.low_t2_multiplier = 1,
+	.low_t2_shift = 31,
+	.low_off2_multiplier = 61,
+	.low_off2_shift = 4,
+	.low_sens2_multiplier = 2,
+	.low_sens2_shift = 0,
+	.vlow_off2_multiplier = 20,
+	.vlow_sens2_multiplier = 12,
+	.has_vhigh_temp = false,
+	.off_t1_shift = 17,
+	.off_shift = 6,
+	.sens_t1_shift = 16,
+	.sens_shift = 7,
+	.press_sens_shift = 21,
+	.press_shift = 15,
+};
+
+static const struct ms_tp_data ms5803_02_data = {
+	.name = "ms5803-02ba",
+	.hw = &ms5803_hw_data,
+	.comp_consts = &ms5803_02_consts,
+};
+
+/*
+ * MS5803-05BA compensation constants
+ * Datasheet: https://www.te.com/commerce/DocumentDelivery/DDEController?Action=srchrtrv&DocNm=MS5803-05BA&DocType=Data%20Sheet&DocLang=English&DocFormat=pdf&PartCntxt=MS580305BA01-00
+ * Pages: 13-14
+ * Sections: Pressure and Temperature Calculation, Second Order Temperature Compensation
+ */
+static const struct ms_tp_comp_consts ms5803_05_consts = {
+	.press_scale = 1000,
+	.high_t2_multiplier = 0,
+	.high_t2_shift = 0,
+	.high_off2_multiplier = 0,
+	.high_off2_shift = 0,
+	.low_t2_multiplier = 3,
+	.low_t2_shift = 33,
+	.low_off2_multiplier = 3,
+	.low_off2_shift = 3,
+	.low_sens2_multiplier = 7,
+	.low_sens2_shift = 3,
+	.vlow_off2_multiplier = 0,
+	.vlow_sens2_multiplier = 3,
+	.has_vhigh_temp = false,
+	.off_t1_shift = 18,
+	.off_shift = 5,
+	.sens_t1_shift = 17,
+	.sens_shift = 7,
+	.press_sens_shift = 21,
+	.press_shift = 15,
+};
+
+static const struct ms_tp_data ms5803_05_data = {
+	.name = "ms5803-05ba",
+	.hw = &ms5803_hw_data,
+	.comp_consts = &ms5803_05_consts,
+};
+
+/*
+ * MS5803-14BA compensation constants
+ * Datasheet: https://www.te.com/commerce/DocumentDelivery/DDEController?Action=srchrtrv&DocNm=MS5803-14BA&DocType=Data%20Sheet&DocLang=English&DocFormat=pdf&PartCntxt=MS580314BA01-50
+ * Pages: 13-14
+ * Sections: Pressure and Temperature Calculation, Second Order Temperature Compensation
+ */
+static const struct ms_tp_comp_consts ms5803_14_consts = {
+	.press_scale = 100,
+	.high_t2_multiplier = 7,
+	.high_t2_shift = 37,
+	.high_off2_multiplier = 1,
+	.high_off2_shift = 4,
+	.low_t2_multiplier = 3,
+	.low_t2_shift = 33,
+	.low_off2_multiplier = 3,
+	.low_off2_shift = 1,
+	.low_sens2_multiplier = 5,
+	.low_sens2_shift = 3,
+	.vlow_off2_multiplier = 7,
+	.vlow_sens2_multiplier = 4,
+	.has_vhigh_temp = false,
+	.off_t1_shift = 16,
+	.off_shift = 7,
+	.sens_t1_shift = 15,
+	.sens_shift = 8,
+	.press_sens_shift = 21,
+	.press_shift = 15,
+};
+
+static const struct ms_tp_data ms5803_14_data = {
+	.name = "ms5803-14ba",
+	.hw = &ms5803_hw_data,
+	.comp_consts = &ms5803_14_consts,
+};
+
+/*
+ * MS5803-30BA compensation constants
+ * Datasheet: https://www.te.com/commerce/DocumentDelivery/DDEController?Action=srchrtrv&DocNm=MS5803-30BA&DocType=Data%20Sheet&DocLang=English&DocFormat=pdf&PartCntxt=MS580330BA01-00
+ * Pages: 13-14
+ * Sections: Pressure and Temperature Calculation, Second Order Temperature Compensation
+ */
+static const struct ms_tp_comp_consts ms5803_30_consts = {
+	.press_scale = 100,
+	.high_t2_multiplier = 7,
+	.high_t2_shift = 37,
+	.high_off2_multiplier = 1,
+	.high_off2_shift = 4,
+	.low_t2_multiplier = 3,
+	.low_t2_shift = 33,
+	.low_off2_multiplier = 3,
+	.low_off2_shift = 1,
+	.low_sens2_multiplier = 5,
+	.low_sens2_shift = 3,
+	.vlow_off2_multiplier = 7,
+	.vlow_sens2_multiplier = 4,
+	.has_vhigh_temp = false,
+	.off_t1_shift = 16,
+	.off_shift = 7,
+	.sens_t1_shift = 15,
+	.sens_shift = 8,
+	.press_sens_shift = 21,
+	.press_shift = 13,
+};
+
+static const struct ms_tp_data ms5803_30_data = {
+	.name = "ms5803-30ba",
+	.hw = &ms5803_hw_data,
+	.comp_consts = &ms5803_30_consts,
+};
+
+/*
+ * MS5805-02BA01 compensation constants
+ * Datasheet: https://www.te.com/commerce/DocumentDelivery/DDEController?Action=srchrtrv&DocNm=MS5805-02BA01&DocType=Data%20Sheet&DocLang=English&DocFormat=pdf&PartCntxt=MS580502BA01-50
+ * Pages: 8-9
+ * Sections: Pressure and Temperature Calculation, Second Order Temperature Compensation
+ */
+static const struct ms_tp_comp_consts ms5805_consts = {
+	.press_scale = 1000,
+	.high_t2_multiplier = 0,
+	.high_t2_shift = 0,
+	.high_off2_multiplier = 0,
+	.high_off2_shift = 0,
+	.low_t2_multiplier = 11,
+	.low_t2_shift = 35,
+	.low_off2_multiplier = 31,
+	.low_off2_shift = 3,
+	.low_sens2_multiplier = 63,
+	.low_sens2_shift = 5,
+	.vlow_off2_multiplier = 0,
+	.vlow_sens2_multiplier = 0,
+	.has_vhigh_temp = false,
+	.off_t1_shift = 17,
+	.off_shift = 6,
+	.sens_t1_shift = 16,
+	.sens_shift = 7,
+	.press_sens_shift = 21,
+	.press_shift = 15,
+};
+
+static const struct ms_tp_data ms5805_data = {
+	.name = "ms5805",
+	.hw = &ms5637_hw_data,
+	.comp_consts = &ms5805_consts,
+};
+
+/*
+ * MS5837-02BA compensation constants
+ * Datasheet: https://www.te.com/commerce/DocumentDelivery/DDEController?Action=srchrtrv&DocNm=MS5837-02BA01&DocType=Data%20Sheet&DocLang=English&DocFormat=pdf&PartCntxt=20000979-00
+ * Pages: 7-8
+ * Sections: Pressure and Temperature Calculation, Second Order Temperature Compensation
+ */
+static const struct ms_tp_comp_consts ms5837_02_consts = {
+	.press_scale = 1000,
+	.high_t2_multiplier = 0,
+	.high_t2_shift = 0,
+	.high_off2_multiplier = 0,
+	.high_off2_shift = 0,
+	.low_t2_multiplier = 11,
+	.low_t2_shift = 35,
+	.low_off2_multiplier = 31,
+	.low_off2_shift = 3,
+	.low_sens2_multiplier = 63,
+	.low_sens2_shift = 5,
+	.vlow_off2_multiplier = 0,
+	.vlow_sens2_multiplier = 0,
+	.has_vhigh_temp = false,
+	.off_t1_shift = 17,
+	.off_shift = 6,
+	.sens_t1_shift = 16,
+	.sens_shift = 7,
+	.press_sens_shift = 21,
+	.press_shift = 15,
+};
+
+static const struct ms_tp_data ms5837_02_data = {
+	.name = "ms5837-02ba",
+	.hw = &ms5637_hw_data,
+	.comp_consts = &ms5837_02_consts,
+};
 
 static const struct ms_tp_data ms8607_data = {
 	.name = "ms8607-temppressure",
 	.hw = &ms5637_hw_data,
+	.comp_consts = &ms5637_02_consts,
+};
+
+/*
+ * MS5837-30BA compensation constants
+ * Datasheet: https://www.te.com/commerce/DocumentDelivery/DDEController?Action=srchrtrv&DocNm=MS5837-30BA&DocType=Data%20Sheet&DocLang=English&DocFormat=pdf&PartCntxt=MS583730BA01-50
+ * Pages: 11-12
+ * Sections: Pressure and Temperature Calculation, Second Order Temperature Compensation
+ */
+static const struct ms_tp_comp_consts ms5837_30_consts = {
+	.press_scale = 100,
+	.high_t2_multiplier = 7,
+	.high_t2_shift = 37,
+	.high_off2_multiplier = 1,
+	.high_off2_shift = 4,
+	.low_t2_multiplier = 3,
+	.low_t2_shift = 33,
+	.low_off2_multiplier = 3,
+	.low_off2_shift = 1,
+	.low_sens2_multiplier = 5,
+	.low_sens2_shift = 3,
+	.vlow_off2_multiplier = 7,
+	.vlow_sens2_multiplier = 4,
+	.has_vhigh_temp = false,
+	.off_t1_shift = 16,
+	.off_shift = 7,
+	.sens_t1_shift = 15,
+	.sens_shift = 8,
+	.press_sens_shift = 21,
+	.press_shift = 13,
+};
+
+static const struct ms_tp_data ms5837_30_data = {
+	.name = "ms5837-30ba",
+	.hw = &ms5637_hw_data,
+	.comp_consts = &ms5837_30_consts,
 };
 
 static const struct i2c_device_id ms5637_id[] = {
-	{ .name = "ms5637", .driver_data = (kernel_ulong_t)&ms5637_data },
+	{ .name = "ms5637", .driver_data = (kernel_ulong_t)&ms5637_02_data },
+	{ .name = "ms5637-02ba", .driver_data = (kernel_ulong_t)&ms5637_02_data },
+	{ .name = "ms5637-30ba", .driver_data = (kernel_ulong_t)&ms5637_30_data },
+	{ .name = "ms5803", .driver_data = (kernel_ulong_t)&ms5803_02_data },
+	{ .name = "ms5803-01ba", .driver_data = (kernel_ulong_t)&ms5803_01_data },
+	{ .name = "ms5803-02ba", .driver_data = (kernel_ulong_t)&ms5803_02_data },
+	{ .name = "ms5803-05ba", .driver_data = (kernel_ulong_t)&ms5803_05_data },
+	{ .name = "ms5803-14ba", .driver_data = (kernel_ulong_t)&ms5803_14_data },
+	{ .name = "ms5803-30ba", .driver_data = (kernel_ulong_t)&ms5803_30_data },
 	{ .name = "ms5805", .driver_data = (kernel_ulong_t)&ms5805_data },
-	{ .name = "ms5837", .driver_data = (kernel_ulong_t)&ms5837_data },
+	{ .name = "ms5837", .driver_data = (kernel_ulong_t)&ms5837_02_data },
+	{ .name = "ms5837-02ba", .driver_data = (kernel_ulong_t)&ms5837_02_data },
+	{ .name = "ms5837-30ba", .driver_data = (kernel_ulong_t)&ms5837_30_data },
 	{ .name = "ms8607-temppressure", .driver_data = (kernel_ulong_t)&ms8607_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ms5637_id);
 
 static const struct of_device_id ms5637_of_match[] = {
-	{ .compatible = "meas,ms5637", .data = &ms5637_data },
-	{ .compatible = "meas,ms5803", .data = &ms5803_data },
+	{ .compatible = "meas,ms5637", .data = &ms5637_02_data },
+	{ .compatible = "meas,ms5637-02ba", .data = &ms5637_02_data },
+	{ .compatible = "meas,ms5637-30ba", .data = &ms5637_30_data },
+	{ .compatible = "meas,ms5803", .data = &ms5803_02_data },
+	{ .compatible = "meas,ms5803-01ba", .data = &ms5803_01_data },
+	{ .compatible = "meas,ms5803-02ba", .data = &ms5803_02_data },
+	{ .compatible = "meas,ms5803-05ba", .data = &ms5803_05_data },
+	{ .compatible = "meas,ms5803-14ba", .data = &ms5803_14_data },
+	{ .compatible = "meas,ms5803-30ba", .data = &ms5803_30_data },
 	{ .compatible = "meas,ms5805", .data = &ms5805_data },
-	{ .compatible = "meas,ms5837", .data = &ms5837_data },
+	{ .compatible = "meas,ms5837", .data = &ms5837_02_data },
+	{ .compatible = "meas,ms5837-02ba", .data = &ms5837_02_data },
+	{ .compatible = "meas,ms5837-30ba", .data = &ms5837_30_data },
 	{ .compatible = "meas,ms8607-temppressure", .data = &ms8607_data },
 	{ }
 };
-- 
2.54.0

Re: [PATCH v2 1/2] dt-bindings: iio: pressure: Add MS5637

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 809 bytes --]

On Tue, Jun 09, 2026 at 10:04:57PM -0400, Louis Adamian wrote:
> Create meas,ms5637 devicetree binding and move existing devices from
> trivial-devices.yaml. Explicit compatible strings are added for the
> different sub-variants because each requires different constants for the
> second-order temperature compensation formulas. Previously, the driver
> exposed the generic compatibles for the ms5637, MS5803, MS5805
> and MS5837 families and applied the MS5637-02BA compensation to all of
> them.To preserve the DT ABI the existing compatibles are retained and
> mapped to the -02BA variant of each family (ms5637-02ba, ms5803-02ba,
> ms5837-02ba).

I think this should be made clear by use of fallbacks, or being marked
deprecated with a comment.

pw-bot: changes-requested

Otherwise, it looks okay to me.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/2] iio: pressure: ms5637: Add variant specific temperature compensation

[Andy Shevchenko]

On Tue, Jun 09, 2026 at 10:04:58PM -0400, Louis Adamian wrote:
> Add correct temperature compensation for ms5637-30BA, MS5803-01BA,02BA,
> 05BA, 14BA, 30BA, MS5837-30BA. The temperature compensation formula is
> shared across these sensors but with different constants. Add
> ms_tp_comp_consts to capture these per-device differences. Add pressure
> variant specific pressure scale variable.

Is there SPI driver? If so, why only i2c is affected?

...

> +/* apply second order temperature compensation */
> +static void ms_tp_compensate(const struct ms_tp_comp_consts *c,
> +			     s32 temp, s32 dt, s64 *t2, s64 *off2, s64 *sens2)
> +{
> +	if (temp < 2000) {
> +		s64 tmp = (s64)temp - 2000;

Why casting?

> +
> +		*t2 = (c->low_t2_multiplier * ((s64)dt * (s64)dt)) >>
> +		      c->low_t2_shift;
> +		*off2 = (c->low_off2_multiplier * tmp * tmp) >>
> +			c->low_off2_shift;
> +		*sens2 = (c->low_sens2_multiplier * tmp * tmp) >>
> +			 c->low_sens2_shift;


> +		if (temp < -1500) {
> +			s64 tmp_vlow = (s64)temp + 1500;

Missing blank line. But Q here is why tmp can't be used here?

> +			*off2 += c->vlow_off2_multiplier * tmp_vlow * tmp_vlow;
> +			*sens2 +=
> +				c->vlow_sens2_multiplier * tmp_vlow * tmp_vlow;

Make it one line.

> +		}
> +	} else {
> +		*sens2 = 0;
> +		if (c->has_vhigh_temp && temp > 4500)
> +			*sens2 -= (((s64)temp - 4500) * ((s64)temp - 4500)) >> 3;

Here...

> +		*t2 = (c->high_t2_multiplier * ((s64)dt * (s64)dt)) >>
> +		      c->high_t2_shift;
> +		*off2 = (c->high_off2_multiplier *
> +			 ((s64)temp - 2000) * ((s64)temp - 2000)) >>

...and here you may also use tmp, just make it global to the function.

> +			c->high_off2_shift;
> +	}
> +}

Overall this all needs a good comment or even comments to explain all
calculations with the references to the respective sections / tables / pages
in the datasheet.

...

> int ms_sensors_read_temp_and_pressure(struct ms_tp_dev *dev_data,

>  	s32 dt, temp;
>  	s64 off, sens, t2, off2, sens2;
>  	u16 *prom = dev_data->prom, delay;
> +	const struct ms_tp_comp_consts *c = dev_data->comp_consts;

Try to keep more or less reversed xmas tree order.

...

>   * struct ms_tp_dev - Temperature/Pressure sensor device structure
>   * @client:	i2c client

>   * @prom:	array of PROM coefficients used for conversion. Added element
>   *              for CRC computation
>   * @res_index:	index to selected sensor resolution
> + * @comp_consts: temperature compensation constants
>   */
>  struct ms_tp_dev {
>  	struct i2c_client *client;

>  	const struct ms_tp_hw_data *hw;
>  	u16 prom[MS_SENSORS_TP_PROM_WORDS_NB];
>  	u8 res_index;
> +	const struct ms_tp_comp_consts *comp_consts;

Please, check with `pahole` if this is the best layout.

>  };

>  struct ms_tp_data {
>  	const char *name;
>  	const struct ms_tp_hw_data *hw;
> +	const struct ms_tp_comp_consts *comp_consts;
>  };

Can this be simply embedded into ms_tp_dev (and copied there if required)?

...

>  		case IIO_PRESSURE:	/* in kPa */
> -			*val = pressure / 1000;
> -			*val2 = (pressure % 1000) * 1000;
> +			*val = pressure / dev_data->comp_consts->press_scale;
> +			*val2 = (pressure %
> +				 (s64)dev_data->comp_consts->press_scale) *
> +				(1000000 / dev_data->comp_consts->press_scale);

MICRO (might need units.h)?

>  			return IIO_VAL_INT_PLUS_MICRO;

...

Have you considered to prepare the infrastructure in one patch and add
the actual compensation data tables in another?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] dt-bindings: iio: pressure: Add MS5637

[Louis Adamian]

On Wed, 10 Jun 2026 17:45:52 +0100
Conor Dooley <conor@kernel.org> wrote:
> I think this should be made clear by use of fallbacks, or being marked
> deprecated with a comment.
I will mark the generic compatibles as deprecated with a comment noting
that they previously all received the ms5637-02ba compensation
(following solomon,ssd1307fb.yaml).

Thanks,
Louis

Re: [PATCH v2 2/2] iio: pressure: ms5637: Add variant specific temperature compensation

[Jonathan Cameron]

On Wed, 10 Jun 2026 21:37:58 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Tue, Jun 09, 2026 at 10:04:58PM -0400, Louis Adamian wrote:
> > Add correct temperature compensation for ms5637-30BA, MS5803-01BA,02BA,
> > 05BA, 14BA, 30BA, MS5837-30BA. The temperature compensation formula is
> > shared across these sensors but with different constants. Add
> > ms_tp_comp_consts to capture these per-device differences. Add pressure
> > variant specific pressure scale variable.  
> 
> Is there SPI driver? If so, why only i2c is affected?

Excellent question.  I went digging.

The library is used by the tsys01 driver which has both SPI and I2C interfaces.
Seems the SPI side never got added.  The particular driver being modified here
is I2C only and the modified library function is only used in that driver.

Feels like maybe we should revisit the division of library and driver, but
probably not as part of this series!

I also too a quick look through and with the stuff Andy raised acted on this
looks fine to me.

Jonathan

Re: [PATCH v2 2/2] iio: pressure: ms5637: Add variant specific temperature compensation

[Louis Adamian]

On Wed, 10 Jun 2026 21:37:58 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Tue, Jun 09, 2026 at 10:04:58PM -0400, Louis Adamian wrote:
> > Add correct temperature compensation for ms5637-30BA,
> > MS5803-01BA,02BA, 05BA, 14BA, 30BA, MS5837-30BA. The temperature
> > compensation formula is shared across these sensors but with
> > different constants. Add ms_tp_comp_consts to capture these
> > per-device differences. Add pressure variant specific pressure
> > scale variable.  
> 
> Is there SPI driver? If so, why only i2c is affected?

As Jonathan noted, there is no SPI driver currently and the
modified function is only used by i2c. 

> > +		s64 tmp = (s64)temp - 2000;  
> 
> Why casting?

This is extraneous, I will remove it.

> Overall this all needs a good comment or even comments to explain all
> calculations with the references to the respective sections / tables
> / pages in the datasheet.

I'll add comments here explaining the shared equations. There are
also references to the individual datasheets in the constant structures
because they differ per device.

> >  struct ms_tp_dev {
> >  	struct i2c_client *client;  
> 
> >  	const struct ms_tp_hw_data *hw;
> >  	u16 prom[MS_SENSORS_TP_PROM_WORDS_NB];
> >  	u8 res_index;
> > +	const struct ms_tp_comp_consts *comp_consts;  
> 
> Please, check with `pahole` if this is the best layout.

I will correct the packing on this per pahole's finding.
> >  struct ms_tp_data {
> >  	const char *name;
> >  	const struct ms_tp_hw_data *hw;
> > +	const struct ms_tp_comp_consts *comp_consts;
> >  };  
> 
> Can this be simply embedded into ms_tp_dev (and copied there if
> required)?

Copying in ms_tp_comp_consts provides no benefit over
referencing the pointer; the values are per-descriptor, not
per-instance and never modified. I'll keep the pointer (same as
ms_tp_hw_data).

> Have you considered to prepare the infrastructure in one patch and add
> the actual compensation data tables in another?

I can split this patch for V3 and I will fix the other formatting
issues.

Thanks,
Louis

Re: [PATCH v2 2/2] iio: pressure: ms5637: Add variant specific temperature compensation

[Andy Shevchenko]

On Thu, Jun 11, 2026 at 03:10:26PM -0400, Louis Adamian wrote:
> On Wed, 10 Jun 2026 21:37:58 +0300
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > On Tue, Jun 09, 2026 at 10:04:58PM -0400, Louis Adamian wrote:

...

> > >  struct ms_tp_dev {
> > >  	struct i2c_client *client;  
> > 
> > >  	const struct ms_tp_hw_data *hw;
> > >  	u16 prom[MS_SENSORS_TP_PROM_WORDS_NB];
> > >  	u8 res_index;
> > > +	const struct ms_tp_comp_consts *comp_consts;  
> > 
> > Please, check with `pahole` if this is the best layout.
> 
> I will correct the packing on this per pahole's finding.
> > >  struct ms_tp_data {
> > >  	const char *name;
> > >  	const struct ms_tp_hw_data *hw;
> > > +	const struct ms_tp_comp_consts *comp_consts;
> > >  };  
> > 
> > Can this be simply embedded into ms_tp_dev (and copied there if
> > required)?
> 
> Copying in ms_tp_comp_consts provides no benefit over
> referencing the pointer; the values are per-descriptor, not
> per-instance and never modified. I'll keep the pointer (same as
> ms_tp_hw_data).

Okay, let me ask differently then: Can we use the same data type in ms_tp_dev
instead of adding basically the same fields as in ms_tp_data?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/2] iio: pressure: ms5637: Add variant specific temperature compensation

[Louis Adamian]

On Thu, 11 Jun 2026 22:32:02 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Thu, Jun 11, 2026 at 03:10:26PM -0400, Louis Adamian wrote:
> > On Wed, 10 Jun 2026 21:37:58 +0300
> > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:  
> > > On Tue, Jun 09, 2026 at 10:04:58PM -0400, Louis Adamian wrote:  
> 
> ...
> 
> > > >  struct ms_tp_dev {
> > > >  	struct i2c_client *client;    
> > >   
> > > >  	const struct ms_tp_hw_data *hw;
> > > >  	u16 prom[MS_SENSORS_TP_PROM_WORDS_NB];
> > > >  	u8 res_index;
> > > > +	const struct ms_tp_comp_consts *comp_consts;    
> > > 
> > > Please, check with `pahole` if this is the best layout.  
> > 
> > I will correct the packing on this per pahole's finding.  
> > > >  struct ms_tp_data {
> > > >  	const char *name;
> > > >  	const struct ms_tp_hw_data *hw;
> > > > +	const struct ms_tp_comp_consts *comp_consts;
> > > >  };    
> > > 
> > > Can this be simply embedded into ms_tp_dev (and copied there if
> > > required)?  
> > 
> > Copying in ms_tp_comp_consts provides no benefit over
> > referencing the pointer; the values are per-descriptor, not
> > per-instance and never modified. I'll keep the pointer (same as
> > ms_tp_hw_data).  
> 
> Okay, let me ask differently then: Can we use the same data type in
> ms_tp_dev instead of adding basically the same fields as in
> ms_tp_data?
> 
Sorry for the very slow reply. Yes, for v3 ms_tp_dev will hold a single
const ms_tp_data *data instead of its own hw and comp_consts. This
drops the duplicated hw that predates this series. 

One thing worth noting before I do that: ms_tp_data is currently
private to ms5637.c and will need to be moved to ms_sensors_i2c.h for
ms_tp_dev to point to it. I don't think this is a problem; it matches
how ms_tp_hw_data is defined now, but let me know if you would like to
avoid it.

Thanks,
Louis 

Re: [PATCH v2 2/2] iio: pressure: ms5637: Add variant specific temperature compensation

[Andy Shevchenko]

On Wed, Aug 05, 2026 at 01:06:51PM -0400, Louis Adamian wrote:
> On Thu, 11 Jun 2026 22:32:02 +0300
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > On Thu, Jun 11, 2026 at 03:10:26PM -0400, Louis Adamian wrote:
> > > On Wed, 10 Jun 2026 21:37:58 +0300
> > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:  
> > > > On Tue, Jun 09, 2026 at 10:04:58PM -0400, Louis Adamian wrote:  

...

> > > > >  struct ms_tp_dev {
> > > > >  	struct i2c_client *client;    
> > > >   
> > > > >  	const struct ms_tp_hw_data *hw;
> > > > >  	u16 prom[MS_SENSORS_TP_PROM_WORDS_NB];
> > > > >  	u8 res_index;
> > > > > +	const struct ms_tp_comp_consts *comp_consts;    
> > > > 
> > > > Please, check with `pahole` if this is the best layout.  
> > > 
> > > I will correct the packing on this per pahole's finding.  
> > > > >  struct ms_tp_data {
> > > > >  	const char *name;
> > > > >  	const struct ms_tp_hw_data *hw;
> > > > > +	const struct ms_tp_comp_consts *comp_consts;
> > > > >  };    
> > > > 
> > > > Can this be simply embedded into ms_tp_dev (and copied there if
> > > > required)?  
> > > 
> > > Copying in ms_tp_comp_consts provides no benefit over
> > > referencing the pointer; the values are per-descriptor, not
> > > per-instance and never modified. I'll keep the pointer (same as
> > > ms_tp_hw_data).  
> > 
> > Okay, let me ask differently then: Can we use the same data type in
> > ms_tp_dev instead of adding basically the same fields as in
> > ms_tp_data?
> > 
> Sorry for the very slow reply. Yes, for v3 ms_tp_dev will hold a single
> const ms_tp_data *data instead of its own hw and comp_consts. This
> drops the duplicated hw that predates this series. 
> 
> One thing worth noting before I do that: ms_tp_data is currently
> private to ms5637.c and will need to be moved to ms_sensors_i2c.h for
> ms_tp_dev to point to it. I don't think this is a problem; it matches
> how ms_tp_hw_data is defined now, but let me know if you would like to
> avoid it.

The pointer may be opaque, in that case it's not needed to be exposed.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/2] iio: pressure: ms5637: Add variant specific temperature compensation

[Louis Adamian]

On Fri, 7 Aug 2026 00:27:30 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Wed, Aug 05, 2026 at 01:06:51PM -0400, Louis Adamian wrote:
> > On Thu, 11 Jun 2026 22:32:02 +0300
> > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:  
> > > On Thu, Jun 11, 2026 at 03:10:26PM -0400, Louis Adamian wrote:  
> > > > On Wed, 10 Jun 2026 21:37:58 +0300
> > > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:    
> > > > > On Tue, Jun 09, 2026 at 10:04:58PM -0400, Louis Adamian
> > > > > wrote:    
> 
> ...
> 
> > > > > >  struct ms_tp_dev {
> > > > > >  	struct i2c_client *client;      
> > > > >     
>  [...]  
> > > > > 
> > > > > Please, check with `pahole` if this is the best layout.    
> > > > 
> > > > I will correct the packing on this per pahole's finding.    
>  [...]  
> > > > > 
> > > > > Can this be simply embedded into ms_tp_dev (and copied there
> > > > > if required)?    
> > > > 
> > > > Copying in ms_tp_comp_consts provides no benefit over
> > > > referencing the pointer; the values are per-descriptor, not
> > > > per-instance and never modified. I'll keep the pointer (same as
> > > > ms_tp_hw_data).    
> > > 
> > > Okay, let me ask differently then: Can we use the same data type
> > > in ms_tp_dev instead of adding basically the same fields as in
> > > ms_tp_data?
> > >   
> > Sorry for the very slow reply. Yes, for v3 ms_tp_dev will hold a
> > single const ms_tp_data *data instead of its own hw and
> > comp_consts. This drops the duplicated hw that predates this
> > series. 
> > 
> > One thing worth noting before I do that: ms_tp_data is currently
> > private to ms5637.c and will need to be moved to ms_sensors_i2c.h
> > for ms_tp_dev to point to it. I don't think this is a problem; it
> > matches how ms_tp_hw_data is defined now, but let me know if you
> > would like to avoid it.  
> 
> The pointer may be opaque, in that case it's not needed to be exposed.
> 

ms_sensors_i2c.c dereferences both ->hw->prom_len in
ms_sensors_tp_read_prom and ->comp_consts in
ms_sensors_read_temp_and_pressure, so an opaque ms_tp_data would require
passing both pointers as parameters. I plan on moving ms_tp_data into
ms_sensors_i2c.h, dropping the original duplicate hw, unless you prefer
otherwise.

-- 
Thanks,
Louis

Re: [PATCH v2 2/2] iio: pressure: ms5637: Add variant specific temperature compensation

[Louis Adamian]

On Fri, 7 Aug 2026 00:27:30 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Wed, Aug 05, 2026 at 01:06:51PM -0400, Louis Adamian wrote:
> > On Thu, 11 Jun 2026 22:32:02 +0300
> > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:  
> > > On Thu, Jun 11, 2026 at 03:10:26PM -0400, Louis Adamian wrote:  
> > > > On Wed, 10 Jun 2026 21:37:58 +0300
> > > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:    
> > > > > On Tue, Jun 09, 2026 at 10:04:58PM -0400, Louis Adamian
> > > > > wrote:    
> 
> ...
> 
> > > > > >  struct ms_tp_dev {
> > > > > >  	struct i2c_client *client;      
> > > > >     
>  [...]  
> > > > > 
> > > > > Please, check with `pahole` if this is the best layout.    
> > > > 
> > > > I will correct the packing on this per pahole's finding.    
>  [...]  
> > > > > 
> > > > > Can this be simply embedded into ms_tp_dev (and copied there
> > > > > if required)?    
> > > > 
> > > > Copying in ms_tp_comp_consts provides no benefit over
> > > > referencing the pointer; the values are per-descriptor, not
> > > > per-instance and never modified. I'll keep the pointer (same as
> > > > ms_tp_hw_data).    
> > > 
> > > Okay, let me ask differently then: Can we use the same data type
> > > in ms_tp_dev instead of adding basically the same fields as in
> > > ms_tp_data?
> > >   
> > Sorry for the very slow reply. Yes, for v3 ms_tp_dev will hold a
> > single const ms_tp_data *data instead of its own hw and
> > comp_consts. This drops the duplicated hw that predates this
> > series. 
> > 
> > One thing worth noting before I do that: ms_tp_data is currently
> > private to ms5637.c and will need to be moved to ms_sensors_i2c.h
> > for ms_tp_dev to point to it. I don't think this is a problem; it
> > matches how ms_tp_hw_data is defined now, but let me know if you
> > would like to avoid it.  
> 
> The pointer may be opaque, in that case it's not needed to be exposed.
> 

ms_sensors_i2c.c dereferences both ->hw->prom_len in
ms_sensors_tp_read_prom and ->comp_consts in
ms_sensors_read_temp_and_pressure, so an opaque ms_tp_data would require
passing both pointers as parameters. I plan on moving ms_tp_data into
ms_sensors_i2c.h, dropping the original duplicate hw, unless you prefer
otherwise.

-- 
Thanks,
Louis