[PATCH] vmbus: hvsock: add proper sync for vmbus_hvsock_device_unregister()

2017-10-09 Thread Dexuan Cui 

Without the patch, vmbus_hvsock_device_unregister() can destroy the device
prematurely when close() is called, and can cause NULl dereferencing or
potential data loss (the last portion of the data stream may be dropped
prematurely).

Signed-off-by: Dexuan Cui 
Cc: K. Y. Srinivasan 
Cc: Haiyang Zhang 
Cc: Stephen Hemminger 
---

The patch is rebased on today's char-misc tree's char-misc-linus branch.
Please consider it for v4.14.

 drivers/hv/channel_mgmt.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 018d2e0..379b0df 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -937,7 +937,10 @@ void vmbus_hvsock_device_unregister(struct vmbus_channel 
*channel)
 {
BUG_ON(!is_hvsock_channel(channel));
 
-   channel->rescind = true;
+   /* We always get a rescind msg when a connection is closed. */
+   while (!READ_ONCE(channel->probe_done) || !READ_ONCE(channel->rescind))
+   msleep(1);
+
vmbus_device_unregister(channel->device_obj);
 }
 EXPORT_SYMBOL_GPL(vmbus_hvsock_device_unregister);
-- 
2.7.4

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

Re: [PATCH] timer: Remove meaningless .data/.function assignments

2017-10-09 Thread David Miller 
From: Kees Cook 
Date: Mon, 9 Oct 2017 17:10:32 -0700

> Several timer users needlessly reset their .function/.data fields during
> their timer callback, but nothing else changes them. Some users do not
> use their .data field at all. Each instance is removed here.
> 
> Cc: Krzysztof Halasa 
> Cc: Aditya Shankar 
> Cc: Ganesh Krishna 
> Cc: Greg Kroah-Hartman 
> Cc: Jens Axboe 
> Cc: net...@vger.kernel.org
> Cc: linux-wirel...@vger.kernel.org
> Cc: de...@driverdev.osuosl.org
> Signed-off-by: Kees Cook 
> Acked-by: Greg Kroah-Hartman  # for staging
> Acked-by: Krzysztof Halasa  # for wan/hdlc*
> Acked-by: Jens Axboe  # for amiflop
> ---
> This should go via the timer/core tree, please. It's been acked by each
> of the maintainers. Thanks!

For networking bits:

Acked-by: David S. Miller 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v5 2/2] staging: ion: create one device entry per heap

2017-10-09 Thread Laura Abbott 
On 10/09/2017 03:08 PM, Mark Brown wrote:
> On Mon, Oct 09, 2017 at 02:25:47PM -0700, Laura Abbott wrote:
> 
>> Anyway, to move this forward I think we need to see a proof of concept
>> of using selinux to protect access to specific heaps.
> 
> Aren't Unix permissions enough with separate files or am I
> misunderstanding what you're looking to see a proof of concept for?
> 

The goal is to be able to restrict heap access to certain services
and selinux groups on Android so straight unix permissions aren't
sufficient.

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

[PATCH] timer: Remove meaningless .data/.function assignments

2017-10-09 Thread Kees Cook 
Several timer users needlessly reset their .function/.data fields during
their timer callback, but nothing else changes them. Some users do not
use their .data field at all. Each instance is removed here.

Cc: Krzysztof Halasa 
Cc: Aditya Shankar 
Cc: Ganesh Krishna 
Cc: Greg Kroah-Hartman 
Cc: Jens Axboe 
Cc: net...@vger.kernel.org
Cc: linux-wirel...@vger.kernel.org
Cc: de...@driverdev.osuosl.org
Signed-off-by: Kees Cook 
Acked-by: Greg Kroah-Hartman  # for staging
Acked-by: Krzysztof Halasa  # for wan/hdlc*
Acked-by: Jens Axboe  # for amiflop
---
This should go via the timer/core tree, please. It's been acked by each
of the maintainers. Thanks!
---
 drivers/block/amiflop.c   | 3 +--
 drivers/net/wan/hdlc_cisco.c  | 2 --
 drivers/net/wan/hdlc_fr.c | 2 --
 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 4 +---
 4 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/block/amiflop.c b/drivers/block/amiflop.c
index 49908c74bfcb..4e3fb9f104af 100644
--- a/drivers/block/amiflop.c
+++ b/drivers/block/amiflop.c
@@ -323,7 +323,7 @@ static void fd_deselect (int drive)
 
 }
 
-static void motor_on_callback(unsigned long nr)
+static void motor_on_callback(unsigned long ignored)
 {
if (!(ciaa.pra & DSKRDY) || --on_attempts == 0) {
complete_all(&motor_on_completion);
@@ -344,7 +344,6 @@ static int fd_motor_on(int nr)
fd_select(nr);
 
reinit_completion(&motor_on_completion);
-   motor_on_timer.data = nr;
mod_timer(&motor_on_timer, jiffies + HZ/2);
 
on_attempts = 10;
diff --git a/drivers/net/wan/hdlc_cisco.c b/drivers/net/wan/hdlc_cisco.c
index a408abc25512..f4b0ab34f048 100644
--- a/drivers/net/wan/hdlc_cisco.c
+++ b/drivers/net/wan/hdlc_cisco.c
@@ -276,8 +276,6 @@ static void cisco_timer(unsigned long arg)
spin_unlock(&st->lock);
 
st->timer.expires = jiffies + st->settings.interval * HZ;
-   st->timer.function = cisco_timer;
-   st->timer.data = arg;
add_timer(&st->timer);
 }
 
diff --git a/drivers/net/wan/hdlc_fr.c b/drivers/net/wan/hdlc_fr.c
index 78596e42a3f3..07f265fa2826 100644
--- a/drivers/net/wan/hdlc_fr.c
+++ b/drivers/net/wan/hdlc_fr.c
@@ -644,8 +644,6 @@ static void fr_timer(unsigned long arg)
state(hdlc)->settings.t391 * HZ;
}
 
-   state(hdlc)->timer.function = fr_timer;
-   state(hdlc)->timer.data = arg;
add_timer(&state(hdlc)->timer);
 }
 
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c 
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index ac5aaafa461c..60f088babf27 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -266,7 +266,7 @@ static void update_scan_time(void)
last_scanned_shadow[i].time_scan = jiffies;
 }
 
-static void remove_network_from_shadow(unsigned long arg)
+static void remove_network_from_shadow(unsigned long unused)
 {
unsigned long now = jiffies;
int i, j;
@@ -287,7 +287,6 @@ static void remove_network_from_shadow(unsigned long arg)
}
 
if (last_scanned_cnt != 0) {
-   hAgingTimer.data = arg;
mod_timer(&hAgingTimer, jiffies + msecs_to_jiffies(AGING_TIME));
}
 }
