The newer usb_control_msg_{send|recv}() API are an improvement on the
existing usb_control_msg() as it ensures that a short read/write is treated
as an error, data can be used off the stack, and raw usb pipes need not be
created in the calling functions.
For this reason, the instance of usb_control_msg() has been replaced with
usb_control_msg_send() appropriately.
Signed-off-by: Anant Thazhemadam <[email protected]>
---
drivers/usb/misc/emi62.c | 30 +++++++-----------------------
1 file changed, 7 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/misc/emi62.c b/drivers/usb/misc/emi62.c
index 3eea60437f56..6ac4e72d53e0 100644
--- a/drivers/usb/misc/emi62.c
+++ b/drivers/usb/misc/emi62.c
@@ -36,7 +36,7 @@
#define INTERNAL_RAM(address) (address <= MAX_INTERNAL_ADDRESS)
static int emi62_writememory(struct usb_device *dev, int address,
- const unsigned char *data, int length,
+ const void *data, int length,
__u8 bRequest);
static int emi62_set_reset(struct usb_device *dev, unsigned char reset_bit);
static int emi62_load_firmware (struct usb_device *dev);
@@ -45,21 +45,11 @@ static void emi62_disconnect(struct usb_interface *intf);
/* thanks to drivers/usb/serial/keyspan_pda.c code */
static int emi62_writememory(struct usb_device *dev, int address,
- const unsigned char *data, int length,
+ const void *data, int length,
__u8 request)
{
- int result;
- unsigned char *buffer = kmemdup(data, length, GFP_KERNEL);
-
- if (!buffer) {
- dev_err(&dev->dev, "kmalloc(%d) failed.\n", length);
- return -ENOMEM;
- }
- /* Note: usb_control_msg returns negative value on error or length of the
- * data that was written! */
- result = usb_control_msg (dev, usb_sndctrlpipe(dev, 0), request, 0x40, address, 0, buffer, length, 300);
- kfree (buffer);
- return result;
+ return usb_control_msg_send(dev, 0, request, 0x40, address,
+ 0, data, length, 300, GFP_KERNEL);
}
/* thanks to drivers/usb/serial/keyspan_pda.c code */
@@ -85,12 +75,9 @@ static int emi62_load_firmware (struct usb_device *dev)
int err = -ENOMEM;
int i;
__u32 addr; /* Address to write */
- __u8 *buf;
+ __u8 buf[FW_LOAD_SIZE];
dev_dbg(&dev->dev, "load_firmware\n");
- buf = kmalloc(FW_LOAD_SIZE, GFP_KERNEL);
- if (!buf)
- goto wraperr;
err = request_ihex_firmware(&loader_fw, "emi62/loader.fw", &dev->dev);
if (err)
@@ -140,11 +127,11 @@ static int emi62_load_firmware (struct usb_device *dev)
/* intel hex records are terminated with type 0 element */
while (rec && (i + be16_to_cpu(rec->len) < FW_LOAD_SIZE)) {
- memcpy(buf + i, rec->data, be16_to_cpu(rec->len));
+ memcpy(&buf[i], rec->data, be16_to_cpu(rec->len));
i += be16_to_cpu(rec->len);
rec = ihex_next_binrec(rec);
}
- err = emi62_writememory(dev, addr, buf, i, ANCHOR_LOAD_FPGA);
+ err = emi62_writememory(dev, addr, &buf, i, ANCHOR_LOAD_FPGA);
if (err < 0)
goto wraperr;
} while (rec);
@@ -209,8 +196,6 @@ static int emi62_load_firmware (struct usb_device *dev)
release_firmware(bitstream_fw);
release_firmware(firmware_fw);
- kfree(buf);
-
/* return 1 to fail the driver inialization
* and give real driver change to load */
return 1;
@@ -223,7 +208,6 @@ static int emi62_load_firmware (struct usb_device *dev)
release_firmware(bitstream_fw);
release_firmware(firmware_fw);
- kfree(buf);
dev_err(&dev->dev, "Error\n");
return err;
}
--
2.25.1
Hi Anant,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on balbi-usb/testing/next peter.chen-usb/ci-for-usb-next v5.10-rc6 next-20201127]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Anant-Thazhemadam/drivers-usb-misc-update-to-use-usb_control_msg_-send-recv/20201130-093816
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: h8300-randconfig-s032-20201130 (attached as .config)
compiler: h8300-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.3-170-g3bc348f6-dirty
# https://github.com/0day-ci/linux/commit/a9e2333efa48de6856185ec35c82b659ff1c1215
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Anant-Thazhemadam/drivers-usb-misc-update-to-use-usb_control_msg_-send-recv/20201130-093816
git checkout a9e2333efa48de6856185ec35c82b659ff1c1215
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=h8300
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
drivers/usb/misc/emi62.c: In function 'emi62_load_firmware':
>> drivers/usb/misc/emi62.c:213:1: warning: the frame size of 1048 bytes is larger than 1024 bytes [-Wframe-larger-than=]
213 | }
| ^
vim +213 drivers/usb/misc/emi62.c
^1da177e4c3f415 Linus Torvalds 2005-04-16 68
^1da177e4c3f415 Linus Torvalds 2005-04-16 69 static int emi62_load_firmware (struct usb_device *dev)
^1da177e4c3f415 Linus Torvalds 2005-04-16 70 {
b8e24bfabb03527 David Woodhouse 2008-05-30 71 const struct firmware *loader_fw = NULL;
b8e24bfabb03527 David Woodhouse 2008-05-30 72 const struct firmware *bitstream_fw = NULL;
b8e24bfabb03527 David Woodhouse 2008-05-30 73 const struct firmware *firmware_fw = NULL;
b8e24bfabb03527 David Woodhouse 2008-05-30 74 const struct ihex_binrec *rec;
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20 75 int err = -ENOMEM;
^1da177e4c3f415 Linus Torvalds 2005-04-16 76 int i;
^1da177e4c3f415 Linus Torvalds 2005-04-16 77 __u32 addr; /* Address to write */
a9e2333efa48de6 Anant Thazhemadam 2020-11-30 78 __u8 buf[FW_LOAD_SIZE];
^1da177e4c3f415 Linus Torvalds 2005-04-16 79
^1da177e4c3f415 Linus Torvalds 2005-04-16 80 dev_dbg(&dev->dev, "load_firmware\n");
^1da177e4c3f415 Linus Torvalds 2005-04-16 81
b8e24bfabb03527 David Woodhouse 2008-05-30 82 err = request_ihex_firmware(&loader_fw, "emi62/loader.fw", &dev->dev);
b8e24bfabb03527 David Woodhouse 2008-05-30 83 if (err)
b8e24bfabb03527 David Woodhouse 2008-05-30 84 goto nofw;
b8e24bfabb03527 David Woodhouse 2008-05-30 85
b8e24bfabb03527 David Woodhouse 2008-05-30 86 err = request_ihex_firmware(&bitstream_fw, "emi62/bitstream.fw",
b8e24bfabb03527 David Woodhouse 2008-05-30 87 &dev->dev);
b8e24bfabb03527 David Woodhouse 2008-05-30 88 if (err)
b8e24bfabb03527 David Woodhouse 2008-05-30 89 goto nofw;
b8e24bfabb03527 David Woodhouse 2008-05-30 90
b8e24bfabb03527 David Woodhouse 2008-05-30 91 err = request_ihex_firmware(&firmware_fw, FIRMWARE_FW, &dev->dev);
b8e24bfabb03527 David Woodhouse 2008-05-30 92 if (err) {
b8e24bfabb03527 David Woodhouse 2008-05-30 93 nofw:
b8e24bfabb03527 David Woodhouse 2008-05-30 94 goto wraperr;
b8e24bfabb03527 David Woodhouse 2008-05-30 95 }
b8e24bfabb03527 David Woodhouse 2008-05-30 96
^1da177e4c3f415 Linus Torvalds 2005-04-16 97 /* Assert reset (stop the CPU in the EMI) */
^1da177e4c3f415 Linus Torvalds 2005-04-16 98 err = emi62_set_reset(dev,1);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20 99 if (err < 0)
^1da177e4c3f415 Linus Torvalds 2005-04-16 100 goto wraperr;
^1da177e4c3f415 Linus Torvalds 2005-04-16 101
b8e24bfabb03527 David Woodhouse 2008-05-30 102 rec = (const struct ihex_binrec *)loader_fw->data;
b8e24bfabb03527 David Woodhouse 2008-05-30 103
^1da177e4c3f415 Linus Torvalds 2005-04-16 104 /* 1. We need to put the loader for the FPGA into the EZ-USB */
b8e24bfabb03527 David Woodhouse 2008-05-30 105 while (rec) {
b8e24bfabb03527 David Woodhouse 2008-05-30 106 err = emi62_writememory(dev, be32_to_cpu(rec->addr),
b8e24bfabb03527 David Woodhouse 2008-05-30 107 rec->data, be16_to_cpu(rec->len),
b8e24bfabb03527 David Woodhouse 2008-05-30 108 ANCHOR_LOAD_INTERNAL);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20 109 if (err < 0)
^1da177e4c3f415 Linus Torvalds 2005-04-16 110 goto wraperr;
b8e24bfabb03527 David Woodhouse 2008-05-30 111 rec = ihex_next_binrec(rec);
^1da177e4c3f415 Linus Torvalds 2005-04-16 112 }
^1da177e4c3f415 Linus Torvalds 2005-04-16 113
^1da177e4c3f415 Linus Torvalds 2005-04-16 114 /* De-assert reset (let the CPU run) */
^1da177e4c3f415 Linus Torvalds 2005-04-16 115 err = emi62_set_reset(dev,0);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20 116 if (err < 0)
5919a43bbc649f4 Oliver Neukum 2007-10-25 117 goto wraperr;
16c23f7d88cbcce Monty 2006-05-09 118 msleep(250); /* let device settle */
^1da177e4c3f415 Linus Torvalds 2005-04-16 119
^1da177e4c3f415 Linus Torvalds 2005-04-16 120 /* 2. We upload the FPGA firmware into the EMI
^1da177e4c3f415 Linus Torvalds 2005-04-16 121 * Note: collect up to 1023 (yes!) bytes and send them with
^1da177e4c3f415 Linus Torvalds 2005-04-16 122 * a single request. This is _much_ faster! */
b8e24bfabb03527 David Woodhouse 2008-05-30 123 rec = (const struct ihex_binrec *)bitstream_fw->data;
^1da177e4c3f415 Linus Torvalds 2005-04-16 124 do {
^1da177e4c3f415 Linus Torvalds 2005-04-16 125 i = 0;
b8e24bfabb03527 David Woodhouse 2008-05-30 126 addr = be32_to_cpu(rec->addr);
^1da177e4c3f415 Linus Torvalds 2005-04-16 127
^1da177e4c3f415 Linus Torvalds 2005-04-16 128 /* intel hex records are terminated with type 0 element */
b8e24bfabb03527 David Woodhouse 2008-05-30 129 while (rec && (i + be16_to_cpu(rec->len) < FW_LOAD_SIZE)) {
a9e2333efa48de6 Anant Thazhemadam 2020-11-30 130 memcpy(&buf[i], rec->data, be16_to_cpu(rec->len));
b8e24bfabb03527 David Woodhouse 2008-05-30 131 i += be16_to_cpu(rec->len);
b8e24bfabb03527 David Woodhouse 2008-05-30 132 rec = ihex_next_binrec(rec);
^1da177e4c3f415 Linus Torvalds 2005-04-16 133 }
a9e2333efa48de6 Anant Thazhemadam 2020-11-30 134 err = emi62_writememory(dev, addr, &buf, i, ANCHOR_LOAD_FPGA);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20 135 if (err < 0)
^1da177e4c3f415 Linus Torvalds 2005-04-16 136 goto wraperr;
ac06c06770bb876 Clemens Ladisch 2009-12-21 137 } while (rec);
^1da177e4c3f415 Linus Torvalds 2005-04-16 138
^1da177e4c3f415 Linus Torvalds 2005-04-16 139 /* Assert reset (stop the CPU in the EMI) */
^1da177e4c3f415 Linus Torvalds 2005-04-16 140 err = emi62_set_reset(dev,1);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20 141 if (err < 0)
^1da177e4c3f415 Linus Torvalds 2005-04-16 142 goto wraperr;
^1da177e4c3f415 Linus Torvalds 2005-04-16 143
^1da177e4c3f415 Linus Torvalds 2005-04-16 144 /* 3. We need to put the loader for the firmware into the EZ-USB (again...) */
b8e24bfabb03527 David Woodhouse 2008-05-30 145 for (rec = (const struct ihex_binrec *)loader_fw->data;
b8e24bfabb03527 David Woodhouse 2008-05-30 146 rec; rec = ihex_next_binrec(rec)) {
b8e24bfabb03527 David Woodhouse 2008-05-30 147 err = emi62_writememory(dev, be32_to_cpu(rec->addr),
b8e24bfabb03527 David Woodhouse 2008-05-30 148 rec->data, be16_to_cpu(rec->len),
b8e24bfabb03527 David Woodhouse 2008-05-30 149 ANCHOR_LOAD_INTERNAL);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20 150 if (err < 0)
^1da177e4c3f415 Linus Torvalds 2005-04-16 151 goto wraperr;
^1da177e4c3f415 Linus Torvalds 2005-04-16 152 }
^1da177e4c3f415 Linus Torvalds 2005-04-16 153
^1da177e4c3f415 Linus Torvalds 2005-04-16 154 /* De-assert reset (let the CPU run) */
^1da177e4c3f415 Linus Torvalds 2005-04-16 155 err = emi62_set_reset(dev,0);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20 156 if (err < 0)
^1da177e4c3f415 Linus Torvalds 2005-04-16 157 goto wraperr;
16c23f7d88cbcce Monty 2006-05-09 158 msleep(250); /* let device settle */
^1da177e4c3f415 Linus Torvalds 2005-04-16 159
^1da177e4c3f415 Linus Torvalds 2005-04-16 160 /* 4. We put the part of the firmware that lies in the external RAM into the EZ-USB */
^1da177e4c3f415 Linus Torvalds 2005-04-16 161
b8e24bfabb03527 David Woodhouse 2008-05-30 162 for (rec = (const struct ihex_binrec *)firmware_fw->data;
b8e24bfabb03527 David Woodhouse 2008-05-30 163 rec; rec = ihex_next_binrec(rec)) {
b8e24bfabb03527 David Woodhouse 2008-05-30 164 if (!INTERNAL_RAM(be32_to_cpu(rec->addr))) {
b8e24bfabb03527 David Woodhouse 2008-05-30 165 err = emi62_writememory(dev, be32_to_cpu(rec->addr),
b8e24bfabb03527 David Woodhouse 2008-05-30 166 rec->data, be16_to_cpu(rec->len),
b8e24bfabb03527 David Woodhouse 2008-05-30 167 ANCHOR_LOAD_EXTERNAL);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20 168 if (err < 0)
^1da177e4c3f415 Linus Torvalds 2005-04-16 169 goto wraperr;
^1da177e4c3f415 Linus Torvalds 2005-04-16 170 }
^1da177e4c3f415 Linus Torvalds 2005-04-16 171 }
b8e24bfabb03527 David Woodhouse 2008-05-30 172
^1da177e4c3f415 Linus Torvalds 2005-04-16 173 /* Assert reset (stop the CPU in the EMI) */
^1da177e4c3f415 Linus Torvalds 2005-04-16 174 err = emi62_set_reset(dev,1);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20 175 if (err < 0)
^1da177e4c3f415 Linus Torvalds 2005-04-16 176 goto wraperr;
^1da177e4c3f415 Linus Torvalds 2005-04-16 177
b8e24bfabb03527 David Woodhouse 2008-05-30 178 for (rec = (const struct ihex_binrec *)firmware_fw->data;
b8e24bfabb03527 David Woodhouse 2008-05-30 179 rec; rec = ihex_next_binrec(rec)) {
b8e24bfabb03527 David Woodhouse 2008-05-30 180 if (INTERNAL_RAM(be32_to_cpu(rec->addr))) {
b8e24bfabb03527 David Woodhouse 2008-05-30 181 err = emi62_writememory(dev, be32_to_cpu(rec->addr),
b8e24bfabb03527 David Woodhouse 2008-05-30 182 rec->data, be16_to_cpu(rec->len),
b8e24bfabb03527 David Woodhouse 2008-05-30 183 ANCHOR_LOAD_EXTERNAL);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20 184 if (err < 0)
^1da177e4c3f415 Linus Torvalds 2005-04-16 185 goto wraperr;
^1da177e4c3f415 Linus Torvalds 2005-04-16 186 }
^1da177e4c3f415 Linus Torvalds 2005-04-16 187 }
^1da177e4c3f415 Linus Torvalds 2005-04-16 188
^1da177e4c3f415 Linus Torvalds 2005-04-16 189 /* De-assert reset (let the CPU run) */
^1da177e4c3f415 Linus Torvalds 2005-04-16 190 err = emi62_set_reset(dev,0);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20 191 if (err < 0)
^1da177e4c3f415 Linus Torvalds 2005-04-16 192 goto wraperr;
16c23f7d88cbcce Monty 2006-05-09 193 msleep(250); /* let device settle */
^1da177e4c3f415 Linus Torvalds 2005-04-16 194
b8e24bfabb03527 David Woodhouse 2008-05-30 195 release_firmware(loader_fw);
b8e24bfabb03527 David Woodhouse 2008-05-30 196 release_firmware(bitstream_fw);
b8e24bfabb03527 David Woodhouse 2008-05-30 197 release_firmware(firmware_fw);
b8e24bfabb03527 David Woodhouse 2008-05-30 198
^1da177e4c3f415 Linus Torvalds 2005-04-16 199 /* return 1 to fail the driver inialization
^1da177e4c3f415 Linus Torvalds 2005-04-16 200 * and give real driver change to load */
^1da177e4c3f415 Linus Torvalds 2005-04-16 201 return 1;
^1da177e4c3f415 Linus Torvalds 2005-04-16 202
^1da177e4c3f415 Linus Torvalds 2005-04-16 203 wraperr:
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20 204 if (err < 0)
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20 205 dev_err(&dev->dev,"%s - error loading firmware: error = %d\n",
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20 206 __func__, err);
b8e24bfabb03527 David Woodhouse 2008-05-30 207 release_firmware(loader_fw);
b8e24bfabb03527 David Woodhouse 2008-05-30 208 release_firmware(bitstream_fw);
b8e24bfabb03527 David Woodhouse 2008-05-30 209 release_firmware(firmware_fw);
b8e24bfabb03527 David Woodhouse 2008-05-30 210
^1da177e4c3f415 Linus Torvalds 2005-04-16 211 dev_err(&dev->dev, "Error\n");
^1da177e4c3f415 Linus Torvalds 2005-04-16 212 return err;
^1da177e4c3f415 Linus Torvalds 2005-04-16 @213 }
^1da177e4c3f415 Linus Torvalds 2005-04-16 214
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On Mon, Nov 30, 2020 at 06:59:25AM +0530, Anant Thazhemadam wrote:
> The newer usb_control_msg_{send|recv}() API are an improvement on the
> existing usb_control_msg() as it ensures that a short read/write is treated
> as an error, data can be used off the stack, and raw usb pipes need not be
> created in the calling functions.
> For this reason, the instance of usb_control_msg() has been replaced with
> usb_control_msg_send() appropriately.
>
> Signed-off-by: Anant Thazhemadam <[email protected]>
> ---
This driver looks like it shares origins with the emi26 one so the same
comments apply (and especially not put large structures on the stack).
Johan