Re: [PATCH] staging: media: lirc: lirc_zilog.c: adjust debug messages

2014-11-03 Thread Aya Mahfouz 
On Sun, Nov 02, 2014 at 12:40:13PM +0100, Konrad Zapalowicz wrote:
 On 11/01, Aya Mahfouz wrote:
  This patch removes one debug message and replaces a dev_err
  call by pr_err.
 
 Usually you would like to send this as two separate patches because
 replacing a debug message is way different than removing some code. It
 should look like:
 
   PATCH 0/2 staging: media: adjust debug messages
   PATCH 1/2 staging: media: replace dev_err...
   PATCH 2/2 staging: media: remove debug message..
 
 The lirc_zilog.c is not necessary in the topic line, as this can be seen
 from the diff.
 

Thanks Konrad for your pieces of advice.

Kind Regards,
Aya Saif El-yazal Mahfouz
 
  Signed-off-by: Aya Mahfouz mahfouz.saif.elya...@gmail.com
  ---
   drivers/staging/media/lirc/lirc_zilog.c | 7 +--
   1 file changed, 1 insertion(+), 6 deletions(-)
  
  diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
  b/drivers/staging/media/lirc/lirc_zilog.c
  index 11a7cb1..ba538cd4 100644
  --- a/drivers/staging/media/lirc/lirc_zilog.c
  +++ b/drivers/staging/media/lirc/lirc_zilog.c
  @@ -1336,11 +1336,6 @@ static int close(struct inode *node, struct file 
  *filep)
  /* find our IR struct */
  struct IR *ir = filep-private_data;
   
  -   if (ir == NULL) {
  -   dev_err(ir-l.dev, close: no private_data attached to the 
  file!\n);
  -   return -ENODEV;
  -   }
  -
 
 What is the reason behind this change? What would happen if the
 filep-private_data is NULL? Are the callers of this function aware that
 it will not return ENODEV anymore?
 
 thanks,
 konrad
 
  atomic_dec(ir-open_count);
   
  put_ir_device(ir, false);
  @@ -1633,7 +1628,7 @@ out_put_xx:
   out_put_ir:
  put_ir_device(ir, true);
   out_no_ir:
  -   dev_err(ir-l.dev, %s: probing IR %s on %s (i2c-%d) failed with %d\n,
  +   pr_err(%s: probing IR %s on %s (i2c-%d) failed with %d\n,
  __func__, tx_probe ? Tx : Rx, adap-name, adap-nr,
 ret);
  mutex_unlock(ir_devices_lock);
  -- 
  1.9.3
  
  ___
  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

Re: [PATCH] staging: media: lirc: lirc_zilog.c: adjust debug messages

2014-11-02 Thread Konrad Zapalowicz 
On 11/01, Aya Mahfouz wrote:
 This patch removes one debug message and replaces a dev_err
 call by pr_err.

Usually you would like to send this as two separate patches because
replacing a debug message is way different than removing some code. It
should look like:

PATCH 0/2 staging: media: adjust debug messages
PATCH 1/2 staging: media: replace dev_err...
PATCH 2/2 staging: media: remove debug message..

The lirc_zilog.c is not necessary in the topic line, as this can be seen
from the diff.
 
 Signed-off-by: Aya Mahfouz mahfouz.saif.elya...@gmail.com
 ---
  drivers/staging/media/lirc/lirc_zilog.c | 7 +--
  1 file changed, 1 insertion(+), 6 deletions(-)
 
 diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
 b/drivers/staging/media/lirc/lirc_zilog.c
 index 11a7cb1..ba538cd4 100644
 --- a/drivers/staging/media/lirc/lirc_zilog.c
 +++ b/drivers/staging/media/lirc/lirc_zilog.c
 @@ -1336,11 +1336,6 @@ static int close(struct inode *node, struct file 
 *filep)
   /* find our IR struct */
   struct IR *ir = filep-private_data;
  
 - if (ir == NULL) {
 - dev_err(ir-l.dev, close: no private_data attached to the 
 file!\n);
 - return -ENODEV;
 - }
 -

What is the reason behind this change? What would happen if the
filep-private_data is NULL? Are the callers of this function aware that
it will not return ENODEV anymore?

thanks,
konrad

   atomic_dec(ir-open_count);
  
   put_ir_device(ir, false);
 @@ -1633,7 +1628,7 @@ out_put_xx:
  out_put_ir:
   put_ir_device(ir, true);
  out_no_ir:
 - dev_err(ir-l.dev, %s: probing IR %s on %s (i2c-%d) failed with %d\n,
 + pr_err(%s: probing IR %s on %s (i2c-%d) failed with %d\n,
   __func__, tx_probe ? Tx : Rx, adap-name, adap-nr,
  ret);
   mutex_unlock(ir_devices_lock);
 -- 
 1.9.3
 
 ___
 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