[PATCH] bootstd: USB devtype detection for script boot

[Simon Glass]

Hi John,

On Thu, 29 Jun 2023 at 01:03, John Clark <inindev at 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 at 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 at 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