Add PCI Express to i.MX6
Subsystems (8)
cover Add PCI Express to i.MX6
This patchset adds a PCI Express driver for the Freescale i.MX6 series of SoCs. It is based on the BSP driver, available from Freescale athttp://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git
This patchset does the following:
1) Add general-purpose LVDS clocks to drive the bus
2) Enable PCI Express on ARM
3) Add the pcie-imx driver
4) Add device tree bindings for imx6qdl.dtsi
Sean Cross (4):
ARM i.MX6q: Add descriptors for LVDS clocks
ARM: Enable PCI Express on ARM
PCI: Add driver for i.MX6 PCI Express
ARM i.MX6: Add PCI Express to device tree
.../devicetree/bindings/clock/imx6q-clock.txt | 4 + .../devicetree/bindings/pci/imx6q-pcie.txt | 20 + arch/arm/Kconfig | 2 + arch/arm/boot/dts/imx6qdl.dtsi | 12 + arch/arm/mach-imx/Kconfig | 1 + arch/arm/mach-imx/clk-imx6q.c | 13 +- drivers/pci/pcie/Kconfig | 10 + drivers/pci/pcie/Makefile | 2 + drivers/pci/pcie/pcie-imx.c | 1049 ++++++++++++++++++++ 9 files changed, 1111 insertions(+), 2 deletions(-) create mode 100644 Documentation/devicetree/bindings/pci/imx6q-pcie.txt create mode 100644 drivers/pci/pcie/pcie-imx.c
Signature · 1 line
1.7.9.5
PATCH 1/4 ARM i.MX6q: Add descriptors for LVDS clocks
There are two general-purpose LVDS clocks available on the i.MX6. Add clock descriptors for both of these clocks, as well as selectors to be able to generate various external signals.
--- .../devicetree/bindings/clock/imx6q-clock.txt | 4 ++++ arch/arm/mach-imx/clk-imx6q.c | 13 +++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/clock/imx6q-clock.txt +4 -0
@@ -208,6 +208,10 @@ clocks and IDs. pll4_post_div 193 pll5_post_div 194 pll5_video_div 195 + lvds1_sel 196 + lvds2_sel 197 + lvds1 198 + lvds2 199 Examples:
arch/arm/mach-imx/clk-imx6q.c +11 -2
@@ -204,7 +204,9 @@ static const char *vdo_axi_sels[] = { "axi", "ahb", };static const char *vpu_axi_sels[] = {"axi" ,"pll2_pfd2_396m" ,"pll2_pfd0_352m" , };static const char *cko1_sels[] = {"pll3_usb_otg" ,"pll2_bus" ,"pll1_sys" ,"pll5_video_div" ,"dummy" ,"axi" ,"enfc" ,"ipu1_di0" ,"ipu1_di1" ,"ipu2_di0" , -"ipu2_di1" ,"ahb" ,"ipg" ,"ipg_per" ,"ckil" ,"pll4_post_div" , }; +"ipu2_di1" ,"ahb" ,"ipg" ,"ipg_per" ,"ckil" ,"pll4_audio" , }; +static const char *lvds1_sels[] = {"dummy" ,"dummy" ,"dummy" ,"dummy" ,"dummy" ,"dummy" ,"pll4_audio" ,"pll5_video" ,"pll8_mlb" ,"enet_ref" ,"pcie_ref" ,"sata_ref" ,"dummy" ,"dummy" ,"dummy" ,"dummy" ,"dummy" ,"dummy" ,"dummy" ,"lvds1" ,"lvds2" , }; +static const char *lvds2_sels[] = {"dummy" ,"dummy" ,"dummy" ,"dummy" ,"dummy" ,"dummy" ,"pll4_audio" ,"pll5_video" ,"pll8_mlb" ,"enet_ref" ,"pcie_ref" ,"sata_ref" ,"dummy" ,"dummy" ,"dummy" ,"dummy" ,"dummy" ,"dummy" ,"dummy" ,"lvds1" ,"lvds2" , };enum mx6q_clks { dummy, ckil, ckih, osc, pll2_pfd0_352m, pll2_pfd1_594m, pll2_pfd2_396m,
@@ -238,7 +240,8 @@ enum mx6q_clks { pll4_audio, pll5_video, pll8_mlb, pll7_usb_host, pll6_enet, ssi1_ipg, ssi2_ipg, ssi3_ipg, rom, usbphy1, usbphy2, ldb_di0_div_3_5, ldb_di1_div_3_5, sata_ref, sata_ref_100m, pcie_ref, pcie_ref_125m, enet_ref, usbphy1_gate, - usbphy2_gate, pll4_post_div, pll5_post_div, pll5_video_div, clk_max + usbphy2_gate, pll4_post_div, pll5_post_div, pll5_video_div, + lvds1_sel, lvds2_sel, lvds1, lvds2, clk_max };static struct clk *clk[clk_max];
@@ -360,6 +363,12 @@ int __init mx6q_clocks_init(void) clk[pll5_post_div] =clk_register_divider_table (NULL ,"pll5_post_div" ,"pll5_video" ,CLK_SET_RATE_PARENT , base +0xa0 ,19 ,2 ,0 , post_div_table, &imx_ccm_lock); clk[pll5_video_div] =clk_register_divider_table (NULL ,"pll5_video_div" ,"pll5_post_div" ,CLK_SET_RATE_PARENT , base +0x170 ,30 ,2 ,0 , video_div_table, &imx_ccm_lock); + clk[lvds1_sel] =imx_clk_mux ("lvds1_sel" , base +0x160 ,0 ,5 , lvds1_sels,ARRAY_SIZE (lvds1_sels)); + clk[lvds2_sel] =imx_clk_mux ("lvds2_sel" , base +0x160 ,0 ,5 , lvds2_sels,ARRAY_SIZE (lvds2_sels)); + + clk[lvds1] =imx_clk_gate ("lvds1" ,"dummy" , base +0x160 ,10 ); + clk[lvds2] =imx_clk_gate ("lvds2" ,"dummy" , base +0x160 ,11 ); + np =of_find_compatible_node (NULL ,NULL ,"fsl,imx6q-ccm" ); base =of_iomap (np,0 );WARN_ON (!base);
Signature · 1 line
1.7.9.5
PATCH 2/4 ARM: Enable PCI Express on ARM
In reply to Sean CrossSome ARM devices have PCI Express hardware, and should be able to take advantage of PCI Express code present in the common driver files. Enable PCI Express on ARM, when PCI is available.
--- arch/arm/Kconfig | 2 ++ 1 file changed, 2 insertions(+)
arch/arm/Kconfig +2 -0
@@ -1418,6 +1418,8 @@ config PCI_HOST_ITE8152 source "drivers/pci/Kconfig" +source "drivers/pci/pcie/Kconfig" + source "drivers/pcmcia/Kconfig" endmenu
Signature · 1 line
1.7.9.5
PATCH 2/4 ARM: Enable PCI Express on ARM
Quoted 14 lines · Sean Cross
Some ARM devices have PCI Express hardware, and should be able to take advantage of PCI Express code present in the common driver files. Enable PCI Express on ARM, when PCI is available.
Signed-off-by Sean Cross <[email protected]>--- arch/arm/Kconfig | 2 ++ 1 file changed, 2 insertions(+)arch/arm/Kconfig +0
-0
arch/arm/Kconfig config PCI_HOST_ITE8152 · line 1423
Quoted 6 lines
arch/arm/Kconfig +2
-0@@ -1418,6 +1418,8 @@ config PCI_HOST_ITE8152 source "drivers/pci/Kconfig" +source "drivers/pci/pcie/Kconfig" +
[PATCH V10 2/4] ARM: EXYNOS: Enable PCIe support for Exynos5440 does the same thing.
source "drivers/pcmcia/Kconfig"
Quoted 3 lines
arch/arm/Kconfig +0
-0endmenu
PATCH 3/4 PCI: Add driver for i.MX6 PCI Express
In reply to Sean CrossThis adds a PCI Express port driver for the on-chip PCI Express port present on the i.MX6 SoC. It is based on the PCI Express driver available in the Freescale BSP.
--- .../devicetree/bindings/pci/imx6q-pcie.txt | 20 + arch/arm/mach-imx/Kconfig | 1 + drivers/pci/pcie/Kconfig | 10 + drivers/pci/pcie/Makefile | 2 + drivers/pci/pcie/pcie-imx.c | 1049 ++++++++++++++++++++ 5 files changed, 1082 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/imx6q-pcie.txt create mode 100644 drivers/pci/pcie/pcie-imx.c
Documentation/devicetree/bindings/pci/imx6q-pcie.txt new file +20 -0
@@ -0,0 +1,20 @@ +* Freescale i.MX6Q PCI Express bridge + +Example (i.MX6Q) + pcie: pcie at 01ffc000 { + compatible = "fsl,imx6q-pcie"; + reg = <0x01ffc000 0x4000>, + <0x01000000 0x100000>, + <0x01100000 0xe00000>, + <0x01f00000 0xfc000>; + interrupts = <0 122 0x04>; + clocks = <&clks 186>, <&clks 189>, <&clks 196>, + <&clks 198>, <&clks 144>; + clock-names = "sata_ref", "pcie_ref_125m", "lvds1_sel", + "lvds1", "pcie_axi"; + power-enable = <&gpio7 12 0>; + pcie-reset = <&gpio3 29 0>; + wake-up = <&gpio3 22 0>; + disable-endpoint = <&gpio2 16 0>; + }; +
arch/arm/mach-imx/Kconfig +1 -0
@@ -811,6 +811,7 @@ config SOC_IMX6Q select PL310_ERRATA_588369 if CACHE_PL310 select PL310_ERRATA_727915 if CACHE_PL310 select PL310_ERRATA_769419 if CACHE_PL310 + select MIGHT_HAVE_PCI select PM_OPP if PM help
drivers/pci/pcie/Kconfig +10 -0
@@ -83,3 +83,13 @@ endchoice config PCIE_PME def_bool y depends on PCIEPORTBUS && PM_RUNTIME + +# +# Platform driver for i.MX6 +# +config PCIE_IMX + bool "Support for i.MX6" + depends on SOC_IMX6Q + help + Enable support for the 1x PCI Express bus on the Freescale i.MX6 + depends on PCIEPORTBUS && PM_RUNTIME
drivers/pci/pcie/Makefile +2 -0
@@ -14,3 +14,5 @@ obj-$(CONFIG_PCIEPORTBUS) += pcieportdrv.o obj-$(CONFIG_PCIEAER) += aer/ obj-$(CONFIG_PCIE_PME) += pme.o + +obj-$(CONFIG_PCIE_IMX) += pcie-imx.o
drivers/pci/pcie/pcie-imx.c new file +1049 -0
1,049 lines not on this page · raw message
Signature · 1 line
1.7.9.5
Re: [PATCH 3/4] PCI: Add driver for i.MX6 PCI Express
Quoted 5 lines
This adds a PCI Express port driver for the on-chip PCI Express port present on the i.MX6 SoC. It is based on the PCI Express driver available in the Freescale BSP.
Signed-off-by Sean Cross <[email protected]>
...
Quoted 8 lines
+++
b/Documentation/devicetree/bindings/pci/imx6q-pcie.txtDocumentation/devicetree/bindings/pci/imx6q-pcie.txt new file +6
-0@@ -0,0 +1,20 @@ +* Freescale i.MX6Q PCI Express bridge + +Example (i.MX6Q) + pcie: pcie at 01ffc000 { + compatible = "fsl,imx6q-pcie"; + reg = <0x01ffc000 0x4000>,
Documentation/devicetree/bindings/pci/imx6q-pcie.txt line 16
Quoted 10 lines
Documentation/devicetree/bindings/pci/imx6q-pcie.txt new file +10
-0+ <0x01000000 0x100000>, + <0x01100000 0xe00000>, + <0x01f00000 0xfc000>; + interrupts = <0 122 0x04>; + clocks = <&clks 186>, <&clks 189>, <&clks 196>, + <&clks 198>, <&clks 144>; + clock-names = "sata_ref", "pcie_ref_125m", "lvds1_sel", + "lvds1", "pcie_axi"; + power-enable = <&gpio7 12 0>; + pcie-reset = <&gpio3 29 0>;
Can this be replaced with regulator/reset API?
Documentation/devicetree/bindings/pci/imx6q-pcie.txt line 19
Quoted 3 lines
Documentation/devicetree/bindings/pci/imx6q-pcie.txt new file +3
-0+ wake-up = <&gpio3 22 0>; + disable-endpoint = <&gpio2 16 0>; + };
---
PATCH 3/4 PCI: Add driver for i.MX6 PCI Express
Quoted 20 lines · Alexander Shiyan
Quoted 5 lines
This adds a PCI Express port driver for the on-chip PCI Express port present on the i.MX6 SoC. It is based on the PCI Express driver available in the Freescale BSP.
Signed-off-by Sean Cross <xobs at kosagi.com (mailto:[email protected])>...
Quoted 14 lines
+++
b/Documentation/devicetree/bindings/pci/imx6q-pcie.txtDocumentation/devicetree/bindings/pci/imx6q-pcie.txt new file +12
-0@@ -0,0 +1,20 @@ +* Freescale i.MX6Q PCI Express bridge + +Example (i.MX6Q) + pcie: pcie at 01ffc000 { + compatible = "fsl,imx6q-pcie"; + reg = <0x01ffc000 0x4000>, + <0x01000000 0x100000>, + <0x01100000 0xe00000>, + <0x01f00000 0xfc000>; + interrupts = <0 122 0x04>; + clocks = <&clks 186>, <&clks 189>, <&clks 196>, + <&clks 198>, <&clks 144>;
Quoted 8 lines
Documentation/devicetree/bindings/pci/imx6q-pcie.txt line 16
Quoted 4 lines
Documentation/devicetree/bindings/pci/imx6q-pcie.txt new file +4
-0+ clock-names = "sata_ref", "pcie_ref_125m", "lvds1_sel", + "lvds1", "pcie_axi"; + power-enable = <&gpio7 12 0>; + pcie-reset = <&gpio3 29 0>;Can this be replaced with regulator/reset API?
I agree that power-enable might be better replaced with regulator API calls, and I'll rework this patch to incorporate a regulator.
I hadn't heard of the reset API before, and I note in the documentation that it says:
Reset signals for whole standalone chips are most likely better represented as GPIOs, although there are likely to be exceptions to this rule.
This is a reset line going directly to a PCIe slot, which I think counts as a "standalone chip".
Quoted 7 lines
Documentation/devicetree/bindings/pci/imx6q-pcie.txt line 19
Quoted 3 lines
Documentation/devicetree/bindings/pci/imx6q-pcie.txt new file +3
-0+ wake-up = <&gpio3 22 0>; + disable-endpoint = <&gpio2 16 0>; + };---
PATCH 3/4 PCI: Add driver for i.MX6 PCI Express
In reply to Sean CrossQuoted 6 lines · Sean Cross
This adds a PCI Express port driver for the on-chip PCI Express port present on the i.MX6 SoC. It is based on the PCI Express driver available in the Freescale BSP.
Signed-off-by Sean Cross <[email protected]>---
...
drivers/pci/pcie/pcie-imx.c
Quoted 5 lines
drivers/pci/pcie/pcie-imx.c new file +0
-0
Should go to drivers/pci/host/
Quoted 4 lines
drivers/pci/pcie/pcie-imx.c new file +3
-0@@ -0,0 +1,1049 @@ +/* +* drivers/pci/pcie/pcie-imx.c +*
...
drivers/pci/pcie/pcie-imx.c line 93
Quoted 9 lines
drivers/pci/pcie/pcie-imx.c new file +9
-0+#define ATU_R_BaseAddress 0x900 +#define PCIE_PL_iATUVR (ATU_R_BaseAddress + 0x0) +#define PCIE_PL_iATURC1 (ATU_R_BaseAddress + 0x4) +#define PCIE_PL_iATURC2 (ATU_R_BaseAddress + 0x8) +#define PCIE_PL_iATURLBA (ATU_R_BaseAddress + 0xC) +#define PCIE_PL_iATURUBA (ATU_R_BaseAddress + 0x10) +#define PCIE_PL_iATURLA (ATU_R_BaseAddress + 0x14) +#define PCIE_PL_iATURLTA (ATU_R_BaseAddress + 0x18) +#define PCIE_PL_iATURUTA (ATU_R_BaseAddress + 0x1C)
I may be wrong, but from these offset it seems to me that it is SNPS controller. If yes, then please go through comments of "[PATCH V1-10 0/4] PCIe support for Samsung Exynos5440 SoC"
Regards
Pratyush
PATCH 3/4 PCI: Add driver for i.MX6 PCI Express
Quoted 12 lines · Pratyush Anand
Quoted 6 lines · Sean Cross
This adds a PCI Express port driver for the on-chip PCI Express port present on the i.MX6 SoC. It is based on the PCI Express driver available in the Freescale BSP.
Signed-off-by Sean Cross <xobs at kosagi.com (mailto:[email protected])>---
...
Quoted 9 lines
drivers/pci/pcie/pcie-imx.c
Quoted 5 lines
drivers/pci/pcie/pcie-imx.c new file +0
-0Should go to
drivers/pci/host/
I'll pull from arm-soc rather than from Linus' tree and place the driver there instead.
Quoted 10 lines
Quoted 4 lines
drivers/pci/pcie/pcie-imx.c new file +3
-0@@ -0,0 +1,1049 @@ +/* +* drivers/pci/pcie/pcie-imx.c +* ...
Quoted 15 lines
drivers/pci/pcie/pcie-imx.c line 93
Quoted 9 lines
drivers/pci/pcie/pcie-imx.c new file +9
-0+#define ATU_R_BaseAddress 0x900 +#define PCIE_PL_iATUVR (ATU_R_BaseAddress + 0x0) +#define PCIE_PL_iATURC1 (ATU_R_BaseAddress + 0x4) +#define PCIE_PL_iATURC2 (ATU_R_BaseAddress + 0x8) +#define PCIE_PL_iATURLBA (ATU_R_BaseAddress + 0xC) +#define PCIE_PL_iATURUBA (ATU_R_BaseAddress + 0x10) +#define PCIE_PL_iATURLA (ATU_R_BaseAddress + 0x14) +#define PCIE_PL_iATURLTA (ATU_R_BaseAddress + 0x18) +#define PCIE_PL_iATURUTA (ATU_R_BaseAddress + 0x1C)I may be wrong, but from these offset it seems to me that it is SNPS controller. If yes, then please go through comments of "[PATCH V1-10 0/4] PCIe support for Samsung Exynos5440 SoC"
Exynos5440 appears to use the same port logic controller. However, the PHYs are different. I'm not exactly certain which comments you want me to notice in that series of patchsets, but I see references to splitting Exynos-specific code into its own project. Based on that, it seems like the best approach would be to:
1) Move Exynos code into its own file, say, pcie-exynos.c. This would leave only Synopsys-specific ATC code in pcie-designware.c
2) Rename generic exynos functions to reflect the fact that they're designware-generic functions.
3) Have pcie-imx.c reference this generic designware ATC code.
I'll rework the patch and re-submit it following these three changes.
--
Sean Cross
PATCH 3/4 PCI: Add driver for i.MX6 PCI Express
Quoted 10 lines · Sean Cross
Quoted 3 lines
Quoted 3 lines
I may be wrong, but from these offset it seems to me that it is SNPS controller. If yes, then please go through comments of "[PATCH V1-10 0/4] PCIe support for Samsung Exynos5440 SoC"
Exynos5440 appears to use the same port logic controller. However, the PHYs are different. I'm not exactly certain which comments you want me to notice in that series of patchsets, but I see references to splitting Exynos-specific code into its own project. Based on that, it seems like the best approach would be to:
1) Move Exynos code into its own file, say,
pcie-exynos.c. This would leave only Synopsys-specific ATC code inpcie-designware.c
2) Rename generic exynos functions to reflect the fact that they're designware-generic functions.
3) Havepcie-imx.creference this generic designware ATC code.I'll rework the patch and re-submit it following these three changes.
Correct, Exactly these steps has to be done. But, Mohit might be doing similar work, so it would be better to get consensus on what has to be done.
IMO, there should be three categories of functions. May be arnd can comment if we can do even something better.
Group I: Initially, These functions should remain in pcie-designware.c (offcourse by changing exynos tag to dw). Latter on, we can see if some of them can even be moved to common pci layer.
sys_to_pciecfg_readcfg_writedw_pcie_prog_viewport_cfg0dw_pcie_prog_viewport_cfg1dw_pcie_prog_viewport_mem_outbounddw_pcie_prog_viewport_io_outbounddw_pcie_rd_other_confdw_pcie_wr_other_confdw_pcie_setupdw_pcie_valid_configdw_pcie_rd_confdw_pcie_wd_confdw_pcie_scan_busdw_pcie_map_irqdw_pcie_setup_rcadd_pcie_port (after a bit of generalization)dw_pcie_probedw_pcie_remove
Group II: These functions should still remain as dummy in pcie-designware.c , and should be classified as __week. So, each implementer of designware controller say Exynos/SPEAr/imx will have to define their own function to super-seed default dummy definitions.
dw_readl_rcdw_writel_rcdw_pcie_rd_own_confdw_pcie_wr_own_confdw_pcie_link_updw_pcie_host_init (will contain all platform specific and phy initialization)
Group III: Functions specific to Exynos, which should move to pcie-exynos.c
exynos_pcie_sideband_dbi_w_modeexynos_pcie_sideband_dbi_r_modeexynos_pcie_assert_core_resetexynos_pcie_deassert_core_resetexynos_pcie_assert_phy_resetexynos_pcie_deassert_phy_resetexynos_pcie_init_phyexynos_pcie_assert_resetexynos_pcie_establish_link
@Mohit, See if you have BW then please take it further.
arnd, are exynos patches applied to any branch of arm-soc git tree?
Regards
Pratyush
PATCH 3/4 PCI: Add driver for i.MX6 PCI Express
Quoted 8 lines · On Tuesday 02 July 2013 10:11:55 Pratyush Anand wrote:
Quoted 7 lines · Sean Cross
Quoted 3 lines
Quoted 3 lines
I may be wrong, but from these offset it seems to me that it is SNPS controller. If yes, then please go through comments of "[PATCH V1-10 0/4] PCIe support for Samsung Exynos5440 SoC"
Exynos5440 appears to use the same port logic controller. However, the PHYs are different. I'm not exactly certain which comments you want me to notice in that series of patchsets, but I see references to splitting Exynos-specific code into its own project. Based on that, it seems like the best approach would be to:
1) Move Exynos code into its own file, say,
pcie-exynos.c. This would leave only Synopsys-specific ATC code inpcie-designware.c
2) Rename generic exynos functions to reflect the fact that they're designware-generic functions.
Ouch, I missed the fact that Jingoo Han had only renamed the file, but not also all the function identifiers. This change needs to be done anyway.
Quoted 6 lines
Quoted 3 lines
3) Have
pcie-imx.creference this generic designware ATC code.I'll rework the patch and re-submit it following these three changes.
Correct, Exactly these steps has to be done. But, Mohit might be doing similar work, so it would be better to get consensus on what has to be done.
If the platform specific parts are small enough, you can actually just leave everything in one file, and use the of_device_id data field to point to a structure describing the differences, e.g. using function pointers.
Quoted 2 lines
IMO, there should be three categories of functions. May be arnd can comment if we can do even something better.
As a general comment in case you are wondering: In a situation like this, the common code should always be structured as a "library" of functions and the hardware specific drivers become the actual driver that register to a particular device 'compatible' string in DT.
Quoted 15 lines
Group I: Initially, These functions should remain in
pcie-designware.c(offcourse by changing exynos tag to dw). Latter on, we can see if some of them can even be moved to common pci layer.
sys_to_pciecfg_readcfg_writedw_pcie_prog_viewport_cfg0dw_pcie_prog_viewport_cfg1dw_pcie_prog_viewport_mem_outbounddw_pcie_prog_viewport_io_outbounddw_pcie_rd_other_confdw_pcie_wr_other_confdw_pcie_setupdw_pcie_valid_config
Quoted 8 lines
dw_pcie_rd_confdw_pcie_wd_confdw_pcie_scan_busdw_pcie_map_irqdw_pcie_setup_rcadd_pcie_port(after a bit of generalization)dw_pcie_probedw_pcie_remove
The probe and remove functions might need be split up into a generic part that gets called by the hardware specific part.
Quoted 5 lines
Group II: These functions should still remain as dummy in
pcie-designware.c, and should be classified as__week. So, each implementer of designware controller say Exynos/SPEAr/imx will have to define their own function to super-seed default dummy definitions.
Quoted 7 lines
dw_readl_rcdw_writel_rcdw_pcie_rd_own_confdw_pcie_wr_own_confdw_pcie_link_updw_pcie_host_init(will contain all platform specific and phy initialization)
I don't like __weak functions really, and they don't work well with loadable modules the way I suggested above. Instead, you probably need a structure with function pointers that gets initialized by platform specific driver.
struct dw_pcie_host_ops {
void (*readl_rc)(struct pcie_port *pp, void *dbi_base, u32 *val);
...
};static inline void dw_pcie_readl_rc(struct pcie_port *pp, void *dbi_base, u32 *val);
{
if (pp->ops->readl_rc)
pp->ops->readl_rc(pp, dbi_base, val);
else
*val = readl(dbi_base);
}dw_pcie_host_init would become the probe function that calls the generic dw_pcie_probe.
Quoted 13 lines
Group III: Functions specific to Exynos, which should move to
pcie-exynos.c
exynos_pcie_sideband_dbi_w_modeexynos_pcie_sideband_dbi_r_modeexynos_pcie_assert_core_resetexynos_pcie_deassert_core_resetexynos_pcie_assert_phy_resetexynos_pcie_deassert_phy_resetexynos_pcie_init_phyexynos_pcie_assert_resetexynos_pcie_establish_link
Quoted 3 lines
@Mohit, See if you have BW then please take it further.
arnd, are exynos patches applied to any branch of arm-soc git tree?
Yes, they are currently in the next/soc branch.
Arnd
PATCH 3/4 PCI: Add driver for i.MX6 PCI Express
In reply to Sean CrossHi Cross:
First of all, Thanks for your efforts to summit the imx pcie support patches.
I looked through your patch-set, and have some concerns:
* PCIe driver used in the FSL linux-2.6.35 BSP, is not good enough,
Based on the latest FSL BSP.
* The PCIe switch/MSI features had been supported.
* In order to support the 8MB memory space size, the mem layout had been updated as listed below, otherwise, the memory space size required by PCIe EP device wouldn't larger than 4MB. /*
* i.MX6 defines 16MB in the AXI address map for PCIe.
*
* That address space excepted the pcie registers is
* split and defined into different regions by iATU,
* with sizes and offsets as follows:
*
* RC:
* 0x0100_0000 --- 0x01DF_FFFF 14MB IORESOURCE_MEM
* 0x01E0_0000 --- 0x01EF_FFFF 1MB IORESOURCE_IO
* 0x01F0_0000 --- 0x01FF_FFFF 1MB Cfg + MSI + Registers
*
* EP (default value):
* 0x0100_0000 --- 0x01FF_C000 16MB - 16KB IORESOURCE_MEM
*/
* BTW, the PCIe EP mode had been verified too.
Best Regards
Richard Zhu
-----Original Message-----
From: [email protected] [mailto:linux-pci-owner at vger.kernel.org] On Behalf Of Sean Cross
Sent: Tuesday, July 02, 2013 11:46 AM
To: Pratyush Anand
Cc: devicetree-discuss at lists.ozlabs.org; linux-pci at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Arnd Bergmann; Mohit KUMAR
Subject: Re: [PATCH 3/4] PCI: Add driver for i.MX6 PCI Express
Quoted 12 lines · Pratyush Anand
Quoted 6 lines · Sean Cross
This adds a PCI Express port driver for the on-chip PCI Express port present on the i.MX6 SoC. It is based on the PCI Express driver available in the Freescale BSP.
Signed-off-by Sean Cross <xobs at kosagi.com (mailto:[email protected])>---
...
Quoted 9 lines
drivers/pci/pcie/pcie-imx.c
Quoted 5 lines
(quoted diff) +0
-0
b/drivers/pci/pcie/pcie-imx.cnew file mode 100644 index 0000000..664679edrivers/pci/pcie/pcie-imx.c new file +0
-0Should go to
drivers/pci/host/
I'll pull from arm-soc rather than from Linus' tree and place the driver there instead.
Quoted 10 lines
Quoted 4 lines
drivers/pci/pcie/pcie-imx.c new file +3
-0@@ -0,0 +1,1049 @@ +/* +* drivers/pci/pcie/pcie-imx.c +* ...
Quoted 15 lines
drivers/pci/pcie/pcie-imx.c
Quoted 9 lines
drivers/pci/pcie/pcie-imx.c new file +9
-0+#define ATU_R_BaseAddress 0x900 +#define PCIE_PL_iATUVR (ATU_R_BaseAddress + 0x0) #define +PCIE_PL_iATURC1 (ATU_R_BaseAddress + 0x4) #define PCIE_PL_iATURC2 +(ATU_R_BaseAddress + 0x8) #define PCIE_PL_iATURLBA +(ATU_R_BaseAddress + 0xC) #define PCIE_PL_iATURUBA +(ATU_R_BaseAddress + 0x10) #define PCIE_PL_iATURLA +(ATU_R_BaseAddress + 0x14) #define PCIE_PL_iATURLTA +(ATU_R_BaseAddress + 0x18) #define PCIE_PL_iATURUTA +(ATU_R_BaseAddress +0x1C )I may be wrong, but from these offset it seems to me that it is SNPS controller. If yes, then please go through comments of "[PATCH V1-10 0/4] PCIe support for Samsung Exynos5440 SoC"
Exynos5440 appears to use the same port logic controller. However, the PHYs are different. I'm not exactly certain which comments you want me to notice in that series of patchsets, but I see references to splitting Exynos-specific code into its own project. Based on that, it seems like the best approach would be to:
1) Move Exynos code into its own file, say, pcie-exynos.c. This would leave only Synopsys-specific ATC code in pcie-designware.c
2) Rename generic exynos functions to reflect the fact that they're designware-generic functions.
3) Have pcie-imx.c reference this generic designware ATC code.
I'll rework the patch and re-submit it following these three changes.
--
Sean Cross
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Sascha Hauer
maintainerPATCH 3/4 PCI: Add driver for i.MX6 PCI Express
In reply to Sean CrossQuoted 140 lines · Sean Cross
This adds a PCI Express port driver for the on-chip PCI Express port present on the i.MX6 SoC. It is based on the PCI Express driver available in the Freescale BSP.
Signed-off-by Sean Cross <[email protected]>--- .../devicetree/bindings/pci/imx6q-pcie.txt | 20 + arch/arm/mach-imx/Kconfig | 1 + drivers/pci/pcie/Kconfig | 10 + drivers/pci/pcie/Makefile | 2 + drivers/pci/pcie/pcie-imx.c | 1049 ++++++++++++++++++++ 5 files changed, 1082 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/imx6q-pcie.txt create mode 100644 drivers/pci/pcie/pcie-imx.cDocumentation/devicetree/bindings/pci/imx6q-pcie.txt new file +20
-0@@ -0,0 +1,20 @@ +* Freescale i.MX6Q PCI Express bridge + +Example (i.MX6Q) + pcie: pcie at 01ffc000 { + compatible = "fsl,imx6q-pcie"; + reg = <0x01ffc000 0x4000>, + <0x01000000 0x100000>, + <0x01100000 0xe00000>, + <0x01f00000 0xfc000>; + interrupts = <0 122 0x04>; + clocks = <&clks 186>, <&clks 189>, <&clks 196>, + <&clks 198>, <&clks 144>; + clock-names = "sata_ref", "pcie_ref_125m", "lvds1_sel", + "lvds1", "pcie_axi"; + power-enable = <&gpio7 12 0>; + pcie-reset = <&gpio3 29 0>; + wake-up = <&gpio3 22 0>; + disable-endpoint = <&gpio2 16 0>; + }; +arch/arm/mach-imx/Kconfig +1
-0@@ -811,6 +811,7 @@ config SOC_IMX6Q select PL310_ERRATA_588369 if CACHE_PL310 select PL310_ERRATA_727915 if CACHE_PL310 select PL310_ERRATA_769419 if CACHE_PL310 + select MIGHT_HAVE_PCI select PM_OPP if PM helpdrivers/pci/pcie/Kconfig +10
-0@@ -83,3 +83,13 @@ endchoice config PCIE_PME def_bool y depends on PCIEPORTBUS && PM_RUNTIME + +# +# Platform driver for i.MX6 +# +config PCIE_IMX + bool "Support for i.MX6" + depends on SOC_IMX6Q + help + Enable support for the 1x PCI Express bus on the Freescale i.MX6 + depends on PCIEPORTBUS && PM_RUNTIMEdrivers/pci/pcie/Makefile +2
-0@@ -14,3 +14,5 @@ obj-$(CONFIG_PCIEPORTBUS) += pcieportdrv.o obj-$(CONFIG_PCIEAER) += aer/ obj-$(CONFIG_PCIE_PME) += pme.o + +obj-$(CONFIG_PCIE_IMX) += pcie-imx.odrivers/pci/pcie/pcie-imx.c new file +53
-0@@ -0,0 +1,1049 @@ +/* +* drivers/pci/pcie/pcie-imx.c +* +* PCIe host controller driver for IMX6 SOCs +* +* Copyright (C) 2012 Freescale Semiconductor, Inc. All Rights Reserved. +* +* Code originally taken from Freescale linux-2.6.35 BSP. +* +* Other bits taken from arch/arm/mach-dove/pcie.c +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. + +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. + +* You should have received a copy of the GNU General Public License along +* with this program; if not, write to the Free Software Foundation, Inc., +* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +*/ + +#include <linux/kernel.h> +#include <linux/pci.h> +#include <linux/interrupt.h> +#include <linux/module.h> +#include <linux/irq.h> +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/gpio.h> +#include <linux/platform_device.h> +#include <linux/types.h> +#include <linux/of.h> +#include <linux/of_gpio.h> +#include <linux/mfd/syscon.h> +#include <linux/of_device.h> +#include <linux/clk-provider.h> +#include <linux/regmap.h> +#include <linux/rfkill.h> + +#include <asm/sizes.h> +#include <asm/io.h> + + +/* IOMUXC */ +#define IOMUXC_GPR0 (0x00) +#define IOMUXC_GPR1 (0x04) +#define IOMUXC_GPR2 (0x08) +#define IOMUXC_GPR3 (0x0C)
drivers/pci/pcie/pcie-imx.c line 63
Quoted 10 lines
drivers/pci/pcie/pcie-imx.c new file +10
-0+#define IOMUXC_GPR4 (0x10) +#define IOMUXC_GPR5 (0x14) +#define IOMUXC_GPR6 (0x18) +#define IOMUXC_GPR7 (0x1C) +#define IOMUXC_GPR8 (0x20) +#define IOMUXC_GPR9 (0x24) +#define IOMUXC_GPR10 (0x28) +#define IOMUXC_GPR11 (0x2C) +#define IOMUXC_GPR12 (0x30) +#define IOMUXC_GPR13 (0x34)
These are already defined in include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
Quoted 20 lines
drivers/pci/pcie/pcie-imx.c new file +20
-0+ + +/* Register Definitions */ +#define PRT_LOG_R_BaseAddress 0x700 + +/* Register DEBUG_R0 */ +/* Debug Register 0 */ +#define DEBUG_R0 (PRT_LOG_R_BaseAddress + 0x28) +#define DEBUG_R0_RegisterSize 32 +#define DEBUG_R0_RegisterResetValue 0x0 +#define DEBUG_R0_RegisterResetMask 0xFFFFFFFF +/* End of Register Definition for DEBUG_R0 */ + +/* Register DEBUG_R1 */ +/* Debug Register 1 */ +#define DEBUG_R1 (PRT_LOG_R_BaseAddress + 0x2c) +#define DEBUG_R1_RegisterSize 32 +#define DEBUG_R1_RegisterResetValue 0x0 +#define DEBUG_R1_RegisterResetMask 0xFFFFFFFF +/* End of Register Definition for DEBUG_R1 */
drivers/pci/pcie/pcie-imx.c line 93
Quoted 10 lines
drivers/pci/pcie/pcie-imx.c new file +10
-0+ +#define ATU_R_BaseAddress 0x900 +#define PCIE_PL_iATUVR (ATU_R_BaseAddress + 0x0) +#define PCIE_PL_iATURC1 (ATU_R_BaseAddress + 0x4) +#define PCIE_PL_iATURC2 (ATU_R_BaseAddress + 0x8) +#define PCIE_PL_iATURLBA (ATU_R_BaseAddress + 0xC) +#define PCIE_PL_iATURUBA (ATU_R_BaseAddress + 0x10) +#define PCIE_PL_iATURLA (ATU_R_BaseAddress + 0x14) +#define PCIE_PL_iATURLTA (ATU_R_BaseAddress + 0x18) +#define PCIE_PL_iATURUTA (ATU_R_BaseAddress + 0x1C)
PleaseGetRidOfThisCamelCase.
drivers/pci/pcie/pcie-imx.c line 98
Quoted 5 lines
drivers/pci/pcie/pcie-imx.c new file +5
-0+ +/* GPR1: iomuxc_gpr1_pcie_ref_clk_en(iomuxc_gpr1[16]) */ +#define iomuxc_gpr1_pcie_ref_clk_en (1 << 16) +/* GPR1: iomuxc_gpr1_test_powerdown(iomuxc_gpr1_18) */ +#define iomuxc_gpr1_test_powerdown (1 << 18)
Defines should use uppercase letters. Besides, we already have these in include/linux/mfd/syscon/imx6q-iomuxc-gpr.h. If there's something missing add them there not in your driver.
Quoted 29 lines
drivers/pci/pcie/pcie-imx.c new file +29
-0+ +/* GPR12: iomuxc_gpr12_los_level(iomuxc_gpr12[8:4]) */ +#define iomuxc_gpr12_los_level (0x1F << 4) +/* GPR12: iomuxc_gpr12_app_ltssm_enable(iomuxc_gpr12[10]) */ +#define iomuxc_gpr12_app_ltssm_enable (1 << 10) +/* GPR12: iomuxc_gpr12_device_type(iomuxc_gpr12[15:12]) */ +#define iomuxc_gpr12_device_type (0xF << 12) + +/* GPR8: iomuxc_gpr8_tx_deemph_gen1(iomuxc_gpr8[5:0]) */ +#define iomuxc_gpr8_tx_deemph_gen1 (0x3F << 0) +/* GPR8: iomuxc_gpr8_tx_deemph_gen2_3p5db(iomuxc_gpr8[11:6]) */ +#define iomuxc_gpr8_tx_deemph_gen2_3p5db (0x3F << 6) +/* GPR8: iomuxc_gpr8_tx_deemph_gen2_6db(iomuxc_gpr8[17:12]) */ +#define iomuxc_gpr8_tx_deemph_gen2_6db (0x3F << 12) +/* GPR8: iomuxc_gpr8_tx_swing_full(iomuxc_gpr8[24:18]) */ +#define iomuxc_gpr8_tx_swing_full (0x7F << 18) +/* GPR8: iomuxc_gpr8_tx_swing_low(iomuxc_gpr8[31:25]) */ +#define iomuxc_gpr8_tx_swing_low (0x7F << 25) + +/* Registers of PHY */ +/* Register PHY_STS_R */ +/* PHY Status Register */ +#define PHY_STS_R (PRT_LOG_R_BaseAddress + 0x110) + +/* Register PHY_CTRL_R */ +/* PHY Control Register */ +#define PHY_CTRL_R (PRT_LOG_R_BaseAddress + 0x114) + +#define SSP_CR_SUP_DIG_MPLL_OVRD_IN_LO 0x0011
drivers/pci/pcie/pcie-imx.c line 137
Quoted 10 lines
drivers/pci/pcie/pcie-imx.c new file +10
-0+/* FIELD: RES_ACK_IN_OVRD [15:15] +// FIELD: RES_ACK_IN [14:14] +// FIELD: RES_REQ_IN_OVRD [13:13] +// FIELD: RES_REQ_IN [12:12] +// FIELD: RTUNE_REQ_OVRD [11:11] +// FIELD: RTUNE_REQ [10:10] +// FIELD: MPLL_MULTIPLIER_OVRD [9:9] +// FIELD: MPLL_MULTIPLIER [8:2] +// FIELD: MPLL_EN_OVRD [1:1] +// FIELD: MPLL_EN [0:0]
If you would use defines instead of comments these would actually be useful.
Quoted 81 lines
drivers/pci/pcie/pcie-imx.c new file +81
-0+*/ + +#define SSP_CR_SUP_DIG_ATEOVRD 0x0010 +/* FIELD: ateovrd_en [2:2] +// FIELD: ref_usb2_en [1:1] +// FIELD: ref_clkdiv2 [0:0] +*/ + +#define SSP_CR_LANE0_DIG_RX_OVRD_IN_LO 0x1005 +/* FIELD: RX_LOS_EN_OVRD [13:13] +// FIELD: RX_LOS_EN [12:12] +// FIELD: RX_TERM_EN_OVRD [11:11] +// FIELD: RX_TERM_EN [10:10] +// FIELD: RX_BIT_SHIFT_OVRD [9:9] +// FIELD: RX_BIT_SHIFT [8:8] +// FIELD: RX_ALIGN_EN_OVRD [7:7] +// FIELD: RX_ALIGN_EN [6:6] +// FIELD: RX_DATA_EN_OVRD [5:5] +// FIELD: RX_DATA_EN [4:4] +// FIELD: RX_PLL_EN_OVRD [3:3] +// FIELD: RX_PLL_EN [2:2] +// FIELD: RX_INVERT_OVRD [1:1] +// FIELD: RX_INVERT [0:0] +*/ + +#define SSP_CR_LANE0_DIG_RX_ASIC_OUT 0x100D +/* FIELD: LOS [2:2] +// FIELD: PLL_STATE [1:1] +// FIELD: VALID [0:0] +*/ + +/* control bus bit definition */ +#define PCIE_CR_CTL_DATA_LOC 0 +#define PCIE_CR_CTL_CAP_ADR_LOC 16 +#define PCIE_CR_CTL_CAP_DAT_LOC 17 +#define PCIE_CR_CTL_WR_LOC 18 +#define PCIE_CR_CTL_RD_LOC 19 +#define PCIE_CR_STAT_DATA_LOC 0 +#define PCIE_CR_STAT_ACK_LOC 16 + +/* End of Register Definitions */ + +#define PCIE_CONF_BUS (b) (((b) & 0xFF) << 16) +#define PCIE_CONF_DEV (d) (((d) & 0x1F) << 11) +#define PCIE_CONF_FUNC (f) (((f) & 0x7) << 8) +#define PCIE_CONF_REG (r) ((r) & ~0x3) + + +/* Taken from PCI specs */ +enum { + MemRdWr =0 , + MemRdLk =1 , + IORdWr =2 , + CfgRdWr0 =4 , + CfgRdWr1 =5 +}; + + +struct imx_pcie_port { +struct device *dev; +u8 index; +u8 root_bus_nr; +int interrupt; + +struct resource *dbi; +struct resource *io; +struct resource *mem; +struct resource *root; + +struct regmap *iomuxc_gpr; + +void __iomem *root_base; +void __iomem *dbi_base; +void __iomem *io_base; +void __iomem *mem_base; +spinlock_t conf_lock; + +char io_space_name[16 ]; +char mem_space_name[16 ]; + +struct list_head next;
drivers/pci/pcie/pcie-imx.c line 228
Quoted 10 lines
drivers/pci/pcie/pcie-imx.c new file +10
-0+ +struct clk *lvds1_sel; +struct clk *lvds1; +struct clk *pcie_ref_125m; +struct clk *pcie_axi; +struct clk *sata_ref; + +unsigned int pcie_pwr_en; +unsigned int pcie_rst; +unsigned int pcie_wake_up;
Whitespace damage here.
drivers/pci/pcie/pcie-imx.c line 236
Quoted 8 lines
drivers/pci/pcie/pcie-imx.c new file +8
-0+ +struct rfkill *rfkill; +}; + +static const struct of_device_id pcie_of_match[] = { + { + .compatible ="fsl,imx6q-pcie" , + .data =NULL ,
No need to set .data to NULL.
Quoted 12 lines
drivers/pci/pcie/pcie-imx.c new file +12
-0+ }, + {}, +}; +MODULE_DEVICE_TABLE (of, pcie_of_match); + +static struct list_head pcie_port_list; +static struct hw_pci imx_pcie; + +static int pcie_phy_cr_read (void __iomem *dbi_base,int addr,int *data); +static int pcie_phy_cr_write (void __iomem *dbi_base,int addr,int data); + +
drivers/pci/pcie/pcie-imx.c line 258
Quoted 10 lines
drivers/pci/pcie/pcie-imx.c new file +10
-0+/* IMX PCIE GPR configure routines */ +static void imx_pcie_clrset (struct imx_pcie_port *pp, +u32 mask,u32 val,u32 reg) +{ +u32 tmp; +regmap_read (pp->iomuxc_gpr, reg, &tmp); + tmp &= ~mask; + tmp |= (val & mask); +regmap_write (pp->iomuxc_gpr, reg, tmp); +}
This duplicates regmap_update_bits without the locking.
Quoted 31 lines
drivers/pci/pcie/pcie-imx.c new file +31
-0+ +static void change_field (int *in,int start,int end,int val) +{ +int mask; + mask = ((0xFFFFFFFF << start) ^ (0xFFFFFFFF << (end +1 ))) &0xFFFFFFFF ; + *in = (*in & ~mask) | (val << start); +} + + +static struct imx_pcie_port *controller_to_port (int index) +{ +struct imx_pcie_port *pp; + +if (index >= imx_pcie.nr_controllers) { +pr_err ("%d exceeded number of controllers %d\n" , + index, imx_pcie.nr_controllers); +return NULL ; + } + +list_for_each_entry (pp, &pcie_port_list, next) { +if (pp->index == index) +return pp; + } +return NULL ; +} + +static struct imx_pcie_port *bus_to_port (int bus) +{ +int i; +int rbus; +struct imx_pcie_port *pp;
drivers/pci/pcie/pcie-imx.c line 299
Quoted 10 lines
drivers/pci/pcie/pcie-imx.c new file +10
-0+ +for (i = imx_pcie.nr_controllers -1 ; i >=0 ; i--) { + pp =controller_to_port (i); + rbus = pp->root_bus_nr; +if (rbus !=-1 && rbus <= bus) +break ; + } + +return i >=0 ? pp :NULL ; +}
There's certainly something wrong here if you have to iterate over lists to find your own private data. struct pci_sys_data has a private_data field exactly for this purpose.
Quoted 58 lines
drivers/pci/pcie/pcie-imx.c new file +58
-0+ +static int __init imx_pcie_setup (int nr,struct pci_sys_data *sys) +{ +struct imx_pcie_port *pp; +int ret; + + pp =controller_to_port (nr); +if (!pp) { +pr_err ("unable to find port %d\n" , nr); +return 0 ; + } + + pp->root_bus_nr = sys->busnr; + +/* +* IORESOURCE_MEM +*/ +snprintf (pp->mem_space_name,sizeof (pp->mem_space_name), +"PCIe %d MEM" , pp->index); + + pp->mem_space_name[sizeof (pp->mem_space_name) -1 ] =0 ; + pp->mem->name = pp->mem_space_name; + pp->mem->flags =IORESOURCE_MEM ; + ret =request_resource (&iomem_resource, pp->mem); +if (ret) +dev_err (pp->dev,"Request PCIe Memory resource failed\n" ); +pci_add_resource_offset (&sys->resources, pp->mem, sys->mem_offset); + + +snprintf (pp->io_space_name,sizeof (pp->io_space_name), +"PCIe %d I/O" , pp->index); + pp->io_space_name[sizeof (pp->io_space_name) -1 ] =0 ; + pp->io->name = pp->io_space_name; + pp->io->flags =IORESOURCE_IO ; + + ret =request_resource (&iomem_resource, pp->io); +if (ret) +dev_err (pp->dev,"Request PCIe IO resource failed\n" ); +pci_add_resource_offset (&sys->resources, pp->io, sys->io_offset); + +/* +* IORESOURCE_IO +*/ + ret =pci_ioremap_io (PCIBIOS_MIN_IO , pp->io->start); +if (ret) +dev_err (pp->dev,"Request PCIe IO resource failed\n" ); + +return 1 ; +} + +static int imx_pcie_link_up (struct platform_device *pdev) +{ +struct imx_pcie_port *pp =platform_get_drvdata (pdev); +int iterations =200 ; +u32 rc, ltssm, rx_valid, temp; + + rc =0 ; +for (iterations =200 ; iterations >0 && !rc; iterations--) {
drivers/pci/pcie/pcie-imx.c line 367
Quoted 10 lines
drivers/pci/pcie/pcie-imx.c new file +10
-0+/* link is debug bit 36, debug register 1 starts at bit 32 */ + rc =readl (pp->dbi_base +DEBUG_R1 ) & (0x1 << (36 -32 )) ; +usleep_range (2000 ,3000 ); + +/* From L0, initiate MAC entry to gen2 if EP/RC supports gen2. +* Wait 2ms (LTSSM timeout is 24ms, PHY lock is ~5us in gen2). +* If (MAC/LTSSM.state == Recovery.RcvrLock) +* && (PHY/rx_valid==0) then pulse PHY/rx_reset. Transition +* to gen2 is stuck +*/
Sorry, what? Do others understand what the problem is? Can we clarify this?
drivers/pci/pcie/pcie-imx.c line 375
Quoted 8 lines
drivers/pci/pcie/pcie-imx.c new file +8
-0+pcie_phy_cr_read (pp->dbi_base,SSP_CR_LANE0_DIG_RX_ASIC_OUT , &rx_valid); + ltssm =readl (pp->dbi_base +DEBUG_R0 ) &0x3F ; +if ((ltssm ==0x0D ) && ((rx_valid &0x01 ) ==0 )) { +dev_err (&pdev->dev, +"transition to gen2 is stuck, reset PHY!\n" ); +pcie_phy_cr_read (pp->dbi_base,SSP_CR_LANE0_DIG_RX_OVRD_IN_LO , &temp); +change_field (&temp,3 ,3 ,0x1 ); +change_field (&temp,5 ,5 ,0x1 );
I wonder how complicated one can write temp |= (1 << 3) | (1 << 5). Please get rid of this change_field function.
Quoted 12 lines
drivers/pci/pcie/pcie-imx.c new file +12
-0+pcie_phy_cr_write (pp->dbi_base,SSP_CR_LANE0_DIG_RX_OVRD_IN_LO , +0x0028 ); +usleep_range (2000 ,3000 ); +pcie_phy_cr_read (pp->dbi_base,SSP_CR_LANE0_DIG_RX_OVRD_IN_LO , &temp); +change_field (&temp,3 ,3 ,0x0 ); +change_field (&temp,5 ,5 ,0x0 ); +pcie_phy_cr_write (pp->dbi_base,SSP_CR_LANE0_DIG_RX_OVRD_IN_LO , +0x0000 ); + } + + } +
drivers/pci/pcie/pcie-imx.c line 397
Quoted 10 lines
drivers/pci/pcie/pcie-imx.c new file +10
-0+if (!rc) { +if (iterations <=0 ) { +dev_err (&pdev->dev, +"link up failed, DEBUG_R0:0x%08x, DEBUG_R1:0x%08x RX_VALID:0x%x!\n" , +readl (pp->dbi_base +DEBUG_R0 ), +readl (pp->dbi_base +DEBUG_R1 ), + rx_valid); +return -ETIMEDOUT ; + } +return -ENODEV ;
I wonder under which circumstances you can reach this -ENODEV.
Quoted 61 lines
drivers/pci/pcie/pcie-imx.c new file +61
-0+ } + +return 0 ; +} + +static int imx_pcie_regions_setup (struct platform_device *pdev, +struct imx_pcie_port *pp) +{ +void __iomem *dbi_base = pp->dbi_base; +/* +* i.MX6 defines 16MB in the AXI address map for PCIe. +* +* That address space excepted the pcie registers is +* split and defined into different regions by iATU, +* with sizes and offsets as follows: +* +* 0x0100_0000 --- 0x010F_FFFF 1MB IORESOURCE_IO +* 0x0110_0000 --- 0x01EF_FFFF 14MB IORESOURCE_MEM +* 0x01F0_0000 --- 0x01FF_FFFF 1MB Cfg + Registers +*/ + +/* CMD reg:I/O space, MEM space, and Bus Master Enable */ +writel (readl (dbi_base +PCI_COMMAND ) + |PCI_COMMAND_IO + |PCI_COMMAND_MEMORY + |PCI_COMMAND_MASTER , + dbi_base +PCI_COMMAND ); + +/* Set the CLASS_REV of RC CFG header to PCI_CLASS_BRIDGE_PCI */ +writel (readl (dbi_base +PCI_CLASS_REVISION ) + | (PCI_CLASS_BRIDGE_PCI <<16 ), + dbi_base +PCI_CLASS_REVISION ); + +/* +* region0 outbound used to access target cfg +*/ +writel (0 , dbi_base + PCIE_PL_iATUVR); +writel (pp->root->start, dbi_base + PCIE_PL_iATURLBA); +writel (pp->dbi->end, dbi_base + PCIE_PL_iATURLA); +writel (0 , dbi_base + PCIE_PL_iATURUBA); + +writel (0 , dbi_base + PCIE_PL_iATURLTA); +writel (0 , dbi_base + PCIE_PL_iATURUTA); +writel (CfgRdWr0, dbi_base + PCIE_PL_iATURC1); +writel ((1 <<31 ), dbi_base + PCIE_PL_iATURC2); + +return 0 ; +} + + +static int imx_pcie_valid_config (struct imx_pcie_port *pp, +struct pci_bus *bus,int devfn) +{ +if (bus->number >=2 ) +return 0 ; + +if (devfn !=0 ) +return 0 ; + +return 1 ; +}
drivers/pci/pcie/pcie-imx.c line 468
Quoted 10 lines
drivers/pci/pcie/pcie-imx.c new file +10
-0+ + +static u32 get_bus_address (struct imx_pcie_port *pp, +struct pci_bus *bus,u32 devfn,int where) +{ +u32 va_address; +if (bus->number ==0 ) { + va_address = (u32 )pp->dbi_base + (where & ~0x3 ); + } +else {
The 'else' is supposed to be on the same line as the '}'
Quoted 81 lines
drivers/pci/pcie/pcie-imx.c new file +81
-0+ va_address = (u32 )pp->root_base + + (PCIE_CONF_BUS (bus->number -1 ) + +PCIE_CONF_DEV (PCI_SLOT (devfn)) + +PCIE_CONF_FUNC (PCI_FUNC (devfn)) + +PCIE_CONF_REG (where)); + } +return va_address; +} + +static int imx_pcie_read_config (struct pci_bus *bus,u32 devfn,int where, +int size,u32 *val) +{ +struct imx_pcie_port *pp =bus_to_port (bus->number); +u32 va_address; + +if (!pp) { +BUG (); +return -EINVAL ; + } + +if (imx_pcie_valid_config (pp, bus,PCI_SLOT (devfn)) ==0 ) { + *val =0xffffffff ; +return PCIBIOS_DEVICE_NOT_FOUND ; + } + + va_address =get_bus_address (pp, bus, devfn, where); + + *val =readl ((u32 *)va_address); + +if (size ==1 ) + *val = (*val >> (8 * (where &3 ))) &0xFF ; +else if (size ==2 ) + *val = (*val >> (8 * (where &3 ))) &0xFFFF ; + +return PCIBIOS_SUCCESSFUL ; +} + +static int imx_pcie_write_config (struct pci_bus *bus,u32 devfn, +int where,int size,u32 val) +{ +struct imx_pcie_port *pp =bus_to_port (bus->number); +u32 va_address =0 , mask =0 , tmp =0 ; +int ret =PCIBIOS_SUCCESSFUL ; + +if (!pp) { +BUG (); +return -EINVAL ; + } + +if (imx_pcie_valid_config (pp, bus,PCI_SLOT (devfn)) ==0 ) +return PCIBIOS_DEVICE_NOT_FOUND ; + + va_address =get_bus_address (pp, bus, devfn, where); + +if (size ==4 ) { +writel (val, (u32 *)va_address); + goto exit; + } + +if (size ==2 ) + mask = ~(0xFFFF << ((where &0x3 ) *8 )); +else if (size ==1 ) + mask = ~(0xFF << ((where &0x3 ) *8 )); +else + ret =PCIBIOS_BAD_REGISTER_NUMBER ; + + tmp =readl ((u32 *)va_address) & mask; + tmp |= val << ((where &0x3 ) *8 ); +writel (tmp, (u32 *)va_address); +exit: + +return ret; +} + + + +static struct pci_ops imx_pcie_ops = { + .read = imx_pcie_read_config, + .write = imx_pcie_write_config, +}; +
drivers/pci/pcie/pcie-imx.c line 559
Quoted 10 lines
drivers/pci/pcie/pcie-imx.c new file +10
-0+static struct pci_bus __init * +imx_pcie_scan_bus (int nr,struct pci_sys_data *sys) +{ +struct imx_pcie_port *pp =controller_to_port (nr); +if (nr >1 ) +return NULL ; + pp->root_bus_nr = sys->busnr; + +return pci_scan_root_bus (NULL , sys->busnr, &imx_pcie_ops, sys, + &sys->resources);
Whitespace damage here.
Quoted 46 lines
drivers/pci/pcie/pcie-imx.c new file +46
-0+} + +static int __init imx_pcie_map_irq (const struct pci_dev *dev,u8 slot,u8 pin) +{ +struct imx_pcie_port *pp =controller_to_port (0 ); +return pp->interrupt; +} + +static struct hw_pci imx_pci__initdata = { + .nr_controllers =1 , + .setup = imx_pcie_setup, + .scan = imx_pcie_scan_bus, + .map_irq = imx_pcie_map_irq, +}; + +/* PHY CR bus acess routines */ +static int pcie_phy_cr_ack_polling (void __iomem *dbi_base,int max_iterations,int exp_val) +{ +u32 temp_rd_data, wait_counter =0 ; + +do { + temp_rd_data =readl (dbi_base +PHY_STS_R ); + temp_rd_data = (temp_rd_data >>PCIE_CR_STAT_ACK_LOC ) &0x1 ; + wait_counter++; + }while ((wait_counter < max_iterations) && (temp_rd_data != exp_val)); + +if (temp_rd_data != exp_val) +return 0 ; +return 1 ; +} + +static int pcie_phy_cr_cap_addr (void __iomem *dbi_base,int addr) +{ +u32 temp_wr_data; + +/* write addr */ + temp_wr_data = addr <<PCIE_CR_CTL_DATA_LOC ; +writel (temp_wr_data, dbi_base +PHY_CTRL_R ); + +/* capture addr */ + temp_wr_data |= (0x1 <<PCIE_CR_CTL_CAP_ADR_LOC ); +writel (temp_wr_data, dbi_base +PHY_CTRL_R ); + +/* wait for ack */ +if (!pcie_phy_cr_ack_polling (dbi_base,100 ,1 )) +return 0 ;
drivers/pci/pcie/pcie-imx.c line 615
Quoted 10 lines
drivers/pci/pcie/pcie-imx.c new file +10
-0+ +/* deassert cap addr */ + temp_wr_data = addr <<PCIE_CR_CTL_DATA_LOC ; +writel (temp_wr_data, dbi_base +PHY_CTRL_R ); + +/* wait for ack de-assetion */ +if (!pcie_phy_cr_ack_polling (dbi_base,100 ,0 )) +return 0 ; + +return 1 ;
Remove the whitespace before the semicolons, also elsewhere in this patch.
Quoted 18 lines
drivers/pci/pcie/pcie-imx.c new file +18
-0+} + +static int pcie_phy_cr_read (void __iomem *dbi_base,int addr ,int *data) +{ +u32 temp_rd_data, temp_wr_data; + +/* write addr */ +/* cap addr */ +if (!pcie_phy_cr_cap_addr (dbi_base, addr)) +return 0 ; + +/* assert rd signal */ + temp_wr_data =0x1 <<PCIE_CR_CTL_RD_LOC ; +writel (temp_wr_data, dbi_base +PHY_CTRL_R ); + +/* wait for ack */ +if (!pcie_phy_cr_ack_polling (dbi_base,100 ,1 )) +return 0 ;
drivers/pci/pcie/pcie-imx.c line 643
Quoted 10 lines
drivers/pci/pcie/pcie-imx.c new file +10
-0+ +/* after got ack return data */ + temp_rd_data =readl (dbi_base +PHY_STS_R ); + *data = (temp_rd_data & (0xffff <<PCIE_CR_STAT_DATA_LOC )) ; + +/* deassert rd signal */ + temp_wr_data =0x0 ; +writel (temp_wr_data, dbi_base +PHY_CTRL_R ); + +/* wait for ack de-assetion */
s/assetion/assertion/
This typo is multiple times in this patch.
Quoted 64 lines
drivers/pci/pcie/pcie-imx.c new file +64
-0+if (!pcie_phy_cr_ack_polling (dbi_base,100 ,0 )) +return 0 ; + +return 1 ; + +} + +static int pcie_phy_cr_write (void __iomem *dbi_base,int addr,int data) +{ +u32 temp_wr_data; + +/* write addr */ +/* cap addr */ +if (!pcie_phy_cr_cap_addr (dbi_base, addr)) +return 0 ; + + temp_wr_data = data <<PCIE_CR_CTL_DATA_LOC ; +writel (temp_wr_data, dbi_base +PHY_CTRL_R ); + +/* capture data */ + temp_wr_data |= (0x1 <<PCIE_CR_CTL_CAP_DAT_LOC ); +writel (temp_wr_data, dbi_base +PHY_CTRL_R ); + +/* wait for ack */ +if (!pcie_phy_cr_ack_polling (dbi_base,100 ,1 )) +return 0 ; + +/* deassert cap data */ + temp_wr_data = data <<PCIE_CR_CTL_DATA_LOC ; +writel (temp_wr_data, dbi_base +PHY_CTRL_R ); + +/* wait for ack de-assetion */ +if (!pcie_phy_cr_ack_polling (dbi_base,100 ,0 )) +return 0 ; + +/* assert wr signal */ + temp_wr_data =0x1 <<PCIE_CR_CTL_WR_LOC ; +writel (temp_wr_data, dbi_base +PHY_CTRL_R ); + +/* wait for ack */ +if (!pcie_phy_cr_ack_polling (dbi_base,100 ,1 )) +return 0 ; + +/* deassert wr signal */ + temp_wr_data = data <<PCIE_CR_CTL_DATA_LOC ; +writel (temp_wr_data, dbi_base +PHY_CTRL_R ); + +/* wait for ack de-assetion */ +if (!pcie_phy_cr_ack_polling (dbi_base,100 ,0 )) +return 0 ; + + temp_wr_data =0x0 ; +writel (temp_wr_data, dbi_base +PHY_CTRL_R ); + +return 1 ; +} + +static int imx_pcie_enable_controller (struct platform_device *pdev) +{ +struct imx_pcie_port *pp =platform_get_drvdata (pdev); +int ret; + +/* Enable PCIE power */ +gpio_set_value (pp->pcie_pwr_en,1 );
drivers/pci/pcie/pcie-imx.c line 717
Quoted 10 lines
drivers/pci/pcie/pcie-imx.c new file +10
-0+ +imx_pcie_clrset (pp, iomuxc_gpr1_test_powerdown,0 <<18 ,IOMUXC_GPR1 ); +imx_pcie_clrset (pp, iomuxc_gpr1_pcie_ref_clk_en,1 <<16 ,IOMUXC_GPR1 ); + +/* Enable clocks */ + ret =clk_set_parent (pp->lvds1_sel, pp->sata_ref); +if (ret) { +dev_err (&pdev->dev,"unable to set lvds1 parent: %d\n" , ret); +return -EINVAL ; + }
This should be done in architecture code I think.
Quoted 8 lines
drivers/pci/pcie/pcie-imx.c new file +8
-0+ + ret =clk_prepare_enable (pp->pcie_ref_125m); +if (ret) { +dev_err (&pdev->dev,"unable to enable pcie_ref_125m: %d\n" , ret); +return -EINVAL ; + } + + ret =clk_prepare_enable (pp->lvds1);
drivers/pci/pcie/pcie-imx.c line 735
Quoted 10 lines
drivers/pci/pcie/pcie-imx.c new file +10
-0+if (ret) { +dev_err (&pdev->dev,"unable to enable lvds1: %d\n" , ret); +return -EINVAL ; + } + + ret =clk_prepare_enable (pp->pcie_axi); +if (ret) { +dev_err (&pdev->dev,"unable to enable pcie_axi: %d\n" , ret); +return -EINVAL ; + }
This function does not disable the already enabled clocks in the error path. Please fix.
Quoted 27 lines
drivers/pci/pcie/pcie-imx.c new file +27
-0+ + +return 0 ; +} + +static void card_reset (struct platform_device *pdev) +{ +struct imx_pcie_port *pp =platform_get_drvdata (pdev); + +imx_pcie_clrset (pp, iomuxc_gpr1_test_powerdown,1 <<18 ,IOMUXC_GPR1 ); +imx_pcie_clrset (pp, iomuxc_gpr12_app_ltssm_enable,1 <<10 ,IOMUXC_GPR12 ); +imx_pcie_clrset (pp, iomuxc_gpr1_pcie_ref_clk_en,0 <<16 ,IOMUXC_GPR1 ); + +gpio_set_value (pp->pcie_rst,0 ); +msleep (100 ); +gpio_set_value (pp->pcie_rst,1 ); +} + +static int __init add_pcie_port (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct imx_pcie_port *pp =platform_get_drvdata (pdev); +int ret; + + ret =imx_pcie_link_up (pdev); +if (ret) { +dev_info (dev,"IMX PCIe port: link down!\n" );
drivers/pci/pcie/pcie-imx.c line 772
Quoted 10 lines
drivers/pci/pcie/pcie-imx.c new file +10
-0+/* Release the clocks, and disable the power */ + +clk_disable (pp->pcie_axi); +clk_put (pp->pcie_axi); + +clk_disable (pp->lvds1); +clk_put (pp->lvds1); + +clk_put (pp->pcie_ref_125m); +clk_put (pp->sata_ref);
You clk_put the clocks in the error path of the probe function. Don't duplicate it.
Quoted 60 lines
drivers/pci/pcie/pcie-imx.c new file +60
-0+ +imx_pcie_clrset (pp, iomuxc_gpr1_pcie_ref_clk_en,0 <<16 , +IOMUXC_GPR1 ); + +/* Disable PCIE power */ +gpio_set_value (pp->pcie_pwr_en,0 ); + +imx_pcie_clrset (pp, iomuxc_gpr1_test_powerdown,1 <<18 , +IOMUXC_GPR1 ); + +return ret; + } + +dev_info (dev,"IMX PCIe port: link up.\n" ); + pp->index =0 ; + pp->root_bus_nr =-1 ; +spin_lock_init (&pp->conf_lock); +return 0 ; +} + + +static int set_pcie_clock_tunings (struct platform_device *pdev) +{ +struct imx_pcie_port *pp =platform_get_drvdata (pdev); +/* FIXME the field name should be aligned to RM */ +imx_pcie_clrset (pp, iomuxc_gpr12_app_ltssm_enable,0 <<10 ,IOMUXC_GPR12 ); + +/* configure constant input signal to the pcie ctrl and phy */ +imx_pcie_clrset (pp, iomuxc_gpr12_device_type,PCI_EXP_TYPE_ROOT_PORT <<12 , +IOMUXC_GPR12 ); +imx_pcie_clrset (pp, iomuxc_gpr12_los_level,9 <<4 ,IOMUXC_GPR12 ); + +imx_pcie_clrset (pp, iomuxc_gpr8_tx_deemph_gen1,0 <<0 ,IOMUXC_GPR8 ); +imx_pcie_clrset (pp, iomuxc_gpr8_tx_deemph_gen2_3p5db,0 <<6 ,IOMUXC_GPR8 ); +imx_pcie_clrset (pp, iomuxc_gpr8_tx_deemph_gen2_6db,20 <<12 ,IOMUXC_GPR8 ); +imx_pcie_clrset (pp, iomuxc_gpr8_tx_swing_full,127 <<18 ,IOMUXC_GPR8 ); +imx_pcie_clrset (pp, iomuxc_gpr8_tx_swing_low,127 <<25 ,IOMUXC_GPR8 ); +return 0 ; +} + + +static int __init imx_pcie_pltfm_probe (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct imx_pcie_port *pp =devm_kzalloc (dev,sizeof (*pp),GFP_KERNEL ); +int ret; + +platform_set_drvdata (pdev, pp); + pp->dev = &pdev->dev; + + pp->pcie_pwr_en =of_get_named_gpio (pdev->dev.of_node, +"power-enable" ,0 ); +if (gpio_is_valid (pp->pcie_pwr_en)) +devm_gpio_request_one (dev, pp->pcie_pwr_en, +GPIOF_OUT_INIT_LOW , +"PCIe power enable" ); + + pp->pcie_rst =of_get_named_gpio (pdev->dev.of_node, +"pcie-reset" ,0 ); +if (gpio_is_valid (pp->pcie_rst))
drivers/pci/pcie/pcie-imx.c line 842
Quoted 10 lines
drivers/pci/pcie/pcie-imx.c new file +10
-0+devm_gpio_request_one (dev, pp->pcie_rst, +GPIOF_OUT_INIT_LOW , +"PCIe reset" ); + + pp->pcie_wake_up =of_get_named_gpio (pdev->dev.of_node, +"wake-up" ,0 ); +if (gpio_is_valid (pp->pcie_wake_up)) +devm_gpio_request_one (dev, pp->pcie_wake_up, +GPIOF_IN , +"PCIe wake up" );
More whitespace damage above.
Quoted 33 lines
drivers/pci/pcie/pcie-imx.c new file +33
-0+ + pp->dbi =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); +if (!pp->dbi) { +dev_err (dev,"no mmio space\n" ); +return -EINVAL ; + } + + pp->dbi_base =devm_request_and_ioremap (&pdev->dev, pp->dbi); +if (!pp->dbi_base) { +pr_err ("unable to remap dbi\n" ); +return -ENOMEM ; + } + + + pp->io =platform_get_resource (pdev,IORESOURCE_MEM ,1 ); +if (!pp->io) { +dev_err (dev,"no mmio space\n" ); +return -EINVAL ; + } + + pp->mem =platform_get_resource (pdev,IORESOURCE_MEM ,2 ); +if (!pp->mem) { +dev_err (dev,"no mmio space\n" ); +return -EINVAL ; + } + + pp->root =platform_get_resource (pdev,IORESOURCE_MEM ,3 ); +if (!pp->root) { +dev_err (dev,"no root memory space\n" ); +return -EINVAL ; + } + + pp->root_base =devm_request_and_ioremap (&pdev->dev, pp->root);
drivers/pci/pcie/pcie-imx.c line 885
Quoted 10 lines
drivers/pci/pcie/pcie-imx.c new file +10
-0+if (!pp->root_base) { +dev_err (&pdev->dev,"unable to remap root mem\n" ); +return -ENOMEM ; + } + + + pp->interrupt =platform_get_irq (pdev,0 ); + + +/* Setup clocks */
Whitespace damage here.
drivers/pci/pcie/pcie-imx.c line 886
Quoted 1 line
drivers/pci/pcie/pcie-imx.c new file +1
-0+ pp->lvds1_sel =clk_get (dev,"lvds1_sel" );
Use devm_clk_get
Quoted 46 lines
drivers/pci/pcie/pcie-imx.c new file +46
-0+if (IS_ERR (pp->lvds1_sel)) { +dev_err (dev, +"lvds1_sel clock missing or invalid\n" ); + ret = -EINVAL ; + goto err_out; + } + + pp->lvds1 =clk_get (dev,"lvds1" ); +if (IS_ERR (pp->lvds1)) { +dev_err (dev, +"lvds1 clock select missing or invalid\n" ); + ret = -EINVAL ; + goto err_out; + } + + pp->pcie_ref_125m =clk_get (dev,"pcie_ref_125m" ); +if (IS_ERR (pp->pcie_ref_125m)) { +dev_err (dev, +"pcie_ref_125m clock source missing or invalid\n" ); + ret = -EINVAL ; + goto err_out; + } + + pp->pcie_axi =clk_get (dev,"pcie_axi" ); +if (IS_ERR (pp->pcie_axi)) { +dev_err (dev,"pcie_axi clock source missing or invalid\n" ); + ret = -EINVAL ; + goto err_out; + } + + pp->sata_ref =clk_get (dev,"sata_ref" ); +if (IS_ERR (pp->sata_ref)) { +dev_err (dev,"sata_ref clock source missing or invalid\n" ); + ret = -EINVAL ; + goto err_out; + } + + pp->iomuxc_gpr =syscon_regmap_lookup_by_compatible ("fsl,imx6q-iomuxc-gpr" ); +if (IS_ERR (pp->iomuxc_gpr)) { +dev_err (dev,"unable to find iomuxc registers\n" ); + ret = -EINVAL ; + goto err_out; + } + +/* togle the external card's reset */ +card_reset (pdev);
drivers/pci/pcie/pcie-imx.c line 942
Quoted 10 lines
drivers/pci/pcie/pcie-imx.c new file +10
-0+ +/* Enable the pwr, clks and so on */ +set_pcie_clock_tunings (pdev); + ret =imx_pcie_enable_controller (pdev); +if (ret) + goto err_out; + +usleep_range (3000 ,4000 ); +imx_pcie_regions_setup (pdev, pp); +usleep_range (3000 ,4000 );
Are these usleeps required?
Quoted 18 lines
drivers/pci/pcie/pcie-imx.c new file +18
-0+ +/* start link up */ +imx_pcie_clrset (pp, iomuxc_gpr12_app_ltssm_enable,1 <<10 ,IOMUXC_GPR12 ); + +/* add the pcie port */ + ret =add_pcie_port (pdev); +if (ret) + goto err_out; + + pp->index = imx_pcie.nr_controllers; + imx_pcie.nr_controllers++; +list_add_tail (&pp->next, &pcie_port_list); + +pci_common_init (&imx_pci); + +return 0 ; + +err_out:
drivers/pci/pcie/pcie-imx.c line 970
Quoted 10 lines
drivers/pci/pcie/pcie-imx.c new file +10
-0+if (pp->lvds1_sel) +clk_put (pp->lvds1_sel); +if (pp->lvds1) +clk_put (pp->lvds1); +if (pp->pcie_ref_125m) +clk_put (pp->pcie_ref_125m); +if (pp->pcie_axi) +clk_put (pp->pcie_axi); +if (pp->sata_ref) +clk_put (pp->sata_ref);
You shouldn't call clk_put with error codes.
Quoted 24 lines
drivers/pci/pcie/pcie-imx.c new file +24
-0+return ret; +} + +static int __exit imx_pcie_pltfm_remove(struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct imx_pcie_port *pp =platform_get_drvdata (pdev); + +if (pp->rfkill) { +rfkill_unregister (pp->rfkill); +rfkill_destroy (pp->rfkill); + pp->rfkill =NULL ; + } + +imx_pcie_clrset (pp, iomuxc_gpr1_pcie_ref_clk_en,0 <<16 ,IOMUXC_GPR1 ); +imx_pcie_clrset (pp, iomuxc_gpr1_test_powerdown,1 <<18 ,IOMUXC_GPR1 ); +imx_pcie_clrset (pp, iomuxc_gpr12_app_ltssm_enable,1 <<10 ,IOMUXC_GPR12 ); + +/* Release clocks, and disable power */ +if (pp->pcie_axi) { +clk_disable (pp->pcie_axi); +clk_put (pp->pcie_axi); + } +
drivers/pci/pcie/pcie-imx.c line 1004
Quoted 10 lines
drivers/pci/pcie/pcie-imx.c new file +10
-0+if (pp->lvds1) { +clk_disable (pp->lvds1); +clk_put (pp->lvds1); + } + +if (pp->pcie_ref_125m) +clk_put (pp->pcie_ref_125m); + +if (pp->sata_ref) +clk_put (pp->sata_ref);
All these tests are *always* true.
drivers/pci/pcie/pcie-imx.c line 1009
Quoted 5 lines
drivers/pci/pcie/pcie-imx.c new file +5
-0+ +gpio_set_value (pp->pcie_rst,0 ); +gpio_set_value (pp->pcie_pwr_en,0 ); + +dev_err (dev,"disabled everything\n" );
Remove this.
drivers/pci/pcie/pcie-imx.c line 1010
Quoted 1 line
drivers/pci/pcie/pcie-imx.c new file +1
-0+msleep (500 );
Why?
Quoted 47 lines
drivers/pci/pcie/pcie-imx.c new file +39
-0+ +platform_set_drvdata (pdev,NULL ); + +return 0 ; +} + +static struct platform_driver imx_pcie_pltfm_driver = { + .driver = { + .name ="imx-pcie" , + .owner =THIS_MODULE , + .of_match_table = pcie_of_match, + }, + .probe = imx_pcie_pltfm_probe, + .remove =__exit_p (imx_pcie_pltfm_remove), +}; + +/*****************************************************************************\ +* * +* Driver init/exit * +* * +\*****************************************************************************/ + +static int __init imx_pcie_drv_init (void ) +{ +INIT_LIST_HEAD (&pcie_port_list); +return platform_driver_register (&imx_pcie_pltfm_driver); +} + +static void __exit imx_pcie_drv_exit(void) +{ +platform_driver_unregister (&imx_pcie_pltfm_driver); +} + +module_init (imx_pcie_drv_init); +module_exit (imx_pcie_drv_exit); + +MODULE_DESCRIPTION ("i.MX PCIE platform driver" ); +MODULE_AUTHOR ("Sean Cross <[email protected]>" ); +MODULE_LICENSE ("GPL v2" );Signature · 6 lines
1.7.9.5 _______________________________________________ devicetree-discuss mailing list devicetree-discuss at lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Signature · 4 lines
Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
PATCH 4/4 ARM i.MX6: Add PCI Express to device tree
In reply to Sean CrossAdd a PCI Express port to the i.MX6 device tree using interrupts, clocks, and memory ranges appropriate for the device.
--- arch/arm/boot/dts/imx6qdl.dtsi | 12 ++++++++++++ 1 file changed, 12 insertions(+)
arch/arm/boot/dts/imx6qdl.dtsi +12 -0
@@ -654,6 +654,18 @@ status = "disabled"; }; + pcie: pcie at 01ffc000 { + compatible = "fsl,imx6q-pcie"; + reg = <0x01ffc000 0x4000>, + <0x01000000 0x100000>, + <0x01100000 0xe00000>, + <0x01f00000 0xfc000>; + interrupts = <0 122 0x04>; + clocks = <&clks 186>, <&clks 189>, <&clks 196>, <&clks 198>, <&clks 144>; + clock-names = "sata_ref", "pcie_ref_125m", "lvds1_sel", "lvds1", "pcie_axi"; + status = "disabled"; + }; + mlb at 0218c000 { reg = <0x0218c000 0x4000>; interrupts = <0 53 0x04 0 117 0x04 0 126 0x04>;
Signature · 1 line
1.7.9.5