[PATCH] mmc-utils: change .gitignore file to hide temporal files in sub folders

2015-04-22 Thread Leon Romanovsky
Signed-off-by: Leon Romanovsky 
---
 .gitignore |6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/.gitignore b/.gitignore
index 5a94d47..f95fc99 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,5 +1,3 @@
-.mmc.o.d
-.mmc_cmds.o.d
+*.o
+.*.o.*
 mmc
-mmc.o
-mmc_cmds.o
-- 
1.7.9.5

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


[PATCH] mmc-utils: delete dependency files after make clean invocation

2015-04-22 Thread Leon Romanovsky
Make clean didn't remove dependency files (.*.o.d) which were generated
by GCC. This change deletes these files in all sub folders.

Signed-off-by: Leon Romanovsky 
---
 Makefile |1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile b/Makefile
index 0533be3..bef6c82 100644
--- a/Makefile
+++ b/Makefile
@@ -45,6 +45,7 @@ install-man:
 
 clean:
rm -f $(progs) $(objects)
+   find . -name .\*.o.d -type f -delete
$(MAKE) -C man clean
 
 install: $(progs) install-man
-- 
1.7.9.5

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


[PATCH] mmc-utils: remove obsolete do_write_extcsd function declaration

2015-04-22 Thread Leon Romanovsky
The do_write_extcsd() function was removed from the C-file in
commit"b9c7a17 Rename extcsd read/write to writeprotect get/set".

This patch removes this function definition from H-file too.

Signed-off-by: Leon Romanovsky 
---
 mmc_cmds.h |1 -
 1 file changed, 1 deletion(-)

diff --git a/mmc_cmds.h b/mmc_cmds.h
index 9e625c9..7a79e87 100644
--- a/mmc_cmds.h
+++ b/mmc_cmds.h
@@ -16,7 +16,6 @@
 
 /* mmc_cmds.c */
 int do_read_extcsd(int nargs, char **argv);
-int do_write_extcsd(int nargs, char **argv);
 int do_writeprotect_get(int nargs, char **argv);
 int do_writeprotect_set(int nargs, char **argv);
 int do_disable_512B_emulation(int nargs, char **argv);
-- 
1.7.9.5

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


emmc 4.4x fast boot question

2015-04-22 Thread Angelo Dureghello

hi experts,

trying to use imx6q "eMMC Fast Boot" mode without success. Can't
get any clarification from freescale still, so i try here.

From the imx6q boot flow diagram seems there can be some gain using it.

Once studying on the Micron eMMC i have connected to imx6q, (as per
TN-52-06: Booting from Embedded MMC - JEDEC 4.41 online pdf) i see
this mode is just called "boot mode" or "alternate boot mode", and seems 
to allow to boot from SLC mmcblk0boot0 or mmcblk0boot1

partitions setting EXT_CSD[179] partition to 1 or 2 in bits 3to5.

In this way i can boot from mmcblk0boot0. but if i enable the hw
imx6q GPIO BOOT_CFG1[4] "Fast Boot" mode, i cannot boot anymore.

If i look signal with scope, booting i don't see the initial
CMD pulled low as should be for the Boot mode.
If i enable imx6q "fast boot" (BOOT_CFG1[4]) i just see a
continuos and long train of bits on both CMD and CLK from the reset.

So i can boot from special SLC mmcblk0boot0 but without imx6
"Fast Boot" enabled. So i am trying to understand why this Fast Boot 
feature (BOOT_CFG1[4] set) can't be used.


In case you can give any help on this it will be really appreciated.

Best regards,
Angelo Dureghello



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


Re: [PATCH 07/10] brcmfmac: fix sdio suspend and resume

2015-04-22 Thread Ulf Hansson
On 22 April 2015 at 11:38, Arend van Spriel  wrote:
> On 04/22/15 11:18, Ulf Hansson wrote:
>>
>> On 22 April 2015 at 10:52, Arend van Spriel  wrote:
>>>
>>> - wireless list/maintainer
>>>
>>>
>>> On 04/22/15 09:32, Ulf Hansson wrote:


 On 14 April 2015 at 20:10, Arend van Spriel   wrote:
