[U-Boot] [PATCH v2] pci: layerscape: remove multiple definitions of SVR
Priyanka Jain
priyanka.jain at nxp.com
Fri Oct 18 05:36:52 UTC 2019
>-----Original Message-----
>From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Priyanka Jain
>Sent: Friday, October 18, 2019 10:46 AM
>To: Pankaj Bansal <pankaj.bansal at nxp.com>; Xiaowei Bao
><xiaowei.bao at nxp.com>; Tom Rini <trini at konsulko.com>; Z.q. Hou
><zhiqiang.hou at nxp.com>
>Cc: u-boot at 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 <priyanka.jain at nxp.com>; Xiaowei Bao
>><xiaowei.bao at nxp.com>; Tom Rini <trini at konsulko.com>; Z.q. Hou
>><zhiqiang.hou at nxp.com>
>>Cc: u-boot at lists.denx.de; Pankaj Bansal <pankaj.bansal at nxp.com>
>>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 <pankaj.bansal at nxp.com>
>>---
>>
>>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 <asm/arch/clock.h>
>>+#include <asm/arch/soc.h>
>> #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) & 0xFFFFFE;
>>+#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;
>> 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 + 0x40000;
>> }
>>+#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
>>-#define SVR_VAR_PER_SHIFT 8
>> #define SVR_LS102XA_MASK 0x700
>>-#define SVR_LS2088A 0x870900
>>-#define SVR_LS2084A 0x870910
>>-#define SVR_LS2048A 0x870920
>>-#define SVR_LS2044A 0x870930
>>-#define SVR_LS2081A 0x870918
>>-#define SVR_LS2041A 0x870914
>>
>> /* LS1021a PCIE space */
>> #define LS1021_PCIE_SPACE_OFFSET 0x4000000000ULL
>>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
>>+ * Copyright 2017, 2019 NXP
>Same as above
>> * Copyright 2014-2015 Freescale Semiconductor, Inc.
>> * Layerscape PCIe driver
>> */
>>@@ -16,6 +16,7 @@
>> #ifdef CONFIG_ARM
>> #include <asm/arch/clock.h>
>> #endif
>>+#include <asm/arch/soc.h>
>> #include "pcie_layerscape.h"
>>
>> #if defined(CONFIG_FSL_LSCH3) || defined(CONFIG_FSL_LSCH2) @@ -83,7
>>+84,7 @@ static void fdt_pcie_set_msi_map_entry(void *blob, struct
>>+ls_pcie
>>*pcie,
>> pcie->dbi_res.start);
>> if (nodeoffset < 0) {
>> #ifdef CONFIG_FSL_PCIE_COMPAT /* Compatible with older version of dts
>>node */
>>- svr = (get_svr() >> SVR_VAR_PER_SHIFT) & 0xFFFFFE;
>>+ svr = SVR_SOC_VER(get_svr());
>> if (svr == SVR_LS2088A || svr == SVR_LS2084A ||
>> svr == SVR_LS2048A || svr == SVR_LS2044A ||
>> svr == SVR_LS2081A || svr == SVR_LS2041A) @@ -137,7
>>+138,7 @@ static void fdt_pcie_set_iommu_map_entry(void *blob, struct
>>ls_pcie *pcie,
>> pcie->dbi_res.start);
>> if (nodeoffset < 0) {
>> #ifdef CONFIG_FSL_PCIE_COMPAT /* Compatible with older version of dts
>>node */
>>- svr = (get_svr() >> SVR_VAR_PER_SHIFT) & 0xFFFFFE;
>>+ svr = SVR_SOC_VER(get_svr());
>> if (svr == SVR_LS2088A || svr == SVR_LS2084A ||
>> svr == SVR_LS2048A || svr == SVR_LS2044A ||
>> svr == SVR_LS2081A || svr == SVR_LS2041A) @@ -221,14
>>+222,16 @@ static void fdt_fixup_pcie(void *blob) static void
>>ft_pcie_rc_fix(void *blob, struct ls_pcie *pcie) {
>> int off;
>>- uint svr;
>>- char *compat = NULL;
>>
>> off = fdt_node_offset_by_compat_reg(blob, "fsl,ls-pcie",
>> pcie->dbi_res.start);
>> if (off < 0) {
>>+#if defined(CONFIG_FSL_LAYERSCAPE)
>> #ifdef CONFIG_FSL_PCIE_COMPAT /* Compatible with older version of dts
>>node */
>>- svr = (get_svr() >> SVR_VAR_PER_SHIFT) & 0xFFFFFE;
>>+ uint svr;
>>+ char *compat = NULL;
>Same as above
>>+
>>+ svr = SVR_SOC_VER(get_svr());
>> if (svr == SVR_LS2088A || svr == SVR_LS2084A ||
>> svr == SVR_LS2048A || svr == SVR_LS2044A ||
>> svr == SVR_LS2081A || svr == SVR_LS2041A) @@ -239,6
>>+242,7 @@ static void ft_pcie_rc_fix(void *blob, struct ls_pcie *pcie)
>> off = fdt_node_offset_by_compat_reg(blob,
>> compat, pcie->dbi_res.start);
>> #endif
>>+#endif /* CONFIG_FSL_LAYERSCAPE */
>> if (off < 0)
>> return;
>> }
>>--
>>2.17.1
>
>--priyankajain
>_______________________________________________
>U-Boot mailing list
>U-Boot at lists.denx.de
>https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.de
>nx.de%2Flistinfo%2Fu-
>boot&data=02%7C01%7Cpriyanka.jain%40nxp.com%7C086fd8c5509d455
>a0eb708d7538a5cb3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6
>37069726040396790&sdata=dkvAocUjVFqBMcnOnOZ1nqiQsrBrcqPkKXhk9
>oloei0%3D&reserved=0
More information about the U-Boot
mailing list