Re: [PATCH 02/13] staging: r8188eu: Remove pointless "alignment" entry in recv_frame
On Sat, 2014-02-15 at 06:54:06 +0800, Larry Finger wrote:
> This alignment entry in union recv_frame does nothing. It certainly
> dues not ensure alignment.
^
typo here.
I accidently sent this as a private mail before, better to resend to
the list.
>
> Suggested-by: jes.soren...@redhat.com
> Signed-off-by: Larry Finger
> Cc: jes.soren...@redhat.com
> ---
> drivers/staging/rtl8188eu/include/rtw_recv.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8188eu/include/rtw_recv.h
> b/drivers/staging/rtl8188eu/include/rtw_recv.h
> index 866c9e4..c6d7a65 100644
> --- a/drivers/staging/rtl8188eu/include/rtw_recv.h
> +++ b/drivers/staging/rtl8188eu/include/rtw_recv.h
> @@ -292,7 +292,6 @@ struct recv_frame_hdr {
> union recv_frame {
> union {
> struct recv_frame_hdr hdr;
> - uint mem[RECVFRAME_HDR_ALIGN>>2];
> } u;
> };
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH V6 1/1] Drivers: hv: Implement the file copy service
> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Saturday, February 15, 2014 12:08 PM
> To: KY Srinivasan
> Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org;
> o...@aepfle.de;
> a...@canonical.com; jasow...@redhat.com
> Subject: Re: [PATCH V6 1/1] Drivers: hv: Implement the file copy service
>
> On Wed, Feb 12, 2014 at 10:14:03AM -0800, K. Y. Srinivasan wrote:
> > Implement the file copy service for Linux guests on Hyper-V. This permits
> > the
> > host to copy a file (over VMBUS) into the guest. This facility is part of
> > "guest integration services" supported on the Windows platform.
> > Here is a link that provides additional details on this functionality:
> >
> > http://technet.microsoft.com/en-us/library/dn464282.aspx
> >
> > In V1 version of the patch I have addressed comments from
> > Olaf Hering and Dan Carpenter
>
> >
> > In V2 version of this patch I did some minor cleanup (making some globals
> > static). In V4 version of the patch I have addressed all of Olaf's
> > most recent set of comments/concerns.
> >
> > In V5 version of the patch I had addressed Greg's most recent comments.
> > I would like to thank Greg for suggesting that I use misc device; it has
> > significantly simplified the code.
> >
> > In this version of the patch I have cleaned up error message based on Olaf's
> > comments. I have also rebased the patch based on the current tip.
> >
> > Signed-off-by: K. Y. Srinivasan
> > ---
> > drivers/hv/Makefile |2 +-
> > drivers/hv/hv_fcopy.c | 388
> +++
> > drivers/hv/hv_util.c| 10 +
> > include/linux/hyperv.h | 16 ++-
> > include/uapi/linux/hyperv.h | 46 +
> > tools/hv/hv_fcopy_daemon.c | 195 ++
> > 6 files changed, 655 insertions(+), 2 deletions(-)
> > create mode 100644 drivers/hv/hv_fcopy.c
> > create mode 100644 tools/hv/hv_fcopy_daemon.c
> >
> > diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
> > index 0a74b56..5e4dfa4 100644
> > --- a/drivers/hv/Makefile
> > +++ b/drivers/hv/Makefile
> > @@ -5,4 +5,4 @@ obj-$(CONFIG_HYPERV_BALLOON)+= hv_balloon.o
> > hv_vmbus-y := vmbus_drv.o \
> > hv.o connection.o channel.o \
> > channel_mgmt.o ring_buffer.o
> > -hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o
> > +hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o hv_fcopy.o
> > diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
> > new file mode 100644
> > index 000..15cf35c
> > --- /dev/null
> > +++ b/drivers/hv/hv_fcopy.c
> > @@ -0,0 +1,388 @@
> > +/*
> > + * An implementation of file copy service.
> > + *
> > + * Copyright (C) 2014, Microsoft, Inc.
> > + *
> > + * Author : K. Y. Srinivasan
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License version 2 as published
> > + * by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE
> or
> > + * NON INFRINGEMENT. See the GNU General Public License for more
> > + * details.
> > + *
> > + */
> > +
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#define WIN8_SRV_MAJOR 1
> > +#define WIN8_SRV_MINOR 1
> > +#define WIN8_SRV_VERSION (WIN8_SRV_MAJOR << 16 |
> WIN8_SRV_MINOR)
> > +
> > +/*
> > + * Global state maintained for transaction that is being processed.
> > + * For a class of integration services, including the "file copy service",
> > + * the specified protocol is a "request/response" protocol which means that
> > + * there can only be single outstanding transaction from the host at any
> > + * given point in time. We use this to simplify memory management in this
> > + * driver - we cache and process only one message at a time.
> > + *
> > + * While the request/response protocol is guaranteed by the host, we
> > further
> > + * ensure this by serializing packet processing in this driver - we do not
> > + * read additional packets from the VMBUs until the current packet is fully
> > + * handled.
> > + *
> > + * The transaction "active" state is set when we receive a request from the
> > + * host and we cleanup this state when the transaction is completed - when
> we
> > + * respond to the host with our response. When the transaction active
> > state is
> > + * set, we defer handling incoming packets.
> > + */
> > +
> > +static struct {
> > + bool active; /* transaction status - active or not */
> > + int recv_len; /* number of bytes received. */
> > + struct hv_fcopy_hdr *fcopy_msg; /* current message */
> > + struct hv_start_fcopy message; /* sen
Re: [PATCH 2/3] staging: r8188eu: delete unnecessary field initialization
On Sat, Feb 15, 2014 at 08:36:12AM +0100, Julia Lawall wrote: > From: Julia Lawall > > On success, the function netdev_alloc_skb initializes the dev field of its > result to its first argument, so this doesn't have to be done in the > calling context. > > The semantic patch that fixes this problem is as follows: > (http://coccinelle.lip6.fr/) > > // > @@ > expression skb,privn,e; > @@ > > skb = netdev_alloc_skb(privn,...); > ... when strict > ( > -skb->dev = privn; > | > ?skb = e > ) > // > > Signed-off-by: Julia Lawall > > --- > drivers/staging/rtl8188eu/hal/usb_ops_linux.c |1 - > 1 file changed, 1 deletion(-) No longer applies to my tree :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 10/13] staging: r8188eu: Remove wrapper _enter_critical_mutex()
On Sat, Feb 15, 2014 at 12:38:11PM -0800, Greg KH wrote: > I've applied this series up to here. Nevermind, the others applied, so I've merged them, just not this patch. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 10/13] staging: r8188eu: Remove wrapper _enter_critical_mutex()
On Fri, Feb 14, 2014 at 04:54:14PM -0600, Larry Finger wrote:
> This wrapper returned the result of mutex_lock_interruptible(); however,
> none of the callers checked the returned value.
Then why is the code logic changing?
>
> As a result of a gcc upgrade to version 4.8.1, two false unitialized variable
> warnings appeared. To silence the warnings, they are initialized to 0.
>
> Signed-off-by: Larry Finger
> ---
> drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 3 ++-
> drivers/staging/rtl8188eu/hal/usb_ops_linux.c | 8 +---
> drivers/staging/rtl8188eu/include/osdep_service.h | 9 -
> drivers/staging/rtl8188eu/os_dep/os_intfs.c | 3 ++-
> 4 files changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> index c197b22..e95a1ba 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> @@ -4429,7 +4429,8 @@ s32 dump_mgntframe_and_wait_ack(struct adapter
> *padapter, struct xmit_frame *pmg
> if (padapter->bSurpriseRemoved || padapter->bDriverStopped)
> return -1;
>
> - _enter_critical_mutex(&pxmitpriv->ack_tx_mutex, NULL);
> + if (mutex_lock_interruptible(&pxmitpriv->ack_tx_mutex))
> + return -1;
That's not what the original code did, so this change could cause
problems, right?
> -static inline int _enter_critical_mutex(struct mutex *pmutex,
> - unsigned long *pirqL)
> -{
> - int ret;
> -
> - ret = mutex_lock_interruptible(pmutex);
> - return ret;
> -}
This didn't affect the caller function's code path...
I've applied this series up to here.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V6 1/1] Drivers: hv: Implement the file copy service
On Wed, Feb 12, 2014 at 10:14:03AM -0800, K. Y. Srinivasan wrote:
> Implement the file copy service for Linux guests on Hyper-V. This permits the
> host to copy a file (over VMBUS) into the guest. This facility is part of
> "guest integration services" supported on the Windows platform.
> Here is a link that provides additional details on this functionality:
>
> http://technet.microsoft.com/en-us/library/dn464282.aspx
>
> In V1 version of the patch I have addressed comments from
> Olaf Hering and Dan Carpenter
>
> In V2 version of this patch I did some minor cleanup (making some globals
> static). In V4 version of the patch I have addressed all of Olaf's
> most recent set of comments/concerns.
>
> In V5 version of the patch I had addressed Greg's most recent comments.
> I would like to thank Greg for suggesting that I use misc device; it has
> significantly simplified the code.
>
> In this version of the patch I have cleaned up error message based on Olaf's
> comments. I have also rebased the patch based on the current tip.
>
> Signed-off-by: K. Y. Srinivasan
> ---
> drivers/hv/Makefile |2 +-
> drivers/hv/hv_fcopy.c | 388
> +++
> drivers/hv/hv_util.c| 10 +
> include/linux/hyperv.h | 16 ++-
> include/uapi/linux/hyperv.h | 46 +
> tools/hv/hv_fcopy_daemon.c | 195 ++
> 6 files changed, 655 insertions(+), 2 deletions(-)
> create mode 100644 drivers/hv/hv_fcopy.c
> create mode 100644 tools/hv/hv_fcopy_daemon.c
>
> diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
> index 0a74b56..5e4dfa4 100644
> --- a/drivers/hv/Makefile
> +++ b/drivers/hv/Makefile
> @@ -5,4 +5,4 @@ obj-$(CONFIG_HYPERV_BALLOON) += hv_balloon.o
> hv_vmbus-y := vmbus_drv.o \
>hv.o connection.o channel.o \
>channel_mgmt.o ring_buffer.o
> -hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o
> +hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o hv_fcopy.o
> diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
> new file mode 100644
> index 000..15cf35c
> --- /dev/null
> +++ b/drivers/hv/hv_fcopy.c
> @@ -0,0 +1,388 @@
> +/*
> + * An implementation of file copy service.
> + *
> + * Copyright (C) 2014, Microsoft, Inc.
> + *
> + * Author : K. Y. Srinivasan
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> + * by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> + * NON INFRINGEMENT. See the GNU General Public License for more
> + * details.
> + *
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define WIN8_SRV_MAJOR 1
> +#define WIN8_SRV_MINOR 1
> +#define WIN8_SRV_VERSION (WIN8_SRV_MAJOR << 16 | WIN8_SRV_MINOR)
> +
> +/*
> + * Global state maintained for transaction that is being processed.
> + * For a class of integration services, including the "file copy service",
> + * the specified protocol is a "request/response" protocol which means that
> + * there can only be single outstanding transaction from the host at any
> + * given point in time. We use this to simplify memory management in this
> + * driver - we cache and process only one message at a time.
> + *
> + * While the request/response protocol is guaranteed by the host, we further
> + * ensure this by serializing packet processing in this driver - we do not
> + * read additional packets from the VMBUs until the current packet is fully
> + * handled.
> + *
> + * The transaction "active" state is set when we receive a request from the
> + * host and we cleanup this state when the transaction is completed - when we
> + * respond to the host with our response. When the transaction active state
> is
> + * set, we defer handling incoming packets.
> + */
> +
> +static struct {
> + bool active; /* transaction status - active or not */
> + int recv_len; /* number of bytes received. */
> + struct hv_fcopy_hdr *fcopy_msg; /* current message */
> + struct hv_start_fcopy message; /* sent to daemon */
> + struct vmbus_channel *recv_channel; /* chn we got the request */
> + u64 recv_req_id; /* request ID. */
> + void *fcopy_context; /* for the channel callback */
> + struct semaphore read_sema;
> +} fcopy_transaction;
> +
> +static bool opened; /* currently device opened */
> +
> +/*
> + * Before we can accept copy messages from the host, we need
> + * to handshake with the user level daemon. This state tracks
> + * if we are in the handshake phase.
> + */
> +static bool in_hand_shake = true;
> +static void fcopy_send_data(void);
>
Re: [PATCH 10/13] staging: r8188eu: Remove wrapper _enter_critical_mutex()
On 02/15/2014 04:41 AM, Dan Carpenter wrote: On Fri, Feb 14, 2014 at 04:54:14PM -0600, Larry Finger wrote: diff --git a/drivers/staging/rtl8188eu/hal/usb_ops_linux.c b/drivers/staging/rtl8188eu/hal/usb_ops_linux.c index 1fa5370..d5f6a32 100644 --- a/drivers/staging/rtl8188eu/hal/usb_ops_linux.c +++ b/drivers/staging/rtl8188eu/hal/usb_ops_linux.c @@ -49,7 +49,9 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u8 request, u16 value, u goto exit; } - _enter_critical_mutex(&dvobjpriv->usb_vendor_req_mutex, NULL); + if (mutex_lock_interruptible(&dvobjpriv->usb_vendor_req_mutex)) + status = -ENOMEM; + goto exit; Missing curly braces. Thanks for seeing this. Fixing this also removes the uninitialized variable warnings, which should have been a clue. Perhaps my wife is right, and I am getting senile! Actually a simple "return -ENOMEM" is sufficient as nothing happens at label exit other than "return status". Larry ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: davinci_vpfe: fix error check
On Sat, Feb 15, 2014 at 11:17:11AM +0100, Levente Kurusa wrote:
> The check would check the pointer, which is never less than 0.
> According to the error message, the correct check would be
> to check the return value of ipipe_mode. Check that instead.
>
> Reported-by: David Binderman
> Signed-off-by: Levente Kurusa
Reviewed-by: Josh Triplett
> drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
> b/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
> index 2d36b60..b2daf5e 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
> @@ -267,7 +267,7 @@ int config_ipipe_hw(struct vpfe_ipipe_device *ipipe)
> }
>
> ipipe_mode = get_ipipe_mode(ipipe);
> - if (ipipe < 0) {
> + if (ipipe_mode < 0) {
> pr_err("Failed to get ipipe mode");
> return -EINVAL;
> }
> --
> 1.8.3.1
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: davinci_vpfe: fix error check
The check would check the pointer, which is never less than 0.
According to the error message, the correct check would be
to check the return value of ipipe_mode. Check that instead.
Reported-by: David Binderman
Signed-off-by: Levente Kurusa
---
drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
b/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
index 2d36b60..b2daf5e 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
@@ -267,7 +267,7 @@ int config_ipipe_hw(struct vpfe_ipipe_device *ipipe)
}
ipipe_mode = get_ipipe_mode(ipipe);
- if (ipipe < 0) {
+ if (ipipe_mode < 0) {
pr_err("Failed to get ipipe mode");
return -EINVAL;
}
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] [RFC] staging: rtl8821ae: fix invalid bit mask on MSR_AP check
On 02/15/2014 11:36 AM, Dan Carpenter wrote: > On Sat, Feb 15, 2014 at 08:53:34AM +0100, Levente Kurusa wrote: >> Thanks Dan, maybe you know some people who could test it? >> RTLXX guys? Or maybe we can take the patch as is? >> I cannot really think of any other solution other than removing >> the other clause, but since that was written to the file, >> there must have been some logic behind that. I am slightly >> disappointed get_maintainer didn't really find anybody for >> this patch... > > This driver is going to be deleted in the next couple weeks and replaced > with a re-written from scratch driver. > Oh, I did not know that. Since this fix isn't that big, I think it won't get into the next -rc, and hence might not hit Linus' tree before the new driver gets into staging. I am not sure if it is worth applying if that is the case. -- Regards, Levente Kurusa ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 10/13] staging: r8188eu: Remove wrapper _enter_critical_mutex()
On Fri, Feb 14, 2014 at 04:54:14PM -0600, Larry Finger wrote: > diff --git a/drivers/staging/rtl8188eu/hal/usb_ops_linux.c > b/drivers/staging/rtl8188eu/hal/usb_ops_linux.c > index 1fa5370..d5f6a32 100644 > --- a/drivers/staging/rtl8188eu/hal/usb_ops_linux.c > +++ b/drivers/staging/rtl8188eu/hal/usb_ops_linux.c > @@ -49,7 +49,9 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u8 > request, u16 value, u > goto exit; > } > > - _enter_critical_mutex(&dvobjpriv->usb_vendor_req_mutex, NULL); > + if (mutex_lock_interruptible(&dvobjpriv->usb_vendor_req_mutex)) > + status = -ENOMEM; > + goto exit; > Missing curly braces. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rts5208: fix always true condition
On Sat, Feb 15, 2014 at 10:44:35AM +0100, Levente Kurusa wrote: > ANDing anything with 0x1E and expecting it to be not 0x03 will always > be true. AND instead with 0x03, so that we check the last two bits, > which should be what was intended there. > This one is less clear what the intent was. Could easily be that the 0x3 is wrong. Sometimes it's better to leave the warning rather than guess at the fix. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] [RFC] staging: rtl8821ae: fix invalid bit mask on MSR_AP check
On Sat, Feb 15, 2014 at 08:53:34AM +0100, Levente Kurusa wrote: > Thanks Dan, maybe you know some people who could test it? > RTLXX guys? Or maybe we can take the patch as is? > I cannot really think of any other solution other than removing > the other clause, but since that was written to the file, > there must have been some logic behind that. I am slightly > disappointed get_maintainer didn't really find anybody for > this patch... This driver is going to be deleted in the next couple weeks and replaced with a re-written from scratch driver. Your fix is correct. It's just that no one tested the AP code for this. I would feel comfortable applying it as-is if no one is available to test it. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rts5208: fix always true condition
ANDing anything with 0x1E and expecting it to be not 0x03 will always
be true. AND instead with 0x03, so that we check the last two bits,
which should be what was intended there.
Reported-by: David Binderman
Signed-off-by: Levente Kurusa
---
drivers/staging/rts5208/sd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/rts5208/sd.c b/drivers/staging/rts5208/sd.c
index c7c1f54..03548e5 100644
--- a/drivers/staging/rts5208/sd.c
+++ b/drivers/staging/rts5208/sd.c
@@ -3512,7 +3512,7 @@ RTY_SEND_CMD:
TRACE_RET(chip, STATUS_FAIL);
} else if (rsp_type == SD_RSP_TYPE_R0) {
- if ((ptr[3] & 0x1E) != 0x03)
+ if ((ptr[3] & 0x03) != 0x03)
TRACE_RET(chip, STATUS_FAIL);
}
}
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
