[PATCH 10/13] ARM: dts: synquacer: Add device trees for DeveloperBox

[PATCH 10/13] ARM: dts: synquacer: Add device trees for DeveloperBox

[Tom Rini]

On Wed, Apr 14, 2021 at 12:30:15AM +0900, Masami Hiramatsu wrote:

> Add device trees for 96boards EE DeveloperBox and basement SynQuacer
> SoC dtsi. These files are imported from EDK2 with cosmetic change
> and modified for accessing SPI NOR flash.
> 
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> ---
>  arch/arm/dts/DeveloperBox.dts     |  146 +++++++++
>  arch/arm/dts/Makefile             |    2 
>  arch/arm/dts/SynQuacer.dtsi       |  590 +++++++++++++++++++++++++++++++++++++
>  arch/arm/dts/SynQuacerCaches.dtsi |   72 +++++

This poses a bit of a naming challenge.  I assume, but please correct me
if I'm wrong, that you don't intend to push these dts files to the Linux
kernel.  So that means EDK2 is the primary source of the files, yes?  We
want to keep them as-is from the upstream project, and note that
relevant git hash/tag/etc that matches where we pulled from, to make
future syncs easier, in the commit message.

I assume this is not part of the uniphier family, so we should start by
naming these as arch/arm/dts/synquacer-... to fit with the general
scheme.  Perhaps -developerbox, -core and -caches ?  And for any changes
we do need, they go in a -u-boot.dtsi file and then potentially
automatically included (if it fits in the logic we have today for that)
or specifically #included otherwise.

Thanks.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210413/5146df24/attachment.sig>

[PATCH 10/13] ARM: dts: synquacer: Add device trees for DeveloperBox

[Masami Hiramatsu]

Hello Tom,

Thank you for your comment!

2021?4?14?(?) 2:47 Tom Rini <trini@konsulko.com>:
>
> On Wed, Apr 14, 2021 at 12:30:15AM +0900, Masami Hiramatsu wrote:
>
> > Add device trees for 96boards EE DeveloperBox and basement SynQuacer
> > SoC dtsi. These files are imported from EDK2 with cosmetic change
> > and modified for accessing SPI NOR flash.
> >
> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > ---
> >  arch/arm/dts/DeveloperBox.dts     |  146 +++++++++
> >  arch/arm/dts/Makefile             |    2
> >  arch/arm/dts/SynQuacer.dtsi       |  590 +++++++++++++++++++++++++++++++++++++
> >  arch/arm/dts/SynQuacerCaches.dtsi |   72 +++++
>
> This poses a bit of a naming challenge.  I assume, but please correct me
> if I'm wrong, that you don't intend to push these dts files to the Linux
> kernel.  So that means EDK2 is the primary source of the files, yes?

Yes, those are originally written for the EDK2 and I ported it.

> We
> want to keep them as-is from the upstream project, and note that
> relevant git hash/tag/etc that matches where we pulled from, to make
> future syncs easier, in the commit message.

Let me confirm what you mean, is the git hash/tag/etc what the commit
I copied from? and note it in commit message or in the devicetree file?

BTW, I made some changes on it for U-Boot drivers.
- Enabled OP-TEE node by default (EDK2 checks OP-TEE existance
  and enables it)
- Add SPI node information for accessing SPI-NOR from U-Boot (EDK2
 embedded such information inside)
Thus the DeveloperBox.dts may not be kept as-is.

> I assume this is not part of the uniphier family, so we should start by
> naming these as arch/arm/dts/synquacer-... to fit with the general
> scheme.  Perhaps -developerbox, -core and -caches ?

OK. BTW, I'm not sure what is the best way to use SYS_CPU and SYS_SOC.
This SoC family name is SynQuacer and the SoC itself is SC2A11.
In this case,
CONFIG_SYS_CPU=synquacer
CONFIG_SYS_SOC=sc2a11
or
CONFIG_SYS_SOC=synquacer-sc2a11
?

> And for any changes
> we do need, they go in a -u-boot.dtsi file and then potentially
> automatically included (if it fits in the logic we have today for that)
> or specifically #included otherwise.

It seems that (CONFIG_SYS_SOC)-u-boot.dtsi is automatically included,
am I correct?

Thank you,

--
Masami Hiramatsu

[PATCH 10/13] ARM: dts: synquacer: Add device trees for DeveloperBox

[Tom Rini]

On Wed, Apr 14, 2021 at 10:06:04AM +0900, Masami Hiramatsu wrote:
> Hello Tom,
> 
> Thank you for your comment!
> 
> 2021?4?14?(?) 2:47 Tom Rini <trini@konsulko.com>:
> >
> > On Wed, Apr 14, 2021 at 12:30:15AM +0900, Masami Hiramatsu wrote:
> >
> > > Add device trees for 96boards EE DeveloperBox and basement SynQuacer
> > > SoC dtsi. These files are imported from EDK2 with cosmetic change
> > > and modified for accessing SPI NOR flash.
> > >
> > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > > ---
> > >  arch/arm/dts/DeveloperBox.dts     |  146 +++++++++
> > >  arch/arm/dts/Makefile             |    2
> > >  arch/arm/dts/SynQuacer.dtsi       |  590 +++++++++++++++++++++++++++++++++++++
> > >  arch/arm/dts/SynQuacerCaches.dtsi |   72 +++++
> >
> > This poses a bit of a naming challenge.  I assume, but please correct me
> > if I'm wrong, that you don't intend to push these dts files to the Linux
> > kernel.  So that means EDK2 is the primary source of the files, yes?
> 
> Yes, those are originally written for the EDK2 and I ported it.

OK.

> > We
> > want to keep them as-is from the upstream project, and note that
> > relevant git hash/tag/etc that matches where we pulled from, to make
> > future syncs easier, in the commit message.
> 
> Let me confirm what you mean, is the git hash/tag/etc what the commit
> I copied from? and note it in commit message or in the devicetree file?

Correct.

> BTW, I made some changes on it for U-Boot drivers.
> - Enabled OP-TEE node by default (EDK2 checks OP-TEE existance
>   and enables it)
> - Add SPI node information for accessing SPI-NOR from U-Boot (EDK2
>  embedded such information inside)
> Thus the DeveloperBox.dts may not be kept as-is.

These kinds of changes can be done in something such as
DeveloperBox-u-boot.dtsi.  Check out the logic in scripts/Makefile.lib
around automatic inclusion of a "-u-boot.dtsi" file.

> > I assume this is not part of the uniphier family, so we should start by
> > naming these as arch/arm/dts/synquacer-... to fit with the general
> > scheme.  Perhaps -developerbox, -core and -caches ?
> 
> OK. BTW, I'm not sure what is the best way to use SYS_CPU and SYS_SOC.
> This SoC family name is SynQuacer and the SoC itself is SC2A11.
> In this case,
> CONFIG_SYS_CPU=synquacer
> CONFIG_SYS_SOC=sc2a11
> or
> CONFIG_SYS_SOC=synquacer-sc2a11

I think the first example you list is likely the best.

