Patchwork [2/2] mmc: tegra: use mmc_of_parse to get the support of standard MMC DT bindings

login
register
mail settings
Submitter Joseph Lo
Date Feb. 20, 2013, 7:05 a.m.
Message ID <1361343902-15223-3-git-send-email-josephl@nvidia.com>
Download mbox | patch
Permalink /patch/221970/
State Superseded, archived
Headers show

Comments

Joseph Lo - Feb. 20, 2013, 7:05 a.m.
Updating the sdhci-tegra driver to use mmc_of_parse to support standard
MMC DT bindings. Then we can remove the redundant code that already support
in generic MMC core.

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 85 ++++--------------------------------------
 1 file changed, 7 insertions(+), 78 deletions(-)
Stephen Warren - Feb. 20, 2013, 5:07 p.m.
On 02/20/2013 12:05 AM, Joseph Lo wrote:
> Updating the sdhci-tegra driver to use mmc_of_parse to support standard
> MMC DT bindings. Then we can remove the redundant code that already support

>  static unsigned int tegra_sdhci_get_ro(struct sdhci_host *host)
>  {
> -	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> -	struct sdhci_tegra *tegra_host = pltfm_host->priv;
> -
> -	if (!gpio_is_valid(tegra_host->wp_gpio))
> -		return -1;
> -
> -	return gpio_get_value(tegra_host->wp_gpio);
> +	return mmc_gpio_get_ro(host->mmc);
>  }

It'd be nice if there was a standard version of this function that could
be plugged directly into struct sdhci_ops, so that each individual
driver doesn't have to re-invent this wrapper.

> @@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
>  					struct sdhci_tegra *tegra_host)
>  {
...
> +	struct sdhci_host *host;
...
> +	host = platform_get_drvdata(to_platform_device(dev));
> +	mmc_of_parse(host->mmc);
>  }

It might be simpler to change the function prototype to simply pass in
the host object too.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joseph Lo - Feb. 21, 2013, 2:23 a.m.
On Thu, 2013-02-21 at 01:07 +0800, Stephen Warren wrote:
> On 02/20/2013 12:05 AM, Joseph Lo wrote:
> > Updating the sdhci-tegra driver to use mmc_of_parse to support standard
> > MMC DT bindings. Then we can remove the redundant code that already support
> 
> >  static unsigned int tegra_sdhci_get_ro(struct sdhci_host *host)
> >  {
> > -	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > -	struct sdhci_tegra *tegra_host = pltfm_host->priv;
> > -
> > -	if (!gpio_is_valid(tegra_host->wp_gpio))
> > -		return -1;
> > -
> > -	return gpio_get_value(tegra_host->wp_gpio);
> > +	return mmc_gpio_get_ro(host->mmc);
> >  }
> 
> It'd be nice if there was a standard version of this function that could
> be plugged directly into struct sdhci_ops, so that each individual
> driver doesn't have to re-invent this wrapper.
> 
> > @@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
> >  					struct sdhci_tegra *tegra_host)
> >  {
> ...
> > +	struct sdhci_host *host;
> ...
> > +	host = platform_get_drvdata(to_platform_device(dev));
> > +	mmc_of_parse(host->mmc);
> >  }
> 
> It might be simpler to change the function prototype to simply pass in
> the host object too.

It's a interface problem that I can't fix now. If sdhci core is going to
integrate mmc_of_parse into sdhci_get_of_property and mmc_gpio_get_ro
into somethere sdhci_do_get_ro, then we can refine later.

Thanks,
Joseph


--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren - Feb. 21, 2013, 4:20 a.m.
On 02/20/2013 07:23 PM, Joseph Lo wrote:
> On Thu, 2013-02-21 at 01:07 +0800, Stephen Warren wrote:
>> On 02/20/2013 12:05 AM, Joseph Lo wrote:
>>> Updating the sdhci-tegra driver to use mmc_of_parse to support standard
>>> MMC DT bindings. Then we can remove the redundant code that already support

>>> @@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
>>>  					struct sdhci_tegra *tegra_host)
>>>  {
>> ...
>>> +	struct sdhci_host *host;
>> ...
>>> +	host = platform_get_drvdata(to_platform_device(dev));
>>> +	mmc_of_parse(host->mmc);
>>>  }
>>
>> It might be simpler to change the function prototype to simply pass in
>> the host object too.
> 
> It's a interface problem that I can't fix now. If sdhci core is going to
> integrate mmc_of_parse into sdhci_get_of_property and mmc_gpio_get_ro
> into somethere sdhci_do_get_ro, then we can refine later.

