On Thu, Aug 06, 2026 at 02:01:30AM -0700, Petar Stepanovic wrote: > +#define PWM_MAX 255 That define has a name that looks much more generic than it actually is. Please call it AX_TACH_PWM_VAL_MAX or something similar. > [...] > +static int axiado_pwm_fan_read(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long *val) > +{ > + struct axiado_pwm_fan_ctx *ctx = dev_get_drvdata(dev); > + > + switch (type) { > + case hwmon_pwm: > + if (attr != hwmon_pwm_input) > + return -EOPNOTSUPP; > + > + scoped_guard(mutex, &ctx->pwm_lock) > + *val = ctx->pwm_value; If there is a hwmon lock that serializes calls to hwmon_ops::read and hwmon_ops::write, .pwm_lock isn't needed. (Unless I miss something, please double check. Also I'm unsure if there is such a hwmon serialization.) > [...] > +static const struct of_device_id axiado_pwm_fan_match[] = { > + { .compatible = "axiado,ax3000-pwm-fan" }, > + { }, No trailing , after the list terminator please. > +}; > +MODULE_DEVICE_TABLE(of, axiado_pwm_fan_match); > + > +static struct platform_driver axiado_pwm_fan_driver = { > + .probe = axiado_pwm_fan_probe, > + .shutdown = axiado_pwm_fan_shutdown, > + .driver = { > + .name = "axiado-pwm-fan", > + .pm = pm_sleep_ptr(&axiado_pwm_fan_pm), > + .of_match_table = axiado_pwm_fan_match, > + }, > +}; > + > +module_platform_driver(axiado_pwm_fan_driver); Typically `module_platform_driver` follows the struct platform_driver without a newline. > +MODULE_AUTHOR("Axiado Corporation"); > +MODULE_DESCRIPTION("Axiado PWM fan controller driver"); > +MODULE_LICENSE("GPL"); > + Trailing newline at end of file. Best regards Uwe