All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Fixed all coding style issues for drivers/staging/media/lirc/
@ 2014-10-02  4:40 Amber Thrall
  2014-10-02  4:49 ` Ilia Mirkin
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Amber Thrall @ 2014-10-02  4:40 UTC (permalink / raw)
  To: jarod, m.chehab; +Cc: linux-media, devel, linux-kernel, Amber Thrall

Fixed various coding style issues, including strings over 80 characters long and many 
deprecated printk's have been replaced with proper methods.

Signed-off-by: Amber Thrall <amber.rose.thrall@gmail.com>
---
 drivers/staging/media/lirc/lirc_bt829.c  |  2 +-
 drivers/staging/media/lirc/lirc_imon.c   |  4 +-
 drivers/staging/media/lirc/lirc_sasem.c  |  6 +--
 drivers/staging/media/lirc/lirc_serial.c | 29 ++++++--------
 drivers/staging/media/lirc/lirc_sir.c    |  3 +-
 drivers/staging/media/lirc/lirc_zilog.c  | 69 +++++++++++++++-----------------
 6 files changed, 52 insertions(+), 61 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_bt829.c b/drivers/staging/media/lirc/lirc_bt829.c
index 4c806ba..c70ca68 100644
--- a/drivers/staging/media/lirc/lirc_bt829.c
+++ b/drivers/staging/media/lirc/lirc_bt829.c
@@ -59,7 +59,7 @@ static bool debug;
 #define dprintk(fmt, args...)						 \
 	do {								 \
 		if (debug)						 \
-			printk(KERN_DEBUG DRIVER_NAME ": "fmt, ## args); \
+			dev_dbg(DRIVER_NAME, ": "fmt, ##args); \
 	} while (0)
 
 static int atir_minor;
diff --git a/drivers/staging/media/lirc/lirc_imon.c b/drivers/staging/media/lirc/lirc_imon.c
index 7aca44f..bce0408 100644
--- a/drivers/staging/media/lirc/lirc_imon.c
+++ b/drivers/staging/media/lirc/lirc_imon.c
@@ -623,8 +623,8 @@ static void imon_incoming_packet(struct imon_context *context,
 	if (debug) {
 		dev_info(dev, "raw packet: ");
 		for (i = 0; i < len; ++i)
-			printk("%02x ", buf[i]);
-		printk("\n");
+			dev_info(dev, "%02x ", buf[i]);
+		dev_info(dev, "\n");
 	}
 
 	/*
diff --git a/drivers/staging/media/lirc/lirc_sasem.c b/drivers/staging/media/lirc/lirc_sasem.c
index c20ef56..e88e246 100644
--- a/drivers/staging/media/lirc/lirc_sasem.c
+++ b/drivers/staging/media/lirc/lirc_sasem.c
@@ -583,10 +583,10 @@ static void incoming_packet(struct sasem_context *context,
 	}
 
 	if (debug) {
-		printk(KERN_INFO "Incoming data: ");
+		pr_info("Incoming data: ");
 		for (i = 0; i < 8; ++i)
-			printk(KERN_CONT "%02x ", buf[i]);
-		printk(KERN_CONT "\n");
+			pr_cont("%02x", buf[i]);
+		pr_cont("\n");
 	}
 
 	/*
diff --git a/drivers/staging/media/lirc/lirc_serial.c b/drivers/staging/media/lirc/lirc_serial.c
index 181b92b..b07671b 100644
--- a/drivers/staging/media/lirc/lirc_serial.c
+++ b/drivers/staging/media/lirc/lirc_serial.c
@@ -116,8 +116,7 @@ static bool txsense;	/* 0 = active high, 1 = active low */
 #define dprintk(fmt, args...)					\
 	do {							\
 		if (debug)					\
-			printk(KERN_DEBUG LIRC_DRIVER_NAME ": "	\
-			       fmt, ## args);			\
+			dev_dbg(LIRC_DRIVER_NAME, ": "fmt, ##args); \
 	} while (0)
 
 /* forward declarations */
@@ -356,9 +355,8 @@ static int init_timing_params(unsigned int new_duty_cycle,
 	/* Derive pulse and space from the period */
 	pulse_width = period * duty_cycle / 100;
 	space_width = period - pulse_width;
-	dprintk("in init_timing_params, freq=%d, duty_cycle=%d, "
-		"clk/jiffy=%ld, pulse=%ld, space=%ld, "
-		"conv_us_to_clocks=%ld\n",
+	dprintk("in init_timing_params, freq=%d, duty_cycle=%d, clk/jiffy=%ld,
+			pulse=%ld, space=%ld, conv_us_to_clocks=%ld\n",
 		freq, duty_cycle, __this_cpu_read(cpu_info.loops_per_jiffy),
 		pulse_width, space_width, conv_us_to_clocks);
 	return 0;
@@ -1075,7 +1073,7 @@ static int __init lirc_serial_init(void)
 
 	result = platform_driver_register(&lirc_serial_driver);
 	if (result) {
-		printk("lirc register returned %d\n", result);
+		dprintk("lirc register returned %d\n", result);
 		goto exit_buffer_free;
 	}
 
@@ -1166,22 +1164,20 @@ module_init(lirc_serial_init_module);
 module_exit(lirc_serial_exit_module);
 
 MODULE_DESCRIPTION("Infra-red receiver driver for serial ports.");
-MODULE_AUTHOR("Ralph Metzler, Trent Piepho, Ben Pfaff, "
-	      "Christoph Bartelmus, Andrei Tanas");
+MODULE_AUTHOR("Ralph Metzler, Trent Piepho, Ben Pfaff, Christoph Bartelmus, Andrei Tanas");
 MODULE_LICENSE("GPL");
 
 module_param(type, int, S_IRUGO);
-MODULE_PARM_DESC(type, "Hardware type (0 = home-brew, 1 = IRdeo,"
-		 " 2 = IRdeo Remote, 3 = AnimaX, 4 = IgorPlug,"
-		 " 5 = NSLU2 RX:CTS2/TX:GreenLED)");
+MODULE_PARM_DESC(type, "Hardware type (0 = home-brew, 1 = IRdeo,
+	2 = IRdeo Remote, 3 = AnimaX, 4 = IgorPlug,
+	5 = NSLU2 RX:CTS2/TX:GreenLED)");
 
 module_param(io, int, S_IRUGO);
 MODULE_PARM_DESC(io, "I/O address base (0x3f8 or 0x2f8)");
 
 /* some architectures (e.g. intel xscale) have memory mapped registers */
 module_param(iommap, bool, S_IRUGO);
-MODULE_PARM_DESC(iommap, "physical base for memory mapped I/O"
-		" (0 = no memory mapped io)");
+MODULE_PARM_DESC(iommap, "physical base for memory mapped I/O (0 = no memory mapped io)");
 
 /*
  * some architectures (e.g. intel xscale) align the 8bit serial registers
@@ -1198,13 +1194,12 @@ module_param(share_irq, bool, S_IRUGO);
 MODULE_PARM_DESC(share_irq, "Share interrupts (0 = off, 1 = on)");
 
 module_param(sense, int, S_IRUGO);
-MODULE_PARM_DESC(sense, "Override autodetection of IR receiver circuit"
-		 " (0 = active high, 1 = active low )");
+MODULE_PARM_DESC(sense, "Override autodetection of IR receiver circuit
+		(0 = active high, 1 = active low )");
 
 #ifdef CONFIG_LIRC_SERIAL_TRANSMITTER
 module_param(txsense, bool, S_IRUGO);
-MODULE_PARM_DESC(txsense, "Sense of transmitter circuit"
-		 " (0 = active high, 1 = active low )");
+MODULE_PARM_DESC(txsense, "Sense of transmitter circuit (0 = active high, 1 = active low )");
 #endif
 
 module_param(softcarrier, bool, S_IRUGO);
diff --git a/drivers/staging/media/lirc/lirc_sir.c b/drivers/staging/media/lirc/lirc_sir.c
index 2ee55ea..cdbb71f 100644
--- a/drivers/staging/media/lirc/lirc_sir.c
+++ b/drivers/staging/media/lirc/lirc_sir.c
@@ -143,8 +143,7 @@ static bool debug;
 #define dprintk(fmt, args...)						\
 	do {								\
 		if (debug)						\
-			printk(KERN_DEBUG LIRC_DRIVER_NAME ": "		\
-				fmt, ## args);				\
+			dev_dbg(LIRC_DRIVER_NAME, ": "fmt, ## args); \
 	} while (0)
 
 /* SECTION: Prototypes */
diff --git a/drivers/staging/media/lirc/lirc_zilog.c b/drivers/staging/media/lirc/lirc_zilog.c
index 567feba..9c3a3d7 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -152,10 +152,9 @@ struct tx_data_struct {
 static struct tx_data_struct *tx_data;
 static struct mutex tx_data_lock;
 
-#define zilog_notify(s, args...) printk(KERN_NOTICE KBUILD_MODNAME ": " s, \
-					## args)
-#define zilog_error(s, args...) printk(KERN_ERR KBUILD_MODNAME ": " s, ## args)
-#define zilog_info(s, args...) printk(KERN_INFO KBUILD_MODNAME ": " s, ## args)
+#define zilog_notify(s, args...) dev_notice(KBUILD_MODNAME, ": " s, ## args)
+#define zilog_error(s, args...) dev_err(KBUILD_MODNAME, ": " s, ## args)
+#define zilog_info(s, args...) dev_info(KBUILD_MODNAME, ": " s, ## args)
 
 /* module parameters */
 static bool debug;	/* debug output */
@@ -165,8 +164,7 @@ static int minor = -1;	/* minor number */
 #define dprintk(fmt, args...)						\
 	do {								\
 		if (debug)						\
-			printk(KERN_DEBUG KBUILD_MODNAME ": " fmt,	\
-				 ## args);				\
+			pr_dbg(KBUILD_MODNAME, ": " fmt, ## args); \
 	} while (0)
 
 
@@ -382,14 +380,14 @@ static int add_to_buf(struct IR *ir)
 			zilog_error("i2c_master_send failed with %d\n",	ret);
 			if (failures >= 3) {
 				mutex_unlock(&ir->ir_lock);
-				zilog_error("unable to read from the IR chip "
-					    "after 3 resets, giving up\n");
+				zilog_error("unable to read from the IR chip
+						after 3 resets, giving up\n");
 				break;
 			}
 
 			/* Looks like the chip crashed, reset it */
-			zilog_error("polling the IR receiver chip failed, "
-				    "trying reset\n");
+			zilog_error("polling the IR receiver chip failed,
+					trying reset\n");
 
 			set_current_state(TASK_UNINTERRUPTIBLE);
 			if (kthread_should_stop()) {
@@ -415,8 +413,8 @@ static int add_to_buf(struct IR *ir)
 		ret = i2c_master_recv(rx->c, keybuf, sizeof(keybuf));
 		mutex_unlock(&ir->ir_lock);
 		if (ret != sizeof(keybuf)) {
-			zilog_error("i2c_master_recv failed with %d -- "
-				    "keeping last read buffer\n", ret);
+			zilog_error("i2c_master_recv failed with %d --
+					keeping last read buffer\n", ret);
 		} else {
 			rx->b[0] = keybuf[3];
 			rx->b[1] = keybuf[4];
@@ -720,8 +718,8 @@ static int send_boot_data(struct IR_tx *tx)
 		zilog_error("unexpected IR TX init response: %02x\n", buf[0]);
 		return 0;
 	}
-	zilog_notify("Zilog/Hauppauge IR blaster firmware version "
-		     "%d.%d.%d loaded\n", buf[1], buf[2], buf[3]);
+	zilog_notify("Zilog/Hauppauge IR blaster firmware version
+			%d.%d.%d loaded\n", buf[1], buf[2], buf[3]);
 
 	return 0;
 }
@@ -803,9 +801,8 @@ static int fw_load(struct IR_tx *tx)
 	if (!read_uint8(&data, tx_data->endp, &version))
 		goto corrupt;
 	if (version != 1) {
-		zilog_error("unsupported code set file version (%u, expected"
-			    "1) -- please upgrade to a newer driver",
-			    version);
+		zilog_error("unsupported code set file version (%u, expected
+			1) -- please upgrade to a newer driver", version);
 		fw_unload_locked();
 		ret = -EFAULT;
 		goto out;
@@ -990,8 +987,8 @@ static int send_code(struct IR_tx *tx, unsigned int code, unsigned int key)
 	ret = get_key_data(data_block, code, key);
 
 	if (ret == -EPROTO) {
-		zilog_error("failed to get data for code %u, key %u -- check "
-			    "lircd.conf entries\n", code, key);
+		zilog_error("failed to get data for code %u, key %u -- check
+				lircd.conf entries\n", code, key);
 		return ret;
 	} else if (ret != 0)
 		return ret;
@@ -1066,12 +1063,12 @@ static int send_code(struct IR_tx *tx, unsigned int code, unsigned int key)
 		ret = i2c_master_send(tx->c, buf, 1);
 		if (ret == 1)
 			break;
-		dprintk("NAK expected: i2c_master_send "
-			"failed with %d (try %d)\n", ret, i+1);
+		dprintk("NAK expected: i2c_master_send
+				failed with %d (try %d)\n", ret, i+1);
 	}
 	if (ret != 1) {
-		zilog_error("IR TX chip never got ready: last i2c_master_send "
-			    "failed with %d\n", ret);
+		zilog_error("IR TX chip never got ready: last i2c_master_send
+				failed with %d\n", ret);
 		return ret < 0 ? ret : -EFAULT;
 	}
 
@@ -1173,12 +1170,12 @@ static ssize_t write(struct file *filep, const char __user *buf, size_t n,
 		 */
 		if (ret != 0) {
 			/* Looks like the chip crashed, reset it */
-			zilog_error("sending to the IR transmitter chip "
-				    "failed, trying reset\n");
+			zilog_error("sending to the IR transmitter chip
+					failed, trying reset\n");
 
 			if (failures >= 3) {
-				zilog_error("unable to send to the IR chip "
-					    "after 3 resets, giving up\n");
+				zilog_error("unable to send to the IR chip
+						after 3 resets, giving up\n");
 				mutex_unlock(&ir->ir_lock);
 				mutex_unlock(&tx->client_lock);
 				put_ir_tx(tx, false);
@@ -1547,8 +1544,8 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id)
 
 		/* Proceed only if the Rx client is also ready or not needed */
 		if (rx == NULL && !tx_only) {
-			zilog_info("probe of IR Tx on %s (i2c-%d) done. Waiting"
-				   " on IR Rx.\n", adap->name, adap->nr);
+			zilog_info("probe of IR Tx on %s (i2c-%d) done. Waiting
+					on IR Rx.\n", adap->name, adap->nr);
 			goto out_ok;
 		}
 	} else {
@@ -1586,8 +1583,8 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id)
 				       "zilog-rx-i2c-%d", adap->nr);
 		if (IS_ERR(rx->task)) {
 			ret = PTR_ERR(rx->task);
-			zilog_error("%s: could not start IR Rx polling thread"
-				    "\n", __func__);
+			zilog_error("%s: could not start IR Rx polling thread
+					\n", __func__);
 			/* Failed kthread, so put back the ir ref */
 			put_ir_device(ir, true);
 			/* Failure exit, so put back rx ref from i2c_client */
@@ -1599,8 +1596,8 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id)
 
 		/* Proceed only if the Tx client is also ready */
 		if (tx == NULL) {
-			zilog_info("probe of IR Rx on %s (i2c-%d) done. Waiting"
-				   " on IR Tx.\n", adap->name, adap->nr);
+			zilog_info("probe of IR Rx on %s (i2c-%d) done. Waiting
+					on IR Tx.\n", adap->name, adap->nr);
 			goto out_ok;
 		}
 	}
@@ -1674,9 +1671,9 @@ module_init(zilog_init);
 module_exit(zilog_exit);
 
 MODULE_DESCRIPTION("Zilog/Hauppauge infrared transmitter driver (i2c stack)");
-MODULE_AUTHOR("Gerd Knorr, Michal Kochanowicz, Christoph Bartelmus, "
-	      "Ulrich Mueller, Stefan Jahn, Jerome Brock, Mark Weaver, "
-	      "Andy Walls");
+MODULE_AUTHOR("Gerd Knorr, Michal Kochanowicz, Christoph Bartelmus,
+		Ulrich Mueller, Stefan Jahn, Jerome Brock, Mark Weaver,
+		Andy Walls");
 MODULE_LICENSE("GPL");
 /* for compat with old name, which isn't all that accurate anymore */
 MODULE_ALIAS("lirc_pvr150");
-- 
2.1.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fixed all coding style issues for drivers/staging/media/lirc/
  2014-10-02  4:40 [PATCH] Fixed all coding style issues for drivers/staging/media/lirc/ Amber Thrall
@ 2014-10-02  4:49 ` Ilia Mirkin
  2014-10-02  6:20 ` Jeremiah Mahler
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Ilia Mirkin @ 2014-10-02  4:49 UTC (permalink / raw)
  To: Amber Thrall; +Cc: jarod, m.chehab, linux-media, devel, linux-kernel

On Thu, Oct 2, 2014 at 12:40 AM, Amber Thrall
<amber.rose.thrall@gmail.com> wrote:
> Fixed various coding style issues, including strings over 80 characters long and many
> deprecated printk's have been replaced with proper methods.
>
> Signed-off-by: Amber Thrall <amber.rose.thrall@gmail.com>
> ---
>  drivers/staging/media/lirc/lirc_bt829.c  |  2 +-
>  drivers/staging/media/lirc/lirc_imon.c   |  4 +-
>  drivers/staging/media/lirc/lirc_sasem.c  |  6 +--
>  drivers/staging/media/lirc/lirc_serial.c | 29 ++++++--------
>  drivers/staging/media/lirc/lirc_sir.c    |  3 +-
>  drivers/staging/media/lirc/lirc_zilog.c  | 69 +++++++++++++++-----------------
>  6 files changed, 52 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/staging/media/lirc/lirc_bt829.c b/drivers/staging/media/lirc/lirc_bt829.c
> index 4c806ba..c70ca68 100644
> --- a/drivers/staging/media/lirc/lirc_bt829.c
> +++ b/drivers/staging/media/lirc/lirc_bt829.c
> @@ -59,7 +59,7 @@ static bool debug;
>  #define dprintk(fmt, args...)                                           \
>         do {                                                             \
>                 if (debug)                                               \
> -                       printk(KERN_DEBUG DRIVER_NAME ": "fmt, ## args); \
> +                       dev_dbg(DRIVER_NAME, ": "fmt, ##args); \
>         } while (0)
>
>  static int atir_minor;
> diff --git a/drivers/staging/media/lirc/lirc_imon.c b/drivers/staging/media/lirc/lirc_imon.c
> index 7aca44f..bce0408 100644
> --- a/drivers/staging/media/lirc/lirc_imon.c
> +++ b/drivers/staging/media/lirc/lirc_imon.c
> @@ -623,8 +623,8 @@ static void imon_incoming_packet(struct imon_context *context,
>         if (debug) {
>                 dev_info(dev, "raw packet: ");
>                 for (i = 0; i < len; ++i)
> -                       printk("%02x ", buf[i]);
> -               printk("\n");
> +                       dev_info(dev, "%02x ", buf[i]);
> +               dev_info(dev, "\n");

Did you mean pr_cont() for both of these? Otherwise each one will get
a level + device info prefix.

>         }
>
>         /*
> diff --git a/drivers/staging/media/lirc/lirc_sasem.c b/drivers/staging/media/lirc/lirc_sasem.c
> index c20ef56..e88e246 100644
> --- a/drivers/staging/media/lirc/lirc_sasem.c
> +++ b/drivers/staging/media/lirc/lirc_sasem.c
> @@ -583,10 +583,10 @@ static void incoming_packet(struct sasem_context *context,
>         }
>
>         if (debug) {
> -               printk(KERN_INFO "Incoming data: ");
> +               pr_info("Incoming data: ");
>                 for (i = 0; i < 8; ++i)
> -                       printk(KERN_CONT "%02x ", buf[i]);
> -               printk(KERN_CONT "\n");
> +                       pr_cont("%02x", buf[i]);
> +               pr_cont("\n");
>         }
>
>         /*
> diff --git a/drivers/staging/media/lirc/lirc_serial.c b/drivers/staging/media/lirc/lirc_serial.c
> index 181b92b..b07671b 100644
> --- a/drivers/staging/media/lirc/lirc_serial.c
> +++ b/drivers/staging/media/lirc/lirc_serial.c
> @@ -116,8 +116,7 @@ static bool txsense;        /* 0 = active high, 1 = active low */
>  #define dprintk(fmt, args...)                                  \
>         do {                                                    \
>                 if (debug)                                      \
> -                       printk(KERN_DEBUG LIRC_DRIVER_NAME ": " \
> -                              fmt, ## args);                   \
> +                       dev_dbg(LIRC_DRIVER_NAME, ": "fmt, ##args); \

This wants a device node, not a driver name string. There's a reason
there was no , in the old macro...

>         } while (0)
>
>  /* forward declarations */
> @@ -356,9 +355,8 @@ static int init_timing_params(unsigned int new_duty_cycle,
>         /* Derive pulse and space from the period */
>         pulse_width = period * duty_cycle / 100;
>         space_width = period - pulse_width;
> -       dprintk("in init_timing_params, freq=%d, duty_cycle=%d, "
> -               "clk/jiffy=%ld, pulse=%ld, space=%ld, "
> -               "conv_us_to_clocks=%ld\n",
> +       dprintk("in init_timing_params, freq=%d, duty_cycle=%d, clk/jiffy=%ld,
> +                       pulse=%ld, space=%ld, conv_us_to_clocks=%ld\n",
>                 freq, duty_cycle, __this_cpu_read(cpu_info.loops_per_jiffy),
>                 pulse_width, space_width, conv_us_to_clocks);
>         return 0;
> @@ -1075,7 +1073,7 @@ static int __init lirc_serial_init(void)
>
>         result = platform_driver_register(&lirc_serial_driver);
>         if (result) {
> -               printk("lirc register returned %d\n", result);
> +               dprintk("lirc register returned %d\n", result);
>                 goto exit_buffer_free;
>         }
>
> @@ -1166,22 +1164,20 @@ module_init(lirc_serial_init_module);
>  module_exit(lirc_serial_exit_module);
>
>  MODULE_DESCRIPTION("Infra-red receiver driver for serial ports.");
> -MODULE_AUTHOR("Ralph Metzler, Trent Piepho, Ben Pfaff, "
> -             "Christoph Bartelmus, Andrei Tanas");
> +MODULE_AUTHOR("Ralph Metzler, Trent Piepho, Ben Pfaff, Christoph Bartelmus, Andrei Tanas");
>  MODULE_LICENSE("GPL");
>
>  module_param(type, int, S_IRUGO);
> -MODULE_PARM_DESC(type, "Hardware type (0 = home-brew, 1 = IRdeo,"
> -                " 2 = IRdeo Remote, 3 = AnimaX, 4 = IgorPlug,"
> -                " 5 = NSLU2 RX:CTS2/TX:GreenLED)");
> +MODULE_PARM_DESC(type, "Hardware type (0 = home-brew, 1 = IRdeo,
> +       2 = IRdeo Remote, 3 = AnimaX, 4 = IgorPlug,
> +       5 = NSLU2 RX:CTS2/TX:GreenLED)");

Does that compile? What was wrong with the old way again?

>
>  module_param(io, int, S_IRUGO);
>  MODULE_PARM_DESC(io, "I/O address base (0x3f8 or 0x2f8)");
>
>  /* some architectures (e.g. intel xscale) have memory mapped registers */
>  module_param(iommap, bool, S_IRUGO);
> -MODULE_PARM_DESC(iommap, "physical base for memory mapped I/O"
> -               " (0 = no memory mapped io)");
> +MODULE_PARM_DESC(iommap, "physical base for memory mapped I/O (0 = no memory mapped io)");
>
>  /*
>   * some architectures (e.g. intel xscale) align the 8bit serial registers
> @@ -1198,13 +1194,12 @@ module_param(share_irq, bool, S_IRUGO);
>  MODULE_PARM_DESC(share_irq, "Share interrupts (0 = off, 1 = on)");
>
>  module_param(sense, int, S_IRUGO);
> -MODULE_PARM_DESC(sense, "Override autodetection of IR receiver circuit"
> -                " (0 = active high, 1 = active low )");
> +MODULE_PARM_DESC(sense, "Override autodetection of IR receiver circuit
> +               (0 = active high, 1 = active low )");
>
>  #ifdef CONFIG_LIRC_SERIAL_TRANSMITTER
>  module_param(txsense, bool, S_IRUGO);
> -MODULE_PARM_DESC(txsense, "Sense of transmitter circuit"
> -                " (0 = active high, 1 = active low )");
> +MODULE_PARM_DESC(txsense, "Sense of transmitter circuit (0 = active high, 1 = active low )");
>  #endif
>
>  module_param(softcarrier, bool, S_IRUGO);
> diff --git a/drivers/staging/media/lirc/lirc_sir.c b/drivers/staging/media/lirc/lirc_sir.c
> index 2ee55ea..cdbb71f 100644
> --- a/drivers/staging/media/lirc/lirc_sir.c
> +++ b/drivers/staging/media/lirc/lirc_sir.c
> @@ -143,8 +143,7 @@ static bool debug;
>  #define dprintk(fmt, args...)                                          \
>         do {                                                            \
>                 if (debug)                                              \
> -                       printk(KERN_DEBUG LIRC_DRIVER_NAME ": "         \
> -                               fmt, ## args);                          \
> +                       dev_dbg(LIRC_DRIVER_NAME, ": "fmt, ## args); \
>         } while (0)
>
>  /* SECTION: Prototypes */
> diff --git a/drivers/staging/media/lirc/lirc_zilog.c b/drivers/staging/media/lirc/lirc_zilog.c
> index 567feba..9c3a3d7 100644
> --- a/drivers/staging/media/lirc/lirc_zilog.c
> +++ b/drivers/staging/media/lirc/lirc_zilog.c
> @@ -152,10 +152,9 @@ struct tx_data_struct {
>  static struct tx_data_struct *tx_data;
>  static struct mutex tx_data_lock;
>
> -#define zilog_notify(s, args...) printk(KERN_NOTICE KBUILD_MODNAME ": " s, \
> -                                       ## args)
> -#define zilog_error(s, args...) printk(KERN_ERR KBUILD_MODNAME ": " s, ## args)
> -#define zilog_info(s, args...) printk(KERN_INFO KBUILD_MODNAME ": " s, ## args)
> +#define zilog_notify(s, args...) dev_notice(KBUILD_MODNAME, ": " s, ## args)
> +#define zilog_error(s, args...) dev_err(KBUILD_MODNAME, ": " s, ## args)
> +#define zilog_info(s, args...) dev_info(KBUILD_MODNAME, ": " s, ## args)

Isn't the first argument for dev_* supposed to be a device? This
should probably be

dev_info(dev, KBUILD_MODNAME ": " s, ##args

just like the old macro had. You're passing the module name as the
device pointer, which should trigger nasty warnings.

[Stopping review here. Please make sure everything compiles and
doesn't introduce horrible-looking warnings]

>
>  /* module parameters */
>  static bool debug;     /* debug output */
> @@ -165,8 +164,7 @@ static int minor = -1;      /* minor number */
>  #define dprintk(fmt, args...)                                          \
>         do {                                                            \
>                 if (debug)                                              \
> -                       printk(KERN_DEBUG KBUILD_MODNAME ": " fmt,      \
> -                                ## args);                              \
> +                       pr_dbg(KBUILD_MODNAME, ": " fmt, ## args); \
>         } while (0)
>
>
> @@ -382,14 +380,14 @@ static int add_to_buf(struct IR *ir)
>                         zilog_error("i2c_master_send failed with %d\n", ret);
>                         if (failures >= 3) {
>                                 mutex_unlock(&ir->ir_lock);
> -                               zilog_error("unable to read from the IR chip "
> -                                           "after 3 resets, giving up\n");
> +                               zilog_error("unable to read from the IR chip
> +                                               after 3 resets, giving up\n");
>                                 break;
>                         }
>
>                         /* Looks like the chip crashed, reset it */
> -                       zilog_error("polling the IR receiver chip failed, "
> -                                   "trying reset\n");
> +                       zilog_error("polling the IR receiver chip failed,
> +                                       trying reset\n");
>
>                         set_current_state(TASK_UNINTERRUPTIBLE);
>                         if (kthread_should_stop()) {
> @@ -415,8 +413,8 @@ static int add_to_buf(struct IR *ir)
>                 ret = i2c_master_recv(rx->c, keybuf, sizeof(keybuf));
>                 mutex_unlock(&ir->ir_lock);
>                 if (ret != sizeof(keybuf)) {
> -                       zilog_error("i2c_master_recv failed with %d -- "
> -                                   "keeping last read buffer\n", ret);
> +                       zilog_error("i2c_master_recv failed with %d --
> +                                       keeping last read buffer\n", ret);
>                 } else {
>                         rx->b[0] = keybuf[3];
>                         rx->b[1] = keybuf[4];
> @@ -720,8 +718,8 @@ static int send_boot_data(struct IR_tx *tx)
>                 zilog_error("unexpected IR TX init response: %02x\n", buf[0]);
>                 return 0;
>         }
> -       zilog_notify("Zilog/Hauppauge IR blaster firmware version "
> -                    "%d.%d.%d loaded\n", buf[1], buf[2], buf[3]);
> +       zilog_notify("Zilog/Hauppauge IR blaster firmware version
> +                       %d.%d.%d loaded\n", buf[1], buf[2], buf[3]);
>
>         return 0;
>  }
> @@ -803,9 +801,8 @@ static int fw_load(struct IR_tx *tx)
>         if (!read_uint8(&data, tx_data->endp, &version))
>                 goto corrupt;
>         if (version != 1) {
> -               zilog_error("unsupported code set file version (%u, expected"
> -                           "1) -- please upgrade to a newer driver",
> -                           version);
> +               zilog_error("unsupported code set file version (%u, expected
> +                       1) -- please upgrade to a newer driver", version);
>                 fw_unload_locked();
>                 ret = -EFAULT;
>                 goto out;
> @@ -990,8 +987,8 @@ static int send_code(struct IR_tx *tx, unsigned int code, unsigned int key)
>         ret = get_key_data(data_block, code, key);
>
>         if (ret == -EPROTO) {
> -               zilog_error("failed to get data for code %u, key %u -- check "
> -                           "lircd.conf entries\n", code, key);
> +               zilog_error("failed to get data for code %u, key %u -- check
> +                               lircd.conf entries\n", code, key);
>                 return ret;
>         } else if (ret != 0)
>                 return ret;
> @@ -1066,12 +1063,12 @@ static int send_code(struct IR_tx *tx, unsigned int code, unsigned int key)
>                 ret = i2c_master_send(tx->c, buf, 1);
>                 if (ret == 1)
>                         break;
> -               dprintk("NAK expected: i2c_master_send "
> -                       "failed with %d (try %d)\n", ret, i+1);
> +               dprintk("NAK expected: i2c_master_send
> +                               failed with %d (try %d)\n", ret, i+1);
>         }
>         if (ret != 1) {
> -               zilog_error("IR TX chip never got ready: last i2c_master_send "
> -                           "failed with %d\n", ret);
> +               zilog_error("IR TX chip never got ready: last i2c_master_send
> +                               failed with %d\n", ret);
>                 return ret < 0 ? ret : -EFAULT;
>         }
>
> @@ -1173,12 +1170,12 @@ static ssize_t write(struct file *filep, const char __user *buf, size_t n,
>                  */
>                 if (ret != 0) {
>                         /* Looks like the chip crashed, reset it */
> -                       zilog_error("sending to the IR transmitter chip "
> -                                   "failed, trying reset\n");
> +                       zilog_error("sending to the IR transmitter chip
> +                                       failed, trying reset\n");
>
>                         if (failures >= 3) {
> -                               zilog_error("unable to send to the IR chip "
> -                                           "after 3 resets, giving up\n");
> +                               zilog_error("unable to send to the IR chip
> +                                               after 3 resets, giving up\n");
>                                 mutex_unlock(&ir->ir_lock);
>                                 mutex_unlock(&tx->client_lock);
>                                 put_ir_tx(tx, false);
> @@ -1547,8 +1544,8 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id)
>
>                 /* Proceed only if the Rx client is also ready or not needed */
>                 if (rx == NULL && !tx_only) {
> -                       zilog_info("probe of IR Tx on %s (i2c-%d) done. Waiting"
> -                                  " on IR Rx.\n", adap->name, adap->nr);
> +                       zilog_info("probe of IR Tx on %s (i2c-%d) done. Waiting
> +                                       on IR Rx.\n", adap->name, adap->nr);
>                         goto out_ok;
>                 }
>         } else {
> @@ -1586,8 +1583,8 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id)
>                                        "zilog-rx-i2c-%d", adap->nr);
>                 if (IS_ERR(rx->task)) {
>                         ret = PTR_ERR(rx->task);
> -                       zilog_error("%s: could not start IR Rx polling thread"
> -                                   "\n", __func__);
> +                       zilog_error("%s: could not start IR Rx polling thread
> +                                       \n", __func__);
>                         /* Failed kthread, so put back the ir ref */
>                         put_ir_device(ir, true);
>                         /* Failure exit, so put back rx ref from i2c_client */
> @@ -1599,8 +1596,8 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id)
>
>                 /* Proceed only if the Tx client is also ready */
>                 if (tx == NULL) {
> -                       zilog_info("probe of IR Rx on %s (i2c-%d) done. Waiting"
> -                                  " on IR Tx.\n", adap->name, adap->nr);
> +                       zilog_info("probe of IR Rx on %s (i2c-%d) done. Waiting
> +                                       on IR Tx.\n", adap->name, adap->nr);
>                         goto out_ok;
>                 }
>         }
> @@ -1674,9 +1671,9 @@ module_init(zilog_init);
>  module_exit(zilog_exit);
>
>  MODULE_DESCRIPTION("Zilog/Hauppauge infrared transmitter driver (i2c stack)");
> -MODULE_AUTHOR("Gerd Knorr, Michal Kochanowicz, Christoph Bartelmus, "
> -             "Ulrich Mueller, Stefan Jahn, Jerome Brock, Mark Weaver, "
> -             "Andy Walls");
> +MODULE_AUTHOR("Gerd Knorr, Michal Kochanowicz, Christoph Bartelmus,
> +               Ulrich Mueller, Stefan Jahn, Jerome Brock, Mark Weaver,
> +               Andy Walls");
>  MODULE_LICENSE("GPL");
>  /* for compat with old name, which isn't all that accurate anymore */
>  MODULE_ALIAS("lirc_pvr150");
> --
> 2.1.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fixed all coding style issues for drivers/staging/media/lirc/
  2014-10-02  4:40 [PATCH] Fixed all coding style issues for drivers/staging/media/lirc/ Amber Thrall
  2014-10-02  4:49 ` Ilia Mirkin
