On Fri, Oct 27, 2023 at 06:10:16PM +0200, Harald Mommer wrote:Reminder for me: Need still to address this, but this is not code I'm currently working on, so comes later.
+config SPI_VIRTIOThis advice is going to be inappropriate for the majortiy of guests.
+ tristate "Virtio SPI SPI Controller"
+ depends on VIRTIO
+ help
+ This enables the Virtio SPI driver.
+
+ Virtio SPI is an SPI driver for virtual machines using Virtio.
+
+ If your Linux is a virtual machine using Virtio, say Y here.
+
Not needed any more, will be removed. Maybe I should remove this ____cacheline_aligned. It's only there because I looked too deeply into struct virtio_i2c_req, not because I think this ____cacheline_aligned is decisive here.+ // clang-format offRemove this clang-format stuff.
+ struct spi_transfer_head transfer_head ____cacheline_aligned;
+ const uint8_t *tx_buf ____cacheline_aligned;
+ uint8_t *rx_buf ____cacheline_aligned;
+ struct spi_transfer_result result ____cacheline_aligned;
+ // clang-format on
It was a reminder for me from where I got some inspiration and to reveal that not everything was invented by me. Served it's purpose, all such comment references to foreign code is being removed.+static struct spi_board_info board_info = {In what way is one supposed to compare with the i2c driver? What
+ .modalias = "spi-virtio",
+ .max_speed_hz = 125000000, /* Arbitrary very high limit */
+ .bus_num = 0, /* Patched later during initialization */
+ .chip_select = 0, /* Patched later during initialization */
+ .mode = SPI_MODE_0,
+};
+/* Compare with file i2c_virtio.c structure virtio_i2c_msg_done */
happens if the I2C driver changes? It would be better to ensure that
the code can be read and understood as a standalone thing.
Spec changed this in the meantime so I can do also.+ /* Fill struct spi_transfer_head */If the spec just copied the Linux terminology it'd have few issues :(
+ th->slave_id = spi->chip_select;
+ th->bits_per_word = spi->bits_per_word;The virtio spec for cs_change is *not* what the Linux cs_change field
+ th->cs_change = xfer->cs_change;
does, this will not do the right thing.
+ th->tx_nbits = xfer->tx_nbits;BUILD_BUG_ON()
+ th->rx_nbits = xfer->rx_nbits;
+ th->reserved[0] = 0;
+ th->reserved[1] = 0;
+ th->reserved[2] = 0;
+
+#if (VIRTIO_SPI_CPHA != SPI_CPHA)
+#error VIRTIO_SPI_CPHA != SPI_CPHA
+#endif