Re: UHS-I voltage switching on OLPC XO-1.75
On Tue, Nov 20, 2012 at 2:53 AM, Johan Rudholm jrudh...@gmail.com wrote: On the other hand, we may have a good basis to add a quirk, triggered by the device tree, for when the hardware physically does not have 1.8v capabilities. This also seems proper, if we know we can't get 1.8V, then we shouldn't even try for it. In this way the detection will also be faster (no 10 retries). I've gone for this approach in the patches just posted. sdhci: add quirk for lack of 1.8v support mmc: add no-1-8-v device tree flag If this is the case, the driver could have another heuristic: if there is no vmmc regulator, there is no way of cutting the card power, therefore we could avoid even trying 1.8v on the basis that we know we can't recover if things go wrong. So the driver could for instance drop the MMC_CAP_UHS_DDR50 cap if there is no way to power cycle the card? I think that sounds reasonable and according to spec, although also a little bit hard since there probably are cards out there that never require a power cycle to perform a proper voltage switch? Thats true, and actually, after some more investigation it doesn't seem like we have a solid basis to add such behaviour. It looks like whether or not the voltage supply is supplied by the same chip that provides the SDHCI interface, or whether it is a purely external factor, varies from board to board. So we can't assume a lack of regulator means a lack of ability to switch voltages. Thanks Daniel -- 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: UHS-I voltage switching on OLPC XO-1.75
Hi! 2012/11/18 Daniel Drake d...@laptop.org: On Fri, Nov 2, 2012 at 8:35 AM, Johan Rudholm jrudh...@gmail.com wrote: Good question. I’d guess that mmc_power_off/up does not work as expected here, that the card is not at all power cycled. Before going further on the find a way to quirk it route, there is something else we could look into. According to our hardware engineers, the external SD card power has been always-on until now. It is actually controlled by our embedded controller, separate from the CPU. In a test firmware, I can now control the SD card power via our OLPC EC interface, I call into that from mmc_power_up and mmc_power_down. And, with your hacky patch to make the voltage switch failure detection work, this fixes it: it tries 10 times at 1.8v then falls back to 3.3 successfully. No more problems with the power cycle. Ah, great! Then we know what the issue was at least. Then I guess that this code did not work with your driver: host-ios.clock = 0; mmc_set_ios(host); so with my original patch ([RFC/PATCH,v2] mmc: core: Fixup signal voltage switch), the clock was actually never stopped during the signal voltage switch? I guess this will need some further investigation also. So we have the option of fixing it that way: if we can fix the voltage switch failure detection, we could implement a custom vmmc regulator driver that uses our EC interface to enable and disable the SD power appropriately, solving our ability to power cycle. Being able to power cycle the SD-card might also come in handy in other situations, perhaps if a poor SD-card misbehaves in some way? SD-cards have no reset cord like eMMCs, so being able to power cycle the card feels like a good thing. On the other hand, we may have a good basis to add a quirk, triggered by the device tree, for when the hardware physically does not have 1.8v capabilities. This also seems proper, if we know we can't get 1.8V, then we shouldn't even try for it. In this way the detection will also be faster (no 10 retries). I'm also curious if there is a 3rd option. It seems like in the case of our SoC, the SoC design mandates that the SD card power is separate from the SDHCI interface - requiring either a GPIO or some other mechanism (e.g. OLPC EC) to be able to control it. I'm wondering if this is the same for all sdhci-pxa devices. And the same for all sdhci devices? Maybe the SDHCI specs would help here, but I guess they aren't public. If this is the case, the driver could have another heuristic: if there is no vmmc regulator, there is no way of cutting the card power, therefore we could avoid even trying 1.8v on the basis that we know we can't recover if things go wrong. So the driver could for instance drop the MMC_CAP_UHS_DDR50 cap if there is no way to power cycle the card? I think that sounds reasonable and according to spec, although also a little bit hard since there probably are cards out there that never require a power cycle to perform a proper voltage switch? Kind regards, Johan -- 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: UHS-I voltage switching on OLPC XO-1.75
On Fri, Nov 2, 2012 at 8:35 AM, Johan Rudholm jrudh...@gmail.com wrote: Good question. I’d guess that mmc_power_off/up does not work as expected here, that the card is not at all power cycled. Before going further on the find a way to quirk it route, there is something else we could look into. According to our hardware engineers, the external SD card power has been always-on until now. It is actually controlled by our embedded controller, separate from the CPU. In a test firmware, I can now control the SD card power via our OLPC EC interface, I call into that from mmc_power_up and mmc_power_down. And, with your hacky patch to make the voltage switch failure detection work, this fixes it: it tries 10 times at 1.8v then falls back to 3.3 successfully. No more problems with the power cycle. So we have the option of fixing it that way: if we can fix the voltage switch failure detection, we could implement a custom vmmc regulator driver that uses our EC interface to enable and disable the SD power appropriately, solving our ability to power cycle. On the other hand, we may have a good basis to add a quirk, triggered by the device tree, for when the hardware physically does not have 1.8v capabilities. I'm also curious if there is a 3rd option. It seems like in the case of our SoC, the SoC design mandates that the SD card power is separate from the SDHCI interface - requiring either a GPIO or some other mechanism (e.g. OLPC EC) to be able to control it. I'm wondering if this is the same for all sdhci-pxa devices. And the same for all sdhci devices? Maybe the SDHCI specs would help here, but I guess they aren't public. If this is the case, the driver could have another heuristic: if there is no vmmc regulator, there is no way of cutting the card power, therefore we could avoid even trying 1.8v on the basis that we know we can't recover if things go wrong. Similarly, for our next product we are looking at adding 1.8v capabilities. However, it seems that again, the SoC design (or something more fundamental?) requires that this power is controlled via some external mechanism - we're planning to use a GPIO to switch between 1.8v and 3.3v, which can be exposed as a regulator. So again, would it be fair for sdhci-pxa or sdhci to drop the UHS-I support when no regulator is present? Or am I over-generalizing? Thanks, Daniel -- 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: UHS-I voltage switching on OLPC XO-1.75
On Fri, Nov 2, 2012 at 8:35 AM, Johan Rudholm jrudh...@gmail.com wrote: An initial question, would it be possible to solve this by disabling the UHS support in the host cap, or through a quirk? Maybe this kind of solution is not applicable in this scenario? I wonder this because I guess it will be very hard to know how a card will react if we first tell it that we're going to switch voltages and then don't, since this is out of spec. That would definitely be possible. I'm not convinced this is outside the spec - the spec talks heavily about power cycling, which you would think would handle such cases (i.e. a full state reset). However, I think I have generated enough evidence in my last mail to show that we are not able to fully power cycle, giving further support for some kind of quirk. Good question. I’d guess that mmc_power_off/up does not work as expected here, that the card is not at all power cycled. But it seems that the power cycle code in sdhci_do_start_signal_voltage_switch works? What if we export a couple of debug functions from sdhci.c which allow us to power cycle and control the clock just like sdhci does, and call these functions from core.c and sd.c? If this works (and the failure is detected properly by the core layer, and 10 retries are made etc), then we know that the problems most likely depend on how mmc_set_ios and mmc_power_off/up work with sdhci. I've attached a patch suggesting this, if you'd like to give it a try (warning: the patch has not been tested). The patch does not do anything about pm_runtime, but perhaps we don't need to worry about this here. Your patch fixes the detection that the voltage switch has failed. It doesn't fix the reset though - it still returns the -110 error after trying to power cycle. I mentioned in an earlier mail that we had this working in 3.0. I went back to that configuration to investigate. Actually, it is maybe only reliable 50% of the time - the other 50% we get the same error. When it does work, here's what happens: - mmc_rescan_try_freq is called to try the 400khz frequency - in mmc_sd_get_cid(), rocr has value c1ff8000, so we try the 1.8 switch, this fails - we go back to 3.3v, then we receive the -110 timeout error immediately after - mmc_rescan_try_freq is called to try the 300khz frequency - the card is now alive for some reason - in mmc_sd_get_cid(), rocr has value c0ff8000, so we stick at 3.3V Rather odd. It seems that 3.0 is more successful at power cycling the card than 3.5, but it is far from reliable. It also appears that the card I have here might have some logic to realise that 1.8V failed so then it stops advertising the capability? Anyway, with all the unreliability, unknowns, and power cycle difficulty, I guess we should go for the quirk approach. Do you have any suggestions for how this should be implemented? Should I go for a new SDHCI quirk triggered by a new sdhci,disable-1-8v device tree property? Thanks Daniel -- 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: UHS-I voltage switching on OLPC XO-1.75
On Thu, Nov 8, 2012 at 12:48 PM, Philip Rakity prak...@nvidia.com wrote: This is already there. Use broken caps and in the board code clear out the nits in caps2. There is no board code; we're working with device tree. So I'm still not exactly clear on the best way to work this. Maybe something like this: sdhci,disable-1-8v property in the device tree, would trigger the sdhci-pxa2 code to: 1. sdhci_readl() to retrieve caps and caps1 2. Drop the 1.8v flag from caps1 3. Set the BROKEN_CAPS quirk It would work but I'm not too fond of it, and I can't really think of an approach I do like (except maybe having a specific 1.8v broken sdhci quirk). Daniel -- 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: UHS-I voltage switching on OLPC XO-1.75
On Thu, Nov 8, 2012 at 1:03 PM, Philip Rakity prak...@nvidia.com wrote: Don't know enough about the device tree to help. Originally I suggested a quirk simular to what you are suggesting but it was felt to be too specific since the bits in caps2 achieve the same effect. Maybe your board specific code can check if the regulator is capable of switching and can be turned on and off. Not sure how to do the later. Checking the usage count does not help. You want exclusive access. Mark Brown might know how to do this else perhaps the regulator framework needs extending. We don't have a regulator in this case. If we did we'd probably be in a different situation. An option closer to overriding caps/caps1 in the board code is to put the caps/caps1 overrides in the device tree directly. Doesn't sound pleasing either, and I'm not sure that it makes sense; really the hardware's reporting of its own capabilities is not wrong, it just does not consider the capabilities of the surrounding components on the motherboard. So a quirk feels more natural to me... Daniel -- 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: UHS-I voltage switching on OLPC XO-1.75
On Nov 8, 2012, at 7:03 PM, Philip Rakity prak...@nvidia.com wrote: Don't know enough about the device tree to help. Originally I suggested a quirk simular to what you are suggesting but it was felt to be too specific since the bits in caps2 achieve the same effect. Maybe your board specific code can check if the regulator is capable of switching and can be turned on and off. Not sure how to do the later. Checking the usage count does not help. You want exclusive access. Mark Brown might know how to do this else perhaps the regulator framework needs extending. upon reflection you can configure the regulator supply with the name of the specific device. That should make it exclusive to the mmc subsystem. In the general case I do not know how one can test for this (in say sdhci.c). - Reply message - From: Daniel Drake d...@laptop.org To: Philip Rakity prak...@nvidia.com Cc: Johan Rudholm jrudh...@gmail.com, Chris Ball c...@laptop.org, linux-mmc@vger.kernel.org linux-mmc@vger.kernel.org Subject: UHS-I voltage switching on OLPC XO-1.75 Date: Thu, Nov 8, 2012 19:52 On Thu, Nov 8, 2012 at 12:48 PM, Philip Rakity prak...@nvidia.com wrote: This is already there. Use broken caps and in the board code clear out the nits in caps2. There is no board code; we're working with device tree. So I'm still not exactly clear on the best way to work this. Maybe something like this: sdhci,disable-1-8v property in the device tree, would trigger the sdhci-pxa2 code to: 1. sdhci_readl() to retrieve caps and caps1 2. Drop the 1.8v flag from caps1 3. Set the BROKEN_CAPS quirk It would work but I'm not too fond of it, and I can't really think of an approach I do like (except maybe having a specific 1.8v broken sdhci quirk). Daniel --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- 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: UHS-I voltage switching on OLPC XO-1.75
Hi Daneil, Chris, I reviewed kevin's patch in September which fixes this issue. Chris -- can we pull it into mmc-next ? This patch is okay as a standalone change. From: Philip Rakity prakity at marvell.com Subject: Re: [PATCH v2 5/8] mmc: sdhci: fix null return check of regulator_get Newsgroups: gmane.linux.kernel.mmc Date: 2012-09-24 15:02:34 GMT (5 weeks, 6 days, 21 hours and 44 minutes ago) Philip On Oct 30, 2012, at 9:11 PM, Daniel Drake d...@laptop.org wrote: Hi, Following on from the recent thread [PATCH 2/2] mmc: core: Power cycle card on voltage switch fail I have tried to get a better grip on the problems being faced. Test setup: OLPC XO-1.75 laptop, based on Marvell MMP2 ARM SoC (includes sdhci-pxa interface) 32GB Sandisk Ultra SD card being inserted into external SD slot The SoC apparently has support for 1.8V, but we physically do not have the right power lines wired on the motherboard, so we need to detect the 1.8V failure and go back to 3.3V mode. Before patching anything, inserting this card results in: sdhci: Switching to 1.8V signalling voltage failed, retrying with S18R set to 0 mmc0: error -110 whilst initialising SD card Now, working on Linux-3.5.4, I add these patches: mmc: core: reset sigal voltage on power up [RFC/PATCH,v2] mmc: core: Fixup signal voltage switch regulator: add missing defintion regulator_is_supported_voltage mmc: sdhci: Add regulator support for vccq (voltage regualor) mmc: sdhci: Let core handle UHS switch failure I also tweaked the sdhci code so that UHS-I flags are not unconditionally disabled by the vccq commit (as explained in sdhci vccq regulator support drops UHS-I flags). Under this setup, the first problem encountered is that the system no longer identifies the fact that the 1.8V voltage failed. Things are not happy afterwards. The card_busy check in sdhci is saying that the card is not busy. Full kernel logs: http://dev.laptop.org/~dsd/20121030/mmc1.log I tracked down what causes this regression. In order to get the failure detection working again, I have to make the following change in mmc_set_signal_voltage(): if (cmd11) { host-ios.clock = 0; - mmc_set_ios(host); + //mmc_set_ios(host); } (i.e. don't mess with ios before asking sdhci to do the voltage switch) and at the top of sdhci_do_1_8v_signal_voltage_switch: + /* Stop SDCLK */ + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); + clk = ~SDHCI_CLOCK_CARD_EN; + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); (i.e. add the equivalent clock disabling code here) With that change, I now get: mmc2: Signal voltage switch failed, power cycling card (retries = 10) mmc2: error -110 whilst initialising SD card The -110 error comes from mmc_send_app_op_cond(), but its worth noting that mmc_send_if_cond() (called immediately above) silently failed with -110 too. Neither of these failed the first time around before 1.8V was tried. Full patch that I'm testing with: http://dev.laptop.org/~dsd/20121030/mmc.patch So, now the question is: why is the card not responding to if_cond/app_op_cond after being powered down and up again? Is the power down/up sequence OK? I tried inserting the mmc_power_off, mmc_power_up, mmc_send_if_cond sequence into various points in the codepaths being discussed. Measuring success as the card responding to mmc_send_if_cond(0x30080) without error (0x30080 is a known good value from when before 1.8V is tried), I find that this reset sequence works just fine up until the point when CMD11 is run. CMD11 is sent and returns successfully without error, but from this point, running the reset sequence will fail (send_if_cond will fail with -110). Is this kind of test valid? Does this suggest that something is wrong with the host controller hardware? My assumption is that whatever state is modified by CMD11 should be erased here, and of course the hope is that mmc_power_off and mmc_power_up will do a full power cycle anyway. But whatever is happening, it looks like the effects of CMD11 are persisting past the reset sequence and are breaking later communication. In the mail last week, I think we found some kind of configuration (on Linux-3.0) where this same setup successfully detected the 1.8V switching failure, and successfully switched back to 3.3V. I plan to go back and test that, maybe there is some kind of sequence that doesn't make the hardware die. Daniel -- 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 --- This email message is for the sole use of the intended recipient(s) and may contain confidential information.
Re: UHS-I voltage switching on OLPC XO-1.75
Hi Daneil, Chris, I reviewed kevin's patch in September which fixes this issue. Chris -- can we pull it into mmc-next ? This patch is okay as a standalone change. From: Philip Rakity prakity at marvell.com Subject: Re: [PATCH v2 5/8] mmc: sdhci: fix null return check of regulator_get Newsgroups: gmane.linux.kernel.mmc Date: 2012-09-24 15:02:34 GMT (5 weeks, 6 days, 21 hours and 44 minutes ago) Philip On Oct 30, 2012, at 9:11 PM, Daniel Drake d...@laptop.org wrote: Hi, Following on from the recent thread [PATCH 2/2] mmc: core: Power cycle card on voltage switch fail I have tried to get a better grip on the problems being faced. Test setup: OLPC XO-1.75 laptop, based on Marvell MMP2 ARM SoC (includes sdhci-pxa interface) 32GB Sandisk Ultra SD card being inserted into external SD slot The SoC apparently has support for 1.8V, but we physically do not have the right power lines wired on the motherboard, so we need to detect the 1.8V failure and go back to 3.3V mode. Before patching anything, inserting this card results in: sdhci: Switching to 1.8V signalling voltage failed, retrying with S18R set to 0 mmc0: error -110 whilst initialising SD card Now, working on Linux-3.5.4, I add these patches: mmc: core: reset sigal voltage on power up [RFC/PATCH,v2] mmc: core: Fixup signal voltage switch regulator: add missing defintion regulator_is_supported_voltage mmc: sdhci: Add regulator support for vccq (voltage regualor) mmc: sdhci: Let core handle UHS switch failure I also tweaked the sdhci code so that UHS-I flags are not unconditionally disabled by the vccq commit (as explained in sdhci vccq regulator support drops UHS-I flags). Under this setup, the first problem encountered is that the system no longer identifies the fact that the 1.8V voltage failed. Things are not happy afterwards. The card_busy check in sdhci is saying that the card is not busy. Full kernel logs: http://dev.laptop.org/~dsd/20121030/mmc1.log I tracked down what causes this regression. In order to get the failure detection working again, I have to make the following change in mmc_set_signal_voltage(): if (cmd11) { host-ios.clock = 0; - mmc_set_ios(host); + //mmc_set_ios(host); } (i.e. don't mess with ios before asking sdhci to do the voltage switch) and at the top of sdhci_do_1_8v_signal_voltage_switch: + /* Stop SDCLK */ + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); + clk = ~SDHCI_CLOCK_CARD_EN; + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); (i.e. add the equivalent clock disabling code here) With that change, I now get: mmc2: Signal voltage switch failed, power cycling card (retries = 10) mmc2: error -110 whilst initialising SD card The -110 error comes from mmc_send_app_op_cond(), but its worth noting that mmc_send_if_cond() (called immediately above) silently failed with -110 too. Neither of these failed the first time around before 1.8V was tried. Full patch that I'm testing with: http://dev.laptop.org/~dsd/20121030/mmc.patch So, now the question is: why is the card not responding to if_cond/app_op_cond after being powered down and up again? Is the power down/up sequence OK? I tried inserting the mmc_power_off, mmc_power_up, mmc_send_if_cond sequence into various points in the codepaths being discussed. Measuring success as the card responding to mmc_send_if_cond(0x30080) without error (0x30080 is a known good value from when before 1.8V is tried), I find that this reset sequence works just fine up until the point when CMD11 is run. CMD11 is sent and returns successfully without error, but from this point, running the reset sequence will fail (send_if_cond will fail with -110). Is this kind of test valid? Does this suggest that something is wrong with the host controller hardware? My assumption is that whatever state is modified by CMD11 should be erased here, and of course the hope is that mmc_power_off and mmc_power_up will do a full power cycle anyway. But whatever is happening, it looks like the effects of CMD11 are persisting past the reset sequence and are breaking later communication. In the mail last week, I think we found some kind of configuration (on Linux-3.0) where this same setup successfully detected the 1.8V switching failure, and successfully switched back to 3.3V. I plan to go back and test that, maybe there is some kind of sequence that doesn't make the hardware die. Daniel -- 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 --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized
Re: UHS-I voltage switching on OLPC XO-1.75
Hi, On Mon, Nov 05 2012, Philip Rakity wrote: Hi Daneil, Chris, I reviewed kevin's patch in September which fixes this issue. Chris -- can we pull it into mmc-next ? This patch is okay as a standalone change. From: Philip Rakity prakity at marvell.com Subject: Re: [PATCH v2 5/8] mmc: sdhci: fix null return check of regulator_get Newsgroups: gmane.linux.kernel.mmc Date: 2012-09-24 15:02:34 GMT (5 weeks, 6 days, 21 hours and 44 minutes ago) It sounds like I misread this patch, then -- I understood it as only affecting whether a pr_info() call is made, which would not fix a voltage switching bug. What am I missing? (Dan, here's a copy of Kevin's patch.) From: Kevin Liu kl...@marvell.com regulator_get() returns NULL when CONFIG_REGULATOR not defined, which should not print out the warning. Reviewed-by: Philip Rakity prak...@marvell.com Signed-off-by: Bin Wang b...@marvell.com Signed-off-by: Kevin Liu kl...@marvell.com --- drivers/mmc/host/sdhci.c | 18 -- 1 files changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 8e6a6f0..0104ae9 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -2845,9 +2845,12 @@ int sdhci_add_host(struct sdhci_host *host) /* If vqmmc regulator and no 1.8V signalling, then there's no UHS */ host-vqmmc = regulator_get(mmc_dev(mmc), vqmmc); - if (IS_ERR(host-vqmmc)) { - pr_info(%s: no vqmmc regulator found\n, mmc_hostname(mmc)); - host-vqmmc = NULL; + if (IS_ERR_OR_NULL(host-vqmmc)) { + if (PTR_ERR(host-vqmmc) 0) { + pr_info(%s: no vqmmc regulator found\n, + mmc_hostname(mmc)); + host-vqmmc = NULL; + } } else if (regulator_is_supported_voltage(host-vqmmc, 180, 180)) regulator_enable(host-vqmmc); @@ -2903,9 +2906,12 @@ int sdhci_add_host(struct sdhci_host *host) ocr_avail = 0; host-vmmc = regulator_get(mmc_dev(mmc), vmmc); - if (IS_ERR(host-vmmc)) { - pr_info(%s: no vmmc regulator found\n, mmc_hostname(mmc)); - host-vmmc = NULL; + if (IS_ERR_OR_NULL(host-vmmc)) { + if (PTR_ERR(host-vmmc) 0) { + pr_info(%s: no vmmc regulator found\n, + mmc_hostname(mmc)); + host-vmmc = NULL; + } } else regulator_enable(host-vmmc); -- 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: UHS-I voltage switching on OLPC XO-1.75
On Nov 5, 2012, at 1:11 PM, Chris Ball c...@laptop.org wrote: Hi, On Mon, Nov 05 2012, Philip Rakity wrote: Hi Daneil, Chris, I reviewed kevin's patch in September which fixes this issue. Chris -- can we pull it into mmc-next ? This patch is okay as a standalone change. That was the original intent. The question is what to do if no regulator. regulator_get was returning NULL in Daniel;s case. IS_ERR patch was not taken so UHS support was removed. The intent of the original code was to remove UHS support if there was a regulator but it could not support voltage switching. Phlip From: Philip Rakity prakity at marvell.com Subject: Re: [PATCH v2 5/8] mmc: sdhci: fix null return check of regulator_get Newsgroups: gmane.linux.kernel.mmc Date: 2012-09-24 15:02:34 GMT (5 weeks, 6 days, 21 hours and 44 minutes ago) It sounds like I misread this patch, then -- I understood it as only affecting whether a pr_info() call is made, which would not fix a voltage switching bug. What am I missing? (Dan, here's a copy of Kevin's patch.) From: Kevin Liu kl...@marvell.com regulator_get() returns NULL when CONFIG_REGULATOR not defined, which should not print out the warning. Reviewed-by: Philip Rakity prak...@marvell.com Signed-off-by: Bin Wang b...@marvell.com Signed-off-by: Kevin Liu kl...@marvell.com --- drivers/mmc/host/sdhci.c | 18 -- 1 files changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 8e6a6f0..0104ae9 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -2845,9 +2845,12 @@ int sdhci_add_host(struct sdhci_host *host) /* If vqmmc regulator and no 1.8V signalling, then there's no UHS */ host-vqmmc = regulator_get(mmc_dev(mmc), vqmmc); - if (IS_ERR(host-vqmmc)) { - pr_info(%s: no vqmmc regulator found\n, mmc_hostname(mmc)); - host-vqmmc = NULL; + if (IS_ERR_OR_NULL(host-vqmmc)) { + if (PTR_ERR(host-vqmmc) 0) { + pr_info(%s: no vqmmc regulator found\n, + mmc_hostname(mmc)); + host-vqmmc = NULL; + } } else if (regulator_is_supported_voltage(host-vqmmc, 180, 180)) regulator_enable(host-vqmmc); @@ -2903,9 +2906,12 @@ int sdhci_add_host(struct sdhci_host *host) ocr_avail = 0; host-vmmc = regulator_get(mmc_dev(mmc), vmmc); - if (IS_ERR(host-vmmc)) { - pr_info(%s: no vmmc regulator found\n, mmc_hostname(mmc)); - host-vmmc = NULL; + if (IS_ERR_OR_NULL(host-vmmc)) { + if (PTR_ERR(host-vmmc) 0) { + pr_info(%s: no vmmc regulator found\n, + mmc_hostname(mmc)); + host-vmmc = NULL; + } } else regulator_enable(host-vmmc); -- Chris Ball c...@laptop.org http://printf.net/ One Laptop Per Child --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- 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: UHS-I voltage switching on OLPC XO-1.75
Hi, adding Kevin, On Mon, Nov 05 2012, Philip Rakity wrote: On Nov 5, 2012, at 1:11 PM, Chris Ball c...@laptop.org wrote: Hi, On Mon, Nov 05 2012, Philip Rakity wrote: Hi Daneil, Chris, I reviewed kevin's patch in September which fixes this issue. Chris -- can we pull it into mmc-next ? This patch is okay as a standalone change. That was the original intent. The question is what to do if no regulator. regulator_get was returning NULL in Daniel;s case. IS_ERR patch was not taken so UHS support was removed. The intent of the original code was to remove UHS support if there was a regulator but it could not support voltage switching. So this patch does fix a real bug, other than the pr_info -- by affecting whether we disable UHS in the else clause -- and the commit message doesn't mention the existence of that real bug at all, and performs a cosmetic fix for vmmc at the same time as a semantic fix for vqmmc. That's terrible. I'll merge Kevin's patch after adding a commit message that explains what's actually going on. 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: UHS-I voltage switching on OLPC XO-1.75
Hi Daniel, 2012/10/30 Daniel Drake d...@laptop.org: Hi, Following on from the recent thread [PATCH 2/2] mmc: core: Power cycle card on voltage switch fail I have tried to get a better grip on the problems being faced. Test setup: OLPC XO-1.75 laptop, based on Marvell MMP2 ARM SoC (includes sdhci-pxa interface) 32GB Sandisk Ultra SD card being inserted into external SD slot The SoC apparently has support for 1.8V, but we physically do not have the right power lines wired on the motherboard, so we need to detect the 1.8V failure and go back to 3.3V mode. An initial question, would it be possible to solve this by disabling the UHS support in the host cap, or through a quirk? Maybe this kind of solution is not applicable in this scenario? I wonder this because I guess it will be very hard to know how a card will react if we first tell it that we're going to switch voltages and then don't, since this is out of spec. I tracked down what causes this regression. In order to get the failure detection working again, I have to make the following change in mmc_set_signal_voltage(): if (cmd11) { host-ios.clock = 0; - mmc_set_ios(host); + //mmc_set_ios(host); } (i.e. don't mess with ios before asking sdhci to do the voltage switch) and at the top of sdhci_do_1_8v_signal_voltage_switch: + /* Stop SDCLK */ + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); + clk = ~SDHCI_CLOCK_CARD_EN; + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); (i.e. add the equivalent clock disabling code here) Ok, so this suggests that the SDHCI driver does not stop the clock when we set ios.clock = 0 and call mmc_set_ios? With that change, I now get: mmc2: Signal voltage switch failed, power cycling card (retries = 10) mmc2: error -110 whilst initialising SD card The -110 error comes from mmc_send_app_op_cond(), but its worth noting that mmc_send_if_cond() (called immediately above) silently failed with -110 too. Neither of these failed the first time around before 1.8V was tried. Full patch that I'm testing with: http://dev.laptop.org/~dsd/20121030/mmc.patch So, now the question is: why is the card not responding to if_cond/app_op_cond after being powered down and up again? Is the power down/up sequence OK? Good question. I’d guess that mmc_power_off/up does not work as expected here, that the card is not at all power cycled. But it seems that the power cycle code in sdhci_do_start_signal_voltage_switch works? What if we export a couple of debug functions from sdhci.c which allow us to power cycle and control the clock just like sdhci does, and call these functions from core.c and sd.c? If this works (and the failure is detected properly by the core layer, and 10 retries are made etc), then we know that the problems most likely depend on how mmc_set_ios and mmc_power_off/up work with sdhci. I've attached a patch suggesting this, if you'd like to give it a try (warning: the patch has not been tested). The patch does not do anything about pm_runtime, but perhaps we don't need to worry about this here. I tried inserting the mmc_power_off, mmc_power_up, mmc_send_if_cond sequence into various points in the codepaths being discussed. Measuring success as the card responding to mmc_send_if_cond(0x30080) without error (0x30080 is a known good value from when before 1.8V is tried), I find that this reset sequence works just fine up until the point when CMD11 is run. CMD11 is sent and returns successfully without error, but from this point, running the reset sequence will fail (send_if_cond will fail with -110). Is this kind of test valid? Does this suggest that something is wrong with the host controller hardware? My assumption is that whatever state is modified by CMD11 should be erased here, and of course the hope is that mmc_power_off and mmc_power_up will do a full power cycle anyway. But whatever is happening, it looks like the effects of CMD11 are persisting past the reset sequence and are breaking later communication. I think that the power cycle has simply failed here, I find it hard to believe that CMD11 has persistent effects :) If mmc_power_off/up actually works, then perhaps the 1ms udelay is too small for this case? Kind regards, Johan 0001-Clock-and-power-control.patch Description: Binary data
Re: UHS-I voltage switching on OLPC XO-1.75
On Nov 2, 2012, at 2:35 PM, Johan Rudholm jrudh...@gmail.com wrote: Hi Daniel, 2012/10/30 Daniel Drake d...@laptop.org: Hi, Following on from the recent thread [PATCH 2/2] mmc: core: Power cycle card on voltage switch fail I have tried to get a better grip on the problems being faced. Test setup: OLPC XO-1.75 laptop, based on Marvell MMP2 ARM SoC (includes sdhci-pxa interface) 32GB Sandisk Ultra SD card being inserted into external SD slot The SoC apparently has support for 1.8V, but we physically do not have the right power lines wired on the motherboard, so we need to detect the 1.8V failure and go back to 3.3V mode. An initial question, would it be possible to solve this by disabling the UHS support in the host cap, or through a quirk? Maybe this kind of solution is not applicable in this scenario? I wonder this because I guess it will be very hard to know how a card will react if we first tell it that we're going to switch voltages and then don't, since this is out of spec. I submitted a patch a while ago that allows the board specific code to change host-caps2 The controller may indicate support for UHS but the board design can prohibit this. I tracked down what causes this regression. In order to get the failure detection working again, I have to make the following change in mmc_set_signal_voltage(): if (cmd11) { host-ios.clock = 0; - mmc_set_ios(host); + //mmc_set_ios(host); } (i.e. don't mess with ios before asking sdhci to do the voltage switch) and at the top of sdhci_do_1_8v_signal_voltage_switch: + /* Stop SDCLK */ + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); + clk = ~SDHCI_CLOCK_CARD_EN; + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); (i.e. add the equivalent clock disabling code here) Ok, so this suggests that the SDHCI driver does not stop the clock when we set ios.clock = 0 and call mmc_set_ios? With that change, I now get: mmc2: Signal voltage switch failed, power cycling card (retries = 10) mmc2: error -110 whilst initialising SD card The -110 error comes from mmc_send_app_op_cond(), but its worth noting that mmc_send_if_cond() (called immediately above) silently failed with -110 too. Neither of these failed the first time around before 1.8V was tried. Full patch that I'm testing with: http://dev.laptop.org/~dsd/20121030/mmc.patch So, now the question is: why is the card not responding to if_cond/app_op_cond after being powered down and up again? Is the power down/up sequence OK? Good question. I’d guess that mmc_power_off/up does not work as expected here, that the card is not at all power cycled. But it seems that the power cycle code in sdhci_do_start_signal_voltage_switch works? What if we export a couple of debug functions from sdhci.c which allow us to power cycle and control the clock just like sdhci does, and call these functions from core.c and sd.c? If this works (and the failure is detected properly by the core layer, and 10 retries are made etc), then we know that the problems most likely depend on how mmc_set_ios and mmc_power_off/up work with sdhci. I've attached a patch suggesting this, if you'd like to give it a try (warning: the patch has not been tested). The patch does not do anything about pm_runtime, but perhaps we don't need to worry about this here. I tried inserting the mmc_power_off, mmc_power_up, mmc_send_if_cond sequence into various points in the codepaths being discussed. Measuring success as the card responding to mmc_send_if_cond(0x30080) without error (0x30080 is a known good value from when before 1.8V is tried), I find that this reset sequence works just fine up until the point when CMD11 is run. CMD11 is sent and returns successfully without error, but from this point, running the reset sequence will fail (send_if_cond will fail with -110). Is this kind of test valid? Does this suggest that something is wrong with the host controller hardware? My assumption is that whatever state is modified by CMD11 should be erased here, and of course the hope is that mmc_power_off and mmc_power_up will do a full power cycle anyway. But whatever is happening, it looks like the effects of CMD11 are persisting past the reset sequence and are breaking later communication. I think that the power cycle has simply failed here, I find it hard to believe that CMD11 has persistent effects :) If mmc_power_off/up actually works, then perhaps the 1ms udelay is too small for this case? Kind regards, Johan 0001-Clock-and-power-control.patch --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended
UHS-I voltage switching on OLPC XO-1.75
Hi, Following on from the recent thread [PATCH 2/2] mmc: core: Power cycle card on voltage switch fail I have tried to get a better grip on the problems being faced. Test setup: OLPC XO-1.75 laptop, based on Marvell MMP2 ARM SoC (includes sdhci-pxa interface) 32GB Sandisk Ultra SD card being inserted into external SD slot The SoC apparently has support for 1.8V, but we physically do not have the right power lines wired on the motherboard, so we need to detect the 1.8V failure and go back to 3.3V mode. Before patching anything, inserting this card results in: sdhci: Switching to 1.8V signalling voltage failed, retrying with S18R set to 0 mmc0: error -110 whilst initialising SD card Now, working on Linux-3.5.4, I add these patches: mmc: core: reset sigal voltage on power up [RFC/PATCH,v2] mmc: core: Fixup signal voltage switch regulator: add missing defintion regulator_is_supported_voltage mmc: sdhci: Add regulator support for vccq (voltage regualor) mmc: sdhci: Let core handle UHS switch failure I also tweaked the sdhci code so that UHS-I flags are not unconditionally disabled by the vccq commit (as explained in sdhci vccq regulator support drops UHS-I flags). Under this setup, the first problem encountered is that the system no longer identifies the fact that the 1.8V voltage failed. Things are not happy afterwards. The card_busy check in sdhci is saying that the card is not busy. Full kernel logs: http://dev.laptop.org/~dsd/20121030/mmc1.log I tracked down what causes this regression. In order to get the failure detection working again, I have to make the following change in mmc_set_signal_voltage(): if (cmd11) { host-ios.clock = 0; - mmc_set_ios(host); + //mmc_set_ios(host); } (i.e. don't mess with ios before asking sdhci to do the voltage switch) and at the top of sdhci_do_1_8v_signal_voltage_switch: + /* Stop SDCLK */ + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); + clk = ~SDHCI_CLOCK_CARD_EN; + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); (i.e. add the equivalent clock disabling code here) With that change, I now get: mmc2: Signal voltage switch failed, power cycling card (retries = 10) mmc2: error -110 whilst initialising SD card The -110 error comes from mmc_send_app_op_cond(), but its worth noting that mmc_send_if_cond() (called immediately above) silently failed with -110 too. Neither of these failed the first time around before 1.8V was tried. Full patch that I'm testing with: http://dev.laptop.org/~dsd/20121030/mmc.patch So, now the question is: why is the card not responding to if_cond/app_op_cond after being powered down and up again? Is the power down/up sequence OK? I tried inserting the mmc_power_off, mmc_power_up, mmc_send_if_cond sequence into various points in the codepaths being discussed. Measuring success as the card responding to mmc_send_if_cond(0x30080) without error (0x30080 is a known good value from when before 1.8V is tried), I find that this reset sequence works just fine up until the point when CMD11 is run. CMD11 is sent and returns successfully without error, but from this point, running the reset sequence will fail (send_if_cond will fail with -110). Is this kind of test valid? Does this suggest that something is wrong with the host controller hardware? My assumption is that whatever state is modified by CMD11 should be erased here, and of course the hope is that mmc_power_off and mmc_power_up will do a full power cycle anyway. But whatever is happening, it looks like the effects of CMD11 are persisting past the reset sequence and are breaking later communication. In the mail last week, I think we found some kind of configuration (on Linux-3.0) where this same setup successfully detected the 1.8V switching failure, and successfully switched back to 3.3V. I plan to go back and test that, maybe there is some kind of sequence that doesn't make the hardware die. Daniel -- 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