2011/4/25 Barry Song <21cnbao@xxxxxxxxx>:
> 2011/4/21 Andrei Warkentin <andreiw@xxxxxxxxxxxx>:
>> Hi,
>>
>> On Thu, Apr 21, 2011 at 3:51 AM, Barry Song <21cnbao@xxxxxxxxx> wrote:
>>> From: Bin Shi <bin.shi@xxxxxxx>
>>>
>>> some controllers share data bus or other pins between
>>> multi-controllers and need to switch the functions of shared pins
>>> runtime
>>> this patch requested those shared pins before actual hardware access
>>> and release them after access
>>>
>>> Signed-off-by: Bin Shi <bin.shi@xxxxxxx>
>>> Cc: Binghua Duan <binghua.duan@xxxxxxx>
>>> Signed-off-by: Barry Song <21cnbao@xxxxxxxxx>
>>> ---
>>> Âdrivers/mmc/host/sdhci.c Â| Â 13 +++++++++++++
>>> Âdrivers/mmc/host/sdhci.h Â| Â Â2 ++
>>> Âinclude/linux/mmc/sdhci.h | Â Â2 ++
>>> Â3 files changed, 17 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index f31077d..7b07152 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -1379,6 +1379,13 @@ static void sdhci_tasklet_finish(unsigned long param)
>>> Â Â Â Â Â Â Â Âsdhci_reset(host, SDHCI_RESET_DATA);
>>> Â Â Â Â}
>>>
>>> + Â Â Â /*
>>> + Â Â Â Â* some controllers share data bus or other pins between multi-controller
>>> + Â Â Â Â* and need to switch the function of pins runtime
>>> + Â Â Â Â*/
>>> + Â Â Â if (host->quirks & SDHCI_QUIRK_SHARED_PINS)
>>> + Â Â Â Â Â Â Â host->ops->get_shared_pins(host);
>>> +
>>
>> Why in tasklet_finish? Why not in sdhci_request?
>
> that is ok in sdhci_request
>
>> No need to waste a quirk flag. Just invoke if method is not NULL.
>
> i think most hardwares have no shared pins issue, it should be a quirk
> but not a generic option for all devices.
>
>>
>> Also, I would assume host->ops->get_shared_pins would need to
>> synchronize with other SDHCI instances
>> it shares the pins with. Since you do it after the host->lock (as you
>> should), what happens if get_shared_pins needs to wait?
>
> sorry, just due to my cross-eye while porting the patch from old
> kernel to linux-mmc tree. In fact i mean:
>
> Â Â Â Â/*
> Â Â Â Â * some controllers share data bus or other pins between
> multi-controller
> Â Â Â Â * and need to switch the function of pins runtime
> Â Â Â Â */
> Â Â Â Âif (host->quirks & SDHCI_QUIRK_SHARED_PINS)
> Â Â Â Â Â Â Â Âhost->ops->get_shared_pins(host);
>
>
> Â Â Â Âmmc_request_done(host->mmc, mrq);
>
> Â Â Â Â/*
> Â Â Â Â * release shared pins so that other controllers can use them
> Â Â Â Â */
> Â Â Â Âif (host->quirks & SDHCI_QUIRK_SHARED_PINS)
> Â Â Â Â Â Â Â Âhost->ops->get_shared_pins(host);
>
In fact we request shared pins in sdhci_request() and release them in
tasklet_finish. Then i'll send patch v2.
>> Can you show the rest (sdhci driver implementing these hooks)?
>
> Yes. sdhci driver implement these hooks, special behavior of getting
> shared pins depends on special hardware, for us, we just request data
> bus mutex and set related hardware registers to switch the role of
> shared pins.
>
>>
>>> Â Â Â Âhost->mrq = NULL;
>>> Â Â Â Âhost->cmd = NULL;
>>> Â Â Â Âhost->data = NULL;
>>> @@ -1391,6 +1398,12 @@ static void sdhci_tasklet_finish(unsigned long param)
>>> Â Â Â Âspin_unlock_irqrestore(&host->lock, flags);
>>>
>>> Â Â Â Âmmc_request_done(host->mmc, mrq);
>>> +
>>> + Â Â Â /*
>>> + Â Â Â Â* release shared pins so that other controllers can use them
>>> + Â Â Â Â*/
>>> + Â Â Â if (host->quirks & SDHCI_QUIRK_SHARED_PINS)
>>> + Â Â Â Â Â Â Â host->ops->get_shared_pins(host);
>>> Â}
>>>
>>> Âstatic void sdhci_timeout_timer(unsigned long data)
>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>> index 85750a9..9d918a5 100644
>>> --- a/drivers/mmc/host/sdhci.h
>>> +++ b/drivers/mmc/host/sdhci.h
>>> @@ -229,6 +229,8 @@ struct sdhci_ops {
>>> Â Â Â Âvoid (*platform_send_init_74_clocks)(struct sdhci_host *host,
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â u8 power_mode);
>>>    Âunsigned int  Â(*get_ro)(struct sdhci_host *host);
>>> +    unsigned int  Â(*get_shared_pins)(struct sdhci_host *host);
>>> +    unsigned int  Â(*put_shared_pins)(struct sdhci_host *host);
>>> Â};
>>>
>>> Â#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
>>> diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
>>> index 83bd9f7..32ab422 100644
>>> --- a/include/linux/mmc/sdhci.h
>>> +++ b/include/linux/mmc/sdhci.h
>>> @@ -85,6 +85,8 @@ struct sdhci_host {
>>> Â#define SDHCI_QUIRK_NO_HISPD_BIT Â Â Â Â Â Â Â Â Â Â Â (1<<29)
>>> Â/* Controller treats ADMA descriptors with length 0000h incorrectly */
>>> Â#define SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC Â Â Â Â Â (1<<30)
>>> +/* Controller shared data bus or other pins with other controllers */
>>> +#define SDHCI_QUIRK_SHARED_PINS Â Â Â Â Â Â Â Â Â Â Â Â (1<<31)
>>>
>>> Â Â Â Âint irq; Â Â Â Â Â Â Â Â/* Device IRQ */
>>> Â Â Â Âvoid __iomem *ioaddr; Â /* Mapped address */
>>> --
>>> 1.7.1
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>> More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html
>>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
