Re: MMC sub-system: SDIO block-mode with increment address issue
Hi Pierre, Thanks for explaining. I guess then I have a crappy SDIO card that needs the "voodoo" bit set for correct operation. There are other registers close to the START ADDRESS so it is a FIFO, but the Incrementing Address flag is needed to read from and write to the FIFO correctly. Regards, Dean. On Wed, 2007-11-21 at 14:08 +0100, Pierre Ossman wrote: > On Wed, 21 Nov 2007 11:57:01 + > Dean Jenkins <[EMAIL PROTECTED]> wrote: > > > Hi Pierre, > > > > I've looked at the SD Card Association's SDIO Part E1 V2.00 > > specification concerning the Incrementing Address OP Code field for > > CMD53. > > > > The specification indicates that the START ADDRESS is inserted into the > > Register Address register field. When the OP Code field has a value of 1 > > then the during the transfer the IO address is internally incremented > > for each byte transferred. This applies to a single CMD53 command. > > > > Looking at the implementation of sdio_io_rw_ext_helper() in sdio_io.c. > > This function can send multiple CMD53 commands. My concern is that with > > incrementing address mode selected the START ADDRESS is erroneously > > changed for subsequent CMD53 commands in the while loop. > > > > Since the caller is not supposed to care about the internal operation of > sdio_io_rw_ext_helper(), the address increase is a must to maintain a > consistent behaviour regardless of what transactions it decides to use. > > I.e. if I write 2048 bytes with start address 0x1000, I expect it to do a > single write to register 0x1000 through 0x17FF, not and unknown number of > writes to some unknown interval. > > Now what I suspect you're championing is to support some broken card that > treats the address increase bit in CMD53 as "Magic voodoo bit #5" instead of > the definition in the SDIO spec. I.e. the only address it cares about is the > start address and hence needs it to be the same for each transaction. This > kind of blatant disregard for the SDIO register design is of course not what > sdio_io_rw_ext_helper() was designed for, and probably never will be. > > > What I am trying to say is I don't believe the START ADDRESS should be > > changed by the core driver when incrementing address mode is used. I > > think incrementing address mode only applies internally to a single > > CMD53 command. The SDIO card must physically have a suitable register > > present at the START ADDRESS so changing this address to something > > dependent on the data size is going to fail I think. > > > > The SDIO card must physically have a suitable register present at the entire > relevant range, not just the start address. If it doesn't then it isn't > following the register interface design of SDIO (having the "increase > address" bit would just be silly if the arguments were arbitrary tokens and > not part of a consistent address space). > > > Do you have any evidence that any card drivers will use > > sdio_io_rw_ext_helper() in a manner that needs the START ADDRESS to be > > changed by the core driver ? > > > > There are no drivers using "increase address" yet. The ones so far have all > used a single byte FIFO port. > > Rgds -- Dean Jenkins Embedded Software Engineer MontaVista Software (UK) Professional Services Division - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: MMC sub-system: SDIO block-mode with increment address issue
On Wed, 21 Nov 2007 11:57:01 + Dean Jenkins <[EMAIL PROTECTED]> wrote: > Hi Pierre, > > I've looked at the SD Card Association's SDIO Part E1 V2.00 > specification concerning the Incrementing Address OP Code field for > CMD53. > > The specification indicates that the START ADDRESS is inserted into the > Register Address register field. When the OP Code field has a value of 1 > then the during the transfer the IO address is internally incremented > for each byte transferred. This applies to a single CMD53 command. > > Looking at the implementation of sdio_io_rw_ext_helper() in sdio_io.c. > This function can send multiple CMD53 commands. My concern is that with > incrementing address mode selected the START ADDRESS is erroneously > changed for subsequent CMD53 commands in the while loop. > Since the caller is not supposed to care about the internal operation of sdio_io_rw_ext_helper(), the address increase is a must to maintain a consistent behaviour regardless of what transactions it decides to use. I.e. if I write 2048 bytes with start address 0x1000, I expect it to do a single write to register 0x1000 through 0x17FF, not and unknown number of writes to some unknown interval. Now what I suspect you're championing is to support some broken card that treats the address increase bit in CMD53 as "Magic voodoo bit #5" instead of the definition in the SDIO spec. I.e. the only address it cares about is the start address and hence needs it to be the same for each transaction. This kind of blatant disregard for the SDIO register design is of course not what sdio_io_rw_ext_helper() was designed for, and probably never will be. > What I am trying to say is I don't believe the START ADDRESS should be > changed by the core driver when incrementing address mode is used. I > think incrementing address mode only applies internally to a single > CMD53 command. The SDIO card must physically have a suitable register > present at the START ADDRESS so changing this address to something > dependent on the data size is going to fail I think. > The SDIO card must physically have a suitable register present at the entire relevant range, not just the start address. If it doesn't then it isn't following the register interface design of SDIO (having the "increase address" bit would just be silly if the arguments were arbitrary tokens and not part of a consistent address space). > Do you have any evidence that any card drivers will use > sdio_io_rw_ext_helper() in a manner that needs the START ADDRESS to be > changed by the core driver ? > There are no drivers using "increase address" yet. The ones so far have all used a single byte FIFO port. Rgds -- -- Pierre Ossman Linux kernel, MMC maintainerhttp://www.kernel.org PulseAudio, core developer http://pulseaudio.org rdesktop, core developer http://www.rdesktop.org - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: MMC sub-system: SDIO block-mode with increment address issue
Hi Pierre, I've looked at the SD Card Association's SDIO Part E1 V2.00 specification concerning the Incrementing Address OP Code field for CMD53. The specification indicates that the START ADDRESS is inserted into the Register Address register field. When the OP Code field has a value of 1 then the during the transfer the IO address is internally incremented for each byte transferred. This applies to a single CMD53 command. Looking at the implementation of sdio_io_rw_ext_helper() in sdio_io.c. This function can send multiple CMD53 commands. My concern is that with incrementing address mode selected the START ADDRESS is erroneously changed for subsequent CMD53 commands in the while loop. What I am trying to say is I don't believe the START ADDRESS should be changed by the core driver when incrementing address mode is used. I think incrementing address mode only applies internally to a single CMD53 command. The SDIO card must physically have a suitable register present at the START ADDRESS so changing this address to something dependent on the data size is going to fail I think. Do you have any evidence that any card drivers will use sdio_io_rw_ext_helper() in a manner that needs the START ADDRESS to be changed by the core driver ? Regards, Dean. On Tue, 2007-11-20 at 15:10 +0100, Pierre Ossman wrote: > On Tue, 20 Nov 2007 12:26:11 + > Dean Jenkins <[EMAIL PROTECTED]> wrote: > > > Hi Pierre, > > > > IMHO the issue is there is no upper bound limit to the valid address > > range in sdio_io_rw_ext_helper() in sdio_io.c. > > > > I call sdio_memcpy_toio() as it enables the incrementing address flag in > > the CMD53 command but if I try passing too much data then the actual > > address of the subsequent CMD53 commands are erroneously incremented out > > of range. > > > > The difficulty is the SDIO card can transfer 8 blocks in a single CMD53 > > command using the incrementing address flag. However > > sdio_io_rw_ext_helper() will not prevent the attempt at sending 9 blocks > > transferred as 2 CMD53 commands of 8 blocks + 1 block and the last block > > goes to the wrong address. This causes a big system crash. I suspect the > > SDIO card internally resets and the MMC sub-system can't handle the > > error condition. > > I'm afraid I still can't see the problem. If you want to transfer 9 blocks, > then the method by which you do so shouldn't matter. So 9, or 8 + 1 should > give the same end result. > > > > > This means the card driver needs to know that it cannot use > > sdio_memcpy_toio() to send any size of data but must ensure it does not > > exceed 8 blocks before calling sdio_memcpy_toio(). IMHO this makes the > > card driver undesirably tightly coupled with the core driver. OK. I'll > > workaround it using multiple calls to sdio_memcpy_toio(). > > > > Well, the problem is that the abstraction used should work just fine > according to how the SDIO standard is defined. The problem seems to be that > some card vendors decided to go their own way and started treating the SDIO > interface as something other than a simple register interface. > > As long as that is the case, there will be a lot of pain supporting these > weird cards. We can only debate where to put that pain and what compromises > to make. > > > BTW. Is the API for the exported SDIO core functions documented > > anywhere ? > > Yes, as kerneldoc tags for the relevant functions. Have a look in > drivers/core/sdio_io.c if you don't want to build the full document. > > Rgds -- Dean Jenkins Embedded Software Engineer MontaVista Software (UK) Professional Services Division - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: MMC sub-system: SDIO block-mode with increment address issue
Hi Pierre, I've looked at the SD Card Association's SDIO Part E1 V2.00 specification concerning the Incrementing Address OP Code field for CMD53. The specification indicates that the START ADDRESS is inserted into the Register Address register field. When the OP Code field has a value of 1 then the during the transfer the IO address is internally incremented for each byte transferred. This applies to a single CMD53 command. Looking at the implementation of sdio_io_rw_ext_helper() in sdio_io.c. This function can send multiple CMD53 commands. My concern is that with incrementing address mode selected the START ADDRESS is erroneously changed for subsequent CMD53 commands in the while loop. What I am trying to say is I don't believe the START ADDRESS should be changed by the core driver when incrementing address mode is used. I think incrementing address mode only applies internally to a single CMD53 command. The SDIO card must physically have a suitable register present at the START ADDRESS so changing this address to something dependent on the data size is going to fail I think. Do you have any evidence that any card drivers will use sdio_io_rw_ext_helper() in a manner that needs the START ADDRESS to be changed by the core driver ? Regards, Dean. On Tue, 2007-11-20 at 15:10 +0100, Pierre Ossman wrote: On Tue, 20 Nov 2007 12:26:11 + Dean Jenkins [EMAIL PROTECTED] wrote: Hi Pierre, IMHO the issue is there is no upper bound limit to the valid address range in sdio_io_rw_ext_helper() in sdio_io.c. I call sdio_memcpy_toio() as it enables the incrementing address flag in the CMD53 command but if I try passing too much data then the actual address of the subsequent CMD53 commands are erroneously incremented out of range. The difficulty is the SDIO card can transfer 8 blocks in a single CMD53 command using the incrementing address flag. However sdio_io_rw_ext_helper() will not prevent the attempt at sending 9 blocks transferred as 2 CMD53 commands of 8 blocks + 1 block and the last block goes to the wrong address. This causes a big system crash. I suspect the SDIO card internally resets and the MMC sub-system can't handle the error condition. I'm afraid I still can't see the problem. If you want to transfer 9 blocks, then the method by which you do so shouldn't matter. So 9, or 8 + 1 should give the same end result. This means the card driver needs to know that it cannot use sdio_memcpy_toio() to send any size of data but must ensure it does not exceed 8 blocks before calling sdio_memcpy_toio(). IMHO this makes the card driver undesirably tightly coupled with the core driver. OK. I'll workaround it using multiple calls to sdio_memcpy_toio(). Well, the problem is that the abstraction used should work just fine according to how the SDIO standard is defined. The problem seems to be that some card vendors decided to go their own way and started treating the SDIO interface as something other than a simple register interface. As long as that is the case, there will be a lot of pain supporting these weird cards. We can only debate where to put that pain and what compromises to make. BTW. Is the API for the exported SDIO core functions documented anywhere ? Yes, as kerneldoc tags for the relevant functions. Have a look in drivers/core/sdio_io.c if you don't want to build the full document. Rgds -- Dean Jenkins Embedded Software Engineer MontaVista Software (UK) Professional Services Division - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: MMC sub-system: SDIO block-mode with increment address issue
On Wed, 21 Nov 2007 11:57:01 + Dean Jenkins [EMAIL PROTECTED] wrote: Hi Pierre, I've looked at the SD Card Association's SDIO Part E1 V2.00 specification concerning the Incrementing Address OP Code field for CMD53. The specification indicates that the START ADDRESS is inserted into the Register Address register field. When the OP Code field has a value of 1 then the during the transfer the IO address is internally incremented for each byte transferred. This applies to a single CMD53 command. Looking at the implementation of sdio_io_rw_ext_helper() in sdio_io.c. This function can send multiple CMD53 commands. My concern is that with incrementing address mode selected the START ADDRESS is erroneously changed for subsequent CMD53 commands in the while loop. Since the caller is not supposed to care about the internal operation of sdio_io_rw_ext_helper(), the address increase is a must to maintain a consistent behaviour regardless of what transactions it decides to use. I.e. if I write 2048 bytes with start address 0x1000, I expect it to do a single write to register 0x1000 through 0x17FF, not and unknown number of writes to some unknown interval. Now what I suspect you're championing is to support some broken card that treats the address increase bit in CMD53 as Magic voodoo bit #5 instead of the definition in the SDIO spec. I.e. the only address it cares about is the start address and hence needs it to be the same for each transaction. This kind of blatant disregard for the SDIO register design is of course not what sdio_io_rw_ext_helper() was designed for, and probably never will be. What I am trying to say is I don't believe the START ADDRESS should be changed by the core driver when incrementing address mode is used. I think incrementing address mode only applies internally to a single CMD53 command. The SDIO card must physically have a suitable register present at the START ADDRESS so changing this address to something dependent on the data size is going to fail I think. The SDIO card must physically have a suitable register present at the entire relevant range, not just the start address. If it doesn't then it isn't following the register interface design of SDIO (having the increase address bit would just be silly if the arguments were arbitrary tokens and not part of a consistent address space). Do you have any evidence that any card drivers will use sdio_io_rw_ext_helper() in a manner that needs the START ADDRESS to be changed by the core driver ? There are no drivers using increase address yet. The ones so far have all used a single byte FIFO port. Rgds -- -- Pierre Ossman Linux kernel, MMC maintainerhttp://www.kernel.org PulseAudio, core developer http://pulseaudio.org rdesktop, core developer http://www.rdesktop.org - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: MMC sub-system: SDIO block-mode with increment address issue
Hi Pierre, Thanks for explaining. I guess then I have a crappy SDIO card that needs the voodoo bit set for correct operation. There are other registers close to the START ADDRESS so it is a FIFO, but the Incrementing Address flag is needed to read from and write to the FIFO correctly. Regards, Dean. On Wed, 2007-11-21 at 14:08 +0100, Pierre Ossman wrote: On Wed, 21 Nov 2007 11:57:01 + Dean Jenkins [EMAIL PROTECTED] wrote: Hi Pierre, I've looked at the SD Card Association's SDIO Part E1 V2.00 specification concerning the Incrementing Address OP Code field for CMD53. The specification indicates that the START ADDRESS is inserted into the Register Address register field. When the OP Code field has a value of 1 then the during the transfer the IO address is internally incremented for each byte transferred. This applies to a single CMD53 command. Looking at the implementation of sdio_io_rw_ext_helper() in sdio_io.c. This function can send multiple CMD53 commands. My concern is that with incrementing address mode selected the START ADDRESS is erroneously changed for subsequent CMD53 commands in the while loop. Since the caller is not supposed to care about the internal operation of sdio_io_rw_ext_helper(), the address increase is a must to maintain a consistent behaviour regardless of what transactions it decides to use. I.e. if I write 2048 bytes with start address 0x1000, I expect it to do a single write to register 0x1000 through 0x17FF, not and unknown number of writes to some unknown interval. Now what I suspect you're championing is to support some broken card that treats the address increase bit in CMD53 as Magic voodoo bit #5 instead of the definition in the SDIO spec. I.e. the only address it cares about is the start address and hence needs it to be the same for each transaction. This kind of blatant disregard for the SDIO register design is of course not what sdio_io_rw_ext_helper() was designed for, and probably never will be. What I am trying to say is I don't believe the START ADDRESS should be changed by the core driver when incrementing address mode is used. I think incrementing address mode only applies internally to a single CMD53 command. The SDIO card must physically have a suitable register present at the START ADDRESS so changing this address to something dependent on the data size is going to fail I think. The SDIO card must physically have a suitable register present at the entire relevant range, not just the start address. If it doesn't then it isn't following the register interface design of SDIO (having the increase address bit would just be silly if the arguments were arbitrary tokens and not part of a consistent address space). Do you have any evidence that any card drivers will use sdio_io_rw_ext_helper() in a manner that needs the START ADDRESS to be changed by the core driver ? There are no drivers using increase address yet. The ones so far have all used a single byte FIFO port. Rgds -- Dean Jenkins Embedded Software Engineer MontaVista Software (UK) Professional Services Division - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Fwd: MMC sub-system: SDIO block-mode with increment address issue]
On Tue, 20 Nov 2007 14:52:37 + Dean Jenkins <[EMAIL PROTECTED]> wrote: > Hi Pierre, > > My card driver needed to set the R/W E4MI bit in the Card Capability > register (0x08) in CCCR (function 0). Perhaps it is unnecessary ? > That bit is pointless given the current design of the MMC layer, so it shouldn't be necessary. > BTW. It is easy to for the card driver to access function 0 registers by > doing the following... > Of course. Any driver can do whatever it pleases since it is in kernel space. But you won't make any friends if you keep trying to bypass every system in the kernel. ;) (Not to mention you'll have a lot more work on your hands.) Rgds -- -- Pierre Ossman Linux kernel, MMC maintainerhttp://www.kernel.org PulseAudio, core developer http://pulseaudio.org rdesktop, core developer http://www.rdesktop.org - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Fwd: MMC sub-system: SDIO block-mode with increment address issue]
Hi Pierre, My card driver needed to set the R/W E4MI bit in the Card Capability register (0x08) in CCCR (function 0). Perhaps it is unnecessary ? BTW. It is easy to for the card driver to access function 0 registers by doing the following... ... int old_num = func->num; /* note the current function number */ /* force access to function 0 area */ func->num = 0; /* enable the e4mi in the card capability register */ x = sdio_readb(func, 0x08, ); if ( ret == 0 ) { x |= 0x20; sdio_writeb(func, x, 0x08, ); } /* restore to original function number */ func->num = old_num; ... Regards, Dean. On Tue, 2007-11-20 at 11:51 +0100, Pierre Ossman wrote: > On Tue, 20 Nov 2007 10:28:53 + > Dean Jenkins <[EMAIL PROTECTED]> wrote: > > > Hi Pierre, > > > > I've managed to find your E-mail address in the Linux kernel mailing > > list. I hope it is OK to directly E-mail you ? > > > > You should also find my address in the MAINTAINERS file, which is where you > should look for contact info. But yes, you can contact me directly. I prefer > that you also add a relevant mailing list though. > > > I work for MontaVista Software in the UK. I guess you know that > > MontaVista are using the MMC/SDIO sub-system in their latest Mobilinux > > 5.0 product ? It is using a 2.6.21 kernel. > > > > Yesterday, I sent an E-mail to you (attached) to the Linux kernel > > mailing list. Do you have any comments to make ? > > > > Didn't notice it. I'll have a look. > > > Do you have any bug reporting facility for the MMC/SDIO sub-system ? I > > would like to report some SDIO API limitations for the SDIO card drivers > > that I needed to workaround. For example, my SDIO driver needs to modify > > the contents of the Card Capability register in function 0 to enable > > block-mode support but the card driver has been restricted to only be > > able to use function 1. There are other registers as well that the card > > driver needs access to. > > > > That is by design (as you probably can tell). Letting function drivers touch > card global stuff under the feet of the SDIO core is a big layering violation > and bound to screw things up eventually. > > Could you explain more in detail what it is you need to fiddle with in > function 0 and why? > > Rgds -- Dean Jenkins Embedded Software Engineer MontaVista Software (UK) Professional Services Division - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: MMC sub-system: SDIO block-mode with increment address issue
On Tue, 20 Nov 2007 12:26:11 + Dean Jenkins <[EMAIL PROTECTED]> wrote: > Hi Pierre, > > IMHO the issue is there is no upper bound limit to the valid address > range in sdio_io_rw_ext_helper() in sdio_io.c. > > I call sdio_memcpy_toio() as it enables the incrementing address flag in > the CMD53 command but if I try passing too much data then the actual > address of the subsequent CMD53 commands are erroneously incremented out > of range. > > The difficulty is the SDIO card can transfer 8 blocks in a single CMD53 > command using the incrementing address flag. However > sdio_io_rw_ext_helper() will not prevent the attempt at sending 9 blocks > transferred as 2 CMD53 commands of 8 blocks + 1 block and the last block > goes to the wrong address. This causes a big system crash. I suspect the > SDIO card internally resets and the MMC sub-system can't handle the > error condition. I'm afraid I still can't see the problem. If you want to transfer 9 blocks, then the method by which you do so shouldn't matter. So 9, or 8 + 1 should give the same end result. > > This means the card driver needs to know that it cannot use > sdio_memcpy_toio() to send any size of data but must ensure it does not > exceed 8 blocks before calling sdio_memcpy_toio(). IMHO this makes the > card driver undesirably tightly coupled with the core driver. OK. I'll > workaround it using multiple calls to sdio_memcpy_toio(). > Well, the problem is that the abstraction used should work just fine according to how the SDIO standard is defined. The problem seems to be that some card vendors decided to go their own way and started treating the SDIO interface as something other than a simple register interface. As long as that is the case, there will be a lot of pain supporting these weird cards. We can only debate where to put that pain and what compromises to make. > BTW. Is the API for the exported SDIO core functions documented > anywhere ? Yes, as kerneldoc tags for the relevant functions. Have a look in drivers/core/sdio_io.c if you don't want to build the full document. Rgds -- -- Pierre Ossman Linux kernel, MMC maintainerhttp://www.kernel.org PulseAudio, core developer http://pulseaudio.org rdesktop, core developer http://www.rdesktop.org - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: MMC sub-system: SDIO block-mode with increment address issue
Hi Pierre, IMHO the issue is there is no upper bound limit to the valid address range in sdio_io_rw_ext_helper() in sdio_io.c. I call sdio_memcpy_toio() as it enables the incrementing address flag in the CMD53 command but if I try passing too much data then the actual address of the subsequent CMD53 commands are erroneously incremented out of range. The difficulty is the SDIO card can transfer 8 blocks in a single CMD53 command using the incrementing address flag. However sdio_io_rw_ext_helper() will not prevent the attempt at sending 9 blocks transferred as 2 CMD53 commands of 8 blocks + 1 block and the last block goes to the wrong address. This causes a big system crash. I suspect the SDIO card internally resets and the MMC sub-system can't handle the error condition. This means the card driver needs to know that it cannot use sdio_memcpy_toio() to send any size of data but must ensure it does not exceed 8 blocks before calling sdio_memcpy_toio(). IMHO this makes the card driver undesirably tightly coupled with the core driver. OK. I'll workaround it using multiple calls to sdio_memcpy_toio(). BTW. Is the API for the exported SDIO core functions documented anywhere ? Thanks, Regards, Dean. On Tue, 2007-11-20 at 11:58 +0100, Pierre Ossman wrote: > On Mon, 19 Nov 2007 11:44:54 + > Dean Jenkins <[EMAIL PROTECTED]> wrote: > > > This E-mail is for the attention of Pierre Ossman (MMC sub-system > > maintainer) > > A cc helps if you want my attention. ;) > > > > > Hi Pierre, > > > > I've being trying to get SDIO block-mode with incrementing address to > > work on an OMAP2430 based reference board with an SDIO card. > > > > Looking at the latest code ( as of 19/11/2007 ) on the mmc-git tree (I'm > > not a git expert so I'm not sure how to reference the codebase). I have > > a comment to make concerning the following code snippet... > > git log or git show will give you your current top commit id. > > > > I think the lines > > > > 227 if (incr_addr) > > 228 addr += size; > > > > are incorrect and should be removed. I think the SDIO increment address > > parameter relates to the internal operation of the SDIO card and NOT to > > the external register address of the FIFO. In other words, I think with > > incrementing address enabled in block mode, the register address of the > > FIFO in the SDIO function register space will be erroneously changed on > > the next block write and will fail (it seems to fail on my card). It > > seems strange to change the register address ? > > > > I don't follow. The SDIO specification very clearly defines the behaviour of > incrementing address. The referenced code is very much needed to keep that > behaviour when we need to split up the transfer. > > I assume what you want is multiple transactions with incrementing address, > but each transaction restarting at the same base address. In that case you'll > have to call the sdio register functions multiple times. > > Rgds -- Dean Jenkins Embedded Software Engineer MontaVista Software (UK) Professional Services Division - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: MMC sub-system: SDIO block-mode with increment address issue
On Mon, 19 Nov 2007 11:44:54 + Dean Jenkins <[EMAIL PROTECTED]> wrote: > This E-mail is for the attention of Pierre Ossman (MMC sub-system > maintainer) A cc helps if you want my attention. ;) > > Hi Pierre, > > I've being trying to get SDIO block-mode with incrementing address to > work on an OMAP2430 based reference board with an SDIO card. > > Looking at the latest code ( as of 19/11/2007 ) on the mmc-git tree (I'm > not a git expert so I'm not sure how to reference the codebase). I have > a comment to make concerning the following code snippet... git log or git show will give you your current top commit id. > > I think the lines > > 227 if (incr_addr) > 228 addr += size; > > are incorrect and should be removed. I think the SDIO increment address > parameter relates to the internal operation of the SDIO card and NOT to > the external register address of the FIFO. In other words, I think with > incrementing address enabled in block mode, the register address of the > FIFO in the SDIO function register space will be erroneously changed on > the next block write and will fail (it seems to fail on my card). It > seems strange to change the register address ? > I don't follow. The SDIO specification very clearly defines the behaviour of incrementing address. The referenced code is very much needed to keep that behaviour when we need to split up the transfer. I assume what you want is multiple transactions with incrementing address, but each transaction restarting at the same base address. In that case you'll have to call the sdio register functions multiple times. Rgds -- -- Pierre Ossman Linux kernel, MMC maintainerhttp://www.kernel.org PulseAudio, core developer http://pulseaudio.org rdesktop, core developer http://www.rdesktop.org - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: MMC sub-system: SDIO block-mode with increment address issue
On Mon, 19 Nov 2007 11:44:54 + Dean Jenkins [EMAIL PROTECTED] wrote: This E-mail is for the attention of Pierre Ossman (MMC sub-system maintainer) A cc helps if you want my attention. ;) Hi Pierre, I've being trying to get SDIO block-mode with incrementing address to work on an OMAP2430 based reference board with an SDIO card. Looking at the latest code ( as of 19/11/2007 ) on the mmc-git tree (I'm not a git expert so I'm not sure how to reference the codebase). I have a comment to make concerning the following code snippet... git log or git show will give you your current top commit id. I think the lines 227 if (incr_addr) 228 addr += size; are incorrect and should be removed. I think the SDIO increment address parameter relates to the internal operation of the SDIO card and NOT to the external register address of the FIFO. In other words, I think with incrementing address enabled in block mode, the register address of the FIFO in the SDIO function register space will be erroneously changed on the next block write and will fail (it seems to fail on my card). It seems strange to change the register address ? I don't follow. The SDIO specification very clearly defines the behaviour of incrementing address. The referenced code is very much needed to keep that behaviour when we need to split up the transfer. I assume what you want is multiple transactions with incrementing address, but each transaction restarting at the same base address. In that case you'll have to call the sdio register functions multiple times. Rgds -- -- Pierre Ossman Linux kernel, MMC maintainerhttp://www.kernel.org PulseAudio, core developer http://pulseaudio.org rdesktop, core developer http://www.rdesktop.org - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: MMC sub-system: SDIO block-mode with increment address issue
Hi Pierre, IMHO the issue is there is no upper bound limit to the valid address range in sdio_io_rw_ext_helper() in sdio_io.c. I call sdio_memcpy_toio() as it enables the incrementing address flag in the CMD53 command but if I try passing too much data then the actual address of the subsequent CMD53 commands are erroneously incremented out of range. The difficulty is the SDIO card can transfer 8 blocks in a single CMD53 command using the incrementing address flag. However sdio_io_rw_ext_helper() will not prevent the attempt at sending 9 blocks transferred as 2 CMD53 commands of 8 blocks + 1 block and the last block goes to the wrong address. This causes a big system crash. I suspect the SDIO card internally resets and the MMC sub-system can't handle the error condition. This means the card driver needs to know that it cannot use sdio_memcpy_toio() to send any size of data but must ensure it does not exceed 8 blocks before calling sdio_memcpy_toio(). IMHO this makes the card driver undesirably tightly coupled with the core driver. OK. I'll workaround it using multiple calls to sdio_memcpy_toio(). BTW. Is the API for the exported SDIO core functions documented anywhere ? Thanks, Regards, Dean. On Tue, 2007-11-20 at 11:58 +0100, Pierre Ossman wrote: On Mon, 19 Nov 2007 11:44:54 + Dean Jenkins [EMAIL PROTECTED] wrote: This E-mail is for the attention of Pierre Ossman (MMC sub-system maintainer) A cc helps if you want my attention. ;) Hi Pierre, I've being trying to get SDIO block-mode with incrementing address to work on an OMAP2430 based reference board with an SDIO card. Looking at the latest code ( as of 19/11/2007 ) on the mmc-git tree (I'm not a git expert so I'm not sure how to reference the codebase). I have a comment to make concerning the following code snippet... git log or git show will give you your current top commit id. I think the lines 227 if (incr_addr) 228 addr += size; are incorrect and should be removed. I think the SDIO increment address parameter relates to the internal operation of the SDIO card and NOT to the external register address of the FIFO. In other words, I think with incrementing address enabled in block mode, the register address of the FIFO in the SDIO function register space will be erroneously changed on the next block write and will fail (it seems to fail on my card). It seems strange to change the register address ? I don't follow. The SDIO specification very clearly defines the behaviour of incrementing address. The referenced code is very much needed to keep that behaviour when we need to split up the transfer. I assume what you want is multiple transactions with incrementing address, but each transaction restarting at the same base address. In that case you'll have to call the sdio register functions multiple times. Rgds -- Dean Jenkins Embedded Software Engineer MontaVista Software (UK) Professional Services Division - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: MMC sub-system: SDIO block-mode with increment address issue
On Tue, 20 Nov 2007 12:26:11 + Dean Jenkins [EMAIL PROTECTED] wrote: Hi Pierre, IMHO the issue is there is no upper bound limit to the valid address range in sdio_io_rw_ext_helper() in sdio_io.c. I call sdio_memcpy_toio() as it enables the incrementing address flag in the CMD53 command but if I try passing too much data then the actual address of the subsequent CMD53 commands are erroneously incremented out of range. The difficulty is the SDIO card can transfer 8 blocks in a single CMD53 command using the incrementing address flag. However sdio_io_rw_ext_helper() will not prevent the attempt at sending 9 blocks transferred as 2 CMD53 commands of 8 blocks + 1 block and the last block goes to the wrong address. This causes a big system crash. I suspect the SDIO card internally resets and the MMC sub-system can't handle the error condition. I'm afraid I still can't see the problem. If you want to transfer 9 blocks, then the method by which you do so shouldn't matter. So 9, or 8 + 1 should give the same end result. This means the card driver needs to know that it cannot use sdio_memcpy_toio() to send any size of data but must ensure it does not exceed 8 blocks before calling sdio_memcpy_toio(). IMHO this makes the card driver undesirably tightly coupled with the core driver. OK. I'll workaround it using multiple calls to sdio_memcpy_toio(). Well, the problem is that the abstraction used should work just fine according to how the SDIO standard is defined. The problem seems to be that some card vendors decided to go their own way and started treating the SDIO interface as something other than a simple register interface. As long as that is the case, there will be a lot of pain supporting these weird cards. We can only debate where to put that pain and what compromises to make. BTW. Is the API for the exported SDIO core functions documented anywhere ? Yes, as kerneldoc tags for the relevant functions. Have a look in drivers/core/sdio_io.c if you don't want to build the full document. Rgds -- -- Pierre Ossman Linux kernel, MMC maintainerhttp://www.kernel.org PulseAudio, core developer http://pulseaudio.org rdesktop, core developer http://www.rdesktop.org - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Fwd: MMC sub-system: SDIO block-mode with increment address issue]
Hi Pierre, My card driver needed to set the R/W E4MI bit in the Card Capability register (0x08) in CCCR (function 0). Perhaps it is unnecessary ? BTW. It is easy to for the card driver to access function 0 registers by doing the following... ... int old_num = func-num; /* note the current function number */ /* force access to function 0 area */ func-num = 0; /* enable the e4mi in the card capability register */ x = sdio_readb(func, 0x08, ret); if ( ret == 0 ) { x |= 0x20; sdio_writeb(func, x, 0x08, ret); } /* restore to original function number */ func-num = old_num; ... Regards, Dean. On Tue, 2007-11-20 at 11:51 +0100, Pierre Ossman wrote: On Tue, 20 Nov 2007 10:28:53 + Dean Jenkins [EMAIL PROTECTED] wrote: Hi Pierre, I've managed to find your E-mail address in the Linux kernel mailing list. I hope it is OK to directly E-mail you ? You should also find my address in the MAINTAINERS file, which is where you should look for contact info. But yes, you can contact me directly. I prefer that you also add a relevant mailing list though. I work for MontaVista Software in the UK. I guess you know that MontaVista are using the MMC/SDIO sub-system in their latest Mobilinux 5.0 product ? It is using a 2.6.21 kernel. Yesterday, I sent an E-mail to you (attached) to the Linux kernel mailing list. Do you have any comments to make ? Didn't notice it. I'll have a look. Do you have any bug reporting facility for the MMC/SDIO sub-system ? I would like to report some SDIO API limitations for the SDIO card drivers that I needed to workaround. For example, my SDIO driver needs to modify the contents of the Card Capability register in function 0 to enable block-mode support but the card driver has been restricted to only be able to use function 1. There are other registers as well that the card driver needs access to. That is by design (as you probably can tell). Letting function drivers touch card global stuff under the feet of the SDIO core is a big layering violation and bound to screw things up eventually. Could you explain more in detail what it is you need to fiddle with in function 0 and why? Rgds -- Dean Jenkins Embedded Software Engineer MontaVista Software (UK) Professional Services Division - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Fwd: MMC sub-system: SDIO block-mode with increment address issue]
On Tue, 20 Nov 2007 14:52:37 + Dean Jenkins [EMAIL PROTECTED] wrote: Hi Pierre, My card driver needed to set the R/W E4MI bit in the Card Capability register (0x08) in CCCR (function 0). Perhaps it is unnecessary ? That bit is pointless given the current design of the MMC layer, so it shouldn't be necessary. BTW. It is easy to for the card driver to access function 0 registers by doing the following... Of course. Any driver can do whatever it pleases since it is in kernel space. But you won't make any friends if you keep trying to bypass every system in the kernel. ;) (Not to mention you'll have a lot more work on your hands.) Rgds -- -- Pierre Ossman Linux kernel, MMC maintainerhttp://www.kernel.org PulseAudio, core developer http://pulseaudio.org rdesktop, core developer http://www.rdesktop.org - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
MMC sub-system: SDIO block-mode with increment address issue
This E-mail is for the attention of Pierre Ossman (MMC sub-system maintainer) Hi Pierre, I've being trying to get SDIO block-mode with incrementing address to work on an OMAP2430 based reference board with an SDIO card. Looking at the latest code ( as of 19/11/2007 ) on the mmc-git tree (I'm not a git expert so I'm not sure how to reference the codebase). I have a comment to make concerning the following code snippet... In drivers/mmc/core/sdio_io.c Function: sdio_io_rw_ext_helper() 219 ret = mmc_io_rw_extended(func->card, write, 220 func->num, addr, incr_addr, buf, 221 blocks, func->cur_blksize); 222 if (ret) 223 return ret; 224 225 remainder -= size; 226 buf += size; 227 if (incr_addr) 228 addr += size; 229 } I think the lines 227 if (incr_addr) 228 addr += size; are incorrect and should be removed. I think the SDIO increment address parameter relates to the internal operation of the SDIO card and NOT to the external register address of the FIFO. In other words, I think with incrementing address enabled in block mode, the register address of the FIFO in the SDIO function register space will be erroneously changed on the next block write and will fail (it seems to fail on my card). It seems strange to change the register address ? This also relates to byte mode. What do you think ? Also, do you have a bug reporting procedure for mmc-git tree ? Regards, Dean. -- Dean Jenkins Embedded Software Engineer MontaVista Software (UK) Professional Services Division - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
MMC sub-system: SDIO block-mode with increment address issue
This E-mail is for the attention of Pierre Ossman (MMC sub-system maintainer) Hi Pierre, I've being trying to get SDIO block-mode with incrementing address to work on an OMAP2430 based reference board with an SDIO card. Looking at the latest code ( as of 19/11/2007 ) on the mmc-git tree (I'm not a git expert so I'm not sure how to reference the codebase). I have a comment to make concerning the following code snippet... In drivers/mmc/core/sdio_io.c Function: sdio_io_rw_ext_helper() snip 219 ret = mmc_io_rw_extended(func-card, write, 220 func-num, addr, incr_addr, buf, 221 blocks, func-cur_blksize); 222 if (ret) 223 return ret; 224 225 remainder -= size; 226 buf += size; 227 if (incr_addr) 228 addr += size; 229 } /snip I think the lines 227 if (incr_addr) 228 addr += size; are incorrect and should be removed. I think the SDIO increment address parameter relates to the internal operation of the SDIO card and NOT to the external register address of the FIFO. In other words, I think with incrementing address enabled in block mode, the register address of the FIFO in the SDIO function register space will be erroneously changed on the next block write and will fail (it seems to fail on my card). It seems strange to change the register address ? This also relates to byte mode. What do you think ? Also, do you have a bug reporting procedure for mmc-git tree ? Regards, Dean. -- Dean Jenkins Embedded Software Engineer MontaVista Software (UK) Professional Services Division - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/