[U-Boot] [PATCH 0/8] dm: pci: tegra: Convert Tegra PCI to driver model

[U-Boot] [PATCH 0/8] dm: pci: tegra: Convert Tegra PCI to driver model

[Simon Glass]

This series converts all Tegra boards to use driver model for PCI. The net
effect should be no change in functionality.

A few additional features are added to make this possible:
- Helper functions to support accessing 8- and 16-bit values within a 32-bit
  word
- Fixing a build error for CONFIG_CMD_PCI_ENUM
- Decoding the PCI ranges property such that configuration ranges are
  ignored
- Supporting bus-master devices on boards where RAM does not start at 0

This series is tested on beaver. It is available at u-boot-dm/tegra-working.


Simon Glass (8):
  dm: tegra: pci: Move CONFIG_PCI_TEGRA to Kconfig
  dm: pci: Avoid a driver model build error with CONFIG_CMD_PCI_ENUM
  RFC: dm: pci: Set up the SDRAM mapping correctly
  dm: pci: Support decoding ranges with duplicate entries
  dm: pci: Add functions to emulate 8- and 16-bit access
  dm: pci: Add a function to get the controller for a bus
  dm: pci: Add a function to find the regions for a PCI bus
  dm: tegra: pci: Convert tegra boards to driver model for PCI

 arch/arm/mach-tegra/Kconfig  |   1 +
 common/cmd_pci.c             |   4 +
 configs/apalis_t30_defconfig |   1 +
 configs/beaver_defconfig     |   1 +
 configs/cardhu_defconfig     |   1 +
 configs/jetson-tk1_defconfig |   1 +
 configs/trimslice_defconfig  |   1 +
 drivers/pci/Kconfig          |  10 ++
 drivers/pci/pci-uclass.c     |  99 ++++++++++-
 drivers/pci/pci_tegra.c      | 402 +++++++++++++++----------------------------
 include/configs/apalis_t30.h |   1 -
 include/configs/beaver.h     |   1 -
 include/configs/cardhu.h     |   1 -
 include/configs/jetson-tk1.h |   1 -
 include/configs/trimslice.h  |   1 -
 include/fdtdec.h             |   3 -
 include/pci.h                |  51 ++++++
 lib/fdtdec.c                 |   3 -
 18 files changed, 305 insertions(+), 278 deletions(-)

-- 
2.6.0.rc2.230.g3dd15c0

[U-Boot] [PATCH 1/8] dm: tegra: pci: Move CONFIG_PCI_TEGRA to Kconfig

[Simon Glass]

Move this option to Kconig and fix up all users.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 configs/apalis_t30_defconfig |  1 +
 configs/beaver_defconfig     |  1 +
 configs/cardhu_defconfig     |  1 +
 configs/jetson-tk1_defconfig |  1 +
 configs/trimslice_defconfig  |  1 +
 drivers/pci/Kconfig          | 10 ++++++++++
 include/configs/apalis_t30.h |  1 -
 include/configs/beaver.h     |  1 -
 include/configs/cardhu.h     |  1 -
 include/configs/jetson-tk1.h |  1 -
 include/configs/trimslice.h  |  1 -
 11 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/configs/apalis_t30_defconfig b/configs/apalis_t30_defconfig
index 8971d29..871dc43 100644
--- a/configs/apalis_t30_defconfig
+++ b/configs/apalis_t30_defconfig
@@ -12,6 +12,7 @@ CONFIG_SYS_PROMPT="Apalis T30 # "
 # CONFIG_CMD_NFS is not set
 CONFIG_NETDEVICES=y
 CONFIG_E1000=y
+CONFIG_PCI_TEGRA=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_USE_PRIVATE_LIBGCC=y
diff --git a/configs/beaver_defconfig b/configs/beaver_defconfig
index 4a6f6e4..dfc9adf 100644
--- a/configs/beaver_defconfig
+++ b/configs/beaver_defconfig
@@ -12,6 +12,7 @@ CONFIG_SYS_PROMPT="Tegra30 (Beaver) # "
 # CONFIG_CMD_SETEXPR is not set
 # CONFIG_CMD_NFS is not set
 CONFIG_SPI_FLASH=y
+CONFIG_PCI_TEGRA=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_USE_PRIVATE_LIBGCC=y
diff --git a/configs/cardhu_defconfig b/configs/cardhu_defconfig
index 722bbeb..9e4a123 100644
--- a/configs/cardhu_defconfig
+++ b/configs/cardhu_defconfig
@@ -12,6 +12,7 @@ CONFIG_SYS_PROMPT="Tegra30 (Cardhu) # "
 # CONFIG_CMD_SETEXPR is not set
 # CONFIG_CMD_NFS is not set
 CONFIG_SPI_FLASH=y
+CONFIG_PCI_TEGRA=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_USE_PRIVATE_LIBGCC=y
diff --git a/configs/jetson-tk1_defconfig b/configs/jetson-tk1_defconfig
index 6df5c7e..7b1992b 100644
--- a/configs/jetson-tk1_defconfig
+++ b/configs/jetson-tk1_defconfig
@@ -12,6 +12,7 @@ CONFIG_SYS_PROMPT="Tegra124 (Jetson TK1) # "
 # CONFIG_CMD_SETEXPR is not set
 # CONFIG_CMD_NFS is not set
 CONFIG_SPI_FLASH=y
+CONFIG_PCI_TEGRA=y
 CONFIG_TEGRA114_SPI=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
diff --git a/configs/trimslice_defconfig b/configs/trimslice_defconfig
index 18292e2..08e1b62 100644
--- a/configs/trimslice_defconfig
+++ b/configs/trimslice_defconfig
@@ -12,6 +12,7 @@ CONFIG_SYS_PROMPT="Tegra20 (TrimSlice) # "
 # CONFIG_CMD_SETEXPR is not set
 # CONFIG_CMD_NFS is not set
 CONFIG_SPI_FLASH=y
+CONFIG_PCI_TEGRA=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_USE_PRIVATE_LIBGCC=y
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 167d405..c219c19 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -19,4 +19,14 @@ config PCI_SANDBOX
 	  the device tree but the normal PCI scan technique is used to find
 	  then.
 
+config PCI_TEGRA
+	bool "Tegra PCI support"
+	depends on TEGRA
+	help
+	  Enable support for the PCIe controller found on some generations of
+	  Tegra. Tegra20 has 2 root ports with a total of 4 lanes, Tegra30 has
+	  3 root ports with a total of 6 lanes and Tegra124 has 2 root ports
+	  with a total of 5 lanes. Some boards require this for Ethernet
+	  support to work (e.g. beaver, jetson-tk1).
+
 endmenu
diff --git a/include/configs/apalis_t30.h b/include/configs/apalis_t30.h
index fe1ef9d..7552a80 100644
--- a/include/configs/apalis_t30.h
+++ b/include/configs/apalis_t30.h
@@ -49,7 +49,6 @@
 
 /* PCI host support */
 #define CONFIG_PCI
-#define CONFIG_PCI_TEGRA
 #define CONFIG_PCI_PNP
 #define CONFIG_CMD_PCI
 #define CONFIG_CMD_PCI_ENUM
diff --git a/include/configs/beaver.h b/include/configs/beaver.h
index 1790f60..9e8dcf3f7 100644
--- a/include/configs/beaver.h
+++ b/include/configs/beaver.h
@@ -73,7 +73,6 @@
 
 /* PCI host support */
 #define CONFIG_PCI
-#define CONFIG_PCI_TEGRA
 #define CONFIG_PCI_PNP
 #define CONFIG_CMD_PCI
 #define CONFIG_CMD_PCI_ENUM
