[PATCH v4 0/5] Add support for Axis, ARTPEC-8 PCIe driver

[PATCH v4 0/5] Add support for Axis, ARTPEC-8 PCIe driver

[Wangseok Lee]

This v4 patchset is improvement several review comments received from patchset v3.

Main changes since v3 [3]:
dt-bindings: pci: Add ARTPEC-8 PCIe controller
-add missing properties

dt-bindings: phy: Add ARTPEC-8 PCIe phy
-add "fsys-sysreg" to properties
-modify the "lcpll-ref-clk" and "clocks" in properties
 "lcpll-ref-clk" is custom properties, so add 'vendor', type(enum),
 description
 Add the maxItem in clocks, add clock-names in properties

PCI: axis: Add ARTPEC-8 PCIe controller driver
-remove unnecessary enum type
-fix indentation

phy: Add ARTPEC-8 PCIe PHY driver
-modify to use GENMASK
-fix indentation
-remove the driver data

Main changes since v2 [2]:
dt-bindings: pci: Add ARTPEC-8 PCIe controller
-modify version history to fit the linux commit rule
-remove 'Device Tree Bindings' on title
-remove the interrupt-names, phy-names entries
-remove '_clk' suffix
-add the compatible entries on required
-change node name to soc from artpec8 on examples

dt-bindings: phy: Add ARTPEC-8 PCIe phy
-modify version history to fit the linux commit rule
-remove 'Device Tree Bindings' on title
-remove clock-names entries
-change node name to soc from artpec8 on excamples

PCI: axis: Add ARTPEC-8 PCIe controller driver
-add 'COMPILE_TEST' and improvement help on kconfig
-reorder obj on makefile
-use clk_bulk_api
-remove unnecessary comment
-redefine the ELBI register to distinguish between offset and
 bit definition
-improvement order local variable of function
-remove unnecessary local return variable

phy: Add ARTPEC-8 PCIe PHY driver
-remove unnecessary indentation
-redefine local struct to statis const
-add static const to struct that requires static const definition
-remove wrappers on writel and readl

Main changes since v1 [1]:
-'make dt_binding_check' result improvement
-Add the missing property list
-improvement review comment of Krzysztof on driver code
-change folder name of phy driver to axis from artpec

[3] https://lore.kernel.org/lkml/20220614011616epcms2p7dcaa67c53b7df5802dd7a697e2d472d7@epcms2p7/T
[2] https://lore.kernel.org/lkml/20220613015023epcms2p70e6700a99042d4deb560e40ab5397001@epcms2p7/T/
[1] https://lore.kernel.org/lkml/20220328014430epcms2p7063834feb0abdf2f38a62723c96c9ff1@epcms2p7/

--------------------------------------------------------------
This series patches include newly PCIe support for Axis ARTPEC-8 SoC.
ARTPEC-8 is the SoC platform of Axis Communications.
PCIe controller driver and phy driver have been newly added.
There is also a new MAINTAINER in the addition of phy driver.
PCIe controller is designed based on Design-Ware PCIe controller IP
and PCIe phy is desinged based on SAMSUNG PHY IP.
It also includes modifications to the Design-Ware controller driver to 
run the 64bit-based ARTPEC-8 PCIe controller driver.
It consists of 6 patches in total.

This series has been tested on AXIS SW bring-up board 
with ARTPEC-8 chipset.
--------------------------------------------------------------

Wangseok Lee (5):
  dt-bindings: pci: Add ARTPEC-8 PCIe controller
  dt-bindings: phy: Add ARTPEC-8 PCIe phy
  PCI: axis: Add ARTPEC-8 PCIe controller driver
  phy: Add ARTPEC-8 PCIe PHY driver
  MAINTAINERS: Add Axis ARTPEC-8 PCIe PHY maintainers

 .../bindings/pci/axis,artpec8-pcie-ep.yaml         | 138 ++++
 .../devicetree/bindings/pci/axis,artpec8-pcie.yaml | 148 ++++
 .../bindings/phy/axis,artpec8-pcie-phy.yaml        |  85 +++
 MAINTAINERS                                        |   2 +
 drivers/pci/controller/dwc/Kconfig                 |  31 +
 drivers/pci/controller/dwc/Makefile                |   1 +
 drivers/pci/controller/dwc/pcie-artpec8.c          | 788 +++++++++++++++++++++
 drivers/phy/Kconfig                                |   1 +
 drivers/phy/Makefile                               |   1 +
 drivers/phy/axis/Kconfig                           |   9 +
 drivers/phy/axis/Makefile                          |   2 +
 drivers/phy/axis/phy-artpec8-pcie.c                | 753 ++++++++++++++++++++
 12 files changed, 1959 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/axis,artpec8-pcie-ep.yaml
 create mode 100644 Documentation/devicetree/bindings/pci/axis,artpec8-pcie.yaml
 create mode 100644 Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
 create mode 100644 drivers/pci/controller/dwc/pcie-artpec8.c
 create mode 100644 drivers/phy/axis/Kconfig
 create mode 100644 drivers/phy/axis/Makefile
 create mode 100644 drivers/phy/axis/phy-artpec8-pcie.c

-- 
2.9.5

[PATCH v4 1/5] dt-bindings: pci: Add ARTPEC-8 PCIe controller

[Wangseok Lee]

Add description to support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC platform
of Axis Communications and PCIe controller is designed based on Design-Ware
PCIe controller.

Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com>
---
v3->v4 :
-Add missing properties

v2->v3 :
-Modify version history to fit the linux commit rule
-Remove 'Device Tree Bindings' on title
-Remove clock-names entries
-Change node name to soc from artpec8 on excamples

v1->v2 :
-'make dt_binding_check' result improvement
-Add the missing property list
-Align the indentation of continued lines/entries
---
 .../bindings/pci/axis,artpec8-pcie-ep.yaml         | 138 +++++++++++++++++++
 .../devicetree/bindings/pci/axis,artpec8-pcie.yaml | 148 +++++++++++++++++++++
 2 files changed, 286 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/axis,artpec8-pcie-ep.yaml
 create mode 100644 Documentation/devicetree/bindings/pci/axis,artpec8-pcie.yaml

diff --git a/Documentation/devicetree/bindings/pci/axis,artpec8-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/axis,artpec8-pcie-ep.yaml
new file mode 100644
index 0000000..435e86f
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/axis,artpec8-pcie-ep.yaml
@@ -0,0 +1,138 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/axis,artpec8-pcie-ep.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ARTPEC-8 SoC PCIe Controller
+
+maintainers:
+  - Jesper Nilsson <jesper.nilsson@axis.com>
+
+description: |+
+  This PCIe end-point controller is based on the Synopsys DesignWare PCIe IP
+  and thus inherits all the common properties defined in snps,dw-pcie-ep.yaml.
+
+allOf:
+  - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
+
+properties:
+  compatible:
+    const: axis,artpec8-pcie-ep
+
+  reg:
+    items:
+      - description: Data Bus Interface (DBI) registers.
+      - description: Data Bus Interface (DBI2) registers.
+      - description: PCIe address space region.
+
+  reg-names:
+    items:
+      - const: dbi
+      - const: dbi2
+      - const: addr_space
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: PIPE clock, used by the controller to clock the PIPE
+      - description: PCIe dbi clock, ungated version
+      - description: PCIe master clock, ungated version
+      - description: PCIe slave clock, ungated version
+
+  clock-names:
+    items:
+      - const: pipe
+      - const: dbi
+      - const: mstr
+      - const: slv
+
+  samsung,fsys-sysreg:
+    description:
+      Phandle to system register of fsys block.
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  samsung,syscon-phandle:
+    description:
+      Phandle to the PMU system controller node.
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  samsung,fsys-bus-s:
+    description:
+      Phandle to bus-s of fsys block, this register
+      is additional control sysreg in fsys block and
+      this is used for pcie slave control setting.
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  samsung,fsys-bus-p:
+    description:
+      Phandle to bus-p of fsys block, this register
+      is additional control sysreg in fsys block and
+      this is used for pcie dbi control setting.
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  phys:
+    maxItems: 1
+
+  phy-names:
+    items:
+      - const: pcie_phy
+
+  num-lanes:
+    const: 2
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - interrupt-names
+  - clocks
+  - clock-names
+  - samsung,fsys-sysreg
+  - samsung,syscon-phandle
+  - samsung,syscon-bus-s-fsys
+  - samsung,syscon-bus-p-fsys
+  - phys
+  - phy-names
+  - num-lanes
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+        pcie_ep: pcie-ep@17200000 {
+            compatible = "axis,artpec8-pcie-ep";
+            reg = <0x0 0x17200000 0x0 0x1000>,
+                  <0x0 0x17201000 0x0 0x1000>,
+                  <0x2 0x00000000 0x6 0x00000000>;
+            reg-names = "dbi", "dbi2", "addr_space";
+            #interrupt-cells = <1>;
+            interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "intr";
+            clocks = <&clock_cmu_fsys 39>,
+                     <&clock_cmu_fsys 38>,
+                     <&clock_cmu_fsys 37>,
+                     <&clock_cmu_fsys 36>;
+            clock-names = "pipe", "dbi", "mstr", "slv";
+            samsung,fsys-sysreg = <&syscon_fsys>;
+            samsung,syscon-phandle = <&pmu_system_controller>;
+            samsung,syscon-bus-s-fsys = <&syscon_bus_s_fsys>;
+            samsung,syscon-bus-p-fsys = <&syscon_bus_p_fsys>;
+            phys = <&pcie_phy>;
+            phy-names = "pcie_phy";
+            num-lanes = <2>;
+            bus-range = <0x00 0xff>;
+            num-ib-windows = <16>;
+            num-ob-windows = <16>;
+        };
+    };
+...
diff --git a/Documentation/devicetree/bindings/pci/axis,artpec8-pcie.yaml b/Documentation/devicetree/bindings/pci/axis,artpec8-pcie.yaml
new file mode 100644
index 0000000..b7cff4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/axis,artpec8-pcie.yaml
@@ -0,0 +1,148 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/axis,artpec8-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Artpec-8 SoC PCIe Controller
+
+maintainers:
+  - Jesper Nilsson <jesper.nilsson@axis.com>
+
+description: |+
+  This PCIe host controller is based on the Synopsys DesignWare PCIe IP
+  and thus inherits all the common properties defined in snps,dw-pcie.yaml.
+
+allOf:
+  - $ref: /schemas/pci/snps,dw-pcie.yaml#
+
+properties:
+  compatible:
+    const: axis,artpec8-pcie
+
+  reg:
+    items:
+      - description: Data Bus Interface (DBI) registers.
+      - description: External Local Bus interface (ELBI) registers.
+      - description: PCIe configuration space region.
+
+  reg-names:
+    items:
+      - const: dbi
+      - const: elbi
+      - const: config
+
+  ranges:
+    maxItems: 2
+
+  num-lanes:
+    const: 2
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: PIPE clock, used by the controller to clock the PIPE
+      - description: PCIe dbi clock, ungated version
+      - description: PCIe master clock,  ungated version
+      - description: PCIe slave clock, ungated version
+
+  clock-names:
+    items:
+      - const: pipe
+      - const: dbi
+      - const: mstr
+      - const: slv
+
+  samsung,fsys-sysreg:
+    description:
+      Phandle to system register of fsys block.
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  samsung,syscon-phandle:
+    description:
+      Phandle to the PMU system controller node.
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  samsung,fsys-bus-s:
+    description:
+      Phandle to bus-s of fsys block, this register
+      is additional control sysreg in fsys block and
+      this is used for pcie slave control setting.
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  samsung,fsys-bus-p:
+    description:
+      Phandle to bus-p of fsys block, this register
+      is additional control sysreg in fsys block and
+      this is used for pcie dbi control setting.
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  phys:
+    maxItems: 1
+
+  phy-names:
+    items:
+      - const: pcie_phy
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - device_type
+  - ranges
+  - num-lanes
+  - interrupts
+  - interrupt-names
+  - clocks
+  - clock-names
+  - samsung,fsys-sysreg
+  - samsung,syscon-phandle
+  - samsung,syscon-bus-s-fsys
+  - samsung,syscon-bus-p-fsys
+  - phys
+  - phy-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+        pcie: pcie@17200000 {
+            compatible = "axis,artpec8-pcie";
+            reg = <0x0 0x17200000 0x0 0x1000>,
+                  <0x0 0x16ca0000 0x0 0x2000>,
+                  <0x7 0x0001e000 0x0 0x2000>;
+            reg-names = "dbi", "elbi", "config";
+            #address-cells = <3>;
+            #size-cells = <2>;
+            device_type = "pci";
+            ranges = </* non-prefetchable memory */
+                      0x83000000 0x0 0x0000000 0x2 0x00000000 0x5 0x00000000
+                      /* downstream I/O */
+                      0x81000000 0x0 0x0000000 0x7 0x00000000 0x0 0x00010000>;
+            num-lanes = <2>;
+            bus-range = <0x00 0xff>;
+            interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "intr";
+            #interrupt-cells = <1>;
+            clocks = <&cmu_fsys 39>,
+                     <&cmu_fsys 38>,
+                     <&cmu_fsys 37>,
+                     <&cmu_fsys 36>;
+            clock-names = "pipe", "dbi", "mstr", "slv";
+            samsung,fsys-sysreg = <&syscon_fsys>;
+            samsung,syscon-phandle = <&pmu_system_controller>;
+            samsung,syscon-bus-s-fsys = <&syscon_bus_s_fsys>;
+            samsung,syscon-bus-p-fsys = <&syscon_bus_p_fsys>;
+            phys = <&pcie_phy>;
+            phy-names = "pcie_phy";
+        };
+    };
+...
-- 
2.9.5

Re: [PATCH v4 1/5] dt-bindings: pci: Add ARTPEC-8 PCIe controller

[Rob Herring]

On Wed, 20 Jul 2022 14:54:36 +0900, Wangseok Lee wrote:
> Add description to support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC platform
> of Axis Communications and PCIe controller is designed based on Design-Ware
> PCIe controller.
> 
> Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com>
> ---
> v3->v4 :
> -Add missing properties
> 
> v2->v3 :
> -Modify version history to fit the linux commit rule
> -Remove 'Device Tree Bindings' on title
> -Remove clock-names entries
> -Change node name to soc from artpec8 on excamples
> 
> v1->v2 :
> -'make dt_binding_check' result improvement
> -Add the missing property list
> -Align the indentation of continued lines/entries
> ---
>  .../bindings/pci/axis,artpec8-pcie-ep.yaml         | 138 +++++++++++++++++++
>  .../devicetree/bindings/pci/axis,artpec8-pcie.yaml | 148 +++++++++++++++++++++
>  2 files changed, 286 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/axis,artpec8-pcie-ep.yaml
>  create mode 100644 Documentation/devicetree/bindings/pci/axis,artpec8-pcie.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/axis,artpec8-pcie-ep.example.dtb: pcie-ep@17200000: Unevaluated properties are not allowed ('#interrupt-cells', 'bus-range', 'interrupt-names', 'samsung,syscon-bus-p-fsys', 'samsung,syscon-bus-s-fsys' were unexpected)
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/axis,artpec8-pcie-ep.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/axis,artpec8-pcie.example.dtb: pcie@17200000: Unevaluated properties are not allowed ('samsung,syscon-bus-p-fsys', 'samsung,syscon-bus-s-fsys' were unexpected)
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/axis,artpec8-pcie.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH v4 1/5] dt-bindings: pci: Add ARTPEC-8 PCIe controller

[Krzysztof Kozlowski]

On 20/07/2022 07:54, Wangseok Lee wrote:
> Add description to support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC platform
> of Axis Communications and PCIe controller is designed based on Design-Ware
> PCIe controller.
> 
> Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com>
> ---
> v3->v4 :
> -Add missing properties
> 
> v2->v3 :
> -Modify version history to fit the linux commit rule
> -Remove 'Device Tree Bindings' on title
> -Remove clock-names entries
> -Change node name to soc from artpec8 on excamples

Please rebase on newest Linux kernel or linux-next and use
get_maintainers.pl script.

> 
> v1->v2 :
> -'make dt_binding_check' result improvement
> -Add the missing property list
> -Align the indentation of continued lines/entries
> ---
>  .../bindings/pci/axis,artpec8-pcie-ep.yaml         | 138 +++++++++++++++++++
>  .../devicetree/bindings/pci/axis,artpec8-pcie.yaml | 148 +++++++++++++++++++++
>  2 files changed, 286 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/axis,artpec8-pcie-ep.yaml
>  create mode 100644 Documentation/devicetree/bindings/pci/axis,artpec8-pcie.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pci/axis,artpec8-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/axis,artpec8-pcie-ep.yaml
> new file mode 100644
> index 0000000..435e86f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/axis,artpec8-pcie-ep.yaml
> @@ -0,0 +1,138 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/axis,artpec8-pcie-ep.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ARTPEC-8 SoC PCIe Controller
> +
> +maintainers:
> +  - Jesper Nilsson <jesper.nilsson@axis.com>
> +
> +description: |+
> +  This PCIe end-point controller is based on the Synopsys DesignWare PCIe IP
> +  and thus inherits all the common properties defined in snps,dw-pcie-ep.yaml.
> +
> +allOf:
> +  - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
> +
> +properties:
> +  compatible:
> +    const: axis,artpec8-pcie-ep
> +
> +  reg:
> +    items:
> +      - description: Data Bus Interface (DBI) registers.
> +      - description: Data Bus Interface (DBI2) registers.
> +      - description: PCIe address space region.
> +
> +  reg-names:
> +    items:
> +      - const: dbi
> +      - const: dbi2
> +      - const: addr_space
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: PIPE clock, used by the controller to clock the PIPE
> +      - description: PCIe dbi clock, ungated version
> +      - description: PCIe master clock, ungated version
> +      - description: PCIe slave clock, ungated version
> +
> +  clock-names:
> +    items:
> +      - const: pipe
> +      - const: dbi
> +      - const: mstr
> +      - const: slv
> +
> +  samsung,fsys-sysreg:
> +    description:
> +      Phandle to system register of fsys block.
> +    $ref: /schemas/types.yaml#/definitions/phandle

Since you wrote this is one register, I expect offset:
https://elixir.bootlin.com/linux/v5.18-rc1/source/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml#L42


> +
> +  samsung,syscon-phandle:
> +    description:
> +      Phandle to the PMU system controller node.
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +
> +  samsung,fsys-bus-s:
> +    description:
> +      Phandle to bus-s of fsys block, this register
> +      is additional control sysreg in fsys block and
> +      this is used for pcie slave control setting.
> +    $ref: /schemas/types.yaml#/definitions/phandle

Ditto

> +
> +  samsung,fsys-bus-p:
> +    description:
> +      Phandle to bus-p of fsys block, this register
> +      is additional control sysreg in fsys block and
> +      this is used for pcie dbi control setting.
> +    $ref: /schemas/types.yaml#/definitions/phandle

Ditto

> +
> +  phys:
> +    maxItems: 1
> +
> +  phy-names:
> +    items:
> +      - const: pcie_phy
> +
> +  num-lanes:
> +    const: 2
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - interrupts
> +  - interrupt-names
> +  - clocks
> +  - clock-names
> +  - samsung,fsys-sysreg
> +  - samsung,syscon-phandle
> +  - samsung,syscon-bus-s-fsys

This does not match what you wrote in properties.

Best regards,
Krzysztof

[PATCH v4 2/5] dt-bindings: phy: Add ARTPEC-8 PCIe phy

[Wangseok Lee]

Add description to support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC platform
of Axis Communications and PCIe PHY is designed based on Samsung PHY.

Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com>
---
v3->v4 :
-Add "fsys-sysreg" to properties
-Modify the "lcpll-ref-clk" and "clocks" in properties
 "lcpll-ref-clk" is custom properties, so add 'vendor', type(enum),
 description
 Add the maxItem in clocks, add clock-names in properties

v2->v3 :
-Modify version history to fit the linux commit rule
-Remove 'Device Tree Bindings' on title
-Remove clock-names entries
-Change node name to soc from artpec8 on excamples

v1->v2 :
-'make dt_binding_check' result improvement
-Add the missing property list
-Align the indentation of continued lines/entries
---
 .../bindings/phy/axis,artpec8-pcie-phy.yaml        | 85 ++++++++++++++++++++++
 1 file changed, 85 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
new file mode 100644
index 0000000..9db39ef
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
@@ -0,0 +1,85 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/axis,artpec8-pcie-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ARTPEC-8 SoC PCIe PHY
+
+maintainers:
+  - Jesper Nilsson <jesper.nilsson@axis.com>
+
+properties:
+  compatible:
+    const: axis,artpec8-pcie-phy
+
+  reg:
+    items:
+      - description: PHY registers.
+      - description: PHY coding sublayer registers.
+
+  reg-names:
+    items:
+      - const: phy
+      - const: pcs
+
+  "#phy-cells":
+    const: 0
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: ref
+
+  samsung,fsys-sysreg:
+    description:
+      Phandle to system register of fsys block.
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  num-lanes:
+    const: 2
+
+  axis,lcpll-ref-clk:
+    description:
+      select the reference clock of phy and initialization is performed
+      with the reference clock according to the selected value.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [ 0, 1, 2, 3, 4 ]
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - "#phy-cells"
+  - clocks
+  - clock-names
+  - samsung,fsys-sysreg
+  - num-lanes
+  - axis,lcpll-ref-clk
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+        pcie_phy: pcie-phy@16c80000 {
+            compatible = "axis,artpec8-pcie-phy";
+            reg = <0x0 0x16c80000 0x0 0x2000>,
+                  <0x0 0x16c90000 0x0 0x1000>;
+            reg-names = "phy", "pcs";
+            #phy-cells = <0>;
+            clocks = <&clock_cmu_fsys 53>;
+            clock-names = "ref";
+            samsung,fsys-sysreg = <&syscon_fsys>;
+            num-lanes = <2>;
+            axis,lcpll-ref-clk = <1>;
+        };
+    };
+...
-- 
2.9.5

Re: [PATCH v4 2/5] dt-bindings: phy: Add ARTPEC-8 PCIe phy

[Krzysztof Kozlowski]

On 20/07/2022 07:57, Wangseok Lee wrote:
> Add description to support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC platform
> of Axis Communications and PCIe PHY is designed based on Samsung PHY.
> 
> Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com>
> ---
> v3->v4 :
> -Add "fsys-sysreg" to properties
> -Modify the "lcpll-ref-clk" and "clocks" in properties
>  "lcpll-ref-clk" is custom properties, so add 'vendor', type(enum),
>  description
>  Add the maxItem in clocks, add clock-names in properties
> 
> v2->v3 :
> -Modify version history to fit the linux commit rule
> -Remove 'Device Tree Bindings' on title
> -Remove clock-names entries
> -Change node name to soc from artpec8 on excamples
> 
> v1->v2 :
> -'make dt_binding_check' result improvement
> -Add the missing property list
> -Align the indentation of continued lines/entries
> ---
>  .../bindings/phy/axis,artpec8-pcie-phy.yaml        | 85 ++++++++++++++++++++++
>  1 file changed, 85 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
> new file mode 100644
> index 0000000..9db39ef
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
> @@ -0,0 +1,85 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/axis,artpec8-pcie-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ARTPEC-8 SoC PCIe PHY
> +
> +maintainers:
> +  - Jesper Nilsson <jesper.nilsson@axis.com>
> +
> +properties:
> +  compatible:
> +    const: axis,artpec8-pcie-phy
> +
> +  reg:
> +    items:
> +      - description: PHY registers.
> +      - description: PHY coding sublayer registers.
> +
> +  reg-names:
> +    items:
> +      - const: phy
> +      - const: pcs
> +
> +  "#phy-cells":
> +    const: 0
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: ref
> +
> +  samsung,fsys-sysreg:
> +    description:
> +      Phandle to system register of fsys block.
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +
> +  num-lanes:
> +    const: 2
> +
> +  axis,lcpll-ref-clk:
> +    description:
> +      select the reference clock of phy and initialization is performed
> +      with the reference clock according to the selected value.
> +    $ref: /schemas/types.yaml#/definitions/uint32

This looks like hardware register value... but after looking at driver
you rather need string... although REF_CLK_RESERVED is confusing. You
need to describe the feature, not some device programming model.

Then looking deeper it seems you just made a workaround around common
clock framework. At least for XO and SOC case - you just implemented a
clock mux in your PHY driver!

Implement a clock driver instead and handle only special case - clock
coming from IO.

Best regards,
Krzysztof

Re: [PATCH v4 2/5] dt-bindings: phy: Add ARTPEC-8 PCIe phy

[Rob Herring]

On Wed, Jul 20, 2022 at 02:57:16PM +0900, Wangseok Lee wrote:
> Add description to support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC platform
> of Axis Communications and PCIe PHY is designed based on Samsung PHY.
> 
> Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com>
> ---
> v3->v4 :
> -Add "fsys-sysreg" to properties
> -Modify the "lcpll-ref-clk" and "clocks" in properties
>  "lcpll-ref-clk" is custom properties, so add 'vendor', type(enum),
>  description
>  Add the maxItem in clocks, add clock-names in properties
> 
> v2->v3 :
> -Modify version history to fit the linux commit rule
> -Remove 'Device Tree Bindings' on title
> -Remove clock-names entries
> -Change node name to soc from artpec8 on excamples
> 
> v1->v2 :
> -'make dt_binding_check' result improvement
> -Add the missing property list
> -Align the indentation of continued lines/entries
> ---
>  .../bindings/phy/axis,artpec8-pcie-phy.yaml        | 85 ++++++++++++++++++++++
>  1 file changed, 85 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
> new file mode 100644
> index 0000000..9db39ef
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
> @@ -0,0 +1,85 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/axis,artpec8-pcie-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ARTPEC-8 SoC PCIe PHY
> +
> +maintainers:
> +  - Jesper Nilsson <jesper.nilsson@axis.com>
> +
> +properties:
> +  compatible:
> +    const: axis,artpec8-pcie-phy
> +
> +  reg:
> +    items:
> +      - description: PHY registers.
> +      - description: PHY coding sublayer registers.
> +
> +  reg-names:
> +    items:
> +      - const: phy
> +      - const: pcs
> +
> +  "#phy-cells":
> +    const: 0
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: ref
> +
> +  samsung,fsys-sysreg:
> +    description:
> +      Phandle to system register of fsys block.
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +
> +  num-lanes:
> +    const: 2

Why do you need num-lanes if 2 is the only possible value?

Rob

[PATCH v4 3/5] PCI: axis: Add ARTPEC-8 PCIe controller driver

[Wangseok Lee]

Add support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC platform of Axis
Communications. This is based on arm64 and support GEN4 & 2lane. This
PCIe controller is based on DesignWare Hardware core and uses DesignWare
core functions to implement the driver. "pcie-artpec6. c" supports artpec6
and artpec7 H/W. artpec8 can not be expanded because H/W configuration is
completely different from artpec6/7. PHY and sub controller are different.

Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com>
Signed-off-by: Jaeho Cho <jaeho79.cho@samsung.com>
---
v3->v4 :
-Remove unnecessary enum type
-Fix indentation

v2->v3 :
-Add 'COMPILE_TEST' and improvement help on kconfig
-Reorder obj on makefile
-Use clk_bulk_api
-Remove unnecessary comment
-Redefine the ELBI register to distinguish between offset and bit
 definition
-Improvement order local variable of function
-Remove unnecessary local return variable

v1->v2 :
Improvement review comment of Krzysztof on driver code.
-Debug messages for probe or other functions.
-Inconsistent coding style (different indentation in structure members)
-Inconsistent code (artpec8_pcie_get_subsystem_resources() gets device
 from pdev and from pci so you have two same pointers; or  artpec8_pcie_
 get_ep_mem_resources() stores dev as local variable but uses instead
 pdev->dev)
-Not using devm_platform_ioremap_resource()
-Printing messages in interrupt handlers
-Several local/static structures or array are not const
---
 drivers/pci/controller/dwc/Kconfig        |  31 ++
 drivers/pci/controller/dwc/Makefile       |   1 +
 drivers/pci/controller/dwc/pcie-artpec8.c | 788 ++++++++++++++++++++++++++++++
 3 files changed, 820 insertions(+)
 create mode 100644 drivers/pci/controller/dwc/pcie-artpec8.c

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 62ce3ab..2b16637 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -222,6 +222,37 @@ config PCIE_ARTPEC6_EP
 	  Enables support for the PCIe controller in the ARTPEC-6 SoC to work in
 	  endpoint mode. This uses the DesignWare core.
 
+config PCIE_ARTPEC8
+	bool "Axis ARTPEC-8 PCIe controller"
+
+config PCIE_ARTPEC8_HOST
+	bool "Axis ARTPEC-8 PCIe controller Host Mode"
+	depends on ARCH_ARTPEC || COMPILE_TEST
+	depends on PCI_MSI_IRQ_DOMAIN
+	depends on PCI_ENDPOINT
+	select PCI_EPF_TEST
+	select PCIE_DW_HOST
+	select PCIE_ARTPEC8
+	help
+	  Say 'Y' here to enable support for the PCIe controller in the
+	  ARTPEC-8 SoC to work in host mode.
+	  This PCIe controller is based on DesignWare hardware core and
+	  uses DesignWare core functions to implement the driver.
+
+config PCIE_ARTPEC8_EP
+	bool "Axis ARTPEC-8 PCIe controller Endpoint Mode"
+	depends on ARCH_ARTPEC || COMPILE_TEST
+	depends on PCI_ENDPOINT
+	depends on PCI_ENDPOINT_CONFIGFS
+	select PCI_EPF_TEST
+	select PCIE_DW_EP
+	select PCIE_ARTPEC8
+	help
+	  Say 'Y' here to enable support for the PCIe controller in the
+	  ARTPEC-8 SoC to work in endpoint mode.
+	  This PCIe controller is based on DesignWare hardware core and
+	  uses DesignWare core functions to implement the driver.
+
 config PCIE_ROCKCHIP_DW_HOST
 	bool "Rockchip DesignWare PCIe controller"
 	select PCIE_DW
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index 8ba7b67..95f5877 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
 obj-$(CONFIG_PCIE_QCOM_EP) += pcie-qcom-ep.o
 obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
 obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
+obj-$(CONFIG_PCIE_ARTPEC8) += pcie-artpec8.o
 obj-$(CONFIG_PCIE_ROCKCHIP_DW_HOST) += pcie-dw-rockchip.o
 obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o
 obj-$(CONFIG_PCIE_KEEMBAY) += pcie-keembay.o
