On Tue, Mar 28, 2023 at 08:10:06PM +0530, Vaibhaav Ram T.L wrote:
> From: Kumaravel Thiagarajan <[email protected]>
>
> 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 adds EEPROM functionality to support the same.
Again, why not use the in-kernel eeprom api instead?
thanks,
greg k-h
On Wed, 2023-03-29 at 12:01 +0200, Greg KH wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> On Tue, Mar 28, 2023 at 08:10:06PM +0530, Vaibhaav Ram T.L wrote:
> > From: Kumaravel Thiagarajan <[email protected]>
> >
> > 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 adds EEPROM functionality to support the same.
>
> Again, why not use the in-kernel eeprom api instead?
Unlike other in-Kernel EEPROM APIs, this EEPROM is not accessible
through any of the i2c/spi buses available to the kernel.
It is only accessible through the register interface available in the
EEPROM controller of the PCI1XXXX device.
The architecture of the device was explained @
https://lore.kernel.org/all/Y+9HOdHGqmPP%[email protected]/
>
> thanks,
>
> greg k-h
On Thu, Mar 30, 2023 at 05:28:43AM +0000, [email protected] wrote:
> On Wed, 2023-03-29 at 12:01 +0200, Greg KH wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> >
> > On Tue, Mar 28, 2023 at 08:10:06PM +0530, Vaibhaav Ram T.L wrote:
> > > From: Kumaravel Thiagarajan <[email protected]>
> > >
> > > 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 adds EEPROM functionality to support the same.
> >
> > Again, why not use the in-kernel eeprom api instead?
> Unlike other in-Kernel EEPROM APIs, this EEPROM is not accessible
> through any of the i2c/spi buses available to the kernel.
>
> It is only accessible through the register interface available in the
> EEPROM controller of the PCI1XXXX device.
>
> The architecture of the device was explained @
> https://lore.kernel.org/all/Y+9HOdHGqmPP%[email protected]/
That shows the architecture, but I left it as "try using the EEPROM api
and let us know how it goes" and you never did that.
If you are going to create your own user/kernel api for something that
the kernel already has a user/kernel api for, you need to document it
in the changelog text really really really well why you can't use the
existing api, and why this new custom one really is the only way to
solve this issue, to explain all of this.
thanks,
greg k-h
[First, please CC people who did comments on previous versions.]
> > > 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 adds EEPROM functionality to support the same.
> >
> > Again, why not use the in-kernel eeprom api instead?
> Unlike other in-Kernel EEPROM APIs, this EEPROM is not accessible
> through any of the i2c/spi buses available to the kernel.
I fail to see how this matters. NVMEM has a generic read/write callback.
There is no dependency on I2C or SPI. Again, you should look into nvmem.
And it should be perfectly fine to use nvmem without nvmem cells at all.
With CONFIG_NVMEM_SYSFS you should get a "nvmem" binary file in sysfs.
Wit config->compat set (although I don't know if that is recommended) you
should get an "eeprom" binary file in sysfs.
> It is only accessible through the register interface available in the
> EEPROM controller of the PCI1XXXX device.
-michael
> -----Original Message-----
> From: Michael Walle <[email protected]>
> Sent: Thursday, March 30, 2023 3:22 PM
> To: VaibhaavRam TL - I69105 <[email protected]>
>
> [First, please CC people who did comments on previous versions.]
Sure. We will do this henceforth.
>
> > > > 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 adds EEPROM functionality to support the same.
> > >
> > > Again, why not use the in-kernel eeprom api instead?
> > Unlike other in-Kernel EEPROM APIs, this EEPROM is not accessible
> > through any of the i2c/spi buses available to the kernel.
>
> I fail to see how this matters. NVMEM has a generic read/write callback.
> There is no dependency on I2C or SPI. Again, you should look into nvmem.
> And it should be perfectly fine to use nvmem without nvmem cells at all.
>
> With CONFIG_NVMEM_SYSFS you should get a "nvmem" binary file in sysfs.
> Wit config->compat set (although I don't know if that is recommended) you
> should get an "eeprom" binary file in sysfs.
>
> > It is only accessible through the register interface available in the
> > EEPROM controller of the PCI1XXXX device.
Michael & Greg,
By in-kernel EEPROM APIs and NVMEM, are you both referring to the same?
Can you please confirm?
We can explore this if that is fine to use this without nvmem cells.
I think we misunderstood as we were referring to drivers/misc/eeprom for examples.
Thank You.
Regards,
Kumar