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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Lorenzo,
Apologies to bug you, but wondering if you have any further comments on this patch that I need to take care of?

Thanks,
Vidya Sagar

On 12/3/2020 5:40 PM, Vidya Sagar wrote:


On 11/25/2020 2:32 AM, Bjorn Helgaas wrote:
External email: Use caution opening links or attachments


On Tue, Nov 24, 2020 at 03:50:01PM +0530, Vidya Sagar wrote:
Hi Bjorn,
Please let me know if this patch needs any further modifications

I'm fine with it, but of course Lorenzo will take care of it.
Thanks Bjorn.

Hi Lorenzo,
Please let me know if you have any comments for this patch.

Thanks,
Vidya Sagar


On 11/12/2020 10:32 PM, Vidya Sagar wrote:
External email: Use caution opening links or attachments


On 11/12/2020 3:59 AM, Bjorn Helgaas wrote:
External email: Use caution opening links or attachments


On Wed, Nov 11, 2020 at 10:21:46PM +0530, Vidya Sagar wrote:


On 11/11/2020 9:57 PM, Jingoo Han wrote:
External email: Use caution opening links or attachments


On 11/11/20, 7:12 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.

Signed-off-by: Vidya Sagar <vidyas@xxxxxxxxxx>
Acked-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
---
V2:
* Addressed Bjorn's comments

    drivers/pci/controller/dwc/pcie-designware.c | 52
++++++++++++++++++--
    drivers/pci/controller/dwc/pcie-designware.h |  1 +
    2 files changed, 49 insertions(+), 4 deletions(-)

diff --git
a/drivers/pci/controller/dwc/pcie-designware.c
b/drivers/pci/controller/dwc/pcie-designware.c
index c2dea8fc97c8..ec0d13ab6bad 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -225,6 +225,46 @@ static void
dw_pcie_writel_ob_unroll(struct dw_pcie *pci, u32 index,
u32 reg,
         dw_pcie_writel_atu(pci, offset + reg, val);
    }

+static inline u32 dw_pcie_enable_ecrc(u32 val)

What is the reason to use inline here?

Actually, I wanted to move the programming part inside the
respective APIs
but then I wanted to give some details as well in comments so to avoid duplication, I came up with this function. But, I'm making it inline for
better code optimization by compiler.

I don't really care either way, but I'd be surprised if the compiler
didn't inline this all by itself even without the explicit "inline".
I just checked it and you are right that compiler is indeed inlining it
without explicitly mentioning 'inline'.
I hope it is ok to leave it that way.


+{
+     /*
+      * DesignWare core version 4.90A has this strange 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 bit. This is contrary to the PCIe spec
which says that the
+      * enablement of the ECRC is solely determined by
the AER registers.
+      *
+      * Because of this, even when the ECRC is enabled through AER
+      * registers, the transactions going through ATU
won't have TLP Digest
+      * as there is no way the AER sub-system could
program the TD bit which
+      * is specific to DesignWare core.
+      *
+      * The best way to handle this scenario is to program the TD bit
+      * always. It affects only the traffic from root
port to downstream
+      * devices.
+      *
+      * At this point,
+      * When ECRC is enabled in AER registers,
everything works normally
+      * When ECRC is NOT enabled in AER registers, then,
+      * on Root Port:- TLP Digest (DWord size) gets
appended to each packet
+      *                even through it is not required.
Since downstream
+      *                TLPs are mostly for
configuration accesses and BAR
+      *                accesses, they are not in
critical path and won't
+      *                have much negative effect on the performance.
+      * on End Point:- TLP Digest is received for
some/all the packets coming
+      *                from the root port. TLP Digest
is ignored because,
+      *                as per the PCIe Spec r5.0 v1.0 section 2.2.3
+      *                "TLP Digest Rules", when an
endpoint receives TLP
+      *                Digest when its ECRC check
functionality is disabled
+      *                in AER registers, received TLP
Digest is just ignored.
+      * Since there is no issue or error reported
either side, best way to
+      * handle the scenario is to program TD bit by default.
+      */
+
+     return val | PCIE_ATU_TD;
+}




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux