On Wed, Apr 20, 2022 at 11:11 PM Jagath Jog J <jagathjog1996@xxxxxxxxx> wrote: > > Added channel for step counter which can be enable or disable > through the sysfs interface. ... > +static int bma400_enable_steps(struct bma400_data *data, int val) > +{ > + int ret; > + > + if (data->steps_enabled == val) > + return 0; > + > + ret = regmap_update_bits(data->regmap, BMA400_INT_CONFIG1_REG, > + BMA400_STEP_INT_MSK, > + FIELD_PREP(BMA400_STEP_INT_MSK, !!val)); > + data->steps_enabled = val; This will update the value even if we got an error and actual device state is unknown here. Does this make sense? > + return ret; > +} ... I perhaps missed why kmalloc() is needed now. Any pointers to the discussion? -- With Best Regards, Andy Shevchenko