[PATCH v2 0/4] PCI: spear: Cleanups

[PATCH v2 0/4] PCI: spear: Cleanups

[Bjorn Helgaas]

  - Remove unused constants.  There's some documentation value in these,
    but they're only marginally useful to people without the spec, and
    I think it's preferable to rely on the spec directly to avoid
    transcription errors.

  - Pass device-specific struct to internal functions for consistency.

  - Tidy up usage of local struct device pointers.

Nothing here should change the behavior of the driver.

Changes from v1:
  I dropped the following patch because it was lot of churn for
  questionable benefit:
    PCI: spear: Name private struct pointer "spear13xx" consistently

---

Bjorn Helgaas (4):
      PCI: spear: Remove unused constants
      PCI: spear: Pass device-specific struct to internal functions
      PCI: spear: Reorder struct spear13xx_pcie
      PCI: spear: Clean up struct device usage


 drivers/pci/host/pcie-spear13xx.c |  108 +++++++------------------------------
 1 file changed, 20 insertions(+), 88 deletions(-)

[PATCH v2 1/4] PCI: spear: Remove unused constants

[Bjorn Helgaas]

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/pcie-spear13xx.c |   70 -------------------------------------
 1 file changed, 70 deletions(-)

diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
index 09aed85..73f17da 100644
--- a/drivers/pci/host/pcie-spear13xx.c
+++ b/drivers/pci/host/pcie-spear13xx.c
@@ -57,87 +57,17 @@ struct pcie_app_reg {
 };
 
 /* CR0 ID */
-#define RX_LANE_FLIP_EN_ID			0
-#define TX_LANE_FLIP_EN_ID			1
-#define SYS_AUX_PWR_DET_ID			2
 #define APP_LTSSM_ENABLE_ID			3
-#define SYS_ATTEN_BUTTON_PRESSED_ID		4
-#define SYS_MRL_SENSOR_STATE_ID			5
-#define SYS_PWR_FAULT_DET_ID			6
-#define SYS_MRL_SENSOR_CHGED_ID			7
-#define SYS_PRE_DET_CHGED_ID			8
-#define SYS_CMD_CPLED_INT_ID			9
-#define APP_INIT_RST_0_ID			11
-#define APP_REQ_ENTR_L1_ID			12
-#define APP_READY_ENTR_L23_ID			13
-#define APP_REQ_EXIT_L1_ID			14
-#define DEVICE_TYPE_EP				(0 << 25)
-#define DEVICE_TYPE_LEP				(1 << 25)
 #define DEVICE_TYPE_RC				(4 << 25)
-#define SYS_INT_ID				29
 #define MISCTRL_EN_ID				30
 #define REG_TRANSLATION_ENABLE			31
 
-/* CR1 ID */
-#define APPS_PM_XMT_TURNOFF_ID			2
-#define APPS_PM_XMT_PME_ID			5
-
 /* CR3 ID */
-#define XMLH_LTSSM_STATE_DETECT_QUIET		0x00
-#define XMLH_LTSSM_STATE_DETECT_ACT		0x01
-#define XMLH_LTSSM_STATE_POLL_ACTIVE		0x02
-#define XMLH_LTSSM_STATE_POLL_COMPLIANCE	0x03
-#define XMLH_LTSSM_STATE_POLL_CONFIG		0x04
-#define XMLH_LTSSM_STATE_PRE_DETECT_QUIET	0x05
-#define XMLH_LTSSM_STATE_DETECT_WAIT		0x06
-#define XMLH_LTSSM_STATE_CFG_LINKWD_START	0x07
-#define XMLH_LTSSM_STATE_CFG_LINKWD_ACEPT	0x08
-#define XMLH_LTSSM_STATE_CFG_LANENUM_WAIT	0x09
-#define XMLH_LTSSM_STATE_CFG_LANENUM_ACEPT	0x0A
-#define XMLH_LTSSM_STATE_CFG_COMPLETE		0x0B
-#define XMLH_LTSSM_STATE_CFG_IDLE		0x0C
-#define XMLH_LTSSM_STATE_RCVRY_LOCK		0x0D
-#define XMLH_LTSSM_STATE_RCVRY_SPEED		0x0E
-#define XMLH_LTSSM_STATE_RCVRY_RCVRCFG		0x0F
-#define XMLH_LTSSM_STATE_RCVRY_IDLE		0x10
-#define XMLH_LTSSM_STATE_L0			0x11
-#define XMLH_LTSSM_STATE_L0S			0x12
-#define XMLH_LTSSM_STATE_L123_SEND_EIDLE	0x13
-#define XMLH_LTSSM_STATE_L1_IDLE		0x14
-#define XMLH_LTSSM_STATE_L2_IDLE		0x15
-#define XMLH_LTSSM_STATE_L2_WAKE		0x16
-#define XMLH_LTSSM_STATE_DISABLED_ENTRY		0x17
-#define XMLH_LTSSM_STATE_DISABLED_IDLE		0x18
-#define XMLH_LTSSM_STATE_DISABLED		0x19
-#define XMLH_LTSSM_STATE_LPBK_ENTRY		0x1A
-#define XMLH_LTSSM_STATE_LPBK_ACTIVE		0x1B
-#define XMLH_LTSSM_STATE_LPBK_EXIT		0x1C
-#define XMLH_LTSSM_STATE_LPBK_EXIT_TIMEOUT	0x1D
-#define XMLH_LTSSM_STATE_HOT_RESET_ENTRY	0x1E
-#define XMLH_LTSSM_STATE_HOT_RESET		0x1F
-#define XMLH_LTSSM_STATE_MASK			0x3F
 #define XMLH_LINK_UP				(1 << 6)
 
-/* CR4 ID */
-#define CFG_MSI_EN_ID				18
-
 /* CR6 */
-#define INTA_CTRL_INT				(1 << 7)
-#define INTB_CTRL_INT				(1 << 8)
-#define INTC_CTRL_INT				(1 << 9)
-#define INTD_CTRL_INT				(1 << 10)
 #define MSI_CTRL_INT				(1 << 26)
 
-/* CR19 ID */
-#define VEN_MSI_REQ_ID				11
-#define VEN_MSI_FUN_NUM_ID			8
-#define VEN_MSI_TC_ID				5
-#define VEN_MSI_VECTOR_ID			0
-#define VEN_MSI_REQ_EN		((u32)0x1 << VEN_MSI_REQ_ID)
-#define VEN_MSI_FUN_NUM_MASK	((u32)0x7 << VEN_MSI_FUN_NUM_ID)
-#define VEN_MSI_TC_MASK		((u32)0x7 << VEN_MSI_TC_ID)
-#define VEN_MSI_VECTOR_MASK	((u32)0x1F << VEN_MSI_VECTOR_ID)
-
 #define EXP_CAP_ID_OFFSET			0x70
 
 #define to_spear13xx_pcie(x)	container_of(x, struct spear13xx_pcie, pp)

[PATCH v2 2/4] PCI: spear: Pass device-specific struct to internal functions

[Bjorn Helgaas]

Only interfaces used from outside the driver, e.g., those called by the
DesignWare core, need to accept pointers to the generic struct pcie_port.
Internal interfaces can accept pointers to the device-specific struct,
which makes them more straightforward.  No functional change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/pcie-spear13xx.c |   29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
index 73f17da..26e905d 100644
--- a/drivers/pci/host/pcie-spear13xx.c
+++ b/drivers/pci/host/pcie-spear13xx.c
@@ -72,11 +72,11 @@ struct pcie_app_reg {
 
 #define to_spear13xx_pcie(x)	container_of(x, struct spear13xx_pcie, pp)
 
-static int spear13xx_pcie_establish_link(struct pcie_port *pp)
+static int spear13xx_pcie_establish_link(struct spear13xx_pcie *spear13xx_pcie)
 {
-	u32 val;
-	struct spear13xx_pcie *spear13xx_pcie = to_spear13xx_pcie(pp);
+	struct pcie_port *pp = &spear13xx_pcie->pp;
 	struct pcie_app_reg *app_reg = spear13xx_pcie->app_base;
+	u32 val;
 	u32 exp_cap_off = EXP_CAP_ID_OFFSET;
 
 	if (dw_pcie_link_up(pp)) {
@@ -133,9 +133,9 @@ static int spear13xx_pcie_establish_link(struct pcie_port *pp)
 
 static irqreturn_t spear13xx_pcie_irq_handler(int irq, void *arg)
 {
-	struct pcie_port *pp = arg;
-	struct spear13xx_pcie *spear13xx_pcie = to_spear13xx_pcie(pp);
+	struct spear13xx_pcie *spear13xx_pcie = arg;
 	struct pcie_app_reg *app_reg = spear13xx_pcie->app_base;
+	struct pcie_port *pp = &spear13xx_pcie->pp;
 	unsigned int status;
 
 	status = readl(&app_reg->int_sts);
@@ -150,9 +150,9 @@ static irqreturn_t spear13xx_pcie_irq_handler(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
-static void spear13xx_pcie_enable_interrupts(struct pcie_port *pp)
+static void spear13xx_pcie_enable_interrupts(struct spear13xx_pcie *spear13xx_pcie)
 {
-	struct spear13xx_pcie *spear13xx_pcie = to_spear13xx_pcie(pp);
+	struct pcie_port *pp = &spear13xx_pcie->pp;
 	struct pcie_app_reg *app_reg = spear13xx_pcie->app_base;
 
 	/* Enable MSI interrupt */
@@ -176,8 +176,10 @@ static int spear13xx_pcie_link_up(struct pcie_port *pp)
 
 static void spear13xx_pcie_host_init(struct pcie_port *pp)
 {
-	spear13xx_pcie_establish_link(pp);
-	spear13xx_pcie_enable_interrupts(pp);
+	struct spear13xx_pcie *spear13xx_pcie = to_spear13xx_pcie(pp);
+
+	spear13xx_pcie_establish_link(spear13xx_pcie);
+	spear13xx_pcie_enable_interrupts(spear13xx_pcie);
 }
 
 static struct pcie_host_ops spear13xx_pcie_host_ops = {
@@ -185,9 +187,10 @@ static struct pcie_host_ops spear13xx_pcie_host_ops = {
 	.host_init = spear13xx_pcie_host_init,
 };
 
-static int spear13xx_add_pcie_port(struct pcie_port *pp,
-					 struct platform_device *pdev)
+static int spear13xx_add_pcie_port(struct spear13xx_pcie *spear13xx_pcie,
+				   struct platform_device *pdev)
 {
+	struct pcie_port *pp = &spear13xx_pcie->pp;
 	struct device *dev = &pdev->dev;
 	int ret;
 
@@ -198,7 +201,7 @@ static int spear13xx_add_pcie_port(struct pcie_port *pp,
 	}
 	ret = devm_request_irq(dev, pp->irq, spear13xx_pcie_irq_handler,
 			       IRQF_SHARED | IRQF_NO_THREAD,
-			       "spear1340-pcie", pp);
+			       "spear1340-pcie", spear13xx_pcie);
 	if (ret) {
 		dev_err(dev, "failed to request irq %d\n", pp->irq);
 		return ret;
@@ -268,7 +271,7 @@ static int spear13xx_pcie_probe(struct platform_device *pdev)
 	if (of_property_read_bool(np, "st,pcie-is-gen1"))
 		spear13xx_pcie->is_gen1 = true;
 
-	ret = spear13xx_add_pcie_port(pp, pdev);
+	ret = spear13xx_add_pcie_port(spear13xx_pcie, pdev);
 	if (ret < 0)
 		goto fail_clk;
 

[PATCH v2 3/4] PCI: spear: Reorder struct spear13xx_pcie

[Bjorn Helgaas]

Reorder struct spear13xx_pcie to put generic fields first.  No functional
change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/pcie-spear13xx.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
index 26e905d..ecfb49e 100644
--- a/drivers/pci/host/pcie-spear13xx.c
+++ b/drivers/pci/host/pcie-spear13xx.c
@@ -25,10 +25,10 @@
 #include "pcie-designware.h"
 
 struct spear13xx_pcie {
+	struct pcie_port	pp;		/* DT dbi is pp.dbi_base */
 	void __iomem		*app_base;
 	struct phy		*phy;
 	struct clk		*clk;
-	struct pcie_port	pp;
 	bool			is_gen1;
 };
 

[PATCH v2 4/4] PCI: spear: Clean up struct device usage

[Bjorn Helgaas]

For consistency with other drivers, use the struct device pointer from
struct pcie_port whenever possible instead of relying on the
platform_device pointer.  No functional change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/pcie-spear13xx.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
index ecfb49e..3cf197b 100644
--- a/drivers/pci/host/pcie-spear13xx.c
+++ b/drivers/pci/host/pcie-spear13xx.c
@@ -191,7 +191,7 @@ static int spear13xx_add_pcie_port(struct spear13xx_pcie *spear13xx_pcie,
 				   struct platform_device *pdev)
 {
 	struct pcie_port *pp = &spear13xx_pcie->pp;
-	struct device *dev = &pdev->dev;
+	struct device *dev = pp->dev;
 	int ret;
 
 	pp->irq = platform_get_irq(pdev, 0);
@@ -221,10 +221,10 @@ static int spear13xx_add_pcie_port(struct spear13xx_pcie *spear13xx_pcie,
 
 static int spear13xx_pcie_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct spear13xx_pcie *spear13xx_pcie;
 	struct pcie_port *pp;
-	struct device *dev = &pdev->dev;
-	struct device_node *np = pdev->dev.of_node;
+	struct device_node *np = dev->of_node;
 	struct resource *dbi_base;
 	int ret;
 
@@ -256,7 +256,6 @@ static int spear13xx_pcie_probe(struct platform_device *pdev)
 	}
 
 	pp = &spear13xx_pcie->pp;
-
 	pp->dev = dev;
 
 	dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");

Re: [PATCH v2 0/4] PCI: spear: Cleanups

[Bjorn Helgaas]

On Wed, Oct 12, 2016 at 09:04:20AM -0500, Bjorn Helgaas wrote:
>   - Remove unused constants.  There's some documentation value in these,
>     but they're only marginally useful to people without the spec, and
>     I think it's preferable to rely on the spec directly to avoid
>     transcription errors.
> 
>   - Pass device-specific struct to internal functions for consistency.
> 
>   - Tidy up usage of local struct device pointers.
> 
> Nothing here should change the behavior of the driver.
> 
> Changes from v1:
>   I dropped the following patch because it was lot of churn for
>   questionable benefit:
>     PCI: spear: Name private struct pointer "spear13xx" consistently
> 
> ---
> 
> Bjorn Helgaas (4):
>       PCI: spear: Remove unused constants
>       PCI: spear: Pass device-specific struct to internal functions
>       PCI: spear: Reorder struct spear13xx_pcie
>       PCI: spear: Clean up struct device usage
> 
> 
>  drivers/pci/host/pcie-spear13xx.c |  108 +++++++------------------------------
>  1 file changed, 20 insertions(+), 88 deletions(-)

I applied these to pci/host-spear for v4.9.  I hope to ask Linus to
pull them tomorrow, so if you see any issues, let me know soon.

Re: [PATCH v2 0/4] PCI: spear: Cleanups

[Pratyush Anand]

On Wed, Oct 12, 2016 at 9:37 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Wed, Oct 12, 2016 at 09:04:20AM -0500, Bjorn Helgaas wrote:
>>   - Remove unused constants.  There's some documentation value in these,
>>     but they're only marginally useful to people without the spec, and
>>     I think it's preferable to rely on the spec directly to avoid
>>     transcription errors.
>>
>>   - Pass device-specific struct to internal functions for consistency.
>>
>>   - Tidy up usage of local struct device pointers.
>>
>> Nothing here should change the behavior of the driver.
>>
>> Changes from v1:
>>   I dropped the following patch because it was lot of churn for
>>   questionable benefit:
>>     PCI: spear: Name private struct pointer "spear13xx" consistently
>>
>> ---
>>
>> Bjorn Helgaas (4):
>>       PCI: spear: Remove unused constants
>>       PCI: spear: Pass device-specific struct to internal functions
>>       PCI: spear: Reorder struct spear13xx_pcie
>>       PCI: spear: Clean up struct device usage
>>
>>
>>  drivers/pci/host/pcie-spear13xx.c |  108 +++++++------------------------------
>>  1 file changed, 20 insertions(+), 88 deletions(-)
>
> I applied these to pci/host-spear for v4.9.  I hope to ask Linus to
> pull them tomorrow, so if you see any issues, let me know soon.

Bjorn, Thanks for the cleanup and sorry for late reply.

Patches were all fine. Anyway, they are already in Linus's tree.

I have no more SPEAr platform with me, and my working area has also
changed in last two years So, if someone would like to take over its
maintainership, I would be very happy.

~Pratyush