[PATCH 1/1 linux-next] zd1201: replace kmalloc/memset by kzalloc

[Fabian Frederick]

Signed-off-by: Fabian Frederick <***@skynet.be>
---
drivers/net/wireless/zd1201.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireless/zd1201.c b/drivers/net/wireless/zd1201.c
index 6f5c793..4630b26 100644
--- a/drivers/net/wireless/zd1201.c
+++ b/drivers/net/wireless/zd1201.c
@@ -525,7 +525,7 @@ static int zd1201_setconfig(struct zd1201 *zd, int rid, void *buf, int len, int
zd->rxdatas = 0;
zd->rxlen = 0;
for (seq=0; len > 0; seq++) {
- request = kmalloc(16, gfp_mask);
+ request = kzalloc(16, gfp_mask);
if (!request)
return -ENOMEM;
urb = usb_alloc_urb(0, gfp_mask);
@@ -533,7 +533,6 @@ static int zd1201_setconfig(struct zd1201 *zd, int rid, void *buf, int len, int
kfree(request);
return -ENOMEM;
}
- memset(request, 0, 16);
reqlen = len>12 ? 12 : len;
request[0] = ZD1201_USB_RESREQ;
request[1] = seq;

[Joe Perches]

It might be clearer to use a structure for this 16 byte thing.

There's a comment bit in the code:

/* cmdreq message:
u32 type
u16 cmd
u16 parm0
u16 parm1
u16 parm2
u8 pad[4]

total: 4 + 2 + 2 + 2 + 2 + 4 = 16
*/

It seems thought that this first u32 is actually
a series of u8s.

Maybe:

struct zd1201_cmdreq {
u8 type;
u8 seq;
u8 a;
u8 b;
__le16 cmd;
__le16 parm0;
__le16 parm1;
__le16 parm2;
u8 pad[4];
};

And does this really need to be alloc'd?

maybe

struct zd1202_cmdreq request = {};

etc...

[Bjørn Mork]

Post by Joe Perches
And does this really need to be alloc'd?

Yes, it does. It is used as a transfer_buffer in usb_fill_bulk_urb() an=
d
must be "suitable for DMA". From include/linux/usb.h:

/**
* struct urb - USB Request Block
=2E.
* @transfer_buffer: This identifies the buffer to (or from) which the=
I/O
* request will be performed unless URB_NO_TRANSFER_DMA_MAP is set
* (however, do not leave garbage in transfer_buffer even then).
* This buffer must be suitable for DMA; allocate it with
* kmalloc() or equivalent. For transfers to "in" endpoints, cont=
ents
* of this buffer will be modified. This buffer is used for the d=
ata
* stage of control transfers.



Bj=C3=B8rn

[Fabian Frederick]

Post by Joe Perches
And does this really need to be alloc'd?

Yes, it does. It is used as a transfer_buffer in usb_fill_bulk_urb() =

and

/**
=C2=A0 * struct urb - USB Request Block
..

m) which the I/O

=C2=A0 *=C2=A0 =C2=A0 =C2=A0 request will be performed unless URB_NO_T=

RANSFER_DMA_MAP is set

=C2=A0 *=C2=A0 =C2=A0 =C2=A0 (however, do not leave garbage in transfe=

r_buffer even then).

=C2=A0 *=C2=A0 =C2=A0 =C2=A0 This buffer must be suitable for DMA; all=

ocate it with

=C2=A0 *=C2=A0 =C2=A0 =C2=A0 kmalloc() or equivalent.=C2=A0 For transf=

ers to "in" endpoints, contents

=C2=A0 *=C2=A0 =C2=A0 =C2=A0 of this buffer will be modified.=C2=A0 Th=

is buffer is used for the data

=C2=A0 *=C2=A0 =C2=A0 =C2=A0 stage of control transfers.

Adding a structure would be a nice design update indeed but this would =
require
some testing...Separate patch maybe ?
=C2=A0
=46abian

Bj=C3=B8rn

[Joe Perches]

Adding a structure would be a nice design update indeed but this would require
some testing...Separate patch maybe ?

Of course that'd be fine.