Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1404326 > unrolled thread

[PATCH 0/2] Add Rockchip PCIe RC controller support

Started byShawn Lin <shawn.lin@rock-chips.com>
First post2016-05-20 12:30 +0200
Last post2016-05-30 13:10 +0200
Articles 7 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/2] Add Rockchip PCIe RC controller support Shawn Lin <shawn.lin@rock-chips.com> - 2016-05-20 12:30 +0200
    [PATCH 1/2] Documentation: add binding description of Rockchip PCIe controller Shawn Lin <shawn.lin@rock-chips.com> - 2016-05-20 12:30 +0200
      Re: [PATCH 1/2] Documentation: add binding description of Rockchip PCIe controller Heiko Stuebner <heiko@sntech.de> - 2016-05-20 13:30 +0200
        Re: [PATCH 1/2] Documentation: add binding description of Rockchip  PCIe controller Shawn Lin <shawn.lin@rock-chips.com> - 2016-05-21 06:00 +0200
          Re: [PATCH 1/2] Documentation: add binding description of Rockchip PCIe controller Heiko Stuebner <heiko@sntech.de> - 2016-05-23 22:00 +0200
            Re: [PATCH 1/2] Documentation: add binding description of Rockchip  PCIe controller Shawn Lin <shawn.lin@rock-chips.com> - 2016-05-24 03:50 +0200
      Re: [PATCH 1/2] Documentation: add binding description of Rockchip  PCIe controller Marc Zyngier <marc.zyngier@arm.com> - 2016-05-30 13:10 +0200

#1404326 — [PATCH 0/2] Add Rockchip PCIe RC controller support

FromShawn Lin <shawn.lin@rock-chips.com>
Date2016-05-20 12:30 +0200
Subject[PATCH 0/2] Add Rockchip PCIe RC controller support
Message-ID<rAPQS-4i2-9@gated-at.bofh.it>
Hi all,

This patchset gonna support Rockchip PCIe controller integrated
in RK3399 Soc supporting Gen2 with max 4 lanes.

Also it supports:
(1) Support Single-root I/O virtualization(SR-IOV)
(2) Support Legacy Interrupt
(3) Support MSI and MSI-X interrupt
(4) Support ECRC Generation and Checking
(5) Support 8 Virtual Functions attached to Physical Function
(6) Support Outbound and Inbound address translation
(7) Support ASPM state L0s and L1
(8) Support L1 Power Management Substate
(9) Support PCI Function power states D0, D1 and D3, and the corresponding
link power states L0, L1 and L2


Please review, test and comment. Thanks.



Shawn Lin (2):
  Documentation: add binding description of Rockchip PCIe controller
  pci: Add PCIe driver for Rockchip Soc

 .../devicetree/bindings/pci/rockchip-pcie.txt      |   93 ++
 drivers/pci/host/Kconfig                           |   12 +
 drivers/pci/host/Makefile                          |    1 +
 drivers/pci/host/pcie-rockchip.c                   | 1181 ++++++++++++++++++++
 4 files changed, 1287 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/rockchip-pcie.txt
 create mode 100644 drivers/pci/host/pcie-rockchip.c

-- 
2.3.7

[toc] | [next] | [standalone]


#1404330 — [PATCH 1/2] Documentation: add binding description of Rockchip PCIe controller

FromShawn Lin <shawn.lin@rock-chips.com>
Date2016-05-20 12:30 +0200
Subject[PATCH 1/2] Documentation: add binding description of Rockchip PCIe controller
Message-ID<rAPQT-4i2-31@gated-at.bofh.it>
In reply to#1404326
This patch add some required and optional properties for Rockchip
PCIe controller. Also we add a example for how to use it.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

---

 .../devicetree/bindings/pci/rockchip-pcie.txt      | 93 ++++++++++++++++++++++
 1 file changed, 93 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/rockchip-pcie.txt

diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
new file mode 100644
index 0000000..69a0804
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
@@ -0,0 +1,93 @@
+* Rockchip AXI PCIe Root Port Bridge DT description
+
+Required properties:
+- #address-cells: Address representation for root ports, set to <3>
+- #size-cells: Size representation for root ports, set to <2>
+- #interrupt-cells: specifies the number of cells needed to encode an
+		interrupt source. The value must be 1.
+- compatible: Should contain "rockchip,rk3399-pcie"
+- reg: Two register ranges as listed in the reg-names property
+- reg-names: The first entry must be "axi-base" for the core registers
+	The second entry must be "apb-base" for the client pcie registers
+- clocks: Must contain an entry for each entry in clock-names.
+		See ../clocks/clock-bindings.txt for details.
+- clock-names: Must include the following entries:
+	- "aclk_pcie"
+	- "aclk_perf_pcie"
+	- "hclk_pcie"
+	- "clk_pciephy_ref"
+- interrupts: Three interrupt entries must be specified.
+- interrupt-names: Must include the following names
+	- "pcie-sys"
+	- "pcie-legacy"
+	- "pcie-client"
+- resets: Must contain five entries for each entry in reset-names.
+	   See ../reset/reset.txt for details.
+- reset-names: Must include the following names
+	- "phy-rst"
+	- "core-rst"
+	- "mgmt-rst"
+	- "mgmt-sticky-rst"
+	- "pipe-rst"
+- rockchip,grf: phandle to the syscon managing the "general register files"
+- pcie-conf: offset of pcie client block for  configuration
+- pcie-status: offset of pcie client block for status
+- pcie-laneoff: offset of pcie client block for lane
+- msi-parent: Link to the hardware entity that serves as the Message
+- pinctrl-names : The pin control state names
+- pinctrl-0: The "default" pinctrl state
+- interrupt-map-mask and interrupt-map: standard PCI properties
+- interrupt-controller: identifies the node as an interrupt controller
+
+Optional Property:
+- ep-gpios: contain the entry for pre-reset gpio
+- num-lanes: number of lanes to use
+- assigned-clocks, assigned-clock-parents and assigned-clock-rates: standard
+		   clock bindings. See ../clock/clock-bindings.txt
+
+Example:
+
+pci_express: axi-pcie@f8000000 {
+	#address-cells = <3>;
+	#size-cells = <2>;
+	compatible = "rockchip,rk3399-pcie";
+	clocks = <&cru ACLK_PCIE>, <&cru ACLK_PERF_PCIE>,
+		 <&cru PCLK_PCIE>, <&cru SCLK_PCIEPHY_REF>;
+	clock-names = "aclk_pcie", "aclk_perf_pcie",
+		      "hclk_pcie", "clk_pciephy_ref";
+	bus-range = <0x0 0x1>;
+	interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
+	interrupt-names: "pcie-sys", "pcie-legacy", "pcie-client";
+	assigned-clocks = <&cru SCLK_PCIEPHY_REF>;
+	assigned-clock-parents = <&cru SCLK_PCIEPHY_REF100M>;
+	assigned-clock-rates = <100000000>;
+	ep-gpios = <&gpio3 13 GPIO_ACTIVE_HIGH>;
+	ranges = < 0x82000000 0 0xfa000000 0x0 0xfa000000 0 0x600000
+		   0x81000000 0 0xfa600000 0x0 0xfa600000 0 0x100000 >;
+	num-lanes = <4>;
+	reg = < 0x0 0xf8000000 0x0 0x2000000 >, < 0x0 0xfd000000 0x0 0x1000000 >;
+	reg-name = "axi-base", "apb-base";
+	resets = <&cru SRST_PCIEPHY>, <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
+		 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>;
+	reset-names = "phy-rst", "core-rst", "mgmt-rst", "mgmt-sticky-rst", "pipe-rst";
+	rockchip,grf = <&grf>;
+	pcie-conf = <0xe220>;
+	pcie-status = <0xe2a4>;
+	pcie-laneoff = <0xe214>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pcie_clkreq>;
+	msi-parent = <&its>;
+	#interrupt-cells = <1>;
+	interrupt-map-mask = <0 0 0 7>;
+	interrupt-map = <0 0 0 1 &pcie_0 1>,
+			<0 0 0 2 &pcie_0 2>,
+			<0 0 0 3 &pcie_0 3>,
+			<0 0 0 4 &pcie_0 4>;
+	pcie_0: interrupt-controller {
+		interrupt-controller;
+		#address-cells = <0>;
+		#interrupt-cells = <1>;
+	};
+
+};
-- 
2.3.7

