staging: media: lirc: lirc_zilog.c: adjust debug messages

PATCH In review Aya Mahfouz 2 participants 3 messages Nov 1, 2014 – Nov 4, 2014

Subsystems (2)

Media (V4L/DVB) Staging

lore

Share & tools

lore

Aya Mahfouz

author

2014-11-01 21:37 UTC #0 lore raw

PATCH 1/1 staging: media: lirc: lirc_zilog.c: adjust debug messages

This patch removes one debug message and replaces a dev_err call by pr_err.

Signed-off-by Aya Mahfouz <[email protected]>

---
 drivers/staging/media/lirc/lirc_zilog.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

drivers/staging/media/lirc/lirc_zilog.c +1 -6

index 11a7cb1..ba538cd4 100644
@@ -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;
-	}
-
 	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);

Signature · 1 line

1.9.3

Konrad Zapalowicz

2014-11-02 11:40 UTC #1 lore raw

Quoted 2 lines · Aya Mahfouz

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.

Quoted 10 lines

Signed-off-by Aya Mahfouz <[email protected]>
---
 drivers/staging/media/lirc/lirc_zilog.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)
drivers/staging/media/lirc/lirc_zilog.c +0 -0
index 11a7cb1..ba538cd4 100644

drivers/staging/media/lirc/lirc_zilog.c static int close(struct inode *node, struct file *filep) · line 1343

Quoted 8 lines

drivers/staging/media/lirc/lirc_zilog.c +0 -5

@@ -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

Quoted 19 lines

drivers/staging/media/lirc/lirc_zilog.c +1 -1

 	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);

Signature · 6 lines

1.9.3

_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Aya Mahfouz

author

2014-11-04 00:17 UTC #2 lore raw

Quoted 8 lines · Konrad Zapalowicz

Quoted 2 lines · Aya Mahfouz
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:

Quoted 7 lines

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

Quoted 45 lines

Quoted 10 lines
Signed-off-by Aya Mahfouz <[email protected]>
---
 drivers/staging/media/lirc/lirc_zilog.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)
drivers/staging/media/lirc/lirc_zilog.c +0 -0
index 11a7cb1..ba538cd4 100644
drivers/staging/media/lirc/lirc_zilog.c static int close(struct inode *node, struct file *filep) · line 1343
Quoted 8 lines
drivers/staging/media/lirc/lirc_zilog.c +0 -5
@@ -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
Quoted 19 lines
drivers/staging/media/lirc/lirc_zilog.c +1 -1
 	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);
Signature · 6 lines
1.9.3

_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel