[PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree bindings

[Phil.Edworthy at renesas.com]

Hi Ben,

On: 24/03/2014 12:16, Ben wrote:
> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree 
bindings
> 
> On 24/03/14 12:04, Phil.Edworthy at renesas.com wrote:
> > Hi Lucas,
> >
> > On: 21/03/2014 15:02, Phil wrote:
> >> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree
> > bindings
> >>
> >> Hi Lucas,
> >>
> >> On: 21/03/2014 14:31, Lucas wrote:
> >>> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree
> > bindings
> >>>
> >>> Am Freitag, den 21.03.2014, 14:18 +0000 schrieb
> >>> Phil.Edworthy at renesas.com:
> >>>> Hi Lucas,
> >>>>
> >>>> On 21/03/2014 11:24, Lucas wrote:
> >>>>> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device
> > tree
> >>>> bindings
> >>>>>
> >>>>> Am Freitag, den 21.03.2014, 10:32 +0000 schrieb Phil Edworthy:
> >>>>>> This patch adds the bindings for the rcar PCIE driver. The
> > driver
> >>>>>> resides under drivers/pci/host/pcie-rcar.c
> >>>>>>
> >>>>>> Signed-off-by: Phil Edworthy <phil.edworthy at renesas.com>
> >>>>>
> >>>>> I have one question below. If you can answer this with yes after
> >>>>> thinking again about it, this is:
> >>>>>
> >>>>> Reviewed-by: Lucas Stach <l.stach at pengutronix.de>
> >>>>>> ---
> >>>>>>   Documentation/devicetree/bindings/pci/rcar-pci.txt | 40
> >>>> ++++++++++++++++++++++
> >>>>>>   1 file changed, 40 insertions(+)
> >>>>>>   create mode 100644
> > Documentation/devicetree/bindings/pci/rcar-pci.txt
> >>>>>>
> >>>>>> diff --git a/Documentation/devicetree/bindings/pci/rcar-pci.txt
> > b/
> >>>>> Documentation/devicetree/bindings/pci/rcar-pci.txt
> >>>>>> new file mode 100644
> >>>>>> index 0000000..0e219b0
> >>>>>> --- /dev/null
> >>>>>> +++ b/Documentation/devicetree/bindings/pci/rcar-pci.txt
> >>>>>> @@ -0,0 +1,40 @@
> >>>>>> +* Renesas RCar PCIe interface
> >>>>>> +
> >>>>>> +Required properties:
> >>>>>> +- compatible: should contain one of the following
> >>>>>> +   "renesas,r8a7779-pcie", "renesas,r8a7790-pcie",
> >>>> "renesas,r8a7791-pcie"
> >>>>>> +- reg: base addresses and lengths of the pcie controller.
> >>>>>> +- #address-cells: set to <3>
> >>>>>> +- #size-cells: set to <2>
> >>>>>> +- device_type: set to "pci"
> >>>>>> +- ranges: ranges for the PCI memory and I/O regions
> >>>>>> +- interrupts: interrupt values for MSI interrupt
> >>>>>> +- #interrupt-cells: set to <1>
> >>>>>> +- interrupt-map-mask and interrupt-map: standard PCI properties
> >>>>>> +   to define the mapping of the PCIe interface to interrupt
> >>>>>> +   numbers.
> >>>>>> +- clocks: from common clock binding: handle to pci clock.
> >>>>>> +- clock-names: from common clock binding: should be "pcie"
> >>>>>
> >>>>> You really only have one PCIe clock? So the controller is
> > generating the
> >>>>> PCIe bus clock from it's register clock? No need to enable any
> > other
> >>>>> clock for PCIe to work?
> >>>> Yes, just the one clock. The PCIe bus clock is an external clock
> > dedicated
> >>>> to PCIe and SATA, and we have no control over it.
> >>>>
> >>> Is this some kind of always-on clock? If not you probably want a
> >>> reference to it from the PCIe driver even if it's shared between 
SATA
> >>> and PCIe. After all you don't want to end up unconditionally 
enabling
> >>> this clock from the board or SoC level just to have PCIe working,
> > right?
> >> Ok, I see, I'll need a reference to a board clock, even if it's 
always
> > on for
> >> these boards.
> > One small question...
> >
> > The patches added the PCIe host controller to two devices, r8a7790 and
> > r8a7791. The r8a7790-based Lager board doesn't have the necessary 
hardware
> > to use PCIe, so should I add a dummy pcie_bus clock to the Lager board
> > dts, even though PCIe is not used?
> >
> > I'm just wondering what the dts file looks like for a board that 
doesn't
> > support many interfaces. Does it become littered with dummy clocks and
> > regulators?
> 
> The bridge node should be set to disabled by default so it never gets
> probed. Any boards that want to use it can over-ride the status to be
> enabled.
Ok, thanks.

Phil