[PATCH 1/2] staging: rtlwifi: silence underflow warning

2017-09-29 Thread Dan Carpenter
I'm not totally certain that it's necessary to put an upper limit here.
I think it happens at lower levels.  But if we are going to do that then
we should have a lower bound as well.

Signed-off-by: Dan Carpenter 

diff --git a/drivers/staging/rtlwifi/core.c b/drivers/staging/rtlwifi/core.c
index 43b8b9efe25f..b55e18304a60 100644
--- a/drivers/staging/rtlwifi/core.c
+++ b/drivers/staging/rtlwifi/core.c
@@ -412,7 +412,8 @@ static void _rtl_add_wowlan_patterns(struct ieee80211_hw 
*hw,
for (i = 0; i < wow->n_patterns; i++) {
memset(&rtl_pattern, 0, sizeof(struct rtl_wow_pattern));
memset(mask, 0, MAX_WOL_BIT_MASK_SIZE);
-   if (patterns[i].pattern_len > MAX_WOL_PATTERN_SIZE) {
+   if (patterns[i].pattern_len < 0 ||
+   patterns[i].pattern_len > MAX_WOL_PATTERN_SIZE) {
RT_TRACE(rtlpriv, COMP_POWER, DBG_WARNING,
 "Pattern[%d] is too long\n", i);
continue;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 13/17] media: v4l2-async: simplify v4l2_async_subdev structure

2017-09-29 Thread Mauro Carvalho Chehab
Em Thu, 28 Sep 2017 15:09:21 +0300
Sakari Ailus  escreveu:

> Hi Mauro,
> 
> On Wed, Sep 27, 2017 at 06:46:56PM -0300, Mauro Carvalho Chehab wrote:
> > The V4L2_ASYNC_MATCH_FWNODE match criteria requires just one
> > struct to be filled (struct fwnode_handle). The V4L2_ASYNC_MATCH_DEVNAME
> > match criteria requires just a device name.
> > 
> > So, it doesn't make sense to enclose those into structs,
> > as the criteria can go directly into the union.
> > 
> > That makes easier to document it, as we don't need to document
> > weird senseless structs.  
> 
> The idea is that in the union, there's a struct which is specific to the
> match_type field. I wouldn't call it senseless.

Why a struct for each specific match_type is **needed**? It it is not
needed, then it is senseless per definition :-) 

In the specific case of fwnode, there's already a named struct
for fwnode_handle. The only thing is that it is declared outside
enum v4l2_async_match_type. So, I don't see any reason to do things
like:

struct {
struct fwnode_handle *fwnode;
} fwnode;

If you're in doubt about that, think on how would you document
both fwnode structs. Both fwnode structs specify the match
criteria if %V4L2_ASYNC_MATCH_FWNODE.

The same applies to this:

struct {
const char *name;
} device_name;

Both device_name and name specifies the match criteria if
%V4L2_ASYNC_MATCH_DEVNAME.

> 
> In the two cases there's just a single field in the containing struct. You
> could remove the struct in that case as you do in this patch, and just use
> the field. But I think the result is less clean and so I wouldn't make this
> change.

It is actually cleaner without the stucts.

Without the useless struct, if one wants to match a firmware node, it
should be doing:

pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_FWNODE;
pdata->asd[i]->match.fwnode = of_fwnode_handle(rem);

And that' it. For anyone that reads the above code, even not knowing
all details of "match", is clear that the match criteria is whatever
of_fwnode_handle() returns.

Now, on this:

pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_FWNODE;
pdata->asd[i]->match.fwnode.fwnode = of_fwnode_handle(rem);

It sounds that something is missing, as only one field of match.fwnode
was specified. Anyone not familiar with that non-conventional usage of
a struct with just one struct field inside would need to seek for the
header file declaring the struct. That would consume a lot of time for
code reviewers for no good reason.

The same apply for devname search:

In this case:
asd->match_type = V4L2_ASYNC_MATCH_DEVNAME;
asd->match.device_name.name = imxsd->devname;

I would be expecting something else to be filled at device_name's
struct, for example to specify a case sensitive or case insensitive
match criteria, to allow seeking for a device's substring, or to
allow using other struct device fields to narrow the seek.

With this:

asd->match_type = V4L2_ASYNC_MATCH_DEVNAME;
asd->match.device_name = imxsd->devname;

It is clear that the match criteria is fully specified.

> The confusion comes possibly from the fact that the struct is named the
> same as the field in the struct. These used to be called of and node, but
> with the fwnode property framework the references to the fwnode are, well,
> typically similarly called "fwnode". There's no underlying firmware
> interface with that name, fwnode property API is just an API.

The duplicated "fwnode" name only made it more evident that we don't
need to enclose a single match criteria field inside a struct.

Thanks,
Mauro
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: wlan-ng: resolve sparse Endianness issue.

2017-09-29 Thread Dan Carpenter
Someone already sent this.  Work against linux-next or staging-next.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 07/16] hyper-v: trace vmbus_ongpadl_torndown()

2017-09-29 Thread Vitaly Kuznetsov
Add tracepoint to CHANNELMSG_GPADL_TORNDOWN handler.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/channel_mgmt.c | 2 ++
 drivers/hv/hv_trace.h | 8 
 2 files changed, 10 insertions(+)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index af2448e245ca..1ff2cc064850 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -1076,6 +1076,8 @@ static void vmbus_ongpadl_torndown(
 
gpadl_torndown = (struct vmbus_channel_gpadl_torndown *)hdr;
 
+   trace_vmbus_ongpadl_torndown(gpadl_torndown);
+
/*
 * Find the open msg, copy the result and signal/unblock the wait event
 */
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index e5973ab27c58..0c50527c815c 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -103,6 +103,14 @@ TRACE_EVENT(vmbus_ongpadl_created,
)
);
 
+TRACE_EVENT(vmbus_ongpadl_torndown,
+   TP_PROTO(const struct vmbus_channel_gpadl_torndown *gpadltorndown),
+   TP_ARGS(gpadltorndown),
+   TP_STRUCT__entry(__field(u32, gpadl)),
+   TP_fast_assign(__entry->gpadl = gpadltorndown->gpadl),
+   TP_printk("gpadl 0x%x", __entry->gpadl)
+   );
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.13.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 00/16] Hyper-V: add tracing to VMBus module and trace all messages

2017-09-29 Thread Vitaly Kuznetsov
Changes since v1:
- Use DECLARE_EVENT_CLASS/DEFINE_EVENT_PRINT for vmbus_on_msg_dpc/
  vmbus_onmessage tracing (Steven Rostedt)

Messages between guest and host are used in Hyper-V as control flow. To
simplify debugging various issues which are often hard to reproduce add
tracepoints to all message senders and handlers. This is not a performance
critical path and tracing overhead should be negligible.

The example usage and output is:

Enable all tracing events:
# echo 1 > /sys/kernel/debug/tracing/events/hyperv/enable 

Do something which causes messages to be sent between host and guest, e.g.
hot remove a VMBus device.

Check events:
# cat /sys/kernel/debug/tracing/trace 

# tracer: nop
#
#  _-=> irqs-off
# / _=> need-resched
#| / _---=> hardirq/softirq
#|| / _--=> preempt-depth
#||| / delay
#   TASK-PID   CPU#  TIMESTAMP  FUNCTION
#  | |   |      | |
  -0 [005] .Ns.   465.923970: vmbus_on_msg_dpc: message 2 
received
 ksoftirqd/5-41[005] ..s.   465.923998: vmbus_on_msg_dpc: message 2 
received
 kworker/5:1-64[005]    465.924005: vmbus_on_message: processing 
message 2
 kworker/5:1-64[005]    465.924005: vmbus_onoffer_rescind: 
child_relid 0x16
  -0 [005] ..s.   465.924114: vmbus_on_msg_dpc: message 2 
received
  -0 [005] ..s.   465.924120: vmbus_on_msg_dpc: message 2 
received
  -0 [005] .Ns.   465.924502: vmbus_on_msg_dpc: message 2 
received
 kworker/5:2-208   [005]    465.924520: vmbus_on_message: processing 
message 2
 kworker/5:2-208   [005]    465.924520: vmbus_onoffer_rescind: 
child_relid 0x18
 kworker/5:0-2533  [005]    465.924713: vmbus_on_message: processing 
message 2
 kworker/5:0-2533  [005]    465.924713: vmbus_onoffer_rescind: 
child_relid 0x17
 kworker/5:3-2534  [005]    465.924810: vmbus_on_message: processing 
message 2
 kworker/5:3-2534  [005]    465.924810: vmbus_onoffer_rescind: 
child_relid 0x15
 kworker/5:4-2535  [005]    465.924900: vmbus_on_message: processing 
message 2
 kworker/5:4-2535  [005]    465.924901: vmbus_onoffer_rescind: 
child_relid 0x10
 kworker/5:4-2535  [005]    465.932588: vmbus_close_internal: sending 
child_relid 0x15, ret 0
 kworker/5:4-2535  [005]    465.932623: vmbus_close_internal: sending 
child_relid 0x16, ret 0
 kworker/5:4-2535  [005]    465.932656: vmbus_close_internal: sending 
child_relid 0x17, ret 0
 kworker/5:4-2535  [005]    465.932683: vmbus_close_internal: sending 
child_relid 0x18, ret 0
 kworker/5:4-2535  [005]    465.932709: vmbus_close_internal: sending 
child_relid 0x10, ret 0
 kworker/5:4-2535  [005]    465.986417: vmbus_release_relid: sending 
child_relid 0x10, ret 0

CHANNELMSG_UNLOAD/CHANNELMSG_UNLOAD_RESPONSE are not traced as these are
mostly used on crash.

Vitaly Kuznetsov (16):
  hyper-v: trace vmbus_on_msg_dpc()
  hyper-v: trace vmbus_on_message()
  hyper-v: trace vmbus_onoffer()
  hyper-v: trace vmbus_onoffer_rescind()
  hyper-v: trace vmbus_onopen_result()
  hyper-v: trace vmbus_ongpadl_created()
  hyper-v: trace vmbus_ongpadl_torndown()
  hyper-v: trace vmbus_onversion_response()
  hyper-v: trace vmbus_request_offers()
  hyper-v: trace vmbus_open()
  hyper-v: trace vmbus_close_internal()
  hyper-v: trace vmbus_establish_gpadl()
  hyper-v: trace vmbus_teardown_gpadl()
  hyper-v: trace vmbus_negotiate_version()
  hyper-v: trace vmbus_release_relid()
  hyper-v: trace vmbus_send_tl_connect_request()

 drivers/hv/Makefile   |   4 +-
 drivers/hv/channel.c  |  19 ++-
 drivers/hv/channel_mgmt.c |  26 +++-
 drivers/hv/connection.c   |   3 +
 drivers/hv/hv_trace.c |   4 +
 drivers/hv/hv_trace.h | 304 ++
 drivers/hv/hyperv_vmbus.h |   2 +
 drivers/hv/vmbus_drv.c|   2 +
 8 files changed, 359 insertions(+), 5 deletions(-)
 create mode 100644 drivers/hv/hv_trace.c
 create mode 100644 drivers/hv/hv_trace.h

-- 
2.13.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 02/16] hyper-v: trace vmbus_on_message()

2017-09-29 Thread Vitaly Kuznetsov
Add tracepoint to vmbus_on_message() which is called when we start
processing a blocking from work context.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/channel_mgmt.c | 2 ++
 drivers/hv/hv_trace.h | 5 +
 2 files changed, 7 insertions(+)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 060df71c2e8b..ddeebba8971e 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -1170,6 +1170,8 @@ void vmbus_onmessage(void *context)
hdr = (struct vmbus_channel_message_header *)msg->u.payload;
size = msg->header.payload_size;
 
+   trace_vmbus_on_message(hdr);
+
if (hdr->msgtype >= CHANNELMSG_COUNT) {
pr_err("Received invalid channel message type %d size %d\n",
   hdr->msgtype, size);
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index 82432e351724..4aac780099b4 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -19,6 +19,11 @@ DEFINE_EVENT_PRINT(vmbus_hdr_msg, vmbus_on_msg_dpc,
TP_ARGS(hdr),
TP_printk("message %u received", __entry->msgtype));
 
+DEFINE_EVENT_PRINT(vmbus_hdr_msg, vmbus_on_message,
+TP_PROTO(const struct vmbus_channel_message_header *hdr),
+TP_ARGS(hdr),
+TP_printk("processing message %u", __entry->msgtype));
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.13.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 03/16] hyper-v: trace vmbus_onoffer()

2017-09-29 Thread Vitaly Kuznetsov
Add tracepoint to CHANNELMSG_OFFERCHANNEL handler.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/channel_mgmt.c |  2 ++
 drivers/hv/hv_trace.h | 37 +
 2 files changed, 39 insertions(+)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index ddeebba8971e..290ea25ce409 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -797,6 +797,8 @@ static void vmbus_onoffer(struct 
vmbus_channel_message_header *hdr)
 
offer = (struct vmbus_channel_offer_channel *)hdr;
 
+   trace_vmbus_onoffer(offer);
+
/* Allocate the channel object and save this offer. */
newchannel = alloc_channel();
if (!newchannel) {
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index 4aac780099b4..d38f8ed272ca 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -24,6 +24,43 @@ DEFINE_EVENT_PRINT(vmbus_hdr_msg, vmbus_on_message,
 TP_ARGS(hdr),
 TP_printk("processing message %u", __entry->msgtype));
 
+TRACE_EVENT(vmbus_onoffer,
+   TP_PROTO(const struct vmbus_channel_offer_channel *offer),
+   TP_ARGS(offer),
+   TP_STRUCT__entry(
+   __field(u32, child_relid)
+   __field(u8, monitorid)
+   __field(u16, is_ddc_int)
+   __field(u32, connection_id)
+   __array(char, if_type, 16)
+   __array(char, if_instance, 16)
+   __field(u16, chn_flags)
+   __field(u16, mmio_mb)
+   __field(u16, sub_idx)
+   ),
+   TP_fast_assign(__entry->child_relid = offer->child_relid;
+  __entry->monitorid = offer->monitorid;
+  __entry->is_ddc_int = offer->is_dedicated_interrupt;
+  __entry->connection_id = offer->connection_id;
+  memcpy(__entry->if_type,
+ &offer->offer.if_type.b, 16);
+  memcpy(__entry->if_instance,
+ &offer->offer.if_instance.b, 16);
+  __entry->chn_flags = offer->offer.chn_flags;
+  __entry->mmio_mb = offer->offer.mmio_megabytes;
+  __entry->sub_idx = offer->offer.sub_channel_index;
+   ),
+   TP_printk("child_relid 0x%x, monitorid 0x%x, is_dedicated %d, "
+ "connection_id 0x%x, if_type %pUl, if_instance %pUl, "
+ "chn_flags 0x%x, mmio_megabytes %d, sub_channel_index %d",
+ __entry->child_relid, __entry->monitorid,
+ __entry->is_ddc_int, __entry->connection_id,
+ __entry->if_type, __entry->if_instance,
+ __entry->chn_flags, __entry->mmio_mb,
+ __entry->sub_idx
+   )
+   );
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.13.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 09/16] hyper-v: trace vmbus_request_offers()

2017-09-29 Thread Vitaly Kuznetsov
Add tracepoint to CHANNELMSG_REQUESTOFFERS sender.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/channel_mgmt.c | 4 +++-
 drivers/hv/hv_trace.h | 8 
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index a9a1fc1424c6..5c39388545f5 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -1218,9 +1218,11 @@ int vmbus_request_offers(void)
 
msg->msgtype = CHANNELMSG_REQUESTOFFERS;
 
-
ret = vmbus_post_msg(msg, sizeof(struct vmbus_channel_message_header),
 true);
+
+   trace_vmbus_request_offers(ret);
+
if (ret != 0) {
pr_err("Unable to request offers - %d\n", ret);
 
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index e99db56d27b7..7212d8898775 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -122,6 +122,14 @@ TRACE_EVENT(vmbus_onversion_response,
TP_printk("version_supported %d", __entry->ver)
);
 
+TRACE_EVENT(vmbus_request_offers,
+   TP_PROTO(int ret),
+   TP_ARGS(ret),
+   TP_STRUCT__entry(__field(int, ret)),
+   TP_fast_assign(__entry->ret = ret),
+   TP_printk("sending ret %d", __entry->ret)
+   );
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.13.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 13/16] hyper-v: trace vmbus_teardown_gpadl()

2017-09-29 Thread Vitaly Kuznetsov
Add tracepoint to CHANNELMSG_GPADL_TEARDOWN sender.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/channel.c  |  2 ++
 drivers/hv/hv_trace.h | 18 ++
 2 files changed, 20 insertions(+)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index cf6bc0667cde..9cb81838e7bb 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -519,6 +519,8 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 
gpadl_handle)
ret = vmbus_post_msg(msg, sizeof(struct vmbus_channel_gpadl_teardown),
 true);
 
+   trace_vmbus_teardown_gpadl(msg, ret);
+
if (ret)
goto post_msg_err;
 
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index 9eb8ee51a77f..6c585a012a40 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -214,6 +214,24 @@ TRACE_EVENT(vmbus_establish_gpadl_body,
)
);
 