> > And for any changes
> > we do need, they go in a -u-boot.dtsi file and then potentially
> > automatically included (if it fits in the logic we have today for that)
> > or specifically #included otherwise.
> 
> It seems that (CONFIG_SYS_SOC)-u-boot.dtsi is automatically included,
> am I correct?

For the full list and how it works, see scripts/Makefile.lib

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210414/7e7b8e1e/attachment.sig>

[PATCH 10/13] ARM: dts: synquacer: Add device trees for DeveloperBox

[Masami Hiramatsu]

Hi Tom,

2021?4?15?(?) 2:33 Tom Rini <trini@konsulko.com>:
> > BTW, I made some changes on it for U-Boot drivers.
> > - Enabled OP-TEE node by default (EDK2 checks OP-TEE existance
> >   and enables it)
> > - Add SPI node information for accessing SPI-NOR from U-Boot (EDK2
> >  embedded such information inside)
> > Thus the DeveloperBox.dts may not be kept as-is.
>
> These kinds of changes can be done in something such as
> DeveloperBox-u-boot.dtsi.  Check out the logic in scripts/Makefile.lib
> around automatic inclusion of a "-u-boot.dtsi" file.

OK, I confirmed adding "CONFIG_DEFAULT_DEVICE_TREE-u-boot.dtsi" works.

> > > I assume this is not part of the uniphier family, so we should start by
> > > naming these as arch/arm/dts/synquacer-... to fit with the general
> > > scheme.  Perhaps -developerbox, -core and -caches ?
> >
> > OK. BTW, I'm not sure what is the best way to use SYS_CPU and SYS_SOC.
> > This SoC family name is SynQuacer and the SoC itself is SC2A11.
> > In this case,
> > CONFIG_SYS_CPU=synquacer
> > CONFIG_SYS_SOC=sc2a11
> > or
> > CONFIG_SYS_SOC=synquacer-sc2a11
>
> I think the first example you list is likely the best.

I found that SYS_CPU is already set to "armv8" and it is used in the
build process. So I will not change it.

Thank you,


-- 
Masami Hiramatsu

[PATCH 11/13] board: synquacer: Add DeveloperBox 96boards EE support

[Tom Rini]

On Wed, Apr 14, 2021 at 12:31:21AM +0900, Masami Hiramatsu wrote:

> Add the DeveloperBox 96boards EE support. This board is also
> known as Socionext SynQuacer E-Series. It contians one "SC2A11"
> SoC, which has 24-cores of arm Cortex-A53, and 4 DDR3 slots,
> 3 PCIe slots (1 4x port and 2 1x ports which are expanded via
> PCIe bridge chip), 2 USB 3.0 ports and 2 USB 2.0 ports, 2 SATA
> ports and 1 GbE, 64MB NOR flash and 8GB eMMC on standard
> MicroATX Form Factor.
> 
> For more information, see this page;
>   https://www.96boards.org/product/developerbox/
> 
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
[snip]
> diff --git a/arch/arm/include/asm/arch-sc2a11/gpio.h b/arch/arm/include/asm/arch-sc2a11/gpio.h
> new file mode 100644
> index 0000000000..6779803080
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-sc2a11/gpio.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright 2021 (C) Linaro Ltd.
> + */
> +
> +#ifndef __ASM_ARCH_SC2A11_GPIO_H
> +#define __ASM_ARCH_SC2A11_GPIO_H
> +
> +#endif

Please update the list in arch/arm/include/asm/gpio.h to not look for
asm/arch/gpio.h on this SoC, thanks.

> diff --git a/board/socionext/developerbox/README b/board/socionext/developerbox/README
> new file mode 100644
> index 0000000000..bb121002dd
> --- /dev/null
> +++ b/board/socionext/developerbox/README

This needs to be in rST form and under doc/board/ now.

[snip]
> +/*
> + * arguments for booti command
> + */
> +#define LINUX_BASIC_BOOTARGS	"basic_bootargs='"					\
> +				"console=ttyAMA0," __stringify(CONFIG_BAUDRATE) " "	\
> +				"rootwait verbose'\0"
> +
> +/* kernel:mmcblk0p1(ext2), rootfs:mmcblk0p1(ext2), devtree:mmcblk0p1(ext2) */
> +#define	LINUX_MMCBOOTCOMMAND1							\
> +	"mmcboot1=echo 'Boot from MMC (ext2 single rootfs)' ; "			\
> +		"mmc dev 0 &&"							\
> +		"ext2load mmc 0:1 ${kernel_addr} /boot/Image && "		\
> +		"ext2load mmc 0:1 ${fdt_addr_base} /boot/DeveloperBox.dtb && "	\
> +		"setenv fdt_addr ${fdt_addr_base} &&"				\
> +		"setenv fdt_size ${filesize} &&"				\
> +		"setenv bootargs ${mmc_bootargs1} && "				\
> +		"booti ${kernel_addr} - ${fdt_addr}\0"				\
> +	"mmc_bootargs1="							\
> +		"root=/dev/mmcblk0p1 rw dtb=0x${fdt_addr},0x${fdt_size} "	\
> +		"${basic_bootargs} \0"

You are strongly encouraged to use the generic distro boot features
instead.  This may require a little work to handle the device tree.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210413/e82d5125/attachment.sig>

[PATCH 11/13] board: synquacer: Add DeveloperBox 96boards EE support

[Masami Hiramatsu]

Hello Tom,

2021?4?14?(?) 2:47 Tom Rini <trini@konsulko.com>:
>
> On Wed, Apr 14, 2021 at 12:31:21AM +0900, Masami Hiramatsu wrote:
>
> > Add the DeveloperBox 96boards EE support. This board is also
> > known as Socionext SynQuacer E-Series. It contians one "SC2A11"
> > SoC, which has 24-cores of arm Cortex-A53, and 4 DDR3 slots,
> > 3 PCIe slots (1 4x port and 2 1x ports which are expanded via
> > PCIe bridge chip), 2 USB 3.0 ports and 2 USB 2.0 ports, 2 SATA
> > ports and 1 GbE, 64MB NOR flash and 8GB eMMC on standard
> > MicroATX Form Factor.
> >
> > For more information, see this page;
> >   https://www.96boards.org/product/developerbox/
> >
> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> [snip]
> > diff --git a/arch/arm/include/asm/arch-sc2a11/gpio.h b/arch/arm/include/asm/arch-sc2a11/gpio.h
> > new file mode 100644
> > index 0000000000..6779803080
> > --- /dev/null
> > +++ b/arch/arm/include/asm/arch-sc2a11/gpio.h
> > @@ -0,0 +1,9 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Copyright 2021 (C) Linaro Ltd.
> > + */
> > +
> > +#ifndef __ASM_ARCH_SC2A11_GPIO_H
> > +#define __ASM_ARCH_SC2A11_GPIO_H
> > +
> > +#endif
>
> Please update the list in arch/arm/include/asm/gpio.h to not look for
> asm/arch/gpio.h on this SoC, thanks.

Ah, I missed that. OK, I'll change arch/arm/include/asm/gpio.h.