[toc] | [prev] | [next] | [standalone]


#1404353 — Re: [PATCH 1/2] Documentation: add binding description of Rockchip PCIe controller

FromHeiko Stuebner <heiko@sntech.de>
Date2016-05-20 13:30 +0200
SubjectRe: [PATCH 1/2] Documentation: add binding description of Rockchip PCIe controller
Message-ID<rAQMV-4QX-11@gated-at.bofh.it>
In reply to#1404330
Hi Shawn,

Am Freitag, 20. Mai 2016, 18:29:06 schrieb Shawn Lin:
> This patch add some required and optional properties for Rockchip
> PCIe controller. Also we add a example for how to use it.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> 
> ---
> 
>  .../devicetree/bindings/pci/rockchip-pcie.txt      | 93
> ++++++++++++++++++++++ 1 file changed, 93 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> 
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt new file mode
> 100644
> index 0000000..69a0804
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> @@ -0,0 +1,93 @@
> +* Rockchip AXI PCIe Root Port Bridge DT description
> +
> +Required properties:
> +- #address-cells: Address representation for root ports, set to <3>
> +- #size-cells: Size representation for root ports, set to <2>
> +- #interrupt-cells: specifies the number of cells needed to encode an
> +		interrupt source. The value must be 1.
> +- compatible: Should contain "rockchip,rk3399-pcie"
> +- reg: Two register ranges as listed in the reg-names property
> +- reg-names: The first entry must be "axi-base" for the core registers
> +	The second entry must be "apb-base" for the client pcie registers
> +- clocks: Must contain an entry for each entry in clock-names.
> +		See ../clocks/clock-bindings.txt for details.
> +- clock-names: Must include the following entries:
> +	- "aclk_pcie"
> +	- "aclk_perf_pcie"
> +	- "hclk_pcie"
> +	- "clk_pciephy_ref"

clock names are always in the scope of the node/driver, so the names could 
easily be "aclk", "aclk-perf", "hclk", "phy".

Although from my cursory-glance at the phy-related code, it looks more like 
it shouldn't be in the pcie-driver itself, but in a an actual phy-driver 
(together with the phy reset and clock)?

> +- interrupts: Three interrupt entries must be specified.
> +- interrupt-names: Must include the following names
> +	- "pcie-sys"
> +	- "pcie-legacy"
> +	- "pcie-client"

Same as above, names could simply be "sys", "legacy", "client"


> +- resets: Must contain five entries for each entry in reset-names.
> +	   See ../reset/reset.txt for details.
> +- reset-names: Must include the following names
> +	- "phy-rst"
> +	- "core-rst"
> +	- "mgmt-rst"
> +	- "mgmt-sticky-rst"
> +	- "pipe-rst"

and again (= without the "-rst")


> +- rockchip,grf: phandle to the syscon managing the "general register
> files" 

> +- pcie-conf: offset of pcie client block for  configuration
> +- pcie-status: offset of pcie client block for status
> +- pcie-laneoff: offset of pcie client block for lane

These are GRF offsets (GRF_SOC_CON8, GRF_SOC_CON5_PCIE, GRF_SOC_STATUS1)
Those registers are generally prone to change (even their layout) for future 
socs and I'd suggest instead of declaring them in the devicetree, move them 
to the per-soc data in the rockchip_pcie_of_match struct.

But it looks as if they're pretty phy-specific, so see comment about possible 
separate phy driver above.


> +- msi-parent: Link to the hardware entity that serves as the Message
> +- pinctrl-names : The pin control state names
> +- pinctrl-0: The "default" pinctrl state

I'm not sure if pinctrl-properties need to be described when you don't need 
special handling in the form of additional pin states. The pcie part does 
not do any pin-handling of its own.


> +- interrupt-map-mask and interrupt-map: standard PCI properties
> +- interrupt-controller: identifies the node as an interrupt controller
> +
> +Optional Property:
> +- ep-gpios: contain the entry for pre-reset gpio
> +- num-lanes: number of lanes to use
> +- assigned-clocks, assigned-clock-parents and assigned-clock-rates:
> standard +		   clock bindings. See ../clock/clock-bindings.txt

Again that (assigned-clocks handling) is not actual part of the pci-
controllers actions, but other parts and also described already elsewhere.


Heiko

[toc] | [prev] | [next] | [standalone]


#1404804 — Re: [PATCH 1/2] Documentation: add binding description of Rockchip PCIe controller

FromShawn Lin <shawn.lin@rock-chips.com>
Date2016-05-21 06:00 +0200
SubjectRe: [PATCH 1/2] Documentation: add binding description of Rockchip PCIe controller
Message-ID<rB6eZ-6Sr-1@gated-at.bofh.it>
In reply to#1404353
On 2016/5/20 19:20, Heiko Stuebner wrote:
> Hi Shawn,
>
> Am Freitag, 20. Mai 2016, 18:29:06 schrieb Shawn Lin:
>> This patch add some required and optional properties for Rockchip
>> PCIe controller. Also we add a example for how to use it.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>
>> ---
>>
>>  .../devicetree/bindings/pci/rockchip-pcie.txt      | 93
>> ++++++++++++++++++++++ 1 file changed, 93 insertions(+)
>>  create mode 100644
>> Documentation/devicetree/bindings/pci/rockchip-pcie.txt
>>
>> diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
>> b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt new file mode
>> 100644
>> index 0000000..69a0804
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
>> @@ -0,0 +1,93 @@
>> +* Rockchip AXI PCIe Root Port Bridge DT description
>> +
>> +Required properties:
>> +- #address-cells: Address representation for root ports, set to <3>
>> +- #size-cells: Size representation for root ports, set to <2>
>> +- #interrupt-cells: specifies the number of cells needed to encode an
>> +		interrupt source. The value must be 1.
>> +- compatible: Should contain "rockchip,rk3399-pcie"
>> +- reg: Two register ranges as listed in the reg-names property
>> +- reg-names: The first entry must be "axi-base" for the core registers
>> +	The second entry must be "apb-base" for the client pcie registers
>> +- clocks: Must contain an entry for each entry in clock-names.
>> +		See ../clocks/clock-bindings.txt for details.
>> +- clock-names: Must include the following entries:
>> +	- "aclk_pcie"
>> +	- "aclk_perf_pcie"
>> +	- "hclk_pcie"
>> +	- "clk_pciephy_ref"
>
> clock names are always in the scope of the node/driver, so the names could
> easily be "aclk", "aclk-perf", "hclk", "phy".
>
> Although from my cursory-glance at the phy-related code, it looks more like
> it shouldn't be in the pcie-driver itself, but in a an actual phy-driver
> (together with the phy reset and clock)?
>
>> +- interrupts: Three interrupt entries must be specified.
>> +- interrupt-names: Must include the following names
>> +	- "pcie-sys"
>> +	- "pcie-legacy"
>> +	- "pcie-client"
>
> Same as above, names could simply be "sys", "legacy", "client"
>
>
>> +- resets: Must contain five entries for each entry in reset-names.
>> +	   See ../reset/reset.txt for details.
>> +- reset-names: Must include the following names
>> +	- "phy-rst"
>> +	- "core-rst"
>> +	- "mgmt-rst"
>> +	- "mgmt-sticky-rst"
>> +	- "pipe-rst"
>
> and again (= without the "-rst")
>

okay, will simplify clk, int and rst mentioned above.

>
>> +- rockchip,grf: phandle to the syscon managing the "general register
>> files"
>
>> +- pcie-conf: offset of pcie client block for  configuration
>> +- pcie-status: offset of pcie client block for status
>> +- pcie-laneoff: offset of pcie client block for lane
>
> These are GRF offsets (GRF_SOC_CON8, GRF_SOC_CON5_PCIE, GRF_SOC_STATUS1)
> Those registers are generally prone to change (even their layout) for future
> socs and I'd suggest instead of declaring them in the devicetree, move them
> to the per-soc data in the rockchip_pcie_of_match struct.

sure.

>
> But it looks as if they're pretty phy-specific, so see comment about possible
> separate phy driver above.
>
>
>> +- msi-parent: Link to the hardware entity that serves as the Message
>> +- pinctrl-names : The pin control state names
>> +- pinctrl-0: The "default" pinctrl state
>
> I'm not sure if pinctrl-properties need to be described when you don't need
> special handling in the form of additional pin states. The pcie part does
> not do any pin-handling of its own.
>

We need it in prevention of any firmwares change the default state
of #CLKREQ which is useful for ASPM. Also we have a backup pin for
clkreqn called clkreqnb, which should be taken more consideration since
when refering to any one of these two, pinctrl should configure the
bit[14] of GRF_SOC_CON7 automatically. But it is unfortunately beyound
the implementation of pinctrl-rk3399.

BTW, I don't know if we wanna support this action inside the pinctrl
code?

>
>> +- interrupt-map-mask and interrupt-map: standard PCI properties
>> +- interrupt-controller: identifies the node as an interrupt controller
>> +
>> +Optional Property:
>> +- ep-gpios: contain the entry for pre-reset gpio
>> +- num-lanes: number of lanes to use
>> +- assigned-clocks, assigned-clock-parents and assigned-clock-rates:
>> standard +		   clock bindings. See ../clock/clock-bindings.txt
>
> Again that (assigned-clocks handling) is not actual part of the pci-
> controllers actions, but other parts and also described already elsewhere.
>

Basically it does. But this is an alternative choice for pcie-phy to
generate the ref_clk. When we want 100MHz src clk for PLL inside the
pcie-phy,we should add them, otherwise it's taken from xin 24MHz.

This is useful for SI testing or some others special cases. So should we
add it as an option and leave a sample here?

>
> Heiko
>
>
>


-- 
Best Regards
Shawn Lin

[toc] | [prev] | [next] | [standalone]


#1405596 — Re: [PATCH 1/2] Documentation: add binding description of Rockchip PCIe controller

FromHeiko Stuebner <heiko@sntech.de>
Date2016-05-23 22:00 +0200
SubjectRe: [PATCH 1/2] Documentation: add binding description of Rockchip PCIe controller
Message-ID<rC4b7-1Ic-1@gated-at.bofh.it>
In reply to#1404804
Am Samstag, 21. Mai 2016, 11:55:35 schrieb Shawn Lin:
> On 2016/5/20 19:20, Heiko Stuebner wrote:
> > Hi Shawn,
> > 
> > Am Freitag, 20. Mai 2016, 18:29:06 schrieb Shawn Lin:
> >> This patch add some required and optional properties for Rockchip
> >> PCIe controller. Also we add a example for how to use it.
> >> 
> >> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> >> 
> >> ---

[...]

> >> +- msi-parent: Link to the hardware entity that serves as the Message
> >> +- pinctrl-names : The pin control state names
> >> +- pinctrl-0: The "default" pinctrl state
> > 
> > I'm not sure if pinctrl-properties need to be described when you don't
> > need special handling in the form of additional pin states. The pcie
> > part does not do any pin-handling of its own.
> 
> We need it in prevention of any firmwares change the default state
> of #CLKREQ which is useful for ASPM. Also we have a backup pin for
> clkreqn called clkreqnb, which should be taken more consideration since
> when refering to any one of these two, pinctrl should configure the
> bit[14] of GRF_SOC_CON7 automatically. But it is unfortunately beyound
> the implementation of pinctrl-rk3399.
> 
> BTW, I don't know if we wanna support this action inside the pinctrl
> code?

The TRM says for me for that bit only "pcie_clkreq_sel port control" and 
that naming really suggests that it is a property of the pcie controller, 
not the generic pinctrl. So if this needs to be touched the pcie controller 
needs to do it.


> >> +- interrupt-map-mask and interrupt-map: standard PCI properties
> >> +- interrupt-controller: identifies the node as an interrupt controller
> >> +
> >> +Optional Property:
> >> +- ep-gpios: contain the entry for pre-reset gpio
> >> +- num-lanes: number of lanes to use
> >> +- assigned-clocks, assigned-clock-parents and assigned-clock-rates:
> >> standard +		   clock bindings. See ../clock/clock-bindings.txt
> > 
> > Again that (assigned-clocks handling) is not actual part of the pci-
> > controllers actions, but other parts and also described already
> > elsewhere.
> Basically it does. But this is an alternative choice for pcie-phy to
> generate the ref_clk. When we want 100MHz src clk for PLL inside the
> pcie-phy,we should add them, otherwise it's taken from xin 24MHz.
> 
> This is useful for SI testing or some others special cases. So should we
> add it as an option and leave a sample here?

What I meant was that while clock handling is important when looking at the 
whole system, the pcie controller itself does only care that it gets a 
clock, but not that much where you get it from.

So while assigned-clocks has its place in the real devicetree, I don't think 
it is an element of the actual pcie-controller binding.


Heiko

[toc] | [prev] | [next] | [standalone]


#1405767 — Re: [PATCH 1/2] Documentation: add binding description of Rockchip PCIe controller

FromShawn Lin <shawn.lin@rock-chips.com>
Date2016-05-24 03:50 +0200
SubjectRe: [PATCH 1/2] Documentation: add binding description of Rockchip PCIe controller
Message-ID<rC9DQ-51L-1@gated-at.bofh.it>
In reply to#1405596
On 2016/5/24 3:53, Heiko Stuebner wrote:
> Am Samstag, 21. Mai 2016, 11:55:35 schrieb Shawn Lin:
>> On 2016/5/20 19:20, Heiko Stuebner wrote:
>>> Hi Shawn,
>>>
>>> Am Freitag, 20. Mai 2016, 18:29:06 schrieb Shawn Lin:
>>>> This patch add some required and optional properties for Rockchip
>>>> PCIe controller. Also we add a example for how to use it.
>>>>
>>>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>>>
>>>> ---
>
> [...]
>
>>>> +- msi-parent: Link to the hardware entity that serves as the Message
>>>> +- pinctrl-names : The pin control state names
>>>> +- pinctrl-0: The "default" pinctrl state
>>>
>>> I'm not sure if pinctrl-properties need to be described when you don't
>>> need special handling in the form of additional pin states. The pcie
>>> part does not do any pin-handling of its own.
>>
>> We need it in prevention of any firmwares change the default state
>> of #CLKREQ which is useful for ASPM. Also we have a backup pin for
>> clkreqn called clkreqnb, which should be taken more consideration since
>> when refering to any one of these two, pinctrl should configure the
>> bit[14] of GRF_SOC_CON7 automatically. But it is unfortunately beyound
>> the implementation of pinctrl-rk3399.
>>
>> BTW, I don't know if we wanna support this action inside the pinctrl
>> code?
>
> The TRM says for me for that bit only "pcie_clkreq_sel port control" and
> that naming really suggests that it is a property of the pcie controller,
> not the generic pinctrl. So if this needs to be touched the pcie controller
> needs to do it.