diff --git a/include/configs/cardhu.h b/include/configs/cardhu.h
index ce6b158..174bb48 100644
--- a/include/configs/cardhu.h
+++ b/include/configs/cardhu.h
@@ -75,7 +75,6 @@
 
 /* PCI host support */
 #define CONFIG_PCI
-#define CONFIG_PCI_TEGRA
 #define CONFIG_PCI_PNP
 #define CONFIG_CMD_PCI
 #define CONFIG_CMD_PCI_ENUM
diff --git a/include/configs/jetson-tk1.h b/include/configs/jetson-tk1.h
index e87a010..618ffa6 100644
--- a/include/configs/jetson-tk1.h
+++ b/include/configs/jetson-tk1.h
@@ -59,7 +59,6 @@
 
 /* PCI host support */
 #define CONFIG_PCI
-#define CONFIG_PCI_TEGRA
 #define CONFIG_PCI_PNP
 #define CONFIG_CMD_PCI
 #define CONFIG_CMD_PCI_ENUM
diff --git a/include/configs/trimslice.h b/include/configs/trimslice.h
index 2ab5511..bdf1bd4 100644
--- a/include/configs/trimslice.h
+++ b/include/configs/trimslice.h
@@ -58,7 +58,6 @@
 
 /* PCI host support */
 #define CONFIG_PCI
-#define CONFIG_PCI_TEGRA
 #define CONFIG_PCI_PNP
 #define CONFIG_CMD_PCI
 #define CONFIG_CMD_PCI_ENUM
-- 
2.6.0.rc2.230.g3dd15c0

[U-Boot] [PATCH 1/8] dm: tegra: pci: Move CONFIG_PCI_TEGRA to Kconfig

[Stephen Warren]

On 10/17/2015 11:49 AM, Simon Glass wrote:
> Move this option to Kconig and fix up all users.

What are your thoughts on how/when to merge this? The series (mainly the 
final patch) conflicts with my series to add Tegra210 support, plus one 
of them needs rebasing onto the other so that either the board support 
my series adds gets adjusted by this series, or my series is adjusted 
for all the changes in this series.

Obviously I'd hope my series gets merged first since I sent it much 
earlier:-)

FWIW, I rebased this series on top of my series to validate it, and 
pushed it to:

git://github.com/swarren/u-boot.git pci-dm-conversion-rebase

That seems to work OK, but the final patch I added there needs 
integration into earlier patches and/or cleanup work.

[U-Boot] [PATCH 1/8] dm: tegra: pci: Move CONFIG_PCI_TEGRA to Kconfig

[Simon Glass]

Hi Stephen,

On 21 October 2015 at 14:13, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 10/17/2015 11:49 AM, Simon Glass wrote:
>>
>> Move this option to Kconig and fix up all users.
>
>
> What are your thoughts on how/when to merge this? The series (mainly the
> final patch) conflicts with my series to add Tegra210 support, plus one of
> them needs rebasing onto the other so that either the board support my
> series adds gets adjusted by this series, or my series is adjusted for all
> the changes in this series.
>
> Obviously I'd hope my series gets merged first since I sent it much
> earlier:-)
>
> FWIW, I rebased this series on top of my series to validate it, and pushed
> it to:
>
> git://github.com/swarren/u-boot.git pci-dm-conversion-rebase
>
> That seems to work OK, but the final patch I added there needs integration
> into earlier patches and/or cleanup work.

I don't mind which order it goes in. I cannot test on T210 anyway so I
may look to you to sort that out. Let me know what suits. I'll likely
respin this early next week and if your stuff in applied to tegra by
then then I'll base on top of it.

Regards,
Simon

[U-Boot] [PATCH 2/8] dm: pci: Avoid a driver model build error with CONFIG_CMD_PCI_ENUM

[Simon Glass]

This is not supported with driver model, so print a message instead of
generating a build error. Rescanning PCI is not yet implemented.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/cmd_pci.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/common/cmd_pci.c b/common/cmd_pci.c
index dcecef8..69c5332 100644
--- a/common/cmd_pci.c
+++ b/common/cmd_pci.c
@@ -457,7 +457,11 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		return pci_cfg_display(bdf, addr, size, value);
 #ifdef CONFIG_CMD_PCI_ENUM
 	case 'e':
+# ifdef CONFIG_DM_PCI
+		printf("This command is not yet supported with driver model\n");
+# else
 		pci_init();
+# endif
 		return 0;
 #endif
 	case 'n':		/* next */
-- 
2.6.0.rc2.230.g3dd15c0

[U-Boot] [PATCH 2/8] dm: pci: Avoid a driver model build error with CONFIG_CMD_PCI_ENUM

[Stephen Warren]

On 10/17/2015 11:49 AM, Simon Glass wrote:
> This is not supported with driver model, so print a message instead of
> generating a build error. Rescanning PCI is not yet implemented.

> diff --git a/common/cmd_pci.c b/common/cmd_pci.c

> @@ -457,7 +457,11 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>   		return pci_cfg_display(bdf, addr, size, value);
>   #ifdef CONFIG_CMD_PCI_ENUM
>   	case 'e':
> +# ifdef CONFIG_DM_PCI
> +		printf("This command is not yet supported with driver model\n");
> +# else
>   		pci_init();
> +# endif

That feature is enabled on most/all Tegra boards with PCI support. 
Hopefully nobody will miss it; I guess I haven't used it so I don't 
object to this change.

However, wouldn't it be better to remove CONFIG_CMD_PCI_ENUM from the 
config header rather than leaving the command enabled yet 
non-functional? Or are you planning on implementing this path in the 
near future?

[U-Boot] [PATCH 2/8] dm: pci: Avoid a driver model build error with CONFIG_CMD_PCI_ENUM

[Simon Glass]

Hi Stephen,

