[RFC] dev_phys_to_bus() and PCI

[Mark Kettenis]

I'm working on support for Apple's M1 systems in U-Boot.  The idea is
that this can be used as a "payload" for the m1n1 bootloader that is
being developed by Hector Martin for Asahi Linux in order to provide
an UEFI implementation that can boot a standard arm64 OS.

My current code, which can be found on the "apple-m1-m1n1" branch at:

  https://github.com/kettenis/u-boot/tree/apple-m1-m1n1

can already do this.  I'm booting OpenBSD/arm64 this way and I've also
booted into grub using a standard arm64 Debian installer.

One of the big challenges I'm facing is that the integrated PCIe
devices on the system sit behind an IOMMU that (as far as we can tell)
doesn't support any kind of bypass mode.  I don't think we want full
IOMMU support in U-Boot, so the approach I'm currently taking is that
I set up the IOMMU to map a chunk of memory at the "top of RAM" where
U-Boot resides after relocation.  But in order to use this mapping I
need to do DMA address translation.

Fortunately Nicolas Saenz Julienne recently introduced
dev_phys_to_bus() and dev_bus_to_phys() interfaces to do this.  Those
interfaces make use of a dma-ranges property in the device tree which
doesn't work so well for PCI devices though.  However, the PCI code in
U-Boot already has a way to describe DMA address translation through
its regions support.  Hooking this up to dev_phys_to_bus() and
dev_bus_to_phys() is fairly easy as illustrated by the diff below.

Would this be viable approach?  This could also help adding support
for PCIe devices on the Raspberry Pi CM4.


commit 4f0e989c7a765291a38b7d10da562f23c5f31239
Author: Mark Kettenis 
Date:   Fri Mar 12 20:23:11 2021 +0100

dm: Add PCI support to dev_phys_to_bus()/dev_bus_to_phys()

For PCI devices, call dm_pci_phys_to_bus()/dm_pci_bus_to_phys()
to do the address translation.  This uses the regions set up
by the PCI host controller driver to do the translation as a
single translation offset may not be sufficient in this case.

Signed-off-by: Mark Kettenis 

diff --git a/include/phys2bus.h b/include/phys2bus.h
index 866b8b51a8..13d23ef4bb 100644
--- a/include/phys2bus.h
+++ b/include/phys2bus.h
@@ -23,14 +23,21 @@ static inline unsigned long bus_to_phys(unsigned long bus)
 
 #if CONFIG_IS_ENABLED(DM)
 #include 
+#include 
 
 static inline dma_addr_t dev_phys_to_bus(struct udevice *dev, phys_addr_t phys)
 {
+   if (device_is_on_pci_bus(dev))
+   return dm_pci_phys_to_bus(dev, phys, PCI_REGION_SYS_MEMORY);
+
return phys - dev_get_dma_offset(dev);
 }
 
 static inline phys_addr_t dev_bus_to_phys(struct udevice *dev, dma_addr_t bus)
 {
+   if (device_is_on_pci_bus(dev))
+   return dm_pci_bus_to_phys(dev, bus, PCI_REGION_SYS_MEMORY);
+
return bus + dev_get_dma_offset(dev);
 }
 #else

Re: [PATCH v2 01/21] dt-bindings: net: Add the old DT bindings for "fixed-link"

[Vladimir Oltean]

On Fri, Mar 12, 2021 at 09:35:42PM +0800, Bin Meng wrote:
> Per the upstream Linux kernel doc:
> 
>   Documentation/devicetree/bindings/net/ethernet-controller.yaml
> 
> There are two ways to describe a fixed PHY attached to an Ethernet
> device. This updates our dt-bindings doc to add the old DT bindings.
> 
> Signed-off-by: Bin Meng 
> Reviewed-by: Ramon Fried 
> ---

Why?
Do we even parse the old-style bindings? And if so, where?

Re: [PATCH v2 02/21] of: extra: Introduce ofnode_phy_is_fixed_link() API

[Vladimir Oltean]

On Fri, Mar 12, 2021 at 09:35:43PM +0800, Bin Meng wrote:
> Introduce a helper API ofnode_phy_is_fixed_link() to detect whether
> the ethernet controller connects to a fixed-link pseudo-PHY device.
>
> Note there are two ways to describe a fixed PHY attached to an
> Ethernet device:
>
> - the new DT binding, where 'fixed-link' is a sub-node of the
>   Ethernet device
> - the old DT binding, where 'fixed-link' is a property with 5
>   cells encoding various information about the fixed PHY
>
> Signed-off-by: Bin Meng 
> Reviewed-by: Simon Glass 
> ---

I ran a 'grep -r "ofnode_get_property.*fixed-link" .' and saw no in-tree
users of the old binding. Why do we bother to be compatible with
something which isn't used?

Re: [PATCH v2 02/21] of: extra: Introduce ofnode_phy_is_fixed_link() API

[Vladimir Oltean]

On Sat, Mar 13, 2021 at 02:14:36PM +0200, Vladimir Oltean wrote:
> On Fri, Mar 12, 2021 at 09:35:43PM +0800, Bin Meng wrote:
> > Introduce a helper API ofnode_phy_is_fixed_link() to detect whether
> > the ethernet controller connects to a fixed-link pseudo-PHY device.
> >
> > Note there are two ways to describe a fixed PHY attached to an
> > Ethernet device:
> >
> > - the new DT binding, where 'fixed-link' is a sub-node of the
> >   Ethernet device
> > - the old DT binding, where 'fixed-link' is a property with 5
> >   cells encoding various information about the fixed PHY
> >
> > Signed-off-by: Bin Meng 
> > Reviewed-by: Simon Glass 
> > ---
> 
> I ran a 'grep -r "ofnode_get_property.*fixed-link" .' and saw no in-tree
> users of the old binding. Why do we bother to be compatible with
> something which isn't used?

Ah, I see what's going on.
QEMU fixes up the device tree here:
https://github.com/qemu/qemu/blob/master/hw/ppc/e500.c#L239
and adds an old-style fixed-link binding.
Can't you modify it to add a new-style fixed-link property? It's not
like you didn't have to modify it for the "ranges" property too :)
https://github.com/qemu/qemu/commit/e5943b00d35efc68ca72ed304cfca98a9f3a647c

Re: [PATCH v2 15/21] dt-bindings: net: Update Freescale TSEC to support "queue-group"

[Vladimir Oltean]