diff --git a/drivers/pci/controller/dwc/pcie-artpec8.c b/drivers/pci/controller/dwc/pcie-artpec8.c
new file mode 100644
index 0000000..11eddf0
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-artpec8.c
@@ -0,0 +1,788 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PCIe controller driver for Axis ARTPEC-8 SoC
+ *
+ * Copyright (C) 2019 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ *
+ * Author: Jaeho Cho <jaeho79.cho@samsung.com>
+ * This file is based on driver/pci/controller/dwc/pci-exynos.c
+ */
+
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+#include <linux/resource.h>
+#include <linux/types.h>
+#include <linux/phy/phy.h>
+
+#include "pcie-designware.h"
+
+#define to_artpec8_pcie(x)	dev_get_drvdata((x)->dev)
+
+/* Gen3 Control Register */
+#define PCIE_GEN3_RELATED_OFF		0x890
+#define PCIE_GEN3_EQUALIZATION_DISABLE	(0x1 << 16)
+#define PCIE_GEN3_EQ_PHASE_2_3		(0x1 << 9)
+#define PCIE_GEN3_RXEQ_PH01_EN		(0x1 << 12)
+#define PCIE_GEN3_RXEQ_RGRDLESS_RXTS	(0x1 << 13)
+
+#define FAST_LINK_MODE			(7)
+
+/* PCIe ELBI registers */
+#define PCIE_IRQ0_STS			0x000
+#define PCIE_IRQ1_STS			0x004
+#define PCIE_IRQ2_STS			0x008
+#define IRQ2_STS_IRQ_MSI_ST		BIT(20)
+#define PCIE_IRQ5_STS			0x00C
+#define PCIE_IRQ0_EN			0x010
+#define PCIE_IRQ1_EN			0x014
+#define PCIE_IRQ2_EN			0x018
+#define IRQ2_EN_IRQ_MSI			BIT(20)
+#define PCIE_IRQ5_EN			0x01C
+#define PCIE_APP_LTSSM_ENABLE		0x054
+#define APP_LTSSM_ENABLE_EN_BIT		BIT(0)
+#define PCIE_ELBI_CXPL_DEBUG_00_31	0x2C8
+#define PCIE_ELBI_CXPL_DEBUG_32_63	0x2CC
+#define PCIE_ARTPEC8_DEVICE_TYPE	0x080
+#define DEVICE_TYPE_EP			0x0
+#define DEVICE_TYPE_LEG_EP		BIT(0)
+#define DEVICE_TYPE_RC			BIT(2)
+#define LTSSM_STATE_MASK		0x3F
+#define LTSSM_STATE_L0			0x11
+
+/* FSYS glue logic system registers */
+#define FSYS_PCIE_CON			0x424
+#define PCIE_PERSTN			BIT(5)
+#define FSYS_PCIE_DBI_ADDR_CON		0x428
+#define FSYS_PCIE_DBI_ADDR_OVR_CDM	0x00
+#define FSYS_PCIE_DBI_ADDR_OVR_SHADOW	0x12
+#define FSYS_PCIE_DBI_ADDR_OVR_ATU	0x36
+
+/* PMU SYSCON Offsets */
+#define PMU_SYSCON_PCIE_ISOLATION	0x3200
+
+/* BUS P/S SYSCON Offsets */
+#define BUS_SYSCON_BUS_PATH_ENABLE	0x0
+
+#define PCIE_CLEAR_ISOLATION		0
+#define PCIE_SET_ISOLATION		1
+
+#define PCIE_REG_BIT_LOW		0
+#define PCIE_REG_BIT_HIGH		1
+
+struct artpec8_pcie {
+	struct dw_pcie			*pci;
+	const struct artpec8_pcie_pdata	*pdata;
+	void __iomem			*elbi_base;
+	struct regmap			*sysreg;
+	struct regmap			*pmu_syscon;
+	struct regmap			*bus_s_syscon;
+	struct regmap			*bus_p_syscon;
+	enum dw_pcie_device_mode	mode;
+	int				link_id;
+	struct phy			*phy;
+};
+
+struct artpec8_pcie_res_ops {
+	int (*get_mem_resources)(struct platform_device *pdev,
+				 struct artpec8_pcie *artpec8_ctrl);
+	int (*get_clk_resources)(struct platform_device *pdev);
+	int (*init_clk_resources)(void);
+	void (*deinit_clk_resources)(void);
+};
+
+struct artpec8_pcie_pdata {
+	const struct dw_pcie_ops		*dwc_ops;
+	const struct dw_pcie_host_ops		*host_ops;
+	const struct artpec8_pcie_res_ops	*res_ops;
+	enum dw_pcie_device_mode		mode;
+};
+
+static const int artpec8_pcie_dbi_addr_con[] = {
+	FSYS_PCIE_DBI_ADDR_CON
+};
+
+static struct clk_bulk_data artpec8_pcie_clks[] = {
+	{ .id = "pipe" },
+	{ .id = "dbi" },
+	{ .id = "mstr" },
+	{ .id = "slv" },
+};
+
+static u32 artpec8_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
+				 u32 reg, size_t size)
+{
+	struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci);
+	u32 val;
+	bool is_atu = false;
+
+	if (base == pci->atu_base) {
+		is_atu = true;
+		base = pci->dbi_base;
+		regmap_write(artpec8_ctrl->sysreg,
+			     artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id],
+			     FSYS_PCIE_DBI_ADDR_OVR_ATU);
+	}
+
+	dw_pcie_read(base + reg, size, &val);
+
+	if (is_atu)
+		regmap_write(artpec8_ctrl->sysreg,
+			     artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id],
+			     FSYS_PCIE_DBI_ADDR_OVR_CDM);
+
+	return val;
+}
+
+static void artpec8_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base,
+				   u32 reg, size_t size, u32 val)
+{
+	struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci);
+	bool is_atu = false;
+
+	if (base == pci->atu_base) {
+		is_atu = true;
+		base = pci->dbi_base;
+		regmap_write(artpec8_ctrl->sysreg,
+			     artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id],
+			     FSYS_PCIE_DBI_ADDR_OVR_ATU);
+	}
+
+	dw_pcie_write(base + reg, size, val);
+
+	if (is_atu)
+		regmap_write(artpec8_ctrl->sysreg,
+			     artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id],
+			     FSYS_PCIE_DBI_ADDR_OVR_CDM);
+}
+
+static void artpec8_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base,
+				    u32 reg, size_t size, u32 val)
+{
+	struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci);
+
+	regmap_write(artpec8_ctrl->sysreg,
+		     artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id],
+		     FSYS_PCIE_DBI_ADDR_OVR_SHADOW);
+
+	dw_pcie_write(base + reg, size, val);
+
+	regmap_write(artpec8_ctrl->sysreg,
+		     artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id],
+		     FSYS_PCIE_DBI_ADDR_OVR_CDM);
+}
+
+static int artpec8_pcie_get_subsys_resources(struct platform_device *pdev,
+					     struct artpec8_pcie *artpec8_ctrl)
+{
+	struct device *dev = &pdev->dev;
+
+	/* External Local Bus interface(ELBI) Register */
+	artpec8_ctrl->elbi_base =
+		devm_platform_ioremap_resource_byname(pdev, "elbi");
+	if (IS_ERR(artpec8_ctrl->elbi_base)) {
+		dev_err(dev, "failed to map elbi_base\n");
+		return PTR_ERR(artpec8_ctrl->elbi_base);
+	}
+
+	artpec8_ctrl->sysreg =
+		syscon_regmap_lookup_by_phandle(dev->of_node,
+						"samsung,fsys-sysreg");
+	if (IS_ERR(artpec8_ctrl->sysreg)) {
+		dev_err(dev, "fsys sysreg regmap lookup failed.\n");
+		return PTR_ERR(artpec8_ctrl->sysreg);
+	}
+
+	artpec8_ctrl->pmu_syscon =
+		syscon_regmap_lookup_by_phandle(dev->of_node,
+						"samsung,syscon-phandle");
+	if (IS_ERR(artpec8_ctrl->pmu_syscon)) {
+		dev_err(dev, "pmu syscon regmap lookup failed.\n");
+		return PTR_ERR(artpec8_ctrl->pmu_syscon);
+	}
+
+	artpec8_ctrl->bus_s_syscon =
+		syscon_regmap_lookup_by_phandle(dev->of_node,
+						"samsung,syscon-bus-s-fsys");
+	if (IS_ERR(artpec8_ctrl->bus_s_syscon)) {
+		dev_err(dev, "bus_s_syscon regmap lookup failed.\n");
+		return PTR_ERR(artpec8_ctrl->bus_s_syscon);
+	}
+
+	artpec8_ctrl->bus_p_syscon =
+		syscon_regmap_lookup_by_phandle(dev->of_node,
+						"samsung,syscon-bus-p-fsys");
+	if (IS_ERR(artpec8_ctrl->bus_p_syscon)) {
+		dev_err(dev, "bus_p_syscon regmap lookup failed.\n");
+		return PTR_ERR(artpec8_ctrl->bus_p_syscon);
+	}
+
+	return 0;
+}
+
+static int artpec8_pcie_get_rc_mem_resources(struct platform_device *pdev,
+					     struct artpec8_pcie *artpec8_ctrl)
+{
+	struct dw_pcie *pci = artpec8_ctrl->pci;
+
+	/* Data Bus Interface(DBI) Register */
+	pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "dbi");
+	if (IS_ERR(pci->dbi_base))
+		return PTR_ERR(pci->dbi_base);
+
+	return 0;
+}
+
+static int artpec8_pcie_get_ep_mem_resources(struct platform_device *pdev,
+					     struct artpec8_pcie *artpec8_ctrl)
+{
+	struct dw_pcie *pci = artpec8_ctrl->pci;
+	struct device *dev = &pdev->dev;
+	struct dw_pcie_ep *ep = &pci->ep;
+	struct resource *res;
+
+	pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "dbi");
+	if (IS_ERR(pci->dbi_base)) {
+		dev_err(dev, "failed to map ep_dbics\n");
+		return -ENOMEM;
+	}
+
+	pci->dbi_base2 = devm_platform_ioremap_resource_byname(pdev, "dbi2");
+	if (IS_ERR(pci->dbi_base2)) {
+		dev_err(dev, "failed to map ep_dbics2\n");
+		return -ENOMEM;
+	}
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space");
+	if (!res)
+		return -EINVAL;
+	ep->phys_base = res->start;
+	ep->addr_size = resource_size(res);
+
+	return 0;
+}
+
+static int artpec8_pcie_get_clk_resources(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(artpec8_pcie_clks),
+				artpec8_pcie_clks);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int artpec8_pcie_init_clk_resources(void)
+{
+	int ret;
+
+	ret = clk_bulk_prepare_enable(ARRAY_SIZE(artpec8_pcie_clks),
+				      artpec8_pcie_clks);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static void artpec8_pcie_deinit_clk_resources(void)
+{
+	clk_bulk_disable_unprepare(ARRAY_SIZE(artpec8_pcie_clks),
+				   artpec8_pcie_clks);
+}
+
+static const struct artpec8_pcie_res_ops artpec8_pcie_rc_res_ops = {
+	.get_mem_resources	= artpec8_pcie_get_rc_mem_resources,
+	.get_clk_resources	= artpec8_pcie_get_clk_resources,
+	.init_clk_resources	= artpec8_pcie_init_clk_resources,
+	.deinit_clk_resources	= artpec8_pcie_deinit_clk_resources,
+};
+
+static const struct artpec8_pcie_res_ops artpec8_pcie_ep_res_ops = {
+	.get_mem_resources	= artpec8_pcie_get_ep_mem_resources,
+	.get_clk_resources	= artpec8_pcie_get_clk_resources,
+	.init_clk_resources	= artpec8_pcie_init_clk_resources,
+	.deinit_clk_resources	= artpec8_pcie_deinit_clk_resources,
+};
+
+static int artpec8_pcie_config_phy_power_isolation(struct dw_pcie *pci, u8 val)
+{
+	struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci);
+
+	return regmap_write(artpec8_ctrl->pmu_syscon, PMU_SYSCON_PCIE_ISOLATION,
+			    val);
+}
+
+static int artpec8_pcie_config_bus_enable(struct dw_pcie *pci, u8 val)
+{
+	struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci);
+	int ret;
+
+	ret = regmap_write(artpec8_ctrl->bus_p_syscon,
+			   BUS_SYSCON_BUS_PATH_ENABLE, val);
+	if (ret)
+		return ret;
+
+	return regmap_write(artpec8_ctrl->bus_s_syscon,
+			    BUS_SYSCON_BUS_PATH_ENABLE, val);
+}
+
+static int artpec8_pcie_config_isolation(struct dw_pcie *pci, u8 val)
+{
+	int ret;
+	/* reg_val[0] : for phy power isolation */
+	/* reg_val[1] : for bus enable */
+	u8 reg_val[2];
+
+	switch (val) {
+	case PCIE_CLEAR_ISOLATION:
+		reg_val[0] = PCIE_REG_BIT_LOW;
+		reg_val[1] = PCIE_REG_BIT_HIGH;
+		break;
+	case PCIE_SET_ISOLATION:
+		reg_val[0] = PCIE_REG_BIT_HIGH;
+		reg_val[1] = PCIE_REG_BIT_LOW;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = artpec8_pcie_config_phy_power_isolation(pci, reg_val[0]);
+	if (ret)
+		return ret;
+
+	return artpec8_pcie_config_bus_enable(pci, reg_val[1]);
+}
+
+static int artpec8_pcie_config_perstn(struct dw_pcie *pci, u8 val)
+{
+	struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci);
+	unsigned int bits;
+
+	if (val == PCIE_REG_BIT_HIGH)
+		bits = PCIE_PERSTN;
+	else
+		bits = 0;
+
+	return regmap_update_bits(artpec8_ctrl->sysreg, FSYS_PCIE_CON,
+				 PCIE_PERSTN, bits);
+}
+
+static void artpec8_pcie_stop_link(struct dw_pcie *pci)
+{
+	struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci);
+	u32 val;
+
+	val = readl(artpec8_ctrl->elbi_base + PCIE_APP_LTSSM_ENABLE);
+
+	val &= ~APP_LTSSM_ENABLE_EN_BIT;
+	writel(val, artpec8_ctrl->elbi_base + PCIE_APP_LTSSM_ENABLE);
+}
+
+static int artpec8_pcie_start_link(struct dw_pcie *pci)
+{
+	struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci);
+	u32 val;
+
+	dw_pcie_dbi_ro_wr_en(pci);
+
+	/* Equalization disable */
+	val = artpec8_pcie_read_dbi(pci, pci->dbi_base, PCIE_GEN3_RELATED_OFF,
+				    4);
+	artpec8_pcie_write_dbi(pci, pci->dbi_base, PCIE_GEN3_RELATED_OFF, 4,
+			       val | PCIE_GEN3_EQUALIZATION_DISABLE);
+
+	dw_pcie_dbi_ro_wr_dis(pci);
+
+	/* assert LTSSM enable */
+	val = readl(artpec8_ctrl->elbi_base + PCIE_APP_LTSSM_ENABLE);
+
+	val |= APP_LTSSM_ENABLE_EN_BIT;
+	writel(val, artpec8_ctrl->elbi_base + PCIE_APP_LTSSM_ENABLE);
+
+	return 0;
+}
+
+static irqreturn_t artpec8_pcie_msi_irq_handler(int irq, void *arg)
+{
+	struct artpec8_pcie *artpec8_ctrl = arg;
+	struct dw_pcie *pci = artpec8_ctrl->pci;
+	struct pcie_port *pp = &pci->pp;
+	u32 val;
+
+	val = readl(artpec8_ctrl->elbi_base + PCIE_IRQ2_STS);
+
+	if ((val & IRQ2_STS_IRQ_MSI_ST) == IRQ2_STS_IRQ_MSI_ST) {
+		val &= IRQ2_STS_IRQ_MSI_ST;
+		writel(val, artpec8_ctrl->elbi_base + PCIE_IRQ2_STS);
+		dw_handle_msi_irq(pp);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static void artpec8_pcie_msi_init(struct artpec8_pcie *artpec8_ctrl)
+{
+	u32 val;
+
+	/* enable MSI interrupt */
+	val = readl(artpec8_ctrl->elbi_base + PCIE_IRQ2_EN);
+	val |= IRQ2_EN_IRQ_MSI;
+	writel(val, artpec8_ctrl->elbi_base + PCIE_IRQ2_EN);
+}
+
+static void artpec8_pcie_enable_interrupts(struct artpec8_pcie *artpec8_ctrl)
+{
+	if (IS_ENABLED(CONFIG_PCI_MSI))
+		artpec8_pcie_msi_init(artpec8_ctrl);
+}
+
+static int artpec8_pcie_rd_own_conf(struct pci_bus *bus, unsigned int devfn,
+				    int where, int size, u32 *val)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(bus->sysdata);
+
+	if (PCI_SLOT(devfn)) {
+		PCI_SET_ERROR_RESPONSE(val);
+		return PCIBIOS_DEVICE_NOT_FOUND;
+	}
+
+	*val = dw_pcie_read_dbi(pci, where, size);
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static int artpec8_pcie_wr_own_conf(struct pci_bus *bus, unsigned int devfn,
+				    int where, int size, u32 val)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(bus->sysdata);
+
+	if (PCI_SLOT(devfn))
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	dw_pcie_write_dbi(pci, where, size, val);
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static struct pci_ops artpec8_pci_ops = {
+	.read = artpec8_pcie_rd_own_conf,
+	.write = artpec8_pcie_wr_own_conf,
+};
+
+static int artpec8_pcie_link_up(struct dw_pcie *pci)
+{
+	struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci);
+	u32 val;
+
+	val = readl(artpec8_ctrl->elbi_base + PCIE_ELBI_CXPL_DEBUG_00_31);
+
+	return (val & LTSSM_STATE_MASK) == LTSSM_STATE_L0;
+}
+
+static int artpec8_pcie_host_init(struct pcie_port *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci);
+
+	pp->bridge->ops = &artpec8_pci_ops;
+
+	dw_pcie_writel_dbi(pci, PCIE_GEN3_RELATED_OFF, (PCIE_GEN3_EQ_PHASE_2_3 |
+			   PCIE_GEN3_RXEQ_PH01_EN |
+			   PCIE_GEN3_RXEQ_RGRDLESS_RXTS));
+
+	artpec8_pcie_enable_interrupts(artpec8_ctrl);
+
+	return 0;
+}
+
+static const struct dw_pcie_host_ops artpec8_pcie_host_ops = {
+	.host_init = artpec8_pcie_host_init,
+};
+
+static u8 artpec8_pcie_iatu_unroll_enabled(struct dw_pcie *pci)
+{
+	u32 val;
+
+	val = dw_pcie_readl_dbi(pci, PCIE_ATU_VIEWPORT);
+	pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET;
+
+	if (val == 0xffffffff)
+		return 1;
+
+	return 0;
+}
+
+static void artpec8_pcie_ep_init(struct dw_pcie_ep *ep)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+	enum pci_barno bar;
+	/*
+	 * Currently PCIe EP core is not setting iatu_unroll_enabled
+	 * so let's handle it here. We need to find proper place to
+	 * initialize this so that it can be used for other EP
+	 * controllers as well.
+	 */
+	pci->iatu_unroll_enabled = artpec8_pcie_iatu_unroll_enabled(pci);
+
+	for (bar = BAR_0; bar <= BAR_5; bar++)
+		dw_pcie_ep_reset_bar(pci, bar);
+}
+
+static int artpec8_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
+				  enum pci_epc_irq_type type, u16 interrupt_num)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+
+	switch (type) {
+	case PCI_EPC_IRQ_LEGACY:
+		return -EINVAL;
+	case PCI_EPC_IRQ_MSI:
+		return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
+	default:
+		dev_err(pci->dev, "UNKNOWN IRQ type\n");
+	}
+
+	return 0;
+}
+
+static const struct pci_epc_features artpec8_pcie_epc_features = {
+	.linkup_notifier = false,
+	.msi_capable = true,
+	.msix_capable = false,
+};
+
+static const struct pci_epc_features*
+artpec8_pcie_ep_get_features(struct dw_pcie_ep *ep)
+{
+	return &artpec8_pcie_epc_features;
+}
+
+static const struct dw_pcie_ep_ops artpec8_dw_pcie_ep_ops = {
+	.ep_init	= artpec8_pcie_ep_init,
+	.raise_irq	= artpec8_pcie_raise_irq,
+	.get_features	= artpec8_pcie_ep_get_features,
+};
+
+static int __init artpec8_add_pcie_ep(struct artpec8_pcie *artpec8_ctrl,
+				      struct platform_device *pdev)
+{
+	struct dw_pcie *pci = artpec8_ctrl->pci;
+	struct dw_pcie_ep *ep = &pci->ep;
+	int ret;
+
+	ep->ops = &artpec8_dw_pcie_ep_ops;
+
+	dw_pcie_writel_dbi(pci, PCIE_GEN3_RELATED_OFF, (PCIE_GEN3_EQ_PHASE_2_3 |
+			   PCIE_GEN3_RXEQ_PH01_EN |
+			   PCIE_GEN3_RXEQ_RGRDLESS_RXTS));
+
+	ret = dw_pcie_ep_init(ep);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int __init artpec8_add_pcie_port(struct artpec8_pcie *artpec8_ctrl,
+					struct platform_device *pdev)
+{
+	struct dw_pcie *pci = artpec8_ctrl->pci;
+	struct pcie_port *pp = &pci->pp;
+	struct device *dev = &pdev->dev;
+	int irq;
+	int irq_flags;
+	int ret;
+
+	if (IS_ENABLED(CONFIG_PCI_MSI)) {
+		irq = platform_get_irq_byname(pdev, "intr");
+		if (!irq)
+			return -ENODEV;
+
+		irq_flags = IRQF_SHARED | IRQF_NO_THREAD;
+
+		ret = devm_request_irq(dev, irq, artpec8_pcie_msi_irq_handler,
+				       irq_flags, "artpec8-pcie", artpec8_ctrl);
+		if (ret)
+			return ret;
+	}
+
+	/* Prevent core from messing with the IRQ, since it's muxed */
+	pp->msi_irq = -ENODEV;
+
+	return dw_pcie_host_init(pp);
+}
+
+static const struct dw_pcie_ops artpec8_dw_pcie_ops = {
+	.read_dbi	= artpec8_pcie_read_dbi,
+	.write_dbi	= artpec8_pcie_write_dbi,
+	.write_dbi2	= artpec8_pcie_write_dbi2,
+	.start_link	= artpec8_pcie_start_link,
+	.stop_link	= artpec8_pcie_stop_link,
+	.link_up	= artpec8_pcie_link_up,
+};
+
+static int artpec8_pcie_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct artpec8_pcie *artpec8_ctrl;
+	struct dw_pcie *pci;
+	const struct artpec8_pcie_pdata *pdata;
+	enum dw_pcie_device_mode mode;
+	struct pcie_port *pp;
+	struct device_node *np = dev->of_node;
+	int ret;
+
+	artpec8_ctrl = devm_kzalloc(dev, sizeof(*artpec8_ctrl), GFP_KERNEL);
+	if (!artpec8_ctrl)
+		return -ENOMEM;
+
+	pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
+	if (!pci)
+		return -ENOMEM;
+
+	pdata = of_device_get_match_data(dev);
+
+	if (!pdata)
+		return -ENODEV;
+
+	mode = (enum dw_pcie_device_mode)pdata->mode;
+
+	artpec8_ctrl->pci = pci;
+	artpec8_ctrl->pdata = pdata;
+	artpec8_ctrl->mode = mode;
+
+	pci->dev = dev;
+	pci->ops = pdata->dwc_ops;
+	pci->dbi_base2 = NULL;
+	pci->dbi_base = NULL;
+	pp = &pci->pp;
+	pp->ops = artpec8_ctrl->pdata->host_ops;
+
+	if (mode == DW_PCIE_RC_TYPE)
+		artpec8_ctrl->link_id = of_alias_get_id(np, "pcierc");
+	else
+		artpec8_ctrl->link_id = of_alias_get_id(np, "pcieep");
+
+	ret = artpec8_pcie_get_subsys_resources(pdev, artpec8_ctrl);
+	if (ret)
+		return ret;
+
+	if (pdata->res_ops && pdata->res_ops->get_mem_resources) {
+		ret = pdata->res_ops->get_mem_resources(pdev, artpec8_ctrl);
+		if (ret)
+			return ret;
+	}
+
+	if (pdata->res_ops && pdata->res_ops->get_clk_resources) {
+		ret = pdata->res_ops->get_clk_resources(pdev);
+		if (ret)
+			return ret;
+
+		ret = pdata->res_ops->init_clk_resources();
+		if (ret)
+			return ret;
+	}
+
+	platform_set_drvdata(pdev, artpec8_ctrl);
+
+	ret = artpec8_pcie_config_isolation(pci, PCIE_CLEAR_ISOLATION);
+	if (ret)
+		return ret;
+
+	ret = artpec8_pcie_config_perstn(pci, PCIE_REG_BIT_HIGH);
+	if (ret)
+		return ret;
+
+	artpec8_ctrl->phy = devm_of_phy_get(dev, np, NULL);
+	if (IS_ERR(artpec8_ctrl->phy))
+		return PTR_ERR(artpec8_ctrl->phy);
+
+	phy_init(artpec8_ctrl->phy);
+	phy_reset(artpec8_ctrl->phy);
+
+	switch (mode) {
+	case DW_PCIE_RC_TYPE:
+		writel(DEVICE_TYPE_RC, artpec8_ctrl->elbi_base +
+				PCIE_ARTPEC8_DEVICE_TYPE);
+		ret = artpec8_add_pcie_port(artpec8_ctrl, pdev);
+		if (ret < 0)
+			goto fail_probe;
+		break;
+	case DW_PCIE_EP_TYPE:
+		writel(DEVICE_TYPE_EP, artpec8_ctrl->elbi_base +
+				PCIE_ARTPEC8_DEVICE_TYPE);
+
+		ret = artpec8_add_pcie_ep(artpec8_ctrl, pdev);
+		if (ret < 0)
+			goto fail_probe;
+		break;
+	default:
+		ret = -EINVAL;
+		goto fail_probe;
+	}
+
+	return 0;
+
+fail_probe:
+	phy_exit(artpec8_ctrl->phy);
+	if (pdata->res_ops && pdata->res_ops->deinit_clk_resources)
+		pdata->res_ops->deinit_clk_resources();
+
+	return ret;
+}
+
+static int __exit artpec8_pcie_remove(struct platform_device *pdev)
+{
+	struct artpec8_pcie *artpec8_ctrl = platform_get_drvdata(pdev);
+	const struct artpec8_pcie_pdata *pdata = artpec8_ctrl->pdata;
+
+	if (pdata->res_ops && pdata->res_ops->deinit_clk_resources)
+		pdata->res_ops->deinit_clk_resources();
+
+	return 0;
+}
+
+static const struct artpec8_pcie_pdata artpec8_pcie_rc_pdata = {
+	.dwc_ops	= &artpec8_dw_pcie_ops,
+	.host_ops	= &artpec8_pcie_host_ops,
+	.res_ops	= &artpec8_pcie_rc_res_ops,
+	.mode		= DW_PCIE_RC_TYPE,
+};
+
+static const struct artpec8_pcie_pdata artpec8_pcie_ep_pdata = {
+	.dwc_ops	= &artpec8_dw_pcie_ops,
+	.host_ops	= &artpec8_pcie_host_ops,
+	.res_ops	= &artpec8_pcie_ep_res_ops,
+	.mode		= DW_PCIE_EP_TYPE,
+};
+
+static const struct of_device_id artpec8_pcie_of_match[] = {
+	{
+		.compatible = "axis,artpec8-pcie",
+		.data = &artpec8_pcie_rc_pdata,
+	},
+	{
+		.compatible = "axis,artpec8-pcie-ep",
+		.data = &artpec8_pcie_ep_pdata,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, artpec8_pcie_of_match);
+
+static struct platform_driver artpec8_pcie_driver = {
+	.probe	= artpec8_pcie_probe,
+	.remove		= __exit_p(artpec8_pcie_remove),
+	.driver = {
+		.name	= "artpec8-pcie",
+		.of_match_table = artpec8_pcie_of_match,
+	},
+};
+
+module_platform_driver(artpec8_pcie_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jaeho Cho <jaeho79.cho@samsung.com>");
-- 
2.9.5

Re: [PATCH v4 3/5] PCI: axis: Add ARTPEC-8 PCIe controller driver

[Krzysztof Kozlowski]

On 20/07/2022 08:01, Wangseok Lee wrote:
> Add support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC platform of Axis
> Communications. This is based on arm64 and support GEN4 & 2lane. This
> PCIe controller is based on DesignWare Hardware core and uses DesignWare
> core functions to implement the driver. "pcie-artpec6. c" supports artpec6
> and artpec7 H/W. artpec8 can not be expanded because H/W configuration is
> completely different from artpec6/7. PHY and sub controller are different.
> 
> Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com>
> Signed-off-by: Jaeho Cho <jaeho79.cho@samsung.com>
> ---
> v3->v4 :
> -Remove unnecessary enum type
> -Fix indentation
> 

Thanks for the changes. This starts to look good, however I am not going
to ack it. This is also not a strong NAK, as I would respect Bjorn and
other maintainers decision.

I don't like the approach of creating only Artpec-8 specific driver.
Samsung heavily reuses its block in all Exynos devices. Now it re-uses
them for other designs as well. Therefore, even if merging with existing
Exynos PCIe driver is not feasible (we had such discussions), I expect
this to cover all Samsung Foundry PCIe devices. From all current designs
up to future licensed blocks, including some new Samsung Exynos SoC. Or
at least be ready for it.

However it seems you are interested only in achieving one goal here -
satisfy Axis. I believe it is not the "upstream approach". Next month
you come up with same driver for different customer and you keep
insisting "it's different!".

To get my ack I want to see something generic for Samsung Exynos SoC and
other licensed or designed blocks, instead of something made for only
one of your customers.

Best regards,
Krzysztof

Re: [PATCH v4 3/5] PCI: axis: Add ARTPEC-8 PCIe controller driver

[Bjorn Helgaas]

On Thu, Jul 21, 2022 at 11:04:00AM +0200, Krzysztof Kozlowski wrote:
> On 20/07/2022 08:01, Wangseok Lee wrote:
> > Add support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC platform of Axis
> > Communications. This is based on arm64 and support GEN4 & 2lane. This
> > PCIe controller is based on DesignWare Hardware core and uses DesignWare
> > core functions to implement the driver. "pcie-artpec6. c" supports artpec6
> > and artpec7 H/W. artpec8 can not be expanded because H/W configuration is
> > completely different from artpec6/7. PHY and sub controller are different.
> > 
> > Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com>
> > Signed-off-by: Jaeho Cho <jaeho79.cho@samsung.com>
> > ---
> > v3->v4 :
> > -Remove unnecessary enum type
> > -Fix indentation
> > 
> 
> Thanks for the changes. This starts to look good, however I am not going
> to ack it. This is also not a strong NAK, as I would respect Bjorn and
> other maintainers decision.
> 
> I don't like the approach of creating only Artpec-8 specific driver.
> Samsung heavily reuses its block in all Exynos devices. Now it re-uses
> them for other designs as well. Therefore, even if merging with existing
> Exynos PCIe driver is not feasible (we had such discussions), I expect
> this to cover all Samsung Foundry PCIe devices. From all current designs
> up to future licensed blocks, including some new Samsung Exynos SoC. Or
> at least be ready for it.

I would certainly prefer fewer drivers but I don't know enough about
the underlying IP and the places it's integrated to to know what's
practical.  The only way I could figure that out would be by manually
comparing the drivers for similarity.  I assume/expect all driver
authors are doing that.

Bjorn

Re: [PATCH v4 3/5] PCI: axis: Add ARTPEC-8 PCIe controller driver

[Krzysztof Kozlowski]

On 21/07/2022 22:58, Bjorn Helgaas wrote:
> On Thu, Jul 21, 2022 at 11:04:00AM +0200, Krzysztof Kozlowski wrote:
>> On 20/07/2022 08:01, Wangseok Lee wrote:
>>> Add support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC platform of Axis
>>> Communications. This is based on arm64 and support GEN4 & 2lane. This
>>> PCIe controller is based on DesignWare Hardware core and uses DesignWare
>>> core functions to implement the driver. "pcie-artpec6. c" supports artpec6
>>> and artpec7 H/W. artpec8 can not be expanded because H/W configuration is
>>> completely different from artpec6/7. PHY and sub controller are different.
>>>
>>> Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com>
>>> Signed-off-by: Jaeho Cho <jaeho79.cho@samsung.com>
>>> ---
>>> v3->v4 :
>>> -Remove unnecessary enum type
>>> -Fix indentation
>>>
>>
>> Thanks for the changes. This starts to look good, however I am not going
>> to ack it. This is also not a strong NAK, as I would respect Bjorn and
>> other maintainers decision.
>>
>> I don't like the approach of creating only Artpec-8 specific driver.
>> Samsung heavily reuses its block in all Exynos devices. Now it re-uses
>> them for other designs as well. Therefore, even if merging with existing
>> Exynos PCIe driver is not feasible (we had such discussions), I expect
>> this to cover all Samsung Foundry PCIe devices. From all current designs
>> up to future licensed blocks, including some new Samsung Exynos SoC. Or
>> at least be ready for it.
> 
> I would certainly prefer fewer drivers but I don't know enough about
> the underlying IP and the places it's integrated to to know what's
> practical.  The only way I could figure that out would be by manually
> comparing the drivers for similarity.  I assume/expect all driver
> authors are doing that.

Merging with existing Exynos PCIe driver (and phy) might be indeed
tricky, as existing one does not support that much as here. However I
really expect that all current designs from Samsung - Exynos SoC, Artpec
and for other customers - have very similar PCIe thus this should be a
generic, new generation Samsung PCIe driver. If designed that way, also
the naming should be back Samsung specific, no Axis/Artpec.

Best regards,
Krzysztof

Re: [PATCH v4 3/5] PCI: axis: Add ARTPEC-8 PCIe controller driver

[Rob Herring]

On Thu, Jul 21, 2022 at 2:58 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Thu, Jul 21, 2022 at 11:04:00AM +0200, Krzysztof Kozlowski wrote:
> > On 20/07/2022 08:01, Wangseok Lee wrote:
> > > Add support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC platform of Axis
> > > Communications. This is based on arm64 and support GEN4 & 2lane. This
> > > PCIe controller is based on DesignWare Hardware core and uses DesignWare
> > > core functions to implement the driver. "pcie-artpec6. c" supports artpec6
> > > and artpec7 H/W. artpec8 can not be expanded because H/W configuration is
> > > completely different from artpec6/7. PHY and sub controller are different.
> > >
> > > Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com>
> > > Signed-off-by: Jaeho Cho <jaeho79.cho@samsung.com>
> > > ---
> > > v3->v4 :
> > > -Remove unnecessary enum type
> > > -Fix indentation
> > >
> >
> > Thanks for the changes. This starts to look good, however I am not going
> > to ack it. This is also not a strong NAK, as I would respect Bjorn and
> > other maintainers decision.
> >
> > I don't like the approach of creating only Artpec-8 specific driver.
> > Samsung heavily reuses its block in all Exynos devices. Now it re-uses
> > them for other designs as well. Therefore, even if merging with existing
> > Exynos PCIe driver is not feasible (we had such discussions), I expect
> > this to cover all Samsung Foundry PCIe devices. From all current designs
> > up to future licensed blocks, including some new Samsung Exynos SoC. Or
> > at least be ready for it.
>
> I would certainly prefer fewer drivers but I don't know enough about
> the underlying IP and the places it's integrated to to know what's
> practical.  The only way I could figure that out would be by manually
> comparing the drivers for similarity.  I assume/expect all driver
> authors are doing that.

Sadly, I would not assume that. :(

Just looking at the ELBI registers between Exynos and this one, the
registers look completely different with nothing shared between the 2.
Maybe sharing is possible with some new, yet to be upstreamed Samsung
PCIe IP, but not the existing one. Same vendor is not always a good
reason to share a driver.

I think the way to further shrink the vendor DWC drivers is getting
the DWC core and PCI core to do more in terms of clocks, resets, and
phys management, handling of PERST, and link state management. We
should also get rid of vendor drivers doing their own abstraction
layers (ops structs) (looking at you QCom).

Rob

Re: [PATCH v4 3/5] PCI: axis: Add ARTPEC-8 PCIe controller driver

[Bjorn Helgaas]

On Wed, Jul 20, 2022 at 03:01:12PM +0900, Wangseok Lee wrote:
> Add support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC platform of Axis
> Communications. This is based on arm64 and support GEN4 & 2lane. This
> PCIe controller is based on DesignWare Hardware core and uses DesignWare
> core functions to implement the driver. "pcie-artpec6. c" supports artpec6
> and artpec7 H/W. artpec8 can not be expanded because H/W configuration is
> completely different from artpec6/7. PHY and sub controller are different.
> 
> Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com>
> Signed-off-by: Jaeho Cho <jaeho79.cho@samsung.com>

Since you are the one sending the patch, your sign-off should be last:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=v5.18#n363

> +config PCIE_ARTPEC8_HOST
> +	bool "Axis ARTPEC-8 PCIe controller Host Mode"
> +	depends on ARCH_ARTPEC || COMPILE_TEST
> +	depends on PCI_MSI_IRQ_DOMAIN
> +	depends on PCI_ENDPOINT
> +	select PCI_EPF_TEST
> +	select PCIE_DW_HOST
> +	select PCIE_ARTPEC8
> +	help
> +	  Say 'Y' here to enable support for the PCIe controller in the
> +	  ARTPEC-8 SoC to work in host mode.

Add a blank line between paragraphs.  Mentioned previously:
https://lore.kernel.org/r/20220603160314.GA76545@bhelgaas

> +	  This PCIe controller is based on DesignWare hardware core and
> +	  uses DesignWare core functions to implement the driver.

I don't think Kconfig users need to know how the driver is
implemented.  Not really even sure they need to know that it's based
on DesignWare hardware.  Users need to be able to connect the hardware
in front of them with the drivers for it, which means they start with
the name on the box.

> +config PCIE_ARTPEC8_EP
> +	bool "Axis ARTPEC-8 PCIe controller Endpoint Mode"
> +	depends on ARCH_ARTPEC || COMPILE_TEST
> +	depends on PCI_ENDPOINT
> +	depends on PCI_ENDPOINT_CONFIGFS
> +	select PCI_EPF_TEST
> +	select PCIE_DW_EP
> +	select PCIE_ARTPEC8
> +	help
> +	  Say 'Y' here to enable support for the PCIe controller in the
> +	  ARTPEC-8 SoC to work in endpoint mode.

Same.

> +	  This PCIe controller is based on DesignWare hardware core and
> +	  uses DesignWare core functions to implement the driver.

> +#define PCIE_GEN3_EQUALIZATION_DISABLE	(0x1 << 16)

Why the mix of "(0x1 << 16)" and "BIT(...)" below?  Pick one.

> +#define PCIE_GEN3_EQ_PHASE_2_3		(0x1 << 9)
> +#define PCIE_GEN3_RXEQ_PH01_EN		(0x1 << 12)
> +#define PCIE_GEN3_RXEQ_RGRDLESS_RXTS	(0x1 << 13)
> +
> +#define FAST_LINK_MODE			(7)
> +
> +/* PCIe ELBI registers */
> +#define PCIE_IRQ0_STS			0x000
> +#define PCIE_IRQ1_STS			0x004
> +#define PCIE_IRQ2_STS			0x008
> +#define IRQ2_STS_IRQ_MSI_ST		BIT(20)

> +static const int artpec8_pcie_dbi_addr_con[] = {
> +	FSYS_PCIE_DBI_ADDR_CON

Since this is an array, I assume you envision adding more entries
eventually.  That means you should have a comma at the end of this
entry, as you do in artpec8_pcie_clks[] below, so the
FSYS_PCIE_DBI_ADDR_CON line will not need to change when you add
entries in the future.

> +};

> +static u32 artpec8_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
> +				 u32 reg, size_t size)
> +{
> +	struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci);

Typical names for pointers like this are "dra7xx", "rockchip",
"artpec6_pcie", and even "pcie".  "artpec6_pcie" seems unnecessarily
wordy.  "pcie" seems possibly confusing since "pci" is the common
pointer to struct dw_pcie.  I think "artpec8" would be perfect.

> +	u32 val;
> +	bool is_atu = false;
> +
> +	if (base == pci->atu_base) {
> +		is_atu = true;
> +		base = pci->dbi_base;
> +		regmap_write(artpec8_ctrl->sysreg,
> +			     artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id],

This is not safe because nothing guarantees that 
artpec8_ctrl->link_id == 0.  Any other index other than zero would be
out of bounds.

That suggests that making artpec8_pcie_dbi_addr_con[] an array is
over-engineered.  If you need more than one value in the future, you
could make it an array at that time.  For now it just makes the code
hard to read.

> +			     FSYS_PCIE_DBI_ADDR_OVR_ATU);
> +	}

> +static int artpec8_pcie_get_subsys_resources(struct platform_device *pdev,
> +					     struct artpec8_pcie *artpec8_ctrl)
> +{
> +	struct device *dev = &pdev->dev;
> +
> +	/* External Local Bus interface(ELBI) Register */

Add a space before things like "(ELBI)".  Also below for "(DBI)".

> +	artpec8_ctrl->elbi_base =
> +		devm_platform_ioremap_resource_byname(pdev, "elbi");
> +	if (IS_ERR(artpec8_ctrl->elbi_base)) {
> +		dev_err(dev, "failed to map elbi_base\n");
> +		return PTR_ERR(artpec8_ctrl->elbi_base);
> +	}
> +
> +	artpec8_ctrl->sysreg =
> +		syscon_regmap_lookup_by_phandle(dev->of_node,
> +						"samsung,fsys-sysreg");
> +	if (IS_ERR(artpec8_ctrl->sysreg)) {
> +		dev_err(dev, "fsys sysreg regmap lookup failed.\n");

Omit periods on error messages (as you did for the elbi_base message
above).  This also applies to several messages below.

> +		return PTR_ERR(artpec8_ctrl->sysreg);
> +	}
> +
> +	artpec8_ctrl->pmu_syscon =
> +		syscon_regmap_lookup_by_phandle(dev->of_node,
> +						"samsung,syscon-phandle");
> +	if (IS_ERR(artpec8_ctrl->pmu_syscon)) {
> +		dev_err(dev, "pmu syscon regmap lookup failed.\n");
> +		return PTR_ERR(artpec8_ctrl->pmu_syscon);
> +	}

> +static int artpec8_pcie_get_rc_mem_resources(struct platform_device *pdev,
> +					     struct artpec8_pcie *artpec8_ctrl)
> +{
> +	struct dw_pcie *pci = artpec8_ctrl->pci;
> +
> +	/* Data Bus Interface(DBI) Register */

> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space");
> +	if (!res)
> +		return -EINVAL;

Add a blank line here.

> +	ep->phys_base = res->start;
> +	ep->addr_size = resource_size(res);

> +	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(artpec8_pcie_clks),
> +				artpec8_pcie_clks);
> +	if (ret)
> +		return ret;
> +
> +	return 0;

This is the same as:

  return devm_clk_bulk_get(dev, ...);

> +	ret = clk_bulk_prepare_enable(ARRAY_SIZE(artpec8_pcie_clks),
> +				      artpec8_pcie_clks);
> +	if (ret)
> +		return ret;
> +
> +	return 0;

Again, same as:

  return clk_bulk_prepare_enable(...);

> +static u8 artpec8_pcie_iatu_unroll_enabled(struct dw_pcie *pci)
> +{
> +	u32 val;
> +
> +	val = dw_pcie_readl_dbi(pci, PCIE_ATU_VIEWPORT);
> +	pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET;

This line is out of place.  This function is essentially a boolean and
should not have side effects like updating pci->atu_base.

> +
> +	if (val == 0xffffffff)
> +		return 1;
> +
> +	return 0;

This function as a whole is a copy of dw_pcie_iatu_unroll_enabled(),
which is static in pcie-designware.c.  I assume you'll resolve this
eventually (maybe not in this initial patch) so we don't have two
copies.

> +static void artpec8_pcie_ep_init(struct dw_pcie_ep *ep)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +	enum pci_barno bar;

Add a blank line here.

> +	/*
> +	 * Currently PCIe EP core is not setting iatu_unroll_enabled
> +	 * so let's handle it here. We need to find proper place to
> +	 * initialize this so that it can be used for other EP
> +	 * controllers as well.
> +	 */
> +	pci->iatu_unroll_enabled = artpec8_pcie_iatu_unroll_enabled(pci);

> +static int __init artpec8_add_pcie_ep(struct artpec8_pcie *artpec8_ctrl,
> +				      struct platform_device *pdev)

Why is this __init when the caller is not __init?

> +	ret = dw_pcie_ep_init(ep);
> +	if (ret)
> +		return ret;
> +
> +	return 0;

Again:

  return dw_pcie_ep_init(ep);

> +static int __init artpec8_add_pcie_port(struct artpec8_pcie *artpec8_ctrl,
> +					struct platform_device *pdev)

Why is this __init?

> +	pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
> +	if (!pci)
> +		return -ENOMEM;
> +
> +	pdata = of_device_get_match_data(dev);
> +
> +	if (!pdata)
> +		return -ENODEV;
> +
> +	mode = (enum dw_pcie_device_mode)pdata->mode;
> +
> +	artpec8_ctrl->pci = pci;
> +	artpec8_ctrl->pdata = pdata;
> +	artpec8_ctrl->mode = mode;
> +
> +	pci->dev = dev;
> +	pci->ops = pdata->dwc_ops;
> +	pci->dbi_base2 = NULL;
> +	pci->dbi_base = NULL;

These are guaranteed NULL already by the kzalloc().

> +static int __exit artpec8_pcie_remove(struct platform_device *pdev)

Why do we need __exit and __exit_p below?  exynos, keystone, and kirin
use them, but I'm not sure they need them either.  I don't know all
the details, but most of our drivers don't use them, so I assume this
one doesn't need them either.

> +static struct platform_driver artpec8_pcie_driver = {
> +	.probe	= artpec8_pcie_probe,
> +	.remove		= __exit_p(artpec8_pcie_remove),

__exit_p(), see above.

> +	.driver = {
> +		.name	= "artpec8-pcie",
> +		.of_match_table = artpec8_pcie_of_match,
> +	},
> +};

Re: [PATCH v4 3/5] PCI: axis: Add ARTPEC-8 PCIe controller driver

[Rob Herring]

. On Wed, Jul 20, 2022 at 12:01 AM Wangseok Lee
<wangseok.lee@samsung.com> wrote:
>
> Add support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC platform of Axis
> Communications. This is based on arm64 and support GEN4 & 2lane. This
> PCIe controller is based on DesignWare Hardware core and uses DesignWare
> core functions to implement the driver. "pcie-artpec6. c" supports artpec6
> and artpec7 H/W. artpec8 can not be expanded because H/W configuration is
> completely different from artpec6/7. PHY and sub controller are different.
>
> Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com>
> Signed-off-by: Jaeho Cho <jaeho79.cho@samsung.com>
> ---
> v3->v4 :
> -Remove unnecessary enum type
> -Fix indentation
>
> v2->v3 :
> -Add 'COMPILE_TEST' and improvement help on kconfig
> -Reorder obj on makefile
> -Use clk_bulk_api
> -Remove unnecessary comment
> -Redefine the ELBI register to distinguish between offset and bit
>  definition
> -Improvement order local variable of function
> -Remove unnecessary local return variable
>
> v1->v2 :
> Improvement review comment of Krzysztof on driver code.
> -Debug messages for probe or other functions.
> -Inconsistent coding style (different indentation in structure members)
> -Inconsistent code (artpec8_pcie_get_subsystem_resources() gets device
>  from pdev and from pci so you have two same pointers; or  artpec8_pcie_
>  get_ep_mem_resources() stores dev as local variable but uses instead
>  pdev->dev)
> -Not using devm_platform_ioremap_resource()
> -Printing messages in interrupt handlers
> -Several local/static structures or array are not const
> ---
>  drivers/pci/controller/dwc/Kconfig        |  31 ++
>  drivers/pci/controller/dwc/Makefile       |   1 +
>  drivers/pci/controller/dwc/pcie-artpec8.c | 788 ++++++++++++++++++++++++++++++
>  3 files changed, 820 insertions(+)
>  create mode 100644 drivers/pci/controller/dwc/pcie-artpec8.c
>
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 62ce3ab..2b16637 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -222,6 +222,37 @@ config PCIE_ARTPEC6_EP
>           Enables support for the PCIe controller in the ARTPEC-6 SoC to work in
>           endpoint mode. This uses the DesignWare core.
>
> +config PCIE_ARTPEC8
> +       bool "Axis ARTPEC-8 PCIe controller"
> +
> +config PCIE_ARTPEC8_HOST
> +       bool "Axis ARTPEC-8 PCIe controller Host Mode"
> +       depends on ARCH_ARTPEC || COMPILE_TEST
> +       depends on PCI_MSI_IRQ_DOMAIN
> +       depends on PCI_ENDPOINT
> +       select PCI_EPF_TEST
> +       select PCIE_DW_HOST
> +       select PCIE_ARTPEC8
> +       help
> +         Say 'Y' here to enable support for the PCIe controller in the
> +         ARTPEC-8 SoC to work in host mode.
> +         This PCIe controller is based on DesignWare hardware core and
> +         uses DesignWare core functions to implement the driver.
> +
> +config PCIE_ARTPEC8_EP
> +       bool "Axis ARTPEC-8 PCIe controller Endpoint Mode"
> +       depends on ARCH_ARTPEC || COMPILE_TEST
> +       depends on PCI_ENDPOINT
> +       depends on PCI_ENDPOINT_CONFIGFS
> +       select PCI_EPF_TEST
> +       select PCIE_DW_EP
> +       select PCIE_ARTPEC8
> +       help
> +         Say 'Y' here to enable support for the PCIe controller in the
> +         ARTPEC-8 SoC to work in endpoint mode.
> +         This PCIe controller is based on DesignWare hardware core and
> +         uses DesignWare core functions to implement the driver.
> +
>  config PCIE_ROCKCHIP_DW_HOST
>         bool "Rockchip DesignWare PCIe controller"
>         select PCIE_DW
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index 8ba7b67..95f5877 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
>  obj-$(CONFIG_PCIE_QCOM_EP) += pcie-qcom-ep.o
>  obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>  obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> +obj-$(CONFIG_PCIE_ARTPEC8) += pcie-artpec8.o
>  obj-$(CONFIG_PCIE_ROCKCHIP_DW_HOST) += pcie-dw-rockchip.o
>  obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o
>  obj-$(CONFIG_PCIE_KEEMBAY) += pcie-keembay.o
> diff --git a/drivers/pci/controller/dwc/pcie-artpec8.c b/drivers/pci/controller/dwc/pcie-artpec8.c
> new file mode 100644
> index 0000000..11eddf0
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-artpec8.c
> @@ -0,0 +1,788 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * PCIe controller driver for Axis ARTPEC-8 SoC
> + *
> + * Copyright (C) 2019 Samsung Electronics Co., Ltd.
> + *             http://www.samsung.com
> + *
> + * Author: Jaeho Cho <jaeho79.cho@samsung.com>
> + * This file is based on driver/pci/controller/dwc/pci-exynos.c
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of_device.h>
> +#include <linux/regmap.h>
> +#include <linux/resource.h>
> +#include <linux/types.h>
> +#include <linux/phy/phy.h>
> +
> +#include "pcie-designware.h"
> +
> +#define to_artpec8_pcie(x)     dev_get_drvdata((x)->dev)
> +
> +/* Gen3 Control Register */
> +#define PCIE_GEN3_RELATED_OFF          0x890

This is a DWC DBI register, so it belongs in the DWC code. IIRC, it already is.

> +#define PCIE_GEN3_EQUALIZATION_DISABLE (0x1 << 16)
> +#define PCIE_GEN3_EQ_PHASE_2_3         (0x1 << 9)
> +#define PCIE_GEN3_RXEQ_PH01_EN         (0x1 << 12)
> +#define PCIE_GEN3_RXEQ_RGRDLESS_RXTS   (0x1 << 13)
> +
> +#define FAST_LINK_MODE                 (7)
> +
> +/* PCIe ELBI registers */
> +#define PCIE_IRQ0_STS                  0x000
> +#define PCIE_IRQ1_STS                  0x004
> +#define PCIE_IRQ2_STS                  0x008
> +#define IRQ2_STS_IRQ_MSI_ST            BIT(20)
> +#define PCIE_IRQ5_STS                  0x00C
> +#define PCIE_IRQ0_EN                   0x010
> +#define PCIE_IRQ1_EN                   0x014
> +#define PCIE_IRQ2_EN                   0x018
> +#define IRQ2_EN_IRQ_MSI                        BIT(20)
> +#define PCIE_IRQ5_EN                   0x01C
> +#define PCIE_APP_LTSSM_ENABLE          0x054
> +#define APP_LTSSM_ENABLE_EN_BIT                BIT(0)
> +#define PCIE_ELBI_CXPL_DEBUG_00_31     0x2C8
> +#define PCIE_ELBI_CXPL_DEBUG_32_63     0x2CC
> +#define PCIE_ARTPEC8_DEVICE_TYPE       0x080
> +#define DEVICE_TYPE_EP                 0x0
> +#define DEVICE_TYPE_LEG_EP             BIT(0)
> +#define DEVICE_TYPE_RC                 BIT(2)
> +#define LTSSM_STATE_MASK               0x3F
> +#define LTSSM_STATE_L0                 0x11
> +
> +/* FSYS glue logic system registers */
> +#define FSYS_PCIE_CON                  0x424
> +#define PCIE_PERSTN                    BIT(5)
> +#define FSYS_PCIE_DBI_ADDR_CON         0x428
> +#define FSYS_PCIE_DBI_ADDR_OVR_CDM     0x00
> +#define FSYS_PCIE_DBI_ADDR_OVR_SHADOW  0x12
> +#define FSYS_PCIE_DBI_ADDR_OVR_ATU     0x36
> +
> +/* PMU SYSCON Offsets */
> +#define PMU_SYSCON_PCIE_ISOLATION      0x3200
> +
> +/* BUS P/S SYSCON Offsets */
> +#define BUS_SYSCON_BUS_PATH_ENABLE     0x0
> +
> +#define PCIE_CLEAR_ISOLATION           0
> +#define PCIE_SET_ISOLATION             1
> +

> +#define PCIE_REG_BIT_LOW               0
> +#define PCIE_REG_BIT_HIGH              1

Kind of pointless defines.

> +
> +struct artpec8_pcie {
> +       struct dw_pcie                  *pci;

Make this a struct so you have 1 memory alloc.

> +       const struct artpec8_pcie_pdata *pdata;
> +       void __iomem                    *elbi_base;
> +       struct regmap                   *sysreg;
> +       struct regmap                   *pmu_syscon;
> +       struct regmap                   *bus_s_syscon;
> +       struct regmap                   *bus_p_syscon;
> +       enum dw_pcie_device_mode        mode;
> +       int                             link_id;
> +       struct phy                      *phy;
> +};
> +
> +struct artpec8_pcie_res_ops {
> +       int (*get_mem_resources)(struct platform_device *pdev,
> +                                struct artpec8_pcie *artpec8_ctrl);
> +       int (*get_clk_resources)(struct platform_device *pdev);
> +       int (*init_clk_resources)(void);
> +       void (*deinit_clk_resources)(void);
> +};
> +
> +struct artpec8_pcie_pdata {
> +       const struct dw_pcie_ops                *dwc_ops;
> +       const struct dw_pcie_host_ops           *host_ops;
> +       const struct artpec8_pcie_res_ops       *res_ops;
> +       enum dw_pcie_device_mode                mode;
> +};
> +
> +static const int artpec8_pcie_dbi_addr_con[] = {
> +       FSYS_PCIE_DBI_ADDR_CON
> +};
> +
> +static struct clk_bulk_data artpec8_pcie_clks[] = {
> +       { .id = "pipe" },
> +       { .id = "dbi" },
> +       { .id = "mstr" },
> +       { .id = "slv" },
> +};
> +
> +static u32 artpec8_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
> +                                u32 reg, size_t size)
> +{
> +       struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci);
> +       u32 val;
> +       bool is_atu = false;
> +
> +       if (base == pci->atu_base) {
> +               is_atu = true;
> +               base = pci->dbi_base;
> +               regmap_write(artpec8_ctrl->sysreg,
> +                            artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id],
> +                            FSYS_PCIE_DBI_ADDR_OVR_ATU);
> +       }
> +
> +       dw_pcie_read(base + reg, size, &val);
> +
> +       if (is_atu)
> +               regmap_write(artpec8_ctrl->sysreg,
> +                            artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id],
> +                            FSYS_PCIE_DBI_ADDR_OVR_CDM);

