* [PATCH v5 0/2] add support for MCP998X
@ 2025-09-18 11:19 victor.duicu
2025-09-18 11:19 ` [PATCH v5 1/2] dt-bindings: iio: temperature: " victor.duicu
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: victor.duicu @ 2025-09-18 11:19 UTC (permalink / raw)
To: jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt
Cc: linux-iio, linux-kernel, devicetree, marius.cristea, victor.duicu
From: Victor Duicu <victor.duicu@microchip.com>
Add support for Microchip MCP998X/33 and MCP998XD/33D Multichannel Automotive Temperature Monitor Family.
The chips in the family have different numbers of external channels, ranging from 1 (MCP9982) to 4 channels (MCP9985).
Reading diodes in anti-parallel connection is supported by MCP9984/85/33 and MCP9984D/85D/33D.
Dedicated hardware shutdown circuitry is present only in MCP998XD and MCP9933D.
Current version of driver does not support interrupts, events and data buffering.
Differences related to previous patch:
v5:
- in yaml edit description of interrupts.
- add min and maxItems to reg.
- remove ideality parameter.
- use pattern recognition in conditionals.
- group conditions based on the chip.
- correct microchip,parasitic-res-on-channel3-4 to true.
- in driver include bitops.h.
- change name of some variables.
- rename mcp9982_parse_of_config() to mcp9982_parse_fw_config().
- implement bulk reading of temp registers.
- lock ideality parameter to default value.
- implement bit flags.
- add compound literal to MCP9982_CHAN.
- remove hysteresis parameter.
- edit comments.
- change values from int to bool in mcp9982_features.
- remove mcp9982_calc_all_3db_values() and hardcode values.
When filter is OFF the 3db value is equal to frequency.
- add .max_register to regmap_config.
- remove devm_kcalloc().
- in mcp9982_read_avail() add an else branch to hw_thermal_shutdown
check.
- in mcp9982_read_raw use USEC_PER_MSEC and set regmap_read_poll_timeout
to never timeout.
Replace switch with bitmap_weight.
- in mcp9982_read_label() remove unnecessary if.
- in mcp9982_write_raw() remove duplicated code.
- in mcp9982_init add error messages when APDD and RECD are incorrectly
set.
- in mcp9982_parse_fw_config() add default for reg_nr.
v4:
- lock beta parameters to default value of beta-autodetect.
Remove beta parameters and checks from devicetree.
- lock temperature range to extended.
This change avoids the issue of the average filter using raw values
with different scales when changing the range.
- change driver to wait an amount of time before reading a raw value
to ensure it is valid.
- change driver to stop calculating the physical temp when reading
in_tempx_raw. Reading from in_tempx_raw will return the raw value.
The physical temp will be calculated with in_tempx_raw, scale and
offset parameters.
- add scale parameter to channel definition.
- initialise chips with "D" to work in Run state and those without
in Standby state.
- when activating the low pass filter for chips without "D",
set the power state to RUN to ensure fresh values for the average.
- add minimum and maximum to microchip,beta1 and microchip,beta2 in yaml.
- rename microchip,resistance-comp-ch1-2-enable and
microchip,resistance-comp-ch3-4-enable to
microchip,parasitic-res-on-channel1-2
and microchip,parasitic-res-on-channel3-4
and edit description in yaml.
- add conditional logic to check if the chip supports APDD
and force default values where necessary in yaml.
- edit comments and coding style.
- replace asm/div64.h with linux/math64.h.
- add delay.h to includes.
- redefine mcp9982_sampl_fr with new structure division.
- in mcp9982_priv remove dev_name,extended_temp_range and beta_values.
Add run_state, wait_before_read, time_limit and pointer to chip
structure to remove all instances of matching strings.
Reorder parameters for memory optimization.
- in mcp9982_features add flags to know if the chip has thermal shutdown
circuitry and supports APDD.
- in mcp9982_read_avail() rework verification of chip type in sampling
frequency case.
- in mcp9982_read_raw() rework switch in low pass filter case.
- in mcp9982_parse_of_config() replace generic -EINVAL code
with -E2BIG and -EOVERFLOW.
v3:
- move beta parameters to devicetree.
- change the name of the interrupts and add
check to match them to the device in yaml.
- remove label for device and remove "0x" from
channel registers in example in yaml.
- edit comments in yaml and driver.
- add minItems to interrupts in yaml.
- rename microchip,recd12 and microchip,recd34 to
microchip,resistance-comp-ch1-2-enable
and microchip,resistance-comp-ch3-4-enable.
- rename microchip,apdd-state to microchip,enable-anti-parallel.
- add static to mcp9982_3db_values_map_tbl to fix
kernel test robot warning.
- in mcp9982_init() add check to ensure that hardware
shutdown feature can't be overridden.
- replace div_u64_rem with do_div and add
asm/div64.h to includes.
- remove unused includes.
- add iio_chan_spec in the macro definition of MCP9982_CHAN.
- remove MCP9982_EXT_BETA_ENBL.
- in mcp9982_init() replace regmap_assign_bits
with regmap_write when setting beta compensation.
- remove custom attribute enable_extended_temp_range and
map it to IIO_CHAN_INFO_OFFSET.
- add unsigned to int variables that allow it.
- reorder parameters in mcp9982_priv, change some
from int to bool, add const to labels and add dev_name.
- add check for chips with "D" in the name to not
allow sampling frequencies lower than 1 to
prevent overriding of hardware shutdown.
- remove mcp9982_attributes.
- move mcp9982_calc_all_3db_values() to before
mcp9982_init().
- use MICRO instead of number constant.
- in mcp9982_write_raw replace ">=" with "==".
- rename index2 to idx in mcp9982_read_raw().
- remove i2c_set_clientdata() in mcp9982_probe().
- since there are no more custom ABI attributes
the testing file was removed.
v2:
- move hysteresis, extended temperature range and beta parameters
from devicetree into user space.
- edit comments in yaml and driver.
- remove "|" in descpriptions, remove "+" from PatternProperties in yaml.
- add default to microchip,ideality-factor, delete blank lines and wrap to
80 chars in yaml.
- remove variables with upper case.
- add check for microchip,apdd-state and microchip,recd34 in yaml.
- improve coding style in driver code.
- add includes for all functions used.
- rename MCP9982_INT_HIGH_BYTE_ADDR to MCP9982_INT_VALUE_ADDR and
MCP9982_INT_LOW_BYTE_ADDR to MCP9982_FRAC_VALUE_ADDR.
- remove custom attribute running_average_window and
running_average_window_available and map them to a low pass filter.
- update sysfs-bus-iio-temperature-mcp9982 to reflect current
driver attributes and point to next kernel version (6.17).
- use compound literal to define driver channels.
- replace device_property_read_string() with i2c_get_match_data() to read
chip name from devicetree.
- remove MCP9982_DEV_ATTR and mcp9982_prep_custom_attributes().
- remove client, chip_name, iio_info from mcp9982_priv.
- replace sprintf() with sysfs_emit().
- remove error messages which are triggered by keyboard input.
- replace devm_kzalloc() with devm_kcalloc(), array mcp9982_chip_config[] with
individual structures, device_property_present() with device_property_read_bool().
- reordered parameters in mcp9982_features and mcp9982_priv to optimize memory
allocation.
- remove .endianness from channel properties.
- change name of some parameters in mcp9982_priv.
- add check for reg value 0 from devicetree (channel 0 is for internal temperature
and can't be disabled).
v1:
- inital version.
Victor Duicu (2):
dt-bindings: iio: temperature: add support for MCP998X
iio: temperature: add support for MCP998X
.../iio/temperature/microchip,mcp9982.yaml | 186 ++++
MAINTAINERS | 7 +
drivers/iio/temperature/Kconfig | 10 +
drivers/iio/temperature/Makefile | 1 +
drivers/iio/temperature/mcp9982.c | 871 ++++++++++++++++++
5 files changed, 1075 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/temperature/microchip,mcp9982.yaml
create mode 100644 drivers/iio/temperature/mcp9982.c
base-commit: 671b9b6d7f4fe17a174c410397e72253877ca64e
--
2.48.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v5 1/2] dt-bindings: iio: temperature: add support for MCP998X
2025-09-18 11:19 [PATCH v5 0/2] add support for MCP998X victor.duicu
@ 2025-09-18 11:19 ` victor.duicu
2025-09-18 14:56 ` Conor Dooley
2025-09-18 11:19 ` [PATCH v5 2/2] " victor.duicu
2025-09-20 11:42 ` [PATCH v5 0/2] " Jonathan Cameron
2 siblings, 1 reply; 11+ messages in thread
From: victor.duicu @ 2025-09-18 11:19 UTC (permalink / raw)
To: jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt
Cc: linux-iio, linux-kernel, devicetree, marius.cristea, victor.duicu
From: Victor Duicu <victor.duicu@microchip.com>
This is the devicetree schema for Microchip MCP998X/33 and MCP998XD/33D
Multichannel Automotive Temperature Monitor Family.
Signed-off-by: Victor Duicu <victor.duicu@microchip.com>
---
.../iio/temperature/microchip,mcp9982.yaml | 186 ++++++++++++++++++
MAINTAINERS | 6 +
2 files changed, 192 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/temperature/microchip,mcp9982.yaml
diff --git a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9982.yaml b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9982.yaml
new file mode 100644
index 000000000000..5aaf9ae2b862
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9982.yaml
@@ -0,0 +1,186 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/temperature/microchip,mcp9982.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip MCP998X/33 and MCP998XD/33D Multichannel Automotive
+ Temperature Monitor Family
+
+maintainers:
+ - Victor Duicu <victor.duicu@microchip.com>
+
+description: |
+ The MCP998X/33 and MCP998XD/33D family is a high-accuracy 2-wire multichannel
+ automotive temperature monitor.
+ The datasheet can be found here:
+ https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP998X-Family-Data-Sheet-DS20006827.pdf
+
+properties:
+ compatible:
+ enum:
+ - microchip,mcp9933
+ - microchip,mcp9933d
+ - microchip,mcp9982
+ - microchip,mcp9982d
+ - microchip,mcp9983
+ - microchip,mcp9983d
+ - microchip,mcp9984
+ - microchip,mcp9984d
+ - microchip,mcp9985
+ - microchip,mcp9985d
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ items:
+ - description: Signal coming from ALERT/THERM pin.
+ - description: Signal coming from THERM/ADDR pin.
+ - description: Signal coming from SYS_SHDN pin.
+
+ interrupt-names:
+ items:
+ - const: alert-therm
+ - const: therm-addr
+ - const: sys-shutdown
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ microchip,enable-anti-parallel:
+ description:
+ Enable anti-parallel diode mode operation.
+ MCP9984/84D/85/85D and MCP9933/33D support reading two external diodes
+ in anti-parallel connection on the same set of pins.
+ type: boolean
+
+ microchip,parasitic-res-on-channel1-2:
+ description:
+ Indicates that the chip and the diodes/transistors are sufficiently far
+ apart that a parasitic resistance is added to the wires, which can affect
+ the measurements. Due to the anti-parallel diode connections, channels
+ 1 and 2 are affected together.
+ type: boolean
+
+ microchip,parasitic-res-on-channel3-4:
+ description:
+ Indicates that the chip and the diodes/transistors are sufficiently far
+ apart that a parasitic resistance is added to the wires, which can affect
+ the measurements. Due to the anti-parallel diode connections, channels
+ 3 and 4 are affected together.
+ type: boolean
+
+ vdd-supply: true
+
+patternProperties:
+ "^channel@[1-4]$":
+ description:
+ Represents the external temperature channels to which
+ a remote diode is connected.
+ type: object
+
+ properties:
+ reg:
+ items:
+ minItems: 1
+ maxItems: 4
+
+ label:
+ description: Unique name to identify which channel this is.
+
+ required:
+ - reg
+
+ additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - vdd-supply
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ pattern: '^.+d$'
+ then:
+ properties:
+ interrupts-names:
+ items:
+ - const: alert-therm
+ - const: sys-shutdown
+ else:
+ properties:
+ interrupts-names:
+ items:
+ - const: alert-therm
+ - const: therm-addr
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ pattern: '^microchip,mcp998(2|3)$'
+ then:
+ properties:
+ microchip,enable-anti-parallel: false
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ pattern: '^microchip,mcp998(2|3)d$'
+ then:
+ properties:
+ microchip,enable-anti-parallel: false
+ microchip,parasitic-res-on-channel1-2: true
+ microchip,parasitic-res-on-channel3-4: true
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ pattern: 'microchip,mcp99(33|8[4-5])d$'
+ then:
+ properties:
+ microchip,parasitic-res-on-channel1-2: true
+ microchip,parasitic-res-on-channel3-4: true
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ temperature-sensor@4c {
+ compatible = "microchip,mcp9985";
+ reg = <0x4c>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ microchip,enable-anti-parallel;
+ microchip,parasitic-res-on-channel1-2;
+ microchip,parasitic-res-on-channel3-4;
+ vdd-supply = <&vdd>;
+
+ channel@1 {
+ reg = <1>;
+ label = "CPU Temperature";
+ };
+
+ channel@2 {
+ reg = <2>;
+ label = "GPU Temperature";
+ };
+ };
+ };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index d53a536288ca..37530194c9fb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16662,6 +16662,12 @@ S: Maintained
F: Documentation/devicetree/bindings/iio/adc/microchip,mcp3911.yaml
F: drivers/iio/adc/mcp3911.c
+MICROCHIP MCP9982 TEMPERATURE DRIVER
+M: Victor Duicu <victor.duicu@microchip.com>
+L: linux-iio@vger.kernel.org
+S: Supported
+F: Documentation/devicetree/bindings/iio/temperature/microchip,mcp9982.yaml
+
MICROCHIP MMC/SD/SDIO MCI DRIVER
M: Aubin Constans <aubin.constans@microchip.com>
S: Maintained
--
2.48.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v5 2/2] iio: temperature: add support for MCP998X
2025-09-18 11:19 [PATCH v5 0/2] add support for MCP998X victor.duicu
2025-09-18 11:19 ` [PATCH v5 1/2] dt-bindings: iio: temperature: " victor.duicu
@ 2025-09-18 11:19 ` victor.duicu
2025-09-19 6:27 ` kernel test robot
` (2 more replies)
2025-09-20 11:42 ` [PATCH v5 0/2] " Jonathan Cameron
2 siblings, 3 replies; 11+ messages in thread
From: victor.duicu @ 2025-09-18 11:19 UTC (permalink / raw)
To: jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt
Cc: linux-iio, linux-kernel, devicetree, marius.cristea, victor.duicu
From: Victor Duicu <victor.duicu@microchip.com>
This is the driver for Microchip MCP998X/33 and MCP998XD/33D Multichannel
Automotive Temperature Monitor Family.
Signed-off-by: Victor Duicu <victor.duicu@microchip.com>
---
MAINTAINERS | 1 +
drivers/iio/temperature/Kconfig | 10 +
drivers/iio/temperature/Makefile | 1 +
drivers/iio/temperature/mcp9982.c | 871 ++++++++++++++++++++++++++++++
4 files changed, 883 insertions(+)
create mode 100644 drivers/iio/temperature/mcp9982.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 37530194c9fb..9aa9d8e29462 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16667,6 +16667,7 @@ M: Victor Duicu <victor.duicu@microchip.com>
L: linux-iio@vger.kernel.org
S: Supported
F: Documentation/devicetree/bindings/iio/temperature/microchip,mcp9982.yaml
+F: drivers/iio/temperature/mcp9982.c
MICROCHIP MMC/SD/SDIO MCI DRIVER
M: Aubin Constans <aubin.constans@microchip.com>
diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
index 9328b2250ace..d1751db6bf6f 100644
--- a/drivers/iio/temperature/Kconfig
+++ b/drivers/iio/temperature/Kconfig
@@ -184,4 +184,14 @@ config MCP9600
This driver can also be built as a module. If so, the module
will be called mcp9600.
+config MCP9982
+ tristate "Microchip Technology MCP9982 driver"
+ depends on I2C
+ help
+ Say yes here to build support for Microchip Technology's MCP998X/33
+ and MCP998XD/33D Multichannel Automotive Temperature Monitor Family.
+
+ This driver can also be built as a module. If so, the module
+ will be called mcp9982.
+
endmenu
diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
index 07d6e65709f7..83f5f4bb4ff3 100644
--- a/drivers/iio/temperature/Makefile
+++ b/drivers/iio/temperature/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_MAX30208) += max30208.o
obj-$(CONFIG_MAX31856) += max31856.o
obj-$(CONFIG_MAX31865) += max31865.o
obj-$(CONFIG_MCP9600) += mcp9600.o
+obj-$(CONFIG_MCP9982) += mcp9982.o
obj-$(CONFIG_MLX90614) += mlx90614.o
obj-$(CONFIG_MLX90632) += mlx90632.o
obj-$(CONFIG_MLX90632) += mlx90635.o
diff --git a/drivers/iio/temperature/mcp9982.c b/drivers/iio/temperature/mcp9982.c
new file mode 100644
index 000000000000..05130b72ba14
--- /dev/null
+++ b/drivers/iio/temperature/mcp9982.c
@@ -0,0 +1,871 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * IIO driver for MCP998X/33 and MCP998XD/33D Multichannel Automotive Temperature Monitor Family
+ *
+ * Copyright (C) 2025 Microchip Technology Inc. and its subsidiaries
+ *
+ * Author: Victor Duicu <victor.duicu@microchip.com>
+ *
+ * Datasheet can be found here:
+ * https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP998X-Family-Data-Sheet-DS20006827.pdf
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/device/devres.h>
+#include <linux/dev_printk.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/math64.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/string.h>
+#include <linux/units.h>
+
+/* MCP9982 Registers */
+#define MCP9982_HIGH_BYTE_ADDR(index) (2 * (index))
+#define MCP9982_ONE_SHOT_ADDR 0x0A
+#define MCP9982_INTERNAL_HIGH_LIMIT_ADDR 0x0B
+#define MCP9982_INTERNAL_LOW_LIMIT_ADDR 0x0C
+#define MCP9982_EXT1_HIGH_LIMIT_HIGH_BYTE_ADDR 0x0D
+#define MCP9982_EXT1_HIGH_LIMIT_LOW_BYTE_ADDR 0x0E
+#define MCP9982_EXT1_LOW_LIMIT_HIGH_BYTE_ADDR 0x0F
+#define MCP9982_EXT1_LOW_LIMIT_LOW_BYTE_ADDR 0x10
+#define MCP9982_INTERNAL_THERM_LIMIT_ADDR 0x1D
+#define MCP9982_EXT1_THERM_LIMIT_ADDR 0x1E
+#define MCP9982_CFG_ADDR 0x22
+#define MCP9982_CONV_ADDR 0x24
+#define MCP9982_HYS_ADDR 0x25
+#define MCP9982_CONSEC_ALRT_ADDR 0x26
+#define MCP9982_ALRT_CFG_ADDR 0x27
+#define MCP9982_RUNNING_AVG_ADDR 0x28
+#define MCP9982_HOTTEST_CFG_ADDR 0x29
+#define MCP9982_STATUS_ADDR 0x2A
+#define MCP9982_EXT_FAULT_STATUS_ADDR 0x2B
+#define MCP9982_HIGH_LIMIT_STATUS_ADDR 0x2C
+#define MCP9982_LOW_LIMIT_STATUS_ADDR 0x2D
+#define MCP9982_THERM_LIMIT_STATUS_ADDR 0x2E
+#define MCP9982_HOTTEST_HIGH_BYTE_ADDR 0x2F
+#define MCP9982_HOTTEST_LOW_BYTE_ADDR 0x30
+#define MCP9982_HOTTEST_STATUS_ADDR 0x31
+#define MCP9982_THERM_SHTDWN_CFG_ADDR 0x32
+#define MCP9982_HRDW_THERM_SHTDWN_LIMIT_ADDR 0x33
+/* 52 is the start address in decimal for the beta registers. */
+#define MCP9982_EXT_BETA_CFG_ADDR(index) ((index) + 52)
+/* 54 is the start address in decimal for ideality registers. */
+#define MCP9982_EXT_IDEAL_ADDR(index) ((index) + 54)
+/* 128 is the start address in decimal for temperature memory block */
+#define MCP9982_TEMP_MEM_BLOCK_ADDR(index) (2 * (index) + 128)
+#define MCP9982_TEMP_MEMORY_BLOCK_LOW 0x80
+#define MCP9982_TEMP_MEMORY_BLOCK_HIGH 0x89
+
+/* MCP9982 Bits */
+#define MCP9982_CFG_MSKAL BIT(7)
+#define MCP9982_CFG_RS BIT(6)
+#define MCP9982_CFG_ATTHM BIT(5)
+#define MCP9982_CFG_RECD12 BIT(4)
+#define MCP9982_CFG_RECD34 BIT(3)
+#define MCP9982_CFG_RANGE BIT(2)
+#define MCP9982_CFG_DA_ENA BIT(1)
+#define MCP9982_CFG_APDD BIT(0)
+#define MCP9982_STATUS_BUSY BIT(5)
+
+/* The maximum number of channels a member of the family can have. */
+#define MCP9982_MAX_NUM_CHANNELS 5
+#define MCP9982_BETA_AUTODETECT 16
+#define MCP9982_IDEALITY_DEFAULT 18
+#define MCP9982_OFFSET -64
+#define MCP9982_SCALE 3906250
+/**
+ * Bit flags and their meaning
+ * @RECD34_ENABLE: state of Resistance Error Correction(REC) on channels 3 and 4
+ * @RECD12_ENABLE: state of Resistance Error Correction(REC) on channels 1 and 2
+ * @APDD_ENABLE: state of anti-parallel diode mode
+ * @RUN_STATE: chip is in run state, otherwise is in standby state
+ * @WAIT_BEFORE_READ: whether we need to wait a delay before reading a new value
+ */
+#define RECD34_ENABLE 0
+#define RECD12_ENABLE 1
+#define APDD_ENABLE 2
+#define RUN_STATE 3
+#define WAIT_BEFORE_READ 4
+#define USE_PREVIOUS_FREQ 5
+
+#define MCP9982_CHAN(index, si, __address) ( \
+ (struct iio_chan_spec) { \
+ .type = IIO_TEMP, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
+ BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY), \
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
+ BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY) | \
+ BIT(IIO_CHAN_INFO_OFFSET) | \
+ BIT(IIO_CHAN_INFO_SCALE), \
+ .channel = index, \
+ .address = __address, \
+ .scan_index = si, \
+ .scan_type = { \
+ .sign = 'u', \
+ .realbits = 8, \
+ .storagebits = 8, \
+ }, \
+ .indexed = 1, \
+ } \
+)
+
+/**
+ * struct mcp9982_features - features of a mcp9982 instance
+ * @name: chip's name
+ * @phys_channels: number of physical channels supported by the chip
+ * @hw_thermal_shutdown: presence of hardware thermal shutdown circuitry
+ * @allow_apdd: whether the chip supports enabling APDD
+ */
+struct mcp9982_features {
+ const char *name;
+ u8 phys_channels;
+ bool hw_thermal_shutdown;
+ bool allow_apdd;
+};
+
+static const struct mcp9982_features mcp9933_chip_config = {
+ .name = "mcp9933",
+ .phys_channels = 3,
+ .hw_thermal_shutdown = false,
+ .allow_apdd = true,
+};
+
+static const struct mcp9982_features mcp9933d_chip_config = {
+ .name = "mcp9933d",
+ .phys_channels = 3,
+ .hw_thermal_shutdown = true,
+ .allow_apdd = true,
+};
+
+static const struct mcp9982_features mcp9982_chip_config = {
+ .name = "mcp9982",
+ .phys_channels = 2,
+ .hw_thermal_shutdown = false,
+ .allow_apdd = false,
+};
+
+static const struct mcp9982_features mcp9982d_chip_config = {
+ .name = "mcp9982d",
+ .phys_channels = 2,
+ .hw_thermal_shutdown = true,
+ .allow_apdd = false,
+};
+
+static const struct mcp9982_features mcp9983_chip_config = {
+ .name = "mcp9983",
+ .phys_channels = 3,
+ .hw_thermal_shutdown = false,
+ .allow_apdd = false,
+};
+
+static const struct mcp9982_features mcp9983d_chip_config = {
+ .name = "mcp9983d",
+ .phys_channels = 3,
+ .hw_thermal_shutdown = true,
+ .allow_apdd = false,
+};
+
+static const struct mcp9982_features mcp9984_chip_config = {
+ .name = "mcp9984",
+ .phys_channels = 4,
+ .hw_thermal_shutdown = false,
+ .allow_apdd = true,
+};
+
+static const struct mcp9982_features mcp9984d_chip_config = {
+ .name = "mcp9984d",
+ .phys_channels = 4,
+ .hw_thermal_shutdown = true,
+ .allow_apdd = true,
+};
+
+static const struct mcp9982_features mcp9985_chip_config = {
+ .name = "mcp9985",
+ .phys_channels = 5,
+ .hw_thermal_shutdown = false,
+ .allow_apdd = true,
+};
+
+static const struct mcp9982_features mcp9985d_chip_config = {
+ .name = "mcp9985d",
+ .phys_channels = 5,
+ .hw_thermal_shutdown = true,
+ .allow_apdd = true,
+};
+
+static const unsigned int mcp9982_conv_rate[][2] = {
+ { 0, 62500 },
+ { 0, 125000 },
+ { 0, 250000 },
+ { 0, 500000 },
+ { 1, 0 },
+ { 2, 0 },
+ { 4, 0 },
+ { 8, 0 },
+ { 16, 0 },
+ { 32, 0 },
+ { 64, 0 },
+};
+
+/*
+ * Constants were calculated using:
+ * (Sampling_Frequency(Hz) * 1000000) / (Window_Size * 2)
+ * The formula is used for Window_Size = {4, 8}.
+ * For Window_Size = 1 the filter is OFF and the 3db value
+ * is equal to the frequency.
+ */
+static const unsigned int mcp9982_3db_values_map_tbl[11][3][2] = {
+ {
+ {0, 62500},
+ {0, 7812},
+ {0, 3906},
+ },
+ {
+ {0, 125000},
+ {0, 15625},
+ {0, 7812},
+ },
+ {
+ {0, 250000},
+ {0, 31250},
+ {0, 15625},
+ },
+ {
+ {0, 500000},
+ {0, 62500},
+ {0, 31250},
+ },
+ {
+ {1, 0},
+ {0, 125000},
+ {0, 62500},
+ },
+ {
+ {2, 0},
+ {0, 250000},
+ {0, 125000},
+ },
+ {
+ {4, 0},
+ {0, 500000},
+ {0, 250000},
+ },
+ {
+ {8, 0},
+ {1, 0},
+ {0, 500000},
+ },
+ {
+ {16, 0},
+ {2, 0},
+ {1, 0},
+ },
+ {
+ {32, 0},
+ {4, 0},
+ {2, 0},
+ },
+ {
+ {64, 0},
+ {8, 0},
+ {4, 0},
+ },
+};
+
+/* The delay, in milliseconds, needed to allow the conversion to end. */
+static const u64 mcp9982_delay_ms[11] = {
+ 16125,
+ 8125,
+ 4125,
+ 2125,
+ 1125,
+ 625,
+ 375,
+ 255,
+ 190,
+ 160,
+ 145,
+};
+
+/* MCP9982 regmap configuration */
+static const struct regmap_range mcp9982_regmap_wr_ranges[] = {
+ regmap_reg_range(MCP9982_ONE_SHOT_ADDR,
+ MCP9982_EXT1_LOW_LIMIT_LOW_BYTE_ADDR),
+ regmap_reg_range(MCP9982_INTERNAL_THERM_LIMIT_ADDR,
+ MCP9982_EXT1_THERM_LIMIT_ADDR),
+ regmap_reg_range(MCP9982_CFG_ADDR, MCP9982_CFG_ADDR),
+ regmap_reg_range(MCP9982_CONV_ADDR, MCP9982_HOTTEST_CFG_ADDR),
+ regmap_reg_range(MCP9982_THERM_SHTDWN_CFG_ADDR,
+ MCP9982_THERM_SHTDWN_CFG_ADDR),
+ regmap_reg_range(MCP9982_EXT_BETA_CFG_ADDR(0),
+ MCP9982_EXT_IDEAL_ADDR(3)),
+ regmap_reg_range(MCP9982_TEMP_MEMORY_BLOCK_LOW,
+ MCP9982_TEMP_MEMORY_BLOCK_HIGH),
+};
+
+static const struct regmap_access_table mcp9982_regmap_wr_table = {
+ .yes_ranges = mcp9982_regmap_wr_ranges,
+ .n_yes_ranges = ARRAY_SIZE(mcp9982_regmap_wr_ranges),
+};
+
+static const struct regmap_range mcp9982_regmap_rd_ranges[] = {
+ regmap_reg_range(MCP9982_HIGH_BYTE_ADDR(0),
+ MCP9982_EXT1_LOW_LIMIT_LOW_BYTE_ADDR),
+ regmap_reg_range(MCP9982_INTERNAL_THERM_LIMIT_ADDR,
+ MCP9982_EXT1_THERM_LIMIT_ADDR),
+ regmap_reg_range(MCP9982_CFG_ADDR, MCP9982_CFG_ADDR),
+ regmap_reg_range(MCP9982_CONV_ADDR, MCP9982_EXT_IDEAL_ADDR(3)),
+ regmap_reg_range(MCP9982_TEMP_MEMORY_BLOCK_LOW,
+ MCP9982_TEMP_MEMORY_BLOCK_HIGH),
+};
+
+static const struct regmap_access_table mcp9982_regmap_rd_table = {
+ .yes_ranges = mcp9982_regmap_rd_ranges,
+ .n_yes_ranges = ARRAY_SIZE(mcp9982_regmap_rd_ranges),
+};
+
+static bool mcp9982_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case MCP9982_ONE_SHOT_ADDR:
+ case MCP9982_INTERNAL_HIGH_LIMIT_ADDR:
+ case MCP9982_INTERNAL_LOW_LIMIT_ADDR:
+ case MCP9982_EXT1_HIGH_LIMIT_HIGH_BYTE_ADDR:
+ case MCP9982_EXT1_HIGH_LIMIT_LOW_BYTE_ADDR:
+ case MCP9982_EXT1_LOW_LIMIT_HIGH_BYTE_ADDR:
+ case MCP9982_EXT1_LOW_LIMIT_LOW_BYTE_ADDR:
+ case MCP9982_INTERNAL_THERM_LIMIT_ADDR:
+ case MCP9982_EXT1_THERM_LIMIT_ADDR:
+ case MCP9982_CFG_ADDR:
+ case MCP9982_CONV_ADDR:
+ case MCP9982_HYS_ADDR:
+ case MCP9982_CONSEC_ALRT_ADDR:
+ case MCP9982_ALRT_CFG_ADDR:
+ case MCP9982_RUNNING_AVG_ADDR:
+ case MCP9982_HOTTEST_CFG_ADDR:
+ case MCP9982_THERM_SHTDWN_CFG_ADDR:
+ return false;
+ default:
+ return true;
+ }
+}
+
+static const struct regmap_config mcp9982_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .rd_table = &mcp9982_regmap_rd_table,
+ .wr_table = &mcp9982_regmap_wr_table,
+ .volatile_reg = mcp9982_is_volatile_reg,
+ .max_register = 137,
+};
+
+/**
+ * struct mcp9992_priv - information about chip parameters
+ * @bit_flags: holds the state of the flags
+ * @regmap: device register map
+ * @chip: pointer to structure holding chip features
+ * @lock: synchronize access to driver's state members
+ * @iio_chan: specifications of channels
+ * @labels: labels of the channels
+ * @sampl_idx: index representing the current sampling frequency
+ * @time_limit: time when it is safe to read
+ * @num_channels: number of active physical channels
+ */
+struct mcp9982_priv {
+ unsigned long bit_flags;
+ struct regmap *regmap;
+ const struct mcp9982_features *chip;
+ /* Synchronize access to driver's state members. */
+ struct mutex lock;
+ struct iio_chan_spec iio_chan[5];
+ const char *labels[MCP9982_MAX_NUM_CHANNELS];
+ unsigned int sampl_idx;
+ unsigned long time_limit;
+ u8 num_channels;
+
+};
+
+static int mcp9982_read_avail(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, const int **vals,
+ int *type, int *length, long mask)
+{
+ struct mcp9982_priv *priv = iio_priv(indio_dev);
+ unsigned int idx;
+ unsigned int sub;
+
+ if (priv->chip->hw_thermal_shutdown) {
+ idx = 4;
+ sub = 8;
+ } else {
+ idx = 0;
+ sub = 0;
+ }
+ switch (mask) {
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ *type = IIO_VAL_INT_PLUS_MICRO;
+ *vals = mcp9982_conv_rate[idx];
+ *length = ARRAY_SIZE(mcp9982_conv_rate) * 2 - sub;
+ return IIO_AVAIL_LIST;
+ case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+ *type = IIO_VAL_INT_PLUS_MICRO;
+ *vals = mcp9982_3db_values_map_tbl[priv->sampl_idx][0];
+ *length = ARRAY_SIZE(mcp9982_3db_values_map_tbl[priv->sampl_idx]) * 2;
+ return IIO_AVAIL_LIST;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int mcp9982_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int *val,
+ int *val2, long mask)
+{
+ unsigned int tmp_reg, reg_status;
+ struct mcp9982_priv *priv = iio_priv(indio_dev);
+ int ret;
+
+ if (test_bit(RUN_STATE, &priv->bit_flags)) {
+ /*
+ * When working in Run mode, after modifying a parameter (like sampling
+ * frequency) we have to wait a delay before reading the new values.
+ * We can't determine when the conversion is done based on the BUSY bit.
+ */
+ if (test_bit(WAIT_BEFORE_READ, &priv->bit_flags)) {
+ if (!time_after(jiffies, priv->time_limit))
+ mdelay(jiffies_to_msecs(priv->time_limit - jiffies));
+ clear_bit(WAIT_BEFORE_READ, &priv->bit_flags);
+ }
+ } else {
+ ret = regmap_write(priv->regmap, MCP9982_ONE_SHOT_ADDR, 1);
+ if (ret)
+ return ret;
+ /*
+ * In Standby state after writing in OneShot register wait for
+ * the start of conversion and then poll the BUSY bit.
+ */
+ mdelay(125);
+ ret = regmap_read_poll_timeout(priv->regmap, MCP9982_STATUS_ADDR,
+ reg_status, !(reg_status & MCP9982_STATUS_BUSY),
+ mcp9982_delay_ms[priv->sampl_idx] * USEC_PER_MSEC,
+ 0);
+ if (ret)
+ return ret;
+ }
+ guard(mutex)(&priv->lock);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ /*
+ * The Block Read Protocol first returns the number of user readable
+ * bytes, held in bulk_read[0], followed by the data.
+ */
+ u8 bulk_read[3];
+
+ ret = regmap_bulk_read(priv->regmap, MCP9982_TEMP_MEM_BLOCK_ADDR(chan->channel),
+ &bulk_read, sizeof(bulk_read));
+ if (ret)
+ return ret;
+
+ *val = (bulk_read[1] << 8) + (bulk_read[2]);
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ *val = 0;
+ *val2 = MCP9982_SCALE;
+ return IIO_VAL_INT_PLUS_NANO;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ *val = mcp9982_conv_rate[priv->sampl_idx][0];
+ *val2 = mcp9982_conv_rate[priv->sampl_idx][1];
+ return IIO_VAL_INT_PLUS_MICRO;
+ case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+ unsigned long *src;
+
+ ret = regmap_read(priv->regmap, MCP9982_RUNNING_AVG_ADDR, &tmp_reg);
+ if (ret)
+ return ret;
+ *src = tmp_reg;
+ *val = mcp9982_3db_values_map_tbl[priv->sampl_idx][bitmap_weight(src, 2)][0];
+ *val2 = mcp9982_3db_values_map_tbl[priv->sampl_idx][bitmap_weight(src, 2)][1];
+ return IIO_VAL_INT_PLUS_MICRO;
+ case IIO_CHAN_INFO_OFFSET:
+ *val = MCP9982_OFFSET;
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int mcp9982_read_label(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, char *label)
+{
+ struct mcp9982_priv *priv = iio_priv(indio_dev);
+
+ return sysfs_emit(label, "%s\n", priv->labels[chan->channel]);
+}
+
+static int mcp9982_write_raw_get_fmt(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, long info)
+{
+ switch (info) {
+ case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ return IIO_VAL_INT_PLUS_MICRO;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int mcp9982_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int val,
+ int val2, long mask)
+{
+ unsigned int i, start, previous_sampl_idx;
+ struct mcp9982_priv *priv = iio_priv(indio_dev);
+ int ret;
+ unsigned long new_time_limit;
+
+ start = 0;
+ guard(mutex)(&priv->lock);
+ switch (mask) {
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ previous_sampl_idx = priv->sampl_idx;
+
+ /* For MCP998XD and MCP9933D sampling frequency can't be set lower than 1. */
+ if (priv->chip->hw_thermal_shutdown)
+ start = 4;
+ for (i = start; i < ARRAY_SIZE(mcp9982_conv_rate); i++)
+ if (val == mcp9982_conv_rate[i][0] &&
+ val2 == mcp9982_conv_rate[i][1])
+ break;
+
+ if (i == ARRAY_SIZE(mcp9982_conv_rate))
+ return -EINVAL;
+
+ ret = regmap_write(priv->regmap, MCP9982_CONV_ADDR, i);
+ if (ret)
+ return ret;
+
+ priv->sampl_idx = i;
+
+ /*
+ * When changing the frequency in Run mode, wait a delay based
+ * on the previous value to ensure the new value becomes active.
+ */
+ if (test_bit(RUN_STATE, &priv->bit_flags))
+ set_bit(USE_PREVIOUS_FREQ, &priv->bit_flags);
+
+ break;
+ case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+ for (i = 0; i < ARRAY_SIZE(mcp9982_3db_values_map_tbl[priv->sampl_idx]); i++)
+ if (val == mcp9982_3db_values_map_tbl[priv->sampl_idx][i][0] &&
+ val2 == mcp9982_3db_values_map_tbl[priv->sampl_idx][i][1])
+ break;
+
+ if (i == ARRAY_SIZE(mcp9982_3db_values_map_tbl[priv->sampl_idx]))
+ return -EINVAL;
+
+ /*
+ * In mcp9982_3db_values_map_tbl the second index maps:
+ * 0 for filter off
+ * 1 for filter at level 1
+ * 2 for filter at level 2
+ */
+ if (i == 2)
+ i = 3;
+ /*
+ * If the digital filter is activated for chips without "D", set
+ * the power state to Run to ensure the averaging is made on fresh values.
+ */
+ if (!priv->chip->hw_thermal_shutdown) {
+ if (i == 0) {
+ ret = regmap_assign_bits(priv->regmap,
+ MCP9982_CFG_ADDR,
+ MCP9982_CFG_RS, 1);
+ clear_bit(RUN_STATE, &priv->bit_flags);
+ } else {
+ ret = regmap_assign_bits(priv->regmap,
+ MCP9982_CFG_ADDR,
+ MCP9982_CFG_RS, 0);
+ set_bit(RUN_STATE, &priv->bit_flags);
+ }
+ }
+
+ ret = regmap_write(priv->regmap, MCP9982_RUNNING_AVG_ADDR, i);
+ if (ret)
+ return ret;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (test_bit(RUN_STATE, &priv->bit_flags)) {
+ if (test_bit(USE_PREVIOUS_FREQ, &priv->bit_flags)) {
+ new_time_limit = jiffies +
+ msecs_to_jiffies(mcp9982_delay_ms[previous_sampl_idx]);
+ clear_bit(USE_PREVIOUS_FREQ, &priv->bit_flags);
+ } else {
+ new_time_limit = jiffies +
+ msecs_to_jiffies(mcp9982_delay_ms[priv->sampl_idx]);
+ }
+ if (time_after(new_time_limit, priv->time_limit)) {
+ priv->time_limit = new_time_limit;
+ set_bit(WAIT_BEFORE_READ, &priv->bit_flags);
+ }
+ }
+
+ return 0;
+}
+
+static const struct iio_info mcp9982_info = {
+ .read_raw = mcp9982_read_raw,
+ .read_label = mcp9982_read_label,
+ .read_avail = mcp9982_read_avail,
+ .write_raw_get_fmt = mcp9982_write_raw_get_fmt,
+ .write_raw = mcp9982_write_raw,
+};
+
+static int mcp9982_init(struct device *dev, struct mcp9982_priv *priv)
+{
+ int ret;
+ unsigned int i;
+ u8 val;
+
+ /* Chips 82/83 and 82D/83D do not support anti-parallel diode mode. */
+ if (!priv->chip->allow_apdd && test_bit(APDD_ENABLE, &priv->bit_flags))
+ return dev_err_probe(dev, -EINVAL, "Incorrect setting of APDD.\n");
+
+ /* Chips with "D" work in Run state and those without work in Standby state. */
+ if (priv->chip->hw_thermal_shutdown)
+ set_bit(RUN_STATE, &priv->bit_flags);
+
+ /*
+ * For chips with "D" in the name resistance error correction must be on
+ * so that hardware shutdown feature can't be overridden.
+ */
+ if (priv->chip->hw_thermal_shutdown)
+ if (!test_bit(RECD34_ENABLE, &priv->bit_flags) ||
+ !test_bit(RECD12_ENABLE, &priv->bit_flags))
+ return dev_err_probe(dev, -EINVAL, "Incorrect setting of RECD.\n");
+ /*
+ * Set default values in registers.
+ *
+ * APDD, RECD12 and RECD34 are active on 0.
+ */
+ val = FIELD_PREP(MCP9982_CFG_MSKAL, 1) |
+ FIELD_PREP(MCP9982_CFG_RS, !test_bit(RUN_STATE, &priv->bit_flags)) |
+ FIELD_PREP(MCP9982_CFG_ATTHM, 1) |
+ FIELD_PREP(MCP9982_CFG_RECD12, !test_bit(RECD12_ENABLE, &priv->bit_flags)) |
+ FIELD_PREP(MCP9982_CFG_RECD34, !test_bit(RECD34_ENABLE, &priv->bit_flags)) |
+ FIELD_PREP(MCP9982_CFG_RANGE, 1) | FIELD_PREP(MCP9982_CFG_DA_ENA, 0) |
+ FIELD_PREP(MCP9982_CFG_APDD, !test_bit(APDD_ENABLE, &priv->bit_flags));
+
+ ret = regmap_write(priv->regmap, MCP9982_CFG_ADDR, val);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(priv->regmap, MCP9982_CONV_ADDR, 6);
+ if (ret)
+ return ret;
+ priv->sampl_idx = 6;
+
+ ret = regmap_write(priv->regmap, MCP9982_HYS_ADDR, 10);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(priv->regmap, MCP9982_CONSEC_ALRT_ADDR, 112);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(priv->regmap, MCP9982_RUNNING_AVG_ADDR, 0);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(priv->regmap, MCP9982_HOTTEST_CFG_ADDR, 0);
+ if (ret)
+ return ret;
+
+ /*
+ * Only external channels 1 and 2 support beta compensation.
+ * Set beta auto-detection.
+ */
+ for (i = 0; i < 2; i++) {
+ ret = regmap_write(priv->regmap, MCP9982_EXT_BETA_CFG_ADDR(i),
+ MCP9982_BETA_AUTODETECT);
+ if (ret)
+ return ret;
+ }
+ /* Set ideality factor to default for all external channels. */
+ for (i = 0; i < 4; i++) {
+ ret = regmap_write(priv->regmap, MCP9982_EXT_IDEAL_ADDR(i),
+ MCP9982_IDEALITY_DEFAULT);
+ if (ret)
+ return ret;
+ }
+
+ priv->time_limit = jiffies;
+
+ return 0;
+}
+
+static int mcp9982_parse_fw_config(struct iio_dev *indio_dev, struct device *dev,
+ int device_nr_channels)
+{
+ unsigned int reg_nr, iio_idx;
+ struct mcp9982_priv *priv = iio_priv(indio_dev);
+
+ if (device_property_read_bool(dev, "microchip,enable-anti-parallel"))
+ set_bit(APDD_ENABLE, &priv->bit_flags);
+
+ if (device_property_read_bool(dev, "microchip,parasitic-res-on-channel1-2"))
+ set_bit(RECD12_ENABLE, &priv->bit_flags);
+
+ if (device_property_read_bool(dev, "microchip,parasitic-res-on-channel3-4"))
+ set_bit(RECD34_ENABLE, &priv->bit_flags);
+
+ priv->num_channels = device_get_child_node_count(dev) + 1;
+
+ if (priv->num_channels > device_nr_channels)
+ return dev_err_probe(dev, -E2BIG,
+ "More channels than the chip supports\n");
+
+ priv->iio_chan[0] = MCP9982_CHAN(0, 0, MCP9982_HIGH_BYTE_ADDR(0));
+
+ priv->labels[0] = "internal diode";
+ iio_idx++;
+ device_for_each_child_node_scoped(dev, child) {
+ reg_nr = 0;
+ fwnode_property_read_u32(child, "reg", ®_nr);
+ if (!reg_nr || reg_nr >= device_nr_channels)
+ return dev_err_probe(dev, -EINVAL,
+ "The index of the channels does not match the chip\n");
+
+ fwnode_property_read_string(child, "label",
+ &priv->labels[reg_nr]);
+
+ priv->iio_chan[iio_idx++] = MCP9982_CHAN(reg_nr, reg_nr,
+ MCP9982_HIGH_BYTE_ADDR(reg_nr));
+ }
+
+ return 0;
+}
+
+static int mcp9982_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct mcp9982_priv *priv;
+ struct iio_dev *indio_dev;
+ const struct mcp9982_features *chip;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*priv));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ priv = iio_priv(indio_dev);
+ priv->regmap = devm_regmap_init_i2c(client, &mcp9982_regmap_config);
+ if (IS_ERR(priv->regmap))
+ return dev_err_probe(dev, PTR_ERR(priv->regmap),
+ "Cannot initialize register map\n");
+
+ ret = devm_mutex_init(dev, &priv->lock);
+ if (ret)
+ return ret;
+
+ chip = i2c_get_match_data(client);
+ if (!chip)
+ return -EINVAL;
+ priv->chip = chip;
+ priv->bit_flags = 0;
+
+ ret = mcp9982_parse_fw_config(indio_dev, dev, chip->phys_channels);
+ if (ret)
+ return ret;
+
+ ret = mcp9982_init(dev, priv);
+ if (ret)
+ return ret;
+
+ indio_dev->name = chip->name;
+ indio_dev->info = &mcp9982_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->channels = priv->iio_chan;
+ indio_dev->num_channels = priv->num_channels;
+
+ ret = devm_iio_device_register(dev, indio_dev);
+ if (ret)
+ return dev_err_probe(dev, ret, "Cannot register IIO device\n");
+
+ return 0;
+}
+
+static const struct i2c_device_id mcp9982_id[] = {
+ { .name = "mcp9933", .driver_data = (kernel_ulong_t)&mcp9933_chip_config },
+ { .name = "mcp9933d", .driver_data = (kernel_ulong_t)&mcp9933d_chip_config },
+ { .name = "mcp9982", .driver_data = (kernel_ulong_t)&mcp9982_chip_config },
+ { .name = "mcp9982d", .driver_data = (kernel_ulong_t)&mcp9982d_chip_config },
+ { .name = "mcp9983", .driver_data = (kernel_ulong_t)&mcp9983_chip_config },
+ { .name = "mcp9983d", .driver_data = (kernel_ulong_t)&mcp9983d_chip_config },
+ { .name = "mcp9984", .driver_data = (kernel_ulong_t)&mcp9984_chip_config },
+ { .name = "mcp9984d", .driver_data = (kernel_ulong_t)&mcp9984d_chip_config },
+ { .name = "mcp9985", .driver_data = (kernel_ulong_t)&mcp9985_chip_config },
+ { .name = "mcp9985d", .driver_data = (kernel_ulong_t)&mcp9985d_chip_config },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, mcp9982_id);
+
+static const struct of_device_id mcp9982_of_match[] = {
+ {
+ .compatible = "microchip,mcp9933",
+ .data = &mcp9933_chip_config
+ }, {
+ .compatible = "microchip,mcp9933d",
+ .data = &mcp9933d_chip_config
+ }, {
+ .compatible = "microchip,mcp9982",
+ .data = &mcp9982_chip_config
+ }, {
+ .compatible = "microchip,mcp9982d",
+ .data = &mcp9982d_chip_config
+ }, {
+ .compatible = "microchip,mcp9983",
+ .data = &mcp9983_chip_config
+ }, {
+ .compatible = "microchip,mcp9983d",
+ .data = &mcp9983d_chip_config
+ }, {
+ .compatible = "microchip,mcp9984",
+ .data = &mcp9984_chip_config
+ }, {
+ .compatible = "microchip,mcp9984d",
+ .data = &mcp9984d_chip_config
+ }, {
+ .compatible = "microchip,mcp9985",
+ .data = &mcp9985_chip_config
+ }, {
+ .compatible = "microchip,mcp9985d",
+ .data = &mcp9985d_chip_config
+ },
+ { }
+};
+MODULE_DEVICE_TABLE(of, mcp9982_of_match);
+
+static struct i2c_driver mcp9982_driver = {
+ .driver = {
+ .name = "mcp9982",
+ .of_match_table = mcp9982_of_match,
+ },
+ .probe = mcp9982_probe,
+ .id_table = mcp9982_id,
+};
+module_i2c_driver(mcp9982_driver);
+
+MODULE_AUTHOR("Victor Duicu <victor.duicu@microchip.com>");
+MODULE_DESCRIPTION("MCP998X/33 and MCP998XD/33D Multichannel Automotive Temperature Monitor Driver");
+MODULE_LICENSE("GPL");
--
2.48.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v5 1/2] dt-bindings: iio: temperature: add support for MCP998X
2025-09-18 11:19 ` [PATCH v5 1/2] dt-bindings: iio: temperature: " victor.duicu
@ 2025-09-18 14:56 ` Conor Dooley
0 siblings, 0 replies; 11+ messages in thread
From: Conor Dooley @ 2025-09-18 14:56 UTC (permalink / raw)
To: victor.duicu
Cc: jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt,
linux-iio, linux-kernel, devicetree, marius.cristea
[-- Attachment #1: Type: text/plain, Size: 341 bytes --]
On Thu, Sep 18, 2025 at 02:19:36PM +0300, victor.duicu@microchip.com wrote:
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + pattern: '^.+d$'
ngl, I think this is being too clever for it's own good. Just list em
out please. Otherwise
Acked-by: Conor Dooley <conor.dooley@microchip.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 2/2] iio: temperature: add support for MCP998X
2025-09-18 11:19 ` [PATCH v5 2/2] " victor.duicu
@ 2025-09-19 6:27 ` kernel test robot
2025-09-20 10:39 ` Jonathan Cameron
2025-09-19 6:59 ` kernel test robot
2025-09-20 10:55 ` Jonathan Cameron
2 siblings, 1 reply; 11+ messages in thread
From: kernel test robot @ 2025-09-19 6:27 UTC (permalink / raw)
To: victor.duicu, jic23, dlechner, nuno.sa, andy, robh, krzk+dt,
conor+dt
Cc: llvm, oe-kbuild-all, linux-iio, linux-kernel, devicetree,
marius.cristea, victor.duicu
Hi,
kernel test robot noticed the following build errors:
[auto build test ERROR on 671b9b6d7f4fe17a174c410397e72253877ca64e]
url: https://github.com/intel-lab-lkp/linux/commits/victor-duicu-microchip-com/dt-bindings-iio-temperature-add-support-for-MCP998X/20250918-192457
base: 671b9b6d7f4fe17a174c410397e72253877ca64e
patch link: https://lore.kernel.org/r/20250918111937.5150-3-victor.duicu%40microchip.com
patch subject: [PATCH v5 2/2] iio: temperature: add support for MCP998X
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20250919/202509191423.1OvJW2X1-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250919/202509191423.1OvJW2X1-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509191423.1OvJW2X1-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/iio/temperature/mcp9982.c:470:3: error: expected expression
470 | u8 bulk_read[3];
| ^
>> drivers/iio/temperature/mcp9982.c:473:31: error: use of undeclared identifier 'bulk_read'; did you mean 'up_read'?
473 | &bulk_read, sizeof(bulk_read));
| ^~~~~~~~~
| up_read
include/linux/rwsem.h:246:13: note: 'up_read' declared here
246 | extern void up_read(struct rw_semaphore *sem);
| ^
drivers/iio/temperature/mcp9982.c:473:13: error: use of undeclared identifier 'bulk_read'; did you mean 'up_read'?
473 | &bulk_read, sizeof(bulk_read));
| ^~~~~~~~~
| up_read
include/linux/rwsem.h:246:13: note: 'up_read' declared here
246 | extern void up_read(struct rw_semaphore *sem);
| ^
drivers/iio/temperature/mcp9982.c:477:11: error: use of undeclared identifier 'bulk_read'; did you mean 'up_read'?
477 | *val = (bulk_read[1] << 8) + (bulk_read[2]);
| ^~~~~~~~~
| up_read
include/linux/rwsem.h:246:13: note: 'up_read' declared here
246 | extern void up_read(struct rw_semaphore *sem);
| ^
>> drivers/iio/temperature/mcp9982.c:477:11: error: subscript of pointer to function type 'void (struct rw_semaphore *)'
477 | *val = (bulk_read[1] << 8) + (bulk_read[2]);
| ^~~~~~~~~
drivers/iio/temperature/mcp9982.c:477:33: error: use of undeclared identifier 'bulk_read'; did you mean 'up_read'?
477 | *val = (bulk_read[1] << 8) + (bulk_read[2]);
| ^~~~~~~~~
| up_read
include/linux/rwsem.h:246:13: note: 'up_read' declared here
246 | extern void up_read(struct rw_semaphore *sem);
| ^
drivers/iio/temperature/mcp9982.c:477:33: error: subscript of pointer to function type 'void (struct rw_semaphore *)'
477 | *val = (bulk_read[1] << 8) + (bulk_read[2]);
| ^~~~~~~~~
drivers/iio/temperature/mcp9982.c:488:3: error: expected expression
488 | unsigned long *src;
| ^
>> drivers/iio/temperature/mcp9982.c:493:4: error: use of undeclared identifier 'src'
493 | *src = tmp_reg;
| ^
drivers/iio/temperature/mcp9982.c:494:68: error: use of undeclared identifier 'src'
494 | *val = mcp9982_3db_values_map_tbl[priv->sampl_idx][bitmap_weight(src, 2)][0];
| ^
drivers/iio/temperature/mcp9982.c:495:69: error: use of undeclared identifier 'src'
495 | *val2 = mcp9982_3db_values_map_tbl[priv->sampl_idx][bitmap_weight(src, 2)][1];
| ^
11 errors generated.
vim +470 drivers/iio/temperature/mcp9982.c
426
427 static int mcp9982_read_raw(struct iio_dev *indio_dev,
428 struct iio_chan_spec const *chan, int *val,
429 int *val2, long mask)
430 {
431 unsigned int tmp_reg, reg_status;
432 struct mcp9982_priv *priv = iio_priv(indio_dev);
433 int ret;
434
435 if (test_bit(RUN_STATE, &priv->bit_flags)) {
436 /*
437 * When working in Run mode, after modifying a parameter (like sampling
438 * frequency) we have to wait a delay before reading the new values.
439 * We can't determine when the conversion is done based on the BUSY bit.
440 */
441 if (test_bit(WAIT_BEFORE_READ, &priv->bit_flags)) {
442 if (!time_after(jiffies, priv->time_limit))
443 mdelay(jiffies_to_msecs(priv->time_limit - jiffies));
444 clear_bit(WAIT_BEFORE_READ, &priv->bit_flags);
445 }
446 } else {
447 ret = regmap_write(priv->regmap, MCP9982_ONE_SHOT_ADDR, 1);
448 if (ret)
449 return ret;
450 /*
451 * In Standby state after writing in OneShot register wait for
452 * the start of conversion and then poll the BUSY bit.
453 */
454 mdelay(125);
455 ret = regmap_read_poll_timeout(priv->regmap, MCP9982_STATUS_ADDR,
456 reg_status, !(reg_status & MCP9982_STATUS_BUSY),
457 mcp9982_delay_ms[priv->sampl_idx] * USEC_PER_MSEC,
458 0);
459 if (ret)
460 return ret;
461 }
462 guard(mutex)(&priv->lock);
463
464 switch (mask) {
465 case IIO_CHAN_INFO_RAW:
466 /*
467 * The Block Read Protocol first returns the number of user readable
468 * bytes, held in bulk_read[0], followed by the data.
469 */
> 470 u8 bulk_read[3];
471
472 ret = regmap_bulk_read(priv->regmap, MCP9982_TEMP_MEM_BLOCK_ADDR(chan->channel),
> 473 &bulk_read, sizeof(bulk_read));
474 if (ret)
475 return ret;
476
> 477 *val = (bulk_read[1] << 8) + (bulk_read[2]);
478 return IIO_VAL_INT;
479 case IIO_CHAN_INFO_SCALE:
480 *val = 0;
481 *val2 = MCP9982_SCALE;
482 return IIO_VAL_INT_PLUS_NANO;
483 case IIO_CHAN_INFO_SAMP_FREQ:
484 *val = mcp9982_conv_rate[priv->sampl_idx][0];
485 *val2 = mcp9982_conv_rate[priv->sampl_idx][1];
486 return IIO_VAL_INT_PLUS_MICRO;
487 case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
488 unsigned long *src;
489
490 ret = regmap_read(priv->regmap, MCP9982_RUNNING_AVG_ADDR, &tmp_reg);
491 if (ret)
492 return ret;
> 493 *src = tmp_reg;
494 *val = mcp9982_3db_values_map_tbl[priv->sampl_idx][bitmap_weight(src, 2)][0];
495 *val2 = mcp9982_3db_values_map_tbl[priv->sampl_idx][bitmap_weight(src, 2)][1];
496 return IIO_VAL_INT_PLUS_MICRO;
497 case IIO_CHAN_INFO_OFFSET:
498 *val = MCP9982_OFFSET;
499 return IIO_VAL_INT;
500 default:
501 return -EINVAL;
502 }
503 }
504
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 2/2] iio: temperature: add support for MCP998X
2025-09-18 11:19 ` [PATCH v5 2/2] " victor.duicu
2025-09-19 6:27 ` kernel test robot
@ 2025-09-19 6:59 ` kernel test robot
2025-09-20 10:55 ` Jonathan Cameron
2 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2025-09-19 6:59 UTC (permalink / raw)
To: victor.duicu, jic23, dlechner, nuno.sa, andy, robh, krzk+dt,
conor+dt
Cc: oe-kbuild-all, linux-iio, linux-kernel, devicetree,
marius.cristea, victor.duicu
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 671b9b6d7f4fe17a174c410397e72253877ca64e]
url: https://github.com/intel-lab-lkp/linux/commits/victor-duicu-microchip-com/dt-bindings-iio-temperature-add-support-for-MCP998X/20250918-192457
base: 671b9b6d7f4fe17a174c410397e72253877ca64e
patch link: https://lore.kernel.org/r/20250918111937.5150-3-victor.duicu%40microchip.com
patch subject: [PATCH v5 2/2] iio: temperature: add support for MCP998X
config: loongarch-allyesconfig (https://download.01.org/0day-ci/archive/20250919/202509191411.ej3JBK3f-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 7c861bcedf61607b6c087380ac711eb7ff918ca6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250919/202509191411.ej3JBK3f-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509191411.ej3JBK3f-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from <built-in>:3:
In file included from include/linux/compiler_types.h:171:
include/linux/compiler-clang.h:28:9: warning: '__SANITIZE_ADDRESS__' macro redefined [-Wmacro-redefined]
28 | #define __SANITIZE_ADDRESS__
| ^
<built-in>:371:9: note: previous definition is here
371 | #define __SANITIZE_ADDRESS__ 1
| ^
>> drivers/iio/temperature/mcp9982.c:470:3: warning: label followed by a declaration is a C23 extension [-Wc23-extensions]
470 | u8 bulk_read[3];
| ^
drivers/iio/temperature/mcp9982.c:488:3: warning: label followed by a declaration is a C23 extension [-Wc23-extensions]
488 | unsigned long *src;
| ^
>> drivers/iio/temperature/mcp9982.c:493:4: warning: variable 'src' is uninitialized when used here [-Wuninitialized]
493 | *src = tmp_reg;
| ^~~
drivers/iio/temperature/mcp9982.c:488:21: note: initialize the variable 'src' to silence this warning
488 | unsigned long *src;
| ^
| = NULL
drivers/iio/temperature/mcp9982.c:741:2: warning: variable 'iio_idx' is uninitialized when used here [-Wuninitialized]
741 | iio_idx++;
| ^~~~~~~
drivers/iio/temperature/mcp9982.c:720:30: note: initialize the variable 'iio_idx' to silence this warning
720 | unsigned int reg_nr, iio_idx;
| ^
| = 0
5 warnings generated.
vim +470 drivers/iio/temperature/mcp9982.c
426
427 static int mcp9982_read_raw(struct iio_dev *indio_dev,
428 struct iio_chan_spec const *chan, int *val,
429 int *val2, long mask)
430 {
431 unsigned int tmp_reg, reg_status;
432 struct mcp9982_priv *priv = iio_priv(indio_dev);
433 int ret;
434
435 if (test_bit(RUN_STATE, &priv->bit_flags)) {
436 /*
437 * When working in Run mode, after modifying a parameter (like sampling
438 * frequency) we have to wait a delay before reading the new values.
439 * We can't determine when the conversion is done based on the BUSY bit.
440 */
441 if (test_bit(WAIT_BEFORE_READ, &priv->bit_flags)) {
442 if (!time_after(jiffies, priv->time_limit))
443 mdelay(jiffies_to_msecs(priv->time_limit - jiffies));
444 clear_bit(WAIT_BEFORE_READ, &priv->bit_flags);
445 }
446 } else {
447 ret = regmap_write(priv->regmap, MCP9982_ONE_SHOT_ADDR, 1);
448 if (ret)
449 return ret;
450 /*
451 * In Standby state after writing in OneShot register wait for
452 * the start of conversion and then poll the BUSY bit.
453 */
454 mdelay(125);
455 ret = regmap_read_poll_timeout(priv->regmap, MCP9982_STATUS_ADDR,
456 reg_status, !(reg_status & MCP9982_STATUS_BUSY),
457 mcp9982_delay_ms[priv->sampl_idx] * USEC_PER_MSEC,
458 0);
459 if (ret)
460 return ret;
461 }
462 guard(mutex)(&priv->lock);
463
464 switch (mask) {
465 case IIO_CHAN_INFO_RAW:
466 /*
467 * The Block Read Protocol first returns the number of user readable
468 * bytes, held in bulk_read[0], followed by the data.
469 */
> 470 u8 bulk_read[3];
471
472 ret = regmap_bulk_read(priv->regmap, MCP9982_TEMP_MEM_BLOCK_ADDR(chan->channel),
473 &bulk_read, sizeof(bulk_read));
474 if (ret)
475 return ret;
476
477 *val = (bulk_read[1] << 8) + (bulk_read[2]);
478 return IIO_VAL_INT;
479 case IIO_CHAN_INFO_SCALE:
480 *val = 0;
481 *val2 = MCP9982_SCALE;
482 return IIO_VAL_INT_PLUS_NANO;
483 case IIO_CHAN_INFO_SAMP_FREQ:
484 *val = mcp9982_conv_rate[priv->sampl_idx][0];
485 *val2 = mcp9982_conv_rate[priv->sampl_idx][1];
486 return IIO_VAL_INT_PLUS_MICRO;
487 case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
488 unsigned long *src;
489
490 ret = regmap_read(priv->regmap, MCP9982_RUNNING_AVG_ADDR, &tmp_reg);
491 if (ret)
492 return ret;
> 493 *src = tmp_reg;
494 *val = mcp9982_3db_values_map_tbl[priv->sampl_idx][bitmap_weight(src, 2)][0];
495 *val2 = mcp9982_3db_values_map_tbl[priv->sampl_idx][bitmap_weight(src, 2)][1];
496 return IIO_VAL_INT_PLUS_MICRO;
497 case IIO_CHAN_INFO_OFFSET:
498 *val = MCP9982_OFFSET;
499 return IIO_VAL_INT;
500 default:
501 return -EINVAL;
502 }
503 }
504
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 2/2] iio: temperature: add support for MCP998X
2025-09-19 6:27 ` kernel test robot
@ 2025-09-20 10:39 ` Jonathan Cameron
0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2025-09-20 10:39 UTC (permalink / raw)
To: kernel test robot
Cc: victor.duicu, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt,
llvm, oe-kbuild-all, linux-iio, linux-kernel, devicetree,
marius.cristea
On Fri, 19 Sep 2025 14:27:55 +0800
kernel test robot <lkp@intel.com> wrote:
> Hi,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on 671b9b6d7f4fe17a174c410397e72253877ca64e]
>
> url: https://github.com/intel-lab-lkp/linux/commits/victor-duicu-microchip-com/dt-bindings-iio-temperature-add-support-for-MCP998X/20250918-192457
> base: 671b9b6d7f4fe17a174c410397e72253877ca64e
> patch link: https://lore.kernel.org/r/20250918111937.5150-3-victor.duicu%40microchip.com
> patch subject: [PATCH v5 2/2] iio: temperature: add support for MCP998X
> config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20250919/202509191423.1OvJW2X1-lkp@intel.com/config)
> compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250919/202509191423.1OvJW2X1-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202509191423.1OvJW2X1-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
> >> drivers/iio/temperature/mcp9982.c:470:3: error: expected expression
> 470 | u8 bulk_read[3];
I guess you may have figured this out by now. It's a scope issue. Needs {}
around these blocks of code.
> | ^
> >> drivers/iio/temperature/mcp9982.c:473:31: error: use of undeclared identifier 'bulk_read'; did you mean 'up_read'?
> 473 | &bulk_read, sizeof(bulk_read));
> | ^~~~~~~~~
> | up_read
> include/linux/rwsem.h:246:13: note: 'up_read' declared here
> 246 | extern void up_read(struct rw_semaphore *sem);
> | ^
> drivers/iio/temperature/mcp9982.c:473:13: error: use of undeclared identifier 'bulk_read'; did you mean 'up_read'?
> 473 | &bulk_read, sizeof(bulk_read));
> | ^~~~~~~~~
> | up_read
> include/linux/rwsem.h:246:13: note: 'up_read' declared here
> 246 | extern void up_read(struct rw_semaphore *sem);
> | ^
> drivers/iio/temperature/mcp9982.c:477:11: error: use of undeclared identifier 'bulk_read'; did you mean 'up_read'?
> 477 | *val = (bulk_read[1] << 8) + (bulk_read[2]);
> | ^~~~~~~~~
> | up_read
> include/linux/rwsem.h:246:13: note: 'up_read' declared here
> 246 | extern void up_read(struct rw_semaphore *sem);
> | ^
> >> drivers/iio/temperature/mcp9982.c:477:11: error: subscript of pointer to function type 'void (struct rw_semaphore *)'
> 477 | *val = (bulk_read[1] << 8) + (bulk_read[2]);
> | ^~~~~~~~~
> drivers/iio/temperature/mcp9982.c:477:33: error: use of undeclared identifier 'bulk_read'; did you mean 'up_read'?
> 477 | *val = (bulk_read[1] << 8) + (bulk_read[2]);
> | ^~~~~~~~~
> | up_read
> include/linux/rwsem.h:246:13: note: 'up_read' declared here
> 246 | extern void up_read(struct rw_semaphore *sem);
> | ^
> drivers/iio/temperature/mcp9982.c:477:33: error: subscript of pointer to function type 'void (struct rw_semaphore *)'
> 477 | *val = (bulk_read[1] << 8) + (bulk_read[2]);
> | ^~~~~~~~~
> drivers/iio/temperature/mcp9982.c:488:3: error: expected expression
> 488 | unsigned long *src;
> | ^
> >> drivers/iio/temperature/mcp9982.c:493:4: error: use of undeclared identifier 'src'
> 493 | *src = tmp_reg;
> | ^
> drivers/iio/temperature/mcp9982.c:494:68: error: use of undeclared identifier 'src'
> 494 | *val = mcp9982_3db_values_map_tbl[priv->sampl_idx][bitmap_weight(src, 2)][0];
> | ^
> drivers/iio/temperature/mcp9982.c:495:69: error: use of undeclared identifier 'src'
> 495 | *val2 = mcp9982_3db_values_map_tbl[priv->sampl_idx][bitmap_weight(src, 2)][1];
> | ^
> 11 errors generated.
>
>
> vim +470 drivers/iio/temperature/mcp9982.c
>
> 426
> 427 static int mcp9982_read_raw(struct iio_dev *indio_dev,
> 428 struct iio_chan_spec const *chan, int *val,
> 429 int *val2, long mask)
> 430 {
> 431 unsigned int tmp_reg, reg_status;
> 432 struct mcp9982_priv *priv = iio_priv(indio_dev);
> 433 int ret;
> 434
> 435 if (test_bit(RUN_STATE, &priv->bit_flags)) {
> 436 /*
> 437 * When working in Run mode, after modifying a parameter (like sampling
> 438 * frequency) we have to wait a delay before reading the new values.
> 439 * We can't determine when the conversion is done based on the BUSY bit.
> 440 */
> 441 if (test_bit(WAIT_BEFORE_READ, &priv->bit_flags)) {
> 442 if (!time_after(jiffies, priv->time_limit))
> 443 mdelay(jiffies_to_msecs(priv->time_limit - jiffies));
> 444 clear_bit(WAIT_BEFORE_READ, &priv->bit_flags);
> 445 }
> 446 } else {
> 447 ret = regmap_write(priv->regmap, MCP9982_ONE_SHOT_ADDR, 1);
> 448 if (ret)
> 449 return ret;
> 450 /*
> 451 * In Standby state after writing in OneShot register wait for
> 452 * the start of conversion and then poll the BUSY bit.
> 453 */
> 454 mdelay(125);
> 455 ret = regmap_read_poll_timeout(priv->regmap, MCP9982_STATUS_ADDR,
> 456 reg_status, !(reg_status & MCP9982_STATUS_BUSY),
> 457 mcp9982_delay_ms[priv->sampl_idx] * USEC_PER_MSEC,
> 458 0);
> 459 if (ret)
> 460 return ret;
> 461 }
> 462 guard(mutex)(&priv->lock);
> 463
> 464 switch (mask) {
> 465 case IIO_CHAN_INFO_RAW:
> 466 /*
> 467 * The Block Read Protocol first returns the number of user readable
> 468 * bytes, held in bulk_read[0], followed by the data.
> 469 */
> > 470 u8 bulk_read[3];
> 471
> 472 ret = regmap_bulk_read(priv->regmap, MCP9982_TEMP_MEM_BLOCK_ADDR(chan->channel),
> > 473 &bulk_read, sizeof(bulk_read));
> 474 if (ret)
> 475 return ret;
> 476
> > 477 *val = (bulk_read[1] << 8) + (bulk_read[2]);
> 478 return IIO_VAL_INT;
> 479 case IIO_CHAN_INFO_SCALE:
> 480 *val = 0;
> 481 *val2 = MCP9982_SCALE;
> 482 return IIO_VAL_INT_PLUS_NANO;
> 483 case IIO_CHAN_INFO_SAMP_FREQ:
> 484 *val = mcp9982_conv_rate[priv->sampl_idx][0];
> 485 *val2 = mcp9982_conv_rate[priv->sampl_idx][1];
> 486 return IIO_VAL_INT_PLUS_MICRO;
> 487 case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> 488 unsigned long *src;
> 489
> 490 ret = regmap_read(priv->regmap, MCP9982_RUNNING_AVG_ADDR, &tmp_reg);
> 491 if (ret)
> 492 return ret;
> > 493 *src = tmp_reg;
> 494 *val = mcp9982_3db_values_map_tbl[priv->sampl_idx][bitmap_weight(src, 2)][0];
> 495 *val2 = mcp9982_3db_values_map_tbl[priv->sampl_idx][bitmap_weight(src, 2)][1];
> 496 return IIO_VAL_INT_PLUS_MICRO;
> 497 case IIO_CHAN_INFO_OFFSET:
> 498 *val = MCP9982_OFFSET;
> 499 return IIO_VAL_INT;
> 500 default:
> 501 return -EINVAL;
> 502 }
> 503 }
> 504
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 2/2] iio: temperature: add support for MCP998X
2025-09-18 11:19 ` [PATCH v5 2/2] " victor.duicu
2025-09-19 6:27 ` kernel test robot
2025-09-19 6:59 ` kernel test robot
@ 2025-09-20 10:55 ` Jonathan Cameron
2025-09-24 12:47 ` Victor.Duicu
2 siblings, 1 reply; 11+ messages in thread
From: Jonathan Cameron @ 2025-09-20 10:55 UTC (permalink / raw)
To: victor.duicu
Cc: dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-iio,
linux-kernel, devicetree, marius.cristea
On Thu, 18 Sep 2025 14:19:37 +0300
<victor.duicu@microchip.com> wrote:
> From: Victor Duicu <victor.duicu@microchip.com>
>
> This is the driver for Microchip MCP998X/33 and MCP998XD/33D Multichannel
> Automotive Temperature Monitor Family.
>
> Signed-off-by: Victor Duicu <victor.duicu@microchip.com>
Hi Victor,
Various minor comments inline.
Given the build warnings I didn't elect to just tidy these up whilst applying.
Seemed like there was slightly too high a risk of me messing it up!
Also we have lots of time as IIO is closed for this cycle now.
Jonathan
> diff --git a/drivers/iio/temperature/mcp9982.c b/drivers/iio/temperature/mcp9982.c
> new file mode 100644
> index 000000000000..05130b72ba14
> --- /dev/null
> +++ b/drivers/iio/temperature/mcp9982.c
> @@ -0,0 +1,871 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * IIO driver for MCP998X/33 and MCP998XD/33D Multichannel Automotive Temperature Monitor Family
> + *
> + * Copyright (C) 2025 Microchip Technology Inc. and its subsidiaries
> + *
> + * Author: Victor Duicu <victor.duicu@microchip.com>
> + *
> + * Datasheet can be found here:
> + * https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP998X-Family-Data-Sheet-DS20006827.pdf
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/bits.h>
> +#include <linux/delay.h>
> +#include <linux/device/devres.h>
> +#include <linux/dev_printk.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/iio/iio.h>
> +#include <linux/math64.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/string.h>
> +#include <linux/units.h>
> +
> +/* MCP9982 Registers */
> +#define MCP9982_HIGH_BYTE_ADDR(index) (2 * (index))
> +#define MCP9982_ONE_SHOT_ADDR 0x0A
> +#define MCP9982_INTERNAL_HIGH_LIMIT_ADDR 0x0B
> +#define MCP9982_INTERNAL_LOW_LIMIT_ADDR 0x0C
> +#define MCP9982_EXT1_HIGH_LIMIT_HIGH_BYTE_ADDR 0x0D
> +#define MCP9982_EXT1_HIGH_LIMIT_LOW_BYTE_ADDR 0x0E
> +#define MCP9982_EXT1_LOW_LIMIT_HIGH_BYTE_ADDR 0x0F
> +#define MCP9982_EXT1_LOW_LIMIT_LOW_BYTE_ADDR 0x10
> +#define MCP9982_INTERNAL_THERM_LIMIT_ADDR 0x1D
> +#define MCP9982_EXT1_THERM_LIMIT_ADDR 0x1E
> +#define MCP9982_CFG_ADDR 0x22
> +#define MCP9982_CONV_ADDR 0x24
> +#define MCP9982_HYS_ADDR 0x25
> +#define MCP9982_CONSEC_ALRT_ADDR 0x26
> +#define MCP9982_ALRT_CFG_ADDR 0x27
> +#define MCP9982_RUNNING_AVG_ADDR 0x28
> +#define MCP9982_HOTTEST_CFG_ADDR 0x29
> +#define MCP9982_STATUS_ADDR 0x2A
> +#define MCP9982_EXT_FAULT_STATUS_ADDR 0x2B
> +#define MCP9982_HIGH_LIMIT_STATUS_ADDR 0x2C
> +#define MCP9982_LOW_LIMIT_STATUS_ADDR 0x2D
> +#define MCP9982_THERM_LIMIT_STATUS_ADDR 0x2E
> +#define MCP9982_HOTTEST_HIGH_BYTE_ADDR 0x2F
> +#define MCP9982_HOTTEST_LOW_BYTE_ADDR 0x30
> +#define MCP9982_HOTTEST_STATUS_ADDR 0x31
> +#define MCP9982_THERM_SHTDWN_CFG_ADDR 0x32
> +#define MCP9982_HRDW_THERM_SHTDWN_LIMIT_ADDR 0x33
> +/* 52 is the start address in decimal for the beta registers. */
> +#define MCP9982_EXT_BETA_CFG_ADDR(index) ((index) + 52)
Why not use a hex value in common with the other ADDR defines?
> +/**
> + * Bit flags and their meaning
As below. I don't think it is worth encoding these in a bitmap. Just use
5 bools to represent the state.
> + * @RECD34_ENABLE: state of Resistance Error Correction(REC) on channels 3 and 4
> + * @RECD12_ENABLE: state of Resistance Error Correction(REC) on channels 1 and 2
> + * @APDD_ENABLE: state of anti-parallel diode mode
> + * @RUN_STATE: chip is in run state, otherwise is in standby state
> + * @WAIT_BEFORE_READ: whether we need to wait a delay before reading a new value
> + */
> +#define RECD34_ENABLE 0
> +#define RECD12_ENABLE 1
> +#define APDD_ENABLE 2
> +#define RUN_STATE 3
> +#define WAIT_BEFORE_READ 4
> +#define USE_PREVIOUS_FREQ 5
> +
> +#define MCP9982_CHAN(index, si, __address) ( \
Why the outer set of ()?
> + (struct iio_chan_spec) { \
> + .type = IIO_TEMP, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
> + BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY), \
For readability push that BIT( one tab further over.
Editors (mine for starters) tend format these things badly so this needs some manual
tweaking but is definitely more readable.
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
> + BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY) | \
> + BIT(IIO_CHAN_INFO_OFFSET) | \
> + BIT(IIO_CHAN_INFO_SCALE), \
> + .channel = index, \
> + .address = __address, \
Wrap macro parameters in () to avoid side effects. Sure we are unlikely to get complex stuff
as parameters to this macro, but best to always avoid any possibility of the unexpected.
> + .scan_index = si, \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = 8, \
> + .storagebits = 8, \
> + }, \
> + .indexed = 1, \
> + } \
> +)
> +/*
> + * Constants were calculated using:
> + * (Sampling_Frequency(Hz) * 1000000) / (Window_Size * 2)
> + * The formula is used for Window_Size = {4, 8}.
> + * For Window_Size = 1 the filter is OFF and the 3db value
> + * is equal to the frequency.
> + */
> +static const unsigned int mcp9982_3db_values_map_tbl[11][3][2] = {
> + {
> + {0, 62500},
> + {0, 7812},
> + {0, 3906},
> + },
> + {
> + {0, 125000},
> + {0, 15625},
> + {0, 7812},
> + },
> + {
> + {0, 250000},
> + {0, 31250},
> + {0, 15625},
> + },
> + {
> + {0, 500000},
> + {0, 62500},
> + {0, 31250},
> + },
> + {
> + {1, 0},
> + {0, 125000},
> + {0, 62500},
> + },
> + {
> + {2, 0},
> + {0, 250000},
> + {0, 125000},
> + },
> + {
> + {4, 0},
> + {0, 500000},
> + {0, 250000},
> + },
> + {
> + {8, 0},
> + {1, 0},
> + {0, 500000},
> + },
> + {
> + {16, 0},
> + {2, 0},
> + {1, 0},
> + },
> + {
> + {32, 0},
> + {4, 0},
> + {2, 0},
> + },
> + {
> + {64, 0},
> + {8, 0},
> + {4, 0},
My preferred style for IIO (we are slowly moving over to it in old code) is
{ 4, 0 },
So spaces after { and before }
Currently this patch is inconsistent on this so definitely move to that style.
> + },
> +};
> +
> +/**
> + * struct mcp9992_priv - information about chip parameters
> + * @bit_flags: holds the state of the flags
> + * @regmap: device register map
> + * @chip: pointer to structure holding chip features
> + * @lock: synchronize access to driver's state members
> + * @iio_chan: specifications of channels
> + * @labels: labels of the channels
> + * @sampl_idx: index representing the current sampling frequency
> + * @time_limit: time when it is safe to read
> + * @num_channels: number of active physical channels
> + */
> +struct mcp9982_priv {
> + unsigned long bit_flags;
This isn't a structure where we much care about minimizing size, so I'd
just use a bunch of bools for the different flags. Will give more readable
code. Probably push them to end of structure though rather than having them
first. Most common access is probably to the regmap so it is best
to have that as first element.
> + struct regmap *regmap;
> + const struct mcp9982_features *chip;
> + /* Synchronize access to driver's state members. */
> + struct mutex lock;
> + struct iio_chan_spec iio_chan[5];
> + const char *labels[MCP9982_MAX_NUM_CHANNELS];
> + unsigned int sampl_idx;
> + unsigned long time_limit;
> + u8 num_channels;
> +
> +};
> +
> +static int mcp9982_read_avail(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, const int **vals,
> + int *type, int *length, long mask)
> +{
> + struct mcp9982_priv *priv = iio_priv(indio_dev);
> + unsigned int idx;
> + unsigned int sub;
> +
> + if (priv->chip->hw_thermal_shutdown) {
> + idx = 4;
> + sub = 8;
> + } else {
> + idx = 0;
> + sub = 0;
> + }
> + switch (mask) {
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + *type = IIO_VAL_INT_PLUS_MICRO;
> + *vals = mcp9982_conv_rate[idx];
> + *length = ARRAY_SIZE(mcp9982_conv_rate) * 2 - sub;
> + return IIO_AVAIL_LIST;
> + case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> + *type = IIO_VAL_INT_PLUS_MICRO;
> + *vals = mcp9982_3db_values_map_tbl[priv->sampl_idx][0];
> + *length = ARRAY_SIZE(mcp9982_3db_values_map_tbl[priv->sampl_idx]) * 2;
> + return IIO_AVAIL_LIST;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int mcp9982_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int *val,
> + int *val2, long mask)
> +{
> + unsigned int tmp_reg, reg_status;
> + struct mcp9982_priv *priv = iio_priv(indio_dev);
> + int ret;
> +
> + if (test_bit(RUN_STATE, &priv->bit_flags)) {
> + /*
> + * When working in Run mode, after modifying a parameter (like sampling
> + * frequency) we have to wait a delay before reading the new values.
> + * We can't determine when the conversion is done based on the BUSY bit.
> + */
> + if (test_bit(WAIT_BEFORE_READ, &priv->bit_flags)) {
> + if (!time_after(jiffies, priv->time_limit))
> + mdelay(jiffies_to_msecs(priv->time_limit - jiffies));
> + clear_bit(WAIT_BEFORE_READ, &priv->bit_flags);
> + }
> + } else {
> + ret = regmap_write(priv->regmap, MCP9982_ONE_SHOT_ADDR, 1);
> + if (ret)
> + return ret;
> + /*
> + * In Standby state after writing in OneShot register wait for
> + * the start of conversion and then poll the BUSY bit.
> + */
> + mdelay(125);
> + ret = regmap_read_poll_timeout(priv->regmap, MCP9982_STATUS_ADDR,
> + reg_status, !(reg_status & MCP9982_STATUS_BUSY),
> + mcp9982_delay_ms[priv->sampl_idx] * USEC_PER_MSEC,
> + 0);
> + if (ret)
> + return ret;
> + }
> + guard(mutex)(&priv->lock);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + /*
> + * The Block Read Protocol first returns the number of user readable
> + * bytes, held in bulk_read[0], followed by the data.
> + */
> + u8 bulk_read[3];
> +
> + ret = regmap_bulk_read(priv->regmap, MCP9982_TEMP_MEM_BLOCK_ADDR(chan->channel),
> + &bulk_read, sizeof(bulk_read));
> + if (ret)
> + return ret;
> +
> + *val = (bulk_read[1] << 8) + (bulk_read[2]);
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + *val = 0;
> + *val2 = MCP9982_SCALE;
> + return IIO_VAL_INT_PLUS_NANO;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + *val = mcp9982_conv_rate[priv->sampl_idx][0];
> + *val2 = mcp9982_conv_rate[priv->sampl_idx][1];
> + return IIO_VAL_INT_PLUS_MICRO;
> + case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> + unsigned long *src;
> +
> + ret = regmap_read(priv->regmap, MCP9982_RUNNING_AVG_ADDR, &tmp_reg);
> + if (ret)
> + return ret;
> + *src = tmp_reg;
> + *val = mcp9982_3db_values_map_tbl[priv->sampl_idx][bitmap_weight(src, 2)][0];
> + *val2 = mcp9982_3db_values_map_tbl[priv->sampl_idx][bitmap_weight(src, 2)][1];
> + return IIO_VAL_INT_PLUS_MICRO;
> + case IIO_CHAN_INFO_OFFSET:
> + *val = MCP9982_OFFSET;
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int mcp9982_read_label(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, char *label)
> +{
> + struct mcp9982_priv *priv = iio_priv(indio_dev);
> +
> + return sysfs_emit(label, "%s\n", priv->labels[chan->channel]);
> +}
> +
> +static int mcp9982_write_raw_get_fmt(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, long info)
> +{
> + switch (info) {
> + case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + return IIO_VAL_INT_PLUS_MICRO;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int mcp9982_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int val,
> + int val2, long mask)
> +{
> + unsigned int i, start, previous_sampl_idx;
> + struct mcp9982_priv *priv = iio_priv(indio_dev);
> + int ret;
> + unsigned long new_time_limit;
> +
> + start = 0;
> + guard(mutex)(&priv->lock);
> + switch (mask) {
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + previous_sampl_idx = priv->sampl_idx;
> +
> + /* For MCP998XD and MCP9933D sampling frequency can't be set lower than 1. */
> + if (priv->chip->hw_thermal_shutdown)
> + start = 4;
> + for (i = start; i < ARRAY_SIZE(mcp9982_conv_rate); i++)
> + if (val == mcp9982_conv_rate[i][0] &&
> + val2 == mcp9982_conv_rate[i][1])
> + break;
> +
> + if (i == ARRAY_SIZE(mcp9982_conv_rate))
> + return -EINVAL;
> +
> + ret = regmap_write(priv->regmap, MCP9982_CONV_ADDR, i);
> + if (ret)
> + return ret;
> +
> + priv->sampl_idx = i;
> +
> + /*
> + * When changing the frequency in Run mode, wait a delay based
> + * on the previous value to ensure the new value becomes active.
> + */
> + if (test_bit(RUN_STATE, &priv->bit_flags))
> + set_bit(USE_PREVIOUS_FREQ, &priv->bit_flags);
> +
> + break;
> + case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> + for (i = 0; i < ARRAY_SIZE(mcp9982_3db_values_map_tbl[priv->sampl_idx]); i++)
> + if (val == mcp9982_3db_values_map_tbl[priv->sampl_idx][i][0] &&
> + val2 == mcp9982_3db_values_map_tbl[priv->sampl_idx][i][1])
> + break;
> +
> + if (i == ARRAY_SIZE(mcp9982_3db_values_map_tbl[priv->sampl_idx]))
> + return -EINVAL;
> +
> + /*
> + * In mcp9982_3db_values_map_tbl the second index maps:
> + * 0 for filter off
> + * 1 for filter at level 1
> + * 2 for filter at level 2
> + */
> + if (i == 2)
> + i = 3;
> + /*
> + * If the digital filter is activated for chips without "D", set
> + * the power state to Run to ensure the averaging is made on fresh values.
> + */
> + if (!priv->chip->hw_thermal_shutdown) {
> + if (i == 0) {
> + ret = regmap_assign_bits(priv->regmap,
> + MCP9982_CFG_ADDR,
> + MCP9982_CFG_RS, 1);
> + clear_bit(RUN_STATE, &priv->bit_flags);
> + } else {
> + ret = regmap_assign_bits(priv->regmap,
> + MCP9982_CFG_ADDR,
> + MCP9982_CFG_RS, 0);
> + set_bit(RUN_STATE, &priv->bit_flags);
> + }
> + }
> +
> + ret = regmap_write(priv->regmap, MCP9982_RUNNING_AVG_ADDR, i);
> + if (ret)
> + return ret;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + if (test_bit(RUN_STATE, &priv->bit_flags)) {
> + if (test_bit(USE_PREVIOUS_FREQ, &priv->bit_flags)) {
> + new_time_limit = jiffies +
> + msecs_to_jiffies(mcp9982_delay_ms[previous_sampl_idx]);
> + clear_bit(USE_PREVIOUS_FREQ, &priv->bit_flags);
> + } else {
> + new_time_limit = jiffies +
> + msecs_to_jiffies(mcp9982_delay_ms[priv->sampl_idx]);
> + }
> + if (time_after(new_time_limit, priv->time_limit)) {
> + priv->time_limit = new_time_limit;
> + set_bit(WAIT_BEFORE_READ, &priv->bit_flags);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static const struct iio_info mcp9982_info = {
> + .read_raw = mcp9982_read_raw,
> + .read_label = mcp9982_read_label,
> + .read_avail = mcp9982_read_avail,
> + .write_raw_get_fmt = mcp9982_write_raw_get_fmt,
> + .write_raw = mcp9982_write_raw,
> +};
> +
> +static int mcp9982_init(struct device *dev, struct mcp9982_priv *priv)
> +{
> + int ret;
> + unsigned int i;
> + u8 val;
> +
> + /* Chips 82/83 and 82D/83D do not support anti-parallel diode mode. */
> + if (!priv->chip->allow_apdd && test_bit(APDD_ENABLE, &priv->bit_flags))
> + return dev_err_probe(dev, -EINVAL, "Incorrect setting of APDD.\n");
> +
> + /* Chips with "D" work in Run state and those without work in Standby state. */
> + if (priv->chip->hw_thermal_shutdown)
> + set_bit(RUN_STATE, &priv->bit_flags);
> +
> + /*
> + * For chips with "D" in the name resistance error correction must be on
> + * so that hardware shutdown feature can't be overridden.
> + */
> + if (priv->chip->hw_thermal_shutdown)
> + if (!test_bit(RECD34_ENABLE, &priv->bit_flags) ||
> + !test_bit(RECD12_ENABLE, &priv->bit_flags))
> + return dev_err_probe(dev, -EINVAL, "Incorrect setting of RECD.\n");
> + /*
> + * Set default values in registers.
> + *
> + * APDD, RECD12 and RECD34 are active on 0.
> + */
> + val = FIELD_PREP(MCP9982_CFG_MSKAL, 1) |
> + FIELD_PREP(MCP9982_CFG_RS, !test_bit(RUN_STATE, &priv->bit_flags)) |
> + FIELD_PREP(MCP9982_CFG_ATTHM, 1) |
> + FIELD_PREP(MCP9982_CFG_RECD12, !test_bit(RECD12_ENABLE, &priv->bit_flags)) |
> + FIELD_PREP(MCP9982_CFG_RECD34, !test_bit(RECD34_ENABLE, &priv->bit_flags)) |
> + FIELD_PREP(MCP9982_CFG_RANGE, 1) | FIELD_PREP(MCP9982_CFG_DA_ENA, 0) |
> + FIELD_PREP(MCP9982_CFG_APDD, !test_bit(APDD_ENABLE, &priv->bit_flags));
> +
> + ret = regmap_write(priv->regmap, MCP9982_CFG_ADDR, val);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(priv->regmap, MCP9982_CONV_ADDR, 6);
> + if (ret)
> + return ret;
> + priv->sampl_idx = 6;
> +
> + ret = regmap_write(priv->regmap, MCP9982_HYS_ADDR, 10);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(priv->regmap, MCP9982_CONSEC_ALRT_ADDR, 112);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(priv->regmap, MCP9982_RUNNING_AVG_ADDR, 0);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(priv->regmap, MCP9982_HOTTEST_CFG_ADDR, 0);
> + if (ret)
> + return ret;
> +
> + /*
> + * Only external channels 1 and 2 support beta compensation.
> + * Set beta auto-detection.
> + */
> + for (i = 0; i < 2; i++) {
> + ret = regmap_write(priv->regmap, MCP9982_EXT_BETA_CFG_ADDR(i),
> + MCP9982_BETA_AUTODETECT);
> + if (ret)
> + return ret;
> + }
> + /* Set ideality factor to default for all external channels. */
> + for (i = 0; i < 4; i++) {
> + ret = regmap_write(priv->regmap, MCP9982_EXT_IDEAL_ADDR(i),
> + MCP9982_IDEALITY_DEFAULT);
> + if (ret)
> + return ret;
> + }
> +
> + priv->time_limit = jiffies;
> +
> + return 0;
> +}
> +
> +static int mcp9982_parse_fw_config(struct iio_dev *indio_dev, struct device *dev,
> + int device_nr_channels)
> +{
> + unsigned int reg_nr, iio_idx;
The bot spotted this. Need to init iio_idx.
> + struct mcp9982_priv *priv = iio_priv(indio_dev);
> +
> + if (device_property_read_bool(dev, "microchip,enable-anti-parallel"))
> + set_bit(APDD_ENABLE, &priv->bit_flags);
> +
> + if (device_property_read_bool(dev, "microchip,parasitic-res-on-channel1-2"))
> + set_bit(RECD12_ENABLE, &priv->bit_flags);
> +
> + if (device_property_read_bool(dev, "microchip,parasitic-res-on-channel3-4"))
> + set_bit(RECD34_ENABLE, &priv->bit_flags);
> +
> + priv->num_channels = device_get_child_node_count(dev) + 1;
> +
> + if (priv->num_channels > device_nr_channels)
> + return dev_err_probe(dev, -E2BIG,
> + "More channels than the chip supports\n");
> +
> + priv->iio_chan[0] = MCP9982_CHAN(0, 0, MCP9982_HIGH_BYTE_ADDR(0));
> +
I'd loose this blank line to keep the two parts of setting stuff for first channel
together. Saves on what I was otherwise going to ask for which was a comment
on what this first channel was.
> + priv->labels[0] = "internal diode";
> + iio_idx++;
> + device_for_each_child_node_scoped(dev, child) {
> + reg_nr = 0;
Perhaps pull the declaration of reg_nr into this scope.
unsigned int reg_nr = 0;
> + fwnode_property_read_u32(child, "reg", ®_nr);
> + if (!reg_nr || reg_nr >= device_nr_channels)
> + return dev_err_probe(dev, -EINVAL,
> + "The index of the channels does not match the chip\n");
One less tab would look better and be more common wrapping style.
return dev_err_probe(dev, -EINVAL,
"The index of the channels does not match the chip\n");
> +
> + fwnode_property_read_string(child, "label",
> + &priv->labels[reg_nr]);
> +
> + priv->iio_chan[iio_idx++] = MCP9982_CHAN(reg_nr, reg_nr,
> + MCP9982_HIGH_BYTE_ADDR(reg_nr));
Slighlty prefer this as;
priv->iio_chan[iio_idx++] =
MCP9982_CHAN(reg_nr, reg_nr, MCP9982_HIGH_BYTE_ADDR(reg_nr));
Though not important.
> + }
> +
> + return 0;
> +}
> +static const struct of_device_id mcp9982_of_match[] = {
> + {
> + .compatible = "microchip,mcp9933",
> + .data = &mcp9933_chip_config
Trailing commas needed as maybe we'll one day have something else
to add here and the lack of comma will make that more noisy than
it would otherwise be.
> + }, {
> + .compatible = "microchip,mcp9933d",
> + .data = &mcp9933d_chip_config
> + }, {
> + .compatible = "microchip,mcp9982",
> + .data = &mcp9982_chip_config
> + }, {
> + .compatible = "microchip,mcp9982d",
> + .data = &mcp9982d_chip_config
> + }, {
> + .compatible = "microchip,mcp9983",
> + .data = &mcp9983_chip_config
> + }, {
> + .compatible = "microchip,mcp9983d",
> + .data = &mcp9983d_chip_config
> + }, {
> + .compatible = "microchip,mcp9984",
> + .data = &mcp9984_chip_config
> + }, {
> + .compatible = "microchip,mcp9984d",
> + .data = &mcp9984d_chip_config
> + }, {
> + .compatible = "microchip,mcp9985",
> + .data = &mcp9985_chip_config
> + }, {
> + .compatible = "microchip,mcp9985d",
> + .data = &mcp9985d_chip_config
> + },
> + { }
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 0/2] add support for MCP998X
2025-09-18 11:19 [PATCH v5 0/2] add support for MCP998X victor.duicu
2025-09-18 11:19 ` [PATCH v5 1/2] dt-bindings: iio: temperature: " victor.duicu
2025-09-18 11:19 ` [PATCH v5 2/2] " victor.duicu
@ 2025-09-20 11:42 ` Jonathan Cameron
2 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2025-09-20 11:42 UTC (permalink / raw)
To: victor.duicu
Cc: dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-iio,
linux-kernel, devicetree, marius.cristea
On Thu, 18 Sep 2025 14:19:35 +0300
<victor.duicu@microchip.com> wrote:
> From: Victor Duicu <victor.duicu@microchip.com>
>
> Add support for Microchip MCP998X/33 and MCP998XD/33D Multichannel Automotive Temperature Monitor Family.
>
> The chips in the family have different numbers of external channels, ranging from 1 (MCP9982) to 4 channels (MCP9985).
> Reading diodes in anti-parallel connection is supported by MCP9984/85/33 and MCP9984D/85D/33D.
> Dedicated hardware shutdown circuitry is present only in MCP998XD and MCP9933D.
>
> Current version of driver does not support interrupts, events and data buffering.
Similar to the emc1812, this probably wants a short statement of why IIO rather than hwmon.
Given it is called out as an automotive sensor I'm more confident this one is suitable
for IIO, but still good to state that in the cover letter.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 2/2] iio: temperature: add support for MCP998X
2025-09-20 10:55 ` Jonathan Cameron
@ 2025-09-24 12:47 ` Victor.Duicu
2025-09-27 15:25 ` Jonathan Cameron
0 siblings, 1 reply; 11+ messages in thread
From: Victor.Duicu @ 2025-09-24 12:47 UTC (permalink / raw)
To: jic23
Cc: dlechner, nuno.sa, linux-iio, devicetree, robh, linux-kernel,
andy, krzk+dt, conor+dt, Marius.Cristea
On Sat, 2025-09-20 at 11:55 +0100, Jonathan Cameron wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
Hi Jonathan,
> On Thu, 18 Sep 2025 14:19:37 +0300
> <victor.duicu@microchip.com> wrote:
>
> > From: Victor Duicu <victor.duicu@microchip.com>
> >
> > This is the driver for Microchip MCP998X/33 and MCP998XD/33D
> > Multichannel
> > Automotive Temperature Monitor Family.
> >
> > Signed-off-by: Victor Duicu <victor.duicu@microchip.com>
> Hi Victor,
>
> Various minor comments inline.
> Given the build warnings I didn't elect to just tidy these up whilst
> applying.
> Seemed like there was slightly too high a risk of me messing it up!
> Also we have lots of time as IIO is closed for this cycle now.
>
> Jonathan
>
...
>
>
> > +/**
> > + * Bit flags and their meaning
>
> As below. I don't think it is worth encoding these in a bitmap. Just
> use
> 5 bools to represent the state.
>
> > + * @RECD34_ENABLE: state of Resistance Error
> > Correction(REC) on channels 3 and 4
> > + * @RECD12_ENABLE: state of Resistance Error
> > Correction(REC) on channels 1 and 2
> > + * @APDD_ENABLE: state of anti-parallel diode mode
> > + * @RUN_STATE: chip is in run state,
> > otherwise is in standby state
> > + * @WAIT_BEFORE_READ: whether we need to wait a
> > delay before reading a new value
> > + */
> > +#define RECD34_ENABLE 0
> > +#define RECD12_ENABLE 1
> > +#define APDD_ENABLE 2
> > +#define RUN_STATE 3
> > +#define WAIT_BEFORE_READ 4
> > +#define USE_PREVIOUS_FREQ 5
> > +
Considering that I am planning to add new features to this driver,
I think that it would be useful to keep the flags.
> > +#define MCP9982_CHAN(index, si, __address)
> > ( \
> Why the outer set of ()?
Without the outer () compiler returns error "Macros with complex values
should be enclosed in parentheses."
> > + (struct iio_chan_spec)
> > { \
> > + .type =
> > IIO_TEMP, \
> > + .info_mask_separate =
> > BIT(IIO_CHAN_INFO_RAW), \
> > + .info_mask_shared_by_all_available =
> > BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
> > +
> > BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),
Kind Regards,
Victor
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 2/2] iio: temperature: add support for MCP998X
2025-09-24 12:47 ` Victor.Duicu
@ 2025-09-27 15:25 ` Jonathan Cameron
0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2025-09-27 15:25 UTC (permalink / raw)
To: Victor.Duicu
Cc: dlechner, nuno.sa, linux-iio, devicetree, robh, linux-kernel,
andy, krzk+dt, conor+dt, Marius.Cristea
On Wed, 24 Sep 2025 12:47:31 +0000
<Victor.Duicu@microchip.com> wrote:
> On Sat, 2025-09-20 at 11:55 +0100, Jonathan Cameron wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> >
>
> Hi Jonathan,
>
> > On Thu, 18 Sep 2025 14:19:37 +0300
> > <victor.duicu@microchip.com> wrote:
> >
> > > From: Victor Duicu <victor.duicu@microchip.com>
> > >
> > > This is the driver for Microchip MCP998X/33 and MCP998XD/33D
> > > Multichannel
> > > Automotive Temperature Monitor Family.
> > >
> > > Signed-off-by: Victor Duicu <victor.duicu@microchip.com>
> > Hi Victor,
> >
> > Various minor comments inline.
> > Given the build warnings I didn't elect to just tidy these up whilst
> > applying.
> > Seemed like there was slightly too high a risk of me messing it up!
> > Also we have lots of time as IIO is closed for this cycle now.
> >
> > Jonathan
> >
> ...
>
> >
> >
> > > +/**
> > > + * Bit flags and their meaning
> >
> > As below. I don't think it is worth encoding these in a bitmap. Just
> > use
> > 5 bools to represent the state.
> >
> > > + * @RECD34_ENABLE: state of Resistance Error
> > > Correction(REC) on channels 3 and 4
> > > + * @RECD12_ENABLE: state of Resistance Error
> > > Correction(REC) on channels 1 and 2
> > > + * @APDD_ENABLE: state of anti-parallel diode mode
> > > + * @RUN_STATE: chip is in run state,
> > > otherwise is in standby state
> > > + * @WAIT_BEFORE_READ: whether we need to wait a
> > > delay before reading a new value
> > > + */
> > > +#define RECD34_ENABLE 0
> > > +#define RECD12_ENABLE 1
> > > +#define APDD_ENABLE 2
> > > +#define RUN_STATE 3
> > > +#define WAIT_BEFORE_READ 4
> > > +#define USE_PREVIOUS_FREQ 5
> > > +
>
> Considering that I am planning to add new features to this driver,
> I think that it would be useful to keep the flags.
Unless there are many of these I think the loss of readability vs
the likely limited extra space used by bools still makes flags a
less than ideal approach. For reasons of DMA safe buffer allocations
on many architectures there is a lot of padding in the iio_dev / iio_priv
allocation, so you can always check if it makes any difference at all
on the allocated data
>
> > > +#define MCP9982_CHAN(index, si, __address)
> > > ( \
> > Why the outer set of ()?
>
> Without the outer () compiler returns error "Macros with complex values
> should be enclosed in parentheses."
Hmm. I'd ignore that as an unhelpful warning in this case.
>
> > > + (struct iio_chan_spec)
> > > { \
> > > + .type =
> > > IIO_TEMP, \
> > > + .info_mask_separate =
> > > BIT(IIO_CHAN_INFO_RAW), \
> > > + .info_mask_shared_by_all_available =
> > > BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
> > > +
> > > BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),
>
> Kind Regards,
> Victor
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-09-27 15:25 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-18 11:19 [PATCH v5 0/2] add support for MCP998X victor.duicu
2025-09-18 11:19 ` [PATCH v5 1/2] dt-bindings: iio: temperature: " victor.duicu
2025-09-18 14:56 ` Conor Dooley
2025-09-18 11:19 ` [PATCH v5 2/2] " victor.duicu
2025-09-19 6:27 ` kernel test robot
2025-09-20 10:39 ` Jonathan Cameron
2025-09-19 6:59 ` kernel test robot
2025-09-20 10:55 ` Jonathan Cameron
2025-09-24 12:47 ` Victor.Duicu
2025-09-27 15:25 ` Jonathan Cameron
2025-09-20 11:42 ` [PATCH v5 0/2] " Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).