Purpose of MMC_DATA_MULTI?

Purpose of MMC_DATA_MULTI?

[Pierre Ossman]

I noticed that a new transfer flag was recently added to the MMC layer
without any immediate users, the MMC_DATA_MULTI flag. I'm guessing the
purpose of the flag is to indicate the difference between
MMC_READ_SINGLE_BLOCK and MMC_READ_MULTIPLE_BLOCKS with just one block.
If so, then that should probably be mentioned in a comment somewhere.

Rgds
Pierre

Re: Purpose of MMC_DATA_MULTI?

[Russell King]

On Wed, Feb 01, 2006 at 07:40:26AM +0100, Pierre Ossman wrote:
> I noticed that a new transfer flag was recently added to the MMC layer
> without any immediate users, the MMC_DATA_MULTI flag. I'm guessing the
> purpose of the flag is to indicate the difference between
> MMC_READ_SINGLE_BLOCK and MMC_READ_MULTIPLE_BLOCKS with just one block.
> If so, then that should probably be mentioned in a comment somewhere.

There are hosts out there (Atmel AT91-based) which need to know if the
transfer is going to be multiple block.  Rather than have them test
the op-code (which is what they're already doing), we provide a flag
instead.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: Purpose of MMC_DATA_MULTI?

[Pierre Ossman]

Russell King wrote:
> On Wed, Feb 01, 2006 at 07:40:26AM +0100, Pierre Ossman wrote:
>   
>> I noticed that a new transfer flag was recently added to the MMC layer
>> without any immediate users, the MMC_DATA_MULTI flag. I'm guessing the
>> purpose of the flag is to indicate the difference between
>> MMC_READ_SINGLE_BLOCK and MMC_READ_MULTIPLE_BLOCKS with just one block.
>> If so, then that should probably be mentioned in a comment somewhere.
>>     
>
> There are hosts out there (Atmel AT91-based) which need to know if the
> transfer is going to be multiple block.  Rather than have them test
> the op-code (which is what they're already doing), we provide a flag
> instead.
>
>   

As far as the hardware is concerned there are two "multi-block" transfers:

 * Multiple, back-to-back blocks.
 * One or more blocks that need to be terminated by some form of stop
command.

The first can be identified by checking the number of blocks in the
request, the latter is harder to identify since it's a protocol semantic
(it could be just one block, but still need a stop). Does MMC_DATA_MULTI
indicate the latter, former or both?

Rgds
Pierre

Re: Purpose of MMC_DATA_MULTI?

[Russell King]

On Wed, Feb 01, 2006 at 10:37:12AM +0100, Pierre Ossman wrote:
> Russell King wrote:
> > On Wed, Feb 01, 2006 at 07:40:26AM +0100, Pierre Ossman wrote:
> >> I noticed that a new transfer flag was recently added to the MMC layer
> >> without any immediate users, the MMC_DATA_MULTI flag. I'm guessing the
> >> purpose of the flag is to indicate the difference between
> >> MMC_READ_SINGLE_BLOCK and MMC_READ_MULTIPLE_BLOCKS with just one block.
> >> If so, then that should probably be mentioned in a comment somewhere.
> >>     
> >
> > There are hosts out there (Atmel AT91-based) which need to know if the
> > transfer is going to be multiple block.  Rather than have them test
> > the op-code (which is what they're already doing), we provide a flag
> > instead.
> 
> As far as the hardware is concerned there are two "multi-block" transfers:
> 
>  * Multiple, back-to-back blocks.
>  * One or more blocks that need to be terminated by some form of stop
> command.
> 
> The first can be identified by checking the number of blocks in the
> request, the latter is harder to identify since it's a protocol semantic
> (it could be just one block, but still need a stop). Does MMC_DATA_MULTI
> indicate the latter, former or both?

In short, it's defined to be whatever AT91_MCI_TRTYP_MULTIPLE means in
the AT91RM9200 MMC host driver, which appears to be set for any of the
multiple block commands.  They currently are doing:

+static const u32 commands[64] = {
+       /* Class 1  (0) */
+       MMC_GO_IDLE_STATE,
+       MMC_SEND_OP_COND        | AT91_MCI_RSPTYP_48,
+       MMC_ALL_SEND_CID        | AT91_MCI_RSPTYP_136,
+       MMC_SET_RELATIVE_ADDR   | AT91_MCI_RSPTYP_48    | AT91_MCI_MAXLAT,
+       MMC_SET_DSR             | AT91_MCI_MAXLAT,
...
+       MMC_READ_SINGLE_BLOCK   | AT91_MCI_RSPTYP_48    | AT91_MCI_MAXLAT | AT91_MCI_TRDIR | AT91_MCI_TRCMD_START | AT91_MCI_TRTYP_BLOCK,
+       MMC_READ_MULTIPLE_BLOCK | AT91_MCI_RSPTYP_48    | AT91_MCI_MAXLAT | AT91_MCI_TRDIR | AT91_MCI_TRCMD_START | AT91_MCI_TRTYP_MULTIPLE,
...
+       MMC_WRITE_BLOCK         | AT91_MCI_RSPTYP_48    | AT91_MCI_MAXLAT | AT91_MCI_TRCMD_START | AT91_MCI_TRTYP_BLOCK,
+       MMC_WRITE_MULTIPLE_BLOCK| AT91_MCI_RSPTYP_48    | AT91_MCI_MAXLAT | AT91_MCI_TRCMD_START | AT91_MCI_TRTYP_MULTIPLE,

and using that as a lookup table by command for the value to put into
the command register.  I want to eliminate that, and not passing the
MULTI flag prevents elimination of this table.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: Purpose of MMC_DATA_MULTI?

[Pierre Ossman]

Russell King wrote:
> On Wed, Feb 01, 2006 at 10:37:12AM +0100, Pierre Ossman wrote:
>   
>> Russell King wrote:
>>     
>>> On Wed, Feb 01, 2006 at 07:40:26AM +0100, Pierre Ossman wrote:
>>>       
>>>> I noticed that a new transfer flag was recently added to the MMC layer
>>>> without any immediate users, the MMC_DATA_MULTI flag. I'm guessing the
>>>> purpose of the flag is to indicate the difference between
>>>> MMC_READ_SINGLE_BLOCK and MMC_READ_MULTIPLE_BLOCKS with just one block.
>>>> If so, then that should probably be mentioned in a comment somewhere.
>>>>     
>>>>         
>>> There are hosts out there (Atmel AT91-based) which need to know if the
>>> transfer is going to be multiple block.  Rather than have them test
>>> the op-code (which is what they're already doing), we provide a flag
>>> instead.
>>>       
>> As far as the hardware is concerned there are two "multi-block" transfers:
>>
>>  * Multiple, back-to-back blocks.
>>  * One or more blocks that need to be terminated by some form of stop
>> command.
>>
>> The first can be identified by checking the number of blocks in the
>> request, the latter is harder to identify since it's a protocol semantic
>> (it could be just one block, but still need a stop). Does MMC_DATA_MULTI
>> indicate the latter, former or both?
>>     
>
> In short, it's defined to be whatever AT91_MCI_TRTYP_MULTIPLE means in
> the AT91RM9200 MMC host driver, which appears to be set for any of the
> multiple block commands.  They currently are doing:
>
>   

That's a bit fuzzy and bound to give problems. Why not settle for the 
first case and change their code to check the block count in the 
request? Less flags should be a good thing. And if that proves to be 
insufficient we should do a more thorough investigation once we have an 
actual failure case.

> and using that as a lookup table by command for the value to put into
> the command register.  I want to eliminate that, and not passing the
> MULTI flag prevents elimination of this table.
>
>   

Seems to be a common theme in the more recent drivers. Don't they teach 
people proper layering in the schools anymore? :)

Rgds
Pierre

Re: Purpose of MMC_DATA_MULTI?

[Russell King]

On Thu, Feb 02, 2006 at 10:49:51AM +0100, Pierre Ossman wrote:
> Russell King wrote:
> >In short, it's defined to be whatever AT91_MCI_TRTYP_MULTIPLE means in
> >the AT91RM9200 MMC host driver, which appears to be set for any of the
> >multiple block commands.  They currently are doing:
> 
> That's a bit fuzzy and bound to give problems. Why not settle for the 
> first case and change their code to check the block count in the 
> request?

Mainly because I don't know if that's sufficient, and until I get around
to finding and reading the AT91RM9200 data sheet, I won't know if it is.
What I do know is that the addition of that flag provides the exact
information which the driver wants.

> >and using that as a lookup table by command for the value to put into
> >the command register.  I want to eliminate that, and not passing the
> >MULTI flag prevents elimination of this table.
> 
> Seems to be a common theme in the more recent drivers. Don't they teach 
> people proper layering in the schools anymore? :)

>From the evidence to date, it would appear not.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: Purpose of MMC_DATA_MULTI?

[Pierre Ossman]

Russell King wrote:
> Mainly because I don't know if that's sufficient, and until I get around
> to finding and reading the AT91RM9200 data sheet, I won't know if it is.
> What I do know is that the addition of that flag provides the exact
> information which the driver wants.
>
>   

I had a look in the spec[1] and things didn't exactly get any clearer. 
All it says is that the encoding 01 (binary) means "Multiple Block", no 
a single word on its semantics or an example of when it should be used. 
So I still advocate the simpler approach (code complexity-wise) until we 
have a test case that says it's insufficient.

Rgds
Pierre

[1] http://www.atmel.com/dyn/resources/prod_documents/doc1768.pdf