On Fri, Feb 05, 2016 at 09:12:50AM -0800, David Daney wrote:

On 02/04/2016 07:10 PM, Bjorn Helgaas wrote:

quoted

On Thu, Feb 04, 2016 at 04:28:29PM -0800, David Daney wrote:

quoted

On 02/04/2016 04:04 PM, Bjorn Helgaas wrote:

quoted

On Tue, Jan 26, 2016 at 01:46:21PM -0800, David Daney wrote:

quoted

From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>

Some Cavium ThunderX processors require quirky access methods for the
config space of the PCIe bridge.  Add a driver to provide these config
space accessor functions.  The pci-host-common code is used to
configure the PCI machinery.

Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
Acked-by: Rob Herring <redacted>
Acked-by: Arnd Bergmann <redacted>
---
 .../devicetree/bindings/pci/pci-thunder-pem.txt    |  43 ++++
 MAINTAINERS                                        |   8 +
 drivers/pci/host/Kconfig                           |   7 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pci-thunder-pem.c                 | 283 +++++++++++++++++++++

What's the significance of the "pem" part of the name?  I'm wondering
if we can shorten the filenames and function names by dropping it and
referring to this simply as "thunder" or "thunderx".

...

quoted

quoted

Since PEM and ECAM are terminology used in the hardware manuals that
have specific meanings relative to the Thunder SoC family, I think
it is not too inconvenient to carry them over into the file names.

As long as PEM and ECAM are really two distinct root complexes that
are unrelated, I guess this is OK.

They are, see above.

OK, I'm convinced.

quoted

quoted

quoted

quoted

+		/*
+		 * 32-bit accesses only.  If the write is for a size
+		 * smaller than 32-bits, we must first read the 32-bit
+		 * value and merge in the desired bits and then write
+		 * the whole 32-bits back out.
+		 */

Ugh.  Another device that only supports 32-bit writes.  I guess this
only affects this single device, and maybe you "know" that it has no
registers where RW1C bits may be corrupted.  Although I suppose this
device has the standard status registers (Status at 0x06, Secondary
Status at 0x1e, Device Status in PCIe capability, etc.), which do
contain RW1C bits.

...

I will add a WARN_ONCE or similar. and send a new patch set.

FWIW, I think I have been able to get the message through to the
hardware architects that building root complexes that are not
exposed as PCI standard ECAMs makes things very difficult.  This was
the original intention, but turned out not to be possible when we
looked more closely at the hardware implementation.  I am optimistic
that subsequent generations of Thunder will be much improved in this
area.

Great!  Since there's apparently only one ThunderX device (devfn == 0
on the root bus) that has this problem, and you know exactly what that
device is and where all its RW1C bits are, you *could* fix this in the
*_config_write() functions by clearing any RW1C bits before the
"modify/write" part of any read/modify/write cycle.

BTW, minor nit, when you repost these, can you reorder the
map_bus/read/write functions in that order so they match the order
they're declared in struct pci_ops?  I think both pem and ecam
versions could benefit from this.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html