On 01/24/2015 06:24 AM, Simon Glass wrote:
Hi Stephen,
On 23 January 2015 at 23:35, Stephen Warren <swar...@wwwdotorg.org> wrote:
On 01/23/2015 09:37 PM, Bin Meng wrote:
Hi Stephen,
On Sat, Jan 24, 2015 at 12:49 AM, Stephen Warren <swar...@wwwdotorg.org> wrote:
On 01/23/2015 03:19 AM, Thierry Reding wrote:
On Thu, Jan 22, 2015 at 12:04:06AM +0800, Bin Meng wrote:
Hi Thierry,
On Wed, Jan 21, 2015 at 5:40 PM, Thierry Reding <tred...@nvidia.com>
wrote:
On Wed, Jan 21, 2015 at 05:15:42PM +0800, Bin Meng wrote:
Hi Thierry,
On Wed, Jan 21, 2015 at 4:24 PM, Thierry Reding <tred...@nvidia.com>
wrote:
On Wed, Jan 21, 2015 at 10:37:07AM +0800, Bin Meng wrote:
Hi,
On Wed, Jan 21, 2015 at 3:05 AM, Simon Glass <s...@chromium.org>
wrote:
Hi Sjoerd,
On 20 January 2015 at 10:06, Sjoerd Simons
<sjoerd.sim...@collabora.co.uk> wrote:
commit a62e84d7b1824a202dd incorrectly changed the tegra pci code
to the
new fdtdec pci helpers. To get the device index of the root port,
the
"reg" property should be parsed from the dtb (as was previously the
case).
With this patch i can successfully network boot my jetson tk1
Signed-off-by: Sjoerd Simons <sjoerd.sim...@collabora.co.uk>
---
drivers/pci/pci_tegra.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
Can you also please take a look at this patch?
http://patchwork.ozlabs.org/patch/430815/
It tries to support both options.
Although I still don't see how the Tegra's dts is written, I feel
this
patch is doing correctly.
It's in the U-Boot tree, look at arch/arm/dts/tegra124.dtsi for an
example.
Got it. I see:
pci@1,0 {
device_type = "pci";
assigned-addresses = <0x82000800 0 0x01000000
0 0x1000>;
reg = <0x000800 0 0 0 0>;
status = "disabled";
#address-cells = <3>;
#size-cells = <2>;
ranges;
nvidia,num-lanes = <2>;
};
So I would read this 'reg = <0x000800 0 0 0 0>' as this is a
downstream port with device number 1 of the root complex.
Correct. Note that these root ports don't appear on the bus using the
regular configuration space accesses, so the definition here is
arbitrary, though in a way to mirror what PCI would typically look like
(host bridge 00:00.0, root ports 00:01.0..00:0N.0).
The Linux kernel driver (and the U-Boot driver for that matter) rely on
this numbering, though, for some aspects of configuration of the root
ports.
diff --git a/drivers/pci/pci_tegra.c b/drivers/pci/pci_tegra.c
index f9e05ad..67b5fdf 100644
--- a/drivers/pci/pci_tegra.c
+++ b/drivers/pci/pci_tegra.c
@@ -459,7 +459,6 @@ static int tegra_pcie_parse_port_info(const
void *fdt, int node,
unsigned int *lanes)
{
struct fdt_pci_addr addr;
- pci_dev_t bdf;
int err;
err = fdtdec_get_int(fdt, node, "nvidia,num-lanes", 0);
@@ -470,13 +469,13 @@ static int tegra_pcie_parse_port_info(const
void *fdt, int node,
*lanes = err;
- err = fdtdec_get_pci_bdf(fdt, node, &addr, &bdf);
+ err = fdtdec_get_pci_addr(fdt, node, 0, "reg", &addr);
I suggest replace 0 to FDT_PCI_SPACE_CONFIG.
I do like how 0 actually transports the meaning of "don't care" here.
The reg property encodes only the BDF, whereas the configuration space
region for the root ports is encoded in the assigned-addresses
property.
Looking at the fdtdec_get_pci_addr() implementation I notice that it
uses the type parameter to match on the type of region. Devices can
have
more than one region of the same type. How is that supposed to work
with
this function. Perhaps it's nothing we care about for the fdtdec API
since we don't access those regions anyway from FDT code?
Ah, yes, some devices may have multiple regions of the same type.
Perhaps we need another parameter bar_index for this api? So far this
API is not used by FDT codes. It is used by the ns16550 driver where
pci ns16550 normally has two bars, one memory and one i/o.
Why not use the BARs directly in the ns16550 driver rather than looking
it up from the device tree? I assume the device will have to be
enumerated anyway to make it work properly, at which point addresses
should've been assigned to the memory and I/O BARs.
It is because we cannot predict which bar to look up if we hardcod
that in the generic ns16550 driver. Normally PCI ns16550 registers can
be memory-mapped or I/O mapped and it could use any of the 6 BARs.
What's more, on x86 for memory-mapped and I/O mapped they use
different instructions to access the registers, and there is one build
time macro (CONFIG_SYS_NS16550_PORT_MAPPED) to control this.
Surely the vendor/device ID (or perhaps subvendor/device ID) directly imply
which BAR is used for this purpose? It's really part of the specification of
the interface to HW, so a particular bit of HW shouldn't be randomly
deciding to use a different BAR register each power-on.
Yes, the vendor/device ID should be unique to identify a device.
However, if we solely rely on this pair to select which BAR to use in
the pci device driver, we may end up with a big table in the driver
codes to map the vendor/device ID to a BAR number. Each time we
support a new device, say pci ns16550, we have to update the driver to
add the vendor/device ID mapping. This will become unmaintainable as
time goes by. The FDT provides us a way to have such configuration
data removed from driver codes and that's why we all are in favor of
FDT.
The other benefit I could think of to use FDT to specify a BAR is that
we can avoid using the erroneous BAR and ask the driver to use what we
know the funtional BAR. For example, for pci ns16550 if we hardcod the
BAR selection in the ns16550 driver to use the memory-mapped BAR, on
some chipset there are possibilities that only i/o-mapped BAR could
work correctly. We can easily do such thing via FDT but if we move
such logic into the driver, we may have to create vendor/device ID
test logic.
In general however, there is no DT. So you'll always have to support a
table in the driver. If you support the table at all, you may as well
always just use the table to avoid the complexity associated with
multiple methods of providing the same data. I don't agree that such a
table would be unmaintainable.
In ns16550 we are using device tree to store this information and
there is no table in the driver.
So boards that wish to use the ns16550 driver must convert to DT to use
it? That seems like quite an imposition.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
