On Tue, 4 Jul 2017 14:58:40 +0800 Jisheng Zhang wrote:

On Mon, 3 Jul 2017 08:27:04 -0500 wrote:

quoted

[+cc Jingoo, Joao]

On Mon, Jul 03, 2017 at 10:35:50AM +0100, Ard Biesheuvel wrote:  

quoted

On 3 July 2017 at 00:18, Bjorn Helgaas [off-list ref] wrote:    

quoted

On Tue, Jun 20, 2017 at 10:17:40AM +0200, Marc Gonzalez wrote:    

quoted

This driver is required to work around several hardware bugs
in the PCIe controller.

NB: Revision 1 does not support legacy interrupts, or IO space.    

I had to apply these manually because of conflicts in Kconfig and
Makefile.  What are these based on?  Easiest for me is if you base
them on the current -rc1 tag.
   

quoted

Signed-off-by: Marc Gonzalez <redacted>
---
 drivers/pci/host/Kconfig      |   8 +++
 drivers/pci/host/Makefile     |   1 +
 drivers/pci/host/pcie-tango.c | 164 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci_ids.h       |   2 +
 4 files changed, 175 insertions(+)
 create mode 100644 drivers/pci/host/pcie-tango.c
   

[..]    

quoted

quoted

+     /*
+      * QUIRK #2
+      * Unfortunately, config and mem spaces are muxed.
+      * Linux does not support such a setting, since drivers are free
+      * to access mem space directly, at any time.
+      * Therefore, we can only PRAY that config and mem space accesses
+      * NEVER occur concurrently.
+      */
+     writel_relaxed(1, pcie->mux);
+     ret = pci_generic_config_read(bus, devfn, where, size, val);
+     writel_relaxed(0, pcie->mux);    

I'm very hesitant about this.  When people stress this, we're going to
get reports of data corruption.  Even with the disclaimer below, I
don't feel good about this.  Adding the driver is an implicit claim
that we support the device, but we know it can't be made reliable.    

I noticed that the Synopsys driver suffers from a similar issue: in
dw_pcie_rd_other_conf(), it happily reprograms the outbound I/O window
to perform a config space access, and switches it back to I/O space
afterwards (unless it has more than 2 viewports, in which case it uses
dedicated windows for I/O space and config space)    

That doesn't sound good.  Jingoo, Joao?  I remember some discussion
about this, but not the details.

I/O accesses use wrappers (inb(), etc), so there's at least the
possibility of a mutex to serialize them with respect to config
accesses.
  

IIRC, for 2 viewports, we don't need to worry about the config space
access, because config space access is serialized by pci_lock; We
do have race between config space and io space. But the accessing config
space and io space at the same time is rare. And the PCIe EPs which
has io space are rare too, supporting these EPs are not the potential
target of those platforms with 2 viewports.

PS: I think most platforms choose 2 pcie designware viewports just because
it's the default setting. And I have send a feature request to ASIC people
to increase the viewports to 3 for future marvell berlin SoCs.