daniel-santos / mcp2210-linux

MCP2210 driver for linux
http://danielthesantos.blogspot.com/search/label/mcp2210
51 stars 31 forks source link

BUG_ON(!xfer->tx_buf); SPI protocol misunderstanding. #42

Closed milkylainen closed 1 year ago

milkylainen commented 4 years ago

I'm looking at the mcp2110-spi.c code and either I am misunderstanding it or it is fundamentally broken.

SPI always shifts as many bytes in as out on the bus. Empty rx-buffers means data is thrown away. Empty tx-buffers means that the driver should shift out zeros on the tx channel for as many bytes are expected in the rx-buf (or atleast word sized transfers if one wants to be picky).

From the Linux kernel documentation on SPI: SPI transfers always write the same number of bytes as they read. Protocol drivers should always provide rx_buf and/or tx_buf. In some cases, they may also want to provide DMA addresses for the data being transferred; that may reduce overhead, when the underlying driver uses dma.

If the transmit buffer is null, zeroes will be shifted out while filling rx_buf. If the receive buffer is null, the data shifted in will be discarded. Only “len” bytes shift out (or in). It’s an error to try to shift out a partial word. (For example, by shifting out three bytes with word size of sixteen or twenty bits; the former uses two bytes per word, the latter uses four bytes.)

In the context of the low level master driver, either of the cases should be valid. Both missing are not. I don't think there is a requirement for the device driver to provide buffers for either of them. Observe the "rx_buf and/or tx_buf.". The SPI layer does not fill any missing buffer pointers, atleast not on older kernels. Please correct me if I am missing something.

daniel-santos commented 4 years ago

Thank you. I believe you are correct!

daniel-santos commented 4 years ago

Would you be ever so kind as to try out https://github.com/daniel-santos/mcp2210-linux/tree/issue-42 both with and without CONFIG_MCP2210_DEBUG for me please? I don't have an mcp2210 circuit ready to test with at the moment. Thanks!

satva commented 4 years ago

https://github.com/daniel-santos/mcp2210-linux/issues/36