+TRACE_EVENT(vmbus_teardown_gpadl,
+   TP_PROTO(const struct vmbus_channel_gpadl_teardown *msg, int ret),
+   TP_ARGS(msg, ret),
+   TP_STRUCT__entry(
+   __field(u32, child_relid)
+   __field(u32, gpadl)
+   __field(int, ret)
+   ),
+   TP_fast_assign(
+   __entry->child_relid = msg->child_relid;
+   __entry->gpadl = msg->gpadl;
+   __entry->ret = ret;
+   ),
+   TP_printk("sending child_relid 0x%x, gpadl 0x%x, ret %d",
+ __entry->child_relid, __entry->gpadl, __entry->ret
+   )
+   );
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.13.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 15/16] hyper-v: trace vmbus_release_relid()

2017-09-29 Thread Vitaly Kuznetsov
Add tracepoint to CHANNELMSG_RELID_RELEASED sender.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/channel_mgmt.c |  7 +--
 drivers/hv/hv_trace.h | 16 
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 5c39388545f5..ca2dea638bfb 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -373,12 +373,15 @@ static void percpu_channel_deq(void *arg)
 static void vmbus_release_relid(u32 relid)
 {
struct vmbus_channel_relid_released msg;
+   int ret;
 
memset(&msg, 0, sizeof(struct vmbus_channel_relid_released));
msg.child_relid = relid;
msg.header.msgtype = CHANNELMSG_RELID_RELEASED;
-   vmbus_post_msg(&msg, sizeof(struct vmbus_channel_relid_released),
-  true);
+   ret = vmbus_post_msg(&msg, sizeof(struct vmbus_channel_relid_released),
+true);
+
+   trace_vmbus_release_relid(&msg, ret);
 }
 
 void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid)
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index 570482014740..3e751f948a5f 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -258,6 +258,22 @@ TRACE_EVENT(vmbus_negotiate_version,
)
);
 
+TRACE_EVENT(vmbus_release_relid,
+   TP_PROTO(const struct vmbus_channel_relid_released *msg, int ret),
+   TP_ARGS(msg, ret),
+   TP_STRUCT__entry(
+   __field(u32, child_relid)
+   __field(int, ret)
+   ),
+   TP_fast_assign(
+   __entry->child_relid = msg->child_relid;
+   __entry->ret = ret;
+   ),
+   TP_printk("sending child_relid 0x%x, ret %d",
+ __entry->child_relid, __entry->ret
+   )
+   );
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.13.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 10/16] hyper-v: trace vmbus_open()

2017-09-29 Thread Vitaly Kuznetsov
Add tracepoint to CHANNELMSG_OPENCHANNEL sender.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/channel.c  |  2 ++
 drivers/hv/hv_trace.h | 27 +++
 2 files changed, 29 insertions(+)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index efd5db743319..82cb57e2d6bd 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -185,6 +185,8 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 
send_ringbuffer_size,
ret = vmbus_post_msg(open_msg,
 sizeof(struct vmbus_channel_open_channel), true);
 
+   trace_vmbus_open(open_msg, ret);
+
if (ret != 0) {
err = ret;
goto error_clean_msglist;
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index 7212d8898775..01acbbf7badf 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -130,6 +130,33 @@ TRACE_EVENT(vmbus_request_offers,
TP_printk("sending ret %d", __entry->ret)
);
 
+TRACE_EVENT(vmbus_open,
+   TP_PROTO(const struct vmbus_channel_open_channel *msg, int ret),
+   TP_ARGS(msg, ret),
+   TP_STRUCT__entry(
+   __field(u32, child_relid)
+   __field(u32, openid)
+   __field(u32, gpadlhandle)
+   __field(u32, target_vp)
+   __field(u32, offset)
+   __field(int, ret)
+   ),
+   TP_fast_assign(
+   __entry->child_relid = msg->child_relid;
+   __entry->openid = msg->openid;
+   __entry->gpadlhandle = msg->ringbuffer_gpadlhandle;
+   __entry->target_vp = msg->target_vp;
+   __entry->offset = msg->downstream_ringbuffer_pageoffset;
+   __entry->ret = ret;
+   ),
+   TP_printk("sending child_relid 0x%x, openid %d, "
+ "gpadlhandle 0x%x, target_vp 0x%x, offset 0x%x, ret %d",
+ __entry->child_relid,  __entry->openid,
+ __entry->gpadlhandle, __entry->target_vp,
+ __entry->offset, __entry->ret
+   )
+   );
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.13.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 04/16] hyper-v: trace vmbus_onoffer_rescind()

2017-09-29 Thread Vitaly Kuznetsov
Add tracepoint to CHANNELMSG_RESCIND_CHANNELOFFER handler.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/channel_mgmt.c | 2 ++
 drivers/hv/hv_trace.h | 8 
 2 files changed, 10 insertions(+)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 290ea25ce409..563653a02ab6 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -841,6 +841,8 @@ static void vmbus_onoffer_rescind(struct 
vmbus_channel_message_header *hdr)
 
rescind = (struct vmbus_channel_rescind_offer *)hdr;
 
+   trace_vmbus_onoffer_rescind(rescind);
+
/*
 * The offer msg and the corresponding rescind msg
 * from the host are guranteed to be ordered -
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index d38f8ed272ca..8bb8806a16f7 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -61,6 +61,14 @@ TRACE_EVENT(vmbus_onoffer,
)
);
 
+TRACE_EVENT(vmbus_onoffer_rescind,
+   TP_PROTO(const struct vmbus_channel_rescind_offer *offer),
+   TP_ARGS(offer),
+   TP_STRUCT__entry(__field(u32, child_relid)),
+   TP_fast_assign(__entry->child_relid = offer->child_relid),
+   TP_printk("child_relid 0x%x", __entry->child_relid)
+   );
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.13.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 08/16] hyper-v: trace vmbus_onversion_response()

2017-09-29 Thread Vitaly Kuznetsov
Add tracepoint to CHANNELMSG_VERSION_RESPONSE handler.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/channel_mgmt.c |  3 +++
 drivers/hv/hv_trace.h | 11 +++
 2 files changed, 14 insertions(+)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 1ff2cc064850..a9a1fc1424c6 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -1121,6 +1121,9 @@ static void vmbus_onversion_response(
unsigned long flags;
 
version_response = (struct vmbus_channel_version_response *)hdr;
+
+   trace_vmbus_onversion_response(version_response);
+
spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags);
 
list_for_each_entry(msginfo, &vmbus_connection.chn_msg_list,
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index 0c50527c815c..e99db56d27b7 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -111,6 +111,17 @@ TRACE_EVENT(vmbus_ongpadl_torndown,
TP_printk("gpadl 0x%x", __entry->gpadl)
);
 
+TRACE_EVENT(vmbus_onversion_response,
+   TP_PROTO(const struct vmbus_channel_version_response *response),
+   TP_ARGS(response),
+   TP_STRUCT__entry(
+   __field(u8, ver)
+   ),
+   TP_fast_assign(__entry->ver = response->version_supported;
+   ),
+   TP_printk("version_supported %d", __entry->ver)
+   );
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.13.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 14/16] hyper-v: trace vmbus_negotiate_version()

2017-09-29 Thread Vitaly Kuznetsov
Add tracepoint to CHANNELMSG_INITIATE_CONTACT sender.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/connection.c |  3 +++
 drivers/hv/hv_trace.h   | 26 ++
 2 files changed, 29 insertions(+)

diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index f41901f80b64..5e36f86d0b0c 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -117,6 +117,9 @@ static int vmbus_negotiate_version(struct 
vmbus_channel_msginfo *msginfo,
ret = vmbus_post_msg(msg,
 sizeof(struct vmbus_channel_initiate_contact),
 true);
+
+   trace_vmbus_negotiate_version(msg, ret);
+
if (ret != 0) {
spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags);
list_del(&msginfo->msglistentry);
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index 6c585a012a40..570482014740 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -232,6 +232,32 @@ TRACE_EVENT(vmbus_teardown_gpadl,
)
);
 
+TRACE_EVENT(vmbus_negotiate_version,
+   TP_PROTO(const struct vmbus_channel_initiate_contact *msg, int ret),
+   TP_ARGS(msg, ret),
+   TP_STRUCT__entry(
+   __field(u32, ver)
+   __field(u32, target_vcpu)
+   __field(int, ret)
+   __field(u64, int_page)
+   __field(u64, mon_page1)
+   __field(u64, mon_page2)
+   ),
+   TP_fast_assign(
+   __entry->ver = msg->vmbus_version_requested;
+   __entry->target_vcpu = msg->target_vcpu;
+   __entry->int_page = msg->interrupt_page;
+   __entry->mon_page1 = msg->monitor_page1;
+   __entry->mon_page2 = msg->monitor_page2;
+   __entry->ret = ret;
+   ),
+   TP_printk("sending vmbus_version_requested %d, target_vcpu 0x%x, "
+ "pages %llx:%llx:%llx, ret %d",
+ __entry->ver, __entry->target_vcpu, __entry->int_page,
+ __entry->mon_page1, __entry->mon_page2, __entry->ret
+   )
+   );
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.13.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 05/16] hyper-v: trace vmbus_onopen_result()

2017-09-29 Thread Vitaly Kuznetsov
Add tracepoint to CHANNELMSG_OPENCHANNEL_RESULT handler.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/channel_mgmt.c |  2 ++
 drivers/hv/hv_trace.h | 17 +
 2 files changed, 19 insertions(+)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 563653a02ab6..2abe0563876b 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -980,6 +980,8 @@ static void vmbus_onopen_result(struct 
vmbus_channel_message_header *hdr)
 
result = (struct vmbus_channel_open_result *)hdr;
 
+   trace_vmbus_onopen_result(result);
+
/*
 * Find the open msg, copy the result and signal/unblock the wait event
 */
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index 8bb8806a16f7..c42653c21238 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -69,6 +69,23 @@ TRACE_EVENT(vmbus_onoffer_rescind,
TP_printk("child_relid 0x%x", __entry->child_relid)
);
 
+TRACE_EVENT(vmbus_onopen_result,
+   TP_PROTO(const struct vmbus_channel_open_result *result),
+   TP_ARGS(result),
+   TP_STRUCT__entry(
+   __field(u32, child_relid)
+   __field(u32, openid)
+   __field(u32, status)
+   ),
+   TP_fast_assign(__entry->child_relid = result->child_relid;
+  __entry->openid = result->openid;
+  __entry->status = result->status;
+   ),
+   TP_printk("child_relid 0x%x, openid %d, status %d",
+ __entry->child_relid,  __entry->openid,  __entry->status
+   )
+   );
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.13.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 11/16] hyper-v: trace vmbus_close_internal()

2017-09-29 Thread Vitaly Kuznetsov
Add tracepoint to CHANNELMSG_CLOSECHANNEL sender.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/channel.c  |  2 ++
 drivers/hv/hv_trace.h | 15 +++
 2 files changed, 17 insertions(+)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 82cb57e2d6bd..f919d9dd984b 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -591,6 +591,8 @@ static int vmbus_close_internal(struct vmbus_channel 
*channel)
ret = vmbus_post_msg(msg, sizeof(struct vmbus_channel_close_channel),
 true);
 