Well, this is "interesting"... I certainly hope this is not how new
Samsung platforms do it.

> +
> +       return val;
> +}
> +
> +static void artpec8_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base,
> +                                  u32 reg, size_t size, u32 val)
> +{
> +       struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci);
> +       bool is_atu = false;
> +
> +       if (base == pci->atu_base) {
> +               is_atu = true;
> +               base = pci->dbi_base;
> +               regmap_write(artpec8_ctrl->sysreg,
> +                            artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id],
> +                            FSYS_PCIE_DBI_ADDR_OVR_ATU);
> +       }
> +
> +       dw_pcie_write(base + reg, size, val);
> +
> +       if (is_atu)
> +               regmap_write(artpec8_ctrl->sysreg,
> +                            artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id],
> +                            FSYS_PCIE_DBI_ADDR_OVR_CDM);
> +}
> +
> +static void artpec8_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base,
> +                                   u32 reg, size_t size, u32 val)
> +{
> +       struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci);
> +
> +       regmap_write(artpec8_ctrl->sysreg,
> +                    artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id],
> +                    FSYS_PCIE_DBI_ADDR_OVR_SHADOW);
> +
> +       dw_pcie_write(base + reg, size, val);
> +
> +       regmap_write(artpec8_ctrl->sysreg,
> +                    artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id],
> +                    FSYS_PCIE_DBI_ADDR_OVR_CDM);
> +}
> +
> +static int artpec8_pcie_get_subsys_resources(struct platform_device *pdev,
> +                                            struct artpec8_pcie *artpec8_ctrl)
> +{
> +       struct device *dev = &pdev->dev;
> +
> +       /* External Local Bus interface(ELBI) Register */
> +       artpec8_ctrl->elbi_base =
> +               devm_platform_ioremap_resource_byname(pdev, "elbi");
> +       if (IS_ERR(artpec8_ctrl->elbi_base)) {
> +               dev_err(dev, "failed to map elbi_base\n");
> +               return PTR_ERR(artpec8_ctrl->elbi_base);
> +       }
> +
> +       artpec8_ctrl->sysreg =
> +               syscon_regmap_lookup_by_phandle(dev->of_node,
> +                                               "samsung,fsys-sysreg");
> +       if (IS_ERR(artpec8_ctrl->sysreg)) {
> +               dev_err(dev, "fsys sysreg regmap lookup failed.\n");
> +               return PTR_ERR(artpec8_ctrl->sysreg);
> +       }
> +
> +       artpec8_ctrl->pmu_syscon =
> +               syscon_regmap_lookup_by_phandle(dev->of_node,
> +                                               "samsung,syscon-phandle");
> +       if (IS_ERR(artpec8_ctrl->pmu_syscon)) {
> +               dev_err(dev, "pmu syscon regmap lookup failed.\n");
> +               return PTR_ERR(artpec8_ctrl->pmu_syscon);
> +       }
> +
> +       artpec8_ctrl->bus_s_syscon =
> +               syscon_regmap_lookup_by_phandle(dev->of_node,
> +                                               "samsung,syscon-bus-s-fsys");
> +       if (IS_ERR(artpec8_ctrl->bus_s_syscon)) {
> +               dev_err(dev, "bus_s_syscon regmap lookup failed.\n");
> +               return PTR_ERR(artpec8_ctrl->bus_s_syscon);
> +       }
> +
> +       artpec8_ctrl->bus_p_syscon =
> +               syscon_regmap_lookup_by_phandle(dev->of_node,
> +                                               "samsung,syscon-bus-p-fsys");
> +       if (IS_ERR(artpec8_ctrl->bus_p_syscon)) {
> +               dev_err(dev, "bus_p_syscon regmap lookup failed.\n");
> +               return PTR_ERR(artpec8_ctrl->bus_p_syscon);
> +       }
> +
> +       return 0;
> +}
> +
> +static int artpec8_pcie_get_rc_mem_resources(struct platform_device *pdev,
> +                                            struct artpec8_pcie *artpec8_ctrl)
> +{
> +       struct dw_pcie *pci = artpec8_ctrl->pci;
> +
> +       /* Data Bus Interface(DBI) Register */
> +       pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "dbi");
> +       if (IS_ERR(pci->dbi_base))
> +               return PTR_ERR(pci->dbi_base);
> +
> +       return 0;
> +}
> +
> +static int artpec8_pcie_get_ep_mem_resources(struct platform_device *pdev,
> +                                            struct artpec8_pcie *artpec8_ctrl)
> +{
> +       struct dw_pcie *pci = artpec8_ctrl->pci;
> +       struct device *dev = &pdev->dev;
> +       struct dw_pcie_ep *ep = &pci->ep;
> +       struct resource *res;
> +
> +       pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "dbi");
> +       if (IS_ERR(pci->dbi_base)) {
> +               dev_err(dev, "failed to map ep_dbics\n");
> +               return -ENOMEM;
> +       }
> +
> +       pci->dbi_base2 = devm_platform_ioremap_resource_byname(pdev, "dbi2");
> +       if (IS_ERR(pci->dbi_base2)) {
> +               dev_err(dev, "failed to map ep_dbics2\n");
> +               return -ENOMEM;
> +       }
> +
> +       res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space");

