[PATCH 0/10] use safer test on the result of find_first_zero_bit
Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may return a larger number than the maximum position argument if that position is not a multiple of BITS_PER_LONG. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/10] staging: tidspbridge: use safer test on the result of find_first_zero_bit
From: Julia Lawall
Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
return a larger number than the maximum position argument if that position
is not a multiple of BITS_PER_LONG.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
//
@@
expression e1,e2,e3;
statement S1,S2;
@@
e1 = find_first_zero_bit(e2,e3)
...
if (e1
- ==
+ >=
e3)
S1 else S2
//
Signed-off-by: Julia Lawall
---
drivers/staging/tidspbridge/rmgr/node.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff -u -p a/drivers/staging/tidspbridge/rmgr/node.c
b/drivers/staging/tidspbridge/rmgr/node.c
--- a/drivers/staging/tidspbridge/rmgr/node.c
+++ b/drivers/staging/tidspbridge/rmgr/node.c
@@ -935,7 +935,7 @@ int node_connect(struct node_object *nod
node2_type == NODE_DAISSOCKET)) {
/* Find available pipe */
pipe_id = find_first_zero_bit(hnode_mgr->pipe_map, MAXPIPES);
- if (pipe_id == MAXPIPES) {
+ if (pipe_id >= MAXPIPES) {
status = -ECONNREFUSED;
goto out_unlock;
}
@@ -1008,7 +1008,7 @@ int node_connect(struct node_object *nod
status = -EINVAL;
goto out_unlock;
}
- if (chnl_id == CHNL_MAXCHANNELS) {
+ if (chnl_id >= CHNL_MAXCHANNELS) {
status = -ECONNREFUSED;
goto out_unlock;
}
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/10] use safer test on the result of find_first_zero_bit
Hi Julia,
On Wed, Jun 4, 2014 at 11:07 AM, Julia Lawall wrote:
> Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
> return a larger number than the maximum position argument if that position
> is not a multiple of BITS_PER_LONG.
Shouldn't this be fixed in find_first_zero_bit() instead?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/10] use safer test on the result of find_first_zero_bit
On Wed, 4 Jun 2014, Geert Uytterhoeven wrote: > Hi Julia, > > On Wed, Jun 4, 2014 at 11:07 AM, Julia Lawall wrote: > > Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may > > return a larger number than the maximum position argument if that position > > is not a multiple of BITS_PER_LONG. > > Shouldn't this be fixed in find_first_zero_bit() instead? OK, I could do that as well. Most of the callers currently test with >=. Should they be left as is, or changed to use ==? julia ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 0/10] use safer test on the result of find_first_zero_bit
From: Julia Lawall > On Wed, 4 Jun 2014, Geert Uytterhoeven wrote: > > > Hi Julia, > > > > On Wed, Jun 4, 2014 at 11:07 AM, Julia Lawall wrote: > > > Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may > > > return a larger number than the maximum position argument if that position > > > is not a multiple of BITS_PER_LONG. > > > > Shouldn't this be fixed in find_first_zero_bit() instead? > > OK, I could do that as well. Most of the callers currently test with >=. > Should they be left as is, or changed to use ==? Do we want to add an extra test to find_first_zero_bit() and effectively slow down all the calls - especially those where the length is a multiple of 8 (probably the most common). Maybe the documented return code should be changed to allow for the existing behaviour. David ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PULL] Move IPUv3 core out of staging, add CSI support
Hi Dave,
The following changes since commit d1db0eea852497762cab43b905b879dfcd3b8987:
Linux 3.15-rc3 (2014-04-27 19:29:27 -0700)
are available in the git repository at:
git://git.pengutronix.de/git/pza/linux.git topic/ipu-destaging
for you to fetch changes up to d6ca8ca7ec555bdd3372687d0d775c837a09ff6e:
gpu: ipu-v3: Register the CSI modules (2014-06-04 11:07:12 +0200)
These move the core i.MX IPU code out of staging into drivers/gpu and add
the foundation for CSI capture support. In the following round we can then
fix remaining issues with imx-drm in staging and start adding V4L2 support
in parallel.
I have rebased the series back to v3.15-rc3, the tag closest to the common
ancestor of drm-next and staging-next, and verified that merging the two
produces the correct result.
regards
Philipp
Philipp Zabel (5):
gpu: ipu-v3: Move i.MX IPUv3 core driver out of staging
gpu: ipu-v3: Add SMFC code
gpu: ipu-v3: Add ipu_idmac_get_current_buffer function
gpu: ipu-v3: Add CSI and SMFC module enable wrappers
gpu: ipu-v3: Register the CSI modules
drivers/gpu/Makefile | 1 +
drivers/gpu/ipu-v3/Kconfig | 7 ++
drivers/{staging/imx-drm => gpu}/ipu-v3/Makefile | 4 +-
.../{staging/imx-drm => gpu}/ipu-v3/ipu-common.c | 82 --
drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-dc.c | 3 +-
drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-di.c | 2 +-
drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-dmfc.c | 2 +-
drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-dp.c | 2 +-
drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-prv.h | 8 +-
drivers/gpu/ipu-v3/ipu-smfc.c | 97 ++
drivers/staging/imx-drm/Kconfig| 11 +--
drivers/staging/imx-drm/Makefile | 1 -
drivers/staging/imx-drm/imx-hdmi.c | 2 +-
drivers/staging/imx-drm/imx-tve.c | 2 +-
drivers/staging/imx-drm/ipuv3-crtc.c | 2 +-
drivers/staging/imx-drm/ipuv3-plane.c | 2 +-
drivers/video/Kconfig | 1 +
.../imx-drm/ipu-v3 => include/video}/imx-ipu-v3.h | 16
18 files changed, 216 insertions(+), 29 deletions(-)
create mode 100644 drivers/gpu/ipu-v3/Kconfig
rename drivers/{staging/imx-drm => gpu}/ipu-v3/Makefile (51%)
rename drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-common.c (94%)
rename drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-dc.c (99%)
rename drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-di.c (99%)
rename drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-dmfc.c (99%)
rename drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-dp.c (99%)
rename drivers/{staging/imx-drm => gpu}/ipu-v3/ipu-prv.h (96%)
create mode 100644 drivers/gpu/ipu-v3/ipu-smfc.c
rename {drivers/staging/imx-drm/ipu-v3 => include/video}/imx-ipu-v3.h (95%)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 0/10] use safer test on the result of find_first_zero_bit
On Wed, 4 Jun 2014, David Laight wrote: > From: Julia Lawall > > On Wed, 4 Jun 2014, Geert Uytterhoeven wrote: > > > > > Hi Julia, > > > > > > On Wed, Jun 4, 2014 at 11:07 AM, Julia Lawall > > > wrote: > > > > Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may > > > > return a larger number than the maximum position argument if that > > > > position > > > > is not a multiple of BITS_PER_LONG. > > > > > > Shouldn't this be fixed in find_first_zero_bit() instead? > > > > OK, I could do that as well. Most of the callers currently test with >=. > > Should they be left as is, or changed to use ==? > > Do we want to add an extra test to find_first_zero_bit() and effectively > slow down all the calls - especially those where the length is a > multiple of 8 (probably the most common). Currently, most of the calls test with >=, and most of the others seem to need to (either the size value did not look like a multiple of anything in particular, or it was eg read from a device). Note that it is BITS_PER_LONG, so it seems like it is typically 32 or 64, not 8. > Maybe the documented return code should be changed to allow for the > existing behaviour. Sorry, I'm not sure to understand what you suggest here. thanks, julia ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/10] use safer test on the result of find_first_zero_bit
Hi Julia,
On Wed, Jun 4, 2014 at 11:52 AM, Julia Lawall wrote:
>> Maybe the documented return code should be changed to allow for the
>> existing behaviour.
>
> Sorry, I'm not sure to understand what you suggest here.
include/asm-generic/bitops/find.h:
| /**
| * find_first_zero_bit - find the first cleared bit in a memory region
| * @addr: The address to start the search at
| * @size: The maximum number of bits to search
| *
| * Returns the bit number of the first cleared bit.
| * If no bits are zero, returns @size.
"If no bits are zero, returns @size or a number larger than @size."
| */
| extern unsigned long find_first_zero_bit(const unsigned long *addr,
| unsigned long size);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/10] use safer test on the result of find_first_zero_bit
On Wed, 4 Jun 2014, Geert Uytterhoeven wrote:
> Hi Julia,
>
> On Wed, Jun 4, 2014 at 11:52 AM, Julia Lawall wrote:
> >> Maybe the documented return code should be changed to allow for the
> >> existing behaviour.
> >
> > Sorry, I'm not sure to understand what you suggest here.
>
> include/asm-generic/bitops/find.h:
>
> | /**
> | * find_first_zero_bit - find the first cleared bit in a memory region
> | * @addr: The address to start the search at
> | * @size: The maximum number of bits to search
> | *
> | * Returns the bit number of the first cleared bit.
> | * If no bits are zero, returns @size.
>
> "If no bits are zero, returns @size or a number larger than @size."
OK, thanks. I was only looking at the C code.
But the C code contains a loop that is followed by:
if (!size)
return result;
tmp = *p;
found_first:
tmp |= ~0UL << size;
if (tmp == ~0UL)/* Are any bits zero? */
return result + size; /* Nope. */
In the first return, it would seem that result == size. Could the second
one be changed to just return size? It should not hurt performance.
julia
>
> | */
> | extern unsigned long find_first_zero_bit(const unsigned long *addr,
> | unsigned long size);
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/10] use safer test on the result of find_first_zero_bit
Hi Julia,
On Wed, Jun 4, 2014 at 1:00 PM, Julia Lawall wrote:
> OK, thanks. I was only looking at the C code.
>
> But the C code contains a loop that is followed by:
>
> if (!size)
> return result;
> tmp = *p;
>
> found_first:
> tmp |= ~0UL << size;
> if (tmp == ~0UL)/* Are any bits zero? */
> return result + size; /* Nope. */
>
> In the first return, it would seem that result == size. Could the second
> one be changed to just return size? It should not hurt performance.
"size" may have been changed between function entry and this line.
So you have to store it in a temporary.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/10] use safer test on the result of find_first_zero_bit
On Wed, 4 Jun 2014, Geert Uytterhoeven wrote: > Hi Julia, > > On Wed, Jun 4, 2014 at 1:00 PM, Julia Lawall wrote: > > OK, thanks. I was only looking at the C code. > > > > But the C code contains a loop that is followed by: > > > > if (!size) > > return result; > > tmp = *p; > > > > found_first: > > tmp |= ~0UL << size; > > if (tmp == ~0UL)/* Are any bits zero? */ > > return result + size; /* Nope. */ > > > > In the first return, it would seem that result == size. Could the second > > one be changed to just return size? It should not hurt performance. > > "size" may have been changed between function entry and this line. > So you have to store it in a temporary. Sorry, after reflection it seems that indeed size + result is always the original size, so it is actually all of the code that uses >= that is doing something unnecessary. == for the failure test is fine. julia ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 0/10] use safer test on the result of find_first_zero_bit
From: Julia Lawall > On Wed, 4 Jun 2014, Geert Uytterhoeven wrote: > > > Hi Julia, > > > > On Wed, Jun 4, 2014 at 1:00 PM, Julia Lawall wrote: > > > OK, thanks. I was only looking at the C code. > > > > > > But the C code contains a loop that is followed by: > > > > > > if (!size) > > > return result; > > > tmp = *p; > > > > > > found_first: > > > tmp |= ~0UL << size; > > > if (tmp == ~0UL)/* Are any bits zero? */ > > > return result + size; /* Nope. */ > > > > > > In the first return, it would seem that result == size. Could the second > > > one be changed to just return size? It should not hurt performance. > > > > "size" may have been changed between function entry and this line. > > So you have to store it in a temporary. > > Sorry, after reflection it seems that indeed size + result is always the > original size, so it is actually all of the code that uses >= that is > doing something unnecessary. == for the failure test is fine. There is nothing wrong with defensive coding. The 'tmp |= ~0UL << size' ensures that the return value is 'correct' when there are no bits set. The function could have been defined so that this wasn't needed. If you assume that the 'no zero bits' is unlikely, then checking the return value from ffz() could well be slightly faster. Not that anything is likely to notice. David ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/4] dma-mapping: Add devm_ interface for dma_map_single()
Hello, On Wed, Jun 04, 2014 at 01:39:07AM +0200, Joerg Roedel wrote: > I fully agree with the points Shuah brought up here. I don't think it is > a good idea to add this kind of resource management to runtime-allocated > (and de-allocated) resources of device drivers. > > Also DMA handles are not something that could be garbage collected at > driver unload time. They are a limited resource that may be used up at > some point. And the whole point of a devm-API is that code can be > simpler because we don't need to de-allocate everything on the > error-path or at unload time, no? Hmmm? Don't we have drivers which map dma buffers on device init and release them on exit? For dynamic usages, its usefulness is limited especially given that dynamic tracking of buffers usually would involve tracking of other information too in addition to dma buffer pointer themselves. If alloc on init and free on exit is a very rare usage pattern, I have no objection against not adding devm interface for dma mappings. Thanks. -- tejun ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/4] dma-mapping: Add devm_ interface for dma_map_single()
On Wed, Jun 04, 2014 at 10:04:08AM -0400, Tejun Heo wrote: > Hmmm? Don't we have drivers which map dma buffers on device init and > release them on exit? For dynamic usages, its usefulness is limited > especially given that dynamic tracking of buffers usually would > involve tracking of other information too in addition to dma buffer > pointer themselves. If alloc on init and free on exit is a very rare > usage pattern, I have no objection against not adding devm interface > for dma mappings. Yes, but those drivers usually get DMA buffers at init time with the dma_alloc_* interfaces. The dma_map_* interfaces discussed here belong to the streaming DMA-API, so they are usually used for only one DMA transaction before dma_unmap_* is called on them. A devm interface for the dma_alloc_* family of functions would actually make sense, but not for the dma_map_* functions. Joerg ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/4] dma-mapping: Add devm_ interface for dma_map_single()
Hello, On Wed, Jun 04, 2014 at 04:12:11PM +0200, Joerg Roedel wrote: > Yes, but those drivers usually get DMA buffers at init time with the > dma_alloc_* interfaces. The dma_map_* interfaces discussed here belong > to the streaming DMA-API, so they are usually used for only one DMA > transaction before dma_unmap_* is called on them. > > A devm interface for the dma_alloc_* family of functions would > actually make sense, but not for the dma_map_* functions. Ah, okay. Fair enough. Thanks. -- tejun ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging:tidspbridge Fix minor checkpatch.pl warning.
From: Adithya K
Fixed few checkpatch.pl warnings.
Signed-off-by: Adithya K
---
drivers/staging/tidspbridge/pmgr/chnl.c |1 +
drivers/staging/tidspbridge/pmgr/dspapi.c |8
drivers/staging/tidspbridge/rmgr/dbdcd.c |1 +
drivers/staging/tidspbridge/rmgr/nldr.c |2 +-
4 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/tidspbridge/pmgr/chnl.c
b/drivers/staging/tidspbridge/pmgr/chnl.c
index 4bd8686..e03c326 100644
--- a/drivers/staging/tidspbridge/pmgr/chnl.c
+++ b/drivers/staging/tidspbridge/pmgr/chnl.c
@@ -75,6 +75,7 @@ int chnl_create(struct chnl_mgr **channel_mgr,
if (!status) {
struct bridge_drv_interface *intf_fxns;
+
dev_get_intf_fxns(hdev_obj, &intf_fxns);
/* Let Bridge channel module finish the create: */
status = (*intf_fxns->chnl_create) (&hchnl_mgr, hdev_obj,
diff --git a/drivers/staging/tidspbridge/pmgr/dspapi.c
b/drivers/staging/tidspbridge/pmgr/dspapi.c
index b7d5c8c..8fb6ed7 100644
--- a/drivers/staging/tidspbridge/pmgr/dspapi.c
+++ b/drivers/staging/tidspbridge/pmgr/dspapi.c
@@ -64,7 +64,7 @@
/* Device IOCtl function pointer */
struct api_cmd {
- u32(*fxn) (union trapped_args *args, void *pr_ctxt);
+ u32 (*fxn)(union trapped_args *args, void *pr_ctxt);
u32 index;
};
@@ -206,7 +206,7 @@ static inline void _cp_to_usr(void __user *to, const void
*from,
inline int api_call_dev_ioctl(u32 cmd, union trapped_args *args,
u32 *result, void *pr_ctxt)
{
- u32(*ioctl_cmd) (union trapped_args *args, void *pr_ctxt) = NULL;
+ u32 (*ioctl_cmd)(union trapped_args *args, void *pr_ctxt) = NULL;
int i;
if (_IOC_TYPE(cmd) != DB) {
@@ -766,7 +766,7 @@ u32 procwrap_load(union trapped_args *args, void *pr_ctxt)
goto func_cont;
}
- argv = kmalloc(count * sizeof(u8 *), GFP_KERNEL);
+ argv = kmalloc_array(count, sizeof(u8 *), GFP_KERNEL);
if (!argv) {
status = -ENOMEM;
goto func_cont;
@@ -812,7 +812,7 @@ u32 procwrap_load(union trapped_args *args, void *pr_ctxt)
}
count++;
} while (temp);
- envp = kmalloc(count * sizeof(u8 *), GFP_KERNEL);
+ envp = kmalloc_array(count, sizeof(u8 *), GFP_KERNEL);
if (!envp) {
status = -ENOMEM;
goto func_cont;
diff --git a/drivers/staging/tidspbridge/rmgr/dbdcd.c
b/drivers/staging/tidspbridge/rmgr/dbdcd.c
index 2ae48c9..c91d1d7 100644
--- a/drivers/staging/tidspbridge/rmgr/dbdcd.c
+++ b/drivers/staging/tidspbridge/rmgr/dbdcd.c
@@ -489,6 +489,7 @@ int dcd_get_object_def(struct dcd_manager *hdcd_mgr,
strncpy(sz_sect_name, ".", 2);
do {
char *uuid = strsep(&tmp, "-");
+
if (!uuid)
break;
len -= strlen(uuid);
diff --git a/drivers/staging/tidspbridge/rmgr/nldr.c
b/drivers/staging/tidspbridge/rmgr/nldr.c
index 5ac507c..bddf6e5 100644
--- a/drivers/staging/tidspbridge/rmgr/nldr.c
+++ b/drivers/staging/tidspbridge/rmgr/nldr.c
@@ -239,7 +239,7 @@ struct nldr_nodeobject {
* Mask indicating whether each mem segment specified in seg_id[]
* is preferred or required.
* For example
-* if (code_data_flag_mask & (1 << EXECUTEDATAFLAGBIT)) != 0,
+* if (code_data_flag_mask & (1 << EXECUTEDATAFLAGBIT)) != 0,
* then it is required to load execute phase data into the memory
* specified by seg_id[EXECUTEDATAFLAGBIT].
*/
--
1.7.9.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/4] dma-mapping: Add devm_ interface for dma_map_single()
Hi, I believe that I need a managed dma_map_single() my own driver, which doesn't fall in the case of a single use: The driver allocates its buffers with __get_free_pages() (or the to-be managed version of it). Then it cuts the allocated memory into smaller buffers (in some cases, and with certain alignment rules), and then calls dma_map_single() to do the DMA mapping for each. The buffers are held along the driver's lifetime, with DMA sync API juggling control back and forth to the hardware. Note that the DMA is noncoherent. Once could argue, that since dma_map_noncoherent() calls __get_free_pages() under the hood, I could request the larger chunk from dma_map_noncoherent(), and cut it into smaller DMA buffers. My concern is that the dma_sync_single_*() functions expect a DMA handle, not a physical address I've made up with my own buffer splitting. I don't see any problem with this trick on platforms I've worked with, but I'm not sure if this is the proper way to go. dma_map_single(), on the other hand, returns a DMA handle. The DMA pool functions could be interesting, but I understand that they would supply me with coherent memory only. Anything I missed? Thanks, Eli On 04/06/14 17:14, Tejun Heo wrote: Hello, On Wed, Jun 04, 2014 at 04:12:11PM +0200, Joerg Roedel wrote: Yes, but those drivers usually get DMA buffers at init time with the dma_alloc_* interfaces. The dma_map_* interfaces discussed here belong to the streaming DMA-API, so they are usually used for only one DMA transaction before dma_unmap_* is called on them. A devm interface for the dma_alloc_* family of functions would actually make sense, but not for the dma_map_* functions. Ah, okay. Fair enough. Thanks. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging:tidspbridge Fix minor checkpatch.pl warning.
On Wed, Jun 04, 2014 at 08:28:36PM +0530, Adithya.K wrote: > From: Adithya K > > Fixed few checkpatch.pl warnings. Be specific about _what_ warnings you fixed. And if you fix more than one type of warning, please break it up into different patches (each patch only can do one thing.) > Signed-off-by: Adithya K I need a "full" name for a kernel patch. I have to ask, is your last name really just one letter? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] [SCSI] Fix a bug in deriving the FLUSH_TIMEOUT from the basic I/O timeout
Commit ID: 7e660100d85af860e7ad763202fff717adcdaacd added code to derive the
FLUSH_TIMEOUT from the basic I/O timeout. However, this patch did not use the
basic I/O timeout of the device. Fix this bug.
Signed-off-by: K. Y. Srinivasan
---
drivers/scsi/sd.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index e9689d5..54150b1 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -832,7 +832,9 @@ static int sd_setup_write_same_cmnd(struct scsi_device
*sdp, struct request *rq)
static int scsi_setup_flush_cmnd(struct scsi_device *sdp, struct request *rq)
{
- rq->timeout *= SD_FLUSH_TIMEOUT_MULTIPLIER;
+ int timeout = sdp->request_queue->rq_timeout;
+
+ rq->timeout = (timeout * SD_FLUSH_TIMEOUT_MULTIPLIER);
rq->retries = SD_MAX_RETRIES;
rq->cmd[0] = SYNCHRONIZE_CACHE;
rq->cmd_len = 10;
--
1.7.4.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/6] staging: comedi: addi_common.h: remove some cruft
On 2014-05-29 18:29, H Hartley Sweeten wrote:
Many of the ADDI-DATA drivers have been separated from the "addi_common"
code and cleaned up. Because of this cleanup there are some unnecessary
members hanging around in the private data struct and a number of defines
that don't add any sigificant clarity to the remaining drivers in the
addi-data directory.
Remove this cruft.
H Hartley Sweeten (6):
staging: comedi: addi_common.h: remove 'b_SingleDiff' from private data
staging: comedi: addi_common.h: remove APCI1710_SAVE_INTERRUPT define
staging: comedi: addi_common.h: remove ADDI_{EN,DIS}ABLE defines
staging: comedi: addi_common.h: remove ADDIDATA_{EN,DIS}ABLE defines
staging: comedi: addi_common.h: remove {LO,HI}WORD macros
staging: comedi: addi_common.h: remove ADDIDATA_* defines
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 1/1] [SCSI] Fix a bug in deriving the FLUSH_TIMEOUT from the basic I/O timeout
On Wed, 2014-06-04 at 09:33 -0700, K. Y. Srinivasan wrote:
> Commit ID: 7e660100d85af860e7ad763202fff717adcdaacd added code to derive the
> FLUSH_TIMEOUT from the basic I/O timeout. However, this patch did not use the
> basic I/O timeout of the device. Fix this bug.
>
> Signed-off-by: K. Y. Srinivasan
> ---
> drivers/scsi/sd.c |4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index e9689d5..54150b1 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -832,7 +832,9 @@ static int sd_setup_write_same_cmnd(struct scsi_device
> *sdp, struct request *rq)
>
> static int scsi_setup_flush_cmnd(struct scsi_device *sdp, struct request *rq)
> {
> - rq->timeout *= SD_FLUSH_TIMEOUT_MULTIPLIER;
> + int timeout = sdp->request_queue->rq_timeout;
> +
> + rq->timeout = (timeout * SD_FLUSH_TIMEOUT_MULTIPLIER);
Could you share where you found this to be a problem? It looks like a
bug in block because all inbound requests being prepared should have a
timeout set, so block would be the place to fix it.
I can't see how this can happen because we definitely add the timer
after the request is prepared in my reading of the block code, unless
I'm missing some path in block that violates this.
James
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 1/1] [SCSI] Fix a bug in deriving the FLUSH_TIMEOUT from the basic I/O timeout
> -Original Message-
> From: James Bottomley [mailto:jbottom...@parallels.com]
> Sent: Wednesday, June 4, 2014 10:02 AM
> To: KY Srinivasan
> Cc: linux-ker...@vger.kernel.org; a...@canonical.com;
> de...@linuxdriverproject.org; h...@infradead.org; linux-
> s...@vger.kernel.org; oher...@suse.com; gre...@linuxfoundation.org;
> jasow...@redhat.com
> Subject: Re: [PATCH 1/1] [SCSI] Fix a bug in deriving the FLUSH_TIMEOUT
> from the basic I/O timeout
>
> On Wed, 2014-06-04 at 09:33 -0700, K. Y. Srinivasan wrote:
> > Commit ID: 7e660100d85af860e7ad763202fff717adcdaacd added code to
> > derive the FLUSH_TIMEOUT from the basic I/O timeout. However, this
> > patch did not use the basic I/O timeout of the device. Fix this bug.
> >
> > Signed-off-by: K. Y. Srinivasan
> > ---
> > drivers/scsi/sd.c |4 +++-
> > 1 files changed, 3 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index
> > e9689d5..54150b1 100644
> > --- a/drivers/scsi/sd.c
> > +++ b/drivers/scsi/sd.c
> > @@ -832,7 +832,9 @@ static int sd_setup_write_same_cmnd(struct
> > scsi_device *sdp, struct request *rq)
> >
> > static int scsi_setup_flush_cmnd(struct scsi_device *sdp, struct
> > request *rq) {
> > - rq->timeout *= SD_FLUSH_TIMEOUT_MULTIPLIER;
> > + int timeout = sdp->request_queue->rq_timeout;
> > +
> > + rq->timeout = (timeout * SD_FLUSH_TIMEOUT_MULTIPLIER);
>
> Could you share where you found this to be a problem? It looks like a bug in
> block because all inbound requests being prepared should have a timeout
> set, so block would be the place to fix it.
Perhaps; what I found was that the value in rq->timeout was 0 coming into
this function and thus multiplying obviously has no effect.
>
> I can't see how this can happen because we definitely add the timer after the
> request is prepared in my reading of the block code, unless I'm missing some
> path in block that violates this.
>
> James
K. Y
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] staging: comedi: adl_pci9111: remove PCI9111_DRIVER_NAME define
On 2014-05-29 18:35, H Hartley Sweeten wrote:
This define is only used in a comedi_error() message. The addition of
the driver name to the message is not necessary. Remove the define.
For aesthetics, convert the comedi_error() into a dev_dbg().
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adl_pci9111.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adl_pci9111.c
b/drivers/staging/comedi/drivers/adl_pci9111.c
index 584fd57..d005faa 100644
--- a/drivers/staging/comedi/drivers/adl_pci9111.c
+++ b/drivers/staging/comedi/drivers/adl_pci9111.c
@@ -75,7 +75,6 @@ TODO:
#include "plx9052.h"
#include "comedi_fc.h"
-#define PCI9111_DRIVER_NAME"adl_pci9111"
#define PCI9111_HR_DEVICE_ID 0x9111
#define PCI9111_FIFO_HALF_SIZE512
@@ -630,7 +629,7 @@ static irqreturn_t pci9111_interrupt(int irq, void
*p_device)
/* '0' means FIFO is full, data may have been lost */
if (!(status & PCI9111_AI_STAT_FF_FF)) {
spin_unlock_irqrestore(&dev->spinlock, irq_flags);
- comedi_error(dev, PCI9111_DRIVER_NAME " fifo overflow");
+ dev_dbg(dev->class_dev, "fifo overflow\n");
outb(0, dev->iobase + PCI9111_INT_CLR_REG);
async->events |= COMEDI_CB_ERROR | COMEDI_CB_EOA;
cfc_handle_events(dev, s);
We usually report those at "error" level in the other Comedi drivers.
The "debug" level messages might not get output at all.
--
-=( 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 2/3] staging: comedi: adl_pci9111: remove PCI9111_HR_DEVICE_ID define
On 2014-05-29 18:35, H Hartley Sweeten wrote: This define is only used in the pci_device_id table and doesn't add any additional clarity to the code. Remove the define and just open code the value. Signed-off-by: H Hartley Sweeten Cc: Ian Abbott Cc: Greg Kroah-Hartman 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 3/3] staging: comedi: adl_pci9111: simplify A/D trigger selection code
On 2014-05-29 18:35, H Hartley Sweeten wrote: The functions pci9111_trigger_source_set(), pci9111_pretrigger_set(), and pci9111_autoscan_set() are all used to select the A/D trigger type. They all do a read/mask/set/write of the A/D Trigger Mode Control register. Simplify the code by removing these helper functions and combining all the trigger bits so that a single write can be used to set the register. Signed-off-by: H Hartley Sweeten Cc: Ian Abbott Cc: Greg Kroah-Hartman --- drivers/staging/comedi/drivers/adl_pci9111.c | 94 1 file changed, 13 insertions(+), 81 deletions(-) Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: rtl8821ae: use kmalloc instead of variable length stack arrays
This removes stack arrays of variable length and use kmalloc() instead, thus
removing the sparse warnings "Variable length array is used".
Signed-off-by: Remi Pommarel
---
drivers/staging/rtl8821ae/efuse.c | 38 +++---
1 file changed, 31 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/rtl8821ae/efuse.c
b/drivers/staging/rtl8821ae/efuse.c
index 206012c..a41aa84 100644
--- a/drivers/staging/rtl8821ae/efuse.c
+++ b/drivers/staging/rtl8821ae/efuse.c
@@ -232,7 +232,7 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16
_size_byte, u8 *pbuf)
{
struct rtl_priv *rtlpriv = rtl_priv(hw);
struct rtl_efuse *rtlefuse = rtl_efuse(rtl_priv(hw));
- u8 efuse_tbl[rtlpriv->cfg->maps[EFUSE_HWSET_MAX_SIZE]];
+ u8 *efuse_tbl;
u8 rtemp8[1];
u16 efuse_addr = 0;
u8 offset, wren;
@@ -243,7 +243,7 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16
_size_byte, u8 *pbuf)
rtlpriv->cfg->maps[EFUSE_MAX_SECTION_MAP];
const u32 efuse_real_content_len =
rtlpriv->cfg->maps[EFUSE_REAL_CONTENT_SIZE];
- u16 efuse_word[efuse_max_section][EFUSE_MAX_WORD_UNIT];
+ u16 **efuse_word;
u16 efuse_utilized = 0;
u8 efuse_usage;
@@ -254,9 +254,26 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16
_size_byte, u8 *pbuf)
return;
}
+ efuse_tbl = kmalloc(rtlpriv->cfg->maps[EFUSE_HWSET_MAX_SIZE] *
+ sizeof(*efuse_tbl), GFP_ATOMIC);
+ if (efuse_tbl == NULL)
+ return;
+
+ efuse_word = kcalloc(EFUSE_MAX_WORD_UNIT, sizeof(*efuse_word),
+ GFP_ATOMIC);
+ if (efuse_word == NULL)
+ goto out;
+
+ for (i = 0; i < EFUSE_MAX_WORD_UNIT; i++) {
+ efuse_word[i] = kmalloc(efuse_max_section *
+ sizeof(**efuse_word), GFP_ATOMIC);
+ if (efuse_word[i] == NULL)
+ goto done;
+ }
+
for (i = 0; i < efuse_max_section; i++)
for (j = 0; j < EFUSE_MAX_WORD_UNIT; j++)
- efuse_word[i][j] = 0x;
+ efuse_word[j][i] = 0x;
read_efuse_byte(hw, efuse_addr, rtemp8);
if (*rtemp8 != 0xFF) {
@@ -304,7 +321,7 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16
_size_byte, u8 *pbuf)
read_efuse_byte(hw, efuse_addr, rtemp8);
efuse_addr++;
efuse_utilized++;
- efuse_word[offset][i] = (*rtemp8 &
+ efuse_word[i][offset] = (*rtemp8 &
0xff);
if (efuse_addr >=
@@ -318,7 +335,7 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16
_size_byte, u8 *pbuf)
read_efuse_byte(hw, efuse_addr, rtemp8);
efuse_addr++;
efuse_utilized++;
- efuse_word[offset][i] |=
+ efuse_word[i][offset] |=
(((u16) * rtemp8 << 8) & 0xff00);
if (efuse_addr >=
efuse_real_content_len)
@@ -341,9 +358,9 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16
_size_byte, u8 *pbuf)
for (i = 0; i < efuse_max_section; i++) {
for (j = 0; j < EFUSE_MAX_WORD_UNIT; j++) {
efuse_tbl[(i * 8) + (j * 2)] =
- (efuse_word[i][j] & 0xff);
+ (efuse_word[j][i] & 0xff);
efuse_tbl[(i * 8) + ((j * 2) + 1)] =
- ((efuse_word[i][j] >> 8) & 0xff);
+ ((efuse_word[j][i] >> 8) & 0xff);
}
}
@@ -357,6 +374,13 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16
_size_byte, u8 *pbuf)
(u8 *) & efuse_utilized);
rtlpriv->cfg->ops->set_hw_reg(hw, HW_VAR_EFUSE_USAGE,
(u8 *) & efuse_usage);
+
+done:
+ for (i = 0; i < EFUSE_MAX_WORD_UNIT; i++)
+ kfree(efuse_word[i]);
+ kfree(efuse_word);
+out:
+ kfree(efuse_tbl);
}
bool efuse_shadow_update_chk(struct ieee80211_hw *hw)
--
1.8.5.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: comedi_buf: introduce comedi_buf_n_bytes_ready()
On 2014-05-29 18:38, H Hartley Sweeten wrote:
Introduce a helper function to return the number of bytes that are
ready to read/write from/to the comedi_async buffer. The "write to"
use doesn't really make much sense but is handled for completeness.
Use the helper function in the comedi drivers that currently do the
calculation as part of the (*poll) operation.
Also, use the helper in comedi_fops where the calculation is used as
part of the subdevice going nonbusy.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/comedi_buf.c| 18 ++
drivers/staging/comedi/comedi_fops.c | 5 ++---
drivers/staging/comedi/comedidev.h | 2 ++
drivers/staging/comedi/drivers/das16m1.c | 2 +-
drivers/staging/comedi/drivers/das1800.c | 2 +-
drivers/staging/comedi/drivers/ni_mio_common.c | 2 +-
drivers/staging/comedi/drivers/ni_pcidio.c | 2 +-
drivers/staging/comedi/drivers/pcl812.c| 2 +-
drivers/staging/comedi/drivers/pcl816.c| 2 +-
9 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/comedi/comedi_buf.c
b/drivers/staging/comedi/comedi_buf.c
index df4a9c4..52fc634 100644
--- a/drivers/staging/comedi/comedi_buf.c
+++ b/drivers/staging/comedi/comedi_buf.c
@@ -243,6 +243,24 @@ void comedi_buf_reset(struct comedi_subdevice *s)
async->events = 0;
}
+/**
+ * comedi_buf_n_bytes_ready() - Returns the number of bytes ready to read/write
+ * @s: comedi_subdevice struct
+ */
+unsigned int comedi_buf_n_bytes_ready(struct comedi_subdevice *s)
+{
+ struct comedi_async *async = s->async;
+
+ if (async->buf_write_count == async->buf_read_count)
+ return 0;
+
+ if (async->buf_write_count > async->buf_read_count)
+ return async->buf_write_count - async->buf_read_count;
+ else
+ return async->buf_read_count - async->buf_write_count;
+}
+EXPORT_SYMBOL_GPL(comedi_buf_n_bytes_ready);
+
This will actually break things. The buf_write_count, buf_read_count,
buf_write_alloc_count, buf_read_alloc_count, and munge_count members of
struct comedi_async are all supposed to work modulo 2^32 (UINT_MAX+1).
Logically (with big-ints):
buf_read_count <= buf_read_alloc_count <= munge_count <= buf_write_count
<= buf_write_alloc_count <= (buf_read_count + prealloc_bufsz)
and logically they only increase in value, but in practice are all
reduced modulo 2^32. The upshot is that this function should always return:
return async->buf_write_count - async->buf_read_count;
which could easily be inlined to avoid an external function call.
static unsigned int comedi_buf_write_n_available(struct comedi_subdevice *s)
{
struct comedi_async *async = s->async;
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index 9d99fb3..f5eda5f 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -997,7 +997,7 @@ static int do_bufinfo_ioctl(struct comedi_device *dev,
comedi_buf_read_free(s, bi.bytes_read);
if (comedi_is_subdevice_idle(s) &&
- async->buf_write_count == async->buf_read_count) {
+ comedi_buf_n_bytes_ready(s) == 0) {
do_become_nonbusy(dev, s);
}
}
@@ -2303,8 +2303,7 @@ static ssize_t comedi_read(struct file *file, char __user
*buf, size_t nbytes,
new_s = comedi_read_subdevice(dev, minor);
if (dev->attached && old_detach_count == dev->detach_count &&
s == new_s && new_s->async == async) {
- if (become_nonbusy ||
- async->buf_read_count - async->buf_write_count == 0)
+ if (become_nonbusy || comedi_buf_n_bytes_ready(s) == 0)
do_become_nonbusy(dev, s);
}
mutex_unlock(&dev->mutex);
diff --git a/drivers/staging/comedi/comedidev.h
b/drivers/staging/comedi/comedidev.h
index 8f4e44b..789b0ea 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -333,6 +333,8 @@ static inline unsigned int bytes_per_sample(const struct
comedi_subdevice *subd)
*/
int comedi_set_hw_dev(struct comedi_device *dev, struct device *hw_dev);
+unsigned int comedi_buf_n_bytes_ready(struct comedi_subdevice *);
+
unsigned int comedi_buf_write_alloc(struct comedi_subdevice *s, unsigned int
n);
unsigned int comedi_buf_write_free(struct comedi_subdevice *s, unsigned int
n);
diff --git a/drivers/staging/comedi/drivers/das16m1.c
b/drivers/staging/comedi/drivers/das16m1.c
index ec039fb..c252ad2 100644
--- a/drivers/staging/comedi/drivers/das16m1.c
+++ b/drivers/staging/comedi/drivers/das16m1.c
@@ -477,7 +477,7 @@ static int das16m1_poll(struct comedi_device *dev, struct
comedi_subdevice *s)
RE: [PATCH] staging: comedi: comedi_buf: introduce comedi_buf_n_bytes_ready()
On Wednesday, June 04, 2014 11:17 AM, Ian Abbott wrote:
> On 2014-05-29 18:38, H Hartley Sweeten wrote:
>> Introduce a helper function to return the number of bytes that are
>> ready to read/write from/to the comedi_async buffer. The "write to"
>> use doesn't really make much sense but is handled for completeness.
>>
>> Use the helper function in the comedi drivers that currently do the
>> calculation as part of the (*poll) operation.
>>
>> Also, use the helper in comedi_fops where the calculation is used as
>> part of the subdevice going nonbusy.
>>
>> Signed-off-by: H Hartley Sweeten
>> Cc: Ian Abbott
>> Cc: Greg Kroah-Hartman
>> ---
>> drivers/staging/comedi/comedi_buf.c| 18 ++
>> drivers/staging/comedi/comedi_fops.c | 5 ++---
>> drivers/staging/comedi/comedidev.h | 2 ++
>> drivers/staging/comedi/drivers/das16m1.c | 2 +-
>> drivers/staging/comedi/drivers/das1800.c | 2 +-
>> drivers/staging/comedi/drivers/ni_mio_common.c | 2 +-
>> drivers/staging/comedi/drivers/ni_pcidio.c | 2 +-
>> drivers/staging/comedi/drivers/pcl812.c| 2 +-
>> drivers/staging/comedi/drivers/pcl816.c| 2 +-
>> 9 files changed, 28 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/staging/comedi/comedi_buf.c
>> b/drivers/staging/comedi/comedi_buf.c
>> index df4a9c4..52fc634 100644
>> --- a/drivers/staging/comedi/comedi_buf.c
>> +++ b/drivers/staging/comedi/comedi_buf.c
>> @@ -243,6 +243,24 @@ void comedi_buf_reset(struct comedi_subdevice *s)
>> async->events = 0;
>> }
>>
>> +/**
>> + * comedi_buf_n_bytes_ready() - Returns the number of bytes ready to
>> read/write
>> + * @s: comedi_subdevice struct
>> + */
>> +unsigned int comedi_buf_n_bytes_ready(struct comedi_subdevice *s)
>> +{
>> +struct comedi_async *async = s->async;
>> +
>> +if (async->buf_write_count == async->buf_read_count)
>> +return 0;
>> +
>> +if (async->buf_write_count > async->buf_read_count)
>> +return async->buf_write_count - async->buf_read_count;
>> +else
>> +return async->buf_read_count - async->buf_write_count;
>> +}
>> +EXPORT_SYMBOL_GPL(comedi_buf_n_bytes_ready);
>> +
>
> This will actually break things. The buf_write_count, buf_read_count,
> buf_write_alloc_count, buf_read_alloc_count, and munge_count members of
> struct comedi_async are all supposed to work modulo 2^32 (UINT_MAX+1).
> Logically (with big-ints):
>
> buf_read_count <= buf_read_alloc_count <= munge_count <= buf_write_count
> <= buf_write_alloc_count <= (buf_read_count + prealloc_bufsz)
>
> and logically they only increase in value, but in practice are all
> Reduced modulo 2^32. The upshot is that this function should always return:
>
> return async->buf_write_count - async->buf_read_count;
>
> which could easily be inlined to avoid an external function call.
I don't see how it "breaks" anything since the two if cases will return the
same result as your proposal above. And if the logic of your "count" values
is correct, and I assume it is, then the 'else' case could never happen.
Regardless, I like your inline proposal better than the exported function.
I'll rebase this as suggested.
Thanks,
Hartley
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: comedi: pcl730: add support for Diamond Systems IR104-PBF module
On 2014-05-29 18:42, H Hartley Sweeten wrote: The Diamond Systems IR104-PBF board is a PC/104 module with 20 optoisolated inputs and 20 relay outputs. This board can be supported by the pcl730 driver. Signed-off-by: H Hartley Sweeten Cc: Ian Abbott Cc: Greg Kroah-Hartman 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 2/2] staging: comedi: pcl724: add support for Diamond Systems ONYX-MM-DIO module
On 2014-05-29 18:42, H Hartley Sweeten wrote: The Diamond Systems ONYX-MM-DIO board is a PC/104 module with two 8255 chips providing 48 digital I/O channels. This board can be supported by the pcl724 driver. Signed-off-by: H Hartley Sweeten Cc: Ian Abbott Cc: Greg Kroah-Hartman 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: comedi: ke_counter: add ability to select counter clock source
On 2014-05-29 18:45, H Hartley Sweeten wrote:
Add an (*insn_config) to the counter subdevice to allow the user to select
the clock source for the counters using the INSN_CONFIG_SET_CLOCK_SRC
instruction. The current selection can be queried with the instruction
INSN_CONFIG_GET_CLOCK_SRC.
Also, handle the INSN_CONFIG_RESET instruction to reset all the counters.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/ke_counter.c | 57 +++--
1 file changed, 54 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ke_counter.c
b/drivers/staging/comedi/drivers/ke_counter.c
index ec43c38..ed873c4 100644
--- a/drivers/staging/comedi/drivers/ke_counter.c
+++ b/drivers/staging/comedi/drivers/ke_counter.c
@@ -93,6 +93,58 @@ static int ke_counter_insn_read(struct comedi_device *dev,
return insn->n;
}
+static void ke_counter_reset(struct comedi_device *dev)
+{
+ unsigned int chan;
+
+ for (chan = 0; chan < 3; chan++)
+ outb(0, dev->iobase + KE_RESET_REG(chan));
+}
+
+static int ke_counter_insn_config(struct comedi_device *dev,
+ struct comedi_subdevice *s,
+ struct comedi_insn *insn,
+ unsigned int *data)
+{
+ switch (data[0]) {
+ case INSN_CONFIG_SET_CLOCK_SRC:
+ switch (data[1]) {
+ case KE_OSC_SEL_EXT:/* Pin 21 on D-sub */
+ case KE_OSC_SEL_4MHZ: /* option */
+ case KE_OSC_SEL_20MHZ: /* default */
+ break;
+ default:
+ return -EINVAL;
+ }
+ outb(data[1], dev->iobase + KE_OSC_SEL_REG);
+ break;
+ case INSN_CONFIG_GET_CLOCK_SRC:
+ data[1] = inb(dev->iobase + KE_OSC_SEL_REG);
+ switch (data[1]) {
+ case KE_OSC_SEL_EXT:
+ data[2] = 0;/* Unknown */
+ break;
+ case KE_OSC_SEL_4MHZ:
+ data[2] = 250; /* 250ns */
+ break;
+ case KE_OSC_SEL_20MHZ:
+ data[2] = 50; /* 50ns */
+ break;
+ default:
+ data[2] = 0;/* Invalid? */
+ break;
+ }
+ break;
+ case INSN_CONFIG_RESET:
+ ke_counter_reset(dev);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return insn->n;
+}
+
static int ke_counter_do_insn_bits(struct comedi_device *dev,
struct comedi_subdevice *s,
struct comedi_insn *insn,
@@ -130,6 +182,7 @@ static int ke_counter_auto_attach(struct comedi_device *dev,
s->range_table = &range_unknown;
s->insn_read = ke_counter_insn_read;
s->insn_write= ke_counter_insn_write;
+ s->insn_config = ke_counter_insn_config;
s = &dev->subdevices[1];
s->type = COMEDI_SUBD_DO;
@@ -141,9 +194,7 @@ static int ke_counter_auto_attach(struct comedi_device *dev,
outb(KE_OSC_SEL_20MHZ, dev->iobase + KE_OSC_SEL_REG);
- outb(0, dev->iobase + KE_RESET_REG(0));
- outb(0, dev->iobase + KE_RESET_REG(1));
- outb(0, dev->iobase + KE_RESET_REG(2));
+ ke_counter_reset(dev);
return 0;
}
Okay, but it would be nice to expose the clock source values in
"comedi.h" as well.
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: comedi: comedi_buf: introduce comedi_buf_n_bytes_ready()
On 2014-06-04 19:30, Hartley Sweeten wrote:
On Wednesday, June 04, 2014 11:17 AM, Ian Abbott wrote:
On 2014-05-29 18:38, H Hartley Sweeten wrote:
Introduce a helper function to return the number of bytes that are
ready to read/write from/to the comedi_async buffer. The "write to"
use doesn't really make much sense but is handled for completeness.
Use the helper function in the comedi drivers that currently do the
calculation as part of the (*poll) operation.
Also, use the helper in comedi_fops where the calculation is used as
part of the subdevice going nonbusy.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/comedi_buf.c| 18 ++
drivers/staging/comedi/comedi_fops.c | 5 ++---
drivers/staging/comedi/comedidev.h | 2 ++
drivers/staging/comedi/drivers/das16m1.c | 2 +-
drivers/staging/comedi/drivers/das1800.c | 2 +-
drivers/staging/comedi/drivers/ni_mio_common.c | 2 +-
drivers/staging/comedi/drivers/ni_pcidio.c | 2 +-
drivers/staging/comedi/drivers/pcl812.c| 2 +-
drivers/staging/comedi/drivers/pcl816.c| 2 +-
9 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/comedi/comedi_buf.c
b/drivers/staging/comedi/comedi_buf.c
index df4a9c4..52fc634 100644
--- a/drivers/staging/comedi/comedi_buf.c
+++ b/drivers/staging/comedi/comedi_buf.c
@@ -243,6 +243,24 @@ void comedi_buf_reset(struct comedi_subdevice *s)
async->events = 0;
}
+/**
+ * comedi_buf_n_bytes_ready() - Returns the number of bytes ready to read/write
+ * @s: comedi_subdevice struct
+ */
+unsigned int comedi_buf_n_bytes_ready(struct comedi_subdevice *s)
+{
+ struct comedi_async *async = s->async;
+
+ if (async->buf_write_count == async->buf_read_count)
+ return 0;
+
+ if (async->buf_write_count > async->buf_read_count)
+ return async->buf_write_count - async->buf_read_count;
+ else
+ return async->buf_read_count - async->buf_write_count;
+}
+EXPORT_SYMBOL_GPL(comedi_buf_n_bytes_ready);
+
This will actually break things. The buf_write_count, buf_read_count,
buf_write_alloc_count, buf_read_alloc_count, and munge_count members of
struct comedi_async are all supposed to work modulo 2^32 (UINT_MAX+1).
Logically (with big-ints):
buf_read_count <= buf_read_alloc_count <= munge_count <= buf_write_count
<= buf_write_alloc_count <= (buf_read_count + prealloc_bufsz)
and logically they only increase in value, but in practice are all
Reduced modulo 2^32. The upshot is that this function should always return:
return async->buf_write_count - async->buf_read_count;
which could easily be inlined to avoid an external function call.
I don't see how it "breaks" anything since the two if cases will return the
same result as your proposal above. And if the logic of your "count" values
is correct, and I assume it is, then the 'else' case could never happen.
It breaks when buf_write_count has wrapped around more often than
buf_read_count. For example if buf_read_count is near the 2^32 boundary
and buf_write_count has already gone past the 2^32 boundary then your
code would hit the 'else' (because of the modulo arithmetic) and return
buf_read_count - buf_write_count instead of buf_write_count -
buf_read_count.
Regardless, I like your inline proposal better than the exported function.
I'll rebase this as suggested.
Thanks,
Hartley
Cheers,
Ian.
--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/4] dma-mapping: Add devm_ interface for dma_map_single()
Hi, On Wed, Jun 04, 2014 at 06:03:36PM +0300, Eli Billauer wrote: > I believe that I need a managed dma_map_single() my own driver, > which doesn't fall in the case of a single use: The driver allocates > its buffers with __get_free_pages() (or the to-be managed version of > it). Then it cuts the allocated memory into smaller buffers (in some > cases, and with certain alignment rules), and then calls > dma_map_single() to do the DMA mapping for each. The buffers are > held along the driver's lifetime, with DMA sync API juggling control > back and forth to the hardware. Note that the DMA is noncoherent. What you are trying to do should work with dma_alloc_noncoherent(). The API allows partial syncs on this memory, so you should be fine. The problem with a devm variant of dma_map_* is that it is too easy to misuse or to use it wrong so that a driver could eat up all available DMA handles on some platforms. Joerg ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: usbip: stub_main.c: Cleaning up missing null-terminate after strncpy call
Added a guaranteed null-terminate after call to strncpy.
This was partly found using a static code analysis program called cppcheck.
Signed-off-by: Rickard Strandqvist
---
drivers/staging/usbip/stub_main.c |1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/usbip/stub_main.c
b/drivers/staging/usbip/stub_main.c
index 9c5832a..f1eb6a2 100644
--- a/drivers/staging/usbip/stub_main.c
+++ b/drivers/staging/usbip/stub_main.c
@@ -166,6 +166,7 @@ static ssize_t store_match_busid(struct device_driver *dev,
const char *buf,
return -EINVAL;
strncpy(busid, buf + 4, BUSID_SIZE);
+ busid[BUSID_SIZE - 1] = '\0';
if (!strncmp(buf, "add ", 4)) {
if (add_match_busid(busid) < 0)
--
1.7.10.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
XVME 6300 with TSI148 bridge on 64 bit Debian (Linux 3.2.57) vme_user issue
Dear All,
I came across the link here and decided to write to you, as I am
facing a very similar problem:
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2013-May/037941.html
With the above linux, I have recompiled the kernel and booting the
image with a vme_user.bus=0 vme_tsi148.geoid=1 vme_tsi148.err_chk=1
flags. I am just starting to get familiar with VME.
Using XVME 6300 (sitting in Slot 1), I am trying to access a ZMI 4100
board (in slot 2, only 2 slots on the chassis whose back plane
supports GA) via geographical addressing. The ZMI board (supports
only A24, D16/32, GA, NO CS/CSR). I pretty much have the same code as
mentioned in the thread, however all I read are 0xff's and my system
hangs every once in a while (needs hard reset). This makes debugging
very hard. I am trying to read valid registers at a given offset (in
this case 0x003C).
My master struct is setup as below and I hope you can help me with the
following questions:
master.enable = 1;
master.vme_addr = 0x1;
master.size = 0x1;
master.aspace = 2; // VME_A24
master.cycle = 0x2000 | 0x8000;// user/data access
master.dwidth = 2; // 16 bit word access
0. I suspect my master struct is packed wrong.
struct vme_master {
int enable; /* State of Window */
unsigned long long vme_addr;/* Starting Address on the VMEbus */
unsigned long long size;/* Window Size */
vme_address_t aspace; /* Address Space */
vme_cycle_t cycle; /* Cycle properties */
vme_width_t dwidth; /* Maximum Data Width */
};
1. I don't believe accessing 64k of the ZMI's VME address space is a
valid operation, could this be causing the bus to hang? Actually, I
have this doubt because I am unsure how exactly the master window is
setup.
2. From the documentation of ZMI 4100, it is claimed that the board
supports VME64 and VME64X. However, I am not sure if master.cycle
bits for 2eVME need to be set or not?!
3. Is there away to prevent the bus from hanging by modifying tsi148
device code?
Thanks for reading this far, and any help is sincerely appreciated!
Cheers,
MM
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch v2] mm, pcp: allow restoring percpu_pagelist_fraction default
Hello!
On Jun 4, 2014, at 8:34 PM, David Rientjes wrote:
> @@ -5850,23 +5851,39 @@ int percpu_pagelist_fraction_sysctl_handler(ctl_table
> *table, int write,
> void __user *buffer, size_t *length, loff_t *ppos)
> {
> struct zone *zone;
> - unsigned int cpu;
> + int old_percpu_pagelist_fraction;
> int ret;
>
> + mutex_lock(&pcp_batch_high_lock);
> + old_percpu_pagelist_fraction = percpu_pagelist_fraction;
> +
> ret = proc_dointvec_minmax(table, write, buffer, length, ppos);
> - if (!write || (ret < 0))
> - return ret;
> + if (!write || ret < 0)
> + goto out;
> +
> + /* Sanity checking to avoid pcp imbalance */
> + if (percpu_pagelist_fraction &&
> + percpu_pagelist_fraction < MIN_PERCPU_PAGELIST_FRACTION) {
> + percpu_pagelist_fraction = old_percpu_pagelist_fraction;
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + ret = 0;
Minor nitpick I guess, but ret cannot be anything but 0 here I think (until
somebody changes the way proc_dointvec_minmax for write=true operates)?
The patch is good otherwise.
Thanks.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] memorystick: rtsx: add cancel_work when remove driver
From: Micky Ching
Add cancel_work_sync() in rtsx_pci_ms_drv_remove(). Using
cancel_work_sync() to cancel pending request handle work when remove
driver.
Signed-off-by: Micky Ching
---
drivers/memstick/host/rtsx_pci_ms.c |1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/memstick/host/rtsx_pci_ms.c
b/drivers/memstick/host/rtsx_pci_ms.c
index 2a635b6..c880ba6 100644
--- a/drivers/memstick/host/rtsx_pci_ms.c
+++ b/drivers/memstick/host/rtsx_pci_ms.c
@@ -601,6 +601,7 @@ static int rtsx_pci_ms_drv_remove(struct platform_device
*pdev)
pcr->slots[RTSX_MS_CARD].card_event = NULL;
msh = host->msh;
host->eject = true;
+ cancel_work_sync(&host->handle_req);
mutex_lock(&host->host_mutex);
if (host->req) {
--
1.7.9.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
