From: Guenter Roeck <linux@roeck-us.net>
To: Grant McEwan <grant.mcewan@alliedtelesis.co.nz>
Cc: jdelvare@suse.com, linux-hwmon@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] hwmon: (adt7475) Convert to use hwmon_device_register_with_groups()
Date: Sat, 20 Jul 2019 07:23:57 -0700 [thread overview]
Message-ID: <20190720142357.GA30793@roeck-us.net> (raw)
In-Reply-To: <20190719014130.1090-1-grant.mcewan@alliedtelesis.co.nz>
On Fri, Jul 19, 2019 at 01:41:30PM +1200, Grant McEwan wrote:
> hwmon_device_register() is a deprecated function and produces a warning.
>
> Converting the driver to use the hwmon_device_register_with_groups()
> instead.
>
> Signed-off-by: Grant McEwan <grant.mcewan@alliedtelesis.co.nz>
> ---
> drivers/hwmon/adt7475.c | 104 +++++++++++++++++++---------------------
> 1 file changed, 48 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
> index c3c6031a7285..4e838555102f 100644
> --- a/drivers/hwmon/adt7475.c
> +++ b/drivers/hwmon/adt7475.c
> @@ -188,6 +188,7 @@ MODULE_DEVICE_TABLE(of, adt7475_of_match);
>
> struct adt7475_data {
> struct device *hwmon_dev;
> + struct i2c_client *client;
> struct mutex lock;
>
> unsigned long measure_updated;
> @@ -212,6 +213,7 @@ struct adt7475_data {
>
> u8 vid;
> u8 vrm;
> + const struct attribute_group *groups[8];
There are a total of 8 groups, and the list of groups needs to be
terminated with a NULL pointer, so there have to be 9 entries in
this array.
> };
>
> static struct i2c_driver adt7475_driver;
> @@ -346,8 +348,8 @@ static ssize_t voltage_store(struct device *dev,
> {
>
> struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7475_data *data = i2c_get_clientdata(client);
> + struct adt7475_data *data = dev_get_drvdata(dev);
> + struct i2c_client *client = data->client;
> unsigned char reg;
> long val;
>
> @@ -440,8 +442,8 @@ static ssize_t temp_store(struct device *dev, struct device_attribute *attr,
> const char *buf, size_t count)
> {
> struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7475_data *data = i2c_get_clientdata(client);
> + struct adt7475_data *data = dev_get_drvdata(dev);
> + struct i2c_client *client = data->client;
> unsigned char reg = 0;
> u8 out;
> int temp;
> @@ -542,8 +544,7 @@ static ssize_t temp_st_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7475_data *data = i2c_get_clientdata(client);
> + struct adt7475_data *data = dev_get_drvdata(dev);
> long val;
>
> switch (sattr->index) {
> @@ -570,8 +571,8 @@ static ssize_t temp_st_store(struct device *dev,
> size_t count)
> {
> struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7475_data *data = i2c_get_clientdata(client);
> + struct adt7475_data *data = dev_get_drvdata(dev);
> + struct i2c_client *client = data->client;
> unsigned char reg;
> int shift, idx;
> ulong val;
> @@ -647,8 +648,8 @@ static ssize_t point2_show(struct device *dev, struct device_attribute *attr,
> static ssize_t point2_store(struct device *dev, struct device_attribute *attr,
> const char *buf, size_t count)
> {
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7475_data *data = i2c_get_clientdata(client);
> + struct adt7475_data *data = dev_get_drvdata(dev);
> + struct i2c_client *client = data->client;
> struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
> int temp;
> long val;
> @@ -710,8 +711,8 @@ static ssize_t tach_store(struct device *dev, struct device_attribute *attr,
> {
>
> struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7475_data *data = i2c_get_clientdata(client);
> + struct adt7475_data *data = dev_get_drvdata(dev);
> + struct i2c_client *client = data->client;
> unsigned long val;
>
> if (kstrtoul(buf, 10, &val))
> @@ -769,8 +770,8 @@ static ssize_t pwm_store(struct device *dev, struct device_attribute *attr,
> {
>
> struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7475_data *data = i2c_get_clientdata(client);
> + struct adt7475_data *data = dev_get_drvdata(dev);
> + struct i2c_client *client = data->client;
> unsigned char reg = 0;
> long val;
>
> @@ -818,8 +819,8 @@ static ssize_t stall_disable_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7475_data *data = i2c_get_clientdata(client);
> + struct adt7475_data *data = dev_get_drvdata(dev);
> +
> u8 mask = BIT(5 + sattr->index);
>
> return sprintf(buf, "%d\n", !!(data->enh_acoustics[0] & mask));
> @@ -830,8 +831,8 @@ static ssize_t stall_disable_store(struct device *dev,
> const char *buf, size_t count)
> {
> struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7475_data *data = i2c_get_clientdata(client);
> + struct adt7475_data *data = dev_get_drvdata(dev);
> + struct i2c_client *client = data->client;
> long val;
> u8 mask = BIT(5 + sattr->index);
>
> @@ -914,8 +915,8 @@ static ssize_t pwmchan_store(struct device *dev,
> size_t count)
> {
> struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7475_data *data = i2c_get_clientdata(client);
> + struct adt7475_data *data = dev_get_drvdata(dev);
> + struct i2c_client *client = data->client;
> int r;
> long val;
>
> @@ -938,8 +939,8 @@ static ssize_t pwmctrl_store(struct device *dev,
> size_t count)
> {
> struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7475_data *data = i2c_get_clientdata(client);
> + struct adt7475_data *data = dev_get_drvdata(dev);
> + struct i2c_client *client = data->client;
> int r;
> long val;
>
> @@ -982,8 +983,8 @@ static ssize_t pwmfreq_store(struct device *dev,
> size_t count)
> {
> struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7475_data *data = i2c_get_clientdata(client);
> + struct adt7475_data *data = dev_get_drvdata(dev);
> + struct i2c_client *client = data->client;
> int out;
> long val;
>
> @@ -1022,8 +1023,8 @@ static ssize_t pwm_use_point2_pwm_at_crit_store(struct device *dev,
> struct device_attribute *devattr,
> const char *buf, size_t count)
> {
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7475_data *data = i2c_get_clientdata(client);
> + struct adt7475_data *data = dev_get_drvdata(dev);
> + struct i2c_client *client = data->client;
> long val;
>
> if (kstrtol(buf, 10, &val))
> @@ -1305,6 +1306,9 @@ static const struct attribute_group in4_attr_group = { .attrs = in4_attrs };
> static const struct attribute_group in5_attr_group = { .attrs = in5_attrs };
> static const struct attribute_group vid_attr_group = { .attrs = vid_attrs };
>
> +/* Number of possible attribute groups to add into sysfs */
> +#define ATTR_GROUP_COUNT 8
> +
Not used anywhere.
> static int adt7475_detect(struct i2c_client *client,
> struct i2c_board_info *info)
> {
> @@ -1489,7 +1493,7 @@ static int adt7475_probe(struct i2c_client *client,
> };
>
> struct adt7475_data *data;
> - int i, ret = 0, revision;
> + int i, ret = 0, revision, group_num = 0;
> u8 config2, config3;
>
> data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
> @@ -1497,6 +1501,7 @@ static int adt7475_probe(struct i2c_client *client,
> return -ENOMEM;
>
> mutex_init(&data->lock);
> + data->client = client;
> i2c_set_clientdata(client, data);
>
> if (client->dev.of_node)
> @@ -1590,49 +1595,37 @@ static int adt7475_probe(struct i2c_client *client,
> break;
> }
>
> - ret = sysfs_create_group(&client->dev.kobj, &adt7475_attr_group);
> - if (ret)
> - return ret;
> + data->groups[group_num++] = &adt7475_attr_group;
>
> /* Features that can be disabled individually */
> if (data->has_fan4) {
> - ret = sysfs_create_group(&client->dev.kobj, &fan4_attr_group);
> - if (ret)
> - goto eremove;
> + data->groups[group_num++] = &fan4_attr_group;
> }
> if (data->has_pwm2) {
> - ret = sysfs_create_group(&client->dev.kobj, &pwm2_attr_group);
> - if (ret)
> - goto eremove;
> + data->groups[group_num++] = &pwm2_attr_group;
> }
> if (data->has_voltage & (1 << 0)) {
> - ret = sysfs_create_group(&client->dev.kobj, &in0_attr_group);
> - if (ret)
> - goto eremove;
> + data->groups[group_num++] = &in0_attr_group;
> }
> if (data->has_voltage & (1 << 3)) {
> - ret = sysfs_create_group(&client->dev.kobj, &in3_attr_group);
> - if (ret)
> - goto eremove;
> + data->groups[group_num++] = &in3_attr_group;
> }
> if (data->has_voltage & (1 << 4)) {
> - ret = sysfs_create_group(&client->dev.kobj, &in4_attr_group);
> - if (ret)
> - goto eremove;
> + data->groups[group_num++] = &in4_attr_group;
> }
> if (data->has_voltage & (1 << 5)) {
> - ret = sysfs_create_group(&client->dev.kobj, &in5_attr_group);
> - if (ret)
> - goto eremove;
> + data->groups[group_num++] = &in5_attr_group;
> }
> if (data->has_vid) {
> data->vrm = vid_which_vrm();
> - ret = sysfs_create_group(&client->dev.kobj, &vid_attr_group);
> - if (ret)
> - goto eremove;
> + data->groups[group_num] = &vid_attr_group;
> }
>
> - data->hwmon_dev = hwmon_device_register(&client->dev);
> + /* register device with all the acquired attributes */
> + data->hwmon_dev = hwmon_device_register_with_groups(&client->dev,
> + client->name, data,
> + data->groups);
> +
You could call devm_hwmon_device_register_with_groups() here, drop the
remove function, and drop hwmon_dev from struct adt7475_data.
Also, adt7475_remove_files() is no longer necessary. With the new API,
attribute files are owned by the hwmon subsystem.
> if (IS_ERR(data->hwmon_dev)) {
> ret = PTR_ERR(data->hwmon_dev);
> goto eremove;
> @@ -1757,8 +1750,8 @@ static void adt7475_read_pwm(struct i2c_client *client, int index)
>
> static int adt7475_update_measure(struct device *dev)
> {
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7475_data *data = i2c_get_clientdata(client);
> + struct adt7475_data *data = dev_get_drvdata(dev);
> + struct i2c_client *client = data->client;
> u16 ext;
> int i;
> int ret;
> @@ -1854,8 +1847,7 @@ static int adt7475_update_measure(struct device *dev)
>
> static struct adt7475_data *adt7475_update_device(struct device *dev)
> {
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7475_data *data = i2c_get_clientdata(client);
> + struct adt7475_data *data = dev_get_drvdata(dev);
> int ret;
>
> mutex_lock(&data->lock);
prev parent reply other threads:[~2019-07-20 14:24 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-19 1:41 [PATCH] hwmon: (adt7475) Convert to use hwmon_device_register_with_groups() Grant McEwan
2019-07-20 14:23 ` Guenter Roeck [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190720142357.GA30793@roeck-us.net \
--to=linux@roeck-us.net \
--cc=grant.mcewan@alliedtelesis.co.nz \
--cc=jdelvare@suse.com \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.