Re: [PATCH 3/4] arm64: Juno: Add support for the PCIe host bridge on Juno R1

[Liviu Dudau <Liviu.Dudau@arm.com>]

On Fri, Oct 09, 2015 at 05:32:52PM +0100, Mark Rutland wrote:
> On Fri, Oct 09, 2015 at 05:09:10PM +0100, Liviu Dudau wrote:
> > On Fri, Oct 09, 2015 at 05:49:18PM +0200, Arnd Bergmann wrote:
> > > On Friday 09 October 2015 16:44:08 Mark Rutland wrote:
> > > > On Fri, Oct 09, 2015 at 03:11:07PM +0100, Liviu Dudau wrote:
> > > > > On Fri, Oct 09, 2015 at 08:54:33AM -0500, Rob Herring wrote:
> > > > > Or maybe I can claim the use of the string on account on being the first on arm64 
> > > > > 
> > > > > I can add a vendor prefix if you want, but pci-host-generic is going to ignore it 
> > > > > *because* it is trying to be a generic driver.
> > > > 
> > > > The point here is to have the string ready if we need it later, so it's
> > > > fine that it's not used currently.
> > > > 
> > > > Rob's suggestion is that the compatible list should look something like:
> > > > 
> > > > compatible = "arm,juno-r1-pcie", "plda,xpressrich3", "pci-host-ecam-generic";
> > > > 
> > > > We can match on "pci-host-ecam-generic" for now (and hopefully forever),
> > > > but if for some reason we need to special-case this host controller (or
> > > > Juno's integration thereof), we can do that based on the compatible
> > > > string.
> > > 
> > > Sounds good to me, it certainly can't hurt.
> > > 
> > > 	Arnd
> > 
> > Hmm, I'm sorry, but this time I'm going to disagree.
> > 
> > I understand the principle that the DTS is a description of the
> > hardware and it should not have any built in knowledge of how a driver
> > works but describe the physical properties of the device (where such
> > description makes sense, in this case it does).
> > 
> > However, when ARM has created the Juno platform it has also created a
> > standard called SBSA and has claimed that Juno is compliant with that
> > standard. My current position (and it used to be MarkR's as well when
> > we have argued internally the pros and cons of having a bespoke driver
> > for PLDA's XpressRICH3) is that SBSA compliant behaviour *is* the
> > expected behaviour and if the device doesn't conform it needs to be
> > fixed in firmware.
> 
> We are not arguing for a bespoke driver, and in fact we hope to never
> have to use the addition strings.
> 
> However, experience shows that we might not be lucky, and if we
> encounter some bizarre quirk in future we might need to handle that by
> knowing what the hardware is.
> 
> > Otherwise, I could've posted months ago the other public driver[1]
> > that I've wrote that doesn't depend on firmware and could have been
> > done with this long time ago.
> 
> We're not arguing for kernel support code. What we're arguing for is
> data in the DTB that ideally turns out to be irrelevant.
> 
> I can't see why you object to that.

It is not a strong objection but I don't want to ever have to add bespoke
code to the pci-host-generic.c driver so adding a compatible property that
is going to be ignored is kind of pointless. Yes, one will have a better
idea on what the hardware actually is, but so what?

I will send a v2 with the added values.

Best regards,
Liviu

> 
> Mark.
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