On 21 October 2015 at 14:16, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 10/17/2015 11:49 AM, Simon Glass wrote:
>>
>> This is not supported with driver model, so print a message instead of
>> generating a build error. Rescanning PCI is not yet implemented.
>
>
>> diff --git a/common/cmd_pci.c b/common/cmd_pci.c
>
>
>> @@ -457,7 +457,11 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int
>> argc, char * const argv[])
>>                 return pci_cfg_display(bdf, addr, size, value);
>>   #ifdef CONFIG_CMD_PCI_ENUM
>>         case 'e':
>> +# ifdef CONFIG_DM_PCI
>> +               printf("This command is not yet supported with driver
>> model\n");
>> +# else
>>                 pci_init();
>> +# endif
>
>
> That feature is enabled on most/all Tegra boards with PCI support. Hopefully
> nobody will miss it; I guess I haven't used it so I don't object to this
> change.
>
> However, wouldn't it be better to remove CONFIG_CMD_PCI_ENUM from the config
> header rather than leaving the command enabled yet non-functional? Or are
> you planning on implementing this path in the near future?

I was looking for feedback on why anyone would use this option. It's
not clear to me what it is for.

We can implement it for driver model if it is needed.

Regards,
Simon

[U-Boot] [PATCH 2/8] dm: pci: Avoid a driver model build error with CONFIG_CMD_PCI_ENUM

[Stephen Warren]

On 10/23/2015 09:47 AM, Simon Glass wrote:
> Hi Stephen,
>
> On 21 October 2015 at 14:16, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 10/17/2015 11:49 AM, Simon Glass wrote:
>>>
>>> This is not supported with driver model, so print a message instead of
>>> generating a build error. Rescanning PCI is not yet implemented.
>>
>>
>>> diff --git a/common/cmd_pci.c b/common/cmd_pci.c
>>
>>
>>> @@ -457,7 +457,11 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int
>>> argc, char * const argv[])
>>>                  return pci_cfg_display(bdf, addr, size, value);
>>>    #ifdef CONFIG_CMD_PCI_ENUM
>>>          case 'e':
>>> +# ifdef CONFIG_DM_PCI
>>> +               printf("This command is not yet supported with driver
>>> model\n");
>>> +# else
>>>                  pci_init();
>>> +# endif
>>
>>
>> That feature is enabled on most/all Tegra boards with PCI support. Hopefully
>> nobody will miss it; I guess I haven't used it so I don't object to this
>> change.
>>
>> However, wouldn't it be better to remove CONFIG_CMD_PCI_ENUM from the config
>> header rather than leaving the command enabled yet non-functional? Or are
>> you planning on implementing this path in the near future?
>
> I was looking for feedback on why anyone would use this option. It's
> not clear to me what it is for.

I assume it's to support hot-pluggable PCI connectors? Or perhaps it's 
just useful during debug of PCI drivers?

> We can implement it for driver model if it is needed.

I'd be happy disabling it in the configs myself. Thierry, you enabled 
the option when you first implemented the Tegra PCIe controller driver. 
Do you have a preference?

[U-Boot] [PATCH 2/8] dm: pci: Avoid a driver model build error with CONFIG_CMD_PCI_ENUM

[Thierry Reding]

On Fri, Oct 23, 2015 at 11:30:04AM -0600, Stephen Warren wrote:
> On 10/23/2015 09:47 AM, Simon Glass wrote:
> >Hi Stephen,
> >
> >On 21 October 2015 at 14:16, Stephen Warren <swarren@wwwdotorg.org> wrote:
> >>On 10/17/2015 11:49 AM, Simon Glass wrote:
> >>>
> >>>This is not supported with driver model, so print a message instead of
> >>>generating a build error. Rescanning PCI is not yet implemented.
> >>
> >>
> >>>diff --git a/common/cmd_pci.c b/common/cmd_pci.c
> >>
> >>
> >>>@@ -457,7 +457,11 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int
> >>>argc, char * const argv[])
> >>>                 return pci_cfg_display(bdf, addr, size, value);
> >>>   #ifdef CONFIG_CMD_PCI_ENUM
> >>>         case 'e':
> >>>+# ifdef CONFIG_DM_PCI
> >>>+               printf("This command is not yet supported with driver
> >>>model\n");
> >>>+# else
> >>>                 pci_init();
> >>>+# endif
> >>
> >>
> >>That feature is enabled on most/all Tegra boards with PCI support. Hopefully
> >>nobody will miss it; I guess I haven't used it so I don't object to this
> >>change.
> >>
> >>However, wouldn't it be better to remove CONFIG_CMD_PCI_ENUM from the config
> >>header rather than leaving the command enabled yet non-functional? Or are
> >>you planning on implementing this path in the near future?
> >
> >I was looking for feedback on why anyone would use this option. It's
> >not clear to me what it is for.
> 
> I assume it's to support hot-pluggable PCI connectors? Or perhaps it's just
> useful during debug of PCI drivers?
> 
> >We can implement it for driver model if it is needed.
> 
> I'd be happy disabling it in the configs myself. Thierry, you enabled the
> option when you first implemented the Tegra PCIe controller driver. Do you
> have a preference?

Removing it from the configs is fine with me. I don't think I've
actually ever used it since all the way back when I was working on the
initial driver prototype.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151109/58344b4f/attachment.sig>

[U-Boot] [PATCH 3/8] RFC: dm: pci: Set up the SDRAM mapping correctly

[Simon Glass]

SDRAM doesn't always start at 0. Adjust the region mapping so that it works
on platforms where SDRAM is somewhere else.

This needs testing on other platforms.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/pci/pci-uclass.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index 0756bbe..6dda056 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -674,8 +674,8 @@ static int decode_regions(struct pci_controller *hose, const void *blob,
 			  int parent_node, int node)
 {
 	int pci_addr_cells, addr_cells, size_cells;
+	phys_addr_t base = 0, addr;
 	int cells_per_record;
-	phys_addr_t addr;
 	const u32 *prop;
 	int len;
 	int i;
@@ -729,8 +729,11 @@ static int decode_regions(struct pci_controller *hose, const void *blob,
 	addr = gd->ram_size;
 	if (gd->pci_ram_top && gd->pci_ram_top < addr)
 		addr = gd->pci_ram_top;
-	pci_set_region(hose->regions + hose->region_count++, 0, 0, addr,
-		       PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
+#ifdef CONFIG_SYS_SDRAM_BASE
+	base = CONFIG_SYS_SDRAM_BASE;
+#endif
+	pci_set_region(hose->regions + hose->region_count++, base, base,
+		       addr, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
 
 	return 0;
 }
-- 
2.6.0.rc2.230.g3dd15c0

[U-Boot] [PATCH 3/8] RFC: dm: pci: Set up the SDRAM mapping correctly

[Stephen Warren]

On 10/17/2015 11:50 AM, Simon Glass wrote:
> SDRAM doesn't always start at 0. Adjust the region mapping so that it works
> on platforms where SDRAM is somewhere else.
>
> This needs testing on other platforms.

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

> @@ -729,8 +729,11 @@ static int decode_regions(struct pci_controller *hose, const void *blob,
>   	addr = gd->ram_size;
>   	if (gd->pci_ram_top && gd->pci_ram_top < addr)
>   		addr = gd->pci_ram_top;
> -	pci_set_region(hose->regions + hose->region_count++, 0, 0, addr,
> -		       PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
> +#ifdef CONFIG_SYS_SDRAM_BASE
> +	base = CONFIG_SYS_SDRAM_BASE;
> +#endif
> +	pci_set_region(hose->regions + hose->region_count++, base, base,
> +		       addr, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);

The variable naming here is extremely misleading; "addr" is actually the 
size of the region not its address. I think it'd be a good idea to 
rename the addr variable while you're fixing this bug, and also 
pci_ram_top too since that's really pci_ram_size.

FWIW, on Tegra210 when I set gd->pci_ram_top = 0x80000000 (coupled with 
SDRAM_BASE being 0x80000000) this function seems to do the right thing; 
at least a PCIe NIC works OK.

[U-Boot] [PATCH 4/8] dm: pci: Support decoding ranges with duplicate entries

[Simon Glass]

At present we add a new resource entry for every range entry. But some range
entries refer to configuration regions. To make this work, avoid adding two
regions of the same time. The later ranges will overwrite the earlier
(configuration) ones.

There does not seem to be a way to distinguish the configuration ranges
other than by ordering.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/pci/pci-uclass.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index 6dda056..a59e468 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -698,6 +698,7 @@ static int decode_regions(struct pci_controller *hose, const void *blob,
 		int space_code;
 		u32 flags;
 		int type;
+		int pos;
 
 		if (len < cells_per_record)
 			break;
@@ -720,9 +721,15 @@ static int decode_regions(struct pci_controller *hose, const void *blob,
 		} else {
 			continue;
 		}
-		debug(" - type=%d\n", type);
-		pci_set_region(hose->regions + hose->region_count++, pci_addr,
-			       addr, size, type);
+		pos = -1;
+		for (i = 0; i < hose->region_count; i++) {
+			if (hose->regions[i].flags == type)
+				pos = i;
+		}
+		if (pos == -1)
+			pos = hose->region_count++;
+		debug(" - type=%d, pos=%d\n", type, pos);
+		pci_set_region(hose->regions + pos, pci_addr, addr, size, type);
 	}
 
 	/* Add a region for our local memory */
-- 
2.6.0.rc2.230.g3dd15c0

[U-Boot] [PATCH 4/8] dm: pci: Support decoding ranges with duplicate entries

[Stephen Warren]

On 10/17/2015 11:50 AM, Simon Glass wrote:
> At present we add a new resource entry for every range entry. But some range
> entries refer to configuration regions. To make this work, avoid adding two
> regions of the same time. The later ranges will overwrite the earlier
> (configuration) ones.

s/time/type/ in the last-but-one line.

What's wrong with having two regions of the same type? Equally, if we 
can "get away" with not storing some of the regions that happen to have 
a duplicate type, why not recast the function so that it only stores 
regions of specific (useful/desired) types, and simply dropping all of 
the other regions. That'd be a lot more consistent than only storing a 
somewhat arbitrary subset of the regions.

> There does not seem to be a way to distinguish the configuration ranges
> other than by ordering.

Well, they do have different addresses too. But yes, the DT binding is 
written so that the entries in ranges must appear in a specific order, 
so order is the correct way to index the entries.

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

> @@ -720,9 +721,15 @@ static int decode_regions(struct pci_controller *hose, const void *blob,
>   		} else {
>   			continue;
>   		}
> -		debug(" - type=%d\n", type);
> -		pci_set_region(hose->regions + hose->region_count++, pci_addr,
> -			       addr, size, type);
> +		pos = -1;
> +		for (i = 0; i < hose->region_count; i++) {
> +			if (hose->regions[i].flags == type)
> +				pos = i;

and break too?

> +		}
> +		if (pos == -1)
> +			pos = hose->region_count++;
> +		debug(" - type=%d, pos=%d\n", type, pos);
> +		pci_set_region(hose->regions + pos, pci_addr, addr, size, type);
>   	}

[U-Boot] [PATCH 4/8] dm: pci: Support decoding ranges with duplicate entries

[Simon Glass]

Hi Stephen,

On 21 October 2015 at 14:25, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 10/17/2015 11:50 AM, Simon Glass wrote:
>>
>> At present we add a new resource entry for every range entry. But some
>> range
>> entries refer to configuration regions. To make this work, avoid adding
>> two
>> regions of the same time. The later ranges will overwrite the earlier
>> (configuration) ones.
>
>
> s/time/type/ in the last-but-one line.
>
> What's wrong with having two regions of the same type? Equally, if we can
> "get away" with not storing some of the regions that happen to have a
> duplicate type, why not recast the function so that it only stores regions
> of specific (useful/desired) types, and simply dropping all of the other
> regions. That'd be a lot more consistent than only storing a somewhat
> arbitrary subset of the regions.

I'm not 100% sure that we want to disallow multiple regions. Although
on non-x86 boards I've haven't seen hardware that supports multiple
regions.

>
>> There does not seem to be a way to distinguish the configuration ranges
>> other than by ordering.
>
>
> Well, they do have different addresses too. But yes, the DT binding is
> written so that the entries in ranges must appear in a specific order, so
> order is the correct way to index the entries.
>
>> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
>
>
>> @@ -720,9 +721,15 @@ static int decode_regions(struct pci_controller
>> *hose, const void *blob,
>>                 } else {
>>                         continue;
>>                 }
>> -               debug(" - type=%d\n", type);
>> -               pci_set_region(hose->regions + hose->region_count++,
>> pci_addr,
>> -                              addr, size, type);
>> +               pos = -1;
>> +               for (i = 0; i < hose->region_count; i++) {
>> +                       if (hose->regions[i].flags == type)
>> +                               pos = i;
>
>
> and break too?

Could do, might be clearer.

>
>
>> +               }
>> +               if (pos == -1)
>> +                       pos = hose->region_count++;
>> +               debug(" - type=%d, pos=%d\n", type, pos);
>> +               pci_set_region(hose->regions + pos, pci_addr, addr, size,
>> type);
>>         }
>
>

Regards,
Simon

[U-Boot] [PATCH 5/8] dm: pci: Add functions to emulate 8- and 16-bit access

[Simon Glass]

Provide a few functions to support using 32-bit access to emulate 8- and
16-bit access.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/pci/pci-uclass.c | 39 +++++++++++++++++++++++++++++++++++++++
 include/pci.h            | 31 +++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index a59e468..1c6df0f 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -916,6 +916,45 @@ int pci_find_first_device(struct udevice **devp)
 	return skip_to_next_device(bus, devp);
 }
 
+ulong pci_conv_32_to_size(ulong value, uint offset, enum pci_size_t size)
+{
+	switch (size) {
+	case PCI_SIZE_8:
+		return (value >> ((offset & 3) * 8)) & 0xff;
+	case PCI_SIZE_16:
+		return (value >> ((offset & 2) * 8)) & 0xffff;
+	default:
+		return value;
+	}
+}
+
+ulong pci_conv_size_to_32(ulong old, ulong value, uint offset,
+			  enum pci_size_t size)
+{
+	uint off_mask;
+	uint val_mask, shift;
+	ulong ldata, mask;
+
+	switch (size) {
+	case PCI_SIZE_8:
+		off_mask = 3;
+		val_mask = 0xff;
+		break;
+	case PCI_SIZE_16:
+		off_mask = 2;
+		val_mask = 0xffff;
+		break;
+	default:
+		return value;
+	}
+	shift = (offset & off_mask) * 8;
+	ldata = (value & val_mask) << shift;
+	mask = val_mask << shift;
+	value = (old & ~mask) | ldata;
+
+	return value;
+}
+
 UCLASS_DRIVER(pci) = {
 	.id		= UCLASS_PCI,
 	.name		= "pci",
diff --git a/include/pci.h b/include/pci.h
index e24c970..f40a83d 100644
--- a/include/pci.h
+++ b/include/pci.h
@@ -1092,6 +1092,37 @@ static inline int pci_read_config_byte(pci_dev_t pcidev, int offset,
 }
 
 /**
+ * pci_conv_32_to_size() - convert a 32-bit read value to the given size
+ *
+ * Some PCI buses must always perform 32-bit reads. The data must then be
+ * shifted and masked to reflect the required access size and offset. This
+ * function performs this transformation.
+ *
+ * @value:	Value to transform (32-bit value read from @offset & ~3)
+ * @offset:	Register offset that was read
+ * @size:	Required size of the result
+ * @return the value that would have been obtained if the read had been
+ * performed@the given offset with the correct size
+ */
+ulong pci_conv_32_to_size(ulong value, uint offset, enum pci_size_t size);
+
+/**
+ * pci_conv_size_to_32() - update a 32-bit value to prepare for a write
+ *
+ * Some PCI buses must always perform 32-bit writes. To emulate a smaller
+ * write the old 32-bit data must be read, updated with the required new data
+ * and written back as a 32-bit value. This function performs the
+ * transformation from the old value to the new value.
+ *
+ * @value:	Value to transform (32-bit value read from @offset & ~3)
+ * @offset:	Register offset that should be written
+ * @size:	Required size of the write
+ * @return the value that should be written as a 32-bit access to @offset & ~3.
+ */
+ulong pci_conv_size_to_32(ulong old, ulong value, uint offset,
+			  enum pci_size_t size);
+
+/**
  * struct dm_pci_emul_ops - PCI device emulator operations
  */
 struct dm_pci_emul_ops {
-- 
2.6.0.rc2.230.g3dd15c0

[U-Boot] [PATCH 5/8] dm: pci: Add functions to emulate 8- and 16-bit access

[Stephen Warren]

On 10/17/2015 11:50 AM, Simon Glass wrote:
> Provide a few functions to support using 32-bit access to emulate 8- and
> 16-bit access.

Reviewed-by: Stephen Warren <swarren@nvidia.com>

[U-Boot] [PATCH 6/8] dm: pci: Add a function to get the controller for a bus

[Simon Glass]

A PCI bus may be a bridge device where the controller is the bridge's
parent. Add a function to return the controller device, given a PCI device.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/pci/pci-uclass.c | 8 ++++++++
 include/pci.h            | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index 1c6df0f..527d3bb 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -53,6 +53,14 @@ struct pci_controller *pci_bus_to_hose(int busnum)
 	return dev_get_uclass_priv(bus);
 }
 
+struct udevice *pci_get_controller(struct udevice *dev)
+{
+	while (device_get_uclass_id(dev->parent) == UCLASS_PCI)
+		dev = dev->parent;
+
+	return dev;
+}
+
 pci_dev_t pci_get_bdf(struct udevice *dev)
 {
 	struct pci_child_platdata *pplat = dev_get_parent_platdata(dev);
diff --git a/include/pci.h b/include/pci.h
index f40a83d..8b471ab 100644
--- a/include/pci.h
+++ b/include/pci.h
@@ -1123,6 +1123,14 @@ ulong pci_conv_size_to_32(ulong old, ulong value, uint offset,
 			  enum pci_size_t size);
 
 /**
+ * pci_get_controller() - obtain the controller to use for a bus
+ *
+ * @dev:	Device to check
+ * @return pointer to the controller device for this bus
+ */
+struct udevice *pci_get_controller(struct udevice *dev);
+
+/**
  * struct dm_pci_emul_ops - PCI device emulator operations
  */
 struct dm_pci_emul_ops {
-- 
2.6.0.rc2.230.g3dd15c0

[U-Boot] [PATCH 6/8] dm: pci: Add a function to get the controller for a bus

[Bin Meng]

Hi Simon,

On Sun, Oct 18, 2015 at 1:50 AM, Simon Glass <sjg@chromium.org> wrote:
> A PCI bus may be a bridge device where the controller is the bridge's
> parent. Add a function to return the controller device, given a PCI device.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  drivers/pci/pci-uclass.c | 8 ++++++++
>  include/pci.h            | 8 ++++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index 1c6df0f..527d3bb 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -53,6 +53,14 @@ struct pci_controller *pci_bus_to_hose(int busnum)
>         return dev_get_uclass_priv(bus);
>  }
>
> +struct udevice *pci_get_controller(struct udevice *dev)
> +{
> +       while (device_get_uclass_id(dev->parent) == UCLASS_PCI)

Please use device_is_on_pci_bus() API.

> +               dev = dev->parent;
> +
> +       return dev;
> +}
> +
>  pci_dev_t pci_get_bdf(struct udevice *dev)
>  {
>         struct pci_child_platdata *pplat = dev_get_parent_platdata(dev);
> diff --git a/include/pci.h b/include/pci.h
> index f40a83d..8b471ab 100644
> --- a/include/pci.h
> +++ b/include/pci.h
> @@ -1123,6 +1123,14 @@ ulong pci_conv_size_to_32(ulong old, ulong value, uint offset,
>                           enum pci_size_t size);
>
>  /**
> + * pci_get_controller() - obtain the controller to use for a bus
> + *
> + * @dev:       Device to check
> + * @return pointer to the controller device for this bus
> + */
> +struct udevice *pci_get_controller(struct udevice *dev);
> +
> +/**
>   * struct dm_pci_emul_ops - PCI device emulator operations
>   */
>  struct dm_pci_emul_ops {
> --

Regards,
Bin

[U-Boot] [PATCH 7/8] dm: pci: Add a function to find the regions for a PCI bus

[Simon Glass]

This function looks up the controller and returns a pointer to each region
type.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/pci/pci-uclass.c | 30 ++++++++++++++++++++++++++++++
 include/pci.h            | 12 ++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index 527d3bb..43db390 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -963,6 +963,36 @@ ulong pci_conv_size_to_32(ulong old, ulong value, uint offset,
 	return value;
 }
 
+int pci_get_regions(struct udevice *dev, struct pci_region **iop,
+		    struct pci_region **memp, struct pci_region **prefp)
+{
+	struct udevice *bus = pci_get_controller(dev);
+	struct pci_controller *hose = dev_get_uclass_priv(bus);
+	int i;
+
+	*iop = NULL;
+	*memp = NULL;
+	*prefp = NULL;
+	for (i = 0; i < hose->region_count; i++) {
+		switch (hose->regions[i].flags) {
+		case PCI_REGION_IO:
+			if (!*iop || (*iop)->size < hose->regions[i].size)
+				*iop = hose->regions + i;
+			break;
+		case PCI_REGION_MEM:
+			if (!*memp || (*memp)->size < hose->regions[i].size)
+				*memp = hose->regions + i;
+			break;
+		case (PCI_REGION_MEM | PCI_REGION_PREFETCH):
+			if (!*prefp || (*prefp)->size < hose->regions[i].size)
+				*prefp = hose->regions + i;
+			break;
+		}
+	}
+
+	return (*iop != NULL) + (*memp != NULL) + (*prefp != NULL);
+}
+
 UCLASS_DRIVER(pci) = {
 	.id		= UCLASS_PCI,
 	.name		= "pci",
diff --git a/include/pci.h b/include/pci.h
index 8b471ab..454d90e 100644
--- a/include/pci.h
+++ b/include/pci.h
@@ -1131,6 +1131,18 @@ ulong pci_conv_size_to_32(ulong old, ulong value, uint offset,
 struct udevice *pci_get_controller(struct udevice *dev);
 
 /**
+ * pci_get_regions() - obtain pointers to all the region types
+ *
+ * @dev:	Device to check
+ * @iop:	Returns a pointer to the I/O region, or NULL if none
+ * @memp:	Returns a pointer to the memory region, or NULL if none
+ * @prefp:	Returns a pointer to the pre-fetch region, or NULL if none
+ * @return the number of non-NULL regions returned, normally 3
+ */
+int pci_get_regions(struct udevice *dev, struct pci_region **iop,
+		    struct pci_region **memp, struct pci_region **prefp);
+
+/**
  * struct dm_pci_emul_ops - PCI device emulator operations
  */
 struct dm_pci_emul_ops {
-- 
2.6.0.rc2.230.g3dd15c0

[U-Boot] [PATCH 7/8] dm: pci: Add a function to find the regions for a PCI bus

[Stephen Warren]

On 10/17/2015 11:50 AM, Simon Glass wrote:
> This function looks up the controller and returns a pointer to each region
> type.

Patches 6, 7,
Acked-by: Stephen Warren <swarren@nvidia.com>

[U-Boot] [PATCH 8/8] dm: tegra: pci: Convert tegra boards to driver model for PCI

[Simon Glass]

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.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/arm/mach-tegra/Kconfig |   1 +
 drivers/pci/pci_tegra.c     | 402 ++++++++++++++++----------------------------
 include/fdtdec.h            |   3 -
 lib/fdtdec.c                |   3 -
 4 files changed, 142 insertions(+), 267 deletions(-)

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_PCI
 
 choice
 	prompt "Tegra SoC select"
diff --git a/drivers/pci/pci_tegra.c b/drivers/pci/pci_tegra.c
index ebb959f..bc29807 100644
--- a/drivers/pci/pci_tegra.c
+++ b/drivers/pci/pci_tegra.c
@@ -10,10 +10,10 @@
  * SPDX-License-Identifier:	GPL-2.0
  */
 
-#define DEBUG
 #define pr_fmt(fmt) "tegra-pcie: " fmt
 
 #include <common.h>
+#include <dm.h>
 #include <errno.h>
 #include <fdtdec.h>
 #include <malloc.h>
@@ -174,7 +174,11 @@ DECLARE_GLOBAL_DATA_PTR;
 #define  RP_LINK_CONTROL_STATUS_DL_LINK_ACTIVE	0x20000000
 #define  RP_LINK_CONTROL_STATUS_LINKSTAT_MASK	0x3fff0000
 
-struct tegra_pcie;
+enum tegra_pci_id {
+	TEGRA20_PCIE,
+	TEGRA30_PCIE,
+	TEGRA124_PCIE,
+};
 
 struct tegra_pcie_port {
 	struct tegra_pcie *pcie;
@@ -203,10 +207,6 @@ struct tegra_pcie {
 	struct fdt_resource afi;
 	struct fdt_resource cs;
 
-	struct fdt_resource prefetch;
-	struct fdt_resource mem;
-	struct fdt_resource io;
-
 	struct list_head ports;
 	unsigned long xbar;
 
@@ -214,11 +214,6 @@ struct tegra_pcie {
 	struct tegra_xusb_phy *phy;
 };
 
-static inline struct tegra_pcie *to_tegra_pcie(struct pci_controller *hose)
-{
-	return container_of(hose, struct tegra_pcie, hose);
-}
-
 static void afi_writel(struct tegra_pcie *pcie, unsigned long value,
 		       unsigned long offset)
 {
@@ -280,46 +275,54 @@ static int tegra_pcie_conf_address(struct tegra_pcie *pcie, pci_dev_t bdf,
 		return 0;
 	}
 
-	return -1;
+	return -EFAULT;
 }
 
-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);
 
 	/* fixup root port class */
 	if (PCI_BUS(bdf) == 0) {
-		if (where == PCI_CLASS_REVISION) {
-			*value &= ~0x00ff0000;
-			*value |= PCI_CLASS_BRIDGE_PCI << 16;
+		if (offset == PCI_CLASS_REVISION) {
+			value &= ~0x00ff0000;
+			value |= PCI_CLASS_BRIDGE_PCI << 16;
 		}
 	}
 
+done:
+	*valuep = pci_conv_32_to_size(value, offset, size);
+
 	return 0;
 }
 
-static int tegra_pcie_write_conf(struct pci_controller *hose, pci_dev_t bdf,
-				 int where, u32 value)
+static int pci_tegra_write_config(struct udevice *bus, pci_dev_t bdf,
+				  uint offset, ulong value,
+				  enum pci_size_t size)
 {
-	struct tegra_pcie *pcie = to_tegra_pcie(hose);
+	struct tegra_pcie *pcie = dev_get_priv(bus);
 	unsigned long address;
+	ulong old;
 	int err;
 
-	err = tegra_pcie_conf_address(pcie, bdf, where, &address);
+	err = tegra_pcie_conf_address(pcie, bdf, offset, &address);
 	if (err < 0)
-		return 1;
+		return 0;
 
+	old = readl(address);
+	value = pci_conv_size_to_32(old, value, offset, size);
 	writel(value, address);
 
 	return 0;
@@ -344,12 +347,10 @@ static int tegra_pcie_port_parse_dt(const void *fdt, int node,
 }
 
 static int tegra_pcie_get_xbar_config(const void *fdt, int node, u32 lanes,
-				      unsigned long *xbar)
+				      enum tegra_pci_id id, unsigned long *xbar)
 {
-	enum fdt_compat_id id = fdtdec_lookup(fdt, node);
-
 	switch (id) {
-	case COMPAT_NVIDIA_TEGRA20_PCIE:
+	case TEGRA20_PCIE:
 		switch (lanes) {
 		case 0x00000004:
 			debug("single-mode configuration\n");
@@ -362,8 +363,7 @@ static int tegra_pcie_get_xbar_config(const void *fdt, int node, u32 lanes,
 			return 0;
 		}
 		break;
-
-	case COMPAT_NVIDIA_TEGRA30_PCIE:
+	case TEGRA30_PCIE:
 		switch (lanes) {
 		case 0x00000204:
 			debug("4x1, 2x1 configuration\n");
@@ -381,8 +381,7 @@ static int tegra_pcie_get_xbar_config(const void *fdt, int node, u32 lanes,
 			return 0;
 		}
 		break;
-
-	case COMPAT_NVIDIA_TEGRA124_PCIE:
+	case TEGRA124_PCIE:
 		switch (lanes) {
 		case 0x0000104:
 			debug("4x1, 1x1 configuration\n");
@@ -395,7 +394,6 @@ static int tegra_pcie_get_xbar_config(const void *fdt, int node, u32 lanes,
 			return 0;
 		}
 		break;
-
 	default:
 		break;
 	}
@@ -403,57 +401,6 @@ static int tegra_pcie_get_xbar_config(const void *fdt, int node, u32 lanes,
 	return -FDT_ERR_NOTFOUND;
 }
 
-static int tegra_pcie_parse_dt_ranges(const void *fdt, int node,
-				      struct tegra_pcie *pcie)
-{
-	const u32 *ptr, *end;
-	int len;
-
-	ptr = fdt_getprop(fdt, node, "ranges", &len);
-	if (!ptr) {
-		error("missing \"ranges\" property");
-		return -FDT_ERR_NOTFOUND;
-	}
-
-	end = ptr + len / 4;
-
-	while (ptr < end) {
-		struct fdt_resource *res = NULL;
-		u32 space = fdt32_to_cpu(*ptr);
-
-		switch ((space >> 24) & 0x3) {
-		case 0x01:
-			res = &pcie->io;
-			break;
-
-		case 0x02: /* 32 bit */
-		case 0x03: /* 64 bit */
-			if (space & (1 << 30))
-				res = &pcie->prefetch;
-			else
-				res = &pcie->mem;
-
-			break;
-		}
-
-		if (res) {
-			res->start = fdt32_to_cpu(ptr[3]);
-			res->end = res->start + fdt32_to_cpu(ptr[5]);
-		}
-
-		ptr += 3 + 1 + 2;
-	}
-
-	debug("PCI regions:\n");
-	debug("  I/O: %pa-%pa\n", &pcie->io.start, &pcie->io.end);
-	debug("  non-prefetchable memory: %pa-%pa\n", &pcie->mem.start,
-	      &pcie->mem.end);
-	debug("  prefetchable memory: %pa-%pa\n", &pcie->prefetch.start,
-	      &pcie->prefetch.end);
-
-	return 0;
-}
-
 static int tegra_pcie_parse_port_info(const void *fdt, int node,
 				      unsigned int *index,
 				      unsigned int *lanes)
@@ -480,7 +427,7 @@ static int tegra_pcie_parse_port_info(const void *fdt, int node,
 	return 0;
 }
 
-static int tegra_pcie_parse_dt(const void *fdt, int node,
+static int tegra_pcie_parse_dt(const void *fdt, int node, enum tegra_pci_id id,
 			       struct tegra_pcie *pcie)
 {
 	int err, subnode;
@@ -516,12 +463,6 @@ static int tegra_pcie_parse_dt(const void *fdt, int node,
 		}
 	}
 
-	err = tegra_pcie_parse_dt_ranges(fdt, node, pcie);
-	if (err < 0) {
-		error("failed to parse \"ranges\" property");
-		return err;
-	}
-
 	fdt_for_each_subnode(fdt, subnode, node) {
 		unsigned int index = 0, num_lanes = 0;
 		struct tegra_pcie_port *port;
@@ -556,7 +497,7 @@ static int tegra_pcie_parse_dt(const void *fdt, int node,
 		port->pcie = pcie;
 	}
 
-	err = tegra_pcie_get_xbar_config(fdt, node, lanes, &pcie->xbar);
+	err = tegra_pcie_get_xbar_config(fdt, node, lanes, id, &pcie->xbar);
 	if (err < 0) {
 		error("invalid lane configuration");
 		return err;
@@ -758,9 +699,12 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
 	return 0;
 }
 
-static void tegra_pcie_setup_translations(struct tegra_pcie *pcie)
+static int tegra_pcie_setup_translations(struct udevice *bus)
 {
+	struct tegra_pcie *pcie = dev_get_priv(bus);
 	unsigned long fpci, axi, size;
+	struct pci_region *io, *mem, *pref;
+	int count;
 
 	/* BAR 0: type 1 extended configuration space */
 	fpci = 0xfe100000;
@@ -771,28 +715,32 @@ static void tegra_pcie_setup_translations(struct tegra_pcie *pcie)
 	afi_writel(pcie, size >> 12, AFI_AXI_BAR0_SZ);
 	afi_writel(pcie, fpci, AFI_FPCI_BAR0);
 
+	count = pci_get_regions(bus, &io, &mem, &pref);
+	if (count != 3)
+		return -EINVAL;
+
 	/* BAR 1: downstream I/O */
 	fpci = 0xfdfc0000;
-	size = fdt_resource_size(&pcie->io);
-	axi = pcie->io.start;
+	size = io->size;
+	axi = io->phys_start;
 
 	afi_writel(pcie, axi, AFI_AXI_BAR1_START);
 	afi_writel(pcie, size >> 12, AFI_AXI_BAR1_SZ);
 	afi_writel(pcie, fpci, AFI_FPCI_BAR1);
 
 	/* BAR 2: prefetchable memory */
-	fpci = (((pcie->prefetch.start >> 12) & 0x0fffffff) << 4) | 0x1;
-	size = fdt_resource_size(&pcie->prefetch);
-	axi = pcie->prefetch.start;
+	fpci = (((pref->phys_start >> 12) & 0x0fffffff) << 4) | 0x1;
+	size = pref->size;
+	axi = pref->phys_start;
 
 	afi_writel(pcie, axi, AFI_AXI_BAR2_START);
 	afi_writel(pcie, size >> 12, AFI_AXI_BAR2_SZ);
 	afi_writel(pcie, fpci, AFI_FPCI_BAR2);
 
 	/* BAR 3: non-prefetchable memory */
-	fpci = (((pcie->mem.start >> 12) & 0x0fffffff) << 4) | 0x1;
-	size = fdt_resource_size(&pcie->mem);
-	axi = pcie->mem.start;
+	fpci = (((mem->phys_start >> 12) & 0x0fffffff) << 4) | 0x1;
+	size = mem->size;
+	axi = mem->phys_start;
 
 	afi_writel(pcie, axi, AFI_AXI_BAR3_START);
 	afi_writel(pcie, size >> 12, AFI_AXI_BAR3_SZ);
@@ -818,6 +766,8 @@ static void tegra_pcie_setup_translations(struct tegra_pcie *pcie)
 	afi_writel(pcie, 0, AFI_MSI_BAR_SZ);
 	afi_writel(pcie, 0, AFI_MSI_AXI_BAR_ST);
 	afi_writel(pcie, 0, AFI_MSI_BAR_SZ);
+
+	return 0;
 }
 
 static unsigned long tegra_pcie_port_get_pex_ctrl(struct tegra_pcie_port *port)
@@ -964,180 +914,110 @@ static int tegra_pcie_enable(struct tegra_pcie *pcie)
 	return 0;
 }
 
-static const struct tegra_pcie_soc tegra20_pcie_soc = {
-	.num_ports = 2,
-	.pads_pll_ctl = PADS_PLL_CTL_TEGRA20,
-	.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_DIV10,
-	.has_pex_clkreq_en = false,
-	.has_pex_bias_ctrl = false,
-	.has_cml_clk = false,
-	.has_gen2 = false,
-};
-
-static const struct tegra_pcie_soc tegra30_pcie_soc = {
-	.num_ports = 3,
-	.pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
-	.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
-	.has_pex_clkreq_en = true,
-	.has_pex_bias_ctrl = true,
-	.has_cml_clk = true,
-	.has_gen2 = false,
-};
-
-static const struct tegra_pcie_soc tegra124_pcie_soc = {
-	.num_ports = 2,
-	.pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
-	.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
-	.has_pex_clkreq_en = true,
-	.has_pex_bias_ctrl = true,
-	.has_cml_clk = true,
-	.has_gen2 = true,
+static const struct tegra_pcie_soc pci_tegra_soc[] = {
+	[TEGRA20_PCIE] = {
+		.num_ports = 2,
+		.pads_pll_ctl = PADS_PLL_CTL_TEGRA20,
+		.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_DIV10,
+		.has_pex_clkreq_en = false,
+		.has_pex_bias_ctrl = false,
+		.has_cml_clk = false,
+		.has_gen2 = false,
+	},
+	[TEGRA30_PCIE] = {
+		.num_ports = 3,
+		.pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
+		.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
+		.has_pex_clkreq_en = true,
+		.has_pex_bias_ctrl = true,
+		.has_cml_clk = true,
+		.has_gen2 = false,
+	},
+	[TEGRA124_PCIE] = {
+		.num_ports = 2,
+		.pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
+		.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
+		.has_pex_clkreq_en = true,
+		.has_pex_bias_ctrl = true,
+		.has_cml_clk = true,
+		.has_gen2 = true,
+	}
 };
 
-static int process_nodes(const void *fdt, int nodes[], unsigned int count)
+static int pci_tegra_ofdata_to_platdata(struct udevice *dev)
 {
-	unsigned int i;
-
-	for (i = 0; i < count; i++) {
-		const struct tegra_pcie_soc *soc;
-		struct tegra_pcie *pcie;
-		enum fdt_compat_id id;
-		int err;
-
-		if (!fdtdec_get_is_enabled(fdt, nodes[i]))
-			continue;
-
-		id = fdtdec_lookup(fdt, nodes[i]);
-		switch (id) {
-		case COMPAT_NVIDIA_TEGRA20_PCIE:
-			soc = &tegra20_pcie_soc;
-			break;
-
-		case COMPAT_NVIDIA_TEGRA30_PCIE:
-			soc = &tegra30_pcie_soc;
-			break;
-
-		case COMPAT_NVIDIA_TEGRA124_PCIE:
-			soc = &tegra124_pcie_soc;
-			break;
-
-		default:
-			error("unsupported compatible: %s",
-			      fdtdec_get_compatible(id));
-			continue;
-		}
-
-		pcie = malloc(sizeof(*pcie));
-		if (!pcie) {
-			error("failed to allocate controller");
-			continue;
-		}
+	struct tegra_pcie *pcie = dev_get_priv(dev);
+	enum tegra_pci_id id;
 
-		memset(pcie, 0, sizeof(*pcie));
-		pcie->soc = soc;
+	id = dev_get_driver_data(dev);
+	pcie->soc = &pci_tegra_soc[id];
 
-		INIT_LIST_HEAD(&pcie->ports);
-
-		err = tegra_pcie_parse_dt(fdt, nodes[i], pcie);
-		if (err < 0) {
-			free(pcie);
-			continue;
-		}
+	INIT_LIST_HEAD(&pcie->ports);
 
-		err = tegra_pcie_power_on(pcie);
-		if (err < 0) {
-			error("failed to power on");
-			continue;
-		}
+	if (tegra_pcie_parse_dt(gd->fdt_blob, dev->of_offset, id, pcie))
+		return -EINVAL;
 
-		err = tegra_pcie_enable_controller(pcie);
-		if (err < 0) {
-			error("failed to enable controller");
-			continue;
-		}
-
-		tegra_pcie_setup_translations(pcie);
-
-		err = tegra_pcie_enable(pcie);
-		if (err < 0) {
-			error("failed to enable PCIe");
-			continue;
-		}
-
-		pcie->hose.first_busno = 0;
-		pcie->hose.current_busno = 0;
-		pcie->hose.last_busno = 0;
-
-		pci_set_region(&pcie->hose.regions[0], NV_PA_SDRAM_BASE,
-			       NV_PA_SDRAM_BASE, gd->ram_size,
-			       PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
-
-		pci_set_region(&pcie->hose.regions[1], pcie->io.start,
-			       pcie->io.start, fdt_resource_size(&pcie->io),
-			       PCI_REGION_IO);
+	return 0;
+}
 
-		pci_set_region(&pcie->hose.regions[2], pcie->mem.start,
-			       pcie->mem.start, fdt_resource_size(&pcie->mem),
-			       PCI_REGION_MEM);
+int pci_skip_dev(struct pci_controller *hose, pci_dev_t dev)
+{
+	if (PCI_BUS(dev) != 0 && PCI_DEV(dev) > 0)
+		return 1;
 
-		pci_set_region(&pcie->hose.regions[3], pcie->prefetch.start,
-			       pcie->prefetch.start,
-			       fdt_resource_size(&pcie->prefetch),
-			       PCI_REGION_MEM | PCI_REGION_PREFETCH);
+	return 0;
+}
 
-		pcie->hose.region_count = 4;
+static int pci_tegra_probe(struct udevice *dev)
+{
+	struct tegra_pcie *pcie = dev_get_priv(dev);
+	int err;
 
-		pci_set_ops(&pcie->hose,
-			    pci_hose_read_config_byte_via_dword,
-			    pci_hose_read_config_word_via_dword,
-			    tegra_pcie_read_conf,
-			    pci_hose_write_config_byte_via_dword,
-			    pci_hose_write_config_word_via_dword,
-			    tegra_pcie_write_conf);
+	err = tegra_pcie_power_on(pcie);
+	if (err < 0) {
+		error("failed to power on");
+		return err;
+	}
 
-		pci_register_hose(&pcie->hose);
+	err = tegra_pcie_enable_controller(pcie);
+	if (err < 0) {
+		error("failed to enable controller");
+		return err;
+	}
 
-#ifdef CONFIG_PCI_SCAN_SHOW
-		printf("PCI: Enumerating devices...\n");
-		printf("---------------------------------------\n");
-		printf("  Device        ID          Description\n");
-		printf("  ------        --          -----------\n");
-#endif
+	err = tegra_pcie_setup_translations(dev);
+	if (err < 0) {
+		error("failed to decode ranges");
+		return err;
+	}
 
-		pcie->hose.last_busno = pci_hose_scan(&pcie->hose);
+	err = tegra_pcie_enable(pcie);
+	if (err < 0) {
+		error("failed to enable PCIe");
+		return err;
 	}
 
 	return 0;
 }
 
-void pci_init_board(void)
-{
-	const void *fdt = gd->fdt_blob;
-	int count, nodes[1];
-
-	count = fdtdec_find_aliases_for_id(fdt, "pcie-controller",
-					   COMPAT_NVIDIA_TEGRA124_PCIE,
-					   nodes, ARRAY_SIZE(nodes));
-	if (process_nodes(fdt, nodes, count))
-		return;
-
-	count = fdtdec_find_aliases_for_id(fdt, "pcie-controller",
-					   COMPAT_NVIDIA_TEGRA30_PCIE,
-					   nodes, ARRAY_SIZE(nodes));
-	if (process_nodes(fdt, nodes, count))
-		return;
-
-	count = fdtdec_find_aliases_for_id(fdt, "pcie-controller",
-					   COMPAT_NVIDIA_TEGRA20_PCIE,
-					   nodes, ARRAY_SIZE(nodes));
-	if (process_nodes(fdt, nodes, count))
-		return;
-}
+static const struct dm_pci_ops pci_tegra_ops = {
+	.read_config	= pci_tegra_read_config,
+	.write_config	= pci_tegra_write_config,
+};
 
-int pci_skip_dev(struct pci_controller *hose, pci_dev_t dev)
-{
-	if (PCI_BUS(dev) != 0 && PCI_DEV(dev) > 0)
-		return 1;
+static const struct udevice_id pci_tegra_ids[] = {
+	{ .compatible = "nvidia,tegra20-pcie", .data = TEGRA20_PCIE },
+	{ .compatible = "nvidia,tegra30-pcie", .data = TEGRA30_PCIE },
+	{ .compatible = "nvidia,tegra124-pcie", .data = TEGRA124_PCIE },
+	{ }
+};
 
-	return 0;
-}
+U_BOOT_DRIVER(pci_tegra) = {
+	.name	= "pci_tegra",
+	.id	= UCLASS_PCI,
+	.of_match = pci_tegra_ids,
+	.ops	= &pci_tegra_ops,
+	.ofdata_to_platdata = pci_tegra_ofdata_to_platdata,
+	.probe	= pci_tegra_probe,
+	.priv_auto_alloc_size = sizeof(struct tegra_pcie),
+};
diff --git a/include/fdtdec.h b/include/fdtdec.h
index 2de6dda..00d8907 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -129,9 +129,6 @@ enum fdt_compat_id {
 	COMPAT_NVIDIA_TEGRA124_SDMMC,	/* Tegra124 SDMMC controller */
 	COMPAT_NVIDIA_TEGRA30_SDMMC,	/* Tegra30 SDMMC controller */
 	COMPAT_NVIDIA_TEGRA20_SDMMC,	/* Tegra20 SDMMC controller */
-	COMPAT_NVIDIA_TEGRA124_PCIE,	/* Tegra 124 PCIe controller */
-	COMPAT_NVIDIA_TEGRA30_PCIE,	/* Tegra 30 PCIe controller */
-	COMPAT_NVIDIA_TEGRA20_PCIE,	/* Tegra 20 PCIe controller */
 	COMPAT_NVIDIA_TEGRA124_XUSB_PADCTL,
 					/* Tegra124 XUSB pad controller */
 	COMPAT_NVIDIA_TEGRA210_XUSB_PADCTL,
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 1a86369..295e695 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -35,9 +35,6 @@ static const char * const compat_names[COMPAT_COUNT] = {
 	COMPAT(NVIDIA_TEGRA124_SDMMC, "nvidia,tegra124-sdhci"),
 	COMPAT(NVIDIA_TEGRA30_SDMMC, "nvidia,tegra30-sdhci"),
 	COMPAT(NVIDIA_TEGRA20_SDMMC, "nvidia,tegra20-sdhci"),
-	COMPAT(NVIDIA_TEGRA124_PCIE, "nvidia,tegra124-pcie"),
-	COMPAT(NVIDIA_TEGRA30_PCIE, "nvidia,tegra30-pcie"),
-	COMPAT(NVIDIA_TEGRA20_PCIE, "nvidia,tegra20-pcie"),
 	COMPAT(NVIDIA_TEGRA124_XUSB_PADCTL, "nvidia,tegra124-xusb-padctl"),
 	COMPAT(NVIDIA_TEGRA210_XUSB_PADCTL, "nvidia,tegra210-xusb-padctl"),
 	COMPAT(SMSC_LAN9215, "smsc,lan9215"),
-- 
2.6.0.rc2.230.g3dd15c0

[U-Boot] [PATCH 8/8] dm: tegra: pci: Convert tegra boards to driver model for PCI

[Stephen Warren]

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.

> 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_PCI

I 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.

> 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?

> +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?

[U-Boot] [PATCH 8/8] dm: tegra: pci: Convert tegra boards to driver model for PCI

[Simon Glass]

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_PCI
>
>
> I 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