>
>
> commit 330b4e4be937 ("brcmfmac: Add wowl support for SDIO devices.")
> changed the behaviour by removing the MMC_PM_KEEP_POWER flag for
> non-wowl scenario, which needs to be restored. Another necessary
> change is to mark the card as being non-removable. With this in place
> the suspend resume test passes successfully doing:
>
># echo devices>   /sys/power/pm_test
># echo mem>   /sys/power/state
>
> Note that power may still be switched off when system is going
> in S3 state.
>
> Reported-by: Fu, Zhonghui<
> Reviewed-by: Pieter-Paul Giesberts
> Reviewed-by: Franky (Zhenhui) Lin
> Signed-off-by: Arend van Spriel
> ---
>drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 18
> +-
>1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> index 9b508bd..8a69544 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> @@ -1011,6 +1011,14 @@ static int brcmf_sdiod_remove(struct
> brcmf_sdio_dev *sdiodev)
>   return 0;
>}
>
> +static void brcmf_sdiod_host_fixup(struct mmc_host *host)
> +{
> +   /* runtime-pm powers off the device */
> +   pm_runtime_forbid(host->parent);



 That you need this, clearly shows that something is broken in the mmc
 core/host layer.
>>>
>>>
>>>
>>> This patch only moved this. The patch introducing this is here [1].
>>
>>
>> OK, thanks for sharing the information!
>>
>>>
 Could you elaborate a bit on what configuration you are using. Like
 what mmc host, which SDIO bus speed mode.
>>>
>>>
>>>
>>> Not just one. My test setup is a dev board hooked up to a card reader
>>> slot
>>> using sdhci-pci driver. Another setup I have is a chromebook with our
>>> device
>>> integrated with dw_mmc-rockchip driver. It is an arm platform with dt
>>> entry:
>>>
>>> &sdio0 {
>>>  broken-cd;
>>>  bus-width =<4>;
>>>  cap-sd-highspeed;
>>>  sd-uhs-sdr12;
>>>  sd-uhs-sdr25;
>>>  sd-uhs-sdr50;
>>>  sd-uhs-sdr104;
>>>  cap-sdio-irq;
>>>  card-external-vcc-supply =<&wifi_regulator>;
>>>  clocks =<&cru HCLK_SDIO0>,<&cru SCLK_SDIO0>,<&cru
>>> SCLK_SDIO0_DRV>,
>>>   <&cru SCLK_SDIO0_SAMPLE>,<&rk808 RK808_CLKOUT1>;
>>>  clock-names = "biu", "ciu", "ciu_drv", "ciu_sample",
>>> "card_ext_clock";
>>>  keep-power-in-suspend;
>>>  non-removable;
>>>  num-slots =<1>;
>>>  default-sample-phase =<90>;
>>>  pinctrl-names = "default";
>>>  pinctrl-0 =<&sdio0_clk&sdio0_cmd&sdio0_bus4>;
>>>  status = "okay";
>>>  vmmc-supply =<&vcc33_sys>;
>>>  vqmmc-supply =<&vcc18_wl>;
>>> };
>>>
>>> I think card-external-vcc-supply is property that chromeos kernel handles
>>> to
>>> power the device.
>>
>>
>> Should then likely be moved in a mmc pwrseq node and handled by the
>> mmc core, right?
>>
>> The same might apply to "card_ext_clock"!?
>
>
> Guess so. I was not involved in these DT changes and my focus is more on
> upstream.
>
>>>
 And have you tested different configurations? Like what happens if you
 use a different SDIO bus speed mode?
>>
>>
>> In the dw_mmc case, the host controller driver doesn't implement
>> runtime PM - only system PM (unless you are carrying some additional
>> out of tree patch :-) ).
>>
>> So, are you sure the bug exists in this configuration at all?
>
>
> Forgot to mention the most applicable setup. I also have a Sony Vaio Duo 13
> with wifi sdio chip integrated through sdhci-acpi driver, which is doing
> runtime-pm. I had a number of people contacting me and I had them disable
> runtime-pm through sysfs to get working wifi. That was the reason for adding
> pm_runtime_forbid() to brcmfmac driver.

Okay.

Anyway, if you find time to try the other configurations and then
change to "allow" runtime PM, it would be nice to know the results.

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


Re: [PATCH 07/10] brcmfmac: fix sdio suspend and resume

2015-04-22 Thread Arend van Spriel

On 04/22/15 11:18, Ulf Hansson wrote:

On 22 April 2015 at 10:52, Arend van Spriel  wrote:

- wireless list/maintainer


On 04/22/15 09:32, Ulf Hansson wrote:


On 14 April 2015 at 20:10, Arend van Spriel   wrote:


commit 330b4e4be937 ("brcmfmac: Add wowl support for SDIO devices.")
changed the behaviour by removing the MMC_PM_KEEP_POWER flag for
non-wowl scenario, which needs to be restored. Another necessary
change is to mark the card as being non-removable. With this in place
the suspend resume test passes successfully doing:

   # echo devices>   /sys/power/pm_test
   # echo mem>   /sys/power/state

Note that power may still be switched off when system is going
in S3 state.

Reported-by: Fu, Zhonghui<
Reviewed-by: Pieter-Paul Giesberts
Reviewed-by: Franky (Zhenhui) Lin
Signed-off-by: Arend van Spriel
---
   drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 18
+-
   1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
index 9b508bd..8a69544 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
@@ -1011,6 +1011,14 @@ static int brcmf_sdiod_remove(struct
brcmf_sdio_dev *sdiodev)
  return 0;
   }

+static void brcmf_sdiod_host_fixup(struct mmc_host *host)
+{
+   /* runtime-pm powers off the device */
+   pm_runtime_forbid(host->parent);



That you need this, clearly shows that something is broken in the mmc
core/host layer.



This patch only moved this. The patch introducing this is here [1].


OK, thanks for sharing the information!




Could you elaborate a bit on what configuration you are using. Like
what mmc host, which SDIO bus speed mode.



Not just one. My test setup is a dev board hooked up to a card reader slot
using sdhci-pci driver. Another setup I have is a chromebook with our device
integrated with dw_mmc-rockchip driver. It is an arm platform with dt entry:

&sdio0 {
 broken-cd;
 bus-width =<4>;
 cap-sd-highspeed;
 sd-uhs-sdr12;
 sd-uhs-sdr25;
 sd-uhs-sdr50;
 sd-uhs-sdr104;
 cap-sdio-irq;
 card-external-vcc-supply =<&wifi_regulator>;
 clocks =<&cru HCLK_SDIO0>,<&cru SCLK_SDIO0>,<&cru
SCLK_SDIO0_DRV>,
  <&cru SCLK_SDIO0_SAMPLE>,<&rk808 RK808_CLKOUT1>;
 clock-names = "biu", "ciu", "ciu_drv", "ciu_sample",
"card_ext_clock";
 keep-power-in-suspend;
 non-removable;
 num-slots =<1>;
 default-sample-phase =<90>;
 pinctrl-names = "default";
 pinctrl-0 =<&sdio0_clk&sdio0_cmd&sdio0_bus4>;
 status = "okay";
 vmmc-supply =<&vcc33_sys>;
 vqmmc-supply =<&vcc18_wl>;
};

I think card-external-vcc-supply is property that chromeos kernel handles to
power the device.


Should then likely be moved in a mmc pwrseq node and handled by the
mmc core, right?

The same might apply to "card_ext_clock"!?


Guess so. I was not involved in these DT changes and my focus is more on 
upstream.





And have you tested different configurations? Like what happens if you
use a different SDIO bus speed mode?


In the dw_mmc case, the host controller driver doesn't implement
runtime PM - only system PM (unless you are carrying some additional
out of tree patch :-) ).

So, are you sure the bug exists in this configuration at all?


Forgot to mention the most applicable setup. I also have a Sony Vaio Duo 
13 with wifi sdio chip integrated through sdhci-acpi driver, which is 
doing runtime-pm. I had a number of people contacting me and I had them 
disable runtime-pm through sysfs to get working wifi. That was the 
reason for adding pm_runtime_forbid() to brcmfmac driver.


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


