Re: [PATCH v8 12/14] PCI: dwc: Apply ECRC workaround to DesignWare 5.00a as well

[Bjorn Helgaas]

On Thu, Apr 09, 2026 at 02:21:57PM +0530, Manikanta Maddireddy wrote:
> On 09/04/26 3:54 am, Bjorn Helgaas wrote:
> > On Wed, Mar 25, 2026 at 12:37:53AM +0530, Manikanta Maddireddy wrote:
> > > The ECRC (TLP digest) workaround was originally added for DesignWare
> > > version 4.90a. Tegra234 SoC has 5.00a DWC HW version, which has
> > > the same ATU TD override behaviour, so apply the workaround for 5.00a
> > > too.
> > > 
> > > Fixes: a54e19073718 ("PCI: tegra194: Add Tegra234 PCIe support")
> > > Reviewed-by: Jon Hunter <jonathanh@xxxxxxxxxx>
> > > Tested-by: Jon Hunter <jonathanh@xxxxxxxxxx>
> > > Reviewed-by: Vidya Sagar <vidyas@xxxxxxxxxx>
> > > Signed-off-by: Manikanta Maddireddy <mmaddireddy@xxxxxxxxxx>
> > > ---
> > > Changes V8: Split into two patches
> > > Changes V1 -> V7: None
> > > 
> > >   drivers/pci/controller/dwc/pcie-designware.c | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > > index 345365ea97c7..c4dc2d88649e 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > @@ -486,7 +486,7 @@ static inline void dw_pcie_writel_atu_ob(struct dw_pcie *pci, u32 index, u32 reg
> > >   static inline u32 dw_pcie_enable_ecrc(u32 val)
> > >   {
> > >   	/*
> > > -	 * DesignWare core version 4.90A has a design issue where the 'TD'
> > > +	 * DWC versions 0x3530302a and 0x3536322a has a design issue where the 'TD'
> > 
> > 0x3536322a looks like DW_PCIE_VER_562A, not DW_PCIE_VER_500A, so this
> > comment doesn't seem to match the commit log or the code.
> > 
> > "0x3530302a and 0x3536322a" is not nearly as readable as 4.90A and
> > 5.00A.
> > 
> > >   	 * bit in the Control register-1 of the ATU outbound region acts
> > >   	 * like an override for the ECRC setting, i.e., the presence of TLP
> > >   	 * Digest (ECRC) in the outgoing TLPs is solely determined by this
> > > @@ -559,7 +559,7 @@ int dw_pcie_prog_outbound_atu(struct dw_pcie *pci,
> > >   	if (upper_32_bits(limit_addr) > upper_32_bits(parent_bus_addr) &&
> > >   	    dw_pcie_ver_is_ge(pci, 460A))
> > >   		val |= PCIE_ATU_INCREASE_REGION_SIZE;
> > > -	if (dw_pcie_ver_is(pci, 490A))
> > > +	if (dw_pcie_ver_is(pci, 490A) || dw_pcie_ver_is(pci, 500A))
> > >   		val = dw_pcie_enable_ecrc(val);
> > 
> > This is in shared DWC code, which raises the question of whether this
> > issue applies *only* to 490A and 500A?  What about other versions,
> > e.g., 520A (unused AFAICS), 540A, 562A?
> > 
> 
> Hi Bjorn,
> 
> I reviewed our internal bug database, I found that this dependency of iATU
> TD bit on ECRC is removed from version 5.10A. A comment from Synopsys case
> is quoted in our internal bug. Shall I prepare patch to address this for all
> versions < 5.10A? Or do we need inputs from Synopsys?

The patch below looks good to me, assuming the commit log is updated
to match this comment and the code.

I don't have any visibility into the Synopsys IP versions.

> Proposed patch
> 
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -486,7 +486,7 @@ static inline void dw_pcie_writel_atu_ob(struct dw_pcie
> *pci, u32 index, u32 reg
>  static inline u32 dw_pcie_enable_ecrc(u32 val)
>  {
>         /*
> -        * DWC versions 0x3530302a and 0x3536322a has a design issue where
> the 'TD'
> +        * DWC versions less than 5.10A has a design issue where the 'TD'
>          * bit in the Control register-1 of the ATU outbound region acts
>          * like an override for the ECRC setting, i.e., the presence of TLP
>          * Digest (ECRC) in the outgoing TLPs is solely determined by this
> @@ -559,7 +559,7 @@ int dw_pcie_prog_outbound_atu(struct dw_pcie *pci,
>         if (upper_32_bits(limit_addr) > upper_32_bits(parent_bus_addr) &&
>             dw_pcie_ver_is_ge(pci, 460A))
>                 val |= PCIE_ATU_INCREASE_REGION_SIZE;
> -       if (dw_pcie_ver_is(pci, 490A) || dw_pcie_ver_is(pci, 500A))
> +       if (!dw_pcie_ver_is_ge(pci, 510A))
>                 val = dw_pcie_enable_ecrc(val);
>         dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL1, val);
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h
> b/drivers/pci/controller/dwc/pcie-designware.h
> index 5bceadbd2c9f..00891adfd07d 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -35,6 +35,7 @@
>  #define DW_PCIE_VER_480A               0x3438302a
>  #define DW_PCIE_VER_490A               0x3439302a
>  #define DW_PCIE_VER_500A               0x3530302a
> +#define DW_PCIE_VER_510A               0x3531302a
>  #define DW_PCIE_VER_520A               0x3532302a
>  #define DW_PCIE_VER_540A               0x3534302a
>  #define DW_PCIE_VER_562A               0x3536322a
> 
> Thanks,
> Manikanta
> 
> > >   	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL1, val);
> > > -- 
> > > 2.34.1
> > > 
> 
> -- 
> nvpublic
>