Alex Dubov wrote:
>> tifm_sd_fetch_resp() could be redone as a for loop
>> to make it more
>> obvious what's going on.
>>
> I'm not sure it's a good idea. The response value is
> returned in 8 16-bit registers, which are mapped over
> 8 32-bit registers (so that only LS part of each
> register is valid). Additionally, the fetch order is
> reversed, so cmd->resp[0] is fetched from offsets 24
> and 28, while cmd->resp[3] is fetched from offsets 0
> and 4. To write this as a loop requires moderately
> complex address calculation that may look even less
> obvious.
>
>
I suppose it's a matter of taste, but personally I think the mere
mentioning of 'for' allows you to directly see that there is some kind
of looping involved. And it shouldn't be terribly complex:
for (i = 0;i < 8;i++) {
resp[i] = readw(addr + RESPONSE + (7 - i)*4) << 16;
resp[i] |= readw(addr + RESPONSE + (6 - i)*4);
}
>> You should probably rename tifm_sd_set_data_to(). It
>> isn't obvious that
>> 'to' stands for 'timeout'. Same thing with other
>> instances of 'to'.
>>
> I agree, yet I wanted to retain the names of the
> registers as defined in datasheet (so it's easier to
> search for them; for some reason it always abbreviates
> timeout as TO). Apparently TI does the same in their
> drivers.
>
>
The problem is that it's a big difference between seeing "data TO" and
seeing "data to" in the code. How about using the three letter
abbreviations in those places? I.e. "cto" and "dto"?
>> What I'd like to see from you is to double check
>> that bytes_xfered is
>> set to the number of bytes successfully sent to the
>> _card_, not the
>> controller. This is critical for correct handling of
>> bus errors.
>>
> The OMAP datasheet is somewhat unclear, but I think
> that block and byte counters truly represent the
> amount of data shifted out to the mmc bus. Whether
> this data really reaches the flash memory I don't know
> to tell.
>
>
Hmm.... I'm planning on putting together a test module to determine this
(as my specs aren't crystal clear on the subject either). I'll try to
remember to send it to you. :)
Rgds
Pierre
--
VGER BF report: U 0.5
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
