linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [Uclinux-dist-devel] spi_mmc bus concurrency fix
       [not found] ` <47C42D50.7020204-9ai+6vhHfRGzQB+pC5nmwQ@public.gmane.org>
@ 2008-02-27  4:23   ` Bryan Wu
       [not found]     ` <386072610802262023t6bd22875kfeb58c797be66024-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Bryan Wu @ 2008-02-27  4:23 UTC (permalink / raw)
  To: Hans Eklund
  Cc: hp-VrBV9hrLPhE,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f,
	uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b

On Tue, Feb 26, 2008 at 11:16 PM, Hans Eklund <hans-9ai+6vhHfRGzQB+pC5nmwQ@public.gmane.org> wrote:
> Hi Bryan, i need some guidance regarding the SPI framwork here.
>
>  The spi_mmc driver i originally wrote was never tested with concurrent
>  access on the SPI bus vs. spi flash chip for example. It is not working
>  well in that case. I have a few days to fix it and im half way there i
>  believe. Core problem is that during a single sequence of commands i
>  need "atomicity" on the bus, ie. i need to hold chip select until im
>  done. And specifically, no other traffic can be allowed while a command
>  sequence is in action vs. the MMC/SD card. But as i see it, SPI messages
>  are of predetermined length? is it not? And after a message is done,
>  there is no guarantee i can use it again without interuption from other
>  protocol drivers, even if chip select is left asserted. True?
>

>From the kernel mainline mmc_spi driver, it mentioned that that
current SPI framework does not allow the MMC operation share the SPI
bus with other SPI devices.

--
/* NOTES:
 *
 * - For now, we won't try to interoperate with a real mmc/sd/sdio
 *   controller, although some of them do have hardware support for
 *   SPI protocol.  The main reason for such configs would be mmc-ish
 *   cards like DataFlash, which don't support that "native" protocol.
 *
 *   We don't have a "DataFlash/MMC/SD/SDIO card slot" abstraction to
 *   switch between driver stacks, and in any case if "native" mode
 *   is available, it will be faster and hence preferable.
 *
 * - MMC depends on a different chipselect management policy than the
 *   SPI interface currently supports for shared bus segments:  it needs
 *   to issue multiple spi_message requests with the chipselect active,
 *   using the results of one message to decide the next one to issue.
 *
 *   Pending updates to the programming interface, this driver expects
 *   that it not share the bus with other drivers (precluding conflicts).
 *
 * - We tell the controller to keep the chipselect active from the
 *   beginning of an mmc_host_ops.request until the end.  So beware
 *   of SPI controller drivers that mis-handle the cs_change flag!
 *
 *   However, many cards seem OK with chipselect flapping up/down
 *   during that time ... at least on unshared bus segments.
 */
---

So we can investigate with the SPI framework update.

>  There is one messaging scheme that i cant figure out how to create
>  correctly using spi messages and transfers that is important for the
>  MMC/SD SPI protocol.
>
>  Example of writing a single 512 byte block to SD/MMC, CS need to be
>  asserted during whole sequence, no other traffic can be allowed.
>
>  Send command:           6 Bytes
>  Poll for cmd response:  1 Byte( appears after 1-3 bytes read)
>  Send data+token:        512+1 Bytes
>  Poll for data response: 1 Byte (within a few bytes)
>     --- card is now busy for a while and it is ok to deselct chip ---
>  Poll for not busy:      coult take 30-10000 bytes at 20 Mhz to finnish
>
>  Last step can be done blockwise, 64 bytes a time by DMA for example..
>  'not busy' is just a matter of transison from busy tokens to idle tokens.
>
>  Note that 'send data+token' can only be done if a specific cmd response
>  was found. Same for data response, if not correct response another path
>  in code will be taken(ask for card status). All this needs to be done
>  "atomically" on the bus. Can this be achieved with the current state of
>  the spi driver?
>

I will copy this email to SPI mail list and mmc_spi maintainers for help.
Thanks Hans,
-Bryan

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Uclinux-dist-devel] spi_mmc bus concurrency fix
       [not found]     ` <386072610802262023t6bd22875kfeb58c797be66024-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2008-02-27  4:46       ` David Brownell
       [not found]         ` <200802262046.29906.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: David Brownell @ 2008-02-27  4:46 UTC (permalink / raw)
  To: Bryan Wu
  Cc: dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f, hp-VrBV9hrLPhE,
	uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Hans Eklund

On Tuesday 26 February 2008, Bryan Wu wrote:
> On Tue, Feb 26, 2008 at 11:16 PM, Hans Eklund <hans-9ai+6vhHfRGzQB+pC5nmwQ@public.gmane.org> wrote:

