Hi Stephen,
On 21 October 2015 at 14:46, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/17/2015 11:50 AM, Simon Glass wrote:
Adjust the Tegra PCI driver to support driver model and move all boards over at the same time. This can make use of some generic driver model code, such as the range decoding logic.
FYI, this patch has quite a few conflicts with my series to add Tegra210 support. See the git branch I mentioned earlier for a quick-n-dirty resolution to the conflicts, and subsequent fixups.
I've rebased to mainline so hopefully we are OK now. I'd like to get this in as I have a PCI series on top of it.
diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig index a5b7e0d..aff4900 100644 --- a/arch/arm/mach-tegra/Kconfig +++ b/arch/arm/mach-tegra/Kconfig @@ -12,6 +12,7 @@ config TEGRA_ARMV7_COMMON select DM_I2C select DM_SPI select DM_GPIO
select DM_PCII wonder if we shouldn't create the TEGRA_COMMON config variable now, since many of the options are common to both TEGRA_ARMV7_COMMON and TEGRA210. Perhaps also TEGRA_ARMV8_COMMON too, although currently there's only support for a single ARMV8 Tegra chip in U-Boot.
Sounds like a good idea to me.
diff --git a/drivers/pci/pci_tegra.c b/drivers/pci/pci_tegra.c
-#define DEBUG
I actually find the debug prints useful, but yes I suppose debug should be turned off!
-static int tegra_pcie_read_conf(struct pci_controller *hose, pci_dev_t bdf,
int where, u32 *value)+static int pci_tegra_read_config(struct udevice *bus, pci_dev_t bdf,
uint offset, ulong *valuep,enum pci_size_t size)
struct tegra_pcie *pcie = to_tegra_pcie(hose);unsigned long address;
struct tegra_pcie *pcie = dev_get_priv(bus);unsigned long address, value; int err;
err = tegra_pcie_conf_address(pcie, bdf, where, &address);
err = tegra_pcie_conf_address(pcie, bdf, offset, &address); if (err < 0) {
*value = 0xffffffff;return 1;
value = 0xffffffff;goto done; }
*value = readl(address);
value = readl(address);This function used to be used only for dword-sized reads. Presumably in that case, "where" was always dword-aligned. When used for word/byte-sized reads, the wrappers to do that (e.g. pci_hose_read_config_word_via_dword) used to force the address passed to this function to be dword-aligned. Now that this function handles all sizes of read, I think it needs to align the address itself, doesn't it; i.e. pass "offset & ~3" to tegra_pcie_conf_address() or "address & ~3" to readl()? Same for the write function below.
This seems to work in practice without generating any alignment faults though; I'm not sure why. Perhaps I'm missing something, or we're just getting lucky?
tegra_pcie_conf_address() should be doing that correctly.
+int pci_skip_dev(struct pci_controller *hose, pci_dev_t dev)
The diff might be slightly smaller/clearer if this function wasn't moved?
OK, I'll leave it. I do like the U_BOOT_DRIVER() thing to be at the end of the file, but it is still very close to the end.
Regards, Simon