Thread (10 messages) 10 messages, 3 authors, 2015-10-19

Re: [PATCH 1/2] mmc: sdhci-of-arasan: add phy support for sdhci-of-arasan

From: Ulf Hansson <hidden>
Date: 2015-10-19 10:54:20
Also in: linux-mmc, linux-rockchip, lkml

[...]
quoted
quoted
quoted
quoted
+               ret = phy_power_on(sdhci_arasan->phy);


This looks a bit weird. Shouldn't you do phy_init() prior
phy_power_on()?

Similar comment applies to phy_exit() and phy_power_off().


Both are okay. It depends how the phy-driver implement the four
interfaces.
 From my case, power_on for arasan's phy driver firstly enable phy's clk
and
open power-domain, then programme phy internal registers to activate phy.
Without enabling phy's clk and power-domain, we cannot do phy_init since
phy
can't be accessed by CPU.

But here, I think you are right. It does look a bit weird.
I think the better way is to remove phy_power_on here, and let phy-driver
do
power_on in phy_init internally. Similarly in the remove call.

That makes sense to me! I think it would also follow other phy clients
use of the phy API.

What makes me wonder though is from a power management point of view.
*When* do you need to have phy initialized and powered?
Whenever controller need to communicate with card, must init/power_on phy
firstly.
quoted
1) For a removable card can leave the phy uninitialised or powered
off, but still detect card insertion/removal via GPIO? Is that a valid
scenario for you?
Theoretically, it is.  Although my soc don't use phy for removable card, I
also consider how to handle this case. Should we add a hook
for sdhci_get_cd, and initialize phy if it's non-removeable device or
removeable card in slot ? Doesn't seem like a good idea.

Also seems not always work if we just use SDHCI_PRESENT_STATE to detect card
insertion/removal without phy in active state.
As you SoC don't use removable card, let's just leave this for now.
Thanks for sharing your thoughts.
quoted
2) Considering the runtime PM case for the sdhci device. Typically you
can gate clocks etc at runtime suspend to save power, but what about
the phy? Can you power off it in runtime suspend?

yes, we can power off it in runtime suspend. So we can append some patches
later to introduce runtime pm for sdhci-of-arasan?
Yes, that's fine. I would thus expect that you want to do phy power
off/on from the runtime PM callbacks, right!?

If that's the case I think $subject patch should deal with *both* phy
init and phy power on during ->probe().

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help