[PATCH] bootstd: USB devtype detection for script boot

[PATCH] bootstd: USB devtype detection for script boot

[John Clark]

Change the device type from "usb_mass_storage" to "usb" when
booting a script.

Before this change:
  => printenv devtype
  devtype=usb_mass_storage

After this change:
  => printenv devtype
  devtype=usb

Signed-off-by: John Clark <inindev@gmail.com>
---

 boot/bootmeth_script.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/boot/bootmeth_script.c b/boot/bootmeth_script.c
index 225eb18ee6..9fdadb3005 100644
--- a/boot/bootmeth_script.c
+++ b/boot/bootmeth_script.c
@@ -187,10 +187,14 @@ static int script_set_bootflow(struct udevice *dev, struct bootflow *bflow,
 static int script_boot(struct udevice *dev, struct bootflow *bflow)
 {
 	struct blk_desc *desc = dev_get_uclass_plat(bflow->blk);
+	const char *devtype = blk_get_devtype(bflow->blk);
 	ulong addr;
 	int ret;
 
-	ret = env_set("devtype", blk_get_devtype(bflow->blk));
+	if (!strcmp("usb_mass_storage", devtype))
+		ret = env_set("devtype", "usb");
+	else
+		ret = env_set("devtype", devtype);
 	if (!ret)
 		ret = env_set_hex("devnum", desc->devnum);
 	if (!ret)
@@ -198,7 +202,7 @@ static int script_boot(struct udevice *dev, struct bootflow *bflow)
 	if (!ret)
 		ret = env_set("prefix", bflow->subdir);
 	if (!ret && IS_ENABLED(CONFIG_ARCH_SUNXI) &&
-	    !strcmp("mmc", blk_get_devtype(bflow->blk)))
+	    !strcmp("mmc", devtype))
 		ret = env_set_hex("mmc_bootdev", desc->devnum);
 	if (ret)
 		return log_msg_ret("env", ret);
-- 
2.39.2

Re: [PATCH] bootstd: USB devtype detection for script boot

[Simon Glass]

Hi John,

On Tue, 27 Jun 2023 at 15:39, John Clark <inindev@gmail.com> wrote:
>
> Change the device type from "usb_mass_storage" to "usb" when
> booting a script.
>
> Before this change:
>   => printenv devtype
>   devtype=usb_mass_storage
>
> After this change:
>   => printenv devtype
>   devtype=usb
>
> Signed-off-by: John Clark <inindev@gmail.com>
> ---
>
>  boot/bootmeth_script.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/boot/bootmeth_script.c b/boot/bootmeth_script.c
> index 225eb18ee6..9fdadb3005 100644
> --- a/boot/bootmeth_script.c
> +++ b/boot/bootmeth_script.c
> @@ -187,10 +187,14 @@ static int script_set_bootflow(struct udevice *dev, struct bootflow *bflow,
>  static int script_boot(struct udevice *dev, struct bootflow *bflow)
>  {
>         struct blk_desc *desc = dev_get_uclass_plat(bflow->blk);
> +       const char *devtype = blk_get_devtype(bflow->blk);
>         ulong addr;
>         int ret;
>
> -       ret = env_set("devtype", blk_get_devtype(bflow->blk));
> +       if (!strcmp("usb_mass_storage", devtype))

I only just thought of this, but I think it is better to check
blk->uclass_id == UCLASS_USB instead, since it avoids a string
comparison.

> +               ret = env_set("devtype", "usb");
> +       else
> +               ret = env_set("devtype", devtype);
>         if (!ret)
>                 ret = env_set_hex("devnum", desc->devnum);
>         if (!ret)
> @@ -198,7 +202,7 @@ static int script_boot(struct udevice *dev, struct bootflow *bflow)
>         if (!ret)
>                 ret = env_set("prefix", bflow->subdir);
>         if (!ret && IS_ENABLED(CONFIG_ARCH_SUNXI) &&
> -           !strcmp("mmc", blk_get_devtype(bflow->blk)))
> +           !strcmp("mmc", devtype))
>                 ret = env_set_hex("mmc_bootdev", desc->devnum);
>         if (ret)
>                 return log_msg_ret("env", ret);
> --
> 2.39.2
>

Regards,
Simon

Re: [PATCH] bootstd: USB devtype detection for script boot

[Simon Glass]

Hi John,

On Thu, 29 Jun 2023 at 01:03, John Clark <inindev@gmail.com> wrote:
>
>  > I only just thought of this, but I think it is better to check
> blk->uclass_id == UCLASS_USB instead, since it avoids a string comparison.
>
> I am not finding anything as direct as your pseudo code above suggests.
> The closest I can find is a compare like this:
>
> UCLASS_MASS_STORAGE == device_get_uclass_id(dev_get_parent(bflow->blk))
>
> Which has the overhead of calling two new functions that otherwise would
> not be called and could match other mass storage devices other than USB
> (am I right to assume this?).
>
> Can you shed additional insight into "blk->uclass_id"?

Oh sorry, I mean the blk_desc structure attached to the block device.

struct blk_desc *desc = dev_get_uclass_plat(blk);

Then desc->uclass_id is what you want. It is the uclass of the media
device (typically the parent of the block device).

>
> blk is a udevice without a uclass_id member:
>
> ./dm/device.h
>
> struct udevice {
>          const struct driver *driver;
>          const char *name;
>          void *plat_;
>          void *parent_plat_;
>          void *uclass_plat_;
>          ulong driver_data;
>          struct udevice *parent;
>          void *priv_;
>          struct uclass *uclass;
>          void *uclass_priv_;
>          void *parent_priv_;
>          struct list_head uclass_node;
>          struct list_head child_head;
>          struct list_head sibling_node;
> #if !CONFIG_IS_ENABLED(OF_PLATDATA_RT)
>          u32 flags_;
> #endif
>          int seq_;
> #if CONFIG_IS_ENABLED(OF_REAL)
>          ofnode node_;
> #endif
> #if CONFIG_IS_ENABLED(DEVRES)
>          struct list_head devres_head;
> #endif
> #if CONFIG_IS_ENABLED(DM_DMA)
>          ulong dma_offset;
> #endif
> #if CONFIG_IS_ENABLED(IOMMU)
>          struct udevice *iommu;
> #endif
> };

You can use device_get_uclass_id(blk) to get the uclass ID of the
block device, but of course it will just be UCLASS_BLK.

>
>
> I am not a prolific open source contributor, so I am not sure if a
> side-bar conversation like this is one we should be having with the
> whole list?  Something this trivial seems like it would just be noise to
> include hundreds of people on.
>
> I appreciate your advice here, and thanks for your time.

You can just copy the list on all emails. I've done it here.

Regards,
Simon


>
>
> John Clark
>
>
> On 6/28/23 3:42 AM, Simon Glass wrote:
> > Hi John,
> >
> > On Tue, 27 Jun 2023 at 15:39, John Clark <inindev@gmail.com> wrote:
> >> Change the device type from "usb_mass_storage" to "usb" when
> >> booting a script.
> >>
> >> Before this change:
> >>    => printenv devtype
> >>    devtype=usb_mass_storage
> >>
> >> After this change:
> >>    => printenv devtype
> >>    devtype=usb
> >>
> >> Signed-off-by: John Clark <inindev@gmail.com>
> >> ---
> >>
> >>   boot/bootmeth_script.c | 8 ++++++--
> >>   1 file changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/boot/bootmeth_script.c b/boot/bootmeth_script.c
> >> index 225eb18ee6..9fdadb3005 100644
> >> --- a/boot/bootmeth_script.c
> >> +++ b/boot/bootmeth_script.c
> >> @@ -187,10 +187,14 @@ static int script_set_bootflow(struct udevice *dev, struct bootflow *bflow,
> >>   static int script_boot(struct udevice *dev, struct bootflow *bflow)
> >>   {
> >>          struct blk_desc *desc = dev_get_uclass_plat(bflow->blk);
> >> +       const char *devtype = blk_get_devtype(bflow->blk);
> >>          ulong addr;
> >>          int ret;
> >>
> >> -       ret = env_set("devtype", blk_get_devtype(bflow->blk));
> >> +       if (!strcmp("usb_mass_storage", devtype))
> > I only just thought of this, but I think it is better to check
> > blk->uclass_id == UCLASS_USB instead, since it avoids a string
> > comparison.
> >
> >> +               ret = env_set("devtype", "usb");
> >> +       else
> >> +               ret = env_set("devtype", devtype);
> >>          if (!ret)
> >>                  ret = env_set_hex("devnum", desc->devnum);
> >>          if (!ret)
> >> @@ -198,7 +202,7 @@ static int script_boot(struct udevice *dev, struct bootflow *bflow)
> >>          if (!ret)
> >>                  ret = env_set("prefix", bflow->subdir);
> >>          if (!ret && IS_ENABLED(CONFIG_ARCH_SUNXI) &&
> >> -           !strcmp("mmc", blk_get_devtype(bflow->blk)))
> >> +           !strcmp("mmc", devtype))
> >>                  ret = env_set_hex("mmc_bootdev", desc->devnum);
> >>          if (ret)
> >>                  return log_msg_ret("env", ret);
> >> --
> >> 2.39.2
> >>
> > Regards,
> > Simon

[PATCH] bootstd: USB devtype detection for script boot

[John Clark]

Change the device type from "usb_mass_storage" to "usb" when
booting a script.

Before this change:
   => printenv devtype
   devtype=usb_mass_storage

After this change:
   => printenv devtype
   devtype=usb

Signed-off-by: John Clark <inindev@gmail.com>
---

 boot/bootmeth_script.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/boot/bootmeth_script.c b/boot/bootmeth_script.c
index 225eb18ee6..a4050c384d 100644
--- a/boot/bootmeth_script.c
+++ b/boot/bootmeth_script.c
@@ -190,7 +190,10 @@ static int script_boot(struct udevice *dev, struct bootflow *bflow)
 	ulong addr;
 	int ret;
 
-	ret = env_set("devtype", blk_get_devtype(bflow->blk));
+	if (desc->uclass_id == UCLASS_USB)
+		ret = env_set("devtype", "usb");
+	else
+		ret = env_set("devtype", blk_get_devtype(bflow->blk));
 	if (!ret)
 		ret = env_set_hex("devnum", desc->devnum);
 	if (!ret)
-- 
2.39.2

Re: [PATCH] bootstd: USB devtype detection for script boot

[Simon Glass]

On Fri, 30 Jun 2023 at 18:50, John Clark <inindev@gmail.com> wrote:
>
> Change the device type from "usb_mass_storage" to "usb" when
> booting a script.
>
> Before this change:
>    => printenv devtype
>    devtype=usb_mass_storage
>
> After this change:
>    => printenv devtype
>    devtype=usb
>
> Signed-off-by: John Clark <inindev@gmail.com>
> ---
>
>  boot/bootmeth_script.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>
Fixes d9409244 ("bootstd: Add an implementation of script boot")

Re: [PATCH] bootstd: USB devtype detection for script boot

[Simon Glass]

On Sat, 1 Jul 2023 at 20:10, Simon Glass <sjg@chromium.org> wrote:
>
> On Fri, 30 Jun 2023 at 18:50, John Clark <inindev@gmail.com> wrote:
> >
> > Change the device type from "usb_mass_storage" to "usb" when
> > booting a script.
> >
> > Before this change:
> >    => printenv devtype
> >    devtype=usb_mass_storage
> >
> > After this change:
> >    => printenv devtype
> >    devtype=usb
> >
> > Signed-off-by: John Clark <inindev@gmail.com>
> > ---
> >
> >  boot/bootmeth_script.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Fixes d9409244 ("bootstd: Add an implementation of script boot")

I mean:
Fixes: d9409244 ("bootstd: Add an implementation of script boot")

Re: [PATCH] bootstd: USB devtype detection for script boot

[Simon Glass]

On Sat, 1 Jul 2023 at 13:12, Simon Glass <sjg@chromium.org> wrote:
>
> On Sat, 1 Jul 2023 at 20:10, Simon Glass <sjg@chromium.org> wrote:
> >
> > On Fri, 30 Jun 2023 at 18:50, John Clark <inindev@gmail.com> wrote:
> > >
> > > Change the device type from "usb_mass_storage" to "usb" when
> > > booting a script.
> > >
> > > Before this change:
> > >    => printenv devtype
> > >    devtype=usb_mass_storage
> > >
> > > After this change:
> > >    => printenv devtype
> > >    devtype=usb
> > >
> > > Signed-off-by: John Clark <inindev@gmail.com>
> > > ---
> > >
> > >  boot/bootmeth_script.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > Fixes d9409244 ("bootstd: Add an implementation of script boot")
>
> I mean:
> Fixes: d9409244 ("bootstd: Add an implementation of script boot")

Applied to u-boot-dm, thanks!