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:   Thu, 30 Mar 2023 09:57:39 +0200
From:   Greg KH <gregkh@linuxfoundation.org>
To:     VaibhaavRam.TL@microchip.com
Cc:     linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
        UNGLinuxDriver@microchip.com, arnd@arndb.de,
        Tharunkumar.Pasumarthi@microchip.com
Subject: Re: [PATCH v8 char-misc-next 3/5] misc: microchip: pci1xxxx: Add
 EEPROM Functionality to read and write into EEPROM bin sysfs
Message-ID: <ZCVA8wE5aYXuvX6J@kroah.com>
References: <20230328144008.4113-1-vaibhaavram.tl@microchip.com>
 <20230328144008.4113-4-vaibhaavram.tl@microchip.com>
 <ZCQMh_TbPyLGhTD2@kroah.com>
 <f999539280139b7085721803f12f836c201edf20.camel@microchip.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <f999539280139b7085721803f12f836c201edf20.camel@microchip.com>
Precedence: bulk

On Thu, Mar 30, 2023 at 05:28:43AM +0000, VaibhaavRam.TL@microchip.com 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 <kumaravel.thiagarajan@microchip.com>
> > > 
> > > 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%2FUde@kroah.com/

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