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

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

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.
> 

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.

>   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.

>   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.

> For reference, I seem to have a w83667hg on an ASRock P55M Pro.
> 
> The current status is that 'reading' seems to work (from what I can tell
> but not looked at the PWM), and I've not converted the writers yet.
> 
Good start. I would suggest to run your patch through checkpatch.
It will tell you, for example, that S_IRUGO et al ran out of favor,
and that you are supposed to use octals instead.

Thanks,
Guenter