Re: [PATCH v2 0/8] Add Keystone PCIe controller driver

[Arnd Bergmann <arnd@arndb.de>]

On Friday 20 June 2014 13:11:37 Santosh Shilimkar wrote:
> > 
> > Unfortunately I lost Jingoo's email from my Inbox. So I cut-n-paste the comment from
> > internet and respond.
> > 
> > Jingoo Han wrote:-
> > =============================================================================
> > I think so, too.
> > 
> > DT maintainers and arch maintainers should review the following
> > dt bindings.
> >   .../devicetree/bindings/pci/designware-pcie.txt    |   42 ++
> >   .../devicetree/bindings/pci/pci-keystone.txt       |   56 +++
> > Generic PHY maintainer (Kishon Vijay Abraham I) should review the
> > following phy driver.
> >   drivers/phy/phy-keystone-serdes.c
> > 
> >>
> >> I'm looking for acks from Mohit and/or Jingoo for the pci/host
> >> changes, and from Arnd for the devicetree/bindings changes.
> >>
> >> Adding these "-dw-3_64" files is sort of ugly.  If that code is only
> >> used by keystone, maybe it could just be moved to pci-keystone.c?  But
> >> I'll defer to Mohit and Jingoo on that and the way you modify
> >> pcie-designware.c.
> > 
> > I agree with Bjorn Helgaas's opinion. These three "-dw-3_64" files
> > look terrible! I don't have a good way to handle this; however,
> > moving this code to pci-keystone.c looks better.
> > ======================================================================
> > 
> > The original RFC I had submitted had all of the application space register
> > handling code as part of the Keystone PCI driver. As per Arnd's comment (See
> > my change log against v1), the code was moved to a separate file so that
> > the next driver that has same version of the DW hw could re-use this code.
> > I agree with Arnd and moved the code to v_3_65 specific files.  What is
> > your proposal?  Do you have objection to the file name? or it's content?
> > 
> > If objection is on the file name,  please suggest alternate names. If you
> > are okay with the file name, and doesn't like the code, it will be helpful
> > to review the code and provide specific comments against the patch itself
> > so that I can address the same.
> > 
> Arnd suggestion was to have the version 3.65 code in generic place since
> its IP specific and just in case some other vendor using the same version
> can leverage the code.
> 
> Concern here seems toe really those name of the files. I can't think of
> any other appropriate name. 

We should definitely keep the version in the DT "compatible" strings
wherever we know it. Regarding a better file name, I have no idea.

	Arnd