I meant to change the prototype of sdhci_tegra_parse_dt(). It would be
simple to change that function in this patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joseph Lo - Feb. 21, 2013, 6:26 a.m.
On Thu, 2013-02-21 at 12:20 +0800, Stephen Warren wrote:
> On 02/20/2013 07:23 PM, Joseph Lo wrote:
> > On Thu, 2013-02-21 at 01:07 +0800, Stephen Warren wrote:
> >> On 02/20/2013 12:05 AM, Joseph Lo wrote:
> >>> Updating the sdhci-tegra driver to use mmc_of_parse to support standard
> >>> MMC DT bindings. Then we can remove the redundant code that already support
> 
> >>> @@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
> >>>  					struct sdhci_tegra *tegra_host)
> >>>  {
> >> ...
> >>> +	struct sdhci_host *host;
> >> ...
> >>> +	host = platform_get_drvdata(to_platform_device(dev));
> >>> +	mmc_of_parse(host->mmc);
> >>>  }
> >>
> >> It might be simpler to change the function prototype to simply pass in
> >> the host object too.
> > 
> > It's a interface problem that I can't fix now. If sdhci core is going to
> > integrate mmc_of_parse into sdhci_get_of_property and mmc_gpio_get_ro
> > into somethere sdhci_do_get_ro, then we can refine later.
> 
> I meant to change the prototype of sdhci_tegra_parse_dt(). It would be
> simple to change that function in this patch.

Hmm. It might not too much different. Which version you prefer?

1. Original version

2.
 static void sdhci_tegra_parse_dt(struct device *dev,
-                                       struct sdhci_tegra *tegra_host)
+                                       struct sdhci_host *host)
 {
        struct device_node *np = dev->of_node;
-       struct sdhci_host *host;
+       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+       struct sdhci_tegra *tegra_host = pltfm_host->priv;
 
        tegra_host->power_gpio = of_get_named_gpio(np, "power-gpios",0);
-
-       host = platform_get_drvdata(to_platform_device(dev));
        mmc_of_parse(host->mmc);
 }
 
@@ -240,7 +239,7 @@ static int sdhci_tegra_probe(struct platform_device
*pdev)
        tegra_host->soc_data = soc_data;
        pltfm_host->priv = tegra_host;
 
-       sdhci_tegra_parse_dt(&pdev->dev, tegra_host);
+       sdhci_tegra_parse_dt(&pdev->dev, host);
 
3.
-static void sdhci_tegra_parse_dt(struct device *dev,
-                                       struct sdhci_tegra *tegra_host)
+static void sdhci_tegra_parse_dt(struct platform_device *pdev)
 {
-       struct device_node *np = dev->of_node;
-       struct sdhci_host *host;
+       struct device_node *np = pdev->dev.of_node;
+       struct sdhci_host *host = platform_get_drvdata(pdev);
+       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+       struct sdhci_tegra *tegra_host = pltfm_host->priv;
 
        tegra_host->power_gpio = of_get_named_gpio(np, "power-gpios",0);
-
-       host = platform_get_drvdata(to_platform_device(dev));
        mmc_of_parse(host->mmc);
 }
 
@@ -240,7 +239,7 @@ static int sdhci_tegra_probe(struct platform_device
*pdev)
        tegra_host->soc_data = soc_data;
        pltfm_host->priv = tegra_host;
 
