>-----Original Message-----
>From: U-Boot <u-boot-boun...@lists.denx.de> On Behalf Of Priyanka Jain
>Sent: Friday, October 18, 2019 10:46 AM
>To: Pankaj Bansal <pankaj.ban...@nxp.com>; Xiaowei Bao
><xiaowei....@nxp.com>; Tom Rini <tr...@konsulko.com>; Z.q. Hou
><zhiqiang....@nxp.com>
>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 <priyanka.j...@nxp.com>; Xiaowei Bao
>><xiaowei....@nxp.com>; Tom Rini <tr...@konsulko.com>; Z.q. Hou
>><zhiqiang....@nxp.com>
>>Cc: u-boot@lists.denx.de; Pankaj Bansal <pankaj.ban...@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.ban...@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&amp;data=02%7C01%7Cpriyanka.j
>ain%4
>>0nxp.com%7C086fd8c5509d455a0eb708d7538a5cb3%7C686ea1d3bc2b4c6fa9
>2cd99c5
>>c301635%7C0%7C0%7C637069726040396790&amp;sdata=21jhOzjm27k4aQW
>AK9BYhcoI
>>l6M78KAV3t9ImmlgSdw%3D&amp;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@lists.denx.de
>https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.de
>nx.de%2Flistinfo%2Fu-
>boot&amp;data=02%7C01%7Cpriyanka.jain%40nxp.com%7C086fd8c5509d455
>a0eb708d7538a5cb3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6
>37069726040396790&amp;sdata=dkvAocUjVFqBMcnOnOZ1nqiQsrBrcqPkKXhk9
>oloei0%3D&amp;reserved=0
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to