I don't agree that pcie controller should do it. As a common driver, it
should not care two much setting related to io selection which is very
likely to be changed in the future Socs. Shuld it always keep a
reference to bit[ABC] of GRF_SOC_CONXYZ, and should it adds some code
to see which IO is selected for #CLKREQ?

Currently I do it in firmware, but it's worth to make some discussion
as there are also some IO backup slelections the GRF of RK3399. Anyway,
let's skip this topic from the $SUBJECT patch.

>
>
>>>> +- interrupt-map-mask and interrupt-map: standard PCI properties
>>>> +- interrupt-controller: identifies the node as an interrupt controller
>>>> +
>>>> +Optional Property:
>>>> +- ep-gpios: contain the entry for pre-reset gpio
>>>> +- num-lanes: number of lanes to use
>>>> +- assigned-clocks, assigned-clock-parents and assigned-clock-rates:
>>>> standard +		   clock bindings. See ../clock/clock-bindings.txt
>>>
>>> Again that (assigned-clocks handling) is not actual part of the pci-
>>> controllers actions, but other parts and also described already
>>> elsewhere.
>> Basically it does. But this is an alternative choice for pcie-phy to
>> generate the ref_clk. When we want 100MHz src clk for PLL inside the
>> pcie-phy,we should add them, otherwise it's taken from xin 24MHz.
>>
>> This is useful for SI testing or some others special cases. So should we
>> add it as an option and leave a sample here?
>
> What I meant was that while clock handling is important when looking at the
> whole system, the pcie controller itself does only care that it gets a
> clock, but not that much where you get it from.
>
> So while assigned-clocks has its place in the real devicetree, I don't think
> it is an element of the actual pcie-controller binding.

Oh, I see.. So it seems good to keep all the assigned-clocks in on
place. I will remove it from this patch.

>
>
> Heiko
>
>
>


-- 
Best Regards
Shawn Lin

[toc] | [prev] | [next] | [standalone]


#1408988 — Re: [PATCH 1/2] Documentation: add binding description of Rockchip PCIe controller

FromMarc Zyngier <marc.zyngier@arm.com>
Date2016-05-30 13:10 +0200
SubjectRe: [PATCH 1/2] Documentation: add binding description of Rockchip PCIe controller
Message-ID<rEtf3-3mA-7@gated-at.bofh.it>
In reply to#1404330
On Fri, 20 May 2016 18:29:06 +0800
Shawn Lin <shawn.lin@rock-chips.com> wrote:

