[media] pwc: Use one shared usb command buffer

The pwc driver used to:
1. kmalloc a buffer
2. memcpy data to send over usb there
3. do the usb_control_msg call (which does not work with data on the stack)
4. free the buffer

For every usb command send. This patch changes the code to instead malloc
a buffer for this purpose once and use it everywhere.

[mchehab@redhat.com: Fix a compilation breakage with allyesconfig:
 drivers/media/video/pwc/pwc-ctrl.c: In function ‘pwc_get_cmos_sensor’:
 drivers/media/video/pwc/pwc-ctrl.c:546:3: warning: passing argument 4 of ‘recv_control_msg’ makes integer from pointer without a cast [en$
 drivers/media/video/pwc/pwc-ctrl.c:107:12: note: expected ‘int’ but argument is of type ‘unsigned char *’
 drivers/media/video/pwc/pwc-ctrl.c:546:3: error: too many arguments to function ‘recv_control_msg’
 drivers/media/video/pwc/pwc-ctrl.c:107:12: note: declared here]

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/video/pwc/pwc-ctrl.c b/drivers/media/video/pwc/pwc-ctrl.c
index 9c1fb3f07dee75630ba529542175fcfec1774738..1f506fde97d0a52433a7477f6ac843cc60af1f2f 100644 (file)
--- a/drivers/media/video/pwc/pwc-ctrl.c
+++ b/drivers/media/video/pwc/pwc-ctrl.c
@@ -104,47 +104,16 @@ static struct Nala_table_entry Nala_table[PSZ_MAX][PWC_FPS_MAX_NALA] =
 
 /****************************************************************************/
 
-static int _send_control_msg(struct pwc_device *pdev,
-       u8 request, u16 value, int index, void *buf, int buflen)
-{
-       int rc;
-       void *kbuf = NULL;
-
-       if (buflen) {
-               kbuf = kmemdup(buf, buflen, GFP_KERNEL); /* not allowed on stack */
-               if (kbuf == NULL)
-                       return -ENOMEM;
-       }
-
-       rc = usb_control_msg(pdev->udev, usb_sndctrlpipe(pdev->udev, 0),
-               request,
-               USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-               value,
-               index,
-               kbuf, buflen, USB_CTRL_SET_TIMEOUT);
-
-       kfree(kbuf);
-       return rc;
-}
-
 static int recv_control_msg(struct pwc_device *pdev,
-       u8 request, u16 value, void *buf, int buflen)
+       u8 request, u16 value, int recv_count)
 {
        int rc;
-       void *kbuf = kmalloc(buflen, GFP_KERNEL); /* not allowed on stack */
-
-       if (kbuf == NULL)
-               return -ENOMEM;
 
        rc = usb_control_msg(pdev->udev, usb_rcvctrlpipe(pdev->udev, 0),
                request,
                USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-               value,
-               pdev->vcinterface,
-               kbuf, buflen, USB_CTRL_GET_TIMEOUT);
-       memcpy(buf, kbuf, buflen);
-       kfree(kbuf);
-
+               value, pdev->vcinterface,
+               pdev->ctrl_buf, recv_count, USB_CTRL_GET_TIMEOUT);
        if (rc < 0)
                PWC_ERROR("recv_control_msg error %d req %02x val %04x\n",
                          rc, request, value);