Re: [PATCH 07/10] brcmfmac: fix sdio suspend and resume

2015-04-22 Thread Ulf Hansson
On 22 April 2015 at 10:52, Arend van Spriel  wrote:
> - wireless list/maintainer
>
>
> On 04/22/15 09:32, Ulf Hansson wrote:
>>
>> On 14 April 2015 at 20:10, Arend van Spriel  wrote:
>>>
>>> commit 330b4e4be937 ("brcmfmac: Add wowl support for SDIO devices.")
>>> changed the behaviour by removing the MMC_PM_KEEP_POWER flag for
>>> non-wowl scenario, which needs to be restored. Another necessary
>>> change is to mark the card as being non-removable. With this in place
>>> the suspend resume test passes successfully doing:
>>>
>>>   # echo devices>  /sys/power/pm_test
>>>   # echo mem>  /sys/power/state
>>>
>>> Note that power may still be switched off when system is going
>>> in S3 state.
>>>
>>> Reported-by: Fu, Zhonghui<
>>> Reviewed-by: Pieter-Paul Giesberts
>>> Reviewed-by: Franky (Zhenhui) Lin
>>> Signed-off-by: Arend van Spriel
>>> ---
>>>   drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 18
>>> +-
>>>   1 file changed, 13 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>>> b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>>> index 9b508bd..8a69544 100644
>>> --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>>> @@ -1011,6 +1011,14 @@ static int brcmf_sdiod_remove(struct
>>> brcmf_sdio_dev *sdiodev)
>>>  return 0;
>>>   }
>>>
>>> +static void brcmf_sdiod_host_fixup(struct mmc_host *host)
>>> +{
>>> +   /* runtime-pm powers off the device */
>>> +   pm_runtime_forbid(host->parent);
>>
>>
>> That you need this, clearly shows that something is broken in the mmc
>> core/host layer.
>
>
> This patch only moved this. The patch introducing this is here [1].

OK, thanks for sharing the information!

>
>> Could you elaborate a bit on what configuration you are using. Like
>> what mmc host, which SDIO bus speed mode.
>
>
> Not just one. My test setup is a dev board hooked up to a card reader slot
> using sdhci-pci driver. Another setup I have is a chromebook with our device
> integrated with dw_mmc-rockchip driver. It is an arm platform with dt entry:
>
> &sdio0 {
> broken-cd;
> bus-width = <4>;
> cap-sd-highspeed;
> sd-uhs-sdr12;
> sd-uhs-sdr25;
> sd-uhs-sdr50;
> sd-uhs-sdr104;
> cap-sdio-irq;
> card-external-vcc-supply = <&wifi_regulator>;
> clocks = <&cru HCLK_SDIO0>, <&cru SCLK_SDIO0>, <&cru
> SCLK_SDIO0_DRV>,
>  <&cru SCLK_SDIO0_SAMPLE>, <&rk808 RK808_CLKOUT1>;
> clock-names = "biu", "ciu", "ciu_drv", "ciu_sample",
> "card_ext_clock";
> keep-power-in-suspend;
> non-removable;
> num-slots = <1>;
> default-sample-phase = <90>;
> pinctrl-names = "default";
> pinctrl-0 = <&sdio0_clk &sdio0_cmd &sdio0_bus4>;
> status = "okay";
> vmmc-supply = <&vcc33_sys>;
> vqmmc-supply = <&vcc18_wl>;
> };
>
> I think card-external-vcc-supply is property that chromeos kernel handles to
> power the device.

Should then likely be moved in a mmc pwrseq node and handled by the
mmc core, right?

The same might apply to "card_ext_clock"!?

>
>> And have you tested different configurations? Like what happens if you
>> use a different SDIO bus speed mode?

In the dw_mmc case, the host controller driver doesn't implement
runtime PM - only system PM (unless you are carrying some additional
out of tree patch :-) ).

So, are you sure the bug exists in this configuration at all?

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


Re: [PATCH 3/3 v3] mmc: sh_mmcif: calculate best clock with parent clock

2015-04-22 Thread Kuninori Morimoto

Hi Geert

Thank you for your feedback

> >> Can't you loop over all MMC dividers, and
> >>   1. calculate the needed parent clock rate for that MMC divider,
> >>   2. use clk_set_rate_range()[*] to find a (close) parent clock rate,
> >>   3. calculate the effective clock rate based on MMC divider and parent
> >>  clock rate.
> >> and use the best effective clock rate found?
(snip)
> So the upper limit is a limitation of the MMCIF hardware, while the lower
> limit is a limitation of the CPG.
> 
> Then the upper limit should be either in the driver (as you did, using
> different compatible values), or it can be described in DT.
> (cfr. "git grep max.*freq -- arch/arm/boot/dts/").
> Personally, I'm leaning towards the latter.
> 
> IMHO the lower limit doesn't belong here.

I think I understood your opinion.
I will try this

Best regards
---
Kuninori Morimoto
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 07/10] brcmfmac: fix sdio suspend and resume

2015-04-22 Thread Arend van Spriel

- wireless list/maintainer

On 04/22/15 09:32, Ulf Hansson wrote:

On 14 April 2015 at 20:10, Arend van Spriel  wrote:

commit 330b4e4be937 ("brcmfmac: Add wowl support for SDIO devices.")
changed the behaviour by removing the MMC_PM_KEEP_POWER flag for
non-wowl scenario, which needs to be restored. Another necessary
change is to mark the card as being non-removable. With this in place
the suspend resume test passes successfully doing:

  # echo devices>  /sys/power/pm_test
  # echo mem>  /sys/power/state

Note that power may still be switched off when system is going
in S3 state.

Reported-by: Fu, Zhonghui<
Reviewed-by: Pieter-Paul Giesberts
Reviewed-by: Franky (Zhenhui) Lin
Signed-off-by: Arend van Spriel
---
  drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 18 +-
  1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c 
b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
index 9b508bd..8a69544 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
@@ -1011,6 +1011,14 @@ static int brcmf_sdiod_remove(struct brcmf_sdio_dev 
*sdiodev)
 return 0;
  }

