Re: [PATCH 1/2] phy: rockchip-emmc: retry calpad busy trimming

[Ziyuan]

hi ,

On 01/02/2018 10:21 AM, Shawn Lin wrote:

It turns out that 5us isn't enough for all cases, so let's
retry some more times to wait for caldone.

Signed-off-by: Shawn Lin<shawn@rock-chips.com>


Tested-by: Ziyuan Xu <xzy...@rock-chips.com>
On the Firefly RK3399 board.

Thanks.

--
Ziyuan Xu

Re: [PATCH 1/2] phy: rockchip-emmc: retry calpad busy trimming

[Ziyuan]

hi ,

On 01/02/2018 10:21 AM, Shawn Lin wrote:

It turns out that 5us isn't enough for all cases, so let's
retry some more times to wait for caldone.

Signed-off-by: Shawn Lin


Tested-by: Ziyuan Xu 
On the Firefly RK3399 board.

Thanks.

--
Ziyuan Xu

Re: [BUG] Boot failure since df9bcc2bc on veyron_speedy

[Ziyuan]

Hi,

On 01/10/2017 09:04 AM, S. Gilles wrote:

On 2017-01-10T08:47:16+0800, Shawn Lin wrote:

Hi

On 2017/1/9 22:49, S. Gilles wrote:

On 2017-01-09T09:19:55-0500, S. Gilles wrote:

Hi,

I have a C201, a veyron_speedy device (which uses rk3288) running vanilla
kernels. With recent kernels it will fail to boot (screen is on, with
blinking cursor, but no login prompt and the machine does not respond
over ssh).

I've bisected this to df9bcc2bc0a1f8d2963bd916698268fb2470713b, and
reverting that commit on -mainline gives me a bootable kernel. I can
provide more information as needed.

The fix is on the way,

https://patchwork.kernel.org/patch/9498527/

Great - thanks for letting me know.


Can you share the result ?

BR
Ziyuan Xu



Thanks,
S. Gilles
--
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: [BUG] Boot failure since df9bcc2bc on veyron_speedy

[Ziyuan]

Hi,

On 01/10/2017 09:04 AM, S. Gilles wrote:

On 2017-01-10T08:47:16+0800, Shawn Lin wrote:

Hi

On 2017/1/9 22:49, S. Gilles wrote:

On 2017-01-09T09:19:55-0500, S. Gilles wrote:

Hi,

I have a C201, a veyron_speedy device (which uses rk3288) running vanilla
kernels. With recent kernels it will fail to boot (screen is on, with
blinking cursor, but no login prompt and the machine does not respond
over ssh).

I've bisected this to df9bcc2bc0a1f8d2963bd916698268fb2470713b, and
reverting that commit on -mainline gives me a bootable kernel. I can
provide more information as needed.

The fix is on the way,

https://patchwork.kernel.org/patch/9498527/

Great - thanks for letting me know.


Can you share the result ?

BR
Ziyuan Xu



Thanks,
S. Gilles
--
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 v4] mmc: dw_mmc: force setup bus if active slots exist

[Ziyuan]

On 01/05/2017 03:34 PM, Shawn Lin wrote:

On 2017/1/5 15:23, Ziyuan Xu wrote:

It's necessary to setup bus if any slots are present.
- update clock after ctrl reset
- if the host has genpd node, we can guarantee the clock is available
before starting request. Otherwies, the clock register is reset once
power off the pd, and host can't output the active clock during
communication.

fixes: e9ed8835e990 ("mmc: dw_mmc: add runtime PM callback")
Reported-by: Randy Li <randy...@rock-chips.com>
Signed-off-by: Ziyuan Xu <xzy...@rock-chips.com>

---
Hi guys,

I found a similar issue on rk3399 platform, which has a genpd node for
SD card host. Power off-on pd will reset the registers to a default
value (ie. CLKENA), so that the host can't output the active clock
during communication.



Indeed, Caesar recently introduced all the genpd for rk3399 platform,
so we need to restore them.


So we need to setup bus in rpm resume. It also wraps the update clock
behaviour which I did in V3.

Thanks,
Ziyuan Xu


Changes in v4:
- update commit message
- fix SD host rpm resume can't work

Changes in v3:
- only reset host with active slot.

Changes in v2:
- update the commit message
- use dw_mci_reset instead of dw_mci_ctrl_reset

 drivers/mmc/host/dw_mmc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index b44306b..b6053b3 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -3354,10 +3354,10 @@ int dw_mci_runtime_resume(struct device *dev)

 if (!slot)
 continue;
-if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER) {
+if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER)
 dw_mci_set_ios(slot->mmc, >mmc->ios);
-dw_mci_setup_bus(slot, true);
-}
+/* Force setup bus to guarantee available clock output */
+dw_mci_setup_bus(slot, true);


So the spamming message about

"Bus speed (slot %d) = %dHz (slot req %dHz, actual %dHZ div = %d)\n"

will always be there, right? So you could append a new patch to shut
up it as I think it's useless no matter for system pm or rpm to print
it.  How about?


Fine, it's favourable with dev_vdbg if the dw_mmc rpm is enabled.
Hi Jaehoon,
What's your opinion? If you think this patch and shawn's advice are 
acceptable, I will send the v5 patch.


BR
Ziyuan Xu




 }

 /* Now that slots are all setup, we can enable card detect */

Re: [PATCH v4] mmc: dw_mmc: force setup bus if active slots exist

[Ziyuan]

On 01/05/2017 03:34 PM, Shawn Lin wrote:

On 2017/1/5 15:23, Ziyuan Xu wrote:

It's necessary to setup bus if any slots are present.
- update clock after ctrl reset
- if the host has genpd node, we can guarantee the clock is available
before starting request. Otherwies, the clock register is reset once
power off the pd, and host can't output the active clock during
communication.

fixes: e9ed8835e990 ("mmc: dw_mmc: add runtime PM callback")
Reported-by: Randy Li 
Signed-off-by: Ziyuan Xu 

---
Hi guys,

I found a similar issue on rk3399 platform, which has a genpd node for
SD card host. Power off-on pd will reset the registers to a default
value (ie. CLKENA), so that the host can't output the active clock
during communication.



Indeed, Caesar recently introduced all the genpd for rk3399 platform,
so we need to restore them.


So we need to setup bus in rpm resume. It also wraps the update clock
behaviour which I did in V3.

Thanks,
Ziyuan Xu


Changes in v4:
- update commit message
- fix SD host rpm resume can't work

Changes in v3:
- only reset host with active slot.

Changes in v2:
- update the commit message
- use dw_mci_reset instead of dw_mci_ctrl_reset

 drivers/mmc/host/dw_mmc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index b44306b..b6053b3 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -3354,10 +3354,10 @@ int dw_mci_runtime_resume(struct device *dev)

 if (!slot)
 continue;
-if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER) {
+if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER)
 dw_mci_set_ios(slot->mmc, >mmc->ios);
-dw_mci_setup_bus(slot, true);
-}
+/* Force setup bus to guarantee available clock output */
+dw_mci_setup_bus(slot, true);


So the spamming message about

"Bus speed (slot %d) = %dHz (slot req %dHz, actual %dHZ div = %d)\n"

will always be there, right? So you could append a new patch to shut
up it as I think it's useless no matter for system pm or rpm to print
it.  How about?


Fine, it's favourable with dev_vdbg if the dw_mmc rpm is enabled.
Hi Jaehoon,
What's your opinion? If you think this patch and shawn's advice are 
acceptable, I will send the v5 patch.


BR
Ziyuan Xu




 }

 /* Now that slots are all setup, we can enable card detect */

Re: [PATCH] mmc: dw_mmc: Fix some coding style

[Ziyuan]

On 01/05/2017 02:32 AM, Joe Perches wrote:

On Wed, 2017-01-04 at 20:52 +0800, Ziyuan Xu wrote:

Let's fix the warnings from checkpatch.pl:

- line over 80 characters;
- block comments should align the * on each Lines;
- statements not starting on a tabstop.

Signed-off-by: Ziyuan Xu <xzy...@rock-chips.com>
---

  drivers/mmc/host/dw_mmc.c | 33 +
  1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c

[]

@@ -94,7 +94,8 @@ struct idmac_desc {
  
  	__le32		des1;	/* Buffer sizes */

  #define IDMAC_SET_BUFFER1_SIZE(d, s) \
-   ((d)->des1 = ((d)->des1 & cpu_to_le32(0x03ffe000)) | (cpu_to_le32((s) & 
0x1fff)))
+   ((d)->des1 = ((d)->des1 & cpu_to_le32(0x03ffe000)) | \
+(cpu_to_le32((s) & 0x1fff)))

Please look to improve code rather than just shut up
the brainless checkpatch script.

If this is really valuable, it'd probably be better as
an inline function, or as it's only used once, just
as direct code in that one place.


Fine, I get it. Thanks for the advice.:-)



--
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] mmc: dw_mmc: Fix some coding style

[Ziyuan]

On 01/05/2017 02:32 AM, Joe Perches wrote:

On Wed, 2017-01-04 at 20:52 +0800, Ziyuan Xu wrote:

Let's fix the warnings from checkpatch.pl:

- line over 80 characters;
- block comments should align the * on each Lines;
- statements not starting on a tabstop.

Signed-off-by: Ziyuan Xu 
---

  drivers/mmc/host/dw_mmc.c | 33 +
  1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c

[]

@@ -94,7 +94,8 @@ struct idmac_desc {
  
  	__le32		des1;	/* Buffer sizes */

  #define IDMAC_SET_BUFFER1_SIZE(d, s) \
-   ((d)->des1 = ((d)->des1 & cpu_to_le32(0x03ffe000)) | (cpu_to_le32((s) & 
0x1fff)))
+   ((d)->des1 = ((d)->des1 & cpu_to_le32(0x03ffe000)) | \
+(cpu_to_le32((s) & 0x1fff)))

Please look to improve code rather than just shut up
the brainless checkpatch script.

If this is really valuable, it'd probably be better as
an inline function, or as it's only used once, just
as direct code in that one place.


Fine, I get it. Thanks for the advice.:-)



--
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 v4] mmc: dw_mmc: force setup bus if active slots exist

[Ziyuan Xu]

It's necessary to setup bus if any slots are present.
- update clock after ctrl reset
- if the host has genpd node, we can guarantee the clock is available
before starting request. Otherwies, the clock register is reset once
power off the pd, and host can't output the active clock during
communication.

fixes: e9ed8835e990 ("mmc: dw_mmc: add runtime PM callback")
Reported-by: Randy Li <randy...@rock-chips.com>
Signed-off-by: Ziyuan Xu <xzy...@rock-chips.com>

---
Hi guys,

I found a similar issue on rk3399 platform, which has a genpd node for
SD card host. Power off-on pd will reset the registers to a default
value (ie. CLKENA), so that the host can't output the active clock
during communication.

So we need to setup bus in rpm resume. It also wraps the update clock
behaviour which I did in V3.

Thanks,
Ziyuan Xu


Changes in v4:
- update commit message
- fix SD host rpm resume can't work

Changes in v3:
- only reset host with active slot.

Changes in v2:
- update the commit message
- use dw_mci_reset instead of dw_mci_ctrl_reset

 drivers/mmc/host/dw_mmc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index b44306b..b6053b3 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -3354,10 +3354,10 @@ int dw_mci_runtime_resume(struct device *dev)
 
if (!slot)
continue;
-   if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER) {
+   if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER)
dw_mci_set_ios(slot->mmc, >mmc->ios);
-   dw_mci_setup_bus(slot, true);
-   }
+   /* Force setup bus to guarantee available clock output */
+   dw_mci_setup_bus(slot, true);
}
 
/* Now that slots are all setup, we can enable card detect */
-- 
2.7.4

[PATCH v4] mmc: dw_mmc: force setup bus if active slots exist

[Ziyuan Xu]

It's necessary to setup bus if any slots are present.
- update clock after ctrl reset
- if the host has genpd node, we can guarantee the clock is available
before starting request. Otherwies, the clock register is reset once
power off the pd, and host can't output the active clock during
communication.

