lkml.org 
[lkml]   [2008]   [Jul]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [Patch 3/4] ST LIS3L02DQ accelerometer
    > +	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.


    \
     
     \ /
      Last update: 2008-07-23 19:57    [W:5.346 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site