> --
> /* NOTES:
>  *
>  * ...
>  *
>  * - MMC depends on a different chipselect management policy than the
>  *   SPI interface currently supports for shared bus segments:  it needs
>  *   to issue multiple spi_message requests with the chipselect active,
>  *   using the results of one message to decide the next one to issue.
>  *
>  *   Pending updates to the programming interface, this driver expects
>  *   that it not share the bus with other drivers (precluding conflicts).
>  *
>  * - We tell the controller to keep the chipselect active from the
>  *   beginning of an mmc_host_ops.request until the end.  So beware
>  *   of SPI controller drivers that mis-handle the cs_change flag!
>  *
>  *   However, many cards seem OK with chipselect flapping up/down
>  *   during that time ... at least on unshared bus segments.
>  */
> ---
> 
> So we can investigate with the SPI framework update.
> 
> >  There is one messaging scheme that i cant figure out how to create
> >  correctly using spi messages and transfers that is important for the
> >  MMC/SD SPI protocol.

I'm not following something here.  Is there another MMC-over-SPI
driver being developed?  Why not use the current one?


> >  Example of writing a single 512 byte block to SD/MMC, CS need to be
> >  asserted during whole sequence, no other traffic can be allowed.
> >
> >  Send command:           6 Bytes
> >  Poll for cmd response:  1 Byte( appears after 1-3 bytes read)
> >  Send data+token:        512+1 Bytes
> >  Poll for data response: 1 Byte (within a few bytes)
> >     --- card is now busy for a while and it is ok to deselct chip ---
> >  Poll for not busy:      coult take 30-10000 bytes at 20 Mhz to finnish
> >
> >  Last step can be done blockwise, 64 bytes a time by DMA for example..
> >  'not busy' is just a matter of transison from busy tokens to idle tokens.

Right, and there are plenty of other similar examples.  The current
drivers/mmc/host/mmc_spi.c code handles that, if the underlying SPI
controller driver handles the chipselecct as expected.


> >  Note that 'send data+token' can only be done if a specific cmd response
> >  was found. Same for data response, if not correct response another path
> >  in code will be taken(ask for card status). All this needs to be done
> >  "atomically" on the bus. Can this be achieved with the current state of
> >  the spi driver?

As noted above.  There are two issues:  (a) keeping other drivers off
the bus, and (b) making sure chipselect doesn't go inactive at the wrong
place.

The *current* assumption is that the bus will be unshared, so (a) isn't
an issue ... and that the SPI master controller driver will implement
the chipselect hinting mechanism (spi_transfer.cs_change set on the
last transfer).


We do need better solutions for (a) though.

- Dave

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Uclinux-dist-devel] spi_mmc bus concurrency fix
       [not found]         ` <200802262046.29906.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
@ 2008-02-27 18:15           ` Mike Frysinger
  2008-02-28  3:53           ` Bryan Wu
  1 sibling, 0 replies; 11+ messages in thread
From: Mike Frysinger @ 2008-02-27 18:15 UTC (permalink / raw)
  To: David Brownell
  Cc: Bryan Wu, dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f,
	hp-VrBV9hrLPhE,
	uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

On Tue, Feb 26, 2008 at 11:46 PM, David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote:
> On Tuesday 26 February 2008, Bryan Wu wrote:
>  > On Tue, Feb 26, 2008 at 11:16 PM, Hans Eklund <hans-9ai+6vhHfRGzQB+pC5nmwQ@public.gmane.org> wrote:
>
>  > --
>  > /* NOTES:
>  >  *
>  >  * ...
>
> >  *
>  >  * - MMC depends on a different chipselect management policy than the
>  >  *   SPI interface currently supports for shared bus segments:  it needs
>  >  *   to issue multiple spi_message requests with the chipselect active,
>  >  *   using the results of one message to decide the next one to issue.
>  >  *
>  >  *   Pending updates to the programming interface, this driver expects
>  >  *   that it not share the bus with other drivers (precluding conflicts).
>  >  *
>  >  * - We tell the controller to keep the chipselect active from the
>  >  *   beginning of an mmc_host_ops.request until the end.  So beware
>  >  *   of SPI controller drivers that mis-handle the cs_change flag!
>  >  *
>  >  *   However, many cards seem OK with chipselect flapping up/down
>  >  *   during that time ... at least on unshared bus segments.
>  >  */
>  > ---
>  >
>  > So we can investigate with the SPI framework update.
>  >
>  > >  There is one messaging scheme that i cant figure out how to create
>  > >  correctly using spi messages and transfers that is important for the
>  > >  MMC/SD SPI protocol.
>
>  I'm not following something here.  Is there another MMC-over-SPI
>  driver being developed?  Why not use the current one?

a MMC-over-SPI driver for the Blackfin was developed by Hans long
before there was a SPI framework in the kernel let alone the
MMC-over-SPI driver that is in there now.  i suggested he look at
integrating anything from his driver into the one that's in mainline
now and while that'll happen at some point, it looks like he can get
the issues he has with the old one sorted out sooner.
-mike

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: spi_mmc bus concurrency fix
       [not found]         ` <200802262046.29906.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
  2008-02-27 18:15           ` Mike Frysinger
@ 2008-02-28  3:53           ` Bryan Wu
  1 sibling, 0 replies; 11+ messages in thread
From: Bryan Wu @ 2008-02-28  3:53 UTC (permalink / raw)
  To: David Brownell
  Cc: dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f, hp-VrBV9hrLPhE,
	uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	jan.nikitenko-Re5JQEeQqe8AvxtiuMwx3w

On Wed, Feb 27, 2008 at 12:46 PM, David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote:
> On Tuesday 26 February 2008, Bryan Wu wrote:
>  > On Tue, Feb 26, 2008 at 11:16 PM, Hans Eklund <hans-9ai+6vhHfRGzQB+pC5nmwQ@public.gmane.org> wrote:
>
>  >
>  > So we can investigate with the SPI framework update.
>  >
>  > >  There is one messaging scheme that i cant figure out how to create
>  > >  correctly using spi messages and transfers that is important for the
>  > >  MMC/SD SPI protocol.
>
>  I'm not following something here.  Is there another MMC-over-SPI
>  driver being developed?  Why not use the current one?
>

Yes, Hans developed a MMC-over-SPI driver before the mainline implementation.
His SPI_MMC driver has been in Blackfin local repo for a long time.
http://docs.blackfin.uclinux.org/doku.php?id=mmc_driver&s=spi

At beginning this driver does not use the SPI framework API,
now Hans is working on to more his driver to use kernel SPI framework API. Then
He met the bus sharing issue.

>
>

>
>  > >  Note that 'send data+token' can only be done if a specific cmd response
>  > >  was found. Same for data response, if not correct response another path
>  > >  in code will be taken(ask for card status). All this needs to be done
>  > >  "atomically" on the bus. Can this be achieved with the current state of
>  > >  the spi driver?
>
>  As noted above.  There are two issues:  (a) keeping other drivers off
>  the bus, and (b) making sure chipselect doesn't go inactive at the wrong
>  place.
>
>  The *current* assumption is that the bus will be unshared, so (a) isn't
>  an issue ... and that the SPI master controller driver will implement
>  the chipselect hinting mechanism (spi_transfer.cs_change set on the
>  last transfer).
>
>
>  We do need better solutions for (a) though.
>

As we discussed here, the main issue is
a) upper level spi device driver can submit I/O request to spi-core,
spi-core will schedule the real read/write I/O operation to lower
level hardware
b) some spi devices require to make sure atomically bus I/O
transcation which can not be interrupt by other spi devices sharing
the same bus.
spi-core should provide some API to meet this requirement.

