Re: [PATCH v3 4/4] PCI: mediatek-gen3: Add Airoha EN7581 support

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Jul 03, 2024 at 05:26:56PM +0200, Lorenzo Bianconi wrote:
> > On Tue, Jul 02, 2024 at 11:12:21AM +0200, Lorenzo Bianconi wrote:
> > > > On Sat, Jun 29, 2024 at 03:51:54PM +0200, Lorenzo Bianconi wrote:
> > > > > Introduce support for Airoha EN7581 PCIe controller to mediatek-gen3
> > > > > PCIe controller driver.
> > > > 
> > > > > +/* PCIe reset line delay in ms */
> > > > > +#define PCIE_RESET_TIME_MS		100
> > > > 
> > > > Is this something required by the PCIe base spec, or is it specific to
> > > > EN7581?  Either way it would be nice to have a citation to the spec
> > > > (revision and section number).  If it's generic to PCIe, it should be
> > > > in drivers/pci/pci.h so other drivers can use the same thing.
> > > 
> > > It is just the time needed by the EN7581 reset controller to
> > > complete the operation, it is not something PCIe generic (it is
> > > something just related to EN7581 SoC).  Do you think we should move
> > > it in EN7581 reset controller codebase?
> > 
> > I have no opinion about moving it.  But it sounds like maybe it should
> > have a less generic name so it doesn't look like a generic PCIe thing.
> > And also a spec citation would be helpful for future maintenance.
> 
> ack, naming is always hard :)
> I do not have any specific spec for it. It is in the vendor sdk and the Airoha
> folks confirmed just this time is needed to complete PCIe reset.

Even a note that this came from an SDK from a specific vendor might be
useful since it shows that it wasn't picked out of thin air and it
gives a hint of who to ask about it.