| Message ID | 1408460913-13863-6-git-send-email-javier.martinez@collabora.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Javier, On Tue, Aug 19, 2014 at 8:08 AM, Javier Martinez Canillas <javier.martinez@collabora.co.uk> wrote: > The VCC/VDD and VCCQ/VDD_IO power supplies for the MMC are > provided by the tps65090 fet4 and max77802 ldo4 regulators > respectively. Add the phandle to the regulators tree nodes > for the the dw_mmc device device. > > These DTS snippets were taken from the downstream ChromeOS > 3.8 kernel Device Tree for Peach Pit and Pi boards. > > Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> > --- > arch/arm/boot/dts/exynos5420-peach-pit.dts | 2 ++ > arch/arm/boot/dts/exynos5800-peach-pi.dts | 2 ++ > 2 files changed, 4 insertions(+) > > diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts > index 8e50042..5b9dbb9 100644 > --- a/arch/arm/boot/dts/exynos5420-peach-pit.dts > +++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts > @@ -547,6 +547,8 @@ > }; > > &mmc_2 { > + vmmc-supply = <&tps65090_fet4>; > + vqmmc-supply = <&vqmmc_sdcard>; > status = "okay"; > num-slots = <1>; > supports-highspeed; > diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts > index 939f91c..dcac443 100644 > --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts > +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts > @@ -545,6 +545,8 @@ > }; > > &mmc_2 { > + vmmc-supply = <&tps65090_fet4>; > + vqmmc-supply = <&vqmmc_sdcard>; > status = "okay"; > num-slots = <1>; > supports-highspeed; While your change is correct, I have a worry that it will break things if it's merged before some patches that Yuvaraj is working on. Specifically the problem on pit and pi (and any exynos5250 / 5420 / 5800 / ... boards using the built-in "card detect") is that the card detect line is on the same power rail as "vqmmc". That means you can't turn off vqmmc if you still need to be able to detect card insertions. ...but you can't turn off vmmc without turning off vqmmc, otherwise current will leak through the IO lines into the card, which is bad. Right now the SDMMC core will try to turn off power to the card at two times: 1. when the card is ejected 2. when it's trying to reset the card Obviously the first problem is a huge problem on exynos because it means that we won't be able to detect card insertions. ...but we still want to turn the power off from #2. To really fix the problem I think the core needs to be extended to treat the above as two separate cases. Your patch might work at the moment because I think dw_mmc doesn't actually try to turn off these rails with the main SDMMC core asks it to. ...but I still worry about merging them before Yuvaraj's changes are ready. -Doug
Hi Javier, Am 19.08.2014 17:08, schrieb Javier Martinez Canillas: > diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts > index 8e50042..5b9dbb9 100644 > --- a/arch/arm/boot/dts/exynos5420-peach-pit.dts > +++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts > @@ -547,6 +547,8 @@ > }; > > &mmc_2 { > + vmmc-supply = <&tps65090_fet4>; > + vqmmc-supply = <&vqmmc_sdcard>; > status = "okay"; > num-slots = <1>; > supports-highspeed; > diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts > index 939f91c..dcac443 100644 > --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts > +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts > @@ -545,6 +545,8 @@ > }; > > &mmc_2 { > + vmmc-supply = <&tps65090_fet4>; > + vqmmc-supply = <&vqmmc_sdcard>; > status = "okay"; > num-slots = <1>; > supports-highspeed; A convention that I picked up was to place overridden status property first. Do your new properties really need to go before that? Also, if you look at today's for-next, then supports-highspeed should be cap-{sd,mmc}-highspeed, so it may be worth to rebase. Regards, Andreas
Hello Doug, On 08/19/2014 06:26 PM, Doug Anderson wrote: > > While your change is correct, I have a worry that it will break things > if it's merged before some patches that Yuvaraj is working on. > > Specifically the problem on pit and pi (and any exynos5250 / 5420 / > 5800 / ... boards using the built-in "card detect") is that the card > detect line is on the same power rail as "vqmmc". That means you > can't turn off vqmmc if you still need to be able to detect card > insertions. > > ...but you can't turn off vmmc without turning off vqmmc, otherwise > current will leak through the IO lines into the card, which is bad. > > > Right now the SDMMC core will try to turn off power to the card at two times: > 1. when the card is ejected > 2. when it's trying to reset the card > > > Obviously the first problem is a huge problem on exynos because it > means that we won't be able to detect card insertions. ...but we > still want to turn the power off from #2. To really fix the problem I > think the core needs to be extended to treat the above as two separate > cases. > Thanks a lot for the explanation, now I remember reading about this on thread "[PATCH 2/3] mmc: dw_mmc: Dont cut off vqmmc and vmmc" [0] but didn't think in the side effect when adding this patch... > > Your patch might work at the moment because I think dw_mmc doesn't > actually try to turn off these rails with the main SDMMC core asks it > to. ...but I still worry about merging them before Yuvaraj's changes > are ready. > Agreed, I'll drop this patch and wait for Yuvaraj's series to land first. Yuvaraj's "UHS support for dw_mmc driver" series that should contain the change you are referring had the issue that when the tps65090 fet4 was used as vmmc-supply, mmc_regulator_get_supply() was failing. I posted a bunch of patches to fix that issue and there were already merged on the relevant trees and shows on linux-next: e1da8cd mmc: core: Use regulator_get_voltage() if OCR mask is empty. a130548 ARM: dts: Improve Peach Pit and Pi power scheme 4f2352c regulator: tps65090: Set voltage for fixed regulators 26988ef regulator: core: Allow to get voltage count and list from parent e303996 regulator: core: Get voltage from parent if not available So hopefully his series have no blockers anymore. > > -Doug > Best regards, Javier [0]: https://www.mail-archive.com/linux-mmc@vger.kernel.org/msg27152.html
Hello Andreas, Thanks a lot for your feedback. On 08/19/2014 08:37 PM, Andreas Färber wrote: >> &mmc_2 { >> + vmmc-supply = <&tps65090_fet4>; >> + vqmmc-supply = <&vqmmc_sdcard>; >> status = "okay"; >> num-slots = <1>; >> supports-highspeed; > > A convention that I picked up was to place overridden status property > first. Do your new properties really need to go before that? > OF doesn't care about the order of the child nodes but I do agree that consistency is a good thing so I'll change that on v2. > Also, if you look at today's for-next, then supports-highspeed should be > cap-{sd,mmc}-highspeed, so it may be worth to rebase. > You are right, this patch used to be part of an older series so I didn't realize that "[PATCHv10 3/5] ARM: dts: exynos: unuse the slot-node and deprecate the supports-highspeed for dw-mmc" was already merged. Thanks for the reminder! > Regards, > Andreas > Best regards, Javier
Hello Andreas, On 08/20/2014 12:21 PM, Javier Martinez Canillas wrote: > Hello Andreas, > > Thanks a lot for your feedback. > > On 08/19/2014 08:37 PM, Andreas Färber wrote: >>> &mmc_2 { >>> + vmmc-supply = <&tps65090_fet4>; >>> + vqmmc-supply = <&vqmmc_sdcard>; >>> status = "okay"; >>> num-slots = <1>; >>> supports-highspeed; >> >> A convention that I picked up was to place overridden status property >> first. Do your new properties really need to go before that? >> > > OF doesn't care about the order of the child nodes but I do agree that > consistency is a good thing so I'll change that on v2. > Actually I meant that I'll change this for a future re-post since I'm completely dropping this patch from the series due Doug's explanation about the CD line and VCCQ/VDD_IO being in the same power rail on Exynos5 SoCs. Sorry for the noise... Best regards, Javier
diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts index 8e50042..5b9dbb9 100644 --- a/arch/arm/boot/dts/exynos5420-peach-pit.dts +++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts @@ -547,6 +547,8 @@ }; &mmc_2 { + vmmc-supply = <&tps65090_fet4>; + vqmmc-supply = <&vqmmc_sdcard>; status = "okay"; num-slots = <1>; supports-highspeed; diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts index 939f91c..dcac443 100644 --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts @@ -545,6 +545,8 @@ }; &mmc_2 { + vmmc-supply = <&tps65090_fet4>; + vqmmc-supply = <&vqmmc_sdcard>; status = "okay"; num-slots = <1>; supports-highspeed;
The VCC/VDD and VCCQ/VDD_IO power supplies for the MMC are provided by the tps65090 fet4 and max77802 ldo4 regulators respectively. Add the phandle to the regulators tree nodes for the the dw_mmc device device. These DTS snippets were taken from the downstream ChromeOS 3.8 kernel Device Tree for Peach Pit and Pi boards. Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> --- arch/arm/boot/dts/exynos5420-peach-pit.dts | 2 ++ arch/arm/boot/dts/exynos5800-peach-pi.dts | 2 ++ 2 files changed, 4 insertions(+)