+static void brcmf_sdiod_host_fixup(struct mmc_host *host)
+{
+   /* runtime-pm powers off the device */
+   pm_runtime_forbid(host->parent);


That you need this, clearly shows that something is broken in the mmc
core/host layer.


This patch only moved this. The patch introducing this is here [1].


Could you elaborate a bit on what configuration you are using. Like
what mmc host, which SDIO bus speed mode.


Not just one. My test setup is a dev board hooked up to a card reader 
slot using sdhci-pci driver. Another setup I have is a chromebook with 
our device integrated with dw_mmc-rockchip driver. It is an arm platform 
with dt entry:


&sdio0 {
broken-cd;
bus-width = <4>;
cap-sd-highspeed;
sd-uhs-sdr12;
sd-uhs-sdr25;
sd-uhs-sdr50;
sd-uhs-sdr104;
cap-sdio-irq;
card-external-vcc-supply = <&wifi_regulator>;
clocks = <&cru HCLK_SDIO0>, <&cru SCLK_SDIO0>, <&cru 
SCLK_SDIO0_DRV>,

 <&cru SCLK_SDIO0_SAMPLE>, <&rk808 RK808_CLKOUT1>;
clock-names = "biu", "ciu", "ciu_drv", "ciu_sample", 
"card_ext_clock";

keep-power-in-suspend;
non-removable;
num-slots = <1>;
default-sample-phase = <90>;
pinctrl-names = "default";
pinctrl-0 = <&sdio0_clk &sdio0_cmd &sdio0_bus4>;
status = "okay";
vmmc-supply = <&vcc33_sys>;
vqmmc-supply = <&vcc18_wl>;
};

I think card-external-vcc-supply is property that chromeos kernel 
handles to power the device.



And have you tested different configurations? Like what happens if you
use a different SDIO bus speed mode?


Regards,
Arend

[1] https://patchwork.kernel.org/patch/6038511/


+   /* avoid removal detection upon resume */
+   host->caps |= MMC_CAP_NONREMOVABLE;
+}
+
  static int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev)
  {
 struct sdio_func *func;
@@ -1076,7 +1084,7 @@ static int brcmf_sdiod_probe(struct brcmf_sdio_dev 
*sdiodev)
 ret = -ENODEV;
 goto out;
 }
-   pm_runtime_forbid(host->parent);
+   brcmf_sdiod_host_fixup(host);
  out:
 if (ret)
 brcmf_sdiod_remove(sdiodev);
@@ -1246,15 +1254,15 @@ static int brcmf_ops_sdio_suspend(struct device *dev)
 brcmf_sdiod_freezer_on(sdiodev);
 brcmf_sdio_wd_timer(sdiodev->bus, 0);

+   sdio_flags = MMC_PM_KEEP_POWER;
 if (sdiodev->wowl_enabled) {
-   sdio_flags = MMC_PM_KEEP_POWER;
 if (sdiodev->pdata->oob_irq_supported)
 enable_irq_wake(sdiodev->pdata->oob_irq_nr);
 else
-   sdio_flags = MMC_PM_WAKE_SDIO_IRQ;
-   if (sdio_set_host_pm_flags(sdiodev->func[1], sdio_flags))
-   brcmf_err("Failed to set pm_flags %x\n", sdio_flags);
+   sdio_flags |= MMC_PM_WAKE_SDIO_IRQ;
 }
+   if (sdio_set_host_pm_flags(sdiodev->func[1], sdio_flags))
+   brcmf_err("Failed to set pm_flags %x\n", sdio_flags);
 return 0;
  }

--
1.9.1

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


Kind regards
Uffe


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


Re: [PATCH V6 08/15] mmc: mmc: Hold re-tuning if the card is put to sleep

2015-04-22 Thread Ulf Hansson
On 22 April 2015 at 10:30, Arend van Spriel  wrote:
> On 04/22/15 09:24, Adrian Hunter wrote:
>>
>> On 21/04/15 21:25, Arend van Spriel wrote:
>>>
>>> On 04/21/15 14:26, Adrian Hunter wrote:

 On 21/04/15 14:53, Ulf Hansson wrote:
>
> On 21 April 2015 at 13:00, Adrian Hunter
> wrote:
>>
>> On 21/04/15 12:42, Ulf Hansson wrote:
>>>
>>> On 20 April 2015 at 14:09, Adrian Hunter
>>> wrote:

 Currently "mmc sleep" is used before power off and
 is not paired with waking up. Nevertheless hold
 re-tuning.

 Signed-off-by: Adrian Hunter
 ---
drivers/mmc/core/mmc.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)

 diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
 index f36c76f..daf9954 100644
 --- a/drivers/mmc/core/mmc.c
 +++ b/drivers/mmc/core/mmc.c
 @@ -21,6 +21,7 @@
#include

#include "core.h"
 +#include "host.h"
#include "bus.h"
#include "mmc_ops.h"
#include "sd_ops.h"
 @@ -1504,6 +1505,7 @@ static int mmc_can_sleep(struct mmc_card
 *card)
   return (card&&   card->ext_csd.rev>= 3);
}

 +/* If necessary, callers must hold re-tuning */
static int mmc_sleep(struct mmc_host *host)
{
   struct mmc_command cmd = {0};
 @@ -1631,6 +1633,7 @@ static int _mmc_suspend(struct mmc_host *host,
 bool is_suspend)
   int err = 0;
   unsigned int notify_type = is_suspend ?
 EXT_CSD_POWER_OFF_SHORT :
   EXT_CSD_POWER_OFF_LONG;
 +   bool retune_release = false;

   BUG_ON(!host);
   BUG_ON(!host->card);
 @@ -1651,17 +1654,22 @@ static int _mmc_suspend(struct mmc_host
 *host,
 bool is_suspend)
   goto out;

   if (mmc_can_poweroff_notify(host->card)&&
 -   ((host->caps2&   MMC_CAP2_FULL_PWR_CYCLE) ||
 !is_suspend))
 +   ((host->caps2&   MMC_CAP2_FULL_PWR_CYCLE) ||
 !is_suspend)) {
   err = mmc_poweroff_notify(host->card,
 notify_type);
 -   else if (mmc_can_sleep(host->card))
 +   } else if (mmc_can_sleep(host->card)) {
 +   mmc_retune_hold(host);
   err = mmc_sleep(host);
 -   else if (!mmc_host_is_spi(host))
 +   } else if (!mmc_host_is_spi(host)) {
   err = mmc_deselect_cards(host);
 +   }

   if (!err) {
   mmc_power_off(host);
   mmc_card_set_suspended(host->card);
   }
 +
 +   if (retune_release)
 +   mmc_retune_release(host);
