[U-Boot] [RFC, PATCH v4 05/16] env: fat: add U-Boot environment context support

[Wolfgang Denk]

Dear Takahiro,

In message <20190719083548.GU21948 at linaro.org> you wrote:
>
> > > +#if defined(CONFIG_ENV_FAT_INTERFACE) && \
> > > +	defined(CONFIG_ENV_FAT_DEVICE_AND_PART) && defined(CONFIG_ENV_FAT_FILE)
> > > +	[ENVCTX_UBOOT] = {
> > > +		CONFIG_ENV_FAT_INTERFACE,
> > > +		CONFIG_ENV_FAT_DEVICE_AND_PART,
> > > +		CONFIG_ENV_FAT_FILE,
> > > +	},
> > > +#endif
> > > +};
> > 
> > Does it make sense to define this in a FAT specific way? I guess we
> > could come up with a common structure that covers all supported
> > storage devices and use this here.
>
> But a different driver has different sets of configurations required.
> How can we *generalize* them?

Does it?  At least all block devices probably share the properties
of interface, device, partition and file name in one way or another.
Or offset instead of file name when accessing the raw device.

I think it should be possible to at least support all file systems
with exactly the sme code - ther eis nothing special about FAT here,
or am I missing something?

> > Also, the "#if defined" looks really dangerous to me, as missing
> > #defines will go unnoticed, and in the end you are accessing
> > uninitialized data... 
>
> Yes, I have made this mistake before.
> But I think that all the drivers must be verified in any case
> before any changes are applied to the upstream. No?

Indeed, testing will be needed.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If it happens once, it's a bug.
If it happens twice, it's a feature.
If it happens more than twice, it's a design philosophy.