| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 18 Sep 2020 11:18:19 +0800
From: Hongtao Wu <wuht06@...il.com>
To: Rob Herring <robh@...nel.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
Orson Zhai <orsonzhai@...il.com>,
Baolin Wang <baolin.wang7@...il.com>,
Chunyan Zhang <zhang.lyra@...il.com>,
linux-pci@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, Hongtao Wu <billows.wu@...soc.com>
Subject: Re: [PATCH v3 1/2] dt-bindings: PCI: sprd: Document Unisoc PCIe RC
host controller
On Wed, Sep 16, 2020 at 1:25 AM Rob Herring <robh@...nel.org> wrote:
>
> On Wed, Sep 09, 2020 at 05:48:31PM +0800, Hongtao Wu wrote:
> > From: Hongtao Wu <billows.wu@...soc.com>
> >
> > This series adds PCIe bindings for Unisoc SoCs.
> > This controller is based on DesignWare PCIe IP.
> >
> > Signed-off-by: Hongtao Wu <billows.wu@...soc.com>
> > ---
> > .../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@...soc.com>
> > +
> > +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@...00000 {
> > + 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
> >
Powered by blists - more mailing lists