| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <a27c761e-12d7-4a00-b5bb-95feabcd1a3c@wanadoo.fr>
Date: Fri, 24 Jan 2025 13:14:51 +0100
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Andrei Lalaev <andrey.lalaev@...il.com>, jdelvare@...e.com,
linux@...ck-us.net, conor+dt@...nel.org, krzk+dt@...nel.org, robh@...nel.org
Cc: linux-hwmon@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] hwmon: add driver for HTU31
Le 23/01/2025 à 21:25, Andrei Lalaev a écrit :
> Add base support for HTU31 temperature and humidity sensor.
>
> Besides temperature and humidity values, the driver also exports a 24-bit
> serial number to sysfs.
>
> Signed-off-by: Andrei Lalaev <andrey.lalaev@...il.com>
> ---
...
> +static int htu31_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long *val)
> +{
> + struct htu31_data *data = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = htu31_data_fetch_command(data);
> +
Unneeded empty line.
> + if (ret < 0)
> + return ret;
> +
> + switch (type) {
> + case hwmon_temp:
> + if (attr != hwmon_temp_input)
> + return -EINVAL;
> +
> + *val = data->temperature;
> + break;
> + case hwmon_humidity:
> + if (attr != hwmon_humidity_input)
> + return -EINVAL;
> +
> + *val = data->humidity;
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static int serial_number_populate(struct htu31_data *data)
> +{
> + struct i2c_client *client = data->client;
> + u8 read_sn_cmd = HTU31_READ_SERIAL_CMD;
> + u8 sn_buf[HTU31_SERIAL_NUMBER_LEN + HTU31_SERIAL_NUMBER_CRC_LEN];
> + struct i2c_msg msgs[] = {
> + {
> + .addr = client->addr,
> + .flags = 0,
> + .len = 1,
> + .buf = &read_sn_cmd,
> + },
> + {
> + .addr = client->addr,
> + .flags = I2C_M_RD,
> + .len = sizeof(sn_buf),
> + .buf = sn_buf,
> + },
> + };
> + int ret;
> + u8 crc;
> +
> + guard(mutex)(&data->i2c_lock);
> +
> + ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> + if (ret < 0)
> + return -EIO;
Why not return ret?
> +
> + crc = crc8(htu31_crc8_table, sn_buf, HTU31_SERIAL_NUMBER_LEN, HTU31_CRC8_INIT_VAL);
> + if (crc != sn_buf[HTU31_SERIAL_NUMBER_CRC_OFFSET]) {
> + dev_err(&client->dev, "Serial number CRC mismatch\n");
> + return -EIO;
> + }
> +
> + memcpy(data->serial_number, sn_buf, HTU31_SERIAL_NUMBER_LEN);
> +
> + return 0;
> +}
...
> +static ssize_t heater_enable_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t count)
> +{
> + struct htu31_data *data = dev_get_drvdata(dev);
> + u8 heater_cmd;
> + bool status;
> + int ret;
> +
> + ret = kstrtobool(buf, &status);
> + if (ret)
> + return ret;
> +
> + heater_cmd = status ? HTU31_HEATER_ON_CMD : HTU31_HEATER_OFF_CMD;
> +
> + guard(mutex)(&data->i2c_lock);
> +
> + ret = i2c_master_send(data->client, &heater_cmd, 1);
> + if (ret < 0)
> + return -EIO;
Why not return ret?
> +
> + data->heater_enable = status;
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR_RW(heater_enable);
> +
> +static ssize_t serial_number_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct htu31_data *data = dev_get_drvdata(dev);
> +
> + return sysfs_emit(buf, "%X%X%X\n", data->serial_number[0],
> + data->serial_number[1],
> + data->serial_number[2]);
> +}
> +
> +static DEVICE_ATTR_RO(serial_number);
> +
> +static struct attribute *htu31_attrs[] = {
> + &dev_attr_heater_enable.attr,
> + &dev_attr_serial_number.attr,
> + NULL,
No need for trailing comma after a terminator
> +};
> +
> +ATTRIBUTE_GROUPS(htu31);
> +
> +static const struct hwmon_channel_info * const htu31_info[] = {
> + HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
> + HWMON_CHANNEL_INFO(humidity, HWMON_H_INPUT),
> + NULL,
No need for trailing comma after a terminator
> +};
...
> +static int htu31_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct device *hwmon_dev;
> + struct htu31_data *data;
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->client = client;
> + data->wait_time = HTU31_TEMPERATURE_CONV_TIME + HTU31_HUMIDITY_CONV_TIME;
> +
> + mutex_init(&data->i2c_lock);
Maybe devm_mutex_init()?
> +
> + crc8_populate_msb(htu31_crc8_table, HTU31_CRC8_POLYNOMIAL);
> +
> + if (serial_number_populate(data)) {
Maybe:
ret = serial_number_populate(data);
if (ret) {
...
return ret;
}
> + dev_err(dev, "Failed to read serial number\n");
> + return -EIO;
> + }
> +
> + hwmon_dev = devm_hwmon_device_register_with_info(dev,
> + client->name,
> + data,
> + &htu31_chip_info,
> + htu31_groups);
> +
> + return PTR_ERR_OR_ZERO(hwmon_dev);
> +}
> +
> +static const struct i2c_device_id htu31_id[] = {
> + { "htu31" },
> + { },
No need for trailing comma after a terminator
> +};
> +MODULE_DEVICE_TABLE(i2c, htu31_id);
> +
> +#if IS_ENABLED(CONFIG_OF)
> +static const struct of_device_id htu31_of_match[] = {
> + { .compatible = "meas,htu31" },
> + { },
No need for trailing comma after a terminator
> +};
> +MODULE_DEVICE_TABLE(of, htu31_of_match);
...
CJ
Powered by blists - more mailing lists