On Thu, Feb 23, 2023 at 09:56:33PM +0530, Krishna Yarlagadda wrote:
> + spi_bus_lock(phy->spi_device->master);
> +
> + while (len) {
Why?
> + spi_xfer[0].tx_buf = phy->iobuf;
> + spi_xfer[0].len = 1;
> + spi_message_add_tail(&spi_xfer[0], &m);
> +
> + spi_xfer[1].tx_buf = phy->iobuf + 1;
> + spi_xfer[1].len = 3;
> + spi_message_add_tail(&spi_xfer[1], &m);
Why would we make these two separate transfers?
> + if (out) {
> + spi_xfer[2].tx_buf = &phy->iobuf[4];
> + spi_xfer[2].rx_buf = NULL;
> + memcpy(&phy->iobuf[4], out, transfer_len);
> + out += transfer_len;
> + }
> +
> + if (in) {
> + spi_xfer[2].tx_buf = NULL;
> + spi_xfer[2].rx_buf = &phy->iobuf[4];
> + }
This will use the same buffer for rx and tx if some bug manages to leave
them both set. That shouldn't be an issue but it's an alarm bell
reading the code.
> index 988aabc31871..b88494e31239 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -184,8 +184,9 @@ struct spi_device {
> u8 chip_select;
> u8 bits_per_word;
> bool rt;
> -#define SPI_NO_TX BIT(31) /* No transmit wire */
> -#define SPI_NO_RX BIT(30) /* No receive wire */
> +#define SPI_NO_TX BIT(31) /* No transmit wire */
> +#define SPI_NO_RX BIT(30) /* No receive wire */
> +#define SPI_TPM_HW_FLOW BIT(29) /* TPM flow control */
Additions to the SPI API should be a separate commit for SPI rather than
merged into a driver change.
Attachment:
signature.asc
Description: PGP signature