The DWC core will get all these
> +       if (!res)
> +               return -EINVAL;
> +       ep->phys_base = res->start;
> +       ep->addr_size = resource_size(res);
> +
> +       return 0;
> +}
> +
> +static int artpec8_pcie_get_clk_resources(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       int ret;
> +
> +       ret = devm_clk_bulk_get(dev, ARRAY_SIZE(artpec8_pcie_clks),
> +                               artpec8_pcie_clks);
> +       if (ret)
> +               return ret;
> +
> +       return 0;
> +}
> +
> +static int artpec8_pcie_init_clk_resources(void)
> +{
> +       int ret;
> +
> +       ret = clk_bulk_prepare_enable(ARRAY_SIZE(artpec8_pcie_clks),
> +                                     artpec8_pcie_clks);
> +       if (ret)
> +               return ret;
> +
> +       return 0;
> +}
> +
> +static void artpec8_pcie_deinit_clk_resources(void)
> +{
> +       clk_bulk_disable_unprepare(ARRAY_SIZE(artpec8_pcie_clks),
> +                                  artpec8_pcie_clks);
> +}
> +
> +static const struct artpec8_pcie_res_ops artpec8_pcie_rc_res_ops = {
> +       .get_mem_resources      = artpec8_pcie_get_rc_mem_resources,
> +       .get_clk_resources      = artpec8_pcie_get_clk_resources,
> +       .init_clk_resources     = artpec8_pcie_init_clk_resources,
> +       .deinit_clk_resources   = artpec8_pcie_deinit_clk_resources,
> +};
> +
> +static const struct artpec8_pcie_res_ops artpec8_pcie_ep_res_ops = {
> +       .get_mem_resources      = artpec8_pcie_get_ep_mem_resources,
> +       .get_clk_resources      = artpec8_pcie_get_clk_resources,
> +       .init_clk_resources     = artpec8_pcie_init_clk_resources,
> +       .deinit_clk_resources   = artpec8_pcie_deinit_clk_resources,

Get rid of artpec8_pcie_res_ops. There's only one difference in functions.

> +};
> +
> +static int artpec8_pcie_config_phy_power_isolation(struct dw_pcie *pci, u8 val)

Doesn't this belong in the PHY driver? Sounds like it should be a
power-domain rather than directly touching some registers.

> +{
> +       struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci);
> +
> +       return regmap_write(artpec8_ctrl->pmu_syscon, PMU_SYSCON_PCIE_ISOLATION,
> +                           val);
> +}
> +
> +static int artpec8_pcie_config_bus_enable(struct dw_pcie *pci, u8 val)
> +{
> +       struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci);
> +       int ret;
> +
> +       ret = regmap_write(artpec8_ctrl->bus_p_syscon,
> +                          BUS_SYSCON_BUS_PATH_ENABLE, val);
> +       if (ret)
> +               return ret;
> +
> +       return regmap_write(artpec8_ctrl->bus_s_syscon,
> +                           BUS_SYSCON_BUS_PATH_ENABLE, val);
> +}
> +
> +static int artpec8_pcie_config_isolation(struct dw_pcie *pci, u8 val)

Again, sounds like a power domain.

> +{
> +       int ret;
> +       /* reg_val[0] : for phy power isolation */
> +       /* reg_val[1] : for bus enable */
> +       u8 reg_val[2];
> +
> +       switch (val) {
> +       case PCIE_CLEAR_ISOLATION:
> +               reg_val[0] = PCIE_REG_BIT_LOW;
> +               reg_val[1] = PCIE_REG_BIT_HIGH;
> +               break;
> +       case PCIE_SET_ISOLATION:
> +               reg_val[0] = PCIE_REG_BIT_HIGH;
> +               reg_val[1] = PCIE_REG_BIT_LOW;
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       ret = artpec8_pcie_config_phy_power_isolation(pci, reg_val[0]);
> +       if (ret)
> +               return ret;
> +
> +       return artpec8_pcie_config_bus_enable(pci, reg_val[1]);
> +}
> +
> +static int artpec8_pcie_config_perstn(struct dw_pcie *pci, u8 val)
> +{
> +       struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci);
> +       unsigned int bits;
> +
> +       if (val == PCIE_REG_BIT_HIGH)
> +               bits = PCIE_PERSTN;
> +       else
> +               bits = 0;
> +
> +       return regmap_update_bits(artpec8_ctrl->sysreg, FSYS_PCIE_CON,
> +                                PCIE_PERSTN, bits);
> +}
> +
> +static void artpec8_pcie_stop_link(struct dw_pcie *pci)
> +{
> +       struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci);
> +       u32 val;
> +
> +       val = readl(artpec8_ctrl->elbi_base + PCIE_APP_LTSSM_ENABLE);
> +
> +       val &= ~APP_LTSSM_ENABLE_EN_BIT;
> +       writel(val, artpec8_ctrl->elbi_base + PCIE_APP_LTSSM_ENABLE);
> +}
> +
> +static int artpec8_pcie_start_link(struct dw_pcie *pci)
> +{
> +       struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci);
> +       u32 val;
> +
> +       dw_pcie_dbi_ro_wr_en(pci);
> +
> +       /* Equalization disable */
> +       val = artpec8_pcie_read_dbi(pci, pci->dbi_base, PCIE_GEN3_RELATED_OFF,
> +                                   4);
> +       artpec8_pcie_write_dbi(pci, pci->dbi_base, PCIE_GEN3_RELATED_OFF, 4,
> +                              val | PCIE_GEN3_EQUALIZATION_DISABLE);

