Re: [PATCH v2 3/3] iio: gyro: adxrs290: Add debugfs register access support

[Nishant Malpani <nish.malpani25@gmail.com>]

Hello,

On Thu, Sep 3, 2020 at 6:55 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Thu, Sep 3, 2020 at 4:10 PM Nishant Malpani <nish.malpani25@gmail.com> wrote:
> >
> > Extend support to read/write byte data from/to the device using
> > debugfs iio interface.
>
> ...
>
> > +static int adxrs290_reg_access(struct iio_dev *indio_dev, unsigned int reg,
> > +                              unsigned int writeval, unsigned int *readval)
> > +{
> > +       struct adxrs290_state *st = iio_priv(indio_dev);
> > +       int ret;
> > +
> > +       if (!readval)
> > +               return adxrs290_spi_write_reg(st->spi, reg, writeval);
> > +
> > +       ret = spi_w8r8(st->spi, ADXRS290_READ_REG(reg));
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       *readval = ret;
> > +
> > +       return 0;
> > +}
>
> Hmm... I would suggest to have it like
>
> adxrs290_reg_access_rw()
> {
>  ...
>  return 0;
> }
>
> adxrs290_reg_access()
> {
>  if (readval)
>   return adxrs290_reg_access_rw();
>  else // it's redundant, but someone can use for better formatting
>   return adxrs290_spi_write_reg();
> }

Umm, I'm sorry, I don't see why'd it be favourable for us to do it
this way. Also, I *think* Jonathan is fine with how it's being done
now.

With regards,
Nishant Malpani

>
> --
> With Best Regards,
> Andy Shevchenko