On Fri, Mar 12, 2021 at 09:35:56PM +0800, Bin Meng wrote:
> At present the Freescale TSEC node DT bindings doc requires a 
> property in the TSEC node. But this might not always be the case.
> In the upstream Linux kernel, there is no DT bindings doc for it
> but the kernel driver tests a subnode of a name prefixed with
> "queue-group", as we can see from gfar_of_init():
> 
>   for_each_available_child_of_node(np, child) {
>   if (!of_node_name_eq(child, "queue-group"))
>   ...
> 
> in drivers/net/ethernet/freescale/gianfar.c
> 
> Update our DT bindings to describe this alternate description.
> 
> Signed-off-by: Bin Meng 
> Reviewed-by: Ramon Fried 
> ---

Fascinating. I did not even notice that the Linux DT bindings do not
have a "reg" property and rely on "ranges" for translation to the
queue-group. I guess that changes things. I'll go again through the
patches starting from the first one.

Re: [PATCH v2 01/21] dt-bindings: net: Add the old DT bindings for "fixed-link"

[Vladimir Oltean]

On Fri, Mar 12, 2021 at 09:35:42PM +0800, Bin Meng wrote:
> +- The old DT binding, where 'fixed-link' is a property with 5 cells
> +  encoding various information about the fixed PHY, in the form of
> +  .
> +
> +  * 'phy_id', emulated PHY ID, choose any but unique to the all specified
> +fixed-links

Could you please explicitly state that U-Boot deliberately ignores the
phy_id and unconditionally uses PHY_FIXED_ID? It would be suicidal to do
anything else (and I see Linux ignores fixed_link_prop[0] too), but at
least we should be upfront about it.

> +  * 'full-duplex', 0 for half duplex or 1 for full duplex
> +  * 'speed', link speed in Mbits/sec, accepts only 10, 100 and 1000
> +  * 'pause', 0 for no pause, 1 for pause
> +  * 'asym-pause', 0 for no asymmetric pause, 1 for asymmetric pause

Reviewed-by: Vladimir Oltean 

Re: [PATCH v2 02/21] of: extra: Introduce ofnode_phy_is_fixed_link() API

[Vladimir Oltean]

On Fri, Mar 12, 2021 at 09:35:43PM +0800, Bin Meng wrote:
> +bool ofnode_phy_is_fixed_link(ofnode eth_node, ofnode *phy_node)
> +{
> + bool found = false;
> + ofnode node, subnode;
> + int len;
> +
> + /* new binding */
> + subnode = ofnode_find_subnode(eth_node, "fixed-link");
> + if (ofnode_valid(subnode)) {
> + node = subnode;
> + found = true;
> + }
> +
> + /* old binding */
> + if (ofnode_get_property(eth_node, "fixed-link", &len) &&

Maybe "else if" here? If an old-style and a new-style binding exist, we
should prefer looking at the new one.

And with that "else if", we could remove the "found" variable:

if (ofnode_valid(subnode)) {
} else if (ofnode_get_property(eth_node, "fixed-link", &len) &&
   len == (5 * sizeof(__be32))) {
} else {
return false;
}

if (phy_node)
*phy_node = node;

> + len == (5 * sizeof(__be32))) {
> + node = eth_node;
> + found = true;
> + }
> +
> + if (found && phy_node)
> + *phy_node = node;
> +
> + return found;
> +}

Re: [PATCH v2 03/21] test: dm: Add a case to test ofnode_phy_is_fixed_link()

[Vladimir Oltean]

On Fri, Mar 12, 2021 at 09:35:44PM +0800, Bin Meng wrote:
> This adds a test case to test the new ofnode_phy_is_fixed_link() API.
> Both the new and old DT bindings are covered.
> 
> Signed-off-by: Bin Meng 
> Reviewed-by: Simon Glass 
> ---

The DSA sandbox patch adds three fixed-link nodes already. Can't you
convert one of those three fixed-link nodes to an old-style binding, and
modify your test case to cover those existing nodes?

Re: [PATCH v2 04/21] dm: mdio: Use ofnode_phy_is_fixed_link() API

[Vladimir Oltean]

On Fri, Mar 12, 2021 at 09:35:45PM +0800, Bin Meng wrote:
> Switch to use the ofnode_phy_is_fixed_link() API which can support
> both the new and old DT bindings.
> 
> Signed-off-by: Bin Meng 
> Reviewed-by: Ramon Fried 
> ---

Reviewed-by: Vladimir Oltean 

Re: [PATCH v2 05/21] net: phy: fixed: Be compatible with live OF tree

[Vladimir Oltean]

On Fri, Mar 12, 2021 at 09:35:46PM +0800, Bin Meng wrote:
> From: Vladimir Oltean 
> 
> On systems that use CONFIG_OF_LIVE, the "ofnode" type is defined
> as const struct device_node *np, while on the flat DT systems it
> is defined as a long of_offset into gd->fdt_blob.
> 
> It is desirable that the fixed PHY driver uses the higher-level
> ofnode abstraction instead of parsing gd->fdt_blob directly,
> because that enables it to work on live OF systems.
> 
> The fixed PHY driver has used a nasty hack since its introduction in
> commit db40c1aa1c10 ("drivers/net/phy: add fixed-phy / fixed-link support"),
> which is to pass the long gd->fdt_blob offset inside int phydev->addr
> (a value that normally holds the MDIO bus address at which the PHY
> responds). Even ignoring the fact that the types were already
> mismatched leading to a potential truncation (flat OF offset was
> supposed to be a long and not an int), we really cannot extend this
> hack any longer, because there's no way an int will hold the other
> representation of ofnode, the struct device_node *np.
> 
> So we unfortunately need to do the right thing, which is to use the
> framework introduced by Grygorii Strashko in
> commit eef0b8a930d1 ("net: phy: add ofnode node to struct phy_device").
> This will populate phydev->node for the fixed PHY.
> 
> Note that phydev->node will not be valid in the probe function, since
> that is called synchronously from phy_device_create and we really have
> no way of passing the ofnode directly through the phy_device_create API.
> So we do what other drivers do too: we move the OF parsing logic from
> the .probe to the .config method of the PHY driver. The new function
> will be called at phy_config() time.
> 
> I do believe I've converted all the possible call paths for creating
> a PHY with PHY_FIXED_ID, so there is really no reason to maintain
> compatibility with the old logic of retrieving a flat OF tree offset
> from phydev->addr. We just pass 0 to phydev->addr now.
> 
> Signed-off-by: Vladimir Oltean 
> Reviewed-by: Bin Meng 
> Tested-by: Bin Meng 
> Message-Id: <20210216224804.3355044-2-olte...@gmail.com>
> [bmeng: keep fixedphy_probe(); update mdio-uclass.c to handle fixed phy]
> Signed-off-by: Bin Meng 
> ---

Reviewed-by: Vladimir Oltean 

Re: [PATCH v2 06/21] net: phy: fixed: Drop #ifdef CONFIG_DM_ETH around phy_connect_fixed

[Vladimir Oltean]

On Fri, Mar 12, 2021 at 09:35:47PM +0800, Bin Meng wrote:
> From: Vladimir Oltean 
> 
> In drivers/net/phy/Kconfig, CONFIG_PHY_FIXED already depends on
> CONFIG_DM_ETH, so the function prototype definition when
> CONFIG_DM_ETH=n does nothing, so it can be dropped. It is also
> never reachable, since the whole function is already under #ifdef
> CONFIG_PHY_FIXED (which again, as I said, depends on CONFIG_DM_ETH=y).
> 
> Signed-off-by: Vladimir Oltean 
> Reviewed-by: Bin Meng 
> Tested-by: Bin Meng 
> Message-Id: <20210216224804.3355044-3-olte...@gmail.com>
> Signed-off-by: Bin Meng 
> ---

Reviewed-by: Vladimir Oltean 

Re: [PATCH v2 08/21] net: phy: xilinx: Drop #ifdef CONFIG_DM_ETH around phy_connect_gmii2rgmii()

[Vladimir Oltean]

On Fri, Mar 12, 2021 at 09:35:49PM +0800, Bin Meng wrote:
> At present phy_connect_gmii2rgmii() is implemented using a DM API
> dev_of_offset() hence it cannot support a non-DM configuration.
> Remove the non-DM version prototype of phy_connect_gmii2rgmii()
> and make the driver depend on CONFIG_DM_ETH.
> 
> Signed-off-by: Bin Meng 
> Acked-by: Michal Simek 
> Reviewed-by: Ramon Fried 
> ---

Reviewed-by: Vladimir Oltean 

Re: [PATCH v2 09/21] net: phy: Simplify the logic of phy_connect_fixed()

[Vladimir Oltean]

On Fri, Mar 12, 2021 at 09:35:50PM +0800, Bin Meng wrote:
> Simplify the logic of phy_connect_fixed() by using the new API
> ofnode_phy_is_fixed_link(), which brings additional bonus of
> supporting the old DT bindings.
> 
> Signed-off-by: Bin Meng 
> Reviewed-by: Ramon Fried 
> ---

Reviewed-by: Vladimir Oltean 

Re: [PATCH v2 10/21] net: phy: fixed: Make driver ops static

[Vladimir Oltean]

On Fri, Mar 12, 2021 at 09:35:51PM +0800, Bin Meng wrote:
> The PHY driver ops should be made static.
> 
> Signed-off-by: Bin Meng 
> Reviewed-by: Ramon Fried 
> ---

Reviewed-by: Vladimir Oltean 

Re: [PATCH v2 11/21] net: phy: fixed: Add the missing ending newline

[Vladimir Oltean]

On Fri, Mar 12, 2021 at 09:35:52PM +0800, Bin Meng wrote:
> The printf statement doesn't end with a newline. Add it.
> 
> Signed-off-by: Bin Meng 
> Reviewed-by: Ramon Fried 
> ---

Reviewed-by: Vladimir Oltean 

Re: [PATCH v2 12/21] net: phy: fixed: Support the old DT binding

[Vladimir Oltean]

On Fri, Mar 12, 2021 at 09:35:53PM +0800, Bin Meng wrote:
> Update fixedphy_probe() to support the old DT binding.
> 
> Signed-off-by: Bin Meng 
> Reviewed-by: Ramon Fried 
> ---
> 
> (no changes since v1)
> 
>  drivers/net/phy/fixed.c | 26 +++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
> index 304e506554..ea05a45244 100644
> --- a/drivers/net/phy/fixed.c
> +++ b/drivers/net/phy/fixed.c
> @@ -27,13 +27,27 @@ static int fixedphy_config(struct phy_device *phydev)
>  {
>   ofnode node = phy_get_ofnode(phydev);
>   struct fixed_link *priv;
> + bool old_binding = false;
>   u32 val;
> + u32 old_val[5];

Perhaps a pet peeve from Linux networking, but could you please keep the
variable definitions sorted by line length? Thanks.

Reviewed-by: Vladimir Oltean 

Re: [PATCH v2 13/21] net: tsec: Use dm_eth_phy_connect() directly for the DM case

[Vladimir Oltean]

On Fri, Mar 12, 2021 at 09:35:54PM +0800, Bin Meng wrote:
> From: Vladimir Oltean 
> 
> Now that the fixed phy driver has been fully adapted to OF APIs,
> and dm_eth_phy_connect() already can handle the fixed phy, call
> dm_eth_phy_connect() directly in the DM tsec driver.
> 
> Signed-off-by: Vladimir Oltean 
> Reviewed-by: Bin Meng 
> Tested-by: Bin Meng 
> Message-Id: <20210216224804.3355044-4-olte...@gmail.com>
> [bmeng: split from "net: mdio: teach dm_eth_phy_connect to connect to fixed 
> PHY"]
> Signed-off-by: Bin Meng 
> 
> ---

Reviewed-by: Vladimir Oltean 

Re: [PATCH 0/7] Add FU740 chip and HiFive Unmatched board support

[Carlos Eduardo de Paula]

Hi Wan, I found a couple of issues while trying to apply/build this
patch series to latest v2021.04-rc branches:

- The defconfig for Unmatched has a typo:
arch/../configs/sifive_hifive_unmatched_fu740_defconfig:55:warning:
unexpected data: ONFIG_DM_PWM=y

- The DM_RESET config depends on TARGET_SIFIVE_HIFIVE_UNMATCHED_FU740
but this target does not exist. I believe the correct is
TARGET_SIFIVE_UNMATCHED right?

Adjusted this and tried to build it and flash both u-boot-spl.bin and
the u-boot.itb as the OpenSBI (from master branch) payload but I got
this while booting:

-
U-Boot SPL 2021.04-rc2-dirty (Mar 12 2021 - 15:09:49 -0500)
Trying to boot from MMC1
Unhandled UUcanhladn dclpdpexocepttiog: Inlllegtal i0nst
uction
C0:PC:0 00f0f:0801Rf0ff08:a 0RA: V00 000802:00f10 TVAL: fff
   f


resetting ...
reset etent p..ngete.ng .e.settRngsu ..e.
sets
#
t enT oet R s pp#rtPed y#et
 ### ERROR ### Please RESET the board ###
-

Carlos


-- 
__
Carlos Eduardo de Paula
carlos...@gmail.com
http://carlosedp.com
http://twitter.com/carlosedp
__

Re: [PATCH v2 16/21] net: tsec: Support property from the subnode "queue-group"

[Vladimir Oltean]

On Fri, Mar 12, 2021 at 09:35:57PM +0800, Bin Meng wrote:
> At present the tsec driver uses a non-standard DT bindings to get
> its  base / size. The upstream Linux kernel seems to require
> the  base / size to be put under a subnode of the eTSEC node
> with a name prefix "queue-group". This is not documented in the
> kernel DT bindings, but it looks every dtsi file that contains the
> eTSEC node was written like this.
> 
> This commit updates the tsec driver to handle this case.
> 
> Signed-off-by: Bin Meng 
> Reviewed-by: Ramon Fried 
> ---
> 
> (no changes since v1)
> 
>  drivers/net/tsec.c | 26 --
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
> index f801d020fb..49bed0c1dd 100644
> --- a/drivers/net/tsec.c
> +++ b/drivers/net/tsec.c
> @@ -827,13 +827,35 @@ int tsec_probe(struct udevice *dev)
>   struct tsec_data *data;
>   const char *phy_mode;
>   fdt_addr_t reg;
> - ofnode parent;
> + ofnode parent, child;

Same comment about the "reverse Christmas tree" notation.

>   int ret;
>  
>   data = (struct tsec_data *)dev_get_driver_data(dev);
>  
>   pdata->iobase = (phys_addr_t)dev_read_addr(dev);
> - priv->regs = dev_remap_addr(dev);
> + if (pdata->iobase != FDT_ADDR_T_NONE) {
> + priv->regs = dev_remap_addr(dev);
> + } else {
> + ofnode_for_each_subnode(child, dev_ofnode(dev)) {
> + if (!strncmp(ofnode_get_name(child), "queue-group",
> +  strlen("queue-group"))) {

I would prefer the syntax:

if (strncmp(ofnode_get_name(child), "queue-group",
strlen("queue-group")))
continue;

which allows us to reduce the indentation level.

> + reg = ofnode_get_addr(child);
> + if (reg == FDT_ADDR_T_NONE) {
> + printf("No 'reg' property of 
> \n");
> + return -ENOENT;
> + }
> + pdata->iobase = reg;
> + priv->regs = map_physmem(pdata->iobase, 0,
> +  MAP_NOCACHE);

Hmm, can't you just populate pdata->iobase, and call the same
dev_remap_addr in the common code path?

> + break;

Could you please add a comment that if there are multiple queue groups,
we only use the first one?

> + }
> + }
> +
> + if (!ofnode_valid(child)) {
> + printf("No child node for ?\n");
> + return -ENOENT;
> + }
> + }

Re: [PATCH v2 19/21] ppc: qemu: Create a virtual memory mapping of the platform bus

[Vladimir Oltean]

On Fri, Mar 12, 2021 at 09:36:00PM +0800, Bin Meng wrote:
> QEMU ppce500 target can dynamically instantiate an eTSEC device on
> a platform bus if "-device eTSEC" is given to QEMU. It is presented
> as a "simple-bus" in the device tree, with an additional compatible
> string "qemu,platform".
> 
> Let's create a virtual memory mapping for it in misc_init_r(), in
> preparation to adding eTSEC support.
> 
> Signed-off-by: Bin Meng 
> 
> ---
> 
> Changes in v2:
> - turn on CONFIG_SIMPLE_BUS_CORRECT_RANGE in qemu-ppce500_defconfig
> 
>  board/emulation/qemu-ppce500/Kconfig|  6 ++
>  board/emulation/qemu-ppce500/qemu-ppce500.c | 18 ++
>  configs/qemu-ppce500_defconfig  |  1 +
>  3 files changed, 25 insertions(+)
> 
> diff --git a/board/emulation/qemu-ppce500/Kconfig 
> b/board/emulation/qemu-ppce500/Kconfig
> index 4312d986d8..1c5aa18aa9 100644
> --- a/board/emulation/qemu-ppce500/Kconfig
> +++ b/board/emulation/qemu-ppce500/Kconfig
> @@ -9,4 +9,10 @@ config SYS_VENDOR
>  config SYS_CONFIG_NAME
>   default "qemu-ppce500"
>  
> +config PLATFORM_BUS_MAP_ADDR
> + hex
> + default 0xf000
> + help
> +   The QEMU platform bus base mapped address in the virtual memory space.
> +
>  endif
> diff --git a/board/emulation/qemu-ppce500/qemu-ppce500.c 
> b/board/emulation/qemu-ppce500/qemu-ppce500.c
> index daa103c564..0960dd1f97 100644
> --- a/board/emulation/qemu-ppce500/qemu-ppce500.c
> +++ b/board/emulation/qemu-ppce500/qemu-ppce500.c
> @@ -14,6 +14,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -148,6 +150,22 @@ int misc_init_r(void)
>*/
>   disable_tlb(find_tlb_idx((void *)CONFIG_SYS_TMPVIRT, 1));
>  
> + /*
> +  * Detect the presence of the platform bus node, and
> +  * create a virtual memory mapping for it.
> +  */
> + for (ret = uclass_find_first_device(UCLASS_SIMPLE_BUS, &dev);
> +  dev;
> +  ret = uclass_find_next_device(&dev)) {
> + if (device_is_compatible(dev, "qemu,platform")) {
> + struct simple_bus_plat *plat = dev_get_uclass_plat(dev);
> +
> + assert(!tlb_map_range(CONFIG_PLATFORM_BUS_MAP_ADDR,
> +   plat->target, plat->size,
> +   TLB_MAP_IO));

A break here, maybe?
If there are multiple qemu,platform nodes, your code will attempt to
create multiple TLB mappings towards the same virtual address
CONFIG_PLATFORM_BUS_MAP_ADDR, which will not work. So better avoid that.

And maybe you can refactor this into a dedicated function, similar to
what exists for PCI.

> + }
> + }
> +
>   return 0;
>  }
}

>  
> diff --git a/configs/qemu-ppce500_defconfig b/configs/qemu-ppce500_defconfig
> index 536fe7d6e1..151834b4cf 100644
> --- a/configs/qemu-ppce500_defconfig
> +++ b/configs/qemu-ppce500_defconfig
> @@ -28,6 +28,7 @@ CONFIG_OF_BOARD=y
>  CONFIG_ENV_OVERWRITE=y
>  CONFIG_SYS_RELOC_GD_ENV_ADDR=y
>  CONFIG_DM=y
> +CONFIG_SIMPLE_BUS_CORRECT_RANGE=y
>  CONFIG_BLK=y
>  CONFIG_HAVE_BLOCK_DEVICE=y
>  CONFIG_MPC8XXX_GPIO=y
> -- 
> 2.25.1
> 

Re: [PATCH v2 20/21] ppc: qemu: Enable eTSEC support

[Vladimir Oltean]

On Fri, Mar 12, 2021 at 09:36:01PM +0800, Bin Meng wrote:
> QEMU ppce500 target can dynamically instantiate an eTSEC device
> if "-device eTSEC" is given to QEMU. This commit enables eTSEC
> driver and the required fixed PHY driver to create a usable
> network configuration using eTSEC.
> 
> Unlike a real world 85xx board that usually stores the eTSEC MAC
> address in an EEPROM, CONFIG_NET_RANDOM_ETHADDR is required for
> QEMU otherwise U-Boot ethernet initialization complains no valid
> ethernet address is set.
> 
> Signed-off-by: Bin Meng 
> ---

Reviewed-by: Vladimir Oltean 

Re: [PATCH v2 21/21] doc: board: qemu-ppce500: Document eTSEC usage

[Vladimir Oltean]

On Fri, Mar 12, 2021 at 09:36:02PM +0800, Bin Meng wrote:
> Document how to launch a QEMU session with eTSEC as a network device.
> 
> Signed-off-by: Bin Meng 
> 
> ---

Reviewed-by: Vladimir Oltean 

Re: [PATCH v2 02/21] of: extra: Introduce ofnode_phy_is_fixed_link() API

[Bin Meng]

Hi Vladimir,

On Sat, Mar 13, 2021 at 9:03 PM Vladimir Oltean  wrote:
>
> On Fri, Mar 12, 2021 at 09:35:43PM +0800, Bin Meng wrote:
> > +bool ofnode_phy_is_fixed_link(ofnode eth_node, ofnode *phy_node)
> > +{
> > + bool found = false;
> > + ofnode node, subnode;
> > + int len;
> > +
> > + /* new binding */
> > + subnode = ofnode_find_subnode(eth_node, "fixed-link");
> > + if (ofnode_valid(subnode)) {
> > + node = subnode;
> > + found = true;
> > + }
> > +
> > + /* old binding */
> > + if (ofnode_get_property(eth_node, "fixed-link", &len) &&
>
> Maybe "else if" here? If an old-style and a new-style binding exist, we
> should prefer looking at the new one.
>

Agree. Will do in v3.

> And with that "else if", we could remove the "found" variable:

Regards,
Bin

Re: [PATCH v2 02/21] of: extra: Introduce ofnode_phy_is_fixed_link() API

[Bin Meng]

Hi Vladimir,

On Sat, Mar 13, 2021 at 8:29 PM Vladimir Oltean  wrote:
>
> On Sat, Mar 13, 2021 at 02:14:36PM +0200, Vladimir Oltean wrote:
> > On Fri, Mar 12, 2021 at 09:35:43PM +0800, Bin Meng wrote:
> > > Introduce a helper API ofnode_phy_is_fixed_link() to detect whether
> > > the ethernet controller connects to a fixed-link pseudo-PHY device.
> > >
> > > Note there are two ways to describe a fixed PHY attached to an
> > > Ethernet device:
> > >
> > > - the new DT binding, where 'fixed-link' is a sub-node of the
> > >   Ethernet device
> > > - the old DT binding, where 'fixed-link' is a property with 5
> > >   cells encoding various information about the fixed PHY
> > >
> > > Signed-off-by: Bin Meng 
> > > Reviewed-by: Simon Glass 
> > > ---
> >
> > I ran a 'grep -r "ofnode_get_property.*fixed-link" .' and saw no in-tree
> > users of the old binding. Why do we bother to be compatible with
> > something which isn't used?
>
> Ah, I see what's going on.
> QEMU fixes up the device tree here:
> https://github.com/qemu/qemu/blob/master/hw/ppc/e500.c#L239
> and adds an old-style fixed-link binding.
> Can't you modify it to add a new-style fixed-link property?

I am afraid that may break guests that haven't supported new-style DT
bindings yet.

> It's not like you didn't have to modify it for the "ranges" property too :)
> https://github.com/qemu/qemu/commit/e5943b00d35efc68ca72ed304cfca98a9f3a647c

Regards,
Bin

Re: [PATCH u-boot v2 32/38] ARM: omap3: fix LTO for DM3730 (and possibly other omap3 boards)

[Adam Ford]

On Fri, Mar 12, 2021 at 7:43 AM Adam Ford  wrote:
>
> On Fri, Mar 12, 2021 at 4:35 AM Marek Behún  wrote:
> >
> > Adam Ford says that DM3730 needs board.c compiled without LTO flags.
> >
> > Also add clock.c, since it says in Makefile that it need different
> > flags.
> >
>
> Tested-by: Adam Ford  #omap3_logic
>
> > Signed-off-by: Marek Behún 
> > Suggested-by: Adam Ford 
> > ---
> >  arch/arm/mach-omap2/omap3/Makefile | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/arm/mach-omap2/omap3/Makefile 
> > b/arch/arm/mach-omap2/omap3/Makefile
> > index 91ed8ebc9f..7d22f04bbf 100644
> > --- a/arch/arm/mach-omap2/omap3/Makefile
> > +++ b/arch/arm/mach-omap2/omap3/Makefile
> > @@ -5,10 +5,12 @@
> >
> >  # If clock.c is compiled for Thumb2, then it fails on OMAP3530
> >  CFLAGS_clock.o += -marm
> > +CFLAGS_REMOVE_clock.o := $(LTO_CFLAGS)
>
> I have two OMAP3530 boards.  I'll test that this weekend.  I think I
> even have them configured for Thumb2

I have tested this on omap35_logic_somlv and the LTO flag removal
isn't necessary for the clock.o
Having the clock built with LTO saves 239 bytes in SPL with my
compiler.  It's not a huge savings, but in SPL, we need as much as
possible.

adam

>
> adam
>
> >
> >  obj-y  := lowlevel_init.o
> >
> >  obj-y  += board.o
> > +CFLAGS_REMOVE_board.o := $(LTO_CFLAGS)
> >  obj-y  += boot.o
> >  obj-y  += clock.o
> >  obj-y  += sys_info.o
> > --
> > 2.26.2
> >

Re: [PATCH] configs: omap3_logic: Enable CONFIG_SPL_ALLOC_BD

[Adam Ford]

On Thu, Mar 4, 2021 at 10:32 AM Adam Ford  wrote:
>
> With bd_info dropped from the data section, the Logic PD OMAP3 boards
> and AM3517 fail to boot. Enabling CONFIG_SPL_ALLOC_BD restores
> them.
>
> Fixes: 38d6b7ebdaee ("spl: Drop bd_info in the data section")
> Signed-off-by: Adam Ford 
>

With the pending 2021.04 release coming soon, we need this patch
applied or the boards won't boot.  Any chance they can be applied
before the release?

thanks

adam

> diff --git a/configs/am3517_evm_defconfig b/configs/am3517_evm_defconfig
> index bae0e0af35..0bc7bf74f9 100644
> --- a/configs/am3517_evm_defconfig
> +++ b/configs/am3517_evm_defconfig
> @@ -15,6 +15,7 @@ CONFIG_DEFAULT_DEVICE_TREE="am3517-evm"
>  CONFIG_DISTRO_DEFAULTS=y
>  CONFIG_BOOTDELAY=10
>  # CONFIG_USE_BOOTCOMMAND is not set
> +CONFIG_SPL_ALLOC_BD=y
>  CONFIG_SPL_SYS_MALLOC_SIMPLE=y
>  CONFIG_SPL_SEPARATE_BSS=y
>  # CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR is not set
> diff --git a/configs/omap35_logic_defconfig b/configs/omap35_logic_defconfig
> index ec0ba110b8..fa751e3bf0 100644
> --- a/configs/omap35_logic_defconfig
> +++ b/configs/omap35_logic_defconfig
> @@ -19,6 +19,7 @@ CONFIG_USE_PREBOOT=y
>  CONFIG_PREBOOT="setenv preboot;saveenv;"
>  CONFIG_DEFAULT_FDT_FILE="logicpd-torpedo-35xx-devkit.dtb"
>  CONFIG_SYS_CONSOLE_INFO_QUIET=y
> +CONFIG_SPL_ALLOC_BD=y
>  CONFIG_SPL_SYS_MALLOC_SIMPLE=y
>  CONFIG_SPL_SEPARATE_BSS=y
>  # CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR is not set
> diff --git a/configs/omap35_logic_somlv_defconfig 
> b/configs/omap35_logic_somlv_defconfig
> index 3c9f0027bc..e6655c7828 100644
> --- a/configs/omap35_logic_somlv_defconfig
> +++ b/configs/omap35_logic_somlv_defconfig
> @@ -18,6 +18,7 @@ CONFIG_USE_PREBOOT=y
>  CONFIG_PREBOOT="setenv preboot;saveenv;"
>  CONFIG_DEFAULT_FDT_FILE="logicpd-som-lv-35xx-devkit.dtb"
>  CONFIG_SYS_CONSOLE_INFO_QUIET=y
> +CONFIG_SPL_ALLOC_BD=y
>  CONFIG_SPL_SYS_MALLOC_SIMPLE=y
>  CONFIG_SPL_SEPARATE_BSS=y
>  # CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR is not set
> diff --git a/configs/omap3_logic_defconfig b/configs/omap3_logic_defconfig
> index 4e37237b86..5414880baa 100644
> --- a/configs/omap3_logic_defconfig
> +++ b/configs/omap3_logic_defconfig
> @@ -18,6 +18,7 @@ CONFIG_ANDROID_BOOT_IMAGE=y
>  CONFIG_USE_PREBOOT=y
>  CONFIG_PREBOOT="setenv preboot;saveenv;"
>  CONFIG_SYS_CONSOLE_INFO_QUIET=y
> +CONFIG_SPL_ALLOC_BD=y
>  CONFIG_SPL_SYS_MALLOC_SIMPLE=y
>  CONFIG_SPL_SEPARATE_BSS=y
>  # CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR is not set
> diff --git a/configs/omap3_logic_somlv_defconfig 
> b/configs/omap3_logic_somlv_defconfig
> index 5947f4bc60..3edd6d126b 100644
> --- a/configs/omap3_logic_somlv_defconfig
> +++ b/configs/omap3_logic_somlv_defconfig
> @@ -19,6 +19,7 @@ CONFIG_USE_PREBOOT=y
>  CONFIG_PREBOOT="setenv preboot;saveenv;"
>  CONFIG_DEFAULT_FDT_FILE="logicpd-som-lv-37xx-devkit.dtb"
>  CONFIG_SYS_CONSOLE_INFO_QUIET=y
> +CONFIG_SPL_ALLOC_BD=y
>  CONFIG_SPL_SYS_MALLOC_SIMPLE=y
>  CONFIG_SPL_SEPARATE_BSS=y
>  # CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR is not set
> --
> 2.25.1
>

[PATCH] configs: omap35_logic_somlv: Fix MMC booting

[Adam Ford]

A previous patch had removed the GPIO nodes from being built
into the SPL Device tree, but CONFIG_SPL_GPIO_SUPPORT remained
which makes the MMC card detect fail and the board does not boot.
Fix this by disabling CONFIG_SPL_GPIO_SUPPORT.

Fixes: 6f1efe81aa84 ("configs: omap3/35_logic and omap3/35_logic_somlv: Reduce 
SPL size")
Signed-off-by: Adam Ford 

diff --git a/configs/omap35_logic_somlv_defconfig 
b/configs/omap35_logic_somlv_defconfig
index e6655c7828..a8eab408f6 100644
--- a/configs/omap35_logic_somlv_defconfig
+++ b/configs/omap35_logic_somlv_defconfig
@@ -4,6 +4,7 @@ CONFIG_ARM=y
 CONFIG_ARCH_OMAP2PLUS=y
 CONFIG_SYS_TEXT_BASE=0x8010
 CONFIG_TI_COMMON_CMD_OPTIONS=y
+# CONFIG_SPL_GPIO_SUPPORT is not set
 CONFIG_SYS_MALLOC_F_LEN=0x4000
 CONFIG_NR_DRAM_BANKS=2
 CONFIG_SPL_TEXT_BASE=0x4020
-- 
2.25.1

Re: [PATCH u-boot v2 32/38] ARM: omap3: fix LTO for DM3730 (and possibly other omap3 boards)

[Marek Behun]

On Sat, 13 Mar 2021 09:23:05 -0600
Adam Ford  wrote:

> I have tested this on omap35_logic_somlv and the LTO flag removal
> isn't necessary for the clock.o
> Having the clock built with LTO saves 239 bytes in SPL with my
> compiler.  It's not a huge savings, but in SPL, we need as much as
> possible.

So I can drop this patch?
Marek

Re: [PATCH u-boot v2 32/38] ARM: omap3: fix LTO for DM3730 (and possibly other omap3 boards)

[Adam Ford]

On Sat, Mar 13, 2021 at 10:05 AM Marek Behun  wrote:
>
> On Sat, 13 Mar 2021 09:23:05 -0600
> Adam Ford  wrote:
>
> > I have tested this on omap35_logic_somlv and the LTO flag removal
> > isn't necessary for the clock.o
> > Having the clock built with LTO saves 239 bytes in SPL with my
> > compiler.  It's not a huge savings, but in SPL, we need as much as
> > possible.
>
> So I can drop this patch?

Drop the LTO stuff for clock.o, but keep the LTO stuff for board.o

adam
> Marek

Re: [PATCH 2/4] pinephone_defconfig: reduce boot delay

[Arnaud Ferraris]

Hi Maxime,

Le 25/02/2021 à 21:58, Maxime Ripard a écrit :
> On Thu, Feb 25, 2021 at 05:02:40PM +, André Przywara wrote:
>> On 20/02/2021 12:14, Nicolas Boulenguez wrote:
>>> From: Arnaud Ferraris 
>>>
>>> On a cellular phone, the vast majority of users can be expected to
>>> have no serial console connected and prefer a short boot.
>>
>> It's a bit tricky to break in with a delay of 0, but indeed most users
>> won't care, so looks fine to me.
>>
>> But it's missing a Signed-off-by: line.
> 
> I'm not sure we should start accepting custom boards changes like it's
> done here, this will only lead to inconsistencies between boards in the
> long run.

The original reasoning behind this was, the PinePhone (and PineTab) is
both a dev board and a consumer device, which is why I assumed it could
use some kind of "special treatment".

However, I don't have a strong opinion on that matter and can certainly
live with the default boot delay.

Regards,
Arnaud

> 
> And a defconfig is really easy to change anyway, even when it's
> integrated in build systems
> 
> Maxime
> 

Re: [PATCH] configs: add PineTab defconfig

[Arnaud Ferraris]

Hi,

Le 08/03/2021 à 01:12, Andre Przywara a écrit :
> On Sun, 7 Mar 2021 13:53:56 +0100
> Nicolas Boulenguez  wrote:
> 
> Hi,
> 
>> From: Arnaud Ferraris 
>>
>> The PineTab device-tree is already in u-boot, this commit adds the 
>> corresponding
>> defconfig, based on pinephone_defconfig.
>>
>> Signed-off-by: Arnaud Ferraris 
>>
>> --- a/board/sunxi/MAINTAINERS
>> +++ b/board/sunxi/MAINTAINERS
>> @@ -471,6 +471,11 @@ M:  Samuel Holland 
>>  S:  Maintained
>>  F:  configs/pinephone_defconfig
>>  
>> +PINETAB BOARD
>> +M:  Arnaud Ferraris 
>> +S:  Maintained
>> +F:  configs/pinetab_defconfig
> 
> Arnaud, do you agree with this?
> Happy to take your patch via Nicolas, but for the maintainer entry I
> would like to have some confirmation.

Yes, I'm perfectly fine with this.

> 
>> +
>>  R16 EVB PARROT BOARD
>>  M:  Quentin Schulz 
>>  S:  Maintained
>> --- /dev/null
>> +++ b/configs/pinetab_defconfig
>> @@ -0,0 +1,21 @@
>> +CONFIG_ARM=y
>> +CONFIG_ARCH_SUNXI=y
>> +CONFIG_SPL=y
>> +CONFIG_IDENT_STRING=""
> 
> Having "Allwinner Technology" here is indeed weird and probably not
> really justified anymore, given the "support" we see from Allwinner.
> I wonder if we should scrap this for all boards. Also it makes the line
> longer than 80 characters.
> 
>> +CONFIG_MACH_SUN50I=y
>> +CONFIG_SUNXI_DRAM_LPDDR3_STOCK=y
>> +CONFIG_DRAM_CLK=552
>> +CONFIG_DRAM_ZQ=3881949
>> +CONFIG_MMC_SUNXI_SLOT_EXTRA=2
>> +# CONFIG_VIDEO_DE2 is not set
>> +CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-pinetab"
>> +# CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
>> +CONFIG_BOOTDELAY=0
> 
> I answered in the other email about the boot delay already.
> 
> So what is the reason for all those other options below?
> Is there any particular reason they were all disabled?
> I can buy CONFIG_NET, but the rest seems unnecessary. There doesn't
> seem to be a driver for the PineTab panel in U-Boot, so this is solely
> suppressing a few lines on the serial? Since this would be surely for
> debug only, I think it's useful to have them, normal users wouldn't see
> them anyway.

I initially created this defconfig from a downstream pinephone_defconfig
IIRC, I guess I just carried those over without thinking too much about
it. I'll improve the defconfig and post an improved version soon.

Cheers,
Arnaud

> 
>> +CONFIG_SYS_CONSOLE_INFO_QUIET=y
>> +# CONFIG_DISPLAY_CPUINFO is not set
>> +# CONFIG_DISPLAY_BOARDINFO is not set
>> +# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
>> +# CONFIG_SPL_BANNER_PRINT is not set
>> +# CONFIG_SPL_POWER_SUPPORT is not set
>> +# CONFIG_NET is not set
>> +# CONFIG_EFI_LOADER is not set
> 
> We should definitely keep EFI_LOADER.
> 
> Cheers,
> Andre
> 

Re: [RFC] dev_phys_to_bus() and PCI

[Simon Glass]

+Tom Rini

Hi Mark,

On Sat, 13 Mar 2021 at 02:24, Mark Kettenis  wrote:
>
> I'm working on support for Apple's M1 systems in U-Boot.  The idea is
> that this can be used as a "payload" for the m1n1 bootloader that is
> being developed by Hector Martin for Asahi Linux in order to provide
> an UEFI implementation that can boot a standard arm64 OS.
>
> My current code, which can be found on the "apple-m1-m1n1" branch at:
>
>   https://github.com/kettenis/u-boot/tree/apple-m1-m1n1
>
> can already do this.  I'm booting OpenBSD/arm64 this way and I've also
> booted into grub using a standard arm64 Debian installer.
>
> One of the big challenges I'm facing is that the integrated PCIe
> devices on the system sit behind an IOMMU that (as far as we can tell)
> doesn't support any kind of bypass mode.  I don't think we want full
> IOMMU support in U-Boot, so the approach I'm currently taking is that
> I set up the IOMMU to map a chunk of memory at the "top of RAM" where
> U-Boot resides after relocation.  But in order to use this mapping I
> need to do DMA address translation.
>
> Fortunately Nicolas Saenz Julienne recently introduced
> dev_phys_to_bus() and dev_bus_to_phys() interfaces to do this.  Those
> interfaces make use of a dma-ranges property in the device tree which
> doesn't work so well for PCI devices though.  However, the PCI code in
> U-Boot already has a way to describe DMA address translation through
> its regions support.  Hooking this up to dev_phys_to_bus() and
> dev_bus_to_phys() is fairly easy as illustrated by the diff below.
>
> Would this be viable approach?  This could also help adding support
> for PCIe devices on the Raspberry Pi CM4.

It seems OK to me.

I suspect IOMMU support will be needed to some degree eventually in U-Boot.

Regards,
Simon


>
>
> commit 4f0e989c7a765291a38b7d10da562f23c5f31239
> Author: Mark Kettenis 
> Date:   Fri Mar 12 20:23:11 2021 +0100
>
> dm: Add PCI support to dev_phys_to_bus()/dev_bus_to_phys()
>
> For PCI devices, call dm_pci_phys_to_bus()/dm_pci_bus_to_phys()
> to do the address translation.  This uses the regions set up
> by the PCI host controller driver to do the translation as a
> single translation offset may not be sufficient in this case.
>
> Signed-off-by: Mark Kettenis 
>
> diff --git a/include/phys2bus.h b/include/phys2bus.h
> index 866b8b51a8..13d23ef4bb 100644
> --- a/include/phys2bus.h
> +++ b/include/phys2bus.h
> @@ -23,14 +23,21 @@ static inline unsigned long bus_to_phys(unsigned long bus)
>
>  #if CONFIG_IS_ENABLED(DM)
>  #include 
> +#include 
>
>  static inline dma_addr_t dev_phys_to_bus(struct udevice *dev, phys_addr_t 
> phys)
>  {
> +   if (device_is_on_pci_bus(dev))
> +   return dm_pci_phys_to_bus(dev, phys, PCI_REGION_SYS_MEMORY);
> +
> return phys - dev_get_dma_offset(dev);
>  }
>
>  static inline phys_addr_t dev_bus_to_phys(struct udevice *dev, dma_addr_t 
> bus)
>  {
> +   if (device_is_on_pci_bus(dev))
> +   return dm_pci_bus_to_phys(dev, bus, PCI_REGION_SYS_MEMORY);
> +
> return bus + dev_get_dma_offset(dev);
>  }
>  #else

[PATCH 0/6 v2] Loadfile2 for initrd loading

[Ilias Apalodimas]

Hi!
This is v2 of [1]

Changes since v1:
 - minor coding style fixes from Heinrich
 - changed the DP format. Instead of VenMedia - 0x01 - initrd, we skip
   the 0x01 between VenMedia and the first file. 
 - final device path is stripped in efi_get_dp_from_boot() instead of 
   get_initrd_fp()
 - Fixed comments on documentation

[1] https://lists.denx.de/pipermail/u-boot/2021-March/443399.html

Ilias Apalodimas (6):
  efi_selftest: Remove loadfile2 for initrd selftests
  efi_loader: Add device path related functions for initrd via Boot
  efi_loader: Introduce helper functions for EFI
  efi_loader: Replace config option for initrd loading
  efidebug: add multiple device path instances on Boot
  doc: Update uefi documentation for initrd loading options

 cmd/bootefi.c |   3 +
 cmd/efidebug.c| 194 ---
 doc/board/emulation/qemu_capsule_update.rst   |   4 +-
 doc/uefi/uefi.rst |  23 +-
 include/efi_helper.h  |  15 ++
 include/efi_loader.h  |   7 +
 lib/efi_loader/Kconfig|  15 +-
 lib/efi_loader/Makefile   |   1 +
 lib/efi_loader/efi_bootmgr.c  |  19 +-
 lib/efi_loader/efi_device_path.c  |  99 +++-
 lib/efi_loader/efi_helper.c   | 133 +++
 lib/efi_loader/efi_load_initrd.c  | 189 +--
 lib/efi_loader/efi_var_common.c   |  33 +++
 lib/efi_selftest/Makefile |   1 -
 lib/efi_selftest/efi_selftest_load_initrd.c   | 221 --
 .../test_efi_capsule/test_capsule_firmware.py |   6 +-
 test/py/tests/test_efi_secboot/test_signed.py |  16 +-
 .../test_efi_secboot/test_signed_intca.py |   8 +-
 .../tests/test_efi_secboot/test_unsigned.py   |   8 +-
 19 files changed, 618 insertions(+), 377 deletions(-)
 create mode 100644 include/efi_helper.h
 create mode 100644 lib/efi_loader/efi_helper.c
 delete mode 100644 lib/efi_selftest/efi_selftest_load_initrd.c

-- 
2.30.1

[PATCH 1/6 v2] efi_selftest: Remove loadfile2 for initrd selftests

[Ilias Apalodimas]

We are redefining how u-boot locates the initrd to load via the kernel
LoadFile2 protocol.  This selftest is not relevant any more, so remove
it. A new one will be added later

Signed-off-by: Ilias Apalodimas 
Reviewed-by: Heinrich Schuchardt 
---
 lib/efi_selftest/Makefile   |   1 -
 lib/efi_selftest/efi_selftest_load_initrd.c | 221 
 2 files changed, 222 deletions(-)
 delete mode 100644 lib/efi_selftest/efi_selftest_load_initrd.c

diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
index b02fd56e0a79..50de581b7763 100644
--- a/lib/efi_selftest/Makefile
+++ b/lib/efi_selftest/Makefile
@@ -61,7 +61,6 @@ obj-$(CONFIG_CPU_V7) += efi_selftest_unaligned.o
 obj-$(CONFIG_EFI_LOADER_HII) += efi_selftest_hii.o
 obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_selftest_rng.o
 obj-$(CONFIG_EFI_GET_TIME) += efi_selftest_rtc.o
-obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_selftest_load_initrd.o
 obj-$(CONFIG_EFI_TCG2_PROTOCOL) += efi_selftest_tcg2.o
 
 ifeq ($(CONFIG_GENERATE_ACPI_TABLE),)
diff --git a/lib/efi_selftest/efi_selftest_load_initrd.c 
b/lib/efi_selftest/efi_selftest_load_initrd.c
deleted file mode 100644
index f591dcd2115e..
--- a/lib/efi_selftest/efi_selftest_load_initrd.c
+++ /dev/null
@@ -1,221 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * efi_selftest_load_initrd
- *
- * Copyright (c) 2020 Ilias Apalodimas 
- *
- * This test checks the FileLoad2 protocol.
- * A known file is read from the file system and verified.
- *
- * An example usage - given a file image with a file system in partition 1
- * holding file initrd - is:
- *
- * * Configure the sandbox with
- *
- *   CONFIG_EFI_SELFTEST=y
- *   CONFIG_EFI_LOAD_FILE2_INITRD=y
- *   CONFIG_EFI_INITRD_FILESPEC="host 0:1 initrd"
- *
- * * Run ./u-boot and execute
- *
- *   host bind 0 image
- *   setenv efi_selftest load initrd
- *   bootefi selftest
- *
- * This would provide a test output like:
- *
- *   Testing EFI API implementation
- *
- *   Selected test: 'load initrd'
- *
- *   Setting up 'load initrd'
- *   Setting up 'load initrd' succeeded
- *
- *   Executing 'load initrd'
- *   Loaded 12378613 bytes
- *   CRC32 2997478465
- *
- * Now the size and CRC32 can be compared to the provided file.
- */
-
-#include 
-#include 
-#include 
-
-static struct efi_boot_services *boottime;
-
-static struct efi_initrd_dp dp = {
-   .vendor = {
-   {
-  DEVICE_PATH_TYPE_MEDIA_DEVICE,
-  DEVICE_PATH_SUB_TYPE_VENDOR_PATH,
-  sizeof(dp.vendor),
-   },
-   EFI_INITRD_MEDIA_GUID,
-   },
-   .end = {
-   DEVICE_PATH_TYPE_END,
-   DEVICE_PATH_SUB_TYPE_END,
-   sizeof(dp.end),
-   }
-};
-
-static struct efi_initrd_dp dp_invalid = {
-   .vendor = {
-   {
-  DEVICE_PATH_TYPE_MEDIA_DEVICE,
-  DEVICE_PATH_SUB_TYPE_VENDOR_PATH,
-  sizeof(dp.vendor),
-   },
-   EFI_INITRD_MEDIA_GUID,
-   },
-   .end = {
-   0x8f, /* invalid */
-   0xfe, /* invalid */
-   sizeof(dp.end),
-   }
-};
-
-static int setup(const efi_handle_t handle,
-const struct efi_system_table *systable)
-{
-   boottime = systable->boottime;
-
-   return EFI_ST_SUCCESS;
-}
-
-static int execute(void)
-{
-   struct efi_load_file_protocol *lf2;
-   struct efi_device_path *dp2, *dp2_invalid;
-   efi_status_t status;
-   efi_handle_t handle;
-   char buffer[64];
-   efi_uintn_t buffer_size;
-   void *buf;
-   u32 crc32;
-
-   memset(buffer, 0, sizeof(buffer));
-
-   dp2 = (struct efi_device_path *)&dp;
-   status = boottime->locate_device_path(&efi_guid_load_file2_protocol,
- &dp2, &handle);
-   if (status != EFI_SUCCESS) {
-   efi_st_error("Unable to locate device path\n");
-   return EFI_ST_FAILURE;
-   }
-
-   status = boottime->handle_protocol(handle,
-  &efi_guid_load_file2_protocol,
-  (void **)&lf2);
-   if (status != EFI_SUCCESS) {
-   efi_st_error("Unable to locate protocol\n");
-   return EFI_ST_FAILURE;
-   }
-
-   /* Case 1:
-* buffer_size can't be NULL
-* protocol can't be NULL
-*/
-   status = lf2->load_file(lf2, dp2, false, NULL, &buffer);
-   if (status != EFI_INVALID_PARAMETER) {
-   efi_st_error("Buffer size can't be NULL\n");
-   return EFI_ST_FAILURE;
-   }
-   buffer_size = sizeof(buffer);
-   status = lf2->load_file(NULL, dp2, false, &buffer_size, &buffer);
-   if (status != EFI_INVALID_PARAMETER) {
-   efi_st_error("Protocol can't be NULL\n");
-   return EFI_ST_FAILURE;
-   }
-
-   /*
-* Case 2: Match 

[PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot####

[Ilias Apalodimas]

On the following patches we allow for an initrd path to be stored in
Boot variables.  Specifically we encode in the FIlePathList[] of
the EFI_LOAD_OPTIONS for each Boot variable.

The FilePathList[] array looks like this:
kernel - 0xff - VenMedia(initrd GUID) - initrd1 - 0x01 initrd2 - 0xff
So let's add the relevant functions to concatenate and retrieve a device
path based on a Vendor GUID.

Signed-off-by: Ilias Apalodimas 
---
 include/efi_loader.h |  4 ++
 lib/efi_loader/efi_device_path.c | 99 ++--
 2 files changed, 98 insertions(+), 5 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index f470bbd636f4..eb11a8c7d4b1 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -738,6 +738,10 @@ struct efi_load_option {
const u8 *optional_data;
 };
 
+struct efi_device_path *efi_dp_from_lo(struct efi_load_option *lo,
+  efi_uintn_t *size, efi_guid_t guid);
+struct efi_device_path *efi_dp_concat(const struct efi_device_path *dp1,
+ const struct efi_device_path *dp2);
 efi_status_t efi_deserialize_load_option(struct efi_load_option *lo, u8 *data,
 efi_uintn_t *size);
 unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data);
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index c9315dd45857..1f55c772dc83 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -282,11 +282,25 @@ struct efi_device_path *efi_dp_dup(const struct 
efi_device_path *dp)
return ndp;
 }
 
-struct efi_device_path *efi_dp_append(const struct efi_device_path *dp1,
- const struct efi_device_path *dp2)
+/** _efi_dp_append() - Append or concatenate two device paths.
+ * Concatenated device path will be separated by a 0xff end
+ * node.
+ *
+ * @dp1:   First device path
+ * @dp2:   Second device path
+ *
+ * Return: concatenated device path or NULL. Caller must free the returned
+ *  value
+ */
+static struct efi_device_path *_efi_dp_append(const struct efi_device_path 
*dp1,
+ const struct efi_device_path *dp2,
+ bool concat)
 {
struct efi_device_path *ret;
+   size_t end_size = sizeof(END);
 
+   if (concat)
+   end_size = 2 * sizeof(END);
if (!dp1 && !dp2) {
/* return an end node */
ret = efi_dp_dup(&END);
@@ -298,18 +312,56 @@ struct efi_device_path *efi_dp_append(const struct 
efi_device_path *dp1,
/* both dp1 and dp2 are non-null */
unsigned sz1 = efi_dp_size(dp1);
unsigned sz2 = efi_dp_size(dp2);
-   void *p = dp_alloc(sz1 + sz2 + sizeof(END));
+   void *p = dp_alloc(sz1 + sz2 + end_size);
if (!p)
return NULL;
+   ret = p;
memcpy(p, dp1, sz1);
+   p += sz1;
+
+   if (concat) {
+   memcpy(p, &END, sizeof(END));
+   p += sizeof(END);
+   }
+
/* the end node of the second device path has to be retained */
-   memcpy(p + sz1, dp2, sz2 + sizeof(END));
-   ret = p;
+   memcpy(p, dp2, sz2);
+   p += sz2;
+   memcpy(p, &END, sizeof(END));
}
 
return ret;
 }
 
+/** efi_dp_append() - Append a device to an existing device path.
+ *
+ * @dp1:   First device path
+ * @dp2:   Second device path
+ *
+ * Return: concatenated device path or NULL. Caller must free the returned
+ *  value
+ */
+struct efi_device_path *efi_dp_append(const struct efi_device_path *dp1,
+ const struct efi_device_path *dp2)
+{
+   return _efi_dp_append(dp1, dp2, false);
+}
+
+/** efi_dp_concat() - Concatenate 2 device paths. The final device path will
+ *contain two device paths separated by and end node 
(0xff).
+ *
+ * @dp1:   First device path
+ * @dp2:   Second device path
+ *
+ * Return: concatenated device path or NULL. Caller must free the returned
+ *  value
+ */
+struct efi_device_path *efi_dp_concat(const struct efi_device_path *dp1,
+ const struct efi_device_path *dp2)
+{
+   return _efi_dp_append(dp1, dp2, true);
+}
+
 struct efi_device_path *efi_dp_append_node(const struct efi_device_path *dp,
   const struct efi_device_path *node)
 {
@@ -1160,3 +1212,40 @@ ssize_t efi_dp_check_length(const struct efi_device_path 
*dp,
dp = (const struct efi_device_path *)((const u8 *)dp + len);
}
 }
+
+/**
+ * efi_dp_from_lo() - Get 

[PATCH 3/6 v2] efi_loader: Introduce helper functions for EFI

[Ilias Apalodimas]

A following patch introduces a different logic for loading initrd's
based on the EFI_LOAD_FILE2_PROTOCOL.
Since similar logic can be applied in the future for other system files
(i.e DTBs), let's add some helper functions which will retrieve and
parse file paths stored in EFI variables.

Signed-off-by: Ilias Apalodimas 
---
 include/efi_helper.h|  15 
 include/efi_loader.h|   2 +
 lib/efi_loader/Makefile |   1 +
 lib/efi_loader/efi_helper.c | 133 
 lib/efi_loader/efi_var_common.c |  33 
 5 files changed, 184 insertions(+)
 create mode 100644 include/efi_helper.h
 create mode 100644 lib/efi_loader/efi_helper.c

diff --git a/include/efi_helper.h b/include/efi_helper.h
new file mode 100644
index ..97492c65b6d2
--- /dev/null
+++ b/include/efi_helper.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2020, Linaro Limited
+ */
+
+#if !defined _EFI_HELPER_H_
+#define _EFI_HELPER_H
+
+#include 
+#include 
+
+efi_status_t efi_get_file_size(struct efi_file_handle *fh, efi_uintn_t *size);
+struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid);
+
+#endif
diff --git a/include/efi_loader.h b/include/efi_loader.h
index eb11a8c7d4b1..aa812a9a3052 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -717,6 +717,8 @@ efi_status_t EFIAPI efi_query_variable_info(
u64 *remaining_variable_storage_size,
u64 *maximum_variable_size);
 
+void *efi_get_var(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size);
+
 /*
  * See section 3.1.3 in the v2.7 UEFI spec for more details on
  * the layout of EFI_LOAD_OPTION.  In short it is:
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index 10b42e8847bf..da2741adecfa 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -23,6 +23,7 @@ endif
 obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
 obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o
 obj-y += efi_boottime.o
+obj-y += efi_helper.o
 obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
 obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o
 obj-y += efi_console.o
diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c
new file mode 100644
index ..df5bdc506dbe
--- /dev/null
+++ b/lib/efi_loader/efi_helper.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020, Linaro Limited
+ */
+
+#define LOG_CATEGORY LOGC_EFI
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+ * efi_create_current_boot_var() - Return Boot name were  is replaced 
by
+ *the value of BootCurrent
+ *
+ * @var_name:  variable name
+ * @var_name_size: size of var_name
+ *
+ * Return: Status code
+ */
+static efi_status_t efi_create_current_boot_var(u16 var_name[],
+   size_t var_name_size)
+{
+   efi_uintn_t boot_current_size;
+   efi_status_t ret;
+   u16 boot_current;
+   u16 *pos;
+
+   boot_current_size = sizeof(boot_current);
+   ret = efi_get_variable_int(L"BootCurrent",
+  &efi_global_variable_guid, NULL,
+  &boot_current_size, &boot_current, NULL);
+   if (ret != EFI_SUCCESS)
+   goto out;
+
+   pos = efi_create_indexed_name(var_name, var_name_size, "Boot",
+ boot_current);
+   if (!pos) {
+   ret = EFI_OUT_OF_RESOURCES;
+   goto out;
+   }
+
+out:
+   return ret;
+}
+
+/**
+ * efi_get_dp_from_boot() - Retrieve and return a device path from an EFI
+ * Boot### variable
+ *
+ * @guid:  Vendor guid of the VenMedia DP
+ *
+ * Return: device path or NULL. Caller must free the returned value
+ */
+struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid)
+{
+   struct efi_device_path *file_path = NULL;
+   struct efi_device_path *tmp;
+   struct efi_load_option lo;
+   void *var_value = NULL;
+   efi_uintn_t size;
+   efi_status_t ret;
+   u16 var_name[16];
+
+   ret = efi_create_current_boot_var(var_name, sizeof(var_name));
+   if (ret != EFI_SUCCESS)
+   return NULL;
+
+   var_value = efi_get_var(var_name, &efi_global_variable_guid, &size);
+   if (!var_value)
+   return NULL;
+
+   ret = efi_deserialize_load_option(&lo, var_value, &size);
+   if (ret != EFI_SUCCESS)
+   goto out;
+
+   tmp = efi_dp_from_lo(&lo, &size, guid);
+   if (!tmp)
+   goto out;
+
+   /* efi_dp_dup will just return NULL if efi_dp_next is NULL */
+   file_path = efi_dp_dup(efi_dp_next(tmp));
+
+out:
+   efi_free_pool(tmp);
+   free(var_value);
+
+   return file_path;
+}
+
+/**
+ * efi_get_file_size() - Get the siz

[PATCH 4/6 v2] efi_loader: Replace config option for initrd loading

[Ilias Apalodimas]

Up to now we install EFI_LOAD_FILE2_PROTOCOL to load an initrd
unconditionally. Although we correctly return various EFI exit codes
depending on the file status (i.e EFI_NO_MEDIA, EFI_NOT_FOUND etc), the
kernel loader, only falls back to the cmdline interpreted initrd if the
protocol is not installed.

This creates a problem for EFI installers, since they won't be able to
load their own initrd and continue the installation. It also makes the
feature hard to use, since we can either have a single initrd or we have
to recompile u-boot if the filename changes.

So let's introduce a different logic that will decouple the initrd
path from the config option we currently have.
When defining a UEFI Boot we can use the filepathlist and store
a file path pointing to our initrd. Specifically the EFI spec describes:

"The first element of the array is a device path that describes the device
and location of the Image for this load option. Other device paths may
optionally exist in the FilePathList, but their usage is OSV specific".
That means we can install a device path to our initrd(s) after the loaded
image looking like this:
VenMedia - initrd1 - end node (0x01) initrd2 - end node (0xff)

When the EFI application is launched through the bootmgr, we'll try to
interpret the extra device path. If that points to a file that exists on
our disk, we'll now install the load_file2 and the efi-stub will be able
to use it.

This opens up another path using U-Boot and defines a new boot flow.
A user will be able to control the kernel/initrd pairs without explicit
cmdline args or GRUB.

Signed-off-by: Ilias Apalodimas 
---
 cmd/bootefi.c|   3 +
 include/efi_loader.h |   1 +
 lib/efi_loader/Kconfig   |  15 +--
 lib/efi_loader/efi_bootmgr.c |  19 +++-
 lib/efi_loader/efi_load_initrd.c | 189 ++-
 5 files changed, 136 insertions(+), 91 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 271b385edea6..cba81ffe75e4 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -358,6 +358,9 @@ static efi_status_t do_bootefi_exec(efi_handle_t handle, 
void *load_options)
 
free(load_options);
 
+   if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD))
+   efi_initrd_deregister();
+
return ret;
 }
 
diff --git a/include/efi_loader.h b/include/efi_loader.h
index aa812a9a3052..9e57eb37ff28 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -431,6 +431,7 @@ efi_status_t efi_net_register(void);
 /* Called by bootefi to make the watchdog available */
 efi_status_t efi_watchdog_register(void);
 efi_status_t efi_initrd_register(void);
+void efi_initrd_deregister(void);
 /* Called by bootefi to make SMBIOS tables available */
 /**
  * efi_acpi_register() - write out ACPI tables
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index e729f727df11..029f0e515f57 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -317,16 +317,11 @@ config EFI_LOAD_FILE2_INITRD
bool "EFI_FILE_LOAD2_PROTOCOL for Linux initial ramdisk"
default n
help
- Expose a EFI_FILE_LOAD2_PROTOCOL that the Linux UEFI stub can
- use to load the initial ramdisk. Once this is enabled using
- initrd= will stop working.
-
-config EFI_INITRD_FILESPEC
-   string "initramfs path"
-   default "host 0:1 initrd"
-   depends on EFI_LOAD_FILE2_INITRD
-   help
- Full path of the initramfs file, e.g. mmc 0:2 initramfs.cpio.gz.
+   Linux v5.7 and later can make use of this option. If the boot 
option
+   selected by the UEFI boot manager specifies an existing file to 
be used
+   as initial RAM disk, a Linux specific Load File2 protocol will 
be
+   installed and Linux 5.7+ will ignore any initrd= 
command line
+   argument.
 
 config EFI_SECURE_BOOT
bool "Enable EFI secure boot support"
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index 25f5cebfdb67..46c8011344b9 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -118,11 +118,13 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t 
*handle,
ret = efi_set_variable_int(L"BootCurrent",
   &efi_global_variable_guid,
   attributes, sizeof(n), &n, false);
-   if (ret != EFI_SUCCESS) {
-   if (EFI_CALL(efi_unload_image(*handle))
-   != EFI_SUCCESS)
-   log_err("Unloading image failed\n");
-   goto error;
+   if (ret != EFI_SUCCESS)
+   goto unload;
+   /* try to register load file2 for initrd's */
+   if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
+   ret = efi_initrd_register();
+   if (ret != EFI_SUCCESS)
+  

[PATCH 6/6 v2] doc: Update uefi documentation for initrd loading options

[Ilias Apalodimas]

Document the command line options for efidebug and initrd loading

Signed-off-by: Ilias Apalodimas 
---
 doc/uefi/uefi.rst | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst
index b3494c22e073..76402bc5cfaa 100644
--- a/doc/uefi/uefi.rst
+++ b/doc/uefi/uefi.rst
@@ -180,6 +180,12 @@ Set up boot parameters on your board::
 
 efidebug boot add -b 1 HELLO mmc 0:1 /helloworld.efi.signed ""
 
+Since kernel 5.7 there's an alternative way of loading an initrd using
+LoadFile2 protocol if CONFIG_EFI_LOAD_FILE2_INITRD is enabled.
+The initrd path can be specified with::
+
+efidebug boot add -b ABE0 'kernel' mmc 0:1 Image -i mmc 0:1 initrd
+
 Now your board can run the signed image via the boot manager (see below).
 You can also try this sequence by running Pytest, test_efi_secboot,
 on the sandbox
@@ -484,7 +490,20 @@ The load file 2 protocol can be used by the Linux kernel 
to load the initial
 RAM disk. U-Boot can be configured to provide an implementation with::
 
 EFI_LOAD_FILE2_INITRD=y
-EFI_INITRD_FILESPEC=interface dev:part path_to_initrd
+
+When the options is enabled the user can add the initrd path with the efidebug
+command.
+The Boot option has a FilePathList[] in it's EFI_LOAD_OPTION.
+The first element of the array (FilePathList[0]) is the loded image.
+When an initrd is specified the Device Path for the initrd is denoted by a
+VenMedia node with the EFI_INITRD_MEDIA_GUID. Each entry of the array is
+terminated by the 'end of entire device path' subtype. If the user wants to
+define multiple initrds those must by separated by the 'end of this instance'
+identifier of the end node (0x01).
+
+So our final format of the FilePathList[] is::
+
+Loaded image - end node (0xff) - VenMedia - initrd1 - end node (0x01) 
initrd2 - end node (0xff)
 
 Links
 -
-- 
2.30.1

[PATCH 5/6 v2] efidebug: add multiple device path instances on Boot####

[Ilias Apalodimas]

The UEFI spec allow a packed array of UEFI device paths in the
FilePathList[] of an EFI_LOAD_OPTION. The first file path must
describe the loaded image but the rest are OS specific.

Previous patches parse the device path and try to use the second
member of the array as an initrd. So let's modify efidebug slightly
and install the second file described in the command line as the
initrd device path.

Signed-off-by: Ilias Apalodimas 
---
 cmd/efidebug.c| 194 ++
 doc/board/emulation/qemu_capsule_update.rst   |   4 +-
 doc/uefi/uefi.rst |   2 +-
 .../test_efi_capsule/test_capsule_firmware.py |   6 +-
 test/py/tests/test_efi_secboot/test_signed.py |  16 +-
 .../test_efi_secboot/test_signed_intca.py |   8 +-
 .../tests/test_efi_secboot/test_unsigned.py   |   8 +-
 7 files changed, 180 insertions(+), 58 deletions(-)

diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index bbbcb0a54643..223cffa389fb 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -9,6 +9,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -19,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define BS systab.boottime
 #define RT systab.runtime
@@ -794,6 +797,66 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int 
flag,
return CMD_RET_SUCCESS;
 }
 
+/**
+ * add_initrd_instance() - Append a device path to load_options pointing to an
+ *inirtd
+ *
+ * @dev:   Device
+ * @part:  Partition of thge disk
+ * @file:  Filename
+ * @fp:Device Path containing the existing load_options
+ * @fp_size:   New size of the device path after the addition
+ * Return: Pointer to the device path or ERR_PTR
+ *
+ */
+static
+struct efi_device_path *add_initrd_instance(const char *dev, const char *part,
+   const char *file,
+   const struct efi_device_path *fp,
+   efi_uintn_t *fp_size)
+{
+   struct efi_device_path *tmp_dp = NULL, *tmp_fp = NULL;
+   struct efi_device_path *final_fp = NULL, *initrd_dp = NULL;
+   efi_status_t ret;
+   const struct efi_initrd_dp id_dp = {
+   .vendor = {
+   {
+   DEVICE_PATH_TYPE_MEDIA_DEVICE,
+   DEVICE_PATH_SUB_TYPE_VENDOR_PATH,
+   sizeof(id_dp.vendor),
+   },
+   EFI_INITRD_MEDIA_GUID,
+   },
+   .end = {
+   DEVICE_PATH_TYPE_END,
+   DEVICE_PATH_SUB_TYPE_END,
+   sizeof(id_dp.end),
+   }
+   };
+
+   ret = efi_dp_from_name(dev, part, file, &tmp_dp, &tmp_fp);
+   if (ret != EFI_SUCCESS) {
+   printf("Cannot create device path for \"%s %s\"\n", part, file);
+   goto out;
+   }
+
+   initrd_dp = efi_dp_append((const struct efi_device_path *)&id_dp,
+ tmp_fp);
+   if (!initrd_dp) {
+   printf("Cannot append media vendor device path path\n");
+   goto out;
+   }
+   final_fp = efi_dp_concat(fp, initrd_dp);
+   *fp_size = efi_dp_size(fp) + efi_dp_size(initrd_dp) +
+   (2 * sizeof(struct efi_device_path));
+
+out:
+   efi_free_pool(initrd_dp);
+   efi_free_pool(tmp_dp);
+   efi_free_pool(tmp_fp);
+   return final_fp ? final_fp : ERR_PTR(-EINVAL);
+}
+
 /**
  * do_efi_boot_add() - set UEFI load option
  *
@@ -806,7 +869,9 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int 
flag,
  *
  * Implement efidebug "boot add" sub-command. Create or change UEFI load 
option.
  *
- * efidebug boot add[:]  

+ * efidebug boot add -b[:] 
+ *   -i   [:] 
+ *   -s ''
  */
 static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag,
   int argc, char *const argv[])
@@ -819,55 +884,98 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int 
flag,
size_t label_len, label_len16;
u16 *label;
struct efi_device_path *device_path = NULL, *file_path = NULL;
+   struct efi_device_path *final_fp = NULL;
struct efi_load_option lo;
void *data = NULL;
efi_uintn_t size;
+   efi_uintn_t fp_size;
efi_status_t ret;
int r = CMD_RET_SUCCESS;
-
-   if (argc < 6 || argc > 7)
-   return CMD_RET_USAGE;
-
-   id = (int)simple_strtoul(argv[1], &endp, 16);
-   if (*endp != '\0' || id > 0x)
-   return CMD_RET_USAGE;
-
-   sprintf(var_name, "Boot%04X", id);
-   p = var_name16;
-   utf8_utf16_strncpy(&p, var_name, 9);
+   int i;
 
guid = efi_global_variable_guid;
 
/* attributes */
lo.attributes = LOAD_OPTION_ACTIVE; /* always ACTIVE */
+   lo.optional_data

Re: rk3399-gru-kevin: issues on bringup

[Marty E. Plummer]

On Wed, Mar 10, 2021 at 11:52:07PM -0500, Simon Glass wrote:
> Hi,
> 
> On Thu, 13 Aug 2020 at 13:35, Alper Nebi Yasak  
> wrote:
> >
> > Hi Simon, Marty,
> >
> > I'm interested in getting U-Boot to work with Kevin as well, but don't
> > have a Servo (or the willingness to open up the case yet), so I've been
> > trying to boot from depthcharge as in README.chromium-chainload.
> >
> > I don't have a way to see serial output and I see no other signs of
> > life. Can you give me a tested configuration that immediately powers-off
> > or reboots a Kevin so I can confirm what I'm doing works on the
> > chainloading side? I mean I can boot Linux, but trying the same with
> > U-Boot just gives me a blank screen even after accounting for a lot of
> > things.
> >
> > Meanwhile, I've wrote some code to automate making depthcharge partition
> > images, and to enable the display on Kevin (and perhaps Bob). Since I
> > don't know if chainloading works, I don't know if these are broken or
> > not either. I'm unsure about sending untested patches to the list, so I
> > put them up here if you want to take a look (and maybe test/fix them?):
> >
> > https://github.com/alpernebbi/u-boot/tree/rk3399-gru-kevin/wip
> >
> > They're not really things that'd make a non-booting Kevin boot, though.
> > I hope at least some of it can be useful in some way.
> 
> I have the em100 working and have got to the same point as you, Marty.
> 
> em100 -s -c gd25lq64 -d /tmp/b/chromebook_kevin/u-boot.rom -r
> 
> So I suppose that means that SDRAM is running and we just need a SPI
> driver? I will see if I can figure out what is missing...
> 
> Update...it seems to just be missing the ID. I pushed a new patch to:
> 
Christ, its always something small and stupid. Perhaps the failure
message should be amended to indicate 'unknown jedec id: %x' or so to be
a bit more informative.
> https://github.com/sjg20/u-boot/tree/kevin
> 
This looks promising. Built it (away from the machine right now so can't
test) and it seems that u-boot-rockchip.bin is just a bit too large to
be flashed (8.8mb)? Or judging by your above em100 invocation this image
is not to be used? If so, why is it produced at all?
> Now I see:
> 
> Channel 0: LPDDR3, 933MHz
> BW=32 Col=10 Bk=8 CS0 Row=15 CS1 Row=15 CS=2 Die BW=16 Size=2048MB
> Channel 1: LPDDR3, 933MHz
> BW=32 Col=10 Bk=8 CS0 Row=15 CS1 Row=15 CS=2 Die BW=16 Size=2048MB
> 256B stride
> 
> U-Boot SPL 2020.10-rc1-00111-gc31b9b4a3f1-dirty (Mar 10 2021 - 21:48:26 -0700)
> Trying to boot from SPI
> 
> 
> U-Boot 2020.10-rc1-00111-gc31b9b4a3f1-dirty (Mar 10 2021 - 21:48:26 -0700)
> 
> Model: Google Kevin
> DRAM:  3.9 GiB
> Cannot find regulator pwm init_voltage
> MMC:   mmc@fe32: 1, sdhci@fe33: 0
> Loading Environment from MMC... *** Warning - bad CRC, using default 
> environment
> 
> Got rc -1, expected 100
> Failed to probe keyboard 'keyboard-controller'
> In:serial@ff1a
> Out:   serial@ff1a
> Err:   serial@ff1a
> Model: Google Kevin
> Net:   No ethernet found.
> Hit any key to stop autoboot:  0
> =>
> 
> No display and various errors on the way up, but at least it boots to a 
> prompt.
> 
A much better situation then before. I'll pull your changes into my tree
and see what can be done with it.
> Regards,
> Simon

Re: rk3399-gru-kevin: issues on bringup

[Simon Glass]

Hi Marty,

On Sat, 13 Mar 2021 at 12:40, Marty E. Plummer  wrote:
>
> On Wed, Mar 10, 2021 at 11:52:07PM -0500, Simon Glass wrote:
> > Hi,
> >
> > On Thu, 13 Aug 2020 at 13:35, Alper Nebi Yasak  
> > wrote:
> > >
> > > Hi Simon, Marty,
> > >
> > > I'm interested in getting U-Boot to work with Kevin as well, but don't
> > > have a Servo (or the willingness to open up the case yet), so I've been
> > > trying to boot from depthcharge as in README.chromium-chainload.
> > >
> > > I don't have a way to see serial output and I see no other signs of
> > > life. Can you give me a tested configuration that immediately powers-off
> > > or reboots a Kevin so I can confirm what I'm doing works on the
> > > chainloading side? I mean I can boot Linux, but trying the same with
> > > U-Boot just gives me a blank screen even after accounting for a lot of
> > > things.
> > >
> > > Meanwhile, I've wrote some code to automate making depthcharge partition
> > > images, and to enable the display on Kevin (and perhaps Bob). Since I
> > > don't know if chainloading works, I don't know if these are broken or
> > > not either. I'm unsure about sending untested patches to the list, so I
> > > put them up here if you want to take a look (and maybe test/fix them?):
> > >
> > > https://github.com/alpernebbi/u-boot/tree/rk3399-gru-kevin/wip
> > >
> > > They're not really things that'd make a non-booting Kevin boot, though.
> > > I hope at least some of it can be useful in some way.
> >
> > I have the em100 working and have got to the same point as you, Marty.
> >
> > em100 -s -c gd25lq64 -d /tmp/b/chromebook_kevin/u-boot.rom -r
> >
> > So I suppose that means that SDRAM is running and we just need a SPI
> > driver? I will see if I can figure out what is missing...
> >
> > Update...it seems to just be missing the ID. I pushed a new patch to:
> >
> Christ, its always something small and stupid. Perhaps the failure
> message should be amended to indicate 'unknown jedec id: %x' or so to be
> a bit more informative.

It doesn't do that because it is the SPL 'tiny' version.

> > https://github.com/sjg20/u-boot/tree/kevin
> >
> This looks promising. Built it (away from the machine right now so can't
> test) and it seems that u-boot-rockchip.bin is just a bit too large to
> be flashed (8.8mb)? Or judging by your above em100 invocation this image
> is not to be used? If so, why is it produced at all?

I think it is for booting from eMMC. But I am booting from SPI flash.

> > Now I see:
> >
> > Channel 0: LPDDR3, 933MHz
> > BW=32 Col=10 Bk=8 CS0 Row=15 CS1 Row=15 CS=2 Die BW=16 Size=2048MB
> > Channel 1: LPDDR3, 933MHz
> > BW=32 Col=10 Bk=8 CS0 Row=15 CS1 Row=15 CS=2 Die BW=16 Size=2048MB
> > 256B stride
> >
> > U-Boot SPL 2020.10-rc1-00111-gc31b9b4a3f1-dirty (Mar 10 2021 - 21:48:26 
> > -0700)
> > Trying to boot from SPI
> >
> >
> > U-Boot 2020.10-rc1-00111-gc31b9b4a3f1-dirty (Mar 10 2021 - 21:48:26 -0700)
> >
> > Model: Google Kevin
> > DRAM:  3.9 GiB
> > Cannot find regulator pwm init_voltage
> > MMC:   mmc@fe32: 1, sdhci@fe33: 0
> > Loading Environment from MMC... *** Warning - bad CRC, using default 
> > environment
> >
> > Got rc -1, expected 100
> > Failed to probe keyboard 'keyboard-controller'
> > In:serial@ff1a
> > Out:   serial@ff1a
> > Err:   serial@ff1a
> > Model: Google Kevin
> > Net:   No ethernet found.
> > Hit any key to stop autoboot:  0
> > =>
> >
> > No display and various errors on the way up, but at least it boots to a 
> > prompt.
> >
> A much better situation then before. I'll pull your changes into my tree
> and see what can be done with it.

OK. I added a little patch to fix the EC as well

Regards,
Simon

[PATCH v3 00/22] ppc: qemu: Add eTSEC support

[Bin Meng]

QEMU ppce500 machine can dynamically instantiate an eTSEC device
if "-device eTSEC" is given to QEMU.

This series updates the fixed-link ethernet PHY driver as well as
the Freescale eTSEC driver to support the QEMU ppce500 board.

3 patches related to fixed phy in v1 are dropped in v2 as the changes
were done by Vladimir's fixed phy & Sandbox DSA series [1]. Vladimir's
series is now included in v2 to avoid dependencies.

This cover letter is cc'ed to QEMU mailing list for a heads-up.
A future patch will be sent to QEMU mailing list to bring its in-tree
U-Boot source codes up-to-date.

Azure results: PASS
https://dev.azure.com/bmeng/GitHub/_build/results?buildId=342&view=results

This series is avaiable at u-boot-x86/eTSEC for testing.

[1] 
https://patchwork.ozlabs.org/project/uboot/patch/20210216224804.3355044-2-olte...@gmail.com/

Changes in v3:
- mention that U-Boot deliberately ignores the 'phy_id' and
  unconditionally uses PHY_FIXED_ID
- update the code logic to prefer the new binding if both new and
  old bindings exist
- sort variable definitions by line length
- reuse the sandbox dsa nodes for the fixed-link testing
- new patch: net: tsec: Use map_physmem() directly instead of dev_remap_addr()
- add "ranges" in the alternate example
- keep the variable definitions sorted
- invert the strncmp logic to reduce the indentation level
- add a comment to mention only the first "queue-group" is used
- call the same map_physmem() in the common code path
- extract platform bus virtual memory mapping codes to a new routine
- add a "break" in case multiple "qemu,platform" nodes exist

Changes in v2:
- move device tree parsing from xilinxgmiitorgmii_probe() to
  xilinxgmiitorgmii_config() and use OF APIs
- new patch: split from <20210216224804.3355044-4-olte...@gmail.com>
- include 
- use a Kconfig option CONFIG_SIMPLE_BUS_CORRECT_RANGE to control the
  new behavior for boards that want this
- default y if SANDBOX for CONFIG_SIMPLE_BUS_CORRECT_RANGE
- turn on CONFIG_SIMPLE_BUS_CORRECT_RANGE in qemu-ppce500_defconfig

Bin Meng (18):
  dt-bindings: net: Add the old DT bindings for "fixed-link"
  of: extra: Introduce ofnode_phy_is_fixed_link() API
  dm: mdio: Use ofnode_phy_is_fixed_link() API
  net: phy: xilinx: Be compatible with live OF tree
  net: phy: xilinx: Drop #ifdef CONFIG_DM_ETH around
phy_connect_gmii2rgmii()
  net: phy: Simplify the logic of phy_connect_fixed()
  net: phy: fixed: Make driver ops static
  net: phy: fixed: Add the missing ending newline
  net: phy: fixed: Support the old DT binding
  test: dm: Add a case to test ofnode_phy_is_fixed_link()
  net: tsec: Use map_physmem() directly instead of dev_remap_addr()
  dt-bindings: net: Update Freescale TSEC to support "queue-group"
  net: tsec: Support  property from the subnode "queue-group"
  dm: core: Correctly read  of simple-bus
  test: dm: Add a test case for simple-bus 
  ppc: qemu: Create a virtual memory mapping of the platform bus
  ppc: qemu: Enable eTSEC support
  doc: board: qemu-ppce500: Document eTSEC usage

Claudiu Manoil (1):
  sandbox: Add a DSA sandbox driver and unit test

Vladimir Oltean (3):
  net: phy: fixed: Be compatible with live OF tree
  net: phy: fixed: Drop #ifdef CONFIG_DM_ETH around phy_connect_fixed
  net: tsec: Use dm_eth_phy_connect() directly for the DM case

 arch/Kconfig  |   2 +
 arch/sandbox/dts/test.dts |  44 +
 board/emulation/qemu-ppce500/Kconfig  |   6 +
 board/emulation/qemu-ppce500/qemu-ppce500.c   |  31 +++
 configs/qemu-ppce500_defconfig|   5 +
 doc/board/emulation/qemu-ppce500.rst  |   5 +
 doc/device-tree-bindings/net/fixed-link.txt   |  48 +++--
 doc/device-tree-bindings/net/fsl-tsec-phy.txt |  16 +-
 drivers/core/Kconfig  |  14 ++
 drivers/core/of_extra.c   |  23 +++
 drivers/core/simple-bus.c |  32 +++-
 drivers/net/Kconfig   |   9 +
 drivers/net/Makefile  |   1 +
 drivers/net/dsa_sandbox.c | 179 ++
 drivers/net/phy/Kconfig   |   1 +
 drivers/net/phy/fixed.c   |  54 --
 drivers/net/phy/phy.c |  63 +++---
 drivers/net/phy/xilinx_gmii2rgmii.c   |  61 +++---
 drivers/net/tsec.c|  36 +++-
 include/configs/sandbox.h |   2 +
 include/dm/of_extra.h |  20 ++
 include/dm/simple_bus.h   |   6 +-
 net/mdio-uclass.c |   7 +-
 test/dm/Makefile  |   2 +
 test/dm/dsa.c |  82 
 test/dm/eth.c |  10 +-
 test/dm/of_extra.c|  18 ++
 test/dm/simple-bus.c  |  33 
 28 files changed, 690 insertions(+), 120 deletions(-)
 create mode 1006

[PATCH v3 01/22] dt-bindings: net: Add the old DT bindings for "fixed-link"

[Bin Meng]

Per the upstream Linux kernel doc:

  Documentation/devicetree/bindings/net/ethernet-controller.yaml

There are two ways to describe a fixed PHY attached to an Ethernet
device. This updates our dt-bindings doc to add the old DT bindings.

Signed-off-by: Bin Meng 
Reviewed-by: Ramon Fried 
Reviewed-by: Vladimir Oltean 

---

Changes in v3:
- mention that U-Boot deliberately ignores the 'phy_id' and
  unconditionally uses PHY_FIXED_ID

 doc/device-tree-bindings/net/fixed-link.txt | 48 +++--
 1 file changed, 36 insertions(+), 12 deletions(-)

diff --git a/doc/device-tree-bindings/net/fixed-link.txt 
b/doc/device-tree-bindings/net/fixed-link.txt
index 5829bd81a2..5efeeb6fc5 100644
--- a/doc/device-tree-bindings/net/fixed-link.txt
+++ b/doc/device-tree-bindings/net/fixed-link.txt
@@ -5,21 +5,37 @@ Some Ethernet MACs have a "fixed link", and are not connected 
to a
 normal MDIO-managed PHY device. For those situations, a Device Tree
 binding allows to describe a "fixed link".
 
-Such a fixed link situation is described by creating a 'fixed-link'
-sub-node of the Ethernet MAC device node, with the following
-properties:
-
-* 'speed' (integer, mandatory), to indicate the link speed. Accepted
-  values are 10, 100 and 1000
-* 'full-duplex' (boolean, optional), to indicate that full duplex is
-  used. When absent, half duplex is assumed.
-* 'pause' (boolean, optional), to indicate that pause should be
-  enabled.
-* 'asym-pause' (boolean, optional), to indicate that asym_pause should
-  be enabled.
+Note there are two ways to describe a fixed PHY attached to an
+Ethernet device:
+
+- The new DT binding, where 'fixed-link' is a sub-node of the Ethernet
+  MAC device node, with the following properties:
+
+  * 'speed' (integer, mandatory), to indicate the link speed. Accepted
+values are 10, 100 and 1000
+  * 'full-duplex' (boolean, optional), to indicate that full duplex is
+used. When absent, half duplex is assumed.
+  * 'pause' (boolean, optional), to indicate that pause should be
+enabled.
+  * 'asym-pause' (boolean, optional), to indicate that asym_pause should
+be enabled.
+
+- The old DT binding, where 'fixed-link' is a property with 5 cells
+  encoding various information about the fixed PHY, in the form of
+  .
+
+  * 'phy_id', emulated PHY ID, choose any but unique to the all specified
+fixed-links. Note U-Boot deliberately ignores the 'phy_id' and
+unconditionally uses PHY_FIXED_ID.
+  * 'full-duplex', 0 for half duplex or 1 for full duplex
+  * 'speed', link speed in Mbits/sec, accepts only 10, 100 and 1000
+  * 'pause', 0 for no pause, 1 for pause
+  * 'asym-pause', 0 for no asymmetric pause, 1 for asymmetric pause
 
 Examples:
 
+The new binding:
+
 ethernet@0 {
...
fixed-link {
@@ -28,3 +44,11 @@ ethernet@0 {
};
...
 };
+
+The old binding:
+
+ethernet@0 {
+   ...
+   fixed-link = <0 1 1000 0 0>;
+   ...
+};
-- 
2.25.1

[PATCH v3 02/22] of: extra: Introduce ofnode_phy_is_fixed_link() API

[Bin Meng]

Introduce a helper API ofnode_phy_is_fixed_link() to detect whether
the ethernet controller connects to a fixed-link pseudo-PHY device.

Note there are two ways to describe a fixed PHY attached to an
Ethernet device:

- the new DT binding, where 'fixed-link' is a sub-node of the
  Ethernet device
- the old DT binding, where 'fixed-link' is a property with 5
  cells encoding various information about the fixed PHY

Signed-off-by: Bin Meng 
Reviewed-by: Simon Glass 

---

Changes in v3:
- update the code logic to prefer the new binding if both new and
  old bindings exist

 drivers/core/of_extra.c | 23 +++
 include/dm/of_extra.h   | 20 
 2 files changed, 43 insertions(+)

diff --git a/drivers/core/of_extra.c b/drivers/core/of_extra.c
index 653344529e..7702beff97 100644
--- a/drivers/core/of_extra.c
+++ b/drivers/core/of_extra.c
@@ -130,3 +130,26 @@ int ofnode_decode_memory_region(ofnode config_node, const 
char *mem_type,
 
return 0;
 }
+
+bool ofnode_phy_is_fixed_link(ofnode eth_node, ofnode *phy_node)
+{
+   ofnode node, subnode;
+   int len;
+
+   subnode = ofnode_find_subnode(eth_node, "fixed-link");
+   if (ofnode_valid(subnode)) {
+   /* new binding */
+   node = subnode;
+   } else if (ofnode_get_property(eth_node, "fixed-link", &len) &&
+  len == (5 * sizeof(__be32))) {
+   /* old binding */
+   node = eth_node;
+   } else {
+   return false;
+   }
+
+   if (phy_node)
+   *phy_node = node;
+
+   return true;
+}
diff --git a/include/dm/of_extra.h b/include/dm/of_extra.h
index ca15df21b0..f54f0ed3aa 100644
--- a/include/dm/of_extra.h
+++ b/include/dm/of_extra.h
@@ -86,4 +86,24 @@ int ofnode_decode_memory_region(ofnode config_node, const 
char *mem_type,
const char *suffix, fdt_addr_t *basep,
fdt_size_t *sizep);
 
+/**
+ * ofnode_phy_is_fixed_link() - Detect fixed-link pseudo-PHY device
+ *
+ * This function detects whether the ethernet controller connects to a
+ * fixed-link pseudo-PHY device.
+ *
+ * This function supports the following two DT bindings:
+ * - the new DT binding, where 'fixed-link' is a sub-node of the
+ *   Ethernet device
+ * - the old DT binding, where 'fixed-link' is a property with 5
+ *   cells encoding various information about the fixed PHY
+ *
+ * If both new and old bindings exist, the new one is preferred.
+ *
+ * @param eth_node ofnode containing the fixed-link subnode/property
+ * @param phy_node if fixed-link PHY detected, containing the PHY ofnode
+ * @return true if a fixed-link pseudo-PHY device exists, false otherwise
+ */
+bool ofnode_phy_is_fixed_link(ofnode eth_node, ofnode *phy_node);
+
 #endif
-- 
2.25.1

[PATCH v3 03/22] dm: mdio: Use ofnode_phy_is_fixed_link() API

[Bin Meng]

Switch to use the ofnode_phy_is_fixed_link() API which can support
both the new and old DT bindings.

Signed-off-by: Bin Meng 
Reviewed-by: Ramon Fried 
Reviewed-by: Vladimir Oltean 
---

(no changes since v1)

 net/mdio-uclass.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/mdio-uclass.c b/net/mdio-uclass.c
index 5da984ca3f..2a9533c88b 100644
--- a/net/mdio-uclass.c
+++ b/net/mdio-uclass.c
@@ -11,6 +11,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -140,7 +141,7 @@ static struct phy_device *dm_eth_connect_phy_handle(struct 
udevice *ethdev,
int i;
 
if (CONFIG_IS_ENABLED(PHY_FIXED) &&
-   ofnode_valid(dev_read_subnode(ethdev, "fixed-link"))) {
+   ofnode_phy_is_fixed_link(dev_ofnode(ethdev), NULL)) {
phy = phy_connect(NULL, -1, ethdev, interface);
goto out;
}
-- 
2.25.1

[PATCH v3 05/22] net: phy: fixed: Drop #ifdef CONFIG_DM_ETH around phy_connect_fixed

[Bin Meng]

From: Vladimir Oltean 

In drivers/net/phy/Kconfig, CONFIG_PHY_FIXED already depends on
CONFIG_DM_ETH, so the function prototype definition when
CONFIG_DM_ETH=n does nothing, so it can be dropped. It is also
never reachable, since the whole function is already under #ifdef
CONFIG_PHY_FIXED (which again, as I said, depends on CONFIG_DM_ETH=y).

Signed-off-by: Vladimir Oltean 
Reviewed-by: Bin Meng 
Tested-by: Bin Meng 
Message-Id: <20210216224804.3355044-3-olte...@gmail.com>
Signed-off-by: Bin Meng 
Reviewed-by: Vladimir Oltean 
---

(no changes since v1)

 drivers/net/phy/phy.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 2feb559bba..eae40cc0d6 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1012,15 +1012,9 @@ struct phy_device *fixed_phy_create(ofnode node)
return phydev;
 }
 
-#ifdef CONFIG_DM_ETH
 static struct phy_device *phy_connect_fixed(struct mii_dev *bus,
struct udevice *dev,
phy_interface_t interface)
-#else
-static struct phy_device *phy_connect_fixed(struct mii_dev *bus,
-   struct eth_device *dev,
-   phy_interface_t interface)
-#endif
 {
ofnode node = dev_ofnode(dev), subnode;
struct phy_device *phydev;
-- 
2.25.1

[PATCH v3 04/22] net: phy: fixed: Be compatible with live OF tree

[Bin Meng]

From: Vladimir Oltean 

On systems that use CONFIG_OF_LIVE, the "ofnode" type is defined
as const struct device_node *np, while on the flat DT systems it
is defined as a long of_offset into gd->fdt_blob.

It is desirable that the fixed PHY driver uses the higher-level
ofnode abstraction instead of parsing gd->fdt_blob directly,
because that enables it to work on live OF systems.

The fixed PHY driver has used a nasty hack since its introduction in
commit db40c1aa1c10 ("drivers/net/phy: add fixed-phy / fixed-link support"),
which is to pass the long gd->fdt_blob offset inside int phydev->addr
(a value that normally holds the MDIO bus address at which the PHY
responds). Even ignoring the fact that the types were already
mismatched leading to a potential truncation (flat OF offset was
supposed to be a long and not an int), we really cannot extend this
hack any longer, because there's no way an int will hold the other
representation of ofnode, the struct device_node *np.

So we unfortunately need to do the right thing, which is to use the
framework introduced by Grygorii Strashko in
commit eef0b8a930d1 ("net: phy: add ofnode node to struct phy_device").
This will populate phydev->node for the fixed PHY.

Note that phydev->node will not be valid in the probe function, since
that is called synchronously from phy_device_create and we really have
no way of passing the ofnode directly through the phy_device_create API.
So we do what other drivers do too: we move the OF parsing logic from
the .probe to the .config method of the PHY driver. The new function
will be called at phy_config() time.

I do believe I've converted all the possible call paths for creating
a PHY with PHY_FIXED_ID, so there is really no reason to maintain
compatibility with the old logic of retrieving a flat OF tree offset
from phydev->addr. We just pass 0 to phydev->addr now.

Signed-off-by: Vladimir Oltean 
Reviewed-by: Bin Meng 
Tested-by: Bin Meng 
Message-Id: <20210216224804.3355044-2-olte...@gmail.com>
[bmeng: keep fixedphy_probe(); update mdio-uclass.c to handle fixed phy]
Signed-off-by: Bin Meng 
Reviewed-by: Vladimir Oltean 
---

(no changes since v1)

 drivers/net/phy/fixed.c | 26 +-
 drivers/net/phy/phy.c   | 30 +++---
 net/mdio-uclass.c   |  6 --
 3 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
index 1a38c29469..11342df1c5 100644
--- a/drivers/net/phy/fixed.c
+++ b/drivers/net/phy/fixed.c
@@ -17,13 +17,23 @@ DECLARE_GLOBAL_DATA_PTR;
 
 int fixedphy_probe(struct phy_device *phydev)
 {
+   /* fixed-link phy must not be reset by core phy code */
+   phydev->flags |= PHY_FLAG_BROKEN_RESET;
+
+   return 0;
+}
+
+int fixedphy_config(struct phy_device *phydev)
+{
+   ofnode node = phy_get_ofnode(phydev);
struct fixed_link *priv;
-   int ofnode = phydev->addr;
u32 val;
 
+   if (!ofnode_valid(node))
+   return -EINVAL;
+
/* check for mandatory properties within fixed-link node */
-   val = fdt_getprop_u32_default_node(gd->fdt_blob,
-  ofnode, 0, "speed", 0);
+   val = ofnode_read_u32_default(node, "speed", 0);
if (val != SPEED_10 && val != SPEED_100 && val != SPEED_1000 &&
val != SPEED_2500 && val != SPEED_1) {
printf("ERROR: no/invalid speed given in fixed-link node!");
@@ -38,12 +48,9 @@ int fixedphy_probe(struct phy_device *phydev)
phydev->priv = priv;
 
priv->link_speed = val;
-   priv->duplex = fdtdec_get_bool(gd->fdt_blob, ofnode, "full-duplex");
-   priv->pause = fdtdec_get_bool(gd->fdt_blob, ofnode, "pause");
-   priv->asym_pause = fdtdec_get_bool(gd->fdt_blob, ofnode, "asym-pause");
-
-   /* fixed-link phy must not be reset by core phy code */
-   phydev->flags |= PHY_FLAG_BROKEN_RESET;
+   priv->duplex = ofnode_read_bool(node, "full-duplex");
+   priv->pause = ofnode_read_bool(node, "pause");
+   priv->asym_pause = ofnode_read_bool(node, "asym-pause");
 
return 0;
 }
@@ -72,6 +79,7 @@ static struct phy_driver fixedphy_driver = {
.name   = "Fixed PHY",
.features   = PHY_GBIT_FEATURES | SUPPORTED_MII,
.probe  = fixedphy_probe,
+   .config = fixedphy_config,
.startup= fixedphy_startup,
.shutdown   = fixedphy_shutdown,
 };
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 89e3076bfd..2feb559bba 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -988,6 +988,7 @@ static struct phy_device *phy_connect_gmii2rgmii(struct 
mii_dev *bus,
 struct phy_device *fixed_phy_create(ofnode node)
 {
phy_interface_t interface = PHY_INTERFACE_MODE_NONE;
+   struct phy_device *phydev;
const char *if_str;
ofnode subnode;
 
@@ -1004,8 +1005,11 @@ struct phy_device *fixed_phy_create(ofnode node)
  

[PATCH v3 06/22] net: phy: xilinx: Be compatible with live OF tree

[Bin Meng]

Following the same updates that were done to the fixed phy driver,
use ofnode_ APIs instead of fdt_ APIs so that the Xilinx PHY driver
can support live DT.

Signed-off-by: Bin Meng 

---

(no changes since v2)

Changes in v2:
- move device tree parsing from xilinxgmiitorgmii_probe() to
  xilinxgmiitorgmii_config() and use OF APIs

 drivers/net/phy/phy.c   | 23 +--
 drivers/net/phy/xilinx_gmii2rgmii.c | 61 ++---
 2 files changed, 40 insertions(+), 44 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index eae40cc0d6..d464379121 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -953,23 +953,20 @@ static struct phy_device *phy_connect_gmii2rgmii(struct 
mii_dev *bus,
 #endif
 {
struct phy_device *phydev = NULL;
-   int sn = dev_of_offset(dev);
-   int off;
-
-   while (sn > 0) {
-   off = fdt_node_offset_by_compatible(gd->fdt_blob, sn,
-   "xlnx,gmii-to-rgmii-1.0");
-   if (off > 0) {
-   phydev = phy_device_create(bus, off,
+   ofnode node = dev_ofnode(dev);
+
+   while (ofnode_valid(node)) {
+   node = ofnode_by_compatible(node, "xlnx,gmii-to-rgmii-1.0");
+   if (ofnode_valid(node)) {
+   phydev = phy_device_create(bus, 0,
   PHY_GMII2RGMII_ID, false,
   interface);
+   if (phydev)
+   phydev->node = node;
break;
}
-   if (off == -FDT_ERR_NOTFOUND)
-   sn = fdt_first_subnode(gd->fdt_blob, sn);
-   else
-   printf("%s: Error finding compat string:%d\n",
-  __func__, off);
+
+   node = ofnode_first_subnode(node);
}
 
return phydev;
diff --git a/drivers/net/phy/xilinx_gmii2rgmii.c 
b/drivers/net/phy/xilinx_gmii2rgmii.c
index 74105c0b7d..635c0570ef 100644
--- a/drivers/net/phy/xilinx_gmii2rgmii.c
+++ b/drivers/net/phy/xilinx_gmii2rgmii.c
@@ -18,9 +18,38 @@ DECLARE_GLOBAL_DATA_PTR;
 
 static int xilinxgmiitorgmii_config(struct phy_device *phydev)
 {
-   struct phy_device *ext_phydev = phydev->priv;
+   ofnode node = phy_get_ofnode(phydev);
+   struct phy_device *ext_phydev;
+   struct ofnode_phandle_args phandle;
+   int ext_phyaddr = -1;
+   int ret;
 
debug("%s\n", __func__);
+
+   if (!ofnode_valid(node))
+   return -EINVAL;
+
+   phydev->addr = ofnode_read_u32_default(node, "reg", -1);
+   ret = ofnode_parse_phandle_with_args(node, "phy-handle",
+NULL, 0, 0, &phandle);
+   if (ret)
+   return ret;
+
+   ext_phyaddr = ofnode_read_u32_default(phandle.node, "reg", -1);
+   ext_phydev = phy_find_by_mask(phydev->bus,
+ 1 << ext_phyaddr,
+ PHY_INTERFACE_MODE_RGMII);
+   if (!ext_phydev) {
+   printf("%s, No external phy device found\n", __func__);
+   return -EINVAL;
+   }
+
+   ext_phydev->node = phandle.node;
+   phydev->priv = ext_phydev;
+
+   debug("%s, gmii2rgmmi:0x%x, extphy:0x%x\n", __func__, phydev->addr,
+ ext_phyaddr);
+
if (ext_phydev->drv->config)
ext_phydev->drv->config(ext_phydev);
 
@@ -83,11 +112,6 @@ static int xilinxgmiitorgmii_startup(struct phy_device 
*phydev)
 
 static int xilinxgmiitorgmii_probe(struct phy_device *phydev)
 {
-   int ofnode = phydev->addr;
-   u32 phy_of_handle;
-   int ext_phyaddr = -1;
-   struct phy_device *ext_phydev;
-
debug("%s\n", __func__);
 
if (phydev->interface != PHY_INTERFACE_MODE_GMII) {
@@ -95,31 +119,6 @@ static int xilinxgmiitorgmii_probe(struct phy_device 
*phydev)
return -EINVAL;
}
 
-   /*
-* Read the phy address again as the one we read in ethernet driver
-* was overwritten for the purpose of storing the ofnode
-*/
-   phydev->addr = fdtdec_get_int(gd->fdt_blob, ofnode, "reg", -1);
-   phy_of_handle = fdtdec_lookup_phandle(gd->fdt_blob, ofnode,
- "phy-handle");
-   if (phy_of_handle > 0)
-   ext_phyaddr = fdtdec_get_int(gd->fdt_blob,
-phy_of_handle,
-"reg", -1);
-   ext_phydev = phy_find_by_mask(phydev->bus,
- 1 << ext_phyaddr,
- PHY_INTERFACE_MODE_RGMII);
-   if (!ext_phydev) {
-   printf("%s, No external phy device found\n", __func__);
-   return -EINVAL;
-   }
-
-   ext_phydev->node = offset_to_

[PATCH v3 07/22] net: phy: xilinx: Drop #ifdef CONFIG_DM_ETH around phy_connect_gmii2rgmii()

[Bin Meng]

At present phy_connect_gmii2rgmii() is implemented using a DM API
dev_of_offset() hence it cannot support a non-DM configuration.
Remove the non-DM version prototype of phy_connect_gmii2rgmii()
and make the driver depend on CONFIG_DM_ETH.

Signed-off-by: Bin Meng 
Acked-by: Michal Simek 
Reviewed-by: Ramon Fried 
Reviewed-by: Vladimir Oltean 
---

(no changes since v1)

 drivers/net/phy/Kconfig | 1 +
 drivers/net/phy/phy.c   | 6 --
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index d69503067d..070ffa82cb 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -292,6 +292,7 @@ config PHY_XILINX
 
 config PHY_XILINX_GMII2RGMII
bool "Xilinx GMII to RGMII Ethernet PHYs support"
+   depends on DM_ETH
help
  This adds support for Xilinx GMII to RGMII IP core. This IP acts
  as bridge between MAC connected over GMII and external phy that
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index d464379121..c7cdf64a0a 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -942,15 +942,9 @@ void phy_connect_dev(struct phy_device *phydev, struct 
eth_device *dev)
 }
 
 #ifdef CONFIG_PHY_XILINX_GMII2RGMII
-#ifdef CONFIG_DM_ETH
 static struct phy_device *phy_connect_gmii2rgmii(struct mii_dev *bus,
 struct udevice *dev,
 phy_interface_t interface)
-#else
-static struct phy_device *phy_connect_gmii2rgmii(struct mii_dev *bus,
-struct eth_device *dev,
-phy_interface_t interface)
-#endif
 {
struct phy_device *phydev = NULL;
ofnode node = dev_ofnode(dev);
-- 
2.25.1

[PATCH v3 08/22] net: phy: Simplify the logic of phy_connect_fixed()

[Bin Meng]

Simplify the logic of phy_connect_fixed() by using the new API
ofnode_phy_is_fixed_link(), which brings additional bonus of
supporting the old DT bindings.

Signed-off-by: Bin Meng 
Reviewed-by: Ramon Fried 
Reviewed-by: Vladimir Oltean 
---

(no changes since v1)

 drivers/net/phy/phy.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index c7cdf64a0a..dcdef9e661 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1008,15 +1009,14 @@ static struct phy_device *phy_connect_fixed(struct 
mii_dev *bus,
phy_interface_t interface)
 {
ofnode node = dev_ofnode(dev), subnode;
-   struct phy_device *phydev;
-
-   subnode = ofnode_find_subnode(node, "fixed-link");
-   if (!ofnode_valid(subnode))
-   return NULL;
+   struct phy_device *phydev = NULL;
 
-   phydev = phy_device_create(bus, 0, PHY_FIXED_ID, false, interface);
-   if (phydev)
-   phydev->node = subnode;
+   if (ofnode_phy_is_fixed_link(node, &subnode)) {
+   phydev = phy_device_create(bus, 0, PHY_FIXED_ID,
+  false, interface);
+   if (phydev)
+   phydev->node = subnode;
+   }
 
return phydev;
 }
-- 
2.25.1

[PATCH v3 09/22] net: phy: fixed: Make driver ops static

[Bin Meng]

The PHY driver ops should be made static.

Signed-off-by: Bin Meng 
Reviewed-by: Ramon Fried 
Reviewed-by: Vladimir Oltean 
---

(no changes since v1)

 drivers/net/phy/fixed.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
index 11342df1c5..1d2587278f 100644
--- a/drivers/net/phy/fixed.c
+++ b/drivers/net/phy/fixed.c
@@ -15,7 +15,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-int fixedphy_probe(struct phy_device *phydev)
+static int fixedphy_probe(struct phy_device *phydev)
 {
/* fixed-link phy must not be reset by core phy code */
phydev->flags |= PHY_FLAG_BROKEN_RESET;
@@ -23,7 +23,7 @@ int fixedphy_probe(struct phy_device *phydev)
return 0;
 }
 
-int fixedphy_config(struct phy_device *phydev)
+static int fixedphy_config(struct phy_device *phydev)
 {
ofnode node = phy_get_ofnode(phydev);
struct fixed_link *priv;
@@ -55,7 +55,7 @@ int fixedphy_config(struct phy_device *phydev)
return 0;
 }
 
-int fixedphy_startup(struct phy_device *phydev)
+static int fixedphy_startup(struct phy_device *phydev)
 {
struct fixed_link *priv = phydev->priv;
 
@@ -68,7 +68,7 @@ int fixedphy_startup(struct phy_device *phydev)
return 0;
 }
 
-int fixedphy_shutdown(struct phy_device *phydev)
+static int fixedphy_shutdown(struct phy_device *phydev)
 {
return 0;
 }
-- 
2.25.1

[PATCH v3 10/22] net: phy: fixed: Add the missing ending newline

[Bin Meng]

The printf statement doesn't end with a newline. Add it.

Signed-off-by: Bin Meng 
Reviewed-by: Ramon Fried 
Reviewed-by: Vladimir Oltean 
---

(no changes since v1)

 drivers/net/phy/fixed.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
index 1d2587278f..304e506554 100644
--- a/drivers/net/phy/fixed.c
+++ b/drivers/net/phy/fixed.c
@@ -36,7 +36,7 @@ static int fixedphy_config(struct phy_device *phydev)
val = ofnode_read_u32_default(node, "speed", 0);
if (val != SPEED_10 && val != SPEED_100 && val != SPEED_1000 &&
val != SPEED_2500 && val != SPEED_1) {
-   printf("ERROR: no/invalid speed given in fixed-link node!");
+   printf("ERROR: no/invalid speed given in fixed-link node!\n");
return -EINVAL;
}
 
-- 
2.25.1

[PATCH v3 11/22] net: phy: fixed: Support the old DT binding

[Bin Meng]

Update fixedphy_probe() to support the old DT binding.

Signed-off-by: Bin Meng 
Reviewed-by: Ramon Fried 
Reviewed-by: Vladimir Oltean 

---

Changes in v3:
- sort variable definitions by line length

 drivers/net/phy/fixed.c | 26 +++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
index 304e506554..1192915ee5 100644
--- a/drivers/net/phy/fixed.c
+++ b/drivers/net/phy/fixed.c
@@ -27,6 +27,8 @@ static int fixedphy_config(struct phy_device *phydev)
 {
ofnode node = phy_get_ofnode(phydev);
struct fixed_link *priv;
+   bool old_binding = false;
+   u32 old_val[5];
u32 val;
 
if (!ofnode_valid(node))
@@ -34,6 +36,18 @@ static int fixedphy_config(struct phy_device *phydev)
 
/* check for mandatory properties within fixed-link node */
val = ofnode_read_u32_default(node, "speed", 0);
+
+   if (!val) {
+   /* try old binding */
+   old_binding = true;
+   if (ofnode_read_u32_array(node, "fixed-link", old_val,
+ ARRAY_SIZE(old_val))) {
+   printf("ERROR: no/invalid  property!\n");
+   return -ENOENT;
+   }
+   val = old_val[2];
+   }
+
if (val != SPEED_10 && val != SPEED_100 && val != SPEED_1000 &&
val != SPEED_2500 && val != SPEED_1) {
printf("ERROR: no/invalid speed given in fixed-link node!\n");
@@ -48,9 +62,15 @@ static int fixedphy_config(struct phy_device *phydev)
phydev->priv = priv;
 
priv->link_speed = val;
-   priv->duplex = ofnode_read_bool(node, "full-duplex");
-   priv->pause = ofnode_read_bool(node, "pause");
-   priv->asym_pause = ofnode_read_bool(node, "asym-pause");
+   if (!old_binding) {
+   priv->duplex = ofnode_read_bool(node, "full-duplex");
+   priv->pause = ofnode_read_bool(node, "pause");
+   priv->asym_pause = ofnode_read_bool(node, "asym-pause");
+   } else {
+   priv->duplex = old_val[1];
+   priv->pause = old_val[3];
+   priv->asym_pause = old_val[4];
+   }
 
return 0;
 }
-- 
2.25.1

[PATCH v3 12/22] net: tsec: Use dm_eth_phy_connect() directly for the DM case

[Bin Meng]

From: Vladimir Oltean 

Now that the fixed phy driver has been fully adapted to OF APIs,
and dm_eth_phy_connect() already can handle the fixed phy, call
dm_eth_phy_connect() directly in the DM tsec driver.

Signed-off-by: Vladimir Oltean 
Reviewed-by: Bin Meng 
Tested-by: Bin Meng 
Message-Id: <20210216224804.3355044-4-olte...@gmail.com>
[bmeng: split from "net: mdio: teach dm_eth_phy_connect to connect to fixed 
PHY"]
Signed-off-by: Bin Meng 
Reviewed-by: Vladimir Oltean 

---

(no changes since v2)

Changes in v2:
- new patch: split from <20210216224804.3355044-4-olte...@gmail.com>

 drivers/net/tsec.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
index ec48689372..f801d020fb 100644
--- a/drivers/net/tsec.c
+++ b/drivers/net/tsec.c
@@ -707,11 +707,7 @@ static int init_phy(struct tsec_private *priv)
tsec_configure_serdes(priv);
 
 #if defined(CONFIG_DM_ETH) && defined(CONFIG_DM_MDIO)
-   if (ofnode_valid(ofnode_find_subnode(dev_ofnode(priv->dev),
-"fixed-link")))
-   phydev = phy_connect(NULL, 0, priv->dev, priv->interface);
-   else
-   phydev = dm_eth_phy_connect(priv->dev);
+   phydev = dm_eth_phy_connect(priv->dev);
 #else
phydev = phy_connect(priv->bus, priv->phyaddr, priv->dev,
 priv->interface);
-- 
2.25.1

[PATCH v3 13/22] sandbox: Add a DSA sandbox driver and unit test

[Bin Meng]

From: Claudiu Manoil 

The DSA sandbox driver is used for unit testing the DSA class code.
It implements a simple 2 port switch plus 1 CPU port, and uses a
very simple tag to identify the ports.

The DSA sandbox device is connected via CPU port to a regular Ethernet
sandbox device, called 'dsa-test-eth, managed by the existing eth
sandbox driver.  The 'dsa-test-eth' is not intended for testing the
eth class code however, but it is used to emulate traffic through the
'lan0' and 'lan1' front pannel switch ports.  To achieve this the dsa
sandbox driver registers a tx handler for the 'dsa-test-eth' device.
The switch ports, labeled as 'lan0' and 'lan1', are also registered
as eth devices by the dsa class code this time.  So pinging through
these switch ports is as easy as:

=> setenv ethact lan0
=> ping 1.2.3.5

Unit tests for the dsa class code were also added.  The 'dsa_probe'
test exercises most API functions from dsa.h.  The 'dsa' unit test
simply exercises ARP/ICMP traffic through the two switch ports,
including tag injection and extraction, with the help of the dsa
sandbox driver.

I took care to minimize the impact on the existing eth unit tests,
though some adjustments needed to be made with the addition of
extra eth interfaces used by the dsa unit tests. The additional eth
interfaces also require MAC addresses, these have been added to the
sandbox default environment.

Signed-off-by: Alex Marginean 
Signed-off-by: Claudiu Manoil 
Reviewed-by: Simon Glass 
Signed-off-by: Vladimir Oltean 
Message-Id: <20210216224804.3355044-5-olte...@gmail.com>
Signed-off-by: Bin Meng 
---

(no changes since v1)

 arch/Kconfig  |   2 +
 arch/sandbox/dts/test.dts |  48 ++
 drivers/net/Kconfig   |   9 ++
 drivers/net/Makefile  |   1 +
 drivers/net/dsa_sandbox.c | 179 ++
 include/configs/sandbox.h |   2 +
 test/dm/Makefile  |   1 +
 test/dm/dsa.c |  82 +
 test/dm/eth.c |  10 +--
 9 files changed, 329 insertions(+), 5 deletions(-)
 create mode 100644 drivers/net/dsa_sandbox.c
 create mode 100644 test/dm/dsa.c

diff --git a/arch/Kconfig b/arch/Kconfig
index 27843cd79c..5672d1df7e 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -160,6 +160,8 @@ config SANDBOX
imply CMD_CLONE
imply SILENT_CONSOLE
imply BOOTARGS_SUBST
+   imply PHY_FIXED
+   imply DM_DSA
 
 config SH
bool "SuperH architecture"
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 2600360224..3ef3ba0b17 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -15,7 +15,9 @@
aliases {
console = &uart0;
eth0 = "/eth@10002000";
+   eth2 = &swp_0;
eth3 = ð_3;
+   eth4 = &dsa_eth0;
eth5 = ð_5;
gpio1 = &gpio_a;
gpio2 = &gpio_b;
@@ -478,6 +480,52 @@
fake-host-hwaddr = [00 00 66 44 22 22];
};
 
+   dsa_eth0: dsa-test-eth {
+   compatible = "sandbox,eth";
+   reg = <0x10006000 0x1000>;
+   fake-host-hwaddr = [00 00 66 44 22 66];
+   };
+
+   dsa-test {
+   compatible = "sandbox,dsa";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   swp_0: port@0 {
+   reg = <0>;
+   label = "lan0";
+   phy-mode = "rgmii-rxid";
+
+   fixed-link {
+   speed = <100>;
+   full-duplex;
+   };
+   };
+
+   swp_1: port@1 {
+   reg = <1>;
+   label = "lan1";
+   phy-mode = "rgmii-txid";
+
+   fixed-link {
+   speed = <100>;
+   full-duplex;
+   };
+   };
+
+   port@2 {
+   reg = <2>;
+   ethernet = <&dsa_eth0>;
+
+   fixed-link {
+   speed = <1000>;
+   full-duplex;
+   };
+   };
+   };
+   };
+
firmware {
sandbox_firmware: sandbox-firmware {
compatible = "sandbox,firmware";
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 0e84c22b50..f96ee64249 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -78,6 +78,15 @@ config DM_ETH_PHY
help
  Enable driver model for Ethernet Generic PHY .
 
+config DSA_SANDBOX
+   depends 

[PATCH v3 14/22] test: dm: Add a case to test ofnode_phy_is_fixed_link()

[Bin Meng]

This adds a test case to test the new ofnode_phy_is_fixed_link() API.
Both the new and old DT bindings are covered.

Signed-off-by: Bin Meng 
Reviewed-by: Simon Glass 

---

Changes in v3:
- reuse the sandbox dsa nodes for the fixed-link testing

 arch/sandbox/dts/test.dts |  6 +-
 test/dm/of_extra.c| 18 ++
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 3ef3ba0b17..7ad16635ad 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -507,11 +507,7 @@
reg = <1>;
label = "lan1";
phy-mode = "rgmii-txid";
-
-   fixed-link {
-   speed = <100>;
-   full-duplex;
-   };
+   fixed-link = <0 1 100 0 0>;
};
 
port@2 {
diff --git a/test/dm/of_extra.c b/test/dm/of_extra.c
index b19cd3787d..ac2d886892 100644
--- a/test/dm/of_extra.c
+++ b/test/dm/of_extra.c
@@ -36,3 +36,21 @@ static int dm_test_ofnode_read_fmap_entry(struct 
unit_test_state *uts)
return 0;
 }
 DM_TEST(dm_test_ofnode_read_fmap_entry, 0);
+
+static int dm_test_ofnode_phy_is_fixed_link(struct unit_test_state *uts)
+{
+   ofnode eth_node, phy_node, node;
+
+   eth_node = ofnode_path("/dsa-test/ports/port@0");
+   ut_assert(ofnode_phy_is_fixed_link(eth_node, &phy_node));
+   node = ofnode_path("/dsa-test/ports/port@0/fixed-link");
+   ut_asserteq_mem(&phy_node, &node, sizeof(ofnode));
+
+   eth_node = ofnode_path("/dsa-test/ports/port@1");
+   ut_assert(ofnode_phy_is_fixed_link(eth_node, &phy_node));
+   node = eth_node;
+   ut_asserteq_mem(&phy_node, &node, sizeof(ofnode));
+
+   return 0;
+}
+DM_TEST(dm_test_ofnode_phy_is_fixed_link, 0);
-- 
2.25.1

[PATCH v3 15/22] net: tsec: Use map_physmem() directly instead of dev_remap_addr()

[Bin Meng]

dev_remap_addr() eventually calls dev_read_addr_index(), while
pdata->iobase holds the return value of dev_read_addr() that calls
dev_read_addr_index() too. Such duplication can be avoided by using
map_physmem() directly.

Signed-off-by: Bin Meng 

---

Changes in v3:
- new patch: net: tsec: Use map_physmem() directly instead of dev_remap_addr()

 drivers/net/tsec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
index f801d020fb..491d2ef1ae 100644
--- a/drivers/net/tsec.c
+++ b/drivers/net/tsec.c
@@ -833,7 +833,9 @@ int tsec_probe(struct udevice *dev)
data = (struct tsec_data *)dev_get_driver_data(dev);
 
pdata->iobase = (phys_addr_t)dev_read_addr(dev);
-   priv->regs = dev_remap_addr(dev);
+   if (pdata->iobase == FDT_ADDR_T_NONE)
+   return -ENOENT;
+   priv->regs = map_physmem(pdata->iobase, 0, MAP_NOCACHE);
 
ret = dev_read_phandle_with_args(dev, "tbi-handle", NULL, 0, 0,
 &phandle_args);
-- 
2.25.1

[PATCH v3 16/22] dt-bindings: net: Update Freescale TSEC to support "queue-group"

[Bin Meng]

At present the Freescale TSEC node DT bindings doc requires a 
property in the TSEC node. But this might not always be the case.
In the upstream Linux kernel, there is no DT bindings doc for it
but the kernel driver tests a subnode of a name prefixed with
"queue-group", as we can see from gfar_of_init():

  for_each_available_child_of_node(np, child) {
  if (!of_node_name_eq(child, "queue-group"))
  ...

in drivers/net/ethernet/freescale/gianfar.c

Update our DT bindings to describe this alternate description.

Signed-off-by: Bin Meng 
Reviewed-by: Ramon Fried 

---

Changes in v3:
- add "ranges" in the alternate example

 doc/device-tree-bindings/net/fsl-tsec-phy.txt | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/doc/device-tree-bindings/net/fsl-tsec-phy.txt 
b/doc/device-tree-bindings/net/fsl-tsec-phy.txt
index a44c5fd9d9..5a73371a97 100644
--- a/doc/device-tree-bindings/net/fsl-tsec-phy.txt
+++ b/doc/device-tree-bindings/net/fsl-tsec-phy.txt
@@ -3,7 +3,9 @@
 Properties:
 
   - compatible : Should be "fsl,etsec2" or "gianfar"
-  - reg : Offset and length of the register set for the device
+  - reg : Offset and length of the register set for the device. If this is
+missing, a subnode with a name prefix "queue-group" must be provided to
+provide the  property.
   - phy-handle : See ethernet.txt file in the same directory.
   - phy-connection-type : See ethernet.txt file in the same directory. This
 property is only really needed if the connection is of type "rgmii-id",
@@ -18,6 +20,18 @@ Example:
phy-connection-type = "sgmii";
};
 
+An alternate description with "queue-group" subnode example:
+   ethernet@24000 {
+   compatible = "fsl,etsec2";
+   phy-handle = <&phy0>;
+   phy-connection-type = "sgmii";
+   ranges;
+
+   queue-group {
+   reg = <0x24000 0x1000>;
+   };
+   };
+
 Child nodes of the TSEC controller are typically the individual PHY devices
 connected via the MDIO bus (sometimes the MDIO bus controller is separate).
 
-- 
2.25.1

[PATCH v3 17/22] net: tsec: Support property from the subnode "queue-group"

[Bin Meng]

At present the tsec driver uses a non-standard DT bindings to get
its  base / size. The upstream Linux kernel seems to require
the  base / size to be put under a subnode of the eTSEC node
with a name prefix "queue-group". This is not documented in the
kernel DT bindings, but it looks every dtsi file that contains the
eTSEC node was written like this.

This commit updates the tsec driver to handle this case.

Signed-off-by: Bin Meng 
Reviewed-by: Ramon Fried 

---

Changes in v3:
- keep the variable definitions sorted
- invert the strncmp logic to reduce the indentation level
- add a comment to mention only the first "queue-group" is used
- call the same map_physmem() in the common code path

 drivers/net/tsec.c | 30 +++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
index 491d2ef1ae..c68e4b7fb5 100644
--- a/drivers/net/tsec.c
+++ b/drivers/net/tsec.c
@@ -826,15 +826,39 @@ int tsec_probe(struct udevice *dev)
u32 tbiaddr = CONFIG_SYS_TBIPA_VALUE;
struct tsec_data *data;
const char *phy_mode;
+   ofnode parent, child;
fdt_addr_t reg;
-   ofnode parent;
int ret;
 
data = (struct tsec_data *)dev_get_driver_data(dev);
 
pdata->iobase = (phys_addr_t)dev_read_addr(dev);
-   if (pdata->iobase == FDT_ADDR_T_NONE)
-   return -ENOENT;
+   if (pdata->iobase == FDT_ADDR_T_NONE) {
+   ofnode_for_each_subnode(child, dev_ofnode(dev)) {
+   if (strncmp(ofnode_get_name(child), "queue-group",
+   strlen("queue-group")))
+   continue;
+
+   reg = ofnode_get_addr(child);
+   if (reg == FDT_ADDR_T_NONE) {
+   printf("No 'reg' property of \n");
+   return -ENOENT;
+   }
+   pdata->iobase = reg;
+
+   /*
+* if there are multiple queue groups,
+* only the first one is used.
+*/
+   break;
+   }
+
+   if (!ofnode_valid(child)) {
+   printf("No child node for ?\n");
+   return -ENOENT;
+   }
+   }
+
priv->regs = map_physmem(pdata->iobase, 0, MAP_NOCACHE);
 
ret = dev_read_phandle_with_args(dev, "tbi-handle", NULL, 0, 0,
-- 
2.25.1

[PATCH v3 18/22] dm: core: Correctly read of simple-bus

[Bin Meng]

At present we decode simple bus  using the following assumption:

- parent #address-cells 1
- child #address-cells 1
- child #size-cells 1

However this might not always be the case.

Update to use fdt_addr_t and fdt_size_t in 'struct simple_bus_plat', and
use fdt_read_ranges() to correctly decode it according to the actual
parent and child #address-cells / #size-cells under a Kconfig option
CONFIG_SIMPLE_BUS_CORRECT_RANGE which can be turned on for any board
that needs it.

Signed-off-by: Bin Meng 
Reviewed-by: Simon Glass 

---

(no changes since v2)

Changes in v2:
- include 
- use a Kconfig option CONFIG_SIMPLE_BUS_CORRECT_RANGE to control the
  new behavior for boards that want this

 drivers/core/Kconfig  | 13 +
 drivers/core/simple-bus.c | 32 +---
 include/dm/simple_bus.h   |  6 +++---
 3 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
index 1eccac28c6..c7504edaf8 100644
--- a/drivers/core/Kconfig
+++ b/drivers/core/Kconfig
@@ -235,6 +235,19 @@ config SPL_SIMPLE_BUS
  Supports the 'simple-bus' driver, which is used on some systems
  in SPL.
 
+config SIMPLE_BUS_CORRECT_RANGE
+   bool "Decode the 'simple-bus'  by honoring the #address-cells 
and #size-cells"
+   depends on SIMPLE_BUS
+   help
+ Decoding the 'simple-bus'  by honoring the #address-cells
+ and #size-cells of parent/child bus. If unset, #address-cells of
+ parent bus is assumed to be 1, #address-cells and #size-cells of
+ child bus is also assumed to be 1, to save some spaces of using
+ an advanced API to decode the , which benefits SPL image
+ builds that have size limits.
+
+ If you are unsure about this, Say N here.
+
 config SIMPLE_PM_BUS
bool "Support simple-pm-bus driver"
depends on DM && OF_CONTROL && CLK && POWER_DOMAIN
diff --git a/drivers/core/simple-bus.c b/drivers/core/simple-bus.c
index b0c2c20958..18f52d26df 100644
--- a/drivers/core/simple-bus.c
+++ b/drivers/core/simple-bus.c
@@ -4,8 +4,12 @@
  */
 
 #include 
+#include 
 #include 
 #include 
+#include 
+
+DECLARE_GLOBAL_DATA_PTR;
 
 fdt_addr_t simple_bus_translate(struct udevice *dev, fdt_addr_t addr)
 {
@@ -22,16 +26,30 @@ static int simple_bus_post_bind(struct udevice *dev)
 #if CONFIG_IS_ENABLED(OF_PLATDATA)
return 0;
 #else
-   u32 cell[3];
+   struct simple_bus_plat *plat = dev_get_uclass_plat(dev);
int ret;
 
-   ret = dev_read_u32_array(dev, "ranges", cell, ARRAY_SIZE(cell));
-   if (!ret) {
-   struct simple_bus_plat *plat = dev_get_uclass_plat(dev);
+   if (CONFIG_IS_ENABLED(SIMPLE_BUS_CORRECT_RANGE)) {
+   uint64_t caddr, paddr, len;
+
+   /* only read range index 0 */
+   ret = fdt_read_range((void *)gd->fdt_blob, dev_of_offset(dev),
+0, &caddr, &paddr, &len);
+   if (!ret) {
+   plat->base = caddr;
+   plat->target = paddr;
+   plat->size = len;
+   }
+   } else {
+   u32 cell[3];
 
-   plat->base = cell[0];
-   plat->target = cell[1];
-   plat->size = cell[2];
+   ret = dev_read_u32_array(dev, "ranges", cell,
+ARRAY_SIZE(cell));
+   if (!ret) {
+   plat->base = cell[0];
+   plat->target = cell[1];
+   plat->size = cell[2];
+   }
}
 
return dm_scan_fdt_dev(dev);
diff --git a/include/dm/simple_bus.h b/include/dm/simple_bus.h
index 4ad4cc4051..b7104013c0 100644
--- a/include/dm/simple_bus.h
+++ b/include/dm/simple_bus.h
@@ -7,9 +7,9 @@
 #define __DM_SIMPLE_BUS_H
 
 struct simple_bus_plat {
-   u32 base;
-   u32 size;
-   u32 target;
+   fdt_addr_t base;
+   fdt_size_t size;
+   fdt_addr_t target;
 };
 
 #endif
-- 
2.25.1

[PATCH v3 19/22] test: dm: Add a test case for simple-bus

[Bin Meng]

This adds a test case to verify reading  of a simple-bus is
working as expected.

Signed-off-by: Bin Meng 
Reviewed-by: Simon Glass 

---

(no changes since v2)

Changes in v2:
- default y if SANDBOX for CONFIG_SIMPLE_BUS_CORRECT_RANGE

 drivers/core/Kconfig |  1 +
 test/dm/Makefile |  1 +
 test/dm/simple-bus.c | 33 +
 3 files changed, 35 insertions(+)
 create mode 100644 test/dm/simple-bus.c

diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
index c7504edaf8..a7c3120860 100644
--- a/drivers/core/Kconfig
+++ b/drivers/core/Kconfig
@@ -238,6 +238,7 @@ config SPL_SIMPLE_BUS
 config SIMPLE_BUS_CORRECT_RANGE
bool "Decode the 'simple-bus'  by honoring the #address-cells 
and #size-cells"
depends on SIMPLE_BUS
+   default y if SANDBOX
help
  Decoding the 'simple-bus'  by honoring the #address-cells
  and #size-cells of parent/child bus. If unset, #address-cells of
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 1c2fa95f4a..d0d46e7f7e 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -90,6 +90,7 @@ obj-$(CONFIG_DM_MDIO) += mdio.o
 obj-$(CONFIG_DM_MDIO_MUX) += mdio_mux.o
 obj-$(CONFIG_DM_RNG) += rng.o
 obj-$(CONFIG_CLK_K210_SET_RATE) += k210_pll.o
+obj-$(CONFIG_SIMPLE_BUS) += simple-bus.o
 obj-$(CONFIG_SIMPLE_PM_BUS) += simple-pm-bus.o
 obj-$(CONFIG_RESET_SYSCON) += syscon-reset.o
 obj-$(CONFIG_SCMI_FIRMWARE) += scmi.o
diff --git a/test/dm/simple-bus.c b/test/dm/simple-bus.c
new file mode 100644
index 00..3530b47fac
--- /dev/null
+++ b/test/dm/simple-bus.c
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2021, Bin Meng 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static int dm_test_simple_bus(struct unit_test_state *uts)
+{
+   struct udevice *dev;
+   struct simple_bus_plat *plat;
+
+   /* locate the dummy device @ translation-test node */
+   ut_assertok(uclass_find_device_by_seq(UCLASS_TEST_DUMMY, 0, &dev));
+   ut_asserteq_str("dev@0,0", dev->name);
+
+   /* locate the parent node which is a simple-bus */
+   ut_assertnonnull(dev = dev_get_parent(dev));
+   ut_asserteq_str("translation-test@8000", dev->name);
+
+   ut_assertnonnull(plat = dev_get_uclass_plat(dev));
+   ut_asserteq(0, plat->base);
+   ut_asserteq(0x8000, plat->target);
+   ut_asserteq(0x1000, plat->size);
+
+   return 0;
+}
+DM_TEST(dm_test_simple_bus, UT_TESTF_SCAN_FDT | UT_TESTF_FLAT_TREE);
-- 
2.25.1

[PATCH v3 20/22] ppc: qemu: Create a virtual memory mapping of the platform bus

[Bin Meng]

QEMU ppce500 target can dynamically instantiate an eTSEC device on
a platform bus if "-device eTSEC" is given to QEMU. It is presented
as a "simple-bus" in the device tree, with an additional compatible
string "qemu,platform".

Let's create a virtual memory mapping for it in misc_init_r(), in
preparation to adding eTSEC support.

Signed-off-by: Bin Meng 

---

Changes in v3:
- extract platform bus virtual memory mapping codes to a new routine
- add a "break" in case multiple "qemu,platform" nodes exist

Changes in v2:
- turn on CONFIG_SIMPLE_BUS_CORRECT_RANGE in qemu-ppce500_defconfig

 board/emulation/qemu-ppce500/Kconfig|  6 
 board/emulation/qemu-ppce500/qemu-ppce500.c | 31 +
 configs/qemu-ppce500_defconfig  |  1 +
 3 files changed, 38 insertions(+)

diff --git a/board/emulation/qemu-ppce500/Kconfig 
b/board/emulation/qemu-ppce500/Kconfig
index 4312d986d8..1c5aa18aa9 100644
--- a/board/emulation/qemu-ppce500/Kconfig
+++ b/board/emulation/qemu-ppce500/Kconfig
@@ -9,4 +9,10 @@ config SYS_VENDOR
 config SYS_CONFIG_NAME
default "qemu-ppce500"
 
+config PLATFORM_BUS_MAP_ADDR
+   hex
+   default 0xf000
+   help
+ The QEMU platform bus base mapped address in the virtual memory space.
+
 endif
diff --git a/board/emulation/qemu-ppce500/qemu-ppce500.c 
b/board/emulation/qemu-ppce500/qemu-ppce500.c
index daa103c564..bbfaaed3d0 100644
--- a/board/emulation/qemu-ppce500/qemu-ppce500.c
+++ b/board/emulation/qemu-ppce500/qemu-ppce500.c
@@ -14,6 +14,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -109,6 +111,19 @@ static int pci_map_region(phys_addr_t paddr, phys_size_t 
size, ulong *pmap_addr)
return 0;
 }
 
+static int platform_bus_map_region(ulong map_addr, phys_addr_t paddr,
+  phys_size_t size)
+{
+   /* Align map_addr */
+   map_addr += size - 1;
+   map_addr &= ~(size - 1);
+
+   /* Map virtual memory for range */
+   assert(!tlb_map_range(map_addr, paddr, size, TLB_MAP_IO));
+
+   return 0;
+}
+
 int misc_init_r(void)
 {
struct udevice *dev;
@@ -148,6 +163,22 @@ int misc_init_r(void)
 */
disable_tlb(find_tlb_idx((void *)CONFIG_SYS_TMPVIRT, 1));
 
+   /*
+* Detect the presence of the platform bus node, and
+* create a virtual memory mapping for it.
+*/
+   for (ret = uclass_find_first_device(UCLASS_SIMPLE_BUS, &dev);
+dev;
+ret = uclass_find_next_device(&dev)) {
+   if (device_is_compatible(dev, "qemu,platform")) {
+   struct simple_bus_plat *plat = dev_get_uclass_plat(dev);
+
+   platform_bus_map_region(CONFIG_PLATFORM_BUS_MAP_ADDR,
+   plat->target, plat->size);
+   break;
+   }
+   }
+
return 0;
 }
 
diff --git a/configs/qemu-ppce500_defconfig b/configs/qemu-ppce500_defconfig
index 536fe7d6e1..151834b4cf 100644
--- a/configs/qemu-ppce500_defconfig
+++ b/configs/qemu-ppce500_defconfig
@@ -28,6 +28,7 @@ CONFIG_OF_BOARD=y
 CONFIG_ENV_OVERWRITE=y
 CONFIG_SYS_RELOC_GD_ENV_ADDR=y
 CONFIG_DM=y
+CONFIG_SIMPLE_BUS_CORRECT_RANGE=y
 CONFIG_BLK=y
 CONFIG_HAVE_BLOCK_DEVICE=y
 CONFIG_MPC8XXX_GPIO=y
-- 
2.25.1

[PATCH v3 21/22] ppc: qemu: Enable eTSEC support

[Bin Meng]

QEMU ppce500 target can dynamically instantiate an eTSEC device
if "-device eTSEC" is given to QEMU. This commit enables eTSEC
driver and the required fixed PHY driver to create a usable
network configuration using eTSEC.

Unlike a real world 85xx board that usually stores the eTSEC MAC
address in an EEPROM, CONFIG_NET_RANDOM_ETHADDR is required for
QEMU otherwise U-Boot ethernet initialization complains no valid
ethernet address is set.

Signed-off-by: Bin Meng 
Reviewed-by: Vladimir Oltean 
---

(no changes since v1)

 configs/qemu-ppce500_defconfig | 4 
 1 file changed, 4 insertions(+)

diff --git a/configs/qemu-ppce500_defconfig b/configs/qemu-ppce500_defconfig
index 151834b4cf..a1b9ea56ca 100644
--- a/configs/qemu-ppce500_defconfig
+++ b/configs/qemu-ppce500_defconfig
@@ -27,6 +27,7 @@ CONFIG_OF_CONTROL=y
 CONFIG_OF_BOARD=y
 CONFIG_ENV_OVERWRITE=y
 CONFIG_SYS_RELOC_GD_ENV_ADDR=y
+CONFIG_NET_RANDOM_ETHADDR=y
 CONFIG_DM=y
 CONFIG_SIMPLE_BUS_CORRECT_RANGE=y
 CONFIG_BLK=y
@@ -35,8 +36,11 @@ CONFIG_MPC8XXX_GPIO=y
 CONFIG_DM_I2C=y
 CONFIG_SYS_I2C_FSL=y
 # CONFIG_MMC is not set
+CONFIG_PHY_FIXED=y
 CONFIG_DM_ETH=y
+CONFIG_DM_MDIO=y
 CONFIG_E1000=y
+CONFIG_TSEC_ENET=y
 CONFIG_DM_PCI=y
 CONFIG_PCI_MPC85XX=y
 CONFIG_DM_RTC=y
-- 
2.25.1

[PATCH v3 22/22] doc: board: qemu-ppce500: Document eTSEC usage

[Bin Meng]

Document how to launch a QEMU session with eTSEC as a network device.

Signed-off-by: Bin Meng 
Reviewed-by: Vladimir Oltean 

---

(no changes since v1)

 doc/board/emulation/qemu-ppce500.rst | 5 +
 1 file changed, 5 insertions(+)

diff --git a/doc/board/emulation/qemu-ppce500.rst 
b/doc/board/emulation/qemu-ppce500.rst
index 0a5c86c61a..5de0aaf55d 100644
--- a/doc/board/emulation/qemu-ppce500.rst
+++ b/doc/board/emulation/qemu-ppce500.rst
@@ -70,6 +70,11 @@ interface at PCI address 0.1.0, but we can switch that to an 
e1000 NIC by::
 $ qemu-system-ppc -nographic -machine ppce500 -bios u-boot \
   -nic tap,ifname=tap0,script=no,downscript=no,model=e1000
 
+The QEMU ppce500 machine can also dynamically instantiate an eTSEC device if
+"-device eTSEC" is given to QEMU::
+
+-netdev tap,ifname=tap0,script=no,downscript=no,id=net0 -device 
eTSEC,netdev=net0
+
 VirtIO BLK driver is also enabled to support booting from a disk image where
 a kernel image is stored. Append the following to QEMU::
 
-- 
2.25.1

Re: [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot####

[Heinrich Schuchardt]

On 3/13/21 10:47 PM, Ilias Apalodimas wrote:

On the following patches we allow for an initrd path to be stored in
Boot variables.  Specifically we encode in the FIlePathList[] of
the EFI_LOAD_OPTIONS for each Boot variable.

The FilePathList[] array looks like this:
kernel - 0xff - VenMedia(initrd GUID) - initrd1 - 0x01 initrd2 - 0xff
So let's add the relevant functions to concatenate and retrieve a device
path based on a Vendor GUID.

Signed-off-by: Ilias Apalodimas 
---
  include/efi_loader.h |  4 ++
  lib/efi_loader/efi_device_path.c | 99 ++--
  2 files changed, 98 insertions(+), 5 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index f470bbd636f4..eb11a8c7d4b1 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -738,6 +738,10 @@ struct efi_load_option {
const u8 *optional_data;
  };

+struct efi_device_path *efi_dp_from_lo(struct efi_load_option *lo,
+  efi_uintn_t *size, efi_guid_t guid);
+struct efi_device_path *efi_dp_concat(const struct efi_device_path *dp1,
+ const struct efi_device_path *dp2);
  efi_status_t efi_deserialize_load_option(struct efi_load_option *lo, u8 *data,
 efi_uintn_t *size);
  unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 
**data);
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index c9315dd45857..1f55c772dc83 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -282,11 +282,25 @@ struct efi_device_path *efi_dp_dup(const struct 
efi_device_path *dp)
return ndp;
  }

-struct efi_device_path *efi_dp_append(const struct efi_device_path *dp1,
- const struct efi_device_path *dp2)
+/** _efi_dp_append() - Append or concatenate two device paths.


Please, add an empty line.


+ * Concatenated device path will be separated by a 0xff end


... by a sub-type 0xff end node.


+ * node.
+ *
+ * @dp1:   First device path
+ * @dp2:   Second device path


Please, describe argument @concat.


+ *
+ * Return: concatenated device path or NULL. Caller must free the returned
+ *  value
+ */
+static struct efi_device_path *_efi_dp_append(const struct efi_device_path 
*dp1,


This function name is very close to efi_dp_append. Maybe
efi_dp_append_or_concatenate() would be a better function name.


+ const struct efi_device_path *dp2,
+ bool concat)
  {
struct efi_device_path *ret;
+   size_t end_size = sizeof(END);

+   if (concat)
+   end_size = 2 * sizeof(END);
if (!dp1 && !dp2) {
/* return an end node */
ret = efi_dp_dup(&END);
@@ -298,18 +312,56 @@ struct efi_device_path *efi_dp_append(const struct 
efi_device_path *dp1,
/* both dp1 and dp2 are non-null */
unsigned sz1 = efi_dp_size(dp1);
unsigned sz2 = efi_dp_size(dp2);
-   void *p = dp_alloc(sz1 + sz2 + sizeof(END));
+   void *p = dp_alloc(sz1 + sz2 + end_size);
if (!p)
return NULL;
+   ret = p;
memcpy(p, dp1, sz1);
+   p += sz1;
+
+   if (concat) {
+   memcpy(p, &END, sizeof(END));
+   p += sizeof(END);
+   }
+
/* the end node of the second device path has to be retained */
-   memcpy(p + sz1, dp2, sz2 + sizeof(END));
-   ret = p;
+   memcpy(p, dp2, sz2);
+   p += sz2;
+   memcpy(p, &END, sizeof(END));
}

return ret;
  }

+/** efi_dp_append() - Append a device to an existing device path.
+ *
+ * @dp1:   First device path
+ * @dp2:   Second device path
+ *
+ * Return: concatenated device path or NULL. Caller must free the returned
+ *  value
+ */
+struct efi_device_path *efi_dp_append(const struct efi_device_path *dp1,
+ const struct efi_device_path *dp2)
+{
+   return _efi_dp_append(dp1, dp2, false);
+}
+
+/** efi_dp_concat() - Concatenate 2 device paths. The final device path will
+ *contain two device paths separated by and end node 
(0xff).
+ *
+ * @dp1:   First device path
+ * @dp2:   Second device path
+ *
+ * Return: concatenated device path or NULL. Caller must free the returned
+ *  value
+ */
+struct efi_device_path *efi_dp_concat(const struct efi_device_path *dp1,
+ const struct efi_device_path *dp2)
+{
+   return _efi_dp_append(dp1, dp2, true);
+}
+
  struct efi_device_path *efi_dp_append_node(const struct efi_device_path *dp,
   

Re: [PATCH 3/6 v2] efi_loader: Introduce helper functions for EFI

[Heinrich Schuchardt]

On 3/13/21 10:47 PM, Ilias Apalodimas wrote:

A following patch introduces a different logic for loading initrd's
based on the EFI_LOAD_FILE2_PROTOCOL.
Since similar logic can be applied in the future for other system files
(i.e DTBs), let's add some helper functions which will retrieve and
parse file paths stored in EFI variables.

Signed-off-by: Ilias Apalodimas 
---
  include/efi_helper.h|  15 
  include/efi_loader.h|   2 +
  lib/efi_loader/Makefile |   1 +
  lib/efi_loader/efi_helper.c | 133 
  lib/efi_loader/efi_var_common.c |  33 
  5 files changed, 184 insertions(+)
  create mode 100644 include/efi_helper.h
  create mode 100644 lib/efi_loader/efi_helper.c

diff --git a/include/efi_helper.h b/include/efi_helper.h
new file mode 100644
index ..97492c65b6d2
--- /dev/null
+++ b/include/efi_helper.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2020, Linaro Limited
+ */
+
+#if !defined _EFI_HELPER_H_
+#define _EFI_HELPER_H
+
+#include 
+#include 
+
+efi_status_t efi_get_file_size(struct efi_file_handle *fh, efi_uintn_t *size);
+struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid);
+
+#endif
diff --git a/include/efi_loader.h b/include/efi_loader.h
index eb11a8c7d4b1..aa812a9a3052 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -717,6 +717,8 @@ efi_status_t EFIAPI efi_query_variable_info(
u64 *remaining_variable_storage_size,
u64 *maximum_variable_size);

+void *efi_get_var(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size);
+
  /*
   * See section 3.1.3 in the v2.7 UEFI spec for more details on
   * the layout of EFI_LOAD_OPTION.  In short it is:
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index 10b42e8847bf..da2741adecfa 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -23,6 +23,7 @@ endif
  obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
  obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o
  obj-y += efi_boottime.o
+obj-y += efi_helper.o
  obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
  obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o
  obj-y += efi_console.o
diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c
new file mode 100644
index ..df5bdc506dbe
--- /dev/null
+++ b/lib/efi_loader/efi_helper.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020, Linaro Limited
+ */
+
+#define LOG_CATEGORY LOGC_EFI
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+ * efi_create_current_boot_var() - Return Boot name were  is replaced 
by
+ *the value of BootCurrent
+ *
+ * @var_name:  variable name
+ * @var_name_size: size of var_name
+ *
+ * Return: Status code
+ */
+static efi_status_t efi_create_current_boot_var(u16 var_name[],
+   size_t var_name_size)
+{
+   efi_uintn_t boot_current_size;
+   efi_status_t ret;
+   u16 boot_current;
+   u16 *pos;
+
+   boot_current_size = sizeof(boot_current);
+   ret = efi_get_variable_int(L"BootCurrent",
+  &efi_global_variable_guid, NULL,
+  &boot_current_size, &boot_current, NULL);
+   if (ret != EFI_SUCCESS)
+   goto out;
+
+   pos = efi_create_indexed_name(var_name, var_name_size, "Boot",
+ boot_current);
+   if (!pos) {
+   ret = EFI_OUT_OF_RESOURCES;
+   goto out;
+   }
+
+out:
+   return ret;
+}
+
+/**
+ * efi_get_dp_from_boot() - Retrieve and return a device path from an EFI
+ * Boot### variable


We could add a paragraph here:

A boot option may contain an array of device paths. We use a VenMedia()
with a specific GUID to identify the usage of the array members. This
function is use to extract a specific device path.


+ *
+ * @guid:  Vendor guid of the VenMedia DP


vendor GUID of the VenMedia() device path node identifying the device path


+ *
+ * Return: device path or NULL. Caller must free the returned value
+ */
+struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid)
+{
+   struct efi_device_path *file_path = NULL;
+   struct efi_device_path *tmp;
+   struct efi_load_option lo;
+   void *var_value = NULL;
+   efi_uintn_t size;
+   efi_status_t ret;
+   u16 var_name[16];
+
+   ret = efi_create_current_boot_var(var_name, sizeof(var_name));
+   if (ret != EFI_SUCCESS)
+   return NULL;
+
+   var_value = efi_get_var(var_name, &efi_global_variable_guid, &size);
+   if (!var_value)
+   return NULL;
+
+   ret = efi_deserialize_load_option(&lo, var_value, &size);
+   if (ret != EFI_SUCCESS)
+  

Re: [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot####

[Ilias Apalodimas]

On Sun, Mar 14, 2021 at 08:19:49AM +0100, Heinrich Schuchardt wrote:
> > + *   Caller must free the returned value

[...]

> > + */
> > +struct
> > +efi_device_path *efi_dp_from_lo(struct efi_load_option *lo,
> > +   efi_uintn_t *size, efi_guid_t guid)
> > +{
> > +   struct efi_device_path *fp = lo->file_path;
> > +   struct efi_device_path_vendor *vendor;
> > +   int lo_len = lo->file_path_length;
> > +
> > +   for (; lo_len >=  sizeof(struct efi_device_path);
> > +lo_len -= fp->length, fp = (void *)fp + fp->length) {
> > +   if (fp->type != DEVICE_PATH_TYPE_MEDIA_DEVICE ||
> > +   fp->sub_type != DEVICE_PATH_SUB_TYPE_VENDOR_PATH)
> > +   continue;
> 
> The device path is provided by the user and may be constructed incorrectly.
> 
> lo_len might be negative here. Or the remaining device path might not
> fit into lo_len.
> 
> Function efi_dp_check_length() can be used to check the size but it
> currently accepts only positive values of maxlen. Maybe we should change
> the type of maxlen to ssize() in that function.
> 

Yea, I forgot to fix this one. 

Regards
/Ilias
> Best regards
> 
> Heinrich
> 
> > +
> > +   vendor = (struct efi_device_path_vendor *)fp;
> > +   if (!guidcmp(&vendor->guid, &guid))
> > +   return efi_dp_dup(fp);
> > +   }
> > +   log_debug("VenMedia(%pUl) not found in %ls\n", &guid, lo->label);
> > +
> > +   return NULL;
> > +}
> > 
> 

Re: [PATCH 3/6 v2] efi_loader: Introduce helper functions for EFI

[Ilias Apalodimas]

Hi Heinrich, 

On Sun, Mar 14, 2021 at 08:31:20AM +0100, Heinrich Schuchardt wrote:
> On 3/13/21 10:47 PM, Ilias Apalodimas wrote:
> > A following patch introduces a different logic for loading initrd's
> > based on the EFI_LOAD_FILE2_PROTOCOL.
> > +/**
> > + * efi_get_file_size() - Get the size of a file using an EFI file handle
> > + *
> > + * @handle:EFI file handle
> > + * @size:  buffer to fill in the discovered size
> > + *
> > + * Return: device path or NULL. Caller must free the returned value
> > + */
> > +efi_status_t efi_get_file_size(struct efi_file_handle *fh, efi_uintn_t 
> > *size)
> 
> Should this function be in lib/efi_loader/efi_file?

there's already a statically defined efi_get_file_size() in that file.  I can
rename that, just let me know what you prefer.

> 
> Best regards
> 
> Heinrich
> 
> > +{
> > +   struct efi_file_info *info = NULL;
> > +   efi_uintn_t bs = 0;
> > +   efi_status_t ret;
> > +
> > +   *size = 0;
> > +   ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,
> > +  info));
> > +   if (ret != EFI_BUFFER_TOO_SMALL) {
> > +   ret = EFI_DEVICE_ERROR;
> > +   goto out;
> > +   }
> > +
> > +   info = malloc(bs);
> > +   if (!info) {
> > +   ret = EFI_OUT_OF_RESOURCES;
> > +   goto out;
> > +   }
> > +   ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,
> > +  info));
> > +   if (ret != EFI_SUCCESS)
> > +   goto out;
> > +
> > +   *size = info->file_size;
> > +
> > +out:
> > +   free(info);
> > +   return ret;
> > +}
> > diff --git a/lib/efi_loader/efi_var_common.c 
> > b/lib/efi_loader/efi_var_common.c
> > index 1c7459266a38..b11ed91a74a4 100644
> > --- a/lib/efi_loader/efi_var_common.c
> > +++ b/lib/efi_loader/efi_var_common.c
> > @@ -9,6 +9,7 @@
> >   #include 
> >   #include 
> >   #include 
> > +#include 
> > 
> >   enum efi_secure_mode {
> > EFI_MODE_SETUP,
> > @@ -343,3 +344,35 @@ enum efi_auth_var_type efi_auth_var_get_type(u16 
> > *name, const efi_guid_t *guid)
> > }
> > return EFI_AUTH_VAR_NONE;
> >   }
> > +
> > +/**
> > + * efi_get_var() - read value of an EFI variable
> > + *
> > + * @name:  variable name
> > + * @start: vendor GUID
> > + * @size:  size of allocated buffer
> > + *
> > + * Return: buffer with variable data or NULL
> > + */
> > +void *efi_get_var(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size)
> > +{
> > +   efi_status_t ret;
> > +   void *buf = NULL;
> > +
> > +   *size = 0;
> > +   ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
> > +   if (ret == EFI_BUFFER_TOO_SMALL) {
> > +   buf = malloc(*size);
> > +   if (!buf)
> > +   return NULL;
> > +   ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
> > +   }
> > +
> > +   if (ret != EFI_SUCCESS) {
> > +   free(buf);
> > +   *size = 0;
> > +   return NULL;
> > +   }
> > +
> > +   return buf;
> > +}
> > 
> 

Re: [PATCH 6/6 v2] doc: Update uefi documentation for initrd loading options

[Heinrich Schuchardt]

On 3/13/21 10:47 PM, Ilias Apalodimas wrote:

Document the command line options for efidebug and initrd loading

Signed-off-by: Ilias Apalodimas 
---
  doc/uefi/uefi.rst | 21 -
  1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst
index b3494c22e073..76402bc5cfaa 100644
--- a/doc/uefi/uefi.rst
+++ b/doc/uefi/uefi.rst
@@ -180,6 +180,12 @@ Set up boot parameters on your board::

  efidebug boot add -b 1 HELLO mmc 0:1 /helloworld.efi.signed ""

+Since kernel 5.7 there's an alternative way of loading an initrd using
+LoadFile2 protocol if CONFIG_EFI_LOAD_FILE2_INITRD is enabled.
+The initrd path can be specified with::
+
+efidebug boot add -b ABE0 'kernel' mmc 0:1 Image -i mmc 0:1 initrd
+
  Now your board can run the signed image via the boot manager (see below).
  You can also try this sequence by running Pytest, test_efi_secboot,
  on the sandbox
@@ -484,7 +490,20 @@ The load file 2 protocol can be used by the Linux kernel 
to load the initial
  RAM disk. U-Boot can be configured to provide an implementation with::

  EFI_LOAD_FILE2_INITRD=y
-EFI_INITRD_FILESPEC=interface dev:part path_to_initrd


Thanks for adding the description. Some nitpicky comments below:


+
+When the options is enabled the user can add the initrd path with the efidebug


%s/options/option/


+command.
+The Boot option has a FilePathList[] in it's EFI_LOAD_OPTION.


Load options Boot have a FilePathList[] member.


+The first element of the array (FilePathList[0]) is the loded image.


is the EFI binary to execute.


+When an initrd is specified the Device Path for the initrd is denoted by a
+VenMedia node with the EFI_INITRD_MEDIA_GUID. Each entry of the array is
+terminated by the 'end of entire device path' subtype. If the user wants to


Below you have (0x01). So add (0xff) here.


+define multiple initrds those must by separated by the 'end of this instance'


%s/initrds/initrds,/


+identifier of the end node (0x01).
+
+So our final format of the FilePathList[] is::
+
+Loaded image - end node (0xff) - VenMedia - initrd1 - end node (0x01) 
initrd2 - end node (0xff)


Loaded image - end node (0xff) - VenMedia - initrd_1 -
[end node (0x01) - initrd_n ...] - end node (0xff)

There can be any number of initrds?

Best regards

Heinrich



  Links
  -