BTW, isn't it better to introduce CONFIG_ARCH_GENERIC_GPIO
instead of updating the header?

> > diff --git a/board/socionext/developerbox/README b/board/socionext/developerbox/README
> > new file mode 100644
> > index 0000000000..bb121002dd
> > --- /dev/null
> > +++ b/board/socionext/developerbox/README
>
> This needs to be in rST form and under doc/board/ now.
>
> [snip]
> > +/*
> > + * arguments for booti command
> > + */
> > +#define LINUX_BASIC_BOOTARGS "basic_bootargs='"                                      \
> > +                             "console=ttyAMA0," __stringify(CONFIG_BAUDRATE) " "     \
> > +                             "rootwait verbose'\0"
> > +
> > +/* kernel:mmcblk0p1(ext2), rootfs:mmcblk0p1(ext2), devtree:mmcblk0p1(ext2) */
> > +#define      LINUX_MMCBOOTCOMMAND1                                                   \
> > +     "mmcboot1=echo 'Boot from MMC (ext2 single rootfs)' ; "                 \
> > +             "mmc dev 0 &&"                                                  \
> > +             "ext2load mmc 0:1 ${kernel_addr} /boot/Image && "               \
> > +             "ext2load mmc 0:1 ${fdt_addr_base} /boot/DeveloperBox.dtb && "  \
> > +             "setenv fdt_addr ${fdt_addr_base} &&"                           \
> > +             "setenv fdt_size ${filesize} &&"                                \
> > +             "setenv bootargs ${mmc_bootargs1} && "                          \
> > +             "booti ${kernel_addr} - ${fdt_addr}\0"                          \
> > +     "mmc_bootargs1="                                                        \
> > +             "root=/dev/mmcblk0p1 rw dtb=0x${fdt_addr},0x${fdt_size} "       \
> > +             "${basic_bootargs} \0"
>
> You are strongly encouraged to use the generic distro boot features
> instead.  This may require a little work to handle the device tree.

OK, anyway I can just drop it, because this platform is expected to use
UEFI boot (the DeveloperBox is shipped with EDK2).
I left this just for reference.

Thank you,

-- 
Masami Hiramatsu

[PATCH 11/13] board: synquacer: Add DeveloperBox 96boards EE support

[Takahiro Akashi]

On Wed, Apr 14, 2021 at 10:12:42AM +0900, Masami Hiramatsu wrote:
> Hello Tom,
> 
> 2021?4?14?(?) 2:47 Tom Rini <trini@konsulko.com>:
> >
> > On Wed, Apr 14, 2021 at 12:31:21AM +0900, Masami Hiramatsu wrote:
> >
> > > Add the DeveloperBox 96boards EE support. This board is also
> > > known as Socionext SynQuacer E-Series. It contians one "SC2A11"
> > > SoC, which has 24-cores of arm Cortex-A53, and 4 DDR3 slots,
> > > 3 PCIe slots (1 4x port and 2 1x ports which are expanded via
> > > PCIe bridge chip), 2 USB 3.0 ports and 2 USB 2.0 ports, 2 SATA
> > > ports and 1 GbE, 64MB NOR flash and 8GB eMMC on standard
> > > MicroATX Form Factor.
> > >
> > > For more information, see this page;
> > >   https://www.96boards.org/product/developerbox/
> > >
> > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > [snip]
> > > diff --git a/arch/arm/include/asm/arch-sc2a11/gpio.h b/arch/arm/include/asm/arch-sc2a11/gpio.h
> > > new file mode 100644
> > > index 0000000000..6779803080
> > > --- /dev/null
> > > +++ b/arch/arm/include/asm/arch-sc2a11/gpio.h
> > > @@ -0,0 +1,9 @@
> > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > +/*
> > > + * Copyright 2021 (C) Linaro Ltd.
> > > + */
> > > +
> > > +#ifndef __ASM_ARCH_SC2A11_GPIO_H
> > > +#define __ASM_ARCH_SC2A11_GPIO_H
> > > +
> > > +#endif
> >
> > Please update the list in arch/arm/include/asm/gpio.h to not look for
> > asm/arch/gpio.h on this SoC, thanks.
> 
> Ah, I missed that. OK, I'll change arch/arm/include/asm/gpio.h.
> 
> BTW, isn't it better to introduce CONFIG_ARCH_GENERIC_GPIO
> instead of updating the header?
> 
> > > diff --git a/board/socionext/developerbox/README b/board/socionext/developerbox/README
> > > new file mode 100644
> > > index 0000000000..bb121002dd
> > > --- /dev/null
> > > +++ b/board/socionext/developerbox/README
> >
> > This needs to be in rST form and under doc/board/ now.
> >
> > [snip]
> > > +/*
> > > + * arguments for booti command
> > > + */
> > > +#define LINUX_BASIC_BOOTARGS "basic_bootargs='"                                      \
> > > +                             "console=ttyAMA0," __stringify(CONFIG_BAUDRATE) " "     \
> > > +                             "rootwait verbose'\0"
> > > +
> > > +/* kernel:mmcblk0p1(ext2), rootfs:mmcblk0p1(ext2), devtree:mmcblk0p1(ext2) */
> > > +#define      LINUX_MMCBOOTCOMMAND1                                                   \
> > > +     "mmcboot1=echo 'Boot from MMC (ext2 single rootfs)' ; "                 \
> > > +             "mmc dev 0 &&"                                                  \
> > > +             "ext2load mmc 0:1 ${kernel_addr} /boot/Image && "               \
> > > +             "ext2load mmc 0:1 ${fdt_addr_base} /boot/DeveloperBox.dtb && "  \
> > > +             "setenv fdt_addr ${fdt_addr_base} &&"                           \
> > > +             "setenv fdt_size ${filesize} &&"                                \
> > > +             "setenv bootargs ${mmc_bootargs1} && "                          \
> > > +             "booti ${kernel_addr} - ${fdt_addr}\0"                          \
> > > +     "mmc_bootargs1="                                                        \
> > > +             "root=/dev/mmcblk0p1 rw dtb=0x${fdt_addr},0x${fdt_size} "       \
> > > +             "${basic_bootargs} \0"
> >
> > You are strongly encouraged to use the generic distro boot features
> > instead.  This may require a little work to handle the device tree.
> 
> OK, anyway I can just drop it, because this platform is expected to use
> UEFI boot (the DeveloperBox is shipped with EDK2).
> I left this just for reference.

So why not define UEFI load options (BOOTxxxx) and use UEFI boot manager
("bootefi bootmgr")?
That is the way how UEFI (at least boot manager) boots the kernel.

-Takahiro Akashi

> Thank you,
> 
> -- 
> Masami Hiramatsu

[PATCH 11/13] board: synquacer: Add DeveloperBox 96boards EE support

[Masami Hiramatsu]

Hi Takahiro,

