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

[Kumaravel.Thiagarajan]

On Thu, 2023-02-16 at 12:49 +0100, Greg KH wrote:
> > > > > 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?
The device's OTP and EEPROM are not directly mapped into the
processor's address space using PCIe's BAR registers.
There is a OTP controller and EEPROM controller in the device and the
registers of these controllers are mapped into the processor's address
space along with other registers using the BAR registers.
OTP/EEPROM driver maps these registers into kernel's virtual space
using devm_ioremap and accomplishes the reads and writes by accessing
these registers. To the user side, the driver shows two separate disks
(one for OTP and one for EEPROM) and both of them could be programmed
using the "linux dd" command with "oflag=direct" option.
The driver handles the IO requests that originate out of the dd command
and this way we would not need a separate user space program also.

> 
> 
> > 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 think exposing is a wrong word.
OTP and EEPROM memories in the device are enumerated as two separate
block devices.
> 
> > 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?
This PCIe device only maps the OTP controller registers and the EEPROM
controller registers into the address space of the processor and the
actual OTP and EEPROM memories themselves are not mapped. Hence I
enumerated the OTP and EEPROM as block devices and handled the IO for
them in the driver.
> 
> > > 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?
Our device maps only the OTP controller registers and EEPROM controller
registers into the processor's address space.
> 
> > 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.
I thought you are mentioning about /dev/mem and was asking why kernel
maps the memory and device registers into /dev/mem.
> 
> > 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?
OTP and the EEPROM memories of the device are not directly mapped into
the processor address space and only their controller interface
registers are mapped.
> 
> And again, a block device is very odd, that is not the normal way to
> access a device's memory.
I wanted to use dd command to program and any binary editor to view the
OTP and EEPROM memories in the device and I was able to accomplish that
with this solution.
> 
> Thank You.


Regards,
Kumar