On Wed, Sep 16, 2020 at 1:25 AM Rob Herring <robh@xxxxxxxxxx> wrote: > > On Wed, Sep 09, 2020 at 05:48:31PM +0800, Hongtao Wu wrote: > > From: Hongtao Wu <billows.wu@xxxxxxxxxx> > > > > This series adds PCIe bindings for Unisoc SoCs. > > This controller is based on DesignWare PCIe IP. > > > > Signed-off-by: Hongtao Wu <billows.wu@xxxxxxxxxx> > > --- > > .../devicetree/bindings/pci/sprd-pcie.yaml | 101 +++++++++++++++++++++ > > 1 file changed, 101 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/pci/sprd-pcie.yaml > > > > diff --git a/Documentation/devicetree/bindings/pci/sprd-pcie.yaml b/Documentation/devicetree/bindings/pci/sprd-pcie.yaml > > new file mode 100644 > > index 0000000..c52edfb > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/pci/sprd-pcie.yaml > > @@ -0,0 +1,101 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/pci/sprd-pcie.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: SoC PCIe Host Controller Device Tree Bindings > > + > > +maintainers: > > + - Hongtao Wu <billows.wu@xxxxxxxxxx> > > + > > +allOf: > > + - $ref: /schemas/pci/pci-bus.yaml# > > + > > +properties: > > + compatible: > > + items: > > + - const: sprd,pcie-rc > > + > > + reg: > > + minItems: 2 > > + items: > > + - description: Controller control and status registers. > > + - description: PCIe configuration registers. > > + > > + reg-names: > > + items: > > + - const: dbi > > + - const: config > > + > > + ranges: > > + maxItems: 2 > > + > > + num-lanes: > > + maximum: 1 > > + description: Number of lanes to use for this port. > > + > > + interrupts: > > + minItems: 1 > > + description: Builtin MSI controller and PCIe host controller. > > + > > + interrupt-names: > > + items: > > + - const: msi > > + > > + sprd-pcie-poweron-syscons: > I am Sorry! I'll fix it. > Doesn't match the example. > > > + minItems: 1 > > + description: Global register. > > + The first value is the phandle to the global registers required to > > + confige PCIe phy, clock and so on. > > + The second value is the global register type which indicates whether it > > + is a set/clear register or not. > > + The third value is the time to delay after the global register is set or > > + cleared. > > + The fourth value is the global register address. > > + The fifth value is the the mask value that the global register must > > + be operate. > > + The sixth value is the value that will be set to the global register. > > + Note that Some Unisoc global registers have not been upstreamed. > > + The global register and its mask can't be found in linux kernel, > > + so we use an offset address and a number to instead them. > > From the example, it looks like you set/clear 2 bits for power on/off. > What's the worst case you expect here? What do the 2 bits do? If they > are for clocks, resets, or power domains, then we have bindings for > those which should be used. This use of phandles to syscons should be > avoided whenever possible. > There are two kinds of global register ( set/clear registers and non-set/clear registers ) about PCIe on Unisoc SoCs. Each set of set/clear registers contain two addresses. One can be written and the other one can be read. Different bits in the set/clear register indicate different functions, so we set/clear one bit for power on/off. The non-set/clear registers are normal which only have one address. The second value in property 'sprd,pcie-poweron-syscons' is a flag which indicates whether the global register is set/clear or not. If this value is 1, we think that it's a set/clear register. If this value is 0, we think it's a non-set/clear register. I wanted to parse all of the global registers about power on/off in an array (include set/clear registers and non-set/clear registers). However, it may not be a good idea. I'll split the property 'sprd,pcie-poweron-syscons' info clocks, power domains, phy and so on in the next version. > If we wanted a language for specifying sequences of register accesses in > DT, we would have defined that a long time ago. > > > + > > +required: > > + - compatible > > + - reg > > + - reg-names > > + - num-lanes > > + - ranges > > + - interrupts > > + - interrupt-names > > + > > +examples: > > + - | > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + > > + ipa { > > + #address-cells = <2>; > > + #size-cells = <2>; > > + > > + pcie0: pcie@2b100000 { > > + compatible = "sprd,pcie-rc"; > > + reg = <0x0 0x2b100000 0x0 0x2000>, > > + <0x2 0x00000000 0x0 0x2000>; > > + reg-names = "dbi", "config"; > > + #address-cells = <3>; > > + #size-cells = <2>; > > + device_type = "pci"; > > + ranges = <0x01000000 0x0 0x00000000 0x2 0x00002000 0x0 0x00010000>, > > + <0x03000000 0x0 0x10000000 0x2 0x10000000 0x1 0xefffffff>; > > + num-lanes = <1>; > > + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>; > > + interrupt-names = "msi"; > > + > > + sprd,pcie-poweron-syscons = > > + <&ap_ipa_ahb_regs 0 0 0x0000 0x40 0x40>, > > + <&ap_ipa_ahb_regs 0 0 0x0000 0x20 0x20>; > > + sprd,pcie-poweroff-syscons = > > Not documented. > Thanks! I'll fix it in the next version. > > + <&ap_ipa_ahb_regs 0 0 0x0000 0x20 0x0>, > > + <&ap_ipa_ahb_regs 0 0 0x0000 0x40 0x0>; > > + }; > > + }; > > -- > > 2.7.4 > >
