Re: [PATCH v4 2/2] hwmon: (max6639) : Add hwmon attributes for fan and pwm

[Guenter Roeck]

On 6/13/24 02:51, Naresh Solanki wrote:
...
> > > +     switch (attr) {
> > > +     case hwmon_fan_pulses:
> > > +             if (val <= 0 || val > 5)
> > > +                     return -EINVAL;
> > > +
> > > +             /* Set Fan pulse per revolution */
> > > +             err = max6639_set_ppr(data, channel, val);
> > > +             if (err < 0)
> > > +                     return err;
> > > +
> > > +             data->ppr[channel] = val;
> >
> > Needs mutex protection to avoid inconsistencies due to concurrent writes.
> This is single i2c access. Still we need mutex protection here ?

In this case, the mutex doesn't protect the i2c access, it protects the
consistency between the chip configuration and the information stored
in data->ppr[].

CPU1			CPU2
[val==1]		[val!=1]

max6639_set_ppr();
			max6639_set_ppr();
			data->ppr[channel] = val;
data->ppr[channel] = val;

The alternative would be to not cache ppr and read it from the regmap cache
when needed.

Guenter