Re: [PATCH 2/2] PCI: dwc: Add support to configure for ECRC

2020-10-26 Thread Jingoo Han


On 10/26/20, 2:59 AM, Vidya Sagar wrote:
> 
> On 10/26/2020 2:19 AM, Jingoo Han wrote:
> > External email: Use caution opening links or attachments
> > 
> > 
> > On 10/25/20, 3:31 AM, Vidya Sagar wrote:
> >>
> >> DesignWare core has a TLP digest (TD) override bit in one of the control
> >> registers of ATU. This bit also needs to be programmed for proper ECRC
> >> functionality. This is currently identified as an issue with DesignWare
> >> IP version 4.90a. This patch does the required programming in ATU upon
> >> querying the system policy for ECRC.
> >>
> >> Signed-off-by: Vidya Sagar 
> >> ---
> >>   drivers/pci/controller/dwc/pcie-designware.c | 8 ++--
> >>   drivers/pci/controller/dwc/pcie-designware.h | 2 ++
> >>   2 files changed, 8 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/pci/controller/dwc/pcie-designware.c 
> >> b/drivers/pci/controller/dwc/pcie-designware.c
> >> index b5e438b70cd5..810dcbdbe869 100644
> >> --- a/drivers/pci/controller/dwc/pcie-designware.c
> >> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> >> @@ -245,7 +245,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct 
> >> dw_pcie *pci, u8 func_no,
> >> lower_32_bits(pci_addr));
> >>dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
> >> upper_32_bits(pci_addr));
> >> - val = type | PCIE_ATU_FUNC_NUM(func_no);
> >> + val = type | PCIE_ATU_FUNC_NUM(func_no) | pci->td << 
> >> PCIE_ATU_TD_SHIFT;
> >>val = upper_32_bits(size - 1) ?
> >>val | PCIE_ATU_INCREASE_REGION_SIZE : val;
> >>dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1, 
> >> val);
> >> @@ -295,7 +295,8 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie 
> >> *pci, u8 func_no,
> >>dw_pcie_writel_dbi(pci, PCIE_ATU_UPPER_TARGET,
> >>   upper_32_bits(pci_addr));
> >>dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, type |
> >> -PCIE_ATU_FUNC_NUM(func_no));
> >> +PCIE_ATU_FUNC_NUM(func_no) |
> >> +pci->td << PCIE_ATU_TD_SHIFT);
> >>dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE);
> >>
> >>/*
> >> @@ -565,6 +566,9 @@ void dw_pcie_setup(struct dw_pcie *pci)
> >>dev_dbg(pci->dev, "iATU unroll: %s\n", pci->iatu_unroll_enabled ?
> >>"enabled" : "disabled");
> >>
> >> + if (pci->version == 0x490A)
> >> + pci->td = pcie_is_ecrc_enabled();
> >> +
> >>if (pci->link_gen > 0)
> >>dw_pcie_link_set_max_speed(pci, pci->link_gen);
> >>
> >> diff --git a/drivers/pci/controller/dwc/pcie-designware.h 
> >> b/drivers/pci/controller/dwc/pcie-designware.h
> >> index 21dd06831b50..d34723e42e79 100644
> >> --- a/drivers/pci/controller/dwc/pcie-designware.h
> >> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> >> @@ -90,6 +90,7 @@
> >>   #define PCIE_ATU_TYPE_IO 0x2
> >>   #define PCIE_ATU_TYPE_CFG0   0x4
> >>   #define PCIE_ATU_TYPE_CFG1   0x5
> >> +#define PCIE_ATU_TD_SHIFT8
> >>   #define PCIE_ATU_FUNC_NUM(pf)   ((pf) << 20)
> >>   #define PCIE_ATU_CR2 0x908
> >>   #define PCIE_ATU_ENABLE  BIT(31)
> >> @@ -276,6 +277,7 @@ struct dw_pcie {
> >>int num_lanes;
> >>int link_gen;
> >>u8  n_fts[2];
> >> + booltd; /* TLP Digest (for ECRC purpose) */
> > 
> > If possible, don't add a new variable to 'dw_pcie' structure.
> > Please find a way to set TD bit without adding a new variable to 'dw_pcie' 
> > structure'.
>
> I can use pcie_is_ecrc_enabled() directly in place of pci->td. That 
> should be fine right? BTW, curious to know if there is any specific 
> reason behind asking not to add any new variables to 'dw_pcie' structure?
>

Vice versa, what is the reason to add a new variable 'td' to 'dw_pcie' 
structure?
It is not good to add a new variable to the global structure without any 
specific reason.


> > 
> Best regards,
> Jingoo Han
> 
>>   };
>>
>>   #define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp)
>> --
>> 2.17.1
> 


Re: [PATCH 2/2] PCI: dwc: Add support to configure for ECRC

2020-10-26 Thread Vidya Sagar




On 10/26/2020 2:19 AM, Jingoo Han wrote:

