2022-07-12 20:08:58

by Marc Ferland

[permalink] [raw]
Subject: [PATCH] hwmon: (mcp3021) improve driver support for newer hwmon interface

From: Marc Ferland <[email protected]>

This driver is currently broken, it does not show the in0_input sysfs
file and also returns the following message on startup:

hwmon_device_register() is deprecated. Please convert the driver to
use hwmon_device_register_with_info().

This patch converts the driver and also cleans up the 'read' function.

Signed-off-by: Marc Ferland <[email protected]>
---
drivers/hwmon/mcp3021.c | 99 ++++++++++++++++++++---------------------
1 file changed, 49 insertions(+), 50 deletions(-)

diff --git a/drivers/hwmon/mcp3021.c b/drivers/hwmon/mcp3021.c
index ce2780768074..e093b1998296 100644
--- a/drivers/hwmon/mcp3021.c
+++ b/drivers/hwmon/mcp3021.c
@@ -7,7 +7,7 @@
* Reworked by Sven Schuchmann <[email protected]>
* DT support added by Clemens Gruber <[email protected]>
*
- * This driver export the value of analog input voltage to sysfs, the
+ * This driver exports the value of analog input voltage to sysfs, the
* voltage unit is mV. Through the sysfs interface, lm-sensors tool
* can also display the input voltage.
*/
@@ -45,19 +45,29 @@ enum chips {
* Client data (each client gets its own)
*/
struct mcp3021_data {
- struct device *hwmon_dev;
+ struct i2c_client *client;
u32 vdd; /* supply and reference voltage in millivolt */
u16 sar_shift;
u16 sar_mask;
u8 output_res;
};

-static int mcp3021_read16(struct i2c_client *client)
+static inline u16 volts_from_reg(struct mcp3021_data *data, u16 val)
{
- struct mcp3021_data *data = i2c_get_clientdata(client);
- int ret;
- u16 reg;
+ return DIV_ROUND_CLOSEST(data->vdd * val, 1 << data->output_res);
+}
+
+static int mcp3021_read(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, long *val)
+{
+ struct mcp3021_data *data = dev_get_drvdata(dev);
+ struct i2c_client *client = data->client;
__be16 buf;
+ u16 reg;
+ int ret;
+
+ if (type != hwmon_in)
+ return -EOPNOTSUPP;

ret = i2c_master_recv(client, (char *)&buf, 2);
if (ret < 0)
@@ -74,39 +84,46 @@ static int mcp3021_read16(struct i2c_client *client)
*/
reg = (reg >> data->sar_shift) & data->sar_mask;

- return reg;
-}
+ *val = volts_from_reg(data, reg);

-static inline u16 volts_from_reg(struct mcp3021_data *data, u16 val)
-{
- return DIV_ROUND_CLOSEST(data->vdd * val, 1 << data->output_res);
+ return 0;
}

-static ssize_t in0_input_show(struct device *dev,
- struct device_attribute *attr, char *buf)
+static umode_t mcp3021_is_visible(const void *_data,
+ enum hwmon_sensor_types type,
+ u32 attr, int channel)
{
- struct i2c_client *client = to_i2c_client(dev);
- struct mcp3021_data *data = i2c_get_clientdata(client);
- int reg, in_input;
+ if (type != hwmon_in)
+ return 0;

- reg = mcp3021_read16(client);
- if (reg < 0)
- return reg;
+ if (attr != hwmon_in_input)
+ return 0;

- in_input = volts_from_reg(data, reg);
-
- return sprintf(buf, "%d\n", in_input);
+ return 0444;
}

-static DEVICE_ATTR_RO(in0_input);
+static const struct hwmon_channel_info *mcp3021_info[] = {
+ HWMON_CHANNEL_INFO(in, HWMON_I_INPUT),
+ NULL
+};
+
+static const struct hwmon_ops mcp3021_hwmon_ops = {
+ .is_visible = mcp3021_is_visible,
+ .read = mcp3021_read,
+};
+
+static const struct hwmon_chip_info mcp3021_chip_info = {
+ .ops = &mcp3021_hwmon_ops,
+ .info = mcp3021_info,
+};

static const struct i2c_device_id mcp3021_id[];

static int mcp3021_probe(struct i2c_client *client)
{
- int err;
struct mcp3021_data *data = NULL;
struct device_node *np = client->dev.of_node;
+ struct device *hwmon_dev;

if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
return -ENODEV;
@@ -147,34 +164,17 @@ static int mcp3021_probe(struct i2c_client *client)
break;
}

+ data->client = client;
+
if (data->vdd > MCP3021_VDD_REF_MAX || data->vdd < MCP3021_VDD_REF_MIN)
return -EINVAL;

- err = sysfs_create_file(&client->dev.kobj, &dev_attr_in0_input.attr);
- if (err)
- return err;
-
- data->hwmon_dev = hwmon_device_register(&client->dev);
- if (IS_ERR(data->hwmon_dev)) {
- err = PTR_ERR(data->hwmon_dev);
- goto exit_remove;
- }
-
- return 0;
-
-exit_remove:
- sysfs_remove_file(&client->dev.kobj, &dev_attr_in0_input.attr);
- return err;
-}
-
-static int mcp3021_remove(struct i2c_client *client)
-{
- struct mcp3021_data *data = i2c_get_clientdata(client);
-
- hwmon_device_unregister(data->hwmon_dev);
- sysfs_remove_file(&client->dev.kobj, &dev_attr_in0_input.attr);
-
- return 0;
+ hwmon_dev = devm_hwmon_device_register_with_info(&client->dev,
+ client->name,
+ data,
+ &mcp3021_chip_info,
+ NULL);
+ return PTR_ERR_OR_ZERO(hwmon_dev);
}

static const struct i2c_device_id mcp3021_id[] = {
@@ -199,7 +199,6 @@ static struct i2c_driver mcp3021_driver = {
.of_match_table = of_match_ptr(of_mcp3021_match),
},
.probe_new = mcp3021_probe,
- .remove = mcp3021_remove,
.id_table = mcp3021_id,
};


base-commit: 9f6ca00b5be47e471e9703e6b017e1e2cfa7f604
--
2.30.2


2022-07-15 18:13:17

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH] hwmon: (mcp3021) improve driver support for newer hwmon interface

On Tue, Jul 12, 2022 at 03:35:04PM -0400, [email protected] wrote:
> From: Marc Ferland <[email protected]>
>
> This driver is currently broken, it does not show the in0_input sysfs
> file and also returns the following message on startup:
>
> hwmon_device_register() is deprecated. Please convert the driver to
> use hwmon_device_register_with_info().
>
> This patch converts the driver and also cleans up the 'read' function.
>
> Signed-off-by: Marc Ferland <[email protected]>

Applied to hwmon-next.

Thanks,
Guenter

> ---
> drivers/hwmon/mcp3021.c | 99 ++++++++++++++++++++---------------------
> 1 file changed, 49 insertions(+), 50 deletions(-)
>
>
> base-commit: 9f6ca00b5be47e471e9703e6b017e1e2cfa7f604
>
> diff --git a/drivers/hwmon/mcp3021.c b/drivers/hwmon/mcp3021.c
> index ce2780768074..e093b1998296 100644
> --- a/drivers/hwmon/mcp3021.c
> +++ b/drivers/hwmon/mcp3021.c
> @@ -7,7 +7,7 @@
> * Reworked by Sven Schuchmann <[email protected]>
> * DT support added by Clemens Gruber <[email protected]>
> *
> - * This driver export the value of analog input voltage to sysfs, the
> + * This driver exports the value of analog input voltage to sysfs, the
> * voltage unit is mV. Through the sysfs interface, lm-sensors tool
> * can also display the input voltage.
> */
> @@ -45,19 +45,29 @@ enum chips {
> * Client data (each client gets its own)
> */
> struct mcp3021_data {
> - struct device *hwmon_dev;
> + struct i2c_client *client;
> u32 vdd; /* supply and reference voltage in millivolt */
> u16 sar_shift;
> u16 sar_mask;
> u8 output_res;
> };
>
> -static int mcp3021_read16(struct i2c_client *client)
> +static inline u16 volts_from_reg(struct mcp3021_data *data, u16 val)
> {
> - struct mcp3021_data *data = i2c_get_clientdata(client);
> - int ret;
> - u16 reg;
> + return DIV_ROUND_CLOSEST(data->vdd * val, 1 << data->output_res);
> +}
> +
> +static int mcp3021_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long *val)
> +{
> + struct mcp3021_data *data = dev_get_drvdata(dev);
> + struct i2c_client *client = data->client;
> __be16 buf;
> + u16 reg;
> + int ret;
> +
> + if (type != hwmon_in)
> + return -EOPNOTSUPP;
>
> ret = i2c_master_recv(client, (char *)&buf, 2);
> if (ret < 0)
> @@ -74,39 +84,46 @@ static int mcp3021_read16(struct i2c_client *client)
> */
> reg = (reg >> data->sar_shift) & data->sar_mask;
>
> - return reg;
> -}
> + *val = volts_from_reg(data, reg);
>
> -static inline u16 volts_from_reg(struct mcp3021_data *data, u16 val)
> -{
> - return DIV_ROUND_CLOSEST(data->vdd * val, 1 << data->output_res);
> + return 0;
> }
>
> -static ssize_t in0_input_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> +static umode_t mcp3021_is_visible(const void *_data,
> + enum hwmon_sensor_types type,
> + u32 attr, int channel)
> {
> - struct i2c_client *client = to_i2c_client(dev);
> - struct mcp3021_data *data = i2c_get_clientdata(client);
> - int reg, in_input;
> + if (type != hwmon_in)
> + return 0;
>
> - reg = mcp3021_read16(client);
> - if (reg < 0)
> - return reg;
> + if (attr != hwmon_in_input)
> + return 0;
>
> - in_input = volts_from_reg(data, reg);
> -
> - return sprintf(buf, "%d\n", in_input);
> + return 0444;
> }
>
> -static DEVICE_ATTR_RO(in0_input);
> +static const struct hwmon_channel_info *mcp3021_info[] = {
> + HWMON_CHANNEL_INFO(in, HWMON_I_INPUT),
> + NULL
> +};
> +
> +static const struct hwmon_ops mcp3021_hwmon_ops = {
> + .is_visible = mcp3021_is_visible,
> + .read = mcp3021_read,
> +};
> +
> +static const struct hwmon_chip_info mcp3021_chip_info = {
> + .ops = &mcp3021_hwmon_ops,
> + .info = mcp3021_info,
> +};
>
> static const struct i2c_device_id mcp3021_id[];
>
> static int mcp3021_probe(struct i2c_client *client)
> {
> - int err;
> struct mcp3021_data *data = NULL;
> struct device_node *np = client->dev.of_node;
> + struct device *hwmon_dev;
>
> if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> return -ENODEV;
> @@ -147,34 +164,17 @@ static int mcp3021_probe(struct i2c_client *client)
> break;
> }
>
> + data->client = client;
> +
> if (data->vdd > MCP3021_VDD_REF_MAX || data->vdd < MCP3021_VDD_REF_MIN)
> return -EINVAL;
>
> - err = sysfs_create_file(&client->dev.kobj, &dev_attr_in0_input.attr);
> - if (err)
> - return err;
> -
> - data->hwmon_dev = hwmon_device_register(&client->dev);
> - if (IS_ERR(data->hwmon_dev)) {
> - err = PTR_ERR(data->hwmon_dev);
> - goto exit_remove;
> - }
> -
> - return 0;
> -
> -exit_remove:
> - sysfs_remove_file(&client->dev.kobj, &dev_attr_in0_input.attr);
> - return err;
> -}
> -
> -static int mcp3021_remove(struct i2c_client *client)
> -{
> - struct mcp3021_data *data = i2c_get_clientdata(client);
> -
> - hwmon_device_unregister(data->hwmon_dev);
> - sysfs_remove_file(&client->dev.kobj, &dev_attr_in0_input.attr);
> -
> - return 0;
> + hwmon_dev = devm_hwmon_device_register_with_info(&client->dev,
> + client->name,
> + data,
> + &mcp3021_chip_info,
> + NULL);
> + return PTR_ERR_OR_ZERO(hwmon_dev);
> }
>
> static const struct i2c_device_id mcp3021_id[] = {
> @@ -199,7 +199,6 @@ static struct i2c_driver mcp3021_driver = {
> .of_match_table = of_match_ptr(of_mcp3021_match),
> },
> .probe_new = mcp3021_probe,
> - .remove = mcp3021_remove,
> .id_table = mcp3021_id,
> };
>