2021?4?14?(?) 10:27 Takahiro Akashi <takahiro.akashi@linaro.org>:
>
> On Wed, Apr 14, 2021 at 10:12:42AM +0900, Masami Hiramatsu wrote:
> > Hello Tom,
> >
> > 2021?4?14?(?) 2:47 Tom Rini <trini@konsulko.com>:
> > >
> > > On Wed, Apr 14, 2021 at 12:31:21AM +0900, Masami Hiramatsu wrote:
> > >
> > > > Add the DeveloperBox 96boards EE support. This board is also
> > > > known as Socionext SynQuacer E-Series. It contians one "SC2A11"
> > > > SoC, which has 24-cores of arm Cortex-A53, and 4 DDR3 slots,
> > > > 3 PCIe slots (1 4x port and 2 1x ports which are expanded via
> > > > PCIe bridge chip), 2 USB 3.0 ports and 2 USB 2.0 ports, 2 SATA
> > > > ports and 1 GbE, 64MB NOR flash and 8GB eMMC on standard
> > > > MicroATX Form Factor.
> > > >
> > > > For more information, see this page;
> > > >   https://www.96boards.org/product/developerbox/
> > > >
> > > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > > [snip]
> > > > diff --git a/arch/arm/include/asm/arch-sc2a11/gpio.h b/arch/arm/include/asm/arch-sc2a11/gpio.h
> > > > new file mode 100644
> > > > index 0000000000..6779803080
> > > > --- /dev/null
> > > > +++ b/arch/arm/include/asm/arch-sc2a11/gpio.h
> > > > @@ -0,0 +1,9 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > +/*
> > > > + * Copyright 2021 (C) Linaro Ltd.
> > > > + */
> > > > +
> > > > +#ifndef __ASM_ARCH_SC2A11_GPIO_H
> > > > +#define __ASM_ARCH_SC2A11_GPIO_H
> > > > +
> > > > +#endif
> > >
> > > Please update the list in arch/arm/include/asm/gpio.h to not look for
> > > asm/arch/gpio.h on this SoC, thanks.
> >
> > Ah, I missed that. OK, I'll change arch/arm/include/asm/gpio.h.
> >
> > BTW, isn't it better to introduce CONFIG_ARCH_GENERIC_GPIO
> > instead of updating the header?
> >
> > > > diff --git a/board/socionext/developerbox/README b/board/socionext/developerbox/README
> > > > new file mode 100644
> > > > index 0000000000..bb121002dd
> > > > --- /dev/null
> > > > +++ b/board/socionext/developerbox/README
> > >
> > > This needs to be in rST form and under doc/board/ now.
> > >
> > > [snip]
> > > > +/*
> > > > + * arguments for booti command
> > > > + */
> > > > +#define LINUX_BASIC_BOOTARGS "basic_bootargs='"                                      \
> > > > +                             "console=ttyAMA0," __stringify(CONFIG_BAUDRATE) " "     \
> > > > +                             "rootwait verbose'\0"
> > > > +
> > > > +/* kernel:mmcblk0p1(ext2), rootfs:mmcblk0p1(ext2), devtree:mmcblk0p1(ext2) */
> > > > +#define      LINUX_MMCBOOTCOMMAND1                                                   \
> > > > +     "mmcboot1=echo 'Boot from MMC (ext2 single rootfs)' ; "                 \
> > > > +             "mmc dev 0 &&"                                                  \
> > > > +             "ext2load mmc 0:1 ${kernel_addr} /boot/Image && "               \
> > > > +             "ext2load mmc 0:1 ${fdt_addr_base} /boot/DeveloperBox.dtb && "  \
> > > > +             "setenv fdt_addr ${fdt_addr_base} &&"                           \
> > > > +             "setenv fdt_size ${filesize} &&"                                \
> > > > +             "setenv bootargs ${mmc_bootargs1} && "                          \
> > > > +             "booti ${kernel_addr} - ${fdt_addr}\0"                          \
> > > > +     "mmc_bootargs1="                                                        \
> > > > +             "root=/dev/mmcblk0p1 rw dtb=0x${fdt_addr},0x${fdt_size} "       \
> > > > +             "${basic_bootargs} \0"
> > >
> > > You are strongly encouraged to use the generic distro boot features
> > > instead.  This may require a little work to handle the device tree.
> >
> > OK, anyway I can just drop it, because this platform is expected to use
> > UEFI boot (the DeveloperBox is shipped with EDK2).
> > I left this just for reference.
>
> So why not define UEFI load options (BOOTxxxx) and use UEFI boot manager
> ("bootefi bootmgr")?
> That is the way how UEFI (at least boot manager) boots the kernel.

Good point! Actually, I'm not sure how to define the BOOTxxxx in
config.h (I only
know how to include efivars file when build). Could you tell me how to do it?
I would like to rewrite the default boot commands.

Thank you,

-- 
Masami Hiramatsu

[PATCH 11/13] board: synquacer: Add DeveloperBox 96boards EE support

[Takahiro Akashi]

