| Message ID | 1463740146-7106-1-git-send-email-shawn.lin@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 > > >
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
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 > > >
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.
Hi Marc, 在 2016/5/30 19:08, Marc Zyngier 写道: > 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. > yes, we don't need msi-parent here. And we don't need msi-map as well, since we don't have a IOMMU block between RC and ITS which need to be described in msi-map. Thanks. >> + #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. >
On 31/05/16 10:48, Shawn Lin wrote: > Hi Marc, > > 在 2016/5/30 19:08, Marc Zyngier 写道: >> 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. >> > > yes, we don't need msi-parent here. And we don't need msi-map as well, > since we don't have a IOMMU block between RC and ITS which need to be > described in msi-map. Well, if have neither msi-parent nor msi-map, you won't get MSI at all. Maybe that's good enough for you, but in that case, make sure you drop all the MSI code from your patches! ;-) More seriously, you do need an msi-map, even if that's with a single entry. Thanks, M.
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>; + }; + +};
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