On 2014年11月13日 00:23, Srikanth Thokala wrote:
Hi Minghuan,
On Wed, Nov 12, 2014 at 3:39 PM, Lian Minghuan-B31939
<B31939@xxxxxxxxxxxxx> wrote:
[...]
return ret;
@@ -770,6 +793,10 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
u32 membase;
u32 memlimit;
+ /* set ATUs setting for MEM and IO */
+ dw_pcie_prog_viewport_mem_outbound(pp);
+ dw_pcie_prog_viewport_io_outbound(pp);
+
I see from the code, these settings are required for the calls other
than
dw_pcie_(rd/wr)_other_conf, is it correct? If it is so, I feel this
change
is
independent of this patch and should go as a separte patch?
[Minghuan] dw_pcie(rd/wr)_other_confg only calls the
dw_pcie_prog_viewport_mem/io_outbound when
we only use 2 ATUs.
The patch is to support 4ATUs. If no the calls, ATU2(MEM) and ATU3 will
not be initialized, and PCIe device driver will be broken.
When you have only 2 ATUs (CFG0=MEM & CFG1=IO), you are calling
mem/io_outbound() twice with the same writes which is not the case in the
original driver. So, I mentioned it should go as a separate patch.
[Minghuan] Sorry, I do not understand what you mean.
How to separate patch?
A patch is to add the following code based on original code
+ /* set ATUs setting for MEM and IO */
+ dw_pcie_prog_viewport_mem_outbound(pp);
+ dw_pcie_prog_viewport_io_outbound(pp);
+
But why add this patch? 2 ATUs does not need them.
Only 4 ATUs support needs them.
Then you may have to add a condition here, num_atus >= 4?
And them are also ok for 2 ATUs.
You are just re-writing them anyway, so I don't see a place for them here.
So, this fragment should just work,
+++
if (num_atus >=4 ) {
dw_pcie_prog_viewport_mem_outbound(pp);
dw_pcie_prog_viewport_io_outbound(pp);
}
+++
Is that correct? Am I still missing?
[Minghuan] For 2 ATUs, ATU0 is for MEM as default, ATU1 for IO.
When to access CFG0/CFG1, ATU will temporarily to be changed to
CFG0/CFG1, then will be changed back to MEM/IO.
So the mem/io initialization I added will not bring any harm for 2 ATUs.
Why do we need the judgement 'num_atus >=4'.
For 2 ATUs, mem/io will be initialized every time read/write PCI device
configuration.
Just out of curiosity, is it really required to initialize mem/io everytime
there is a config read/write? Would it not work when initialized just once like
for the case of 4 ATUs?
[Minghuan] For 2 ATUs, CFG0 and MEM share ATU0. So when access PCIe
device configuration
ATU0 will be changed to CFG0 setting, and then be changed to MEM setting
for MEM support.
- Srikanth
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
