Re: [PATCH] drm/repaper: combine allocs in repaper_spi_transfer()

Den 13.03.2022 15.10, skrev trix@redhat.com:

From: Tom Rix trix@redhat.com

repaper_spi_transfer() allocates a single byte for the spi header and then another buffer for the payload. Combine the allocs into a single buffer with offsets. To simplify the offsets put the header after the payload.

Signed-off-by: Tom Rix trix@redhat.com

drivers/gpu/drm/tiny/repaper.c | 40 ++++++++++------------------------ 1 file changed, 12 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c index 37b6bb90e46e1..22a6732f35a01 100644 --- a/drivers/gpu/drm/tiny/repaper.c +++ b/drivers/gpu/drm/tiny/repaper.c @@ -100,50 +100,34 @@ static inline struct repaper_epd *drm_to_epd(struct drm_device *drm) static int repaper_spi_transfer(struct spi_device *spi, u8 header, const void *tx, void *rx, size_t len) {

• void *txbuf = NULL, *rxbuf = NULL; struct spi_transfer tr[2] = {};
• u8 *headerbuf;

• u8 *buf; int ret;

• headerbuf = kmalloc(1, GFP_KERNEL);
• if (!headerbuf)

• buf = kmalloc(1 + len, GFP_KERNEL);
• if (!buf) return -ENOMEM;

• headerbuf[0] = header;
• tr[0].tx_buf = headerbuf;

• buf[len] = header;
• tr[0].tx_buf = &buf[len];

I don't think this will work since the buffer is used directly for DMA on some platforms[1] so the buffers need to be at the correct alignment for that to work. For this reason I think it's better to leave this as-is since we know the kmalloc buffers will always be useable for DMA and the code is also easy to read and understand instead of calculating offsets.

[1] tinydrm/mipi-dbi: Use dma-safe buffers for all SPI transfers (a89bfc5d9a07)

Noralf.

tr[0].len = 1;

• /* Stack allocated tx? */
• if (tx && len <= 32) {

• txbuf = kmemdup(tx, len, GFP_KERNEL);
  

• if (!txbuf) {
  

• 	ret = -ENOMEM;
  

• 	goto out_free;
  

• }
  

• if (tx) {

• memcpy(buf, tx, len);
  

• tr[1].tx_buf = buf;
  

  }

• if (rx) {

• rxbuf = kmalloc(len, GFP_KERNEL);
  

• if (!rxbuf) {
  

• 	ret = -ENOMEM;
  

• 	goto out_free;
  

• }
  

• }

• if (rx)

• tr[1].rx_buf = buf;
  
  

• tr[1].tx_buf = txbuf ? txbuf : tx;

• tr[1].rx_buf = rxbuf; tr[1].len = len;

  ndelay(80); ret = spi_sync_transfer(spi, tr, 2); if (rx && !ret)

• memcpy(rx, rxbuf, len);
  

-out_free:

• kfree(headerbuf);
• kfree(txbuf);
• kfree(rxbuf);

• memcpy(rx, buf, len);
  
  

• kfree(buf); return ret;

}