[...] >>> >>>> >>>>> + 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. > >> 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. > >> 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
