Re: [PATCH 4/8] Drivers: scsi: storvsc: Filter WRITE_SAME_16
On Wed, Jul 09, 2014 at 10:27:24PM +, James Bottomley wrote: If we fix it at source, why would there be any need to filter? That's the reason the no_write_same flag was introduced. If we can find and fix the bug, it can go back into the stable trees as a bug fix, hence nothing should ever emit write_same(10 or 16) and additional driver code is redundant (and counter productive, since if this ever breaks again you're our best canary). This looks like it might be the problem but Martin should confirm (I think the problem comes to us from the RC16 code which unconditionally sets WS16). I think the problem is a differnet one. If we have the logical provisioning EVPD it configures what method to use, but if we don't have one we simply check for a max unmap blocks field, and if that's not present use WRITE SAME. The patch checks the no_write_same flag before doing that, for which we also have to do the write_same setup before the discard setup in sd_revalidate_disk. Ky: does hyperv support UNMAP? If so any idea why it doesn't set the maximum unmap block count field in the EVPD? If we want to enable UNMAP in this case I'd prefer a blacklist entry than trying UNMAP despite the device not advertising it. diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index ba756b1..fbccfd2 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -2614,9 +2614,10 @@ static void sd_read_block_limits(struct scsi_disk *sdkp) if (sdkp-max_unmap_blocks) sd_config_discard(sdkp, SD_LBP_UNMAP); - else + else if (!sdkp-device-no_write_same) sd_config_discard(sdkp, SD_LBP_WS16); - + else + sd_config_discard(sdkp, SD_LBP_DISABLE); } else {/* LBP VPD page tells us what to use */ if (sdkp-lbpu sdkp-max_unmap_blocks) @@ -2766,6 +2767,7 @@ static int sd_revalidate_disk(struct gendisk *disk) */ if (sdkp-media_present) { sd_read_capacity(sdkp, buffer); + sd_read_write_same(sdkp, buffer); if (sd_try_extended_inquiry(sdp)) { sd_read_block_provisioning(sdkp); @@ -2776,7 +2778,6 @@ static int sd_revalidate_disk(struct gendisk *disk) sd_read_write_protect_flag(sdkp, buffer); sd_read_cache_type(sdkp, buffer); sd_read_app_tag_own(sdkp, buffer); - sd_read_write_same(sdkp, buffer); } sdkp-first_scan = 0; ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: silicom: function return fixes
On Wed, Jul 09, 2014 at 11:59:14, Greg Kroah-Hartman wrote: On Tue, Jul 01, 2014 at 02:33:43PM +0200, Davide Gianforte wrote: + uint32_t ctrl_ext = BPCTL_READ_REG(pbpctl_dev, CTRL_EXT); - ctrl_ext = BPCTL_READ_REG(pbpctl_dev, CTRL_EXT); How about just removing the = 0; part of the variable definition? That would be a smaller patch, and still keep everything clean. thanks, greg k-h Also I prefer the style to keep definitions separated from assignments. I've seen a lot of kernel code which initialize variables with function returns, but as you said, it is not clean, even if it is code-correct. I'll rewrite the patch and I will send asap. thanks davide ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/8] Drivers: scsi: storvsc: Implement an abort handler
On Fri, Jul 11, 2014 at 11:52:55AM +0200, Hannes Reinecke wrote: Something like this should be sufficient: Right. That plus a detailed comment explaining why it's there.. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 59/60] staging: ced1401: ced_ioc.c fix checkpatch warnings
On 11/07/2014 00:12, Greg KH wrote: On Thu, Jul 10, 2014 at 11:04:15AM +0200, Luca Ellero wrote: Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/ced_ioc.c | 212 +++-- 1 file changed, 131 insertions(+), 81 deletions(-) That's a lot of changes all in one patch. Please take these last 4 patches and rework them to be more specific, and to only do one thing per patch (logical thing, like fixing comments in a file, etc.) I've applied all of the other ones in the series, so this should give you an easier base to work off of. thanks, greg k-h OK, I will rework the patches and resend regards Luca ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] drivers/staging/comedi/drivers/ni_atmio16d.c: remove pointless condition
The issue was discovered with static analysis and has two instances in this file. The code looks like this if (x 65536000) { ... } else if (x 65536) { ... } else if (x = 0x /* 655360 */) { ... } else if (x = 0x /* 6553600 */) { ... } The meaning of this block is to select appropriate clock frequency for interval timer basing on x, which is amount of time. Notes: 1. That last condition matches previous one - that's the issue. 2. Decimal numbers in comments don't match hex numbers in expressions. But in first case the numbers have same order, while in the second case the hex number is the same, and the decimal one is 10 times bigger. 3. Actually type of x is unsigned int, so its exact upper limit is not obviously known. 4. There's no else block. So it makes sense to make an else block from last else if case. The code inside the block seems correct for such usage. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=79871 Reported-by: David Binderman dcb...@hotmail.com Signed-off-by: Andrey Utkin andrey.krieger.ut...@gmail.com --- drivers/staging/comedi/drivers/ni_atmio16d.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/comedi/drivers/ni_atmio16d.c b/drivers/staging/comedi/drivers/ni_atmio16d.c index 6ad27f5..895b56d 100644 --- a/drivers/staging/comedi/drivers/ni_atmio16d.c +++ b/drivers/staging/comedi/drivers/ni_atmio16d.c @@ -338,7 +338,7 @@ static int atmio16d_ai_cmd(struct comedi_device *dev, } else if (cmd-convert_arg = 0x /* 655360 */) { base_clock = CLOCK_10_KHZ; timer = cmd-convert_arg / 10; - } else if (cmd-convert_arg = 0x /* 6553600 */) { + } else { base_clock = CLOCK_1_KHZ; timer = cmd-convert_arg / 100; } @@ -406,7 +406,7 @@ static int atmio16d_ai_cmd(struct comedi_device *dev, } else if (cmd-scan_begin_arg 0x /* 655360 */) { base_clock = CLOCK_10_KHZ; timer = cmd-scan_begin_arg / 10; - } else if (cmd-scan_begin_arg 0x /* 6553600 */) { + } else { base_clock = CLOCK_1_KHZ; timer = cmd-scan_begin_arg / 100; } -- 1.8.3.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/9] staging: dgap: redundant NULL and magic check in dgap_get_modem_info()
The ch is already checking in caller. Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- drivers/staging/dgap/dgap.c |3 --- 1 files changed, 0 insertions(+), 3 deletions(-) diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c index 9affc5b..79ea116 100644 --- a/drivers/staging/dgap/dgap.c +++ b/drivers/staging/dgap/dgap.c @@ -3061,9 +3061,6 @@ static int dgap_get_modem_info(struct channel_t *ch, unsigned int __user *value) ulong lock_flags; int rc; - if (!ch || ch-magic != DGAP_CHANNEL_MAGIC) - return -EIO; - spin_lock_irqsave(ch-ch_lock, lock_flags); mstat = readb((ch-ch_bs-m_stat)); -- 1.7.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/9] staging: dgap: remove unused variable in dgap_param()
The ts is not used in dgap_param(). Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- drivers/staging/dgap/dgap.c |3 --- 1 files changed, 0 insertions(+), 3 deletions(-) diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c index 79ea116..ad7b462 100644 --- a/drivers/staging/dgap/dgap.c +++ b/drivers/staging/dgap/dgap.c @@ -5068,7 +5068,6 @@ static void dgap_firmware_reset_port(struct channel_t *ch) *===*/ static int dgap_param(struct tty_struct *tty) { - struct ktermios *ts; struct board_t *bd; struct channel_t *ch; struct bs_t __iomem *bs; @@ -5098,8 +5097,6 @@ static int dgap_param(struct tty_struct *tty) if (!bs) return -EIO; - ts = tty-termios; - /* * If baud rate is zero, flush queues, and set mval to drop DTR. */ -- 1.7.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/9] staging: dgap: removes redundant null check and change the paramter for dgap_param()
The dgap_param() has a paramter which is tty_struct and use variables in that struct. That variables which are ch, bd, bs and un do not need to check NULL so these statements are removed. And also change the parameter of this function because it is possible to let someone know what paramters are needed for this function. Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- drivers/staging/dgap/dgap.c | 49 +++--- 1 files changed, 13 insertions(+), 36 deletions(-) diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c index ad7b462..4e5b594 100644 --- a/drivers/staging/dgap/dgap.c +++ b/drivers/staging/dgap/dgap.c @@ -141,7 +141,7 @@ static void dgap_cmdb(struct channel_t *ch, u8 cmd, u8 byte1, u8 byte2, uint ncmds); static void dgap_cmdw(struct channel_t *ch, u8 cmd, u16 word, uint ncmds); static void dgap_wmove(struct channel_t *ch, char *buf, uint cnt); -static int dgap_param(struct tty_struct *tty); +static int dgap_param(struct channel_t *ch, struct board_t *bd, u32 un_type); static void dgap_parity_scan(struct channel_t *ch, unsigned char *cbuf, unsigned char *fbuf, int *len); static uint dgap_get_custom_baud(struct channel_t *ch); @@ -2029,7 +2029,7 @@ static int dgap_tty_open(struct tty_struct *tty, struct file *file) /* * Run param in case we changed anything */ - dgap_param(tty); + dgap_param(ch, brd, un-un_type); /* * follow protocol for opening port @@ -2927,7 +2927,7 @@ static int dgap_tty_tiocmset(struct tty_struct *tty, ch-ch_mval = ~(D_DTR(ch)); } - dgap_param(tty); + dgap_param(ch, bd, un-un_type); spin_unlock_irqrestore(ch-ch_lock, lock_flags2); spin_unlock_irqrestore(bd-bd_lock, lock_flags); @@ -3173,7 +3173,7 @@ static int dgap_set_modem_info(struct tty_struct *tty, unsigned int command, spin_lock_irqsave(bd-bd_lock, lock_flags); spin_lock_irqsave(ch-ch_lock, lock_flags2); - dgap_param(tty); + dgap_param(ch, bd, un-un_type); spin_unlock_irqrestore(ch-ch_lock, lock_flags2); spin_unlock_irqrestore(bd-bd_lock, lock_flags); @@ -3285,7 +3285,7 @@ static int dgap_tty_digiseta(struct tty_struct *tty, if (ch-ch_digi.digi_offlen DIGI_PLEN) ch-ch_digi.digi_offlen = DIGI_PLEN; - dgap_param(tty); + dgap_param(ch, bd, un-un_type); spin_unlock_irqrestore(ch-ch_lock, lock_flags2); spin_unlock_irqrestore(bd-bd_lock, lock_flags); @@ -3372,7 +3372,7 @@ static int dgap_tty_digisetedelay(struct tty_struct *tty, int __user *new_info) writew((u16) new_digi, (ch-ch_bs-edelay)); - dgap_param(tty); + dgap_param(ch, bd, un-un_type); spin_unlock_irqrestore(ch-ch_lock, lock_flags2); spin_unlock_irqrestore(bd-bd_lock, lock_flags); @@ -3460,7 +3460,7 @@ static int dgap_tty_digisetcustombaud(struct tty_struct *tty, ch-ch_custom_speed = new_rate; - dgap_param(tty); + dgap_param(ch, bd, un-un_type); spin_unlock_irqrestore(ch-ch_lock, lock_flags2); spin_unlock_irqrestore(bd-bd_lock, lock_flags); @@ -3507,7 +3507,7 @@ static void dgap_tty_set_termios(struct tty_struct *tty, ch-ch_stopc = tty-termios.c_cc[VSTOP]; dgap_carrier(ch); - dgap_param(tty); + dgap_param(ch, bd, un-un_type); spin_unlock_irqrestore(ch-ch_lock, lock_flags2); spin_unlock_irqrestore(bd-bd_lock, lock_flags); @@ -3914,7 +3914,7 @@ static int dgap_tty_ioctl(struct tty_struct *tty, unsigned int cmd, spin_lock_irqsave(ch-ch_lock, lock_flags2); tty-termios.c_cflag = ((tty-termios.c_cflag ~CLOCAL) | (arg ? CLOCAL : 0)); - dgap_param(tty); + dgap_param(ch, bd, un-un_type); spin_unlock_irqrestore(ch-ch_lock, lock_flags2); spin_unlock_irqrestore(bd-bd_lock, lock_flags); @@ -4124,7 +4124,7 @@ static int dgap_tty_ioctl(struct tty_struct *tty, unsigned int cmd, case DIGI_RESET_PORT: dgap_firmware_reset_port(ch); - dgap_param(tty); + dgap_param(ch, bd, un-un_type); spin_unlock_irqrestore(ch-ch_lock, lock_flags2); spin_unlock_irqrestore(bd-bd_lock, lock_flags); return 0; @@ -5066,37 +5066,14 @@ static void dgap_firmware_reset_port(struct channel_t *ch) * struct tty_struct * - TTY for port. * *===*/ -static int dgap_param(struct tty_struct *tty) +static int dgap_param(struct channel_t *ch, struct board_t *bd, u32 un_type) { - struct board_t *bd; - struct channel_t *ch; - struct bs_t __iomem *bs; -
[PATCH 4/9] staging: dgap: removes redundant null check and change paramter for dgap_tty_digiseta()
Null checks in dgap_tty_digiseta() are already done by dgap_tty_ioctl() and change tty as a paramter of this function to ch and bd which are used in dgap_tty_digiseta(). Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- drivers/staging/dgap/dgap.c | 28 +--- 1 files changed, 5 insertions(+), 23 deletions(-) diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c index 4e5b594..160a960 100644 --- a/drivers/staging/dgap/dgap.c +++ b/drivers/staging/dgap/dgap.c @@ -88,8 +88,8 @@ static int dgap_tty_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg); static int dgap_tty_digigeta(struct tty_struct *tty, struct digi_t __user *retinfo); -static int dgap_tty_digiseta(struct tty_struct *tty, - struct digi_t __user *new_info); +static int dgap_tty_digiseta(struct channel_t *ch, struct board_t *bd, +struct un_t *un, struct digi_t __user *new_info); static int dgap_tty_digigetedelay(struct tty_struct *tty, int __user *retinfo); static int dgap_tty_digisetedelay(struct tty_struct *tty, int __user *new_info); static int dgap_tty_write_room(struct tty_struct *tty); @@ -3231,31 +3231,13 @@ static int dgap_tty_digigeta(struct tty_struct *tty, * * */ -static int dgap_tty_digiseta(struct tty_struct *tty, - struct digi_t __user *new_info) +static int dgap_tty_digiseta(struct channel_t *ch, struct board_t *bd, +struct un_t *un, struct digi_t __user *new_info) { - struct board_t *bd; - struct channel_t *ch; - struct un_t *un; struct digi_t new_digi; ulong lock_flags = 0; unsigned long lock_flags2; - if (!tty || tty-magic != TTY_MAGIC) - return -EFAULT; - - un = tty-driver_data; - if (!un || un-magic != DGAP_UNIT_MAGIC) - return -EFAULT; - - ch = un-un_ch; - if (!ch || ch-magic != DGAP_CHANNEL_MAGIC) - return -EFAULT; - - bd = ch-ch_bd; - if (!bd || bd-magic != DGAP_BOARD_MAGIC) - return -EFAULT; - if (copy_from_user(new_digi, new_info, sizeof(struct digi_t))) return -EFAULT; @@ -4100,7 +4082,7 @@ static int dgap_tty_ioctl(struct tty_struct *tty, unsigned int cmd, case DIGI_SETA: spin_unlock_irqrestore(ch-ch_lock, lock_flags2); spin_unlock_irqrestore(bd-bd_lock, lock_flags); - return dgap_tty_digiseta(tty, uarg); + return dgap_tty_digiseta(ch, bd, un, uarg); case DIGI_GEDELAY: spin_unlock_irqrestore(ch-ch_lock, lock_flags2); -- 1.7.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 09/12] staging: ced1401: usb1401.c fix checkpatch warning
Fix checkpatch warning suspect code indent for conditional statements in file usb1401.c Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/usb1401.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/ced1401/usb1401.c b/drivers/staging/ced1401/usb1401.c index eab5f04..9dce6cc 100644 --- a/drivers/staging/ced1401/usb1401.c +++ b/drivers/staging/ced1401/usb1401.c @@ -1256,7 +1256,8 @@ static void ced_readchar_callback(struct urb *urb) } if ((ced-num_input + got) = INBUF_SZ) - /* Adjust the buffer count accordingly */ + /* Adjust the buffer count */ + /* accordingly */ ced-num_input += got; } else dev_dbg(ced-interface-dev, %s: read ZLP\n, -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 06/12] staging: ced1401: ced_ioc.c fix checkpatch warning
Fix checkpatch warning else is not generally useful after a break or return in file ced_ioc.c, function ced_wait_event() Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/ced_ioc.c | 65 +++-- 1 file changed, 33 insertions(+), 32 deletions(-) diff --git a/drivers/staging/ced1401/ced_ioc.c b/drivers/staging/ced1401/ced_ioc.c index cb075af..0a40246 100644 --- a/drivers/staging/ced1401/ced_ioc.c +++ b/drivers/staging/ced1401/ced_ioc.c @@ -887,48 +887,49 @@ int ced_set_event(struct ced_data *ced, struct transfer_event __user *ute) int ced_wait_event(struct ced_data *ced, int area, int time_out) { int ret; + int wait; + struct transarea *ta; if ((unsigned)area = MAX_TRANSAREAS) return U14ERR_BADAREA; - else { - int wait; - struct transarea *ta = ced-trans_def[area]; -/* convert timeout to jiffies */ - time_out = (time_out * HZ + 999) / 1000; + ta = ced-trans_def[area]; - /* We cannot wait holding the mutex, but we check the flags */ - /* while holding it. This may well be pointless as another */ - /* thread could get in between releasing it and the wait */ - /* call. However, this would have to clear the wake_up flag. */ - /* However, the !ta-used may help us in this case. */ +/* convert timeout to jiffies */ + time_out = (time_out * HZ + 999) / 1000; - /* make sure we have no competitor */ - mutex_lock(ced-io_mutex); - if (!ta-used || !ta-event_sz) /* check something to */ - /* wait for...*/ - return U14ERR_NOTSET; /* ...else we do nothing */ - mutex_unlock(ced-io_mutex); + /* We cannot wait holding the mutex, but we check the flags */ + /* while holding it. This may well be pointless as another */ + /* thread could get in between releasing it and the wait */ + /* call. However, this would have to clear the wake_up flag. */ + /* However, the !ta-used may help us in this case. */ - if (time_out) - wait = wait_event_interruptible_timeout(ta-event, - ta-wake_up || - !ta-used, - time_out); - else - wait = wait_event_interruptible(ta-event, + /* make sure we have no competitor */ + mutex_lock(ced-io_mutex); + if (!ta-used || !ta-event_sz) /* check something to */ + /* wait for...*/ + return U14ERR_NOTSET; /* ...else we do nothing */ + mutex_unlock(ced-io_mutex); + + if (time_out) + wait = wait_event_interruptible_timeout(ta-event, ta-wake_up || - !ta-used); + !ta-used, + time_out); + else + wait = wait_event_interruptible(ta-event, + ta-wake_up || + !ta-used); + + if (wait) + ret = -ERESTARTSYS; /* oops - we have had a SIGNAL */ + else + ret = ta-wake_up; /* else the wakeup count */ - if (wait) - ret = -ERESTARTSYS; /* oops - we have had a SIGNAL */ - else - ret = ta-wake_up; /* else the wakeup count */ + spin_lock_irq(ced-staged_lock); + ta-wake_up = 0;/* clear the flag */ + spin_unlock_irq(ced-staged_lock); - spin_lock_irq(ced-staged_lock); - ta-wake_up = 0;/* clear the flag */ - spin_unlock_irq(ced-staged_lock); - } return ret; } -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 04/12] staging: ced1401: ced_ioc.c fix checkpatch warning
Fix checkpatch warning please, no space before tabs in file ced_ioc.c Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/ced_ioc.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/ced1401/ced_ioc.c b/drivers/staging/ced1401/ced_ioc.c index 397df43..a4bd013 100644 --- a/drivers/staging/ced1401/ced_ioc.c +++ b/drivers/staging/ced1401/ced_ioc.c @@ -344,7 +344,7 @@ static bool ced_is_1401(struct ced_data *ced) unsigned int state; ret = ced_in_self_test(ced, state); /* see if likely in */ -/* self test*/ +/* self test*/ if (ret 0) { /* do we need to wait for self-test? */ /* when to give up */ unsigned long timeout = jiffies + 30 * HZ; -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 00/12] staging: ced1401: rename camel case function and vars
This patchset renames all camel case and Hungarian notation names in ced1401. It fixes as well some checkpatch warnings and errors. I've not modified any logic in the driver. Please note that I've tryed to not add any checkpatch warning/error. If some patches give warnings or errors they were present before modification. Anyway I tryed to fix the most of them in the last 5 patches. Changes for v2: reworking of patches 54 to 115 which didn't apply properly onto linux-next. First 53 patches are already applied to linux-next. For simplification some patches are also melded together. Changes for v3: reworking of patches 111 to 115 (57-60 in v2): - split them on multiple patches - add more verbose comment Luca Ellero (12): staging: ced1401: usb1401.c fix checkpatch errors staging: ced1401: usb1401.h fix checkpatch errors staging: ced1401: ced_ioc.c fix checkpatch warnings staging: ced1401: ced_ioc.c fix checkpatch warning staging: ced1401: ced_ioc.c fix checkpatch warning staging: ced1401: ced_ioc.c fix checkpatch warning staging: ced1401: ced_ioc.c fix checkpatch warnings staging: ced1401: usb1401.c fix checkpatch warnings staging: ced1401: usb1401.c fix checkpatch warning staging: ced1401: rename camel case varialble staging: ced1401: usb1401.c fix checkpatch warnings staging: ced1401: usb1401.c fix checkpatch warnings drivers/staging/ced1401/ced_ioc.c | 195 drivers/staging/ced1401/usb1401.c | 225 ++--- drivers/staging/ced1401/usb1401.h |2 +- 3 files changed, 259 insertions(+), 163 deletions(-) -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 07/12] staging: ced1401: ced_ioc.c fix checkpatch warnings
Fix checkpatch warnings line over 80 characters in file ced_ioc.c Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/ced_ioc.c | 66 +++-- 1 file changed, 42 insertions(+), 24 deletions(-) diff --git a/drivers/staging/ced1401/ced_ioc.c b/drivers/staging/ced1401/ced_ioc.c index 0a40246..8cb6ea9 100644 --- a/drivers/staging/ced1401/ced_ioc.c +++ b/drivers/staging/ced1401/ced_ioc.c @@ -63,8 +63,10 @@ static void ced_flush_in_buff(struct ced_data *ced) { dev_dbg(ced-interface-dev, %s: current_state=%d\n, __func__, ced-current_state); - if (ced-current_state == U14ERR_TIME) /* Do nothing if hardware in trouble */ - return; + + if (ced-current_state == U14ERR_TIME) + return; /* Do nothing if hardware in trouble */ + /* Kill off any pending I/O */ /* CharRead_Cancel(pDevObject); */ spin_lock_irq(ced-char_in_lock); @@ -96,7 +98,9 @@ static int ced_put_chars(struct ced_data *ced, const char *ch, } ced-num_output += count; spin_unlock_irq(ced-char_out_lock); - ret = ced_send_chars(ced); /* ...give a chance to transmit data */ + + /* ...give a chance to transmit data */ + ret = ced_send_chars(ced); } else { ret = U14ERR_NOOUT; /* no room at the out (ha-ha) */ spin_unlock_irq(ced-char_out_lock); @@ -238,24 +242,31 @@ int ced_read_write_cancel(struct ced_data *ced) int ntStatus = STATUS_SUCCESS; bool bResult = false; unsigned int i; - /* We can fill this in when we know how we will implement the staged transfer stuff */ + + /* We can fill this in when we know how we will implement the staged */ + /* transfer stuff*/ spin_lock_irq(ced-staged_lock); - if (ced-staged_urb_pending) { /* anything to be cancelled? May need more... */ + if (ced-staged_urb_pending) {/* anything to be cancelled? */ + /* May need more... */ dev_info(ced-interface - dev, ced_read_write_cancel about to cancel Urb\n); /* Clear the staging done flag */ /* KeClearEvent(ced-StagingDoneEvent); */ USB_ASSERT(ced-pStagedIrp != NULL); - /* Release the spinlock first otherwise the completion routine may hang */ - /* on the spinlock while this function hands waiting for the event. */ + /* Release the spinlock first otherwise the completion */ + /* routine may hang on the spinlock while this function */ + /* hands waiting for the event. */ spin_unlock_irq(ced-staged_lock); - bResult = IoCancelIrp(ced-pStagedIrp); /* Actually do the cancel */ + + /* Actually do the cancel */ + bResult = IoCancelIrp(ced-pStagedIrp); if (bResult) { LARGE_INTEGER timeout; - timeout.QuadPart = -1000; /* Use a timeout of 1 second */ + /* Use a timeout of 1 second */ + timeout.QuadPart = -1000; dev_info(ced-interface - dev, %s: about to wait till done\n, __func__); ntStatus = @@ -268,8 +279,8 @@ int ced_read_write_cancel(struct ced_data *ced) ntStatus = U14ERR_FAIL; } USB_KdPrint(DBGLVL_DEFAULT, - (ced_read_write_cancel ntStatus = 0x%x decimal %d\n, -ntStatus, ntStatus)); + (ced_read_write_cancel ntStatus = 0x%x decimal %d\n, + ntStatus, ntStatus)); } else spin_unlock_irq(ced-staged_lock); @@ -389,7 +400,8 @@ static bool ced_quick_check(struct ced_data *ced, bool test_buff, short_test = ((ced-dma_flag == MODE_CHAR)/* no DMA running */ (!ced-force_reset) /* Not had a real reset forced */ - (ced-current_state = U14ERR_STD)); /* No 1401 errors stored */ + /* No 1401 errors stored */ + (ced-current_state = U14ERR_STD)); dev_dbg(ced-interface-dev, %s: DMAFlag:%d, state:%d, force:%d, testBuff:%d, short:%d\n, @@ -397,19 +409,24 @@ static bool ced_quick_check(struct ced_data *ced, bool test_buff, test_buff, short_test); if ((test_buff) /* Buffer check requested, and... */ - (ced-num_input || ced-num_output)) { /* ...characters were in the buffer? */ + (ced-num_input || ced-num_output)) {/* ...characters were in */ +
[PATCH v3 01/12] staging: ced1401: usb1401.c fix checkpatch errors
Fix checkpatch errors code indent should use tabs where possible in file usb1401.c Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/usb1401.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/ced1401/usb1401.c b/drivers/staging/ced1401/usb1401.c index 26e5f1b..a3d1ed6 100644 --- a/drivers/staging/ced1401/usb1401.c +++ b/drivers/staging/ced1401/usb1401.c @@ -610,9 +610,9 @@ static void staged_callback(struct urb *urb) ta-blocks[1].offset); } } else { /* If block 1 is not used, we try to add */ -/*to block 0 */ +/*to block 0 */ - /* Got stored block 0 information? */ + /* Got stored block 0 information? */ if (ta-blocks[0].size 0) { /* Must append onto the */ /*existing block 0 */ @@ -738,7 +738,7 @@ static void staged_callback(struct urb *urb) /* This feels wrong as we should ask which spin lock protects*/ /* dma_flag. */ restart_char_input = !cancel (ced-dma_flag == MODE_CHAR) - !ced-xfer_waiting; +!ced-xfer_waiting; spin_unlock(ced-staged_lock); /* Finally release the lock again */ @@ -803,7 +803,7 @@ static int ced_stage_chunk(struct ced_data *ced) ced-pipe_error[pipe] = 1; /* Flag an error to be */ /* handled later */ dev_err(ced-interface-dev, - %s: submit urb failed, code %d\n, + %s: submit urb failed, code %d\n, __func__, retval); } else /* Set the flag for staged URB pending */ -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 11/12] staging: ced1401: usb1401.c fix checkpatch warnings
Fix checkpatch warnings line over 80 characters in file usb1401.c Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/usb1401.c | 168 +++-- 1 file changed, 105 insertions(+), 63 deletions(-) diff --git a/drivers/staging/ced1401/usb1401.c b/drivers/staging/ced1401/usb1401.c index 6d0b013..907b6cf 100644 --- a/drivers/staging/ced1401/usb1401.c +++ b/drivers/staging/ced1401/usb1401.c @@ -1,11 +1,12 @@ -/*** - CED1401 usb driver. This basic loading is based on the usb-skeleton.c code that is: +/*** + CED1401 usb driver. This basic loading is based on the usb-skeleton.c code that + is: Copyright (C) 2001-2004 Greg Kroah-Hartman (g...@kroah.com) Copyright (C) 2012 Alois Schloegl alois.schlo...@ist.ac.at There is not a great deal of the skeleton left. - All the remainder dealing specifically with the CED1401 is based on drivers written - by CED for other systems (mainly Windows) and is: + All the remainder dealing specifically with the CED1401 is based on drivers + written by CED for other systems (mainly Windows) and is: Copyright (C) 2010 Cambridge Electronic Design Ltd Author Greg P Smith (g...@ced.co.uk) @@ -125,8 +126,9 @@ static void ced_delete(struct kref *kref) { struct ced_data *ced = to_ced_data(kref); - /* Free up the output buffer, then free the output urb. Note that the interface member */ - /* of ced will probably be NULL, so cannot be used to get to dev. */ + /* Free up the output buffer, then free the output urb. Note that the */ + /* interface member of ced will probably be NULL, so cannot be used */ + /* to get to dev. */ usb_free_coherent(ced-udev, OUTBUF_SZ, ced-coher_char_out, ced-urb_char_out-transfer_dma); usb_free_urb(ced-urb_char_out); @@ -183,7 +185,7 @@ static int ced_open(struct inode *inode, struct file *file) kref_put(ced-kref, ced_delete); goto exit; } - } else {/* uncomment this block if you want exclusive open */ + } else { /* uncomment this block if you want exclusive open */ dev_err(interface-dev, %s: fail: already open\n, __func__); retval = -EBUSY; ced-open_count--; @@ -214,7 +216,8 @@ static int ced_release(struct inode *inode, struct file *file) usb_autopm_put_interface(ced-interface); mutex_unlock(ced-io_mutex); - kref_put(ced-kref, ced_delete); /* decrement the count on our device */ + /* decrement the count on our device */ + kref_put(ced-kref, ced_delete); return 0; } @@ -296,7 +299,9 @@ static void ced_writechar_callback(struct urb *urb) spin_lock(ced-char_out_lock); /* already at irq level */ ced-num_output -= got; /* Now adjust the char send buffer */ ced-out_buff_get += got; /* to match what we did */ - if (ced-out_buff_get = OUTBUF_SZ) /* Can't do this any earlier as data could be overwritten */ + + /* Can't do this any earlier as data could be overwritten */ + if (ced-out_buff_get = OUTBUF_SZ) ced-out_buff_get = 0; if (ced-num_output 0) { /* if more to be done... */ @@ -306,13 +311,15 @@ static void ced_writechar_callback(struct urb *urb) /* maximum to send */ unsigned int count = ced-num_output; - if ((ced-out_buff_get + count) OUTBUF_SZ)/* does it cross buffer end? */ + /* does it cross buffer end? */ + if ((ced-out_buff_get + count) OUTBUF_SZ) count = OUTBUF_SZ - ced-out_buff_get; /* we are done with stuff that changes */ spin_unlock(ced-char_out_lock); - memcpy(ced-coher_char_out, pDat, count); /* copy output data to the buffer */ + /* copy output data to the buffer */ + memcpy(ced-coher_char_out, pDat, count); usb_fill_bulk_urb(ced-urb_char_out, ced-udev, usb_sndbulkpipe(ced-udev, ced-ep_addr[0]), @@ -320,14 +327,21 @@ static void ced_writechar_callback(struct urb *urb) ced_writechar_callback, ced); ced-urb_char_out-transfer_flags |= URB_NO_TRANSFER_DMA_MAP; - usb_anchor_urb(ced-urb_char_out, ced-submitted); /* in case we need to kill it */ + +
[PATCH v3 03/12] staging: ced1401: ced_ioc.c fix checkpatch warnings
Fix checkpatch warnings Missing a blank line after declarations in file ced_ioc.c Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/ced_ioc.c | 26 ++ 1 file changed, 26 insertions(+) diff --git a/drivers/staging/ced1401/ced_ioc.c b/drivers/staging/ced1401/ced_ioc.c index fb4e8a3..397df43 100644 --- a/drivers/staging/ced1401/ced_ioc.c +++ b/drivers/staging/ced1401/ced_ioc.c @@ -84,9 +84,11 @@ static int ced_put_chars(struct ced_data *ced, const char *ch, unsigned int count) { int ret; + spin_lock_irq(ced-char_out_lock); /* get the output spin lock */ if ((OUTBUF_SZ - ced-num_output) = count) { unsigned int u; + for (u = 0; u count; u++) { ced-output_buffer[ced-out_buff_put++] = ch[u]; if (ced-out_buff_put = OUTBUF_SZ) @@ -140,6 +142,7 @@ int ced_send_string(struct ced_data *ced, const char __user *data, int ced_send_char(struct ced_data *ced, char c) { int ret; + mutex_lock(ced-io_mutex); /* Protect disconnect from new i/o */ ret = ced_put_chars(ced, c, 1); dev_dbg(ced-interface-dev, ced_send_char %c (0x%02x)\n, c, c); @@ -177,6 +180,7 @@ int ced_send_char(struct ced_data *ced, char c) int ced_get_state(struct ced_data *ced, __u32 *state, __u32 *error) { int got; + dev_dbg(ced-interface-dev, %s: entry\n, __func__); *state = 0x;/* Start off with invalid state */ @@ -192,6 +196,7 @@ int ced_get_state(struct ced_data *ced, __u32 *state, __u32 *error) *error = 0; } else { int device; + dev_dbg(ced-interface-dev, %s: Success, state: 0x%x, 0x%x\n, __func__, ced-stat_buf[0], ced-stat_buf[1]); @@ -249,6 +254,7 @@ int ced_read_write_cancel(struct ced_data *ced) bResult = IoCancelIrp(ced-pStagedIrp); /* Actually do the cancel */ if (bResult) { LARGE_INTEGER timeout; + timeout.QuadPart = -1000; /* Use a timeout of 1 second */ dev_info(ced-interface - dev, %s: about to wait till done\n, __func__); @@ -283,6 +289,7 @@ static int ced_in_self_test(struct ced_data *ced, unsigned int *stat) { unsigned int state, error; int ret = ced_get_state(ced, state, error); /* see if in self-test */ + if (ret == U14ERR_NOERROR) /* if all still OK */ ret = (state == (unsigned int)-1) ||/* TX problem or... */ ((state 0xff) == 0x80); /* ...self test */ @@ -311,6 +318,7 @@ static int ced_in_self_test(struct ced_data *ced, unsigned int *stat) static bool ced_is_1401(struct ced_data *ced) { int ret; + dev_dbg(ced-interface-dev, %s\n, __func__); ced_draw_down(ced); /* wait for, then kill outstanding Urbs */ @@ -334,11 +342,13 @@ static bool ced_is_1401(struct ced_data *ced) ced-dma_flag = MODE_CHAR; /* Clear DMA mode flag regardless! */ if (ret == 0) { /* if all is OK still */ unsigned int state; + ret = ced_in_self_test(ced, state); /* see if likely in */ /* self test*/ if (ret 0) { /* do we need to wait for self-test? */ /* when to give up */ unsigned long timeout = jiffies + 30 * HZ; + while ((ret 0) time_before(jiffies, timeout)) { schedule(); /* let other stuff run */ @@ -396,6 +406,7 @@ static bool ced_quick_check(struct ced_data *ced, bool test_buff, if (short_test || !can_reset) { /* Still OK to try the short test? */ /* Always test if no reset - we want state update */ unsigned int state, error; + dev_dbg(ced-interface-dev, %s: ced_get_state\n, __func__); if (ced_get_state(ced, state, error) == U14ERR_NOERROR) { /* Check on the 1401 state */ if ((state 0xFF) == 0)/* If call worked, check the status value */ @@ -437,6 +448,7 @@ int ced_reset(struct ced_data *ced) int ced_get_char(struct ced_data *ced) { int ret = U14ERR_NOIN; /* assume we will get nothing */ + mutex_lock(ced-io_mutex); /* Protect disconnect from new i/o */ dev_dbg(ced-interface-dev, %s\n, __func__); @@ -475,6 +487,7 @@ int ced_get_string(struct ced_data *ced, char __user *user, int n) { int available; /* character in the buffer */ int ret = U14ERR_NOIN; + if (n = 0) return -ENOMEM; @@ -492,6 +505,7 @@ int ced_get_string(struct ced_data *ced, char __user *user, int n)
[PATCH v3 08/12] staging: ced1401: usb1401.c fix checkpatch warnings
Fix checkpatch warnings Missing a blank line after declarations in file usb1401.c Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/usb1401.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/staging/ced1401/usb1401.c b/drivers/staging/ced1401/usb1401.c index a3d1ed6..eab5f04 100644 --- a/drivers/staging/ced1401/usb1401.c +++ b/drivers/staging/ced1401/usb1401.c @@ -204,6 +204,7 @@ exit: static int ced_release(struct inode *inode, struct file *file) { struct ced_data *ced = file-private_data; + if (ced == NULL) return -ENODEV; @@ -221,6 +222,7 @@ static int ced_flush(struct file *file, fl_owner_t id) { int res; struct ced_data *ced = file-private_data; + if (ced == NULL) return -ENODEV; @@ -302,6 +304,7 @@ static void ced_writechar_callback(struct urb *urb) int ret; char *pDat = ced-output_buffer[ced-out_buff_get]; unsigned int dwCount = ced-num_output; /* maximum to send */ + if ((ced-out_buff_get + dwCount) OUTBUF_SZ) /* does it cross buffer end? */ dwCount = OUTBUF_SZ - ced-out_buff_get; @@ -449,12 +452,14 @@ int ced_send_chars(struct ced_data *ced) static void ced_copy_user_space(struct ced_data *ced, int n) { unsigned int area = ced-staged_id; + if (area MAX_TRANSAREAS) { /* area to be used */ struct transarea *ta = ced-trans_def[area]; unsigned int offset = ced-staged_done + ced-staged_offset + ta-base_offset; char *coher_buf = ced-coher_staged_io; /* coherent buffer */ + if (!ta-used) { dev_err(ced-interface-dev, %s: area %d unused\n, __func__, area); @@ -711,6 +716,7 @@ static void staged_callback(struct urb *urb) /* If we have a transfer waiting, kick it off */ if (ced-xfer_waiting) {/* Got a block xfer waiting? */ int retval; + dev_info(ced-interface-dev, *** RWM_Complete *** pending transfer will now be set up!!!\n); @@ -1231,6 +1237,7 @@ static void ced_readchar_callback(struct urb *urb) if (got 0) { unsigned int i; + if (got INBUF_SZ) { /* tidy the string */ ced-coher_char_in[got] = 0; @@ -1339,6 +1346,7 @@ static long ced_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { int err = 0; struct ced_data *ced = file-private_data; + if (!can_accept_io_requests(ced)) /* check we still exist */ return -ENODEV; @@ -1629,6 +1637,7 @@ static void ced_disconnect(struct usb_interface *interface) ced_draw_down(ced); /* ...wait for then kill any io */ for (i = 0; i MAX_TRANSAREAS; ++i) { int err = ced_clear_area(ced, i); /* ...release any used memory */ + if (err == U14ERR_UNLOCKFAIL) dev_err(ced-interface-dev, %s: Area %d was in used\n, __func__, i); @@ -1650,6 +1659,7 @@ static void ced_disconnect(struct usb_interface *interface) void ced_draw_down(struct ced_data *ced) { int time; + dev_dbg(ced-interface-dev, %s: called\n, __func__); ced-in_draw_down = true; @@ -1664,6 +1674,7 @@ void ced_draw_down(struct ced_data *ced) static int ced_suspend(struct usb_interface *intf, pm_message_t message) { struct ced_data *ced = usb_get_intfdata(intf); + if (!ced) return 0; ced_draw_down(ced); @@ -1675,6 +1686,7 @@ static int ced_suspend(struct usb_interface *intf, pm_message_t message) static int ced_resume(struct usb_interface *intf) { struct ced_data *ced = usb_get_intfdata(intf); + if (!ced) return 0; dev_dbg(ced-interface-dev, %s: called\n, __func__); @@ -1684,6 +1696,7 @@ static int ced_resume(struct usb_interface *intf) static int ced_pre_reset(struct usb_interface *intf) { struct ced_data *ced = usb_get_intfdata(intf); + dev_dbg(ced-interface-dev, %s\n, __func__); mutex_lock(ced-io_mutex); ced_draw_down(ced); @@ -1693,6 +1706,7 @@ static int ced_pre_reset(struct usb_interface *intf) static int ced_post_reset(struct usb_interface *intf) { struct ced_data *ced = usb_get_intfdata(intf); + dev_dbg(ced-interface-dev, %s\n, __func__); /* we are sure no URBs are active - no locking needed */ -- 1.7.10.4 ___
[PATCH 6/9] staging: dgap: removes redundant null check and change paramter for dgap_set_modem_info()
Null checks in dgap_set_modem_info() are already done by dgap_tty_ioctl() and change tty as a paramter of this function to ch, bd and un. Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- drivers/staging/dgap/dgap.c | 36 1 files changed, 8 insertions(+), 28 deletions(-) diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c index 7ebbf50..1d9e3e8 100644 --- a/drivers/staging/dgap/dgap.c +++ b/drivers/staging/dgap/dgap.c @@ -86,8 +86,7 @@ static int dgap_block_til_ready(struct tty_struct *tty, struct file *file, struct channel_t *ch); static int dgap_tty_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg); -static int dgap_tty_digigeta(struct channel_t *ch, struct un_t *un, -struct digi_t __user *retinfo); +static int dgap_tty_digigeta(struct channel_t *ch, struct digi_t __user *retinfo); static int dgap_tty_digiseta(struct channel_t *ch, struct board_t *bd, struct un_t *un, struct digi_t __user *new_info); static int dgap_tty_digigetedelay(struct tty_struct *tty, int __user *retinfo); @@ -102,8 +101,8 @@ static void dgap_tty_flush_chars(struct tty_struct *tty); static void dgap_tty_flush_buffer(struct tty_struct *tty); static void dgap_tty_hangup(struct tty_struct *tty); static int dgap_wait_for_drain(struct tty_struct *tty); -static int dgap_set_modem_info(struct tty_struct *tty, unsigned int command, - unsigned int __user *value); +static int dgap_set_modem_info(struct channel_t *ch, struct board_t *bd, struct un_t *un, + unsigned int command, unsigned int __user *value); static int dgap_get_modem_info(struct channel_t *ch, unsigned int __user *value); static int dgap_tty_digisetcustombaud(struct tty_struct *tty, @@ -3094,32 +3093,14 @@ static int dgap_get_modem_info(struct channel_t *ch, unsigned int __user *value) * * Set modem signals, called by ld. */ -static int dgap_set_modem_info(struct tty_struct *tty, unsigned int command, - unsigned int __user *value) +static int dgap_set_modem_info(struct channel_t *ch, struct board_t *bd, struct un_t *un, + unsigned int command, unsigned int __user *value) { - struct board_t *bd; - struct channel_t *ch; - struct un_t *un; int ret; unsigned int arg; ulong lock_flags; ulong lock_flags2; - if (!tty || tty-magic != TTY_MAGIC) - return -EIO; - - un = tty-driver_data; - if (!un || un-magic != DGAP_UNIT_MAGIC) - return -EIO; - - ch = un-un_ch; - if (!ch || ch-magic != DGAP_CHANNEL_MAGIC) - return -EIO; - - bd = ch-ch_bd; - if (!bd || bd-magic != DGAP_BOARD_MAGIC) - return -EIO; - ret = get_user(arg, value); if (ret) return ret; @@ -3189,8 +3170,7 @@ static int dgap_set_modem_info(struct tty_struct *tty, unsigned int command, * * */ -static int dgap_tty_digigeta(struct channel_t *ch, struct un_t *un, -struct digi_t __user *retinfo) +static int dgap_tty_digigeta(struct channel_t *ch, struct digi_t __user *retinfo) { struct digi_t tmp; ulong lock_flags; @@ -3899,7 +3879,7 @@ static int dgap_tty_ioctl(struct tty_struct *tty, unsigned int cmd, case TIOCMSET: spin_unlock_irqrestore(ch-ch_lock, lock_flags2); spin_unlock_irqrestore(bd-bd_lock, lock_flags); - return dgap_set_modem_info(tty, cmd, uarg); + return dgap_set_modem_info(ch, bd, un, cmd, uarg); /* * Here are any additional ioctl's that we want to implement @@ -4047,7 +4027,7 @@ static int dgap_tty_ioctl(struct tty_struct *tty, unsigned int cmd, /* get information for ditty */ spin_unlock_irqrestore(ch-ch_lock, lock_flags2); spin_unlock_irqrestore(bd-bd_lock, lock_flags); - return dgap_tty_digigeta(ch, un, uarg); + return dgap_tty_digigeta(ch, uarg); case DIGI_SETAW: case DIGI_SETAF: -- 1.7.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 02/12] staging: ced1401: usb1401.h fix checkpatch errors
Fix checkpatch error foo * bar should be foo *bar in file usb1401.h Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/usb1401.h |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/ced1401/usb1401.h b/drivers/staging/ced1401/usb1401.h index 8e800c3..da4d90c 100644 --- a/drivers/staging/ced1401/usb1401.h +++ b/drivers/staging/ced1401/usb1401.h @@ -213,7 +213,7 @@ struct ced_data { /* Definitions of routimes used between compilation object files */ /* in usb1401.c */ -extern int ced_allowi(struct ced_data * ced); +extern int ced_allowi(struct ced_data *ced); extern int ced_send_chars(struct ced_data *ced); extern void ced_draw_down(struct ced_data *ced); extern int ced_read_write_mem(struct ced_data *ced, bool read, -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 7/9] staging: dgap: removes redundant null check and change paramter for dgap_tty_digisetedelay()
Null checks in dgap_tty_digisetedelay() are already done by dgap_tty_ioctl() and change tty as a paramter of this function to ch, bd and un. Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- drivers/staging/dgap/dgap.c | 26 +- 1 files changed, 5 insertions(+), 21 deletions(-) diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c index 1d9e3e8..6bc4a30 100644 --- a/drivers/staging/dgap/dgap.c +++ b/drivers/staging/dgap/dgap.c @@ -90,7 +90,8 @@ static int dgap_tty_digigeta(struct channel_t *ch, struct digi_t __user *retinfo static int dgap_tty_digiseta(struct channel_t *ch, struct board_t *bd, struct un_t *un, struct digi_t __user *new_info); static int dgap_tty_digigetedelay(struct tty_struct *tty, int __user *retinfo); -static int dgap_tty_digisetedelay(struct tty_struct *tty, int __user *new_info); +static int dgap_tty_digisetedelay(struct channel_t *ch, struct board_t *bd, + struct un_t *un, int __user *new_info); static int dgap_tty_write_room(struct tty_struct *tty); static int dgap_tty_chars_in_buffer(struct tty_struct *tty); static void dgap_tty_start(struct tty_struct *tty); @@ -3289,30 +3290,13 @@ static int dgap_tty_digigetedelay(struct tty_struct *tty, int __user *retinfo) * Ioctl to set the EDELAY setting * */ -static int dgap_tty_digisetedelay(struct tty_struct *tty, int __user *new_info) +static int dgap_tty_digisetedelay(struct channel_t *ch, struct board_t *bd, + struct un_t *un, int __user *new_info) { - struct board_t *bd; - struct channel_t *ch; - struct un_t *un; int new_digi; ulong lock_flags; ulong lock_flags2; - if (!tty || tty-magic != TTY_MAGIC) - return -EFAULT; - - un = tty-driver_data; - if (!un || un-magic != DGAP_UNIT_MAGIC) - return -EFAULT; - - ch = un-un_ch; - if (!ch || ch-magic != DGAP_CHANNEL_MAGIC) - return -EFAULT; - - bd = ch-ch_bd; - if (!bd || bd-magic != DGAP_BOARD_MAGIC) - return -EFAULT; - if (copy_from_user(new_digi, new_info, sizeof(int))) return -EFAULT; @@ -4059,7 +4043,7 @@ static int dgap_tty_ioctl(struct tty_struct *tty, unsigned int cmd, case DIGI_SEDELAY: spin_unlock_irqrestore(ch-ch_lock, lock_flags2); spin_unlock_irqrestore(bd-bd_lock, lock_flags); - return dgap_tty_digisetedelay(tty, uarg); + return dgap_tty_digisetedelay(ch, bd, un, uarg); case DIGI_GETCUSTOMBAUD: spin_unlock_irqrestore(ch-ch_lock, lock_flags2); -- 1.7.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 05/12] staging: ced1401: ced_ioc.c fix checkpatch warning
Fix checkpatch warning else is not generally useful after a break or return in file ced_ioc.c, function ced_set_event() Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/ced_ioc.c | 36 +++- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/drivers/staging/ced1401/ced_ioc.c b/drivers/staging/ced1401/ced_ioc.c index a4bd013..cb075af 100644 --- a/drivers/staging/ced1401/ced_ioc.c +++ b/drivers/staging/ced1401/ced_ioc.c @@ -844,6 +844,7 @@ int ced_set_event(struct ced_data *ced, struct transfer_event __user *ute) { int ret = U14ERR_NOERROR; struct transfer_event te; + struct transarea *ta; /* get a local copy of the data */ if (copy_from_user(te, ute, sizeof(te))) @@ -851,27 +852,28 @@ int ced_set_event(struct ced_data *ced, struct transfer_event __user *ute) if (te.wAreaNum = MAX_TRANSAREAS) /* the area must exist */ return U14ERR_BADAREA; - else { - struct transarea *ta = ced-trans_def[te.wAreaNum]; - /* make sure we have no competitor */ - mutex_lock(ced-io_mutex); - spin_lock_irq(ced-staged_lock); + ta = ced-trans_def[te.wAreaNum]; - if (ta-used) { /* area must be in use */ - ta-event_st = te.dwStart; /* set area regions */ + /* make sure we have no competitor */ + mutex_lock(ced-io_mutex); + spin_lock_irq(ced-staged_lock); -/* set size (0 cancels it) */ - ta-event_sz = te.dwLength; + if (ta-used) { /* area must be in use */ + ta-event_st = te.dwStart; /* set area regions */ + +/* set size (0 cancels it) */ + ta-event_sz = te.dwLength; + +/* set the direction */ + ta-event_to_host = te.wFlags 1; + ta-wake_up = 0;/* zero the wake up count */ + } else + ret = U14ERR_NOTSET; + + spin_unlock_irq(ced-staged_lock); + mutex_unlock(ced-io_mutex); -/* set the direction */ - ta-event_to_host = te.wFlags 1; - ta-wake_up = 0;/* zero the wake up count */ - } else - ret = U14ERR_NOTSET; - spin_unlock_irq(ced-staged_lock); - mutex_unlock(ced-io_mutex); - } return ret == U14ERR_NOERROR ? (te.iSetEvent ? 1 : U14ERR_NOERROR) : ret; } -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/4] staging: comedi: addi_apci_1564: small fixes in apci1564_auto_attach() and apci1564_detach()
On 2014-07-11 05:00, Chase Southwood wrote: This is a small patchset containing a handful of fixes to the ADDI-DATA APCI1564 driver that I would like to get out of the way before I forget to take care of them. From here, I will move to start fixing the digital input/timer/counters/watchdog functionality of the board. Chase Southwood (4): staging: comedi: addi_apci_1564: remove len_chanlist from di and do subdevices staging: comedi: addi_apci_1564: remove unnecessary dev-board_name initialization staging: comedi: addi_apci_1564: remove null check of devpriv in apci1564_detach() staging: comedi: addi_apci_1564: fix s-maxdata assignment in do subdevice init. drivers/staging/comedi/drivers/addi_apci_1564.c | 18 +- 1 file changed, 5 insertions(+), 13 deletions(-) Reviewed-by: Ian Abbott abbo...@mev.co.uk -- -=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=- -=( 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
[PATCH v3 12/12] staging: ced1401: usb1401.c fix checkpatch warnings
Fix checkpatch warnings quoted string split across lines in file usb1401.c Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/usb1401.c | 23 +++ 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/drivers/staging/ced1401/usb1401.c b/drivers/staging/ced1401/usb1401.c index 907b6cf..9988677 100644 --- a/drivers/staging/ced1401/usb1401.c +++ b/drivers/staging/ced1401/usb1401.c @@ -627,8 +627,7 @@ static void staged_callback(struct urb *urb) ta-blocks[1].size += ced-staged_length; dev_dbg(ced-interface-dev, - RWM_Complete, circ block 1 - now %d bytes at %d\n, + RWM_Complete, circ block 1 now %d bytes at %d\n, ta-blocks[1].size, ta-blocks[1].offset); } else { @@ -640,8 +639,7 @@ static void staged_callback(struct urb *urb) ta-blocks[1].size = ced-staged_length; dev_err(ced-interface-dev, - %s: ERROR, circ block 1 - re-started %d bytes at %d\n, + %s: ERROR, circ block 1 re-started %d bytes at %d\n, __func__, ta-blocks[1].size, ta-blocks[1].offset); @@ -660,9 +658,7 @@ static void staged_callback(struct urb *urb) ta-blocks[0].size += ced-staged_length; dev_dbg(ced-interface-dev, - RWM_Complete, circ - block 0 now %d bytes - at %d\n, + RWM_Complete, circ block 0 now %d bytes at %d\n, ta-blocks[0].size, ta-blocks[0].offset); @@ -673,9 +669,7 @@ static void staged_callback(struct urb *urb) ta-blocks[1].size = ced-staged_length; dev_dbg(ced-interface-dev, - RWM_Complete, circ - block 1 started %d - bytes at %d\n, + RWM_Complete, circ block 1 started %d bytes at %d\n, ta-blocks[1].size, ta-blocks[1].offset); } @@ -686,8 +680,7 @@ static void staged_callback(struct urb *urb) ta-blocks[0].size = ced-staged_length; dev_dbg(ced-interface-dev, - RWM_Complete, circ block 0 - started %d bytes at %d\n, + RWM_Complete, circ block 0 started %d bytes at %d\n, ta-blocks[0].size, ta-blocks[0].offset); } @@ -696,8 +689,7 @@ static void staged_callback(struct urb *urb) if (!cancel) { /* Don't generate an event if cancelled */ dev_dbg(ced-interface-dev, - RWM_Complete, bCircular %d, bToHost %d, - eStart %d, eSize %d\n, + RWM_Complete, bCircular %d, bToHost %d, eStart %d, eSize %d\n, ta-circular, ta-event_to_host, ta-event_st, ta-event_sz); /* Set a user-mode event... */ @@ -752,8 +744,7 @@ static void staged_callback(struct urb *urb) int retval; dev_info(ced-interface-dev, -*** RWM_Complete *** pending transfer - will now be set
[PATCH v3 10/12] staging: ced1401: rename camel case varialble
Rename camel case variable dwCount in function ced_writechar_callback Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/usb1401.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/staging/ced1401/usb1401.c b/drivers/staging/ced1401/usb1401.c index 9dce6cc..6d0b013 100644 --- a/drivers/staging/ced1401/usb1401.c +++ b/drivers/staging/ced1401/usb1401.c @@ -303,26 +303,27 @@ static void ced_writechar_callback(struct urb *urb) int pipe = 0; /* The pipe number to use */ int ret; char *pDat = ced-output_buffer[ced-out_buff_get]; - unsigned int dwCount = ced-num_output; /* maximum to send */ + /* maximum to send */ + unsigned int count = ced-num_output; - if ((ced-out_buff_get + dwCount) OUTBUF_SZ) /* does it cross buffer end? */ - dwCount = OUTBUF_SZ - ced-out_buff_get; + if ((ced-out_buff_get + count) OUTBUF_SZ)/* does it cross buffer end? */ + count = OUTBUF_SZ - ced-out_buff_get; /* we are done with stuff that changes */ spin_unlock(ced-char_out_lock); - memcpy(ced-coher_char_out, pDat, dwCount); /* copy output data to the buffer */ + memcpy(ced-coher_char_out, pDat, count); /* copy output data to the buffer */ usb_fill_bulk_urb(ced-urb_char_out, ced-udev, usb_sndbulkpipe(ced-udev, ced-ep_addr[0]), - ced-coher_char_out, dwCount, + ced-coher_char_out, count, ced_writechar_callback, ced); ced-urb_char_out-transfer_flags |= URB_NO_TRANSFER_DMA_MAP; usb_anchor_urb(ced-urb_char_out, ced-submitted); /* in case we need to kill it */ ret = usb_submit_urb(ced-urb_char_out, GFP_ATOMIC); dev_dbg(ced-interface-dev, %s: n=%d%s\n, - __func__, dwCount, pDat); + __func__, count, pDat); spin_lock(ced-char_out_lock); /* grab lock for errors */ if (ret) { ced-pipe_error[pipe] = 1; /* Flag an error to be handled later */ -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 9/9] staging: dgap: removes redundant null check and change paramter for dgap_tty_digisetcustombaud()
Null checks in dgap_tty_digisetcustombaud() are already done by dgap_tty_ioctl() and change tty as a paramter of this function to ch, bd and un. Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- drivers/staging/dgap/dgap.c | 29 + 1 files changed, 5 insertions(+), 24 deletions(-) diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c index 3b76958..32988a8 100644 --- a/drivers/staging/dgap/dgap.c +++ b/drivers/staging/dgap/dgap.c @@ -106,8 +106,8 @@ static int dgap_set_modem_info(struct channel_t *ch, struct board_t *bd, struct unsigned int command, unsigned int __user *value); static int dgap_get_modem_info(struct channel_t *ch, unsigned int __user *value); -static int dgap_tty_digisetcustombaud(struct tty_struct *tty, - int __user *new_info); +static int dgap_tty_digisetcustombaud(struct channel_t *ch, struct board_t *bd, + struct un_t *un, int __user *new_info); static int dgap_tty_digigetcustombaud(struct channel_t *ch, struct un_t *un, int __user *retinfo); static int dgap_tty_tiocmget(struct tty_struct *tty); @@ -3344,32 +3344,13 @@ static int dgap_tty_digigetcustombaud(struct channel_t *ch, struct un_t *un, * * Ioctl to set the custom baud rate setting */ -static int dgap_tty_digisetcustombaud(struct tty_struct *tty, - int __user *new_info) +static int dgap_tty_digisetcustombaud(struct channel_t *ch, struct board_t *bd, + struct un_t *un, int __user *new_info) { - struct board_t *bd; - struct channel_t *ch; - struct un_t *un; uint new_rate; ulong lock_flags; ulong lock_flags2; - if (!tty || tty-magic != TTY_MAGIC) - return -EFAULT; - - un = tty-driver_data; - if (!un || un-magic != DGAP_UNIT_MAGIC) - return -EFAULT; - - ch = un-un_ch; - if (!ch || ch-magic != DGAP_CHANNEL_MAGIC) - return -EFAULT; - - bd = ch-ch_bd; - if (!bd || bd-magic != DGAP_BOARD_MAGIC) - return -EFAULT; - - if (copy_from_user(new_rate, new_info, sizeof(unsigned int))) return -EFAULT; @@ -4040,7 +4021,7 @@ static int dgap_tty_ioctl(struct tty_struct *tty, unsigned int cmd, case DIGI_SETCUSTOMBAUD: spin_unlock_irqrestore(ch-ch_lock, lock_flags2); spin_unlock_irqrestore(bd-bd_lock, lock_flags); - return dgap_tty_digisetcustombaud(tty, uarg); + return dgap_tty_digisetcustombaud(ch, bd, un, uarg); case DIGI_RESET_PORT: dgap_firmware_reset_port(ch); -- 1.7.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 8/9] staging: dgap: removes redundant null check and change paramter for dgap_tty_digigetcustombaud()
Null checks in dgap_tty_digigetcustombaud() are already done by dgap_tty_ioctl() and change tty as a paramter of this function to ch and un. Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- drivers/staging/dgap/dgap.c | 23 +-- 1 files changed, 5 insertions(+), 18 deletions(-) diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c index 6bc4a30..3b76958 100644 --- a/drivers/staging/dgap/dgap.c +++ b/drivers/staging/dgap/dgap.c @@ -108,8 +108,8 @@ static int dgap_get_modem_info(struct channel_t *ch, unsigned int __user *value); static int dgap_tty_digisetcustombaud(struct tty_struct *tty, int __user *new_info); -static int dgap_tty_digigetcustombaud(struct tty_struct *tty, - int __user *retinfo); +static int dgap_tty_digigetcustombaud(struct channel_t *ch, struct un_t *un, + int __user *retinfo); static int dgap_tty_tiocmget(struct tty_struct *tty); static int dgap_tty_tiocmset(struct tty_struct *tty, unsigned int set, unsigned int clear); @@ -3318,28 +3318,15 @@ static int dgap_tty_digisetedelay(struct channel_t *ch, struct board_t *bd, * * Ioctl to get the current custom baud rate setting. */ -static int dgap_tty_digigetcustombaud(struct tty_struct *tty, - int __user *retinfo) +static int dgap_tty_digigetcustombaud(struct channel_t *ch, struct un_t *un, + int __user *retinfo) { - struct channel_t *ch; - struct un_t *un; int tmp; ulong lock_flags; if (!retinfo) return -EFAULT; - if (!tty || tty-magic != TTY_MAGIC) - return -EFAULT; - - un = tty-driver_data; - if (!un || un-magic != DGAP_UNIT_MAGIC) - return -EFAULT; - - ch = un-un_ch; - if (!ch || ch-magic != DGAP_CHANNEL_MAGIC) - return -EFAULT; - memset(tmp, 0, sizeof(tmp)); spin_lock_irqsave(ch-ch_lock, lock_flags); @@ -4048,7 +4035,7 @@ static int dgap_tty_ioctl(struct tty_struct *tty, unsigned int cmd, case DIGI_GETCUSTOMBAUD: spin_unlock_irqrestore(ch-ch_lock, lock_flags2); spin_unlock_irqrestore(bd-bd_lock, lock_flags); - return dgap_tty_digigetcustombaud(tty, uarg); + return dgap_tty_digigetcustombaud(ch, un, uarg); case DIGI_SETCUSTOMBAUD: spin_unlock_irqrestore(ch-ch_lock, lock_flags2); -- 1.7.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers/staging/comedi/drivers/ni_atmio16d.c: remove pointless condition
On 2014-07-11 11:13, Andrey Utkin wrote: The issue was discovered with static analysis and has two instances in this file. The code looks like this if (x 65536000) { ... } else if (x 65536) { ... } else if (x = 0x /* 655360 */) { ... } else if (x = 0x /* 6553600 */) { ... } The meaning of this block is to select appropriate clock frequency for interval timer basing on x, which is amount of time. Notes: 1. That last condition matches previous one - that's the issue. 2. Decimal numbers in comments don't match hex numbers in expressions. But in first case the numbers have same order, while in the second case the hex number is the same, and the decimal one is 10 times bigger. 3. Actually type of x is unsigned int, so its exact upper limit is not obviously known. 4. There's no else block. So it makes sense to make an else block from last else if case. The code inside the block seems correct for such usage. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=79871 Reported-by: David Binderman dcb...@hotmail.com Signed-off-by: Andrey Utkin andrey.krieger.ut...@gmail.com --- drivers/staging/comedi/drivers/ni_atmio16d.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/comedi/drivers/ni_atmio16d.c b/drivers/staging/comedi/drivers/ni_atmio16d.c index 6ad27f5..895b56d 100644 --- a/drivers/staging/comedi/drivers/ni_atmio16d.c +++ b/drivers/staging/comedi/drivers/ni_atmio16d.c @@ -338,7 +338,7 @@ static int atmio16d_ai_cmd(struct comedi_device *dev, } else if (cmd-convert_arg = 0x /* 655360 */) { base_clock = CLOCK_10_KHZ; timer = cmd-convert_arg / 10; - } else if (cmd-convert_arg = 0x /* 6553600 */) { + } else { base_clock = CLOCK_1_KHZ; timer = cmd-convert_arg / 100; } Since 0x is the maximum value 'cmd-convert_arg' can be, the final else can be moved to the 'base_clock = CLOCK_10_KHZ' block and the 'base_clock = CLOCK_1_KHZ' block can be removed altogether. @@ -406,7 +406,7 @@ static int atmio16d_ai_cmd(struct comedi_device *dev, } else if (cmd-scan_begin_arg 0x /* 655360 */) { base_clock = CLOCK_10_KHZ; timer = cmd-scan_begin_arg / 10; - } else if (cmd-scan_begin_arg 0x /* 6553600 */) { + } else { base_clock = CLOCK_1_KHZ; timer = cmd-scan_begin_arg / 100; } Same here as well for 'cmd-scan_begin_arg'. (And I think the original code should have used '= 0x' rather than ' 0x'.) -- -=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=- -=( 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] drivers/staging/comedi/drivers/ni_atmio16d.c: remove pointless condition
2014-07-11 15:01 GMT+03:00 Ian Abbott abbo...@mev.co.uk: On 2014-07-11 11:13, Andrey Utkin wrote: The issue was discovered with static analysis and has two instances in this file. The code looks like this if (x 65536000) { ... } else if (x 65536) { ... } else if (x = 0x /* 655360 */) { ... } else if (x = 0x /* 6553600 */) { ... } The meaning of this block is to select appropriate clock frequency for interval timer basing on x, which is amount of time. Notes: 1. That last condition matches previous one - that's the issue. 2. Decimal numbers in comments don't match hex numbers in expressions. But in first case the numbers have same order, while in the second case the hex number is the same, and the decimal one is 10 times bigger. 3. Actually type of x is unsigned int, so its exact upper limit is not obviously known. 4. There's no else block. So it makes sense to make an else block from last else if case. The code inside the block seems correct for such usage. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=79871 Reported-by: David Binderman dcb...@hotmail.com Signed-off-by: Andrey Utkin andrey.krieger.ut...@gmail.com --- drivers/staging/comedi/drivers/ni_atmio16d.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/comedi/drivers/ni_atmio16d.c b/drivers/staging/comedi/drivers/ni_atmio16d.c index 6ad27f5..895b56d 100644 --- a/drivers/staging/comedi/drivers/ni_atmio16d.c +++ b/drivers/staging/comedi/drivers/ni_atmio16d.c @@ -338,7 +338,7 @@ static int atmio16d_ai_cmd(struct comedi_device *dev, } else if (cmd-convert_arg = 0x /* 655360 */) { base_clock = CLOCK_10_KHZ; timer = cmd-convert_arg / 10; - } else if (cmd-convert_arg = 0x /* 6553600 */) { + } else { base_clock = CLOCK_1_KHZ; timer = cmd-convert_arg / 100; } Since 0x is the maximum value 'cmd-convert_arg' can be, Could you please substantiate this? I see that convert_arg has type unsigned int which may be 8 bytes on 64-bit platform. I haven't tracked where from actual values come, if the values are limited to 4 bytes, maybe we need to set the type to u32. the final else can be moved to the 'base_clock = CLOCK_10_KHZ' block and the 'base_clock = CLOCK_1_KHZ' block can be removed altogether. Feel free to prepare such patch, i'm not really keen on the subject. -- Andrey Utkin ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/8] Drivers: scsi: storvsc: Filter WRITE_SAME_16
hch == hch@infradead org h...@infradead.org writes: (Back from vacation: Bear with me while I'm catching up on two weeks of linux-scsi stuff...) hch I think the problem is a differnet one. If we have the logical hch provisioning EVPD it configures what method to use, but if we don't hch have one we simply check for a max unmap blocks field, and if hch that's not present use WRITE SAME. Yeah, that was done to accommodate the devices out there that predate the LBP VPD. There sadly are several still. And it's hard to motivate people to update their storage array firmware even when updates are readily available. hch The patch checks the no_write_same flag before doing that, for hch which we also have to do the write_same setup before the discard hch setup in sd_revalidate_disk. The no_write_same was introduced to disable the REQ_WRITE_SAME use case where we have no INQUIRY/READ CAPACITY/VPD flags to key off of to determine whether it is safe to send the commands. no_write_same does not preclude the REQ_DISCARD variants of WRITE_SAME. Those are gated by the discard heuristics exclusively. So first of all I'd like to know whether it's a WRITE SAME(16) or a WRITE SAME(16) with the UNMAP bit set that's coming down. hch Ky: does hyperv support UNMAP? If so any idea why it doesn't set hch the maximum unmap block count field in the EVPD? We shouldn't be sending down WRITE SAME(16) with the UNMAP bit set unless the device sets LBPME=1 in the READ CAPACITY(16) response. So what does the storsvc report as its thin provisioning capabilities? -- Martin K. Petersen Oracle Linux Engineering ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: unisys: added virtpci info entry
On Thu, 2014-07-10 at 07:51 -0700, Greg KH wrote: On Thu, Jul 10, 2014 at 10:34:14AM -0400, Erik Arfvidson wrote: This patch adds the virtpci debugfs directory and the info entry inside of it. Signed-off-by: Erik Arfvidson erik.arfvid...@unisys.com Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com 2/2? Where is patch 1/2? Please resend this correctly, and properly version your patches (this is what, version 10 or something?). Your subsystem maintainer has a short-term memory of a gnat, you need to be explicit otherwise I'll ignore it... greg k-h I mistakenly clicked reply instead of reply all the first time. :( Anyhow, I'm really sorry Greg, this patch was the second of a two part set and you'd already taken the first one, so I thought we should only resend the corrected patch and I told Erik to do that, so it's my fault. Should we send you this as a [1/1 version 4] or resend the entire set despite one patch already being in? -- Ben Romer | Software Engineer | Virtual Systems Development Unisys Corporation | 2476 Swedesford Rd | Malvern, PA 19355 | 610-648-7140 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers/staging/comedi/drivers/ni_atmio16d.c: remove pointless condition
On Fri, Jul 11, 2014 at 03:30:15PM +0300, Andrey Utkin wrote: 2014-07-11 15:01 GMT+03:00 Ian Abbott abbo...@mev.co.uk: On 2014-07-11 11:13, Andrey Utkin wrote: The issue was discovered with static analysis and has two instances in this file. The code looks like this if (x 65536000) { ... } else if (x 65536) { ... } else if (x = 0x /* 655360 */) { ... } else if (x = 0x /* 6553600 */) { ... } The meaning of this block is to select appropriate clock frequency for interval timer basing on x, which is amount of time. Notes: 1. That last condition matches previous one - that's the issue. 2. Decimal numbers in comments don't match hex numbers in expressions. But in first case the numbers have same order, while in the second case the hex number is the same, and the decimal one is 10 times bigger. 3. Actually type of x is unsigned int, so its exact upper limit is not obviously known. 4. There's no else block. So it makes sense to make an else block from last else if case. The code inside the block seems correct for such usage. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=79871 Reported-by: David Binderman dcb...@hotmail.com Signed-off-by: Andrey Utkin andrey.krieger.ut...@gmail.com --- drivers/staging/comedi/drivers/ni_atmio16d.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/comedi/drivers/ni_atmio16d.c b/drivers/staging/comedi/drivers/ni_atmio16d.c index 6ad27f5..895b56d 100644 --- a/drivers/staging/comedi/drivers/ni_atmio16d.c +++ b/drivers/staging/comedi/drivers/ni_atmio16d.c @@ -338,7 +338,7 @@ static int atmio16d_ai_cmd(struct comedi_device *dev, } else if (cmd-convert_arg = 0x /* 655360 */) { base_clock = CLOCK_10_KHZ; timer = cmd-convert_arg / 10; - } else if (cmd-convert_arg = 0x /* 6553600 */) { + } else { base_clock = CLOCK_1_KHZ; timer = cmd-convert_arg / 100; } Since 0x is the maximum value 'cmd-convert_arg' can be, Could you please substantiate this? I see that convert_arg has type unsigned int which may be 8 bytes on 64-bit platform. No. On linux unsigned int is always 32 bits. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 9/9] staging: dgap: removes redundant null check and change paramter for dgap_tty_digisetcustombaud()
Looks good to me. You should be CC'ing Mark as well. He's in MAINTAINERS now. I wish the two of you would CC each other and review each other's work. Daeseok, add yourself to the MAINTAINERS file for this driver as well so everyone CC's you on their dgap patches. You're doing good work. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: unisys: added virtpci info entry
On Fri, Jul 11, 2014 at 08:00:57AM -0500, Romer, Benjamin M wrote: On Thu, 2014-07-10 at 07:51 -0700, Greg KH wrote: On Thu, Jul 10, 2014 at 10:34:14AM -0400, Erik Arfvidson wrote: This patch adds the virtpci debugfs directory and the info entry inside of it. Signed-off-by: Erik Arfvidson erik.arfvid...@unisys.com Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com 2/2? Where is patch 1/2? Please resend this correctly, and properly version your patches (this is what, version 10 or something?). Your subsystem maintainer has a short-term memory of a gnat, you need to be explicit otherwise I'll ignore it... greg k-h I mistakenly clicked reply instead of reply all the first time. :( Anyhow, I'm really sorry Greg, this patch was the second of a two part set and you'd already taken the first one, so I thought we should only resend the corrected patch and I told Erik to do that, so it's my fault. Guys, stop over apologizing for trivial crap. Should we send you this as a [1/1 version 4] or resend the entire set despite one patch already being in? Don't resend patches which were already merged. Just say [PATCH v4] and then under the --- cut off put: Signed-off-by: you --- v4: This patch was part of a series which was merged earlier. It fixes blah blah blah issue from v3. v3: fix foo v2: fix bar regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: silicom: function return fixes
Some of these functions aren't called and can just be deleted. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers/staging/comedi/drivers/ni_atmio16d.c: remove pointless condition
Hi! On Fre, 2014-07-11 at 15:30 +0300, Andrey Utkin wrote: [...] Could you please substantiate this? I see that convert_arg has type unsigned int which may be 8 bytes on 64-bit platform. I haven't At least in the x86_64 world, unsigned int has 32bit. TTBOMK, it is similar on all other 64bit - otherwise there is no way to address 32bit (short int is usually 16 bit). Bernd -- I dislike type abstraction if it has no real reason. And saving on typing is not a good reason - if your typing speed is the main issue when you're coding, you're doing something seriously wrong. - Linus Torvalds ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] From: Andrey Utkin andrey.krieger.ut...@gmail.com
From: Andrey Utkin andrey.krieger.ut...@gmail.com drivers/staging/comedi/drivers/ni_atmio16d.c: remove pointless condition The issue was discovered with static analysis and has two instances in this file. The code looks like this if (x 65536000) { ... } else if (x 65536) { ... } else if (x = 0x /* 655360 */) { ... } else if (x = 0x /* 6553600 */) { ... } The meaning of this block is to select appropriate clock frequency for interval timer basing on x, which is amount of time. Notes: 1. That last condition matches previous one - that's the issue. 2. Decimal numbers in comments don't match hex numbers in expressions. But in first case the numbers have same order, while in the second case the hex number is the same, and the decimal one is 10 times bigger. 3. Actually type of x is unsigned int, so its exact upper limit is not obviously known. 4. There's no else block. So it makes sense to make an else block from last else if case. The code inside the block seems correct for such usage. [ Actually, get rid of the final else if case and change the next-to-last else if case to an else as the upper limit of x _is_ known to be 0x (UINT_MAX), which is less than 655360 -- Ian ] Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=79871 Reported-by: David Binderman dcb...@hotmail.com Signed-off-by: Andrey Utkin andrey.krieger.ut...@gmail.com Signed-off-by: Ian Abbott abbo...@mev.co.uk --- v2: Removed final else if block and changed preceding else if block to else block as the condition is always true due to limited range of unsigned int. -- Ian --- drivers/staging/comedi/drivers/ni_atmio16d.c | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/staging/comedi/drivers/ni_atmio16d.c b/drivers/staging/comedi/drivers/ni_atmio16d.c index 6ad27f5..b1b4744 100644 --- a/drivers/staging/comedi/drivers/ni_atmio16d.c +++ b/drivers/staging/comedi/drivers/ni_atmio16d.c @@ -335,12 +335,9 @@ static int atmio16d_ai_cmd(struct comedi_device *dev, } else if (cmd-convert_arg 65536) { base_clock = CLOCK_100_KHZ; timer = cmd-convert_arg / 1; - } else if (cmd-convert_arg = 0x /* 655360 */) { + } else /* cmd-convert_arg 655360 */ { base_clock = CLOCK_10_KHZ; timer = cmd-convert_arg / 10; - } else if (cmd-convert_arg = 0x /* 6553600 */) { - base_clock = CLOCK_1_KHZ; - timer = cmd-convert_arg / 100; } outw(0xFF03, dev-iobase + AM9513A_COM_REG); outw(base_clock, dev-iobase + AM9513A_DATA_REG); @@ -403,12 +400,9 @@ static int atmio16d_ai_cmd(struct comedi_device *dev, } else if (cmd-scan_begin_arg 65536) { base_clock = CLOCK_100_KHZ; timer = cmd-scan_begin_arg / 1; - } else if (cmd-scan_begin_arg 0x /* 655360 */) { + } else /* cmd-scan_begin_arg 655360 */ { base_clock = CLOCK_10_KHZ; timer = cmd-scan_begin_arg / 10; - } else if (cmd-scan_begin_arg 0x /* 6553600 */) { - base_clock = CLOCK_1_KHZ; - timer = cmd-scan_begin_arg / 100; } outw(0xFF02, dev-iobase + AM9513A_COM_REG); outw(base_clock, dev-iobase + AM9513A_DATA_REG); -- 2.0.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 04/12] staging: ced1401: ced_ioc.c fix checkpatch warning
You have a lot of patches with the exact same subject. Use different subjects like: [patch] staging: ced1401: ced_ioc.c: change spaces to tabs or whatever. regards, dan carpeneter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] From: Andrey Utkin andrey.krieger.ut...@gmail.com
On 2014-07-11 15:32, Ian Abbott wrote: From: Andrey Utkin andrey.krieger.ut...@gmail.com drivers/staging/comedi/drivers/ni_atmio16d.c: remove pointless condition Sorry, I messed up the subject line. Let me try that again! -- -=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=- -=( 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
[PATCH v3] drivers/staging/comedi/drivers/ni_atmio16d.c: remove pointless condition
From: Andrey Utkin andrey.krieger.ut...@gmail.com From: Andrey Utkin andrey.krieger.ut...@gmail.com The issue was discovered with static analysis and has two instances in this file. The code looks like this if (x 65536000) { ... } else if (x 65536) { ... } else if (x = 0x /* 655360 */) { ... } else if (x = 0x /* 6553600 */) { ... } The meaning of this block is to select appropriate clock frequency for interval timer basing on x, which is amount of time. Notes: 1. That last condition matches previous one - that's the issue. 2. Decimal numbers in comments don't match hex numbers in expressions. But in first case the numbers have same order, while in the second case the hex number is the same, and the decimal one is 10 times bigger. 3. Actually type of x is unsigned int, so its exact upper limit is not obviously known. 4. There's no else block. So it makes sense to make an else block from last else if case. The code inside the block seems correct for such usage. [ Actually, get rid of the final else if case and change the next-to-last else if case to an else as the upper limit of x _is_ known to be 0x (UINT_MAX), which is less than 655360 -- Ian ] Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=79871 Reported-by: David Binderman dcb...@hotmail.com Signed-off-by: Andrey Utkin andrey.krieger.ut...@gmail.com Signed-off-by: Ian Abbott abbo...@mev.co.uk --- v2: Removed final else if block and changed preceding else if block to else block as the condition is always true due to limited range of unsigned int. -- Ian v3: Corrected subject line I messed up in v2. -- Ian --- drivers/staging/comedi/drivers/ni_atmio16d.c | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/staging/comedi/drivers/ni_atmio16d.c b/drivers/staging/comedi/drivers/ni_atmio16d.c index 6ad27f5..b1b4744 100644 --- a/drivers/staging/comedi/drivers/ni_atmio16d.c +++ b/drivers/staging/comedi/drivers/ni_atmio16d.c @@ -335,12 +335,9 @@ static int atmio16d_ai_cmd(struct comedi_device *dev, } else if (cmd-convert_arg 65536) { base_clock = CLOCK_100_KHZ; timer = cmd-convert_arg / 1; - } else if (cmd-convert_arg = 0x /* 655360 */) { + } else /* cmd-convert_arg 655360 */ { base_clock = CLOCK_10_KHZ; timer = cmd-convert_arg / 10; - } else if (cmd-convert_arg = 0x /* 6553600 */) { - base_clock = CLOCK_1_KHZ; - timer = cmd-convert_arg / 100; } outw(0xFF03, dev-iobase + AM9513A_COM_REG); outw(base_clock, dev-iobase + AM9513A_DATA_REG); @@ -403,12 +400,9 @@ static int atmio16d_ai_cmd(struct comedi_device *dev, } else if (cmd-scan_begin_arg 65536) { base_clock = CLOCK_100_KHZ; timer = cmd-scan_begin_arg / 1; - } else if (cmd-scan_begin_arg 0x /* 655360 */) { + } else /* cmd-scan_begin_arg 655360 */ { base_clock = CLOCK_10_KHZ; timer = cmd-scan_begin_arg / 10; - } else if (cmd-scan_begin_arg 0x /* 6553600 */) { - base_clock = CLOCK_1_KHZ; - timer = cmd-scan_begin_arg / 100; } outw(0xFF02, dev-iobase + AM9513A_COM_REG); outw(base_clock, dev-iobase + AM9513A_DATA_REG); -- 2.0.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] drivers/staging/comedi/drivers/ni_atmio16d.c: remove pointless condition
On 2014-07-11 15:38, Ian Abbott wrote: From: Andrey Utkin andrey.krieger.ut...@gmail.com From: Andrey Utkin andrey.krieger.ut...@gmail.com Dammit! Greg, do you want to sort that out or should I have another go? -- -=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=- -=( 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 v3 04/12] staging: ced1401: ced_ioc.c fix checkpatch warning
Il 11/07/2014 16:17, Dan Carpenter ha scritto: You have a lot of patches with the exact same subject. Use different subjects like: [patch] staging: ced1401: ced_ioc.c: change spaces to tabs or whatever. regards, dan carpeneter Hi Dan, OK, I'll resend regards Luca Ellero ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 06/12] staging: ced1401: ced_ioc.c: remove else branch in ced_wait_event
Fix checkpatch warning else is not generally useful after a break or return in file ced_ioc.c, function ced_wait_event() Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/ced_ioc.c | 65 +++-- 1 file changed, 33 insertions(+), 32 deletions(-) diff --git a/drivers/staging/ced1401/ced_ioc.c b/drivers/staging/ced1401/ced_ioc.c index cb075af..0a40246 100644 --- a/drivers/staging/ced1401/ced_ioc.c +++ b/drivers/staging/ced1401/ced_ioc.c @@ -887,48 +887,49 @@ int ced_set_event(struct ced_data *ced, struct transfer_event __user *ute) int ced_wait_event(struct ced_data *ced, int area, int time_out) { int ret; + int wait; + struct transarea *ta; if ((unsigned)area = MAX_TRANSAREAS) return U14ERR_BADAREA; - else { - int wait; - struct transarea *ta = ced-trans_def[area]; -/* convert timeout to jiffies */ - time_out = (time_out * HZ + 999) / 1000; + ta = ced-trans_def[area]; - /* We cannot wait holding the mutex, but we check the flags */ - /* while holding it. This may well be pointless as another */ - /* thread could get in between releasing it and the wait */ - /* call. However, this would have to clear the wake_up flag. */ - /* However, the !ta-used may help us in this case. */ +/* convert timeout to jiffies */ + time_out = (time_out * HZ + 999) / 1000; - /* make sure we have no competitor */ - mutex_lock(ced-io_mutex); - if (!ta-used || !ta-event_sz) /* check something to */ - /* wait for...*/ - return U14ERR_NOTSET; /* ...else we do nothing */ - mutex_unlock(ced-io_mutex); + /* We cannot wait holding the mutex, but we check the flags */ + /* while holding it. This may well be pointless as another */ + /* thread could get in between releasing it and the wait */ + /* call. However, this would have to clear the wake_up flag. */ + /* However, the !ta-used may help us in this case. */ - if (time_out) - wait = wait_event_interruptible_timeout(ta-event, - ta-wake_up || - !ta-used, - time_out); - else - wait = wait_event_interruptible(ta-event, + /* make sure we have no competitor */ + mutex_lock(ced-io_mutex); + if (!ta-used || !ta-event_sz) /* check something to */ + /* wait for...*/ + return U14ERR_NOTSET; /* ...else we do nothing */ + mutex_unlock(ced-io_mutex); + + if (time_out) + wait = wait_event_interruptible_timeout(ta-event, ta-wake_up || - !ta-used); + !ta-used, + time_out); + else + wait = wait_event_interruptible(ta-event, + ta-wake_up || + !ta-used); + + if (wait) + ret = -ERESTARTSYS; /* oops - we have had a SIGNAL */ + else + ret = ta-wake_up; /* else the wakeup count */ - if (wait) - ret = -ERESTARTSYS; /* oops - we have had a SIGNAL */ - else - ret = ta-wake_up; /* else the wakeup count */ + spin_lock_irq(ced-staged_lock); + ta-wake_up = 0;/* clear the flag */ + spin_unlock_irq(ced-staged_lock); - spin_lock_irq(ced-staged_lock); - ta-wake_up = 0;/* clear the flag */ - spin_unlock_irq(ced-staged_lock); - } return ret; } -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 07/12] staging: ced1401: ced_ioc.c : split long lines
Fix checkpatch warnings line over 80 characters in file ced_ioc.c Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/ced_ioc.c | 66 +++-- 1 file changed, 42 insertions(+), 24 deletions(-) diff --git a/drivers/staging/ced1401/ced_ioc.c b/drivers/staging/ced1401/ced_ioc.c index 0a40246..8cb6ea9 100644 --- a/drivers/staging/ced1401/ced_ioc.c +++ b/drivers/staging/ced1401/ced_ioc.c @@ -63,8 +63,10 @@ static void ced_flush_in_buff(struct ced_data *ced) { dev_dbg(ced-interface-dev, %s: current_state=%d\n, __func__, ced-current_state); - if (ced-current_state == U14ERR_TIME) /* Do nothing if hardware in trouble */ - return; + + if (ced-current_state == U14ERR_TIME) + return; /* Do nothing if hardware in trouble */ + /* Kill off any pending I/O */ /* CharRead_Cancel(pDevObject); */ spin_lock_irq(ced-char_in_lock); @@ -96,7 +98,9 @@ static int ced_put_chars(struct ced_data *ced, const char *ch, } ced-num_output += count; spin_unlock_irq(ced-char_out_lock); - ret = ced_send_chars(ced); /* ...give a chance to transmit data */ + + /* ...give a chance to transmit data */ + ret = ced_send_chars(ced); } else { ret = U14ERR_NOOUT; /* no room at the out (ha-ha) */ spin_unlock_irq(ced-char_out_lock); @@ -238,24 +242,31 @@ int ced_read_write_cancel(struct ced_data *ced) int ntStatus = STATUS_SUCCESS; bool bResult = false; unsigned int i; - /* We can fill this in when we know how we will implement the staged transfer stuff */ + + /* We can fill this in when we know how we will implement the staged */ + /* transfer stuff*/ spin_lock_irq(ced-staged_lock); - if (ced-staged_urb_pending) { /* anything to be cancelled? May need more... */ + if (ced-staged_urb_pending) {/* anything to be cancelled? */ + /* May need more... */ dev_info(ced-interface - dev, ced_read_write_cancel about to cancel Urb\n); /* Clear the staging done flag */ /* KeClearEvent(ced-StagingDoneEvent); */ USB_ASSERT(ced-pStagedIrp != NULL); - /* Release the spinlock first otherwise the completion routine may hang */ - /* on the spinlock while this function hands waiting for the event. */ + /* Release the spinlock first otherwise the completion */ + /* routine may hang on the spinlock while this function */ + /* hands waiting for the event. */ spin_unlock_irq(ced-staged_lock); - bResult = IoCancelIrp(ced-pStagedIrp); /* Actually do the cancel */ + + /* Actually do the cancel */ + bResult = IoCancelIrp(ced-pStagedIrp); if (bResult) { LARGE_INTEGER timeout; - timeout.QuadPart = -1000; /* Use a timeout of 1 second */ + /* Use a timeout of 1 second */ + timeout.QuadPart = -1000; dev_info(ced-interface - dev, %s: about to wait till done\n, __func__); ntStatus = @@ -268,8 +279,8 @@ int ced_read_write_cancel(struct ced_data *ced) ntStatus = U14ERR_FAIL; } USB_KdPrint(DBGLVL_DEFAULT, - (ced_read_write_cancel ntStatus = 0x%x decimal %d\n, -ntStatus, ntStatus)); + (ced_read_write_cancel ntStatus = 0x%x decimal %d\n, + ntStatus, ntStatus)); } else spin_unlock_irq(ced-staged_lock); @@ -389,7 +400,8 @@ static bool ced_quick_check(struct ced_data *ced, bool test_buff, short_test = ((ced-dma_flag == MODE_CHAR)/* no DMA running */ (!ced-force_reset) /* Not had a real reset forced */ - (ced-current_state = U14ERR_STD)); /* No 1401 errors stored */ + /* No 1401 errors stored */ + (ced-current_state = U14ERR_STD)); dev_dbg(ced-interface-dev, %s: DMAFlag:%d, state:%d, force:%d, testBuff:%d, short:%d\n, @@ -397,19 +409,24 @@ static bool ced_quick_check(struct ced_data *ced, bool test_buff, test_buff, short_test); if ((test_buff) /* Buffer check requested, and... */ - (ced-num_input || ced-num_output)) { /* ...characters were in the buffer? */ + (ced-num_input || ced-num_output)) {/* ...characters were in */ +
[PATCH v4 01/12] staging: ced1401: usb1401.c: change spaces to tabs
Fix checkpatch errors code indent should use tabs where possible in file usb1401.c Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/usb1401.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/ced1401/usb1401.c b/drivers/staging/ced1401/usb1401.c index 26e5f1b..a3d1ed6 100644 --- a/drivers/staging/ced1401/usb1401.c +++ b/drivers/staging/ced1401/usb1401.c @@ -610,9 +610,9 @@ static void staged_callback(struct urb *urb) ta-blocks[1].offset); } } else { /* If block 1 is not used, we try to add */ -/*to block 0 */ +/*to block 0 */ - /* Got stored block 0 information? */ + /* Got stored block 0 information? */ if (ta-blocks[0].size 0) { /* Must append onto the */ /*existing block 0 */ @@ -738,7 +738,7 @@ static void staged_callback(struct urb *urb) /* This feels wrong as we should ask which spin lock protects*/ /* dma_flag. */ restart_char_input = !cancel (ced-dma_flag == MODE_CHAR) - !ced-xfer_waiting; +!ced-xfer_waiting; spin_unlock(ced-staged_lock); /* Finally release the lock again */ @@ -803,7 +803,7 @@ static int ced_stage_chunk(struct ced_data *ced) ced-pipe_error[pipe] = 1; /* Flag an error to be */ /* handled later */ dev_err(ced-interface-dev, - %s: submit urb failed, code %d\n, + %s: submit urb failed, code %d\n, __func__, retval); } else /* Set the flag for staged URB pending */ -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 11/12] staging: ced1401: usb1401.c: split long lines
Fix checkpatch warnings line over 80 characters in file usb1401.c Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/usb1401.c | 168 +++-- 1 file changed, 105 insertions(+), 63 deletions(-) diff --git a/drivers/staging/ced1401/usb1401.c b/drivers/staging/ced1401/usb1401.c index 6d0b013..907b6cf 100644 --- a/drivers/staging/ced1401/usb1401.c +++ b/drivers/staging/ced1401/usb1401.c @@ -1,11 +1,12 @@ -/*** - CED1401 usb driver. This basic loading is based on the usb-skeleton.c code that is: +/*** + CED1401 usb driver. This basic loading is based on the usb-skeleton.c code that + is: Copyright (C) 2001-2004 Greg Kroah-Hartman (g...@kroah.com) Copyright (C) 2012 Alois Schloegl alois.schlo...@ist.ac.at There is not a great deal of the skeleton left. - All the remainder dealing specifically with the CED1401 is based on drivers written - by CED for other systems (mainly Windows) and is: + All the remainder dealing specifically with the CED1401 is based on drivers + written by CED for other systems (mainly Windows) and is: Copyright (C) 2010 Cambridge Electronic Design Ltd Author Greg P Smith (g...@ced.co.uk) @@ -125,8 +126,9 @@ static void ced_delete(struct kref *kref) { struct ced_data *ced = to_ced_data(kref); - /* Free up the output buffer, then free the output urb. Note that the interface member */ - /* of ced will probably be NULL, so cannot be used to get to dev. */ + /* Free up the output buffer, then free the output urb. Note that the */ + /* interface member of ced will probably be NULL, so cannot be used */ + /* to get to dev. */ usb_free_coherent(ced-udev, OUTBUF_SZ, ced-coher_char_out, ced-urb_char_out-transfer_dma); usb_free_urb(ced-urb_char_out); @@ -183,7 +185,7 @@ static int ced_open(struct inode *inode, struct file *file) kref_put(ced-kref, ced_delete); goto exit; } - } else {/* uncomment this block if you want exclusive open */ + } else { /* uncomment this block if you want exclusive open */ dev_err(interface-dev, %s: fail: already open\n, __func__); retval = -EBUSY; ced-open_count--; @@ -214,7 +216,8 @@ static int ced_release(struct inode *inode, struct file *file) usb_autopm_put_interface(ced-interface); mutex_unlock(ced-io_mutex); - kref_put(ced-kref, ced_delete); /* decrement the count on our device */ + /* decrement the count on our device */ + kref_put(ced-kref, ced_delete); return 0; } @@ -296,7 +299,9 @@ static void ced_writechar_callback(struct urb *urb) spin_lock(ced-char_out_lock); /* already at irq level */ ced-num_output -= got; /* Now adjust the char send buffer */ ced-out_buff_get += got; /* to match what we did */ - if (ced-out_buff_get = OUTBUF_SZ) /* Can't do this any earlier as data could be overwritten */ + + /* Can't do this any earlier as data could be overwritten */ + if (ced-out_buff_get = OUTBUF_SZ) ced-out_buff_get = 0; if (ced-num_output 0) { /* if more to be done... */ @@ -306,13 +311,15 @@ static void ced_writechar_callback(struct urb *urb) /* maximum to send */ unsigned int count = ced-num_output; - if ((ced-out_buff_get + count) OUTBUF_SZ)/* does it cross buffer end? */ + /* does it cross buffer end? */ + if ((ced-out_buff_get + count) OUTBUF_SZ) count = OUTBUF_SZ - ced-out_buff_get; /* we are done with stuff that changes */ spin_unlock(ced-char_out_lock); - memcpy(ced-coher_char_out, pDat, count); /* copy output data to the buffer */ + /* copy output data to the buffer */ + memcpy(ced-coher_char_out, pDat, count); usb_fill_bulk_urb(ced-urb_char_out, ced-udev, usb_sndbulkpipe(ced-udev, ced-ep_addr[0]), @@ -320,14 +327,21 @@ static void ced_writechar_callback(struct urb *urb) ced_writechar_callback, ced); ced-urb_char_out-transfer_flags |= URB_NO_TRANSFER_DMA_MAP; - usb_anchor_urb(ced-urb_char_out, ced-submitted); /* in case we need to kill it */ + +
[PATCH v4 05/12] staging: ced1401: ced_ioc.c: remove else branch in ced_set_event
Fix checkpatch warning else is not generally useful after a break or return in file ced_ioc.c, function ced_set_event() Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/ced_ioc.c | 36 +++- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/drivers/staging/ced1401/ced_ioc.c b/drivers/staging/ced1401/ced_ioc.c index a4bd013..cb075af 100644 --- a/drivers/staging/ced1401/ced_ioc.c +++ b/drivers/staging/ced1401/ced_ioc.c @@ -844,6 +844,7 @@ int ced_set_event(struct ced_data *ced, struct transfer_event __user *ute) { int ret = U14ERR_NOERROR; struct transfer_event te; + struct transarea *ta; /* get a local copy of the data */ if (copy_from_user(te, ute, sizeof(te))) @@ -851,27 +852,28 @@ int ced_set_event(struct ced_data *ced, struct transfer_event __user *ute) if (te.wAreaNum = MAX_TRANSAREAS) /* the area must exist */ return U14ERR_BADAREA; - else { - struct transarea *ta = ced-trans_def[te.wAreaNum]; - /* make sure we have no competitor */ - mutex_lock(ced-io_mutex); - spin_lock_irq(ced-staged_lock); + ta = ced-trans_def[te.wAreaNum]; - if (ta-used) { /* area must be in use */ - ta-event_st = te.dwStart; /* set area regions */ + /* make sure we have no competitor */ + mutex_lock(ced-io_mutex); + spin_lock_irq(ced-staged_lock); -/* set size (0 cancels it) */ - ta-event_sz = te.dwLength; + if (ta-used) { /* area must be in use */ + ta-event_st = te.dwStart; /* set area regions */ + +/* set size (0 cancels it) */ + ta-event_sz = te.dwLength; + +/* set the direction */ + ta-event_to_host = te.wFlags 1; + ta-wake_up = 0;/* zero the wake up count */ + } else + ret = U14ERR_NOTSET; + + spin_unlock_irq(ced-staged_lock); + mutex_unlock(ced-io_mutex); -/* set the direction */ - ta-event_to_host = te.wFlags 1; - ta-wake_up = 0;/* zero the wake up count */ - } else - ret = U14ERR_NOTSET; - spin_unlock_irq(ced-staged_lock); - mutex_unlock(ced-io_mutex); - } return ret == U14ERR_NOERROR ? (te.iSetEvent ? 1 : U14ERR_NOERROR) : ret; } -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 10/12] staging: ced1401: usb1401.c rename camel case variable
Rename camel case variable dwCount in function ced_writechar_callback Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/usb1401.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/staging/ced1401/usb1401.c b/drivers/staging/ced1401/usb1401.c index 9dce6cc..6d0b013 100644 --- a/drivers/staging/ced1401/usb1401.c +++ b/drivers/staging/ced1401/usb1401.c @@ -303,26 +303,27 @@ static void ced_writechar_callback(struct urb *urb) int pipe = 0; /* The pipe number to use */ int ret; char *pDat = ced-output_buffer[ced-out_buff_get]; - unsigned int dwCount = ced-num_output; /* maximum to send */ + /* maximum to send */ + unsigned int count = ced-num_output; - if ((ced-out_buff_get + dwCount) OUTBUF_SZ) /* does it cross buffer end? */ - dwCount = OUTBUF_SZ - ced-out_buff_get; + if ((ced-out_buff_get + count) OUTBUF_SZ)/* does it cross buffer end? */ + count = OUTBUF_SZ - ced-out_buff_get; /* we are done with stuff that changes */ spin_unlock(ced-char_out_lock); - memcpy(ced-coher_char_out, pDat, dwCount); /* copy output data to the buffer */ + memcpy(ced-coher_char_out, pDat, count); /* copy output data to the buffer */ usb_fill_bulk_urb(ced-urb_char_out, ced-udev, usb_sndbulkpipe(ced-udev, ced-ep_addr[0]), - ced-coher_char_out, dwCount, + ced-coher_char_out, count, ced_writechar_callback, ced); ced-urb_char_out-transfer_flags |= URB_NO_TRANSFER_DMA_MAP; usb_anchor_urb(ced-urb_char_out, ced-submitted); /* in case we need to kill it */ ret = usb_submit_urb(ced-urb_char_out, GFP_ATOMIC); dev_dbg(ced-interface-dev, %s: n=%d%s\n, - __func__, dwCount, pDat); + __func__, count, pDat); spin_lock(ced-char_out_lock); /* grab lock for errors */ if (ret) { ced-pipe_error[pipe] = 1; /* Flag an error to be handled later */ -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 09/12] staging: ced1401: usb1401.c: fix code indent
Fix checkpatch warning suspect code indent for conditional statements in file usb1401.c Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/usb1401.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/ced1401/usb1401.c b/drivers/staging/ced1401/usb1401.c index eab5f04..9dce6cc 100644 --- a/drivers/staging/ced1401/usb1401.c +++ b/drivers/staging/ced1401/usb1401.c @@ -1256,7 +1256,8 @@ static void ced_readchar_callback(struct urb *urb) } if ((ced-num_input + got) = INBUF_SZ) - /* Adjust the buffer count accordingly */ + /* Adjust the buffer count */ + /* accordingly */ ced-num_input += got; } else dev_dbg(ced-interface-dev, %s: read ZLP\n, -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 08/12] staging: ced1401: usb1401.c: add blank line after declarations
Fix checkpatch warnings Missing a blank line after declarations in file usb1401.c Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/usb1401.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/staging/ced1401/usb1401.c b/drivers/staging/ced1401/usb1401.c index a3d1ed6..eab5f04 100644 --- a/drivers/staging/ced1401/usb1401.c +++ b/drivers/staging/ced1401/usb1401.c @@ -204,6 +204,7 @@ exit: static int ced_release(struct inode *inode, struct file *file) { struct ced_data *ced = file-private_data; + if (ced == NULL) return -ENODEV; @@ -221,6 +222,7 @@ static int ced_flush(struct file *file, fl_owner_t id) { int res; struct ced_data *ced = file-private_data; + if (ced == NULL) return -ENODEV; @@ -302,6 +304,7 @@ static void ced_writechar_callback(struct urb *urb) int ret; char *pDat = ced-output_buffer[ced-out_buff_get]; unsigned int dwCount = ced-num_output; /* maximum to send */ + if ((ced-out_buff_get + dwCount) OUTBUF_SZ) /* does it cross buffer end? */ dwCount = OUTBUF_SZ - ced-out_buff_get; @@ -449,12 +452,14 @@ int ced_send_chars(struct ced_data *ced) static void ced_copy_user_space(struct ced_data *ced, int n) { unsigned int area = ced-staged_id; + if (area MAX_TRANSAREAS) { /* area to be used */ struct transarea *ta = ced-trans_def[area]; unsigned int offset = ced-staged_done + ced-staged_offset + ta-base_offset; char *coher_buf = ced-coher_staged_io; /* coherent buffer */ + if (!ta-used) { dev_err(ced-interface-dev, %s: area %d unused\n, __func__, area); @@ -711,6 +716,7 @@ static void staged_callback(struct urb *urb) /* If we have a transfer waiting, kick it off */ if (ced-xfer_waiting) {/* Got a block xfer waiting? */ int retval; + dev_info(ced-interface-dev, *** RWM_Complete *** pending transfer will now be set up!!!\n); @@ -1231,6 +1237,7 @@ static void ced_readchar_callback(struct urb *urb) if (got 0) { unsigned int i; + if (got INBUF_SZ) { /* tidy the string */ ced-coher_char_in[got] = 0; @@ -1339,6 +1346,7 @@ static long ced_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { int err = 0; struct ced_data *ced = file-private_data; + if (!can_accept_io_requests(ced)) /* check we still exist */ return -ENODEV; @@ -1629,6 +1637,7 @@ static void ced_disconnect(struct usb_interface *interface) ced_draw_down(ced); /* ...wait for then kill any io */ for (i = 0; i MAX_TRANSAREAS; ++i) { int err = ced_clear_area(ced, i); /* ...release any used memory */ + if (err == U14ERR_UNLOCKFAIL) dev_err(ced-interface-dev, %s: Area %d was in used\n, __func__, i); @@ -1650,6 +1659,7 @@ static void ced_disconnect(struct usb_interface *interface) void ced_draw_down(struct ced_data *ced) { int time; + dev_dbg(ced-interface-dev, %s: called\n, __func__); ced-in_draw_down = true; @@ -1664,6 +1674,7 @@ void ced_draw_down(struct ced_data *ced) static int ced_suspend(struct usb_interface *intf, pm_message_t message) { struct ced_data *ced = usb_get_intfdata(intf); + if (!ced) return 0; ced_draw_down(ced); @@ -1675,6 +1686,7 @@ static int ced_suspend(struct usb_interface *intf, pm_message_t message) static int ced_resume(struct usb_interface *intf) { struct ced_data *ced = usb_get_intfdata(intf); + if (!ced) return 0; dev_dbg(ced-interface-dev, %s: called\n, __func__); @@ -1684,6 +1696,7 @@ static int ced_resume(struct usb_interface *intf) static int ced_pre_reset(struct usb_interface *intf) { struct ced_data *ced = usb_get_intfdata(intf); + dev_dbg(ced-interface-dev, %s\n, __func__); mutex_lock(ced-io_mutex); ced_draw_down(ced); @@ -1693,6 +1706,7 @@ static int ced_pre_reset(struct usb_interface *intf) static int ced_post_reset(struct usb_interface *intf) { struct ced_data *ced = usb_get_intfdata(intf); + dev_dbg(ced-interface-dev, %s\n, __func__); /* we are sure no URBs are active - no locking needed */ -- 1.7.10.4 ___
[PATCH v4 12/12] staging: ced1401: usb1401.c: join quoted strings
Fix checkpatch warnings quoted string split across lines in file usb1401.c Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/usb1401.c | 23 +++ 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/drivers/staging/ced1401/usb1401.c b/drivers/staging/ced1401/usb1401.c index 907b6cf..9988677 100644 --- a/drivers/staging/ced1401/usb1401.c +++ b/drivers/staging/ced1401/usb1401.c @@ -627,8 +627,7 @@ static void staged_callback(struct urb *urb) ta-blocks[1].size += ced-staged_length; dev_dbg(ced-interface-dev, - RWM_Complete, circ block 1 - now %d bytes at %d\n, + RWM_Complete, circ block 1 now %d bytes at %d\n, ta-blocks[1].size, ta-blocks[1].offset); } else { @@ -640,8 +639,7 @@ static void staged_callback(struct urb *urb) ta-blocks[1].size = ced-staged_length; dev_err(ced-interface-dev, - %s: ERROR, circ block 1 - re-started %d bytes at %d\n, + %s: ERROR, circ block 1 re-started %d bytes at %d\n, __func__, ta-blocks[1].size, ta-blocks[1].offset); @@ -660,9 +658,7 @@ static void staged_callback(struct urb *urb) ta-blocks[0].size += ced-staged_length; dev_dbg(ced-interface-dev, - RWM_Complete, circ - block 0 now %d bytes - at %d\n, + RWM_Complete, circ block 0 now %d bytes at %d\n, ta-blocks[0].size, ta-blocks[0].offset); @@ -673,9 +669,7 @@ static void staged_callback(struct urb *urb) ta-blocks[1].size = ced-staged_length; dev_dbg(ced-interface-dev, - RWM_Complete, circ - block 1 started %d - bytes at %d\n, + RWM_Complete, circ block 1 started %d bytes at %d\n, ta-blocks[1].size, ta-blocks[1].offset); } @@ -686,8 +680,7 @@ static void staged_callback(struct urb *urb) ta-blocks[0].size = ced-staged_length; dev_dbg(ced-interface-dev, - RWM_Complete, circ block 0 - started %d bytes at %d\n, + RWM_Complete, circ block 0 started %d bytes at %d\n, ta-blocks[0].size, ta-blocks[0].offset); } @@ -696,8 +689,7 @@ static void staged_callback(struct urb *urb) if (!cancel) { /* Don't generate an event if cancelled */ dev_dbg(ced-interface-dev, - RWM_Complete, bCircular %d, bToHost %d, - eStart %d, eSize %d\n, + RWM_Complete, bCircular %d, bToHost %d, eStart %d, eSize %d\n, ta-circular, ta-event_to_host, ta-event_st, ta-event_sz); /* Set a user-mode event... */ @@ -752,8 +744,7 @@ static void staged_callback(struct urb *urb) int retval; dev_info(ced-interface-dev, -*** RWM_Complete *** pending transfer - will now be set
[PATCH v4 04/12] staging: ced1401: ced_ioc.c: remove space before tabs
Fix checkpatch warning please, no space before tabs in file ced_ioc.c Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/ced_ioc.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/ced1401/ced_ioc.c b/drivers/staging/ced1401/ced_ioc.c index 397df43..a4bd013 100644 --- a/drivers/staging/ced1401/ced_ioc.c +++ b/drivers/staging/ced1401/ced_ioc.c @@ -344,7 +344,7 @@ static bool ced_is_1401(struct ced_data *ced) unsigned int state; ret = ced_in_self_test(ced, state); /* see if likely in */ -/* self test*/ +/* self test*/ if (ret 0) { /* do we need to wait for self-test? */ /* when to give up */ unsigned long timeout = jiffies + 30 * HZ; -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 00/12] staging: ced1401: rename camel case function and vars
This patchset renames all camel case and Hungarian notation names in ced1401. It fixes as well some checkpatch warnings and errors. I've not modified any logic in the driver. Please note that I've tryed to not add any checkpatch warning/error. If some patches give warnings or errors they were present before modification. Anyway I tryed to fix the most of them in the last 5 patches. Changes for v2: reworking of patches 54 to 115 which didn't apply properly onto linux-next. First 53 patches are already applied to linux-next. For simplification some patches are also melded together. Changes for v3: reworking of patches 111 to 115 (57-60 in v2): - split them on multiple patches - add more verbose comment Changes for v4: change subjects of patches to more specific ones Luca Ellero (12): staging: ced1401: usb1401.c: change spaces to tabs staging: ced1401: usb1401.h: fix foo * bar staging: ced1401: ced_ioc.c: add blank line after declarations staging: ced1401: ced_ioc.c: remove space before tabs staging: ced1401: ced_ioc.c: remove else branch in ced_set_event staging: ced1401: ced_ioc.c: remove else branch in ced_wait_event staging: ced1401: ced_ioc.c : split long lines staging: ced1401: usb1401.c: add blank line after declarations staging: ced1401: usb1401.c: fix code indent staging: ced1401: usb1401.c rename camel case variable staging: ced1401: usb1401.c: split long lines staging: ced1401: usb1401.c: join quoted strings drivers/staging/ced1401/ced_ioc.c | 195 drivers/staging/ced1401/usb1401.c | 225 ++--- drivers/staging/ced1401/usb1401.h |2 +- 3 files changed, 259 insertions(+), 163 deletions(-) -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 03/12] staging: ced1401: ced_ioc.c: add blank line after declarations
Fix checkpatch warnings Missing a blank line after declarations in file ced_ioc.c Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/ced_ioc.c | 26 ++ 1 file changed, 26 insertions(+) diff --git a/drivers/staging/ced1401/ced_ioc.c b/drivers/staging/ced1401/ced_ioc.c index fb4e8a3..397df43 100644 --- a/drivers/staging/ced1401/ced_ioc.c +++ b/drivers/staging/ced1401/ced_ioc.c @@ -84,9 +84,11 @@ static int ced_put_chars(struct ced_data *ced, const char *ch, unsigned int count) { int ret; + spin_lock_irq(ced-char_out_lock); /* get the output spin lock */ if ((OUTBUF_SZ - ced-num_output) = count) { unsigned int u; + for (u = 0; u count; u++) { ced-output_buffer[ced-out_buff_put++] = ch[u]; if (ced-out_buff_put = OUTBUF_SZ) @@ -140,6 +142,7 @@ int ced_send_string(struct ced_data *ced, const char __user *data, int ced_send_char(struct ced_data *ced, char c) { int ret; + mutex_lock(ced-io_mutex); /* Protect disconnect from new i/o */ ret = ced_put_chars(ced, c, 1); dev_dbg(ced-interface-dev, ced_send_char %c (0x%02x)\n, c, c); @@ -177,6 +180,7 @@ int ced_send_char(struct ced_data *ced, char c) int ced_get_state(struct ced_data *ced, __u32 *state, __u32 *error) { int got; + dev_dbg(ced-interface-dev, %s: entry\n, __func__); *state = 0x;/* Start off with invalid state */ @@ -192,6 +196,7 @@ int ced_get_state(struct ced_data *ced, __u32 *state, __u32 *error) *error = 0; } else { int device; + dev_dbg(ced-interface-dev, %s: Success, state: 0x%x, 0x%x\n, __func__, ced-stat_buf[0], ced-stat_buf[1]); @@ -249,6 +254,7 @@ int ced_read_write_cancel(struct ced_data *ced) bResult = IoCancelIrp(ced-pStagedIrp); /* Actually do the cancel */ if (bResult) { LARGE_INTEGER timeout; + timeout.QuadPart = -1000; /* Use a timeout of 1 second */ dev_info(ced-interface - dev, %s: about to wait till done\n, __func__); @@ -283,6 +289,7 @@ static int ced_in_self_test(struct ced_data *ced, unsigned int *stat) { unsigned int state, error; int ret = ced_get_state(ced, state, error); /* see if in self-test */ + if (ret == U14ERR_NOERROR) /* if all still OK */ ret = (state == (unsigned int)-1) ||/* TX problem or... */ ((state 0xff) == 0x80); /* ...self test */ @@ -311,6 +318,7 @@ static int ced_in_self_test(struct ced_data *ced, unsigned int *stat) static bool ced_is_1401(struct ced_data *ced) { int ret; + dev_dbg(ced-interface-dev, %s\n, __func__); ced_draw_down(ced); /* wait for, then kill outstanding Urbs */ @@ -334,11 +342,13 @@ static bool ced_is_1401(struct ced_data *ced) ced-dma_flag = MODE_CHAR; /* Clear DMA mode flag regardless! */ if (ret == 0) { /* if all is OK still */ unsigned int state; + ret = ced_in_self_test(ced, state); /* see if likely in */ /* self test*/ if (ret 0) { /* do we need to wait for self-test? */ /* when to give up */ unsigned long timeout = jiffies + 30 * HZ; + while ((ret 0) time_before(jiffies, timeout)) { schedule(); /* let other stuff run */ @@ -396,6 +406,7 @@ static bool ced_quick_check(struct ced_data *ced, bool test_buff, if (short_test || !can_reset) { /* Still OK to try the short test? */ /* Always test if no reset - we want state update */ unsigned int state, error; + dev_dbg(ced-interface-dev, %s: ced_get_state\n, __func__); if (ced_get_state(ced, state, error) == U14ERR_NOERROR) { /* Check on the 1401 state */ if ((state 0xFF) == 0)/* If call worked, check the status value */ @@ -437,6 +448,7 @@ int ced_reset(struct ced_data *ced) int ced_get_char(struct ced_data *ced) { int ret = U14ERR_NOIN; /* assume we will get nothing */ + mutex_lock(ced-io_mutex); /* Protect disconnect from new i/o */ dev_dbg(ced-interface-dev, %s\n, __func__); @@ -475,6 +487,7 @@ int ced_get_string(struct ced_data *ced, char __user *user, int n) { int available; /* character in the buffer */ int ret = U14ERR_NOIN; + if (n = 0) return -ENOMEM; @@ -492,6 +505,7 @@ int ced_get_string(struct ced_data *ced, char __user *user, int n)
Re: [PATCH v3 04/12] staging: ced1401: ced_ioc.c fix checkpatch warning
On Fri, Jul 11, 2014 at 04:44:32PM +0200, Luca Ellero wrote: Il 11/07/2014 16:17, Dan Carpenter ha scritto: You have a lot of patches with the exact same subject. Use different subjects like: [patch] staging: ced1401: ced_ioc.c: change spaces to tabs or whatever. regards, dan carpeneter Hi Dan, OK, I'll resend Oh wow. And you resent them very quickly too... It's normally a better idea to wait a bit before redoing everything in case other people have review comments. But in this case it's probably fine. I looked over the v4 patches and they seem ok. Thanks. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 9/9] staging: dgap: removes redundant null check and change paramter for dgap_tty_digisetcustombaud()
Hi, Dan 2014-07-11 22:20 GMT+09:00, Dan Carpenter dan.carpen...@oracle.com: Looks good to me. You should be CC'ing Mark as well. He's in MAINTAINERS now. I'd added Mark to cc list. All of maintainers including open mailing lists from get_maintainer.pl were added in my patches. :-) I wish the two of you would CC each other and review each other's work. I would like to review dgap patches. Daeseok, add yourself to the MAINTAINERS file for this driver as well so everyone CC's you on their dgap patches. You're doing good work. Really? :o Can I modify the MAINTAINERS file for adding myself? Thank you very much!! Regards, Daeseok Youn. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] drivers/staging/comedi/drivers/ni_atmio16d.c: remove pointless condition
On Fri, Jul 11, 2014 at 03:38:30PM +0100, Ian Abbott wrote: From: Andrey Utkin andrey.krieger.ut...@gmail.com From: Andrey Utkin andrey.krieger.ut...@gmail.com The issue was discovered with static analysis and has two instances in this file. The code looks like this if (x 65536000) { ... } else if (x 65536) { ... } else if (x = 0x /* 655360 */) { ... } else if (x = 0x /* 6553600 */) { ... } The meaning of this block is to select appropriate clock frequency for interval timer basing on x, which is amount of time. Notes: 1. That last condition matches previous one - that's the issue. 2. Decimal numbers in comments don't match hex numbers in expressions. But in first case the numbers have same order, while in the second case the hex number is the same, and the decimal one is 10 times bigger. 3. Actually type of x is unsigned int, so its exact upper limit is not obviously known. 4. There's no else block. So it makes sense to make an else block from last else if case. The code inside the block seems correct for such usage. [ Actually, get rid of the final else if case and change the next-to-last else if case to an else as the upper limit of x _is_ known to be 0x (UINT_MAX), which is less than 655360 -- Ian ] Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=79871 Reported-by: David Binderman dcb...@hotmail.com Signed-off-by: Andrey Utkin andrey.krieger.ut...@gmail.com Signed-off-by: Ian Abbott abbo...@mev.co.uk --- v2: Removed final else if block and changed preceding else if block to else block as the condition is always true due to limited range of unsigned int. -- Ian v3: Corrected subject line I messed up in v2. -- Ian --- drivers/staging/comedi/drivers/ni_atmio16d.c | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/staging/comedi/drivers/ni_atmio16d.c b/drivers/staging/comedi/drivers/ni_atmio16d.c index 6ad27f5..b1b4744 100644 --- a/drivers/staging/comedi/drivers/ni_atmio16d.c +++ b/drivers/staging/comedi/drivers/ni_atmio16d.c @@ -335,12 +335,9 @@ static int atmio16d_ai_cmd(struct comedi_device *dev, } else if (cmd-convert_arg 65536) { base_clock = CLOCK_100_KHZ; timer = cmd-convert_arg / 1; - } else if (cmd-convert_arg = 0x /* 655360 */) { + } else /* cmd-convert_arg 655360 */ { I think the comment is meant to be /* cmd-convert_arg = 65536 */ There is an extra zero on 655360. The comment is obvious and should be removed. Or maybe I haven't understood the comment and in that case it is a useless sucky comment so we should delete it? regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 9/9] staging: dgap: removes redundant null check and change paramter for dgap_tty_digisetcustombaud()
On Sat, Jul 12, 2014 at 12:23:16AM +0900, DaeSeok Youn wrote: Can I modify the MAINTAINERS file for adding myself? Yes. Just send a patch to add yourself. git log -p MAINTAINERS regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 9/9] staging: dgap: removes redundant null check and change paramter for dgap_tty_digisetcustombaud()
2014-07-12 0:37 GMT+09:00, Dan Carpenter dan.carpen...@oracle.com: On Sat, Jul 12, 2014 at 12:23:16AM +0900, DaeSeok Youn wrote: Can I modify the MAINTAINERS file for adding myself? Yes. Just send a patch to add yourself. OK. I will. Thanks. Regards, Daeseok Youn. git log -p MAINTAINERS regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [BISECTED][REGRESSION] Loading Hyper-V network drivers is racy in 3.14+ on Hyper-V 2012 R2
-Original Message- From: Sitsofe Wheeler [mailto:sits...@gmail.com] Sent: Friday, July 11, 2014 1:53 AM To: Haiyang Zhang Cc: KY Srinivasan; David S. Miller; de...@linuxdriverproject.org; linux- ker...@vger.kernel.org; net...@vger.kernel.org Subject: Re: [BISECTED][REGRESSION] Loading Hyper-V network drivers is racy in 3.14+ on Hyper-V 2012 R2 Oops that should have been https://bugzilla.kernel.org/attachment.cgi?id=142351 (either way it's information linked off https://bugzilla.kernel.org/show_bug.cgi?id=78771 ). Thanks for the dmesg. By looking at it, seems the netvsc driver was loaded properly, and 2 NICs are up, one NIC is down (probably not set to connected in HyperV manager?). Or, this dmesg wasn't the one when bug happens? [8.514493] hv_netvsc: hv_netvsc channel opened successfully [9.343318] hv_netvsc vmbus_0_14: Send section size: 6144, Section count:170 [9.345831] hv_netvsc vmbus_0_14: Device MAC 00:15:5d:6f:02:8f link state up [9.347101] hv_netvsc: hv_netvsc channel opened successfully [ 10.170308] hv_netvsc vmbus_0_15: Send section size: 6144, Section count:170 [ 10.170702] hv_netvsc vmbus_0_15: Device MAC 00:15:5d:6f:02:a5 link state up [ 10.172826] hv_netvsc: hv_netvsc channel opened successfully [ 10.988146] hv_netvsc vmbus_0_16: Send section size: 6144, Section count:170 [ 10.989069] hv_netvsc vmbus_0_16: Device MAC 00:15:5d:6f:02:a6 link state down Since you found the commit b679ef73edc is related to this problem, could you do a simple test: Reduce the receive buffer size back to 2MB, like below, then re-test it, see if the problem goes away? drivers/net/hyperv/hyperv_net.h #define NETVSC_RECEIVE_BUFFER_SIZE (1024*1024*2) /* 2MB */ Thanks, - Haiyang ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] drivers/staging/comedi/drivers/ni_atmio16d.c: remove pointless condition
On 2014-07-11 16:34, Dan Carpenter wrote: On Fri, Jul 11, 2014 at 03:38:30PM +0100, Ian Abbott wrote: From: Andrey Utkin andrey.krieger.ut...@gmail.com From: Andrey Utkin andrey.krieger.ut...@gmail.com The issue was discovered with static analysis and has two instances in this file. The code looks like this if (x 65536000) { ... } else if (x 65536) { ... } else if (x = 0x /* 655360 */) { ... } else if (x = 0x /* 6553600 */) { ... } The meaning of this block is to select appropriate clock frequency for interval timer basing on x, which is amount of time. Notes: 1. That last condition matches previous one - that's the issue. 2. Decimal numbers in comments don't match hex numbers in expressions. But in first case the numbers have same order, while in the second case the hex number is the same, and the decimal one is 10 times bigger. 3. Actually type of x is unsigned int, so its exact upper limit is not obviously known. 4. There's no else block. So it makes sense to make an else block from last else if case. The code inside the block seems correct for such usage. [ Actually, get rid of the final else if case and change the next-to-last else if case to an else as the upper limit of x _is_ known to be 0x (UINT_MAX), which is less than 655360 -- Ian ] Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=79871 Reported-by: David Binderman dcb...@hotmail.com Signed-off-by: Andrey Utkin andrey.krieger.ut...@gmail.com Signed-off-by: Ian Abbott abbo...@mev.co.uk --- v2: Removed final else if block and changed preceding else if block to else block as the condition is always true due to limited range of unsigned int. -- Ian v3: Corrected subject line I messed up in v2. -- Ian --- drivers/staging/comedi/drivers/ni_atmio16d.c | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/staging/comedi/drivers/ni_atmio16d.c b/drivers/staging/comedi/drivers/ni_atmio16d.c index 6ad27f5..b1b4744 100644 --- a/drivers/staging/comedi/drivers/ni_atmio16d.c +++ b/drivers/staging/comedi/drivers/ni_atmio16d.c @@ -335,12 +335,9 @@ static int atmio16d_ai_cmd(struct comedi_device *dev, } else if (cmd-convert_arg 65536) { base_clock = CLOCK_100_KHZ; timer = cmd-convert_arg / 1; - } else if (cmd-convert_arg = 0x /* 655360 */) { + } else /* cmd-convert_arg 655360 */ { I think the comment is meant to be /* cmd-convert_arg = 65536 */ There is an extra zero on 655360. That's not what I was intending to convey. The preceding chain of if/else if was checking if cmd-convert_arg 65536000, else if cmd-convert_arg 65536, and I was trying to convey that the final else part was valid for all remaining values less than 655360, which in fact is all remaining unsigned int values. (Obviously, I failed to convey this meaning to everyone!) -- -=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=- -=( 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 v3] drivers/staging/comedi/drivers/ni_atmio16d.c: remove pointless condition
On Fri, Jul 11, 2014 at 06:06:36PM +0100, Ian Abbott wrote: On 2014-07-11 16:34, Dan Carpenter wrote: On Fri, Jul 11, 2014 at 03:38:30PM +0100, Ian Abbott wrote: From: Andrey Utkin andrey.krieger.ut...@gmail.com From: Andrey Utkin andrey.krieger.ut...@gmail.com The issue was discovered with static analysis and has two instances in this file. The code looks like this if (x 65536000) { ... } else if (x 65536) { ... } else if (x = 0x /* 655360 */) { ... } else if (x = 0x /* 6553600 */) { ... } The meaning of this block is to select appropriate clock frequency for interval timer basing on x, which is amount of time. Notes: 1. That last condition matches previous one - that's the issue. 2. Decimal numbers in comments don't match hex numbers in expressions. But in first case the numbers have same order, while in the second case the hex number is the same, and the decimal one is 10 times bigger. 3. Actually type of x is unsigned int, so its exact upper limit is not obviously known. 4. There's no else block. So it makes sense to make an else block from last else if case. The code inside the block seems correct for such usage. [ Actually, get rid of the final else if case and change the next-to-last else if case to an else as the upper limit of x _is_ known to be 0x (UINT_MAX), which is less than 655360 -- Ian ] Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=79871 Reported-by: David Binderman dcb...@hotmail.com Signed-off-by: Andrey Utkin andrey.krieger.ut...@gmail.com Signed-off-by: Ian Abbott abbo...@mev.co.uk --- v2: Removed final else if block and changed preceding else if block to else block as the condition is always true due to limited range of unsigned int. -- Ian v3: Corrected subject line I messed up in v2. -- Ian --- drivers/staging/comedi/drivers/ni_atmio16d.c | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/staging/comedi/drivers/ni_atmio16d.c b/drivers/staging/comedi/drivers/ni_atmio16d.c index 6ad27f5..b1b4744 100644 --- a/drivers/staging/comedi/drivers/ni_atmio16d.c +++ b/drivers/staging/comedi/drivers/ni_atmio16d.c @@ -335,12 +335,9 @@ static int atmio16d_ai_cmd(struct comedi_device *dev, } else if (cmd-convert_arg 65536) { base_clock = CLOCK_100_KHZ; timer = cmd-convert_arg / 1; - } else if (cmd-convert_arg = 0x /* 655360 */) { + } else /* cmd-convert_arg 655360 */ { I think the comment is meant to be /* cmd-convert_arg = 65536 */ There is an extra zero on 655360. That's not what I was intending to convey. The preceding chain of if/else if was checking if cmd-convert_arg 65536000, else if cmd-convert_arg 65536, and I was trying to convey that the final else part was valid for all remaining values less than 655360, which in fact is all remaining unsigned int values. (Obviously, I failed to convey this meaning to everyone!) Oh. I could have figured it out if I had looked at the context maybe instead of just in the patch. It's weird that we are saying 0x1 x 1000 because we're mixing hex and decimal. Anyway, ignore me... I am a bad drive by reviewer. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] drivers/staging/comedi/drivers/ni_atmio16d.c: remove pointless condition
2014-07-11 17:38 GMT+03:00 Ian Abbott abbo...@mev.co.uk: Signed-off-by: Andrey Utkin andrey.krieger.ut...@gmail.com I think it's incorrect that you have instantly placed my signoff statement on this new patch. Anyway, thanks for your work on the issue. -- Andrey Utkin ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] drivers/staging/comedi/drivers/ni_atmio16d.c: remove pointless condition
On Fri, Jul 11, 2014 at 08:54:01PM +0300, Andrey Utkin wrote: 2014-07-11 17:38 GMT+03:00 Ian Abbott abbo...@mev.co.uk: Signed-off-by: Andrey Utkin andrey.krieger.ut...@gmail.com I think it's incorrect that you have instantly placed my signoff statement on this new patch. Ian gave you credit but also wrote in the changelog that he modified the patch from what you sent. That's pretty normal. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 7/8] drivers: scsi: storvsc: Set srb_flags in all cases
-Original Message- From: Christoph Hellwig [mailto:h...@infradead.org] Sent: Thursday, July 10, 2014 3:19 AM To: KY Srinivasan Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org; oher...@suse.com; jbottom...@parallels.com; jasow...@redhat.com; a...@canonical.com; linux-s...@vger.kernel.org; sta...@vger.kernel.org Subject: Re: [PATCH 7/8] drivers: scsi: storvsc: Set srb_flags in all cases default: vm_srb-data_in = UNKNOWN_TYPE; - vm_srb-win8_extension.srb_flags = 0; + vm_srb-win8_extension.srb_flags |= (SRB_FLAGS_DATA_IN | +SRB_FLAGS_DATA_OUT); This would usually be a command that doesn't transfer data (e.g. TEST_UNIT_READY or SYNCHRONIZE_CACHE), do you really want to set the in and out flags here? On the host, before they forward the command to the native driver stack, I am told they validate that the flags be correctly set because of some bugs in the Emulex driver. K. Y ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: unisys: added virtpci info entry
This patch adds the virtpci debugfs directory and the info entry inside of it. Signed-off-by: Erik Arfvidson erik.arfvid...@unisys.com Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com --- v3: Fixed formating and comments. Also added debufs_remove_recursive() and simple simple_read_from_buffer() v2: fixed comments and applied Dan Carpenter suggestions drivers/staging/unisys/virtpci/virtpci.c | 108 ++- 1 file changed, 105 insertions(+), 3 deletions(-) diff --git a/drivers/staging/unisys/virtpci/virtpci.c b/drivers/staging/unisys/virtpci/virtpci.c index 7d840b0..6ea29f5 100644 --- a/drivers/staging/unisys/virtpci/virtpci.c +++ b/drivers/staging/unisys/virtpci/virtpci.c @@ -36,6 +36,7 @@ #include linux/mod_devicetable.h #include linux/if_ether.h #include linux/version.h +#include linux/debugfs.h #include version.h #include guestlinuxdebug.h #include timskmodutils.h @@ -100,6 +101,12 @@ static int virtpci_device_resume(struct device *dev); static int virtpci_device_probe(struct device *dev); static int virtpci_device_remove(struct device *dev); +static ssize_t info_debugfs_read(struct file *file, char __user *buf, + size_t len, loff_t *offset); + +static const struct file_operations debugfs_info_fops = { + .read = info_debugfs_read, +}; /*/ /* Globals */ @@ -139,7 +146,19 @@ static DEFINE_RWLOCK(VpcidevListLock); /* filled in with info about this driver, wrt it servicing client busses */ static ULTRA_VBUS_DEVICEINFO Bus_DriverInfo; - +/*/ +/* DebugFS entries */ +/*/ +/* dentry is used to create the debugfs entry directory + * for virtpci + */ +static struct dentry *virtpci_debugfs_dir; +static struct dentry *info_debugfs_entry; +/* info_debugfs_entry is used to tell virtpci to display current info + * kept in the driver + */ +#define DIR_DEBUGFS_ENTRY virtpci +#define INFO_DEBUGFS_ENTRY_FN info struct virtpci_busdev { struct device virtpci_bus_device; @@ -1375,6 +1394,85 @@ void virtpci_unregister_driver(struct virtpci_driver *drv) DBGINF(Leaving\n); } EXPORT_SYMBOL_GPL(virtpci_unregister_driver); +/*/ +/* debugfs filesystem functions */ +/*/ +struct print_vbus_info { + int *length; + char *buf; +}; + +static int print_vbus(struct device *vbus, void *data) +{ + struct print_vbus_info *p = (struct print_vbus_info *)data; + + *p-length += sprintf(p-buf + *p-length, bus_id:%s\n, + dev_name(vbus)); + return 0; +} + +static ssize_t info_debugfs_read(struct file *file, char __user *buf, + size_t len, loff_t *offset) +{ + ssize_t bytes_read = 0; + int str_pos = 0; + struct virtpci_dev *tmpvpcidev; + unsigned long flags; + struct print_vbus_info printparam; + char *vbuf; + + vbuf = kzalloc(len, GFP_KERNEL); + if (!vbuf) + return -ENOMEM; + + str_pos += scnprintf(vbuf + str_pos, len - str_pos, +Virtual PCI Bus devices\n); + printparam.length = str_pos; + printparam.buf = vbuf; + if (bus_for_each_dev(virtpci_bus_type, NULL, +(void *) printparam, print_vbus)) + LOGERR(Failed to find bus\n); + + str_pos += scnprintf(vbuf + str_pos, len - str_pos, + \n Virtual PCI devices\n); + read_lock_irqsave(VpcidevListLock, flags); + tmpvpcidev = VpcidevListHead; + while (tmpvpcidev) { + if (tmpvpcidev-devtype == VIRTHBA_TYPE) { + str_pos += scnprintf(vbuf + str_pos, len - str_pos, + [%d:%d] VHba:%08x:%08x max-config:%d-%d-%d-%d, + tmpvpcidev-busNo, tmpvpcidev-deviceNo, + tmpvpcidev-scsi.wwnn.wwnn1, + tmpvpcidev-scsi.wwnn.wwnn2, + tmpvpcidev-scsi.max.max_channel, + tmpvpcidev-scsi.max.max_id, + tmpvpcidev-scsi.max.max_lun, + tmpvpcidev-scsi.max.cmd_per_lun); + } else { + str_pos += scnprintf(vbuf + str_pos, len - str_pos, + [%d:%d] VNic:%02x:%02x:%02x:%02x:%02x:%02x num_rcv_bufs:%d mtu:%d, + tmpvpcidev-busNo, tmpvpcidev-deviceNo, + tmpvpcidev-net.mac_addr[0], +
Re: [PATCH] staging: unisys: added virtpci info entry
On Fri, Jul 11, 2014 at 07:11:45PM -0400, Erik Arfvidson wrote: This patch adds the virtpci debugfs directory and the info entry inside of it. Signed-off-by: Erik Arfvidson erik.arfvid...@unisys.com Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com --- v3: Fixed formating and comments. Also added debufs_remove_recursive() and simple simple_read_from_buffer() v2: fixed comments and applied Dan Carpenter suggestions drivers/staging/unisys/virtpci/virtpci.c | 108 ++- 1 file changed, 105 insertions(+), 3 deletions(-) diff --git a/drivers/staging/unisys/virtpci/virtpci.c b/drivers/staging/unisys/virtpci/virtpci.c index 7d840b0..6ea29f5 100644 --- a/drivers/staging/unisys/virtpci/virtpci.c +++ b/drivers/staging/unisys/virtpci/virtpci.c @@ -36,6 +36,7 @@ #include linux/mod_devicetable.h #include linux/if_ether.h #include linux/version.h +#include linux/debugfs.h #include version.h #include guestlinuxdebug.h #include timskmodutils.h @@ -100,6 +101,12 @@ static int virtpci_device_resume(struct device *dev); static int virtpci_device_probe(struct device *dev); static int virtpci_device_remove(struct device *dev); +static ssize_t info_debugfs_read(struct file *file, char __user *buf, + size_t len, loff_t *offset); + +static const struct file_operations debugfs_info_fops = { + .read = info_debugfs_read, +}; /*/ /* Globals */ @@ -139,7 +146,19 @@ static DEFINE_RWLOCK(VpcidevListLock); /* filled in with info about this driver, wrt it servicing client busses */ static ULTRA_VBUS_DEVICEINFO Bus_DriverInfo; - +/*/ +/* DebugFS entries */ +/*/ +/* dentry is used to create the debugfs entry directory + * for virtpci + */ +static struct dentry *virtpci_debugfs_dir; +static struct dentry *info_debugfs_entry; +/* info_debugfs_entry is used to tell virtpci to display current info + * kept in the driver + */ You don't need this variable as all you do is set it, and then never do anything with it. Worse case, make it a local variable. As you never check it, just don't even do that... +#define DIR_DEBUGFS_ENTRY virtpci +#define INFO_DEBUGFS_ENTRY_FN info You only reference these #defines once, no need for them at all. struct virtpci_busdev { struct device virtpci_bus_device; @@ -1375,6 +1394,85 @@ void virtpci_unregister_driver(struct virtpci_driver *drv) DBGINF(Leaving\n); } EXPORT_SYMBOL_GPL(virtpci_unregister_driver); +/*/ +/* debugfs filesystem functions */ +/*/ +struct print_vbus_info { + int *length; + char *buf; +}; + +static int print_vbus(struct device *vbus, void *data) +{ + struct print_vbus_info *p = (struct print_vbus_info *)data; + + *p-length += sprintf(p-buf + *p-length, bus_id:%s\n, + dev_name(vbus)); + return 0; +} + +static ssize_t info_debugfs_read(struct file *file, char __user *buf, + size_t len, loff_t *offset) +{ + ssize_t bytes_read = 0; + int str_pos = 0; + struct virtpci_dev *tmpvpcidev; + unsigned long flags; + struct print_vbus_info printparam; + char *vbuf; + + vbuf = kzalloc(len, GFP_KERNEL); Sorry I missed this last time, but what happens if userspace asks for 10Gb? Nasty problems will happen if you try to allocate that. Or 100Mb. Set a reasonable upper bound here please. Remember: All input is evil. + if (!vbuf) + return -ENOMEM; + + str_pos += scnprintf(vbuf + str_pos, len - str_pos, + Virtual PCI Bus devices\n); Why the leading ' '? + printparam.length = str_pos; + printparam.buf = vbuf; + if (bus_for_each_dev(virtpci_bus_type, NULL, + (void *) printparam, print_vbus)) + LOGERR(Failed to find bus\n); + + str_pos += scnprintf(vbuf + str_pos, len - str_pos, + \n Virtual PCI devices\n); + read_lock_irqsave(VpcidevListLock, flags); + tmpvpcidev = VpcidevListHead; + while (tmpvpcidev) { + if (tmpvpcidev-devtype == VIRTHBA_TYPE) { + str_pos += scnprintf(vbuf + str_pos, len - str_pos, + [%d:%d] VHba:%08x:%08x max-config:%d-%d-%d-%d, + tmpvpcidev-busNo, tmpvpcidev-deviceNo, + tmpvpcidev-scsi.wwnn.wwnn1, + tmpvpcidev-scsi.wwnn.wwnn2, + tmpvpcidev-scsi.max.max_channel, +
Re: [PATCH v3] drivers/staging/comedi/drivers/ni_atmio16d.c: remove pointless condition
On Fri, Jul 11, 2014 at 03:39:48PM +0100, Ian Abbott wrote: On 2014-07-11 15:38, Ian Abbott wrote: From: Andrey Utkin andrey.krieger.ut...@gmail.com From: Andrey Utkin andrey.krieger.ut...@gmail.com Dammit! Greg, do you want to sort that out or should I have another go? Heh, no worries, I can fix it up, thanks. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote: A simple script to run checkpatch --fix for various types of of cleanups. This script is useful primarily for staging. This reformats code to a more CodingStyle conforming style, compiles it, verifies that the object code hasn't changed, and git commits it too. And 'git commits' it? Heh, I should just run this myself to clean up staging and beat everyone else to it... I know some people already have private versions of these things, might as well make it public for all to abuse :) thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote: A simple script to run checkpatch --fix for various types of of cleanups. This script is useful primarily for staging. This reformats code to a more CodingStyle conforming style, compiles it, verifies that the object code hasn't changed, and git commits it too. You must have the necessary development tools, git, and a recent git tree. Ideally use Greg KH's staging-next, which can be retrieved via these commands: git clone git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git git checkout staging-next To use this script try a sequence of commands like: cd linux_repository git checkout -b your_branch make allyesconfig mkdir patches ./scripts/reformat_with_checkpatch.sh drivers/staging/dir/*.[ch] git format-patch --cover-letter -o patches/your_branch staging-next git send-email patches/your_branch Signed-off-by: Joe Perches j...@perches.com --- scripts/reformat_with_checkpatch.sh | 141 1 file changed, 141 insertions(+) create mode 100755 scripts/reformat_with_checkpatch.sh No --help option? How do I run this thing? I ran it on a file that had no problems and got a mess, all I think due to something odd in checkpatch itself right now: $ ./scripts/checkpatch.pl --file drivers/staging/lustre/include/linux/lnet/api.h Useless use of greediness modifier '+' in regex; marked by -- HERE in m/(^\+.*) {8,8}+ -- HERE \t/ at ./scripts/checkpatch.pl line 2217. total: 0 errors, 0 warnings, 220 lines checked What's with that 'Useless use... error? perl 5.20 in use, is that the issue? Anyway, running this script on the file gave me this: $ ./reformat_with_checkpatch drivers/staging/lustre/include/linux/lnet/api.h file: drivers/staging/lustre/include/linux/lnet/api.h description: whitespace neatening types:spacing,space_before_tab,pointer_location,trailing_whitespace,bracket_space Useless use of greediness modifier '+' in regex; marked by -- HERE in m/(^\+.*) {8,8}+ -- HERE \t/ at ./scripts/checkpatch.pl line 2217. total: 0 errors, 0 warnings, 0 checks, 220 lines checked NOTE: Used message types: BRACKET_SPACE POINTER_LOCATION SPACE_BEFORE_TAB SPACING TRAILING_WHITESPACE drivers/staging/lustre/include/linux/lnet/api.h has no obvious style problems and is ready for submission. file: drivers/staging/lustre/include/linux/lnet/api.h description: remove spaces before tabs types:space_before_tab Useless use of greediness modifier '+' in regex; marked by -- HERE in m/(^\+.*) {8,8}+ -- HERE \t/ at ./scripts/checkpatch.pl line 2217. total: 0 errors, 0 warnings, 0 checks, 220 lines checked NOTE: Used message types: SPACE_BEFORE_TAB drivers/staging/lustre/include/linux/lnet/api.h has no obvious style problems and is ready for submission. file: drivers/staging/lustre/include/linux/lnet/api.h description: fix label positions types:indented_label Useless use of greediness modifier '+' in regex; marked by -- HERE in m/(^\+.*) {8,8}+ -- HERE \t/ at ./scripts/checkpatch.pl line 2217. total: 0 errors, 0 warnings, 0 checks, 220 lines checked NOTE: Used message types: INDENTED_LABEL drivers/staging/lustre/include/linux/lnet/api.h has no obvious style problems and is ready for submission. file: drivers/staging/lustre/include/linux/lnet/api.h description: align arguments to parenthesis types:parenthesis_alignment Useless use of greediness modifier '+' in regex; marked by -- HERE in m/(^\+.*) {8,8}+ -- HERE \t/ at ./scripts/checkpatch.pl line 2217. total: 0 errors, 0 warnings, 0 checks, 220 lines checked NOTE: Used message types: PARENTHESIS_ALIGNMENT drivers/staging/lustre/include/linux/lnet/api.h has no obvious style problems and is ready for submission. file: drivers/staging/lustre/include/linux/lnet/api.h description: fix brace positions types:open_brace,braces,else_after_brace,while_after_brace Useless use of greediness modifier '+' in regex; marked by -- HERE in m/(^\+.*) {8,8}+ -- HERE \t/ at ./scripts/checkpatch.pl line 2217. total: 0 errors, 0 warnings, 0 checks, 220 lines checked NOTE: Used message types: BRACES ELSE_AFTER_BRACE OPEN_BRACE WHILE_AFTER_BRACE drivers/staging/lustre/include/linux/lnet/api.h has no obvious style problems and is ready for submission. file: drivers/staging/lustre/include/linux/lnet/api.h description: fix blank lines types:line_spacing Useless use of greediness modifier '+' in regex; marked by -- HERE in m/(^\+.*) {8,8}+ -- HERE \t/ at ./scripts/checkpatch.pl line 2217. total: 0 errors, 0 warnings, 0 checks, 220 lines checked NOTE: Used message types: LINE_SPACING drivers/staging/lustre/include/linux/lnet/api.h has no obvious style problems and is ready for submission. file: drivers/staging/lustre/include/linux/lnet/api.h description: use standard attributes types:prefer_packed,prefer_aligned Useless use of greediness modifier '+' in regex; marked by -- HERE in m/(^\+.*)
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote: A simple script to run checkpatch --fix for various types of of cleanups. This script is useful primarily for staging. This reformats code to a more CodingStyle conforming style, compiles it, verifies that the object code hasn't changed, and git commits it too. You must have the necessary development tools, git, and a recent git tree. Ideally use Greg KH's staging-next, which can be retrieved via these commands: git clone git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git git checkout staging-next To use this script try a sequence of commands like: cd linux_repository git checkout -b your_branch make allyesconfig mkdir patches ./scripts/reformat_with_checkpatch.sh drivers/staging/dir/*.[ch] git format-patch --cover-letter -o patches/your_branch staging-next git send-email patches/your_branch Signed-off-by: Joe Perches j...@perches.com --- scripts/reformat_with_checkpatch.sh | 141 1 file changed, 141 insertions(+) create mode 100755 scripts/reformat_with_checkpatch.sh diff --git a/scripts/reformat_with_checkpatch.sh b/scripts/reformat_with_checkpatch.sh new file mode 100755 index 000..5415a8e --- /dev/null +++ b/scripts/reformat_with_checkpatch.sh @@ -0,0 +1,141 @@ +#!/bin/bash +# (c) 2014, Joe Perches j...@perches.com +# +# Automate checkpatch modifications and git commits to +# neaten code that doesn't conform to CodingStyle. +# Useful primarily for files in drivers/staging +# +# Licensed under the terms of the GNU GPL License version 2 + +declare -ar whitespace_types=( \ + 'whitespace_neatening:spacing,space_before_tab,pointer_location,trailing_whitespace,bracket_space' \ +'remove_spaces_before_tabs:space_before_tab' \ +'fix_label_positions:indented_label' \ +'align_arguments_to_parenthesis:parenthesis_alignment' \ +) + +declare -ar changecode_types=( \ + 'fix_brace_positions:open_brace,braces,else_after_brace,while_after_brace' \ +'fix_blank_lines:line_spacing' \ +'use_standard_attributes:prefer_packed,prefer_aligned' \ +'remove_unnecessary_externs:avoid_externs' \ +'update_c90_comment_style:c99_comments' \ +) + +checkpatch_update () +{ +file=$1 + +desc=$(echo $2 | cut -f1 -d: | sed 's/_/ /g') +types=$(echo $2 | cut -f2- -d:) + +echo file: $file description: $desc types:$types + +./scripts/checkpatch.pl --file --fix-inplace --strict --types=$types $file + +checkpatch_fixes=$file.diff +git diff --stat -p --exit-code $file $checkpatch_fixes +if [ $? == 0 ] ; then + rm -f $checkpatch_fixes + return +fi + +basename=$(basename $file) +if [ ${basename##*.} == c ] ; then + + git checkout $file + obj=$(echo $file | sed 's/\.c$/\.o/') + if [ -e $obj ] ; then + rm -f $obj + fi + + echo Compiling original version... + + ${CROSS_COMPILE}make $obj + + ${CROSS_COMPILE}objdump -D $obj | \ + sed s/^[[:space:]]\+[0-9a-f]\+// $obj.old + + patch -p1 $checkpatch_fixes + + echo Compiling modified version... + + ${CROSS_COMPILE}make $obj + + ${CROSS_COMPILE}objdump -D $obj | \ + sed s/^[[:space:]]\+[0-9a-f]\+// $obj.new + + echo Comparing objects... + diff -Nurd $obj.new $obj.old + if [ $? -ne 0 ] ; then + echo Object differences exist! - Verify changes before commit! + read -s -p Press the 'enter' key to continue: + else + echo No object differences found + fi + rm -f $obj.old + rm -f $obj.new +fi + +echo running checkpatch on possible checkpatch fixes... + +./scripts/checkpatch.pl --no-summary --no-signoff $checkpatch_fixes +rm -f $checkpatch_fixes + +echo Verify checkpatch output and make any necessary changes +echo Edit '$file' if appropriate +read -s -p Press the 'enter' key to continue: + +commit_log_file=$(mktemp git_commit.XX) + +if [ -e $commit_log_file ] ; then + rm -f $commit_log_file +fi + +if [[ $file =~ ^drivers/staging/ ]] ; then + echo -n staging: $commit_log_file +fi +echo $(basename $(dirname $file)): checkpatch cleanup: $desc $commit_log_file If I pick drivers/staging/lustre/include/linux/lnet/types.h, then I get: staging: lnet: checkpatch cleanup: whitespace neatening and no 'types.h' here, is that intentional? If so, why? And this is fun, I'm going to let this rip on the lustre code... thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote: A simple script to run checkpatch --fix for various types of of cleanups. This script is useful primarily for staging. This reformats code to a more CodingStyle conforming style, compiles it, verifies that the object code hasn't changed, and git commits it too. You must have the necessary development tools, git, and a recent git tree. Ideally use Greg KH's staging-next, which can be retrieved via these commands: git clone git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git git checkout staging-next To use this script try a sequence of commands like: cd linux_repository git checkout -b your_branch make allyesconfig mkdir patches ./scripts/reformat_with_checkpatch.sh drivers/staging/dir/*.[ch] git format-patch --cover-letter -o patches/your_branch staging-next git send-email patches/your_branch Signed-off-by: Joe Perches j...@perches.com --- scripts/reformat_with_checkpatch.sh | 141 1 file changed, 141 insertions(+) create mode 100755 scripts/reformat_with_checkpatch.sh diff --git a/scripts/reformat_with_checkpatch.sh b/scripts/reformat_with_checkpatch.sh new file mode 100755 index 000..5415a8e --- /dev/null +++ b/scripts/reformat_with_checkpatch.sh @@ -0,0 +1,141 @@ +#!/bin/bash +# (c) 2014, Joe Perches j...@perches.com +# +# Automate checkpatch modifications and git commits to +# neaten code that doesn't conform to CodingStyle. +# Useful primarily for files in drivers/staging +# +# Licensed under the terms of the GNU GPL License version 2 + +declare -ar whitespace_types=( \ + 'whitespace_neatening:spacing,space_before_tab,pointer_location,trailing_whitespace,bracket_space' \ +'remove_spaces_before_tabs:space_before_tab' \ +'fix_label_positions:indented_label' \ +'align_arguments_to_parenthesis:parenthesis_alignment' \ +) + +declare -ar changecode_types=( \ + 'fix_brace_positions:open_brace,braces,else_after_brace,while_after_brace' \ +'fix_blank_lines:line_spacing' \ +'use_standard_attributes:prefer_packed,prefer_aligned' \ +'remove_unnecessary_externs:avoid_externs' \ +'update_c90_comment_style:c99_comments' \ +) + +checkpatch_update () +{ +file=$1 + +desc=$(echo $2 | cut -f1 -d: | sed 's/_/ /g') +types=$(echo $2 | cut -f2- -d:) + +echo file: $file description: $desc types:$types + +./scripts/checkpatch.pl --file --fix-inplace --strict --types=$types $file + +checkpatch_fixes=$file.diff +git diff --stat -p --exit-code $file $checkpatch_fixes +if [ $? == 0 ] ; then + rm -f $checkpatch_fixes + return +fi + +basename=$(basename $file) +if [ ${basename##*.} == c ] ; then + + git checkout $file + obj=$(echo $file | sed 's/\.c$/\.o/') + if [ -e $obj ] ; then + rm -f $obj + fi + + echo Compiling original version... + + ${CROSS_COMPILE}make $obj This fails for when a cflags option is set (like an include path). Now, I could argue that having an include path override in a kernel Makefile is a bug, but well, it's staging... Anyway, try running this script on drivers/staging/lustre/lnet/lnet/acceptor.c to see how this build fails. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, Jul 11, 2014 at 06:46:52PM -0700, Joe Perches wrote: On Fri, 2014-07-11 at 18:39 -0700, Greg KH wrote: On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote: A simple script to run checkpatch --fix for various types of of cleanups. ] drivers/staging/lustre/include/linux/lnet/api.h has no obvious style problems and is ready for submission. file: drivers/staging/lustre/include/linux/lnet/api.h description: update c90 comment style types:c99_comments Useless use of greediness modifier '+' in regex; marked by -- HERE in m/(^\+.*) {8,8}+ -- HERE \t/ at ./scripts/checkpatch.pl line 2217. [] drivers/staging/lustre/include/linux/lnet/api.h has no obvious style problems and is ready for submission. [] Is that expected? No, I haven't seen that. Can you tell me what git tree you're working on? My staging-next branch of staging.git on git.kernel.org Also, can you use the scripts/checkpatch from -next tag next-20140711 that will take a bit to checkout, I'll do that afterward. My system has: $ perl --version This is perl 5, version 18, subversion 2 (v5.18.2) built for i686-linux-gnu-thread-multi-64int (with 41 registered patches, see perl -V for more detail) I think this started showing up for me for perl 5.20. Let me go checkout linux-next and see if that fixes anything or not... thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, 2014-07-11 at 18:34 -0700, Greg KH wrote: On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote: A simple script to run checkpatch --fix for various types of of cleanups. This script is useful primarily for staging. This reformats code to a more CodingStyle conforming style, compiles it, verifies that the object code hasn't changed, and git commits it too. And 'git commits' it? The thought I had was to made it easier to submit my first kernel patch correctly. like that tuxradar article you wrote a few years ago. http://www.tuxradar.com/content/newbies-guide-hacking-linux-kernel Heh, I should just run this myself to clean up staging and beat everyone else to it... At least for the whitespace noise, you could but I hope it'll encourage a few more people to try kernel patching instead. I know some people already have private versions of these things, might as well make it public for all to abuse :) And I hope they can improve it too as it's just a brainless little script. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, Jul 11, 2014 at 07:01:14PM -0700, Greg KH wrote: On Fri, Jul 11, 2014 at 06:46:52PM -0700, Joe Perches wrote: On Fri, 2014-07-11 at 18:39 -0700, Greg KH wrote: On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote: A simple script to run checkpatch --fix for various types of of cleanups. ] drivers/staging/lustre/include/linux/lnet/api.h has no obvious style problems and is ready for submission. file: drivers/staging/lustre/include/linux/lnet/api.h description: update c90 comment style types:c99_comments Useless use of greediness modifier '+' in regex; marked by -- HERE in m/(^\+.*) {8,8}+ -- HERE \t/ at ./scripts/checkpatch.pl line 2217. [] drivers/staging/lustre/include/linux/lnet/api.h has no obvious style problems and is ready for submission. [] Is that expected? No, I haven't seen that. Can you tell me what git tree you're working on? My staging-next branch of staging.git on git.kernel.org Also, can you use the scripts/checkpatch from -next tag next-20140711 that will take a bit to checkout, I'll do that afterward. My system has: $ perl --version This is perl 5, version 18, subversion 2 (v5.18.2) built for i686-linux-gnu-thread-multi-64int (with 41 registered patches, see perl -V for more detail) I think this started showing up for me for perl 5.20. Let me go checkout linux-next and see if that fixes anything or not... Ok, with linux-next I get the same thing: ~/linux/tmp/linux-next $ ./scripts/checkpatch.pl -f --strict drivers/staging/lustre/include/linux/lnet/api.h --types=c99_comments Useless use of greediness modifier '+' in regex; marked by -- HERE in m/(^\+.*) {8,8}+ -- HERE \t/ at ./scripts/checkpatch.pl line 2358. total: 0 errors, 0 warnings, 0 checks, 220 lines checked NOTE: Used message types: C99_COMMENTS drivers/staging/lustre/include/linux/lnet/api.h has no obvious style problems and is ready for submission. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, 2014-07-11 at 18:43 -0700, Greg KH wrote: On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote: A simple script to run checkpatch --fix for various types of of cleanups. [] If I pick drivers/staging/lustre/include/linux/lnet/types.h, then I get: staging: lnet: checkpatch cleanup: whitespace neatening and no 'types.h' here, is that intentional? If so, why? Yes, it's how it's written. It uses just the directory name, not any basename($file) It can be changed if that's what's desired. And this is fun, I'm going to let this rip on the lustre code... It doesn't autocommit, it does show the various changes it makes and asks you to accept them. I suppose that could be automated with something better than yes, but I didn't want it to be completely automatic. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, Jul 11, 2014 at 06:57:24PM -0700, Joe Perches wrote: On Fri, 2014-07-11 at 18:53 -0700, Greg KH wrote: On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote: A simple script to run checkpatch --fix for various types of of cleanups. [] Anyway, try running this script on drivers/staging/lustre/lnet/lnet/acceptor.c to see how this build fails. lustre doesn't use normal kernel makefiles. Well, it does, it just has a: subdir-ccflags-y := -I$(src)/include/ line in drivers/staging/lustre/Makefile that messes up making .o files in subdirectories. I'll go fix that include file mess up now to be able to remove that line so the the .o file building will work. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, 2014-07-11 at 18:53 -0700, Greg KH wrote: On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote: A simple script to run checkpatch --fix for various types of of cleanups. [] Anyway, try running this script on drivers/staging/lustre/lnet/lnet/acceptor.c to see how this build fails. lustre doesn't use normal kernel makefiles. I supposed that the entire staging/lustre/ path could be built for every change, but for most of staging, and nearly all of the rest of the kernel tree, using: make pathfile.o works. I know there can be many enhancements to the reformat script, but I think a minimally acceptable version is a decent start. So, thanks for trying it out. And keep the complaints coming. cheers, Joe ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] checkpatch: Remove unnecessary + after {8,8}
On Fri, Jul 11, 2014 at 07:09:30PM -0700, Joe Perches wrote: There's a useless + use that needs to be removed as perl 5.20 emits a Useless use of greediness modifier '+' message each time it's hit. Reported-by: Greg KH gre...@linuxfoundation.org Signed-off-by: Joe Perches j...@perches.com --- On Fri, 2014-07-11 at 19:05 -0700, Greg KH wrote: Ok, with linux-next I get the same thing: Thanks Greg. Very nice, thanks for this: Tested-by: Greg Kroah-Hartman gre...@linuxfoundation.org ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] checkpatch: Remove unnecessary + after {8,8}
There's a useless + use that needs to be removed as perl 5.20 emits a Useless use of greediness modifier '+' message each time it's hit. Reported-by: Greg KH gre...@linuxfoundation.org Signed-off-by: Joe Perches j...@perches.com --- On Fri, 2014-07-11 at 19:05 -0700, Greg KH wrote: Ok, with linux-next I get the same thing: Thanks Greg. scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index d5ac001..370a974 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2376,7 +2376,7 @@ sub process { please, no space before tabs\n . $herevet) $fix) { while ($fixed[$fixlinenr] =~ - s/(^\+.*) {8,8}+\t/$1\t\t/) {} + s/(^\+.*) {8,8}\t/$1\t\t/) {} while ($fixed[$fixlinenr] =~ s/(^\+.*) +\t/$1\t/) {} } ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, Jul 11, 2014 at 06:40:16PM -0700, Joe Perches wrote: On Fri, 2014-07-11 at 18:34 -0700, Greg KH wrote: On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote: A simple script to run checkpatch --fix for various types of of cleanups. This script is useful primarily for staging. This reformats code to a more CodingStyle conforming style, compiles it, verifies that the object code hasn't changed, and git commits it too. And 'git commits' it? The thought I had was to made it easier to submit my first kernel patch correctly. like that tuxradar article you wrote a few years ago. http://www.tuxradar.com/content/newbies-guide-hacking-linux-kernel Yeah, but it's good for people to actually understand the change they are making, and edit it themselves. Or at least I think so, but I know others don't, so I don't mind this script. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 4/8] Drivers: scsi: storvsc: Filter WRITE_SAME_16
-Original Message- From: h...@infradead.org [mailto:h...@infradead.org] Sent: Thursday, July 10, 2014 11:32 PM To: James Bottomley Cc: KY Srinivasan; linux-ker...@vger.kernel.org; m...@mkp.net; h...@infradead.org; de...@linuxdriverproject.org; a...@canonical.com; sta...@vger.kernel.org; linux-s...@vger.kernel.org; oher...@suse.com; jasow...@redhat.com Subject: Re: [PATCH 4/8] Drivers: scsi: storvsc: Filter WRITE_SAME_16 On Wed, Jul 09, 2014 at 10:27:24PM +, James Bottomley wrote: If we fix it at source, why would there be any need to filter? That's the reason the no_write_same flag was introduced. If we can find and fix the bug, it can go back into the stable trees as a bug fix, hence nothing should ever emit write_same(10 or 16) and additional driver code is redundant (and counter productive, since if this ever breaks again you're our best canary). This looks like it might be the problem but Martin should confirm (I think the problem comes to us from the RC16 code which unconditionally sets WS16). I think the problem is a differnet one. If we have the logical provisioning EVPD it configures what method to use, but if we don't have one we simply check for a max unmap blocks field, and if that's not present use WRITE SAME. The patch checks the no_write_same flag before doing that, for which we also have to do the write_same setup before the discard setup in sd_revalidate_disk. Ky: does hyperv support UNMAP? If so any idea why it doesn't set the maximum unmap block count field in the EVPD? Windows hosts do support UNMAP and set the field in the EVPD. However, since the host advertises SPC-2 compliance, Linux does not even query the VPD page. If we want to enable UNMAP in this case I'd prefer a blacklist entry than trying UNMAP despite the device not advertising it. diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index ba756b1..fbccfd2 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -2614,9 +2614,10 @@ static void sd_read_block_limits(struct scsi_disk *sdkp) if (sdkp-max_unmap_blocks) sd_config_discard(sdkp, SD_LBP_UNMAP); - else + else if (!sdkp-device-no_write_same) sd_config_discard(sdkp, SD_LBP_WS16); - + else + sd_config_discard(sdkp, SD_LBP_DISABLE); } else {/* LBP VPD page tells us what to use */ if (sdkp-lbpu sdkp-max_unmap_blocks) @@ - 2766,6 +2767,7 @@ static int sd_revalidate_disk(struct gendisk *disk) */ if (sdkp-media_present) { sd_read_capacity(sdkp, buffer); + sd_read_write_same(sdkp, buffer); if (sd_try_extended_inquiry(sdp)) { sd_read_block_provisioning(sdkp); @@ -2776,7 +2778,6 @@ static int sd_revalidate_disk(struct gendisk *disk) sd_read_write_protect_flag(sdkp, buffer); sd_read_cache_type(sdkp, buffer); sd_read_app_tag_own(sdkp, buffer); - sd_read_write_same(sdkp, buffer); } sdkp-first_scan = 0; ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 4/8] Drivers: scsi: storvsc: Filter WRITE_SAME_16
-Original Message- From: Martin K. Petersen [mailto:martin.peter...@oracle.com] Sent: Friday, July 11, 2014 5:54 AM To: h...@infradead.org Cc: James Bottomley; KY Srinivasan; linux-ker...@vger.kernel.org; de...@linuxdriverproject.org; a...@canonical.com; sta...@vger.kernel.org; linux-s...@vger.kernel.org; oher...@suse.com; jasow...@redhat.com Subject: Re: [PATCH 4/8] Drivers: scsi: storvsc: Filter WRITE_SAME_16 hch == hch@infradead org h...@infradead.org writes: (Back from vacation: Bear with me while I'm catching up on two weeks of linux-scsi stuff...) hch I think the problem is a differnet one. If we have the logical hch provisioning EVPD it configures what method to use, but if we don't hch have one we simply check for a max unmap blocks field, and if hch that's not present use WRITE SAME. Yeah, that was done to accommodate the devices out there that predate the LBP VPD. There sadly are several still. And it's hard to motivate people to update their storage array firmware even when updates are readily available. hch The patch checks the no_write_same flag before doing that, for hch which we also have to do the write_same setup before the discard hch setup in sd_revalidate_disk. The no_write_same was introduced to disable the REQ_WRITE_SAME use case where we have no INQUIRY/READ CAPACITY/VPD flags to key off of to determine whether it is safe to send the commands. no_write_same does not preclude the REQ_DISCARD variants of WRITE_SAME. Those are gated by the discard heuristics exclusively. So first of all I'd like to know whether it's a WRITE SAME(16) or a WRITE SAME(16) with the UNMAP bit set that's coming down. hch Ky: does hyperv support UNMAP? If so any idea why it doesn't set hch the maximum unmap block count field in the EVPD? We shouldn't be sending down WRITE SAME(16) with the UNMAP bit set unless the device sets LBPME=1 in the READ CAPACITY(16) response. So what does the storsvc report as its thin provisioning capabilities? Given that our current Host (ws2012 r2) advertises SPC-2 compliance, Linux does not even query this page. We support UNMAP. We just prototyped a host where we advertised SPC-3 compliance and Linux queries the EVPD page and does not use WRITE_SAME_16 K. Y ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, 2014-07-11 at 18:39 -0700, Greg KH wrote: On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote: A simple script to run checkpatch --fix for various types of of cleanups. ] drivers/staging/lustre/include/linux/lnet/api.h has no obvious style problems and is ready for submission. file: drivers/staging/lustre/include/linux/lnet/api.h description: update c90 comment style types:c99_comments Useless use of greediness modifier '+' in regex; marked by -- HERE in m/(^\+.*) {8,8}+ -- HERE \t/ at ./scripts/checkpatch.pl line 2217. [] drivers/staging/lustre/include/linux/lnet/api.h has no obvious style problems and is ready for submission. [] Is that expected? No, I haven't seen that. Can you tell me what git tree you're working on? Also, can you use the scripts/checkpatch from -next tag next-20140711 My system has: $ perl --version This is perl 5, version 18, subversion 2 (v5.18.2) built for i686-linux-gnu-thread-multi-64int (with 41 registered patches, see perl -V for more detail) Copyright 1987-2013, Larry Wall Perl may be copied only under the terms of either the Artistic License or the GNU General Public License, which may be found in the Perl 5 source kit. Complete documentation for Perl, including FAQ lists, should be found on this system using man perl or perldoc perl. If you have access to the Internet, point your browser at http://www.perl.org/, the Perl Home Page. and using that: $ ./scripts/checkpatch.pl -f --strict drivers/staging/lustre/include/linux/lnet/api.h --types=c99_comments total: 0 errors, 0 warnings, 0 checks, 220 lines checked NOTE: Used message types: C99_COMMENTS drivers/staging/lustre/include/linux/lnet/api.h has no obvious style problems and is ready for submission. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] reformat_with_checkpatch: Add automation to checkpatch
A simple script to run checkpatch --fix for various types of of cleanups. This script is useful primarily for staging. This reformats code to a more CodingStyle conforming style, compiles it, verifies that the object code hasn't changed, and git commits it too. You must have the necessary development tools, git, and a recent git tree. Ideally use Greg KH's staging-next, which can be retrieved via these commands: git clone git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git git checkout staging-next To use this script try a sequence of commands like: cd linux_repository git checkout -b your_branch make allyesconfig mkdir patches ./scripts/reformat_with_checkpatch.sh drivers/staging/dir/*.[ch] git format-patch --cover-letter -o patches/your_branch staging-next git send-email patches/your_branch Signed-off-by: Joe Perches j...@perches.com --- scripts/reformat_with_checkpatch.sh | 141 1 file changed, 141 insertions(+) create mode 100755 scripts/reformat_with_checkpatch.sh diff --git a/scripts/reformat_with_checkpatch.sh b/scripts/reformat_with_checkpatch.sh new file mode 100755 index 000..5415a8e --- /dev/null +++ b/scripts/reformat_with_checkpatch.sh @@ -0,0 +1,141 @@ +#!/bin/bash +# (c) 2014, Joe Perches j...@perches.com +# +# Automate checkpatch modifications and git commits to +# neaten code that doesn't conform to CodingStyle. +# Useful primarily for files in drivers/staging +# +# Licensed under the terms of the GNU GPL License version 2 + +declare -ar whitespace_types=( \ + 'whitespace_neatening:spacing,space_before_tab,pointer_location,trailing_whitespace,bracket_space' \ +'remove_spaces_before_tabs:space_before_tab' \ +'fix_label_positions:indented_label' \ +'align_arguments_to_parenthesis:parenthesis_alignment' \ +) + +declare -ar changecode_types=( \ +'fix_brace_positions:open_brace,braces,else_after_brace,while_after_brace' \ +'fix_blank_lines:line_spacing' \ +'use_standard_attributes:prefer_packed,prefer_aligned' \ +'remove_unnecessary_externs:avoid_externs' \ +'update_c90_comment_style:c99_comments'\ +) + +checkpatch_update () +{ +file=$1 + +desc=$(echo $2 | cut -f1 -d: | sed 's/_/ /g') +types=$(echo $2 | cut -f2- -d:) + +echo file: $file description: $desc types:$types + +./scripts/checkpatch.pl --file --fix-inplace --strict --types=$types $file + +checkpatch_fixes=$file.diff +git diff --stat -p --exit-code $file $checkpatch_fixes +if [ $? == 0 ] ; then + rm -f $checkpatch_fixes + return +fi + +basename=$(basename $file) +if [ ${basename##*.} == c ] ; then + + git checkout $file + obj=$(echo $file | sed 's/\.c$/\.o/') + if [ -e $obj ] ; then + rm -f $obj + fi + + echo Compiling original version... + + ${CROSS_COMPILE}make $obj + + ${CROSS_COMPILE}objdump -D $obj | \ + sed s/^[[:space:]]\+[0-9a-f]\+// $obj.old + + patch -p1 $checkpatch_fixes + + echo Compiling modified version... + + ${CROSS_COMPILE}make $obj + + ${CROSS_COMPILE}objdump -D $obj | \ + sed s/^[[:space:]]\+[0-9a-f]\+// $obj.new + + echo Comparing objects... + diff -Nurd $obj.new $obj.old + if [ $? -ne 0 ] ; then + echo Object differences exist! - Verify changes before commit! + read -s -p Press the 'enter' key to continue: + else + echo No object differences found + fi + rm -f $obj.old + rm -f $obj.new +fi + +echo running checkpatch on possible checkpatch fixes... + +./scripts/checkpatch.pl --no-summary --no-signoff $checkpatch_fixes +rm -f $checkpatch_fixes + +echo Verify checkpatch output and make any necessary changes +echo Edit '$file' if appropriate +read -s -p Press the 'enter' key to continue: + +commit_log_file=$(mktemp git_commit.XX) + +if [ -e $commit_log_file ] ; then + rm -f $commit_log_file +fi + +if [[ $file =~ ^drivers/staging/ ]] ; then + echo -n staging: $commit_log_file +fi +echo $(basename $(dirname $file)): checkpatch cleanup: $desc $commit_log_file +echo $commit_log_file +git diff --exit-code -w $file /dev/null +if [ $? == 0 ] ; then + echo whitespace changes only - git diff -w shows no difference $commit_log_file +fi + +git diff $file | cat + +echo Ready to commit - First verify all diffs and make any necessary changes +echo +read -r -p Would you like to commit these changes y: response +response=${response,,} +if [[ $response =~ ^(yes|y|)$ ]] ; then + git commit -s -F $commit_log_file -e $file +else + git checkout $file +fi + +rm -f $commit_log_file +} + +which git /dev/null +if [ $? -ne 0 ] ||