On Wed, Apr 14, 2021 at 11:06:36AM +0900, Masami Hiramatsu wrote:
> Hi Takahiro,
> 
> 2021?4?14?(?) 10:27 Takahiro Akashi <takahiro.akashi@linaro.org>:
> >
> > On Wed, Apr 14, 2021 at 10:12:42AM +0900, Masami Hiramatsu wrote:
> > > Hello Tom,
> > >
> > > 2021?4?14?(?) 2:47 Tom Rini <trini@konsulko.com>:
> > > >
> > > > On Wed, Apr 14, 2021 at 12:31:21AM +0900, Masami Hiramatsu wrote:
> > > >
> > > > > Add the DeveloperBox 96boards EE support. This board is also
> > > > > known as Socionext SynQuacer E-Series. It contians one "SC2A11"
> > > > > SoC, which has 24-cores of arm Cortex-A53, and 4 DDR3 slots,
> > > > > 3 PCIe slots (1 4x port and 2 1x ports which are expanded via
> > > > > PCIe bridge chip), 2 USB 3.0 ports and 2 USB 2.0 ports, 2 SATA
> > > > > ports and 1 GbE, 64MB NOR flash and 8GB eMMC on standard
> > > > > MicroATX Form Factor.
> > > > >
> > > > > For more information, see this page;
> > > > >   https://www.96boards.org/product/developerbox/
> > > > >
> > > > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > > > [snip]
> > > > > diff --git a/arch/arm/include/asm/arch-sc2a11/gpio.h b/arch/arm/include/asm/arch-sc2a11/gpio.h
> > > > > new file mode 100644
> > > > > index 0000000000..6779803080
> > > > > --- /dev/null
> > > > > +++ b/arch/arm/include/asm/arch-sc2a11/gpio.h
> > > > > @@ -0,0 +1,9 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > > +/*
> > > > > + * Copyright 2021 (C) Linaro Ltd.
> > > > > + */
> > > > > +
> > > > > +#ifndef __ASM_ARCH_SC2A11_GPIO_H
> > > > > +#define __ASM_ARCH_SC2A11_GPIO_H
> > > > > +
> > > > > +#endif
> > > >
> > > > Please update the list in arch/arm/include/asm/gpio.h to not look for
> > > > asm/arch/gpio.h on this SoC, thanks.
> > >
> > > Ah, I missed that. OK, I'll change arch/arm/include/asm/gpio.h.
> > >
> > > BTW, isn't it better to introduce CONFIG_ARCH_GENERIC_GPIO
> > > instead of updating the header?
> > >
> > > > > diff --git a/board/socionext/developerbox/README b/board/socionext/developerbox/README
> > > > > new file mode 100644
> > > > > index 0000000000..bb121002dd
> > > > > --- /dev/null
> > > > > +++ b/board/socionext/developerbox/README
> > > >
> > > > This needs to be in rST form and under doc/board/ now.
> > > >
> > > > [snip]
> > > > > +/*
> > > > > + * arguments for booti command
> > > > > + */
> > > > > +#define LINUX_BASIC_BOOTARGS "basic_bootargs='"                                      \
> > > > > +                             "console=ttyAMA0," __stringify(CONFIG_BAUDRATE) " "     \
> > > > > +                             "rootwait verbose'\0"
> > > > > +
> > > > > +/* kernel:mmcblk0p1(ext2), rootfs:mmcblk0p1(ext2), devtree:mmcblk0p1(ext2) */
> > > > > +#define      LINUX_MMCBOOTCOMMAND1                                                   \
> > > > > +     "mmcboot1=echo 'Boot from MMC (ext2 single rootfs)' ; "                 \
> > > > > +             "mmc dev 0 &&"                                                  \
> > > > > +             "ext2load mmc 0:1 ${kernel_addr} /boot/Image && "               \
> > > > > +             "ext2load mmc 0:1 ${fdt_addr_base} /boot/DeveloperBox.dtb && "  \
> > > > > +             "setenv fdt_addr ${fdt_addr_base} &&"                           \
> > > > > +             "setenv fdt_size ${filesize} &&"                                \
> > > > > +             "setenv bootargs ${mmc_bootargs1} && "                          \
> > > > > +             "booti ${kernel_addr} - ${fdt_addr}\0"                          \
> > > > > +     "mmc_bootargs1="                                                        \
> > > > > +             "root=/dev/mmcblk0p1 rw dtb=0x${fdt_addr},0x${fdt_size} "       \
> > > > > +             "${basic_bootargs} \0"
> > > >
> > > > You are strongly encouraged to use the generic distro boot features
> > > > instead.  This may require a little work to handle the device tree.
> > >
> > > OK, anyway I can just drop it, because this platform is expected to use
> > > UEFI boot (the DeveloperBox is shipped with EDK2).
> > > I left this just for reference.
> >
> > So why not define UEFI load options (BOOTxxxx) and use UEFI boot manager
> > ("bootefi bootmgr")?
> > That is the way how UEFI (at least boot manager) boots the kernel.
> 
> Good point! Actually, I'm not sure how to define the BOOTxxxx in
> config.h (I only
> know how to include efivars file when build). Could you tell me how to do it?
> I would like to rewrite the default boot commands.

For example,
=> efidebug boot add 1 USBBOOT usb 0:1 /EFI/BOOT/bootaa64.efi <boot args>
=> efidebug boot add 2 MMCBOOT mmc 0:1 /EFI/BOOT/bootaa64.efi <boot args>
=> efidebug boot order 1 2
=> bootefi bootmgr

Since "BOOTxxx" are non-volatile variables, we don't have to
set them again once those commands are run.

