Re: [PATCH 2/2 v4] staging: comedi: addi_apci_1564: tidy up apci1564_reset()
>On Thursday, April 17, 2014 1:33 AM, Chase Southwood > wrote: >The reset function for this driver is a bit of a mess; clean it up to >ensure that it is functioning properly. > >Signed-off-by: Chase Southwood >--- >2: *Changed order of register accesses for digital input registers back to >original ordering. >*Removed read of digital output status register and reordered the accesses >of digital output registers to agree with the order found in addi_apci_2032. >*Fixed copy/paste error in the counter register reset lines. > >3: *Returned the reset callback to the boardinfo...oops. >*Don't write APCI1564_DO_IRQ_REG (it's read only). >*Don't clear counter reload values...counters have already been >disabled. > >4: Leave this in hwdrv_apci1564.c for now (it can be moved after separating >this driver from addi_common.c) > >NB: This patch has been renamed in this version. It was previously known >as "staging: comedi: addi_apci_1564: fixup and absorb apci1564_reset()" > Ian and Hartley, Just following up to see if you have any comments on this version of this patch. PATCH 1/2 from this set was added to staging-next today, and I'd like to know where this one stands before I work on separating this driver from addi_common.c. I'd be happy to make any more changes that are desired, or if it's just better to drop the patch for now, that's ok too. Just want to make sure it's taken care of before I move on! Thanks a lot, Chase ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 0/3] staging: gdm72xx: Minor cleanup
On Wed, Apr 23, 2014 at 08:39:06AM +0800, Michalis Pappas wrote: > After all patches have been applied, the only remaining issue on the > TODO list is to conform to the coding standards. The remaining issues > reported by checkpatch.pl are probably pedantic, so if agreed, that > task can be removed from the list too. So I did a: for i in $(find drivers/staging/gdm72xx/ -name \*.c) ; do ./scripts/checkpatch.pl --strict -f $i 2>&1 ; done | tee err-list I didn't look through all the issues but the ones I looked at seemed valid. The "no space after a cast" rule is good because cast precedence used to be a common source of bugs. The code in extract_qos_list() was badly mangled to get around the 80 character limit and checkpatch.pl finds that. That function is very ugly. Just reverse the if statements instead of jamming the code up against the far right side of the screen. if (!qcb->csr[i].enabled) continue; if (qcb->csr[i].qos_buf_count >= qcb->qos_limit_size) continue; if (list_empty(&qcb->qos_list[i])) continue; Also why is returning a u32? Kernel style is int. But this doesn't return errors and the callers don't check so it should be void. This driver suffers from prefering u32 over int in many places. This was my only static checker warning (harmless): drivers/staging/gdm72xx/gdm_wimax.c:780 gdm_wimax_transmit_pkt() warn: we tested 'len' before and it was 'true' In gdm_usb_send(), the "BUG_ON(len > TX_BUF_SIZE - padding - 1);" should be proper error handling. I didn't really do a proper review of this code when I saw all the checkpatch.pl complaints still. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: silicom: remove redundant pci_get_drvdata() call
The pci_get_drvdata() and checking NULL for dev are
called twice in while loop in is_bypass_dev().
Signed-off-by: Daeseok Youn
---
This patch has an warning from checkpatch.pl.
checkpatch.pl warning:
WARNING: Too many leading tabs - consider code refactoring
drivers/staging/silicom/bypasslib/bypass.c | 51 ---
1 files changed, 23 insertions(+), 28 deletions(-)
diff --git a/drivers/staging/silicom/bypasslib/bypass.c
b/drivers/staging/silicom/bypasslib/bypass.c
index 09e00da..a58251f 100644
--- a/drivers/staging/silicom/bypasslib/bypass.c
+++ b/drivers/staging/silicom/bypasslib/bypass.c
@@ -149,38 +149,33 @@ static int is_bypass_dev(int if_index)
while ((pdev = pci_get_class(PCI_CLASS_NETWORK_ETHERNET << 8, pdev))) {
dev = pci_get_drvdata(pdev);
- if (dev != NULL) {
- dev = pci_get_drvdata(pdev);
- if ((dev != NULL) && (dev->ifindex == if_index)) {
- if ((pdev->vendor == SILICOM_VID) &&
- (pdev->device >= SILICOM_BP_PID_MIN) &&
- (pdev->device <= SILICOM_BP_PID_MAX)) {
- goto send_cmd;
- }
+ if ((dev != NULL) && (dev->ifindex == if_index)) {
+ if ((pdev->vendor == SILICOM_VID) &&
+ (pdev->device >= SILICOM_BP_PID_MIN) &&
+ (pdev->device <= SILICOM_BP_PID_MAX)) {
+ goto send_cmd;
+ }
#if defined(BP_VENDOR_SUPPORT) && defined(ETHTOOL_GDRVINFO)
- else {
- struct ethtool_drvinfo info;
- const struct ethtool_ops *ops =
- dev->ethtool_ops;
- int k = 0;
-
- if (ops->get_drvinfo) {
- memset(&info, 0, sizeof(info));
- info.cmd = ETHTOOL_GDRVINFO;
- ops->get_drvinfo(dev, &info);
- for (; bp_desc_array[k]; k++)
- if (!
- (strcmp
-(bp_desc_array[k],
- info.driver)))
- goto send_cmd;
-
- }
+ else {
+ struct ethtool_drvinfo info;
+ const struct ethtool_ops *ops =
+ dev->ethtool_ops;
+ int k = 0;
+
+ if (ops->get_drvinfo) {
+ memset(&info, 0, sizeof(info));
+ info.cmd = ETHTOOL_GDRVINFO;
+ ops->get_drvinfo(dev, &info);
+ for (; bp_desc_array[k]; k++)
+ if (!(strcmp(bp_desc_array[k],
+info.driver)))
+ goto send_cmd;
}
-#endif
- return -1;
+
}
+#endif
+ return -1;
}
}
send_cmd:
--
1.7.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: silicom: remove redundant pci_get_drvdata() call
On Wed, Apr 23, 2014 at 05:18:42PM +0900, Daeseok Youn wrote: > The pci_get_drvdata() and checking NULL for dev are > called twice in while loop in is_bypass_dev(). > > Signed-off-by: Daeseok Youn > --- > This patch has an warning from checkpatch.pl. > checkpatch.pl warning: > WARNING: Too many leading tabs - consider code refactoring No problem. Those were there and worse before your patch. If someone wanted to fix these then just reverse every if condition in the function. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 09/10] staging: rtl8188eu: Remove unused enum LED_STATE_871x members.
On Wed, Apr 23, 2014 at 09:01:24AM +0530, navin patidar wrote: > dan, It is ok to rearrange and remove members of enum LED_STATE_871x > and enum LED_CTL_MODE. > these enum are not related to firmware . Ok. Cool. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 0/3] staging: gdm72xx: Minor cleanup
On 04/23/2014 04:04 PM, Dan Carpenter wrote:
> On Wed, Apr 23, 2014 at 08:39:06AM +0800, Michalis Pappas wrote:
>> After all patches have been applied, the only remaining issue on the
>> TODO list is to conform to the coding standards. The remaining issues
>> reported by checkpatch.pl are probably pedantic, so if agreed, that
>> task can be removed from the list too.
>
> So I did a:
> for i in $(find drivers/staging/gdm72xx/ -name \*.c) ; do
> ./scripts/checkpatch.pl --strict -f $i 2>&1 ; done | tee err-list
>
Hi Dan, thanks for looking at this. From the above snippet I realize that I
wasn't aware of the strict flag, so significantly less errors were produced.
The issues I was referring to as pedantic are:
WARNING: unchecked sscanf return value
#296: FILE: gdm_wimax.c:296:
+ sscanf(e->dev->name, "wm%d", &idx);
does this really need to be checked?
ERROR: Macros with complex values should be enclosed in parenthesis
#34: FILE: usb_ids.h:34:
+#define USB_DEVICE_BOOTLOADER(vid, pid)\
+ {USB_DEVICE((vid), ((pid)&BL_PID_MASK)|B_DOWNLOAD)},\
+ {USB_DEVICE((vid), ((pid)&BL_PID_MASK)|B_DOWNLOAD|B_DIFF_DL_DRV)}
these macros are only used for brevity in a subsequent array declaration, so it
seems that the parenthesis are not really needed.
Moreover, due to recent commits on checkpatch.pl, a few more issues are now
reported, even when not using the strict flag. In any case, I can re-run using
strict and submit an additional set of patches for the remaining issues.
Regards,
Michalis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 0/3] staging: gdm72xx: Minor cleanup
On Wed, Apr 23, 2014 at 04:49:26PM +0800, Michalis Pappas wrote:
> Hi Dan, thanks for looking at this. From the above snippet I realize
> that I wasn't aware of the strict flag, so significantly less errors
> were produced.
>
> The issues I was referring to as pedantic are:
>
> WARNING: unchecked sscanf return value
> #296: FILE: gdm_wimax.c:296:
> + sscanf(e->dev->name, "wm%d", &idx);
>
> does this really need to be checked?
Just check it. The code as is looks like a information leak (security
vulnerability) until you realize that e->dev->name is probably a known,
trusted string.
>
> ERROR: Macros with complex values should be enclosed in parenthesis
> #34: FILE: usb_ids.h:34:
> +#define USB_DEVICE_BOOTLOADER(vid, pid) \
> + {USB_DEVICE((vid), ((pid)&BL_PID_MASK)|B_DOWNLOAD)},\
> + {USB_DEVICE((vid), ((pid)&BL_PID_MASK)|B_DOWNLOAD|B_DIFF_DL_DRV)}
>
> these macros are only used for brevity in a subsequent array
> declaration, so it seems that the parenthesis are not really needed.
Yeah. You're right. Just ignore this one. Adding parenthis will break
the build.
checkpatch doesn't totally need to be happy.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 0/3] staging: gdm72xx: Minor cleanup
On Wed, Apr 23, 2014 at 12:05:57PM +0300, Dan Carpenter wrote: > On Wed, Apr 23, 2014 at 04:49:26PM +0800, Michalis Pappas wrote: > > Hi Dan, thanks for looking at this. From the above snippet I realize > > that I wasn't aware of the strict flag, so significantly less errors > > were produced. > > > > The issues I was referring to as pedantic are: > > > > WARNING: unchecked sscanf return value > > #296: FILE: gdm_wimax.c:296: > > + sscanf(e->dev->name, "wm%d", &idx); > > > > does this really need to be checked? > > Just check it. The code as is looks like a information leak (security > vulnerability) until you realize that e->dev->name is probably a known, > trusted string. Btw, we saw a "fix" for this earlier which just printed an error message. Don't do that. Assume that static checkers will soon start complaining about the info leak instead of just looking at sscanf(). regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2 v4] staging: comedi: addi_apci_1564: tidy up apci1564_reset()
On 2014-04-17 07:33, Chase Southwood wrote: The reset function for this driver is a bit of a mess; clean it up to ensure that it is functioning properly. Signed-off-by: Chase Southwood --- 2: *Changed order of register accesses for digital input registers back to original ordering. *Removed read of digital output status register and reordered the accesses of digital output registers to agree with the order found in addi_apci_2032. *Fixed copy/paste error in the counter register reset lines. 3: *Returned the reset callback to the boardinfo...oops. *Don't write APCI1564_DO_IRQ_REG (it's read only). *Don't clear counter reload values...counters have already been disabled. 4: Leave this in hwdrv_apci1564.c for now (it can be moved after separating this driver from addi_common.c) NB: This patch has been renamed in this version. It was previously known as "staging: comedi: addi_apci_1564: fixup and absorb apci1564_reset()" .../comedi/drivers/addi-data/hwdrv_apci1564.c | 24 ++ 1 file changed, 15 insertions(+), 9 deletions(-) Looks okay and it addresses the concerns raised by Hartley. Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: silicom: remove redundant pci_get_drvdata() call
2014-04-23 17:35 GMT+09:00, Dan Carpenter : > On Wed, Apr 23, 2014 at 05:18:42PM +0900, Daeseok Youn wrote: >> The pci_get_drvdata() and checking NULL for dev are >> called twice in while loop in is_bypass_dev(). >> >> Signed-off-by: Daeseok Youn >> --- >> This patch has an warning from checkpatch.pl. >> checkpatch.pl warning: >> WARNING: Too many leading tabs - consider code refactoring > > No problem. Those were there and worse before your patch. > > If someone wanted to fix these then just reverse every if condition in > the function. Ok. Thanks for comment. Regards, Daeseok Youn > > regards, > dan carpenter > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH][linux-next] staging/rtl8821ae: fix sparse address space warning
When unmapping the pci memory, the pointer was explicitly casted to void*,
thus omitting the __iomem designation.
Signed-off-by: Anders Darander
---
drivers/staging/rtl8821ae/pci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8821ae/pci.c b/drivers/staging/rtl8821ae/pci.c
index a562aa6..d934ecb 100644
--- a/drivers/staging/rtl8821ae/pci.c
+++ b/drivers/staging/rtl8821ae/pci.c
@@ -2416,7 +2416,7 @@ fail3:
ieee80211_free_hw(hw);
if (rtlpriv->io.pci_mem_start != 0)
- pci_iounmap(pdev, (void *)rtlpriv->io.pci_mem_start);
+ pci_iounmap(pdev, (void __iomem *)rtlpriv->io.pci_mem_start);
fail2:
pci_release_regions(pdev);
@@ -2479,7 +2479,7 @@ void rtl_pci_disconnect(struct pci_dev *pdev)
list_del(&rtlpriv->list);
if (rtlpriv->io.pci_mem_start != 0) {
- pci_iounmap(pdev, (void *)rtlpriv->io.pci_mem_start);
+ pci_iounmap(pdev, (void __iomem *)rtlpriv->io.pci_mem_start);
pci_release_regions(pdev);
}
--
2.0.0.rc0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH][linux-next] staging/rtl8821ae: fix sparse address space warning
On Wed, Apr 23, 2014 at 02:06:25PM +0200, Anders Darander wrote: > When unmapping the pci memory, the pointer was explicitly casted to void*, > thus omitting the __iomem designation. It looks like the struct definition should be updated instead of every single reference being casted. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH linux-next v2] staging/rtl8821ae: fix sparse address space warning
Change the definition of pci_mem_start|end from correct from pci_io(un)map's
point of view.
Signed-off-by: Anders Darander
---
Changes v1 -> v2:
* Change the struct definition instead of casting all
pci_iomap and pci_iounmap calls.
drivers/staging/rtl8821ae/pci.c | 14 +++---
drivers/staging/rtl8821ae/wifi.h | 4 ++--
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/rtl8821ae/pci.c b/drivers/staging/rtl8821ae/pci.c
index a562aa6..5306e66 100644
--- a/drivers/staging/rtl8821ae/pci.c
+++ b/drivers/staging/rtl8821ae/pci.c
@@ -2313,16 +2313,16 @@ int __devinit rtl_pci_probe(struct pci_dev *pdev,
/*shared mem start */
rtlpriv->io.pci_mem_start =
- (unsigned long)pci_iomap(pdev,
+ pci_iomap(pdev,
rtlpriv->cfg->bar_id, pmem_len);
- if (rtlpriv->io.pci_mem_start == 0) {
+ if (rtlpriv->io.pci_mem_start == NULL) {
RT_ASSERT(false, ("Can't map PCI mem\n"));
goto fail2;
}
RT_TRACE(COMP_INIT, DBG_DMESG,
("mem mapped space: start: 0x%08lx len:%08lx "
- "flags:%08lx, after map:0x%08lx\n",
+ "flags:%08lx, after map:0x%p\n",
pmem_start, pmem_len, pmem_flags,
rtlpriv->io.pci_mem_start));
@@ -2415,8 +2415,8 @@ fail3:
rtl_deinit_core(hw);
ieee80211_free_hw(hw);
- if (rtlpriv->io.pci_mem_start != 0)
- pci_iounmap(pdev, (void *)rtlpriv->io.pci_mem_start);
+ if (rtlpriv->io.pci_mem_start != NULL )
+ pci_iounmap(pdev, rtlpriv->io.pci_mem_start);
fail2:
pci_release_regions(pdev);
@@ -2478,8 +2478,8 @@ void rtl_pci_disconnect(struct pci_dev *pdev)
pci_disable_msi(rtlpci->pdev);
list_del(&rtlpriv->list);
- if (rtlpriv->io.pci_mem_start != 0) {
- pci_iounmap(pdev, (void *)rtlpriv->io.pci_mem_start);
+ if (rtlpriv->io.pci_mem_start != NULL) {
+ pci_iounmap(pdev, rtlpriv->io.pci_mem_start);
pci_release_regions(pdev);
}
diff --git a/drivers/staging/rtl8821ae/wifi.h b/drivers/staging/rtl8821ae/wifi.h
index 17a9d9f..e8250da 100644
--- a/drivers/staging/rtl8821ae/wifi.h
+++ b/drivers/staging/rtl8821ae/wifi.h
@@ -1086,8 +1086,8 @@ struct rtl_io {
struct device *dev;
/*PCI MEM map */
- unsigned long pci_mem_end; /*shared mem end*/
- unsigned long pci_mem_start;/*shared mem start */
+ void __iomem *pci_mem_end; /*shared mem end*/
+ void __iomem *pci_mem_start;/*shared mem start */
/*PCI IO map */
unsigned long pci_base_addr;/*device I/O address */
--
2.0.0.rc0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH linux-next v3] staging/rtl8821ae: fix sparse address space warning
Change the definition of pci_mem_start|end from correct from pci_io(un)map's
point of view.
Signed-off-by: Anders Darander
---
Changes v2 - v3
* Fix a style error
v1 -> v2:
* Change the struct definition instead of casting all
pci_iomap and pci_iounmap calls.
drivers/staging/rtl8821ae/pci.c | 14 +++---
drivers/staging/rtl8821ae/wifi.h | 4 ++--
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/rtl8821ae/pci.c b/drivers/staging/rtl8821ae/pci.c
index a562aa6..94f7283 100644
--- a/drivers/staging/rtl8821ae/pci.c
+++ b/drivers/staging/rtl8821ae/pci.c
@@ -2313,16 +2313,16 @@ int __devinit rtl_pci_probe(struct pci_dev *pdev,
/*shared mem start */
rtlpriv->io.pci_mem_start =
- (unsigned long)pci_iomap(pdev,
+ pci_iomap(pdev,
rtlpriv->cfg->bar_id, pmem_len);
- if (rtlpriv->io.pci_mem_start == 0) {
+ if (rtlpriv->io.pci_mem_start == NULL) {
RT_ASSERT(false, ("Can't map PCI mem\n"));
goto fail2;
}
RT_TRACE(COMP_INIT, DBG_DMESG,
("mem mapped space: start: 0x%08lx len:%08lx "
- "flags:%08lx, after map:0x%08lx\n",
+ "flags:%08lx, after map:0x%p\n",
pmem_start, pmem_len, pmem_flags,
rtlpriv->io.pci_mem_start));
@@ -2415,8 +2415,8 @@ fail3:
rtl_deinit_core(hw);
ieee80211_free_hw(hw);
- if (rtlpriv->io.pci_mem_start != 0)
- pci_iounmap(pdev, (void *)rtlpriv->io.pci_mem_start);
+ if (rtlpriv->io.pci_mem_start != NULL)
+ pci_iounmap(pdev, rtlpriv->io.pci_mem_start);
fail2:
pci_release_regions(pdev);
@@ -2478,8 +2478,8 @@ void rtl_pci_disconnect(struct pci_dev *pdev)
pci_disable_msi(rtlpci->pdev);
list_del(&rtlpriv->list);
- if (rtlpriv->io.pci_mem_start != 0) {
- pci_iounmap(pdev, (void *)rtlpriv->io.pci_mem_start);
+ if (rtlpriv->io.pci_mem_start != NULL) {
+ pci_iounmap(pdev, rtlpriv->io.pci_mem_start);
pci_release_regions(pdev);
}
diff --git a/drivers/staging/rtl8821ae/wifi.h b/drivers/staging/rtl8821ae/wifi.h
index 17a9d9f..e8250da 100644
--- a/drivers/staging/rtl8821ae/wifi.h
+++ b/drivers/staging/rtl8821ae/wifi.h
@@ -1086,8 +1086,8 @@ struct rtl_io {
struct device *dev;
/*PCI MEM map */
- unsigned long pci_mem_end; /*shared mem end*/
- unsigned long pci_mem_start;/*shared mem start */
+ void __iomem *pci_mem_end; /*shared mem end*/
+ void __iomem *pci_mem_start;/*shared mem start */
/*PCI IO map */
unsigned long pci_base_addr;/*device I/O address */
--
2.0.0.rc0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH linux-next v2] staging/rtl8821ae: fix sparse address space warning
On Wed, Apr 23, 2014 at 02:59:46PM +0200, Anders Darander wrote: > Change the definition of pci_mem_start|end from correct from pci_io(un)map's > point of view. > Terrific. Thanks! If you really wanted to then you could just delete pci_mem_end and also you could remove all the casting of pci_mem_start in drivers/staging/rtl8821ae/pci.h but this patch is an improvement on its own so someone can do that later if they want. Reviewed-by: Dan Carpenter regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
re: staging: add Lustre file system client support
Hello Peng Tao,
The patch d7e09d0397e8: "staging: add Lustre file system client
support" from May 2, 2013, leads to the following static checker
warning:
drivers/staging/lustre/include//linux/libcfs/libcfs_ioctl.h:200
libcfs_ioctl_is_invalid()
error: buffer overflow 'data->ioc_bulk' 896 <= 1073741823
drivers/staging/lustre/include//linux/libcfs/libcfs_ioctl.h
157 static inline int libcfs_ioctl_is_invalid(struct libcfs_ioctl_data
*data)
158 {
159 if (data->ioc_len > (1<<30)) {
160 CERROR ("LIBCFS ioctl: ioc_len larger than 1<<30\n");
161 return 1;
162 }
163 if (data->ioc_inllen1 > (1<<30)) {
164 CERROR ("LIBCFS ioctl: ioc_inllen1 larger than
1<<30\n");
We limit data->ioc_inllen1 to 1073741823 bytes here.
165 return 1;
166 }
167 if (data->ioc_inllen2 > (1<<30)) {
168 CERROR ("LIBCFS ioctl: ioc_inllen2 larger than
1<<30\n");
169 return 1;
170 }
171 if (data->ioc_inlbuf1 && !data->ioc_inllen1) {
172 CERROR ("LIBCFS ioctl: inlbuf1 pointer but 0 length\n");
173 return 1;
174 }
175 if (data->ioc_inlbuf2 && !data->ioc_inllen2) {
176 CERROR ("LIBCFS ioctl: inlbuf2 pointer but 0 length\n");
177 return 1;
178 }
179 if (data->ioc_pbuf1 && !data->ioc_plen1) {
180 CERROR ("LIBCFS ioctl: pbuf1 pointer but 0 length\n");
181 return 1;
182 }
183 if (data->ioc_pbuf2 && !data->ioc_plen2) {
184 CERROR ("LIBCFS ioctl: pbuf2 pointer but 0 length\n");
185 return 1;
186 }
187 if (data->ioc_plen1 && !data->ioc_pbuf1) {
188 CERROR ("LIBCFS ioctl: plen1 nonzero but no pbuf1
pointer\n");
189 return 1;
190 }
191 if (data->ioc_plen2 && !data->ioc_pbuf2) {
192 CERROR ("LIBCFS ioctl: plen2 nonzero but no pbuf2
pointer\n");
193 return 1;
194 }
195 if ((__u32)libcfs_ioctl_packlen(data) != data->ioc_len ) {
196 CERROR ("LIBCFS ioctl: packlen != ioc_len\n");
The idea was this would check it but this broken because we check
data->ioc_len in obd_ioctl_getdata() and then we do a second copy from
the user so the current value of ioc_len is unchecked.
197 return 1;
198 }
199 if (data->ioc_inllen1 &&
200 data->ioc_bulk[data->ioc_inllen1 - 1] != '\0') {
^
But data is 1024 byte struct with 896 bytes dedicated for ->ioc_bulk[]
so we are reading far beyond on the end of the array here. (Can cause
an oops).
The caller is obd_ioctl_getdata() and data->ioc_inllen1 comes directly
from the user. If we added this in obd_ioctl_getdata() then it would
fix the bug:
orig_len = hdr->ioc_len;
if (copy_from_user(buf, (void *)arg, hdr->ioc_len))
return -EFAULT; /* the original return code is buggy */
if (orig_len != hdr->ioc_len)
return -EINVAL;
if (libcfs_ioctl_is_invalid(data)) {
Why do we even have all the "> (1<<30)" checks? I don't understand.
Anything over 1024 is invalid.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: add Lustre file system client support
Btw, what's the trick to navigating the lustre source? I normally do
a make cscope but that doesn't work and I am having a very hard time
with this code.
regards,
dan carpenter
On Wed, Apr 23, 2014 at 04:54:26PM +0300, Dan Carpenter wrote:
> Hello Peng Tao,
>
> The patch d7e09d0397e8: "staging: add Lustre file system client
> support" from May 2, 2013, leads to the following static checker
> warning:
>
> drivers/staging/lustre/include//linux/libcfs/libcfs_ioctl.h:200
> libcfs_ioctl_is_invalid()
> error: buffer overflow 'data->ioc_bulk' 896 <= 1073741823
>
> drivers/staging/lustre/include//linux/libcfs/libcfs_ioctl.h
>157 static inline int libcfs_ioctl_is_invalid(struct libcfs_ioctl_data
> *data)
>158 {
>159 if (data->ioc_len > (1<<30)) {
>160 CERROR ("LIBCFS ioctl: ioc_len larger than 1<<30\n");
>161 return 1;
>162 }
>163 if (data->ioc_inllen1 > (1<<30)) {
>164 CERROR ("LIBCFS ioctl: ioc_inllen1 larger than
> 1<<30\n");
>
> We limit data->ioc_inllen1 to 1073741823 bytes here.
>
>165 return 1;
>166 }
>167 if (data->ioc_inllen2 > (1<<30)) {
>168 CERROR ("LIBCFS ioctl: ioc_inllen2 larger than
> 1<<30\n");
>169 return 1;
>170 }
>171 if (data->ioc_inlbuf1 && !data->ioc_inllen1) {
>172 CERROR ("LIBCFS ioctl: inlbuf1 pointer but 0
> length\n");
>173 return 1;
>174 }
>175 if (data->ioc_inlbuf2 && !data->ioc_inllen2) {
>176 CERROR ("LIBCFS ioctl: inlbuf2 pointer but 0
> length\n");
>177 return 1;
>178 }
>179 if (data->ioc_pbuf1 && !data->ioc_plen1) {
>180 CERROR ("LIBCFS ioctl: pbuf1 pointer but 0 length\n");
>181 return 1;
>182 }
>183 if (data->ioc_pbuf2 && !data->ioc_plen2) {
>184 CERROR ("LIBCFS ioctl: pbuf2 pointer but 0 length\n");
>185 return 1;
>186 }
>187 if (data->ioc_plen1 && !data->ioc_pbuf1) {
>188 CERROR ("LIBCFS ioctl: plen1 nonzero but no pbuf1
> pointer\n");
>189 return 1;
>190 }
>191 if (data->ioc_plen2 && !data->ioc_pbuf2) {
>192 CERROR ("LIBCFS ioctl: plen2 nonzero but no pbuf2
> pointer\n");
>193 return 1;
>194 }
>195 if ((__u32)libcfs_ioctl_packlen(data) != data->ioc_len ) {
>196 CERROR ("LIBCFS ioctl: packlen != ioc_len\n");
>
> The idea was this would check it but this broken because we check
> data->ioc_len in obd_ioctl_getdata() and then we do a second copy from
> the user so the current value of ioc_len is unchecked.
>
>197 return 1;
>198 }
>199 if (data->ioc_inllen1 &&
>200 data->ioc_bulk[data->ioc_inllen1 - 1] != '\0') {
> ^
>
> But data is 1024 byte struct with 896 bytes dedicated for ->ioc_bulk[]
> so we are reading far beyond on the end of the array here. (Can cause
> an oops).
>
> The caller is obd_ioctl_getdata() and data->ioc_inllen1 comes directly
> from the user. If we added this in obd_ioctl_getdata() then it would
> fix the bug:
>
> orig_len = hdr->ioc_len;
> if (copy_from_user(buf, (void *)arg, hdr->ioc_len))
> return -EFAULT; /* the original return code is buggy */
> if (orig_len != hdr->ioc_len)
> return -EINVAL;
>
> if (libcfs_ioctl_is_invalid(data)) {
>
> Why do we even have all the "> (1<<30)" checks? I don't understand.
> Anything over 1024 is invalid.
>
> regards,
> dan carpenter
> ___
> devel mailing list
> de...@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 01/02] staging: dgap: Fix how we insure config data is a string
This patch changes the way we insure the config data
is a string. Clearly this was just wrong. After a
certain number of loads/unloads various OOPs were
generated indicating something other than this driver
had a problem. It was this driver.
Signed-off-by: Mark Hounschell
Tested-by: Mark Hounschell
Reported-by: Mark Hounschell
Cc: Greg Kroah-Hartman
---
drivers/staging/dgap/dgap.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index 3c9278a..e0b8d0f 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -834,7 +834,7 @@ static int dgap_firmware_load(struct pci_dev *pdev, int
card_type)
return ret;
}
- dgap_config_buf = kmalloc(fw->size + 1, GFP_KERNEL);
+ dgap_config_buf = kzalloc(fw->size + 1, GFP_KERNEL);
if (!dgap_config_buf) {
release_firmware(fw);
return -ENOMEM;
@@ -842,7 +842,6 @@ static int dgap_firmware_load(struct pci_dev *pdev, int
card_type)
memcpy(dgap_config_buf, fw->data, fw->size);
release_firmware(fw);
- dgap_config_buf[fw->size + 1] = '\0';
if (dgap_parsefile(&dgap_config_buf, TRUE) != 0) {
kfree(dgap_config_buf);
--
1.8.4.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 02/02] staging: dgap: Fix BUG in processing config file
This patch fixes an OOPS caused by a pointer being
changed between the malloc and free.
Signed-off-by: Mark Hounschell
Tested-by: Mark Hounschell
Reported-by: Mark Hounschell
Cc: Greg Kroah-Hartman
---
drivers/staging/dgap/dgap.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index e0b8d0f..55a23b0 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -820,6 +820,7 @@ static int dgap_firmware_load(struct pci_dev *pdev, int
card_type)
{
struct board_t *brd = dgap_Board[dgap_NumBoards - 1];
const struct firmware *fw;
+ char *tmp_ptr;
int ret;
dgap_get_vpd(brd);
@@ -843,7 +844,14 @@ static int dgap_firmware_load(struct pci_dev *pdev, int
card_type)
memcpy(dgap_config_buf, fw->data, fw->size);
release_firmware(fw);
- if (dgap_parsefile(&dgap_config_buf, TRUE) != 0) {
+ /*
+* preserve dgap_config_buf
+* as dgap_parsefile would
+* otherwise alter it.
+*/
+ tmp_ptr = dgap_config_buf;
+
+ if (dgap_parsefile(&tmp_ptr, TRUE) != 0) {
kfree(dgap_config_buf);
return -EINVAL;
}
--
1.8.4.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 00/02] staging: dgap: Digi International dgap driver
This patch series fixes 2 different BUGS that didn't reveal themselves until the 3.15 series started. patch 1 fixes how we insure the config data is a string. The original code is clearly just wrong. After many loads/unloads of the driver, various OOPs would occure. None of which indicated this driver as the problem. patch 2 fixes the following OOPS. We were freeing memory with a pointer that had been changed between the malloc and free. kernel: [ 191.855797] dgap: module is from the staging directory, the quality is unknown, you have been warned. kernel: [ 191.856091] dgap-1.3-16, Digi International Part Number 40002347_C kernel: [ 191.856092] For the tools package please visit http://www.digi.com kernel: [ 191.856541] dgap: board 0: AccelePort Xr920 8 port (rev 1), irq 17 kernel: [ 191.874909] [ cut here ] kernel: [ 191.874914] WARNING: CPU: 0 PID: 3792 at kernel/rcu/tree.c:2470 __call_rcu.constprop.66+0x10c/0x1c0() kernel: [ 191.874915] Modules linked in: dgap(C+) bnep bluetooth rfkill 6lowpan_iphc nvidia(PO) kvm snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hwdep crc32_pclmul snd_pcm r8169 3c59x crc32c_intel snd_seq mii aesni_intel snd_timer snd_seq_device synclink_gt hdlc snd ablk_helper cryptd sr_mod cdrom gpiohsd(O) eprm(O) rfm2g(O) lrw soundcore shpchp tpm_infineon tpm_tis tpm sp5100_tco mxm_wmi rtom(O) aes_i586 pcspkr i2c_piix4 wmi serio_raw k10temp fam15h_power button xts sg ohci_pci gf128mul edd autofs4 xhci_hcd processor thermal_sys scsi_dh_rdac scsi_dh_emc scsi_dh_alua scsi_dh_hp_sw scsi_dh ata_generic aic7xxx aic79xx scsi_transport_spi pata_atiixp kernel: [ 191.874944] CPU: 0 PID: 3792 Comm: modprobe Tainted: P C O 3.14.0-lcrs #2 kernel: [ 191.874946] Hardware name: Gigabyte Technology Co., Ltd. GA-990FXA-UD5/GA-990FXA-UD5, BIOS F12 10/03/2013 kernel: [ 191.874947] d98cba54 c0712079 d98cba84 c024146f c08940d0 kernel: [ 191.874951] 0ed0 c089e845 09a6 c02938dc c02938dc c033fdf0 eab1c5c5 kernel: [ 191.874955] eab1c5c5 d98cba94 c02414ad 0009 d98cbab0 c02938dc c0960880 kernel: [ 191.874958] Call Trace: kernel: [ 191.874962] [] dump_stack+0x48/0x69 kernel: [ 191.874965] [] warn_slowpath_common+0x7f/0xa0 kernel: [ 191.874967] [] ? __call_rcu.constprop.66+0x10c/0x1c0 kernel: [ 191.874969] [] ? __call_rcu.constprop.66+0x10c/0x1c0 kernel: [ 191.874972] [] ? get_max_files+0x10/0x10 kernel: [ 191.874974] [] warn_slowpath_null+0x1d/0x20 kernel: [ 191.874976] [] __call_rcu.constprop.66+0x10c/0x1c0 kernel: [ 191.874977] [] call_rcu+0x17/0x20 kernel: [ 191.874979] [] put_filp+0x45/0x50 kernel: [ 191.874981] [] path_openat+0xdd/0x520 kernel: [ 191.874983] [] ? kmap_atomic+0xe/0x10 kernel: [ 191.874986] [] ? get_page_from_freelist+0x310/0x5a0 kernel: [ 191.874988] [] do_filp_open+0x30/0x80 kernel: [ 191.874991] [] ? string.isra.5+0x2f/0xa0 kernel: [ 191.874993] [] ? vsnprintf+0x1d6/0x400 kernel: [ 191.874995] [] file_open_name+0xe7/0x140 kernel: [ 191.874997] [] filp_open+0x29/0x30 kernel: [ 191.875000] [] _request_firmware+0x33d/0xa50 kernel: [ 191.875002] [] ? alloc_chrdev_region+0x16/0x30 kernel: [ 191.875004] [] request_firmware+0x2e/0x50 kernel: [ 191.875008] [] dgap_firmware_load+0x4be/0x12f0 [dgap] kernel: [ 191.875011] [] dgap_init_one+0x371/0x420 [dgap] kernel: [ 191.875014] [] pci_device_probe+0x71/0xc0 kernel: [ 191.875017] [] ? sysfs_create_link+0x20/0x40 kernel: [ 191.875020] [] driver_probe_device+0x74/0x360 kernel: [ 191.875022] [] ? pci_match_device+0xaa/0xc0 kernel: [ 191.875024] [] __driver_attach+0x89/0x90 kernel: [ 191.875026] [] ? driver_probe_device+0x360/0x360 kernel: [ 191.875028] [] bus_for_each_dev+0x42/0x80 kernel: [ 191.875030] [] driver_attach+0x19/0x20 kernel: [ 191.875031] [] ? driver_probe_device+0x360/0x360 kernel: [ 191.875033] [] bus_add_driver+0xe4/0x210 kernel: [ 191.875036] [] driver_register+0x54/0xe0 kernel: [ 191.875038] [] __pci_register_driver+0x2e/0x40 kernel: [ 191.875041] [] dgap_init_module+0x18d/0x240 [dgap] kernel: [ 191.875044] [] ? dgap_cleanup_module+0x2c0/0x2c0 [dgap] kernel: [ 191.875046] [] do_one_initcall+0xb2/0x160 kernel: [ 191.875049] [] ? 0xef872fff kernel: [ 191.875051] [] ? set_memory_ro+0x32/0x40 kernel: [ 191.875053] [] ? set_section_ro_nx+0x4f/0x54 kernel: [ 191.875056] [] load_module+0x1b11/0x23d0 kernel: [ 191.875059] [] ? 0xef872fff kernel: [ 191.875064] [] ? error_code+0x5a/0x60 kernel: [ 191.875066] [] SyS_init_module+0x97/0x100 kernel: [ 191.875072] [] syscall_call+0x7/0xb kernel: [ 191.875074] ---[ end trace d8632c9a4b07ee1b ]--- -- 1.8.1.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 2/2 v4] staging: comedi: addi_apci_1564: tidy up apci1564_reset()
On Wednesday, April 23, 2014 12:09 AM, Chase Southwood wrote: >>On Thursday, April 17, 2014 1:33 AM, Chase Southwood >> wrote: > >> The reset function for this driver is a bit of a mess; clean it up to >> ensure that it is functioning properly. >> >> Signed-off-by: Chase Southwood >> --- >> 2: *Changed order of register accesses for digital input registers back to >> original ordering. >> *Removed read of digital output status register and reordered the accesses >> of digital output registers to agree with the order found in addi_apci_2032. >> *Fixed copy/paste error in the counter register reset lines. >> >> 3: *Returned the reset callback to the boardinfo...oops. >> *Don't write APCI1564_DO_IRQ_REG (it's read only). >> *Don't clear counter reload values...counters have already been >> disabled. >> >> 4: Leave this in hwdrv_apci1564.c for now (it can be moved after separating >> this driver from addi_common.c) >> >>NB: This patch has been renamed in this version. It was previously known >>as "staging: comedi: addi_apci_1564: fixup and absorb apci1564_reset()" >> > > > Ian and Hartley, > > Just following up to see if you have any comments on this version of this > patch. > PATCH 1/2 from this set was added to staging-next today, and I'd like to know > where this one stands before I work on separating this driver from > addi_common.c. > I'd be happy to make any more changes that are desired, or if it's just > better to > drop the patch for now, that's ok too. Just want to make sure it's taken > care of > before I move on! Sorry, I thought I already responded to this... Looks good now. Reviewed-by: H Hartley Sweeten ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging/android: Remove ram_console.h
On Tue, Apr 22, 2014 at 2:22 AM, Bintian Wang wrote:
> ram_console is replaced by pstore and pstore_ram drivers,
> and there is no code to use this head file, so remove it.
>
> Signed-off-by: Bintian Wang
> ---
> drivers/staging/android/ram_console.h | 22 --
> 1 file changed, 22 deletions(-)
> delete mode 100644 drivers/staging/android/ram_console.h
>
> diff --git a/drivers/staging/android/ram_console.h
> b/drivers/staging/android/ram_console.h
> deleted file mode 100644
> index 9f1125c..000
> --- a/drivers/staging/android/ram_console.h
> +++ /dev/null
> @@ -1,22 +0,0 @@
> -/*
> - * Copyright (C) 2010 Google, Inc.
> - *
> - * This software is licensed under the terms of the GNU General Public
> - * License version 2, as published by the Free Software Foundation, and
> - * may be copied, distributed, and modified under those terms.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - *
> - */
> -
> -#ifndef _INCLUDE_LINUX_PLATFORM_DATA_RAM_CONSOLE_H_
> -#define _INCLUDE_LINUX_PLATFORM_DATA_RAM_CONSOLE_H_
> -
> -struct ram_console_platform_data {
> - const char *bootinfo;
> -};
> -
> -#endif /* _INCLUDE_LINUX_PLATFORM_DATA_RAM_CONSOLE_H_ */
> --
> 1.7.9.5
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
Acked-by: Colin Cross
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/2] staging: panel: fix sparse warnings in lcd_write
On Tue, Apr 22, 2014 at 01:01:45PM +0300, Dan Carpenter wrote:
> Out of curiosity, have you tested this patch?
>
> On Fri, Apr 18, 2014 at 06:10:57PM +0200, Bastien Armand wrote:
> > This patch fixes two sparse warnings related to lcd_write :
> > warning: incorrect type in argument 1 (different address spaces)
> > warning: incorrect type in initializer (incompatible argument 2
> > (different address spaces))
> >
> > lcd_write can be used from kernel space (in panel_lcd_print) or from user
> > space. So we introduce the lcd_write_char function that will write a char to
> > the device. We modify lcd_write and panel_lcd_print to use it. Finally we
> > add
> > __user annotation missing in lcd_write.
> >
> >
> > Signed-off-by: Bastien Armand
> > ---
> > drivers/staging/panel/panel.c | 205
> > ++---
> > 1 file changed, 109 insertions(+), 96 deletions(-)
> >
> > diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
> > index 1569e26..dc34254 100644
> > --- a/drivers/staging/panel/panel.c
> > +++ b/drivers/staging/panel/panel.c
> > @@ -1217,111 +1217,113 @@ static inline int handle_lcd_special_code(void)
> > return processed;
> > }
> > ...
>
> This is buggy. You are getting the first character over and over. You
> should be doing:
>
> if (get_user(c, tmp))
> return -EFAULT;
>
Hi,
You're totally right. I am sorry to have missed that... As the patch has
been accepted in linux-next. I will send a correction right away.
> Btw, this whole function is terrible. It should be reading larger
> chunks at once instead of get_user() for each character.
>
>
The aim of my patch was basically to add __user annotation. I tried to
keep the change minimal to lessen the risk of regression (it seems I have
failed here...). Perhaps, I can write an other patch to change that.
> > ...
> > static void panel_lcd_print(const char *s)
> > {
> > - if (lcd_enabled && lcd_initialized)
> > - lcd_write(NULL, s, strlen(s), NULL);
> > + const char *tmp = s;
> > + int count = strlen(s);
> > +
> > + if (lcd_enabled && lcd_initialized) {
> > + for (; count-- > 0; tmp++) {
> > + if (!in_interrupt() && (((count + 1) & 0x1f) == 0))
> > + /* let's be a little nice with other processes
> > + that need some CPU */
> > + schedule();
>
> This schedule() isn't needed. It just prints a line or two at start up
> and some other debug output or something. Small small.
>
I hesitated to remove it. I leave it here as it was allready in lcd_write.
Perhaps, I could send another patch to remove it.
> regards,
> dan carpenter
>
Thanks for your review. It was really appreciated.
Regards,
bastien armand
> > +
> > + lcd_write_char(*tmp);
> > + }
> > + }
> > }
> >
> > /* initialize the LCD driver */
> > --
> > 1.7.10.4
> >
> > ___
> > devel mailing list
> > de...@linuxdriverproject.org
> > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: panel: fix regression in lcd_write
This patch fix a regression in lcd_write caused by commit 70a8c3eb8546cefe40fb0bc7991e8899b7b91075 Signed-off-by: Bastien Armand --- drivers/staging/panel/panel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c index 136671a..acd4b8e 100644 --- a/drivers/staging/panel/panel.c +++ b/drivers/staging/panel/panel.c @@ -1324,7 +1324,7 @@ static ssize_t lcd_write(struct file *file, that need some CPU */ schedule(); - if (get_user(c, buf)) + if (get_user(c, tmp)) return -EFAULT; lcd_write_char(c); -- 1.8.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 0/2] Eliminate spin locks in the vmbus channel callback path
> -Original Message- > From: K. Y. Srinivasan [mailto:k...@microsoft.com] > Sent: Tuesday, April 8, 2014 6:45 PM > To: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > jasow...@redhat.com > Cc: KY Srinivasan > Subject: [PATCH 0/2] Eliminate spin locks in the vmbus channel callback path > > Currently we map the channel handle to the channel under the protection of > a spin lock. Additionally, we dispatch the channel callback function under the > protection of the channel inbound lock (another spin lock). In some recent > nework performance runs, the time spent acquiring and releasing these locks > were identified as potential bottlenecks. This patch-set gets rid of these > locks > by leveraging the interrupt bindings that we support starting with win8. > > K. Y. Srinivasan (2): > Drivers: hv: Eliminate the channel spinlock in the callback path > Drivers: hv: vmbus: Implement per-CPU mapping of relid to channel > > drivers/hv/channel.c | 16 ++--- > drivers/hv/channel_mgmt.c | 52 > > drivers/hv/connection.c | 35 +++--- > drivers/hv/hv.c |2 + > drivers/hv/hyperv_vmbus.h |5 > include/linux/hyperv.h|7 ++ > 6 files changed, 100 insertions(+), 17 deletions(-) > > -- > 1.7.4.1 Greg, Should I resend this patch set. K. Y ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net-next,v2,1/2] hyperv: Remove recv_pkt_list and lock
From: Haiyang Zhang Date: Mon, 21 Apr 2014 14:54:43 -0700 > Removed recv_pkt_list and lock, and updated related code, so that > the locking overhead is reduced especially when multiple channels > are in use. > > The recv_pkt_list isn't actually necessary because the packets are > processed sequentially in each channel. It has been replaced by a > local variable, and the related lock for this list is also removed. > The is_data_pkt field is not used in receive path, so its assignment > is cleaned up. > > Signed-off-by: Haiyang Zhang > Reviewed-by: K. Y. Srinivasan Applied. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net-next,v2,2/2] hyperv: Simplify the send_completion variables
From: Haiyang Zhang Date: Mon, 21 Apr 2014 14:54:44 -0700 > The union contains only one member now, so we use the variables in it > directly. > > Signed-off-by: Haiyang Zhang > Reviewed-by: K. Y. Srinivasan Also applied, thanks. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: unisys: fix copyright notices
This patch changes all of the various representations of the copyright symbol to (C). Signed-off-by: Benjamin Romer --- drivers/staging/unisys/channels/channel.c | 2 +- drivers/staging/unisys/channels/chanstub.c | 2 +- drivers/staging/unisys/channels/chanstub.h | 2 +- drivers/staging/unisys/common-spar/include/channels/channel.h | 2 +- drivers/staging/unisys/common-spar/include/channels/channel_guid.h | 2 +- drivers/staging/unisys/common-spar/include/channels/controlframework.h | 2 +- drivers/staging/unisys/common-spar/include/channels/controlvmchannel.h | 2 +- drivers/staging/unisys/common-spar/include/channels/diagchannel.h | 2 +- drivers/staging/unisys/common-spar/include/channels/iochannel.h | 2 +- drivers/staging/unisys/common-spar/include/channels/vbuschannel.h | 2 +- drivers/staging/unisys/common-spar/include/controlvmcompletionstatus.h | 2 +- .../staging/unisys/common-spar/include/diagnostics/appos_subsystems.h | 2 +- drivers/staging/unisys/common-spar/include/iovmcall_gnuc.h | 2 +- drivers/staging/unisys/common-spar/include/vbusdeviceinfo.h | 2 +- drivers/staging/unisys/common-spar/include/version.h| 2 +- drivers/staging/unisys/common-spar/include/vmcallinterface.h| 2 +- drivers/staging/unisys/include/commontypes.h| 2 +- drivers/staging/unisys/include/guestlinuxdebug.h| 2 +- drivers/staging/unisys/include/guidutils.h | 2 +- drivers/staging/unisys/include/periodic_work.h | 2 +- drivers/staging/unisys/include/procobjecttree.h | 2 +- drivers/staging/unisys/include/sparstop.h | 2 +- drivers/staging/unisys/include/timskmod.h | 2 +- drivers/staging/unisys/include/timskmodutils.h | 2 +- drivers/staging/unisys/include/uisqueue.h | 2 +- drivers/staging/unisys/include/uisthread.h | 2 +- drivers/staging/unisys/include/uisutils.h | 2 +- drivers/staging/unisys/include/uniklog.h| 2 +- drivers/staging/unisys/uislib/uislib.c | 2 +- drivers/staging/unisys/uislib/uisqueue.c| 2 +- drivers/staging/unisys/uislib/uisthread.c | 2 +- drivers/staging/unisys/uislib/uisutils.c| 2 +- drivers/staging/unisys/virthba/virthba.c| 2 +- drivers/staging/unisys/virthba/virthba.h| 2 +- drivers/staging/unisys/virtpci/virtpci.c| 2 +- drivers/staging/unisys/virtpci/virtpci.h| 2 +- drivers/staging/unisys/visorchannel/globals.h | 2 +- drivers/staging/unisys/visorchannel/visorchannel.h | 2 +- drivers/staging/unisys/visorchannel/visorchannel_funcs.c| 2 +- drivers/staging/unisys/visorchannel/visorchannel_main.c | 2 +- drivers/staging/unisys/visorchipset/controlvm.h | 2 +- drivers/staging/unisys/visorchipset/controlvm_direct.c | 2 +- drivers/staging/unisys/visorchipset/file.c | 2 +- drivers/staging/unisys/visorchipset/file.h | 2 +- drivers/staging/unisys/visorchipset/globals.h | 2 +- drivers/staging/unisys/visorchipset/parser.c| 2 +- drivers/staging/unisys/visorchipset/parser.h| 2 +- drivers/staging/unisys/visorchipset/testing.h | 2 +- drivers/staging/unisys/visorchipset/visorchipset.h | 2 +- drivers/staging/unisys/visorchipset/visorchipset_main.c | 2 +- drivers/staging/unisys/visorchipset/visorchipset_umode.h| 2 +- drivers/staging/unisys/visorutil/charqueue.c| 2 +- drivers/staging/unisys/visorutil/charqueue.h| 2 +- drivers/staging/unisys/visorutil/easyproc.c | 2 +- drivers/staging/unisys/visorutil/easyproc.h | 2 +- drivers/staging/unisys/visorutil/memregion.h| 2 +- drivers/staging/unisys/visorutil/memregion_direct.c | 2 +- drivers/staging/unisys/visorutil/periodic_work.c| 2 +- drivers/staging/unisys/visorutil/procobjecttree.c | 2 +- drivers/staging/unisys/visorutil/visorkmodutils.c | 2 +- 60 files changed, 60 insertions(+), 60 deletions(-) diff --git a/drivers/st
Re: [PATCH net-next 1/1] hyperv: Enable sendbuf mechanism on the send path
From: "K. Y. Srinivasan" Date: Tue, 22 Apr 2014 11:03:32 -0700 > + skb = (struct sk_buff *) > + packet->send_completion_tid; As in netvsc_xmit_completion() this must be coded as: skb = (struct sk_buff *) (unsigned long)packet->send_completion_tid; ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/2] staging: panel: fix sparse warnings in lcd_write
On Wed, Apr 23, 2014 at 07:35:02PM +0200, Bastien Armand wrote: > On Tue, Apr 22, 2014 at 01:01:45PM +0300, Dan Carpenter wrote: > The aim of my patch was basically to add __user annotation. I tried to > keep the change minimal to lessen the risk of regression Yes. That's the right approach. These comments are just a free bonus in case you want to do more work on this. Or you can ignore if you don't want to. It's not something you introduced so it's not your responsibility. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: dgap: fix sparse warnings for re_map_membase and re_map_port
This patch fixes sparse warnings for the
re_map_membase and re_map_port variables.
Signed-off-by: Mark Hounschell
Cc: Greg Kroah-Hartman
---
drivers/staging/dgap/dgap.c | 24
drivers/staging/dgap/dgap.h | 4 ++--
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index 55a23b0..38db01a 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -1300,7 +1300,7 @@ static int dgap_tty_init(struct board_t *brd)
int i;
int tlw;
uint true_count = 0;
- u8 *vaddr;
+ u8 __iomem *vaddr;
u8 modem = 0;
struct channel_t *ch;
struct bs_t *bs;
@@ -4310,7 +4310,7 @@ static int dgap_tty_register_ports(struct board_t *brd)
*/
static void dgap_do_bios_load(struct board_t *brd, const u8 *ubios, int len)
{
- u8 *addr;
+ u8 __iomem *addr;
uint offset;
int i;
@@ -4343,7 +4343,7 @@ static void dgap_do_bios_load(struct board_t *brd, const
u8 *ubios, int len)
*/
static int dgap_test_bios(struct board_t *brd)
{
- u8 *addr;
+ u8 __iomem *addr;
u16 word;
u16 err1;
u16 err2;
@@ -4386,7 +4386,7 @@ static int dgap_test_bios(struct board_t *brd)
*/
static void dgap_do_fep_load(struct board_t *brd, const u8 *ufep, int len)
{
- u8 *addr;
+ u8 __iomem *addr;
uint offset;
if (!brd || (brd->magic != DGAP_BOARD_MAGIC) || !brd->re_map_membase)
@@ -4430,7 +4430,7 @@ static void dgap_do_fep_load(struct board_t *brd, const
u8 *ufep, int len)
*/
static int dgap_test_fep(struct board_t *brd)
{
- u8 *addr;
+ u8 __iomem *addr;
u16 word;
u16 err1;
u16 err2;
@@ -4529,7 +4529,7 @@ static void dgap_do_reset_board(struct board_t *brd)
*/
static void dgap_do_conc_load(struct board_t *brd, u8 *uaddr, int len)
{
- char *vaddr;
+ char __iomem *vaddr;
u16 offset = 0;
struct downld_t *to_dp;
@@ -4667,7 +4667,7 @@ static void dgap_poll_tasklet(unsigned long data)
{
struct board_t *bd = (struct board_t *) data;
ulong lock_flags;
- char *vaddr;
+ char __iomem *vaddr;
u16 head, tail;
if (!bd || (bd->magic != DGAP_BOARD_MAGIC))
@@ -4743,7 +4743,7 @@ out:
static void dgap_cmdb(struct channel_t *ch, u8 cmd, u8 byte1,
u8 byte2, uint ncmds)
{
- char*vaddr = NULL;
+ char __iomem*vaddr;
struct cm_t *cm_addr = NULL;
uintcount;
uintn;
@@ -4828,7 +4828,7 @@ static void dgap_cmdb(struct channel_t *ch, u8 cmd, u8
byte1,
*===*/
static void dgap_cmdw(struct channel_t *ch, u8 cmd, u16 word, uint ncmds)
{
- char*vaddr = NULL;
+ char __iomem*vaddr;
struct cm_t *cm_addr = NULL;
uintcount;
uintn;
@@ -4911,7 +4911,7 @@ static void dgap_cmdw(struct channel_t *ch, u8 cmd, u16
word, uint ncmds)
*===*/
static void dgap_cmdw_ext(struct channel_t *ch, u16 cmd, u16 word, uint ncmds)
{
- char*vaddr = NULL;
+ char __iomem*vaddr;
struct cm_t *cm_addr = NULL;
uintcount;
uintn;
@@ -5058,7 +5058,7 @@ static void dgap_wmove(struct channel_t *ch, char *buf,
uint cnt)
*/
static uint dgap_get_custom_baud(struct channel_t *ch)
{
- u8 *vaddr;
+ u8 __iomem *vaddr;
ulong offset = 0;
uint value = 0;
@@ -5580,7 +5580,7 @@ static int dgap_event(struct board_t *bd)
ulong lock_flags2;
struct bs_t *bs;
u8 *event;
- u8 *vaddr = NULL;
+ u8 __iomem *vaddr;
struct ev_t *eaddr = NULL;
uinthead;
uinttail;
diff --git a/drivers/staging/dgap/dgap.h b/drivers/staging/dgap/dgap.h
index b23570b..72b3d0c 100644
--- a/drivers/staging/dgap/dgap.h
+++ b/drivers/staging/dgap/dgap.h
@@ -571,8 +571,8 @@ struct board_t {
ulong membase;/* Start of base memory of the card */
ulong membase_end;/* End of base memory of the card */
- u8 *re_map_port; /* Remapped io port of the card */
- u8 *re_map_membase;/* Remapped memory of the card */
+ u8 __iomem *re_map_port; /* Remapped io port of the card */
+ u8 __iomem *re_map_membase;/* Remapped memory of the card */
u8 runwait;/* # Processes waiting for FEP */
u8 inhibit_poller; /* Tells the poller to leave us alone */
--
1.8.4.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverp
[PATCH 1/1] Drivers: hv: balloon: Ensure pressure reports are posted regularly
The current code posts periodic memory pressure status from a dedicated thread.
Under some conditions, especially when we are releasing a lot of memory into
the guest, we may not send timely pressure reports back to the host. Fix this
issue by reporting pressure in all contexts that can be active in this driver.
Signed-off-by: K. Y. Srinivasan
Cc: sta...@vger.kernel.org
---
drivers/hv/hv_balloon.c | 29 ++---
1 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 7e6d78d..5e90c5d 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -19,6 +19,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include
+#include
#include
#include
#include
@@ -459,6 +460,11 @@ static bool do_hot_add;
*/
static uint pressure_report_delay = 45;
+/*
+ * The last time we posted a pressure report to host.
+ */
+static unsigned long last_post_time;
+
module_param(hot_add, bool, (S_IRUGO | S_IWUSR));
MODULE_PARM_DESC(hot_add, "If set attempt memory hot_add");
@@ -542,6 +548,7 @@ struct hv_dynmem_device {
static struct hv_dynmem_device dm_device;
+static void post_status(struct hv_dynmem_device *dm);
#ifdef CONFIG_MEMORY_HOTPLUG
static void hv_bring_pgs_online(unsigned long start_pfn, unsigned long size)
@@ -612,7 +619,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned
long size,
* have not been "onlined" within the allowed time.
*/
wait_for_completion_timeout(&dm_device.ol_waitevent, 5*HZ);
-
+ post_status(&dm_device);
}
return;
@@ -951,11 +958,17 @@ static void post_status(struct hv_dynmem_device *dm)
{
struct dm_status status;
struct sysinfo val;
+ unsigned long now = jiffies;
+ unsigned long last_post = last_post_time;
if (pressure_report_delay > 0) {
--pressure_report_delay;
return;
}
+
+ if (!time_after(now, (last_post_time + HZ)))
+ return;
+
si_meminfo(&val);
memset(&status, 0, sizeof(struct dm_status));
status.hdr.type = DM_STATUS_REPORT;
@@ -983,6 +996,14 @@ static void post_status(struct hv_dynmem_device *dm)
if (status.hdr.trans_id != atomic_read(&trans_id))
return;
+ /*
+* If the last post time that we sampled has changed,
+* we have raced, don't post the status.
+*/
+ if (last_post != last_post_time)
+ return;
+
+ last_post_time = jiffies;
vmbus_sendpacket(dm->dev->channel, &status,
sizeof(struct dm_status),
(unsigned long)NULL,
@@ -1117,7 +1138,7 @@ static void balloon_up(struct work_struct *dummy)
if (ret == -EAGAIN)
msleep(20);
-
+ post_status(&dm_device);
} while (ret == -EAGAIN);
if (ret) {
@@ -1144,8 +1165,10 @@ static void balloon_down(struct hv_dynmem_device *dm,
struct dm_unballoon_response resp;
int i;
- for (i = 0; i < range_count; i++)
+ for (i = 0; i < range_count; i++) {
free_balloon_pages(dm, &range_array[i]);
+ post_status(&dm_device);
+ }
if (req->more_pages == 1)
return;
--
1.7.4.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: dgap: fix sparse warnings for the entire bs_t structure
This patch fixes sparse warnings for the entire bs_t structure
This entire structure defines a hardware segment
Signed-off-by: Mark Hounschell
Tested-by: Mark Hounschell
Cc: Greg Kroah-Hartman
---
drivers/staging/dgap/dgap.c | 22 +++---
drivers/staging/dgap/dgap.h | 4 ++--
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index 38db01a..0bf22f7 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -1303,7 +1303,7 @@ static int dgap_tty_init(struct board_t *brd)
u8 __iomem *vaddr;
u8 modem = 0;
struct channel_t *ch;
- struct bs_t *bs;
+ struct bs_t __iomem *bs;
struct cm_t *cm;
if (!brd)
@@ -1635,7 +1635,7 @@ static void dgap_sniff_nowait_nolock(struct channel_t
*ch, u8 *text,
static void dgap_input(struct channel_t *ch)
{
struct board_t *bd;
- struct bs_t *bs;
+ struct bs_t __iomem *bs;
struct tty_struct *tp;
struct tty_ldisc *ld;
uintrmask;
@@ -1962,7 +1962,7 @@ static int dgap_tty_open(struct tty_struct *tty, struct
file *file)
struct board_t *brd;
struct channel_t *ch;
struct un_t *un;
- struct bs_t *bs;
+ struct bs_t __iomem *bs;
uintmajor = 0;
uintminor = 0;
int rc = 0;
@@ -2416,7 +2416,7 @@ static int dgap_tty_chars_in_buffer(struct tty_struct
*tty)
struct board_t *bd = NULL;
struct channel_t *ch = NULL;
struct un_t *un = NULL;
- struct bs_t *bs = NULL;
+ struct bs_t __iomem *bs;
u8 tbusy;
uint chars = 0;
u16 thead, ttail, tmask, chead, ctail;
@@ -2507,7 +2507,7 @@ static int dgap_wait_for_drain(struct tty_struct *tty)
{
struct channel_t *ch;
struct un_t *un;
- struct bs_t *bs;
+ struct bs_t __iomem *bs;
int ret = -EIO;
uint count = 1;
ulong lock_flags = 0;
@@ -2618,7 +2618,7 @@ static int dgap_maxcps_room(struct tty_struct *tty, int
bytes_available)
static inline void dgap_set_firmware_event(struct un_t *un, unsigned int event)
{
struct channel_t *ch = NULL;
- struct bs_t *bs = NULL;
+ struct bs_t __iomem *bs = NULL;
if (!un || un->magic != DGAP_UNIT_MAGIC)
return;
@@ -2652,7 +2652,7 @@ static int dgap_tty_write_room(struct tty_struct *tty)
{
struct channel_t *ch = NULL;
struct un_t *un = NULL;
- struct bs_t *bs = NULL;
+ struct bs_t __iomem *bs;
u16 head, tail, tmask;
int ret = 0;
ulong lock_flags = 0;
@@ -2741,7 +2741,7 @@ static int dgap_tty_write(struct tty_struct *tty, const
unsigned char *buf,
{
struct channel_t *ch = NULL;
struct un_t *un = NULL;
- struct bs_t *bs = NULL;
+ struct bs_t __iomem *bs;
char *vaddr = NULL;
u16 head, tail, tmask, remain;
int bufcount = 0, n = 0;
@@ -5006,7 +5006,7 @@ static void dgap_wmove(struct channel_t *ch, char *buf,
uint cnt)
{
intn;
char *taddr;
- struct bs_t*bs;
+ struct bs_t __iomem *bs;
u16head;
if (!ch || ch->magic != DGAP_CHANNEL_MAGIC)
@@ -5125,7 +5125,7 @@ static int dgap_param(struct tty_struct *tty)
struct ktermios *ts;
struct board_t *bd;
struct channel_t *ch;
- struct bs_t *bs;
+ struct bs_t __iomem *bs;
struct un_t *un;
u16 head;
u16 cflag;
@@ -5578,7 +5578,7 @@ static int dgap_event(struct board_t *bd)
struct channel_t *ch;
ulong lock_flags;
ulong lock_flags2;
- struct bs_t *bs;
+ struct bs_t __iomem *bs;
u8 *event;
u8 __iomem *vaddr;
struct ev_t *eaddr = NULL;
diff --git a/drivers/staging/dgap/dgap.h b/drivers/staging/dgap/dgap.h
index 72b3d0c..8d75d32 100644
--- a/drivers/staging/dgap/dgap.h
+++ b/drivers/staging/dgap/dgap.h
@@ -593,7 +593,7 @@ struct board_t {
u32 dgap_Serial_Major;
u32 dgap_TransparentPrint_Major;
- struct bs_t *bd_bs; /* Base structure pointer */
+ struct bs_t __iomem *bd_bs; /* Base structure pointer */
char*flipbuf; /* Our flip buffer, alloced if*/
/* board is found */
@@ -968,7 +968,7 @@ struct digi_cmd {
/
struct channel_t {
int magic; /* Channel Magic Number */
- struct bs_t *ch_bs; /* Base structure pointer */
+ struct bs_t __iomem *ch_bs; /* Base structure pointer */
struct cm_t *ch_cm; /* Command queue pointer*/
struct board_t
[PATCH V1 net-next 1/1] hyperv: Enable sendbuf mechanism on the send path
We send packets using a copy-free mechanism (this is the Guest to Host transport
via VMBUS). While this is obviously optimal for large packets,
it may not be optimal for small packets. Hyper-V host supports
a second mechanism for sending packets that is "copy based". We implement that
mechanism in this patch.
In this version of the patch I have addressed a comment from David Miller.
With this patch (and all of the other offload and VRSS patches), we are now able
to almost saturate a 10G interface between Linux VMs on Hyper-V
on different hosts - close to 9 Gbps as measured via iperf.
Signed-off-by: K. Y. Srinivasan
Reviewed-by: Haiyang Zhang
---
drivers/net/hyperv/hyperv_net.h | 14 +++
drivers/net/hyperv/netvsc.c | 226 +--
drivers/net/hyperv/netvsc_drv.c |3 +-
3 files changed, 234 insertions(+), 9 deletions(-)
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index d1f7826..4b7df5a 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -140,6 +140,8 @@ struct hv_netvsc_packet {
void *send_completion_ctx;
void (*send_completion)(void *context);
+ u32 send_buf_index;
+
/* This points to the memory after page_buf */
struct rndis_message *rndis_msg;
@@ -582,6 +584,9 @@ struct nvsp_message {
#define NETVSC_RECEIVE_BUFFER_SIZE (1024*1024*16) /* 16MB */
#define NETVSC_RECEIVE_BUFFER_SIZE_LEGACY (1024*1024*15) /* 15MB */
+#define NETVSC_SEND_BUFFER_SIZE(1024 * 1024) /* 1MB
*/
+#define NETVSC_INVALID_INDEX -1
+
#define NETVSC_RECEIVE_BUFFER_ID 0xcafe
@@ -607,6 +612,15 @@ struct netvsc_device {
u32 recv_section_cnt;
struct nvsp_1_receive_buffer_section *recv_section;
+ /* Send buffer allocated by us */
+ void *send_buf;
+ u32 send_buf_size;
+ u32 send_buf_gpadl_handle;
+ u32 send_section_cnt;
+ u32 send_section_size;
+ unsigned long *send_section_map;
+ int map_words;
+
/* Used for NetVSP initialization protocol */
struct completion channel_init_wait;
struct nvsp_message channel_init_pkt;
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index bbee446..c041f63 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include "hyperv_net.h"
@@ -80,7 +81,7 @@ get_in_err:
}
-static int netvsc_destroy_recv_buf(struct netvsc_device *net_device)
+static int netvsc_destroy_buf(struct netvsc_device *net_device)
{
struct nvsp_message *revoke_packet;
int ret = 0;
@@ -146,10 +147,62 @@ static int netvsc_destroy_recv_buf(struct netvsc_device
*net_device)
net_device->recv_section = NULL;
}
+ /* Deal with the send buffer we may have setup.
+* If we got a send section size, it means we received a
+* SendsendBufferComplete msg (ie sent
+* NvspMessage1TypeSendReceiveBuffer msg) therefore, we need
+* to send a revoke msg here
+*/
+ if (net_device->send_section_size) {
+ /* Send the revoke receive buffer */
+ revoke_packet = &net_device->revoke_packet;
+ memset(revoke_packet, 0, sizeof(struct nvsp_message));
+
+ revoke_packet->hdr.msg_type =
+ NVSP_MSG1_TYPE_REVOKE_SEND_BUF;
+ revoke_packet->msg.v1_msg.revoke_recv_buf.id = 0;
+
+ ret = vmbus_sendpacket(net_device->dev->channel,
+ revoke_packet,
+ sizeof(struct nvsp_message),
+ (unsigned long)revoke_packet,
+ VM_PKT_DATA_INBAND, 0);
+ /* If we failed here, we might as well return and
+* have a leak rather than continue and a bugchk
+*/
+ if (ret != 0) {
+ netdev_err(ndev, "unable to send "
+ "revoke send buffer to netvsp\n");
+ return ret;
+ }
+ }
+ /* Teardown the gpadl on the vsp end */
+ if (net_device->send_buf_gpadl_handle) {
+ ret = vmbus_teardown_gpadl(net_device->dev->channel,
+ net_device->send_buf_gpadl_handle);
+
+ /* If we failed here, we might as well return and have a leak
+* rather than continue and a bugchk
+*/
+ if (ret != 0) {
+ netdev_err(ndev,
+ "unable to teardown send buffer's gpadl\n");
+ return ret;
+ }
+ net_device->recv_buf_gpadl_handle = 0;
+ }
+ if (net_device->send_buf) {
+
[PATCH] staging: dgap: fix sparse warnings for the entire cm_t structure
This patch fixes sparse warnings for the entire cm_t structure
This entire structure defines a hardware segment
Signed-off-by: Mark Hounschell
Tested-by: Mark Hounschell
Cc: Greg Kroah-Hartman
---
drivers/staging/dgap/dgap.c | 8
drivers/staging/dgap/dgap.h | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index 0bf22f7..1e7f40e 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -1304,7 +1304,7 @@ static int dgap_tty_init(struct board_t *brd)
u8 modem = 0;
struct channel_t *ch;
struct bs_t __iomem *bs;
- struct cm_t *cm;
+ struct cm_t __iomem *cm;
if (!brd)
return -ENXIO;
@@ -4744,7 +4744,7 @@ static void dgap_cmdb(struct channel_t *ch, u8 cmd, u8
byte1,
u8 byte2, uint ncmds)
{
char __iomem*vaddr;
- struct cm_t *cm_addr = NULL;
+ struct __iomem cm_t *cm_addr;
uintcount;
uintn;
u16 head;
@@ -4829,7 +4829,7 @@ static void dgap_cmdb(struct channel_t *ch, u8 cmd, u8
byte1,
static void dgap_cmdw(struct channel_t *ch, u8 cmd, u16 word, uint ncmds)
{
char __iomem*vaddr;
- struct cm_t *cm_addr = NULL;
+ struct __iomem cm_t *cm_addr;
uintcount;
uintn;
u16 head;
@@ -4912,7 +4912,7 @@ static void dgap_cmdw(struct channel_t *ch, u8 cmd, u16
word, uint ncmds)
static void dgap_cmdw_ext(struct channel_t *ch, u16 cmd, u16 word, uint ncmds)
{
char __iomem*vaddr;
- struct cm_t *cm_addr = NULL;
+ struct __iomem cm_t *cm_addr;
uintcount;
uintn;
u16 head;
diff --git a/drivers/staging/dgap/dgap.h b/drivers/staging/dgap/dgap.h
index 8d75d32..e8bb8c6 100644
--- a/drivers/staging/dgap/dgap.h
+++ b/drivers/staging/dgap/dgap.h
@@ -969,7 +969,7 @@ struct digi_cmd {
struct channel_t {
int magic; /* Channel Magic Number */
struct bs_t __iomem *ch_bs; /* Base structure pointer */
- struct cm_t *ch_cm; /* Command queue pointer*/
+ struct cm_t __iomem *ch_cm; /* Command queue pointer*/
struct board_t *ch_bd; /* Board structure pointer */
unsigned char *ch_vaddr;/* FEP memory origin*/
unsigned char *ch_taddr;/* Write buffer origin */
--
1.8.4.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: Fix typo in iio
On 22/04/14 22:41, Randy Dunlap wrote:
On 04/22/14 04:23, Masanari Iida wrote:
Correct spelling typo in comment within staging/iio
Signed-off-by: Masanari Iida
Acked-by: Randy Dunlap
Oh good, a cut and paste typo ;)
Anyhow, applied to the togreg branch of iio.git
Might not be pushed out for a little while as I'm travelling.
Note that a fair bit of fuzz occurred but given the nature of the
patch I doubt I managed to mess this one up.
Thanks for noticing and fixing this one.
Jonathan
---
drivers/staging/iio/adc/ad7606.h| 4 ++--
drivers/staging/iio/adc/ad7816.c| 2 +-
drivers/staging/iio/adc/ad799x.h| 2 +-
drivers/staging/iio/addac/adt7316.c | 4 ++--
drivers/staging/iio/cdc/ad7152.c| 2 +-
drivers/staging/iio/cdc/ad7746.c| 2 +-
6 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index 93c7299..ec89d05 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -14,7 +14,7 @@
*/
/**
- * struct ad7606_platform_data - platform/board specifc information
+ * struct ad7606_platform_data - platform/board specific information
* @default_os: default oversampling value {0, 2, 4, 8, 16, 32,
64}
* @default_range:default range +/-{5000, 1} mVolt
* @gpio_convst: number of gpio connected to the CONVST pin
@@ -41,7 +41,7 @@ struct ad7606_platform_data {
};
/**
- * struct ad7606_chip_info - chip specifc information
+ * struct ad7606_chip_info - chip specific information
* @name: identification string for chip
* @int_vref_mv: the internal reference voltage
* @channels: channel specification
diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
index 2369cf2..158d770 100644
--- a/drivers/staging/iio/adc/ad7816.c
+++ b/drivers/staging/iio/adc/ad7816.c
@@ -40,7 +40,7 @@
/*
- * struct ad7816_chip_info - chip specifc information
+ * struct ad7816_chip_info - chip specific information
*/
struct ad7816_chip_info {
diff --git a/drivers/staging/iio/adc/ad799x.h b/drivers/staging/iio/adc/ad799x.h
index fc8c852..70ddfa2 100644
--- a/drivers/staging/iio/adc/ad799x.h
+++ b/drivers/staging/iio/adc/ad799x.h
@@ -76,7 +76,7 @@ enum {
struct ad799x_state;
/**
- * struct ad799x_chip_info - chip specifc information
+ * struct ad799x_chip_info - chip specific information
* @channel: channel specification
* @num_channels: number of channels
* @monitor_mode: whether the chip supports monitor interrupts
diff --git a/drivers/staging/iio/addac/adt7316.c
b/drivers/staging/iio/addac/adt7316.c
index 9f0ebb3..5f1770e 100644
--- a/drivers/staging/iio/addac/adt7316.c
+++ b/drivers/staging/iio/addac/adt7316.c
@@ -172,7 +172,7 @@
#define ID_ADT75XX0x10
/*
- * struct adt7316_chip_info - chip specifc information
+ * struct adt7316_chip_info - chip specific information
*/
struct adt7316_chip_info {
@@ -208,7 +208,7 @@ struct adt7316_chip_info {
(ADT7316_TEMP_INT_MASK)
/*
- * struct adt7316_chip_info - chip specifc information
+ * struct adt7316_chip_info - chip specific information
*/
struct adt7316_limit_regs {
diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c
index f2c309d..87110d9 100644
--- a/drivers/staging/iio/cdc/ad7152.c
+++ b/drivers/staging/iio/cdc/ad7152.c
@@ -78,7 +78,7 @@ enum {
};
/*
- * struct ad7152_chip_info - chip specifc information
+ * struct ad7152_chip_info - chip specific information
*/
struct ad7152_chip_info {
diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index cbb1588..e6e9eaa 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -91,7 +91,7 @@
#define AD7746_CAPDAC_DACP(x) ((x) & 0x7F)
/*
- * struct ad7746_chip_info - chip specifc information
+ * struct ad7746_chip_info - chip specific information
*/
struct ad7746_chip_info {
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V1 net-next 1/1] hyperv: Enable sendbuf mechanism on the send path
TLDR; Style nits and we should return -ENOMEM on error instead of
success.
On Wed, Apr 23, 2014 at 02:24:45PM -0700, K. Y. Srinivasan wrote:
> We send packets using a copy-free mechanism (this is the Guest to Host
> transport
> via VMBUS). While this is obviously optimal for large packets,
> it may not be optimal for small packets. Hyper-V host supports
> a second mechanism for sending packets that is "copy based". We implement that
> mechanism in this patch.
>
> In this version of the patch I have addressed a comment from David Miller.
>
> With this patch (and all of the other offload and VRSS patches), we are now
> able
> to almost saturate a 10G interface between Linux VMs on Hyper-V
> on different hosts - close to 9 Gbps as measured via iperf.
>
> Signed-off-by: K. Y. Srinivasan
> Reviewed-by: Haiyang Zhang
> ---
> drivers/net/hyperv/hyperv_net.h | 14 +++
> drivers/net/hyperv/netvsc.c | 226
> +--
> drivers/net/hyperv/netvsc_drv.c |3 +-
> 3 files changed, 234 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
> index d1f7826..4b7df5a 100644
> --- a/drivers/net/hyperv/hyperv_net.h
> +++ b/drivers/net/hyperv/hyperv_net.h
> @@ -140,6 +140,8 @@ struct hv_netvsc_packet {
> void *send_completion_ctx;
> void (*send_completion)(void *context);
>
> + u32 send_buf_index;
> +
> /* This points to the memory after page_buf */
> struct rndis_message *rndis_msg;
>
> @@ -582,6 +584,9 @@ struct nvsp_message {
>
> #define NETVSC_RECEIVE_BUFFER_SIZE (1024*1024*16) /* 16MB */
> #define NETVSC_RECEIVE_BUFFER_SIZE_LEGACY(1024*1024*15) /* 15MB */
> +#define NETVSC_SEND_BUFFER_SIZE (1024 * 1024) /* 1MB
> */
> +#define NETVSC_INVALID_INDEX -1
> +
>
> #define NETVSC_RECEIVE_BUFFER_ID 0xcafe
>
> @@ -607,6 +612,15 @@ struct netvsc_device {
> u32 recv_section_cnt;
> struct nvsp_1_receive_buffer_section *recv_section;
>
> + /* Send buffer allocated by us */
> + void *send_buf;
> + u32 send_buf_size;
> + u32 send_buf_gpadl_handle;
> + u32 send_section_cnt;
> + u32 send_section_size;
> + unsigned long *send_section_map;
> + int map_words;
> +
> /* Used for NetVSP initialization protocol */
> struct completion channel_init_wait;
> struct nvsp_message channel_init_pkt;
> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index bbee446..c041f63 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
> @@ -28,6 +28,7 @@
> #include
> #include
> #include
> +#include
>
> #include "hyperv_net.h"
>
> @@ -80,7 +81,7 @@ get_in_err:
> }
>
>
> -static int netvsc_destroy_recv_buf(struct netvsc_device *net_device)
> +static int netvsc_destroy_buf(struct netvsc_device *net_device)
> {
> struct nvsp_message *revoke_packet;
> int ret = 0;
> @@ -146,10 +147,62 @@ static int netvsc_destroy_recv_buf(struct netvsc_device
> *net_device)
> net_device->recv_section = NULL;
> }
>
> + /* Deal with the send buffer we may have setup.
> + * If we got a send section size, it means we received a
> + * SendsendBufferComplete msg (ie sent
> + * NvspMessage1TypeSendReceiveBuffer msg) therefore, we need
> + * to send a revoke msg here
> + */
> + if (net_device->send_section_size) {
> + /* Send the revoke receive buffer */
> + revoke_packet = &net_device->revoke_packet;
> + memset(revoke_packet, 0, sizeof(struct nvsp_message));
> +
> + revoke_packet->hdr.msg_type =
> + NVSP_MSG1_TYPE_REVOKE_SEND_BUF;
This fits in 80 characters.
revoke_packet->hdr.msg_type = NVSP_MSG1_TYPE_REVOKE_SEND_BUF;
> + revoke_packet->msg.v1_msg.revoke_recv_buf.id = 0;
> +
> + ret = vmbus_sendpacket(net_device->dev->channel,
> +revoke_packet,
> +sizeof(struct nvsp_message),
> +(unsigned long)revoke_packet,
> +VM_PKT_DATA_INBAND, 0);
> + /* If we failed here, we might as well return and
> + * have a leak rather than continue and a bugchk
> + */
> + if (ret != 0) {
I have never been a big fan of these double negative conditions. It's
simpler and more normal style to say: "if (ret) { ". There are some
cases where it makes sense to compare against zero. For example, if you
are dealing with a variable as a number then you would say
"if (x == 0) ... else if (x == 1) ". Also for strcmp() then you should
use == 0 and != 0. But for error codes it's better it's more idiomatic
to just say "if (ret) ".
> + netdev_err(ndev, "unable to send "
> +
[PATCH 3/3] staging: comedi: me4000: remove unnecessary Step 2b test in (*do_cmdtest)
This test is unnecessary. It covers all the possible combinations of the
scan_end_src and stop_src triggers so the final else can never be reached.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/me4000.c | 11 ---
1 file changed, 11 deletions(-)
diff --git a/drivers/staging/comedi/drivers/me4000.c
b/drivers/staging/comedi/drivers/me4000.c
index f43221f..5fc4850 100644
--- a/drivers/staging/comedi/drivers/me4000.c
+++ b/drivers/staging/comedi/drivers/me4000.c
@@ -902,17 +902,6 @@ static int me4000_ai_do_cmd_test(struct comedi_device *dev,
err |= -EINVAL;
}
- if (cmd->stop_src == TRIG_NONE && cmd->scan_end_src == TRIG_NONE) {
- } else if (cmd->stop_src == TRIG_COUNT &&
- cmd->scan_end_src == TRIG_NONE) {
- } else if (cmd->stop_src == TRIG_NONE &&
- cmd->scan_end_src == TRIG_COUNT) {
- } else if (cmd->stop_src == TRIG_COUNT &&
- cmd->scan_end_src == TRIG_COUNT) {
- } else {
- err |= -EINVAL;
- }
-
if (err)
return 2;
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging: comedi: adl_pci9111: clarify Step 2b of the (*do_cmdtest)
This step of the (*do_cmdtest) verifies that the selected trigger sources
are mutually compatible.
For this driver the scan_begin_src must be TRIG_FOLLOW or the same source
as the convert_src.
Simplify the logic to clarify this.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adl_pci9111.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adl_pci9111.c
b/drivers/staging/comedi/drivers/adl_pci9111.c
index 6fc8593..9225a38 100644
--- a/drivers/staging/comedi/drivers/adl_pci9111.c
+++ b/drivers/staging/comedi/drivers/adl_pci9111.c
@@ -379,14 +379,10 @@ static int pci9111_ai_do_cmd_test(struct comedi_device
*dev,
/* Step 2b : and mutually compatible */
- if ((cmd->convert_src == TRIG_TIMER) &&
- !((cmd->scan_begin_src == TRIG_TIMER) ||
- (cmd->scan_begin_src == TRIG_FOLLOW)))
- err |= -EINVAL;
- if ((cmd->convert_src == TRIG_EXT) &&
- !((cmd->scan_begin_src == TRIG_EXT) ||
- (cmd->scan_begin_src == TRIG_FOLLOW)))
- err |= -EINVAL;
+ if (cmd->scan_begin_src != TRIG_FOLLOW) {
+ if (cmd->scan_begin_src != cmd->convert_src)
+ err |= -EINVAL;
+ }
if (err)
return 2;
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging: comedi: cb_pcidas64: remove unnecessary Step 2b test in (*do_cmdtest)
This test is unnecessary. The cfc_check_trigger_src() in Step 1 ensures that the trigger source is one of these values and Step 2 makes sure it's only one of these values. Signed-off-by: H Hartley Sweeten Cc: Ian Abbott Cc: Greg Kroah-Hartman --- drivers/staging/comedi/drivers/cb_pcidas64.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/staging/comedi/drivers/cb_pcidas64.c b/drivers/staging/comedi/drivers/cb_pcidas64.c index 4061f04..0e13ac9 100644 --- a/drivers/staging/comedi/drivers/cb_pcidas64.c +++ b/drivers/staging/comedi/drivers/cb_pcidas64.c @@ -2078,9 +2078,6 @@ static int ai_cmdtest(struct comedi_device *dev, struct comedi_subdevice *s, if (cmd->convert_src == TRIG_EXT && cmd->scan_begin_src == TRIG_TIMER) err |= -EINVAL; - if (cmd->stop_src != TRIG_COUNT && - cmd->stop_src != TRIG_NONE && cmd->stop_src != TRIG_EXT) - err |= -EINVAL; if (err) return 2; -- 1.9.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/3] staging: comedi: drivers: cleanup (*do_cmdtest) step 2b
Clarify step 2b of the adl_pci9111 driver. Remove the unnecessary step 2b tests in the cb_pcidas64 and me4000 drivers. H Hartley Sweeten (3): staging: comedi: adl_pci9111: clarify Step 2b of the (*do_cmdtest) staging: comedi: cb_pcidas64: remove unnecessary Step 2b test in (*do_cmdtest) staging: comedi: me4000: remove unnecessary Step 2b test in (*do_cmdtest) drivers/staging/comedi/drivers/adl_pci9111.c | 12 drivers/staging/comedi/drivers/cb_pcidas64.c | 3 --- drivers/staging/comedi/drivers/me4000.c | 11 --- 3 files changed, 4 insertions(+), 22 deletions(-) -- 1.9.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/4] staging: comedi: amplc_pci224: factor out the 'start pacer' code
To clarify the analog output (*do_cmd) function, factor out the
code that starts the pacer.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/amplc_pci224.c | 142 +-
1 file changed, 73 insertions(+), 69 deletions(-)
diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c
b/drivers/staging/comedi/drivers/amplc_pci224.c
index a34bd9d..727fc22 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -901,9 +901,77 @@ pci224_ao_cmdtest(struct comedi_device *dev, struct
comedi_subdevice *s,
return 0;
}
-/*
- * 'do_cmd' function for AO subdevice.
- */
+static void pci224_ao_start_pacer(struct comedi_device *dev,
+ struct comedi_subdevice *s)
+{
+ struct pci224_private *devpriv = dev->private;
+ struct comedi_cmd *cmd = &s->async->cmd;
+ unsigned int div1, div2, round;
+ unsigned int ns = cmd->scan_begin_arg;
+ int round_mode = cmd->flags & TRIG_ROUND_MASK;
+
+ /* Check whether to use a single timer. */
+ switch (round_mode) {
+ case TRIG_ROUND_NEAREST:
+ default:
+ round = I8254_OSC_BASE_10MHZ / 2;
+ break;
+ case TRIG_ROUND_DOWN:
+ round = 0;
+ break;
+ case TRIG_ROUND_UP:
+ round = I8254_OSC_BASE_10MHZ - 1;
+ break;
+ }
+ /* Be careful to avoid overflow! */
+ div2 = cmd->scan_begin_arg / I8254_OSC_BASE_10MHZ;
+ div2 += (round + cmd->scan_begin_arg % I8254_OSC_BASE_10MHZ) /
+ I8254_OSC_BASE_10MHZ;
+ if (div2 <= 0x1) {
+ /* A single timer will suffice. */
+ if (div2 < 2)
+ div2 = 2;
+ div2 &= 0x;
+ div1 = 1; /* Flag that single timer to be used. */
+ } else {
+ /* Use two timers. */
+ div1 = devpriv->cached_div1;
+ div2 = devpriv->cached_div2;
+ i8253_cascade_ns_to_timer(I8254_OSC_BASE_10MHZ,
+&div1, &div2,
+&ns, round_mode);
+ }
+
+ /*
+* The output of timer Z2-0 will be used as the scan trigger
+* source.
+*/
+ /* Make sure Z2-0 is gated on. */
+ outb(GAT_CONFIG(0, GAT_VCC),
+ devpriv->iobase1 + PCI224_ZGAT_SCE);
+ if (div1 == 1) {
+ /* Not cascading. Z2-0 needs 10 MHz clock. */
+ outb(CLK_CONFIG(0, CLK_10MHZ),
+ devpriv->iobase1 + PCI224_ZCLK_SCE);
+ } else {
+ /* Cascading with Z2-2. */
+ /* Make sure Z2-2 is gated on. */
+ outb(GAT_CONFIG(2, GAT_VCC),
+ devpriv->iobase1 + PCI224_ZGAT_SCE);
+ /* Z2-2 needs 10 MHz clock. */
+ outb(CLK_CONFIG(2, CLK_10MHZ),
+ devpriv->iobase1 + PCI224_ZCLK_SCE);
+ /* Load Z2-2 mode (2) and counter (div1). */
+ i8254_load(devpriv->iobase1 + PCI224_Z2_CT0, 0,
+ 2, div1, 2);
+ /* Z2-0 is clocked from Z2-2's output. */
+ outb(CLK_CONFIG(0, CLK_OUTNM1),
+ devpriv->iobase1 + PCI224_ZCLK_SCE);
+ }
+ /* Load Z2-0 mode (2) and counter (div2). */
+ i8254_load(devpriv->iobase1 + PCI224_Z2_CT0, 0, 0, div2, 2);
+}
+
static int pci224_ao_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
{
struct pci224_private *devpriv = dev->private;
@@ -959,72 +1027,8 @@ static int pci224_ao_cmd(struct comedi_device *dev,
struct comedi_subdevice *s)
outw(devpriv->daccon | PCI224_DACCON_FIFORESET,
dev->iobase + PCI224_DACCON);
- if (cmd->scan_begin_src == TRIG_TIMER) {
- unsigned int div1, div2, round;
- unsigned int ns = cmd->scan_begin_arg;
- int round_mode = cmd->flags & TRIG_ROUND_MASK;
-
- /* Check whether to use a single timer. */
- switch (round_mode) {
- case TRIG_ROUND_NEAREST:
- default:
- round = I8254_OSC_BASE_10MHZ / 2;
- break;
- case TRIG_ROUND_DOWN:
- round = 0;
- break;
- case TRIG_ROUND_UP:
- round = I8254_OSC_BASE_10MHZ - 1;
- break;
- }
- /* Be careful to avoid overflow! */
- div2 = cmd->scan_begin_arg / I8254_OSC_BASE_10MHZ;
- div2 += (round + cmd->scan_begin_arg % I8254_OSC_BASE_10MHZ) /
- I8254_OSC_BASE_10MHZ;
- if (div2 <= 0x1) {
- /* A single timer will suffice. */
- if (div2
[PATCH 4/4] staging: comedi: amplc_pci224: only calc the pacer divisors once
When the cmd->scan_begin_src == TRIG_TIMER the divisors needed to
generate the pacer time are calculated in the (*do_cmdtest) to
validate the cmd->scan_begin_arg. The core always does the
(*do_cmdtest) before the (*do_cmd) so there is no reason to recalc
the divisors.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/amplc_pci224.c | 8
1 file changed, 8 deletions(-)
diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c
b/drivers/staging/comedi/drivers/amplc_pci224.c
index 37a81b7..f0b04ca 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -869,14 +869,6 @@ static void pci224_ao_start_pacer(struct comedi_device
*dev,
struct comedi_subdevice *s)
{
struct pci224_private *devpriv = dev->private;
- struct comedi_cmd *cmd = &s->async->cmd;
-
- /* Use two timers. */
- i8253_cascade_ns_to_timer(I8254_OSC_BASE_10MHZ,
- &devpriv->cached_div1,
- &devpriv->cached_div2,
- &cmd->scan_begin_arg,
- cmd->flags);
/*
* The output of timer Z2-0 will be used as the scan trigger
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/4] staging: comedi: amplc_pci224: cleanup timer code
Tidy up the timer/pacer code in this driver. H Hartley Sweeten (4): staging: comedi: amplc_pci224: remove pci224_cascade_ns_to_timer() staging: comedi: amplc_pci224: factor out the 'start pacer' code staging: comedi: amplc_pci224: always cascade the 8254 timers staging: comedi: amplc_pci224: only calc the pacer divisors once drivers/staging/comedi/drivers/amplc_pci224.c | 155 ++ 1 file changed, 34 insertions(+), 121 deletions(-) -- 1.9.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/4] staging: comedi: amplc_pci224: always cascade the 8254 timers
The 8254 timers are only used in this driver to generate the analog
output pacer. To simplify the driver, always cascade the timers.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/amplc_pci224.c | 123 ++
1 file changed, 25 insertions(+), 98 deletions(-)
diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c
b/drivers/staging/comedi/drivers/amplc_pci224.c
index 727fc22..37a81b7 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -841,51 +841,15 @@ pci224_ao_cmdtest(struct comedi_device *dev, struct
comedi_subdevice *s,
/* Step 4: fix up any arguments. */
if (cmd->scan_begin_src == TRIG_TIMER) {
- unsigned int div1, div2, round;
- int round_mode = cmd->flags & TRIG_ROUND_MASK;
-
tmp = cmd->scan_begin_arg;
- /* Check whether to use a single timer. */
- switch (round_mode) {
- case TRIG_ROUND_NEAREST:
- default:
- round = I8254_OSC_BASE_10MHZ / 2;
- break;
- case TRIG_ROUND_DOWN:
- round = 0;
- break;
- case TRIG_ROUND_UP:
- round = I8254_OSC_BASE_10MHZ - 1;
- break;
- }
- /* Be careful to avoid overflow! */
- div2 = cmd->scan_begin_arg / I8254_OSC_BASE_10MHZ;
- div2 += (round + cmd->scan_begin_arg % I8254_OSC_BASE_10MHZ) /
- I8254_OSC_BASE_10MHZ;
- if (div2 <= 0x1) {
- /* A single timer will suffice. */
- if (div2 < 2)
- div2 = 2;
- cmd->scan_begin_arg = div2 * I8254_OSC_BASE_10MHZ;
- if (cmd->scan_begin_arg < div2 ||
- cmd->scan_begin_arg < I8254_OSC_BASE_10MHZ) {
- /* Overflow! */
- cmd->scan_begin_arg = MAX_SCAN_PERIOD;
- }
- } else {
- /* Use two timers. */
- div1 = devpriv->cached_div1;
- div2 = devpriv->cached_div2;
- i8253_cascade_ns_to_timer(I8254_OSC_BASE_10MHZ,
- &div1, &div2,
- &cmd->scan_begin_arg,
- round_mode);
- devpriv->cached_div1 = div1;
- devpriv->cached_div2 = div2;
- }
+ /* Use two timers. */
+ i8253_cascade_ns_to_timer(I8254_OSC_BASE_10MHZ,
+ &devpriv->cached_div1,
+ &devpriv->cached_div2,
+ &cmd->scan_begin_arg,
+ cmd->flags);
if (tmp != cmd->scan_begin_arg)
err++;
-
}
if (err)
@@ -906,70 +870,33 @@ static void pci224_ao_start_pacer(struct comedi_device
*dev,
{
struct pci224_private *devpriv = dev->private;
struct comedi_cmd *cmd = &s->async->cmd;
- unsigned int div1, div2, round;
- unsigned int ns = cmd->scan_begin_arg;
- int round_mode = cmd->flags & TRIG_ROUND_MASK;
- /* Check whether to use a single timer. */
- switch (round_mode) {
- case TRIG_ROUND_NEAREST:
- default:
- round = I8254_OSC_BASE_10MHZ / 2;
- break;
- case TRIG_ROUND_DOWN:
- round = 0;
- break;
- case TRIG_ROUND_UP:
- round = I8254_OSC_BASE_10MHZ - 1;
- break;
- }
- /* Be careful to avoid overflow! */
- div2 = cmd->scan_begin_arg / I8254_OSC_BASE_10MHZ;
- div2 += (round + cmd->scan_begin_arg % I8254_OSC_BASE_10MHZ) /
- I8254_OSC_BASE_10MHZ;
- if (div2 <= 0x1) {
- /* A single timer will suffice. */
- if (div2 < 2)
- div2 = 2;
- div2 &= 0x;
- div1 = 1; /* Flag that single timer to be used. */
- } else {
- /* Use two timers. */
- div1 = devpriv->cached_div1;
- div2 = devpriv->cached_div2;
- i8253_cascade_ns_to_timer(I8254_OSC_BASE_10MHZ,
-&div1, &div2,
-&ns, round_mode);
- }
+ /* Use two timers. */
+ i8253_cascade_ns_to_timer(I8254_OSC_BASE_10MHZ,
+ &devpriv->cached_div1,
+ &devpriv->cached_div2,
+
[PATCH 1/4] staging: comedi: amplc_pci224: remove pci224_cascade_ns_to_timer()
This function is just a wrapper around i8253_cascade_ns_to_timer().
Remove it.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/amplc_pci224.c | 24 +++-
1 file changed, 7 insertions(+), 17 deletions(-)
diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c
b/drivers/staging/comedi/drivers/amplc_pci224.c
index 3b9a8b9..a34bd9d 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -467,16 +467,6 @@ pci224_ao_insn_read(struct comedi_device *dev, struct
comedi_subdevice *s,
}
/*
- * Just a wrapper for the inline function 'i8253_cascade_ns_to_timer'.
- */
-static void
-pci224_cascade_ns_to_timer(int osc_base, unsigned int *d1, unsigned int *d2,
- unsigned int *nanosec, int round_mode)
-{
- i8253_cascade_ns_to_timer(osc_base, d1, d2, nanosec, round_mode);
-}
-
-/*
* Kills a command running on the AO subdevice.
*/
static void pci224_ao_stop(struct comedi_device *dev,
@@ -886,10 +876,10 @@ pci224_ao_cmdtest(struct comedi_device *dev, struct
comedi_subdevice *s,
/* Use two timers. */
div1 = devpriv->cached_div1;
div2 = devpriv->cached_div2;
- pci224_cascade_ns_to_timer(I8254_OSC_BASE_10MHZ,
- &div1, &div2,
- &cmd->scan_begin_arg,
- round_mode);
+ i8253_cascade_ns_to_timer(I8254_OSC_BASE_10MHZ,
+ &div1, &div2,
+ &cmd->scan_begin_arg,
+ round_mode);
devpriv->cached_div1 = div1;
devpriv->cached_div2 = div2;
}
@@ -1001,9 +991,9 @@ static int pci224_ao_cmd(struct comedi_device *dev, struct
comedi_subdevice *s)
/* Use two timers. */
div1 = devpriv->cached_div1;
div2 = devpriv->cached_div2;
- pci224_cascade_ns_to_timer(I8254_OSC_BASE_10MHZ,
- &div1, &div2,
- &ns, round_mode);
+ i8253_cascade_ns_to_timer(I8254_OSC_BASE_10MHZ,
+ &div1, &div2,
+ &ns, round_mode);
}
/*
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 00/22] staging: comedi: adv_pci1710: cleanup async command support
Remove the unnecessary members from the private data and cleanup the async command support code. H Hartley Sweeten (22): staging: comedi: adv_pci1710: remove 'ai_timer2' from private data staging: comedi: adv_pci1710: remove 'ai_timer1' from private data staging: comedi: adv_pci1710: remove 'ai_flags' from private data staging: comedi: adv_pci1710: remove 'ai_chanlist' from private data staging: comedi: adv_pci1710: remove 'ai_n_chan' from private data staging: comedi: adv_pci1710: remove 'ai_scans' from private data staging: comedi: adv_pci1710: remove 'ai_data_len' from private data staging: comedi: adv_pci1710: cmd->scan_begin_src can only be TRIG_FOLLOW staging: comedi: adv_pci1710: absorb pci171x_ai_docmd_and_mode() staging: comedi: adv_pci1710: remove 'ai_do' from private data staging: comedi: adv_pci1710: rename check_channel_list() staging: comedi: adv_pci1710: tidy up pci171x_ai_check_chanlist() staging: comedi: adv_pci1710: don't check the chanlist twice staging: comedi: adv_pci1710: remove 'neverending_ai' from private data staging: comedi: adv_pci1710: remove 'ai_buf_ptr' from private data staging: comedi: adv_pci1710: remove 'ai_eos' from private data staging: comedi: adv_pci1710: only calc the pacer divisors once staging: comedi: adv_pci1710: remove local var in pci171x_ai_cmd() staging: comedi: adv_pci1710: tidy up start_pacer() staging: comedi: adv_pci1710: rename interrupt helper functions staging: comedi: adv_pci1710: handle events and clear interrupt in common code staging: comedi: adv_pci1710: always enable PCI171x_PARANOIDCHECK code drivers/staging/comedi/drivers/adv_pci1710.c | 485 ++- 1 file changed, 183 insertions(+), 302 deletions(-) -- 1.9.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 03/22] staging: comedi: adv_pci1710: remove 'ai_flags' from private data
This member of the private data is just a copy of the cmd->flags. Use
that instead.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index c1e0e41..447addd 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -317,7 +317,6 @@ struct pci1710_private {
unsigned int ai_scans; /* len of scanlist */
unsigned int ai_n_chan; /* how many channels is measured */
unsigned int *ai_chanlist; /* actaul chanlist */
- unsigned int ai_flags; /* flaglist */
unsigned int ai_data_len; /* len of data buffer */
unsigned short ao_data[4]; /* data output buffer */
unsigned int cnt0_write_wait; /* after a write, wait for update of the
@@ -981,7 +980,7 @@ static int pci171x_ai_docmd_and_mode(int mode, struct
comedi_device *dev,
devpriv->CntrlReg &= Control_CNT0;
/* don't we want wake up every scan? devpriv->ai_eos=1; */
- if ((devpriv->ai_flags & TRIG_WAKE_EOS)) {
+ if (cmd->flags & TRIG_WAKE_EOS) {
devpriv->ai_eos = 1;
} else {
devpriv->CntrlReg |= Control_ONEFH;
@@ -1010,7 +1009,7 @@ static int pci171x_ai_docmd_and_mode(int mode, struct
comedi_device *dev,
i8253_cascade_ns_to_timer(devpriv->i8254_osc_base,
&divisor1, &divisor2,
&cmd->convert_arg,
- devpriv->ai_flags);
+ cmd->flags);
outw(devpriv->CntrlReg, dev->iobase + PCI171x_CONTROL);
if (mode != 2) {
/* start pacer */
@@ -1122,7 +1121,6 @@ static int pci171x_ai_cmd(struct comedi_device *dev,
struct comedi_subdevice *s)
devpriv->ai_n_chan = cmd->chanlist_len;
devpriv->ai_chanlist = cmd->chanlist;
- devpriv->ai_flags = cmd->flags;
devpriv->ai_data_len = s->async->prealloc_bufsz;
if (cmd->stop_src == TRIG_COUNT)
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 09/22] staging: comedi: adv_pci1710: absorb pci171x_ai_docmd_and_mode()
This helper function is called by the ai (*do_cmd) to setup and
start the async command based on the "mode" of operation. This
"mode" is determined by the cmd convert_src and start_src.
Move this "mode" determination and combine the functions.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 35 +---
1 file changed, 11 insertions(+), 24 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index 60b926e..6ab78bb 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -947,16 +947,22 @@ static irqreturn_t interrupt_service_pci1710(int irq,
void *d)
return IRQ_HANDLED;
}
-/*
-==
-*/
-static int pci171x_ai_docmd_and_mode(int mode, struct comedi_device *dev,
-struct comedi_subdevice *s)
+static int pci171x_ai_cmd(struct comedi_device *dev, struct comedi_subdevice
*s)
{
struct pci1710_private *devpriv = dev->private;
struct comedi_cmd *cmd = &s->async->cmd;
unsigned int divisor1 = 0, divisor2 = 0;
unsigned int seglen;
+ int mode;
+
+ if (cmd->convert_src == TRIG_TIMER) {
+ if (cmd->start_src == TRIG_EXT)
+ mode = 2;
+ else
+ mode = 1;
+ } else {/* TRIG_EXT */
+ mode = 3;
+ }
start_pacer(dev, -1, 0, 0); /* stop pacer */
@@ -,25 +1117,6 @@ static int pci171x_ai_cmdtest(struct comedi_device *dev,
/*
==
*/
-static int pci171x_ai_cmd(struct comedi_device *dev, struct comedi_subdevice
*s)
-{
- struct comedi_cmd *cmd = &s->async->cmd;
-
- if (cmd->convert_src == TRIG_TIMER) { /* mode 1 and 2 */
- return pci171x_ai_docmd_and_mode(cmd->start_src ==
-TRIG_EXT ? 2 : 1, dev,
-s);
- }
- if (cmd->convert_src == TRIG_EXT) { /* mode 3 */
- return pci171x_ai_docmd_and_mode(3, dev, s);
- }
-
- return -1;
-}
-
-/*
-==
-*/
static int pci171x_reset(struct comedi_device *dev)
{
const struct boardtype *this_board = comedi_board(dev);
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 15/22] staging: comedi: adv_pci1710: remove 'ai_buf_ptr' from private data
This member of the private data is set to 0 but never used. Just remove it.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index e6c002a..b8e7e2f 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -302,7 +302,6 @@ struct pci1710_private {
unsigned int i8254_osc_base;/* frequence of onboard oscilator */
unsigned int ai_act_scan; /* how many scans we finished */
unsigned int ai_act_chan; /* actual position in actual scan */
- unsigned int ai_buf_ptr;/* data buffer ptr in samples */
unsigned char ai_eos; /* 1=EOS wake up */
unsigned char ai_et;
unsigned int ai_et_CntrlReg;
@@ -733,7 +732,6 @@ static int pci171x_ai_cancel(struct comedi_device *dev,
devpriv->ai_act_scan = 0;
s->async->cur_chan = 0;
- devpriv->ai_buf_ptr = 0;
return 0;
}
@@ -976,7 +974,6 @@ static int pci171x_ai_cmd(struct comedi_device *dev, struct
comedi_subdevice *s)
devpriv->ai_act_scan = 0;
s->async->cur_chan = 0;
- devpriv->ai_buf_ptr = 0;
devpriv->CntrlReg &= Control_CNT0;
/* don't we want wake up every scan? devpriv->ai_eos=1; */
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 04/22] staging: comedi: adv_pci1710: remove 'ai_chanlist' from private data
This member of the private data is just a copy of the cmd->chanlist.
Use that instead.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index 447addd..4d2bdc1 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -316,7 +316,6 @@ struct pci1710_private {
unsigned char da_ranges;/* copy of D/A outpit range register */
unsigned int ai_scans; /* len of scanlist */
unsigned int ai_n_chan; /* how many channels is measured */
- unsigned int *ai_chanlist; /* actaul chanlist */
unsigned int ai_data_len; /* len of data buffer */
unsigned short ao_data[4]; /* data output buffer */
unsigned int cnt0_write_wait; /* after a write, wait for update of the
@@ -961,11 +960,11 @@ static int pci171x_ai_docmd_and_mode(int mode, struct
comedi_device *dev,
start_pacer(dev, -1, 0, 0); /* stop pacer */
- seglen = check_channel_list(dev, s, devpriv->ai_chanlist,
+ seglen = check_channel_list(dev, s, cmd->chanlist,
devpriv->ai_n_chan);
if (seglen < 1)
return -EINVAL;
- setup_channel_list(dev, s, devpriv->ai_chanlist,
+ setup_channel_list(dev, s, cmd->chanlist,
devpriv->ai_n_chan, seglen);
outb(0, dev->iobase + PCI171x_CLRFIFO);
@@ -1120,7 +1119,6 @@ static int pci171x_ai_cmd(struct comedi_device *dev,
struct comedi_subdevice *s)
struct comedi_cmd *cmd = &s->async->cmd;
devpriv->ai_n_chan = cmd->chanlist_len;
- devpriv->ai_chanlist = cmd->chanlist;
devpriv->ai_data_len = s->async->prealloc_bufsz;
if (cmd->stop_src == TRIG_COUNT)
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 02/22] staging: comedi: adv_pci1710: remove 'ai_timer1' from private data
This member of the private data is just a copy of the cmd->convert_arg.
Use that instead and remove the unnecessary sanity checking since it
was already validated in the (*do_cmdtest).
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index 0d0ec79..c1e0e41 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -319,7 +319,6 @@ struct pci1710_private {
unsigned int *ai_chanlist; /* actaul chanlist */
unsigned int ai_flags; /* flaglist */
unsigned int ai_data_len; /* len of data buffer */
- unsigned int ai_timer1; /* timers */
unsigned short ao_data[4]; /* data output buffer */
unsigned int cnt0_write_wait; /* after a write, wait for update of the
* internal state */
@@ -956,8 +955,8 @@ static irqreturn_t interrupt_service_pci1710(int irq, void
*d)
static int pci171x_ai_docmd_and_mode(int mode, struct comedi_device *dev,
struct comedi_subdevice *s)
{
- const struct boardtype *this_board = comedi_board(dev);
struct pci1710_private *devpriv = dev->private;
+ struct comedi_cmd *cmd = &s->async->cmd;
unsigned int divisor1 = 0, divisor2 = 0;
unsigned int seglen;
@@ -998,8 +997,6 @@ static int pci171x_ai_docmd_and_mode(int mode, struct
comedi_device *dev,
switch (mode) {
case 1:
case 2:
- if (devpriv->ai_timer1 < this_board->ai_ns_min)
- devpriv->ai_timer1 = this_board->ai_ns_min;
devpriv->CntrlReg |= Control_PACER | Control_IRQEN;
if (mode == 2) {
devpriv->ai_et_CntrlReg = devpriv->CntrlReg;
@@ -1012,7 +1009,7 @@ static int pci171x_ai_docmd_and_mode(int mode, struct
comedi_device *dev,
}
i8253_cascade_ns_to_timer(devpriv->i8254_osc_base,
&divisor1, &divisor2,
- &devpriv->ai_timer1,
+ &cmd->convert_arg,
devpriv->ai_flags);
outw(devpriv->CntrlReg, dev->iobase + PCI171x_CONTROL);
if (mode != 2) {
@@ -1127,7 +1124,6 @@ static int pci171x_ai_cmd(struct comedi_device *dev,
struct comedi_subdevice *s)
devpriv->ai_chanlist = cmd->chanlist;
devpriv->ai_flags = cmd->flags;
devpriv->ai_data_len = s->async->prealloc_bufsz;
- devpriv->ai_timer1 = 0;
if (cmd->stop_src == TRIG_COUNT)
devpriv->ai_scans = cmd->stop_arg;
@@ -1137,7 +1133,6 @@ static int pci171x_ai_cmd(struct comedi_device *dev,
struct comedi_subdevice *s)
if (cmd->scan_begin_src == TRIG_FOLLOW) { /* mode 1, 2, 3 */
if (cmd->convert_src == TRIG_TIMER) { /* mode 1 and 2 */
- devpriv->ai_timer1 = cmd->convert_arg;
return pci171x_ai_docmd_and_mode(cmd->start_src ==
TRIG_EXT ? 2 : 1, dev,
s);
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 01/22] staging: comedi: adv_pci1710: remove 'ai_timer2' from private data
This member of the private data is set to 0 but it is never used. Just
remove it.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index 28ec485..0d0ec79 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -320,7 +320,6 @@ struct pci1710_private {
unsigned int ai_flags; /* flaglist */
unsigned int ai_data_len; /* len of data buffer */
unsigned int ai_timer1; /* timers */
- unsigned int ai_timer2;
unsigned short ao_data[4]; /* data output buffer */
unsigned int cnt0_write_wait; /* after a write, wait for update of the
* internal state */
@@ -1129,7 +1128,6 @@ static int pci171x_ai_cmd(struct comedi_device *dev,
struct comedi_subdevice *s)
devpriv->ai_flags = cmd->flags;
devpriv->ai_data_len = s->async->prealloc_bufsz;
devpriv->ai_timer1 = 0;
- devpriv->ai_timer2 = 0;
if (cmd->stop_src == TRIG_COUNT)
devpriv->ai_scans = cmd->stop_arg;
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 06/22] staging: comedi: adv_pci1710: remove 'ai_scans' from private data
This member of the private data is just a copy of the cmd->stop_arg.
Use that instead.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 16 +---
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index 707def0..8fcc67a 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -314,7 +314,6 @@ struct pci1710_private {
unsigned char act_chanlist_len; /* len of scanlist */
unsigned char act_chanlist_pos; /* actual position in MUX list */
unsigned char da_ranges;/* copy of D/A outpit range register */
- unsigned int ai_scans; /* len of scanlist */
unsigned int ai_data_len; /* len of data buffer */
unsigned short ao_data[4]; /* data output buffer */
unsigned int cnt0_write_wait; /* after a write, wait for update of the
@@ -801,7 +800,7 @@ static void interrupt_pci1710_every_sample(void *d)
if (s->async->cur_chan == 0) { /* one scan done */
devpriv->ai_act_scan++;
if ((!devpriv->neverending_ai) &&
- (devpriv->ai_act_scan >= devpriv->ai_scans)) {
+ (devpriv->ai_act_scan >= cmd->stop_arg)) {
/* all data sampled */
s->async->events |= COMEDI_CB_EOA;
cfc_handle_events(dev, s);
@@ -870,6 +869,7 @@ static void interrupt_pci1710_half_fifo(void *d)
const struct boardtype *this_board = comedi_board(dev);
struct pci1710_private *devpriv = dev->private;
struct comedi_subdevice *s = dev->read_subdev;
+ struct comedi_cmd *cmd = &s->async->cmd;
int m, samplesinbuf;
m = inw(dev->iobase + PCI171x_STATUS);
@@ -901,8 +901,8 @@ static void interrupt_pci1710_half_fifo(void *d)
}
if (!devpriv->neverending_ai)
- if (devpriv->ai_act_scan >= devpriv->ai_scans) { /* all data
- sampled */
+ if (devpriv->ai_act_scan >= cmd->stop_arg) {
+ /* all data sampled */
s->async->events |= COMEDI_CB_EOA;
cfc_handle_events(dev, s);
return;
@@ -985,7 +985,7 @@ static int pci171x_ai_docmd_and_mode(int mode, struct
comedi_device *dev,
devpriv->ai_eos = 0;
}
- if ((devpriv->ai_scans == 0) || (devpriv->ai_scans == -1))
+ if (cmd->stop_arg == 0)
devpriv->neverending_ai = 1;
/* well, user want neverending */
else
@@ -1119,12 +1119,6 @@ static int pci171x_ai_cmd(struct comedi_device *dev,
struct comedi_subdevice *s)
devpriv->ai_data_len = s->async->prealloc_bufsz;
- if (cmd->stop_src == TRIG_COUNT)
- devpriv->ai_scans = cmd->stop_arg;
- else
- devpriv->ai_scans = 0;
-
-
if (cmd->scan_begin_src == TRIG_FOLLOW) { /* mode 1, 2, 3 */
if (cmd->convert_src == TRIG_TIMER) { /* mode 1 and 2 */
return pci171x_ai_docmd_and_mode(cmd->start_src ==
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 07/22] staging: comedi: adv_pci1710: remove 'ai_data_len' from private data
This member of the private data is just a copy of the s->async->prealloc_bufsz.
Use that instead.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index 8fcc67a..9ce0535 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -314,7 +314,6 @@ struct pci1710_private {
unsigned char act_chanlist_len; /* len of scanlist */
unsigned char act_chanlist_pos; /* actual position in MUX list */
unsigned char da_ranges;/* copy of D/A outpit range register */
- unsigned int ai_data_len; /* len of data buffer */
unsigned short ao_data[4]; /* data output buffer */
unsigned int cnt0_write_wait; /* after a write, wait for update of the
* internal state */
@@ -888,8 +887,8 @@ static void interrupt_pci1710_half_fifo(void *d)
}
samplesinbuf = this_board->fifo_half_size;
- if (samplesinbuf * sizeof(short) >= devpriv->ai_data_len) {
- m = devpriv->ai_data_len / sizeof(short);
+ if (samplesinbuf * sizeof(short) >= s->async->prealloc_bufsz) {
+ m = s->async->prealloc_bufsz / sizeof(short);
if (move_block_from_fifo(dev, s, m, 0))
return;
samplesinbuf -= m;
@@ -1114,11 +1113,8 @@ static int pci171x_ai_cmdtest(struct comedi_device *dev,
*/
static int pci171x_ai_cmd(struct comedi_device *dev, struct comedi_subdevice
*s)
{
- struct pci1710_private *devpriv = dev->private;
struct comedi_cmd *cmd = &s->async->cmd;
- devpriv->ai_data_len = s->async->prealloc_bufsz;
-
if (cmd->scan_begin_src == TRIG_FOLLOW) { /* mode 1, 2, 3 */
if (cmd->convert_src == TRIG_TIMER) { /* mode 1 and 2 */
return pci171x_ai_docmd_and_mode(cmd->start_src ==
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 20/22] staging: comedi: adv_pci1710: rename interrupt helper functions
For aesthetics, rename the helper functions that are called by
the interrupt function to handle reading the analog input samples.
Also, change the parameters to the helpers to the comedi_device
and comedi_subdevice pointers.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 20 ++--
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index 601d7e9..353b832 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -737,14 +737,10 @@ static int pci171x_ai_cancel(struct comedi_device *dev,
return 0;
}
-/*
-==
-*/
-static void interrupt_pci1710_every_sample(void *d)
+static void pci1710_handle_every_sample(struct comedi_device *dev,
+ struct comedi_subdevice *s)
{
- struct comedi_device *dev = d;
struct pci1710_private *devpriv = dev->private;
- struct comedi_subdevice *s = dev->read_subdev;
struct comedi_cmd *cmd = &s->async->cmd;
int m;
#ifdef PCI171x_PARANOIDCHECK
@@ -861,15 +857,11 @@ static int move_block_from_fifo(struct comedi_device *dev,
return 0;
}
-/*
-==
-*/
-static void interrupt_pci1710_half_fifo(void *d)
+static void pci1710_handle_fifo(struct comedi_device *dev,
+ struct comedi_subdevice *s)
{
- struct comedi_device *dev = d;
const struct boardtype *this_board = comedi_board(dev);
struct pci1710_private *devpriv = dev->private;
- struct comedi_subdevice *s = dev->read_subdev;
struct comedi_cmd *cmd = &s->async->cmd;
int m, samplesinbuf;
@@ -949,9 +941,9 @@ static irqreturn_t interrupt_service_pci1710(int irq, void
*d)
}
if (cmd->flags & TRIG_WAKE_EOS)
- interrupt_pci1710_every_sample(d);
+ pci1710_handle_every_sample(dev, s);
else
- interrupt_pci1710_half_fifo(d);
+ pci1710_handle_fifo(dev, s);
return IRQ_HANDLED;
}
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 14/22] staging: comedi: adv_pci1710: remove 'neverending_ai' from private data
This member of the private data is not necessary. We can determine if
the analog input command is neverending by checking the cmd->stop_src:
TRIG_COUNT -> !neverending_ai
TRIG_NONE -> neverending_ai
Do that instead and remove the unnecessary member.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 27 +--
1 file changed, 9 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index 6698d3c..e6c002a 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -298,7 +298,6 @@ static const struct boardtype boardtypes[] = {
};
struct pci1710_private {
- char neverending_ai;/* we do unlimited AI */
unsigned int CntrlReg; /* Control register */
unsigned int i8254_osc_base;/* frequence of onboard oscilator */
unsigned int ai_act_scan; /* how many scans we finished */
@@ -735,7 +734,6 @@ static int pci171x_ai_cancel(struct comedi_device *dev,
devpriv->ai_act_scan = 0;
s->async->cur_chan = 0;
devpriv->ai_buf_ptr = 0;
- devpriv->neverending_ai = 0;
return 0;
}
@@ -803,8 +801,8 @@ static void interrupt_pci1710_every_sample(void *d)
if (s->async->cur_chan == 0) { /* one scan done */
devpriv->ai_act_scan++;
- if ((!devpriv->neverending_ai) &&
- (devpriv->ai_act_scan >= cmd->stop_arg)) {
+ if (cmd->stop_src == TRIG_COUNT &&
+ devpriv->ai_act_scan >= cmd->stop_arg) {
/* all data sampled */
s->async->events |= COMEDI_CB_EOA;
cfc_handle_events(dev, s);
@@ -904,13 +902,13 @@ static void interrupt_pci1710_half_fifo(void *d)
return;
}
- if (!devpriv->neverending_ai)
- if (devpriv->ai_act_scan >= cmd->stop_arg) {
- /* all data sampled */
- s->async->events |= COMEDI_CB_EOA;
- cfc_handle_events(dev, s);
- return;
- }
+ if (cmd->stop_src == TRIG_COUNT &&
+ devpriv->ai_act_scan >= cmd->stop_arg) {
+ /* all data sampled */
+ s->async->events |= COMEDI_CB_EOA;
+ cfc_handle_events(dev, s);
+ return;
+ }
outb(0, dev->iobase + PCI171x_CLRINT); /* clear our INT request */
cfc_handle_events(dev, s);
@@ -979,7 +977,6 @@ static int pci171x_ai_cmd(struct comedi_device *dev, struct
comedi_subdevice *s)
devpriv->ai_act_scan = 0;
s->async->cur_chan = 0;
devpriv->ai_buf_ptr = 0;
- devpriv->neverending_ai = 0;
devpriv->CntrlReg &= Control_CNT0;
/* don't we want wake up every scan? devpriv->ai_eos=1; */
@@ -990,12 +987,6 @@ static int pci171x_ai_cmd(struct comedi_device *dev,
struct comedi_subdevice *s)
devpriv->ai_eos = 0;
}
- if (cmd->stop_arg == 0)
- devpriv->neverending_ai = 1;
- /* well, user want neverending */
- else
- devpriv->neverending_ai = 0;
-
switch (mode) {
case 1:
case 2:
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 10/22] staging: comedi: adv_pci1710: remove 'ai_do' from private data
This member of the private data is set to the "mode" that the ai command
is operating in but nothing uses it. Just remove it.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index 6ab78bb..24dcc2e 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -301,7 +301,6 @@ struct pci1710_private {
char neverending_ai;/* we do unlimited AI */
unsigned int CntrlReg; /* Control register */
unsigned int i8254_osc_base;/* frequence of onboard oscilator */
- unsigned int ai_do; /* what do AI? 0=nothing, 1 to 4 mode */
unsigned int ai_act_scan; /* how many scans we finished */
unsigned int ai_act_chan; /* actual position in actual scan */
unsigned int ai_buf_ptr;/* data buffer ptr in samples */
@@ -726,7 +725,6 @@ static int pci171x_ai_cancel(struct comedi_device *dev,
break;
}
- devpriv->ai_do = 0;
devpriv->ai_act_scan = 0;
s->async->cur_chan = 0;
devpriv->ai_buf_ptr = 0;
@@ -974,8 +972,6 @@ static int pci171x_ai_cmd(struct comedi_device *dev, struct
comedi_subdevice *s)
outb(0, dev->iobase + PCI171x_CLRFIFO);
outb(0, dev->iobase + PCI171x_CLRINT);
- devpriv->ai_do = mode;
-
devpriv->ai_act_scan = 0;
s->async->cur_chan = 0;
devpriv->ai_buf_ptr = 0;
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 16/22] staging: comedi: adv_pci1710: remove 'ai_eos' from private data
This member of the private data is is not necessary. We can just check the
cmd->flags for TRIG_WAKE_EOS when needed. Remvoe the member.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 21 +++--
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index b8e7e2f..c1aa16a 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -302,7 +302,6 @@ struct pci1710_private {
unsigned int i8254_osc_base;/* frequence of onboard oscilator */
unsigned int ai_act_scan; /* how many scans we finished */
unsigned int ai_act_chan; /* actual position in actual scan */
- unsigned char ai_eos; /* 1=EOS wake up */
unsigned char ai_et;
unsigned int ai_et_CntrlReg;
unsigned int ai_et_MuxVal;
@@ -919,9 +918,15 @@ static irqreturn_t interrupt_service_pci1710(int irq, void
*d)
{
struct comedi_device *dev = d;
struct pci1710_private *devpriv = dev->private;
+ struct comedi_subdevice *s;
+ struct comedi_cmd *cmd;
if (!dev->attached) /* is device attached? */
return IRQ_NONE;/* no, exit */
+
+ s = dev->read_subdev;
+ cmd = &s->async->cmd;
+
/* is this interrupt from our board? */
if (!(inw(dev->iobase + PCI171x_STATUS) & Status_IRQ))
return IRQ_NONE;/* no, exit */
@@ -940,11 +945,12 @@ static irqreturn_t interrupt_service_pci1710(int irq,
void *d)
start_pacer(dev, 1, devpriv->ai_et_div1, devpriv->ai_et_div2);
return IRQ_HANDLED;
}
- if (devpriv->ai_eos) { /* We use FIFO half full INT or not? */
+
+ if (cmd->flags & TRIG_WAKE_EOS)
interrupt_pci1710_every_sample(d);
- } else {
+ else
interrupt_pci1710_half_fifo(d);
- }
+
return IRQ_HANDLED;
}
@@ -976,13 +982,8 @@ static int pci171x_ai_cmd(struct comedi_device *dev,
struct comedi_subdevice *s)
s->async->cur_chan = 0;
devpriv->CntrlReg &= Control_CNT0;
- /* don't we want wake up every scan? devpriv->ai_eos=1; */
- if (cmd->flags & TRIG_WAKE_EOS) {
- devpriv->ai_eos = 1;
- } else {
+ if ((cmd->flags & TRIG_WAKE_EOS) == 0)
devpriv->CntrlReg |= Control_ONEFH;
- devpriv->ai_eos = 0;
- }
switch (mode) {
case 1:
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 11/22] staging: comedi: adv_pci1710: rename check_channel_list()
For aesthetics, rename this function so it has namespace associated
with the driver.
Also, change it's parameters. The cmd->chanlist and cmd->chanlist_len
are always passed, just pass the comedi_cmd pointer instead.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 19 +--
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index 24dcc2e..765b1f0 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -332,12 +332,14 @@ static const unsigned int muxonechan[] = {
If it's ok, then program scan/gain logic.
This works for all cards.
*/
-static int check_channel_list(struct comedi_device *dev,
- struct comedi_subdevice *s,
- unsigned int *chanlist, unsigned int n_chan)
+static int pci171x_ai_check_chanlist(struct comedi_device *dev,
+struct comedi_subdevice *s,
+struct comedi_cmd *cmd)
{
unsigned int chansegment[32];
unsigned int i, nowmustbechan, seglen, segpos;
+ unsigned int *chanlist = cmd->chanlist;
+ unsigned int n_chan = cmd->chanlist_len;
/* correct channel and range number check itself comedi/range.c */
if (n_chan < 1) {
@@ -964,7 +966,7 @@ static int pci171x_ai_cmd(struct comedi_device *dev, struct
comedi_subdevice *s)
start_pacer(dev, -1, 0, 0); /* stop pacer */
- seglen = check_channel_list(dev, s, cmd->chanlist, cmd->chanlist_len);
+ seglen = pci171x_ai_check_chanlist(dev, s, cmd);
if (seglen < 1)
return -EINVAL;
setup_channel_list(dev, s, cmd->chanlist, cmd->chanlist_len, seglen);
@@ -1099,13 +1101,10 @@ static int pci171x_ai_cmdtest(struct comedi_device *dev,
if (err)
return 4;
- /* step 5: complain about special chanlist considerations */
+ /* Step 5: check channel list */
- if (cmd->chanlist) {
- if (!check_channel_list(dev, s, cmd->chanlist,
- cmd->chanlist_len))
- return 5; /* incorrect channels list */
- }
+ if (!pci171x_ai_check_chanlist(dev, s, cmd))
+ return 5;
return 0;
}
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 19/22] staging: comedi: adv_pci1710: tidy up start_pacer()
For aesthetics, rename this function so it has namespace associated
with the driver.
Change the parameters to the function. The 'mode' is really a flag to
load the counters and the divisors can be found in the private data.
To clarify the code and remove the magic numbers, use the 8253.h
helpers to set the timer mode and load the counters.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 42 +---
1 file changed, 20 insertions(+), 22 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index a9e1cda..601d7e9 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -73,6 +73,9 @@ Configuration options:
#define PCI171x_DAREF 14 /* W: D/A reference control */
#define PCI171x_DI 16 /* R: digi inputs */
#define PCI171x_DO 16 /* R: digi inputs */
+
+#define PCI171X_TIMER_BASE 0x18
+
#define PCI171x_CNT0 24 /* R/W: 8254 counter 0 */
#define PCI171x_CNT1 26 /* R/W: 8254 counter 1 */
#define PCI171x_CNT2 28 /* R/W: 8254 counter 2 */
@@ -568,20 +571,18 @@ static int pci171x_insn_bits_do(struct comedi_device *dev,
return insn->n;
}
-/*
-==
-*/
-static void start_pacer(struct comedi_device *dev, int mode,
- unsigned int divisor1, unsigned int divisor2)
+static void pci171x_start_pacer(struct comedi_device *dev,
+ bool load_counters)
{
- outw(0xb4, dev->iobase + PCI171x_CNTCTRL);
- outw(0x74, dev->iobase + PCI171x_CNTCTRL);
-
- if (mode == 1) {
- outw(divisor2 & 0xff, dev->iobase + PCI171x_CNT2);
- outw((divisor2 >> 8) & 0xff, dev->iobase + PCI171x_CNT2);
- outw(divisor1 & 0xff, dev->iobase + PCI171x_CNT1);
- outw((divisor1 >> 8) & 0xff, dev->iobase + PCI171x_CNT1);
+ struct pci1710_private *devpriv = dev->private;
+ unsigned long timer_base = dev->iobase + PCI171X_TIMER_BASE;
+
+ i8254_set_mode(timer_base, 1, 2, I8254_MODE2 | I8254_BINARY);
+ i8254_set_mode(timer_base, 1, 1, I8254_MODE2 | I8254_BINARY);
+
+ if (load_counters) {
+ i8254_write(timer_base, 1, 2, devpriv->divisor2);
+ i8254_write(timer_base, 1, 1, devpriv->divisor2);
}
}
@@ -724,7 +725,7 @@ static int pci171x_ai_cancel(struct comedi_device *dev,
devpriv->CntrlReg |= Control_SW;
outw(devpriv->CntrlReg, dev->iobase + PCI171x_CONTROL); /*
reset any operations */
- start_pacer(dev, -1, 0, 0);
+ pci171x_start_pacer(dev, false);
outb(0, dev->iobase + PCI171x_CLRFIFO);
outb(0, dev->iobase + PCI171x_CLRINT);
break;
@@ -943,7 +944,7 @@ static irqreturn_t interrupt_service_pci1710(int irq, void
*d)
outw(devpriv->ai_et_MuxVal, dev->iobase + PCI171x_MUX);
outw(devpriv->CntrlReg, dev->iobase + PCI171x_CONTROL);
/* start pacer */
- start_pacer(dev, 1, devpriv->divisor1, devpriv->divisor2);
+ pci171x_start_pacer(dev, true);
return IRQ_HANDLED;
}
@@ -960,7 +961,7 @@ static int pci171x_ai_cmd(struct comedi_device *dev, struct
comedi_subdevice *s)
struct pci1710_private *devpriv = dev->private;
struct comedi_cmd *cmd = &s->async->cmd;
- start_pacer(dev, -1, 0, 0); /* stop pacer */
+ pci171x_start_pacer(dev, false);
setup_channel_list(dev, s, cmd->chanlist, cmd->chanlist_len,
devpriv->act_chanlist_len);
@@ -988,11 +989,8 @@ static int pci171x_ai_cmd(struct comedi_device *dev,
struct comedi_subdevice *s)
}
outw(devpriv->CntrlReg, dev->iobase + PCI171x_CONTROL);
- if (cmd->start_src == TRIG_NOW) {
- /* start pacer */
- start_pacer(dev, 1,
- devpriv->divisor1, devpriv->divisor2);
- }
+ if (cmd->start_src == TRIG_NOW)
+ pci171x_start_pacer(dev, true);
} else {/* TRIG_EXT */
devpriv->CntrlReg |= Control_EXT | Control_IRQEN;
outw(devpriv->CntrlReg, dev->iobase + PCI171x_CONTROL);
@@ -1096,7 +1094,7 @@ static int pci171x_reset(struct comedi_device *dev)
outw(devpriv->CntrlReg, dev->iobase + PCI171x_CONTROL); /* reset any
operations */
outb(0, dev->iobase + PCI171x_CLRFIFO); /* clear FIFO */
outb(0, dev->iobase + PCI171x_CLRINT); /* clear INT request */
- start_pacer(dev, -1, 0, 0); /* stop 8254 */
+ pci171x_start_pacer(dev, false);
devpriv->da_ranges = 0;
i
[PATCH 21/22] staging: comedi: adv_pci1710: handle events and clear interrupt in common code
The helper functions that handle reading the analog input samples for
the interrupt function both call cfc_handle_events() and clear the
interrupt request at various times.
Move this to the main interrupt handler to make sure the events are
posted and the interrupt request is cleared.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 22 --
1 file changed, 4 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index 353b832..56186f0 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -752,19 +752,15 @@ static void pci1710_handle_every_sample(struct
comedi_device *dev,
if (m & Status_FE) {
dev_dbg(dev->class_dev, "A/D FIFO empty (%4x)\n", m);
s->async->events |= COMEDI_CB_EOA | COMEDI_CB_ERROR;
- cfc_handle_events(dev, s);
return;
}
if (m & Status_FF) {
dev_dbg(dev->class_dev,
"A/D FIFO Full status (Fatal Error!) (%4x)\n", m);
s->async->events |= COMEDI_CB_EOA | COMEDI_CB_ERROR;
- cfc_handle_events(dev, s);
return;
}
- outb(0, dev->iobase + PCI171x_CLRINT); /* clear our INT request */
-
for (; !(inw(dev->iobase + PCI171x_STATUS) & Status_FE);) {
#ifdef PCI171x_PARANOIDCHECK
sampl = inw(dev->iobase + PCI171x_AD_DATA);
@@ -780,7 +776,6 @@ static void pci1710_handle_every_sample(struct
comedi_device *dev,
12);
s->async->events |=
COMEDI_CB_EOA | COMEDI_CB_ERROR;
- cfc_handle_events(dev, s);
return;
}
comedi_buf_put(s->async, sampl & 0x0fff);
@@ -793,22 +788,16 @@ static void pci1710_handle_every_sample(struct
comedi_device *dev,
if (s->async->cur_chan >= cmd->chanlist_len)
s->async->cur_chan = 0;
-
if (s->async->cur_chan == 0) { /* one scan done */
devpriv->ai_act_scan++;
if (cmd->stop_src == TRIG_COUNT &&
devpriv->ai_act_scan >= cmd->stop_arg) {
/* all data sampled */
s->async->events |= COMEDI_CB_EOA;
- cfc_handle_events(dev, s);
return;
}
}
}
-
- outb(0, dev->iobase + PCI171x_CLRINT); /* clear our INT request */
-
- cfc_handle_events(dev, s);
}
/*
@@ -839,7 +828,6 @@ static int move_block_from_fifo(struct comedi_device *dev,
sampl);
s->async->events |=
COMEDI_CB_EOA | COMEDI_CB_ERROR;
- cfc_handle_events(dev, s);
return 1;
}
comedi_buf_put(s->async, sampl & 0x0fff);
@@ -869,14 +857,12 @@ static void pci1710_handle_fifo(struct comedi_device *dev,
if (!(m & Status_FH)) {
dev_dbg(dev->class_dev, "A/D FIFO not half full! (%4x)\n", m);
s->async->events |= COMEDI_CB_EOA | COMEDI_CB_ERROR;
- cfc_handle_events(dev, s);
return;
}
if (m & Status_FF) {
dev_dbg(dev->class_dev,
"A/D FIFO Full status (Fatal Error!) (%4x)\n", m);
s->async->events |= COMEDI_CB_EOA | COMEDI_CB_ERROR;
- cfc_handle_events(dev, s);
return;
}
@@ -897,12 +883,8 @@ static void pci1710_handle_fifo(struct comedi_device *dev,
devpriv->ai_act_scan >= cmd->stop_arg) {
/* all data sampled */
s->async->events |= COMEDI_CB_EOA;
- cfc_handle_events(dev, s);
return;
}
- outb(0, dev->iobase + PCI171x_CLRINT); /* clear our INT request */
-
- cfc_handle_events(dev, s);
}
/*
@@ -945,6 +927,10 @@ static irqreturn_t interrupt_service_pci1710(int irq, void
*d)
else
pci1710_handle_fifo(dev, s);
+ cfc_handle_events(dev, s);
+
+ outb(0, dev->iobase + PCI171x_CLRINT);
+
return IRQ_HANDLED;
}
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 22/22] staging: comedi: adv_pci1710: always enable PCI171x_PARANOIDCHECK code
This define enables code that checks for analog input channel dropout
when reading sampled. The define is enabled so we might as well always
enable the code and remove the define.
Factor out the common channel dropout detect code as a helper function.
And cleanup the code.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 146 +++
1 file changed, 60 insertions(+), 86 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index 56186f0..a69a283 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -51,10 +51,6 @@ Configuration options:
#include "8253.h"
#include "amcc_s5933.h"
-#define PCI171x_PARANOIDCHECK /* if defined, then is used code which control
-* correct channel number on every 12 bit
-* sample */
-
/* hardware types of the cards */
#define TYPE_PCI171X 0
#define TYPE_PCI1713 2
@@ -327,6 +323,26 @@ static const unsigned int muxonechan[] = {
0x1818, 0x1919, 0x1a1a, 0x1b1b, 0x1c1c, 0x1d1d, 0x1e1e, 0x1f1f
};
+static int pci171x_ai_dropout(struct comedi_device *dev,
+ struct comedi_subdevice *s,
+ unsigned int chan,
+ unsigned int val)
+{
+ const struct boardtype *board = comedi_board(dev);
+ struct pci1710_private *devpriv = dev->private;
+
+ if (board->cardtype != TYPE_PCI1713) {
+ if ((val & 0xf000) != devpriv->act_chanlist[chan]) {
+ dev_err(dev->class_dev,
+ "A/D data droput: received from channel %d,
expected %d\n",
+ (val >> 12) & 0xf,
+ (devpriv->act_chanlist[chan] >> 12) & 0xf);
+ return -ENODATA;
+ }
+ }
+ return 0;
+}
+
static int pci171x_ai_check_chanlist(struct comedi_device *dev,
struct comedi_subdevice *s,
struct comedi_cmd *cmd)
@@ -411,17 +427,13 @@ static void setup_channel_list(struct comedi_device *dev,
if (CR_AREF(chanlist[i]) == AREF_DIFF)
range |= 0x0020;
outw(range, dev->iobase + PCI171x_RANGE); /* select gain */
-#ifdef PCI171x_PARANOIDCHECK
devpriv->act_chanlist[i] =
(CR_CHAN(chanlist[i]) << 12) & 0xf000;
-#endif
}
-#ifdef PCI171x_PARANOIDCHECK
for ( ; i < n_chan; i++) { /* store remainder of channel list */
devpriv->act_chanlist[i] =
(CR_CHAN(chanlist[i]) << 12) & 0xf000;
}
-#endif
devpriv->ai_et_MuxVal =
CR_CHAN(chanlist[0]) | (CR_CHAN(chanlist[seglen - 1]) << 8);
@@ -447,12 +459,9 @@ static int pci171x_insn_read_ai(struct comedi_device *dev,
struct comedi_insn *insn, unsigned int *data)
{
struct pci1710_private *devpriv = dev->private;
- int ret;
+ unsigned int chan = CR_CHAN(insn->chanspec);
+ int ret = 0;
int n;
-#ifdef PCI171x_PARANOIDCHECK
- const struct boardtype *this_board = comedi_board(dev);
- unsigned int idata;
-#endif
devpriv->CntrlReg &= Control_CNT0;
devpriv->CntrlReg |= Control_SW;/* set software trigger */
@@ -463,33 +472,26 @@ static int pci171x_insn_read_ai(struct comedi_device *dev,
setup_channel_list(dev, s, &insn->chanspec, 1, 1);
for (n = 0; n < insn->n; n++) {
+ unsigned int val;
+
outw(0, dev->iobase + PCI171x_SOFTTRG); /* start conversion */
ret = comedi_timeout(dev, s, insn, pci171x_ai_eoc, 0);
- if (ret) {
- outb(0, dev->iobase + PCI171x_CLRFIFO);
- outb(0, dev->iobase + PCI171x_CLRINT);
- return ret;
- }
+ if (ret)
+ break;
-#ifdef PCI171x_PARANOIDCHECK
- idata = inw(dev->iobase + PCI171x_AD_DATA);
- if (this_board->cardtype != TYPE_PCI1713)
- if ((idata & 0xf000) != devpriv->act_chanlist[0]) {
- comedi_error(dev, "A/D insn data droput!");
- return -ETIME;
- }
- data[n] = idata & 0x0fff;
-#else
- data[n] = inw(dev->iobase + PCI171x_AD_DATA) & 0x0fff;
-#endif
+ val = inw(dev->iobase + PCI171x_AD_DATA);
+ ret = pci171x_ai_dropout(dev, s, chan, val);
+ if (ret)
+ break;
+ data[n] = val & s->maxdata;
}
outb(0, dev->iobase + PCI171x_CLRFIFO);
outb(0, dev->io
[PATCH 18/22] staging: comedi: adv_pci1710: remove local var in pci171x_ai_cmd()
The local variable 'mode' is not necessary. We can determine the mode
by checking the cmd->convert_src and cmd->start_src. Do this instead
to clarify the code.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 27 +++
1 file changed, 7 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index 99e3a30..a9e1cda 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -959,16 +959,6 @@ static int pci171x_ai_cmd(struct comedi_device *dev,
struct comedi_subdevice *s)
{
struct pci1710_private *devpriv = dev->private;
struct comedi_cmd *cmd = &s->async->cmd;
- int mode;
-
- if (cmd->convert_src == TRIG_TIMER) {
- if (cmd->start_src == TRIG_EXT)
- mode = 2;
- else
- mode = 1;
- } else {/* TRIG_EXT */
- mode = 3;
- }
start_pacer(dev, -1, 0, 0); /* stop pacer */
@@ -985,30 +975,27 @@ static int pci171x_ai_cmd(struct comedi_device *dev,
struct comedi_subdevice *s)
if ((cmd->flags & TRIG_WAKE_EOS) == 0)
devpriv->CntrlReg |= Control_ONEFH;
- switch (mode) {
- case 1:
- case 2:
+ if (cmd->convert_src == TRIG_TIMER) {
devpriv->CntrlReg |= Control_PACER | Control_IRQEN;
- if (mode == 2) {
+ if (cmd->start_src == TRIG_EXT) {
devpriv->ai_et_CntrlReg = devpriv->CntrlReg;
devpriv->CntrlReg &=
~(Control_PACER | Control_ONEFH | Control_GATE);
devpriv->CntrlReg |= Control_EXT;
devpriv->ai_et = 1;
- } else {
+ } else {/* TRIG_NOW */
devpriv->ai_et = 0;
}
outw(devpriv->CntrlReg, dev->iobase + PCI171x_CONTROL);
- if (mode != 2) {
+
+ if (cmd->start_src == TRIG_NOW) {
/* start pacer */
- start_pacer(dev, mode,
+ start_pacer(dev, 1,
devpriv->divisor1, devpriv->divisor2);
}
- break;
- case 3:
+ } else {/* TRIG_EXT */
devpriv->CntrlReg |= Control_EXT | Control_IRQEN;
outw(devpriv->CntrlReg, dev->iobase + PCI171x_CONTROL);
- break;
}
return 0;
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 08/22] staging: comedi: adv_pci1710: cmd->scan_begin_src can only be TRIG_FOLLOW
In Step 1 of the (*do_cmdtest), the cmd->scan_begin_src is checked to
only allow TRIG_FOLLOW. The (*do_cmd) does not need to recheck this.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 16 +++-
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index 9ce0535..60b926e 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -1115,15 +1115,13 @@ static int pci171x_ai_cmd(struct comedi_device *dev,
struct comedi_subdevice *s)
{
struct comedi_cmd *cmd = &s->async->cmd;
- if (cmd->scan_begin_src == TRIG_FOLLOW) { /* mode 1, 2, 3 */
- if (cmd->convert_src == TRIG_TIMER) { /* mode 1 and 2 */
- return pci171x_ai_docmd_and_mode(cmd->start_src ==
-TRIG_EXT ? 2 : 1, dev,
-s);
- }
- if (cmd->convert_src == TRIG_EXT) { /* mode 3 */
- return pci171x_ai_docmd_and_mode(3, dev, s);
- }
+ if (cmd->convert_src == TRIG_TIMER) { /* mode 1 and 2 */
+ return pci171x_ai_docmd_and_mode(cmd->start_src ==
+TRIG_EXT ? 2 : 1, dev,
+s);
+ }
+ if (cmd->convert_src == TRIG_EXT) { /* mode 3 */
+ return pci171x_ai_docmd_and_mode(3, dev, s);
}
return -1;
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 12/22] staging: comedi: adv_pci1710: tidy up pci171x_ai_check_chanlist()
Tidy up this function to clarify what the chanlist is being checked for.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 81 ++--
1 file changed, 41 insertions(+), 40 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index 765b1f0..59c8bc4 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -326,60 +326,61 @@ static const unsigned int muxonechan[] = {
0x1818, 0x1919, 0x1a1a, 0x1b1b, 0x1c1c, 0x1d1d, 0x1e1e, 0x1f1f
};
-/*
-==
- Check if channel list from user is built correctly
- If it's ok, then program scan/gain logic.
- This works for all cards.
-*/
static int pci171x_ai_check_chanlist(struct comedi_device *dev,
struct comedi_subdevice *s,
struct comedi_cmd *cmd)
{
+ unsigned int chan0 = CR_CHAN(cmd->chanlist[0]);
+ unsigned int last_aref = CR_AREF(cmd->chanlist[0]);
+ unsigned int next_chan = (chan0 + 1) & s->n_chan;
unsigned int chansegment[32];
- unsigned int i, nowmustbechan, seglen, segpos;
- unsigned int *chanlist = cmd->chanlist;
- unsigned int n_chan = cmd->chanlist_len;
-
- /* correct channel and range number check itself comedi/range.c */
- if (n_chan < 1) {
- comedi_error(dev, "range/channel list is empty!");
- return 0;
- }
+ unsigned int seglen;
+ int i;
- if (n_chan == 1)
+ if (cmd->chanlist_len == 1)
return 1; /* seglen=1 */
- chansegment[0] = chanlist[0]; /* first channel is every time ok */
- for (i = 1, seglen = 1; i < n_chan; i++, seglen++) {
- if (chanlist[0] == chanlist[i])
+ /* first channel is always ok */
+ chansegment[0] = cmd->chanlist[0];
+
+ for (i = 1; i < cmd->chanlist_len; i++) {
+ unsigned int chan = CR_CHAN(cmd->chanlist[i]);
+ unsigned int aref = CR_AREF(cmd->chanlist[i]);
+
+ if (cmd->chanlist[0] == cmd->chanlist[i])
break; /* we detected a loop, stop */
- if ((CR_CHAN(chanlist[i]) & 1) &&
- (CR_AREF(chanlist[i]) == AREF_DIFF)) {
- comedi_error(dev, "Odd channel cannot be differential
input!\n");
+
+ if (aref == AREF_DIFF && (chan & 1)) {
+ dev_err(dev->class_dev,
+ "Odd channel cannot be differential input!\n");
return 0;
}
- nowmustbechan = (CR_CHAN(chansegment[i - 1]) + 1) % s->n_chan;
- if (CR_AREF(chansegment[i - 1]) == AREF_DIFF)
- nowmustbechan = (nowmustbechan + 1) % s->n_chan;
- if (nowmustbechan != CR_CHAN(chanlist[i])) {
- printk("channel list must be continuous!
chanlist[%i]=%d but must be %d or %d!\n",
- i, CR_CHAN(chanlist[i]), nowmustbechan,
- CR_CHAN(chanlist[0]));
+
+ if (last_aref == AREF_DIFF)
+ next_chan = (next_chan + 1) % s->n_chan;
+ if (chan != next_chan) {
+ dev_err(dev->class_dev,
+ "channel list must be continuous!
chanlist[%i]=%d but must be %d or %d!\n",
+ i, chan, next_chan, chan0);
return 0;
}
- chansegment[i] = chanlist[i]; /* next correct channel in list */
- }
- for (i = 0, segpos = 0; i < n_chan; i++) {
- if (chanlist[i] != chansegment[i % seglen]) {
- printk("bad channel, reference or range number!
chanlist[%i]=%d,%d,%d and not %d,%d,%d!\n",
- i, CR_CHAN(chansegment[i]),
- CR_RANGE(chansegment[i]),
- CR_AREF(chansegment[i]),
- CR_CHAN(chanlist[i % seglen]),
- CR_RANGE(chanlist[i % seglen]),
- CR_AREF(chansegment[i % seglen]));
+ /* next correct channel in list */
+ chansegment[i] = cmd->chanlist[i];
+ last_aref = aref;
+ }
+ seglen = i;
+
+ for (i = 0; i < cmd->chanlist_len; i++) {
+ if (cmd->chanlist[i] != chansegment[i % seglen]) {
+ dev_err(dev->class_dev,
+ "bad channel, reference or range number!
chanlist[%i]=%d,%d,%d and not %d,%d,%d!\n",
+ i, CR_CHAN(chansegment[i]),
+ CR_RANGE(chansegment[i]),
+
[PATCH 05/22] staging: comedi: adv_pci1710: remove 'ai_n_chan' from private data
This member of the private data is just a copy of the cmd->chanlist_len.
Use that instead.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index 4d2bdc1..707def0 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -315,7 +315,6 @@ struct pci1710_private {
unsigned char act_chanlist_pos; /* actual position in MUX list */
unsigned char da_ranges;/* copy of D/A outpit range register */
unsigned int ai_scans; /* len of scanlist */
- unsigned int ai_n_chan; /* how many channels is measured */
unsigned int ai_data_len; /* len of data buffer */
unsigned short ao_data[4]; /* data output buffer */
unsigned int cnt0_write_wait; /* after a write, wait for update of the
@@ -746,6 +745,7 @@ static void interrupt_pci1710_every_sample(void *d)
struct comedi_device *dev = d;
struct pci1710_private *devpriv = dev->private;
struct comedi_subdevice *s = dev->read_subdev;
+ struct comedi_cmd *cmd = &s->async->cmd;
int m;
#ifdef PCI171x_PARANOIDCHECK
const struct boardtype *this_board = comedi_board(dev);
@@ -794,7 +794,7 @@ static void interrupt_pci1710_every_sample(void *d)
#endif
++s->async->cur_chan;
- if (s->async->cur_chan >= devpriv->ai_n_chan)
+ if (s->async->cur_chan >= cmd->chanlist_len)
s->async->cur_chan = 0;
@@ -822,6 +822,7 @@ static int move_block_from_fifo(struct comedi_device *dev,
struct comedi_subdevice *s, int n, int turn)
{
struct pci1710_private *devpriv = dev->private;
+ struct comedi_cmd *cmd = &s->async->cmd;
int i, j;
#ifdef PCI171x_PARANOIDCHECK
const struct boardtype *this_board = comedi_board(dev);
@@ -851,7 +852,7 @@ static int move_block_from_fifo(struct comedi_device *dev,
inw(dev->iobase + PCI171x_AD_DATA) & 0x0fff);
#endif
j++;
- if (j >= devpriv->ai_n_chan) {
+ if (j >= cmd->chanlist_len) {
j = 0;
devpriv->ai_act_scan++;
}
@@ -960,12 +961,10 @@ static int pci171x_ai_docmd_and_mode(int mode, struct
comedi_device *dev,
start_pacer(dev, -1, 0, 0); /* stop pacer */
- seglen = check_channel_list(dev, s, cmd->chanlist,
- devpriv->ai_n_chan);
+ seglen = check_channel_list(dev, s, cmd->chanlist, cmd->chanlist_len);
if (seglen < 1)
return -EINVAL;
- setup_channel_list(dev, s, cmd->chanlist,
- devpriv->ai_n_chan, seglen);
+ setup_channel_list(dev, s, cmd->chanlist, cmd->chanlist_len, seglen);
outb(0, dev->iobase + PCI171x_CLRFIFO);
outb(0, dev->iobase + PCI171x_CLRINT);
@@ -1118,7 +1117,6 @@ static int pci171x_ai_cmd(struct comedi_device *dev,
struct comedi_subdevice *s)
struct pci1710_private *devpriv = dev->private;
struct comedi_cmd *cmd = &s->async->cmd;
- devpriv->ai_n_chan = cmd->chanlist_len;
devpriv->ai_data_len = s->async->prealloc_bufsz;
if (cmd->stop_src == TRIG_COUNT)
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 13/22] staging: comedi: adv_pci1710: don't check the chanlist twice
The chanlist is checked in Step 5 of the (*do_cmdtest) there is no
reason to check it again in the (*do_cmd). The only reasonm its done
is to get the actual 'seglen', the non-repeating length of the chanlist.
Save the 'seglen' found by pci171x_ai_check_chanlist() in the private
data and use that in the (*do_cmd).
Refactor the error handling in pci171x_ai_check_chanlist() to work like
the cfc_check_*() helpers.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 29 +++-
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index 59c8bc4..6698d3c 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -330,6 +330,7 @@ static int pci171x_ai_check_chanlist(struct comedi_device
*dev,
struct comedi_subdevice *s,
struct comedi_cmd *cmd)
{
+ struct pci1710_private *devpriv = dev->private;
unsigned int chan0 = CR_CHAN(cmd->chanlist[0]);
unsigned int last_aref = CR_AREF(cmd->chanlist[0]);
unsigned int next_chan = (chan0 + 1) & s->n_chan;
@@ -337,8 +338,10 @@ static int pci171x_ai_check_chanlist(struct comedi_device
*dev,
unsigned int seglen;
int i;
- if (cmd->chanlist_len == 1)
- return 1; /* seglen=1 */
+ if (cmd->chanlist_len == 1) {
+ devpriv->act_chanlist_len = cmd->chanlist_len;
+ return 0;
+ }
/* first channel is always ok */
chansegment[0] = cmd->chanlist[0];
@@ -353,7 +356,7 @@ static int pci171x_ai_check_chanlist(struct comedi_device
*dev,
if (aref == AREF_DIFF && (chan & 1)) {
dev_err(dev->class_dev,
"Odd channel cannot be differential input!\n");
- return 0;
+ return -EINVAL;
}
if (last_aref == AREF_DIFF)
@@ -362,7 +365,7 @@ static int pci171x_ai_check_chanlist(struct comedi_device
*dev,
dev_err(dev->class_dev,
"channel list must be continuous!
chanlist[%i]=%d but must be %d or %d!\n",
i, chan, next_chan, chan0);
- return 0;
+ return -EINVAL;
}
/* next correct channel in list */
@@ -381,10 +384,12 @@ static int pci171x_ai_check_chanlist(struct comedi_device
*dev,
CR_CHAN(cmd->chanlist[i % seglen]),
CR_RANGE(cmd->chanlist[i % seglen]),
CR_AREF(chansegment[i % seglen]));
- return 0;
+ return -EINVAL;
}
}
- return seglen;
+ devpriv->act_chanlist_len = seglen;
+
+ return 0;
}
static void setup_channel_list(struct comedi_device *dev,
@@ -396,7 +401,6 @@ static void setup_channel_list(struct comedi_device *dev,
struct pci1710_private *devpriv = dev->private;
unsigned int i, range, chanprog;
- devpriv->act_chanlist_len = seglen;
devpriv->act_chanlist_pos = 0;
for (i = 0; i < seglen; i++) { /* store range list to card */
@@ -953,7 +957,6 @@ static int pci171x_ai_cmd(struct comedi_device *dev, struct
comedi_subdevice *s)
struct pci1710_private *devpriv = dev->private;
struct comedi_cmd *cmd = &s->async->cmd;
unsigned int divisor1 = 0, divisor2 = 0;
- unsigned int seglen;
int mode;
if (cmd->convert_src == TRIG_TIMER) {
@@ -967,10 +970,8 @@ static int pci171x_ai_cmd(struct comedi_device *dev,
struct comedi_subdevice *s)
start_pacer(dev, -1, 0, 0); /* stop pacer */
- seglen = pci171x_ai_check_chanlist(dev, s, cmd);
- if (seglen < 1)
- return -EINVAL;
- setup_channel_list(dev, s, cmd->chanlist, cmd->chanlist_len, seglen);
+ setup_channel_list(dev, s, cmd->chanlist, cmd->chanlist_len,
+ devpriv->act_chanlist_len);
outb(0, dev->iobase + PCI171x_CLRFIFO);
outb(0, dev->iobase + PCI171x_CLRINT);
@@ -1104,7 +1105,9 @@ static int pci171x_ai_cmdtest(struct comedi_device *dev,
/* Step 5: check channel list */
- if (!pci171x_ai_check_chanlist(dev, s, cmd))
+ err |= pci171x_ai_check_chanlist(dev, s, cmd);
+
+ if (err)
return 5;
return 0;
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 17/22] staging: comedi: adv_pci1710: only calc the pacer divisors once
When the cmd->convert_src == TRIG_TIMER the divisors needed to
generate the pacer time are calculated in the (*do_cmdtest) to
validate the cmd->convert_arg. The core always does the (*do_cmdtest)
before the (*do_cmd) so there is no reason to recalc the divisors.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index c1aa16a..99e3a30 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -305,7 +305,8 @@ struct pci1710_private {
unsigned char ai_et;
unsigned int ai_et_CntrlReg;
unsigned int ai_et_MuxVal;
- unsigned int ai_et_div1, ai_et_div2;
+ unsigned int divisor1;
+ unsigned int divisor2;
unsigned int act_chanlist[32]; /* list of scanned channel */
unsigned char act_chanlist_len; /* len of scanlist */
unsigned char act_chanlist_pos; /* actual position in MUX list */
@@ -942,7 +943,7 @@ static irqreturn_t interrupt_service_pci1710(int irq, void
*d)
outw(devpriv->ai_et_MuxVal, dev->iobase + PCI171x_MUX);
outw(devpriv->CntrlReg, dev->iobase + PCI171x_CONTROL);
/* start pacer */
- start_pacer(dev, 1, devpriv->ai_et_div1, devpriv->ai_et_div2);
+ start_pacer(dev, 1, devpriv->divisor1, devpriv->divisor2);
return IRQ_HANDLED;
}
@@ -958,7 +959,6 @@ static int pci171x_ai_cmd(struct comedi_device *dev, struct
comedi_subdevice *s)
{
struct pci1710_private *devpriv = dev->private;
struct comedi_cmd *cmd = &s->async->cmd;
- unsigned int divisor1 = 0, divisor2 = 0;
int mode;
if (cmd->convert_src == TRIG_TIMER) {
@@ -998,17 +998,11 @@ static int pci171x_ai_cmd(struct comedi_device *dev,
struct comedi_subdevice *s)
} else {
devpriv->ai_et = 0;
}
- i8253_cascade_ns_to_timer(devpriv->i8254_osc_base,
- &divisor1, &divisor2,
- &cmd->convert_arg,
- cmd->flags);
outw(devpriv->CntrlReg, dev->iobase + PCI171x_CONTROL);
if (mode != 2) {
/* start pacer */
- start_pacer(dev, mode, divisor1, divisor2);
- } else {
- devpriv->ai_et_div1 = divisor1;
- devpriv->ai_et_div2 = divisor2;
+ start_pacer(dev, mode,
+ devpriv->divisor1, devpriv->divisor2);
}
break;
case 3:
@@ -1031,7 +1025,6 @@ static int pci171x_ai_cmdtest(struct comedi_device *dev,
struct pci1710_private *devpriv = dev->private;
int err = 0;
int tmp;
- unsigned int divisor1 = 0, divisor2 = 0;
/* Step 1 : check if triggers are trivially valid */
@@ -1081,7 +1074,8 @@ static int pci171x_ai_cmdtest(struct comedi_device *dev,
if (cmd->convert_src == TRIG_TIMER) {
tmp = cmd->convert_arg;
i8253_cascade_ns_to_timer(devpriv->i8254_osc_base,
- &divisor1, &divisor2,
+ &devpriv->divisor1,
+ &devpriv->divisor2,
&cmd->convert_arg, cmd->flags);
if (cmd->convert_arg < this_board->ai_ns_min)
cmd->convert_arg = this_board->ai_ns_min;
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: add Lustre file system client support
Hi Dan,
Thanks for reporting this.
On Wed, Apr 23, 2014 at 10:13 PM, Dan Carpenter
wrote:
> Btw, what's the trick to navigating the lustre source? I normally do
> a make cscope but that doesn't work and I am having a very hard time
> with this code.
>
I use cscope + ctags to navigate the lustre source. I guess you hit
some dead corners mainly on macros like
vim drivers/staging/lustre/lustre/include/obd_class.h +323
323 #define OBT(dev)(dev)->obd_type
324 #define OBP(dev, op)(dev)->obd_type->typ_dt_ops->o_ ## op
325 #define MDP(dev, op)(dev)->obd_type->typ_md_ops->m_ ## op
326 #define CTXTP(ctxt, op) (ctxt)->loc_logops->lop_##op
These macros did hit me as well when I first started reading the
lustre code. I guess we should get rid of them somehow in the end, is
it correct Andreas and Oleg?
> regards,
> dan carpenter
>
> On Wed, Apr 23, 2014 at 04:54:26PM +0300, Dan Carpenter wrote:
>> Hello Peng Tao,
>>
>> The patch d7e09d0397e8: "staging: add Lustre file system client
>> support" from May 2, 2013, leads to the following static checker
>> warning:
>>
>> drivers/staging/lustre/include//linux/libcfs/libcfs_ioctl.h:200
>> libcfs_ioctl_is_invalid()
>> error: buffer overflow 'data->ioc_bulk' 896 <= 1073741823
>>
>> drivers/staging/lustre/include//linux/libcfs/libcfs_ioctl.h
>>157 static inline int libcfs_ioctl_is_invalid(struct libcfs_ioctl_data
>> *data)
>>158 {
>>159 if (data->ioc_len > (1<<30)) {
>>160 CERROR ("LIBCFS ioctl: ioc_len larger than 1<<30\n");
>>161 return 1;
>>162 }
>>163 if (data->ioc_inllen1 > (1<<30)) {
>>164 CERROR ("LIBCFS ioctl: ioc_inllen1 larger than
>> 1<<30\n");
>>
>> We limit data->ioc_inllen1 to 1073741823 bytes here.
>>
>>165 return 1;
>>166 }
>>167 if (data->ioc_inllen2 > (1<<30)) {
>>168 CERROR ("LIBCFS ioctl: ioc_inllen2 larger than
>> 1<<30\n");
>>169 return 1;
>>170 }
>>171 if (data->ioc_inlbuf1 && !data->ioc_inllen1) {
>>172 CERROR ("LIBCFS ioctl: inlbuf1 pointer but 0
>> length\n");
>>173 return 1;
>>174 }
>>175 if (data->ioc_inlbuf2 && !data->ioc_inllen2) {
>>176 CERROR ("LIBCFS ioctl: inlbuf2 pointer but 0
>> length\n");
>>177 return 1;
>>178 }
>>179 if (data->ioc_pbuf1 && !data->ioc_plen1) {
>>180 CERROR ("LIBCFS ioctl: pbuf1 pointer but 0
>> length\n");
>>181 return 1;
>>182 }
>>183 if (data->ioc_pbuf2 && !data->ioc_plen2) {
>>184 CERROR ("LIBCFS ioctl: pbuf2 pointer but 0
>> length\n");
>>185 return 1;
>>186 }
>>187 if (data->ioc_plen1 && !data->ioc_pbuf1) {
>>188 CERROR ("LIBCFS ioctl: plen1 nonzero but no pbuf1
>> pointer\n");
>>189 return 1;
>>190 }
>>191 if (data->ioc_plen2 && !data->ioc_pbuf2) {
>>192 CERROR ("LIBCFS ioctl: plen2 nonzero but no pbuf2
>> pointer\n");
>>193 return 1;
>>194 }
>>195 if ((__u32)libcfs_ioctl_packlen(data) != data->ioc_len ) {
>>196 CERROR ("LIBCFS ioctl: packlen != ioc_len\n");
>>
>> The idea was this would check it but this broken because we check
>> data->ioc_len in obd_ioctl_getdata() and then we do a second copy from
>> the user so the current value of ioc_len is unchecked.
So it is more of a security problem because you are considering users
modifying its buffer during an ioctl call, is it correct?
>>
>>197 return 1;
>>198 }
>>199 if (data->ioc_inllen1 &&
>>200 data->ioc_bulk[data->ioc_inllen1 - 1] != '\0') {
>> ^
>>
>> But data is 1024 byte struct with 896 bytes dedicated for ->ioc_bulk[]
Why?
We currently have
#define CONFIG_LUSTRE_OBD_MAX_IOCTL_BUFFER 8192
and clearly it is configurable.
>> so we are reading far beyond on the end of the array here. (Can cause
>> an oops).
>>
>> The caller is obd_ioctl_getdata() and data->ioc_inllen1 comes directly
>> from the user. If we added this in obd_ioctl_getdata() then it would
>> fix the bug:
>>
>> orig_len = hdr->ioc_len;
>> if (copy_from_user(buf, (void *)arg, hdr->ioc_len))
>> return -EFAULT; /* the original return code is buggy */
YES! We certainly cannot return copy_from_user()'s return value.
>> if (orig_len != hdr->ioc_len)
>> return -EINVAL;
>>
And no... the above code snip does not work as expected...
hdr->ioc_len does not get modified in the copy_from_user() and then it
is just comparing with its own copy. You should comp
Re: staging: add Lustre file system client support
Hello! On Apr 23, 2014, at 11:14 PM, Peng Tao wrote: >> Btw, what's the trick to navigating the lustre source? I normally do >> a make cscope but that doesn't work and I am having a very hard time >> with this code. >> > I use cscope + ctags to navigate the lustre source. I guess you hit > some dead corners mainly on macros like > vim drivers/staging/lustre/lustre/include/obd_class.h +323 > 323 #define OBT(dev)(dev)->obd_type > 324 #define OBP(dev, op)(dev)->obd_type->typ_dt_ops->o_ ## op > 325 #define MDP(dev, op)(dev)->obd_type->typ_md_ops->m_ ## op > 326 #define CTXTP(ctxt, op) (ctxt)->loc_logops->lop_##op > > These macros did hit me as well when I first started reading the > lustre code. I guess we should get rid of them somehow in the end, is > it correct Andreas and Oleg? I don't really use either cscope or ctags. I'm ok if we can git rid of them in some way that preserves the intended functionality, I guess, all without actually spelling out every method possible by hand. Bye, Oleg ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
