2022-10-20 09:05:11

by Cosmin Tanislav

[permalink] [raw]
Subject: [PATCH v2 0/5] Support more parts in LTC2983

Add support for the following parts:
* LTC2984
* LTC2986
* LTM2985

The LTC2984 is a variant of the LTC2983 with EEPROM.
The LTC2986 is a variant of the LTC2983 with only 10 channels,
EEPROM and support for active analog temperature sensors.
The LTM2985 is software-compatible with the LTC2986.

Also, remove excessive allocations on resume.

V1 -> V2:
* add Fixes tag
* add patch that fixes the regmap_bulk_write() call with stack allocated
buffer
* add patch that refines the binding in preperation for adding new
parts support to it
* do not use stack allocated buffer for writing the EEPROM key

Cosmin Tanislav (5):
iio: temperature: ltc2983: allocate iio channels once
iio: temperature: ltc2983: make bulk write buffer DMA-safe
dt-bindings: iio: temperature: ltc2983: refine
dt-bindings: iio: temperature: ltc2983: support more parts
iio: temperature: ltc2983: support more parts

.../bindings/iio/temperature/adi,ltc2983.yaml | 378 ++++++++++++------
drivers/iio/temperature/ltc2983.c | 206 +++++++++-
2 files changed, 434 insertions(+), 150 deletions(-)

--
2.38.1


2022-10-20 09:14:22

by Cosmin Tanislav

[permalink] [raw]
Subject: [PATCH v2 4/5] dt-bindings: iio: temperature: ltc2983: support more parts

From: Cosmin Tanislav <[email protected]>

Add support for the following parts:
* LTC2984
* LTC2986
* LTM2985

The LTC2984 is a variant of the LTC2983 with EEPROM.
The LTC2986 is a variant of the LTC2983 with only 10 channels,
EEPROM and support for active analog temperature sensors.
The LTM2985 is software-compatible with the LTC2986.

Signed-off-by: Cosmin Tanislav <[email protected]>
---
.../bindings/iio/temperature/adi,ltc2983.yaml | 69 +++++++++++++++++--
1 file changed, 65 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml b/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
index 3e97ec841fd6..1cb31d896c9a 100644
--- a/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
+++ b/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
@@ -4,19 +4,30 @@
$id: http://devicetree.org/schemas/iio/temperature/adi,ltc2983.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#

-title: Analog Devices LTC2983 Multi-sensor Temperature system
+title: Analog Devices LTC2983, LTC2986, LTM2985 Multi-sensor Temperature system

maintainers:
- Nuno Sá <[email protected]>

description: |
- Analog Devices LTC2983 Multi-Sensor Digital Temperature Measurement System
+ Analog Devices LTC2983, LTC2984, LTC2986, LTM2985 Multi-Sensor Digital
+ Temperature Measurement Systems
+
https://www.analog.com/media/en/technical-documentation/data-sheets/2983fc.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/2984fb.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/29861fa.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ltm2985.pdf

properties:
compatible:
- enum:
- - adi,ltc2983
+ oneOf:
+ - enum:
+ - adi,ltc2983
+ - adi,ltc2986
+ - adi,ltm2985
+ - items:
+ - const: adi,ltc2984
+ - const: adi,ltc2983

reg:
maxItems: 1
@@ -413,6 +424,44 @@ patternProperties:
reg:
minimum: 1

+ "^temp@":
+ type: object
+ description: Active analog temperature sensor.
+ properties:
+ adi,sensor-type:
+ description: Sensor type for active analog temperature sensors.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ const: 31
+
+ adi,single-ended:
+ description: Whether the sensor is single-ended.
+ type: boolean
+
+ adi,custom-temp:
+ description:
+ Used for digitizing active analog temperature sensors.
+ See Page 67 of the LTM2985 datasheet.
+ $ref: /schemas/types.yaml#/definitions/uint64-matrix
+ minItems: 3
+ maxItems: 64
+ items:
+ items:
+ - description: Voltage point in nV, signed.
+ - description: Temperature point in uK.
+
+ required:
+ - adi,custom-temp
+
+ allOf:
+ - if:
+ properties:
+ adi,single-ended:
+ const: true
+ then:
+ properties:
+ reg:
+ minimum: 1
+
"^rsense@":
type: object
description: Sense resistor sensor.
@@ -439,6 +488,18 @@ required:

additionalProperties: false

+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - adi,ltc2983
+ - adi,ltc2984
+ then:
+ patternProperties:
+ "^temp@": false
+
examples:
- |
#include <dt-bindings/interrupt-controller/irq.h>
--
2.38.1

2022-10-20 09:14:53

by Cosmin Tanislav

[permalink] [raw]
Subject: [PATCH v2 2/5] iio: temperature: ltc2983: make bulk write buffer DMA-safe

From: Cosmin Tanislav <[email protected]>

regmap_bulk_write() does not guarantee implicit DMA-safety,
even though the current implementation duplicates the given
buffer. Do not rely on it.

Fixes: f110f3188e56 ("iio: temperature: Add support for LTC2983")
Signed-off-by: Cosmin Tanislav <[email protected]>
---
drivers/iio/temperature/ltc2983.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
index a60ccf183687..1117991ca2ab 100644
--- a/drivers/iio/temperature/ltc2983.c
+++ b/drivers/iio/temperature/ltc2983.c
@@ -209,6 +209,7 @@ struct ltc2983_data {
* Holds the converted temperature
*/
__be32 temp __aligned(IIO_DMA_MINALIGN);
+ __be32 chan_val;
};

