Cleaned up and fixed coding convention issues as reporteed by
checkpatch.pl tool on the file `drivers.c'.
Signed-off-by: Henri Häkkinen <[email protected]>
---
drivers/staging/comedi/drivers.c | 85 +++++++++++++++++++------------------
1 files changed, 44 insertions(+), 41 deletions(-)
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index 4a29ed7..55521d5 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -117,8 +117,8 @@ int comedi_device_attach(struct comedi_device *dev, struct comedi_devconfig *it)
for (driv = comedi_drivers; driv; driv = driv->next) {
if (!try_module_get(driv->module)) {
- printk
- (KERN_INFO "comedi: failed to increment module count, skipping\n");
+ printk(KERN_INFO "comedi: failed to increment module "
+ "count, skipping\n");
continue;
}
if (driv->num_names) {
@@ -205,9 +205,8 @@ int comedi_driver_unregister(struct comedi_driver *driver)
mutex_lock(&dev->mutex);
if (dev->attached && dev->driver == driver) {
if (dev->use_count)
- printk
- (KERN_WARNING "BUG! detaching device with use_count=%d\n",
- dev->use_count);
+ printk(KERN_WARNING "BUG! detaching device "
+ "with use_count=%d\n", dev->use_count);
comedi_device_detach(dev);
}
mutex_unlock(&dev->mutex);
@@ -441,21 +440,19 @@ int comedi_buf_alloc(struct comedi_device *dev, struct comedi_subdevice *s,
if (async->buf_page_list) {
unsigned i;
for (i = 0; i < async->n_buf_pages; ++i) {
- if (async->buf_page_list[i].virt_addr) {
- clear_bit(PG_reserved, &(virt_to_page(async->buf_page_list[i].virt_addr)->flags));
+ void *virt_addr = async->buf_page_list[i].virt_addr;
+ dma_addr_t dma_addr = async->buf_page_list[i].dma_addr;
+
+ if (virt_addr) {
+ struct page *page = virt_to_page(virt_addr);
+ clear_bit(PG_reserved, &page->flags);
if (s->async_dma_dir != DMA_NONE) {
dma_free_coherent(dev->hw_dev,
PAGE_SIZE,
- async->
- buf_page_list
- [i].virt_addr,
- async->
- buf_page_list
- [i].dma_addr);
+ virt_addr,
+ dma_addr);
} else {
- free_page((unsigned long)
- async->buf_page_list[i].
- virt_addr);
+ free_page((unsigned long) virt_addr);
}
}
}
@@ -478,26 +475,29 @@ int comedi_buf_alloc(struct comedi_device *dev, struct comedi_subdevice *s,
}
if (pages) {
for (i = 0; i < n_pages; i++) {
+ struct comedi_buf_page *buf_page =
+ &async->buf_page_list[i];
+ dma_addr_t dma_addr = buf_page->dma_addr;
+ struct page *page;
+
if (s->async_dma_dir != DMA_NONE) {
- async->buf_page_list[i].virt_addr =
+ buf_page->virt_addr =
dma_alloc_coherent(dev->hw_dev,
PAGE_SIZE,
- &async->
- buf_page_list
- [i].dma_addr,
+ &dma_addr,
GFP_KERNEL |
__GFP_COMP);
} else {
- async->buf_page_list[i].virt_addr =
+ buf_page->virt_addr =
(void *)
get_zeroed_page(GFP_KERNEL);
}
- if (async->buf_page_list[i].virt_addr == NULL)
+ if (buf_page->virt_addr == NULL)
break;
- set_bit(PG_reserved,
- &(virt_to_page(async->buf_page_list[i].virt_addr)->flags));
- pages[i] = virt_to_page(async->buf_page_list[i].virt_addr);
+ page = virt_to_page(buf_page->virt_addr);
+ set_bit(PG_reserved, &page->flags);
+ pages[i] = page;
}
}
if (i == n_pages) {
@@ -510,24 +510,27 @@ int comedi_buf_alloc(struct comedi_device *dev, struct comedi_subdevice *s,
/* Some allocation failed above. */
if (async->buf_page_list) {
for (i = 0; i < n_pages; i++) {
- if (async->buf_page_list[i].virt_addr ==
- NULL) {
+ struct comedi_buf_page *buf_page =
+ &async->buf_page_list[i];
+ void *virt_addr =
+ buf_page->virt_addr;
+ dma_addr_t dma_addr =
+ buf_page->dma_addr;
+ struct page *page =
+ virt_to_page(virt_addr);
+
+ if (virt_addr == NULL)
break;
- }
- clear_bit(PG_reserved, &(virt_to_page(async->buf_page_list[i].virt_addr)->flags));
+
+ clear_bit(PG_reserved, &page->flags);
if (s->async_dma_dir != DMA_NONE) {
dma_free_coherent(dev->hw_dev,
PAGE_SIZE,
- async->
- buf_page_list
- [i].virt_addr,
- async->
- buf_page_list
- [i].dma_addr);
+ virt_addr,
+ dma_addr);
} else {
free_page((unsigned long)
- async->buf_page_list
- [i].virt_addr);
+ virt_addr);
}
}
vfree(async->buf_page_list);
@@ -646,8 +649,8 @@ unsigned comedi_buf_write_free(struct comedi_async *async, unsigned int nbytes)
{
if ((int)(async->buf_write_count + nbytes -
async->buf_write_alloc_count) > 0) {
- printk
- (KERN_INFO "comedi: attempted to write-free more bytes than have been write-allocated.\n");
+ printk(KERN_INFO "comedi: attempted to write-free more bytes "
+ "than have been write-allocated.\n");
nbytes = async->buf_write_alloc_count - async->buf_write_count;
}
async->buf_write_count += nbytes;
@@ -683,8 +686,8 @@ unsigned comedi_buf_read_free(struct comedi_async *async, unsigned int nbytes)
smp_mb();
if ((int)(async->buf_read_count + nbytes -
async->buf_read_alloc_count) > 0) {
- printk(KERN_INFO
- "comedi: attempted to read-free more bytes than have been read-allocated.\n");
+ printk(KERN_INFO "comedi: attempted to read-free more bytes "
+ "than have been read-allocated.\n");
nbytes = async->buf_read_alloc_count - async->buf_read_count;
}
async->buf_read_count += nbytes;
--
1.7.1
2010/6/12 Henri H?kkinen <[email protected]>:
> - ? ? ? ? ? ? ? ? ? ? ? printk
> - ? ? ? ? ? ? ? ? ? ? ? ? ? (KERN_INFO "comedi: failed to increment module count, skipping\n");
> + ? ? ? ? ? ? ? ? ? ? ? printk(KERN_INFO "comedi: failed to increment module "
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"count, skipping\n");
Hi Henri,
Regarding your breaking up of printk statements, although some of
those lines do go over 80 characters, it is preferable to keep the
strings together since then those are searchable within the code.
I figure it is quite acceptable to break the string after "comedi: ",
so maybe that will fix the line length issue, otherwise it is
preferable to keep the checkpatch warning in this case.
Mark
On Sun, 2010-06-13 at 10:14 +0800, Mark Rankilor wrote:
> 2010/6/12 Henri Häkkinen <[email protected]>:
> > - printk
> > - (KERN_INFO "comedi: failed to increment module count, skipping\n");
> > + printk(KERN_INFO "comedi: failed to increment module "
> > + "count, skipping\n");
>
> Regarding your breaking up of printk statements, although some of
> those lines do go over 80 characters, it is preferable to keep the
> strings together since then those are searchable within the code.
>
> I figure it is quite acceptable to break the string after "comedi: ",
> so maybe that will fix the line length issue, otherwise it is
> preferable to keep the checkpatch warning in this case.
A couple of options for comedi:
1: Use #define pr_fmt(fmt) "comedi: " fmt
pr_<level>(format, ...)
2: Create some comedi logging functions or macros like:
comedi_<level>(fmt, arg...) (ie: comedi_info, comedi_err, etc)
where "comedi:" is always prefixed and an
optional #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
could be used.
That'd shorten line lengths quite a bit and add
some better standardization to comedi.
On Sat, 2010-06-12 at 22:07 -0700, Joe Perches wrote:
> 2: Create some comedi logging functions or macros like:
> comedi_<level>(fmt, arg...) (ie: comedi_info, comedi_err, etc)
> where "comedi:" is always prefixed and an
> optional #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> could be used.
Maybe this is a start:
Signed-off-by: Joe Perches <[email protected]>
---
drivers/staging/comedi/comedidev.h | 54 ++++++++++++++++++++++++++++++++++--
1 files changed, 51 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/comedi/comedidev.h b/drivers/staging/comedi/comedidev.h
index 4eb2b77..6c2bdde 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -43,11 +43,59 @@
#include "comedi.h"
-#define DPRINTK(format, args...) do { \
- if (comedi_debug) \
- printk(KERN_DEBUG "comedi: " format , ## args); \
+#define comedi_printk(level, fmt, args...) \
+ printk(level "comedi: " pr_fmt(fmt), ##args)
+
+#define DPRINTK(format, args...) \
+do { \
+ if (comedi_debug) \
+ comedi_printk(KERN_DEBUG, fmt, ##args); \
} while (0)
+#define comedi_emerg(fmt, ...) \
+ comedi_printk(KERN_EMERG, fmt, ##__VA_ARGS__)
+#define comedi_alert(fmt, ...) \
+ comedi_printk(KERN_ALERT, fmt, ##__VA_ARGS__)
+#define comedi_crit(fmt, ...) \
+ comedi_printk(KERN_CRIT, fmt, ##__VA_ARGS__)
+#define comedi_err(fmt, ...) \
+ comedi_printk(KERN_ERR, fmt, ##__VA_ARGS__)
+#define comedi_warn(fmt, ...) \
+ comedi_printk(KERN_WARNING, fmt, ##__VA_ARGS__)
+#define comedi_notice(fmt, ...) \
+ comedi_printk(KERN_NOTICE, fmt, ##__VA_ARGS__)
+#define comedi_info(fmt, ...) \
+ comedi_printk(KERN_INFO, fmt, ##__VA_ARGS__)
+
+/* comedi_devel() should produce zero code unless DEBUG is defined */
+#ifdef DEBUG
+#define comedi_devel(fmt, ...) \
+ comedi_printk(KERN_DEBUG, fmt, ##__VA_ARGS__)
+#else
+#define comedi_devel(fmt, ...) \
+({ \
+ if (0) \
+ comedi_printk(KERN_DEBUG, fmt, ##__VA_ARGS__); \
+ 0; \
+})
+#endif
+
+#if defined(DEBUG)
+#define comedi_debug(fmt, ...) \
+ comedi_printk(KERN_DEBUG, fmt, ##__VA_ARGS__)
+#elif defined(CONFIG_DYNAMIC_DEBUG)
+/* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */
+#define comedi_debug(fmt, ...) \
+ dynamic_pr_debug(fmt, ##__VA_ARGS__)
+#else
+#define comedi_debug(fmt, ...) \
+({ \
+ if (0) \
+ comedi_printk(KERN_DEBUG, fmt, ##__VA_ARGS__); \
+ 0; \
+})
+#endif
+
#define COMEDI_VERSION(a, b, c) (((a) << 16) + ((b) << 8) + (c))
#define COMEDI_VERSION_CODE COMEDI_VERSION(COMEDI_MAJORVERSION, \
COMEDI_MINORVERSION, COMEDI_MICROVERSION)
Hello
There are several printk statements without the "comedi:" prefix. Such as:
printk(KERN_WARNING "BUG: dev->driver=NULL in comedi_device_detach()\n");
Do you think it is better to leave these as they are, or should they be changed to use comedi_xxx macros (which will print the "comedi:" prefix)?
Also even with logging macros, there will be few lines which go beyond the 80 character boundary.
On 13.6.2010, at 8.30, Joe Perches wrote:
> On Sat, 2010-06-12 at 22:07 -0700, Joe Perches wrote:
>> 2: Create some comedi logging functions or macros like:
>> comedi_<level>(fmt, arg...) (ie: comedi_info, comedi_err, etc)
>> where "comedi:" is always prefixed and an
>> optional #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> could be used.
>
> Maybe this is a start:
>
> Signed-off-by: Joe Perches <[email protected]>
> ---
> drivers/staging/comedi/comedidev.h | 54 ++++++++++++++++++++++++++++++++++--
> 1 files changed, 51 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/comedi/comedidev.h b/drivers/staging/comedi/comedidev.h
> index 4eb2b77..6c2bdde 100644
> --- a/drivers/staging/comedi/comedidev.h
> +++ b/drivers/staging/comedi/comedidev.h
> @@ -43,11 +43,59 @@
>
> #include "comedi.h"
>
> -#define DPRINTK(format, args...) do { \
> - if (comedi_debug) \
> - printk(KERN_DEBUG "comedi: " format , ## args); \
> +#define comedi_printk(level, fmt, args...) \
> + printk(level "comedi: " pr_fmt(fmt), ##args)
> +
> +#define DPRINTK(format, args...) \
> +do { \
> + if (comedi_debug) \
> + comedi_printk(KERN_DEBUG, fmt, ##args); \
> } while (0)
>
> +#define comedi_emerg(fmt, ...) \
> + comedi_printk(KERN_EMERG, fmt, ##__VA_ARGS__)
> +#define comedi_alert(fmt, ...) \
> + comedi_printk(KERN_ALERT, fmt, ##__VA_ARGS__)
> +#define comedi_crit(fmt, ...) \
> + comedi_printk(KERN_CRIT, fmt, ##__VA_ARGS__)
> +#define comedi_err(fmt, ...) \
> + comedi_printk(KERN_ERR, fmt, ##__VA_ARGS__)
> +#define comedi_warn(fmt, ...) \
> + comedi_printk(KERN_WARNING, fmt, ##__VA_ARGS__)
> +#define comedi_notice(fmt, ...) \
> + comedi_printk(KERN_NOTICE, fmt, ##__VA_ARGS__)
> +#define comedi_info(fmt, ...) \
> + comedi_printk(KERN_INFO, fmt, ##__VA_ARGS__)
> +
> +/* comedi_devel() should produce zero code unless DEBUG is defined */
> +#ifdef DEBUG
> +#define comedi_devel(fmt, ...) \
> + comedi_printk(KERN_DEBUG, fmt, ##__VA_ARGS__)
> +#else
> +#define comedi_devel(fmt, ...) \
> +({ \
> + if (0) \
> + comedi_printk(KERN_DEBUG, fmt, ##__VA_ARGS__); \
> + 0; \
> +})
> +#endif
> +
> +#if defined(DEBUG)
> +#define comedi_debug(fmt, ...) \
> + comedi_printk(KERN_DEBUG, fmt, ##__VA_ARGS__)
> +#elif defined(CONFIG_DYNAMIC_DEBUG)
> +/* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */
> +#define comedi_debug(fmt, ...) \
> + dynamic_pr_debug(fmt, ##__VA_ARGS__)
> +#else
> +#define comedi_debug(fmt, ...) \
> +({ \
> + if (0) \
> + comedi_printk(KERN_DEBUG, fmt, ##__VA_ARGS__); \
> + 0; \
> +})
> +#endif
> +
> #define COMEDI_VERSION(a, b, c) (((a) << 16) + ((b) << 8) + (c))
> #define COMEDI_VERSION_CODE COMEDI_VERSION(COMEDI_MAJORVERSION, \
> COMEDI_MINORVERSION, COMEDI_MICROVERSION)
>
>
On Sun, 2010-06-13 at 14:27 +0300, Henri Häkkinen wrote:
> There are several printk statements without the "comedi:" prefix.
> Do you think it is better to leave these as they are, or should they
> be changed to use comedi_xxx macros (which will print the "comedi:"
> prefix)?
>
> printk(KERN_WARNING "BUG: dev->driver=NULL in comedi_device_detach()\n");
I think it's better to convert them.
Anything with "BUG" in the format
maybe should be converted to BUG_ON.
> Also even with logging macros, there will be few lines which go beyond
> the 80 character boundary.
I'd ignore printk related long line warnings.
I suggest coalescing the format string to a single line
where reasonable. If a single printk has non trailing
'\n's in a format, it may be better to split them up.
comedi_info("some incredibly long output line with error: %d\n"
"Another line with some other information: %d\n",
err, info);
On Sat, Jun 12, 2010 at 10:30:50PM -0700, Joe Perches wrote:
> On Sat, 2010-06-12 at 22:07 -0700, Joe Perches wrote:
> > 2: Create some comedi logging functions or macros like:
> > comedi_<level>(fmt, arg...) (ie: comedi_info, comedi_err, etc)
> > where "comedi:" is always prefixed and an
> > optional #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > could be used.
>
> Maybe this is a start:
>
> Signed-off-by: Joe Perches <[email protected]>
> ---
> drivers/staging/comedi/comedidev.h | 54 ++++++++++++++++++++++++++++++++++--
> 1 files changed, 51 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/comedi/comedidev.h b/drivers/staging/comedi/comedidev.h
> index 4eb2b77..6c2bdde 100644
> --- a/drivers/staging/comedi/comedidev.h
> +++ b/drivers/staging/comedi/comedidev.h
> @@ -43,11 +43,59 @@
>
> #include "comedi.h"
>
> -#define DPRINTK(format, args...) do { \
> - if (comedi_debug) \
> - printk(KERN_DEBUG "comedi: " format , ## args); \
> +#define comedi_printk(level, fmt, args...) \
> + printk(level "comedi: " pr_fmt(fmt), ##args)
> +
> +#define DPRINTK(format, args...) \
> +do { \
> + if (comedi_debug) \
> + comedi_printk(KERN_DEBUG, fmt, ##args); \
> } while (0)
>
> +#define comedi_emerg(fmt, ...) \
> + comedi_printk(KERN_EMERG, fmt, ##__VA_ARGS__)
I would prefer the conversion of everything over to the dev_printk()
versions instead of creating a new macro for every individual subsystem.
That way you get the advantage of logging everything in the common
format and the dynamic debug functionality as well.
thanks,
greg k-h
On Thu, 2010-06-17 at 15:51 -0700, Greg KH wrote:
> On Sat, Jun 12, 2010 at 10:30:50PM -0700, Joe Perches wrote:
> > On Sat, 2010-06-12 at 22:07 -0700, Joe Perches wrote:
> > > 2: Create some comedi logging functions or macros like:
> > > comedi_<level>(fmt, arg...) (ie: comedi_info, comedi_err, etc)
> > > where "comedi:" is always prefixed and an
> > > optional #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > could be used.
> I would prefer the conversion of everything over to the dev_printk()
> versions instead of creating a new macro for every individual subsystem.
> That way you get the advantage of logging everything in the common
> format and the dynamic debug functionality as well.
What I posted has dynamic_debug.
+#elif defined(CONFIG_DYNAMIC_DEBUG)
+/* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */
+#define comedi_debug(fmt, ...) \
+ dynamic_pr_debug(fmt, ##__VA_ARGS__)
As far as I know, comedi doesn't always take a struct device *.
I believe it's only used when there's a DMA.
In struct comedi_device, there are two struct device *'s.
struct device *class_dev;
...
struct device *hw_dev;
On Thu, Jun 17, 2010 at 04:15:58PM -0700, Joe Perches wrote:
> On Thu, 2010-06-17 at 15:51 -0700, Greg KH wrote:
> > On Sat, Jun 12, 2010 at 10:30:50PM -0700, Joe Perches wrote:
> > > On Sat, 2010-06-12 at 22:07 -0700, Joe Perches wrote:
> > > > 2: Create some comedi logging functions or macros like:
> > > > comedi_<level>(fmt, arg...) (ie: comedi_info, comedi_err, etc)
> > > > where "comedi:" is always prefixed and an
> > > > optional #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > > could be used.
>
> > I would prefer the conversion of everything over to the dev_printk()
> > versions instead of creating a new macro for every individual subsystem.
> > That way you get the advantage of logging everything in the common
> > format and the dynamic debug functionality as well.
>
> What I posted has dynamic_debug.
>
> +#elif defined(CONFIG_DYNAMIC_DEBUG)
> +/* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */
> +#define comedi_debug(fmt, ...) \
> + dynamic_pr_debug(fmt, ##__VA_ARGS__)
>
> As far as I know, comedi doesn't always take a struct device *.
> I believe it's only used when there's a DMA.
No, there's a struct device down in the device almost always.
> In struct comedi_device, there are two struct device *'s.
>
> struct device *class_dev;
> ...
> struct device *hw_dev;
hw_dev is what we want to use.
thanks,
greg k-h
On Thu, 2010-06-17 at 16:28 -0700, Greg KH wrote:
> On Thu, Jun 17, 2010 at 04:15:58PM -0700, Joe Perches wrote:
> > On Thu, 2010-06-17 at 15:51 -0700, Greg KH wrote:
> > > On Sat, Jun 12, 2010 at 10:30:50PM -0700, Joe Perches wrote:
> > > > On Sat, 2010-06-12 at 22:07 -0700, Joe Perches wrote:
> > > > > 2: Create some comedi logging functions or macros like:
> > > > > comedi_<level>(fmt, arg...) (ie: comedi_info, comedi_err, etc)
> > > > > where "comedi:" is always prefixed and an
> > > > > optional #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > > > could be used.
> >
> > > I would prefer the conversion of everything over to the dev_printk()
> > > versions instead of creating a new macro for every individual subsystem.
> > > That way you get the advantage of logging everything in the common
> > > format and the dynamic debug functionality as well.
> >
> > What I posted has dynamic_debug.
> >
> > +#elif defined(CONFIG_DYNAMIC_DEBUG)
> > +/* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */
> > +#define comedi_debug(fmt, ...) \
> > + dynamic_pr_debug(fmt, ##__VA_ARGS__)
> >
> > As far as I know, comedi doesn't always take a struct device *.
> > I believe it's only used when there's a DMA.
>
> No, there's a struct device down in the device almost always.
>
> > In struct comedi_device, there are two struct device *'s.
> >
> > struct device *class_dev;
> > ...
> > struct device *hw_dev;
>
> hw_dev is what we want to use.
Perhaps Ian or Frank might clarify if
that's reasonable. It doesn't look like
it to me.
Look at comedi_set_hw_dev.
See how often it's used?
On 18/06/10 00:47, Joe Perches wrote:
> On Thu, 2010-06-17 at 16:28 -0700, Greg KH wrote:
>> On Thu, Jun 17, 2010 at 04:15:58PM -0700, Joe Perches wrote:
>>> On Thu, 2010-06-17 at 15:51 -0700, Greg KH wrote:
>>>> On Sat, Jun 12, 2010 at 10:30:50PM -0700, Joe Perches wrote:
>>>>> On Sat, 2010-06-12 at 22:07 -0700, Joe Perches wrote:
>>>>>> 2: Create some comedi logging functions or macros like:
>>>>>> comedi_<level>(fmt, arg...) (ie: comedi_info, comedi_err, etc)
>>>>>> where "comedi:" is always prefixed and an
>>>>>> optional #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>>>>> could be used.
>>>
>>>> I would prefer the conversion of everything over to the dev_printk()
>>>> versions instead of creating a new macro for every individual subsystem.
>>>> That way you get the advantage of logging everything in the common
>>>> format and the dynamic debug functionality as well.
>>>
>>> What I posted has dynamic_debug.
>>>
>>> +#elif defined(CONFIG_DYNAMIC_DEBUG)
>>> +/* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */
>>> +#define comedi_debug(fmt, ...) \
>>> + dynamic_pr_debug(fmt, ##__VA_ARGS__)
>>>
>>> As far as I know, comedi doesn't always take a struct device *.
>>> I believe it's only used when there's a DMA.
>>
>> No, there's a struct device down in the device almost always.
>>
>>> In struct comedi_device, there are two struct device *'s.
>>>
>>> struct device *class_dev;
>>> ...
>>> struct device *hw_dev;
>>
>> hw_dev is what we want to use.
>
> Perhaps Ian or Frank might clarify if
> that's reasonable. It doesn't look like
> it to me.
>
> Look at comedi_set_hw_dev.
> See how often it's used?
It's only set by a few drivers currently. Perhaps it should be set by
comedi_alloc_board_minor() using the 'hardware_device' parameter.
However, in the case of "legacy" comedi devices (the ones not configured
automatically via comedi_auto_config()), that parameter will be NULL.
On a side note, if comedi_alloc_board_minor() were changed to call
comedi_set_hw_dev(), then the driver "_attach" routines could be changed
to check that they really are attaching to the correct hardware device
rather than relying on the struct comedi_devconfig options array values
(which aren't set anyway for auto-configured USB devices).
comedi_set_hw_dev() also ought to return early if the new hw_dev value
is the same as the previous value.
--
-=( Ian Abbott @ MEV Ltd. E-mail: <[email protected]> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-