Re: [PATCH v2] hwmon: add driver for ARCTIC Fan Controller

[Aureo Serrano]

On 2026-03-13 09:59:56-0700, Guenter Roeck wrote:
> > +	buf = kmalloc(ARCTIC_REPORT_LEN, GFP_KERNEL);
> > +	if (!buf)
> > +		return -ENOMEM;
>
> The second problem does not exist since the hwmon core serializes sysfs
> attribute accesses, and a single once-allocated buffer would be sufficient
> for the same reason.

Moved the buffer into the struct (u8 buf[ARCTIC_REPORT_LEN]), allocated
once via devm_kzalloc with the rest of the struct. Per-write kmalloc/kfree
removed. Does that match what you had in mind?

> > +	{
> > +		guard(mutex)(&priv->lock);
> >
> The { } around the guard() are unnecessary, both here and elsewhere in
> the code.
>
> If the guard() is added is because the call is from an event handler,
> use hwmon_lock() and hwmon_unlock() to serialize accesses.

Braces removed. arctic_fan_parse_report() now uses hwmon_lock()/
hwmon_unlock() as suggested.

One consequence worth checking: since read and write callbacks are already
invoked with the hwmon core lock held, keeping a separate private mutex
alongside hwmon_lock() would protect the same data with two different locks.
I removed the private mutex entirely instead of converting it with
devm_mutex_init(). Is that the right conclusion, or would you prefer keeping
the mutex?

> > +	mutex_init(&priv->lock);
>
> Use devm_mutex_init().

Addressed as described above.