@ 2014-10-02  6:20 ` Jeremiah Mahler
  2014-10-02 12:45 ` Greg KH
  2014-10-02 13:29 ` Mauro Carvalho Chehab
  3 siblings, 0 replies; 11+ messages in thread
From: Jeremiah Mahler @ 2014-10-02  6:20 UTC (permalink / raw)
  To: Amber Thrall; +Cc: linux-kernel

Amber,

On Wed, Oct 01, 2014 at 09:40:02PM -0700, Amber Thrall wrote:
> Fixed various coding style issues, including strings over 80 characters long and many 

Log messages are easier to read if they are less than 80 characters wide too.

I like to use a width of 72 characters.  It allows enough room for a few
replies without wrapping.

> deprecated printk's have been replaced with proper methods.
> 
> Signed-off-by: Amber Thrall <amber.rose.thrall@gmail.com>
> ---
...

-- 
Jeremiah Mahler
jmmahler@gmail.com
http://github.com/jmahler

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fixed all coding style issues for drivers/staging/media/lirc/
  2014-10-02  4:40 [PATCH] Fixed all coding style issues for drivers/staging/media/lirc/ Amber Thrall
  2014-10-02  4:49 ` Ilia Mirkin
  2014-10-02  6:20 ` Jeremiah Mahler
@ 2014-10-02 12:45 ` Greg KH
  2014-10-02 13:29 ` Mauro Carvalho Chehab
  3 siblings, 0 replies; 11+ messages in thread
From: Greg KH @ 2014-10-02 12:45 UTC (permalink / raw)
  To: Amber Thrall; +Cc: jarod, m.chehab, devel, linux-kernel, linux-media

On Wed, Oct 01, 2014 at 09:40:02PM -0700, Amber Thrall wrote:
> Fixed various coding style issues, including strings over 80 characters long and many 
> deprecated printk's have been replaced with proper methods.

Only do one thing per patch, you are doing lots of different things
here, and as Dan pointed out, you broke the build.

greg k-h

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fixed all coding style issues for drivers/staging/media/lirc/
  2014-10-02  4:40 [PATCH] Fixed all coding style issues for drivers/staging/media/lirc/ Amber Thrall
                   ` (2 preceding siblings ...)
  2014-10-02 12:45 ` Greg KH
@ 2014-10-02 13:29 ` Mauro Carvalho Chehab
  2014-10-02 16:45   ` Joe Perches
  3 siblings, 1 reply; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2014-10-02 13:29 UTC (permalink / raw)
  To: Amber Thrall; +Cc: jarod, linux-media, devel, linux-kernel

Em Wed, 01 Oct 2014 21:40:02 -0700
Amber Thrall <amber.rose.thrall@gmail.com> escreveu:

> Fixed various coding style issues, including strings over 80 characters long and many 
> deprecated printk's have been replaced with proper methods.
> 
> Signed-off-by: Amber Thrall <amber.rose.thrall@gmail.com>
> ---
>  drivers/staging/media/lirc/lirc_bt829.c  |  2 +-
>  drivers/staging/media/lirc/lirc_imon.c   |  4 +-
>  drivers/staging/media/lirc/lirc_sasem.c  |  6 +--
>  drivers/staging/media/lirc/lirc_serial.c | 29 ++++++--------
>  drivers/staging/media/lirc/lirc_sir.c    |  3 +-
>  drivers/staging/media/lirc/lirc_zilog.c  | 69 +++++++++++++++-----------------
>  6 files changed, 52 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/staging/media/lirc/lirc_bt829.c b/drivers/staging/media/lirc/lirc_bt829.c
> index 4c806ba..c70ca68 100644
> --- a/drivers/staging/media/lirc/lirc_bt829.c
> +++ b/drivers/staging/media/lirc/lirc_bt829.c
> @@ -59,7 +59,7 @@ static bool debug;
>  #define dprintk(fmt, args...)						 \
>  	do {								 \
>  		if (debug)						 \
> -			printk(KERN_DEBUG DRIVER_NAME ": "fmt, ## args); \
> +			dev_dbg(DRIVER_NAME, ": "fmt, ##args); \

Please don't do that. If dynamic printk is enabled, that would mean that
someone needing to use the debug prints would need to both load the module
with debug=1 and enable the debug prints via sysfs.

In this specific case, probably the best would be to just replace dprintk
for dev_dbg or pr_dbg and remove the macro.

Also, please notice that dev_dbg() is defined as:
	#define dev_dbg(dev, format, arg...) 

So, the dev argument should be passed to it. You'll need to be sure that
you have dev on all places and that it was already initialized on all
places you use it, or otherwise you'll break compilation (or runtime, if
you use an initialized dev).

>  	} while (0)
>  
>  static int atir_minor;
> diff --git a/drivers/staging/media/lirc/lirc_imon.c b/drivers/staging/media/lirc/lirc_imon.c
> index 7aca44f..bce0408 100644
> --- a/drivers/staging/media/lirc/lirc_imon.c
> +++ b/drivers/staging/media/lirc/lirc_imon.c
> @@ -623,8 +623,8 @@ static void imon_incoming_packet(struct imon_context *context,
>  	if (debug) {
>  		dev_info(dev, "raw packet: ");
>  		for (i = 0; i < len; ++i)
> -			printk("%02x ", buf[i]);
> -		printk("\n");
> +			dev_info(dev, "%02x ", buf[i]);
> +		dev_info(dev, "\n");
>  	}

This is wrong, as the second printk should be printk_cont.

The best here would be to change all above to use %*ph.
So, just:

	dev_debug(dev, "raw packet: %*ph\n", len, buf);

>  
>  	/*
> diff --git a/drivers/staging/media/lirc/lirc_sasem.c b/drivers/staging/media/lirc/lirc_sasem.c
> index c20ef56..e88e246 100644
> --- a/drivers/staging/media/lirc/lirc_sasem.c
> +++ b/drivers/staging/media/lirc/lirc_sasem.c
> @@ -583,10 +583,10 @@ static void incoming_packet(struct sasem_context *context,
>  	}
>  
>  	if (debug) {
> -		printk(KERN_INFO "Incoming data: ");
> +		pr_info("Incoming data: ");
>  		for (i = 0; i < 8; ++i)
> -			printk(KERN_CONT "%02x ", buf[i]);
> -		printk(KERN_CONT "\n");
> +			pr_cont("%02x", buf[i]);
> +		pr_cont("\n");
>  	}

Same as above: use %*ph.

>  
>  	/*
> diff --git a/drivers/staging/media/lirc/lirc_serial.c b/drivers/staging/media/lirc/lirc_serial.c
> index 181b92b..b07671b 100644
> --- a/drivers/staging/media/lirc/lirc_serial.c
> +++ b/drivers/staging/media/lirc/lirc_serial.c
> @@ -116,8 +116,7 @@ static bool txsense;	/* 0 = active high, 1 = active low */
>  #define dprintk(fmt, args...)					\
>  	do {							\
>  		if (debug)					\
> -			printk(KERN_DEBUG LIRC_DRIVER_NAME ": "	\
> -			       fmt, ## args);			\
> +			dev_dbg(LIRC_DRIVER_NAME, ": "fmt, ##args); \
>  	} while (0)

Again, see my comment #1.

>  
>  /* forward declarations */
> @@ -356,9 +355,8 @@ static int init_timing_params(unsigned int new_duty_cycle,
>  	/* Derive pulse and space from the period */
>  	pulse_width = period * duty_cycle / 100;
>  	space_width = period - pulse_width;
> -	dprintk("in init_timing_params, freq=%d, duty_cycle=%d, "
> -		"clk/jiffy=%ld, pulse=%ld, space=%ld, "
> -		"conv_us_to_clocks=%ld\n",
> +	dprintk("in init_timing_params, freq=%d, duty_cycle=%d, clk/jiffy=%ld,
> +			pulse=%ld, space=%ld, conv_us_to_clocks=%ld\n",
>  		freq, duty_cycle, __this_cpu_read(cpu_info.loops_per_jiffy),
>  		pulse_width, space_width, conv_us_to_clocks);
>  	return 0;
> @@ -1075,7 +1073,7 @@ static int __init lirc_serial_init(void)
>  
>  	result = platform_driver_register(&lirc_serial_driver);
>  	if (result) {
> -		printk("lirc register returned %d\n", result);
> +		dprintk("lirc register returned %d\n", result);
>  		goto exit_buffer_free;
>  	}
>  
> @@ -1166,22 +1164,20 @@ module_init(lirc_serial_init_module);
>  module_exit(lirc_serial_exit_module);
>  
>  MODULE_DESCRIPTION("Infra-red receiver driver for serial ports.");
> -MODULE_AUTHOR("Ralph Metzler, Trent Piepho, Ben Pfaff, "
> -	      "Christoph Bartelmus, Andrei Tanas");
> +MODULE_AUTHOR("Ralph Metzler, Trent Piepho, Ben Pfaff, Christoph Bartelmus, Andrei Tanas");
>  MODULE_LICENSE("GPL");
>  
>  module_param(type, int, S_IRUGO);
> -MODULE_PARM_DESC(type, "Hardware type (0 = home-brew, 1 = IRdeo,"
> -		 " 2 = IRdeo Remote, 3 = AnimaX, 4 = IgorPlug,"
> -		 " 5 = NSLU2 RX:CTS2/TX:GreenLED)");
> +MODULE_PARM_DESC(type, "Hardware type (0 = home-brew, 1 = IRdeo,
> +	2 = IRdeo Remote, 3 = AnimaX, 4 = IgorPlug,
> +	5 = NSLU2 RX:CTS2/TX:GreenLED)");

No, please don't do that. The previous code is better to read than
the second one. Also, you're adding extra \n there.

>  
>  module_param(io, int, S_IRUGO);
>  MODULE_PARM_DESC(io, "I/O address base (0x3f8 or 0x2f8)");
>  
>  /* some architectures (e.g. intel xscale) have memory mapped registers */
>  module_param(iommap, bool, S_IRUGO);
> -MODULE_PARM_DESC(iommap, "physical base for memory mapped I/O"
> -		" (0 = no memory mapped io)");
> +MODULE_PARM_DESC(iommap, "physical base for memory mapped I/O (0 = no memory mapped io)");
>  
>  /*
>   * some architectures (e.g. intel xscale) align the 8bit serial registers
> @@ -1198,13 +1194,12 @@ module_param(share_irq, bool, S_IRUGO);
>  MODULE_PARM_DESC(share_irq, "Share interrupts (0 = off, 1 = on)");
>  
>  module_param(sense, int, S_IRUGO);
> -MODULE_PARM_DESC(sense, "Override autodetection of IR receiver circuit"
> -		 " (0 = active high, 1 = active low )");
> +MODULE_PARM_DESC(sense, "Override autodetection of IR receiver circuit
> +		(0 = active high, 1 = active low )");

Same as my above comment.

>  #ifdef CONFIG_LIRC_SERIAL_TRANSMITTER
>  module_param(txsense, bool, S_IRUGO);
> -MODULE_PARM_DESC(txsense, "Sense of transmitter circuit"
> -		 " (0 = active high, 1 = active low )");
> +MODULE_PARM_DESC(txsense, "Sense of transmitter circuit (0 = active high, 1 = active low )");
>  #endif
>  
>  module_param(softcarrier, bool, S_IRUGO);
> diff --git a/drivers/staging/media/lirc/lirc_sir.c b/drivers/staging/media/lirc/lirc_sir.c
> index 2ee55ea..cdbb71f 100644
> --- a/drivers/staging/media/lirc/lirc_sir.c
> +++ b/drivers/staging/media/lirc/lirc_sir.c
> @@ -143,8 +143,7 @@ static bool debug;
>  #define dprintk(fmt, args...)						\
>  	do {								\
>  		if (debug)						\
> -			printk(KERN_DEBUG LIRC_DRIVER_NAME ": "		\
> -				fmt, ## args);				\
> +			dev_dbg(LIRC_DRIVER_NAME, ": "fmt, ## args); \
>  	} while (0)

See my comment #1. Well, I'm repeat myself too many times.. You got it.

The same issues I pointed above also applies to the code below.

Regards,
Mauro

>  
>  /* SECTION: Prototypes */
> diff --git a/drivers/staging/media/lirc/lirc_zilog.c b/drivers/staging/media/lirc/lirc_zilog.c
> index 567feba..9c3a3d7 100644
> --- a/drivers/staging/media/lirc/lirc_zilog.c
> +++ b/drivers/staging/media/lirc/lirc_zilog.c
> @@ -152,10 +152,9 @@ struct tx_data_struct {
>  static struct tx_data_struct *tx_data;
>  static struct mutex tx_data_lock;
>  
> -#define zilog_notify(s, args...) printk(KERN_NOTICE KBUILD_MODNAME ": " s, \
> -					## args)
> -#define zilog_error(s, args...) printk(KERN_ERR KBUILD_MODNAME ": " s, ## args)
> -#define zilog_info(s, args...) printk(KERN_INFO KBUILD_MODNAME ": " s, ## args)
> +#define zilog_notify(s, args...) dev_notice(KBUILD_MODNAME, ": " s, ## args)
> +#define zilog_error(s, args...) dev_err(KBUILD_MODNAME, ": " s, ## args)
> +#define zilog_info(s, args...) dev_info(KBUILD_MODNAME, ": " s, ## args)
>  
>  /* module parameters */
>  static bool debug;	/* debug output */
> @@ -165,8 +164,7 @@ static int minor = -1;	/* minor number */
>  #define dprintk(fmt, args...)						\
>  	do {								\
>  		if (debug)						\
> -			printk(KERN_DEBUG KBUILD_MODNAME ": " fmt,	\
> -				 ## args);				\
> +			pr_dbg(KBUILD_MODNAME, ": " fmt, ## args); \
>  	} while (0)
>  
>  
> @@ -382,14 +380,14 @@ static int add_to_buf(struct IR *ir)
>  			zilog_error("i2c_master_send failed with %d\n",	ret);
>  			if (failures >= 3) {
>  				mutex_unlock(&ir->ir_lock);
> -				zilog_error("unable to read from the IR chip "
> -					    "after 3 resets, giving up\n");
> +				zilog_error("unable to read from the IR chip
> +						after 3 resets, giving up\n");
>  				break;
>  			}
>  
>  			/* Looks like the chip crashed, reset it */
> -			zilog_error("polling the IR receiver chip failed, "
> -				    "trying reset\n");
> +			zilog_error("polling the IR receiver chip failed,
> +					trying reset\n");
>  
>  			set_current_state(TASK_UNINTERRUPTIBLE);
>  			if (kthread_should_stop()) {
> @@ -415,8 +413,8 @@ static int add_to_buf(struct IR *ir)
>  		ret = i2c_master_recv(rx->c, keybuf, sizeof(keybuf));
>  		mutex_unlock(&ir->ir_lock);
>  		if (ret != sizeof(keybuf)) {
> -			zilog_error("i2c_master_recv failed with %d -- "
> -				    "keeping last read buffer\n", ret);
> +			zilog_error("i2c_master_recv failed with %d --
> +					keeping last read buffer\n", ret);
>  		} else {
>  			rx->b[0] = keybuf[3];
>  			rx->b[1] = keybuf[4];
> @@ -720,8 +718,8 @@ static int send_boot_data(struct IR_tx *tx)
>  		zilog_error("unexpected IR TX init response: %02x\n", buf[0]);
>  		return 0;
>  	}
> -	zilog_notify("Zilog/Hauppauge IR blaster firmware version "
> -		     "%d.%d.%d loaded\n", buf[1], buf[2], buf[3]);
> +	zilog_notify("Zilog/Hauppauge IR blaster firmware version
> +			%d.%d.%d loaded\n", buf[1], buf[2], buf[3]);
>  
>  	return 0;
>  }
> @@ -803,9 +801,8 @@ static int fw_load(struct IR_tx *tx)
>  	if (!read_uint8(&data, tx_data->endp, &version))
>  		goto corrupt;
>  	if (version != 1) {
> -		zilog_error("unsupported code set file version (%u, expected"
> -			    "1) -- please upgrade to a newer driver",
> -			    version);
> +		zilog_error("unsupported code set file version (%u, expected
> +			1) -- please upgrade to a newer driver", version);
>  		fw_unload_locked();
>  		ret = -EFAULT;
>  		goto out;
> @@ -990,8 +987,8 @@ static int send_code(struct IR_tx *tx, unsigned int code, unsigned int key)
>  	ret = get_key_data(data_block, code, key);
>  
>  	if (ret == -EPROTO) {
> -		zilog_error("failed to get data for code %u, key %u -- check "
> -			    "lircd.conf entries\n", code, key);
> +		zilog_error("failed to get data for code %u, key %u -- check
> +				lircd.conf entries\n", code, key);
>  		return ret;
>  	} else if (ret != 0)
>  		return ret;
> @@ -1066,12 +1063,12 @@ static int send_code(struct IR_tx *tx, unsigned int code, unsigned int key)
>  		ret = i2c_master_send(tx->c, buf, 1);
>  		if (ret == 1)
>  			break;
> -		dprintk("NAK expected: i2c_master_send "
> -			"failed with %d (try %d)\n", ret, i+1);
> +		dprintk("NAK expected: i2c_master_send
> +				failed with %d (try %d)\n", ret, i+1);
>  	}
>  	if (ret != 1) {
> -		zilog_error("IR TX chip never got ready: last i2c_master_send "
> -			    "failed with %d\n", ret);
> +		zilog_error("IR TX chip never got ready: last i2c_master_send
> +				failed with %d\n", ret);
>  		return ret < 0 ? ret : -EFAULT;
>  	}
>  
> @@ -1173,12 +1170,12 @@ static ssize_t write(struct file *filep, const char __user *buf, size_t n,
>  		 */
>  		if (ret != 0) {
>  			/* Looks like the chip crashed, reset it */
> -			zilog_error("sending to the IR transmitter chip "
> -				    "failed, trying reset\n");
> +			zilog_error("sending to the IR transmitter chip
> +					failed, trying reset\n");
>  
>  			if (failures >= 3) {
> -				zilog_error("unable to send to the IR chip "
> -					    "after 3 resets, giving up\n");
> +				zilog_error("unable to send to the IR chip
> +						after 3 resets, giving up\n");
>  				mutex_unlock(&ir->ir_lock);
>  				mutex_unlock(&tx->client_lock);
>  				put_ir_tx(tx, false);
> @@ -1547,8 +1544,8 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  
>  		/* Proceed only if the Rx client is also ready or not needed */
>  		if (rx == NULL && !tx_only) {
> -			zilog_info("probe of IR Tx on %s (i2c-%d) done. Waiting"
> -				   " on IR Rx.\n", adap->name, adap->nr);
> +			zilog_info("probe of IR Tx on %s (i2c-%d) done. Waiting
> +					on IR Rx.\n", adap->name, adap->nr);
>  			goto out_ok;
>  		}
>  	} else {
> @@ -1586,8 +1583,8 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  				       "zilog-rx-i2c-%d", adap->nr);
>  		if (IS_ERR(rx->task)) {
>  			ret = PTR_ERR(rx->task);
> -			zilog_error("%s: could not start IR Rx polling thread"
> -				    "\n", __func__);
> +			zilog_error("%s: could not start IR Rx polling thread
> +					\n", __func__);
>  			/* Failed kthread, so put back the ir ref */
>  			put_ir_device(ir, true);
>  			/* Failure exit, so put back rx ref from i2c_client */
> @@ -1599,8 +1596,8 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  
>  		/* Proceed only if the Tx client is also ready */
>  		if (tx == NULL) {
> -			zilog_info("probe of IR Rx on %s (i2c-%d) done. Waiting"
> -				   " on IR Tx.\n", adap->name, adap->nr);
> +			zilog_info("probe of IR Rx on %s (i2c-%d) done. Waiting
> +					on IR Tx.\n", adap->name, adap->nr);
>  			goto out_ok;
>  		}
>  	}
> @@ -1674,9 +1671,9 @@ module_init(zilog_init);
>  module_exit(zilog_exit);
>  
>  MODULE_DESCRIPTION("Zilog/Hauppauge infrared transmitter driver (i2c stack)");
> -MODULE_AUTHOR("Gerd Knorr, Michal Kochanowicz, Christoph Bartelmus, "
> -	      "Ulrich Mueller, Stefan Jahn, Jerome Brock, Mark Weaver, "
> -	      "Andy Walls");
> +MODULE_AUTHOR("Gerd Knorr, Michal Kochanowicz, Christoph Bartelmus,
> +		Ulrich Mueller, Stefan Jahn, Jerome Brock, Mark Weaver,
> +		Andy Walls");
>  MODULE_LICENSE("GPL");
>  /* for compat with old name, which isn't all that accurate anymore */
>  MODULE_ALIAS("lirc_pvr150");

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fixed all coding style issues for drivers/staging/media/lirc/
  2014-10-02 13:29 ` Mauro Carvalho Chehab