@@ -304,7 +303,6 @@ static int is_network_in_shadow(struct network_info 
*pstrNetworkInfo,
int i;
 
if (last_scanned_cnt == 0) {
-   hAgingTimer.data = (unsigned long)user_void;
mod_timer(&hAgingTimer, jiffies + msecs_to_jiffies(AGING_TIME));
state = -1;
} else {
-- 
2.7.4


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

Re: [PATCH] Staging: rtlwifi: Remove NULL pointer dereference

2017-10-09 Thread Tobin C. Harding 
On Tue, Oct 10, 2017 at 02:48:58AM +0530, Shreeya Patel wrote:
> Remove NULL pointer dereference as it results in undefined
> behaviour, and will usually lead to a runtime error.

The diff does not show any pointer dereference so it is hard to understand what 
you are trying to do
with this patch.

> Signed-off-by: Shreeya Patel 
> ---
>  drivers/staging/rtlwifi/base.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtlwifi/base.c b/drivers/staging/rtlwifi/base.c
> index b88b0e8..5bb8f98 100644
> --- a/drivers/staging/rtlwifi/base.c
> +++ b/drivers/staging/rtlwifi/base.c
> @@ -781,7 +781,7 @@ static void _rtl_txrate_selectmode(struct ieee80211_hw 
> *hw,
>  
>   struct rtl_priv *rtlpriv = rtl_priv(hw);
>   struct rtl_mac *mac = rtl_mac(rtl_priv(hw));
> - struct rtl_sta_info *sta_entry = NULL;
> + struct rtl_sta_info *sta_entry;

Now the pointer just has garbage in it instead of the testable value of NULL. 
If you are concerned
with the dereference perhaps you could add a NULL check, again it's hard to say 
without seeing the
code.

It is hard to see how this patch is correct though.

Hope this helps,
Tobin.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: typec: tcpm: Validate source and sink caps

2017-10-09 Thread Badhri Jagan Sridharan 
Sorry about delay. Just sent the rebased patchset along with the comments
addressed.

Thanks & Regards,
Badhri.

On Mon, Sep 18, 2017 at 3:20 AM, Greg Kroah-Hartman
 wrote:
> On Thu, Sep 07, 2017 at 06:22:13PM -0700, Badhri Jagan Sridharan wrote:
>> The source and sink caps should follow the following rules.
>> This patch validates whether the src_caps/snk_caps adheres
>> to it.
>>
>> 6.4.1 Capabilities Message
>> A Capabilities message (Source Capabilities message or Sink
>> Capabilities message) shall have at least one Power
>> Data Object for vSafe5V. The Capabilities message shall also
>> contain the sending Port’s information followed by up to
>> 6 additional Power Data Objects. Power Data Objects in a
>> Capabilities message shall be sent in the following order:
>>
>> 1. The vSafe5V Fixed Supply Object shall always be the first object.
>> 2. The remaining Fixed Supply Objects, if present, shall be sent
>>in voltage order; lowest to highest.
>> 3. The Battery Supply Objects, if present shall be sent in Minimum
>>Voltage order; lowest to highest.
>> 4. The Variable Supply (non-battery) Objects, if present, shall be
>>sent in Minimum Voltage order; lowest to highest.
>>
>> Errors in source/sink_caps of the local port will prevent
>> the port registration. Whereas, errors in source caps of partner
>> device would only log them.
>>
>> Signed-off-by: Badhri Jagan Sridharan 
>> ---
>>  drivers/staging/typec/pd.h   |   2 +
>>  drivers/staging/typec/tcpm.c | 107 
>> +++
>>  drivers/staging/typec/tcpm.h |  16 +++
>>  3 files changed, 108 insertions(+), 17 deletions(-)
>
>
> Now that these files are moved in my usb tree, can you rebase and resend
> them?
>
> thanks,
>
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v5 2/2] staging: ion: create one device entry per heap

2017-10-09 Thread Mark Brown 
On Mon, Oct 09, 2017 at 02:25:47PM -0700, Laura Abbott wrote:

> Anyway, to move this forward I think we need to see a proof of concept
> of using selinux to protect access to specific heaps.

Aren't Unix permissions enough with separate files or am I
misunderstanding what you're looking to see a proof of concept for?


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

Re: [PATCH v5 2/2] staging: ion: create one device entry per heap

2017-10-09 Thread Laura Abbott 
On 10/05/2017 06:06 AM, Benjamin Gaignard wrote:
> 2017-10-04 12:17 GMT+02:00 Mark Brown :
>> On Tue, Oct 03, 2017 at 04:08:30PM -0700, Sandeep Patil wrote:
>>
>>> It is entirely possible and easy in android/ueventd to create those nodes
>>> under "/dev/ion/".  (assuming the heap 'subsystem' for these new devices 
>>> will
>>> point to 'ion').
> 
> I think it is the same problem than for webcam under v4l framework.
> Each time you plug a webcam you got a v4l node but android/uevent rules
> the plug order doesn't have impact.
> The same think will happen for ion nodes it may be even easier because
> the heap will always being created in the smae order for a given product
> configuration.
> 

Relying on the heap being created in the same order seems troublesome.
If for some reason it changes in the kernel we might break something
in userspace.

Anyway, to move this forward I think we need to see a proof of concept
of using selinux to protect access to specific heaps.

Thanks,
Laura

>>
>> The reason I didn't say /dev/ion/foo initially is that if people want to
>> keep the existing /dev/ion around for compatibility reasons then the
>> /dev/ion name isn't available which might cause issues.  Otherwise just
>> dumping everything under a directory (perhaps with a different name) was
>> my first thought as well.
>>
>>> (Also FWIW, the SELinux permissions are also possible with the current ion
>>> implementation by adding rules to disallow specific ioctls instead of adding
>>> permissions to access device node as this change would do)
>>
>> AIUI the request is to limit access to specific heaps, and obviously not
>> everyone wants to deal with SELinux at all.

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

[PATCH] Staging: rtlwifi: Remove NULL pointer dereference

2017-10-09 Thread Shreeya Patel 
Remove NULL pointer dereference as it results in undefined
behaviour, and will usually lead to a runtime error.

Signed-off-by: Shreeya Patel 
---
 drivers/staging/rtlwifi/base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtlwifi/base.c b/drivers/staging/rtlwifi/base.c
index b88b0e8..5bb8f98 100644
--- a/drivers/staging/rtlwifi/base.c
+++ b/drivers/staging/rtlwifi/base.c
@@ -781,7 +781,7 @@ static void _rtl_txrate_selectmode(struct ieee80211_hw *hw,
 
struct rtl_priv *rtlpriv = rtl_priv(hw);
struct rtl_mac *mac = rtl_mac(rtl_priv(hw));
-   struct rtl_sta_info *sta_entry = NULL;
+   struct rtl_sta_info *sta_entry;
u8 ratr_index = SET_RATE_ID(RATR_INX_WIRELESS_MC);
 
if (sta) {
-- 
2.7.4

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

Re: [PATCH] pci: Fix a possible sleep-in-atomic bug in pci_set_power_state

2017-10-09 Thread Rafael J. Wysocki 
On Mon, Oct 9, 2017 at 7:18 PM, Bjorn Helgaas  wrote:
> On Mon, Oct 09, 2017 at 12:15:17PM -0500, Bjorn Helgaas wrote:
>> [+cc Rafael, linux-pm]
>>
>> Hi Jia-Ju,
>>
>> On Mon, Oct 09, 2017 at 04:16:20PM +0800, Jia-Ju Bai wrote:
>> > The drivers vt6655 and gma500 call pci_set_power_state under a spinlock, 
>> > which may sleep.
>> > The function call paths are:
>> > gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
>> >   gma_resume_pci
>> > pci_set_power_state
>> >   __pci_start_power_transition (drivers/pci/pci.c)
>> > msleep --> may sleep
>> >
>> > gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
>> >   gma_resume_pci
>> > pci_enable_device
>> >   pci_enable_device_flags (drivers/pci/pci.c)
>> > do_pci_enable_device
>> >   pci_set_power_state
>> > __pci_start_power_transition
>> >   msleep --> may sleep
>> >
>> > vt6655_suspend (acquire the spinlock) 
>> > (drivers/staging/vt6655/device_main.c)
>> >   pci_set_power_state
>> > __pci_start_power_transition (drivers/pci/pci.c)
>> >   msleep --> may sleep
>> >
>> > To fix these bugs, msleep is replaced with mdelay in 
>> > __pci_start_power_transition
>> >
>> > These bugs are found by my static analysis tool and my code review.
>>
>> We can either
>>
>>   - change pci_set_power_state() so it can be called while holding a
>> spinlock (as this patch does), or
>>
>>   - change the drivers so they don't hold the spinlock while calling
>> pci_set_power_state().
>>
>> I think the latter is better because d3cold_delay is typically 100ms,
>> and that's a long time to spin with interrupts disabled.
>>
>> I assume it's easy to produce an actual failure here?  Why haven't we
>> seen bug reports about this?
>
> Sigh, could have saved myself some time if I'd read the whole thread
> before responding :)  Sorry for repeating what Greg already said!

Well, calling pci_set_power_state() with a spinlock held is a bug,
plain and simple, among other things because it may involve running
AML.  Messing up with the single delay in it simply doesn't help.

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

Re: [PATCH 03/15] staging: comedi: ni_routing: Add NI signal routing info

2017-10-09 Thread Spencer E Olson 
On Mon, 2017-10-09 at 10:56 +0100, Ian Abbott wrote:
> On 08/10/17 07:44, Spencer E Olson wrote:
> > On Thu, 2016-11-10 at 18:16 +, Ian Abbott wrote:
> >> On 10/11/16 17:54, Greg Kroah-Hartman wrote:
> >>> On Thu, Nov 10, 2016 at 05:08:36PM +, Ian Abbott wrote:
>  On 12/10/16 12:05, Spencer E. Olson wrote:
> > See README for a thorough discussion of this content.
> >
> > Adds two different collections of CSV files that:
> > 1) summarize the various register values for creating routes
> > for a particular family of NI hardware devices;
> > 2) summarize all possible (direct) routes that a particular device can
> > make--in this case, one file per device (this data is currently only
> > known to be found by examining a screenshot of the "Available 
> > Routes"
> > tab of NI MAX control panel, which is only found on Windows
> > installations of the NI driver).
> >
> > The collection and maintenance of this information is somewhat tedious 
> > and
> > requires frequent re-examination and comparison of NI-MAX and/or the 
> > NI-MHDDK
> > documentation (register programming information) and NI-MHDDK examples.
> > These CSV files are constructed so-as to allow near direct comparison
> > to NI-MAX and NI-MHDDK.  As such, these serve to ease the task of
> > maintaining this knowledge and more quickly enables addition of new NI
> > devices.
> >
> > Signed-off-by: Spencer E. Olson 
> >
> > *** PLEASE FIND ACTUAL PATCH AT:
> > http://www.umich.edu/~olsonse/patches/comedi-devglobal-v1/0003-staging-comedi-ni_routing-Add-NI-signal-routing-info.patch
> >
> > (This patch included some lines that were too long for email)
> > ---
> >   drivers/staging/comedi/drivers/ni_routing/README   | 110 
> > +
> >   .../ni_routing/ni_device_routes/PCI-6070E.csv  |  40 
> >   .../ni_routing/ni_device_routes/PCI-6220.csv   |  46 +
> >   .../ni_routing/ni_device_routes/PCI-6221.csv   |  50 ++
> >   .../ni_routing/ni_device_routes/PCI-6251.csv   |  51 ++
> >   .../ni_routing/ni_device_routes/PCI-6254.csv   |  47 +
> >   .../ni_routing/ni_device_routes/PCI-6259.csv   |  51 ++
> >   .../ni_routing/ni_device_routes/PCI-6534.csv   |  29 ++
> >   .../ni_routing/ni_device_routes/PCI-6602.csv   |  78 
> > +++
> >   .../ni_routing/ni_device_routes/PCI-6713.csv   |  32 ++
> >   .../ni_routing/ni_device_routes/PCI-6723.csv   |  32 ++
> >   .../ni_routing/ni_device_routes/PCI-6733.csv   |  34 +++
> >   .../ni_routing/ni_device_routes/PXI-6030E.csv  |  39 
> >   .../ni_routing/ni_device_routes/PXI-6224.csv   |  46 +
> >   .../ni_routing/ni_device_routes/PXI-6225.csv   |  49 +
> >   .../ni_routing/ni_device_routes/PXI-6251.csv   |  50 ++
> >   .../ni_routing/ni_device_routes/PXI-6733.csv   |  35 +++
> >   .../ni_routing/ni_device_routes/PXIe-6251.csv  |  52 ++
> >   .../drivers/ni_routing/ni_route_values/ni_660x.csv | 100 
> > +++
> >   .../ni_routing/ni_route_values/ni_eseries.csv  |  78 
> > +++
> >   .../ni_routing/ni_route_values/ni_mseries.csv  |  90 
> > +
> >   21 files changed, 1139 insertions(+)
> >   create mode 100644 drivers/staging/comedi/drivers/ni_routing/README
> >   create mode 100644 
> > drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6070E.csv
> >   create mode 100644 
> > drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6220.csv
> >   create mode 100644 
> > drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6221.csv
> >   create mode 100644 
> > drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6251.csv
> >   create mode 100644 
> > drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6254.csv
> >   create mode 100644 
> > drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6259.csv
> >   create mode 100644 
> > drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6534.csv
> >   create mode 100644 
> > drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6602.csv
> >   create mode 100644 
> > drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6713.csv
> >   create mode 100644 
> > drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6723.csv
> >   create mode 100644 
> > drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6733.csv
> >   create mode 100644 
> > drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PXI-6030E.csv
> >   create mode 100644 
> > drivers/staging/comedi/drivers/ni_

Re: [PATCH] pci: Fix a possible sleep-in-atomic bug in pci_set_power_state

2017-10-09 Thread Bjorn Helgaas 
On Mon, Oct 09, 2017 at 12:15:17PM -0500, Bjorn Helgaas wrote:
> [+cc Rafael, linux-pm]
> 
> Hi Jia-Ju,
> 
> On Mon, Oct 09, 2017 at 04:16:20PM +0800, Jia-Ju Bai wrote:
> > The drivers vt6655 and gma500 call pci_set_power_state under a spinlock, 
> > which may sleep.
> > The function call paths are:
> > gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
> >   gma_resume_pci
> > pci_set_power_state
> >   __pci_start_power_transition (drivers/pci/pci.c)
> > msleep --> may sleep
> > 
> > gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
> >   gma_resume_pci
> > pci_enable_device
> >   pci_enable_device_flags (drivers/pci/pci.c)
> > do_pci_enable_device
> >   pci_set_power_state
> > __pci_start_power_transition
> >   msleep --> may sleep
> > 
> > vt6655_suspend (acquire the spinlock) (drivers/staging/vt6655/device_main.c)
> >   pci_set_power_state
> > __pci_start_power_transition (drivers/pci/pci.c)
> >   msleep --> may sleep
> > 
> > To fix these bugs, msleep is replaced with mdelay in 
> > __pci_start_power_transition
> > 
> > These bugs are found by my static analysis tool and my code review.
> 
> We can either
> 
>   - change pci_set_power_state() so it can be called while holding a
> spinlock (as this patch does), or
> 
>   - change the drivers so they don't hold the spinlock while calling 
> pci_set_power_state().
> 
> I think the latter is better because d3cold_delay is typically 100ms,
> and that's a long time to spin with interrupts disabled.
> 
> I assume it's easy to produce an actual failure here?  Why haven't we
> seen bug reports about this?

Sigh, could have saved myself some time if I'd read the whole thread
before responding :)  Sorry for repeating what Greg already said!

