PCI: Add new Unisoc PCIe driver
Subsystems (4)
v3 cover PCI: Add new Unisoc PCIe driver
From: Hongtao Wu <[email protected]>
This series adds PCIe controller driver for Unisoc SoCs. This controller is based on DesignWare PCIe IP.
Changes from v1:
1) Test this patch on top of Rob Herring's 40 part series of DWC clean-ups:
https://lore.kernel.org/linux-pci/[email protected]/
2) Delete empty function
3) Document property "sprd,pcie-poweron-syscons" and 'sprd,pcie-poweroff-syscons'
4) Delete runtime suspend/resume function
5) Add COMPILE_TEST which CONFIG_PCIE_SPRD depends on
Changes from v2:
1) Change RC mode to host mode in drivers/pci/controller/dwc/Kconfig
2) Change Signed-off-by from Billows Wu to Hongtao Wu
Hongtao Wu (2):
dt-bindings: PCI: sprd: Document Unisoc PCIe RC host controller
PCI: sprd: Add support for Unisoc SoCs' PCIe controller
.../devicetree/bindings/pci/sprd-pcie.yaml | 101 +++++++++ drivers/pci/controller/dwc/Kconfig | 13 ++ drivers/pci/controller/dwc/Makefile | 1 + drivers/pci/controller/dwc/pcie-sprd.c | 231 +++++++++++++++++++++ 4 files changed, 346 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/sprd-pcie.yaml create mode 100644 drivers/pci/controller/dwc/pcie-sprd.c
--
2.7.4
PATCH v3 1/2 dt-bindings: PCI: sprd: Document Unisoc PCIe RC host controller
From: Hongtao Wu <[email protected]>
This series adds PCIe bindings for Unisoc SoCs.
This controller is based on DesignWare PCIe IP.
--- .../devicetree/bindings/pci/sprd-pcie.yaml | 101 +++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/sprd-pcie.yaml
Documentation/devicetree/bindings/pci/sprd-pcie.yaml new file +101 -0
@@ -0,0 +1,101 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pci/sprd-pcie.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: SoC PCIe Host Controller Device Tree Bindings + +maintainers: + - Hongtao Wu <[email protected]> + +allOf: + - $ref: /schemas/pci/pci-bus.yaml# + +properties: + compatible: + items: + - const: sprd,pcie-rc + + reg: + minItems: 2 + items: + - description: Controller control and status registers. + - description: PCIe configuration registers. + + reg-names: + items: + - const: dbi + - const: config + + ranges: + maxItems: 2 + + num-lanes: + maximum: 1 + description: Number of lanes to use for this port. + + interrupts: + minItems: 1 + description: Builtin MSI controller and PCIe host controller. + + interrupt-names: + items: + - const: msi + + sprd-pcie-poweron-syscons: + minItems: 1 + description: Global register. + The first value is the phandle to the global registers required to + confige PCIe phy, clock and so on. + The second value is the global register type which indicates whether it + is a set/clear register or not. + The third value is the time to delay after the global register is set or + cleared. + The fourth value is the global register address. + The fifth value is the the mask value that the global register must + be operate. + The sixth value is the value that will be set to the global register. + Note that Some Unisoc global registers have not been upstreamed. + The global register and its mask can't be found in linux kernel, + so we use an offset address and a number to instead them. + +required: + - compatible + - reg + - reg-names + - num-lanes + - ranges + - interrupts + - interrupt-names + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + + ipa { + #address-cells = <2>; + #size-cells = <2>; + + pcie0: pcie@2b100000 { + compatible = "sprd,pcie-rc"; + reg = <0x0 0x2b100000 0x0 0x2000>, + <0x2 0x00000000 0x0 0x2000>; + reg-names = "dbi", "config"; + #address-cells = <3>; + #size-cells = <2>; + device_type = "pci"; + ranges = <0x01000000 0x0 0x00000000 0x2 0x00002000 0x0 0x00010000>, + <0x03000000 0x0 0x10000000 0x2 0x10000000 0x1 0xefffffff>; + num-lanes = <1>; + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "msi"; + + sprd,pcie-poweron-syscons = + <&ap_ipa_ahb_regs 0 0 0x0000 0x40 0x40>, + <&ap_ipa_ahb_regs 0 0 0x0000 0x20 0x20>; + sprd,pcie-poweroff-syscons = + <&ap_ipa_ahb_regs 0 0 0x0000 0x20 0x0>, + <&ap_ipa_ahb_regs 0 0 0x0000 0x40 0x0>; + }; + };
--
2.7.4
Rob Herring
maintainerRe: [PATCH v3 1/2] dt-bindings: PCI: sprd: Document Unisoc PCIe RC host controller
Quoted 53 lines · Hongtao Wu
From: Hongtao Wu <[email protected]>
This series adds PCIe bindings for Unisoc SoCs.
This controller is based on DesignWare PCIe IP.Signed-off-by Hongtao Wu <[email protected]>--- .../devicetree/bindings/pci/sprd-pcie.yaml | 101 +++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/sprd-pcie.yamlDocumentation/devicetree/bindings/pci/sprd-pcie.yaml new file +36
-0@@ -0,0 +1,101 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pci/sprd-pcie.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: SoC PCIe Host Controller Device Tree Bindings + +maintainers: + - Hongtao Wu <[email protected]> + +allOf: + - $ref: /schemas/pci/pci-bus.yaml# + +properties: + compatible: + items: + - const: sprd,pcie-rc + + reg: + minItems: 2 + items: + - description: Controller control and status registers. + - description: PCIe configuration registers. + + reg-names: + items: + - const: dbi + - const: config + + ranges: + maxItems: 2 + + num-lanes: + maximum: 1 + description: Number of lanes to use for this port.
Documentation/devicetree/bindings/pci/sprd-pcie.yaml line 46
Quoted 10 lines
Documentation/devicetree/bindings/pci/sprd-pcie.yaml new file +10
-0+ + interrupts: + minItems: 1 + description: Builtin MSI controller and PCIe host controller. + + interrupt-names: + items: + - const: msi + + sprd-pcie-poweron-syscons:
Doesn't match the example.
Quoted 5 lines
Documentation/devicetree/bindings/pci/sprd-pcie.yaml new file +5
-0+ minItems: 1 + description: Global register. + The first value is the phandle to the global registers required to + confige PCIe phy, clock and so on. + The second value is the global register type which indicates whether it
Documentation/devicetree/bindings/pci/sprd-pcie.yaml line 61
Quoted 10 lines
Documentation/devicetree/bindings/pci/sprd-pcie.yaml new file +10
-0+ is a set/clear register or not. + The third value is the time to delay after the global register is set or + cleared. + The fourth value is the global register address. + The fifth value is the the mask value that the global register must + be operate. + The sixth value is the value that will be set to the global register. + Note that Some Unisoc global registers have not been upstreamed. + The global register and its mask can't be found in linux kernel, + so we use an offset address and a number to instead them.
From the example, it looks like you set/clear 2 bits for power on/off. What's the worst case you expect here? What do the 2 bits do? If they are for clocks, resets, or power domains, then we have bindings for those which should be used. This use of phandles to syscons should be avoided whenever possible.
If we wanted a language for specifying sequences of register accesses in DT, we would have defined that a long time ago.
Quoted 26 lines
Documentation/devicetree/bindings/pci/sprd-pcie.yaml new file +26
-0+ +required: + - compatible + - reg + - reg-names + - num-lanes + - ranges + - interrupts + - interrupt-names + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + + ipa { + #address-cells = <2>; + #size-cells = <2>; + + pcie0: pcie@2b100000 { + compatible = "sprd,pcie-rc"; + reg = <0x0 0x2b100000 0x0 0x2000>, + <0x2 0x00000000 0x0 0x2000>; + reg-names = "dbi", "config"; + #address-cells = <3>; + #size-cells = <2>; + device_type = "pci";
Documentation/devicetree/bindings/pci/sprd-pcie.yaml line 97
Quoted 10 lines
Documentation/devicetree/bindings/pci/sprd-pcie.yaml new file +10
-0+ ranges = <0x01000000 0x0 0x00000000 0x2 0x00002000 0x0 0x00010000>, + <0x03000000 0x0 0x10000000 0x2 0x10000000 0x1 0xefffffff>; + num-lanes = <1>; + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "msi"; + + sprd,pcie-poweron-syscons = + <&ap_ipa_ahb_regs 0 0 0x0000 0x40 0x40>, + <&ap_ipa_ahb_regs 0 0 0x0000 0x20 0x20>; + sprd,pcie-poweroff-syscons =
Not documented.
Quoted 7 lines
Documentation/devicetree/bindings/pci/sprd-pcie.yaml new file +4
-1+ <&ap_ipa_ahb_regs 0 0 0x0000 0x20 0x0>, + <&ap_ipa_ahb_regs 0 0 0x0000 0x40 0x0>; + }; + }; --2.7.4
Re: [PATCH v3 1/2] dt-bindings: PCI: sprd: Document Unisoc PCIe RC host controller
Quoted 57 lines · Rob Herring
Quoted 55 lines · Hongtao Wu
From: Hongtao Wu <[email protected]>
This series adds PCIe bindings for Unisoc SoCs.
This controller is based on DesignWare PCIe IP.Signed-off-by Hongtao Wu <[email protected]>--- .../devicetree/bindings/pci/sprd-pcie.yaml | 101 +++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/sprd-pcie.yamlDocumentation/devicetree/bindings/pci/sprd-pcie.yaml new file +38
-0@@ -0,0 +1,101 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pci/sprd-pcie.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: SoC PCIe Host Controller Device Tree Bindings + +maintainers: + - Hongtao Wu <[email protected]> + +allOf: + - $ref: /schemas/pci/pci-bus.yaml# + +properties: + compatible: + items: + - const: sprd,pcie-rc + + reg: + minItems: 2 + items: + - description: Controller control and status registers. + - description: PCIe configuration registers. + + reg-names: + items: + - const: dbi + - const: config + + ranges: + maxItems: 2 + + num-lanes: + maximum: 1 + description: Number of lanes to use for this port. + + interrupts:
Quoted 9 lines
Documentation/devicetree/bindings/pci/sprd-pcie.yaml line 46
Quoted 8 lines
Documentation/devicetree/bindings/pci/sprd-pcie.yaml new file +8
-0+ minItems: 1 + description: Builtin MSI controller and PCIe host controller. + + interrupt-names: + items: + - const: msi + + sprd-pcie-poweron-syscons:
I am Sorry!
I'll fix it.
Quoted 18 lines
Doesn't match the example.
Quoted 5 lines
Documentation/devicetree/bindings/pci/sprd-pcie.yaml new file +5
-0+ minItems: 1 + description: Global register. + The first value is the phandle to the global registers required to + confige PCIe phy, clock and so on. + The second value is the global register type which indicates whether itDocumentation/devicetree/bindings/pci/sprd-pcie.yaml line 61
Quoted 10 lines
Documentation/devicetree/bindings/pci/sprd-pcie.yaml new file +10
-0+ is a set/clear register or not. + The third value is the time to delay after the global register is set or + cleared. + The fourth value is the global register address. + The fifth value is the the mask value that the global register must + be operate. + The sixth value is the value that will be set to the global register. + Note that Some Unisoc global registers have not been upstreamed. + The global register and its mask can't be found in linux kernel, + so we use an offset address and a number to instead them.
Quoted 6 lines
From the example, it looks like you set/clear 2 bits for power on/off. What's the worst case you expect here? What do the 2 bits do? If they are for clocks, resets, or power domains, then we have bindings for those which should be used. This use of phandles to syscons should be avoided whenever possible.
There are two kinds of global register ( set/clear registers and non-set/clear registers )
about PCIe on Unisoc SoCs.
Each set of set/clear registers contain two addresses. One can be written and the other one
can be read. Different bits in the set/clear register indicate different functions, so we
set/clear one bit for power on/off.
The non-set/clear registers are normal which only have one address.
The second value in property 'sprd,pcie-poweron-syscons' is a flag which indicates whether
the global register is set/clear or not. If this value is 1, we think that it's a set/clear register.
If this value is 0, we think it's a non-set/clear register.
I wanted to parse all of the global registers about power on/off in an array (include set/clear
registers and non-set/clear registers). However, it may not be a good idea. I'll split the property 'sprd,pcie-poweron-syscons' info clocks, power domains, phy and so on
in the next version.
Quoted 34 lines
If we wanted a language for specifying sequences of register accesses in DT, we would have defined that a long time ago.
Quoted 30 lines
Documentation/devicetree/bindings/pci/sprd-pcie.yaml new file +30
-0+ +required: + - compatible + - reg + - reg-names + - num-lanes + - ranges + - interrupts + - interrupt-names + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + + ipa { + #address-cells = <2>; + #size-cells = <2>; + + pcie0: pcie@2b100000 { + compatible = "sprd,pcie-rc"; + reg = <0x0 0x2b100000 0x0 0x2000>, + <0x2 0x00000000 0x0 0x2000>; + reg-names = "dbi", "config"; + #address-cells = <3>; + #size-cells = <2>; + device_type = "pci"; + ranges = <0x01000000 0x0 0x00000000 0x2 0x00002000 0x0 0x00010000>, + <0x03000000 0x0 0x10000000 0x2 0x10000000 0x1 0xefffffff>; + num-lanes = <1>; + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
Quoted 9 lines
Documentation/devicetree/bindings/pci/sprd-pcie.yaml line 97
Quoted 6 lines
Documentation/devicetree/bindings/pci/sprd-pcie.yaml new file +6
-0+ interrupt-names = "msi"; + + sprd,pcie-poweron-syscons = + <&ap_ipa_ahb_regs 0 0 0x0000 0x40 0x40>, + <&ap_ipa_ahb_regs 0 0 0x0000 0x20 0x20>; + sprd,pcie-poweroff-syscons =Not documented.
Thanks! I'll fix it in the next version.
Quoted 7 lines
Quoted 7 lines
Documentation/devicetree/bindings/pci/sprd-pcie.yaml new file +4
-1+ <&ap_ipa_ahb_regs 0 0 0x0000 0x20 0x0>, + <&ap_ipa_ahb_regs 0 0 0x0000 0x40 0x0>; + }; + }; --2.7.4
PATCH v3 2/2 PCI: sprd: Add support for Unisoc SoCs' PCIe controller
In reply to Hongtao WuFrom: Hongtao Wu <[email protected]>
This series adds PCIe controller driver for Unisoc SoCs. This controller is based on DesignWare PCIe IP.
--- drivers/pci/controller/dwc/Kconfig | 13 ++ drivers/pci/controller/dwc/Makefile | 1 + drivers/pci/controller/dwc/pcie-sprd.c | 231 +++++++++++++++++++++++++++++++++ 3 files changed, 245 insertions(+) create mode 100644 drivers/pci/controller/dwc/pcie-sprd.c
drivers/pci/controller/dwc/Kconfig +13 -0
@@ -311,4 +311,17 @@ config PCIE_AL required only for DT-based platforms. ACPI platforms with the Annapurna Labs PCIe controller don't need to enable this. + +config PCIE_SPRD + tristate "Unisoc PCIe controller - Host Mode" + depends on ARCH_SPRD || COMPILE_TEST + depends on PCI_MSI_IRQ_DOMAIN + select PCIE_DW_HOST + help + Unisoc PCIe controller uses the DesignWare core. It can be configured + as an Endpoint (EP) or a Root complex (RC). In order to enable host + mode (the controller works as RC), PCIE_SPRD must be selected. + Say Y or M here if you want to PCIe RC controller support on Unisoc + SoCs. + endmenu
drivers/pci/controller/dwc/Makefile +1 -0
@@ -20,6 +20,7 @@ obj-$(CONFIG_PCI_MESON) += pci-meson.o obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o +obj-$(CONFIG_PCIE_SPRD) += pcie-sprd.o # The following drivers are for devices that use the generic ACPI # pci_root.c driver but don't support standard ECAM config access.
drivers/pci/controller/dwc/pcie-sprd.c new file +231 -0
231 lines not on this page · raw message
--
2.7.4
Rob Herring
maintainerRe: [PATCH v3 2/2] PCI: sprd: Add support for Unisoc SoCs' PCIe controller
Quoted 133 lines · Hongtao Wu
From: Hongtao Wu <[email protected]>
This series adds PCIe controller driver for Unisoc SoCs. This controller is based on DesignWare PCIe IP.
Signed-off-by Hongtao Wu <[email protected]>--- drivers/pci/controller/dwc/Kconfig | 13 ++ drivers/pci/controller/dwc/Makefile | 1 + drivers/pci/controller/dwc/pcie-sprd.c | 231 +++++++++++++++++++++++++++++++++ 3 files changed, 245 insertions(+) create mode 100644 drivers/pci/controller/dwc/pcie-sprd.cdrivers/pci/controller/dwc/Kconfig +13
-0@@ -311,4 +311,17 @@ config PCIE_AL required only for DT-based platforms. ACPI platforms with the Annapurna Labs PCIe controller don't need to enable this. + +config PCIE_SPRD + tristate "Unisoc PCIe controller - Host Mode" + depends on ARCH_SPRD || COMPILE_TEST + depends on PCI_MSI_IRQ_DOMAIN + select PCIE_DW_HOST + help + Unisoc PCIe controller uses the DesignWare core. It can be configured + as an Endpoint (EP) or a Root complex (RC). In order to enable host + mode (the controller works as RC), PCIE_SPRD must be selected. + Say Y or M here if you want to PCIe RC controller support on Unisoc + SoCs. + endmenudrivers/pci/controller/dwc/Makefile +1
-0@@ -20,6 +20,7 @@ obj-$(CONFIG_PCI_MESON) += pci-meson.o obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o +obj-$(CONFIG_PCIE_SPRD) += pcie-sprd.o # The following drivers are for devices that use the generic ACPI # pci_root.c driver but don't support standard ECAM config access.drivers/pci/controller/dwc/pcie-sprd.c new file +80
-0@@ -0,0 +1,231 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* PCIe host controller driver for Unisoc SoCs +* +* Copyright (C) 2020 Unisoc, Inc. +* +* Author: Hongtao Wu <[email protected]> +*/ + +#include <linux/delay.h> +#include <linux/gpio/consumer.h> +#include <linux/interrupt.h> +#include <linux/mfd/syscon.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/of_irq.h> +#include <linux/platform_device.h> +#include <linux/property.h> +#include <linux/regmap.h> + +#include "pcie-designware.h" + +#define NUM_OF_ARGS 5 + +struct sprd_pcie { +struct dw_pcie pci; +}; + +static int sprd_pcie_syscon_setting (struct platform_device *pdev,char *env) +{ +struct device_node *np = pdev->dev.of_node; +int i, count, err; +u32 type, delay, reg, mask, val, tmp_val; +struct of_phandle_args out_args; +struct regmap *iomap; +struct device *dev = &pdev->dev; + +if (!of_find_property (np, env,NULL )) { +dev_info (dev,"There isn't property %s in dts\n" , env); +return 0 ; + } + + count =of_property_count_elems_of_size (np, env, + (NUM_OF_ARGS +1 ) *sizeof (u32)); +dev_info (dev,"Property (%s) reg count is %d :\n" , env, count); + +for (i =0 ; i < count; i++) { + err =of_parse_phandle_with_fixed_args (np, env,NUM_OF_ARGS , + i, &out_args); +if (err <0 ) +return err; + + type = out_args.args[0 ]; + delay = out_args.args[1 ]; + reg = out_args.args[2 ]; + mask = out_args.args[3 ]; + val = out_args.args[4 ]; + + iomap =syscon_node_to_regmap (out_args.np); + +switch (type) { +case 0 : +regmap_update_bits (iomap, reg, mask, val); +break ; + +case 1 : +regmap_read (iomap, reg, &tmp_val); + tmp_val &= (~mask); + tmp_val |= (val & mask); +regmap_write (iomap, reg, tmp_val); +break ; +default : +break ; + } + +if (delay) +usleep_range (delay, delay +10 ); + +regmap_read (iomap, reg, &tmp_val); + dev_dbg(dev,
drivers/pci/controller/dwc/pcie-sprd.c line 90
Quoted 10 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +10
-0+"%2d:reg[0x%8x] mask[0x%8x] val[0x%8x] result[0x%8x]\n" , + i, reg, mask, val, tmp_val); + } + +return i; +} + +static int sprd_pcie_perst_assert (struct platform_device *pdev) +{ +return sprd_pcie_syscon_setting (pdev,"sprd,pcie-perst-assert" );
Not documented. This should probably use the reset binding.
Quoted 64 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +64
-0+} + +static int sprd_pcie_perst_deassert (struct platform_device *pdev) +{ +return sprd_pcie_syscon_setting (pdev,"sprd,pcie-perst-deassert" ); +} + +static int sprd_pcie_power_on (struct platform_device *pdev) +{ +int ret; +struct device *dev = &pdev->dev; + + ret =sprd_pcie_syscon_setting (pdev,"sprd,pcie-poweron-syscons" ); +if (ret <0 ) +dev_err (dev, +"failed to set pcie poweroff syscons, return %d\n" , + ret); + +sprd_pcie_perst_deassert (pdev); + +return ret; +} + +static int sprd_pcie_power_off (struct platform_device *pdev) +{ +int ret; +struct device *dev = &pdev->dev; + +sprd_pcie_perst_assert (pdev); + + ret =sprd_pcie_syscon_setting (pdev,"sprd,pcie-poweroff-syscons" ); +if (ret <0 ) +dev_err (dev, +"failed to set pcie poweroff syscons, return %d\n" , + ret); + +return ret; +} + +static int sprd_pcie_host_init (struct pcie_port *pp) +{ +int ret; +struct dw_pcie *pci =to_dw_pcie_from_pp (pp); + +dw_pcie_setup_rc (pp); +dw_pcie_msi_init (pp); + + ret =dw_pcie_wait_for_link (pci); +if (ret) +dev_err (pci->dev,"pcie ep may has not been powered on yet\n" ); + +return ret; +} + +static const struct dw_pcie_host_ops sprd_pcie_host_ops = { + .host_init = sprd_pcie_host_init, +}; + +static int sprd_add_pcie_port (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct sprd_pcie *ctrl =platform_get_drvdata (pdev); +struct dw_pcie *pci = &ctrl->pci; +struct pcie_port *pp = &pci->pp;
drivers/pci/controller/dwc/pcie-sprd.c line 164
Quoted 10 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +10
-0+ + pci->dbi_base =devm_platform_ioremap_resource_byname (pdev,"dbi" ); +if (IS_ERR (pci->dbi_base)) { +dev_err (dev,"failed to get rc dbi base\n" ); +return PTR_ERR (pci->dbi_base); + } + + pp->ops = &sprd_pcie_host_ops; + +if (IS_ENABLED (CONFIG_PCI_MSI )) {
I don't think this check is needed. The DW core won't setup the MSI if not enabled, so doesn't matter if msi_irq is initialized.
drivers/pci/controller/dwc/pcie-sprd.c line 168
Quoted 4 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +4
-0+ pp->msi_irq =platform_get_irq_byname (pdev,"msi" ); +if (pp->msi_irq <0 ) { +dev_err (dev,"failed to get msi, return %d\n" , + pp->msi_irq);
No need to print an error, the core does this.
Quoted 43 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +43
-0+return pp->msi_irq; + } + } + +return dw_pcie_host_init (pp); +} + +static int sprd_pcie_probe (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct sprd_pcie *ctrl; +struct dw_pcie *pci; +int ret; + + ctrl =devm_kzalloc (dev,sizeof (*ctrl),GFP_KERNEL ); +if (!ctrl) +return -ENOMEM ; + + pci = &ctrl->pci; + pci->dev = dev; + +platform_set_drvdata (pdev, ctrl); + + ret =sprd_pcie_power_on (pdev); +if (ret <0 ) { +dev_err (dev,"failed to get pcie poweron syscons, return %d\n" , + ret); + goto err_power_off; + } + + ret =sprd_add_pcie_port (pdev); +if (ret) { +dev_warn (dev,"failed to initialize RC controller\n" ); +return ret; + } + +return 0 ; + +err_power_off: +sprd_pcie_power_off (pdev); + +return ret; +}
drivers/pci/controller/dwc/pcie-sprd.c line 221
Quoted 10 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +10
-0+ +static const struct of_device_id sprd_pcie_of_match[] = { + { + .compatible ="sprd,pcie-rc" , + }, + {}, +}; + +static struct platform_driver sprd_pcie_driver = { + .probe = sprd_pcie_probe,
You need a .remove hook.
Quoted 13 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +10
-1+ .driver = { + .name = "sprd-pcie", + .of_match_table = sprd_pcie_of_match, + }, +}; + +module_platform_driver (sprd_pcie_driver); + +MODULE_DESCRIPTION ("Unisoc PCIe host controller driver" ); +MODULE_LICENSE ("GPL v2" ); --2.7.4
Re: [PATCH v3 2/2] PCI: sprd: Add support for Unisoc SoCs' PCIe controller
Hi Bob,
Thanks for the review.
Quoted 139 lines · Rob Herring
Quoted 137 lines · Hongtao Wu
From: Hongtao Wu <[email protected]>
This series adds PCIe controller driver for Unisoc SoCs. This controller is based on DesignWare PCIe IP.
Signed-off-by Hongtao Wu <[email protected]>--- drivers/pci/controller/dwc/Kconfig | 13 ++ drivers/pci/controller/dwc/Makefile | 1 + drivers/pci/controller/dwc/pcie-sprd.c | 231 +++++++++++++++++++++++++++++++++ 3 files changed, 245 insertions(+) create mode 100644 drivers/pci/controller/dwc/pcie-sprd.cdrivers/pci/controller/dwc/Kconfig +13
-0@@ -311,4 +311,17 @@ config PCIE_AL required only for DT-based platforms. ACPI platforms with the Annapurna Labs PCIe controller don't need to enable this. + +config PCIE_SPRD + tristate "Unisoc PCIe controller - Host Mode" + depends on ARCH_SPRD || COMPILE_TEST + depends on PCI_MSI_IRQ_DOMAIN + select PCIE_DW_HOST + help + Unisoc PCIe controller uses the DesignWare core. It can be configured + as an Endpoint (EP) or a Root complex (RC). In order to enable host + mode (the controller works as RC), PCIE_SPRD must be selected. + Say Y or M here if you want to PCIe RC controller support on Unisoc + SoCs. + endmenudrivers/pci/controller/dwc/Makefile +1
-0@@ -20,6 +20,7 @@ obj-$(CONFIG_PCI_MESON) += pci-meson.o obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o +obj-$(CONFIG_PCIE_SPRD) += pcie-sprd.o # The following drivers are for devices that use the generic ACPI # pci_root.c driver but don't support standard ECAM config access.drivers/pci/controller/dwc/pcie-sprd.c new file +84
-0@@ -0,0 +1,231 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* PCIe host controller driver for Unisoc SoCs +* +* Copyright (C) 2020 Unisoc, Inc. +* +* Author: Hongtao Wu <[email protected]> +*/ + +#include <linux/delay.h> +#include <linux/gpio/consumer.h> +#include <linux/interrupt.h> +#include <linux/mfd/syscon.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/of_irq.h> +#include <linux/platform_device.h> +#include <linux/property.h> +#include <linux/regmap.h> + +#include "pcie-designware.h" + +#define NUM_OF_ARGS 5 + +struct sprd_pcie { +struct dw_pcie pci; +}; + +static int sprd_pcie_syscon_setting (struct platform_device *pdev,char *env) +{ +struct device_node *np = pdev->dev.of_node; +int i, count, err; +u32 type, delay, reg, mask, val, tmp_val; +struct of_phandle_args out_args; +struct regmap *iomap; +struct device *dev = &pdev->dev; + +if (!of_find_property (np, env,NULL )) { +dev_info (dev,"There isn't property %s in dts\n" , env); +return 0 ; + } + + count =of_property_count_elems_of_size (np, env, + (NUM_OF_ARGS +1 ) *sizeof (u32)); +dev_info (dev,"Property (%s) reg count is %d :\n" , env, count); + +for (i =0 ; i < count; i++) { + err =of_parse_phandle_with_fixed_args (np, env,NUM_OF_ARGS , + i, &out_args); +if (err <0 ) +return err; + + type = out_args.args[0 ]; + delay = out_args.args[1 ]; + reg = out_args.args[2 ]; + mask = out_args.args[3 ]; + val = out_args.args[4 ]; + + iomap =syscon_node_to_regmap (out_args.np); + +switch (type) { +case 0 : +regmap_update_bits (iomap, reg, mask, val); +break ; + +case 1 : +regmap_read (iomap, reg, &tmp_val); + tmp_val &= (~mask); + tmp_val |= (val & mask); +regmap_write (iomap, reg, tmp_val); +break ; +default : +break ; + } + +if (delay) +usleep_range (delay, delay +10 ); + +regmap_read (iomap, reg, &tmp_val); +dev_dbg (dev, +"%2d:reg[0x%8x] mask[0x%8x] val[0x%8x] result[0x%8x]\n" , + i, reg, mask, val, tmp_val); + } +
Quoted 9 lines
drivers/pci/controller/dwc/pcie-sprd.c line 90
Quoted 6 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +6
-0+return i; +} + +static int sprd_pcie_perst_assert (struct platform_device *pdev) +{ +return sprd_pcie_syscon_setting (pdev,"sprd,pcie-perst-assert" );Not documented. This should probably use the reset binding.
Thanks! I'll Document it in the next version.
Quoted 69 lines
Quoted 69 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +69
-0+} + +static int sprd_pcie_perst_deassert (struct platform_device *pdev) +{ +return sprd_pcie_syscon_setting (pdev,"sprd,pcie-perst-deassert" ); +} + +static int sprd_pcie_power_on (struct platform_device *pdev) +{ +int ret; +struct device *dev = &pdev->dev; + + ret =sprd_pcie_syscon_setting (pdev,"sprd,pcie-poweron-syscons" ); +if (ret <0 ) +dev_err (dev, +"failed to set pcie poweroff syscons, return %d\n" , + ret); + +sprd_pcie_perst_deassert (pdev); + +return ret; +} + +static int sprd_pcie_power_off (struct platform_device *pdev) +{ +int ret; +struct device *dev = &pdev->dev; + +sprd_pcie_perst_assert (pdev); + + ret =sprd_pcie_syscon_setting (pdev,"sprd,pcie-poweroff-syscons" ); +if (ret <0 ) +dev_err (dev, +"failed to set pcie poweroff syscons, return %d\n" , + ret); + +return ret; +} + +static int sprd_pcie_host_init (struct pcie_port *pp) +{ +int ret; +struct dw_pcie *pci =to_dw_pcie_from_pp (pp); + +dw_pcie_setup_rc (pp); +dw_pcie_msi_init (pp); + + ret =dw_pcie_wait_for_link (pci); +if (ret) +dev_err (pci->dev,"pcie ep may has not been powered on yet\n" ); + +return ret; +} + +static const struct dw_pcie_host_ops sprd_pcie_host_ops = { + .host_init = sprd_pcie_host_init, +}; + +static int sprd_add_pcie_port (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct sprd_pcie *ctrl =platform_get_drvdata (pdev); +struct dw_pcie *pci = &ctrl->pci; +struct pcie_port *pp = &pci->pp; + + pci->dbi_base =devm_platform_ioremap_resource_byname (pdev,"dbi" ); +if (IS_ERR (pci->dbi_base)) { +dev_err (dev,"failed to get rc dbi base\n" ); +return PTR_ERR (pci->dbi_base);
Quoted 9 lines
drivers/pci/controller/dwc/pcie-sprd.c line 164
Quoted 5 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +5
-0+ } + + pp->ops = &sprd_pcie_host_ops; + +if (IS_ENABLED (CONFIG_PCI_MSI )) {I don't think this check is needed. The DW core won't setup the MSI if not enabled, so doesn't matter if
msi_irqis initialized.
Thanks!
I'll delete it in the next version.
Quoted 7 lines
drivers/pci/controller/dwc/pcie-sprd.c line 168
Quoted 4 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +4
-0+ pp->msi_irq =platform_get_irq_byname (pdev,"msi" ); +if (pp->msi_irq <0 ) { +dev_err (dev,"failed to get msi, return %d\n" , + pp->msi_irq);No need to print an error, the core does this.
Thanks!
I'll delete it in the next version.
Quoted 47 lines
Quoted 47 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +47
-0+return pp->msi_irq; + } + } + +return dw_pcie_host_init (pp); +} + +static int sprd_pcie_probe (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct sprd_pcie *ctrl; +struct dw_pcie *pci; +int ret; + + ctrl =devm_kzalloc (dev,sizeof (*ctrl),GFP_KERNEL ); +if (!ctrl) +return -ENOMEM ; + + pci = &ctrl->pci; + pci->dev = dev; + +platform_set_drvdata (pdev, ctrl); + + ret =sprd_pcie_power_on (pdev); +if (ret <0 ) { +dev_err (dev,"failed to get pcie poweron syscons, return %d\n" , + ret); + goto err_power_off; + } + + ret =sprd_add_pcie_port (pdev); +if (ret) { +dev_warn (dev,"failed to initialize RC controller\n" ); +return ret; + } + +return 0 ; + +err_power_off: +sprd_pcie_power_off (pdev); + +return ret; +} + +static const struct of_device_id sprd_pcie_of_match[] = { + { + .compatible ="sprd,pcie-rc" ,
Quoted 9 lines
drivers/pci/controller/dwc/pcie-sprd.c line 221
Quoted 6 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +6
-0+ }, + {}, +}; + +static struct platform_driver sprd_pcie_driver = { + .probe = sprd_pcie_probe,You need a .remove hook.
Yes! I'll fix it.
Quoted 13 lines
Quoted 13 lines
drivers/pci/controller/dwc/pcie-sprd.c new file +10
-1+ .driver = { + .name = "sprd-pcie", + .of_match_table = sprd_pcie_of_match, + }, +}; + +module_platform_driver (sprd_pcie_driver); + +MODULE_DESCRIPTION ("Unisoc PCIe host controller driver" ); +MODULE_LICENSE ("GPL v2" ); --2.7.4