fixes: e9ed8835e990 ("mmc: dw_mmc: add runtime PM callback")
Reported-by: Randy Li 
Signed-off-by: Ziyuan Xu 

---
Hi guys,

I found a similar issue on rk3399 platform, which has a genpd node for
SD card host. Power off-on pd will reset the registers to a default
value (ie. CLKENA), so that the host can't output the active clock
during communication.

So we need to setup bus in rpm resume. It also wraps the update clock
behaviour which I did in V3.

Thanks,
Ziyuan Xu


Changes in v4:
- update commit message
- fix SD host rpm resume can't work

Changes in v3:
- only reset host with active slot.

Changes in v2:
- update the commit message
- use dw_mci_reset instead of dw_mci_ctrl_reset

 drivers/mmc/host/dw_mmc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index b44306b..b6053b3 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -3354,10 +3354,10 @@ int dw_mci_runtime_resume(struct device *dev)
 
if (!slot)
continue;
-   if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER) {
+   if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER)
dw_mci_set_ios(slot->mmc, >mmc->ios);
-   dw_mci_setup_bus(slot, true);
-   }
+   /* Force setup bus to guarantee available clock output */
+   dw_mci_setup_bus(slot, true);
}
 
/* Now that slots are all setup, we can enable card detect */
-- 
2.7.4

[PATCH v3] mmc: dw_mmc: revise the reset path in runtime resume

[Ziyuan Xu]

Immediately after reset, issue the command which sets
update_clock_register_only bit, the card clock will restart. Revise
dw_mci_ctrl_reset to dw_mci_reset, which has wrapped this sequence.

Moreover, we don't need to reset host without active slot.

