* [PATCH 0/2] Added set_power handler to mmc sdhci host @ 2013-05-22 17:47 Felipe F. Tonello 2013-05-22 17:47 ` [PATCH 1/2] mmc: sdhci: Added set_power sdhci_ops handler Felipe F. Tonello 2013-05-22 17:47 ` [PATCH 2/2] mmc: sdhci-s3c: Added set_power handler to platdata Felipe F. Tonello 0 siblings, 2 replies; 9+ messages in thread From: Felipe F. Tonello @ 2013-05-22 17:47 UTC (permalink / raw) To: linux-mmc; +Cc: ben-linux, cjb, arnd, linux-kernel, Felipe F. Tonello From: "Felipe F. Tonello" <eu@felipetonello.com> This set_power handler is useful for modules that need to be turned off when the mmc host turns off. Felipe F. Tonello (2): mmc: sdhci: Added set_power sdhci_ops handler. mmc: sdhci-s3c: Added set_power handler to platdata drivers/mmc/host/sdhci-s3c.c | 8 ++++++++ drivers/mmc/host/sdhci.c | 8 ++++++++ drivers/mmc/host/sdhci.h | 1 + include/linux/platform_data/mmc-sdhci-s3c.h | 1 + 4 files changed, 18 insertions(+) -- 1.8.1.4 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] mmc: sdhci: Added set_power sdhci_ops handler. 2013-05-22 17:47 [PATCH 0/2] Added set_power handler to mmc sdhci host Felipe F. Tonello @ 2013-05-22 17:47 ` Felipe F. Tonello 2013-05-22 20:30 ` Guennadi Liakhovetski 2013-05-22 17:47 ` [PATCH 2/2] mmc: sdhci-s3c: Added set_power handler to platdata Felipe F. Tonello 1 sibling, 1 reply; 9+ messages in thread From: Felipe F. Tonello @ 2013-05-22 17:47 UTC (permalink / raw) To: linux-mmc; +Cc: ben-linux, cjb, arnd, linux-kernel, Felipe F. Tonello From: "Felipe F. Tonello" <eu@felipetonello.com> This is useful for power managment purposes if a sdhci child host wants to turn off some other peripheral also. Signed-off-by: Felipe F. Tonello <eu@felipetonello.com> --- drivers/mmc/host/sdhci.c | 8 ++++++++ drivers/mmc/host/sdhci.h | 1 + 2 files changed, 9 insertions(+) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 2ea429c..0a026c6 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1244,6 +1244,10 @@ static int sdhci_set_power(struct sdhci_host *host, unsigned short power) u8 pwr = 0; if (power != (unsigned short)-1) { + + if (host->ops->set_power) + host->ops->set_power(host, true); + switch (1 << power) { case MMC_VDD_165_195: pwr = SDHCI_POWER_180; @@ -1259,6 +1263,10 @@ static int sdhci_set_power(struct sdhci_host *host, unsigned short power) default: BUG(); } + } else { + + if (host->ops->set_power) + host->ops->set_power(host, false); } if (host->pwr == pwr) diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h index 379e09d..293d56d 100644 --- a/drivers/mmc/host/sdhci.h +++ b/drivers/mmc/host/sdhci.h @@ -294,6 +294,7 @@ struct sdhci_ops { void (*platform_resume)(struct sdhci_host *host); void (*adma_workaround)(struct sdhci_host *host, u32 intmask); void (*platform_init)(struct sdhci_host *host); + void (*set_power)(struct sdhci_host *host, bool power); }; #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] mmc: sdhci: Added set_power sdhci_ops handler. 2013-05-22 17:47 ` [PATCH 1/2] mmc: sdhci: Added set_power sdhci_ops handler Felipe F. Tonello @ 2013-05-22 20:30 ` Guennadi Liakhovetski 2013-05-22 21:22 ` Felipe Ferreri Tonello 0 siblings, 1 reply; 9+ messages in thread From: Guennadi Liakhovetski @ 2013-05-22 20:30 UTC (permalink / raw) To: Felipe F. Tonello; +Cc: linux-mmc, ben-linux, cjb, arnd, linux-kernel On Wed, 22 May 2013, Felipe F. Tonello wrote: > From: "Felipe F. Tonello" <eu@felipetonello.com> > > This is useful for power managment purposes if a sdhci child host wants to > turn off some other peripheral also. Sorry, could you elaborate a bit? In what situations is it exactly useful? And why cannot the regulator API be used there? Thanks Guennadi > > Signed-off-by: Felipe F. Tonello <eu@felipetonello.com> > --- > drivers/mmc/host/sdhci.c | 8 ++++++++ > drivers/mmc/host/sdhci.h | 1 + > 2 files changed, 9 insertions(+) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 2ea429c..0a026c6 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -1244,6 +1244,10 @@ static int sdhci_set_power(struct sdhci_host *host, unsigned short power) > u8 pwr = 0; > > if (power != (unsigned short)-1) { > + > + if (host->ops->set_power) > + host->ops->set_power(host, true); > + > switch (1 << power) { > case MMC_VDD_165_195: > pwr = SDHCI_POWER_180; > @@ -1259,6 +1263,10 @@ static int sdhci_set_power(struct sdhci_host *host, unsigned short power) > default: > BUG(); > } > + } else { > + > + if (host->ops->set_power) > + host->ops->set_power(host, false); > } > > if (host->pwr == pwr) > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h > index 379e09d..293d56d 100644 > --- a/drivers/mmc/host/sdhci.h > +++ b/drivers/mmc/host/sdhci.h > @@ -294,6 +294,7 @@ struct sdhci_ops { > void (*platform_resume)(struct sdhci_host *host); > void (*adma_workaround)(struct sdhci_host *host, u32 intmask); > void (*platform_init)(struct sdhci_host *host); > + void (*set_power)(struct sdhci_host *host, bool power); > }; > > #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS > -- > 1.8.1.4 > > -- > 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/ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] mmc: sdhci: Added set_power sdhci_ops handler. 2013-05-22 20:30 ` Guennadi Liakhovetski @ 2013-05-22 21:22 ` Felipe Ferreri Tonello 2013-05-23 7:25 ` Guennadi Liakhovetski 0 siblings, 1 reply; 9+ messages in thread From: Felipe Ferreri Tonello @ 2013-05-22 21:22 UTC (permalink / raw) To: Guennadi Liakhovetski; +Cc: linux-mmc, ben-linux, cjb, arnd, linux-kernel Hi Guennadi, On Wednesday, May 22, 2013 10:30:40 PM Guennadi Liakhovetski wrote: > On Wed, 22 May 2013, Felipe F. Tonello wrote: > > From: "Felipe F. Tonello" <eu@felipetonello.com> > > > > This is useful for power managment purposes if a sdhci child host wants to > > turn off some other peripheral also. > > Sorry, could you elaborate a bit? In what situations is it exactly useful? > And why cannot the regulator API be used there? Sorry about that. One example that I can think of is when you have a wifi module connected as a mmc card via sdio. So you can register a callback function in your machine source code to turn on/off the wifi module based on the mmc host power. I've seen this implementation in others mmc hosts, such as omap. Regards, Felipe > > Thanks > Guennadi > > > Signed-off-by: Felipe F. Tonello <eu@felipetonello.com> > > --- > > > > drivers/mmc/host/sdhci.c | 8 ++++++++ > > drivers/mmc/host/sdhci.h | 1 + > > 2 files changed, 9 insertions(+) > > > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > > index 2ea429c..0a026c6 100644 > > --- a/drivers/mmc/host/sdhci.c > > +++ b/drivers/mmc/host/sdhci.c > > @@ -1244,6 +1244,10 @@ static int sdhci_set_power(struct sdhci_host *host, > > unsigned short power)> > > u8 pwr = 0; > > > > if (power != (unsigned short)-1) { > > > > + > > + if (host->ops->set_power) > > + host->ops->set_power(host, true); > > + > > > > switch (1 << power) { > > > > case MMC_VDD_165_195: > > pwr = SDHCI_POWER_180; > > > > @@ -1259,6 +1263,10 @@ static int sdhci_set_power(struct sdhci_host *host, > > unsigned short power)> > > default: > > BUG(); > > > > } > > > > + } else { > > + > > + if (host->ops->set_power) > > + host->ops->set_power(host, false); > > > > } > > > > if (host->pwr == pwr) > > > > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h > > index 379e09d..293d56d 100644 > > --- a/drivers/mmc/host/sdhci.h > > +++ b/drivers/mmc/host/sdhci.h > > @@ -294,6 +294,7 @@ struct sdhci_ops { > > > > void (*platform_resume)(struct sdhci_host *host); > > void (*adma_workaround)(struct sdhci_host *host, u32 intmask); > > void (*platform_init)(struct sdhci_host *host); > > > > + void (*set_power)(struct sdhci_host *host, bool power); > > > > }; > > > > #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS > > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] mmc: sdhci: Added set_power sdhci_ops handler. 2013-05-22 21:22 ` Felipe Ferreri Tonello @ 2013-05-23 7:25 ` Guennadi Liakhovetski 2013-05-24 4:02 ` Jaehoon Chung 0 siblings, 1 reply; 9+ messages in thread From: Guennadi Liakhovetski @ 2013-05-23 7:25 UTC (permalink / raw) To: Felipe Ferreri Tonello; +Cc: linux-mmc, ben-linux, cjb, arnd, linux-kernel On Wed, 22 May 2013, Felipe Ferreri Tonello wrote: > Hi Guennadi, > > On Wednesday, May 22, 2013 10:30:40 PM Guennadi Liakhovetski wrote: > > On Wed, 22 May 2013, Felipe F. Tonello wrote: > > > From: "Felipe F. Tonello" <eu@felipetonello.com> > > > > > > This is useful for power managment purposes if a sdhci child host wants to > > > turn off some other peripheral also. > > > > Sorry, could you elaborate a bit? In what situations is it exactly useful? > > And why cannot the regulator API be used there? > > Sorry about that. > > One example that I can think of is when you have a wifi module connected as a > mmc card via sdio. So you can register a callback function in your machine > source code to turn on/off the wifi module based on the mmc host power. Ok, understand. Your second patch in this series adds such a callback in your SDHCI host driver and there it just calls a platform callback. I don't think this is a good idea. First, we want to go away from platform callbacks, because they are incompatible with DT. Second, because the proper solution IMHO would be for your platform to export a regulator, and the SDHCI core driver already includes regulator support. > I've seen this implementation in others mmc hosts, such as omap. Which, however, doesn't yet mean, it's a good idea :) Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] mmc: sdhci: Added set_power sdhci_ops handler. 2013-05-23 7:25 ` Guennadi Liakhovetski @ 2013-05-24 4:02 ` Jaehoon Chung 2013-05-24 22:12 ` Felipe Tonello 0 siblings, 1 reply; 9+ messages in thread From: Jaehoon Chung @ 2013-05-24 4:02 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: Felipe Ferreri Tonello, linux-mmc, ben-linux, cjb, arnd, linux-kernel On 05/23/2013 04:25 PM, Guennadi Liakhovetski wrote: > On Wed, 22 May 2013, Felipe Ferreri Tonello wrote: > >> Hi Guennadi, >> >> On Wednesday, May 22, 2013 10:30:40 PM Guennadi Liakhovetski wrote: >>> On Wed, 22 May 2013, Felipe F. Tonello wrote: >>>> From: "Felipe F. Tonello" <eu@felipetonello.com> >>>> >>>> This is useful for power managment purposes if a sdhci child host wants to >>>> turn off some other peripheral also. >>> >>> Sorry, could you elaborate a bit? In what situations is it exactly useful? >>> And why cannot the regulator API be used there? >> >> Sorry about that. >> >> One example that I can think of is when you have a wifi module connected as a >> mmc card via sdio. So you can register a callback function in your machine >> source code to turn on/off the wifi module based on the mmc host power. > > Ok, understand. Your second patch in this series adds such a callback in > your SDHCI host driver and there it just calls a platform callback. I > don't think this is a good idea. First, we want to go away from platform > callbacks, because they are incompatible with DT. Second, because the > proper solution IMHO would be for your platform to export a regulator, and > the SDHCI core driver already includes regulator support. We can use the regulator framework. i think this callback function didn't need. Best Regards, Jaehoon Chung > >> I've seen this implementation in others mmc hosts, such as omap. > > Which, however, doesn't yet mean, it's a good idea :) > > Thanks > Guennadi > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ > -- > 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 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] mmc: sdhci: Added set_power sdhci_ops handler. 2013-05-24 4:02 ` Jaehoon Chung @ 2013-05-24 22:12 ` Felipe Tonello 0 siblings, 0 replies; 9+ messages in thread From: Felipe Tonello @ 2013-05-24 22:12 UTC (permalink / raw) To: Jaehoon Chung Cc: Guennadi Liakhovetski, linux-mmc, ben-linux, cjb, arnd, linux-kernel Hi all, On Thu, May 23, 2013 at 9:02 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote: > On 05/23/2013 04:25 PM, Guennadi Liakhovetski wrote: >> On Wed, 22 May 2013, Felipe Ferreri Tonello wrote: >> >>> Hi Guennadi, >>> >>> On Wednesday, May 22, 2013 10:30:40 PM Guennadi Liakhovetski wrote: >>>> On Wed, 22 May 2013, Felipe F. Tonello wrote: >>>>> From: "Felipe F. Tonello" <eu@felipetonello.com> >>>>> >>>>> This is useful for power managment purposes if a sdhci child host wants to >>>>> turn off some other peripheral also. >>>> >>>> Sorry, could you elaborate a bit? In what situations is it exactly useful? >>>> And why cannot the regulator API be used there? >>> >>> Sorry about that. >>> >>> One example that I can think of is when you have a wifi module connected as a >>> mmc card via sdio. So you can register a callback function in your machine >>> source code to turn on/off the wifi module based on the mmc host power. >> >> Ok, understand. Your second patch in this series adds such a callback in >> your SDHCI host driver and there it just calls a platform callback. I >> don't think this is a good idea. First, we want to go away from platform >> callbacks, because they are incompatible with DT. Second, because the >> proper solution IMHO would be for your platform to export a regulator, and >> the SDHCI core driver already includes regulator support. > We can use the regulator framework. > i think this callback function didn't need. > Ok, thanks for the feed back. Just to get things clear, is this implementation using regulator framework already done? BR, Felipe > Best Regards, > Jaehoon Chung >> >>> I've seen this implementation in others mmc hosts, such as omap. >> >> Which, however, doesn't yet mean, it's a good idea :) >> >> Thanks >> Guennadi >> --- >> Guennadi Liakhovetski, Ph.D. >> Freelance Open-Source Software Developer >> http://www.open-technology.de/ >> -- >> 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 >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] mmc: sdhci-s3c: Added set_power handler to platdata 2013-05-22 17:47 [PATCH 0/2] Added set_power handler to mmc sdhci host Felipe F. Tonello 2013-05-22 17:47 ` [PATCH 1/2] mmc: sdhci: Added set_power sdhci_ops handler Felipe F. Tonello @ 2013-05-22 17:47 ` Felipe F. Tonello 2013-05-24 3:57 ` Jaehoon Chung 1 sibling, 1 reply; 9+ messages in thread From: Felipe F. Tonello @ 2013-05-22 17:47 UTC (permalink / raw) To: linux-mmc; +Cc: ben-linux, cjb, arnd, linux-kernel, Felipe F. Tonello From: "Felipe F. Tonello" <eu@felipetonello.com> This is useful to turn off peripherals that are related to the mmc host. One common case is when the wifi module is connected as an mmc card to the host. Signed-off-by: Felipe F. Tonello <eu@felipetonello.com> --- drivers/mmc/host/sdhci-s3c.c | 8 ++++++++ include/linux/platform_data/mmc-sdhci-s3c.h | 1 + 2 files changed, 9 insertions(+) diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c index c6f6246..f7e740c 100644 --- a/drivers/mmc/host/sdhci-s3c.c +++ b/drivers/mmc/host/sdhci-s3c.c @@ -360,11 +360,19 @@ static int sdhci_s3c_platform_bus_width(struct sdhci_host *host, int width) return 0; } +static void sdhci_s3c_set_power(struct sdhci_host *host, bool power) +{ + struct sdhci_s3c *ourhost = to_s3c(host); + if (ourhost->pdata->set_power) + ourhost->pdata->set_power(power); +} + static struct sdhci_ops sdhci_s3c_ops = { .get_max_clock = sdhci_s3c_get_max_clk, .set_clock = sdhci_s3c_set_clock, .get_min_clock = sdhci_s3c_get_min_clock, .platform_bus_width = sdhci_s3c_platform_bus_width, + .set_power = sdhci_s3c_set_power, }; static void sdhci_s3c_notify_change(struct platform_device *dev, int state) diff --git a/include/linux/platform_data/mmc-sdhci-s3c.h b/include/linux/platform_data/mmc-sdhci-s3c.h index 249f023..55be925 100644 --- a/include/linux/platform_data/mmc-sdhci-s3c.h +++ b/include/linux/platform_data/mmc-sdhci-s3c.h @@ -50,6 +50,7 @@ struct s3c_sdhci_platdata { int state)); void (*cfg_gpio)(struct platform_device *dev, int width); + void (*set_power)(bool power); }; -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] mmc: sdhci-s3c: Added set_power handler to platdata 2013-05-22 17:47 ` [PATCH 2/2] mmc: sdhci-s3c: Added set_power handler to platdata Felipe F. Tonello @ 2013-05-24 3:57 ` Jaehoon Chung 0 siblings, 0 replies; 9+ messages in thread From: Jaehoon Chung @ 2013-05-24 3:57 UTC (permalink / raw) To: Felipe F. Tonello; +Cc: linux-mmc, ben-linux, cjb, arnd, linux-kernel Hi Felipe, I didn't understand this patch, why need to add set_power? We can use to control the power with the fixed regulator. Then we can also use the regulator framework. And i know also control the module like wifi with rfkill. In set_power, what is it controlled? Best Regards, Jaehoon Chung On 05/23/2013 02:47 AM, Felipe F. Tonello wrote: > From: "Felipe F. Tonello" <eu@felipetonello.com> > > This is useful to turn off peripherals that are related to the mmc host. One > common case is when the wifi module is connected as an mmc card to the host. > > Signed-off-by: Felipe F. Tonello <eu@felipetonello.com> > --- > drivers/mmc/host/sdhci-s3c.c | 8 ++++++++ > include/linux/platform_data/mmc-sdhci-s3c.h | 1 + > 2 files changed, 9 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c > index c6f6246..f7e740c 100644 > --- a/drivers/mmc/host/sdhci-s3c.c > +++ b/drivers/mmc/host/sdhci-s3c.c > @@ -360,11 +360,19 @@ static int sdhci_s3c_platform_bus_width(struct sdhci_host *host, int width) > return 0; > } > > +static void sdhci_s3c_set_power(struct sdhci_host *host, bool power) > +{ > + struct sdhci_s3c *ourhost = to_s3c(host); > + if (ourhost->pdata->set_power) > + ourhost->pdata->set_power(power); > +} > + > static struct sdhci_ops sdhci_s3c_ops = { > .get_max_clock = sdhci_s3c_get_max_clk, > .set_clock = sdhci_s3c_set_clock, > .get_min_clock = sdhci_s3c_get_min_clock, > .platform_bus_width = sdhci_s3c_platform_bus_width, > + .set_power = sdhci_s3c_set_power, > }; > > static void sdhci_s3c_notify_change(struct platform_device *dev, int state) > diff --git a/include/linux/platform_data/mmc-sdhci-s3c.h b/include/linux/platform_data/mmc-sdhci-s3c.h > index 249f023..55be925 100644 > --- a/include/linux/platform_data/mmc-sdhci-s3c.h > +++ b/include/linux/platform_data/mmc-sdhci-s3c.h > @@ -50,6 +50,7 @@ struct s3c_sdhci_platdata { > int state)); > > void (*cfg_gpio)(struct platform_device *dev, int width); > + void (*set_power)(bool power); > }; > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-05-24 22:12 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-05-22 17:47 [PATCH 0/2] Added set_power handler to mmc sdhci host Felipe F. Tonello 2013-05-22 17:47 ` [PATCH 1/2] mmc: sdhci: Added set_power sdhci_ops handler Felipe F. Tonello 2013-05-22 20:30 ` Guennadi Liakhovetski 2013-05-22 21:22 ` Felipe Ferreri Tonello 2013-05-23 7:25 ` Guennadi Liakhovetski 2013-05-24 4:02 ` Jaehoon Chung 2013-05-24 22:12 ` Felipe Tonello 2013-05-22 17:47 ` [PATCH 2/2] mmc: sdhci-s3c: Added set_power handler to platdata Felipe F. Tonello 2013-05-24 3:57 ` Jaehoon Chung
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).