mbox series

[0/6] Add DW PCIe support for Exynos5433 SoCs

Message ID 20201019094715.15343-1-m.szyprowski@samsung.com
Headers show
Series Add DW PCIe support for Exynos5433 SoCs | expand

Message

Marek Szyprowski Oct. 19, 2020, 9:47 a.m. UTC 
Dear All,

This patchset is a resurrection of the DW PCIe support for the Exynos5433
SoCs posted long time ago here: https://lkml.org/lkml/2016/12/26/6 and
later here: https://lkml.org/lkml/2017/12/21/296 .

In meantime the support for the Exynos5440 SoCs has been completely
dropped from mainline kernel, as those SoCs never reached the market. The
PCIe driver for Exynos5440 variant however has not been removed yet. This
patchset simply reworks it to support the Exynos5433 variant. The lack of
the need to support both variants significantly simplifies the driver
code.

Best regards,
Marek Szyprowski


Patch summary:

Jaehoon Chung (5):
  Documetation: dt-bindings: add the samsung,exynos-pcie binding
  Documetation: dt-bindings: add the samsung,exynos-pcie-phy binding
  phy: samsung: phy-exynos-pcie: rework driver to support Exynos5433
    PCIe PHY
  pci: dwc: pci-exynos: rework the driver to support Exynos5433 variant
  arm64: dts: exynos: add the WiFi/PCIe support to TM2(e) boards

Marek Szyprowski (1):
  Documetation: dt-bindings: drop samsung,exynos5440-pcie binding

 .../bindings/pci/samsung,exynos-pcie.yaml     | 104 ++++++
 .../bindings/pci/samsung,exynos5440-pcie.txt  |  58 ---
 .../bindings/phy/samsung,exynos-pcie-phy.yaml |  51 +++
 .../boot/dts/exynos/exynos5433-pinctrl.dtsi   |   2 +-
 .../dts/exynos/exynos5433-tm2-common.dtsi     |  24 +-
 arch/arm64/boot/dts/exynos/exynos5433.dtsi    |  36 ++
 drivers/pci/controller/dwc/Kconfig            |   3 +-
 drivers/pci/controller/dwc/pci-exynos.c       | 358 +++++++-----------
 drivers/pci/quirks.c                          |   1 +
 drivers/phy/samsung/phy-exynos-pcie.c         | 304 ++++++---------
 10 files changed, 472 insertions(+), 469 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
 delete mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
 create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml

Comments

