On 9/23/2013 7:08 PM, Trent Piepho wrote: > On Mon, Sep 23, 2013 at 2:14 PM, Stephen Warren <swarren@xxxxxxxxxxxxx> wrote: >> >> That sounds broken. Normally, shouldn't CS assert before a transaction, >> stay asserted during a transaction, then deassert after the transaction? >> It shouldn't rise and fall very quickly in between parts of the transaction. > > That is normal, where a transaction is a spi_message made up of > multiple spi_transfers. The cs_change bit for a transfer will insert > a de-asserted pulse after a transfer or leave CS de-asserted after the > last transfer. > >>>>> need to generate a falling-edge to trigger the beginning of a SPI >>>>> transaction. Doing this write with the default value of SPI_COMMAND1 >>>>> causes a brief rise and fall of CS, giving us our falling-edge. > > I wonder, is the real problem that the spi layer allows CS to possibly > remain asserted between transactions to the same device? Normally you > would expect it to be de-asserted at the end of a spi_message, but I > believe the Linux spi semantics are that it may or may not actually be > de-asserted at that time. It only guarantees that is will be > de-asserted before a message to a different device starts. > > I guess this is supposed to be an optimization. Some drivers, like > gpio bit-banging, probably have a cost associated with any CS change. > Usually many messages in a row are to the same device. Most devices > don't care if CS pulses between messages. Thus not pulsing CS between > each message is faster. > >>> >>> Otherwise, this logic allows us to skip the spi of COMMAND1 which would >>> normally be used to create the falling edge to start a new transaction, >>> leaving the previous one open for more transfers. >> >> This sounds like something the SPI core should be managing. If a driver >> is using the SPI bus to communicate with a device in a way that needs CS >> left active while outside a transaction, it shouldn't be possible for >> another driver to come along and communicate with another device before >> the first transaction is all complete. The bus should be locked. >> Allowing anything else could corrupt the protocol that required specific >> CS states outside the transaction. > > If the transaction is one message, which can be multiple transfers and > multiple CS pulses, then the spi core always does it atomically. The > limitation is the driver can't get the result of the transaction until > the entire transaction is finished. > > If a driver needs to get part of a transaction to complete the rest, > e.g. read a 16-bit length from the device and then read that many > bytes, it can still be done. It doesn't seem to be documented in > spi-summary, but the way to do this is with spi_bus_(un)lock() and > spi_(a)sync_locked() calls. The driver must lock the bus, used the > _locked versions to issue spi_messages, then unlock when done. This > should prevent another device on the bus from getting a messages, and > thus CS pulses, in the middle of the transaction. I suppose I can should reword my comment then on the code which checked: + if (tspi->cs_control) { + if (tspi->cs_control != spi) + tegra_spi_writel(tspi, command1, SPI_COMMAND1); While this does do exactly what I said, ending a previous "on-going" transaction in favor of a new one, this shouldn't be expected to be the way for clients to guarantee that they have locked a bus. This is more of a way internally to the Tegra SPI driver to clear its state. -rhyland -- nvpublic -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html