On 15/11/2018 18:24, 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! > Please change "designware" by "dwc" on the title (which is the normal tag) as I suggested and put the first letter of "don't" in capital letter. With that fix, please re-submit patch version. Acked-by: Gustavo Pimentel <gustavo.pimentel@xxxxxxxxxxxx> Thanks. Regards, Gustavo
