Re: [PATCH v9 char-misc-next 1/2] misc: microchip: pci1xxxx: Add support to read and write into PCI1XXXX OTP via NVMEM sysfs

[Greg KH]

On Tue, Apr 18, 2023 at 11:00:18AM +0200, Michael Walle wrote:
> Am 2023-04-17 14:47, schrieb VaibhaavRam.TL@xxxxxxxxxxxxx:
> > > > +#include <linux/auxiliary_bus.h>
> > > > +#include <linux/bio.h>
> > > > +#include <linux/device.h>
> > > > +#include <linux/iopoll.h>
> > > > +#include <linux/kthread.h>
> > > 
> > > Is this needed? I don't see any threads. Also bio.h. Please double
> > > check
> > > your includes.
> > Ok, Will remove in next version of patch
> > > > +     if (priv != NULL)
> > > > +             rb = priv->reg_base;
> > > > +     else
> > > > +             return -ENODEV;
> > > 
> > > Unneeded check, priv cannot be NULL, right?
> > Ok, I think this can be removed
> > > > +
> > > > +             data = readl(rb +
> > > > MMAP_OTP_OFFSET(OTP_PASS_FAIL_OFFSET));
> > > > +             if (ret < 0 || data & OTP_FAIL_BIT)
> > > > +                     break;
> > > 
> > > No error handling?
> > We have implemented short read which returns count of successful bytes
> > read and therefore userspace will recognise the situation when the
> > requested count is not obtained.
> 
> I'll leave that up to Greg, but I'd prefer a proper error to userspace.
> I'm not sure if you'd need to differentiate between a short read/write
> and an error.

A short read isn't an error, it's just returning the amount of data
present which might be less than the buffer passed in, which is totally
normal.  If there is an error, return an error.

thanks,

greg k-h