@ 2014-10-02 16:45   ` Joe Perches
  2014-10-02 23:58     ` Antti Palosaari
  0 siblings, 1 reply; 11+ messages in thread
From: Joe Perches @ 2014-10-02 16:45 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Amber Thrall, jarod, linux-media, devel, linux-kernel

On Thu, 2014-10-02 at 10:29 -0300, Mauro Carvalho Chehab wrote:
> Em Wed, 01 Oct 2014 21:40:02 -0700 Amber Thrall <amber.rose.thrall@gmail.com> escreveu:
> > Fixed various coding style issues, including strings over 80 characters long and many 
> > deprecated printk's have been replaced with proper methods.
[]
> > diff --git a/drivers/staging/media/lirc/lirc_imon.c b/drivers/staging/media/lirc/lirc_imon.c
[]
> > @@ -623,8 +623,8 @@ static void imon_incoming_packet(struct imon_context *context,
> >  	if (debug) {
> >  		dev_info(dev, "raw packet: ");
> >  		for (i = 0; i < len; ++i)
> > -			printk("%02x ", buf[i]);
> > -		printk("\n");
> > +			dev_info(dev, "%02x ", buf[i]);
> > +		dev_info(dev, "\n");
> >  	}
> 
> This is wrong, as the second printk should be printk_cont.
> 
> The best here would be to change all above to use %*ph.
> So, just:
> 
> 	dev_debug(dev, "raw packet: %*ph\n", len, buf);

Not quite.

%*ph is length limited and only useful for lengths < 30 or so.
Even then, it's pretty ugly.

print_hex_dump_debug() is generally better.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fixed all coding style issues for drivers/staging/media/lirc/
  2014-10-02 16:45   ` Joe Perches
@ 2014-10-02 23:58     ` Antti Palosaari
  2014-10-03  1:41       ` Joe Perches
  0 siblings, 1 reply; 11+ messages in thread
From: Antti Palosaari @ 2014-10-02 23:58 UTC (permalink / raw)
  To: Joe Perches, Mauro Carvalho Chehab
  Cc: Amber Thrall, jarod, linux-media, devel, linux-kernel



On 10/02/2014 07:45 PM, Joe Perches wrote:
> On Thu, 2014-10-02 at 10:29 -0300, Mauro Carvalho Chehab wrote:
>> Em Wed, 01 Oct 2014 21:40:02 -0700 Amber Thrall <amber.rose.thrall@gmail.com> escreveu:
>>> Fixed various coding style issues, including strings over 80 characters long and many
>>> deprecated printk's have been replaced with proper methods.
> []
>>> diff --git a/drivers/staging/media/lirc/lirc_imon.c b/drivers/staging/media/lirc/lirc_imon.c
> []
>>> @@ -623,8 +623,8 @@ static void imon_incoming_packet(struct imon_context *context,
>>>   	if (debug) {
>>>   		dev_info(dev, "raw packet: ");
>>>   		for (i = 0; i < len; ++i)
>>> -			printk("%02x ", buf[i]);
>>> -		printk("\n");
>>> +			dev_info(dev, "%02x ", buf[i]);
>>> +		dev_info(dev, "\n");
>>>   	}
>>
>> This is wrong, as the second printk should be printk_cont.
>>
>> The best here would be to change all above to use %*ph.
>> So, just:
>>
>> 	dev_debug(dev, "raw packet: %*ph\n", len, buf);
>
> Not quite.
>
> %*ph is length limited and only useful for lengths < 30 or so.
> Even then, it's pretty ugly.
>
> print_hex_dump_debug() is generally better.

That is place where you print 8 debug bytes, which are received remote 
controller code. IMHO %*ph format is just what you like in that case.

Why print_hex_dump_debug() is better? IIRC it could not be even 
controlled like those dynamic debug printings.

regards
Antti

-- 
http://palosaari.fi/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fixed all coding style issues for drivers/staging/media/lirc/
  2014-10-02 23:58     ` Antti Palosaari
@ 2014-10-03  1:41       ` Joe Perches
  0 siblings, 0 replies; 11+ messages in thread
From: Joe Perches @ 2014-10-03  1:41 UTC (permalink / raw)
  To: Antti Palosaari
  Cc: Mauro Carvalho Chehab, Amber Thrall, jarod, linux-media, devel,
	linux-kernel

On Fri, 2014-10-03 at 02:58 +0300, Antti Palosaari wrote:
> On 10/02/2014 07:45 PM, Joe Perches wrote:
> > On Thu, 2014-10-02 at 10:29 -0300, Mauro Carvalho Chehab wrote:
> >> Em Wed, 01 Oct 2014 21:40:02 -0700 Amber Thrall <amber.rose.thrall@gmail.com> escreveu:
> >>> Fixed various coding style issues, including strings over 80 characters long and many
> >>> deprecated printk's have been replaced with proper methods.
> > []
> >>> diff --git a/drivers/staging/media/lirc/lirc_imon.c b/drivers/staging/media/lirc/lirc_imon.c
> > []
> >>> @@ -623,8 +623,8 @@ static void imon_incoming_packet(struct imon_context *context,
> >>>   	if (debug) {
> >>>   		dev_info(dev, "raw packet: ");
> >>>   		for (i = 0; i < len; ++i)
> >>> -			printk("%02x ", buf[i]);
> >>> -		printk("\n");
> >>> +			dev_info(dev, "%02x ", buf[i]);
> >>> +		dev_info(dev, "\n");
> >>>   	}
> >>
> >> This is wrong, as the second printk should be printk_cont.
> >>
> >> The best here would be to change all above to use %*ph.
> >> So, just:
> >>
> >> 	dev_debug(dev, "raw packet: %*ph\n", len, buf);
> >
> > Not quite.
> >
> > %*ph is length limited and only useful for lengths < 30 or so.
> > Even then, it's pretty ugly.
> >
> > print_hex_dump_debug() is generally better.
> 
> That is place where you print 8 debug bytes, which are received remote 
> controller code. IMHO %*ph format is just what you like in that case.

Hi Antti.

I stand by my statement as I only looked at the
patch snippet itself, not any function real code.

In the actual code, there's a test above it:

	if (len != 8) {
		dev_warn(dev, "imon %s: invalid incoming packet size (len = %d, intf%d)\n",
			__func__, len, intf);
		return;
	}

So in my opinion, this would be better/smaller as:

	dev_dbg(dev, "raw packet: %8ph\n", urb->transfer_buffer);

> Why print_hex_dump_debug() is better? IIRC it could not be even 
> controlled like those dynamic debug printings.

Nope, it is. (from printk.h)

#if defined(CONFIG_DYNAMIC_DEBUG)
#define print_hex_dump_debug(prefix_str, prefix_type, rowsize,	\
			     groupsize, buf, len, ascii)	\
	dynamic_hex_dump(prefix_str, prefix_type, rowsize,	\
			 groupsize, buf, len, ascii)
#else
#define print_hex_dump_debug(prefix_str, prefix_type, rowsize,		\
			     groupsize, buf, len, ascii)		\
	print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, rowsize,	\
		       groupsize, buf, len, ascii)
#endif /* defined(CONFIG_DYNAMIC_DEBUG) */

It prints multiple lines when the length is > 16.
It prints the ascii along with the hex if desired.

cheers, Joe


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fixed all coding style issues for drivers/staging/media/lirc/
  2014-10-02  2:35 Amber Thrall
  2014-10-02  2:46 ` Greg KH
@ 2014-10-02  8:13 ` Dan Carpenter
  1 sibling, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2014-10-02  8:13 UTC (permalink / raw)
  To: Amber Thrall; +Cc: greg, jarod, devel, linux-kernel, linux-media

On Wed, Oct 01, 2014 at 07:35:51PM -0700, Amber Thrall wrote:
> Fixed various coding sytles.
> 

Fix one type of thing at a time.

> Signed-off-by: Amber Thrall <amber.rose.thrall@gmail.com>
> ---
>  drivers/staging/media/lirc/lirc_bt829.c  |  2 +-
>  drivers/staging/media/lirc/lirc_imon.c   |  4 +-
>  drivers/staging/media/lirc/lirc_sasem.c  |  6 +--
>  drivers/staging/media/lirc/lirc_serial.c | 29 ++++++--------
>  drivers/staging/media/lirc/lirc_sir.c    |  3 +-
>  drivers/staging/media/lirc/lirc_zilog.c  | 69 +++++++++++++++-----------------
>  6 files changed, 52 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/staging/media/lirc/lirc_bt829.c b/drivers/staging/media/lirc/lirc_bt829.c
> index 4c806ba..c70ca68 100644
> --- a/drivers/staging/media/lirc/lirc_bt829.c
> +++ b/drivers/staging/media/lirc/lirc_bt829.c
> @@ -59,7 +59,7 @@ static bool debug;
>  #define dprintk(fmt, args...)						 \
>  	do {								 \
>  		if (debug)						 \
> -			printk(KERN_DEBUG DRIVER_NAME ": "fmt, ## args); \
> +			dev_dbg(DRIVER_NAME, ": "fmt, ##args); \

I think we need to pass a dev pointer to the dev_dbg() functions.  Does
this even compile?  I can't test it myself at this minute.

My guess is the reason this compiles is because dprintk() is #ifdefed
out of the actual code.

>  	} while (0)
>  
>  static int atir_minor;
> diff --git a/drivers/staging/media/lirc/lirc_imon.c b/drivers/staging/media/lirc/lirc_imon.c
> index 7aca44f..bce0408 100644
> --- a/drivers/staging/media/lirc/lirc_imon.c
> +++ b/drivers/staging/media/lirc/lirc_imon.c
> @@ -623,8 +623,8 @@ static void imon_incoming_packet(struct imon_context *context,
>  	if (debug) {
>  		dev_info(dev, "raw packet: ");
>  		for (i = 0; i < len; ++i)
> -			printk("%02x ", buf[i]);
> -		printk("\n");
> +			dev_info(dev, "%02x ", buf[i]);
> +		dev_info(dev, "\n");

This doesn't work.  The dev_ functions can't really be split across more
than one line.  The try to put stuff at the start of the line but
we're putting it all on the same line they're putting a bunch of garbage
in the middle of the line.

>  	}
>  
>  	/*
> diff --git a/drivers/staging/media/lirc/lirc_sasem.c b/drivers/staging/media/lirc/lirc_sasem.c
> index c20ef56..e88e246 100644
> --- a/drivers/staging/media/lirc/lirc_sasem.c
> +++ b/drivers/staging/media/lirc/lirc_sasem.c
> @@ -583,10 +583,10 @@ static void incoming_packet(struct sasem_context *context,
>  	}
>  
>  	if (debug) {
> -		printk(KERN_INFO "Incoming data: ");
> +		pr_info("Incoming data: ");
>  		for (i = 0; i < 8; ++i)
> -			printk(KERN_CONT "%02x ", buf[i]);
> -		printk(KERN_CONT "\n");
> +			pr_cont("%02x", buf[i]);
> +		pr_cont("\n");

The debug variable really isn't needed if we're going to use the
standard debugging printks.

>  	}
>  
>  	/*
> diff --git a/drivers/staging/media/lirc/lirc_serial.c b/drivers/staging/media/lirc/lirc_serial.c
> index 181b92b..b07671b 100644
> --- a/drivers/staging/media/lirc/lirc_serial.c
> +++ b/drivers/staging/media/lirc/lirc_serial.c
> @@ -116,8 +116,7 @@ static bool txsense;	/* 0 = active high, 1 = active low */
>  #define dprintk(fmt, args...)					\
>  	do {							\
>  		if (debug)					\
> -			printk(KERN_DEBUG LIRC_DRIVER_NAME ": "	\
> -			       fmt, ## args);			\
> +			dev_dbg(LIRC_DRIVER_NAME, ": "fmt, ##args); \
>  	} while (0)
>  
>  /* forward declarations */
> @@ -356,9 +355,8 @@ static int init_timing_params(unsigned int new_duty_cycle,
>  	/* Derive pulse and space from the period */
>  	pulse_width = period * duty_cycle / 100;
>  	space_width = period - pulse_width;
> -	dprintk("in init_timing_params, freq=%d, duty_cycle=%d, "
> -		"clk/jiffy=%ld, pulse=%ld, space=%ld, "
> -		"conv_us_to_clocks=%ld\n",
> +	dprintk("in init_timing_params, freq=%d, duty_cycle=%d, clk/jiffy=%ld,
> +			pulse=%ld, space=%ld, conv_us_to_clocks=%ld\n",
>  		freq, duty_cycle, __this_cpu_read(cpu_info.loops_per_jiffy),
>  		pulse_width, space_width, conv_us_to_clocks);
>  	return 0;
> @@ -1075,7 +1073,7 @@ static int __init lirc_serial_init(void)
>  
>  	result = platform_driver_register(&lirc_serial_driver);
>  	if (result) {
> -		printk("lirc register returned %d\n", result);
> +		dprintk("lirc register returned %d\n", result);

Check to see what printks platform_driver_register() will print on
failure.  People add printks all over the place out of helpfulness but
a lot of the time they aren't needed.

>  		goto exit_buffer_free;
>  	}
>  
> @@ -1166,22 +1164,20 @@ module_init(lirc_serial_init_module);
>  module_exit(lirc_serial_exit_module);
>  
>  MODULE_DESCRIPTION("Infra-red receiver driver for serial ports.");
> -MODULE_AUTHOR("Ralph Metzler, Trent Piepho, Ben Pfaff, "
> -	      "Christoph Bartelmus, Andrei Tanas");
> +MODULE_AUTHOR("Ralph Metzler, Trent Piepho, Ben Pfaff, Christoph Bartelmus, Andrei Tanas");
>  MODULE_LICENSE("GPL");
>  
>  module_param(type, int, S_IRUGO);
> -MODULE_PARM_DESC(type, "Hardware type (0 = home-brew, 1 = IRdeo,"
> -		 " 2 = IRdeo Remote, 3 = AnimaX, 4 = IgorPlug,"
> -		 " 5 = NSLU2 RX:CTS2/TX:GreenLED)");
> +MODULE_PARM_DESC(type, "Hardware type (0 = home-brew, 1 = IRdeo,
> +	2 = IRdeo Remote, 3 = AnimaX, 4 = IgorPlug,
> +	5 = NSLU2 RX:CTS2/TX:GreenLED)");

The formatting is messed up now.

>  
>  module_param(io, int, S_IRUGO);
>  MODULE_PARM_DESC(io, "I/O address base (0x3f8 or 0x2f8)");
>  
>  /* some architectures (e.g. intel xscale) have memory mapped registers */
>  module_param(iommap, bool, S_IRUGO);
> -MODULE_PARM_DESC(iommap, "physical base for memory mapped I/O"
> -		" (0 = no memory mapped io)");
> +MODULE_PARM_DESC(iommap, "physical base for memory mapped I/O (0 = no memory mapped io)");
>  
>  /*
>   * some architectures (e.g. intel xscale) align the 8bit serial registers
> @@ -1198,13 +1194,12 @@ module_param(share_irq, bool, S_IRUGO);
>  MODULE_PARM_DESC(share_irq, "Share interrupts (0 = off, 1 = on)");
>  
>  module_param(sense, int, S_IRUGO);
> -MODULE_PARM_DESC(sense, "Override autodetection of IR receiver circuit"
> -		 " (0 = active high, 1 = active low )");
> +MODULE_PARM_DESC(sense, "Override autodetection of IR receiver circuit
> +		(0 = active high, 1 = active low )");

This messes up the formatting.

>  
>  #ifdef CONFIG_LIRC_SERIAL_TRANSMITTER
>  module_param(txsense, bool, S_IRUGO);
> -MODULE_PARM_DESC(txsense, "Sense of transmitter circuit"
> -		 " (0 = active high, 1 = active low )");
> +MODULE_PARM_DESC(txsense, "Sense of transmitter circuit (0 = active high, 1 = active low )");
>  #endif
>  
>  module_param(softcarrier, bool, S_IRUGO);
> diff --git a/drivers/staging/media/lirc/lirc_sir.c b/drivers/staging/media/lirc/lirc_sir.c
> index 2ee55ea..cdbb71f 100644
> --- a/drivers/staging/media/lirc/lirc_sir.c
> +++ b/drivers/staging/media/lirc/lirc_sir.c
> @@ -143,8 +143,7 @@ static bool debug;
>  #define dprintk(fmt, args...)						\
>  	do {								\
>  		if (debug)						\
> -			printk(KERN_DEBUG LIRC_DRIVER_NAME ": "		\
> -				fmt, ## args);				\
> +			dev_dbg(LIRC_DRIVER_NAME, ": "fmt, ## args); \
>  	} while (0)
>  
>  /* SECTION: Prototypes */
> diff --git a/drivers/staging/media/lirc/lirc_zilog.c b/drivers/staging/media/lirc/lirc_zilog.c
> index 567feba..9c3a3d7 100644
> --- a/drivers/staging/media/lirc/lirc_zilog.c
> +++ b/drivers/staging/media/lirc/lirc_zilog.c
> @@ -152,10 +152,9 @@ struct tx_data_struct {
>  static struct tx_data_struct *tx_data;
>  static struct mutex tx_data_lock;
>  
> -#define zilog_notify(s, args...) printk(KERN_NOTICE KBUILD_MODNAME ": " s, \
> -					## args)
> -#define zilog_error(s, args...) printk(KERN_ERR KBUILD_MODNAME ": " s, ## args)
> -#define zilog_info(s, args...) printk(KERN_INFO KBUILD_MODNAME ": " s, ## args)
> +#define zilog_notify(s, args...) dev_notice(KBUILD_MODNAME, ": " s, ## args)
> +#define zilog_error(s, args...) dev_err(KBUILD_MODNAME, ": " s, ## args)
> +#define zilog_info(s, args...) dev_info(KBUILD_MODNAME, ": " s, ## args)

These defines are rubbish and pointless.  Just delete them all.

>  
>  /* module parameters */
>  static bool debug;	/* debug output */
> @@ -165,8 +164,7 @@ static int minor = -1;	/* minor number */
>  #define dprintk(fmt, args...)						\
>  	do {								\
>  		if (debug)						\
> -			printk(KERN_DEBUG KBUILD_MODNAME ": " fmt,	\
> -				 ## args);				\
> +			pr_dbg(KBUILD_MODNAME, ": " fmt, ## args); \
>  	} while (0)
>  
>  
> @@ -382,14 +380,14 @@ static int add_to_buf(struct IR *ir)
>  			zilog_error("i2c_master_send failed with %d\n",	ret);
>  			if (failures >= 3) {
>  				mutex_unlock(&ir->ir_lock);
> -				zilog_error("unable to read from the IR chip "
> -					    "after 3 resets, giving up\n");
> +				zilog_error("unable to read from the IR chip
> +						after 3 resets, giving up\n");


Formatting messed up for everything else beyond this point.  We don't
want to print all those tabs in dmesg.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fixed all coding style issues for drivers/staging/media/lirc/
  2014-10-02  2:35 Amber Thrall
@ 2014-10-02  2:46 ` Greg KH
  2014-10-02  8:13 ` Dan Carpenter
  1 sibling, 0 replies; 11+ messages in thread
From: Greg KH @ 2014-10-02  2:46 UTC (permalink / raw)
  To: Amber Thrall; +Cc: jarod, devel, linux-media, linux-kernel

On Wed, Oct 01, 2014 at 07:35:51PM -0700, Amber Thrall wrote:
> Fixed various coding sytles.

You need to be specific as to what you changed.

Also, use get_maintainer to figure out who to sent this to (hint, not
me...)

greg k-h

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] Fixed all coding style issues for drivers/staging/media/lirc/
@ 2014-10-02  2:35 Amber Thrall
  2014-10-02  2:46 ` Greg KH
  2014-10-02  8:13 ` Dan Carpenter
  0 siblings, 2 replies; 11+ messages in thread
From: Amber Thrall @ 2014-10-02  2:35 UTC (permalink / raw)
  To: greg, jarod; +Cc: devel, linux-media, linux-kernel, Amber Thrall

Fixed various coding sytles.

Signed-off-by: Amber Thrall <amber.rose.thrall@gmail.com>
---
 drivers/staging/media/lirc/lirc_bt829.c  |  2 +-
 drivers/staging/media/lirc/lirc_imon.c   |  4 +-
 drivers/staging/media/lirc/lirc_sasem.c  |  6 +--
 drivers/staging/media/lirc/lirc_serial.c | 29 ++++++--------
 drivers/staging/media/lirc/lirc_sir.c    |  3 +-
 drivers/staging/media/lirc/lirc_zilog.c  | 69 +++++++++++++++-----------------
 6 files changed, 52 insertions(+), 61 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_bt829.c b/drivers/staging/media/lirc/lirc_bt829.c
index 4c806ba..c70ca68 100644
--- a/drivers/staging/media/lirc/lirc_bt829.c
+++ b/drivers/staging/media/lirc/lirc_bt829.c
@@ -59,7 +59,7 @@ static bool debug;
 #define dprintk(fmt, args...)						 \
 	do {								 \
 		if (debug)						 \
-			printk(KERN_DEBUG DRIVER_NAME ": "fmt, ## args); \
+			dev_dbg(DRIVER_NAME, ": "fmt, ##args); \
 	} while (0)
 
 static int atir_minor;
diff --git a/drivers/staging/media/lirc/lirc_imon.c b/drivers/staging/media/lirc/lirc_imon.c
index 7aca44f..bce0408 100644
--- a/drivers/staging/media/lirc/lirc_imon.c
+++ b/drivers/staging/media/lirc/lirc_imon.c
@@ -623,8 +623,8 @@ static void imon_incoming_packet(struct imon_context *context,
 	if (debug) {
 		dev_info(dev, "raw packet: ");
 		for (i = 0; i < len; ++i)
-			printk("%02x ", buf[i]);
-		printk("\n");
+			dev_info(dev, "%02x ", buf[i]);
+		dev_info(dev, "\n");
 	}
 
 	/*
diff --git a/drivers/staging/media/lirc/lirc_sasem.c b/drivers/staging/media/lirc/lirc_sasem.c
index c20ef56..e88e246 100644
--- a/drivers/staging/media/lirc/lirc_sasem.c
+++ b/drivers/staging/media/lirc/lirc_sasem.c
@@ -583,10 +583,10 @@ static void incoming_packet(struct sasem_context *context,
 	}
 
 	if (debug) {
-		printk(KERN_INFO "Incoming data: ");
+		pr_info("Incoming data: ");
 		for (i = 0; i < 8; ++i)
-			printk(KERN_CONT "%02x ", buf[i]);
-		printk(KERN_CONT "\n");
+			pr_cont("%02x", buf[i]);
+		pr_cont("\n");
 	}
 
 	/*
diff --git a/drivers/staging/media/lirc/lirc_serial.c b/drivers/staging/media/lirc/lirc_serial.c
index 181b92b..b07671b 100644
--- a/drivers/staging/media/lirc/lirc_serial.c
+++ b/drivers/staging/media/lirc/lirc_serial.c
@@ -116,8 +116,7 @@ static bool txsense;	/* 0 = active high, 1 = active low */
 #define dprintk(fmt, args...)					\
 	do {							\
 		if (debug)					\
-			printk(KERN_DEBUG LIRC_DRIVER_NAME ": "	\
-			       fmt, ## args);			\
+			dev_dbg(LIRC_DRIVER_NAME, ": "fmt, ##args); \
 	} while (0)
 
 /* forward declarations */
@@ -356,9 +355,8 @@ static int init_timing_params(unsigned int new_duty_cycle,
 	/* Derive pulse and space from the period */
 	pulse_width = period * duty_cycle / 100;
 	space_width = period - pulse_width;
-	dprintk("in init_timing_params, freq=%d, duty_cycle=%d, "
-		"clk/jiffy=%ld, pulse=%ld, space=%ld, "
-		"conv_us_to_clocks=%ld\n",
+	dprintk("in init_timing_params, freq=%d, duty_cycle=%d, clk/jiffy=%ld,
+			pulse=%ld, space=%ld, conv_us_to_clocks=%ld\n",
 		freq, duty_cycle, __this_cpu_read(cpu_info.loops_per_jiffy),
 		pulse_width, space_width, conv_us_to_clocks);
 	return 0;
@@ -1075,7 +1073,7 @@ static int __init lirc_serial_init(void)
 
 	result = platform_driver_register(&lirc_serial_driver);
 	if (result) {
-		printk("lirc register returned %d\n", result);
+		dprintk("lirc register returned %d\n", result);
 		goto exit_buffer_free;
 	}
 
@@ -1166,22 +1164,20 @@ module_init(lirc_serial_init_module);
 module_exit(lirc_serial_exit_module);
 
 MODULE_DESCRIPTION("Infra-red receiver driver for serial ports.");
-MODULE_AUTHOR("Ralph Metzler, Trent Piepho, Ben Pfaff, "
-	      "Christoph Bartelmus, Andrei Tanas");
+MODULE_AUTHOR("Ralph Metzler, Trent Piepho, Ben Pfaff, Christoph Bartelmus, Andrei Tanas");
 MODULE_LICENSE("GPL");
 
 module_param(type, int, S_IRUGO);
-MODULE_PARM_DESC(type, "Hardware type (0 = home-brew, 1 = IRdeo,"
-		 " 2 = IRdeo Remote, 3 = AnimaX, 4 = IgorPlug,"
-		 " 5 = NSLU2 RX:CTS2/TX:GreenLED)");
+MODULE_PARM_DESC(type, "Hardware type (0 = home-brew, 1 = IRdeo,
+	2 = IRdeo Remote, 3 = AnimaX, 4 = IgorPlug,
+	5 = NSLU2 RX:CTS2/TX:GreenLED)");
 
 module_param(io, int, S_IRUGO);
 MODULE_PARM_DESC(io, "I/O address base (0x3f8 or 0x2f8)");
 
 /* some architectures (e.g. intel xscale) have memory mapped registers */
 module_param(iommap, bool, S_IRUGO);
-MODULE_PARM_DESC(iommap, "physical base for memory mapped I/O"
-		" (0 = no memory mapped io)");
+MODULE_PARM_DESC(iommap, "physical base for memory mapped I/O (0 = no memory mapped io)");
 
 /*
  * some architectures (e.g. intel xscale) align the 8bit serial registers
@@ -1198,13 +1194,12 @@ module_param(share_irq, bool, S_IRUGO);
 MODULE_PARM_DESC(share_irq, "Share interrupts (0 = off, 1 = on)");
 
 module_param(sense, int, S_IRUGO);
-MODULE_PARM_DESC(sense, "Override autodetection of IR receiver circuit"
-		 " (0 = active high, 1 = active low )");
+MODULE_PARM_DESC(sense, "Override autodetection of IR receiver circuit
+		(0 = active high, 1 = active low )");
 
 #ifdef CONFIG_LIRC_SERIAL_TRANSMITTER
 module_param(txsense, bool, S_IRUGO);
-MODULE_PARM_DESC(txsense, "Sense of transmitter circuit"
-		 " (0 = active high, 1 = active low )");
+MODULE_PARM_DESC(txsense, "Sense of transmitter circuit (0 = active high, 1 = active low )");
 #endif
 
 module_param(softcarrier, bool, S_IRUGO);
diff --git a/drivers/staging/media/lirc/lirc_sir.c b/drivers/staging/media/lirc/lirc_sir.c
index 2ee55ea..cdbb71f 100644
--- a/drivers/staging/media/lirc/lirc_sir.c
+++ b/drivers/staging/media/lirc/lirc_sir.c
@@ -143,8 +143,7 @@ static bool debug;
 #define dprintk(fmt, args...)						\
 	do {								\
 		if (debug)						\
-			printk(KERN_DEBUG LIRC_DRIVER_NAME ": "		\
-				fmt, ## args);				\
+			dev_dbg(LIRC_DRIVER_NAME, ": "fmt, ## args); \
 	} while (0)
 
 /* SECTION: Prototypes */
diff --git a/drivers/staging/media/lirc/lirc_zilog.c b/drivers/staging/media/lirc/lirc_zilog.c
index 567feba..9c3a3d7 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -152,10 +152,9 @@ struct tx_data_struct {
 static struct tx_data_struct *tx_data;
 static struct mutex tx_data_lock;
 
-#define zilog_notify(s, args...) printk(KERN_NOTICE KBUILD_MODNAME ": " s, \
-					## args)
-#define zilog_error(s, args...) printk(KERN_ERR KBUILD_MODNAME ": " s, ## args)
-#define zilog_info(s, args...) printk(KERN_INFO KBUILD_MODNAME ": " s, ## args)
+#define zilog_notify(s, args...) dev_notice(KBUILD_MODNAME, ": " s, ## args)
+#define zilog_error(s, args...) dev_err(KBUILD_MODNAME, ": " s, ## args)
+#define zilog_info(s, args...) dev_info(KBUILD_MODNAME, ": " s, ## args)
 
 /* module parameters */
 static bool debug;	/* debug output */
@@ -165,8 +164,7 @@ static int minor = -1;	/* minor number */
 #define dprintk(fmt, args...)						\
 	do {								\
 		if (debug)						\
-			printk(KERN_DEBUG KBUILD_MODNAME ": " fmt,	\
-				 ## args);				\
+			pr_dbg(KBUILD_MODNAME, ": " fmt, ## args); \
 	} while (0)
 
 
@@ -382,14 +380,14 @@ static int add_to_buf(struct IR *ir)
 			zilog_error("i2c_master_send failed with %d\n",	ret);
 			if (failures >= 3) {
 				mutex_unlock(&ir->ir_lock);
-				zilog_error("unable to read from the IR chip "
-					    "after 3 resets, giving up\n");
+				zilog_error("unable to read from the IR chip
+						after 3 resets, giving up\n");
 				break;
 			}
 
 			/* Looks like the chip crashed, reset it */
-			zilog_error("polling the IR receiver chip failed, "
-				    "trying reset\n");
+			zilog_error("polling the IR receiver chip failed,
+					trying reset\n");
 
 			set_current_state(TASK_UNINTERRUPTIBLE);
 			if (kthread_should_stop()) {
@@ -415,8 +413,8 @@ static int add_to_buf(struct IR *ir)
 		ret = i2c_master_recv(rx->c, keybuf, sizeof(keybuf));
 		mutex_unlock(&ir->ir_lock);
 		if (ret != sizeof(keybuf)) {
-			zilog_error("i2c_master_recv failed with %d -- "
-				    "keeping last read buffer\n", ret);
+			zilog_error("i2c_master_recv failed with %d --
+					keeping last read buffer\n", ret);
 		} else {
 			rx->b[0] = keybuf[3];
 			rx->b[1] = keybuf[4];
@@ -720,8 +718,8 @@ static int send_boot_data(struct IR_tx *tx)
 		zilog_error("unexpected IR TX init response: %02x\n", buf[0]);
 		return 0;
 	}
-	zilog_notify("Zilog/Hauppauge IR blaster firmware version "
-		     "%d.%d.%d loaded\n", buf[1], buf[2], buf[3]);
+	zilog_notify("Zilog/Hauppauge IR blaster firmware version
+			%d.%d.%d loaded\n", buf[1], buf[2], buf[3]);
 
 	return 0;
 }
@@ -803,9 +801,8 @@ static int fw_load(struct IR_tx *tx)
 	if (!read_uint8(&data, tx_data->endp, &version))
 		goto corrupt;
 	if (version != 1) {
-		zilog_error("unsupported code set file version (%u, expected"
-			    "1) -- please upgrade to a newer driver",
-			    version);
+		zilog_error("unsupported code set file version (%u, expected
+			1) -- please upgrade to a newer driver", version);
 		fw_unload_locked();
 		ret = -EFAULT;
 		goto out;
@@ -990,8 +987,8 @@ static int send_code(struct IR_tx *tx, unsigned int code, unsigned int key)
 	ret = get_key_data(data_block, code, key);
 
 	if (ret == -EPROTO) {
-		zilog_error("failed to get data for code %u, key %u -- check "
-			    "lircd.conf entries\n", code, key);
+		zilog_error("failed to get data for code %u, key %u -- check
+				lircd.conf entries\n", code, key);
 		return ret;
 	} else if (ret != 0)
 		return ret;
@@ -1066,12 +1063,12 @@ static int send_code(struct IR_tx *tx, unsigned int code, unsigned int key)
 		ret = i2c_master_send(tx->c, buf, 1);
 		if (ret == 1)
 			break;
-		dprintk("NAK expected: i2c_master_send "
-			"failed with %d (try %d)\n", ret, i+1);
+		dprintk("NAK expected: i2c_master_send
+				failed with %d (try %d)\n", ret, i+1);
 	}
 	if (ret != 1) {
-		zilog_error("IR TX chip never got ready: last i2c_master_send "
-			    "failed with %d\n", ret);
+		zilog_error("IR TX chip never got ready: last i2c_master_send
+				failed with %d\n", ret);
 		return ret < 0 ? ret : -EFAULT;
 	}
 
@@ -1173,12 +1170,12 @@ static ssize_t write(struct file *filep, const char __user *buf, size_t n,
 		 */
 		if (ret != 0) {
 			/* Looks like the chip crashed, reset it */
-			zilog_error("sending to the IR transmitter chip "
-				    "failed, trying reset\n");
+			zilog_error("sending to the IR transmitter chip
+					failed, trying reset\n");
 
 			if (failures >= 3) {
-				zilog_error("unable to send to the IR chip "
-					    "after 3 resets, giving up\n");
+				zilog_error("unable to send to the IR chip
+						after 3 resets, giving up\n");
 				mutex_unlock(&ir->ir_lock);
 				mutex_unlock(&tx->client_lock);
 				put_ir_tx(tx, false);
@@ -1547,8 +1544,8 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id)
 
 		/* Proceed only if the Rx client is also ready or not needed */
 		if (rx == NULL && !tx_only) {
-			zilog_info("probe of IR Tx on %s (i2c-%d) done. Waiting"
-				   " on IR Rx.\n", adap->name, adap->nr);
+			zilog_info("probe of IR Tx on %s (i2c-%d) done. Waiting
+					on IR Rx.\n", adap->name, adap->nr);
 			goto out_ok;
 		}
 	} else {
@@ -1586,8 +1583,8 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id)
 				       "zilog-rx-i2c-%d", adap->nr);
 		if (IS_ERR(rx->task)) {
 			ret = PTR_ERR(rx->task);
-			zilog_error("%s: could not start IR Rx polling thread"
-				    "\n", __func__);
+			zilog_error("%s: could not start IR Rx polling thread
+					\n", __func__);
 			/* Failed kthread, so put back the ir ref */
 			put_ir_device(ir, true);
 			/* Failure exit, so put back rx ref from i2c_client */
@@ -1599,8 +1596,8 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id)
 
 		/* Proceed only if the Tx client is also ready */
 		if (tx == NULL) {
-			zilog_info("probe of IR Rx on %s (i2c-%d) done. Waiting"
-				   " on IR Tx.\n", adap->name, adap->nr);
+			zilog_info("probe of IR Rx on %s (i2c-%d) done. Waiting
+					on IR Tx.\n", adap->name, adap->nr);
 			goto out_ok;
 		}
 	}
@@ -1674,9 +1671,9 @@ module_init(zilog_init);
 module_exit(zilog_exit);
 
 MODULE_DESCRIPTION("Zilog/Hauppauge infrared transmitter driver (i2c stack)");
-MODULE_AUTHOR("Gerd Knorr, Michal Kochanowicz, Christoph Bartelmus, "
-	      "Ulrich Mueller, Stefan Jahn, Jerome Brock, Mark Weaver, "
-	      "Andy Walls");
+MODULE_AUTHOR("Gerd Knorr, Michal Kochanowicz, Christoph Bartelmus,
+		Ulrich Mueller, Stefan Jahn, Jerome Brock, Mark Weaver,
+		Andy Walls");
 MODULE_LICENSE("GPL");
 /* for compat with old name, which isn't all that accurate anymore */
 MODULE_ALIAS("lirc_pvr150");
-- 
2.1.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2014-10-03  1:41 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-02  4:40 [PATCH] Fixed all coding style issues for drivers/staging/media/lirc/ Amber Thrall
2014-10-02  4:49 ` Ilia Mirkin
2014-10-02  6:20 ` Jeremiah Mahler
2014-10-02 12:45 ` Greg KH
2014-10-02 13:29 ` Mauro Carvalho Chehab
2014-10-02 16:45   ` Joe Perches
2014-10-02 23:58     ` Antti Palosaari
2014-10-03  1:41       ` Joe Perches
  -- strict thread matches above, loose matches on Subject: below --
2014-10-02  2:35 Amber Thrall
2014-10-02  2:46 ` Greg KH
2014-10-02  8:13 ` Dan Carpenter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.