[U-Boot] spi subystem maintainer?
Dear Kumar Gala,
In message 9A3702C1-3ED1-4F24-A0BC-CA7DAEA46182@kernel.crashing.org you wrote:
Do we have one?
Not yet. Are you volunteering?
Best regards,
Wolfgang Denk
On Feb 1, 2011, at 1:29 PM, Wolfgang Denk wrote:
Dear Kumar Gala,
In message 9A3702C1-3ED1-4F24-A0BC-CA7DAEA46182@kernel.crashing.org you wrote:
Do we have one?
Not yet. Are you volunteering?
Nope, too much to do as it stands.
Wanted to see if anyone had input on how to deal with the SPI controller on some of our newer parts. It expects command & data xfer's to happen together. However our current code does not call spi_xfer() that way.
- k
On 02/02/2011 08:23 AM, Kumar Gala wrote:
Wanted to see if anyone had input on how to deal with the SPI controller on some of our newer parts. It expects command & data xfer's to happen together. However our current code does not call spi_xfer() that way.
Which is your concrete case ? spi_xfer is responsible to setup the controller and to start the transfer, and everything could be done inside this function. What do you mean exactly with command and data ?
Regards, Stefano
Dear Stefano Babic:
On 02/02/2011 08:23 AM, Kumar Gala wrote:
Wanted to see if anyone had input on how to deal with the SPI controller on some of our newer parts. It expects command & data xfer's to happen together. However our current code does not call spi_xfer() that way.
Which is your concrete case ? spi_xfer is responsible to setup the controller and to start the transfer, and everything could be done inside this function. What do you mean exactly with command and data ?
Regards, Stefano
I think he refers to the common "problem" that many SPI devices require CS to stay low during both "phases" of issuing the read/write command and transfering the actual data.
Current u-boot code calls spi_xfer() two times.
Hardware controlled CS often go high between both calls, which requires you to (at least) use GPIO controlled CS, or, even worse, use bitbang SPI (in cases where the SPI pin assignment is in groups, not individually).
Best Regards, Reinhard
On Feb 2, 2011, at 3:30 AM, Reinhard Meyer wrote:
Dear Stefano Babic:
On 02/02/2011 08:23 AM, Kumar Gala wrote:
Wanted to see if anyone had input on how to deal with the SPI controller on some of our newer parts. It expects command & data xfer's to happen together. However our current code does not call spi_xfer() that way.
Which is your concrete case ? spi_xfer is responsible to setup the controller and to start the transfer, and everything could be done inside this function. What do you mean exactly with command and data ?
Regards, Stefano
I think he refers to the common "problem" that many SPI devices require CS to stay low during both "phases" of issuing the read/write command and transfering the actual data.
Current u-boot code calls spi_xfer() two times.
Hardware controlled CS often go high between both calls, which requires you to (at least) use GPIO controlled CS, or, even worse, use bitbang SPI (in cases where the SPI pin assignment is in groups, not individually).
That's correct, and with the newer FSL controller's we dont have direct control over the CS. I'm thinking we need to have the command and response dealt with in a single call to spi_xfer instead of what we seem to do all over the place today:
ret = spi_xfer(spi, 8, &cmd, NULL, flags); if (ret) { debug("SF: Failed to send command %02x: %d\n", cmd, ret); return ret; }
if (len) { ret = spi_xfer(spi, len * 8, NULL, response, SPI_XFER_END); if (ret) debug("SF: Failed to read response (%zu bytes): %d\n", len, ret); }
Needs to turn into something like:
ret = spi_xfer(spi, 8 + len * 8, &cmd, response, flags | SPI_XFER_END)
- k
On 02/03/2011 11:36 AM, Kumar Gala wrote:
That's correct, and with the newer FSL controller's we dont have direct control over the CS.
I know. You are probably talking about PowerPC, but it is the same for the i.MX processors.
I'm thinking we need to have the command and response dealt with in a single call to spi_xfer instead of what we seem to do all over the place today:
ret = spi_xfer(spi, 8, &cmd, NULL, flags); if (ret) { debug("SF: Failed to send command %02x: %d\n", cmd, ret); return ret; }
Ok. You are not talking generally about spi, but how to manage spi flash. I saw the same problem with the i.MX51, but then a GPIO was used instead of the internal SS and I forgot this issue. We do not need to change the spi_xfer() call (I mean, in the spi controllers).
if (len) { ret = spi_xfer(spi, len * 8, NULL, response, SPI_XFER_END); if (ret) debug("SF: Failed to read response (%zu bytes): %d\n", len, ret); }
Needs to turn into something like:
ret = spi_xfer(spi, 8 + len * 8, &cmd, response, flags | SPI_XFER_END)
I think it could depend on the SPI flash you use. I checked in stmicro.c, and the mechanism to split the enabling of SS and the transfer of data is really used.
In stmicro_wait_ready(), it is used to poll the status of the flash. The SS is active until the flash has complete the operation. Probably the flash can accept a single call, too, but I am unsure.
Stefano
On Thursday, February 03, 2011 05:36:38 Kumar Gala wrote:
On Feb 2, 2011, at 3:30 AM, Reinhard Meyer wrote:
Dear Stefano Babic:
On 02/02/2011 08:23 AM, Kumar Gala wrote:
Wanted to see if anyone had input on how to deal with the SPI controller on some of our newer parts. It expects command & data xfer's to happen together. However our current code does not call spi_xfer() that way.
Which is your concrete case ? spi_xfer is responsible to setup the controller and to start the transfer, and everything could be done inside this function. What do you mean exactly with command and data ?
Regards, Stefano
I think he refers to the common "problem" that many SPI devices require CS to stay low during both "phases" of issuing the read/write command and transfering the actual data.
Current u-boot code calls spi_xfer() two times.
Hardware controlled CS often go high between both calls, which requires you to (at least) use GPIO controlled CS, or, even worse, use bitbang SPI (in cases where the SPI pin assignment is in groups, not individually).
That's correct, and with the newer FSL controller's we dont have direct control over the CS. I'm thinking we need to have the command and response dealt with in a single call to spi_xfer instead of what we seem to do all over the place today:
ret = spi_xfer(spi, 8, &cmd, NULL, flags); if (ret) { debug("SF: Failed to send command %02x: %d\n", cmd, ret); return ret; } if (len) { ret = spi_xfer(spi, len * 8, NULL, response, SPI_XFER_END); if (ret) debug("SF: Failed to read response (%zu bytes):%d\n", len, ret); }
Needs to turn into something like:
ret = spi_xfer(spi, 8 + len * 8, &cmd, response, flags | SPI_XFER_END)
this should be easier in my sf branch as i unified a bunch of the functions. but while this will probably work for the main commands, how is this supposed to work for the status polling ? that function is fundamentally based around setting up a transfer/command and then continuously shifting out a single result and checking it before shifting out another. for your controller, the only way to make it work is to do the full transaction every time. -mike
On Feb 15, 2011, at 2:36 AM, Mike Frysinger wrote:
On Thursday, February 03, 2011 05:36:38 Kumar Gala wrote:
On Feb 2, 2011, at 3:30 AM, Reinhard Meyer wrote:
Dear Stefano Babic:
On 02/02/2011 08:23 AM, Kumar Gala wrote:
Wanted to see if anyone had input on how to deal with the SPI controller on some of our newer parts. It expects command & data xfer's to happen together. However our current code does not call spi_xfer() that way.
Which is your concrete case ? spi_xfer is responsible to setup the controller and to start the transfer, and everything could be done inside this function. What do you mean exactly with command and data ?
Regards, Stefano
I think he refers to the common "problem" that many SPI devices require CS to stay low during both "phases" of issuing the read/write command and transfering the actual data.
Current u-boot code calls spi_xfer() two times.
Hardware controlled CS often go high between both calls, which requires you to (at least) use GPIO controlled CS, or, even worse, use bitbang SPI (in cases where the SPI pin assignment is in groups, not individually).
That's correct, and with the newer FSL controller's we dont have direct control over the CS. I'm thinking we need to have the command and response dealt with in a single call to spi_xfer instead of what we seem to do all over the place today:
ret = spi_xfer(spi, 8, &cmd, NULL, flags); if (ret) { debug("SF: Failed to send command %02x: %d\n", cmd, ret); return ret; } if (len) { ret = spi_xfer(spi, len * 8, NULL, response, SPI_XFER_END); if (ret) debug("SF: Failed to read response (%zu bytes):%d\n", len, ret); }
Needs to turn into something like:
ret = spi_xfer(spi, 8 + len * 8, &cmd, response, flags | SPI_XFER_END)
this should be easier in my sf branch as i unified a bunch of the functions. but while this will probably work for the main commands, how is this supposed to work for the status polling ? that function is fundamentally based around setting up a transfer/command and then continuously shifting out a single result and checking it before shifting out another. for your controller, the only way to make it work is to do the full transaction every time. -mike
Probably have to do a transaction every time.
Do you have a tree for these changes? Do you expect them to be in place for release after v2011.03
- k
On Tuesday, February 15, 2011 18:10:47 Kumar Gala wrote:
On Feb 15, 2011, at 2:36 AM, Mike Frysinger wrote:
On Thursday, February 03, 2011 05:36:38 Kumar Gala wrote:
On Feb 2, 2011, at 3:30 AM, Reinhard Meyer wrote:
Dear Stefano Babic:
On 02/02/2011 08:23 AM, Kumar Gala wrote:
Wanted to see if anyone had input on how to deal with the SPI controller on some of our newer parts. It expects command & data xfer's to happen together. However our current code does not call spi_xfer() that way.
Which is your concrete case ? spi_xfer is responsible to setup the controller and to start the transfer, and everything could be done inside this function. What do you mean exactly with command and data ?
I think he refers to the common "problem" that many SPI devices require CS to stay low during both "phases" of issuing the read/write command and transfering the actual data.
Current u-boot code calls spi_xfer() two times.
Hardware controlled CS often go high between both calls, which requires you to (at least) use GPIO controlled CS, or, even worse, use bitbang SPI (in cases where the SPI pin assignment is in groups, not individually).
That's correct, and with the newer FSL controller's we dont have direct control over the CS. I'm thinking we need to have the command and response dealt with in a single call to spi_xfer instead of what we seem
to do all over the place today: ret = spi_xfer(spi, 8, &cmd, NULL, flags); if (ret) {
debug("SF: Failed to send command %02x: %d\n", cmd, ret); return ret; } if (len) { ret = spi_xfer(spi, len * 8, NULL, response, SPI_XFER_END); if (ret) debug("SF: Failed to read response (%zu bytes):%d\n", len, ret);
}Needs to turn into something like: ret = spi_xfer(spi, 8 + len * 8, &cmd, response, flags |
SPI_XFER_END)
this should be easier in my sf branch as i unified a bunch of the functions. but while this will probably work for the main commands, how is this supposed to work for the status polling ? that function is fundamentally based around setting up a transfer/command and then continuously shifting out a single result and checking it before shifting out another. for your controller, the only way to make it work is to do the full transaction every time.
Probably have to do a transaction every time.
looking at the Linux driver, it seems to do just that. i guess if Linux is getting by with a stricter API, then there shouldnt be a problem for U-Boot either. i dont suppose anyone knows of devices that are problematic in Linux or would be broken in U-Boot by this API change in general ?
this assumes of course that the SPI API as used in Linux works for you ?
Do you have a tree for these changes? Do you expect them to be in place for release after v2011.03
the sf branch of my blackfin tree. if no one gives me any feedback (i posted the patches on the list a while ago), i guess i'll see about merging post the next release. -mike
On Thursday, February 17, 2011 00:04:35 Mike Frysinger wrote:
On Tuesday, February 15, 2011 18:10:47 Kumar Gala wrote:
On Feb 15, 2011, at 2:36 AM, Mike Frysinger wrote:
On Thursday, February 03, 2011 05:36:38 Kumar Gala wrote:
Needs to turn into something like: ret = spi_xfer(spi, 8 + len * 8, &cmd, response, flags | SPI_XFER_END)
this should be easier in my sf branch as i unified a bunch of the functions. but while this will probably work for the main commands, how is this supposed to work for the status polling ? that function is fundamentally based around setting up a transfer/command and then continuously shifting out a single result and checking it before shifting out another. for your controller, the only way to make it work is to do the full transaction every time.
Probably have to do a transaction every time.
looking at the Linux driver, it seems to do just that. i guess if Linux is getting by with a stricter API, then there shouldnt be a problem for U-Boot either. i dont suppose anyone knows of devices that are problematic in Linux or would be broken in U-Boot by this API change in general ?
this assumes of course that the SPI API as used in Linux works for you ?
seems this stalled because you guys instead took a queue+flush approach in your spi bus. but ive come across another hardware bus which (sounds like it) has the same limitations -- the ICH SPI controller from intel. you have to program into its hardware registers the command, the optional address, and whether you're going to be reading/writing afterwards. they needed to drop the SPI_XFER_{START,END} flags just like you in order to make things work.
further, while talking to people, it isnt as big of a deal as i had originally thought. the overhead from polling the SPI flash does go up slightly, but really only like by one byte on the bus (and the overhead to setup/bring down the transfer, but that's fairly minuscule).
so what i'm thinking is that since no other flags have materialized, we punt the SPI_XFER_{BEGIN,END} and make the API more Linux like. when i looked through the source, i couldnt find any other consumers that'd be adversely affected.
any other thoughts on the matter ? -mike
On Tuesday, February 01, 2011 11:00:39 Kumar Gala wrote:
Do we have one?
someone else asked me that and the answer i gave was that arch-drivers should go through arch trees, but common code changes i can help run through my tree. but in this case i guess you're not asking about how to get a patch merged ... -mike
participants (5)
-
Kumar Gala -
Mike Frysinger -
Reinhard Meyer -
Stefano Babic -
Wolfgang Denk