diff mbox

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

Message ID 1395397968-6242-6-git-send-email-phil.edworthy@renesas.com (mailing list archive)
State New, archived
Headers show

Commit Message

Phil Edworthy March 21, 2014, 10:32 a.m. UTC
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>
---
 Documentation/devicetree/bindings/pci/rcar-pci.txt | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/rcar-pci.txt

Comments

Lucas Stach March 21, 2014, 11:23 a.m. UTC | #1
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?

> +
> +Example:
> +
> +SoC specific DT Entry:
> +
> +	pcie: pcie@0x01000000 {
> +		compatible = "renesas,r8a7790-pcie";
> +		reg = <0 0xfe000000 0 0x80000>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		ranges = <0x01000000 0 0x00000000 0 0xfe100000 0 0x00100000
> +			  0x02000000 0 0xfe200000 0 0xfe200000 0 0x00200000
> +			  0x02000000 0 0x30000000 0 0x30000000 0 0x08000000
> +			  0x42000000 0 0x38000000 0 0x38000000 0 0x08000000>;
> +		interrupts = <0 116 4>;
> +		#interrupt-cells = <1>;
> +		interrupt-map-mask = <0 0 0 0>;
> +		interrupt-map = <0 0 0 0 &gic 0 116 4>;
> +		clocks = <&mstp3_clks R8A7790_CLK_PCIE>;
> +		clock-names = "pcie";
> +		status = "disabled";
> +	};
Phil Edworthy March 21, 2014, 2:18 p.m. UTC | #2
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.


> > +
> > +Example:
> > +
> > +SoC specific DT Entry:
> > +
> > +   pcie: pcie@0x01000000 {
> > +      compatible = "renesas,r8a7790-pcie";
> > +      reg = <0 0xfe000000 0 0x80000>;
> > +      #address-cells = <3>;
> > +      #size-cells = <2>;
> > +      device_type = "pci";
> > +      ranges = <0x01000000 0 0x00000000 0 0xfe100000 0 0x00100000
> > +           0x02000000 0 0xfe200000 0 0xfe200000 0 0x00200000
> > +           0x02000000 0 0x30000000 0 0x30000000 0 0x08000000
> > +           0x42000000 0 0x38000000 0 0x38000000 0 0x08000000>;
> > +      interrupts = <0 116 4>;
> > +      #interrupt-cells = <1>;
> > +      interrupt-map-mask = <0 0 0 0>;
> > +      interrupt-map = <0 0 0 0 &gic 0 116 4>;
> > +      clocks = <&mstp3_clks R8A7790_CLK_PCIE>;
> > +      clock-names = "pcie";
> > +      status = "disabled";
> > +   };
> 
> -- 
> Pengutronix e.K.                           | Lucas Stach |
> Industrial Linux Solutions                 | http://www.pengutronix.de/ 
|
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 
|
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 
|
> 

Thanks
Phil
Lucas Stach March 21, 2014, 2:30 p.m. UTC | #3
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?

Regards,
Lucas
Phil Edworthy March 21, 2014, 3:02 p.m. UTC | #4
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. 

> Regards,
> Lucas
> -- 
> Pengutronix e.K.                           | Lucas Stach |
> Industrial Linux Solutions                 | http://www.pengutronix.de/ 
|
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 
|
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 
|
> 

Thanks
Phil
Phil Edworthy March 24, 2014, 12:04 p.m. UTC | #5
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?

Thanks
Phil
Ben Dooks March 24, 2014, 12:15 p.m. UTC | #6
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.
Phil Edworthy March 24, 2014, 12:25 p.m. UTC | #7
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@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.
Ok, thanks.

Phil
Phil Edworthy March 24, 2014, 12:46 p.m. UTC | #8
Hi Ben,

On: 24/03/2014 12:25, Phil wrote:
> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree 
bindings
> 
> 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@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.
> Ok, thanks.
Hmm, yes it doesn't get probed, but you still get a build problem as the 
DT entry references an clock that doesn't exist. Maybe I'm missing 
something...

Phil
Lucas Stach March 24, 2014, 12:53 p.m. UTC | #9
Am Montag, den 24.03.2014, 12:46 +0000 schrieb
Phil.Edworthy@renesas.com:
> Hi Ben,
> 
> On: 24/03/2014 12:25, Phil wrote:
> > Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree 
> bindings
> > 
> > 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@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.
> > Ok, thanks.
> Hmm, yes it doesn't get probed, but you still get a build problem as the 
> DT entry references an clock that doesn't exist. Maybe I'm missing 
> something...
> 
If the clock is a board supplied one then you have to supply to clocks
property from the board dts, to get rid of the compile failure. You can
not have phandles in the PCIe node referencing a non-existing clock.

Regards,
Lucas
diff mbox

Patch

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"
+
+Example:
+
+SoC specific DT Entry:
+
+	pcie: pcie@0x01000000 {
+		compatible = "renesas,r8a7790-pcie";
+		reg = <0 0xfe000000 0 0x80000>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		ranges = <0x01000000 0 0x00000000 0 0xfe100000 0 0x00100000
+			  0x02000000 0 0xfe200000 0 0xfe200000 0 0x00200000
+			  0x02000000 0 0x30000000 0 0x30000000 0 0x08000000
+			  0x42000000 0 0x38000000 0 0x38000000 0 0x08000000>;
+		interrupts = <0 116 4>;
+		#interrupt-cells = <1>;
+		interrupt-map-mask = <0 0 0 0>;
+		interrupt-map = <0 0 0 0 &gic 0 116 4>;
+		clocks = <&mstp3_clks R8A7790_CLK_PCIE>;
+		clock-names = "pcie";
+		status = "disabled";
+	};