RE: [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in the signalling logic with kick_q
> -Original Message- > From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On > Behalf Of KY Srinivasan > Sent: Wednesday, March 11, 2015 8:32 PM > To: Jason Wang > Cc: o...@aepfle.de; net...@vger.kernel.org; linux-ker...@vger.kernel.org; > gre...@linuxfoundation.org; a...@canonical.com; > de...@linuxdriverproject.org; da...@davemloft.net > Subject: RE: [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in the > signalling logic with kick_q > > > > > -Original Message- > > From: Jason Wang [mailto:jasow...@redhat.com] > > Sent: Wednesday, March 11, 2015 8:08 PM > > To: KY Srinivasan > > Cc: da...@davemloft.net; net...@vger.kernel.org; linux- > > ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de; > > a...@canonical.com; gre...@linuxfoundation.org; KY Srinivasan > > Subject: Re: [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in > > the signalling logic with kick_q > > > > > > > > On Thu, Mar 12, 2015 at 3:04 AM, K. Y. Srinivasan > > wrote: > > > When the caller specifies that signalling should be deferred, we > > > need to address the case where we are not able to place the current > > > packet because the buffer is full. In this case, we will signal the > > > host as some packets may have been placed on the ring buffer. > > > I would like to thank Jason Wang for pointing > > > out this issue. > > > > > > Signed-off-by: K. Y. Srinivasan > > > --- > > > drivers/hv/channel.c | 32 > > > 1 files changed, 32 insertions(+), 0 deletions(-) > > > > > > diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index > > > e58cdb7..ae06ba9 100644 > > > --- a/drivers/hv/channel.c > > > +++ b/drivers/hv/channel.c > > > @@ -614,8 +614,24 @@ int vmbus_sendpacket_ctl(struct > vmbus_channel > > > *channel, void *buffer, > > > > > > ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, > > > &signal); > > > > > > + /* > > > + * Here is the logic for signalling the host: > > > + * 1. If the host is already draining the ringbuffer, > > > + *don't signal. This is indicated by the parameter > > > + *"signal". > > > + * > > > + * 2. If we are not able to write, signal if kick_q is false. > > > + *kick_q being false indicates that we may have placed zero or > > > + *more packets with more packets to come. We will signal in > > > + *this case even if potentially we may have not placed any > > > + *packet. This is a rare enough condition that it should not > > > + *matter. > > > + */ > > > + > > > if ((ret == 0) && kick_q && signal) > > > vmbus_setevent(channel); > > > + else if ((ret != 0) && !kick_q) > > > + vmbus_setevent(channel); > > > > > > return ret; > > > } > > > @@ -705,8 +721,24 @@ int vmbus_sendpacket_pagebuffer_ctl(struct > > > vmbus_channel *channel, > > > > > > ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, > > > &signal); > > > > > > + /* > > > + * Here is the logic for signalling the host: > > > + * 1. If the host is already draining the ringbuffer, > > > + *don't signal. This is indicated by the parameter > > > + *"signal". > > > + * > > > + * 2. If we are not able to write, signal if kick_q is false. > > > + *kick_q being false indicates that we may have placed zero or > > > + *more packets with more packets to come. We will signal in > > > + *this case even if potentially we may have not placed any > > > + *packet. This is a rare enough condition that it should not > > > + *matter. > > > + */ > > > + > > > if ((ret == 0) && kick_q && signal) > > > vmbus_setevent(channel); > > > + else if ((ret != 0) && !kick_q) > > > + vmbus_setevent(channel); > > > > > > return ret; > > > } > > > -- > > > > Looks like we need to kick unconditionally here. Consider we may get - > > EAGAIN when we want to send the last skb (kick_q is true) from the > > list. We need kick host in this case. > > > > Btw, another method is let the driver to decide e.g exporting the > > vmbus_setevent() and call it in netvsc_start_xmit(). > > There is other state that governs if the host needs to be signaled and I don't > think we want to expose that to netvsc. In any case, netvsc will have to > signal > the host when EGAIN is received. > We might as well do it in the vmbus driver. > > Thank you Jason, I will respin and resubmit the series. Looks like there may be a way to make the signaling more precise even in this case. I am going to experiment with it. K. Y > > K. Y > > > > > > > > ___ > devel mailing list > de...@linuxdriverproject.org > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: fsl-mc: Corrected email addresses in TODO file
Signed-off-by: J. German Rivera --- drivers/staging/fsl-mc/TODO | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/staging/fsl-mc/TODO b/drivers/staging/fsl-mc/TODO index 49ebfd9..d78288b 100644 --- a/drivers/staging/fsl-mc/TODO +++ b/drivers/staging/fsl-mc/TODO @@ -8,5 +8,6 @@ * Add at least one device driver for a DPAA2 object (child device of the fsl-mc bus). -Please send any patches to Greg Kroah-Hartman , -de...@driverdev.osuosl.org, linux-ker...@vger.kernel.org +Please send any patches to Greg Kroah-Hartman , +german.riv...@freescale.com, de...@driverdev.osuosl.org, +linux-ker...@vger.kernel.org -- 2.3.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH V2 3/3 net-next] hyperv: Support batched notification
> -Original Message-
> From: Jason Wang [mailto:jasow...@redhat.com]
> Sent: Wednesday, March 11, 2015 8:09 PM
> To: KY Srinivasan
> Cc: da...@davemloft.net; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de;
> a...@canonical.com; gre...@linuxfoundation.org; KY Srinivasan
> Subject: Re: [PATCH V2 3/3 net-next] hyperv: Support batched notification
>
>
>
> On Thu, Mar 12, 2015 at 3:04 AM, K. Y. Srinivasan
> wrote:
> > Optimize notifying the host by deferring notification until there are
> > no more packets to be sent. This will help in batching the requests on
> > the host.
> >
> > Signed-off-by: K. Y. Srinivasan
> > ---
> > drivers/net/hyperv/hyperv_net.h |2 +-
> > drivers/net/hyperv/netvsc.c | 14 +-
> > drivers/net/hyperv/netvsc_drv.c |3 ++-
> > drivers/net/hyperv/rndis_filter.c |2 +-
> > 4 files changed, 13 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/hyperv/hyperv_net.h
> > b/drivers/net/hyperv/hyperv_net.h index 4815843..3fd9896 100644
> > --- a/drivers/net/hyperv/hyperv_net.h
> > +++ b/drivers/net/hyperv/hyperv_net.h
> > @@ -184,7 +184,7 @@ struct rndis_device { int
> > netvsc_device_add(struct hv_device *device, void *additional_info);
> > int netvsc_device_remove(struct hv_device *device); int
> > netvsc_send(struct hv_device *device,
> > - struct hv_netvsc_packet *packet);
> > + struct hv_netvsc_packet *packet, bool kick_q);
> > void netvsc_linkstatus_callback(struct hv_device *device_obj,
> > struct rndis_message *resp);
> > int netvsc_recv_callback(struct hv_device *device_obj, diff --git
> > a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c index
> > 208eb05..9003b94 100644
> > --- a/drivers/net/hyperv/netvsc.c
> > +++ b/drivers/net/hyperv/netvsc.c
> > @@ -707,7 +707,7 @@ static u32 netvsc_copy_to_send_buf(struct
> > netvsc_device *net_device, }
> >
> > int netvsc_send(struct hv_device *device,
> > - struct hv_netvsc_packet *packet)
> > + struct hv_netvsc_packet *packet, bool kick_q)
> > {
> > struct netvsc_device *net_device;
> > int ret = 0;
> > @@ -719,6 +719,7 @@ int netvsc_send(struct hv_device *device,
> > u32 msg_size = 0;
> > struct sk_buff *skb = NULL;
> > u16 q_idx = packet->q_idx;
> > + u32 vmbus_flags =
> VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED;
> >
> >
> > net_device = get_outbound_net_device(device); @@ -768,18
> +769,21 @@
> > int netvsc_send(struct hv_device *device,
> > return -ENODEV;
> >
> > if (packet->page_buf_cnt) {
> > - ret = vmbus_sendpacket_pagebuffer(out_channel,
> > + ret = vmbus_sendpacket_pagebuffer_ctl(out_channel,
> > packet->page_buf,
> > packet->page_buf_cnt,
> > &sendMessage,
> > sizeof(struct
> nvsp_message),
> > - req_id);
> > + req_id,
> > + vmbus_flags,
> > + kick_q);
> > } else {
> > - ret = vmbus_sendpacket(out_channel, &sendMessage,
> > + ret = vmbus_sendpacket_ctl(out_channel, &sendMessage,
> > sizeof(struct nvsp_message),
> > req_id,
> > VM_PKT_DATA_INBAND,
> > -
> VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
> > + vmbus_flags,
> > + kick_q);
> > }
> >
> > if (ret == 0) {
> > diff --git a/drivers/net/hyperv/netvsc_drv.c
> > b/drivers/net/hyperv/netvsc_drv.c index a06bd66..80b4b29 100644
> > --- a/drivers/net/hyperv/netvsc_drv.c
> > +++ b/drivers/net/hyperv/netvsc_drv.c
> > @@ -384,6 +384,7 @@ static int netvsc_start_xmit(struct sk_buff *skb,
> > struct net_device *net)
> > u32 net_trans_info;
> > u32 hash;
> > u32 skb_length = skb->len;
> > + bool kick_q = !skb->xmit_more;
> >
> >
> > /* We will atmost need two pages to describe the rndis @@ -556,7
> > +557,7 @@ do_send:
> > packet->page_buf_cnt = init_page_array(rndis_msg,
> rndis_msg_size,
> > skb, &packet->page_buf[0]);
> >
> > - ret = netvsc_send(net_device_ctx->device_ctx, packet);
> > + ret = netvsc_send(net_device_ctx->device_ctx, packet, kick_q);
>
> Maybe just a !skb->xmit_more here to save a local variable.
Will fix it.
K. Y
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in the signalling logic with kick_q
> -Original Message- > From: Jason Wang [mailto:jasow...@redhat.com] > Sent: Wednesday, March 11, 2015 8:08 PM > To: KY Srinivasan > Cc: da...@davemloft.net; net...@vger.kernel.org; linux- > ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de; > a...@canonical.com; gre...@linuxfoundation.org; KY Srinivasan > Subject: Re: [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in the > signalling logic with kick_q > > > > On Thu, Mar 12, 2015 at 3:04 AM, K. Y. Srinivasan > wrote: > > When the caller specifies that signalling should be deferred, we need > > to address the case where we are not able to place the current packet > > because the buffer is full. In this case, we will signal the host as > > some packets may have been placed on the ring buffer. > > I would like to thank Jason Wang for pointing > > out this issue. > > > > Signed-off-by: K. Y. Srinivasan > > --- > > drivers/hv/channel.c | 32 > > 1 files changed, 32 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index > > e58cdb7..ae06ba9 100644 > > --- a/drivers/hv/channel.c > > +++ b/drivers/hv/channel.c > > @@ -614,8 +614,24 @@ int vmbus_sendpacket_ctl(struct vmbus_channel > > *channel, void *buffer, > > > > ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, > > &signal); > > > > + /* > > +* Here is the logic for signalling the host: > > +* 1. If the host is already draining the ringbuffer, > > +*don't signal. This is indicated by the parameter > > +*"signal". > > +* > > +* 2. If we are not able to write, signal if kick_q is false. > > +*kick_q being false indicates that we may have placed zero or > > +*more packets with more packets to come. We will signal in > > +*this case even if potentially we may have not placed any > > +*packet. This is a rare enough condition that it should not > > +*matter. > > +*/ > > + > > if ((ret == 0) && kick_q && signal) > > vmbus_setevent(channel); > > + else if ((ret != 0) && !kick_q) > > + vmbus_setevent(channel); > > > > return ret; > > } > > @@ -705,8 +721,24 @@ int vmbus_sendpacket_pagebuffer_ctl(struct > > vmbus_channel *channel, > > > > ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, > > &signal); > > > > + /* > > +* Here is the logic for signalling the host: > > +* 1. If the host is already draining the ringbuffer, > > +*don't signal. This is indicated by the parameter > > +*"signal". > > +* > > +* 2. If we are not able to write, signal if kick_q is false. > > +*kick_q being false indicates that we may have placed zero or > > +*more packets with more packets to come. We will signal in > > +*this case even if potentially we may have not placed any > > +*packet. This is a rare enough condition that it should not > > +*matter. > > +*/ > > + > > if ((ret == 0) && kick_q && signal) > > vmbus_setevent(channel); > > + else if ((ret != 0) && !kick_q) > > + vmbus_setevent(channel); > > > > return ret; > > } > > -- > > Looks like we need to kick unconditionally here. Consider we may get - > EAGAIN when we want to send the last skb (kick_q is true) from the list. We > need kick host in this case. > > Btw, another method is let the driver to decide e.g exporting the > vmbus_setevent() and call it in netvsc_start_xmit(). There is other state that governs if the host needs to be signaled and I don't think we want to expose that to netvsc. In any case, netvsc will have to signal the host when EGAIN is received. We might as well do it in the vmbus driver. Thank you Jason, I will respin and resubmit the series. K. Y > > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V2 3/3 net-next] hyperv: Support batched notification
On Thu, Mar 12, 2015 at 3:04 AM, K. Y. Srinivasan
wrote:
Optimize notifying the host by deferring notification until there
are no more packets to be sent. This will help in batching the
requests
on the host.
Signed-off-by: K. Y. Srinivasan
---
drivers/net/hyperv/hyperv_net.h |2 +-
drivers/net/hyperv/netvsc.c | 14 +-
drivers/net/hyperv/netvsc_drv.c |3 ++-
drivers/net/hyperv/rndis_filter.c |2 +-
4 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/net/hyperv/hyperv_net.h
b/drivers/net/hyperv/hyperv_net.h
index 4815843..3fd9896 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -184,7 +184,7 @@ struct rndis_device {
int netvsc_device_add(struct hv_device *device, void
*additional_info);
int netvsc_device_remove(struct hv_device *device);
int netvsc_send(struct hv_device *device,
- struct hv_netvsc_packet *packet);
+ struct hv_netvsc_packet *packet, bool kick_q);
void netvsc_linkstatus_callback(struct hv_device *device_obj,
struct rndis_message *resp);
int netvsc_recv_callback(struct hv_device *device_obj,
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 208eb05..9003b94 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -707,7 +707,7 @@ static u32 netvsc_copy_to_send_buf(struct
netvsc_device *net_device,
}
int netvsc_send(struct hv_device *device,
- struct hv_netvsc_packet *packet)
+ struct hv_netvsc_packet *packet, bool kick_q)
{
struct netvsc_device *net_device;
int ret = 0;
@@ -719,6 +719,7 @@ int netvsc_send(struct hv_device *device,
u32 msg_size = 0;
struct sk_buff *skb = NULL;
u16 q_idx = packet->q_idx;
+ u32 vmbus_flags = VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED;
net_device = get_outbound_net_device(device);
@@ -768,18 +769,21 @@ int netvsc_send(struct hv_device *device,
return -ENODEV;
if (packet->page_buf_cnt) {
- ret = vmbus_sendpacket_pagebuffer(out_channel,
+ ret = vmbus_sendpacket_pagebuffer_ctl(out_channel,
packet->page_buf,
packet->page_buf_cnt,
&sendMessage,
sizeof(struct nvsp_message),
- req_id);
+ req_id,
+ vmbus_flags,
+ kick_q);
} else {
- ret = vmbus_sendpacket(out_channel, &sendMessage,
+ ret = vmbus_sendpacket_ctl(out_channel, &sendMessage,
sizeof(struct nvsp_message),
req_id,
VM_PKT_DATA_INBAND,
- VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
+ vmbus_flags,
+ kick_q);
}
if (ret == 0) {
diff --git a/drivers/net/hyperv/netvsc_drv.c
b/drivers/net/hyperv/netvsc_drv.c
index a06bd66..80b4b29 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -384,6 +384,7 @@ static int netvsc_start_xmit(struct sk_buff *skb,
struct net_device *net)
u32 net_trans_info;
u32 hash;
u32 skb_length = skb->len;
+ bool kick_q = !skb->xmit_more;
/* We will atmost need two pages to describe the rndis
@@ -556,7 +557,7 @@ do_send:
packet->page_buf_cnt = init_page_array(rndis_msg, rndis_msg_size,
skb, &packet->page_buf[0]);
- ret = netvsc_send(net_device_ctx->device_ctx, packet);
+ ret = netvsc_send(net_device_ctx->device_ctx, packet, kick_q);
Maybe just a !skb->xmit_more here to save a local variable.
drop:
if (ret == 0) {
diff --git a/drivers/net/hyperv/rndis_filter.c
b/drivers/net/hyperv/rndis_filter.c
index ca81de0..05f3792 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -238,7 +238,7 @@ static int rndis_filter_send_request(struct
rndis_device *dev,
packet->send_completion = NULL;
- ret = netvsc_send(dev->net_dev->dev, packet);
+ ret = netvsc_send(dev->net_dev->dev, packet, true);
return ret;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in the signalling logic with kick_q
On Thu, Mar 12, 2015 at 3:04 AM, K. Y. Srinivasan wrote: When the caller specifies that signalling should be deferred, we need to address the case where we are not able to place the current packet because the buffer is full. In this case, we will signal the host as some packets may have been placed on the ring buffer. I would like to thank Jason Wang for pointing out this issue. Signed-off-by: K. Y. Srinivasan --- drivers/hv/channel.c | 32 1 files changed, 32 insertions(+), 0 deletions(-) diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index e58cdb7..ae06ba9 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -614,8 +614,24 @@ int vmbus_sendpacket_ctl(struct vmbus_channel *channel, void *buffer, ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, &signal); + /* +* Here is the logic for signalling the host: +* 1. If the host is already draining the ringbuffer, +*don't signal. This is indicated by the parameter +*"signal". +* +* 2. If we are not able to write, signal if kick_q is false. +*kick_q being false indicates that we may have placed zero or +*more packets with more packets to come. We will signal in +*this case even if potentially we may have not placed any +*packet. This is a rare enough condition that it should not +*matter. +*/ + if ((ret == 0) && kick_q && signal) vmbus_setevent(channel); + else if ((ret != 0) && !kick_q) + vmbus_setevent(channel); return ret; } @@ -705,8 +721,24 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel *channel, ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, &signal); + /* +* Here is the logic for signalling the host: +* 1. If the host is already draining the ringbuffer, +*don't signal. This is indicated by the parameter +*"signal". +* +* 2. If we are not able to write, signal if kick_q is false. +*kick_q being false indicates that we may have placed zero or +*more packets with more packets to come. We will signal in +*this case even if potentially we may have not placed any +*packet. This is a rare enough condition that it should not +*matter. +*/ + if ((ret == 0) && kick_q && signal) vmbus_setevent(channel); + else if ((ret != 0) && !kick_q) + vmbus_setevent(channel); return ret; } -- Looks like we need to kick unconditionally here. Consider we may get -EAGAIN when we want to send the last skb (kick_q is true) from the list. We need kick host in this case. Btw, another method is let the driver to decide e.g exporting the vmbus_setevent() and call it in netvsc_start_xmit(). ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V2 1/3 net-next] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl()
On Thu, Mar 12, 2015 at 3:04 AM, K. Y. Srinivasan wrote: Export the vmbus_sendpacket_pagebuffer_ctl() interface. This interface will be used in the netvsc driver to optimize signalling the host. Signed-off-by: K. Y. Srinivasan --- drivers/hv/channel.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index da53180..e58cdb7 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -710,6 +710,7 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel *channel, return ret; } +EXPORT_SYMBOL_GPL(vmbus_sendpacket_pagebuffer_ctl); /* * vmbus_sendpacket_pagebuffer - Send a range of single-page buffer -- 1.7.4.1 Acked-by: Jason Wang ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/6] Drivers: hv: vmbus: Fix a bug in rescind processing in vmbus_close_internal()
When a channel has been rescinded, the close operation is a noop.
Restructure the code so we deal with the rescind condition after
we properly cleanup the channel. I would like to thank
Dexuan Cui for observing this problem.
The current code leaks memory when the channel is rescinded.
The current Linux 4.0 RC3 tree is broken and this patch fixes the problem.
Signed-off-by: K. Y. Srinivasan
---
drivers/hv/channel.c | 15 ++-
1 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index da53180..2c8206d 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -501,15 +501,6 @@ static int vmbus_close_internal(struct vmbus_channel
*channel)
put_cpu();
}
- /*
-* If the channel has been rescinded; process device removal.
-*/
- if (channel->rescind) {
- hv_process_channel_removal(channel,
- channel->offermsg.child_relid);
- return 0;
- }
-
/* Send a closing message */
msg = &channel->close_msg.msg;
@@ -549,6 +540,12 @@ static int vmbus_close_internal(struct vmbus_channel
*channel)
free_pages((unsigned long)channel->ringbuffer_pages,
get_order(channel->ringbuffer_pagecount * PAGE_SIZE));
+ /*
+* If the channel has been rescinded; process device removal.
+*/
+ if (channel->rescind)
+ hv_process_channel_removal(channel,
+ channel->offermsg.child_relid);
return ret;
}
--
1.7.4.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/6] Drivers: hv: hv_balloon: don't lose memory when onlining order is not natural
From: Vitaly Kuznetsov
Memory blocks can be onlined in random order. When this order is not natural
some memory pages are not onlined because of the redundant check in
hv_online_page().
Here is a real world scenario:
1) Host tries to hot-add the following (process_hot_add):
pg_start=rg_start=0x48000, pfn_cnt=111616, rg_size=262144
2) This results in adding 4 memory blocks:
[ 109.057866] init_memory_mapping: [mem 0x4800-0x4fff]
[ 114.102698] init_memory_mapping: [mem 0x5000-0x57ff]
[ 119.168039] init_memory_mapping: [mem 0x5800-0x5fff]
[ 124.233053] init_memory_mapping: [mem 0x6000-0x67ff]
The last one is incomplete but we have special has->covered_end_pfn counter to
avoid onlining non-backed frames and hv_bring_pgs_online() function to bring
them online later on.
3) Now we have 4 offline memory blocks: /sys/devices/system/memory/memory9-12
$ for f in /sys/devices/system/memory/memory*/state; do echo $f `cat $f`; done
| grep -v onlin
/sys/devices/system/memory/memory10/state offline
/sys/devices/system/memory/memory11/state offline
/sys/devices/system/memory/memory12/state offline
/sys/devices/system/memory/memory9/state offline
4) We bring them online in non-natural order:
$grep MemTotal /proc/meminfo
MemTotal: 966348 kB
$echo online > /sys/devices/system/memory/memory12/state && grep MemTotal
/proc/meminfo
MemTotal:1019596 kB
$echo online > /sys/devices/system/memory/memory11/state && grep MemTotal
/proc/meminfo
MemTotal:1150668 kB
$echo online > /sys/devices/system/memory/memory9/state && grep MemTotal
/proc/meminfo
MemTotal:1150668 kB
As you can see memory9 block gives us zero additional memory. We can also
observe a huge discrepancy between host- and guest-reported memory sizes.
The root cause of the issue is the redundant pg >= covered_start_pfn check (and
covered_start_pfn advancing) in hv_online_page(). When upper memory block in
being onlined before the lower one (memory12 and memory11 in the above case) we
advance the covered_start_pfn pointer and all memory9 pages do not pass the
check. If the assumption that host always gives us requests in sequential order
and pg_start always equals rg_start when the first request for the new HA
region is received (that's the case in my testing) is correct than we can get
rid of covered_start_pfn and pg >= start_pfn check in hv_online_page() is
sufficient.
The current Linux 4.0 RC3 tree is broken and this patch fixes the problem.
Signed-off-by: Vitaly Kuznetsov
Signed-off-by: K. Y. Srinivasan
---
drivers/hv/hv_balloon.c | 14 --
1 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index f1f17c5..014256a 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -428,14 +428,13 @@ struct dm_info_msg {
* currently hot added. We hot add in multiples of 128M
* chunks; it is possible that we may not be able to bring
* online all the pages in the region. The range
- * covered_start_pfn : covered_end_pfn defines the pages that can
+ * covered_end_pfn defines the pages that can
* be brough online.
*/
struct hv_hotadd_state {
struct list_head list;
unsigned long start_pfn;
- unsigned long covered_start_pfn;
unsigned long covered_end_pfn;
unsigned long ha_end_pfn;
unsigned long end_pfn;
@@ -679,8 +678,7 @@ static void hv_online_page(struct page *pg)
list_for_each(cur, &dm_device.ha_region_list) {
has = list_entry(cur, struct hv_hotadd_state, list);
- cur_start_pgp = (unsigned long)
- pfn_to_page(has->covered_start_pfn);
+ cur_start_pgp = (unsigned long)pfn_to_page(has->start_pfn);
cur_end_pgp = (unsigned long)pfn_to_page(has->covered_end_pfn);
if (((unsigned long)pg >= cur_start_pgp) &&
@@ -692,7 +690,6 @@ static void hv_online_page(struct page *pg)
__online_page_set_limits(pg);
__online_page_increment_counters(pg);
__online_page_free(pg);
- has->covered_start_pfn++;
}
}
}
@@ -736,10 +733,9 @@ static bool pfn_covered(unsigned long start_pfn, unsigned
long pfn_cnt)
* is, update it.
*/
- if (has->covered_end_pfn != start_pfn) {
+ if (has->covered_end_pfn != start_pfn)
has->covered_end_pfn = start_pfn;
- has->covered_start_pfn = start_pfn;
- }
+
return true;
}
@@ -784,7 +780,6 @@ static unsigned long handle_pg_range(unsigned long pg_start,
pgs_ol = pfn_cnt;
hv_bring_pgs_online(start_pfn, pgs_ol);
has->covered_end_pfn += pgs_ol;
- has->covered_start_pfn += pgs_ol;
[PATCH 2/6] Drivers: hv: hv_balloon: keep locks balanced on add_memory() failure
From: Vitaly Kuznetsov When add_memory() fails the following BUG is observed: [ 743.646107] hv_balloon: hot_add memory failed error is -17 [ 743.679973] [ 743.680930] = [ 743.680930] [ BUG: bad unlock balance detected! ] [ 743.680930] 3.19.0-rc5_bug1131426+ #552 Not tainted [ 743.680930] - [ 743.680930] kworker/0:2/255 is trying to release lock (&dm_device.ha_region_mutex) at: [ 743.680930] [] mutex_unlock+0xe/0x10 [ 743.680930] but there are no more locks to release! This happens as we don't acquire ha_region_mutex and hot_add_req() expects us to as it does unconditional mutex_unlock(). Acquire the lock on the error path. The current Linux 4.0 RC3 tree is broken and this patch fixes the problem. Signed-off-by: Vitaly Kuznetsov Acked-by: Jason Wang Signed-off-by: K. Y. Srinivasan --- drivers/hv/hv_balloon.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c index c5bb872..f1f17c5 100644 --- a/drivers/hv/hv_balloon.c +++ b/drivers/hv/hv_balloon.c @@ -652,6 +652,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size, } has->ha_end_pfn -= HA_CHUNK; has->covered_end_pfn -= processed_pfn; + mutex_lock(&dm_device.ha_region_mutex); break; } -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/6] Correcting truncation error for constant HV_CRASH_CTL_CRASH_NOTIFY
From: Nick Meier
HV_CRASH_CTL_CRASH_NOTIFY is a 64 bit number. Depending on the usage context,
the value may be truncated. This patch is in response from the following
email from Intel:
[char-misc:char-misc-testing 25/45] drivers/hv/vmbus_drv.c:67:9: sparse:
constant 0x8000 is so big it is unsigned long
tree: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
char-misc-testing
head: b3de8e3719e582f3182bb504295e4a8e43c8c96f
commit: 96c1d0581d00f7abe033350edb021a9d947d8d81 [25/45] Drivers: hv:
vmbus: Add support for VMBus panic notifier handler
reproduce:
# apt-get install sparse
git checkout 96c1d0581d00f7abe033350edb021a9d947d8d81
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
drivers/hv/vmbus_drv.c:67:9: sparse: constant 0x8000 is so big
it is unsigned long
...
The current Linux 4.0 RC3 tree is broken and this patch fixes the problem.
Signed-off-by: Nick Meier
Signed-off-by: K. Y. Srinivasan
---
drivers/hv/hyperv_vmbus.h |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 6339589..c8e27e0 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -58,7 +58,7 @@ enum hv_cpuid_function {
#define HV_X64_MSR_CRASH_P4 0x4104
#define HV_X64_MSR_CRASH_CTL 0x4105
-#define HV_CRASH_CTL_CRASH_NOTIFY 0x8000
+#define HV_CRASH_CTL_CRASH_NOTIFY (1ULL << 63)
/* Define version of the synthetic interrupt controller. */
#define HV_SYNIC_VERSION (1)
--
1.7.4.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/6] tools: hv: fcopy_daemon: support >2GB files for x86_32 guest
From: Dexuan Cui Without this patch, hv_fcopy_daemon's hv_copy_data() -> pwrite() will fail for >2GB file offset. The current Linux 4.0 RC3 tree is broken and this patch fixes the problem. Signed-off-by: Alex Ng Signed-off-by: Dexuan Cui Cc: K. Y. Srinivasan Signed-off-by: K. Y. Srinivasan --- tools/hv/Makefile |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/hv/Makefile b/tools/hv/Makefile index 99ffe61..a8ab795 100644 --- a/tools/hv/Makefile +++ b/tools/hv/Makefile @@ -3,7 +3,7 @@ CC = $(CROSS_COMPILE)gcc PTHREAD_LIBS = -lpthread WARNINGS = -Wall -Wextra -CFLAGS = $(WARNINGS) -g $(PTHREAD_LIBS) +CFLAGS = $(WARNINGS) -g $(PTHREAD_LIBS) $(shell getconf LFS_CFLAGS) all: hv_kvp_daemon hv_vss_daemon hv_fcopy_daemon %: %.c -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/6] Drivers: hv: vmbus: Perform device register in the per-channel work element
This patch is a continuation of the rescind handling cleanup work. We cannot
block in the global message handling work context especially if we are blocking
waiting for the host to wake us up. I would like to thank
Dexuan Cui for observing this problem.
The current Linux 4.0 RC3 tree is broken and this patch fixes the problem.
Signed-off-by: K. Y. Srinivasan
---
drivers/hv/channel_mgmt.c | 143 +++-
drivers/hv/connection.c |6 ++-
drivers/hv/hyperv_vmbus.h |2 +-
3 files changed, 107 insertions(+), 44 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 6117891..5f8e47b 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -37,6 +38,10 @@ struct vmbus_channel_message_table_entry {
void (*message_handler)(struct vmbus_channel_message_header *msg);
};
+struct vmbus_rescind_work {
+ struct work_struct work;
+ struct vmbus_channel *channel;
+};
/**
* vmbus_prep_negotiate_resp() - Create default response for Hyper-V Negotiate
message
@@ -134,20 +139,6 @@ fw_error:
EXPORT_SYMBOL_GPL(vmbus_prep_negotiate_resp);
-static void vmbus_process_device_unregister(struct work_struct *work)
-{
- struct device *dev;
- struct vmbus_channel *channel = container_of(work,
- struct vmbus_channel,
- work);
-
- dev = get_device(&channel->device_obj->device);
- if (dev) {
- vmbus_device_unregister(channel->device_obj);
- put_device(dev);
- }
-}
-
static void vmbus_sc_creation_cb(struct work_struct *work)
{
struct vmbus_channel *newchannel = container_of(work,
@@ -220,6 +211,40 @@ static void free_channel(struct vmbus_channel *channel)
queue_work(vmbus_connection.work_queue, &channel->work);
}
+static void process_rescind_fn(struct work_struct *work)
+{
+ struct vmbus_rescind_work *rc_work;
+ struct vmbus_channel *channel;
+ struct device *dev;
+
+ rc_work = container_of(work, struct vmbus_rescind_work, work);
+ channel = rc_work->channel;
+
+ /*
+* We have already acquired a reference on the channel
+* and so it cannot vanish underneath us.
+* It is possible (while very unlikely) that we may
+* get here while the processing of the initial offer
+* is still not complete. Deal with this situation by
+* just waiting until the channel is in the correct state.
+*/
+
+ while (channel->work.func != release_channel)
+ msleep(1000);
+
+ if (channel->device_obj) {
+ dev = get_device(&channel->device_obj->device);
+ if (dev) {
+ vmbus_device_unregister(channel->device_obj);
+ put_device(dev);
+ }
+ } else {
+ hv_process_channel_removal(channel,
+ channel->offermsg.child_relid);
+ }
+ kfree(work);
+}
+
static void percpu_channel_enq(void *arg)
{
struct vmbus_channel *channel = arg;
@@ -282,6 +307,46 @@ void vmbus_free_channels(void)
}
}
+static void vmbus_do_device_register(struct work_struct *work)
+{
+ struct hv_device *device_obj;
+ int ret;
+ unsigned long flags;
+ struct vmbus_channel *newchannel = container_of(work,
+struct vmbus_channel,
+work);
+
+ ret = vmbus_device_register(newchannel->device_obj);
+ if (ret != 0) {
+ pr_err("unable to add child device object (relid %d)\n",
+ newchannel->offermsg.child_relid);
+ spin_lock_irqsave(&vmbus_connection.channel_lock, flags);
+ list_del(&newchannel->listentry);
+ device_obj = newchannel->device_obj;
+ newchannel->device_obj = NULL;
+ spin_unlock_irqrestore(&vmbus_connection.channel_lock, flags);
+
+ if (newchannel->target_cpu != get_cpu()) {
+ put_cpu();
+ smp_call_function_single(newchannel->target_cpu,
+percpu_channel_deq, newchannel, true);
+ } else {
+ percpu_channel_deq(newchannel);
+ put_cpu();
+ }
+
+ kfree(device_obj);
+ if (!newchannel->rescind) {
+ free_channel(newchannel);
+ return;
+ }
+ }
+ /*
+* The next state for this channel is to be freed.
+*/
+ INIT_WORK(&newchannel->work, release_channel);
+}
+
/*
* vmbus_process_offer - Process the
[PATCH 0/6] drivers: hv: vmbus: Some miscellaneous fixes for Linux 4.0
Some miscellaneous fixes to the vmbus driver and the balloon driver. Currently the Linux 4.0 RC3 tree is broken on Hyper-V and this patch-set fixes the issues. Dexuan Cui (1): tools: hv: fcopy_daemon: support >2GB files for x86_32 guest K. Y. Srinivasan (2): Drivers: hv: vmbus: Perform device register in the per-channel work element Drivers: hv: vmbus: Fix a bug in rescind processing in vmbus_close_internal() Nick Meier (1): Correcting truncation error for constant HV_CRASH_CTL_CRASH_NOTIFY Vitaly Kuznetsov (2): Drivers: hv: hv_balloon: keep locks balanced on add_memory() failure Drivers: hv: hv_balloon: don't lose memory when onlining order is not natural drivers/hv/channel.c | 15 ++--- drivers/hv/channel_mgmt.c | 143 +++- drivers/hv/connection.c |6 ++- drivers/hv/hv_balloon.c | 15 ++--- drivers/hv/hyperv_vmbus.h |4 +- tools/hv/Makefile |2 +- 6 files changed, 120 insertions(+), 65 deletions(-) -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv4 1/2] ft1000-pcmcia: ft1000_hw.c: fix style issues not requiring code refactoring
Hi Daniele,
On 2015.03.12 00:05, Daniele Alessandrelli wrote:
> Fix all the trivial style issues (as reported by checkpatch.pl) not requiring
> code refactoring. A following patch is expected to fix the remaining issues by
> performing some code refactoring.
>
> Signed-off-by: Daniele Alessandrelli
> ---
> drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c | 199
> +--
> 1 file changed, 110 insertions(+), 89 deletions(-)
>
> ==Changelog==
>
> v4: updated to leatest Greg's tree (and added changelog)
>
> v3: resent (unchanged) because of error in the other patch of the patchset
>
> v2: updated to apply cleanly to Greg's tree
>
> v1: initial realease
>
> diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
> b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
> index bc959ff..5ff4da8 100644
> --- a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
> +++ b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
> @@ -28,8 +28,8 @@
> #include
> #include
> #include
> -#include
> -#include
> +#include
> +#include
>
> #include
> #include
> @@ -67,8 +67,7 @@ static void ft1000_disable_interrupts(struct net_device
> *dev);
>
> /* new kernel */
> MODULE_AUTHOR("");
> -MODULE_DESCRIPTION
> -("Support for Flarion Flash OFDM NIC Device. Support for PCMCIA when used
> with ft1000_cs.");
> +MODULE_DESCRIPTION("Support for Flarion Flash OFDM NIC Device. Support for
> PCMCIA when used with ft1000_cs.");
> MODULE_LICENSE("GPL");
> MODULE_SUPPORTED_DEVICE("FT1000");
>
> @@ -267,7 +266,8 @@ void ft1000_write_dpram_mag_32(struct net_device *dev,
> int offset, u32 value)
> /*---
>
>Function: ft1000_enable_interrupts
> - Description: This function will enable interrupts base on the current
> interrupt mask.
> + Description: This function will enable interrupts base on the current
> +interrupt mask.
>Input:
>dev- device structure
>Output:
> @@ -375,7 +375,8 @@ static int ft1000_reset_card(struct net_device *dev)
> /* Make sure we free any memory reserve for provisioning */
> while (list_empty(&info->prov_list) == 0) {
> pr_debug("deleting provisioning record\n");
> - ptr = list_entry(info->prov_list.next, struct prov_record,
> list);
> + ptr = list_entry(info->prov_list.next, struct prov_record,
> + list);
> list_del(&ptr->list);
> kfree(ptr->pprov_data);
> kfree(ptr);
> @@ -406,7 +407,8 @@ static int ft1000_reset_card(struct net_device *dev)
>FT1000_DPRAM_MAG_RX_BASE);
> for (i = 0; i < MAX_DSP_SESS_REC / 2; i++) {
> info->DSPSess.MagRec[i] =
> - inl(dev->base_addr +
> FT1000_REG_MAG_DPDATA);
> + inl(dev->base_addr
> + + FT1000_REG_MAG_DPDATA);
> }
> }
> spin_unlock_irqrestore(&info->dpram_lock, flags);
> @@ -435,11 +437,12 @@ static int ft1000_reset_card(struct net_device *dev)
> mdelay(10);
> pr_debug("Take DSP out of reset\n");
>
> - /* Wait for 0xfefe indicating dsp ready before starting
> download */
> + /* Wait for 0xfefe indicating dsp ready before starting
> + * download */
Minor nitpick: multi-line comments beggining /* and ending */ go on
seperate lines. You can see this style used later in unchanged places
visible in this patch.
> for (i = 0; i < 50; i++) {
> - tempword =
> - ft1000_read_dpram_mag_16(dev,
> FT1000_MAG_DPRAM_FEFE,
> -
> FT1000_MAG_DPRAM_FEFE_INDX);
> + tempword = ft1000_read_dpram_mag_16(dev,
> + FT1000_MAG_DPRAM_FEFE,
> + FT1000_MAG_DPRAM_FEFE_INDX);
> if (tempword == 0xfefe)
> break;
> mdelay(20);
> @@ -466,8 +469,8 @@ static int ft1000_reset_card(struct net_device *dev)
>
> if (info->AsicID == ELECTRABUZZ_ID) {
> /*
> - * Need to initialize the FIFO length counter to zero in order
> to sync up
> - * with the DSP
> + * Need to initialize the FIFO length counter to zero in order
> + * to sync up with the DSP
>*/
> info->fifo_cnt = 0;
> ft1000_write_dpram(dev, FT1000_FIFO_LEN, info->fifo_cnt);
> @@ -664,8 +667,8 @@ static void ft1000_hbchk(u_long data)
> return;
> }
> /*
> - * Set dedicated area to hi and ring appropriate
Re: [PATCH v2] staging: rtl8192e: rtllib_wx: code style improvements
On 11.03.2015 08:53, Greg KH wrote: > On Wed, Mar 11, 2015 at 08:46:00AM +0100, Mateusz Kulikowski wrote: >> Code reformatting based on checkpatch.pl: >> - Replaced min() with min_t() >> - Replaced printk() with netdev_*() >> - Merged broken string > > That's three different things, which means this should be at least 3 > different patches. Please fix up and resend a patch series, as each > patch should only do one thing. Will split in v3 (and do it for future patches). Thanks for the comments, Mateusz ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 0/7] drivers: hv: vmbus: Some miscellaneous fixes
> -Original Message- > From: K. Y. Srinivasan [mailto:k...@microsoft.com] > Sent: Tuesday, March 10, 2015 5:07 PM > To: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > vkuzn...@redhat.com > Cc: KY Srinivasan > Subject: [PATCH 0/7] drivers: hv: vmbus: Some miscellaneous fixes > > Some miscellaneous fixes to the vmbus driver and the balloon driver. > Currently the linux-next tree is broken and some of the patches in this set > fix > the issue. > > Dan Carpenter (1): > hv: vmbus: missing curly braces in vmbus_process_offer() > > Dexuan Cui (1): > tools: hv: fcopy_daemon: support >2GB files for x86_32 guest > > K. Y. Srinivasan (2): > Drivers: hv: vmbus: Perform device register in the per-channel work > element > Drivers: hv: vmbus: Fix a bug in rescind processing in > vmbus_close_internal() > > Nick Meier (1): > Correcting truncation error for constant HV_CRASH_CTL_CRASH_NOTIFY > > Vitaly Kuznetsov (2): > Drivers: hv: hv_balloon: keep locks balanced on add_memory() failure > Drivers: hv: hv_balloon: don't lose memory when onlining order is not > natural Greg, Please drop this set. I am going to break this up as you had suggested into a set that needs to go into 4.0 final and a set that can go into 4.1 Regards, K. Y ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv4 2/2] ft1000-pcmcia: ft1000_hw.c: code refactoring: add ft1000_read_dsp_timer()
Add new function ft1000_read_dsp_timer() replacing recurring code block for
reading DSP timer. Such code refactoring solves all remaining "line over 80
characters" warnings reported by checkpatch.pl.
Signed-off-by: Daniele Alessandrelli
---
drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c | 199 +--
1 file changed, 41 insertions(+), 158 deletions(-)
==Changelog==
v4: updated to leatest Greg's tree (and added changelog)
v3: resent because of mistake in previous submission
v2: updated to apply cleanly to Greg's tree; but I actually set a wrong version
of the patch :/
v1: initial realease
diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
index 5ff4da8..4a492dd 100644
--- a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
+++ b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
@@ -303,6 +303,41 @@ static void ft1000_disable_interrupts(struct net_device
*dev)
}
/*---
+ Function:ft1000_read_dsp_timer
+ Description: This function reads the DSP timer and stores its value in the
+ DSP_TIME field of the ft1000_info struct passed as argument
+ Input:
+ dev- device structure
+ info - ft1000_info structure
+ Output:
+ None.
+
+ -*/
+static void ft1000_read_dsp_timer(struct net_device *dev,
+ struct ft1000_info *info)
+{
+ if (info->AsicID == ELECTRABUZZ_ID) {
+ info->DSP_TIME[0] = ft1000_read_dpram(dev, FT1000_DSP_TIMER0);
+ info->DSP_TIME[1] = ft1000_read_dpram(dev, FT1000_DSP_TIMER1);
+ info->DSP_TIME[2] = ft1000_read_dpram(dev, FT1000_DSP_TIMER2);
+ info->DSP_TIME[3] = ft1000_read_dpram(dev, FT1000_DSP_TIMER3);
+ } else {
+ info->DSP_TIME[0] =
+ ft1000_read_dpram_mag_16(dev, FT1000_MAG_DSP_TIMER0,
+FT1000_MAG_DSP_TIMER0_INDX);
+ info->DSP_TIME[1] =
+ ft1000_read_dpram_mag_16(dev, FT1000_MAG_DSP_TIMER1,
+FT1000_MAG_DSP_TIMER1_INDX);
+ info->DSP_TIME[2] =
+ ft1000_read_dpram_mag_16(dev, FT1000_MAG_DSP_TIMER2,
+FT1000_MAG_DSP_TIMER2_INDX);
+ info->DSP_TIME[3] =
+ ft1000_read_dpram_mag_16(dev, FT1000_MAG_DSP_TIMER3,
+FT1000_MAG_DSP_TIMER3_INDX);
+ }
+}
+
+/*---
Function: ft1000_reset_asic
Description: This function will call the Card Service function to reset the
@@ -580,33 +615,7 @@ static void ft1000_hbchk(u_long data)
}
if (tempword != ho) {
pr_info("heartbeat failed - no ho detected\n");
- if (info->AsicID == ELECTRABUZZ_ID) {
- info->DSP_TIME[0] =
- ft1000_read_dpram(dev,
FT1000_DSP_TIMER0);
- info->DSP_TIME[1] =
- ft1000_read_dpram(dev,
FT1000_DSP_TIMER1);
- info->DSP_TIME[2] =
- ft1000_read_dpram(dev,
FT1000_DSP_TIMER2);
- info->DSP_TIME[3] =
- ft1000_read_dpram(dev,
FT1000_DSP_TIMER3);
- } else {
- info->DSP_TIME[0] =
- ft1000_read_dpram_mag_16(dev,
-
FT1000_MAG_DSP_TIMER0,
-
FT1000_MAG_DSP_TIMER0_INDX);
- info->DSP_TIME[1] =
- ft1000_read_dpram_mag_16(dev,
-
FT1000_MAG_DSP_TIMER1,
-
FT1000_MAG_DSP_TIMER1_INDX);
- info->DSP_TIME[2] =
- ft1000_read_dpram_mag_16(dev,
-
FT1000_MAG_DSP_TIMER2,
-
FT1000_MAG_DSP_TIMER2_INDX);
- info->DSP_TIME[3] =
- ft1000_read_dpram_mag_16(dev,
-
FT1000_MAG_DSP_TIMER3,
-
FT1000_MAG_DSP_TIMER3_INDX);
- }
+ ft1000_read_dsp_
[PATCHv4 1/2] ft1000-pcmcia: ft1000_hw.c: fix style issues not requiring code refactoring
Fix all the trivial style issues (as reported by checkpatch.pl) not requiring
code refactoring. A following patch is expected to fix the remaining issues by
performing some code refactoring.
Signed-off-by: Daniele Alessandrelli
---
drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c | 199 +--
1 file changed, 110 insertions(+), 89 deletions(-)
==Changelog==
v4: updated to leatest Greg's tree (and added changelog)
v3: resent (unchanged) because of error in the other patch of the patchset
v2: updated to apply cleanly to Greg's tree
v1: initial realease
diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
index bc959ff..5ff4da8 100644
--- a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
+++ b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
@@ -28,8 +28,8 @@
#include
#include
#include
-#include
-#include
+#include
+#include
#include
#include
@@ -67,8 +67,7 @@ static void ft1000_disable_interrupts(struct net_device *dev);
/* new kernel */
MODULE_AUTHOR("");
-MODULE_DESCRIPTION
-("Support for Flarion Flash OFDM NIC Device. Support for PCMCIA when used with
ft1000_cs.");
+MODULE_DESCRIPTION("Support for Flarion Flash OFDM NIC Device. Support for
PCMCIA when used with ft1000_cs.");
MODULE_LICENSE("GPL");
MODULE_SUPPORTED_DEVICE("FT1000");
@@ -267,7 +266,8 @@ void ft1000_write_dpram_mag_32(struct net_device *dev, int
offset, u32 value)
/*---
Function: ft1000_enable_interrupts
- Description: This function will enable interrupts base on the current
interrupt mask.
+ Description: This function will enable interrupts base on the current
+ interrupt mask.
Input:
dev- device structure
Output:
@@ -375,7 +375,8 @@ static int ft1000_reset_card(struct net_device *dev)
/* Make sure we free any memory reserve for provisioning */
while (list_empty(&info->prov_list) == 0) {
pr_debug("deleting provisioning record\n");
- ptr = list_entry(info->prov_list.next, struct prov_record,
list);
+ ptr = list_entry(info->prov_list.next, struct prov_record,
+list);
list_del(&ptr->list);
kfree(ptr->pprov_data);
kfree(ptr);
@@ -406,7 +407,8 @@ static int ft1000_reset_card(struct net_device *dev)
FT1000_DPRAM_MAG_RX_BASE);
for (i = 0; i < MAX_DSP_SESS_REC / 2; i++) {
info->DSPSess.MagRec[i] =
- inl(dev->base_addr +
FT1000_REG_MAG_DPDATA);
+ inl(dev->base_addr
+ + FT1000_REG_MAG_DPDATA);
}
}
spin_unlock_irqrestore(&info->dpram_lock, flags);
@@ -435,11 +437,12 @@ static int ft1000_reset_card(struct net_device *dev)
mdelay(10);
pr_debug("Take DSP out of reset\n");
- /* Wait for 0xfefe indicating dsp ready before starting
download */
+ /* Wait for 0xfefe indicating dsp ready before starting
+* download */
for (i = 0; i < 50; i++) {
- tempword =
- ft1000_read_dpram_mag_16(dev,
FT1000_MAG_DPRAM_FEFE,
-
FT1000_MAG_DPRAM_FEFE_INDX);
+ tempword = ft1000_read_dpram_mag_16(dev,
+ FT1000_MAG_DPRAM_FEFE,
+ FT1000_MAG_DPRAM_FEFE_INDX);
if (tempword == 0xfefe)
break;
mdelay(20);
@@ -466,8 +469,8 @@ static int ft1000_reset_card(struct net_device *dev)
if (info->AsicID == ELECTRABUZZ_ID) {
/*
-* Need to initialize the FIFO length counter to zero in order
to sync up
-* with the DSP
+* Need to initialize the FIFO length counter to zero in order
+* to sync up with the DSP
*/
info->fifo_cnt = 0;
ft1000_write_dpram(dev, FT1000_FIFO_LEN, info->fifo_cnt);
@@ -664,8 +667,8 @@ static void ft1000_hbchk(u_long data)
return;
}
/*
-* Set dedicated area to hi and ring appropriate doorbell
according
-* to hi/ho heartbeat protocol
+* Set dedicated area to hi and ring appropriate doorbell
+* according to hi/ho heartbeat protocol
*/
if (info->AsicID == ELECTRABUZZ_ID) {
ft1000_write_dpram(dev, FT1000_HI_HO, hi);
@@ -687,13 +690,15 @@ static void
[PATCHv4 0/2] staging: ft1000-pcmcia: ft1000_hw.c: fix all checkpatch warnings
This small patchset fixes all the style problems reported by checkpatch.pl on ft1000-pcmcia/ft1000_hw.c * Patch 1/2 fixes all trivial issues not requiring code refactoring * Patch 2/2 fixes all remaining "line over 80 characters" warnings by means of some code refactoring. Specifically, the function ft1000_read_dsp_timer() is introduced to replace a recurring block of code used for reading the DSP timer value. This updated version of the patchset should apply cleanly to Greg's staging-testing tree. Daniele Alessandrelli (2): ft1000-pcmcia: ft1000_hw.c: fix style issues not requiring code refactoring ft1000-pcmcia: ft1000_hw.c: code refactoring: add ft1000_read_dsp_timer() drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c | 382 +-- 1 file changed, 143 insertions(+), 239 deletions(-) -- 1.8.3.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] Drivers: Staging: ft1000: checkpatch warning fixes
On Wed, 2015-03-11 at 14:03 -0700, Janakarajan Natarajan wrote:
> Addition of blank line after declaration in ft1000_hw.c
> Minor changes to remove {} from single line if and remove extra parenthesis.
> Fixes checkpatch warning for and usage.
Most prefer separate patches for each type of change,
so this single patch could be 3 patches.
> diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
> b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
and for more trivia:
> @@ -1948,11 +1948,11 @@ static irqreturn_t ft1000_interrupt(int irq, void
> *dev_id)
> ft1000_read_reg(dev,
>
> FT1000_REG_MAG_DFSR);
> }
> - if (tempword & 0x1f) {
> + if (tempword & 0x1f)
> ft1000_copy_up_pkt(dev);
> - } else {
> + else
> break;
> - }
> +
> cnt++;
> } while (cnt < MAX_RCV_LOOP);
do {
if (foo)
bar;
else
break;
} while (baz);
is generally better written
do {
if (!foo)
break;
bar;
} while (baz);
so
if (!(tempword & 0x1f))
break;
ft100_copy_up_pkt(dev);
cnt++;
} while (cnt < MAX_RCV_LOOP);
or maybe
if (!(tempword & 0x1f))
break;
ft100_copy_up_pkt(dev);
} while (++cnt < MAX_RCV_LOOP);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] Drivers: Staging: ft1000: checkpatch warning fixes
On 2015.03.11 23:03, Janakarajan Natarajan wrote:
> Addition of blank line after declaration in ft1000_hw.c
> Minor changes to remove {} from single line if and remove extra parenthesis.
> Fixes checkpatch warning for and usage.
>
Greg may complain that you've done too much in 1 patch.
> Signed-off-by: Janakarajan Natarajan
> ---
> drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c | 12 ++--
> drivers/staging/ft1000/ft1000-usb/ft1000_debug.c | 9 +++--
> 2 files changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
> b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
> index 017c3b9..a01b9e3 100644
> --- a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
> +++ b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
> @@ -28,8 +28,8 @@
> #include
> #include
> #include
> -#include
> -#include
> +#include
> +#include
>
> #include
> #include
> @@ -1948,11 +1948,11 @@ static irqreturn_t ft1000_interrupt(int irq, void
> *dev_id)
> ft1000_read_reg(dev,
>
> FT1000_REG_MAG_DFSR);
> }
> - if (tempword & 0x1f) {
> + if (tempword & 0x1f)
> ft1000_copy_up_pkt(dev);
> - } else {
> + else
> break;
> - }
> +
> cnt++;
> } while (cnt < MAX_RCV_LOOP);
>
> @@ -2008,8 +2008,8 @@ static void ft1000_get_drvinfo(struct net_device *dev,
> struct ethtool_drvinfo *info)
> {
> struct ft1000_info *ft_info;
> - ft_info = netdev_priv(dev);
>
> + ft_info = netdev_priv(dev);
You could just do:
struct ft1000_info *ft_info = netdev_priv(dev)
> strlcpy(info->driver, "ft1000", sizeof(info->driver));
> snprintf(info->bus_info, sizeof(info->bus_info), "PCMCIA 0x%lx",
>dev->base_addr);
> diff --git a/drivers/staging/ft1000/ft1000-usb/ft1000_debug.c
> b/drivers/staging/ft1000/ft1000-usb/ft1000_debug.c
> index c8d2782..bd0d25c 100644
> --- a/drivers/staging/ft1000/ft1000-usb/ft1000_debug.c
> +++ b/drivers/staging/ft1000/ft1000-usb/ft1000_debug.c
> @@ -317,9 +317,8 @@ static int ft1000_open(struct inode *inode, struct file
> *file)
>
> /* Search for available application info block */
> for (i = 0; i < MAX_NUM_APP; i++) {
> - if ((dev->app_info[i].fileobject == NULL)) {
> + if (dev->app_info[i].fileobject == NULL)
> break;
> - }
Here you can simply do:
if (!dev->app_info[i].file_object)
> }
>
> /* Fail due to lack of application info block */
> @@ -575,9 +574,8 @@ static long ft1000_ioctl(struct file *file, unsigned int
> command,
> } else {
> /* Check if this message came from a registered
> application */
> for (i = 0; i < MAX_NUM_APP; i++) {
> - if (ft1000dev->app_info[i].fileobject
> == &file->f_owner) {
> + if (ft1000dev->app_info[i].fileobject
> == &file->f_owner)
> break;
> - }
> }
> if (i == MAX_NUM_APP) {
> pr_debug("No matching application
> fileobject\n");
> @@ -629,9 +627,8 @@ static long ft1000_ioctl(struct file *file, unsigned int
> command,
> pmsg = (u16
> *)&dpram_data->pseudohdr;
> ppseudo_hdr = (struct
> pseudo_hdr *)pmsg;
> total_len = msgsz+2;
> - if (total_len & 0x1) {
> + if (total_len & 0x1)
> total_len++;
> - }
>
> /* Insert slow queue sequence
> number */
> ppseudo_hdr->seq_num =
> info->squeseqnum++;
>
--
Thanks,
Giedrius
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] Drivers: Staging: ft1000: checkpatch warning fixes
Addition of blank line after declaration in ft1000_hw.c
Minor changes to remove {} from single line if and remove extra parenthesis.
Fixes checkpatch warning for and usage.
Signed-off-by: Janakarajan Natarajan
---
drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c | 12 ++--
drivers/staging/ft1000/ft1000-usb/ft1000_debug.c | 9 +++--
2 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
index 017c3b9..a01b9e3 100644
--- a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
+++ b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
@@ -28,8 +28,8 @@
#include
#include
#include
-#include
-#include
+#include
+#include
#include
#include
@@ -1948,11 +1948,11 @@ static irqreturn_t ft1000_interrupt(int irq, void
*dev_id)
ft1000_read_reg(dev,
FT1000_REG_MAG_DFSR);
}
- if (tempword & 0x1f) {
+ if (tempword & 0x1f)
ft1000_copy_up_pkt(dev);
- } else {
+ else
break;
- }
+
cnt++;
} while (cnt < MAX_RCV_LOOP);
@@ -2008,8 +2008,8 @@ static void ft1000_get_drvinfo(struct net_device *dev,
struct ethtool_drvinfo *info)
{
struct ft1000_info *ft_info;
- ft_info = netdev_priv(dev);
+ ft_info = netdev_priv(dev);
strlcpy(info->driver, "ft1000", sizeof(info->driver));
snprintf(info->bus_info, sizeof(info->bus_info), "PCMCIA 0x%lx",
dev->base_addr);
diff --git a/drivers/staging/ft1000/ft1000-usb/ft1000_debug.c
b/drivers/staging/ft1000/ft1000-usb/ft1000_debug.c
index c8d2782..bd0d25c 100644
--- a/drivers/staging/ft1000/ft1000-usb/ft1000_debug.c
+++ b/drivers/staging/ft1000/ft1000-usb/ft1000_debug.c
@@ -317,9 +317,8 @@ static int ft1000_open(struct inode *inode, struct file
*file)
/* Search for available application info block */
for (i = 0; i < MAX_NUM_APP; i++) {
- if ((dev->app_info[i].fileobject == NULL)) {
+ if (dev->app_info[i].fileobject == NULL)
break;
- }
}
/* Fail due to lack of application info block */
@@ -575,9 +574,8 @@ static long ft1000_ioctl(struct file *file, unsigned int
command,
} else {
/* Check if this message came from a registered
application */
for (i = 0; i < MAX_NUM_APP; i++) {
- if (ft1000dev->app_info[i].fileobject
== &file->f_owner) {
+ if (ft1000dev->app_info[i].fileobject
== &file->f_owner)
break;
- }
}
if (i == MAX_NUM_APP) {
pr_debug("No matching application
fileobject\n");
@@ -629,9 +627,8 @@ static long ft1000_ioctl(struct file *file, unsigned int
command,
pmsg = (u16
*)&dpram_data->pseudohdr;
ppseudo_hdr = (struct
pseudo_hdr *)pmsg;
total_len = msgsz+2;
- if (total_len & 0x1) {
+ if (total_len & 0x1)
total_len++;
- }
/* Insert slow queue sequence
number */
ppseudo_hdr->seq_num =
info->squeseqnum++;
--
1.9.1
---
This email has been checked for viruses by Avast antivirus software.
http://www.avast.com
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723au: Fix sparse warning cast to restricted __le16
Marcus Folkesson writes:
> This patch fixes the following sparse warnings:
>
> CHECK drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c
> drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c:265:37: warning:
> cast to restricted __le16
> drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c:267:39: warning:
> cast to restricted __le16
>
> Signed-off-by: Marcus Folkesson
> ---
> drivers/staging/rtl8723au/include/rtl8723a_hal.h | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/rtl8723au/include/rtl8723a_hal.h
> b/drivers/staging/rtl8723au/include/rtl8723a_hal.h
> index e146336..f642b11 100644
> --- a/drivers/staging/rtl8723au/include/rtl8723a_hal.h
> +++ b/drivers/staging/rtl8723au/include/rtl8723a_hal.h
> @@ -255,10 +255,10 @@ struct hal_data_8723a {
> struct hal_version VersionID;
> enum rt_customer_id CustomerID;
>
> - u16 FirmwareVersion;
> - u16 FirmwareVersionRev;
> - u16 FirmwareSubVersion;
> - u16 FirmwareSignature;
> + __le16 FirmwareVersion;
> + __le16 FirmwareVersionRev;
> + __le16 FirmwareSubVersion;
> + __le16 FirmwareSignature;
Ehm I am pretty sure it doesn't:
rtl8723au_hal_init.c:265
pHalData->FirmwareVersion = le16_to_cpu(pFwHdr->Version);
pHalData->FirmwareSubVersion = pFwHdr->Subversion;
pHalData->FirmwareSignature = le16_to_cpu(pFwHdr->Signature);
If anything, the second assignment there should be changed to use
le16_to_cpu(), but your conversion is definitely wrong.
NACK
Jes
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V2 1/3 net-next] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl()
Export the vmbus_sendpacket_pagebuffer_ctl() interface. This interface will be used in the netvsc driver to optimize signalling the host. Signed-off-by: K. Y. Srinivasan --- drivers/hv/channel.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index da53180..e58cdb7 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -710,6 +710,7 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel *channel, return ret; } +EXPORT_SYMBOL_GPL(vmbus_sendpacket_pagebuffer_ctl); /* * vmbus_sendpacket_pagebuffer - Send a range of single-page buffer -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V2 3/3 net-next] hyperv: Support batched notification
Optimize notifying the host by deferring notification until there
are no more packets to be sent. This will help in batching the requests
on the host.
Signed-off-by: K. Y. Srinivasan
---
drivers/net/hyperv/hyperv_net.h |2 +-
drivers/net/hyperv/netvsc.c | 14 +-
drivers/net/hyperv/netvsc_drv.c |3 ++-
drivers/net/hyperv/rndis_filter.c |2 +-
4 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 4815843..3fd9896 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -184,7 +184,7 @@ struct rndis_device {
int netvsc_device_add(struct hv_device *device, void *additional_info);
int netvsc_device_remove(struct hv_device *device);
int netvsc_send(struct hv_device *device,
- struct hv_netvsc_packet *packet);
+ struct hv_netvsc_packet *packet, bool kick_q);
void netvsc_linkstatus_callback(struct hv_device *device_obj,
struct rndis_message *resp);
int netvsc_recv_callback(struct hv_device *device_obj,
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 208eb05..9003b94 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -707,7 +707,7 @@ static u32 netvsc_copy_to_send_buf(struct netvsc_device
*net_device,
}
int netvsc_send(struct hv_device *device,
- struct hv_netvsc_packet *packet)
+ struct hv_netvsc_packet *packet, bool kick_q)
{
struct netvsc_device *net_device;
int ret = 0;
@@ -719,6 +719,7 @@ int netvsc_send(struct hv_device *device,
u32 msg_size = 0;
struct sk_buff *skb = NULL;
u16 q_idx = packet->q_idx;
+ u32 vmbus_flags = VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED;
net_device = get_outbound_net_device(device);
@@ -768,18 +769,21 @@ int netvsc_send(struct hv_device *device,
return -ENODEV;
if (packet->page_buf_cnt) {
- ret = vmbus_sendpacket_pagebuffer(out_channel,
+ ret = vmbus_sendpacket_pagebuffer_ctl(out_channel,
packet->page_buf,
packet->page_buf_cnt,
&sendMessage,
sizeof(struct nvsp_message),
- req_id);
+ req_id,
+ vmbus_flags,
+ kick_q);
} else {
- ret = vmbus_sendpacket(out_channel, &sendMessage,
+ ret = vmbus_sendpacket_ctl(out_channel, &sendMessage,
sizeof(struct nvsp_message),
req_id,
VM_PKT_DATA_INBAND,
- VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
+ vmbus_flags,
+ kick_q);
}
if (ret == 0) {
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index a06bd66..80b4b29 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -384,6 +384,7 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct
net_device *net)
u32 net_trans_info;
u32 hash;
u32 skb_length = skb->len;
+ bool kick_q = !skb->xmit_more;
/* We will atmost need two pages to describe the rndis
@@ -556,7 +557,7 @@ do_send:
packet->page_buf_cnt = init_page_array(rndis_msg, rndis_msg_size,
skb, &packet->page_buf[0]);
- ret = netvsc_send(net_device_ctx->device_ctx, packet);
+ ret = netvsc_send(net_device_ctx->device_ctx, packet, kick_q);
drop:
if (ret == 0) {
diff --git a/drivers/net/hyperv/rndis_filter.c
b/drivers/net/hyperv/rndis_filter.c
index ca81de0..05f3792 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -238,7 +238,7 @@ static int rndis_filter_send_request(struct rndis_device
*dev,
packet->send_completion = NULL;
- ret = netvsc_send(dev->net_dev->dev, packet);
+ ret = netvsc_send(dev->net_dev->dev, packet, true);
return ret;
}
--
1.7.4.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V2 0/3 net-next] hyperv: Enable batched notification
Take into consideration the xmit_more flag in skb to decide if we should notify the host as we place packets in VMBUS. The VMBUS API that would give us this control is already in Greg's tree, in this patch-set, that API is exported so it can be used in the netvsc driver. Also included here is a siganlling related fix in the vmbus driver that is exposed by the netvsc driver. I would like to thank Jason Wang for pointing out this issue. This version of the patch-set, adds an additional patch to fix the signalling issue. K. Y. Srinivasan (3): Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl() Drivers: hv: vmbus: Fix a bug in the signalling logic with kick_q hyperv: Support batched notification drivers/hv/channel.c | 33 + drivers/net/hyperv/hyperv_net.h |2 +- drivers/net/hyperv/netvsc.c | 14 +- drivers/net/hyperv/netvsc_drv.c |3 ++- drivers/net/hyperv/rndis_filter.c |2 +- 5 files changed, 46 insertions(+), 8 deletions(-) -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 2/2 net-next] hyperv: Support batched notification
> -Original Message- > From: Olaf Hering [mailto:o...@aepfle.de] > Sent: Wednesday, March 11, 2015 2:24 AM > To: KY Srinivasan > Cc: da...@davemloft.net; net...@vger.kernel.org; > gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; a...@canonical.com; jasow...@redhat.com > Subject: Re: [PATCH 2/2 net-next] hyperv: Support batched notification > > On Tue, Mar 10, K. Y. Srinivasan wrote: > > > Optimize notifying the host by deferring notification until there are > > no more packets to be sent. This will help in batching the requests on > > the host. > > The subjects for the network driver say "hyperv:". But all such changes > should get "netvsc:" as prefix to make it clear what each single patch is all > about. > > This should be changed for upcoming submissions. Olaf, This is the convention that we have used for patches submitted to David's tree. You will see net-next in the subject line and you can key off of that. K. Y > > Thanks! > > Olaf ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8723au: Fix sparse warning cast to restricted __le16
This patch fixes the following sparse warnings:
CHECK drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c
drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c:265:37: warning:
cast to restricted __le16
drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c:267:39: warning:
cast to restricted __le16
Signed-off-by: Marcus Folkesson
---
drivers/staging/rtl8723au/include/rtl8723a_hal.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8723au/include/rtl8723a_hal.h
b/drivers/staging/rtl8723au/include/rtl8723a_hal.h
index e146336..f642b11 100644
--- a/drivers/staging/rtl8723au/include/rtl8723a_hal.h
+++ b/drivers/staging/rtl8723au/include/rtl8723a_hal.h
@@ -255,10 +255,10 @@ struct hal_data_8723a {
struct hal_version VersionID;
enum rt_customer_id CustomerID;
- u16 FirmwareVersion;
- u16 FirmwareVersionRev;
- u16 FirmwareSubVersion;
- u16 FirmwareSignature;
+ __le16 FirmwareVersion;
+ __le16 FirmwareVersionRev;
+ __le16 FirmwareSubVersion;
+ __le16 FirmwareSignature;
/* current WIFI_PHY values */
u32 ReceiveConfig;
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] Staging: dgnc: Use goto for error handling
On 2015.03.11 16:04, Quentin Lambert wrote:
> This patch introduces goto statments for error handling
> and in cases where a lock needs to be released.
>
> A simplified version of the semantic patch that finds this problem is as
> follows: (http://coccinelle.lip6.fr)
>
> @candidates exists@
> identifier f, label;
> statement s;
> position p1, p2, p3;
> @@
>
> f@p1(...) {
> ...when any
>
> if@p2(...) {
> ...when any
> s
>
> return@p3 ...;
> }
> ...when any
> }
>
> @good1 exists@
> identifier candidates.f, candidates.label;
> statement candidates.s;
> position candidates.p1, candidates.p2;
> @@
>
> f@p1(...) {
> ...when any
>
> if(...) {
> ...when any
> s
> return ...;
> }
> ...when any
>
> if@p2(...) {...}
> ...when any
> }
>
> @depends on good1@
> identifier candidates.f, candidates.label;
> position candidates.p1, candidates.p3;
> @@
>
>f@p1(...) {
>...when any
> * return@p3 ...;
> }
>
> Signed-off-by: Quentin Lambert
> ---
> drivers/staging/dgnc/dgnc_cls.c| 19 +++
> drivers/staging/dgnc/dgnc_driver.c | 14 +++---
> drivers/staging/dgnc/dgnc_neo.c| 30 --
> 3 files changed, 22 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/staging/dgnc/dgnc_cls.c b/drivers/staging/dgnc/dgnc_cls.c
> index bedc522..02f19f1 100644
> --- a/drivers/staging/dgnc/dgnc_cls.c
> +++ b/drivers/staging/dgnc/dgnc_cls.c
> @@ -1029,22 +1029,16 @@ static void cls_copy_data_from_queue_to_uart(struct
> channel_t *ch)
> spin_lock_irqsave(&ch->ch_lock, flags);
>
> /* No data to write to the UART */
> - if (ch->ch_w_tail == ch->ch_w_head) {
> - spin_unlock_irqrestore(&ch->ch_lock, flags);
> - return;
> - }
> + if (ch->ch_w_tail == ch->ch_w_head)
> + goto exit_unlock;
>
> /* If port is "stopped", don't send any data to the UART */
> if ((ch->ch_flags & CH_FORCED_STOP) ||
> - (ch->ch_flags & CH_BREAK_SENDING)) {
> - spin_unlock_irqrestore(&ch->ch_lock, flags);
> - return;
> - }
> + (ch->ch_flags & CH_BREAK_SENDING))
> + goto exit_unlock;
>
> - if (!(ch->ch_flags & (CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM))) {
> - spin_unlock_irqrestore(&ch->ch_lock, flags);
> - return;
> - }
> + if (!(ch->ch_flags & (CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM)))
> + goto exit_unlock;
>
> n = 32;
>
> @@ -1094,6 +1088,7 @@ static void cls_copy_data_from_queue_to_uart(struct
> channel_t *ch)
> if (len_written > 0)
> ch->ch_flags &= ~(CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM);
>
> +exit_unlock:
> spin_unlock_irqrestore(&ch->ch_lock, flags);
> }
>
> diff --git a/drivers/staging/dgnc/dgnc_driver.c
> b/drivers/staging/dgnc/dgnc_driver.c
> index b7fd2bf..9387a0e 100644
> --- a/drivers/staging/dgnc/dgnc_driver.c
> +++ b/drivers/staging/dgnc/dgnc_driver.c
> @@ -572,30 +572,19 @@ static int dgnc_found_board(struct pci_dev *pdev, int
> id)
>
> rc = dgnc_tty_register(brd);
> if (rc < 0) {
> - dgnc_tty_uninit(brd);
> pr_err(DRVSTR ": Can't register tty devices (%d)\n", rc);
> - brd->state = BOARD_FAILED;
> - brd->dpastatus = BD_NOFEP;
> goto failed;
> }
>
> rc = dgnc_finalize_board_init(brd);
> if (rc < 0) {
> - dgnc_tty_uninit(brd);
> pr_err(DRVSTR ": Can't finalize board init (%d)\n", rc);
> - brd->state = BOARD_FAILED;
> - brd->dpastatus = BD_NOFEP;
> -
> goto failed;
> }
>
> rc = dgnc_tty_init(brd);
> if (rc < 0) {
> - dgnc_tty_uninit(brd);
> pr_err(DRVSTR ": Can't init tty devices (%d)\n", rc);
> - brd->state = BOARD_FAILED;
> - brd->dpastatus = BD_NOFEP;
> -
> goto failed;
> }
>
> @@ -629,6 +618,9 @@ static int dgnc_found_board(struct pci_dev *pdev, int id)
> return 0;
>
> failed:
> + dgnc_tty_uninit(brd);
> + brd->state = BOARD_FAILED;
> + brd->dpastatus = BD_NOFEP;
You'll probably have to remake this patch without this change because a
patch I've submitted a few days ago completely removes these and 2
other ones depend on it. It's still not in staging-testing for some reason :(
--
Thanks,
Giedrius
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v6 04/22] power_supply: Move run-time configuration to separate structure
On Tue, 10 Mar 2015, Krzysztof Kozlowski wrote: > Add new structure 'power_supply_config' for holding run-time > initialization data like of_node, supplies and private driver data. > > The power_supply_register() function is changed so all power supply > drivers need updating. > > When registering the power supply this new 'power_supply_config' should be > used instead of directly initializing 'struct power_supply'. This allows > changing the ownership of power_supply structure from driver to the > power supply core in next patches. > > When a driver does not use of_node or supplies then it should use NULL > as config. If driver uses of_node or supplies then it should allocate > config on stack and initialize it with proper values. > > Signed-off-by: Krzysztof Kozlowski > Reviewed-by: Bartlomiej Zolnierkiewicz > Acked-by: Pavel Machek > > [for the nvec part] > Reviewed-by: Marc Dietrich > > [for drivers/platform/x86/compal-laptop.c] > Reviewed-by: Darren Hart Reviewed-by: Jiri Kosina for changes in drivers/hid/* -- Jiri Kosina SUSE Labs ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 6/6] staging: sm750fb: Remove spinlock helper function
On 11 March 2015 at 14:58, Sudip Mukherjee wrote: > > tested the whole series on hardware, looks ok. > Great! > now it depends on Greg if he wants to accept patches with such big > commit logs, and another problem he will face is to get some patches from > this thread and some from the old thread. > (while testing i faced this problem to maintain the sequence of > patches) I am more than happy to resend the whole series as necessary if that makes it easier for Greg or anyone else :) I have previously resubmitted an entire patch series only for people to find the v2 resubmits of unchanged code to be confusing. I am absolutely happy to do whatever works for Greg of course! Best, -- Lorenzo Stoakes https:/ljs.io ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] Staging: dgnc: Use goto for spinlock release before return
spin_unlock_irqrestore() is called at several
different places before exiting. This patch uses a goto statement
to factorize these calls.
Coccinelle was used to generate this patch.
Signed-off-by: Quentin Lambert
---
drivers/staging/dgnc/dgnc_tty.c | 48 -
1 file changed, 23 insertions(+), 25 deletions(-)
diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index 8179342..2dcc51e 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -507,7 +507,7 @@ void dgnc_input(struct channel_t *ch)
{
struct dgnc_board *bd;
struct tty_struct *tp;
- struct tty_ldisc *ld;
+ struct tty_ldisc *ld = NULL;
uintrmask;
ushort head;
ushort tail;
@@ -539,10 +539,8 @@ void dgnc_input(struct channel_t *ch)
tail = ch->ch_r_tail & rmask;
data_len = (head - tail) & rmask;
- if (data_len == 0) {
- spin_unlock_irqrestore(&ch->ch_lock, flags);
- return;
- }
+ if (data_len == 0)
+ goto exit_unlock;
/*
* If the device is not open, or CREAD is off,
@@ -556,17 +554,14 @@ void dgnc_input(struct channel_t *ch)
/* Force queue flow control to be released, if needed */
dgnc_check_queue_flow_control(ch);
- spin_unlock_irqrestore(&ch->ch_lock, flags);
- return;
+ goto exit_unlock;
}
/*
* If we are throttled, simply don't read any data.
*/
- if (ch->ch_flags & CH_FORCED_STOPI) {
- spin_unlock_irqrestore(&ch->ch_lock, flags);
- return;
- }
+ if (ch->ch_flags & CH_FORCED_STOPI)
+ goto exit_unlock;
flip_len = TTY_FLIPBUF_SIZE;
@@ -604,12 +599,8 @@ void dgnc_input(struct channel_t *ch)
}
}
- if (len <= 0) {
- spin_unlock_irqrestore(&ch->ch_lock, flags);
- if (ld)
- tty_ldisc_deref(ld);
- return;
- }
+ if (len <= 0)
+ goto exit_unlock;
/*
* The tty layer in the kernel has changed in 2.6.16+.
@@ -677,6 +668,12 @@ void dgnc_input(struct channel_t *ch)
if (ld)
tty_ldisc_deref(ld);
+ return;
+
+exit_unlock:
+ if (ld)
+ tty_ldisc_deref(ld);
+ spin_unlock_irqrestore(&ch->ch_lock, flags);
}
@@ -1773,10 +1770,8 @@ static int dgnc_tty_write(struct tty_struct *tty,
/*
* Bail if no space left.
*/
- if (count <= 0) {
- spin_unlock_irqrestore(&ch->ch_lock, flags);
- return 0;
- }
+ if (count <= 0)
+ goto exit_retry;
/*
* Output the printer ON string, if we are in terminal mode, but
@@ -1803,10 +1798,8 @@ static int dgnc_tty_write(struct tty_struct *tty,
/*
* If there is nothing left to copy, or I can't handle any more data,
leave.
*/
- if (count <= 0) {
- spin_unlock_irqrestore(&ch->ch_lock, flags);
- return 0;
- }
+ if (count <= 0)
+ goto exit_retry;
if (from_user) {
@@ -1892,6 +1885,11 @@ static int dgnc_tty_write(struct tty_struct *tty,
}
return count;
+
+exit_retry:
+
+ spin_unlock_irqrestore(&ch->ch_lock, flags);
+ return 0;
}
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] Staging: dgnc: Use goto for error handling
This patch introduces goto statments for error handling
and in cases where a lock needs to be released.
A simplified version of the semantic patch that finds this problem is as
follows: (http://coccinelle.lip6.fr)
@candidates exists@
identifier f, label;
statement s;
position p1, p2, p3;
@@
f@p1(...) {
...when any
if@p2(...) {
...when any
s
return@p3 ...;
}
...when any
}
@good1 exists@
identifier candidates.f, candidates.label;
statement candidates.s;
position candidates.p1, candidates.p2;
@@
f@p1(...) {
...when any
if(...) {
...when any
s
return ...;
}
...when any
if@p2(...) {...}
...when any
}
@depends on good1@
identifier candidates.f, candidates.label;
position candidates.p1, candidates.p3;
@@
f@p1(...) {
...when any
* return@p3 ...;
}
Signed-off-by: Quentin Lambert
---
drivers/staging/dgnc/dgnc_cls.c| 19 +++
drivers/staging/dgnc/dgnc_driver.c | 14 +++---
drivers/staging/dgnc/dgnc_neo.c| 30 --
3 files changed, 22 insertions(+), 41 deletions(-)
diff --git a/drivers/staging/dgnc/dgnc_cls.c b/drivers/staging/dgnc/dgnc_cls.c
index bedc522..02f19f1 100644
--- a/drivers/staging/dgnc/dgnc_cls.c
+++ b/drivers/staging/dgnc/dgnc_cls.c
@@ -1029,22 +1029,16 @@ static void cls_copy_data_from_queue_to_uart(struct
channel_t *ch)
spin_lock_irqsave(&ch->ch_lock, flags);
/* No data to write to the UART */
- if (ch->ch_w_tail == ch->ch_w_head) {
- spin_unlock_irqrestore(&ch->ch_lock, flags);
- return;
- }
+ if (ch->ch_w_tail == ch->ch_w_head)
+ goto exit_unlock;
/* If port is "stopped", don't send any data to the UART */
if ((ch->ch_flags & CH_FORCED_STOP) ||
-(ch->ch_flags & CH_BREAK_SENDING)) {
- spin_unlock_irqrestore(&ch->ch_lock, flags);
- return;
- }
+(ch->ch_flags & CH_BREAK_SENDING))
+ goto exit_unlock;
- if (!(ch->ch_flags & (CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM))) {
- spin_unlock_irqrestore(&ch->ch_lock, flags);
- return;
- }
+ if (!(ch->ch_flags & (CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM)))
+ goto exit_unlock;
n = 32;
@@ -1094,6 +1088,7 @@ static void cls_copy_data_from_queue_to_uart(struct
channel_t *ch)
if (len_written > 0)
ch->ch_flags &= ~(CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM);
+exit_unlock:
spin_unlock_irqrestore(&ch->ch_lock, flags);
}
diff --git a/drivers/staging/dgnc/dgnc_driver.c
b/drivers/staging/dgnc/dgnc_driver.c
index b7fd2bf..9387a0e 100644
--- a/drivers/staging/dgnc/dgnc_driver.c
+++ b/drivers/staging/dgnc/dgnc_driver.c
@@ -572,30 +572,19 @@ static int dgnc_found_board(struct pci_dev *pdev, int id)
rc = dgnc_tty_register(brd);
if (rc < 0) {
- dgnc_tty_uninit(brd);
pr_err(DRVSTR ": Can't register tty devices (%d)\n", rc);
- brd->state = BOARD_FAILED;
- brd->dpastatus = BD_NOFEP;
goto failed;
}
rc = dgnc_finalize_board_init(brd);
if (rc < 0) {
- dgnc_tty_uninit(brd);
pr_err(DRVSTR ": Can't finalize board init (%d)\n", rc);
- brd->state = BOARD_FAILED;
- brd->dpastatus = BD_NOFEP;
-
goto failed;
}
rc = dgnc_tty_init(brd);
if (rc < 0) {
- dgnc_tty_uninit(brd);
pr_err(DRVSTR ": Can't init tty devices (%d)\n", rc);
- brd->state = BOARD_FAILED;
- brd->dpastatus = BD_NOFEP;
-
goto failed;
}
@@ -629,6 +618,9 @@ static int dgnc_found_board(struct pci_dev *pdev, int id)
return 0;
failed:
+ dgnc_tty_uninit(brd);
+ brd->state = BOARD_FAILED;
+ brd->dpastatus = BD_NOFEP;
return -ENXIO;
diff --git a/drivers/staging/dgnc/dgnc_neo.c b/drivers/staging/dgnc/dgnc_neo.c
index 1268aa9..4deae2d 100644
--- a/drivers/staging/dgnc/dgnc_neo.c
+++ b/drivers/staging/dgnc/dgnc_neo.c
@@ -1436,16 +1436,13 @@ static void neo_copy_data_from_queue_to_uart(struct
channel_t *ch)
spin_lock_irqsave(&ch->ch_lock, flags);
/* No data to write to the UART */
- if (ch->ch_w_tail == ch->ch_w_head) {
- spin_unlock_irqrestore(&ch->ch_lock, flags);
- return;
- }
+ if (ch->ch_w_tail == ch->ch_w_head)
+ goto exit_unlock;
/* If port is "stopped", don't send any data to the UART */
- if ((ch->ch_flags & CH_FORCED_STOP) || (ch->ch_flags &
CH_BREAK_SENDING)) {
- spin_unlock_irqrestore(&ch->ch_lock, flags);
- return;
- }
+ if ((ch->ch_flags & CH_FORCED_STOP) ||
+(ch->ch_flags & CH_BREAK_SENDING))
+
[PATCH 0/3] Introducing goto for error and lock handling
The first patch fix a missing call to dgnc_tty_uninit. The other two patches focus on the TODO item: use goto statements for error handling when appropriate The second patch resolves cases where the improvement seems clear. The third patch deals with cases where the added value did not seem as important or the changes felt more complex. Quentin Lambert (3): Staging: dgnc: dgnc_driver: Add a missing call to dgnc_tty_uninit Staging: dgnc: Use goto for error handling Staging: dgnc: Use goto for spinlock release before return drivers/staging/dgnc/dgnc_cls.c| 19 ++- drivers/staging/dgnc/dgnc_driver.c | 13 +++ drivers/staging/dgnc/dgnc_neo.c| 30 ++-- drivers/staging/dgnc/dgnc_tty.c| 48 ++ 4 files changed, 45 insertions(+), 65 deletions(-) -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: rtl8712: fix potential null pointer dereference
From: Matteo Semenzato
Check if kmalloc succeded before using the pointer in memcpy.
Signed-off-by: Matteo Semenzato
---
drivers/staging/rtl8712/rtl871x_mlme.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c
b/drivers/staging/rtl8712/rtl871x_mlme.c
index 977a833..c6e9012 100644
--- a/drivers/staging/rtl8712/rtl871x_mlme.c
+++ b/drivers/staging/rtl8712/rtl871x_mlme.c
@@ -727,6 +727,8 @@ void r8712_joinbss_event_callback(struct _adapter *adapter,
u8 *pbuf)
if (sizeof(struct list_head) == 4 * sizeof(u32)) {
pnetwork = kmalloc(sizeof(struct wlan_network), GFP_ATOMIC);
+ if (!pnetwork)
+ return;
memcpy((u8 *)pnetwork+16, (u8 *)pbuf + 8,
sizeof(struct wlan_network) - 16);
} else
--
2.3.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFCv2 08/21] Drivers: hv: util: introduce state machine for util drivers
KVP/VSS/FCOPY drivers work in fully serialized mode: we wait till userspace
daemon registers, wait for a message from the host, send this message to the
daemon, get the reply, send it back to host, wait for another message.
Introduce enum hvutil_device_state to represend this state in all 3 drivers.
Signed-off-by: Vitaly Kuznetsov
---
drivers/hv/hyperv_vmbus.h | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 177dbec..62efcce 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -724,4 +724,13 @@ static inline void hv_poll_channel(struct vmbus_channel
*channel,
cb(channel);
}
+enum hvutil_device_state {
+ HVUTIL_DEVICE_INIT = 0, /* driver is loaded, waiting for userspace */
+ HVUTIL_READY,/* userspace is registered */
+ HVUTIL_HOSTMSG_RECEIVED, /* message from the host was received */
+ HVUTIL_USERSPACE_REQ,/* request to userspace was sent */
+ HVUTIL_USERSPACE_RECV, /* reply from userspace was received */
+ HVUTIL_DEVICE_DYING, /* driver unload is in progress */
+};
+
#endif /* _HYPERV_VMBUS_H */
--
1.9.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFCv2 13/21] Drivers: hv: util: introduce hv_utils_transport abstraction
The intention is to make KVP/VSS drivers work through misc char devices.
Introduce an abstraction for kernel/userspace communication to make the
migration smoother. Transport operational mode (netlink or char device)
is determined by the first received message. To support driver upgrades
the switch from netlink to chardev operational mode is supported.
Every hv_util daemon is supposed to register 2 callbacks:
1) on_msg() to get notified when the userspace daemon sent a message;
2) on_reset() to get notified when the userspace daemon drops the connection.
Signed-off-by: Vitaly Kuznetsov
---
drivers/hv/Makefile | 2 +-
drivers/hv/hv_utils_transport.c | 276
drivers/hv/hv_utils_transport.h | 51
3 files changed, 328 insertions(+), 1 deletion(-)
create mode 100644 drivers/hv/hv_utils_transport.c
create mode 100644 drivers/hv/hv_utils_transport.h
diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
index 5e4dfa4..39c9b2c 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_fcopy.o
+hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o hv_fcopy.o hv_utils_transport.o
diff --git a/drivers/hv/hv_utils_transport.c b/drivers/hv/hv_utils_transport.c
new file mode 100644
index 000..ea7ba5e
--- /dev/null
+++ b/drivers/hv/hv_utils_transport.c
@@ -0,0 +1,276 @@
+/*
+ * Kernel/userspace transport abstraction for Hyper-V util driver.
+ *
+ * Copyright (C) 2015, Vitaly Kuznetsov
+ *
+ * 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.
+ *
+ */
+
+#include
+#include
+#include
+
+#include "hyperv_vmbus.h"
+#include "hv_utils_transport.h"
+
+static DEFINE_SPINLOCK(hvt_list_lock);
+static struct list_head hvt_list = LIST_HEAD_INIT(hvt_list);
+
+static void hvt_reset(struct hvutil_transport *hvt)
+{
+ mutex_lock(&hvt->outmsg_lock);
+ kfree(hvt->outmsg);
+ hvt->outmsg = NULL;
+ hvt->outmsg_len = 0;
+ mutex_unlock(&hvt->outmsg_lock);
+ if (hvt->on_reset)
+ hvt->on_reset();
+}
+
+static ssize_t hvt_op_read(struct file *file, char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ struct hvutil_transport *hvt;
+ int ret;
+
+ hvt = container_of(file->f_op, struct hvutil_transport, fops);
+
+ if (wait_event_interruptible(hvt->outmsg_q, hvt->outmsg_len > 0))
+ return -EINTR;
+
+ mutex_lock(&hvt->outmsg_lock);
+ if (!hvt->outmsg) {
+ ret = -EAGAIN;
+ goto out_unlock;
+ }
+
+ if (count < hvt->outmsg_len) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ if (!copy_to_user(buf, hvt->outmsg, hvt->outmsg_len))
+ ret = hvt->outmsg_len;
+ else
+ ret = -EFAULT;
+
+ kfree(hvt->outmsg);
+ hvt->outmsg = NULL;
+ hvt->outmsg_len = 0;
+
+out_unlock:
+ mutex_unlock(&hvt->outmsg_lock);
+ return ret;
+}
+
+static ssize_t hvt_op_write(struct file *file, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ struct hvutil_transport *hvt;
+ u8 *inmsg;
+
+ hvt = container_of(file->f_op, struct hvutil_transport, fops);
+
+ inmsg = kzalloc(count, GFP_KERNEL);
+ if (copy_from_user(inmsg, buf, count)) {
+ kfree(inmsg);
+ return -EFAULT;
+ }
+ if (hvt->on_msg(inmsg, count))
+ return -EFAULT;
+ kfree(inmsg);
+
+ return count;
+}
+
+static unsigned int hvt_op_poll(struct file *file, poll_table *wait)
+{
+ struct hvutil_transport *hvt;
+
+ hvt = container_of(file->f_op, struct hvutil_transport, fops);
+
+ poll_wait(file, &hvt->outmsg_q, wait);
+ if (hvt->outmsg_len > 0)
+ return POLLIN | POLLRDNORM;
+
+ return 0;
+}
+
+static int hvt_op_open(struct inode *inode, struct file *file)
+{
+ struct hvutil_transport *hvt;
+
+ hvt = container_of(file->f_op, struct hvutil_transport, fops);
+
+ /*
+* Switching to CHARDEV mode. We switch bach to INIT when device
+* gets released.
+*/
+ if (hvt->mode == HVUTIL_TRANSPORT_INIT)
+ hvt->mode = HVUTIL_TRANSPORT_CHARDEV;
+ else if (hvt->mode == HVUTIL_TRANSPORT_NETLINK) {
+ /*
+* We
[PATCH RFCv2 12/21] Drivers: hv: fcopy: set .owner reference for file operations
... otherwise a crash is observed when hv_utils module is being unloaded while
fcopy daemon is still running. .owner gives us an additional reference when
someone holds a descriptor for the device.
Signed-off-by: Vitaly Kuznetsov
---
drivers/hv/hv_fcopy.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
index a501301..d1475e6 100644
--- a/drivers/hv/hv_fcopy.c
+++ b/drivers/hv/hv_fcopy.c
@@ -360,12 +360,9 @@ static int fcopy_open(struct inode *inode, struct file *f)
}
/* XXX: there are still some tricky corner cases, e.g.,
- * 1) In a SMP guest, when fcopy_release() runs between
+ * In an SMP guest, when fcopy_release() runs between
* schedule_delayed_work() and fcopy_send_data(), there is
* still a chance an obsolete message will be queued.
- *
- * 2) When the fcopy daemon is running, if we unload the driver,
- * we'll notice a kernel oops when we kill the daemon later.
*/
static int fcopy_release(struct inode *inode, struct file *f)
{
@@ -385,6 +382,7 @@ static int fcopy_release(struct inode *inode, struct file
*f)
static const struct file_operations fcopy_fops = {
+ .owner = THIS_MODULE,
.read = fcopy_read,
.write = fcopy_write,
.release= fcopy_release,
--
1.9.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFCv2 03/21] Drivers: hv: kvp: move poll_channel() to hyperv_vmbus.h
..., make it inline and rename it to hv_poll_channel() so it can be reused
in other hv_util modules.
Signed-off-by: Vitaly Kuznetsov
---
drivers/hv/hv_kvp.c | 17 +++--
drivers/hv/hyperv_vmbus.h | 12
2 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index caa467d..939c3e7 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -128,17 +128,6 @@ kvp_work_func(struct work_struct *dummy)
kvp_respond_to_host(NULL, HV_E_FAIL);
}
-static void poll_channel(struct vmbus_channel *channel)
-{
- if (channel->target_cpu != smp_processor_id())
- smp_call_function_single(channel->target_cpu,
-hv_kvp_onchannelcallback,
-channel, true);
- else
- hv_kvp_onchannelcallback(channel);
-}
-
-
static int kvp_handle_handshake(struct hv_kvp_msg *msg)
{
int ret = 1;
@@ -166,8 +155,8 @@ static int kvp_handle_handshake(struct hv_kvp_msg *msg)
pr_info("KVP: user-mode registering done.\n");
kvp_register(dm_reg_value);
kvp_transaction.active = false;
- if (kvp_transaction.kvp_context)
- poll_channel(kvp_transaction.kvp_context);
+ hv_poll_channel(kvp_transaction.kvp_context,
+ hv_kvp_onchannelcallback);
}
return ret;
}
@@ -587,7 +576,7 @@ response_done:
vmbus_sendpacket(channel, recv_buffer, buf_len, req_id,
VM_PKT_DATA_INBAND, 0);
- poll_channel(channel);
+ hv_poll_channel(channel, hv_kvp_onchannelcallback);
}
/*
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 0d85dd1..177dbec 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -711,5 +711,17 @@ int hv_fcopy_init(struct hv_util_service *);
void hv_fcopy_deinit(void);
void hv_fcopy_onchannelcallback(void *);
+static inline void hv_poll_channel(struct vmbus_channel *channel,
+ void (*cb)(void *))
+{
+ if (!channel)
+ return;
+
+ if (channel->target_cpu != smp_processor_id())
+ smp_call_function_single(channel->target_cpu,
+cb, channel, true);
+ else
+ cb(channel);
+}
#endif /* _HYPERV_VMBUS_H */
--
1.9.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFCv2 19/21] Drivers: hv: vss: full handshake support
Introduce VSS_OP_REGISTER1 to support kernel replying to the negotiation
message with its own version.
Signed-off-by: Vitaly Kuznetsov
---
drivers/hv/hv_snapshot.c| 49 ++---
include/uapi/linux/hyperv.h | 5 +
tools/hv/hv_vss_daemon.c| 14 +
3 files changed, 56 insertions(+), 12 deletions(-)
diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c
index 2c8c246..ee1762b 100644
--- a/drivers/hv/hv_snapshot.c
+++ b/drivers/hv/hv_snapshot.c
@@ -59,6 +59,11 @@ static struct {
static void vss_respond_to_host(int error);
+/*
+ * This state maintains the version number registered by the daemon.
+ */
+static int dm_reg_value;
+
static const char vss_devname[] = "vmbus/hv_vss";
static __u8 *recv_buffer;
static struct hvutil_transport *hvt;
@@ -89,6 +94,29 @@ static void vss_timeout_func(struct work_struct *dummy)
hv_vss_onchannelcallback);
}
+static int vss_handle_handshake(struct hv_vss_msg *vss_msg)
+{
+ u32 our_ver = VSS_OP_REGISTER1;
+
+ switch (vss_msg->vss_hdr.operation) {
+ case VSS_OP_REGISTER:
+ /* Daemon doesn't expect us to reply */
+ dm_reg_value = VSS_OP_REGISTER;
+ break;
+ case VSS_OP_REGISTER1:
+ /* Daemon expects us to reply with our own version*/
+ if (hvutil_transport_send(hvt, &our_ver, sizeof(our_ver)))
+ return -EFAULT;
+ dm_reg_value = VSS_OP_REGISTER1;
+ break;
+ default:
+ return -EINVAL;
+ }
+ vss_transaction.state = HVUTIL_READY;
+ pr_info("VSS daemon registered\n");
+ return 0;
+}
+
static int vss_on_msg(void *msg, int len)
{
struct hv_vss_msg *vss_msg = (struct hv_vss_msg *)msg;
@@ -96,18 +124,15 @@ static int vss_on_msg(void *msg, int len)
if (len != sizeof(*vss_msg))
return -EINVAL;
- /*
-* Don't process registration messages if we're in the middle of
-* a transaction processing.
-*/
- if (vss_transaction.state > HVUTIL_READY &&
- vss_msg->vss_hdr.operation == VSS_OP_REGISTER)
- return -EINVAL;
-
- if (vss_transaction.state == HVUTIL_DEVICE_INIT &&
- vss_msg->vss_hdr.operation == VSS_OP_REGISTER) {
- pr_info("VSS daemon registered\n");
- vss_transaction.state = HVUTIL_READY;
+ if (vss_msg->vss_hdr.operation == VSS_OP_REGISTER ||
+ vss_msg->vss_hdr.operation == VSS_OP_REGISTER1) {
+ /*
+* Don't process registration messages if we're in the middle
+* of a transaction processing.
+*/
+ if (vss_transaction.state > HVUTIL_READY)
+ return -EINVAL;
+ return vss_handle_handshake(vss_msg);
} else if (vss_transaction.state == HVUTIL_USERSPACE_REQ) {
vss_transaction.state = HVUTIL_USERSPACE_RECV;
if (cancel_delayed_work_sync(&vss_timeout_work)) {
diff --git a/include/uapi/linux/hyperv.h b/include/uapi/linux/hyperv.h
index bb1cb73..66c76df 100644
--- a/include/uapi/linux/hyperv.h
+++ b/include/uapi/linux/hyperv.h
@@ -45,6 +45,11 @@
#define VSS_OP_REGISTER 128
+/*
+ Daemon code with full handshake support.
+ */
+#define VSS_OP_REGISTER1 129
+
enum hv_vss_op {
VSS_OP_CREATE = 0,
VSS_OP_DELETE,
diff --git a/tools/hv/hv_vss_daemon.c b/tools/hv/hv_vss_daemon.c
index 135425a..f5db9f0 100644
--- a/tools/hv/hv_vss_daemon.c
+++ b/tools/hv/hv_vss_daemon.c
@@ -142,6 +142,8 @@ int main(int argc, char *argv[])
int op;
struct hv_vss_msg vss_msg[1];
int daemonize = 1, long_index = 0, opt;
+ int in_handshake = 1;
+ __u32 kernel_modver;
static struct option long_options[] = {
{"help",no_argument, 0, 'h' },
@@ -205,6 +207,18 @@ int main(int argc, char *argv[])
len = read(vss_fd, vss_msg, sizeof(struct hv_vss_msg));
+ if (in_handshake) {
+ if (len != sizeof(kernel_modver)) {
+ syslog(LOG_ERR, "invalid version negotiation");
+ exit(EXIT_FAILURE);
+ }
+ kernel_modver = *(__u32 *)vss_msg;
+ in_handshake = 0;
+ syslog(LOG_INFO, "VSS: kernel module version: %d",
+ kernel_modver);
+ continue;
+ }
+
if (len != sizeof(struct hv_vss_msg)) {
syslog(LOG_ERR, "read failed; error:%d %s",
errno, strerror(errno));
--
1.9.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFCv2 20/21] Drivers: hv: fcopy: full handshake support
Introduce FCOPY_VERSION_1 to support kernel replying to the negotiation
message with its own version.
Signed-off-by: Vitaly Kuznetsov
---
drivers/hv/hv_fcopy.c | 16 +++-
include/uapi/linux/hyperv.h | 3 ++-
tools/hv/hv_fcopy_daemon.c | 15 +++
3 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
index 6a8ec9f..b7b528c 100644
--- a/drivers/hv/hv_fcopy.c
+++ b/drivers/hv/hv_fcopy.c
@@ -62,6 +62,10 @@ static DECLARE_WORK(fcopy_send_work, fcopy_send_data);
static const char fcopy_devname[] = "vmbus/hv_fcopy";
static u8 *recv_buffer;
static struct hvutil_transport *hvt;
+/*
+ * This state maintains the version number registered by the daemon.
+ */
+static int dm_reg_value;
static void fcopy_timeout_func(struct work_struct *dummy)
{
@@ -81,8 +85,18 @@ static void fcopy_timeout_func(struct work_struct *dummy)
static int fcopy_handle_handshake(u32 version)
{
+ u32 our_ver = FCOPY_CURRENT_VERSION;
+
switch (version) {
- case FCOPY_CURRENT_VERSION:
+ case FCOPY_VERSION_0:
+ /* Daemon doesn't expect us to reply */
+ dm_reg_value = version;
+ break;
+ case FCOPY_VERSION_1:
+ /* Daemon expects us to reply with our own version */
+ if (hvutil_transport_send(hvt, &our_ver, sizeof(our_ver)))
+ return -EFAULT;
+ dm_reg_value = version;
break;
default:
/*
diff --git a/include/uapi/linux/hyperv.h b/include/uapi/linux/hyperv.h
index 66c76df..e4c0a35 100644
--- a/include/uapi/linux/hyperv.h
+++ b/include/uapi/linux/hyperv.h
@@ -105,7 +105,8 @@ struct hv_vss_msg {
*/
#define FCOPY_VERSION_0 0
-#define FCOPY_CURRENT_VERSION FCOPY_VERSION_0
+#define FCOPY_VERSION_1 1
+#define FCOPY_CURRENT_VERSION FCOPY_VERSION_1
#define W_MAX_PATH 260
enum hv_fcopy_op {
diff --git a/tools/hv/hv_fcopy_daemon.c b/tools/hv/hv_fcopy_daemon.c
index 9445d8f..5480e4e 100644
--- a/tools/hv/hv_fcopy_daemon.c
+++ b/tools/hv/hv_fcopy_daemon.c
@@ -137,6 +137,8 @@ int main(int argc, char *argv[])
int version = FCOPY_CURRENT_VERSION;
char *buffer[4096 * 2];
struct hv_fcopy_hdr *in_msg;
+ int in_handshake = 1;
+ __u32 kernel_modver;
static struct option long_options[] = {
{"help",no_argument, 0, 'h' },
@@ -191,6 +193,19 @@ int main(int argc, char *argv[])
syslog(LOG_ERR, "pread failed: %s", strerror(errno));
exit(EXIT_FAILURE);
}
+
+ if (in_handshake) {
+ if (len != sizeof(kernel_modver)) {
+ syslog(LOG_ERR, "invalid version negotiation");
+ exit(EXIT_FAILURE);
+ }
+ kernel_modver = *(__u32 *)buffer;
+ in_handshake = 0;
+ syslog(LOG_INFO, "HV_FCOPY: kernel module version: %d",
+ kernel_modver);
+ continue;
+ }
+
in_msg = (struct hv_fcopy_hdr *)buffer;
switch (in_msg->operation) {
--
1.9.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFCv2 05/21] Drivers: hv: vss: process deferred messages when we complete the transaction
In theory, the host is not supposed to issue any requests before be reply to
the previous one. In KVP we, however, support the following scenarios:
1) A message was received before userspace daemon registered;
2) A message was received while the previous one is still being processed.
In VSS we support only the former. Add support for the later, use
hv_poll_channel() to do the job.
Signed-off-by: Vitaly Kuznetsov
---
drivers/hv/hv_snapshot.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c
index c1a3604..4bb9b1c 100644
--- a/drivers/hv/hv_snapshot.c
+++ b/drivers/hv/hv_snapshot.c
@@ -47,6 +47,7 @@ static struct {
struct vmbus_channel *recv_channel; /* chn we got the request */
u64 recv_req_id; /* request ID. */
struct hv_vss_msg *msg; /* current message */
+ void *vss_context; /* for the channel callback */
} vss_transaction;
@@ -73,6 +74,9 @@ static void vss_timeout_func(struct work_struct *dummy)
*/
pr_warn("VSS: timeout waiting for daemon to reply\n");
vss_respond_to_host(HV_E_FAIL);
+
+ hv_poll_channel(vss_transaction.vss_context,
+ hv_vss_onchannelcallback);
}
static void
@@ -85,13 +89,12 @@ vss_cn_callback(struct cn_msg *msg, struct
netlink_skb_parms *nsp)
if (vss_msg->vss_hdr.operation == VSS_OP_REGISTER) {
pr_info("VSS daemon registered\n");
vss_transaction.active = false;
- if (vss_transaction.recv_channel != NULL)
- hv_vss_onchannelcallback(vss_transaction.recv_channel);
- return;
-
}
if (cancel_delayed_work_sync(&vss_timeout_work))
vss_respond_to_host(vss_msg->error);
+
+ hv_poll_channel(vss_transaction.vss_context,
+ hv_vss_onchannelcallback);
}
@@ -198,9 +201,10 @@ void hv_vss_onchannelcallback(void *context)
* We will defer processing this callback once
* the current transaction is complete.
*/
- vss_transaction.recv_channel = channel;
+ vss_transaction.vss_context = context;
return;
}
+ vss_transaction.vss_context = NULL;
vmbus_recvpacket(channel, recv_buffer, PAGE_SIZE * 2, &recvlen,
&requestid);
--
1.9.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFCv2 07/21] Drivers: hv: fcopy: rename fcopy_work -> fcopy_timeout_work
'fcopy_work' (and fcopy_work_func) is a misnomer as it sounds like we expect
this useful work to happen and in reality it is just an emergency escape when
timeout happens.
Signed-off-by: Vitaly Kuznetsov
---
drivers/hv/hv_fcopy.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
index 8bdf752..c14f0f4 100644
--- a/drivers/hv/hv_fcopy.c
+++ b/drivers/hv/hv_fcopy.c
@@ -75,11 +75,11 @@ static bool opened; /* currently device opened */
static bool in_hand_shake = true;
static void fcopy_send_data(void);
static void fcopy_respond_to_host(int error);
-static void fcopy_work_func(struct work_struct *dummy);
-static DECLARE_DELAYED_WORK(fcopy_work, fcopy_work_func);
+static void fcopy_timeout_func(struct work_struct *dummy);
+static DECLARE_DELAYED_WORK(fcopy_timeout_work, fcopy_timeout_func);
static u8 *recv_buffer;
-static void fcopy_work_func(struct work_struct *dummy)
+static void fcopy_timeout_func(struct work_struct *dummy)
{
/*
* If the timer fires, the user-mode component has not responded;
@@ -261,7 +261,7 @@ void hv_fcopy_onchannelcallback(void *context)
/*
* Send the information to the user-level daemon.
*/
- schedule_delayed_work(&fcopy_work, 5*HZ);
+ schedule_delayed_work(&fcopy_timeout_work, 5*HZ);
fcopy_send_data();
return;
}
@@ -336,7 +336,7 @@ static ssize_t fcopy_write(struct file *file, const char
__user *buf,
* Complete the transaction by forwarding the result
* to the host. But first, cancel the timeout.
*/
- if (cancel_delayed_work_sync(&fcopy_work)) {
+ if (cancel_delayed_work_sync(&fcopy_timeout_work))
fcopy_respond_to_host(response);
hv_poll_channel(fcopy_transaction.fcopy_context,
hv_fcopy_onchannelcallback);
@@ -378,7 +378,7 @@ static int fcopy_release(struct inode *inode, struct file
*f)
in_hand_shake = true;
opened = false;
- if (cancel_delayed_work_sync(&fcopy_work)) {
+ if (cancel_delayed_work_sync(&fcopy_timeout_work)) {
/* We haven't up()-ed the semaphore(very rare)? */
if (down_trylock(&fcopy_transaction.read_sema))
;
@@ -442,6 +442,6 @@ int hv_fcopy_init(struct hv_util_service *srv)
void hv_fcopy_deinit(void)
{
- cancel_delayed_work_sync(&fcopy_work);
+ cancel_delayed_work_sync(&fcopy_timeout_work);
fcopy_dev_deinit();
}
--
1.9.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFCv2 16/21] Drivers: hv: kvp: convert to hv_utils_transport
... to support both netlink and /dev/vmbus/hv_kvp communication methods.
Signed-off-by: Vitaly Kuznetsov
---
drivers/hv/hv_kvp.c | 91 +
1 file changed, 42 insertions(+), 49 deletions(-)
diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index a70d202..baa1208 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -29,6 +29,7 @@
#include
#include "hyperv_vmbus.h"
+#include "hv_utils_transport.h"
/*
* Pre win8 version numbers used in ws2008 and ws 2008 r2 (win7)
@@ -83,9 +84,9 @@ static void kvp_register(int);
static DECLARE_DELAYED_WORK(kvp_timeout_work, kvp_timeout_func);
static DECLARE_WORK(kvp_sendkey_work, kvp_send_key);
-static struct cb_id kvp_id = { CN_KVP_IDX, CN_KVP_VAL };
-static const char kvp_name[] = "kvp_kernel_module";
+static const char kvp_devname[] = "vmbus/hv_kvp";
static u8 *recv_buffer;
+static struct hvutil_transport *hvt;
/*
* Register the kernel component with the user-level daemon.
* As part of this registration, pass the LIC version number.
@@ -97,23 +98,18 @@ static void
kvp_register(int reg_value)
{
- struct cn_msg *msg;
struct hv_kvp_msg *kvp_msg;
char *version;
- msg = kzalloc(sizeof(*msg) + sizeof(struct hv_kvp_msg), GFP_ATOMIC);
+ kvp_msg = kzalloc(sizeof(*kvp_msg), GFP_KERNEL);
- if (msg) {
- kvp_msg = (struct hv_kvp_msg *)msg->data;
+ if (kvp_msg) {
version = kvp_msg->body.kvp_register.version;
- msg->id.idx = CN_KVP_IDX;
- msg->id.val = CN_KVP_VAL;
-
kvp_msg->kvp_hdr.operation = reg_value;
strcpy(version, HV_DRV_VERSION);
- msg->len = sizeof(struct hv_kvp_msg);
- cn_netlink_send(msg, 0, 0, GFP_ATOMIC);
- kfree(msg);
+
+ hvutil_transport_send(hvt, kvp_msg, sizeof(*kvp_msg));
+ kfree(kvp_msg);
}
}
@@ -135,8 +131,6 @@ static void kvp_timeout_func(struct work_struct *dummy)
static int kvp_handle_handshake(struct hv_kvp_msg *msg)
{
- int ret = 1;
-
switch (msg->kvp_hdr.operation) {
case KVP_OP_REGISTER:
dm_reg_value = KVP_OP_REGISTER;
@@ -150,18 +144,17 @@ static int kvp_handle_handshake(struct hv_kvp_msg *msg)
pr_info("KVP: incompatible daemon\n");
pr_info("KVP: KVP version: %d, Daemon version: %d\n",
KVP_OP_REGISTER1, msg->kvp_hdr.operation);
- ret = 0;
+ return -EINVAL;
}
- if (ret) {
- /*
-* We have a compatible daemon; complete the handshake.
-*/
- pr_info("KVP: user-mode registering done.\n");
- kvp_register(dm_reg_value);
- kvp_transaction.state = HVUTIL_READY;
- }
- return ret;
+ /*
+* We have a compatible daemon; complete the handshake.
+*/
+ pr_info("KVP: user-mode registering done.\n");
+ kvp_register(dm_reg_value);
+ kvp_transaction.state = HVUTIL_READY;
+
+ return 0;
}
@@ -169,14 +162,14 @@ static int kvp_handle_handshake(struct hv_kvp_msg *msg)
* Callback when data is received from user mode.
*/
-static void
-kvp_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
+static int kvp_on_msg(void *msg, int len)
{
- struct hv_kvp_msg *message;
+ struct hv_kvp_msg *message = (struct hv_kvp_msg *)msg;
struct hv_kvp_msg_enumerate *data;
int error = 0;
- message = (struct hv_kvp_msg *)msg->data;
+ if (len < sizeof(*message))
+ return -EINVAL;
/*
* If we are negotiating the version information
@@ -184,13 +177,13 @@ kvp_cn_callback(struct cn_msg *msg, struct
netlink_skb_parms *nsp)
*/
if (kvp_transaction.state < HVUTIL_READY) {
- kvp_handle_handshake(message);
- return;
+ return kvp_handle_handshake(message);
}
/* We didn't send anything to userspace so the reply is spurious */
if (kvp_transaction.state < HVUTIL_USERSPACE_REQ)
- return;
+ return -EINVAL;
+
kvp_transaction.state = HVUTIL_USERSPACE_RECV;
/*
@@ -228,6 +221,8 @@ kvp_cn_callback(struct cn_msg *msg, struct
netlink_skb_parms *nsp)
hv_poll_channel(kvp_transaction.kvp_context,
hv_kvp_onchannelcallback);
}
+
+ return 0;
}
@@ -344,7 +339,6 @@ static void process_ib_ipinfo(void *in_msg, void *out_msg,
int op)
static void
kvp_send_key(struct work_struct *dummy)
{
- struct cn_msg *msg;
struct hv_kvp_msg *message;
struct hv_kvp_msg *in_msg;
__u8 operation = kvp_transaction.kvp_msg->kvp_hdr.operation;
@@ -357,14 +351,7 @@ kvp_send_key(struct work_struct *dummy)
if (kvp_transaction.state != HVUT
[PATCH RFCv2 15/21] Drivers: hv: fcopy: convert to hv_utils_transport
Unify the code with the recently introduced hv_utils_transport. Netlink
communication is disabled for fcopy.
Signed-off-by: Vitaly Kuznetsov
---
drivers/hv/hv_fcopy.c | 194 --
1 file changed, 46 insertions(+), 148 deletions(-)
diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
index d1475e6..6a8ec9f 100644
--- a/drivers/hv/hv_fcopy.c
+++ b/drivers/hv/hv_fcopy.c
@@ -19,17 +19,13 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-#include
-#include
#include
#include
-#include
#include
#include
-#include
-#include
#include "hyperv_vmbus.h"
+#include "hv_utils_transport.h"
#define WIN8_SRV_MAJOR 1
#define WIN8_SRV_MINOR 1
@@ -53,18 +49,19 @@ static struct {
int state; /* hvutil_device_state */
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 void fcopy_send_data(void);
static void fcopy_respond_to_host(int error);
+static void fcopy_send_data(struct work_struct *dummy);
static void fcopy_timeout_func(struct work_struct *dummy);
static DECLARE_DELAYED_WORK(fcopy_timeout_work, fcopy_timeout_func);
+static DECLARE_WORK(fcopy_send_work, fcopy_send_data);
+static const char fcopy_devname[] = "vmbus/hv_fcopy";
static u8 *recv_buffer;
+static struct hvutil_transport *hvt;
static void fcopy_timeout_func(struct work_struct *dummy)
{
@@ -78,17 +75,6 @@ static void fcopy_timeout_func(struct work_struct *dummy)
if (fcopy_transaction.state > HVUTIL_READY)
fcopy_transaction.state = HVUTIL_READY;
- /* In the case the user-space daemon crashes, hangs or is killed, we
-* need to down the semaphore, otherwise, after the daemon starts next
-* time, the obsolete data in fcopy_transaction.message or
-* fcopy_transaction.fcopy_msg will be used immediately.
-*
-* NOTE: fcopy_read() happens to get the semaphore (very rare)? We're
-* still OK, because we've reported the failure to the host.
-*/
- if (down_trylock(&fcopy_transaction.read_sema))
- ;
-
hv_poll_channel(fcopy_transaction.fcopy_context,
hv_fcopy_onchannelcallback);
}
@@ -115,11 +101,13 @@ static int fcopy_handle_handshake(u32 version)
return 0;
}
-static void fcopy_send_data(void)
+static void fcopy_send_data(struct work_struct *dummy)
{
- struct hv_start_fcopy *smsg_out = &fcopy_transaction.message;
+ struct hv_start_fcopy smsg_out;
int operation = fcopy_transaction.fcopy_msg->operation;
struct hv_start_fcopy *smsg_in;
+ void *out_src;
+ int rc, out_len;
/*
* The strings sent from the host are encoded in
@@ -134,26 +122,39 @@ static void fcopy_send_data(void)
switch (operation) {
case START_FILE_COPY:
- memset(smsg_out, 0, sizeof(struct hv_start_fcopy));
- smsg_out->hdr.operation = operation;
+ out_len = sizeof(struct hv_start_fcopy);
+ memset(&smsg_out, 0, out_len);
+ smsg_out.hdr.operation = operation;
smsg_in = (struct hv_start_fcopy *)fcopy_transaction.fcopy_msg;
utf16s_to_utf8s((wchar_t *)smsg_in->file_name, W_MAX_PATH,
UTF16_LITTLE_ENDIAN,
- (__u8 *)smsg_out->file_name, W_MAX_PATH - 1);
+ (__u8 *)&smsg_out.file_name, W_MAX_PATH - 1);
utf16s_to_utf8s((wchar_t *)smsg_in->path_name, W_MAX_PATH,
UTF16_LITTLE_ENDIAN,
- (__u8 *)smsg_out->path_name, W_MAX_PATH - 1);
+ (__u8 *)&smsg_out.path_name, W_MAX_PATH - 1);
- smsg_out->copy_flags = smsg_in->copy_flags;
- smsg_out->file_size = smsg_in->file_size;
+ smsg_out.copy_flags = smsg_in->copy_flags;
+ smsg_out.file_size = smsg_in->file_size;
+ out_src = &smsg_out;
break;
default:
+ out_src = fcopy_transaction.fcopy_msg;
+ out_len = fcopy_transaction.recv_len;
break;
}
- up(&fcopy_transaction.read_sema);
+
+ fcopy_transaction.state = HVUTIL_USERSPACE_REQ;
+ rc = hvutil_transport_send(hvt, out_src, out_len);
+ if (rc) {
+ pr_debug("FCP: failed to communicate to the daemon: %d\n", rc);
+ if (cancel_delayed_work_sync(&fcopy_timeout_work)) {
+ fcopy_respond_to_host(HV_E_FAIL);
+ fcop
[PATCH RFCv2 17/21] Tools: hv: kvp: use misc char device to communicate with kernel
Use /dev/vmbus/hv_kvp instead of netlink.
Signed-off-by: Vitaly Kuznetsov
---
tools/hv/hv_kvp_daemon.c | 166 +--
1 file changed, 31 insertions(+), 135 deletions(-)
diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 408bb07..0d9f48e 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -33,7 +33,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -79,7 +78,6 @@ enum {
DNS
};
-static struct sockaddr_nl addr;
static int in_hand_shake = 1;
static char *os_name = "";
@@ -1387,34 +1385,6 @@ kvp_get_domain_name(char *buffer, int length)
freeaddrinfo(info);
}
-static int
-netlink_send(int fd, struct cn_msg *msg)
-{
- struct nlmsghdr nlh = { .nlmsg_type = NLMSG_DONE };
- unsigned int size;
- struct msghdr message;
- struct iovec iov[2];
-
- size = sizeof(struct cn_msg) + msg->len;
-
- nlh.nlmsg_pid = getpid();
- nlh.nlmsg_len = NLMSG_LENGTH(size);
-
- iov[0].iov_base = &nlh;
- iov[0].iov_len = sizeof(nlh);
-
- iov[1].iov_base = msg;
- iov[1].iov_len = size;
-
- memset(&message, 0, sizeof(message));
- message.msg_name = &addr;
- message.msg_namelen = sizeof(addr);
- message.msg_iov = iov;
- message.msg_iovlen = 2;
-
- return sendmsg(fd, &message, 0);
-}
-
void print_usage(char *argv[])
{
fprintf(stderr, "Usage: %s [options]\n"
@@ -1425,22 +1395,17 @@ void print_usage(char *argv[])
int main(int argc, char *argv[])
{
- int fd, len, nl_group;
+ int kvp_fd, len;
int error;
- struct cn_msg *message;
struct pollfd pfd;
- struct nlmsghdr *incoming_msg;
- struct cn_msg *incoming_cn_msg;
- struct hv_kvp_msg *hv_msg;
- char*p;
+ char*p;
+ struct hv_kvp_msg hv_msg[1];
char*key_value;
char*key_name;
int op;
int pool;
char*if_name;
struct hv_kvp_ipaddr_value *kvp_ip_val;
- char *kvp_recv_buffer;
- size_t kvp_recv_buffer_len;
int daemonize = 1, long_index = 0, opt;
static struct option long_options[] = {
@@ -1468,12 +1433,14 @@ int main(int argc, char *argv[])
openlog("KVP", 0, LOG_USER);
syslog(LOG_INFO, "KVP starting; pid is:%d", getpid());
- kvp_recv_buffer_len = NLMSG_LENGTH(0) + sizeof(struct cn_msg) +
sizeof(struct hv_kvp_msg);
- kvp_recv_buffer = calloc(1, kvp_recv_buffer_len);
- if (!kvp_recv_buffer) {
- syslog(LOG_ERR, "Failed to allocate netlink buffer");
+ kvp_fd = open("/dev/vmbus/hv_kvp", O_RDWR);
+
+ if (kvp_fd < 0) {
+ syslog(LOG_ERR, "open /dev/vmbus/hv_kvp failed; error: %d %s",
+ errno, strerror(errno));
exit(EXIT_FAILURE);
}
+
/*
* Retrieve OS release information.
*/
@@ -1489,100 +1456,44 @@ int main(int argc, char *argv[])
exit(EXIT_FAILURE);
}
- fd = socket(AF_NETLINK, SOCK_DGRAM, NETLINK_CONNECTOR);
- if (fd < 0) {
- syslog(LOG_ERR, "netlink socket creation failed; error: %d %s",
errno,
- strerror(errno));
- exit(EXIT_FAILURE);
- }
- addr.nl_family = AF_NETLINK;
- addr.nl_pad = 0;
- addr.nl_pid = 0;
- addr.nl_groups = 0;
-
-
- error = bind(fd, (struct sockaddr *)&addr, sizeof(addr));
- if (error < 0) {
- syslog(LOG_ERR, "bind failed; error: %d %s", errno,
strerror(errno));
- close(fd);
- exit(EXIT_FAILURE);
- }
- nl_group = CN_KVP_IDX;
-
- if (setsockopt(fd, SOL_NETLINK, NETLINK_ADD_MEMBERSHIP, &nl_group,
sizeof(nl_group)) < 0) {
- syslog(LOG_ERR, "setsockopt failed; error: %d %s", errno,
strerror(errno));
- close(fd);
- exit(EXIT_FAILURE);
- }
-
/*
* Register ourselves with the kernel.
*/
- message = (struct cn_msg *)kvp_recv_buffer;
- message->id.idx = CN_KVP_IDX;
- message->id.val = CN_KVP_VAL;
-
- hv_msg = (struct hv_kvp_msg *)message->data;
hv_msg->kvp_hdr.operation = KVP_OP_REGISTER1;
- message->ack = 0;
- message->len = sizeof(struct hv_kvp_msg);
-
- len = netlink_send(fd, message);
- if (len < 0) {
- syslog(LOG_ERR, "netlink_send failed; error: %d %s", errno,
strerror(errno));
- close(fd);
+ len = write(kvp_fd, hv_msg, sizeof(struct hv_kvp_msg));
+ if (len != sizeof(struct hv_kvp_msg)) {
+ syslog(LOG_ERR, "registration to kernel failed; error: %d %s",
+ errno, strerror(errno));
+ close(kvp_fd);
exit(EXIT_FAILURE);
}
- pfd.fd = fd;
+ pfd.fd = kvp_fd;
while (1) {
[PATCH RFCv2 04/21] Drivers: hv: fcopy: process deferred messages when we complete the transaction
In theory, the host is not supposed to issue any requests before be reply to
the previous one. In KVP we, however, support the following scenarios:
1) A message was received before userspace daemon registered;
2) A message was received while the previous one is still being processed.
In FCOPY we support only the former. Add support for the later, use
hv_poll_channel() to do the job.
Signed-off-by: Vitaly Kuznetsov
---
drivers/hv/hv_fcopy.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
index cd453e4..8bdf752 100644
--- a/drivers/hv/hv_fcopy.c
+++ b/drivers/hv/hv_fcopy.c
@@ -98,6 +98,8 @@ static void fcopy_work_func(struct work_struct *dummy)
if (down_trylock(&fcopy_transaction.read_sema))
;
+ hv_poll_channel(fcopy_transaction.fcopy_context,
+ hv_fcopy_onchannelcallback);
}
static int fcopy_handle_handshake(u32 version)
@@ -117,8 +119,8 @@ static int fcopy_handle_handshake(u32 version)
pr_info("FCP: user-mode registering done. Daemon version: %d\n",
version);
fcopy_transaction.active = false;
- if (fcopy_transaction.fcopy_context)
- hv_fcopy_onchannelcallback(fcopy_transaction.fcopy_context);
+ hv_poll_channel(fcopy_transaction.fcopy_context,
+ hv_fcopy_onchannelcallback);
in_hand_shake = false;
return 0;
}
@@ -226,6 +228,7 @@ void hv_fcopy_onchannelcallback(void *context)
fcopy_transaction.fcopy_context = context;
return;
}
+ fcopy_transaction.fcopy_context = NULL;
vmbus_recvpacket(channel, recv_buffer, PAGE_SIZE * 2, &recvlen,
&requestid);
@@ -333,8 +336,11 @@ static ssize_t fcopy_write(struct file *file, const char
__user *buf,
* Complete the transaction by forwarding the result
* to the host. But first, cancel the timeout.
*/
- if (cancel_delayed_work_sync(&fcopy_work))
+ if (cancel_delayed_work_sync(&fcopy_work)) {
fcopy_respond_to_host(response);
+ hv_poll_channel(fcopy_transaction.fcopy_context,
+ hv_fcopy_onchannelcallback);
+ }
return sizeof(int);
}
--
1.9.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFCv2 10/21] Drivers: hv: vss: switch to using the hvutil_device_state state machine
... from using kvp_transaction.active.
State transitions are:
-> HVUTIL_DEVICE_INIT when driver loads or on device release
-> HVUTIL_READY if the handshake was successful
-> HVUTIL_HOSTMSG_RECEIVED when there is a non-negotiation message from the host
-> HVUTIL_USERSPACE_REQ after we sent the message to the userspace daemon
-> HVUTIL_USERSPACE_RECV after/if the userspace daemon has replied
-> HVUTIL_READY after we respond to the host
-> HVUTIL_DEVICE_DYING on driver unload
In hv_vss_onchannelcallback() process ICMSGTYPE_NEGOTIATE messages even when
the userspace daemon is disconnected, otherwise we can make the host think
we don't support VSS and disable the service completely.
Unfortunately there is no good way we can figure out that the userspace daemon
has died (unless we start treating all timeouts as such), add a protection
against processing new VSS_OP_REGISTER messages while being in the middle of a
transaction (HVUTIL_USERSPACE_REQ or HVUTIL_USERSPACE_RECV state).
Signed-off-by: Vitaly Kuznetsov
---
drivers/hv/hv_snapshot.c | 87
1 file changed, 58 insertions(+), 29 deletions(-)
diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c
index 4bb9b1c..ddb1cda 100644
--- a/drivers/hv/hv_snapshot.c
+++ b/drivers/hv/hv_snapshot.c
@@ -33,16 +33,21 @@
#define VSS_USERSPACE_TIMEOUT (msecs_to_jiffies(10 * 1000))
/*
- * Global state maintained for transaction that is being processed.
- * Note that only one transaction can be active at any point in time.
+ * Global state maintained for transaction that is being processed. For a class
+ * of integration services, including the "VSS 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.
*
- * This state is set when we receive a request from the host; we
- * cleanup this state when the transaction is completed - when we respond
- * to the host with the key value.
+ * 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.
*/
static struct {
- bool active; /* transaction status - active or not */
+ int state; /* hvutil_device_state */
int recv_len; /* number of bytes received. */
struct vmbus_channel *recv_channel; /* chn we got the request */
u64 recv_req_id; /* request ID. */
@@ -75,6 +80,10 @@ static void vss_timeout_func(struct work_struct *dummy)
pr_warn("VSS: timeout waiting for daemon to reply\n");
vss_respond_to_host(HV_E_FAIL);
+ /* Transaction is finished, reset the state. */
+ if (vss_transaction.state > HVUTIL_READY)
+ vss_transaction.state = HVUTIL_READY;
+
hv_poll_channel(vss_transaction.vss_context,
hv_vss_onchannelcallback);
}
@@ -86,15 +95,32 @@ vss_cn_callback(struct cn_msg *msg, struct
netlink_skb_parms *nsp)
vss_msg = (struct hv_vss_msg *)msg->data;
- if (vss_msg->vss_hdr.operation == VSS_OP_REGISTER) {
+ /*
+* Don't process registration messages if we're in the middle of
+* a transaction processing.
+*/
+ if (vss_transaction.state > HVUTIL_READY &&
+ vss_msg->vss_hdr.operation == VSS_OP_REGISTER)
+ return;
+
+ if (vss_transaction.state == HVUTIL_DEVICE_INIT &&
+ vss_msg->vss_hdr.operation == VSS_OP_REGISTER) {
pr_info("VSS daemon registered\n");
- vss_transaction.active = false;
+ vss_transaction.state = HVUTIL_READY;
+ } else if (vss_transaction.state == HVUTIL_USERSPACE_REQ) {
+ vss_transaction.state = HVUTIL_USERSPACE_RECV;
+ if (cancel_delayed_work_sync(&vss_timeout_work)) {
+ vss_respond_to_host(vss_msg->error);
+ /* Transaction is finished, reset the state. */
+ vss_transaction.state = HVUTIL_READY;
+ hv_poll_channel(vss_transaction.vss_context,
+ hv_vss_onchannelcallback);
+ }
+ } else {
+ /* This is a spurious call! */
+ pr_warn("VSS: Transaction not active\n");
+ return;
}
- if (cancel_delayed_work_sync(&vss_timeout_work))
- vss_respond_to_host(vss_msg->error);
-
- hv_poll_channel(vss_transaction.vss_context,
- hv_vss_onchannelcallback);
}
@@ -105,6 +131,10 @@ static void vss_send_op(struct work_struct *dummy)
struct cn_msg *msg;
struct hv_vss_msg *vss_msg;
+ /* The transaction state is w
[PATCH RFCv2 00/21] Drivers: hv: utils: re-implement the kernel/userspace communication layer
Changes in RFCv2: - Preserve backwards compatibility with netlink-speaking daemons. [K. Y. Srinivasan] - Introduce transport abstraction layer. [K. Y. Srinivasan] - Get rid of ioctls [Radim Krcmar] - Make the series reviewable by splitting it into smaller patches. Anatomy of the series: Patches 01 - 07 are cleanup with minor functional change. Patch 08 defines the state machine. Patches 09-11 convert all 3 drivers to using the state machine. Patch 12 fixes a bug in fcopy. This change is going away in Patch 15, I just want to highlight the fix. Patch 13 introduces a transport abstraction. Patch 14-16 convert all drivers to using the transport abstraction. Patches 17-18 switch KVP and VSS daemon to using char devices. Patches 19-20 convert FCOPY and VSS to hull handshake (the same we have in KVP). These two can be postponed till we really need to distinguish between different kernels in the daemon code. Patch 21 unifies log messages on daemons connect across all drivers and moves these messages to debug level. I smoke-tested this series with both old (netlink) and new (char devices) daemons and tested the daemon upgrade procedure. Original description: This series converts kvp/vss daemons to use misc char devices instead of netlink for userspace/kernel communication and then updates fcopy to be consistent with kvp/vss. Userspace/kernel communication via netlink has a number of issues: - It is hard for userspace to figure out if the kernel part was loaded or not and this fact can change as there is a way to enable/disable the service from host side. Racy daemon startup is also a problem. - When the userspace daemon restarts/dies kernel part doesn't receive a notification. - Netlink communication is not stable under heavy load. - ... Vitaly Kuznetsov (21): Drivers: hv: util: move kvp/vss function declarations to hyperv_vmbus.h Drivers: hv: kvp: reset kvp_context Drivers: hv: kvp: move poll_channel() to hyperv_vmbus.h Drivers: hv: fcopy: process deferred messages when we complete the transaction Drivers: hv: vss: process deferred messages when we complete the transaction Drivers: hv: kvp: rename kvp_work -> kvp_timeout_work Drivers: hv: fcopy: rename fcopy_work -> fcopy_timeout_work Drivers: hv: util: introduce state machine for util drivers Drivers: hv: kvp: switch to using the hvutil_device_state state machine Drivers: hv: vss: switch to using the hvutil_device_state state machine Drivers: hv: fcopy: switch to using the hvutil_device_state state machine Drivers: hv: fcopy: set .owner reference for file operations Drivers: hv: util: introduce hv_utils_transport abstraction Drivers: hv: vss: convert to hv_utils_transport Drivers: hv: fcopy: convert to hv_utils_transport Drivers: hv: kvp: convert to hv_utils_transport Tools: hv: kvp: use misc char device to communicate with kernel Tools: hv: vss: use misc char device to communicate with kernel Drivers: hv: vss: full handshake support Drivers: hv: fcopy: full handshake support Drivers: hv: utils: unify driver registration reporting drivers/hv/Makefile | 2 +- drivers/hv/hv_fcopy.c | 287 ++-- drivers/hv/hv_kvp.c | 192 +-- drivers/hv/hv_snapshot.c| 168 +++ drivers/hv/hv_utils_transport.c | 276 ++ drivers/hv/hv_utils_transport.h | 51 +++ drivers/hv/hyperv_vmbus.h | 29 include/linux/hyperv.h | 8 -- include/uapi/linux/hyperv.h | 8 +- tools/hv/hv_fcopy_daemon.c | 15 +++ tools/hv/hv_kvp_daemon.c| 166 +-- tools/hv/hv_vss_daemon.c| 149 ++--- 12 files changed, 752 insertions(+), 599 deletions(-) create mode 100644 drivers/hv/hv_utils_transport.c create mode 100644 drivers/hv/hv_utils_transport.h -- 1.9.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFCv2 06/21] Drivers: hv: kvp: rename kvp_work -> kvp_timeout_work
'kvp_work' (and kvp_work_func) is a misnomer as it sounds like we expect
this useful work to happen and in reality it is just an emergency escape when
timeout happens.
Signed-off-by: Vitaly Kuznetsov
---
drivers/hv/hv_kvp.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index 939c3e7..14c62e2 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -79,10 +79,10 @@ static void kvp_send_key(struct work_struct *dummy);
static void kvp_respond_to_host(struct hv_kvp_msg *msg, int error);
-static void kvp_work_func(struct work_struct *dummy);
+static void kvp_timeout_func(struct work_struct *dummy);
static void kvp_register(int);
-static DECLARE_DELAYED_WORK(kvp_work, kvp_work_func);
+static DECLARE_DELAYED_WORK(kvp_timeout_work, kvp_timeout_func);
static DECLARE_WORK(kvp_sendkey_work, kvp_send_key);
static struct cb_id kvp_id = { CN_KVP_IDX, CN_KVP_VAL };
@@ -118,8 +118,8 @@ kvp_register(int reg_value)
kfree(msg);
}
}
-static void
-kvp_work_func(struct work_struct *dummy)
+
+static void kvp_timeout_func(struct work_struct *dummy)
{
/*
* If the timer fires, the user-mode component has not responded;
@@ -215,7 +215,7 @@ kvp_cn_callback(struct cn_msg *msg, struct
netlink_skb_parms *nsp)
* Complete the transaction by forwarding the key value
* to the host. But first, cancel the timeout.
*/
- if (cancel_delayed_work_sync(&kvp_work))
+ if (cancel_delayed_work_sync(&kvp_timeout_work))
kvp_respond_to_host(message, error);
}
@@ -440,7 +440,7 @@ kvp_send_key(struct work_struct *dummy)
rc = cn_netlink_send(msg, 0, 0, GFP_ATOMIC);
if (rc) {
pr_debug("KVP: failed to communicate to the daemon: %d\n", rc);
- if (cancel_delayed_work_sync(&kvp_work))
+ if (cancel_delayed_work_sync(&kvp_timeout_work))
kvp_respond_to_host(message, HV_E_FAIL);
}
@@ -668,7 +668,7 @@ void hv_kvp_onchannelcallback(void *context)
* user-mode not responding.
*/
schedule_work(&kvp_sendkey_work);
- schedule_delayed_work(&kvp_work, 5*HZ);
+ schedule_delayed_work(&kvp_timeout_work, 5*HZ);
return;
@@ -708,6 +708,6 @@ hv_kvp_init(struct hv_util_service *srv)
void hv_kvp_deinit(void)
{
cn_del_callback(&kvp_id);
- cancel_delayed_work_sync(&kvp_work);
+ cancel_delayed_work_sync(&kvp_timeout_work);
cancel_work_sync(&kvp_sendkey_work);
}
--
1.9.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFCv2 01/21] Drivers: hv: util: move kvp/vss function declarations to hyperv_vmbus.h
These declarations are internal to hv_util module and hv_fcopy_* declarations already reside there. Signed-off-by: Vitaly Kuznetsov --- drivers/hv/hv_kvp.c | 1 + drivers/hv/hv_snapshot.c | 2 ++ drivers/hv/hyperv_vmbus.h | 8 include/linux/hyperv.h| 8 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c index beb8105..a414c83 100644 --- a/drivers/hv/hv_kvp.c +++ b/drivers/hv/hv_kvp.c @@ -28,6 +28,7 @@ #include #include +#include "hyperv_vmbus.h" /* * Pre win8 version numbers used in ws2008 and ws 2008 r2 (win7) diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c index 9d5e0d1..c1a3604 100644 --- a/drivers/hv/hv_snapshot.c +++ b/drivers/hv/hv_snapshot.c @@ -24,6 +24,8 @@ #include #include +#include "hyperv_vmbus.h" + #define VSS_MAJOR 5 #define VSS_MINOR 0 #define VSS_VERSION(VSS_MAJOR << 16 | VSS_MINOR) diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h index 44b1c94..0d85dd1 100644 --- a/drivers/hv/hyperv_vmbus.h +++ b/drivers/hv/hyperv_vmbus.h @@ -699,6 +699,14 @@ int vmbus_set_event(struct vmbus_channel *channel); void vmbus_on_event(unsigned long data); +int hv_kvp_init(struct hv_util_service *); +void hv_kvp_deinit(void); +void hv_kvp_onchannelcallback(void *); + +int hv_vss_init(struct hv_util_service *); +void hv_vss_deinit(void); +void hv_vss_onchannelcallback(void *); + int hv_fcopy_init(struct hv_util_service *); void hv_fcopy_deinit(void); void hv_fcopy_onchannelcallback(void *); diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index 5a2ba67..78cfd65 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -1206,14 +1206,6 @@ extern bool vmbus_prep_negotiate_resp(struct icmsg_hdr *, struct icmsg_negotiate *, u8 *, int, int); -int hv_kvp_init(struct hv_util_service *); -void hv_kvp_deinit(void); -void hv_kvp_onchannelcallback(void *); - -int hv_vss_init(struct hv_util_service *); -void hv_vss_deinit(void); -void hv_vss_onchannelcallback(void *); - extern struct resource hyperv_mmio; /* -- 1.9.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFCv2 09/21] Drivers: hv: kvp: switch to using the hvutil_device_state state machine
... from using 2 different state variables: kvp_transaction.active and
in_hand_shake.
State transitions are:
-> HVUTIL_DEVICE_INIT when driver loads or on device release
-> HVUTIL_READY if the handshake was successful
-> HVUTIL_HOSTMSG_RECEIVED when there is a non-negotiation message from the host
-> HVUTIL_USERSPACE_REQ after we sent the message to the userspace daemon
-> HVUTIL_USERSPACE_RECV after/if the userspace daemon has replied
-> HVUTIL_READY after we respond to the host
-> HVUTIL_DEVICE_DYING on driver unload
In hv_kvp_onchannelcallback() process ICMSGTYPE_NEGOTIATE messages even when
the userspace daemon is disconnected, otherwise we can make the host think
we don't support KVP and disable the service completely.
Unfortunately there is no good way we can figure out that the userspace daemon
has died (unless we start treating all timeouts as such). In case the daemon
restarts we skip the negotiation procedure (so the daemon is supposed to has
the same version). This behavior is unchanged from in_handshake approach.
Signed-off-by: Vitaly Kuznetsov
---
drivers/hv/hv_kvp.c | 87 ++---
1 file changed, 49 insertions(+), 38 deletions(-)
diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index 14c62e2..a70d202 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -46,16 +46,21 @@
#define WIN8_SRV_VERSION (WIN8_SRV_MAJOR << 16 | WIN8_SRV_MINOR)
/*
- * Global state maintained for transaction that is being processed.
- * Note that only one transaction can be active at any point in time.
+ * Global state maintained for transaction that is being processed. For a class
+ * of integration services, including the "KVP 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.
*
- * This state is set when we receive a request from the host; we
- * cleanup this state when the transaction is completed - when we respond
- * to the host with the key value.
+ * 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.
*/
static struct {
- bool active; /* transaction status - active or not */
+ int state; /* hvutil_device_state */
int recv_len; /* number of bytes received. */
struct hv_kvp_msg *kvp_msg; /* current message */
struct vmbus_channel *recv_channel; /* chn we got the request */
@@ -64,13 +69,6 @@ static struct {
} kvp_transaction;
/*
- * Before we can accept KVP 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;
-
-/*
* This state maintains the version number registered by the daemon.
*/
static int dm_reg_value;
@@ -126,6 +124,13 @@ static void kvp_timeout_func(struct work_struct *dummy)
* process the pending transaction.
*/
kvp_respond_to_host(NULL, HV_E_FAIL);
+
+ /* Transaction is finished, reset the state. */
+ if (kvp_transaction.state > HVUTIL_READY)
+ kvp_transaction.state = HVUTIL_READY;
+
+ hv_poll_channel(kvp_transaction.kvp_context,
+ hv_kvp_onchannelcallback);
}
static int kvp_handle_handshake(struct hv_kvp_msg *msg)
@@ -154,9 +159,7 @@ static int kvp_handle_handshake(struct hv_kvp_msg *msg)
*/
pr_info("KVP: user-mode registering done.\n");
kvp_register(dm_reg_value);
- kvp_transaction.active = false;
- hv_poll_channel(kvp_transaction.kvp_context,
- hv_kvp_onchannelcallback);
+ kvp_transaction.state = HVUTIL_READY;
}
return ret;
}
@@ -180,12 +183,16 @@ kvp_cn_callback(struct cn_msg *msg, struct
netlink_skb_parms *nsp)
* with the daemon; handle that first.
*/
- if (in_hand_shake) {
- if (kvp_handle_handshake(message))
- in_hand_shake = false;
+ if (kvp_transaction.state < HVUTIL_READY) {
+ kvp_handle_handshake(message);
return;
}
+ /* We didn't send anything to userspace so the reply is spurious */
+ if (kvp_transaction.state < HVUTIL_USERSPACE_REQ)
+ return;
+ kvp_transaction.state = HVUTIL_USERSPACE_RECV;
+
/*
* Based on the version of the daemon, we propagate errors from the
* daemon differently.
@@ -215,8 +222,12 @@ kvp_cn_callback(struct cn_msg *msg, struct
netlink_skb_parms *nsp)
* Complete the transaction by forwarding the key value
[PATCH RFCv2 11/21] Drivers: hv: fcopy: switch to using the hvutil_device_state state machine
... from using 3 different state variables: fcopy_transaction.active, opened,
and in_hand_shake.
State transitions are:
-> HVUTIL_DEVICE_INIT when driver loads or on device release
-> HVUTIL_READY if the handshake was successful
-> HVUTIL_HOSTMSG_RECEIVED when there is a non-negotiation message from the host
-> HVUTIL_USERSPACE_REQ after userspace daemon read the message
-> HVUTIL_USERSPACE_RECV after/if userspace has replied
-> HVUTIL_READY after we respond to the host
-> HVUTIL_DEVICE_DYING on driver unload
In hv_fcopy_onchannelcallback() process ICMSGTYPE_NEGOTIATE messages even when
the userspace daemon is disconnected, otherwise we can make the host think
we don't support FCOPY and disable the service completely.
Signed-off-by: Vitaly Kuznetsov
---
drivers/hv/hv_fcopy.c | 70 ++-
1 file changed, 30 insertions(+), 40 deletions(-)
diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
index c14f0f4..a501301 100644
--- a/drivers/hv/hv_fcopy.c
+++ b/drivers/hv/hv_fcopy.c
@@ -47,15 +47,10 @@
* 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 state; /* hvutil_device_state */
int recv_len; /* number of bytes received. */
struct hv_fcopy_hdr *fcopy_msg; /* current message */
struct hv_start_fcopy message; /* sent to daemon */
@@ -65,14 +60,6 @@ static struct {
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);
static void fcopy_respond_to_host(int error);
static void fcopy_timeout_func(struct work_struct *dummy);
@@ -87,6 +74,10 @@ static void fcopy_timeout_func(struct work_struct *dummy)
*/
fcopy_respond_to_host(HV_E_FAIL);
+ /* Transaction is finished, reset the state. */
+ if (fcopy_transaction.state > HVUTIL_READY)
+ fcopy_transaction.state = HVUTIL_READY;
+
/* In the case the user-space daemon crashes, hangs or is killed, we
* need to down the semaphore, otherwise, after the daemon starts next
* time, the obsolete data in fcopy_transaction.message or
@@ -118,10 +109,9 @@ static int fcopy_handle_handshake(u32 version)
}
pr_info("FCP: user-mode registering done. Daemon version: %d\n",
version);
- fcopy_transaction.active = false;
+ fcopy_transaction.state = HVUTIL_READY;
hv_poll_channel(fcopy_transaction.fcopy_context,
hv_fcopy_onchannelcallback);
- in_hand_shake = false;
return 0;
}
@@ -191,8 +181,6 @@ fcopy_respond_to_host(int error)
channel = fcopy_transaction.recv_channel;
req_id = fcopy_transaction.recv_req_id;
- fcopy_transaction.active = false;
-
icmsghdr = (struct icmsg_hdr *)
&recv_buffer[sizeof(struct vmbuspipe_hdr)];
@@ -220,7 +208,7 @@ void hv_fcopy_onchannelcallback(void *context)
int util_fw_version;
int fcopy_srv_version;
- if (fcopy_transaction.active) {
+ if (fcopy_transaction.state > HVUTIL_READY) {
/*
* We will defer processing this callback once
* the current transaction is complete.
@@ -252,12 +240,18 @@ void hv_fcopy_onchannelcallback(void *context)
* transaction; note transactions are serialized.
*/
- fcopy_transaction.active = true;
fcopy_transaction.recv_len = recvlen;
fcopy_transaction.recv_channel = channel;
fcopy_transaction.recv_req_id = requestid;
fcopy_transaction.fcopy_msg = fcopy_msg;
+ if (fcopy_transaction.state < HVUTIL_READY) {
+ /* Userspace is not registered yet */
+ fcopy_respond_to_host(HV_E_FAIL);
+ return;
+ }
+ fcopy_transaction.state = HVUTIL_HOSTMSG_RECEIVED;
+
/*
* Send the information to the user-level daemon.
*/
@@ -290,10 +284,10 @@ static ssize_t fcopy_read(struct file *file, char __user
*buf,
/*
* The channel may be rescinded and in this case, we will wakeup the
-* the thread blocked o
[PATCH RFCv2 02/21] Drivers: hv: kvp: reset kvp_context
We set kvp_context when we want to postpone receiving a packet from vmbus due to the previous transaction being unfinished. We, however, never reset this state, all consequent kvp_respond_to_host() calls will result in poll_channel() calling hv_kvp_onchannelcallback(). This doesn't cause real issues as: 1) Host is supposed to serialize transactions as well 2) If no message is pending vmbus_recvpacket() will return 0 recvlen. This is just a cleanup. Signed-off-by: Vitaly Kuznetsov --- drivers/hv/hv_kvp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c index a414c83..caa467d 100644 --- a/drivers/hv/hv_kvp.c +++ b/drivers/hv/hv_kvp.c @@ -621,6 +621,7 @@ void hv_kvp_onchannelcallback(void *context) kvp_transaction.kvp_context = context; return; } + kvp_transaction.kvp_context = NULL; vmbus_recvpacket(channel, recv_buffer, PAGE_SIZE * 4, &recvlen, &requestid); -- 1.9.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 4/6] staging: sm750fb: Remove unused function
On Wed, Mar 11, 2015 at 10:59:41AM +, Lorenzo Stoakes wrote: > On 11 March 2015 at 10:57, Sudip Mukherjee wrote: > > but function prototype still remains in sm750_accel.h > > No it doesn't, v2 of patch 3 in the series no longer puts it there :) oops.. i am sorry.. i still had your previous patches applied thatiswhy i got the prototype in sm750_accel.h regards sudip > > -- > Lorenzo Stoakes > https:/ljs.io ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 4/6] staging: sm750fb: Remove unused function
On 11 March 2015 at 10:57, Sudip Mukherjee wrote: > but function prototype still remains in sm750_accel.h No it doesn't, v2 of patch 3 in the series no longer puts it there :) -- Lorenzo Stoakes https:/ljs.io ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 4/6] staging: sm750fb: Remove unused function
On Wed, Mar 11, 2015 at 10:45:48AM +, Lorenzo Stoakes wrote: > This patch removes the unused hw712_fillrect function. This patch fixes > the following sparse warning:- > > drivers/staging/sm750fb/sm750_accel.c:95:5: warning: symbol 'hw712_fillrect' > was not declared. Should it be static? > > Signed-off-by: Lorenzo Stoakes > --- > drivers/staging/sm750fb/sm750_accel.c | 78 > --- > 1 file changed, 78 deletions(-) > > diff --git a/drivers/staging/sm750fb/sm750_accel.c > b/drivers/staging/sm750fb/sm750_accel.c > index 6521c3b..c5a3726 100644 > --- a/drivers/staging/sm750fb/sm750_accel.c > +++ b/drivers/staging/sm750fb/sm750_accel.c but function prototype still remains in sm750_accel.h regards sudip ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 4/6] staging: sm750fb: Remove unused function
This patch removes the unused hw712_fillrect function. This patch fixes
the following sparse warning:-
drivers/staging/sm750fb/sm750_accel.c:95:5: warning: symbol 'hw712_fillrect'
was not declared. Should it be static?
Signed-off-by: Lorenzo Stoakes
---
drivers/staging/sm750fb/sm750_accel.c | 78 ---
1 file changed, 78 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750_accel.c
b/drivers/staging/sm750fb/sm750_accel.c
index 6521c3b..c5a3726 100644
--- a/drivers/staging/sm750fb/sm750_accel.c
+++ b/drivers/staging/sm750fb/sm750_accel.c
@@ -89,84 +89,6 @@ void hw_set2dformat(struct lynx_accel * accel,int fmt)
write_dpr(accel,DE_STRETCH_FORMAT,reg);
}
-/* seems sm712 RectFill command is broken,so need use BitBlt to
- * replace it. */
-
-int hw712_fillrect(struct lynx_accel * accel,
- u32 base,u32 pitch,u32 Bpp,
- u32 x,u32 y,u32 width,u32 height,
- u32 color,u32 rop)
-{
- u32 deCtrl;
- if(accel->de_wait() != 0)
- {
- /* int time wait and always busy,seems hardware
-* got something error */
- pr_debug("%s:De engine always bussy\n",__func__);
- return -1;
- }
- /* 24bpp 2d acceleration still not work,we already support 2d on
-* both 8/16/32 bpp now, so there is no harm if we disable 2d on
-* 24bpp for current stage. */
-#if 0
- if(Bpp == 3){
- width *= 3;
- x *= 3;
- write_dpr(accel,DE_PITCH,
- FIELD_VALUE(0,DE_PITCH,DESTINATION,pitch)|
- FIELD_VALUE(0,DE_PITCH,SOURCE,pitch));//dpr10
- }
- else
-#endif
- {
- write_dpr(accel,DE_PITCH,
- FIELD_VALUE(0,DE_PITCH,DESTINATION,pitch/Bpp)|
- FIELD_VALUE(0,DE_PITCH,SOURCE,pitch/Bpp));//dpr10
-
- }
-
- write_dpr(accel,DE_FOREGROUND,color);//DPR14
- write_dpr(accel,DE_MONO_PATTERN_HIGH,~0);//DPR34
- write_dpr(accel,DE_MONO_PATTERN_LOW,~0);//DPR38
-
- write_dpr(accel,DE_WINDOW_SOURCE_BASE,base);//dpr44
- write_dpr(accel,DE_WINDOW_DESTINATION_BASE,base);//dpr40
-
-
- write_dpr(accel,DE_WINDOW_WIDTH,
- FIELD_VALUE(0,DE_WINDOW_WIDTH,DESTINATION,pitch/Bpp)|
- FIELD_VALUE(0,DE_WINDOW_WIDTH,SOURCE,pitch/Bpp));//dpr3c
-
-
- write_dpr(accel,DE_DESTINATION,
- FIELD_SET(0,DE_DESTINATION,WRAP,DISABLE)|
- FIELD_VALUE(0,DE_DESTINATION,X,x)|
- FIELD_VALUE(0,DE_DESTINATION,Y,y));//dpr4
-
- write_dpr(accel,DE_DIMENSION,
- FIELD_VALUE(0,DE_DIMENSION,X,width)|
- FIELD_VALUE(0,DE_DIMENSION,Y_ET,height));//dpr8
-
- deCtrl =
- FIELD_SET(0,DE_CONTROL,STATUS,START)|
- FIELD_SET(0,DE_CONTROL,COMMAND,BITBLT)|
- FIELD_SET(0,DE_CONTROL,ROP2_SOURCE,PATTERN)|
- FIELD_SET(0,DE_CONTROL,ROP_SELECT,ROP2)|
- FIELD_VALUE(0,DE_CONTROL,ROP,rop);//dpr0xc
-#if 0
- /* dump registers */
- int i;
- inf_msg("x,y,w,h = %d,%d,%d,%d\n",x,y,width,height);
- for(i=0x04;i<=0x44;i+=4){
- inf_msg("dpr%02x = %08x\n",i,read_dpr(accel,i));
- }
- inf_msg("deCtrl = %08x\n",deCtrl);
-#endif
-
- write_dpr(accel,DE_CONTROL,deCtrl);
- return 0;
-}
-
int hw_fillrect(struct lynx_accel * accel,
u32 base,u32 pitch,u32 Bpp,
u32 x,u32 y,u32 width,u32 height,
--
2.3.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 3/6] staging: sm750fb: Make internal functions static
This patch declares externally unavailable functions static. This fixes
the following sparse warnings:-
drivers/staging/sm750fb/ddk750_swi2c.c:223:6: warning: symbol 'swI2CStart' was
not declared. Should it be static?
drivers/staging/sm750fb/ddk750_swi2c.c:234:6: warning: symbol 'swI2CStop' was
not declared. Should it be static?
drivers/staging/sm750fb/ddk750_swi2c.c:252:6: warning: symbol 'swI2CWriteByte'
was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_swi2c.c:320:15: warning: symbol 'swI2CReadByte'
was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_swi2c.c:361:6: warning: symbol
'swI2CInit_SM750LE' was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_hwi2c.c:63:6: warning: symbol 'hwI2CWaitTXDone'
was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_hwi2c.c:93:14: warning: symbol 'hwI2CWriteData'
was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_hwi2c.c:160:14: warning: symbol 'hwI2CReadData'
was not declared. Should it be static?
Signed-off-by: Lorenzo Stoakes
---
drivers/staging/sm750fb/ddk750_hwi2c.c | 6 +++---
drivers/staging/sm750fb/ddk750_swi2c.c | 10 +-
drivers/staging/sm750fb/sm750_accel.c | 2 +-
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/sm750fb/ddk750_hwi2c.c
b/drivers/staging/sm750fb/ddk750_hwi2c.c
index 84dfb6f..7826376 100644
--- a/drivers/staging/sm750fb/ddk750_hwi2c.c
+++ b/drivers/staging/sm750fb/ddk750_hwi2c.c
@@ -60,7 +60,7 @@ void hwI2CClose(void)
}
-long hwI2CWaitTXDone(void)
+static long hwI2CWaitTXDone(void)
{
unsigned int timeout;
@@ -90,7 +90,7 @@ long hwI2CWaitTXDone(void)
* Return Value:
* Total number of bytes those are actually written.
*/
-unsigned int hwI2CWriteData(
+static unsigned int hwI2CWriteData(
unsigned char deviceAddress,
unsigned int length,
unsigned char *pBuffer
@@ -157,7 +157,7 @@ unsigned int hwI2CWriteData(
* Return Value:
* Total number of actual bytes read from the slave device
*/
-unsigned int hwI2CReadData(
+static unsigned int hwI2CReadData(
unsigned char deviceAddress,
unsigned int length,
unsigned char *pBuffer
diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c
b/drivers/staging/sm750fb/ddk750_swi2c.c
index 1249759..516f5bb 100644
--- a/drivers/staging/sm750fb/ddk750_swi2c.c
+++ b/drivers/staging/sm750fb/ddk750_swi2c.c
@@ -220,7 +220,7 @@ static void swI2CAck(void)
/*
* This function sends the start command to the slave device
*/
-void swI2CStart(void)
+static void swI2CStart(void)
{
/* Start I2C */
swI2CSDA(1);
@@ -231,7 +231,7 @@ void swI2CStart(void)
/*
* This function sends the stop command to the slave device
*/
-void swI2CStop(void)
+static void swI2CStop(void)
{
/* Stop the I2C */
swI2CSCL(1);
@@ -249,7 +249,7 @@ void swI2CStop(void)
* 0 - Success
* -1 - Fail to write byte
*/
-long swI2CWriteByte(unsigned char data)
+static long swI2CWriteByte(unsigned char data)
{
unsigned char value = data;
int i;
@@ -317,7 +317,7 @@ long swI2CWriteByte(unsigned char data)
* Return Value:
* One byte data read from the Slave device
*/
-unsigned char swI2CReadByte(unsigned char ack)
+static unsigned char swI2CReadByte(unsigned char ack)
{
int i;
unsigned char data = 0;
@@ -358,7 +358,7 @@ unsigned char swI2CReadByte(unsigned char ack)
* -1 - Fail to initialize the i2c
* 0 - Success
*/
-long swI2CInit_SM750LE(
+static long swI2CInit_SM750LE(
unsigned char i2cClkGPIO,
unsigned char i2cDataGPIO
)
--
2.3.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] staging: sm750fb: Use memset_io instead of memset
On 11 March 2015 at 10:35, Sudip Mukherjee > i think i will better check v2 of your series on hardware This is incoming in just a moment (though I only v2 patches in the series I've changed which I think is the right way to make modifications with a patch series.) > , and while > you are preparing that v2 keep in mind the changelog should not exceed > 72 characters. in your this series for all patches it was more than > that. I will update the messages in the changed patches accordingly, I'm not sure this is worth a resend of all previous patches for however? I do see quite a few patches in the log that exceed this. Additionally, I suspect it would make the patches less readable to wrap sparse warning lines so I think those ought to sit outside of this limit. I am more than happy to change these though if these ought to be kept *strictly* to a 72 character limit throughout? Best, -- Lorenzo Stoakes https:/ljs.io ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] staging: sm750fb: Use memset_io instead of memset
On Wed, Mar 11, 2015 at 03:18:06PM +0530, Sudip Mukherjee wrote: > On Wed, Mar 11, 2015 at 09:11:52AM +, Lorenzo Stoakes wrote: > > On 11 March 2015 at 08:54, Dan Carpenter wrote: > > > Btw, do you have this hardware? Are you able to test these changes? > > > > Unfortunately not, I am trying to keep these changes as simple code > > fixes that ought not to affect actual hardware behaviour as I can > > (though of course you can never be entirely sure that's the case!) > > > > I suspect that Sudip must have some real hardware, is this the case > > Sudip? If it isn't too presumptuous of me to ask, perhaps you might be > > able to check patches that are successfully merged into > > staging-testing? > yes, i have the hardware and will test on it. but your patch 5/6 and > 6/6 is scaring me :) i think i will better check v2 of your series on hardware, and while you are preparing that v2 keep in mind the changelog should not exceed 72 characters. in your this series for all patches it was more than that. regards sudip > > regards > sudip > > > > Best, > > > > -- > > Lorenzo Stoakes > > https:/ljs.io ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] staging: sm750fb: Use memset_io instead of memset
On Wed, Mar 11, 2015 at 09:11:52AM +, Lorenzo Stoakes wrote: > On 11 March 2015 at 08:54, Dan Carpenter wrote: > > Btw, do you have this hardware? Are you able to test these changes? > > Unfortunately not, I am trying to keep these changes as simple code > fixes that ought not to affect actual hardware behaviour as I can > (though of course you can never be entirely sure that's the case!) > > I suspect that Sudip must have some real hardware, is this the case > Sudip? If it isn't too presumptuous of me to ask, perhaps you might be > able to check patches that are successfully merged into > staging-testing? yes, i have the hardware and will test on it. but your patch 5/6 and 6/6 is scaring me :) regards sudip > > Best, > > -- > Lorenzo Stoakes > https:/ljs.io ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/6] staging: sm750fb: Expose hw712_fillrect externally
On 11 March 2015 at 09:37, Sudip Mukherjee wrote: > in your previous patch 3/6 you made it static now you are again > removing the static keyword. may i ask why you changed it in 3/6 if you > again change it back to original in this patch? There's no good reason, it's just a mistake :) I'll fix it shortly. > anyways, like Dan said, delete this function, its not used anywhere. > it will not be used also, i missed removing this function from the > vendor crude drver. Will do! Best, -- Lorenzo Stoakes https:/ljs.io ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/6] staging: sm750fb: Make internal functions static
On 11 March 2015 at 09:30, Sudip Mukherjee > this is introducing a build warning, better remove this from your patch > and send a separate patch to remove the function as this function is > not used anywhere. Hi Sudip, I didn't realise I'd included the move to static in this patch. In a later patch I expose this function in the header file. I'll update this patch not to touch hw712_fillrect then remove it in a later patch altogether. Best, -- Lorenzo Stoakes https:/ljs.io ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/6] staging: sm750fb: Expose hw712_fillrect externally
On Wed, Mar 11, 2015 at 01:28:43AM +, Lorenzo Stoakes wrote: > > diff --git a/drivers/staging/sm750fb/sm750_accel.c > b/drivers/staging/sm750fb/sm750_accel.c > index 4aa763b..6521c3b 100644 > --- a/drivers/staging/sm750fb/sm750_accel.c > +++ b/drivers/staging/sm750fb/sm750_accel.c > @@ -92,7 +92,7 @@ void hw_set2dformat(struct lynx_accel * accel,int fmt) > /* seems sm712 RectFill command is broken,so need use BitBlt to > * replace it. */ > > -static int hw712_fillrect(struct lynx_accel * accel, > +int hw712_fillrect(struct lynx_accel * accel, > u32 base,u32 pitch,u32 Bpp, > u32 x,u32 y,u32 width,u32 height, > u32 color,u32 rop) in your previous patch 3/6 you made it static now you are again removing the static keyword. may i ask why you changed it in 3/6 if you again change it back to original in this patch? anyways, like Dan said, delete this function, its not used anywhere. it will not be used also, i missed removing this function from the vendor crude drver. regards sudip > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
TELEKOM MALAYSIA BONUS
Telekom Malaysia Berhad G.03B, Ground Floor, Kompleks Antarabangsa, Jln Sultan Ismail, Off Jalan Ampang 50250 Kuala Lumpur Malaysia. NOTIS RASMI HADIAH TELEKOM MALAYSIA Pihak Telekom Malaysia @Program Kemenangan yang telah diadakan pada 8th March 2015 di mana alamat email anda yang disertakan beraama Tiket Kemenangan nombor 2 - 4 - 16 -37 - 89 - 40 -85 dengan siri nombor 2268/02 telah memenangi loteri kategori hadiah kedua khas keluarga Telekom Malaysia. Untuk menuntut hadiah kemenangan ini anda dikehendaki menghubungi melalui e mail Bahagian Tuntutan untuk tujuan pemerosesan dan pembayaran hadiah wang tunai kepada anda. Di sepanjang program Khas Keluarga Telekom yang telah diadakan di Ibupejabat di Kuala Lumpur sejumlah Rm270,000.00 (Ringgit Malaysia : Dua Ratus Tujoh Puloh Ribu) telah dianugerahkan kepada anda oleh Telekom Malaysia Berhad kepada anda dan keluarga anda sempena sambutsn Hari Raya 2015 ini. Program ini turut dibiayai bersama oleh Toyota Malaysia dan Tenaga Nasional sebagai pakej istimewa Telekom 2015 dan anda perlu memahami bahawa e mail ini adalah 100% sah dan diiktiraf kerana program ini kebiasaannya diadakan sekali dalam masa lima tahun. Sila hubungi agen kami untuk menuntut hadiah ini : EN SHAFIE BIN HASSAN Pengarah Bahagian Tuntutan E-mail: bonustmb2...@outlook.my Untuk tujuan pemerosesan sila hubungi agen kami dengan maklumat-maklumat berikut: 1) Nama Penuh: 2). Umur: 3). Pekerjaan: 4). Telefon: 5). Negeri / Bandar: Perlu diingatkan bahawa hadiah akhir tahun Telekom Malaysia Berhad 2015 ini adalah diberikan khas kepada anda dan keluarga anda dan anda hendaklah membuat tunttan ini sebelum 3rd April 2015. Terima kasih. Mrs Nadia binti Rafik Pengurus Eksekutif Anugerah Telekom Malaysia Ibupejabat telekom Malaysia. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/6] staging: sm750fb: Make internal functions static
On Wed, Mar 11, 2015 at 01:28:42AM +, Lorenzo Stoakes wrote: > This patch declares externally unavailable functions static. This fixes the > following sparse warnings:- > diff --git a/drivers/staging/sm750fb/sm750_accel.c > b/drivers/staging/sm750fb/sm750_accel.c > index 6521c3b..4aa763b 100644 > --- a/drivers/staging/sm750fb/sm750_accel.c > +++ b/drivers/staging/sm750fb/sm750_accel.c > @@ -92,7 +92,7 @@ void hw_set2dformat(struct lynx_accel * accel,int fmt) > /* seems sm712 RectFill command is broken,so need use BitBlt to > * replace it. */ > > -int hw712_fillrect(struct lynx_accel * accel, > +static int hw712_fillrect(struct lynx_accel * accel, > u32 base,u32 pitch,u32 Bpp, > u32 x,u32 y,u32 width,u32 height, > u32 color,u32 rop) this is introducing a build warning, better remove this from your patch and send a separate patch to remove the function as this function is not used anywhere. regards sudip > -- > 2.3.2 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2 net-next] hyperv: Support batched notification
On Tue, Mar 10, K. Y. Srinivasan wrote: > Optimize notifying the host by deferring notification until there > are no more packets to be sent. This will help in batching the requests > on the host. The subjects for the network driver say "hyperv:". But all such changes should get "netvsc:" as prefix to make it clear what each single patch is all about. This should be changed for upcoming submissions. Thanks! Olaf ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] staging: sm750fb: Use memset_io instead of memset
On Wed, Mar 11, 2015 at 09:11:52AM +, Lorenzo Stoakes wrote: > On 11 March 2015 at 08:54, Dan Carpenter wrote: > > When I see a patch like this, then I worry, "What if the Sparse > > annotations are wrong? The patch description doesn't say anything about > > that." After review then I think the annotations are correct so that's > > fine. > > How do you mean? I was careful to check what sparse was referring to, > then investigate how memset should be used with pointers with a > __iomem qualifier. I'd like to be able to improve my patch > descriptions going forward as best I can :) > Yes. The patch is correct. I wasn't asking you to redo it. From later patches it's actually clear that you know that this change is a bugfix and a behavior change. But we get a lot of patches where people just randomly change things to please Sparse and it maybe silences a warning but it's not correct. I can think of a few recentish examples where people used standard struct types which hold __iomem or __user pointers but they used them in non-standard ways so the pointers were actually normal kernel pointers. I guess the rule here is that the patch should explain the effect of the bugfix for the user. Often you won't know the effect, but it's a helpful thing to think about. > > Btw, do you have this hardware? Are you able to test these changes? > > Unfortunately not, I am trying to keep these changes as simple code > fixes that ought not to affect actual hardware behaviour as I can > (though of course you can never be entirely sure that's the case!) That's fine. I was just wondering. It affects how paranoid I am when I review the code. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] staging: sm750fb: Use memset_io instead of memset
On 11 March 2015 at 08:54, Dan Carpenter wrote: > When I see a patch like this, then I worry, "What if the Sparse > annotations are wrong? The patch description doesn't say anything about > that." After review then I think the annotations are correct so that's > fine. How do you mean? I was careful to check what sparse was referring to, then investigate how memset should be used with pointers with a __iomem qualifier. I'd like to be able to improve my patch descriptions going forward as best I can :) > Btw, do you have this hardware? Are you able to test these changes? Unfortunately not, I am trying to keep these changes as simple code fixes that ought not to affect actual hardware behaviour as I can (though of course you can never be entirely sure that's the case!) I suspect that Sudip must have some real hardware, is this the case Sudip? If it isn't too presumptuous of me to ask, perhaps you might be able to check patches that are successfully merged into staging-testing? Best, -- Lorenzo Stoakes https:/ljs.io ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/6] staging: sm750fb: Spinlock and unlock in the same block
On Wed, Mar 11, 2015 at 01:28:45AM +, Lorenzo Stoakes wrote:
> -static inline void myspin_lock(spinlock_t * sl){
> - struct lynx_share * share;
> - share = container_of(sl,struct lynx_share,slock);
> - if(share->dual){
> - spin_lock(sl);
> - }
> -}
Yes, good. We all hate locking wrappers but these are worse than
normal.
> + /* if not use spin_lock,system will die if user load driver
> + * and immediatly unload driver frequently (dual)*/
> + if (share->dual) {
> + spin_lock(&share->slock);
> + share->accel.de_fillrect(&share->accel,
> + base,pitch,Bpp,
> + region->dx,region->dy,
> + region->width,region->height,
> + color,rop);
> + spin_unlock(&share->slock);
> + } else
> + share->accel.de_fillrect(&share->accel,
> + base,pitch,Bpp,
> + region->dx,region->dy,
> + region->width,region->height,
> + color,rop);
> }
No. You've made the code uglier to work around Sparse stupidness. Also
the braces are not according to kernel style.
if (share->dual)
spin_lock(&share->slock);
share->accel.de_fillrect(&share->accel,
base,pitch,Bpp,
region->dx,region->dy,
region->width,region->height,
color,rop);
if (share->dual)
spin_unlock(&share->slock);
Sparse will still complain but no one cares.
regards,
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] staging: sm7xxfb: change return of sm7xx_vga_setup
On Mon, Mar 09, 2015 at 05:04:14PM +0300, Dan Carpenter wrote: > On Mon, Mar 09, 2015 at 07:23:43PM +0530, Sudip Mukherjee wrote: > > > Hm... That's a good question. > > > > > > I suspect we should just go with fb_get_mode() and do it quickly. This > > > driver is so new in the kernel that there are no users to get upset. > > > > i am afraid that is not correct. this driver was in staging from long > > back, and Greg removed it from staging as no one was working with it. > > But after that removal Lemote Yeeloong laptop of Steven Rostedt > > stopped working. (https://lkml.org/lkml/2014/11/20/698) , so i guess > > there will be many such users. > > Hm... I suspect we should still change it so it matches the other FB > drivers but I don't know. > > Perhaps Greg has an opinion? maybe, i should make the change and then we can request Steven Rostedt to test it on his Lemote Yeeloong. if nothing breaks, then alright, else revert the patch. regards sudip > > regards, > dan carpenter > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/6] staging: sm750fb: Expose hw712_fillrect externally
On Wed, Mar 11, 2015 at 01:28:43AM +, Lorenzo Stoakes wrote: > This patch adds a reference to hw712_fillrect which is not used elsewhere in > the driver, > but appears to be an alternative to the hw_fillrect method. This patch fixes > the following sparse warning:- > > drivers/staging/sm750fb/sm750_accel.c:95:5: warning: symbol 'hw712_fillrect' > was not declared. Should it be static? > Just delete it. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/6] staging: sm750fb: correct integer comparison
On Wed, Mar 11, 2015 at 10:57:43AM +0300, Dan Carpenter wrote:
> On Tue, Mar 10, 2015 at 10:46:57PM +0530, Sudip Mukherjee wrote:
> >
> > -while((opt = strsep(&src,":")) != NULL && *opt != NULL){
> > +while((opt = strsep(&src,":")) != NULL && *opt != 0){
>
> Use '\0' for the NUL char.
i got a doubt while making that patch, so i checked lib/string.c and
saw both 0 and '\0' are used.
regards
sudip
>
> regards,
> dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] staging: sm750fb: Use memset_io instead of memset
On Wed, Mar 11, 2015 at 01:28:40AM +, Lorenzo Stoakes wrote: > This patch uses memset_io instead of memset when using memset on __iomem > qualified pointers. This fixes the following sparse warnings:- > > drivers/staging/sm750fb/sm750.c:489:17: warning: incorrect type in argument 1 > (different address spaces) > drivers/staging/sm750fb/sm750.c:490:17: warning: incorrect type in argument 1 > (different address spaces) > drivers/staging/sm750fb/sm750.c:501:17: warning: incorrect type in argument 1 > (different address spaces) > drivers/staging/sm750fb/sm750.c:502:17: warning: incorrect type in argument 1 > (different address spaces) > drivers/staging/sm750fb/sm750.c:833:5: warning: incorrect type in argument 1 > (different address spaces) > drivers/staging/sm750fb/sm750.c:1154:9: warning: incorrect type in argument 1 > (different address spaces) > > Signed-off-by: Lorenzo Stoakes When I see a patch like this, then I worry, "What if the Sparse annotations are wrong? The patch description doesn't say anything about that." After review then I think the annotations are correct so that's fine. Btw, do you have this hardware? Are you able to test these changes? regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] dgnc: remove DGNC_VERIFY_BOARD macro
On Tue, Mar 10, 2015 at 09:29:19PM +0200, Giedrius Statkevičius wrote:
> static ssize_t dgnc_vpd_show(struct device *p, struct device_attribute
> *attr, char *buf)
> {
> - struct dgnc_board *bd;
> - int count = 0;
> - int i = 0;
> -
> - DGNC_VERIFY_BOARD(p, bd);
> + int count = 0, i = 0;
Don't redo the patch, but really these should be on separate lines.
This is an unwritten rule. :) Also there is no need to set "i".
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/6] staging: sm750fb: correct integer comparison
On Tue, Mar 10, 2015 at 10:46:57PM +0530, Sudip Mukherjee wrote:
> fixed the build warning about comparison of pointer and integer.
> end of string was being compared to NULL.
>
> Signed-off-by: Sudip Mukherjee
> ---
> drivers/staging/sm750fb/sm750.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> index 021b863..5532a28 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -1000,7 +1000,7 @@ static void sm750fb_setup(struct lynx_share *
> share,char * src)
> goto NO_PARAM;
> }
>
> -while((opt = strsep(&src,":")) != NULL && *opt != NULL){
> +while((opt = strsep(&src,":")) != NULL && *opt != 0){
Use '\0' for the NUL char.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: rtl8192e: rtllib_wx: code style improvements
On Wed, Mar 11, 2015 at 08:46:00AM +0100, Mateusz Kulikowski wrote: > Code reformatting based on checkpatch.pl: > - Replaced min() with min_t() > - Replaced printk() with netdev_*() > - Merged broken string That's three different things, which means this should be at least 3 different patches. Please fix up and resend a patch series, as each patch should only do one thing. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: rtl8192e: rtllib_wx: code style improvements
Code reformatting based on checkpatch.pl:
- Replaced min() with min_t()
- Replaced printk() with netdev_*()
- Merged broken string
Signed-off-by: Mateusz Kulikowski
---
Notes:
v2 changes - removed double messages (via netdev_info and RTLLIB_DEBUG_WX)
drivers/staging/rtl8192e/rtllib_wx.c | 33 -
1 file changed, 12 insertions(+), 21 deletions(-)
diff --git a/drivers/staging/rtl8192e/rtllib_wx.c
b/drivers/staging/rtl8192e/rtllib_wx.c
index 9e0f975..72818f3 100644
--- a/drivers/staging/rtl8192e/rtllib_wx.c
+++ b/drivers/staging/rtl8192e/rtllib_wx.c
@@ -74,7 +74,7 @@ static inline char *rtl819x_translate_scan(struct
rtllib_device *ieee,
iwe.cmd = SIOCGIWESSID;
iwe.u.data.flags = 1;
if (network->ssid_len > 0) {
- iwe.u.data.length = min(network->ssid_len, (u8)32);
+ iwe.u.data.length = min_t(u8, network->ssid_len, 32);
start = iwe_stream_add_point_rsl(info, start, stop, &iwe,
network->ssid);
} else if (network->hidden_ssid_len == 0) {
@@ -82,7 +82,7 @@ static inline char *rtl819x_translate_scan(struct
rtllib_device *ieee,
start = iwe_stream_add_point_rsl(info, start, stop,
&iwe, "");
} else {
- iwe.u.data.length = min(network->hidden_ssid_len, (u8)32);
+ iwe.u.data.length = min_t(u8, network->hidden_ssid_len, 32);
start = iwe_stream_add_point_rsl(info, start, stop, &iwe,
network->hidden_ssid);
}
@@ -390,9 +390,7 @@ int rtllib_wx_set_encode(struct rtllib_device *ieee,
kfree(new_crypt);
new_crypt = NULL;
- printk(KERN_WARNING "%s: could not initialize WEP: "
- "load module rtllib_crypt_wep\n",
- dev->name);
+ netdev_warn(dev, "could not initialize WEP: load module
rtllib_crypt_wep\n");
return -EOPNOTSUPP;
}
*crypt = new_crypt;
@@ -423,11 +421,7 @@ int rtllib_wx_set_encode(struct rtllib_device *ieee,
NULL, (*crypt)->priv);
if (len == 0) {
/* Set a default key of all 0 */
- printk(KERN_INFO "Setting key %d to all zero.\n",
- key);
-
- RTLLIB_DEBUG_WX("Setting key %d to all zero.\n",
- key);
+ netdev_info(dev, "Setting key %d to all zero.\n", key);
memset(sec.keys[key], 0, 13);
(*crypt)->ops->set_key(sec.keys[key], 13, NULL,
(*crypt)->priv);
@@ -437,8 +431,8 @@ int rtllib_wx_set_encode(struct rtllib_device *ieee,
/* No key data - just set the default TX key index */
if (key_provided) {
- RTLLIB_DEBUG_WX(
- "Setting key %d to default Tx key.\n", key);
+ RTLLIB_DEBUG_WX("Setting key %d to default Tx key.\n",
+ key);
ieee->crypt_info.tx_keyidx = key;
sec.active_key = key;
sec.flags |= SEC_ACTIVE_KEY;
@@ -469,7 +463,7 @@ int rtllib_wx_set_encode(struct rtllib_device *ieee,
if (ieee->reset_on_keychange &&
ieee->iw_mode != IW_MODE_INFRA &&
ieee->reset_port && ieee->reset_port(dev)) {
- printk(KERN_DEBUG "%s: reset_port failed\n", dev->name);
+ netdev_dbg(dev, "reset_port failed\n");
return -EINVAL;
}
return 0;
@@ -596,7 +590,7 @@ int rtllib_wx_set_encode_ext(struct rtllib_device *ieee,
ret = -EINVAL;
goto done;
}
- printk(KERN_INFO "alg name:%s\n", alg);
+ netdev_info(dev, "alg name:%s\n", alg);
ops = lib80211_get_crypto_ops(alg);
if (ops == NULL) {
@@ -608,9 +602,7 @@ int rtllib_wx_set_encode_ext(struct rtllib_device *ieee,
ops = lib80211_get_crypto_ops(alg);
}
if (ops == NULL) {
- RTLLIB_DEBUG_WX("%s: unknown crypto alg %d\n",
- dev->name, ext->alg);
- printk(KERN_INFO ">unknown crypto alg %d\n", ext->alg);
+ netdev_info(dev, "unknown crypto alg %d\n", ext->alg);
ret = -EINVAL;
goto done;
}
@@ -641,8 +633,7 @@ int rtllib_wx_set_encode_ext(struct rtllib_device *ieee,
if (ext->key_len > 0 && (*crypt)->ops->set_key &&
(*crypt)->ops->set_key(ext->key, ext->key_len, ext->rx_seq,
(*cr
Re: [PATCH 4/6] staging: sm750fb: correct incompatible pointer type
On Wed, Mar 11, 2015 at 12:58:42PM +0530, Sudip Mukherjee wrote: > On Tue, Mar 10, 2015 at 09:17:54PM +0100, Greg Kroah-Hartman wrote: > > On Tue, Mar 10, 2015 at 09:11:00PM +0100, Greg Kroah-Hartman wrote: > > > On Tue, Mar 10, 2015 at 10:46:55PM +0530, Sudip Mukherjee wrote: > > > > we were getting build warnings about assignment of incompatible > > > > > > > That's a nice cleanup, but it's not even the correct cleanup. > > > > Oops, sorry, it is the correct cleanup, I was looking at the '*' > > placement, not the const part. > > > > but the patch needs to be redone, I'll just go fix up the build warnings > > for now... > > thanks Greg for your patience and time to redo this patch. I saw you > broke it into two - three different patches. > But just one doubt, was your compiler showing any warning for the > patch - "Staging: sm750fb: provide error path for > hw_sm750le_setBLANK()" , commit id - > e74ac550298ec4635cc32e99f966568a808fd370 . my compiler didnot show > any warning for that. > mine is gcc 4.7.3, is it time to update? Yes it is, that's a really old version of gcc. Also please wrap your email lines at 72 columns :) > thanks again, and sorry that you had to do fix this. I should have > made the patches in a more proper way. Not a problem, it's part of the learning process. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8192e: rtllib_wx: code style improvements
On 11.03.2015 00:01, Joe Perches wrote:
> On Tue, 2015-03-10 at 23:53 +0100, Mateusz Kulikowski wrote:
>> - Replaced printk() with netdev_*()
>
> trivia:
>
>> diff --git a/drivers/staging/rtl8192e/rtllib_wx.c
>> b/drivers/staging/rtl8192e/rtllib_wx.c
> []
>> @@ -423,11 +421,8 @@ int rtllib_wx_set_encode(struct rtllib_device *ieee,
>> NULL, (*crypt)->priv);
>> if (len == 0) {
>> /* Set a default key of all 0 */
>> -printk(KERN_INFO "Setting key %d to all zero.\n",
>> - key);
>> -
>> -RTLLIB_DEBUG_WX("Setting key %d to all zero.\n",
>> - key);
>> +netdev_info(dev, "Setting key %d to all zero.\n", key);
>> +RTLLIB_DEBUG_WX("Setting key %d to all zero.\n", key);
>
> This isn't something you've created but it
> seems more than a bit nonsensical to emit the
> same message at different logging levels.
Thanks for hint - I'll throw away duplicate messages for v2.
As for all standalone RTLLIB_DEBUG* entries (in whole driver) -
I think it may be better to replace them with netdev_dbg, and use dynamic debug
when needed.
M.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/6] staging: sm750fb: correct incompatible pointer type
On Tue, Mar 10, 2015 at 09:17:54PM +0100, Greg Kroah-Hartman wrote: > On Tue, Mar 10, 2015 at 09:11:00PM +0100, Greg Kroah-Hartman wrote: > > On Tue, Mar 10, 2015 at 10:46:55PM +0530, Sudip Mukherjee wrote: > > > we were getting build warnings about assignment of incompatible > > > > That's a nice cleanup, but it's not even the correct cleanup. > > Oops, sorry, it is the correct cleanup, I was looking at the '*' > placement, not the const part. > > but the patch needs to be redone, I'll just go fix up the build warnings > for now... thanks Greg for your patience and time to redo this patch. I saw you broke it into two - three different patches. But just one doubt, was your compiler showing any warning for the patch - "Staging: sm750fb: provide error path for hw_sm750le_setBLANK()" , commit id - e74ac550298ec4635cc32e99f966568a808fd370 . my compiler didnot show any warning for that. mine is gcc 4.7.3, is it time to update? thanks again, and sorry that you had to do fix this. I should have made the patches in a more proper way. regards sudip > > greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