-Bryan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Uclinux-dist-devel] spi_mmc bus concurrency fix
       [not found]     ` <47C5B084.6050408-j9pdmedNgrk@public.gmane.org>
@ 2008-02-28  5:55       ` David Brownell
       [not found]         ` <200802272155.29706.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: David Brownell @ 2008-02-28  5:55 UTC (permalink / raw)
  To: pwilshire-j9pdmedNgrk
  Cc: bryan.wu-OyLXuOCK7orQT0dZR+AlfA,
	uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b, Hans Eklund,
	Mike Frysinger,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

On Wednesday 27 February 2008, Phil Wilshire wrote:
> 
> Thats why I wanted an optional callback after each single transfer.

That would preclude using chained DMA requests in those cases, but
presumably that'd be OK in this context.


> So if you are waiting  for a status byte in the spi data
> then the callback would add the a small read transfer a few times while 
> checking the status.

Just a few?  If you're assuming it would add a new spi_transfer after
the existing ones in that message ... that could add up to quite a lot
of additional transfers, presumably ones that are dynamically allocated.

I've seen some MMC/SD cards need quite a lot of polling there...


> When it times out (IE reaches a max retry limit) then the transfer can 
> be halted returning some kind of failure state.
> 
> If the correct status is found then the next transfer can be the data to 
> be written  to the device.
> 
> I thought about the single device queue based in chip select but that 
> looked a but more complex.

That is, one queue per device.  Yes, more complicated than one
shared between devices.  I was just pointing that out as an option
that could be implemented transparently ... albeit not portably.


> With the TRANSFER callback we have a good (IMHO) way to modify the 
> message flow based on the data contents.
> 
> The complete function may need two arguments. One to hold the SPI
> Transfer in progress and the other to hold user state
> (like max read loops and final state).
> 
> Let me know if this would fit in the "overall formal design" and I'll 
> stop hacking a custom solution.

It doesn't seem like the right approach to me.  In the specific use
case of an MMC interface, three'd be a *LOT* of those callbacks, each
adding another transfer to the current message.

And that breaks assumptions like being able to validate entire
messages before starting to process them, bounded memory use,
and so on.  I'd far rather see something that does less damage
to the underlying models.

- Dave


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Uclinux-dist-devel] spi_mmc bus concurrency fix
       [not found]     ` <386072610802272023o71b99c2bn4d28547bd8783c9c-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2008-02-28  6:04       ` David Brownell
  0 siblings, 0 replies; 11+ messages in thread
From: David Brownell @ 2008-02-28  6:04 UTC (permalink / raw)
  To: Bryan Wu
  Cc: bryan.wu-OyLXuOCK7orQT0dZR+AlfA,
	uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b,
	Mike Frysinger,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

On Wednesday 27 February 2008, Bryan Wu wrote:
> On Wed, Feb 27, 2008 at 5:59 PM, David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote:
> > > >  To speak my mind, i feel i am imposed a troublesome messaging scheme
> >  > >  from the SPI framework. Maybe it has to do with buggy controller,
> >  > >  controller driver, buggy chips with erratas longer than the reference
> >  > >  manual. Well, wont rant about that right now. Need to solve this problem
> >  > >  instead.
> >
> >  We sort of lack a testing framework for SPI master drivers.  The best
> >  solution for now seems to be to make sure several existing drivers
> >  work with it ...
>  
> I remember some LTP guys are developing bus driver testing case such
> as i2c, spi ,etc, but don't know the detail.

When I did it for USB, I essentially defined a "golden device" that could
be the target of such testing.  Easy to do with various microcontrollers
with I2C; a bit less so with SPI, because you'd also want to test higher
data rates than most micros like.


> For the testing of Blackfin, we tested SPI touchscreen, SPI
> joystick, SPI flash and a SPI MMC driver over SPI framework.

Better coverage than many systems.  :)


> >  > >  I hope to make the block writing  working this
> >  > >  weekend. I need to make it work since we want to use the inexpensive
> >  > >  BF532 chip + SPI ethernet + MMC/SD for storage in a product.
> >  > >
> >  > >  I´ll commit my result to the drivers/mmc/spi_mmc, even though its
> >  > >  obsolete maybe the mainline driver developers can make use of my progress.
> >  >
> >  > sounds great !
> >
> >  The spi-devel list archives have at least one patch providing a
> >  mutual exclusion mechanism at the bus level.  One problem is that
> >  all such mechanisms evidently need controller driver changes,
> >  since they involve changing the I/O queue managemnt policy.
> >
> 
> So how about move the controller driver queue management stuff to spi-core?
> controller driver just send out data and receive data.

That can be *part* of the solution, although I'd rather see the
controller drivers talking in terms of messages:  "get me the
next message", with the core knowing how to handle things like
this exclusive-access policy.

The rest of the solution involves implementation and interface
details, including just how to gracefully migrate to that model.

The original notion was not to have any kind of mid-layer ... in
my experience they're magnets for code bloat.  But in this case
I'm not sure I see a really good solution other than that.

- Dave


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: spi_mmc bus concurrency fix
       [not found]         ` <200802272155.29706.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
@ 2008-02-28  8:15           ` Phil Wilshire
       [not found]             ` <47C66D8A.3010904-j9pdmedNgrk@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Phil Wilshire @ 2008-02-28  8:15 UTC (permalink / raw)
  To: David Brownell
  Cc: bryan.wu-OyLXuOCK7orQT0dZR+AlfA,
	uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

