Re: Support for TI FlashMedia (pci id 104c:8033, 104c:803b) flash card readers

[Alex Dubov <oakad@yahoo.com>]

> The constants you've borrowed from OMAP, have you
> confirmed all of them?
All the constants defined in tifm_sd.c are used in
accordance to OMAP datasheet with expected effect.
Constants that I've guessed mostly reside in the
tifm.h file and marked accordingly.

> 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.

> 
> 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.

> 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.


__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

-- 
VGER BF report: U 0.499996
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/