Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Thursday, July 30, 2015 at 02:18:09 PM, Michal Suchanek wrote: > On 30 July 2015 at 13:24, Marek Vasut wrote: > > On Monday, July 27, 2015 at 10:43:05 PM, Michal Suchanek wrote: > >> On 27 July 2015 at 19:43, Marek Vasut wrote: > >> > On Monday, July 27, 2015 at 11:46:25 AM, Michal Suchanek wrote: > >> >> On 24 July 2015 at 10:34, Marek Vasut wrote: > >> >> > On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: > >> >> Ok, so here is some summary. > >> >> > >> >> I have a NOR flash attached to a s3c64xx SPI controller with 64byte > >> >> fifo and a pl330 dma controller. > >> >> > >> >> Normally DMA controller is used for transfers that do not fit into > >> >> the FIFO. > >> >> > >> >> I tried adding the flash memory ID to the spi-nor driver table and > >> >> adding a DT node for it. > >> >> > >> >> The flash is rated at 120MHz so I used that speed but the ID was > >> >> bit-shifted and identification failed. There is DT property > >> >> samsung,spi-feedback-delay for addressing this and at 120MHz it must > >> >> be 2 or 3 on this board. 40MHz works with default value 0. > >> >> > >> >> The next step after identification worked was to try reading the > >> >> flash content. For this the DMA controller is used because data is > >> >> transferred in blocks larger than 64 bytes. When reading the whole > >> >> 4MB flash the transfer failed silently. I got a 4MB file of all ones > >> >> or all zeroes. > >> >> > >> >> It turns out that > >> >> > >> >> - the pl330 locks up when transfering large amount of data. > >> >> > >> >> Specifically, the maximum power of 2 sized transfer at 120MHz is 128 > >> >> bytes and 64k at 40MHz. Transferring more than this results in the > >> >> pl330 locking up and never signalling completion of the transfer. > >> > > >> > Hypothesis: > >> > I think this might just be that the controller didn't catch all the > >> > inbound clock ticks and thus counted less inbound data than it was > >> > set up to receive. The controller thus waits for more data. > >> > >> The flash chip can transfer data as long as you keep the clock going, > >> especially when you transfer 64k off a 4M flash. > >> > >> The read command is bound just by stopping the clock. There is always > >> more data to be had. > > > > Sure, if you keep clocking the chip, the data will keep going. Is the > > chip being clocked ? > > There is always data. Like in whenever you want to read SPI data you > sample the input pin and watever you sample is data so far as SPI bus > is concerned. Aren't you forgetting that the data are synchronous to the clock which you (your SPI block) generate ? > > Doesn't the PL330 have some kind of a counter register where you can > > check how much data were received so far ? That should be sufficient to > > verify this hypothesis of mine. > > Could check that, yes. Maybe I could read the counter in a function > which dmaengine client uses to tear down the dma transfer. Might be a good idea. Check if the register is stuck for too long and then zap the transfer. > On a related note I enabled more prints on the spidev test program. > > I have code that tests maximum transfer size up to the 4k spidev limit > and it can transfer full 4k buffer of NOPs at 80MHz. > > The recieved data is all ones except a 00 at the start. So it looks > like read-only transfers fail but full-duplex sort of work. And it > reproduces the 00 prefix that was seen in the dma-only setup in > otherwise working setup :-S > > An attempt to read a page of data using the fast-read command fails > with timeout. It would be a good idea to detail what your program exactly does on the bus (like, "my program sends 3 bytes of data 0xf0 0x0b 0xar and then receives 60 bytes of data"). Maybe I'm lost, but your test is really not clear. > >> >> Data > >> >> is left in FIFO which causes subsequent commands to fail since > >> >> garbage is returned instead of command reply. Also subsequent read > >> >> may cause I/O error or or return an empty page depending on the > >> >> garbage around. > >> > > >> > So the driver for the DMA controller might need to be augmented to > >> > handle this case -- add a timeout and in case DMA times out, drain > >> > the FIFO to make sure subsequent transfers do not fail. > >> > >> There is no way to add timeout in the DMA driver since it does not > >> know the SPI clock. > >> > >> The timeout is handled by the SPI driver and it should be possible to > >> augment it to drain the FIFO when DMA fails so long as FIFO level is > >> readable somewhere. > > > > If the DMA doesn't complete in certain amount of time, it should time out > > I'd say. Don't you think ? > > No. The DMA driver has no idea if the transfer is at 133MHz, 40MHz, > 1MHz, 1kHz, whatever. That's not really important, is it ? If the transfer doesn't finish in, say, 1 second, and it is a 4096b transfer, then it is most likely timeout. > On the other hand, adding a flush_fifo in the SPI driver after DMA
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 30 July 2015 at 13:24, Marek Vasut wrote: > On Monday, July 27, 2015 at 10:43:05 PM, Michal Suchanek wrote: >> On 27 July 2015 at 19:43, Marek Vasut wrote: >> > On Monday, July 27, 2015 at 11:46:25 AM, Michal Suchanek wrote: >> >> On 24 July 2015 at 10:34, Marek Vasut wrote: >> >> > On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: >> >> Ok, so here is some summary. >> >> >> >> I have a NOR flash attached to a s3c64xx SPI controller with 64byte >> >> fifo and a pl330 dma controller. >> >> >> >> Normally DMA controller is used for transfers that do not fit into the >> >> FIFO. >> >> >> >> I tried adding the flash memory ID to the spi-nor driver table and >> >> adding a DT node for it. >> >> >> >> The flash is rated at 120MHz so I used that speed but the ID was >> >> bit-shifted and identification failed. There is DT property >> >> samsung,spi-feedback-delay for addressing this and at 120MHz it must >> >> be 2 or 3 on this board. 40MHz works with default value 0. >> >> >> >> The next step after identification worked was to try reading the flash >> >> content. For this the DMA controller is used because data is >> >> transferred in blocks larger than 64 bytes. When reading the whole 4MB >> >> flash the transfer failed silently. I got a 4MB file of all ones or >> >> all zeroes. >> >> >> >> It turns out that >> >> >> >> - the pl330 locks up when transfering large amount of data. >> >> >> >> Specifically, the maximum power of 2 sized transfer at 120MHz is 128 >> >> bytes and 64k at 40MHz. Transferring more than this results in the >> >> pl330 locking up and never signalling completion of the transfer. >> > >> > Hypothesis: >> > I think this might just be that the controller didn't catch all the >> > inbound clock ticks and thus counted less inbound data than it was >> > set up to receive. The controller thus waits for more data. >> >> The flash chip can transfer data as long as you keep the clock going, >> especially when you transfer 64k off a 4M flash. >> >> The read command is bound just by stopping the clock. There is always >> more data to be had. > > Sure, if you keep clocking the chip, the data will keep going. Is the > chip being clocked ? There is always data. Like in whenever you want to read SPI data you sample the input pin and watever you sample is data so far as SPI bus is concerned. > > Doesn't the PL330 have some kind of a counter register where you can check > how much data were received so far ? That should be sufficient to verify > this hypothesis of mine. Could check that, yes. Maybe I could read the counter in a function which dmaengine client uses to tear down the dma transfer. On a related note I enabled more prints on the spidev test program. I have code that tests maximum transfer size up to the 4k spidev limit and it can transfer full 4k buffer of NOPs at 80MHz. The recieved data is all ones except a 00 at the start. So it looks like read-only transfers fail but full-duplex sort of work. And it reproduces the 00 prefix that was seen in the dma-only setup in otherwise working setup :-S An attempt to read a page of data using the fast-read command fails with timeout. > >> >> Data >> >> is left in FIFO which causes subsequent commands to fail since garbage >> >> is returned instead of command reply. Also subsequent read may cause >> >> I/O error or or return an empty page depending on the garbage around. >> > >> > So the driver for the DMA controller might need to be augmented to handle >> > this case -- add a timeout and in case DMA times out, drain the FIFO to >> > make sure subsequent transfers do not fail. >> >> There is no way to add timeout in the DMA driver since it does not >> know the SPI clock. >> >> The timeout is handled by the SPI driver and it should be possible to >> augment it to drain the FIFO when DMA fails so long as FIFO level is >> readable somewhere. > > If the DMA doesn't complete in certain amount of time, it should time out > I'd say. Don't you think ? No. The DMA driver has no idea if the transfer is at 133MHz, 40MHz, 1MHz, 1kHz, whatever. On the other hand, adding a flush_fifo in the SPI driver after DMA failure allows probing the chip reliably by realoding the driver even just after a failed transfer. > >> >> - the I/O errors are not checked in spi-nor at all which leads to >> >> silent data corruption. >> >> >> >> On a suggestion that this may improve reliability I changed the >> >> s3c64xx driver to use DMA for all transfers. This caused >> >> identification to fail in spin-nor because the ID was prefixed with >> >> extra 00 byte. Testing with spidev confirmed that everything is >> >> prefixed with extra 00. >> > >> > The determinism of this is in fact excellent news. >> > >> >> Also when the DMA controller locked up no >> >> transfers were possible anymore. When DMA was not used for sending >> >> commands the controller would recover on next command. I made the >> >> option to always use DMA configurable and it turns out
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Monday, July 27, 2015 at 10:43:05 PM, Michal Suchanek wrote: > On 27 July 2015 at 19:43, Marek Vasut wrote: > > On Monday, July 27, 2015 at 11:46:25 AM, Michal Suchanek wrote: > >> On 24 July 2015 at 10:34, Marek Vasut wrote: > >> > On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: > >> Ok, so here is some summary. > >> > >> I have a NOR flash attached to a s3c64xx SPI controller with 64byte > >> fifo and a pl330 dma controller. > >> > >> Normally DMA controller is used for transfers that do not fit into the > >> FIFO. > >> > >> I tried adding the flash memory ID to the spi-nor driver table and > >> adding a DT node for it. > >> > >> The flash is rated at 120MHz so I used that speed but the ID was > >> bit-shifted and identification failed. There is DT property > >> samsung,spi-feedback-delay for addressing this and at 120MHz it must > >> be 2 or 3 on this board. 40MHz works with default value 0. > >> > >> The next step after identification worked was to try reading the flash > >> content. For this the DMA controller is used because data is > >> transferred in blocks larger than 64 bytes. When reading the whole 4MB > >> flash the transfer failed silently. I got a 4MB file of all ones or > >> all zeroes. > >> > >> It turns out that > >> > >> - the pl330 locks up when transfering large amount of data. > >> > >> Specifically, the maximum power of 2 sized transfer at 120MHz is 128 > >> bytes and 64k at 40MHz. Transferring more than this results in the > >> pl330 locking up and never signalling completion of the transfer. > > > > Hypothesis: > > I think this might just be that the controller didn't catch all the > > inbound clock ticks and thus counted less inbound data than it was > > set up to receive. The controller thus waits for more data. > > The flash chip can transfer data as long as you keep the clock going, > especially when you transfer 64k off a 4M flash. > > The read command is bound just by stopping the clock. There is always > more data to be had. Sure, if you keep clocking the chip, the data will keep going. Is the chip being clocked ? Doesn't the PL330 have some kind of a counter register where you can check how much data were received so far ? That should be sufficient to verify this hypothesis of mine. > >> Data > >> is left in FIFO which causes subsequent commands to fail since garbage > >> is returned instead of command reply. Also subsequent read may cause > >> I/O error or or return an empty page depending on the garbage around. > > > > So the driver for the DMA controller might need to be augmented to handle > > this case -- add a timeout and in case DMA times out, drain the FIFO to > > make sure subsequent transfers do not fail. > > There is no way to add timeout in the DMA driver since it does not > know the SPI clock. > > The timeout is handled by the SPI driver and it should be possible to > augment it to drain the FIFO when DMA fails so long as FIFO level is > readable somewhere. If the DMA doesn't complete in certain amount of time, it should time out I'd say. Don't you think ? > >> - the I/O errors are not checked in spi-nor at all which leads to > >> silent data corruption. > >> > >> On a suggestion that this may improve reliability I changed the > >> s3c64xx driver to use DMA for all transfers. This caused > >> identification to fail in spin-nor because the ID was prefixed with > >> extra 00 byte. Testing with spidev confirmed that everything is > >> prefixed with extra 00. > > > > The determinism of this is in fact excellent news. > > > >> Also when the DMA controller locked up no > >> transfers were possible anymore. When DMA was not used for sending > >> commands the controller would recover on next command. I made the > >> option to always use DMA configurable and it turns out that the > >> returned data is prefixed with 00 only when no transfer without DMA > >> was ever made. Loading the spi-nor driver with the dma-only option off > >> and then with dma-only option on results in correct operation. Only > >> loading the dma-only driver first causes the 00 prefix. > > > > Can we conclude that the PIO codepath somehow programs a register > > somewhere which gets rid of this 0x00 prefix ? If so, this should then > > also be part of the DMA codepath, or even better, this should be set in > > probe() somewhere. > > Yes, it looks like it. Did you find what this could be please ? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 30 July 2015 at 13:24, Marek Vasut ma...@denx.de wrote: On Monday, July 27, 2015 at 10:43:05 PM, Michal Suchanek wrote: On 27 July 2015 at 19:43, Marek Vasut ma...@denx.de wrote: On Monday, July 27, 2015 at 11:46:25 AM, Michal Suchanek wrote: On 24 July 2015 at 10:34, Marek Vasut ma...@denx.de wrote: On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: Ok, so here is some summary. I have a NOR flash attached to a s3c64xx SPI controller with 64byte fifo and a pl330 dma controller. Normally DMA controller is used for transfers that do not fit into the FIFO. I tried adding the flash memory ID to the spi-nor driver table and adding a DT node for it. The flash is rated at 120MHz so I used that speed but the ID was bit-shifted and identification failed. There is DT property samsung,spi-feedback-delay for addressing this and at 120MHz it must be 2 or 3 on this board. 40MHz works with default value 0. The next step after identification worked was to try reading the flash content. For this the DMA controller is used because data is transferred in blocks larger than 64 bytes. When reading the whole 4MB flash the transfer failed silently. I got a 4MB file of all ones or all zeroes. It turns out that - the pl330 locks up when transfering large amount of data. Specifically, the maximum power of 2 sized transfer at 120MHz is 128 bytes and 64k at 40MHz. Transferring more than this results in the pl330 locking up and never signalling completion of the transfer. Hypothesis: I think this might just be that the controller didn't catch all the inbound clock ticks and thus counted less inbound data than it was set up to receive. The controller thus waits for more data. The flash chip can transfer data as long as you keep the clock going, especially when you transfer 64k off a 4M flash. The read command is bound just by stopping the clock. There is always more data to be had. Sure, if you keep clocking the chip, the data will keep going. Is the chip being clocked ? There is always data. Like in whenever you want to read SPI data you sample the input pin and watever you sample is data so far as SPI bus is concerned. Doesn't the PL330 have some kind of a counter register where you can check how much data were received so far ? That should be sufficient to verify this hypothesis of mine. Could check that, yes. Maybe I could read the counter in a function which dmaengine client uses to tear down the dma transfer. On a related note I enabled more prints on the spidev test program. I have code that tests maximum transfer size up to the 4k spidev limit and it can transfer full 4k buffer of NOPs at 80MHz. The recieved data is all ones except a 00 at the start. So it looks like read-only transfers fail but full-duplex sort of work. And it reproduces the 00 prefix that was seen in the dma-only setup in otherwise working setup :-S An attempt to read a page of data using the fast-read command fails with timeout. Data is left in FIFO which causes subsequent commands to fail since garbage is returned instead of command reply. Also subsequent read may cause I/O error or or return an empty page depending on the garbage around. So the driver for the DMA controller might need to be augmented to handle this case -- add a timeout and in case DMA times out, drain the FIFO to make sure subsequent transfers do not fail. There is no way to add timeout in the DMA driver since it does not know the SPI clock. The timeout is handled by the SPI driver and it should be possible to augment it to drain the FIFO when DMA fails so long as FIFO level is readable somewhere. If the DMA doesn't complete in certain amount of time, it should time out I'd say. Don't you think ? No. The DMA driver has no idea if the transfer is at 133MHz, 40MHz, 1MHz, 1kHz, whatever. On the other hand, adding a flush_fifo in the SPI driver after DMA failure allows probing the chip reliably by realoding the driver even just after a failed transfer. - the I/O errors are not checked in spi-nor at all which leads to silent data corruption. On a suggestion that this may improve reliability I changed the s3c64xx driver to use DMA for all transfers. This caused identification to fail in spin-nor because the ID was prefixed with extra 00 byte. Testing with spidev confirmed that everything is prefixed with extra 00. The determinism of this is in fact excellent news. Also when the DMA controller locked up no transfers were possible anymore. When DMA was not used for sending commands the controller would recover on next command. I made the option to always use DMA configurable and it turns out that the returned data is prefixed with 00 only when no transfer without DMA was ever made. Loading the spi-nor driver with the dma-only option off and then with dma-only option on results in correct operation. Only loading
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Thursday, July 30, 2015 at 02:18:09 PM, Michal Suchanek wrote: On 30 July 2015 at 13:24, Marek Vasut ma...@denx.de wrote: On Monday, July 27, 2015 at 10:43:05 PM, Michal Suchanek wrote: On 27 July 2015 at 19:43, Marek Vasut ma...@denx.de wrote: On Monday, July 27, 2015 at 11:46:25 AM, Michal Suchanek wrote: On 24 July 2015 at 10:34, Marek Vasut ma...@denx.de wrote: On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: Ok, so here is some summary. I have a NOR flash attached to a s3c64xx SPI controller with 64byte fifo and a pl330 dma controller. Normally DMA controller is used for transfers that do not fit into the FIFO. I tried adding the flash memory ID to the spi-nor driver table and adding a DT node for it. The flash is rated at 120MHz so I used that speed but the ID was bit-shifted and identification failed. There is DT property samsung,spi-feedback-delay for addressing this and at 120MHz it must be 2 or 3 on this board. 40MHz works with default value 0. The next step after identification worked was to try reading the flash content. For this the DMA controller is used because data is transferred in blocks larger than 64 bytes. When reading the whole 4MB flash the transfer failed silently. I got a 4MB file of all ones or all zeroes. It turns out that - the pl330 locks up when transfering large amount of data. Specifically, the maximum power of 2 sized transfer at 120MHz is 128 bytes and 64k at 40MHz. Transferring more than this results in the pl330 locking up and never signalling completion of the transfer. Hypothesis: I think this might just be that the controller didn't catch all the inbound clock ticks and thus counted less inbound data than it was set up to receive. The controller thus waits for more data. The flash chip can transfer data as long as you keep the clock going, especially when you transfer 64k off a 4M flash. The read command is bound just by stopping the clock. There is always more data to be had. Sure, if you keep clocking the chip, the data will keep going. Is the chip being clocked ? There is always data. Like in whenever you want to read SPI data you sample the input pin and watever you sample is data so far as SPI bus is concerned. Aren't you forgetting that the data are synchronous to the clock which you (your SPI block) generate ? Doesn't the PL330 have some kind of a counter register where you can check how much data were received so far ? That should be sufficient to verify this hypothesis of mine. Could check that, yes. Maybe I could read the counter in a function which dmaengine client uses to tear down the dma transfer. Might be a good idea. Check if the register is stuck for too long and then zap the transfer. On a related note I enabled more prints on the spidev test program. I have code that tests maximum transfer size up to the 4k spidev limit and it can transfer full 4k buffer of NOPs at 80MHz. The recieved data is all ones except a 00 at the start. So it looks like read-only transfers fail but full-duplex sort of work. And it reproduces the 00 prefix that was seen in the dma-only setup in otherwise working setup :-S An attempt to read a page of data using the fast-read command fails with timeout. It would be a good idea to detail what your program exactly does on the bus (like, my program sends 3 bytes of data 0xf0 0x0b 0xar and then receives 60 bytes of data). Maybe I'm lost, but your test is really not clear. Data is left in FIFO which causes subsequent commands to fail since garbage is returned instead of command reply. Also subsequent read may cause I/O error or or return an empty page depending on the garbage around. So the driver for the DMA controller might need to be augmented to handle this case -- add a timeout and in case DMA times out, drain the FIFO to make sure subsequent transfers do not fail. There is no way to add timeout in the DMA driver since it does not know the SPI clock. The timeout is handled by the SPI driver and it should be possible to augment it to drain the FIFO when DMA fails so long as FIFO level is readable somewhere. If the DMA doesn't complete in certain amount of time, it should time out I'd say. Don't you think ? No. The DMA driver has no idea if the transfer is at 133MHz, 40MHz, 1MHz, 1kHz, whatever. That's not really important, is it ? If the transfer doesn't finish in, say, 1 second, and it is a 4096b transfer, then it is most likely timeout. On the other hand, adding a flush_fifo in the SPI driver after DMA failure allows probing the chip reliably by realoding the driver even just after a failed transfer. OK - the I/O errors are not checked in spi-nor at all which leads to silent data corruption. On a suggestion that this may improve reliability I changed
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Monday, July 27, 2015 at 10:43:05 PM, Michal Suchanek wrote: On 27 July 2015 at 19:43, Marek Vasut ma...@denx.de wrote: On Monday, July 27, 2015 at 11:46:25 AM, Michal Suchanek wrote: On 24 July 2015 at 10:34, Marek Vasut ma...@denx.de wrote: On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: Ok, so here is some summary. I have a NOR flash attached to a s3c64xx SPI controller with 64byte fifo and a pl330 dma controller. Normally DMA controller is used for transfers that do not fit into the FIFO. I tried adding the flash memory ID to the spi-nor driver table and adding a DT node for it. The flash is rated at 120MHz so I used that speed but the ID was bit-shifted and identification failed. There is DT property samsung,spi-feedback-delay for addressing this and at 120MHz it must be 2 or 3 on this board. 40MHz works with default value 0. The next step after identification worked was to try reading the flash content. For this the DMA controller is used because data is transferred in blocks larger than 64 bytes. When reading the whole 4MB flash the transfer failed silently. I got a 4MB file of all ones or all zeroes. It turns out that - the pl330 locks up when transfering large amount of data. Specifically, the maximum power of 2 sized transfer at 120MHz is 128 bytes and 64k at 40MHz. Transferring more than this results in the pl330 locking up and never signalling completion of the transfer. Hypothesis: I think this might just be that the controller didn't catch all the inbound clock ticks and thus counted less inbound data than it was set up to receive. The controller thus waits for more data. The flash chip can transfer data as long as you keep the clock going, especially when you transfer 64k off a 4M flash. The read command is bound just by stopping the clock. There is always more data to be had. Sure, if you keep clocking the chip, the data will keep going. Is the chip being clocked ? Doesn't the PL330 have some kind of a counter register where you can check how much data were received so far ? That should be sufficient to verify this hypothesis of mine. Data is left in FIFO which causes subsequent commands to fail since garbage is returned instead of command reply. Also subsequent read may cause I/O error or or return an empty page depending on the garbage around. So the driver for the DMA controller might need to be augmented to handle this case -- add a timeout and in case DMA times out, drain the FIFO to make sure subsequent transfers do not fail. There is no way to add timeout in the DMA driver since it does not know the SPI clock. The timeout is handled by the SPI driver and it should be possible to augment it to drain the FIFO when DMA fails so long as FIFO level is readable somewhere. If the DMA doesn't complete in certain amount of time, it should time out I'd say. Don't you think ? - the I/O errors are not checked in spi-nor at all which leads to silent data corruption. On a suggestion that this may improve reliability I changed the s3c64xx driver to use DMA for all transfers. This caused identification to fail in spin-nor because the ID was prefixed with extra 00 byte. Testing with spidev confirmed that everything is prefixed with extra 00. The determinism of this is in fact excellent news. Also when the DMA controller locked up no transfers were possible anymore. When DMA was not used for sending commands the controller would recover on next command. I made the option to always use DMA configurable and it turns out that the returned data is prefixed with 00 only when no transfer without DMA was ever made. Loading the spi-nor driver with the dma-only option off and then with dma-only option on results in correct operation. Only loading the dma-only driver first causes the 00 prefix. Can we conclude that the PIO codepath somehow programs a register somewhere which gets rid of this 0x00 prefix ? If so, this should then also be part of the DMA codepath, or even better, this should be set in probe() somewhere. Yes, it looks like it. Did you find what this could be please ? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 27 July 2015 at 19:43, Marek Vasut wrote: > On Monday, July 27, 2015 at 11:46:25 AM, Michal Suchanek wrote: >> On 24 July 2015 at 10:34, Marek Vasut wrote: >> > On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: >> > >> Ok, so here is some summary. >> >> I have a NOR flash attached to a s3c64xx SPI controller with 64byte >> fifo and a pl330 dma controller. >> >> Normally DMA controller is used for transfers that do not fit into the >> FIFO. >> >> I tried adding the flash memory ID to the spi-nor driver table and >> adding a DT node for it. >> >> The flash is rated at 120MHz so I used that speed but the ID was >> bit-shifted and identification failed. There is DT property >> samsung,spi-feedback-delay for addressing this and at 120MHz it must >> be 2 or 3 on this board. 40MHz works with default value 0. >> >> The next step after identification worked was to try reading the flash >> content. For this the DMA controller is used because data is >> transferred in blocks larger than 64 bytes. When reading the whole 4MB >> flash the transfer failed silently. I got a 4MB file of all ones or >> all zeroes. >> >> It turns out that >> >> - the pl330 locks up when transfering large amount of data. >> Specifically, the maximum power of 2 sized transfer at 120MHz is 128 >> bytes and 64k at 40MHz. Transferring more than this results in the >> pl330 locking up and never signalling completion of the transfer. > > Hypothesis: > I think this might just be that the controller didn't catch all the > inbound clock ticks and thus counted less inbound data than it was > set up to receive. The controller thus waits for more data. The flash chip can transfer data as long as you keep the clock going, especially when you transfer 64k off a 4M flash. The read command is bound just by stopping the clock. There is always more data to be had. > >> Data >> is left in FIFO which causes subsequent commands to fail since garbage >> is returned instead of command reply. Also subsequent read may cause >> I/O error or or return an empty page depending on the garbage around. > > So the driver for the DMA controller might need to be augmented to handle > this case -- add a timeout and in case DMA times out, drain the FIFO to > make sure subsequent transfers do not fail. There is no way to add timeout in the DMA driver since it does not know the SPI clock. The timeout is handled by the SPI driver and it should be possible to augment it to drain the FIFO when DMA fails so long as FIFO level is readable somewhere. > >> - the I/O errors are not checked in spi-nor at all which leads to >> silent data corruption. >> >> On a suggestion that this may improve reliability I changed the >> s3c64xx driver to use DMA for all transfers. This caused >> identification to fail in spin-nor because the ID was prefixed with >> extra 00 byte. Testing with spidev confirmed that everything is >> prefixed with extra 00. > > The determinism of this is in fact excellent news. > >> Also when the DMA controller locked up no >> transfers were possible anymore. When DMA was not used for sending >> commands the controller would recover on next command. I made the >> option to always use DMA configurable and it turns out that the >> returned data is prefixed with 00 only when no transfer without DMA >> was ever made. Loading the spi-nor driver with the dma-only option off >> and then with dma-only option on results in correct operation. Only >> loading the dma-only driver first causes the 00 prefix. > > Can we conclude that the PIO codepath somehow programs a register somewhere > which gets rid of this 0x00 prefix ? If so, this should then also be part > of the DMA codepath, or even better, this should be set in probe() somewhere. Yes, it looks like it. Thanks Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Monday, July 27, 2015 at 11:46:25 AM, Michal Suchanek wrote: > On 24 July 2015 at 10:34, Marek Vasut wrote: > > On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: > > > > Hi! > > > > [...] > > > >> >>> It's probably slower to set up DMA for 2-byte commands but it might > >> >>> work nonetheless. > >> >> > >> >> It is, the overhead will be considerable. It might help the stability > >> >> though. I'm really looking forward to the results! > >> > > >> > Hello, > >> > > >> > this does not quite work. > >> > > >> > My test with spidev: > >> > > >> > # ./spinor /dev/spidev1.0 > >> > Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > >> > Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 > >> > Sending 90 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > >> > Received 00 ff ff ff ff c8 15 c8 15 c8 15 c8 15 c8 15 c8 > >> > Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > >> > Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 > >> > > >> > I receive correct ID but spi-nor complains it does not know ID 00 c8 > >> > 60. IIRC garbage should be sent only at the time command is > >> > transferred so only one byte of garbage should be received. Also the > >> > garbage tends to be the last state of the data output - all 0 or all > >> > 1. > >> > So it seems using DMA for all transfers including 1-byte commands > >> > results in (some?) received data getting an extra 00 prefix. > >> > > >> > > >> > I also managed to lock up the controller completely since there is > >> > some error passing the SPI speed somewhere :( > >> > > >> > [ 1352.977530] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max > >> > --> 0 [ 1352.977540] spidev spi1.0: spi mode 0 > >> > [ 1352.977576] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max > >> > --> 0 [ 1352.977582] spidev spi1.0: msb first > >> > [ 1352.977614] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max > >> > --> 0 [ 1352.977620] spidev spi1.0: 0 bits per word > >> > [ 1352.977652] spidev spi1.0: setup mode 0, 8 bits/w, 2690588672 Hz > >> > max --> 0 [ 1352.977726] spi_master spi1: s3c64xx_spi_config: > >> > clk_from_cmu 1 src_clk sclk_spi1 mode bpw 8 > >> > [ 1352.977753] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: xfer > >> > bpw 8 speed -1604378624 > >> > [ 1352.977760] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 > >> > src_clk sclk_spi1 mode bpw 8 > >> > [ 1352.977781] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: using > >> > dma [ 1352.977797] dma-pl330 121b.pdma: setting up request on > >> > thread 1 > >> > >> Hmm, on a second thought it probably works as expected more or less. > >> > >> The nonsensical value was passed from application and there is no > >> guard against that. > >> > >> Since I don't do PIO the controller remains locked up indefinitely. > > > > I have to admit, I don't quite understand the above. I also don't quite > > know what your spidev test does. Can you possibly just bind a regular > > SPI NOR driver and run mtdtests to see if it is stable ? > > Ok, so here is some summary. > > I have a NOR flash attached to a s3c64xx SPI controller with 64byte > fifo and a pl330 dma controller. > > Normally DMA controller is used for transfers that do not fit into the > FIFO. > > I tried adding the flash memory ID to the spi-nor driver table and > adding a DT node for it. > > The flash is rated at 120MHz so I used that speed but the ID was > bit-shifted and identification failed. There is DT property > samsung,spi-feedback-delay for addressing this and at 120MHz it must > be 2 or 3 on this board. 40MHz works with default value 0. > > The next step after identification worked was to try reading the flash > content. For this the DMA controller is used because data is > transferred in blocks larger than 64 bytes. When reading the whole 4MB > flash the transfer failed silently. I got a 4MB file of all ones or > all zeroes. > > It turns out that > > - the pl330 locks up when transfering large amount of data. > Specifically, the maximum power of 2 sized transfer at 120MHz is 128 > bytes and 64k at 40MHz. Transferring more than this results in the > pl330 locking up and never signalling completion of the transfer. Hypothesis: I think this might just be that the controller didn't catch all the inbound clock ticks and thus counted less inbound data than it was set up to receive. The controller thus waits for more data. > Data > is left in FIFO which causes subsequent commands to fail since garbage > is returned instead of command reply. Also subsequent read may cause > I/O error or or return an empty page depending on the garbage around. So the driver for the DMA controller might need to be augmented to handle this case -- add a timeout and in case DMA times out, drain the FIFO to make sure subsequent transfers do not fail. > - the I/O errors are not checked in spi-nor at all which leads to > silent data corruption. > > On a suggestion that this may improve
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 24 July 2015 at 10:34, Marek Vasut wrote: > On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: > > Hi! > > [...] > >> >>> It's probably slower to set up DMA for 2-byte commands but it might >> >>> work nonetheless. >> >> >> >> It is, the overhead will be considerable. It might help the stability >> >> though. I'm really looking forward to the results! >> > >> > Hello, >> > >> > this does not quite work. >> > >> > My test with spidev: >> > >> > # ./spinor /dev/spidev1.0 >> > Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> > Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 >> > Sending 90 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> > Received 00 ff ff ff ff c8 15 c8 15 c8 15 c8 15 c8 15 c8 >> > Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> > Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 >> > >> > I receive correct ID but spi-nor complains it does not know ID 00 c8 60. >> > IIRC garbage should be sent only at the time command is transferred so >> > only one byte of garbage should be received. Also the garbage tends to >> > be the last state of the data output - all 0 or all 1. >> > So it seems using DMA for all transfers including 1-byte commands >> > results in (some?) received data getting an extra 00 prefix. >> > >> > >> > I also managed to lock up the controller completely since there is >> > some error passing the SPI speed somewhere :( >> > >> > [ 1352.977530] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max --> >> > 0 [ 1352.977540] spidev spi1.0: spi mode 0 >> > [ 1352.977576] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max --> >> > 0 [ 1352.977582] spidev spi1.0: msb first >> > [ 1352.977614] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max --> >> > 0 [ 1352.977620] spidev spi1.0: 0 bits per word >> > [ 1352.977652] spidev spi1.0: setup mode 0, 8 bits/w, 2690588672 Hz max >> > --> 0 [ 1352.977726] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 >> > src_clk sclk_spi1 mode bpw 8 >> > [ 1352.977753] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: xfer >> > bpw 8 speed -1604378624 >> > [ 1352.977760] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 >> > src_clk sclk_spi1 mode bpw 8 >> > [ 1352.977781] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: using >> > dma [ 1352.977797] dma-pl330 121b.pdma: setting up request on thread >> > 1 >> >> Hmm, on a second thought it probably works as expected more or less. >> >> The nonsensical value was passed from application and there is no >> guard against that. >> >> Since I don't do PIO the controller remains locked up indefinitely. > > I have to admit, I don't quite understand the above. I also don't quite know > what your spidev test does. Can you possibly just bind a regular SPI NOR > driver > and run mtdtests to see if it is stable ? Ok, so here is some summary. I have a NOR flash attached to a s3c64xx SPI controller with 64byte fifo and a pl330 dma controller. Normally DMA controller is used for transfers that do not fit into the FIFO. I tried adding the flash memory ID to the spi-nor driver table and adding a DT node for it. The flash is rated at 120MHz so I used that speed but the ID was bit-shifted and identification failed. There is DT property samsung,spi-feedback-delay for addressing this and at 120MHz it must be 2 or 3 on this board. 40MHz works with default value 0. The next step after identification worked was to try reading the flash content. For this the DMA controller is used because data is transferred in blocks larger than 64 bytes. When reading the whole 4MB flash the transfer failed silently. I got a 4MB file of all ones or all zeroes. It turns out that - the pl330 locks up when transfering large amount of data. Specifically, the maximum power of 2 sized transfer at 120MHz is 128 bytes and 64k at 40MHz. Transferring more than this results in the pl330 locking up and never signalling completion of the transfer. Data is left in FIFO which causes subsequent commands to fail since garbage is returned instead of command reply. Also subsequent read may cause I/O error or or return an empty page depending on the garbage around. - the I/O errors are not checked in spi-nor at all which leads to silent data corruption. On a suggestion that this may improve reliability I changed the s3c64xx driver to use DMA for all transfers. This caused identification to fail in spin-nor because the ID was prefixed with extra 00 byte. Testing with spidev confirmed that everything is prefixed with extra 00. Also when the DMA controller locked up no transfers were possible anymore. When DMA was not used for sending commands the controller would recover on next command. I made the option to always use DMA configurable and it turns out that the returned data is prefixed with 00 only when no transfer without DMA was ever made. Loading the spi-nor driver with the dma-only option off and then with dma-only option on results in correct operation. Only loading the
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Monday, July 27, 2015 at 11:46:25 AM, Michal Suchanek wrote: On 24 July 2015 at 10:34, Marek Vasut ma...@denx.de wrote: On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: Hi! [...] It's probably slower to set up DMA for 2-byte commands but it might work nonetheless. It is, the overhead will be considerable. It might help the stability though. I'm really looking forward to the results! Hello, this does not quite work. My test with spidev: # ./spinor /dev/spidev1.0 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 Sending 90 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff ff ff ff c8 15 c8 15 c8 15 c8 15 c8 15 c8 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 I receive correct ID but spi-nor complains it does not know ID 00 c8 60. IIRC garbage should be sent only at the time command is transferred so only one byte of garbage should be received. Also the garbage tends to be the last state of the data output - all 0 or all 1. So it seems using DMA for all transfers including 1-byte commands results in (some?) received data getting an extra 00 prefix. I also managed to lock up the controller completely since there is some error passing the SPI speed somewhere :( [ 1352.977530] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977540] spidev spi1.0: spi mode 0 [ 1352.977576] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977582] spidev spi1.0: msb first [ 1352.977614] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977620] spidev spi1.0: 0 bits per word [ 1352.977652] spidev spi1.0: setup mode 0, 8 bits/w, 2690588672 Hz max -- 0 [ 1352.977726] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 src_clk sclk_spi1 mode bpw 8 [ 1352.977753] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: xfer bpw 8 speed -1604378624 [ 1352.977760] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 src_clk sclk_spi1 mode bpw 8 [ 1352.977781] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: using dma [ 1352.977797] dma-pl330 121b.pdma: setting up request on thread 1 Hmm, on a second thought it probably works as expected more or less. The nonsensical value was passed from application and there is no guard against that. Since I don't do PIO the controller remains locked up indefinitely. I have to admit, I don't quite understand the above. I also don't quite know what your spidev test does. Can you possibly just bind a regular SPI NOR driver and run mtdtests to see if it is stable ? Ok, so here is some summary. I have a NOR flash attached to a s3c64xx SPI controller with 64byte fifo and a pl330 dma controller. Normally DMA controller is used for transfers that do not fit into the FIFO. I tried adding the flash memory ID to the spi-nor driver table and adding a DT node for it. The flash is rated at 120MHz so I used that speed but the ID was bit-shifted and identification failed. There is DT property samsung,spi-feedback-delay for addressing this and at 120MHz it must be 2 or 3 on this board. 40MHz works with default value 0. The next step after identification worked was to try reading the flash content. For this the DMA controller is used because data is transferred in blocks larger than 64 bytes. When reading the whole 4MB flash the transfer failed silently. I got a 4MB file of all ones or all zeroes. It turns out that - the pl330 locks up when transfering large amount of data. Specifically, the maximum power of 2 sized transfer at 120MHz is 128 bytes and 64k at 40MHz. Transferring more than this results in the pl330 locking up and never signalling completion of the transfer. Hypothesis: I think this might just be that the controller didn't catch all the inbound clock ticks and thus counted less inbound data than it was set up to receive. The controller thus waits for more data. Data is left in FIFO which causes subsequent commands to fail since garbage is returned instead of command reply. Also subsequent read may cause I/O error or or return an empty page depending on the garbage around. So the driver for the DMA controller might need to be augmented to handle this case -- add a timeout and in case DMA times out, drain the FIFO to make sure subsequent transfers do not fail. - the I/O errors are not checked in spi-nor at all which leads to silent data corruption. On a suggestion that this may improve reliability I changed the s3c64xx driver to use DMA for all transfers. This caused identification to fail in spin-nor because the ID was prefixed with extra 00 byte. Testing with spidev confirmed that everything is prefixed with extra 00. The determinism of this is in fact
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 27 July 2015 at 19:43, Marek Vasut ma...@denx.de wrote: On Monday, July 27, 2015 at 11:46:25 AM, Michal Suchanek wrote: On 24 July 2015 at 10:34, Marek Vasut ma...@denx.de wrote: On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: Ok, so here is some summary. I have a NOR flash attached to a s3c64xx SPI controller with 64byte fifo and a pl330 dma controller. Normally DMA controller is used for transfers that do not fit into the FIFO. I tried adding the flash memory ID to the spi-nor driver table and adding a DT node for it. The flash is rated at 120MHz so I used that speed but the ID was bit-shifted and identification failed. There is DT property samsung,spi-feedback-delay for addressing this and at 120MHz it must be 2 or 3 on this board. 40MHz works with default value 0. The next step after identification worked was to try reading the flash content. For this the DMA controller is used because data is transferred in blocks larger than 64 bytes. When reading the whole 4MB flash the transfer failed silently. I got a 4MB file of all ones or all zeroes. It turns out that - the pl330 locks up when transfering large amount of data. Specifically, the maximum power of 2 sized transfer at 120MHz is 128 bytes and 64k at 40MHz. Transferring more than this results in the pl330 locking up and never signalling completion of the transfer. Hypothesis: I think this might just be that the controller didn't catch all the inbound clock ticks and thus counted less inbound data than it was set up to receive. The controller thus waits for more data. The flash chip can transfer data as long as you keep the clock going, especially when you transfer 64k off a 4M flash. The read command is bound just by stopping the clock. There is always more data to be had. Data is left in FIFO which causes subsequent commands to fail since garbage is returned instead of command reply. Also subsequent read may cause I/O error or or return an empty page depending on the garbage around. So the driver for the DMA controller might need to be augmented to handle this case -- add a timeout and in case DMA times out, drain the FIFO to make sure subsequent transfers do not fail. There is no way to add timeout in the DMA driver since it does not know the SPI clock. The timeout is handled by the SPI driver and it should be possible to augment it to drain the FIFO when DMA fails so long as FIFO level is readable somewhere. - the I/O errors are not checked in spi-nor at all which leads to silent data corruption. On a suggestion that this may improve reliability I changed the s3c64xx driver to use DMA for all transfers. This caused identification to fail in spin-nor because the ID was prefixed with extra 00 byte. Testing with spidev confirmed that everything is prefixed with extra 00. The determinism of this is in fact excellent news. Also when the DMA controller locked up no transfers were possible anymore. When DMA was not used for sending commands the controller would recover on next command. I made the option to always use DMA configurable and it turns out that the returned data is prefixed with 00 only when no transfer without DMA was ever made. Loading the spi-nor driver with the dma-only option off and then with dma-only option on results in correct operation. Only loading the dma-only driver first causes the 00 prefix. Can we conclude that the PIO codepath somehow programs a register somewhere which gets rid of this 0x00 prefix ? If so, this should then also be part of the DMA codepath, or even better, this should be set in probe() somewhere. Yes, it looks like it. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 24 July 2015 at 10:34, Marek Vasut ma...@denx.de wrote: On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: Hi! [...] It's probably slower to set up DMA for 2-byte commands but it might work nonetheless. It is, the overhead will be considerable. It might help the stability though. I'm really looking forward to the results! Hello, this does not quite work. My test with spidev: # ./spinor /dev/spidev1.0 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 Sending 90 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff ff ff ff c8 15 c8 15 c8 15 c8 15 c8 15 c8 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 I receive correct ID but spi-nor complains it does not know ID 00 c8 60. IIRC garbage should be sent only at the time command is transferred so only one byte of garbage should be received. Also the garbage tends to be the last state of the data output - all 0 or all 1. So it seems using DMA for all transfers including 1-byte commands results in (some?) received data getting an extra 00 prefix. I also managed to lock up the controller completely since there is some error passing the SPI speed somewhere :( [ 1352.977530] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977540] spidev spi1.0: spi mode 0 [ 1352.977576] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977582] spidev spi1.0: msb first [ 1352.977614] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977620] spidev spi1.0: 0 bits per word [ 1352.977652] spidev spi1.0: setup mode 0, 8 bits/w, 2690588672 Hz max -- 0 [ 1352.977726] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 src_clk sclk_spi1 mode bpw 8 [ 1352.977753] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: xfer bpw 8 speed -1604378624 [ 1352.977760] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 src_clk sclk_spi1 mode bpw 8 [ 1352.977781] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: using dma [ 1352.977797] dma-pl330 121b.pdma: setting up request on thread 1 Hmm, on a second thought it probably works as expected more or less. The nonsensical value was passed from application and there is no guard against that. Since I don't do PIO the controller remains locked up indefinitely. I have to admit, I don't quite understand the above. I also don't quite know what your spidev test does. Can you possibly just bind a regular SPI NOR driver and run mtdtests to see if it is stable ? Ok, so here is some summary. I have a NOR flash attached to a s3c64xx SPI controller with 64byte fifo and a pl330 dma controller. Normally DMA controller is used for transfers that do not fit into the FIFO. I tried adding the flash memory ID to the spi-nor driver table and adding a DT node for it. The flash is rated at 120MHz so I used that speed but the ID was bit-shifted and identification failed. There is DT property samsung,spi-feedback-delay for addressing this and at 120MHz it must be 2 or 3 on this board. 40MHz works with default value 0. The next step after identification worked was to try reading the flash content. For this the DMA controller is used because data is transferred in blocks larger than 64 bytes. When reading the whole 4MB flash the transfer failed silently. I got a 4MB file of all ones or all zeroes. It turns out that - the pl330 locks up when transfering large amount of data. Specifically, the maximum power of 2 sized transfer at 120MHz is 128 bytes and 64k at 40MHz. Transferring more than this results in the pl330 locking up and never signalling completion of the transfer. Data is left in FIFO which causes subsequent commands to fail since garbage is returned instead of command reply. Also subsequent read may cause I/O error or or return an empty page depending on the garbage around. - the I/O errors are not checked in spi-nor at all which leads to silent data corruption. On a suggestion that this may improve reliability I changed the s3c64xx driver to use DMA for all transfers. This caused identification to fail in spin-nor because the ID was prefixed with extra 00 byte. Testing with spidev confirmed that everything is prefixed with extra 00. Also when the DMA controller locked up no transfers were possible anymore. When DMA was not used for sending commands the controller would recover on next command. I made the option to always use DMA configurable and it turns out that the returned data is prefixed with 00 only when no transfer without DMA was ever made. Loading the spi-nor driver with the dma-only option off and then with dma-only option on results in correct operation. Only loading the dma-only driver first causes the 00 prefix. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 24 July 2015 at 10:34, Marek Vasut wrote: > On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: > > Hi! > > [...] > >> >>> It's probably slower to set up DMA for 2-byte commands but it might >> >>> work nonetheless. >> >> >> >> It is, the overhead will be considerable. It might help the stability >> >> though. I'm really looking forward to the results! >> > >> > Hello, >> > >> > this does not quite work. >> > >> > My test with spidev: >> > >> > # ./spinor /dev/spidev1.0 >> > Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> > Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 >> > Sending 90 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> > Received 00 ff ff ff ff c8 15 c8 15 c8 15 c8 15 c8 15 c8 >> > Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> > Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 >> > >> > I receive correct ID but spi-nor complains it does not know ID 00 c8 60. >> > IIRC garbage should be sent only at the time command is transferred so >> > only one byte of garbage should be received. Also the garbage tends to >> > be the last state of the data output - all 0 or all 1. >> > So it seems using DMA for all transfers including 1-byte commands >> > results in (some?) received data getting an extra 00 prefix. >> > >> > >> > I also managed to lock up the controller completely since there is >> > some error passing the SPI speed somewhere :( >> > >> > [ 1352.977530] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max --> >> > 0 [ 1352.977540] spidev spi1.0: spi mode 0 >> > [ 1352.977576] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max --> >> > 0 [ 1352.977582] spidev spi1.0: msb first >> > [ 1352.977614] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max --> >> > 0 [ 1352.977620] spidev spi1.0: 0 bits per word >> > [ 1352.977652] spidev spi1.0: setup mode 0, 8 bits/w, 2690588672 Hz max >> > --> 0 [ 1352.977726] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 >> > src_clk sclk_spi1 mode bpw 8 >> > [ 1352.977753] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: xfer >> > bpw 8 speed -1604378624 >> > [ 1352.977760] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 >> > src_clk sclk_spi1 mode bpw 8 >> > [ 1352.977781] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: using >> > dma [ 1352.977797] dma-pl330 121b.pdma: setting up request on thread >> > 1 >> >> Hmm, on a second thought it probably works as expected more or less. >> >> The nonsensical value was passed from application and there is no >> guard against that. >> >> Since I don't do PIO the controller remains locked up indefinitely. > > I have to admit, I don't quite understand the above. I also don't quite know > what your spidev test does. It does a full duplex transfer sending what is printed and printing what is received. > Can you possibly just bind a regular SPI NOR driver > and run mtdtests to see if it is stable ? I can if I use PIO for short transfers. Using DMA for all transfers results in the received data prefixed with 00 so the NOR flash identification fails. Admittedly I have no idea what the flash memory actually contains so if all DMA reads were always prefixed with 00 I could not tell. I vaguely recall reading the whole content and parsing the I can probably make the minimum length for DMA configurable so I can fall back to PIO when the controler locks up. It seems setting up a PIO transfer makes it work again. Thanks Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: Hi! [...] > >>> It's probably slower to set up DMA for 2-byte commands but it might > >>> work nonetheless. > >> > >> It is, the overhead will be considerable. It might help the stability > >> though. I'm really looking forward to the results! > > > > Hello, > > > > this does not quite work. > > > > My test with spidev: > > > > # ./spinor /dev/spidev1.0 > > Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 > > Sending 90 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > Received 00 ff ff ff ff c8 15 c8 15 c8 15 c8 15 c8 15 c8 > > Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 > > > > I receive correct ID but spi-nor complains it does not know ID 00 c8 60. > > IIRC garbage should be sent only at the time command is transferred so > > only one byte of garbage should be received. Also the garbage tends to > > be the last state of the data output - all 0 or all 1. > > So it seems using DMA for all transfers including 1-byte commands > > results in (some?) received data getting an extra 00 prefix. > > > > > > I also managed to lock up the controller completely since there is > > some error passing the SPI speed somewhere :( > > > > [ 1352.977530] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max --> > > 0 [ 1352.977540] spidev spi1.0: spi mode 0 > > [ 1352.977576] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max --> > > 0 [ 1352.977582] spidev spi1.0: msb first > > [ 1352.977614] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max --> > > 0 [ 1352.977620] spidev spi1.0: 0 bits per word > > [ 1352.977652] spidev spi1.0: setup mode 0, 8 bits/w, 2690588672 Hz max > > --> 0 [ 1352.977726] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 > > src_clk sclk_spi1 mode bpw 8 > > [ 1352.977753] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: xfer > > bpw 8 speed -1604378624 > > [ 1352.977760] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 > > src_clk sclk_spi1 mode bpw 8 > > [ 1352.977781] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: using > > dma [ 1352.977797] dma-pl330 121b.pdma: setting up request on thread > > 1 > > Hmm, on a second thought it probably works as expected more or less. > > The nonsensical value was passed from application and there is no > guard against that. > > Since I don't do PIO the controller remains locked up indefinitely. I have to admit, I don't quite understand the above. I also don't quite know what your spidev test does. Can you possibly just bind a regular SPI NOR driver and run mtdtests to see if it is stable ? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: Hi! [...] It's probably slower to set up DMA for 2-byte commands but it might work nonetheless. It is, the overhead will be considerable. It might help the stability though. I'm really looking forward to the results! Hello, this does not quite work. My test with spidev: # ./spinor /dev/spidev1.0 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 Sending 90 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff ff ff ff c8 15 c8 15 c8 15 c8 15 c8 15 c8 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 I receive correct ID but spi-nor complains it does not know ID 00 c8 60. IIRC garbage should be sent only at the time command is transferred so only one byte of garbage should be received. Also the garbage tends to be the last state of the data output - all 0 or all 1. So it seems using DMA for all transfers including 1-byte commands results in (some?) received data getting an extra 00 prefix. I also managed to lock up the controller completely since there is some error passing the SPI speed somewhere :( [ 1352.977530] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977540] spidev spi1.0: spi mode 0 [ 1352.977576] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977582] spidev spi1.0: msb first [ 1352.977614] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977620] spidev spi1.0: 0 bits per word [ 1352.977652] spidev spi1.0: setup mode 0, 8 bits/w, 2690588672 Hz max -- 0 [ 1352.977726] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 src_clk sclk_spi1 mode bpw 8 [ 1352.977753] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: xfer bpw 8 speed -1604378624 [ 1352.977760] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 src_clk sclk_spi1 mode bpw 8 [ 1352.977781] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: using dma [ 1352.977797] dma-pl330 121b.pdma: setting up request on thread 1 Hmm, on a second thought it probably works as expected more or less. The nonsensical value was passed from application and there is no guard against that. Since I don't do PIO the controller remains locked up indefinitely. I have to admit, I don't quite understand the above. I also don't quite know what your spidev test does. Can you possibly just bind a regular SPI NOR driver and run mtdtests to see if it is stable ? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 24 July 2015 at 10:34, Marek Vasut ma...@denx.de wrote: On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: Hi! [...] It's probably slower to set up DMA for 2-byte commands but it might work nonetheless. It is, the overhead will be considerable. It might help the stability though. I'm really looking forward to the results! Hello, this does not quite work. My test with spidev: # ./spinor /dev/spidev1.0 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 Sending 90 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff ff ff ff c8 15 c8 15 c8 15 c8 15 c8 15 c8 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 I receive correct ID but spi-nor complains it does not know ID 00 c8 60. IIRC garbage should be sent only at the time command is transferred so only one byte of garbage should be received. Also the garbage tends to be the last state of the data output - all 0 or all 1. So it seems using DMA for all transfers including 1-byte commands results in (some?) received data getting an extra 00 prefix. I also managed to lock up the controller completely since there is some error passing the SPI speed somewhere :( [ 1352.977530] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977540] spidev spi1.0: spi mode 0 [ 1352.977576] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977582] spidev spi1.0: msb first [ 1352.977614] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977620] spidev spi1.0: 0 bits per word [ 1352.977652] spidev spi1.0: setup mode 0, 8 bits/w, 2690588672 Hz max -- 0 [ 1352.977726] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 src_clk sclk_spi1 mode bpw 8 [ 1352.977753] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: xfer bpw 8 speed -1604378624 [ 1352.977760] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 src_clk sclk_spi1 mode bpw 8 [ 1352.977781] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: using dma [ 1352.977797] dma-pl330 121b.pdma: setting up request on thread 1 Hmm, on a second thought it probably works as expected more or less. The nonsensical value was passed from application and there is no guard against that. Since I don't do PIO the controller remains locked up indefinitely. I have to admit, I don't quite understand the above. I also don't quite know what your spidev test does. It does a full duplex transfer sending what is printed and printing what is received. Can you possibly just bind a regular SPI NOR driver and run mtdtests to see if it is stable ? I can if I use PIO for short transfers. Using DMA for all transfers results in the received data prefixed with 00 so the NOR flash identification fails. Admittedly I have no idea what the flash memory actually contains so if all DMA reads were always prefixed with 00 I could not tell. I vaguely recall reading the whole content and parsing the I can probably make the minimum length for DMA configurable so I can fall back to PIO when the controler locks up. It seems setting up a PIO transfer makes it work again. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 23 July 2015 at 18:46, Michal Suchanek wrote: > On 22 July 2015 at 11:01, Marek Vasut wrote: >> On Wednesday, July 22, 2015 at 10:38:14 AM, Michal Suchanek wrote: >>> On 22 July 2015 at 10:24, Marek Vasut wrote: >>> > On Wednesday, July 22, 2015 at 10:18:04 AM, Michal Suchanek wrote: >>> >> On 22 July 2015 at 09:58, Marek Vasut wrote: >>> >> > On Wednesday, July 22, 2015 at 09:45:27 AM, Michal Suchanek wrote: >>> >> >> On 22 July 2015 at 09:33, Marek Vasut wrote: >>> >> >> > On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: >>> >> >> >> On 22 July 2015 at 06:49, Vinod Koul wrote: >>> >> >> >> > On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: >>> >> >> >> >> > Or alternatively we could publish the limitations of the >>> >> >> >> >> > channel using capabilities so SPI knows I have a dmaengine >>> >> >> >> >> > channel and it can transfer max N length transfers so would >>> >> >> >> >> > be able to break rather than guessing it or coding in DT. >>> >> >> >> >> > Yes it may come from DT but that should be dmaengine driver >>> >> >> >> >> > rather than client driver :) >>> >> >> >> >> > >>> >> >> >> >> > This can be done by dma_get_slave_caps(chan, ) >>> >> >> >> >> > >>> >> >> >> >> > And we add max_length as one more parameter to existing set >>> >> >> >> >> > >>> >> >> >> >> > Also all this could be handled in generic SPI-dmaengine layer >>> >> >> >> >> > so that individual drivers don't have to code it in >>> >> >> >> >> > >>> >> >> >> >> > Let me know if this idea is okay, I can push the dmaengine >>> >> >> >> >> > bits... >>> >> >> >> >> >>> >> >> >> >> It would be ok if there was a fixed limit. However, the limit >>> >> >> >> >> depends on SPI slave settings. Presumably for other buses using >>> >> >> >> >> the dmaengine the limit would depend on the bus or slave >>> >> >> >> >> settings as well. I do not see a sane way of passing this all >>> >> >> >> >> the way to the dmaengine driver. >>> >> >> >> > >>> >> >> >> > I don't see why this should be client (SPI) dependent. The max >>> >> >> >> > length supported is a dmaengine constraint, typically flowing >>> >> >> >> > from max blocks/length it can transfer. Know this limit can >>> >> >> >> > allow clients to split transfers. >>> >> >> >> >>> >> >> >> In practice on the board I have the maximum transfer length before >>> >> >> >> it fails depends on SPI bus speed which is set up per slave. I >>> >> >> >> did not try searching the space of possible settings thorougly >>> >> >> >> and settled for a setting that gives reasonable speed and >>> >> >> >> transfer length. >>> >> >> > >>> >> >> > This looks more like a signal integrity issue though. >>> >> >> >>> >> >> It certainly does on the surface. However, when wrong data is >>> >> >> delivered over the SPI bus (such as when I use wrong phase setting) >>> >> >> the SPI controller happily delivers wrong data over PIO. >>> >> >> >>> >> >> The failure I am seeing is that the pl330 DMA program which >>> >> >> repeatedly waits for data from the SPI controller never finishes the >>> >> >> read loop and does not signal the interrupt. It seems it also leaves >>> >> >> some data in a FIFO somewhere so next command on the flash returns >>> >> >> garbage and fails. >>> >> > >>> >> > I observed something similar on MXS (mx28) SPI block. Do you use mixed >>> >> > PIO/DMA mode perhaps ? >>> >> >>> >> The SPI driver uses PIO for short transfers and DMA for transfers >>> >> longer than the controller FIFO. This seems to be the standard way to >>> >> do things.It works flawlessly so long as submitting overly long DMA >>> >> programs is avoided. >>> > >>> > Can you try doing JUST DMA, no PIO ? I remember seeing some bus >>> > synchronisation issues when I did mixed PIO/DMA on the MXS and it was >>> > nasty to track down. Just give pure DMA a go to see if the thing >>> > stabilizes somehow. >>> >>> It's probably slower to set up DMA for 2-byte commands but it might >>> work nonetheless. >> >> It is, the overhead will be considerable. It might help the stability >> though. I'm really looking forward to the results! >> > > Hello, > > this does not quite work. > > My test with spidev: > > # ./spinor /dev/spidev1.0 > Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 > Sending 90 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > Received 00 ff ff ff ff c8 15 c8 15 c8 15 c8 15 c8 15 c8 > Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 > > I receive correct ID but spi-nor complains it does not know ID 00 c8 60. > IIRC garbage should be sent only at the time command is transferred so > only one byte of garbage should be received. Also the garbage tends to > be the last state of the data output - all 0 or all 1. > So it seems using DMA for all transfers including 1-byte commands > results in (some?) received data getting an extra 00 prefix. > > I also managed to
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 22 July 2015 at 11:01, Marek Vasut wrote: > On Wednesday, July 22, 2015 at 10:38:14 AM, Michal Suchanek wrote: >> On 22 July 2015 at 10:24, Marek Vasut wrote: >> > On Wednesday, July 22, 2015 at 10:18:04 AM, Michal Suchanek wrote: >> >> On 22 July 2015 at 09:58, Marek Vasut wrote: >> >> > On Wednesday, July 22, 2015 at 09:45:27 AM, Michal Suchanek wrote: >> >> >> On 22 July 2015 at 09:33, Marek Vasut wrote: >> >> >> > On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: >> >> >> >> On 22 July 2015 at 06:49, Vinod Koul wrote: >> >> >> >> > On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: >> >> >> >> >> > Or alternatively we could publish the limitations of the >> >> >> >> >> > channel using capabilities so SPI knows I have a dmaengine >> >> >> >> >> > channel and it can transfer max N length transfers so would >> >> >> >> >> > be able to break rather than guessing it or coding in DT. >> >> >> >> >> > Yes it may come from DT but that should be dmaengine driver >> >> >> >> >> > rather than client driver :) >> >> >> >> >> > >> >> >> >> >> > This can be done by dma_get_slave_caps(chan, ) >> >> >> >> >> > >> >> >> >> >> > And we add max_length as one more parameter to existing set >> >> >> >> >> > >> >> >> >> >> > Also all this could be handled in generic SPI-dmaengine layer >> >> >> >> >> > so that individual drivers don't have to code it in >> >> >> >> >> > >> >> >> >> >> > Let me know if this idea is okay, I can push the dmaengine >> >> >> >> >> > bits... >> >> >> >> >> >> >> >> >> >> It would be ok if there was a fixed limit. However, the limit >> >> >> >> >> depends on SPI slave settings. Presumably for other buses using >> >> >> >> >> the dmaengine the limit would depend on the bus or slave >> >> >> >> >> settings as well. I do not see a sane way of passing this all >> >> >> >> >> the way to the dmaengine driver. >> >> >> >> > >> >> >> >> > I don't see why this should be client (SPI) dependent. The max >> >> >> >> > length supported is a dmaengine constraint, typically flowing >> >> >> >> > from max blocks/length it can transfer. Know this limit can >> >> >> >> > allow clients to split transfers. >> >> >> >> >> >> >> >> In practice on the board I have the maximum transfer length before >> >> >> >> it fails depends on SPI bus speed which is set up per slave. I >> >> >> >> did not try searching the space of possible settings thorougly >> >> >> >> and settled for a setting that gives reasonable speed and >> >> >> >> transfer length. >> >> >> > >> >> >> > This looks more like a signal integrity issue though. >> >> >> >> >> >> It certainly does on the surface. However, when wrong data is >> >> >> delivered over the SPI bus (such as when I use wrong phase setting) >> >> >> the SPI controller happily delivers wrong data over PIO. >> >> >> >> >> >> The failure I am seeing is that the pl330 DMA program which >> >> >> repeatedly waits for data from the SPI controller never finishes the >> >> >> read loop and does not signal the interrupt. It seems it also leaves >> >> >> some data in a FIFO somewhere so next command on the flash returns >> >> >> garbage and fails. >> >> > >> >> > I observed something similar on MXS (mx28) SPI block. Do you use mixed >> >> > PIO/DMA mode perhaps ? >> >> >> >> The SPI driver uses PIO for short transfers and DMA for transfers >> >> longer than the controller FIFO. This seems to be the standard way to >> >> do things.It works flawlessly so long as submitting overly long DMA >> >> programs is avoided. >> > >> > Can you try doing JUST DMA, no PIO ? I remember seeing some bus >> > synchronisation issues when I did mixed PIO/DMA on the MXS and it was >> > nasty to track down. Just give pure DMA a go to see if the thing >> > stabilizes somehow. >> >> It's probably slower to set up DMA for 2-byte commands but it might >> work nonetheless. > > It is, the overhead will be considerable. It might help the stability > though. I'm really looking forward to the results! > Hello, this does not quite work. My test with spidev: # ./spinor /dev/spidev1.0 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 Sending 90 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff ff ff ff c8 15 c8 15 c8 15 c8 15 c8 15 c8 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 I receive correct ID but spi-nor complains it does not know ID 00 c8 60. IIRC garbage should be sent only at the time command is transferred so only one byte of garbage should be received. Also the garbage tends to be the last state of the data output - all 0 or all 1. So it seems using DMA for all transfers including 1-byte commands results in (some?) received data getting an extra 00 prefix. [ 26.702690] spi spi1.0: setup mode 0, 8 bits/w, 4000 Hz max --> 0 [ 26.703474] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 src_clk sclk_spi1 mode bpw 8 [ 26.703534]
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 22 July 2015 at 11:01, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 10:38:14 AM, Michal Suchanek wrote: On 22 July 2015 at 10:24, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 10:18:04 AM, Michal Suchanek wrote: On 22 July 2015 at 09:58, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:45:27 AM, Michal Suchanek wrote: On 22 July 2015 at 09:33, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: On 22 July 2015 at 06:49, Vinod Koul vinod.k...@intel.com wrote: On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, caps) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... It would be ok if there was a fixed limit. However, the limit depends on SPI slave settings. Presumably for other buses using the dmaengine the limit would depend on the bus or slave settings as well. I do not see a sane way of passing this all the way to the dmaengine driver. I don't see why this should be client (SPI) dependent. The max length supported is a dmaengine constraint, typically flowing from max blocks/length it can transfer. Know this limit can allow clients to split transfers. In practice on the board I have the maximum transfer length before it fails depends on SPI bus speed which is set up per slave. I did not try searching the space of possible settings thorougly and settled for a setting that gives reasonable speed and transfer length. This looks more like a signal integrity issue though. It certainly does on the surface. However, when wrong data is delivered over the SPI bus (such as when I use wrong phase setting) the SPI controller happily delivers wrong data over PIO. The failure I am seeing is that the pl330 DMA program which repeatedly waits for data from the SPI controller never finishes the read loop and does not signal the interrupt. It seems it also leaves some data in a FIFO somewhere so next command on the flash returns garbage and fails. I observed something similar on MXS (mx28) SPI block. Do you use mixed PIO/DMA mode perhaps ? The SPI driver uses PIO for short transfers and DMA for transfers longer than the controller FIFO. This seems to be the standard way to do things.It works flawlessly so long as submitting overly long DMA programs is avoided. Can you try doing JUST DMA, no PIO ? I remember seeing some bus synchronisation issues when I did mixed PIO/DMA on the MXS and it was nasty to track down. Just give pure DMA a go to see if the thing stabilizes somehow. It's probably slower to set up DMA for 2-byte commands but it might work nonetheless. It is, the overhead will be considerable. It might help the stability though. I'm really looking forward to the results! Hello, this does not quite work. My test with spidev: # ./spinor /dev/spidev1.0 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 Sending 90 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff ff ff ff c8 15 c8 15 c8 15 c8 15 c8 15 c8 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 I receive correct ID but spi-nor complains it does not know ID 00 c8 60. IIRC garbage should be sent only at the time command is transferred so only one byte of garbage should be received. Also the garbage tends to be the last state of the data output - all 0 or all 1. So it seems using DMA for all transfers including 1-byte commands results in (some?) received data getting an extra 00 prefix. [ 26.702690] spi spi1.0: setup mode 0, 8 bits/w, 4000 Hz max -- 0 [ 26.703474] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 src_clk sclk_spi1 mode bpw 8 [ 26.703534] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: xfer bpw 8 speed 4000 [ 26.703543] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: using dma [ 26.703568] dma-pl330 121b.pdma: setting up request on thread 1 [ 26.703576] bc041200:DMAMOV CCR 0x800201 [ 26.703585] bc041206:DMAMOV SAR 0x6e151280 [ 26.703593] bc04120c:DMAMOV DAR 0x12d30018 [ 26.703601] bc041212:DMAWFPS 5 [
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 23 July 2015 at 18:46, Michal Suchanek hramr...@gmail.com wrote: On 22 July 2015 at 11:01, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 10:38:14 AM, Michal Suchanek wrote: On 22 July 2015 at 10:24, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 10:18:04 AM, Michal Suchanek wrote: On 22 July 2015 at 09:58, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:45:27 AM, Michal Suchanek wrote: On 22 July 2015 at 09:33, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: On 22 July 2015 at 06:49, Vinod Koul vinod.k...@intel.com wrote: On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, caps) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... It would be ok if there was a fixed limit. However, the limit depends on SPI slave settings. Presumably for other buses using the dmaengine the limit would depend on the bus or slave settings as well. I do not see a sane way of passing this all the way to the dmaengine driver. I don't see why this should be client (SPI) dependent. The max length supported is a dmaengine constraint, typically flowing from max blocks/length it can transfer. Know this limit can allow clients to split transfers. In practice on the board I have the maximum transfer length before it fails depends on SPI bus speed which is set up per slave. I did not try searching the space of possible settings thorougly and settled for a setting that gives reasonable speed and transfer length. This looks more like a signal integrity issue though. It certainly does on the surface. However, when wrong data is delivered over the SPI bus (such as when I use wrong phase setting) the SPI controller happily delivers wrong data over PIO. The failure I am seeing is that the pl330 DMA program which repeatedly waits for data from the SPI controller never finishes the read loop and does not signal the interrupt. It seems it also leaves some data in a FIFO somewhere so next command on the flash returns garbage and fails. I observed something similar on MXS (mx28) SPI block. Do you use mixed PIO/DMA mode perhaps ? The SPI driver uses PIO for short transfers and DMA for transfers longer than the controller FIFO. This seems to be the standard way to do things.It works flawlessly so long as submitting overly long DMA programs is avoided. Can you try doing JUST DMA, no PIO ? I remember seeing some bus synchronisation issues when I did mixed PIO/DMA on the MXS and it was nasty to track down. Just give pure DMA a go to see if the thing stabilizes somehow. It's probably slower to set up DMA for 2-byte commands but it might work nonetheless. It is, the overhead will be considerable. It might help the stability though. I'm really looking forward to the results! Hello, this does not quite work. My test with spidev: # ./spinor /dev/spidev1.0 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 Sending 90 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff ff ff ff c8 15 c8 15 c8 15 c8 15 c8 15 c8 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 I receive correct ID but spi-nor complains it does not know ID 00 c8 60. IIRC garbage should be sent only at the time command is transferred so only one byte of garbage should be received. Also the garbage tends to be the last state of the data output - all 0 or all 1. So it seems using DMA for all transfers including 1-byte commands results in (some?) received data getting an extra 00 prefix. I also managed to lock up the controller completely since there is some error passing the SPI speed somewhere :( [ 1352.977530] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977540] spidev spi1.0: spi mode 0 [ 1352.977576] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977582] spidev spi1.0: msb first [ 1352.977614] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977620] spidev spi1.0: 0 bits per word [ 1352.977652] spidev spi1.0: setup mode 0, 8 bits/w,
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 22, 2015 at 10:38:14 AM, Michal Suchanek wrote: > On 22 July 2015 at 10:24, Marek Vasut wrote: > > On Wednesday, July 22, 2015 at 10:18:04 AM, Michal Suchanek wrote: > >> On 22 July 2015 at 09:58, Marek Vasut wrote: > >> > On Wednesday, July 22, 2015 at 09:45:27 AM, Michal Suchanek wrote: > >> >> On 22 July 2015 at 09:33, Marek Vasut wrote: > >> >> > On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: > >> >> >> On 22 July 2015 at 06:49, Vinod Koul wrote: > >> >> >> > On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: > >> >> >> >> > Or alternatively we could publish the limitations of the > >> >> >> >> > channel using capabilities so SPI knows I have a dmaengine > >> >> >> >> > channel and it can transfer max N length transfers so would > >> >> >> >> > be able to break rather than guessing it or coding in DT. > >> >> >> >> > Yes it may come from DT but that should be dmaengine driver > >> >> >> >> > rather than client driver :) > >> >> >> >> > > >> >> >> >> > This can be done by dma_get_slave_caps(chan, ) > >> >> >> >> > > >> >> >> >> > And we add max_length as one more parameter to existing set > >> >> >> >> > > >> >> >> >> > Also all this could be handled in generic SPI-dmaengine layer > >> >> >> >> > so that individual drivers don't have to code it in > >> >> >> >> > > >> >> >> >> > Let me know if this idea is okay, I can push the dmaengine > >> >> >> >> > bits... > >> >> >> >> > >> >> >> >> It would be ok if there was a fixed limit. However, the limit > >> >> >> >> depends on SPI slave settings. Presumably for other buses using > >> >> >> >> the dmaengine the limit would depend on the bus or slave > >> >> >> >> settings as well. I do not see a sane way of passing this all > >> >> >> >> the way to the dmaengine driver. > >> >> >> > > >> >> >> > I don't see why this should be client (SPI) dependent. The max > >> >> >> > length supported is a dmaengine constraint, typically flowing > >> >> >> > from max blocks/length it can transfer. Know this limit can > >> >> >> > allow clients to split transfers. > >> >> >> > >> >> >> In practice on the board I have the maximum transfer length before > >> >> >> it fails depends on SPI bus speed which is set up per slave. I > >> >> >> did not try searching the space of possible settings thorougly > >> >> >> and settled for a setting that gives reasonable speed and > >> >> >> transfer length. > >> >> > > >> >> > This looks more like a signal integrity issue though. > >> >> > >> >> It certainly does on the surface. However, when wrong data is > >> >> delivered over the SPI bus (such as when I use wrong phase setting) > >> >> the SPI controller happily delivers wrong data over PIO. > >> >> > >> >> The failure I am seeing is that the pl330 DMA program which > >> >> repeatedly waits for data from the SPI controller never finishes the > >> >> read loop and does not signal the interrupt. It seems it also leaves > >> >> some data in a FIFO somewhere so next command on the flash returns > >> >> garbage and fails. > >> > > >> > I observed something similar on MXS (mx28) SPI block. Do you use mixed > >> > PIO/DMA mode perhaps ? > >> > >> The SPI driver uses PIO for short transfers and DMA for transfers > >> longer than the controller FIFO. This seems to be the standard way to > >> do things.It works flawlessly so long as submitting overly long DMA > >> programs is avoided. > > > > Can you try doing JUST DMA, no PIO ? I remember seeing some bus > > synchronisation issues when I did mixed PIO/DMA on the MXS and it was > > nasty to track down. Just give pure DMA a go to see if the thing > > stabilizes somehow. > > It's probably slower to set up DMA for 2-byte commands but it might > work nonetheless. It is, the overhead will be considerable. It might help the stability though. I'm really looking forward to the results! > I will give it a go. > > >> > Do you have the option to connect a bus analyzer? > >> > I can probably offer you some tools, I'm in Prague ... > >> > >> The flash chip is accessible when removing the bottom cover. It is > >> VSOP8 package slightly lower than SOP8 so attaching clips to it might > >> be a bit problematic. That's the only accessible part. Everything > >> other than SPI is inside the SoC. > > > > That might be doable, though you might want to try the above thing first. > > > >> Since SPI has no verification whatsoever the chip might be completely > >> dead and you can still read fine all zeroes or all ones when > >> attempting a read from it. I observed this behaviour when I used a > >> flash chip in a socket and it was not firmly seated. It was with a > >> different SPI controller, though. > > > > You should run into issues as soon as the SPI NOR framework tries to read > > status register, no ? > > Yes, when the DMA transfer fails the next command fails due to garbage > lying around. However, you can unload the SPI NOR driver, load spidev > driver, and read enough garbage to
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 22 July 2015 at 10:24, Marek Vasut wrote: > On Wednesday, July 22, 2015 at 10:18:04 AM, Michal Suchanek wrote: >> On 22 July 2015 at 09:58, Marek Vasut wrote: >> > On Wednesday, July 22, 2015 at 09:45:27 AM, Michal Suchanek wrote: >> >> On 22 July 2015 at 09:33, Marek Vasut wrote: >> >> > On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: >> >> >> On 22 July 2015 at 06:49, Vinod Koul wrote: >> >> >> > On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: >> >> >> >> > Or alternatively we could publish the limitations of the channel >> >> >> >> > using capabilities so SPI knows I have a dmaengine channel and >> >> >> >> > it can transfer max N length transfers so would be able to >> >> >> >> > break rather than guessing it or coding in DT. Yes it may come >> >> >> >> > from DT but that should be dmaengine driver rather than client >> >> >> >> > driver :) >> >> >> >> > >> >> >> >> > This can be done by dma_get_slave_caps(chan, ) >> >> >> >> > >> >> >> >> > And we add max_length as one more parameter to existing set >> >> >> >> > >> >> >> >> > Also all this could be handled in generic SPI-dmaengine layer so >> >> >> >> > that individual drivers don't have to code it in >> >> >> >> > >> >> >> >> > Let me know if this idea is okay, I can push the dmaengine >> >> >> >> > bits... >> >> >> >> >> >> >> >> It would be ok if there was a fixed limit. However, the limit >> >> >> >> depends on SPI slave settings. Presumably for other buses using >> >> >> >> the dmaengine the limit would depend on the bus or slave settings >> >> >> >> as well. I do not see a sane way of passing this all the way to >> >> >> >> the dmaengine driver. >> >> >> > >> >> >> > I don't see why this should be client (SPI) dependent. The max >> >> >> > length supported is a dmaengine constraint, typically flowing from >> >> >> > max blocks/length it can transfer. Know this limit can allow >> >> >> > clients to split transfers. >> >> >> >> >> >> In practice on the board I have the maximum transfer length before it >> >> >> fails depends on SPI bus speed which is set up per slave. I did not >> >> >> try searching the space of possible settings thorougly and settled >> >> >> for a setting that gives reasonable speed and transfer length. >> >> > >> >> > This looks more like a signal integrity issue though. >> >> >> >> It certainly does on the surface. However, when wrong data is >> >> delivered over the SPI bus (such as when I use wrong phase setting) >> >> the SPI controller happily delivers wrong data over PIO. >> >> >> >> The failure I am seeing is that the pl330 DMA program which repeatedly >> >> waits for data from the SPI controller never finishes the read loop >> >> and does not signal the interrupt. It seems it also leaves some data >> >> in a FIFO somewhere so next command on the flash returns garbage and >> >> fails. >> > >> > I observed something similar on MXS (mx28) SPI block. Do you use mixed >> > PIO/DMA mode perhaps ? >> >> The SPI driver uses PIO for short transfers and DMA for transfers >> longer than the controller FIFO. This seems to be the standard way to >> do things.It works flawlessly so long as submitting overly long DMA >> programs is avoided. > > Can you try doing JUST DMA, no PIO ? I remember seeing some bus > synchronisation > issues when I did mixed PIO/DMA on the MXS and it was nasty to track down. > Just > give pure DMA a go to see if the thing stabilizes somehow. It's probably slower to set up DMA for 2-byte commands but it might work nonetheless. I will give it a go. > >> > Do you have the option to connect a bus analyzer? >> > I can probably offer you some tools, I'm in Prague ... >> >> The flash chip is accessible when removing the bottom cover. It is >> VSOP8 package slightly lower than SOP8 so attaching clips to it might >> be a bit problematic. That's the only accessible part. Everything >> other than SPI is inside the SoC. > > That might be doable, though you might want to try the above thing first. > >> Since SPI has no verification whatsoever the chip might be completely >> dead and you can still read fine all zeroes or all ones when >> attempting a read from it. I observed this behaviour when I used a >> flash chip in a socket and it was not firmly seated. It was with a >> different SPI controller, though. > > You should run into issues as soon as the SPI NOR framework tries to read > status register, no ? Yes, when the DMA transfer fails the next command fails due to garbage lying around. However, you can unload the SPI NOR driver, load spidev driver, and read enough garbage to empty the fifos. Then the flash identifies as normal again and you can access it. When the flash is not seated properly and acts autistic you get all 0xff or all 0 back whatever you send to it, obvously. The identification by the SPI NOR driver fails then. Thanks Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 22, 2015 at 10:18:04 AM, Michal Suchanek wrote: > On 22 July 2015 at 09:58, Marek Vasut wrote: > > On Wednesday, July 22, 2015 at 09:45:27 AM, Michal Suchanek wrote: > >> On 22 July 2015 at 09:33, Marek Vasut wrote: > >> > On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: > >> >> On 22 July 2015 at 06:49, Vinod Koul wrote: > >> >> > On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: > >> >> >> > Or alternatively we could publish the limitations of the channel > >> >> >> > using capabilities so SPI knows I have a dmaengine channel and > >> >> >> > it can transfer max N length transfers so would be able to > >> >> >> > break rather than guessing it or coding in DT. Yes it may come > >> >> >> > from DT but that should be dmaengine driver rather than client > >> >> >> > driver :) > >> >> >> > > >> >> >> > This can be done by dma_get_slave_caps(chan, ) > >> >> >> > > >> >> >> > And we add max_length as one more parameter to existing set > >> >> >> > > >> >> >> > Also all this could be handled in generic SPI-dmaengine layer so > >> >> >> > that individual drivers don't have to code it in > >> >> >> > > >> >> >> > Let me know if this idea is okay, I can push the dmaengine > >> >> >> > bits... > >> >> >> > >> >> >> It would be ok if there was a fixed limit. However, the limit > >> >> >> depends on SPI slave settings. Presumably for other buses using > >> >> >> the dmaengine the limit would depend on the bus or slave settings > >> >> >> as well. I do not see a sane way of passing this all the way to > >> >> >> the dmaengine driver. > >> >> > > >> >> > I don't see why this should be client (SPI) dependent. The max > >> >> > length supported is a dmaengine constraint, typically flowing from > >> >> > max blocks/length it can transfer. Know this limit can allow > >> >> > clients to split transfers. > >> >> > >> >> In practice on the board I have the maximum transfer length before it > >> >> fails depends on SPI bus speed which is set up per slave. I did not > >> >> try searching the space of possible settings thorougly and settled > >> >> for a setting that gives reasonable speed and transfer length. > >> > > >> > This looks more like a signal integrity issue though. > >> > >> It certainly does on the surface. However, when wrong data is > >> delivered over the SPI bus (such as when I use wrong phase setting) > >> the SPI controller happily delivers wrong data over PIO. > >> > >> The failure I am seeing is that the pl330 DMA program which repeatedly > >> waits for data from the SPI controller never finishes the read loop > >> and does not signal the interrupt. It seems it also leaves some data > >> in a FIFO somewhere so next command on the flash returns garbage and > >> fails. > > > > I observed something similar on MXS (mx28) SPI block. Do you use mixed > > PIO/DMA mode perhaps ? > > The SPI driver uses PIO for short transfers and DMA for transfers > longer than the controller FIFO. This seems to be the standard way to > do things.It works flawlessly so long as submitting overly long DMA > programs is avoided. Can you try doing JUST DMA, no PIO ? I remember seeing some bus synchronisation issues when I did mixed PIO/DMA on the MXS and it was nasty to track down. Just give pure DMA a go to see if the thing stabilizes somehow. > > Do you have the option to connect a bus analyzer? > > I can probably offer you some tools, I'm in Prague ... > > The flash chip is accessible when removing the bottom cover. It is > VSOP8 package slightly lower than SOP8 so attaching clips to it might > be a bit problematic. That's the only accessible part. Everything > other than SPI is inside the SoC. That might be doable, though you might want to try the above thing first. > Since SPI has no verification whatsoever the chip might be completely > dead and you can still read fine all zeroes or all ones when > attempting a read from it. I observed this behaviour when I used a > flash chip in a socket and it was not firmly seated. It was with a > different SPI controller, though. You should run into issues as soon as the SPI NOR framework tries to read status register, no ? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 22 July 2015 at 09:58, Marek Vasut wrote: > On Wednesday, July 22, 2015 at 09:45:27 AM, Michal Suchanek wrote: >> On 22 July 2015 at 09:33, Marek Vasut wrote: >> > On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: >> >> On 22 July 2015 at 06:49, Vinod Koul wrote: >> >> > On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: >> >> >> > Or alternatively we could publish the limitations of the channel >> >> >> > using capabilities so SPI knows I have a dmaengine channel and it >> >> >> > can transfer max N length transfers so would be able to break >> >> >> > rather than guessing it or coding in DT. Yes it may come from DT >> >> >> > but that should be dmaengine driver rather than client driver :) >> >> >> > >> >> >> > This can be done by dma_get_slave_caps(chan, ) >> >> >> > >> >> >> > And we add max_length as one more parameter to existing set >> >> >> > >> >> >> > Also all this could be handled in generic SPI-dmaengine layer so >> >> >> > that individual drivers don't have to code it in >> >> >> > >> >> >> > Let me know if this idea is okay, I can push the dmaengine bits... >> >> >> >> >> >> It would be ok if there was a fixed limit. However, the limit depends >> >> >> on SPI slave settings. Presumably for other buses using the dmaengine >> >> >> the limit would depend on the bus or slave settings as well. I do not >> >> >> see a sane way of passing this all the way to the dmaengine driver. >> >> > >> >> > I don't see why this should be client (SPI) dependent. The max length >> >> > supported is a dmaengine constraint, typically flowing from max >> >> > blocks/length it can transfer. Know this limit can allow clients to >> >> > split transfers. >> >> >> >> In practice on the board I have the maximum transfer length before it >> >> fails depends on SPI bus speed which is set up per slave. I did not >> >> try searching the space of possible settings thorougly and settled for >> >> a setting that gives reasonable speed and transfer length. >> > >> > This looks more like a signal integrity issue though. >> >> It certainly does on the surface. However, when wrong data is >> delivered over the SPI bus (such as when I use wrong phase setting) >> the SPI controller happily delivers wrong data over PIO. >> >> The failure I am seeing is that the pl330 DMA program which repeatedly >> waits for data from the SPI controller never finishes the read loop >> and does not signal the interrupt. It seems it also leaves some data >> in a FIFO somewhere so next command on the flash returns garbage and >> fails. > > I observed something similar on MXS (mx28) SPI block. Do you use mixed > PIO/DMA mode perhaps ? The SPI driver uses PIO for short transfers and DMA for transfers longer than the controller FIFO. This seems to be the standard way to do things.It works flawlessly so long as submitting overly long DMA programs is avoided. > Do you have the option to connect a bus analyzer? > I can probably offer you some tools, I'm in Prague ... The flash chip is accessible when removing the bottom cover. It is VSOP8 package slightly lower than SOP8 so attaching clips to it might be a bit problematic. That's the only accessible part. Everything other than SPI is inside the SoC. Since SPI has no verification whatsoever the chip might be completely dead and you can still read fine all zeroes or all ones when attempting a read from it. I observed this behaviour when I used a flash chip in a socket and it was not firmly seated. It was with a different SPI controller, though. Thanks Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 22, 2015 at 09:45:27 AM, Michal Suchanek wrote: > On 22 July 2015 at 09:33, Marek Vasut wrote: > > On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: > >> On 22 July 2015 at 06:49, Vinod Koul wrote: > >> > On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: > >> >> > Or alternatively we could publish the limitations of the channel > >> >> > using capabilities so SPI knows I have a dmaengine channel and it > >> >> > can transfer max N length transfers so would be able to break > >> >> > rather than guessing it or coding in DT. Yes it may come from DT > >> >> > but that should be dmaengine driver rather than client driver :) > >> >> > > >> >> > This can be done by dma_get_slave_caps(chan, ) > >> >> > > >> >> > And we add max_length as one more parameter to existing set > >> >> > > >> >> > Also all this could be handled in generic SPI-dmaengine layer so > >> >> > that individual drivers don't have to code it in > >> >> > > >> >> > Let me know if this idea is okay, I can push the dmaengine bits... > >> >> > >> >> It would be ok if there was a fixed limit. However, the limit depends > >> >> on SPI slave settings. Presumably for other buses using the dmaengine > >> >> the limit would depend on the bus or slave settings as well. I do not > >> >> see a sane way of passing this all the way to the dmaengine driver. > >> > > >> > I don't see why this should be client (SPI) dependent. The max length > >> > supported is a dmaengine constraint, typically flowing from max > >> > blocks/length it can transfer. Know this limit can allow clients to > >> > split transfers. > >> > >> In practice on the board I have the maximum transfer length before it > >> fails depends on SPI bus speed which is set up per slave. I did not > >> try searching the space of possible settings thorougly and settled for > >> a setting that gives reasonable speed and transfer length. > > > > This looks more like a signal integrity issue though. > > It certainly does on the surface. However, when wrong data is > delivered over the SPI bus (such as when I use wrong phase setting) > the SPI controller happily delivers wrong data over PIO. > > The failure I am seeing is that the pl330 DMA program which repeatedly > waits for data from the SPI controller never finishes the read loop > and does not signal the interrupt. It seems it also leaves some data > in a FIFO somewhere so next command on the flash returns garbage and > fails. I observed something similar on MXS (mx28) SPI block. Do you use mixed PIO/DMA mode perhaps ? Do you have the option to connect a bus analyzer? I can probably offer you some tools, I'm in Prague ... Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 22 July 2015 at 09:33, Marek Vasut wrote: > On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: >> On 22 July 2015 at 06:49, Vinod Koul wrote: >> > On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: >> >> > Or alternatively we could publish the limitations of the channel using >> >> > capabilities so SPI knows I have a dmaengine channel and it can >> >> > transfer max N length transfers so would be able to break rather than >> >> > guessing it or coding in DT. Yes it may come from DT but that should >> >> > be dmaengine driver rather than client driver :) >> >> > >> >> > This can be done by dma_get_slave_caps(chan, ) >> >> > >> >> > And we add max_length as one more parameter to existing set >> >> > >> >> > Also all this could be handled in generic SPI-dmaengine layer so that >> >> > individual drivers don't have to code it in >> >> > >> >> > Let me know if this idea is okay, I can push the dmaengine bits... >> >> >> >> It would be ok if there was a fixed limit. However, the limit depends >> >> on SPI slave settings. Presumably for other buses using the dmaengine >> >> the limit would depend on the bus or slave settings as well. I do not >> >> see a sane way of passing this all the way to the dmaengine driver. >> > >> > I don't see why this should be client (SPI) dependent. The max length >> > supported is a dmaengine constraint, typically flowing from max >> > blocks/length it can transfer. Know this limit can allow clients to split >> > transfers. >> >> In practice on the board I have the maximum transfer length before it >> fails depends on SPI bus speed which is set up per slave. I did not >> try searching the space of possible settings thorougly and settled for >> a setting that gives reasonable speed and transfer length. > > This looks more like a signal integrity issue though. > It certainly does on the surface. However, when wrong data is delivered over the SPI bus (such as when I use wrong phase setting) the SPI controller happily delivers wrong data over PIO. The failure I am seeing is that the pl330 DMA program which repeatedly waits for data from the SPI controller never finishes the read loop and does not signal the interrupt. It seems it also leaves some data in a FIFO somewhere so next command on the flash returns garbage and fails. Thanks Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: > On 22 July 2015 at 06:49, Vinod Koul wrote: > > On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: > >> > Or alternatively we could publish the limitations of the channel using > >> > capabilities so SPI knows I have a dmaengine channel and it can > >> > transfer max N length transfers so would be able to break rather than > >> > guessing it or coding in DT. Yes it may come from DT but that should > >> > be dmaengine driver rather than client driver :) > >> > > >> > This can be done by dma_get_slave_caps(chan, ) > >> > > >> > And we add max_length as one more parameter to existing set > >> > > >> > Also all this could be handled in generic SPI-dmaengine layer so that > >> > individual drivers don't have to code it in > >> > > >> > Let me know if this idea is okay, I can push the dmaengine bits... > >> > >> It would be ok if there was a fixed limit. However, the limit depends > >> on SPI slave settings. Presumably for other buses using the dmaengine > >> the limit would depend on the bus or slave settings as well. I do not > >> see a sane way of passing this all the way to the dmaengine driver. > > > > I don't see why this should be client (SPI) dependent. The max length > > supported is a dmaengine constraint, typically flowing from max > > blocks/length it can transfer. Know this limit can allow clients to split > > transfers. > > In practice on the board I have the maximum transfer length before it > fails depends on SPI bus speed which is set up per slave. I did not > try searching the space of possible settings thorougly and settled for > a setting that gives reasonable speed and transfer length. This looks more like a signal integrity issue though. > However, if this was not tied to the particular slave setting picked > in the current DT a formula would be needed that translates arbitrary > client settings to transfer size limit and there would be need to > somehow get the client settings to the formula in the dmaengine > driver. > > Thanks > > Michal Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 22 July 2015 at 06:49, Vinod Koul wrote: > On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: >> > Or alternatively we could publish the limitations of the channel using >> > capabilities so SPI knows I have a dmaengine channel and it can transfer >> > max N >> > length transfers so would be able to break rather than guessing it or >> > coding >> > in DT. Yes it may come from DT but that should be dmaengine driver rather >> > than client driver :) >> > >> > This can be done by dma_get_slave_caps(chan, ) >> > >> > And we add max_length as one more parameter to existing set >> > >> > Also all this could be handled in generic SPI-dmaengine layer so that >> > individual drivers don't have to code it in >> > >> > Let me know if this idea is okay, I can push the dmaengine bits... >> >> It would be ok if there was a fixed limit. However, the limit depends >> on SPI slave settings. Presumably for other buses using the dmaengine >> the limit would depend on the bus or slave settings as well. I do not >> see a sane way of passing this all the way to the dmaengine driver. > I don't see why this should be client (SPI) dependent. The max length > supported is a dmaengine constraint, typically flowing from max > blocks/length it can transfer. Know this limit can allow clients to split > transfers. In practice on the board I have the maximum transfer length before it fails depends on SPI bus speed which is set up per slave. I did not try searching the space of possible settings thorougly and settled for a setting that gives reasonable speed and transfer length. However, if this was not tied to the particular slave setting picked in the current DT a formula would be needed that translates arbitrary client settings to transfer size limit and there would be need to somehow get the client settings to the formula in the dmaengine driver. Thanks Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: On 22 July 2015 at 06:49, Vinod Koul vinod.k...@intel.com wrote: On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, caps) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... It would be ok if there was a fixed limit. However, the limit depends on SPI slave settings. Presumably for other buses using the dmaengine the limit would depend on the bus or slave settings as well. I do not see a sane way of passing this all the way to the dmaengine driver. I don't see why this should be client (SPI) dependent. The max length supported is a dmaengine constraint, typically flowing from max blocks/length it can transfer. Know this limit can allow clients to split transfers. In practice on the board I have the maximum transfer length before it fails depends on SPI bus speed which is set up per slave. I did not try searching the space of possible settings thorougly and settled for a setting that gives reasonable speed and transfer length. This looks more like a signal integrity issue though. However, if this was not tied to the particular slave setting picked in the current DT a formula would be needed that translates arbitrary client settings to transfer size limit and there would be need to somehow get the client settings to the formula in the dmaengine driver. Thanks Michal Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 22 July 2015 at 06:49, Vinod Koul vinod.k...@intel.com wrote: On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, caps) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... It would be ok if there was a fixed limit. However, the limit depends on SPI slave settings. Presumably for other buses using the dmaengine the limit would depend on the bus or slave settings as well. I do not see a sane way of passing this all the way to the dmaengine driver. I don't see why this should be client (SPI) dependent. The max length supported is a dmaengine constraint, typically flowing from max blocks/length it can transfer. Know this limit can allow clients to split transfers. In practice on the board I have the maximum transfer length before it fails depends on SPI bus speed which is set up per slave. I did not try searching the space of possible settings thorougly and settled for a setting that gives reasonable speed and transfer length. However, if this was not tied to the particular slave setting picked in the current DT a formula would be needed that translates arbitrary client settings to transfer size limit and there would be need to somehow get the client settings to the formula in the dmaengine driver. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 22 July 2015 at 09:58, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:45:27 AM, Michal Suchanek wrote: On 22 July 2015 at 09:33, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: On 22 July 2015 at 06:49, Vinod Koul vinod.k...@intel.com wrote: On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, caps) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... It would be ok if there was a fixed limit. However, the limit depends on SPI slave settings. Presumably for other buses using the dmaengine the limit would depend on the bus or slave settings as well. I do not see a sane way of passing this all the way to the dmaengine driver. I don't see why this should be client (SPI) dependent. The max length supported is a dmaengine constraint, typically flowing from max blocks/length it can transfer. Know this limit can allow clients to split transfers. In practice on the board I have the maximum transfer length before it fails depends on SPI bus speed which is set up per slave. I did not try searching the space of possible settings thorougly and settled for a setting that gives reasonable speed and transfer length. This looks more like a signal integrity issue though. It certainly does on the surface. However, when wrong data is delivered over the SPI bus (such as when I use wrong phase setting) the SPI controller happily delivers wrong data over PIO. The failure I am seeing is that the pl330 DMA program which repeatedly waits for data from the SPI controller never finishes the read loop and does not signal the interrupt. It seems it also leaves some data in a FIFO somewhere so next command on the flash returns garbage and fails. I observed something similar on MXS (mx28) SPI block. Do you use mixed PIO/DMA mode perhaps ? The SPI driver uses PIO for short transfers and DMA for transfers longer than the controller FIFO. This seems to be the standard way to do things.It works flawlessly so long as submitting overly long DMA programs is avoided. Do you have the option to connect a bus analyzer? I can probably offer you some tools, I'm in Prague ... The flash chip is accessible when removing the bottom cover. It is VSOP8 package slightly lower than SOP8 so attaching clips to it might be a bit problematic. That's the only accessible part. Everything other than SPI is inside the SoC. Since SPI has no verification whatsoever the chip might be completely dead and you can still read fine all zeroes or all ones when attempting a read from it. I observed this behaviour when I used a flash chip in a socket and it was not firmly seated. It was with a different SPI controller, though. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 22 July 2015 at 09:33, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: On 22 July 2015 at 06:49, Vinod Koul vinod.k...@intel.com wrote: On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, caps) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... It would be ok if there was a fixed limit. However, the limit depends on SPI slave settings. Presumably for other buses using the dmaengine the limit would depend on the bus or slave settings as well. I do not see a sane way of passing this all the way to the dmaengine driver. I don't see why this should be client (SPI) dependent. The max length supported is a dmaengine constraint, typically flowing from max blocks/length it can transfer. Know this limit can allow clients to split transfers. In practice on the board I have the maximum transfer length before it fails depends on SPI bus speed which is set up per slave. I did not try searching the space of possible settings thorougly and settled for a setting that gives reasonable speed and transfer length. This looks more like a signal integrity issue though. It certainly does on the surface. However, when wrong data is delivered over the SPI bus (such as when I use wrong phase setting) the SPI controller happily delivers wrong data over PIO. The failure I am seeing is that the pl330 DMA program which repeatedly waits for data from the SPI controller never finishes the read loop and does not signal the interrupt. It seems it also leaves some data in a FIFO somewhere so next command on the flash returns garbage and fails. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 22, 2015 at 10:38:14 AM, Michal Suchanek wrote: On 22 July 2015 at 10:24, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 10:18:04 AM, Michal Suchanek wrote: On 22 July 2015 at 09:58, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:45:27 AM, Michal Suchanek wrote: On 22 July 2015 at 09:33, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: On 22 July 2015 at 06:49, Vinod Koul vinod.k...@intel.com wrote: On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, caps) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... It would be ok if there was a fixed limit. However, the limit depends on SPI slave settings. Presumably for other buses using the dmaengine the limit would depend on the bus or slave settings as well. I do not see a sane way of passing this all the way to the dmaengine driver. I don't see why this should be client (SPI) dependent. The max length supported is a dmaengine constraint, typically flowing from max blocks/length it can transfer. Know this limit can allow clients to split transfers. In practice on the board I have the maximum transfer length before it fails depends on SPI bus speed which is set up per slave. I did not try searching the space of possible settings thorougly and settled for a setting that gives reasonable speed and transfer length. This looks more like a signal integrity issue though. It certainly does on the surface. However, when wrong data is delivered over the SPI bus (such as when I use wrong phase setting) the SPI controller happily delivers wrong data over PIO. The failure I am seeing is that the pl330 DMA program which repeatedly waits for data from the SPI controller never finishes the read loop and does not signal the interrupt. It seems it also leaves some data in a FIFO somewhere so next command on the flash returns garbage and fails. I observed something similar on MXS (mx28) SPI block. Do you use mixed PIO/DMA mode perhaps ? The SPI driver uses PIO for short transfers and DMA for transfers longer than the controller FIFO. This seems to be the standard way to do things.It works flawlessly so long as submitting overly long DMA programs is avoided. Can you try doing JUST DMA, no PIO ? I remember seeing some bus synchronisation issues when I did mixed PIO/DMA on the MXS and it was nasty to track down. Just give pure DMA a go to see if the thing stabilizes somehow. It's probably slower to set up DMA for 2-byte commands but it might work nonetheless. It is, the overhead will be considerable. It might help the stability though. I'm really looking forward to the results! I will give it a go. Do you have the option to connect a bus analyzer? I can probably offer you some tools, I'm in Prague ... The flash chip is accessible when removing the bottom cover. It is VSOP8 package slightly lower than SOP8 so attaching clips to it might be a bit problematic. That's the only accessible part. Everything other than SPI is inside the SoC. That might be doable, though you might want to try the above thing first. Since SPI has no verification whatsoever the chip might be completely dead and you can still read fine all zeroes or all ones when attempting a read from it. I observed this behaviour when I used a flash chip in a socket and it was not firmly seated. It was with a different SPI controller, though. You should run into issues as soon as the SPI NOR framework tries to read status register, no ? Yes, when the DMA transfer fails the next command fails due to garbage lying around. However, you can unload the SPI NOR driver, load spidev driver, and read enough garbage to empty the fifos. Then the flash identifies as normal again and you can access it. Yikes :( When the flash is not seated properly and acts autistic you get all 0xff or all 0 back whatever you send to it, obvously. The identification by the SPI NOR driver fails then. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 22, 2015 at 09:45:27 AM, Michal Suchanek wrote: On 22 July 2015 at 09:33, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: On 22 July 2015 at 06:49, Vinod Koul vinod.k...@intel.com wrote: On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, caps) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... It would be ok if there was a fixed limit. However, the limit depends on SPI slave settings. Presumably for other buses using the dmaengine the limit would depend on the bus or slave settings as well. I do not see a sane way of passing this all the way to the dmaengine driver. I don't see why this should be client (SPI) dependent. The max length supported is a dmaengine constraint, typically flowing from max blocks/length it can transfer. Know this limit can allow clients to split transfers. In practice on the board I have the maximum transfer length before it fails depends on SPI bus speed which is set up per slave. I did not try searching the space of possible settings thorougly and settled for a setting that gives reasonable speed and transfer length. This looks more like a signal integrity issue though. It certainly does on the surface. However, when wrong data is delivered over the SPI bus (such as when I use wrong phase setting) the SPI controller happily delivers wrong data over PIO. The failure I am seeing is that the pl330 DMA program which repeatedly waits for data from the SPI controller never finishes the read loop and does not signal the interrupt. It seems it also leaves some data in a FIFO somewhere so next command on the flash returns garbage and fails. I observed something similar on MXS (mx28) SPI block. Do you use mixed PIO/DMA mode perhaps ? Do you have the option to connect a bus analyzer? I can probably offer you some tools, I'm in Prague ... Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 22, 2015 at 10:18:04 AM, Michal Suchanek wrote: On 22 July 2015 at 09:58, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:45:27 AM, Michal Suchanek wrote: On 22 July 2015 at 09:33, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: On 22 July 2015 at 06:49, Vinod Koul vinod.k...@intel.com wrote: On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, caps) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... It would be ok if there was a fixed limit. However, the limit depends on SPI slave settings. Presumably for other buses using the dmaengine the limit would depend on the bus or slave settings as well. I do not see a sane way of passing this all the way to the dmaengine driver. I don't see why this should be client (SPI) dependent. The max length supported is a dmaengine constraint, typically flowing from max blocks/length it can transfer. Know this limit can allow clients to split transfers. In practice on the board I have the maximum transfer length before it fails depends on SPI bus speed which is set up per slave. I did not try searching the space of possible settings thorougly and settled for a setting that gives reasonable speed and transfer length. This looks more like a signal integrity issue though. It certainly does on the surface. However, when wrong data is delivered over the SPI bus (such as when I use wrong phase setting) the SPI controller happily delivers wrong data over PIO. The failure I am seeing is that the pl330 DMA program which repeatedly waits for data from the SPI controller never finishes the read loop and does not signal the interrupt. It seems it also leaves some data in a FIFO somewhere so next command on the flash returns garbage and fails. I observed something similar on MXS (mx28) SPI block. Do you use mixed PIO/DMA mode perhaps ? The SPI driver uses PIO for short transfers and DMA for transfers longer than the controller FIFO. This seems to be the standard way to do things.It works flawlessly so long as submitting overly long DMA programs is avoided. Can you try doing JUST DMA, no PIO ? I remember seeing some bus synchronisation issues when I did mixed PIO/DMA on the MXS and it was nasty to track down. Just give pure DMA a go to see if the thing stabilizes somehow. Do you have the option to connect a bus analyzer? I can probably offer you some tools, I'm in Prague ... The flash chip is accessible when removing the bottom cover. It is VSOP8 package slightly lower than SOP8 so attaching clips to it might be a bit problematic. That's the only accessible part. Everything other than SPI is inside the SoC. That might be doable, though you might want to try the above thing first. Since SPI has no verification whatsoever the chip might be completely dead and you can still read fine all zeroes or all ones when attempting a read from it. I observed this behaviour when I used a flash chip in a socket and it was not firmly seated. It was with a different SPI controller, though. You should run into issues as soon as the SPI NOR framework tries to read status register, no ? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 22 July 2015 at 10:24, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 10:18:04 AM, Michal Suchanek wrote: On 22 July 2015 at 09:58, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:45:27 AM, Michal Suchanek wrote: On 22 July 2015 at 09:33, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: On 22 July 2015 at 06:49, Vinod Koul vinod.k...@intel.com wrote: On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, caps) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... It would be ok if there was a fixed limit. However, the limit depends on SPI slave settings. Presumably for other buses using the dmaengine the limit would depend on the bus or slave settings as well. I do not see a sane way of passing this all the way to the dmaengine driver. I don't see why this should be client (SPI) dependent. The max length supported is a dmaengine constraint, typically flowing from max blocks/length it can transfer. Know this limit can allow clients to split transfers. In practice on the board I have the maximum transfer length before it fails depends on SPI bus speed which is set up per slave. I did not try searching the space of possible settings thorougly and settled for a setting that gives reasonable speed and transfer length. This looks more like a signal integrity issue though. It certainly does on the surface. However, when wrong data is delivered over the SPI bus (such as when I use wrong phase setting) the SPI controller happily delivers wrong data over PIO. The failure I am seeing is that the pl330 DMA program which repeatedly waits for data from the SPI controller never finishes the read loop and does not signal the interrupt. It seems it also leaves some data in a FIFO somewhere so next command on the flash returns garbage and fails. I observed something similar on MXS (mx28) SPI block. Do you use mixed PIO/DMA mode perhaps ? The SPI driver uses PIO for short transfers and DMA for transfers longer than the controller FIFO. This seems to be the standard way to do things.It works flawlessly so long as submitting overly long DMA programs is avoided. Can you try doing JUST DMA, no PIO ? I remember seeing some bus synchronisation issues when I did mixed PIO/DMA on the MXS and it was nasty to track down. Just give pure DMA a go to see if the thing stabilizes somehow. It's probably slower to set up DMA for 2-byte commands but it might work nonetheless. I will give it a go. Do you have the option to connect a bus analyzer? I can probably offer you some tools, I'm in Prague ... The flash chip is accessible when removing the bottom cover. It is VSOP8 package slightly lower than SOP8 so attaching clips to it might be a bit problematic. That's the only accessible part. Everything other than SPI is inside the SoC. That might be doable, though you might want to try the above thing first. Since SPI has no verification whatsoever the chip might be completely dead and you can still read fine all zeroes or all ones when attempting a read from it. I observed this behaviour when I used a flash chip in a socket and it was not firmly seated. It was with a different SPI controller, though. You should run into issues as soon as the SPI NOR framework tries to read status register, no ? Yes, when the DMA transfer fails the next command fails due to garbage lying around. However, you can unload the SPI NOR driver, load spidev driver, and read enough garbage to empty the fifos. Then the flash identifies as normal again and you can access it. When the flash is not seated properly and acts autistic you get all 0xff or all 0 back whatever you send to it, obvously. The identification by the SPI NOR driver fails then. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: > > Or alternatively we could publish the limitations of the channel using > > capabilities so SPI knows I have a dmaengine channel and it can transfer > > max N > > length transfers so would be able to break rather than guessing it or coding > > in DT. Yes it may come from DT but that should be dmaengine driver rather > > than client driver :) > > > > This can be done by dma_get_slave_caps(chan, ) > > > > And we add max_length as one more parameter to existing set > > > > Also all this could be handled in generic SPI-dmaengine layer so that > > individual drivers don't have to code it in > > > > Let me know if this idea is okay, I can push the dmaengine bits... > > It would be ok if there was a fixed limit. However, the limit depends > on SPI slave settings. Presumably for other buses using the dmaengine > the limit would depend on the bus or slave settings as well. I do not > see a sane way of passing this all the way to the dmaengine driver. I don't see why this should be client (SPI) dependent. The max length supported is a dmaengine constraint, typically flowing from max blocks/length it can transfer. Know this limit can allow clients to split transfers. -- ~Vinod -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
Hello, On 21 July 2015 at 06:29, Vinod Koul wrote: > On Sun, Jul 19, 2015 at 09:01:34PM +0200, Michal Suchanek wrote: >> Hello, >> >> On 15 July 2015 at 17:59, Brian Norris wrote: >> > Hi Michal, >> > >> > On Wed, Jul 15, 2015 at 01:52:27PM +0200, Marek Vasut wrote: >> >> The problem is, if you add a new DT binding, you'd have to support it >> >> forever, no matter how bad idea that binding turned out to be. >> > >> > Agreed, and a solid NAK to this patch. I could have sworn I gave such a >> > response when this was originally being discussed a month ago. >> > >> > AFAICT, you have one of two general approaches available to you: >> > >> > 1. Fix up the SPI driver so that it knows how to break large SPI >> > transfers up into smaller segments that its constituent hardware (DMA >> > controllers, fast clocks, etc.) can handle. >> > >> > 2. Utilize/create a parameter within the SPI subsystem to communicate >> > that the SPI master has a limited max transfer size (notably: NOT a >> > per-device DT property, but a SPI API property), and modify SPI device >> > drivers (like m25p80) to honor it. Mark Brown seemed open to this, and I >> > thought he suggested it somewhere. >> >> It is not known what exactly is limited here. >> >> It seems that the pl330 fails but it is not possible to transfer that >> much data over the spi bus in one go without the help of the pl330. >> >> With either approach the limit depends on the SPI transfer settings >> which are known the the SPI driver. The pl330 driver is oblivious to >> these because it just transfers data from one port to another port and >> has no idea that the port is wired to SPI in the SoC. >> >> On the other hand, AFAICT the SPI driver only allocates a DMA channel >> which it receives through DT binding and is oblivious to the fact the >> DMA channel lives on a pl330. It could probably determine that the >> channel is indeed driven by a pl330. I don't think it's a great idea >> to add device-specific handling to a generic dmaengine driver or a >> dmaengine-spiecific handling to a SPI driver. >> >> It's technically possible to pass SPI transfer parameters to the >> dmaengine driver prior to transfer and the dmaengine could impose some >> limitation based on those parameters. However, generalising this to >> drivers other than SPI might be problematic. Should this interface >> also handle i2c parameters, VE parameters, audio parameters, ethernet >> parameters, etc? > Or alternatively we could publish the limitations of the channel using > capabilities so SPI knows I have a dmaengine channel and it can transfer max N > length transfers so would be able to break rather than guessing it or coding > in DT. Yes it may come from DT but that should be dmaengine driver rather > than client driver :) > > This can be done by dma_get_slave_caps(chan, ) > > And we add max_length as one more parameter to existing set > > Also all this could be handled in generic SPI-dmaengine layer so that > individual drivers don't have to code it in > > Let me know if this idea is okay, I can push the dmaengine bits... > It would be ok if there was a fixed limit. However, the limit depends on SPI slave settings. Presumably for other buses using the dmaengine the limit would depend on the bus or slave settings as well. I do not see a sane way of passing this all the way to the dmaengine driver. Thanks Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, caps) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... It would be ok if there was a fixed limit. However, the limit depends on SPI slave settings. Presumably for other buses using the dmaengine the limit would depend on the bus or slave settings as well. I do not see a sane way of passing this all the way to the dmaengine driver. I don't see why this should be client (SPI) dependent. The max length supported is a dmaengine constraint, typically flowing from max blocks/length it can transfer. Know this limit can allow clients to split transfers. -- ~Vinod -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
Hello, On 21 July 2015 at 06:29, Vinod Koul vinod.k...@intel.com wrote: On Sun, Jul 19, 2015 at 09:01:34PM +0200, Michal Suchanek wrote: Hello, On 15 July 2015 at 17:59, Brian Norris computersforpe...@gmail.com wrote: Hi Michal, On Wed, Jul 15, 2015 at 01:52:27PM +0200, Marek Vasut wrote: The problem is, if you add a new DT binding, you'd have to support it forever, no matter how bad idea that binding turned out to be. Agreed, and a solid NAK to this patch. I could have sworn I gave such a response when this was originally being discussed a month ago. AFAICT, you have one of two general approaches available to you: 1. Fix up the SPI driver so that it knows how to break large SPI transfers up into smaller segments that its constituent hardware (DMA controllers, fast clocks, etc.) can handle. 2. Utilize/create a parameter within the SPI subsystem to communicate that the SPI master has a limited max transfer size (notably: NOT a per-device DT property, but a SPI API property), and modify SPI device drivers (like m25p80) to honor it. Mark Brown seemed open to this, and I thought he suggested it somewhere. It is not known what exactly is limited here. It seems that the pl330 fails but it is not possible to transfer that much data over the spi bus in one go without the help of the pl330. With either approach the limit depends on the SPI transfer settings which are known the the SPI driver. The pl330 driver is oblivious to these because it just transfers data from one port to another port and has no idea that the port is wired to SPI in the SoC. On the other hand, AFAICT the SPI driver only allocates a DMA channel which it receives through DT binding and is oblivious to the fact the DMA channel lives on a pl330. It could probably determine that the channel is indeed driven by a pl330. I don't think it's a great idea to add device-specific handling to a generic dmaengine driver or a dmaengine-spiecific handling to a SPI driver. It's technically possible to pass SPI transfer parameters to the dmaengine driver prior to transfer and the dmaengine could impose some limitation based on those parameters. However, generalising this to drivers other than SPI might be problematic. Should this interface also handle i2c parameters, VE parameters, audio parameters, ethernet parameters, etc? Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, caps) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... It would be ok if there was a fixed limit. However, the limit depends on SPI slave settings. Presumably for other buses using the dmaengine the limit would depend on the bus or slave settings as well. I do not see a sane way of passing this all the way to the dmaengine driver. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Sun, Jul 19, 2015 at 09:01:34PM +0200, Michal Suchanek wrote: > Hello, > > On 15 July 2015 at 17:59, Brian Norris wrote: > > Hi Michal, > > > > On Wed, Jul 15, 2015 at 01:52:27PM +0200, Marek Vasut wrote: > >> The problem is, if you add a new DT binding, you'd have to support it > >> forever, no matter how bad idea that binding turned out to be. > > > > Agreed, and a solid NAK to this patch. I could have sworn I gave such a > > response when this was originally being discussed a month ago. > > > > AFAICT, you have one of two general approaches available to you: > > > > 1. Fix up the SPI driver so that it knows how to break large SPI > > transfers up into smaller segments that its constituent hardware (DMA > > controllers, fast clocks, etc.) can handle. > > > > 2. Utilize/create a parameter within the SPI subsystem to communicate > > that the SPI master has a limited max transfer size (notably: NOT a > > per-device DT property, but a SPI API property), and modify SPI device > > drivers (like m25p80) to honor it. Mark Brown seemed open to this, and I > > thought he suggested it somewhere. > > It is not known what exactly is limited here. > > It seems that the pl330 fails but it is not possible to transfer that > much data over the spi bus in one go without the help of the pl330. > > With either approach the limit depends on the SPI transfer settings > which are known the the SPI driver. The pl330 driver is oblivious to > these because it just transfers data from one port to another port and > has no idea that the port is wired to SPI in the SoC. > > On the other hand, AFAICT the SPI driver only allocates a DMA channel > which it receives through DT binding and is oblivious to the fact the > DMA channel lives on a pl330. It could probably determine that the > channel is indeed driven by a pl330. I don't think it's a great idea > to add device-specific handling to a generic dmaengine driver or a > dmaengine-spiecific handling to a SPI driver. > > It's technically possible to pass SPI transfer parameters to the > dmaengine driver prior to transfer and the dmaengine could impose some > limitation based on those parameters. However, generalising this to > drivers other than SPI might be problematic. Should this interface > also handle i2c parameters, VE parameters, audio parameters, ethernet > parameters, etc? Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, ) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... Thanks -- ~Vinod -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Sun, Jul 19, 2015 at 09:01:34PM +0200, Michal Suchanek wrote: Hello, On 15 July 2015 at 17:59, Brian Norris computersforpe...@gmail.com wrote: Hi Michal, On Wed, Jul 15, 2015 at 01:52:27PM +0200, Marek Vasut wrote: The problem is, if you add a new DT binding, you'd have to support it forever, no matter how bad idea that binding turned out to be. Agreed, and a solid NAK to this patch. I could have sworn I gave such a response when this was originally being discussed a month ago. AFAICT, you have one of two general approaches available to you: 1. Fix up the SPI driver so that it knows how to break large SPI transfers up into smaller segments that its constituent hardware (DMA controllers, fast clocks, etc.) can handle. 2. Utilize/create a parameter within the SPI subsystem to communicate that the SPI master has a limited max transfer size (notably: NOT a per-device DT property, but a SPI API property), and modify SPI device drivers (like m25p80) to honor it. Mark Brown seemed open to this, and I thought he suggested it somewhere. It is not known what exactly is limited here. It seems that the pl330 fails but it is not possible to transfer that much data over the spi bus in one go without the help of the pl330. With either approach the limit depends on the SPI transfer settings which are known the the SPI driver. The pl330 driver is oblivious to these because it just transfers data from one port to another port and has no idea that the port is wired to SPI in the SoC. On the other hand, AFAICT the SPI driver only allocates a DMA channel which it receives through DT binding and is oblivious to the fact the DMA channel lives on a pl330. It could probably determine that the channel is indeed driven by a pl330. I don't think it's a great idea to add device-specific handling to a generic dmaengine driver or a dmaengine-spiecific handling to a SPI driver. It's technically possible to pass SPI transfer parameters to the dmaengine driver prior to transfer and the dmaengine could impose some limitation based on those parameters. However, generalising this to drivers other than SPI might be problematic. Should this interface also handle i2c parameters, VE parameters, audio parameters, ethernet parameters, etc? Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, caps) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... Thanks -- ~Vinod -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
Hello, On 15 July 2015 at 17:59, Brian Norris wrote: > Hi Michal, > > On Wed, Jul 15, 2015 at 01:52:27PM +0200, Marek Vasut wrote: >> The problem is, if you add a new DT binding, you'd have to support it >> forever, no matter how bad idea that binding turned out to be. > > Agreed, and a solid NAK to this patch. I could have sworn I gave such a > response when this was originally being discussed a month ago. > > AFAICT, you have one of two general approaches available to you: > > 1. Fix up the SPI driver so that it knows how to break large SPI > transfers up into smaller segments that its constituent hardware (DMA > controllers, fast clocks, etc.) can handle. > > 2. Utilize/create a parameter within the SPI subsystem to communicate > that the SPI master has a limited max transfer size (notably: NOT a > per-device DT property, but a SPI API property), and modify SPI device > drivers (like m25p80) to honor it. Mark Brown seemed open to this, and I > thought he suggested it somewhere. It is not known what exactly is limited here. It seems that the pl330 fails but it is not possible to transfer that much data over the spi bus in one go without the help of the pl330. With either approach the limit depends on the SPI transfer settings which are known the the SPI driver. The pl330 driver is oblivious to these because it just transfers data from one port to another port and has no idea that the port is wired to SPI in the SoC. On the other hand, AFAICT the SPI driver only allocates a DMA channel which it receives through DT binding and is oblivious to the fact the DMA channel lives on a pl330. It could probably determine that the channel is indeed driven by a pl330. I don't think it's a great idea to add device-specific handling to a generic dmaengine driver or a dmaengine-spiecific handling to a SPI driver. It's technically possible to pass SPI transfer parameters to the dmaengine driver prior to transfer and the dmaengine could impose some limitation based on those parameters. However, generalising this to drivers other than SPI might be problematic. Should this interface also handle i2c parameters, VE parameters, audio parameters, ethernet parameters, etc? In the DT you have the information that this particular device is connected to a Samsung SPI controller connected to a pl330 dma engine. > > It is most definitely wrong to put this information solely in the slave > device DT (i.e., for the flash), when it is not a property of the flash > device at all. It's a property of the SPI master and/or its clocks and > DMA controllers. However, the clocks are set by the parameters of the device, not the parameters of the master. So the limitation applies for the master with the settings of the particular slave device. Different slave settings do impose different master limitations. Thanks Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
Hello, On 15 July 2015 at 17:59, Brian Norris computersforpe...@gmail.com wrote: Hi Michal, On Wed, Jul 15, 2015 at 01:52:27PM +0200, Marek Vasut wrote: The problem is, if you add a new DT binding, you'd have to support it forever, no matter how bad idea that binding turned out to be. Agreed, and a solid NAK to this patch. I could have sworn I gave such a response when this was originally being discussed a month ago. AFAICT, you have one of two general approaches available to you: 1. Fix up the SPI driver so that it knows how to break large SPI transfers up into smaller segments that its constituent hardware (DMA controllers, fast clocks, etc.) can handle. 2. Utilize/create a parameter within the SPI subsystem to communicate that the SPI master has a limited max transfer size (notably: NOT a per-device DT property, but a SPI API property), and modify SPI device drivers (like m25p80) to honor it. Mark Brown seemed open to this, and I thought he suggested it somewhere. It is not known what exactly is limited here. It seems that the pl330 fails but it is not possible to transfer that much data over the spi bus in one go without the help of the pl330. With either approach the limit depends on the SPI transfer settings which are known the the SPI driver. The pl330 driver is oblivious to these because it just transfers data from one port to another port and has no idea that the port is wired to SPI in the SoC. On the other hand, AFAICT the SPI driver only allocates a DMA channel which it receives through DT binding and is oblivious to the fact the DMA channel lives on a pl330. It could probably determine that the channel is indeed driven by a pl330. I don't think it's a great idea to add device-specific handling to a generic dmaengine driver or a dmaengine-spiecific handling to a SPI driver. It's technically possible to pass SPI transfer parameters to the dmaengine driver prior to transfer and the dmaengine could impose some limitation based on those parameters. However, generalising this to drivers other than SPI might be problematic. Should this interface also handle i2c parameters, VE parameters, audio parameters, ethernet parameters, etc? In the DT you have the information that this particular device is connected to a Samsung SPI controller connected to a pl330 dma engine. It is most definitely wrong to put this information solely in the slave device DT (i.e., for the flash), when it is not a property of the flash device at all. It's a property of the SPI master and/or its clocks and DMA controllers. However, the clocks are set by the parameters of the device, not the parameters of the master. So the limitation applies for the master with the settings of the particular slave device. Different slave settings do impose different master limitations. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Thursday, July 16, 2015 at 03:19:35 AM, Brian Norris wrote: > On Wed, Jul 15, 2015 at 07:15:50PM +0200, Marek Vasut wrote: > > On Wednesday, July 15, 2015 at 05:59:46 PM, Brian Norris wrote: > > > 1. Fix up the SPI driver so that it knows how to break large SPI > > > transfers up into smaller segments that its constituent hardware (DMA > > > controllers, fast clocks, etc.) can handle. > > BTW, Mark Brown already commented on this approach: > > http://lists.infradead.org/pipermail/linux-mtd/2015-May/059364.html > > I quote: > | With modern drivers using transfer_one() where we have control of the > | chip select we do also have the ability to add support to the core for > | chopping large transfers up into smaller ones that the hardware can > | handle transparently. That won't for everything though since it depends > | on us being able to manually control the chip select which not all > | hardware can do. > | > > I think this might actually be easier -- just do a transfer where you > > don't toggle CS and just stops the clock at the last bit, then do another > > (multiple) transfers which don't toggle CS at all, then finally do a > > transfer which toggles a CS at the end. > > Sounds OK to me. And as Mark noted, this could probably be done in the > core. I suppose Mark could suggest whether the most expedient path is to > hack the buggy driver to do this, or whether reworking the SPI core to > have the appropriate spi_master field(s) (and caveats) to support this. Yep, agreed. > > This should be pretty trivial > > to do and I think for example spi-mxs.c does this. > > I don't think spi-mxs.c really does this; it chooses between PIO and DMA > based on length, but with either option, it runs through each SPI > transfer in one go. You might be confused by the fact that this driver > implements ->transfer_one_message instead of ->transfer_one, so it has > to loop through all transfers in the message. It does chop up the DMA transfers between multiple runs of the DMA engine IIRC. But it does so within the driver, which apparently is not the way to go :) > (IIUC, spi-mxs.c could easily be rewritten to use ->transfer, and some > of that not-too-complicated boilerplate could be killed off.) Most likely, yes. Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wed, Jul 15, 2015 at 07:15:50PM +0200, Marek Vasut wrote: > On Wednesday, July 15, 2015 at 05:59:46 PM, Brian Norris wrote: > > 1. Fix up the SPI driver so that it knows how to break large SPI > > transfers up into smaller segments that its constituent hardware (DMA > > controllers, fast clocks, etc.) can handle. BTW, Mark Brown already commented on this approach: http://lists.infradead.org/pipermail/linux-mtd/2015-May/059364.html I quote: | With modern drivers using transfer_one() where we have control of the | chip select we do also have the ability to add support to the core for | chopping large transfers up into smaller ones that the hardware can | handle transparently. That won't for everything though since it depends | on us being able to manually control the chip select which not all | hardware can do. > I think this might actually be easier -- just do a transfer where you > don't toggle CS and just stops the clock at the last bit, then do another > (multiple) transfers which don't toggle CS at all, then finally do a > transfer which toggles a CS at the end. Sounds OK to me. And as Mark noted, this could probably be done in the core. I suppose Mark could suggest whether the most expedient path is to hack the buggy driver to do this, or whether reworking the SPI core to have the appropriate spi_master field(s) (and caveats) to support this. > This should be pretty trivial > to do and I think for example spi-mxs.c does this. I don't think spi-mxs.c really does this; it chooses between PIO and DMA based on length, but with either option, it runs through each SPI transfer in one go. You might be confused by the fact that this driver implements ->transfer_one_message instead of ->transfer_one, so it has to loop through all transfers in the message. (IIUC, spi-mxs.c could easily be rewritten to use ->transfer, and some of that not-too-complicated boilerplate could be killed off.) Brian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 15, 2015 at 05:59:46 PM, Brian Norris wrote: > Hi Michal, Hi all, > On Wed, Jul 15, 2015 at 01:52:27PM +0200, Marek Vasut wrote: > > The problem is, if you add a new DT binding, you'd have to support it > > forever, no matter how bad idea that binding turned out to be. > > Agreed, and a solid NAK to this patch. I could have sworn I gave such a > response when this was originally being discussed a month ago. > > AFAICT, you have one of two general approaches available to you: > > 1. Fix up the SPI driver so that it knows how to break large SPI > transfers up into smaller segments that its constituent hardware (DMA > controllers, fast clocks, etc.) can handle. I think this might actually be easier -- just do a transfer where you don't toggle CS and just stops the clock at the last bit, then do another (multiple) transfers which don't toggle CS at all, then finally do a transfer which toggles a CS at the end. This should be pretty trivial to do and I think for example spi-mxs.c does this. Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
Hi Michal, On Wed, Jul 15, 2015 at 01:52:27PM +0200, Marek Vasut wrote: > The problem is, if you add a new DT binding, you'd have to support it > forever, no matter how bad idea that binding turned out to be. Agreed, and a solid NAK to this patch. I could have sworn I gave such a response when this was originally being discussed a month ago. AFAICT, you have one of two general approaches available to you: 1. Fix up the SPI driver so that it knows how to break large SPI transfers up into smaller segments that its constituent hardware (DMA controllers, fast clocks, etc.) can handle. 2. Utilize/create a parameter within the SPI subsystem to communicate that the SPI master has a limited max transfer size (notably: NOT a per-device DT property, but a SPI API property), and modify SPI device drivers (like m25p80) to honor it. Mark Brown seemed open to this, and I thought he suggested it somewhere. It is most definitely wrong to put this information solely in the slave device DT (i.e., for the flash), when it is not a property of the flash device at all. It's a property of the SPI master and/or its clocks and DMA controllers. Brian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 15, 2015 at 11:45:07 AM, Michal Suchanek wrote: > On 4 June 2015 at 19:15, Richard Cochran wrote: > > On Thu, Jun 04, 2015 at 10:31:45AM +0200, Michal Suchanek wrote: > >> You might want to try to run the bus at 60MHz or 80MHz and then the > >> values would probably again be different. > >> > >> The first two values are set in DT so the logical place for setting > >> the third is also in DT. > >> > >> Otherwise you would need some in-kernel table of these settings. > > > > Or a formula. > > This formula probably needs to take into account > > - the unknown reason for the pl330 to fail transfer Shouldn't that be fixed at the PL330 level ? This looks like fixing a problem at the wrong place :) > - the device transfer speed and transfer phase as set in DT > - possibly device-specific latency and board-specific trace design > and assembly tolerances If the design is broken, then cap the speed as for normal SPI device. > Seriously, until I have at least a vague idea why the transfer fails I > am not comfortable pulling some formula out of thin air and pretending > I have a working patch. > > On the other hand, a parameter you can set in the DT and which comes > with a suggested value which can be tuned depending on the system > seems more viable. The problem is, if you add a new DT binding, you'd have to support it forever, no matter how bad idea that binding turned out to be. Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 4 June 2015 at 19:15, Richard Cochran wrote: > On Thu, Jun 04, 2015 at 10:31:45AM +0200, Michal Suchanek wrote: >> You might want to try to run the bus at 60MHz or 80MHz and then the >> values would probably again be different. >> >> The first two values are set in DT so the logical place for setting >> the third is also in DT. >> >> Otherwise you would need some in-kernel table of these settings. > > Or a formula. > This formula probably needs to take into account - the unknown reason for the pl330 to fail transfer - the device transfer speed and transfer phase as set in DT - possibly device-specific latency and board-specific trace design and assembly tolerances Seriously, until I have at least a vague idea why the transfer fails I am not comfortable pulling some formula out of thin air and pretending I have a working patch. On the other hand, a parameter you can set in the DT and which comes with a suggested value which can be tuned depending on the system seems more viable. Thanks Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Thursday, July 16, 2015 at 03:19:35 AM, Brian Norris wrote: On Wed, Jul 15, 2015 at 07:15:50PM +0200, Marek Vasut wrote: On Wednesday, July 15, 2015 at 05:59:46 PM, Brian Norris wrote: 1. Fix up the SPI driver so that it knows how to break large SPI transfers up into smaller segments that its constituent hardware (DMA controllers, fast clocks, etc.) can handle. BTW, Mark Brown already commented on this approach: http://lists.infradead.org/pipermail/linux-mtd/2015-May/059364.html I quote: | With modern drivers using transfer_one() where we have control of the | chip select we do also have the ability to add support to the core for | chopping large transfers up into smaller ones that the hardware can | handle transparently. That won't for everything though since it depends | on us being able to manually control the chip select which not all | hardware can do. | I think this might actually be easier -- just do a transfer where you don't toggle CS and just stops the clock at the last bit, then do another (multiple) transfers which don't toggle CS at all, then finally do a transfer which toggles a CS at the end. Sounds OK to me. And as Mark noted, this could probably be done in the core. I suppose Mark could suggest whether the most expedient path is to hack the buggy driver to do this, or whether reworking the SPI core to have the appropriate spi_master field(s) (and caveats) to support this. Yep, agreed. This should be pretty trivial to do and I think for example spi-mxs.c does this. I don't think spi-mxs.c really does this; it chooses between PIO and DMA based on length, but with either option, it runs through each SPI transfer in one go. You might be confused by the fact that this driver implements -transfer_one_message instead of -transfer_one, so it has to loop through all transfers in the message. It does chop up the DMA transfers between multiple runs of the DMA engine IIRC. But it does so within the driver, which apparently is not the way to go :) (IIUC, spi-mxs.c could easily be rewritten to use -transfer, and some of that not-too-complicated boilerplate could be killed off.) Most likely, yes. Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wed, Jul 15, 2015 at 07:15:50PM +0200, Marek Vasut wrote: On Wednesday, July 15, 2015 at 05:59:46 PM, Brian Norris wrote: 1. Fix up the SPI driver so that it knows how to break large SPI transfers up into smaller segments that its constituent hardware (DMA controllers, fast clocks, etc.) can handle. BTW, Mark Brown already commented on this approach: http://lists.infradead.org/pipermail/linux-mtd/2015-May/059364.html I quote: | With modern drivers using transfer_one() where we have control of the | chip select we do also have the ability to add support to the core for | chopping large transfers up into smaller ones that the hardware can | handle transparently. That won't for everything though since it depends | on us being able to manually control the chip select which not all | hardware can do. I think this might actually be easier -- just do a transfer where you don't toggle CS and just stops the clock at the last bit, then do another (multiple) transfers which don't toggle CS at all, then finally do a transfer which toggles a CS at the end. Sounds OK to me. And as Mark noted, this could probably be done in the core. I suppose Mark could suggest whether the most expedient path is to hack the buggy driver to do this, or whether reworking the SPI core to have the appropriate spi_master field(s) (and caveats) to support this. This should be pretty trivial to do and I think for example spi-mxs.c does this. I don't think spi-mxs.c really does this; it chooses between PIO and DMA based on length, but with either option, it runs through each SPI transfer in one go. You might be confused by the fact that this driver implements -transfer_one_message instead of -transfer_one, so it has to loop through all transfers in the message. (IIUC, spi-mxs.c could easily be rewritten to use -transfer, and some of that not-too-complicated boilerplate could be killed off.) Brian -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 4 June 2015 at 19:15, Richard Cochran richardcoch...@gmail.com wrote: On Thu, Jun 04, 2015 at 10:31:45AM +0200, Michal Suchanek wrote: You might want to try to run the bus at 60MHz or 80MHz and then the values would probably again be different. The first two values are set in DT so the logical place for setting the third is also in DT. Otherwise you would need some in-kernel table of these settings. Or a formula. This formula probably needs to take into account - the unknown reason for the pl330 to fail transfer - the device transfer speed and transfer phase as set in DT - possibly device-specific latency and board-specific trace design and assembly tolerances Seriously, until I have at least a vague idea why the transfer fails I am not comfortable pulling some formula out of thin air and pretending I have a working patch. On the other hand, a parameter you can set in the DT and which comes with a suggested value which can be tuned depending on the system seems more viable. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 15, 2015 at 05:59:46 PM, Brian Norris wrote: Hi Michal, Hi all, On Wed, Jul 15, 2015 at 01:52:27PM +0200, Marek Vasut wrote: The problem is, if you add a new DT binding, you'd have to support it forever, no matter how bad idea that binding turned out to be. Agreed, and a solid NAK to this patch. I could have sworn I gave such a response when this was originally being discussed a month ago. AFAICT, you have one of two general approaches available to you: 1. Fix up the SPI driver so that it knows how to break large SPI transfers up into smaller segments that its constituent hardware (DMA controllers, fast clocks, etc.) can handle. I think this might actually be easier -- just do a transfer where you don't toggle CS and just stops the clock at the last bit, then do another (multiple) transfers which don't toggle CS at all, then finally do a transfer which toggles a CS at the end. This should be pretty trivial to do and I think for example spi-mxs.c does this. Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 15, 2015 at 11:45:07 AM, Michal Suchanek wrote: On 4 June 2015 at 19:15, Richard Cochran richardcoch...@gmail.com wrote: On Thu, Jun 04, 2015 at 10:31:45AM +0200, Michal Suchanek wrote: You might want to try to run the bus at 60MHz or 80MHz and then the values would probably again be different. The first two values are set in DT so the logical place for setting the third is also in DT. Otherwise you would need some in-kernel table of these settings. Or a formula. This formula probably needs to take into account - the unknown reason for the pl330 to fail transfer Shouldn't that be fixed at the PL330 level ? This looks like fixing a problem at the wrong place :) - the device transfer speed and transfer phase as set in DT - possibly device-specific latency and board-specific trace design and assembly tolerances If the design is broken, then cap the speed as for normal SPI device. Seriously, until I have at least a vague idea why the transfer fails I am not comfortable pulling some formula out of thin air and pretending I have a working patch. On the other hand, a parameter you can set in the DT and which comes with a suggested value which can be tuned depending on the system seems more viable. The problem is, if you add a new DT binding, you'd have to support it forever, no matter how bad idea that binding turned out to be. Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
Hi Michal, On Wed, Jul 15, 2015 at 01:52:27PM +0200, Marek Vasut wrote: The problem is, if you add a new DT binding, you'd have to support it forever, no matter how bad idea that binding turned out to be. Agreed, and a solid NAK to this patch. I could have sworn I gave such a response when this was originally being discussed a month ago. AFAICT, you have one of two general approaches available to you: 1. Fix up the SPI driver so that it knows how to break large SPI transfers up into smaller segments that its constituent hardware (DMA controllers, fast clocks, etc.) can handle. 2. Utilize/create a parameter within the SPI subsystem to communicate that the SPI master has a limited max transfer size (notably: NOT a per-device DT property, but a SPI API property), and modify SPI device drivers (like m25p80) to honor it. Mark Brown seemed open to this, and I thought he suggested it somewhere. It is most definitely wrong to put this information solely in the slave device DT (i.e., for the flash), when it is not a property of the flash device at all. It's a property of the SPI master and/or its clocks and DMA controllers. Brian -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Thu, Jun 04, 2015 at 10:31:45AM +0200, Michal Suchanek wrote: > You might want to try to run the bus at 60MHz or 80MHz and then the > values would probably again be different. > > The first two values are set in DT so the logical place for setting > the third is also in DT. > > Otherwise you would need some in-kernel table of these settings. Or a formula. Thanks, Richard -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Thursday, June 04, 2015 at 06:31:45 AM, Michal Suchanek wrote: > On 4 June 2015 at 01:03, Marek Vasut wrote: > > On Wednesday, June 03, 2015 at 11:26:41 PM, Michal Suchanek wrote: > >> On sunxi the SPI controller currently does not have DMA support and > >> fails any transfer larger than 63 bytes. > >> > >> On Exynos the pl330 DMA controller fails any transfer larger than 64kb > >> when using slower speed like 40MHz and any transfer larger than 128bytes > >> when running at 133MHz. > > > > This smells more like some corruption of the data on the bus or something > > even more obscure. > > If the data was corrupted you would get corrupted data and not > transfer failure. AFAIK there is no checksum or anything. I actually > do get corrupted data when I use wrong feedback delay setting. OK > >> The best thing is that in both cases the controller can just lock up and > >> never finish potentially leaving the hardware in unusable state. [...] > >> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt > >> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt > >> > >> @@ -19,6 +19,12 @@ Optional properties: > >> all chips and support for it can not be detected at > >> > >> runtime. Refer to your chips' datasheet to check if this is supported by > >> your chip. > >> +- linux,max_tx_len : With some SPI controller drivers possible transfer > >> size is + limited. This may be hardware or driver > >> bug. + Transfer data in chunks no larger than this > >> value. + > >> > >> Using this option may severely degrade performance > >> and > >> > >> + possibly flash memory life when max_tx_len is > >> smaller than + flash page size (typically 256 bytes) > > > > Will we need similar patch for all other SPI slave drivers, like SPI NAND > > ? > > Probably. Some SPI slave drivers already do have similar option. So the SPI device drivers are implementing workarounds for buggy SPI hosts, even if those SPI devices themselves don't suffer from such limitations. Yes, this seems like the seriously wrong thing to do. > In general it cannot be expected that you can reliably transfer > arbitrarily large data over SPI it seems. This is what should be expected because this is how the bus is supposed to work in fact. You can transfer an arbitrary amount of data over SPI bus. If some driver has a limitation, it's that (bus, dma) driver which needs to implement a fix or a workaround. > However, if the nand driver transfers data a page at a time it should > be fine. ... until someone issues multi-block read, in which case it all falls down like a house of cards. It is impossible to build a reliable system on "should" and similar assumptions ; the driver must be solid. Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 4 June 2015 at 08:42, Geert Uytterhoeven wrote: > On Wed, Jun 3, 2015 at 11:26 PM, Michal Suchanek wrote: >> On sunxi the SPI controller currently does not have DMA support and fails >> any transfer larger than 63 bytes. > > This is a driver limitation, not a hardware limitation. > >> On Exynos the pl330 DMA controller fails any transfer larger than 64kb >> when using slower speed like 40MHz and any transfer larger than 128bytes >> when running at 133MHz. > > This may be a driver bug. > >> The best thing is that in both cases the controller can just lock up and >> never finish potentially leaving the hardware in unusable state. >> >> So it is required that the m25p80 driver actively prevents doing >> transfers that are too large for the current driver state on a >> particular piece of hardware. > > OK. > > DT describes the hardware, not buggy drivers. > > So IMHO this doesn't belong in DT, but it can be a field in struct spi_master. > Unfortunately, this cannot be a field in struct spi_master. The value can and does vary with device configuration and device configuration is only available in DT. For example, on the Snow board one working configuration is 40MHz spi bus speed, feedback delay 0 and maximum transfer size 64k. Another working configuration is 133MHz bus speed, feedback delay 3, and maximum transfer size 128 bytes. You might want to try to run the bus at 60MHz or 80MHz and then the values would probably again be different. The first two values are set in DT so the logical place for setting the third is also in DT. Otherwise you would need some in-kernel table of these settings. Thanks Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wed, Jun 3, 2015 at 11:26 PM, Michal Suchanek wrote:
> On sunxi the SPI controller currently does not have DMA support and fails
> any transfer larger than 63 bytes.
This is a driver limitation, not a hardware limitation.
> On Exynos the pl330 DMA controller fails any transfer larger than 64kb
> when using slower speed like 40MHz and any transfer larger than 128bytes
> when running at 133MHz.
This may be a driver bug.
> The best thing is that in both cases the controller can just lock up and
> never finish potentially leaving the hardware in unusable state.
>
> So it is required that the m25p80 driver actively prevents doing
> transfers that are too large for the current driver state on a
> particular piece of hardware.
OK.
> Signed-off-by: Michal Suchanek
>
> --
>
> Update commit message and documentation text.
> ---
> .../devicetree/bindings/mtd/jedec,spi-nor.txt | 6 ++
> drivers/mtd/devices/m25p80.c | 24
> --
> 2 files changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> index 2bee681..4e63ae8 100644
> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> @@ -19,6 +19,12 @@ Optional properties:
> all chips and support for it can not be detected at
> runtime.
> Refer to your chips' datasheet to check if this is
> supported
> by your chip.
> +- linux,max_tx_len : With some SPI controller drivers possible transfer size
> is
> + limited. This may be hardware or driver bug.
> + Transfer data in chunks no larger than this value.
> + Using this option may severely degrade performance and
> + possibly flash memory life when max_tx_len is smaller
> than
> + flash page size (typically 256 bytes)
DT describes the hardware, not buggy drivers.
So IMHO this doesn't belong in DT, but it can be a field in struct spi_master.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Thu, Jun 04, 2015 at 10:31:45AM +0200, Michal Suchanek wrote: You might want to try to run the bus at 60MHz or 80MHz and then the values would probably again be different. The first two values are set in DT so the logical place for setting the third is also in DT. Otherwise you would need some in-kernel table of these settings. Or a formula. Thanks, Richard -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Thursday, June 04, 2015 at 06:31:45 AM, Michal Suchanek wrote: On 4 June 2015 at 01:03, Marek Vasut ma...@denx.de wrote: On Wednesday, June 03, 2015 at 11:26:41 PM, Michal Suchanek wrote: On sunxi the SPI controller currently does not have DMA support and fails any transfer larger than 63 bytes. On Exynos the pl330 DMA controller fails any transfer larger than 64kb when using slower speed like 40MHz and any transfer larger than 128bytes when running at 133MHz. This smells more like some corruption of the data on the bus or something even more obscure. If the data was corrupted you would get corrupted data and not transfer failure. AFAIK there is no checksum or anything. I actually do get corrupted data when I use wrong feedback delay setting. OK The best thing is that in both cases the controller can just lock up and never finish potentially leaving the hardware in unusable state. [...] --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt @@ -19,6 +19,12 @@ Optional properties: all chips and support for it can not be detected at runtime. Refer to your chips' datasheet to check if this is supported by your chip. +- linux,max_tx_len : With some SPI controller drivers possible transfer size is + limited. This may be hardware or driver bug. + Transfer data in chunks no larger than this value. + Using this option may severely degrade performance and + possibly flash memory life when max_tx_len is smaller than + flash page size (typically 256 bytes) Will we need similar patch for all other SPI slave drivers, like SPI NAND ? Probably. Some SPI slave drivers already do have similar option. So the SPI device drivers are implementing workarounds for buggy SPI hosts, even if those SPI devices themselves don't suffer from such limitations. Yes, this seems like the seriously wrong thing to do. In general it cannot be expected that you can reliably transfer arbitrarily large data over SPI it seems. This is what should be expected because this is how the bus is supposed to work in fact. You can transfer an arbitrary amount of data over SPI bus. If some driver has a limitation, it's that (bus, dma) driver which needs to implement a fix or a workaround. However, if the nand driver transfers data a page at a time it should be fine. ... until someone issues multi-block read, in which case it all falls down like a house of cards. It is impossible to build a reliable system on should and similar assumptions ; the driver must be solid. Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wed, Jun 3, 2015 at 11:26 PM, Michal Suchanek hramr...@gmail.com wrote:
On sunxi the SPI controller currently does not have DMA support and fails
any transfer larger than 63 bytes.
This is a driver limitation, not a hardware limitation.
On Exynos the pl330 DMA controller fails any transfer larger than 64kb
when using slower speed like 40MHz and any transfer larger than 128bytes
when running at 133MHz.
This may be a driver bug.
The best thing is that in both cases the controller can just lock up and
never finish potentially leaving the hardware in unusable state.
So it is required that the m25p80 driver actively prevents doing
transfers that are too large for the current driver state on a
particular piece of hardware.
OK.
Signed-off-by: Michal Suchanek hramr...@gmail.com
--
Update commit message and documentation text.
---
.../devicetree/bindings/mtd/jedec,spi-nor.txt | 6 ++
drivers/mtd/devices/m25p80.c | 24
--
2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
index 2bee681..4e63ae8 100644
--- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
+++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
@@ -19,6 +19,12 @@ Optional properties:
all chips and support for it can not be detected at
runtime.
Refer to your chips' datasheet to check if this is
supported
by your chip.
+- linux,max_tx_len : With some SPI controller drivers possible transfer size
is
+ limited. This may be hardware or driver bug.
+ Transfer data in chunks no larger than this value.
+ Using this option may severely degrade performance and
+ possibly flash memory life when max_tx_len is smaller
than
+ flash page size (typically 256 bytes)
DT describes the hardware, not buggy drivers.
So IMHO this doesn't belong in DT, but it can be a field in struct spi_master.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 4 June 2015 at 08:42, Geert Uytterhoeven ge...@linux-m68k.org wrote: On Wed, Jun 3, 2015 at 11:26 PM, Michal Suchanek hramr...@gmail.com wrote: On sunxi the SPI controller currently does not have DMA support and fails any transfer larger than 63 bytes. This is a driver limitation, not a hardware limitation. On Exynos the pl330 DMA controller fails any transfer larger than 64kb when using slower speed like 40MHz and any transfer larger than 128bytes when running at 133MHz. This may be a driver bug. The best thing is that in both cases the controller can just lock up and never finish potentially leaving the hardware in unusable state. So it is required that the m25p80 driver actively prevents doing transfers that are too large for the current driver state on a particular piece of hardware. OK. DT describes the hardware, not buggy drivers. So IMHO this doesn't belong in DT, but it can be a field in struct spi_master. Unfortunately, this cannot be a field in struct spi_master. The value can and does vary with device configuration and device configuration is only available in DT. For example, on the Snow board one working configuration is 40MHz spi bus speed, feedback delay 0 and maximum transfer size 64k. Another working configuration is 133MHz bus speed, feedback delay 3, and maximum transfer size 128 bytes. You might want to try to run the bus at 60MHz or 80MHz and then the values would probably again be different. The first two values are set in DT so the logical place for setting the third is also in DT. Otherwise you would need some in-kernel table of these settings. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 4 June 2015 at 01:03, Marek Vasut wrote: > On Wednesday, June 03, 2015 at 11:26:41 PM, Michal Suchanek wrote: >> On sunxi the SPI controller currently does not have DMA support and fails >> any transfer larger than 63 bytes. >> >> On Exynos the pl330 DMA controller fails any transfer larger than 64kb >> when using slower speed like 40MHz and any transfer larger than 128bytes >> when running at 133MHz. > > This smells more like some corruption of the data on the bus or something > even more obscure. If the data was corrupted you would get corrupted data and not transfer failure. AFAIK there is no checksum or anything. I actually do get corrupted data when I use wrong feedback delay setting. > >> The best thing is that in both cases the controller can just lock up and >> never finish potentially leaving the hardware in unusable state. >> >> So it is required that the m25p80 driver actively prevents doing >> transfers that are too large for the current driver state on a >> particular piece of hardware. >> >> Signed-off-by: Michal Suchanek >> >> -- >> >> Update commit message and documentation text. >> --- >> .../devicetree/bindings/mtd/jedec,spi-nor.txt | 6 ++ >> drivers/mtd/devices/m25p80.c | 24 >> -- 2 files changed, 28 insertions(+), 2 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt >> b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt index >> 2bee681..4e63ae8 100644 >> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt >> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt >> @@ -19,6 +19,12 @@ Optional properties: >> all chips and support for it can not be detected at >> runtime. Refer to your chips' datasheet to check if this is supported by >> your chip. >> +- linux,max_tx_len : With some SPI controller drivers possible transfer >> size is + limited. This may be hardware or driver bug. >> + Transfer data in chunks no larger than this value. + >> Using this option may severely degrade performance and >> + possibly flash memory life when max_tx_len is >> smaller than + flash page size (typically 256 bytes) > > Will we need similar patch for all other SPI slave drivers, like SPI NAND ? Probably. Some SPI slave drivers already do have similar option. In general it cannot be expected that you can reliably transfer arbitrarily large data over SPI it seems. However, if the nand driver transfers data a page at a time it should be fine. Thanks Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, June 03, 2015 at 11:26:41 PM, Michal Suchanek wrote: > On sunxi the SPI controller currently does not have DMA support and fails > any transfer larger than 63 bytes. > > On Exynos the pl330 DMA controller fails any transfer larger than 64kb > when using slower speed like 40MHz and any transfer larger than 128bytes > when running at 133MHz. This smells more like some corruption of the data on the bus or something even more obscure. > The best thing is that in both cases the controller can just lock up and > never finish potentially leaving the hardware in unusable state. > > So it is required that the m25p80 driver actively prevents doing > transfers that are too large for the current driver state on a > particular piece of hardware. > > Signed-off-by: Michal Suchanek > > -- > > Update commit message and documentation text. > --- > .../devicetree/bindings/mtd/jedec,spi-nor.txt | 6 ++ > drivers/mtd/devices/m25p80.c | 24 > -- 2 files changed, 28 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt > b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt index > 2bee681..4e63ae8 100644 > --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt > +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt > @@ -19,6 +19,12 @@ Optional properties: > all chips and support for it can not be detected at > runtime. Refer to your chips' datasheet to check if this is supported by > your chip. > +- linux,max_tx_len : With some SPI controller drivers possible transfer > size is + limited. This may be hardware or driver bug. > + Transfer data in chunks no larger than this value. + > Using this option may severely degrade performance and > + possibly flash memory life when max_tx_len is > smaller than + flash page size (typically 256 bytes) Will we need similar patch for all other SPI slave drivers, like SPI NAND ? Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, June 03, 2015 at 11:26:41 PM, Michal Suchanek wrote: On sunxi the SPI controller currently does not have DMA support and fails any transfer larger than 63 bytes. On Exynos the pl330 DMA controller fails any transfer larger than 64kb when using slower speed like 40MHz and any transfer larger than 128bytes when running at 133MHz. This smells more like some corruption of the data on the bus or something even more obscure. The best thing is that in both cases the controller can just lock up and never finish potentially leaving the hardware in unusable state. So it is required that the m25p80 driver actively prevents doing transfers that are too large for the current driver state on a particular piece of hardware. Signed-off-by: Michal Suchanek hramr...@gmail.com -- Update commit message and documentation text. --- .../devicetree/bindings/mtd/jedec,spi-nor.txt | 6 ++ drivers/mtd/devices/m25p80.c | 24 -- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt index 2bee681..4e63ae8 100644 --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt @@ -19,6 +19,12 @@ Optional properties: all chips and support for it can not be detected at runtime. Refer to your chips' datasheet to check if this is supported by your chip. +- linux,max_tx_len : With some SPI controller drivers possible transfer size is + limited. This may be hardware or driver bug. + Transfer data in chunks no larger than this value. + Using this option may severely degrade performance and + possibly flash memory life when max_tx_len is smaller than + flash page size (typically 256 bytes) Will we need similar patch for all other SPI slave drivers, like SPI NAND ? Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 4 June 2015 at 01:03, Marek Vasut ma...@denx.de wrote: On Wednesday, June 03, 2015 at 11:26:41 PM, Michal Suchanek wrote: On sunxi the SPI controller currently does not have DMA support and fails any transfer larger than 63 bytes. On Exynos the pl330 DMA controller fails any transfer larger than 64kb when using slower speed like 40MHz and any transfer larger than 128bytes when running at 133MHz. This smells more like some corruption of the data on the bus or something even more obscure. If the data was corrupted you would get corrupted data and not transfer failure. AFAIK there is no checksum or anything. I actually do get corrupted data when I use wrong feedback delay setting. The best thing is that in both cases the controller can just lock up and never finish potentially leaving the hardware in unusable state. So it is required that the m25p80 driver actively prevents doing transfers that are too large for the current driver state on a particular piece of hardware. Signed-off-by: Michal Suchanek hramr...@gmail.com -- Update commit message and documentation text. --- .../devicetree/bindings/mtd/jedec,spi-nor.txt | 6 ++ drivers/mtd/devices/m25p80.c | 24 -- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt index 2bee681..4e63ae8 100644 --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt @@ -19,6 +19,12 @@ Optional properties: all chips and support for it can not be detected at runtime. Refer to your chips' datasheet to check if this is supported by your chip. +- linux,max_tx_len : With some SPI controller drivers possible transfer size is + limited. This may be hardware or driver bug. + Transfer data in chunks no larger than this value. + Using this option may severely degrade performance and + possibly flash memory life when max_tx_len is smaller than + flash page size (typically 256 bytes) Will we need similar patch for all other SPI slave drivers, like SPI NAND ? Probably. Some SPI slave drivers already do have similar option. In general it cannot be expected that you can reliably transfer arbitrarily large data over SPI it seems. However, if the nand driver transfers data a page at a time it should be fine. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