David Brownell wrote:
> On Wednesday 27 February 2008, Phil Wilshire wrote:
>> Thats why I wanted an optional callback after each single transfer.
> 
> That would preclude using chained DMA requests in those cases, but
> presumably that'd be OK in this context.
> 
> 
>> So if you are waiting  for a status byte in the spi data
>> then the callback would add the a small read transfer a few times while 
>> checking the status.
> 
> Just a few?  If you're assuming it would add a new spi_transfer after
> the existing ones in that message ... that could add up to quite a lot
> of additional transfers, presumably ones that are dynamically allocated.
> 
> I've seen some MMC/SD cards need quite a lot of polling there...
> 
> 
>> When it times out (IE reaches a max retry limit) then the transfer can 
>> be halted returning some kind of failure state.
>>
>> If the correct status is found then the next transfer can be the data to 
>> be written  to the device.
>>
>> I thought about the single device queue based in chip select but that 
>> looked a but more complex.
> 
> That is, one queue per device.  Yes, more complicated than one
> shared between devices.  I was just pointing that out as an option
> that could be implemented transparently ... albeit not portably.
> 
> 
>> With the TRANSFER callback we have a good (IMHO) way to modify the 
>> message flow based on the data contents.
>>
>> The complete function may need two arguments. One to hold the SPI
>> Transfer in progress and the other to hold user state
>> (like max read loops and final state).
>>
>> Let me know if this would fit in the "overall formal design" and I'll 
>> stop hacking a custom solution.
> 
> It doesn't seem like the right approach to me.  In the specific use
> case of an MMC interface, three'd be a *LOT* of those callbacks, each
> adding another transfer to the current message.
> 
> And that breaks assumptions like being able to validate entire
> messages before starting to process them, bounded memory use,
> and so on.  I'd far rather see something that does less damage
> to the underlying models.
> 
> - Dave
> 
> 
Hi Dave,

I agree, I did have some concerns about the callback issue.
It also looks like we have enough people + momentum on this problem to
get a fix in good time. So I'll wait to see what happens.

Thanks for the reply
   Phil Wilshire

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Uclinux-dist-devel] spi_mmc bus concurrency fix
       [not found]             ` <47C66D8A.3010904-j9pdmedNgrk@public.gmane.org>
@ 2008-03-12 22:01               ` David Brownell
       [not found]                 ` <200803121501.19131.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: David Brownell @ 2008-03-12 22:01 UTC (permalink / raw)
  To: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	pwilshire-j9pdmedNgrk
  Cc: bryan.wu-OyLXuOCK7orQT0dZR+AlfA,
	uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b, Hans Eklund,
	Mike Frysinger

On Thursday 28 February 2008, Phil Wilshire wrote:
> I agree, I did have some concerns about the callback issue.
> It also looks like we have enough people + momentum on this problem to
> get a fix in good time. So I'll wait to see what happens.

Actually, I'm not sure I'd agree on that.  Folk want a fix,
but nobody (including me!) has sent something that's quite
mergeable yet.

I think part of the solution may be what someone (Brian?)
observed:  that growing the mid-layer to handle some of the
queue management functions should help, no matter what
eventual solution is adopted.

That might be as little as providing a default "add to queue"
and "take from queue" mechanism and starting to phase it into
use in the various master controller drivers.  When they use
that scheme, it should be easy to splice in alternative queue
management policies (notably:  "only spi_mmc can access the
bus for now") without changing the controller drivers.

So it'd be really nice if someone could come up with a patch
like that to help get some forward motion ... unless there's
a better suggestion for how to move forward?

- Dave



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [spi-devel-general] spi_mmc bus concurrency fix
       [not found]                 ` <200803121501.19131.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
@ 2008-03-20 11:22                   ` Hans Eklund
  2008-03-25 10:25                   ` Hans Eklund
  1 sibling, 0 replies; 11+ messages in thread
From: Hans Eklund @ 2008-03-20 11:22 UTC (permalink / raw)
  To: David Brownell
  Cc: bryan.wu-OyLXuOCK7orQT0dZR+AlfA,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b

David Brownell skrev:
> On Thursday 28 February 2008, Phil Wilshire wrote:
>> I agree, I did have some concerns about the callback issue.
>> It also looks like we have enough people + momentum on this problem to
>> get a fix in good time. So I'll wait to see what happens.
> 
> Actually, I'm not sure I'd agree on that.  Folk want a fix,
> but nobody (including me!) has sent something that's quite
> mergeable yet.
Hi again.

I have not solved the roots to the problem. Im not sure im the man to do 
that design in the end. But, I have actually MMC/SD cards running 
concurrently with m25p80 spi flash driver! No fix to either the master 
driver or the framework. Both reading and writing. No multiple block 
writings so far. But it is quite cool to see it spin.

It should be easy to move the solution over to the mainline mmc_spi 
driver once im satisfied with the implementation

you can see how i build the command and response transfers on before 
hand and then issue a single spi_sync to get it onto the bus under a 
single CS. I´ll have to add some more error detection and probably a 
status check after the busy poll phase in the end.

Hope to test it with enc28j60 driver this afternoon and send it to Phil 
who i believe was eager to test something.

/regards
Hans Eklund


static int spi_mmc_write_mmc_block(mmc_info_t* pdev, unsigned char* buf, 
unsigned int address)
{
	struct spi_transfer t[4];
	struct spi_message m;
	struct spi_message b;
	struct spi_transfer bt;
	int i=0;
	unsigned char wanted_token;

	#define MMC_CMD_LEN		6
	#define CMD_RESP_LEN		10
	#define TOKEN_AND_DATA_LEN	513
	#define DATA_RESP_LEN		100
	#define BUSY_BLOCK_LEN		64

	unsigned char mmc_cmd[MMC_CMD_LEN];
	unsigned char cmd_resp[CMD_RESP_LEN];
	unsigned char token_and_data[TOKEN_AND_DATA_LEN];
	unsigned char data_resp[DATA_RESP_LEN];
	unsigned char busy_block[BUSY_BLOCK_LEN];

	// Build write block command
	mmc_cmd[0] = 0x40 + 24;
	mmc_cmd[1] = (unsigned char)(address >> 24 & 0xff);
	mmc_cmd[2] = (unsigned char)(address >> 16 & 0xff);
	mmc_cmd[3] = (unsigned char)(address >> 8 & 0xff);
	mmc_cmd[4] = (unsigned char)(address & 0xff);
	mmc_cmd[5] = 0x95;	// CRC from CMD0 actually, but valid for all since 
SPI dont care

	// Build token+data chunk
	token_and_data[0] = (unsigned char)0xFE;	
	memcpy(token_and_data+1, buf, TOKEN_AND_DATA_LEN-1);

	// Zero out messages
	spi_message_init(&m);
	memset(t, 0, (sizeof t));

	#ifdef CONFIG_BLACKFIN
	blackfin_dcache_flush_range((unsigned long)mmc_cmd,(unsigned 
long)(mmc_cmd+MMC_CMD_LEN));
	blackfin_dcache_flush_range((unsigned long)token_and_data,(unsigned 
long)(token_and_data+TOKEN_AND_DATA_LEN));
	#endif

	t[0].tx_buf = mmc_cmd;
	t[0].len = MMC_CMD_LEN;
	spi_message_add_tail(&t[0], &m);

	t[1].rx_buf = cmd_resp;
	t[1].len = CMD_RESP_LEN;
	spi_message_add_tail(&t[1], &m);

	t[2].tx_buf = token_and_data;
	t[2].len = TOKEN_AND_DATA_LEN;
	spi_message_add_tail(&t[2], &m);

	t[3].rx_buf = data_resp;
	t[3].len = DATA_RESP_LEN;
	spi_message_add_tail(&t[3], &m);

	// Send it away, Fingers crossed!
	spi_sync(pdev->spi_dev, &m);

	// Cache coherency stuff
	#ifdef CONFIG_BLACKFIN
	blackfin_dcache_invalidate_range((unsigned long)cmd_resp,(unsigned 
long)(cmd_resp+CMD_RESP_LEN));
	blackfin_dcache_invalidate_range((unsigned long)data_resp,(unsigned 
long)(data_resp+DATA_RESP_LEN));
	#endif

	// Analyze response buffers

	// R1_OK token
	wanted_token = 0x00;
	for(i=0; i<CMD_RESP_LEN; i++) {
		if(cmd_resp[i] == wanted_token) {
			DPRINTK("Found wanted CMD token(0x%02x) at %d\n", cmd_resp[i], i);
			goto found_cmd_token;
		}
	}
	// error case here:
	DPRINTK("Did not find CMD response token!\n");
	return -1;

	// OK!
	found_cmd_token:

	// DR_ACCEPTED
	wanted_token = 0x05;
	for(i=0; i<DATA_RESP_LEN; i++) {
		if(data_resp[i] == wanted_token || data_resp[i] == 0x00 || 
data_resp[i] == 0x01 || data_resp[i] == 0x04) {
			DPRINTK("Found wanted DATA response token(0x%02x) at %d\n", 
data_resp[i], i);
			goto found_acc_token;
		}
	}
	// error case here:
	DPRINTK("Did not find DATA response token!\n");
	return -1;

	// OK!
	found_acc_token:

	// Read until NOT busy anymore
	i=1000;
	while(i--) {
		// it wont be done right away, be nice...
		schedule();
		memset(busy_block, 0, BUSY_BLOCK_LEN);
		spi_message_init(&b);
		memset(&bt, 0, (sizeof bt));
		bt.rx_buf = busy_block;
		bt.len = BUSY_BLOCK_LEN;
		spi_message_add_tail(&bt, &b);
		spi_sync(pdev->spi_dev, &b);
		blackfin_dcache_invalidate_range((unsigned long)busy_block,(unsigned 
long)(busy_block+BUSY_BLOCK_LEN));
		if(busy_block[BUSY_BLOCK_LEN-1] == 0xFF) {
			DPRINTK("Detected transiton from busy to NOT busy, doNE!\n");
			goto done;
		}
	}
	printk("Busy transition timeout! Not good!");
	return -1;

	done:

	// Successfully wrote block to MMC!
	return 0;

}

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [spi-devel-general] spi_mmc bus concurrency fix
       [not found]                 ` <200803121501.19131.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
  2008-03-20 11:22                   ` [spi-devel-general] " Hans Eklund
