lkml.org 
[lkml]   [2017]   [Jun]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    Subject[PATCH 3.16 072/212] w1: ds2490: USB transfer buffers need to be DMAable
    3.16.44-rc1 review patch.  If anyone has any objections, please let me know.

    ------------------

    From: "Maciej S. Szmigiero" <mail@maciej.szmigiero.name>

    commit 61cd1b4cd1e8f7f7642ab64529d9bd52e8374641 upstream.

    ds2490 driver was doing USB transfers from / to buffers on a stack.
    This is not permitted and made the driver non-working with vmapped stacks.

    Since all these transfers are done under the same bus_mutex lock we can
    simply use shared buffers in a device private structure for two most common
    of them.

    While we are at it, let's also fix a comparison between int and size_t in
    ds9490r_search() which made the driver spin in this function if state
    register get requests were failing.

    Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
    Acked-by: Evgeniy Polyakov <zbr@ioremap.net>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    [bwh: Backported to 3.16: old code was using printk() directly]
    Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
    ---
    drivers/w1/masters/ds2490.c | 142 ++++++++++++++++++++++++++------------------
    1 file changed, 84 insertions(+), 58 deletions(-)

    --- a/drivers/w1/masters/ds2490.c
    +++ b/drivers/w1/masters/ds2490.c
    @@ -153,6 +153,9 @@ struct ds_device
    */
    u16 spu_bit;

    + u8 st_buf[ST_SIZE];
    + u8 byte_buf;
    +
    struct w1_bus_master master;
    };

    @@ -174,7 +177,6 @@ struct ds_status
    u8 data_in_buffer_status;
    u8 reserved1;
    u8 reserved2;
    -
    };

    static struct usb_device_id ds_id_table [] = {
    @@ -244,27 +246,6 @@ static int ds_send_control(struct ds_dev
    return err;
    }

    -static int ds_recv_status_nodump(struct ds_device *dev, struct ds_status *st,
    - unsigned char *buf, int size)
    -{
    - int count, err;
    -
    - memset(st, 0, sizeof(*st));
    -
    - count = 0;
    - err = usb_interrupt_msg(dev->udev, usb_rcvintpipe(dev->udev,
    - dev->ep[EP_STATUS]), buf, size, &count, 1000);
    - if (err < 0) {
    - printk(KERN_ERR "Failed to read 1-wire data from 0x%x: err=%d.\n", dev->ep[EP_STATUS], err);
    - return err;
    - }
    -
    - if (count >= sizeof(*st))
    - memcpy(st, buf, sizeof(*st));
    -
    - return count;
    -}
    -
    static inline void ds_print_msg(unsigned char *buf, unsigned char *str, int off)
    {
    printk(KERN_INFO "%45s: %8x\n", str, buf[off]);
    @@ -323,6 +304,35 @@ static void ds_dump_status(struct ds_dev
    }
    }

    +static int ds_recv_status(struct ds_device *dev, struct ds_status *st,
    + bool dump)
    +{
    + int count, err;
    +
    + if (st)
    + memset(st, 0, sizeof(*st));
    +
    + count = 0;
    + err = usb_interrupt_msg(dev->udev,
    + usb_rcvintpipe(dev->udev,
    + dev->ep[EP_STATUS]),
    + dev->st_buf, sizeof(dev->st_buf),
    + &count, 1000);
    + if (err < 0) {
    + pr_err("Failed to read 1-wire data from 0x%x: err=%d.\n",
    + dev->ep[EP_STATUS], err);
    + return err;
    + }
    +
    + if (dump)
    + ds_dump_status(dev, dev->st_buf, count);
    +
    + if (st && count >= sizeof(*st))
    + memcpy(st, dev->st_buf, sizeof(*st));
    +
    + return count;
    +}
    +
    static void ds_reset_device(struct ds_device *dev)
    {
    ds_send_control_cmd(dev, CTL_RESET_DEVICE, 0);
    @@ -345,7 +355,6 @@ static void ds_reset_device(struct ds_de
    static int ds_recv_data(struct ds_device *dev, unsigned char *buf, int size)
    {
    int count, err;
    - struct ds_status st;

    /* Careful on size. If size is less than what is available in
    * the input buffer, the device fails the bulk transfer and
    @@ -360,14 +369,9 @@ static int ds_recv_data(struct ds_device
    err = usb_bulk_msg(dev->udev, usb_rcvbulkpipe(dev->udev, dev->ep[EP_DATA_IN]),
    buf, size, &count, 1000);
    if (err < 0) {
    - u8 buf[ST_SIZE];
    - int count;
    -
    printk(KERN_INFO "Clearing ep0x%x.\n", dev->ep[EP_DATA_IN]);
    usb_clear_halt(dev->udev, usb_rcvbulkpipe(dev->udev, dev->ep[EP_DATA_IN]));
    -
    - count = ds_recv_status_nodump(dev, &st, buf, sizeof(buf));
    - ds_dump_status(dev, buf, count);
    + ds_recv_status(dev, NULL, true);
    return err;
    }

    @@ -405,7 +409,6 @@ int ds_stop_pulse(struct ds_device *dev,
    {
    struct ds_status st;
    int count = 0, err = 0;
    - u8 buf[ST_SIZE];

    do {
    err = ds_send_control(dev, CTL_HALT_EXE_IDLE, 0);
    @@ -414,7 +417,7 @@ int ds_stop_pulse(struct ds_device *dev,
    err = ds_send_control(dev, CTL_RESUME_EXE, 0);
    if (err)
    break;
    - err = ds_recv_status_nodump(dev, &st, buf, sizeof(buf));
    + err = ds_recv_status(dev, &st, false);
    if (err)
    break;

    @@ -457,18 +460,17 @@ int ds_detect(struct ds_device *dev, str

    static int ds_wait_status(struct ds_device *dev, struct ds_status *st)
    {
    - u8 buf[ST_SIZE];
    int err, count = 0;

    do {
    st->status = 0;
    - err = ds_recv_status_nodump(dev, st, buf, sizeof(buf));
    + err = ds_recv_status(dev, st, false);
    #if 0
    if (err >= 0) {
    int i;
    printk("0x%x: count=%d, status: ", dev->ep[EP_STATUS], err);
    for (i=0; i<err; ++i)
    - printk("%02x ", buf[i]);
    + printk("%02x ", dev->st_buf[i]);
    printk("\n");
    }
    #endif
    @@ -486,7 +488,7 @@ static int ds_wait_status(struct ds_devi
    * can do something with it).
    */
    if (err > 16 || count >= 100 || err < 0)
    - ds_dump_status(dev, buf, err);
    + ds_dump_status(dev, dev->st_buf, err);

    /* Extended data isn't an error. Well, a short is, but the dump
    * would have already told the user that and we can't do anything
    @@ -609,7 +611,6 @@ static int ds_write_byte(struct ds_devic
    {
    int err;
    struct ds_status st;
    - u8 rbyte;

    err = ds_send_control(dev, COMM_BYTE_IO | COMM_IM | dev->spu_bit, byte);
    if (err)
    @@ -622,11 +623,11 @@ static int ds_write_byte(struct ds_devic
    if (err)
    return err;

    - err = ds_recv_data(dev, &rbyte, sizeof(rbyte));
    + err = ds_recv_data(dev, &dev->byte_buf, 1);
    if (err < 0)
    return err;

    - return !(byte == rbyte);
    + return !(byte == dev->byte_buf);
    }

    static int ds_read_byte(struct ds_device *dev, u8 *byte)
    @@ -713,7 +714,6 @@ static void ds9490r_search(void *data, s
    int err;
    u16 value, index;
    struct ds_status st;
    - u8 st_buf[ST_SIZE];
    int search_limit;
    int found = 0;
    int i;
    @@ -725,7 +725,12 @@ static void ds9490r_search(void *data, s
    /* FIFO 128 bytes, bulk packet size 64, read a multiple of the
    * packet size.
    */
    - u64 buf[2*64/8];
    + const size_t bufsize = 2 * 64;
    + u64 *buf;
    +
    + buf = kmalloc(bufsize, GFP_KERNEL);
    + if (!buf)
    + return;

    mutex_lock(&master->bus_mutex);

    @@ -746,10 +751,9 @@ static void ds9490r_search(void *data, s
    do {
    schedule_timeout(jtime);

    - if (ds_recv_status_nodump(dev, &st, st_buf, sizeof(st_buf)) <
    - sizeof(st)) {
    + err = ds_recv_status(dev, &st, false);
    + if (err < 0 || err < sizeof(st))
    break;
    - }

    if (st.data_in_buffer_status) {
    /* Bulk in can receive partial ids, but when it does
    @@ -759,7 +763,7 @@ static void ds9490r_search(void *data, s
    * bulk without first checking if status says there
    * is data to read.
    */
    - err = ds_recv_data(dev, (u8 *)buf, sizeof(buf));
    + err = ds_recv_data(dev, (u8 *)buf, bufsize);
    if (err < 0)
    break;
    for (i = 0; i < err/8; ++i) {
    @@ -795,9 +799,14 @@ static void ds9490r_search(void *data, s
    }
    search_out:
    mutex_unlock(&master->bus_mutex);
    + kfree(buf);
    }

    #if 0
    +/*
    + * FIXME: if this disabled code is ever used in the future all ds_send_data()
    + * calls must be changed to use a DMAable buffer.
    + */
    static int ds_match_access(struct ds_device *dev, u64 init)
    {
    int err;
    @@ -846,13 +855,12 @@ static int ds_set_path(struct ds_device

    static u8 ds9490r_touch_bit(void *data, u8 bit)
    {
    - u8 ret;
    struct ds_device *dev = data;

    - if (ds_touch_bit(dev, bit, &ret))
    + if (ds_touch_bit(dev, bit, &dev->byte_buf))
    return 0;

    - return ret;
    + return dev->byte_buf;
    }

    #if 0
    @@ -867,13 +875,12 @@ static u8 ds9490r_read_bit(void *data)
    {
    struct ds_device *dev = data;
    int err;
    - u8 bit = 0;

    - err = ds_touch_bit(dev, 1, &bit);
    + err = ds_touch_bit(dev, 1, &dev->byte_buf);
    if (err)
    return 0;

    - return bit & 1;
    + return dev->byte_buf & 1;
    }
    #endif

    @@ -888,32 +895,52 @@ static u8 ds9490r_read_byte(void *data)
    {
    struct ds_device *dev = data;
    int err;
    - u8 byte = 0;

    - err = ds_read_byte(dev, &byte);
    + err = ds_read_byte(dev, &dev->byte_buf);
    if (err)
    return 0;

    - return byte;
    + return dev->byte_buf;
    }

    static void ds9490r_write_block(void *data, const u8 *buf, int len)
    {
    struct ds_device *dev = data;
    + u8 *tbuf;
    +
    + if (len <= 0)
    + return;
    +
    + tbuf = kmalloc(len, GFP_KERNEL);
    + if (!tbuf)
    + return;
    +
    + memcpy(tbuf, buf, len);
    + ds_write_block(dev, tbuf, len);

    - ds_write_block(dev, (u8 *)buf, len);
    + kfree(tbuf);
    }

    static u8 ds9490r_read_block(void *data, u8 *buf, int len)
    {
    struct ds_device *dev = data;
    int err;
    + u8 *tbuf;

    - err = ds_read_block(dev, buf, len);
    - if (err < 0)
    + if (len <= 0)
    + return 0;
    +
    + tbuf = kmalloc(len, GFP_KERNEL);
    + if (!tbuf)
    return 0;

    - return len;
    + err = ds_read_block(dev, tbuf, len);
    + if (err >= 0)
    + memcpy(buf, tbuf, len);
    +
    + kfree(tbuf);
    +
    + return err >= 0 ? len : 0;
    }

    static u8 ds9490r_reset(void *data)
    \
     
     \ /
      Last update: 2017-06-01 18:31    [W:4.137 / U:0.208 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site