Re: [PATCH V2 1/3] mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators

2014-10-02 Thread Bartlomiej Zolnierkiewicz 

Hi,

On Wednesday, October 01, 2014 09:04:09 AM Doug Anderson wrote:
 Hi,
 
 On Wed, Oct 1, 2014 at 7:00 AM, Bartlomiej Zolnierkiewicz
 b.zolnier...@samsung.com wrote:
 
  Hi,
 
  On Wednesday, October 01, 2014 12:47:52 AM YUVARAJ CD wrote:
 
  Since I am out of station, i dont have an access to my work set up.
  Can you send me the dts entries of sd crad and their corresponding 
  regulator entries?
 
  From arch/arm/boot/dts/exynos5420-arndale-octa.dts:
 
  ...
  mmc@1220 {
  status = okay;
  broken-cd;
  supports-highspeed;
  card-detect-delay = 200;
  samsung,dw-mshc-ciu-div = 3;
  samsung,dw-mshc-sdr-timing = 0 4;
  samsung,dw-mshc-ddr-timing = 0 2;
  pinctrl-names = default;
  pinctrl-0 = sd0_clk sd0_cmd sd0_bus4 sd0_bus8;
  vmmc-supply = ldo10_reg;
 
  slot@0 {
  reg = 0;
  bus-width = 8;
  };
  };
 
  mmc@1222 {
  status = okay;
  supports-highspeed;
  card-detect-delay = 200;
  samsung,dw-mshc-ciu-div = 3;
  samsung,dw-mshc-sdr-timing = 2 3;
  samsung,dw-mshc-ddr-timing = 1 2;
  pinctrl-names = default;
  pinctrl-0 = sd2_clk sd2_cmd sd2_cd sd2_bus4;
  vmmc-supply = ldo10_reg;
 
  slot@0 {
  reg = 0;
  bus-width = 4;
  };
  };
  ...
  ldo10_reg: LDO10 {
  regulator-name = PVDD_PRE_1V8;
  regulator-min-microvolt = 180;
  regulator-max-microvolt = 180;
  regulator-always-on;
  };
 
 I don't have schematics for Arndale Octa, but the above is really
 fishy.  vmmc shouldn't be 1.8V.  That's the general power signal to
 MMC and should be 2.7V - 3.6V.  vqmmc could be 1.8V in certain
 situations, but my understanding is that for maximum compatibility it
 should at least start out identical to vmmc (and later go down to
 1.7V - 1.95V).
 
 My first thought would be to just remove the vmmc-supply from your
 DTS.  I think we could land that and pick it back easily.  That will
 get you something working and won't introduce any regressions because:
 1. MMC core will give you a dummy regulator
 2. The code will default to assuming that vmmc is 3.3V, which is what
 it used to do anyway.
 3. The only referenced regulator is always on anyway.
 
 Separately you could specify a proper vmmc and maybe even a vqmmc.
 
 On SMDK5420 I see this for the SD card (mmc2):
 * vmmc should be VDD_SD_2V8.  From LDO19.
 * vqmmc should be VDDQ_MMC2_AP.  From LDO13.
 
 OK, I dug up the Arndale schematics.  For mmc2:
 * vmmc should be PVDD_TFLASH_2V8.  That's LDO19.
 * vqmmc (hooked up to VDDQ_MMC2): PVDD_APIO_MMCOFF_2V8.  LDO13 just like SMDK.
 
 ...sadly it looks like Anrdale has a schematics problem that prevents
 you from doing UHS.  I see that the data lines are pulled up to
 PVDD_TFLASH_2V8 (vmmc), not pulled up to PVDD_APIO_MMCOFF_2V8 (vqmmc).
 I think that means that if you ever lower vqmmc to 1.8V (as needed for
 UHS) then you'll still be pulling up to 2.8V.  That's not good.  You
 should probably make sure that both LDO13 and LDO19 are listed as
 being exactly 2.8V.
 
 
 Anyway, the above has (obviously) not been tested and is just based on
 a casual browsing of schematics.  Please let me know how it goes.

Removing the vmmc-supply entry fixes the problem.  Using LDO13 for vmmc
and LDO19 for vqmmc also works fine.  I'll post a patch fixing the mmc2 DT
entry to Kukjin in a few minutes.  Thank you for your help.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung RD Institute Poland
Samsung Electronics

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

Re: [PATCH V2 1/3] mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators

2014-10-01 Thread Bartlomiej Zolnierkiewicz 

Hi,

On Tuesday, September 30, 2014 10:22:34 AM Doug Anderson wrote:
 Bartlomiej
 
 On Mon, Sep 29, 2014 at 5:31 AM, Bartlomiej Zolnierkiewicz
 b.zolnier...@samsung.com wrote:
 
  Hi,
 
  On Friday, August 29, 2014 01:34:44 PM Ulf Hansson wrote:
  On 22 August 2014 15:47, Yuvaraj Kumar C D yuvaraj...@gmail.com wrote:
   This patch makes use of mmc_regulator_get_supply() to handle
   the vmmc and vqmmc regulators.Also it moves the code handling
   the these regulators to dw_mci_set_ios().It turned on the vmmc
   and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
   during MMC_POWER_OFF.
  
   Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
 
  Thanks! Applied for next.
 
  Unfortunately this patch breaks mmc1 card (Kingston 32GB micro SDHC)
  detection on Exynos5420 Arndale Octa for me:
 
  [   10.797979] dwmmc_exynos 1222.mmc: no support for card's volts
  [   10.797998] mmc1: error -22 whilst initialising SD card
 
  Without the patch:
 
  [   10.866926] mmc_host mmc1: Bus speed (slot 0) = 5000Hz (slot req 
  5000Hz, actual 5000HZ div = 0)
  [   10.866977] mmc1: new high speed SDHC card at address 1234
  [   10.868730] mmcblk1: mmc1:1234 SA32G 29.3 GiB
  [   10.915054]  mmcblk1: p1 p2 p3
 
  The config is attached (exynos_defconfig doesn't work correctly for
  this board yet).
 
 Yup, this is an expected behavior, unfortunately.  This was talked
 about extensively during the review of this patch series.

Do you mean that a patch with a known regression has been merged
into next branch of mmc tree?  It would be quite sad if it would be
true.

 I believe that patch #3 in Yuvaraj's series would fix your problem.
 Specifically https://patchwork.kernel.org/patch/4763891/.

