On Mon, Sep 27, 2010 at 12:09 AM, Wolfram Sang <w.sang@xxxxxxxxxxxxxx> wrote:
> On Sun, Sep 26, 2010 at 04:52:06AM -0400, zhangfei gao wrote:
>> Add several call back to sdhci-pltfm.c, help give suggestion
>
> Just formal things, without looking at the code yet.
>
> Make seperate patches out of these, everyone with a proper description.
>
>> 1. struct sdhci_host *(*alloc_host)(struct device *dev), since
>> specific driver need some private variable to allocate and free,
>> including clk.
>> 2. unsigned int Â(*get_quirk)(struct sdhci_host *host); add this
>> because one driver may serve several device, each one may require
>> different quirk, and specific driver is right one to know.
>> 3. void (*set_max_speed)(struct sdhci_host *host); this should be done
>> after add_host, which impact f_max.
>> 4. int (*init)(struct sdhci_host *host, struct sdhci_pltfm_data
>> *pdata, void* priv_pdata); copy from Wolfram Sang's patch to transfer
>> platform data, copy here for test.
>
Just a rough idea, considering the potential differences (I believe
there will be
more in the future) between the SDHCI of different SoCs, would it make more
sense to make sdhci-pltfm.c as a common function library for sdhci-<soc>.c?
Or from OO point of view (though I don't quite like OO), the sdhci-<soc> is
actually a derivative of sdhci-pltfm, which by concept, sdhci-<soc> could be
an individual driver, and can call its super functions in sdhci-pltfm. E.g. in
sdhci_<soc>_probe(), one can call sdhci_pltfm_probe()?
The platform driver version may not be able to resemble the pci version so
just provide an alternate way of solving this issue.
> No copying. If you think my patch is useful, add an Acked-by and say your
> patches depend on mine.
>
>>
>> From 8c59d0b917f434d7c424ce1e0896af45c3e5fbba Mon Sep 17 00:00:00 2001
>> From: Zhangfei Gao <zhangfei.gao@xxxxxxxxxxx>
>> Date: Sun, 26 Sep 2010 16:37:11 -0400
>> Subject: [PATCH 2/2] sdhci-pltfm: add call back function
>>
>>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@xxxxxxxxxxx>
>> ---
>> Âdrivers/mmc/host/sdhci-pltfm.c | Â 24 ++++++++++++++++++------
>> Âinclude/linux/sdhci-pltfm.h  Â|  Â5 ++++-
>> Â2 files changed, 22 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
>> index e045e3c..e06f047 100644
>> --- a/drivers/mmc/host/sdhci-pltfm.c
>> +++ b/drivers/mmc/host/sdhci-pltfm.c
>> @@ -54,12 +54,17 @@ static int __devinit sdhci_pltfm_probe(struct
>> platform_device *pdev)
>> Â{
>> Â Â Â struct sdhci_pltfm_data *pdata = pdev->dev.platform_data;
>> Â Â Â const struct platform_device_id *platid = platform_get_device_id(pdev);
>> + Â Â void *priv_pdata = NULL;
>> Â Â Â struct sdhci_host *host;
>> Â Â Â struct resource *iomem;
>> Â Â Â int ret;
>>
>> - Â Â if (!pdata && platid && platid->driver_data)
>> + Â Â if (platid && platid->driver_data) {
>> Â Â Â Â Â Â Â pdata = (void *)platid->driver_data;
>> + Â Â Â Â Â Â priv_pdata = pdev->dev.platform_data;
>> + Â Â } else {
>> + Â Â Â Â Â Â pdata = pdev->dev.platform_data;
>> + Â Â }
>>
>> Â Â Â iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> Â Â Â if (!iomem) {
>> @@ -71,8 +76,8 @@ static int __devinit sdhci_pltfm_probe(struct
>> platform_device *pdev)
>> Â Â Â Â Â Â Â dev_err(&pdev->dev, "Invalid iomem size. You may "
>> Â Â Â Â Â Â Â Â Â Â Â "experience problems.\n");
>>
>> - Â Â if (pdev->dev.parent)
>> - Â Â Â Â Â Â host = sdhci_alloc_host(pdev->dev.parent, 0);
>> + Â Â if (pdata && pdata->alloc_host)
>> + Â Â Â Â Â Â host = pdata->alloc_host(&pdev->dev);
>> Â Â Â else
>> Â Â Â Â Â Â Â host = sdhci_alloc_host(&pdev->dev, 0);
>>
>> @@ -86,8 +91,7 @@ static int __devinit sdhci_pltfm_probe(struct
>> platform_device *pdev)
>> Â Â Â Â Â Â Â host->ops = pdata->ops;
>> Â Â Â else
>> Â Â Â Â Â Â Â host->ops = &sdhci_pltfm_ops;
>> - Â Â if (pdata)
>> - Â Â Â Â Â Â host->quirks = pdata->quirks;
>> +
>> Â Â Â host->irq = platform_get_irq(pdev, 0);
>>
>> Â Â Â if (!request_mem_region(iomem->start, resource_size(iomem),
>> @@ -105,15 +109,23 @@ static int __devinit sdhci_pltfm_probe(struct
>> platform_device *pdev)
>> Â Â Â }
>>
>> Â Â Â if (pdata && pdata->init) {
>> - Â Â Â Â Â Â ret = pdata->init(host);
>> + Â Â Â Â Â Â ret = pdata->init(host, pdata, priv_pdata);
>> Â Â Â Â Â Â Â if (ret)
>> Â Â Â Â Â Â Â Â Â Â Â goto err_plat_init;
>> Â Â Â }
>>
>> + Â Â if (pdata && pdata->get_quirk)
>> + Â Â Â Â Â Â host->quirks = pdata->get_quirk(host);
>> + Â Â else if (pdata)
>> + Â Â Â Â Â Â host->quirks = pdata->quirks;
>> +
>> Â Â Â ret = sdhci_add_host(host);
>> Â Â Â if (ret)
>> Â Â Â Â Â Â Â goto err_add_host;
>>
>> + Â Â if (pdata && pdata->set_max_speed)
>> + Â Â Â Â Â Â pdata->set_max_speed(host);
>> +
>> Â Â Â platform_set_drvdata(pdev, host);
>>
>> Â Â Â return 0;
>> diff --git a/include/linux/sdhci-pltfm.h b/include/linux/sdhci-pltfm.h
>> index 0239bd7..0d8e8f6 100644
>> --- a/include/linux/sdhci-pltfm.h
>> +++ b/include/linux/sdhci-pltfm.h
>> @@ -28,8 +28,11 @@ struct sdhci_host;
>> Âstruct sdhci_pltfm_data {
>> Â Â Â struct sdhci_ops *ops;
>> Â Â Â unsigned int quirks;
>> - Â Â int (*init)(struct sdhci_host *host);
>> + Â Â int (*init)(struct sdhci_host *host, struct sdhci_pltfm_data *pdata,
>> void* priv_pdata);
>> Â Â Â void (*exit)(struct sdhci_host *host);
>> + Â Â void (*set_max_speed)(struct sdhci_host *host);
>> + Â Â unsigned int Â(*get_quirk)(struct sdhci_host *host);
>> + Â Â struct sdhci_host *(*alloc_host)(struct device *dev);
>> Â};
>>
>> Â#endif /* _SDHCI_PLTFM_H */
>> --
>> 1.7.0.4
>
>> From 8c59d0b917f434d7c424ce1e0896af45c3e5fbba Mon Sep 17 00:00:00 2001
>> From: Zhangfei Gao <zhangfei.gao@xxxxxxxxxxx>
>> Date: Sun, 26 Sep 2010 16:37:11 -0400
>> Subject: [PATCH 2/2] sdhci-pltfm: add call back function
>>
>>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@xxxxxxxxxxx>
>> ---
>> Âdrivers/mmc/host/sdhci-pltfm.c | Â 24 ++++++++++++++++++------
>> Âinclude/linux/sdhci-pltfm.h  Â|  Â5 ++++-
>> Â2 files changed, 22 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
>> index e045e3c..e06f047 100644
>> --- a/drivers/mmc/host/sdhci-pltfm.c
>> +++ b/drivers/mmc/host/sdhci-pltfm.c
>> @@ -54,12 +54,17 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
>> Â{
>> Â Â Â struct sdhci_pltfm_data *pdata = pdev->dev.platform_data;
>> Â Â Â const struct platform_device_id *platid = platform_get_device_id(pdev);
>> + Â Â void *priv_pdata = NULL;
>> Â Â Â struct sdhci_host *host;
>> Â Â Â struct resource *iomem;
>> Â Â Â int ret;
>>
>> - Â Â if (!pdata && platid && platid->driver_data)
>> + Â Â if (platid && platid->driver_data) {
>> Â Â Â Â Â Â Â pdata = (void *)platid->driver_data;
>> + Â Â Â Â Â Â priv_pdata = pdev->dev.platform_data;
>> + Â Â } else {
>> + Â Â Â Â Â Â pdata = pdev->dev.platform_data;
>> + Â Â }
>>
>> Â Â Â iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> Â Â Â if (!iomem) {
>> @@ -71,8 +76,8 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
>> Â Â Â Â Â Â Â dev_err(&pdev->dev, "Invalid iomem size. You may "
>> Â Â Â Â Â Â Â Â Â Â Â "experience problems.\n");
>>
>> - Â Â if (pdev->dev.parent)
>> - Â Â Â Â Â Â host = sdhci_alloc_host(pdev->dev.parent, 0);
>> + Â Â if (pdata && pdata->alloc_host)
>> + Â Â Â Â Â Â host = pdata->alloc_host(&pdev->dev);
>> Â Â Â else
>> Â Â Â Â Â Â Â host = sdhci_alloc_host(&pdev->dev, 0);
>>
>> @@ -86,8 +91,7 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
>> Â Â Â Â Â Â Â host->ops = pdata->ops;
>> Â Â Â else
>> Â Â Â Â Â Â Â host->ops = &sdhci_pltfm_ops;
>> - Â Â if (pdata)
>> - Â Â Â Â Â Â host->quirks = pdata->quirks;
>> +
>> Â Â Â host->irq = platform_get_irq(pdev, 0);
>>
>> Â Â Â if (!request_mem_region(iomem->start, resource_size(iomem),
>> @@ -105,15 +109,23 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
>> Â Â Â }
>>
>> Â Â Â if (pdata && pdata->init) {
>> - Â Â Â Â Â Â ret = pdata->init(host);
>> + Â Â Â Â Â Â ret = pdata->init(host, pdata, priv_pdata);
>> Â Â Â Â Â Â Â if (ret)
>> Â Â Â Â Â Â Â Â Â Â Â goto err_plat_init;
>> Â Â Â }
>>
>> + Â Â if (pdata && pdata->get_quirk)
>> + Â Â Â Â Â Â host->quirks = pdata->get_quirk(host);
>> + Â Â else if (pdata)
>> + Â Â Â Â Â Â host->quirks = pdata->quirks;
>> +
>> Â Â Â ret = sdhci_add_host(host);
>> Â Â Â if (ret)
>> Â Â Â Â Â Â Â goto err_add_host;
>>
>> + Â Â if (pdata && pdata->set_max_speed)
>> + Â Â Â Â Â Â pdata->set_max_speed(host);
>> +
>> Â Â Â platform_set_drvdata(pdev, host);
>>
>> Â Â Â return 0;
>> diff --git a/include/linux/sdhci-pltfm.h b/include/linux/sdhci-pltfm.h
>> index 0239bd7..0d8e8f6 100644
>> --- a/include/linux/sdhci-pltfm.h
>> +++ b/include/linux/sdhci-pltfm.h
>> @@ -28,8 +28,11 @@ struct sdhci_host;
>> Âstruct sdhci_pltfm_data {
>> Â Â Â struct sdhci_ops *ops;
>> Â Â Â unsigned int quirks;
>> - Â Â int (*init)(struct sdhci_host *host);
>> + Â Â int (*init)(struct sdhci_host *host, struct sdhci_pltfm_data *pdata, void* priv_pdata);
>> Â Â Â void (*exit)(struct sdhci_host *host);
>> + Â Â void (*set_max_speed)(struct sdhci_host *host);
>> + Â Â unsigned int Â(*get_quirk)(struct sdhci_host *host);
>> + Â Â struct sdhci_host *(*alloc_host)(struct device *dev);
>> Â};
>>
>> Â#endif /* _SDHCI_PLTFM_H */
>> --
>> 1.7.0.4
>>
>
>
> --
> Pengutronix e.K.              | Wolfram Sang        Â|
> Industrial Linux Solutions         | http://www.pengutronix.de/ Â|
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkyfcD0ACgkQD27XaX1/VRt1rgCdHfpBgfsUinbhf4wXCvIMreul
> tCwAoIvA7nFDgH4jBIZgLX6xRstcENaG
> =3A6W
> -----END PGP SIGNATURE-----
>
>
--
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