-       sdhci_tegra_parse_dt(&pdev->dev, tegra_host);
+       sdhci_tegra_parse_dt(pdev);

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joseph Lo - Feb. 21, 2013, 6:34 a.m.
On Thu, 2013-02-21 at 14:26 +0800, Joseph Lo wrote:
> On Thu, 2013-02-21 at 12:20 +0800, Stephen Warren wrote:
> > On 02/20/2013 07:23 PM, Joseph Lo wrote:
> > > On Thu, 2013-02-21 at 01:07 +0800, Stephen Warren wrote:
> > >> On 02/20/2013 12:05 AM, Joseph Lo wrote:
> > >>> Updating the sdhci-tegra driver to use mmc_of_parse to support standard
> > >>> MMC DT bindings. Then we can remove the redundant code that already support
> > 
> > >>> @@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
> > >>>  					struct sdhci_tegra *tegra_host)
> > >>>  {
> > >> ...
> > >>> +	struct sdhci_host *host;
> > >> ...
> > >>> +	host = platform_get_drvdata(to_platform_device(dev));
> > >>> +	mmc_of_parse(host->mmc);
> > >>>  }
> > >>
> > >> It might be simpler to change the function prototype to simply pass in
> > >> the host object too.
> > > 
> > > It's a interface problem that I can't fix now. If sdhci core is going to
> > > integrate mmc_of_parse into sdhci_get_of_property and mmc_gpio_get_ro
> > > into somethere sdhci_do_get_ro, then we can refine later.
> > 
> > I meant to change the prototype of sdhci_tegra_parse_dt(). It would be
> > simple to change that function in this patch.
> 
> Hmm. It might not too much different. Which version you prefer?
> 
> 1. Original version
> 
> 2.
>  static void sdhci_tegra_parse_dt(struct device *dev,
> -                                       struct sdhci_tegra *tegra_host)
> +                                       struct sdhci_host *host)
>  {
>         struct device_node *np = dev->of_node;
> -       struct sdhci_host *host;
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_tegra *tegra_host = pltfm_host->priv;
>  
>         tegra_host->power_gpio = of_get_named_gpio(np, "power-gpios",0);
> -
> -       host = platform_get_drvdata(to_platform_device(dev));
>         mmc_of_parse(host->mmc);
>  }
>  
> @@ -240,7 +239,7 @@ static int sdhci_tegra_probe(struct platform_device
> *pdev)
>         tegra_host->soc_data = soc_data;
>         pltfm_host->priv = tegra_host;
>  
> -       sdhci_tegra_parse_dt(&pdev->dev, tegra_host);
> +       sdhci_tegra_parse_dt(&pdev->dev, host);
>  
> 3.
> -static void sdhci_tegra_parse_dt(struct device *dev,
> -                                       struct sdhci_tegra *tegra_host)
> +static void sdhci_tegra_parse_dt(struct platform_device *pdev)
>  {
> -       struct device_node *np = dev->of_node;
> -       struct sdhci_host *host;
> +       struct device_node *np = pdev->dev.of_node;
> +       struct sdhci_host *host = platform_get_drvdata(pdev);
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_tegra *tegra_host = pltfm_host->priv;
>  
>         tegra_host->power_gpio = of_get_named_gpio(np, "power-gpios",0);
> -
> -       host = platform_get_drvdata(to_platform_device(dev));
>         mmc_of_parse(host->mmc);
>  }
>  
> @@ -240,7 +239,7 @@ static int sdhci_tegra_probe(struct platform_device
> *pdev)
>         tegra_host->soc_data = soc_data;
>         pltfm_host->priv = tegra_host;
>  
> -       sdhci_tegra_parse_dt(&pdev->dev, tegra_host);
> +       sdhci_tegra_parse_dt(pdev);
> 

or
4. This would be simpler.
@@ -203,12 +203,8 @@ static void sdhci_tegra_parse_dt(struct device
*dev,
                                        struct sdhci_tegra *tegra_host)
 {
        struct device_node *np = dev->of_node;
-       struct sdhci_host *host;
 
        tegra_host->power_gpio = of_get_named_gpio(np, "power-gpios",0);
-
-       host = platform_get_drvdata(to_platform_device(dev));
-       mmc_of_parse(host->mmc);
 }
 
 static int sdhci_tegra_probe(struct platform_device *pdev)
@@ -241,6 +237,7 @@ static int sdhci_tegra_probe(struct platform_device
*pdev)
        pltfm_host->priv = tegra_host;
 
        sdhci_tegra_parse_dt(&pdev->dev, tegra_host);
+       mmc_of_parse(host->mmc);


