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