> + xfer.tx_buf = kmalloc(4, GFP_KERNEL); > + if (xfer.tx_buf == NULL) { > + ret = -ENOMEM; > + goto error_ret; > + } > + > + xfer.rx_buf = kmalloc(4, GFP_KERNEL); > + if (xfer.rx_buf == NULL) { > + ret = -ENOMEM; > + goto error_free_tx; > + } Do these really need to be two kmallocs > + if (ret) { > + dev_err(&st->us->dev, "problem with get x offset"); > + goto error_free_rx; > + } > + *val = ((unsigned char *)(xfer.rx_buf))[0]; > + kfree(xfer.rx_buf); > + kfree(xfer.tx_buf); > + return ret; > +error_free_rx: > + kfree(xfer.rx_buf); > +error_free_tx: > + kfree(xfer.tx_buf); > +error_ret: > + return ret; That lot makes no sense. You could just drop through.. > +{ > + uint8_t val; > + int8_t ret; Kernel types (u8, s8 etc) > > + local_rx_buf = kmalloc(4, GFP_KERNEL); > + local_tx_buf = kmalloc(4, GFP_KERNEL); > + > + local_tx_buf[1] = LIS3L02DQ_READ_REG(reg_address); OOPS on out of memory case > + struct spi_transfer xfer = { > + .tx_buf = NULL, > + .rx_buf = NULL, > + .bits_per_word = 16, > + .len = 2, > + }; > + int ret; > + > + local_tx_buf = kmalloc(4, GFP_KERNEL); > + if (local_tx_buf == NULL) { > + ret = -ENOMEM; > + goto error_ret; > + } > + xfer.tx_buf = local_tx_buf; You seem to have a lot of almost identical routines here. It looks like with a few helpers this driver could be vastly shorter.