Re: WIP: [PATCH] hwmon: (w83627ehf) convert to with_info interface

[Guenter Roeck <linux@roeck-us.net>]

On Wed, Nov 20, 2019 at 05:26:16PM +0000, Dr. David Alan Gilbert wrote:
> * Guenter Roeck (linux@roeck-us.net) wrote:
> > On Tue, Nov 19, 2019 at 06:18:22PM +0000, Dr. David Alan Gilbert wrote:
> > > 
> > > Hi Jean, Guenter,
> > >   I'm part way through converting w83627ehf to use the devm_hwmon_device_register_with_info
> > > interface and had some questions I'd appreciate the answer to.  My WIP
> > > code is attached below.
> > > 
> > >   a) In the existing driver, all the pseudo files are showing up as:
> > >      /sys/devices/platform/w83627ehf.656/blah_input
> > >      with the rework:
> > >      /sys/devices/platform/w83627ehf.656/hwmon/hwmon1/
> > > 
> > >      my reading is that the reworked one is correct?
> > >      Although I guess the change is a pain for people with paths
> > >      in tools.
> > > 
> 
> Thanks for the reply.
> 
> > No one should have absolute path names like the above in their tools.
> > So far none of the driver conversions caused trouble, so hopefully
> > we should be fine.
> 
> OK, great.
> 
> > >   b) The device has an intrusion0_alarm & intrusion1_alarm
> > >      that seems pretty common looking in drivers/hwmon - some other
> > >      devices have a intrustion%d_beep.  Does it make sense to add
> > >      a new hwmon_intrusion type to hwmon_sensor_types  ?
> > > 
> > Yes, we should add hwmon_intrusion to hwmon_sensor_types, with _alarm
> > and _beep as supported attributes.
> 
> OK, will do.
> 
> > >   c) The device has a bunch more pwm variants:
> > >      pwm2_max_output, pwm2_start_output, pwm2_step_output, pwm2_stop_output,
> > >      pwm2_stop_time, pwm2_target, pwm2_tolerance
> > > 
> > >      for each/some of it's outputs.   What's the right thing to
> > >      do there? Add them all to hwmon_pwm_attr_templates ?
> > >      (Unfortunately it looks like everyone has fun with their own
> > >       pwm settings).
> > > 
> > We'll have to keep sysfs files for those for the time being,
> > unless there are some which are officially listed in
> > Documentation/hwmon/sysfs-interface.rst.
> 
> OK, that's a bit messy; so just keep the existing sysfs code - do
> I need to tweak that to match the new directory path?
> Would another way be to use @extra_groups on the with_info call?
> 
Yes, that is what you are supposed to do.

Guenter