Unfortunately this patch doesn't fix the problem (there is no longer
a lockup on regulators initialization but -22 error is still present).

 The current summary of this issue is (Ulf, please correct me if I got
 anything wrong):
 
 1. If nothing else, Yuvaraj's patch should probably be split in two.
 One half should be the MMC core half that I originally sent Yuvaraj.
 I just rebased and re-uploaed it at
 https://chromium-review.googlesource.com/220560 in case you're
 curious.  The second half should be the dw_mmc piece that Yuvaraj
 wrote.
 
 2. Ulf has indicated that he thinks that the MMC core change (and thus
 Yuvaraj's patch) is ugly and not necessary.  He advocates instead
 using the MMC_CAP_NEEDS_POLL on all affected platforms.  That means
 we'll turn on the power every second, check for the card, then turn
 off power.
 
 3. I'm still of the opinion that the MMC core change isn't _that_
 ugly.  Given that there are a large number of systems affected (across

It also doesn't look that bad for me and well, when the hardware is
quirky then the resulting code can't be esthetically perfect..

 at least two SoC vendors) and that it would be nice if those systems
 didn't have to poll, I'd still be happy if the MMC core change could
 go in.  ...but I'm a pragmatist and know that the polling isn't
 _terrible_, so if that's the way we need to go then so be it.
 
 --
 
 My understanding was that Yuvaraj was going to spin his patch to use a
 similar type of scheme to autodetect affected SoCs (looking for SoCs
 known to have this problem where the platform data shows them using
 the built-in card detect) and then enable MMC_CAP_NEEDS_POLL.  I
 haven't seen a patch from him, so maybe you could post this up?

I worry that I have too little time and MMC code expertise to do this.

 Note: the reason why exynos5250-snow and some other boards aren't
 affected yet is that the regulator is simply not specified in the
 device tree (it's just left always on).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung RD Institute Poland
Samsung Electronics

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

Re: [PATCH V2 1/3] mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators

2014-10-01 Thread Bartlomiej Zolnierkiewicz 

Hi,

On Tuesday, September 30, 2014 02:23:44 PM Jaehoon Chung wrote:
 Hi
 
 On 09/29/2014 09:31 PM, Bartlomiej Zolnierkiewicz wrote:
  
  Hi,
  
  On Friday, August 29, 2014 01:34:44 PM Ulf Hansson wrote:
  On 22 August 2014 15:47, Yuvaraj Kumar C D yuvaraj...@gmail.com wrote:
  This patch makes use of mmc_regulator_get_supply() to handle
  the vmmc and vqmmc regulators.Also it moves the code handling
  the these regulators to dw_mci_set_ios().It turned on the vmmc
  and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
  during MMC_POWER_OFF.
 
  Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
 
  Thanks! Applied for next.
  
  Unfortunately this patch breaks mmc1 card (Kingston 32GB micro SDHC)
  detection on Exynos5420 Arndale Octa for me:
  
  [   10.797979] dwmmc_exynos 1222.mmc: no support for card's volts
  [   10.797998] mmc1: error -22 whilst initialising SD card
 
 OCR value is not matched. Which values are supported about the mmc_host's 
 value and card's value?
 Could you share the value?

Sure.  I've added dev_info()s at the beginning of mmc_select_voltage():

+   dev_warn(mmc_dev(host), card's volts: 0x%0x\n, ocr);
+   dev_warn(mmc_dev(host), host's volts: 0x%0x\n, host-ocr_avail);

and got following results:

[   10.851678] dwmmc_exynos 1222.mmc: card's volts: 0xff8000
[   10.851691] dwmmc_exynos 1222.mmc: host's volts: 0x80

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung RD Institute Poland
Samsung Electronics

 Best Regards,
 Jaehoon Chung
 
  
  Without the patch:
  
  [   10.866926] mmc_host mmc1: Bus speed (slot 0) = 5000Hz (slot req 
  5000Hz, actual 5000HZ div = 0)
  [   10.866977] mmc1: new high speed SDHC card at address 1234
  [   10.868730] mmcblk1: mmc1:1234 SA32G 29.3 GiB 
  [   10.915054]  mmcblk1: p1 p2 p3
  
  The config is attached (exynos_defconfig doesn't work correctly for
  this board yet).
  
  Best regards,
  --
  Bartlomiej Zolnierkiewicz
  Samsung RD Institute Poland
  Samsung Electronics
  
  Kind regards
  Uffe
 
  ---
  changes from v1:
  1.Used mmc_regulator_set_ocr() instead of regulator_enable() for 
  vmmc.
  2.Turned on vmmc and vqmmc during MMC_POWER_UP.
  3. Removed the flags DW_MMC_CARD_POWERED and DW_MMC_IO_POWERED 
  which
 added during the initial version of this patch.
  4. Added error message, if it failed to turn on regulator's.
 
   drivers/mmc/host/dw_mmc.c  |   72 
  +---
   include/linux/mmc/dw_mmc.h |2 +-
   2 files changed, 36 insertions(+), 38 deletions(-)
 
  diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
  index 7f227e9..aadb0d6 100644
  --- a/drivers/mmc/host/dw_mmc.c
  +++ b/drivers/mmc/host/dw_mmc.c
  @@ -936,6 +936,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc, 
  struct mmc_ios *ios)
  struct dw_mci_slot *slot = mmc_priv(mmc);
  const struct dw_mci_drv_data *drv_data = slot-host-drv_data;
  u32 regs;
  +   int ret;
 
  switch (ios-bus_width) {
  case MMC_BUS_WIDTH_4:
  @@ -974,12 +975,38 @@ static void dw_mci_set_ios(struct mmc_host *mmc, 
  struct mmc_ios *ios)
 
  switch (ios-power_mode) {
  case MMC_POWER_UP:
  +   if (!IS_ERR(mmc-supply.vmmc)) {
  +   ret = mmc_regulator_set_ocr(mmc, mmc-supply.vmmc,
  +   ios-vdd);
  +   if (ret) {
  +   dev_err(slot-host-dev,
  +   failed to enable vmmc 
  regulator\n);
  +   /*return, if failed turn on vmmc*/
  +   return;
  +   }
  +   }
  +   if (!IS_ERR(mmc-supply.vqmmc)  
  !slot-host-vqmmc_enabled) {
  +   ret = regulator_enable(mmc-supply.vqmmc);
  +   if (ret  0)
  +   dev_err(slot-host-dev,
  +   failed to enable vqmmc 
  regulator\n);
  +   else
  +   slot-host-vqmmc_enabled = true;
  +   }
  set_bit(DW_MMC_CARD_NEED_INIT, slot-flags);
  regs = mci_readl(slot-host, PWREN);
  regs |= (1  slot-id);
  mci_writel(slot-host, PWREN, regs);
  break;
  case MMC_POWER_OFF:
  +   if (!IS_ERR(mmc-supply.vmmc))
  +   mmc_regulator_set_ocr(mmc, mmc-supply.vmmc, 0);
  +
  +   if (!IS_ERR(mmc-supply.vqmmc)  
  slot-host-vqmmc_enabled) {
  +   regulator_disable(mmc-supply.vqmmc);
  +   slot-host-vqmmc_enabled = false;
  +   }
  +
  regs = mci_readl(slot-host, PWREN);
  regs = ~(1  slot-id);
  mci_writel(slot-host, PWREN, regs);
  @@ -2110,7

Re: [PATCH V2 1/3] mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators

2014-10-01 Thread Bartlomiej Zolnierkiewicz 

Hi,

On Wednesday, October 01, 2014 12:47:52 AM YUVARAJ CD wrote:

 Since I am out of station, i dont have an access to my work set up.
 Can you send me the dts entries of sd crad and their corresponding regulator 
 entries?

From arch/arm/boot/dts/exynos5420-arndale-octa.dts:

...
mmc@1220 {
status = okay;
broken-cd;
supports-highspeed;
card-detect-delay = 200;
samsung,dw-mshc-ciu-div = 3;
samsung,dw-mshc-sdr-timing = 0 4;
samsung,dw-mshc-ddr-timing = 0 2;
pinctrl-names = default;
pinctrl-0 = sd0_clk sd0_cmd sd0_bus4 sd0_bus8;
vmmc-supply = ldo10_reg;

slot@0 {
reg = 0;
bus-width = 8;
};
};

mmc@1222 {
status = okay;
supports-highspeed;
card-detect-delay = 200;
samsung,dw-mshc-ciu-div = 3;
samsung,dw-mshc-sdr-timing = 2 3;
samsung,dw-mshc-ddr-timing = 1 2;
pinctrl-names = default;
pinctrl-0 = sd2_clk sd2_cmd sd2_cd sd2_bus4;
vmmc-supply = ldo10_reg;

slot@0 {
reg = 0;
bus-width = 4;
};
};
...
ldo10_reg: LDO10 {
regulator-name = PVDD_PRE_1V8;
regulator-min-microvolt = 180;
regulator-max-microvolt = 180;
regulator-always-on;
};
...

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung RD Institute Poland
Samsung Electronics

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

Re: [PATCH V2 1/3] mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators

2014-10-01 Thread Bartlomiej Zolnierkiewicz 
On Wednesday, October 01, 2014 03:57:54 PM Bartlomiej Zolnierkiewicz wrote:
 
 Hi,
 
 On Tuesday, September 30, 2014 02:23:44 PM Jaehoon Chung wrote:
  Hi
  
  On 09/29/2014 09:31 PM, Bartlomiej Zolnierkiewicz wrote:
   
   Hi,
   
   On Friday, August 29, 2014 01:34:44 PM Ulf Hansson wrote:
   On 22 August 2014 15:47, Yuvaraj Kumar C D yuvaraj...@gmail.com wrote:
   This patch makes use of mmc_regulator_get_supply() to handle
   the vmmc and vqmmc regulators.Also it moves the code handling
   the these regulators to dw_mci_set_ios().It turned on the vmmc
   and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
   during MMC_POWER_OFF.
  
   Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
  
   Thanks! Applied for next.
   
   Unfortunately this patch breaks mmc1 card (Kingston 32GB micro SDHC)
   detection on Exynos5420 Arndale Octa for me:
   
   [   10.797979] dwmmc_exynos 1222.mmc: no support for card's volts
   [   10.797998] mmc1: error -22 whilst initialising SD card
  
  OCR value is not matched. Which values are supported about the mmc_host's 
  value and card's value?
  Could you share the value?
 
 Sure.  I've added dev_info()s at the beginning of mmc_select_voltage():
 
 + dev_warn(mmc_dev(host), card's volts: 0x%0x\n, ocr);
 + dev_warn(mmc_dev(host), host's volts: 0x%0x\n, host-ocr_avail);
 
 and got following results:
 
 [   10.851678] dwmmc_exynos 1222.mmc: card's volts: 0xff8000
 [   10.851691] dwmmc_exynos 1222.mmc: host's volts: 0x80

Data for the working kernel:

[   10.856214] dwmmc_exynos 1222.mmc: card's volts: 0xff8000
[   10.856227] dwmmc_exynos 1222.mmc: host's volts: 0x30

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung RD Institute Poland
Samsung Electronics

 Best regards,
 --
 Bartlomiej Zolnierkiewicz
 Samsung RD Institute Poland
 Samsung Electronics
 
  Best Regards,
  Jaehoon Chung
  
   
   Without the patch:
   
   [   10.866926] mmc_host mmc1: Bus speed (slot 0) = 5000Hz (slot req 
   5000Hz, actual 5000HZ div = 0)
   [   10.866977] mmc1: new high speed SDHC card at address 1234
   [   10.868730] mmcblk1: mmc1:1234 SA32G 29.3 GiB 
   [   10.915054]  mmcblk1: p1 p2 p3
   
   The config is attached (exynos_defconfig doesn't work correctly for
   this board yet).
   
   Best regards,
   --
   Bartlomiej Zolnierkiewicz
   Samsung RD Institute Poland
   Samsung Electronics
   
   Kind regards
   Uffe
  
   ---
   changes from v1:
   1.Used mmc_regulator_set_ocr() instead of regulator_enable() 
   for vmmc.
   2.Turned on vmmc and vqmmc during MMC_POWER_UP.
   3. Removed the flags DW_MMC_CARD_POWERED and DW_MMC_IO_POWERED 
   which
  added during the initial version of this patch.
   4. Added error message, if it failed to turn on regulator's.
  
drivers/mmc/host/dw_mmc.c  |   72 
   +---
include/linux/mmc/dw_mmc.h |2 +-
2 files changed, 36 insertions(+), 38 deletions(-)
  
   diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
   index 7f227e9..aadb0d6 100644
   --- a/drivers/mmc/host/dw_mmc.c
   +++ b/drivers/mmc/host/dw_mmc.c
   @@ -936,6 +936,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc, 
   struct mmc_ios *ios)
   struct dw_mci_slot *slot = mmc_priv(mmc);
   const struct dw_mci_drv_data *drv_data = slot-host-drv_data;
   u32 regs;
   +   int ret;
  
   switch (ios-bus_width) {
   case MMC_BUS_WIDTH_4:
   @@ -974,12 +975,38 @@ static void dw_mci_set_ios(struct mmc_host *mmc, 
   struct mmc_ios *ios)
  
   switch (ios-power_mode) {
   case MMC_POWER_UP:
   +   if (!IS_ERR(mmc-supply.vmmc)) {
   +   ret = mmc_regulator_set_ocr(mmc, 
   mmc-supply.vmmc,
   +   ios-vdd);
   +   if (ret) {
   +   dev_err(slot-host-dev,
   +   failed to enable vmmc 
   regulator\n);
   +   /*return, if failed turn on vmmc*/
   +   return;
   +   }
   +   }
   +   if (!IS_ERR(mmc-supply.vqmmc)  
   !slot-host-vqmmc_enabled) {
   +   ret = regulator_enable(mmc-supply.vqmmc);
   +   if (ret  0)
   +   dev_err(slot-host-dev,
   +   failed to enable vqmmc 
   regulator\n);
   +   else
   +   slot-host-vqmmc_enabled = true;
   +   }
   set_bit(DW_MMC_CARD_NEED_INIT, slot-flags);
   regs = mci_readl(slot-host, PWREN);
   regs |= (1  slot-id);
   mci_writel(slot-host, PWREN, regs);
   break;
   case MMC_POWER_OFF:
   +   if (!IS_ERR(mmc-supply.vmmc))
   +

Re: [PATCH V2 1/3] mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators

2014-10-01 Thread Doug Anderson 
Hi,

On Wed, Oct 1, 2014 at 6:06 AM, Bartlomiej Zolnierkiewicz
b.zolnier...@samsung.com wrote:

 Hi,

 On Tuesday, September 30, 2014 10:22:34 AM Doug Anderson wrote:
 Bartlomiej

 On Mon, Sep 29, 2014 at 5:31 AM, Bartlomiej Zolnierkiewicz
 b.zolnier...@samsung.com wrote:
 
  Hi,
 
  On Friday, August 29, 2014 01:34:44 PM Ulf Hansson wrote:
  On 22 August 2014 15:47, Yuvaraj Kumar C D yuvaraj...@gmail.com wrote:
   This patch makes use of mmc_regulator_get_supply() to handle
   the vmmc and vqmmc regulators.Also it moves the code handling
   the these regulators to dw_mci_set_ios().It turned on the vmmc
   and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
   during MMC_POWER_OFF.
  
   Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
 
  Thanks! Applied for next.
 
  Unfortunately this patch breaks mmc1 card (Kingston 32GB micro SDHC)
  detection on Exynos5420 Arndale Octa for me:
 
  [   10.797979] dwmmc_exynos 1222.mmc: no support for card's volts
  [   10.797998] mmc1: error -22 whilst initialising SD card
 
  Without the patch:
 
  [   10.866926] mmc_host mmc1: Bus speed (slot 0) = 5000Hz (slot req 
  5000Hz, actual 5000HZ div = 0)
  [   10.866977] mmc1: new high speed SDHC card at address 1234
  [   10.868730] mmcblk1: mmc1:1234 SA32G 29.3 GiB
  [   10.915054]  mmcblk1: p1 p2 p3
 
  The config is attached (exynos_defconfig doesn't work correctly for
  this board yet).

 Yup, this is an expected behavior, unfortunately.  This was talked
 about extensively during the review of this patch series.

 Do you mean that a patch with a known regression has been merged
 into next branch of mmc tree?  It would be quite sad if it would be
 true.

...so looking at your dts file, it looks like this _isn't_ your
problem.  Your vmmc regulator is listed as always on, so I believe
that you're OK.  Your regulator probably _shouldn't_ be always on
(it prevents power cycling the SD card) but right now it's good that
it is...

If any board has a vmmc that is not listed as always on then it
would regress with the first two patches applied (and without the
3rd), but there are none that I personally know of that do.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 1/3] mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators

2014-10-01 Thread Doug Anderson 
Hi,

On Wed, Oct 1, 2014 at 7:00 AM, Bartlomiej Zolnierkiewicz
b.zolnier...@samsung.com wrote:

 Hi,

 On Wednesday, October 01, 2014 12:47:52 AM YUVARAJ CD wrote:

 Since I am out of station, i dont have an access to my work set up.
 Can you send me the dts entries of sd crad and their corresponding regulator 
 entries?

 From arch/arm/boot/dts/exynos5420-arndale-octa.dts:

 ...
 mmc@1220 {
 status = okay;
 broken-cd;
 supports-highspeed;
 card-detect-delay = 200;
 samsung,dw-mshc-ciu-div = 3;
 samsung,dw-mshc-sdr-timing = 0 4;
 samsung,dw-mshc-ddr-timing = 0 2;
 pinctrl-names = default;
 pinctrl-0 = sd0_clk sd0_cmd sd0_bus4 sd0_bus8;
 vmmc-supply = ldo10_reg;

 slot@0 {
 reg = 0;
 bus-width = 8;
 };
 };

 mmc@1222 {
 status = okay;
 supports-highspeed;
 card-detect-delay = 200;
 samsung,dw-mshc-ciu-div = 3;
 samsung,dw-mshc-sdr-timing = 2 3;
 samsung,dw-mshc-ddr-timing = 1 2;
 pinctrl-names = default;
 pinctrl-0 = sd2_clk sd2_cmd sd2_cd sd2_bus4;
 vmmc-supply = ldo10_reg;

 slot@0 {
 reg = 0;
 bus-width = 4;
 };
 };
 ...
 ldo10_reg: LDO10 {
 regulator-name = PVDD_PRE_1V8;
 regulator-min-microvolt = 180;
 regulator-max-microvolt = 180;
 regulator-always-on;
 };

I don't have schematics for Arndale Octa, but the above is really
fishy.  vmmc shouldn't be 1.8V.  That's the general power signal to
MMC and should be 2.7V - 3.6V.  vqmmc could be 1.8V in certain
situations, but my understanding is that for maximum compatibility it
should at least start out identical to vmmc (and later go down to
1.7V - 1.95V).

My first thought would be to just remove the vmmc-supply from your
DTS.  I think we could land that and pick it back easily.  That will
get you something working and won't introduce any regressions because:
1. MMC core will give you a dummy regulator
2. The code will default to assuming that vmmc is 3.3V, which is what
it used to do anyway.
3. The only referenced regulator is always on anyway.

Separately you could specify a proper vmmc and maybe even a vqmmc.

On SMDK5420 I see this for the SD card (mmc2):
* vmmc should be VDD_SD_2V8.  From LDO19.
* vqmmc should be VDDQ_MMC2_AP.  From LDO13.

OK, I dug up the Arndale schematics.  For mmc2:
* vmmc should be PVDD_TFLASH_2V8.  That's LDO19.
* vqmmc (hooked up to VDDQ_MMC2): PVDD_APIO_MMCOFF_2V8.  LDO13 just like SMDK.

...sadly it looks like Anrdale has a schematics problem that prevents
you from doing UHS.  I see that the data lines are pulled up to
PVDD_TFLASH_2V8 (vmmc), not pulled up to PVDD_APIO_MMCOFF_2V8 (vqmmc).
I think that means that if you ever lower vqmmc to 1.8V (as needed for
UHS) then you'll still be pulling up to 2.8V.  That's not good.  You
should probably make sure that both LDO13 and LDO19 are listed as
being exactly 2.8V.


Anyway, the above has (obviously) not been tested and is just based on
a casual browsing of schematics.  Please let me know how it goes.



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

Re: [PATCH V2 1/3] mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators

2014-09-30 Thread Doug Anderson 
Bartlomiej

On Mon, Sep 29, 2014 at 5:31 AM, Bartlomiej Zolnierkiewicz
b.zolnier...@samsung.com wrote:

 Hi,

 On Friday, August 29, 2014 01:34:44 PM Ulf Hansson wrote:
 On 22 August 2014 15:47, Yuvaraj Kumar C D yuvaraj...@gmail.com wrote:
  This patch makes use of mmc_regulator_get_supply() to handle
  the vmmc and vqmmc regulators.Also it moves the code handling
  the these regulators to dw_mci_set_ios().It turned on the vmmc
  and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
  during MMC_POWER_OFF.
 
  Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com

 Thanks! Applied for next.

 Unfortunately this patch breaks mmc1 card (Kingston 32GB micro SDHC)
 detection on Exynos5420 Arndale Octa for me:

 [   10.797979] dwmmc_exynos 1222.mmc: no support for card's volts
 [   10.797998] mmc1: error -22 whilst initialising SD card

 Without the patch:

 [   10.866926] mmc_host mmc1: Bus speed (slot 0) = 5000Hz (slot req 
 5000Hz, actual 5000HZ div = 0)
 [   10.866977] mmc1: new high speed SDHC card at address 1234
 [   10.868730] mmcblk1: mmc1:1234 SA32G 29.3 GiB
 [   10.915054]  mmcblk1: p1 p2 p3

 The config is attached (exynos_defconfig doesn't work correctly for
 this board yet).

Yup, this is an expected behavior, unfortunately.  This was talked
about extensively during the review of this patch series.

I believe that patch #3 in Yuvaraj's series would fix your problem.
Specifically https://patchwork.kernel.org/patch/4763891/.


The current summary of this issue is (Ulf, please correct me if I got
anything wrong):

1. If nothing else, Yuvaraj's patch should probably be split in two.
One half should be the MMC core half that I originally sent Yuvaraj.
I just rebased and re-uploaed it at
https://chromium-review.googlesource.com/220560 in case you're
curious.  The second half should be the dw_mmc piece that Yuvaraj
wrote.

2. Ulf has indicated that he thinks that the MMC core change (and thus
Yuvaraj's patch) is ugly and not necessary.  He advocates instead
using the MMC_CAP_NEEDS_POLL on all affected platforms.  That means
we'll turn on the power every second, check for the card, then turn
off power.

3. I'm still of the opinion that the MMC core change isn't _that_
ugly.  Given that there are a large number of systems affected (across
at least two SoC vendors) and that it would be nice if those systems
didn't have to poll, I'd still be happy if the MMC core change could
go in.  ...but I'm a pragmatist and know that the polling isn't
_terrible_, so if that's the way we need to go then so be it.

--

My understanding was that Yuvaraj was going to spin his patch to use a
similar type of scheme to autodetect affected SoCs (looking for SoCs
known to have this problem where the platform data shows them using
the built-in card detect) and then enable MMC_CAP_NEEDS_POLL.  I
haven't seen a patch from him, so maybe you could post this up?

Note: the reason why exynos5250-snow and some other boards aren't
affected yet is that the regulator is simply not specified in the
device tree (it's just left always on).

---

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

Re: [PATCH V2 1/3] mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators

2014-09-29 Thread Bartlomiej Zolnierkiewicz 

Hi,

On Friday, August 29, 2014 01:34:44 PM Ulf Hansson wrote:
 On 22 August 2014 15:47, Yuvaraj Kumar C D yuvaraj...@gmail.com wrote:
  This patch makes use of mmc_regulator_get_supply() to handle
  the vmmc and vqmmc regulators.Also it moves the code handling
  the these regulators to dw_mci_set_ios().It turned on the vmmc
  and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
  during MMC_POWER_OFF.
 
  Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
 
 Thanks! Applied for next.

Unfortunately this patch breaks mmc1 card (Kingston 32GB micro SDHC)
detection on Exynos5420 Arndale Octa for me:

[   10.797979] dwmmc_exynos 1222.mmc: no support for card's volts
[   10.797998] mmc1: error -22 whilst initialising SD card

Without the patch:

[   10.866926] mmc_host mmc1: Bus speed (slot 0) = 5000Hz (slot req 
5000Hz, actual 5000HZ div = 0)
[   10.866977] mmc1: new high speed SDHC card at address 1234
[   10.868730] mmcblk1: mmc1:1234 SA32G 29.3 GiB 
[   10.915054]  mmcblk1: p1 p2 p3

The config is attached (exynos_defconfig doesn't work correctly for
this board yet).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung RD Institute Poland
Samsung Electronics

 Kind regards
 Uffe
 
  ---
  changes from v1:
  1.Used mmc_regulator_set_ocr() instead of regulator_enable() for 
  vmmc.
  2.Turned on vmmc and vqmmc during MMC_POWER_UP.
  3. Removed the flags DW_MMC_CARD_POWERED and DW_MMC_IO_POWERED which
 added during the initial version of this patch.
  4. Added error message, if it failed to turn on regulator's.
 
   drivers/mmc/host/dw_mmc.c  |   72 
  +---
   include/linux/mmc/dw_mmc.h |2 +-
   2 files changed, 36 insertions(+), 38 deletions(-)
 
  diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
  index 7f227e9..aadb0d6 100644
  --- a/drivers/mmc/host/dw_mmc.c
  +++ b/drivers/mmc/host/dw_mmc.c
  @@ -936,6 +936,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct 
  mmc_ios *ios)
  struct dw_mci_slot *slot = mmc_priv(mmc);
  const struct dw_mci_drv_data *drv_data = slot-host-drv_data;
  u32 regs;
  +   int ret;
 
  switch (ios-bus_width) {
  case MMC_BUS_WIDTH_4:
  @@ -974,12 +975,38 @@ static void dw_mci_set_ios(struct mmc_host *mmc, 
  struct mmc_ios *ios)
 
  switch (ios-power_mode) {
  case MMC_POWER_UP:
  +   if (!IS_ERR(mmc-supply.vmmc)) {
  +   ret = mmc_regulator_set_ocr(mmc, mmc-supply.vmmc,
  +   ios-vdd);
  +   if (ret) {
  +   dev_err(slot-host-dev,
  +   failed to enable vmmc 
  regulator\n);
  +   /*return, if failed turn on vmmc*/
  +   return;
  +   }
  +   }
  +   if (!IS_ERR(mmc-supply.vqmmc)  
  !slot-host-vqmmc_enabled) {
  +   ret = regulator_enable(mmc-supply.vqmmc);
  +   if (ret  0)
  +   dev_err(slot-host-dev,
  +   failed to enable vqmmc 
  regulator\n);
  +   else
  +   slot-host-vqmmc_enabled = true;
  +   }
  set_bit(DW_MMC_CARD_NEED_INIT, slot-flags);
  regs = mci_readl(slot-host, PWREN);
  regs |= (1  slot-id);
  mci_writel(slot-host, PWREN, regs);
  break;
  case MMC_POWER_OFF:
  +   if (!IS_ERR(mmc-supply.vmmc))
  +   mmc_regulator_set_ocr(mmc, mmc-supply.vmmc, 0);
  +
  +   if (!IS_ERR(mmc-supply.vqmmc)  
  slot-host-vqmmc_enabled) {
  +   regulator_disable(mmc-supply.vqmmc);
  +   slot-host-vqmmc_enabled = false;
  +   }
  +
  regs = mci_readl(slot-host, PWREN);
  regs = ~(1  slot-id);
  mci_writel(slot-host, PWREN, regs);
  @@ -2110,7 +2137,13 @@ static int dw_mci_init_slot(struct dw_mci *host, 
  unsigned int id)
  mmc-f_max = freq[1];
  }
 
  -   mmc-ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
  +   /*if there are external regulators, get them*/
  +   ret = mmc_regulator_get_supply(mmc);
  +   if (ret == -EPROBE_DEFER)
  +   goto err_setup_bus;
  +
  +   if (!mmc-ocr_avail)
  +   mmc-ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
 
  if (host-pdata-caps)
  mmc-caps = host-pdata-caps;
  @@ -2176,7 +2209,7 @@ static int dw_mci_init_slot(struct dw_mci *host, 
  unsigned int id)
 
   err_setup_bus:
  mmc_free_host(mmc);
  -   return -EINVAL;
  +   return ret;
   }
 
   static void dw_mci_cleanup_slot(struct dw_mci_slot *slot, unsigned

Re: [PATCH V2 1/3] mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators

2014-09-29 Thread Jaehoon Chung 
Hi

On 09/29/2014 09:31 PM, Bartlomiej Zolnierkiewicz wrote:
 
 Hi,
 
 On Friday, August 29, 2014 01:34:44 PM Ulf Hansson wrote:
 On 22 August 2014 15:47, Yuvaraj Kumar C D yuvaraj...@gmail.com wrote:
 This patch makes use of mmc_regulator_get_supply() to handle
 the vmmc and vqmmc regulators.Also it moves the code handling
 the these regulators to dw_mci_set_ios().It turned on the vmmc
 and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
 during MMC_POWER_OFF.

 Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com

 Thanks! Applied for next.
 
 Unfortunately this patch breaks mmc1 card (Kingston 32GB micro SDHC)
 detection on Exynos5420 Arndale Octa for me:
 
 [   10.797979] dwmmc_exynos 1222.mmc: no support for card's volts
 [   10.797998] mmc1: error -22 whilst initialising SD card

OCR value is not matched. Which values are supported about the mmc_host's value 
and card's value?
Could you share the value?

Best Regards,
Jaehoon Chung

 
 Without the patch:
 
 [   10.866926] mmc_host mmc1: Bus speed (slot 0) = 5000Hz (slot req 
 5000Hz, actual 5000HZ div = 0)
 [   10.866977] mmc1: new high speed SDHC card at address 1234
 [   10.868730] mmcblk1: mmc1:1234 SA32G 29.3 GiB 
 [   10.915054]  mmcblk1: p1 p2 p3
 
 The config is attached (exynos_defconfig doesn't work correctly for
 this board yet).
 
 Best regards,
 --
 Bartlomiej Zolnierkiewicz
 Samsung RD Institute Poland
 Samsung Electronics
 
 Kind regards
 Uffe

 ---
 changes from v1:
 1.Used mmc_regulator_set_ocr() instead of regulator_enable() for 
 vmmc.
 2.Turned on vmmc and vqmmc during MMC_POWER_UP.
 3. Removed the flags DW_MMC_CARD_POWERED and DW_MMC_IO_POWERED which
added during the initial version of this patch.
 4. Added error message, if it failed to turn on regulator's.

  drivers/mmc/host/dw_mmc.c  |   72 
 +---
  include/linux/mmc/dw_mmc.h |2 +-
  2 files changed, 36 insertions(+), 38 deletions(-)

 diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
 index 7f227e9..aadb0d6 100644
 --- a/drivers/mmc/host/dw_mmc.c
 +++ b/drivers/mmc/host/dw_mmc.c
 @@ -936,6 +936,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct 
 mmc_ios *ios)
 struct dw_mci_slot *slot = mmc_priv(mmc);
 const struct dw_mci_drv_data *drv_data = slot-host-drv_data;
 u32 regs;
 +   int ret;

 switch (ios-bus_width) {
 case MMC_BUS_WIDTH_4:
 @@ -974,12 +975,38 @@ static void dw_mci_set_ios(struct mmc_host *mmc, 
 struct mmc_ios *ios)

 switch (ios-power_mode) {
 case MMC_POWER_UP:
 +   if (!IS_ERR(mmc-supply.vmmc)) {
 +   ret = mmc_regulator_set_ocr(mmc, mmc-supply.vmmc,
 +   ios-vdd);
 +   if (ret) {
 +   dev_err(slot-host-dev,
 +   failed to enable vmmc 
 regulator\n);
 +   /*return, if failed turn on vmmc*/
 +   return;
 +   }
 +   }
 +   if (!IS_ERR(mmc-supply.vqmmc)  
 !slot-host-vqmmc_enabled) {
 +   ret = regulator_enable(mmc-supply.vqmmc);
 +   if (ret  0)
 +   dev_err(slot-host-dev,
 +   failed to enable vqmmc 
 regulator\n);
 +   else
 +   slot-host-vqmmc_enabled = true;
 +   }
 set_bit(DW_MMC_CARD_NEED_INIT, slot-flags);
 regs = mci_readl(slot-host, PWREN);
 regs |= (1  slot-id);
 mci_writel(slot-host, PWREN, regs);
 break;
 case MMC_POWER_OFF:
 +   if (!IS_ERR(mmc-supply.vmmc))
 +   mmc_regulator_set_ocr(mmc, mmc-supply.vmmc, 0);
 +
 +   if (!IS_ERR(mmc-supply.vqmmc)  
 slot-host-vqmmc_enabled) {
 +   regulator_disable(mmc-supply.vqmmc);
 +   slot-host-vqmmc_enabled = false;
 +   }
 +
 regs = mci_readl(slot-host, PWREN);
 regs = ~(1  slot-id);
 mci_writel(slot-host, PWREN, regs);
 @@ -2110,7 +2137,13 @@ static int dw_mci_init_slot(struct dw_mci *host, 
 unsigned int id)
 mmc-f_max = freq[1];
 }

 -   mmc-ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
 +   /*if there are external regulators, get them*/
 +   ret = mmc_regulator_get_supply(mmc);
 +   if (ret == -EPROBE_DEFER)
 +   goto err_setup_bus;
 +
 +   if (!mmc-ocr_avail)
 +   mmc-ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;

 if (host-pdata-caps)
 mmc-caps = host-pdata-caps;
 @@ -2176,7 +2209,7 @@ static int dw_mci_init_slot(struct dw_mci *host, 
 unsigned int id)

  err_setup_bus:

Re: [PATCH V2 1/3] mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators

2014-08-29 Thread Ulf Hansson 
On 22 August 2014 15:47, Yuvaraj Kumar C D yuvaraj...@gmail.com wrote:
 This patch makes use of mmc_regulator_get_supply() to handle
 the vmmc and vqmmc regulators.Also it moves the code handling
 the these regulators to dw_mci_set_ios().It turned on the vmmc
 and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
 during MMC_POWER_OFF.

 Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com

Thanks! Applied for next.

Kind regards
Uffe

 ---
 changes from v1:
 1.Used mmc_regulator_set_ocr() instead of regulator_enable() for vmmc.
 2.Turned on vmmc and vqmmc during MMC_POWER_UP.
 3. Removed the flags DW_MMC_CARD_POWERED and DW_MMC_IO_POWERED which
added during the initial version of this patch.
 4. Added error message, if it failed to turn on regulator's.

  drivers/mmc/host/dw_mmc.c  |   72 
 +---
  include/linux/mmc/dw_mmc.h |2 +-
  2 files changed, 36 insertions(+), 38 deletions(-)

 diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
 index 7f227e9..aadb0d6 100644
 --- a/drivers/mmc/host/dw_mmc.c
 +++ b/drivers/mmc/host/dw_mmc.c
 @@ -936,6 +936,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct 
 mmc_ios *ios)
 struct dw_mci_slot *slot = mmc_priv(mmc);
 const struct dw_mci_drv_data *drv_data = slot-host-drv_data;
 u32 regs;
 +   int ret;

 switch (ios-bus_width) {
 case MMC_BUS_WIDTH_4:
 @@ -974,12 +975,38 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct 
 mmc_ios *ios)

 switch (ios-power_mode) {
 case MMC_POWER_UP:
 +   if (!IS_ERR(mmc-supply.vmmc)) {
 +   ret = mmc_regulator_set_ocr(mmc, mmc-supply.vmmc,
 +   ios-vdd);
 +   if (ret) {
 +   dev_err(slot-host-dev,
 +   failed to enable vmmc regulator\n);
 +   /*return, if failed turn on vmmc*/
 +   return;
 +   }
 +   }
 +   if (!IS_ERR(mmc-supply.vqmmc)  !slot-host-vqmmc_enabled) 
 {
 +   ret = regulator_enable(mmc-supply.vqmmc);
 +   if (ret  0)
 +   dev_err(slot-host-dev,
 +   failed to enable vqmmc regulator\n);
 +   else
 +   slot-host-vqmmc_enabled = true;
 +   }
 set_bit(DW_MMC_CARD_NEED_INIT, slot-flags);
 regs = mci_readl(slot-host, PWREN);
 regs |= (1  slot-id);
 mci_writel(slot-host, PWREN, regs);
 break;
 case MMC_POWER_OFF:
 +   if (!IS_ERR(mmc-supply.vmmc))
 +   mmc_regulator_set_ocr(mmc, mmc-supply.vmmc, 0);
 +
 +   if (!IS_ERR(mmc-supply.vqmmc)  slot-host-vqmmc_enabled) {
 +   regulator_disable(mmc-supply.vqmmc);
 +   slot-host-vqmmc_enabled = false;
 +   }
 +
 regs = mci_readl(slot-host, PWREN);
 regs = ~(1  slot-id);
 mci_writel(slot-host, PWREN, regs);
 @@ -2110,7 +2137,13 @@ static int dw_mci_init_slot(struct dw_mci *host, 
 unsigned int id)
 mmc-f_max = freq[1];
 }

 -   mmc-ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
 +   /*if there are external regulators, get them*/
 +   ret = mmc_regulator_get_supply(mmc);
 +   if (ret == -EPROBE_DEFER)
 +   goto err_setup_bus;
 +
 +   if (!mmc-ocr_avail)
 +   mmc-ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;

 if (host-pdata-caps)
 mmc-caps = host-pdata-caps;
 @@ -2176,7 +2209,7 @@ static int dw_mci_init_slot(struct dw_mci *host, 
 unsigned int id)

  err_setup_bus:
 mmc_free_host(mmc);
 -   return -EINVAL;
 +   return ret;
  }

  static void dw_mci_cleanup_slot(struct dw_mci_slot *slot, unsigned int id)
 @@ -2469,24 +2502,6 @@ int dw_mci_probe(struct dw_mci *host)
 }
 }

 -   host-vmmc = devm_regulator_get_optional(host-dev, vmmc);
 -   if (IS_ERR(host-vmmc)) {
 -   ret = PTR_ERR(host-vmmc);
 -   if (ret == -EPROBE_DEFER)
 -   goto err_clk_ciu;
 -
 -   dev_info(host-dev, no vmmc regulator found: %d\n, ret);
 -   host-vmmc = NULL;
 -   } else {
 -   ret = regulator_enable(host-vmmc);
 -   if (ret) {
 -   if (ret != -EPROBE_DEFER)
 -   dev_err(host-dev,
 -   regulator_enable fail: %d\n, ret);
 -   goto err_clk_ciu;
 -   }
 -   }
 -
 host-quirks = host-pdata-quirks;

 spin_lock_init(host-lock);
 @@ -2630,8

Re: [PATCH V2 1/3] mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators

2014-08-25 Thread Jaehoon Chung 
On 08/22/2014 10:47 PM, Yuvaraj Kumar C D wrote:
 This patch makes use of mmc_regulator_get_supply() to handle
 the vmmc and vqmmc regulators.Also it moves the code handling
 the these regulators to dw_mci_set_ios().It turned on the vmmc
 and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
 during MMC_POWER_OFF.
 
 Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
 ---
 changes from v1:
   1.Used mmc_regulator_set_ocr() instead of regulator_enable() for vmmc.
   2.Turned on vmmc and vqmmc during MMC_POWER_UP.
   3. Removed the flags DW_MMC_CARD_POWERED and DW_MMC_IO_POWERED which
  added during the initial version of this patch.
   4. Added error message, if it failed to turn on regulator's.
 
  drivers/mmc/host/dw_mmc.c  |   72 
 +---
  include/linux/mmc/dw_mmc.h |2 +-
  2 files changed, 36 insertions(+), 38 deletions(-)
 
 diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
 index 7f227e9..aadb0d6 100644
 --- a/drivers/mmc/host/dw_mmc.c
 +++ b/drivers/mmc/host/dw_mmc.c
 @@ -936,6 +936,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct 
 mmc_ios *ios)
   struct dw_mci_slot *slot = mmc_priv(mmc);
   const struct dw_mci_drv_data *drv_data = slot-host-drv_data;
   u32 regs;
 + int ret;
  
   switch (ios-bus_width) {
   case MMC_BUS_WIDTH_4:
 @@ -974,12 +975,38 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct 
 mmc_ios *ios)
  
   switch (ios-power_mode) {
   case MMC_POWER_UP:
 + if (!IS_ERR(mmc-supply.vmmc)) {
 + ret = mmc_regulator_set_ocr(mmc, mmc-supply.vmmc,
 + ios-vdd);
 + if (ret) {
 + dev_err(slot-host-dev,
 + failed to enable vmmc regulator\n);
 + /*return, if failed turn on vmmc*/
 + return;
 + }
 + }
 + if (!IS_ERR(mmc-supply.vqmmc)  !slot-host-vqmmc_enabled) {

Can't use the regulator_is_enabled() instead of slot-host-vqmmc_enabled?

Best Regards,
Jaehoon Chung
 + ret = regulator_enable(mmc-supply.vqmmc);
 + if (ret  0)
 + dev_err(slot-host-dev,
 + failed to enable vqmmc regulator\n);
 + else
 + slot-host-vqmmc_enabled = true;
 + }
   set_bit(DW_MMC_CARD_NEED_INIT, slot-flags);
   regs = mci_readl(slot-host, PWREN);
   regs |= (1  slot-id);
   mci_writel(slot-host, PWREN, regs);
   break;
   case MMC_POWER_OFF:
 + if (!IS_ERR(mmc-supply.vmmc))
 + mmc_regulator_set_ocr(mmc, mmc-supply.vmmc, 0);
 +
 + if (!IS_ERR(mmc-supply.vqmmc)  slot-host-vqmmc_enabled) {
 + regulator_disable(mmc-supply.vqmmc);
 + slot-host-vqmmc_enabled = false;
 + }
 +
   regs = mci_readl(slot-host, PWREN);
   regs = ~(1  slot-id);
   mci_writel(slot-host, PWREN, regs);
 @@ -2110,7 +2137,13 @@ static int dw_mci_init_slot(struct dw_mci *host, 
 unsigned int id)
   mmc-f_max = freq[1];
   }
  
 - mmc-ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
 + /*if there are external regulators, get them*/
 + ret = mmc_regulator_get_supply(mmc);
 + if (ret == -EPROBE_DEFER)
 + goto err_setup_bus;
 +
 + if (!mmc-ocr_avail)
 + mmc-ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
  
   if (host-pdata-caps)
   mmc-caps = host-pdata-caps;
 @@ -2176,7 +2209,7 @@ static int dw_mci_init_slot(struct dw_mci *host, 
 unsigned int id)
  
  err_setup_bus:
   mmc_free_host(mmc);
 - return -EINVAL;
 + return ret;
  }
  
  static void dw_mci_cleanup_slot(struct dw_mci_slot *slot, unsigned int id)
 @@ -2469,24 +2502,6 @@ int dw_mci_probe(struct dw_mci *host)
   }
   }
  
 - host-vmmc = devm_regulator_get_optional(host-dev, vmmc);
 - if (IS_ERR(host-vmmc)) {
 - ret = PTR_ERR(host-vmmc);
 - if (ret == -EPROBE_DEFER)
 - goto err_clk_ciu;
 -
 - dev_info(host-dev, no vmmc regulator found: %d\n, ret);
 - host-vmmc = NULL;
 - } else {
 - ret = regulator_enable(host-vmmc);
 - if (ret) {
 - if (ret != -EPROBE_DEFER)
 - dev_err(host-dev,
 - regulator_enable fail: %d\n, ret);
 - goto err_clk_ciu;
 - }
 - }
 -
   host-quirks = host-pdata-quirks;
  
   spin_lock_init(host-lock);
 @@ -2630,8 +2645,6 @@ err_workqueue:
  err_dmaunmap:
   if (host-use_dma  host-dma_ops-exit)

Re: [PATCH V2 1/3] mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators

2014-08-25 Thread Doug Anderson 
Jaehoon,

On Mon, Aug 25, 2014 at 5:32 AM, Jaehoon Chung jh80.ch...@samsung.com wrote:
 On 08/22/2014 10:47 PM, Yuvaraj Kumar C D wrote:
 This patch makes use of mmc_regulator_get_supply() to handle
 the vmmc and vqmmc regulators.Also it moves the code handling
 the these regulators to dw_mci_set_ios().It turned on the vmmc
 and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
 during MMC_POWER_OFF.

 Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
 ---
 changes from v1:
   1.Used mmc_regulator_set_ocr() instead of regulator_enable() for vmmc.
   2.Turned on vmmc and vqmmc during MMC_POWER_UP.
   3. Removed the flags DW_MMC_CARD_POWERED and DW_MMC_IO_POWERED which
  added during the initial version of this patch.
   4. Added error message, if it failed to turn on regulator's.

  drivers/mmc/host/dw_mmc.c  |   72 
 +---
  include/linux/mmc/dw_mmc.h |2 +-
  2 files changed, 36 insertions(+), 38 deletions(-)

 diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
 index 7f227e9..aadb0d6 100644
 --- a/drivers/mmc/host/dw_mmc.c
 +++ b/drivers/mmc/host/dw_mmc.c
 @@ -936,6 +936,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct 
 mmc_ios *ios)
   struct dw_mci_slot *slot = mmc_priv(mmc);
   const struct dw_mci_drv_data *drv_data = slot-host-drv_data;
   u32 regs;
 + int ret;

   switch (ios-bus_width) {
   case MMC_BUS_WIDTH_4:
 @@ -974,12 +975,38 @@ static void dw_mci_set_ios(struct mmc_host *mmc, 
 struct mmc_ios *ios)

   switch (ios-power_mode) {
   case MMC_POWER_UP:
 + if (!IS_ERR(mmc-supply.vmmc)) {
 + ret = mmc_regulator_set_ocr(mmc, mmc-supply.vmmc,
 + ios-vdd);
 + if (ret) {
 + dev_err(slot-host-dev,
 + failed to enable vmmc regulator\n);
 + /*return, if failed turn on vmmc*/
 + return;
 + }
 + }
 + if (!IS_ERR(mmc-supply.vqmmc)  !slot-host-vqmmc_enabled) {

 Can't use the regulator_is_enabled() instead of slot-host-vqmmc_enabled?

I think we mentioned this before, but regulator_is_enabled() won't do
what you want with shared regulators since they are refcounted.  You
need to keep track of whether you've enabled the regulator yourself.

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

[PATCH V2 1/3] mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators

2014-08-22 Thread Yuvaraj Kumar C D 
This patch makes use of mmc_regulator_get_supply() to handle
the vmmc and vqmmc regulators.Also it moves the code handling
the these regulators to dw_mci_set_ios().It turned on the vmmc
and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
during MMC_POWER_OFF.

Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
---
changes from v1:
1.Used mmc_regulator_set_ocr() instead of regulator_enable() for vmmc.
2.Turned on vmmc and vqmmc during MMC_POWER_UP.
3. Removed the flags DW_MMC_CARD_POWERED and DW_MMC_IO_POWERED which
   added during the initial version of this patch.
4. Added error message, if it failed to turn on regulator's.

 drivers/mmc/host/dw_mmc.c  |   72 +---
 include/linux/mmc/dw_mmc.h |2 +-
 2 files changed, 36 insertions(+), 38 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 7f227e9..aadb0d6 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -936,6 +936,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct 
mmc_ios *ios)
struct dw_mci_slot *slot = mmc_priv(mmc);
const struct dw_mci_drv_data *drv_data = slot-host-drv_data;
u32 regs;
+   int ret;
 
switch (ios-bus_width) {
case MMC_BUS_WIDTH_4:
@@ -974,12 +975,38 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct 
mmc_ios *ios)
 
switch (ios-power_mode) {
case MMC_POWER_UP:
+   if (!IS_ERR(mmc-supply.vmmc)) {
+   ret = mmc_regulator_set_ocr(mmc, mmc-supply.vmmc,
+   ios-vdd);
+   if (ret) {
+   dev_err(slot-host-dev,
+   failed to enable vmmc regulator\n);
+   /*return, if failed turn on vmmc*/
+   return;
+   }
+   }
+   if (!IS_ERR(mmc-supply.vqmmc)  !slot-host-vqmmc_enabled) {
+   ret = regulator_enable(mmc-supply.vqmmc);
+   if (ret  0)
+   dev_err(slot-host-dev,
+   failed to enable vqmmc regulator\n);
+   else
+   slot-host-vqmmc_enabled = true;
+   }
set_bit(DW_MMC_CARD_NEED_INIT, slot-flags);
regs = mci_readl(slot-host, PWREN);
regs |= (1  slot-id);
mci_writel(slot-host, PWREN, regs);
break;
case MMC_POWER_OFF:
+   if (!IS_ERR(mmc-supply.vmmc))
+   mmc_regulator_set_ocr(mmc, mmc-supply.vmmc, 0);
+
+   if (!IS_ERR(mmc-supply.vqmmc)  slot-host-vqmmc_enabled) {
+   regulator_disable(mmc-supply.vqmmc);
+   slot-host-vqmmc_enabled = false;
+   }
+
regs = mci_readl(slot-host, PWREN);
regs = ~(1  slot-id);
mci_writel(slot-host, PWREN, regs);
@@ -2110,7 +2137,13 @@ static int dw_mci_init_slot(struct dw_mci *host, 
unsigned int id)
mmc-f_max = freq[1];
}
 
-   mmc-ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
+   /*if there are external regulators, get them*/
+   ret = mmc_regulator_get_supply(mmc);
+   if (ret == -EPROBE_DEFER)
+   goto err_setup_bus;
+
+   if (!mmc-ocr_avail)
+   mmc-ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
 
if (host-pdata-caps)
mmc-caps = host-pdata-caps;
@@ -2176,7 +2209,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned 
int id)
 
 err_setup_bus:
mmc_free_host(mmc);
-   return -EINVAL;
+   return ret;
 }
 
 static void dw_mci_cleanup_slot(struct dw_mci_slot *slot, unsigned int id)
@@ -2469,24 +2502,6 @@ int dw_mci_probe(struct dw_mci *host)
}
}
 
-   host-vmmc = devm_regulator_get_optional(host-dev, vmmc);
-   if (IS_ERR(host-vmmc)) {
-   ret = PTR_ERR(host-vmmc);
-   if (ret == -EPROBE_DEFER)
-   goto err_clk_ciu;
-
-   dev_info(host-dev, no vmmc regulator found: %d\n, ret);
-   host-vmmc = NULL;
-   } else {
-   ret = regulator_enable(host-vmmc);
-   if (ret) {
-   if (ret != -EPROBE_DEFER)
-   dev_err(host-dev,
-   regulator_enable fail: %d\n, ret);
-   goto err_clk_ciu;
-   }
-   }
-
host-quirks = host-pdata-quirks;
 
spin_lock_init(host-lock);
@@ -2630,8 +2645,6 @@ err_workqueue:
 err_dmaunmap:
if (host-use_dma  host-dma_ops-exit)
host-dma_ops-exit(host);
-   if (host-vmmc)
-   regulator_disable(host-vmmc);
 
 err_clk_ciu:
if