External email: Use caution opening links or attachments


On 10/25/20, 3:31 AM, Vidya Sagar wrote:


DesignWare core has a TLP digest (TD) override bit in one of the control
registers of ATU. This bit also needs to be programmed for proper ECRC
functionality. This is currently identified as an issue with DesignWare
IP version 4.90a. This patch does the required programming in ATU upon
querying the system policy for ECRC.

Signed-off-by: Vidya Sagar 
---
  drivers/pci/controller/dwc/pcie-designware.c | 8 ++--
  drivers/pci/controller/dwc/pcie-designware.h | 2 ++
  2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c 
b/drivers/pci/controller/dwc/pcie-designware.c
index b5e438b70cd5..810dcbdbe869 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -245,7 +245,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie 
*pci, u8 func_no,
lower_32_bits(pci_addr));
   dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
upper_32_bits(pci_addr));
- val = type | PCIE_ATU_FUNC_NUM(func_no);
+ val = type | PCIE_ATU_FUNC_NUM(func_no) | pci->td << PCIE_ATU_TD_SHIFT;
   val = upper_32_bits(size - 1) ?
   val | PCIE_ATU_INCREASE_REGION_SIZE : val;
   dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1, val);
@@ -295,7 +295,8 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie 
*pci, u8 func_no,
   dw_pcie_writel_dbi(pci, PCIE_ATU_UPPER_TARGET,
  upper_32_bits(pci_addr));
   dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, type |
-PCIE_ATU_FUNC_NUM(func_no));
+PCIE_ATU_FUNC_NUM(func_no) |
+pci->td << PCIE_ATU_TD_SHIFT);
   dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE);

   /*
@@ -565,6 +566,9 @@ void dw_pcie_setup(struct dw_pcie *pci)
   dev_dbg(pci->dev, "iATU unroll: %s\n", pci->iatu_unroll_enabled ?
   "enabled" : "disabled");

+ if (pci->version == 0x490A)
+ pci->td = pcie_is_ecrc_enabled();
+
   if (pci->link_gen > 0)
   dw_pcie_link_set_max_speed(pci, pci->link_gen);

diff --git a/drivers/pci/controller/dwc/pcie-designware.h 
b/drivers/pci/controller/dwc/pcie-designware.h
index 21dd06831b50..d34723e42e79 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -90,6 +90,7 @@
  #define PCIE_ATU_TYPE_IO 0x2
  #define PCIE_ATU_TYPE_CFG0   0x4
  #define PCIE_ATU_TYPE_CFG1   0x5
+#define PCIE_ATU_TD_SHIFT8
  #define PCIE_ATU_FUNC_NUM(pf)   ((pf) << 20)
  #define PCIE_ATU_CR2 0x908
  #define PCIE_ATU_ENABLE  BIT(31)
@@ -276,6 +277,7 @@ struct dw_pcie {
   int num_lanes;
   int link_gen;
   u8  n_fts[2];
+ booltd; /* TLP Digest (for ECRC purpose) */


If possible, don't add a new variable to 'dw_pcie' structure.
Please find a way to set TD bit without adding a new variable to 'dw_pcie' 
structure'.


I can use pcie_is_ecrc_enabled() directly in place of pci->td. That 
should be fine right? BTW, curious to know if there is any specific 
reason behind asking not to add any new variables to 'dw_pcie' structure?




Best regards,
Jingoo Han


  };

  #define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp)
--
2.17.1




Re: [PATCH 2/2] PCI: dwc: Add support to configure for ECRC