> This patch add some required and optional properties for Rockchip
> PCIe controller. Also we add a example for how to use it.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> 
> ---
> 
>  .../devicetree/bindings/pci/rockchip-pcie.txt      | 93 ++++++++++++++++++++++
>  1 file changed, 93 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> 
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> new file mode 100644
> index 0000000..69a0804
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> @@ -0,0 +1,93 @@
> +* Rockchip AXI PCIe Root Port Bridge DT description
> +
> +Required properties:
> +- #address-cells: Address representation for root ports, set to <3>
> +- #size-cells: Size representation for root ports, set to <2>
> +- #interrupt-cells: specifies the number of cells needed to encode an
> +		interrupt source. The value must be 1.
> +- compatible: Should contain "rockchip,rk3399-pcie"
> +- reg: Two register ranges as listed in the reg-names property
> +- reg-names: The first entry must be "axi-base" for the core registers
> +	The second entry must be "apb-base" for the client pcie registers
> +- clocks: Must contain an entry for each entry in clock-names.
> +		See ../clocks/clock-bindings.txt for details.
> +- clock-names: Must include the following entries:
> +	- "aclk_pcie"
> +	- "aclk_perf_pcie"
> +	- "hclk_pcie"
> +	- "clk_pciephy_ref"
> +- interrupts: Three interrupt entries must be specified.
> +- interrupt-names: Must include the following names
> +	- "pcie-sys"
> +	- "pcie-legacy"
> +	- "pcie-client"
> +- resets: Must contain five entries for each entry in reset-names.
> +	   See ../reset/reset.txt for details.
> +- reset-names: Must include the following names
> +	- "phy-rst"
> +	- "core-rst"
> +	- "mgmt-rst"
> +	- "mgmt-sticky-rst"
> +	- "pipe-rst"
> +- rockchip,grf: phandle to the syscon managing the "general register files"
> +- pcie-conf: offset of pcie client block for  configuration
> +- pcie-status: offset of pcie client block for status
> +- pcie-laneoff: offset of pcie client block for lane
> +- msi-parent: Link to the hardware entity that serves as the Message
> +- pinctrl-names : The pin control state names
> +- pinctrl-0: The "default" pinctrl state
> +- interrupt-map-mask and interrupt-map: standard PCI properties
> +- interrupt-controller: identifies the node as an interrupt controller
> +
> +Optional Property:
> +- ep-gpios: contain the entry for pre-reset gpio
> +- num-lanes: number of lanes to use
> +- assigned-clocks, assigned-clock-parents and assigned-clock-rates: standard
> +		   clock bindings. See ../clock/clock-bindings.txt
> +
> +Example:
> +
> +pci_express: axi-pcie@f8000000 {
> +	#address-cells = <3>;
> +	#size-cells = <2>;
> +	compatible = "rockchip,rk3399-pcie";
> +	clocks = <&cru ACLK_PCIE>, <&cru ACLK_PERF_PCIE>,
> +		 <&cru PCLK_PCIE>, <&cru SCLK_PCIEPHY_REF>;
> +	clock-names = "aclk_pcie", "aclk_perf_pcie",
> +		      "hclk_pcie", "clk_pciephy_ref";
> +	bus-range = <0x0 0x1>;
> +	interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>,
> +		     <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
> +	interrupt-names: "pcie-sys", "pcie-legacy", "pcie-client";
> +	assigned-clocks = <&cru SCLK_PCIEPHY_REF>;
> +	assigned-clock-parents = <&cru SCLK_PCIEPHY_REF100M>;
> +	assigned-clock-rates = <100000000>;
> +	ep-gpios = <&gpio3 13 GPIO_ACTIVE_HIGH>;
> +	ranges = < 0x82000000 0 0xfa000000 0x0 0xfa000000 0 0x600000
> +		   0x81000000 0 0xfa600000 0x0 0xfa600000 0 0x100000 >;
> +	num-lanes = <4>;
> +	reg = < 0x0 0xf8000000 0x0 0x2000000 >, < 0x0 0xfd000000 0x0 0x1000000 >;
> +	reg-name = "axi-base", "apb-base";
> +	resets = <&cru SRST_PCIEPHY>, <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
> +		 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>;
> +	reset-names = "phy-rst", "core-rst", "mgmt-rst", "mgmt-sticky-rst", "pipe-rst";
> +	rockchip,grf = <&grf>;
> +	pcie-conf = <0xe220>;
> +	pcie-status = <0xe2a4>;
> +	pcie-laneoff = <0xe214>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie_clkreq>;
> +	msi-parent = <&its>;

We've moved away from bare msi-parent for the ITS (notice that the
binding mandates #msi-cells = <1>). So what you need here is an msi-map
(as described in Documentation/devicetree/bindings/pci/pci-msi.txt).

Do you have an IOMMU (or some other remapping HW) between the RC and the
GIC ITS? If so, yo may have to describe the mappings between the PCIe
RID and the ITS DevID in this msi-map.

> +	#interrupt-cells = <1>;
> +	interrupt-map-mask = <0 0 0 7>;
> +	interrupt-map = <0 0 0 1 &pcie_0 1>,
> +			<0 0 0 2 &pcie_0 2>,
> +			<0 0 0 3 &pcie_0 3>,
> +			<0 0 0 4 &pcie_0 4>;
> +	pcie_0: interrupt-controller {
> +		interrupt-controller;
> +		#address-cells = <0>;
> +		#interrupt-cells = <1>;
> +	};
> +
> +};

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web