Message ID | 20230718070114.3871-1-andre.werner@systec-electronic.com |
---|---|
State | Superseded, archived |
Headers | show |
Series | [v2,1/2] dt-bindings: hwmon: Add description for new hwmon driver hs3001 | expand |
Context | Check | Description |
---|---|---|
robh/checkpatch | warning | total: 0 errors, 1 warnings, 8 lines checked |
robh/patch-applied | success | |
robh/dtbs-check | warning | build log |
robh/dt-meta-schema | success |
On 18/07/2023 09:01, Andre Werner wrote: > This is the initial description. > > Signed-off-by: Andre Werner <andre.werner@systec-electronic.com> > > v1: Using separate dt-binding hs300x.yaml > v2: Reviewer recommends documentation of driver dt-binding in > trivial-devices.yaml because the driver has no special properties > to describe. Changelog goes after ---. Even if you intended to keep it in commit msg, definitely not after SoB. Does this even pass checkpatch? > --- > Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml > index ba2bfb547909..a4f4701337cf 100644 > --- a/Documentation/devicetree/bindings/trivial-devices.yaml > +++ b/Documentation/devicetree/bindings/trivial-devices.yaml > @@ -315,6 +315,8 @@ properties: > - plx,pex8648 > # Pulsedlight LIDAR range-finding sensor > - pulsedlight,lidar-lite-v2 > + # Renesas HS300[1,2,3,4] Temperature and Relative Humidity Sensors > + - renesas,hs3001 What about the rest of the devices - hs300[234]? Usually we ask for specific compatibles, that's why separate binding made some sense. Best regards, Krzysztof
On 18/07/2023 09:01, Andre Werner wrote: > This is the initial description. > > Signed-off-by: Andre Werner <andre.werner@systec-electronic.com> > > v1: Using separate dt-binding hs300x.yaml > v2: Reviewer recommends documentation of driver dt-binding in > trivial-devices.yaml because the driver has no special properties > to describe. BTW, you already sent v2, so this is v3. Please version your patches correctly. Best regards, Krzysztof
On 18/07/2023 09:01, Andre Werner wrote: > Add base support for Renesas HS3001 temperature > and humidity sensors and its compatibles HS3002, > HS3003 and HS3004. > > The sensor has a fix I2C address 0x44. The resolution > is fixed to 14bit (ref. Missing feature). > > Missing feature: > - Accessing non-volatile memory: Custom board has no > possibility to control voltage supply of sensor. Thus, > we cannot send the necessary control commands within > the first 10ms after power-on. > > Signed-off-by: Andre Werner <andre.werner@systec-electronic.com> > > Changelog: > v1: Initial version > v2: Extensive refactoring following recommendations of reviewers: > - Delete unused defines and device properties. These are added in > the initial version because the device supports a programming mode, > but I was not able to implement it, because the custom board was > not able to control the power supply of the device and so I cannot > enter the programming mode of the device. > - Correct missunderstanding comments for defines. > - Delete mutexes for data and I2C bus accesses. > - Replace attributes with recommented chip-info structure. In the > initial version I followed the sth3x.c implementation that uses > files and attributes in sysfs. The show functions are replaced by > is_visible and read callbacks from the HWMON ABI. I also delete pointless > function argument checks. > - Correct Yoda programming. > - Refactor probe function and delete sleep and measurement of humidity > and temperature in probe function. I kept an initial I2C > communication to ensure that the device is accessible during probe. > - Reduce the number of atteributes to humidity and temperature input. Also wrong placement of SoB and changelog. > --- > Documentation/hwmon/hs3001.rst | 37 +++++ > MAINTAINERS | 6 + > drivers/hwmon/Kconfig | 10 ++ > drivers/hwmon/Makefile | 1 + > drivers/hwmon/hs3001.c | 261 +++++++++++++++++++++++++++++++++ > 5 files changed, 315 insertions(+) > create mode 100644 Documentation/hwmon/hs3001.rst > create mode 100644 drivers/hwmon/hs3001.c ... > +/* Definitions for Status Bits of A/D Data */ > +#define HS3001_DATA_VALID 0x00 /* Valid Data */ > +#define HS3001_DATA_STALE 0x01 /* Stale Data */ > + > +#define LIMIT_MAX 0 > +#define LIMIT_MIN 1 > + > +enum hs3001_chips { > + hs3001, Drop, not effectively used. > +}; ... > + > +/* device ID table */ > +static const struct i2c_device_id hs3001_ids[] = { > + { "hs3001", hs3001 }, Drop match data > + { }, > +}; > + > +MODULE_DEVICE_TABLE(i2c, hs3001_ids); > + > +static const struct of_device_id hs3001_of_match[] = { > + {.compatible = "renesas,hs3001", > + .data = (void *)hs3001 Drop > + }, > + { }, > +}; > + > +MODULE_DEVICE_TABLE(of, hs3001_of_match); > + > +static int hs3001_probe(struct i2c_client *client) > +{ > + struct hs3001_data *data; > + struct device *hwmon_dev; > + struct device *dev = &client->dev; > + int ret; > + > + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) > + return -EOPNOTSUPP; > + > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->client = client; > + > + if (client->dev.of_node) > + data->type = (enum hs3001_chips)of_device_get_match_data(&client->dev); > + else > + data->type = i2c_match_id(hs3001_ids, client)->driver_data; This is useless and dead code. You have only one type of device. Don't over-complicate simple things. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml index ba2bfb547909..a4f4701337cf 100644 --- a/Documentation/devicetree/bindings/trivial-devices.yaml +++ b/Documentation/devicetree/bindings/trivial-devices.yaml @@ -315,6 +315,8 @@ properties: - plx,pex8648 # Pulsedlight LIDAR range-finding sensor - pulsedlight,lidar-lite-v2 + # Renesas HS300[1,2,3,4] Temperature and Relative Humidity Sensors + - renesas,hs3001 # Renesas ISL29501 time-of-flight sensor - renesas,isl29501 # Rohm DH2228FV
This is the initial description. Signed-off-by: Andre Werner <andre.werner@systec-electronic.com> v1: Using separate dt-binding hs300x.yaml v2: Reviewer recommends documentation of driver dt-binding in trivial-devices.yaml because the driver has no special properties to describe. --- Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++ 1 file changed, 2 insertions(+)