[PATCH -next] staging: rtlwifi: Remove set but not used variable 'ppsc'

2018-09-27 Thread YueHaibing
Fixes gcc '-Wunused-but-set-variable' warning:

drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c: In function 
'halbtc_leave_lps':
drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c:284:21: warning:
 variable 'ppsc' set but not used [-Wunused-but-set-variable]

drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c: In function 
'halbtc_enter_lps':
drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c:307:21: warning:
 variable 'ppsc' set but not used [-Wunused-but-set-variable]

Signed-off-by: YueHaibing 
---
 drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c 
b/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
index 85a7490..24e19ff 100644
--- a/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
+++ b/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
@@ -281,11 +281,9 @@ bool halbtc_send_bt_mp_operation(struct btc_coexist 
*btcoexist, u8 op_code,
 static void halbtc_leave_lps(struct btc_coexist *btcoexist)
 {
struct rtl_priv *rtlpriv;
-   struct rtl_ps_ctl *ppsc;
bool ap_enable = false;
 
rtlpriv = btcoexist->adapter;
-   ppsc = rtl_psc(rtlpriv);
 
btcoexist->btc_get(btcoexist, BTC_GET_BL_WIFI_AP_MODE_ENABLE,
   &ap_enable);
@@ -304,11 +302,9 @@ static void halbtc_leave_lps(struct btc_coexist *btcoexist)
 static void halbtc_enter_lps(struct btc_coexist *btcoexist)
 {
struct rtl_priv *rtlpriv;
-   struct rtl_ps_ctl *ppsc;
bool ap_enable = false;
 
rtlpriv = btcoexist->adapter;
-   ppsc = rtl_psc(rtlpriv);
 
btcoexist->btc_get(btcoexist, BTC_GET_BL_WIFI_AP_MODE_ENABLE,
   &ap_enable);

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


[PATCH] staging: bcm2835-camera: Avoid unneeded internal declaration warning

2018-09-27 Thread Nathan Chancellor
Clang warns:

drivers/staging/vc04_services/bcm2835-camera/controls.c:59:18: warning:
variable 'mains_freq_qmenu' is not needed and will not be emitted
[-Wunneeded-internal-declaration]
static const s64 mains_freq_qmenu[] = {
 ^
1 warning generated.

This is because mains_freq_qmenu is currently only used in an ARRAY_SIZE
macro, which is a compile time evaluation in this case. Avoid this by
adding mains_freq_qmenu as the imenu member of this structure, which
matches all other controls that uses the ARRAY_SIZE macro in v4l2_ctrls.
This turns out to be a no-op because V4L2_CID_MPEG_VIDEO_BITRATE_MODE is
defined as a MMAL_CONTROL_TYPE_STD_MENU, which does not pass the imenu
definition along to v4l2_ctrl_new in bm2835_mmal_init_controls.

Link: https://github.com/ClangBuiltLinux/linux/issues/122
Signed-off-by: Nathan Chancellor 
---
 drivers/staging/vc04_services/bcm2835-camera/controls.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c 
b/drivers/staging/vc04_services/bcm2835-camera/controls.c
index cff7b1e07153..a2c55cb2192a 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
@@ -1106,7 +1106,7 @@ static const struct bm2835_mmal_v4l2_ctrl 
v4l2_ctrls[V4L2_CTRL_COUNT] = {
{
V4L2_CID_POWER_LINE_FREQUENCY, MMAL_CONTROL_TYPE_STD_MENU,
0, ARRAY_SIZE(mains_freq_qmenu) - 1,
-   1, 1, NULL,
+   1, 1, mains_freq_qmenu,
MMAL_PARAMETER_FLICKER_AVOID,
&ctrl_set_flicker_avoidance,
false
-- 
2.19.0

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


[PATCH] staging: rtl8723bs: Skip unnecessary field checks

2018-09-27 Thread Aymen Qader
Skip unnecessary request field checks when the information element
pointer is null.

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

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c 
b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
index bf055935ef65..69c7abc0e3a5 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
@@ -1267,13 +1267,12 @@ unsigned int OnAssocReq(struct adapter *padapter, union 
recv_frame *precv_frame)
/*  checking SSID */
p = rtw_get_ie(pframe + WLAN_HDR_A3_LEN + ie_offset, _SSID_IE_, &ie_len,
pkt_len - WLAN_HDR_A3_LEN - ie_offset);
-   if (p == NULL) {
-   status = _STATS_FAILURE_;
-   }
 
-   if (ie_len == 0) /*  broadcast ssid, however it is not allowed in 
assocreq */
+   if (!p || ie_len == 0) {
+   /*  broadcast ssid, however it is not allowed in assocreq */
status = _STATS_FAILURE_;
-   else {
+   goto OnAssocReqFail;
+   } else {
/*  check if ssid match */
if (memcmp((void *)(p+2), cur->Ssid.Ssid, cur->Ssid.SsidLength))
status = _STATS_FAILURE_;
-- 
2.17.1

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


Re: [PATCH v2] staging: rtl8188eu: Skip unnecessary field checks

2018-09-27 Thread Larry Finger

On 9/27/18 4:19 PM, Aymen Qader wrote:

Skip unnecessary request field checks when the information element
pointer is null.

Signed-off-by: Aymen Qader 
---
v2: combine pointer check and length check & change commit message to be
more appropriate

  drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c 
b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
index 834053a0ae9d..4d55bbdf8fb7 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
@@ -2971,11 +2971,11 @@ static unsigned int OnAssocReq(struct adapter *padapter,
/*  checking SSID */
p = rtw_get_ie(pframe + WLAN_HDR_A3_LEN + ie_offset, _SSID_IE_, &ie_len,
pkt_len - WLAN_HDR_A3_LEN - ie_offset);
-   if (!p)
-   status = _STATS_FAILURE_;
  
-	if (ie_len == 0) { /*  broadcast ssid, however it is not allowed in assocreq */

+   if (!p || ie_len == 0) {
+   /*  broadcast ssid, however it is not allowed in assocreq */
status = _STATS_FAILURE_;
+   goto OnAssocReqFail;
} else {
/*  check if ssid match */
if (memcmp((void *)(p+2), cur->Ssid.Ssid, cur->Ssid.SsidLength))



ACKed-by: Larry Finger 


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


[PATCH v2] staging: rtl8188eu: Skip unnecessary field checks

2018-09-27 Thread Aymen Qader
Skip unnecessary request field checks when the information element
pointer is null.

Signed-off-by: Aymen Qader 
---
v2: combine pointer check and length check & change commit message to be
more appropriate

 drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c 
b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
index 834053a0ae9d..4d55bbdf8fb7 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
@@ -2971,11 +2971,11 @@ static unsigned int OnAssocReq(struct adapter *padapter,
/*  checking SSID */
p = rtw_get_ie(pframe + WLAN_HDR_A3_LEN + ie_offset, _SSID_IE_, &ie_len,
pkt_len - WLAN_HDR_A3_LEN - ie_offset);
-   if (!p)
-   status = _STATS_FAILURE_;
 
-   if (ie_len == 0) { /*  broadcast ssid, however it is not allowed in 
assocreq */
+   if (!p || ie_len == 0) {
+   /*  broadcast ssid, however it is not allowed in assocreq */
status = _STATS_FAILURE_;
+   goto OnAssocReqFail;
} else {
/*  check if ssid match */
if (memcmp((void *)(p+2), cur->Ssid.Ssid, cur->Ssid.SsidLength))
-- 
2.17.1

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


Re: [PATCH] staging: rtl8188eu: Avoid null pointer arithmetic

2018-09-27 Thread Aymen Qader
On Thu, Sep 27, 2018 at 03:52:53PM -0500, Larry Finger wrote:
> On 9/27/18 12:04 PM, Aymen Qader wrote:
> > Avoid null pointer arithmetic in rtw_mlme_ext.c by skipping other field
> > checks if the information element pointer is null.
> > 
> > Signed-off-by: Aymen Qader 
> > ---
> >   drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c 
> > b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> > index 834053a0ae9d..8a3a71456cd0 100644
> > --- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> > +++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> > @@ -2971,8 +2971,10 @@ static unsigned int OnAssocReq(struct adapter 
> > *padapter,
> > /*  checking SSID */
> > p = rtw_get_ie(pframe + WLAN_HDR_A3_LEN + ie_offset, _SSID_IE_, &ie_len,
> > pkt_len - WLAN_HDR_A3_LEN - ie_offset);
> > -   if (!p)
> > +   if (!p) {
> > status = _STATS_FAILURE_;
> > +   goto OnAssocReqFail;
> > +   }
> > if (ie_len == 0) { /*  broadcast ssid, however it is not allowed in 
> > assocreq */
> > status = _STATS_FAILURE_;
> 
> I do not think this patch avoids any pointer arithmetic. If p is NULL, then
> ie_len will be zero and the branch with the memcmp() call, where the pointer
> arithmetic is done, will be skipped.
I'm sincerely sorry, you're completely right--that was a bad oversight
from me, I should have checked more thoroughly.

> 
> That said, it is better to bail out with the first failure condition. I do
> not require the following, but the code would be even simpler if you test p
> and ie_len==0 in a single if statement and eliminate some code as in
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> index 1115050077e4..71722cec84a0 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> @@ -2982,11 +2982,10 @@ static unsigned int OnAssocReq(struct adapter 
> *padapter,
> /*  checking SSID */
> p = rtw_get_ie(pframe + WLAN_HDR_A3_LEN + ie_offset, _SSID_IE_, 
> &ie_len,
> pkt_len - WLAN_HDR_A3_LEN - ie_offset);
> -   if (!p)
> -   status = _STATS_FAILURE_;
> 
> -   if (ie_len == 0) { /*  broadcast ssid, however it is not allowed in
> assocreq */
> +   if (!p || ie_len == 0) { /*  broadcast ssid, however it is not
> allowed in assocreq */
> status = _STATS_FAILURE_;
> +   goto OnAssocReqFail;
> } else {
> /*  check if ssid match */
> if (memcmp((void *)(p+2), cur->Ssid.Ssid, 
> cur->Ssid.SsidLength))
> 

Yep, I understand that would be a lot better. If it's alright, I'll send
this in with a v2 (w/ a more appropriate commit message).

> 
> ACKed-by: Larry Finger 
> 
> Thanks,
> 
> Larry
> 

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


Re: [PATCH] staging: rtl8188eu: Avoid null pointer arithmetic

2018-09-27 Thread Larry Finger

On 9/27/18 12:04 PM, Aymen Qader wrote:

Avoid null pointer arithmetic in rtw_mlme_ext.c by skipping other field
checks if the information element pointer is null.

Signed-off-by: Aymen Qader 
---
  drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c 
b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
index 834053a0ae9d..8a3a71456cd0 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
@@ -2971,8 +2971,10 @@ static unsigned int OnAssocReq(struct adapter *padapter,
/*  checking SSID */
p = rtw_get_ie(pframe + WLAN_HDR_A3_LEN + ie_offset, _SSID_IE_, &ie_len,
pkt_len - WLAN_HDR_A3_LEN - ie_offset);
-   if (!p)
+   if (!p) {
status = _STATS_FAILURE_;
+   goto OnAssocReqFail;
+   }
  
  	if (ie_len == 0) { /*  broadcast ssid, however it is not allowed in assocreq */

status = _STATS_FAILURE_;


I do not think this patch avoids any pointer arithmetic. If p is NULL, then 
ie_len will be zero and the branch with the memcmp() call, where the pointer 
arithmetic is done, will be skipped.


That said, it is better to bail out with the first failure condition. I do not 
require the following, but the code would be even simpler if you test p and 
ie_len==0 in a single if statement and eliminate some code as in


diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c 
b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c

index 1115050077e4..71722cec84a0 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
@@ -2982,11 +2982,10 @@ static unsigned int OnAssocReq(struct adapter *padapter,
/*  checking SSID */
p = rtw_get_ie(pframe + WLAN_HDR_A3_LEN + ie_offset, _SSID_IE_, &ie_len,
pkt_len - WLAN_HDR_A3_LEN - ie_offset);
-   if (!p)
-   status = _STATS_FAILURE_;

-   if (ie_len == 0) { /*  broadcast ssid, however it is not allowed in 
assocreq */
+   if (!p || ie_len == 0) { /*  broadcast ssid, however it is not allowed 
in assocreq */

status = _STATS_FAILURE_;
+   goto OnAssocReqFail;
} else {
/*  check if ssid match */
if (memcmp((void *)(p+2), cur->Ssid.Ssid, cur->Ssid.SsidLength))


ACKed-by: Larry Finger 

Thanks,

Larry

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


[tip:x86/hyperv] x86/hyperv: Suppress "PCI: Fatal: No config space access function found"

2018-09-27 Thread tip-bot for Dexuan Cui
Commit-ID:  2f285f46240d67060061d153786740d4df53cd78
Gitweb: https://git.kernel.org/tip/2f285f46240d67060061d153786740d4df53cd78
Author: Dexuan Cui 
AuthorDate: Tue, 18 Sep 2018 22:29:50 +
Committer:  Thomas Gleixner 
CommitDate: Thu, 27 Sep 2018 21:19:14 +0200

x86/hyperv: Suppress "PCI: Fatal: No config space access function found"

A Generation-2 Linux VM on Hyper-V doesn't have the legacy PCI bus, and
users always see the scary warning, which is actually harmless.

Suppress it.

Signed-off-by: Dexuan Cui 
Signed-off-by: Thomas Gleixner 
Reviewed-by: Michael Kelley 
Cc: "H. Peter Anvin" 
Cc: KY Srinivasan 
Cc: Haiyang Zhang 
Cc: Stephen Hemminger 
Cc: "de...@linuxdriverproject.org" 
Cc: Olaf Aepfle 
Cc: Andy Whitcroft 
Cc: Jason Wang 
Cc: Vitaly Kuznetsov 
Cc: Marcelo Cerri 
Cc: Josh Poulson 
Link: https://lkml.kernel.org/r/ 

 #include 
 #include 
 #include 
@@ -253,6 +254,22 @@ static int hv_cpu_die(unsigned int cpu)
return 0;
 }
 
+static int __init hv_pci_init(void)
+{
+   int gen2vm = efi_enabled(EFI_BOOT);
+
+   /*
+* For Generation-2 VM, we exit from pci_arch_init() by returning 0.
+* The purpose is to suppress the harmless warning:
+* "PCI: Fatal: No config space access function found"
+*/
+   if (gen2vm)
+   return 0;
+
+   /* For Generation-1 VM, we'll proceed in pci_arch_init().  */
+   return 1;
+}
+
 /*
  * This function is to be invoked early in the boot sequence after the
  * hypervisor has been detected.
@@ -329,6 +346,8 @@ void __init hyperv_init(void)
 
hv_apic_init();
 
+   x86_init.pci.arch_init = hv_pci_init;
+
/*
 * Register Hyper-V specific clocksource.
 */
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[tip:x86/hyperv] x86/hyperv: Remove unused include

2018-09-27 Thread tip-bot for YueHaibing
Commit-ID:  5140a6f471137205687428b0b8f12f7187bffd18
Gitweb: https://git.kernel.org/tip/5140a6f471137205687428b0b8f12f7187bffd18
Author: YueHaibing 
AuthorDate: Sun, 23 Sep 2018 08:20:22 +
Committer:  Thomas Gleixner 
CommitDate: Thu, 27 Sep 2018 21:21:00 +0200

x86/hyperv: Remove unused include

Remove including . It's not needed.

Signed-off-by: YueHaibing 
Signed-off-by: Thomas Gleixner 
Reviewed-by: Michael Kelley 
Cc: "K. Y. Srinivasan" 
Cc: Haiyang Zhang 
Cc: Stephen Hemminger 
Cc: "H. Peter Anvin" 
Cc: 
Cc: 
Link: 
https://lkml.kernel.org/r/1537690822-97455-1-git-send-email-yuehaib...@huawei.com

---
 arch/x86/hyperv/hv_apic.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
index 2c43e3055948..8eb6fbee8e13 100644
--- a/arch/x86/hyperv/hv_apic.c
+++ b/arch/x86/hyperv/hv_apic.c
@@ -20,7 +20,6 @@
  */
 
 #include 
-#include 
 #include 
 #include 
 #include 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 05/13] staging: comedi: add interface to ni routing table information

2018-09-27 Thread Spencer E. Olson
Adds interface and associated unittests for accessing/looking-up/validating
the new ni routing table information.

Signed-off-by: Spencer E. Olson 
---

Changes since last submission:
  - [PATCH v2 05/13]: Tweak Makefile to build routing info for newly added
hardware in updates to [PATCH v2 04/13].
  - [PATCH v2 05/13]: Fixes placement of "select COMEDI_NI_ROUTING" as per Ian's
suggestion.
  - [PATCH v2 05/13]: Removes a few inline function declarations in unit test.

 drivers/staging/comedi/Kconfig|   4 +
 drivers/staging/comedi/drivers/Makefile   |  27 +
 drivers/staging/comedi/drivers/ni_routes.c| 521 +++
 drivers/staging/comedi/drivers/ni_routes.h| 329 ++
 drivers/staging/comedi/drivers/ni_stc.h   |   4 +
 drivers/staging/comedi/drivers/tests/Makefile |   3 +-
 .../comedi/drivers/tests/ni_routes_test.c | 613 ++
 7 files changed, 1500 insertions(+), 1 deletion(-)
 create mode 100644 drivers/staging/comedi/drivers/ni_routes.c
 create mode 100644 drivers/staging/comedi/drivers/ni_routes.h
 create mode 100644 drivers/staging/comedi/drivers/tests/ni_routes_test.c

diff --git a/drivers/staging/comedi/Kconfig b/drivers/staging/comedi/Kconfig
index 583bce9bb18e..9ab1ee7d36bf 100644
--- a/drivers/staging/comedi/Kconfig
+++ b/drivers/staging/comedi/Kconfig
@@ -1313,5 +1313,9 @@ config COMEDI_NI_LABPC_ISADMA
 
 config COMEDI_NI_TIO
tristate
+   select COMEDI_NI_ROUTING
+
+config COMEDI_NI_ROUTING
+   tristate
 
 endif # COMEDI
diff --git a/drivers/staging/comedi/drivers/Makefile 
b/drivers/staging/comedi/drivers/Makefile
index 8cb518190fc7..b24ac00cab73 100644
--- a/drivers/staging/comedi/drivers/Makefile
+++ b/drivers/staging/comedi/drivers/Makefile
@@ -137,6 +137,33 @@ obj-$(CONFIG_COMEDI_VMK80XX)   += vmk80xx.o
 obj-$(CONFIG_COMEDI_MITE)  += mite.o
 obj-$(CONFIG_COMEDI_NI_TIO)+= ni_tio.o
 obj-$(CONFIG_COMEDI_NI_TIOCMD) += ni_tiocmd.o
+obj-$(CONFIG_COMEDI_NI_ROUTING)+= ni_routing.o
+ni_routing-objs+= ni_routes.o \
+  ni_routing/ni_route_values.o \
+  ni_routing/ni_route_values/ni_660x.o 
\
+  
ni_routing/ni_route_values/ni_eseries.o \
+  
ni_routing/ni_route_values/ni_mseries.o \
+  ni_routing/ni_device_routes.o \
+  
ni_routing/ni_device_routes/pxi-6030e.o \
+  
ni_routing/ni_device_routes/pci-6070e.o \
+  
ni_routing/ni_device_routes/pci-6220.o \
+  
ni_routing/ni_device_routes/pci-6221.o \
+  
ni_routing/ni_device_routes/pxi-6224.o \
+  
ni_routing/ni_device_routes/pxi-6225.o \
+  
ni_routing/ni_device_routes/pci-6229.o \
+  
ni_routing/ni_device_routes/pci-6251.o \
+  
ni_routing/ni_device_routes/pxi-6251.o \
+  
ni_routing/ni_device_routes/pxie-6251.o \
+  
ni_routing/ni_device_routes/pci-6254.o \
+  
ni_routing/ni_device_routes/pci-6259.o \
+  
ni_routing/ni_device_routes/pci-6534.o \
+  
ni_routing/ni_device_routes/pxie-6535.o \
+  
ni_routing/ni_device_routes/pci-6602.o \
+  
ni_routing/ni_device_routes/pci-6713.o \
+  
ni_routing/ni_device_routes/pci-6723.o \
+  
ni_routing/ni_device_routes/pci-6733.o \
+  
ni_routing/ni_device_routes/pxi-6733.o \
+  
ni_routing/ni_device_routes/pxie-6738.o
 obj-$(CONFIG_COMEDI_NI_LABPC)  += ni_labpc_common.o
 obj-$(CONFIG_COMEDI_NI_LABPC_ISADMA)   += ni_labpc_isadma.o
 
diff --git a/drivers/staging/comedi/drivers/ni_routes.c 
b/drivers/staging/comedi/drivers/ni_routes.c
new file mode 100644
index ..9e999bc4f3c4
--- /dev/null
+++ b/drivers/staging/comedi/drivers/ni_routes.c
@@ -0,0 +1,521 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* vim: set ts=8 sw=8 noet tw=80 nowrap: */
+/*
+ *  comedi/drivers/ni_routes.c
+ *  Route information for NI boards.
+ *
+ *  COMEDI - Linux Control and Measurement Device Interface
+ *  Copyright (C) 2016 Spencer E. Olson 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License 

[PATCH v2 12/13] staging: comedi: ni_660x: clean up pfi routing

2018-09-27 Thread Spencer E. Olson
Cleans up the pfi routing code to make it easier to follow, read, and also
to prepare to use this cleaned up code for enabling the device-global
routing interface for ni_660x devices.

Signed-off-by: Spencer E. Olson 
---
 drivers/staging/comedi/drivers/ni_660x.c | 72 ++--
 1 file changed, 56 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_660x.c 
b/drivers/staging/comedi/drivers/ni_660x.c
index 0dfaf8ed093d..59055f366138 100644
--- a/drivers/staging/comedi/drivers/ni_660x.c
+++ b/drivers/staging/comedi/drivers/ni_660x.c
@@ -596,6 +596,37 @@ static void ni_660x_select_pfi_output(struct comedi_device 
*dev,
ni_660x_write(dev, active_chip, bits, NI660X_IO_CFG(chan));
 }
 
+static void ni_660x_set_pfi_direction(struct comedi_device *dev,
+ unsigned int chan,
+ unsigned int direction)
+{
+   struct ni_660x_private *devpriv = dev->private;
+   u64 bit;
+
+   bit = 1ULL << chan;
+
+   if (direction == COMEDI_OUTPUT) {
+   devpriv->io_dir |= bit;
+   /* reset the output to currently assigned output value */
+   ni_660x_select_pfi_output(dev, chan, devpriv->io_cfg[chan]);
+   } else {
+   devpriv->io_dir &= ~bit;
+   /* set pin to high-z; do not change currently assigned route */
+   ni_660x_select_pfi_output(dev, chan, 0);
+   }
+}
+
+static unsigned int ni_660x_get_pfi_direction(struct comedi_device *dev,
+ unsigned int chan)
+{
+   struct ni_660x_private *devpriv = dev->private;
+   u64 bit;
+
+   bit = 1ULL << chan;
+
+   return (devpriv->io_dir & bit) ? COMEDI_OUTPUT : COMEDI_INPUT;
+}
+
 static int ni_660x_set_pfi_routing(struct comedi_device *dev,
   unsigned int chan, unsigned int source)
 {
@@ -614,36 +645,48 @@ static int ni_660x_set_pfi_routing(struct comedi_device 
*dev,
}
 
devpriv->io_cfg[chan] = source;
-   if (devpriv->io_dir & (1ULL << chan))
+   if (ni_660x_get_pfi_direction(dev, chan) == COMEDI_OUTPUT)
ni_660x_select_pfi_output(dev, chan, devpriv->io_cfg[chan]);
return 0;
 }
 
+static int ni_660x_get_pfi_routing(struct comedi_device *dev, unsigned int 
chan)
+{
+   struct ni_660x_private *devpriv = dev->private;
+
+   return devpriv->io_cfg[chan];
+}
+
+static void ni_660x_set_pfi_filter(struct comedi_device *dev,
+  unsigned int chan, unsigned int value)
+{
+   unsigned int val;
+
+   val = ni_660x_read(dev, 0, NI660X_IO_CFG(chan));
+   val &= ~NI660X_IO_CFG_IN_SEL_MASK(chan);
+   val |= NI660X_IO_CFG_IN_SEL(chan, value);
+   ni_660x_write(dev, 0, val, NI660X_IO_CFG(chan));
+}
+
 static int ni_660x_dio_insn_config(struct comedi_device *dev,
   struct comedi_subdevice *s,
   struct comedi_insn *insn,
   unsigned int *data)
 {
-   struct ni_660x_private *devpriv = dev->private;
unsigned int chan = CR_CHAN(insn->chanspec);
-   u64 bit = 1ULL << chan;
-   unsigned int val;
int ret;
 
switch (data[0]) {
case INSN_CONFIG_DIO_OUTPUT:
-   devpriv->io_dir |= bit;
-   ni_660x_select_pfi_output(dev, chan, devpriv->io_cfg[chan]);
+   ni_660x_set_pfi_direction(dev, chan, COMEDI_OUTPUT);
break;
 
case INSN_CONFIG_DIO_INPUT:
-   devpriv->io_dir &= ~bit;
-   ni_660x_select_pfi_output(dev, chan, 0);/* high-z */
+   ni_660x_set_pfi_direction(dev, chan, COMEDI_INPUT);
break;
 
case INSN_CONFIG_DIO_QUERY:
-   data[1] = (devpriv->io_dir & bit) ? COMEDI_OUTPUT
- : COMEDI_INPUT;
+   data[1] = ni_660x_get_pfi_direction(dev, chan);
break;
 
case INSN_CONFIG_SET_ROUTING:
@@ -653,14 +696,11 @@ static int ni_660x_dio_insn_config(struct comedi_device 
*dev,
break;
 
case INSN_CONFIG_GET_ROUTING:
-   data[1] = devpriv->io_cfg[chan];
+   data[1] = ni_660x_get_pfi_routing(dev, chan);
break;
 
case INSN_CONFIG_FILTER:
-   val = ni_660x_read(dev, 0, NI660X_IO_CFG(chan));
-   val &= ~NI660X_IO_CFG_IN_SEL_MASK(chan);
-   val |= NI660X_IO_CFG_IN_SEL(chan, data[1]);
-   ni_660x_write(dev, 0, val, NI660X_IO_CFG(chan));
+   ni_660x_set_pfi_filter(dev, chan, data[1]);
break;
 
default:
@@ -840,7 +880,7 @@ static int ni_660x_auto_attach(struct comedi_device *dev,
  : NI_660X_PFI_OUTPUT_COUNTER;
 
ni_660x_set_pfi_routing(dev, i, source);
-  

[PATCH v2 02/13] staging: comedi: add abstracted NI signal/terminal named constants

2018-09-27 Thread Spencer E. Olson
This change adds abstracted constants for National Instruments
terminal/signal names.

Some background:
  There have been significant confusions over the past many years for users
  when trying to understand how to connect to/from signals and terminals on
  NI hardware using comedi.  The major reason for this is that the actual
  register values were exposed and required to be used by users.  Several
  major reasons exist why this caused major confusion for users:

  1) The register values are _NOT_ in user documentation, but rather in
arcane locations, such as a few register programming manuals that are
increasingly hard to find and the NI-MHDDK (comments in in example
code).  There is no one place to find the various valid values of the
registers.

  2) The register values are _NOT_ completely consistent.  There is no way
to gain any sense of intuition of which values, or even enums one
should use for various registers.  There was some attempt in prior use
of comedi to name enums such that a user might know which enums should
be used for varying purposes, but the end-user had to gain a knowledge
of register values to correctly wield this approach.

  3) The names for signals and registers found in the various register
level programming manuals and vendor-provided documentation are _not_
even close to the same names that are in the end-user documentation.

Similar confusion, albeit less, plagued NI's previous version of their own
proprietary drivers.  Earlier than 2003, NI greatly simplified the
situation for users by releasing a new API that abstracted the names of
signals/terminals to a common and intuitive set of names.  In addition,
this new API provided a much more common interface to use for most of NI
hardware.

The names added here mirror the names chosen and well documented by NI.
These names are exposed to the user via the comedilib user library.  By
keeping the names in this format, in spite of the use of CamelScript,
maintenance will be greatly eased and confusion for users _and_ comedi
developers will be greatly reduced.

Signed-off-by: Spencer E. Olson 
---

Changes since last submission:
  - [PATCH v2 02/13]: Update signal/terminal names found after adding additional
devices to routing list in [PATCH v2 04/13].

 drivers/staging/comedi/comedi.h | 151 
 1 file changed, 151 insertions(+)

diff --git a/drivers/staging/comedi/comedi.h b/drivers/staging/comedi/comedi.h
index bb961ac79b7e..6d7ad76bed97 100644
--- a/drivers/staging/comedi/comedi.h
+++ b/drivers/staging/comedi/comedi.h
@@ -928,6 +928,157 @@ enum i8254_mode {
I8254_BINARY = 0
 };
 
+/* *** BEGIN GLOBALLY-NAMED NI TERMINALS/SIGNALS *** */
+
+/*
+ * Common National Instruments Terminal/Signal names.
+ * Some of these have no NI_ prefix as they are useful for non-NI hardware, 
such
+ * as those that utilize the PXI/RTSI trigger lines.
+ *
+ * NOTE ABOUT THE CHOICE OF NAMES HERE AND THE CAMELSCRIPT:
+ *   The choice to use CamelScript and the exact names below is for
+ *   maintainability, clarity, similarity to manufacturer's documentation,
+ *   _and_ a mitigation for confusion that has plagued the use of these drivers
+ *   for years!
+ *
+ *   More detail:
+ *   There have been significant confusions over the past many years for users
+ *   when trying to understand how to connect to/from signals and terminals on
+ *   NI hardware using comedi.  The major reason for this is that the actual
+ *   register values were exposed and required to be used by users.  Several
+ *   major reasons exist why this caused major confusion for users:
+ *   1) The register values are _NOT_ in user documentation, but rather in
+ * arcane locations, such as a few register programming manuals that are
+ * increasingly hard to find and the NI MHDDK (comments in in example 
code).
+ * There is no one place to find the various valid values of the registers.
+ *   2) The register values are _NOT_ completely consistent.  There is no way 
to
+ * gain any sense of intuition of which values, or even enums one should 
use
+ * for various registers.  There was some attempt in prior use of comedi to
+ * name enums such that a user might know which enums should be used for
+ * varying purposes, but the end-user had to gain a knowledge of register
+ * values to correctly wield this approach.
+ *   3) The names for signals and registers found in the various register level
+ * programming manuals and vendor-provided documentation are _not_ even
+ * close to the same names that are in the end-user documentation.
+ *
+ *   Similar, albeit less, confusion plagued NI's previous version of their own
+ *   drivers.  Earlier than 2003, NI greatly simplified the situation for users
+ *   by releasing a new API that abstracted the names of signals/terminals to a
+ *   common and intuitive set of names.
+ *
+ *   The names below mirror the names chosen

[PATCH v2 09/13] staging: comedi: tio: implement global tio/ctr routing

2018-09-27 Thread Spencer E. Olson
Adds ability to use device-global names in command args, in particular
cmd->start_arg (for NI_CtrArmStartTrigger), and cmd->scan_begin_arg or
cmd->convert_arg (either is used to specify NI_CtrGate, with preference
given to cmd->scan_begin_arg, if it is set).

The actual arguments of cmd->start_arg are not fully checked against known
register values for the particular devices because these are not documented
or currently known.  This follows the precedence of prior versions of the
tio driver.  Should these become known, they should be annotated in the
route_values tables and the set of lines in ni_tio_cmdtest should be
uncommented to allow the tests to be made.

This patch also adds interface functions that allow routes for particular
counter route destinations to be made/queried/unmade.  This allows overseer
modules to implement test_route, connect_route, and disconnect_route.  As a
part of these changes, various functions were cleaned up and clarified.

These new interface functions allow direct writing/reading of register
values.  This is an example of exactly what the new device-global access
was intended to solve:  the old interface was not consistent with other
portions of the ni_* drivers--it did not allow full register values to be
given for various MUXes.  Instead, the old interface _did_ abstract away
some of the actual hardware from the underlying devices, but it was not
consistent with any other NI hardware.  Allowing the device-global
identifiers to be used, the new patch provides for consistency across all
ni_* drivers.  One final note:  these changes provide for backwards
compatibility by allowing the older values to still be used in through the
pre-existing kernel interfaces--though not in the new device-global
test/dis/connect/route interfaces.

Signed-off-by: Spencer E. Olson 
---
 drivers/staging/comedi/drivers/ni_660x.c  |  18 +-
 .../staging/comedi/drivers/ni_mio_common.c|   6 +-
 drivers/staging/comedi/drivers/ni_tio.c   | 457 ++
 drivers/staging/comedi/drivers/ni_tio.h   |  42 +-
 .../staging/comedi/drivers/ni_tio_internal.h  |   2 +
 drivers/staging/comedi/drivers/ni_tiocmd.c|  66 ++-
 6 files changed, 476 insertions(+), 115 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_660x.c 
b/drivers/staging/comedi/drivers/ni_660x.c
index e521ed9d0887..498b2868c957 100644
--- a/drivers/staging/comedi/drivers/ni_660x.c
+++ b/drivers/staging/comedi/drivers/ni_660x.c
@@ -31,6 +31,7 @@
 
 #include "mite.h"
 #include "ni_tio.h"
+#include "ni_routes.h"
 
 /* See Register-Level Programmer Manual page 3.1 */
 enum ni_660x_register {
@@ -259,6 +260,7 @@ struct ni_660x_private {
unsigned int dma_cfg[NI660X_MAX_CHIPS];
unsigned int io_cfg[NI660X_NUM_PFI_CHANNELS];
u64 io_dir;
+   struct ni_route_tables routing_tables;
 };
 
 static void ni_660x_write(struct comedi_device *dev, unsigned int chip,
@@ -730,12 +732,23 @@ static int ni_660x_auto_attach(struct comedi_device *dev,
 
ni_660x_init_tio_chips(dev, board->n_chips);
 
+   /* prepare the device for globally-named routes. */
+   if (ni_assign_device_routes("ni_660x", board->name,
+   &devpriv->routing_tables) < 0) {
+   dev_warn(dev->class_dev, "%s: %s device has no signal routing 
table.\n",
+__func__, board->name);
+   dev_warn(dev->class_dev, "%s: High level NI signal names will 
not be available for this %s board.\n",
+__func__, board->name);
+   }
+
n_counters = board->n_chips * NI660X_COUNTERS_PER_CHIP;
gpct_dev = ni_gpct_device_construct(dev,
ni_660x_gpct_write,
ni_660x_gpct_read,
ni_gpct_variant_660x,
-   n_counters);
+   n_counters,
+   NI660X_COUNTERS_PER_CHIP,
+   &devpriv->routing_tables);
if (!gpct_dev)
return -ENOMEM;
devpriv->counter_dev = gpct_dev;
@@ -831,9 +844,6 @@ static int ni_660x_auto_attach(struct comedi_device *dev,
if (i < n_counters) {
struct ni_gpct *counter = &gpct_dev->counters[i];
 
-   counter->chip_index = i / NI660X_COUNTERS_PER_CHIP;
-   counter->counter_index = i % NI660X_COUNTERS_PER_CHIP;
-
s->type = COMEDI_SUBD_COUNTER;
s->subdev_flags = SDF_READABLE | SDF_WRITABLE |
  SDF_LSAMPL | SDF_CMD_READ;
diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c 
b/drivers/staging/comedi/drivers/ni_mio_common.c
index d3290c28b1ce..d7862787e064 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/sta

[PATCH v2 00/13] device-global identifiers and routes introduced

2018-09-27 Thread Spencer E. Olson
This patch set is the second revision of a recent patch set of the same name.
Changes and notes:
  - [PATCH v2 02/13]: Update signal/terminal names found after adding additional
devices to routing list in [PATCH v2 04/13].
  - [PATCH v2 04/13]: Add routing information for PXIe-6535 and PXIe-6738
devices.
  - [PATCH v2 04/13]: Implements Ian's suggestion to break up components of new
ni_routing module into multiple compile units so that .c files are not
included from .c files.
  - [PATCH v2 04/13]: Fixes various function prototypes and "const" variable
declarations as per Ian's suggestions.
  - [PATCH v2 05/13]: Tweak Makefile to build routing info for newly added
hardware in updates to [PATCH v2 04/13].
  - [PATCH v2 05/13]: Fixes placement of "select COMEDI_NI_ROUTING" to ensure
ni_routing module is enabled for all dependent modules.
  - [PATCH v2 05/13]: Removes a few inline function declarations in unit test.
  - [PATCH v2 07/13]: This patch must be built upon an earlier patch recently
submitted and in the queue for integration:
"staging: comedi: ni_mio_common: protect register write overflow"

--

This patchset introduces a new framework for providing and maintaining a
consistent namespace to define terminal/signal names for a set of comedi
devices.  This effort was primarily focused on supporting NI hardware, but the
interfaces introduced here can be implemented by all other hardware drivers, if
desired.  Otherwise, these new interfaces do not effect any interfaces
previously defined or prior use cases (i.e. backwards compatibility).

Some background:
  There have been significant confusions over the past many years for users
  when trying to understand how to connect to/from signals and terminals on
  NI hardware using comedi.  The major reason for this is that the actual
  register values were exposed and required to be used by end users.  Several
  major reasons exist why this caused major confusion for users:

  1) The register values are _NOT_ in user documentation, but rather in
arcane locations, such as a few register programming manuals that are
increasingly hard to find.  Some information is found in the register level
programming libraries provided by National Instruments (NI-MHDDK), but
many items are only vaguely found/mentioned in the comments of the NI-MHDDK
example code.  There is no one place to find the various valid values of the
registers.

  2) The register values are _NOT_ completely consistent.  There is no way to
gain any sense of intuition of which values, or even enums one should use
for various registers.  There was some attempt in prior use of comedi to
name enums such that a user might know which enums should be used for
varying purposes, but the end-user had to gain a knowledge of register
values to correctly wield this approach.

  3) The names for signals and registers found in the various register level
programming manuals and vendor-provided documentation are _not_ even
close to the same names that are in the end-user documentation.

  4) The sets of routes that are valid are not consistent from device to device.
One additional major challenge is that this information is not documented
and does not seem to be obtainable in any programmatic fashion, neither
through the proprietary NIDAQmx(-base) c-libraries, nor with register level
programming.  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.

Similar confusion, albeit less, plagued NI's previous version of their own
proprietary drivers.  Earlier than 2003, NI greatly simplified the situation for
users by releasing a new API that abstracted the names of signals/terminals to a
common and intuitive set of names.  In addition, this new API provided a much
more common interface to use for most of NI hardware.

Comedi already provides such a common interface for data-acquisition and control
hardware.  This effort complements comedi's abstraction layers by further
abstracting much more of the use cases for NI hardware, but allowing users _and_
developers to directly refer to NI documentation (user-level, register-level,
and the register-level examples of the NI-MHDDK).

The goal of these patches are:
  0) Allow current code to function as is, providing backwards compatibility to
the current interface, following a suggestion by Eric Piel.
  1) Provide an interface to connect routes or identify signal sources and
destinations using a consistent naming scheme, global to a driver family.
  2) For NI devices, use terminal/signal naming that is consistent with (a) the
NI's user level documentation, (b) NI's user-level code, (c) the information
as provided by the proprietary NI-MAX software, and (d) the use

[PATCH v2 01/13] staging: comedi: tests: add unittest framework for comedi

2018-09-27 Thread Spencer E. Olson
Adds a framework for unittests for comedi drivers.  It was certainly
possible to write some unit tests before and test various aspects of a
particular driver, but this framework makes this a bit easier and hopefully
inspires more unittest modules to be written.

Signed-off-by: Spencer E. Olson 
---
 drivers/staging/comedi/drivers/Makefile   |  1 +
 drivers/staging/comedi/drivers/tests/Makefile |  6 ++
 .../comedi/drivers/tests/example_test.c   | 72 +++
 .../staging/comedi/drivers/tests/unittest.h   | 63 
 4 files changed, 142 insertions(+)
 create mode 100644 drivers/staging/comedi/drivers/tests/Makefile
 create mode 100644 drivers/staging/comedi/drivers/tests/example_test.c
 create mode 100644 drivers/staging/comedi/drivers/tests/unittest.h

diff --git a/drivers/staging/comedi/drivers/Makefile 
b/drivers/staging/comedi/drivers/Makefile
index 98b42b47dfe1..8cb518190fc7 100644
--- a/drivers/staging/comedi/drivers/Makefile
+++ b/drivers/staging/comedi/drivers/Makefile
@@ -145,3 +145,4 @@ obj-$(CONFIG_COMEDI_8255_SA)+= 8255.o
 obj-$(CONFIG_COMEDI_AMPLC_DIO200)  += amplc_dio200_common.o
 obj-$(CONFIG_COMEDI_AMPLC_PC236)   += amplc_pc236_common.o
 obj-$(CONFIG_COMEDI_DAS08) += das08.o
+obj-$(CONFIG_COMEDI_TESTS) += tests/
diff --git a/drivers/staging/comedi/drivers/tests/Makefile 
b/drivers/staging/comedi/drivers/tests/Makefile
new file mode 100644
index ..1d58ede0bdf6
--- /dev/null
+++ b/drivers/staging/comedi/drivers/tests/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+# Makefile for comedi drivers unit tests
+#
+ccflags-$(CONFIG_COMEDI_DEBUG) := -DDEBUG
+
+obj-$(CONFIG_COMEDI_TESTS) += example_test.o
diff --git a/drivers/staging/comedi/drivers/tests/example_test.c 
b/drivers/staging/comedi/drivers/tests/example_test.c
new file mode 100644
index ..fc65158b8e8e
--- /dev/null
+++ b/drivers/staging/comedi/drivers/tests/example_test.c
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* vim: set ts=8 sw=8 noet tw=80 nowrap: */
+/*
+ *  comedi/drivers/tests/example_test.c
+ *  Example set of unit tests.
+ *
+ *  COMEDI - Linux Control and Measurement Device Interface
+ *  Copyright (C) 2016 Spencer E. Olson 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ */
+
+#include 
+
+#include "unittest.h"
+
+/* *** BEGIN fake board data *** */
+struct comedi_device {
+   const char *board_name;
+   int item;
+};
+
+static struct comedi_device dev = {
+   .board_name = "fake_device",
+};
+
+/* *** END fake board data *** */
+
+/* *** BEGIN fake data init *** */
+void init_fake(void)
+{
+   dev.item = 10;
+}
+
+/* *** END fake data init *** */
+
+void test0(void)
+{
+   init_fake();
+   unittest(dev.item != 11, "negative result\n");
+   unittest(dev.item == 10, "positive result\n");
+}
+
+/*  BEGIN simple module entry/exit functions  */
+static int __init unittest_enter(void)
+{
+   const unittest_fptr unit_tests[] = {
+   (unittest_fptr)test0,
+   NULL,
+   };
+
+   exec_unittests("example", unit_tests);
+   return 0;
+}
+
+static void __exit unittest_exit(void) { }
+
+module_init(unittest_enter);
+module_exit(unittest_exit);
+
+MODULE_AUTHOR("Spencer Olson ");
+MODULE_DESCRIPTION("Comedi unit-tests example");
+MODULE_LICENSE("GPL");
+/*  END simple module entry/exit functions  */
diff --git a/drivers/staging/comedi/drivers/tests/unittest.h 
b/drivers/staging/comedi/drivers/tests/unittest.h
new file mode 100644
index ..b8e622ea1de1
--- /dev/null
+++ b/drivers/staging/comedi/drivers/tests/unittest.h
@@ -0,0 +1,63 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* vim: set ts=8 sw=8 noet tw=80 nowrap: */
+/*
+ *  comedi/drivers/tests/unittest.h
+ *  Simple framework for unittests for comedi drivers.
+ *
+ *  COMEDI - Linux Control and Measurement Device Interface
+ *  Copyright (C) 2016 Spencer E. Olson 
+ *  based of parts of drivers/of/unittest.c
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GN

[PATCH v2 13/13] staging: comedi: ni_660x: add device-global routing

2018-09-27 Thread Spencer E. Olson
Provides the device-global routing interface for ni_660x devices.  Using
the device-global names in comedi_cmd structures for commands was already
supported through the ni_tio module.

Signed-off-by: Spencer E. Olson 
---
 drivers/staging/comedi/drivers/ni_660x.c | 265 +++
 1 file changed, 265 insertions(+)

diff --git a/drivers/staging/comedi/drivers/ni_660x.c 
b/drivers/staging/comedi/drivers/ni_660x.c
index 59055f366138..e70a461e723f 100644
--- a/drivers/staging/comedi/drivers/ni_660x.c
+++ b/drivers/staging/comedi/drivers/ni_660x.c
@@ -568,6 +568,10 @@ static void ni_660x_select_pfi_output(struct comedi_device 
*dev,
unsigned int idle_chip = 0;
unsigned int bits;
 
+   if (chan >= NI_PFI(0))
+   /* allow new and old names of pfi channels to work. */
+   chan -= NI_PFI(0);
+
if (board->n_chips > 1) {
if (out_sel == NI_660X_PFI_OUTPUT_COUNTER &&
chan >= 8 && chan <= 23) {
@@ -603,6 +607,10 @@ static void ni_660x_set_pfi_direction(struct comedi_device 
*dev,
struct ni_660x_private *devpriv = dev->private;
u64 bit;
 
+   if (chan >= NI_PFI(0))
+   /* allow new and old names of pfi channels to work. */
+   chan -= NI_PFI(0);
+
bit = 1ULL << chan;
 
if (direction == COMEDI_OUTPUT) {
@@ -622,6 +630,10 @@ static unsigned int ni_660x_get_pfi_direction(struct 
comedi_device *dev,
struct ni_660x_private *devpriv = dev->private;
u64 bit;
 
+   if (chan >= NI_PFI(0))
+   /* allow new and old names of pfi channels to work. */
+   chan -= NI_PFI(0);
+
bit = 1ULL << chan;
 
return (devpriv->io_dir & bit) ? COMEDI_OUTPUT : COMEDI_INPUT;
@@ -632,6 +644,10 @@ static int ni_660x_set_pfi_routing(struct comedi_device 
*dev,
 {
struct ni_660x_private *devpriv = dev->private;
 
+   if (chan >= NI_PFI(0))
+   /* allow new and old names of pfi channels to work. */
+   chan -= NI_PFI(0);
+
switch (source) {
case NI_660X_PFI_OUTPUT_COUNTER:
if (chan < 8)
@@ -654,6 +670,10 @@ static int ni_660x_get_pfi_routing(struct comedi_device 
*dev, unsigned int chan)
 {
struct ni_660x_private *devpriv = dev->private;
 
+   if (chan >= NI_PFI(0))
+   /* allow new and old names of pfi channels to work. */
+   chan -= NI_PFI(0);
+
return devpriv->io_cfg[chan];
 }
 
@@ -662,6 +682,10 @@ static void ni_660x_set_pfi_filter(struct comedi_device 
*dev,
 {
unsigned int val;
 
+   if (chan >= NI_PFI(0))
+   /* allow new and old names of pfi channels to work. */
+   chan -= NI_PFI(0);
+
val = ni_660x_read(dev, 0, NI660X_IO_CFG(chan));
val &= ~NI660X_IO_CFG_IN_SEL_MASK(chan);
val |= NI660X_IO_CFG_IN_SEL(chan, value);
@@ -710,6 +734,240 @@ static int ni_660x_dio_insn_config(struct comedi_device 
*dev,
return insn->n;
 }
 
+static unsigned int _ni_get_valid_routes(struct comedi_device *dev,
+unsigned int n_pairs,
+unsigned int *pair_data)
+{
+   struct ni_660x_private *devpriv = dev->private;
+
+   return ni_get_valid_routes(&devpriv->routing_tables, n_pairs,
+  pair_data);
+}
+
+/*
+ * Retrieves the current source of the output selector for the given
+ * destination.  If the terminal for the destination is not already configured
+ * as an output, this function returns -EINVAL as error.
+ *
+ * Return: The register value of the destination output selector;
+ *-EINVAL if terminal is not configured for output.
+ */
+static inline int get_output_select_source(int dest, struct comedi_device *dev)
+{
+   struct ni_660x_private *devpriv = dev->private;
+   int reg = -1;
+
+   if (channel_is_pfi(dest)) {
+   if (ni_660x_get_pfi_direction(dev, dest) == COMEDI_OUTPUT)
+   reg = ni_660x_get_pfi_routing(dev, dest);
+   } else if (channel_is_rtsi(dest)) {
+   dev_dbg(dev->class_dev,
+   "%s: unhandled rtsi destination (%d) queried\n",
+   __func__, dest);
+   /*
+* The following can be enabled when RTSI routing info is
+* determined (not currently documented):
+* if (ni_get_rtsi_direction(dev, dest) == COMEDI_OUTPUT) {
+*  reg = ni_get_rtsi_routing(dev, dest);
+
+*  if (reg == NI_RTSI_OUTPUT_RGOUT0) {
+*  dest = NI_RGOUT0; ** prepare for lookup below **
+*  reg = get_rgout0_reg(dev);
+*  } else if (reg >= NI_RTSI_OUTPUT_RTSI_BRD(0) &&
+* reg <= NI_RTSI_OUTPUT_RTSI_BRD(3)) {
+*  const int i = reg - NI_RTSI_OUTPUT

[PATCH v2 08/13] staging: comedi: ni_mio_common: implement output selection of GPFO_{0, 1}

2018-09-27 Thread Spencer E. Olson
Implement the ability to route various signals to NI_CtrOut(x) pin.  This
pin is also known as GPFO_{0,1} in the DAQ STC.

Signed-off-by: Spencer E. Olson 
---
 .../staging/comedi/drivers/ni_mio_common.c| 106 ++
 drivers/staging/comedi/drivers/ni_stc.h   |   6 +-
 2 files changed, 111 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c 
b/drivers/staging/comedi/drivers/ni_mio_common.c
index c42d480df7ff..d3290c28b1ce 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -5508,6 +5508,77 @@ static void ni_rtsi_init(struct comedi_device *dev)
set_rgout0_reg(0, dev);
 }
 
+/* Get route of GPFO_i/CtrOut pins */
+static inline int ni_get_gout_routing(unsigned int dest,
+ struct comedi_device *dev)
+{
+   struct ni_private *devpriv = dev->private;
+   unsigned int reg = devpriv->an_trig_etc_reg;
+
+   switch (dest) {
+   case 0:
+   if (reg & NISTC_ATRIG_ETC_GPFO_0_ENA)
+   return NISTC_ATRIG_ETC_GPFO_0_SEL_TO_SRC(reg);
+   break;
+   case 1:
+   if (reg & NISTC_ATRIG_ETC_GPFO_1_ENA)
+   return NISTC_ATRIG_ETC_GPFO_1_SEL_TO_SRC(reg);
+   break;
+   }
+
+   return -EINVAL;
+}
+
+/* Set route of GPFO_i/CtrOut pins */
+static inline int ni_disable_gout_routing(unsigned int dest,
+ struct comedi_device *dev)
+{
+   struct ni_private *devpriv = dev->private;
+
+   switch (dest) {
+   case 0:
+   devpriv->an_trig_etc_reg &= ~NISTC_ATRIG_ETC_GPFO_0_ENA;
+   break;
+   case 1:
+   devpriv->an_trig_etc_reg &= ~NISTC_ATRIG_ETC_GPFO_1_ENA;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   ni_stc_writew(dev, devpriv->an_trig_etc_reg, NISTC_ATRIG_ETC_REG);
+   return 0;
+}
+
+/* Set route of GPFO_i/CtrOut pins */
+static inline int ni_set_gout_routing(unsigned int src, unsigned int dest,
+ struct comedi_device *dev)
+{
+   struct ni_private *devpriv = dev->private;
+
+   switch (dest) {
+   case 0:
+   /* clear reg */
+   devpriv->an_trig_etc_reg &= ~NISTC_ATRIG_ETC_GPFO_0_SEL(-1);
+   /* set reg */
+   devpriv->an_trig_etc_reg |= NISTC_ATRIG_ETC_GPFO_0_ENA
+|  NISTC_ATRIG_ETC_GPFO_0_SEL(src);
+   break;
+   case 1:
+   /* clear reg */
+   devpriv->an_trig_etc_reg &= ~NISTC_ATRIG_ETC_GPFO_1_SEL;
+   src = src ? NISTC_ATRIG_ETC_GPFO_1_SEL : 0;
+   /* set reg */
+   devpriv->an_trig_etc_reg |= NISTC_ATRIG_ETC_GPFO_1_ENA | src;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   ni_stc_writew(dev, devpriv->an_trig_etc_reg, NISTC_ATRIG_ETC_REG);
+   return 0;
+}
+
 /*
  * Retrieves the current source of the output selector for the given
  * destination.  If the terminal for the destination is not already configured
@@ -5539,6 +5610,16 @@ static int get_output_select_source(int dest, struct 
comedi_device *dev)
reg = get_ith_rtsi_brd_reg(i, dev);
}
}
+   } else if (dest >= NI_CtrOut(0) && dest <= NI_CtrOut(-1)) {
+   /*
+* not handled by ni_tio.  Only available for GPFO registers in
+* e/m series.
+*/
+   dest -= NI_CtrOut(0);
+   if (dest > 1)
+   /* there are only two g_out outputs. */
+   return -EINVAL;
+   reg = ni_get_gout_routing(dest, dev);
} else {
dev_dbg(dev->class_dev, "%s: unhandled destination (%d) 
queried\n",
__func__, dest);
@@ -5616,6 +5697,17 @@ static int connect_route(unsigned int src, unsigned int 
dest,
 
ni_set_rtsi_direction(dev, dest, COMEDI_OUTPUT);
ni_set_rtsi_routing(dev, dest, reg);
+   } else if (dest >= NI_CtrOut(0) && dest <= NI_CtrOut(-1)) {
+   /*
+* not handled by ni_tio.  Only available for GPFO registers in
+* e/m series.
+*/
+   dest -= NI_CtrOut(0);
+   if (dest > 1)
+   /* there are only two g_out outputs. */
+   return -EINVAL;
+   if (ni_set_gout_routing(src, dest, dev))
+   return -EINVAL;
} else {
return -EINVAL;
}
@@ -5664,6 +5756,16 @@ static int disconnect_route(unsigned int src, unsigned 
int dest,
reg = default_rtsi_routing[dest - TRIGGER_LINE(0)];
ni_set_rtsi_direction(dev, dest, COMEDI_INPUT);
  

[PATCH v2 10/13] staging: comedi: ni_mio_common: create device-global access to tio

2018-09-27 Thread Spencer E. Olson
Adds tio sub-devices of ni_mio_common supported hardware to the
implementation of test_route, connect_route, disconnect_route.  This change
delegates the actual functionality to the ni_tio module.

Signed-off-by: Spencer E. Olson 
---
 drivers/staging/comedi/drivers/ni_mio_common.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c 
b/drivers/staging/comedi/drivers/ni_mio_common.c
index d7862787e064..ecb05b3f9d35 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -5620,6 +5620,8 @@ static int get_output_select_source(int dest, struct 
comedi_device *dev)
/* there are only two g_out outputs. */
return -EINVAL;
reg = ni_get_gout_routing(dest, dev);
+   } else if (channel_is_ctr(dest)) {
+   reg = ni_tio_get_routing(devpriv->counter_dev, dest);
} else {
dev_dbg(dev->class_dev, "%s: unhandled destination (%d) 
queried\n",
__func__, dest);
@@ -5708,6 +5710,13 @@ static int connect_route(unsigned int src, unsigned int 
dest,
return -EINVAL;
if (ni_set_gout_routing(src, dest, dev))
return -EINVAL;
+   } else if (channel_is_ctr(dest)) {
+   /*
+* we are adding back the channel modifier info to set
+* invert/edge info passed by the user
+*/
+   ni_tio_set_routing(devpriv->counter_dev, dest,
+  reg | (src & ~CR_CHAN(-1)));
} else {
return -EINVAL;
}
@@ -5766,6 +5775,8 @@ static int disconnect_route(unsigned int src, unsigned 
int dest,
/* there are only two g_out outputs. */
return -EINVAL;
reg = ni_disable_gout_routing(dest, dev);
+   } else if (channel_is_ctr(dest)) {
+   ni_tio_unset_routing(devpriv->counter_dev, dest);
} else {
return -EINVAL;
}
-- 
2.17.1

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


[PATCH v2 03/13] staging: comedi: add new device-global config interface

2018-09-27 Thread Spencer E. Olson
Adds interface for configuring options that are global to all sub-devices.
For now, only options to configure device-globally identified signal routes
have been defined.

Signed-off-by: Spencer E. Olson 
---
 drivers/staging/comedi/comedi.h  | 18 
 drivers/staging/comedi/comedi_fops.c | 69 
 drivers/staging/comedi/comedidev.h   | 14 ++
 drivers/staging/comedi/drivers.c | 19 
 4 files changed, 120 insertions(+)

diff --git a/drivers/staging/comedi/comedi.h b/drivers/staging/comedi/comedi.h
index 6d7ad76bed97..a13c4b9cc569 100644
--- a/drivers/staging/comedi/comedi.h
+++ b/drivers/staging/comedi/comedi.h
@@ -107,6 +107,7 @@
 #define INSN_WRITE (1 | INSN_MASK_WRITE)
 #define INSN_BITS  (2 | INSN_MASK_READ | INSN_MASK_WRITE)
 #define INSN_CONFIG(3 | INSN_MASK_READ | INSN_MASK_WRITE)
+#define INSN_DEVICE_CONFIG (INSN_CONFIG | INSN_MASK_SPECIAL)
 #define INSN_GTOD  (4 | INSN_MASK_READ | INSN_MASK_SPECIAL)
 #define INSN_WAIT  (5 | INSN_MASK_WRITE | INSN_MASK_SPECIAL)
 #define INSN_INTTRIG   (6 | INSN_MASK_WRITE | INSN_MASK_SPECIAL)
@@ -347,6 +348,23 @@ enum configuration_ids {
INSN_CONFIG_PWM_GET_H_BRIDGE = 5004
 };
 
+/**
+ * enum device_configuration_ids - COMEDI configuration instruction codes 
global
+ * to an entire device.
+ * @INSN_DEVICE_CONFIG_TEST_ROUTE: Validate the possibility of a
+ * globally-named route
+ * @INSN_DEVICE_CONFIG_CONNECT_ROUTE:  Connect a globally-named route
+ * @INSN_DEVICE_CONFIG_DISCONNECT_ROUTE:Disconnect a globally-named route
+ * @INSN_DEVICE_CONFIG_GET_ROUTES: Get a list of all globally-named routes
+ * that are valid for a particular device.
+ */
+enum device_config_route_ids {
+   INSN_DEVICE_CONFIG_TEST_ROUTE = 0,
+   INSN_DEVICE_CONFIG_CONNECT_ROUTE = 1,
+   INSN_DEVICE_CONFIG_DISCONNECT_ROUTE = 2,
+   INSN_DEVICE_CONFIG_GET_ROUTES = 3,
+};
+
 /**
  * enum comedi_digital_trig_op - operations for configuring a digital trigger
  * @COMEDI_DIGITAL_TRIG_DISABLE:   Return digital trigger to its default,
diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index e18b61cdbdeb..5e7c5e71260f 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1230,6 +1230,57 @@ static int check_insn_config_length(struct comedi_insn 
*insn,
return -EINVAL;
 }
 
+static int check_insn_device_config_length(struct comedi_insn *insn,
+  unsigned int *data)
+{
+   if (insn->n < 1)
+   return -EINVAL;
+
+   switch (data[0]) {
+   case INSN_DEVICE_CONFIG_TEST_ROUTE:
+   case INSN_DEVICE_CONFIG_CONNECT_ROUTE:
+   case INSN_DEVICE_CONFIG_DISCONNECT_ROUTE:
+   if (insn->n == 3)
+   return 0;
+   break;
+   case INSN_DEVICE_CONFIG_GET_ROUTES:
+   /*
+* Big enough for config_id and the length of the userland
+* memory buffer.  Additional length should be in factors of 2
+* to communicate any returned route pairs (source,destination).
+*/
+   if (insn->n >= 2)
+   return 0;
+   break;
+   }
+   return -EINVAL;
+}
+
+/**
+ * get_valid_routes() - Calls low-level driver get_valid_routes function to
+ * either return a count of valid routes to user, or copy
+ * of list of all valid device routes to buffer in
+ * userspace.
+ * @dev: comedi device pointer
+ * @data: data from user insn call.  The length of the data must be >= 2.
+ *   data[0] must contain the INSN_DEVICE_CONFIG config_id.
+ *   data[1](input) contains the number of _pairs_ for which memory is
+ *   allotted from the user.  If the user specifies '0', then only
+ *   the number of pairs available is returned.
+ *   data[1](output) returns either the number of pairs available (if none
+ *   where requested) or the number of _pairs_ that are copied back
+ *   to the user.
+ *   data[2::2] returns each (source, destination) pair.
+ *
+ * Return: -EINVAL if low-level driver does not allocate and return routes as
+ *expected.  Returns 0 otherwise.
+ */
+static int get_valid_routes(struct comedi_device *dev, unsigned int *data)
+{
+   data[1] = dev->get_valid_routes(dev, data[1], data + 2);
+   return 0;
+}
+
 static int parse_insn(struct comedi_device *dev, struct comedi_insn *insn,
  unsigned int *data, void *file)
 {
@@ -1293,6 +1344,24 @@ static int parse_insn(struct comedi_device *dev, struct 
comedi_insn *insn,
if (ret >= 0)
ret = 1;
break;
+

[PATCH v2 07/13] staging: comedi: ni_mio_common: implement global pfi, rtsi routing

2018-09-27 Thread Spencer E. Olson
Implement device-global config interface for ni_mio devices.  In
particular, this patch implements:
INSN_DEVICE_CONFIG_TEST_ROUTE,
INSN_DEVICE_CONFIG_CONNECT_ROUTE,
INSN_DEVICE_CONFIG_DISCONNECT_ROUTE,
INSN_DEVICE_CONFIG_GET_ROUTES
for the ni mio devices.  This means that the new abstracted signal/terminal
names can be used to define signal routing with regards to the PFI
terminals and RTSI trigger bus lines.

This also adds ability to identify PFI and RTSI channels on the PFI and
RTSI subdevices using the new device-global names.  This does not change
the values that are set for channel output selections using the subdevice
interfaces--these still require direct register values.

Annotates and updates tables of register values to reflect this new
implementation status.

Signed-off-by: Spencer E. Olson 
---

Notes:
  - [PATCH 07/13]: This patch must be built upon an earlier patch recently
submitted and in the queue for integration:
"staging: comedi: ni_mio_common: protect register write overflow"

 .../staging/comedi/drivers/ni_mio_common.c| 687 --
 drivers/staging/comedi/drivers/ni_stc.h   |  68 ++
 2 files changed, 683 insertions(+), 72 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c 
b/drivers/staging/comedi/drivers/ni_mio_common.c
index b033f0be9b8a..c42d480df7ff 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -351,7 +351,8 @@ static const struct mio_regmap m_series_stc_write_regmap[] 
= {
[NISTC_AO_PERSONAL_REG] = { 0x19c, 2 },
[NISTC_RTSI_TRIGA_OUT_REG]  = { 0x19e, 2 },
[NISTC_RTSI_TRIGB_OUT_REG]  = { 0x1a0, 2 },
-   [NISTC_RTSI_BOARD_REG]  = { 0, 0 }, /* Unknown */
+   /* doc for following line: mhddk/nimseries/ChipObjects/tMSeries.h */
+   [NISTC_RTSI_BOARD_REG]  = { 0x1a2, 2 },
[NISTC_CFG_MEM_CLR_REG] = { 0x1a4, 2 },
[NISTC_ADC_FIFO_CLR_REG]= { 0x1a6, 2 },
[NISTC_DAC_FIFO_CLR_REG]= { 0x1a8, 2 },
@@ -4566,24 +4567,33 @@ static unsigned int ni_get_pfi_routing(struct 
comedi_device *dev,
 {
struct ni_private *devpriv = dev->private;
 
+   if (chan >= NI_PFI(0)) {
+   /* allow new and old names of pfi channels to work. */
+   chan -= NI_PFI(0);
+   }
return (devpriv->is_m_series)
? ni_m_series_get_pfi_routing(dev, chan)
: ni_old_get_pfi_routing(dev, chan);
 }
 
+/* Sets the output mux for the specified PFI channel. */
 static int ni_set_pfi_routing(struct comedi_device *dev,
  unsigned int chan, unsigned int source)
 {
struct ni_private *devpriv = dev->private;
 
+   if (chan >= NI_PFI(0)) {
+   /* allow new and old names of pfi channels to work. */
+   chan -= NI_PFI(0);
+   }
return (devpriv->is_m_series)
? ni_m_series_set_pfi_routing(dev, chan, source)
: ni_old_set_pfi_routing(dev, chan, source);
 }
 
-static int ni_config_filter(struct comedi_device *dev,
-   unsigned int pfi_channel,
-   enum ni_pfi_filter_select filter)
+static int ni_config_pfi_filter(struct comedi_device *dev,
+   unsigned int chan,
+   enum ni_pfi_filter_select filter)
 {
struct ni_private *devpriv = dev->private;
unsigned int bits;
@@ -4591,19 +4601,46 @@ static int ni_config_filter(struct comedi_device *dev,
if (!devpriv->is_m_series)
return -ENOTSUPP;
 
+   if (chan >= NI_PFI(0)) {
+   /* allow new and old names of pfi channels to work. */
+   chan -= NI_PFI(0);
+   }
+
bits = ni_readl(dev, NI_M_PFI_FILTER_REG);
-   bits &= ~NI_M_PFI_FILTER_SEL_MASK(pfi_channel);
-   bits |= NI_M_PFI_FILTER_SEL(pfi_channel, filter);
+   bits &= ~NI_M_PFI_FILTER_SEL_MASK(chan);
+   bits |= NI_M_PFI_FILTER_SEL(chan, filter);
ni_writel(dev, bits, NI_M_PFI_FILTER_REG);
return 0;
 }
 
+static void ni_set_pfi_direction(struct comedi_device *dev, int chan,
+unsigned int direction)
+{
+   if (chan >= NI_PFI(0)) {
+   /* allow new and old names of pfi channels to work. */
+   chan -= NI_PFI(0);
+   }
+   direction = (direction == COMEDI_OUTPUT) ? 1u : 0u;
+   ni_set_bits(dev, NISTC_IO_BIDIR_PIN_REG, 1 << chan, direction);
+}
+
+static int ni_get_pfi_direction(struct comedi_device *dev, int chan)
+{
+   struct ni_private *devpriv = dev->private;
+
+   if (chan >= NI_PFI(0)) {
+   /* allow new and old names of pfi channels to work. */
+   chan -= NI_PFI(0);
+   }
+   return devpriv->io_bidirection_pin_reg & (1 << chan) ?
+  COMEDI_OUTPUT : COMEDI_INPUT;
+}
+
 stat

[PATCH v2 11/13] staging: comedi: ni_660x: Add NI PCI-6608 to list of supported devices

2018-09-27 Thread Spencer E. Olson
Previously, only the PXI version of the NI-6608 board was supported.  This
change adds support for the PCI version as well.

Signed-off-by: Spencer E. Olson 
---
 drivers/staging/comedi/drivers/ni_660x.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers/ni_660x.c 
b/drivers/staging/comedi/drivers/ni_660x.c
index 498b2868c957..0dfaf8ed093d 100644
--- a/drivers/staging/comedi/drivers/ni_660x.c
+++ b/drivers/staging/comedi/drivers/ni_660x.c
@@ -7,7 +7,7 @@
  * Driver: ni_660x
  * Description: National Instruments 660x counter/timer boards
  * Devices: [National Instruments] PCI-6601 (ni_660x), PCI-6602, PXI-6602,
- *   PXI-6608, PCI-6624, PXI-6624
+ *   PCI-6608, PXI-6608, PCI-6624, PXI-6624
  * Author: J.P. Mellor ,
  *   herman.bruynin...@mech.kuleuven.ac.be,
  *   wim.meeus...@mech.kuleuven.ac.be,
@@ -202,6 +202,7 @@ enum ni_660x_boardid {
BOARD_PCI6601,
BOARD_PCI6602,
BOARD_PXI6602,
+   BOARD_PCI6608,
BOARD_PXI6608,
BOARD_PCI6624,
BOARD_PXI6624
@@ -225,6 +226,10 @@ static const struct ni_660x_board ni_660x_boards[] = {
.name   = "PXI-6602",
.n_chips= 2,
},
+   [BOARD_PCI6608] = {
+   .name   = "PCI-6608",
+   .n_chips= 2,
+   },
[BOARD_PXI6608] = {
.name   = "PXI-6608",
.n_chips= 2,
@@ -925,6 +930,7 @@ static const struct pci_device_id ni_660x_pci_table[] = {
{ PCI_VDEVICE(NI, 0x1310), BOARD_PCI6602 },
{ PCI_VDEVICE(NI, 0x1360), BOARD_PXI6602 },
{ PCI_VDEVICE(NI, 0x2c60), BOARD_PCI6601 },
+   { PCI_VDEVICE(NI, 0x2db0), BOARD_PCI6608 },
{ PCI_VDEVICE(NI, 0x2cc0), BOARD_PXI6608 },
{ PCI_VDEVICE(NI, 0x1e30), BOARD_PCI6624 },
{ PCI_VDEVICE(NI, 0x1e40), BOARD_PXI6624 },
-- 
2.17.1

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


[PATCH v2 06/13] staging: comedi: ni_mio_common: implement new routing for TRIG_EXT

2018-09-27 Thread Spencer E. Olson
Use new signal routing capability for all comedi command *_src == TRIG_EXT
options.  This new interface allows the user specify signals and terminals
as TRIG_EXT sources using a very consistent naming convention. Furthermore,
the interface allows backwards compatibility to prior behavior of
specifying register-level (or near register-level) values as *_arg options
when *_src == TRIG_EXT.

Annotates and updates tables of register values to reflect this new
implementation status.

Signed-off-by: Spencer E. Olson 
---
 .../staging/comedi/drivers/ni_mio_common.c| 106 +++---
 1 file changed, 66 insertions(+), 40 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c 
b/drivers/staging/comedi/drivers/ni_mio_common.c
index 4d0d0621780e..b033f0be9b8a 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -2006,7 +2006,6 @@ static int ni_ai_cmdtest(struct comedi_device *dev, 
struct comedi_subdevice *s,
const struct ni_board_struct *board = dev->board_ptr;
struct ni_private *devpriv = dev->private;
int err = 0;
-   unsigned int tmp;
unsigned int sources;
 
/* Step 1 : check if triggers are trivially valid */
@@ -2047,12 +2046,9 @@ static int ni_ai_cmdtest(struct comedi_device *dev, 
struct comedi_subdevice *s,
err |= comedi_check_trigger_arg_is(&cmd->start_arg, 0);
break;
case TRIG_EXT:
-   tmp = CR_CHAN(cmd->start_arg);
-
-   if (tmp > 16)
-   tmp = 16;
-   tmp |= (cmd->start_arg & (CR_INVERT | CR_EDGE));
-   err |= comedi_check_trigger_arg_is(&cmd->start_arg, tmp);
+   err |= ni_check_trigger_arg_roffs(CR_CHAN(cmd->start_arg),
+ NI_AI_StartTrigger,
+ &devpriv->routing_tables, 1);
break;
}
 
@@ -2064,12 +2060,9 @@ static int ni_ai_cmdtest(struct comedi_device *dev, 
struct comedi_subdevice *s,
0xff);
} else if (cmd->scan_begin_src == TRIG_EXT) {
/* external trigger */
-   unsigned int tmp = CR_CHAN(cmd->scan_begin_arg);
-
-   if (tmp > 16)
-   tmp = 16;
-   tmp |= (cmd->scan_begin_arg & (CR_INVERT | CR_EDGE));
-   err |= comedi_check_trigger_arg_is(&cmd->scan_begin_arg, tmp);
+   err |= ni_check_trigger_arg_roffs(CR_CHAN(cmd->scan_begin_arg),
+ NI_AI_SampleClock,
+ &devpriv->routing_tables, 1);
} else {/* TRIG_OTHER */
err |= comedi_check_trigger_arg_is(&cmd->scan_begin_arg, 0);
}
@@ -2087,12 +2080,9 @@ static int ni_ai_cmdtest(struct comedi_device *dev, 
struct comedi_subdevice *s,
}
} else if (cmd->convert_src == TRIG_EXT) {
/* external trigger */
-   unsigned int tmp = CR_CHAN(cmd->convert_arg);
-
-   if (tmp > 16)
-   tmp = 16;
-   tmp |= (cmd->convert_arg & (CR_ALT_FILTER | CR_INVERT));
-   err |= comedi_check_trigger_arg_is(&cmd->convert_arg, tmp);
+   err |= ni_check_trigger_arg_roffs(CR_CHAN(cmd->convert_arg),
+ NI_AI_ConvertClock,
+ &devpriv->routing_tables, 1);
} else if (cmd->convert_src == TRIG_NOW) {
err |= comedi_check_trigger_arg_is(&cmd->convert_arg, 0);
}
@@ -2118,7 +2108,7 @@ static int ni_ai_cmdtest(struct comedi_device *dev, 
struct comedi_subdevice *s,
/* step 4: fix up any arguments */
 
if (cmd->scan_begin_src == TRIG_TIMER) {
-   tmp = cmd->scan_begin_arg;
+   unsigned int tmp = cmd->scan_begin_arg;
cmd->scan_begin_arg =
ni_timer_to_ns(dev, ni_ns_to_timer(dev,
   cmd->scan_begin_arg,
@@ -2128,7 +2118,7 @@ static int ni_ai_cmdtest(struct comedi_device *dev, 
struct comedi_subdevice *s,
}
if (cmd->convert_src == TRIG_TIMER) {
if (!devpriv->is_611x && !devpriv->is_6143) {
-   tmp = cmd->convert_arg;
+   unsigned int tmp = cmd->convert_arg;
cmd->convert_arg =
ni_timer_to_ns(dev, ni_ns_to_timer(dev,
   cmd->convert_arg,
@@ -2206,8 +2196,10 @@ static int ni_ai_cmd(struct comedi_device *dev, struct 
comedi_subdevice *s)
   NISTC_AI_TRIG_START1_SEL(0);
break;
case TRIG_EXT:
-   ai_trig |= NISTC_AI_TRIG_START1_SEL(CR_CHA

Re: [PATCH v3] staging: ks7010: Add null pointer check for skb

2018-09-27 Thread Aymen Qader
Retraction: in hindsight I see that with the current usage of this
function, there is already a check for the socket buffer so this check
is unnecessary. However, I'm not sure if it's considered good practice
to keep this check anyway--in any case, ENOMEM isn't the right error
to return.

On Thu, Sep 27, 2018 at 04:16:13PM +0100, Aymen Qader wrote:
> Add a null pointer check for the socket buffer in ks_hostif.c to avoid a
> possible null pointer deference, and remove a later now-redundant null
> pointer check.
> 
> Signed-off-by: Aymen Qader 
> ---
> v2: Remove redundant pointer check
> v3: Style fix
> 
>  drivers/staging/ks7010/ks_hostif.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/ks7010/ks_hostif.c 
> b/drivers/staging/ks7010/ks_hostif.c
> index 0e554e3359b5..95b6c7557e84 100644
> --- a/drivers/staging/ks7010/ks_hostif.c
> +++ b/drivers/staging/ks7010/ks_hostif.c
> @@ -1011,6 +1011,11 @@ int hostif_data_request(struct ks_wlan_private *priv, 
> struct sk_buff *skb)
>   size_t size;
>   int ret;
>  
> + if (!skb) {
> + ret = -ENOMEM;
> + goto err_kfree;
> + }
> +
>   skb_len = skb->len;
>   if (skb_len > ETH_FRAME_LEN) {
>   netdev_err(priv->net_dev, "bad length skb_len=%d\n", skb_len);
> @@ -1023,7 +1028,6 @@ int hostif_data_request(struct ks_wlan_private *priv, 
> struct sk_buff *skb)
>   priv->wpa.mic_failure.stop) {
>   if (netif_queue_stopped(priv->net_dev))
>   netif_wake_queue(priv->net_dev);
> - if (skb)
>   dev_kfree_skb(skb);
>  
>   return 0;
> -- 
> 2.17.1
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: rtl8188eu: Avoid null pointer arithmetic

2018-09-27 Thread Aymen Qader
Avoid null pointer arithmetic in rtw_mlme_ext.c by skipping other field
checks if the information element pointer is null.

Signed-off-by: Aymen Qader 
---
 drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c 
b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
index 834053a0ae9d..8a3a71456cd0 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
@@ -2971,8 +2971,10 @@ static unsigned int OnAssocReq(struct adapter *padapter,
/*  checking SSID */
p = rtw_get_ie(pframe + WLAN_HDR_A3_LEN + ie_offset, _SSID_IE_, &ie_len,
pkt_len - WLAN_HDR_A3_LEN - ie_offset);
-   if (!p)
+   if (!p) {
status = _STATS_FAILURE_;
+   goto OnAssocReqFail;
+   }
 
if (ie_len == 0) { /*  broadcast ssid, however it is not allowed in 
assocreq */
status = _STATS_FAILURE_;
-- 
2.17.1

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


Re: [PATCH] fix error handling in drivers/staging/rtl8192u/ieee80211/ieee80211_module.c

2018-09-27 Thread Kees Cook
On Thu, Sep 27, 2018 at 7:24 AM, Dan Carpenter  wrote:
> On Wed, Sep 26, 2018 at 01:52:17PM -0400, valdis.kletni...@vt.edu wrote:
>> John notes that if the kzalloc of ieee->pHTInfo fails, we fail to call
>> ieee80211_networks_free().  In addition, that function has an un-needed check
>> before kfree().
>>
>> Reported-by: John Whitmore 
>> Signed-off-by: Valdis Kletnieks 
>> ---
>> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c 
>> b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
>> index 90a097f2cd4e..97ff0371b5bb 100644
>> --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
>> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
>> @@ -78,8 +78,6 @@ static inline int ieee80211_networks_allocate(struct 
>> ieee80211_device *ieee)
>>
>>  static inline void ieee80211_networks_free(struct ieee80211_device *ieee)
>>  {
>> - if (!ieee->networks)
>> - return;
>>   kfree(ieee->networks);
>>   ieee->networks = NULL;
>>  }
>> @@ -180,6 +178,7 @@ struct net_device *alloc_ieee80211(int sizeof_priv)
>>   return dev;
>>
>>   failed:
>> + ieee80211_networks_free(ieee);
>>   if (dev)
>>   free_netdev(dev);
>
> When there is a "goto failed;" then it's called "one err style" error
> handling and we're just asking for bugs...  In this case the bug is that
> we're not allowd to call ieee80211_networks_free() with a NULL
> "ieee" parameter.  The right thing to do is to only call
> ieee80211_networks_free() if we know that ieee80211_networks_allocate()
> succeeded.

Ah yeah, thanks for the extra eyes. Looks like it would need something
more like this:

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
index 90a097f2cd4e..8d6a28af96dc 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
@@ -78,7 +78,7 @@ static inline int ieee80211_networks_allocate(struct
ieee80211_device *ieee)

 static inline void ieee80211_networks_free(struct ieee80211_device *ieee)
 {
-   if (!ieee->networks)
+   if (!ieee)
return;
kfree(ieee->networks);
ieee->networks = NULL;
@@ -97,7 +97,7 @@ static inline void
ieee80211_networks_initialize(struct ieee80211_device *ieee)

 struct net_device *alloc_ieee80211(int sizeof_priv)
 {
-   struct ieee80211_device *ieee;
+   struct ieee80211_device *ieee = NULL;
struct net_device *dev;
int i, err;

@@ -180,6 +180,7 @@ struct net_device *alloc_ieee80211(int sizeof_priv)
return dev;

  failed:
+   ieee80211_networks_free(ieee);
if (dev)
free_netdev(dev);


Valdis, can you respin the patch?

-Kees

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


[PATCH v3] staging: mt7621-mmc: Remove #if 0 blocks and fix macros in sd.c

2018-09-27 Thread Nishad Kamdar
This patch removes #if 0 code blocks and usages of the
functions defined in the #if 0 code block. It removes
the macro msdc_irq_restore() and replaces its usage
with call to the function called in the macro definition.
Issue found by checkpatch.

Signed-off-by: Nishad Kamdar 
---
Changes in v3:
  - Remove the #if 0 code blocks and usages of the functions
defined in the #if 0 code block.
  - Delete msdc_irq_restore() and replace its usage directly
with call to the function being called by the macro.
Changes in v2:
  - Convert msdc_gate_clk() and msdc_ungate_clk() to inline functions.
  - Delete msdc_irq_restore(), msdc_vcore_on(), msdc_vcore_off(),
msdc_vdd_on() and msdc_vdd_off() and replace their usages directly
with calls to the function being called by these macros.
---
 drivers/staging/mt7621-mmc/sd.c | 306 +---
 1 file changed, 2 insertions(+), 304 deletions(-)

diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
index e3c1546373ba..dbf4dcb28b3c 100644
--- a/drivers/staging/mt7621-mmc/sd.c
+++ b/drivers/staging/mt7621-mmc/sd.c
@@ -72,11 +72,6 @@
 #define GPIO_PULL_DOWN  (0)
 #define GPIO_PULL_UP(1)
 
-#if 0 /* --- by chhung */
-#define MSDC_CLKSRC_REG (0xf10C)
-#define PDN_REG   (0xF110)
-#endif /* end of --- */
-
 #define DEFAULT_DEBOUNCE(8)   /* 8 cycles */
 #define DEFAULT_DTOC(40)  /* data timeout counter. 65536x40 sclk. 
*/
 
@@ -100,26 +95,6 @@ static int cd_active_low = 1;
 //#define PERI_MSDC2_PDN(17)
 //#define PERI_MSDC3_PDN(18)
 
-#if 0 /* --- by chhung */
-/* gate means clock power down */
-static int g_clk_gate = 0;
-#define msdc_gate_clock(id) \
-   do {   \
-   g_clk_gate &= ~(1 << ((id) + PERI_MSDC0_PDN));  \
-   } while (0)
-/* not like power down register. 1 means clock on. */
-#define msdc_ungate_clock(id) \
-   do {\
-   g_clk_gate |= 1 << ((id) + PERI_MSDC0_PDN); \
-   } while (0)
-
-// do we need sync object or not
-void msdc_clk_status(int *status)
-{
-   *status = g_clk_gate;
-}
-#endif /* end of --- */
-
 /* +++ by chhung */
 struct msdc_hw msdc0_hw = {
.clk_src= 0,
@@ -169,11 +144,6 @@ static void msdc_clr_fifo(struct msdc_host *host)
sdr_clr_bits(host->base + MSDC_INTEN, val); \
} while (0)
 
-#define msdc_irq_restore(val) \
-   do {\
-   sdr_set_bits(host->base + MSDC_INTEN, val); \
-   } while (0)
-
 /* clock source for host: global */
 #if defined(CONFIG_SOC_MT7620)
 static u32 hclks[] = {4800}; /* +/- by chhung */
@@ -181,32 +151,6 @@ static u32 hclks[] = {4800}; /* +/- by chhung */
 static u32 hclks[] = {5000}; /* +/- by chhung */
 #endif
 
-//
-// the power for msdc host controller: global
-//always keep the VMC on.
-//
-#define msdc_vcore_on(host) \
-   do {\
-   (void)hwPowerOn(MT65XX_POWER_LDO_VMC, VOL_3300, "SD");  \
-   } while (0)
-#define msdc_vcore_off(host) \
-   do {\
-   (void)hwPowerDown(MT65XX_POWER_LDO_VMC, "SD");  \
-   } while (0)
-
-//
-// the vdd output for card: global
-//   always keep the VMCH on.
-//
-#define msdc_vdd_on(host) \
-   do {\
-   (void)hwPowerOn(MT65XX_POWER_LDO_VMCH, VOL_3300, "SD"); \
-   } while (0)
-#define msdc_vdd_off(host) \
-   do {\
-   (void)hwPowerDown(MT65XX_POWER_LDO_VMCH, "SD"); \
-   } while (0)
-
 #define sdc_is_busy()  (readl(host->base + SDC_STS) & SDC_STS_SDCBUSY)
 #define sdc_is_cmd_busy()  (readl(host->base + SDC_STS) & SDC_STS_CMDBUSY)
 
@@ -252,7 +196,6 @@ static void msdc_tasklet_card(struct work_struct *work)
struct msdc_host, card_delaywork.work);
u32 inserted;
u32 status = 0;
-//u32 change = 0;
 
spin_lock(&host->lock);
 
@@ -262,53 +205,17 @@ static void msdc_tasklet_card(struct work_struct *work)
else
inserted = (status & MSDC_PS_CDSTS) ? 1 : 0;
 
-#if 0
-   change = host->card_inserted ^ inserted;
-   host->card_inserted = inserted;
-
-   if (change && !host->suspend) {
-   if (inserted)
-   host->mmc->f_max = HOST_MAX_MCLK;  // work around
-   mmc_detect_change(host->mmc, msecs_to_jiffies(20));
-   }
-#else  /* Make sure: handle the last interrupt */
+   /* Make sure: handle the last interrupt */

[PATCH v3] staging: ks7010: Add null pointer check for skb

2018-09-27 Thread Aymen Qader
Add a null pointer check for the socket buffer in ks_hostif.c to avoid a
possible null pointer deference, and remove a later now-redundant null
pointer check.

Signed-off-by: Aymen Qader 
---
v2: Remove redundant pointer check
v3: Style fix

 drivers/staging/ks7010/ks_hostif.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c 
b/drivers/staging/ks7010/ks_hostif.c
index 0e554e3359b5..95b6c7557e84 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -1011,6 +1011,11 @@ int hostif_data_request(struct ks_wlan_private *priv, 
struct sk_buff *skb)
size_t size;
int ret;
 
+   if (!skb) {
+   ret = -ENOMEM;
+   goto err_kfree;
+   }
+
skb_len = skb->len;
if (skb_len > ETH_FRAME_LEN) {
netdev_err(priv->net_dev, "bad length skb_len=%d\n", skb_len);
@@ -1023,7 +1028,6 @@ int hostif_data_request(struct ks_wlan_private *priv, 
struct sk_buff *skb)
priv->wpa.mic_failure.stop) {
if (netif_queue_stopped(priv->net_dev))
netif_wake_queue(priv->net_dev);
-   if (skb)
dev_kfree_skb(skb);
 
return 0;
-- 
2.17.1

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


[PATCH v2] staging: ks7010: Add null pointer check for skb

2018-09-27 Thread Aymen Qader
Add a null pointer check for the socket buffer in ks_hostif.c to avoid a
possible null pointer deference, and remove a later now-redundant null
pointer check.

Signed-off-by: Aymen Qader 
---
v2: Remove redundant pointer check

 drivers/staging/ks7010/ks_hostif.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c 
b/drivers/staging/ks7010/ks_hostif.c
index 0e554e3359b5..fdfee760a54f 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -1011,6 +1011,11 @@ int hostif_data_request(struct ks_wlan_private *priv, 
struct sk_buff *skb)
size_t size;
int ret;
 
+   if(!skb) {
+   ret = -ENOMEM;
+   goto err_kfree;
+   }
+
skb_len = skb->len;
if (skb_len > ETH_FRAME_LEN) {
netdev_err(priv->net_dev, "bad length skb_len=%d\n", skb_len);
@@ -1023,7 +1028,6 @@ int hostif_data_request(struct ks_wlan_private *priv, 
struct sk_buff *skb)
priv->wpa.mic_failure.stop) {
if (netif_queue_stopped(priv->net_dev))
netif_wake_queue(priv->net_dev);
-   if (skb)
dev_kfree_skb(skb);
 
return 0;
-- 
2.17.1

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


For editing of your photos 17

2018-09-27 Thread Jessica

Do you have needs for your photos cutting out and retouching?

We do editing for e-commerce photos, portrait photos and wedding photos.

You may choose to send us one or tow photos, we will provide testing to
check quality.

Thanks,
Jessica

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


[PATCH] staging: ks7010: Add null pointer check for skb

2018-09-27 Thread Aymen Qader
Add a null pointer check for the socket buffer in ks_hostif.c to avoid a
possible null pointer deference.

Signed-off-by: Aymen Qader 
---
 drivers/staging/ks7010/ks_hostif.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/staging/ks7010/ks_hostif.c 
b/drivers/staging/ks7010/ks_hostif.c
index 0e554e3359b5..4a5bc7858ef7 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -1011,6 +1011,11 @@ int hostif_data_request(struct ks_wlan_private *priv, 
struct sk_buff *skb)
size_t size;
int ret;
 
+   if(!skb) {
+   ret = -ENOMEM;
+   goto err_kfree;
+   }
+
skb_len = skb->len;
if (skb_len > ETH_FRAME_LEN) {
netdev_err(priv->net_dev, "bad length skb_len=%d\n", skb_len);
-- 
2.17.1

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


Re: [patch 09/11] x86/vdso: Simplify the invalid vclock case

2018-09-27 Thread Thomas Gleixner
On Tue, 18 Sep 2018, Thomas Gleixner wrote:
> On Tue, 18 Sep 2018, Thomas Gleixner wrote:
> > So if the TSC on CPU1 is slightly behind the TSC on CPU0 then now1 can be
> > smaller than cycle_last. The TSC sync stuff does not catch the small delta
> > for unknown raisins. I'll go and find that machine and test that again.
> 
> Of course it does not trigger anymore. We accumulated code between the
> point in timekeeping_advance() where the TSC is read and the update of the
> VDSO data.
> 
> I'll might have to get an 2.6ish kernel booted on that machine and try with
> that again. /me shudders

Actually it does happen, because the TSC is very slowly drifting apart due
to SMI wreckage trying to hide itself. It just takes a very long time.

Thanks,

tglx


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


Re: [patch 09/11] x86/vdso: Simplify the invalid vclock case

2018-09-27 Thread Andy Lutomirski


> On Sep 27, 2018, at 7:36 AM, Thomas Gleixner  wrote:
> 
>> On Wed, 19 Sep 2018, Thomas Gleixner wrote:
>> On Tue, 18 Sep 2018, Andy Lutomirski wrote:
 On Sep 18, 2018, at 3:46 PM, Thomas Gleixner  wrote:
> On Tue, 18 Sep 2018, Andy Lutomirski wrote:
> Do we do better if we use signed arithmetic for the whole calculation?
> Then a small backwards movement would result in a small backwards result.
> Or we could offset everything so that we’d have to go back several
> hundred ms before we cross zero.
 
 That would be probably the better solution as signed math would be
 problematic when the resulting ns value becomes negative. As the delta is
 really small, otherwise the TSC sync check would have caught it, the caller
 should never be able to observe time going backwards.
 
 I'll have a look into that. It needs some thought vs. the fractional part
 of the base time, but it should be not rocket science to get that
 correct. Famous last words...
 
>>> 
>>> It’s also fiddly to tune. If you offset it too much, then the fancy
>>> divide-by-repeated-subtraction loop will hurt more than the comparison to
>>> last.
>> 
>> Not really. It's sufficient to offset it by at max. 1000 cycles or so. That
>> won't hurt the magic loop, but it will definitely cover that slight offset
>> case.
> 
> I got it working, but first of all the gain is close to 0.
> 
> There is this other subtle issue that we've seen TSCs slowly drifting apart
> which is caught by the TSC watchdog eventually, but if it exeeds the offset
> _before_ the watchdog triggers, we're back to square one.
> 
> So I rather stay on the safe side and just accept that we have to deal with
> that. Sigh.
> 
> 

Seems okay to me. Oh well. 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [patch 09/11] x86/vdso: Simplify the invalid vclock case

2018-09-27 Thread Thomas Gleixner
On Wed, 19 Sep 2018, Thomas Gleixner wrote:
> On Tue, 18 Sep 2018, Andy Lutomirski wrote:
> > > On Sep 18, 2018, at 3:46 PM, Thomas Gleixner  wrote:
> > > On Tue, 18 Sep 2018, Andy Lutomirski wrote:
> > >> Do we do better if we use signed arithmetic for the whole calculation?
> > >> Then a small backwards movement would result in a small backwards result.
> > >> Or we could offset everything so that we’d have to go back several
> > >> hundred ms before we cross zero.
> > > 
> > > That would be probably the better solution as signed math would be
> > > problematic when the resulting ns value becomes negative. As the delta is
> > > really small, otherwise the TSC sync check would have caught it, the 
> > > caller
> > > should never be able to observe time going backwards.
> > > 
> > > I'll have a look into that. It needs some thought vs. the fractional part
> > > of the base time, but it should be not rocket science to get that
> > > correct. Famous last words...
> > > 
> > 
> > It’s also fiddly to tune. If you offset it too much, then the fancy
> > divide-by-repeated-subtraction loop will hurt more than the comparison to
> > last.
> 
> Not really. It's sufficient to offset it by at max. 1000 cycles or so. That
> won't hurt the magic loop, but it will definitely cover that slight offset
> case.

I got it working, but first of all the gain is close to 0.

There is this other subtle issue that we've seen TSCs slowly drifting apart
which is caught by the TSC watchdog eventually, but if it exeeds the offset
_before_ the watchdog triggers, we're back to square one.

So I rather stay on the safe side and just accept that we have to deal with
that. Sigh.

Thanks,

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


Re: [PATCH] fix error handling in drivers/staging/rtl8192u/ieee80211/ieee80211_module.c

2018-09-27 Thread Dan Carpenter
On Wed, Sep 26, 2018 at 01:52:17PM -0400, valdis.kletni...@vt.edu wrote:
> John notes that if the kzalloc of ieee->pHTInfo fails, we fail to call
> ieee80211_networks_free().  In addition, that function has an un-needed check
> before kfree().
> 
> Reported-by: John Whitmore 
> Signed-off-by: Valdis Kletnieks 
> ---
> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c 
> b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
> index 90a097f2cd4e..97ff0371b5bb 100644
> --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
> @@ -78,8 +78,6 @@ static inline int ieee80211_networks_allocate(struct 
> ieee80211_device *ieee)
>  
>  static inline void ieee80211_networks_free(struct ieee80211_device *ieee)
>  {
> - if (!ieee->networks)
> - return;
>   kfree(ieee->networks);
>   ieee->networks = NULL;
>  }
> @@ -180,6 +178,7 @@ struct net_device *alloc_ieee80211(int sizeof_priv)
>   return dev;
>  
>   failed:
> + ieee80211_networks_free(ieee);
>   if (dev)
>   free_netdev(dev);

When there is a "goto failed;" then it's called "one err style" error
handling and we're just asking for bugs...  In this case the bug is that
we're not allowd to call ieee80211_networks_free() with a NULL
"ieee" parameter.  The right thing to do is to only call
ieee80211_networks_free() if we know that ieee80211_networks_allocate()
succeeded.

regards,
dan carpenter

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


Re: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging/

2018-09-27 Thread Andrew Lunn
On Thu, Sep 27, 2018 at 07:12:27PM +0800, Yangbo Lu wrote:
> This patch is to move DPAA2 PTP driver out of staging/
> since the dpaa2-eth had been moved out.
> 
> Signed-off-by: Yangbo Lu 
> ---
>  drivers/net/ethernet/freescale/Kconfig |9 +
>  drivers/net/ethernet/freescale/dpaa2/Kconfig   |   15 +++
>  drivers/net/ethernet/freescale/dpaa2/Makefile  |6 --
>  .../ethernet/freescale/dpaa2}/dprtc-cmd.h  |0
>  .../rtc => net/ethernet/freescale/dpaa2}/dprtc.c   |0
>  .../rtc => net/ethernet/freescale/dpaa2}/dprtc.h   |0
>  .../rtc => net/ethernet/freescale/dpaa2}/rtc.c |0
>  .../rtc => net/ethernet/freescale/dpaa2}/rtc.h |0
>  drivers/staging/fsl-dpaa2/Kconfig  |8 
>  drivers/staging/fsl-dpaa2/Makefile |1 -
>  drivers/staging/fsl-dpaa2/rtc/Makefile |7 ---
>  11 files changed, 20 insertions(+), 26 deletions(-)
>  create mode 100644 drivers/net/ethernet/freescale/dpaa2/Kconfig
>  rename drivers/{staging/fsl-dpaa2/rtc => 
> net/ethernet/freescale/dpaa2}/dprtc-cmd.h (100%)
>  rename drivers/{staging/fsl-dpaa2/rtc => 
> net/ethernet/freescale/dpaa2}/dprtc.c (100%)
>  rename drivers/{staging/fsl-dpaa2/rtc => 
> net/ethernet/freescale/dpaa2}/dprtc.h (100%)
>  rename drivers/{staging/fsl-dpaa2/rtc => net/ethernet/freescale/dpaa2}/rtc.c 
> (100%)
>  rename drivers/{staging/fsl-dpaa2/rtc => net/ethernet/freescale/dpaa2}/rtc.h 
> (100%)

Hi Yangbo

Calling a ptp driver rtc.[ch] seems rather odd. Could you fixup the
name, change it to ptp.[ch]. Also, some of the function names, and
structures, rtc_probe->ptp_probe, rtc_remove->ptp_remove,
rtc_match_id_table-> ptp_match_id_table, etc.

ptp_dpaa2_adjfreq() probably should return err, not 0.
ptp_dpaa2_gettime() again does not return the error.
If fact, it seems like all the main functions ignore errors.

kzalloc() could be changed to devm_kzalloc() to simplify the cleanup
Can ptp_dpaa2_caps be made const?
dpaa2_phc_index does not appear to be used.
dev_set_drvdata(dev, NULL); is not needed.
Can rtc_drv be made const?
Is rtc.h used by anything other than rtc.c? It seems like it can be removed.

It seems like there is a lot of code in dprtc.c which is unused. rtc.c
does nothing with interrupts for example. Do you plan to make use of
this extra code? Or can it be removed leaving just what is needed?

struct dprtc_cmd_get_irq - Putting pad at the beginning of a struct
seems very odd. And it is not the only example.

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


[PATCH][staging-next] staging: wilc1000: fix incorrect allocation size for structure

2018-09-27 Thread Colin King
From: Colin Ian King 

Currently the allocation for str_vals is for the sizeof the pointer
rather than the size of the structure.  Fix this.

Detected by smatch
"wilc_wlan_cfg_init() error: not allocating enough data 392 vs 8"

Fixes: acceb12a9f8b ("staging: wilc1000: refactor code to avoid static 
variables for config parameters")
Signed-off-by: Colin Ian King 
---
 drivers/staging/wilc1000/wilc_wlan_cfg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wilc1000/wilc_wlan_cfg.c 
b/drivers/staging/wilc1000/wilc_wlan_cfg.c
index 930a3896f4ae..faa001c75681 100644
--- a/drivers/staging/wilc1000/wilc_wlan_cfg.c
+++ b/drivers/staging/wilc1000/wilc_wlan_cfg.c
@@ -457,7 +457,7 @@ int wilc_wlan_cfg_init(struct wilc *wl)
if (!wl->cfg.s)
goto out_w;
 
-   str_vals = kzalloc(sizeof(str_vals), GFP_KERNEL);
+   str_vals = kzalloc(sizeof(*str_vals), GFP_KERNEL);
if (!str_vals)
goto out_s;
 
-- 
2.17.1

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


[PATCH] staging: rtl8188eu: remove get_right_chnl_for_iqk()

2018-09-27 Thread Michael Straube
The function get_right_chnl_for_iqk() only returns non zero values for
channels > 14. According to the TODO, code valid only for 5 GHz should
be removed.

- find and remove remaining code valid only for 5 GHz. Most of the obvious
  ones have been removed, but things like channel > 14 still exist.

Remove get_right_chnl_for_iqk() and the variable chn_index that is
used to save the return value. Replace the uses of chn_index with zero.

Remove the now unused define ODM_TARGET_CHNL_NUM_2G_5G.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/hal/phy.c | 27 +++--
 drivers/staging/rtl8188eu/include/phy.h |  1 -
 2 files changed, 3 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/phy.c 
b/drivers/staging/rtl8188eu/hal/phy.c
index 3c7cf8720df8..482d48e003b7 100644
--- a/drivers/staging/rtl8188eu/hal/phy.c
+++ b/drivers/staging/rtl8188eu/hal/phy.c
@@ -298,25 +298,6 @@ void rtw_hal_set_chan(struct adapter *adapt, u8 channel)
 
 #define ODM_TXPWRTRACK_MAX_IDX_88E  6
 
-static u8 get_right_chnl_for_iqk(u8 chnl)
-{
-   u8 place;
-   u8 channel_all[ODM_TARGET_CHNL_NUM_2G_5G] = {
-   36, 38, 40, 42, 44, 46, 48, 50, 52, 54, 56, 58, 60, 62, 64,
-   100, 102, 104, 106, 108, 110, 112, 114, 116, 118, 120, 122,
-   124, 126, 128, 130, 132, 134, 136, 138, 140, 149, 151, 153,
-   155, 157, 159, 161, 163, 165
-   };
-
-   if (chnl > 14) {
-   for (place = 0; place < sizeof(channel_all); place++) {
-   if (channel_all[place] == chnl)
-   return ++place;
-   }
-   }
-   return 0;
-}
-
 void rtl88eu_dm_txpower_track_adjust(struct odm_dm_struct *dm_odm, u8 type,
 u8 *direction, u32 *out_write_val)
 {
@@ -1215,7 +1196,7 @@ void rtl88eu_phy_iq_calibrate(struct adapter *adapt, bool 
recovery)
 {
struct odm_dm_struct *dm_odm = &adapt->HalData->odmpriv;
s32 result[4][8];
-   u8 i, final, chn_index;
+   u8 i, final;
bool pathaok, pathbok;
s32 reg_e94, reg_e9c, reg_ea4, reg_eb4, reg_ebc, reg_ec4;
bool is12simular, is13simular, is23simular;
@@ -1324,12 +1305,10 @@ void rtl88eu_phy_iq_calibrate(struct adapter *adapt, 
bool recovery)
   (reg_ec4 == 0));
}
 
-   chn_index = get_right_chnl_for_iqk(adapt->HalData->CurrentChannel);
-
if (final < 4) {
for (i = 0; i < IQK_Matrix_REG_NUM; i++)
-   
dm_odm->RFCalibrateInfo.IQKMatrixRegSetting[chn_index].Value[0][i] = 
result[final][i];
-   dm_odm->RFCalibrateInfo.IQKMatrixRegSetting[chn_index].bIQKDone 
= true;
+   
dm_odm->RFCalibrateInfo.IQKMatrixRegSetting[0].Value[0][i] = result[final][i];
+   dm_odm->RFCalibrateInfo.IQKMatrixRegSetting[0].bIQKDone = true;
}
 
save_adda_registers(adapt, iqk_bb_reg_92c,
diff --git a/drivers/staging/rtl8188eu/include/phy.h 
b/drivers/staging/rtl8188eu/include/phy.h
index e99ac3910787..40901d6dcaf5 100644
--- a/drivers/staging/rtl8188eu/include/phy.h
+++ b/drivers/staging/rtl8188eu/include/phy.h
@@ -4,7 +4,6 @@
 #define IQK_DELAY_TIME_88E 10
 #define index_mapping_NUM_88E  15
 #define AVG_THERMAL_NUM_88E4
-#define ODM_TARGET_CHNL_NUM_2G_5G   59
 
 bool rtl88eu_phy_mac_config(struct adapter *adapt);
 bool rtl88eu_phy_rf_config(struct adapter *adapt);
-- 
2.19.0

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


[PATCH 2/2] MAINTAINERS: update files maintained under DPAA2 PTP/ETHERNET

2018-09-27 Thread Yangbo Lu
The files maintained under DPAA2 PTP/ETHERNET needs to
be updated since dpaa2 ptp driver had been moved into
drivers/net/ethernet/freescale/dpaa2/.

Signed-off-by: Yangbo Lu 
---
 MAINTAINERS |   11 ---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 15565de..ba6f441 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4530,7 +4530,11 @@ DPAA2 ETHERNET DRIVER
 M: Ioana Radulescu 
 L: net...@vger.kernel.org
 S: Maintained
-F: drivers/net/ethernet/freescale/dpaa2
+F: drivers/net/ethernet/freescale/dpaa2/dpaa2-eth*
+F: drivers/net/ethernet/freescale/dpaa2/dpni*
+F: drivers/net/ethernet/freescale/dpaa2/dpkg.h
+F: drivers/net/ethernet/freescale/dpaa2/Makefile
+F: drivers/net/ethernet/freescale/dpaa2/Kconfig
 
 DPAA2 ETHERNET SWITCH DRIVER
 M: Ioana Radulescu 
@@ -4541,9 +4545,10 @@ F:   drivers/staging/fsl-dpaa2/ethsw
 
 DPAA2 PTP CLOCK DRIVER
 M: Yangbo Lu 
-L: linux-ker...@vger.kernel.org
+L: net...@vger.kernel.org
 S: Maintained
-F: drivers/staging/fsl-dpaa2/rtc
+F: drivers/net/ethernet/freescale/dpaa2/rtc*
+F: drivers/net/ethernet/freescale/dpaa2/dprtc*
 
 DPT_I2O SCSI RAID DRIVER
 M: Adaptec OEM Raid Solutions 
-- 
1.7.1

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


[PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging/

2018-09-27 Thread Yangbo Lu
This patch is to move DPAA2 PTP driver out of staging/
since the dpaa2-eth had been moved out.

Signed-off-by: Yangbo Lu 
---
 drivers/net/ethernet/freescale/Kconfig |9 +
 drivers/net/ethernet/freescale/dpaa2/Kconfig   |   15 +++
 drivers/net/ethernet/freescale/dpaa2/Makefile  |6 --
 .../ethernet/freescale/dpaa2}/dprtc-cmd.h  |0
 .../rtc => net/ethernet/freescale/dpaa2}/dprtc.c   |0
 .../rtc => net/ethernet/freescale/dpaa2}/dprtc.h   |0
 .../rtc => net/ethernet/freescale/dpaa2}/rtc.c |0
 .../rtc => net/ethernet/freescale/dpaa2}/rtc.h |0
 drivers/staging/fsl-dpaa2/Kconfig  |8 
 drivers/staging/fsl-dpaa2/Makefile |1 -
 drivers/staging/fsl-dpaa2/rtc/Makefile |7 ---
 11 files changed, 20 insertions(+), 26 deletions(-)
 create mode 100644 drivers/net/ethernet/freescale/dpaa2/Kconfig
 rename drivers/{staging/fsl-dpaa2/rtc => 
net/ethernet/freescale/dpaa2}/dprtc-cmd.h (100%)
 rename drivers/{staging/fsl-dpaa2/rtc => net/ethernet/freescale/dpaa2}/dprtc.c 
(100%)
 rename drivers/{staging/fsl-dpaa2/rtc => net/ethernet/freescale/dpaa2}/dprtc.h 
(100%)
 rename drivers/{staging/fsl-dpaa2/rtc => net/ethernet/freescale/dpaa2}/rtc.c 
(100%)
 rename drivers/{staging/fsl-dpaa2/rtc => net/ethernet/freescale/dpaa2}/rtc.h 
(100%)
 delete mode 100644 drivers/staging/fsl-dpaa2/rtc/Makefile

diff --git a/drivers/net/ethernet/freescale/Kconfig 
b/drivers/net/ethernet/freescale/Kconfig
index 7a30276..d3a62bc 100644
--- a/drivers/net/ethernet/freescale/Kconfig
+++ b/drivers/net/ethernet/freescale/Kconfig
@@ -96,13 +96,6 @@ config GIANFAR
  on the 8540.
 
 source "drivers/net/ethernet/freescale/dpaa/Kconfig"
-
-config FSL_DPAA2_ETH
-   tristate "Freescale DPAA2 Ethernet"
-   depends on FSL_MC_BUS && FSL_MC_DPIO
-   depends on NETDEVICES && ETHERNET
-   ---help---
- Ethernet driver for Freescale DPAA2 SoCs, using the
- Freescale MC bus driver
+source "drivers/net/ethernet/freescale/dpaa2/Kconfig"
 
 endif # NET_VENDOR_FREESCALE
diff --git a/drivers/net/ethernet/freescale/dpaa2/Kconfig 
b/drivers/net/ethernet/freescale/dpaa2/Kconfig
new file mode 100644
index 000..44c5c3a
--- /dev/null
+++ b/drivers/net/ethernet/freescale/dpaa2/Kconfig
@@ -0,0 +1,15 @@
+config FSL_DPAA2_ETH
+   tristate "Freescale DPAA2 Ethernet"
+   depends on FSL_MC_BUS && FSL_MC_DPIO
+   depends on NETDEVICES && ETHERNET
+   help
+ Ethernet driver for Freescale DPAA2 SoCs, using the
+ Freescale MC bus driver
+
+config FSL_DPAA2_PTP_CLOCK
+   tristate "Freescale DPAA2 PTP Clock"
+   depends on FSL_DPAA2_ETH && POSIX_TIMERS
+   select PTP_1588_CLOCK
+   help
+ This driver adds support for using the DPAA2 1588 timer module
+ as a PTP clock.
diff --git a/drivers/net/ethernet/freescale/dpaa2/Makefile 
b/drivers/net/ethernet/freescale/dpaa2/Makefile
index 9315ecd..312e37f 100644
--- a/drivers/net/ethernet/freescale/dpaa2/Makefile
+++ b/drivers/net/ethernet/freescale/dpaa2/Makefile
@@ -3,9 +3,11 @@
 # Makefile for the Freescale DPAA2 Ethernet controller
 #
 
-obj-$(CONFIG_FSL_DPAA2_ETH) += fsl-dpaa2-eth.o
+obj-$(CONFIG_FSL_DPAA2_ETH)+= fsl-dpaa2-eth.o
+obj-$(CONFIG_FSL_DPAA2_PTP_CLOCK)  += fsl-dpaa2-rtc.o
 
-fsl-dpaa2-eth-objs:= dpaa2-eth.o dpaa2-ethtool.o dpni.o
+fsl-dpaa2-eth-objs := dpaa2-eth.o dpaa2-ethtool.o dpni.o
+fsl-dpaa2-rtc-objs := rtc.o dprtc.o
 
 # Needed by the tracing framework
 CFLAGS_dpaa2-eth.o := -I$(src)
diff --git a/drivers/staging/fsl-dpaa2/rtc/dprtc-cmd.h 
b/drivers/net/ethernet/freescale/dpaa2/dprtc-cmd.h
similarity index 100%
rename from drivers/staging/fsl-dpaa2/rtc/dprtc-cmd.h
rename to drivers/net/ethernet/freescale/dpaa2/dprtc-cmd.h
diff --git a/drivers/staging/fsl-dpaa2/rtc/dprtc.c 
b/drivers/net/ethernet/freescale/dpaa2/dprtc.c
similarity index 100%
rename from drivers/staging/fsl-dpaa2/rtc/dprtc.c
rename to drivers/net/ethernet/freescale/dpaa2/dprtc.c
diff --git a/drivers/staging/fsl-dpaa2/rtc/dprtc.h 
b/drivers/net/ethernet/freescale/dpaa2/dprtc.h
similarity index 100%
rename from drivers/staging/fsl-dpaa2/rtc/dprtc.h
rename to drivers/net/ethernet/freescale/dpaa2/dprtc.h
diff --git a/drivers/staging/fsl-dpaa2/rtc/rtc.c 
b/drivers/net/ethernet/freescale/dpaa2/rtc.c
similarity index 100%
rename from drivers/staging/fsl-dpaa2/rtc/rtc.c
rename to drivers/net/ethernet/freescale/dpaa2/rtc.c
diff --git a/drivers/staging/fsl-dpaa2/rtc/rtc.h 
b/drivers/net/ethernet/freescale/dpaa2/rtc.h
similarity index 100%
rename from drivers/staging/fsl-dpaa2/rtc/rtc.h
rename to drivers/net/ethernet/freescale/dpaa2/rtc.h
diff --git a/drivers/staging/fsl-dpaa2/Kconfig 
b/drivers/staging/fsl-dpaa2/Kconfig
index 59aaae7..991e154 100644
--- a/drivers/staging/fsl-dpaa2/Kconfig
+++ b/drivers/staging/fsl-dpaa2/Kconfig
@@ -16,11 +16,3 @@ config FSL_DPAA2_ETHSW
---help---

Re: [PATCH] hv_netvsc: Make sure out channel is fully opened on send

2018-09-27 Thread Mohammed Gamal
On Thu, 2018-09-27 at 12:23 +0200, Stephen Hemminger wrote:
> On Thu, 27 Sep 2018 10:57:05 +0200
> Mohammed Gamal  wrote:
> 
> > On Wed, 2018-09-26 at 17:13 +, Haiyang Zhang wrote:
> > > > -Original Message-
> > > > From: Mohammed Gamal 
> > > > Sent: Wednesday, September 26, 2018 12:34 PM
> > > > To: Stephen Hemminger ; net...@vger.ker
> > > > nel.
> > > > org
> > > > Cc: KY Srinivasan ; Haiyang Zhang
> > > > ; vkuznets ;
> > > > ot...@redhat.com; cavery ; linux-
> > > > ker...@vger.kernel.org; de...@linuxdriverproject.org; Mohammed
> > > > Gamal
> > > > 
> > > > Subject: [PATCH] hv_netvsc: Make sure out channel is fully
> > > > opened
> > > > on send
> > > > 
> > > > Dring high network traffic changes to network interface
> > > > parameters
> > > > such as
> > > > number of channels or MTU can cause a kernel panic with a NULL
> > > > pointer
> > > > dereference. This is due to netvsc_device_remove() being called
> > > > and
> > > > deallocating the channel ring buffers, which can then be
> > > > accessed
> > > > by
> > > > netvsc_send_pkt() before they're allocated on calling
> > > > netvsc_device_add()
> > > > 
> > > > The patch fixes this problem by checking the channel state and
> > > > returning
> > > > ENODEV if not yet opened. We also move the call to
> > > > hv_ringbuf_avail_percent()
> > > > which may access the uninitialized ring buffer.
> > > > 
> > > > Signed-off-by: Mohammed Gamal 
> > > > ---
> > > >  drivers/net/hyperv/netvsc.c | 7 ++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/net/hyperv/netvsc.c
> > > > b/drivers/net/hyperv/netvsc.c index
> > > > fe01e14..75f1b31 100644
> > > > --- a/drivers/net/hyperv/netvsc.c
> > > > +++ b/drivers/net/hyperv/netvsc.c
> > > > @@ -825,7 +825,12 @@ static inline int netvsc_send_pkt(
> > > >     struct netdev_queue *txq = netdev_get_tx_queue(ndev,
> > > > packet->q_idx);
> > > >     u64 req_id;
> > > >     int ret;
> > > > -   u32 ring_avail =
> > > > hv_get_avail_to_write_percent(&out_channel-  
> > > > > outbound);  
> > > > 
> > > > +   u32 ring_avail;
> > > > +
> > > > +   if (out_channel->state != CHANNEL_OPENED_STATE)
> > > > +   return -ENODEV;
> > > > +
> > > > +   ring_avail =
> > > > hv_get_avail_to_write_percent(&out_channel-  
> > > > > outbound);  
> > > 
> > > When you reproducing the NULL ptr panic, does your kernel include
> > > the
> > > following patch?
> > > hv_netvsc: common detach logic
> > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.g
> > > it/c
> > > ommit/?id=7b2ee50c0cd513a176a26a71f2989facdd75bfea
> > >   
> > 
> > Yes it is included. And the commit did reduce the occurrence of
> > this
> > race condition, but it still nevertheless occurs albeit rarely.
> > 
> > > We call netif_tx_disable(ndev) and netif_device_detach(ndev)
> > > before
> > > doing the changes 
> > > on MTU or #channels. So there should be no call to start_xmit()
> > > when
> > > channel is not ready.
> > > 
> > > If you see the check for CHANNEL_OPENED_STATE is still necessary
> > > on
> > > upstream kernel (including 
> > > the patch " common detach logic "), we should debug further on
> > > the
> > > code and find out the 
> > > root cause.
> > > 
> > > Thanks,
> > > - Haiyang
> > >   
> > 
> > ___
> > devel mailing list
> > de...@linuxdriverproject.org
> > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-
> > devel
> 
> Is there some workload, that can be used to reproduce this?
> The stress test from Vitaly with changing parameters while running
> network traffic
> passes now.
> 
> Can you reproduce this with the upstream current kernel?
> 
> Adding the check in start xmit is still racy, and won't cure the
> problem.
> 
> Another solution would be to add a grace period in the netvsc detach
> logic.
> 

Steps to reproduce are listed here:
https://bugzilla.redhat.com/show_bug.cgi?id=1632653

We've also managed to reproduce the same issue upstream. It's more
likely to be reproduced on Windows 2012R2 than 2016.

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


Re: [PATCH] hv_netvsc: Make sure out channel is fully opened on send

2018-09-27 Thread Stephen Hemminger
On Thu, 27 Sep 2018 10:57:05 +0200
Mohammed Gamal  wrote:

> On Wed, 2018-09-26 at 17:13 +, Haiyang Zhang wrote:
> > > -Original Message-
> > > From: Mohammed Gamal 
> > > Sent: Wednesday, September 26, 2018 12:34 PM
> > > To: Stephen Hemminger ; netdev@vger.kernel.
> > > org
> > > Cc: KY Srinivasan ; Haiyang Zhang
> > > ; vkuznets ;
> > > ot...@redhat.com; cavery ; linux-
> > > ker...@vger.kernel.org; de...@linuxdriverproject.org; Mohammed
> > > Gamal
> > > 
> > > Subject: [PATCH] hv_netvsc: Make sure out channel is fully opened
> > > on send
> > > 
> > > Dring high network traffic changes to network interface parameters
> > > such as
> > > number of channels or MTU can cause a kernel panic with a NULL
> > > pointer
> > > dereference. This is due to netvsc_device_remove() being called and
> > > deallocating the channel ring buffers, which can then be accessed
> > > by
> > > netvsc_send_pkt() before they're allocated on calling
> > > netvsc_device_add()
> > > 
> > > The patch fixes this problem by checking the channel state and
> > > returning
> > > ENODEV if not yet opened. We also move the call to
> > > hv_ringbuf_avail_percent()
> > > which may access the uninitialized ring buffer.
> > > 
> > > Signed-off-by: Mohammed Gamal 
> > > ---
> > >  drivers/net/hyperv/netvsc.c | 7 ++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/net/hyperv/netvsc.c
> > > b/drivers/net/hyperv/netvsc.c index
> > > fe01e14..75f1b31 100644
> > > --- a/drivers/net/hyperv/netvsc.c
> > > +++ b/drivers/net/hyperv/netvsc.c
> > > @@ -825,7 +825,12 @@ static inline int netvsc_send_pkt(
> > >   struct netdev_queue *txq = netdev_get_tx_queue(ndev,
> > > packet->q_idx);
> > >   u64 req_id;
> > >   int ret;
> > > - u32 ring_avail =
> > > hv_get_avail_to_write_percent(&out_channel-  
> > > > outbound);  
> > > 
> > > + u32 ring_avail;
> > > +
> > > + if (out_channel->state != CHANNEL_OPENED_STATE)
> > > + return -ENODEV;
> > > +
> > > + ring_avail = hv_get_avail_to_write_percent(&out_channel-  
> > > >outbound);  
> > 
> > When you reproducing the NULL ptr panic, does your kernel include the
> > following patch?
> > hv_netvsc: common detach logic
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/c
> > ommit/?id=7b2ee50c0cd513a176a26a71f2989facdd75bfea
> >   
> Yes it is included. And the commit did reduce the occurrence of this
> race condition, but it still nevertheless occurs albeit rarely.
> 
> > We call netif_tx_disable(ndev) and netif_device_detach(ndev) before
> > doing the changes 
> > on MTU or #channels. So there should be no call to start_xmit() when
> > channel is not ready.
> > 
> > If you see the check for CHANNEL_OPENED_STATE is still necessary on
> > upstream kernel (including 
> > the patch " common detach logic "), we should debug further on the
> > code and find out the 
> > root cause.
> > 
> > Thanks,
> > - Haiyang
> >   
> ___
> devel mailing list
> de...@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Is there some workload, that can be used to reproduce this?
The stress test from Vitaly with changing parameters while running network 
traffic
passes now.

Can you reproduce this with the upstream current kernel?

Adding the check in start xmit is still racy, and won't cure the problem.

Another solution would be to add a grace period in the netvsc detach logic.

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


[PATCH v3 5/6] powerpc/powernv: hold device_hotplug_lock when calling memtrace_offline_pages()

2018-09-27 Thread David Hildenbrand
Let's perform all checking + offlining + removing under
device_hotplug_lock, so nobody can mess with these devices via
sysfs concurrently.

Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: Rashmica Gupta 
Cc: Balbir Singh 
Cc: Michael Neuling 
Reviewed-by: Pavel Tatashin 
Reviewed-by: Rashmica Gupta 
Acked-by: Balbir Singh 
Signed-off-by: David Hildenbrand 
---
 arch/powerpc/platforms/powernv/memtrace.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/memtrace.c 
b/arch/powerpc/platforms/powernv/memtrace.c
index fdd48f1a39f7..84d038ed3882 100644
--- a/arch/powerpc/platforms/powernv/memtrace.c
+++ b/arch/powerpc/platforms/powernv/memtrace.c
@@ -70,6 +70,7 @@ static int change_memblock_state(struct memory_block *mem, 
void *arg)
return 0;
 }
 
+/* called with device_hotplug_lock held */
 static bool memtrace_offline_pages(u32 nid, u64 start_pfn, u64 nr_pages)
 {
u64 end_pfn = start_pfn + nr_pages - 1;
@@ -110,6 +111,7 @@ static u64 memtrace_alloc_node(u32 nid, u64 size)
/* Trace memory needs to be aligned to the size */
end_pfn = round_down(end_pfn - nr_pages, nr_pages);
 
+   lock_device_hotplug();
for (base_pfn = end_pfn; base_pfn > start_pfn; base_pfn -= nr_pages) {
if (memtrace_offline_pages(nid, base_pfn, nr_pages) == true) {
/*
@@ -118,7 +120,6 @@ static u64 memtrace_alloc_node(u32 nid, u64 size)
 * we never try to remove memory that spans two iomem
 * resources.
 */
-   lock_device_hotplug();
end_pfn = base_pfn + nr_pages;
for (pfn = base_pfn; pfn < end_pfn; pfn += bytes>> 
PAGE_SHIFT) {
__remove_memory(nid, pfn << PAGE_SHIFT, bytes);
@@ -127,6 +128,7 @@ static u64 memtrace_alloc_node(u32 nid, u64 size)
return base_pfn << PAGE_SHIFT;
}
}
+   unlock_device_hotplug();
 
return 0;
 }
-- 
2.17.1

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


[PATCH v3 4/6] powerpc/powernv: hold device_hotplug_lock when calling device_online()

2018-09-27 Thread David Hildenbrand
device_online() should be called with device_hotplug_lock() held.

Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: Rashmica Gupta 
Cc: Balbir Singh 
Cc: Michael Neuling 
Reviewed-by: Pavel Tatashin 
Reviewed-by: Rashmica Gupta 
Signed-off-by: David Hildenbrand 
---
 arch/powerpc/platforms/powernv/memtrace.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/platforms/powernv/memtrace.c 
b/arch/powerpc/platforms/powernv/memtrace.c
index 773623f6bfb1..fdd48f1a39f7 100644
--- a/arch/powerpc/platforms/powernv/memtrace.c
+++ b/arch/powerpc/platforms/powernv/memtrace.c
@@ -242,9 +242,11 @@ static int memtrace_online(void)
 * we need to online the memory ourselves.
 */
if (!memhp_auto_online) {
+   lock_device_hotplug();
walk_memory_range(PFN_DOWN(ent->start),
  PFN_UP(ent->start + ent->size - 1),
  NULL, online_mem_block);
+   unlock_device_hotplug();
}
 
/*
-- 
2.17.1

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


[PATCH v3 6/6] memory-hotplug.txt: Add some details about locking internals

2018-09-27 Thread David Hildenbrand
Let's document the magic a bit, especially why device_hotplug_lock is
required when adding/removing memory and how it all play together with
requests to online/offline memory from user space.

Cc: Jonathan Corbet 
Cc: Michal Hocko 
Cc: Andrew Morton 
Reviewed-by: Pavel Tatashin 
Reviewed-by: Rashmica Gupta 
Signed-off-by: David Hildenbrand 
---
 Documentation/memory-hotplug.txt | 42 +++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/Documentation/memory-hotplug.txt b/Documentation/memory-hotplug.txt
index 7f49ebf3ddb2..ce4faa5530fa 100644
--- a/Documentation/memory-hotplug.txt
+++ b/Documentation/memory-hotplug.txt
@@ -3,7 +3,7 @@ Memory Hotplug
 ==
 
 :Created:  Jul 28 2007
-:Updated: Add description of notifier of memory hotplug:   Oct 11 2007
+:Updated: Add some details about locking internals:Aug 20 2018
 
 This document is about memory hotplug including how-to-use and current status.
 Because Memory Hotplug is still under development, contents of this text will
@@ -495,6 +495,46 @@ further processing of the notification queue.
 
 NOTIFY_STOP stops further processing of the notification queue.
 
+
+Locking Internals
+=
+
+When adding/removing memory that uses memory block devices (i.e. ordinary RAM),
+the device_hotplug_lock should be held to:
+
+- synchronize against online/offline requests (e.g. via sysfs). This way, 
memory
+  block devices can only be accessed (.online/.state attributes) by user
+  space once memory has been fully added. And when removing memory, we
+  know nobody is in critical sections.
+- synchronize against CPU hotplug and similar (e.g. relevant for ACPI and PPC)
+
+Especially, there is a possible lock inversion that is avoided using
+device_hotplug_lock when adding memory and user space tries to online that
+memory faster than expected:
+
+- device_online() will first take the device_lock(), followed by
+  mem_hotplug_lock
+- add_memory_resource() will first take the mem_hotplug_lock, followed by
+  the device_lock() (while creating the devices, during bus_add_device()).
+
+As the device is visible to user space before taking the device_lock(), this
+can result in a lock inversion.
+
+onlining/offlining of memory should be done via device_online()/
+device_offline() - to make sure it is properly synchronized to actions
+via sysfs. Holding device_hotplug_lock is advised (to e.g. protect online_type)
+
+When adding/removing/onlining/offlining memory or adding/removing
+heterogeneous/device memory, we should always hold the mem_hotplug_lock in
+write mode to serialise memory hotplug (e.g. access to global/zone
+variables).
+
+In addition, mem_hotplug_lock (in contrast to device_hotplug_lock) in read
+mode allows for a quite efficient get_online_mems/put_online_mems
+implementation, so code accessing memory can protect from that memory
+vanishing.
+
+
 Future Work
 ===
 
-- 
2.17.1

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


[PATCH v3 1/6] mm/memory_hotplug: make remove_memory() take the device_hotplug_lock

2018-09-27 Thread David Hildenbrand
remove_memory() is exported right now but requires the
device_hotplug_lock, which is not exported. So let's provide a variant
that takes the lock and only export that one.

The lock is already held in
arch/powerpc/platforms/pseries/hotplug-memory.c
drivers/acpi/acpi_memhotplug.c
arch/powerpc/platforms/powernv/memtrace.c

Apart from that, there are not other users in the tree.

Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: "Rafael J. Wysocki" 
Cc: Len Brown 
Cc: Rashmica Gupta 
Cc: Michael Neuling 
Cc: Balbir Singh 
Cc: Nathan Fontenot 
Cc: John Allen 
Cc: Andrew Morton 
Cc: Michal Hocko 
Cc: Dan Williams 
Cc: Joonsoo Kim 
Cc: Vlastimil Babka 
Cc: Pavel Tatashin 
Cc: Greg Kroah-Hartman 
Cc: Oscar Salvador 
Cc: YASUAKI ISHIMATSU 
Cc: Mathieu Malaterre 
Reviewed-by: Pavel Tatashin 
Reviewed-by: Rafael J. Wysocki 
Reviewed-by: Rashmica Gupta 
Signed-off-by: David Hildenbrand 
---
 arch/powerpc/platforms/powernv/memtrace.c   | 2 +-
 arch/powerpc/platforms/pseries/hotplug-memory.c | 6 +++---
 drivers/acpi/acpi_memhotplug.c  | 2 +-
 include/linux/memory_hotplug.h  | 3 ++-
 mm/memory_hotplug.c | 9 -
 5 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/memtrace.c 
b/arch/powerpc/platforms/powernv/memtrace.c
index a29fdf8a2e56..773623f6bfb1 100644
--- a/arch/powerpc/platforms/powernv/memtrace.c
+++ b/arch/powerpc/platforms/powernv/memtrace.c
@@ -121,7 +121,7 @@ static u64 memtrace_alloc_node(u32 nid, u64 size)
lock_device_hotplug();
end_pfn = base_pfn + nr_pages;
for (pfn = base_pfn; pfn < end_pfn; pfn += bytes>> 
PAGE_SHIFT) {
-   remove_memory(nid, pfn << PAGE_SHIFT, bytes);
+   __remove_memory(nid, pfn << PAGE_SHIFT, bytes);
}
unlock_device_hotplug();
return base_pfn << PAGE_SHIFT;
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c 
b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 9a15d39995e5..dd0264c43f3e 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -305,7 +305,7 @@ static int pseries_remove_memblock(unsigned long base, 
unsigned int memblock_siz
nid = memory_add_physaddr_to_nid(base);
 
for (i = 0; i < sections_per_block; i++) {
-   remove_memory(nid, base, MIN_MEMORY_BLOCK_SIZE);
+   __remove_memory(nid, base, MIN_MEMORY_BLOCK_SIZE);
base += MIN_MEMORY_BLOCK_SIZE;
}
 
@@ -394,7 +394,7 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb)
block_sz = pseries_memory_block_size();
nid = memory_add_physaddr_to_nid(lmb->base_addr);
 
-   remove_memory(nid, lmb->base_addr, block_sz);
+   __remove_memory(nid, lmb->base_addr, block_sz);
 
/* Update memory regions for memory remove */
memblock_remove(lmb->base_addr, block_sz);
@@ -681,7 +681,7 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
 
rc = dlpar_online_lmb(lmb);
if (rc) {
-   remove_memory(nid, lmb->base_addr, block_sz);
+   __remove_memory(nid, lmb->base_addr, block_sz);
invalidate_lmb_associativity_index(lmb);
} else {
lmb->flags |= DRCONF_MEM_ASSIGNED;
diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 6b0d3ef7309c..811148415993 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -282,7 +282,7 @@ static void acpi_memory_remove_memory(struct 
acpi_memory_device *mem_device)
nid = memory_add_physaddr_to_nid(info->start_addr);
 
acpi_unbind_memory_blocks(info);
-   remove_memory(nid, info->start_addr, info->length);
+   __remove_memory(nid, info->start_addr, info->length);
list_del(&info->list);
kfree(info);
}
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 34a28227068d..1f096852f479 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -301,6 +301,7 @@ extern bool is_mem_section_removable(unsigned long pfn, 
unsigned long nr_pages);
 extern void try_offline_node(int nid);
 extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
 extern void remove_memory(int nid, u64 start, u64 size);
+extern void __remove_memory(int nid, u64 start, u64 size);
 
 #else
 static inline bool is_mem_section_removable(unsigned long pfn,
@@ -317,6 +318,7 @@ static inline int offline_pages(unsigned long start_pfn, 
unsigned long nr_pages)
 }
 
 static inline void remove_memory(int nid, u64 start, u64 size) {}
+static inline void __remove_memory(int nid, u64 start, u64 size) {}
 #endif /* CONFIG_MEMORY_HOT

[PATCH v3 3/6] mm/memory_hotplug: fix online/offline_pages called w.o. mem_hotplug_lock

2018-09-27 Thread David Hildenbrand
There seem to be some problems as result of 30467e0b3be ("mm, hotplug:
fix concurrent memory hot-add deadlock"), which tried to fix a possible
lock inversion reported and discussed in [1] due to the two locks
a) device_lock()
b) mem_hotplug_lock

While add_memory() first takes b), followed by a) during
bus_probe_device(), onlining of memory from user space first took a),
followed by b), exposing a possible deadlock.

In [1], and it was decided to not make use of device_hotplug_lock, but
rather to enforce a locking order.

The problems I spotted related to this:

1. Memory block device attributes: While .state first calls
   mem_hotplug_begin() and the calls device_online() - which takes
   device_lock() - .online does no longer call mem_hotplug_begin(), so
   effectively calls online_pages() without mem_hotplug_lock.

2. device_online() should be called under device_hotplug_lock, however
   onlining memory during add_memory() does not take care of that.

In addition, I think there is also something wrong about the locking in

3. arch/powerpc/platforms/powernv/memtrace.c calls offline_pages()
   without locks. This was introduced after 30467e0b3be. And skimming over
   the code, I assume it could need some more care in regards to locking
   (e.g. device_online() called without device_hotplug_lock. This will
   be addressed in the following patches.

Now that we hold the device_hotplug_lock when
- adding memory (e.g. via add_memory()/add_memory_resource())
- removing memory (e.g. via remove_memory())
- device_online()/device_offline()

We can move mem_hotplug_lock usage back into
online_pages()/offline_pages().

Why is mem_hotplug_lock still needed? Essentially to make
get_online_mems()/put_online_mems() be very fast (relying on
device_hotplug_lock would be very slow), and to serialize against
addition of memory that does not create memory block devices (hmm).

[1] http://driverdev.linuxdriverproject.org/pipermail/ driverdev-devel/
2015-February/065324.html

This patch is partly based on a patch by Vitaly Kuznetsov.

Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: "Rafael J. Wysocki" 
Cc: Len Brown 
Cc: Greg Kroah-Hartman 
Cc: "K. Y. Srinivasan" 
Cc: Haiyang Zhang 
Cc: Stephen Hemminger 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
Cc: Boris Ostrovsky 
Cc: Juergen Gross 
Cc: Rashmica Gupta 
Cc: Michael Neuling 
Cc: Balbir Singh 
Cc: Kate Stewart 
Cc: Thomas Gleixner 
Cc: Philippe Ombredanne 
Cc: Andrew Morton 
Cc: Michal Hocko 
Cc: Pavel Tatashin 
Cc: Vlastimil Babka 
Cc: Dan Williams 
Cc: Oscar Salvador 
Cc: YASUAKI ISHIMATSU 
Cc: Mathieu Malaterre 
Reviewed-by: Pavel Tatashin 
Reviewed-by: Rashmica Gupta 
Signed-off-by: David Hildenbrand 
---
 drivers/base/memory.c | 13 +
 mm/memory_hotplug.c   | 28 
 2 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 40cac122ec73..0e5985682642 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -228,7 +228,6 @@ static bool pages_correctly_probed(unsigned long start_pfn)
 /*
  * MEMORY_HOTPLUG depends on SPARSEMEM in mm/Kconfig, so it is
  * OK to have direct references to sparsemem variables in here.
- * Must already be protected by mem_hotplug_begin().
  */
 static int
 memory_block_action(unsigned long phys_index, unsigned long action, int 
online_type)
@@ -294,7 +293,6 @@ static int memory_subsys_online(struct device *dev)
if (mem->online_type < 0)
mem->online_type = MMOP_ONLINE_KEEP;
 
-   /* Already under protection of mem_hotplug_begin() */
ret = memory_block_change_state(mem, MEM_ONLINE, MEM_OFFLINE);
 
/* clear online_type */
@@ -341,19 +339,11 @@ store_mem_state(struct device *dev,
goto err;
}
 
-   /*
-* Memory hotplug needs to hold mem_hotplug_begin() for probe to find
-* the correct memory block to online before doing device_online(dev),
-* which will take dev->mutex.  Take the lock early to prevent an
-* inversion, memory_subsys_online() callbacks will be implemented by
-* assuming it's already protected.
-*/
-   mem_hotplug_begin();
-
switch (online_type) {
case MMOP_ONLINE_KERNEL:
case MMOP_ONLINE_MOVABLE:
case MMOP_ONLINE_KEEP:
+   /* mem->online_type is protected by device_hotplug_lock */
mem->online_type = online_type;
ret = device_online(&mem->dev);
break;
@@ -364,7 +354,6 @@ store_mem_state(struct device *dev,
ret = -EINVAL; /* should never happen */
}
 
-   mem_hotplug_done();
 err:
unlock_device_hotplug();
 
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index affb03e0dfef..d4c7e42e46f3 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -860,7 +860,6 @@ static struct zone * __meminit move_pfn_range(int 
online_type, int ni

[PATCH v3 2/6] mm/memory_hotplug: make add_memory() take the device_hotplug_lock

2018-09-27 Thread David Hildenbrand
add_memory() currently does not take the device_hotplug_lock, however
is aleady called under the lock from
arch/powerpc/platforms/pseries/hotplug-memory.c
drivers/acpi/acpi_memhotplug.c
to synchronize against CPU hot-remove and similar.

In general, we should hold the device_hotplug_lock when adding memory
to synchronize against online/offline request (e.g. from user space) -
which already resulted in lock inversions due to device_lock() and
mem_hotplug_lock - see 30467e0b3be ("mm, hotplug: fix concurrent memory
hot-add deadlock"). add_memory()/add_memory_resource() will create memory
block devices, so this really feels like the right thing to do.

Holding the device_hotplug_lock makes sure that a memory block device
can really only be accessed (e.g. via .online/.state) from user space,
once the memory has been fully added to the system.

The lock is not held yet in
drivers/xen/balloon.c
arch/powerpc/platforms/powernv/memtrace.c
drivers/s390/char/sclp_cmd.c
drivers/hv/hv_balloon.c
So, let's either use the locked variants or take the lock.

Don't export add_memory_resource(), as it once was exported to be used
by XEN, which is never built as a module. If somebody requires it, we
also have to export a locked variant (as device_hotplug_lock is never
exported).

Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: "Rafael J. Wysocki" 
Cc: Len Brown 
Cc: Greg Kroah-Hartman 
Cc: Boris Ostrovsky 
Cc: Juergen Gross 
Cc: Nathan Fontenot 
Cc: John Allen 
Cc: Andrew Morton 
Cc: Michal Hocko 
Cc: Dan Williams 
Cc: Joonsoo Kim 
Cc: Vlastimil Babka 
Cc: Oscar Salvador 
Cc: Mathieu Malaterre 
Cc: Pavel Tatashin 
Cc: YASUAKI ISHIMATSU 
Reviewed-by: Pavel Tatashin 
Reviewed-by: Rafael J. Wysocki 
Reviewed-by: Rashmica Gupta 
Signed-off-by: David Hildenbrand 
---
 .../platforms/pseries/hotplug-memory.c|  2 +-
 drivers/acpi/acpi_memhotplug.c|  2 +-
 drivers/base/memory.c |  9 ++--
 drivers/xen/balloon.c |  3 +++
 include/linux/memory_hotplug.h|  1 +
 mm/memory_hotplug.c   | 22 ---
 6 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c 
b/arch/powerpc/platforms/pseries/hotplug-memory.c
index dd0264c43f3e..d26a771d985e 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -673,7 +673,7 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
nid = memory_add_physaddr_to_nid(lmb->base_addr);
 
/* Add the memory */
-   rc = add_memory(nid, lmb->base_addr, block_sz);
+   rc = __add_memory(nid, lmb->base_addr, block_sz);
if (rc) {
invalidate_lmb_associativity_index(lmb);
return rc;
diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 811148415993..8fe0960ea572 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -228,7 +228,7 @@ static int acpi_memory_enable_device(struct 
acpi_memory_device *mem_device)
if (node < 0)
node = memory_add_physaddr_to_nid(info->start_addr);
 
-   result = add_memory(node, info->start_addr, info->length);
+   result = __add_memory(node, info->start_addr, info->length);
 
/*
 * If the memory block has been used by the kernel, add_memory()
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 817320c7c4c1..40cac122ec73 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -519,15 +519,20 @@ memory_probe_store(struct device *dev, struct 
device_attribute *attr,
if (phys_addr & ((pages_per_block << PAGE_SHIFT) - 1))
return -EINVAL;
 
+   ret = lock_device_hotplug_sysfs();
+   if (ret)
+   goto out;
+
nid = memory_add_physaddr_to_nid(phys_addr);
-   ret = add_memory(nid, phys_addr,
-MIN_MEMORY_BLOCK_SIZE * sections_per_block);
+   ret = __add_memory(nid, phys_addr,
+  MIN_MEMORY_BLOCK_SIZE * sections_per_block);
 
if (ret)
goto out;
 
ret = count;
 out:
+   unlock_device_hotplug();
return ret;
 }
 
diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index a3f5cbfcd4a1..fdfc64f5acea 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -395,7 +395,10 @@ static enum bp_state reserve_additional_memory(void)
 * callers drop the mutex before trying again.
 */
mutex_unlock(&balloon_mutex);
+   /* add_memory_resource() requires the device_hotplug lock */
+   lock_device_hotplug();
rc = add_memory_resource(nid, resource, memhp_auto_online);
+   unlock_device_hotplug();
mutex_lock(&balloon_mutex);
 
if (rc) {
diff --git a/include/linux/mem

[PATCH v3 0/6] mm: online/offline_pages called w.o. mem_hotplug_lock

2018-09-27 Thread David Hildenbrand
@Andrew, Only patch #5 changed (see change notes below). Thanks!


Reading through the code and studying how mem_hotplug_lock is to be used,
I noticed that there are two places where we can end up calling
device_online()/device_offline() - online_pages()/offline_pages() without
the mem_hotplug_lock. And there are other places where we call
device_online()/device_offline() without the device_hotplug_lock.

While e.g.
echo "online" > /sys/devices/system/memory/memory9/state
is fine, e.g.
echo 1 > /sys/devices/system/memory/memory9/online
Will not take the mem_hotplug_lock. However the device_lock() and
device_hotplug_lock.

E.g. via memory_probe_store(), we can end up calling
add_memory()->online_pages() without the device_hotplug_lock. So we can
have concurrent callers in online_pages(). We e.g. touch in online_pages()
basically unprotected zone->present_pages then.

Looks like there is a longer history to that (see Patch #2 for details),
and fixing it to work the way it was intended is not really possible. We
would e.g. have to take the mem_hotplug_lock in device/base/core.c, which
sounds wrong.

Summary: We had a lock inversion on mem_hotplug_lock and device_lock().
More details can be found in patch 3 and patch 6.

I propose the general rules (documentation added in patch 6):

1. add_memory/add_memory_resource() must only be called with
   device_hotplug_lock.
2. remove_memory() must only be called with device_hotplug_lock. This is
   already documented and holds for all callers.
3. device_online()/device_offline() must only be called with
   device_hotplug_lock. This is already documented and true for now in core
   code. Other callers (related to memory hotplug) have to be fixed up.
4. mem_hotplug_lock is taken inside of add_memory/remove_memory/
   online_pages/offline_pages.

To me, this looks way cleaner than what we have right now (and easier to
verify). And looking at the documentation of remove_memory, using
lock_device_hotplug also for add_memory() feels natural.


v2 -> v3:
- Take device_hotplug_lock outside of loop in patch #5
- Added Ack to patch #5

v1 -> v2:
- Upstream changes in powerpc/powernv code required modifications to
  patch #1, #4 and #5.
- Minor patch description changes.
- Added more locking details in patch #6.
- Added rb's

RFCv2 -> v1:
- Dropped an unnecessary _ref from remove_memory() in patch #1
- Minor patch description fixes.
- Added rb's

RFC -> RFCv2:
- Don't export device_hotplug_lock, provide proper remove_memory/add_memory
  wrappers.
- Split up the patches a bit.
- Try to improve powernv memtrace locking
- Add some documentation for locking that matches my knowledg

David Hildenbrand (6):
  mm/memory_hotplug: make remove_memory() take the device_hotplug_lock
  mm/memory_hotplug: make add_memory() take the device_hotplug_lock
  mm/memory_hotplug: fix online/offline_pages called w.o.
mem_hotplug_lock
  powerpc/powernv: hold device_hotplug_lock when calling device_online()
  powerpc/powernv: hold device_hotplug_lock when calling
memtrace_offline_pages()
  memory-hotplug.txt: Add some details about locking internals

 Documentation/memory-hotplug.txt  | 42 -
 arch/powerpc/platforms/powernv/memtrace.c |  8 ++-
 .../platforms/pseries/hotplug-memory.c|  8 +--
 drivers/acpi/acpi_memhotplug.c|  4 +-
 drivers/base/memory.c | 22 +++
 drivers/xen/balloon.c |  3 +
 include/linux/memory_hotplug.h|  4 +-
 mm/memory_hotplug.c   | 59 +++
 8 files changed, 114 insertions(+), 36 deletions(-)

-- 
2.17.1

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


Re: [PATCH] hv_netvsc: Make sure out channel is fully opened on send

2018-09-27 Thread Mohammed Gamal
On Wed, 2018-09-26 at 17:13 +, Haiyang Zhang wrote:
> > -Original Message-
> > From: Mohammed Gamal 
> > Sent: Wednesday, September 26, 2018 12:34 PM
> > To: Stephen Hemminger ; netdev@vger.kernel.
> > org
> > Cc: KY Srinivasan ; Haiyang Zhang
> > ; vkuznets ;
> > ot...@redhat.com; cavery ; linux-
> > ker...@vger.kernel.org; de...@linuxdriverproject.org; Mohammed
> > Gamal
> > 
> > Subject: [PATCH] hv_netvsc: Make sure out channel is fully opened
> > on send
> > 
> > Dring high network traffic changes to network interface parameters
> > such as
> > number of channels or MTU can cause a kernel panic with a NULL
> > pointer
> > dereference. This is due to netvsc_device_remove() being called and
> > deallocating the channel ring buffers, which can then be accessed
> > by
> > netvsc_send_pkt() before they're allocated on calling
> > netvsc_device_add()
> > 
> > The patch fixes this problem by checking the channel state and
> > returning
> > ENODEV if not yet opened. We also move the call to
> > hv_ringbuf_avail_percent()
> > which may access the uninitialized ring buffer.
> > 
> > Signed-off-by: Mohammed Gamal 
> > ---
> >  drivers/net/hyperv/netvsc.c | 7 ++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/hyperv/netvsc.c
> > b/drivers/net/hyperv/netvsc.c index
> > fe01e14..75f1b31 100644
> > --- a/drivers/net/hyperv/netvsc.c
> > +++ b/drivers/net/hyperv/netvsc.c
> > @@ -825,7 +825,12 @@ static inline int netvsc_send_pkt(
> >     struct netdev_queue *txq = netdev_get_tx_queue(ndev,
> > packet->q_idx);
> >     u64 req_id;
> >     int ret;
> > -   u32 ring_avail =
> > hv_get_avail_to_write_percent(&out_channel-
> > > outbound);
> > 
> > +   u32 ring_avail;
> > +
> > +   if (out_channel->state != CHANNEL_OPENED_STATE)
> > +   return -ENODEV;
> > +
> > +   ring_avail = hv_get_avail_to_write_percent(&out_channel-
> > >outbound);
> 
> When you reproducing the NULL ptr panic, does your kernel include the
> following patch?
> hv_netvsc: common detach logic
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/c
> ommit/?id=7b2ee50c0cd513a176a26a71f2989facdd75bfea
> 
Yes it is included. And the commit did reduce the occurrence of this
race condition, but it still nevertheless occurs albeit rarely.

> We call netif_tx_disable(ndev) and netif_device_detach(ndev) before
> doing the changes 
> on MTU or #channels. So there should be no call to start_xmit() when
> channel is not ready.
> 
> If you see the check for CHANNEL_OPENED_STATE is still necessary on
> upstream kernel (including 
> the patch " common detach logic "), we should debug further on the
> code and find out the 
> root cause.
> 
> Thanks,
> - Haiyang
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 2/2] mmc: Add mmc_force_detect_change_begin / _end functions

2018-09-27 Thread Maxime Ripard
On Wed, Sep 26, 2018 at 10:19:22PM +0200, Hans de Goede wrote:
> On 26-09-18 16:44, Frieder Schrempf wrote:
> > Hi,
> > 
> > On Fri, Feb 09, 2018 at 03:01:00PM +0100, Ulf Hansson wrote:
> > > [...]
> > > 
> > > >> > I'd like to know if any progress has been made on that problem
> > (I may
> > > >> > have missed patches).
> > > >> > Had you had the time to look at the issue?
> > > >>
> > > >> I have looked at the issue, but not manage to cook some patches
> > for it.
> > > >>
> > > >> However, it's on my top of my TODO list for mmc. No promises, but
> > > >> perhaps and hopefully I manage to get something posted during the
> > > >> coming release cycle.
> > 
> > I would be interested in a ESP8089 driver in mainline and that's why I want 
> > to pick up this discussion.
> > 
> > What is the current status of the "mmc_reprobe_device" implementation, that 
> > Hans was explaining and Ulf wanted to provide some months ago?
> 
> Ulf did eventually write a new way to deal with this and then Quentin
> did manage to get the esp8089 driver to work with it, the new function
> to use for this is added by this commit:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mmc/core?id=1433269c4d2461be1f36db5dbb453976b38996ff
> 
> I'm not sure what the status of upstreaming the ep8089 driver is now
> that we've this in place.
> 
> Quentin, do you have a version of the esp8089 driver somewhere
> which will work correctly with the new mmc_sw_reset() function?
> 
> Also what is the status of adding this driver to say staging?

IIRC, we tried to get it into staging, and we got told that it was too
nice for staging at this point. So we're basically stuck somewhere
between staging and !staging, with the driver being too nice for the
former, and not nice enough for the latter :)

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v3] memory_hotplug: Free pages as higher order

2018-09-27 Thread Arun KS

On 2018-09-27 12:41, Juergen Gross wrote:

On 27/09/18 08:58, Arun KS wrote:

When free pages are done with higher order, time spend on
coalescing pages by buddy allocator can be reduced. With
section size of 256MB, hot add latency of a single section
shows improvement from 50-60 ms to less than 1 ms, hence
improving the hot add latency by 60%.

Modify external providers of online callback to align with
the change.

Signed-off-by: Arun KS 
---
Changes since v2:
reuse code from __free_pages_boot_core()

Changes since v1:
- Removed prefetch()

Changes since RFC:
- Rebase.
- As suggested by Michal Hocko remove pages_per_block.
- Modifed external providers of online_page_callback.

v2: https://lore.kernel.org/patchwork/patch/991363/
v1: https://lore.kernel.org/patchwork/patch/989445/
RFC: https://lore.kernel.org/patchwork/patch/984754/

---
 drivers/hv/hv_balloon.c|  6 --
 drivers/xen/balloon.c  | 18 ++---
 include/linux/memory_hotplug.h |  2 +-
 mm/internal.h  |  1 +
 mm/memory_hotplug.c| 44 
++

 mm/page_alloc.c|  2 +-
 6 files changed, 54 insertions(+), 19 deletions(-)



...


diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index e12bb25..010cf4d 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -390,8 +390,8 @@ static enum bp_state 
reserve_additional_memory(void)


/*
 * add_memory_resource() will call online_pages() which in its turn
-* will call xen_online_page() callback causing deadlock if we don't
-* release balloon_mutex here. Unlocking here is safe because the
+* will call xen_bring_pgs_online() callback causing deadlock if we
+	 * don't release balloon_mutex here. Unlocking here is safe because 
the

 * callers drop the mutex before trying again.
 */
mutex_unlock(&balloon_mutex);
@@ -422,6 +422,18 @@ static void xen_online_page(struct page *page)
mutex_unlock(&balloon_mutex);
 }

+static int xen_bring_pgs_online(struct page *pg, unsigned int order)
+{
+   unsigned long i, size = (1 << order);
+   unsigned long start_pfn = page_to_pfn(pg);
+
+	pr_debug("Online %lu pages starting at pfn 0x%lx\n", size, 
start_pfn);

+   for (i = 0; i < size; i++)
+   xen_online_page(pfn_to_page(start_pfn + i));




Hi,


xen_online_page() isn't very complex and this is the only user.

Why don't you move its body in here and drop the extra function?
And now you can execute the loop with balloon_mutex held instead of
taking and releasing it in each iteration of the loop.

Point taken. Will incorporate them.

Regards,
Arun



Juergen

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


Re: [PATCH v3] memory_hotplug: Free pages as higher order

2018-09-27 Thread Arun KS

On 2018-09-27 12:39, Oscar Salvador wrote:

On Thu, Sep 27, 2018 at 12:28:50PM +0530, Arun KS wrote:

+   __free_pages_boot_core(page, order);



Hi,


I am not sure, but if we are going to use that function from the
memory-hotplug code,
we might want to rename that function to something more generic?
The word "boot" suggests that this is only called from the boot stage.

I ll rename it to __free_pages_core()



And what about the prefetch operations?
I saw that you removed them in your previous patch and that had some
benefits [1].

Should we remove them here as well?

Sure. Will update this as well.

Thanks,
Arun


[1] https://patchwork.kernel.org/patch/10613359/

Thanks

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


Re: [PATCH v3] memory_hotplug: Free pages as higher order

2018-09-27 Thread Juergen Gross
On 27/09/18 08:58, Arun KS wrote:
> When free pages are done with higher order, time spend on
> coalescing pages by buddy allocator can be reduced. With
> section size of 256MB, hot add latency of a single section
> shows improvement from 50-60 ms to less than 1 ms, hence
> improving the hot add latency by 60%.
> 
> Modify external providers of online callback to align with
> the change.
> 
> Signed-off-by: Arun KS 
> ---
> Changes since v2:
> reuse code from __free_pages_boot_core()
> 
> Changes since v1:
> - Removed prefetch()
> 
> Changes since RFC:
> - Rebase.
> - As suggested by Michal Hocko remove pages_per_block.
> - Modifed external providers of online_page_callback.
> 
> v2: https://lore.kernel.org/patchwork/patch/991363/
> v1: https://lore.kernel.org/patchwork/patch/989445/
> RFC: https://lore.kernel.org/patchwork/patch/984754/
> 
> ---
>  drivers/hv/hv_balloon.c|  6 --
>  drivers/xen/balloon.c  | 18 ++---
>  include/linux/memory_hotplug.h |  2 +-
>  mm/internal.h  |  1 +
>  mm/memory_hotplug.c| 44 
> ++
>  mm/page_alloc.c|  2 +-
>  6 files changed, 54 insertions(+), 19 deletions(-)
>

...

> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index e12bb25..010cf4d 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -390,8 +390,8 @@ static enum bp_state reserve_additional_memory(void)
>  
>   /*
>* add_memory_resource() will call online_pages() which in its turn
> -  * will call xen_online_page() callback causing deadlock if we don't
> -  * release balloon_mutex here. Unlocking here is safe because the
> +  * will call xen_bring_pgs_online() callback causing deadlock if we
> +  * don't release balloon_mutex here. Unlocking here is safe because the
>* callers drop the mutex before trying again.
>*/
>   mutex_unlock(&balloon_mutex);
> @@ -422,6 +422,18 @@ static void xen_online_page(struct page *page)
>   mutex_unlock(&balloon_mutex);
>  }
>  
> +static int xen_bring_pgs_online(struct page *pg, unsigned int order)
> +{
> + unsigned long i, size = (1 << order);
> + unsigned long start_pfn = page_to_pfn(pg);
> +
> + pr_debug("Online %lu pages starting at pfn 0x%lx\n", size, start_pfn);
> + for (i = 0; i < size; i++)
> + xen_online_page(pfn_to_page(start_pfn + i));

xen_online_page() isn't very complex and this is the only user.

Why don't you move its body in here and drop the extra function?
And now you can execute the loop with balloon_mutex held instead of
taking and releasing it in each iteration of the loop.


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


Re: [PATCH v3] memory_hotplug: Free pages as higher order

2018-09-27 Thread Oscar Salvador
On Thu, Sep 27, 2018 at 12:28:50PM +0530, Arun KS wrote:
> + __free_pages_boot_core(page, order);

I am not sure, but if we are going to use that function from the memory-hotplug 
code,
we might want to rename that function to something more generic?
The word "boot" suggests that this is only called from the boot stage.

And what about the prefetch operations? 
I saw that you removed them in your previous patch and that had some benefits 
[1].

Should we remove them here as well?

[1] https://patchwork.kernel.org/patch/10613359/ 

Thanks
-- 
Oscar Salvador
SUSE L3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v3] memory_hotplug: Free pages as higher order

2018-09-27 Thread Arun KS
When free pages are done with higher order, time spend on
coalescing pages by buddy allocator can be reduced. With
section size of 256MB, hot add latency of a single section
shows improvement from 50-60 ms to less than 1 ms, hence
improving the hot add latency by 60%.

Modify external providers of online callback to align with
the change.

Signed-off-by: Arun KS 
---
Changes since v2:
reuse code from __free_pages_boot_core()

Changes since v1:
- Removed prefetch()

Changes since RFC:
- Rebase.
- As suggested by Michal Hocko remove pages_per_block.
- Modifed external providers of online_page_callback.

v2: https://lore.kernel.org/patchwork/patch/991363/
v1: https://lore.kernel.org/patchwork/patch/989445/
RFC: https://lore.kernel.org/patchwork/patch/984754/

---
 drivers/hv/hv_balloon.c|  6 --
 drivers/xen/balloon.c  | 18 ++---
 include/linux/memory_hotplug.h |  2 +-
 mm/internal.h  |  1 +
 mm/memory_hotplug.c| 44 ++
 mm/page_alloc.c|  2 +-
 6 files changed, 54 insertions(+), 19 deletions(-)

diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index b1b7880..c5bc0b5 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -771,7 +771,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned 
long size,
}
 }
 
-static void hv_online_page(struct page *pg)
+static int hv_online_page(struct page *pg, unsigned int order)
 {
struct hv_hotadd_state *has;
unsigned long flags;
@@ -783,10 +783,12 @@ static void hv_online_page(struct page *pg)
if ((pfn < has->start_pfn) || (pfn >= has->end_pfn))
continue;
 
-   hv_page_online_one(has, pg);
+   hv_bring_pgs_online(has, pfn, (1UL << order));
break;
}
spin_unlock_irqrestore(&dm_device.ha_lock, flags);
+
+   return 0;
 }
 
 static int pfn_covered(unsigned long start_pfn, unsigned long pfn_cnt)
diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index e12bb25..010cf4d 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -390,8 +390,8 @@ static enum bp_state reserve_additional_memory(void)
 
/*
 * add_memory_resource() will call online_pages() which in its turn
-* will call xen_online_page() callback causing deadlock if we don't
-* release balloon_mutex here. Unlocking here is safe because the
+* will call xen_bring_pgs_online() callback causing deadlock if we
+* don't release balloon_mutex here. Unlocking here is safe because the
 * callers drop the mutex before trying again.
 */
mutex_unlock(&balloon_mutex);
@@ -422,6 +422,18 @@ static void xen_online_page(struct page *page)
mutex_unlock(&balloon_mutex);
 }
 
+static int xen_bring_pgs_online(struct page *pg, unsigned int order)
+{
+   unsigned long i, size = (1 << order);
+   unsigned long start_pfn = page_to_pfn(pg);
+
+   pr_debug("Online %lu pages starting at pfn 0x%lx\n", size, start_pfn);
+   for (i = 0; i < size; i++)
+   xen_online_page(pfn_to_page(start_pfn + i));
+
+   return 0;
+}
+
 static int xen_memory_notifier(struct notifier_block *nb, unsigned long val, 
void *v)
 {
if (val == MEM_ONLINE)
@@ -744,7 +756,7 @@ static int __init balloon_init(void)
balloon_stats.max_retry_count = RETRY_UNLIMITED;
 
 #ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
-   set_online_page_callback(&xen_online_page);
+   set_online_page_callback(&xen_bring_pgs_online);
register_memory_notifier(&xen_memory_nb);
register_sysctl_table(xen_root);
 
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 34a2822..7b04c1d 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -87,7 +87,7 @@ extern int test_pages_in_a_zone(unsigned long start_pfn, 
unsigned long end_pfn,
unsigned long *valid_start, unsigned long *valid_end);
 extern void __offline_isolated_pages(unsigned long, unsigned long);
 
-typedef void (*online_page_callback_t)(struct page *page);
+typedef int (*online_page_callback_t)(struct page *page, unsigned int order);
 
 extern int set_online_page_callback(online_page_callback_t callback);
 extern int restore_online_page_callback(online_page_callback_t callback);
diff --git a/mm/internal.h b/mm/internal.h
index 87256ae..2b0efac 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -163,6 +163,7 @@ static inline struct page *pageblock_pfn_to_page(unsigned 
long start_pfn,
 extern int __isolate_free_page(struct page *page, unsigned int order);
 extern void __free_pages_bootmem(struct page *page, unsigned long pfn,
unsigned int order);
+extern void __free_pages_boot_core(struct page *page, unsigned int order);
 extern void prep_compound_page(struct page *page, unsigned int order);
 extern void post_alloc_hook