On 09/21/2010 11:44 AM, Wolfram Sang wrote:
> On Tue, Sep 21, 2010 at 11:21:48AM +0200, Peppe CAVALLARO wrote:
>> On 09/20/2010 10:38 AM, Wolfram Sang wrote:
>>> On Mon, Sep 20, 2010 at 10:20:17AM +0200, Giuseppe CAVALLARO wrote:
>>>> This patch adds the Arasan MMC/SD/SDIO driver
>>>> for the STM ST40 platforms. It is based on the
>>>> SDHCI driver.
>>>> It has been tested on the STx7106/STx7108/STx5289
>>>> platforms.
>>>>
>>>> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@xxxxxx>
>>>
>>> Looks to me that you could just use the sdhci-pltfm driver?
>>>
>>
>> Hello Wolfram,
>> some weeks ago I discussed about this driver and I reworked it according
>> to the changes required. This is the meaning of this patch.
>>
>> At any rate, I can look at the sdhci-pltfm driver: at first glance, it
>> could actually help on our platforms. This could take a while.
>
> Sorry, I didn't spot the first version of your patch. I would have said
> the same then, simply to avoid the code duplication. Oh, and no need to
> hurry from my side :)
Hi Wolfran,
I agree that it's a good idea to reduce duplicated code when possible
(i.e. the probe function that, at first glance, is almost equal for
several sdhci drivers based).
Maybe, I could use on our ST boxes the sdhci_pltfm but I have the
following questions and ideas. Welcome advice and feedback.
1) I've already a patch to add the suspend/resume in the sdhci_pltfm
driver. Please note this is mandatory for me.
Note: I'd like to look at the wake-up on card that should be nice to
have in the future. IIUC, it is missing in the sdhci. Please correct
me if I'm wrong.
2) sdhci_pltfm_data has a "quirk" flag but IMO the quirk macros, that
currently are in linux-2.6/drivers/mmc/host/sdhci.h, should be
moved in a separate file:
include/linux/mmc/sdhci.h or
include/linux/mmc/sdhci-quirk.h or ...
I don't know if it has been already done but I could create a patch
for this too. Let me know the name convention you like, eventually.
Otherwise, in my platforms, where I need to set this flag (e.g. the
sdhci-stm needs: SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC), I should include
drivers/mmc/host/sdhci.h?!? I don't like it :-(
Please, correct me if I've missed something.
3) In the end, another hook could be added in the sdhci_pltfm_data to
invoke specific own functions for claiming resources etc.
For example, I need an extra callback to invoke the STM pad manager
that's used for managing clocks, PIO lines and syscfg registers.
I'm thinking about something like this:
struct sdhci_pltfm_data {
struct sdhci_ops *ops;
unsigned int quirks;
int (*init)(struct sdhci_host *host);
void (*exit)(struct sdhci_host *host);
int (*claim_resource)(struct platform_device *pdev);
|
|_ we can use another name.
};
What do you think?
Regards
Peppe
--
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
