On 11/20/18 2:41 AM, Kishon Vijay Abraham I wrote:
On 15/11/18 11:54 PM, Stephen Warren wrote:
On 11/14/18 9:33 PM, Gustavo Pimentel wrote:
On 14/11/2018 04:31, Stephen Warren wrote:
On 11/12/18 9:19 PM, Gustavo Pimentel wrote:
On 12/11/2018 22:57, Stephen Warren wrote:
From: Stephen Warren <swarren@xxxxxxxxxx>
The DWC PCIe core contains various separate register spaces: DBI, DBI2,
ATU, DMA, etc. The relationship between the addresses of these register
spaces is entirely determined by the implementation of the IP block, not
by the IP block design itself. Hence, the DWC driver must not make
assumptions that one register space can be accessed at a fixed offset from
any other register space. To avoid such assumptions, introduce an
explicit/separate register pointer for the ATU register space. In
particular, the current assumption is not valid for NVIDIA's T194
SoC.
If I understood this patch correctly, you basically replace the dbi_base offset
by atu_base offset that still depends of dbi_base offset.
That's not what the patch does.
The patch leaves most DBI accesses still using dbi_base, but updates
just a few accesses to use atu_base. Thus, after the patch, all accesses
use the correct base address for the register being accessed.
There is a default value supplied for atu_base, which does indeed depend
on dbi_base. This maintains backwards compatibility, so that all the
existing drivers don't need to be updated to explicitly set atu_base,
and will continue to use the existing offset between DBI and ATU base.
I think we're talking about the same thing, but with different terms.
In the future, we'll send a driver for the NVIDIA Tegra SoC which does
explicitly set atu_base to a non-default value.
That's what I wanted to know. Because otherwise this patch was just
to turn the code more readable.
So it sounds like I've addressed your questions? If so, is the next step
for you to ack the patch and Bjorn to apply it? Thanks!
I had posted a patch sometime back to fix the same issue of hardcoding
ATU offset [1]. However that patch also fixed ATU identification logic.
I can post that as a separate patch if Lorenzo wants to merge this one.
FWIW:
Acked-by: Kishon Vijay Abraham I <kishon@xxxxxx>
Thanks
Kishon
[1] -> https://lkml.org/lkml/2018/9/21/484
Ah, interesting. I see one problem with that patch on NVIDIA hardware.
Specifically, it looks like your patch enhances the driver to always set
iatu_unroll_enabled in dw_pcie_setup() for either RP or EP mode? If so,
that won't work well for us since the DBI registers can't be accessed at
that time on our HW. Still, in our particular case, the driver would set
the version number field and so that register access would actually be
avoided, so the issue won't actually happen, so maybe the patch is OK...
I'd love to see at least one of the patches applied.