2020-10-25 Thread Jingoo Han
On 10/25/20, 3:31 AM, Vidya Sagar wrote:
> 
> DesignWare core has a TLP digest (TD) override bit in one of the control
> registers of ATU. This bit also needs to be programmed for proper ECRC
> functionality. This is currently identified as an issue with DesignWare
> IP version 4.90a. This patch does the required programming in ATU upon
> querying the system policy for ECRC.
>
> Signed-off-by: Vidya Sagar 
> ---
>  drivers/pci/controller/dwc/pcie-designware.c | 8 ++--
>  drivers/pci/controller/dwc/pcie-designware.h | 2 ++
>  2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c 
> b/drivers/pci/controller/dwc/pcie-designware.c
> index b5e438b70cd5..810dcbdbe869 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -245,7 +245,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct 
> dw_pcie *pci, u8 func_no,
>lower_32_bits(pci_addr));
>   dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
>upper_32_bits(pci_addr));
> - val = type | PCIE_ATU_FUNC_NUM(func_no);
> + val = type | PCIE_ATU_FUNC_NUM(func_no) | pci->td << PCIE_ATU_TD_SHIFT;
>   val = upper_32_bits(size - 1) ?
>   val | PCIE_ATU_INCREASE_REGION_SIZE : val;
>   dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1, val);
> @@ -295,7 +295,8 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie 
> *pci, u8 func_no,
>   dw_pcie_writel_dbi(pci, PCIE_ATU_UPPER_TARGET,
>  upper_32_bits(pci_addr));
>   dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, type |
> -PCIE_ATU_FUNC_NUM(func_no));
> +PCIE_ATU_FUNC_NUM(func_no) |
> +pci->td << PCIE_ATU_TD_SHIFT);
>   dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE);
>
>   /*
> @@ -565,6 +566,9 @@ void dw_pcie_setup(struct dw_pcie *pci)
>   dev_dbg(pci->dev, "iATU unroll: %s\n", pci->iatu_unroll_enabled ?
>   "enabled" : "disabled");
>
> + if (pci->version == 0x490A)
> + pci->td = pcie_is_ecrc_enabled();
> +
>   if (pci->link_gen > 0)
>   dw_pcie_link_set_max_speed(pci, pci->link_gen);
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h 
> b/drivers/pci/controller/dwc/pcie-designware.h
> index 21dd06831b50..d34723e42e79 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -90,6 +90,7 @@
>  #define PCIE_ATU_TYPE_IO 0x2
>  #define PCIE_ATU_TYPE_CFG0   0x4
>  #define PCIE_ATU_TYPE_CFG1   0x5
> +#define PCIE_ATU_TD_SHIFT8
>  #define PCIE_ATU_FUNC_NUM(pf)   ((pf) << 20)
>  #define PCIE_ATU_CR2 0x908
>  #define PCIE_ATU_ENABLE  BIT(31)
> @@ -276,6 +277,7 @@ struct dw_pcie {
>   int num_lanes;
>   int link_gen;
>   u8  n_fts[2];
> + booltd; /* TLP Digest (for ECRC purpose) */

If possible, don't add a new variable to 'dw_pcie' structure.
Please find a way to set TD bit without adding a new variable to 'dw_pcie' 
structure'.

Best regards,
Jingoo Han

>  };
>
>  #define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp)
> -- 
> 2.17.1



[PATCH 2/2] PCI: dwc: Add support to configure for ECRC

2020-10-25 Thread Vidya Sagar
DesignWare core has a TLP digest (TD) override bit in one of the control
registers of ATU. This bit also needs to be programmed for proper ECRC
functionality. This is currently identified as an issue with DesignWare
IP version 4.90a. This patch does the required programming in ATU upon
querying the system policy for ECRC.

Signed-off-by: Vidya Sagar 
---
 drivers/pci/controller/dwc/pcie-designware.c | 8 ++--
 drivers/pci/controller/dwc/pcie-designware.h | 2 ++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c 
b/drivers/pci/controller/dwc/pcie-designware.c
index b5e438b70cd5..810dcbdbe869 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -245,7 +245,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie 
*pci, u8 func_no,
 lower_32_bits(pci_addr));
dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
 upper_32_bits(pci_addr));
-   val = type | PCIE_ATU_FUNC_NUM(func_no);
+   val = type | PCIE_ATU_FUNC_NUM(func_no) | pci->td << PCIE_ATU_TD_SHIFT;
val = upper_32_bits(size - 1) ?
val | PCIE_ATU_INCREASE_REGION_SIZE : val;
dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1, val);
@@ -295,7 +295,8 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie 
*pci, u8 func_no,
dw_pcie_writel_dbi(pci, PCIE_ATU_UPPER_TARGET,
   upper_32_bits(pci_addr));
dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, type |
-  PCIE_ATU_FUNC_NUM(func_no));
+  PCIE_ATU_FUNC_NUM(func_no) |
+  pci->td << PCIE_ATU_TD_SHIFT);
dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE);
 
/*
@@ -565,6 +566,9 @@ void dw_pcie_setup(struct dw_pcie *pci)
dev_dbg(pci->dev, "iATU unroll: %s\n", pci->iatu_unroll_enabled ?
"enabled" : "disabled");
 
+   if (pci->version == 0x490A)
+   pci->td = pcie_is_ecrc_enabled();
+
if (pci->link_gen > 0)
dw_pcie_link_set_max_speed(pci, pci->link_gen);
 
diff --git a/drivers/pci/controller/dwc/pcie-designware.h 
b/drivers/pci/controller/dwc/pcie-designware.h
index 21dd06831b50..d34723e42e79 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -90,6 +90,7 @@
 #define PCIE_ATU_TYPE_IO   0x2
 #define PCIE_ATU_TYPE_CFG0 0x4
 #define PCIE_ATU_TYPE_CFG1 0x5
+#define PCIE_ATU_TD_SHIFT  8
 #define PCIE_ATU_FUNC_NUM(pf)   ((pf) << 20)
 #define PCIE_ATU_CR2   0x908
 #define PCIE_ATU_ENABLEBIT(31)
@@ -276,6 +277,7 @@ struct dw_pcie {
int num_lanes;
int link_gen;
u8  n_fts[2];
+   booltd; /* TLP Digest (for ECRC purpose) */
 };
 
 #define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp)
-- 
2.17.1