out:
   mmc_release_host(host);
   return err;
 --
 1.9.1

>>>
>>> According to our previous discussions I have given this some more
>>> thinking.
>>>
>>> I don't think we can allow to hold/disable re-tune in this path at
>>> all. That's because we are claiming the host here and the sleep
>>> command might then be the first command we invoke during the system
>>> PM
>>> sequence.
>>>
>>> That means sdhci might have flagged need_retune, since it's been
>>> runtime PM suspended. And for those scenarios I guess we really need
>>> to do a re-tune prior sending the sleep command, right?
>>
>>
>> Yes, although that is how it works.
>
>
> Ohh, you are one step ahead of me. Good! :-)
>
>>
>> Previously I had two functions mmc_retune_hold() and
>> mmc_retune_and_hold()
>> but after one of the revisions I found that only one was needed. I
>> stuck
>> with the mmc_retune_hold() name because it doesn't necessarily cause a
>> re-tune, but only if the hold count was zero and a retune is needed.
>>
>>>
>>> Earlier I only had the re-tune timer in mind, which is why I was less
>>> restrictive and suggesting you to add hold/disable. Sorry about that.
>>>
>>> Now, with the above in mind I believe you have similar issues with
>>> patch5 (mmc: core: Hold re-tuning during switch commands) and patch6
>>> (mmc: core: Hold re-tuning during erase commands). And that's because
>>> there are cases when the switch/erase commands are the first commands
>>> sent, after the sdhci host has been runtime PM suspended. I guess we
>>> need a way to make sure we don't hold re-tune for these cases.
>>>
>>> An option to deal with that is to use a separate flag set by host
>>> drivers

Re: [PATCH V6 08/15] mmc: mmc: Hold re-tuning if the card is put to sleep

2015-04-22 Thread Arend van Spriel

On 04/22/15 09:24, Adrian Hunter wrote:

On 21/04/15 21:25, Arend van Spriel wrote:

On 04/21/15 14:26, Adrian Hunter wrote:

On 21/04/15 14:53, Ulf Hansson wrote:

On 21 April 2015 at 13:00, Adrian Hunter   wrote:

On 21/04/15 12:42, Ulf Hansson wrote:

On 20 April 2015 at 14:09, Adrian Hunter   wrote:

Currently "mmc sleep" is used before power off and
is not paired with waking up. Nevertheless hold
re-tuning.

Signed-off-by: Adrian Hunter
---
   drivers/mmc/core/mmc.c | 14 +++---
   1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index f36c76f..daf9954 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -21,6 +21,7 @@
   #include

   #include "core.h"
+#include "host.h"
   #include "bus.h"
   #include "mmc_ops.h"
   #include "sd_ops.h"
@@ -1504,6 +1505,7 @@ static int mmc_can_sleep(struct mmc_card *card)
  return (card&&   card->ext_csd.rev>= 3);
   }