> > Signed-off-by: Jia-Ju Bai 
> > ---
> >  drivers/pci/pci.c |2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 6078dfc..7b763a3 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -823,7 +823,7 @@ static void __pci_start_power_transition(struct pci_dev 
> > *dev, pci_power_t state)
> >  */
> > if (dev->runtime_d3cold) {
> > if (dev->d3cold_delay)
> > -   msleep(dev->d3cold_delay);
> > +   mdelay(dev->d3cold_delay);
> > /*
> >  * When powering on a bridge from D3cold, the
> >  * whole hierarchy may be powered on into
> > -- 
> > 1.7.9.5
> > 
> > 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] pci: Fix a possible sleep-in-atomic bug in pci_set_power_state

2017-10-09 Thread Bjorn Helgaas 
[+cc Rafael, linux-pm]

Hi Jia-Ju,

On Mon, Oct 09, 2017 at 04:16:20PM +0800, Jia-Ju Bai wrote:
> The drivers vt6655 and gma500 call pci_set_power_state under a spinlock, 
> which may sleep.
> The function call paths are:
> gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
>   gma_resume_pci
> pci_set_power_state
>   __pci_start_power_transition (drivers/pci/pci.c)
> msleep --> may sleep
> 
> gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
>   gma_resume_pci
> pci_enable_device
>   pci_enable_device_flags (drivers/pci/pci.c)
> do_pci_enable_device
>   pci_set_power_state
> __pci_start_power_transition
>   msleep --> may sleep
> 
> vt6655_suspend (acquire the spinlock) (drivers/staging/vt6655/device_main.c)
>   pci_set_power_state
> __pci_start_power_transition (drivers/pci/pci.c)
>   msleep --> may sleep
> 
> To fix these bugs, msleep is replaced with mdelay in 
> __pci_start_power_transition
> 
> These bugs are found by my static analysis tool and my code review.

We can either

  - change pci_set_power_state() so it can be called while holding a
spinlock (as this patch does), or

  - change the drivers so they don't hold the spinlock while calling 
pci_set_power_state().

I think the latter is better because d3cold_delay is typically 100ms,
and that's a long time to spin with interrupts disabled.

I assume it's easy to produce an actual failure here?  Why haven't we
seen bug reports about this?

Bjorn

> Signed-off-by: Jia-Ju Bai 
> ---
>  drivers/pci/pci.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 6078dfc..7b763a3 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -823,7 +823,7 @@ static void __pci_start_power_transition(struct pci_dev 
> *dev, pci_power_t state)
>*/
>   if (dev->runtime_d3cold) {
>   if (dev->d3cold_delay)
> - msleep(dev->d3cold_delay);
> + mdelay(dev->d3cold_delay);
>   /*
>* When powering on a bridge from D3cold, the
>* whole hierarchy may be powered on into
> -- 
> 1.7.9.5
> 
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2] staging: rtl8723bs: remove implicit int->bool conversions

2017-10-09 Thread Aishwarya Pant 
Implicit type conversions are bad; they hinder readability of code and
have potential to cause bugs. Here the variable wait_ack is always
supplied a bool value while in function declarations it is defined as an
int type. Fix it by defining wait_ack a bool type in all usages.

Signed-off-by: Aishwarya Pant 
Acked-by: Julia Lawall 
---
Changes in v2:
-Wrap commit message in 72 characters

drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c 
b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
index 355ce9b19d1c..0dca5cdef2e7 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
@@ -2831,7 +2831,9 @@ void issue_probersp(struct adapter *padapter, unsigned 
char *da, u8 is_valid_p2p
 
 }
 
-static int _issue_probereq(struct adapter *padapter, struct ndis_802_11_ssid 
*pssid, u8 *da, u8 ch, bool append_wps, int wait_ack)
+static int _issue_probereq(struct adapter *padapter,
+  struct ndis_802_11_ssid *pssid,
+  u8 *da, u8 ch, bool append_wps, bool wait_ack)
 {
int ret = _FAIL;
struct xmit_frame   *pmgntframe;
@@ -3430,7 +3432,8 @@ void issue_assocreq(struct adapter *padapter)
 }
 
 /* when wait_ack is ture, this function shoule be called at process context */
-static int _issue_nulldata(struct adapter *padapter, unsigned char *da, 
unsigned int power_mode, int wait_ack)
+static int _issue_nulldata(struct adapter *padapter, unsigned char *da,
+  unsigned int power_mode, bool wait_ack)
 {
int ret = _FAIL;
struct xmit_frame   *pmgntframe;
@@ -3591,7 +3594,8 @@ s32 issue_nulldata_in_interrupt(struct adapter *padapter, 
u8 *da)
 }
 
 /* when wait_ack is ture, this function shoule be called at process context */
-static int _issue_qos_nulldata(struct adapter *padapter, unsigned char *da, 
u16 tid, int wait_ack)
+static int _issue_qos_nulldata(struct adapter *padapter, unsigned char *da,
+  u16 tid, bool wait_ack)
 {
int ret = _FAIL;
struct xmit_frame   *pmgntframe;
@@ -3715,7 +3719,8 @@ int issue_qos_nulldata(struct adapter *padapter, unsigned 
char *da, u16 tid, int
return ret;
 }
 
-static int _issue_deauth(struct adapter *padapter, unsigned char *da, unsigned 
short reason, u8 wait_ack)
+static int _issue_deauth(struct adapter *padapter, unsigned char *da,
+unsigned short reason, bool wait_ack)
 {
struct xmit_frame   *pmgntframe;
struct pkt_attrib   *pattrib;
-- 
2.11.0

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

Re: [PATCH v5 1/2] staging: ion: simplify ioctl args checking function

2017-10-09 Thread Laura Abbott 
On 10/09/2017 02:21 AM, Benjamin Gaignard wrote:
> 2017-09-27 15:20 GMT+02:00 Benjamin Gaignard :
>> Make arguments checking more easy to read.
>>
> 
> Hi Laura,
> 
> Even if we don't have found a solution for the second patch I believe
> this one could be useful.
> May I ask you your point of view on those few lines ?
> 
> Benjamin
> 

Yes, this looks better.

Acked-by: Laura Abbott 

>> Signed-off-by: Benjamin Gaignard 
>> ---
>>  drivers/staging/android/ion/ion-ioctl.c | 11 +--
>>  1 file changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion-ioctl.c 
>> b/drivers/staging/android/ion/ion-ioctl.c
>> index d9f8b14..e26b786 100644
>> --- a/drivers/staging/android/ion/ion-ioctl.c
>> +++ b/drivers/staging/android/ion/ion-ioctl.c
>> @@ -27,19 +27,18 @@ union ion_ioctl_arg {
>>
>>  static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)
>>  {
>> -   int ret = 0;
>> -
>> switch (cmd) {
>> case ION_IOC_HEAP_QUERY:
>> -   ret = arg->query.reserved0 != 0;
>> -   ret |= arg->query.reserved1 != 0;
>> -   ret |= arg->query.reserved2 != 0;
>> +   if (arg->query.reserved0 ||
>> +   arg->query.reserved1 ||
>> +   arg->query.reserved2)
>> +   return -EINVAL;
>> break;
>> default:
>> break;
>> }
>>
>> -   return ret ? -EINVAL : 0;
>> +   return 0;
>>  }
>>
>>  /* fix up the cases where the ioctl direction bits are incorrect */
>> --
>> 2.7.4
>>
> 
> 
> 

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

Re: [Outreachy kernel] [PATCH] staging: rtl8723bs: remove implicit int->bool conversions

2017-10-09 Thread Julia Lawall 


On Mon, 9 Oct 2017, Aishwarya Pant wrote:

> Implicit type conversions are bad; they hinder readability of code and have
> potential to cause bugs. Here the variable wait_ack is always supplied a bool
> value while in function declarations it is defined as an int type. Fix it by
> defining wait_ack a bool type in all usages.

The commit message lines are a tiny bit on the long side.  If you do git
log, you will see that a few spaces get added in front of your text.  So
you shouldn't really go up to 80 characters.

>
> Signed-off-by: Aishwarya Pant 

Acked-by: Julia Lawall 

> ---
>  drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 13 +
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c 
> b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> index 355ce9b19d1c..0dca5cdef2e7 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> @@ -2831,7 +2831,9 @@ void issue_probersp(struct adapter *padapter, unsigned 
> char *da, u8 is_valid_p2p
>
>  }
>
> -static int _issue_probereq(struct adapter *padapter, struct ndis_802_11_ssid 
> *pssid, u8 *da, u8 ch, bool append_wps, int wait_ack)
> +static int _issue_probereq(struct adapter *padapter,
> +struct ndis_802_11_ssid *pssid,
> +u8 *da, u8 ch, bool append_wps, bool wait_ack)
>  {
>   int ret = _FAIL;
>   struct xmit_frame   *pmgntframe;
> @@ -3430,7 +3432,8 @@ void issue_assocreq(struct adapter *padapter)
>  }
>
>  /* when wait_ack is ture, this function shoule be called at process context 
> */
> -static int _issue_nulldata(struct adapter *padapter, unsigned char *da, 
> unsigned int power_mode, int wait_ack)
> +static int _issue_nulldata(struct adapter *padapter, unsigned char *da,
> +unsigned int power_mode, bool wait_ack)
>  {
>   int ret = _FAIL;
>   struct xmit_frame   *pmgntframe;
> @@ -3591,7 +3594,8 @@ s32 issue_nulldata_in_interrupt(struct adapter 
> *padapter, u8 *da)
>  }
>
>  /* when wait_ack is ture, this function shoule be called at process context 
> */
> -static int _issue_qos_nulldata(struct adapter *padapter, unsigned char *da, 
> u16 tid, int wait_ack)
> +static int _issue_qos_nulldata(struct adapter *padapter, unsigned char *da,
> +u16 tid, bool wait_ack)
>  {
>   int ret = _FAIL;
>   struct xmit_frame   *pmgntframe;
> @@ -3715,7 +3719,8 @@ int issue_qos_nulldata(struct adapter *padapter, 
> unsigned char *da, u16 tid, int
>   return ret;
>  }
>
> -static int _issue_deauth(struct adapter *padapter, unsigned char *da, 
> unsigned short reason, u8 wait_ack)
> +static int _issue_deauth(struct adapter *padapter, unsigned char *da,
> +  unsigned short reason, bool wait_ack)
>  {
>   struct xmit_frame   *pmgntframe;
>   struct pkt_attrib   *pattrib;
> --
> 2.11.0
>
> --
> 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/20171009124918.omgogujj2titces2%40aishwarya.
> 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] staging: rtl8723bs: remove implicit int->bool conversions

2017-10-09 Thread Aishwarya Pant 
Implicit type conversions are bad; they hinder readability of code and have
potential to cause bugs. Here the variable wait_ack is always supplied a bool
value while in function declarations it is defined as an int type. Fix it by
defining wait_ack a bool type in all usages.

Signed-off-by: Aishwarya Pant 
---
 drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c 
b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
index 355ce9b19d1c..0dca5cdef2e7 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
@@ -2831,7 +2831,9 @@ void issue_probersp(struct adapter *padapter, unsigned 
char *da, u8 is_valid_p2p
 
 }
 
-static int _issue_probereq(struct adapter *padapter, struct ndis_802_11_ssid 
*pssid, u8 *da, u8 ch, bool append_wps, int wait_ack)
+static int _issue_probereq(struct adapter *padapter,
+  struct ndis_802_11_ssid *pssid,
+  u8 *da, u8 ch, bool append_wps, bool wait_ack)
 {
int ret = _FAIL;
struct xmit_frame   *pmgntframe;
@@ -3430,7 +3432,8 @@ void issue_assocreq(struct adapter *padapter)
 }
 
 /* when wait_ack is ture, this function shoule be called at process context */
-static int _issue_nulldata(struct adapter *padapter, unsigned char *da, 
unsigned int power_mode, int wait_ack)
+static int _issue_nulldata(struct adapter *padapter, unsigned char *da,
+  unsigned int power_mode, bool wait_ack)
 {
int ret = _FAIL;
struct xmit_frame   *pmgntframe;
@@ -3591,7 +3594,8 @@ s32 issue_nulldata_in_interrupt(struct adapter *padapter, 
u8 *da)
 }
 
 /* when wait_ack is ture, this function shoule be called at process context */
-static int _issue_qos_nulldata(struct adapter *padapter, unsigned char *da, 
u16 tid, int wait_ack)
+static int _issue_qos_nulldata(struct adapter *padapter, unsigned char *da,
+  u16 tid, bool wait_ack)
 {
int ret = _FAIL;
struct xmit_frame   *pmgntframe;
@@ -3715,7 +3719,8 @@ int issue_qos_nulldata(struct adapter *padapter, unsigned 
char *da, u16 tid, int
return ret;
 }
 
-static int _issue_deauth(struct adapter *padapter, unsigned char *da, unsigned 
short reason, u8 wait_ack)
+static int _issue_deauth(struct adapter *padapter, unsigned char *da,
+unsigned short reason, bool wait_ack)
 {
struct xmit_frame   *pmgntframe;
struct pkt_attrib   *pattrib;
-- 
2.11.0

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

Re: [PATCH] ANDROID: binder: call poll_wait() unconditionally.

2017-10-09 Thread Martijn Coenen 
On Mon, Oct 9, 2017 at 2:37 PM, Greg KH  wrote:
> Does this need to get into 4.14-final, or is 4.15-rc1 ok?  I'm a bit
> lost as to which patches I applied to what tree...

This fixes a race that is somewhat hard to hit, I've only ever seen it
with test code that creates the right conditions. But when it does hit
it's pretty bad (process blocks forever), so 4.14 would be my
preference. I just looked at v4.14-rc4 and it applied cleanly.

>
> thanks,
>
> greg "drowning in trees" k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] ANDROID: binder: call poll_wait() unconditionally.

2017-10-09 Thread Greg KH 
On Mon, Oct 09, 2017 at 02:26:56PM +0200, Martijn Coenen wrote:
> Because we're not guaranteed that subsequent calls
> to poll() will have a poll_table_struct parameter
> with _qproc set. When _qproc is not set, poll_wait()
> is a noop, and we won't be woken up correctly.
> 
> Signed-off-by: Martijn Coenen 
> ---
>  drivers/android/binder.c | 11 +--
>  1 file changed, 1 insertion(+), 10 deletions(-)

Does this need to get into 4.14-final, or is 4.15-rc1 ok?  I'm a bit
lost as to which patches I applied to what tree...

thanks,

greg "drowning in trees" k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] ANDROID: binder: call poll_wait() unconditionally.

2017-10-09 Thread Martijn Coenen 
Because we're not guaranteed that subsequent calls
to poll() will have a poll_table_struct parameter
with _qproc set. When _qproc is not set, poll_wait()
is a noop, and we won't be woken up correctly.

Signed-off-by: Martijn Coenen 
---
 drivers/android/binder.c | 11 +--
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index d055b3f2a207..99f77fec1aaa 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -3623,12 +3623,6 @@ static void binder_stat_br(struct binder_proc *proc,
}
 }
 
-static int binder_has_thread_work(struct binder_thread *thread)
-{
-   return !binder_worklist_empty(thread->proc, &thread->todo) ||
-   thread->looper_need_return;
-}
-
 static int binder_put_node_cmd(struct binder_proc *proc,
   struct binder_thread *thread,
   void __user **ptrp,
@@ -4258,12 +4252,9 @@ static unsigned int binder_poll(struct file *filp,
 
binder_inner_proc_unlock(thread->proc);
 
-   if (binder_has_work(thread, wait_for_proc_work))
-   return POLLIN;
-
poll_wait(filp, &thread->wait, wait);
 
-   if (binder_has_thread_work(thread))
+   if (binder_has_work(thread, wait_for_proc_work))
return POLLIN;
 
return 0;
-- 
2.14.2.920.gcf0c67979c-goog

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

Re: [PATCH v2 03/13] ANDROID: binder: add support for RT prio inheritance.

2017-10-09 Thread Greg KH 
On Mon, Oct 09, 2017 at 01:21:23PM +0200, Martijn Coenen wrote:
> On Fri, Sep 1, 2017 at 9:24 AM, Greg KH  wrote:
> >
> > I've now applied patches 1, 2, 7, 9, 11, and 12 from this series to my
> > tree, so feel free to rebase on it for the next round of these patches.
> 
> Thanks Greg. You should also be able to apply patch 10 from this
> series ("ANDROID: binder: call poll_wait() unconditionally.") as it is
> not related to the priority inheritance work. Let me know if you
> prefer me to send this one as a separate patch.

Please resend it, it is long gone from my patch queue.

thanks,

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

Re: [PATCH v2 03/13] ANDROID: binder: add support for RT prio inheritance.

2017-10-09 Thread Martijn Coenen 
On Fri, Sep 1, 2017 at 9:24 AM, Greg KH  wrote:
>
> I've now applied patches 1, 2, 7, 9, 11, and 12 from this series to my
> tree, so feel free to rebase on it for the next round of these patches.

Thanks Greg. You should also be able to apply patch 10 from this
series ("ANDROID: binder: call poll_wait() unconditionally.") as it is
not related to the priority inheritance work. Let me know if you
prefer me to send this one as a separate patch.

Thanks,
Martijn

> thanks,
>
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 04/15] staging: comedi: ni_routing: add ni routing tables

2017-10-09 Thread Ian Abbott 

On 08/10/17 07:50, Spencer E Olson wrote:

On Thu, 2016-11-10 at 11:27 -0700, Spencer E Olson wrote:

On Thu, 2016-11-10 at 18:18 +, Ian Abbott wrote:

On 10/11/16 17:54, Greg Kroah-Hartman wrote:

On Thu, Nov 10, 2016 at 05:17:22PM +, Ian Abbott wrote:

On 12/10/16 12:05, Spencer E. Olson wrote:

Adds tables of all register values for routing various signals to various
terminals on National Instruments hardware.  This information is directly
compared to and taken from register-level programming documentation and/or
register-level programming examples as provided by National Instruments.

Furthermore, this information was mostly compared (favorably) to the
register values already used in the comedi drivers for NI hardware.

Adds tables of valid routes for many devices.  This information is not
consistent from device to device, nor entirely consistent within device
families.  One additional major challenge is that this information does not
seem to be obtainable in any programmatic fashion, neither through the
proprietary NIDAQmx(-base) c-libraries, nor with register level
programming, _nor_ through any documentation.  In fact, the only consistent
source of this information is through the proprietary NI-MAX software,
which currently only runs on Windows platforms.  A further challenge is
that this information cannot be exported from NI-MAX, except by screenshot.

As described in ni_routing/README and as provided by this commit, the
device route information is primarily stored in a spreadsheet so-as to
enhance the ability to compare to screenshots obtained of NI-MAX.  This
commit provides the ability to parse the spreadsheets and generate
code following kernel conventions.

Signed-off-by: Spencer E. Olson 

*** PLEASE FIND ACTUAL PATCH AT:
http://www.umich.edu/~olsonse/patches/comedi-devglobal-v1/0004-staging-comedi-ni_routing-add-ni-routing-tables.patch

(This patch was over 500kB in size, too large for inline patch submission)
---
  .../staging/comedi/drivers/ni_routing/.gitignore   | 3 +
  drivers/staging/comedi/drivers/ni_routing/Makefile |40 +
  .../comedi/drivers/ni_routing/extract_tables.py|   259 +
  .../comedi/drivers/ni_routing/ni_device_routes.c   | 20251 +++
  .../comedi/drivers/ni_routing/ni_route_values.c|  2724 +++
  5 files changed, 23277 insertions(+)
  create mode 100644 drivers/staging/comedi/drivers/ni_routing/.gitignore
  create mode 100644 drivers/staging/comedi/drivers/ni_routing/Makefile
  create mode 100755 drivers/staging/comedi/drivers/ni_routing/extract_tables.py
  create mode 100644 
drivers/staging/comedi/drivers/ni_routing/ni_device_routes.c
  create mode 100644 drivers/staging/comedi/drivers/ni_routing/ni_route_values.c

<... SNIP ...>



The file heading comments in ni_device_routes.c and ni_route_values.c have
completely blank lines that need filling in.

I'm not sure if the fact that this patch cannot be emailed is a problem for
it to be accepted.  Since the problem is the number of lines, perhaps it can
be split?


Why not auto-generate the .c files from the csv files as part of the
kernel build process?  We do that today for other auto-generated files.


At the moment, the C files are generated by a Python script.  Is that
acceptable for a "normal" kernel build?



I'm not against just auto-generating the .c files.  I couldn't actually
identify something similar in the kernel, but if that already occurs...

I do think that keeping the .csv files is critical, since they will
facilitate maintenance and also addition of new hardware support the
easiest.

I'd rather not re-implement the script, but I supposed that is not out
of the question.  I was trying to capitalize on standard packages (only)
from python that let me implement the script is some succinct(ish)
fashion.



As I am going through to get this patchset ready for resubmission after
a long hiatus, I think I was expecting at least some type of response on
my last email above.  The issues are pertaining to generating .c files
perhaps as late as at compile time and also the means of this .c-file
generation.  I used Python for the reasons described above.  Is it
critical that these conversion scripts be re-implemented in Perl?


Python is used for building documentation and for various analysis 
tools, but is not currently a prerequisite for a "normal" kernel build. 
Perl has always been a prerequisite, but I don't know which optional 
Perl modules (from CPAN) are required.


--
-=( Ian Abbott @ MEV Ltd.E-mail:  )=-
-=(  Web: http://www.mev.co.uk/  )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 03/15] staging: comedi: ni_routing: Add NI signal routing info

2017-10-09 Thread Ian Abbott 

On 08/10/17 07:44, Spencer E Olson wrote:

On Thu, 2016-11-10 at 18:16 +, Ian Abbott wrote:

On 10/11/16 17:54, Greg Kroah-Hartman wrote:

On Thu, Nov 10, 2016 at 05:08:36PM +, Ian Abbott wrote:

On 12/10/16 12:05, Spencer E. Olson wrote:

See README for a thorough discussion of this content.

Adds two different collections of CSV files that:
1) summarize the various register values for creating routes
for a particular family of NI hardware devices;
2) summarize all possible (direct) routes that a particular device can
make--in this case, one file per device (this data is currently only
known to be found by examining a screenshot of the "Available Routes"
tab of NI MAX control panel, which is only found on Windows
installations of the NI driver).

The collection and maintenance of this information is somewhat tedious and
requires frequent re-examination and comparison of NI-MAX and/or the NI-MHDDK
documentation (register programming information) and NI-MHDDK examples.
These CSV files are constructed so-as to allow near direct comparison
to NI-MAX and NI-MHDDK.  As such, these serve to ease the task of
maintaining this knowledge and more quickly enables addition of new NI
devices.

Signed-off-by: Spencer E. Olson 

*** PLEASE FIND ACTUAL PATCH AT:
http://www.umich.edu/~olsonse/patches/comedi-devglobal-v1/0003-staging-comedi-ni_routing-Add-NI-signal-routing-info.patch

(This patch included some lines that were too long for email)
---
  drivers/staging/comedi/drivers/ni_routing/README   | 110 +
  .../ni_routing/ni_device_routes/PCI-6070E.csv  |  40 
  .../ni_routing/ni_device_routes/PCI-6220.csv   |  46 +
  .../ni_routing/ni_device_routes/PCI-6221.csv   |  50 ++
  .../ni_routing/ni_device_routes/PCI-6251.csv   |  51 ++
  .../ni_routing/ni_device_routes/PCI-6254.csv   |  47 +
  .../ni_routing/ni_device_routes/PCI-6259.csv   |  51 ++
  .../ni_routing/ni_device_routes/PCI-6534.csv   |  29 ++
  .../ni_routing/ni_device_routes/PCI-6602.csv   |  78 +++
  .../ni_routing/ni_device_routes/PCI-6713.csv   |  32 ++
  .../ni_routing/ni_device_routes/PCI-6723.csv   |  32 ++
  .../ni_routing/ni_device_routes/PCI-6733.csv   |  34 +++
  .../ni_routing/ni_device_routes/PXI-6030E.csv  |  39 
  .../ni_routing/ni_device_routes/PXI-6224.csv   |  46 +
  .../ni_routing/ni_device_routes/PXI-6225.csv   |  49 +
  .../ni_routing/ni_device_routes/PXI-6251.csv   |  50 ++
  .../ni_routing/ni_device_routes/PXI-6733.csv   |  35 +++
  .../ni_routing/ni_device_routes/PXIe-6251.csv  |  52 ++
  .../drivers/ni_routing/ni_route_values/ni_660x.csv | 100 +++
  .../ni_routing/ni_route_values/ni_eseries.csv  |  78 +++
  .../ni_routing/ni_route_values/ni_mseries.csv  |  90 +
  21 files changed, 1139 insertions(+)
  create mode 100644 drivers/staging/comedi/drivers/ni_routing/README
  create mode 100644 
drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6070E.csv
  create mode 100644 
drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6220.csv
  create mode 100644 
drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6221.csv
  create mode 100644 
drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6251.csv
  create mode 100644 
drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6254.csv
  create mode 100644 
drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6259.csv
  create mode 100644 
drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6534.csv
  create mode 100644 
drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6602.csv
  create mode 100644 
drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6713.csv
  create mode 100644 
drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6723.csv
  create mode 100644 
drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6733.csv
  create mode 100644 
drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PXI-6030E.csv
  create mode 100644 
drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PXI-6224.csv
  create mode 100644 
drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PXI-6225.csv
  create mode 100644 
drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PXI-6251.csv
  create mode 100644 
drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PXI-6733.csv
  create mode 100644 
drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PXIe-6251.csv
  create mode 100644 
drivers/staging/comedi/drivers/ni_routing/ni_route_values/ni_660x.csv
  create mode 100644 
drivers/staging/comedi/drivers/ni_routing/ni_route_values/ni_eseries.csv
  create mode 100644 
drivers/staging/comedi/drivers/ni_routing/ni_route_values/ni_mseries.csv

<... SNIP ...>



While it's nice to hav

Re: [PATCH v5 1/2] staging: ion: simplify ioctl args checking function

2017-10-09 Thread Benjamin Gaignard 
2017-09-27 15:20 GMT+02:00 Benjamin Gaignard :
> Make arguments checking more easy to read.
>

Hi Laura,

Even if we don't have found a solution for the second patch I believe
this one could be useful.
May I ask you your point of view on those few lines ?

Benjamin

> Signed-off-by: Benjamin Gaignard 
> ---
>  drivers/staging/android/ion/ion-ioctl.c | 11 +--
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/android/ion/ion-ioctl.c 
> b/drivers/staging/android/ion/ion-ioctl.c
> index d9f8b14..e26b786 100644
> --- a/drivers/staging/android/ion/ion-ioctl.c
> +++ b/drivers/staging/android/ion/ion-ioctl.c
> @@ -27,19 +27,18 @@ union ion_ioctl_arg {
>
>  static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)
>  {
> -   int ret = 0;
> -
> switch (cmd) {
> case ION_IOC_HEAP_QUERY:
> -   ret = arg->query.reserved0 != 0;
> -   ret |= arg->query.reserved1 != 0;
> -   ret |= arg->query.reserved2 != 0;
> +   if (arg->query.reserved0 ||
> +   arg->query.reserved1 ||
> +   arg->query.reserved2)
> +   return -EINVAL;
> break;
> default:
> break;
> }
>
> -   return ret ? -EINVAL : 0;
> +   return 0;
>  }
>
>  /* fix up the cases where the ioctl direction bits are incorrect */
> --
> 2.7.4
>



-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] staging: comedi: dt282x: fix IRQ assignment for dev->irq.

2017-10-09 Thread Ian Abbott 

On 06/10/17 19:25, Arvind Yadav wrote:

Here, dev->irq is not assigned with irq. comedi_legacy_detach()
is using dev->irq for release irq and dt282x_attach() is using dev->irq
for initialize comedi_subdevice.

Signed-off-by: Arvind Yadav 
---
changes in v2:
comedi_isadma_alloc() can fail. adding dev->irq assignment after
 successful return of comedi_isadma_alloc().

  drivers/staging/comedi/drivers/dt282x.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/drivers/staging/comedi/drivers/dt282x.c 
b/drivers/staging/comedi/drivers/dt282x.c
index d5295bb..217a4b8 100644
--- a/drivers/staging/comedi/drivers/dt282x.c
+++ b/drivers/staging/comedi/drivers/dt282x.c
@@ -1062,6 +1062,8 @@ static void dt282x_alloc_dma(struct comedi_device *dev,
   PAGE_SIZE, 0);
if (!devpriv->dma)
free_irq(irq_num, dev);
+   else
+   dev->irq = irq_num;
  }
  
  static void dt282x_free_dma(struct comedi_device *dev)




Good catch. Thanks!

Reviewed-by: Ian Abbott 

--
-=( Ian Abbott @ MEV Ltd.E-mail:  )=-
-=(  Web: http://www.mev.co.uk/  )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] vt6655: Fix a possible sleep-in-atomic bug in vt6655_suspend

2017-10-09 Thread Jia-Ju Bai 
The driver may sleep under a spinlock, and the function call path is:
vt6655_suspend (acquire the spinlock)
  pci_set_power_state
__pci_start_power_transition (drivers/pci/pci.c)
  msleep --> may sleep

To fix it, pci_set_power_state is called without having a spinlock.

This bug is found by my static analysis tool and my code review. 

Signed-off-by: Jia-Ju Bai 
---
 drivers/staging/vt6655/device_main.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/vt6655/device_main.c 
b/drivers/staging/vt6655/device_main.c
index 9fcf2e2..1123b4f 100644
--- a/drivers/staging/vt6655/device_main.c
+++ b/drivers/staging/vt6655/device_main.c
@@ -1693,10 +1693,11 @@ static int vt6655_suspend(struct pci_dev *pcid, 
pm_message_t state)
MACbShutdown(priv);
 
pci_disable_device(pcid);
-   pci_set_power_state(pcid, pci_choose_state(pcid, state));
 
spin_unlock_irqrestore(&priv->lock, flags);
 
+   pci_set_power_state(pcid, pci_choose_state(pcid, state));
+
return 0;
 }
 
-- 
1.7.9.5


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

Re: [PATCH] pci: Fix a possible sleep-in-atomic bug in pci_set_power_state

2017-10-09 Thread Jia-Ju Bai 

Oh, sorry, I will send the patches for each driver.


Thanks,
Jia-Ju Bai

On 2017/10/9 16:17, Greg KH wrote:

On Mon, Oct 09, 2017 at 04:16:20PM +0800, Jia-Ju Bai wrote:

The drivers vt6655 and gma500 call pci_set_power_state under a spinlock, which 
may sleep.
The function call paths are:
gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
   gma_resume_pci
 pci_set_power_state
   __pci_start_power_transition (drivers/pci/pci.c)
 msleep --> may sleep

gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
   gma_resume_pci
 pci_enable_device
   pci_enable_device_flags (drivers/pci/pci.c)
 do_pci_enable_device
   pci_set_power_state
 __pci_start_power_transition
   msleep --> may sleep

vt6655_suspend (acquire the spinlock) (drivers/staging/vt6655/device_main.c)
   pci_set_power_state
 __pci_start_power_transition (drivers/pci/pci.c)
   msleep --> may sleep

To fix these bugs, msleep is replaced with mdelay in 
__pci_start_power_transition

These bugs are found by my static analysis tool and my code review.

Wait, no, why not fix the callers to not have a spinlock.  Those are the
only users of these calls that are doing so incorrectly, don't change
the PCI core for the fault of 2 broken drivers.

thanks,

greg k-h



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

Re: [PATCH] pci: Fix a possible sleep-in-atomic bug in pci_set_power_state

2017-10-09 Thread Greg KH 
On Mon, Oct 09, 2017 at 04:16:20PM +0800, Jia-Ju Bai wrote:
> The drivers vt6655 and gma500 call pci_set_power_state under a spinlock, 
> which may sleep.
> The function call paths are:
> gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
>   gma_resume_pci
> pci_set_power_state
>   __pci_start_power_transition (drivers/pci/pci.c)
> msleep --> may sleep
> 
> gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
>   gma_resume_pci
> pci_enable_device
>   pci_enable_device_flags (drivers/pci/pci.c)
> do_pci_enable_device
>   pci_set_power_state
> __pci_start_power_transition
>   msleep --> may sleep
> 
> vt6655_suspend (acquire the spinlock) (drivers/staging/vt6655/device_main.c)
>   pci_set_power_state
> __pci_start_power_transition (drivers/pci/pci.c)
>   msleep --> may sleep
> 
> To fix these bugs, msleep is replaced with mdelay in 
> __pci_start_power_transition
> 
> These bugs are found by my static analysis tool and my code review.

Wait, no, why not fix the callers to not have a spinlock.  Those are the
only users of these calls that are doing so incorrectly, don't change
the PCI core for the fault of 2 broken drivers.

thanks,

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

[PATCH] pci: Fix a possible sleep-in-atomic bug in pci_set_power_state

2017-10-09 Thread Jia-Ju Bai 
The drivers vt6655 and gma500 call pci_set_power_state under a spinlock, which 
may sleep.
The function call paths are:
gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
  gma_resume_pci
pci_set_power_state
  __pci_start_power_transition (drivers/pci/pci.c)
msleep --> may sleep

gma_power_begin (acquire the spinlock) (drivers/gpu/drm/gma500/power.c)
  gma_resume_pci
pci_enable_device
  pci_enable_device_flags (drivers/pci/pci.c)
do_pci_enable_device
  pci_set_power_state
__pci_start_power_transition
  msleep --> may sleep

vt6655_suspend (acquire the spinlock) (drivers/staging/vt6655/device_main.c)
  pci_set_power_state
__pci_start_power_transition (drivers/pci/pci.c)
  msleep --> may sleep

To fix these bugs, msleep is replaced with mdelay in 
__pci_start_power_transition

These bugs are found by my static analysis tool and my code review.

Signed-off-by: Jia-Ju Bai 
---
 drivers/pci/pci.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 6078dfc..7b763a3 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -823,7 +823,7 @@ static void __pci_start_power_transition(struct pci_dev 
*dev, pci_power_t state)
 */
if (dev->runtime_d3cold) {
if (dev->d3cold_delay)
-   msleep(dev->d3cold_delay);
+   mdelay(dev->d3cold_delay);
/*
 * When powering on a bridge from D3cold, the
 * whole hierarchy may be powered on into
-- 
1.7.9.5


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

Re: [BUG] vt6655: A possible sleep-in-atomic bug in vt6655_suspend

2017-10-09 Thread Jia-Ju Bai 

Okay, I will send a patch :)


Thanks,
Jia-Ju Bai

On 2017/10/9 13:43, Greg KH wrote:

On Mon, Oct 09, 2017 at 09:10:28AM +0800, Jia-Ju Bai wrote:

According to device_main.c, the driver may sleep under a spinlock,
and the function call path is:
vt6655_suspend (acquire the spinlock)
   pci_set_power_state
 __pci_start_power_transition (drivers/pci/pci.c)
   msleep --> may sleep

A possible fix is to replace msleep with mdelay in
__pci_start_power_transition in drivers/pci/pci.c.

Patches are usually best to send in for things that you find like this.

thanks,

greg k-h



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