@ 2008-03-25 10:25                   ` Hans Eklund
  1 sibling, 0 replies; 11+ messages in thread
From: Hans Eklund @ 2008-03-25 10:25 UTC (permalink / raw)
  To: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
  Cc: David Brownell,
	uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b,
	bryan.wu-OyLXuOCK7orQT0dZR+AlfA

Hi again.

Lets move on.

I have not solved the roots to the problem. I´ve not been given time to 
fiddle with the framework internals. Im not sure im the man to do that 
in the end. But, I have managed to get MMC/SD cards running concurrently 
with m25p80 spi flash driver. No fix to either the master
driver or the framework. Both reading and writing and card init works. 
No multiple block writing yet though. All done vs uClinux 2008R1-RC8 
(kernel 2.6.22) and my old spi_mmc driver which implemented a separate 
block layer. Written and tested mostly within the Blackfin uClinux 
community.

About the fix: In short i predefine a sequence of transfers in a single 
message matching the MMC/SD protocol, simply assuming it will work, 
checking the tokens and if tokens looked fine return with a smile. It 
seem to work. It is the only way to get atomicity on the bus for now. It 
is surley best to patch up the drivers/mmc/host/mmc_spi.c code and use 
that driver from now on, putting my old spi_mmc driver on the shelf.

Bryan, Mike and other Blackfin-uClinux developers:

What is the status on the drivers/mmc/host/mmc_spi.c when connected to 
the Blackfin spi master? Mike pointed out that my old spi_mmc should be 
obsoleted(with all rights), with that statement I got the feeling the 
"new" mainline driver was tested to some extent, correct? I would need a 
test/develop environment to move on with this. I´ll try to backport the 
mmc_spi found in 2.6.24 to fit 2008R1-RC8 as a start, or do you have any 
other suggestions?

I hope i get the attention of Jan Nikitenko and Hans-Peter Nilsson on 
this subject also as they seem to have been a part of the mmc_spi driver 
developing.

/regards
Hans Eklund,
Rubico AB,
Sweden.

David Brownell skrev:
> On Thursday 28 February 2008, Phil Wilshire wrote:
>> I agree, I did have some concerns about the callback issue.
>> It also looks like we have enough people + momentum on this problem to
>> get a fix in good time. So I'll wait to see what happens.
> 
> Actually, I'm not sure I'd agree on that.  Folk want a fix,
> but nobody (including me!) has sent something that's quite
> mergeable yet.
> 
> I think part of the solution may be what someone (Brian?)
> observed:  that growing the mid-layer to handle some of the
> queue management functions should help, no matter what
> eventual solution is adopted.
> 
> That might be as little as providing a default "add to queue"
> and "take from queue" mechanism and starting to phase it into
> use in the various master controller drivers.  When they use
> that scheme, it should be easy to splice in alternative queue
> management policies (notably:  "only spi_mmc can access the
> bus for now") without changing the controller drivers.
> 
> So it'd be really nice if someone could come up with a patch
> like that to help get some forward motion ... unless there's
> a better suggestion for how to move forward?
> 
> - Dave
> 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Uclinux-dist-devel] spi_mmc bus concurrency fix
@ 2008-03-25 10:25 Hans Eklund
  0 siblings, 0 replies; 11+ messages in thread
From: Hans Eklund @ 2008-03-25 10:25 UTC (permalink / raw)
  To: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f


[-- Attachment #1.1: Type: text/plain, Size: 2973 bytes --]

Hi again.

Lets move on.

I have not solved the roots to the problem. I´ve not been given time to
fiddle with the framework internals. Im not sure im the man to do that in
the end. But, I have managed to get MMC/SD cards running concurrently with
m25p80 spi flash driver. No fix to either the master driver or the
framework. Both reading and writing and card init works. No multiple block
writing yet though. All done vs uClinux 2008R1-RC8 (kernel 2.6.22) and my
old spi_mmc driver which implemented a separate block layer. Written and
tested mostly within the Blackfin uClinux community.

About the fix: In short i predefine a sequence of transfers in a single
message matching the MMC/SD protocol, simply assuming it will work, checking
the tokens and if tokens looked fine return with a smile. It seem to work.
It is the only way to get atomicity on the bus for now. It is surley best to
patch up the drivers/mmc/host/mmc_spi.c code and use that driver from now
on, putting my old spi_mmc driver on the shelf.

Bryan, Mike and other Blackfin-uClinux developers:

What is the status on the drivers/mmc/host/mmc_spi.c when connected to the
Blackfin spi master? Mike pointed out that my old spi_mmc should be
obsoleted(with all rights), with that statement I got the feeling the "new"
mainline driver was tested to some extent, correct? I would need a
test/develop environment to move on with this. I´ll try to backport the
mmc_spi found in 2.6.24 to fit 2008R1-RC8 as a start, or do you have any
other suggestions?

I hope i get the attention of Jan Nikitenko and Hans-Peter Nilsson on this
subject also as they seem to have been a part of the mmc_spi driver
developing.

/regards
Hans Eklund,
Rubico AB,
Sweden.

David Brownell skrev:
> On Thursday 28 February 2008, Phil Wilshire wrote:
>> I agree, I did have some concerns about the callback issue.
>> It also looks like we have enough people + momentum on this problem to
>> get a fix in good time. So I'll wait to see what happens.
>
> Actually, I'm not sure I'd agree on that. Folk want a fix,
> but nobody (including me!) has sent something that's quite
> mergeable yet.
>
> I think part of the solution may be what someone (Brian?)
> observed: that growing the mid-layer to handle some of the
> queue management functions should help, no matter what
> eventual solution is adopted.
>
> That might be as little as providing a default "add to queue"
> and "take from queue" mechanism and starting to phase it into
> use in the various master controller drivers. When they use
> that scheme, it should be easy to splice in alternative queue
> management policies (notably: "only spi_mmc can access the
> bus for now") without changing the controller drivers.
>
> So it'd be really nice if someone could come up with a patch
> like that to help get some forward motion ... unless there's
> a better suggestion for how to move forward?
>
> - Dave
>
>

[-- Attachment #1.2: Type: text/html, Size: 3242 bytes --]

[-- Attachment #2: Type: text/plain, Size: 228 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

[-- Attachment #3: Type: text/plain, Size: 210 bytes --]

_______________________________________________
spi-devel-general mailing list
spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2008-03-25 10:25 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <47C42D50.7020204@rubico.se>
     [not found] ` <47C42D50.7020204-9ai+6vhHfRGzQB+pC5nmwQ@public.gmane.org>