Krzysztof Kozlowski Oct. 19, 2020, 10:08 a.m. UTC | #1 
On Mon, Oct 19, 2020 at 11:47:10AM +0200, Marek Szyprowski wrote:
> Exynos5440 SoC support has been dropped since commit 8c83315da1cf ("ARM:
> dts: exynos: Remove Exynos5440"). Drop the obsolete bindings for
> exynos5440-pcie.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  .../bindings/pci/samsung,exynos5440-pcie.txt  | 58 -------------------
>  1 file changed, 58 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof
Krzysztof Kozlowski Oct. 19, 2020, 10:16 a.m. UTC | #2 
On Mon, Oct 19, 2020 at 11:47:13AM +0200, Marek Szyprowski wrote:
> From: Jaehoon Chung <jh80.chung@samsung.com>
> 
> Exynos5440 SoC support has been dropped since commit 8c83315da1cf ("ARM:
> dts: exynos: Remove Exynos5440"). Rework this driver to support PCIe PHY
> variant found in the Exynos5433 SoCs.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> [mszyprow: reworked the driver to support only Exynos5433 variant, rebased
> 	   onto current kernel code, rewrote commit message]
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/phy/samsung/phy-exynos-pcie.c | 304 ++++++++++----------------
>  1 file changed, 112 insertions(+), 192 deletions(-)
> 

Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof
Krzysztof Kozlowski Oct. 19, 2020, 10:18 a.m. UTC | #3 
On Mon, 19 Oct 2020 at 12:12, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Mon, Oct 19, 2020 at 11:47:11AM +0200, Marek Szyprowski wrote:
> > From: Jaehoon Chung <jh80.chung@samsung.com>
> >
> > Add dt-bindings for the Samsung Exynos PCIe controller (Exynos5433
> > variant).
>
> The title has typo and actually entire "Doc" should be dropped. Just
> "dt-bindings: pci:".  This applies to all DT patches.
>
> >
> > Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> > [mszyprow: updated the binding to latest driver changes, rewrote it in yaml,
> >          rewrote commit message]
>
> If you wrote them in YAML it should be a new patch of yours. It is the
> same then as converting TXT to YAML.
>
> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > ---
> >  .../bindings/pci/samsung,exynos-pcie.yaml     | 106 ++++++++++++++++++
> >  1 file changed, 106 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
> > new file mode 100644
> > index 000000000000..48fb569c238c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
> > @@ -0,0 +1,104 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pci/samsung,exynos-pcie.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Samsung SoC series PCIe Host Controller Device Tree Bindings
> > +
> > +maintainers:
> > +  - Jaehoon Chung <jh80.chung@samsung.com>
> > +
> > +description: |+
> > +  Exynos5433 SoC PCIe host controller is based on the Synopsys DesignWare
> > +  PCIe IP and thus inherits all the common properties defined in
> > +  designware-pcie.txt.
> > +
> > +allOf:
> > +  - $ref: /schemas/pci/pci-bus.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - samsung,exynos5433-pcie
>
> const, not enum
>
> > +
> > +  reg:
> > +    items:
> > +      - description: External Local Bus interface (ELBI) registers.
> > +      - description: Data Bus Interface (DBI) registers.
> > +      - description: PCIe configuration space region.
> > +
> > +  reg-names:
> > +    items:
> > +      - const: elbi
> > +      - const: bdi
> > +      - const: config
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: PCIe bridge clock
> > +      - description: PCIe bus clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: pcie
> > +      - const: pcie_bus
> > +
> > +  phys:
> > +    maxItems: 1
> > +
> > +  phy-names:
> > +    const: pcie-phy
> > +
> > +  vdd10-supply:
> > +    description:
> > +      Phandle to a regulator that provides 1.0V power to the PCIe block.
> > +
> > +  vdd18-supply:
> > +    description:
> > +      Phandle to a regulator that provides 1.8V power to the PCIe block.
> > +
> > +required:
> > +  - reg
> > +  - reg-names
> > +  - interrupts
> > +  - interrupt-names
> > +  - clocks
> > +  - clock-names
> > +  - phys
> > +  - phy-names
> > +  - vdd10-supply
>
> additionalProperties: false

This can be unevaluatedProperties, since you include pci-bus schema.
However still you should either include designware schema or include
it's properties here.

Best regards,
Krzysztof
Marek Szyprowski Oct. 21, 2020, 11:59 a.m. UTC | #4 
Hi Krzysztof,

On 19.10.2020 12:18, Krzysztof Kozlowski wrote:
> On Mon, 19 Oct 2020 at 12:12, Krzysztof Kozlowski <krzk@kernel.org> wrote:

>> On Mon, Oct 19, 2020 at 11:47:11AM +0200, Marek Szyprowski wrote:

>>> From: Jaehoon Chung <jh80.chung@samsung.com>

>>>

>>> Add dt-bindings for the Samsung Exynos PCIe controller (Exynos5433

>>> variant).

>> The title has typo and actually entire "Doc" should be dropped. Just

>> "dt-bindings: pci:".  This applies to all DT patches.

>>

>>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>

>>> [mszyprow: updated the binding to latest driver changes, rewrote it in yaml,

>>>           rewrote commit message]

>> If you wrote them in YAML it should be a new patch of yours. It is the

>> same then as converting TXT to YAML.

>>

>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

>>> ---

>>>   .../bindings/pci/samsung,exynos-pcie.yaml     | 106 ++++++++++++++++++

>>>   1 file changed, 106 insertions(+)

>>>   create mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml

>>>

>>> diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml

>>> new file mode 100644

>>> index 000000000000..48fb569c238c

>>> --- /dev/null

>>> +++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml

>>> @@ -0,0 +1,104 @@

>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

>>> +%YAML 1.2

>>> +---

>>> +$id: https://protect2.fireeye.com/v1/url?k=a6caf3f8-fb18e55d-a6cb78b7-0cc47a31bee8-bb3776dee0a03bbb&q=1&e=5f1b0c1e-e4d1-4ae2-b527-8cd5ec52695f&u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fpci%2Fsamsung%2Cexynos-pcie.yaml%23

>>> +$schema: https://protect2.fireeye.com/v1/url?k=591573a2-04c76507-5914f8ed-0cc47a31bee8-bd08b2eac7a5040d&q=1&e=5f1b0c1e-e4d1-4ae2-b527-8cd5ec52695f&u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23

>>> +

>>> +title: Samsung SoC series PCIe Host Controller Device Tree Bindings

>>> +

>>> +maintainers:

>>> +  - Jaehoon Chung <jh80.chung@samsung.com>

>>> +

>>> +description: |+

>>> +  Exynos5433 SoC PCIe host controller is based on the Synopsys DesignWare

>>> +  PCIe IP and thus inherits all the common properties defined in

>>> +  designware-pcie.txt.

>>> +

>>> +allOf:

>>> +  - $ref: /schemas/pci/pci-bus.yaml#

>>> +

>>> +properties:

>>> +  compatible:

>>> +    enum:

>>> +      - samsung,exynos5433-pcie

>> const, not enum

>>

>>> +

>>> +  reg:

>>> +    items:

>>> +      - description: External Local Bus interface (ELBI) registers.

>>> +      - description: Data Bus Interface (DBI) registers.

>>> +      - description: PCIe configuration space region.

>>> +

>>> +  reg-names:

>>> +    items:

>>> +      - const: elbi

>>> +      - const: bdi

>>> +      - const: config

>>> +

>>> +  interrupts:

>>> +    maxItems: 1

>>> +

>>> +  clocks:

>>> +    items:

>>> +      - description: PCIe bridge clock

>>> +      - description: PCIe bus clock

>>> +

>>> +  clock-names:

>>> +    items:

>>> +      - const: pcie

>>> +      - const: pcie_bus

>>> +

>>> +  phys:

>>> +    maxItems: 1

>>> +

>>> +  phy-names:

>>> +    const: pcie-phy

>>> +

>>> +  vdd10-supply:

>>> +    description:

>>> +      Phandle to a regulator that provides 1.0V power to the PCIe block.

>>> +

>>> +  vdd18-supply:

>>> +    description:

>>> +      Phandle to a regulator that provides 1.8V power to the PCIe block.

>>> +

>>> +required:

>>> +  - reg

>>> +  - reg-names

>>> +  - interrupts

>>> +  - interrupt-names

>>> +  - clocks

>>> +  - clock-names

>>> +  - phys

>>> +  - phy-names

>>> +  - vdd10-supply

>> additionalProperties: false

> This can be unevaluatedProperties, since you include pci-bus schema.

> However still you should either include designware schema or include

> it's properties here.


Frankly, I would like to include designware-pci bindling/schema, but it 
has not been converted to yaml yet. I don't feel that I know PCI enough 
to do that conversion...

Best regards

-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Marek Szyprowski Oct. 21, 2020, 12:05 p.m. UTC | #5 
Hi Rob,

On 19.10.2020 15:38, Rob Herring wrote:
> On Mon, Oct 19, 2020 at 4:47 AM Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
>> From: Jaehoon Chung <jh80.chung@samsung.com>
>>
>> Add dt-bindings for the Samsung Exynos PCIe controller (Exynos5433
>> variant).
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> [mszyprow: updated the binding to latest driver changes, rewrote it in yaml,
>>             rewrote commit message]
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>   .../bindings/pci/samsung,exynos-pcie.yaml     | 106 ++++++++++++++++++
>>   1 file changed, 106 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
>> new file mode 100644
>> index 000000000000..48fb569c238c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
>> @@ -0,0 +1,104 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: https://protect2.fireeye.com/v1/url?k=3dfd0348-6067aaeb-3dfc8807-002590f5b904-a68fd848316a7cc4&q=1&e=261ae2d1-4457-43b7-8727-35f3cfbc45c0&u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fpci%2Fsamsung%2Cexynos-pcie.yaml%23
>> +$schema: https://protect2.fireeye.com/v1/url?k=ab825ba1-f618f202-ab83d0ee-002590f5b904-4aba44c12cb70753&q=1&e=261ae2d1-4457-43b7-8727-35f3cfbc45c0&u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
>> +
>> +title: Samsung SoC series PCIe Host Controller Device Tree Bindings
>> +
>> +maintainers:
>> +  - Jaehoon Chung <jh80.chung@samsung.com>
>> +
>> +description: |+
>> +  Exynos5433 SoC PCIe host controller is based on the Synopsys DesignWare
>> +  PCIe IP and thus inherits all the common properties defined in
>> +  designware-pcie.txt.
>> +
>> +allOf:
>> +  - $ref: /schemas/pci/pci-bus.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - samsung,exynos5433-pcie
>> +
>> +  reg:
>> +    items:
>> +      - description: External Local Bus interface (ELBI) registers.
>> +      - description: Data Bus Interface (DBI) registers.
>> +      - description: PCIe configuration space region.
>> +
>> +  reg-names:
>> +    items:
>> +      - const: elbi
>> +      - const: bdi
> dbi
>
>> +      - const: config
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    items:
>> +      - description: PCIe bridge clock
>> +      - description: PCIe bus clock
>> +
>> +  clock-names:
>> +    items:
>> +      - const: pcie
>> +      - const: pcie_bus
>> +
>> +  phys:
>> +    maxItems: 1
>> +
>> +  phy-names:
>> +    const: pcie-phy
> Kind of a pointless name.

Most of the other PCI(e) drivers uses such:

# git grep "phy-names =" Documentation/devicetree/bindings/pci/

Do you want me to change it to simple "pcie"?

>> +
>> +  vdd10-supply:
>> +    description:
>> +      Phandle to a regulator that provides 1.0V power to the PCIe block.
>> +
>> +  vdd18-supply:
>> +    description:
>> +      Phandle to a regulator that provides 1.8V power to the PCIe block.
>> +
>> +required:
>> +  - reg
>> +  - reg-names
>> +  - interrupts
>> +  - interrupt-names
>> +  - clocks
>> +  - clock-names
>> +  - phys
>> +  - phy-names
>> +  - vdd10-supply
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/clock/exynos5433.h>
>> +
>> +    pcie: pcie@15700000 {
>> +        compatible = "samsung,exynos5433-pcie";
>> +        reg = <0x156b0000 0x1000>, <0x15700000 0x1000>, <0x0c000000 0x1000>;
>> +        reg-names = "elbi", "dbi", "config";
>> +        #address-cells = <3>;
>> +        #size-cells = <2>;
>> +        #interrupt-cells = <1>;
>> +        device_type = "pci";
>> +        interrupts = <GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>;
>> +        clocks = <&cmu_fsys CLK_PCIE>, <&cmu_fsys CLK_PCLK_PCIE_PHY>;
>> +        clock-names = "pcie", "pcie_bus";
>> +        phys = <&pcie_phy>;
>> +        phy-names = "pcie-phy";
>> +        pinctrl-names = "default";
>> +        pinctrl-0 = <&pcie_bus &pcie_wlanen>;
>> +        num-lanes = <1>;
>> +        bus-range = <0x00 0xff>;
>> +        ranges = <0x81000000 0 0         0x0c001000 0 0x00010000>,
>> +                 <0x82000000 0 0x0c011000 0x0c011000 0 0x03feefff>;
>> +        vdd10-supply = <&ldo6_reg>;
>> +        vdd18-supply = <&ldo7_reg>;
>> +        iterrupt-map-mask = <0 0 0 0>;
> typo
>
>> +        interrupt-map = <0 0 0 0 &gic GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>;
>> +    };
>> --
>> 2.17.1
>>
Best regards
Krzysztof Kozlowski Oct. 21, 2020, 12:12 p.m. UTC | #6 
On Wed, Oct 21, 2020 at 01:59:25PM +0200, Marek Szyprowski wrote:
 >>> +required:

> >>> +  - reg

> >>> +  - reg-names

> >>> +  - interrupts

> >>> +  - interrupt-names

> >>> +  - clocks

> >>> +  - clock-names

> >>> +  - phys

> >>> +  - phy-names

> >>> +  - vdd10-supply

> >> additionalProperties: false

> > This can be unevaluatedProperties, since you include pci-bus schema.

> > However still you should either include designware schema or include

> > it's properties here.

> 

> Frankly, I would like to include designware-pci bindling/schema, but it 

> has not been converted to yaml yet. I don't feel that I know PCI enough 

> to do that conversion...


I think you need then include all properties in your dtschema. Otherwise
DTS will not pass (neither the example here) the checks.

Best regards,
Krzysztof