* [PATCH v6 0/3] iio: proximity: Add TYHX HX9023S sensor driver
@ 2024-06-21 7:40 Yasin Lee
2024-06-21 7:40 ` [PATCH v6 1/3] dt-bindings: vendor-prefixes: add tyhx Yasin Lee
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Yasin Lee @ 2024-06-21 7:40 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
Lars-Peter Clausen, yasin.lee.x
Cc: devicetree, linux-kernel, linux-iio, Yasin Lee
Signed-off-by: Yasin Lee <yasin.lee.x@gmail.com>
---
Changes in v6:
**vendor-prefixes.yaml:**
- Formatted the description in the commit
**tyhx,hx9023s.yaml:**
- Added a link to the DataSheet
- Added `#address-cells` and `#size-cells`
- Removed the following properties:
- `channel-in-use`
- `channel-positive`
- `channel-negative`
- Corrected the reference path to `adc.yaml`
- Inherited `diff-channels` from `adc.yaml` as differential channels
- Added `input-channel` property as a single-ended configuration channel
- Made `vdd-supply` a required option
- Used the generic name `proximity` instead of the chip name in the DTS example
**hx9023s.c:**
- Removed the following headers:
- `#include <linux/acpi.h>`
- `#include <linux/byteorder/generic.h>`
- `#include <linux/kernel.h>`
- Added `#include <asm/byteorder.h>`
- Updated some macro names:
- `HX9023S_DATA_2BYTES` ---> `HX9023S_2BYTES`
- `HX9023S_DATA_3BYTES` ---> `HX9023S_3BYTES`
- `HX9023S_DATA_BYTES_MAX` ---> `HX9023S_BYTES_MAX`
- Added the following macro definitions:
- `#define HX9023S_POS 0x03`
- `#define HX9023S_NEG 0x02`
- `#define HX9023S_NOT_CONNECTED 16`
- `#define HX9023S_GLOBAL_CTRL0 0x00`
- `struct hx9023s_ch_data`:
- Added comments explaining the members `raw`, `lp`, `bl`, `diff`
- Changed the type of struct member `thre` from `int` to `unsigned int`
- Removed member `cs_position`
- Changed the type of `struct buffer` member `channels[HX9023S_CH_NUM]` from `__be16` to `__le16`
- Moved `struct hx9023s_ch_data ch_data` to the bottom
- Updated comments in the array `hx9023s_reg_init_list`:
- Changed `adc` to `ADC`, `avg` to `average`, and `osr` to `OSR`
- Provided a more detailed comment for the register `HX9023S_RAW_BL_RD_CFG`
- `struct regmap_config hx9023s_regmap_config`:
- Added `.rd_type` member
- Changed `.cache_type` from `REGCACHE_RBTREE` to `REGCACHE_MAPLE`
- Bugfix: The function `hx9023s_data_lock` now only operates on one bit instead of two
- `hx9023s_ch_cfg` function:
- Replaced magic characters with meaningful macro definitions
- Changed byte concatenation operations to use `put_unaligned_le16`
- Reviewed each function and made the following changes:
- Some returns now directly return `regmap_read/write()`
- For single-byte write operations, changed from `regmap_bulk_write(,,,1)` to `regmap_write(,,)`
- Changed the type of loop variable `i` to `unsigned`
- `hx9023s_write_far_debounce` and `hx9023s_write_near_debounce` functions:
- Removed magic numbers and optimized the code
- `hx9023s_get_thres_near` and `hx9023s_get_thres_far` functions:
- Used intermediate variables for readability and simplified the code logic
- `hx9023s_set_thres_near` and `hx9023s_set_thres_far` functions:
- Optimized the logic and simplified the code
- `hx9023s_get_prox_state` function:
- Changed the return value type to `int`
- `hx9023s_data_select` function:
- Changed the return value type to `int`
- `hx9023s_sample` function:
- Added intermediate variables for better readability
- `hx9023s_ch_en` function:
- Optimized the code logic to reduce line count
- `hx9023s_property_get` function:
- Rewritten due to DTS changes, and calls `fwnode_handle_put` on error to prevent memory leaks
- `hx9023s_get_proximity` function:
- Added error handling logic for data
- `hx9023s_get_samp_freq` function:
- Used macro definitions from `units.h` instead of specific numbers for frequency calculation
- `hx9023s_set_samp_freq` function:
- Used macro definitions from `units.h` instead of specific numbers for frequency calculation
- `hx9023s_write_raw` function:
- Changed the type of local variable `dir` to `unsigned`
- `hx9023s_write_event_config` function:
- Replaced `set_bit` and `clear_bit` with `__assign_bit`
- `hx9023s_trigger_handler` function:
- Added error handling logic for data; fixed uninitialized variable `i` bug
- `hx9023s_buffer_preenable` function:
- Fixed uninitialized variable `channel` bug
- Added `hx9023s_id_check` function
- `hx9023s_probe` function:
- Returns directly on `devm_iio_device_alloc` error, without logging; removed `fsleep`
- `hx9023s_resume` function:
- Calls `hx9023s_interrupt_enable` only when `data->trigger_enabled` is true
- Removed `hx9023s_acpi_match`
- Link to v5: https://lore.kernel.org/linux-iio/SN7PR12MB8101B6D0AB1246797C67E25BA4CE2@SN7PR12MB8101.namprd12.prod.outlook.com/
Changes in v5:
- I have addressed all the issues mentioned in the email responses.
Additionally, regarding the IIO-related header files, I have checked and found no unused headers.
- Link to v4: https://lore.kernel.org/linux-iio/SN7PR12MB810129D8180B1C9593A8E078A4FB2@SN7PR12MB8101.namprd12.prod.outlook.com/
Changes in v4:
- Removed hardware-irrelevant properties from dt-bindings, retaining only channel configuration
related `channel-positive` and `channel-negative`. Grouped by channel.
Retained `channel-in-use` as it is hardware-related.
- Removed redundant register definitions.
- Reorganized `struct hx9023s_data`, extracting channel-related attributes
into a new `struct hx9023s_ch_data`.
- Optimized bit operation related code.
- Replaced `of_` versions with generic firmware parsing functions.
- Fixed other issues mentioned in the email feedback.
- Link to v3: https://lore.kernel.org/linux-iio/20240602152638.2c674930@jic23-huawei/
---
Yasin Lee (3):
dt-bindings: vendor-prefixes: add tyhx
dt-bindings: iio: proximity: Add TYHX HX9023S
iio: proximity: Add driver support for TYHX's HX9023S capacitive proximity sensor
.../bindings/iio/proximity/tyhx,hx9023s.yaml | 115 ++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
drivers/iio/proximity/Kconfig | 14 +
drivers/iio/proximity/Makefile | 1 +
drivers/iio/proximity/hx9023s.c | 1150 ++++++++++++++++++++
5 files changed, 1282 insertions(+)
---
base-commit: 0291f73eaa59fe0dbb715fa405bb47a1d8206e0e
change-id: 20240616-add-tyhx-hx9023s-sensor-driver-e7dbe3bfe596
Best regards,
--
Yasin Lee <yasin.lee.x@gmail.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v6 1/3] dt-bindings: vendor-prefixes: add tyhx
2024-06-21 7:40 [PATCH v6 0/3] iio: proximity: Add TYHX HX9023S sensor driver Yasin Lee
@ 2024-06-21 7:40 ` Yasin Lee
2024-06-21 10:07 ` Krzysztof Kozlowski
2024-06-21 7:40 ` [PATCH v6 2/3] dt-bindings: iio: proximity: Add TYHX HX9023S Yasin Lee
2024-06-21 7:40 ` [PATCH v6 3/3] iio: proximity: Add driver support for TYHX's HX9023S capacitive proximity sensor Yasin Lee
2 siblings, 1 reply; 15+ messages in thread
From: Yasin Lee @ 2024-06-21 7:40 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
Lars-Peter Clausen, yasin.lee.x
Cc: devicetree, linux-kernel, linux-iio, Yasin Lee
Add vendor prefix for NanjingTianyihexin Electronics Ltd.
http://www.tianyihexin.com
Signed-off-by: Yasin Lee <yasin.lee.x@gmail.com>
---
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index fbf47f0bacf1..989242da4f44 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1515,6 +1515,8 @@ patternProperties:
description: Turing Machines, Inc.
"^tyan,.*":
description: Tyan Computer Corporation
+ "^tyhx,.*":
+ description: NanjingTianyihexin Electronics Ltd.
"^u-blox,.*":
description: u-blox
"^u-boot,.*":
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v6 2/3] dt-bindings: iio: proximity: Add TYHX HX9023S
2024-06-21 7:40 [PATCH v6 0/3] iio: proximity: Add TYHX HX9023S sensor driver Yasin Lee
2024-06-21 7:40 ` [PATCH v6 1/3] dt-bindings: vendor-prefixes: add tyhx Yasin Lee
@ 2024-06-21 7:40 ` Yasin Lee
2024-06-21 10:12 ` Krzysztof Kozlowski
2024-06-21 7:40 ` [PATCH v6 3/3] iio: proximity: Add driver support for TYHX's HX9023S capacitive proximity sensor Yasin Lee
2 siblings, 1 reply; 15+ messages in thread
From: Yasin Lee @ 2024-06-21 7:40 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
Lars-Peter Clausen, yasin.lee.x
Cc: devicetree, linux-kernel, linux-iio, Yasin Lee
A capacitive proximity sensor
Signed-off-by: Yasin Lee <yasin.lee.x@gmail.com>
---
.../bindings/iio/proximity/tyhx,hx9023s.yaml | 115 +++++++++++++++++++++
1 file changed, 115 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml b/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
new file mode 100644
index 000000000000..beca70ce7609
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
@@ -0,0 +1,115 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/proximity/tyhx,hx9023s.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TYHX HX9023S capacitive proximity sensor
+
+maintainers:
+ - Yasin Lee <yasin.lee.x@gmail.com>
+
+description: |
+ TYHX HX9023S proximity sensor. Datasheet can be found here:
+ http://www.tianyihexin.com/ueditor/php/upload/file/20240614/1718336303992081.pdf
+
+allOf:
+ - $ref: /schemas/iio/iio.yaml#
+
+properties:
+ compatible:
+ const: tyhx,hx9023s
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ description:
+ Generated by device to announce preceding read request has finished
+ and data is available or that a close/far proximity event has happened.
+ maxItems: 1
+
+ vdd-supply: true
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+patternProperties:
+ "^channel@[0-4]$":
+ $ref: /schemas/iio/adc/adc.yaml
+ type: object
+
+ properties:
+ reg:
+ minimum: 0
+ maximum: 4
+ description: The channel number.
+
+ input-channel:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 4
+ description:
+ Specify the input pin used in single-ended configuration.
+
+ diff-channels: true
+
+ oneOf:
+ - required:
+ - input-channel
+ - required:
+ - diff-channels
+
+ required:
+ - reg
+
+ additionalProperties: false
+
+required:
+ - compatible
+ - vdd-supply
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ proximity@2a {
+ compatible = "tyhx,hx9023s";
+ reg = <0x2a>;
+ interrupt-parent = <&pio>;
+ interrupts = <16 IRQ_TYPE_EDGE_FALLING>;
+ vdd-supply = <&pp1800_prox>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ channel@0 {
+ reg = <0>;
+ input-channel = <0>;
+ };
+ channel@1 {
+ reg = <1>;
+ input-channel = <1>;
+ };
+ channel@2 {
+ reg = <2>;
+ input-channel = <2>;
+ };
+ channel@3 {
+ reg = <3>;
+ diff-channels = <1 0>;
+ };
+ channel@4 {
+ reg = <4>;
+ diff-channels = <2 0>;
+ };
+ };
+ };
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v6 3/3] iio: proximity: Add driver support for TYHX's HX9023S capacitive proximity sensor
2024-06-21 7:40 [PATCH v6 0/3] iio: proximity: Add TYHX HX9023S sensor driver Yasin Lee
2024-06-21 7:40 ` [PATCH v6 1/3] dt-bindings: vendor-prefixes: add tyhx Yasin Lee
2024-06-21 7:40 ` [PATCH v6 2/3] dt-bindings: iio: proximity: Add TYHX HX9023S Yasin Lee
@ 2024-06-21 7:40 ` Yasin Lee
2024-06-21 14:09 ` Alexandru Ardelean
2024-06-23 11:56 ` Jonathan Cameron
2 siblings, 2 replies; 15+ messages in thread
From: Yasin Lee @ 2024-06-21 7:40 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
Lars-Peter Clausen, yasin.lee.x
Cc: devicetree, linux-kernel, linux-iio, Yasin Lee
A SAR sensor from NanjingTianyihexin Electronics Ltd.
The device has the following entry points:
Usual frequency:
- sampling_frequency
Instant reading of current values for different sensors:
- in_proximity0_raw
- in_proximity1_raw
- in_proximity2_raw
- in_proximity3_raw
- in_proximity4_raw
and associated events in events/
Signed-off-by: Yasin Lee <yasin.lee.x@gmail.com>
---
drivers/iio/proximity/Kconfig | 14 +
drivers/iio/proximity/Makefile | 1 +
drivers/iio/proximity/hx9023s.c | 1150 +++++++++++++++++++++++++++++++++++++++
3 files changed, 1165 insertions(+)
diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
index 2ca3b0bc5eba..0694f625b432 100644
--- a/drivers/iio/proximity/Kconfig
+++ b/drivers/iio/proximity/Kconfig
@@ -32,6 +32,20 @@ config CROS_EC_MKBP_PROXIMITY
To compile this driver as a module, choose M here: the
module will be called cros_ec_mkbp_proximity.
+config HX9023S
+ tristate "TYHX HX9023S SAR sensor"
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
+ select REGMAP_I2C
+ depends on I2C
+ help
+ Say Y here to build a driver for TYHX HX9023S capacitive SAR sensor.
+ This driver supports the TYHX HX9023S capacitive
+ SAR sensors. This sensors is used for proximity detection applications.
+
+ To compile this driver as a module, choose M here: the
+ module will be called hx9023s.
+
config IRSD200
tristate "Murata IRS-D200 PIR sensor"
select IIO_BUFFER
diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
index f36598380446..ab381cd27dbb 100644
--- a/drivers/iio/proximity/Makefile
+++ b/drivers/iio/proximity/Makefile
@@ -6,6 +6,7 @@
# When adding new entries keep the list in alphabetical order
obj-$(CONFIG_AS3935) += as3935.o
obj-$(CONFIG_CROS_EC_MKBP_PROXIMITY) += cros_ec_mkbp_proximity.o
+obj-$(CONFIG_HX9023S) += hx9023s.o
obj-$(CONFIG_IRSD200) += irsd200.o
obj-$(CONFIG_ISL29501) += isl29501.o
obj-$(CONFIG_LIDAR_LITE_V2) += pulsedlight-lidar-lite-v2.o
diff --git a/drivers/iio/proximity/hx9023s.c b/drivers/iio/proximity/hx9023s.c
new file mode 100644
index 000000000000..1d8cb9a05d8a
--- /dev/null
+++ b/drivers/iio/proximity/hx9023s.c
@@ -0,0 +1,1150 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2024 NanjingTianyihexin Electronics Ltd.
+ * http://www.tianyihexin.com
+ *
+ * Driver for NanjingTianyihexin HX9023S Cap Sensor.
+ * Datasheet available at:
+ * http://www.tianyihexin.com/ueditor/php/upload/file/20240614/1718336303992081.pdf
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/irqreturn.h>
+#include <linux/math.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pm.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/types.h>
+#include <linux/units.h>
+
+#include <asm/byteorder.h>
+#include <asm/unaligned.h>
+
+#include <linux/iio/buffer.h>
+#include <linux/iio/events.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/types.h>
+
+#define HX9023S_CHIP_ID 0x1D
+#define HX9023S_CH_NUM 5
+#define HX9023S_2BYTES 2
+#define HX9023S_3BYTES 3
+#define HX9023S_BYTES_MAX HX9023S_3BYTES
+#define HX9023S_POS 0x03
+#define HX9023S_NEG 0x02
+#define HX9023S_NOT_CONNECTED 16
+
+#define HX9023S_GLOBAL_CTRL0 0x00
+#define HX9023S_PRF_CFG 0x02
+#define HX9023S_CH0_CFG_7_0 0x03
+#define HX9023S_CH4_CFG_9_8 0x0C
+#define HX9023S_RANGE_7_0 0x0D
+#define HX9023S_RANGE_9_8 0x0E
+#define HX9023S_RANGE_18_16 0x0F
+#define HX9023S_AVG0_NOSR0_CFG 0x10
+#define HX9023S_NOSR12_CFG 0x11
+#define HX9023S_NOSR34_CFG 0x12
+#define HX9023S_AVG12_CFG 0x13
+#define HX9023S_AVG34_CFG 0x14
+#define HX9023S_OFFSET_DAC0_7_0 0x15
+#define HX9023S_OFFSET_DAC4_9_8 0x1E
+#define HX9023S_SAMPLE_NUM_7_0 0x1F
+#define HX9023S_INTEGRATION_NUM_7_0 0x21
+#define HX9023S_CH_NUM_CFG 0x24
+#define HX9023S_LP_ALP_4_CFG 0x29
+#define HX9023S_LP_ALP_1_0_CFG 0x2A
+#define HX9023S_LP_ALP_3_2_CFG 0x2B
+#define HX9023S_UP_ALP_1_0_CFG 0x2C
+#define HX9023S_UP_ALP_3_2_CFG 0x2D
+#define HX9023S_DN_UP_ALP_0_4_CFG 0x2E
+#define HX9023S_DN_ALP_2_1_CFG 0x2F
+#define HX9023S_DN_ALP_4_3_CFG 0x30
+#define HX9023S_RAW_BL_RD_CFG 0x38
+#define HX9023S_INTERRUPT_CFG 0x39
+#define HX9023S_INTERRUPT_CFG1 0x3A
+#define HX9023S_CALI_DIFF_CFG 0x3B
+#define HX9023S_DITHER_CFG 0x3C
+#define HX9023S_DEVICE_ID 0x60
+#define HX9023S_PROX_STATUS 0x6B
+#define HX9023S_PROX_INT_HIGH_CFG 0x6C
+#define HX9023S_PROX_INT_LOW_CFG 0x6D
+#define HX9023S_PROX_HIGH_DIFF_CFG_CH0_0 0x80
+#define HX9023S_PROX_LOW_DIFF_CFG_CH0_0 0x88
+#define HX9023S_PROX_LOW_DIFF_CFG_CH3_1 0x8F
+#define HX9023S_PROX_HIGH_DIFF_CFG_CH4_0 0x9E
+#define HX9023S_PROX_HIGH_DIFF_CFG_CH4_1 0x9F
+#define HX9023S_PROX_LOW_DIFF_CFG_CH4_0 0xA2
+#define HX9023S_PROX_LOW_DIFF_CFG_CH4_1 0xA3
+#define HX9023S_CAP_INI_CH4_0 0xB3
+#define HX9023S_LP_DIFF_CH4_2 0xBA
+#define HX9023S_RAW_BL_CH4_0 0xB5
+#define HX9023S_LP_DIFF_CH4_0 0xB8
+#define HX9023S_DSP_CONFIG_CTRL1 0xC8
+#define HX9023S_CAP_INI_CH0_0 0xE0
+#define HX9023S_RAW_BL_CH0_0 0xE8
+#define HX9023S_LP_DIFF_CH0_0 0xF4
+#define HX9023S_LP_DIFF_CH3_2 0xFF
+
+#define HX9023S_DATA_LOCK_MASK BIT(4)
+#define HX9023S_INTERRUPT_MASK GENMASK(9, 0)
+#define HX9023S_PROX_DEBOUNCE_MASK GENMASK(3, 0)
+
+struct hx9023s_addr_val_pair {
+ u8 addr;
+ u8 val;
+};
+
+struct hx9023s_ch_data {
+ int raw; /* Raw Data*/
+ int lp; /* Low Pass Filter Data*/
+ int bl; /* Base Line Data */
+ int diff; /* difference of Low Pass Data and Base Line Data */
+
+ struct {
+ unsigned int near;
+ unsigned int far;
+ } thres;
+
+ u16 dac;
+ u8 channel_positive;
+ u8 channel_negative;
+ bool sel_bl;
+ bool sel_raw;
+ bool sel_diff;
+ bool sel_lp;
+ bool enable;
+};
+
+struct hx9023s_data {
+ struct iio_trigger *trig;
+ struct regmap *regmap;
+ unsigned long chan_prox_stat;
+ unsigned long chan_read;
+ unsigned long chan_event;
+ unsigned long ch_en_stat;
+ unsigned long chan_in_use;
+ unsigned int prox_state_reg;
+ bool trigger_enabled;
+
+ struct {
+ __le16 channels[HX9023S_CH_NUM];
+ s64 ts __aligned(8);
+ } buffer;
+
+ struct mutex mutex;
+ struct hx9023s_ch_data ch_data[HX9023S_CH_NUM];
+};
+
+static struct hx9023s_addr_val_pair hx9023s_reg_init_list[] = {
+ /* scan period */
+ { HX9023S_PRF_CFG, 0x17 },
+
+ /* full scale of conversion phase of each channel */
+ { HX9023S_RANGE_7_0, 0x11 },
+ { HX9023S_RANGE_9_8, 0x02 },
+ { HX9023S_RANGE_18_16, 0x00 },
+
+ /* ADC average number and OSR number of each channel */
+ { HX9023S_AVG0_NOSR0_CFG, 0x71 },
+ { HX9023S_NOSR12_CFG, 0x44 },
+ { HX9023S_NOSR34_CFG, 0x00 },
+ { HX9023S_AVG12_CFG, 0x33 },
+ { HX9023S_AVG34_CFG, 0x00 },
+
+ /* sample & integration frequency of the ADC */
+ { HX9023S_SAMPLE_NUM_7_0, 0x65 },
+ { HX9023S_INTEGRATION_NUM_7_0, 0x65 },
+
+ /* coefficient of the first order low pass filter during each channel */
+ { HX9023S_LP_ALP_1_0_CFG, 0x22 },
+ { HX9023S_LP_ALP_3_2_CFG, 0x22 },
+ { HX9023S_LP_ALP_4_CFG, 0x02 },
+
+ /* up coefficient of the first order low pass filter during each channel */
+ { HX9023S_UP_ALP_1_0_CFG, 0x88 },
+ { HX9023S_UP_ALP_3_2_CFG, 0x88 },
+ { HX9023S_DN_UP_ALP_0_4_CFG, 0x18 },
+
+ /* down coefficient of the first order low pass filter during each channel */
+ { HX9023S_DN_ALP_2_1_CFG, 0x11 },
+ { HX9023S_DN_ALP_4_3_CFG, 0x11 },
+
+ /* selection of data for the Data Mux Register to output data */
+ { HX9023S_RAW_BL_RD_CFG, 0xF0 },
+
+ /* enable the interrupt function */
+ { HX9023S_INTERRUPT_CFG, 0xFF },
+ { HX9023S_INTERRUPT_CFG1, 0x3B },
+ { HX9023S_DITHER_CFG, 0x21 },
+
+ /* threshold of the offset compensation */
+ { HX9023S_CALI_DIFF_CFG, 0x07 },
+
+ /* proximity persistency number(near & far) */
+ { HX9023S_PROX_INT_HIGH_CFG, 0x01 },
+ { HX9023S_PROX_INT_LOW_CFG, 0x01 },
+
+ /* disable the data lock */
+ { HX9023S_DSP_CONFIG_CTRL1, 0x00 },
+};
+
+static const struct iio_event_spec hx9023s_events[] = {
+ {
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_RISING,
+ .mask_shared_by_all = BIT(IIO_EV_INFO_PERIOD),
+ .mask_separate = BIT(IIO_EV_INFO_VALUE),
+ },
+ {
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_FALLING,
+ .mask_shared_by_all = BIT(IIO_EV_INFO_PERIOD),
+ .mask_separate = BIT(IIO_EV_INFO_VALUE),
+
+ },
+ {
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_EITHER,
+ .mask_separate = BIT(IIO_EV_INFO_ENABLE),
+ },
+};
+
+#define HX9023S_CHANNEL(idx) \
+{ \
+ .type = IIO_PROXIMITY, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
+ .indexed = 1, \
+ .channel = idx, \
+ .address = 0, \
+ .event_spec = hx9023s_events, \
+ .num_event_specs = ARRAY_SIZE(hx9023s_events), \
+ .scan_index = idx, \
+ .scan_type = { \
+ .sign = 's', \
+ .realbits = 16, \
+ .storagebits = 16, \
+ .endianness = IIO_BE, \
+ }, \
+}
+
+static const struct iio_chan_spec hx9023s_channels[] = {
+ HX9023S_CHANNEL(0),
+ HX9023S_CHANNEL(1),
+ HX9023S_CHANNEL(2),
+ HX9023S_CHANNEL(3),
+ HX9023S_CHANNEL(4),
+ IIO_CHAN_SOFT_TIMESTAMP(5),
+};
+
+static const unsigned int hx9023s_samp_freq_table[] = {
+ 2, 2, 4, 6, 8, 10, 14, 18, 22, 26,
+ 30, 34, 38, 42, 46, 50, 56, 62, 68, 74,
+ 80, 90, 100, 200, 300, 400, 600, 800, 1000, 2000,
+ 3000, 4000,
+};
+
+static const struct regmap_range hx9023s_rd_reg_ranges[] = {
+ regmap_reg_range(HX9023S_GLOBAL_CTRL0, HX9023S_LP_DIFF_CH3_2),
+};
+
+static const struct regmap_range hx9023s_wr_reg_ranges[] = {
+ regmap_reg_range(HX9023S_GLOBAL_CTRL0, HX9023S_LP_DIFF_CH3_2),
+};
+
+static const struct regmap_range hx9023s_volatile_reg_ranges[] = {
+ regmap_reg_range(HX9023S_CAP_INI_CH4_0, HX9023S_LP_DIFF_CH4_2),
+ regmap_reg_range(HX9023S_CAP_INI_CH0_0, HX9023S_LP_DIFF_CH3_2),
+ regmap_reg_range(HX9023S_PROX_STATUS, HX9023S_PROX_STATUS),
+};
+
+static const struct regmap_access_table hx9023s_rd_regs = {
+ .yes_ranges = hx9023s_rd_reg_ranges,
+ .n_yes_ranges = ARRAY_SIZE(hx9023s_rd_reg_ranges),
+};
+
+static const struct regmap_access_table hx9023s_wr_regs = {
+ .yes_ranges = hx9023s_wr_reg_ranges,
+ .n_yes_ranges = ARRAY_SIZE(hx9023s_wr_reg_ranges),
+};
+
+static const struct regmap_access_table hx9023s_volatile_regs = {
+ .yes_ranges = hx9023s_volatile_reg_ranges,
+ .n_yes_ranges = ARRAY_SIZE(hx9023s_volatile_reg_ranges),
+};
+
+static const struct regmap_config hx9023s_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .cache_type = REGCACHE_MAPLE,
+ .rd_table = &hx9023s_rd_regs,
+ .wr_table = &hx9023s_wr_regs,
+ .volatile_table = &hx9023s_volatile_regs,
+};
+
+static int hx9023s_interrupt_enable(struct hx9023s_data *data)
+{
+ return regmap_update_bits(data->regmap, HX9023S_INTERRUPT_CFG,
+ HX9023S_INTERRUPT_MASK, HX9023S_INTERRUPT_MASK);
+}
+
+static int hx9023s_interrupt_disable(struct hx9023s_data *data)
+{
+ return regmap_update_bits(data->regmap, HX9023S_INTERRUPT_CFG,
+ HX9023S_INTERRUPT_MASK, 0x00);
+}
+
+static int hx9023s_data_lock(struct hx9023s_data *data, bool locked)
+{
+ if (locked)
+ return regmap_update_bits(data->regmap, HX9023S_DSP_CONFIG_CTRL1,
+ HX9023S_DATA_LOCK_MASK, HX9023S_DATA_LOCK_MASK);
+ else
+ return regmap_update_bits(data->regmap, HX9023S_DSP_CONFIG_CTRL1,
+ HX9023S_DATA_LOCK_MASK, 0);
+}
+
+static int hx9023s_ch_cfg(struct hx9023s_data *data)
+{
+ unsigned int i;
+ u16 reg;
+ u8 reg_list[HX9023S_CH_NUM * 2];
+ u8 ch_pos[HX9023S_CH_NUM];
+ u8 ch_neg[HX9023S_CH_NUM];
+ /* Bit positions corresponding to input pin connections */
+ u8 conn_cs[HX9023S_CH_NUM] = {0, 2, 4, 6, 8};
+
+ for (i = 0; i < HX9023S_CH_NUM; i++) {
+ ch_pos[i] = data->ch_data[i].channel_positive == HX9023S_NOT_CONNECTED ?
+ HX9023S_NOT_CONNECTED : conn_cs[data->ch_data[i].channel_positive];
+ ch_neg[i] = data->ch_data[i].channel_negative == HX9023S_NOT_CONNECTED ?
+ HX9023S_NOT_CONNECTED : conn_cs[data->ch_data[i].channel_negative];
+
+ reg = (HX9023S_POS << ch_pos[i]) | (HX9023S_NEG << ch_neg[i]);
+ put_unaligned_le16(reg, ®_list[i * 2]);
+ }
+
+ return regmap_bulk_write(data->regmap, HX9023S_CH0_CFG_7_0, reg_list, HX9023S_CH_NUM * 2);
+}
+
+static int hx9023s_reg_init(struct hx9023s_data *data)
+{
+ unsigned int i = 0;
+ int ret;
+
+ for (i = 0; i < (int)ARRAY_SIZE(hx9023s_reg_init_list); i++) {
+ ret = regmap_write(data->regmap, hx9023s_reg_init_list[i].addr,
+ hx9023s_reg_init_list[i].val);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int hx9023s_write_far_debounce(struct hx9023s_data *data, int val)
+{
+ guard(mutex)(&data->mutex);
+ return regmap_update_bits(data->regmap, HX9023S_PROX_INT_LOW_CFG,
+ HX9023S_PROX_DEBOUNCE_MASK,
+ FIELD_GET(HX9023S_PROX_DEBOUNCE_MASK, val));
+}
+
+static int hx9023s_write_near_debounce(struct hx9023s_data *data, int val)
+{
+ guard(mutex)(&data->mutex);
+ return regmap_update_bits(data->regmap, HX9023S_PROX_INT_HIGH_CFG,
+ HX9023S_PROX_DEBOUNCE_MASK,
+ FIELD_GET(HX9023S_PROX_DEBOUNCE_MASK, val));
+}
+
+static int hx9023s_read_far_debounce(struct hx9023s_data *data, int *val)
+{
+ int ret;
+
+ ret = regmap_read(data->regmap, HX9023S_PROX_INT_LOW_CFG, val);
+ if (ret)
+ return ret;
+
+ *val = FIELD_GET(HX9023S_PROX_DEBOUNCE_MASK, *val);
+
+ return IIO_VAL_INT;
+}
+
+static int hx9023s_read_near_debounce(struct hx9023s_data *data, int *val)
+{
+ int ret;
+
+ ret = regmap_read(data->regmap, HX9023S_PROX_INT_HIGH_CFG, val);
+ if (ret)
+ return ret;
+
+ *val = FIELD_GET(HX9023S_PROX_DEBOUNCE_MASK, *val);
+
+ return IIO_VAL_INT;
+}
+
+static int hx9023s_get_thres_near(struct hx9023s_data *data, u8 ch, int *val)
+{
+ int ret;
+ u8 buf[2];
+ unsigned int reg, tmp;
+
+ reg = (ch == 4) ? HX9023S_PROX_HIGH_DIFF_CFG_CH4_0 :
+ HX9023S_PROX_HIGH_DIFF_CFG_CH0_0 + (ch * HX9023S_2BYTES);
+
+ ret = regmap_bulk_read(data->regmap, reg, buf, sizeof(buf));
+ if (ret)
+ return ret;
+
+ tmp = (get_unaligned_le16(buf) & GENMASK(9, 0)) * 32;
+ data->ch_data[ch].thres.near = tmp;
+ *val = tmp;
+
+ return IIO_VAL_INT;
+}
+
+static int hx9023s_get_thres_far(struct hx9023s_data *data, u8 ch, int *val)
+{
+ int ret;
+ u8 buf[2];
+ unsigned int reg, tmp;
+
+ reg = (ch == 4) ? HX9023S_PROX_LOW_DIFF_CFG_CH4_0 :
+ HX9023S_PROX_LOW_DIFF_CFG_CH0_0 + (ch * HX9023S_2BYTES);
+
+ ret = regmap_bulk_read(data->regmap, reg, buf, sizeof(buf));
+ if (ret)
+ return ret;
+
+ tmp = (get_unaligned_le16(buf) & GENMASK(9, 0)) * 32;
+ data->ch_data[ch].thres.far = tmp;
+ *val = tmp;
+
+ return IIO_VAL_INT;
+}
+
+static int hx9023s_set_thres_near(struct hx9023s_data *data, u8 ch, int val)
+{
+ __le16 val_le16 = cpu_to_le16((val / 32) & GENMASK(9, 0));
+ unsigned int reg;
+
+ data->ch_data[ch].thres.near = ((val / 32) & GENMASK(9, 0)) * 32;
+ reg = (ch == 4) ? HX9023S_PROX_HIGH_DIFF_CFG_CH4_0 :
+ HX9023S_PROX_HIGH_DIFF_CFG_CH0_0 + (ch * HX9023S_2BYTES);
+
+ return regmap_bulk_write(data->regmap, reg, &val_le16, sizeof(val_le16));
+}
+
+static int hx9023s_set_thres_far(struct hx9023s_data *data, u8 ch, int val)
+{
+ __le16 val_le16 = cpu_to_le16((val / 32) & GENMASK(9, 0));
+ unsigned int reg;
+
+ data->ch_data[ch].thres.far = ((val / 32) & GENMASK(9, 0)) * 32;
+ reg = (ch == 4) ? HX9023S_PROX_LOW_DIFF_CFG_CH4_0 :
+ HX9023S_PROX_LOW_DIFF_CFG_CH0_0 + (ch * HX9023S_2BYTES);
+
+ return regmap_bulk_write(data->regmap, reg, &val_le16, sizeof(val_le16));
+}
+
+static int hx9023s_get_prox_state(struct hx9023s_data *data)
+{
+ return regmap_read(data->regmap, HX9023S_PROX_STATUS, &data->prox_state_reg);
+}
+
+static int hx9023s_data_select(struct hx9023s_data *data)
+{
+ int ret;
+ unsigned int i, buf;
+ unsigned long tmp;
+
+ ret = regmap_read(data->regmap, HX9023S_RAW_BL_RD_CFG, &buf);
+ if (ret)
+ return ret;
+
+ tmp = buf;
+ for (i = 0; i < 4; i++) {
+ data->ch_data[i].sel_diff = test_bit(i, &tmp);
+ data->ch_data[i].sel_lp = !data->ch_data[i].sel_diff;
+ data->ch_data[i].sel_bl = test_bit(i + 4, &tmp);
+ data->ch_data[i].sel_raw = !data->ch_data[i].sel_bl;
+ }
+
+ ret = regmap_read(data->regmap, HX9023S_INTERRUPT_CFG1, &buf);
+ if (ret)
+ return ret;
+
+ tmp = buf;
+ data->ch_data[4].sel_diff = test_bit(2, &tmp);
+ data->ch_data[4].sel_lp = !data->ch_data[4].sel_diff;
+ data->ch_data[4].sel_bl = test_bit(3, &tmp);
+ data->ch_data[4].sel_raw = !data->ch_data[4].sel_bl;
+
+ return 0;
+}
+
+static int hx9023s_sample(struct hx9023s_data *data)
+{
+ int ret, value;
+ unsigned int i;
+ u8 data_size, offset_data_size, *p, size, rx_buf[HX9023S_CH_NUM * HX9023S_BYTES_MAX];
+
+ ret = hx9023s_data_lock(data, true);
+ if (ret)
+ return ret;
+
+ ret = hx9023s_data_select(data);
+ if (ret)
+ return ret;
+
+ data_size = HX9023S_3BYTES;
+
+ /* ch0~ch3 */
+ p = rx_buf;
+ size = (HX9023S_CH_NUM - 1) * data_size;
+ ret = regmap_bulk_read(data->regmap, HX9023S_RAW_BL_CH0_0, p, size);
+ if (ret)
+ return ret;
+
+ /* ch4 */
+ p = rx_buf + size;
+ size = data_size;
+ ret = regmap_bulk_read(data->regmap, HX9023S_RAW_BL_CH4_0, p, size);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < HX9023S_CH_NUM; i++) {
+ value = get_unaligned_le16(&rx_buf[i * data_size + 1]);
+ value = sign_extend32(value, 15);
+ data->ch_data[i].raw = 0;
+ data->ch_data[i].bl = 0;
+ if (data->ch_data[i].sel_raw == true)
+ data->ch_data[i].raw = value;
+ if (data->ch_data[i].sel_bl == true)
+ data->ch_data[i].bl = value;
+ }
+
+ /* ch0~ch3 */
+ p = rx_buf;
+ size = (HX9023S_CH_NUM - 1) * data_size;
+ ret = regmap_bulk_read(data->regmap, HX9023S_LP_DIFF_CH0_0, p, size);
+ if (ret)
+ return ret;
+
+ /* ch4 */
+ p = rx_buf + size;
+ size = data_size;
+ ret = regmap_bulk_read(data->regmap, HX9023S_LP_DIFF_CH4_0, p, size);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < HX9023S_CH_NUM; i++) {
+ value = get_unaligned_le16(&rx_buf[i * data_size + 1]);
+ value = sign_extend32(value, 15);
+ data->ch_data[i].lp = 0;
+ data->ch_data[i].diff = 0;
+ if (data->ch_data[i].sel_lp == true)
+ data->ch_data[i].lp = value;
+ if (data->ch_data[i].sel_diff == true)
+ data->ch_data[i].diff = value;
+ }
+
+ for (i = 0; i < HX9023S_CH_NUM; i++) {
+ if (data->ch_data[i].sel_lp == true && data->ch_data[i].sel_bl == true)
+ data->ch_data[i].diff = data->ch_data[i].lp - data->ch_data[i].bl;
+ }
+
+ /* offset DAC */
+ offset_data_size = HX9023S_2BYTES;
+ p = rx_buf;
+ size = HX9023S_CH_NUM * offset_data_size;
+ ret = regmap_bulk_read(data->regmap, HX9023S_OFFSET_DAC0_7_0, p, size);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < HX9023S_CH_NUM; i++) {
+ value = get_unaligned_le16(&rx_buf[i * offset_data_size]);
+ value = FIELD_GET(GENMASK(11, 0), value);
+ data->ch_data[i].dac = value;
+ }
+
+ ret = hx9023s_data_lock(data, false);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int hx9023s_ch_en(struct hx9023s_data *data, u8 ch_id, bool en)
+{
+ int ret;
+ unsigned int buf;
+
+ ret = regmap_read(data->regmap, HX9023S_CH_NUM_CFG, &buf);
+ if (ret)
+ return ret;
+
+ data->ch_en_stat = buf;
+ if (en && data->ch_en_stat == 0)
+ data->prox_state_reg = 0;
+
+ data->ch_data[ch_id].enable = en;
+ __assign_bit(ch_id, &data->ch_en_stat, en);
+
+ return regmap_write(data->regmap, HX9023S_CH_NUM_CFG, data->ch_en_stat);
+}
+
+static int hx9023s_property_get(struct hx9023s_data *data)
+{
+ struct fwnode_handle *child;
+ struct device *dev = regmap_get_device(data->regmap);
+ int ret;
+ u32 i, reg, temp, array[2];
+
+ data->chan_in_use = 0;
+ for (i = 0; i < HX9023S_CH_NUM; i++) {
+ data->ch_data[i].channel_positive = HX9023S_NOT_CONNECTED;
+ data->ch_data[i].channel_negative = HX9023S_NOT_CONNECTED;
+ }
+
+ device_for_each_child_node(dev, child) {
+ ret = fwnode_property_read_u32(child, "reg", ®);
+ if (ret) {
+ fwnode_handle_put(child);
+ return dev_err_probe(dev, ret, "Failed to read reg\n");
+ }
+ __set_bit(reg, &data->chan_in_use);
+
+ if (fwnode_property_read_u32(child, "input-channel", &temp) == 0) {
+ data->ch_data[reg].channel_positive = temp;
+ data->ch_data[reg].channel_negative = HX9023S_NOT_CONNECTED;
+ } else if (fwnode_property_read_u32_array(child, "diff-channels",
+ array, sizeof(array)) == 0) {
+ data->ch_data[reg].channel_positive = array[0];
+ data->ch_data[reg].channel_negative = array[1];
+ } else {
+ fwnode_handle_put(child);
+ return dev_err_probe(dev, ret,
+ "Failed to read channel input for channel %d\n", reg);
+ }
+ }
+
+ return 0;
+}
+
+static int hx9023s_update_chan_en(struct hx9023s_data *data,
+ unsigned long chan_read,
+ unsigned long chan_event)
+{
+ unsigned int i;
+ unsigned long channels = chan_read | chan_event;
+
+ if ((data->chan_read | data->chan_event) != channels) {
+ for_each_set_bit(i, &channels, HX9023S_CH_NUM)
+ hx9023s_ch_en(data, i, test_bit(i, &data->chan_in_use));
+ for_each_clear_bit(i, &channels, HX9023S_CH_NUM)
+ hx9023s_ch_en(data, i, false);
+ }
+
+ data->chan_read = chan_read;
+ data->chan_event = chan_event;
+
+ return 0;
+}
+
+static int hx9023s_get_proximity(struct hx9023s_data *data,
+ const struct iio_chan_spec *chan,
+ int *val)
+{
+ int ret;
+
+ ret = hx9023s_sample(data);
+ if (ret)
+ return ret;
+
+ ret = hx9023s_get_prox_state(data);
+ if (ret)
+ return ret;
+
+ *val = data->ch_data[chan->channel].diff;
+ return IIO_VAL_INT;
+}
+
+static int hx9023s_get_samp_freq(struct hx9023s_data *data, int *val, int *val2)
+{
+ int ret;
+ unsigned int odr, index;
+
+ ret = regmap_read(data->regmap, HX9023S_PRF_CFG, &index);
+ if (ret)
+ return ret;
+
+ odr = hx9023s_samp_freq_table[index];
+ *val = KILO / odr;
+ *val2 = div_u64((KILO % odr) * MICRO, odr);
+
+ return IIO_VAL_INT_PLUS_MICRO;
+}
+
+static int hx9023s_read_raw(struct iio_dev *indio_dev, const struct iio_chan_spec *chan,
+ int *val, int *val2, long mask)
+{
+ struct hx9023s_data *data = iio_priv(indio_dev);
+ int ret;
+
+ if (chan->type != IIO_PROXIMITY)
+ return -EINVAL;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret)
+ return ret;
+
+ ret = hx9023s_get_proximity(data, chan, val);
+ iio_device_release_direct_mode(indio_dev);
+ return ret;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ return hx9023s_get_samp_freq(data, val, val2);
+ default:
+ return -EINVAL;
+ }
+}
+
+static int hx9023s_set_samp_freq(struct hx9023s_data *data, int val, int val2)
+{
+ struct device *dev = regmap_get_device(data->regmap);
+ unsigned int i, period_ms;
+
+ period_ms = div_u64(NANO, (val * MEGA + val2));
+
+ for (i = 0; i < ARRAY_SIZE(hx9023s_samp_freq_table); i++) {
+ if (period_ms == hx9023s_samp_freq_table[i])
+ break;
+ }
+ if (i == ARRAY_SIZE(hx9023s_samp_freq_table)) {
+ dev_err(dev, "Period:%dms NOT found!\n", period_ms);
+ return -EINVAL;
+ }
+
+ return regmap_write(data->regmap, HX9023S_PRF_CFG, i);
+}
+
+static int hx9023s_write_raw(struct iio_dev *indio_dev, const struct iio_chan_spec *chan,
+ int val, int val2, long mask)
+{
+ struct hx9023s_data *data = iio_priv(indio_dev);
+
+ if (chan->type != IIO_PROXIMITY)
+ return -EINVAL;
+
+ if (mask != IIO_CHAN_INFO_SAMP_FREQ)
+ return -EINVAL;
+
+ return hx9023s_set_samp_freq(data, val, val2);
+}
+
+static irqreturn_t hx9023s_irq_handler(int irq, void *private)
+{
+ struct iio_dev *indio_dev = private;
+ struct hx9023s_data *data = iio_priv(indio_dev);
+
+ if (data->trigger_enabled)
+ iio_trigger_poll(data->trig);
+
+ return IRQ_WAKE_THREAD;
+}
+
+static void hx9023s_push_events(struct iio_dev *indio_dev)
+{
+ struct hx9023s_data *data = iio_priv(indio_dev);
+ s64 timestamp = iio_get_time_ns(indio_dev);
+ unsigned long prox_changed;
+ unsigned int chan;
+
+ hx9023s_sample(data);
+ hx9023s_get_prox_state(data);
+
+ prox_changed = (data->chan_prox_stat ^ data->prox_state_reg) & data->chan_event;
+ for_each_set_bit(chan, &prox_changed, HX9023S_CH_NUM) {
+ unsigned int dir;
+
+ dir = (data->prox_state_reg & BIT(chan)) ? IIO_EV_DIR_FALLING : IIO_EV_DIR_RISING;
+
+ iio_push_event(indio_dev,
+ IIO_UNMOD_EVENT_CODE(IIO_PROXIMITY, chan, IIO_EV_TYPE_THRESH, dir),
+ timestamp);
+ }
+ data->chan_prox_stat = data->prox_state_reg;
+}
+
+static irqreturn_t hx9023s_irq_thread_handler(int irq, void *private)
+{
+ struct iio_dev *indio_dev = private;
+ struct hx9023s_data *data = iio_priv(indio_dev);
+
+ guard(mutex)(&data->mutex);
+ hx9023s_push_events(indio_dev);
+
+ return IRQ_HANDLED;
+}
+
+static int hx9023s_read_event_val(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info, int *val, int *val2)
+{
+ struct hx9023s_data *data = iio_priv(indio_dev);
+
+ if (chan->type != IIO_PROXIMITY)
+ return -EINVAL;
+
+ switch (info) {
+ case IIO_EV_INFO_VALUE:
+ switch (dir) {
+ case IIO_EV_DIR_RISING:
+ return hx9023s_get_thres_far(data, chan->channel, val);
+ case IIO_EV_DIR_FALLING:
+ return hx9023s_get_thres_near(data, chan->channel, val);
+ default:
+ return -EINVAL;
+ }
+ case IIO_EV_INFO_PERIOD:
+ switch (dir) {
+ case IIO_EV_DIR_RISING:
+ return hx9023s_read_far_debounce(data, val);
+ case IIO_EV_DIR_FALLING:
+ return hx9023s_read_near_debounce(data, val);
+ default:
+ return -EINVAL;
+ }
+ default:
+ return -EINVAL;
+ }
+}
+
+static int hx9023s_write_event_val(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info, int val, int val2)
+{
+ struct hx9023s_data *data = iio_priv(indio_dev);
+
+ if (chan->type != IIO_PROXIMITY)
+ return -EINVAL;
+
+ switch (info) {
+ case IIO_EV_INFO_VALUE:
+ switch (dir) {
+ case IIO_EV_DIR_RISING:
+ return hx9023s_set_thres_far(data, chan->channel, val);
+ case IIO_EV_DIR_FALLING:
+ return hx9023s_set_thres_near(data, chan->channel, val);
+ default:
+ return -EINVAL;
+ }
+ case IIO_EV_INFO_PERIOD:
+ switch (dir) {
+ case IIO_EV_DIR_RISING:
+ return hx9023s_write_far_debounce(data, val);
+ case IIO_EV_DIR_FALLING:
+ return hx9023s_write_near_debounce(data, val);
+ default:
+ return -EINVAL;
+ }
+ default:
+ return -EINVAL;
+ }
+}
+
+static int hx9023s_read_event_config(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir)
+{
+ struct hx9023s_data *data = iio_priv(indio_dev);
+
+ return test_bit(chan->channel, &data->chan_event);
+}
+
+static int hx9023s_write_event_config(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ int state)
+{
+ struct hx9023s_data *data = iio_priv(indio_dev);
+
+ if (test_bit(chan->channel, &data->chan_in_use)) {
+ hx9023s_ch_en(data, chan->channel, !!state);
+ __assign_bit(chan->channel, &data->chan_event, data->ch_data[chan->channel].enable);
+ }
+
+ return 0;
+}
+
+static const struct iio_info hx9023s_info = {
+ .read_raw = hx9023s_read_raw,
+ .write_raw = hx9023s_write_raw,
+ .read_event_value = hx9023s_read_event_val,
+ .write_event_value = hx9023s_write_event_val,
+ .read_event_config = hx9023s_read_event_config,
+ .write_event_config = hx9023s_write_event_config,
+};
+
+static int hx9023s_set_trigger_state(struct iio_trigger *trig, bool state)
+{
+ struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
+ struct hx9023s_data *data = iio_priv(indio_dev);
+
+ guard(mutex)(&data->mutex);
+ if (state)
+ hx9023s_interrupt_enable(data);
+ else if (!data->chan_read)
+ hx9023s_interrupt_disable(data);
+ data->trigger_enabled = state;
+
+ return 0;
+}
+
+static const struct iio_trigger_ops hx9023s_trigger_ops = {
+ .set_trigger_state = hx9023s_set_trigger_state,
+};
+
+static irqreturn_t hx9023s_trigger_handler(int irq, void *private)
+{
+ struct iio_poll_func *pf = private;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct hx9023s_data *data = iio_priv(indio_dev);
+ struct device *dev = regmap_get_device(data->regmap);
+ int ret;
+ unsigned int bit, index, i = 0;
+
+ guard(mutex)(&data->mutex);
+ ret = hx9023s_sample(data);
+ if (ret) {
+ dev_warn(dev, "sampling failed\n");
+ goto out;
+ }
+
+ ret = hx9023s_get_prox_state(data);
+ if (ret) {
+ dev_warn(dev, "get prox failed\n");
+ goto out;
+ }
+
+ for_each_set_bit(bit, indio_dev->active_scan_mask, indio_dev->masklength) {
+ index = indio_dev->channels[bit].channel;
+ data->buffer.channels[i++] = cpu_to_le16(data->ch_data[index].diff);
+ }
+
+ iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer, pf->timestamp);
+
+out:
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
+static int hx9023s_buffer_preenable(struct iio_dev *indio_dev)
+{
+ struct hx9023s_data *data = iio_priv(indio_dev);
+ unsigned long channels = 0;
+ unsigned int bit;
+
+ guard(mutex)(&data->mutex);
+ for_each_set_bit(bit, indio_dev->active_scan_mask, indio_dev->masklength)
+ __set_bit(indio_dev->channels[bit].channel, &channels);
+
+ hx9023s_update_chan_en(data, channels, data->chan_event);
+
+ return 0;
+}
+
+static int hx9023s_buffer_postdisable(struct iio_dev *indio_dev)
+{
+ struct hx9023s_data *data = iio_priv(indio_dev);
+
+ guard(mutex)(&data->mutex);
+ hx9023s_update_chan_en(data, 0, data->chan_event);
+
+ return 0;
+}
+
+static const struct iio_buffer_setup_ops hx9023s_buffer_setup_ops = {
+ .preenable = hx9023s_buffer_preenable,
+ .postdisable = hx9023s_buffer_postdisable,
+};
+
+static int hx9023s_id_check(struct iio_dev *indio_dev)
+{
+ struct hx9023s_data *data = iio_priv(indio_dev);
+ int ret;
+ unsigned int id;
+
+ ret = regmap_read(data->regmap, HX9023S_DEVICE_ID, &id);
+ if (ret)
+ return ret;
+
+ if (id == HX9023S_CHIP_ID) {
+ indio_dev->name = "hx9023s";
+ return 0;
+ }
+
+ return -ENODEV;
+}
+
+static int hx9023s_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct iio_dev *indio_dev;
+ struct hx9023s_data *data;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(struct hx9023s_data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ data = iio_priv(indio_dev);
+ mutex_init(&data->mutex);
+
+ data->regmap = devm_regmap_init_i2c(client, &hx9023s_regmap_config);
+ if (IS_ERR(data->regmap))
+ return dev_err_probe(dev, PTR_ERR(data->regmap), "regmap init failed\n");
+
+ ret = hx9023s_property_get(data);
+ if (ret)
+ return dev_err_probe(dev, ret, "dts phase failed\n");
+
+ ret = devm_regulator_get_enable(dev, "vdd");
+ if (ret)
+ return dev_err_probe(dev, ret, "regulator get failed\n");
+
+ ret = hx9023s_id_check(indio_dev);
+ if (ret)
+ return dev_err_probe(dev, ret, "id check failed\n");
+
+ indio_dev->channels = hx9023s_channels;
+ indio_dev->num_channels = ARRAY_SIZE(hx9023s_channels);
+ indio_dev->info = &hx9023s_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ i2c_set_clientdata(client, indio_dev);
+
+ ret = hx9023s_reg_init(data);
+ if (ret)
+ return dev_err_probe(dev, ret, "device init failed\n");
+
+ ret = hx9023s_ch_cfg(data);
+ if (ret)
+ return dev_err_probe(dev, ret, "channel config failed\n");
+
+ ret = regcache_sync(data->regmap);
+ if (ret)
+ return dev_err_probe(dev, ret, "regcache sync failed\n");
+
+ if (client->irq) {
+ ret = devm_request_threaded_irq(dev, client->irq, hx9023s_irq_handler,
+ hx9023s_irq_thread_handler, IRQF_ONESHOT,
+ "hx9023s_event", indio_dev);
+ if (ret)
+ return dev_err_probe(dev, ret, "irq request failed\n");
+
+ data->trig = devm_iio_trigger_alloc(dev, "%s-dev%d", indio_dev->name,
+ iio_device_id(indio_dev));
+ if (!data->trig)
+ return dev_err_probe(dev, -ENOMEM,
+ "iio trigger alloc failed\n");
+
+ data->trig->ops = &hx9023s_trigger_ops;
+ iio_trigger_set_drvdata(data->trig, indio_dev);
+
+ ret = devm_iio_trigger_register(dev, data->trig);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "iio trigger register failed\n");
+ }
+
+ ret = devm_iio_triggered_buffer_setup(dev, indio_dev, iio_pollfunc_store_time,
+ hx9023s_trigger_handler, &hx9023s_buffer_setup_ops);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "iio triggered buffer setup failed\n");
+
+ ret = devm_iio_device_register(dev, indio_dev);
+ if (ret)
+ return dev_err_probe(dev, ret, "iio device register failed\n");
+
+ return 0;
+}
+
+static int hx9023s_suspend(struct device *dev)
+{
+ struct hx9023s_data *data = iio_priv(dev_get_drvdata(dev));
+
+ hx9023s_interrupt_disable(data);
+
+ return 0;
+}
+
+static int hx9023s_resume(struct device *dev)
+{
+ struct hx9023s_data *data = iio_priv(dev_get_drvdata(dev));
+
+ if (data->trigger_enabled)
+ hx9023s_interrupt_enable(data);
+
+ return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(hx9023s_pm_ops, hx9023s_suspend, hx9023s_resume);
+
+static const struct of_device_id hx9023s_of_match[] = {
+ { .compatible = "tyhx,hx9023s" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, hx9023s_of_match);
+
+static const struct i2c_device_id hx9023s_id[] = {
+ { "hx9023s" },
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, hx9023s_id);
+
+static struct i2c_driver hx9023s_driver = {
+ .driver = {
+ .name = "hx9023s",
+ .of_match_table = hx9023s_of_match,
+ .pm = &hx9023s_pm_ops,
+
+ /*
+ * The I2C operations in hx9023s_reg_init() and hx9023s_ch_cfg()
+ * are time-consuming. Prefer async so we don't delay boot
+ * if we're builtin to the kernel.
+ */
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ },
+ .probe = hx9023s_probe,
+ .id_table = hx9023s_id,
+};
+module_i2c_driver(hx9023s_driver);
+
+MODULE_AUTHOR("Yasin Lee <yasin.lee.x@gmail.com>");
+MODULE_DESCRIPTION("Driver for TYHX HX9023S SAR sensor");
+MODULE_LICENSE("GPL");
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v6 1/3] dt-bindings: vendor-prefixes: add tyhx
2024-06-21 7:40 ` [PATCH v6 1/3] dt-bindings: vendor-prefixes: add tyhx Yasin Lee
@ 2024-06-21 10:07 ` Krzysztof Kozlowski
0 siblings, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-06-21 10:07 UTC (permalink / raw)
To: Yasin Lee, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Cameron, Lars-Peter Clausen, yasin.lee.x
Cc: devicetree, linux-kernel, linux-iio
On 21/06/2024 09:40, Yasin Lee wrote:
> Add vendor prefix for NanjingTianyihexin Electronics Ltd.
> http://www.tianyihexin.com
>
> Signed-off-by: Yasin Lee <yasin.lee.x@gmail.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
<form letter>
This is an automated instruction, just in case, because many review tags
are being ignored. If you know the process, you can skip it (please do
not feel offended by me posting it here - no bad intentions intended).
If you do not know the process, here is a short explanation:
Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.
https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577
</form letter>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 2/3] dt-bindings: iio: proximity: Add TYHX HX9023S
2024-06-21 7:40 ` [PATCH v6 2/3] dt-bindings: iio: proximity: Add TYHX HX9023S Yasin Lee
@ 2024-06-21 10:12 ` Krzysztof Kozlowski
2024-06-22 5:56 ` Yasin Lee
0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-06-21 10:12 UTC (permalink / raw)
To: Yasin Lee, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Cameron, Lars-Peter Clausen, yasin.lee.x
Cc: devicetree, linux-kernel, linux-iio
On 21/06/2024 09:40, Yasin Lee wrote:
> A capacitive proximity sensor
>
> Signed-off-by: Yasin Lee <yasin.lee.x@gmail.com>
> ---
> .../bindings/iio/proximity/tyhx,hx9023s.yaml | 115 +++++++++++++++++++++
> 1 file changed, 115 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml b/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
> new file mode 100644
> index 000000000000..beca70ce7609
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
> @@ -0,0 +1,115 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/proximity/tyhx,hx9023s.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TYHX HX9023S capacitive proximity sensor
> +
> +maintainers:
> + - Yasin Lee <yasin.lee.x@gmail.com>
> +
> +description: |
> + TYHX HX9023S proximity sensor. Datasheet can be found here:
> + http://www.tianyihexin.com/ueditor/php/upload/file/20240614/1718336303992081.pdf
> +
> +allOf:
> + - $ref: /schemas/iio/iio.yaml#
Which part of iio.yaml binding do you use here? I cannot find anything,
so this looks wrong.
> +
> +properties:
> + compatible:
> + const: tyhx,hx9023s
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + description:
> + Generated by device to announce preceding read request has finished
> + and data is available or that a close/far proximity event has happened.
> + maxItems: 1
> +
> + vdd-supply: true
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> +patternProperties:
> + "^channel@[0-4]$":
> + $ref: /schemas/iio/adc/adc.yaml
> + type: object
> +
> + properties:
> + reg:
> + minimum: 0
> + maximum: 4
> + description: The channel number.
> +
> + input-channel:
Isn't this duplicating single-channel property?
Where is this property defined (which common schema)?
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 4
> + description:
> + Specify the input pin used in single-ended configuration.
> +
> + diff-channels: true
> +
> + oneOf:
> + - required:
> + - input-channel
> + - required:
> + - diff-channels
> +
> + required:
> + - reg
> +
> + additionalProperties: false
> +
> +required:
> + - compatible
> + - vdd-supply
> + - reg
Keep the same order as in properties.
> +
> +unevaluatedProperties: false
> +
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 3/3] iio: proximity: Add driver support for TYHX's HX9023S capacitive proximity sensor
2024-06-21 7:40 ` [PATCH v6 3/3] iio: proximity: Add driver support for TYHX's HX9023S capacitive proximity sensor Yasin Lee
@ 2024-06-21 14:09 ` Alexandru Ardelean
2024-06-22 12:15 ` Yasin Lee
2024-06-23 11:56 ` Jonathan Cameron
1 sibling, 1 reply; 15+ messages in thread
From: Alexandru Ardelean @ 2024-06-21 14:09 UTC (permalink / raw)
To: Yasin Lee
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
Lars-Peter Clausen, yasin.lee.x, devicetree, linux-kernel,
linux-iio
On Fri, Jun 21, 2024 at 10:44 AM Yasin Lee <yasin.lee.x@gmail.com> wrote:
>
> A SAR sensor from NanjingTianyihexin Electronics Ltd.
>
> The device has the following entry points:
>
> Usual frequency:
> - sampling_frequency
>
> Instant reading of current values for different sensors:
> - in_proximity0_raw
> - in_proximity1_raw
> - in_proximity2_raw
> - in_proximity3_raw
> - in_proximity4_raw
> and associated events in events/
>
Hello :)
> Signed-off-by: Yasin Lee <yasin.lee.x@gmail.com>
> ---
> drivers/iio/proximity/Kconfig | 14 +
> drivers/iio/proximity/Makefile | 1 +
> drivers/iio/proximity/hx9023s.c | 1150 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 1165 insertions(+)
>
> diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
> index 2ca3b0bc5eba..0694f625b432 100644
> --- a/drivers/iio/proximity/Kconfig
> +++ b/drivers/iio/proximity/Kconfig
> @@ -32,6 +32,20 @@ config CROS_EC_MKBP_PROXIMITY
> To compile this driver as a module, choose M here: the
> module will be called cros_ec_mkbp_proximity.
>
> +config HX9023S
> + tristate "TYHX HX9023S SAR sensor"
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + select REGMAP_I2C
> + depends on I2C
> + help
> + Say Y here to build a driver for TYHX HX9023S capacitive SAR sensor.
> + This driver supports the TYHX HX9023S capacitive
> + SAR sensors. This sensors is used for proximity detection applications.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called hx9023s.
> +
> config IRSD200
> tristate "Murata IRS-D200 PIR sensor"
> select IIO_BUFFER
> diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
> index f36598380446..ab381cd27dbb 100644
> --- a/drivers/iio/proximity/Makefile
> +++ b/drivers/iio/proximity/Makefile
> @@ -6,6 +6,7 @@
> # When adding new entries keep the list in alphabetical order
> obj-$(CONFIG_AS3935) += as3935.o
> obj-$(CONFIG_CROS_EC_MKBP_PROXIMITY) += cros_ec_mkbp_proximity.o
> +obj-$(CONFIG_HX9023S) += hx9023s.o
> obj-$(CONFIG_IRSD200) += irsd200.o
> obj-$(CONFIG_ISL29501) += isl29501.o
> obj-$(CONFIG_LIDAR_LITE_V2) += pulsedlight-lidar-lite-v2.o
> diff --git a/drivers/iio/proximity/hx9023s.c b/drivers/iio/proximity/hx9023s.c
> new file mode 100644
> index 000000000000..1d8cb9a05d8a
> --- /dev/null
> +++ b/drivers/iio/proximity/hx9023s.c
> @@ -0,0 +1,1150 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2024 NanjingTianyihexin Electronics Ltd.
> + * http://www.tianyihexin.com
> + *
> + * Driver for NanjingTianyihexin HX9023S Cap Sensor.
> + * Datasheet available at:
> + * http://www.tianyihexin.com/ueditor/php/upload/file/20240614/1718336303992081.pdf
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/irqreturn.h>
> +#include <linux/math.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/pm.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/types.h>
> +#include <linux/units.h>
> +
> +#include <asm/byteorder.h>
> +#include <asm/unaligned.h>
> +
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/events.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/types.h>
A first question is: are all these headers required?
Looks like some of them could be removed.
> +
> +#define HX9023S_CHIP_ID 0x1D
> +#define HX9023S_CH_NUM 5
> +#define HX9023S_2BYTES 2
> +#define HX9023S_3BYTES 3
> +#define HX9023S_BYTES_MAX HX9023S_3BYTES
> +#define HX9023S_POS 0x03
> +#define HX9023S_NEG 0x02
> +#define HX9023S_NOT_CONNECTED 16
> +
> +#define HX9023S_GLOBAL_CTRL0 0x00
> +#define HX9023S_PRF_CFG 0x02
> +#define HX9023S_CH0_CFG_7_0 0x03
> +#define HX9023S_CH4_CFG_9_8 0x0C
> +#define HX9023S_RANGE_7_0 0x0D
> +#define HX9023S_RANGE_9_8 0x0E
> +#define HX9023S_RANGE_18_16 0x0F
> +#define HX9023S_AVG0_NOSR0_CFG 0x10
> +#define HX9023S_NOSR12_CFG 0x11
> +#define HX9023S_NOSR34_CFG 0x12
> +#define HX9023S_AVG12_CFG 0x13
> +#define HX9023S_AVG34_CFG 0x14
> +#define HX9023S_OFFSET_DAC0_7_0 0x15
> +#define HX9023S_OFFSET_DAC4_9_8 0x1E
> +#define HX9023S_SAMPLE_NUM_7_0 0x1F
> +#define HX9023S_INTEGRATION_NUM_7_0 0x21
> +#define HX9023S_CH_NUM_CFG 0x24
> +#define HX9023S_LP_ALP_4_CFG 0x29
> +#define HX9023S_LP_ALP_1_0_CFG 0x2A
> +#define HX9023S_LP_ALP_3_2_CFG 0x2B
> +#define HX9023S_UP_ALP_1_0_CFG 0x2C
> +#define HX9023S_UP_ALP_3_2_CFG 0x2D
> +#define HX9023S_DN_UP_ALP_0_4_CFG 0x2E
> +#define HX9023S_DN_ALP_2_1_CFG 0x2F
> +#define HX9023S_DN_ALP_4_3_CFG 0x30
> +#define HX9023S_RAW_BL_RD_CFG 0x38
> +#define HX9023S_INTERRUPT_CFG 0x39
> +#define HX9023S_INTERRUPT_CFG1 0x3A
> +#define HX9023S_CALI_DIFF_CFG 0x3B
> +#define HX9023S_DITHER_CFG 0x3C
> +#define HX9023S_DEVICE_ID 0x60
> +#define HX9023S_PROX_STATUS 0x6B
> +#define HX9023S_PROX_INT_HIGH_CFG 0x6C
> +#define HX9023S_PROX_INT_LOW_CFG 0x6D
> +#define HX9023S_PROX_HIGH_DIFF_CFG_CH0_0 0x80
> +#define HX9023S_PROX_LOW_DIFF_CFG_CH0_0 0x88
> +#define HX9023S_PROX_LOW_DIFF_CFG_CH3_1 0x8F
> +#define HX9023S_PROX_HIGH_DIFF_CFG_CH4_0 0x9E
> +#define HX9023S_PROX_HIGH_DIFF_CFG_CH4_1 0x9F
> +#define HX9023S_PROX_LOW_DIFF_CFG_CH4_0 0xA2
> +#define HX9023S_PROX_LOW_DIFF_CFG_CH4_1 0xA3
> +#define HX9023S_CAP_INI_CH4_0 0xB3
> +#define HX9023S_LP_DIFF_CH4_2 0xBA
> +#define HX9023S_RAW_BL_CH4_0 0xB5
> +#define HX9023S_LP_DIFF_CH4_0 0xB8
> +#define HX9023S_DSP_CONFIG_CTRL1 0xC8
> +#define HX9023S_CAP_INI_CH0_0 0xE0
> +#define HX9023S_RAW_BL_CH0_0 0xE8
> +#define HX9023S_LP_DIFF_CH0_0 0xF4
> +#define HX9023S_LP_DIFF_CH3_2 0xFF
> +
> +#define HX9023S_DATA_LOCK_MASK BIT(4)
> +#define HX9023S_INTERRUPT_MASK GENMASK(9, 0)
> +#define HX9023S_PROX_DEBOUNCE_MASK GENMASK(3, 0)
> +
> +struct hx9023s_addr_val_pair {
> + u8 addr;
> + u8 val;
> +};
This looks like:
struct reg_sequence {
unsigned int reg;
unsigned int def;
unsigned int delay_us;
};
This is defined in include/linux/regmap.h
> +
> +struct hx9023s_ch_data {
> + int raw; /* Raw Data*/
> + int lp; /* Low Pass Filter Data*/
> + int bl; /* Base Line Data */
> + int diff; /* difference of Low Pass Data and Base Line Data */
> +
> + struct {
> + unsigned int near;
> + unsigned int far;
> + } thres;
> +
> + u16 dac;
> + u8 channel_positive;
> + u8 channel_negative;
> + bool sel_bl;
> + bool sel_raw;
> + bool sel_diff;
> + bool sel_lp;
> + bool enable;
> +};
> +
> +struct hx9023s_data {
> + struct iio_trigger *trig;
> + struct regmap *regmap;
> + unsigned long chan_prox_stat;
> + unsigned long chan_read;
> + unsigned long chan_event;
> + unsigned long ch_en_stat;
> + unsigned long chan_in_use;
> + unsigned int prox_state_reg;
> + bool trigger_enabled;
> +
> + struct {
> + __le16 channels[HX9023S_CH_NUM];
> + s64 ts __aligned(8);
> + } buffer;
> +
> + struct mutex mutex;
> + struct hx9023s_ch_data ch_data[HX9023S_CH_NUM];
> +};
> +
> +static struct hx9023s_addr_val_pair hx9023s_reg_init_list[] = {
Globals like this should be `static const`
Also, it would be a good idea to define this as `static const struct
reg_sequence `
Then the `regmap_multi_reg_write()` function could be used.
> + /* scan period */
> + { HX9023S_PRF_CFG, 0x17 },
> +
> + /* full scale of conversion phase of each channel */
> + { HX9023S_RANGE_7_0, 0x11 },
> + { HX9023S_RANGE_9_8, 0x02 },
> + { HX9023S_RANGE_18_16, 0x00 },
> +
> + /* ADC average number and OSR number of each channel */
> + { HX9023S_AVG0_NOSR0_CFG, 0x71 },
> + { HX9023S_NOSR12_CFG, 0x44 },
> + { HX9023S_NOSR34_CFG, 0x00 },
> + { HX9023S_AVG12_CFG, 0x33 },
> + { HX9023S_AVG34_CFG, 0x00 },
> +
> + /* sample & integration frequency of the ADC */
> + { HX9023S_SAMPLE_NUM_7_0, 0x65 },
> + { HX9023S_INTEGRATION_NUM_7_0, 0x65 },
> +
> + /* coefficient of the first order low pass filter during each channel */
> + { HX9023S_LP_ALP_1_0_CFG, 0x22 },
> + { HX9023S_LP_ALP_3_2_CFG, 0x22 },
> + { HX9023S_LP_ALP_4_CFG, 0x02 },
> +
> + /* up coefficient of the first order low pass filter during each channel */
> + { HX9023S_UP_ALP_1_0_CFG, 0x88 },
> + { HX9023S_UP_ALP_3_2_CFG, 0x88 },
> + { HX9023S_DN_UP_ALP_0_4_CFG, 0x18 },
> +
> + /* down coefficient of the first order low pass filter during each channel */
> + { HX9023S_DN_ALP_2_1_CFG, 0x11 },
> + { HX9023S_DN_ALP_4_3_CFG, 0x11 },
> +
> + /* selection of data for the Data Mux Register to output data */
> + { HX9023S_RAW_BL_RD_CFG, 0xF0 },
> +
> + /* enable the interrupt function */
> + { HX9023S_INTERRUPT_CFG, 0xFF },
> + { HX9023S_INTERRUPT_CFG1, 0x3B },
> + { HX9023S_DITHER_CFG, 0x21 },
> +
> + /* threshold of the offset compensation */
> + { HX9023S_CALI_DIFF_CFG, 0x07 },
> +
> + /* proximity persistency number(near & far) */
> + { HX9023S_PROX_INT_HIGH_CFG, 0x01 },
> + { HX9023S_PROX_INT_LOW_CFG, 0x01 },
> +
> + /* disable the data lock */
> + { HX9023S_DSP_CONFIG_CTRL1, 0x00 },
> +};
> +
> +static const struct iio_event_spec hx9023s_events[] = {
> + {
> + .type = IIO_EV_TYPE_THRESH,
> + .dir = IIO_EV_DIR_RISING,
> + .mask_shared_by_all = BIT(IIO_EV_INFO_PERIOD),
> + .mask_separate = BIT(IIO_EV_INFO_VALUE),
> + },
> + {
> + .type = IIO_EV_TYPE_THRESH,
> + .dir = IIO_EV_DIR_FALLING,
> + .mask_shared_by_all = BIT(IIO_EV_INFO_PERIOD),
> + .mask_separate = BIT(IIO_EV_INFO_VALUE),
> +
> + },
> + {
> + .type = IIO_EV_TYPE_THRESH,
> + .dir = IIO_EV_DIR_EITHER,
> + .mask_separate = BIT(IIO_EV_INFO_ENABLE),
> + },
> +};
> +
> +#define HX9023S_CHANNEL(idx) \
> +{ \
> + .type = IIO_PROXIMITY, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
> + .indexed = 1, \
> + .channel = idx, \
> + .address = 0, \
> + .event_spec = hx9023s_events, \
> + .num_event_specs = ARRAY_SIZE(hx9023s_events), \
> + .scan_index = idx, \
> + .scan_type = { \
> + .sign = 's', \
> + .realbits = 16, \
> + .storagebits = 16, \
> + .endianness = IIO_BE, \
> + }, \
> +}
> +
> +static const struct iio_chan_spec hx9023s_channels[] = {
> + HX9023S_CHANNEL(0),
> + HX9023S_CHANNEL(1),
> + HX9023S_CHANNEL(2),
> + HX9023S_CHANNEL(3),
> + HX9023S_CHANNEL(4),
> + IIO_CHAN_SOFT_TIMESTAMP(5),
> +};
> +
> +static const unsigned int hx9023s_samp_freq_table[] = {
> + 2, 2, 4, 6, 8, 10, 14, 18, 22, 26,
> + 30, 34, 38, 42, 46, 50, 56, 62, 68, 74,
> + 80, 90, 100, 200, 300, 400, 600, 800, 1000, 2000,
> + 3000, 4000,
> +};
> +
> +static const struct regmap_range hx9023s_rd_reg_ranges[] = {
> + regmap_reg_range(HX9023S_GLOBAL_CTRL0, HX9023S_LP_DIFF_CH3_2),
> +};
> +
> +static const struct regmap_range hx9023s_wr_reg_ranges[] = {
> + regmap_reg_range(HX9023S_GLOBAL_CTRL0, HX9023S_LP_DIFF_CH3_2),
> +};
> +
> +static const struct regmap_range hx9023s_volatile_reg_ranges[] = {
> + regmap_reg_range(HX9023S_CAP_INI_CH4_0, HX9023S_LP_DIFF_CH4_2),
> + regmap_reg_range(HX9023S_CAP_INI_CH0_0, HX9023S_LP_DIFF_CH3_2),
> + regmap_reg_range(HX9023S_PROX_STATUS, HX9023S_PROX_STATUS),
> +};
> +
> +static const struct regmap_access_table hx9023s_rd_regs = {
> + .yes_ranges = hx9023s_rd_reg_ranges,
> + .n_yes_ranges = ARRAY_SIZE(hx9023s_rd_reg_ranges),
> +};
> +
> +static const struct regmap_access_table hx9023s_wr_regs = {
> + .yes_ranges = hx9023s_wr_reg_ranges,
> + .n_yes_ranges = ARRAY_SIZE(hx9023s_wr_reg_ranges),
> +};
> +
> +static const struct regmap_access_table hx9023s_volatile_regs = {
> + .yes_ranges = hx9023s_volatile_reg_ranges,
> + .n_yes_ranges = ARRAY_SIZE(hx9023s_volatile_reg_ranges),
> +};
> +
> +static const struct regmap_config hx9023s_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .cache_type = REGCACHE_MAPLE,
> + .rd_table = &hx9023s_rd_regs,
> + .wr_table = &hx9023s_wr_regs,
> + .volatile_table = &hx9023s_volatile_regs,
> +};
> +
> +static int hx9023s_interrupt_enable(struct hx9023s_data *data)
> +{
> + return regmap_update_bits(data->regmap, HX9023S_INTERRUPT_CFG,
> + HX9023S_INTERRUPT_MASK, HX9023S_INTERRUPT_MASK);
> +}
> +
> +static int hx9023s_interrupt_disable(struct hx9023s_data *data)
> +{
> + return regmap_update_bits(data->regmap, HX9023S_INTERRUPT_CFG,
> + HX9023S_INTERRUPT_MASK, 0x00);
> +}
> +
> +static int hx9023s_data_lock(struct hx9023s_data *data, bool locked)
> +{
> + if (locked)
> + return regmap_update_bits(data->regmap, HX9023S_DSP_CONFIG_CTRL1,
> + HX9023S_DATA_LOCK_MASK, HX9023S_DATA_LOCK_MASK);
> + else
> + return regmap_update_bits(data->regmap, HX9023S_DSP_CONFIG_CTRL1,
> + HX9023S_DATA_LOCK_MASK, 0);
> +}
> +
> +static int hx9023s_ch_cfg(struct hx9023s_data *data)
> +{
> + unsigned int i;
> + u16 reg;
> + u8 reg_list[HX9023S_CH_NUM * 2];
> + u8 ch_pos[HX9023S_CH_NUM];
> + u8 ch_neg[HX9023S_CH_NUM];
> + /* Bit positions corresponding to input pin connections */
> + u8 conn_cs[HX9023S_CH_NUM] = {0, 2, 4, 6, 8};
> +
> + for (i = 0; i < HX9023S_CH_NUM; i++) {
See comment [1]
> + ch_pos[i] = data->ch_data[i].channel_positive == HX9023S_NOT_CONNECTED ?
> + HX9023S_NOT_CONNECTED : conn_cs[data->ch_data[i].channel_positive];
> + ch_neg[i] = data->ch_data[i].channel_negative == HX9023S_NOT_CONNECTED ?
> + HX9023S_NOT_CONNECTED : conn_cs[data->ch_data[i].channel_negative];
> +
> + reg = (HX9023S_POS << ch_pos[i]) | (HX9023S_NEG << ch_neg[i]);
> + put_unaligned_le16(reg, ®_list[i * 2]);
> + }
> +
> + return regmap_bulk_write(data->regmap, HX9023S_CH0_CFG_7_0, reg_list, HX9023S_CH_NUM * 2);
> +}
> +
> +static int hx9023s_reg_init(struct hx9023s_data *data)
> +{
> + unsigned int i = 0;
> + int ret;
> +
> + for (i = 0; i < (int)ARRAY_SIZE(hx9023s_reg_init_list); i++) {
> + ret = regmap_write(data->regmap, hx9023s_reg_init_list[i].addr,
> + hx9023s_reg_init_list[i].val);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int hx9023s_write_far_debounce(struct hx9023s_data *data, int val)
> +{
> + guard(mutex)(&data->mutex);
> + return regmap_update_bits(data->regmap, HX9023S_PROX_INT_LOW_CFG,
> + HX9023S_PROX_DEBOUNCE_MASK,
> + FIELD_GET(HX9023S_PROX_DEBOUNCE_MASK, val));
> +}
> +
> +static int hx9023s_write_near_debounce(struct hx9023s_data *data, int val)
> +{
> + guard(mutex)(&data->mutex);
> + return regmap_update_bits(data->regmap, HX9023S_PROX_INT_HIGH_CFG,
> + HX9023S_PROX_DEBOUNCE_MASK,
> + FIELD_GET(HX9023S_PROX_DEBOUNCE_MASK, val));
> +}
> +
> +static int hx9023s_read_far_debounce(struct hx9023s_data *data, int *val)
> +{
> + int ret;
> +
> + ret = regmap_read(data->regmap, HX9023S_PROX_INT_LOW_CFG, val);
> + if (ret)
> + return ret;
> +
> + *val = FIELD_GET(HX9023S_PROX_DEBOUNCE_MASK, *val);
> +
> + return IIO_VAL_INT;
> +}
> +
> +static int hx9023s_read_near_debounce(struct hx9023s_data *data, int *val)
> +{
> + int ret;
> +
> + ret = regmap_read(data->regmap, HX9023S_PROX_INT_HIGH_CFG, val);
> + if (ret)
> + return ret;
> +
> + *val = FIELD_GET(HX9023S_PROX_DEBOUNCE_MASK, *val);
> +
> + return IIO_VAL_INT;
> +}
> +
> +static int hx9023s_get_thres_near(struct hx9023s_data *data, u8 ch, int *val)
> +{
> + int ret;
> + u8 buf[2];
> + unsigned int reg, tmp;
> +
> + reg = (ch == 4) ? HX9023S_PROX_HIGH_DIFF_CFG_CH4_0 :
> + HX9023S_PROX_HIGH_DIFF_CFG_CH0_0 + (ch * HX9023S_2BYTES);
> +
> + ret = regmap_bulk_read(data->regmap, reg, buf, sizeof(buf));
> + if (ret)
> + return ret;
> +
> + tmp = (get_unaligned_le16(buf) & GENMASK(9, 0)) * 32;
> + data->ch_data[ch].thres.near = tmp;
> + *val = tmp;
> +
> + return IIO_VAL_INT;
> +}
> +
> +static int hx9023s_get_thres_far(struct hx9023s_data *data, u8 ch, int *val)
> +{
> + int ret;
> + u8 buf[2];
> + unsigned int reg, tmp;
> +
> + reg = (ch == 4) ? HX9023S_PROX_LOW_DIFF_CFG_CH4_0 :
> + HX9023S_PROX_LOW_DIFF_CFG_CH0_0 + (ch * HX9023S_2BYTES);
> +
> + ret = regmap_bulk_read(data->regmap, reg, buf, sizeof(buf));
> + if (ret)
> + return ret;
> +
> + tmp = (get_unaligned_le16(buf) & GENMASK(9, 0)) * 32;
> + data->ch_data[ch].thres.far = tmp;
> + *val = tmp;
> +
> + return IIO_VAL_INT;
> +}
> +
> +static int hx9023s_set_thres_near(struct hx9023s_data *data, u8 ch, int val)
> +{
> + __le16 val_le16 = cpu_to_le16((val / 32) & GENMASK(9, 0));
> + unsigned int reg;
> +
> + data->ch_data[ch].thres.near = ((val / 32) & GENMASK(9, 0)) * 32;
> + reg = (ch == 4) ? HX9023S_PROX_HIGH_DIFF_CFG_CH4_0 :
> + HX9023S_PROX_HIGH_DIFF_CFG_CH0_0 + (ch * HX9023S_2BYTES);
> +
> + return regmap_bulk_write(data->regmap, reg, &val_le16, sizeof(val_le16));
> +}
> +
> +static int hx9023s_set_thres_far(struct hx9023s_data *data, u8 ch, int val)
> +{
> + __le16 val_le16 = cpu_to_le16((val / 32) & GENMASK(9, 0));
> + unsigned int reg;
> +
> + data->ch_data[ch].thres.far = ((val / 32) & GENMASK(9, 0)) * 32;
> + reg = (ch == 4) ? HX9023S_PROX_LOW_DIFF_CFG_CH4_0 :
> + HX9023S_PROX_LOW_DIFF_CFG_CH0_0 + (ch * HX9023S_2BYTES);
> +
> + return regmap_bulk_write(data->regmap, reg, &val_le16, sizeof(val_le16));
> +}
> +
> +static int hx9023s_get_prox_state(struct hx9023s_data *data)
> +{
> + return regmap_read(data->regmap, HX9023S_PROX_STATUS, &data->prox_state_reg);
> +}
> +
> +static int hx9023s_data_select(struct hx9023s_data *data)
> +{
> + int ret;
> + unsigned int i, buf;
> + unsigned long tmp;
> +
> + ret = regmap_read(data->regmap, HX9023S_RAW_BL_RD_CFG, &buf);
> + if (ret)
> + return ret;
> +
> + tmp = buf;
> + for (i = 0; i < 4; i++) {
> + data->ch_data[i].sel_diff = test_bit(i, &tmp);
> + data->ch_data[i].sel_lp = !data->ch_data[i].sel_diff;
> + data->ch_data[i].sel_bl = test_bit(i + 4, &tmp);
> + data->ch_data[i].sel_raw = !data->ch_data[i].sel_bl;
> + }
> +
> + ret = regmap_read(data->regmap, HX9023S_INTERRUPT_CFG1, &buf);
> + if (ret)
> + return ret;
> +
> + tmp = buf;
> + data->ch_data[4].sel_diff = test_bit(2, &tmp);
> + data->ch_data[4].sel_lp = !data->ch_data[4].sel_diff;
> + data->ch_data[4].sel_bl = test_bit(3, &tmp);
> + data->ch_data[4].sel_raw = !data->ch_data[4].sel_bl;
> +
> + return 0;
> +}
> +
> +static int hx9023s_sample(struct hx9023s_data *data)
> +{
> + int ret, value;
> + unsigned int i;
> + u8 data_size, offset_data_size, *p, size, rx_buf[HX9023S_CH_NUM * HX9023S_BYTES_MAX];
> +
> + ret = hx9023s_data_lock(data, true);
> + if (ret)
> + return ret;
> +
> + ret = hx9023s_data_select(data);
> + if (ret)
> + return ret;
From here onwards, it looks like if there is an error, then
`hx9023s_data_lock(data, false)` does not get called.
Is that expected?
Maybe some `goto err` statements would be needed?
> +
> + data_size = HX9023S_3BYTES;
> +
> + /* ch0~ch3 */
> + p = rx_buf;
> + size = (HX9023S_CH_NUM - 1) * data_size;
> + ret = regmap_bulk_read(data->regmap, HX9023S_RAW_BL_CH0_0, p, size);
> + if (ret)
> + return ret;
> +
> + /* ch4 */
> + p = rx_buf + size;
> + size = data_size;
> + ret = regmap_bulk_read(data->regmap, HX9023S_RAW_BL_CH4_0, p, size);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < HX9023S_CH_NUM; i++) {
[1]
Maybe use some per-device (example: indio_dev->num_channels) here
(instead of HX9023S_CH_NUM)?
If adding support for a part with fewer channels, this would crash.
This comment is for all places where for (i = 0; i < HX9023S_CH_NUM;
i++) is used
> + value = get_unaligned_le16(&rx_buf[i * data_size + 1]);
> + value = sign_extend32(value, 15);
> + data->ch_data[i].raw = 0;
> + data->ch_data[i].bl = 0;
> + if (data->ch_data[i].sel_raw == true)
> + data->ch_data[i].raw = value;
> + if (data->ch_data[i].sel_bl == true)
> + data->ch_data[i].bl = value;
> + }
> +
> + /* ch0~ch3 */
> + p = rx_buf;
> + size = (HX9023S_CH_NUM - 1) * data_size;
> + ret = regmap_bulk_read(data->regmap, HX9023S_LP_DIFF_CH0_0, p, size);
> + if (ret)
> + return ret;
> +
> + /* ch4 */
> + p = rx_buf + size;
> + size = data_size;
> + ret = regmap_bulk_read(data->regmap, HX9023S_LP_DIFF_CH4_0, p, size);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < HX9023S_CH_NUM; i++) {
See comment [1]
> + value = get_unaligned_le16(&rx_buf[i * data_size + 1]);
> + value = sign_extend32(value, 15);
> + data->ch_data[i].lp = 0;
> + data->ch_data[i].diff = 0;
> + if (data->ch_data[i].sel_lp == true)
> + data->ch_data[i].lp = value;
> + if (data->ch_data[i].sel_diff == true)
> + data->ch_data[i].diff = value;
> + }
> +
> + for (i = 0; i < HX9023S_CH_NUM; i++) {
See comment [1]
> + if (data->ch_data[i].sel_lp == true && data->ch_data[i].sel_bl == true)
> + data->ch_data[i].diff = data->ch_data[i].lp - data->ch_data[i].bl;
> + }
> +
> + /* offset DAC */
> + offset_data_size = HX9023S_2BYTES;
> + p = rx_buf;
> + size = HX9023S_CH_NUM * offset_data_size;
> + ret = regmap_bulk_read(data->regmap, HX9023S_OFFSET_DAC0_7_0, p, size);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < HX9023S_CH_NUM; i++) {
See comment [1]
> + value = get_unaligned_le16(&rx_buf[i * offset_data_size]);
> + value = FIELD_GET(GENMASK(11, 0), value);
> + data->ch_data[i].dac = value;
> + }
> +
> + ret = hx9023s_data_lock(data, false);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int hx9023s_ch_en(struct hx9023s_data *data, u8 ch_id, bool en)
> +{
> + int ret;
> + unsigned int buf;
> +
> + ret = regmap_read(data->regmap, HX9023S_CH_NUM_CFG, &buf);
> + if (ret)
> + return ret;
> +
> + data->ch_en_stat = buf;
> + if (en && data->ch_en_stat == 0)
> + data->prox_state_reg = 0;
> +
> + data->ch_data[ch_id].enable = en;
> + __assign_bit(ch_id, &data->ch_en_stat, en);
> +
> + return regmap_write(data->regmap, HX9023S_CH_NUM_CFG, data->ch_en_stat);
> +}
> +
> +static int hx9023s_property_get(struct hx9023s_data *data)
> +{
> + struct fwnode_handle *child;
> + struct device *dev = regmap_get_device(data->regmap);
> + int ret;
> + u32 i, reg, temp, array[2];
> +
> + data->chan_in_use = 0;
> + for (i = 0; i < HX9023S_CH_NUM; i++) {
See comment [1]
> + data->ch_data[i].channel_positive = HX9023S_NOT_CONNECTED;
> + data->ch_data[i].channel_negative = HX9023S_NOT_CONNECTED;
> + }
> +
> + device_for_each_child_node(dev, child) {
> + ret = fwnode_property_read_u32(child, "reg", ®);
Maybe add a protection for when reg >= num_channels (HX9023S_CH_NUM)?
> + if (ret) {
> + fwnode_handle_put(child);
> + return dev_err_probe(dev, ret, "Failed to read reg\n");
> + }
> + __set_bit(reg, &data->chan_in_use);
> +
> + if (fwnode_property_read_u32(child, "input-channel", &temp) == 0) {
> + data->ch_data[reg].channel_positive = temp;
> + data->ch_data[reg].channel_negative = HX9023S_NOT_CONNECTED;
> + } else if (fwnode_property_read_u32_array(child, "diff-channels",
> + array, sizeof(array)) == 0) {
> + data->ch_data[reg].channel_positive = array[0];
> + data->ch_data[reg].channel_negative = array[1];
> + } else {
> + fwnode_handle_put(child);
> + return dev_err_probe(dev, ret,
> + "Failed to read channel input for channel %d\n", reg);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int hx9023s_update_chan_en(struct hx9023s_data *data,
> + unsigned long chan_read,
> + unsigned long chan_event)
> +{
> + unsigned int i;
> + unsigned long channels = chan_read | chan_event;
> +
> + if ((data->chan_read | data->chan_event) != channels) {
> + for_each_set_bit(i, &channels, HX9023S_CH_NUM)
> + hx9023s_ch_en(data, i, test_bit(i, &data->chan_in_use));
> + for_each_clear_bit(i, &channels, HX9023S_CH_NUM)
> + hx9023s_ch_en(data, i, false);
> + }
> +
> + data->chan_read = chan_read;
> + data->chan_event = chan_event;
> +
> + return 0;
> +}
> +
> +static int hx9023s_get_proximity(struct hx9023s_data *data,
> + const struct iio_chan_spec *chan,
> + int *val)
> +{
> + int ret;
> +
> + ret = hx9023s_sample(data);
> + if (ret)
> + return ret;
> +
> + ret = hx9023s_get_prox_state(data);
> + if (ret)
> + return ret;
> +
> + *val = data->ch_data[chan->channel].diff;
> + return IIO_VAL_INT;
> +}
> +
> +static int hx9023s_get_samp_freq(struct hx9023s_data *data, int *val, int *val2)
> +{
> + int ret;
> + unsigned int odr, index;
> +
> + ret = regmap_read(data->regmap, HX9023S_PRF_CFG, &index);
> + if (ret)
> + return ret;
> +
> + odr = hx9023s_samp_freq_table[index];
> + *val = KILO / odr;
> + *val2 = div_u64((KILO % odr) * MICRO, odr);
> +
> + return IIO_VAL_INT_PLUS_MICRO;
> +}
> +
> +static int hx9023s_read_raw(struct iio_dev *indio_dev, const struct iio_chan_spec *chan,
> + int *val, int *val2, long mask)
> +{
> + struct hx9023s_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + if (chan->type != IIO_PROXIMITY)
> + return -EINVAL;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = iio_device_claim_direct_mode(indio_dev);
> + if (ret)
> + return ret;
> +
> + ret = hx9023s_get_proximity(data, chan, val);
> + iio_device_release_direct_mode(indio_dev);
> + return ret;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + return hx9023s_get_samp_freq(data, val, val2);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int hx9023s_set_samp_freq(struct hx9023s_data *data, int val, int val2)
> +{
> + struct device *dev = regmap_get_device(data->regmap);
> + unsigned int i, period_ms;
> +
> + period_ms = div_u64(NANO, (val * MEGA + val2));
> +
> + for (i = 0; i < ARRAY_SIZE(hx9023s_samp_freq_table); i++) {
> + if (period_ms == hx9023s_samp_freq_table[i])
> + break;
> + }
> + if (i == ARRAY_SIZE(hx9023s_samp_freq_table)) {
> + dev_err(dev, "Period:%dms NOT found!\n", period_ms);
> + return -EINVAL;
> + }
> +
> + return regmap_write(data->regmap, HX9023S_PRF_CFG, i);
> +}
> +
> +static int hx9023s_write_raw(struct iio_dev *indio_dev, const struct iio_chan_spec *chan,
> + int val, int val2, long mask)
> +{
> + struct hx9023s_data *data = iio_priv(indio_dev);
> +
> + if (chan->type != IIO_PROXIMITY)
> + return -EINVAL;
> +
> + if (mask != IIO_CHAN_INFO_SAMP_FREQ)
> + return -EINVAL;
> +
> + return hx9023s_set_samp_freq(data, val, val2);
> +}
> +
> +static irqreturn_t hx9023s_irq_handler(int irq, void *private)
> +{
> + struct iio_dev *indio_dev = private;
> + struct hx9023s_data *data = iio_priv(indio_dev);
> +
> + if (data->trigger_enabled)
> + iio_trigger_poll(data->trig);
> +
> + return IRQ_WAKE_THREAD;
> +}
> +
> +static void hx9023s_push_events(struct iio_dev *indio_dev)
> +{
> + struct hx9023s_data *data = iio_priv(indio_dev);
> + s64 timestamp = iio_get_time_ns(indio_dev);
> + unsigned long prox_changed;
> + unsigned int chan;
> +
> + hx9023s_sample(data);
> + hx9023s_get_prox_state(data);
> +
> + prox_changed = (data->chan_prox_stat ^ data->prox_state_reg) & data->chan_event;
> + for_each_set_bit(chan, &prox_changed, HX9023S_CH_NUM) {
> + unsigned int dir;
> +
> + dir = (data->prox_state_reg & BIT(chan)) ? IIO_EV_DIR_FALLING : IIO_EV_DIR_RISING;
> +
> + iio_push_event(indio_dev,
> + IIO_UNMOD_EVENT_CODE(IIO_PROXIMITY, chan, IIO_EV_TYPE_THRESH, dir),
> + timestamp);
> + }
> + data->chan_prox_stat = data->prox_state_reg;
> +}
> +
> +static irqreturn_t hx9023s_irq_thread_handler(int irq, void *private)
> +{
> + struct iio_dev *indio_dev = private;
> + struct hx9023s_data *data = iio_priv(indio_dev);
> +
> + guard(mutex)(&data->mutex);
> + hx9023s_push_events(indio_dev);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int hx9023s_read_event_val(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + enum iio_event_info info, int *val, int *val2)
> +{
> + struct hx9023s_data *data = iio_priv(indio_dev);
> +
> + if (chan->type != IIO_PROXIMITY)
> + return -EINVAL;
> +
> + switch (info) {
> + case IIO_EV_INFO_VALUE:
> + switch (dir) {
> + case IIO_EV_DIR_RISING:
> + return hx9023s_get_thres_far(data, chan->channel, val);
> + case IIO_EV_DIR_FALLING:
> + return hx9023s_get_thres_near(data, chan->channel, val);
> + default:
> + return -EINVAL;
> + }
> + case IIO_EV_INFO_PERIOD:
> + switch (dir) {
> + case IIO_EV_DIR_RISING:
> + return hx9023s_read_far_debounce(data, val);
> + case IIO_EV_DIR_FALLING:
> + return hx9023s_read_near_debounce(data, val);
> + default:
> + return -EINVAL;
> + }
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int hx9023s_write_event_val(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + enum iio_event_info info, int val, int val2)
> +{
> + struct hx9023s_data *data = iio_priv(indio_dev);
> +
> + if (chan->type != IIO_PROXIMITY)
> + return -EINVAL;
> +
> + switch (info) {
> + case IIO_EV_INFO_VALUE:
> + switch (dir) {
> + case IIO_EV_DIR_RISING:
> + return hx9023s_set_thres_far(data, chan->channel, val);
> + case IIO_EV_DIR_FALLING:
> + return hx9023s_set_thres_near(data, chan->channel, val);
> + default:
> + return -EINVAL;
> + }
> + case IIO_EV_INFO_PERIOD:
> + switch (dir) {
> + case IIO_EV_DIR_RISING:
> + return hx9023s_write_far_debounce(data, val);
> + case IIO_EV_DIR_FALLING:
> + return hx9023s_write_near_debounce(data, val);
> + default:
> + return -EINVAL;
> + }
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int hx9023s_read_event_config(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir)
> +{
> + struct hx9023s_data *data = iio_priv(indio_dev);
> +
> + return test_bit(chan->channel, &data->chan_event);
> +}
> +
> +static int hx9023s_write_event_config(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + int state)
> +{
> + struct hx9023s_data *data = iio_priv(indio_dev);
> +
> + if (test_bit(chan->channel, &data->chan_in_use)) {
> + hx9023s_ch_en(data, chan->channel, !!state);
> + __assign_bit(chan->channel, &data->chan_event, data->ch_data[chan->channel].enable);
> + }
> +
> + return 0;
> +}
> +
> +static const struct iio_info hx9023s_info = {
> + .read_raw = hx9023s_read_raw,
> + .write_raw = hx9023s_write_raw,
> + .read_event_value = hx9023s_read_event_val,
> + .write_event_value = hx9023s_write_event_val,
> + .read_event_config = hx9023s_read_event_config,
> + .write_event_config = hx9023s_write_event_config,
> +};
> +
> +static int hx9023s_set_trigger_state(struct iio_trigger *trig, bool state)
> +{
> + struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> + struct hx9023s_data *data = iio_priv(indio_dev);
> +
> + guard(mutex)(&data->mutex);
> + if (state)
> + hx9023s_interrupt_enable(data);
> + else if (!data->chan_read)
> + hx9023s_interrupt_disable(data);
> + data->trigger_enabled = state;
> +
> + return 0;
> +}
> +
> +static const struct iio_trigger_ops hx9023s_trigger_ops = {
> + .set_trigger_state = hx9023s_set_trigger_state,
> +};
> +
> +static irqreturn_t hx9023s_trigger_handler(int irq, void *private)
> +{
> + struct iio_poll_func *pf = private;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct hx9023s_data *data = iio_priv(indio_dev);
> + struct device *dev = regmap_get_device(data->regmap);
> + int ret;
> + unsigned int bit, index, i = 0;
> +
> + guard(mutex)(&data->mutex);
> + ret = hx9023s_sample(data);
> + if (ret) {
> + dev_warn(dev, "sampling failed\n");
> + goto out;
> + }
> +
> + ret = hx9023s_get_prox_state(data);
> + if (ret) {
> + dev_warn(dev, "get prox failed\n");
> + goto out;
> + }
> +
> + for_each_set_bit(bit, indio_dev->active_scan_mask, indio_dev->masklength) {
> + index = indio_dev->channels[bit].channel;
> + data->buffer.channels[i++] = cpu_to_le16(data->ch_data[index].diff);
> + }
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer, pf->timestamp);
> +
> +out:
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int hx9023s_buffer_preenable(struct iio_dev *indio_dev)
> +{
> + struct hx9023s_data *data = iio_priv(indio_dev);
> + unsigned long channels = 0;
> + unsigned int bit;
> +
> + guard(mutex)(&data->mutex);
> + for_each_set_bit(bit, indio_dev->active_scan_mask, indio_dev->masklength)
> + __set_bit(indio_dev->channels[bit].channel, &channels);
> +
> + hx9023s_update_chan_en(data, channels, data->chan_event);
> +
> + return 0;
> +}
> +
> +static int hx9023s_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> + struct hx9023s_data *data = iio_priv(indio_dev);
> +
> + guard(mutex)(&data->mutex);
> + hx9023s_update_chan_en(data, 0, data->chan_event);
> +
> + return 0;
> +}
> +
> +static const struct iio_buffer_setup_ops hx9023s_buffer_setup_ops = {
> + .preenable = hx9023s_buffer_preenable,
> + .postdisable = hx9023s_buffer_postdisable,
> +};
> +
> +static int hx9023s_id_check(struct iio_dev *indio_dev)
> +{
> + struct hx9023s_data *data = iio_priv(indio_dev);
> + int ret;
> + unsigned int id;
> +
> + ret = regmap_read(data->regmap, HX9023S_DEVICE_ID, &id);
> + if (ret)
> + return ret;
> +
> + if (id == HX9023S_CHIP_ID) {
> + indio_dev->name = "hx9023s";
This assignment is quirky here.
Maybe move this into the probe function?
The rest of the function looks fine.
> + return 0;
> + }
> +
> + return -ENODEV;
> +}
> +
> +static int hx9023s_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct iio_dev *indio_dev;
> + struct hx9023s_data *data;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(dev, sizeof(struct hx9023s_data));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + data = iio_priv(indio_dev);
> + mutex_init(&data->mutex);
> +
> + data->regmap = devm_regmap_init_i2c(client, &hx9023s_regmap_config);
> + if (IS_ERR(data->regmap))
> + return dev_err_probe(dev, PTR_ERR(data->regmap), "regmap init failed\n");
> +
> + ret = hx9023s_property_get(data);
> + if (ret)
> + return dev_err_probe(dev, ret, "dts phase failed\n");
> +
> + ret = devm_regulator_get_enable(dev, "vdd");
> + if (ret)
> + return dev_err_probe(dev, ret, "regulator get failed\n");
> +
> + ret = hx9023s_id_check(indio_dev);
> + if (ret)
> + return dev_err_probe(dev, ret, "id check failed\n");
> +
> + indio_dev->channels = hx9023s_channels;
> + indio_dev->num_channels = ARRAY_SIZE(hx9023s_channels);
> + indio_dev->info = &hx9023s_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + i2c_set_clientdata(client, indio_dev);
> +
> + ret = hx9023s_reg_init(data);
> + if (ret)
> + return dev_err_probe(dev, ret, "device init failed\n");
> +
> + ret = hx9023s_ch_cfg(data);
> + if (ret)
> + return dev_err_probe(dev, ret, "channel config failed\n");
> +
> + ret = regcache_sync(data->regmap);
> + if (ret)
> + return dev_err_probe(dev, ret, "regcache sync failed\n");
> +
> + if (client->irq) {
> + ret = devm_request_threaded_irq(dev, client->irq, hx9023s_irq_handler,
> + hx9023s_irq_thread_handler, IRQF_ONESHOT,
> + "hx9023s_event", indio_dev);
> + if (ret)
> + return dev_err_probe(dev, ret, "irq request failed\n");
> +
> + data->trig = devm_iio_trigger_alloc(dev, "%s-dev%d", indio_dev->name,
> + iio_device_id(indio_dev));
> + if (!data->trig)
> + return dev_err_probe(dev, -ENOMEM,
> + "iio trigger alloc failed\n");
> +
> + data->trig->ops = &hx9023s_trigger_ops;
> + iio_trigger_set_drvdata(data->trig, indio_dev);
> +
> + ret = devm_iio_trigger_register(dev, data->trig);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "iio trigger register failed\n");
> + }
> +
> + ret = devm_iio_triggered_buffer_setup(dev, indio_dev, iio_pollfunc_store_time,
> + hx9023s_trigger_handler, &hx9023s_buffer_setup_ops);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "iio triggered buffer setup failed\n");
> +
> + ret = devm_iio_device_register(dev, indio_dev);
A direct return would also work:
return devm_iio_device_register(dev, indio_dev);
And it would get logged if it happens.
> + if (ret)
> + return dev_err_probe(dev, ret, "iio device register failed\n");
> +
> + return 0;
> +}
> +
> +static int hx9023s_suspend(struct device *dev)
> +{
> + struct hx9023s_data *data = iio_priv(dev_get_drvdata(dev));
> +
> + hx9023s_interrupt_disable(data);
> +
> + return 0;
> +}
> +
> +static int hx9023s_resume(struct device *dev)
> +{
> + struct hx9023s_data *data = iio_priv(dev_get_drvdata(dev));
> +
> + if (data->trigger_enabled)
> + hx9023s_interrupt_enable(data);
> +
> + return 0;
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(hx9023s_pm_ops, hx9023s_suspend, hx9023s_resume);
> +
> +static const struct of_device_id hx9023s_of_match[] = {
> + { .compatible = "tyhx,hx9023s" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, hx9023s_of_match);
> +
> +static const struct i2c_device_id hx9023s_id[] = {
> + { "hx9023s" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, hx9023s_id);
> +
> +static struct i2c_driver hx9023s_driver = {
> + .driver = {
> + .name = "hx9023s",
> + .of_match_table = hx9023s_of_match,
> + .pm = &hx9023s_pm_ops,
> +
> + /*
> + * The I2C operations in hx9023s_reg_init() and hx9023s_ch_cfg()
> + * are time-consuming. Prefer async so we don't delay boot
> + * if we're builtin to the kernel.
> + */
> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> + },
> + .probe = hx9023s_probe,
> + .id_table = hx9023s_id,
> +};
> +module_i2c_driver(hx9023s_driver);
> +
> +MODULE_AUTHOR("Yasin Lee <yasin.lee.x@gmail.com>");
> +MODULE_DESCRIPTION("Driver for TYHX HX9023S SAR sensor");
> +MODULE_LICENSE("GPL");
>
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 2/3] dt-bindings: iio: proximity: Add TYHX HX9023S
2024-06-21 10:12 ` Krzysztof Kozlowski
@ 2024-06-22 5:56 ` Yasin Lee
2024-06-22 10:51 ` Conor Dooley
0 siblings, 1 reply; 15+ messages in thread
From: Yasin Lee @ 2024-06-22 5:56 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jonathan Cameron, Lars-Peter Clausen, yasin.lee.x
Cc: devicetree, linux-kernel, linux-iio
On 2024/6/21 18:12, Krzysztof Kozlowski wrote:
Hi ,Krzysztof
Thank you for your reply. I have some questions inline.
Best regards,
Yasin
> On 21/06/2024 09:40, Yasin Lee wrote:
>> A capacitive proximity sensor
>>
>> Signed-off-by: Yasin Lee <yasin.lee.x@gmail.com>
>> ---
>> .../bindings/iio/proximity/tyhx,hx9023s.yaml | 115 +++++++++++++++++++++
>> 1 file changed, 115 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml b/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
>> new file mode 100644
>> index 000000000000..beca70ce7609
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
>> @@ -0,0 +1,115 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/iio/proximity/tyhx,hx9023s.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: TYHX HX9023S capacitive proximity sensor
>> +
>> +maintainers:
>> + - Yasin Lee <yasin.lee.x@gmail.com>
>> +
>> +description: |
>> + TYHX HX9023S proximity sensor. Datasheet can be found here:
>> + http://www.tianyihexin.com/ueditor/php/upload/file/20240614/1718336303992081.pdf
>> +
>> +allOf:
>> + - $ref: /schemas/iio/iio.yaml#
> Which part of iio.yaml binding do you use here? I cannot find anything,
> so this looks wrong.
>
I will remove this reference.
>> +
>> +properties:
>> + compatible:
>> + const: tyhx,hx9023s
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + interrupts:
>> + description:
>> + Generated by device to announce preceding read request has finished
>> + and data is available or that a close/far proximity event has happened.
>> + maxItems: 1
>> +
>> + vdd-supply: true
>> +
>> + "#address-cells":
>> + const: 1
>> +
>> + "#size-cells":
>> + const: 0
>> +
>> +patternProperties:
>> + "^channel@[0-4]$":
>> + $ref: /schemas/iio/adc/adc.yaml
>> + type: object
>> +
>> + properties:
>> + reg:
>> + minimum: 0
>> + maximum: 4
>> + description: The channel number.
>> +
>> + input-channel:
> Isn't this duplicating single-channel property?
>
> Where is this property defined (which common schema)?
>
|input-channel| is indeed intended for single-ended configuration, but I
couldn't find a definition
or reference for |single-channel| anywhere. If possible, should I rename
|input-channel| to |single-channel|?
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + minimum: 0
>> + maximum: 4
>> + description:
>> + Specify the input pin used in single-ended configuration.
>> +
>> + diff-channels: true
>> +
>> + oneOf:
>> + - required:
>> + - input-channel
>> + - required:
>> + - diff-channels
>> +
>> + required:
>> + - reg
>> +
>> + additionalProperties: false
>> +
>> +required:
>> + - compatible
>> + - vdd-supply
>> + - reg
> Keep the same order as in properties.
OK, I will correct the order in the next version.
>> +
>> +unevaluatedProperties: false
>> +
>
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 2/3] dt-bindings: iio: proximity: Add TYHX HX9023S
2024-06-22 5:56 ` Yasin Lee
@ 2024-06-22 10:51 ` Conor Dooley
2024-06-22 12:35 ` Yasin Lee
0 siblings, 1 reply; 15+ messages in thread
From: Conor Dooley @ 2024-06-22 10:51 UTC (permalink / raw)
To: Yasin Lee
Cc: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jonathan Cameron, Lars-Peter Clausen, yasin.lee.x,
devicetree, linux-kernel, linux-iio
[-- Attachment #1: Type: text/plain, Size: 2956 bytes --]
On Sat, Jun 22, 2024 at 01:56:42PM +0800, Yasin Lee wrote:
>
> On 2024/6/21 18:12, Krzysztof Kozlowski wrote:
>
> Hi ,Krzysztof
> Thank you for your reply. I have some questions inline.
>
> Best regards,
> Yasin
>
> > On 21/06/2024 09:40, Yasin Lee wrote:
> > > A capacitive proximity sensor
> > >
> > > Signed-off-by: Yasin Lee <yasin.lee.x@gmail.com>
> > > ---
> > > .../bindings/iio/proximity/tyhx,hx9023s.yaml | 115 +++++++++++++++++++++
> > > 1 file changed, 115 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml b/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
> > > new file mode 100644
> > > index 000000000000..beca70ce7609
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
> > > @@ -0,0 +1,115 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/iio/proximity/tyhx,hx9023s.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: TYHX HX9023S capacitive proximity sensor
> > > +
> > > +maintainers:
> > > + - Yasin Lee <yasin.lee.x@gmail.com>
> > > +
> > > +description: |
> > > + TYHX HX9023S proximity sensor. Datasheet can be found here:
> > > + http://www.tianyihexin.com/ueditor/php/upload/file/20240614/1718336303992081.pdf
> > > +
> > > +allOf:
> > > + - $ref: /schemas/iio/iio.yaml#
> > Which part of iio.yaml binding do you use here? I cannot find anything,
> > so this looks wrong.
> >
>
> I will remove this reference.
>
>
> > > +
> > > +properties:
> > > + compatible:
> > > + const: tyhx,hx9023s
> > > +
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + interrupts:
> > > + description:
> > > + Generated by device to announce preceding read request has finished
> > > + and data is available or that a close/far proximity event has happened.
> > > + maxItems: 1
> > > +
> > > + vdd-supply: true
> > > +
> > > + "#address-cells":
> > > + const: 1
> > > +
> > > + "#size-cells":
> > > + const: 0
> > > +
> > > +patternProperties:
> > > + "^channel@[0-4]$":
> > > + $ref: /schemas/iio/adc/adc.yaml
> > > + type: object
> > > +
> > > + properties:
> > > + reg:
> > > + minimum: 0
> > > + maximum: 4
> > > + description: The channel number.
> > > +
> > > + input-channel:
> > Isn't this duplicating single-channel property?
> >
> > Where is this property defined (which common schema)?
> >
> |input-channel| is indeed intended for single-ended configuration, but I
> couldn't find a definition
>
> or reference for |single-channel| anywhere. If possible, should I rename
> |input-channel| to |single-channel|?
Single-channel is new, it should be the next branch of the iio tree and
in linux-next.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 3/3] iio: proximity: Add driver support for TYHX's HX9023S capacitive proximity sensor
2024-06-21 14:09 ` Alexandru Ardelean
@ 2024-06-22 12:15 ` Yasin Lee
0 siblings, 0 replies; 15+ messages in thread
From: Yasin Lee @ 2024-06-22 12:15 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
Lars-Peter Clausen, yasin.lee.x, devicetree, linux-kernel,
linux-iio
On 2024/6/21 22:09, Alexandru Ardelean wrote:
> On Fri, Jun 21, 2024 at 10:44 AM Yasin Lee <yasin.lee.x@gmail.com> wrote:
Hi Alexandru,
Thank you for your reply. I have provided some explanations regarding
the use of HX9023S_CH_NUM in the inline comments. Please review them.
Best regards,
Yasin
>> A SAR sensor from NanjingTianyihexin Electronics Ltd.
>>
>> The device has the following entry points:
>>
>> Usual frequency:
>> - sampling_frequency
>>
>> Instant reading of current values for different sensors:
>> - in_proximity0_raw
>> - in_proximity1_raw
>> - in_proximity2_raw
>> - in_proximity3_raw
>> - in_proximity4_raw
>> and associated events in events/
>>
> Hello :)
Hello ^_^
>> Signed-off-by: Yasin Lee <yasin.lee.x@gmail.com>
>> ---
>> drivers/iio/proximity/Kconfig | 14 +
>> drivers/iio/proximity/Makefile | 1 +
>> drivers/iio/proximity/hx9023s.c | 1150 +++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 1165 insertions(+)
>>
...
>> +
>> +#include <linux/array_size.h>
>> +#include <linux/bitfield.h>
>> +#include <linux/bitops.h>
>> +#include <linux/delay.h>
>> +#include <linux/device.h>
>> +#include <linux/errno.h>
>> +#include <linux/i2c.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/irqreturn.h>
>> +#include <linux/math.h>
>> +#include <linux/mod_devicetable.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/pm.h>
>> +#include <linux/property.h>
>> +#include <linux/regmap.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/types.h>
>> +#include <linux/units.h>
>> +
>> +#include <asm/byteorder.h>
>> +#include <asm/unaligned.h>
>> +
>> +#include <linux/iio/buffer.h>
>> +#include <linux/iio/events.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/trigger.h>
>> +#include <linux/iio/triggered_buffer.h>
>> +#include <linux/iio/trigger_consumer.h>
>> +#include <linux/iio/types.h>
> A first question is: are all these headers required?
> Looks like some of them could be removed.
I checked all the header files again, following the IWYU principle
("include what you use").
I confirm that they are all necessary. Below I listed the usage for each
file:
#include <linux/array_size.h> //ARRAY_SIZE
#include <linux/bitfield.h> //FIELD_*
#include <linux/bitops.h> //assign_bit
#include <linux/device.h> //dev_get_drvdata
#include <linux/errno.h> //ENOMEM @ #include <uapi/linux/errno.h>
#include <linux/i2c.h> //i2c_client
#include <linux/interrupt.h> //IRQF_ONESHOT
#include <linux/irqreturn.h> //irqreturn_t
#include <linux/math64.h> //div_u64
#include <linux/mod_devicetable.h> //MODULE_DEVICE_TABLE
#include <linux/module.h> //MODULE_AUTHOR
#include <linux/mutex.h> //mutex_init
#include <linux/pm.h> //DEFINE_SIMPLE_DEV_PM_OPS
#include <linux/property.h> //fwnode_*
#include <linux/regmap.h> //regmap*
#include <linux/regulator/consumer.h> //devm_regulator_get_enable
#include <linux/types.h> //u8 u32
#include <linux/units.h> //NANO MEGA
#include <asm/byteorder.h> //__be* __le*
#include <asm/unaligned.h> //get_unaligned_le16 why not
<asm-generic/unaligned.h>
#include <linux/iio/buffer.h> //iio_push_to_buffers_with_timestamp
#include <linux/iio/events.h> //IIO_UNMOD_EVENT_CODE
#include <linux/iio/iio.h> //iio_*
#include <linux/iio/trigger.h> //iio_trigger*
#include <linux/iio/triggered_buffer.h> //iio_triggered_buffer*
#include <linux/iio/trigger_consumer.h> //iio_trigger_notify_done
#include <linux/iio/types.h> //IIO_*
>> +
>> +#define HX9023S_CHIP_ID 0x1D
>> +#define HX9023S_CH_NUM 5
>> +#define HX9023S_2BYTES 2
...
>> +
>> +struct hx9023s_addr_val_pair {
>> + u8 addr;
>> + u8 val;
>> +};
> This looks like:
>
> struct reg_sequence {
> unsigned int reg;
> unsigned int def;
> unsigned int delay_us;
> };
>
> This is defined in include/linux/regmap.h
I will remove |hx9023s_addr_val_pair|.
...
>> +
>> +static struct hx9023s_addr_val_pair hx9023s_reg_init_list[] = {
> Globals like this should be `static const`
> Also, it would be a good idea to define this as `static const struct
> reg_sequence `
>
> Then the `regmap_multi_reg_write()` function could be used.
I will add the |const| qualifier and define the array type as
|reg_sequence|.
I will replace the for loop with |regmap_multi_reg_write()|. This is a
good idea.
...
>> +
>> +static int hx9023s_ch_cfg(struct hx9023s_data *data)
>> +{
>> + unsigned int i;
>> + u16 reg;
>> + u8 reg_list[HX9023S_CH_NUM * 2];
>> + u8 ch_pos[HX9023S_CH_NUM];
>> + u8 ch_neg[HX9023S_CH_NUM];
>> + /* Bit positions corresponding to input pin connections */
>> + u8 conn_cs[HX9023S_CH_NUM] = {0, 2, 4, 6, 8};
>> +
>> + for (i = 0; i < HX9023S_CH_NUM; i++) {
> See comment [1]
>
>> + ch_pos[i] = data->ch_data[i].channel_positive == HX9023S_NOT_CONNECTED ?
>> + HX9023S_NOT_CONNECTED : conn_cs[data->ch_data[i].channel_positive];
>> + ch_neg[i] = data->ch_data[i].channel_negative == HX9023S_NOT_CONNECTED ?
>> + HX9023S_NOT_CONNECTED : conn_cs[data->ch_data[i].channel_negative];
>> +
>> + reg = (HX9023S_POS << ch_pos[i]) | (HX9023S_NEG << ch_neg[i]);
>> + put_unaligned_le16(reg, ®_list[i * 2]);
>> + }
>> +
>> + return regmap_bulk_write(data->regmap, HX9023S_CH0_CFG_7_0, reg_list, HX9023S_CH_NUM * 2);
>> +}
>> +
...
>> +
>> +static int hx9023s_sample(struct hx9023s_data *data)
>> +{
>> + int ret, value;
>> + unsigned int i;
>> + u8 data_size, offset_data_size, *p, size, rx_buf[HX9023S_CH_NUM * HX9023S_BYTES_MAX];
>> +
>> + ret = hx9023s_data_lock(data, true);
>> + if (ret)
>> + return ret;
>> +
>> + ret = hx9023s_data_select(data);
>> + if (ret)
>> + return ret;
> From here onwards, it looks like if there is an error, then
> `hx9023s_data_lock(data, false)` does not get called.
> Is that expected?
> Maybe some `goto err` statements would be needed?
>
This is a bug, I will fix it.
>> +
>> + data_size = HX9023S_3BYTES;
>> +
>> + /* ch0~ch3 */
>> + p = rx_buf;
>> + size = (HX9023S_CH_NUM - 1) * data_size;
>> + ret = regmap_bulk_read(data->regmap, HX9023S_RAW_BL_CH0_0, p, size);
>> + if (ret)
>> + return ret;
>> +
>> + /* ch4 */
>> + p = rx_buf + size;
>> + size = data_size;
>> + ret = regmap_bulk_read(data->regmap, HX9023S_RAW_BL_CH4_0, p, size);
>> + if (ret)
>> + return ret;
>> +
>> + for (i = 0; i < HX9023S_CH_NUM; i++) {
> [1]
> Maybe use some per-device (example: indio_dev->num_channels) here
> (instead of HX9023S_CH_NUM)?
> If adding support for a part with fewer channels, this would crash.
> This comment is for all places where for (i = 0; i < HX9023S_CH_NUM;
> i++) is used
>
HX9023S_CH_NUM represents the number of configuration registers related to the channels
for this series of chips. This is the maximum value.
Regardless of the actual number of channels used, all these registers need to be configured.
Even if a certain model in this series has fewer actual channels,
Therefore, this value does not decrease due to the use of fewer channels.
Hence, I believe it should remain as it is.
>> + value = get_unaligned_le16(&rx_buf[i * data_size + 1]);
>> + value = sign_extend32(value, 15);
>> + data->ch_data[i].raw = 0;
>> + data->ch_data[i].bl = 0;
>> + if (data->ch_data[i].sel_raw == true)
>> + data->ch_data[i].raw = value;
>> + if (data->ch_data[i].sel_bl == true)
>> + data->ch_data[i].bl = value;
>> + }
>> +
>> + /* ch0~ch3 */
>> + p = rx_buf;
>> + size = (HX9023S_CH_NUM - 1) * data_size;
>> + ret = regmap_bulk_read(data->regmap, HX9023S_LP_DIFF_CH0_0, p, size);
>> + if (ret)
>> + return ret;
>> +
>> + /* ch4 */
>> + p = rx_buf + size;
>> + size = data_size;
>> + ret = regmap_bulk_read(data->regmap, HX9023S_LP_DIFF_CH4_0, p, size);
>> + if (ret)
>> + return ret;
>> +
>> + for (i = 0; i < HX9023S_CH_NUM; i++) {
> See comment [1]
>
>> + value = get_unaligned_le16(&rx_buf[i * data_size + 1]);
>> + value = sign_extend32(value, 15);
>> + data->ch_data[i].lp = 0;
>> + data->ch_data[i].diff = 0;
>> + if (data->ch_data[i].sel_lp == true)
>> + data->ch_data[i].lp = value;
>> + if (data->ch_data[i].sel_diff == true)
>> + data->ch_data[i].diff = value;
>> + }
>> +
>> + for (i = 0; i < HX9023S_CH_NUM; i++) {
> See comment [1]
>
>> + if (data->ch_data[i].sel_lp == true && data->ch_data[i].sel_bl == true)
>> + data->ch_data[i].diff = data->ch_data[i].lp - data->ch_data[i].bl;
>> + }
>> +
>> + /* offset DAC */
>> + offset_data_size = HX9023S_2BYTES;
>> + p = rx_buf;
>> + size = HX9023S_CH_NUM * offset_data_size;
>> + ret = regmap_bulk_read(data->regmap, HX9023S_OFFSET_DAC0_7_0, p, size);
>> + if (ret)
>> + return ret;
>> +
>> + for (i = 0; i < HX9023S_CH_NUM; i++) {
> See comment [1]
>
>> + value = get_unaligned_le16(&rx_buf[i * offset_data_size]);
>> + value = FIELD_GET(GENMASK(11, 0), value);
>> + data->ch_data[i].dac = value;
>> + }
>> +
>> + ret = hx9023s_data_lock(data, false);
>> + if (ret)
>> + return ret;
>> +
>> + return 0;
>> +}
>> +
...
>> +
>> +static int hx9023s_property_get(struct hx9023s_data *data)
>> +{
>> + struct fwnode_handle *child;
>> + struct device *dev = regmap_get_device(data->regmap);
>> + int ret;
>> + u32 i, reg, temp, array[2];
>> +
>> + data->chan_in_use = 0;
>> + for (i = 0; i < HX9023S_CH_NUM; i++) {
> See comment [1]
>
>> + data->ch_data[i].channel_positive = HX9023S_NOT_CONNECTED;
>> + data->ch_data[i].channel_negative = HX9023S_NOT_CONNECTED;
>> + }
>> +
>> + device_for_each_child_node(dev, child) {
>> + ret = fwnode_property_read_u32(child, "reg", ®);
> Maybe add a protection for when reg >= num_channels (HX9023S_CH_NUM)?
This protection is necessary. I will add it in the next version.
>> + if (ret) {
>> + fwnode_handle_put(child);
>> + return dev_err_probe(dev, ret, "Failed to read reg\n");
>> + }
>> + __set_bit(reg, &data->chan_in_use);
>> +
>> + if (fwnode_property_read_u32(child, "input-channel", &temp) == 0) {
>> + data->ch_data[reg].channel_positive = temp;
>> + data->ch_data[reg].channel_negative = HX9023S_NOT_CONNECTED;
>> + } else if (fwnode_property_read_u32_array(child, "diff-channels",
>> + array, sizeof(array)) == 0) {
>> + data->ch_data[reg].channel_positive = array[0];
>> + data->ch_data[reg].channel_negative = array[1];
>> + } else {
>> + fwnode_handle_put(child);
>> + return dev_err_probe(dev, ret,
>> + "Failed to read channel input for channel %d\n", reg);
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
...
>> +
>> +static int hx9023s_id_check(struct iio_dev *indio_dev)
>> +{
>> + struct hx9023s_data *data = iio_priv(indio_dev);
>> + int ret;
>> + unsigned int id;
>> +
>> + ret = regmap_read(data->regmap, HX9023S_DEVICE_ID, &id);
>> + if (ret)
>> + return ret;
>> +
>> + if (id == HX9023S_CHIP_ID) {
>> + indio_dev->name = "hx9023s";
> This assignment is quirky here.
> Maybe move this into the probe function?
> The rest of the function looks fine.
Okay, I will fix this as suggested.
>> + return 0;
>> + }
>> +
>> + return -ENODEV;
>> +}
>> +
>> +static int hx9023s_probe(struct i2c_client *client)
>> +{
>> + struct device *dev = &client->dev;
>> + struct iio_dev *indio_dev;
>> + struct hx9023s_data *data;
>> + int ret;
>> +
>> + indio_dev = devm_iio_device_alloc(dev, sizeof(struct hx9023s_data));
>> + if (!indio_dev)
>> + return -ENOMEM;
>> +
>> + data = iio_priv(indio_dev);
>> + mutex_init(&data->mutex);
>> +
>> + data->regmap = devm_regmap_init_i2c(client, &hx9023s_regmap_config);
>> + if (IS_ERR(data->regmap))
>> + return dev_err_probe(dev, PTR_ERR(data->regmap), "regmap init failed\n");
>> +
>> + ret = hx9023s_property_get(data);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "dts phase failed\n");
>> +
>> + ret = devm_regulator_get_enable(dev, "vdd");
>> + if (ret)
>> + return dev_err_probe(dev, ret, "regulator get failed\n");
>> +
>> + ret = hx9023s_id_check(indio_dev);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "id check failed\n");
>> +
>> + indio_dev->channels = hx9023s_channels;
>> + indio_dev->num_channels = ARRAY_SIZE(hx9023s_channels);
>> + indio_dev->info = &hx9023s_info;
>> + indio_dev->modes = INDIO_DIRECT_MODE;
>> + i2c_set_clientdata(client, indio_dev);
>> +
>> + ret = hx9023s_reg_init(data);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "device init failed\n");
>> +
>> + ret = hx9023s_ch_cfg(data);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "channel config failed\n");
>> +
>> + ret = regcache_sync(data->regmap);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "regcache sync failed\n");
>> +
>> + if (client->irq) {
>> + ret = devm_request_threaded_irq(dev, client->irq, hx9023s_irq_handler,
>> + hx9023s_irq_thread_handler, IRQF_ONESHOT,
>> + "hx9023s_event", indio_dev);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "irq request failed\n");
>> +
>> + data->trig = devm_iio_trigger_alloc(dev, "%s-dev%d", indio_dev->name,
>> + iio_device_id(indio_dev));
>> + if (!data->trig)
>> + return dev_err_probe(dev, -ENOMEM,
>> + "iio trigger alloc failed\n");
>> +
>> + data->trig->ops = &hx9023s_trigger_ops;
>> + iio_trigger_set_drvdata(data->trig, indio_dev);
>> +
>> + ret = devm_iio_trigger_register(dev, data->trig);
>> + if (ret)
>> + return dev_err_probe(dev, ret,
>> + "iio trigger register failed\n");
>> + }
>> +
>> + ret = devm_iio_triggered_buffer_setup(dev, indio_dev, iio_pollfunc_store_time,
>> + hx9023s_trigger_handler, &hx9023s_buffer_setup_ops);
>> + if (ret)
>> + return dev_err_probe(dev, ret,
>> + "iio triggered buffer setup failed\n");
>> +
>> + ret = devm_iio_device_register(dev, indio_dev);
> A direct return would also work:
> return devm_iio_device_register(dev, indio_dev);
>
> And it would get logged if it happens.
I will fix it in V7
>> + if (ret)
>> + return dev_err_probe(dev, ret, "iio device register failed\n");
>> +
>> + return 0;
>> +}
>> +
...
>>
>> --
>> 2.25.1
>>
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 2/3] dt-bindings: iio: proximity: Add TYHX HX9023S
2024-06-22 10:51 ` Conor Dooley
@ 2024-06-22 12:35 ` Yasin Lee
2024-06-22 18:04 ` Krzysztof Kozlowski
0 siblings, 1 reply; 15+ messages in thread
From: Yasin Lee @ 2024-06-22 12:35 UTC (permalink / raw)
To: Conor Dooley
Cc: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jonathan Cameron, Lars-Peter Clausen, yasin.lee.x,
devicetree, linux-kernel, linux-iio
On 2024/6/22 18:51, Conor Dooley wrote:
> On Sat, Jun 22, 2024 at 01:56:42PM +0800, Yasin Lee wrote:
>> On 2024/6/21 18:12, Krzysztof Kozlowski wrote:
>>
>> Hi ,Krzysztof
>> Thank you for your reply. I have some questions inline.
>>
>> Best regards,
>> Yasin
>>
>>> On 21/06/2024 09:40, Yasin Lee wrote:
>>>> A capacitive proximity sensor
>>>>
>>>> Signed-off-by: Yasin Lee <yasin.lee.x@gmail.com>
>>>> ---
>>>> .../bindings/iio/proximity/tyhx,hx9023s.yaml | 115 +++++++++++++++++++++
>>>> 1 file changed, 115 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml b/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
>>>> new file mode 100644
>>>> index 000000000000..beca70ce7609
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
>>>> @@ -0,0 +1,115 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/iio/proximity/tyhx,hx9023s.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: TYHX HX9023S capacitive proximity sensor
>>>> +
>>>> +maintainers:
>>>> + - Yasin Lee <yasin.lee.x@gmail.com>
>>>> +
>>>> +description: |
>>>> + TYHX HX9023S proximity sensor. Datasheet can be found here:
>>>> + http://www.tianyihexin.com/ueditor/php/upload/file/20240614/1718336303992081.pdf
>>>> +
>>>> +allOf:
>>>> + - $ref: /schemas/iio/iio.yaml#
>>> Which part of iio.yaml binding do you use here? I cannot find anything,
>>> so this looks wrong.
>>>
>> I will remove this reference.
>>
>>
>>>> +
>>>> +properties:
>>>> + compatible:
>>>> + const: tyhx,hx9023s
>>>> +
>>>> + reg:
>>>> + maxItems: 1
>>>> +
>>>> + interrupts:
>>>> + description:
>>>> + Generated by device to announce preceding read request has finished
>>>> + and data is available or that a close/far proximity event has happened.
>>>> + maxItems: 1
>>>> +
>>>> + vdd-supply: true
>>>> +
>>>> + "#address-cells":
>>>> + const: 1
>>>> +
>>>> + "#size-cells":
>>>> + const: 0
>>>> +
>>>> +patternProperties:
>>>> + "^channel@[0-4]$":
>>>> + $ref: /schemas/iio/adc/adc.yaml
>>>> + type: object
>>>> +
>>>> + properties:
>>>> + reg:
>>>> + minimum: 0
>>>> + maximum: 4
>>>> + description: The channel number.
>>>> +
>>>> + input-channel:
>>> Isn't this duplicating single-channel property?
>>>
>>> Where is this property defined (which common schema)?
>>>
>> |input-channel| is indeed intended for single-ended configuration, but I
>> couldn't find a definition
>>
>> or reference for |single-channel| anywhere. If possible, should I rename
>> |input-channel| to |single-channel|?
> Single-channel is new, it should be the next branch of the iio tree and
> in linux-next.
Hi Conor，
Thank you for informing me. I plan to temporarily add a prefix to this
attribute to distinguish it and update it in the future. Is this the
correct approach?
- single-channel:
+ tyhx,single-channel:
$ref: /schemas/types.yaml#/definitions/uint32
minimum: 0
maximum: 4
description:
Specify the input pin used in single-ended configuration.
Best regards，
Yasin
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 2/3] dt-bindings: iio: proximity: Add TYHX HX9023S
2024-06-22 12:35 ` Yasin Lee
@ 2024-06-22 18:04 ` Krzysztof Kozlowski
2024-06-23 0:50 ` Yasin Lee
0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-06-22 18:04 UTC (permalink / raw)
To: Yasin Lee, Conor Dooley
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
Lars-Peter Clausen, yasin.lee.x, devicetree, linux-kernel,
linux-iio
On 22/06/2024 14:35, Yasin Lee wrote:
>
> On 2024/6/22 18:51, Conor Dooley wrote:
>> On Sat, Jun 22, 2024 at 01:56:42PM +0800, Yasin Lee wrote:
>>> On 2024/6/21 18:12, Krzysztof Kozlowski wrote:
>>>
>>> Hi ,Krzysztof
>>> Thank you for your reply. I have some questions inline.
>>>
>>> Best regards,
>>> Yasin
>>>
>>>> On 21/06/2024 09:40, Yasin Lee wrote:
>>>>> A capacitive proximity sensor
>>>>>
>>>>> Signed-off-by: Yasin Lee <yasin.lee.x@gmail.com>
>>>>> ---
>>>>> .../bindings/iio/proximity/tyhx,hx9023s.yaml | 115 +++++++++++++++++++++
>>>>> 1 file changed, 115 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml b/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..beca70ce7609
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
>>>>> @@ -0,0 +1,115 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/iio/proximity/tyhx,hx9023s.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: TYHX HX9023S capacitive proximity sensor
>>>>> +
>>>>> +maintainers:
>>>>> + - Yasin Lee <yasin.lee.x@gmail.com>
>>>>> +
>>>>> +description: |
>>>>> + TYHX HX9023S proximity sensor. Datasheet can be found here:
>>>>> + http://www.tianyihexin.com/ueditor/php/upload/file/20240614/1718336303992081.pdf
>>>>> +
>>>>> +allOf:
>>>>> + - $ref: /schemas/iio/iio.yaml#
>>>> Which part of iio.yaml binding do you use here? I cannot find anything,
>>>> so this looks wrong.
>>>>
>>> I will remove this reference.
>>>
>>>
>>>>> +
>>>>> +properties:
>>>>> + compatible:
>>>>> + const: tyhx,hx9023s
>>>>> +
>>>>> + reg:
>>>>> + maxItems: 1
>>>>> +
>>>>> + interrupts:
>>>>> + description:
>>>>> + Generated by device to announce preceding read request has finished
>>>>> + and data is available or that a close/far proximity event has happened.
>>>>> + maxItems: 1
>>>>> +
>>>>> + vdd-supply: true
>>>>> +
>>>>> + "#address-cells":
>>>>> + const: 1
>>>>> +
>>>>> + "#size-cells":
>>>>> + const: 0
>>>>> +
>>>>> +patternProperties:
>>>>> + "^channel@[0-4]$":
>>>>> + $ref: /schemas/iio/adc/adc.yaml
>>>>> + type: object
>>>>> +
>>>>> + properties:
>>>>> + reg:
>>>>> + minimum: 0
>>>>> + maximum: 4
>>>>> + description: The channel number.
>>>>> +
>>>>> + input-channel:
>>>> Isn't this duplicating single-channel property?
>>>>
>>>> Where is this property defined (which common schema)?
>>>>
>>> |input-channel| is indeed intended for single-ended configuration, but I
>>> couldn't find a definition
>>>
>>> or reference for |single-channel| anywhere. If possible, should I rename
>>> |input-channel| to |single-channel|?
>> Single-channel is new, it should be the next branch of the iio tree and
>> in linux-next.
>
> Hi Conor，
>
> Thank you for informing me. I plan to temporarily add a prefix to this
> attribute to distinguish it and update it in the future. Is this the
> correct approach?
No, because there is no need. You are supposed to work on maintainer
tree (linux-next works usually as well).
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 2/3] dt-bindings: iio: proximity: Add TYHX HX9023S
2024-06-22 18:04 ` Krzysztof Kozlowski
@ 2024-06-23 0:50 ` Yasin Lee
0 siblings, 0 replies; 15+ messages in thread
From: Yasin Lee @ 2024-06-23 0:50 UTC (permalink / raw)
To: Krzysztof Kozlowski, Conor Dooley
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
Lars-Peter Clausen, yasin.lee.x, devicetree, linux-kernel,
linux-iio
On 2024/6/23 02:04, Krzysztof Kozlowski wrote:
> On 22/06/2024 14:35, Yasin Lee wrote:
>> On 2024/6/22 18:51, Conor Dooley wrote:
>>> On Sat, Jun 22, 2024 at 01:56:42PM +0800, Yasin Lee wrote:
>>>> On 2024/6/21 18:12, Krzysztof Kozlowski wrote:
>>>>
>>>> Hi ,Krzysztof
>>>> Thank you for your reply. I have some questions inline.
>>>>
>>>> Best regards,
>>>> Yasin
>>>>
>>>>> On 21/06/2024 09:40, Yasin Lee wrote:
>>>>>> A capacitive proximity sensor
>>>>>>
>>>>>> Signed-off-by: Yasin Lee <yasin.lee.x@gmail.com>
>>>>>> ---
>>>>>> .../bindings/iio/proximity/tyhx,hx9023s.yaml | 115 +++++++++++++++++++++
>>>>>> 1 file changed, 115 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml b/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
>>>>>> new file mode 100644
>>>>>> index 000000000000..beca70ce7609
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
>>>>>> @@ -0,0 +1,115 @@
>>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>>> +%YAML 1.2
>>>>>> +---
>>>>>> +$id: http://devicetree.org/schemas/iio/proximity/tyhx,hx9023s.yaml#
>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>> +
>>>>>> +title: TYHX HX9023S capacitive proximity sensor
>>>>>> +
>>>>>> +maintainers:
>>>>>> + - Yasin Lee <yasin.lee.x@gmail.com>
>>>>>> +
>>>>>> +description: |
>>>>>> + TYHX HX9023S proximity sensor. Datasheet can be found here:
>>>>>> + http://www.tianyihexin.com/ueditor/php/upload/file/20240614/1718336303992081.pdf
>>>>>> +
>>>>>> +allOf:
>>>>>> + - $ref: /schemas/iio/iio.yaml#
>>>>> Which part of iio.yaml binding do you use here? I cannot find anything,
>>>>> so this looks wrong.
>>>>>
>>>> I will remove this reference.
>>>>
>>>>
>>>>>> +
>>>>>> +properties:
>>>>>> + compatible:
>>>>>> + const: tyhx,hx9023s
>>>>>> +
>>>>>> + reg:
>>>>>> + maxItems: 1
>>>>>> +
>>>>>> + interrupts:
>>>>>> + description:
>>>>>> + Generated by device to announce preceding read request has finished
>>>>>> + and data is available or that a close/far proximity event has happened.
>>>>>> + maxItems: 1
>>>>>> +
>>>>>> + vdd-supply: true
>>>>>> +
>>>>>> + "#address-cells":
>>>>>> + const: 1
>>>>>> +
>>>>>> + "#size-cells":
>>>>>> + const: 0
>>>>>> +
>>>>>> +patternProperties:
>>>>>> + "^channel@[0-4]$":
>>>>>> + $ref: /schemas/iio/adc/adc.yaml
>>>>>> + type: object
>>>>>> +
>>>>>> + properties:
>>>>>> + reg:
>>>>>> + minimum: 0
>>>>>> + maximum: 4
>>>>>> + description: The channel number.
>>>>>> +
>>>>>> + input-channel:
>>>>> Isn't this duplicating single-channel property?
>>>>>
>>>>> Where is this property defined (which common schema)?
>>>>>
>>>> |input-channel| is indeed intended for single-ended configuration, but I
>>>> couldn't find a definition
>>>>
>>>> or reference for |single-channel| anywhere. If possible, should I rename
>>>> |input-channel| to |single-channel|?
>>> Single-channel is new, it should be the next branch of the iio tree and
>>> in linux-next.
>> Hi Conor，
>>
>> Thank you for informing me. I plan to temporarily add a prefix to this
>> attribute to distinguish it and update it in the future. Is this the
>> correct approach?
> No, because there is no need. You are supposed to work on maintainer
> tree (linux-next works usually as well).
>
> Best regards,
> Krzysztof
Hi Krzysztof,
Understood, Thank you for the reply, I will reference single-channel
directly as I do with diff-channel.
Best regards，
Yasin
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 3/3] iio: proximity: Add driver support for TYHX's HX9023S capacitive proximity sensor
2024-06-21 7:40 ` [PATCH v6 3/3] iio: proximity: Add driver support for TYHX's HX9023S capacitive proximity sensor Yasin Lee
2024-06-21 14:09 ` Alexandru Ardelean
@ 2024-06-23 11:56 ` Jonathan Cameron
2024-06-25 2:06 ` Yasin Lee
1 sibling, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2024-06-23 11:56 UTC (permalink / raw)
To: Yasin Lee
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Lars-Peter Clausen, yasin.lee.x, devicetree, linux-kernel,
linux-iio
On Fri, 21 Jun 2024 15:40:51 +0800
Yasin Lee <yasin.lee.x@gmail.com> wrote:
> A SAR sensor from NanjingTianyihexin Electronics Ltd.
>
> The device has the following entry points:
>
> Usual frequency:
> - sampling_frequency
>
> Instant reading of current values for different sensors:
> - in_proximity0_raw
> - in_proximity1_raw
> - in_proximity2_raw
> - in_proximity3_raw
> - in_proximity4_raw
> and associated events in events/
>
> Signed-off-by: Yasin Lee <yasin.lee.x@gmail.com>
Hi Yasin,
Some good reviews in already for this version, so I only took a quick look
this time. It seems to be in a reasonable state now.
Jonathan
> diff --git a/drivers/iio/proximity/hx9023s.c b/drivers/iio/proximity/hx9023s.c
> new file mode 100644
> index 000000000000..1d8cb9a05d8a
> --- /dev/null
> +++ b/drivers/iio/proximity/hx9023s.c
>
> +struct hx9023s_data {
> + struct iio_trigger *trig;
> + struct regmap *regmap;
> + unsigned long chan_prox_stat;
> + unsigned long chan_read;
> + unsigned long chan_event;
> + unsigned long ch_en_stat;
> + unsigned long chan_in_use;
> + unsigned int prox_state_reg;
> + bool trigger_enabled;
> +
> + struct {
> + __le16 channels[HX9023S_CH_NUM];
> + s64 ts __aligned(8);
> + } buffer;
> +
> + struct mutex mutex;
Add a comment explaining the data this mutex is protecting
(that may be in this structure, or for example on the device)
> + struct hx9023s_ch_data ch_data[HX9023S_CH_NUM];
> +};
> +
> +static int hx9023s_sample(struct hx9023s_data *data)
> +{
...
> +
> + for (i = 0; i < HX9023S_CH_NUM; i++) {
> + value = get_unaligned_le16(&rx_buf[i * data_size + 1]);
> + value = sign_extend32(value, 15);
> + data->ch_data[i].lp = 0;
> + data->ch_data[i].diff = 0;
> + if (data->ch_data[i].sel_lp == true)
> + data->ch_data[i].lp = value;
> + if (data->ch_data[i].sel_diff == true)
>
Run checkpatch.pl --strict and it will probably moan about these.
if (data->ch_data[i].sel_diff) is the same thing so just use that.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 3/3] iio: proximity: Add driver support for TYHX's HX9023S capacitive proximity sensor
2024-06-23 11:56 ` Jonathan Cameron
@ 2024-06-25 2:06 ` Yasin Lee
0 siblings, 0 replies; 15+ messages in thread
From: Yasin Lee @ 2024-06-25 2:06 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Lars-Peter Clausen, yasin.lee.x, devicetree, linux-kernel,
linux-iio
On 2024/6/23 19:56, Jonathan Cameron wrote:
> On Fri, 21 Jun 2024 15:40:51 +0800
> Yasin Lee <yasin.lee.x@gmail.com> wrote:
>
>> A SAR sensor from NanjingTianyihexin Electronics Ltd.
>>
>> The device has the following entry points:
>>
>> Usual frequency:
>> - sampling_frequency
>>
>> Instant reading of current values for different sensors:
>> - in_proximity0_raw
>> - in_proximity1_raw
>> - in_proximity2_raw
>> - in_proximity3_raw
>> - in_proximity4_raw
>> and associated events in events/
>>
>> Signed-off-by: Yasin Lee <yasin.lee.x@gmail.com>
> Hi Yasin,
>
> Some good reviews in already for this version, so I only took a quick look
> this time. It seems to be in a reasonable state now.
>
> Jonathan
Hi Jonathan,
Thank you for your very patient guidance. I have updated according to
each reviewer's suggestions.
Best regards,
Yasin
>> diff --git a/drivers/iio/proximity/hx9023s.c b/drivers/iio/proximity/hx9023s.c
>> new file mode 100644
>> index 000000000000..1d8cb9a05d8a
>> --- /dev/null
>> +++ b/drivers/iio/proximity/hx9023s.c
>>
>> +struct hx9023s_data {
>> + struct iio_trigger *trig;
>> + struct regmap *regmap;
>> + unsigned long chan_prox_stat;
>> + unsigned long chan_read;
>> + unsigned long chan_event;
>> + unsigned long ch_en_stat;
>> + unsigned long chan_in_use;
>> + unsigned int prox_state_reg;
>> + bool trigger_enabled;
>> +
>> + struct {
>> + __le16 channels[HX9023S_CH_NUM];
>> + s64 ts __aligned(8);
>> + } buffer;
>> +
>> + struct mutex mutex;
> Add a comment explaining the data this mutex is protecting
> (that may be in this structure, or for example on the device)
Done, you will see this comment in V7.
>> + struct hx9023s_ch_data ch_data[HX9023S_CH_NUM];
>> +};
>
>> +
>> +static int hx9023s_sample(struct hx9023s_data *data)
>> +{
> ...
>> +
>> + for (i = 0; i < HX9023S_CH_NUM; i++) {
>> + value = get_unaligned_le16(&rx_buf[i * data_size + 1]);
>> + value = sign_extend32(value, 15);
>> + data->ch_data[i].lp = 0;
>> + data->ch_data[i].diff = 0;
>> + if (data->ch_data[i].sel_lp == true)
>> + data->ch_data[i].lp = value;
>> + if (data->ch_data[i].sel_diff == true)
>>
> Run checkpatch.pl --strict and it will probably moan about these.
> if (data->ch_data[i].sel_diff) is the same thing so just use that.
Done, and I have fixed the issues identified by the script.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2024-06-25 2:06 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-21 7:40 [PATCH v6 0/3] iio: proximity: Add TYHX HX9023S sensor driver Yasin Lee
2024-06-21 7:40 ` [PATCH v6 1/3] dt-bindings: vendor-prefixes: add tyhx Yasin Lee
2024-06-21 10:07 ` Krzysztof Kozlowski
2024-06-21 7:40 ` [PATCH v6 2/3] dt-bindings: iio: proximity: Add TYHX HX9023S Yasin Lee
2024-06-21 10:12 ` Krzysztof Kozlowski
2024-06-22 5:56 ` Yasin Lee
2024-06-22 10:51 ` Conor Dooley
2024-06-22 12:35 ` Yasin Lee
2024-06-22 18:04 ` Krzysztof Kozlowski
2024-06-23 0:50 ` Yasin Lee
2024-06-21 7:40 ` [PATCH v6 3/3] iio: proximity: Add driver support for TYHX's HX9023S capacitive proximity sensor Yasin Lee
2024-06-21 14:09 ` Alexandru Ardelean
2024-06-22 12:15 ` Yasin Lee
2024-06-23 11:56 ` Jonathan Cameron
2024-06-25 2:06 ` Yasin Lee
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).