2008-02-27  4:23   ` [Uclinux-dist-devel] spi_mmc bus concurrency fix Bryan Wu
     [not found]     ` <386072610802262023t6bd22875kfeb58c797be66024-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-02-27  4:46       ` David Brownell
     [not found]         ` <200802262046.29906.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2008-02-27 18:15           ` Mike Frysinger
2008-02-28  3:53           ` Bryan Wu
     [not found] ` <200802271036.07909.david-b@pacbell.net>
     [not found]   ` <47C5B084.6050408@cox.net>
     [not found]     ` <47C5B084.6050408-j9pdmedNgrk@public.gmane.org>
2008-02-28  5:55       ` [Uclinux-dist-devel] " David Brownell
     [not found]         ` <200802272155.29706.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2008-02-28  8:15           ` Phil Wilshire
     [not found]             ` <47C66D8A.3010904-j9pdmedNgrk@public.gmane.org>
2008-03-12 22:01               ` [Uclinux-dist-devel] " David Brownell
     [not found]                 ` <200803121501.19131.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2008-03-20 11:22                   ` [spi-devel-general] " Hans Eklund
2008-03-25 10:25                   ` Hans Eklund
     [not found] ` <200802270159.33231.david-b@pacbell.net>
     [not found]   ` <386072610802272023o71b99c2bn4d28547bd8783c9c@mail.gmail.com>
     [not found]     ` <386072610802272023o71b99c2bn4d28547bd8783c9c-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-02-28  6:04       ` [Uclinux-dist-devel] " David Brownell
2008-03-25 10:25 Hans Eklund

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).