On 2015/10/19 15:50, Ulf Hansson wrote:
[...]
I understand the phy is optional, but you still need to handle the
EPROBE_DEFER case.
Perhaps you should also use devm_phy_optional_get() instead!?
I already changed it in version-2 [1]. :)
phy is mandatory for sdhci-arasan,5.1.
[1]: https://patchwork.kernel.org/patch/7173251/
Oh, apologize for reviewing the old version!
+ 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.
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?
[...]
Kind regards
Uffe
--
Best Regards
Shawn Lin
--
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