--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding - Feb. 21, 2013, 8:03 a.m.
On Thu, Feb 21, 2013 at 02:26:37PM +0800, Joseph Lo wrote:
> On Thu, 2013-02-21 at 12:20 +0800, Stephen Warren wrote:
> > On 02/20/2013 07:23 PM, Joseph Lo wrote:
> > > On Thu, 2013-02-21 at 01:07 +0800, Stephen Warren wrote:
> > >> On 02/20/2013 12:05 AM, Joseph Lo wrote:
> > >>> Updating the sdhci-tegra driver to use mmc_of_parse to support standard
> > >>> MMC DT bindings. Then we can remove the redundant code that already support
> > 
> > >>> @@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
> > >>>  					struct sdhci_tegra *tegra_host)
> > >>>  {
> > >> ...
> > >>> +	struct sdhci_host *host;
> > >> ...
> > >>> +	host = platform_get_drvdata(to_platform_device(dev));
> > >>> +	mmc_of_parse(host->mmc);
> > >>>  }
> > >>
> > >> It might be simpler to change the function prototype to simply pass in
> > >> the host object too.
> > > 
> > > It's a interface problem that I can't fix now. If sdhci core is going to
> > > integrate mmc_of_parse into sdhci_get_of_property and mmc_gpio_get_ro
> > > into somethere sdhci_do_get_ro, then we can refine later.
> > 
> > I meant to change the prototype of sdhci_tegra_parse_dt(). It would be
> > simple to change that function in this patch.
> 
> Hmm. It might not too much different. Which version you prefer?
> 
> 1. Original version
> 
> 2.
>  static void sdhci_tegra_parse_dt(struct device *dev,
> -                                       struct sdhci_tegra *tegra_host)
> +                                       struct sdhci_host *host)
>  {
>         struct device_node *np = dev->of_node;
> -       struct sdhci_host *host;
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_tegra *tegra_host = pltfm_host->priv;
>  
>         tegra_host->power_gpio = of_get_named_gpio(np, "power-gpios",0);
> -
> -       host = platform_get_drvdata(to_platform_device(dev));
>         mmc_of_parse(host->mmc);
>  }
>  
> @@ -240,7 +239,7 @@ static int sdhci_tegra_probe(struct platform_device
> *pdev)
>         tegra_host->soc_data = soc_data;
>         pltfm_host->priv = tegra_host;
>  
> -       sdhci_tegra_parse_dt(&pdev->dev, tegra_host);
> +       sdhci_tegra_parse_dt(&pdev->dev, host);
>  

I personally prefer this second version. It keeps with the tradition to
pass a struct device into the *_parse_dt() and also provides easy access
to the sdhci_host.

And on a side-note:

	host = platform_get_drvdata(to_platform_device(dev));

can be more simply written as:

	host = dev_get_drvdata(dev);

Thierry
Joseph Lo - Feb. 21, 2013, 8:15 a.m.
On Thu, 2013-02-21 at 16:03 +0800, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Thu, Feb 21, 2013 at 02:26:37PM +0800, Joseph Lo wrote:
> > On Thu, 2013-02-21 at 12:20 +0800, Stephen Warren wrote:
> > > On 02/20/2013 07:23 PM, Joseph Lo wrote:
> > > > On Thu, 2013-02-21 at 01:07 +0800, Stephen Warren wrote:
> > > >> On 02/20/2013 12:05 AM, Joseph Lo wrote:
> > > >>> Updating the sdhci-tegra driver to use mmc_of_parse to support standard
> > > >>> MMC DT bindings. Then we can remove the redundant code that already support
> > > 
> > > >>> @@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
> > > >>>  					struct sdhci_tegra *tegra_host)
> > > >>>  {
> > > >> ...
> > > >>> +	struct sdhci_host *host;
> > > >> ...
> > > >>> +	host = platform_get_drvdata(to_platform_device(dev));
> > > >>> +	mmc_of_parse(host->mmc);
> > > >>>  }
> > > >>
> > > >> It might be simpler to change the function prototype to simply pass in
> > > >> the host object too.
> > > > 
> > > > It's a interface problem that I can't fix now. If sdhci core is going to
> > > > integrate mmc_of_parse into sdhci_get_of_property and mmc_gpio_get_ro
> > > > into somethere sdhci_do_get_ro, then we can refine later.
> > > 
> > > I meant to change the prototype of sdhci_tegra_parse_dt(). It would be
> > > simple to change that function in this patch.
> > 
> > Hmm. It might not too much different. Which version you prefer?
> > 
> > 1. Original version
> > 
> > 2.
> >  static void sdhci_tegra_parse_dt(struct device *dev,
> > -                                       struct sdhci_tegra *tegra_host)
> > +                                       struct sdhci_host *host)
> >  {
> >         struct device_node *np = dev->of_node;
> > -       struct sdhci_host *host;
> > +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > +       struct sdhci_tegra *tegra_host = pltfm_host->priv;
> >  
> >         tegra_host->power_gpio = of_get_named_gpio(np, "power-gpios",0);
> > -
> > -       host = platform_get_drvdata(to_platform_device(dev));
> >         mmc_of_parse(host->mmc);
> >  }
> >  
> > @@ -240,7 +239,7 @@ static int sdhci_tegra_probe(struct platform_device
> > *pdev)
> >         tegra_host->soc_data = soc_data;
> >         pltfm_host->priv = tegra_host;
> >  
> > -       sdhci_tegra_parse_dt(&pdev->dev, tegra_host);
> > +       sdhci_tegra_parse_dt(&pdev->dev, host);
> >  
> 
> I personally prefer this second version. It keeps with the tradition to
> pass a struct device into the *_parse_dt() and also provides easy access
> to the sdhci_host.
> 
> And on a side-note:
> 
> 	host = platform_get_drvdata(to_platform_device(dev));
> 
> can be more simply written as:
> 
> 	host = dev_get_drvdata(dev);
> 
Indeed, it's simpler and works. Thanks for sharing. :)