struct ltc2983_sensor {
@@ -313,19 +314,18 @@ static int __ltc2983_fault_handler(const struct ltc2983_data *st,
return 0;
}

-static int __ltc2983_chan_assign_common(const struct ltc2983_data *st,
+static int __ltc2983_chan_assign_common(struct ltc2983_data *st,
const struct ltc2983_sensor *sensor,
u32 chan_val)
{
u32 reg = LTC2983_CHAN_START_ADDR(sensor->chan);
- __be32 __chan_val;

chan_val |= LTC2983_CHAN_TYPE(sensor->type);
dev_dbg(&st->spi->dev, "Assign reg:0x%04X, val:0x%08X\n", reg,
chan_val);
- __chan_val = cpu_to_be32(chan_val);
- return regmap_bulk_write(st->regmap, reg, &__chan_val,
- sizeof(__chan_val));
+ st->chan_val = cpu_to_be32(chan_val);
+ return regmap_bulk_write(st->regmap, reg, &st->chan_val,
+ sizeof(st->chan_val));
}

static int __ltc2983_chan_custom_sensor_assign(struct ltc2983_data *st,
--
2.38.1

2022-10-20 09:15:53

by Cosmin Tanislav

[permalink] [raw]
Subject: [PATCH v2 3/5] dt-bindings: iio: temperature: ltc2983: refine

From: Cosmin Tanislav <[email protected]>

* make sure addresses are represented as hex
* add note about wrong unit value for adi,mux-delay-config-us
* simplify descriptions
* add descriptions for the items of custom sensor tables
* add default property values where applicable
* use conditionals to extend minimum reg value
for single ended sensors
* remove " around phandle schema $ref
* remove label from example and use generic temperature
sensor name

Signed-off-by: Cosmin Tanislav <[email protected]>
---
.../bindings/iio/temperature/adi,ltc2983.yaml | 309 +++++++++++-------
1 file changed, 182 insertions(+), 127 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml b/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
index 722781aa4697..3e97ec841fd6 100644
--- a/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
+++ b/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
@@ -26,25 +26,25 @@ properties:

adi,mux-delay-config-us:
description:
- The LTC2983 performs 2 or 3 internal conversion cycles per temperature
- result. Each conversion cycle is performed with different excitation and
- input multiplexer configurations. Prior to each conversion, these
- excitation circuits and input switch configurations are changed and an
- internal 1ms delay ensures settling prior to the conversion cycle in most
- cases. An extra delay can be configured using this property. The value is
- rounded to nearest 100us.
+ Extra delay prior to each conversion, in addition to the internal 1ms
+ delay, for the multiplexer to switch input configurations and
+ excitation values.
+
+ This property is supposed to be in microseconds, but to maintain
+ compatibility, this value will be multiplied by 100 before usage.
maximum: 255
+ default: 0

adi,filter-notch-freq:
description:
- Set's the default setting of the digital filter. The default is
- simultaneous 50/60Hz rejection.
+ Notch frequency of the digital filter.
0 - 50/60Hz rejection
1 - 60Hz rejection
2 - 50Hz rejection
$ref: /schemas/types.yaml#/definitions/uint32
minimum: 0
maximum: 2
+ default: 0

'#address-cells':
const: 1
@@ -53,19 +53,19 @@ properties:
const: 0

patternProperties:
- "@([1-9]|1[0-9]|20)$":
+ "@([0-9a-f]+)$":
type: object
-
+ description: Sensor.
properties:
reg:
description:
- The channel number. It can be connected to one of the 20 channels of
- the device.
- minimum: 1
+ Channel number. Connects the sensor to the channel with this number
+ of the device.
+ minimum: 2
maximum: 20

adi,sensor-type:
- description: Identifies the type of sensor connected to the device.
+ description: Type of sensor connected to the device.
$ref: /schemas/types.yaml#/definitions/uint32

required:
@@ -74,10 +74,7 @@ patternProperties:

"^thermocouple@":
type: object
- description:
- Represents a thermocouple sensor which is connected to one of the device
- channels.
-
+ description: Thermocouple sensor.
properties:
adi,sensor-type:
description: |
@@ -95,87 +92,104 @@ patternProperties:
maximum: 9

adi,single-ended:
- description:
- Boolean property which set's the thermocouple as single-ended.
+ description: Whether the sensor is single-ended.
type: boolean

adi,sensor-oc-current-microamp:
- description:
- This property set's the pulsed current value applied during
- open-circuit detect.
+ description: Pulsed current value applied during open-circuit detect.
enum: [10, 100, 500, 1000]
+ default: 10

adi,cold-junction-handle:
description:
- Phandle which points to a sensor object responsible for measuring
- the thermocouple cold junction temperature.
- $ref: "/schemas/types.yaml#/definitions/phandle"
+ Sensor responsible for measuring the thermocouple cold junction
+ temperature.
+ $ref: /schemas/types.yaml#/definitions/phandle

adi,custom-thermocouple:
description:
- This is a table, where each entry should be a pair of
- voltage(mv)-temperature(K). The entries must be given in nv and uK
- so that, the original values must be multiplied by 1000000. For
- more details look at table 69 and 70.
- Note should be signed, but dtc doesn't currently maintain the
- sign.
+ Used for digitizing custom thermocouples.
+ See Page 59 of the datasheet.
$ref: /schemas/types.yaml#/definitions/uint64-matrix
minItems: 3
maxItems: 64
items:
- minItems: 2
- maxItems: 2
+ items:
+ - description: Voltage point in nV, signed.
+ - description: Temperature point in uK.
+
+ allOf:
+ - if:
+ properties:
+ adi,single-ended:
+ const: true
+ then:
+ properties:
+ reg:
+ minimum: 1
+ - if:
+ properties:
+ adi,sensor-type:
+ const: 9
+ then:
+ required:
+ - adi,custom-thermocouple

"^diode@":
type: object
- description:
- Represents a diode sensor which is connected to one of the device
- channels.
-
+ description: Diode sensor.
properties:
adi,sensor-type:
- description: Identifies the sensor as a diode.
+ description: Sensor type for diodes.
$ref: /schemas/types.yaml#/definitions/uint32
const: 28

adi,single-ended:
- description: Boolean property which set's the diode as single-ended.
+ description: Whether the sensor is single-ended.
type: boolean

adi,three-conversion-cycles:
description:
- Boolean property which set's three conversion cycles removing
- parasitic resistance effects between the LTC2983 and the diode.
+ Whether to use three conversion cycles to remove parasitic
+ resistance between the device and the diode.
type: boolean

adi,average-on:
description:
- Boolean property which enables a running average of the diode
- temperature reading. This reduces the noise when the diode is used
- as a cold junction temperature element on an isothermal block
- where temperatures change slowly.
+ Whether to use a running average of the diode temperature
+ reading to reduce the noise when the diode is used as a cold
+ junction temperature element on an isothermal block where
+ temperatures change slowly.
type: boolean

adi,excitation-current-microamp:
description:
- This property controls the magnitude of the excitation current
- applied to the diode. Depending on the number of conversions
- cycles, this property will assume different predefined values on
- each cycle. Just set the value of the first cycle (1l).
+ Magnitude of the 1l excitation current applied to the diode.
+ 4l excitation current will be 4 times this value, and 8l
+ excitation current will be 8 times value.
enum: [10, 20, 40, 80]
+ default: 10

adi,ideal-factor-value:
description:
- This property sets the diode ideality factor. The real value must
- be multiplied by 1000000 to remove the fractional part. For more
- information look at table 20 of the datasheet.
+ Diode ideality factor.
+ Set this property to 1000000 times the real value.
$ref: /schemas/types.yaml#/definitions/uint32
+ default: 0
+
+ allOf:
+ - if:
+ properties:
+ adi,single-ended:
+ const: true
+ then:
+ properties:
+ reg:
+ minimum: 1

"^rtd@":
type: object
- description:
- Represents a rtd sensor which is connected to one of the device channels.
-
+ description: RTD sensor.
properties:
reg:
minimum: 2
@@ -197,56 +211,57 @@ patternProperties:
maximum: 18

adi,rsense-handle:
- description:
- Phandle pointing to a rsense object associated with this RTD.
- $ref: "/schemas/types.yaml#/definitions/phandle"
+ description: Associated sense resistor sensor.
+ $ref: /schemas/types.yaml#/definitions/phandle

adi,number-of-wires:
description:
- Identifies the number of wires used by the RTD. Setting this
- property to 5 means 4 wires with Kelvin Rsense.
+ Number of wires used by the RTD.
+ 5 means 4 wires with Kelvin sense resistor.
$ref: /schemas/types.yaml#/definitions/uint32
enum: [2, 3, 4, 5]
+ default: 2

adi,rsense-share:
description:
- Boolean property which enables Rsense sharing, where one sense
- resistor is used for multiple 2-, 3-, and/or 4-wire RTDs.
+ Whether to enable sense resistor sharing, where one sense
+ resistor is used by multiple sensors.
type: boolean

adi,current-rotate:
description:
- Boolean property which enables excitation current rotation to
- automatically remove parasitic thermocouple effects. Note that
- this property is not allowed for 2- and 3-wire RTDs.
+ Whether to enable excitation current rotation to automatically
+ remove parasitic thermocouple effects.
type: boolean

adi,excitation-current-microamp:
- description:
- This property controls the magnitude of the excitation current
- applied to the RTD.
+ description: Excitation current applied to the RTD.
enum: [5, 10, 25, 50, 100, 250, 500, 1000]
+ default: 5

adi,rtd-curve:
description:
- This property set the RTD curve used and the corresponding
- Callendar-VanDusen constants. Look at table 30 of the datasheet.
+ RTD curve and the corresponding Callendar-VanDusen constants.
+ 0 - European
+ 1 - American
+ 2 - Japanese
+ 3 - ITS-90
$ref: /schemas/types.yaml#/definitions/uint32
minimum: 0
maximum: 3
+ default: 0

adi,custom-rtd:
description:
- This is a table, where each entry should be a pair of
- resistance(ohm)-temperature(K). The entries added here are in uohm
- and uK. For more details values look at table 74 and 75.
+ Used for digitizing custom RTDs.
+ See Page 62 of the datasheet.
$ref: /schemas/types.yaml#/definitions/uint64-matrix
+ minItems: 3
+ maxItems: 64
items:
- minItems: 3
- maxItems: 64
items:
- minItems: 2
- maxItems: 2
+ - description: Resistance point in uOhms.
+ - description: Temperature point in uK.

required:
- adi,rsense-handle
@@ -254,12 +269,25 @@ patternProperties:
dependencies:
adi,current-rotate: [ "adi,rsense-share" ]

+ allOf:
+ - if:
+ properties:
+ adi,number-of-wires:
+ enum: [2, 3]
+ then:
+ properties:
+ adi,current-rotate: false
+ - if:
+ properties:
+ adi,sensor-type:
+ const: 18
+ then:
+ required:
+ - adi,custom-rtd
+
"^thermistor@":
type: object
- description:
- Represents a thermistor sensor which is connected to one of the device
- channels.
-
+ description: Thermistor sensor.
properties:
adi,sensor-type:
description:
@@ -277,61 +305,54 @@ patternProperties:
maximum: 27

adi,rsense-handle:
- description:
- Phandle pointing to a rsense object associated with this
- thermistor.
- $ref: "/schemas/types.yaml#/definitions/phandle"
+ description: Associated sense resistor sensor.
+ $ref: /schemas/types.yaml#/definitions/phandle

adi,single-ended:
- description:
- Boolean property which set's the thermistor as single-ended.
+ description: Whether the sensor is single-ended.
type: boolean

adi,rsense-share:
description:
- Boolean property which enables Rsense sharing, where one sense
- resistor is used for multiple thermistors. Note that this property
- is ignored if adi,single-ended is set.
+ Whether to enable sense resistor sharing, where one sense
+ resistor is used by multiple sensors.
type: boolean

adi,current-rotate:
description:
- Boolean property which enables excitation current rotation to
- automatically remove parasitic thermocouple effects.
+ Whether to enable excitation current rotation to automatically
+ remove parasitic thermocouple effects.
type: boolean

adi,excitation-current-nanoamp:
description:
- This property controls the magnitude of the excitation current
- applied to the thermistor. Value 0 set's the sensor in auto-range
- mode.
+ Excitation current applied to the thermistor.
+ 0 sets the sensor in auto-range mode.
$ref: /schemas/types.yaml#/definitions/uint32
enum: [0, 250, 500, 1000, 5000, 10000, 25000, 50000, 100000, 250000,
500000, 1000000]
+ default: 0
+
+ adi,custom-steinhart:
+ description:
+ Steinhart-Hart coefficients in raw format, used for digitizing
+ custom thermistors.
+ See Page 68 of the datasheet.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ minItems: 6
+ maxItems: 6

adi,custom-thermistor:
description:
- This is a table, where each entry should be a pair of
- resistance(ohm)-temperature(K). The entries added here are in uohm
- and uK only for custom thermistors. For more details look at table
- 78 and 79.
+ Used for digitizing custom thermistors.
+ See Page 65 of the datasheet.
$ref: /schemas/types.yaml#/definitions/uint64-matrix
minItems: 3
maxItems: 64
items:
- minItems: 2
- maxItems: 2
-
- adi,custom-steinhart:
- description:
- Steinhart-Hart coefficients are also supported and can
- be programmed into the device memory using this property. For
- Steinhart sensors the coefficients are given in the raw
- format. Look at table 82 for more information.
- $ref: /schemas/types.yaml#/definitions/uint32-array
- items:
- minItems: 6
- maxItems: 6
+ items:
+ - description: Resistance point in uOhms.
+ - description: Temperature point in uK.

required:
- adi,rsense-handle
@@ -339,40 +360,74 @@ patternProperties:
dependencies:
adi,current-rotate: [ "adi,rsense-share" ]

+ allOf:
+ - if:
+ properties:
+ adi,single-ended:
+ const: true
+ then:
+ properties:
+ reg:
+ minimum: 1
+ - if:
+ properties:
+ adi,sensor-type:
+ const: 26
+ then:
+ properties:
+ adi,excitation-current-nanoamp:
+ default: 1000
+ required:
+ - adi,custom-steinhart
+ - if:
+ properties:
+ adi,sensor-type:
+ const: 27
+ then:
+ properties:
+ adi,excitation-current-nanoamp:
+ default: 1000
+ required:
+ - adi,custom-thermistor
+
"^adc@":
type: object
- description: Represents a channel which is being used as a direct adc.
-
+ description: Direct ADC sensor.
properties:
adi,sensor-type:
- description: Identifies the sensor as a direct adc.
+ description: Sensor type for direct ADC sensors.
$ref: /schemas/types.yaml#/definitions/uint32
const: 30

adi,single-ended:
- description: Boolean property which set's the adc as single-ended.
+ description: Whether the sensor is single-ended.
type: boolean

+ allOf:
+ - if:
+ properties:
+ adi,single-ended:
+ const: true
+ then:
+ properties:
+ reg:
+ minimum: 1
+
"^rsense@":
type: object
- description:
- Represents a rsense which is connected to one of the device channels.
- Rsense are used by thermistors and RTD's.
-
+ description: Sense resistor sensor.
properties:
reg:
minimum: 2
maximum: 20

adi,sensor-type:
- description: Identifies the sensor as a rsense.
+ description: Sensor type sense resistor sensors.
$ref: /schemas/types.yaml#/definitions/uint32
const: 29

adi,rsense-val-milli-ohms:
- description:
- Sets the value of the sense resistor. Look at table 20 of the
- datasheet for information.
+ description: Value of the sense resistor.

required:
- adi,rsense-val-milli-ohms
@@ -391,7 +446,7 @@ examples:
#address-cells = <1>;
#size-cells = <0>;

- sensor_ltc2983: ltc2983@0 {
+ temp@0 {
compatible = "adi,ltc2983";
reg = <0>;

--
2.38.1

2022-10-20 09:19:06

by Cosmin Tanislav

[permalink] [raw]
Subject: [PATCH v2 1/5] iio: temperature: ltc2983: allocate iio channels once

From: Cosmin Tanislav <[email protected]>

Currently, every time the device wakes up from sleep, the
iio_chan array is reallocated, leaking the previous one
until the device is removed (basically never).

Move the allocation to the probe function to avoid this.

Fixes: f110f3188e56 ("iio: temperature: Add support for LTC2983")
Signed-off-by: Cosmin Tanislav <[email protected]>
---
drivers/iio/temperature/ltc2983.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
index b652d2b39bcf..a60ccf183687 100644
--- a/drivers/iio/temperature/ltc2983.c
+++ b/drivers/iio/temperature/ltc2983.c
@@ -1385,13 +1385,6 @@ static int ltc2983_setup(struct ltc2983_data *st, bool assign_iio)
return ret;
}

- st->iio_chan = devm_kzalloc(&st->spi->dev,
- st->iio_channels * sizeof(*st->iio_chan),
- GFP_KERNEL);
-
- if (!st->iio_chan)
- return -ENOMEM;
-
ret = regmap_update_bits(st->regmap, LTC2983_GLOBAL_CONFIG_REG,
LTC2983_NOTCH_FREQ_MASK,
LTC2983_NOTCH_FREQ(st->filter_notch_freq));
@@ -1514,6 +1507,12 @@ static int ltc2983_probe(struct spi_device *spi)
gpiod_set_value_cansleep(gpio, 0);
}

+ st->iio_chan = devm_kzalloc(&spi->dev,
+ st->iio_channels * sizeof(*st->iio_chan),
+ GFP_KERNEL);
+ if (!st->iio_chan)
+ return -ENOMEM;
+
ret = ltc2983_setup(st, true);
if (ret)
return ret;
--
2.38.1

2022-10-20 09:36:23

by Cosmin Tanislav

[permalink] [raw]
Subject: [PATCH v2 5/5] iio: temperature: ltc2983: support more parts

From: Cosmin Tanislav <[email protected]>

Add support for the following parts:
* LTC2984
* LTC2986
* LTM2985

The LTC2984 is a variant of the LTC2983 with EEPROM.
The LTC2986 is a variant of the LTC2983 with only 10 channels,
EEPROM and support for active analog temperature sensors.
The LTM2985 is software-compatible with the LTC2986.

Signed-off-by: Cosmin Tanislav <[email protected]>
---
drivers/iio/temperature/ltc2983.c | 183 ++++++++++++++++++++++++++++--
1 file changed, 176 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
index 1117991ca2ab..fcb96c44d954 100644
--- a/drivers/iio/temperature/ltc2983.c
+++ b/drivers/iio/temperature/ltc2983.c
@@ -25,9 +25,12 @@
#define LTC2983_STATUS_REG 0x0000
#define LTC2983_TEMP_RES_START_REG 0x0010
#define LTC2983_TEMP_RES_END_REG 0x005F
+#define LTC2983_EEPROM_KEY_REG 0x00B0
+#define LTC2983_EEPROM_READ_STATUS_REG 0x00D0
#define LTC2983_GLOBAL_CONFIG_REG 0x00F0
#define LTC2983_MULT_CHANNEL_START_REG 0x00F4
#define LTC2983_MULT_CHANNEL_END_REG 0x00F7
+#define LTC2986_EEPROM_STATUS_REG 0x00F9
#define LTC2983_MUX_CONFIG_REG 0x00FF
#define LTC2983_CHAN_ASSIGN_START_REG 0x0200
#define LTC2983_CHAN_ASSIGN_END_REG 0x024F
@@ -35,13 +38,21 @@
#define LTC2983_CUST_SENS_TBL_END_REG 0x03CF

#define LTC2983_DIFFERENTIAL_CHAN_MIN 2
-#define LTC2983_MAX_CHANNELS_NR 20
#define LTC2983_MIN_CHANNELS_NR 1
#define LTC2983_SLEEP 0x97
#define LTC2983_CUSTOM_STEINHART_SIZE 24
#define LTC2983_CUSTOM_SENSOR_ENTRY_SZ 6
#define LTC2983_CUSTOM_STEINHART_ENTRY_SZ 4

+#define LTC2983_EEPROM_KEY 0xA53C0F5A
+#define LTC2983_EEPROM_WRITE_CMD 0x15
+#define LTC2983_EEPROM_READ_CMD 0x16
+#define LTC2983_EEPROM_STATUS_FAILURE_MASK GENMASK(3, 1)
+#define LTC2983_EEPROM_READ_FAILURE_MASK GENMASK(7, 0)
+
+#define LTC2983_EEPROM_WRITE_TIME_MS 2600
+#define LTC2983_EEPROM_READ_TIME_MS 20
+
#define LTC2983_CHAN_START_ADDR(chan) \
(((chan - 1) * 4) + LTC2983_CHAN_ASSIGN_START_REG)
#define LTC2983_CHAN_RES_ADDR(chan) \
@@ -171,6 +182,7 @@ enum {
LTC2983_SENSOR_DIODE = 28,
LTC2983_SENSOR_SENSE_RESISTOR = 29,
LTC2983_SENSOR_DIRECT_ADC = 30,
+ LTC2983_SENSOR_ACTIVE_TEMP = 31,
};

#define to_thermocouple(_sensor) \
@@ -191,7 +203,17 @@ enum {
#define to_adc(_sensor) \
container_of(_sensor, struct ltc2983_adc, sensor)

+#define to_temp(_sensor) \
+ container_of(_sensor, struct ltc2983_temp, sensor)
+
+struct ltc2983_chip_info {
+ unsigned int max_channels_nr;
+ bool has_temp;
+ bool has_eeprom;
+};
+
struct ltc2983_data {
+ const struct ltc2983_chip_info *info;
struct regmap *regmap;
struct spi_device *spi;
struct mutex lock;
@@ -210,6 +232,7 @@ struct ltc2983_data {
*/
__be32 temp __aligned(IIO_DMA_MINALIGN);
__be32 chan_val;
+ __be32 eeprom_key;
};

struct ltc2983_sensor {
@@ -272,6 +295,12 @@ struct ltc2983_adc {
bool single_ended;
};

+struct ltc2983_temp {
+ struct ltc2983_sensor sensor;
+ struct ltc2983_custom_sensor *custom;
+ bool single_ended;
+};
+
/*
* Convert to Q format numbers. These number's are integers where
* the number of integer and fractional bits are specified. The resolution
@@ -606,6 +635,22 @@ static int ltc2983_adc_assign_chan(struct ltc2983_data *st,
return __ltc2983_chan_assign_common(st, sensor, chan_val);
}

+static int ltc2983_temp_assign_chan(struct ltc2983_data *st,
+ const struct ltc2983_sensor *sensor)
+{
+ struct ltc2983_temp *temp = to_temp(sensor);
+ u32 chan_val;
+ int ret;
+
+ chan_val = LTC2983_ADC_SINGLE_ENDED(temp->single_ended);
+
+ ret = __ltc2983_chan_custom_sensor_assign(st, temp->custom, &chan_val);
+ if (ret)
+ return ret;
+
+ return __ltc2983_chan_assign_common(st, sensor, chan_val);
+}
+
static struct ltc2983_sensor *
ltc2983_thermocouple_new(const struct fwnode_handle *child, struct ltc2983_data *st,
const struct ltc2983_sensor *sensor)
@@ -771,10 +816,10 @@ ltc2983_rtd_new(const struct fwnode_handle *child, struct ltc2983_data *st,
if (rtd->sensor_config & LTC2983_RTD_4_WIRE_MASK) {
/* 4-wire */
u8 min = LTC2983_DIFFERENTIAL_CHAN_MIN,
- max = LTC2983_MAX_CHANNELS_NR;
+ max = st->info->max_channels_nr;

if (rtd->sensor_config & LTC2983_RTD_ROTATION_MASK)
- max = LTC2983_MAX_CHANNELS_NR - 1;
+ max = st->info->max_channels_nr - 1;

if (((rtd->sensor_config & LTC2983_RTD_KELVIN_R_SENSE_MASK)
== LTC2983_RTD_KELVIN_R_SENSE_MASK) &&
@@ -1143,6 +1188,38 @@ static struct ltc2983_sensor *ltc2983_adc_new(struct fwnode_handle *child,
return &adc->sensor;
}

+static struct ltc2983_sensor *ltc2983_temp_new(struct fwnode_handle *child,
+ struct ltc2983_data *st,
+ const struct ltc2983_sensor *sensor)
+{
+ struct ltc2983_temp *temp;
+
+ temp = devm_kzalloc(&st->spi->dev, sizeof(*temp), GFP_KERNEL);
+ if (!temp)
+ return ERR_PTR(-ENOMEM);
+
+ if (fwnode_property_read_bool(child, "adi,single-ended"))
+ temp->single_ended = true;
+
+ if (!temp->single_ended &&
+ sensor->chan < LTC2983_DIFFERENTIAL_CHAN_MIN) {
+ dev_err(&st->spi->dev, "Invalid chan:%d for differential temp\n",
+ sensor->chan);
+ return ERR_PTR(-EINVAL);
+ }
+
+ temp->custom = __ltc2983_custom_sensor_new(st, child, "adi,custom-temp",
+ false, 4096, true);
+ if (IS_ERR(temp->custom))
+ return ERR_CAST(temp->custom);
+
+ /* set common parameters */
+ temp->sensor.assign_chan = ltc2983_temp_assign_chan;
+ temp->sensor.fault_handler = ltc2983_common_fault_handler;
+
+ return &temp->sensor;
+}
+
static int ltc2983_chan_read(struct ltc2983_data *st,
const struct ltc2983_sensor *sensor, int *val)
{
@@ -1302,10 +1379,10 @@ static int ltc2983_parse_dt(struct ltc2983_data *st)

/* check if we have a valid channel */
if (sensor.chan < LTC2983_MIN_CHANNELS_NR ||
- sensor.chan > LTC2983_MAX_CHANNELS_NR) {
+ sensor.chan > st->info->max_channels_nr) {
ret = -EINVAL;
dev_err(dev, "chan:%d must be from %u to %u\n", sensor.chan,
- LTC2983_MIN_CHANNELS_NR, LTC2983_MAX_CHANNELS_NR);
+ LTC2983_MIN_CHANNELS_NR, st->info->max_channels_nr);
goto put_child;
} else if (channel_avail_mask & BIT(sensor.chan)) {
ret = -EINVAL;
@@ -1345,6 +1422,9 @@ static int ltc2983_parse_dt(struct ltc2983_data *st)
st->iio_channels--;
} else if (sensor.type == LTC2983_SENSOR_DIRECT_ADC) {
st->sensors[chan] = ltc2983_adc_new(child, st, &sensor);
+ } else if (st->info->has_temp &&
+ sensor.type == LTC2983_SENSOR_ACTIVE_TEMP) {
+ st->sensors[chan] = ltc2983_temp_new(child, st, &sensor);
} else {
dev_err(dev, "Unknown sensor type %d\n", sensor.type);
ret = -EINVAL;
@@ -1371,6 +1451,45 @@ static int ltc2983_parse_dt(struct ltc2983_data *st)
return ret;
}

+static int ltc2983_eeprom_cmd(struct ltc2983_data *st, unsigned int cmd,
+ unsigned int wait_time, unsigned int status_reg,
+ unsigned long status_fail_mask)
+{
+ unsigned long time;
+ unsigned int val;
+ int ret;
+
+ ret = regmap_bulk_write(st->regmap, LTC2983_EEPROM_KEY_REG,
+ &st->eeprom_key, sizeof(st->eeprom_key));
+ if (ret)
+ return ret;
+
+ reinit_completion(&st->completion);
+
+ ret = regmap_write(st->regmap, LTC2983_STATUS_REG,
+ LTC2983_STATUS_START(true) | cmd);
+ if (ret)
+ return ret;
+
+ time = wait_for_completion_timeout(&st->completion,
+ msecs_to_jiffies(wait_time));
+ if (!time) {
+ dev_err(&st->spi->dev, "EEPROM command timed out\n");
+ return -ETIMEDOUT;
+ }
+
+ ret = regmap_read(st->regmap, status_reg, &val);
+ if (ret)
+ return ret;
+
+ if (val & status_fail_mask) {
+ dev_err(&st->spi->dev, "EEPROM command failed: 0x%02X\n", val);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int ltc2983_setup(struct ltc2983_data *st, bool assign_iio)
{
u32 iio_chan_t = 0, iio_chan_v = 0, chan, iio_idx = 0, status;
@@ -1396,6 +1515,15 @@ static int ltc2983_setup(struct ltc2983_data *st, bool assign_iio)
if (ret)
return ret;

+ if (st->info->has_eeprom && !assign_iio) {
+ ret = ltc2983_eeprom_cmd(st, LTC2983_EEPROM_READ_CMD,
+ LTC2983_EEPROM_READ_TIME_MS,
+ LTC2983_EEPROM_READ_STATUS_REG,
+ LTC2983_EEPROM_READ_FAILURE_MASK);
+ if (!ret)
+ return 0;
+ }
+
for (chan = 0; chan < st->num_channels; chan++) {
u32 chan_type = 0, *iio_chan;

@@ -1435,9 +1563,13 @@ static int ltc2983_setup(struct ltc2983_data *st, bool assign_iio)
static const struct regmap_range ltc2983_reg_ranges[] = {
regmap_reg_range(LTC2983_STATUS_REG, LTC2983_STATUS_REG),
regmap_reg_range(LTC2983_TEMP_RES_START_REG, LTC2983_TEMP_RES_END_REG),
+ regmap_reg_range(LTC2983_EEPROM_KEY_REG, LTC2983_EEPROM_KEY_REG),
+ regmap_reg_range(LTC2983_EEPROM_READ_STATUS_REG,
+ LTC2983_EEPROM_READ_STATUS_REG),
regmap_reg_range(LTC2983_GLOBAL_CONFIG_REG, LTC2983_GLOBAL_CONFIG_REG),
regmap_reg_range(LTC2983_MULT_CHANNEL_START_REG,
LTC2983_MULT_CHANNEL_END_REG),
+ regmap_reg_range(LTC2986_EEPROM_STATUS_REG, LTC2986_EEPROM_STATUS_REG),
regmap_reg_range(LTC2983_MUX_CONFIG_REG, LTC2983_MUX_CONFIG_REG),
regmap_reg_range(LTC2983_CHAN_ASSIGN_START_REG,
LTC2983_CHAN_ASSIGN_END_REG),
@@ -1482,6 +1614,12 @@ static int ltc2983_probe(struct spi_device *spi)

st = iio_priv(indio_dev);

+ st->info = device_get_match_data(&spi->dev);
+ if (!st->info)
+ st->info = (void *)spi_get_device_id(spi)->driver_data;
+ if (!st->info)
+ return -ENODEV;
+
st->regmap = devm_regmap_init_spi(spi, &ltc2983_regmap_config);
if (IS_ERR(st->regmap)) {
dev_err(&spi->dev, "Failed to initialize regmap\n");
@@ -1491,6 +1629,7 @@ static int ltc2983_probe(struct spi_device *spi)
mutex_init(&st->lock);
init_completion(&st->completion);
st->spi = spi;
+ st->eeprom_key = cpu_to_be32(LTC2983_EEPROM_KEY);
spi_set_drvdata(spi, st);

ret = ltc2983_parse_dt(st);
@@ -1524,6 +1663,15 @@ static int ltc2983_probe(struct spi_device *spi)
return ret;
}

+ if (st->info->has_eeprom) {
+ ret = ltc2983_eeprom_cmd(st, LTC2983_EEPROM_WRITE_CMD,
+ LTC2983_EEPROM_WRITE_TIME_MS,
+ LTC2986_EEPROM_STATUS_REG,
+ LTC2983_EEPROM_STATUS_FAILURE_MASK);
+ if (ret)
+ return ret;
+ }
+
indio_dev->name = name;
indio_dev->num_channels = st->iio_channels;
indio_dev->channels = st->iio_chan;
@@ -1554,14 +1702,35 @@ static int ltc2983_suspend(struct device *dev)
static DEFINE_SIMPLE_DEV_PM_OPS(ltc2983_pm_ops, ltc2983_suspend,
ltc2983_resume);

+static const struct ltc2983_chip_info ltc2983_chip_info_data = {
+ .max_channels_nr = 20,
+};
+
+static const struct ltc2983_chip_info ltc2984_chip_info_data = {
+ .max_channels_nr = 20,
+ .has_eeprom = true,
+};
+
+static const struct ltc2983_chip_info ltc2986_chip_info_data = {
+ .max_channels_nr = 10,
+ .has_temp = true,
+ .has_eeprom = true,
+};
+
static const struct spi_device_id ltc2983_id_table[] = {
- { "ltc2983" },
+ { "ltc2983", (kernel_ulong_t)&ltc2983_chip_info_data },
+ { "ltc2984", (kernel_ulong_t)&ltc2984_chip_info_data },
+ { "ltc2986", (kernel_ulong_t)&ltc2986_chip_info_data },
+ { "ltm2985", (kernel_ulong_t)&ltc2986_chip_info_data },
{},
};
MODULE_DEVICE_TABLE(spi, ltc2983_id_table);

static const struct of_device_id ltc2983_of_match[] = {
- { .compatible = "adi,ltc2983" },
+ { .compatible = "adi,ltc2983", .data = &ltc2983_chip_info_data },
+ { .compatible = "adi,ltc2984", .data = &ltc2984_chip_info_data },
+ { .compatible = "adi,ltc2986", .data = &ltc2986_chip_info_data },
+ { .compatible = "adi,ltm2985", .data = &ltc2986_chip_info_data },
{},
};
MODULE_DEVICE_TABLE(of, ltc2983_of_match);
--
2.38.1

2022-10-20 13:07:41

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 3/5] dt-bindings: iio: temperature: ltc2983: refine

On Thu, 20 Oct 2022 12:02:55 +0300, Cosmin Tanislav wrote:
> From: Cosmin Tanislav <[email protected]>
>
> * make sure addresses are represented as hex
> * add note about wrong unit value for adi,mux-delay-config-us
> * simplify descriptions
> * add descriptions for the items of custom sensor tables
> * add default property values where applicable
> * use conditionals to extend minimum reg value
> for single ended sensors
> * remove " around phandle schema $ref
> * remove label from example and use generic temperature
> sensor name
>
> Signed-off-by: Cosmin Tanislav <[email protected]>
> ---
> .../bindings/iio/temperature/adi,ltc2983.yaml | 309 +++++++++++-------
> 1 file changed, 182 insertions(+), 127 deletions(-)
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml: patternProperties:^diode@:allOf:0:if:properties:adi,single-ended:const: True is not of type 'integer', 'string'
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml: patternProperties:^thermocouple@:allOf:0:if:properties:adi,single-ended:const: True is not of type 'integer', 'string'
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml: patternProperties:^adc@:allOf:0:if:properties:adi,single-ended:const: True is not of type 'integer', 'string'
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml: patternProperties:^thermistor@:allOf:0:if:properties:adi,single-ended:const: True is not of type 'integer', 'string'
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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.

2022-10-23 12:55:18

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] iio: temperature: ltc2983: allocate iio channels once

On Thu, 20 Oct 2022 12:02:53 +0300
Cosmin Tanislav <[email protected]> wrote:

> From: Cosmin Tanislav <[email protected]>
>
> Currently, every time the device wakes up from sleep, the
> iio_chan array is reallocated, leaking the previous one
> until the device is removed (basically never).
>
> Move the allocation to the probe function to avoid this.
>
> Fixes: f110f3188e56 ("iio: temperature: Add support for LTC2983")
> Signed-off-by: Cosmin Tanislav <[email protected]>
Note I already have this queued as a fix and I've sent the pull request to Greg this
morning. For future versions of this series, maybe just put a note in the cover
letter to cover the dependency.

Jonathan

> ---
> drivers/iio/temperature/ltc2983.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
> index b652d2b39bcf..a60ccf183687 100644
> --- a/drivers/iio/temperature/ltc2983.c
> +++ b/drivers/iio/temperature/ltc2983.c
> @@ -1385,13 +1385,6 @@ static int ltc2983_setup(struct ltc2983_data *st, bool assign_iio)
> return ret;
> }
>
> - st->iio_chan = devm_kzalloc(&st->spi->dev,
> - st->iio_channels * sizeof(*st->iio_chan),
> - GFP_KERNEL);
> -
> - if (!st->iio_chan)
> - return -ENOMEM;
> -
> ret = regmap_update_bits(st->regmap, LTC2983_GLOBAL_CONFIG_REG,
> LTC2983_NOTCH_FREQ_MASK,
> LTC2983_NOTCH_FREQ(st->filter_notch_freq));
> @@ -1514,6 +1507,12 @@ static int ltc2983_probe(struct spi_device *spi)
> gpiod_set_value_cansleep(gpio, 0);
> }
>
> + st->iio_chan = devm_kzalloc(&spi->dev,
> + st->iio_channels * sizeof(*st->iio_chan),
> + GFP_KERNEL);
> + if (!st->iio_chan)
> + return -ENOMEM;
> +
> ret = ltc2983_setup(st, true);
> if (ret)
> return ret;

2022-10-23 12:56:49

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] iio: temperature: ltc2983: support more parts

On Thu, 20 Oct 2022 12:02:57 +0300
Cosmin Tanislav <[email protected]> wrote:

> From: Cosmin Tanislav <[email protected]>
>
> Add support for the following parts:
> * LTC2984
> * LTC2986
> * LTM2985
>
> The LTC2984 is a variant of the LTC2983 with EEPROM.
> The LTC2986 is a variant of the LTC2983 with only 10 channels,
> EEPROM and support for active analog temperature sensors.
> The LTM2985 is software-compatible with the LTC2986.
>
> Signed-off-by: Cosmin Tanislav <[email protected]>
LGTM. I'll pick this up once the DT binding part is resolved.

Thanks,

Jonathan

2022-10-23 12:56:57

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 3/5] dt-bindings: iio: temperature: ltc2983: refine

On Thu, 20 Oct 2022 12:02:55 +0300
Cosmin Tanislav <[email protected]> wrote:

> From: Cosmin Tanislav <[email protected]>
>
> * make sure addresses are represented as hex
> * add note about wrong unit value for adi,mux-delay-config-us
> * simplify descriptions
> * add descriptions for the items of custom sensor tables
> * add default property values where applicable
> * use conditionals to extend minimum reg value
> for single ended sensors
> * remove " around phandle schema $ref
> * remove label from example and use generic temperature
> sensor name
>
> Signed-off-by: Cosmin Tanislav <[email protected]>

Hi Cosmin,

Just one question inline from me (other than the build bot report that I'll
assume you'll fix for v3).

Otherwise looks like a nice cleanup to me.

I wonder a bit on whether it is worth splitting up, but that would be
rather messy to actually do so will leave that to the dt experts to comment
on.

Jonathan


> ---
> .../bindings/iio/temperature/adi,ltc2983.yaml | 309 +++++++++++-------
> 1 file changed, 182 insertions(+), 127 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml b/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
> index 722781aa4697..3e97ec841fd6 100644
> --- a/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
> +++ b/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
> @@ -26,25 +26,25 @@ properties:
>
> adi,mux-delay-config-us:
> description:
> - The LTC2983 performs 2 or 3 internal conversion cycles per temperature
> - result. Each conversion cycle is performed with different excitation and
> - input multiplexer configurations. Prior to each conversion, these
> - excitation circuits and input switch configurations are changed and an
> - internal 1ms delay ensures settling prior to the conversion cycle in most
> - cases. An extra delay can be configured using this property. The value is
> - rounded to nearest 100us.
> + Extra delay prior to each conversion, in addition to the internal 1ms
> + delay, for the multiplexer to switch input configurations and
> + excitation values.
> +
> + This property is supposed to be in microseconds, but to maintain
> + compatibility, this value will be multiplied by 100 before usage.

This new text has me a little confused. Previously we talked rounding, now it
is saying the value is multiplied (which would make it definitely not in micro
secs!).. So are we papering over a driver bug here?



2022-10-23 12:58:00

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] iio: temperature: ltc2983: make bulk write buffer DMA-safe

On Thu, 20 Oct 2022 12:02:54 +0300
Cosmin Tanislav <[email protected]> wrote:

> From: Cosmin Tanislav <[email protected]>
>
> regmap_bulk_write() does not guarantee implicit DMA-safety,
> even though the current implementation duplicates the given
> buffer. Do not rely on it.
>
> Fixes: f110f3188e56 ("iio: temperature: Add support for LTC2983")
> Signed-off-by: Cosmin Tanislav <[email protected]>
LGTM.

As you right observed this is only sort of a fix because right now we
are fine anyway, so in the interests of getting the rest of the series
upstream quicker I'll take this one for the next merge window along
with the rest of the set.

Thanks,

Jonathan

> ---
> drivers/iio/temperature/ltc2983.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
> index a60ccf183687..1117991ca2ab 100644
> --- a/drivers/iio/temperature/ltc2983.c
> +++ b/drivers/iio/temperature/ltc2983.c
> @@ -209,6 +209,7 @@ struct ltc2983_data {
> * Holds the converted temperature
> */
> __be32 temp __aligned(IIO_DMA_MINALIGN);
> + __be32 chan_val;
> };
>
> struct ltc2983_sensor {
> @@ -313,19 +314,18 @@ static int __ltc2983_fault_handler(const struct ltc2983_data *st,
> return 0;
> }
>
> -static int __ltc2983_chan_assign_common(const struct ltc2983_data *st,
> +static int __ltc2983_chan_assign_common(struct ltc2983_data *st,
> const struct ltc2983_sensor *sensor,
> u32 chan_val)
> {
> u32 reg = LTC2983_CHAN_START_ADDR(sensor->chan);
> - __be32 __chan_val;
>
> chan_val |= LTC2983_CHAN_TYPE(sensor->type);
> dev_dbg(&st->spi->dev, "Assign reg:0x%04X, val:0x%08X\n", reg,
> chan_val);
> - __chan_val = cpu_to_be32(chan_val);
> - return regmap_bulk_write(st->regmap, reg, &__chan_val,
> - sizeof(__chan_val));
> + st->chan_val = cpu_to_be32(chan_val);
> + return regmap_bulk_write(st->regmap, reg, &st->chan_val,
> + sizeof(st->chan_val));
> }
>
> static int __ltc2983_chan_custom_sensor_assign(struct ltc2983_data *st,

2022-10-23 14:34:21

by Nuno Sá

[permalink] [raw]
Subject: Re: [PATCH v2 3/5] dt-bindings: iio: temperature: ltc2983: refine

On Sun, 2022-10-23 at 13:51 +0100, Jonathan Cameron wrote:
> On Thu, 20 Oct 2022 12:02:55 +0300
> Cosmin Tanislav <[email protected]> wrote:
>
> > From: Cosmin Tanislav <[email protected]>
> >
> >  * make sure addresses are represented as hex
> >  * add note about wrong unit value for adi,mux-delay-config-us
> >  * simplify descriptions
> >  * add descriptions for the items of custom sensor tables
> >  * add default property values where applicable
> >  * use conditionals to extend minimum reg value
> >    for single ended sensors
> >  * remove " around phandle schema $ref
> >  * remove label from example and use generic temperature
> >    sensor name
> >
> > Signed-off-by: Cosmin Tanislav <[email protected]>
>
> Hi Cosmin,
>
> Just one question inline from me (other than the build bot report
> that I'll
> assume you'll fix for v3).
>
> Otherwise looks like a nice cleanup to me.
>
> I wonder a bit on whether it is worth splitting up, but that would be
> rather messy to actually do so will leave that to the dt experts to
> comment
> on.
>
> Jonathan
>
>
> > ---
> >  .../bindings/iio/temperature/adi,ltc2983.yaml | 309 +++++++++++---
> > ----
> >  1 file changed, 182 insertions(+), 127 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yam
> > l
> > b/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yam
> > l
> > index 722781aa4697..3e97ec841fd6 100644
> > ---
> > a/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yam
> > l
> > +++
> > b/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yam
> > l
> > @@ -26,25 +26,25 @@ properties:
> >  
> >    adi,mux-delay-config-us:
> >      description:
> > -      The LTC2983 performs 2 or 3 internal conversion cycles per
> > temperature
> > -      result. Each conversion cycle is performed with different
> > excitation and
> > -      input multiplexer configurations. Prior to each conversion,
> > these
> > -      excitation circuits and input switch configurations are
> > changed and an
> > -      internal 1ms delay ensures settling prior to the conversion
> > cycle in most
> > -      cases. An extra delay can be configured using this property.
> > The value is
> > -      rounded to nearest 100us.
> > +      Extra delay prior to each conversion, in addition to the
> > internal 1ms
> > +      delay, for the multiplexer to switch input configurations
> > and
> > +      excitation values.
> > +
> > +      This property is supposed to be in microseconds, but to
> > maintain
> > +      compatibility, this value will be multiplied by 100 before
> > usage.
>
> This new text has me a little confused.  Previously we talked
> rounding, now it
> is saying the value is multiplied (which would make it definitely not
> in micro
> secs!)..  So are we papering over a driver bug here?
>
>

Hi Jonathan,

Let me try to make this one clear as it was my mess...

The multiplication is done internally by the device. I messed up in
this one as this value is clearly not in us but it is the raw value.
So, tecnically, there's nothing wrong in the driver as it just reads
this property and directly writes it. But of course this is misleading
and wrong from the bindings point of view.

That said, me and Cosmin did spoke about just having this property
'deprecated' and add a new one (the driver would need to be changed
accordingly) - no idea also about a new name for it :)

But for this round, Cosmin decided to have this stated on the
description and see what you and dt maintainers had to say about it and
if making it 'deprecated' is the way to go (or something else).

- Nuno Sá

2022-10-24 06:41:27

by Nuno Sa

[permalink] [raw]
Subject: RE: [PATCH v2 5/5] iio: temperature: ltc2983: support more parts



> -----Original Message-----
> From: Jonathan Cameron <[email protected]>
> Sent: Sunday, October 23, 2022 2:55 PM
> To: Cosmin Tanislav <[email protected]>
> Cc: Sa, Nuno <[email protected]>; Lars-Peter Clausen
> <[email protected]>; Hennerich, Michael <[email protected]>;
> Rob Herring <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; [email protected];
> [email protected]; [email protected]; Tanislav, Cosmin
> <[email protected]>
> Subject: Re: [PATCH v2 5/5] iio: temperature: ltc2983: support more parts
>
> [External]
>
> On Thu, 20 Oct 2022 12:02:57 +0300
> Cosmin Tanislav <[email protected]> wrote:
>
> > From: Cosmin Tanislav <[email protected]>
> >
> > Add support for the following parts:
> > * LTC2984
> > * LTC2986
> > * LTM2985
> >
> > The LTC2984 is a variant of the LTC2983 with EEPROM.
> > The LTC2986 is a variant of the LTC2983 with only 10 channels,
> > EEPROM and support for active analog temperature sensors.
> > The LTM2985 is software-compatible with the LTC2986.
> >
> > Signed-off-by: Cosmin Tanislav <[email protected]>
> LGTM. I'll pick this up once the DT binding part is resolved.
>
> Thanks,
>
> Jonathan

Reviewed-by: Nuno S? <[email protected]>

2022-10-24 06:54:36

by Nuno Sa

[permalink] [raw]
Subject: RE: [PATCH v2 2/5] iio: temperature: ltc2983: make bulk write buffer DMA-safe



> -----Original Message-----
> From: Jonathan Cameron <[email protected]>
> Sent: Sunday, October 23, 2022 2:43 PM
> To: Cosmin Tanislav <[email protected]>
> Cc: Sa, Nuno <[email protected]>; Lars-Peter Clausen
> <[email protected]>; Hennerich, Michael <[email protected]>;
> Rob Herring <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; [email protected];
> [email protected]; [email protected]; Tanislav, Cosmin
> <[email protected]>
> Subject: Re: [PATCH v2 2/5] iio: temperature: ltc2983: make bulk write buffer
> DMA-safe
>
> [External]
>
> On Thu, 20 Oct 2022 12:02:54 +0300
> Cosmin Tanislav <[email protected]> wrote:
>
> > From: Cosmin Tanislav <[email protected]>
> >
> > regmap_bulk_write() does not guarantee implicit DMA-safety,
> > even though the current implementation duplicates the given
> > buffer. Do not rely on it.
> >
> > Fixes: f110f3188e56 ("iio: temperature: Add support for LTC2983")
> > Signed-off-by: Cosmin Tanislav <[email protected]>
> LGTM.
>
> As you right observed this is only sort of a fix because right now we
> are fine anyway, so in the interests of getting the rest of the series
> upstream quicker I'll take this one for the next merge window along
> with the rest of the set.
>
> Thanks,
>
> Jonathan
>

Not sure if you already applied this... Anyways:

Reviewed-by: Nuno S? <[email protected]>