+   trace_vmbus_close_internal(msg, ret);
+
if (ret) {
pr_err("Close failed: close post msg return is %d\n", ret);
/*
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index 01acbbf7badf..ecfacb114331 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -157,6 +157,21 @@ TRACE_EVENT(vmbus_open,
)
);
 
+TRACE_EVENT(vmbus_close_internal,
+   TP_PROTO(const struct vmbus_channel_close_channel *msg, int ret),
+   TP_ARGS(msg, ret),
+   TP_STRUCT__entry(
+   __field(u32, child_relid)
+   __field(int, ret)
+   ),
+   TP_fast_assign(
+   __entry->child_relid = msg->child_relid;
+   __entry->ret = ret;
+   ),
+   TP_printk("sending child_relid 0x%x, ret %d", __entry->child_relid,
+   __entry->ret)
+   );
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.13.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 12/16] hyper-v: trace vmbus_establish_gpadl()

2017-09-29 Thread Vitaly Kuznetsov
Add tracepoint to CHANNELMSG_GPADL_HEADER/CHANNELMSG_GPADL_BODY sender.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/channel.c  |  6 ++
 drivers/hv/hv_trace.h | 42 ++
 2 files changed, 48 insertions(+)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index f919d9dd984b..cf6bc0667cde 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -435,6 +435,9 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, 
void *kbuffer,
 
ret = vmbus_post_msg(gpadlmsg, msginfo->msgsize -
 sizeof(*msginfo), true);
+
+   trace_vmbus_establish_gpadl_header(gpadlmsg, ret);
+
if (ret != 0)
goto cleanup;
 
@@ -450,6 +453,9 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, 
void *kbuffer,
ret = vmbus_post_msg(gpadl_body,
 submsginfo->msgsize - sizeof(*submsginfo),
 true);
+
+   trace_vmbus_establish_gpadl_body(gpadl_body, ret);
+
if (ret != 0)
goto cleanup;
 
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index ecfacb114331..9eb8ee51a77f 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -172,6 +172,48 @@ TRACE_EVENT(vmbus_close_internal,
__entry->ret)
);
 
+TRACE_EVENT(vmbus_establish_gpadl_header,
+   TP_PROTO(const struct vmbus_channel_gpadl_header *msg, int ret),
+   TP_ARGS(msg, ret),
+   TP_STRUCT__entry(
+   __field(u32, child_relid)
+   __field(u32, gpadl)
+   __field(u16, range_buflen)
+   __field(u16, rangecount)
+   __field(int, ret)
+   ),
+   TP_fast_assign(
+   __entry->child_relid = msg->child_relid;
+   __entry->gpadl = msg->gpadl;
+   __entry->range_buflen = msg->range_buflen;
+   __entry->rangecount = msg->rangecount;
+   __entry->ret = ret;
+   ),
+   TP_printk("sending child_relid 0x%x, gpadl 0x%x, range_buflen %d "
+ "rangecount %d, ret %d",
+ __entry->child_relid, __entry->gpadl,
+ __entry->range_buflen, __entry->rangecount, __entry->ret
+   )
+   );
+
+TRACE_EVENT(vmbus_establish_gpadl_body,
+   TP_PROTO(const struct vmbus_channel_gpadl_body *msg, int ret),
+   TP_ARGS(msg, ret),
+   TP_STRUCT__entry(
+   __field(u32, msgnumber)
+   __field(u32, gpadl)
+   __field(int, ret)
+   ),
+   TP_fast_assign(
+   __entry->msgnumber = msg->msgnumber;
+   __entry->gpadl = msg->gpadl;
+   __entry->ret = ret;
+   ),
+   TP_printk("sending msgnumber %d, gpadl 0x%x, ret %d",
+ __entry->msgnumber, __entry->gpadl, __entry->ret
+   )
+   );
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.13.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 16/16] hyper-v: trace vmbus_send_tl_connect_request()

2017-09-29 Thread Vitaly Kuznetsov
Add tracepoint to CHANNELMSG_TL_CONNECT_REQUEST sender.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/channel.c  |  7 ++-
 drivers/hv/hv_trace.h | 20 
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 9cb81838e7bb..3efe9e8ab079 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -236,13 +236,18 @@ int vmbus_send_tl_connect_request(const uuid_le 
*shv_guest_servie_id,
  const uuid_le *shv_host_servie_id)
 {
struct vmbus_channel_tl_connect_request conn_msg;
+   int ret;
 
memset(&conn_msg, 0, sizeof(conn_msg));
conn_msg.header.msgtype = CHANNELMSG_TL_CONNECT_REQUEST;
conn_msg.guest_endpoint_id = *shv_guest_servie_id;
conn_msg.host_service_id = *shv_host_servie_id;
 
-   return vmbus_post_msg(&conn_msg, sizeof(conn_msg), true);
+   ret = vmbus_post_msg(&conn_msg, sizeof(conn_msg), true);
+
+   trace_vmbus_send_tl_connect_request(&conn_msg, ret);
+
+   return ret;
 }
 EXPORT_SYMBOL_GPL(vmbus_send_tl_connect_request);
 
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index 3e751f948a5f..3a7eaff355dd 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -274,6 +274,26 @@ TRACE_EVENT(vmbus_release_relid,
)
);
 
+TRACE_EVENT(vmbus_send_tl_connect_request,
+   TP_PROTO(const struct vmbus_channel_tl_connect_request *msg,
+int ret),
+   TP_ARGS(msg, ret),
+   TP_STRUCT__entry(
+   __array(char, guest_id, 16)
+   __array(char, host_id, 16)
+   __field(int, ret)
+   ),
+   TP_fast_assign(
+   memcpy(__entry->guest_id, &msg->guest_endpoint_id.b, 16);
+   memcpy(__entry->host_id, &msg->host_service_id.b, 16);
+   __entry->ret = ret;
+   ),
+   TP_printk("sending guest_endpoint_id %pUl, host_service_id %pUl, "
+ "ret %d",
+ __entry->guest_id, __entry->host_id, __entry->ret
+   )
+   );
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.13.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 01/16] hyper-v: trace vmbus_on_msg_dpc()

2017-09-29 Thread Vitaly Kuznetsov
Add tracing subsystem to Hyper-V VMBus module and add tracepoint
to vmbus_on_msg_dpc() which is called when we receive a message from host.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/Makefile   |  4 +++-
 drivers/hv/hv_trace.c |  4 
 drivers/hv/hv_trace.h | 29 +
 drivers/hv/hyperv_vmbus.h |  2 ++
 drivers/hv/vmbus_drv.c|  2 ++
 5 files changed, 40 insertions(+), 1 deletion(-)
 create mode 100644 drivers/hv/hv_trace.c
 create mode 100644 drivers/hv/hv_trace.h

diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
index 39c9b2c08d33..ad791e00230f 100644
--- a/drivers/hv/Makefile
+++ b/drivers/hv/Makefile
@@ -2,7 +2,9 @@ obj-$(CONFIG_HYPERV)+= hv_vmbus.o
 obj-$(CONFIG_HYPERV_UTILS) += hv_utils.o
 obj-$(CONFIG_HYPERV_BALLOON)   += hv_balloon.o
 
+CFLAGS_hv_trace.o = -I$(src)
+
 hv_vmbus-y := vmbus_drv.o \
 hv.o connection.o channel.o \
-channel_mgmt.o ring_buffer.o
+channel_mgmt.o ring_buffer.o hv_trace.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_trace.c b/drivers/hv/hv_trace.c
new file mode 100644
index ..df47acd01a81
--- /dev/null
+++ b/drivers/hv/hv_trace.c
@@ -0,0 +1,4 @@
+#include "hyperv_vmbus.h"
+
+#define CREATE_TRACE_POINTS
+#include "hv_trace.h"
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
new file mode 100644
index ..82432e351724
--- /dev/null
+++ b/drivers/hv/hv_trace.h
@@ -0,0 +1,29 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM hyperv
+
+#if !defined(_HV_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _HV_TRACE_H
+
+#include 
+
+DECLARE_EVENT_CLASS(vmbus_hdr_msg,
+   TP_PROTO(const struct vmbus_channel_message_header *hdr),
+   TP_ARGS(hdr),
+   TP_STRUCT__entry(__field(unsigned int, msgtype)),
+   TP_fast_assign(__entry->msgtype = hdr->msgtype;),
+   TP_printk("msgtype=%d", __entry->msgtype)
+);
+
+DEFINE_EVENT_PRINT(vmbus_hdr_msg, vmbus_on_msg_dpc,
+   TP_PROTO(const struct vmbus_channel_message_header *hdr),
+   TP_ARGS(hdr),
+   TP_printk("message %u received", __entry->msgtype));
+
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE hv_trace
+#endif /* _HV_TRACE_H */
+
+/* This part must be outside protection */
+#include 
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 49569f8fe038..82eb082f3302 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -31,6 +31,8 @@
 #include 
 #include 
 
+#include "hv_trace.h"
+
 /*
  * Timeout for services such as KVP and fcopy.
  */
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index a9d49f6f6501..ced33b1982c4 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -834,6 +834,8 @@ void vmbus_on_msg_dpc(unsigned long data)
 
hdr = (struct vmbus_channel_message_header *)msg->u.payload;
 
+   trace_vmbus_on_msg_dpc(hdr);
+
if (hdr->msgtype >= CHANNELMSG_COUNT) {
WARN_ONCE(1, "unknown msgtype=%d\n", hdr->msgtype);
goto msg_handled;
-- 
2.13.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 06/16] hyper-v: trace vmbus_ongpadl_created()

2017-09-29 Thread Vitaly Kuznetsov
Add tracepoint to CHANNELMSG_GPADL_CREATED handler.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/channel_mgmt.c |  2 ++
 drivers/hv/hv_trace.h | 17 +
 2 files changed, 19 insertions(+)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 2abe0563876b..af2448e245ca 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -1026,6 +1026,8 @@ static void vmbus_ongpadl_created(struct 
vmbus_channel_message_header *hdr)
 
gpadlcreated = (struct vmbus_channel_gpadl_created *)hdr;
 
+   trace_vmbus_ongpadl_created(gpadlcreated);
+
/*
 * Find the establish msg, copy the result and signal/unblock the wait
 * event
diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
index c42653c21238..e5973ab27c58 100644
--- a/drivers/hv/hv_trace.h
+++ b/drivers/hv/hv_trace.h
@@ -86,6 +86,23 @@ TRACE_EVENT(vmbus_onopen_result,
)
);
 
+TRACE_EVENT(vmbus_ongpadl_created,
+   TP_PROTO(const struct vmbus_channel_gpadl_created *gpadlcreated),
+   TP_ARGS(gpadlcreated),
+   TP_STRUCT__entry(
+   __field(u32, child_relid)
+   __field(u32, gpadl)
+   __field(u32, status)
+   ),
+   TP_fast_assign(__entry->child_relid = gpadlcreated->child_relid;
+  __entry->gpadl = gpadlcreated->gpadl;
+  __entry->status = gpadlcreated->creation_status;
+   ),
+   TP_printk("child_relid 0x%x, gpadl 0x%x, creation_status %d",
+ __entry->child_relid,  __entry->gpadl,  __entry->status
+   )
+   );
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
-- 
2.13.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [RESEND PATCH 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver

2017-09-29 Thread Bogdan Purcareata
> Introduce the DPAA2 Ethernet Switch driver, which manages Datapath Switch
> (DPSW) objects discovered on the MC bus.
> 
> Suggested-by: Alexandru Marginean 
> Signed-off-by: Razvan Stefanescu 
> ---
>  drivers/staging/fsl-dpaa2/ethsw/Makefile |2 +-
>  drivers/staging/fsl-dpaa2/ethsw/ethsw.c  | 1523 
> ++
>  drivers/staging/fsl-dpaa2/ethsw/ethsw.h  |   88 ++
>  3 files changed, 1612 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/staging/fsl-dpaa2/ethsw/ethsw.c
>  create mode 100644 drivers/staging/fsl-dpaa2/ethsw/ethsw.h
> 
> diff --git a/drivers/staging/fsl-dpaa2/ethsw/Makefile b/drivers/staging/fsl-
> dpaa2/ethsw/Makefile
> index db137f7..a6d72d1 100644
> --- a/drivers/staging/fsl-dpaa2/ethsw/Makefile
> +++ b/drivers/staging/fsl-dpaa2/ethsw/Makefile
> @@ -4,4 +4,4 @@
> 
>  obj-$(CONFIG_FSL_DPAA2_ETHSW) += dpaa2-ethsw.o
> 
> -dpaa2-ethsw-objs := dpsw.o
> +dpaa2-ethsw-objs := ethsw.o dpsw.o
> diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-
> dpaa2/ethsw/ethsw.c
> new file mode 100644
> index 000..ae86078
> --- /dev/null
> +++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
> @@ -0,0 +1,1523 @@
> +/* Copyright 2014-2016 Freescale Semiconductor Inc.
> + * Copyright 2017 NXP
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are 
> met:
> + * * Redistributions of source code must retain the above copyright
> + *notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> + *notice, this list of conditions and the following disclaimer in the
> + *documentation and/or other materials provided with the distribution.
> + * * Neither the name of the above-listed copyright holders nor the
> + *names of any contributors may be used to endorse or promote products
> + *derived from this software without specific prior written permission.
> + *
> + *
> + * ALTERNATIVELY, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") as published by the Free Software
> + * Foundation, either version 2 of that License or (at your option) any
> + * later version.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS 
> IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "../../fsl-mc/include/mc.h"
> +
> +#include "ethsw.h"
> +
> +static struct workqueue_struct *ethsw_owq;
> +
> +/* Minimal supported DPSW version */
> +#define DPSW_MIN_VER_MAJOR   8
> +#define DPSW_MIN_VER_MINOR   0
> +
> +#define DEFAULT_VLAN_ID  1
> +
> +static int ethsw_add_vlan(struct ethsw_core *ethsw, u16 vid)
> +{
> + int err;
> +
> + struct dpsw_vlan_cfgvcfg = {
> + .fdb_id = 0,
> + };
> +
> + if (ethsw->vlans[vid]) {
> + dev_err(ethsw->dev, "VLAN already configured\n");
> + return -EEXIST;
> + }
> +
> + err = dpsw_vlan_add(ethsw->mc_io, 0,
> + ethsw->dpsw_handle, vid, &vcfg);
> + if (err) {
> + dev_err(ethsw->dev, "dpsw_vlan_add err %d\n", err);
> + return err;
> + }
> + ethsw->vlans[vid] = ETHSW_VLAN_MEMBER;/
> +
> + return 0;
> +}
> +
> +static int ethsw_port_add_vlan(struct ethsw_port_priv *port_priv,
> +u16 vid, u16 flags)
> +{
> + struct ethsw_core *ethsw = port_priv->ethsw_data;
> + struct net_device *netdev = port_priv->netdev;
> + struct dpsw_vlan_if_cfg vcfg;
> + bool is_oper;
> + int err, err2;

Mild suggestion - s/err2/ret/, just because it sounds better, at least to me 
(same for similar situations in the rest of the file).

> +
> + if (port_priv->vlans[vid]) {
> + netdev_warn(netdev, "VLAN %d already configured\n", vid);
> + return -EEXIST;
> + }
> +
> + vcfg.num_ifs = 1;
> + vcfg.if_id[0] = port_priv->idx;
> + err = dpsw_vlan_add_if(ethsw->mc_io, 0, ethsw->dpsw_handle, vid, &vcfg);
> + if (err) {
> + netdev_err(netdev, "dpsw_vlan_add_if err %d\n", err);
>

RE: [RESEND PATCH 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver

2017-09-29 Thread Razvan Stefanescu


> -Original Message-
> From: Bogdan Purcareata
> Sent: Friday, September 29, 2017 16:36
> To: Razvan Stefanescu ;
> gre...@linuxfoundation.org
> Cc: de...@driverdev.osuosl.org; linux-ker...@vger.kernel.org;
> net...@vger.kernel.org; ag...@suse.de; a...@arndb.de; Alexandru Marginean
> ; Ruxandra Ioana Radulescu
> ; Laurentiu Tudor ;
> stuyo...@gmail.com
> Subject: RE: [RESEND PATCH 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2
> Ethernet Switch driver
> 
> > Introduce the DPAA2 Ethernet Switch driver, which manages Datapath Switch
> > (DPSW) objects discovered on the MC bus.
> >
> > Suggested-by: Alexandru Marginean 
> > Signed-off-by: Razvan Stefanescu 
> > ---
> >  drivers/staging/fsl-dpaa2/ethsw/Makefile |2 +-
> >  drivers/staging/fsl-dpaa2/ethsw/ethsw.c  | 1523
> ++
> >  drivers/staging/fsl-dpaa2/ethsw/ethsw.h  |   88 ++
> >  3 files changed, 1612 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/staging/fsl-dpaa2/ethsw/ethsw.c
> >  create mode 100644 drivers/staging/fsl-dpaa2/ethsw/ethsw.h
> >
> > diff --git a/drivers/staging/fsl-dpaa2/ethsw/Makefile b/drivers/staging/fsl-
> > dpaa2/ethsw/Makefile
> > index db137f7..a6d72d1 100644
> > --- a/drivers/staging/fsl-dpaa2/ethsw/Makefile
> > +++ b/drivers/staging/fsl-dpaa2/ethsw/Makefile
> > @@ -4,4 +4,4 @@
> >
> >  obj-$(CONFIG_FSL_DPAA2_ETHSW) += dpaa2-ethsw.o
> >
> > -dpaa2-ethsw-objs := dpsw.o
> > +dpaa2-ethsw-objs := ethsw.o dpsw.o
> > diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-
> > dpaa2/ethsw/ethsw.c
> > new file mode 100644
> > index 000..ae86078
> > --- /dev/null
> > +++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
> > @@ -0,0 +1,1523 @@
> > +/* Copyright 2014-2016 Freescale Semiconductor Inc.
> > + * Copyright 2017 NXP
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions are 
> > met:
> > + * * Redistributions of source code must retain the above copyright
> > + *  notice, this list of conditions and the following disclaimer.
> > + * * Redistributions in binary form must reproduce the above copyright
> > + *  notice, this list of conditions and the following disclaimer in the
> > + *  documentation and/or other materials provided with the distribution.
> > + * * Neither the name of the above-listed copyright holders nor the
> > + *  names of any contributors may be used to endorse or promote products
> > + *  derived from this software without specific prior written permission.
> > + *
> > + *
> > + * ALTERNATIVELY, this software may be distributed under the terms of the
> > + * GNU General Public License ("GPL") as published by the Free Software
> > + * Foundation, either version 2 of that License or (at your option) any
> > + * later version.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS "AS IS"
> > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> LIMITED TO, THE
> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
> PARTICULAR PURPOSE
> > + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR
> CONTRIBUTORS BE
> > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
> PROCUREMENT OF
> > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> BUSINESS
> > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
> WHETHER IN
> > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
> OTHERWISE)
> > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
> ADVISED OF THE
> > + * POSSIBILITY OF SUCH DAMAGE.
> > + */
> > +
> > +#include 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "../../fsl-mc/include/mc.h"
> > +
> > +#include "ethsw.h"
> > +
> > +static struct workqueue_struct *ethsw_owq;
> > +
> > +/* Minimal supported DPSW version */
> > +#define DPSW_MIN_VER_MAJOR 8
> > +#define DPSW_MIN_VER_MINOR 0
> > +
> > +#define DEFAULT_VLAN_ID1
> > +
> > +static int ethsw_add_vlan(struct ethsw_core *ethsw, u16 vid)
> > +{
> > +   int err;
> > +
> > +   struct dpsw_vlan_cfgvcfg = {
> > +   .fdb_id = 0,
> > +   };
> > +
> > +   if (ethsw->vlans[vid]) {
> > +   dev_err(ethsw->dev, "VLAN already configured\n");
> > +   return -EEXIST;
> > +   }
> > +
> > +   err = dpsw_vlan_add(ethsw->mc_io, 0,
> > +   ethsw->dpsw_handle, vid, &vcfg);
> > +   if (err) {
> > +   dev_err(ethsw->dev, "dpsw_vlan_add err %d\n", err);
> > +   return err;
> > +   }
> > +   ethsw->vlans[vid] = ETHSW_VLAN_MEMBER;/
> > +
> > +   return 0;
> > +}
> > +
> > +static int ethsw_port_add_vlan(struct ethsw_port_priv *port_priv,
> > +  u16 vid, u16 flags)
> > +{
> > +   struct ethsw_core *ethsw = port_priv->ethsw_data;
> > +   struct n

Re: [PATCH 1/2] staging: rtlwifi: silence underflow warning

2017-09-29 Thread Larry Finger

On 09/29/2017 02:51 AM, Dan Carpenter wrote:

I'm not totally certain that it's necessary to put an upper limit here.
I think it happens at lower levels.  But if we are going to do that then
we should have a lower bound as well.

Signed-off-by: Dan Carpenter 

diff --git a/drivers/staging/rtlwifi/core.c b/drivers/staging/rtlwifi/core.c
index 43b8b9efe25f..b55e18304a60 100644
--- a/drivers/staging/rtlwifi/core.c
+++ b/drivers/staging/rtlwifi/core.c
@@ -412,7 +412,8 @@ static void _rtl_add_wowlan_patterns(struct ieee80211_hw 
*hw,
for (i = 0; i < wow->n_patterns; i++) {
memset(&rtl_pattern, 0, sizeof(struct rtl_wow_pattern));
memset(mask, 0, MAX_WOL_BIT_MASK_SIZE);
-   if (patterns[i].pattern_len > MAX_WOL_PATTERN_SIZE) {
+   if (patterns[i].pattern_len < 0 ||
+   patterns[i].pattern_len > MAX_WOL_PATTERN_SIZE) {
RT_TRACE(rtlpriv, COMP_POWER, DBG_WARNING,
 "Pattern[%d] is too long\n", i);
continue;


In principle, both patches are correct, but perhaps the debug message should be 
something like

'"Pattern[%d] has bad length of %d\n", i, patterns[i].pattern_len'

Larry

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [RESEND PATCH 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver

2017-09-29 Thread Florian Fainelli
On September 29, 2017 6:59:18 AM PDT, Razvan Stefanescu 
 wrote:
>
>
>> -Original Message-
>> From: Bogdan Purcareata
>> Sent: Friday, September 29, 2017 16:36
>> To: Razvan Stefanescu ;
>> gre...@linuxfoundation.org
>> Cc: de...@driverdev.osuosl.org; linux-ker...@vger.kernel.org;
>> net...@vger.kernel.org; ag...@suse.de; a...@arndb.de; Alexandru
>Marginean
>> ; Ruxandra Ioana Radulescu
>> ; Laurentiu Tudor
>;
>> stuyo...@gmail.com
>> Subject: RE: [RESEND PATCH 2/6] staging: fsl-dpaa2/ethsw: Add
>Freescale DPAA2
>> Ethernet Switch driver
>> 
>> > Introduce the DPAA2 Ethernet Switch driver, which manages Datapath
>Switch
>> > (DPSW) objects discovered on the MC bus.
>> >
>> > Suggested-by: Alexandru Marginean 
>> > Signed-off-by: Razvan Stefanescu 

This looks pretty good for a new switchdev driver, is there a reason you can't 
target drivers/net/ethernet instead of staging? Is it because the MC bus code 
is still in staging (AFAICT)?

-- 
Florian
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] Staging: rtl8723bs: Remove unnecessary comments.

2017-09-29 Thread Shreeya Patel
The comments regarding memset are not needed in the
files which have been modified since the necessary changes
are already there in the files.

Signed-off-by: Shreeya Patel 
---
 drivers/staging/rtl8723bs/core/rtw_mlme.c | 3 ---
 drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 3 ---
 drivers/staging/rtl8723bs/core/rtw_recv.c | 3 ---
 drivers/staging/rtl8723bs/core/rtw_xmit.c | 3 ---
 4 files changed, 12 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c 
b/drivers/staging/rtl8723bs/core/rtw_mlme.c
index 6b77820..5b583f7 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
@@ -28,9 +28,6 @@ sint  _rtw_init_mlme_priv(struct adapter *padapter)
struct mlme_priv*pmlmepriv = &padapter->mlmepriv;
sintres = _SUCCESS;
 
-   /*  We don't need to memset padapter->XXX to zero, because adapter is 
allocated by vzalloc(). */
-   /* memset((u8 *)pmlmepriv, 0, sizeof(struct mlme_priv)); */
-
pmlmepriv->nic_hdl = (u8 *)padapter;
 
pmlmepriv->pscanned = NULL;
diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c 
b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
index b6d137f..ca35c1c 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
@@ -474,9 +474,6 @@ int init_mlme_ext_priv(struct adapter *padapter)
struct mlme_priv *pmlmepriv = &(padapter->mlmepriv);
struct mlme_ext_info *pmlmeinfo = &(pmlmeext->mlmext_info);
 
-   /*  We don't need to memset padapter->XXX to zero, because adapter is 
allocated by vzalloc(). */
-   /* memset((u8 *)pmlmeext, 0, sizeof(struct mlme_ext_priv)); */
-
pmlmeext->padapter = padapter;
 
/* fill_fwpriv(padapter, &(pmlmeext->fwpriv)); */
diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c 
b/drivers/staging/rtl8723bs/core/rtw_recv.c
index 68a6303..9c08307 100644
--- a/drivers/staging/rtl8723bs/core/rtw_recv.c
+++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
@@ -46,9 +46,6 @@ sint _rtw_init_recv_priv(struct recv_priv *precvpriv, struct 
adapter *padapter)
union recv_frame *precvframe;
sintres = _SUCCESS;
 
-   /*  We don't need to memset padapter->XXX to zero, because adapter is 
allocated by vzalloc(). */
-   /* memset((unsigned char *)precvpriv, 0, sizeof (struct  recv_priv)); */
-
spin_lock_init(&precvpriv->lock);
 
_rtw_init_queue(&precvpriv->free_recv_queue);
diff --git a/drivers/staging/rtl8723bs/core/rtw_xmit.c 
b/drivers/staging/rtl8723bs/core/rtw_xmit.c
index 022f654..8cd05f8 100644
--- a/drivers/staging/rtl8723bs/core/rtw_xmit.c
+++ b/drivers/staging/rtl8723bs/core/rtw_xmit.c
@@ -51,9 +51,6 @@ s32   _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct 
adapter *padapter)
struct xmit_frame *pxframe;
sintres = _SUCCESS;
 
-   /*  We don't need to memset padapter->XXX to zero, because adapter is 
allocated by vzalloc(). */
-   /* memset((unsigned char *)pxmitpriv, 0, sizeof(struct xmit_priv)); */
-
spin_lock_init(&pxmitpriv->lock);
spin_lock_init(&pxmitpriv->lock_sctx);
sema_init(&pxmitpriv->xmit_sema, 0);
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [Outreachy kernel] [PATCH] Staging: rtl8723bs: Remove unnecessary comments.

2017-09-29 Thread Julia Lawall


On Sat, 30 Sep 2017, Shreeya Patel wrote:

> The comments regarding memset are not needed in the
> files which have been modified since the necessary changes
> are already there in the files.

The comments don't look very useful, but I don't understand "since the
necessary changes are alread there in the files"

julia


>
> Signed-off-by: Shreeya Patel 
> ---
>  drivers/staging/rtl8723bs/core/rtw_mlme.c | 3 ---
>  drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 3 ---
>  drivers/staging/rtl8723bs/core/rtw_recv.c | 3 ---
>  drivers/staging/rtl8723bs/core/rtw_xmit.c | 3 ---
>  4 files changed, 12 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c 
> b/drivers/staging/rtl8723bs/core/rtw_mlme.c
> index 6b77820..5b583f7 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
> @@ -28,9 +28,6 @@ sint_rtw_init_mlme_priv(struct adapter *padapter)
>   struct mlme_priv*pmlmepriv = &padapter->mlmepriv;
>   sintres = _SUCCESS;
>
> - /*  We don't need to memset padapter->XXX to zero, because adapter is 
> allocated by vzalloc(). */
> - /* memset((u8 *)pmlmepriv, 0, sizeof(struct mlme_priv)); */
> -
>   pmlmepriv->nic_hdl = (u8 *)padapter;
>
>   pmlmepriv->pscanned = NULL;
> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c 
> b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> index b6d137f..ca35c1c 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> @@ -474,9 +474,6 @@ int   init_mlme_ext_priv(struct adapter *padapter)
>   struct mlme_priv *pmlmepriv = &(padapter->mlmepriv);
>   struct mlme_ext_info *pmlmeinfo = &(pmlmeext->mlmext_info);
>
> - /*  We don't need to memset padapter->XXX to zero, because adapter is 
> allocated by vzalloc(). */
> - /* memset((u8 *)pmlmeext, 0, sizeof(struct mlme_ext_priv)); */
> -
>   pmlmeext->padapter = padapter;
>
>   /* fill_fwpriv(padapter, &(pmlmeext->fwpriv)); */
> diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c 
> b/drivers/staging/rtl8723bs/core/rtw_recv.c
> index 68a6303..9c08307 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_recv.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
> @@ -46,9 +46,6 @@ sint _rtw_init_recv_priv(struct recv_priv *precvpriv, 
> struct adapter *padapter)
>   union recv_frame *precvframe;
>   sintres = _SUCCESS;
>
> - /*  We don't need to memset padapter->XXX to zero, because adapter is 
> allocated by vzalloc(). */
> - /* memset((unsigned char *)precvpriv, 0, sizeof (struct  recv_priv)); */
> -
>   spin_lock_init(&precvpriv->lock);
>
>   _rtw_init_queue(&precvpriv->free_recv_queue);
> diff --git a/drivers/staging/rtl8723bs/core/rtw_xmit.c 
> b/drivers/staging/rtl8723bs/core/rtw_xmit.c
> index 022f654..8cd05f8 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_xmit.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_xmit.c
> @@ -51,9 +51,6 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct 
> adapter *padapter)
>   struct xmit_frame *pxframe;
>   sintres = _SUCCESS;
>
> - /*  We don't need to memset padapter->XXX to zero, because adapter is 
> allocated by vzalloc(). */
> - /* memset((unsigned char *)pxmitpriv, 0, sizeof(struct xmit_priv)); */
> -
>   spin_lock_init(&pxmitpriv->lock);
>   spin_lock_init(&pxmitpriv->lock_sctx);
>   sema_init(&pxmitpriv->xmit_sema, 0);
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/1506711232-19670-1-git-send-email-shreeya.patel23498%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [Outreachy kernel] [PATCH] Staging: rtl8723bs: Remove unnecessary comments.

2017-09-29 Thread Shreeya Patel
On Fri, 2017-09-29 at 23:10 +0200, Julia Lawall wrote:
> 
> On Sat, 30 Sep 2017, Shreeya Patel wrote:
> 
> > 
> > The comments regarding memset are not needed in the
> > files which have been modified since the necessary changes
> > are already there in the files.
> The comments don't look very useful, but I don't understand "since
> the
> necessary changes are alread there in the files"
> 
> julia


The comments are put to notify that memset() is not required because
vzalloc is used. But we don't need this comment as patches for this
driver has already been applied to remove memset() code. Also I have
checked the whole file, but there isn't any place where memset() needs
to be removed.

 
> 
> 
> > 
> > 
> > Signed-off-by: Shreeya Patel 
> > ---
> >  drivers/staging/rtl8723bs/core/rtw_mlme.c | 3 ---
> >  drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 3 ---
> >  drivers/staging/rtl8723bs/core/rtw_recv.c | 3 ---
> >  drivers/staging/rtl8723bs/core/rtw_xmit.c | 3 ---
> >  4 files changed, 12 deletions(-)
> > 
> > diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c
> > b/drivers/staging/rtl8723bs/core/rtw_mlme.c
> > index 6b77820..5b583f7 100644
> > --- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
> > +++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
> > @@ -28,9 +28,6 @@ sint  _rtw_init_mlme_priv(struct adapter
> > *padapter)
> >     struct mlme_priv    *pmlmepriv = &padapter->mlmepriv;
> >     sintres = _SUCCESS;
> > 
> > -   /*  We don't need to memset padapter->XXX to zero, because
> > adapter is allocated by vzalloc(). */
> > -   /* memset((u8 *)pmlmepriv, 0, sizeof(struct mlme_priv));
> > */
> > -
> >     pmlmepriv->nic_hdl = (u8 *)padapter;
> > 
> >     pmlmepriv->pscanned = NULL;
> > diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> > b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> > index b6d137f..ca35c1c 100644
> > --- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> > +++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> > @@ -474,9 +474,6 @@ int init_mlme_ext_priv(struct adapter
> > *padapter)
> >     struct mlme_priv *pmlmepriv = &(padapter->mlmepriv);
> >     struct mlme_ext_info *pmlmeinfo = &(pmlmeext-
> > >mlmext_info);
> > 
> > -   /*  We don't need to memset padapter->XXX to zero, because
> > adapter is allocated by vzalloc(). */
> > -   /* memset((u8 *)pmlmeext, 0, sizeof(struct
> > mlme_ext_priv)); */
> > -
> >     pmlmeext->padapter = padapter;
> > 
> >     /* fill_fwpriv(padapter, &(pmlmeext->fwpriv)); */
> > diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c
> > b/drivers/staging/rtl8723bs/core/rtw_recv.c
> > index 68a6303..9c08307 100644
> > --- a/drivers/staging/rtl8723bs/core/rtw_recv.c
> > +++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
> > @@ -46,9 +46,6 @@ sint _rtw_init_recv_priv(struct recv_priv
> > *precvpriv, struct adapter *padapter)
> >     union recv_frame *precvframe;
> >     sintres = _SUCCESS;
> > 
> > -   /*  We don't need to memset padapter->XXX to zero, because
> > adapter is allocated by vzalloc(). */
> > -   /* memset((unsigned char *)precvpriv, 0, sizeof
> > (struct  recv_priv)); */
> > -
> >     spin_lock_init(&precvpriv->lock);
> > 
> >     _rtw_init_queue(&precvpriv->free_recv_queue);
> > diff --git a/drivers/staging/rtl8723bs/core/rtw_xmit.c
> > b/drivers/staging/rtl8723bs/core/rtw_xmit.c
> > index 022f654..8cd05f8 100644
> > --- a/drivers/staging/rtl8723bs/core/rtw_xmit.c
> > +++ b/drivers/staging/rtl8723bs/core/rtw_xmit.c
> > @@ -51,9 +51,6 @@ s32   _rtw_init_xmit_priv(struct xmit_priv
> > *pxmitpriv, struct adapter *padapter)
> >     struct xmit_frame *pxframe;
> >     sintres = _SUCCESS;
> > 
> > -   /*  We don't need to memset padapter->XXX to zero, because
> > adapter is allocated by vzalloc(). */
> > -   /* memset((unsigned char *)pxmitpriv, 0, sizeof(struct
> > xmit_priv)); */
> > -
> >     spin_lock_init(&pxmitpriv->lock);
> >     spin_lock_init(&pxmitpriv->lock_sctx);
> >     sema_init(&pxmitpriv->xmit_sema, 0);
> > --
> > 2.7.4
> > 
> > --
> > You received this message because you are subscribed to the Google
> > Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it,
> > send an email to outreachy-kernel+unsubscr...@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.
> > com.
> > To view this discussion on the web visit https://groups.google.com/
> > d/msgid/outreachy-kernel/1506711232-19670-1-git-send-email-
> > shreeya.patel23498%40gmail.com.
> > For more options, visit https://groups.google.com/d/optout.
> > 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH RFC] media: staging/imx: fix complete handler

2017-09-29 Thread Russell King
The complete handler walks all entities, expecting to find an imx
subdevice for each and every entity.

However, camera drivers such as smiapp can themselves contain multiple
entities, for which there will not be an imx subdevice.  This causes
imx_media_find_subdev_by_sd() to fail, making the imx capture system
unusable with such cameras.

Work around this by killing the error entirely, thereby allowing
the imx capture to be used with such cameras.

Signed-off-by: Russell King 
---
Not the best solution, but the only one I can think of to fix the
regression that happened sometime between a previous revision of
Steve's patch set and the version that got merged.

 drivers/staging/media/imx/imx-media-dev.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-dev.c 
b/drivers/staging/media/imx/imx-media-dev.c
index d96f4512224f..6ba59939dd7a 100644
--- a/drivers/staging/media/imx/imx-media-dev.c
+++ b/drivers/staging/media/imx/imx-media-dev.c
@@ -345,8 +345,11 @@ static int imx_media_add_vdev_to_pad(struct imx_media_dev 
*imxmd,
 
sd = media_entity_to_v4l2_subdev(entity);
imxsd = imx_media_find_subdev_by_sd(imxmd, sd);
-   if (IS_ERR(imxsd))
-   return PTR_ERR(imxsd);
+   if (IS_ERR(imxsd)) {
+   v4l2_err(&imxmd->v4l2_dev, "failed to find subdev for entity 
%s, sd %p err %ld\n",
+entity->name, sd, PTR_ERR(imxsd));
+   return 0;
+   }
 
imxpad = &imxsd->pad[srcpad->index];
vdev_idx = imxpad->num_vdevs;
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] imx-csi: fix burst size

2017-09-29 Thread Russell King
Setting a burst size of "8" doesn't work for IMX219 with 8-bit bayer,
but a burst size of "16" does.  Fix this.

Signed-off-by: Russell King 
---
 drivers/staging/media/imx/imx-media-csi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index 6d856118c223..e27bcdb63973 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -341,7 +341,7 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
case V4L2_PIX_FMT_SGBRG8:
case V4L2_PIX_FMT_SGRBG8:
case V4L2_PIX_FMT_SRGGB8:
-   burst_size = 8;
+   burst_size = 16;
passthrough = true;
passthrough_bits = 8;
break;
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 13/17] media: v4l2-async: simplify v4l2_async_subdev structure

2017-09-29 Thread Sakari Ailus
Hi Mauro,

(Removing the non-list recipients.)

On Fri, Sep 29, 2017 at 06:27:13AM -0300, Mauro Carvalho Chehab wrote:
> Em Thu, 28 Sep 2017 15:09:21 +0300
> Sakari Ailus  escreveu:
> 
> > Hi Mauro,
> > 
> > On Wed, Sep 27, 2017 at 06:46:56PM -0300, Mauro Carvalho Chehab wrote:
> > > The V4L2_ASYNC_MATCH_FWNODE match criteria requires just one
> > > struct to be filled (struct fwnode_handle). The V4L2_ASYNC_MATCH_DEVNAME
> > > match criteria requires just a device name.
> > > 
> > > So, it doesn't make sense to enclose those into structs,
> > > as the criteria can go directly into the union.
> > > 
> > > That makes easier to document it, as we don't need to document
> > > weird senseless structs.  
> > 
> > The idea is that in the union, there's a struct which is specific to the
> > match_type field. I wouldn't call it senseless.
> 
> Why a struct for each specific match_type is **needed**? It it is not
> needed, then it is senseless per definition :-) 
> 
> In the specific case of fwnode, there's already a named struct
> for fwnode_handle. The only thing is that it is declared outside
> enum v4l2_async_match_type. So, I don't see any reason to do things
> like:
> 
>   struct {
>   struct fwnode_handle *fwnode;
>   } fwnode;
> 
> If you're in doubt about that, think on how would you document
> both fwnode structs. Both fwnode structs specify the match
> criteria if %V4L2_ASYNC_MATCH_FWNODE.
> 
> The same applies to this:
> 
>   struct {
>   const char *name;
>   } device_name;
> 
> Both device_name and name specifies the match criteria if
> %V4L2_ASYNC_MATCH_DEVNAME.
> 
> > 
> > In the two cases there's just a single field in the containing struct. You
> > could remove the struct in that case as you do in this patch, and just use
> > the field. But I think the result is less clean and so I wouldn't make this
> > change.
> 
> It is actually cleaner without the stucts.
> 
> Without the useless struct, if one wants to match a firmware node, it
> should be doing:
> 
>   pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_FWNODE;
>   pdata->asd[i]->match.fwnode = of_fwnode_handle(rem);

This code should be and will be moved out of drivers. See:

http://www.spinics.net/lists/linux-media/msg122688.html>

So there are going to be quite a bit fewer instances of it, and none should
remain in drivers. I frankly don't have a strong opinion on this; there are
arguments for and against. I just don't see a reason to change it.

It'd be nice to have Hans's and Laurent's opinion though.

> 
> And that' it. For anyone that reads the above code, even not knowing
> all details of "match", is clear that the match criteria is whatever
> of_fwnode_handle() returns.
> 
> Now, on this:
> 
>   pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_FWNODE;
>   pdata->asd[i]->match.fwnode.fwnode = of_fwnode_handle(rem);
> 
> It sounds that something is missing, as only one field of match.fwnode
> was specified. Anyone not familiar with that non-conventional usage of
> a struct with just one struct field inside would need to seek for the
> header file declaring the struct. That would consume a lot of time for
> code reviewers for no good reason.
> 
> The same apply for devname search:
> 
> In this case:
>   asd->match_type = V4L2_ASYNC_MATCH_DEVNAME;
>   asd->match.device_name.name = imxsd->devname;
> 
> I would be expecting something else to be filled at device_name's
> struct, for example to specify a case sensitive or case insensitive
> match criteria, to allow seeking for a device's substring, or to
> allow using other struct device fields to narrow the seek.
> 
> With this:
> 
>   asd->match_type = V4L2_ASYNC_MATCH_DEVNAME;
>   asd->match.device_name = imxsd->devname;
> 
> It is clear that the match criteria is fully specified.
> 
> > The confusion comes possibly from the fact that the struct is named the
> > same as the field in the struct. These used to be called of and node, but
> > with the fwnode property framework the references to the fwnode are, well,
> > typically similarly called "fwnode". There's no underlying firmware
> > interface with that name, fwnode property API is just an API.
> 
> The duplicated "fwnode" name only made it more evident that we don't
> need to enclose a single match criteria field inside a struct.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] binder: fix use-after-free in binder_transaction()

2017-09-29 Thread Todd Kjos
User-space normally keeps the node alive when creating a transaction
since it has a reference to the target. The local strong ref keeps it
alive if the sending process dies before the target process processes
the transaction. If the source process is malicious or has a reference
counting bug, this can fail.

In this case, when we attempt to decrement the node in the failure
path, the node has already been freed.

This is fixed by taking a tmpref on the node while constructing
the transaction. To avoid re-acquiring the node lock and inner
proc lock to increment the proc's tmpref, a helper is used that
does the ref increments on both the node and proc.

Signed-off-by: Todd Kjos 
---
 drivers/android/binder.c | 93 ++--
 1 file changed, 66 insertions(+), 27 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index d055b3f2a207..40e8a58391cc 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2583,6 +2583,48 @@ static bool binder_proc_transaction(struct 
binder_transaction *t,
return true;
 }
 
+/**
+ * binder_get_node_refs_for_txn() - Get required refs on node for txn
+ * @node: struct binder_node for which to get refs
+ * @proc: returns @node->proc if valid
+ * @error:if no @proc then returns BR_DEAD_REPLY
+ *
+ * User-space normally keeps the node alive when creating a transaction
+ * since it has a reference to the target. The local strong ref keeps it
+ * alive if the sending process dies before the target process processes
+ * the transaction. If the source process is malicious or has a reference
+ * counting bug, relying on the local strong ref can fail.
+ *
+ * Since user-space can cause the local strong ref to go away, we also take
+ * a tmpref on the node to ensure it survives while we are constructing
+ * the transaction. We also need a tmpref on the proc while we are
+ * constructing the transaction, so we take that here as well.
+ *
+ * Return: The target_node with refs taken or NULL if no @node->proc is NULL.
+ * Also sets @proc if valid. If the @node->proc is NULL indicating that the
+ * target proc has died, @error is set to BR_DEAD_REPLY
+ */
+static struct binder_node *binder_get_node_refs_for_txn(
+   struct binder_node *node,
+   struct binder_proc **procp,
+   uint32_t *error)
+{
+   struct binder_node *target_node = NULL;
+
+   binder_node_inner_lock(node);
+   if (node->proc) {
+   target_node = node;
+   binder_inc_node_nilocked(node, 1, 0, NULL);
+   binder_inc_node_tmpref_ilocked(node);
+   node->proc->tmp_ref++;
+   *procp = node->proc;
+   } else
+   *error = BR_DEAD_REPLY;
+   binder_node_inner_unlock(node);
+
+   return target_node;
+}
+
 static void binder_transaction(struct binder_proc *proc,
   struct binder_thread *thread,
   struct binder_transaction_data *tr, int reply,
@@ -2686,43 +2728,35 @@ static void binder_transaction(struct binder_proc *proc,
ref = binder_get_ref_olocked(proc, tr->target.handle,
 true);
if (ref) {
-   binder_inc_node(ref->node, 1, 0, NULL);
-   target_node = ref->node;
-   }
-   binder_proc_unlock(proc);
-   if (target_node == NULL) {
+   target_node = binder_get_node_refs_for_txn(
+   ref->node, &target_proc,
+   &return_error);
+   } else {
binder_user_error("%d:%d got transaction to 
invalid handle\n",
-   proc->pid, thread->pid);
+ proc->pid, thread->pid);
return_error = BR_FAILED_REPLY;
-   return_error_param = -EINVAL;
-   return_error_line = __LINE__;
-   goto err_invalid_target_handle;
}
+   binder_proc_unlock(proc);
} else {
mutex_lock(&context->context_mgr_node_lock);
target_node = context->binder_context_mgr_node;
-   if (target_node == NULL) {
+   if (target_node)
+   target_node = binder_get_node_refs_for_txn(
+   target_node, &target_proc,
+   &return_error);
+   else
return_error = BR_DEAD_REPLY;
-   mutex_unlock(&context

[PATCH] Staging: rtl8188eu: core: Use list_entry instead of container_of

2017-09-29 Thread Srishti Sharma
For variables that have type struct list_head* use list_entry to
access current list element instead of using container_of.
Done using the following semantic patch by coccinelle.

@r@
identifier e;
struct list_head* l;
@@

<... when != l == NULL
l;
...>

(
e=
-container_of
+list_entry
(
 ...)
)

Signed-off-by: Srishti Sharma 
---
 drivers/staging/rtl8188eu/core/rtw_recv.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c 
b/drivers/staging/rtl8188eu/core/rtw_recv.c
index 3fd5f41..af59c16 100644
--- a/drivers/staging/rtl8188eu/core/rtw_recv.c
+++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
@@ -193,7 +193,7 @@ void rtw_free_recvframe_queue(struct __queue *pframequeue,  
struct __queue *pfre
plist = phead->next;
 
while (phead != plist) {
-   hdr = container_of(plist, struct recv_frame, list);
+   hdr = list_entry(plist, struct recv_frame, list);
 
plist = plist->next;
 
@@ -943,7 +943,7 @@ static int validate_recv_ctrl_frame(struct adapter 
*padapter,
xmitframe_plist = xmitframe_phead->next;
 
if (xmitframe_phead != xmitframe_plist) {
-   pxmitframe = container_of(xmitframe_plist, 
struct xmit_frame, list);
+   pxmitframe = list_entry(xmitframe_plist, struct 
xmit_frame, list);
 
xmitframe_plist = xmitframe_plist->next;
 
@@ -1347,7 +1347,7 @@ static struct recv_frame *recvframe_defrag(struct adapter 
*adapter,
 
phead = get_list_head(defrag_q);
plist = phead->next;
-   pfhdr = container_of(plist, struct recv_frame, list);
+   pfhdr = list_entry(plist, struct recv_frame, list);
prframe = pfhdr;
list_del_init(&(prframe->list));
 
@@ -1367,7 +1367,7 @@ static struct recv_frame *recvframe_defrag(struct adapter 
*adapter,
plist = plist->next;
 
while (phead != plist) {
-   pnfhdr = container_of(plist, struct recv_frame, list);
+   pnfhdr = list_entry(plist, struct recv_frame, list);
pnextrframe = pnfhdr;
 
/* check the fragment sequence  (2nd ~n fragment frame) */
@@ -1655,7 +1655,7 @@ static int enqueue_reorder_recvframe(struct 
recv_reorder_ctrl *preorder_ctrl,
plist = phead->next;
 
while (phead != plist) {
-   hdr = container_of(plist, struct recv_frame, list);
+   hdr = list_entry(plist, struct recv_frame, list);
pnextattrib = &hdr->attrib;
 
if (SN_LESS(pnextattrib->seq_num, pattrib->seq_num))
@@ -1690,7 +1690,7 @@ static int recv_indicatepkts_in_order(struct adapter 
*padapter, struct recv_reor
if (list_empty(phead))
return true;
 
-   prhdr = container_of(plist, struct recv_frame, list);
+   prhdr = list_entry(plist, struct recv_frame, list);
pattrib = &prhdr->attrib;
preorder_ctrl->indicate_seq = pattrib->seq_num;
}
@@ -1698,7 +1698,7 @@ static int recv_indicatepkts_in_order(struct adapter 
*padapter, struct recv_reor
/*  Prepare indication list and indication. */
/*  Check if there is any packet need indicate. */
while (!list_empty(phead)) {
-   prhdr = container_of(plist, struct recv_frame, list);
+   prhdr = list_entry(plist, struct recv_frame, list);
prframe = prhdr;
pattrib = &prframe->attrib;
 
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 1/3] staging: iio: tsl2x7x: rename tsl2x7x_settings variable to settings

2017-09-29 Thread Brian Masney
The length of the 'tsl2x7x_settings' variable within the tsl2X7X_chip
structure makes some of the line lengths greater than 80 characters for
upcoming patches. This patch shortens the name of the 'tsl2x7x_settings'
variable in this structure to just 'settings' to improve code
readability.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/tsl2x7x.c | 183 +---
 1 file changed, 85 insertions(+), 98 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c 
b/drivers/staging/iio/light/tsl2x7x.c
index d9defc8ece83..e340ea624e5c 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -174,7 +174,7 @@ struct tsl2X7X_chip {
struct i2c_client *client;
u16 prox_data;
struct tsl2x7x_als_info als_cur_info;
-   struct tsl2x7x_settings tsl2x7x_settings;
+   struct tsl2x7x_settings settings;
struct tsl2X7X_platform_data *pdata;
int als_time_scale;
int als_saturation;
@@ -390,9 +390,9 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
lux = 0;
} else {
ch0lux = DIV_ROUND_UP(ch0 * p->ch0,
-   tsl2X7X_als_gainadj[chip->tsl2x7x_settings.als_gain]);
+   tsl2X7X_als_gainadj[chip->settings.als_gain]);
ch1lux = DIV_ROUND_UP(ch1 * p->ch1,
-   tsl2X7X_als_gainadj[chip->tsl2x7x_settings.als_gain]);
+   tsl2X7X_als_gainadj[chip->settings.als_gain]);
lux = ch0lux - ch1lux;
}
 
@@ -419,7 +419,7 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
 */
 
lux64 = lux;
-   lux64 = lux64 * chip->tsl2x7x_settings.als_gain_trim;
+   lux64 = lux64 * chip->settings.als_gain_trim;
lux64 >>= 8;
lux = lux64;
lux = (lux + 500) / 1000;
@@ -514,12 +514,10 @@ static void tsl2x7x_defaults(struct tsl2X7X_chip *chip)
 {
/* If Operational settings defined elsewhere.. */
if (chip->pdata && chip->pdata->platform_default_settings)
-   memcpy(&chip->tsl2x7x_settings,
-  chip->pdata->platform_default_settings,
+   memcpy(&chip->settings, chip->pdata->platform_default_settings,
   sizeof(tsl2x7x_default_settings));
else
-   memcpy(&chip->tsl2x7x_settings,
-  &tsl2x7x_default_settings,
+   memcpy(&chip->settings, &tsl2x7x_default_settings,
   sizeof(tsl2x7x_default_settings));
 
/* Load up the proper lux table. */
@@ -542,9 +540,7 @@ static void tsl2x7x_defaults(struct tsl2X7X_chip *chip)
 static int tsl2x7x_als_calibrate(struct iio_dev *indio_dev)
 {
struct tsl2X7X_chip *chip = iio_priv(indio_dev);
-   int gain_trim_val;
-   int ret;
-   int lux_val;
+   int ret, lux_val;
 
ret = i2c_smbus_read_byte_data(chip->client,
   TSL2X7X_CMD_REG | TSL2X7X_CNTRL);
@@ -575,16 +571,16 @@ static int tsl2x7x_als_calibrate(struct iio_dev 
*indio_dev)
return lux_val;
}
 
-   gain_trim_val =  ((chip->tsl2x7x_settings.als_cal_target)
-   * chip->tsl2x7x_settings.als_gain_trim) / lux_val;
-   if ((gain_trim_val < 250) || (gain_trim_val > 4000))
+   ret = (chip->settings.als_cal_target * chip->settings.als_gain_trim) /
+   lux_val;
+   if (ret < 250 || ret > 4000)
return -ERANGE;
 
-   chip->tsl2x7x_settings.als_gain_trim = gain_trim_val;
+   chip->settings.als_gain_trim = ret;
dev_info(&chip->client->dev,
 "%s als_calibrate completed\n", chip->client->name);
 
-   return (int)gain_trim_val;
+   return ret;
 }
 
 static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
@@ -602,34 +598,30 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
chip->pdata->power_on(indio_dev);
 
/* Non calculated parameters */
-   chip->tsl2x7x_config[TSL2X7X_PRX_TIME] =
-   chip->tsl2x7x_settings.prx_time;
-   chip->tsl2x7x_config[TSL2X7X_WAIT_TIME] =
-   chip->tsl2x7x_settings.wait_time;
-   chip->tsl2x7x_config[TSL2X7X_PRX_CONFIG] =
-   chip->tsl2x7x_settings.prox_config;
+   chip->tsl2x7x_config[TSL2X7X_PRX_TIME] = chip->settings.prx_time;
+   chip->tsl2x7x_config[TSL2X7X_WAIT_TIME] = chip->settings.wait_time;
+   chip->tsl2x7x_config[TSL2X7X_PRX_CONFIG] = chip->settings.prox_config;
 
chip->tsl2x7x_config[TSL2X7X_ALS_MINTHRESHLO] =
-   (chip->tsl2x7x_settings.als_thresh_low) & 0xFF;
+   (chip->settings.als_thresh_low) & 0xFF;
chip->tsl2x7x_config[TSL2X7X_ALS_MINTHRESHHI] =
-   (chip->tsl2x7x_settings.als_thresh_low >> 8) & 0xFF;
+   (chip->settings.als_thresh_low >> 8) & 0xFF;
chip->tsl2x7x_config[TSL2X7X_AL

[PATCH 3/3] staging: iio: tsl2x7x: migrate *_thresh_period sysfs attributes to iio_event_spec

2017-09-29 Thread Brian Masney
The sysfs attributes in_intensity0_thresh_period and
in_proximity0_thresh_period are currently directly created by the driver.
This patch migrates the creation of these sysfs attributes from the driver
to using the IIO core via iio_event_spec.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/tsl2x7x.c | 196 ++--
 1 file changed, 52 insertions(+), 144 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c 
b/drivers/staging/iio/light/tsl2x7x.c
index e6a71f5fc9cb..be2806593e59 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -932,108 +932,6 @@ static ssize_t in_illuminance0_target_input_store(struct 
device *dev,
return len;
 }
 
-/* persistence settings */
-static ssize_t in_intensity0_thresh_period_show(struct device *dev,
-   struct device_attribute *attr,
-   char *buf)
-{
-   struct tsl2X7X_chip *chip = iio_priv(dev_to_iio_dev(dev));
-   int y, z, filter_delay;
-
-   /* Determine integration time */
-   y = (TSL2X7X_MAX_TIMER_CNT - (u8)chip->settings.als_time) + 1;
-   z = y * TSL2X7X_MIN_ITIME;
-   filter_delay = z * (chip->settings.persistence & 0x0F);
-   y = filter_delay / 1000;
-   z = filter_delay % 1000;
-
-   return snprintf(buf, PAGE_SIZE, "%d.%03d\n", y, z);
-}
-
-static ssize_t in_intensity0_thresh_period_store(struct device *dev,
-struct device_attribute *attr,
-const char *buf, size_t len)
-{
-   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-   struct tsl2X7X_chip *chip = iio_priv(indio_dev);
-   struct tsl2x7x_parse_result result;
-   int y, z, filter_delay;
-   int ret;
-
-   ret = iio_str_to_fixpoint(buf, 100, &result.integer, &result.fract);
-   if (ret)
-   return ret;
-
-   y = (TSL2X7X_MAX_TIMER_CNT - (u8)chip->settings.als_time) + 1;
-   z = y * TSL2X7X_MIN_ITIME;
-
-   filter_delay =
-   DIV_ROUND_UP((result.integer * 1000) + result.fract, z);
-
-   chip->settings.persistence &= 0xF0;
-   chip->settings.persistence |= (filter_delay & 0x0F);
-
-   dev_info(&chip->client->dev, "%s: als persistence = %d",
-__func__, filter_delay);
-
-   ret = tsl2x7x_invoke_change(indio_dev);
-   if (ret < 0)
-   return ret;
-
-   return IIO_VAL_INT_PLUS_MICRO;
-}
-
-static ssize_t in_proximity0_thresh_period_show(struct device *dev,
-struct device_attribute *attr,
-char *buf)
-{
-   struct tsl2X7X_chip *chip = iio_priv(dev_to_iio_dev(dev));
-   int y, z, filter_delay;
-
-   /* Determine integration time */
-   y = (TSL2X7X_MAX_TIMER_CNT - (u8)chip->settings.prx_time) + 1;
-   z = y * TSL2X7X_MIN_ITIME;
-   filter_delay = z * ((chip->settings.persistence & 0xF0) >> 4);
-   y = filter_delay / 1000;
-   z = filter_delay % 1000;
-
-   return snprintf(buf, PAGE_SIZE, "%d.%03d\n", y, z);
-}
-
-static ssize_t in_proximity0_thresh_period_store(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t len)
-{
-   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-   struct tsl2X7X_chip *chip = iio_priv(indio_dev);
-   struct tsl2x7x_parse_result result;
-   int y, z, filter_delay;
-   int ret;
-
-   ret = iio_str_to_fixpoint(buf, 100, &result.integer, &result.fract);
-   if (ret)
-   return ret;
-
-   y = (TSL2X7X_MAX_TIMER_CNT - (u8)chip->settings.prx_time) + 1;
-   z = y * TSL2X7X_MIN_ITIME;
-
-   filter_delay =
-   DIV_ROUND_UP((result.integer * 1000) + result.fract, z);
-
-   chip->settings.persistence &= 0x0F;
-   chip->settings.persistence |= ((filter_delay << 4) & 0xF0);
-
-   dev_info(&chip->client->dev, "%s: prox persistence = %d",
-__func__, filter_delay);
-
-   ret = tsl2x7x_invoke_change(indio_dev);
-   if (ret < 0)
-   return ret;
-
-
-   return IIO_VAL_INT_PLUS_MICRO;
-}
-
 static ssize_t in_illuminance0_calibrate_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t len)
@@ -1198,7 +1096,8 @@ static int tsl2x7x_write_event_value(struct iio_dev 
*indio_dev,
 int val, int val2)
 {
struct tsl2X7X_chip *chip = iio_priv(indio_dev);
-   int ret = -EINVAL;
+   int ret = -EINVAL, y, z, filter_delay;
+   u8 time;
 
switch (info) {
case IIO_EV_INFO_VALUE:
@@ -1230,6 +1129,33 @@ static int tsl2x7x_write_event_value(struct iio_dev 
*indio_dev,
}
   

[PATCH 0/3] staging: iio: tsl2x7x: staging cleanups

2017-09-29 Thread Brian Masney
This patch set converts several sysfs attributes from directly being
created by the driver to be created by iio_chan_spec and iio_event_spec.
There is also a patch to shorten the length of a variable to fix an
issue that I encountered with some lines that were over 80 characters
with this refactoring.

Driver tested using a TSL2771 hooked up to a Raspberry Pi 2.

List of sysfs attributes before and after my changes:

raspberrypi:/sys/bus/iio/devices/iio:device0$ find . -type f | sort
./dev
./events/in_intensity0_thresh_falling_en
./events/in_intensity0_thresh_falling_value
./events/in_intensity0_thresh_period
./events/in_intensity0_thresh_rising_en
./events/in_intensity0_thresh_rising_value
./events/in_proximity0_thresh_falling_en
./events/in_proximity0_thresh_falling_value
./events/in_proximity0_thresh_period
./events/in_proximity0_thresh_rising_en
./events/in_proximity0_thresh_rising_value
./in_illuminance0_calibrate
./in_illuminance0_calibscale_available
./in_illuminance0_input
./in_illuminance0_integration_time
./in_illuminance0_integration_time_available
./in_illuminance0_lux_table
./in_illuminance0_target_input
./in_intensity0_calibbias
./in_intensity0_calibscale
./in_intensity0_raw
./in_intensity1_raw
./in_proximity0_calibrate
./in_proximity0_raw
./name
./power/autosuspend_delay_ms
./power/control
./power/runtime_active_time
./power/runtime_status
./power/runtime_suspended_time
./uevent

Brian Masney (3):
  staging: iio: tsl2x7x: rename tsl2x7x_settings variable to settings
  staging: iio: tsl2x7x: migrate in_illuminance0_integration_time sysfs
attribute to iio_chan_spec
  staging: iio: tsl2x7x: migrate *_thresh_period sysfs attributes to
iio_event_spec

 drivers/staging/iio/light/tsl2x7x.c | 418 
 1 file changed, 141 insertions(+), 277 deletions(-)

-- 
2.13.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 2/3] staging: iio: tsl2x7x: migrate in_illuminance0_integration_time sysfs attribute to iio_chan_spec

2017-09-29 Thread Brian Masney
The driver explicitly creates the in_illuminance0_integration_time sysfs
attribute outside the IIO core. This attribute is available in the IIO
core so this patches migrates the attribute to be created by
the iio_chan_spec.

Signed-off-by: Brian Masney 
---
Changes since v1 (Jul 9 2017):
- Use MIN_ITIME instead of hardcoded 3 in tsl2x7x_write_raw()

 drivers/staging/iio/light/tsl2x7x.c | 65 ++---
 1 file changed, 17 insertions(+), 48 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c 
b/drivers/staging/iio/light/tsl2x7x.c
index e340ea624e5c..e6a71f5fc9cb 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -898,45 +898,6 @@ static ssize_t 
in_proximity0_calibscale_available_show(struct device *dev,
return snprintf(buf, PAGE_SIZE, "%s\n", "1 2 4 8");
 }
 
-static ssize_t in_illuminance0_integration_time_show(struct device *dev,
-struct device_attribute *attr,
-char *buf)
-{
-   struct tsl2X7X_chip *chip = iio_priv(dev_to_iio_dev(dev));
-   int y, z;
-
-   y = (TSL2X7X_MAX_TIMER_CNT - (u8)chip->settings.als_time) + 1;
-   z = y * TSL2X7X_MIN_ITIME;
-   y /= 1000;
-   z %= 1000;
-
-   return snprintf(buf, PAGE_SIZE, "%d.%03d\n", y, z);
-}
-
-static ssize_t in_illuminance0_integration_time_store(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t len)
-{
-   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-   struct tsl2X7X_chip *chip = iio_priv(indio_dev);
-   struct tsl2x7x_parse_result result;
-   int ret;
-
-   ret = iio_str_to_fixpoint(buf, 100, &result.integer, &result.fract);
-   if (ret)
-   return ret;
-
-   result.fract /= 3;
-   chip->settings.als_time = TSL2X7X_MAX_TIMER_CNT - (u8)result.fract;
-
-   dev_info(&chip->client->dev, "%s: als time = %d",
-__func__, chip->settings.als_time);
-
-   tsl2x7x_invoke_change(indio_dev);
-
-   return IIO_VAL_INT_PLUS_MICRO;
-}
-
 static IIO_CONST_ATTR(in_illuminance0_integration_time_available,
".00272 - .696");
 
@@ -1377,7 +1338,11 @@ static int tsl2x7x_read_raw(struct iio_dev *indio_dev,
*val = chip->settings.als_gain_trim;
ret = IIO_VAL_INT;
break;
-
+   case IIO_CHAN_INFO_INT_TIME:
+   *val = (TSL2X7X_MAX_TIMER_CNT - chip->settings.als_time) + 1;
+   *val2 = ((*val * TSL2X7X_MIN_ITIME) % 1000) / 1000;
+   ret = IIO_VAL_INT_PLUS_MICRO;
+   break;
default:
ret = -EINVAL;
}
@@ -1453,7 +1418,13 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
case IIO_CHAN_INFO_CALIBBIAS:
chip->settings.als_gain_trim = val;
break;
+   case IIO_CHAN_INFO_INT_TIME:
+   chip->settings.als_time =
+   TSL2X7X_MAX_TIMER_CNT - (val2 / TSL2X7X_MIN_ITIME);
 
+   dev_info(&chip->client->dev, "%s: als time = %d",
+__func__, chip->settings.als_time);
+   break;
default:
return -EINVAL;
}
@@ -1465,8 +1436,6 @@ static DEVICE_ATTR_RO(in_proximity0_calibscale_available);
 
 static DEVICE_ATTR_RO(in_illuminance0_calibscale_available);
 
-static DEVICE_ATTR_RW(in_illuminance0_integration_time);
-
 static DEVICE_ATTR_RW(in_illuminance0_target_input);
 
 static DEVICE_ATTR_WO(in_illuminance0_calibrate);
@@ -1546,7 +1515,6 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void 
*private)
 
 static struct attribute *tsl2x7x_ALS_device_attrs[] = {
&dev_attr_in_illuminance0_calibscale_available.attr,
-   &dev_attr_in_illuminance0_integration_time.attr,

&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
&dev_attr_in_illuminance0_target_input.attr,
&dev_attr_in_illuminance0_calibrate.attr,
@@ -1561,7 +1529,6 @@ static struct attribute *tsl2x7x_PRX_device_attrs[] = {
 
 static struct attribute *tsl2x7x_ALSPRX_device_attrs[] = {
&dev_attr_in_illuminance0_calibscale_available.attr,
-   &dev_attr_in_illuminance0_integration_time.attr,

&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
&dev_attr_in_illuminance0_target_input.attr,
&dev_attr_in_illuminance0_calibrate.attr,
@@ -1578,7 +1545,6 @@ static struct attribute *tsl2x7x_PRX2_device_attrs[] = {
 
 static struct attribute *tsl2x7x_ALSPRX2_device_attrs[] = {
&dev_attr_in_illuminance0_calibscale_available.attr,
-   &dev_attr_in_illuminance0_integration_time.attr,

&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
&dev_attr_in_illuminance0_target_input.attr,
&dev_attr_in_illuminance0_calibrate.attr,
@@ -

[PATCH] Staging: rtl8188eu: core: Use list_entry instead of container_of

2017-09-29 Thread Srishti Sharma
For variables of the type struct list_head* use list_entry to access
the current list element instead of using container_of.
Done using the following semantic patch by coccinelle.

@r@
identifier e;
struct list_head* l;
@@

<... when != l == NULL
l;
...>

(
e =
-container_of
+list_entry
 (
  ...)
)

Signed-off-by: Srishti Sharma 
---
 drivers/staging/rtl8188eu/core/rtw_sta_mgt.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c 
b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
index 22cf362..f9df4ac 100644
--- a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
+++ b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
@@ -152,8 +152,8 @@ u32 _rtw_free_sta_priv(struct   sta_priv *pstapriv)
while (phead != plist) {
int i;
 
-   psta = container_of(plist, struct sta_info,
-   hash_list);
+   psta = list_entry(plist, struct sta_info,
+ hash_list);
plist = plist->next;
 
for (i = 0; i < 16; i++) {
@@ -323,7 +323,7 @@ u32 rtw_free_stainfo(struct adapter *padapter, struct 
sta_info *psta)
plist = phead->next;
 
while (!list_empty(phead)) {
-   prframe = container_of(plist, struct recv_frame, list);
+   prframe = list_entry(plist, struct recv_frame, list);
 
plist = plist->next;
 
@@ -399,7 +399,7 @@ void rtw_free_all_stainfo(struct adapter *padapter)
plist = phead->next;
 
while (phead != plist) {
-   psta = container_of(plist, struct sta_info, hash_list);
+   psta = list_entry(plist, struct sta_info, hash_list);
 
plist = plist->next;
 
@@ -435,7 +435,7 @@ struct sta_info *rtw_get_stainfo(struct sta_priv *pstapriv, 
u8 *hwaddr)
plist = phead->next;
 
while (phead != plist) {
-   psta = container_of(plist, struct sta_info, hash_list);
+   psta = list_entry(plist, struct sta_info, hash_list);
 
if ((!memcmp(psta->hwaddr, addr, ETH_ALEN)) == true) {
/*  if found the matched address */
@@ -493,7 +493,7 @@ u8 rtw_access_ctrl(struct adapter *padapter, u8 *mac_addr)
phead = get_list_head(pacl_node_q);
plist = phead->next;
while (phead != plist) {
-   paclnode = container_of(plist, struct rtw_wlan_acl_node, list);
+   paclnode = list_entry(plist, struct rtw_wlan_acl_node, list);
plist = plist->next;
 
if (!memcmp(paclnode->addr, mac_addr, ETH_ALEN)) {
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 1/1] Drivers: hv: vmbus: Fix bugs in rescind handling

2017-09-29 Thread kys
From: "K. Y. Srinivasan" 

This patch addresses the following bugs in the current rescind handling code:

1. Fixes a race condition where we may be invoking hv_process_channel_removal()
on an already freed channel.

2. Prevents indefinite wait when rescinding sub-channels by correctly setting
the probe_complete state.

I would like to thank Dexuan for patiently reviewing earlier versions of this
patch and identifying many of the issues fixed here.

Greg, please apply this to 4.14-final.

Fixes: '54a66265d675 ("Drivers: hv: vmbus: Fix rescind handling")'

Signed-off-by: K. Y. Srinivasan 
Reviewed-by: Dexuan Cui 
Cc: sta...@vger.kernel.org (4.13 and above)
---
 drivers/hv/channel.c  |  6 +++---
 drivers/hv/channel_mgmt.c | 37 ++---
 drivers/hv/vmbus_drv.c|  3 +--
 include/linux/hyperv.h|  2 +-
 4 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 63ac1c6a825f..be3fccab07fe 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -640,6 +640,7 @@ void vmbus_close(struct vmbus_channel *channel)
 */
return;
}
+   mutex_lock(&vmbus_connection.channel_mutex);
/*
 * Close all the sub-channels first and then close the
 * primary channel.
@@ -648,16 +649,15 @@ void vmbus_close(struct vmbus_channel *channel)
cur_channel = list_entry(cur, struct vmbus_channel, sc_list);
vmbus_close_internal(cur_channel);
if (cur_channel->rescind) {
-   mutex_lock(&vmbus_connection.channel_mutex);
-   hv_process_channel_removal(cur_channel,
+   hv_process_channel_removal(
   cur_channel->offermsg.child_relid);
-   mutex_unlock(&vmbus_connection.channel_mutex);
}
}
/*
 * Now close the primary.
 */
vmbus_close_internal(channel);
+   mutex_unlock(&vmbus_connection.channel_mutex);
 }
 EXPORT_SYMBOL_GPL(vmbus_close);
 
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 624d815745e4..18c94ed02562 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -159,7 +159,7 @@ static void vmbus_rescind_cleanup(struct vmbus_channel 
*channel)
 
 
spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags);
-
+   channel->rescind = true;
list_for_each_entry(msginfo, &vmbus_connection.chn_msg_list,
msglistentry) {
 
@@ -381,14 +381,21 @@ static void vmbus_release_relid(u32 relid)
   true);
 }
 
-void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid)
+void hv_process_channel_removal(u32 relid)
 {
unsigned long flags;
-   struct vmbus_channel *primary_channel;
+   struct vmbus_channel *primary_channel, *channel;
 
-   BUG_ON(!channel->rescind);
BUG_ON(!mutex_is_locked(&vmbus_connection.channel_mutex));
 
+   /*
+* Make sure channel is valid as we may have raced.
+*/
+   channel = relid2channel(relid);
+   if (!channel)
+   return;
+
+   BUG_ON(!channel->rescind);
if (channel->target_cpu != get_cpu()) {
put_cpu();
smp_call_function_single(channel->target_cpu,
@@ -515,6 +522,7 @@ static void vmbus_process_offer(struct vmbus_channel 
*newchannel)
if (!fnew) {
if (channel->sc_creation_callback != NULL)
channel->sc_creation_callback(newchannel);
+   newchannel->probe_done = true;
return;
}
 
@@ -843,7 +851,6 @@ static void vmbus_onoffer_rescind(struct 
vmbus_channel_message_header *hdr)
 {
struct vmbus_channel_rescind_offer *rescind;
struct vmbus_channel *channel;
-   unsigned long flags;
struct device *dev;
 
rescind = (struct vmbus_channel_rescind_offer *)hdr;
@@ -882,16 +889,6 @@ static void vmbus_onoffer_rescind(struct 
vmbus_channel_message_header *hdr)
return;
}
 
-   spin_lock_irqsave(&channel->lock, flags);
-   channel->rescind = true;
-   spin_unlock_irqrestore(&channel->lock, flags);
-
-   /*
-* Now that we have posted the rescind state, perform
-* rescind related cleanup.
-*/
-   vmbus_rescind_cleanup(channel);
-
/*
 * Now wait for offer handling to complete.
 */
@@ -910,6 +907,7 @@ static void vmbus_onoffer_rescind(struct 
vmbus_channel_message_header *hdr)
if (channel->device_obj) {
if (channel->chn_rescind_callback) {
channel->chn_rescind_callback(channel);
+   vmbus_rescind_cleanup(channel);
return;
}
/*
@@ -918,6 +916,7 @@ static void vmbus_onoffer_rescind(struct 
vmbus_chann

Re: [Outreachy kernel] [PATCH] Staging: rtl8188eu: core: Use list_entry instead of container_of

2017-09-29 Thread Julia Lawall


On Sat, 30 Sep 2017, Srishti Sharma wrote:

> For variables of the type struct list_head* use list_entry to access
> the current list element instead of using container_of.
> Done using the following semantic patch by coccinelle.
>
> @r@
> identifier e;
> struct list_head* l;
> @@
>
> <... when != l == NULL
> l;
> ...>

I don't see what is the goal of the above code.  The list_head variable is
not going to be in a statement by itself.  There is also no need to check
for l being NULL.  If it is NULL, the original code is incorrect too.

> (
> e =
> -container_of
> +list_entry
>  (
>   ...)
> )

Here you don't need the outer ( ).  This makes a disjunction with only
one choice.  Since there is only one choice, you don't need the
disjunction.

julia

> Signed-off-by: Srishti Sharma 
> ---
>  drivers/staging/rtl8188eu/core/rtw_sta_mgt.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c 
> b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> index 22cf362..f9df4ac 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> @@ -152,8 +152,8 @@ u32   _rtw_free_sta_priv(struct   sta_priv 
> *pstapriv)
>   while (phead != plist) {
>   int i;
>
> - psta = container_of(plist, struct sta_info,
> - hash_list);
> + psta = list_entry(plist, struct sta_info,
> +   hash_list);
>   plist = plist->next;
>
>   for (i = 0; i < 16; i++) {
> @@ -323,7 +323,7 @@ u32   rtw_free_stainfo(struct adapter *padapter, 
> struct sta_info *psta)
>   plist = phead->next;
>
>   while (!list_empty(phead)) {
> - prframe = container_of(plist, struct recv_frame, list);
> + prframe = list_entry(plist, struct recv_frame, list);
>
>   plist = plist->next;
>
> @@ -399,7 +399,7 @@ void rtw_free_all_stainfo(struct adapter *padapter)
>   plist = phead->next;
>
>   while (phead != plist) {
> - psta = container_of(plist, struct sta_info, hash_list);
> + psta = list_entry(plist, struct sta_info, hash_list);
>
>   plist = plist->next;
>
> @@ -435,7 +435,7 @@ struct sta_info *rtw_get_stainfo(struct sta_priv 
> *pstapriv, u8 *hwaddr)
>   plist = phead->next;
>
>   while (phead != plist) {
> - psta = container_of(plist, struct sta_info, hash_list);
> + psta = list_entry(plist, struct sta_info, hash_list);
>
>   if ((!memcmp(psta->hwaddr, addr, ETH_ALEN)) == true) {
>   /*  if found the matched address */
> @@ -493,7 +493,7 @@ u8 rtw_access_ctrl(struct adapter *padapter, u8 *mac_addr)
>   phead = get_list_head(pacl_node_q);
>   plist = phead->next;
>   while (phead != plist) {
> - paclnode = container_of(plist, struct rtw_wlan_acl_node, list);
> + paclnode = list_entry(plist, struct rtw_wlan_acl_node, list);
>   plist = plist->next;
>
>   if (!memcmp(paclnode->addr, mac_addr, ETH_ALEN)) {
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/1506734581-10932-1-git-send-email-srishtishar%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [Outreachy kernel] [PATCH] Staging: rtl8723bs: Remove unnecessary comments.

2017-09-29 Thread Julia Lawall


On Sat, 30 Sep 2017, Shreeya Patel wrote:

> On Fri, 2017-09-29 at 23:10 +0200, Julia Lawall wrote:
> >
> > On Sat, 30 Sep 2017, Shreeya Patel wrote:
> >
> > >
> > > The comments regarding memset are not needed in the
> > > files which have been modified since the necessary changes
> > > are already there in the files.
> > The comments don't look very useful, but I don't understand "since
> > the
> > necessary changes are alread there in the files"
> >
> > julia
>
>
> The comments are put to notify that memset() is not required because
> vzalloc is used. But we don't need this comment as patches for this
> driver has already been applied to remove memset() code. Also I have
> checked the whole file, but there isn't any place where memset() needs
> to be removed.

My interpretation was that the comments were there to explain why the call
to memset is in comments.  But both could just go.  There is another
commented call to memset in rtw_recv.c that doesn't have the preceding
justification.  That could go too.

julia

>
>  
> >
> >
> > >
> > >
> > > Signed-off-by: Shreeya Patel 
> > > ---
> > >  drivers/staging/rtl8723bs/core/rtw_mlme.c | 3 ---
> > >  drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 3 ---
> > >  drivers/staging/rtl8723bs/core/rtw_recv.c | 3 ---
> > >  drivers/staging/rtl8723bs/core/rtw_xmit.c | 3 ---
> > >  4 files changed, 12 deletions(-)
> > >
> > > diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c
> > > b/drivers/staging/rtl8723bs/core/rtw_mlme.c
> > > index 6b77820..5b583f7 100644
> > > --- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
> > > +++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
> > > @@ -28,9 +28,6 @@ sint_rtw_init_mlme_priv(struct adapter
> > > *padapter)
> > >   struct mlme_priv    *pmlmepriv = &padapter->mlmepriv;
> > >   sintres = _SUCCESS;
> > >
> > > - /*  We don't need to memset padapter->XXX to zero, because
> > > adapter is allocated by vzalloc(). */
> > > - /* memset((u8 *)pmlmepriv, 0, sizeof(struct mlme_priv));
> > > */
> > > -
> > >   pmlmepriv->nic_hdl = (u8 *)padapter;
> > >
> > >   pmlmepriv->pscanned = NULL;
> > > diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> > > b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> > > index b6d137f..ca35c1c 100644
> > > --- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> > > +++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> > > @@ -474,9 +474,6 @@ int   init_mlme_ext_priv(struct adapter
> > > *padapter)
> > >   struct mlme_priv *pmlmepriv = &(padapter->mlmepriv);
> > >   struct mlme_ext_info *pmlmeinfo = &(pmlmeext-
> > > >mlmext_info);
> > >
> > > - /*  We don't need to memset padapter->XXX to zero, because
> > > adapter is allocated by vzalloc(). */
> > > - /* memset((u8 *)pmlmeext, 0, sizeof(struct
> > > mlme_ext_priv)); */
> > > -
> > >   pmlmeext->padapter = padapter;
> > >
> > >   /* fill_fwpriv(padapter, &(pmlmeext->fwpriv)); */
> > > diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c
> > > b/drivers/staging/rtl8723bs/core/rtw_recv.c
> > > index 68a6303..9c08307 100644
> > > --- a/drivers/staging/rtl8723bs/core/rtw_recv.c
> > > +++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
> > > @@ -46,9 +46,6 @@ sint _rtw_init_recv_priv(struct recv_priv
> > > *precvpriv, struct adapter *padapter)
> > >   union recv_frame *precvframe;
> > >   sintres = _SUCCESS;
> > >
> > > - /*  We don't need to memset padapter->XXX to zero, because
> > > adapter is allocated by vzalloc(). */
> > > - /* memset((unsigned char *)precvpriv, 0, sizeof
> > > (struct  recv_priv)); */
> > > -
> > >   spin_lock_init(&precvpriv->lock);
> > >
> > >   _rtw_init_queue(&precvpriv->free_recv_queue);
> > > diff --git a/drivers/staging/rtl8723bs/core/rtw_xmit.c
> > > b/drivers/staging/rtl8723bs/core/rtw_xmit.c
> > > index 022f654..8cd05f8 100644
> > > --- a/drivers/staging/rtl8723bs/core/rtw_xmit.c
> > > +++ b/drivers/staging/rtl8723bs/core/rtw_xmit.c
> > > @@ -51,9 +51,6 @@ s32 _rtw_init_xmit_priv(struct xmit_priv
> > > *pxmitpriv, struct adapter *padapter)
> > >   struct xmit_frame *pxframe;
> > >   sintres = _SUCCESS;
> > >
> > > - /*  We don't need to memset padapter->XXX to zero, because
> > > adapter is allocated by vzalloc(). */
> > > - /* memset((unsigned char *)pxmitpriv, 0, sizeof(struct
> > > xmit_priv)); */
> > > -
> > >   spin_lock_init(&pxmitpriv->lock);
> > >   spin_lock_init(&pxmitpriv->lock_sctx);
> > >   sema_init(&pxmitpriv->xmit_sema, 0);
> > > --
> > > 2.7.4
> > >
> > > --
> > > You received this message because you are subscribed to the Google
> > > Groups "outreachy-kernel" group.
> > > To unsubscribe from this group and stop receiving emails from it,
> > > send an email to outreachy-kernel+unsubscr...@googlegroups.com.
> > > To post to this group, send email to outreachy-kernel@googlegroups.
> > > com.
> > > To view this discussion on the web visit https://groups.google.com/
> > > d/msgid/outreachy-kernel/1506711232-19670-1-git-send-email-
> > > shreeya.patel23498%40gmail.com.
> >

Re: [Outreachy kernel] [PATCH] Staging: rtl8188eu: core: Use list_entry instead of container_of

2017-09-29 Thread Srishti Sharma
On Sat, Sep 30, 2017 at 10:35 AM, Julia Lawall  wrote:
>
>
> On Sat, 30 Sep 2017, Srishti Sharma wrote:
>
>> For variables of the type struct list_head* use list_entry to access
>> the current list element instead of using container_of.
>> Done using the following semantic patch by coccinelle.
>>
>> @r@
>> identifier e;
>> struct list_head* l;
>> @@
>>
>> <... when != l == NULL
>> l;
>> ...>
> I don't see what is the goal of the above code.  The list_head variable is
> not going to be in a statement by itself.  There is also no need to check
> for l being NULL.  If it is NULL, the original code is incorrect too.

Since only those container_of are to replaced with list_entry which
have a variable of type list_head*  , I wanted to check if it occurs
anywhere before
container_of , which it only does in it's declaration , because it
can't be in any
statement by itself . I think it will be better to write .

@r@
identifier e;
struct list_head* l;
@@

<...
container_of(l,...);
...>

e =
-container_of
+list_entry
 (
  )

>> (
>> e =
>> -container_of
>> +list_entry
>>  (
>>   ...)
>> )
>
> Here you don't need the outer ( ).  This makes a disjunction with only
> one choice.  Since there is only one choice, you don't need the
> disjunction.

Thanks a lot , for pointing out the errors .

Regards,
Srishti
> julia
>
>> Signed-off-by: Srishti Sharma 
>> ---
>>  drivers/staging/rtl8188eu/core/rtw_sta_mgt.c | 12 ++--
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c 
>> b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
>> index 22cf362..f9df4ac 100644
>> --- a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
>> +++ b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
>> @@ -152,8 +152,8 @@ u32   _rtw_free_sta_priv(struct   sta_priv 
>> *pstapriv)
>>   while (phead != plist) {
>>   int i;
>>
>> - psta = container_of(plist, struct sta_info,
>> - hash_list);
>> + psta = list_entry(plist, struct sta_info,
>> +   hash_list);
>>   plist = plist->next;
>>
>>   for (i = 0; i < 16; i++) {
>> @@ -323,7 +323,7 @@ u32   rtw_free_stainfo(struct adapter *padapter, 
>> struct sta_info *psta)
>>   plist = phead->next;
>>
>>   while (!list_empty(phead)) {
>> - prframe = container_of(plist, struct recv_frame, list);
>> + prframe = list_entry(plist, struct recv_frame, list);
>>
>>   plist = plist->next;
>>
>> @@ -399,7 +399,7 @@ void rtw_free_all_stainfo(struct adapter *padapter)
>>   plist = phead->next;
>>
>>   while (phead != plist) {
>> - psta = container_of(plist, struct sta_info, hash_list);
>> + psta = list_entry(plist, struct sta_info, hash_list);
>>
>>   plist = plist->next;
>>
>> @@ -435,7 +435,7 @@ struct sta_info *rtw_get_stainfo(struct sta_priv 
>> *pstapriv, u8 *hwaddr)
>>   plist = phead->next;
>>
>>   while (phead != plist) {
>> - psta = container_of(plist, struct sta_info, hash_list);
>> + psta = list_entry(plist, struct sta_info, hash_list);
>>
>>   if ((!memcmp(psta->hwaddr, addr, ETH_ALEN)) == true) {
>>   /*  if found the matched address */
>> @@ -493,7 +493,7 @@ u8 rtw_access_ctrl(struct adapter *padapter, u8 
>> *mac_addr)
>>   phead = get_list_head(pacl_node_q);
>>   plist = phead->next;
>>   while (phead != plist) {
>> - paclnode = container_of(plist, struct rtw_wlan_acl_node, list);
>> + paclnode = list_entry(plist, struct rtw_wlan_acl_node, list);
>>   plist = plist->next;
>>
>>   if (!memcmp(paclnode->addr, mac_addr, ETH_ALEN)) {
>> --
>> 2.7.4
>>
>> --
>> You received this message because you are subscribed to the Google Groups 
>> "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to outreachy-kernel+unsubscr...@googlegroups.com.
>> To post to this group, send email to outreachy-ker...@googlegroups.com.
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/outreachy-kernel/1506734581-10932-1-git-send-email-srishtishar%40gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [Outreachy kernel] [PATCH] Staging: rtl8188eu: core: Use list_entry instead of container_of

2017-09-29 Thread Julia Lawall


On Sat, 30 Sep 2017, Srishti Sharma wrote:

> On Sat, Sep 30, 2017 at 10:35 AM, Julia Lawall  wrote:
> >
> >
> > On Sat, 30 Sep 2017, Srishti Sharma wrote:
> >
> >> For variables of the type struct list_head* use list_entry to access
> >> the current list element instead of using container_of.
> >> Done using the following semantic patch by coccinelle.
> >>
> >> @r@
> >> identifier e;
> >> struct list_head* l;
> >> @@
> >>
> >> <... when != l == NULL
> >> l;
> >> ...>
> > I don't see what is the goal of the above code.  The list_head variable is
> > not going to be in a statement by itself.  There is also no need to check
> > for l being NULL.  If it is NULL, the original code is incorrect too.
>
> Since only those container_of are to replaced with list_entry which
> have a variable of type list_head*  , I wanted to check if it occurs
> anywhere before
> container_of ,

Why?  If it is a list, then it seems appropriate to access it using
list_head.

> which it only does in it's declaration , because it
> can't be in any
> statement by itself . I think it will be better to write .
>
> @r@
> identifier e;
> struct list_head* l;
> @@
>
> <...
> container_of(l,...);
> ...>

This doesn't ensure that there is a preceding container_of, if that is
what you are trying to do.  The problem is that <... P ...> finds 0 or
more occurrences of pattern P, not 1 or more occurrences.  1 or more
occurrences is <+... P ...+>.  But it would be simpler if you want an
occurrence of container_of before the thing you are transforming to put

container_of(l,...);
...
e = ...

But this doesn't make sense either, partly because the preceding
container_of is just not needed, and also because a container_of would not
be in a statement by itself.  In Coccinelle, when you put a ; after
something is means that the thing is a complete statement, not just the
end half of a statement.

julia

> e =
> -container_of
> +list_entry
>  (
>   )
>
> >> (
> >> e =
> >> -container_of
> >> +list_entry
> >>  (
> >>   ...)
> >> )
> >
> > Here you don't need the outer ( ).  This makes a disjunction with only
> > one choice.  Since there is only one choice, you don't need the
> > disjunction.
>
> Thanks a lot , for pointing out the errors .
>
> Regards,
> Srishti
> > julia
> >
> >> Signed-off-by: Srishti Sharma 
> >> ---
> >>  drivers/staging/rtl8188eu/core/rtw_sta_mgt.c | 12 ++--
> >>  1 file changed, 6 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c 
> >> b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> >> index 22cf362..f9df4ac 100644
> >> --- a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> >> +++ b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> >> @@ -152,8 +152,8 @@ u32   _rtw_free_sta_priv(struct   sta_priv 
> >> *pstapriv)
> >>   while (phead != plist) {
> >>   int i;
> >>
> >> - psta = container_of(plist, struct sta_info,
> >> - hash_list);
> >> + psta = list_entry(plist, struct sta_info,
> >> +   hash_list);
> >>   plist = plist->next;
> >>
> >>   for (i = 0; i < 16; i++) {
> >> @@ -323,7 +323,7 @@ u32   rtw_free_stainfo(struct adapter *padapter, 
> >> struct sta_info *psta)
> >>   plist = phead->next;
> >>
> >>   while (!list_empty(phead)) {
> >> - prframe = container_of(plist, struct recv_frame, 
> >> list);
> >> + prframe = list_entry(plist, struct recv_frame, list);
> >>
> >>   plist = plist->next;
> >>
> >> @@ -399,7 +399,7 @@ void rtw_free_all_stainfo(struct adapter *padapter)
> >>   plist = phead->next;
> >>
> >>   while (phead != plist) {
> >> - psta = container_of(plist, struct sta_info, 
> >> hash_list);
> >> + psta = list_entry(plist, struct sta_info, hash_list);
> >>
> >>   plist = plist->next;
> >>
> >> @@ -435,7 +435,7 @@ struct sta_info *rtw_get_stainfo(struct sta_priv 
> >> *pstapriv, u8 *hwaddr)
> >>   plist = phead->next;
> >>
> >>   while (phead != plist) {
> >> - psta = container_of(plist, struct sta_info, hash_list);
> >> + psta = list_entry(plist, struct sta_info, hash_list);
> >>
> >>   if ((!memcmp(psta->hwaddr, addr, ETH_ALEN)) == true) {
> >>   /*  if found the matched address */
> >> @@ -493,7 +493,7 @@ u8 rtw_access_ctrl(struct adapter *padapter, u8 
> >> *mac_addr)
> >>   phead = get_list_head(pacl_node_q);
> >>   plist = phead->next;
> >>   while (phead != plist) {
> >> - paclnode = container_of(plist, struct rtw_wlan_acl_node, 
> >> list);
> >> + paclnode = list_entry(plist, struct rtw_wlan_acl_node, list);
> >>

Re: [Outreachy kernel] [PATCH] Staging: rtl8188eu: core: Use list_entry instead of container_of

2017-09-29 Thread Srishti Sharma
On Sat, Sep 30, 2017 at 11:36 AM, Julia Lawall  wrote:
>
>
> On Sat, 30 Sep 2017, Srishti Sharma wrote:
>
>> On Sat, Sep 30, 2017 at 10:35 AM, Julia Lawall  wrote:
>> >
>> >
>> > On Sat, 30 Sep 2017, Srishti Sharma wrote:
>> >
>> >> For variables of the type struct list_head* use list_entry to access
>> >> the current list element instead of using container_of.
>> >> Done using the following semantic patch by coccinelle.
>> >>
>> >> @r@
>> >> identifier e;
>> >> struct list_head* l;
>> >> @@
>> >>
>> >> <... when != l == NULL
>> >> l;
>> >> ...>
>> > I don't see what is the goal of the above code.  The list_head variable is
>> > not going to be in a statement by itself.  There is also no need to check
>> > for l being NULL.  If it is NULL, the original code is incorrect too.
>>
>> Since only those container_of are to replaced with list_entry which
>> have a variable of type list_head*  , I wanted to check if it occurs
>> anywhere before
>> container_of ,
>
> Why?  If it is a list, then it seems appropriate to access it using
> list_head.
>
>> which it only does in it's declaration , because it
>> can't be in any
>> statement by itself . I think it will be better to write .
>>
>> @r@
>> identifier e;
>> struct list_head* l;
>> @@
>>
>> <...
>> container_of(l,...);
>> ...>
>
> This doesn't ensure that there is a preceding container_of, if that is
> what you are trying to do.  The problem is that <... P ...> finds 0 or
> more occurrences of pattern P, not 1 or more occurrences.  1 or more
> occurrences is <+... P ...+>.  But it would be simpler if you want an
> occurrence of container_of before the thing you are transforming to put
>
> container_of(l,...);
> ...
> e = ...
>
> But this doesn't make sense either, partly because the preceding
> container_of is just not needed, and also because a container_of would not
> be in a statement by itself.  In Coccinelle, when you put a ; after
> something is means that the thing is a complete statement, not just the
> end half of a statement.

So , I guess we can simply write.

e =
-container_of
+list_entry
 (l,...)

Regards,
Srishti

>
> julia
>
>> e =
>> -container_of
>> +list_entry
>>  (
>>   )
>>
>> >> (
>> >> e =
>> >> -container_of
>> >> +list_entry
>> >>  (
>> >>   ...)
>> >> )
>> >
>> > Here you don't need the outer ( ).  This makes a disjunction with only
>> > one choice.  Since there is only one choice, you don't need the
>> > disjunction.
>>
>> Thanks a lot , for pointing out the errors .
>>
>> Regards,
>> Srishti
>> > julia
>> >
>> >> Signed-off-by: Srishti Sharma 
>> >> ---
>> >>  drivers/staging/rtl8188eu/core/rtw_sta_mgt.c | 12 ++--
>> >>  1 file changed, 6 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c 
>> >> b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
>> >> index 22cf362..f9df4ac 100644
>> >> --- a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
>> >> +++ b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
>> >> @@ -152,8 +152,8 @@ u32   _rtw_free_sta_priv(struct   sta_priv 
>> >> *pstapriv)
>> >>   while (phead != plist) {
>> >>   int i;
>> >>
>> >> - psta = container_of(plist, struct sta_info,
>> >> - hash_list);
>> >> + psta = list_entry(plist, struct sta_info,
>> >> +   hash_list);
>> >>   plist = plist->next;
>> >>
>> >>   for (i = 0; i < 16; i++) {
>> >> @@ -323,7 +323,7 @@ u32   rtw_free_stainfo(struct adapter *padapter, 
>> >> struct sta_info *psta)
>> >>   plist = phead->next;
>> >>
>> >>   while (!list_empty(phead)) {
>> >> - prframe = container_of(plist, struct recv_frame, 
>> >> list);
>> >> + prframe = list_entry(plist, struct recv_frame, 
>> >> list);
>> >>
>> >>   plist = plist->next;
>> >>
>> >> @@ -399,7 +399,7 @@ void rtw_free_all_stainfo(struct adapter *padapter)
>> >>   plist = phead->next;
>> >>
>> >>   while (phead != plist) {
>> >> - psta = container_of(plist, struct sta_info, 
>> >> hash_list);
>> >> + psta = list_entry(plist, struct sta_info, 
>> >> hash_list);
>> >>
>> >>   plist = plist->next;
>> >>
>> >> @@ -435,7 +435,7 @@ struct sta_info *rtw_get_stainfo(struct sta_priv 
>> >> *pstapriv, u8 *hwaddr)
>> >>   plist = phead->next;
>> >>
>> >>   while (phead != plist) {
>> >> - psta = container_of(plist, struct sta_info, hash_list);
>> >> + psta = list_entry(plist, struct sta_info, hash_list);
>> >>
>> >>   if ((!memcmp(psta->hwaddr, addr, ETH_ALEN)) == true) {
>> >>   /*  if found the matched address */
>> >> @@ -493,7 +493,7 @@ u8 rtw_access_ctrl(struct adapte

Re: [Outreachy kernel] [PATCH] Staging: rtl8188eu: core: Use list_entry instead of container_of

2017-09-29 Thread Julia Lawall


On Sat, 30 Sep 2017, Srishti Sharma wrote:

> On Sat, Sep 30, 2017 at 11:36 AM, Julia Lawall  wrote:
> >
> >
> > On Sat, 30 Sep 2017, Srishti Sharma wrote:
> >
> >> On Sat, Sep 30, 2017 at 10:35 AM, Julia Lawall  
> >> wrote:
> >> >
> >> >
> >> > On Sat, 30 Sep 2017, Srishti Sharma wrote:
> >> >
> >> >> For variables of the type struct list_head* use list_entry to access
> >> >> the current list element instead of using container_of.
> >> >> Done using the following semantic patch by coccinelle.
> >> >>
> >> >> @r@
> >> >> identifier e;
> >> >> struct list_head* l;
> >> >> @@
> >> >>
> >> >> <... when != l == NULL
> >> >> l;
> >> >> ...>
> >> > I don't see what is the goal of the above code.  The list_head variable 
> >> > is
> >> > not going to be in a statement by itself.  There is also no need to check
> >> > for l being NULL.  If it is NULL, the original code is incorrect too.
> >>
> >> Since only those container_of are to replaced with list_entry which
> >> have a variable of type list_head*  , I wanted to check if it occurs
> >> anywhere before
> >> container_of ,
> >
> > Why?  If it is a list, then it seems appropriate to access it using
> > list_head.
> >
> >> which it only does in it's declaration , because it
> >> can't be in any
> >> statement by itself . I think it will be better to write .
> >>
> >> @r@
> >> identifier e;
> >> struct list_head* l;
> >> @@
> >>
> >> <...
> >> container_of(l,...);
> >> ...>
> >
> > This doesn't ensure that there is a preceding container_of, if that is
> > what you are trying to do.  The problem is that <... P ...> finds 0 or
> > more occurrences of pattern P, not 1 or more occurrences.  1 or more
> > occurrences is <+... P ...+>.  But it would be simpler if you want an
> > occurrence of container_of before the thing you are transforming to put
> >
> > container_of(l,...);
> > ...
> > e = ...
> >
> > But this doesn't make sense either, partly because the preceding
> > container_of is just not needed, and also because a container_of would not
> > be in a statement by itself.  In Coccinelle, when you put a ; after
> > something is means that the thing is a complete statement, not just the
> > end half of a statement.
>
> So , I guess we can simply write.
>
> e =
> -container_of
> +list_entry
>  (l,...)

Yes.  You don't even need the e =.  As long as there is a call to
container_of on a list_head * value, you can change it.

julia

>
> Regards,
> Srishti
>
> >
> > julia
> >
> >> e =
> >> -container_of
> >> +list_entry
> >>  (
> >>   )
> >>
> >> >> (
> >> >> e =
> >> >> -container_of
> >> >> +list_entry
> >> >>  (
> >> >>   ...)
> >> >> )
> >> >
> >> > Here you don't need the outer ( ).  This makes a disjunction with only
> >> > one choice.  Since there is only one choice, you don't need the
> >> > disjunction.
> >>
> >> Thanks a lot , for pointing out the errors .
> >>
> >> Regards,
> >> Srishti
> >> > julia
> >> >
> >> >> Signed-off-by: Srishti Sharma 
> >> >> ---
> >> >>  drivers/staging/rtl8188eu/core/rtw_sta_mgt.c | 12 ++--
> >> >>  1 file changed, 6 insertions(+), 6 deletions(-)
> >> >>
> >> >> diff --git a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c 
> >> >> b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> >> >> index 22cf362..f9df4ac 100644
> >> >> --- a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> >> >> +++ b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> >> >> @@ -152,8 +152,8 @@ u32   _rtw_free_sta_priv(struct   sta_priv 
> >> >> *pstapriv)
> >> >>   while (phead != plist) {
> >> >>   int i;
> >> >>
> >> >> - psta = container_of(plist, struct 
> >> >> sta_info,
> >> >> - hash_list);
> >> >> + psta = list_entry(plist, struct sta_info,
> >> >> +   hash_list);
> >> >>   plist = plist->next;
> >> >>
> >> >>   for (i = 0; i < 16; i++) {
> >> >> @@ -323,7 +323,7 @@ u32   rtw_free_stainfo(struct adapter 
> >> >> *padapter, struct sta_info *psta)
> >> >>   plist = phead->next;
> >> >>
> >> >>   while (!list_empty(phead)) {
> >> >> - prframe = container_of(plist, struct recv_frame, 
> >> >> list);
> >> >> + prframe = list_entry(plist, struct recv_frame, 
> >> >> list);
> >> >>
> >> >>   plist = plist->next;
> >> >>
> >> >> @@ -399,7 +399,7 @@ void rtw_free_all_stainfo(struct adapter *padapter)
> >> >>   plist = phead->next;
> >> >>
> >> >>   while (phead != plist) {
> >> >> - psta = container_of(plist, struct sta_info, 
> >> >> hash_list);
> >> >> + psta = list_entry(plist, struct sta_info, 
> >> >> hash_list);
> >> >>
> >> >>   plist = plist->next;
> >> >>
> >> >> @@ -435,7 +435,7 @@ struct sta_info *rtw

Re: [Outreachy kernel] [PATCH] Staging: rtl8188eu: core: Use list_entry instead of container_of

2017-09-29 Thread Srishti Sharma
On Sat, Sep 30, 2017 at 11:51 AM, Julia Lawall  wrote:
>
>
> On Sat, 30 Sep 2017, Srishti Sharma wrote:
>
>> On Sat, Sep 30, 2017 at 11:36 AM, Julia Lawall  wrote:
>> >
>> >
>> > On Sat, 30 Sep 2017, Srishti Sharma wrote:
>> >
>> >> On Sat, Sep 30, 2017 at 10:35 AM, Julia Lawall  
>> >> wrote:
>> >> >
>> >> >
>> >> > On Sat, 30 Sep 2017, Srishti Sharma wrote:
>> >> >
>> >> >> For variables of the type struct list_head* use list_entry to access
>> >> >> the current list element instead of using container_of.
>> >> >> Done using the following semantic patch by coccinelle.
>> >> >>
>> >> >> @r@
>> >> >> identifier e;
>> >> >> struct list_head* l;
>> >> >> @@
>> >> >>
>> >> >> <... when != l == NULL
>> >> >> l;
>> >> >> ...>
>> >> > I don't see what is the goal of the above code.  The list_head variable 
>> >> > is
>> >> > not going to be in a statement by itself.  There is also no need to 
>> >> > check
>> >> > for l being NULL.  If it is NULL, the original code is incorrect too.
>> >>
>> >> Since only those container_of are to replaced with list_entry which
>> >> have a variable of type list_head*  , I wanted to check if it occurs
>> >> anywhere before
>> >> container_of ,
>> >
>> > Why?  If it is a list, then it seems appropriate to access it using
>> > list_head.
>> >
>> >> which it only does in it's declaration , because it
>> >> can't be in any
>> >> statement by itself . I think it will be better to write .
>> >>
>> >> @r@
>> >> identifier e;
>> >> struct list_head* l;
>> >> @@
>> >>
>> >> <...
>> >> container_of(l,...);
>> >> ...>
>> >
>> > This doesn't ensure that there is a preceding container_of, if that is
>> > what you are trying to do.  The problem is that <... P ...> finds 0 or
>> > more occurrences of pattern P, not 1 or more occurrences.  1 or more
>> > occurrences is <+... P ...+>.  But it would be simpler if you want an
>> > occurrence of container_of before the thing you are transforming to put
>> >
>> > container_of(l,...);
>> > ...
>> > e = ...
>> >
>> > But this doesn't make sense either, partly because the preceding
>> > container_of is just not needed, and also because a container_of would not
>> > be in a statement by itself.  In Coccinelle, when you put a ; after
>> > something is means that the thing is a complete statement, not just the
>> > end half of a statement.
>>
>> So , I guess we can simply write.
>>
>> e =
>> -container_of
>> +list_entry
>>  (l,...)
>
> Yes.  You don't even need the e =.  As long as there is a call to
> container_of on a list_head * value, you can change it.

Okay, Thanks again :) , I'll make the changes and resend .

Regards,
Srishti
>
> julia
>
>>
>> Regards,
>> Srishti
>>
>> >
>> > julia
>> >
>> >> e =
>> >> -container_of
>> >> +list_entry
>> >>  (
>> >>   )
>> >>
>> >> >> (
>> >> >> e =
>> >> >> -container_of
>> >> >> +list_entry
>> >> >>  (
>> >> >>   ...)
>> >> >> )
>> >> >
>> >> > Here you don't need the outer ( ).  This makes a disjunction with only
>> >> > one choice.  Since there is only one choice, you don't need the
>> >> > disjunction.
>> >>
>> >> Thanks a lot , for pointing out the errors .
>> >>
>> >> Regards,
>> >> Srishti
>> >> > julia
>> >> >
>> >> >> Signed-off-by: Srishti Sharma 
>> >> >> ---
>> >> >>  drivers/staging/rtl8188eu/core/rtw_sta_mgt.c | 12 ++--
>> >> >>  1 file changed, 6 insertions(+), 6 deletions(-)
>> >> >>
>> >> >> diff --git a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c 
>> >> >> b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
>> >> >> index 22cf362..f9df4ac 100644
>> >> >> --- a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
>> >> >> +++ b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
>> >> >> @@ -152,8 +152,8 @@ u32   _rtw_free_sta_priv(struct   sta_priv 
>> >> >> *pstapriv)
>> >> >>   while (phead != plist) {
>> >> >>   int i;
>> >> >>
>> >> >> - psta = container_of(plist, struct 
>> >> >> sta_info,
>> >> >> - hash_list);
>> >> >> + psta = list_entry(plist, struct sta_info,
>> >> >> +   hash_list);
>> >> >>   plist = plist->next;
>> >> >>
>> >> >>   for (i = 0; i < 16; i++) {
>> >> >> @@ -323,7 +323,7 @@ u32   rtw_free_stainfo(struct adapter 
>> >> >> *padapter, struct sta_info *psta)
>> >> >>   plist = phead->next;
>> >> >>
>> >> >>   while (!list_empty(phead)) {
>> >> >> - prframe = container_of(plist, struct recv_frame, 
>> >> >> list);
>> >> >> + prframe = list_entry(plist, struct recv_frame, 
>> >> >> list);
>> >> >>
>> >> >>   plist = plist->next;
>> >> >>
>> >> >> @@ -399,7 +399,7 @@ void rtw_free_all_stainfo(struct adapter *padapter)
>> >> >>   plist = phead->next;
>> >> >>
>> >> >>   while (phead != plist) {
>> >>