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

Lei Wen adrian.wenl at gmail.com
Tue Oct 11 02:48:41 CEST 2011


Hi Wolfgang,

On Tue, Oct 11, 2011 at 1:33 AM, Wolfgang Denk <wd at denx.de> wrote:
> Dear Lei Wen,
>
> In message <CALZhoSQbvKj0MtqryeHX-4LkvQJR2=B9u_m4yJjfM1mjv2MSEw at mail.gmail.com> you wrote:
>>
>> >> So macro may looks more concise and could parse from its meaning easily eno=
>> >> ugh.
>> >
>> > We do not accept (typeless) register offset definitions. Please use a
>> > struct, so the compiler has a chance to perform type checking.
>>
>> I check the code again, and find there is a reason for previous
>> defined macro to use.
>> That is those register offset defined as macro may need later passing
>> as a function
>> parameter like:
>>         err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, 1);
>
> I don;t see that any register (which now would be an address in a
> struct insteadd of being an offset to be added to a base address)
> would be passed as parameter to mmc_switch().
>
> So this is not an issue at all.
>
>> So if the ext_csd change to structure, maybe the function call here
>> don't looks like so
>> concise as before... What do you think for this?
>
> The EXT_CSD field definitions are completely independent of the way
> how you access the registers.
>

So do you means I should keep the EXT_CSD_HS_TIMING? Or I change like below?

from
err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, 1);
to:
err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, &ext_csd.hs_timing - &ext_csd, 1);

Best regards,
Lei


More information about the U-Boot mailing list