[U-Boot] [PATCH V2 1/2] mmc: change magic number to macro define

Andy Fleming afleming at gmail.com
Tue Oct 11 18:37:37 CEST 2011


On Mon, Oct 10, 2011 at 8:12 PM, Graeme Russ <graeme.russ at gmail.com> wrote:
> Hi Prafulla,
>
>>> >
>>> > Hi Lei,
>>> > this is better, but what about structure-based access ?
>>> >
>>> > struct somrthing {
>>> >  u8 a1;
>>> >  u8 a2;
>>> > ...
>>> > };
>>> >
>>> > Like this.
>>> >
>>> > Also, CC Andy.
>>> >
>>>
>>> The ext_csd current usage in mmc.c is not too much, here I mean only few
>>> of
>>> the fields of the ext_csd is used, also fully definition of ext_csd
>>> member would seems so huge a structure at its appearence...
>>>
>>> So macro may looks more concise and could parse from its meaning easily
>>> enough.
>>>
>>> Anyway, more comments on this welcomes. :)
>>
>> Dear Lei
>> Using c-struct is our strategy, may be full definition is huge, you may skip them inserting padding in the c-struct.
>
> I personally prefer the complete struct to be defined from day one. This
> means you don't have to patch it when you end up needing to use another
> member later on and risk fouling up the offsets

One possible issue with defining a struct for the ext CSD is that the
512-byte structure changes depending on the version. This will require
great care to make sure the unions are all set up properly to cover
the various specs.

Linux uses a character array with index offsets, but parses the ext
csd into a software structure in the "card" object.

I don't object to using a struct, but we may find it's a bit
heavyweight. I do think it's ugly to pass in pointer math to
mmc_switch.  Let's hide it with a macro:

#define mmc_ext_csd_offset(x) (((struct ext_csd *)0)->x)

Which would remake that:

err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, mmc_ext_csd_offset(hs_timing), 1);


More information about the U-Boot mailing list