diff mbox

mmc: dw_mmc: fix dw_mci_get_cd

Message ID 52D6938A.9050806@linaro.org
State Superseded
Headers show

Commit Message

Zhangfei Gao Jan. 15, 2014, 1:56 p.m. UTC 
On 01/15/2014 08:26 PM, Seungwon Jeon wrote:

>>>> @@ -1033,7 +1033,8 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
>>>>    	int present;
>>>>    	struct dw_mci_slot *slot = mmc_priv(mmc);
>>>>    	struct dw_mci_board *brd = slot->host->pdata;
>>>> -	int gpio_cd = !mmc_gpio_get_cd(mmc);
>>>> +	struct dw_mci *host = slot->host;
>>>> +	int gpio_cd = mmc_gpio_get_cd(mmc);
>>>>
>>>>    	/* Use platform get_cd function, else try onboard card detect */
>>>>    	if (brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
>>>> @@ -1041,11 +1042,12 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
>>>>    	else if (brd->get_cd)
>>>>    		present = !brd->get_cd(slot->id);
>>>>    	else if (!IS_ERR_VALUE(gpio_cd))
>>>> -		present = !!gpio_cd;
>>>> +		present = !gpio_cd;
>>> !!gpio_cd or gpio_cd is correct, isn't it?
>>>
>>
>> No, mmc_gpio_get_cd(mmc) has to revert.
> I'm missing something?
> If card is detected, mmc_gpio_get_cd() returns non-zero, right?
> I guess gpio_cd should be kept.
> 

Hmm, looks you are right.
Though not see clearly mmc_gpio_get_cd declaratoin, other drivers
directly set get_cd as mmc_gpio_get_cd.
.get_cd	= mmc_gpio_get_cd

However, in our board cd =0 when card is deteced while cd=1 when card is
removed.
In order to mmc_gpio_get_cd return 1, MMC_CAP2_CD_ACTIVE_HIGH has to be
set, as well as new property "caps2-mmc-cd-active-low".

int mmc_gpio_get_cd(struct mmc_host *host)
{
        return !gpio_get_value_cansleep(ctx->cd_gpio) ^
                !!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH);
}

Thanks

Comments

Zhangfei Gao Jan. 15, 2014, 3:36 p.m. UTC | #1 
On 01/15/2014 10:20 PM, Arnd Bergmann wrote:
> On Wednesday 15 January 2014 14:59:59 Arnd Bergmann wrote:
>> On Wednesday 15 January 2014 21:56:26 zhangfei wrote:
>>> However, in our board cd =0 when card is deteced while cd=1 when card is
>>> removed.
>>> In order to mmc_gpio_get_cd return 1, MMC_CAP2_CD_ACTIVE_HIGH has to be
>>> set, as well as new property "caps2-mmc-cd-active-low".
>>>
>>> --- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>>> +++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>>> @@ -73,6 +73,8 @@ Optional properties:
>>> +* caps2-mmc-cd-active-low: cd pin is low when card active
>>> +
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>> +       if (of_find_property(np, "caps2-mmc-cd-active-low", NULL))
>>> +               pdata->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
>>> +
>>
>> The MMC_CAP2_CD_ACTIVE_HIGH flag should only be required for
>> legacy platforms. With DT parsing, you can normally specify
>> the polarity of the GPIO line in the GPIO specifier in DT.
>
> I missed the fact that we already have a "cd-inverted" property
> as specified in bindings/mmc/mmc.txt. If your GPIO controller
> does not handle polarity, you can use that.
>

Thanks Arnd

GPIO controller drivers/gpio/gpio-pl061.c still not handle polarity, so 
use "cd-inverted" instead.
diff mbox

Patch

--- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
@@ -73,6 +73,8 @@  Optional properties:
+* caps2-mmc-cd-active-low: cd pin is low when card active
+

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
+       if (of_find_property(np, "caps2-mmc-cd-active-low", NULL))
+               pdata->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
+

But it looks strange "cd-active-low" describing "CD_ACTIVE_HIGH" flag.
When card active, cd = 0, and ACTIVE_HIGH is required to make
mmc_gpio_get_cd return 1.