Please send patches as text/plain - it makes it difficult to reply to
them otherwise.
On Mon, Jan 09, 2006 at 06:15:00PM -0400, Anderson Briglia wrote:
> When a card is locked, only commands from the "basic" and "lock card" classes
> are accepted. To be able to use the other commands, the card must be unlocked
> first.
I don't think this works as you intend.
When a card is initially inserted, we discover the cards via mmc_setup()
and mmc_discover_cards(). This means that we'll never set the locked
status for newly inserted cards.
> ===================================================================
> --- linux-2.6.15-rc4.orig/drivers/mmc/mmc.c 2005-12-15 14:06:52.000000000 -0400
> +++ linux-2.6.15-rc4/drivers/mmc/mmc.c 2005-12-15 17:00:37.000000000 -0400
> @@ -986,10 +986,15 @@ static void mmc_check_cards(struct mmc_h
> cmd.flags = MMC_RSP_R1;
>
> err = mmc_wait_for_cmd(host, &cmd, CMD_RETRIES);
> - if (err == MMC_ERR_NONE)
> + if (err != MMC_ERR_NONE) {
> + mmc_card_set_dead(card);
> continue;
> + }
>
> - mmc_card_set_dead(card);
> + if (cmd.resp[0] & R1_CARD_IS_LOCKED)
> + mmc_card_set_locked(card);
> + else
> + card->state &= ~MMC_STATE_LOCKED;
We need a mmc_card_clear_locked() here.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
Russell King wrote:
>On Mon, Jan 09, 2006 at 06:15:00PM -0400, Anderson Briglia wrote:
>
>
>>When a card is locked, only commands from the "basic" and "lock card" classes
>>are accepted. To be able to use the other commands, the card must be unlocked
>>first.
>>
>>
>
>I don't think this works as you intend.
>
>When a card is initially inserted, we discover the cards via mmc_setup()
>and mmc_discover_cards(). This means that we'll never set the locked
>status for newly inserted cards.
>
>
mmc_setup() calls mmc_check_cards(). Our patch adds the necessary code
to mmc_check_cards() to set the locked state when the card is locked.
>>===================================================================
>>--- linux-2.6.15-rc4.orig/drivers/mmc/mmc.c 2005-12-15 14:06:52.000000000 -0400
>>+++ linux-2.6.15-rc4/drivers/mmc/mmc.c 2005-12-15 17:00:37.000000000 -0400
>>@@ -986,10 +986,15 @@ static void mmc_check_cards(struct mmc_h
>> cmd.flags = MMC_RSP_R1;
>>
>> err = mmc_wait_for_cmd(host, &cmd, CMD_RETRIES);
>>- if (err == MMC_ERR_NONE)
>>+ if (err != MMC_ERR_NONE) {
>>+ mmc_card_set_dead(card);
>> continue;
>>+ }
>>
>>- mmc_card_set_dead(card);
>>+ if (cmd.resp[0] & R1_CARD_IS_LOCKED)
>>+ mmc_card_set_locked(card);
>>+ else
>>+ card->state &= ~MMC_STATE_LOCKED;
>>
>>
Anderson Briglia
INdT - Manaus
On Wed, Jan 11, 2006 at 09:16:28AM -0400, Anderson Briglia wrote:
> Russell King wrote:
>
> >On Mon, Jan 09, 2006 at 06:15:00PM -0400, Anderson Briglia wrote:
> >
> >
> >>When a card is locked, only commands from the "basic" and "lock card" classes
> >>are accepted. To be able to use the other commands, the card must be unlocked
> >>first.
> >>
> >>
> >
> >I don't think this works as you intend.
> >
> >When a card is initially inserted, we discover the cards via mmc_setup()
> >and mmc_discover_cards(). This means that we'll never set the locked
> >status for newly inserted cards.
> >
> >
> mmc_setup() calls mmc_check_cards(). Our patch adds the necessary code
> to mmc_check_cards() to set the locked state when the card is locked.
Not in Linus' kernel, it doesn't.
If you're working off the OMAP tree, bear in mind that I've found in
the past that they have a large number of wrong or inappropriate
changes to the MMC layer in there. They don't regularly merge either,
and they certainly don't forward any bug fixes for review in a timely
manner.
mmc_rescan() is the only caller of mmc_check_cards(), and this only
happens if mmc_rescan() is called with the power already applied
(iow, cards are already known to the MMC layer.)
This call is done _before_ we call mmc_setup() to discover new cards.
Calling mmc_check_cards() from mmc_rescan() is _wrong_, and from what
I can ascertain, it's probably due to unforwarded broken changes in
the OMAP tree.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
* Russell King <[email protected]> [060111 06:44]:
> On Wed, Jan 11, 2006 at 09:16:28AM -0400, Anderson Briglia wrote:
> > Russell King wrote:
> >
> > >On Mon, Jan 09, 2006 at 06:15:00PM -0400, Anderson Briglia wrote:
> > >
> > >
> > >>When a card is locked, only commands from the "basic" and "lock card" classes
> > >>are accepted. To be able to use the other commands, the card must be unlocked
> > >>first.
> > >>
> > >>
> > >
> > >I don't think this works as you intend.
> > >
> > >When a card is initially inserted, we discover the cards via mmc_setup()
> > >and mmc_discover_cards(). This means that we'll never set the locked
> > >status for newly inserted cards.
> > >
> > >
> > mmc_setup() calls mmc_check_cards(). Our patch adds the necessary code
> > to mmc_check_cards() to set the locked state when the card is locked.
>
> Not in Linus' kernel, it doesn't.
>
> If you're working off the OMAP tree, bear in mind that I've found in
> the past that they have a large number of wrong or inappropriate
> changes to the MMC layer in there. They don't regularly merge either,
> and they certainly don't forward any bug fixes for review in a timely
> manner.
I agree the omap MMC driver should be cleaned-up and finally merged.
Anderson, since you are already patching it, do you want to take care of
cleaning it up a bit and posting it here?
There are some mmc core hacks there for buggy cards that might worth
testing on other platforms too. (It could also be that they are
only needed on omap).
As far as I remember for omap we've had to do hacks for cards that claim
to be ready before the data is transferred, and also hacks for cards
that never set the ready bit...
Regards,
Tony
Tony Lindgren wrote:
> * Russell King <[email protected]> [060111 06:44]:
>
>>On Wed, Jan 11, 2006 at 09:16:28AM -0400, Anderson Briglia wrote:
>>
>>>Russell King wrote:
>>>
>>>
>>>>On Mon, Jan 09, 2006 at 06:15:00PM -0400, Anderson Briglia wrote:
>>>>
>>>>
>>>>
>>>>>When a card is locked, only commands from the "basic" and "lock card" classes
>>>>>are accepted. To be able to use the other commands, the card must be unlocked
>>>>>first.
>>>>>
>>>>>
>>>>
>>>>I don't think this works as you intend.
>>>>
>>>>When a card is initially inserted, we discover the cards via mmc_setup()
>>>>and mmc_discover_cards(). This means that we'll never set the locked
>>>>status for newly inserted cards.
>>>>
>>>>
>>>
>>>mmc_setup() calls mmc_check_cards(). Our patch adds the necessary code
>>>to mmc_check_cards() to set the locked state when the card is locked.
>>
>>Not in Linus' kernel, it doesn't.
>>
>>If you're working off the OMAP tree, bear in mind that I've found in
>>the past that they have a large number of wrong or inappropriate
>>changes to the MMC layer in there. They don't regularly merge either,
>>and they certainly don't forward any bug fixes for review in a timely
>>manner.
>
>
> I agree the omap MMC driver should be cleaned-up and finally merged.
>
> Anderson, since you are already patching it, do you want to take care of
> cleaning it up a bit and posting it here?
OK Tony. I'll do it and post here asap.
Regards,
Anderson Briglia
INdT - Manaus
* Anderson Briglia <[email protected]> [060111 11:26]:
> Tony Lindgren wrote:
> > * Russell King <[email protected]> [060111 06:44]:
> >
> >>On Wed, Jan 11, 2006 at 09:16:28AM -0400, Anderson Briglia wrote:
> >>
> >>>Russell King wrote:
> >>>
> >>>
> >>>>On Mon, Jan 09, 2006 at 06:15:00PM -0400, Anderson Briglia wrote:
> >>>>
> >>>>
> >>>>
> >>>>>When a card is locked, only commands from the "basic" and "lock card" classes
> >>>>>are accepted. To be able to use the other commands, the card must be unlocked
> >>>>>first.
> >>>>>
> >>>>>
> >>>>
> >>>>I don't think this works as you intend.
> >>>>
> >>>>When a card is initially inserted, we discover the cards via mmc_setup()
> >>>>and mmc_discover_cards(). This means that we'll never set the locked
> >>>>status for newly inserted cards.
> >>>>
> >>>>
> >>>
> >>>mmc_setup() calls mmc_check_cards(). Our patch adds the necessary code
> >>>to mmc_check_cards() to set the locked state when the card is locked.
> >>
> >>Not in Linus' kernel, it doesn't.
> >>
> >>If you're working off the OMAP tree, bear in mind that I've found in
> >>the past that they have a large number of wrong or inappropriate
> >>changes to the MMC layer in there. They don't regularly merge either,
> >>and they certainly don't forward any bug fixes for review in a timely
> >>manner.
> >
> >
> > I agree the omap MMC driver should be cleaned-up and finally merged.
> >
> > Anderson, since you are already patching it, do you want to take care of
> > cleaning it up a bit and posting it here?
>
> OK Tony. I'll do it and post here asap.
Cool, thanks!
Tony