RE: [PATCH v5 char-misc-next] misc: microchip: pci1xxxx: Add OTP/EEPROM driver for the pci1xxxx switch

From: Kumaravel.Thiagarajan
Date: Wed Feb 15 2023 - 04:48:22 EST


> From: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
> Sent: Wednesday, February 15, 2023 2:29 PM
> To: Michael Walle <michael@xxxxxxxx>
> Cc: Kumaravel Thiagarajan - I21417
> <Kumaravel.Thiagarajan@xxxxxxxxxxxxx>; Tharunkumar Pasumarthi -
> I67821 <Tharunkumar.Pasumarthi@xxxxxxxxxxxxx>; UNGLinuxDriver
> <UNGLinuxDriver@xxxxxxxxxxxxx>; arnd@xxxxxxxx; linux-
> gpio@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> srinivas.kandagatla@xxxxxxxxxx
> Subject: Re: [PATCH v5 char-misc-next] misc: microchip: pci1xxxx: Add
> OTP/EEPROM driver for the pci1xxxx switch
>
> EXTERNAL EMAIL: Do not click links or open attachments unless you know
> the content is safe
>
> On Wed, Feb 15, 2023 at 09:20:10AM +0100, Michael Walle wrote:
> > Hi,
> >
> > > > > Microchip's pci1xxxx is an unmanaged PCIe3.1a switch for
> > > > > consumer, industrial, and automotive applications. This switch
> > > > > integrates OTP and EEPROM to enable customization of the part
> > > > > in the field. This patch provides the OTP/EEPROM driver to
> > > > > support the
> same.
> > > >
> > > > Why isn't this driver using the nvmem subsystem which is usually
> > > > used for OTP and EEPROM?
> > > Michael, these OTP and EEPROM memories do not have any fixed
> > > location registers which store values (Eg. mac address, config
> > > parameters, etc) at fixed offsets.
> > > It stores a bunch of records, each of which has some data to be
> > > written into the device's hardware registers at different locations.
> > > These records are directly consumed by the hardware and
> > > interpreted without the involvement of the software.
> > > Therefore, we don't require any OTP / EEPROM register map to be
> > > input to the OS / driver through device tree or board files.
> > > I only had to enumerate two separate block devices using the
> > > driver so that the config binary files can be overlayed using the
> > > dd command.
> > > Since this is not fitting like a conventional nvme device, I
> > > didn't choose the nvme subsystem.
> > > Please let me know your thoughts / comments if any.
> >
> > So this is only for provisioning. i.e. during manufacturing a board
> > which uses this PCI bridge? There are no kernel users, nor is there
> > a common interface towards user-space. But just some block device
> > (why not a char device?) exposed to userspace. I presume there is a
> > companion userspace application for it? Why do you take the extra
> > step and have a (random) kernel interface, you could also just
> > access the PCI device directly from userspace within your companion
> > application, e.g. through libpci.
>
> Yeah, why not just use userspace, I missed that, thanks!
Greg & Michael, I do not want to expose the entire or even partial set of device
registers to the user space access directly for safety reasons.
I think hardware registers shall be accessible only through safe and robust kernel
mode components and that the user space shall only be able to access the device
through the kernel mode services.
I want the user to use the hardware only in those ways designated by the driver.
We were using the "busybox devmem" to access the hardware registers directly and to program the EEPROM / OTP.
But we understood that it cannot be an end user solution in all cases and based on some of the operating
environments, there can be some restrictions in opening the direct hardware access to the user space.
Please let me know your thoughts / comments if any.

Thank You.

Regards,
Kumar