* [PATCH] MMC: Select only one voltage bit in OCR response
@ 2006-10-09 15:00 Timo Teras
2006-10-09 16:53 ` Russell King
0 siblings, 1 reply; 9+ messages in thread
From: Timo Teras @ 2006-10-09 15:00 UTC (permalink / raw)
To: drzeus-list, rmk+mmc; +Cc: linux-kernel
The card might go to inactive state (according to specification), if
there are unsupported bits set in the OCR.
Signed-off-by: Timo Teras <timo.teras@solidboot.com>
---
drivers/mmc/mmc.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
8208e380f51ca772e84a0b83098d24aacfa3cc2b
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index acc5365..61bf3fc 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -475,7 +475,7 @@ static u32 mmc_select_voltage(struct mmc
if (bit) {
bit -= 1;
- ocr = 3 << bit;
+ ocr = 1 << bit;
host->ios.vdd = bit;
mmc_set_ios(host);
--
1.2.3.g90cc1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] MMC: Select only one voltage bit in OCR response
2006-10-09 15:00 [PATCH] MMC: Select only one voltage bit in OCR response Timo Teras
@ 2006-10-09 16:53 ` Russell King
2006-10-09 17:23 ` Timo Teras
2006-10-10 13:23 ` [PATCH] MMC: Do not set unsupported bits " Timo Teras
0 siblings, 2 replies; 9+ messages in thread
From: Russell King @ 2006-10-09 16:53 UTC (permalink / raw)
To: Timo Teras; +Cc: drzeus-list, linux-kernel
On Mon, Oct 09, 2006 at 06:00:44PM +0300, Timo Teras wrote:
> The card might go to inactive state (according to specification), if
> there are unsupported bits set in the OCR.
NAK. This breaks some MMC cards.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MMC: Select only one voltage bit in OCR response
2006-10-09 16:53 ` Russell King
@ 2006-10-09 17:23 ` Timo Teras
2006-10-16 6:34 ` Pierre Ossman
2006-10-10 13:23 ` [PATCH] MMC: Do not set unsupported bits " Timo Teras
1 sibling, 1 reply; 9+ messages in thread
From: Timo Teras @ 2006-10-09 17:23 UTC (permalink / raw)
To: Timo Teras, drzeus-list, linux-kernel
On Mon, Oct 09, 2006 at 05:53:17PM +0100, Russell King wrote:
> On Mon, Oct 09, 2006 at 06:00:44PM +0300, Timo Teras wrote:
> > The card might go to inactive state (according to specification), if
> > there are unsupported bits set in the OCR.
>
> NAK. This breaks some MMC cards.
I see. But if we do send an OCR with an unsupported bit set, the card will
go to inactive state and is unusable. This problem is masked on controllers
with only 3.3V support, but I'm working with a controller supporting several
different voltages.
For example, I have a card giving an OCR reply of 0x0ff80080. The current
code will reply to this with 0x00000180 which is clearly incorrect.
Maybe something like "ocr &= 3 << bit;" would be more approriate?
Cheers,
Timo
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] MMC: Do not set unsupported bits in OCR response
2006-10-09 16:53 ` Russell King
2006-10-09 17:23 ` Timo Teras
@ 2006-10-10 13:23 ` Timo Teras
1 sibling, 0 replies; 9+ messages in thread
From: Timo Teras @ 2006-10-10 13:23 UTC (permalink / raw)
To: Russell King, drzeus-list; +Cc: Timo Teras, linux-kernel
The card might go to inactive state (according to specification), if
there are unsupported bits set in the OCR.
Signed-off-by: Timo Teras <timo.teras@solidboot.com>
---
drivers/mmc/mmc.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index ee8863c..c6f3077 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -475,7 +475,7 @@ static u32 mmc_select_voltage(struct mmc
if (bit) {
bit -= 1;
- ocr = 3 << bit;
+ ocr &= 3 << bit;
host->ios.vdd = bit;
mmc_set_ios(host);
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] MMC: Select only one voltage bit in OCR response
2006-10-09 17:23 ` Timo Teras
@ 2006-10-16 6:34 ` Pierre Ossman
2006-10-22 9:25 ` Pierre Ossman
2006-10-31 10:05 ` Russell King
0 siblings, 2 replies; 9+ messages in thread
From: Pierre Ossman @ 2006-10-16 6:34 UTC (permalink / raw)
To: Timo Teras; +Cc: linux-kernel
Timo Teras wrote:
> I see. But if we do send an OCR with an unsupported bit set, the card will
> go to inactive state and is unusable. This problem is masked on controllers
> with only 3.3V support, but I'm working with a controller supporting several
> different voltages.
>
> For example, I have a card giving an OCR reply of 0x0ff80080. The current
> code will reply to this with 0x00000180 which is clearly incorrect.
>
> Maybe something like "ocr &= 3 << bit;" would be more approriate?
>
Russell? Comments? Do you still have the offending card?
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
PulseAudio, core developer http://pulseaudio.org
rdesktop, core developer http://www.rdesktop.org
OLPC, developer http://www.laptop.org
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MMC: Select only one voltage bit in OCR response
2006-10-16 6:34 ` Pierre Ossman
@ 2006-10-22 9:25 ` Pierre Ossman
2006-10-31 10:05 ` Russell King
1 sibling, 0 replies; 9+ messages in thread
From: Pierre Ossman @ 2006-10-22 9:25 UTC (permalink / raw)
To: Russell King; +Cc: Timo Teras, linux-kernel
(In case you've missed it)
Pierre Ossman wrote:
> Timo Teras wrote:
>> I see. But if we do send an OCR with an unsupported bit set, the card will
>> go to inactive state and is unusable. This problem is masked on controllers
>> with only 3.3V support, but I'm working with a controller supporting several
>> different voltages.
>>
>> For example, I have a card giving an OCR reply of 0x0ff80080. The current
>> code will reply to this with 0x00000180 which is clearly incorrect.
>>
>> Maybe something like "ocr &= 3 << bit;" would be more approriate?
>>
>
> Russell? Comments? Do you still have the offending card?
>
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
PulseAudio, core developer http://pulseaudio.org
rdesktop, core developer http://www.rdesktop.org
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MMC: Select only one voltage bit in OCR response
2006-10-16 6:34 ` Pierre Ossman
2006-10-22 9:25 ` Pierre Ossman
@ 2006-10-31 10:05 ` Russell King
2006-10-31 15:24 ` Juha Yrjola
1 sibling, 1 reply; 9+ messages in thread
From: Russell King @ 2006-10-31 10:05 UTC (permalink / raw)
To: Pierre Ossman; +Cc: Timo Teras, linux-kernel
On Mon, Oct 16, 2006 at 08:34:20AM +0200, Pierre Ossman wrote:
> Timo Teras wrote:
> > I see. But if we do send an OCR with an unsupported bit set, the card will
> > go to inactive state and is unusable. This problem is masked on controllers
> > with only 3.3V support, but I'm working with a controller supporting several
> > different voltages.
> >
> > For example, I have a card giving an OCR reply of 0x0ff80080. The current
> > code will reply to this with 0x00000180 which is clearly incorrect.
> >
> > Maybe something like "ocr &= 3 << bit;" would be more approriate?
> >
>
> Russell? Comments? Do you still have the offending card?
It wasn't my cards, but was reported by several other folk. I don't think
we can revert on this without breakage.
However, we should probably ensure that we don't end up setting voltage
bits which the cards don't support. So maybe masking the resulting OCR
value with the received combined OCR would be a good idea? Such as:
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index ee8863c..45e0598 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -467,23 +467,24 @@ static inline void mmc_delay(unsigned in
*/
static u32 mmc_select_voltage(struct mmc_host *host, u32 ocr)
{
+ u32 selected_ocr;
int bit;
- ocr &= host->ocr_avail;
+ selected_ocr = ocr & host->ocr_avail;
- bit = ffs(ocr);
+ bit = ffs(selected_ocr);
if (bit) {
bit -= 1;
- ocr = 3 << bit;
+ selected_ocr = 3 << bit;
host->ios.vdd = bit;
mmc_set_ios(host);
} else {
- ocr = 0;
+ selected_ocr = 0;
}
- return ocr;
+ return selected_ocr & ocr;
}
#define UNSTUFF_BITS(resp,start,size) \
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] MMC: Select only one voltage bit in OCR response
2006-10-31 10:05 ` Russell King
@ 2006-10-31 15:24 ` Juha Yrjola
2006-11-02 18:37 ` Pierre Ossman
0 siblings, 1 reply; 9+ messages in thread
From: Juha Yrjola @ 2006-10-31 15:24 UTC (permalink / raw)
To: Pierre Ossman, Timo Teras, linux-kernel
Russell King wrote:
>>> Maybe something like "ocr &= 3 << bit;" would be more approriate?
>>>
>> Russell? Comments? Do you still have the offending card?
>
> It wasn't my cards, but was reported by several other folk. I don't think
> we can revert on this without breakage.
>
> However, we should probably ensure that we don't end up setting voltage
> bits which the cards don't support. So maybe masking the resulting OCR
> value with the received combined OCR would be a good idea? Such as:
Isn't this exactly what Timo is proposing above?
Cheers,
Juha
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index ee8863c..45e0598 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -467,23 +467,24 @@ static inline void mmc_delay(unsigned in
> */
> static u32 mmc_select_voltage(struct mmc_host *host, u32 ocr)
> {
> + u32 selected_ocr;
> int bit;
>
> - ocr &= host->ocr_avail;
> + selected_ocr = ocr & host->ocr_avail;
>
> - bit = ffs(ocr);
> + bit = ffs(selected_ocr);
> if (bit) {
> bit -= 1;
>
> - ocr = 3 << bit;
> + selected_ocr = 3 << bit;
>
> host->ios.vdd = bit;
> mmc_set_ios(host);
> } else {
> - ocr = 0;
> + selected_ocr = 0;
> }
>
> - return ocr;
> + return selected_ocr & ocr;
> }
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MMC: Select only one voltage bit in OCR response
2006-10-31 15:24 ` Juha Yrjola
@ 2006-11-02 18:37 ` Pierre Ossman
0 siblings, 0 replies; 9+ messages in thread
From: Pierre Ossman @ 2006-11-02 18:37 UTC (permalink / raw)
To: Juha Yrjola; +Cc: Timo Teras, linux-kernel
Juha Yrjola wrote:
>
> Isn't this exactly what Timo is proposing above?
>
I sure can't see the difference. Timo's patch queued up for -mm.
Rgds
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
PulseAudio, core developer http://pulseaudio.org
rdesktop, core developer http://www.rdesktop.org
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2006-11-02 18:37 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-10-09 15:00 [PATCH] MMC: Select only one voltage bit in OCR response Timo Teras
2006-10-09 16:53 ` Russell King
2006-10-09 17:23 ` Timo Teras
2006-10-16 6:34 ` Pierre Ossman
2006-10-22 9:25 ` Pierre Ossman
2006-10-31 10:05 ` Russell King
2006-10-31 15:24 ` Juha Yrjola
2006-11-02 18:37 ` Pierre Ossman
2006-10-10 13:23 ` [PATCH] MMC: Do not set unsupported bits " Timo Teras
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).