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

[Greg KH <gregkh@linuxfoundation.org>]

On Thu, Feb 16, 2023 at 11:39:12AM +0000, Kumaravel.Thiagarajan@microchip.com wrote:
> > -----Original Message-----
> > From: Greg KH <gregkh@linuxfoundation.org>
> > Sent: Wednesday, February 15, 2023 5:15 PM
> > To: Kumaravel Thiagarajan - I21417 <Kumaravel.Thiagarajan@microchip.com>
> > Subject: Re: [PATCH v5 char-misc-next] misc: microchip: pci1xxxx: Add
> > OTP/EEPROM driver for the pci1xxxx switch

What is this header for?

Please fix your email client to not add this to the body of the text...

> > > > 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.
> > 
> > But that's all exposed here through this block device, right?
> The block device created by this driver does not expose the device registers to the user space applications.

What is it exposing?

And please use line-wrapping :)

> The device hardware provides separate set of registers to read and write into the OTP memory and EEPROM.
> The driver uses these hardware registers and abstracts the programming logic inside and exposes the only the memory as devices to the user space.

What memory is being exposed?  And how?

> I don't have any user program to program the device. I use the Linux dd command only.
> If I want to view the contents of the memory, I can use any hex editor tool in Linux this way.

Exposing the memory of a device as a block device is not normal, it
should just be mmapped, right?

> > And this is already exposed to userspace today, no need to add anything the
> > kernel already provides this.
> Can you explain this? Are you referring to any sysfs directories / files? What is the necessity to do this? I am trying to understand this.

PCI device accesses can go through userspace directly.  Is this just
memory mapped in your PCI device?

> If this is for any debug purpose and whether kernel does this under some conditional compilation or is that the default behavior?

Is this only for debugging?  If so, please document it as such so that
no one accidentally enables it as a valid build option.

> Even if the user is a super user, should he be allowed to access the device hardware registers mandatorily. It should depend on the policy the system owner want to adopt. Right?

Again, is this PCI memory that can be accessed directly?

And again, a block device is very odd, that is not the normal way to
access a device's memory.

thanks,

greg k-h