* Guenter Roeck (linux@xxxxxxxxxxxx) 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? > > 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. Yeh I've already done that and used it to get rid of most of my space/tab screwups; I'll fix the remains up before I post the final version. I should nail the rest of this either this weekend or next. Thanks again for your comments, Dave > Thanks, > Guenter -- -----Open up your eyes, open up your mind, open up your code ------- / Dr. David Alan Gilbert | Running GNU/Linux | Happy \ \ dave @ treblig.org | | In Hex / \ _________________________|_____ http://www.treblig.org |_______/