@@ -152,26 +121,38 @@ static int recv_control_msg(struct pwc_device *pdev,
 }
 
 static inline int send_video_command(struct pwc_device *pdev,
-       int index, void *buf, int buflen)
+       int index, const unsigned char *buf, int buflen)
 {
-       return _send_control_msg(pdev,
-               SET_EP_STREAM_CTL,
-               VIDEO_OUTPUT_CONTROL_FORMATTER,
-               index,
-               buf, buflen);
+       int rc;
+
+       memcpy(pdev->ctrl_buf, buf, buflen);
+
+       rc = usb_control_msg(pdev->udev, usb_sndctrlpipe(pdev->udev, 0),
+                       SET_EP_STREAM_CTL,
+                       USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+                       VIDEO_OUTPUT_CONTROL_FORMATTER, index,
+                       pdev->ctrl_buf, buflen, USB_CTRL_SET_TIMEOUT);
+       if (rc >= 0)
+               memcpy(pdev->cmd_buf, buf, buflen);
+       else
+               PWC_ERROR("send_video_command error %d\n", rc);
+
+       return rc;
 }
 
 int send_control_msg(struct pwc_device *pdev,
        u8 request, u16 value, void *buf, int buflen)
 {
-       return _send_control_msg(pdev,
-               request, value, pdev->vcinterface, buf, buflen);
+       return usb_control_msg(pdev->udev, usb_sndctrlpipe(pdev->udev, 0),
+                       request,
+                       USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+                       value, pdev->vcinterface,
+                       buf, buflen, USB_CTRL_SET_TIMEOUT);
 }
 
 static int set_video_mode_Nala(struct pwc_device *pdev, int size, int pixfmt,
                               int frames, int *compression, int send_to_cam)
 {
-       unsigned char buf[3];
        int fps, ret = 0;
        struct Nala_table_entry *pEntry;
        int frames2frames[31] =
@@ -206,18 +187,14 @@ static int set_video_mode_Nala(struct pwc_device *pdev, int size, int pixfmt,
        if (pEntry->alternate == 0)
                return -EINVAL;
 
-       memcpy(buf, pEntry->mode, 3);
        if (send_to_cam)
-               ret = send_video_command(pdev, pdev->vendpoint, buf, 3);
-       if (ret < 0) {
-               PWC_DEBUG_MODULE("Failed to send video command... %d\n", ret);
+               ret = send_video_command(pdev, pdev->vendpoint,
+                                        pEntry->mode, 3);
+       if (ret < 0)
                return ret;
-       }
-       if (pEntry->compressed && pixfmt == V4L2_PIX_FMT_YUV420)
-               pwc_dec1_init(pdev, buf);
 
-       pdev->cmd_len = 3;
-       memcpy(pdev->cmd_buf, buf, 3);
+       if (pEntry->compressed && pixfmt == V4L2_PIX_FMT_YUV420)
+               pwc_dec1_init(pdev, pEntry->mode);
 
        /* Set various parameters */
        pdev->pixfmt = pixfmt;
@@ -249,7 +226,6 @@ static int set_video_mode_Nala(struct pwc_device *pdev, int size, int pixfmt,
 static int set_video_mode_Timon(struct pwc_device *pdev, int size, int pixfmt,
                                int frames, int *compression, int send_to_cam)
 {
-       unsigned char buf[13];
        const struct Timon_table_entry *pChoose;
        int fps, ret = 0;
 
@@ -274,17 +250,14 @@ static int set_video_mode_Timon(struct pwc_device *pdev, int size, int pixfmt,
        if (pChoose == NULL || pChoose->alternate == 0)
                return -ENOENT; /* Not supported. */
 
-       memcpy(buf, pChoose->mode, 13);
        if (send_to_cam)
-               ret = send_video_command(pdev, pdev->vendpoint, buf, 13);
+               ret = send_video_command(pdev, pdev->vendpoint,
+                                        pChoose->mode, 13);
        if (ret < 0)
                return ret;
 
        if (pChoose->bandlength > 0 && pixfmt == V4L2_PIX_FMT_YUV420)
-               pwc_dec23_init(pdev, buf);
-
-       pdev->cmd_len = 13;
-       memcpy(pdev->cmd_buf, buf, 13);
+               pwc_dec23_init(pdev, pChoose->mode);
 
        /* Set various parameters */
        pdev->pixfmt = pixfmt;
@@ -306,7 +279,6 @@ static int set_video_mode_Kiara(struct pwc_device *pdev, int size, int pixfmt,
 {
        const struct Kiara_table_entry *pChoose = NULL;
        int fps, ret = 0;
-       unsigned char buf[12];
 
        if (size >= PSZ_MAX || *compression < 0 || *compression > 3)
                return -EINVAL;
@@ -328,22 +300,15 @@ static int set_video_mode_Kiara(struct pwc_device *pdev, int size, int pixfmt,
        if (pChoose == NULL || pChoose->alternate == 0)
                return -ENOENT; /* Not supported. */
 
-       PWC_TRACE("Using alternate setting %d.\n", pChoose->alternate);
-
-       /* usb_control_msg won't take staticly allocated arrays as argument?? */
-       memcpy(buf, pChoose->mode, 12);
-
        /* Firmware bug: video endpoint is 5, but commands are sent to endpoint 4 */
        if (send_to_cam)
-               ret = send_video_command(pdev, 4, buf, 12);
+               ret = send_video_command(pdev, 4, pChoose->mode, 12);
        if (ret < 0)
                return ret;
 
        if (pChoose->bandlength > 0 && pixfmt == V4L2_PIX_FMT_YUV420)
-               pwc_dec23_init(pdev, buf);
+               pwc_dec23_init(pdev, pChoose->mode);
 
-       pdev->cmd_len = 12;
-       memcpy(pdev->cmd_buf, buf, 12);
        /* All set and go */
        pdev->pixfmt = pixfmt;
        pdev->vframes = (fps + 1) * 5;
@@ -445,13 +410,12 @@ unsigned int pwc_get_fps(struct pwc_device *pdev, unsigned int index, unsigned i
 int pwc_get_u8_ctrl(struct pwc_device *pdev, u8 request, u16 value, int *data)
 {
        int ret;
-       u8 buf;
 
-       ret = recv_control_msg(pdev, request, value, &buf, sizeof(buf));
+       ret = recv_control_msg(pdev, request, value, 1);
        if (ret < 0)
                return ret;
 
-       *data = buf;
+       *data = pdev->ctrl_buf[0];
        return 0;
 }
 
@@ -459,7 +423,8 @@ int pwc_set_u8_ctrl(struct pwc_device *pdev, u8 request, u16 value, u8 data)
 {
        int ret;
 
-       ret = send_control_msg(pdev, request, value, &data, sizeof(data));
+       pdev->ctrl_buf[0] = data;
+       ret = send_control_msg(pdev, request, value, pdev->ctrl_buf, 1);
        if (ret < 0)
                return ret;
 
@@ -469,37 +434,34 @@ int pwc_set_u8_ctrl(struct pwc_device *pdev, u8 request, u16 value, u8 data)
 int pwc_get_s8_ctrl(struct pwc_device *pdev, u8 request, u16 value, int *data)
 {
        int ret;
-       s8 buf;
 
-       ret = recv_control_msg(pdev, request, value, &buf, sizeof(buf));
+       ret = recv_control_msg(pdev, request, value, 1);
        if (ret < 0)
                return ret;
 
-       *data = buf;
+       *data = ((s8 *)pdev->ctrl_buf)[0];
        return 0;
 }
 
 int pwc_get_u16_ctrl(struct pwc_device *pdev, u8 request, u16 value, int *data)
 {
        int ret;
-       u8 buf[2];
 
-       ret = recv_control_msg(pdev, request, value, buf, sizeof(buf));
+       ret = recv_control_msg(pdev, request, value, 2);
        if (ret < 0)
                return ret;
 
-       *data = (buf[1] << 8) | buf[0];
+       *data = (pdev->ctrl_buf[1] << 8) | pdev->ctrl_buf[0];
        return 0;
 }
 
 int pwc_set_u16_ctrl(struct pwc_device *pdev, u8 request, u16 value, u16 data)
 {
        int ret;
-       u8 buf[2];
 
-       buf[0] = data & 0xff;
-       buf[1] = data >> 8;
-       ret = send_control_msg(pdev, request, value, buf, sizeof(buf));
+       pdev->ctrl_buf[0] = data & 0xff;
+       pdev->ctrl_buf[1] = data >> 8;
+       ret = send_control_msg(pdev, request, value, pdev->ctrl_buf, 2);
        if (ret < 0)
                return ret;
 
@@ -520,7 +482,6 @@ int pwc_button_ctrl(struct pwc_device *pdev, u16 value)
 /* POWER */
 void pwc_camera_power(struct pwc_device *pdev, int power)
 {
-       char buf;
        int r;
 
        if (!pdev->power_save)
@@ -530,13 +491,11 @@ void pwc_camera_power(struct pwc_device *pdev, int power)
                return; /* Not supported by Nala or Timon < release 6 */
 
        if (power)
-               buf = 0x00; /* active */
+               pdev->ctrl_buf[0] = 0x00; /* active */
        else
-               buf = 0xFF; /* power save */
-       r = send_control_msg(pdev,
-               SET_STATUS_CTL, SET_POWER_SAVE_MODE_FORMATTER,
-               &buf, sizeof(buf));
-
+               pdev->ctrl_buf[0] = 0xFF; /* power save */
+       r = send_control_msg(pdev, SET_STATUS_CTL,
+               SET_POWER_SAVE_MODE_FORMATTER, pdev->ctrl_buf, 1);
        if (r < 0)
                PWC_ERROR("Failed to power %s camera (%d)\n",
                          power ? "on" : "off", r);
@@ -544,7 +503,6 @@ void pwc_camera_power(struct pwc_device *pdev, int power)
 
 int pwc_set_leds(struct pwc_device *pdev, int on_value, int off_value)
 {
-       unsigned char buf[2];
        int r;
 
        if (pdev->type < 730)
@@ -560,11 +518,11 @@ int pwc_set_leds(struct pwc_device *pdev, int on_value, int off_value)
        if (off_value > 0xff)
                off_value = 0xff;
 
-       buf[0] = on_value;
-       buf[1] = off_value;
+       pdev->ctrl_buf[0] = on_value;
+       pdev->ctrl_buf[1] = off_value;
 
        r = send_control_msg(pdev,
-               SET_STATUS_CTL, LED_FORMATTER, &buf, sizeof(buf));
+               SET_STATUS_CTL, LED_FORMATTER, pdev->ctrl_buf, 2);
        if (r < 0)
                PWC_ERROR("Failed to set LED on/off time (%d)\n", r);
 
@@ -574,7 +532,6 @@ int pwc_set_leds(struct pwc_device *pdev, int on_value, int off_value)
 #ifdef CONFIG_USB_PWC_DEBUG
 int pwc_get_cmos_sensor(struct pwc_device *pdev, int *sensor)
 {
-       unsigned char buf;
        int ret = -1, request;
 
        if (pdev->type < 675)
@@ -584,14 +541,13 @@ int pwc_get_cmos_sensor(struct pwc_device *pdev, int *sensor)
        else
                request = SENSOR_TYPE_FORMATTER2;
 
-       ret = recv_control_msg(pdev,
-               GET_STATUS_CTL, request, &buf, sizeof(buf));
+       ret = recv_control_msg(pdev, GET_STATUS_CTL, request, 1);
        if (ret < 0)
                return ret;
        if (pdev->type < 675)
-               *sensor = buf | 0x100;
+               *sensor = pdev->ctrl_buf[0] | 0x100;
        else
-               *sensor = buf;
+               *sensor = pdev->ctrl_buf[0];
        return 0;
 }
 #endif

diff --git a/drivers/media/video/pwc/pwc-dec1.c b/drivers/media/video/pwc/pwc-dec1.c
index bac0d83fe11909b58b82f7d2e094c8dea7f006c4..e899036aadf4a4734e0dd611386bf47909fe34bb 100644 (file)
--- a/drivers/media/video/pwc/pwc-dec1.c
+++ b/drivers/media/video/pwc/pwc-dec1.c
@@ -24,7 +24,7 @@
 */
 #include "pwc.h"
 
-void pwc_dec1_init(struct pwc_device *pdev, void *buffer)
+void pwc_dec1_init(struct pwc_device *pdev, const unsigned char *cmd)
 {
        struct pwc_dec1_private *pdec = &pdev->dec1;
 

diff --git a/drivers/media/video/pwc/pwc-dec1.h b/drivers/media/video/pwc/pwc-dec1.h
index 6e8f3c561c9bdd55af41771bff3c20161564f5ef..c565ef8f52fb3a83e29f5c428744aaf354b16db6 100644 (file)
--- a/drivers/media/video/pwc/pwc-dec1.h
+++ b/drivers/media/video/pwc/pwc-dec1.h
@@ -34,6 +34,6 @@ struct pwc_dec1_private
        int version;
 };
 
-void pwc_dec1_init(struct pwc_device *pdev, void *buffer);
+void pwc_dec1_init(struct pwc_device *pdev, const unsigned char *cmd);
 
 #endif

diff --git a/drivers/media/video/pwc/pwc-dec23.c b/drivers/media/video/pwc/pwc-dec23.c
index 98772efc5bdefbf0467e673c75ac374bdc179003..3792fedff9515e85734200d8ffb22aa1ed15bb93 100644 (file)
--- a/drivers/media/video/pwc/pwc-dec23.c
+++ b/drivers/media/video/pwc/pwc-dec23.c
@@ -294,7 +294,7 @@ static unsigned char pwc_crop_table[256 + 2*MAX_OUTER_CROP_VALUE];
 
 
 /* If the type or the command change, we rebuild the lookup table */
-void pwc_dec23_init(struct pwc_device *pdev, unsigned char *cmd)
+void pwc_dec23_init(struct pwc_device *pdev, const unsigned char *cmd)
 {
        int flags, version, shift, i;
        struct pwc_dec23_private *pdec = &pdev->dec23;

diff --git a/drivers/media/video/pwc/pwc-dec23.h b/drivers/media/video/pwc/pwc-dec23.h
index af31d6047bb200e3e24e9a30bbce717ea6dfa8b0..c655b1c1e6a9f7d7c7e8ff725672190bb2829e0c 100644 (file)
--- a/drivers/media/video/pwc/pwc-dec23.h
+++ b/drivers/media/video/pwc/pwc-dec23.h
@@ -54,7 +54,7 @@ struct pwc_dec23_private
 
 };
 
-void pwc_dec23_init(struct pwc_device *pdev, unsigned char *cmd);
+void pwc_dec23_init(struct pwc_device *pdev, const unsigned char *cmd);
 void pwc_dec23_decompress(struct pwc_device *pdev,
                          const void *src,
                          void *dst);

diff --git a/drivers/media/video/pwc/pwc-if.c b/drivers/media/video/pwc/pwc-if.c
index 23eaceea4862030bcbfa4f47ed35c914caf88003..a07df4e4aa04238b1923c78fe174fbfe743538b2 100644 (file)
--- a/drivers/media/video/pwc/pwc-if.c
+++ b/drivers/media/video/pwc/pwc-if.c
@@ -605,6 +605,7 @@ static void pwc_video_release(struct v4l2_device *v)
 
        v4l2_ctrl_handler_free(&pdev->ctrl_handler);
 
+       kfree(pdev->ctrl_buf);
        kfree(pdev);
 }
 
@@ -1115,6 +1116,14 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id
        if (hint < MAX_DEV_HINTS)
                device_hint[hint].pdev = pdev;
 
+       /* Allocate USB command buffers */
+       pdev->ctrl_buf = kmalloc(sizeof(pdev->cmd_buf), GFP_KERNEL);
+       if (!pdev->ctrl_buf) {
+               PWC_ERROR("Oops, could not allocate memory for pwc_device.\n");
+               rc = -ENOMEM;
+               goto err_free_mem;
+       }
+
 #ifdef CONFIG_USB_PWC_DEBUG
        /* Query sensor type */
        if (pwc_get_cmos_sensor(pdev, &rc) >= 0) {
@@ -1199,6 +1208,7 @@ err_free_controls:
 err_free_mem:
        if (hint < MAX_DEV_HINTS)
                device_hint[hint].pdev = NULL;
+       kfree(pdev->ctrl_buf);
        kfree(pdev);
        return rc;
 }

diff --git a/drivers/media/video/pwc/pwc-v4l.c b/drivers/media/video/pwc/pwc-v4l.c
index 46feece3885258494a0cabb6371d89a3524d9558..f495eeb5403aaff31dd66ba582a07d86297752b9 100644 (file)
--- a/drivers/media/video/pwc/pwc-v4l.c
+++ b/drivers/media/video/pwc/pwc-v4l.c
@@ -772,33 +772,33 @@ static int pwc_set_autogain_expo(struct pwc_device *pdev)
 static int pwc_set_motor(struct pwc_device *pdev)
 {
        int ret;
-       u8 buf[4];
 
-       buf[0] = 0;
+       pdev->ctrl_buf[0] = 0;
        if (pdev->motor_pan_reset->is_new)
-               buf[0] |= 0x01;
+               pdev->ctrl_buf[0] |= 0x01;
        if (pdev->motor_tilt_reset->is_new)
-               buf[0] |= 0x02;
+               pdev->ctrl_buf[0] |= 0x02;
        if (pdev->motor_pan_reset->is_new || pdev->motor_tilt_reset->is_new) {
                ret = send_control_msg(pdev, SET_MPT_CTL,
-                                      PT_RESET_CONTROL_FORMATTER, buf, 1);
+                                      PT_RESET_CONTROL_FORMATTER,
+                                      pdev->ctrl_buf, 1);
                if (ret < 0)
                        return ret;
        }
 
-       memset(buf, 0, sizeof(buf));
+       memset(pdev->ctrl_buf, 0, 4);
        if (pdev->motor_pan->is_new) {
-               buf[0] = pdev->motor_pan->val & 0xFF;
-               buf[1] = (pdev->motor_pan->val >> 8);
+               pdev->ctrl_buf[0] = pdev->motor_pan->val & 0xFF;
+               pdev->ctrl_buf[1] = (pdev->motor_pan->val >> 8);
        }
        if (pdev->motor_tilt->is_new) {
-               buf[2] = pdev->motor_tilt->val & 0xFF;
-               buf[3] = (pdev->motor_tilt->val >> 8);
+               pdev->ctrl_buf[2] = pdev->motor_tilt->val & 0xFF;
+               pdev->ctrl_buf[3] = (pdev->motor_tilt->val >> 8);
        }
        if (pdev->motor_pan->is_new || pdev->motor_tilt->is_new) {
                ret = send_control_msg(pdev, SET_MPT_CTL,
                                       PT_RELATIVE_CONTROL_FORMATTER,
-                                      buf, sizeof(buf));
+                                      pdev->ctrl_buf, 4);
                if (ret < 0)
                        return ret;
        }

diff --git a/drivers/media/video/pwc/pwc.h b/drivers/media/video/pwc/pwc.h
index f441999e5bd19c164a12e9be0c1ec86a866fd920..e4d4d711dd1f4df7182d20bcc3591fda9e32f628 100644 (file)
--- a/drivers/media/video/pwc/pwc.h
+++ b/drivers/media/video/pwc/pwc.h
@@ -134,9 +134,6 @@
 #define DEVICE_USE_CODEC3(x) ((x)>=700)
 #define DEVICE_USE_CODEC23(x) ((x)>=675)
 
-/* from pwc-dec.h */
-#define PWCX_FLAG_PLANAR        0x0001
-
 /* Request types: video */
 #define SET_LUM_CTL                    0x01
 #define GET_LUM_CTL                    0x02
@@ -250,8 +247,8 @@ struct pwc_device
        char vmirror;           /* for ToUCaM series */
        char power_save;        /* Do powersaving for this cam */
 
-       int cmd_len;
        unsigned char cmd_buf[13];
+       unsigned char *ctrl_buf;
 
        struct urb *urbs[MAX_ISO_BUFS];
        char iso_init;