PCI: Add SPEAr13xx PCIe support

DT field name for the phy address changed since kernel 3.10. Set the snps,phy-addr to 0xffffffff so that the driver probes for the phy.

Signed-off-by Mohit Kumar <[email protected]>

Cc Pratyush Anand <[email protected]>

Cc Viresh Kumar <[email protected]>

Cc spear-devel at list.st.com

Cc linux-arm-kernel at lists.infradead.org

Cc devicetree at vger.kernel.org

---
 arch/arm/boot/dts/spear13xx.dtsi |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

arch/arm/boot/dts/spear13xx.dtsi +0 -0

index 4382547..3518803 100644

Quoted 2 lines · Mohit Kumar

DT field name for the phy address changed since kernel 3.10. Set the snps,phy-addr to 0xffffffff so that the driver probes for the phy.

Quoted 16 lines

Signed-off-by Mohit Kumar <[email protected]>

Cc Pratyush Anand <[email protected]>

Cc Viresh Kumar <[email protected]>

Cc spear-devel at list.st.com

Cc linux-arm-kernel at lists.infradead.org

Cc devicetree at vger.kernel.org

---
 arch/arm/boot/dts/spear13xx.dtsi |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

arch/arm/boot/dts/spear13xx.dtsi +0 -0

index 4382547..3518803 100644

arch/arm/boot/dts/spear13xx.dtsi line 161

Quoted 7 lines

arch/arm/boot/dts/spear13xx.dtsi +1 -0

@@ -155,6 +155,7 @@
 
                gmac0: eth at e2000000 {
                        compatible = "st,spear600-gmac";
+                       snps,phy-addr = <0xffffffff>;
                        reg = <0xe2000000 0x8000>;
                        interrupts = <0 33 0x4
                                      0 34 0x4>;

Acked-by Viresh Kumar <[email protected]>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-----Original Message-----
From: wens213 at gmail.com [mailto:wens213 at gmail.com] On Behalf Of Chen-Yu Tsai
Sent: Friday, January 24, 2014 11:33 AM
To: Viresh Kumar
Cc: Mohit KUMAR DCG; Pratyush ANAND; spear-devel; linux-arm- kernel at lists.infradead.org; devicetree
Subject: Re: [PATCH V2 1/8] SPEAr13xx: Set dt field entry <stmmac,phy- addr> for phy probe

Hi,

Quoted 1 line · Viresh Kumar

On Thu, Jan 23, 2014 at 4:02 PM, Mohit Kumar <[email protected]>

Quoted 2 lines · wrote:

Quoted 2 lines

DT field name for the phy address changed since kernel 3.10. Set the snps,phy-addr to 0xffffffff so that the driver probes for the phy.

This will no longer be required. The default behavior for DT based setups has been changed to auto-detecting the phy.

See http://patchwork.ozlabs.org/patch/312063/

Cheers
ChenYu

Quoted 31 lines

Quoted 17 lines

Signed-off-by Mohit Kumar <[email protected]>

Cc Pratyush Anand <[email protected]>

Cc Viresh Kumar <[email protected]>

Cc spear-devel at list.st.com

Cc linux-arm-kernel at lists.infradead.org

Cc devicetree at vger.kernel.org

---
 arch/arm/boot/dts/spear13xx.dtsi |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

arch/arm/boot/dts/spear13xx.dtsi +0 -0

index 4382547..3518803 100644

arch/arm/boot/dts/spear13xx.dtsi line 161

Quoted 7 lines

arch/arm/boot/dts/spear13xx.dtsi +1 -0

@@ -155,6 +155,7 @@
 
                gmac0: eth at e2000000 {
                        compatible = "st,spear600-gmac";
+                       snps,phy-addr = <0xffffffff>;
                        reg = <0xe2000000 0x8000>;
                        interrupts = <0 33 0x4
                                      0 34 0x4>;

Acked-by Viresh Kumar <[email protected]>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Enable EABI, OEABI, VFP and NFS configs in default configuration file for SPEAr13xx.

Signed-off-by Pratyush Anand <[email protected]>

Signed-off-by Mohit Kumar <[email protected]>

Cc Mohit Kumar <[email protected]>

Cc Viresh Kumar <[email protected]>

Cc spear-devel at list.st.com

Cc linux-arm-kernel at lists.infradead.org

include/linux/ahci_platform.h struct ahci_platform_data { · line 31

Quoted 9 lines · Mohit Kumar

include/linux/ahci_platform.h +1 -0

index 73a2500..76d35e8 100644
@@ -28,6 +28,7 @@ struct ahci_platform_data {
 	const struct ata_port_info *ata_port_info;
 	unsigned int force_port_map;
 	unsigned int mask_port_map;
+	void *driver_data;

Please use private_data instead for consistency with other ata data structures.

Thanks.

Signature · 1 line

tejun

arch/arm/boot/dts/spear13xx.dtsi +0 -0

index 3518803..2b4e58e 100644

arch/arm/boot/dts/spear13xx.dtsi +66 -0

+/* SPEAr1340 Registers */
+/* Power Management Registers */
+#define SPEAR1340_PCM_CFG			0x100
+	#define SPEAR1340_PCM_CFG_SATA_POWER_EN	0x800
+#define SPEAR1340_PCM_WKUP_CFG			0x104
+#define SPEAR1340_SWITCH_CTR			0x108
+
+#define SPEAR1340_PERIP1_SW_RST			0x318
+	#define SPEAR1340_PERIP1_SW_RST_SATA	0x1000
+#define SPEAR1340_PERIP2_SW_RST			0x31C
+#define SPEAR1340_PERIP3_SW_RST			0x320
+
+/* PCIE - SATA configuration registers */
+#define SPEAR1340_PCIE_SATA_CFG			0x424
+	/* PCIE CFG MASks */
+	#define SPEAR1340_PCIE_CFG_DEVICE_PRESENT	(1 << 11)
+	#define SPEAR1340_PCIE_CFG_POWERUP_RESET	(1 << 10)
+	#define SPEAR1340_PCIE_CFG_CORE_CLK_EN		(1 << 9)
+	#define SPEAR1340_PCIE_CFG_AUX_CLK_EN		(1 << 8)
+	#define SPEAR1340_SATA_CFG_TX_CLK_EN		(1 << 4)
+	#define SPEAR1340_SATA_CFG_RX_CLK_EN		(1 << 3)
+	#define SPEAR1340_SATA_CFG_POWERUP_RESET	(1 << 2)
+	#define SPEAR1340_SATA_CFG_PM_CLK_EN		(1 << 1)
+	#define SPEAR1340_PCIE_SATA_SEL_PCIE		(0)
+	#define SPEAR1340_PCIE_SATA_SEL_SATA		(1)
+	#define SPEAR1340_PCIE_SATA_CFG_MASK		0xF1F
+	#define SPEAR1340_PCIE_CFG_VAL	(SPEAR1340_PCIE_SATA_SEL_PCIE | \
+			SPEAR1340_PCIE_CFG_AUX_CLK_EN | \
+			SPEAR1340_PCIE_CFG_CORE_CLK_EN | \
+			SPEAR1340_PCIE_CFG_POWERUP_RESET | \
+			SPEAR1340_PCIE_CFG_DEVICE_PRESENT)
+	#define SPEAR1340_SATA_CFG_VAL	(SPEAR1340_PCIE_SATA_SEL_SATA | \
+			SPEAR1340_SATA_CFG_PM_CLK_EN | \
+			SPEAR1340_SATA_CFG_POWERUP_RESET | \
+			SPEAR1340_SATA_CFG_RX_CLK_EN | \
+			SPEAR1340_SATA_CFG_TX_CLK_EN)
+
+#define SPEAR1340_PCIE_MIPHY_CFG		0x428
+	#define SPEAR1340_MIPHY_OSC_BYPASS_EXT		(1 << 31)
+	#define SPEAR1340_MIPHY_CLK_REF_DIV2		(1 << 27)
+	#define SPEAR1340_MIPHY_CLK_REF_DIV4		(2 << 27)
+	#define SPEAR1340_MIPHY_CLK_REF_DIV8		(3 << 27)
+	#define SPEAR1340_MIPHY_PLL_RATIO_TOP(x)	(x << 0)
+	#define SPEAR1340_PCIE_MIPHY_CFG_MASK		0xF80000FF
+	#define SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA \
+			(SPEAR1340_MIPHY_OSC_BYPASS_EXT | \
+			SPEAR1340_MIPHY_CLK_REF_DIV2 | \
+			SPEAR1340_MIPHY_PLL_RATIO_TOP(60))
+	#define SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA_25M_CRYSTAL_CLK \
+			(SPEAR1340_MIPHY_PLL_RATIO_TOP(120))
+	#define SPEAR1340_PCIE_SATA_MIPHY_CFG_PCIE \
+			(SPEAR1340_MIPHY_OSC_BYPASS_EXT | \
+			SPEAR1340_MIPHY_PLL_RATIO_TOP(25))
+
+struct spear13xx_cfg_priv {
+	struct regmap		*misc;
+};
+
+/* SATA device registration */
+static void spear1340_sata_miphy_init(struct spear13xx_cfg_priv *cfgpriv)
+{
+	regmap_update_bits(cfgpriv->misc, SPEAR1340_PCIE_SATA_CFG,
+			SPEAR1340_PCIE_SATA_CFG_MASK, SPEAR1340_SATA_CFG_VAL);
+	regmap_update_bits(cfgpriv->misc, SPEAR1340_PCIE_MIPHY_CFG,
+			SPEAR1340_PCIE_MIPHY_CFG_MASK,
+			SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA_25M_CRYSTAL_CLK);

Quoted 8 lines · Mohit Kumar

arch/arm/boot/dts/spear13xx.dtsi +0 -0

index 3518803..2b4e58e 100644
@@ -78,6 +78,10 @@
 		status = "disabled";
 	};
 

arch/arm/boot/dts/spear13xx.dtsi line 87

Quoted 7 lines

arch/arm/boot/dts/spear13xx.dtsi +4 -0

+	cfg {
+		compatible = "st,spear13xx-cfg";
+	};
+
 	ahb {
 		#address-cells = <1>;
 		#size-cells = <1>;

I only saw some of the patches, and did not get a patch with the binding description for this device. Please forward that patch to me, or add it to the series if you didn't have one.

I assume you'd want a phandle pointing to the syscon device in here as well?

Regarding the naming, please do not use 'xx' wildcards in DT compatible strings. Instead, use the exact model name of the first supported version of the hardware (e.g. spear1300 or spear600) that remains compatible. If there are minor variations between the versions, use a list with the most specific version as well as the older ones it's compatible with.

Quoted 5 lines

arch/arm/boot/dts/spear13xx.dtsi +0 -0

@@ -221,6 +225,11 @@
 				  0xd8000000 0xd8000000 0x01000000
 				  0xe0000000 0xe0000000 0x10000000>;
 

arch/arm/boot/dts/spear13xx.dtsi line 232

Quoted 5 lines

arch/arm/boot/dts/spear13xx.dtsi +5 -0

+			misc: misc at e0700000 {
+				compatible = "st,spear13xx-misc", "syscon";
+				reg = <0xe0700000 0x1000>;
+			};
+

Same here. Also, I would make this 'misc: syscon at e0700000', since 'misc' does not seem like an appropriate device name.

Quoted 73 lines

arch/arm/boot/dts/spear13xx.dtsi +71 -0

+/* SPEAr1340 Registers */
+/* Power Management Registers */
+#define SPEAR1340_PCM_CFG			0x100
+	#define SPEAR1340_PCM_CFG_SATA_POWER_EN	0x800
+#define SPEAR1340_PCM_WKUP_CFG			0x104
+#define SPEAR1340_SWITCH_CTR			0x108
+
+#define SPEAR1340_PERIP1_SW_RST			0x318
+	#define SPEAR1340_PERIP1_SW_RST_SATA	0x1000
+#define SPEAR1340_PERIP2_SW_RST			0x31C
+#define SPEAR1340_PERIP3_SW_RST			0x320
+
+/* PCIE - SATA configuration registers */
+#define SPEAR1340_PCIE_SATA_CFG			0x424
+	/* PCIE CFG MASks */
+	#define SPEAR1340_PCIE_CFG_DEVICE_PRESENT	(1 << 11)
+	#define SPEAR1340_PCIE_CFG_POWERUP_RESET	(1 << 10)
+	#define SPEAR1340_PCIE_CFG_CORE_CLK_EN		(1 << 9)
+	#define SPEAR1340_PCIE_CFG_AUX_CLK_EN		(1 << 8)
+	#define SPEAR1340_SATA_CFG_TX_CLK_EN		(1 << 4)
+	#define SPEAR1340_SATA_CFG_RX_CLK_EN		(1 << 3)
+	#define SPEAR1340_SATA_CFG_POWERUP_RESET	(1 << 2)
+	#define SPEAR1340_SATA_CFG_PM_CLK_EN		(1 << 1)
+	#define SPEAR1340_PCIE_SATA_SEL_PCIE		(0)
+	#define SPEAR1340_PCIE_SATA_SEL_SATA		(1)
+	#define SPEAR1340_PCIE_SATA_CFG_MASK		0xF1F
+	#define SPEAR1340_PCIE_CFG_VAL	(SPEAR1340_PCIE_SATA_SEL_PCIE | \
+			SPEAR1340_PCIE_CFG_AUX_CLK_EN | \
+			SPEAR1340_PCIE_CFG_CORE_CLK_EN | \
+			SPEAR1340_PCIE_CFG_POWERUP_RESET | \
+			SPEAR1340_PCIE_CFG_DEVICE_PRESENT)
+	#define SPEAR1340_SATA_CFG_VAL	(SPEAR1340_PCIE_SATA_SEL_SATA | \
+			SPEAR1340_SATA_CFG_PM_CLK_EN | \
+			SPEAR1340_SATA_CFG_POWERUP_RESET | \
+			SPEAR1340_SATA_CFG_RX_CLK_EN | \
+			SPEAR1340_SATA_CFG_TX_CLK_EN)
+
+#define SPEAR1340_PCIE_MIPHY_CFG		0x428
+	#define SPEAR1340_MIPHY_OSC_BYPASS_EXT		(1 << 31)
+	#define SPEAR1340_MIPHY_CLK_REF_DIV2		(1 << 27)
+	#define SPEAR1340_MIPHY_CLK_REF_DIV4		(2 << 27)
+	#define SPEAR1340_MIPHY_CLK_REF_DIV8		(3 << 27)
+	#define SPEAR1340_MIPHY_PLL_RATIO_TOP(x)	(x << 0)
+	#define SPEAR1340_PCIE_MIPHY_CFG_MASK		0xF80000FF
+	#define SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA \
+			(SPEAR1340_MIPHY_OSC_BYPASS_EXT | \
+			SPEAR1340_MIPHY_CLK_REF_DIV2 | \
+			SPEAR1340_MIPHY_PLL_RATIO_TOP(60))
+	#define SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA_25M_CRYSTAL_CLK \
+			(SPEAR1340_MIPHY_PLL_RATIO_TOP(120))
+	#define SPEAR1340_PCIE_SATA_MIPHY_CFG_PCIE \
+			(SPEAR1340_MIPHY_OSC_BYPASS_EXT | \
+			SPEAR1340_MIPHY_PLL_RATIO_TOP(25))
+
+struct spear13xx_cfg_priv {
+	struct regmap		*misc;
+};
+
+/* SATA device registration */
+static void spear1340_sata_miphy_init(struct spear13xx_cfg_priv *cfgpriv)
+{
+	regmap_update_bits(cfgpriv->misc, SPEAR1340_PCIE_SATA_CFG,
+			SPEAR1340_PCIE_SATA_CFG_MASK, SPEAR1340_SATA_CFG_VAL);
+	regmap_update_bits(cfgpriv->misc, SPEAR1340_PCIE_MIPHY_CFG,
+			SPEAR1340_PCIE_MIPHY_CFG_MASK,
+			SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA_25M_CRYSTAL_CLK);
+	/* Switch on sata power domain */
+	regmap_update_bits(cfgpriv->misc, SPEAR1340_PCM_CFG,
+			SPEAR1340_PCM_CFG_SATA_POWER_EN,
+			SPEAR1340_PCM_CFG_SATA_POWER_EN);
+	msleep(20);

arch/arm/boot/dts/spear13xx.dtsi

Quoted 5 lines

arch/arm/boot/dts/spear13xx.dtsi +5 -0

+	/* Disable PCIE SATA Controller reset */
+	regmap_update_bits(cfgpriv->misc, SPEAR1340_PERIP1_SW_RST,
+			SPEAR1340_PERIP1_SW_RST_SATA, 0);
+	msleep(20);
+}

Looking at the actual code now, this very much looks like it ought to be a "phy" driver and get put in drivers/phy/.

Please see the recent mailing list discussions about making the ahci driver more generic. Once you put this code in a proper phy driver, you should be able to avoid a lot of your workaround and just use the regular ahci-platform driver without any hand-crafted platform data callbacks.

Arnd

Quoted 4 lines · Arnd Bergmann

On Thursday 23 January 2014, Mohit Kumar wrote:

I assume you'd want a phandle pointing to the syscon device in here as well?

Since there is only one syscon device in the whole DT, so do I really need to add phandle? Currently I am using syscon_regmap_lookup_by_compatible to find syscon device.

Quoted 71 lines

Quoted 71 lines

drivers/mfd/spear13xx-cfg.c new file +71 -0

+/* SPEAr1340 Registers */
+/* Power Management Registers */
+#define SPEAR1340_PCM_CFG			0x100
+	#define SPEAR1340_PCM_CFG_SATA_POWER_EN	0x800
+#define SPEAR1340_PCM_WKUP_CFG			0x104
+#define SPEAR1340_SWITCH_CTR			0x108
+
+#define SPEAR1340_PERIP1_SW_RST			0x318
+	#define SPEAR1340_PERIP1_SW_RST_SATA	0x1000
+#define SPEAR1340_PERIP2_SW_RST			0x31C
+#define SPEAR1340_PERIP3_SW_RST			0x320
+
+/* PCIE - SATA configuration registers */
+#define SPEAR1340_PCIE_SATA_CFG			0x424
+	/* PCIE CFG MASks */
+	#define SPEAR1340_PCIE_CFG_DEVICE_PRESENT	(1 << 11)
+	#define SPEAR1340_PCIE_CFG_POWERUP_RESET	(1 << 10)
+	#define SPEAR1340_PCIE_CFG_CORE_CLK_EN		(1 << 9)
+	#define SPEAR1340_PCIE_CFG_AUX_CLK_EN		(1 << 8)
+	#define SPEAR1340_SATA_CFG_TX_CLK_EN		(1 << 4)
+	#define SPEAR1340_SATA_CFG_RX_CLK_EN		(1 << 3)
+	#define SPEAR1340_SATA_CFG_POWERUP_RESET	(1 << 2)
+	#define SPEAR1340_SATA_CFG_PM_CLK_EN		(1 << 1)
+	#define SPEAR1340_PCIE_SATA_SEL_PCIE		(0)
+	#define SPEAR1340_PCIE_SATA_SEL_SATA		(1)
+	#define SPEAR1340_PCIE_SATA_CFG_MASK		0xF1F
+	#define SPEAR1340_PCIE_CFG_VAL	(SPEAR1340_PCIE_SATA_SEL_PCIE | \
+			SPEAR1340_PCIE_CFG_AUX_CLK_EN | \
+			SPEAR1340_PCIE_CFG_CORE_CLK_EN | \
+			SPEAR1340_PCIE_CFG_POWERUP_RESET | \
+			SPEAR1340_PCIE_CFG_DEVICE_PRESENT)
+	#define SPEAR1340_SATA_CFG_VAL	(SPEAR1340_PCIE_SATA_SEL_SATA | \
+			SPEAR1340_SATA_CFG_PM_CLK_EN | \
+			SPEAR1340_SATA_CFG_POWERUP_RESET | \
+			SPEAR1340_SATA_CFG_RX_CLK_EN | \
+			SPEAR1340_SATA_CFG_TX_CLK_EN)
+
+#define SPEAR1340_PCIE_MIPHY_CFG		0x428
+	#define SPEAR1340_MIPHY_OSC_BYPASS_EXT		(1 << 31)
+	#define SPEAR1340_MIPHY_CLK_REF_DIV2		(1 << 27)
+	#define SPEAR1340_MIPHY_CLK_REF_DIV4		(2 << 27)
+	#define SPEAR1340_MIPHY_CLK_REF_DIV8		(3 << 27)
+	#define SPEAR1340_MIPHY_PLL_RATIO_TOP(x)	(x << 0)
+	#define SPEAR1340_PCIE_MIPHY_CFG_MASK		0xF80000FF
+	#define SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA \
+			(SPEAR1340_MIPHY_OSC_BYPASS_EXT | \
+			SPEAR1340_MIPHY_CLK_REF_DIV2 | \
+			SPEAR1340_MIPHY_PLL_RATIO_TOP(60))
+	#define SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA_25M_CRYSTAL_CLK \
+			(SPEAR1340_MIPHY_PLL_RATIO_TOP(120))
+	#define SPEAR1340_PCIE_SATA_MIPHY_CFG_PCIE \
+			(SPEAR1340_MIPHY_OSC_BYPASS_EXT | \
+			SPEAR1340_MIPHY_PLL_RATIO_TOP(25))
+
+struct spear13xx_cfg_priv {
+	struct regmap		*misc;
+};
+
+/* SATA device registration */
+static void spear1340_sata_miphy_init(struct spear13xx_cfg_priv *cfgpriv)
+{
+	regmap_update_bits(cfgpriv->misc, SPEAR1340_PCIE_SATA_CFG,
+			SPEAR1340_PCIE_SATA_CFG_MASK, SPEAR1340_SATA_CFG_VAL);
+	regmap_update_bits(cfgpriv->misc, SPEAR1340_PCIE_MIPHY_CFG,
+			SPEAR1340_PCIE_MIPHY_CFG_MASK,
+			SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA_25M_CRYSTAL_CLK);
+	/* Switch on sata power domain */
+	regmap_update_bits(cfgpriv->misc, SPEAR1340_PCM_CFG,
+			SPEAR1340_PCM_CFG_SATA_POWER_EN,
+			SPEAR1340_PCM_CFG_SATA_POWER_EN);
+	msleep(20);

Quoted 8 lines

drivers/mfd/spear13xx-cfg.c line 97

Quoted 5 lines

drivers/mfd/spear13xx-cfg.c new file +5 -0

+	/* Disable PCIE SATA Controller reset */
+	regmap_update_bits(cfgpriv->misc, SPEAR1340_PERIP1_SW_RST,
+			SPEAR1340_PERIP1_SW_RST_SATA, 0);
+	msleep(20);
+}

