[U-Boot] [PATCH 3/6] sandbox: Implement host dev [device]

[Sjoerd Simons]

On Fri, 2015-02-20 at 12:23 -0700, Simon Glass wrote:
> Hi Sjoerd,
> 
> On 19 February 2015 at 15:41, Sjoerd Simons
> <sjoerd.simons at collabora.co.uk> wrote:
> > A common pattern to check if a certain device exists (e.g. in
> > config_distro_bootcmd) is to use: <interface> dev [device]
> >
> > Implement host dev [device] so this pattern can be used for sandbox host
> > devices.
> 
> I don't see where this actually affects anything? In other words, does
> it really use the device you select, or just ignore you?

It gets ignored, it's only real usage is to detect whether a device
exists. 

To step back the reason I implemented it here is just to make it simpler
to integrate with the command boot commands as it's common blockdevice
macro is:

#define BOOTENV_SHARED_BLKDEV_BODY(devtypel) \
	"if " #devtypel " dev ${devnum}; then " \
		"setenv devtype " #devtypel "; " \
		"run scan_dev_for_boot_part; " \
	"fi\0"

Which follows exactly that pattern. 

> > Signed-off-by: Sjoerd Simons <sjoerd.simons at collabora.co.uk>
> > ---
> >  common/cmd_sandbox.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 48 insertions(+)
> >
> > diff --git a/common/cmd_sandbox.c b/common/cmd_sandbox.c
> > index 4286969..7447d43 100644
> > --- a/common/cmd_sandbox.c
> > +++ b/common/cmd_sandbox.c
> > @@ -10,6 +10,8 @@
> >  #include <sandboxblockdev.h>

> > +       ret = host_get_dev_err(dev, &blk_dev);
> > +       if (ret) {
> > +               if (ret == -ENOENT)
> > +                       puts("Not bound to a backing file\n");
> 
> Just use printf(), we should avoid puts() now.

ok

> > +               else if (ret == -ENODEV)
> > +                       puts("Invalid host device number\n");
> > +
> > +               return 1;
> > +       }
> > +
> > +       sandbox_curr_device = dev;
> > +       return 0;
> > +}
> > +
> > +U_BOOT_CMD(
> > +       host, 3, 2, do_host_dev,
> > +       "Set or get current host device", "dev [dev] - Set or retrieve the current host device"
> 
> Can we make this command part of the 'sb' command? Like 'sh host ...'.

In principle, sure, however that breaks the consistency with other
commands which all use <interface> <interface-commands>, where
<interface> matches the names used in the fs and partition commands.
Which is exactly the consistency _distro_bootcmd takes advantage of in
the macro i mentioned above.

My thinking was that the sb set of commands are used to manage/setup the
sandbox interface (e.g. setting up the host device bindings) while in
this case the host commands are for the more standard interaction with
host interface devices.


Another way of addressing this and essentially side-stepping this
discussion would be to add a a new generic dev command to allow testing
device existance e.g.
  dev exists host 0
  dev exists mmc 0

And switch _distro_bootcmd to that instead of it relying on all block
interfaces having a dev subcommand with the same semantics.

Opinions ? :)
-- 
Sjoerd Simons <sjoerd.simons at collabora.co.uk>
Collabora Ltd.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 6170 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150228/b86906a7/attachment.bin>