Can this be in host_init instead? You shouldn't need
dw_pcie_dbi_ro_wr_en() in that case.

> +
> +       dw_pcie_dbi_ro_wr_dis(pci);
> +
> +       /* assert LTSSM enable */
> +       val = readl(artpec8_ctrl->elbi_base + PCIE_APP_LTSSM_ENABLE);
> +
> +       val |= APP_LTSSM_ENABLE_EN_BIT;
> +       writel(val, artpec8_ctrl->elbi_base + PCIE_APP_LTSSM_ENABLE);
> +
> +       return 0;
> +}
> +
> +static irqreturn_t artpec8_pcie_msi_irq_handler(int irq, void *arg)
> +{
> +       struct artpec8_pcie *artpec8_ctrl = arg;
> +       struct dw_pcie *pci = artpec8_ctrl->pci;
> +       struct pcie_port *pp = &pci->pp;
> +       u32 val;
> +
> +       val = readl(artpec8_ctrl->elbi_base + PCIE_IRQ2_STS);
> +
> +       if ((val & IRQ2_STS_IRQ_MSI_ST) == IRQ2_STS_IRQ_MSI_ST) {
> +               val &= IRQ2_STS_IRQ_MSI_ST;
> +               writel(val, artpec8_ctrl->elbi_base + PCIE_IRQ2_STS);
> +               dw_handle_msi_irq(pp);
> +       }
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static void artpec8_pcie_msi_init(struct artpec8_pcie *artpec8_ctrl)
> +{
> +       u32 val;
> +
> +       /* enable MSI interrupt */
> +       val = readl(artpec8_ctrl->elbi_base + PCIE_IRQ2_EN);
> +       val |= IRQ2_EN_IRQ_MSI;
> +       writel(val, artpec8_ctrl->elbi_base + PCIE_IRQ2_EN);
> +}
> +
> +static void artpec8_pcie_enable_interrupts(struct artpec8_pcie *artpec8_ctrl)
> +{
> +       if (IS_ENABLED(CONFIG_PCI_MSI))
> +               artpec8_pcie_msi_init(artpec8_ctrl);
> +}
> +
> +static int artpec8_pcie_rd_own_conf(struct pci_bus *bus, unsigned int devfn,
> +                                   int where, int size, u32 *val)
> +{
> +       struct dw_pcie *pci = to_dw_pcie_from_pp(bus->sysdata);
> +
> +       if (PCI_SLOT(devfn)) {
> +               PCI_SET_ERROR_RESPONSE(val);
> +               return PCIBIOS_DEVICE_NOT_FOUND;
> +       }

Why do you need custom read/write functions? The DWC core handles this check.

> +
> +       *val = dw_pcie_read_dbi(pci, where, size);
> +       return PCIBIOS_SUCCESSFUL;
> +}
> +
> +static int artpec8_pcie_wr_own_conf(struct pci_bus *bus, unsigned int devfn,
> +                                   int where, int size, u32 val)
> +{
> +       struct dw_pcie *pci = to_dw_pcie_from_pp(bus->sysdata);
> +
> +       if (PCI_SLOT(devfn))
> +               return PCIBIOS_DEVICE_NOT_FOUND;
> +
> +       dw_pcie_write_dbi(pci, where, size, val);
> +       return PCIBIOS_SUCCESSFUL;
> +}
> +
> +static struct pci_ops artpec8_pci_ops = {
> +       .read = artpec8_pcie_rd_own_conf,
> +       .write = artpec8_pcie_wr_own_conf,
> +};
> +
> +static int artpec8_pcie_link_up(struct dw_pcie *pci)
> +{
> +       struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci);
> +       u32 val;
> +
> +       val = readl(artpec8_ctrl->elbi_base + PCIE_ELBI_CXPL_DEBUG_00_31);
> +
> +       return (val & LTSSM_STATE_MASK) == LTSSM_STATE_L0;
> +}
> +
> +static int artpec8_pcie_host_init(struct pcie_port *pp)
> +{
> +       struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +       struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci);
> +
> +       pp->bridge->ops = &artpec8_pci_ops;
> +
> +       dw_pcie_writel_dbi(pci, PCIE_GEN3_RELATED_OFF, (PCIE_GEN3_EQ_PHASE_2_3 |
> +                          PCIE_GEN3_RXEQ_PH01_EN |
> +                          PCIE_GEN3_RXEQ_RGRDLESS_RXTS));
> +
> +       artpec8_pcie_enable_interrupts(artpec8_ctrl);
> +
> +       return 0;
> +}
> +
> +static const struct dw_pcie_host_ops artpec8_pcie_host_ops = {
> +       .host_init = artpec8_pcie_host_init,
> +};
> +
> +static u8 artpec8_pcie_iatu_unroll_enabled(struct dw_pcie *pci)
> +{
> +       u32 val;
> +
> +       val = dw_pcie_readl_dbi(pci, PCIE_ATU_VIEWPORT);
> +       pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET;
> +
> +       if (val == 0xffffffff)
> +               return 1;
> +
> +       return 0;
> +}
> +
> +static void artpec8_pcie_ep_init(struct dw_pcie_ep *ep)
> +{
> +       struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +       enum pci_barno bar;
> +       /*
> +        * Currently PCIe EP core is not setting iatu_unroll_enabled
> +        * so let's handle it here. We need to find proper place to
> +        * initialize this so that it can be used for other EP
> +        * controllers as well.

I think there's patches to address this. In any case, don't
work-around issues with other code here. Fix the DWC core to do what
you need.

> +        */
> +       pci->iatu_unroll_enabled = artpec8_pcie_iatu_unroll_enabled(pci);

Why do you need to detect this? You don't know which type of iatu
access your h/w supports?

> +
> +       for (bar = BAR_0; bar <= BAR_5; bar++)
> +               dw_pcie_ep_reset_bar(pci, bar);

Also seen a patch for this.

> +}
> +
> +static int artpec8_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> +                                 enum pci_epc_irq_type type, u16 interrupt_num)
> +{
> +       struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +
> +       switch (type) {
> +       case PCI_EPC_IRQ_LEGACY:
> +               return -EINVAL;
> +       case PCI_EPC_IRQ_MSI:
> +               return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
> +       default:
> +               dev_err(pci->dev, "UNKNOWN IRQ type\n");
> +       }
> +
> +       return 0;
> +}
> +
> +static const struct pci_epc_features artpec8_pcie_epc_features = {
> +       .linkup_notifier = false,
> +       .msi_capable = true,
> +       .msix_capable = false,
> +};
> +
> +static const struct pci_epc_features*
> +artpec8_pcie_ep_get_features(struct dw_pcie_ep *ep)
> +{
> +       return &artpec8_pcie_epc_features;
> +}
> +
> +static const struct dw_pcie_ep_ops artpec8_dw_pcie_ep_ops = {
> +       .ep_init        = artpec8_pcie_ep_init,
> +       .raise_irq      = artpec8_pcie_raise_irq,
> +       .get_features   = artpec8_pcie_ep_get_features,
> +};
> +
> +static int __init artpec8_add_pcie_ep(struct artpec8_pcie *artpec8_ctrl,
> +                                     struct platform_device *pdev)
> +{
> +       struct dw_pcie *pci = artpec8_ctrl->pci;
> +       struct dw_pcie_ep *ep = &pci->ep;
> +       int ret;
> +
> +       ep->ops = &artpec8_dw_pcie_ep_ops;
> +
> +       dw_pcie_writel_dbi(pci, PCIE_GEN3_RELATED_OFF, (PCIE_GEN3_EQ_PHASE_2_3 |
> +                          PCIE_GEN3_RXEQ_PH01_EN |
> +                          PCIE_GEN3_RXEQ_RGRDLESS_RXTS));
> +
> +       ret = dw_pcie_ep_init(ep);
> +       if (ret)
> +               return ret;
> +
> +       return 0;
> +}
> +
> +static int __init artpec8_add_pcie_port(struct artpec8_pcie *artpec8_ctrl,
> +                                       struct platform_device *pdev)
> +{
> +       struct dw_pcie *pci = artpec8_ctrl->pci;
> +       struct pcie_port *pp = &pci->pp;
> +       struct device *dev = &pdev->dev;
> +       int irq;
> +       int irq_flags;
> +       int ret;
> +
> +       if (IS_ENABLED(CONFIG_PCI_MSI)) {
> +               irq = platform_get_irq_byname(pdev, "intr");
> +               if (!irq)
> +                       return -ENODEV;
> +
> +               irq_flags = IRQF_SHARED | IRQF_NO_THREAD;
> +
> +               ret = devm_request_irq(dev, irq, artpec8_pcie_msi_irq_handler,
> +                                      irq_flags, "artpec8-pcie", artpec8_ctrl);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       /* Prevent core from messing with the IRQ, since it's muxed */
> +       pp->msi_irq = -ENODEV;
> +
> +       return dw_pcie_host_init(pp);
> +}
> +
> +static const struct dw_pcie_ops artpec8_dw_pcie_ops = {
> +       .read_dbi       = artpec8_pcie_read_dbi,
> +       .write_dbi      = artpec8_pcie_write_dbi,
> +       .write_dbi2     = artpec8_pcie_write_dbi2,
> +       .start_link     = artpec8_pcie_start_link,
> +       .stop_link      = artpec8_pcie_stop_link,
> +       .link_up        = artpec8_pcie_link_up,
> +};
> +
> +static int artpec8_pcie_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct artpec8_pcie *artpec8_ctrl;
> +       struct dw_pcie *pci;
> +       const struct artpec8_pcie_pdata *pdata;
> +       enum dw_pcie_device_mode mode;
> +       struct pcie_port *pp;
> +       struct device_node *np = dev->of_node;
> +       int ret;
> +
> +       artpec8_ctrl = devm_kzalloc(dev, sizeof(*artpec8_ctrl), GFP_KERNEL);
> +       if (!artpec8_ctrl)
> +               return -ENOMEM;
> +
> +       pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
> +       if (!pci)
> +               return -ENOMEM;
> +
> +       pdata = of_device_get_match_data(dev);
> +
> +       if (!pdata)
> +               return -ENODEV;
> +
> +       mode = (enum dw_pcie_device_mode)pdata->mode;
> +
> +       artpec8_ctrl->pci = pci;
> +       artpec8_ctrl->pdata = pdata;
> +       artpec8_ctrl->mode = mode;
> +
> +       pci->dev = dev;
> +       pci->ops = pdata->dwc_ops;
> +       pci->dbi_base2 = NULL;
> +       pci->dbi_base = NULL;
> +       pp = &pci->pp;
> +       pp->ops = artpec8_ctrl->pdata->host_ops;
> +
> +       if (mode == DW_PCIE_RC_TYPE)
> +               artpec8_ctrl->link_id = of_alias_get_id(np, "pcierc");

Nope. We don't use aliases for PCI devices.

Based on how you use it, this should be an arg cell for the sysreg phandle.

> +       else
> +               artpec8_ctrl->link_id = of_alias_get_id(np, "pcieep");
> +
> +       ret = artpec8_pcie_get_subsys_resources(pdev, artpec8_ctrl);
> +       if (ret)
> +               return ret;
> +
> +       if (pdata->res_ops && pdata->res_ops->get_mem_resources) {
> +               ret = pdata->res_ops->get_mem_resources(pdev, artpec8_ctrl);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       if (pdata->res_ops && pdata->res_ops->get_clk_resources) {
> +               ret = pdata->res_ops->get_clk_resources(pdev);
> +               if (ret)
> +                       return ret;
> +
> +               ret = pdata->res_ops->init_clk_resources();
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       platform_set_drvdata(pdev, artpec8_ctrl);
> +
> +       ret = artpec8_pcie_config_isolation(pci, PCIE_CLEAR_ISOLATION);
> +       if (ret)
> +               return ret;
> +
> +       ret = artpec8_pcie_config_perstn(pci, PCIE_REG_BIT_HIGH);
> +       if (ret)
> +               return ret;
> +
> +       artpec8_ctrl->phy = devm_of_phy_get(dev, np, NULL);
> +       if (IS_ERR(artpec8_ctrl->phy))
> +               return PTR_ERR(artpec8_ctrl->phy);
> +
> +       phy_init(artpec8_ctrl->phy);
> +       phy_reset(artpec8_ctrl->phy);
> +
> +       switch (mode) {
> +       case DW_PCIE_RC_TYPE:
> +               writel(DEVICE_TYPE_RC, artpec8_ctrl->elbi_base +
> +                               PCIE_ARTPEC8_DEVICE_TYPE);
> +               ret = artpec8_add_pcie_port(artpec8_ctrl, pdev);
> +               if (ret < 0)
> +                       goto fail_probe;
> +               break;
> +       case DW_PCIE_EP_TYPE:
> +               writel(DEVICE_TYPE_EP, artpec8_ctrl->elbi_base +
> +                               PCIE_ARTPEC8_DEVICE_TYPE);
> +
> +               ret = artpec8_add_pcie_ep(artpec8_ctrl, pdev);
> +               if (ret < 0)
> +                       goto fail_probe;
> +               break;
> +       default:
> +               ret = -EINVAL;
> +               goto fail_probe;
> +       }
> +
> +       return 0;
> +
> +fail_probe:
> +       phy_exit(artpec8_ctrl->phy);
> +       if (pdata->res_ops && pdata->res_ops->deinit_clk_resources)
> +               pdata->res_ops->deinit_clk_resources();
> +
> +       return ret;
> +}
> +
> +static int __exit artpec8_pcie_remove(struct platform_device *pdev)
> +{
> +       struct artpec8_pcie *artpec8_ctrl = platform_get_drvdata(pdev);
> +       const struct artpec8_pcie_pdata *pdata = artpec8_ctrl->pdata;
> +
> +       if (pdata->res_ops && pdata->res_ops->deinit_clk_resources)
> +               pdata->res_ops->deinit_clk_resources();
> +
> +       return 0;
> +}
> +
> +static const struct artpec8_pcie_pdata artpec8_pcie_rc_pdata = {
> +       .dwc_ops        = &artpec8_dw_pcie_ops,
> +       .host_ops       = &artpec8_pcie_host_ops,
> +       .res_ops        = &artpec8_pcie_rc_res_ops,
> +       .mode           = DW_PCIE_RC_TYPE,
> +};
> +
> +static const struct artpec8_pcie_pdata artpec8_pcie_ep_pdata = {
> +       .dwc_ops        = &artpec8_dw_pcie_ops,
> +       .host_ops       = &artpec8_pcie_host_ops,
> +       .res_ops        = &artpec8_pcie_ep_res_ops,
> +       .mode           = DW_PCIE_EP_TYPE,
> +};
> +
> +static const struct of_device_id artpec8_pcie_of_match[] = {
> +       {
> +               .compatible = "axis,artpec8-pcie",
> +               .data = &artpec8_pcie_rc_pdata,
> +       },
> +       {
> +               .compatible = "axis,artpec8-pcie-ep",
> +               .data = &artpec8_pcie_ep_pdata,
> +       },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, artpec8_pcie_of_match);
> +
> +static struct platform_driver artpec8_pcie_driver = {
> +       .probe  = artpec8_pcie_probe,
> +       .remove         = __exit_p(artpec8_pcie_remove),
> +       .driver = {
> +               .name   = "artpec8-pcie",
> +               .of_match_table = artpec8_pcie_of_match,
> +       },
> +};
> +
> +module_platform_driver(artpec8_pcie_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Jaeho Cho <jaeho79.cho@samsung.com>");
> --
> 2.9.5

[PATCH v4 4/5] phy: Add ARTPEC-8 PCIe PHY driver

[Wangseok Lee]

Add support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC platform of Axis
Communications. This is based on arm64 and support GEN4 & 2lane. This
driver provides PHY interface for ARTPEC-8 SoC PCIe controller, based on
Samsung PCIe PHY IP.

Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com>
Signed-off-by: Jaeho Cho <jaeho79.cho@samsung.com>
---
v3->v4 :
-Modified to use GENMASK
-Fix indentation
-Remove the driver data

v2->v3 :
-Remove unnecessary indentation
-Redefine local struct to statis const
-Add static const to struct that requires static const definition
-Remove wrappers on writel and readl

v1->v2 :
-Change folder name of phy driver to axis from artpec
---
 drivers/phy/Kconfig                 |   1 +
 drivers/phy/Makefile                |   1 +
 drivers/phy/axis/Kconfig            |   9 +
 drivers/phy/axis/Makefile           |   2 +
 drivers/phy/axis/phy-artpec8-pcie.c | 753 ++++++++++++++++++++++++++++++++++++
 5 files changed, 766 insertions(+)
 create mode 100644 drivers/phy/axis/Kconfig
 create mode 100644 drivers/phy/axis/Makefile
 create mode 100644 drivers/phy/axis/phy-artpec8-pcie.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 300b0f2..92b8232 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -73,6 +73,7 @@ config PHY_CAN_TRANSCEIVER
 
 source "drivers/phy/allwinner/Kconfig"
 source "drivers/phy/amlogic/Kconfig"
+source "drivers/phy/axis/Kconfig"
 source "drivers/phy/broadcom/Kconfig"
 source "drivers/phy/cadence/Kconfig"
 source "drivers/phy/freescale/Kconfig"
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 01e9eff..808c055e 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_PHY_PISTACHIO_USB)		+= phy-pistachio-usb.o
 obj-$(CONFIG_USB_LGM_PHY)		+= phy-lgm-usb.o
 obj-y					+= allwinner/	\
 					   amlogic/	\
+					   axis/		\
 					   broadcom/	\
 					   cadence/	\
 					   freescale/	\
diff --git a/drivers/phy/axis/Kconfig b/drivers/phy/axis/Kconfig
new file mode 100644
index 0000000..68f7ddf
--- /dev/null
+++ b/drivers/phy/axis/Kconfig
@@ -0,0 +1,9 @@
+config PHY_ARTPEC8_PCIE
+	bool "ARTPEC-8 PCIe PHY driver"
+	depends on OF && (ARCH_ARTPEC || COMPILE_TEST)
+	select GENERIC_PHY
+	help
+	  Enable PCIe PHY support for ARTPEC-8 SoC.
+	  This driver provides PHY interface for ARTPEC-8 SoC
+	  PCIe controller.
+	  This is based on Samsung PCIe PHY IP.
diff --git a/drivers/phy/axis/Makefile b/drivers/phy/axis/Makefile
new file mode 100644
index 0000000..45d853c
--- /dev/null
+++ b/drivers/phy/axis/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_PHY_ARTPEC8_PCIE)		+= phy-artpec8-pcie.o
diff --git a/drivers/phy/axis/phy-artpec8-pcie.c b/drivers/phy/axis/phy-artpec8-pcie.c
new file mode 100644
index 0000000..b292d40
--- /dev/null
+++ b/drivers/phy/axis/phy-artpec8-pcie.c
@@ -0,0 +1,753 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PHY provider for ARTPEC-8 PCIe controller
+ *
+ * Copyright (C) 2019 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ *
+ * Author: Jaeho Cho <jaeho79.cho@samsung.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/init.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/phy/phy.h>
+#include <linux/regmap.h>
+#include <linux/debugfs.h>
+
+/* ARTPEC-8 PCIe PHY registers */
+/* CMN registers */
+#define PCIE_PHY_CMN_REG004		0x10
+#define PCIE_PHY_CMN_REG00B		0x2C
+#define PCIE_PHY_CMN_REG016		0x58
+#define PCIE_PHY_CMN_REG01C		0x70
+#define PCIE_PHY_CMN_REG021		0x84
+#define PCIE_PHY_CMN_REG024		0x90
+#define PCIE_PHY_CMN_REG025		0x94
+#define PCIE_PHY_CMN_REG0E6		0x398
+#define PCIE_PHY_CMN_REG0E7		0x39C
+#define PCIE_PHY_CMN_REG0E8		0x3A0
+#define PCIE_PHY_CMN_REG0E9		0x3A4
+#define PCIE_PHY_CMN_REG0EA		0x3A8
+#define PCIE_PHY_CMN_REG0EB		0x3AC
+#define PCIE_PHY_CMN_REG0EC		0x3B0
+#define PCIE_PHY_CMN_REG0EE		0x3B8
+#define PCIE_PHY_CMN_REG0EF		0x3BC
+#define PCIE_PHY_CMN_REG0F1		0x3C4
+#define PCIE_PHY_CMN_REG0F3		0x3CC
+#define PCIE_PHY_CMN_REG0F4		0x3D0
+
+#define PCIE_PHY_CMN_REG101		0x404
+#define OV_I_CMN_RSTN			BIT(4)
+#define OV_I_INIT_RSTN			BIT(6)
+
+#define PCIE_PHY_CMN_REG131		0x4C4
+#define PCIE_PHY_CMN_REG17B		0x5EC
+#define PCIE_PHY_CMN_REG17D		0x5F4
+#define PCIE_PHY_CMN_REG190		0x640
+#define PCIE_PHY_CMN_REG191		0x644
+#define PCIE_PHY_CMN_REG192		0x648
+#define PCIE_PHY_CMN_REG1C7		0x71C
+#define PCIE_PHY_CMN_REG1DF		0x77C
+#define PCIE_PHY_CMN_REG1E0		0x780
+
+#define PCIE_PHY_CMN_REG0B1		0x2C4
+#define ANA_ROPLL_REF_DIG_CLK_SEL	BIT(2)
+
+/* External clock */
+#define PCIE_PHY_CMN_REG14D		0x534
+#define PCIE_AUX_RX_MODE_EXTEND		BIT(7)
+
+#define PCIE_PHY_CMN_REG0D9		0x364
+#define ANA_AUX_EXT_REF_CLK_SEL		BIT(4)
+
+#define PCIE_PHY_CMN_REG10F		0x43C
+#define AUX_PLL_EN_EXTEND		BIT(4)
+
+#define PCIE_PHY_CMN_REG11E		0x478
+#define AUX2_PLL_EN_EXTEND		BIT(3)
+
+#define PCIE_PHY_CMN_REG0D4		0x350
+#define OV_S_ANA_AUX_EN			BIT(3)
+#define OV_I_ANA_AUX_EN			BIT(2)
+
+/* LANE registers */
+#define PCIE_PHY_TRSV_REG22D		0x8B4
+#define PCIE_PHY_TRSV_REG23E		0x8F8
+#define PCIE_PHY_TRSV_REG2A5		0xA94
+#define PCIE_PHY_TRSV_REG3E3		0xF8C
+#define PCIE_PHY_TRSV_REG3ED		0xFB4
+#define PCIE_PHY_TRSV_REG20B		0x82C
+#define PCIE_PHY_TRSV_REG20C		0x830
+#define PCIE_PHY_TRSV_REG234		0x8D0
+#define PCIE_PHY_TRSV_REG235		0x8D4
+#define PCIE_PHY_TRSV_REG237		0x8DC
+#define PCIE_PHY_TRSV_REG239		0x8E4
+#define PCIE_PHY_TRSV_REG23A		0x8E8
+#define PCIE_PHY_TRSV_REG23B		0x8EC
+#define PCIE_PHY_TRSV_REG24B		0x92C
+#define PCIE_PHY_TRSV_REG25D		0x974
+#define PCIE_PHY_TRSV_REG262		0x988
+#define PCIE_PHY_TRSV_REG271		0x9C4
+#define PCIE_PHY_TRSV_REG272		0x9C8
+#define PCIE_PHY_TRSV_REG27C		0x9F0
+#define PCIE_PHY_TRSV_REG27D		0x9F4
+#define PCIE_PHY_TRSV_REG27E		0x9F8
+#define PCIE_PHY_TRSV_REG284		0xA10
+#define PCIE_PHY_TRSV_REG289		0xA24
+#define PCIE_PHY_TRSV_REG28A		0xA28
+#define PCIE_PHY_TRSV_REG28B		0xA2C
+#define PCIE_PHY_TRSV_REG28C		0xA30
+#define PCIE_PHY_TRSV_REG28E		0xA38
+#define PCIE_PHY_TRSV_REG28F		0xA3C
+#define PCIE_PHY_TRSV_REG290		0xA40
+#define PCIE_PHY_TRSV_REG291		0xA44
+#define PCIE_PHY_TRSV_REG292		0xA48
+#define PCIE_PHY_TRSV_REG294		0xA50
+#define PCIE_PHY_TRSV_REG295		0xA54
+#define PCIE_PHY_TRSV_REG296		0xA58
+#define PCIE_PHY_TRSV_REG297		0xA5C
+#define PCIE_PHY_TRSV_REG298		0xA60
+#define PCIE_PHY_TRSV_REG29B		0xA6C
+#define PCIE_PHY_TRSV_REG29C		0xA70
+#define PCIE_PHY_TRSV_REG29D		0xA74
+#define PCIE_PHY_TRSV_REG29E		0xA78
+#define PCIE_PHY_TRSV_REG2AA		0xAA8
+#define PCIE_PHY_TRSV_REG2AE		0xAB8
+#define PCIE_PHY_TRSV_REG2C2		0xB08
+#define PCIE_PHY_TRSV_REG2C6		0xB18
+#define PCIE_PHY_TRSV_REG2C7		0xB1C
+#define PCIE_PHY_TRSV_REG2CB		0xB2C
+#define PCIE_PHY_TRSV_REG2CC		0xB30
+#define PCIE_PHY_TRSV_REG2CD		0xB34
+#define PCIE_PHY_TRSV_REG2CE		0xB38
+#define PCIE_PHY_TRSV_REG2D0		0xB40
+#define PCIE_PHY_TRSV_REG2CF		0xB3C
+#define PCIE_PHY_TRSV_REG2E0		0xB80
+#define PCIE_PHY_TRSV_REG2E9		0xBA4
+#define PCIE_PHY_TRSV_REG2EA		0xBA8
+#define PCIE_PHY_TRSV_REG2EB		0xBAC
+#define PCIE_PHY_TRSV_REG315		0xC54
+#define PCIE_PHY_TRSV_REG317		0xC5C
+#define PCIE_PHY_TRSV_REG319		0xC64
+#define PCIE_PHY_TRSV_REG364		0xD90
+#define PCIE_PHY_TRSV_REG36C		0xDB0
+#define PCIE_PHY_TRSV_REG36D		0xDB4
+#define PCIE_PHY_TRSV_REG37E		0xDF8
+#define PCIE_PHY_TRSV_REG37F		0xDFC
+#define PCIE_PHY_TRSV_REG38F		0xE3C
+#define PCIE_PHY_TRSV_REG391		0xE44
+#define PCIE_PHY_TRSV_REG39C		0xE70
+#define PCIE_PHY_TRSV_REG3A8		0xEA0
+#define PCIE_PHY_TRSV_REG3E0		0xF80
+#define PCIE_PHY_TRSV_REG3E1		0xF84
+#define PCIE_PHY_TRSV_REG3E7		0xF9C
+#define PCIE_PHY_TRSV_REG3E9		0xFA4
+#define PCIE_PHY_TRSV_REG3EA		0xFA8
+#define PCIE_PHY_TRSV_REG3EE		0xFB8
+#define PCIE_PHY_TRSV_REG3EF		0xFBC
+#define PCIE_PHY_TRSV_REG3F0		0xFC0
+
+#define PCIE_PHY_TRSV_REG2C0		0xB00
+#define LN_EQ_CTRL_RX_DATA_HOLD		BIT(5)
+
+/* RX Preset registers */
+#define PCIE_PHY_CMN_REG17E		0x5F8
+#define PCIE_PHY_CMN_REG180		0x600
+#define PCIE_PHY_CMN_REG181		0x604
+#define PCIE_PHY_CMN_REG182		0x608
+#define PCIE_PHY_CMN_REG183		0x60C
+#define PCIE_PHY_CMN_REG184		0x610
+#define PCIE_PHY_CMN_REG185		0x614
+#define PCIE_PHY_CMN_REG186		0x618
+#define PCIE_PHY_CMN_REG187		0x61C
+
+/* ARTPEC-8 PCIe PCS registers */
+#define PCIE_PCS_OUT_VEC_4		0x154
+#define B1_DYNAMIC			BIT(3)
+
+/* ARTPEC-8 SYS REG registers */
+#define FSYS_PCIE_CON			0x424
+#define PCIE_PHY_LCPLL_REFCLK_SEL	0x3
+#define PCIE_PHY_ROPLL_REFCLK_SEL	GENMASK(3, 2)
+#define ROPLL_REFCLK_NOT_AVAILABLE	BIT(3)
+#define PCIE_PHY_LN0_REFCLK_PAD_EN	BIT(10)
+#define PCIE_PHY_LN1_REFCLK_PAD_EN	BIT(11)
+#define PCIE_PHY_PWR_OFF		BIT(7)
+
+/* ARTPEC-8 Sub Controller registers */
+#define SFR_INIT_RSTN			0x1404
+#define SFR_CMN_RSTN			0x1408
+
+#define PCIE_PHY_LN0_REG_START		0x800
+#define PCIE_PHY_LN0_REG_END		0xFCC
+#define OFFSET_PER_LANE			0x800
+
+enum artpec8_pcie_phy_num_lanes {
+	LANE0 = 0,
+	LANE1,
+	LANE_MAX
+};
+
+struct artpec8_pcie_phy {
+	void __iomem *phy_base;
+	void __iomem *pcs_base;
+	void __iomem *elbi_base;
+	struct clk *soc_pll_clk;
+	struct regmap *sysreg;
+	u32 lcpll_ref_clk;
+	const char *mode;
+	u32 num_lanes;
+};
+
+enum artpec8_pcie_ref_clk {
+	REF_CLK_FROM_XO = 0,
+	REF_CLK_FROM_IO,
+	REF_CLK_RESERVED,
+	REF_CLK_FROM_SOC_PLL,
+	REF_CLK_MAX
+};
+
+struct artpec8_pcie_phy_tune_reg {
+	u32 offset;
+	u32 val;
+};
+
+/* ARTPEC-8 PCIe Gen4 x2 PHY CMN register settings */
+static const struct artpec8_pcie_phy_tune_reg cmn_regs[] = {
+	{PCIE_PHY_CMN_REG004, 0x65},
+	{PCIE_PHY_CMN_REG00B, 0x18},
+	{PCIE_PHY_CMN_REG016, 0x0E},
+	{PCIE_PHY_CMN_REG01C, 0x4F},
+	{PCIE_PHY_CMN_REG021, 0x01},
+	{PCIE_PHY_CMN_REG024, 0x58},
+	{PCIE_PHY_CMN_REG025, 0x98},
+	{PCIE_PHY_CMN_REG0E6, 0x00},
+	{PCIE_PHY_CMN_REG0E7, 0x00},
+	{PCIE_PHY_CMN_REG0E8, 0x3F},
+	{PCIE_PHY_CMN_REG0E9, 0x3F},
+	{PCIE_PHY_CMN_REG0EA, 0xFF},
+	{PCIE_PHY_CMN_REG0EB, 0xFF},
+	{PCIE_PHY_CMN_REG0EC, 0x42},
+	{PCIE_PHY_CMN_REG0EE, 0x3F},
+	{PCIE_PHY_CMN_REG0EF, 0x7F},
+	{PCIE_PHY_CMN_REG0F1, 0x02},
+	{PCIE_PHY_CMN_REG0F3, 0xFF},
+	{PCIE_PHY_CMN_REG0F4, 0xFF},
+	{PCIE_PHY_CMN_REG131, 0x01},
+	{PCIE_PHY_CMN_REG17B, 0xC0},
+	{PCIE_PHY_CMN_REG17D, 0xAF},
+	{PCIE_PHY_CMN_REG190, 0x27},
+	{PCIE_PHY_CMN_REG191, 0x0F},
+	{PCIE_PHY_CMN_REG192, 0x3F},
+	{PCIE_PHY_CMN_REG1C7, 0x05},
+	{PCIE_PHY_CMN_REG1DF, 0x28},
+	{PCIE_PHY_CMN_REG1E0, 0x28},
+};
+
+/* ARTPEC-8 PCIe Gen4 x2 PHY lane register settings */
+static const struct artpec8_pcie_phy_tune_reg lane_regs[] = {
+	{PCIE_PHY_TRSV_REG22D, 0x00},
+	{PCIE_PHY_TRSV_REG23E, 0x00},
+	{PCIE_PHY_TRSV_REG2A5, 0x73},
+	{PCIE_PHY_TRSV_REG3E3, 0x7B},
+	{PCIE_PHY_TRSV_REG3ED, 0x4B},
+	{PCIE_PHY_TRSV_REG20B, 0x02},
+	{PCIE_PHY_TRSV_REG20C, 0xEA},
+	{PCIE_PHY_TRSV_REG234, 0x7A},
+	{PCIE_PHY_TRSV_REG235, 0x1C},
+	{PCIE_PHY_TRSV_REG237, 0x10},
+	{PCIE_PHY_TRSV_REG239, 0x68},
+	{PCIE_PHY_TRSV_REG23A, 0xC0},
+	{PCIE_PHY_TRSV_REG23B, 0x0B},
+	{PCIE_PHY_TRSV_REG24B, 0x00},
+	{PCIE_PHY_TRSV_REG25D, 0x07},
+	{PCIE_PHY_TRSV_REG262, 0x07},
+	{PCIE_PHY_TRSV_REG271, 0x23},
+	{PCIE_PHY_TRSV_REG272, 0x5E},
+	{PCIE_PHY_TRSV_REG27C, 0x8C},
+	{PCIE_PHY_TRSV_REG27D, 0x5B},
+	{PCIE_PHY_TRSV_REG27E, 0x2C},
+	{PCIE_PHY_TRSV_REG284, 0x33},
+	{PCIE_PHY_TRSV_REG289, 0xD4},
+	{PCIE_PHY_TRSV_REG28A, 0xCC},
+	{PCIE_PHY_TRSV_REG28B, 0xD9},
+	{PCIE_PHY_TRSV_REG28C, 0xDC},
+	{PCIE_PHY_TRSV_REG28E, 0xC6},
+	{PCIE_PHY_TRSV_REG28F, 0x90},
+	{PCIE_PHY_TRSV_REG290, 0x4D},
+	{PCIE_PHY_TRSV_REG291, 0x19},
+	{PCIE_PHY_TRSV_REG292, 0x1C},
+	{PCIE_PHY_TRSV_REG294, 0x05},
+	{PCIE_PHY_TRSV_REG295, 0x10},
+	{PCIE_PHY_TRSV_REG296, 0x0C},
+	{PCIE_PHY_TRSV_REG297, 0x19},
+	{PCIE_PHY_TRSV_REG298, 0x04},
+	{PCIE_PHY_TRSV_REG29B, 0x03},
+	{PCIE_PHY_TRSV_REG29C, 0x1B},
+	{PCIE_PHY_TRSV_REG29D, 0x1B},
+	{PCIE_PHY_TRSV_REG29E, 0x1F},
+	{PCIE_PHY_TRSV_REG2AA, 0x00},
+	{PCIE_PHY_TRSV_REG2AE, 0x1F},
+	{PCIE_PHY_TRSV_REG2C2, 0x25},
+	{PCIE_PHY_TRSV_REG2C6, 0x10},
+	{PCIE_PHY_TRSV_REG2C7, 0x06},
+	{PCIE_PHY_TRSV_REG2CB, 0x10},
+	{PCIE_PHY_TRSV_REG2CC, 0x06},
+	{PCIE_PHY_TRSV_REG2CD, 0x20},
+	{PCIE_PHY_TRSV_REG2CE, 0x27},
+	{PCIE_PHY_TRSV_REG2D0, 0x10},
+	{PCIE_PHY_TRSV_REG2CF, 0x0A},
+	{PCIE_PHY_TRSV_REG2E0, 0x01},
+	{PCIE_PHY_TRSV_REG2E9, 0x11},
+	{PCIE_PHY_TRSV_REG2EA, 0x05},
+	{PCIE_PHY_TRSV_REG2EB, 0x4C},
+	{PCIE_PHY_TRSV_REG315, 0x18},
+	{PCIE_PHY_TRSV_REG317, 0x86},
+	{PCIE_PHY_TRSV_REG319, 0x8E},
+	{PCIE_PHY_TRSV_REG364, 0x00},
+	{PCIE_PHY_TRSV_REG36C, 0x03},
+	{PCIE_PHY_TRSV_REG36D, 0x04},
+	{PCIE_PHY_TRSV_REG37E, 0x06},
+	{PCIE_PHY_TRSV_REG37F, 0x04},
+	{PCIE_PHY_TRSV_REG38F, 0x40},
+	{PCIE_PHY_TRSV_REG391, 0x8B},
+	{PCIE_PHY_TRSV_REG39C, 0xFF},
+	{PCIE_PHY_TRSV_REG3A8, 0x02},
+	{PCIE_PHY_TRSV_REG3E0, 0x93},
+	{PCIE_PHY_TRSV_REG3E1, 0x79},
+	{PCIE_PHY_TRSV_REG3E7, 0xF5},
+	{PCIE_PHY_TRSV_REG3E9, 0x75},
+	{PCIE_PHY_TRSV_REG3EA, 0x0D},
+	{PCIE_PHY_TRSV_REG3EE, 0xE2},
+	{PCIE_PHY_TRSV_REG3EF, 0x6F},
+	{PCIE_PHY_TRSV_REG3F0, 0x3D}
+};
+
+static const struct artpec8_pcie_phy_tune_reg rx_preset_regs[] = {
+	/* 0 */
+	{PCIE_PHY_CMN_REG17E, 0x00},
+	{PCIE_PHY_CMN_REG180, 0x23},
+	{PCIE_PHY_CMN_REG181, 0x44},
+	{PCIE_PHY_CMN_REG182, 0x61},
+	{PCIE_PHY_CMN_REG183, 0x55},
+	{PCIE_PHY_CMN_REG184, 0x14},
+	{PCIE_PHY_CMN_REG185, 0x23},
+	{PCIE_PHY_CMN_REG186, 0x1A},
+	{PCIE_PHY_CMN_REG187, 0x04},
+	{PCIE_PHY_CMN_REG17E, 0x04},
+	{PCIE_PHY_CMN_REG17E, 0x00},
+	/* 1 */
+	{PCIE_PHY_CMN_REG17E, 0x08},
+	{PCIE_PHY_CMN_REG181, 0x42},
+	{PCIE_PHY_CMN_REG17E, 0x0C},
+	{PCIE_PHY_CMN_REG17E, 0x08},
+	/* 2 */
+	{PCIE_PHY_CMN_REG17E, 0x10},
+	{PCIE_PHY_CMN_REG181, 0x40},
+	{PCIE_PHY_CMN_REG17E, 0x14},
+	{PCIE_PHY_CMN_REG17E, 0x10},
+	/* 3 */
+	{PCIE_PHY_CMN_REG17E, 0x18},
+	{PCIE_PHY_CMN_REG181, 0x45},
+	{PCIE_PHY_CMN_REG17E, 0x1C},
+	{PCIE_PHY_CMN_REG17E, 0x18},
+	/* 4 */
+	{PCIE_PHY_CMN_REG17E, 0x20},
+	{PCIE_PHY_CMN_REG181, 0x46},
+	{PCIE_PHY_CMN_REG17E, 0x24},
+	{PCIE_PHY_CMN_REG17E, 0x20},
+	/* 5 */
+	{PCIE_PHY_CMN_REG17E, 0x28},
+	{PCIE_PHY_CMN_REG181, 0x48},
+	{PCIE_PHY_CMN_REG17E, 0x2C},
+	{PCIE_PHY_CMN_REG17E, 0x28},
+	/* 6 */
+	{PCIE_PHY_CMN_REG17E, 0x30},
+	{PCIE_PHY_CMN_REG181, 0x4A},
+	{PCIE_PHY_CMN_REG17E, 0x34},
+	{PCIE_PHY_CMN_REG17E, 0x30},
+	/* 7 */
+	{PCIE_PHY_CMN_REG17E, 0x38},
+	{PCIE_PHY_CMN_REG181, 0x4C},
+	{PCIE_PHY_CMN_REG17E, 0x3C},
+	{PCIE_PHY_CMN_REG17E, 0x38},
+	/* 8 */
+	{PCIE_PHY_CMN_REG17E, 0x40},
+	{PCIE_PHY_CMN_REG180, 0x20},
+	{PCIE_PHY_CMN_REG181, 0x20},
+	{PCIE_PHY_CMN_REG182, 0x01},
+	{PCIE_PHY_CMN_REG17E, 0x44},
+	{PCIE_PHY_CMN_REG17E, 0x40},
+	/* 9 */
+	{PCIE_PHY_CMN_REG17E, 0x48},
+	{PCIE_PHY_CMN_REG180, 0x20},
+	{PCIE_PHY_CMN_REG181, 0x21},
+	{PCIE_PHY_CMN_REG182, 0x01},
+	{PCIE_PHY_CMN_REG17E, 0x4C},
+	{PCIE_PHY_CMN_REG17E, 0x48},
+	/* 10 */
+	{PCIE_PHY_CMN_REG17E, 0x50},
+	{PCIE_PHY_CMN_REG180, 0x24},
+	{PCIE_PHY_CMN_REG181, 0x80},
+	{PCIE_PHY_CMN_REG182, 0x41},
+	{PCIE_PHY_CMN_REG183, 0xAF},
+	{PCIE_PHY_CMN_REG184, 0x26},
+	{PCIE_PHY_CMN_REG185, 0x34},
+	{PCIE_PHY_CMN_REG186, 0x24},
+	{PCIE_PHY_CMN_REG187, 0x06},
+	{PCIE_PHY_CMN_REG17E, 0x54},
+	{PCIE_PHY_CMN_REG17E, 0x50},
+	/* 11 */
+	{PCIE_PHY_CMN_REG17E, 0x58},
+	{PCIE_PHY_CMN_REG181, 0x81},
+	{PCIE_PHY_CMN_REG17E, 0x5C},
+	{PCIE_PHY_CMN_REG17E, 0x58},
+	/* 12 */
+	{PCIE_PHY_CMN_REG17E, 0x60},
+	{PCIE_PHY_CMN_REG181, 0x82},
+	{PCIE_PHY_CMN_REG17E, 0x64},
+	{PCIE_PHY_CMN_REG17E, 0x60},
+	/* 13 */
+	{PCIE_PHY_CMN_REG17E, 0x68},
+	{PCIE_PHY_CMN_REG181, 0x83},
+	{PCIE_PHY_CMN_REG17E, 0x6C},
+	{PCIE_PHY_CMN_REG17E, 0x68},
+	/* 14 */
+	{PCIE_PHY_CMN_REG17E, 0x70},
+	{PCIE_PHY_CMN_REG181, 0x84},
+	{PCIE_PHY_CMN_REG17E, 0x74},
+	{PCIE_PHY_CMN_REG17E, 0x70},
+	/* 15 */
+	{PCIE_PHY_CMN_REG17E, 0x78},
+	{PCIE_PHY_CMN_REG180, 0x24},
+	{PCIE_PHY_CMN_REG181, 0x85},
+	{PCIE_PHY_CMN_REG182, 0x80},
+	{PCIE_PHY_CMN_REG183, 0x7F},
+	{PCIE_PHY_CMN_REG184, 0x2D},
+	{PCIE_PHY_CMN_REG185, 0x34},
+	{PCIE_PHY_CMN_REG186, 0x24},
+	{PCIE_PHY_CMN_REG187, 0x05},
+	{PCIE_PHY_CMN_REG17E, 0x7C},
+	{PCIE_PHY_CMN_REG17E, 0x78},
+	/* 16 */
+	{PCIE_PHY_CMN_REG17E, 0x80},
+	{PCIE_PHY_CMN_REG181, 0x86},
+	{PCIE_PHY_CMN_REG17E, 0x84},
+	{PCIE_PHY_CMN_REG17E, 0x80},
+	/* 17 */
+	{PCIE_PHY_CMN_REG17E, 0x88},
+	{PCIE_PHY_CMN_REG181, 0x87},
+	{PCIE_PHY_CMN_REG17E, 0x8C},
+	{PCIE_PHY_CMN_REG17E, 0x88},
+	/* 18 */
+	{PCIE_PHY_CMN_REG17E, 0x90},
+	{PCIE_PHY_CMN_REG181, 0x88},
+	{PCIE_PHY_CMN_REG17E, 0x94},
+	{PCIE_PHY_CMN_REG17E, 0x90},
+	/* 19 */
+	{PCIE_PHY_CMN_REG17E, 0x98},
+	{PCIE_PHY_CMN_REG181, 0x89},
+	{PCIE_PHY_CMN_REG17E, 0x9C},
+	{PCIE_PHY_CMN_REG17E, 0x98},
+};
+
+static void artpec8_pcie_phy_reg_update(void __iomem *base, u32 mask,
+					u32 update, u32 reg)
+{
+	u32 val;
+
+	val = readl(base + reg);
+	val &= ~(mask);
+	val |= update;
+	writel(val, base + reg);
+};
+
+static void artpec8_pcie_enable_ref_clk_from_xo(struct artpec8_pcie_phy *phy)
+{
+	regmap_update_bits(phy->sysreg, FSYS_PCIE_CON,
+			   PCIE_PHY_LCPLL_REFCLK_SEL, REF_CLK_FROM_XO);
+	regmap_update_bits(phy->sysreg, FSYS_PCIE_CON,
+			   PCIE_PHY_ROPLL_REFCLK_SEL,
+			   ROPLL_REFCLK_NOT_AVAILABLE);
+};
+
+static void artpec8_pcie_enable_ref_clk_from_io(struct artpec8_pcie_phy *phy)
+{
+	artpec8_pcie_phy_reg_update(phy->phy_base,
+				    PCIE_AUX_RX_MODE_EXTEND, 0,
+				    PCIE_PHY_CMN_REG14D);
+	artpec8_pcie_phy_reg_update(phy->phy_base,
+				    ANA_AUX_EXT_REF_CLK_SEL, 0,
+				    PCIE_PHY_CMN_REG0D9);
+	artpec8_pcie_phy_reg_update(phy->phy_base,
+				    AUX_PLL_EN_EXTEND, 0, PCIE_PHY_CMN_REG10F);
+	artpec8_pcie_phy_reg_update(phy->phy_base,
+				    AUX2_PLL_EN_EXTEND, 0, PCIE_PHY_CMN_REG11E);
+	artpec8_pcie_phy_reg_update(phy->phy_base,
+				    OV_S_ANA_AUX_EN, OV_S_ANA_AUX_EN,
+				    PCIE_PHY_CMN_REG0D4);
+	artpec8_pcie_phy_reg_update(phy->phy_base,
+				    OV_I_ANA_AUX_EN, OV_I_ANA_AUX_EN,
+				    PCIE_PHY_CMN_REG0D4);
+
+	regmap_update_bits(phy->sysreg, FSYS_PCIE_CON,
+			   PCIE_PHY_LCPLL_REFCLK_SEL, REF_CLK_FROM_IO);
+	regmap_update_bits(phy->sysreg, FSYS_PCIE_CON,
+			   PCIE_PHY_ROPLL_REFCLK_SEL,
+			   ROPLL_REFCLK_NOT_AVAILABLE);
+	regmap_update_bits(phy->sysreg, FSYS_PCIE_CON,
+			   PCIE_PHY_LN0_REFCLK_PAD_EN,
+			   PCIE_PHY_LN0_REFCLK_PAD_EN);
+	regmap_update_bits(phy->sysreg, FSYS_PCIE_CON,
+			   PCIE_PHY_LN1_REFCLK_PAD_EN,
+			   PCIE_PHY_LN1_REFCLK_PAD_EN);
+}
+
+static void artpec8_pcie_enable_ref_clk_from_soc(struct artpec8_pcie_phy *phy)
+{
+	regmap_update_bits(phy->sysreg, FSYS_PCIE_CON,
+			   PCIE_PHY_LCPLL_REFCLK_SEL, REF_CLK_FROM_SOC_PLL);
+	regmap_update_bits(phy->sysreg, FSYS_PCIE_CON,
+			   PCIE_PHY_ROPLL_REFCLK_SEL,
+			   ROPLL_REFCLK_NOT_AVAILABLE);
+}
+
+static void artpec8_pcie_lane_control(struct phy *phy, u32 lane0_reg, u32 mask,
+				      u32 val)
+{
+	struct artpec8_pcie_phy *pciephy = phy_get_drvdata(phy);
+	u32 lanex_reg = lane0_reg;
+	int i;
+
+	if (lane0_reg < PCIE_PHY_LN0_REG_START ||
+	    lane0_reg > PCIE_PHY_LN0_REG_END) {
+		return;
+	}
+
+	for (i = 0; i < pciephy->num_lanes; i++) {
+		lanex_reg += OFFSET_PER_LANE * i;
+		artpec8_pcie_phy_reg_update(pciephy->phy_base, mask, val,
+					    lanex_reg);
+	}
+}
+
+static void artpec8_pcie_phy_tune(struct artpec8_pcie_phy *phy)
+{
+	int i, count;
+
+	/* init cmn registers */
+	count = ARRAY_SIZE(cmn_regs);
+	for (i = 0; i < count; i++)
+		writel(cmn_regs[i].val, phy->phy_base + cmn_regs[i].offset);
+
+	/* init lane registers */
+	count = ARRAY_SIZE(lane_regs);
+	for (i = 0; i < count; i++) {
+		writel(lane_regs[i].val, phy->phy_base + lane_regs[i].offset);
+		writel(lane_regs[i].val, phy->phy_base + lane_regs[i].offset +
+		       OFFSET_PER_LANE);
+	}
+
+	/* rx preset registers */
+	count = ARRAY_SIZE(rx_preset_regs);
+	for (i = 0; i < count; i++)
+		writel(rx_preset_regs[i].val, phy->phy_base +
+		rx_preset_regs[i].offset);
+}
+
+static int artpec8_pcie_phy_init(struct phy *phy)
+{
+	struct artpec8_pcie_phy *pciephy = phy_get_drvdata(phy);
+
+	/* reset init_rstn and cmn_rstn */
+	artpec8_pcie_phy_reg_update(pciephy->phy_base, OV_I_CMN_RSTN |
+				    OV_I_INIT_RSTN, 0, PCIE_PHY_CMN_REG101);
+
+	/* reference clock selection */
+	switch (pciephy->lcpll_ref_clk) {
+	case REF_CLK_FROM_XO:
+		artpec8_pcie_enable_ref_clk_from_xo(pciephy);
+		break;
+	case REF_CLK_FROM_IO:
+		artpec8_pcie_enable_ref_clk_from_io(pciephy);
+		break;
+	case REF_CLK_FROM_SOC_PLL:
+		artpec8_pcie_enable_ref_clk_from_soc(pciephy);
+		break;
+	default:
+		break;
+	}
+
+	/* release i_init_rstn */
+	artpec8_pcie_phy_reg_update(pciephy->phy_base, OV_I_INIT_RSTN,
+				    OV_I_INIT_RSTN, PCIE_PHY_CMN_REG101);
+
+	/* phy initial settings */
+	artpec8_pcie_phy_tune(pciephy);
+
+	/* pll_en should be set to off when PM_STATE is P1.CPM */
+	if (!strncmp(pciephy->mode, "pcie_ep", strlen("pcie_ep"))) {
+		artpec8_pcie_phy_reg_update(pciephy->pcs_base, B1_DYNAMIC,
+					    B1_DYNAMIC, PCIE_PCS_OUT_VEC_4);
+	}
+
+	/* disable lane eq ctrl rx data hold */
+	artpec8_pcie_lane_control(phy, PCIE_PHY_TRSV_REG2C0,
+				  LN_EQ_CTRL_RX_DATA_HOLD, 0);
+
+	return 0;
+}
+
+static int artpec8_pcie_phy_exit(struct phy *phy)
+{
+	return 0;
+}
+
+static int artpec8_pcie_phy_reset(struct phy *phy)
+{
+	struct artpec8_pcie_phy *pciephy = phy_get_drvdata(phy);
+
+	artpec8_pcie_phy_reg_update(pciephy->phy_base, OV_I_CMN_RSTN,
+				    0, PCIE_PHY_CMN_REG101);
+	udelay(10);
+	artpec8_pcie_phy_reg_update(pciephy->phy_base, OV_I_CMN_RSTN,
+				    OV_I_CMN_RSTN, PCIE_PHY_CMN_REG101);
+
+	return 0;
+}
+
+static int artpec8_pcie_phy_power_on(struct phy *phy)
+{
+	struct artpec8_pcie_phy *pciephy = phy_get_drvdata(phy);
+
+	regmap_update_bits(pciephy->sysreg, FSYS_PCIE_CON,
+			   PCIE_PHY_PWR_OFF, 0);
+
+	return 0;
+}
+
+static int artpec8_pcie_phy_power_off(struct phy *phy)
+{
+	struct artpec8_pcie_phy *pciephy = phy_get_drvdata(phy);
+
+	regmap_update_bits(pciephy->sysreg, FSYS_PCIE_CON,
+			   PCIE_PHY_PWR_OFF, PCIE_PHY_PWR_OFF);
+
+	return 0;
+}
+
+static const struct phy_ops artpec8_phy_ops = {
+	.init		= artpec8_pcie_phy_init,
+	.exit		= artpec8_pcie_phy_exit,
+	.reset		= artpec8_pcie_phy_reset,
+	.power_on	= artpec8_pcie_phy_power_on,
+	.power_off	= artpec8_pcie_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static int artpec8_pcie_phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct artpec8_pcie_phy *artpec8_phy;
+	struct phy *generic_phy;
+	struct phy_provider *phy_provider;
+
+	artpec8_phy = devm_kzalloc(dev, sizeof(*artpec8_phy), GFP_KERNEL);
+	if (!artpec8_phy)
+		return -ENOMEM;
+
+	/* reference clock */
+	if (of_property_read_u32(dev->of_node, "axis,lcpll-ref-clk",
+				 &artpec8_phy->lcpll_ref_clk)) {
+		return -EINVAL;
+	}
+	/* PLL SOC reference clock */
+	if (artpec8_phy->lcpll_ref_clk == REF_CLK_FROM_SOC_PLL) {
+		artpec8_phy->soc_pll_clk = devm_clk_get(dev, "ref");
+		if (IS_ERR(artpec8_phy->soc_pll_clk))
+			return -EINVAL;
+		clk_prepare_enable(artpec8_phy->soc_pll_clk);
+	}
+
+	/* link mode */
+	if (of_property_read_string(dev->of_node, "mode", &artpec8_phy->mode))
+		return -EINVAL;
+
+	/* number of lanes */
+	if (of_property_read_u32(dev->of_node, "num-lanes",
+				 &artpec8_phy->num_lanes))
+		return -EINVAL;
+
+	if (artpec8_phy->num_lanes > LANE_MAX)
+		return -EINVAL;
+
+	/* PHY base register */
+	artpec8_phy->phy_base =
+			devm_platform_ioremap_resource_byname(pdev, "phy");
+	if (IS_ERR(artpec8_phy->phy_base))
+		return PTR_ERR(artpec8_phy->phy_base);
+
+	/* PCS base register */
+	artpec8_phy->pcs_base =
+			devm_platform_ioremap_resource_byname(pdev, "pcs");
+	if (IS_ERR(artpec8_phy->pcs_base))
+		return PTR_ERR(artpec8_phy->pcs_base);
+
+	/* sysreg regmap handle, need to change using smc */
+	artpec8_phy->sysreg =
+		syscon_regmap_lookup_by_phandle(dev->of_node,
+						"samsung,fsys-sysreg");
+	if (IS_ERR(artpec8_phy->sysreg))
+		return PTR_ERR(artpec8_phy->sysreg);
+
+	generic_phy = devm_phy_create(dev, dev->of_node, &artpec8_phy_ops);
+	if (IS_ERR(generic_phy))
+		return PTR_ERR(generic_phy);
+
+	phy_set_drvdata(generic_phy, artpec8_phy);
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	return 0;
+}
+
+static int __exit artpec8_pcie_remove(struct platform_device *pdev)
+{
+	struct artpec8_pcie_phy *artpec8_phy = platform_get_drvdata(pdev);
+
+	if (artpec8_phy->soc_pll_clk)
+		clk_disable_unprepare(artpec8_phy->soc_pll_clk);
+
+	return 0;
+}
+
+static const struct of_device_id artpec8_pcie_phy_match[] = {
+	{
+		.compatible = "axis,artpec8-pcie-phy",
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, artpec8_pcie_phy_match);
+
+static struct platform_driver artpec8_pcie_phy_driver = {
+	.probe	= artpec8_pcie_phy_probe,
+	.remove	= __exit_p(artpec8_pcie_phy_remove),
+	.driver = {
+		.of_match_table	= artpec8_pcie_phy_match,
+		.name		= "artpec8_pcie_phy",
+	}
+};
+
+module_platform_driver(artpec8_pcie_phy_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jaeho Cho <jaeho79.cho@samsung.com>");
-- 
2.9.5

[PATCH v4 5/5] MAINTAINERS: Add Axis ARTPEC-8 PCIe PHY maintainers

[Wangseok Lee]

Add maintainer for Axis ARTPEC-8 PCIe PHY. Add Jesper Nilsson
<jesper.nilsson@axis.com> and Lars Persson <lars.persson@axis.com>
as maintainer for these files. ARTPEC-8 is the SoC platform of Axis
Communications and PCIe PHY is designed based on Samsung PHY.

Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com>
---
v3->v4 :
-Add axis,artpec8-pcie.yaml and axis,artpec8-pcie-ep.yaml
-Change file path to axis from artpec
---
 MAINTAINERS | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 264e7a7..e4a0635 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1897,12 +1897,16 @@ M:	Jesper Nilsson <jesper.nilsson@axis.com>
 M:	Lars Persson <lars.persson@axis.com>
 L:	linux-arm-kernel@axis.com
 S:	Maintained
+F:	Documentation/devicetree/bindings/pci/axis,artpec8-pcie-ep.yaml
+F:	Documentation/devicetree/bindings/pci/axis,artpec8-pcie.yaml
+F:	Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
 F:	Documentation/devicetree/bindings/pinctrl/axis,artpec6-pinctrl.txt
 F:	arch/arm/boot/dts/artpec6*
 F:	arch/arm/mach-artpec
 F:	drivers/clk/axis
 F:	drivers/crypto/axis
 F:	drivers/mmc/host/usdhi6rol0.c
+F:	drivers/phy/axis/phy-artpec*
 F:	drivers/pinctrl/pinctrl-artpec*
 
 ARM/ASPEED I2C DRIVER
-- 
2.9.5