Looking at the actual code now, this very much looks like it ought to be a "phy" driver and get put in drivers/phy/.

Actually these registers are part of common system configurations register space (called as misc space) for SPEAr SOC. So we opted for syscon framework.

PHY registers space starts from 0xEB800000, which can be programmed for various phy specific functions like power management, tx/rx settings, comparator settings etc. In most of the cases phy works with default settings, however there are few exceptions for which we will be adding a phy driver for further improvement of SPEAr drivers.

Quoted 9 lines · Pratyush Anand

Quoted 4 lines · Arnd Bergmann

On Thursday 23 January 2014, Mohit Kumar wrote:

I assume you'd want a phandle pointing to the syscon device in here as well?

Since there is only one syscon device in the whole DT, so do I really need to add phandle? Currently I am using syscon_regmap_lookup_by_compatible to find syscon device.

I'd much rather use syscon_regmap_lookup_by_phandle than syscon_regmap_lookup_by_compatible, all the time, since this makes the relationship between the devices explicit.

The phandle method also allows you to pass regmap indexes in the same property, which can be handy if two variants of the chip have the same registers at a different offset.

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

drivers/mfd/spear13xx-cfg.c new file +2 -0

+/* SPEAr1340 Registers */
+/* Power Management Registers */

Quoted 6 lines

