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

[Ben Dooks <ben.dooks@codethink.co.uk>]

On 24/03/14 12:04, Phil.Edworthy@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@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@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@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.


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius