Re: [U-Boot] [PATCH v2] pci: layerscape: remove multiple definitions of SVR
>-Original Message-
>From: U-Boot On Behalf Of Priyanka Jain
>Sent: Friday, October 18, 2019 10:46 AM
>To: Pankaj Bansal ; Xiaowei Bao
>; Tom Rini ; Z.q. Hou
>
>Cc: u-boot@lists.denx.de
>Subject: Re: [U-Boot] [PATCH v2] pci: layerscape: remove multiple definitions
>of SVR
>
>
>
>>-Original Message-
>>From: Pankaj Bansal
>>Sent: Wednesday, October 16, 2019 3:43 PM
>>To: Priyanka Jain ; Xiaowei Bao
>>; Tom Rini ; Z.q. Hou
>>
>>Cc: u-boot@lists.denx.de; Pankaj Bansal
>>Subject: [PATCH v2] pci: layerscape: remove multiple definitions of SVR
>>
>>SVR values for various nxp SOCs are defined in asm/arch/soc.h we can
>>use these values in any peripheral driver.
>>we need not to redefine these values in peripheral driver, as this
>>becomes difficult to manage (add or change new values)
>>
>>Signed-off-by: Pankaj Bansal
>>---
>>
>>Notes:
>>V2:
>>- fixed compilation errors in LS1021A based targets by adding SVR checking
>> for layerscape series SOCs under LAYERSCAPE Macro.
>>[Dependencies]
>>-
>>https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
>>work.ozlabs.org%2Fpatch%2F1177732%2F&data=02%7C01%7Cpriyanka.j
>ain%4
>>0nxp.com%7C086fd8c5509d455a0eb708d7538a5cb3%7C686ea1d3bc2b4c6fa9
>2cd99c5
>>c301635%7C0%7C0%7C637069726040396790&sdata=21jhOzjm27k4aQW
>AK9BYhcoI
>>l6M78KAV3t9ImmlgSdw%3D&reserved=0
>>
>> drivers/pci/pcie_layerscape.c | 18 --
>> drivers/pci/pcie_layerscape.h | 9 +
>> drivers/pci/pcie_layerscape_fixup.c | 16 ++--
>> 3 files changed, 23 insertions(+), 20 deletions(-)
>>
>>diff --git a/drivers/pci/pcie_layerscape.c
>>b/drivers/pci/pcie_layerscape.c index
>>5ad7c28773..12357127a8 100644
>>--- a/drivers/pci/pcie_layerscape.c
>>+++ b/drivers/pci/pcie_layerscape.c
>>@@ -1,6 +1,6 @@
>> // SPDX-License-Identifier: GPL-2.0+
>> /*
>>- * Copyright 2017 NXP
>>+ * Copyright 2017, 2019 NXP
>Were there changes in this file in 2018? If yes, please update to 2017-2019
>> * Copyright 2014-2015 Freescale Semiconductor, Inc.
>> * Layerscape PCIe driver
>> */
>>@@ -15,6 +15,7 @@
>> #if defined(CONFIG_FSL_LSCH2) || defined(CONFIG_FSL_LSCH3) || \
>> defined(CONFIG_ARM)
>> #include
>>+#include
>> #endif
>> #include "pcie_layerscape.h"
>>
>>@@ -56,7 +57,7 @@ static int ls_pcie_ltssm(struct ls_pcie *pcie)
>> uint svr;
>>
>> svr = get_svr();
>>- if (((svr >> SVR_VAR_PER_SHIFT) & SVR_LS102XA_MASK) ==
>>SVR_LS102XA) {
>>+ if ((SVR_DEV(svr) & SVR_LS102XA_MASK) == SVR_LS102XA) {
>Is "SVR_LS102XA_MASK" LS102XA specific? Can we make it generic across
>other SoCs?
>
>> state = ctrl_readl(pcie, LS1021_PEXMSCPORTSR(pcie->idx));
>> state = (state >> LS1021_LTSSM_STATE_SHIFT) &
>LTSSM_STATE_MASK;
>> } else {
>>@@ -149,7 +150,7 @@ static void ls_pcie_setup_atu(struct ls_pcie *pcie)
>> uint svr;
>>
>> svr = get_svr();
>>- if (((svr >> SVR_VAR_PER_SHIFT) & SVR_LS102XA_MASK) ==
>>SVR_LS102XA) {
>>+ if ((SVR_DEV(svr) & SVR_LS102XA_MASK) == SVR_LS102XA) {
>Same as above
>> offset = LS1021_PCIE_SPACE_OFFSET +
>> LS1021_PCIE_SPACE_SIZE * pcie->idx;
>> }
>>@@ -172,7 +173,8 @@ static void ls_pcie_setup_atu(struct ls_pcie *pcie)
>> idx = PCIE_ATU_REGION_INDEX1 + 1;
>>
>> /* Fix the pcie memory map for LS2088A series SoCs */
>>- svr = (svr >> SVR_VAR_PER_SHIFT) & 0xFE;
>>+#if defined(CONFIG_FSL_LAYERSCAPE)
One more comment: use device tree instead of CONFIG
--priyankajain
>>+ svr = SVR_SOC_VER(svr);
>Please confirm that svr vaue is used only inside "CONFIG_FSL_LAYERSCAPE"
>> if (svr == SVR_LS2088A || svr == SVR_LS2084A ||
>> svr == SVR_LS2048A || svr == SVR_LS2044A ||
>> svr == SVR_LS2081A || svr == SVR_LS2041A) { @@ -192,6 +194,7
>@@
>>static void ls_pcie_setup_atu(struct ls_pcie *pcie)
>> LS2088A_PCIE1_PHYS_ADDR +
>> LS2088A_PCIE_PHYS_SIZE * pcie->idx;
>> }
>>+#endif /* CONFIG_FSL_LAYERSCAPE */
>>
>> if (io)
>> /* ATU : OUTBOUND : IO */
>>@@ -446,9 +449,7 @@ static int ls_pcie_probe(struct udevice *dev)
>> const void *fdt = gd->fdt_blob;
>>
Re: [U-Boot] [PATCH v2] pci: layerscape: remove multiple definitions of SVR
>-Original Message-
>From: Pankaj Bansal
>Sent: Wednesday, October 16, 2019 3:43 PM
>To: Priyanka Jain ; Xiaowei Bao
>; Tom Rini ; Z.q. Hou
>
>Cc: u-boot@lists.denx.de; Pankaj Bansal
>Subject: [PATCH v2] pci: layerscape: remove multiple definitions of SVR
>
>SVR values for various nxp SOCs are defined in asm/arch/soc.h we can use
>these values in any peripheral driver.
>we need not to redefine these values in peripheral driver, as this becomes
>difficult to manage (add or change new values)
>
>Signed-off-by: Pankaj Bansal
>---
>
>Notes:
>V2:
>- fixed compilation errors in LS1021A based targets by adding SVR checking
> for layerscape series SOCs under LAYERSCAPE Macro.
>[Dependencies]
>- https://patchwork.ozlabs.org/patch/1177732/
>
> drivers/pci/pcie_layerscape.c | 18 --
> drivers/pci/pcie_layerscape.h | 9 +
> drivers/pci/pcie_layerscape_fixup.c | 16 ++--
> 3 files changed, 23 insertions(+), 20 deletions(-)
>
>diff --git a/drivers/pci/pcie_layerscape.c b/drivers/pci/pcie_layerscape.c
>index
>5ad7c28773..12357127a8 100644
>--- a/drivers/pci/pcie_layerscape.c
>+++ b/drivers/pci/pcie_layerscape.c
>@@ -1,6 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0+
> /*
>- * Copyright 2017 NXP
>+ * Copyright 2017, 2019 NXP
Were there changes in this file in 2018? If yes, please update to 2017-2019
> * Copyright 2014-2015 Freescale Semiconductor, Inc.
> * Layerscape PCIe driver
> */
>@@ -15,6 +15,7 @@
> #if defined(CONFIG_FSL_LSCH2) || defined(CONFIG_FSL_LSCH3) || \
> defined(CONFIG_ARM)
> #include
>+#include
> #endif
> #include "pcie_layerscape.h"
>
>@@ -56,7 +57,7 @@ static int ls_pcie_ltssm(struct ls_pcie *pcie)
> uint svr;
>
> svr = get_svr();
>- if (((svr >> SVR_VAR_PER_SHIFT) & SVR_LS102XA_MASK) ==
>SVR_LS102XA) {
>+ if ((SVR_DEV(svr) & SVR_LS102XA_MASK) == SVR_LS102XA) {
Is "SVR_LS102XA_MASK" LS102XA specific? Can we make it generic across other
SoCs?
> state = ctrl_readl(pcie, LS1021_PEXMSCPORTSR(pcie->idx));
> state = (state >> LS1021_LTSSM_STATE_SHIFT) &
>LTSSM_STATE_MASK;
> } else {
>@@ -149,7 +150,7 @@ static void ls_pcie_setup_atu(struct ls_pcie *pcie)
> uint svr;
>
> svr = get_svr();
>- if (((svr >> SVR_VAR_PER_SHIFT) & SVR_LS102XA_MASK) ==
>SVR_LS102XA) {
>+ if ((SVR_DEV(svr) & SVR_LS102XA_MASK) == SVR_LS102XA) {
Same as above
> offset = LS1021_PCIE_SPACE_OFFSET +
>LS1021_PCIE_SPACE_SIZE * pcie->idx;
> }
>@@ -172,7 +173,8 @@ static void ls_pcie_setup_atu(struct ls_pcie *pcie)
> idx = PCIE_ATU_REGION_INDEX1 + 1;
>
> /* Fix the pcie memory map for LS2088A series SoCs */
>- svr = (svr >> SVR_VAR_PER_SHIFT) & 0xFE;
>+#if defined(CONFIG_FSL_LAYERSCAPE)
>+ svr = SVR_SOC_VER(svr);
Please confirm that svr vaue is used only inside "CONFIG_FSL_LAYERSCAPE"
> if (svr == SVR_LS2088A || svr == SVR_LS2084A ||
> svr == SVR_LS2048A || svr == SVR_LS2044A ||
> svr == SVR_LS2081A || svr == SVR_LS2041A) { @@ -192,6 +194,7
>@@ static void ls_pcie_setup_atu(struct ls_pcie *pcie)
>LS2088A_PCIE1_PHYS_ADDR +
>LS2088A_PCIE_PHYS_SIZE * pcie->idx;
> }
>+#endif /* CONFIG_FSL_LAYERSCAPE */
>
> if (io)
> /* ATU : OUTBOUND : IO */
>@@ -446,9 +449,7 @@ static int ls_pcie_probe(struct udevice *dev)
> const void *fdt = gd->fdt_blob;
> int node = dev_of_offset(dev);
> u16 link_sta;
>- uint svr;
> int ret;
>- fdt_size_t cfg_size;
>
> pcie->bus = dev;
>
>@@ -501,6 +502,10 @@ static int ls_pcie_probe(struct udevice *dev)
> return ret;
> }
>
>+#if defined(CONFIG_FSL_LAYERSCAPE)
>+ uint svr;
>+ fdt_size_t cfg_size;
>+
As per u-boot coding style the preference, is to define variables in start of
function
> /*
>* Fix the pcie memory map address and PF control registers address
>* for LS2088A series SoCs
>@@ -516,6 +521,7 @@ static int ls_pcie_probe(struct udevice *dev)
> pcie->cfg_res.end = pcie->cfg_res.start + cfg_size;
> pcie->ctrl = pcie->lut + 0x4;
> }
>+#endif /* CONFIG_FSL_LAYERSCAPE */
>
> pcie->cfg0 = map_physmem(pcie->cfg_res.start,
>fdt_resource_size(&pcie->cfg_res),
>diff --git a/drivers/pci/pcie_layerscape.h b/drivers/pci/pcie_layerscape.h
>index
>ddfbba6538..9a19993568 100644
>--- a/drivers/pci/pcie_layerscape.h
>+++ b/drivers/pci/pcie_layerscape.h
>@@ -1,6 +1,6 @@
> /* SPDX-License-Identifier: GPL-2.0+ */
> /*
>- * Copyright 2017 NXP
>+ * Copyright 2017, 2019 NXP
Same as above
> * Copyright 2014-2015 Freescale Semiconductor, Inc.
> * Layerscape PCIe driver
> */
>@@ -111,14 +111,7 @@
> #define PCIE_CS2_OFFSET 0x1000 /* For PCIe without SR-IOV */
>
> #define SVR_LS102XA 0
>-#de
[U-Boot] [PATCH v2] pci: layerscape: remove multiple definitions of SVR
SVR values for various nxp SOCs are defined in asm/arch/soc.h
we can use these values in any peripheral driver.
we need not to redefine these values in peripheral driver, as this
becomes difficult to manage (add or change new values)
Signed-off-by: Pankaj Bansal
---
Notes:
V2:
- fixed compilation errors in LS1021A based targets by adding SVR checking
for layerscape series SOCs under LAYERSCAPE Macro.
[Dependencies]
- https://patchwork.ozlabs.org/patch/1177732/
drivers/pci/pcie_layerscape.c | 18 --
drivers/pci/pcie_layerscape.h | 9 +
drivers/pci/pcie_layerscape_fixup.c | 16 ++--
3 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/drivers/pci/pcie_layerscape.c b/drivers/pci/pcie_layerscape.c
index 5ad7c28773..12357127a8 100644
--- a/drivers/pci/pcie_layerscape.c
+++ b/drivers/pci/pcie_layerscape.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0+
/*
- * Copyright 2017 NXP
+ * Copyright 2017, 2019 NXP
* Copyright 2014-2015 Freescale Semiconductor, Inc.
* Layerscape PCIe driver
*/
@@ -15,6 +15,7 @@
#if defined(CONFIG_FSL_LSCH2) || defined(CONFIG_FSL_LSCH3) || \
defined(CONFIG_ARM)
#include
+#include
#endif
#include "pcie_layerscape.h"
@@ -56,7 +57,7 @@ static int ls_pcie_ltssm(struct ls_pcie *pcie)
uint svr;
svr = get_svr();
- if (((svr >> SVR_VAR_PER_SHIFT) & SVR_LS102XA_MASK) == SVR_LS102XA) {
+ if ((SVR_DEV(svr) & SVR_LS102XA_MASK) == SVR_LS102XA) {
state = ctrl_readl(pcie, LS1021_PEXMSCPORTSR(pcie->idx));
state = (state >> LS1021_LTSSM_STATE_SHIFT) & LTSSM_STATE_MASK;
} else {
@@ -149,7 +150,7 @@ static void ls_pcie_setup_atu(struct ls_pcie *pcie)
uint svr;
svr = get_svr();
- if (((svr >> SVR_VAR_PER_SHIFT) & SVR_LS102XA_MASK) == SVR_LS102XA) {
+ if ((SVR_DEV(svr) & SVR_LS102XA_MASK) == SVR_LS102XA) {
offset = LS1021_PCIE_SPACE_OFFSET +
LS1021_PCIE_SPACE_SIZE * pcie->idx;
}
@@ -172,7 +173,8 @@ static void ls_pcie_setup_atu(struct ls_pcie *pcie)
idx = PCIE_ATU_REGION_INDEX1 + 1;
/* Fix the pcie memory map for LS2088A series SoCs */
- svr = (svr >> SVR_VAR_PER_SHIFT) & 0xFE;
+#if defined(CONFIG_FSL_LAYERSCAPE)
+ svr = SVR_SOC_VER(svr);
if (svr == SVR_LS2088A || svr == SVR_LS2084A ||
svr == SVR_LS2048A || svr == SVR_LS2044A ||
svr == SVR_LS2081A || svr == SVR_LS2041A) {
@@ -192,6 +194,7 @@ static void ls_pcie_setup_atu(struct ls_pcie *pcie)
LS2088A_PCIE1_PHYS_ADDR +
LS2088A_PCIE_PHYS_SIZE * pcie->idx;
}
+#endif /* CONFIG_FSL_LAYERSCAPE */
if (io)
/* ATU : OUTBOUND : IO */
@@ -446,9 +449,7 @@ static int ls_pcie_probe(struct udevice *dev)
const void *fdt = gd->fdt_blob;
int node = dev_of_offset(dev);
u16 link_sta;
- uint svr;
int ret;
- fdt_size_t cfg_size;
pcie->bus = dev;
@@ -501,6 +502,10 @@ static int ls_pcie_probe(struct udevice *dev)
return ret;
}
+#if defined(CONFIG_FSL_LAYERSCAPE)
+ uint svr;
+ fdt_size_t cfg_size;
+
/*
* Fix the pcie memory map address and PF control registers address
* for LS2088A series SoCs
@@ -516,6 +521,7 @@ static int ls_pcie_probe(struct udevice *dev)
pcie->cfg_res.end = pcie->cfg_res.start + cfg_size;
pcie->ctrl = pcie->lut + 0x4;
}
+#endif /* CONFIG_FSL_LAYERSCAPE */
pcie->cfg0 = map_physmem(pcie->cfg_res.start,
fdt_resource_size(&pcie->cfg_res),
diff --git a/drivers/pci/pcie_layerscape.h b/drivers/pci/pcie_layerscape.h
index ddfbba6538..9a19993568 100644
--- a/drivers/pci/pcie_layerscape.h
+++ b/drivers/pci/pcie_layerscape.h
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0+ */
/*
- * Copyright 2017 NXP
+ * Copyright 2017, 2019 NXP
* Copyright 2014-2015 Freescale Semiconductor, Inc.
* Layerscape PCIe driver
*/
@@ -111,14 +111,7 @@
#define PCIE_CS2_OFFSET0x1000 /* For PCIe without SR-IOV */
#define SVR_LS102XA0
-#define SVR_VAR_PER_SHIFT 8
#define SVR_LS102XA_MASK 0x700
-#define SVR_LS2088A0x870900
-#define SVR_LS2084A0x870910
-#define SVR_LS2048A0x870920
-#define SVR_LS2044A0x870930
-#define SVR_LS2081A0x870918
-#define SVR_LS2041A0x870914
/* LS1021a PCIE space */
#define LS1021_PCIE_SPACE_OFFSET 0x40ULL
diff --git a/drivers/pci/pcie_layerscape_fixup.c
b/drivers/pci/pcie_layerscape_fixup.c
index 089e031724..ea8c330c07 100644
--- a/drivers/pci/pcie_layerscape_fixup.c
+++ b/drivers/pci/pcie_layerscape_fixup.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0+
/*
- * Copyright 2017 NXP
+
