Re: [PATCH 2/4] hwmon: (adt7475) Convert to hwmon_device_register_with_info()

["Davíð Steinn Geirsson" <david@dsg.is>]

On Fri, Nov 13, 2026 at 10:07:53PM -0800, Guenter Roeck wrote:
> On 11/13/26 16:41, Davíð Steinn Geirsson wrote:
> > Convert the driver from hwmon_device_register_with_groups() to
> > hwmon_device_register_with_info(). The standard voltage, temperature,
> > fan and PWM attributes move to hwmon_ops. The chip-specific attributes
> > stay driver-defined and are passed as extra groups: the automatic fan
> > control points, temperature smoothing, PWM stall disable,
> > pwm_use_point2_pwm_at_crit and the VID attributes.
> >
> > The driver keeps data->lock, although the hwmon core serialises the
> > hwmon_ops callbacks. The attributes in the extra groups run outside that
> > serialisation, and they share the register cache and its periodic update
> > with the hwmon_ops callbacks. They do not take hwmon_lock() instead, to
> > keep this conversion free of locking changes.
> >
> 
> I disagree here. If you don't want to change the locks to use the hwmon
> subsystem lock in this patch, add another patch to change the code to
> use it. The subsystem lock was not introduced to be ignored.

Understood. I did notice one issue while planning that change. Patch 4
makes the chip a PWM provider, so its callbacks will also need the
lock. If the consumer is pwm-fan, then when userspace writes pwm-fan's
pwm attribute, the hwmon core holds pwm-fan's lock. pwm-fan then calls
into the PWM core, which calls into adt7475, which then needs to take
its own hwmon lock:

  hwmon_attr_store()              takes pwm-fan's hwmon lock
    pwm_fan_write()
      pwm_apply_might_sleep()
        adt7475_pwm_write_waveform()
          hwmon_lock()            takes adt7475's hwmon lock

As far as I can tell no driver does this today. adt7470 and
aspeed-g6-pwm-tach are the other PWM providers I found in drivers/hwmon,
and neither takes the subsystem lock in its PWM callbacks.

I haven't written any code for this yet, so this is based on my read
of the current code only. If correct, I think this would need a hwmon
core change to give each hwmon device its own lockdep class.

I'm not quite confident enough in my understanding of the core to
commit to implementing such a change. But I'd appreciate your thoughts
on this.

> 
> Thanks,
> Guenter
> 

Thanks again,
Davíð