The patch fixes commit e9ed8835e990 ("mmc: dw_mmc: add runtime PM
callback"). MMC_PM_KEEP_POWEP is disabled for SD card and eMMC slots, so
that they have no chance to invoke dw_mci_setup_bus for update clock
behaviour. Let's consummate it.

Reported-by: Randy Li <randy...@rock-chips.com>
Signed-off-by: Ziyuan Xu <xzy...@rock-chips.com>

---

Changes in v3:
- only reset host with active slot.

Changes in v2:
- update the commit message
- use dw_mci_reset instead of dw_mci_ctrl_reset

 drivers/mmc/host/dw_mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index b44306b..bd21242 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -3324,7 +3324,7 @@ int dw_mci_runtime_resume(struct device *dev)
if (ret)
goto err;
 
-   if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS)) {
+   if (host->cur_slot && !dw_mci_reset(host)) {
clk_disable_unprepare(host->ciu_clk);
ret = -ENODEV;
goto err;
-- 
2.7.4

[PATCH v3] mmc: dw_mmc: revise the reset path in runtime resume

[Ziyuan Xu]

Immediately after reset, issue the command which sets
update_clock_register_only bit, the card clock will restart. Revise
dw_mci_ctrl_reset to dw_mci_reset, which has wrapped this sequence.

Moreover, we don't need to reset host without active slot.

The patch fixes commit e9ed8835e990 ("mmc: dw_mmc: add runtime PM
callback"). MMC_PM_KEEP_POWEP is disabled for SD card and eMMC slots, so
that they have no chance to invoke dw_mci_setup_bus for update clock
behaviour. Let's consummate it.

Reported-by: Randy Li 
Signed-off-by: Ziyuan Xu 

---

Changes in v3:
- only reset host with active slot.

Changes in v2:
- update the commit message
- use dw_mci_reset instead of dw_mci_ctrl_reset

 drivers/mmc/host/dw_mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index b44306b..bd21242 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -3324,7 +3324,7 @@ int dw_mci_runtime_resume(struct device *dev)
if (ret)
goto err;
 
-   if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS)) {
+   if (host->cur_slot && !dw_mci_reset(host)) {
clk_disable_unprepare(host->ciu_clk);
ret = -ENODEV;
goto err;
-- 
2.7.4

Re: [PATCH v2] mmc: dw_mmc: revise the reset path in runtime resume

[Ziyuan]

On 01/05/2017 09:29 AM, Jaehoon Chung wrote:

On 01/05/2017 01:04 AM, ayaka wrote:


On 01/04/2017 08:51 PM, Ziyuan Xu wrote:

Immediately after reset, issue the command which sets
update_clock_register_only bit, the card clock will restart. Revise
dw_mci_ctrl_reset to dw_mci_reset, which has wrapped this sequence.

The patch fixes commit e9ed8835e990 ("mmc: dw_mmc: add runtime PM
callback"). MMC_PM_KEEP_POWEP is disabled for SD card and eMMC slots, so
that they have no chance to invoke dw_mci_setup_bus for update clock
behaviour. Let's consummate it.

Reported-by: Randy Li <randy...@rock-chips.com>
Signed-off-by: Ziyuan Xu <xzy...@rock-chips.com>

Thank you Ziyuan, it works for me, but I capture those warning message in boot 
log
[5.943096] Unable to handle kernel NULL pointer dereference at virtual 
address 0004
[5.951227] pgd = c0004000
[5.953958] [0004] *pgd=
[5.957563] Internal error: Oops: 5 [#1] SMP ARM
[5.962177] Modules linked in:
[5.965238] CPU: 0 PID: 117 Comm: kworker/0:3 Not tainted 
4.9.0-next-20161224+ #122
[5.972875] Hardware name: Rockchip (Device Tree)
[5.977586] Workqueue: events_freezable mmc_rescan
[5.982377] task: ee186740 task.stack: ee1d2000
[5.986908] PC is at mci_send_cmd.constprop.5+0x20/0xb0
[5.992129] LR is at dw_mci_reset+0xdc/0x130
[5.996400] pc : []lr : [] psr: 6013
[5.996400] sp : ee1d3d38  ip : ee1d3d68  fp : ee1d3d64
[6.007862] r10: ee1d2000  r9 : c04fe918  r8 : c04fe918
[6.013083] r7 : c0c03d00  r6 : ee171780  r5 : c0c03d00  r4 : ee17e618
[6.019604] r3 : 0201  r2 : f08e4000  r1 : 0020  r0 : 
[6.026127] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM Segment none
[6.033253] Control: 10c5387d  Table: 406a  DAC: 0051
[6.038994] Process kworker/0:3 (pid: 117, stack limit = 0xee1d2210)
[6.045340] Stack: (0xee1d3d38 to 0xee1d4000)
[6.049696] 3d20:   
c0495ed0 c0714e60
[6.057870] 3d40: ee17e618 0001 ee171780 c0c03d00 c04fe918 c04fe918 
ee1d3d7c ee1d3d68
[6.066043] 3d60: c05d0308 c05cf450 ee17e618  ee1d3da4 ee1d3d80 
c05d07fc c05d0238
[6.074216] 3d80:  eeae5a10  c0c03d00 c04fe918 c04fe918 
ee1d3db4 ee1d3da8
[6.082389] 3da0: c04fe954 c05d0718 ee1d3dec ee1d3db8 c0501c70 c04fe924 
c0162310 c044f7b0
[6.090563] 3dc0:  eeae5a10 c0c62510  c0c03d00 c04fe918 
c0175574 ee1d2000
[6.098737] 3de0: ee1d3e0c ee1d3df0 c0501df0 c0501b88 eeae5a10 c0c62510 
 c0c03d00
[6.106911] 3e00: ee1d3e5c ee1d3e10 c0501898 c0501dcc c0c0408c eeae5aa8 
eeda0d00 
[6.115085] 3e20: c0c04108 ee1867c0 ee1d3eac 00040900 c016e53c eeae5a10 
0004 eeae5a84
[6.123259] 3e40: 6013 ee1d2000 c0c0408c ee160c00 ee1d3e7c ee1d3e60 
c0501b5c c05013e4
[6.131433] 3e60:  ee160e1c 2013  ee1d3ecc ee1d3e80 
c05b0128 c0501af8
[6.139607] 3e80: e000 ee160e80  ee186740 c0156324 0100 
0200 00040900
[6.147781] 3ea0: ee1d3f14 ee160e94 ee160e1c ee160c00 6013 eeda3e00 
ee160e98 
[6.155956] 3ec0: ee1d3ef4 ee1d3ed0 c05b3a68 c05aff80 ee160e94 ee176380 
eeda0880 
[6.164130] 3ee0: eeda3e00 ee160e98 ee1d3f2c ee1d3ef8 c0142374 c05b383c 
c0c03d00 eeda0898
[6.172303] 3f00: ee1d2000 eeda0880 ee176398 0008 c0c03d00 eeda0898 
ee1d2000 ee176380
[6.180477] 3f20: ee1d3f74 ee1d3f30 c014329c c01421d0 ee1d3f54 c0959c8c 
ee1d2000 
[6.188650] 3f40: ee15a880 c0c20f1a ee1d1e7c eea00600 ee1d2000  
ee15a880 ee176380
[6.196824] 3f60: ee1d1e7c eea0061c ee1d3fac ee1d3f78 c0148ed0 c0143248 
 c014323c
[6.204996] 3f80: ee1d3fac ee15a880 c0148da8    
 
[6.213168] 3fa0:  ee1d3fb0 c0108e90 c0148db4   
 
[6.221341] 3fc0:       
 
[6.229514] 3fe0:     0013  
 
[6.237696] [] (mci_send_cmd.constprop.5) from [] 
(dw_mci_reset+0xdc/0x130)
[6.246393] [] (dw_mci_reset) from [] 
(dw_mci_runtime_resume+0xf0/0x1cc)
[6.254833] [] (dw_mci_runtime_resume) from [] 
(pm_generic_runtime_resume+0x3c/0x48)
[6.264313] [] (pm_generic_runtime_resume) from [] 
(__rpm_callback+0xf4/0x244)
[6.273273] [] (__rpm_callback) from [] 
(rpm_callback+0x30/0x90)
[6.281017] [] (rpm_callback) from [] 
(rpm_resume+0x4c0/0x714)
[6.288588] [] (rpm_resume) from [] 
(__pm_runtime_resume+0x70/0x90)
[6.296592] [] (__pm_runtime_resume) from [] 
(__mmc_claim_host+0x1b4/0x1ec)
[6.305287] [] (__mmc_claim_host) from [] 
(mmc_rescan+0x238/0x3f4)
[6.313206] [] (mmc_rescan) from [] 
(process_one_work+0x1b0/0x4c0)
[6.321125] [] (process_one_work) from [] 
(worker_thread+0x60/0x548)
[6.329215] [] (worker_thread) from [] 
(kthread+0x12

Re: [PATCH v2] mmc: dw_mmc: revise the reset path in runtime resume

[Ziyuan]

On 01/05/2017 09:29 AM, Jaehoon Chung wrote:

On 01/05/2017 01:04 AM, ayaka wrote:


On 01/04/2017 08:51 PM, Ziyuan Xu wrote:

Immediately after reset, issue the command which sets
update_clock_register_only bit, the card clock will restart. Revise
dw_mci_ctrl_reset to dw_mci_reset, which has wrapped this sequence.

The patch fixes commit e9ed8835e990 ("mmc: dw_mmc: add runtime PM
callback"). MMC_PM_KEEP_POWEP is disabled for SD card and eMMC slots, so
that they have no chance to invoke dw_mci_setup_bus for update clock
behaviour. Let's consummate it.

Reported-by: Randy Li 
Signed-off-by: Ziyuan Xu 

Thank you Ziyuan, it works for me, but I capture those warning message in boot 
log
[5.943096] Unable to handle kernel NULL pointer dereference at virtual 
address 0004
[5.951227] pgd = c0004000
[5.953958] [0004] *pgd=
[5.957563] Internal error: Oops: 5 [#1] SMP ARM
[5.962177] Modules linked in:
[5.965238] CPU: 0 PID: 117 Comm: kworker/0:3 Not tainted 
4.9.0-next-20161224+ #122
[5.972875] Hardware name: Rockchip (Device Tree)
[5.977586] Workqueue: events_freezable mmc_rescan
[5.982377] task: ee186740 task.stack: ee1d2000
[5.986908] PC is at mci_send_cmd.constprop.5+0x20/0xb0
[5.992129] LR is at dw_mci_reset+0xdc/0x130
[5.996400] pc : []lr : [] psr: 6013
[5.996400] sp : ee1d3d38  ip : ee1d3d68  fp : ee1d3d64
[6.007862] r10: ee1d2000  r9 : c04fe918  r8 : c04fe918
[6.013083] r7 : c0c03d00  r6 : ee171780  r5 : c0c03d00  r4 : ee17e618
[6.019604] r3 : 0201  r2 : f08e4000  r1 : 0020  r0 : 
[6.026127] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM Segment none
[6.033253] Control: 10c5387d  Table: 406a  DAC: 0051
[6.038994] Process kworker/0:3 (pid: 117, stack limit = 0xee1d2210)
[6.045340] Stack: (0xee1d3d38 to 0xee1d4000)
[6.049696] 3d20:   
c0495ed0 c0714e60
[6.057870] 3d40: ee17e618 0001 ee171780 c0c03d00 c04fe918 c04fe918 
ee1d3d7c ee1d3d68
[6.066043] 3d60: c05d0308 c05cf450 ee17e618  ee1d3da4 ee1d3d80 
c05d07fc c05d0238
[6.074216] 3d80:  eeae5a10  c0c03d00 c04fe918 c04fe918 
ee1d3db4 ee1d3da8
[6.082389] 3da0: c04fe954 c05d0718 ee1d3dec ee1d3db8 c0501c70 c04fe924 
c0162310 c044f7b0
[6.090563] 3dc0:  eeae5a10 c0c62510  c0c03d00 c04fe918 
c0175574 ee1d2000
[6.098737] 3de0: ee1d3e0c ee1d3df0 c0501df0 c0501b88 eeae5a10 c0c62510 
 c0c03d00
[6.106911] 3e00: ee1d3e5c ee1d3e10 c0501898 c0501dcc c0c0408c eeae5aa8 
eeda0d00 
[6.115085] 3e20: c0c04108 ee1867c0 ee1d3eac 00040900 c016e53c eeae5a10 
0004 eeae5a84
[6.123259] 3e40: 6013 ee1d2000 c0c0408c ee160c00 ee1d3e7c ee1d3e60 
c0501b5c c05013e4
[6.131433] 3e60:  ee160e1c 2013  ee1d3ecc ee1d3e80 
c05b0128 c0501af8
[6.139607] 3e80: e000 ee160e80  ee186740 c0156324 0100 
0200 00040900
[6.147781] 3ea0: ee1d3f14 ee160e94 ee160e1c ee160c00 6013 eeda3e00 
ee160e98 
[6.155956] 3ec0: ee1d3ef4 ee1d3ed0 c05b3a68 c05aff80 ee160e94 ee176380 
eeda0880 
[6.164130] 3ee0: eeda3e00 ee160e98 ee1d3f2c ee1d3ef8 c0142374 c05b383c 
c0c03d00 eeda0898
[6.172303] 3f00: ee1d2000 eeda0880 ee176398 0008 c0c03d00 eeda0898 
ee1d2000 ee176380
[6.180477] 3f20: ee1d3f74 ee1d3f30 c014329c c01421d0 ee1d3f54 c0959c8c 
ee1d2000 
[6.188650] 3f40: ee15a880 c0c20f1a ee1d1e7c eea00600 ee1d2000  
ee15a880 ee176380
[6.196824] 3f60: ee1d1e7c eea0061c ee1d3fac ee1d3f78 c0148ed0 c0143248 
 c014323c
[6.204996] 3f80: ee1d3fac ee15a880 c0148da8    
 
[6.213168] 3fa0:  ee1d3fb0 c0108e90 c0148db4   
 
[6.221341] 3fc0:       
 
[6.229514] 3fe0:     0013  
 
[6.237696] [] (mci_send_cmd.constprop.5) from [] 
(dw_mci_reset+0xdc/0x130)
[6.246393] [] (dw_mci_reset) from [] 
(dw_mci_runtime_resume+0xf0/0x1cc)
[6.254833] [] (dw_mci_runtime_resume) from [] 
(pm_generic_runtime_resume+0x3c/0x48)
[6.264313] [] (pm_generic_runtime_resume) from [] 
(__rpm_callback+0xf4/0x244)
[6.273273] [] (__rpm_callback) from [] 
(rpm_callback+0x30/0x90)
[6.281017] [] (rpm_callback) from [] 
(rpm_resume+0x4c0/0x714)
[6.288588] [] (rpm_resume) from [] 
(__pm_runtime_resume+0x70/0x90)
[6.296592] [] (__pm_runtime_resume) from [] 
(__mmc_claim_host+0x1b4/0x1ec)
[6.305287] [] (__mmc_claim_host) from [] 
(mmc_rescan+0x238/0x3f4)
[6.313206] [] (mmc_rescan) from [] 
(process_one_work+0x1b0/0x4c0)
[6.321125] [] (process_one_work) from [] 
(worker_thread+0x60/0x548)
[6.329215] [] (worker_thread) from [] 
(kthread+0x128/0x158)
[6.336614] [] (kthread) from [] 
(ret_from_fork

[RESEND PATCH v3] mmc: dw_mmc: revise the reset path in runtime resume

[Ziyuan Xu]

Immediately after reset, issue the command which sets
update_clock_register_only bit, the card clock will restart. Revise
dw_mci_ctrl_reset to dw_mci_reset, which has wrapped this sequence.

Moreover, we don't need to reset host without active slot.

The patch fixes commit e9ed8835e990 ("mmc: dw_mmc: add runtime PM
callback"). MMC_PM_KEEP_POWEP is disabled for SD card and eMMC slots, so
that they have no chance to invoke dw_mci_setup_bus for update clock
behaviour. Let's consummate it.

Reported-by: Randy Li <randy...@rock-chips.com>
Signed-off-by: Ziyuan Xu <xzy...@rock-chips.com>
Signed-off-by: Shawn Lin <shawn@rock-chips.com>

---

Changes in v3:
- only reset host with active slot.
- add Shawn's Signed-off tag

Changes in v2:
- update the commit message
- use dw_mci_reset instead of dw_mci_ctrl_reset

 drivers/mmc/host/dw_mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index b44306b..bd21242 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -3324,7 +3324,7 @@ int dw_mci_runtime_resume(struct device *dev)
if (ret)
goto err;
 
-   if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS)) {
+   if (host->cur_slot && !dw_mci_reset(host)) {
clk_disable_unprepare(host->ciu_clk);
ret = -ENODEV;
goto err;
-- 
2.7.4

[RESEND PATCH v3] mmc: dw_mmc: revise the reset path in runtime resume

[Ziyuan Xu]

Immediately after reset, issue the command which sets
update_clock_register_only bit, the card clock will restart. Revise
dw_mci_ctrl_reset to dw_mci_reset, which has wrapped this sequence.

Moreover, we don't need to reset host without active slot.

The patch fixes commit e9ed8835e990 ("mmc: dw_mmc: add runtime PM
callback"). MMC_PM_KEEP_POWEP is disabled for SD card and eMMC slots, so
that they have no chance to invoke dw_mci_setup_bus for update clock
behaviour. Let's consummate it.

Reported-by: Randy Li 
Signed-off-by: Ziyuan Xu 
Signed-off-by: Shawn Lin 

---

Changes in v3:
- only reset host with active slot.
- add Shawn's Signed-off tag

Changes in v2:
- update the commit message
- use dw_mci_reset instead of dw_mci_ctrl_reset

 drivers/mmc/host/dw_mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index b44306b..bd21242 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -3324,7 +3324,7 @@ int dw_mci_runtime_resume(struct device *dev)
if (ret)
goto err;
 
-   if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS)) {
+   if (host->cur_slot && !dw_mci_reset(host)) {
clk_disable_unprepare(host->ciu_clk);
ret = -ENODEV;
goto err;
-- 
2.7.4

[PATCH v2] mmc: dw_mmc: revise the reset path in runtime resume

[Ziyuan Xu]

Immediately after reset, issue the command which sets
update_clock_register_only bit, the card clock will restart. Revise
dw_mci_ctrl_reset to dw_mci_reset, which has wrapped this sequence.

The patch fixes commit e9ed8835e990 ("mmc: dw_mmc: add runtime PM
callback"). MMC_PM_KEEP_POWEP is disabled for SD card and eMMC slots, so
that they have no chance to invoke dw_mci_setup_bus for update clock
behaviour. Let's consummate it.

Reported-by: Randy Li <randy...@rock-chips.com>
Signed-off-by: Ziyuan Xu <xzy...@rock-chips.com>

---

Changes in v2:
- update the commit message
- use dw_mci_reset instead of dw_mci_ctrl_reset

 drivers/mmc/host/dw_mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index b44306b..ed63237 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -3324,7 +3324,7 @@ int dw_mci_runtime_resume(struct device *dev)
if (ret)
goto err;
 
-   if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS)) {
+   if (!dw_mci_reset(host)) {
clk_disable_unprepare(host->ciu_clk);
ret = -ENODEV;
goto err;
-- 
2.7.4

[PATCH v2] mmc: dw_mmc: revise the reset path in runtime resume

[Ziyuan Xu]

Immediately after reset, issue the command which sets
update_clock_register_only bit, the card clock will restart. Revise
dw_mci_ctrl_reset to dw_mci_reset, which has wrapped this sequence.

The patch fixes commit e9ed8835e990 ("mmc: dw_mmc: add runtime PM
callback"). MMC_PM_KEEP_POWEP is disabled for SD card and eMMC slots, so
that they have no chance to invoke dw_mci_setup_bus for update clock
behaviour. Let's consummate it.

Reported-by: Randy Li 
Signed-off-by: Ziyuan Xu 

---

Changes in v2:
- update the commit message
- use dw_mci_reset instead of dw_mci_ctrl_reset

 drivers/mmc/host/dw_mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index b44306b..ed63237 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -3324,7 +3324,7 @@ int dw_mci_runtime_resume(struct device *dev)
if (ret)
goto err;
 
-   if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS)) {
+   if (!dw_mci_reset(host)) {
clk_disable_unprepare(host->ciu_clk);
ret = -ENODEV;
goto err;
-- 
2.7.4

[PATCH] mmc: dw_mmc: Fix some coding style

[Ziyuan Xu]

Let's fix the warnings from checkpatch.pl:

- line over 80 characters;
- block comments should align the * on each Lines;
- statements not starting on a tabstop.

Signed-off-by: Ziyuan Xu <xzy...@rock-chips.com>
---

 drivers/mmc/host/dw_mmc.c | 33 +
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index ed63237..a8efe3f 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -94,7 +94,8 @@ struct idmac_desc {
 
__le32  des1;   /* Buffer sizes */
 #define IDMAC_SET_BUFFER1_SIZE(d, s) \
-   ((d)->des1 = ((d)->des1 & cpu_to_le32(0x03ffe000)) | (cpu_to_le32((s) & 
0x1fff)))
+   ((d)->des1 = ((d)->des1 & cpu_to_le32(0x03ffe000)) | \
+(cpu_to_le32((s) & 0x1fff)))
 
__le32  des2;   /* buffer 1 physical address */
 
@@ -2733,16 +2734,16 @@ static void dw_mci_init_dma(struct dw_mci *host)
struct device_node *np = dev->of_node;
 
/*
-   * Check tansfer mode from HCON[17:16]
-   * Clear the ambiguous description of dw_mmc databook:
-   * 2b'00: No DMA Interface -> Actually means using Internal DMA block
-   * 2b'01: DesignWare DMA Interface -> Synopsys DW-DMA block
-   * 2b'10: Generic DMA Interface -> non-Synopsys generic DMA block
-   * 2b'11: Non DW DMA Interface -> pio only
-   * Compared to DesignWare DMA Interface, Generic DMA Interface has a
-   * simpler request/acknowledge handshake mechanism and both of them
-   * are regarded as external dma master for dw_mmc.
-   */
+* Check tansfer mode from HCON[17:16]
+* Clear the ambiguous description of dw_mmc databook:
+* 2b'00: No DMA Interface -> Actually means using Internal DMA block
+* 2b'01: DesignWare DMA Interface -> Synopsys DW-DMA block
+* 2b'10: Generic DMA Interface -> non-Synopsys generic DMA block
+* 2b'11: Non DW DMA Interface -> pio only
+* Compared to DesignWare DMA Interface, Generic DMA Interface has a
+* simpler request/acknowledge handshake mechanism and both of them
+* are regarded as external dma master for dw_mmc.
+*/
host->use_dma = SDMMC_GET_TRANS_MODE(mci_readl(host, HCON));
if (host->use_dma == DMA_INTERFACE_IDMA) {
host->use_dma = TRANS_MODE_IDMAC;
@@ -2756,9 +2757,9 @@ static void dw_mci_init_dma(struct dw_mci *host)
/* Determine which DMA interface to use */
if (host->use_dma == TRANS_MODE_IDMAC) {
/*
-   * Check ADDR_CONFIG bit in HCON to find
-   * IDMAC address bus width
-   */
+* Check ADDR_CONFIG bit in HCON to find
+* IDMAC address bus width
+*/
addr_config = SDMMC_GET_ADDR_CONFIG(mci_readl(host, HCON));
 
if (addr_config == 1) {
@@ -3337,8 +3338,8 @@ int dw_mci_runtime_resume(struct device *dev)
 * Restore the initial value at FIFOTH register
 * And Invalidate the prev_blksz with zero
 */
-mci_writel(host, FIFOTH, host->fifoth_val);
-host->prev_blksz = 0;
+   mci_writel(host, FIFOTH, host->fifoth_val);
+   host->prev_blksz = 0;
 
/* Put in max timeout */
mci_writel(host, TMOUT, 0x);
-- 
2.7.4

[PATCH] mmc: dw_mmc: Fix some coding style

[Ziyuan Xu]

Let's fix the warnings from checkpatch.pl:

- line over 80 characters;
- block comments should align the * on each Lines;
- statements not starting on a tabstop.

Signed-off-by: Ziyuan Xu 
---

 drivers/mmc/host/dw_mmc.c | 33 +
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index ed63237..a8efe3f 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -94,7 +94,8 @@ struct idmac_desc {
 
__le32  des1;   /* Buffer sizes */
 #define IDMAC_SET_BUFFER1_SIZE(d, s) \
-   ((d)->des1 = ((d)->des1 & cpu_to_le32(0x03ffe000)) | (cpu_to_le32((s) & 
0x1fff)))
+   ((d)->des1 = ((d)->des1 & cpu_to_le32(0x03ffe000)) | \
+(cpu_to_le32((s) & 0x1fff)))
 
__le32  des2;   /* buffer 1 physical address */
 
@@ -2733,16 +2734,16 @@ static void dw_mci_init_dma(struct dw_mci *host)
struct device_node *np = dev->of_node;
 
/*
-   * Check tansfer mode from HCON[17:16]
-   * Clear the ambiguous description of dw_mmc databook:
-   * 2b'00: No DMA Interface -> Actually means using Internal DMA block
-   * 2b'01: DesignWare DMA Interface -> Synopsys DW-DMA block
-   * 2b'10: Generic DMA Interface -> non-Synopsys generic DMA block
-   * 2b'11: Non DW DMA Interface -> pio only
-   * Compared to DesignWare DMA Interface, Generic DMA Interface has a
-   * simpler request/acknowledge handshake mechanism and both of them
-   * are regarded as external dma master for dw_mmc.
-   */
+* Check tansfer mode from HCON[17:16]
+* Clear the ambiguous description of dw_mmc databook:
+* 2b'00: No DMA Interface -> Actually means using Internal DMA block
+* 2b'01: DesignWare DMA Interface -> Synopsys DW-DMA block
+* 2b'10: Generic DMA Interface -> non-Synopsys generic DMA block
+* 2b'11: Non DW DMA Interface -> pio only
+* Compared to DesignWare DMA Interface, Generic DMA Interface has a
+* simpler request/acknowledge handshake mechanism and both of them
+* are regarded as external dma master for dw_mmc.
+*/
host->use_dma = SDMMC_GET_TRANS_MODE(mci_readl(host, HCON));
if (host->use_dma == DMA_INTERFACE_IDMA) {
host->use_dma = TRANS_MODE_IDMAC;
@@ -2756,9 +2757,9 @@ static void dw_mci_init_dma(struct dw_mci *host)
/* Determine which DMA interface to use */
if (host->use_dma == TRANS_MODE_IDMAC) {
/*
-   * Check ADDR_CONFIG bit in HCON to find
-   * IDMAC address bus width
-   */
+* Check ADDR_CONFIG bit in HCON to find
+* IDMAC address bus width
+*/
addr_config = SDMMC_GET_ADDR_CONFIG(mci_readl(host, HCON));
 
if (addr_config == 1) {
@@ -3337,8 +3338,8 @@ int dw_mci_runtime_resume(struct device *dev)
 * Restore the initial value at FIFOTH register
 * And Invalidate the prev_blksz with zero
 */
-mci_writel(host, FIFOTH, host->fifoth_val);
-host->prev_blksz = 0;
+   mci_writel(host, FIFOTH, host->fifoth_val);
+   host->prev_blksz = 0;
 
/* Put in max timeout */
mci_writel(host, TMOUT, 0x);
-- 
2.7.4

Re: [PATCH 2/6] arm64: dts: rockchip: add pd_emmc power node for rk3399

[Ziyuan Xu]

hi Elaine,


On 2016年09月09日 15:40, Elaine Zhang wrote:

1.add pd node for RK3399 Soc
2.create power domain tree
3.add qos node for domain
4.add the pd_emmc consumers node

Signed-off-by: Elaine Zhang 


I had push this patch, see [0]

[0] https://patchwork.kernel.org/patch/9320423/


---
  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 52b4bfb52288..869b52d60b73 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -269,6 +269,7 @@
#clock-cells = <0>;
phys = <_phy>;
phy-names = "phy_arasan";
+   power-domains = < RK3399_PD_EMMC>;
status = "disabled";
};
  
@@ -690,6 +691,10 @@

status = "disabled";
};
  
+	qos_emmc: qos@ffa58000 {

+   compatible = "syscon";
+   reg = <0x0 0xffa58000 0x0 0x20>;
+   };
qos_gmac: qos@ffa5c000 {
compatible = "syscon";
reg = <0x0 0xffa5c000 0x0 0x20>;
@@ -827,6 +832,11 @@
reg = ;
clocks = < PCLK_EDP_CTRL>;
};
+   pd_emmc@RK3399_PD_EMMC {
+   reg = ;
+   clocks = < ACLK_EMMC>;
+   pm_qos = <_emmc>;
+   };
pd_gmac@RK3399_PD_GMAC {
reg = ;
clocks = < ACLK_GMAC>;

Re: [PATCH 2/6] arm64: dts: rockchip: add pd_emmc power node for rk3399

[Ziyuan Xu]

hi Elaine,


On 2016年09月09日 15:40, Elaine Zhang wrote:

1.add pd node for RK3399 Soc
2.create power domain tree
3.add qos node for domain
4.add the pd_emmc consumers node

Signed-off-by: Elaine Zhang 


I had push this patch, see [0]

[0] https://patchwork.kernel.org/patch/9320423/


---
  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 52b4bfb52288..869b52d60b73 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -269,6 +269,7 @@
#clock-cells = <0>;
phys = <_phy>;
phy-names = "phy_arasan";
+   power-domains = < RK3399_PD_EMMC>;
status = "disabled";
};
  
@@ -690,6 +691,10 @@

status = "disabled";
};
  
+	qos_emmc: qos@ffa58000 {

+   compatible = "syscon";
+   reg = <0x0 0xffa58000 0x0 0x20>;
+   };
qos_gmac: qos@ffa5c000 {
compatible = "syscon";
reg = <0x0 0xffa5c000 0x0 0x20>;
@@ -827,6 +832,11 @@
reg = ;
clocks = < PCLK_EDP_CTRL>;
};
+   pd_emmc@RK3399_PD_EMMC {
+   reg = ;
+   clocks = < ACLK_EMMC>;
+   pm_qos = <_emmc>;
+   };
pd_gmac@RK3399_PD_GMAC {
reg = ;
clocks = < ACLK_GMAC>;

Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Ziyuan Xu]

Hi Ulf,


On 2016年09月02日 18:24, Ulf Hansson wrote:

On 1 September 2016 at 23:50, Doug Anderson  wrote:

>Hi,
>
>On Thu, Sep 1, 2016 at 6:45 AM, Ulf Hansson  wrote:

>>I was reading the discussion regarding this change and browsing the DT
>>documentation around this... Can you guys explain what really goes on
>>here, please.
>>
>>To me, it seems like you are managing one device's resources in one
>>separate genpd. One genpd per device. Is that correct?
>>
>>Perhaps each device actually has its own PM domain and thus it makes
>>sense to assign one genpd per device?

>
>I'm not as familiar with genpd as I should be, so hopefully this makes sense.
>
>...in hardware there is a "pd_emmc" that is the power domain for just
>eMMC.  That will be referenced hooked up via device tree, like:
>
>power-domains = < RK3399_PD_EMMC>;

Yes, I noticed that and this is what puzzles me a bit.


>
>I believe that means that power will automatically be removed whenever
>the device is runtime suspended or suspended.

Well, it depends if the genpd has a subdomain or other devices in it
being runtime resumed.
The genpd will not power off unless all devices within it are runtime
enabled+suspended and that its subdomains are also powered off.

So, in case you only have one device and no subdomains, then your
statement is correct.


Yup, pd_emmc is a individual power domain which is only deployed to eMMC 
on rk3399. It has no subdomains.





>
>If w're not supporting "autosuspend" and nobody is tweaking anything

I guess you mean runtime PM autosuspend? Then why don't you support this?

Wouldn't that allow you to avoid wasting power in runtime when the
device is idle?


pd_emmc manages the sdhci controller, phy and corecfg_* stuff, if we 
support autosuspend in driver, we have to re-init context. I didn't test 
the latency, if it's acceptable, we will apply it.:-)

But it's not a blocker, right?




>manually, then it's possible (I think) that runtime suspend happens at
>exactly the same time as suspend.  ...but my point was that it was

I am not sure I follow you here. You must not rely on that the device
always becomes runtime suspended during system suspend, as there are
no guarantees for this.

Instead that is something you need to take care of in the
subsystem/driver for the device, of course.


>cleaner to actually do it any restoring in the "runtime resume" hooks
>to match what genpd does.  This matches what you say: use runtime PM.

Yes!

Using genpd without deploying runtime PM for the devices doesn't make
much sense, at least to me.


>
>...but it also sounds like it might not be terribly important to
>restore these values since they're a bit silly to begin with.  If
>that's true then I guess we don't need to do anything special here.
>
>
>Did that all make sense (it's entirely possible it didn't since
>somehow my brain still hasn't absorbed all runtime PM and genpd
>concepts)

No worries. I understand this might be a bit tricky, that's why I also
tries to help review related changes.

Kind regards
Uffe

Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Ziyuan Xu]

Hi Ulf,


On 2016年09月02日 18:24, Ulf Hansson wrote:

On 1 September 2016 at 23:50, Doug Anderson  wrote:

>Hi,
>
>On Thu, Sep 1, 2016 at 6:45 AM, Ulf Hansson  wrote:

>>I was reading the discussion regarding this change and browsing the DT
>>documentation around this... Can you guys explain what really goes on
>>here, please.
>>
>>To me, it seems like you are managing one device's resources in one
>>separate genpd. One genpd per device. Is that correct?
>>
>>Perhaps each device actually has its own PM domain and thus it makes
>>sense to assign one genpd per device?

>
>I'm not as familiar with genpd as I should be, so hopefully this makes sense.
>
>...in hardware there is a "pd_emmc" that is the power domain for just
>eMMC.  That will be referenced hooked up via device tree, like:
>
>power-domains = < RK3399_PD_EMMC>;

Yes, I noticed that and this is what puzzles me a bit.


>
>I believe that means that power will automatically be removed whenever
>the device is runtime suspended or suspended.

Well, it depends if the genpd has a subdomain or other devices in it
being runtime resumed.
The genpd will not power off unless all devices within it are runtime
enabled+suspended and that its subdomains are also powered off.

So, in case you only have one device and no subdomains, then your
statement is correct.


Yup, pd_emmc is a individual power domain which is only deployed to eMMC 
on rk3399. It has no subdomains.





>
>If w're not supporting "autosuspend" and nobody is tweaking anything

I guess you mean runtime PM autosuspend? Then why don't you support this?

Wouldn't that allow you to avoid wasting power in runtime when the
device is idle?


pd_emmc manages the sdhci controller, phy and corecfg_* stuff, if we 
support autosuspend in driver, we have to re-init context. I didn't test 
the latency, if it's acceptable, we will apply it.:-)

But it's not a blocker, right?




>manually, then it's possible (I think) that runtime suspend happens at
>exactly the same time as suspend.  ...but my point was that it was

I am not sure I follow you here. You must not rely on that the device
always becomes runtime suspended during system suspend, as there are
no guarantees for this.

Instead that is something you need to take care of in the
subsystem/driver for the device, of course.


>cleaner to actually do it any restoring in the "runtime resume" hooks
>to match what genpd does.  This matches what you say: use runtime PM.

Yes!

Using genpd without deploying runtime PM for the devices doesn't make
much sense, at least to me.


>
>...but it also sounds like it might not be terribly important to
>restore these values since they're a bit silly to begin with.  If
>that's true then I guess we don't need to do anything special here.
>
>
>Did that all make sense (it's entirely possible it didn't since
>somehow my brain still hasn't absorbed all runtime PM and genpd
>concepts)

No worries. I understand this might be a bit tricky, that's why I also
tries to help review related changes.

Kind regards
Uffe

Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Ziyuan Xu]

On 2016年09月02日 05:29, Doug Anderson wrote:

Hi,

On Wed, Aug 31, 2016 at 11:56 PM, Ziyuan Xu <xzy...@rock-chips.com> wrote:

Hi


On 2016年09月01日 12:20, Doug Anderson wrote:

Hi,

On Wed, Aug 31, 2016 at 7:29 PM, Ziyuan Xu <xzy...@rock-chips.com> wrote:

This is fine to pick up _only_ if you don't care about suspend/resume.
If you care about suspend/resume then someone needs to first write a
patch that will re-init all "corecfg" values after power is turned on.


Do you mean corecfg_clockmultiplier and corecfg_baseclkfreq, if yes, we
don't need to strore/re-init it after resume.
corecfg_clockmultiplier is only used to fetch host->clk_mul, and
host->clk_mul has been a fixed value at run-time, unless driver unbind.
The same as corecfg_clockmultiplier, corecfg_baseclkfreq is used to check
the xin_clk at probe time, we don't reference it at run-time.
BTW, I have tested suspend/resume on rk3399 prior to this sumbit, eMMC
works
fine.

I guess I don't actually know how the corecfg_clockmultiplier and
corecfg_baseclkfreq fields are actually used, but I presume that they
actually do something useful and aren't used to just communicate back
to software?


Take corecfg_clockmultiplier as example.
1. sdhci driver fetch host->clk_mul from corecfg_clockmultiplier
2. mmc->f_min and mmc->f_max are calculated via host->clk_mul, they're used
for further initialization.
3. if the corecfg_clockmultiplier is incorrect, sdhci will use improper
frequency to play.

I think we don't need to store it due to it's a fixed value at run-time,
even if it is reset after a power cycle, the above will not be changed via
software, except for dirver unbind .


I know that:

1. If I don't pick this patch and I suspend/resume,
corecfg_clockmultiplier and corecfg_baseclkfreq are still fine after
suspend / resume.

2. If I do pick this patch and I suspend/resume,
corecfg_clockmultiplier and corecfg_baseclkfreq are wrong after
suspend/resume (tested by reading /dev/mem directly from userspace
after suspend/resume).


Are you saying that it is unimportant that corecfg_clockmultiplier and
corecfg_baseclkfreq are wrong?


Yup, corecfg_* stuff will be reset after a power cycle.
I mean that we need only to guarantee they're correct at probe time.

So are you saying that the entire purpose of "corecfg_clockmultiplier"
is that causes the "ClockMultiplier" field of the "EMMCCORE_CAP"
register to get a certain value?
...and that the entire purpose of "corecfg_baseclkfreq" is that it
causes the "BaseClockFreqSDClock" field of the "EMMCCORE_CAP" register
to get a certain value?

Yes, on rk3399:
corecfg_clockmultiplier <===> EMMCCORE_CAP1[23:16] ClockMultiplier
corecfg_baseclkfreq <===> EMMCCORE_CAP[15:8] BaseClockFreqSDClock

If you re-write to either corecfg_* stuff,  the corresponding CAP 
register field will be changed too.
sdhci driver will fetch CAP register for initialization, we only need to 
guarantee they're correct at probe time.


Did that all make sense?


That would have been nice to know before.  I had assumed that those
"corecfg" settings did something else more useful.

If it is indeed true that these corecfg values are as silly as it
seems, then I guess it's not terribly important to re-set them after
suspend/resume.  Eventually it would be nice/clean to actually do so
(in case the SDHCI driver eventually changes), but I guess we wouldn't
need to block. this patch from landing.

Can you please confirm my understanding above?


-Doug

___
Linux-rockchip mailing list
linux-rockc...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Ziyuan Xu]

On 2016年09月02日 05:29, Doug Anderson wrote:

Hi,

On Wed, Aug 31, 2016 at 11:56 PM, Ziyuan Xu  wrote:

Hi


On 2016年09月01日 12:20, Doug Anderson wrote:

Hi,

On Wed, Aug 31, 2016 at 7:29 PM, Ziyuan Xu  wrote:

This is fine to pick up _only_ if you don't care about suspend/resume.
If you care about suspend/resume then someone needs to first write a
patch that will re-init all "corecfg" values after power is turned on.


Do you mean corecfg_clockmultiplier and corecfg_baseclkfreq, if yes, we
don't need to strore/re-init it after resume.
corecfg_clockmultiplier is only used to fetch host->clk_mul, and
host->clk_mul has been a fixed value at run-time, unless driver unbind.
The same as corecfg_clockmultiplier, corecfg_baseclkfreq is used to check
the xin_clk at probe time, we don't reference it at run-time.
BTW, I have tested suspend/resume on rk3399 prior to this sumbit, eMMC
works
fine.

I guess I don't actually know how the corecfg_clockmultiplier and
corecfg_baseclkfreq fields are actually used, but I presume that they
actually do something useful and aren't used to just communicate back
to software?


Take corecfg_clockmultiplier as example.
1. sdhci driver fetch host->clk_mul from corecfg_clockmultiplier
2. mmc->f_min and mmc->f_max are calculated via host->clk_mul, they're used
for further initialization.
3. if the corecfg_clockmultiplier is incorrect, sdhci will use improper
frequency to play.

I think we don't need to store it due to it's a fixed value at run-time,
even if it is reset after a power cycle, the above will not be changed via
software, except for dirver unbind .


I know that:

1. If I don't pick this patch and I suspend/resume,
corecfg_clockmultiplier and corecfg_baseclkfreq are still fine after
suspend / resume.

2. If I do pick this patch and I suspend/resume,
corecfg_clockmultiplier and corecfg_baseclkfreq are wrong after
suspend/resume (tested by reading /dev/mem directly from userspace
after suspend/resume).


Are you saying that it is unimportant that corecfg_clockmultiplier and
corecfg_baseclkfreq are wrong?


Yup, corecfg_* stuff will be reset after a power cycle.
I mean that we need only to guarantee they're correct at probe time.

So are you saying that the entire purpose of "corecfg_clockmultiplier"
is that causes the "ClockMultiplier" field of the "EMMCCORE_CAP"
register to get a certain value?
...and that the entire purpose of "corecfg_baseclkfreq" is that it
causes the "BaseClockFreqSDClock" field of the "EMMCCORE_CAP" register
to get a certain value?

Yes, on rk3399:
corecfg_clockmultiplier <===> EMMCCORE_CAP1[23:16] ClockMultiplier
corecfg_baseclkfreq <===> EMMCCORE_CAP[15:8] BaseClockFreqSDClock

If you re-write to either corecfg_* stuff,  the corresponding CAP 
register field will be changed too.
sdhci driver will fetch CAP register for initialization, we only need to 
guarantee they're correct at probe time.


Did that all make sense?


That would have been nice to know before.  I had assumed that those
"corecfg" settings did something else more useful.

If it is indeed true that these corecfg values are as silly as it
seems, then I guess it's not terribly important to re-set them after
suspend/resume.  Eventually it would be nice/clean to actually do so
(in case the SDHCI driver eventually changes), but I guess we wouldn't
need to block. this patch from landing.

Can you please confirm my understanding above?


-Doug

___
Linux-rockchip mailing list
linux-rockc...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Ziyuan Xu]

Hi


On 2016年09月01日 12:20, Doug Anderson wrote:

Hi,

On Wed, Aug 31, 2016 at 7:29 PM, Ziyuan Xu <xzy...@rock-chips.com> wrote:

This is fine to pick up _only_ if you don't care about suspend/resume.
If you care about suspend/resume then someone needs to first write a
patch that will re-init all "corecfg" values after power is turned on.


Do you mean corecfg_clockmultiplier and corecfg_baseclkfreq, if yes, we
don't need to strore/re-init it after resume.
corecfg_clockmultiplier is only used to fetch host->clk_mul, and
host->clk_mul has been a fixed value at run-time, unless driver unbind.
The same as corecfg_clockmultiplier, corecfg_baseclkfreq is used to check
the xin_clk at probe time, we don't reference it at run-time.
BTW, I have tested suspend/resume on rk3399 prior to this sumbit, eMMC works
fine.

I guess I don't actually know how the corecfg_clockmultiplier and
corecfg_baseclkfreq fields are actually used, but I presume that they
actually do something useful and aren't used to just communicate back
to software?


Take corecfg_clockmultiplier as example.
1. sdhci driver fetch host->clk_mul from corecfg_clockmultiplier
2. mmc->f_min and mmc->f_max are calculated via host->clk_mul, they're 
used for further initialization.
3. if the corecfg_clockmultiplier is incorrect, sdhci will use improper 
frequency to play.


I think we don't need to store it due to it's a fixed value at run-time, 
even if it is reset after a power cycle, the above will not be changed 
via software, except for dirver unbind .




I know that:

1. If I don't pick this patch and I suspend/resume,
corecfg_clockmultiplier and corecfg_baseclkfreq are still fine after
suspend / resume.

2. If I do pick this patch and I suspend/resume,
corecfg_clockmultiplier and corecfg_baseclkfreq are wrong after
suspend/resume (tested by reading /dev/mem directly from userspace
after suspend/resume).


Are you saying that it is unimportant that corecfg_clockmultiplier and
corecfg_baseclkfreq are wrong?


Yup, corecfg_* stuff will be reset after a power cycle.
I mean that we need only to guarantee they're correct at probe time.




Technically I think this should probably use "pm runtime" and not
normal suspend/resume hooks.  Any time we end up pm runtime suspended
then I think our power will go off (because of genpd?) and we need to
restore values.


I understand your consideration. BUT genpd is in charge of on/off pd if the
corresponding device node has "power-domains" property. RPM is unnecessary
for this situation, we will not use autosuspend, right?

@shawn, what's your opinion?

I haven't dug.  If Runtime PM isn't enabled for sdhci-of-arasan then I
guess we can just worry about suspend/resume, though.

-Doug

Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Ziyuan Xu]

Hi


On 2016年09月01日 12:20, Doug Anderson wrote:

Hi,

On Wed, Aug 31, 2016 at 7:29 PM, Ziyuan Xu  wrote:

This is fine to pick up _only_ if you don't care about suspend/resume.
If you care about suspend/resume then someone needs to first write a
patch that will re-init all "corecfg" values after power is turned on.


Do you mean corecfg_clockmultiplier and corecfg_baseclkfreq, if yes, we
don't need to strore/re-init it after resume.
corecfg_clockmultiplier is only used to fetch host->clk_mul, and
host->clk_mul has been a fixed value at run-time, unless driver unbind.
The same as corecfg_clockmultiplier, corecfg_baseclkfreq is used to check
the xin_clk at probe time, we don't reference it at run-time.
BTW, I have tested suspend/resume on rk3399 prior to this sumbit, eMMC works
fine.

I guess I don't actually know how the corecfg_clockmultiplier and
corecfg_baseclkfreq fields are actually used, but I presume that they
actually do something useful and aren't used to just communicate back
to software?


Take corecfg_clockmultiplier as example.
1. sdhci driver fetch host->clk_mul from corecfg_clockmultiplier
2. mmc->f_min and mmc->f_max are calculated via host->clk_mul, they're 
used for further initialization.
3. if the corecfg_clockmultiplier is incorrect, sdhci will use improper 
frequency to play.


I think we don't need to store it due to it's a fixed value at run-time, 
even if it is reset after a power cycle, the above will not be changed 
via software, except for dirver unbind .




I know that:

1. If I don't pick this patch and I suspend/resume,
corecfg_clockmultiplier and corecfg_baseclkfreq are still fine after
suspend / resume.

2. If I do pick this patch and I suspend/resume,
corecfg_clockmultiplier and corecfg_baseclkfreq are wrong after
suspend/resume (tested by reading /dev/mem directly from userspace
after suspend/resume).


Are you saying that it is unimportant that corecfg_clockmultiplier and
corecfg_baseclkfreq are wrong?


Yup, corecfg_* stuff will be reset after a power cycle.
I mean that we need only to guarantee they're correct at probe time.




Technically I think this should probably use "pm runtime" and not
normal suspend/resume hooks.  Any time we end up pm runtime suspended
then I think our power will go off (because of genpd?) and we need to
restore values.


I understand your consideration. BUT genpd is in charge of on/off pd if the
corresponding device node has "power-domains" property. RPM is unnecessary
for this situation, we will not use autosuspend, right?

@shawn, what's your opinion?

I haven't dug.  If Runtime PM isn't enabled for sdhci-of-arasan then I
guess we can just worry about suspend/resume, though.

-Doug

Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Ziyuan Xu]

Hi,


On 2016年09月01日 01:42, Doug Anderson wrote:

Hi,

On Sun, Aug 28, 2016 at 8:25 PM, Shawn Lin <shawn@rock-chips.com> wrote:

On 2016/8/29 10:50, Elaine Zhang wrote:



On 08/27/2016 11:05 PM, Shawn Lin wrote:

On 2016/8/27 21:41, Ziyuan Xu wrote:

Control power domain for eMMC via genpd to reduce power consumption.

Signed-off-by: Elaine Zhang <zhangq...@rock-chips.com>
Signed-off-by: Ziyuan Xu <xzy...@rock-chips.com>


It looks nice to me. But this should be merged after applying that[0]
as your patch will break bind/unbind test for sdhci-of-arasan on rk3399
without it[0]. Moreover, Elaine should make sure that upstreamed
rockchip power domain stuff would not off pd for emmc, *otherwise*, I
should update my patch to make sure we update clkmul every time when
doing suspend 2 resume..



Forgot to say:
If use pd, Although there is no call to power odd the pd_emmc,
it will be power off when the system doing suspend 2 resume.
(Because the system call
__device_suspend_noirq->pm_genpd_suspend_noirq->rockchip_pd_power_off)


Thanks for explaining this. I checked the code a bit and actually I
don't need to updata clkmul since it was recorded, although it is still
reset to 0x10 reading from syscon. So for that, we can now pick it
up without waiting for my sdhci-of-arasan's update.

Reviewed-by: Shawn Lin <shawn@rock-chips.com>

This is fine to pick up _only_ if you don't care about suspend/resume.
If you care about suspend/resume then someone needs to first write a
patch that will re-init all "corecfg" values after power is turned on.


Do you mean corecfg_clockmultiplier and corecfg_baseclkfreq, if yes, we 
don't need to strore/re-init it after resume.
corecfg_clockmultiplier is only used to fetch host->clk_mul, and 
host->clk_mul has been a fixed value at run-time, unless driver unbind.
The same as corecfg_clockmultiplier, corecfg_baseclkfreq is used to 
check the xin_clk at probe time, we don't reference it at run-time.
BTW, I have tested suspend/resume on rk3399 prior to this sumbit, eMMC 
works fine.




Technically I think this should probably use "pm runtime" and not
normal suspend/resume hooks.  Any time we end up pm runtime suspended
then I think our power will go off (because of genpd?) and we need to
restore values.


I understand your consideration. BUT genpd is in charge of on/off pd if 
the corresponding device node has "power-domains" property. RPM is 
unnecessary for this situation, we will not use autosuspend, right?


@shawn, what's your opinion?



I'm not sure if this should be done in a generic way where we try to
save and restore all values in the "sdhci_arasan_soc_ctl_map" or if we
should try to be smarter...


-Doug

___
Linux-rockchip mailing list
linux-rockc...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Ziyuan Xu]

Hi,


On 2016年09月01日 01:42, Doug Anderson wrote:

Hi,

On Sun, Aug 28, 2016 at 8:25 PM, Shawn Lin  wrote:

On 2016/8/29 10:50, Elaine Zhang wrote:



On 08/27/2016 11:05 PM, Shawn Lin wrote:

On 2016/8/27 21:41, Ziyuan Xu wrote:

Control power domain for eMMC via genpd to reduce power consumption.

Signed-off-by: Elaine Zhang 
Signed-off-by: Ziyuan Xu 


It looks nice to me. But this should be merged after applying that[0]
as your patch will break bind/unbind test for sdhci-of-arasan on rk3399
without it[0]. Moreover, Elaine should make sure that upstreamed
rockchip power domain stuff would not off pd for emmc, *otherwise*, I
should update my patch to make sure we update clkmul every time when
doing suspend 2 resume..



Forgot to say:
If use pd, Although there is no call to power odd the pd_emmc,
it will be power off when the system doing suspend 2 resume.
(Because the system call
__device_suspend_noirq->pm_genpd_suspend_noirq->rockchip_pd_power_off)


Thanks for explaining this. I checked the code a bit and actually I
don't need to updata clkmul since it was recorded, although it is still
reset to 0x10 reading from syscon. So for that, we can now pick it
up without waiting for my sdhci-of-arasan's update.

Reviewed-by: Shawn Lin 

This is fine to pick up _only_ if you don't care about suspend/resume.
If you care about suspend/resume then someone needs to first write a
patch that will re-init all "corecfg" values after power is turned on.


Do you mean corecfg_clockmultiplier and corecfg_baseclkfreq, if yes, we 
don't need to strore/re-init it after resume.
corecfg_clockmultiplier is only used to fetch host->clk_mul, and 
host->clk_mul has been a fixed value at run-time, unless driver unbind.
The same as corecfg_clockmultiplier, corecfg_baseclkfreq is used to 
check the xin_clk at probe time, we don't reference it at run-time.
BTW, I have tested suspend/resume on rk3399 prior to this sumbit, eMMC 
works fine.




Technically I think this should probably use "pm runtime" and not
normal suspend/resume hooks.  Any time we end up pm runtime suspended
then I think our power will go off (because of genpd?) and we need to
restore values.


I understand your consideration. BUT genpd is in charge of on/off pd if 
the corresponding device node has "power-domains" property. RPM is 
unnecessary for this situation, we will not use autosuspend, right?


@shawn, what's your opinion?



I'm not sure if this should be done in a generic way where we try to
save and restore all values in the "sdhci_arasan_soc_ctl_map" or if we
should try to be smarter...


-Doug

___
Linux-rockchip mailing list
linux-rockc...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v2 2/4] mmc: sdhci-of-arasan: Control clock for accessing syscon

[Ziyuan Xu]

Hi Shawn,


On 2016年08月31日 09:37, Shawn Lin wrote:

In the eariler commit 65820199272d ("Documentation: mmc:
sdhci-of-arasan: Add soc-ctl-syscon for corecfg regs"), we
introduced syscon to control corecfg_* stuff provided by
arasan. But given that we may need to ungate the clock for
accessing corecfg_*, it not so perfect as it depends on
whether specific clock driver disables it if not referenced.
Meanwhile, if we don't need arasan contoller to work anymore,
there is no reason to still enable it. So let's control this
clock when needed.

Signed-off-by: Shawn Lin <shawn@rock-chips.com>

---


Without  this series of patches, we can't gate aclk_emmcgrf even though 
driver enter suspend/remove. It meaningful to save power consumption.

It looks nice to me, and also I have tested on rk3399 board, so

Tested-by: Ziyuan Xu <xzy...@rock-chips.com>



Changes in v2:
- assign NULL to clk_syscon if it's not deferral error.

  drivers/mmc/host/sdhci-of-arasan.c | 33 ++---
  1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c 
b/drivers/mmc/host/sdhci-of-arasan.c
index 0b3a9cf..3169f81 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -78,6 +78,7 @@ struct sdhci_arasan_soc_ctl_map {
   * struct sdhci_arasan_data
   * @host: Pointer to the main SDHCI host structure.
   * @clk_ahb:  Pointer to the AHB clock
+ * @clk_syscon:Pointer to the optional clock for accessing 
syscon
   * @phy:  Pointer to the generic phy
   * @is_phy_on:True if the PHY is on; false if not.
   * @sdcardclk_hw: Struct for the clock we might provide to a PHY.
@@ -88,6 +89,7 @@ struct sdhci_arasan_soc_ctl_map {
  struct sdhci_arasan_data {
struct sdhci_host *host;
struct clk  *clk_ahb;
+   struct clk  *clk_syscon;
struct phy  *phy;
boolis_phy_on;
  
@@ -290,6 +292,7 @@ static int sdhci_arasan_suspend(struct device *dev)
  
  	clk_disable(pltfm_host->clk);

clk_disable(sdhci_arasan->clk_ahb);
+   clk_disable(sdhci_arasan->clk_syscon);
  
  	return 0;

  }
@@ -309,6 +312,12 @@ static int sdhci_arasan_resume(struct device *dev)
struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
int ret;
  
+	ret = clk_enable(sdhci_arasan->clk_syscon);

+   if (ret) {
+   dev_err(dev, "Cannot enable syscon clock.\n");
+   return ret;
+   }
+
ret = clk_enable(sdhci_arasan->clk_ahb);
if (ret) {
dev_err(dev, "Cannot enable AHB clock.\n");
@@ -528,26 +537,41 @@ static int sdhci_arasan_probe(struct platform_device 
*pdev)
ret);
goto err_pltfm_free;
}
+
+   sdhci_arasan->clk_syscon = devm_clk_get(>dev,
+   "clk_syscon");
+   if (IS_ERR(sdhci_arasan->clk_syscon)) {
+   ret = PTR_ERR(sdhci_arasan->clk_syscon);
+   if (ret == -EPROBE_DEFER)
+   goto err_pltfm_free;
+   else
+   sdhci_arasan->clk_syscon = NULL;
+   }
+
+   if (clk_prepare_enable(sdhci_arasan->clk_syscon)) {
+   dev_err(>dev, "Unable to enable syscon clock.\n");
+   goto err_pltfm_free;
+   }
}
  
  	sdhci_arasan->clk_ahb = devm_clk_get(>dev, "clk_ahb");

if (IS_ERR(sdhci_arasan->clk_ahb)) {
dev_err(>dev, "clk_ahb clock not found.\n");
ret = PTR_ERR(sdhci_arasan->clk_ahb);
-   goto err_pltfm_free;
+   goto clk_dis_syscon;
}
  
  	clk_xin = devm_clk_get(>dev, "clk_xin");

if (IS_ERR(clk_xin)) {
dev_err(>dev, "clk_xin clock not found.\n");
ret = PTR_ERR(clk_xin);
-   goto err_pltfm_free;
+   goto clk_dis_syscon;
}
  
  	ret = clk_prepare_enable(sdhci_arasan->clk_ahb);

if (ret) {
dev_err(>dev, "Unable to enable AHB clock.\n");
-   goto err_pltfm_free;
+   goto clk_dis_syscon;
}
  
  	ret = clk_prepare_enable(clk_xin);

@@ -607,6 +631,8 @@ clk_disable_all:
clk_disable_unprepare(clk_xin);
  clk_dis_ahb:
clk_disable_unprepare(sdhci_arasan->clk_ahb);
+clk_dis_syscon:
+   clk_disable_unprepare(sdhci_arasan->clk_syscon);
  err_pltfm_free:
sdhci_pltfm_free(pdev);
return ret;
@@ -631,6 +657,7 @@ static int sdhci_arasan_remove(struct platform_device *pdev)
ret = sdhci_pltfm_unregister(pdev);
  
  	clk_disable_unprepare(clk_ahb);

+   clk_disable_unprepare(sdhci_arasan->clk_syscon);
  
  	return ret;

  }

Re: [PATCH v2 3/4] arm64: dts: rockchip: add clk_syscon for sdhci on rk3399

[Ziyuan Xu]

Hi Shawn,


On 2016年08月31日 09:37, Shawn Lin wrote:

We are intent on letting the sdhci variant driver handle this
optional clock on rk3399 platform now.

Signed-off-by: Shawn Lin <shawn@rock-chips.com>
---


Thanks for your patch, we can gate aclk_emmcgrf now as soon as sdhci 
driver suspend.


Reviewed-by: Ziyuan Xu <xzy...@rock-chips.com>
Tested-by: Ziyuan Xu <xzy...@rock-chips.com>



Changes in v2: None

  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index bc86e8c..d26c6ad 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -233,8 +233,9 @@
arasan,soc-ctl-syscon = <>;
assigned-clocks = < SCLK_EMMC>;
assigned-clock-rates = <2>;
-   clocks = < SCLK_EMMC>, < ACLK_EMMC>;
-   clock-names = "clk_xin", "clk_ahb";
+   clocks = < SCLK_EMMC>, < ACLK_EMMC>,
+< ACLK_EMMC_GRF>;
+   clock-names = "clk_xin", "clk_ahb", "clk_syscon";
clock-output-names = "emmc_cardclock";
#clock-cells = <0>;
phys = <_phy>;

Re: [PATCH v2 2/4] mmc: sdhci-of-arasan: Control clock for accessing syscon

[Ziyuan Xu]

Hi Shawn,


On 2016年08月31日 09:37, Shawn Lin wrote:

In the eariler commit 65820199272d ("Documentation: mmc:
sdhci-of-arasan: Add soc-ctl-syscon for corecfg regs"), we
introduced syscon to control corecfg_* stuff provided by
arasan. But given that we may need to ungate the clock for
accessing corecfg_*, it not so perfect as it depends on
whether specific clock driver disables it if not referenced.
Meanwhile, if we don't need arasan contoller to work anymore,
there is no reason to still enable it. So let's control this
clock when needed.

Signed-off-by: Shawn Lin 

---


Without  this series of patches, we can't gate aclk_emmcgrf even though 
driver enter suspend/remove. It meaningful to save power consumption.

It looks nice to me, and also I have tested on rk3399 board, so

Tested-by: Ziyuan Xu 



Changes in v2:
- assign NULL to clk_syscon if it's not deferral error.

  drivers/mmc/host/sdhci-of-arasan.c | 33 ++---
  1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c 
b/drivers/mmc/host/sdhci-of-arasan.c
index 0b3a9cf..3169f81 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -78,6 +78,7 @@ struct sdhci_arasan_soc_ctl_map {
   * struct sdhci_arasan_data
   * @host: Pointer to the main SDHCI host structure.
   * @clk_ahb:  Pointer to the AHB clock
+ * @clk_syscon:Pointer to the optional clock for accessing 
syscon
   * @phy:  Pointer to the generic phy
   * @is_phy_on:True if the PHY is on; false if not.
   * @sdcardclk_hw: Struct for the clock we might provide to a PHY.
@@ -88,6 +89,7 @@ struct sdhci_arasan_soc_ctl_map {
  struct sdhci_arasan_data {
struct sdhci_host *host;
struct clk  *clk_ahb;
+   struct clk  *clk_syscon;
struct phy  *phy;
boolis_phy_on;
  
@@ -290,6 +292,7 @@ static int sdhci_arasan_suspend(struct device *dev)
  
  	clk_disable(pltfm_host->clk);

clk_disable(sdhci_arasan->clk_ahb);
+   clk_disable(sdhci_arasan->clk_syscon);
  
  	return 0;

  }
@@ -309,6 +312,12 @@ static int sdhci_arasan_resume(struct device *dev)
struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
int ret;
  
+	ret = clk_enable(sdhci_arasan->clk_syscon);

+   if (ret) {
+   dev_err(dev, "Cannot enable syscon clock.\n");
+   return ret;
+   }
+
ret = clk_enable(sdhci_arasan->clk_ahb);
if (ret) {
dev_err(dev, "Cannot enable AHB clock.\n");
@@ -528,26 +537,41 @@ static int sdhci_arasan_probe(struct platform_device 
*pdev)
ret);
goto err_pltfm_free;
}
+
+   sdhci_arasan->clk_syscon = devm_clk_get(>dev,
+   "clk_syscon");
+   if (IS_ERR(sdhci_arasan->clk_syscon)) {
+   ret = PTR_ERR(sdhci_arasan->clk_syscon);
+   if (ret == -EPROBE_DEFER)
+   goto err_pltfm_free;
+   else
+   sdhci_arasan->clk_syscon = NULL;
+   }
+
+   if (clk_prepare_enable(sdhci_arasan->clk_syscon)) {
+   dev_err(>dev, "Unable to enable syscon clock.\n");
+   goto err_pltfm_free;
+   }
}
  
  	sdhci_arasan->clk_ahb = devm_clk_get(>dev, "clk_ahb");

if (IS_ERR(sdhci_arasan->clk_ahb)) {
dev_err(>dev, "clk_ahb clock not found.\n");
ret = PTR_ERR(sdhci_arasan->clk_ahb);
-   goto err_pltfm_free;
+   goto clk_dis_syscon;
}
  
  	clk_xin = devm_clk_get(>dev, "clk_xin");

if (IS_ERR(clk_xin)) {
dev_err(>dev, "clk_xin clock not found.\n");
ret = PTR_ERR(clk_xin);
-   goto err_pltfm_free;
+   goto clk_dis_syscon;
}
  
  	ret = clk_prepare_enable(sdhci_arasan->clk_ahb);

if (ret) {
dev_err(>dev, "Unable to enable AHB clock.\n");
-   goto err_pltfm_free;
+   goto clk_dis_syscon;
}
  
  	ret = clk_prepare_enable(clk_xin);

@@ -607,6 +631,8 @@ clk_disable_all:
clk_disable_unprepare(clk_xin);
  clk_dis_ahb:
clk_disable_unprepare(sdhci_arasan->clk_ahb);
+clk_dis_syscon:
+   clk_disable_unprepare(sdhci_arasan->clk_syscon);
  err_pltfm_free:
sdhci_pltfm_free(pdev);
return ret;
@@ -631,6 +657,7 @@ static int sdhci_arasan_remove(struct platform_device *pdev)
ret = sdhci_pltfm_unregister(pdev);
  
  	clk_disable_unprepare(clk_ahb);

+   clk_disable_unprepare(sdhci_arasan->clk_syscon);
  
  	return ret;

  }

Re: [PATCH v2 3/4] arm64: dts: rockchip: add clk_syscon for sdhci on rk3399

[Ziyuan Xu]

Hi Shawn,


On 2016年08月31日 09:37, Shawn Lin wrote:

We are intent on letting the sdhci variant driver handle this
optional clock on rk3399 platform now.

Signed-off-by: Shawn Lin 
---


Thanks for your patch, we can gate aclk_emmcgrf now as soon as sdhci 
driver suspend.


Reviewed-by: Ziyuan Xu 
Tested-by: Ziyuan Xu 



Changes in v2: None

  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index bc86e8c..d26c6ad 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -233,8 +233,9 @@
arasan,soc-ctl-syscon = <>;
assigned-clocks = < SCLK_EMMC>;
assigned-clock-rates = <2>;
-   clocks = < SCLK_EMMC>, < ACLK_EMMC>;
-   clock-names = "clk_xin", "clk_ahb";
+   clocks = < SCLK_EMMC>, < ACLK_EMMC>,
+< ACLK_EMMC_GRF>;
+   clock-names = "clk_xin", "clk_ahb", "clk_syscon";
clock-output-names = "emmc_cardclock";
#clock-cells = <0>;
phys = <_phy>;

Re: [PATCH v2 4/4] clk: rockchip: remove CLK_IGNORE_UNUSED flag for aclk_emmc_grf on rk3399

[Ziyuan Xu]

On 2016年08月31日 09:37, Shawn Lin wrote:

aclk_emmc_grf is used for accessing corecfg_* of emmc stuff within
GRF block. We don't need to add CLK_IGNORE_UNUSED for it now as the
emmc driver will enable/disable it explicitly when needed.

Signed-off-by: Shawn Lin <shawn@rock-chips.com>


This patchset test result:

localhost driver # cat /sys/kernel/debug/clk/clk_summary | grep emmc
  gpll_aclk_emmc_src  11 
59400  0 0
 aclk_emmc35 
19800  0 0
aclk_emmcgrf  12 
19800  0 0
aclk_emmc_noc 11 
19800  0 0
aclk_emmccore 00 
19800  0 0
  clk_emmc12 
14850  0 0
 emmc_cardclock   00 
14850  0 0
  cpll_aclk_emmc_src  00 
8  0 0


After unbind driver.
localhost driver # cat /sys/kernel/debug/clk/clk_summary | grep emmc
  gpll_aclk_emmc_src  11 
59400  0 0
 aclk_emmc12 
19800  0 0
aclk_emmcgrf  00 
19800  0 0
aclk_emmc_noc 11 
19800  0 0
aclk_emmccore 00 
19800  0 0
  clk_emmc00 
14850  0 0
  cpll_aclk_emmc_src  00 
8  0 0


And aclk_emmcgrf(Bit 10, when high, enable clock) had gate.
localhost driver # mem r 0xff760380
0x0415

It works sane, so

Tested-by: Ziyuan Xu <xzy...@rock-chips.com>


---

Changes in v2: None

  drivers/clk/rockchip/clk-rk3399.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/rockchip/clk-rk3399.c 
b/drivers/clk/rockchip/clk-rk3399.c
index ede6c47..908f684 100644
--- a/drivers/clk/rockchip/clk-rk3399.c
+++ b/drivers/clk/rockchip/clk-rk3399.c
@@ -934,7 +934,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] 
__initdata = {
RK3399_CLKGATE_CON(32), 8, GFLAGS),
GATE(ACLK_EMMC_NOC, "aclk_emmc_noc", "aclk_emmc", CLK_IGNORE_UNUSED,
RK3399_CLKGATE_CON(32), 9, GFLAGS),
-   GATE(ACLK_EMMC_GRF, "aclk_emmcgrf", "aclk_emmc", CLK_IGNORE_UNUSED,
+   GATE(ACLK_EMMC_GRF, "aclk_emmcgrf", "aclk_emmc", 0,
RK3399_CLKGATE_CON(32), 10, GFLAGS),
  
  	/* perilp0 */

Re: [PATCH v2 4/4] clk: rockchip: remove CLK_IGNORE_UNUSED flag for aclk_emmc_grf on rk3399

[Ziyuan Xu]

On 2016年08月31日 09:37, Shawn Lin wrote:

aclk_emmc_grf is used for accessing corecfg_* of emmc stuff within
GRF block. We don't need to add CLK_IGNORE_UNUSED for it now as the
emmc driver will enable/disable it explicitly when needed.

Signed-off-by: Shawn Lin 


This patchset test result:

localhost driver # cat /sys/kernel/debug/clk/clk_summary | grep emmc
  gpll_aclk_emmc_src  11 
59400  0 0
 aclk_emmc35 
19800  0 0
aclk_emmcgrf  12 
19800  0 0
aclk_emmc_noc 11 
19800  0 0
aclk_emmccore 00 
19800  0 0
  clk_emmc12 
14850  0 0
 emmc_cardclock   00 
14850  0 0
  cpll_aclk_emmc_src  00 
8  0 0


After unbind driver.
localhost driver # cat /sys/kernel/debug/clk/clk_summary | grep emmc
  gpll_aclk_emmc_src  11 
59400  0 0
 aclk_emmc12 
19800  0 0
aclk_emmcgrf  00 
19800  0 0
aclk_emmc_noc 11 
19800  0 0
aclk_emmccore 00 
19800  0 0
  clk_emmc00 
14850  0 0
  cpll_aclk_emmc_src  00 
8  0 0


And aclk_emmcgrf(Bit 10, when high, enable clock) had gate.
localhost driver # mem r 0xff760380
0x0415

It works sane, so

Tested-by: Ziyuan Xu 


---

Changes in v2: None

  drivers/clk/rockchip/clk-rk3399.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/rockchip/clk-rk3399.c 
b/drivers/clk/rockchip/clk-rk3399.c
index ede6c47..908f684 100644
--- a/drivers/clk/rockchip/clk-rk3399.c
+++ b/drivers/clk/rockchip/clk-rk3399.c
@@ -934,7 +934,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] 
__initdata = {
RK3399_CLKGATE_CON(32), 8, GFLAGS),
GATE(ACLK_EMMC_NOC, "aclk_emmc_noc", "aclk_emmc", CLK_IGNORE_UNUSED,
RK3399_CLKGATE_CON(32), 9, GFLAGS),
-   GATE(ACLK_EMMC_GRF, "aclk_emmcgrf", "aclk_emmc", CLK_IGNORE_UNUSED,
+   GATE(ACLK_EMMC_GRF, "aclk_emmcgrf", "aclk_emmc", 0,
RK3399_CLKGATE_CON(32), 10, GFLAGS),
  
  	/* perilp0 */

Re: [PATCH 1/2] Documentation: mmc: sdhci-of-arasan: add description of power domain

[Ziyuan Xu]

On 2016年08月27日 22:50, Shawn Lin wrote:

On 2016/8/27 21:41, Ziyuan Xu wrote:

Add power domain as a optional property for sdhci-of-arasan, which can
be truned off in the so-called unused condition, such as suspend and
remove. Aim to lower power requirements.

Signed-off-by: Ziyuan Xu <xzy...@rock-chips.com>

---

 Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt 
b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt

index 3404afa..3f5189c 100644
--- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
@@ -1,12 +1,14 @@
 Device Tree Bindings for the Arasan SDHCI Controller

-  The bindings follow the mmc[1], clock[2], interrupt[3] and phy[4] 
bindings.
+  The bindings follow the mmc[1], clock[2], interrupt[3], phy[4] and 
power

+  domain[5] bindings.
   Only deviations are documented here.

   [1] Documentation/devicetree/bindings/mmc/mmc.txt
   [2] Documentation/devicetree/bindings/clock/clock-bindings.txt
   [3] 
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt

   [4] Documentation/devicetree/bindings/phy/phy-bindings.txt
+  [5] Documentation/devicetree/bindings/power/power_domain.txt

 Required Properties:
   - compatible: Compatibility string.  One of:
@@ -36,6 +38,8 @@ Optional Properties:
   - #clock-cells: If specified this should be the value <0>.  With 
this property
 in place we will export a clock representing the Card Clock.  
This clock

 is expected to be consumed by our PHY.  You must also specify
+  - power-domains: A phandle of and PM domain as specifier defined 
by bindings


A phandle of what?


That's a typo.




+of the power controller specified by phandle.

 Example:
 sdhci@e010 {


It would be nice to add the example code.


Okay, I will show a sample code in V2.

Thanks, shawn.

Re: [PATCH 1/2] Documentation: mmc: sdhci-of-arasan: add description of power domain

[Ziyuan Xu]

On 2016年08月27日 22:50, Shawn Lin wrote:

On 2016/8/27 21:41, Ziyuan Xu wrote:

Add power domain as a optional property for sdhci-of-arasan, which can
be truned off in the so-called unused condition, such as suspend and
remove. Aim to lower power requirements.

Signed-off-by: Ziyuan Xu 

---

 Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt 
b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt

index 3404afa..3f5189c 100644
--- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
@@ -1,12 +1,14 @@
 Device Tree Bindings for the Arasan SDHCI Controller

-  The bindings follow the mmc[1], clock[2], interrupt[3] and phy[4] 
bindings.
+  The bindings follow the mmc[1], clock[2], interrupt[3], phy[4] and 
power

+  domain[5] bindings.
   Only deviations are documented here.

   [1] Documentation/devicetree/bindings/mmc/mmc.txt
   [2] Documentation/devicetree/bindings/clock/clock-bindings.txt
   [3] 
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt

   [4] Documentation/devicetree/bindings/phy/phy-bindings.txt
+  [5] Documentation/devicetree/bindings/power/power_domain.txt

 Required Properties:
   - compatible: Compatibility string.  One of:
@@ -36,6 +38,8 @@ Optional Properties:
   - #clock-cells: If specified this should be the value <0>.  With 
this property
 in place we will export a clock representing the Card Clock.  
This clock

 is expected to be consumed by our PHY.  You must also specify
+  - power-domains: A phandle of and PM domain as specifier defined 
by bindings


A phandle of what?


That's a typo.




+of the power controller specified by phandle.

 Example:
 sdhci@e010 {


It would be nice to add the example code.


Okay, I will show a sample code in V2.

Thanks, shawn.

[PATCH 1/2] Documentation: mmc: sdhci-of-arasan: add description of power domain

[Ziyuan Xu]

Add power domain as a optional property for sdhci-of-arasan, which can
be truned off in the so-called unused condition, such as suspend and
remove. Aim to lower power requirements.

Signed-off-by: Ziyuan Xu <xzy...@rock-chips.com>

---

 Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt 
b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
index 3404afa..3f5189c 100644
--- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
@@ -1,12 +1,14 @@
 Device Tree Bindings for the Arasan SDHCI Controller
 
-  The bindings follow the mmc[1], clock[2], interrupt[3] and phy[4] bindings.
+  The bindings follow the mmc[1], clock[2], interrupt[3], phy[4] and power
+  domain[5] bindings.
   Only deviations are documented here.
 
   [1] Documentation/devicetree/bindings/mmc/mmc.txt
   [2] Documentation/devicetree/bindings/clock/clock-bindings.txt
   [3] Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
   [4] Documentation/devicetree/bindings/phy/phy-bindings.txt
+  [5] Documentation/devicetree/bindings/power/power_domain.txt
 
 Required Properties:
   - compatible: Compatibility string.  One of:
@@ -36,6 +38,8 @@ Optional Properties:
   - #clock-cells: If specified this should be the value <0>.  With this 
property
 in place we will export a clock representing the Card Clock.  This clock
 is expected to be consumed by our PHY.  You must also specify
+  - power-domains: A phandle of and PM domain as specifier defined by bindings
+of the power controller specified by phandle.
 
 Example:
sdhci@e010 {
-- 
2.9.2

[PATCH 1/2] Documentation: mmc: sdhci-of-arasan: add description of power domain

[Ziyuan Xu]

Add power domain as a optional property for sdhci-of-arasan, which can
be truned off in the so-called unused condition, such as suspend and
remove. Aim to lower power requirements.

Signed-off-by: Ziyuan Xu 

---

 Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt 
b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
index 3404afa..3f5189c 100644
--- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
@@ -1,12 +1,14 @@
 Device Tree Bindings for the Arasan SDHCI Controller
 
-  The bindings follow the mmc[1], clock[2], interrupt[3] and phy[4] bindings.
+  The bindings follow the mmc[1], clock[2], interrupt[3], phy[4] and power
+  domain[5] bindings.
   Only deviations are documented here.
 
   [1] Documentation/devicetree/bindings/mmc/mmc.txt
   [2] Documentation/devicetree/bindings/clock/clock-bindings.txt
   [3] Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
   [4] Documentation/devicetree/bindings/phy/phy-bindings.txt
+  [5] Documentation/devicetree/bindings/power/power_domain.txt
 
 Required Properties:
   - compatible: Compatibility string.  One of:
@@ -36,6 +38,8 @@ Optional Properties:
   - #clock-cells: If specified this should be the value <0>.  With this 
property
 in place we will export a clock representing the Card Clock.  This clock
 is expected to be consumed by our PHY.  You must also specify
+  - power-domains: A phandle of and PM domain as specifier defined by bindings
+of the power controller specified by phandle.
 
 Example:
sdhci@e010 {
-- 
2.9.2

[PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Ziyuan Xu]

Control power domain for eMMC via genpd to reduce power consumption.

Signed-off-by: Elaine Zhang <zhangq...@rock-chips.com>
Signed-off-by: Ziyuan Xu <xzy...@rock-chips.com>

---

 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 32aebc8..71733d4 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -239,6 +239,7 @@
#clock-cells = <0>;
phys = <_phy>;
phy-names = "phy_arasan";
+   power-domains = < RK3399_PD_EMMC>;
status = "disabled";
};
 
@@ -611,6 +612,11 @@
status = "disabled";
};
 
+   qos_emmc: qos@ffa58000 {
+   compatible = "syscon";
+   reg = <0x0 0xffa58000 0x0 0x20>;
+   };
+
qos_hdcp: qos@ffa9 {
compatible = "syscon";
reg = <0x0 0xffa9 0x0 0x20>;
@@ -739,6 +745,11 @@
};
 
/* These power domains are grouped by VD_LOGIC */
+   pd_emmc@RK3399_PD_EMMC {
+   reg = ;
+   clocks = < ACLK_EMMC>;
+   pm_qos = <_emmc>;
+   };
pd_vio@RK3399_PD_VIO {
reg = ;
#address-cells = <1>;
-- 
2.9.2

[PATCH 0/2] Add power domain support for eMMC node on rk3399

[Ziyuan Xu]

This series add power domain for eMMC node which will be controlled by
genpd to make sure it's available in probing state, and will be off
once suspend/remove.


Ziyuan Xu (2):
  Documentation: mmc: sdhci-of-arasan: add description of power domain
  arm64: dts: rockchip: add eMMC's power domain support for rk3399

 Documentation/devicetree/bindings/mmc/arasan,sdhci.txt |  6 +-
 arch/arm64/boot/dts/rockchip/rk3399.dtsi   | 11 +++
 2 files changed, 16 insertions(+), 1 deletion(-)

-- 
2.9.2

[PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Ziyuan Xu]

Control power domain for eMMC via genpd to reduce power consumption.

Signed-off-by: Elaine Zhang 
Signed-off-by: Ziyuan Xu 

---

 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 32aebc8..71733d4 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -239,6 +239,7 @@
#clock-cells = <0>;
phys = <_phy>;
phy-names = "phy_arasan";
+   power-domains = < RK3399_PD_EMMC>;
status = "disabled";
};
 
@@ -611,6 +612,11 @@
status = "disabled";
};
 
+   qos_emmc: qos@ffa58000 {
+   compatible = "syscon";
+   reg = <0x0 0xffa58000 0x0 0x20>;
+   };
+
qos_hdcp: qos@ffa9 {
compatible = "syscon";
reg = <0x0 0xffa9 0x0 0x20>;
@@ -739,6 +745,11 @@
};
 
/* These power domains are grouped by VD_LOGIC */
+   pd_emmc@RK3399_PD_EMMC {
+   reg = ;
+   clocks = < ACLK_EMMC>;
+   pm_qos = <_emmc>;
+   };
pd_vio@RK3399_PD_VIO {
reg = ;
#address-cells = <1>;
-- 
2.9.2

[PATCH 0/2] Add power domain support for eMMC node on rk3399

[Ziyuan Xu]

This series add power domain for eMMC node which will be controlled by
genpd to make sure it's available in probing state, and will be off
once suspend/remove.


Ziyuan Xu (2):
  Documentation: mmc: sdhci-of-arasan: add description of power domain
  arm64: dts: rockchip: add eMMC's power domain support for rk3399

 Documentation/devicetree/bindings/mmc/arasan,sdhci.txt |  6 +-
 arch/arm64/boot/dts/rockchip/rk3399.dtsi   | 11 +++
 2 files changed, 16 insertions(+), 1 deletion(-)

-- 
2.9.2