public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Stephen Warren <swarren@wwwdotorg.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v4 8/8] dm: tegra: pci: Convert tegra boards to driver model for PCI
Date: Mon, 16 Nov 2015 16:13:57 -0700	[thread overview]
Message-ID: <564A6335.5060601@wwwdotorg.org> (raw)
In-Reply-To: <1447474781-26929-9-git-send-email-sjg@chromium.org>

On 11/13/2015 09:19 PM, 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.

To make this series work on Jetson TX1, two things need to happen.

First, this patch reverts a change I made earlier to move the call to 
tegra_pcie_board_init() to an earlier point in the initialization 
process, i.e. before /any/ part of the PCIe or pad HW is accessed. To 
solve that, apply the following:

> diff --git a/drivers/pci/pci_tegra.c b/drivers/pci/pci_tegra.c
> index ed363e4220c4..8f089b3f6bbe 100644
> --- a/drivers/pci/pci_tegra.c
> +++ b/drivers/pci/pci_tegra.c
> @@ -433,6 +435,11 @@ static int tegra_pcie_parse_port_info(const void *fdt, int node,
>         return 0;
>  }
>
> +int __weak tegra_pcie_board_init(void)
> +{
> +       return 0;
> +}
> +
>  static int tegra_pcie_parse_dt(const void *fdt, int node, enum tegra_pci_id id,
>                                struct tegra_pcie *pcie)
>  {
> @@ -460,6 +467,8 @@ static int tegra_pcie_parse_dt(const void *fdt, int node, enum tegra_pci_id id,
>                 return err;
>         }
>
> +       tegra_pcie_board_init();
> +
>         pcie->phy = tegra_xusb_phy_get(TEGRA_XUSB_PADCTL_PCIE);
>         if (pcie->phy) {
>                 err = tegra_xusb_phy_prepare(pcie->phy);
> @@ -512,11 +521,6 @@ static int tegra_pcie_parse_dt(const void *fdt, int node, enum tegra_pci_id id,
>         return 0;
>  }
>
> -int __weak tegra_pcie_board_init(void)
> -{
> -       return 0;
> -}
> -
>  static int tegra_pcie_power_on(struct tegra_pcie *pcie)
>  {
>         const struct tegra_pcie_soc *soc = pcie->soc;
> @@ -534,8 +538,6 @@ static int tegra_pcie_power_on(struct tegra_pcie *pcie)
>                 return err;
>         }
>
> -       tegra_pcie_board_init();
> -
>         err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_PCIE,
>                                                 PERIPH_ID_PCIE);
>         if (err < 0) {

Second, the removal of the following piece of code:

> diff --git a/drivers/pci/pci_tegra.c b/drivers/pci/pci_tegra.c

> -static int process_nodes(const void *fdt, int nodes[], unsigned int count)
>   {
> -	unsigned int i;
> -	uint64_t dram_end;
> -	uint32_t pci_dram_size;
> -
> -	/* Clip PCI-accessible DRAM to 32-bits */
> -	dram_end = ((uint64_t)NV_PA_SDRAM_BASE) + gd->ram_size;
> -	if (dram_end > 0x100000000)
> -		dram_end = 0x100000000;
> -	pci_dram_size = dram_end - NV_PA_SDRAM_BASE;

... means that the PCI resource structure that represents system memory 
ends up with size=0 rather than the expected size=0x80000000. That's 
because gd->ram_size is 4GB and sizeof(res->size)==4.

Perhaps I could solve this by defining CONFIG_SYS_PCI_64BIT, which 
changes the size of res->size to 8 bytes. However:

a) I'm a bit worried that changing the size of that type will lead to 
other unexpected behavioural changes, e.g. interactions with DT parsing 
due to cell count differences.

b) This only solves the integer overflow issue. In fact, 
res->base+res->size should fit inside 32-bits since IIRC the PCIe 
controller can only access 32-bit addresses, and hence we need to clip 
the RAM size to acknowledge that, which the code above was doing. Doing 
such a clipping operation in the generic/shared decode_regions() doesn't 
feel like a good idea, at least not unconditionally.

In practice, (a) doesn't seem to be a problem, and perhaps we can ignore 
(b), since board_get_usable_ram_top() (in arch/arm/mach-tegra/board2.c) 
limits usable RAM size to fit within 32 bits for similar reasons. So, I 
know that no allocated memory buffer will ever be above 32 bits, so it 
doesn't matter in practice whether a PCI region's base+size fits into 32 
bits or not.

What are your thoughts on this?

BTW, your V4 plus these fixes is available at:

git://github.com/swarren/u-boot.git test-pci-dm-v4

  reply	other threads:[~2015-11-16 23:13 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-14  4:19 [U-Boot] [PATCH v4 0/8] dm: pci: tegra: Convert Tegra PCI to driver model Simon Glass
2015-11-14  4:19 ` [U-Boot] [PATCH v4 1/8] dm: tegra: pci: Move CONFIG_PCI_TEGRA to Kconfig Simon Glass
2015-11-14  4:19 ` [U-Boot] [PATCH v4 2/8] dm: pci: Avoid a driver model build error with CONFIG_CMD_PCI_ENUM Simon Glass
2015-11-14  4:19 ` [U-Boot] [PATCH v4 3/8] dm: pci: Set up the SDRAM mapping correctly Simon Glass
2015-11-14  4:19 ` [U-Boot] [PATCH v4 4/8] dm: pci: Support decoding ranges with duplicate entries Simon Glass
2015-11-14  4:19 ` [U-Boot] [PATCH v4 5/8] dm: pci: Add functions to emulate 8- and 16-bit access Simon Glass
2015-11-14  4:19 ` [U-Boot] [PATCH v4 6/8] dm: pci: Add a function to get the controller for a bus Simon Glass
2015-11-14  4:19 ` [U-Boot] [PATCH v4 7/8] dm: pci: Add a function to find the regions for a PCI bus Simon Glass
2015-11-14  4:19 ` [U-Boot] [PATCH v4 8/8] dm: tegra: pci: Convert tegra boards to driver model for PCI Simon Glass
2015-11-16 23:13   ` Stephen Warren [this message]
2015-11-16 23:20     ` Simon Glass
2015-11-16 23:27       ` Stephen Warren
2015-11-16 19:12 ` [U-Boot] [PATCH v4 0/8] dm: pci: tegra: Convert Tegra PCI to driver model Stephen Warren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=564A6335.5060601@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox