Re: [PATCH 2/3] staging: comedi: report success/failure of autoconfig

2014-01-06 Thread Dan Carpenter
On Mon, Jan 06, 2014 at 11:56:22AM +, Ian Abbott wrote:
> 
> I know the checkpatch.pl script used to complain about concatenated
> string literals, preferring a single string literal even if the code
> went over 80 columns as a result.  It doesn't seem to complain here,
> but I think it's still recommended practice.  (Could someone clarify
> the recommended practice here?)
> 

Better to put everything as a string literal so we can grep for it.

I think that is still the fashion.  :)

regards,
dan carpenter


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 2/3] staging: comedi: report success/failure of autoconfig

2014-01-06 Thread Ian Abbott

On 2013-12-28 21:32, Bernd Porr wrote:

Added success message to the driver autoconfig and error
message in case it fails. A success message is required
so that the user can find out which comedi driver has been
associated with which udev device. This also makes troubleshooting
much easier when more than one card is in the computer or a
mix of USB and PCI devices.
As Ian suggested we should report both the driver and the board
which might have different names, esp if one driver covers a
range of different boards.

Signed-off-by: Bernd Porr 
---
  drivers/staging/comedi/drivers.c | 19 +--
  1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index d6dc58a..59a8909 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -580,8 +580,12 @@ int comedi_auto_config(struct device *hardware_device,
}

dev = comedi_alloc_board_minor(hardware_device);
-   if (IS_ERR(dev))
+   if (IS_ERR(dev)) {
+   dev_warn(hardware_device,
+"driver '%s' could not create device.\n",
+driver->driver_name);
return PTR_ERR(dev);
+   }
/* Note: comedi_alloc_board_minor() locked dev->mutex. */

dev->driver = driver;
@@ -593,8 +597,19 @@ int comedi_auto_config(struct device *hardware_device,
comedi_device_detach(dev);
mutex_unlock(&dev->mutex);

-   if (ret < 0)
+   if (ret < 0) {
+   dev_warn(hardware_device,
+"driver '%s' faild to auto-configure device.\n",


A typo here: 'faild' should be 'failed'.


+driver->driver_name);
comedi_release_hardware_device(hardware_device);
+   } else {
+   /* class_dev should be set properly here
+  after a successful auto config */


The preferred style for multi-line comments is:

/*
 * blah blah
 * blah blah
 */


+   dev_info(dev->class_dev,
+"driver '%s' has successfully "
+"auto-configured '%s'.\n",
+driver->driver_name, dev->board_name);
+   }


I know the checkpatch.pl script used to complain about concatenated 
string literals, preferring a single string literal even if the code 
went over 80 columns as a result.  It doesn't seem to complain here, but 
I think it's still recommended practice.  (Could someone clarify the 
recommended practice here?)



return ret;
  }
  EXPORT_SYMBOL_GPL(comedi_auto_config);




--
-=( 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