Re: [PATCH v3 8/9] omap: hsmmc: split mmc23 power control
Hi Ohad, On 08/11/2010 01:12 AM, ext Ohad Ben-Cohen wrote: Prepare for mmc3 regulator power control by splitting the power control functions of mmc2 and mmc3, and expecting mmc3 to have a single, dedicated, regulator support. Signed-off-by: Ohad Ben-Coheno...@wizery.com --- arch/arm/mach-omap2/hsmmc.c | 10 -- drivers/mmc/host/omap_hsmmc.c | 67 2 files changed, 66 insertions(+), 11 deletions(-) diff --git a/arch/arm/mach-omap2/hsmmc.c b/arch/arm/mach-omap2/hsmmc.c index 1ef54b0..5d3d789 100644 --- a/arch/arm/mach-omap2/hsmmc.c +++ b/arch/arm/mach-omap2/hsmmc.c @@ -174,7 +174,7 @@ static void omap4_hsmmc1_after_set_reg(struct device *dev, int slot, } } -static void hsmmc23_before_set_reg(struct device *dev, int slot, +static void hsmmc2_before_set_reg(struct device *dev, int slot, int power_on, int vdd) { struct omap_mmc_platform_data *mmc = dev-platform_data; @@ -325,14 +325,16 @@ void __init omap2_hsmmc_init(struct omap2_hsmmc_info *controllers) c-transceiver = 1; if (c-transceiver c-wires 4) c-wires = 4; - /* FALLTHROUGH */ - case 3: if (mmc-slots[0].features HSMMC_HAS_PBIAS) { /* off-chip level shifting, or none */ - mmc-slots[0].before_set_reg = hsmmc23_before_set_reg; + mmc-slots[0].before_set_reg = hsmmc2_before_set_reg; mmc-slots[0].after_set_reg = NULL; } break; + case 3: + mmc-slots[0].before_set_reg = NULL; + mmc-slots[0].after_set_reg = NULL; + break; default: pr_err(MMC%d configuration not supported!\n, c-mmc); kfree(mmc); diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index d50e917..6f5cea0 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -258,7 +258,7 @@ static int omap_hsmmc_1_set_power(struct device *dev, int slot, int power_on, return ret; } -static int omap_hsmmc_23_set_power(struct device *dev, int slot, int power_on, +static int omap_hsmmc_2_set_power(struct device *dev, int slot, int power_on, int vdd) { struct omap_hsmmc_host *host = @@ -309,6 +309,31 @@ static int omap_hsmmc_23_set_power(struct device *dev, int slot, int power_on, return ret; } +static int omap_hsmmc_3_set_power(struct device *dev, int slot, int power_on, + int vdd) +{ + struct omap_hsmmc_host *host = + platform_get_drvdata(to_platform_device(dev)); + int ret = 0; + + if (power_on) { + ret = mmc_regulator_set_ocr(host-vcc, vdd); + /* Enable interface voltage rail, if needed */ Why is this needed? In this case interface voltage rail seems to be the same as card supply. If you notice mmc_regulator_set_ocr code, it is doing the regulator enable, so don't need to enable it again below. + if (ret == 0 host-vcc) { + ret = regulator_enable(host-vcc); + if (ret 0) + ret = mmc_regulator_set_ocr(host-vcc, 0); + } + } else { + if (host-vcc) + ret = regulator_disable(host-vcc); + if (ret == 0) + ret = mmc_regulator_set_ocr(host-vcc, 0); + } + + return ret; +} All This can be replaced to the following to simply if (power_on) { return mmc_regulator_set_ocr(host-vcc, vdd); else return mmc_regulator_set_ocr(host-vcc, 0); regards, -roger -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 8/9] omap: hsmmc: split mmc23 power control
Ohad Ben-Cohen wrote: Prepare for mmc3 regulator power control by splitting the power control functions of mmc2 and mmc3, and expecting mmc3 to have a single, dedicated, regulator support. Why? Can't the controller be connected to an eMMC with 2 power supplies? At a glance, it is not obvious why this patch is needed. Signed-off-by: Ohad Ben-Cohen o...@wizery.com --- arch/arm/mach-omap2/hsmmc.c | 10 -- drivers/mmc/host/omap_hsmmc.c | 67 2 files changed, 66 insertions(+), 11 deletions(-) diff --git a/arch/arm/mach-omap2/hsmmc.c b/arch/arm/mach-omap2/hsmmc.c index 1ef54b0..5d3d789 100644 --- a/arch/arm/mach-omap2/hsmmc.c +++ b/arch/arm/mach-omap2/hsmmc.c @@ -174,7 +174,7 @@ static void omap4_hsmmc1_after_set_reg(struct device *dev, int slot, } } -static void hsmmc23_before_set_reg(struct device *dev, int slot, +static void hsmmc2_before_set_reg(struct device *dev, int slot, int power_on, int vdd) { struct omap_mmc_platform_data *mmc = dev-platform_data; @@ -325,14 +325,16 @@ void __init omap2_hsmmc_init(struct omap2_hsmmc_info *controllers) c-transceiver = 1; if (c-transceiver c-wires 4) c-wires = 4; - /* FALLTHROUGH */ - case 3: if (mmc-slots[0].features HSMMC_HAS_PBIAS) { /* off-chip level shifting, or none */ - mmc-slots[0].before_set_reg = hsmmc23_before_set_reg; + mmc-slots[0].before_set_reg = hsmmc2_before_set_reg; mmc-slots[0].after_set_reg = NULL; } break; + case 3: + mmc-slots[0].before_set_reg = NULL; + mmc-slots[0].after_set_reg = NULL; + break; default: pr_err(MMC%d configuration not supported!\n, c-mmc); kfree(mmc); diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index d50e917..6f5cea0 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -258,7 +258,7 @@ static int omap_hsmmc_1_set_power(struct device *dev, int slot, int power_on, return ret; } -static int omap_hsmmc_23_set_power(struct device *dev, int slot, int power_on, +static int omap_hsmmc_2_set_power(struct device *dev, int slot, int power_on, int vdd) { struct omap_hsmmc_host *host = @@ -309,6 +309,31 @@ static int omap_hsmmc_23_set_power(struct device *dev, int slot, int power_on, return ret; } +static int omap_hsmmc_3_set_power(struct device *dev, int slot, int power_on, + int vdd) +{ + struct omap_hsmmc_host *host = + platform_get_drvdata(to_platform_device(dev)); + int ret = 0; + + if (power_on) { + ret = mmc_regulator_set_ocr(host-vcc, vdd); + /* Enable interface voltage rail, if needed */ + if (ret == 0 host-vcc) { + ret = regulator_enable(host-vcc); + if (ret 0) + ret = mmc_regulator_set_ocr(host-vcc, 0); + } + } else { + if (host-vcc) + ret = regulator_disable(host-vcc); + if (ret == 0) + ret = mmc_regulator_set_ocr(host-vcc, 0); + } + + return ret; +} + static int omap_hsmmc_1_set_sleep(struct device *dev, int slot, int sleep, int vdd, int cardsleep) { @@ -319,7 +344,7 @@ static int omap_hsmmc_1_set_sleep(struct device *dev, int slot, int sleep, return regulator_set_mode(host-vcc, mode); } -static int omap_hsmmc_23_set_sleep(struct device *dev, int slot, int sleep, +static int omap_hsmmc_2_set_sleep(struct device *dev, int slot, int sleep, int vdd, int cardsleep) { struct omap_hsmmc_host *host = @@ -358,6 +383,31 @@ static int omap_hsmmc_23_set_sleep(struct device *dev, int slot, int sleep, return regulator_enable(host-vcc_aux); } +static int omap_hsmmc_3_set_sleep(struct device *dev, int slot, int sleep, + int vdd, int cardsleep) +{ + struct omap_hsmmc_host *host = + platform_get_drvdata(to_platform_device(dev)); + int err = 0; + + /* +* If we don't see a Vcc regulator, assume it's a fixed +* voltage always-on regulator. +*/ + if (!host-vcc) + return 0; + + if (cardsleep) { + /* VCC can be turned off if card is asleep */ + if (sleep) + err =
Re: [PATCH v3 8/9] omap: hsmmc: split mmc23 power control
On 08/11/2010 01:05 PM, Hunter Adrian (Nokia-MS/Helsinki) wrote: Ohad Ben-Cohen wrote: Prepare for mmc3 regulator power control by splitting the power control functions of mmc2 and mmc3, and expecting mmc3 to have a single, dedicated, regulator support. Why? Can't the controller be connected to an eMMC with 2 power supplies? At a glance, it is not obvious why this patch is needed. I agree with Adrian. This patch is not required at all. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 8/9] omap: hsmmc: split mmc23 power control
On Wed, Aug 11, 2010 at 1:52 PM, Roger Quadros roger.quad...@nokia.com wrote: On 08/11/2010 01:05 PM, Hunter Adrian (Nokia-MS/Helsinki) wrote: Ohad Ben-Cohen wrote: Prepare for mmc3 regulator power control by splitting the power control functions of mmc2 and mmc3, and expecting mmc3 to have a single, dedicated, regulator support. Why? Can't the controller be connected to an eMMC with 2 power supplies? At a glance, it is not obvious why this patch is needed. I agree with Adrian. This patch is not required at all. Thanks guys, I'll check it out. I'm about to leave until e/o August and I still have to sort out some stuff, so I might not respond right away, but please send any comments you may still have. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 8/9] omap: hsmmc: split mmc23 power control
On Wed, Aug 11, 2010 at 1:05 PM, Adrian Hunter adrian.hun...@nokia.com wrote: Ohad Ben-Cohen wrote: Prepare for mmc3 regulator power control by splitting the power control functions of mmc2 and mmc3, and expecting mmc3 to have a single, dedicated, regulator support. Why? Can't the controller be connected to an eMMC with 2 power supplies? At a glance, it is not obvious why this patch is needed. I had ZOOM too much in mind when I did that. Removed now, thanks ! -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 8/9] omap: hsmmc: split mmc23 power control
Prepare for mmc3 regulator power control by splitting the power control functions of mmc2 and mmc3, and expecting mmc3 to have a single, dedicated, regulator support. Signed-off-by: Ohad Ben-Cohen o...@wizery.com --- arch/arm/mach-omap2/hsmmc.c | 10 -- drivers/mmc/host/omap_hsmmc.c | 67 2 files changed, 66 insertions(+), 11 deletions(-) diff --git a/arch/arm/mach-omap2/hsmmc.c b/arch/arm/mach-omap2/hsmmc.c index 1ef54b0..5d3d789 100644 --- a/arch/arm/mach-omap2/hsmmc.c +++ b/arch/arm/mach-omap2/hsmmc.c @@ -174,7 +174,7 @@ static void omap4_hsmmc1_after_set_reg(struct device *dev, int slot, } } -static void hsmmc23_before_set_reg(struct device *dev, int slot, +static void hsmmc2_before_set_reg(struct device *dev, int slot, int power_on, int vdd) { struct omap_mmc_platform_data *mmc = dev-platform_data; @@ -325,14 +325,16 @@ void __init omap2_hsmmc_init(struct omap2_hsmmc_info *controllers) c-transceiver = 1; if (c-transceiver c-wires 4) c-wires = 4; - /* FALLTHROUGH */ - case 3: if (mmc-slots[0].features HSMMC_HAS_PBIAS) { /* off-chip level shifting, or none */ - mmc-slots[0].before_set_reg = hsmmc23_before_set_reg; + mmc-slots[0].before_set_reg = hsmmc2_before_set_reg; mmc-slots[0].after_set_reg = NULL; } break; + case 3: + mmc-slots[0].before_set_reg = NULL; + mmc-slots[0].after_set_reg = NULL; + break; default: pr_err(MMC%d configuration not supported!\n, c-mmc); kfree(mmc); diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index d50e917..6f5cea0 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -258,7 +258,7 @@ static int omap_hsmmc_1_set_power(struct device *dev, int slot, int power_on, return ret; } -static int omap_hsmmc_23_set_power(struct device *dev, int slot, int power_on, +static int omap_hsmmc_2_set_power(struct device *dev, int slot, int power_on, int vdd) { struct omap_hsmmc_host *host = @@ -309,6 +309,31 @@ static int omap_hsmmc_23_set_power(struct device *dev, int slot, int power_on, return ret; } +static int omap_hsmmc_3_set_power(struct device *dev, int slot, int power_on, + int vdd) +{ + struct omap_hsmmc_host *host = + platform_get_drvdata(to_platform_device(dev)); + int ret = 0; + + if (power_on) { + ret = mmc_regulator_set_ocr(host-vcc, vdd); + /* Enable interface voltage rail, if needed */ + if (ret == 0 host-vcc) { + ret = regulator_enable(host-vcc); + if (ret 0) + ret = mmc_regulator_set_ocr(host-vcc, 0); + } + } else { + if (host-vcc) + ret = regulator_disable(host-vcc); + if (ret == 0) + ret = mmc_regulator_set_ocr(host-vcc, 0); + } + + return ret; +} + static int omap_hsmmc_1_set_sleep(struct device *dev, int slot, int sleep, int vdd, int cardsleep) { @@ -319,7 +344,7 @@ static int omap_hsmmc_1_set_sleep(struct device *dev, int slot, int sleep, return regulator_set_mode(host-vcc, mode); } -static int omap_hsmmc_23_set_sleep(struct device *dev, int slot, int sleep, +static int omap_hsmmc_2_set_sleep(struct device *dev, int slot, int sleep, int vdd, int cardsleep) { struct omap_hsmmc_host *host = @@ -358,6 +383,31 @@ static int omap_hsmmc_23_set_sleep(struct device *dev, int slot, int sleep, return regulator_enable(host-vcc_aux); } +static int omap_hsmmc_3_set_sleep(struct device *dev, int slot, int sleep, + int vdd, int cardsleep) +{ + struct omap_hsmmc_host *host = + platform_get_drvdata(to_platform_device(dev)); + int err = 0; + + /* +* If we don't see a Vcc regulator, assume it's a fixed +* voltage always-on regulator. +*/ + if (!host-vcc) + return 0; + + if (cardsleep) { + /* VCC can be turned off if card is asleep */ + if (sleep) + err = mmc_regulator_set_ocr(host-vcc, 0); + else + err = mmc_regulator_set_ocr(host-vcc, vdd); + } + + return err; +} + static int