Re: [RFC PATCH 3/5] hwmon: (ds1621) Convert to usehwmon_device_register_groups

[Guenter Roeck]

On Sat, Jul 06, 2013 at 08:01:51PM +0200, Jean Delvare wrote:
> On Sat, 6 Jul 2013 10:57:47 -0700, Greg Kroah-Hartman wrote:
> > On Sat, Jul 06, 2013 at 10:24:53AM -0700, Guenter Roeck wrote:
> > > Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> > > ---
> > >  drivers/hwmon/ds1621.c |   25 +++++++++----------------
> > >  1 file changed, 9 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/drivers/hwmon/ds1621.c b/drivers/hwmon/ds1621.c
> > > index a26ba7a..f1d0fa0 100644
> > > --- a/drivers/hwmon/ds1621.c
> > > +++ b/drivers/hwmon/ds1621.c
> > > @@ -358,11 +358,15 @@ static const struct attribute_group ds1621_group = {
> > >  	.is_visible = ds1621_attribute_visible
> > >  };
> > >  
> > > +static const struct attribute_group *ds1621_groups[] = {
> > > +	&ds1621_group,
> > > +	NULL
> > > +};
> > > +
> > >  static int ds1621_probe(struct i2c_client *client,
> > >  			const struct i2c_device_id *id)
> > >  {
> > >  	struct ds1621_data *data;
> > > -	int err;
> > >  
> > >  	data = devm_kzalloc(&client->dev, sizeof(struct ds1621_data),
> > >  			    GFP_KERNEL);
> > > @@ -377,22 +381,12 @@ static int ds1621_probe(struct i2c_client *client,
> > >  	/* Initialize the DS1621 chip */
> > >  	ds1621_init_client(client);
> > >  
> > > -	/* Register sysfs hooks */
> > > -	err = sysfs_create_group(&client->dev.kobj, &ds1621_group);
> > > -	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_files;
> > > -	}
> > > +	data->hwmon_dev = hwmon_device_register_groups(&client->dev, data,
> > > +						       ds1621_groups);
> > > +	if (IS_ERR(data->hwmon_dev))
> > > +		return PTR_ERR(data->hwmon_dev);
> > >  
> > >  	return 0;
> > > -
> > > - exit_remove_files:
> > > -	sysfs_remove_group(&client->dev.kobj, &ds1621_group);
> > > -	return err;
> > 
> > Aren't your sysfs files now being created attached to a different device
> > than they originally were?  Before this patch, they were being attached
> > to the parent device of the hwmon device being created with the call to
> > hwmon_device_register(), and now they are attached to the device created
> > with that call.
> > 
> > Is that what you really want?  Can userspace handle this movement of
> > files?
> 
> libsensors supports both, as well as pwmconfig/fancontrol. Some drivers
> are already doing that so every application dealing with hwmon devices
> should handle it already.
> 
Yes, and I tested it with the new nct6775 driver to make sure that it works.
The i2c and spi drivers need some more work, though - the name attribute is
now missing and will have to be created for the hwmon device itself. If you have
an idea how to do that without adding it in every driver, please let me know.

Thanks,
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/