Re: [PATCH] PCI: designware: don't hard-code DBI/ATU offset

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

 



Hi,

On 15/11/18 11:54 PM, Stephen Warren wrote:
On 11/14/18 9:33 PM, Gustavo Pimentel wrote:
Hi Stephen,

On 14/11/2018 04:31, Stephen Warren wrote:
On 11/12/18 9:19 PM, Gustavo Pimentel wrote:
Hi Stephen,

I'd suggest to change the designware on the patch name by dwc and use another
description for the patch instead of don't hard-code DBI/ATU offset.

What issue do you see with the patch description? I believe it's
correct/accurate. Perhaps the confusion is due to the question you
raised below; see my answer there.

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




[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