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

Graeme Russ graeme.russ at gmail.com
Tue Oct 11 03:09:31 CEST 2011


Hi Lei Wen,

On Tue, Oct 11, 2011 at 11:48 AM, Lei Wen <adrian.wenl at gmail.com> wrote:
> 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);

I imagine the compiler will complain that the types for &ext_csd.hs_timing
and &ext_csd are different.

Try:
	struct ext_csd *ext_csd_ptr = 0;

	err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, &ext_csd_ptr->hs_timing, 1);

Regards,

Graeme


More information about the U-Boot mailing list