Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 18 Apr 2023 11:18:19 +0200
From:   Greg KH <gregkh@linuxfoundation.org>
To:     Michael Walle <michael@walle.cc>
Cc:     VaibhaavRam.TL@microchip.com, linux-gpio@vger.kernel.org,
        linux-kernel@vger.kernel.org, UNGLinuxDriver@microchip.com,
        arnd@arndb.de
Subject: Re: [PATCH v9 char-misc-next 1/2] misc: microchip: pci1xxxx: Add
 support to read and write into PCI1XXXX OTP via NVMEM sysfs
Message-ID: <2023041825-reviving-property-3e3a@gregkh>
References: <20230413105318.6070-1-vaibhaavram.tl@microchip.com>
 <20230413105318.6070-2-vaibhaavram.tl@microchip.com>
 <51847629489852d493af928580c5a2de@walle.cc>
 <49405ea2bb0bee16a41ce88b7d679ff714823585.camel@microchip.com>
 <8b7b98e60fd871cc7aafda6f9b4e146a@walle.cc>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <8b7b98e60fd871cc7aafda6f9b4e146a@walle.cc>
Precedence: bulk

On Tue, Apr 18, 2023 at 11:00:18AM +0200, Michael Walle wrote:
> Am 2023-04-17 14:47, schrieb VaibhaavRam.TL@microchip.com:
> > > > +#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