Re: [PATCH v4 1/6] spi: Enable controllers to extend the SPI protocol with MOSI idle configuration

[Marcelo Schmitt <marcelo.schmitt1@xxxxxxxxx>]

On 06/20, David Lechner wrote:
> On 6/20/24 10:12 AM, Marcelo Schmitt wrote:
> > On 06/19, David Lechner wrote:
> >> On 6/19/24 1:58 PM, Marcelo Schmitt wrote:
> >>> On 06/19, David Lechner wrote:
> >>>> On 6/18/24 6:10 PM, Marcelo Schmitt wrote:
> >>>>
> >>>>
> >>
> >> ...
> >>
> >>>>
> >>>>> +the peripheral and also when CS is inactive.
> >>>>
> >>>> As I mentioned in a previous review, I think the key detail here is that the
> >>>> MOSI line has to be in the required state during the CS line assertion
> >>>> (falling edge). I didn't really get that from the current wording. The current
> >>>> wording makes it sound like MOSI needs to be high indefinitely longer.
> >>>
> >>> It may be that we only need MOSI high just before bringing CS low. Though,
> >>> I don't see that info in the datasheets. How much time would MOSI be required
> >>> to be high prior to bringing CS low? The timing diagrams for register access and
> >>> ADC sampling in "3-wire" mode all start and end with MOSI at logical 1 (high).
> >>> I think reg access work if MOSI is brought low after CS gets low, but sample
> >>> read definitely don't work.
> >>>
> >>> From the info available in datasheets, it looks like MOSI is indeed expected 
> >>> to be high indefinitely amount of time. Except when the controller is clocking
> >>> out data to the peripheral.
> >>>
> >>> Even if find out the amount of time MOSI would be required high prior to CS low,
> >>> then we would need some sort of MOSI high/low state set with a delay prior to
> >>> active CS. That might be enough to support the AD4000 series of devices but,
> >>> would it be worth the added complexity?
> >>>
> >>
> >> It needs to happen at the same time as setting CPOL for the SCLK line for the
> >> device that is about to have the CS asserted. So I don't think we are breaking
> >> new ground here. Typically, in most datasheets I've seen they tend to say
> >> something like 2 ns before the CS change. So in most cases, I don't think
> > which datasheets? Are any of those for devices supported by the ad4000 driver?
> 
> In the AD4000 datasheet, Figure 59, it shows the time needed for SDI setup
> before CS assertion, labeled as t_SSDICNV. Table 2 gives this value to be
> 2 ns.

That delay is for "4-wire" mode and it specifies the delay before bringing CS
high. In "3-wire" mode, we are bringing CS low to start transfers so it doesn't
look like t_SSDICNV applies to the "3-wire" mode setup.

> 
> So unless a SPI controller has a functional clock of > 500 MHz or somehow
> sets the SDI state and the CS assertion in the same register write this
> minimum delay will always be met. I highly suspect noting like this has
> happened before so no one ever needed to worry about the timing and it
> just works (for the similar case of CPOL).
> 
> > 
> >> anyone bothers adding a delay. But if a longer delay was really needed for
> >> a specific peripheral, we could add a SPI xfer with no read/write that has
> >> cs_off=1 and a delay to get the correct state of both MOSI and SCLK a longer
> >> time before the CS change.
> > 
> > I don't know if that would actually work. I have not tested doing something like that.
> > This also implies the controller will be able to start the next transfer right
> > after the first preparatory transfer ends and it will meet that inter-transfer
> > timing requirement (which I still didn't find documented anywhere).
> > I'm not convinced that would be the best way to support those devices.
> 
> I did something like this in the ad7944 driver where we needed an up to
> 500ns delay before asserting CS. On SPI controllers without a hardware
> sleep or FIFO, the delay will of course be much longer. But the delay
> is just a minimum delay, so longer doesn't hurt. It just affects the
> max sample rate that can be reliably achieved.
> 
In ad7944_3wire_cs_mode_init_msg(), xfers[1] is prepared with spi_transfer.delay
which is the "delay to be introduced after this transfer before
(optionally) changing the chipselect status, then starting the next transfer or
completing this @spi_message." That should work for ad7944 because
it has ADC SDI physically connected to VIO in that setup.
For ad4000, we would want to set MOSI high (by writing 1s) such that MOSI
is high when CS is brought high (if I'm getting what you are suggesting).
But spi_transfer.delay is introduced after the transfer and before changing CS
so I think MOSI may return to low if the controller idles MOSI low.
I can't see how this would work for ad4000.
Other delays we have for spi_transfer (cs_change_delay, word_delay) don't seem
to help for this particular case either.