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. Thierry
pgpTNNAR4YuKN.pgp
Description: PGP signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot