From: "Felipe F. Tonello" <[email protected]>
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
From: "Felipe F. Tonello" <[email protected]>
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 <[email protected]>
---
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
From: "Felipe F. Tonello" <[email protected]>
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 <[email protected]>
---
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
On Wed, 22 May 2013, Felipe F. Tonello wrote:
> From: "Felipe F. Tonello" <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 [email protected]
> 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/
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" <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> >
> > 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/
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" <[email protected]>
> > >
> > > 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/
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" <[email protected]>
>
> 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 <[email protected]>
> ---
> 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);
> };
>
>
>
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" <[email protected]>
>>>>
>>>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Hi all,
On Thu, May 23, 2013 at 9:02 PM, Jaehoon Chung <[email protected]> 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" <[email protected]>
>>>>>
>>>>> 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 [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>