[U-Boot] [PATCH] mmc: Implement card detection.

[Andy Fleming]

On Mon, Nov 28, 2011 at 11:47 AM, Thierry Reding
<thierry.reding at avionic-design.de> wrote:
>>
>> Also, to handle code like is in fsl_esdhc.c for fallback in case no
>> board-specific code is written, I'm thinking we should use a mechanism
>> similar to the ethernet drivers:
>>
>>         if (board_eth_init != __def_eth_init) {
>>                 if (board_eth_init(bis) < 0)
>>                         printf("Board Net Initialization Failed\n");
>>         } else if (cpu_eth_init != __def_eth_init) {
>>                 if (cpu_eth_init(bis) < 0)
>>                         printf("CPU Net Initialization Failed\n");
>>
>> Basically, if the board_eth_init is set to something, call it.
>> Otherwise, call the cpu_eth_init, if it exists.
>>
>> So we could have:
>>
>> int mmc_getcd(mmc, &cd) // mmc should always have been the first argument...
>
> Yes, the cd parameter really should be second, or, as you propose later, not
> be a parameter at all.
>
>> {
>>     if (board_mmc_getcd != __def_mmc_getcd)
>>         return board_mmc_getcd(mmc, cd);
>>     else if (cpu_mmc_getcd != __def_mmc_getcd)
>>         return cpu_mmc_getcd(mmc, cd);
>>
>>     return -1;
>> }
>
> I don't see how that buys us much. What's wrong with the following? That's
> much more straightforward in my opinion.
>
>        int mmc_getcd(struct mmc *mmc)
>        {
>                int cd;
>
>                cd = board_mmc_getcd(mmc);
>                if (cd < 0)
>                        cd = cpu_mmc_getcd(mmc);
>
>                return cd;
>        }
>
> I don't have any real objections, though. It's probably more of a matter of
> taste than technical merit. The only advantage that the second version has is
> that it allows the cpu_mmc_getcd() to serve as fallback if board_mmc_getcd()
> is implemented but still returns -1 (not implemented, for whatever reason).


That's a good point. I don't have any issues with this implementation.


>
>> Open questions:
>> 1) If we use this sort of mechanism, we don't really need the "-1
>> means it's not implemented" error. Perhaps we could change it to be:
>>     cd = mmc_getcd(mmc) ?
>
> That can even work if we keep -1 to mean "not implemented".
>
>> 2) Should we add the ability for *drivers* to specify a card detect
>> mechanism, and if so, where should it fit in the priority order? Most
>> systems seem to provide GPIOs for the card detection, and even the
>> controllers which support card detection from the device are often not
>> always hooked up in such a way to actually *use* that support.
>
> I'm not sure I understand what cpu_mmc_getcd() is supposed to mean. Shouldn't
> that really be the driver default implementation instead? Typically the CPU
> implementation will be the SoC implementation, right? In that case it would
> really be the driver card-detect mechanism, not that of the CPU.


Well, while this isn't the case in any systems I know of now, it is
quite possible for there to be more than one *type* of SD/MMC
controller on an SoC, and there's always the possibility that an SoC
provides a non-controller-specific card-detect mechanism. The idea is
that, lacking a board-specific card-detect mechanism, the SoC might be
able to direct the query to the right place.

But I'm talking very theoretically, here. I wouldn't object to a
mechanism that was just:

cd = board_mmc_getcd(mmc);

if (cd < 0 && mmc->getcd)
   cd = mmc->getcd(mmc);

If we ever ran into a case where an SoC had better knowledge than the
driver, then it's easy to fix the code.

Andy