Re: FW: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
Hello, On 11/30/2012 5:48 PM, Chris Ball wrote: On Tue, Nov 20 2012, Marek Szyprowski wrote: The problem with dummy regulator is the fact that it can be enabled only globally for all devices in the system. I think that the best solution would be to introduce regulator_can_change_voltage() as Mark suggested. I will post patches soon. Does this mean that I shouldn't merge either yours or Kevin's patch for 3.8, while we wait for this? Any ETA on it? I've just posted an updated patches to solve those issues. I'm sorry for the delay as I was terribly busy with other stuff. Kevin's patch is still required and it should stay in the tree because it indeed fixes other, in some cases related issues. However my earlier patch mmc: sdhci: apply voltage range check only for non-fixed regulators can be replaced with the new patches from the [PATCH v2 0/3] Fix fixed regulators support thread posted a few minutes ago (those patches are based on mmc-next branch). Best regards -- Marek Szyprowski Samsung Poland RD Center -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: FW: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
Hi Marek, On Tue, Nov 20 2012, Marek Szyprowski wrote: The problem with dummy regulator is the fact that it can be enabled only globally for all devices in the system. I think that the best solution would be to introduce regulator_can_change_voltage() as Mark suggested. I will post patches soon. Does this mean that I shouldn't merge either yours or Kevin's patch for 3.8, while we wait for this? Any ETA on it? Thanks very much, - Chris. -- Chris Ball c...@laptop.org http://printf.net/ One Laptop Per Child -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
On 11/13/2012 10:23 PM, Philip Rakity wrote: Hi Marek, Is the regulator dedicated ? or is it shared ? Is it used for eMMC ? If it cannot be turned off -- then just don't list it in the regulators list for vmmc. If it CAN be turned off then need to get back to you. It is dedicated to eMMC device and can be turned off. Patch mmc: sdhci: apply voltage range check only for non-fixed regulators restored sdhci to working state after merging the regulator fix. However there are lots of error messages from sdhci driver: s3c-sdhci s3c-sdhci.0: could not set regulator OCR (-1) The only remaining problem is sdhci driver operation with dummy regulator. Right now it simply fails to initialize if dummy regulator is enabled. Do you have any idea how to fix it properly? Best regards -- Marek Szyprowski Samsung Poland RD Center -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: FW: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
2012/11/20 Marek Szyprowski m.szyprow...@samsung.com:
Hello,
On 11/14/2012 8:11 AM, Kevin Liu wrote:
From: linux-mmc-ow...@vger.kernel.org
[mailto:linux-mmc-ow...@vger.kernel.org] On Behalf Of Chris Ball
Sent: Tuesday, November 13, 2012 10:14 PM
To: Marek Szyprowski
Cc: linux-ker...@vger.kernel.org; linux-mmc@vger.kernel.org; Kyungmin
Park; Mark Brown; Liam Girdwood; Philip Rakity
Subject: Re: [PATCH v2] mmc: sdhci: apply voltage range check only for
non-fixed regulators
Hi,
On Tue, Nov 13 2012, Marek Szyprowski wrote:
On Tue, Nov 13 2012, Marek Szyprowski wrote:
Fixed regulators cannot change their voltage, so disable all voltage
range checking for them, otherwise the driver fails to operate with
fixed regulators. Up to now it worked only by luck, because
regulator_is_supported_voltage() function returned incorrect values.
Commit regulator: fix voltage check in
regulator_is_supported_voltage()
fixed that function and now additional check is needed for fixed
regulators.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host *host)
regulator_enable(host-vmmc);
#ifdef CONFIG_REGULATOR
- if (host-vmmc) {
+ if (host-vmmc regulator_count_voltages(host-vmmc) 1) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
Thanks for the longer explanation. I'm still missing something,
though;
what's wrong with running the check as it was with the new regulator
code?
(I haven't tried it yet.)
#ifdef CONFIG_REGULATOR
if (host-vmmc) {
ret = regulator_is_supported_voltage(host-vmmc,
330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
caps[0] = ~SDHCI_CAN_VDD_330;
ret = regulator_is_supported_voltage(host-vmmc,
300,
300);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_300)))
caps[0] = ~SDHCI_CAN_VDD_300;
ret = regulator_is_supported_voltage(host-vmmc,
180,
180);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_180)))
caps[0] = ~SDHCI_CAN_VDD_180;
}
#endif /* CONFIG_REGULATOR */
The point is to remove unsupported voltages, so if someone sets up a
fixed regulator at 330, all of the other caps are disabled. Why
wouldn't that work without this change, and how are we supposed to
remove those caps on a fixed regulator after your patchset?
Thanks, sorry if I'm missing something obvious,
On our boards eMMC is connected to fixed 2.8V regulator, what results
in
clearing all available voltages and fail. The same situation is when
one
enable dummy regulator and try to use sdhci with it. My patch fixes
this
and restores sdhci to working state as it was before (before fixing
regulator regulator_is_supported_voltage() function and earlier when
MMC_BROKEN_VOLATGE capability was used).
I see. Sounds like a separate bug -- Philip (or anyone else), any
idea how we should be treating eMMCs with a fixed voltage here?
I think we should check the voltage range rather than the voltage
point accoring to the spec.
Otherwise some valid voltage like 2.8v will be discarded by mistake.
My below old patch aim to fix this issue.
How do you think?
-Original Message-
From: Kevin Liu [mailto:keyuan@gmail.com]
Sent: Friday, September 28, 2012 3:56 PM
To: linux-mmc@vger.kernel.org; c...@laptop.org; pie...@ossman.eu;
ulf.hans...@linaro.org; Zhangfei Gao
Cc: Haojian Zhuang; Chao Xie; Philip Rakity; Kevin Liu; Jialing Fu
Subject: [PATCH v5 03/13] mmc: sdhci: use regulator min/max voltage
range according to spec
From: Kevin Liu kl...@marvell.com
For regulator vmmc/vmmcq, use voltage range as below
3.3v/3.0v: (2.7v, 3.6v)
1.8v: (1.7v, 1.95v)
Original code use the specific value which may fail in regulator
driver if it does NOT support the specific voltage.
Signed-off-by: Jialing Fu j...@marvell.com
Signed-off-by: Kevin Liu kl...@marvell.com
Tested-by: Marek Szyprowski m.szyprow...@samsung.com
This patch restores sdhci devices to working state on Samsung boards
(tested on GONI and UniversalC210) after merging regulator: fix voltage
check in regulator_is_supported_voltage() patch to v3.7-rc6 (commit
f0f98b19e23d4426ca185e3d4ca80e6aff5ef51b). Would be great to have it
merged before the final v3.7 is out
Re: FW: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
Hello,
On 11/20/2012 9:59 AM, Kevin Liu wrote:
2012/11/20 Marek Szyprowski m.szyprow...@samsung.com:
Hello,
On 11/14/2012 8:11 AM, Kevin Liu wrote:
From: linux-mmc-ow...@vger.kernel.org
[mailto:linux-mmc-ow...@vger.kernel.org] On Behalf Of Chris Ball
Sent: Tuesday, November 13, 2012 10:14 PM
To: Marek Szyprowski
Cc: linux-ker...@vger.kernel.org; linux-mmc@vger.kernel.org; Kyungmin
Park; Mark Brown; Liam Girdwood; Philip Rakity
Subject: Re: [PATCH v2] mmc: sdhci: apply voltage range check only for
non-fixed regulators
Hi,
On Tue, Nov 13 2012, Marek Szyprowski wrote:
On Tue, Nov 13 2012, Marek Szyprowski wrote:
Fixed regulators cannot change their voltage, so disable all voltage
range checking for them, otherwise the driver fails to operate with
fixed regulators. Up to now it worked only by luck, because
regulator_is_supported_voltage() function returned incorrect values.
Commit regulator: fix voltage check in
regulator_is_supported_voltage()
fixed that function and now additional check is needed for fixed
regulators.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host *host)
regulator_enable(host-vmmc);
#ifdef CONFIG_REGULATOR
- if (host-vmmc) {
+ if (host-vmmc regulator_count_voltages(host-vmmc) 1) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
Thanks for the longer explanation. I'm still missing something,
though;
what's wrong with running the check as it was with the new regulator
code?
(I haven't tried it yet.)
#ifdef CONFIG_REGULATOR
if (host-vmmc) {
ret = regulator_is_supported_voltage(host-vmmc,
330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
caps[0] = ~SDHCI_CAN_VDD_330;
ret = regulator_is_supported_voltage(host-vmmc,
300,
300);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_300)))
caps[0] = ~SDHCI_CAN_VDD_300;
ret = regulator_is_supported_voltage(host-vmmc,
180,
180);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_180)))
caps[0] = ~SDHCI_CAN_VDD_180;
}
#endif /* CONFIG_REGULATOR */
The point is to remove unsupported voltages, so if someone sets up a
fixed regulator at 330, all of the other caps are disabled. Why
wouldn't that work without this change, and how are we supposed to
remove those caps on a fixed regulator after your patchset?
Thanks, sorry if I'm missing something obvious,
On our boards eMMC is connected to fixed 2.8V regulator, what results
in
clearing all available voltages and fail. The same situation is when
one
enable dummy regulator and try to use sdhci with it. My patch fixes
this
and restores sdhci to working state as it was before (before fixing
regulator regulator_is_supported_voltage() function and earlier when
MMC_BROKEN_VOLATGE capability was used).
I see. Sounds like a separate bug -- Philip (or anyone else), any
idea how we should be treating eMMCs with a fixed voltage here?
I think we should check the voltage range rather than the voltage
point accoring to the spec.
Otherwise some valid voltage like 2.8v will be discarded by mistake.
My below old patch aim to fix this issue.
How do you think?
-Original Message-
From: Kevin Liu [mailto:keyuan@gmail.com]
Sent: Friday, September 28, 2012 3:56 PM
To: linux-mmc@vger.kernel.org; c...@laptop.org; pie...@ossman.eu;
ulf.hans...@linaro.org; Zhangfei Gao
Cc: Haojian Zhuang; Chao Xie; Philip Rakity; Kevin Liu; Jialing Fu
Subject: [PATCH v5 03/13] mmc: sdhci: use regulator min/max voltage
range according to spec
From: Kevin Liu kl...@marvell.com
For regulator vmmc/vmmcq, use voltage range as below
3.3v/3.0v: (2.7v, 3.6v)
1.8v: (1.7v, 1.95v)
Original code use the specific value which may fail in regulator
driver if it does NOT support the specific voltage.
Signed-off-by: Jialing Fu j...@marvell.com
Signed-off-by: Kevin Liu kl...@marvell.com
Tested-by: Marek Szyprowski m.szyprow...@samsung.com
This patch restores sdhci devices to working state on Samsung boards
(tested on GONI and UniversalC210) after merging regulator: fix voltage
check in regulator_is_supported_voltage() patch to v3.7-rc6 (commit
f0f98b19e23d4426ca185e3d4ca80e6aff5ef51b). Would be great
Re: FW: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
2012/11/20 Marek Szyprowski m.szyprow...@samsung.com:
Hello,
On 11/20/2012 9:59 AM, Kevin Liu wrote:
2012/11/20 Marek Szyprowski m.szyprow...@samsung.com:
Hello,
On 11/14/2012 8:11 AM, Kevin Liu wrote:
From: linux-mmc-ow...@vger.kernel.org
[mailto:linux-mmc-ow...@vger.kernel.org] On Behalf Of Chris Ball
Sent: Tuesday, November 13, 2012 10:14 PM
To: Marek Szyprowski
Cc: linux-ker...@vger.kernel.org; linux-mmc@vger.kernel.org; Kyungmin
Park; Mark Brown; Liam Girdwood; Philip Rakity
Subject: Re: [PATCH v2] mmc: sdhci: apply voltage range check only
for
non-fixed regulators
Hi,
On Tue, Nov 13 2012, Marek Szyprowski wrote:
On Tue, Nov 13 2012, Marek Szyprowski wrote:
Fixed regulators cannot change their voltage, so disable all
voltage
range checking for them, otherwise the driver fails to operate
with
fixed regulators. Up to now it worked only by luck, because
regulator_is_supported_voltage() function returned incorrect
values.
Commit regulator: fix voltage check in
regulator_is_supported_voltage()
fixed that function and now additional check is needed for fixed
regulators.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host *host)
regulator_enable(host-vmmc);
#ifdef CONFIG_REGULATOR
- if (host-vmmc) {
+ if (host-vmmc regulator_count_voltages(host-vmmc) 1) {
ret = regulator_is_supported_voltage(host-vmmc,
330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
Thanks for the longer explanation. I'm still missing something,
though;
what's wrong with running the check as it was with the new
regulator
code?
(I haven't tried it yet.)
#ifdef CONFIG_REGULATOR
if (host-vmmc) {
ret = regulator_is_supported_voltage(host-vmmc,
330,
330);
if ((ret = 0) || (!(caps[0]
SDHCI_CAN_VDD_330)))
caps[0] = ~SDHCI_CAN_VDD_330;
ret = regulator_is_supported_voltage(host-vmmc,
300,
300);
if ((ret = 0) || (!(caps[0]
SDHCI_CAN_VDD_300)))
caps[0] = ~SDHCI_CAN_VDD_300;
ret = regulator_is_supported_voltage(host-vmmc,
180,
180);
if ((ret = 0) || (!(caps[0]
SDHCI_CAN_VDD_180)))
caps[0] = ~SDHCI_CAN_VDD_180;
}
#endif /* CONFIG_REGULATOR */
The point is to remove unsupported voltages, so if someone sets up
a
fixed regulator at 330, all of the other caps are disabled.
Why
wouldn't that work without this change, and how are we supposed to
remove those caps on a fixed regulator after your patchset?
Thanks, sorry if I'm missing something obvious,
On our boards eMMC is connected to fixed 2.8V regulator, what
results
in
clearing all available voltages and fail. The same situation is when
one
enable dummy regulator and try to use sdhci with it. My patch fixes
this
and restores sdhci to working state as it was before (before fixing
regulator regulator_is_supported_voltage() function and earlier when
MMC_BROKEN_VOLATGE capability was used).
I see. Sounds like a separate bug -- Philip (or anyone else), any
idea how we should be treating eMMCs with a fixed voltage here?
I think we should check the voltage range rather than the voltage
point accoring to the spec.
Otherwise some valid voltage like 2.8v will be discarded by mistake.
My below old patch aim to fix this issue.
How do you think?
-Original Message-
From: Kevin Liu [mailto:keyuan@gmail.com]
Sent: Friday, September 28, 2012 3:56 PM
To: linux-mmc@vger.kernel.org; c...@laptop.org; pie...@ossman.eu;
ulf.hans...@linaro.org; Zhangfei Gao
Cc: Haojian Zhuang; Chao Xie; Philip Rakity; Kevin Liu; Jialing Fu
Subject: [PATCH v5 03/13] mmc: sdhci: use regulator min/max voltage
range according to spec
From: Kevin Liu kl...@marvell.com
For regulator vmmc/vmmcq, use voltage range as below
3.3v/3.0v: (2.7v, 3.6v)
1.8v: (1.7v, 1.95v)
Original code use the specific value which may fail in regulator
driver if it does NOT support the specific voltage.
Signed-off-by: Jialing Fu j...@marvell.com
Signed-off-by: Kevin Liu kl...@marvell.com
Tested-by: Marek Szyprowski m.szyprow...@samsung.com
This patch restores sdhci devices to working state
Re: FW: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
Hello,
On 11/20/2012 12:36 PM, Kevin Liu wrote:
2012/11/20 Marek Szyprowski m.szyprow...@samsung.com:
On 11/20/2012 9:59 AM, Kevin Liu wrote:
2012/11/20 Marek Szyprowski m.szyprow...@samsung.com:
On 11/14/2012 8:11 AM, Kevin Liu wrote:
From: linux-mmc-ow...@vger.kernel.org
[mailto:linux-mmc-ow...@vger.kernel.org] On Behalf Of Chris Ball
Sent: Tuesday, November 13, 2012 10:14 PM
To: Marek Szyprowski
Cc: linux-ker...@vger.kernel.org; linux-mmc@vger.kernel.org; Kyungmin
Park; Mark Brown; Liam Girdwood; Philip Rakity
Subject: Re: [PATCH v2] mmc: sdhci: apply voltage range check only
for non-fixed regulators
On Tue, Nov 13 2012, Marek Szyprowski wrote:
On Tue, Nov 13 2012, Marek Szyprowski wrote:
Fixed regulators cannot change their voltage, so disable all
voltage
range checking for them, otherwise the driver fails to operate
with
fixed regulators. Up to now it worked only by luck, because
regulator_is_supported_voltage() function returned incorrect
values.
Commit regulator: fix voltage check in
regulator_is_supported_voltage()
fixed that function and now additional check is needed for fixed
regulators.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host *host)
regulator_enable(host-vmmc);
#ifdef CONFIG_REGULATOR
- if (host-vmmc) {
+ if (host-vmmc regulator_count_voltages(host-vmmc) 1) {
ret = regulator_is_supported_voltage(host-vmmc,
330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
Thanks for the longer explanation. I'm still missing something,
though;
what's wrong with running the check as it was with the new
regulator
code?
(I haven't tried it yet.)
#ifdef CONFIG_REGULATOR
if (host-vmmc) {
ret = regulator_is_supported_voltage(host-vmmc,
330,
330);
if ((ret = 0) || (!(caps[0]
SDHCI_CAN_VDD_330)))
caps[0] = ~SDHCI_CAN_VDD_330;
ret = regulator_is_supported_voltage(host-vmmc,
300,
300);
if ((ret = 0) || (!(caps[0]
SDHCI_CAN_VDD_300)))
caps[0] = ~SDHCI_CAN_VDD_300;
ret = regulator_is_supported_voltage(host-vmmc,
180,
180);
if ((ret = 0) || (!(caps[0]
SDHCI_CAN_VDD_180)))
caps[0] = ~SDHCI_CAN_VDD_180;
}
#endif /* CONFIG_REGULATOR */
The point is to remove unsupported voltages, so if someone sets up
a
fixed regulator at 330, all of the other caps are disabled.
Why
wouldn't that work without this change, and how are we supposed to
remove those caps on a fixed regulator after your patchset?
Thanks, sorry if I'm missing something obvious,
On our boards eMMC is connected to fixed 2.8V regulator, what
results
in
clearing all available voltages and fail. The same situation is when
one
enable dummy regulator and try to use sdhci with it. My patch fixes
this
and restores sdhci to working state as it was before (before fixing
regulator regulator_is_supported_voltage() function and earlier when
MMC_BROKEN_VOLATGE capability was used).
I see. Sounds like a separate bug -- Philip (or anyone else), any
idea how we should be treating eMMCs with a fixed voltage here?
I think we should check the voltage range rather than the voltage
point accoring to the spec.
Otherwise some valid voltage like 2.8v will be discarded by mistake.
My below old patch aim to fix this issue.
How do you think?
-Original Message-
From: Kevin Liu [mailto:keyuan@gmail.com]
Sent: Friday, September 28, 2012 3:56 PM
To: linux-mmc@vger.kernel.org; c...@laptop.org; pie...@ossman.eu;
ulf.hans...@linaro.org; Zhangfei Gao
Cc: Haojian Zhuang; Chao Xie; Philip Rakity; Kevin Liu; Jialing Fu
Subject: [PATCH v5 03/13] mmc: sdhci: use regulator min/max voltage
range according to spec
From: Kevin Liu kl...@marvell.com
For regulator vmmc/vmmcq, use voltage range as below
3.3v/3.0v: (2.7v, 3.6v)
1.8v: (1.7v, 1.95v)
Original code use the specific value which may fail in regulator
driver if it does NOT support the specific voltage.
Signed-off-by: Jialing Fu j...@marvell.com
Signed-off-by: Kevin Liu kl...@marvell.com
Tested-by: Marek Szyprowski m.szyprow...@samsung.com
This patch restores sdhci devices
Re: FW: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
Hi, On Tue, Nov 20 2012, Marek Szyprowski wrote: Subject: [PATCH v5 03/13] mmc: sdhci: use regulator min/max voltage range according to spec From: Kevin Liu kl...@marvell.com For regulator vmmc/vmmcq, use voltage range as below 3.3v/3.0v: (2.7v, 3.6v) 1.8v: (1.7v, 1.95v) Original code use the specific value which may fail in regulator driver if it does NOT support the specific voltage. Signed-off-by: Jialing Fu j...@marvell.com Signed-off-by: Kevin Liu kl...@marvell.com Tested-by: Marek Szyprowski m.szyprow...@samsung.com This patch restores sdhci devices to working state on Samsung boards (tested on GONI and UniversalC210) after merging regulator: fix voltage check in regulator_is_supported_voltage() patch to v3.7-rc6 (commit f0f98b19e23d4426ca185e3d4ca80e6aff5ef51b). Would be great to have it merged before the final v3.7 is out. Marek, thanks a lot for the verification! And your patch mmc: sdhci: apply voltage range check only for non-fixed regulators (commit d5b5205f2d480a47863dda0772d2d9dc47c2b51b, which has been merged in mmc-next) can be reverted if this patch merged? Yes, it can be replaced with it, although there is still an issue that need to be resolved somehow. Right now SDHCI driver fails to initialize if support for dummy regulator is enabled. Then I think the dummy issue can be resolved with your patch merged and if you can just update your patch from regulator_count_voltages(host-vmmc) 1 to regulator_count_voltages(host-vmmc) 0 to let fix regulator work. regulator_count_voltages() returns 1 for both fixed regulators and for virtual dummy regulator, so the above change makes no sense. However I was so focused on fixing the 2.8V supply case that I missed the fact that my mmc: sdhci: apply voltage range check only for non-fixed regulators patch also fixed the dummy regulator case. The conclusion is that applying both patches should finally fix the regulator issues with for the Samsung boards (2.8V supply for eMMC) and 'dummy-regulators' cases. Thanks, I've pushed v5 of mmc: sdhci: use regulator min/max voltage range according to spec to mmc-next for 3.7 with Marek's Tested-by now. - Chris. -- Chris Ball c...@laptop.org http://printf.net/ One Laptop Per Child -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: FW: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
2012/11/20 Marek Szyprowski m.szyprow...@samsung.com:
Hello,
On 11/20/2012 12:36 PM, Kevin Liu wrote:
2012/11/20 Marek Szyprowski m.szyprow...@samsung.com:
On 11/20/2012 9:59 AM, Kevin Liu wrote:
2012/11/20 Marek Szyprowski m.szyprow...@samsung.com:
On 11/14/2012 8:11 AM, Kevin Liu wrote:
From: linux-mmc-ow...@vger.kernel.org
[mailto:linux-mmc-ow...@vger.kernel.org] On Behalf Of Chris Ball
Sent: Tuesday, November 13, 2012 10:14 PM
To: Marek Szyprowski
Cc: linux-ker...@vger.kernel.org; linux-mmc@vger.kernel.org;
Kyungmin
Park; Mark Brown; Liam Girdwood; Philip Rakity
Subject: Re: [PATCH v2] mmc: sdhci: apply voltage range check only
for non-fixed regulators
On Tue, Nov 13 2012, Marek Szyprowski wrote:
On Tue, Nov 13 2012, Marek Szyprowski wrote:
Fixed regulators cannot change their voltage, so disable all
voltage
range checking for them, otherwise the driver fails to operate
with
fixed regulators. Up to now it worked only by luck, because
regulator_is_supported_voltage() function returned incorrect
values.
Commit regulator: fix voltage check in
regulator_is_supported_voltage()
fixed that function and now additional check is needed for
fixed
regulators.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c
b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host
*host)
regulator_enable(host-vmmc);
#ifdef CONFIG_REGULATOR
- if (host-vmmc) {
+ if (host-vmmc regulator_count_voltages(host-vmmc) 1)
{
ret = regulator_is_supported_voltage(host-vmmc,
330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
Thanks for the longer explanation. I'm still missing something,
though;
what's wrong with running the check as it was with the new
regulator
code?
(I haven't tried it yet.)
#ifdef CONFIG_REGULATOR
if (host-vmmc) {
ret =
regulator_is_supported_voltage(host-vmmc,
330,
330);
if ((ret = 0) || (!(caps[0]
SDHCI_CAN_VDD_330)))
caps[0] = ~SDHCI_CAN_VDD_330;
ret =
regulator_is_supported_voltage(host-vmmc,
300,
300);
if ((ret = 0) || (!(caps[0]
SDHCI_CAN_VDD_300)))
caps[0] = ~SDHCI_CAN_VDD_300;
ret =
regulator_is_supported_voltage(host-vmmc,
180,
180);
if ((ret = 0) || (!(caps[0]
SDHCI_CAN_VDD_180)))
caps[0] = ~SDHCI_CAN_VDD_180;
}
#endif /* CONFIG_REGULATOR */
The point is to remove unsupported voltages, so if someone sets
up
a
fixed regulator at 330, all of the other caps are disabled.
Why
wouldn't that work without this change, and how are we supposed
to
remove those caps on a fixed regulator after your patchset?
Thanks, sorry if I'm missing something obvious,
On our boards eMMC is connected to fixed 2.8V regulator, what
results
in
clearing all available voltages and fail. The same situation is
when
one
enable dummy regulator and try to use sdhci with it. My patch
fixes
this
and restores sdhci to working state as it was before (before
fixing
regulator regulator_is_supported_voltage() function and earlier
when
MMC_BROKEN_VOLATGE capability was used).
I see. Sounds like a separate bug -- Philip (or anyone else), any
idea how we should be treating eMMCs with a fixed voltage here?
I think we should check the voltage range rather than the voltage
point accoring to the spec.
Otherwise some valid voltage like 2.8v will be discarded by mistake.
My below old patch aim to fix this issue.
How do you think?
-Original Message-
From: Kevin Liu [mailto:keyuan@gmail.com]
Sent: Friday, September 28, 2012 3:56 PM
To: linux-mmc@vger.kernel.org; c...@laptop.org; pie...@ossman.eu;
ulf.hans...@linaro.org; Zhangfei Gao
Cc: Haojian Zhuang; Chao Xie; Philip Rakity; Kevin Liu; Jialing Fu
Subject: [PATCH v5 03/13] mmc: sdhci: use regulator min/max voltage
range according to spec
From: Kevin Liu kl...@marvell.com
For regulator vmmc/vmmcq, use voltage range as below
3.3v/3.0v: (2.7v, 3.6v)
1.8v: (1.7v, 1.95v)
Original code use the specific value which may fail in regulator
driver
Re: FW: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
Hello,
On 11/20/2012 3:14 PM, Kevin Liu wrote:
2012/11/20 Marek Szyprowski m.szyprow...@samsung.com:
Hello,
On 11/20/2012 12:36 PM, Kevin Liu wrote:
2012/11/20 Marek Szyprowski m.szyprow...@samsung.com:
On 11/20/2012 9:59 AM, Kevin Liu wrote:
2012/11/20 Marek Szyprowski m.szyprow...@samsung.com:
On 11/14/2012 8:11 AM, Kevin Liu wrote:
From: linux-mmc-ow...@vger.kernel.org
[mailto:linux-mmc-ow...@vger.kernel.org] On Behalf Of Chris Ball
Sent: Tuesday, November 13, 2012 10:14 PM
To: Marek Szyprowski
Cc: linux-ker...@vger.kernel.org; linux-mmc@vger.kernel.org;
Kyungmin
Park; Mark Brown; Liam Girdwood; Philip Rakity
Subject: Re: [PATCH v2] mmc: sdhci: apply voltage range check only
for non-fixed regulators
On Tue, Nov 13 2012, Marek Szyprowski wrote:
On Tue, Nov 13 2012, Marek Szyprowski wrote:
Fixed regulators cannot change their voltage, so disable all
voltage
range checking for them, otherwise the driver fails to operate
with
fixed regulators. Up to now it worked only by luck, because
regulator_is_supported_voltage() function returned incorrect
values.
Commit regulator: fix voltage check in
regulator_is_supported_voltage()
fixed that function and now additional check is needed for
fixed
regulators.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c
b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host
*host)
regulator_enable(host-vmmc);
#ifdef CONFIG_REGULATOR
- if (host-vmmc) {
+ if (host-vmmc regulator_count_voltages(host-vmmc) 1)
{
ret = regulator_is_supported_voltage(host-vmmc,
330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
Thanks for the longer explanation. I'm still missing something,
though;
what's wrong with running the check as it was with the new
regulator
code?
(I haven't tried it yet.)
#ifdef CONFIG_REGULATOR
if (host-vmmc) {
ret =
regulator_is_supported_voltage(host-vmmc,
330,
330);
if ((ret = 0) || (!(caps[0]
SDHCI_CAN_VDD_330)))
caps[0] = ~SDHCI_CAN_VDD_330;
ret =
regulator_is_supported_voltage(host-vmmc,
300,
300);
if ((ret = 0) || (!(caps[0]
SDHCI_CAN_VDD_300)))
caps[0] = ~SDHCI_CAN_VDD_300;
ret =
regulator_is_supported_voltage(host-vmmc,
180,
180);
if ((ret = 0) || (!(caps[0]
SDHCI_CAN_VDD_180)))
caps[0] = ~SDHCI_CAN_VDD_180;
}
#endif /* CONFIG_REGULATOR */
The point is to remove unsupported voltages, so if someone sets
up
a
fixed regulator at 330, all of the other caps are disabled.
Why
wouldn't that work without this change, and how are we supposed
to
remove those caps on a fixed regulator after your patchset?
Thanks, sorry if I'm missing something obvious,
On our boards eMMC is connected to fixed 2.8V regulator, what
results
in
clearing all available voltages and fail. The same situation is
when
one
enable dummy regulator and try to use sdhci with it. My patch
fixes
this
and restores sdhci to working state as it was before (before
fixing
regulator regulator_is_supported_voltage() function and earlier
when
MMC_BROKEN_VOLATGE capability was used).
I see. Sounds like a separate bug -- Philip (or anyone else), any
idea how we should be treating eMMCs with a fixed voltage here?
I think we should check the voltage range rather than the voltage
point accoring to the spec.
Otherwise some valid voltage like 2.8v will be discarded by mistake.
My below old patch aim to fix this issue.
How do you think?
-Original Message-
From: Kevin Liu [mailto:keyuan@gmail.com]
Sent: Friday, September 28, 2012 3:56 PM
To: linux-mmc@vger.kernel.org; c...@laptop.org; pie...@ossman.eu;
ulf.hans...@linaro.org; Zhangfei Gao
Cc: Haojian Zhuang; Chao Xie; Philip Rakity; Kevin Liu; Jialing Fu
Subject: [PATCH v5 03/13] mmc: sdhci: use regulator min/max voltage
range according to spec
From: Kevin Liu kl...@marvell.com
For regulator vmmc/vmmcq, use voltage range as below
3.3v/3.0v: (2.7v, 3.6v)
1.8v: (1.7v, 1.95v)
Original code use
Re: FW: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
2012/11/20 Marek Szyprowski m.szyprow...@samsung.com:
Hello,
On 11/20/2012 3:14 PM, Kevin Liu wrote:
2012/11/20 Marek Szyprowski m.szyprow...@samsung.com:
Hello,
On 11/20/2012 12:36 PM, Kevin Liu wrote:
2012/11/20 Marek Szyprowski m.szyprow...@samsung.com:
On 11/20/2012 9:59 AM, Kevin Liu wrote:
2012/11/20 Marek Szyprowski m.szyprow...@samsung.com:
On 11/14/2012 8:11 AM, Kevin Liu wrote:
From: linux-mmc-ow...@vger.kernel.org
[mailto:linux-mmc-ow...@vger.kernel.org] On Behalf Of Chris
Ball
Sent: Tuesday, November 13, 2012 10:14 PM
To: Marek Szyprowski
Cc: linux-ker...@vger.kernel.org; linux-mmc@vger.kernel.org;
Kyungmin
Park; Mark Brown; Liam Girdwood; Philip Rakity
Subject: Re: [PATCH v2] mmc: sdhci: apply voltage range check
only
for non-fixed regulators
On Tue, Nov 13 2012, Marek Szyprowski wrote:
On Tue, Nov 13 2012, Marek Szyprowski wrote:
Fixed regulators cannot change their voltage, so disable
all
voltage
range checking for them, otherwise the driver fails to
operate
with
fixed regulators. Up to now it worked only by luck, because
regulator_is_supported_voltage() function returned
incorrect
values.
Commit regulator: fix voltage check in
regulator_is_supported_voltage()
fixed that function and now additional check is needed for
fixed
regulators.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c
b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host
*host)
regulator_enable(host-vmmc);
#ifdef CONFIG_REGULATOR
- if (host-vmmc) {
+ if (host-vmmc regulator_count_voltages(host-vmmc)
1)
{
ret = regulator_is_supported_voltage(host-vmmc,
330,
330);
if ((ret = 0) || (!(caps[0]
SDHCI_CAN_VDD_330)))
Thanks for the longer explanation. I'm still missing
something,
though;
what's wrong with running the check as it was with the new
regulator
code?
(I haven't tried it yet.)
#ifdef CONFIG_REGULATOR
if (host-vmmc) {
ret =
regulator_is_supported_voltage(host-vmmc,
330,
330);
if ((ret = 0) || (!(caps[0]
SDHCI_CAN_VDD_330)))
caps[0] = ~SDHCI_CAN_VDD_330;
ret =
regulator_is_supported_voltage(host-vmmc,
300,
300);
if ((ret = 0) || (!(caps[0]
SDHCI_CAN_VDD_300)))
caps[0] = ~SDHCI_CAN_VDD_300;
ret =
regulator_is_supported_voltage(host-vmmc,
180,
180);
if ((ret = 0) || (!(caps[0]
SDHCI_CAN_VDD_180)))
caps[0] = ~SDHCI_CAN_VDD_180;
}
#endif /* CONFIG_REGULATOR */
The point is to remove unsupported voltages, so if someone
sets
up
a
fixed regulator at 330, all of the other caps are
disabled.
Why
wouldn't that work without this change, and how are we
supposed
to
remove those caps on a fixed regulator after your patchset?
Thanks, sorry if I'm missing something obvious,
On our boards eMMC is connected to fixed 2.8V regulator, what
results
in
clearing all available voltages and fail. The same situation
is
when
one
enable dummy regulator and try to use sdhci with it. My patch
fixes
this
and restores sdhci to working state as it was before (before
fixing
regulator regulator_is_supported_voltage() function and
earlier
when
MMC_BROKEN_VOLATGE capability was used).
I see. Sounds like a separate bug -- Philip (or anyone else),
any
idea how we should be treating eMMCs with a fixed voltage here?
I think we should check the voltage range rather than the voltage
point accoring to the spec.
Otherwise some valid voltage like 2.8v will be discarded by
mistake.
My below old patch aim to fix this issue.
How do you think?
-Original Message-
From: Kevin Liu [mailto:keyuan@gmail.com]
Sent: Friday, September 28, 2012 3:56 PM
To: linux-mmc@vger.kernel.org; c...@laptop.org; pie...@ossman.eu;
ulf.hans...@linaro.org; Zhangfei Gao
Cc: Haojian Zhuang; Chao Xie; Philip Rakity; Kevin Liu; Jialing
Fu
Re: FW: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
2012/11/14 Mark Brown broo...@opensource.wolfsonmicro.com: On Wed, Nov 14, 2012 at 03:11:37PM +0800, Kevin Liu wrote: - ret = regulator_set_voltage(host-vqmmc, 330, 330); + ret = regulator_set_voltage(host-vqmmc, 270, 360); Should this be regulator_set_voltage_tol()? Otherwise it'd be good to explain where the numbers come from. In SD physical layer spec 3.01 chapter 6.6.1, the threshold level for voltage range is defined as below: Vdd(min) = 2.7V while Vdd(max) = 3.6V. The card should work within the voltage range. If you are afraid the voltage value is too aggressive, maybe we can use regulator_set_voltage_tol() to set a smaller range. But which range should be reasonable? + ret = regulator_is_supported_voltage(host-vmmc, 170, + 195); We should really add a regulator_is_supported_voltage_tol... let me just do that. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: FW: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
On Wed, Nov 14, 2012 at 04:36:28PM +0800, Kevin Liu wrote: 2012/11/14 Mark Brown broo...@opensource.wolfsonmicro.com: Should this be regulator_set_voltage_tol()? Otherwise it'd be good to explain where the numbers come from. In SD physical layer spec 3.01 chapter 6.6.1, the threshold level for voltage range is defined as below: Vdd(min) = 2.7V while Vdd(max) = 3.6V. The card should work within the voltage range. If you are afraid the voltage value is too aggressive, maybe we can use regulator_set_voltage_tol() to set a smaller range. But which range should be reasonable? The above makes total sense - thanks! I just wasn't aware that the range was specified in this fashion in the spec. Might be worth a comment in the code if you need to respin. signature.asc Description: Digital signature
Re: FW: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
2012/11/14 Mark Brown broo...@opensource.wolfsonmicro.com: On Wed, Nov 14, 2012 at 04:36:28PM +0800, Kevin Liu wrote: 2012/11/14 Mark Brown broo...@opensource.wolfsonmicro.com: Should this be regulator_set_voltage_tol()? Otherwise it'd be good to explain where the numbers come from. In SD physical layer spec 3.01 chapter 6.6.1, the threshold level for voltage range is defined as below: Vdd(min) = 2.7V while Vdd(max) = 3.6V. The card should work within the voltage range. If you are afraid the voltage value is too aggressive, maybe we can use regulator_set_voltage_tol() to set a smaller range. But which range should be reasonable? The above makes total sense - thanks! I just wasn't aware that the range was specified in this fashion in the spec. Might be worth a comment in the code if you need to respin. Sure, I will update the patch. Thanks! -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
On Nov 14, 2012, at 8:57 AM, Kevin Liu keyuan@gmail.com wrote: 2012/11/14 Mark Brown broo...@opensource.wolfsonmicro.com: On Wed, Nov 14, 2012 at 04:36:28PM +0800, Kevin Liu wrote: 2012/11/14 Mark Brown broo...@opensource.wolfsonmicro.com: Should this be regulator_set_voltage_tol()? Otherwise it'd be good to explain where the numbers come from. In SD physical layer spec 3.01 chapter 6.6.1, the threshold level for voltage range is defined as below: Vdd(min) = 2.7V while Vdd(max) = 3.6V. The card should work within the voltage range. If you are afraid the voltage value is too aggressive, maybe we can use regulator_set_voltage_tol() to set a smaller range. But which range should be reasonable? The above makes total sense - thanks! I just wasn't aware that the range was specified in this fashion in the spec. Might be worth a comment in the code if you need to respin. Sure, I will update the patch. Thanks! -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html re-read spec. Please apply Kevin;s patch. Reviewed-by: Philip Rakity prak...@nvidia.com -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
2012/11/14 Philip Rakity prak...@nvidia.com: On Nov 14, 2012, at 8:57 AM, Kevin Liu keyuan@gmail.com wrote: 2012/11/14 Mark Brown broo...@opensource.wolfsonmicro.com: On Wed, Nov 14, 2012 at 04:36:28PM +0800, Kevin Liu wrote: 2012/11/14 Mark Brown broo...@opensource.wolfsonmicro.com: Should this be regulator_set_voltage_tol()? Otherwise it'd be good to explain where the numbers come from. In SD physical layer spec 3.01 chapter 6.6.1, the threshold level for voltage range is defined as below: Vdd(min) = 2.7V while Vdd(max) = 3.6V. The card should work within the voltage range. If you are afraid the voltage value is too aggressive, maybe we can use regulator_set_voltage_tol() to set a smaller range. But which range should be reasonable? The above makes total sense - thanks! I just wasn't aware that the range was specified in this fashion in the spec. Might be worth a comment in the code if you need to respin. Sure, I will update the patch. Thanks! -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html re-read spec. Please apply Kevin;s patch. Reviewed-by: Philip Rakity prak...@nvidia.com Philip, thanks! -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
Fixed regulators cannot change their voltage, so disable all voltage
range checking for them, otherwise the driver fails to operate with
fixed regulators. Up to now it worked only by luck, because
regulator_is_supported_voltage() function returned incorrect values.
Commit regulator: fix voltage check in regulator_is_supported_voltage()
fixed that function and now additional check is needed for fixed
regulators.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host *host)
regulator_enable(host-vmmc);
#ifdef CONFIG_REGULATOR
- if (host-vmmc) {
+ if (host-vmmc regulator_count_voltages(host-vmmc) 1) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
Hi Marek,
On Tue, Nov 13 2012, Marek Szyprowski wrote:
Fixed regulators cannot change their voltage, so disable all voltage
range checking for them, otherwise the driver fails to operate with
fixed regulators. Up to now it worked only by luck, because
regulator_is_supported_voltage() function returned incorrect values.
Commit regulator: fix voltage check in regulator_is_supported_voltage()
fixed that function and now additional check is needed for fixed
regulators.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host *host)
regulator_enable(host-vmmc);
#ifdef CONFIG_REGULATOR
- if (host-vmmc) {
+ if (host-vmmc regulator_count_voltages(host-vmmc) 1) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
Thanks for the longer explanation. I'm still missing something, though;
what's wrong with running the check as it was with the new regulator code?
(I haven't tried it yet.)
#ifdef CONFIG_REGULATOR
if (host-vmmc) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
caps[0] = ~SDHCI_CAN_VDD_330;
ret = regulator_is_supported_voltage(host-vmmc, 300,
300);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_300)))
caps[0] = ~SDHCI_CAN_VDD_300;
ret = regulator_is_supported_voltage(host-vmmc, 180,
180);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_180)))
caps[0] = ~SDHCI_CAN_VDD_180;
}
#endif /* CONFIG_REGULATOR */
The point is to remove unsupported voltages, so if someone sets up a
fixed regulator at 330, all of the other caps are disabled. Why
wouldn't that work without this change, and how are we supposed to
remove those caps on a fixed regulator after your patchset?
Thanks, sorry if I'm missing something obvious,
- Chris.
--
Chris Ball c...@laptop.org http://printf.net/
One Laptop Per Child
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
Hello,
On 11/13/2012 2:45 PM, Chris Ball wrote:
Hi Marek,
On Tue, Nov 13 2012, Marek Szyprowski wrote:
Fixed regulators cannot change their voltage, so disable all voltage
range checking for them, otherwise the driver fails to operate with
fixed regulators. Up to now it worked only by luck, because
regulator_is_supported_voltage() function returned incorrect values.
Commit regulator: fix voltage check in regulator_is_supported_voltage()
fixed that function and now additional check is needed for fixed
regulators.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host *host)
regulator_enable(host-vmmc);
#ifdef CONFIG_REGULATOR
- if (host-vmmc) {
+ if (host-vmmc regulator_count_voltages(host-vmmc) 1) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
Thanks for the longer explanation. I'm still missing something, though;
what's wrong with running the check as it was with the new regulator code?
(I haven't tried it yet.)
#ifdef CONFIG_REGULATOR
if (host-vmmc) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
caps[0] = ~SDHCI_CAN_VDD_330;
ret = regulator_is_supported_voltage(host-vmmc, 300,
300);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_300)))
caps[0] = ~SDHCI_CAN_VDD_300;
ret = regulator_is_supported_voltage(host-vmmc, 180,
180);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_180)))
caps[0] = ~SDHCI_CAN_VDD_180;
}
#endif /* CONFIG_REGULATOR */
The point is to remove unsupported voltages, so if someone sets up a
fixed regulator at 330, all of the other caps are disabled. Why
wouldn't that work without this change, and how are we supposed to
remove those caps on a fixed regulator after your patchset?
Thanks, sorry if I'm missing something obvious,
On our boards eMMC is connected to fixed 2.8V regulator, what results in
clearing all available voltages and fail. The same situation is when one
enable dummy regulator and try to use sdhci with it. My patch fixes this
and restores sdhci to working state as it was before (before fixing
regulator regulator_is_supported_voltage() function and earlier when
MMC_BROKEN_VOLATGE capability was used).
Best regards
--
Marek Szyprowski
Samsung Poland RD Center
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
Hi,
On Tue, Nov 13 2012, Marek Szyprowski wrote:
On Tue, Nov 13 2012, Marek Szyprowski wrote:
Fixed regulators cannot change their voltage, so disable all voltage
range checking for them, otherwise the driver fails to operate with
fixed regulators. Up to now it worked only by luck, because
regulator_is_supported_voltage() function returned incorrect values.
Commit regulator: fix voltage check in regulator_is_supported_voltage()
fixed that function and now additional check is needed for fixed
regulators.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host *host)
regulator_enable(host-vmmc);
#ifdef CONFIG_REGULATOR
- if (host-vmmc) {
+ if (host-vmmc regulator_count_voltages(host-vmmc) 1) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
Thanks for the longer explanation. I'm still missing something, though;
what's wrong with running the check as it was with the new regulator code?
(I haven't tried it yet.)
#ifdef CONFIG_REGULATOR
if (host-vmmc) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
caps[0] = ~SDHCI_CAN_VDD_330;
ret = regulator_is_supported_voltage(host-vmmc, 300,
300);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_300)))
caps[0] = ~SDHCI_CAN_VDD_300;
ret = regulator_is_supported_voltage(host-vmmc, 180,
180);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_180)))
caps[0] = ~SDHCI_CAN_VDD_180;
}
#endif /* CONFIG_REGULATOR */
The point is to remove unsupported voltages, so if someone sets up a
fixed regulator at 330, all of the other caps are disabled. Why
wouldn't that work without this change, and how are we supposed to
remove those caps on a fixed regulator after your patchset?
Thanks, sorry if I'm missing something obvious,
On our boards eMMC is connected to fixed 2.8V regulator, what results in
clearing all available voltages and fail. The same situation is when one
enable dummy regulator and try to use sdhci with it. My patch fixes this
and restores sdhci to working state as it was before (before fixing
regulator regulator_is_supported_voltage() function and earlier when
MMC_BROKEN_VOLATGE capability was used).
I see. Sounds like a separate bug -- Philip (or anyone else), any
idea how we should be treating eMMCs with a fixed voltage here?
Thanks,
- Chris.
--
Chris Ball c...@laptop.org http://printf.net/
One Laptop Per Child
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
Hi Marek,
Is the regulator dedicated ? or is it shared ? Is it used for eMMC ?
If it cannot be turned off -- then just don't list it in the regulators list
for vmmc.
If it CAN be turned off then need to get back to you.
Philip
On Nov 13, 2012, at 2:14 PM, Chris Ball c...@laptop.org wrote:
Hi,
On Tue, Nov 13 2012, Marek Szyprowski wrote:
On Tue, Nov 13 2012, Marek Szyprowski wrote:
Fixed regulators cannot change their voltage, so disable all voltage
range checking for them, otherwise the driver fails to operate with
fixed regulators. Up to now it worked only by luck, because
regulator_is_supported_voltage() function returned incorrect values.
Commit regulator: fix voltage check in regulator_is_supported_voltage()
fixed that function and now additional check is needed for fixed
regulators.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host *host)
regulator_enable(host-vmmc);
#ifdef CONFIG_REGULATOR
- if (host-vmmc) {
+ if (host-vmmc regulator_count_voltages(host-vmmc) 1) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
Thanks for the longer explanation. I'm still missing something, though;
what's wrong with running the check as it was with the new regulator code?
(I haven't tried it yet.)
#ifdef CONFIG_REGULATOR
if (host-vmmc) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
caps[0] = ~SDHCI_CAN_VDD_330;
ret = regulator_is_supported_voltage(host-vmmc, 300,
300);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_300)))
caps[0] = ~SDHCI_CAN_VDD_300;
ret = regulator_is_supported_voltage(host-vmmc, 180,
180);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_180)))
caps[0] = ~SDHCI_CAN_VDD_180;
}
#endif /* CONFIG_REGULATOR */
The point is to remove unsupported voltages, so if someone sets up a
fixed regulator at 330, all of the other caps are disabled. Why
wouldn't that work without this change, and how are we supposed to
remove those caps on a fixed regulator after your patchset?
Thanks, sorry if I'm missing something obvious,
On our boards eMMC is connected to fixed 2.8V regulator, what results in
clearing all available voltages and fail. The same situation is when one
enable dummy regulator and try to use sdhci with it. My patch fixes this
and restores sdhci to working state as it was before (before fixing
regulator regulator_is_supported_voltage() function and earlier when
MMC_BROKEN_VOLATGE capability was used).
I see. Sounds like a separate bug -- Philip (or anyone else), any
idea how we should be treating eMMCs with a fixed voltage here?
Thanks,
- Chris.
--
Chris Ball c...@laptop.org http://printf.net/
One Laptop Per Child
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: FW: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
From: linux-mmc-ow...@vger.kernel.org
[mailto:linux-mmc-ow...@vger.kernel.org] On Behalf Of Chris Ball
Sent: Tuesday, November 13, 2012 10:14 PM
To: Marek Szyprowski
Cc: linux-ker...@vger.kernel.org; linux-mmc@vger.kernel.org; Kyungmin Park;
Mark Brown; Liam Girdwood; Philip Rakity
Subject: Re: [PATCH v2] mmc: sdhci: apply voltage range check only for
non-fixed regulators
Hi,
On Tue, Nov 13 2012, Marek Szyprowski wrote:
On Tue, Nov 13 2012, Marek Szyprowski wrote:
Fixed regulators cannot change their voltage, so disable all voltage
range checking for them, otherwise the driver fails to operate with
fixed regulators. Up to now it worked only by luck, because
regulator_is_supported_voltage() function returned incorrect values.
Commit regulator: fix voltage check in regulator_is_supported_voltage()
fixed that function and now additional check is needed for fixed
regulators.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host *host)
regulator_enable(host-vmmc);
#ifdef CONFIG_REGULATOR
- if (host-vmmc) {
+ if (host-vmmc regulator_count_voltages(host-vmmc) 1) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
Thanks for the longer explanation. I'm still missing something, though;
what's wrong with running the check as it was with the new regulator code?
(I haven't tried it yet.)
#ifdef CONFIG_REGULATOR
if (host-vmmc) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
caps[0] = ~SDHCI_CAN_VDD_330;
ret = regulator_is_supported_voltage(host-vmmc, 300,
300);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_300)))
caps[0] = ~SDHCI_CAN_VDD_300;
ret = regulator_is_supported_voltage(host-vmmc, 180,
180);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_180)))
caps[0] = ~SDHCI_CAN_VDD_180;
}
#endif /* CONFIG_REGULATOR */
The point is to remove unsupported voltages, so if someone sets up a
fixed regulator at 330, all of the other caps are disabled. Why
wouldn't that work without this change, and how are we supposed to
remove those caps on a fixed regulator after your patchset?
Thanks, sorry if I'm missing something obvious,
On our boards eMMC is connected to fixed 2.8V regulator, what results in
clearing all available voltages and fail. The same situation is when one
enable dummy regulator and try to use sdhci with it. My patch fixes this
and restores sdhci to working state as it was before (before fixing
regulator regulator_is_supported_voltage() function and earlier when
MMC_BROKEN_VOLATGE capability was used).
I see. Sounds like a separate bug -- Philip (or anyone else), any
idea how we should be treating eMMCs with a fixed voltage here?
I think we should check the voltage range rather than the voltage
point accoring to the spec.
Otherwise some valid voltage like 2.8v will be discarded by mistake.
My below old patch aim to fix this issue.
How do you think?
-Original Message-
From: Kevin Liu [mailto:keyuan@gmail.com]
Sent: Friday, September 28, 2012 3:56 PM
To: linux-mmc@vger.kernel.org; c...@laptop.org; pie...@ossman.eu;
ulf.hans...@linaro.org; Zhangfei Gao
Cc: Haojian Zhuang; Chao Xie; Philip Rakity; Kevin Liu; Jialing Fu
Subject: [PATCH v5 03/13] mmc: sdhci: use regulator min/max voltage
range according to spec
From: Kevin Liu kl...@marvell.com
For regulator vmmc/vmmcq, use voltage range as below
3.3v/3.0v: (2.7v, 3.6v)
1.8v: (1.7v, 1.95v)
Original code use the specific value which may fail in regulator
driver if it does NOT support the specific voltage.
Signed-off-by: Jialing Fu j...@marvell.com
Signed-off-by: Kevin Liu kl...@marvell.com
---
drivers/mmc/host/sdhci.c | 16 +++-
1 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 3aef580..36afd47 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1628,7 +1628,7 @@ static int
sdhci_do_3_3v_signal_voltage_switch(struct sdhci_host *host,
sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
if (host-vqmmc) {
- ret = regulator_set_voltage(host-vqmmc, 330, 330);
+ ret = regulator_set_voltage(host-vqmmc, 270, 360
Re: FW: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
On Wed, Nov 14, 2012 at 03:11:37PM +0800, Kevin Liu wrote: - ret = regulator_set_voltage(host-vqmmc, 330, 330); + ret = regulator_set_voltage(host-vqmmc, 270, 360); Should this be regulator_set_voltage_tol()? Otherwise it'd be good to explain where the numbers come from. + ret = regulator_is_supported_voltage(host-vmmc, 170, + 195); We should really add a regulator_is_supported_voltage_tol... let me just do that. signature.asc Description: Digital signature
