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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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@xxxxxxxxxxxxxx>

---

[...]

+- 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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux