Re: [patch 1/3] sdhci-pltfm: add call back get_quirk

zhangfei gao
2010-09-28T15:23:25Z

On Tue, Sep 28, 2010 at 10:20 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> On Tue, Sep 28, 2010 at 10:11:05AM -0400, zhangfei gao wrote:
>> On Sun, Sep 26, 2010 at 10:57 PM, zhangfei gao <zhangfei.gao@gmail.com> wrote:
>> > From af7aa1232efd3bb3bf3eac151b71dba7d4a0c875 Mon Sep 17 00:00:00 2001
>> > From: Zhangfei Gao <zhangfei.gao@marvell.com>
>> > Date: Mon, 27 Sep 2010 10:33:38 -0400
>> > Subject: [PATCH 2/3] dhci-pltfm: add call back get_quirk
>> >
>> >        One driver may serve several device, each one may have different
>> > quirks instead of the initial one.
>> >
>> > Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
>> > ---
>> >  drivers/mmc/host/sdhci-pltfm.c |    8 ++++++--
>> >  include/linux/sdhci-pltfm.h    |    1 +
>> >  2 files changed, 7 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
>> > index 82d3d05..5c3c513 100644
>> > --- a/drivers/mmc/host/sdhci-pltfm.c
>> > +++ b/drivers/mmc/host/sdhci-pltfm.c
>> > @@ -91,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),
>> > @@ -115,6 +114,11 @@ static int __devinit sdhci_pltfm_probe(struct
>> > platform_device *pdev)
>> >                        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;
>> > diff --git a/include/linux/sdhci-pltfm.h b/include/linux/sdhci-pltfm.h
>> > index 80c415e..0d20cb4 100644
>> > --- a/include/linux/sdhci-pltfm.h
>> > +++ b/include/linux/sdhci-pltfm.h
>> > @@ -30,6 +30,7 @@ struct sdhci_pltfm_data {
>> >        unsigned int quirks;
>> >        int (*init)(struct sdhci_host *host, struct sdhci_pltfm_data *pdata,
>> > void* priv_pdata);
>> >        void (*exit)(struct sdhci_host *host);
>> > +       unsigned int  (*get_quirk)(struct sdhci_host *host);
>> >        struct sdhci_host *(*alloc_host)(struct device *dev);
>> >  };
>> >
>> > --
>> > 1.7.0.4
>> >
>>
>> Any feed back, the different device may require different quirk, so we
>> can not set the same one set in init.
>
> I might be blind, but I can't see why you can't handle that in init()?

The issue is not init, but the common quirk in pdata.
If using only one quirks in pdata combined with one driver, then
different device would impact each other.
for example device.0 set QUIRK_BROKEN_A to pdata->quirks, the device.1
would use this quirk as default one, device.1 set QUIRK_BROKEN_B to
pdata->quirks, then what device.2 default quirk would become
QUIRK_DEFAULT | QUIRK_BROKEN_A | QUIRK_BROKEN_B.

So differnet device should have quirk by iteslf, and pltfm.c would get
such quirk after init.

>
> --
> Pengutronix e.K.                           | Wolfram Sang                |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iEYEARECAAYFAkyh+acACgkQD27XaX1/VRsCxQCfZVEKl4Nzoosrez5K1m35X/9G
> RgUAnR/Zw2lTeCx3tDMRRUC8TDNJIOkl
> =en2a
> -----END PGP SIGNATURE-----
>
>