[PATCH 12/16] staging: lustre: Remove duplicate header file inclusion in lmv_fld.c

2013-07-25 Thread Sachin Kamat
Removed the header file included twice.

Signed-off-by: Sachin Kamat 
---
 drivers/staging/lustre/lustre/lmv/lmv_fld.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/lmv/lmv_fld.c 
b/drivers/staging/lustre/lustre/lmv/lmv_fld.c
index a4805ae..08a5b69 100644
--- a/drivers/staging/lustre/lustre/lmv/lmv_fld.c
+++ b/drivers/staging/lustre/lustre/lmv/lmv_fld.c
@@ -38,7 +38,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
-- 
1.7.9.5

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


[PATCH 07/16] staging: lustre: Remove duplicate header file inclusion in dir.c

2013-07-25 Thread Sachin Kamat
Removed the header files included twice.

Signed-off-by: Sachin Kamat 
---
 drivers/staging/lustre/lustre/llite/dir.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/llite/dir.c 
b/drivers/staging/lustre/lustre/llite/dir.c
index 002b374..2ca8c45 100644
--- a/drivers/staging/lustre/lustre/llite/dir.c
+++ b/drivers/staging/lustre/lustre/llite/dir.c
@@ -48,7 +48,6 @@
 
 #define DEBUG_SUBSYSTEM S_LLITE
 
-#include 
 #include 
 #include 
 #include 
-- 
1.7.9.5

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


[PATCH 04/16] staging: lustre: linux-debug: Remove duplicate inclusion of header file

2013-07-25 Thread Sachin Kamat
Removed the header files included twice.

Signed-off-by: Sachin Kamat 
---
 .../lustre/lustre/libcfs/linux/linux-debug.c   |3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/lustre/lustre/libcfs/linux/linux-debug.c 
b/drivers/staging/lustre/lustre/libcfs/linux/linux-debug.c
index 0069b5e..6afb350 100644
--- a/drivers/staging/lustre/lustre/libcfs/linux/linux-debug.c
+++ b/drivers/staging/lustre/lustre/libcfs/linux/linux-debug.c
@@ -48,11 +48,8 @@
 #include 
 #include 
 #include 
-#include 
 #include 
-
 #include 
-#include 
 #include 
 #include 
 
-- 
1.7.9.5

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


[PATCH 01/16] staging: lustre: o2iblnd: Remove duplicate inclusion of header file

2013-07-25 Thread Sachin Kamat
Removed the header files included twice.

Signed-off-by: Sachin Kamat 
---
 .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h|1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h 
b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h
index e4626bf..938df0c 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h
@@ -53,7 +53,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
-- 
1.7.9.5

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


[PATCH 15/16] staging: lustre: Remove duplicate header file inclusion in lvfs_linux.c

2013-07-25 Thread Sachin Kamat
Removed the header file included twice.

Signed-off-by: Sachin Kamat 
---
 drivers/staging/lustre/lustre/lvfs/lvfs_linux.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/lvfs/lvfs_linux.c 
b/drivers/staging/lustre/lustre/lvfs/lvfs_linux.c
index 9d19d0a..0f38791 100644
--- a/drivers/staging/lustre/lustre/lvfs/lvfs_linux.c
+++ b/drivers/staging/lustre/lustre/lvfs/lvfs_linux.c
@@ -46,7 +46,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
-- 
1.7.9.5

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


Re: [PATCH v4] staging: usbip: replace pr_warning() with dev_warn().

2013-07-25 Thread Greg KH
On Thu, Jul 25, 2013 at 10:19:31AM +0530, navin patidar wrote:
> >> -pr_warning("Unable to start control thread\n");
> >> +struct device *dev;
> >> +
> >> +if (ud->side == USBIP_STUB)
> >> +dev = &container_of(ud, struct stub_device, ud)->udev->dev;
> >> +else
> >> +dev = &container_of(ud, struct vhci_device, ud)->udev->dev;
> >
> > Putting '&' in front of container_of seems odd, are you sure it's needed
> > here?  If ud is a pointer, everything else should "just work" properly
> > without that.
> 
> Removing '&'   caused following error.
> drivers/staging/usbip/usbip_event.c: In function ‘usbip_start_eh’:
> drivers/staging/usbip/usbip_event.c:93:8: error: incompatible types
> when assigning to type ‘struct device *’ from type ‘struct device’
> drivers/staging/usbip/usbip_event.c:95:8: error: incompatible types
> when assigning to type ‘struct device *’ from type ‘struct device’
> 
> dev needs to be initialized with address of dev (struct device ) which
> is member of struct usb_device pointed by the udev.
> 
> To make it more clear i can change it to
> 
> dev = &(container_of(ud, struct vhci_device, ud)->udev->dev);
> 
>  or
> 
> struct vhci_device *vdev = container_of(ud, struct vhci_device, ud);
> dev = &(vdev->udev->dev);

Or perhaps:
dev = container_of(ud, struct stub_device, ud).udev->dev;

or ->udev.dev; I don't remember which, but that should work, right?

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


[PATCH 14/16] staging: lustre: Remove duplicate header file inclusion in lmv_obd.c

2013-07-25 Thread Sachin Kamat
Removed the header file included twice.

Signed-off-by: Sachin Kamat 
---
 drivers/staging/lustre/lustre/lmv/lmv_obd.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/lmv/lmv_obd.c 
b/drivers/staging/lustre/lustre/lmv/lmv_obd.c
index 11c8d64..1987d4b 100644
--- a/drivers/staging/lustre/lustre/lmv/lmv_obd.c
+++ b/drivers/staging/lustre/lustre/lmv/lmv_obd.c
@@ -38,7 +38,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
-- 
1.7.9.5

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


[PATCH 13/16] staging: lustre: Remove duplicate header file inclusion in lmv_intent.c

2013-07-25 Thread Sachin Kamat
Removed the header file included twice.

Signed-off-by: Sachin Kamat 
---
 drivers/staging/lustre/lustre/lmv/lmv_intent.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/lmv/lmv_intent.c 
b/drivers/staging/lustre/lustre/lmv/lmv_intent.c
index 7eefab5..4bc8c6a 100644
--- a/drivers/staging/lustre/lustre/lmv/lmv_intent.c
+++ b/drivers/staging/lustre/lustre/lmv/lmv_intent.c
@@ -38,7 +38,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
-- 
1.7.9.5

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


[PATCH 10/16] staging: lustre: Remove duplicate header file inclusion in rw.c

2013-07-25 Thread Sachin Kamat
Removed the header files included twice.

Signed-off-by: Sachin Kamat 
---
 drivers/staging/lustre/lustre/llite/rw.c |3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/rw.c 
b/drivers/staging/lustre/lustre/llite/rw.c
index 43b5f0d2..ab4e95c 100644
--- a/drivers/staging/lustre/lustre/llite/rw.c
+++ b/drivers/staging/lustre/lustre/llite/rw.c
@@ -48,9 +48,6 @@
 #include 
 
 #include 
-#include 
-#include 
-#include 
 #include 
 /* current_is_kswapd() */
 #include 
-- 
1.7.9.5

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


[PATCH] staging/lustre: add BLOCK depends in Kconfig

2013-07-25 Thread Xiong Zhou
From: Xiong Zhou 

Add BLOCK depends in Kconfig for LUSTRE to fix this:
drivers/staging/lustre/lustre/fid/../include/linux/lustre_compat25.h:117:2: 
error: implicit declaration of function ‘unregister_blkdev’ 

Signed-off-by: Xiong Zhou 
---
 drivers/staging/lustre/lustre/Kconfig |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/Kconfig 
b/drivers/staging/lustre/lustre/Kconfig
index 9ae7fa8..0b45de0 100644
--- a/drivers/staging/lustre/lustre/Kconfig
+++ b/drivers/staging/lustre/lustre/Kconfig
@@ -1,6 +1,6 @@
 config LUSTRE_FS
tristate "Lustre file system client support"
-   depends on STAGING && INET && m
+   depends on STAGING && INET && BLOCK && m
select LNET
select CRYPTO
select CRYPTO_CRC32___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 09/16] staging: lustre: Remove duplicate header file inclusion in lloop.c

2013-07-25 Thread Sachin Kamat
Removed the header files included twice.

Signed-off-by: Sachin Kamat 
---
 drivers/staging/lustre/lustre/llite/lloop.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/llite/lloop.c 
b/drivers/staging/lustre/lustre/llite/lloop.c
index 9d4c17e..e063efc 100644
--- a/drivers/staging/lustre/lustre/llite/lloop.c
+++ b/drivers/staging/lustre/lustre/llite/lloop.c
@@ -99,7 +99,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 #include 
-- 
1.7.9.5

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


[PATCH 05/16] staging: lustre: libcfs: Remove duplicate inclusion of header file

2013-07-25 Thread Sachin Kamat
Removed the header files included twice.

Signed-off-by: Sachin Kamat 
---
 .../lustre/lustre/libcfs/linux/linux-proc.c|2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/libcfs/linux/linux-proc.c 
b/drivers/staging/lustre/lustre/libcfs/linux/linux-proc.c
index 522b28e..80a9fab 100644
--- a/drivers/staging/lustre/lustre/libcfs/linux/linux-proc.c
+++ b/drivers/staging/lustre/lustre/libcfs/linux/linux-proc.c
@@ -54,9 +54,7 @@
 
 #include 
 #include 
-#include 
 #include 
-#include 
 
 #include 
 #include 
-- 
1.7.9.5

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


[PATCH 02/16] staging: lustre: socklnd: Remove duplicate inclusion of header files

2013-07-25 Thread Sachin Kamat
Removed the header files included twice.

Signed-off-by: Sachin Kamat 
---
 .../lustre/lnet/klnds/socklnd/socklnd_lib-linux.h  |2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_lib-linux.h 
b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_lib-linux.h
index 384eb7c..1cfc1b1 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_lib-linux.h
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_lib-linux.h
@@ -57,11 +57,9 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
-- 
1.7.9.5

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


[PATCH 06/16] staging: lustre: linux-tcpip: Remove duplicate header file inclusion

2013-07-25 Thread Sachin Kamat
Removed the header files included twice.

Signed-off-by: Sachin Kamat 
---
 .../lustre/lustre/libcfs/linux/linux-tcpip.c   |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/libcfs/linux/linux-tcpip.c 
b/drivers/staging/lustre/lustre/libcfs/linux/linux-tcpip.c
index f5e78b5..e6069d7 100644
--- a/drivers/staging/lustre/lustre/libcfs/linux/linux-tcpip.c
+++ b/drivers/staging/lustre/lustre/libcfs/linux/linux-tcpip.c
@@ -36,7 +36,6 @@
 #define DEBUG_SUBSYSTEM S_LNET
 
 #include 
-#include 
 
 #include 
 #include 
-- 
1.7.9.5

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


[PATCH 03/16] staging: lustre: obd: Remove duplicate inclusion of header file

2013-07-25 Thread Sachin Kamat
Removed the header file included twice.

Signed-off-by: Sachin Kamat 
---
 drivers/staging/lustre/lustre/include/obd.h |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/include/obd.h 
b/drivers/staging/lustre/lustre/include/obd.h
index 094eb96..0034664 100644
--- a/drivers/staging/lustre/lustre/include/obd.h
+++ b/drivers/staging/lustre/lustre/include/obd.h
@@ -52,7 +52,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
-- 
1.7.9.5

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


[PATCH 11/16] staging: lustre: Remove duplicate header file inclusion in rw26.c

2013-07-25 Thread Sachin Kamat
Removed the header files included twice.

Signed-off-by: Sachin Kamat 
---
 drivers/staging/lustre/lustre/llite/rw26.c |3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/rw26.c 
b/drivers/staging/lustre/lustre/llite/rw26.c
index 97088e6..256ee63 100644
--- a/drivers/staging/lustre/lustre/llite/rw26.c
+++ b/drivers/staging/lustre/lustre/llite/rw26.c
@@ -51,9 +51,6 @@
 #include 
 #include 
 #include 
-#include 
-#include 
-#include 
 #include 
 
 #define DEBUG_SUBSYSTEM S_LLITE
-- 
1.7.9.5

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


[PATCH 08/16] staging: lustre: Remove duplicate header file inclusion in llite_mmap.c

2013-07-25 Thread Sachin Kamat
Removed the header files included twice.

Signed-off-by: Sachin Kamat 
---
 drivers/staging/lustre/lustre/llite/llite_mmap.c |3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/llite_mmap.c 
b/drivers/staging/lustre/lustre/llite/llite_mmap.c
index a4061ee..b59cbd7 100644
--- a/drivers/staging/lustre/lustre/llite/llite_mmap.c
+++ b/drivers/staging/lustre/lustre/llite/llite_mmap.c
@@ -43,9 +43,6 @@
 #include 
 
 #include 
-#include 
-#include 
-#include 
 #include 
 
 #define DEBUG_SUBSYSTEM S_LLITE
-- 
1.7.9.5

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


[PATCH 16/16] staging: lustre: obdclass: Remove duplicate header file inclusion

2013-07-25 Thread Sachin Kamat
Removed the header file included twice.

Signed-off-by: Sachin Kamat 
---
 .../lustre/lustre/obdclass/linux/linux-sysctl.c|1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/obdclass/linux/linux-sysctl.c 
b/drivers/staging/lustre/lustre/obdclass/linux/linux-sysctl.c
index 8fe6a01..c4a7402 100644
--- a/drivers/staging/lustre/lustre/obdclass/linux/linux-sysctl.c
+++ b/drivers/staging/lustre/lustre/obdclass/linux/linux-sysctl.c
@@ -38,7 +38,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
-- 
1.7.9.5

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


Re: [PATCH v4] staging: usbip: replace pr_warning() with dev_warn().

2013-07-25 Thread navin patidar
On Thu, Jul 25, 2013, Greg KH  said:

> On Thu, Jul 25, 2013 at 10:19:31AM +0530, navin patidar wrote:
>> >> -pr_warning("Unable to start control thread\n");
>> >> +struct device *dev;
>> >> +
>> >> +if (ud->side == USBIP_STUB)
>> >> +dev = &container_of(ud, struct stub_device, ud)->udev->dev;
>> >> +else
>> >> +dev = &container_of(ud, struct vhci_device, ud)->udev->dev;
>> >
>> > Putting '&' in front of container_of seems odd, are you sure it's needed
>> > here?  If ud is a pointer, everything else should "just work" properly
>> > without that.
>>
>> Removing '&'   caused following error.
>> drivers/staging/usbip/usbip_event.c: In function ‘usbip_start_eh’:
>> drivers/staging/usbip/usbip_event.c:93:8: error: incompatible types
>> when assigning to type ‘struct device *’ from type ‘struct device’
>> drivers/staging/usbip/usbip_event.c:95:8: error: incompatible types
>> when assigning to type ‘struct device *’ from type ‘struct device’
>>
>> dev needs to be initialized with address of dev (struct device ) which
>> is member of struct usb_device pointed by the udev.
>>
>> To make it more clear i can change it to
>>
>> dev = &(container_of(ud, struct vhci_device, ud)->udev->dev);
>>
>>  or
>>
>> struct vhci_device *vdev = container_of(ud, struct vhci_device, ud);
>> dev = &(vdev->udev->dev);
>
> Or perhaps:
> dev = container_of(ud, struct stub_device, ud).udev->dev;

 container_of() returns stub_device pointer so "container_of(ud,
struct stub_device, ud).udev->dev"  won't  work.

> or ->udev.dev; I don't remember which, but that should work, right?
udev is also a pointer to usb_device structure inside  stub_device structure.
->udev->dev   only will work.

v4 patch gets compiled without any error or warning.
and actually Joe Perches reviewed v3 of the patch and suggested changes.
https://lkml.org/lkml/2013/6/27/15

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


[PATCH 6/7] staging: gdm724x: Remove version.h header inclusion in gdm_usb.c

2013-07-25 Thread Sachin Kamat
version.h header inclusion is not necessary as detected by
versioncheck.

Signed-off-by: Sachin Kamat 
---
 drivers/staging/gdm724x/gdm_usb.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/gdm724x/gdm_usb.c 
b/drivers/staging/gdm724x/gdm_usb.c
index 2e658f8..4b10f238 100644
--- a/drivers/staging/gdm724x/gdm_usb.c
+++ b/drivers/staging/gdm724x/gdm_usb.c
@@ -14,7 +14,6 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include 
-#include 
 #include 
 #include 
 #include 
-- 
1.7.9.5

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


[PATCH 7/7] staging: gdm724x: Remove version.h header inclusion in gdm_usb.h

2013-07-25 Thread Sachin Kamat
version.h header inclusion is not necessary as detected by
versioncheck.

Signed-off-by: Sachin Kamat 
---
 drivers/staging/gdm724x/gdm_usb.h |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/gdm724x/gdm_usb.h 
b/drivers/staging/gdm724x/gdm_usb.h
index 38f6537..e6486e7 100644
--- a/drivers/staging/gdm724x/gdm_usb.h
+++ b/drivers/staging/gdm724x/gdm_usb.h
@@ -14,7 +14,6 @@
 #ifndef _GDM_USB_H_
 #define _GDM_USB_H_
 
-#include 
 #include 
 #include 
 #include 
-- 
1.7.9.5

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


[PATCH 3/7] staging: gdm724x: Remove version.h header inclusion in gdm_mux.c

2013-07-25 Thread Sachin Kamat
version.h header inclusion is not necessary as detected by
versioncheck.

Signed-off-by: Sachin Kamat 
---
 drivers/staging/gdm724x/gdm_mux.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/gdm724x/gdm_mux.c 
b/drivers/staging/gdm724x/gdm_mux.c
index f570bc0..2e619d2 100644
--- a/drivers/staging/gdm724x/gdm_mux.c
+++ b/drivers/staging/gdm724x/gdm_mux.c
@@ -14,7 +14,6 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include 
-#include 
 #include 
 #include 
 #include 
-- 
1.7.9.5

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


[PATCH 2/7] staging: gdm724x: Remove version.h header inclusion in gdm_lte.h

2013-07-25 Thread Sachin Kamat
version.h header inclusion is not necessary as detected by
versioncheck.

Signed-off-by: Sachin Kamat 
---
 drivers/staging/gdm724x/gdm_lte.h |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/gdm724x/gdm_lte.h 
b/drivers/staging/gdm724x/gdm_lte.h
index 90c9d51..9287d31 100644
--- a/drivers/staging/gdm724x/gdm_lte.h
+++ b/drivers/staging/gdm724x/gdm_lte.h
@@ -15,7 +15,6 @@
 #define _GDM_LTE_H_
 
 #include 
-#include 
 #include 
 
 #include "gdm_endian.h"
-- 
1.7.9.5

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


[PATCH 4/7] staging: gdm724x: Remove version.h header inclusion in gdm_tty.c

2013-07-25 Thread Sachin Kamat
version.h header inclusion is not necessary as detected by
versioncheck.

Signed-off-by: Sachin Kamat 
---
 drivers/staging/gdm724x/gdm_tty.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/gdm724x/gdm_tty.c 
b/drivers/staging/gdm724x/gdm_tty.c
index 357daa8..912022b 100644
--- a/drivers/staging/gdm724x/gdm_tty.c
+++ b/drivers/staging/gdm724x/gdm_tty.c
@@ -13,7 +13,6 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include 
 #include 
 #include 
 #include 
-- 
1.7.9.5

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


[PATCH 5/7] staging: gdm724x: Remove version.h header inclusion in gdm_tty.h

2013-07-25 Thread Sachin Kamat
version.h header inclusion is not necessary as detected by
versioncheck.

Signed-off-by: Sachin Kamat 
---
 drivers/staging/gdm724x/gdm_tty.h |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/gdm724x/gdm_tty.h 
b/drivers/staging/gdm724x/gdm_tty.h
index faeaa41..a75be1d 100644
--- a/drivers/staging/gdm724x/gdm_tty.h
+++ b/drivers/staging/gdm724x/gdm_tty.h
@@ -14,7 +14,6 @@
 #ifndef _GDM_TTY_H_
 #define _GDM_TTY_H_
 
-#include 
 #include 
 #include 
 
-- 
1.7.9.5

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


Re: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining memory blocks

2013-07-25 Thread Michal Hocko
On Wed 24-07-13 14:02:32, Dave Hansen wrote:
> On 07/24/2013 12:45 PM, KY Srinivasan wrote:
> > All I am saying is that I see two classes of failures: (a) Our
> > inability to allocate memory to manage the memory that is being hot added
> > and (b) Our inability to bring the hot added memory online within a 
> > reasonable
> > amount of time. I am not sure the cause for (b) and I was just speculating 
> > that
> > this could be memory related. What is interesting is that I have seen 
> > failure related
> > to our inability to online the memory after having succeeded in hot adding 
> > the
> > memory.
> 
> I think we should hold off on this patch and other like it until we've
> been sufficiently able to explain how (b) happens.

Agreed.

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


Re: [PATCH v4] staging: usbip: replace pr_warning() with dev_warn().

2013-07-25 Thread navin patidar
On Wed, Jul 24, 2013, Greg KH  said:

> On Thu, Jun 27, 2013 at 03:34:52PM +0530, navin patidar wrote:
>> dev_warn() is preferred over pr_warning().
>>
>> container_of() is used to get usb_driver pointer from usbip_device container
>> (stub_device or vhci_device), to get device structure required for 
>> dev_warn().
>>
>> Signed-off-by: navin patidar 
>> ---
>>  drivers/staging/usbip/usbip_event.c |   11 ++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/usbip/usbip_event.c 
>> b/drivers/staging/usbip/usbip_event.c
>> index 82123be..1f3a571 100644
>> --- a/drivers/staging/usbip/usbip_event.c
>> +++ b/drivers/staging/usbip/usbip_event.c
>> @@ -21,6 +21,8 @@
>>  #include 
>>
>>  #include "usbip_common.h"
>> +#include "stub.h"
>> +#include "vhci.h"
>>
>>  static int event_handler(struct usbip_device *ud)
>>  {
>> @@ -85,7 +87,14 @@ int usbip_start_eh(struct usbip_device *ud)
>>
>>  ud->eh = kthread_run(event_handler_loop, ud, "usbip_eh");
>>  if (IS_ERR(ud->eh)) {
>> -pr_warning("Unable to start control thread\n");
>> +struct device *dev;
>> +
>> +if (ud->side == USBIP_STUB)
>> +dev = &container_of(ud, struct stub_device, ud)->udev->dev;
>> +else
>> +dev = &container_of(ud, struct vhci_device, ud)->udev->dev;
>
> Putting '&' in front of container_of seems odd, are you sure it's needed
> here?  If ud is a pointer, everything else should "just work" properly
> without that.

Removing '&'   caused following error.
drivers/staging/usbip/usbip_event.c: In function ‘usbip_start_eh’:
drivers/staging/usbip/usbip_event.c:93:8: error: incompatible types
when assigning to type ‘struct device *’ from type ‘struct device’
drivers/staging/usbip/usbip_event.c:95:8: error: incompatible types
when assigning to type ‘struct device *’ from type ‘struct device’

dev needs to be initialized with address of dev (struct device ) which
is member of struct usb_device pointed by the udev.

To make it more clear i can change it to

dev = &(container_of(ud, struct vhci_device, ud)->udev->dev);

 or

struct vhci_device *vdev = container_of(ud, struct vhci_device, ud);
dev = &(vdev->udev->dev);

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


[PATCH 1/7] staging: gdm724x: Remove version.h header inclusion in gdm_lte.c

2013-07-25 Thread Sachin Kamat
version.h header inclusion is not necessary as detected by
versioncheck.

Signed-off-by: Sachin Kamat 
---
 drivers/staging/gdm724x/gdm_lte.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/gdm724x/gdm_lte.c 
b/drivers/staging/gdm724x/gdm_lte.c
index 0c33634..c4006f6 100644
--- a/drivers/staging/gdm724x/gdm_lte.c
+++ b/drivers/staging/gdm724x/gdm_lte.c
@@ -13,7 +13,6 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include 
 #include 
 #include 
 #include 
-- 
1.7.9.5

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


Re: [PATCH staging] zcache: fix "zcache=" kernel parameter

2013-07-25 Thread Michal Hocko
On Wed 24-07-13 19:04:14, Bartlomiej Zolnierkiewicz wrote:
> From: Piotr Sarna 
> Subject: [PATCH] zcache: fix "zcache=" kernel parameter
> 
> Commit 835f2f5 ("staging: zcache: enable zcache to be built/loaded as
> a module") introduced an incorrect handling of "zcache=" parameter.
> 
> Inside zcache_comp_init() function, zcache_comp_name variable is
> checked for being empty. If not empty, the above variable is tested
> for being compatible with Crypto API. Unfortunately, after that
> function ends unconditionally (by the "goto out" directive) and returns:
> - non-zero value if verification succeeded, wrongly indicating an error
> - zero value if verification failed, falsely informing that function
>   zcache_comp_init() ended properly.
> 
> A solution to this problem is as following:
> 1. Move the "goto out" directive inside the "if (!ret)" statement
> 2. In case that crypto_has_comp() returned 0, change the value of ret
>to non-zero before "goto out" to indicate an error.
> 
> This patch replaces an earlier one from Michal Hocko (based on report
> from Cristian Rodríguez):
> 
>   http://permalink.gmane.org/gmane.linux.kernel.mm/102484
> 
> It also addressed the same issue but didn't fix the zcache_comp_init()
> for case when the compressor data passed to "zcache=" option was invalid
> or unsupported.
> 
> Signed-off-by: Piotr Sarna 
> [bzolnier: updated patch description]
> Acked-by: Bartlomiej Zolnierkiewicz 
> Signed-off-by: Kyungmin Park 
> Acked-by: Konrad Rzeszutek Wilk 
> Cc: Cristian Rodríguez 
> Cc: Michal Hocko 
> Cc: Bob Liu 

Acked-by: Michal Hocko 

Thanks!

> ---
>  drivers/staging/zcache/zcache-main.c |6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/zcache/zcache-main.c 
> b/drivers/staging/zcache/zcache-main.c
> index dcceed2..81972fa 100644
> --- a/drivers/staging/zcache/zcache-main.c
> +++ b/drivers/staging/zcache/zcache-main.c
> @@ -1811,10 +1811,12 @@ static int zcache_comp_init(void)
>  #else
>   if (*zcache_comp_name != '\0') {
>   ret = crypto_has_comp(zcache_comp_name, 0, 0);
> - if (!ret)
> + if (!ret) {
>   pr_info("zcache: %s not supported\n",
>   zcache_comp_name);
> - goto out;
> + ret = 1;
> + goto out;
> + }
>   }
>   if (!ret)
>   strcpy(zcache_comp_name, "lzo");
> -- 
> 1.7.9.5
> 
> 
> 

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


Re: [PATCH v2 1/2] staging: gdm7240: adding LTE USB driver

2013-07-25 Thread Won Kang
GDM7240 is in LE where as GDM7243 (currently under development) is in BE.

User space applications needs to discover the endianess to properly
encode/decode LTE control protocols. We have existing customers
already deploying units in large volume, and want to avoid forcing
them to change SDK APIs along with kernel updates.
At some point, It will have to be fixed with SDK API can interoperate properly.

On Thu, Jul 25, 2013 at 7:06 AM, Greg KH  wrote:
> On Thu, Jul 25, 2013 at 12:53:40AM +0300, Dan Carpenter wrote:
>> > +static int gdm_lte_ioctl_get_data(struct wm_req_t *req, struct net_device 
>> > *dev)
>> > +{
>> > +   u16 id = req->data_id;
>> > +
>> > +   switch (id) {
>> > +   case GET_ENDIAN_INFO:
>> > +   /* required for the user space application to find out device 
>> > endian */
>> > +   get_dev_endian(&req->data, dev);
>> > +   break;
>> > +   default:
>> > +   printk(KERN_ERR "glte: ioctl - unknown type %d\n", id);
>> > +   break;
>> > +   }
>> > +   return 0;
>> > +}
>>
>> This should be a sysfs file not an ioctl.  What would break if we
>> fixed this right now (as opposed to waiting until we can't change
>> the API?).
>>
>> Otherwise if we can't change this, then it should return an error
>> code instead of printing a message.
>
> It should just be removed, why would you care about the endian-ness of
> the device at all?
>
> thanks,
>
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH V2 4/4] x86: correctly detect hypervisor

2013-07-25 Thread Jason Wang
We try to handle the hypervisor compatibility mode by detecting hypervisor
through a specific order. This is not robust, since hypervisors may implement
each others features.

This patch tries to handle this situation by always choosing the last one in the
CPUID leaves. This is done by letting .detect() returns a priority instead of
true/false and just re-using the CPUID leaf where the signature were found as
the priority (or 1 if it was found by DMI). Then we can just pick hypervisor who
has the highest priority. Other sophisticated detection method could also be
implemented on top.

Suggested by H. Peter Anvin and Paolo Bonzini.

Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: H. Peter Anvin 
Cc: x...@kernel.org
Cc: K. Y. Srinivasan 
Cc: Haiyang Zhang 
Cc: Konrad Rzeszutek Wilk 
Cc: Jeremy Fitzhardinge 
Cc: Doug Covelli 
Cc: Borislav Petkov 
Cc: Dan Hecht 
Cc: Paul Gortmaker 
Cc: Marcelo Tosatti 
Cc: Gleb Natapov 
Cc: Paolo Bonzini 
Cc: Frederic Weisbecker 
Cc: linux-ker...@vger.kernel.org
Cc: de...@linuxdriverproject.org
Cc: k...@vger.kernel.org
Cc: xen-de...@lists.xensource.com
Cc: virtualizat...@lists.linux-foundation.org
Signed-off-by: Jason Wang 
---
 arch/x86/include/asm/hypervisor.h |2 +-
 arch/x86/kernel/cpu/hypervisor.c  |   15 +++
 arch/x86/kernel/cpu/mshyperv.c|   13 -
 arch/x86/kernel/cpu/vmware.c  |8 
 arch/x86/kernel/kvm.c |6 ++
 arch/x86/xen/enlighten.c  |9 +++--
 6 files changed, 25 insertions(+), 28 deletions(-)

diff --git a/arch/x86/include/asm/hypervisor.h 
b/arch/x86/include/asm/hypervisor.h
index 2d4b5e6..e42f758 100644
--- a/arch/x86/include/asm/hypervisor.h
+++ b/arch/x86/include/asm/hypervisor.h
@@ -33,7 +33,7 @@ struct hypervisor_x86 {
const char  *name;
 
/* Detection routine */
-   bool(*detect)(void);
+   uint32_t(*detect)(void);
 
/* Adjust CPU feature bits (run once per CPU) */
void(*set_cpu_features)(struct cpuinfo_x86 *);
diff --git a/arch/x86/kernel/cpu/hypervisor.c b/arch/x86/kernel/cpu/hypervisor.c
index 8727921..36ce402 100644
--- a/arch/x86/kernel/cpu/hypervisor.c
+++ b/arch/x86/kernel/cpu/hypervisor.c
@@ -25,11 +25,6 @@
 #include 
 #include 
 
-/*
- * Hypervisor detect order.  This is specified explicitly here because
- * some hypervisors might implement compatibility modes for other
- * hypervisors and therefore need to be detected in specific sequence.
- */
 static const __initconst struct hypervisor_x86 * const hypervisors[] =
 {
 #ifdef CONFIG_XEN_PVHVM
@@ -49,15 +44,19 @@ static inline void __init
 detect_hypervisor_vendor(void)
 {
const struct hypervisor_x86 *h, * const *p;
+   uint32_t pri, max_pri = 0;
 
for (p = hypervisors; p < hypervisors + ARRAY_SIZE(hypervisors); p++) {
h = *p;
-   if (h->detect()) {
+   pri = h->detect();
+   if (pri != 0 && pri > max_pri) {
+   max_pri = pri;
x86_hyper = h;
-   printk(KERN_INFO "Hypervisor detected: %s\n", h->name);
-   break;
}
}
+
+   if (max_pri)
+   printk(KERN_INFO "Hypervisor detected: %s\n", x86_hyper->name);
 }
 
 void init_hypervisor(struct cpuinfo_x86 *c)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 8f4be53..71a39f3 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -27,20 +27,23 @@
 struct ms_hyperv_info ms_hyperv;
 EXPORT_SYMBOL_GPL(ms_hyperv);
 
-static bool __init ms_hyperv_platform(void)
+static uint32_t  __init ms_hyperv_platform(void)
 {
u32 eax;
u32 hyp_signature[3];
 
if (!boot_cpu_has(X86_FEATURE_HYPERVISOR))
-   return false;
+   return 0;
 
cpuid(HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS,
  &eax, &hyp_signature[0], &hyp_signature[1], &hyp_signature[2]);
 
-   return eax >= HYPERV_CPUID_MIN &&
-   eax <= HYPERV_CPUID_MAX &&
-   !memcmp("Microsoft Hv", hyp_signature, 12);
+   if (eax >= HYPERV_CPUID_MIN &&
+   eax <= HYPERV_CPUID_MAX &&
+   !memcmp("Microsoft Hv", hyp_signature, 12))
+   return HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS;
+
+   return 0;
 }
 
 static cycle_t read_hv_clock(struct clocksource *arg)
diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
index 7076878..628a059 100644
--- a/arch/x86/kernel/cpu/vmware.c
+++ b/arch/x86/kernel/cpu/vmware.c
@@ -93,7 +93,7 @@ static void __init vmware_platform_setup(void)
  * serial key should be enough, as this will always have a VMware
  * specific string when running under VMware hypervisor.
  */
-static bool __init vmware_platform(void)
+static uint32_t __init vmware_platform(void)
 {
if (cpu_has_hypervisor) {
unsigned int eax;
@@ -102,12 +102,12 @@ static bool __

Re: [PATCH v2 1/2] staging: gdm7240: adding LTE USB driver

2013-07-25 Thread Greg KH

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Thu, Jul 25, 2013 at 01:15:07PM +0900, Won Kang wrote:
> GDM7240 is in LE where as GDM7243 (currently under development) is in BE.

But why does this information need to be sent to userspace?

> User space applications needs to discover the endianess to properly
> encode/decode LTE control protocols. We have existing customers
> already deploying units in large volume, and want to avoid forcing
> them to change SDK APIs along with kernel updates.
> At some point, It will have to be fixed with SDK API can interoperate 
> properly.

Ugh, really?  What special tools does userspace need to talk to this
device?  It "should" just be a network device, and a serial port, why
does the endianness of the device mean anything?

Custom ioctls aren't ok for new drivers, if at all possible.  Can't
userspace just trigger this based on the device id instead?

thanks,

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


Re: [PATCH] staging: ozwpan: Fix coding style.

2013-07-25 Thread Rupesh Gujare

On 24/07/13 15:06, Rupesh Gujare wrote:

This patch fixes coding style issues reported by Dan here:-
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2012-June/027767.html

Signed-off-by: Rupesh Gujare 
---
  drivers/staging/ozwpan/ozcdev.c |   15 ---
  1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/ozwpan/ozcdev.c b/drivers/staging/ozwpan/ozcdev.c
index 3e29760..73b582c 100644
--- a/drivers/staging/ozwpan/ozcdev.c
+++ b/drivers/staging/ozwpan/ozcdev.c
@@ -333,10 +333,11 @@ int oz_cdev_register(void)
  {
int err;
struct device *dev;
+
memset(&g_cdev, 0, sizeof(g_cdev));
err = alloc_chrdev_region(&g_cdev.devnum, 0, 1, "ozwpan");
if (err < 0)
-   goto out3;
+   return err;
oz_dbg(ON, "Alloc dev number %d:%d\n",
   MAJOR(g_cdev.devnum), MINOR(g_cdev.devnum));
cdev_init(&g_cdev.cdev, &oz_fops);
@@ -347,26 +348,26 @@ int oz_cdev_register(void)
err = cdev_add(&g_cdev.cdev, g_cdev.devnum, 1);
if (err < 0) {
oz_dbg(ON, "Failed to add cdev\n");
-   goto out2;
+   goto unregister;
}
g_oz_class = class_create(THIS_MODULE, "ozmo_wpan");
if (IS_ERR(g_oz_class)) {
oz_dbg(ON, "Failed to register ozmo_wpan class\n");
err = PTR_ERR(g_oz_class);
-   goto out1;
+   goto delete;
}
dev = device_create(g_oz_class, NULL, g_cdev.devnum, NULL, "ozwpan");
if (IS_ERR(dev)) {
oz_dbg(ON, "Failed to create sysfs entry for cdev\n");
err = PTR_ERR(dev);
-   goto out1;
+   goto delete;
}
return 0;
-out1:
+
+delete:
cdev_del(&g_cdev.cdev);
-out2:
+unregister:
unregister_chrdev_region(g_cdev.devnum, 1);
-out3:
return err;
  }
  
/*--

Greg,

Did you missed this patch ? Or did I made another mistake. :( ?

--
Regards,
Rupesh Gujare

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


Re: [PATCH v2] staging: comedi: range: tidy up comedi_check_chanlist()

2013-07-25 Thread Ian Abbott

On 2013-07-24 18:00, H Hartley Sweeten wrote:

The only difference in the if() and else if() check of the chanlist
is the source of the range table length. Consolidate the checks to
make the function a bit more concise.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
v2: fix an issue when the range_table_list is used for the range_len

  drivers/staging/comedi/range.c | 43 --
  1 file changed, 20 insertions(+), 23 deletions(-)


Reviewed-by: Ian Abbott 

--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 1/2] staging: gdm7240: adding LTE USB driver

2013-07-25 Thread Won Kang
>> GDM7240 is in LE where as GDM7243 (currently under development) is in BE.
>
> But why does this information need to be sent to userspace?
>

GDM724x chips have different endianess because of their internal
architecture.The device sends out and accepts LTE protocol information
in its own endianess. This is the design decision we made and we let
the host converts the byte order. To decode the protocols, userspace
needs this endianess information.

>> User space applications needs to discover the endianess to properly
>> encode/decode LTE control protocols. We have existing customers
>> already deploying units in large volume, and want to avoid forcing
>> them to change SDK APIs along with kernel updates.
>> At some point, It will have to be fixed with SDK API can interoperate 
>> properly.
>
> Ugh, really?  What special tools does userspace need to talk to this
> device?  It "should" just be a network device, and a serial port, why
> does the endianness of the device mean anything?

Just like 3G modems, network device and a serial port is enough for
basic operation. For this, no ioctl for endianess is required.
However, customers and operators want more information and programming
capabilities, and this is really difficult to achieve with standard
3GPP AT commands alone. Therefore, if wanted, we also provide SDK APIs
for handling more advanced features. Examples of such information
include detailed connection status, rf controls, debug logs, and even
firmware download.

> Custom ioctls aren't ok for new drivers, if at all possible.  Can't
> userspace just trigger this based on the device id instead?

Historically, the SDK is cross-platform designed and we wanted to have
common methods for different operating systems, and have chosen to use
ioctl. Windows does have one :). We will probably go with reading
device id

Anyway, you are right that endianess matters only when using this SDK.
If you suggest to remove the ioctl because SDK is optional, I will
agree.

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


[PATCH] staging: zcache: fix "zcache=" kernel parameter

2013-07-25 Thread Bartlomiej Zolnierkiewicz
From: Piotr Sarna 

Commit 835f2f5 ("staging: zcache: enable zcache to be built/loaded as
a module") introduced an incorrect handling of "zcache=" parameter.

Inside zcache_comp_init() function, zcache_comp_name variable is
checked for being empty. If not empty, the above variable is tested
for being compatible with Crypto API. Unfortunately, after that
function ends unconditionally (by the "goto out" directive) and returns:
- non-zero value if verification succeeded, wrongly indicating an error
- zero value if verification failed, falsely informing that function
  zcache_comp_init() ended properly.

A solution to this problem is as following:
1. Move the "goto out" directive inside the "if (!ret)" statement
2. In case that crypto_has_comp() returned 0, change the value of ret
   to non-zero before "goto out" to indicate an error.

This patch replaces an earlier one from Michal Hocko (based on report
from Cristian Rodríguez):

http://permalink.gmane.org/gmane.linux.kernel.mm/102484

It also addressed the same issue but didn't fix the zcache_comp_init()
for case when the compressor data passed to "zcache=" option was invalid
or unsupported.

Signed-off-by: Piotr Sarna 
[bzolnier: updated patch description]
Acked-by: Bartlomiej Zolnierkiewicz 
Signed-off-by: Kyungmin Park 
Acked-by: Konrad Rzeszutek Wilk 
Acked-by: Michal Hocko 
Cc: Cristian Rodríguez 
Cc: Bob Liu 
---
 drivers/staging/zcache/zcache-main.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/zcache/zcache-main.c 
b/drivers/staging/zcache/zcache-main.c
index dcceed2..81972fa 100644
--- a/drivers/staging/zcache/zcache-main.c
+++ b/drivers/staging/zcache/zcache-main.c
@@ -1811,10 +1811,12 @@ static int zcache_comp_init(void)
 #else
if (*zcache_comp_name != '\0') {
ret = crypto_has_comp(zcache_comp_name, 0, 0);
-   if (!ret)
+   if (!ret) {
pr_info("zcache: %s not supported\n",
zcache_comp_name);
-   goto out;
+   ret = 1;
+   goto out;
+   }
}
if (!ret)
strcpy(zcache_comp_name, "lzo");
-- 
1.8.2.3

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


Re: [PATCH] staging: comedi: adv_pci1724: remove ao_range_list_1724

2013-07-25 Thread Ian Abbott

On 2013-07-24 18:03, H Hartley Sweeten wrote:

All the AO channels have the same ranges. Remove the subdevice
s->range_table_list and just use the s->range_table to setup the
ranges.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
  drivers/staging/comedi/drivers/adv_pci1724.c | 6 +-
  1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/staging/comedi/drivers/adv_pci1724.c 
b/drivers/staging/comedi/drivers/adv_pci1724.c
index 84907c7..009a303 100644
--- a/drivers/staging/comedi/drivers/adv_pci1724.c
+++ b/drivers/staging/comedi/drivers/adv_pci1724.c
@@ -125,10 +125,6 @@ static const struct comedi_lrange ao_ranges_1724 = { 4,
}
  };

-static const struct comedi_lrange *const ao_range_list_1724[NUM_AO_CHANNELS] = 
{
-   [0 ... NUM_AO_CHANNELS - 1] = &ao_ranges_1724,
-};
-
  /* this structure is for data unique to this hardware driver. */
  struct adv_pci1724_private {
int ao_value[NUM_AO_CHANNELS];
@@ -308,7 +304,7 @@ static int setup_subdevices(struct comedi_device *dev)
s->subdev_flags = SDF_READABLE | SDF_WRITABLE | SDF_GROUND;
s->n_chan = NUM_AO_CHANNELS;
s->maxdata = 0x3fff;
-   s->range_table_list = ao_range_list_1724;
+   s->range_table = &ao_ranges_1724;
s->insn_read = ao_readback_insn;
s->insn_write = ao_winsn;


This was actually made into a range_table_list by me deliberately (see 
 
but that patch was part of a series of 13 patches that were later 
squashed into a single patch).


Reviewing the original rationale again, I'm no longer convinced that 
making is a range_table_list was justified.  All the channels do have 
the same range table even though the selection of which ranges from that 
range table actually work as expected is decided on a per-channel basis 
by per-channel hardware jumpers (choosing between voltage output and 
current output per-channel).


So I'm happy for it to change back to a single range_table.

Reviewed-by: Ian Abbott 

--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: ozwpan: Fix coding style.

2013-07-25 Thread Dan Carpenter
On Thu, Jul 25, 2013 at 10:30:30AM +0100, Rupesh Gujare wrote:
> Greg,
> 
> Did you missed this patch ? Or did I made another mistake. :( ?
> 

Chillax.  It has only been one day.  Ask again after two or three
weeks.

regards,
dan carpenter

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


Re: [PATCH] staging: comedi: allow ISA and PC/104 drivers on non-ISA systems

2013-07-25 Thread Ian Abbott

On 2013-07-24 19:24, H Hartley Sweeten wrote:

Embedded systems with a PC/104 bus might have a configuration that
does not have ISA enabled. This creates a problem in Comedi where
the PC/104 drivers cannot be enabled.

Introduce a Kconfig option to allow enabling the Comedi ISA and
PC/104 drivers if ISA is disabled.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
  drivers/staging/comedi/Kconfig | 13 -
  1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/Kconfig b/drivers/staging/comedi/Kconfig
index 01782fc..4a5dc4c 100644
--- a/drivers/staging/comedi/Kconfig
+++ b/drivers/staging/comedi/Kconfig
@@ -98,9 +98,20 @@ config COMEDI_SKEL

  endif # COMEDI_MISC_DRIVERS

+if !ISA
+
+config COMEDI_ENABLE_ISA
+   bool "Enable Comedi ISA and PC/104 drivers on non-ISA systems"
+   ---help---
+ Many of the Comedi PC/104 drivers work on embeded systems that
+ might not have the ISA option enabled. Select this option to
+ enable Comedi ISA and PC/104 drivers on these systems.
+
+endif # !ISA
+
  menuconfig COMEDI_ISA_DRIVERS
bool "Comedi ISA and PC/104 drivers"
-   depends on ISA
+   depends on ISA || COMEDI_ENABLE_ISA
---help---
  Enable comedi ISA and PC/104 drivers to be built




I know there are one or two PC/104-Plus CPU boards with 64-bit 
processors that have a PC/104 ISA bus, but I don't know how well PC/104 
ISA boards work on such systems when run in 64-bit mode.  Is it worth 
worrying about them?


If you just want the option to build-test the ISA drivers, there is a 
COMPILE_TEST configuration option that could be checked, e.g.:


menuconfig COMEDI_ISA_DRIVERS
bool "Comedi ISA and PC/104 drivers"
depends on ISA || COMPILE_TEST
---help---
  Enable comedi ISA and PC/104 drivers to be built

--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH V2 4/4] x86: correctly detect hypervisor

2013-07-25 Thread KY Srinivasan


> -Original Message-
> From: Jason Wang [mailto:jasow...@redhat.com]
> Sent: Thursday, July 25, 2013 4:55 AM
> To: t...@linutronix.de; mi...@redhat.com; h...@zytor.com; x...@kernel.org;
> linux-ker...@vger.kernel.org; pbonz...@redhat.com
> Cc: k...@vger.kernel.org; Jason Wang; KY Srinivasan; Haiyang Zhang; Konrad
> Rzeszutek Wilk; Jeremy Fitzhardinge; Doug Covelli; Borislav Petkov; Dan Hecht;
> Paul Gortmaker; Marcelo Tosatti; Gleb Natapov; Frederic Weisbecker;
> de...@linuxdriverproject.org; xen-de...@lists.xensource.com;
> virtualizat...@lists.linux-foundation.org
> Subject: [PATCH V2 4/4] x86: correctly detect hypervisor
> 
> We try to handle the hypervisor compatibility mode by detecting hypervisor
> through a specific order. This is not robust, since hypervisors may implement
> each others features.
> 
> This patch tries to handle this situation by always choosing the last one in 
> the
> CPUID leaves. This is done by letting .detect() returns a priority instead of
> true/false and just re-using the CPUID leaf where the signature were found as
> the priority (or 1 if it was found by DMI). Then we can just pick hypervisor 
> who
> has the highest priority. Other sophisticated detection method could also be
> implemented on top.
> 
> Suggested by H. Peter Anvin and Paolo Bonzini.
> 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: H. Peter Anvin 
> Cc: x...@kernel.org
> Cc: K. Y. Srinivasan 
> Cc: Haiyang Zhang 
> Cc: Konrad Rzeszutek Wilk 
> Cc: Jeremy Fitzhardinge 
> Cc: Doug Covelli 
> Cc: Borislav Petkov 
> Cc: Dan Hecht 
> Cc: Paul Gortmaker 
> Cc: Marcelo Tosatti 
> Cc: Gleb Natapov 
> Cc: Paolo Bonzini 
> Cc: Frederic Weisbecker 
> Cc: linux-ker...@vger.kernel.org
> Cc: de...@linuxdriverproject.org
> Cc: k...@vger.kernel.org
> Cc: xen-de...@lists.xensource.com
> Cc: virtualizat...@lists.linux-foundation.org
> Signed-off-by: Jason Wang 
Acked-by:  K. Y. Srinivasan 

> ---
>  arch/x86/include/asm/hypervisor.h |2 +-
>  arch/x86/kernel/cpu/hypervisor.c  |   15 +++
>  arch/x86/kernel/cpu/mshyperv.c|   13 -
>  arch/x86/kernel/cpu/vmware.c  |8 
>  arch/x86/kernel/kvm.c |6 ++
>  arch/x86/xen/enlighten.c  |9 +++--
>  6 files changed, 25 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/x86/include/asm/hypervisor.h
> b/arch/x86/include/asm/hypervisor.h
> index 2d4b5e6..e42f758 100644
> --- a/arch/x86/include/asm/hypervisor.h
> +++ b/arch/x86/include/asm/hypervisor.h
> @@ -33,7 +33,7 @@ struct hypervisor_x86 {
>   const char  *name;
> 
>   /* Detection routine */
> - bool(*detect)(void);
> + uint32_t(*detect)(void);
> 
>   /* Adjust CPU feature bits (run once per CPU) */
>   void(*set_cpu_features)(struct cpuinfo_x86 *);
> diff --git a/arch/x86/kernel/cpu/hypervisor.c 
> b/arch/x86/kernel/cpu/hypervisor.c
> index 8727921..36ce402 100644
> --- a/arch/x86/kernel/cpu/hypervisor.c
> +++ b/arch/x86/kernel/cpu/hypervisor.c
> @@ -25,11 +25,6 @@
>  #include 
>  #include 
> 
> -/*
> - * Hypervisor detect order.  This is specified explicitly here because
> - * some hypervisors might implement compatibility modes for other
> - * hypervisors and therefore need to be detected in specific sequence.
> - */
>  static const __initconst struct hypervisor_x86 * const hypervisors[] =
>  {
>  #ifdef CONFIG_XEN_PVHVM
> @@ -49,15 +44,19 @@ static inline void __init
>  detect_hypervisor_vendor(void)
>  {
>   const struct hypervisor_x86 *h, * const *p;
> + uint32_t pri, max_pri = 0;
> 
>   for (p = hypervisors; p < hypervisors + ARRAY_SIZE(hypervisors); p++) {
>   h = *p;
> - if (h->detect()) {
> + pri = h->detect();
> + if (pri != 0 && pri > max_pri) {
> + max_pri = pri;
>   x86_hyper = h;
> - printk(KERN_INFO "Hypervisor detected: %s\n", h-
> >name);
> - break;
>   }
>   }
> +
> + if (max_pri)
> + printk(KERN_INFO "Hypervisor detected: %s\n", x86_hyper-
> >name);
>  }
> 
>  void init_hypervisor(struct cpuinfo_x86 *c)
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 8f4be53..71a39f3 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -27,20 +27,23 @@
>  struct ms_hyperv_info ms_hyperv;
>  EXPORT_SYMBOL_GPL(ms_hyperv);
> 
> -static bool __init ms_hyperv_platform(void)
> +static uint32_t  __init ms_hyperv_platform(void)
>  {
>   u32 eax;
>   u32 hyp_signature[3];
> 
>   if (!boot_cpu_has(X86_FEATURE_HYPERVISOR))
> - return false;
> + return 0;
> 
>   cpuid(HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS,
> &eax, &hyp_signature[0], &hyp_signature[1], &hyp_signature[2]);
> 
> - return eax >= HYPERV_CPUID_MIN &&
> - eax <= HYPERV_CPUID_MAX &&
> - !memcmp("Microsoft Hv",

Re: [PATCH] staging: ozwpan: Fix coding style.

2013-07-25 Thread Rupesh Gujare

On 25/07/13 11:39, Dan Carpenter wrote:

On Thu, Jul 25, 2013 at 10:30:30AM +0100, Rupesh Gujare wrote:

Greg,

Did you missed this patch ? Or did I made another mistake. :( ?


Chillax.  It has only been one day.  Ask again after two or three
weeks.




All right Dan,

Probably I was getting too impatient, while watching that other patches 
in queue are getting pulled in. :(


--
Regards,
Rupesh Gujare

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


Re: [PATCH] staging/lustre: add BLOCK depends in Kconfig

2013-07-25 Thread Paul Bolle
On Thu, 2013-07-25 at 15:06 +0800, Xiong Zhou wrote:
> Add BLOCK depends in Kconfig for LUSTRE to fix this:
> drivers/staging/lustre/lustre/fid/../include/linux/lustre_compat25.h:117:2: 
> error: implicit declaration of function ‘unregister_blkdev’ 
> 
> Signed-off-by: Xiong Zhou 
> ---
>  drivers/staging/lustre/lustre/Kconfig |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/lustre/lustre/Kconfig 
> b/drivers/staging/lustre/lustre/Kconfig
> index 9ae7fa8..0b45de0 100644
> --- a/drivers/staging/lustre/lustre/Kconfig
> +++ b/drivers/staging/lustre/lustre/Kconfig
> @@ -1,6 +1,6 @@
>  config LUSTRE_FS
> tristate "Lustre file system client support"
> -   depends on STAGING && INET && m
> +   depends on STAGING && INET && BLOCK && m

Everything below drivers/staging/lustre/Kconfig is only sourced if
STAGING is set. You can drop the dependency on STAGING.

> select LNET
> select CRYPTO
> select CRYPTO_CRC32


Paul Bolle

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


RE: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining memory blocks

2013-07-25 Thread KY Srinivasan


> -Original Message-
> From: Michal Hocko [mailto:mho...@suse.cz]
> Sent: Thursday, July 25, 2013 3:57 AM
> To: Dave Hansen
> Cc: KY Srinivasan; Dave Hansen; gre...@linuxfoundation.org; linux-
> ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de;
> a...@canonical.com; a...@firstfloor.org; a...@linux-foundation.org; linux-
> m...@kvack.org; kamezawa.hiroy...@gmail.com; han...@cmpxchg.org;
> ying...@google.com; jasow...@redhat.com; k...@vrfy.org
> Subject: Re: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining
> memory blocks
> 
> On Wed 24-07-13 14:02:32, Dave Hansen wrote:
> > On 07/24/2013 12:45 PM, KY Srinivasan wrote:
> > > All I am saying is that I see two classes of failures: (a) Our
> > > inability to allocate memory to manage the memory that is being hot added
> > > and (b) Our inability to bring the hot added memory online within a
> reasonable
> > > amount of time. I am not sure the cause for (b) and I was just 
> > > speculating that
> > > this could be memory related. What is interesting is that I have seen 
> > > failure
> related
> > > to our inability to online the memory after having succeeded in hot 
> > > adding the
> > > memory.
> >
> > I think we should hold off on this patch and other like it until we've
> > been sufficiently able to explain how (b) happens.
> 
> Agreed.

As promised, I have sent out the patches for (a) an implementation of an 
in-kernel API
for onlining  and a consumer for this API. While I don't know the exact reason 
why the
user mode code is delayed (under some low memory conditions), what is the harm 
in having
a mechanism to online memory that has been hot added without involving user 
space code.
Based on Michal's feedback, the onlininig API hides all of the internal details 
and presents a
generic interface.

Regards,

K. Y



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


Re: [PATCH] staging: comedi: fix subdev_flags for all digital subdevices

2013-07-25 Thread Ian Abbott

On 2013-07-24 21:14, H Hartley Sweeten wrote:

The SDF_GROUND and SDF_COMMON flags only apply to analog subdevices.
It makes no sense to set these flags for digital subdevices.


Possibly.  It depends on the device.  I suppose the reported flags only 
matter if the grounding is configurable.



Digital input (COMEDI_SUBD_DI) subdevices only need the SDF_READABLE
flag and optionally SDF_CMD_READ it the subdevice supports commands.

Digital output (COMEDI_SUBD_DO) subdevices only need the SDF_WRITABLE
flag.


I think the rule should be that if the INSN_WRITE instruction works it 
should have the SDF_WRITABLE flag set and ditto for INSN_READ and 
SDF_READABLE.  Digital output subdevices generally support INSN_READ 
even if its emulated by the insn_bits handler, so ought to have the 
SDF_READABLE flag set in that case.


In some drivers the data read back from an digital output subdevice is 
just a software shadow of what was last written, although in other 
drivers it reads back the data from actual hardware registers.  For some 
hardware, it's even possible for the data read back to differ from the 
data written (for example if the outputs are open collector (or open 
drain) outputs, but those should probably be modelled as DIO subdevices 
by comedi).


Perhaps __comedi_device_postconfig() in drivers/staging/comedi/drivers.c 
should just overwrite the SDF_READABLE and SDF_WRITABLE flags according 
to which insn_read and insn_write handlers are set (after they may have 
been set to insn_rw_emulate_bits()).




Digital In/Out (COMEDI_SUBD_DIO) subdevices need both flags.

The aio_iiro_16 driver sets the type of the subdevices incorrectly.
Subdevice 0 is actually a COMEDI_SUBD_DO and subdevice 1 is a
COMEDI_SUBD_DI. Fix them and rename the (*insn_bits) functions.

The addi_apci_1032 driver incorrectly sets the SDF_CMD_READ subdev_flag
in the subdevice type. Fix it.

The adv_pci_dio driver sets the SDF_LSAMPL subdev_flag if the
number of channels is > 16. This flag has nothing to do with the
number of channels and is not needed. Remove it.


The SDF_LSAMPL flag might be meaningful if it supported asynchronous 
commands, but it doesn't in this case.


--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 0/4] staging: comedi: remove addi_apci_1710 driver

2013-07-25 Thread Ian Abbott

On 2013-07-24 19:11, H Hartley Sweeten wrote:

This driver is a great example of "bad kernel code"...

It greatly abuses the comedi API and will not work without patching
the comedi core. It's CodingStyle is terrible. It uses floating point
math. And, it's almost impossible to read.

A Kconfig option to enable this driver isn't even available.

Tidy it up a bit so it does not rely on any other source files then
remove it cleanly.

H Hartley Sweeten (4):
   staging: comedi: addi_apci_1710: fix some compile errors/warnings
   staging: comedi: addi_apci_1710: separate from addi_common.h
   staging: comedi: addi_apci_1710: delete driver
   staging: comedi: addi_common.h: cleanup after removal of
 addi_apci_1710

  .../comedi/drivers/addi-data/APCI1710_82x54.c  | 1068 
  .../comedi/drivers/addi-data/APCI1710_Chrono.c | 2050 
  .../comedi/drivers/addi-data/APCI1710_Dig_io.c | 1037 
  .../comedi/drivers/addi-data/APCI1710_INCCPT.c | 5461 
  .../comedi/drivers/addi-data/APCI1710_Inp_cpt.c|  866 
  .../comedi/drivers/addi-data/APCI1710_Pwm.c| 3582 -
  .../comedi/drivers/addi-data/APCI1710_Ssi.c|  845 ---
  .../comedi/drivers/addi-data/APCI1710_Tor.c| 2065 
  .../comedi/drivers/addi-data/APCI1710_Ttl.c| 1044 
  .../staging/comedi/drivers/addi-data/addi_common.h |  169 -
  .../comedi/drivers/addi-data/hwdrv_APCI1710.c  | 1314 -
  drivers/staging/comedi/drivers/addi_apci_1710.c|   99 -
  12 files changed, 19600 deletions(-)
  delete mode 100644 drivers/staging/comedi/drivers/addi-data/APCI1710_82x54.c
  delete mode 100644 drivers/staging/comedi/drivers/addi-data/APCI1710_Chrono.c
  delete mode 100644 drivers/staging/comedi/drivers/addi-data/APCI1710_Dig_io.c
  delete mode 100644 drivers/staging/comedi/drivers/addi-data/APCI1710_INCCPT.c
  delete mode 100644 drivers/staging/comedi/drivers/addi-data/APCI1710_Inp_cpt.c
  delete mode 100644 drivers/staging/comedi/drivers/addi-data/APCI1710_Pwm.c
  delete mode 100644 drivers/staging/comedi/drivers/addi-data/APCI1710_Ssi.c
  delete mode 100644 drivers/staging/comedi/drivers/addi-data/APCI1710_Tor.c
  delete mode 100644 drivers/staging/comedi/drivers/addi-data/APCI1710_Ttl.c
  delete mode 100644 drivers/staging/comedi/drivers/addi-data/hwdrv_APCI1710.c
  delete mode 100644 drivers/staging/comedi/drivers/addi_apci_1710.c


Seems fair enough as it's currently dead code anyway.

Reviewed-by: Ian Abbott 

--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 01/13] staging: comedi: pcmuio: fix interrupt requests

2013-07-25 Thread Ian Abbott

On 2013-07-24 19:45, H Hartley Sweeten wrote:

Legacy (ISA) interrupts are not sharable so this driver should not
be passing the IRQF_SHARED flag when requesting the interrupts.

This driver supports two board types:
   PCM-UIO48 with one asic (one interrupt source)
   PCM-UIO96 with two asics (two interrupt sources)

The PCM-UIO96 has a jumper that allows the two interrupt sources to
share an interrupt. This is safe for legacy interrupts as long as
the "shared" interrupt is handled by a single driver.

Modify the request_irq() code in this driver to correctly request the
interrupts. For the PCM-UI048 case (one asic) only one request_irq()
is needed. For the PCM-UIO96 (two asics) there are two cases:

   1) interrupt is shared, one request_irq() call
   2) two interrupts, two request_irq() calls

Do the first request_irq() in all cases. Only do the second if the
boardinfo indicates that the board has two asics and the user passed
the 2nd interrupt number.

Pack the two interrupt numbers in dev->irq instead of storing them in
the private data. During the detach of the board, this driver will
free the 2nd irq if needed and the core will free the 1st.

Move the board reset and interrupt request code so it occurs early
in the attach. We can then check dev->irq to see if the subdevice
interrupt support actually needs to be initialized.

Add a call to pcmuio_reset() in the (*detach) to make sure the
interrupts are disabled before freeing the irqs.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
  drivers/staging/comedi/drivers/pcmuio.c | 96 ++---
  1 file changed, 54 insertions(+), 42 deletions(-)


Packing the two irq values in a single member seems a bit hackish.  I 
think adding an 'irq2' member to struct pcmuio_private would be cleaner.




diff --git a/drivers/staging/comedi/drivers/pcmuio.c 
b/drivers/staging/comedi/drivers/pcmuio.c
index 13f1943..2c491be 100644
--- a/drivers/staging/comedi/drivers/pcmuio.c
+++ b/drivers/staging/comedi/drivers/pcmuio.c
@@ -146,7 +146,6 @@ struct pcmuio_subdev_private {

  struct pcmuio_private {
struct {
-   unsigned int irq;
spinlock_t spinlock;
} asics[PCMUIO_MAX_ASICS];
struct pcmuio_subdev_private *sprivs;
@@ -397,20 +396,14 @@ static int pcmuio_handle_asic_interrupt(struct 
comedi_device *dev, int asic)
  static irqreturn_t pcmuio_interrupt(int irq, void *d)
  {
struct comedi_device *dev = d;
-   struct pcmuio_private *devpriv = dev->private;
-   int got1 = 0;
-   int asic;
+   int handled = 0;

-   for (asic = 0; asic < PCMUIO_MAX_ASICS; ++asic) {
-   if (irq == devpriv->asics[asic].irq) {
-   /* it is an interrupt for ASIC #asic */
-   if (pcmuio_handle_asic_interrupt(dev, asic))
-   got1++;
-   }
-   }
-   if (!got1)
-   return IRQ_NONE;/* interrupt from other source */
-   return IRQ_HANDLED;
+   if (irq == (dev->irq & 0xff))
+   handled += pcmuio_handle_asic_interrupt(dev, 0);
+   if (irq == ((dev->irq >> 8) & 0xff))
+   handled += pcmuio_handle_asic_interrupt(dev, 1);
+
+   return handled ? IRQ_HANDLED : IRQ_NONE;
  }

  static int pcmuio_start_intr(struct comedi_device *dev,
@@ -599,12 +592,9 @@ static int pcmuio_attach(struct comedi_device *dev, struct 
comedi_devconfig *it)
struct pcmuio_private *devpriv;
struct pcmuio_subdev_private *subpriv;
int sdev_no, n_subdevs, asic;
-   unsigned int irq[PCMUIO_MAX_ASICS];
+   unsigned int irq;
int ret;

-   irq[0] = it->options[1];
-   irq[1] = it->options[2];
-
ret = comedi_request_region(dev, it->options[0],
board->num_asics * PCMUIO_ASIC_IOSIZE);
if (ret)
@@ -617,6 +607,39 @@ static int pcmuio_attach(struct comedi_device *dev, struct 
comedi_devconfig *it)
for (asic = 0; asic < PCMUIO_MAX_ASICS; ++asic)
spin_lock_init(&devpriv->asics[asic].spinlock);

+   pcmuio_reset(dev);
+
+   /* request the 1st irq */
+   irq = it->options[1];
+   if (irq) {
+   ret = request_irq(irq, pcmuio_interrupt, 0,
+ board->name, dev);
+   if (ret == 0) {
+   /* save the irq for the core to free */
+   dev->irq = irq;
+   }
+   }
+
+   /* request the 2nd irq if needed */
+   if (board->num_asics == 2 && dev->irq) {
+   irq = it->options[2];
+   if (irq && irq != dev->irq) {
+   /* 1st and 2nd irq differ */
+   ret = request_irq(irq, pcmuio_interrupt, 0,
+ board->name, dev);
+   if (ret == 0) {
+   /* save the irq for (*detach) to free */
+ 

Re: [PATCH 06/13] staging: comedi: pcmuio: pcmuio_handle_asic_interrupt() does not need the subdevice

2013-07-25 Thread Ian Abbott

On 2013-07-24 19:47, H Hartley Sweeten wrote:

The comedi_subdevice pointer is not used in this function. Its simply
passed on to pcmuio_handle_intr_subdev(). That function then needs to
calculate the 'asic' associated with the subdevice. Just pass on the
'asic' instead and let pcmuio_handle_intr_subdev() get the subdevice
from that.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 


In that case, it would make sense to rename the function to 
pcmuio_handle_intr_asic().


--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 07/13] staging: comedi: pcmuio: make sure input channels stay inputs

2013-07-25 Thread Ian Abbott

On 2013-07-24 19:48, H Hartley Sweeten wrote:

When updating the output channels make sure the channels configured as
inputs stay inputs.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
  drivers/staging/comedi/drivers/pcmuio.c | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers/pcmuio.c 
b/drivers/staging/comedi/drivers/pcmuio.c
index 076a08a..5b0ade2 100644
--- a/drivers/staging/comedi/drivers/pcmuio.c
+++ b/drivers/staging/comedi/drivers/pcmuio.c
@@ -215,8 +215,14 @@ static int pcmuio_dio_insn_bits(struct comedi_device *dev,
s->state &= ~mask;
s->state |= (mask & bits);

-   /* invert the state and update the channels */
+   /*
+* Invert the state and update the channels.
+*
+* The s->io_bits mask makes sure inputs are '0' so
+* that the output pins stay in a high-z state.
+*/
val = s->state ^ ((0x1 << s->n_chan) - 1);
+   val &= s->io_bits;
pcmuio_write(dev, val, asic, 0, port);
}




That's fine although the original function has other problems, for 
example the value returned in data[1] should reflect the modified data, 
not the original data and probably shouldn't be modifying s->state based 
on the data read from the hardware.  I think the function should be 
something like this:


static int pcmuio_dio_insn_bits(struct comedi_device *dev,
struct comedi_subdevice *s,
struct comedi_insn *insn,
unsigned int *data)
{
unsigned int chanmask = ((1 << s->n_chan) - 1;
unsigned int mask = data[0] & chanmask;
unsigned int bits = data[1];
int asic = s->index / 2;
int port = (s->index % 2) ? 3 : 0;
unsigned int val;

if (mask) {
s->state &= ~mask;
s->state |= (mask & bits);
/*
 * Invert the state and update the channels.
 *
 * The s->io_bits mask makes sure inputs are '0' so
 * that the output pins stay in a high-z state.
 */
val = ~s->state & s->io_bits & chanmask;
pcmuio_write(dev, val, asic, 0, port);
}

/* get inverted state of the channels from the port */
val = pcmuio_read(dev, asic, 0, port);

/* get the true state of the channels */
data[1] = ~val & chanmask;

return insn->n;
}


--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 00/13] staging: comedi: pcmuio: additional cleanup

2013-07-25 Thread Ian Abbott

On 2013-07-24 19:44, H Hartley Sweeten wrote:

Tidy up the interrupt support and the private data usage. Do a bit
of additional cleanup to clarify the driver a bit.

H Hartley Sweeten (13):
   staging: comedi: pcmuio: fix interrupt requests
   staging: comedi: pcmuio: spinlock pcmuio_{write,read}()
   staging: comedi: pcmuio: tidy up pcmuio_handle_asic_interrupt()
   staging: comedi: pcmuio: remove 'asic' from subdevice private data
   staging: comedi: pcmuio: remove subdevice private data
   staging: comedi: pcmuio: pcmuio_handle_asic_interrupt() does not need the 
subdevice
   staging: comedi: pcmuio: make sure input channels stay inputs
   staging: comedi: pcmuio: remove unnecessary mask of triggered channels
   staging: comedi: pcmuio: add inline helpers to get the 'iobase', 'asic', and 
'port'
   staging: comedi: pcmuio: document asics[].spinlock
   staging: comedi: pcmuio: fix types of asics members
   staging: comedi: pcmuio: remove unneeded include
   staging: comedi: pcmuio: tidy up some function declarations

  drivers/staging/comedi/drivers/pcmuio.c | 342 
  1 file changed, 171 insertions(+), 171 deletions(-)


I commented on patches 01, 06 and 07.

--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH staging] zcache: fix "zcache=" kernel parameter

2013-07-25 Thread Bartlomiej Zolnierkiewicz
On Wednesday, July 24, 2013 11:02:46 AM Greg KH wrote:
> On Wed, Jul 24, 2013 at 07:04:14PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > From: Piotr Sarna 
> > Subject: [PATCH] zcache: fix "zcache=" kernel parameter
> 
> What's with the duplicated Subject: line here?  Why?  That means the
> From: line there will probably not get recognized by git (well, it
> might), but then the subject is duplicated again, just like Linus has
> complained about not doing in the past.

Uh, sorry about that.

> And in a mime-encoded format?  Ugh, please fix your email client and
> send it again (yes, git can handle it, but it's a pain at times...)

This seems to be some bug in my version of KMail (it gets confused on
"í" character). I've used git send-email this time.

> third time's a charm?

I hope that the patch is OK now (BTW I applied Michal's ACK that come
in the meantime so there is some positive side of the delay ;).

Once again sorry for the bad patch format and thanks for the patience.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

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


Re: [PATCH 14/14] staging: comedi: ii_pci20kc: reorder subdevices

2013-07-25 Thread Ian Abbott

On 2013-07-24 20:14, H Hartley Sweeten wrote:

Make the built-on dio subdevice 0 and the three add-on modules
subdevices 1-3. This allows the driver to consistently use the
ii20k_module_iobase() helper to get the base address needed to
access a the registers used by a given subdevice.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 


It's best to avoid reordering the subdevices if you can help it to avoid 
screwing with userspace unnecessarily.


--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 00/14] staging: comedi: ii_pci20kc: cleanup driver

2013-07-25 Thread Ian Abbott

On 2013-07-24 20:08, H Hartley Sweeten wrote:

Tidy up this legacy comedi driver.

H Hartley Sweeten (14):
   staging: comedi: ii_pci20kc: use comedi_alloc_spriv()
   staging: comedi: ii_pci20kc: remove forward declarations 1
   staging: comedi: ii_pci20kc: remove forward declarations 2
   staging: comedi: ii_pci20kc: remove forward declarations 3
   staging: comedi: ii_pci20kc: move comedi_lrange tables
   staging: comedi: ii_pci20kc: remove CHAN macro
   staging: comedi: ii_pci20kc.c: cleanup the dio subdevice
   staging: comedi: ii_pci20kc.c: tidy up the subdevice module init
   staging: comedi: ii_pci20kc.c: remove 'iobase' from the subdevice private 
data
   staging: comedi: ii_pci20kc.c: break up the subdevice private data union
   staging: comedi: ii_pci20kc: cleanup the ao subdevice
   staging: comedi: ii_pci20kc: cleanup the ai subdevice
   staging: comedi: ii_pci20kc: cleanup final pieces
   staging: comedi: ii_pci20kc: reorder subdevices

  drivers/staging/comedi/drivers/ii_pci20kc.c | 1002 ---
  1 file changed, 443 insertions(+), 559 deletions(-)


I only commented on patch 14 (reordering the subdevices) although I 
don't know enough about the hardware to comment on patch 12 (handling 
multiple AI ranges).


--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 34/53] staging: comedi: usbdux: tidy up usbdux_ai_cmd()

2013-07-25 Thread Ian Abbott

On 2013-07-24 22:19, H Hartley Sweeten wrote:

Rename the local variable used for the private data pointer to the
comedi "norm".

Remove the unnecessary sanity check of the private data pointer. This
function can only be called is the private data was allocated during
the attach.

Tidy up the exit path using goto to ensure that the semaphore is
released.

Set the ai_cmd_running flag after submitting the urbs to remove the
need to clear the flag if the submit fails.


As for ai_inttrig, this could cause the URb completion routine to 
execute before the ai_cmd_running flag has been set.




Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
  drivers/staging/comedi/drivers/usbdux.c | 92 -
  1 file changed, 44 insertions(+), 48 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index 204867b..1982e23 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -881,86 +881,79 @@ ai_trig_exit:

  static int usbdux_ai_cmd(struct comedi_device *dev, struct comedi_subdevice 
*s)
  {
+   struct usbdux_private *devpriv = dev->private;
struct comedi_cmd *cmd = &s->async->cmd;
-   unsigned int chan, range;
-   int i, ret;
-   struct usbdux_private *this_usbduxsub = dev->private;
-   int result;
-
-   if (!this_usbduxsub)
-   return -EFAULT;
+   int len = cmd->chanlist_len;
+   int ret = -EBUSY;
+   int i;

/* block other CPUs from starting an ai_cmd */
-   down(&this_usbduxsub->sem);
-   if (this_usbduxsub->ai_cmd_running) {
-   up(&this_usbduxsub->sem);
-   return -EBUSY;
-   }
+   down(&devpriv->sem);
+
+   if (devpriv->ai_cmd_running)
+   goto ai_cmd_exit;
+
/* set current channel of the running acquisition to zero */
s->async->cur_chan = 0;

-   this_usbduxsub->dux_commands[1] = cmd->chanlist_len;
-   for (i = 0; i < cmd->chanlist_len; ++i) {
-   chan = CR_CHAN(cmd->chanlist[i]);
-   range = CR_RANGE(cmd->chanlist[i]);
+   devpriv->dux_commands[1] = len;
+   for (i = 0; i < len; ++i) {
+   unsigned int chan = CR_CHAN(cmd->chanlist[i]);
+   unsigned int range = CR_RANGE(cmd->chanlist[i]);
+
if (i >= NUMCHANNELS)
break;
-   this_usbduxsub->dux_commands[i + 2] =
-   create_adc_command(chan, range);
-   }

-   result = send_dux_commands(dev, SENDADCOMMANDS);
-   if (result < 0) {
-   up(&this_usbduxsub->sem);
-   return result;
+   devpriv->dux_commands[i + 2] = create_adc_command(chan, range);
}

-   if (this_usbduxsub->high_speed) {
+   ret = send_dux_commands(dev, SENDADCOMMANDS);
+   if (ret < 0)
+   goto ai_cmd_exit;
+
+   if (devpriv->high_speed) {
/*
 * every channel gets a time window of 125us. Thus, if we
 * sample all 8 channels we need 1ms. If we sample only one
 * channel we need only 125us
 */
-   this_usbduxsub->ai_interval = 1;
+   devpriv->ai_interval = 1;
/* find a power of 2 for the interval */
-   while ((this_usbduxsub->ai_interval) < (cmd->chanlist_len)) {
-   this_usbduxsub->ai_interval =
-   (this_usbduxsub->ai_interval) * 2;
-   }
-   this_usbduxsub->ai_timer = cmd->scan_begin_arg / (125000 *
- (this_usbduxsub->
-  ai_interval));
+   while (devpriv->ai_interval < len)
+   devpriv->ai_interval *= 2;
+
+   devpriv->ai_timer = cmd->scan_begin_arg /
+   (125000 * devpriv->ai_interval);
} else {
/* interval always 1ms */
-   this_usbduxsub->ai_interval = 1;
-   this_usbduxsub->ai_timer = cmd->scan_begin_arg / 100;
+   devpriv->ai_interval = 1;
+   devpriv->ai_timer = cmd->scan_begin_arg / 100;
}
-   if (this_usbduxsub->ai_timer < 1) {
-   up(&this_usbduxsub->sem);
-   return -EINVAL;
+   if (devpriv->ai_timer < 1) {
+   ret = -EINVAL;
+   goto ai_cmd_exit;
}
-   this_usbduxsub->ai_counter = this_usbduxsub->ai_timer;
+
+   devpriv->ai_counter = devpriv->ai_timer;

if (cmd->stop_src == TRIG_COUNT) {
/* data arrives as one packet */
-   this_usbduxsub->ai_sample_count = cmd->stop_arg;
-   this_usbduxsub->ai_continous = 0;
+   devpriv->ai_sample_count = cmd->stop_arg;
+   devpriv->ai_continou

Re: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining memory blocks

2013-07-25 Thread Dave Hansen
On 07/25/2013 04:14 AM, KY Srinivasan wrote:
> As promised, I have sent out the patches for (a) an implementation of an 
> in-kernel API
> for onlining  and a consumer for this API. While I don't know the exact 
> reason why the
> user mode code is delayed (under some low memory conditions), what is the 
> harm in having
> a mechanism to online memory that has been hot added without involving user 
> space code.

KY, your potential problem, not being able to online more memory because
of a shortage of memory, is a serious one.

However, this potential issue exists in long-standing code, and
potentially affects all users of memory hotplug.  The problem has not
been described in sufficient detail for the rest of the developers to
tell if you are facing a new problem, or whether *any* proposed solution
will help the problem you face.

Your propsed solution changes the semantics of existing user/kernel
interfaces, duplicates existing functionality, and adds code complexity
to the kernel.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 33/53] staging: comedi: usbdux: tidy up usbdux_ai_inttrig()

2013-07-25 Thread Ian Abbott

On 2013-07-24 22:18, H Hartley Sweeten wrote:

Rename the local variable used for the private data pointer to the
comedi "norm".

Remove the unnecessary sanity check of the private data pointer. This
function can only be called is the private data was allocated during
the attach.

Tidy up the exit path using goto to ensure that the semaphore it
released.

Set the ai_cmd_running flag after submitting the urbs to remove the
need to clear the flag if the submit fails.


It's possible that (some of) the urbs could complete before you set the 
ai_cmd_running flag, and then usbdux_ai_urb_complete() would see the 
wrong value for ai_cmd_running.  (I don't know if it needs memory 
barriers as well - there are probably enough memory barriers in the usb 
subsystem already.)




Return -EBUSY if an ai command is already running.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
  drivers/staging/comedi/drivers/usbdux.c | 38 -
  1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index d0d683b..204867b 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -852,31 +852,31 @@ static int receive_dux_commands(struct comedi_device 
*dev, int command)
  }

  static int usbdux_ai_inttrig(struct comedi_device *dev,
-struct comedi_subdevice *s, unsigned int trignum)
+struct comedi_subdevice *s,
+unsigned int trignum)
  {
-   int ret;
-   struct usbdux_private *this_usbduxsub = dev->private;
-   if (!this_usbduxsub)
-   return -EFAULT;
+   struct usbdux_private *devpriv = dev->private;
+   int ret = -EINVAL;

-   down(&this_usbduxsub->sem);
+   down(&devpriv->sem);

-   if (trignum != 0) {
-   up(&this_usbduxsub->sem);
-   return -EINVAL;
-   }
-   if (!(this_usbduxsub->ai_cmd_running)) {
-   this_usbduxsub->ai_cmd_running = 1;
+   if (trignum != 0)
+   goto ai_trig_exit;
+
+   if (!devpriv->ai_cmd_running) {
ret = usbduxsub_submit_inurbs(dev);
-   if (ret < 0) {
-   this_usbduxsub->ai_cmd_running = 0;
-   up(&this_usbduxsub->sem);
-   return ret;
-   }
+   if (ret < 0)
+   goto ai_trig_exit;
+
+   devpriv->ai_cmd_running = 1;
s->async->inttrig = NULL;
+   } else {
+   ret = -EBUSY;
}
-   up(&this_usbduxsub->sem);
-   return 1;
+
+ai_trig_exit:
+   up(&devpriv->sem);
+   return ret;
  }

  static int usbdux_ai_cmd(struct comedi_device *dev, struct comedi_subdevice 
*s)




--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 33/53] staging: comedi: usbdux: tidy up usbdux_ai_inttrig()

2013-07-25 Thread Ian Abbott

On 2013-07-25 16:01, Ian Abbott wrote:

On 2013-07-24 22:18, H Hartley Sweeten wrote:

Rename the local variable used for the private data pointer to the
comedi "norm".

Remove the unnecessary sanity check of the private data pointer. This
function can only be called is the private data was allocated during
the attach.

Tidy up the exit path using goto to ensure that the semaphore it
released.

Set the ai_cmd_running flag after submitting the urbs to remove the
need to clear the flag if the submit fails.


It's possible that (some of) the urbs could complete before you set the
ai_cmd_running flag, and then usbdux_ai_urb_complete() would see the


I got the function name wrong - it's actually usbduxsub_ai_isoc_irq().


wrong value for ai_cmd_running.  (I don't know if it needs memory
barriers as well - there are probably enough memory barriers in the usb
subsystem already.)



--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 39/53] staging: comedi: usbdux: tidy up usbdux_ao_inttrig()

2013-07-25 Thread Ian Abbott

On 2013-07-24 22:21, H Hartley Sweeten wrote:

Rename the local variable used for the private data pointer to the
comedi "norm".

Remove the unnecessary sanity check of the private data pointer. This
function can only be called is the private data was allocated during
the attach.

Tidy up the exit path using goto to ensure that the semaphore it
released.

Set the ao_cmd_running flag after submitting the urbs to remove the
need to clear the flag if the submit fails.


As for AI, the URB completion handler usbduxsub_ao_isoc_irq() could 
execute before you set ao_cmd_running.




Return -EBUSY if an ao command is already running.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
  drivers/staging/comedi/drivers/usbdux.c | 38 -
  1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index 3898aea..9efd125 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -1072,31 +1072,31 @@ ao_write_exit:
  }

  static int usbdux_ao_inttrig(struct comedi_device *dev,
-struct comedi_subdevice *s, unsigned int trignum)
+struct comedi_subdevice *s,
+unsigned int trignum)
  {
-   int ret;
-   struct usbdux_private *this_usbduxsub = dev->private;
+   struct usbdux_private *devpriv = dev->private;
+   int ret = -EINVAL;

-   if (!this_usbduxsub)
-   return -EFAULT;
+   down(&devpriv->sem);

-   down(&this_usbduxsub->sem);
-   if (trignum != 0) {
-   up(&this_usbduxsub->sem);
-   return -EINVAL;
-   }
-   if (!(this_usbduxsub->ao_cmd_running)) {
-   this_usbduxsub->ao_cmd_running = 1;
+   if (trignum != 0)
+   goto ao_trig_exit;
+
+   if (!devpriv->ao_cmd_running) {
ret = usbduxsub_submit_outurbs(dev);
-   if (ret < 0) {
-   this_usbduxsub->ao_cmd_running = 0;
-   up(&this_usbduxsub->sem);
-   return ret;
-   }
+   if (ret < 0)
+   goto ao_trig_exit;
+
+   devpriv->ao_cmd_running = 1;
s->async->inttrig = NULL;
+   } else {
+   ret = -EBUSY;
}
-   up(&this_usbduxsub->sem);
-   return 1;
+
+ao_trig_exit:
+   up(&devpriv->sem);
+   return ret;
  }

  static int usbdux_ao_cmdtest(struct comedi_device *dev,




--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 40/53] staging: comedi: usbdux: tidy up usbdux_ao_cmd()

2013-07-25 Thread Ian Abbott

On 2013-07-24 22:22, H Hartley Sweeten wrote:

Rename the local variable used for the private data pointer to the
comedi "norm".

Remove the unnecessary sanity check of the private data pointer. This
function can only be called is the private data was allocated during
the attach.

Make sure an ao command is not already running and return -EBUSY.

Tidy up the exit path using goto to ensure that the semaphore is
released.

Set the ao_cmd_running flag after submitting the urbs to remove the
need to clear the flag if the submit fails.


Same as ao_inttrig, the URB completion handler could execute before you 
set ao_cmd_running.




Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
  drivers/staging/comedi/drivers/usbdux.c | 61 +
  1 file changed, 32 insertions(+), 29 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index 9efd125..f9fd0e3 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -1185,73 +1185,74 @@ static int usbdux_ao_cmdtest(struct comedi_device *dev,

  static int usbdux_ao_cmd(struct comedi_device *dev, struct comedi_subdevice 
*s)
  {
+   struct usbdux_private *devpriv = dev->private;
struct comedi_cmd *cmd = &s->async->cmd;
-   unsigned int chan, gain;
-   int i, ret;
-   struct usbdux_private *this_usbduxsub = dev->private;
+   int ret = -EBUSY;
+   int i;

-   if (!this_usbduxsub)
-   return -EFAULT;
+   down(&devpriv->sem);

-   down(&this_usbduxsub->sem);
+   if (devpriv->ao_cmd_running)
+   goto ao_cmd_exit;

/* set current channel of the running acquisition to zero */
s->async->cur_chan = 0;
+
for (i = 0; i < cmd->chanlist_len; ++i) {
-   chan = CR_CHAN(cmd->chanlist[i]);
-   gain = CR_RANGE(cmd->chanlist[i]);
+   unsigned int chan = CR_CHAN(cmd->chanlist[i]);
+
if (i >= NUMOUTCHANNELS)
break;
-   this_usbduxsub->dac_commands[i] = (chan << 6);
+
+   devpriv->dac_commands[i] = chan << 6;
}

/* we count in steps of 1ms (125us) */
/* 125us mode not used yet */
-   if (0) {/* (this_usbduxsub->high_speed) */
+   if (0) {/* (devpriv->high_speed) */
/* 125us */
/* timing of the conversion itself: every 125 us */
-   this_usbduxsub->ao_timer = cmd->convert_arg / 125000;
+   devpriv->ao_timer = cmd->convert_arg / 125000;
} else {
/* 1ms */
/* timing of the scan: we get all channels at once */
-   this_usbduxsub->ao_timer = cmd->scan_begin_arg / 100;
-   if (this_usbduxsub->ao_timer < 1) {
-   up(&this_usbduxsub->sem);
-   return -EINVAL;
+   devpriv->ao_timer = cmd->scan_begin_arg / 100;
+   if (devpriv->ao_timer < 1) {
+   ret = -EINVAL;
+   goto ao_cmd_exit;
}
}
-   this_usbduxsub->ao_counter = this_usbduxsub->ao_timer;
+
+   devpriv->ao_counter = devpriv->ao_timer;

if (cmd->stop_src == TRIG_COUNT) {
/* not continuous */
/* counter */
/* high speed also scans everything at once */
-   if (0) {/* (this_usbduxsub->high_speed) */
-   this_usbduxsub->ao_sample_count =
-   (cmd->stop_arg) * (cmd->scan_end_arg);
+   if (0) {/* (devpriv->high_speed) */
+   devpriv->ao_sample_count = cmd->stop_arg *
+  cmd->scan_end_arg;
} else {
/* there's no scan as the scan has been */
/* perf inside the FX2 */
/* data arrives as one packet */
-   this_usbduxsub->ao_sample_count = cmd->stop_arg;
+   devpriv->ao_sample_count = cmd->stop_arg;
}
-   this_usbduxsub->ao_continous = 0;
+   devpriv->ao_continous = 0;
} else {
/* continous acquisition */
-   this_usbduxsub->ao_continous = 1;
-   this_usbduxsub->ao_sample_count = 0;
+   devpriv->ao_continous = 1;
+   devpriv->ao_sample_count = 0;
}

if (cmd->start_src == TRIG_NOW) {
/* enable this acquisition operation */
-   this_usbduxsub->ao_cmd_running = 1;
ret = usbduxsub_submit_outurbs(dev);
if (ret < 0) {
-   this_usbduxsub->ao_cmd_running = 0;
/* fixme: unlink here?? */
-   up(&this_usbduxsub->sem);
-  

Re: [PATCH 47/53] staging: comedi: usbdux: tidy up usbdux_pwm_start()

2013-07-25 Thread Ian Abbott

On 2013-07-24 22:24, H Hartley Sweeten wrote:

Rename the local variable used for the private data pointer to the
comedi "norm".

Use memset() to initialize the urb transfer_buffer.

Set the pwm_cmd_running after submitting the pwm urbs so we don't have
to clear it if the submit fails.


Same problem as for AI and AO subdevices in that the URB completion 
routine may execute before you set pwm_cmd_running.




Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
  drivers/staging/comedi/drivers/usbdux.c | 21 -
  1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index 2bfb1b4..7181bc3 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -1538,33 +1538,28 @@ static int usbdux_pwm_period(struct comedi_device *dev,
return 0;
  }

-/* is called from insn so there's no need to do all the sanity checks */
  static int usbdux_pwm_start(struct comedi_device *dev,
struct comedi_subdevice *s)
  {
-   int ret, i;
-   struct usbdux_private *this_usbduxsub = dev->private;
+   struct usbdux_private *devpriv = dev->private;
+   int ret;

-   if (this_usbduxsub->pwm_cmd_running) {
-   /* already running */
+   if (devpriv->pwm_cmd_running)
return 0;
-   }

-   this_usbduxsub->dux_commands[1] = ((int8_t) this_usbduxsub->pwn_delay);
+   devpriv->dux_commands[1] = devpriv->pwn_delay;
ret = send_dux_commands(dev, SENDPWMON);
if (ret < 0)
return ret;

/* initialise the buffer */
-   for (i = 0; i < this_usbduxsub->size_pwm_buf; i++)
-   ((char *)(this_usbduxsub->urb_pwm->transfer_buffer))[i] = 0;
+   memset(devpriv->urb_pwm->transfer_buffer, 0, devpriv->size_pwm_buf);

-   this_usbduxsub->pwm_cmd_running = 1;
ret = usbduxsub_submit_pwm_urbs(dev);
-   if (ret < 0) {
-   this_usbduxsub->pwm_cmd_running = 0;
+   if (ret < 0)
return ret;
-   }
+   devpriv->pwm_cmd_running = 1;
+
return 0;
  }





--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 00/53] staging: comedi: usbdux: cleanup driver

2013-07-25 Thread Ian Abbott

On 2013-07-24 22:05, H Hartley Sweeten wrote:

Move all the usb_driver (*probe) and (*disconnect) operations into the
comedi_driver (*auto_attach) and (*detach). This allows the per device
private data to be kzalloc()'d and removes the 16 device limitation.

Remove all the unnecessary information from the private data.

Tidy up the driver to remove any cruft.

H Hartley Sweeten (53):
   staging: comedi: usbdux: rename struct usbduxsub
   staging: comedi: usbdux: remove the usb_driver (*probe) noise
   staging: comedi: usbdux: tidy up usbdux_usb_probe()
   staging: comedi: usbdux: move usb buffer allocation into new function
   staging: comedi: usbdux: push usb (*disconnect) into comedi (*detach)
   staging: comedi: usbdux: push usb (*probe) into comedi (*auto_attach)
   staging: comedi: usbdux: remove unnecessary tidy_up() calls
   staging: comedi: usbdux: cleanup the (*detach)
   staging: comedi: usbdux: remove NOISY_DUX_DEBUGBUG
   staging: comedi: usbdux: tidy up usbdux_attach_common()
   staging: comedi: usbdux: absorb usbdux_attach_common into caller
   staging: comedi: usbdux: tidy up usbduxsub_ai_isoc_irq()
   staging: comedi: usbdux: tidy up usbduxsub_ao_isoc_irq()
   staging: comedi: usbdux: tidy up usbduxsub_pwm_irq()
   staging: comedi: usbdux: remove the SUBDEV_* defines
   staging: comedi: usbdux: tidy up the comedi_lrange tables
   staging: comedi: usbdux: remove dev_printk() noise
   staging: comedi: usbdux: tidy up usbduxsub_submit_inurbs()
   staging: comedi: usbdux: tidy up usbduxsub_submit_outurbs()
   staging: comedi: usbdux: tidy up usbduxsub_submit_pwm_urbs()
   staging: comedi: usbdux: remove 'comedidev' from private data
   staging: comedi: usbdux: remove 'interface' from private data
   staging: comedi: usbdux: tidy up send_dux_commands()
   staging: comedi: usbdux: tidy up receive_dux_commands()
   staging: comedi: usbdux: pass comedi_device pointer to 
usbdux_alloc_usb_buffers()
   staging: comedi: usbdux: remove usb_device back pointer from private data
   staging: comedi: usbdux: remove 'ifnum' from the private data
   staging: comedi: usbdux: make private data flags bit-fields
   staging: comedi: usbdux: tidy up usbdux_ai_stop()
   staging: comedi: usbdux: tidy up usbdux_ai_cancel()
   staging: comedi: usbdux: tidy up usbdux_ao_stop()
   staging: comedi: usbdux: tidy up usbdux_ao_cancel()
   staging: comedi: usbdux: tidy up usbdux_ai_inttrig()
   staging: comedi: usbdux: tidy up usbdux_ai_cmd()
   staging: comedi: usbdux: tidy up usbdux_ai_insn_read()
   staging: comedi: usbdux: clarify bipolar ai data
   staging: comedi: usbdux: tidy up usbdux_ao_insn_read()
   staging: comedi: usbdux: tidy up usbdux_ao_insn_write()
   staging: comedi: usbdux: tidy up usbdux_ao_inttrig()
   staging: comedi: usbdux: tidy up usbdux_ao_cmd()
   staging: comedi: usbdux: tidy up usbdux_dio_insn_config()
   staging: comedi: usbdux: tidy up usbdux_dio_insn_bits()
   staging: comedi: usbdux: fix usbdux_counter_read()
   staging: comedi: usbdux: fix usbdux_counter_write()
   staging: comedi: usbdux: tidy up usbdux_pwm_stop()
   staging: comedi: usbdux: fix usbdux_pwm_cancel()
   staging: comedi: usbdux: tidy up usbdux_pwm_start()
   staging: comedi: usbdux: tidy up unlink and stop helpers
   staging: comedi: usbdux: use the stop helpers in the detach
   staging: comedi: usbdux: remove the usb endpoint defines
   staging: comedi: usbdux: remove some unused defines
   staging: comedi: usbdux: move usbdux_firmware_upload()
   staging: comedi: usbdux: clarify bipolar ai data in usbduxsub_ai_isoc_irq()

  drivers/staging/comedi/drivers/usbdux.c | 2265 +++
  1 file changed, 822 insertions(+), 1443 deletions(-)


I commented on patches 33, 34, 39, 40 and 47 to do with the ordering of 
setting the foo_cmd_start variables and submitting the URBs, but the 
remaining look okay.


I wish I'd noticed the ordering problem when the patches for usbduxfast 
and usbduxsigma were submitted!


--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining memory blocks

2013-07-25 Thread Kay Sievers
On Thu, Jul 25, 2013 at 5:03 PM, Dave Hansen  wrote:
> On 07/25/2013 04:14 AM, KY Srinivasan wrote:
>> As promised, I have sent out the patches for (a) an implementation of an 
>> in-kernel API
>> for onlining  and a consumer for this API. While I don't know the exact 
>> reason why the
>> user mode code is delayed (under some low memory conditions), what is the 
>> harm in having
>> a mechanism to online memory that has been hot added without involving user 
>> space code.
>
> KY, your potential problem, not being able to online more memory because
> of a shortage of memory, is a serious one.
>
> However, this potential issue exists in long-standing code, and
> potentially affects all users of memory hotplug.  The problem has not
> been described in sufficient detail for the rest of the developers to
> tell if you are facing a new problem, or whether *any* proposed solution
> will help the problem you face.
>
> Your propsed solution changes the semantics of existing user/kernel
> interfaces, duplicates existing functionality, and adds code complexity
> to the kernel.

Complexity, well, it's just a bit of code which belongs in the kernel.
The mentioned unconditional hotplug loop through userspace is
absolutely pointless. Such defaults never belong in userspace tools if
they do not involve data that is only available in userspace and
something would make a decision about that. Saying "hello" to
userspace and usrspace has a hardcoded "yes" in return makes no sense
at all. The kernel can just go ahead and do its job, like it does for
all other devices it finds too.

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


RE: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining memory blocks

2013-07-25 Thread KY Srinivasan


> -Original Message-
> From: Dave Hansen [mailto:d...@sr71.net]
> Sent: Thursday, July 25, 2013 11:04 AM
> To: KY Srinivasan
> Cc: Michal Hocko; gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> a...@firstfloor.org; a...@linux-foundation.org; linux...@kvack.org;
> kamezawa.hiroy...@gmail.com; han...@cmpxchg.org; ying...@google.com;
> jasow...@redhat.com; k...@vrfy.org
> Subject: Re: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining
> memory blocks
> 
> On 07/25/2013 04:14 AM, KY Srinivasan wrote:
> > As promised, I have sent out the patches for (a) an implementation of an in-
> kernel API
> > for onlining  and a consumer for this API. While I don't know the exact 
> > reason
> why the
> > user mode code is delayed (under some low memory conditions), what is the
> harm in having
> > a mechanism to online memory that has been hot added without involving user
> space code.
> 
> KY, your potential problem, not being able to online more memory because
> of a shortage of memory, is a serious one.

All I can say is that the online is not happening within the allowed time (5 
seconds in
the current code).
> 
> However, this potential issue exists in long-standing code, and
> potentially affects all users of memory hotplug.  The problem has not
> been described in sufficient detail for the rest of the developers to
> tell if you are facing a new problem, or whether *any* proposed solution
> will help the problem you face.
> 
> Your propsed solution changes the semantics of existing user/kernel
> interfaces, duplicates existing functionality, and adds code complexity
> to the kernel.

How so? All I am doing is to use the existing infrastructure to online
memory. The only change I have made is to export an API that allows
onlining without involving any user space code. I don't see how  this
adds complexity to the kernel. This would be an useful extension as can
be seen from its usage in the Hyper-V balloon driver.

In my particular use case, I need to wait for the memory to be onlined before
I can proceed to hot add the next chunk. This synchronization can be completely
avoided if we can avoid the involvement of user level code. I would submit to 
you that
this is a valid use case that we ought to be able to support.

Regards,

K. Y



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


Re: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining memory blocks

2013-07-25 Thread Dave Hansen
On 07/25/2013 08:15 AM, Kay Sievers wrote:
> Complexity, well, it's just a bit of code which belongs in the kernel.
> The mentioned unconditional hotplug loop through userspace is
> absolutely pointless. Such defaults never belong in userspace tools if
> they do not involve data that is only available in userspace and
> something would make a decision about that. Saying "hello" to
> userspace and usrspace has a hardcoded "yes" in return makes no sense
> at all. The kernel can just go ahead and do its job, like it does for
> all other devices it finds too.

Sorry, but memory is different than all other devices.  You never need a
mouse in order to add another mouse to the kernel.

I'll repaste something I said earlier in this thread:

> A system under memory pressure is going to have troubles doing a
> hot-add.  You need memory to add memory.  Of the two operations ("add"
> and "online"), "add" is the one vastly more likely to fail.  It has to
> allocate several large swaths of contiguous physical memory.  For that
> reason, the system was designed so that you could "add" and "online"
> separately.  The intention was that you could "add" far in advance and
> then "online" under memory pressure, with the "online" having *VASTLY*
> smaller memory requirements and being much more likely to succeed.

So, no, it makes no sense to just have userspace always unconditionally
online all the memory that the kernel adds.  But, the way it's set up,
we _have_ a method that can work under lots memory pressure, and it is
available for users that want it.  It was designed 10 years ago, and
maybe it's outdated, or history has proved that nobody is going to use
it the way it was designed.

If I had it to do over again, I'd probably set up configurable per-node
sets of spare kernel metadata.  That way, you could say "make sure we
have enough memory reserved to add $FOO sections to node $BAR".  Use
that for the largest allocations, then depend on PF_MEMALLOC to get us
enough for the other little bits along the way.

Also, if this is a problem, it's going to be a problem for *EVERY* user
of memory hotplug, not just hyperv.  So, let's see it fixed generically
for *EVERY* user.  Something along the lines of:

1. Come up with an interface that specifies a default policy for
   newly-added memory sections.  Currently, added memory gets "added",
   but not "onlined", and the default should stay doing that.
2. Make sure that we at least WARN_ONCE() if someone tries to online an
   already-kernel-onlined memory section.  That way, if someone trips
   over this new policy, we have a _chance_ of explaining to them what
   is going on.
3. Think about what we do in the failure case where we are able to
   "add", but fail to "online" in the kernel.  Do we tear the
   newly-added structures down and back out of the operation, or do
   we leave the memory added, but offline (what happens in the normal
   case now)?



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


RE: [PATCH 01/13] staging: comedi: pcmuio: fix interrupt requests

2013-07-25 Thread H Hartley Sweeten
On Thursday, July 25, 2013 5:34 AM, Ian Abbott wrote:
> On 2013-07-24 19:45, H Hartley Sweeten wrote:
>> Legacy (ISA) interrupts are not sharable so this driver should not
>> be passing the IRQF_SHARED flag when requesting the interrupts.
>>
>> This driver supports two board types:
>>PCM-UIO48 with one asic (one interrupt source)
>>PCM-UIO96 with two asics (two interrupt sources)
>>
>> The PCM-UIO96 has a jumper that allows the two interrupt sources to
>> share an interrupt. This is safe for legacy interrupts as long as
>> the "shared" interrupt is handled by a single driver.
>>
>> Modify the request_irq() code in this driver to correctly request the
>> interrupts. For the PCM-UI048 case (one asic) only one request_irq()
>> is needed. For the PCM-UIO96 (two asics) there are two cases:
>>
>>1) interrupt is shared, one request_irq() call
>>2) two interrupts, two request_irq() calls
>>
>> Do the first request_irq() in all cases. Only do the second if the
>> boardinfo indicates that the board has two asics and the user passed
>> the 2nd interrupt number.
>>
>> Pack the two interrupt numbers in dev->irq instead of storing them in
>> the private data. During the detach of the board, this driver will
>> free the 2nd irq if needed and the core will free the 1st.
>>
>> Move the board reset and interrupt request code so it occurs early
>> in the attach. We can then check dev->irq to see if the subdevice
>> interrupt support actually needs to be initialized.
>>
>> Add a call to pcmuio_reset() in the (*detach) to make sure the
>> interrupts are disabled before freeing the irqs.
>>
>> Signed-off-by: H Hartley Sweeten 
>> Cc: Ian Abbott 
>> Cc: Greg Kroah-Hartman 
>> ---
>>   drivers/staging/comedi/drivers/pcmuio.c | 96 
>> ++---
>>   1 file changed, 54 insertions(+), 42 deletions(-)
>
> Packing the two irq values in a single member seems a bit hackish.  I 
> think adding an 'irq2' member to struct pcmuio_private would be cleaner.

It is a bit "hackish" but the hack is documented with comments. The 'irq' member
is only used by the core in comedi_legacy_detach() so the hack will not have any
side effects.



>> @@ -617,6 +607,39 @@ static int pcmuio_attach(struct comedi_device *dev, 
>> struct comedi_devconfig *it)
>>  for (asic = 0; asic < PCMUIO_MAX_ASICS; ++asic)
>>  spin_lock_init(&devpriv->asics[asic].spinlock);
>>
>> +pcmuio_reset(dev);
>> +
>> +/* request the 1st irq */
>> +irq = it->options[1];
>> +if (irq) {
>> +ret = request_irq(irq, pcmuio_interrupt, 0,
>> +  board->name, dev);
>> +if (ret == 0) {
>> +/* save the irq for the core to free */
>> +dev->irq = irq;
>> +}
>> +}
>> +
>> +/* request the 2nd irq if needed */
>> +if (board->num_asics == 2 && dev->irq) {
>> +irq = it->options[2];
>> +if (irq && irq != dev->irq) {
>> +/* 1st and 2nd irq differ */
>> +ret = request_irq(irq, pcmuio_interrupt, 0,
>> +  board->name, dev);
>> +if (ret == 0) {
>> +/* save the irq for (*detach) to free */
>> +dev->irq |= (irq << 8);
>> +} else {
>> +free_irq(dev->irq, dev);
>> +dev->irq = 0;
>> +}
>> +} else {
>> +/* 1st and 2nd irq are the same (or 2nd is zero) */
>> +dev->irq |= (irq << 8);
>> +}
>> +}
>> +
>
> It could possibly allow just the second IRQ to be used even if the first 
> IRQ is not used (0), but the original driver didn't allow that.

This might be desired?

>>  n_subdevs = board->num_asics * 2;
>>  devpriv->sprivs = kcalloc(n_subdevs, sizeof(*subpriv), GFP_KERNEL);
>>  if (!devpriv->sprivs)
>> @@ -639,7 +662,7 @@ static int pcmuio_attach(struct comedi_device *dev, 
>> struct comedi_devconfig *it)
>>  s->n_chan = 24;
>>
>>  /* subdevices 0 and 2 suppport interrupts */
>> -if ((sdev_no % 2) == 0) {
>> +if ((sdev_no % 2) == 0 && dev->irq) {
>
> You've checked if irq is set before allowing commands to be set-up, but 
> you haven't checked which of the possibly two irqs is set.

Hmm.. True.

I guess this test should really be:

if ((sdev_no % 2) == 0 && (dev->irq >> (8 * (sdev_no % 2 {

Then the subdevice command operations will only be hooked up if the irq is
available.

Can this be fixed as a separate patch or would you prefer that I rebase the
series?

Also, if you would prefer using a separate 'irq' member in the private data for
the 2nd irq let me know. This will of course effect multiple parts of the 
series.

Thanks,
Hartley

___
devel mailing list
de...@linuxdriver

RE: [PATCH 06/13] staging: comedi: pcmuio: pcmuio_handle_asic_interrupt() does not need the subdevice

2013-07-25 Thread H Hartley Sweeten
On Thursday, July 25, 2013 5:50 AM, Ian Abbott wrote:
> On 2013-07-24 19:47, H Hartley Sweeten wrote:
>> The comedi_subdevice pointer is not used in this function. Its simply
>> passed on to pcmuio_handle_intr_subdev(). That function then needs to
>> calculate the 'asic' associated with the subdevice. Just pass on the
>> 'asic' instead and let pcmuio_handle_intr_subdev() get the subdevice
>> from that.
>>
>> Signed-off-by: H Hartley Sweeten 
>> Cc: Ian Abbott 
>> Cc: Greg Kroah-Hartman 
>
> In that case, it would make sense to rename the function to 
> pcmuio_handle_intr_asic().

Or just pcmuio_handle_pending_intr(), then the comment could also be
removed.

This could be a separate patch. But I can rebase the series it you wish.

Regards,
Hartley

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


RE: [PATCH 07/13] staging: comedi: pcmuio: make sure input channels stay inputs

2013-07-25 Thread H Hartley Sweeten
On Thursday, July 25, 2013 6:39 AM, Ian Abbott wrote:
> On 2013-07-24 19:48, H Hartley Sweeten wrote:
>> When updating the output channels make sure the channels configured as
>> inputs stay inputs.
>>
>> Signed-off-by: H Hartley Sweeten 
>> Cc: Ian Abbott 
>> Cc: Greg Kroah-Hartman 
>> ---
>>   drivers/staging/comedi/drivers/pcmuio.c | 8 +++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/comedi/drivers/pcmuio.c 
>> b/drivers/staging/comedi/drivers/pcmuio.c
>> index 076a08a..5b0ade2 100644
>> --- a/drivers/staging/comedi/drivers/pcmuio.c
>> +++ b/drivers/staging/comedi/drivers/pcmuio.c
>> @@ -215,8 +215,14 @@ static int pcmuio_dio_insn_bits(struct comedi_device 
>> *dev,
>>  s->state &= ~mask;
>>  s->state |= (mask & bits);
>>
>> -/* invert the state and update the channels */
>> +/*
>> + * Invert the state and update the channels.
>> + *
>> + * The s->io_bits mask makes sure inputs are '0' so
>> + * that the output pins stay in a high-z state.
>> + */
>>  val = s->state ^ ((0x1 << s->n_chan) - 1);
>> +val &= s->io_bits;
>>  pcmuio_write(dev, val, asic, 0, port);
>>  }
>>
>>
>
> That's fine although the original function has other problems, for 
> example the value returned in data[1] should reflect the modified data, 
> not the original data and probably shouldn't be modifying s->state based 
> on the data read from the hardware.  I think the function should be 
> something like this:
>
> static int pcmuio_dio_insn_bits(struct comedi_device *dev,
>   struct comedi_subdevice *s,
>   struct comedi_insn *insn,
>   unsigned int *data)
> {
>   unsigned int chanmask = ((1 << s->n_chan) - 1;
>   unsigned int mask = data[0] & chanmask;
>   unsigned int bits = data[1];
>   int asic = s->index / 2;
>   int port = (s->index % 2) ? 3 : 0;
>   unsigned int val;
> 
>   if (mask) {
>   s->state &= ~mask;
>   s->state |= (mask & bits);
>   /*
>* Invert the state and update the channels.
>*
>* The s->io_bits mask makes sure inputs are '0' so
>* that the output pins stay in a high-z state.
>*/
>   val = ~s->state & s->io_bits & chanmask;
>   pcmuio_write(dev, val, asic, 0, port);
>   }
>
>   /* get inverted state of the channels from the port */
>   val = pcmuio_read(dev, asic, 0, port);
> 
>   /* get the true state of the channels */
>   data[1] = ~val & chanmask;
>
>   return insn->n;
> }

This routine made my head hurt...

End result I "think" both solutions end up doing the same thing. But, I like
your proposal better.

I'll try to get this series rebased today. Please let me know if you want me
to add the 'irq' member to the private data for the 2nd interrupt.

Regards,
Hartley

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


Re: [PATCH v2 1/2] staging: gdm7240: adding LTE USB driver

2013-07-25 Thread Greg KH
On Thu, Jul 25, 2013 at 02:56:51PM +0900, Won Kang wrote:
> Anyway, you are right that endianess matters only when using this SDK.
> If you suggest to remove the ioctl because SDK is optional, I will
> agree.

Thanks, I've now removed it from the driver.

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


RE: [PATCH 14/14] staging: comedi: ii_pci20kc: reorder subdevices

2013-07-25 Thread H Hartley Sweeten
On Thursday, July 25, 2013 7:18 AM, Ian Abbott wrote:
> On 2013-07-24 20:14, H Hartley Sweeten wrote:
>> Make the built-on dio subdevice 0 and the three add-on modules
>> subdevices 1-3. This allows the driver to consistently use the
>> ii20k_module_iobase() helper to get the base address needed to
>> access a the registers used by a given subdevice.
>>
>> Signed-off-by: H Hartley Sweeten 
>> Cc: Ian Abbott 
>> Cc: Greg Kroah-Hartman 
>
> It's best to avoid reordering the subdevices if you can help it to avoid 
> screwing with userspace unnecessarily.

This one can be dropped. It was added last just to make the 'iobase' use
consistent.

But...

Shouldn't the userspace be getting the subdevice using
comedi_find_subdevice_by_type()? Or can userspace directly
access a subdevice based on the index?

The original order was:
0 - AO or AI subdevice (module 1)
1 - AO or AI subdevice (module 2)
2 - AO or AI subdevice (module 3)
3 - DIO subdevice (built-in module)

The new order is:
0 - DIO subdevice (built-in module)
1 - AO or AI subdevice (module 1)
2 - AO or AI subdevice (module 2)
3 - AO or AI subdevice (module 3)

So logically the add-on modules are still in the same order. So
the first call of comedi_find_subdevice_by_type() to find the
AO or AI subdevice will still return the module 1 subdevice,
second will return module 2, and third will return module 3.
I see no functional change in the userspace interaction.

Thanks,
Hartley

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


RE: [PATCH 00/53] staging: comedi: usbdux: cleanup driver

2013-07-25 Thread H Hartley Sweeten
On Thursday, July 25, 2013 8:25 AM, Ian Abbott wrote:
> On 2013-07-24 22:05, H Hartley Sweeten wrote:
>> Move all the usb_driver (*probe) and (*disconnect) operations into the
>> comedi_driver (*auto_attach) and (*detach). This allows the per device
>> private data to be kzalloc()'d and removes the 16 device limitation.
>>
>> Remove all the unnecessary information from the private data.
>>
>> Tidy up the driver to remove any cruft.
>>
>> H Hartley Sweeten (53):



> I commented on patches 33, 34, 39, 40 and 47 to do with the ordering of 
> setting the foo_cmd_start variables and submitting the URBs, but the 
> remaining look okay.

I'll rebase this series and drop those changes.

> I wish I'd noticed the ordering problem when the patches for usbduxfast 
> and usbduxsigma were submitted!

I'll need to fix those as well.

I'm hoping to merge the usbdux* drivers. They are all _very_ similar. The
Only real differences appears to be low-level data packets sent to the usb
device. I'm still trying to unwind it.

Thanks,
Hartley


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


RE: [PATCH 00/53] staging: comedi: usbdux: cleanup driver

2013-07-25 Thread H Hartley Sweeten
On Thursday, July 25, 2013 10:08 AM, H Hartley Sweeten wrote:
> On Thursday, July 25, 2013 8:25 AM, Ian Abbott wrote:
>> On 2013-07-24 22:05, H Hartley Sweeten wrote:
>>> Move all the usb_driver (*probe) and (*disconnect) operations into the
>>> comedi_driver (*auto_attach) and (*detach). This allows the per device
>>> private data to be kzalloc()'d and removes the 16 device limitation.
>>>
>>> Remove all the unnecessary information from the private data.
>>>
>>> Tidy up the driver to remove any cruft.
>>>
>>> H Hartley Sweeten (53):
>
>> I commented on patches 33, 34, 39, 40 and 47 to do with the ordering of 
>> setting the foo_cmd_start variables and submitting the URBs, but the 
>> remaining look okay.
>
> I'll rebase this series and drop those changes.

Greg,

If you don't see any issues with patches 1-32 of this series, could you
apply them? That way I don't have to spam the list with those ones
again.

Thanks,
Hartley

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


Re: [PATCH] staging: zcache: fix "zcache=" kernel parameter

2013-07-25 Thread Greg KH
On Thu, Jul 25, 2013 at 12:13:54PM +0200, Bartlomiej Zolnierkiewicz wrote:
> From: Piotr Sarna 



This whole email is base64, making it impossible for me to add the
needed Cc: stable@ tag to it...

Try it again?  It shouldn't be this hard...

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


Re: [PATCH 00/53] staging: comedi: usbdux: cleanup driver

2013-07-25 Thread Greg Kroah-Hartman
On Thu, Jul 25, 2013 at 12:14:44PM -0500, H Hartley Sweeten wrote:
> On Thursday, July 25, 2013 10:08 AM, H Hartley Sweeten wrote:
> > On Thursday, July 25, 2013 8:25 AM, Ian Abbott wrote:
> >> On 2013-07-24 22:05, H Hartley Sweeten wrote:
> >>> Move all the usb_driver (*probe) and (*disconnect) operations into the
> >>> comedi_driver (*auto_attach) and (*detach). This allows the per device
> >>> private data to be kzalloc()'d and removes the 16 device limitation.
> >>>
> >>> Remove all the unnecessary information from the private data.
> >>>
> >>> Tidy up the driver to remove any cruft.
> >>>
> >>> H Hartley Sweeten (53):
> >
> >> I commented on patches 33, 34, 39, 40 and 47 to do with the ordering of 
> >> setting the foo_cmd_start variables and submitting the URBs, but the 
> >> remaining look okay.
> >
> > I'll rebase this series and drop those changes.
> 
> Greg,
> 
> If you don't see any issues with patches 1-32 of this series, could you
> apply them? That way I don't have to spam the list with those ones
> again.

Yes, I will go do that after lunch, thanks.

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


[PATCH] Staging: olpc_dcon: change to msleep to usleep_range

2013-07-25 Thread Jens Frederich
The resolution of msleep is related to HZ, so with HZ set to
100 any msleep of less then 10ms will become ~10ms. This is
not what we want. Use usleep_range to get more control of
what is happening here.

Signed-off-by: Jens Frederich 
---
 drivers/staging/olpc_dcon/olpc_dcon.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/olpc_dcon/olpc_dcon.c 
b/drivers/staging/olpc_dcon/olpc_dcon.c
index 193e1c6..7c460f2 100644
--- a/drivers/staging/olpc_dcon/olpc_dcon.c
+++ b/drivers/staging/olpc_dcon/olpc_dcon.c
@@ -131,13 +131,13 @@ power_up:
pr_warn("unable to force dcon to power up: %d!\n", x);
return x;
}
-   msleep(10); /* we'll be conservative */
+   usleep_range(1, 11000);  /* we'll be conservative */
}
 
pdata->bus_stabilize_wiggle();
 
for (x = -1, timeout = 50; timeout && x < 0; timeout--) {
-   msleep(1);
+   usleep_range(1000, 1100);
x = dcon_read(dcon, DCON_REG_ID);
}
if (x < 0) {
-- 
1.7.9.5

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


[PATCH -next] staging: comedi: Add missing #include

2013-07-25 Thread Geert Uytterhoeven
sparc64 allmodconfig:

drivers/staging/comedi/drivers/addi_apci_2032.c: In function 
'apci2032_auto_attach':
drivers/staging/comedi/drivers/addi_apci_2032.c:328:3: error: implicit 
declaration of function 'kzalloc' [-Werror=implicit-function-declaration]
drivers/staging/comedi/drivers/addi_apci_2032.c:328:11: warning: assignment 
makes pointer from integer without a cast [enabled by default]
drivers/staging/comedi/drivers/addi_apci_2032.c: In function 'apci2032_detach':
drivers/staging/comedi/drivers/addi_apci_2032.c:350:3: error: implicit 
declaration of function 'kfree' [-Werror=implicit-function-declaration]

drivers/staging/comedi/drivers/amplc_pci224.c: In function 
'pci224_attach_common':
drivers/staging/comedi/drivers/amplc_pci224.c:1289:2: error: implicit 
declaration of function 'kmalloc' [-Werror=implicit-function-declaration]
drivers/staging/comedi/drivers/amplc_pci224.c:1289:23: warning: assignment 
makes pointer from integer without a cast [enabled by default]
drivers/staging/comedi/drivers/amplc_pci224.c:1296:24: warning: assignment 
makes pointer from integer without a cast [enabled by default]
drivers/staging/comedi/drivers/amplc_pci224.c:1303:25: warning: assignment 
makes pointer from integer without a cast [enabled by default]
drivers/staging/comedi/drivers/amplc_pci224.c:1348:42: warning: assignment 
makes pointer from integer without a cast [enabled by default]
drivers/staging/comedi/drivers/amplc_pci224.c: In function 'pci224_detach':
drivers/staging/comedi/drivers/amplc_pci224.c:1474:3: error: implicit 
declaration of function 'kfree' [-Werror=implicit-function-declaration]

Introduced by commit 0bdab509bf9c6d838dc0a3b1d68bbf841fc20b5a ("staging:
comedi: use comedi_alloc_devpriv()"), which removed some inclusions of
.

Signed-off-by: Geert Uytterhoeven 
---
http://kisskb.ellerman.id.au/kisskb/buildresult/9187516/
http://kisskb.ellerman.id.au/kisskb/buildresult/9186576/

 drivers/staging/comedi/drivers/addi_apci_2032.c |1 +
 drivers/staging/comedi/drivers/amplc_pci224.c   |1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/staging/comedi/drivers/addi_apci_2032.c 
b/drivers/staging/comedi/drivers/addi_apci_2032.c
index cb5d26a..6b0ea16 100644
--- a/drivers/staging/comedi/drivers/addi_apci_2032.c
+++ b/drivers/staging/comedi/drivers/addi_apci_2032.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "../comedidev.h"
 #include "addi_watchdog.h"
diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c 
b/drivers/staging/comedi/drivers/amplc_pci224.c
index 2fc56c4..179de53 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -101,6 +101,7 @@ Caveats:
 #include 
 #include 
 #include 
+#include 
 
 #include "../comedidev.h"
 
-- 
1.7.9.5

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


Re: [PATCH v6] staging: New driver: Xillybus generic interface for FPGA

2013-07-25 Thread Greg KH
On Mon, Jun 24, 2013 at 06:55:47PM +0300, Eli Billauer wrote:
> This is the driver for Xillybus, which is a general-purpose interface for
> data communication with FPGAs (programmable logic). Please refer to the
> README included in this patch for a detailed explanation.
> 
> It was previously submitted for misc-devices, but it appears like noone's
> willing to review the code (which I can understand, given its magnitude).
> Hence submitted as a staging driver.

I've applied this now, but can you send a follow-on patch that fixes up
all of the sparse errors that the driver(s) produce?

thanks,

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


Re: [PATCH] staging: dwc2: add microframe scheduler from downstream Pi kernel

2013-07-25 Thread Matthijs Kooijman
Hi Paul,

> > Furthermore, I wonder about how this scheduler works exactly. What I see
> > is:
> >  - the number usecs needed for a single transfer in a periodic qh is
> >calculated
> >  - When the qh is scheduled, the first of the 8 microframes with enough
> >usecs available is picked for this qh (disregarding FS transfers that
> >don't fit in 1 microframe for now)
> > 
> > However, this seems correct only for endpoints with an interval of 8
> > microframes? If an isoc endpoint has an interval of 1, it should be
> > scheduled in _every_ microframe, right?
> > 
> > The old code was pessimistic (the dwc2_check_periodic_bandwidth
> > essentially assumes an interval of 1 for everything) but the new code is
> > actually too optimistic (as it essentially assumes an interval of 8 for
> > everything).
> > 
> > This leads me to believe that this code works because it makes the
> > scheduler way to optimistic and because it removes the "one channel per
> > periodic endpoint" policy (which is way too optimistic), but it would
> > break when adding too much (periodic) devices (in nasty ways, no nice
> > "not enough bandwidth" messages).

It occurred to me that there is in fact more merit to this scheduler
than I originally thought. Before, I thought its only actual purpose was
to detect if there was room or not in the schedule for a new periodic
transfer. (and if I'm not mistaken, it does that badly when intervals of
< 8 uframes are involved).

However, what this scheduler also does is (obviously...) scheduling :-)
It makes sure that the various periodic transfers actually get sent to
the hardware spaced over different uframes instead of all at the first
or last uframe in a frame as before. This allows the driver to actually
accept more than 1 uframe worth of periodic transfers.


> > Overall, I don't think this patch is not even nearly ready to be
> > merged...
>
> Well, I disagree :)
I would argue that at least any questions about what this patch does and
why need to be resolved. Even if you did not write the code, you're the
one submitting it. Also, regardless of who wrote the code, the quality
of it should be high enough to accept in mainline. And frankly, I do not
think "But it works" is sufficient indication of quality by itself.

What I'm afraid of, is that if we introduce some code now that isn't
really needed or solves an unknown problem in the wrong way and we'll
end up carrying that code around without properly understanding what
it's for forever.

Of course this is just how I _think_ things should work, I'm not really
in a position to decide anything. So I'll just stick to commenting on
the code now, as far as I understand it, and try to understand better so
I can offer suggestions for improvements.

> > > - chan = list_first_entry(&hsotg->free_hc_list, struct dwc2_host_chan,
> > > - hc_list_entry);
> > > + if (!chan) {
> > > + dev_dbg(hsotg->dev, "No free channel to assign\n");
> > > + return -ENOMEM;
> > > + }
> > As above, I don't think this can ever happen.
> 
> Sure it can. If hsotg->free_hc_list is empty on entry to this
> function, for example.
Ah, you're right.

> > Only the addition of nak_frame and frame_usecs seem relevant to this
> > patch, the rest seems noise. And those variables could use some actual
> > documentation. I propose:
> > 
> >  * @nak_frame:  (Micro)frame number in which this qh was
> > re-queued because a NAK was received. Is 0x
> > when no NAK was received.
> >  * @frame_usecs:Internal variable used by the microframe scheduler
> 
> No, its not noise. I took the opportunity while adding the new
> members to reorder the existing ones from smallest to largest, to
> reduce the amount of wasted space due to structure padding. I think
> that's a legitimate thing to do in this patch.
I'm not saying it's a useless change, I meant it is noise for _this_
patch and should go into a separate one.

Gr.

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


[PATCH 00/30] staging: comedi: usbdux: cleanup driver take 2

2013-07-25 Thread H Hartley Sweeten
Patches 1-32 of [PATCH 00/53] staging: comedi: usbdux: cleanup driver
have already be applied to Greg Kroah-Hartman's staging tree. This
continues the cleanup of the usbdux comedi driver from that point.

Patches 1, 2, 7, 9, and 15 have been updated to address an issue
pointed out by Ian Abbott concerning the urb completion routine
executing before the *_cmd_running flag has been set.

The last 9 patches in this series are new. These have been added to
work toward merging the usbduxfast driver into this one.

H Hartley Sweeten (30):
  staging: comedi: usbdux: tidy up usbdux_ai_inttrig()
  staging: comedi: usbdux: tidy up usbdux_ai_cmd()
  staging: comedi: usbdux: tidy up usbdux_ai_insn_read()
  staging: comedi: usbdux: clarify bipolar ai data
  staging: comedi: usbdux: tidy up usbdux_ao_insn_read()
  staging: comedi: usbdux: tidy up usbdux_ao_insn_write()
  staging: comedi: usbdux: tidy up usbdux_ao_inttrig()
  staging: comedi: usbdux: tidy up usbdux_ao_cmd()
  staging: comedi: usbdux: tidy up usbdux_dio_insn_config()
  staging: comedi: usbdux: tidy up usbdux_dio_insn_bits()
  staging: comedi: usbdux: fix usbdux_counter_read()
  staging: comedi: usbdux: fix usbdux_counter_write()
  staging: comedi: usbdux: tidy up usbdux_pwm_stop()
  staging: comedi: usbdux: fix usbdux_pwm_cancel()
  staging: comedi: usbdux: fix usbdux_pwm_start()
  staging: comedi: usbdux: tidy up unlink and stop helpers
  staging: comedi: usbdux: use the stop helpers in the detach
  staging: comedi: usbdux: remove the usb endpoint defines
  staging: comedi: usbdux: remove some unused defines
  staging: comedi: usbdux: move usbdux_firmware_upload()
  staging: comedi: usbdux: clarify bipolar ai data in usbduxsub_ai_isoc_irq()
  staging: comedi: usbdux: rename private data variables
  staging: comedi: usbdux: cleanup the private data 'outBuffer'
  staging: comedi: usbdux: simplify initializing the ao urb transfer_buffer
  staging: comedi: usbdux: remove unnecessary check in usbdux_ai_cmd()
  staging: comedi: usbdux: remove unnecessary check in usbdux_ao_cmd()
  staging: comedi: usbdux: 'dac_commands' does not need to be kzalloc()'d
  staging: comedi: usbdux: remove unused define
  staging: comedi: usbdux: move and rename the bulk transfer commands
  staging: comedi: usbdux: consolidate usbduxsub_unlink_{in,out}urbs()

 drivers/staging/comedi/drivers/usbdux.c | 1094 ++-
 1 file changed, 494 insertions(+), 600 deletions(-)

-- 
1.8.3.2

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


[PATCH 02/30] staging: comedi: usbdux: tidy up usbdux_ai_cmd()

2013-07-25 Thread H Hartley Sweeten
Rename the local variable used for the private data pointer to the
comedi "norm".

Remove the unnecessary sanity check of the private data pointer. This
function can only be called is the private data was allocated during
the attach.

Tidy up the exit path using goto to ensure that the semaphore is
released.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 93 -
 1 file changed, 45 insertions(+), 48 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index 95c11e8..aebc728 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -882,85 +882,79 @@ ai_trig_exit:
 
 static int usbdux_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 {
+   struct usbdux_private *devpriv = dev->private;
struct comedi_cmd *cmd = &s->async->cmd;
-   unsigned int chan, range;
-   int i, ret;
-   struct usbdux_private *this_usbduxsub = dev->private;
-   int result;
-
-   if (!this_usbduxsub)
-   return -EFAULT;
+   int len = cmd->chanlist_len;
+   int ret = -EBUSY;
+   int i;
 
/* block other CPUs from starting an ai_cmd */
-   down(&this_usbduxsub->sem);
-   if (this_usbduxsub->ai_cmd_running) {
-   up(&this_usbduxsub->sem);
-   return -EBUSY;
-   }
+   down(&devpriv->sem);
+
+   if (devpriv->ai_cmd_running)
+   goto ai_cmd_exit;
+
/* set current channel of the running acquisition to zero */
s->async->cur_chan = 0;
 
-   this_usbduxsub->dux_commands[1] = cmd->chanlist_len;
-   for (i = 0; i < cmd->chanlist_len; ++i) {
-   chan = CR_CHAN(cmd->chanlist[i]);
-   range = CR_RANGE(cmd->chanlist[i]);
+   devpriv->dux_commands[1] = len;
+   for (i = 0; i < len; ++i) {
+   unsigned int chan = CR_CHAN(cmd->chanlist[i]);
+   unsigned int range = CR_RANGE(cmd->chanlist[i]);
+
if (i >= NUMCHANNELS)
break;
-   this_usbduxsub->dux_commands[i + 2] =
-   create_adc_command(chan, range);
-   }
 
-   result = send_dux_commands(dev, SENDADCOMMANDS);
-   if (result < 0) {
-   up(&this_usbduxsub->sem);
-   return result;
+   devpriv->dux_commands[i + 2] = create_adc_command(chan, range);
}
 
-   if (this_usbduxsub->high_speed) {
+   ret = send_dux_commands(dev, SENDADCOMMANDS);
+   if (ret < 0)
+   goto ai_cmd_exit;
+
+   if (devpriv->high_speed) {
/*
 * every channel gets a time window of 125us. Thus, if we
 * sample all 8 channels we need 1ms. If we sample only one
 * channel we need only 125us
 */
-   this_usbduxsub->ai_interval = 1;
+   devpriv->ai_interval = 1;
/* find a power of 2 for the interval */
-   while ((this_usbduxsub->ai_interval) < (cmd->chanlist_len)) {
-   this_usbduxsub->ai_interval =
-   (this_usbduxsub->ai_interval) * 2;
-   }
-   this_usbduxsub->ai_timer = cmd->scan_begin_arg / (125000 *
- (this_usbduxsub->
-  ai_interval));
+   while (devpriv->ai_interval < len)
+   devpriv->ai_interval *= 2;
+
+   devpriv->ai_timer = cmd->scan_begin_arg /
+   (125000 * devpriv->ai_interval);
} else {
/* interval always 1ms */
-   this_usbduxsub->ai_interval = 1;
-   this_usbduxsub->ai_timer = cmd->scan_begin_arg / 100;
+   devpriv->ai_interval = 1;
+   devpriv->ai_timer = cmd->scan_begin_arg / 100;
}
-   if (this_usbduxsub->ai_timer < 1) {
-   up(&this_usbduxsub->sem);
-   return -EINVAL;
+   if (devpriv->ai_timer < 1) {
+   ret = -EINVAL;
+   goto ai_cmd_exit;
}
-   this_usbduxsub->ai_counter = this_usbduxsub->ai_timer;
+
+   devpriv->ai_counter = devpriv->ai_timer;
 
if (cmd->stop_src == TRIG_COUNT) {
/* data arrives as one packet */
-   this_usbduxsub->ai_sample_count = cmd->stop_arg;
-   this_usbduxsub->ai_continous = 0;
+   devpriv->ai_sample_count = cmd->stop_arg;
+   devpriv->ai_continous = 0;
} else {
/* continous acquisition */
-   this_usbduxsub->ai_continous = 1;
-   this_usbduxsub->ai_sample_count = 0;
+   devpriv->ai_continous = 1;
+   devpriv->ai_sample_count = 0;
}
 
if (c

[PATCH 01/30] staging: comedi: usbdux: tidy up usbdux_ai_inttrig()

2013-07-25 Thread H Hartley Sweeten
Rename the local variable used for the private data pointer to the
comedi "norm".

Remove the unnecessary sanity check of the private data pointer. This
function can only be called is the private data was allocated during
the attach.

Tidy up the exit path using goto to ensure that the semaphore is
released.

Return -EBUSY if an ai command is already running.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 35 +
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index d0d683b..95c11e8 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -852,31 +852,32 @@ static int receive_dux_commands(struct comedi_device 
*dev, int command)
 }
 
 static int usbdux_ai_inttrig(struct comedi_device *dev,
-struct comedi_subdevice *s, unsigned int trignum)
+struct comedi_subdevice *s,
+unsigned int trignum)
 {
-   int ret;
-   struct usbdux_private *this_usbduxsub = dev->private;
-   if (!this_usbduxsub)
-   return -EFAULT;
+   struct usbdux_private *devpriv = dev->private;
+   int ret = -EINVAL;
 
-   down(&this_usbduxsub->sem);
+   down(&devpriv->sem);
 
-   if (trignum != 0) {
-   up(&this_usbduxsub->sem);
-   return -EINVAL;
-   }
-   if (!(this_usbduxsub->ai_cmd_running)) {
-   this_usbduxsub->ai_cmd_running = 1;
+   if (trignum != 0)
+   goto ai_trig_exit;
+
+   if (!devpriv->ai_cmd_running) {
+   devpriv->ai_cmd_running = 1;
ret = usbduxsub_submit_inurbs(dev);
if (ret < 0) {
-   this_usbduxsub->ai_cmd_running = 0;
-   up(&this_usbduxsub->sem);
-   return ret;
+   devpriv->ai_cmd_running = 0;
+   goto ai_trig_exit;
}
s->async->inttrig = NULL;
+   } else {
+   ret = -EBUSY;
}
-   up(&this_usbduxsub->sem);
-   return 1;
+
+ai_trig_exit:
+   up(&devpriv->sem);
+   return ret;
 }
 
 static int usbdux_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
-- 
1.8.3.2

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


[PATCH 04/30] staging: comedi: usbdux: clarify bipolar ai data

2013-07-25 Thread H Hartley Sweeten
Use the comedi_range_is_bipolar() helper instead of checking the
'range' index against a magic number.

Also, use the s->maxdata to calculate the value needed to munge the
value for bipolar data instead of the magic number.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index a536fa3..a81b4b1 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -1002,8 +1002,10 @@ static int usbdux_ai_insn_read(struct comedi_device *dev,
goto ai_read_exit;
 
val = le16_to_cpu(devpriv->insn_buffer[1]);
-   if (range <= 1)
-   val ^= 0x800;
+
+   /* bipolar data is two's-complement */
+   if (comedi_range_is_bipolar(s, range))
+   val ^= ((s->maxdata + 1) >> 1);
 
data[i] = val;
}
-- 
1.8.3.2

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


[PATCH 03/30] staging: comedi: usbdux: tidy up usbdux_ai_insn_read()

2013-07-25 Thread H Hartley Sweeten
Rename the local variable used for the private data pointer to the
comedi "norm".

Remove the unnecessary sanity check of the private data pointer. This
function can only be called is the private data was allocated during
the attach.

Tidy up the exit path using goto to ensure that the semaphore is
released.

Return -EBUSY instead of 0 if the (*insn_read) cannot be done because
a command is running.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 59 +++--
 1 file changed, 27 insertions(+), 32 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index aebc728..a536fa3 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -973,50 +973,45 @@ ai_cmd_exit:
 /* Mode 0 is used to get a single conversion on demand */
 static int usbdux_ai_insn_read(struct comedi_device *dev,
   struct comedi_subdevice *s,
-  struct comedi_insn *insn, unsigned int *data)
+  struct comedi_insn *insn,
+  unsigned int *data)
 {
+   struct usbdux_private *devpriv = dev->private;
+   unsigned int chan = CR_CHAN(insn->chanspec);
+   unsigned int range = CR_RANGE(insn->chanspec);
+   unsigned int val;
+   int ret = -EBUSY;
int i;
-   unsigned int one = 0;
-   int chan, range;
-   int err;
-   struct usbdux_private *this_usbduxsub = dev->private;
 
-   if (!this_usbduxsub)
-   return 0;
+   down(&devpriv->sem);
 
-   down(&this_usbduxsub->sem);
-   if (this_usbduxsub->ai_cmd_running) {
-   up(&this_usbduxsub->sem);
-   return 0;
-   }
+   if (devpriv->ai_cmd_running)
+   goto ai_read_exit;
 
-   /* sample one channel */
-   chan = CR_CHAN(insn->chanspec);
-   range = CR_RANGE(insn->chanspec);
/* set command for the first channel */
-   this_usbduxsub->dux_commands[1] = create_adc_command(chan, range);
+   devpriv->dux_commands[1] = create_adc_command(chan, range);
 
/* adc commands */
-   err = send_dux_commands(dev, SENDSINGLEAD);
-   if (err < 0) {
-   up(&this_usbduxsub->sem);
-   return err;
-   }
+   ret = send_dux_commands(dev, SENDSINGLEAD);
+   if (ret < 0)
+   goto ai_read_exit;
 
for (i = 0; i < insn->n; i++) {
-   err = receive_dux_commands(dev, SENDSINGLEAD);
-   if (err < 0) {
-   up(&this_usbduxsub->sem);
-   return 0;
-   }
-   one = le16_to_cpu(this_usbduxsub->insn_buffer[1]);
-   if (CR_RANGE(insn->chanspec) <= 1)
-   one = one ^ 0x800;
+   ret = receive_dux_commands(dev, SENDSINGLEAD);
+   if (ret < 0)
+   goto ai_read_exit;
+
+   val = le16_to_cpu(devpriv->insn_buffer[1]);
+   if (range <= 1)
+   val ^= 0x800;
 
-   data[i] = one;
+   data[i] = val;
}
-   up(&this_usbduxsub->sem);
-   return i;
+
+ai_read_exit:
+   up(&devpriv->sem);
+
+   return ret ? ret : insn->n;
 }
 
 //
-- 
1.8.3.2

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


[PATCH 05/30] staging: comedi: usbdux: tidy up usbdux_ao_insn_read()

2013-07-25 Thread H Hartley Sweeten
Rename the local variable used for the private data pointer to the
comedi "norm".

Remove the unnecessary sanity check of the private data pointer. This
function can only be called is the private data was allocated during
the attach.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 21 -
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index a81b4b1..4159936 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -1016,26 +1016,21 @@ ai_read_exit:
return ret ? ret : insn->n;
 }
 
-//
-/* analog out */
-
 static int usbdux_ao_insn_read(struct comedi_device *dev,
   struct comedi_subdevice *s,
-  struct comedi_insn *insn, unsigned int *data)
+  struct comedi_insn *insn,
+  unsigned int *data)
 {
+   struct usbdux_private *devpriv = dev->private;
+   unsigned int chan = CR_CHAN(insn->chanspec);
int i;
-   int chan = CR_CHAN(insn->chanspec);
-   struct usbdux_private *this_usbduxsub = dev->private;
 
-   if (!this_usbduxsub)
-   return -EFAULT;
-
-   down(&this_usbduxsub->sem);
+   down(&devpriv->sem);
for (i = 0; i < insn->n; i++)
-   data[i] = this_usbduxsub->out_buffer[chan];
+   data[i] = devpriv->out_buffer[chan];
+   up(&devpriv->sem);
 
-   up(&this_usbduxsub->sem);
-   return i;
+   return insn->n;
 }
 
 static int usbdux_ao_insn_write(struct comedi_device *dev,
-- 
1.8.3.2

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


[PATCH 06/30] staging: comedi: usbdux: tidy up usbdux_ao_insn_write()

2013-07-25 Thread H Hartley Sweeten
Rename the local variable used for the private data pointer to the
comedi "norm".

Remove the unnecessary sanity check of the private data pointer. This
function can only be called is the private data was allocated during
the attach.

Tidy up the exit path using goto to ensure that the semaphore is
released.

Return -EBUSY instead of 0 if the (*insn_write) cannot be done because
a command is running.

Tidy up the for() loop that writes the data. The dux_commands[1] and [4]
can be set outside the loop since they are constant. Use a local pointer
for dux_commands[2] to load the value to write. Only the last value needs
to be cached in the private data for read back.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 53 +
 1 file changed, 28 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index 4159936..1e6cb0a 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -1035,39 +1035,42 @@ static int usbdux_ao_insn_read(struct comedi_device 
*dev,
 
 static int usbdux_ao_insn_write(struct comedi_device *dev,
struct comedi_subdevice *s,
-   struct comedi_insn *insn, unsigned int *data)
+   struct comedi_insn *insn,
+   unsigned int *data)
 {
-   int i, err;
-   int chan = CR_CHAN(insn->chanspec);
-   struct usbdux_private *this_usbduxsub = dev->private;
+   struct usbdux_private *devpriv = dev->private;
+   unsigned int chan = CR_CHAN(insn->chanspec);
+   unsigned int val = devpriv->out_buffer[chan];
+   int16_t *p = (int16_t *)&devpriv->dux_commands[2];
+   int ret = -EBUSY;
+   int i;
 
-   if (!this_usbduxsub)
-   return -EFAULT;
+   down(&devpriv->sem);
 
-   down(&this_usbduxsub->sem);
-   if (this_usbduxsub->ao_cmd_running) {
-   up(&this_usbduxsub->sem);
-   return 0;
-   }
+   if (devpriv->ao_cmd_running)
+   goto ao_write_exit;
+
+   /* number of channels: 1 */
+   devpriv->dux_commands[1] = 1;
+   /* channel number */
+   devpriv->dux_commands[4] = chan << 6;
 
for (i = 0; i < insn->n; i++) {
-   /* number of channels: 1 */
-   this_usbduxsub->dux_commands[1] = 1;
+   val = data[i];
+
/* one 16 bit value */
-   *((int16_t *) (this_usbduxsub->dux_commands + 2)) =
-   cpu_to_le16(data[i]);
-   this_usbduxsub->out_buffer[chan] = data[i];
-   /* channel number */
-   this_usbduxsub->dux_commands[4] = (chan << 6);
-   err = send_dux_commands(dev, SENDDACOMMANDS);
-   if (err < 0) {
-   up(&this_usbduxsub->sem);
-   return err;
-   }
+   *p = cpu_to_le16(val);
+
+   ret = send_dux_commands(dev, SENDDACOMMANDS);
+   if (ret < 0)
+   goto ao_write_exit;
}
-   up(&this_usbduxsub->sem);
+   devpriv->out_buffer[chan] = val;
 
-   return i;
+ao_write_exit:
+   up(&devpriv->sem);
+
+   return ret ? ret : insn->n;
 }
 
 static int usbdux_ao_inttrig(struct comedi_device *dev,
-- 
1.8.3.2

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


[PATCH 08/30] staging: comedi: usbdux: tidy up usbdux_ao_cmd()

2013-07-25 Thread H Hartley Sweeten
Rename the local variable used for the private data pointer to the
comedi "norm".

Remove the unnecessary sanity check of the private data pointer. This
function can only be called is the private data was allocated during
the attach.

Make sure an ao command is not already running and return -EBUSY.

Tidy up the exit path using goto to ensure that the semaphore is
released.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 62 ++---
 1 file changed, 33 insertions(+), 29 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index 9aa2e9e..40e1424 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -1188,72 +1188,74 @@ static int usbdux_ao_cmdtest(struct comedi_device *dev,
 
 static int usbdux_ao_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 {
+   struct usbdux_private *devpriv = dev->private;
struct comedi_cmd *cmd = &s->async->cmd;
-   unsigned int chan, gain;
-   int i, ret;
-   struct usbdux_private *this_usbduxsub = dev->private;
+   int ret = -EBUSY;
+   int i;
 
-   if (!this_usbduxsub)
-   return -EFAULT;
+   down(&devpriv->sem);
 
-   down(&this_usbduxsub->sem);
+   if (devpriv->ao_cmd_running)
+   goto ao_cmd_exit;
 
/* set current channel of the running acquisition to zero */
s->async->cur_chan = 0;
+
for (i = 0; i < cmd->chanlist_len; ++i) {
-   chan = CR_CHAN(cmd->chanlist[i]);
-   gain = CR_RANGE(cmd->chanlist[i]);
+   unsigned int chan = CR_CHAN(cmd->chanlist[i]);
+
if (i >= NUMOUTCHANNELS)
break;
-   this_usbduxsub->dac_commands[i] = (chan << 6);
+
+   devpriv->dac_commands[i] = chan << 6;
}
 
/* we count in steps of 1ms (125us) */
/* 125us mode not used yet */
-   if (0) {/* (this_usbduxsub->high_speed) */
+   if (0) {/* (devpriv->high_speed) */
/* 125us */
/* timing of the conversion itself: every 125 us */
-   this_usbduxsub->ao_timer = cmd->convert_arg / 125000;
+   devpriv->ao_timer = cmd->convert_arg / 125000;
} else {
/* 1ms */
/* timing of the scan: we get all channels at once */
-   this_usbduxsub->ao_timer = cmd->scan_begin_arg / 100;
-   if (this_usbduxsub->ao_timer < 1) {
-   up(&this_usbduxsub->sem);
-   return -EINVAL;
+   devpriv->ao_timer = cmd->scan_begin_arg / 100;
+   if (devpriv->ao_timer < 1) {
+   ret = -EINVAL;
+   goto ao_cmd_exit;
}
}
-   this_usbduxsub->ao_counter = this_usbduxsub->ao_timer;
+
+   devpriv->ao_counter = devpriv->ao_timer;
 
if (cmd->stop_src == TRIG_COUNT) {
/* not continuous */
/* counter */
/* high speed also scans everything at once */
-   if (0) {/* (this_usbduxsub->high_speed) */
-   this_usbduxsub->ao_sample_count =
-   (cmd->stop_arg) * (cmd->scan_end_arg);
+   if (0) {/* (devpriv->high_speed) */
+   devpriv->ao_sample_count = cmd->stop_arg *
+  cmd->scan_end_arg;
} else {
/* there's no scan as the scan has been */
/* perf inside the FX2 */
/* data arrives as one packet */
-   this_usbduxsub->ao_sample_count = cmd->stop_arg;
+   devpriv->ao_sample_count = cmd->stop_arg;
}
-   this_usbduxsub->ao_continous = 0;
+   devpriv->ao_continous = 0;
} else {
/* continous acquisition */
-   this_usbduxsub->ao_continous = 1;
-   this_usbduxsub->ao_sample_count = 0;
+   devpriv->ao_continous = 1;
+   devpriv->ao_sample_count = 0;
}
 
if (cmd->start_src == TRIG_NOW) {
/* enable this acquisition operation */
-   this_usbduxsub->ao_cmd_running = 1;
+   devpriv->ao_cmd_running = 1;
ret = usbduxsub_submit_outurbs(dev);
if (ret < 0) {
-   this_usbduxsub->ao_cmd_running = 0;
+   devpriv->ao_cmd_running = 0;
/* fixme: unlink here?? */
-   up(&this_usbduxsub->sem);
-   return ret;
+   goto ao_cmd_exit;
}
s->async->inttrig = NULL;
} else {
@@ -1263,8 

[PATCH 09/30] staging: comedi: usbdux: tidy up usbdux_dio_insn_config()

2013-07-25 Thread H Hartley Sweeten
Tidy up this function a bit.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 24 +++-
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index 40e1424..cdb0287 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -1273,32 +1273,30 @@ ao_cmd_exit:
 
 static int usbdux_dio_insn_config(struct comedi_device *dev,
  struct comedi_subdevice *s,
- struct comedi_insn *insn, unsigned int *data)
+ struct comedi_insn *insn,
+ unsigned int *data)
 {
-   int chan = CR_CHAN(insn->chanspec);
-
-   /* The input or output configuration of each digital line is
-* configured by a special insn_config instruction.  chanspec
-* contains the channel to be changed, and data[0] contains the
-* value COMEDI_INPUT or COMEDI_OUTPUT. */
+   unsigned int mask = 1 << CR_CHAN(insn->chanspec);
 
switch (data[0]) {
case INSN_CONFIG_DIO_OUTPUT:
-   s->io_bits |= 1 << chan;/* 1 means Out */
+   s->io_bits |= mask;
break;
case INSN_CONFIG_DIO_INPUT:
-   s->io_bits &= ~(1 << chan);
+   s->io_bits &= ~mask;
break;
case INSN_CONFIG_DIO_QUERY:
-   data[1] =
-   (s->io_bits & (1 << chan)) ? COMEDI_OUTPUT : COMEDI_INPUT;
+   data[1] = (s->io_bits & mask) ? COMEDI_OUTPUT : COMEDI_INPUT;
break;
default:
return -EINVAL;
break;
}
-   /* we don't tell the firmware here as it would take 8 frames */
-   /* to submit the information. We do it in the insn_bits. */
+
+   /*
+* We don't tell the firmware here as it would take 8 frames
+* to submit the information. We do it in the insn_bits.
+*/
return insn->n;
 }
 
-- 
1.8.3.2

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


[PATCH 07/30] staging: comedi: usbdux: tidy up usbdux_ao_inttrig()

2013-07-25 Thread H Hartley Sweeten
Rename the local variable used for the private data pointer to the
comedi "norm".

Remove the unnecessary sanity check of the private data pointer. This
function can only be called is the private data was allocated during
the attach.

Tidy up the exit path using goto to ensure that the semaphore is
released.

Return -EBUSY instead if an ao command is already running.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 35 +
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index 1e6cb0a..9aa2e9e 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -1074,31 +1074,32 @@ ao_write_exit:
 }
 
 static int usbdux_ao_inttrig(struct comedi_device *dev,
-struct comedi_subdevice *s, unsigned int trignum)
+struct comedi_subdevice *s,
+unsigned int trignum)
 {
-   int ret;
-   struct usbdux_private *this_usbduxsub = dev->private;
+   struct usbdux_private *devpriv = dev->private;
+   int ret = -EINVAL;
 
-   if (!this_usbduxsub)
-   return -EFAULT;
+   down(&devpriv->sem);
 
-   down(&this_usbduxsub->sem);
-   if (trignum != 0) {
-   up(&this_usbduxsub->sem);
-   return -EINVAL;
-   }
-   if (!(this_usbduxsub->ao_cmd_running)) {
-   this_usbduxsub->ao_cmd_running = 1;
+   if (trignum != 0)
+   goto ao_trig_exit;
+
+   if (!devpriv->ao_cmd_running) {
+   devpriv->ao_cmd_running = 1;
ret = usbduxsub_submit_outurbs(dev);
if (ret < 0) {
-   this_usbduxsub->ao_cmd_running = 0;
-   up(&this_usbduxsub->sem);
-   return ret;
+   devpriv->ao_cmd_running = 0;
+   goto ao_trig_exit;
}
s->async->inttrig = NULL;
+   } else {
+   ret = -EBUSY;
}
-   up(&this_usbduxsub->sem);
-   return 1;
+
+ao_trig_exit:
+   up(&devpriv->sem);
+   return ret;
 }
 
 static int usbdux_ao_cmdtest(struct comedi_device *dev,
-- 
1.8.3.2

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


[PATCH 11/30] staging: comedi: usbdux: fix usbdux_counter_read()

2013-07-25 Thread H Hartley Sweeten
Comedi (*insn_read) operations are supposed to read and return insn->n
values. Fix this function to work like the core expects.

Rename the local variable used for the private data pointer to the
comedi "norm".

Remove the unnecessary sanity check of the private data pointer. This
function can only be called is the private data was allocated during
the attach.

Tidy up the exit path using goto to ensure that the semaphore is
released.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 39 -
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index a1e5cf1..05bea98 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -1338,34 +1338,33 @@ dio_exit:
return ret ? ret : insn->n;
 }
 
-/* reads the 4 counters, only two are used just now */
 static int usbdux_counter_read(struct comedi_device *dev,
   struct comedi_subdevice *s,
-  struct comedi_insn *insn, unsigned int *data)
+  struct comedi_insn *insn,
+  unsigned int *data)
 {
-   struct usbdux_private *this_usbduxsub = dev->private;
-   int chan = insn->chanspec;
-   int err;
+   struct usbdux_private *devpriv = dev->private;
+   unsigned int chan = CR_CHAN(insn->chanspec);
+   int ret = 0;
+   int i;
 
-   if (!this_usbduxsub)
-   return -EFAULT;
+   down(&devpriv->sem);
 
-   down(&this_usbduxsub->sem);
-   err = send_dux_commands(dev, READCOUNTERCOMMAND);
-   if (err < 0) {
-   up(&this_usbduxsub->sem);
-   return err;
-   }
+   for (i = 0; i < insn->n; i++) {
+   ret = send_dux_commands(dev, READCOUNTERCOMMAND);
+   if (ret < 0)
+   goto counter_read_exit;
+   ret = receive_dux_commands(dev, READCOUNTERCOMMAND);
+   if (ret < 0)
+   goto counter_read_exit;
 
-   err = receive_dux_commands(dev, READCOUNTERCOMMAND);
-   if (err < 0) {
-   up(&this_usbduxsub->sem);
-   return err;
+   data[i] = le16_to_cpu(devpriv->insn_buffer[chan + 1]);
}
 
-   data[0] = le16_to_cpu(this_usbduxsub->insn_buffer[chan + 1]);
-   up(&this_usbduxsub->sem);
-   return 1;
+counter_read_exit:
+   up(&devpriv->sem);
+
+   return ret ? ret : insn->n;
 }
 
 static int usbdux_counter_write(struct comedi_device *dev,
-- 
1.8.3.2

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


[PATCH 14/30] staging: comedi: usbdux: fix usbdux_pwm_cancel()

2013-07-25 Thread H Hartley Sweeten
Add the missing down/up of the semaphore to prevent other commands
from being issued to the usb device while the pwn is being stopped.

Rename the local variable used for the private data pointer to the
comedi "norm".

Make sure to check that usbdux_pwm_stop() was successful before
sending command to the usb device to stop the pwm.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index 61e9df9..14c26e5 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -1430,17 +1430,25 @@ static int usbdux_pwm_stop(struct comedi_device *dev, 
int do_unlink)
return ret;
 }
 
-/* force unlink - is called by comedi */
 static int usbdux_pwm_cancel(struct comedi_device *dev,
 struct comedi_subdevice *s)
 {
-   struct usbdux_private *this_usbduxsub = dev->private;
-   int res = 0;
+   struct usbdux_private *devpriv = dev->private;
+   int ret;
+
+   down(&devpriv->sem);
 
/* unlink only if it is really running */
-   res = usbdux_pwm_stop(dev, this_usbduxsub->pwm_cmd_running);
+   ret = usbdux_pwm_stop(dev, devpriv->pwm_cmd_running);
+   if (ret)
+   goto pwm_cancel_exit;
+
+   ret = send_dux_commands(dev, SENDPWMOFF);
 
-   return send_dux_commands(dev, SENDPWMOFF);
+pwm_cancel_exit:
+   up(&devpriv->sem);
+
+   return ret;
 }
 
 static void usbduxsub_pwm_irq(struct urb *urb)
-- 
1.8.3.2

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


[PATCH 13/30] staging: comedi: usbdux: tidy up usbdux_pwm_stop()

2013-07-25 Thread H Hartley Sweeten
For aesthetic reasons, pass the comedi_device pointer to this function
instead of the private data pointer. Rename the local variable used
for the private data pointer to the comedi "norm".

Remove the unnecessary sanity check of the private data pointer. This
function can only be called is the private data was allocated during
the attach.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 21 -
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index 82624a2..61e9df9 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -1417,20 +1417,15 @@ static int usbduxsub_unlink_pwm_urbs(struct 
usbdux_private *usbduxsub_tmp)
return err;
 }
 
-/* This cancels a running acquisition operation
- * in any context.
- */
-static int usbdux_pwm_stop(struct usbdux_private *this_usbduxsub, int 
do_unlink)
+static int usbdux_pwm_stop(struct comedi_device *dev, int do_unlink)
 {
+   struct usbdux_private *devpriv = dev->private;
int ret = 0;
 
-   if (!this_usbduxsub)
-   return -EFAULT;
-
if (do_unlink)
-   ret = usbduxsub_unlink_pwm_urbs(this_usbduxsub);
+   ret = usbduxsub_unlink_pwm_urbs(devpriv);
 
-   this_usbduxsub->pwm_cmd_running = 0;
+   devpriv->pwm_cmd_running = 0;
 
return ret;
 }
@@ -1443,7 +1438,7 @@ static int usbdux_pwm_cancel(struct comedi_device *dev,
int res = 0;
 
/* unlink only if it is really running */
-   res = usbdux_pwm_stop(this_usbduxsub, this_usbduxsub->pwm_cmd_running);
+   res = usbdux_pwm_stop(dev, this_usbduxsub->pwm_cmd_running);
 
return send_dux_commands(dev, SENDPWMOFF);
 }
@@ -1468,7 +1463,7 @@ static void usbduxsub_pwm_irq(struct urb *urb)
 * no unlink needed here. Already shutting down.
 */
if (devpriv->pwm_cmd_running)
-   usbdux_pwm_stop(devpriv, 0);
+   usbdux_pwm_stop(dev, 0);
 
return;
 
@@ -1478,7 +1473,7 @@ static void usbduxsub_pwm_irq(struct urb *urb)
dev_err(dev->class_dev,
"Non-zero urb status received in pwm intr 
context: %d\n",
urb->status);
-   usbdux_pwm_stop(devpriv, 0);
+   usbdux_pwm_stop(dev, 0);
}
return;
}
@@ -1501,7 +1496,7 @@ static void usbduxsub_pwm_irq(struct urb *urb)
"buggy USB host controller or bug in 
IRQ handling!\n");
 
/* don't do an unlink here */
-   usbdux_pwm_stop(devpriv, 0);
+   usbdux_pwm_stop(dev, 0);
}
}
 }
-- 
1.8.3.2

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


[PATCH 12/30] staging: comedi: usbdux: fix usbdux_counter_write()

2013-07-25 Thread H Hartley Sweeten
Comedi (*insn_write) operations are supposed to write insn->n values.
Fix this function to work like the core expects.

Rename the local variable used for the private data pointer to the
comedi "norm".

Remove the unnecessary sanity check of the private data pointer. This
function can only be called is the private data was allocated during
the attach.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 31 +--
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index 05bea98..82624a2 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -1369,27 +1369,30 @@ counter_read_exit:
 
 static int usbdux_counter_write(struct comedi_device *dev,
struct comedi_subdevice *s,
-   struct comedi_insn *insn, unsigned int *data)
+   struct comedi_insn *insn,
+   unsigned int *data)
 {
-   struct usbdux_private *this_usbduxsub = dev->private;
-   int err;
+   struct usbdux_private *devpriv = dev->private;
+   unsigned int chan = CR_CHAN(insn->chanspec);
+   int16_t *p = (int16_t *)&devpriv->dux_commands[2];
+   int ret = 0;
+   int i;
 
-   if (!this_usbduxsub)
-   return -EFAULT;
+   down(&devpriv->sem);
 
-   down(&this_usbduxsub->sem);
-   this_usbduxsub->dux_commands[1] = insn->chanspec;
-   *((int16_t *) (this_usbduxsub->dux_commands + 2)) = cpu_to_le16(*data);
+   devpriv->dux_commands[1] = chan;
 
-   err = send_dux_commands(dev, WRITECOUNTERCOMMAND);
-   if (err < 0) {
-   up(&this_usbduxsub->sem);
-   return err;
+   for (i = 0; i < insn->n; i++) {
+   *p = cpu_to_le16(data[i]);
+
+   ret = send_dux_commands(dev, WRITECOUNTERCOMMAND);
+   if (ret < 0)
+   break;
}
 
-   up(&this_usbduxsub->sem);
+   up(&devpriv->sem);
 
-   return 1;
+   return ret ? ret : insn->n;
 }
 
 static int usbdux_counter_config(struct comedi_device *dev,
-- 
1.8.3.2

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


[PATCH 10/30] staging: comedi: usbdux: tidy up usbdux_dio_insn_bits()

2013-07-25 Thread H Hartley Sweeten
Rename the local variable used for the private data pointer to the
comedi "norm".

Remove the unnecessary sanity check of the private data pointer. This
function can only be called is the private data was allocated during
the attach.

Tidy up the exit path using goto to ensure that the semaphore is
released.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 54 -
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index cdb0287..a1e5cf1 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -1302,40 +1302,40 @@ static int usbdux_dio_insn_config(struct comedi_device 
*dev,
 
 static int usbdux_dio_insn_bits(struct comedi_device *dev,
struct comedi_subdevice *s,
-   struct comedi_insn *insn, unsigned int *data)
+   struct comedi_insn *insn,
+   unsigned int *data)
 {
 
-   struct usbdux_private *this_usbduxsub = dev->private;
-   int err;
+   struct usbdux_private *devpriv = dev->private;
+   unsigned int mask = data[0];
+   unsigned int bits = data[1];
+   int ret;
 
-   if (!this_usbduxsub)
-   return -EFAULT;
+   down(&devpriv->sem);
 
-   down(&this_usbduxsub->sem);
+   s->state &= ~mask;
+   s->state |= (bits & mask);
 
-   /* The insn data is a mask in data[0] and the new data
-* in data[1], each channel cooresponding to a bit. */
-   s->state &= ~data[0];
-   s->state |= data[0] & data[1];
-   this_usbduxsub->dux_commands[1] = s->io_bits;
-   this_usbduxsub->dux_commands[2] = s->state;
+   devpriv->dux_commands[1] = s->io_bits;
+   devpriv->dux_commands[2] = s->state;
 
-   /* This command also tells the firmware to return */
-   /* the digital input lines */
-   err = send_dux_commands(dev, SENDDIOBITSCOMMAND);
-   if (err < 0) {
-   up(&this_usbduxsub->sem);
-   return err;
-   }
-   err = receive_dux_commands(dev, SENDDIOBITSCOMMAND);
-   if (err < 0) {
-   up(&this_usbduxsub->sem);
-   return err;
-   }
+   /*
+* This command also tells the firmware to return
+* the digital input lines.
+*/
+   ret = send_dux_commands(dev, SENDDIOBITSCOMMAND);
+   if (ret < 0)
+   goto dio_exit;
+   ret = receive_dux_commands(dev, SENDDIOBITSCOMMAND);
+   if (ret < 0)
+   goto dio_exit;
 
-   data[1] = le16_to_cpu(this_usbduxsub->insn_buffer[1]);
-   up(&this_usbduxsub->sem);
-   return insn->n;
+   data[1] = le16_to_cpu(devpriv->insn_buffer[1]);
+
+dio_exit:
+   up(&devpriv->sem);
+
+   return ret ? ret : insn->n;
 }
 
 /* reads the 4 counters, only two are used just now */
-- 
1.8.3.2

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


[PATCH 16/30] staging: comedi: usbdux: tidy up unlink and stop helpers

2013-07-25 Thread H Hartley Sweeten
For aesthetic reasons, pass the comedi_device pointer to the unlink
helpers instead of the private data pointer.

All the unlink helpers simply call usb_kill_urb() to cancel any pending
transfer requests. The usb passed to usb_kill_urb() can be NULL so the
extra sanity check is not required.

The unlink helpers will always return success so just make them void
functions.

Since the stop helpers will also always return success, make them void
functions as well.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 95 ++---
 1 file changed, 29 insertions(+), 66 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index 0337da1..7077d26 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -249,53 +249,39 @@ struct usbdux_private {
struct semaphore sem;
 };
 
-/*
- * Stops the data acquision
- * It should be safe to call this function from any context
- */
-static int usbduxsub_unlink_inurbs(struct usbdux_private *usbduxsub_tmp)
+static void usbduxsub_unlink_inurbs(struct comedi_device *dev)
 {
-   int i = 0;
-   int err = 0;
+   struct usbdux_private *devpriv = dev->private;
+   int i;
 
-   if (usbduxsub_tmp && usbduxsub_tmp->urb_in) {
-   for (i = 0; i < usbduxsub_tmp->num_in_buffers; i++) {
-   if (usbduxsub_tmp->urb_in[i]) {
-   /* We wait here until all transfers have been
-* cancelled. */
-   usb_kill_urb(usbduxsub_tmp->urb_in[i]);
-   }
-   }
+   if (devpriv->urb_in) {
+   for (i = 0; i < devpriv->num_in_buffers; i++)
+   usb_kill_urb(devpriv->urb_in[i]);
}
-   return err;
 }
 
-static int usbdux_ai_stop(struct comedi_device *dev, int do_unlink)
+static void usbdux_ai_stop(struct comedi_device *dev, int do_unlink)
 {
struct usbdux_private *devpriv = dev->private;
-   int ret = 0;
 
if (do_unlink)
-   ret = usbduxsub_unlink_inurbs(devpriv);
+   usbduxsub_unlink_inurbs(dev);
 
devpriv->ai_cmd_running = 0;
-
-   return ret;
 }
 
 static int usbdux_ai_cancel(struct comedi_device *dev,
struct comedi_subdevice *s)
 {
struct usbdux_private *devpriv = dev->private;
-   int ret = 0;
 
/* prevent other CPUs from submitting new commands just now */
down(&devpriv->sem);
/* unlink only if the urb really has been submitted */
-   ret = usbdux_ai_stop(dev, devpriv->ai_cmd_running);
+   usbdux_ai_stop(dev, devpriv->ai_cmd_running);
up(&devpriv->sem);
 
-   return ret;
+   return 0;
 }
 
 /* analogue IN - interrupt service routine */
@@ -422,46 +408,39 @@ static void usbduxsub_ai_isoc_irq(struct urb *urb)
comedi_event(dev, s);
 }
 
-static int usbduxsub_unlink_outurbs(struct usbdux_private *usbduxsub_tmp)
+static void usbduxsub_unlink_outurbs(struct comedi_device *dev)
 {
-   int i = 0;
-   int err = 0;
+   struct usbdux_private *devpriv = dev->private;
+   int i;
 
-   if (usbduxsub_tmp && usbduxsub_tmp->urb_out) {
-   for (i = 0; i < usbduxsub_tmp->num_out_buffers; i++) {
-   if (usbduxsub_tmp->urb_out[i])
-   usb_kill_urb(usbduxsub_tmp->urb_out[i]);
-   }
+   if (devpriv->urb_out) {
+   for (i = 0; i < devpriv->num_out_buffers; i++)
+   usb_kill_urb(devpriv->urb_out[i]);
}
-   return err;
 }
 
-static int usbdux_ao_stop(struct comedi_device *dev, int do_unlink)
+static void usbdux_ao_stop(struct comedi_device *dev, int do_unlink)
 {
struct usbdux_private *devpriv = dev->private;
-   int ret = 0;
 
if (do_unlink)
-   ret = usbduxsub_unlink_outurbs(devpriv);
+   usbduxsub_unlink_outurbs(dev);
 
devpriv->ao_cmd_running = 0;
-
-   return ret;
 }
 
 static int usbdux_ao_cancel(struct comedi_device *dev,
struct comedi_subdevice *s)
 {
struct usbdux_private *devpriv = dev->private;
-   int ret = 0;
 
/* prevent other CPUs from submitting a command just now */
down(&devpriv->sem);
/* unlink only if it is really running */
-   ret = usbdux_ao_stop(dev, devpriv->ao_cmd_running);
+   usbdux_ao_stop(dev, devpriv->ao_cmd_running);
up(&devpriv->sem);
 
-   return ret;
+   return 0;
 }
 
 static void usbduxsub_ao_isoc_irq(struct urb *urb)
@@ -1403,31 +1382,21 @@ static int usbdux_counter_config(struct comedi_device 
*dev,
return 2;
 }
 
-/***/
-/* PWM */
-
-static int usbduxsub_unlink_pwm_urbs(struct usbdux_private *usbduxsub_tmp)
+static voi

[PATCH 15/30] staging: comedi: usbdux: fix usbdux_pwm_start()

2013-07-25 Thread H Hartley Sweeten
Add the missing down/up of the semaphore to prevent other commands
from being issued to the usb device while the pwn is being stopped.

Rename the local variable used for the private data pointer to the
comedi "norm".

Use memset() to initialize the urb transfer buffer.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 34 -
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index 14c26e5..0337da1 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -1544,34 +1544,34 @@ static int usbdux_pwm_period(struct comedi_device *dev,
return 0;
 }
 
-/* is called from insn so there's no need to do all the sanity checks */
 static int usbdux_pwm_start(struct comedi_device *dev,
struct comedi_subdevice *s)
 {
-   int ret, i;
-   struct usbdux_private *this_usbduxsub = dev->private;
+   struct usbdux_private *devpriv = dev->private;
+   int ret = 0;
 
-   if (this_usbduxsub->pwm_cmd_running) {
-   /* already running */
-   return 0;
-   }
+   down(&devpriv->sem);
+
+   if (devpriv->pwm_cmd_running)
+   goto pwm_start_exit;
 
-   this_usbduxsub->dux_commands[1] = ((int8_t) this_usbduxsub->pwn_delay);
+   devpriv->dux_commands[1] = devpriv->pwn_delay;
ret = send_dux_commands(dev, SENDPWMON);
if (ret < 0)
-   return ret;
+   goto pwm_start_exit;
 
/* initialise the buffer */
-   for (i = 0; i < this_usbduxsub->size_pwm_buf; i++)
-   ((char *)(this_usbduxsub->urb_pwm->transfer_buffer))[i] = 0;
+   memset(devpriv->urb_pwm->transfer_buffer, 0, devpriv->size_pwm_buf);
 
-   this_usbduxsub->pwm_cmd_running = 1;
+   devpriv->pwm_cmd_running = 1;
ret = usbduxsub_submit_pwm_urbs(dev);
-   if (ret < 0) {
-   this_usbduxsub->pwm_cmd_running = 0;
-   return ret;
-   }
-   return 0;
+   if (ret < 0)
+   devpriv->pwm_cmd_running = 0;
+
+pwm_start_exit:
+   up(&devpriv->sem);
+
+   return ret;
 }
 
 /* generates the bit pattern for PWM with the optional sign bit */
-- 
1.8.3.2

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


[PATCH 18/30] staging: comedi: usbdux: remove the usb endpoint defines

2013-07-25 Thread H Hartley Sweeten
The endpoint defines are each only used in one place and don't help
clarify the code. Remove the defines and just open code the values.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 25 +
 1 file changed, 5 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index f606743..644f6ab 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -113,21 +113,6 @@ sampling rate. If you sample two channels you get 4kHz and 
so on.
 /* max lenghth of the transfer-buffer for software upload */
 #define TB_LEN 0x2000
 
-/* Input endpoint number: ISO/IRQ */
-#define ISOINEP   6
-
-/* Output endpoint number: ISO/IRQ */
-#define ISOOUTEP  2
-
-/* This EP sends DUX commands to USBDUX */
-#define COMMAND_OUT_EP 1
-
-/* This EP receives the DUX commands from USBDUX */
-#define COMMAND_IN_EP8
-
-/* Output endpoint for PWM */
-#define PWM_EP 4
-
 /* 300Hz max frequ under PWM */
 #define MIN_PWM_PERIOD  ((long)(1E9/300))
 
@@ -804,7 +789,7 @@ static int send_dux_commands(struct comedi_device *dev, int 
cmd_type)
 
devpriv->dux_commands[0] = cmd_type;
 
-   return usb_bulk_msg(usb, usb_sndbulkpipe(usb, COMMAND_OUT_EP),
+   return usb_bulk_msg(usb, usb_sndbulkpipe(usb, 1),
devpriv->dux_commands, SIZEOFDUXBUFFER,
&nsent, BULK_TIMEOUT);
 }
@@ -818,7 +803,7 @@ static int receive_dux_commands(struct comedi_device *dev, 
int command)
int i;
 
for (i = 0; i < RETRIES; i++) {
-   ret = usb_bulk_msg(usb, usb_rcvbulkpipe(usb, COMMAND_IN_EP),
+   ret = usb_bulk_msg(usb, usb_rcvbulkpipe(usb, 8),
  devpriv->insn_buffer, SIZEINSNBUF,
  &nrec, BULK_TIMEOUT);
if (ret < 0)
@@ -1479,7 +1464,7 @@ static int usbduxsub_submit_pwm_urbs(struct comedi_device 
*dev)
struct urb *urb = devpriv->urb_pwm;
 
/* in case of a resubmission after an unlink... */
-   usb_fill_bulk_urb(urb, usb, usb_sndbulkpipe(usb, PWM_EP),
+   usb_fill_bulk_urb(urb, usb, usb_sndbulkpipe(usb, 4),
  urb->transfer_buffer,
  devpriv->size_pwm_buf,
  usbduxsub_pwm_irq,
@@ -1712,7 +1697,7 @@ static int usbdux_alloc_usb_buffers(struct comedi_device 
*dev)
/* will be filled later with a pointer to the comedi-device */
/* and ONLY then the urb should be submitted */
urb->context = NULL;
-   urb->pipe = usb_rcvisocpipe(usb, ISOINEP);
+   urb->pipe = usb_rcvisocpipe(usb, 6);
urb->transfer_flags = URB_ISO_ASAP;
urb->transfer_buffer = kzalloc(SIZEINBUF, GFP_KERNEL);
if (!urb->transfer_buffer)
@@ -1742,7 +1727,7 @@ static int usbdux_alloc_usb_buffers(struct comedi_device 
*dev)
/* will be filled later with a pointer to the comedi-device */
/* and ONLY then the urb should be submitted */
urb->context = NULL;
-   urb->pipe = usb_sndisocpipe(usb, ISOOUTEP);
+   urb->pipe = usb_sndisocpipe(usb, 2);
urb->transfer_flags = URB_ISO_ASAP;
urb->transfer_buffer = kzalloc(SIZEOUTBUF, GFP_KERNEL);
if (!urb->transfer_buffer)
-- 
1.8.3.2

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


[PATCH 17/30] staging: comedi: usbdux: use the stop helpers in the detach

2013-07-25 Thread H Hartley Sweeten
Use the stop helpers instead of duplicating the code in the detach.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index 7077d26..f606743 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -1941,12 +1941,10 @@ static void usbdux_detach(struct comedi_device *dev)
 
usb_set_intfdata(intf, NULL);
 
-   if (devpriv->pwm_cmd_running)
-   usbduxsub_unlink_pwm_urbs(dev);
-   if (devpriv->ao_cmd_running)
-   usbduxsub_unlink_outurbs(dev);
-   if (devpriv->ai_cmd_running)
-   usbduxsub_unlink_inurbs(dev);
+   /* stop and unlink any submitted urbs */
+   usbdux_pwm_stop(dev, devpriv->pwm_cmd_running);
+   usbdux_ao_stop(dev, devpriv->ao_cmd_running);
+   usbdux_ai_stop(dev, devpriv->ai_cmd_running);
 
usbdux_free_usb_buffers(devpriv);
 
-- 
1.8.3.2

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


[PATCH 20/30] staging: comedi: usbdux: move usbdux_firmware_upload()

2013-07-25 Thread H Hartley Sweeten
For aesthetics, move this function closer to the (*auto_attach).

Also, rename some of the defined constants that are used by the
firmware upload.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 170 +++-
 1 file changed, 82 insertions(+), 88 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index 6991e85..92d740a 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -91,17 +91,16 @@ sampling rate. If you sample two channels you get 4kHz and 
so on.
 
 #include "comedi_fc.h"
 
-/* timeout for the USB-transfer in ms*/
-#define BULK_TIMEOUT 1000
+/* constants for firmware upload and download */
+#define USBDUX_FIRMWARE"usbdux_firmware.bin"
+#define USBDUX_FIRMWARE_MAX_LEN0x2000
+#define USBDUX_FIRMWARE_CMD0xa0
+#define VENDOR_DIR_IN  0xc0
+#define VENDOR_DIR_OUT 0x40
+#define USBDUX_CPU_CS  0xe600
 
-/* constants for "firmware" upload and download */
-#define FIRMWARE "usbdux_firmware.bin"
-#define USBDUXSUB_FIRMWARE 0xA0
-#define VENDOR_DIR_IN  0xC0
-#define VENDOR_DIR_OUT 0x40
-
-/* internal addresses of the 8051 processor */
-#define USBDUXSUB_CPUCS 0xE600
+/* timeout for the USB-transfer in ms */
+#define BULK_TIMEOUT   1000
 
 /* 300Hz max frequ under PWM */
 #define MIN_PWM_PERIOD  ((long)(1E9/300))
@@ -539,80 +538,6 @@ static void usbduxsub_ao_isoc_irq(struct urb *urb)
}
 }
 
-#define FIRMWARE_MAX_LEN 0x2000
-
-static int usbdux_firmware_upload(struct comedi_device *dev,
- const u8 *data, size_t size,
- unsigned long context)
-{
-   struct usb_device *usb = comedi_to_usb_dev(dev);
-   uint8_t *buf;
-   uint8_t *tmp;
-   int ret;
-
-   if (!data)
-   return 0;
-
-   if (size > FIRMWARE_MAX_LEN) {
-   dev_err(dev->class_dev,
-   "usbdux firmware binary it too large for FX2.\n");
-   return -ENOMEM;
-   }
-
-   /* we generate a local buffer for the firmware */
-   buf = kmemdup(data, size, GFP_KERNEL);
-   if (!buf)
-   return -ENOMEM;
-
-   /* we need a malloc'ed buffer for usb_control_msg() */
-   tmp = kmalloc(1, GFP_KERNEL);
-   if (!tmp) {
-   kfree(buf);
-   return -ENOMEM;
-   }
-
-   /* stop the current firmware on the device */
-   *tmp = 1;   /* 7f92 to one */
-   ret = usb_control_msg(usb, usb_sndctrlpipe(usb, 0),
- USBDUXSUB_FIRMWARE,
- VENDOR_DIR_OUT,
- USBDUXSUB_CPUCS, 0x,
- tmp, 1,
- BULK_TIMEOUT);
-   if (ret < 0) {
-   dev_err(dev->class_dev, "can not stop firmware\n");
-   goto done;
-   }
-
-   /* upload the new firmware to the device */
-   ret = usb_control_msg(usb, usb_sndctrlpipe(usb, 0),
- USBDUXSUB_FIRMWARE,
- VENDOR_DIR_OUT,
- 0, 0x,
- buf, size,
- BULK_TIMEOUT);
-   if (ret < 0) {
-   dev_err(dev->class_dev, "firmware upload failed\n");
-   goto done;
-   }
-
-   /* start the new firmware on the device */
-   *tmp = 0;   /* 7f92 to zero */
-   ret = usb_control_msg(usb, usb_sndctrlpipe(usb, 0),
- USBDUXSUB_FIRMWARE,
- VENDOR_DIR_OUT,
- USBDUXSUB_CPUCS, 0x,
- tmp, 1,
- BULK_TIMEOUT);
-   if (ret < 0)
-   dev_err(dev->class_dev, "can not start firmware\n");
-
-done:
-   kfree(tmp);
-   kfree(buf);
-   return ret;
-}
-
 static int usbduxsub_submit_inurbs(struct comedi_device *dev)
 {
struct usb_device *usb = comedi_to_usb_dev(dev);
@@ -1635,8 +1560,77 @@ static int usbdux_pwm_config(struct comedi_device *dev,
return -EINVAL;
 }
 
-/* end of PWM */
-/*/
+static int usbdux_firmware_upload(struct comedi_device *dev,
+ const u8 *data, size_t size,
+ unsigned long context)
+{
+   struct usb_device *usb = comedi_to_usb_dev(dev);
+   uint8_t *buf;
+   uint8_t *tmp;
+   int ret;
+
+   if (!data)
+   return 0;
+
+   if (size > USBDUX_FIRMWARE_MAX_LEN) {
+   dev_err(dev->class_dev,
+   "usbdux firmware binary it too large for FX2.\n");
+   return -ENOMEM;
+   }
+
+   /* we generate a local buffer for the firmware */
+   buf = 

[PATCH 22/30] staging: comedi: usbdux: rename private data variables

2013-07-25 Thread H Hartley Sweeten
The usbdux and usbduxsigma drivers are _very_ similar. For aesthetic
reasons, rename the private data variables in this driver to match
the names in the usbduxsigma driver so we can start sharing the
common code.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 136 
 1 file changed, 68 insertions(+), 68 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index 20b705e..bd46bec 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -177,24 +177,24 @@ static const struct comedi_lrange range_usbdux_ao_range = 
{
 
 struct usbdux_private {
/* actual number of in-buffers */
-   int num_in_buffers;
+   int n_ai_urbs;
/* actual number of out-buffers */
-   int num_out_buffers;
+   int n_ao_urbs;
/* ISO-transfer handling: buffers */
-   struct urb **urb_in;
-   struct urb **urb_out;
+   struct urb **ai_urbs;
+   struct urb **ao_urbs;
/* pwm-transfer handling */
-   struct urb *urb_pwm;
+   struct urb *pwm_urb;
/* PWM period */
unsigned int pwm_period;
/* PWM internal delay for the GPIF in the FX2 */
-   int8_t pwn_delay;
+   int8_t pwm_delay;
/* size of the PWM buffer which holds the bit pattern */
-   int size_pwm_buf;
+   int pwm_buf_sz;
/* input buffer for the ISO-transfer */
-   int16_t *in_buffer;
+   int16_t *in_buf;
/* input buffer for single insn */
-   int16_t *insn_buffer;
+   int16_t *insn_buf;
/* output buffer for single DA outputs */
int16_t *out_buffer;
 
@@ -228,9 +228,9 @@ static void usbduxsub_unlink_inurbs(struct comedi_device 
*dev)
struct usbdux_private *devpriv = dev->private;
int i;
 
-   if (devpriv->urb_in) {
-   for (i = 0; i < devpriv->num_in_buffers; i++)
-   usb_kill_urb(devpriv->urb_in[i]);
+   if (devpriv->ai_urbs) {
+   for (i = 0; i < devpriv->n_ai_urbs; i++)
+   usb_kill_urb(devpriv->ai_urbs[i]);
}
 }
 
@@ -270,7 +270,7 @@ static void usbduxsub_ai_isoc_irq(struct urb *urb)
switch (urb->status) {
case 0:
/* copy the result in the transfer buffer */
-   memcpy(devpriv->in_buffer, urb->transfer_buffer, SIZEINBUF);
+   memcpy(devpriv->in_buf, urb->transfer_buffer, SIZEINBUF);
break;
case -EILSEQ:
/* error in the ISOchronous data */
@@ -364,7 +364,7 @@ static void usbduxsub_ai_isoc_irq(struct urb *urb)
n = s->async->cmd.chanlist_len;
for (i = 0; i < n; i++) {
unsigned int range = CR_RANGE(s->async->cmd.chanlist[i]);
-   int16_t val = le16_to_cpu(devpriv->in_buffer[i]);
+   int16_t val = le16_to_cpu(devpriv->in_buf[i]);
 
/* bipolar data is two's-complement */
if (comedi_range_is_bipolar(s, range))
@@ -388,9 +388,9 @@ static void usbduxsub_unlink_outurbs(struct comedi_device 
*dev)
struct usbdux_private *devpriv = dev->private;
int i;
 
-   if (devpriv->urb_out) {
-   for (i = 0; i < devpriv->num_out_buffers; i++)
-   usb_kill_urb(devpriv->urb_out[i]);
+   if (devpriv->ao_urbs) {
+   for (i = 0; i < devpriv->n_ao_urbs; i++)
+   usb_kill_urb(devpriv->ao_urbs[i]);
}
 }
 
@@ -548,8 +548,8 @@ static int usbduxsub_submit_inurbs(struct comedi_device 
*dev)
int i;
 
/* Submit all URBs and start the transfer on the bus */
-   for (i = 0; i < devpriv->num_in_buffers; i++) {
-   urb = devpriv->urb_in[i];
+   for (i = 0; i < devpriv->n_ai_urbs; i++) {
+   urb = devpriv->ai_urbs[i];
 
/* in case of a resubmission after an unlink... */
urb->interval = devpriv->ai_interval;
@@ -573,8 +573,8 @@ static int usbduxsub_submit_outurbs(struct comedi_device 
*dev)
int ret;
int i;
 
-   for (i = 0; i < devpriv->num_out_buffers; i++) {
-   urb = devpriv->urb_out[i];
+   for (i = 0; i < devpriv->n_ao_urbs; i++) {
+   urb = devpriv->ao_urbs[i];
 
/* in case of a resubmission after an unlink... */
urb->context = dev;
@@ -720,11 +720,11 @@ static int receive_dux_commands(struct comedi_device 
*dev, int command)
 
for (i = 0; i < RETRIES; i++) {
ret = usb_bulk_msg(usb, usb_rcvbulkpipe(usb, 8),
- devpriv->insn_buffer, SIZEINSNBUF,
+ devpriv->insn_buf, SIZEINSNBUF,
  &nrec, BULK_TIMEOUT);
if (ret < 0)
return ret;
-   if (le16_to_cpu(devpriv-

[PATCH 19/30] staging: comedi: usbdux: remove some unused defines

2013-07-25 Thread H Hartley Sweeten
These defines are not used by the driver. Remove them.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 10 --
 1 file changed, 10 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index 644f6ab..6991e85 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -103,16 +103,6 @@ sampling rate. If you sample two channels you get 4kHz and 
so on.
 /* internal addresses of the 8051 processor */
 #define USBDUXSUB_CPUCS 0xE600
 
-/*
- * the minor device number, major is 180 only for debugging purposes and to
- * upload special firmware (programming the eeprom etc) which is not compatible
- * with the comedi framwork
- */
-#define USBDUXSUB_MINOR 32
-
-/* max lenghth of the transfer-buffer for software upload */
-#define TB_LEN 0x2000
-
 /* 300Hz max frequ under PWM */
 #define MIN_PWM_PERIOD  ((long)(1E9/300))
 
-- 
1.8.3.2

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


[PATCH 21/30] staging: comedi: usbdux: clarify bipolar ai data in usbduxsub_ai_isoc_irq()

2013-07-25 Thread H Hartley Sweeten
Use the comedi_range_is_bipolar() helper instead of checking the
'range' index against a magic number.

Also, use the s->maxdata to calculate the value needed to munge the
value for bipolar data instead of the magic number.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index 92d740a..20b705e 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -363,14 +363,15 @@ static void usbduxsub_ai_isoc_irq(struct urb *urb)
/* get the data from the USB bus and hand it over to comedi */
n = s->async->cmd.chanlist_len;
for (i = 0; i < n; i++) {
+   unsigned int range = CR_RANGE(s->async->cmd.chanlist[i]);
+   int16_t val = le16_to_cpu(devpriv->in_buffer[i]);
+
+   /* bipolar data is two's-complement */
+   if (comedi_range_is_bipolar(s, range))
+   val ^= ((s->maxdata + 1) >> 1);
+
/* transfer data */
-   if (CR_RANGE(s->async->cmd.chanlist[i]) <= 1) {
-   err = comedi_buf_put(s->async,
-le16_to_cpu(devpriv->in_buffer[i]) ^ 0x800);
-   } else {
-   err = comedi_buf_put(s->async,
-le16_to_cpu(devpriv->in_buffer[i]));
-   }
+   err = comedi_buf_put(s->async, val);
if (unlikely(err == 0)) {
/* buffer overflow */
usbdux_ai_stop(dev, 0);
-- 
1.8.3.2

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


[PATCH 23/30] staging: comedi: usbdux: cleanup the private data 'outBuffer'

2013-07-25 Thread H Hartley Sweeten
This buffer is used to cache the values that are written to the
analog output channels. Currently it only caches the single writes
to the channels using the (*insn_write) callback. The async command
writes are not cached. The buffer is also being kzalloc'ed during
the attach of the driver to a size much larger that required.

Rename the CamelCase buffer and change it to an array in the private
data of the correct size to cache the analog output channel values.

Modify the analog output urb callback so it updates the cached values
with those used for the asynchronous command to allow readback after
the command completes.

The sanity check of the index to dac_commands[] (i.e. the 'chan' being
written) is not needed. The chanlist_len will always be less than the
number of channels.

Also, fix the dev_err() message so it uses the proper device pointer.

Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/staging/comedi/drivers/usbdux.c | 49 +++--
 1 file changed, 22 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c 
b/drivers/staging/comedi/drivers/usbdux.c
index bd46bec..1399305 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -99,6 +99,8 @@ sampling rate. If you sample two channels you get 4kHz and so 
on.
 #define VENDOR_DIR_OUT 0x40
 #define USBDUX_CPU_CS  0xe600
 
+#define USBDUX_NUM_AO_CHAN 4
+
 /* timeout for the USB-transfer in ms */
 #define BULK_TIMEOUT   1000
 
@@ -195,8 +197,8 @@ struct usbdux_private {
int16_t *in_buf;
/* input buffer for single insn */
int16_t *insn_buf;
-   /* output buffer for single DA outputs */
-   int16_t *out_buffer;
+
+   unsigned int ao_readback[USBDUX_NUM_AO_CHAN];
 
unsigned int high_speed:1;
unsigned int ai_cmd_running:1;
@@ -486,25 +488,24 @@ static void usbduxsub_ao_isoc_irq(struct urb *urb)
((uint8_t *) (urb->transfer_buffer))[0] =
s->async->cmd.chanlist_len;
for (i = 0; i < s->async->cmd.chanlist_len; i++) {
-   short temp;
-   if (i >= NUMOUTCHANNELS)
-   break;
+   unsigned int chan = devpriv->dac_commands[i];
+   short val;
 
+   ret = comedi_buf_get(s->async, &val);
+   if (ret < 0) {
+   dev_err(dev->class_dev, "buffer underflow\n");
+   s->async->events |= (COMEDI_CB_EOA |
+COMEDI_CB_OVERFLOW);
+   }
/* pointer to the DA */
datap =
(&(((int8_t *) urb->transfer_buffer)[i * 3 + 1]));
/* get the data from comedi */
-   ret = comedi_buf_get(s->async, &temp);
-   datap[0] = temp;
-   datap[1] = temp >> 8;
-   datap[2] = devpriv->dac_commands[i];
-   if (ret < 0) {
-   dev_err(&urb->dev->dev,
-   "comedi: buffer underflow\n");
-   s->async->events |= COMEDI_CB_EOA;
-   s->async->events |= COMEDI_CB_OVERFLOW;
-   }
-   /* transmit data to comedi */
+   datap[0] = val;
+   datap[1] = val >> 8;
+   datap[2] = chan;
+   devpriv->ao_readback[chan] = val;
+
s->async->events |= COMEDI_CB_BLOCK;
comedi_event(dev, s);
}
@@ -907,7 +908,7 @@ static int usbdux_ao_insn_read(struct comedi_device *dev,
 
down(&devpriv->sem);
for (i = 0; i < insn->n; i++)
-   data[i] = devpriv->out_buffer[chan];
+   data[i] = devpriv->ao_readback[chan];
up(&devpriv->sem);
 
return insn->n;
@@ -920,7 +921,7 @@ static int usbdux_ao_insn_write(struct comedi_device *dev,
 {
struct usbdux_private *devpriv = dev->private;
unsigned int chan = CR_CHAN(insn->chanspec);
-   unsigned int val = devpriv->out_buffer[chan];
+   unsigned int val = devpriv->ao_readback[chan];
int16_t *p = (int16_t *)&devpriv->dux_commands[2];
int ret = -EBUSY;
int i;
@@ -945,7 +946,7 @@ static int usbdux_ao_insn_write(struct comedi_device *dev,
if (ret < 0)
goto ao_write_exit;
}
-   devpriv->out_buffer[chan] = val;
+   devpriv->ao_readback[chan] = val;
 
 ao_write_exit:
up(&devpriv->sem);
@@ -1660,11 +1661,6 @@ static int usbdux_alloc_usb_buffers(struct comedi_device 
*dev)
if (!devpriv->insn_buf)
  

  1   2   >