--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren - Feb. 21, 2013, 6:50 p.m.
On 02/20/2013 11:26 PM, Joseph Lo wrote:
> On Thu, 2013-02-21 at 12:20 +0800, Stephen Warren wrote:
>> On 02/20/2013 07:23 PM, Joseph Lo wrote:
>>> On Thu, 2013-02-21 at 01:07 +0800, Stephen Warren wrote:
>>>> On 02/20/2013 12:05 AM, Joseph Lo wrote:
>>>>> Updating the sdhci-tegra driver to use mmc_of_parse to support standard
>>>>> MMC DT bindings. Then we can remove the redundant code that already support
>>
>>>>> @@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
>>>>>  					struct sdhci_tegra *tegra_host)
>>>>>  {
>>>> ...
>>>>> +	struct sdhci_host *host;
>>>> ...
>>>>> +	host = platform_get_drvdata(to_platform_device(dev));
>>>>> +	mmc_of_parse(host->mmc);
>>>>>  }
>>>>
>>>> It might be simpler to change the function prototype to simply pass in
>>>> the host object too.
>>>
>>> It's a interface problem that I can't fix now. If sdhci core is going to
>>> integrate mmc_of_parse into sdhci_get_of_property and mmc_gpio_get_ro
>>> into somethere sdhci_do_get_ro, then we can refine later.
>>
>> I meant to change the prototype of sdhci_tegra_parse_dt(). It would be
>> simple to change that function in this patch.
> 
> Hmm. It might not too much different. Which version you prefer?
> 
> 1. Original version
> 
> 2.

sdhci_tegra_probe() already has all the values you need; I was thinking
of just passing in all of the values that this function needs, rather
than deriving anything within the function.

>  static void sdhci_tegra_parse_dt(struct device *dev,
> -                                       struct sdhci_tegra *tegra_host)
> +                                       struct sdhci_host *host)
>  {
>         struct device_node *np = dev->of_node;
> -       struct sdhci_host *host;
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_tegra *tegra_host = pltfm_host->priv;
>  
>         tegra_host->power_gpio = of_get_named_gpio(np, "power-gpios",0);
> -
> -       host = platform_get_drvdata(to_platform_device(dev));
>         mmc_of_parse(host->mmc);
>  }
>  
> @@ -240,7 +239,7 @@ static int sdhci_tegra_probe(struct platform_device
> *pdev)
>         tegra_host->soc_data = soc_data;
>         pltfm_host->priv = tegra_host;
>  
> -       sdhci_tegra_parse_dt(&pdev->dev, tegra_host);
> +       sdhci_tegra_parse_dt(&pdev->dev, host);

That seems fairly direct though. Is "dev" available inside host,
platfm_host or tegra_host? You could obtain it from there if you wanted
to reduce the number of parameters passed to the function.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Guennadi Liakhovetski - Feb. 21, 2013, 10:44 p.m.
Hi Joseph

On Wed, 20 Feb 2013, Joseph Lo wrote:

> Updating the sdhci-tegra driver to use mmc_of_parse to support standard
> MMC DT bindings. Then we can remove the redundant code that already support
> in generic MMC core.
> 
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
>  drivers/mmc/host/sdhci-tegra.c | 85 ++++--------------------------------------
>  1 file changed, 7 insertions(+), 78 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index 08b06e9..4f6b5c4 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -24,6 +24,7 @@
>  #include <linux/gpio.h>
>  #include <linux/mmc/card.h>
>  #include <linux/mmc/host.h>
> +#include <linux/mmc/slot-gpio.h>
>  
>  #include <asm/gpio.h>
>  
> @@ -44,10 +45,7 @@ struct sdhci_tegra_soc_data {
>  
>  struct sdhci_tegra {
>  	const struct sdhci_tegra_soc_data *soc_data;
> -	int cd_gpio;
> -	int wp_gpio;
>  	int power_gpio;
> -	int is_8bit;
>  };
>  
>  static u32 tegra_sdhci_readl(struct sdhci_host *host, int reg)
> @@ -107,23 +105,9 @@ static void tegra_sdhci_writel(struct sdhci_host *host, u32 val, int reg)
>  
>  static unsigned int tegra_sdhci_get_ro(struct sdhci_host *host)
>  {
> -	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> -	struct sdhci_tegra *tegra_host = pltfm_host->priv;
> -
> -	if (!gpio_is_valid(tegra_host->wp_gpio))
> -		return -1;
> -
> -	return gpio_get_value(tegra_host->wp_gpio);
> +	return mmc_gpio_get_ro(host->mmc);
>  }
>  
> -static irqreturn_t carddetect_irq(int irq, void *data)
> -{
> -	struct sdhci_host *sdhost = (struct sdhci_host *)data;
> -
> -	tasklet_schedule(&sdhost->card_tasklet);
> -	return IRQ_HANDLED;
> -};
> -
>  static void tegra_sdhci_reset_exit(struct sdhci_host *host, u8 mask)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -145,12 +129,11 @@ static void tegra_sdhci_reset_exit(struct sdhci_host *host, u8 mask)
>  
>  static int tegra_sdhci_buswidth(struct sdhci_host *host, int bus_width)
>  {
> -	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> -	struct sdhci_tegra *tegra_host = pltfm_host->priv;
>  	u32 ctrl;
>  
>  	ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
> -	if (tegra_host->is_8bit && bus_width == MMC_BUS_WIDTH_8) {
> +	if (host->mmc->caps == MMC_CAP_8_BIT_DATA &&

Don't you want to check for just one bit? -Šcaps can contain multiple 
flags set:

+	if ((host->mmc->caps & MMC_CAP_8_BIT_DATA) &&

Thanks
Guennadi

> +	    bus_width == MMC_BUS_WIDTH_8) {
>  		ctrl &= ~SDHCI_CTRL_4BITBUS;
>  		ctrl |= SDHCI_CTRL_8BITBUS;
>  	} else {
> @@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
>  					struct sdhci_tegra *tegra_host)
>  {
>  	struct device_node *np = dev->of_node;
> -	u32 bus_width;
> +	struct sdhci_host *host;
>  
> -	tegra_host->cd_gpio = of_get_named_gpio(np, "cd-gpios", 0);
> -	tegra_host->wp_gpio = of_get_named_gpio(np, "wp-gpios", 0);
>  	tegra_host->power_gpio = of_get_named_gpio(np, "power-gpios", 0);
>  
> -	if (of_property_read_u32(np, "bus-width", &bus_width) == 0 &&
> -	    bus_width == 8)
> -		tegra_host->is_8bit = 1;
> +	host = platform_get_drvdata(to_platform_device(dev));
> +	mmc_of_parse(host->mmc);
>  }
>  
>  static int sdhci_tegra_probe(struct platform_device *pdev)
> @@ -272,37 +252,6 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
>  		gpio_direction_output(tegra_host->power_gpio, 1);
>  	}
>  
> -	if (gpio_is_valid(tegra_host->cd_gpio)) {
> -		rc = gpio_request(tegra_host->cd_gpio, "sdhci_cd");
> -		if (rc) {
> -			dev_err(mmc_dev(host->mmc),
> -				"failed to allocate cd gpio\n");
> -			goto err_cd_req;
> -		}
> -		gpio_direction_input(tegra_host->cd_gpio);
> -
> -		rc = request_irq(gpio_to_irq(tegra_host->cd_gpio),
> -				 carddetect_irq,
> -				 IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
> -				 mmc_hostname(host->mmc), host);
> -
> -		if (rc)	{
> -			dev_err(mmc_dev(host->mmc), "request irq error\n");
> -			goto err_cd_irq_req;
> -		}
> -
> -	}
> -
> -	if (gpio_is_valid(tegra_host->wp_gpio)) {
> -		rc = gpio_request(tegra_host->wp_gpio, "sdhci_wp");
> -		if (rc) {
> -			dev_err(mmc_dev(host->mmc),
> -				"failed to allocate wp gpio\n");
> -			goto err_wp_req;
> -		}
> -		gpio_direction_input(tegra_host->wp_gpio);
> -	}
> -
>  	clk = clk_get(mmc_dev(host->mmc), NULL);
>  	if (IS_ERR(clk)) {
>  		dev_err(mmc_dev(host->mmc), "clk err\n");
> @@ -312,9 +261,6 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
>  	clk_prepare_enable(clk);
>  	pltfm_host->clk = clk;
>  
> -	if (tegra_host->is_8bit)
> -		host->mmc->caps |= MMC_CAP_8_BIT_DATA;
> -
>  	rc = sdhci_add_host(host);
>  	if (rc)
>  		goto err_add_host;
> @@ -325,15 +271,6 @@ err_add_host:
>  	clk_disable_unprepare(pltfm_host->clk);
>  	clk_put(pltfm_host->clk);
>  err_clk_get:
> -	if (gpio_is_valid(tegra_host->wp_gpio))
> -		gpio_free(tegra_host->wp_gpio);
> -err_wp_req:
> -	if (gpio_is_valid(tegra_host->cd_gpio))
> -		free_irq(gpio_to_irq(tegra_host->cd_gpio), host);
> -err_cd_irq_req:
> -	if (gpio_is_valid(tegra_host->cd_gpio))
> -		gpio_free(tegra_host->cd_gpio);
> -err_cd_req:
>  	if (gpio_is_valid(tegra_host->power_gpio))
>  		gpio_free(tegra_host->power_gpio);
>  err_power_req:
> @@ -351,14 +288,6 @@ static int sdhci_tegra_remove(struct platform_device *pdev)
>  
>  	sdhci_remove_host(host, dead);
>  
> -	if (gpio_is_valid(tegra_host->wp_gpio))
> -		gpio_free(tegra_host->wp_gpio);
> -
> -	if (gpio_is_valid(tegra_host->cd_gpio)) {
> -		free_irq(gpio_to_irq(tegra_host->cd_gpio), host);
> -		gpio_free(tegra_host->cd_gpio);
> -	}
> -
>  	if (gpio_is_valid(tegra_host->power_gpio))
>  		gpio_free(tegra_host->power_gpio);
>  
> -- 
> 1.8.1.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
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joseph Lo - Feb. 22, 2013, 2:07 a.m.
On Fri, 2013-02-22 at 06:44 +0800, Guennadi Liakhovetski wrote:
> Hi Joseph
> 
> On Wed, 20 Feb 2013, Joseph Lo wrote:
> >  static int tegra_sdhci_buswidth(struct sdhci_host *host, int bus_width)
> >  {
> > -	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > -	struct sdhci_tegra *tegra_host = pltfm_host->priv;
> >  	u32 ctrl;
> >  
> >  	ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
> > -	if (tegra_host->is_8bit && bus_width == MMC_BUS_WIDTH_8) {
> > +	if (host->mmc->caps == MMC_CAP_8_BIT_DATA &&
> 
> Don't you want to check for just one bit? -Šcaps can contain multiple 
> flags set:
> 
> +	if ((host->mmc->caps & MMC_CAP_8_BIT_DATA) &&
> 
Ah, yes. Thanks for your review.

Joseph


--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 08b06e9..4f6b5c4 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -24,6 +24,7 @@ 
 #include <linux/gpio.h>
 #include <linux/mmc/card.h>
 #include <linux/mmc/host.h>
+#include <linux/mmc/slot-gpio.h>
 
 #include <asm/gpio.h>
 
@@ -44,10 +45,7 @@  struct sdhci_tegra_soc_data {
 
 struct sdhci_tegra {
 	const struct sdhci_tegra_soc_data *soc_data;
-	int cd_gpio;
-	int wp_gpio;
 	int power_gpio;
-	int is_8bit;
 };
 
 static u32 tegra_sdhci_readl(struct sdhci_host *host, int reg)
@@ -107,23 +105,9 @@  static void tegra_sdhci_writel(struct sdhci_host *host, u32 val, int reg)
 
 static unsigned int tegra_sdhci_get_ro(struct sdhci_host *host)
 {
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	struct sdhci_tegra *tegra_host = pltfm_host->priv;
-
-	if (!gpio_is_valid(tegra_host->wp_gpio))
-		return -1;
-
-	return gpio_get_value(tegra_host->wp_gpio);
+	return mmc_gpio_get_ro(host->mmc);
 }
 
-static irqreturn_t carddetect_irq(int irq, void *data)
-{
-	struct sdhci_host *sdhost = (struct sdhci_host *)data;
-
-	tasklet_schedule(&sdhost->card_tasklet);
-	return IRQ_HANDLED;
-};
-
 static void tegra_sdhci_reset_exit(struct sdhci_host *host, u8 mask)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -145,12 +129,11 @@  static void tegra_sdhci_reset_exit(struct sdhci_host *host, u8 mask)
 
 static int tegra_sdhci_buswidth(struct sdhci_host *host, int bus_width)
 {
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	struct sdhci_tegra *tegra_host = pltfm_host->priv;
 	u32 ctrl;
 
 	ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
-	if (tegra_host->is_8bit && bus_width == MMC_BUS_WIDTH_8) {
+	if (host->mmc->caps == MMC_CAP_8_BIT_DATA &&
+	    bus_width == MMC_BUS_WIDTH_8) {
 		ctrl &= ~SDHCI_CTRL_4BITBUS;
 		ctrl |= SDHCI_CTRL_8BITBUS;
 	} else {
@@ -220,15 +203,12 @@  static void sdhci_tegra_parse_dt(struct device *dev,
 					struct sdhci_tegra *tegra_host)
 {
 	struct device_node *np = dev->of_node;
-	u32 bus_width;
+	struct sdhci_host *host;
 
-	tegra_host->cd_gpio = of_get_named_gpio(np, "cd-gpios", 0);
-	tegra_host->wp_gpio = of_get_named_gpio(np, "wp-gpios", 0);
 	tegra_host->power_gpio = of_get_named_gpio(np, "power-gpios", 0);
 
-	if (of_property_read_u32(np, "bus-width", &bus_width) == 0 &&
-	    bus_width == 8)
-		tegra_host->is_8bit = 1;
+	host = platform_get_drvdata(to_platform_device(dev));
+	mmc_of_parse(host->mmc);
 }
 
 static int sdhci_tegra_probe(struct platform_device *pdev)
@@ -272,37 +252,6 @@  static int sdhci_tegra_probe(struct platform_device *pdev)
 		gpio_direction_output(tegra_host->power_gpio, 1);
 	}
 
-	if (gpio_is_valid(tegra_host->cd_gpio)) {
-		rc = gpio_request(tegra_host->cd_gpio, "sdhci_cd");
-		if (rc) {
-			dev_err(mmc_dev(host->mmc),
-				"failed to allocate cd gpio\n");
-			goto err_cd_req;
-		}
-		gpio_direction_input(tegra_host->cd_gpio);
-
-		rc = request_irq(gpio_to_irq(tegra_host->cd_gpio),
-				 carddetect_irq,
-				 IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
-				 mmc_hostname(host->mmc), host);
-
-		if (rc)	{
-			dev_err(mmc_dev(host->mmc), "request irq error\n");
-			goto err_cd_irq_req;
-		}
-
-	}
-
-	if (gpio_is_valid(tegra_host->wp_gpio)) {
-		rc = gpio_request(tegra_host->wp_gpio, "sdhci_wp");
-		if (rc) {
-			dev_err(mmc_dev(host->mmc),
-				"failed to allocate wp gpio\n");
-			goto err_wp_req;
-		}
-		gpio_direction_input(tegra_host->wp_gpio);
-	}
-
 	clk = clk_get(mmc_dev(host->mmc), NULL);
 	if (IS_ERR(clk)) {
 		dev_err(mmc_dev(host->mmc), "clk err\n");
@@ -312,9 +261,6 @@  static int sdhci_tegra_probe(struct platform_device *pdev)
 	clk_prepare_enable(clk);
 	pltfm_host->clk = clk;
 
-	if (tegra_host->is_8bit)
-		host->mmc->caps |= MMC_CAP_8_BIT_DATA;
-
 	rc = sdhci_add_host(host);
 	if (rc)
 		goto err_add_host;
@@ -325,15 +271,6 @@  err_add_host:
 	clk_disable_unprepare(pltfm_host->clk);
 	clk_put(pltfm_host->clk);
 err_clk_get:
-	if (gpio_is_valid(tegra_host->wp_gpio))
-		gpio_free(tegra_host->wp_gpio);
-err_wp_req:
-	if (gpio_is_valid(tegra_host->cd_gpio))
-		free_irq(gpio_to_irq(tegra_host->cd_gpio), host);
-err_cd_irq_req:
-	if (gpio_is_valid(tegra_host->cd_gpio))
-		gpio_free(tegra_host->cd_gpio);
-err_cd_req:
 	if (gpio_is_valid(tegra_host->power_gpio))
 		gpio_free(tegra_host->power_gpio);
 err_power_req:
@@ -351,14 +288,6 @@  static int sdhci_tegra_remove(struct platform_device *pdev)
 
 	sdhci_remove_host(host, dead);
 
-	if (gpio_is_valid(tegra_host->wp_gpio))
-		gpio_free(tegra_host->wp_gpio);
-
-	if (gpio_is_valid(tegra_host->cd_gpio)) {
-		free_irq(gpio_to_irq(tegra_host->cd_gpio), host);
-		gpio_free(tegra_host->cd_gpio);
-	}
-
 	if (gpio_is_valid(tegra_host->power_gpio))
 		gpio_free(tegra_host->power_gpio);