Quoted 2 lines

Looking at the actual code now, this very much looks like it ought to be a "phy" driver and get put in drivers/phy/.

Actually these registers are part of common system configurations register space (called as misc space) for SPEAr SOC. So we opted for syscon framework.

The use of syscon for this is good, I have no objection to that, and was suggesting that you create a logical "phy" device that uses the misc syscon device as a backend.

Quoted 6 lines

PHY registers space starts from 0xEB800000, which can be programmed for various phy specific functions like power management, tx/rx settings, comparator settings etc. In most of the cases phy works with default settings, however there are few exceptions for which we will be adding a phy driver for further improvement of SPEAr drivers.

I see. So while the code you have here could be expressed as a phy driver by itself, there is another part of the SoC that controls the actual phy. How about if you add the phy device node to DT, and write a driver that doesn't actually program the phy registers for now, but does contain the code that you have posted here. That would give you flexibility for future extensions and at the same time let you remove all SPEAr specific code from the actual AHCI driver by using the generic ahci-platform driver.

Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

From: Pratyush Anand <[email protected]>

Follow dt clock naming convention for PCIe clocks.

Signed-off-by Pratyush Anand <[email protected]>

Tested-by Mohit Kumar <[email protected]>

Cc Viresh Kumar <[email protected]>

Cc spear-devel at list.st.com

Cc linux-arm-kernel at lists.infradead.org

Cc Arnd Bergmann <[email protected]>

---
 drivers/clk/spear/spear1310_clock.c |    6 +++---
 drivers/clk/spear/spear1340_clock.c |    2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

From: Pratyush Anand <[email protected]>

SPEAr13xx was using virtual address space 0xFE000000 to map physical address space 0xB3000000. pci_remap_io uses 0xFEE00000 as virtual address. So change 0xFE000000 to 0xF9000000.

Signed-off-by Pratyush Anand <[email protected]>

Tested-by Mohit Kumar <[email protected]>

Cc Viresh Kumar <[email protected]>

Cc spear-devel at list.st.com

Cc linux-arm-kernel at lists.infradead.org

Cc Arnd Bergmann <[email protected]>

Quoted 6 lines · Mohit Kumar

From: Pratyush Anand <[email protected]>

SPEAr13xx was using virtual address space 0xFE000000 to map physical address space 0xB3000000. pci_remap_io uses 0xFEE00000 as virtual address. So change 0xFE000000 to 0xF9000000.

Quoted 6 lines

Signed-off-by Pratyush Anand <[email protected]>

Tested-by Mohit Kumar <[email protected]>

Cc Viresh Kumar <[email protected]>

Cc spear-devel at list.st.com

Cc linux-arm-kernel at lists.infradead.org

Cc Arnd Bergmann <[email protected]>

Surely this is needed in backports, so please add stable at vger.kernel.org to the Cc list in the changeset text. Otherwise

Acked-by Arnd Bergmann <[email protected]>

From: Pratyush Anand <[email protected]>

SPEAr13xx was using virtual address space 0xFE000000 to map physical address space 0xB3000000. pci_remap_io uses 0xFEE00000 as virtual address. So change 0xFE000000 to 0xF9000000.

Signed-off-by Pratyush Anand <[email protected]>

Tested-by Mohit Kumar <[email protected]>

Cc Viresh Kumar <[email protected]>

Cc spear-devel at list.st.com

Cc linux-arm-kernel at lists.infradead.org

Cc Arnd Bergmann <[email protected]>

---
 arch/arm/mach-spear/include/mach/spear.h |    4 ++--
 arch/arm/mach-spear/spear13xx.c          |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

arch/arm/mach-spear/include/mach/spear.h +2 -2

index 5cdc53d..f2d6a01 100644
@@ -52,10 +52,10 @@
 #ifdef CONFIG_ARCH_SPEAR13XX
 
 #define PERIP_GRP2_BASE                                UL(0xB3000000)
-#define VA_PERIP_GRP2_BASE                     IOMEM(0xFE000000)
+#define VA_PERIP_GRP2_BASE                     IOMEM(0xF9000000)
 #define MCIF_SDHCI_BASE                                UL(0xB3000000)
 #define SYSRAM0_BASE                           UL(0xB3800000)
-#define VA_SYSRAM0_BASE                                IOMEM(0xFE800000)
+#define VA_SYSRAM0_BASE                                IOMEM(0xF9800000)
 #define SYS_LOCATION                           (VA_SYSRAM0_BASE + 0x600)
 
 #define PERIP_GRP1_BASE                                UL(0xE0000000)

arch/arm/mach-spear/spear13xx.c +0 -0

index 7aa6e8c..20ce885 100644

Quoted 43 lines · Mohit Kumar

From: Pratyush Anand <[email protected]>

SPEAr13xx was using virtual address space 0xFE000000 to map physical address space 0xB3000000. pci_remap_io uses 0xFEE00000 as virtual address. So change 0xFE000000 to 0xF9000000.

Signed-off-by Pratyush Anand <[email protected]>

Tested-by Mohit Kumar <[email protected]>

Cc Viresh Kumar <[email protected]>

Cc spear-devel at list.st.com

Cc linux-arm-kernel at lists.infradead.org

Cc Arnd Bergmann <[email protected]>

---
 arch/arm/mach-spear/include/mach/spear.h |    4 ++--
 arch/arm/mach-spear/spear13xx.c          |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

arch/arm/mach-spear/include/mach/spear.h +2 -2

index 5cdc53d..f2d6a01 100644
@@ -52,10 +52,10 @@
 #ifdef CONFIG_ARCH_SPEAR13XX
 
 #define PERIP_GRP2_BASE                                UL(0xB3000000)
-#define VA_PERIP_GRP2_BASE                     IOMEM(0xFE000000)
+#define VA_PERIP_GRP2_BASE                     IOMEM(0xF9000000)
 #define MCIF_SDHCI_BASE                                UL(0xB3000000)
 #define SYSRAM0_BASE                           UL(0xB3800000)
-#define VA_SYSRAM0_BASE                                IOMEM(0xFE800000)
+#define VA_SYSRAM0_BASE                                IOMEM(0xF9800000)
 #define SYS_LOCATION                           (VA_SYSRAM0_BASE + 0x600)
 
 #define PERIP_GRP1_BASE                                UL(0xE0000000)

arch/arm/mach-spear/spear13xx.c +0 -1

index 7aa6e8c..20ce885 100644
@@ -52,10 +52,10 @@ void __init spear13xx_l2x0_init(void)
 /*
  * Following will create 16MB static virtual/physical mappings
  * PHYSICAL            VIRTUAL
- * 0xB3000000          0xFE000000

arch/arm/mach-spear/spear13xx.c void __init spear13xx_l2x0_init(void) · line 58

Quoted 4 lines

arch/arm/mach-spear/spear13xx.c +1 -0

  * 0xE0000000          0xFD000000
  * 0xEC000000          0xFC000000
  * 0xED000000          0xFB000000
+ * 0xB3000000          0xF9000000

Why have you moved this to bottom of list? It was probably kept in increasing order and so please keep the same.

Other than that:

Acked-by Viresh Kumar <[email protected]>

-----Original Message-----
From: Mohit Kumar [mailto:[email protected]]
Sent: Thursday, January 23, 2014 7:33 PM
Cc: Pratyush Anand; Mohit Kumar; Jingoo Han; Viresh Kumar; [email protected]; linux- [email protected]
Subject: [PATCH V2 7/8] pcie: SPEAr13xx: Add designware pcie support

From: Pratyush Anand <[email protected]>

SPEAr1310 and SPEAr1340 SOC uses designware PCIe controller. Add SPEAr13xx PCIe driver based on designware controller driver.

SPEAr1310 has 3 PCIe ports and SPEAr1340 has 1, which are multiplexed with ahci/sata pins. By default evaluation board of both controller works for ahci mode.
To use these patches on SPEAr1340/1310 evaluation board, do the necessary modifications on board and enable (okay) pcie from respective evb dtsi file.

Signed-off-by Pratyush Anand <[email protected]>

Signed-off-by Mohit Kumar <[email protected]>

Cc Mohit Kumar <[email protected]>

Cc Jingoo Han <[email protected]>

Cc Viresh Kumar <[email protected]>

Cc [email protected]

Cc [email protected]

---
 arch/arm/boot/dts/spear13xx.dtsi     |   53 +++++-
 arch/arm/configs/spear13xx_defconfig |    2 +
 arch/arm/mach-spear/Kconfig          |    1 +
 drivers/mfd/spear13xx-cfg.c          |  205 ++++++++++++++++++
 drivers/pci/host/Kconfig             |    5 +
 drivers/pci/host/Makefile            |    1 +
 drivers/pci/host/pcie-spear13xx.c    |  394 ++++++++++++++++++++++++++++++++++

 7 files changed, 659 insertions(+), 2 deletions(-)
 create mode 100644 drivers/pci/host/pcie-spear13xx.c

Quoted 3 lines

-----Orig,

Quoted 25 lines

From: Mohit Kumar [mailto:[email protected]]
Sent: Thursday, January 23, 2014 7:33 PM
Cc: Pratyush Anand; Mohit Kumar; Jingoo Han; Viresh Kumar; [email protected]; linux- [email protected]
Subject: [PATCH V2 7/8] pcie: SPEAr13xx: Add designware pcie support

From: Pratyush Anand <[email protected]>

SPEAr1310 and SPEAr1340 SOC uses designware PCIe controller. Add SPEAr13xx PCIe driver based on designware controller driver.

SPEAr1310 has 3 PCIe ports and SPEAr1340 has 1, which are multiplexed with ahci/sata pins. By default evaluation board of both controller works for ahci mode.
To use these patches on SPEAr1340/1310 evaluation board, do the necessary modifications on board and enable (okay) pcie from respective evb dtsi file.

Signed-off-by Pratyush Anand <[email protected]>

Signed-off-by Mohit Kumar <[email protected]>

Cc Mohit Kumar <[email protected]>

Cc Jingoo Han <[email protected]>

Cc Viresh Kumar <[email protected]>

Cc [email protected]

Cc [email protected]

Quoted 8 lines

---
 arch/arm/boot/dts/spear13xx.dtsi     |   53 +++++-
 arch/arm/configs/spear13xx_defconfig |    2 +
 arch/arm/mach-spear/Kconfig          |    1 +
 drivers/mfd/spear13xx-cfg.c          |  205 ++++++++++++++++++
 drivers/pci/host/Kconfig             |    5 +
 drivers/pci/host/Makefile            |    1 +
 drivers/pci/host/pcie-spear13xx.c    |  394 ++++++++++++++++++++++++++++++++++

(+cc Thierry Reding)

How about changing the file name to 'pci-spear13xx.c'? Now, all PCI host drivers are using the prefix 'pci-', not 'pcie-'.

According to the Thierry Reding's comment,
"I think we should keep these sorted alphabetically. Also Tegra and Marvell are PCIe controllers but they still use the pci- prefix instead of pcie-. Perhaps it'd be good to keep consistency here? I initially chose pci- because from a software point of view it doesn't matter all that much whether it's PCI or PCIe and because the drivers are part of the PCI subsystem. However if Exynos now uses the pcie- prefix it makes it look like Tegra and Marvell are plain old PCI."
(https://groups.google.com/forum/#!msg/linux.kernel/qtimJoNSc3w/_1aayHaG54YJ)

If there is a special reason, please let us know.

Other things look good. :-)
Thank you.

Best regards,
Jingoo Han

Quoted 2 lines

 7 files changed, 659 insertions(+), 2 deletions(-)
 create mode 100644 drivers/pci/host/pcie-spear13xx.c