Message-Id: <20240702-add-tyhx-hx9023s-sensor-driver-v9-0-c030f1801d9b@gmail.com>
Date: Tue, 02 Jul 2024 22:12:31 +0800
From: Yasin Lee <yasin.lee.x@...il.com>
To: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, 
 Conor Dooley <conor+dt@...nel.org>, Jonathan Cameron <jic23@...nel.org>, 
 Lars-Peter Clausen <lars@...afoo.de>, yasin.lee.x@...look.com
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
 linux-iio@...r.kernel.org, Yasin Lee <yasin.lee.x@...il.com>, 
 Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Subject: [PATCH v9 0/3] iio: proximity: Add TYHX HX9023S sensor driver

Signed-off-by: Yasin Lee <yasin.lee.x@...il.com>
---
Changes in v9:

- Macro definitions:
  - Deleted: HX9023S_2BYTES, HX9023S_3BYTES, HX9023S_BYTES_MAX,
    and replaced them with numbers in the code for better readability.
- struct hx9023s_ch_data:
  - Changed the types of raw, lp, bl, and diff from int to s16.
- Function hx9023s_sample:
  - Removed unnecessary local variables: data_size, offset_data_size, *p, size.
  - Added comments to explain the data length.
  - Stored the result of get_unaligned_le16() directly in s16 and removed sign_extend32.
- Function hx9023s_property_get:
  - Replaced device_for_each_child_node with device_for_each_child_node_scoped
    and removed fwnode_handle_put().
  - Bugfix: changed sizeof(array) to ARRAY_SIZE(array).
- Function hx9023s_push_events:
  - Added error handling for hx9023s_sample() and hx9023s_get_prox_state().
- Function hx9023s_id_check:
  - Returns an error only when the ID cannot be read.
    If the read ID is different from the expected one,
    it only prints a warning and does not return an error.
    This helps with backward compatibility of the driver.
- Link to v8: https://lore.kernel.org/r/20240625-add-tyhx-hx9023s-sensor-driver-v8-0-0c224e3bddbc@gmail.com

Changes in v8:

- Removed incorrect tags from the commit messages 
- Removed duplicate properties in the dt-binding.
- Link to v7: https://lore.kernel.org/r/20240625-add-tyhx-hx9023s-sensor-driver-v7-0-b1d65b221811@gmail.com

Changes in v7:

**tyhx,hx9023s.yaml:**

- Removed the unused reference to iio.yaml
- Removed the `input-channel` property and switched to inheriting the `single-channel` property from adc.yaml
- Sorted the items in the `required` list

**hx9023s.c:**

- Header Files Section:
  - Added `<linux/cleanup.h>`
  - Removed `<linux/delay.h>`
  - Replaced `<linux/math.h>` with `<linux/math64.h>`
- Register Initialization:
  - Deleted the `struct hx9023s_addr_val_pair`
  - Changed the type of `hx9023s_reg_init_list` to `struct reg_sequence`
  - Deleted the `hx9023s_reg_init` function and used `regmap_multi_reg_write` for register initialization in `probe`
- struct hx9023s_ch_data:
  - Added a comment to the member `mutex` explaining the data it protects
- Function hx9023s_sample:
  - Fixed the error related to `hx9023s_data_lock` calls
- Function hx9023s_property_get:
  - Added boundary protection for `reg`
- Function hx9023s_id_check:
  - Optimized the function
- Function hx9023s_probe:
  - Changed the call to `devm_iio_device_register` to `return devm_iio_device_register()`
- Functions hx9023s_suspend and hx9023s_resume:
  - Added `guard(mutex)(&data->mutex)`
- Code Check:
  - Ran `checkpatch.pl --strict` and fixed the identified issues
- Link to v6: https://lore.kernel.org/r/20240621-add-tyhx-hx9023s-sensor-driver-v6-0-65196a9020f1@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       |   93 ++
 .../devicetree/bindings/vendor-prefixes.yaml       |    2 +
 drivers/iio/proximity/Kconfig                      |   14 +
 drivers/iio/proximity/Makefile                     |    1 +
 drivers/iio/proximity/hx9023s.c                    | 1124 ++++++++++++++++++++
 5 files changed, 1234 insertions(+)
---
base-commit: 1231e065edd236295c31904fe760117551608efa
change-id: 20240616-add-tyhx-hx9023s-sensor-driver-e7dbe3bfe596

Best regards,
-- 
Yasin Lee <yasin.lee.x@...il.com>

Powered by blists - more mailing lists