+/* If necessary, callers must hold re-tuning */
   static int mmc_sleep(struct mmc_host *host)
   {
  struct mmc_command cmd = {0};
@@ -1631,6 +1633,7 @@ static int _mmc_suspend(struct mmc_host *host,
bool is_suspend)
  int err = 0;
  unsigned int notify_type = is_suspend ?
EXT_CSD_POWER_OFF_SHORT :
  EXT_CSD_POWER_OFF_LONG;
+   bool retune_release = false;

  BUG_ON(!host);
  BUG_ON(!host->card);
@@ -1651,17 +1654,22 @@ static int _mmc_suspend(struct mmc_host *host,
bool is_suspend)
  goto out;

  if (mmc_can_poweroff_notify(host->card)&&
-   ((host->caps2&   MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
+   ((host->caps2&   MMC_CAP2_FULL_PWR_CYCLE) ||
!is_suspend)) {
  err = mmc_poweroff_notify(host->card, notify_type);
-   else if (mmc_can_sleep(host->card))
+   } else if (mmc_can_sleep(host->card)) {
+   mmc_retune_hold(host);
  err = mmc_sleep(host);
-   else if (!mmc_host_is_spi(host))
+   } else if (!mmc_host_is_spi(host)) {
  err = mmc_deselect_cards(host);
+   }

  if (!err) {
  mmc_power_off(host);
  mmc_card_set_suspended(host->card);
  }
+
+   if (retune_release)
+   mmc_retune_release(host);
   out:
  mmc_release_host(host);
  return err;
--
1.9.1



According to our previous discussions I have given this some more
thinking.

I don't think we can allow to hold/disable re-tune in this path at
all. That's because we are claiming the host here and the sleep
command might then be the first command we invoke during the system PM
sequence.

That means sdhci might have flagged need_retune, since it's been
runtime PM suspended. And for those scenarios I guess we really need
to do a re-tune prior sending the sleep command, right?


Yes, although that is how it works.


Ohh, you are one step ahead of me. Good! :-)



Previously I had two functions mmc_retune_hold() and mmc_retune_and_hold()
but after one of the revisions I found that only one was needed. I stuck
with the mmc_retune_hold() name because it doesn't necessarily cause a
re-tune, but only if the hold count was zero and a retune is needed.



Earlier I only had the re-tune timer in mind, which is why I was less
restrictive and suggesting you to add hold/disable. Sorry about that.

Now, with the above in mind I believe you have similar issues with
patch5 (mmc: core: Hold re-tuning during switch commands) and patch6
(mmc: core: Hold re-tuning during erase commands). And that's because
there are cases when the switch/erase commands are the first commands
sent, after the sdhci host has been runtime PM suspended. I guess we
need a way to make sure we don't hold re-tune for these cases.

An option to deal with that is to use a separate flag set by host
drivers, though the mmc_needs_retune() API and let that one override
another.

Forgive me for pushing you back and forth for how to do this, but it


Not a problem. Thanks for persevering.


seems like we still have some outstanding issues to resolve.


So that then more or less leaves us with one outstanding issue. The
SDIO irq wakeup scenario.

How will that work for sdhci?

Your suggestion is to hold re-tune for the SDIO wakeup command. If I
understand correct that could be overridden when the host flags
need_retune from its runtime PM suspend callback, right?

That then mean that the re-tuning will be done prior sending the
wakeup command? That wouldn't work, unless the re-tune command also
act as wakeup, which I doubt.


The wakeup command has to come first.



If I _haven't_ understand correctly and you mean that the SDIO wakeup
command shall be invoked prior re-tuning is done; that would mean that
SDHCI will send a command to the card without first satisfying its
need for a re-tune. And that wouldn't work either, right?


My understanding is that the wakeup command 

Re: [PATCH 3/3 v3] mmc: sh_mmcif: calculate best clock with parent clock

2015-04-22 Thread Geert Uytterhoeven
On Wed, Apr 22, 2015 at 10:18 AM, Ulf Hansson  wrote:
>> The MMCIF driver shouldn't need to be aware of the possible values supported
>> by the CPG driver.
>>
>> Can't you loop over all MMC dividers, and
>>   1. calculate the needed parent clock rate for that MMC divider,
>>   2. use clk_set_rate_range()[*] to find a (close) parent clock rate,
>>   3. calculate the effective clock rate based on MMC divider and parent
>>  clock rate.
>> and use the best effective clock rate found?
>>
>> [*] It's a pity the clk API doesn't seem to have a function to query or check
>> clock rates without actually setting them. Or am I missing something?
>
> What about clk_round_rate(), can't that be used?

Thanks, I knew there had to be something, but couldn't find it at first sight.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3 v3] mmc: sh_mmcif: calculate best clock with parent clock

2015-04-22 Thread Ulf Hansson
On 22 April 2015 at 09:49, Geert Uytterhoeven  wrote:
> Hi Morimoto-san,
>
> On Wed, Apr 22, 2015 at 3:04 AM, Kuninori Morimoto
>  wrote:
>>> > +static const struct sh_mmcif_parent_clk mmcif_gen2_parent_clk = {
>>> > +   .max = 9750,
>>> > +   .min = 12187500,
>>> > +   .clkdiv_map = 0x3ff,
>>>
>>> Shouldn't this come from private data in the CPG clock driver, which 
>>> supplies
>>> the parent clock? Then the mmcif driver will be independent from the parent
>>> clock.
>>
>> As Laurent mentioned in other feedback, parent clock (= CFG::MMCxCKCR)
>> is div6, and it can set 1/1 - 1/64 (= 78000 - 12187500)
>> But, MMC can use 1/8 - 1/64 (= 9750 - 12187500), we don't know this limit
>> came from. we thought that having limit on DIV6 is not good idea.
>
> So the upper limit is a limitation of the MMCIF hardware, while the lower
> limit is a limitation of the CPG.
>
> Then the upper limit should be either in the driver (as you did, using
> different compatible values), or it can be described in DT.
> (cfr. "git grep max.*freq -- arch/arm/boot/dts/").
> Personally, I'm leaning towards the latter.
>
> IMHO the lower limit doesn't belong here.
>
>>> > +   /* FIXME */
>>> > +   if (pclk->max != pclk->min * (pclk->max / pclk->min)) {
>>> > +   dev_err(&host->pd->dev, "not assumed parent 
>>> > clk\n");
>>> > +   return;
>>> > +   }
>>>
>>> Why?
>>
>> This time, max/min = 9750/12187500 = 8.
>> But, we don't know the future.
>> It will be non-assumed result if it was below or similar
>> max = 91406250
>> min = 12187500 (max = min * 15/2)
>
> You can still calculate a divisor if max is not an integer multiple of min,
> can't you?
>
> And currently the check above cannot trigger, as you have hardcoded values
> in the driver source that don't trigger it.
>
>>> > +   parent_freq = 0;
>>> > +   clkdiv = 0;
>>> > +   diff_min = ~0;
>>> > +   for (i = pclk->max / pclk->min; i > 0; i--) {
>>> > +   for (j = fls(pclk->clkdiv_map); j >= 0; j--) {
>>> > +   if (!((1 << j) & pclk->clkdiv_map))
>>> > +   continue;
>>> > +
>>> > +   myclk = ((pclk->min * i) / (1 << (j + 
>>> > 1)));
>>> > +   diff = (myclk > clk) ? myclk - clk : clk 
>>> > - myclk;
>>> > +
>>> > +   if (diff <= diff_min) {
>>> > +   parent_freq = pclk->min * i;
>>> > +   clkdiv = j;
>>> > +   diff_min = diff;
>>> > +   }
>>> > +   }
>>> > +   }
>>>
>>> Shouldn't the above be handled by the CPG clock driver, through a clk API?
>>
>> I don't think so.
>> it calculates best of parent clock (= from CPG) and divider (= from MMC)
>> Why CPG driver need to care about MMC's divider ??
>
> The MMCIF driver shouldn't need to be aware of the possible values supported
> by the CPG driver.
>
> Can't you loop over all MMC dividers, and
>   1. calculate the needed parent clock rate for that MMC divider,
>   2. use clk_set_rate_range()[*] to find a (close) parent clock rate,
>   3. calculate the effective clock rate based on MMC divider and parent
>  clock rate.
> and use the best effective clock rate found?
>
> [*] It's a pity the clk API doesn't seem to have a function to query or check
> clock rates without actually setting them. Or am I missing something?

What about clk_round_rate(), can't that be used?

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


Re: [RFC] i.MX25/35/SDHCI: switch off DMA usage

2015-04-22 Thread Dong Aisheng
On Tue, Apr 14, 2015 at 11:42:00AM +0200, Juergen Borleis wrote:
> Hi,
> 
> On Friday 27 March 2015 12:44:03 Dong Aisheng wrote:
> > On Fri, Mar 27, 2015 at 11:52:04AM +0100, Juergen Borleis wrote:
> > > DMA and the required overhead on very small data blocks seems an
> > > expensive operation. Due to erratum ENGCM07207 for i.MX25 and i.MX35 SoCs
> > > the support for multiblock transfers is disabled which results into a
> > > huge amount of single 512 byte sector transfers and interrupts. This
> > > slows down the transmission speed to below 500 kiB/s (even at 50 MHz SD
> > > card clock). Using PIO instead of DMA to avoid ENGCM07207 happens and
> > > re-enabling multiblock transfers again improve the transmission
> > > capability up to about 2.5 MiB/s.
> > >
> > > I'm still not sure if ENGCM07207 is related to DMA only and can not
> > > happen when PIO is used instead. Someone out there with experience
> > > regarding this topic?
> >
> > The errata does not state it's related to DMA only.
> > http://cache.freescale.com/files/dsp/doc/errata/IMX35CE.pdf
> > I could double check with our IC guys to confirm it.
> 
> Gentle ping.
> 

Hi Juergen,

Got the info from our IC guy.
PIO mode is not related to AHB access, so AHB hang is not exist.
But, he supposes the incorrect state machine of IP after sending CMD12 should
also exist on PIO mode. We may still needs reset the controller after sending
CMD12.(Reset before sending CMD12 should also work)

BTW the errata already indicates a workaround for AHB hang:
"To abort data transfers on the AHB, software can reset the eSDHC by
writing 1 to SYSCTL[24] (RSTA)."

So the issue could be avoided by reset controller.

And the current SDHCI driver already does reset if any error(cmd or data)
happens, and current MMC core seems only sending stop command when meets error.
(not sending stop during normal transfer).
So it looks to me the issue may already be workarounded and can not
be reproduced.

I'm not sure if anyone really meets the issue.

By googling the original post of this patch, it seems the patch
owner also did not reproduce this issue.
See:
http://lists.infradead.org/pipermail/linux-arm-kernel/2010-October/029818.html

I would suggest someone who has the MX35/MX25 boards, to run the test again by
manually triggering a data error.(e.g: shorting data line to ground during
transfer). Then to see if the controller can still work well.

If can not reproduce, then we can remove this errata and back to DMA mode.

Regards
Dong Aisheng

> Regards,
> Juergen
> 
> -- 
> Pengutronix e.K.                              | Juergen Borleis             |
> Industrial Linux Solutions        | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[GIT PULL] MMC fixes for v.4.1 rc1

2015-04-22 Thread Ulf Hansson
Hi Linus,

Here is two mmc fixes for v.4.1 rc1, based on your master branch a few days ago.

Details are as usual found in the signed tag. Please pull this in!

Kind regards
Ulf Hansson


The following changes since commit 4fc8adcfec3da639da76e8314c9ccefe5bf9a045:

  Merge branch 'for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs (2015-04-16
23:27:56 -0400)

are available in the git repository at:


  git://git.linaro.org/people/ulf.hansson/mmc.git tags/mmc-4.1-rc1

for you to fetch changes up to 96541bac0b4e62efa42e7900d9b32e6baa9a214c:

  Revert "mmc: core: Convert mmc_driver to device_driver" (2015-04-17
11:48:01 +0200)


MMC core:
- Fix error code propagation in mmc_pwrseq_simple_alloc()
- Revert "mmc: core: Convert mmc_driver to device_driver"


Javier Martinez Canillas (1):
  mmc: pwrseq: Fix error code propagation in mmc_pwrseq_simple_alloc()

Ulf Hansson (1):
  Revert "mmc: core: Convert mmc_driver to device_driver"

 drivers/mmc/card/block.c| 34 +-
 drivers/mmc/card/mmc_test.c | 18 +++---
 drivers/mmc/core/bus.c  | 41 +
 drivers/mmc/core/pwrseq.c   |  2 +-
 include/linux/mmc/card.h| 14 --
 5 files changed, 70 insertions(+), 39 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3 v3] mmc: sh_mmcif: calculate best clock with parent clock

2015-04-22 Thread Geert Uytterhoeven
Hi Morimoto-san,

On Wed, Apr 22, 2015 at 3:04 AM, Kuninori Morimoto
 wrote:
>> > +static const struct sh_mmcif_parent_clk mmcif_gen2_parent_clk = {
>> > +   .max = 9750,
>> > +   .min = 12187500,
>> > +   .clkdiv_map = 0x3ff,
>>
>> Shouldn't this come from private data in the CPG clock driver, which supplies
>> the parent clock? Then the mmcif driver will be independent from the parent
>> clock.
>
> As Laurent mentioned in other feedback, parent clock (= CFG::MMCxCKCR)
> is div6, and it can set 1/1 - 1/64 (= 78000 - 12187500)
> But, MMC can use 1/8 - 1/64 (= 9750 - 12187500), we don't know this limit
> came from. we thought that having limit on DIV6 is not good idea.

So the upper limit is a limitation of the MMCIF hardware, while the lower
limit is a limitation of the CPG.

Then the upper limit should be either in the driver (as you did, using
different compatible values), or it can be described in DT.
(cfr. "git grep max.*freq -- arch/arm/boot/dts/").
Personally, I'm leaning towards the latter.

IMHO the lower limit doesn't belong here.

>> > +   /* FIXME */
>> > +   if (pclk->max != pclk->min * (pclk->max / pclk->min)) {
>> > +   dev_err(&host->pd->dev, "not assumed parent 
>> > clk\n");
>> > +   return;
>> > +   }
>>
>> Why?
>
> This time, max/min = 9750/12187500 = 8.
> But, we don't know the future.
> It will be non-assumed result if it was below or similar
> max = 91406250
> min = 12187500 (max = min * 15/2)

You can still calculate a divisor if max is not an integer multiple of min,
can't you?

And currently the check above cannot trigger, as you have hardcoded values
in the driver source that don't trigger it.

>> > +   parent_freq = 0;
>> > +   clkdiv = 0;
>> > +   diff_min = ~0;
>> > +   for (i = pclk->max / pclk->min; i > 0; i--) {
>> > +   for (j = fls(pclk->clkdiv_map); j >= 0; j--) {
>> > +   if (!((1 << j) & pclk->clkdiv_map))
>> > +   continue;
>> > +
>> > +   myclk = ((pclk->min * i) / (1 << (j + 1)));
>> > +   diff = (myclk > clk) ? myclk - clk : clk - 
>> > myclk;
>> > +
>> > +   if (diff <= diff_min) {
>> > +   parent_freq = pclk->min * i;
>> > +   clkdiv = j;
>> > +   diff_min = diff;
>> > +   }
>> > +   }
>> > +   }
>>
>> Shouldn't the above be handled by the CPG clock driver, through a clk API?
>
> I don't think so.
> it calculates best of parent clock (= from CPG) and divider (= from MMC)
> Why CPG driver need to care about MMC's divider ??

The MMCIF driver shouldn't need to be aware of the possible values supported
by the CPG driver.

Can't you loop over all MMC dividers, and
  1. calculate the needed parent clock rate for that MMC divider,
  2. use clk_set_rate_range()[*] to find a (close) parent clock rate,
  3. calculate the effective clock rate based on MMC divider and parent
 clock rate.
and use the best effective clock rate found?

[*] It's a pity the clk API doesn't seem to have a function to query or check
clock rates without actually setting them. Or am I missing something?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V6 08/15] mmc: mmc: Hold re-tuning if the card is put to sleep

2015-04-22 Thread Adrian Hunter
On 21/04/15 21:25, Arend van Spriel wrote:
> On 04/21/15 14:26, Adrian Hunter wrote:
>> On 21/04/15 14:53, Ulf Hansson wrote:
>>> On 21 April 2015 at 13:00, Adrian Hunter  wrote:
 On 21/04/15 12:42, Ulf Hansson wrote:
> On 20 April 2015 at 14:09, Adrian Hunter  wrote:
>> Currently "mmc sleep" is used before power off and
>> is not paired with waking up. Nevertheless hold
>> re-tuning.
>>
>> Signed-off-by: Adrian Hunter
>> ---
>>   drivers/mmc/core/mmc.c | 14 +++---
>>   1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index f36c76f..daf9954 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -21,6 +21,7 @@
>>   #include
>>
>>   #include "core.h"
>> +#include "host.h"
>>   #include "bus.h"
>>   #include "mmc_ops.h"
>>   #include "sd_ops.h"
>> @@ -1504,6 +1505,7 @@ static int mmc_can_sleep(struct mmc_card *card)
>>  return (card&&  card->ext_csd.rev>= 3);
>>   }
>>
>> +/* If necessary, callers must hold re-tuning */
>>   static int mmc_sleep(struct mmc_host *host)
>>   {
>>  struct mmc_command cmd = {0};
>> @@ -1631,6 +1633,7 @@ static int _mmc_suspend(struct mmc_host *host,
>> bool is_suspend)
>>  int err = 0;
>>  unsigned int notify_type = is_suspend ?
>> EXT_CSD_POWER_OFF_SHORT :
>>  EXT_CSD_POWER_OFF_LONG;
>> +   bool retune_release = false;
>>
>>  BUG_ON(!host);
>>  BUG_ON(!host->card);
>> @@ -1651,17 +1654,22 @@ static int _mmc_suspend(struct mmc_host *host,
>> bool is_suspend)
>>  goto out;
>>
>>  if (mmc_can_poweroff_notify(host->card)&&
>> -   ((host->caps2&  MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
>> +   ((host->caps2&  MMC_CAP2_FULL_PWR_CYCLE) ||
>> !is_suspend)) {
>>  err = mmc_poweroff_notify(host->card, notify_type);
>> -   else if (mmc_can_sleep(host->card))
>> +   } else if (mmc_can_sleep(host->card)) {
>> +   mmc_retune_hold(host);
>>  err = mmc_sleep(host);
>> -   else if (!mmc_host_is_spi(host))
>> +   } else if (!mmc_host_is_spi(host)) {
>>  err = mmc_deselect_cards(host);
>> +   }
>>
>>  if (!err) {
>>  mmc_power_off(host);
>>  mmc_card_set_suspended(host->card);
>>  }
>> +
>> +   if (retune_release)
>> +   mmc_retune_release(host);
>>   out:
>>  mmc_release_host(host);
>>  return err;
>> -- 
>> 1.9.1
>>
>
> According to our previous discussions I have given this some more
> thinking.
>
> I don't think we can allow to hold/disable re-tune in this path at
> all. That's because we are claiming the host here and the sleep
> command might then be the first command we invoke during the system PM
> sequence.
>
> That means sdhci might have flagged need_retune, since it's been
> runtime PM suspended. And for those scenarios I guess we really need
> to do a re-tune prior sending the sleep command, right?

 Yes, although that is how it works.
>>>
>>> Ohh, you are one step ahead of me. Good! :-)
>>>

 Previously I had two functions mmc_retune_hold() and mmc_retune_and_hold()
 but after one of the revisions I found that only one was needed. I stuck
 with the mmc_retune_hold() name because it doesn't necessarily cause a
 re-tune, but only if the hold count was zero and a retune is needed.

>
> Earlier I only had the re-tune timer in mind, which is why I was less
> restrictive and suggesting you to add hold/disable. Sorry about that.
>
> Now, with the above in mind I believe you have similar issues with
> patch5 (mmc: core: Hold re-tuning during switch commands) and patch6
> (mmc: core: Hold re-tuning during erase commands). And that's because
> there are cases when the switch/erase commands are the first commands
> sent, after the sdhci host has been runtime PM suspended. I guess we
> need a way to make sure we don't hold re-tune for these cases.
>
> An option to deal with that is to use a separate flag set by host
> drivers, though the mmc_needs_retune() API and let that one override
> another.
>
> Forgive me for pushing you back and forth for how to do this, but it

 Not a problem. Thanks for persevering.

> seems like we still have some outstanding issues to resolve.
>>>
>>> So that then more or less leaves us with one outstanding issue. The
>>> SDIO irq wakeup scenario.
>>>
>>> How will that work for sdhci?
>>>
>>> Your suggestion is to hold re-tune for the S