But distro_bootcmd can also detect and try to boot "bootaa64.efi" anyway.
(I'm not sure about the order of devices to detect though.)

-Takahiro Akashi

> Thank you,
> 
> -- 
> Masami Hiramatsu

[PATCH 11/13] board: synquacer: Add DeveloperBox 96boards EE support

[Masami Hiramatsu]

Hi Takahiro,

2021?4?14?(?) 13:48 Takahiro Akashi <takahiro.akashi@linaro.org>:
>
> > > So why not define UEFI load options (BOOTxxxx) and use UEFI boot manager
> > > ("bootefi bootmgr")?
> > > That is the way how UEFI (at least boot manager) boots the kernel.
> >
> > Good point! Actually, I'm not sure how to define the BOOTxxxx in
> > config.h (I only
> > know how to include efivars file when build). Could you tell me how to do it?
> > I would like to rewrite the default boot commands.
>
> For example,
> => efidebug boot add 1 USBBOOT usb 0:1 /EFI/BOOT/bootaa64.efi <boot args>
> => efidebug boot add 2 MMCBOOT mmc 0:1 /EFI/BOOT/bootaa64.efi <boot args>
> => efidebug boot order 1 2
> => bootefi bootmgr

Hmm, but this can not be embedded in the build process, can this?

>
> Since "BOOTxxx" are non-volatile variables, we don't have to
> set them again once those commands are run.

What is the default behavior of "bootefi bootmgr" if there is no
BOOTxxxx is set?
If it just do nothing and exit, I think I can add it to the top of
CONFIG_BOOTCOMMAND
so that U-Boot can try it first.
(BOOTxxxx will be set by user after boot)

> But distro_bootcmd can also detect and try to boot "bootaa64.efi" anyway.
> (I'm not sure about the order of devices to detect though.)

Hmm, interesting. OK, I'll try to enable distro_bootcmd.

Thank you,

>
> -Takahiro Akashi
>
> > Thank you,
> >
> > --
> > Masami Hiramatsu



-- 
Masami Hiramatsu

[PATCH 11/13] board: synquacer: Add DeveloperBox 96boards EE support

[Takahiro Akashi]

On Wed, Apr 14, 2021 at 03:29:23PM +0900, Masami Hiramatsu wrote:
> Hi Takahiro,
> 
> 2021?4?14?(?) 13:48 Takahiro Akashi <takahiro.akashi@linaro.org>:
> >
> > > > So why not define UEFI load options (BOOTxxxx) and use UEFI boot manager
> > > > ("bootefi bootmgr")?
> > > > That is the way how UEFI (at least boot manager) boots the kernel.
> > >
> > > Good point! Actually, I'm not sure how to define the BOOTxxxx in
> > > config.h (I only
> > > know how to include efivars file when build). Could you tell me how to do it?
> > > I would like to rewrite the default boot commands.
> >
> > For example,
> > => efidebug boot add 1 USBBOOT usb 0:1 /EFI/BOOT/bootaa64.efi <boot args>
> > => efidebug boot add 2 MMCBOOT mmc 0:1 /EFI/BOOT/bootaa64.efi <boot args>
> > => efidebug boot order 1 2
> > => bootefi bootmgr
> 
> Hmm, but this can not be embedded in the build process, can this?

Probably there are a couple of ways;
You may put them in "pre_boot_environment_command" if you don't mind :)

But it would be best to run them as part of OS installation process.
Or you may want to provide a default efivars file(?).

> >
> > Since "BOOTxxx" are non-volatile variables, we don't have to
> > set them again once those commands are run.
> 
> What is the default behavior of "bootefi bootmgr" if there is no
> BOOTxxxx is set?

Nothing will happen.

> If it just do nothing and exit, I think I can add it to the top of
> CONFIG_BOOTCOMMAND
> so that U-Boot can try it first.
> (BOOTxxxx will be set by user after boot)
> 
> > But distro_bootcmd can also detect and try to boot "bootaa64.efi" anyway.
> > (I'm not sure about the order of devices to detect though.)
> 
> Hmm, interesting. OK, I'll try to enable distro_bootcmd.

I'm pretty sure it will work.

-Takahiro Akashi

> Thank you,
> 
> >
> > -Takahiro Akashi
> >
> > > Thank you,
> > >
> > > --
> > > Masami Hiramatsu
> 
> 
> 
> -- 
> Masami Hiramatsu

[PATCH 11/13] board: synquacer: Add DeveloperBox 96boards EE support

[Tom Rini]

On Wed, Apr 14, 2021 at 10:12:42AM +0900, Masami Hiramatsu wrote:
> Hello Tom,
> 
> 2021?4?14?(?) 2:47 Tom Rini <trini@konsulko.com>:
> >
> > On Wed, Apr 14, 2021 at 12:31:21AM +0900, Masami Hiramatsu wrote:
> >
> > > Add the DeveloperBox 96boards EE support. This board is also
> > > known as Socionext SynQuacer E-Series. It contians one "SC2A11"
> > > SoC, which has 24-cores of arm Cortex-A53, and 4 DDR3 slots,
> > > 3 PCIe slots (1 4x port and 2 1x ports which are expanded via
> > > PCIe bridge chip), 2 USB 3.0 ports and 2 USB 2.0 ports, 2 SATA
> > > ports and 1 GbE, 64MB NOR flash and 8GB eMMC on standard
> > > MicroATX Form Factor.
> > >
> > > For more information, see this page;
> > >   https://www.96boards.org/product/developerbox/
> > >
> > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > [snip]
> > > diff --git a/arch/arm/include/asm/arch-sc2a11/gpio.h b/arch/arm/include/asm/arch-sc2a11/gpio.h
> > > new file mode 100644
> > > index 0000000000..6779803080
> > > --- /dev/null
> > > +++ b/arch/arm/include/asm/arch-sc2a11/gpio.h
> > > @@ -0,0 +1,9 @@
> > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > +/*
> > > + * Copyright 2021 (C) Linaro Ltd.
> > > + */
> > > +
> > > +#ifndef __ASM_ARCH_SC2A11_GPIO_H
> > > +#define __ASM_ARCH_SC2A11_GPIO_H
> > > +
> > > +#endif
> >
> > Please update the list in arch/arm/include/asm/gpio.h to not look for
> > asm/arch/gpio.h on this SoC, thanks.
> 
> Ah, I missed that. OK, I'll change arch/arm/include/asm/gpio.h.
> 
> BTW, isn't it better to introduce CONFIG_ARCH_GENERIC_GPIO
> instead of updating the header?

Such a clean-up would be appreciated.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210414/d79b4cae/attachment.sig>

[PATCH 01/13] pci: Update the highest subordinate bus number for bridge setup

[Simon Glass]

On Tue, 13 Apr 2021 at 16:21, Masami Hiramatsu
<masami.hiramatsu@linaro.org> wrote:
>
> Update the highest subordinate bus number after probing the devices
> under the bus for setting up the bridge correctly.
> The commit 42f3663a3f67 ("pci: Update to use new sequence numbers")
> removed this but it is required if a PCIe bridge is under the bus.
>
> Fixes: 42f3663a3f67 ("pci: Update to use new sequence numbers")
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> ---
>  drivers/pci/pci-uclass.c |    3 +++
>  1 file changed, 3 insertions(+)

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

Can we add a test for this?

[PATCH 01/13] pci: Update the highest subordinate bus number for bridge setup

[Tim Harvey]

On Wed, Apr 14, 2021 at 12:38 PM Simon Glass <sjg@chromium.org> wrote:
>
> On Tue, 13 Apr 2021 at 16:21, Masami Hiramatsu
> <masami.hiramatsu@linaro.org> wrote:
> >
> > Update the highest subordinate bus number after probing the devices
> > under the bus for setting up the bridge correctly.
> > The commit 42f3663a3f67 ("pci: Update to use new sequence numbers")
> > removed this but it is required if a PCIe bridge is under the bus.
> >
> > Fixes: 42f3663a3f67 ("pci: Update to use new sequence numbers")
> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > ---
> >  drivers/pci/pci-uclass.c |    3 +++
> >  1 file changed, 3 insertions(+)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> Can we add a test for this?

Simon / Masami,

I ran into this regression as well and likely need the same patch. My
mailer did not receive the original patch and I can't seem to find it
anywhere, can you please resend this?

Best regards,

Tim

[PATCH 01/13] pci: Update the highest subordinate bus number for bridge setup

[Masami Hiramatsu]

Hi Tim,

Sorry for confusion. It may be my git configuration issue.
It seems not to correctly send the series to the SMTP server.
Here is the patch.

Thank you,

2021?4?16?(?) 0:07 Tim Harvey <tharvey@gateworks.com>:
>
> On Wed, Apr 14, 2021 at 12:38 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > On Tue, 13 Apr 2021 at 16:21, Masami Hiramatsu
> > <masami.hiramatsu@linaro.org> wrote:
> > >
> > > Update the highest subordinate bus number after probing the devices
> > > under the bus for setting up the bridge correctly.
> > > The commit 42f3663a3f67 ("pci: Update to use new sequence numbers")
> > > removed this but it is required if a PCIe bridge is under the bus.
> > >
> > > Fixes: 42f3663a3f67 ("pci: Update to use new sequence numbers")
> > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > > ---
> > >  drivers/pci/pci-uclass.c |    3 +++
> > >  1 file changed, 3 insertions(+)
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> >
> > Can we add a test for this?
>
> Simon / Masami,
>
> I ran into this regression as well and likely need the same patch. My
> mailer did not receive the original patch and I can't seem to find it
> anywhere, can you please resend this?
>
> Best regards,
>
> Tim



-- 
Masami Hiramatsu
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pci-update-the-highest
Type: application/octet-stream
Size: 967 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210416/111733fa/attachment.obj>

[PATCH 01/13] pci: Update the highest subordinate bus number for bridge setup

[Tim Harvey]

On Thu, Apr 15, 2021 at 5:15 PM Masami Hiramatsu
<masami.hiramatsu@linaro.org> wrote:
>
> Hi Tim,
>
> Sorry for confusion. It may be my git configuration issue.
> It seems not to correctly send the series to the SMTP server.
> Here is the patch.
>
> Thank you,
>
> 2021?4?16?(?) 0:07 Tim Harvey <tharvey@gateworks.com>:
> >
> > On Wed, Apr 14, 2021 at 12:38 PM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > On Tue, 13 Apr 2021 at 16:21, Masami Hiramatsu
> > > <masami.hiramatsu@linaro.org> wrote:
> > > >
> > > > Update the highest subordinate bus number after probing the devices
> > > > under the bus for setting up the bridge correctly.
> > > > The commit 42f3663a3f67 ("pci: Update to use new sequence numbers")
> > > > removed this but it is required if a PCIe bridge is under the bus.
> > > >
> > > > Fixes: 42f3663a3f67 ("pci: Update to use new sequence numbers")
> > > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > > > ---
> > > >  drivers/pci/pci-uclass.c |    3 +++
> > > >  1 file changed, 3 insertions(+)
> > >
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > >
> > > Can we add a test for this?
> >
> > Simon / Masami,
> >
> > I ran into this regression as well and likely need the same patch. My
> > mailer did not receive the original patch and I can't seem to find it
> > anywhere, can you please resend this?
> >
> > Best regards,
> >
> > Tim
>

Masami,

Thanks - yes, this is the patch I have also which is required for
dm-pci with a bridge which I have on Gateworks Ventana boards.

Tested-by: Tim Harvey <tharvey@gateworks.com>

Tim

[PATCH 01/13] pci: Update the highest subordinate bus number for bridge setup

[Masami Hiramatsu]

Hi Tim,

2021?4?16?(?) 9:21 Tim Harvey <tharvey@gateworks.com>:
>
> On Thu, Apr 15, 2021 at 5:15 PM Masami Hiramatsu
> <masami.hiramatsu@linaro.org> wrote:
> >
> > Hi Tim,
> >
> > Sorry for confusion. It may be my git configuration issue.
> > It seems not to correctly send the series to the SMTP server.
> > Here is the patch.
> >
> > Thank you,
> >
> > 2021?4?16?(?) 0:07 Tim Harvey <tharvey@gateworks.com>:
> > >
> > > On Wed, Apr 14, 2021 at 12:38 PM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > On Tue, 13 Apr 2021 at 16:21, Masami Hiramatsu
> > > > <masami.hiramatsu@linaro.org> wrote:
> > > > >
> > > > > Update the highest subordinate bus number after probing the devices
> > > > > under the bus for setting up the bridge correctly.
> > > > > The commit 42f3663a3f67 ("pci: Update to use new sequence numbers")
> > > > > removed this but it is required if a PCIe bridge is under the bus.
> > > > >
> > > > > Fixes: 42f3663a3f67 ("pci: Update to use new sequence numbers")
> > > > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > > > > ---
> > > > >  drivers/pci/pci-uclass.c |    3 +++
> > > > >  1 file changed, 3 insertions(+)
> > > >
> > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > >
> > > > Can we add a test for this?
> > >
> > > Simon / Masami,
> > >
> > > I ran into this regression as well and likely need the same patch. My
> > > mailer did not receive the original patch and I can't seem to find it
> > > anywhere, can you please resend this?
> > >
> > > Best regards,
> > >
> > > Tim
> >
>
> Masami,
>
> Thanks - yes, this is the patch I have also which is required for
> dm-pci with a bridge which I have on Gateworks Ventana boards.
>
> Tested-by: Tim Harvey <tharvey@gateworks.com>

Thanks for testing!


>
> Tim



-- 
Masami Hiramatsu

[PATCH 02/13] ata: ahci-pci: Use scsi_ops to initialize ops

[Simon Glass]

On Tue, 13 Apr 2021 at 16:22, Masami Hiramatsu
<masami.hiramatsu@linaro.org> wrote:
>
> Without this fix, scsi-scan will cause a synchronous abort
> when accessing ops->scan.
>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> ---
>  drivers/ata/ahci-pci.c |    2 ++
>  1 file changed, 2 insertions(+)

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

[PATCH 03/13] dm: pci: Skip setting VGA bridge bits if parent device is the host bus

[Simon Glass]

On Tue, 13 Apr 2021 at 16:23, Masami Hiramatsu
<masami.hiramatsu@linaro.org> wrote:
>
> Commit bbbcb5262839 ("dm: pci: Enable VGA address forwarding on bridges")
> sets the VGA bridge bits by checking pplat->class, but if the parent
> device is the pci host bus device, it can be skipped. Moreover, it
> shouldn't access the pplat because the parent has different plat data.
>
> Without this fix, "pci enum" command cause a synchronous abort.
>
> pci_auto_config_devices: start
> PCI Autoconfig: Bus Memory region: [78000000-7fffffff],
>                 Physical Memory [78000000-7fffffffx]
> PCI Autoconfig: Bus I/O region: [0-ffff],
>                 Physical Memory [77f00000-77f0ffffx]
> pci_auto_config_devices: device pci_6:0.0
> PCI Autoconfig: BAR 0, Mem, size=0x1000000, address=0x78000000 bus_lower=0x79000000
>
> PCI Autoconfig: BAR 1, Mem, size=0x8000000, No room in resource, avail start=79000000 / size=8000000, need=8000000
> PCI: Failed autoconfig bar 14
>
> PCI Autoconfig: BAR 2, I/O, size=0x4, address=0x1000 bus_lower=0x1004
>
> PCI Autoconfig: BAR 3, Mem, size=0x2000000, address=0x7a000000 bus_lower=0x7c000000
>
> PCI Autoconfig: BAR 4, I/O, size=0x80, address=0x1080 bus_lower=0x1100
>
> PCI Autoconfig: ROM, size=0x80000, address=0x7c000000 bus_lower=0x7c080000
>
> "Synchronous Abort" handler, esr 0x96000006
> elr: 00000000e002bd28 lr : 00000000e002bce8 (reloc)
> elr: 00000000fff6fd28 lr : 00000000fff6fce8
> x0 : 0000000000001041 x1 : 000000000000003e
> x2 : 00000000ffb0f8c8 x3 : 0000000000000001
> x4 : 0000000000000080 x5 : 0000000000000000
> x6 : 00000000fff718fc x7 : 000000000000000f
> x8 : 00000000ffb0f238 x9 : 0000000000000008
> x10: 0000000000000000 x11: 0000000000000010
> x12: 0000000000000006 x13: 000000000001869f
> x14: 00000000ffb0fcd0 x15: 0000000000000020
> x16: 00000000fff71cc4 x17: 0000000000000000
> x18: 00000000ffb13d90 x19: 00000000ffb14320
> x20: 0000000000000000 x21: 00000000ffb14090
> x22: 00000000ffb0f8c8 x23: 0000000000000001
> x24: 00000000ffb14c10 x25: 0000000000000000
> x26: 0000000000000000 x27: 0000000000000000
> x28: 00000000ffb14c70 x29: 00000000ffb0f830
>
> Code: 52800843 52800061 52800e00 97ffcf65 (b9400280)
> Resetting CPU ...
>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> ---
>  drivers/pci/pci-uclass.c |    3 +++
>  1 file changed, 3 insertions(+)

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

[PATCH 03/13] dm: pci: Skip setting VGA bridge bits if parent device is the host bus

[Bin Meng]

Hi,

On Thu, Apr 15, 2021 at 3:39 AM Simon Glass <sjg@chromium.org> wrote:
>
> On Tue, 13 Apr 2021 at 16:23, Masami Hiramatsu
> <masami.hiramatsu@linaro.org> wrote:
> >
> > Commit bbbcb5262839 ("dm: pci: Enable VGA address forwarding on bridges")
> > sets the VGA bridge bits by checking pplat->class, but if the parent
> > device is the pci host bus device, it can be skipped. Moreover, it
> > shouldn't access the pplat because the parent has different plat data.
> >
> > Without this fix, "pci enum" command cause a synchronous abort.
> >
> > pci_auto_config_devices: start
> > PCI Autoconfig: Bus Memory region: [78000000-7fffffff],
> >                 Physical Memory [78000000-7fffffffx]
> > PCI Autoconfig: Bus I/O region: [0-ffff],
> >                 Physical Memory [77f00000-77f0ffffx]
> > pci_auto_config_devices: device pci_6:0.0
> > PCI Autoconfig: BAR 0, Mem, size=0x1000000, address=0x78000000 bus_lower=0x79000000
> >
> > PCI Autoconfig: BAR 1, Mem, size=0x8000000, No room in resource, avail start=79000000 / size=8000000, need=8000000
> > PCI: Failed autoconfig bar 14
> >
> > PCI Autoconfig: BAR 2, I/O, size=0x4, address=0x1000 bus_lower=0x1004
> >
> > PCI Autoconfig: BAR 3, Mem, size=0x2000000, address=0x7a000000 bus_lower=0x7c000000
> >
> > PCI Autoconfig: BAR 4, I/O, size=0x80, address=0x1080 bus_lower=0x1100
> >
> > PCI Autoconfig: ROM, size=0x80000, address=0x7c000000 bus_lower=0x7c080000
> >
> > "Synchronous Abort" handler, esr 0x96000006
> > elr: 00000000e002bd28 lr : 00000000e002bce8 (reloc)
> > elr: 00000000fff6fd28 lr : 00000000fff6fce8
> > x0 : 0000000000001041 x1 : 000000000000003e
> > x2 : 00000000ffb0f8c8 x3 : 0000000000000001
> > x4 : 0000000000000080 x5 : 0000000000000000
> > x6 : 00000000fff718fc x7 : 000000000000000f
> > x8 : 00000000ffb0f238 x9 : 0000000000000008
> > x10: 0000000000000000 x11: 0000000000000010
> > x12: 0000000000000006 x13: 000000000001869f
> > x14: 00000000ffb0fcd0 x15: 0000000000000020
> > x16: 00000000fff71cc4 x17: 0000000000000000
> > x18: 00000000ffb13d90 x19: 00000000ffb14320
> > x20: 0000000000000000 x21: 00000000ffb14090
> > x22: 00000000ffb0f8c8 x23: 0000000000000001
> > x24: 00000000ffb14c10 x25: 0000000000000000
> > x26: 0000000000000000 x27: 0000000000000000
> > x28: 00000000ffb14c70 x29: 00000000ffb0f830
> >
> > Code: 52800843 52800061 52800e00 97ffcf65 (b9400280)
> > Resetting CPU ...
> >
> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > ---
> >  drivers/pci/pci-uclass.c |    3 +++
> >  1 file changed, 3 insertions(+)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>

I can't find this patch in my inbox, nor in the patchwork.

Could you please resend?

Regards,
Bin

[PATCH 03/13] dm: pci: Skip setting VGA bridge bits if parent device is the host bus

[Masami Hiramatsu]

Hi Bin,

On Thu, 15 Apr 2021 06:30:27 +0800
Bin Meng <bmeng.cn@gmail.com> wrote:

> Hi,
> 
> On Thu, Apr 15, 2021 at 3:39 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > On Tue, 13 Apr 2021 at 16:23, Masami Hiramatsu
> > <masami.hiramatsu@linaro.org> wrote:
> > >
> > > Commit bbbcb5262839 ("dm: pci: Enable VGA address forwarding on bridges")
> > > sets the VGA bridge bits by checking pplat->class, but if the parent
> > > device is the pci host bus device, it can be skipped. Moreover, it
> > > shouldn't access the pplat because the parent has different plat data.
> > >
> > > Without this fix, "pci enum" command cause a synchronous abort.
> > >
> > > pci_auto_config_devices: start
> > > PCI Autoconfig: Bus Memory region: [78000000-7fffffff],
> > >                 Physical Memory [78000000-7fffffffx]
> > > PCI Autoconfig: Bus I/O region: [0-ffff],
> > >                 Physical Memory [77f00000-77f0ffffx]
> > > pci_auto_config_devices: device pci_6:0.0
> > > PCI Autoconfig: BAR 0, Mem, size=0x1000000, address=0x78000000 bus_lower=0x79000000
> > >
> > > PCI Autoconfig: BAR 1, Mem, size=0x8000000, No room in resource, avail start=79000000 / size=8000000, need=8000000
> > > PCI: Failed autoconfig bar 14
> > >
> > > PCI Autoconfig: BAR 2, I/O, size=0x4, address=0x1000 bus_lower=0x1004
> > >
> > > PCI Autoconfig: BAR 3, Mem, size=0x2000000, address=0x7a000000 bus_lower=0x7c000000
> > >
> > > PCI Autoconfig: BAR 4, I/O, size=0x80, address=0x1080 bus_lower=0x1100
> > >
> > > PCI Autoconfig: ROM, size=0x80000, address=0x7c000000 bus_lower=0x7c080000
> > >
> > > "Synchronous Abort" handler, esr 0x96000006
> > > elr: 00000000e002bd28 lr : 00000000e002bce8 (reloc)
> > > elr: 00000000fff6fd28 lr : 00000000fff6fce8
> > > x0 : 0000000000001041 x1 : 000000000000003e
> > > x2 : 00000000ffb0f8c8 x3 : 0000000000000001
> > > x4 : 0000000000000080 x5 : 0000000000000000
> > > x6 : 00000000fff718fc x7 : 000000000000000f
> > > x8 : 00000000ffb0f238 x9 : 0000000000000008
> > > x10: 0000000000000000 x11: 0000000000000010
> > > x12: 0000000000000006 x13: 000000000001869f
> > > x14: 00000000ffb0fcd0 x15: 0000000000000020
> > > x16: 00000000fff71cc4 x17: 0000000000000000
> > > x18: 00000000ffb13d90 x19: 00000000ffb14320
> > > x20: 0000000000000000 x21: 00000000ffb14090
> > > x22: 00000000ffb0f8c8 x23: 0000000000000001
> > > x24: 00000000ffb14c10 x25: 0000000000000000
> > > x26: 0000000000000000 x27: 0000000000000000
> > > x28: 00000000ffb14c70 x29: 00000000ffb0f830
> > >
> > > Code: 52800843 52800061 52800e00 97ffcf65 (b9400280)
> > > Resetting CPU ...
> > >
> > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > > ---
> > >  drivers/pci/pci-uclass.c |    3 +++
> > >  1 file changed, 3 insertions(+)
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> I can't find this patch in my inbox, nor in the patchwork.

Hmm, it is strange... I set up gitconfig but it seems not working well.
Let me send patch via my MUA next time.

> Could you please resend?

OK, I attached it to this mail. (maybe it is safer in this case)

Thank you,


-- 
Linaro <masami.hiramatsu@linaro.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dm-pci-skip-setting-vga-bridge
Type: application/octet-stream
Size: 2727 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210415/1d48b771/attachment.obj>