| Message ID | 1361343902-15223-3-git-send-email-josephl@nvidia.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
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
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
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
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
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
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
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
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
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
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
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);
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(-)