[U-Boot] [PATCH 1/4] fs: Add command to retrieve the filesystem type

[Sjoerd Simons]

On Mon, 2015-01-05 at 13:18 -0700, Stephen Warren wrote:
> On 01/05/2015 10:13 AM, Sjoerd Simons wrote:
> > New command to determine the filesystem type of a given partition.
> > Optionally stores the filesystem type in a environment variable.
> 
> > diff --git a/common/cmd_fs.c b/common/cmd_fs.c
> 
> > +U_BOOT_CMD(
> > +	fstype, 4, 1, do_fstype_wrapper,
> > +	"Look up a filesystem type",
> > +	"<interface> <dev>:<part>\n"
> 
> Should this line ...
> 
> > +	"- print filesystem type\n"
> > +	"fstype <interface> <dev>:<part> <varname>\n"
> 
> ... be consistent with this one - namely either both or neither include 
> "fstype" at the start?

Nope, the cmd_usage implementation does (summarized):

printf("Usage:\n%s ", cmdtp->name);
puts(cmdtp->help);
putc('\n');

So the "fstype" at the start of the first line gets added by that code,
hence the declaration needs to be inconsistent to have a consistent
output for the user :)

> > diff --git a/fs/fs.c b/fs/fs.c
> 
> > +int do_fs_type(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> > +{
> > +	struct fstype_info *info;
> > +
> > +	if (argc < 3 || argc > 4)
> > +		return CMD_RET_USAGE;
> > +
> > +	if (fs_set_blk_dev(argv[1], argv[2], FS_TYPE_ANY))
> > +		return 1;
> > +
> > +	info = fs_get_info(fs_type);
> > +
> > +	if (argc == 4)
> > +		setenv(argv[3], info->name);
> > +	else
> > +		printf("%s\n", info->name);
> > +
> > +	return CMD_RET_SUCCESS;
> > +}
> 
> That function has both the cmdline interface and implementation logic in 
> one place. Many of the other features (read, write, ls, ...) separate 
> them out into two functions so that U-Boot code can call the 
> implementation, and shell code can call the cmdline parsing wrapper. 
> Should we do the same here? Admittedly, the implementation would be 
> pretty simple, but perhaps it's still useful?

Ah i did wonder why the splitup was there in some functions, that
explains it :).

I would expect that u-boot code which would like to use it would be more
interested in getting the fstype struct rather then necessarily the name
as a string (or printed out).. But i could well be wrong. 

> I don't feel that strongly though, and we can easily refactor that later 
> if required.

Same here, although i'd slightly prefer refactoring if-needed rather
then pre-emptively :)

-- 
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/20150106/58c2687c/attachment.bin>