[virtio-dev] Re: [virtio-comment] [PATCH] [PATCH v5] virtio-spi: add the device specification

Hi Viresh,

On 11/29/2023 3:30 PM, Viresh Kumar wrote:
> On 28-11-23, 20:58, Haixu Cui wrote:
>> On 11/27/2023 6:17 PM, Viresh Kumar wrote:
>>> On 24-11-23, 15:20, Haixu Cui wrote:
>>>> +For half-duplex read transfer, \field{rx_buf} is filled by Virtio SPI device and consumed
>>>> +by Virtio SPI driver. For half-duplex write transfer, \field{tx_buf} is filled by Virtio
>>>> +SPI driver and consumed by Virtio SPI device. And for full-duplex read and write transfer,
>>>> +both \field{tx_buf} and \field{rx_buf} are used.
>>>
>>> Should the length of both the buffers in full-duplex mode be same ? If yes, then
>>> this should be mentioned (in case it is not).
>>>
>>
>> No, there is no such limitation. Write and read buffers may be different is
>> size.
> 
> Hmm, I worked with a SPI controller over a decade ago, and I must be forgetting
> something here I guess. But from whatever little I remember, with full-duplex
> transfer, data flows on both MOSI and MISO lines as soon as clock signal is
> applied.  And so amount of data sent is always be equal to amount of data
> received by both sides.
> 
> Also if I see Linux's implementation of the `struct spi_transfer` [1], I see
> `tx_buf`, `rx_buf` and a single `len` field, which applies to both the buffers.
> Which I guess is indicating that both buffers are supposed to be of same length.
> 
> What am I missing ?

Oh so sorry for that. And I don't make it clear. Yes, tx_buf and rx_buf 
have the same size, Linux has such restriction. Just as you mention, 
kernel level spi_transfer has single "len", the same for 
spi_ioc_transfer passed from the userland.

But I am not sure if this is in the scope of the spec. Because this is 
ensured by Linux, but Virtio SPI driver won't also can't verify this.
This is a prerequisite for virtio spi processing requests.

What is your suggestion? How about adding some descriptions here, like 
"for full-duplex, tx_buf and rx_buf are same in size, this is guaranteed 
by the kernel"?

> 
>>>> +Virtio SPI device MUST NOT change the data in \field{tx_buf} if transfer type is half-duplex write
>>>> +or full-duplex read and write.
>>>
>>> The device MUST NOT change the contents of the tx_buf ever, i.e. for every mode
>>> there is. In half-duplex read mode, there is no tx_buf, but just rx_buf.
>>>
>>
>> Sorry I don'y understand here. In this statement, the transfer type is
>> either half-duplex write or full-duplex read and write. half-duplex read
>> type has no tx_buf, so I didn't mention it.
> 
> Sorry for not being clear earlier. I was suggesting you to just write this
> instead:
> 
> The Virtio SPI device MUST NOT change the data in the \field{tx_buf} field.
> 
> You don't really need to specify the modes here, as we are talking about tx_buf,
> which isn't available in the half-duplex read mode but all others.
> 

OK, thank you for your explanation. I will remove the "if" statement here.

> -- > Viresh
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/spi/spi.h#n1031

Thanks again.

Best Regards
Haixu Cui

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]