Re: [PATCH v5 1/3] mfd: Add realtek USB card reader driver
On Fri, 2014-03-28 at 11:33 +0800, Roger wrote: On 03/26/2014 10:36 PM, Oliver Neukum wrote: On Tue, 2014-03-25 at 18:44 +0800, rogera...@realtek.com wrote: From: Roger Tseng rogera...@realtek.com + if (ret) + goto out_init_fail; + + /* initialize USB SG transfer timer */ + init_timer(ucr-sg_timer); + setup_timer(ucr-sg_timer, rtsx_usb_sg_timed_out, (unsigned long) ucr); +#ifdef CONFIG_PM + intf-needs_remote_wakeup = 1; Why? Our reader supports remote wake-up from card slot event(insertion, removal). It should be enabled to let the driver be able to detect the newly inserted card. Interesting. This capability has been lacking for a long time. That is cool hardware. Are you sure the upper layers implement the event infrastructure so that devices are not polled? The LED is not in a permanent cut-off state after here. It is called to guarantee the LED is off during suspend to save more power. It could be lit up in the card host drivers(e.g. rtsx_usb_sdmmc.c) anytime whenever necessary. This has been asked may times. I'm considering putting some comment in next revision. Good idea. Regards Oliver ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[patch] Staging: rtl8192e: array overflow in set_swcam()
EntryNo comes from the user in rtl8192_ioctl(). We should limit it
to 31 to prevent memory corruption.
Also we may as well return on invalid data in setKey() as well.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
Static analysis. I'm not certain this fix is enough to solve the whole
problem. Please review.
diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_cam.c
b/drivers/staging/rtl8192e/rtl8192e/rtl_cam.c
index c46c65c..a6ca8982 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_cam.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_cam.c
@@ -87,6 +87,10 @@ void set_swcam(struct net_device *dev, u8 EntryNo, u8
KeyIndex, u16 KeyType,
RT_TRACE(COMP_DBG, ===%s():EntryNo is %d,KeyIndex is
%d,KeyType is %d,is_mesh is %d\n, __func__, EntryNo,
KeyIndex, KeyType, is_mesh);
+
+ if (EntryNo = TOTAL_CAM_ENTRY)
+ return;
+
if (!is_mesh) {
ieee-swcamtable[EntryNo].bused = true;
ieee-swcamtable[EntryNo].key_index = KeyIndex;
@@ -121,8 +125,10 @@ void setKey(struct net_device *dev, u8 EntryNo, u8
KeyIndex, u16 KeyType,
}
}
priv-rtllib-is_set_key = true;
- if (EntryNo = TOTAL_CAM_ENTRY)
+ if (EntryNo = TOTAL_CAM_ENTRY) {
RT_TRACE(COMP_ERR, cam entry exceeds in setKey()\n);
+ return;
+ }
RT_TRACE(COMP_SEC, to setKey(), dev:%p, EntryNo:%d, KeyIndex:%d,
KeyType:%d, MacAddr %pM\n, dev, EntryNo, KeyIndex,
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 1/3] mfd: Add realtek USB card reader driver
On 03/28/2014 04:31 PM, Oliver Neukum wrote: On Fri, 2014-03-28 at 11:33 +0800, Roger wrote: On 03/26/2014 10:36 PM, Oliver Neukum wrote: On Tue, 2014-03-25 at 18:44 +0800, rogera...@realtek.com wrote: From: Roger Tseng rogera...@realtek.com + if (ret) + goto out_init_fail; + + /* initialize USB SG transfer timer */ + init_timer(ucr-sg_timer); + setup_timer(ucr-sg_timer, rtsx_usb_sg_timed_out, (unsigned long) ucr); +#ifdef CONFIG_PM + intf-needs_remote_wakeup = 1; Why? Our reader supports remote wake-up from card slot event(insertion, removal). It should be enabled to let the driver be able to detect the newly inserted card. Interesting. This capability has been lacking for a long time. That is cool hardware. Are you sure the upper layers implement the event infrastructure so that devices are not polled? Polling is still necessary because USB essentially doesn't support interrupt. However, if remote wake-up is not enabled or supported, the device will have to be resumed every time when polling and then suspended right away if there isn't a new card. This should consume more power than the designed behavior: suspending device while card removed, taking failed polls as no-card, and card being successfully polled after insertion-triggered remote wake-up. The LED is not in a permanent cut-off state after here. It is called to guarantee the LED is off during suspend to save more power. It could be lit up in the card host drivers(e.g. rtsx_usb_sdmmc.c) anytime whenever necessary. This has been asked may times. I'm considering putting some comment in next revision. Good idea. Regards Oliver Best regards, Roger Tseng ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Fix coding style issue in xlr_net.c
checkpatch script returns the following warning:
WARNING: line over 80 characters
310: FILE: drivers/staging/netlogic/xlr_net.c:310:
+ void *accel_priv, select_queue_fallback_t fallback)
This patch fixes the coding style issue.
Signed-off-by: Neil Armstrong superna9...@gmail.com
---
drivers/staging/netlogic/xlr_net.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/staging/netlogic/xlr_net.c
b/drivers/staging/netlogic/xlr_net.c
index 31b269a..cef0b8a 100644
--- a/drivers/staging/netlogic/xlr_net.c
+++ b/drivers/staging/netlogic/xlr_net.c
@@ -307,7 +307,8 @@ static netdev_tx_t xlr_net_start_xmit(struct sk_buff
*skb,
}
static u16 xlr_net_select_queue(struct net_device *ndev, struct sk_buff
*skb,
- void *accel_priv, select_queue_fallback_t
fallback)
+ struct sk_buff *skb, void *accel_priv,
+ select_queue_fallback_t fallback)
{
return (u16)smp_processor_id();
}
--
1.7.0.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Fix coding style issue in xlr_net.c
On Fri, Mar 28, 2014 at 10:43:43AM +0100, Neil Armstrong wrote: checkpatch script returns the following warning: WARNING: line over 80 characters 310: FILE: drivers/staging/netlogic/xlr_net.c:310: + void *accel_priv, select_queue_fallback_t fallback) This patch fixes the coding style issue. Signed-off-by: Neil Armstrong superna9...@gmail.com Patch is corrupted by mail client read Documentation/email-clients.txt or `use git send-email`. Also the patch looks wrong. Did you try compiling it? regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Fix coding style issue in xlr_net.c
The subject should be: [PATCH] Staging: netlogic: long lines in xlr_net.c You were missing the subsystem prefix: Staging: netlogic:. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] staging/ozwpan: coding style ether_addr_copy
On Fri, Mar 14, 2014 at 12:39:11AM +0900, Jérôme Pinot wrote:
On 03/13/14 02:28, Greg Kroah-Hartman wrote:
On Thu, Mar 13, 2014 at 10:21:44AM +0900, Jérôme Pinot wrote:
[...]
diff --git a/drivers/staging/ozwpan/ozcdev.c
b/drivers/staging/ozwpan/ozcdev.c
index 5de5981..10c0a96 100644
--- a/drivers/staging/ozwpan/ozcdev.c
+++ b/drivers/staging/ozwpan/ozcdev.c
@@ -217,7 +217,7 @@ static int oz_set_active_pd(const u8 *addr)
pd = oz_pd_find(addr);
if (pd) {
spin_lock_bh(g_cdev.lock);
- memcpy(g_cdev.active_addr, addr, ETH_ALEN);
+ ether_addr_copy(g_cdev.active_addr, addr);
Are you sure this will work?
No. But the ozwpan driver uses already ether_addr_equal which is not
alignment-safe. As
https://www.kernel.org/doc/Documentation/unaligned-memory-access.txt
said:
This alignment-unsafe function is still useful as it is a decent
optimization for the cases when you can ensure alignment, which is
true almost all of the time in ethernet networking context.
I expected the maintainer to confirm/infirm this. I'm just seeing that's
actually Chris Kelly who did write this part of code, so I'm CC'ing him
too.
It is aligned ok, but don't rely on the maintainer to fix your bugs.
Don't send patches which you are not sure about.
Joe, this seems like a very bad warning message from checkpatch.pl
because people will constantly send us patches over and over which
introduce bugs and they rely on the maintainer to catch it every time.
Can we get rid of the warning or move it under --strict or something?
Do we have a mailing list yet for checkpatch issues?
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 03/03] staging: dgap: remove more unneeded brd-state states
These patches are fine and they were applied already.
On Wed, Mar 12, 2014 at 12:50:55PM -0400, Mark Hounschell wrote:
@@ -4368,15 +4364,16 @@ static void dgap_do_bios_load(struct board_t *brd,
uchar __user *ubios, int len)
/*
* Checks to see if the BIOS completed running on the card.
*/
-static void dgap_do_wait_for_bios(struct board_t *brd)
+static int dgap_do_wait_for_bios(struct board_t *brd)
I wish this funciton returned negative error codes on error. It is
poorly named for a boolean function.
{
uchar *addr;
u16 word;
u16 err1;
u16 err2;
+ int ret = 0;
The ret variable is not needed. Replace it with zero literal for better
readability.
@@ -4455,15 +4452,16 @@ static void dgap_do_fep_load(struct board_t *brd,
uchar *ufep, int len)
/*
* Waits for the FEP to report thats its ready for us to use.
*/
-static void dgap_do_wait_for_fep(struct board_t *brd)
+static int dgap_do_wait_for_fep(struct board_t *brd)
Same as dgap_do_wait_for_bios().
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: lustre: fld: moved EXPORT_SYMBOL() for a fix.
On Thu, Mar 13, 2014 at 12:30:48AM -0400, Gary Rookard wrote: I moved EXPORT_SYMBOL(fld_client_proc_fini); for a EXPORT_SYMBOL should be immediately below its function warning fix. Checkpatch.pl is wrong here and your patch breaks the build if LPROCFS is defined. (Probably it breaks the build. I didn't test. Lustre build is already broken for me.) regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] Staging: unisys: channels: Cleanup sparse warnings
On Thu, Mar 13, 2014 at 03:39:18PM -0500, Ken Cox wrote:
Clean up multiple sparse warnings mostly due to different address spaces
when accessing I/O memory.
The missing calls to readl() are bug fixes, not cleanups. (Cleanups
mean it doesn't change how the code works).
-visor_signal_insert(pCHANNEL_HEADER pChannel, U32 Queue, void *pSignal)
+visor_signal_insert(CHANNEL_HEADER __iomem *pChannel, U32 Queue, void
*pSignal)
{
- void *psignal;
- unsigned int head, tail;
- pSIGNAL_QUEUE_HEADER pqhdr =
- (pSIGNAL_QUEUE_HEADER) ((char *) pChannel +
- pChannel-oChannelSpace) + Queue;
+ void __iomem *psignal;
+ unsigned int head, tail, nof;
+
Don't put a blank line in the middle of a declaration block.
+ SIGNAL_QUEUE_HEADER __iomem *pqhdr =
+ (SIGNAL_QUEUE_HEADER __iomem *)
+ ((char __iomem *) pChannel + readq(pChannel-oChannelSpace))
+ + Queue;
Really this assignement is too complicated for the declaration block but
that was in the original code and not something introduced by this
patch.
For future reference, this patch should have been broken up into
separate patches:
[1/3] add readl/writel for iomem access
(don't mix bugfixes and unrelated cleanups).
[2/3] delete uncalled functions
[3/3] other sparse warnings
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/6] Staging: unisys: visorchannel: Clean up sparse warnings in visorchannel code
On Thu, Mar 13, 2014 at 03:39:21PM -0500, Ken Cox wrote:
Clean up code to get rid of sparse warnings, mostly due to accessing I/O
space.
Remove visorchannel_get_safe_queue(), visorchannel_safesignalremove(),
and visorchannel_safesignalinsert() since they were not called from anywhere.
Signed-off-by: Ken Cox j...@redhat.com
---
drivers/staging/unisys/visorchannel/visorchannel.h | 2 +-
.../unisys/visorchannel/visorchannel_funcs.c | 136
-
2 files changed, 1 insertion(+), 137 deletions(-)
diff --git a/drivers/staging/unisys/visorchannel/visorchannel.h
b/drivers/staging/unisys/visorchannel/visorchannel.h
index 4546686..62d29a2 100644
--- a/drivers/staging/unisys/visorchannel/visorchannel.h
+++ b/drivers/staging/unisys/visorchannel/visorchannel.h
@@ -92,7 +92,7 @@ void *visorchannel_get_header(VISORCHANNEL *channel);
do {\
U8 *p = (U8 *)visorchannel_get_header(chan);\
if (p) {\
- ULTRA_CHANNEL_CLIENT_TRANSITION(p, chanId, CliStateOS, \
+ ULTRA_CHANNEL_CLIENT_TRANSITION(p, chanId, \
newstate, logCtx); \
Gar... You broke the build in [PATCH 2/6] and you're fixing it up here.
That's not allowed.
If this patchset hasn't already been applied, then please redo it.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 03/03] staging: dgap: remove more unneeded brd-state states
On 03/28/2014 07:34 AM, Dan Carpenter wrote:
These patches are fine and they were applied already.
On Wed, Mar 12, 2014 at 12:50:55PM -0400, Mark Hounschell wrote:
@@ -4368,15 +4364,16 @@ static void dgap_do_bios_load(struct board_t *brd,
uchar __user *ubios, int len)
/*
* Checks to see if the BIOS completed running on the card.
*/
-static void dgap_do_wait_for_bios(struct board_t *brd)
+static int dgap_do_wait_for_bios(struct board_t *brd)
I wish this funciton returned negative error codes on error. It is
poorly named for a boolean function.
{
uchar *addr;
u16 word;
u16 err1;
u16 err2;
+int ret = 0;
The ret variable is not needed. Replace it with zero literal for better
readability.
@@ -4455,15 +4452,16 @@ static void dgap_do_fep_load(struct board_t *brd,
uchar *ufep, int len)
/*
* Waits for the FEP to report thats its ready for us to use.
*/
-static void dgap_do_wait_for_fep(struct board_t *brd)
+static int dgap_do_wait_for_fep(struct board_t *brd)
Same as dgap_do_wait_for_bios().
Yes, they were not originally boolean functions. Would names like
dgap_test_bios and dgap_test_fep be better names? And returns of
-EIO if they fail and 0 if good?
Sample patch:
diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index 4bbedae..d0e486b 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -194,8 +194,8 @@ static int dgap_finalize_board_init(struct board_t *brd);
static void dgap_get_vpd(struct board_t *brd);
static void dgap_do_reset_board(struct board_t *brd);
-static int dgap_do_wait_for_bios(struct board_t *brd);
-static int dgap_do_wait_for_fep(struct board_t *brd);
+static int dgap_test_bios(struct board_t *brd);
+static int dgap_test_fep(struct board_t *brd);
static int dgap_tty_register_ports(struct board_t *brd);
static int dgap_firmware_load(struct pci_dev *pdev, int card_type);
@@ -890,8 +890,9 @@ static int dgap_firmware_load(struct pci_dev *pdev, int
card_type)
release_firmware(fw);
/* Wait for BIOS to test board... */
- if (!dgap_do_wait_for_bios(brd))
- return -ENXIO;
+ ret = dgap_test_bios(brd)
+ if (ret)
+ return ret;
}
if (fw_info[card_type].fep_name) {
@@ -906,8 +907,9 @@ static int dgap_firmware_load(struct pci_dev *pdev, int
card_type)
release_firmware(fw);
/* Wait for FEP to load on board... */
- if (!dgap_do_wait_for_fep(brd))
- return -ENXIO;
+ ret = dgap_test_fep(brd)
+ if (ret)
+ return ret;
}
#ifdef DIGI_CONCENTRATORS_SUPPORTED
@@ -4332,13 +4334,12 @@ static void dgap_do_bios_load(struct board_t *brd,
const uchar *ubios, int len)
/*
* Checks to see if the BIOS completed running on the card.
*/
-static int dgap_do_wait_for_bios(struct board_t *brd)
+static int dgap_test_bios(struct board_t *brd)
{
uchar *addr;
u16 word;
u16 err1;
u16 err2;
- int ret = 0;
if (!brd || (brd-magic != DGAP_BOARD_MAGIC) || !brd-re_map_membase)
return ret;
@@ -4355,7 +4356,7 @@ static int dgap_do_wait_for_bios(struct board_t *brd)
while (brd-wait_for_bios 1000) {
/* Check to see if BIOS thinks board is good. (GD). */
if (word == *(u16 *) GD)
- return 1;
+ return 0;
msleep_interruptible(10);
brd-wait_for_bios++;
word = readw(addr + POSTAREA);
@@ -4369,7 +4370,7 @@ static int dgap_do_wait_for_bios(struct board_t *brd)
brd-state = BOARD_FAILED;
brd-dpastatus = BD_NOBIOS;
- return ret;
+ return -EIO;
}
/*
@@ -4420,13 +4421,12 @@ static void dgap_do_fep_load(struct board_t *brd, const
uchar *ufep, int len)
/*
* Waits for the FEP to report thats its ready for us to use.
*/
-static int dgap_do_wait_for_fep(struct board_t *brd)
+static int dgap_test_fep(struct board_t *brd)
{
uchar *addr;
u16 word;
u16 err1;
u16 err2;
- int ret = 0;
if (!brd || (brd-magic != DGAP_BOARD_MAGIC) || !brd-re_map_membase)
return ret;
@@ -4449,7 +4449,7 @@ static int dgap_do_wait_for_fep(struct board_t *brd)
if (word == *(u16 *) 5A)
brd-bd_flags |= BD_FEP5PLUS;
- return 1;
+ return 0;
}
msleep_interruptible(10);
brd-wait_for_fep++;
@@ -4464,7 +4464,7 @@ static int dgap_do_wait_for_fep(struct board_t *brd)
brd-state = BOARD_FAILED;
brd-dpastatus = BD_NOFEP;
- return ret;
+ return -EIO;
}
/*
Thanks
Mark
___
Re: [PATCH 03/03] staging: dgap: remove more unneeded brd-state states
On 03/28/2014 09:08 AM, Mark Hounschell wrote:
On 03/28/2014 07:34 AM, Dan Carpenter wrote:
These patches are fine and they were applied already.
On Wed, Mar 12, 2014 at 12:50:55PM -0400, Mark Hounschell wrote:
@@ -4368,15 +4364,16 @@ static void dgap_do_bios_load(struct board_t *brd,
uchar __user *ubios, int len)
/*
* Checks to see if the BIOS completed running on the card.
*/
-static void dgap_do_wait_for_bios(struct board_t *brd)
+static int dgap_do_wait_for_bios(struct board_t *brd)
I wish this funciton returned negative error codes on error. It is
poorly named for a boolean function.
{
uchar *addr;
u16 word;
u16 err1;
u16 err2;
+ int ret = 0;
The ret variable is not needed. Replace it with zero literal for better
readability.
@@ -4455,15 +4452,16 @@ static void dgap_do_fep_load(struct board_t *brd,
uchar *ufep, int len)
/*
* Waits for the FEP to report thats its ready for us to use.
*/
-static void dgap_do_wait_for_fep(struct board_t *brd)
+static int dgap_do_wait_for_fep(struct board_t *brd)
Same as dgap_do_wait_for_bios().
Yes, they were not originally boolean functions. Would names like
dgap_test_bios and dgap_test_fep be better names? And returns of
-EIO if they fail and 0 if good?
I'll just post a new patch for review and fix as required.
Mark
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] staging/ozwpan: coding style ether_addr_copy
(adding Andrew Morton, David Miller and LKML to cc's)
On Fri, 2014-03-28 at 14:18 +0300, Dan Carpenter wrote:
On Fri, Mar 14, 2014 at 12:39:11AM +0900, Jérôme Pinot wrote:
On 03/13/14 02:28, Greg Kroah-Hartman wrote:
On Thu, Mar 13, 2014 at 10:21:44AM +0900, Jérôme Pinot wrote:
[...]
diff --git a/drivers/staging/ozwpan/ozcdev.c
b/drivers/staging/ozwpan/ozcdev.c
index 5de5981..10c0a96 100644
--- a/drivers/staging/ozwpan/ozcdev.c
+++ b/drivers/staging/ozwpan/ozcdev.c
@@ -217,7 +217,7 @@ static int oz_set_active_pd(const u8 *addr)
pd = oz_pd_find(addr);
if (pd) {
spin_lock_bh(g_cdev.lock);
- memcpy(g_cdev.active_addr, addr, ETH_ALEN);
+ ether_addr_copy(g_cdev.active_addr, addr);
Are you sure this will work?
No. But the ozwpan driver uses already ether_addr_equal which is not
alignment-safe. As
https://www.kernel.org/doc/Documentation/unaligned-memory-access.txt
said:
This alignment-unsafe function is still useful as it is a decent
optimization for the cases when you can ensure alignment, which is
true almost all of the time in ethernet networking context.
I expected the maintainer to confirm/infirm this. I'm just seeing that's
actually Chris Kelly who did write this part of code, so I'm CC'ing him
too.
It is aligned ok, but don't rely on the maintainer to fix your bugs.
Don't send patches which you are not sure about.
Joe, this seems like a very bad warning message from checkpatch.pl
because people will constantly send us patches over and over which
introduce bugs and they rely on the maintainer to catch it every time.
Can we get rid of the warning or move it under --strict or something?
Hi Dan.
Maybe.
The checkpatch message is:
Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are
__aligned(2)
My personal preference would be to add YA inline function
for unaligned copies ether_addr_copy_unaligned for symmetry
to ether_addr_equal_unaligned to etherdevice.h though.
Then the message could be changed to something like
Prefer ether_addr_copy[_unaligned] to memcpy(foo, bar, ETH_ALEN)
Do we have a mailing list yet for checkpatch issues?
No and I'm not going to advocate for one. I think the
subscriber count would be about 4 people total.
You could add yourself to the checkpatch MAINTAINERS entry
if you want to see more of the patches and discussions.
They are pretty rare.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/6] Staging: unisys: visorchannel: Clean up sparse warnings in visorchannel code
On Fri, Mar 28, 2014 at 09:05:38AM -0500, Ken Cox wrote:
On 03/28/2014 07:53 AM, Dan Carpenter wrote:
On Thu, Mar 13, 2014 at 03:39:21PM -0500, Ken Cox wrote:
Clean up code to get rid of sparse warnings, mostly due to accessing I/O
space.
Remove visorchannel_get_safe_queue(), visorchannel_safesignalremove(),
and visorchannel_safesignalinsert() since they were not called from
anywhere.
Signed-off-by: Ken Cox j...@redhat.com
---
drivers/staging/unisys/visorchannel/visorchannel.h | 2 +-
.../unisys/visorchannel/visorchannel_funcs.c | 136
-
2 files changed, 1 insertion(+), 137 deletions(-)
diff --git a/drivers/staging/unisys/visorchannel/visorchannel.h
b/drivers/staging/unisys/visorchannel/visorchannel.h
index 4546686..62d29a2 100644
--- a/drivers/staging/unisys/visorchannel/visorchannel.h
+++ b/drivers/staging/unisys/visorchannel/visorchannel.h
@@ -92,7 +92,7 @@ void *visorchannel_get_header(VISORCHANNEL *channel);
do {\
U8 *p = (U8 *)visorchannel_get_header(chan);\
if (p) {\
- ULTRA_CHANNEL_CLIENT_TRANSITION(p, chanId, CliStateOS, \
+ ULTRA_CHANNEL_CLIENT_TRANSITION(p, chanId, \
newstate, logCtx); \
Gar... You broke the build in [PATCH 2/6] and you're fixing it up here.
That's not allowed.
If this patchset hasn't already been applied, then please redo it.
I didn't catch that because the VISORCHANNEL_CHANGE_CLIENT_STATE
macro is never used. Because of that, it still builds.
Ah. Ok.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
adoze
cal matter, and is neare ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: comedi: usbdux: bug fix for accessing 'ao_chanlist' in private data
In usbdux_ao_cmd(), the channels for the command are transfered from the
cmd-chanlist and stored in the private data 'ao_chanlist'. The channel
numbers are bit-shifted when stored so that they become the command
that is transfered to the device. The channel to command conversion
results in the 'ao_chanlist' having these values for the channels:
channel 0 - ao_chanlist = 0x00
channel 1 - ao_chanlist = 0x40
channel 2 - ao_chanlist = 0x80
channel 3 - ao_chanlist = 0xc0
The problem is, the usbduxsub_ao_isoc_irq() function uses the 'chan' value
from 'ao_chanlist' to access the 'ao_readback' array in the private data.
So instead of accessing the array as 0, 1, 2, 3, it accesses it as 0x00,
0x40, 0x80, 0xc0.
Fix this by storing the raw channel number in 'ao_chanlist' and doing the
bit-shift when creating the command.
Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com
Cc: Ian Abbott abbo...@mev.co.uk
Cc: Bernd Porr m...@berndporr.me.uk
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
---
drivers/staging/comedi/drivers/usbdux.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/comedi/drivers/usbdux.c
b/drivers/staging/comedi/drivers/usbdux.c
index 71db683..b59af03 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -493,7 +493,7 @@ static void usbduxsub_ao_isoc_irq(struct urb *urb)
/* pointer to the DA */
*datap++ = val 0xff;
*datap++ = (val 8) 0xff;
- *datap++ = chan;
+ *datap++ = chan 6;
devpriv-ao_readback[chan] = val;
s-async-events |= COMEDI_CB_BLOCK;
@@ -1040,11 +1040,8 @@ static int usbdux_ao_cmd(struct comedi_device *dev,
struct comedi_subdevice *s)
/* set current channel of the running acquisition to zero */
s-async-cur_chan = 0;
- for (i = 0; i cmd-chanlist_len; ++i) {
- unsigned int chan = CR_CHAN(cmd-chanlist[i]);
-
- devpriv-ao_chanlist[i] = chan 6;
- }
+ for (i = 0; i cmd-chanlist_len; ++i)
+ devpriv-ao_chanlist[i] = CR_CHAN(cmd-chanlist[i]);
/* we count in steps of 1ms (125us) */
/* 125us mode not used yet */
--
1.8.5.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: netlogic: long lines in xlr_net.c
On Fri, Mar 28, 2014 at 02:43:01PM +0100, Neil Armstrong wrote: checkpatch script returns the following warning: WARNING: line over 80 characters 310: FILE: drivers/staging/netlogic/xlr_net.c:310: + void *accel_priv, select_queue_fallback_t fallback) This patch fixes the coding style issue. Signed-off-by: Neil Armstrong superna9...@gmail.com Heh. Looks ok this time but it doesn't apply to linux-next. Someone already sent this patch. Sorry about that. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 03/03] staging: dgap: remove more unneeded brd-state states
On Fri, Mar 28, 2014 at 09:08:34AM -0400, Mark Hounschell wrote:
On 03/28/2014 07:34 AM, Dan Carpenter wrote:
These patches are fine and they were applied already.
On Wed, Mar 12, 2014 at 12:50:55PM -0400, Mark Hounschell wrote:
@@ -4368,15 +4364,16 @@ static void dgap_do_bios_load(struct board_t *brd,
uchar __user *ubios, int len)
/*
* Checks to see if the BIOS completed running on the card.
*/
-static void dgap_do_wait_for_bios(struct board_t *brd)
+static int dgap_do_wait_for_bios(struct board_t *brd)
I wish this funciton returned negative error codes on error. It is
poorly named for a boolean function.
{
uchar *addr;
u16 word;
u16 err1;
u16 err2;
+ int ret = 0;
The ret variable is not needed. Replace it with zero literal for better
readability.
@@ -4455,15 +4452,16 @@ static void dgap_do_fep_load(struct board_t *brd,
uchar *ufep, int len)
/*
* Waits for the FEP to report thats its ready for us to use.
*/
-static void dgap_do_wait_for_fep(struct board_t *brd)
+static int dgap_do_wait_for_fep(struct board_t *brd)
Same as dgap_do_wait_for_bios().
Yes, they were not originally boolean functions. Would names like
dgap_test_bios and dgap_test_fep be better names? And returns of
-EIO if they fail and 0 if good?
What I'm saying is that by default kernel functions return zero on
success and negative error codes. If you're going to make a boolean
function then the name has to be clear.
if (!dgap_read_bios_is_wonderful(...))
return -ENXIO;
If it returns negative error codes then the current name is fine.
This isn't the kind of thing where you have to redo the function, I try
not to get too nit picky for naming in staging stuff because we can redo
it later anyway. It's just a comment for later consideration.
But yes, the new patch looks fine.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] staging: fix bcm/hostmibs.c checkpatch problems
On Sun, 23 Mar 2014 10:45:09 -0700 Joe Perches wrote: On Sat, 2014-03-22 at 09:50 -0600, Jake Edge wrote: Fix 4 checkpatch errors, many warnings in bcm/hostmibs.c Making code checkpatch clean shouldn't be the primary goal here. Perhaps not, but it was *my* goal :) Removing uses of Hungarian-style notation, CamelCase naming, and long variable names would be more helpful overall. I don't have enough experience to feel comfortable about doing that yet, but I did take some of your suggestions and am respinning the patch ... thanks, jake -- Jake Edge - j...@edge2.net - http://www.edge2.net ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/1] staging/bcm: fix hostmibs.c checkpatch problems
From: Jake Edge j...@edge2.net
Fix 4 checkpatch errors, many warnings in bcm/hostmibs.c
Against next-20140328 tree
Signed-off-by: Jake Edge j...@edge2.net
---
There were two 80 lines that I couldn't find a sensible way to split
up, otherwise it is checkpatch clean
v2: fixes suggested by Joe Perches
diff --git a/drivers/staging/bcm/hostmibs.c b/drivers/staging/bcm/hostmibs.c
index 39ace55..d3b9adb 100644
--- a/drivers/staging/bcm/hostmibs.c
+++ b/drivers/staging/bcm/hostmibs.c
@@ -9,37 +9,42 @@
#include headers.h
-INT ProcessGetHostMibs(struct bcm_mini_adapter *Adapter, struct
bcm_host_stats_mibs *pstHostMibs)
+INT ProcessGetHostMibs(struct bcm_mini_adapter *Adapter,
+ struct bcm_host_stats_mibs *pstHostMibs)
{
struct bcm_phs_entry *pstServiceFlowEntry = NULL;
struct bcm_phs_rule *pstPhsRule = NULL;
struct bcm_phs_classifier_table *pstClassifierTable = NULL;
struct bcm_phs_classifier_entry *pstClassifierRule = NULL;
- struct bcm_phs_extension *pDeviceExtension = (struct bcm_phs_extension
*) Adapter-stBCMPhsContext;
+ struct bcm_phs_extension *pDeviceExtension = Adapter-stBCMPhsContext;
- UINT nClassifierIndex = 0, nPhsTableIndex = 0, nSfIndex = 0, uiIndex =
0;
+ UINT nClassifierIndex = 0,
+nPhsTableIndex = 0,
+nSfIndex = 0,
+uiIndex = 0;
if (pDeviceExtension == NULL) {
- BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, HOST_MIBS,
DBG_LVL_ALL, Invalid Device Extension\n);
+ BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, HOST_MIBS,
+ DBG_LVL_ALL, Invalid Device Extension\n);
return STATUS_FAILURE;
}
/* Copy the classifier Table */
- for (nClassifierIndex = 0; nClassifierIndex MAX_CLASSIFIERS;
nClassifierIndex++) {
+ for (nClassifierIndex = 0; nClassifierIndex MAX_CLASSIFIERS;
+ nClassifierIndex++) {
if (Adapter-astClassifierTable[nClassifierIndex].bUsed == TRUE)
- memcpy((PVOID) pstHostMibs-
- astClassifierTable[nClassifierIndex],
- (PVOID) Adapter-
- astClassifierTable[nClassifierIndex],
+ memcpy(pstHostMibs-
+ astClassifierTable[nClassifierIndex],
+ Adapter-astClassifierTable[nClassifierIndex],
sizeof(struct bcm_mibs_classifier_rule));
}
/* Copy the SF Table */
for (nSfIndex = 0; nSfIndex NO_OF_QUEUES; nSfIndex++) {
if (Adapter-PackInfo[nSfIndex].bValid) {
- memcpy((PVOID) pstHostMibs-astSFtable[nSfIndex],
- (PVOID) Adapter-PackInfo[nSfIndex],
- sizeof(struct bcm_mibs_table));
+ memcpy(pstHostMibs-astSFtable[nSfIndex],
+ Adapter-PackInfo[nSfIndex],
+ sizeof(struct bcm_mibs_table));
} else {
/* If index in not valid,
* don't process this for the PHS table.
@@ -70,7 +75,8 @@ INT ProcessGetHostMibs(struct bcm_mini_adapter *Adapter,
struct bcm_host_stats_m
memcpy(pstHostMibs-
astPhsRulesTable[nPhsTableIndex].u8PHSI,
- pstPhsRule-u8PHSI, sizeof(struct
bcm_phs_rule));
+ pstPhsRule-u8PHSI,
+ sizeof(struct bcm_phs_rule));
nPhsTableIndex++;
}
@@ -82,53 +88,70 @@ INT ProcessGetHostMibs(struct bcm_mini_adapter *Adapter,
struct bcm_host_stats_m
/* Copy other Host Statistics parameters */
pstHostMibs-stHostInfo.GoodTransmits = Adapter-dev-stats.tx_packets;
pstHostMibs-stHostInfo.GoodReceives = Adapter-dev-stats.rx_packets;
- pstHostMibs-stHostInfo.CurrNumFreeDesc =
atomic_read(Adapter-CurrNumFreeTxDesc);
+ pstHostMibs-stHostInfo.CurrNumFreeDesc =
+ atomic_read(Adapter-CurrNumFreeTxDesc);
pstHostMibs-stHostInfo.BEBucketSize = Adapter-BEBucketSize;
pstHostMibs-stHostInfo.rtPSBucketSize = Adapter-rtPSBucketSize;
pstHostMibs-stHostInfo.TimerActive = Adapter-TimerActive;
pstHostMibs-stHostInfo.u32TotalDSD = Adapter-u32TotalDSD;
- memcpy(pstHostMibs-stHostInfo.aTxPktSizeHist, Adapter-aTxPktSizeHist,
sizeof(UINT32) * MIBS_MAX_HIST_ENTRIES);
- memcpy(pstHostMibs-stHostInfo.aRxPktSizeHist, Adapter-aRxPktSizeHist,
sizeof(UINT32) * MIBS_MAX_HIST_ENTRIES);
+ memcpy(pstHostMibs-stHostInfo.aTxPktSizeHist, Adapter-aTxPktSizeHist,
+ sizeof(UINT32
I need your help,
Good Day, Going through dossiers of personalities and properties in your Country, I discretely selected you for this proposition I am about to make you in,I am Khan Habib a personal consultant to Colonel Mauma Gaddafi of Libya. As you may be aware of the civil revolution that has just engulfed Libya, I happened to be in custody of several Millions of US dollars from the sovereign wealth fund. Without mincing words, what I am looking for is a trusted foreign partner that I can entrust the fund in his care for the purpose of future investment in low tax areas that can yield optimum returns to my capital Express your interest. I will very much to see your responses soon. Best regards. Khan Habib Email: hab...@qq.com NB: Upon your reply send to me your full name and Telephone Number ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] Drivers: hid: hid-hyperv: Implement a stub raw_request() entry point
commit 3c86726cfe38952f0366f86acfbbb025813ec1c2
Author: Benjamin Tissoires benjamin.tissoi...@redhat.com
Date: Thu Feb 20 15:24:49 2014 -0500
HID: make .raw_request mandatory
SET_REPORT and GET_REPORT are mandatory in the HID specification.
Make the corresponding API in hid-core mandatory too, which removes the
need to test against it in some various places.
Signed-off-by: Benjamin Tissoires benjamin.tissoi...@redhat.com
Reviewed-by: David Herrmann dh.herrm...@gmail.com
Signed-off-by: Jiri Kosina jkos...@suse.cz
Made .raw_request mandatory and broke the Hyper-V mouse driver. This patch
fixes the problem.
Signed-off-by: K. Y. Srinivasan k...@microsoft.com
---
drivers/hid/hid-hyperv.c | 10 ++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
index c24908f..89a9234 100644
--- a/drivers/hid/hid-hyperv.c
+++ b/drivers/hid/hid-hyperv.c
@@ -460,12 +460,22 @@ static void mousevsc_hid_stop(struct hid_device *hid)
{
}
+static int mousevsc_hid_raw_request(struct hid_device *hid,
+ unsigned char report_num,
+ __u8 buf, size_t len,
+ unsigned char rtype,
+ int reqtype)
+{
+ return 0;
+}
+
static struct hid_ll_driver mousevsc_ll_driver = {
.parse = mousevsc_hid_parse,
.open = mousevsc_hid_open,
.close = mousevsc_hid_close,
.start = mousevsc_hid_start,
.stop = mousevsc_hid_stop,
+ .raw_request = mousevsc_hid_raw_request,
};
static struct hid_driver mousevsc_hid_driver;
--
1.7.4.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
