Re: [PATCH] sdhci: add quirk to support shared bus controller

[Barry Song <21cnbao@gmail.com>]

2011/4/21 Andrei Warkentin <andreiw@motorola.com>:
> Hi,
>
> On Thu, Apr 21, 2011 at 3:51 AM, Barry Song <21cnbao@gmail.com> wrote:
>> From: Bin Shi <bin.shi@csr.com>
>>
>> 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@csr.com>
>> Cc: Binghua Duan <binghua.duan@csr.com>
>> Signed-off-by: Barry Song <21cnbao@gmail.com>
>> ---
>>  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);

> 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@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>