[08/22] m68k/mac: iop - Modernize printing of kernel messages
diff mbox series

Message ID 1481123360-10978-9-git-send-email-geert@linux-m68k.org
State New, archived
Headers show
Series
  • m68k: Modernize printing of kernel messages
Related show

Commit Message

Geert Uytterhoeven Dec. 7, 2016, 3:09 p.m. UTC
- Introduce helpers for printing debug messages, incl. dummies for
    validating format strings when debugging is disabled,
  - Convert from printk() to pr_*(),
  - Add missing continuations,
  - Drop superfluous casts.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 arch/m68k/mac/iop.c | 69 ++++++++++++++++++++++-------------------------------
 1 file changed, 29 insertions(+), 40 deletions(-)

Comments

Finn Thain Dec. 7, 2016, 11:06 p.m. UTC | #1
On Wed, 7 Dec 2016, Geert Uytterhoeven wrote:

>   - Introduce helpers for printing debug messages, incl. dummies for
>     validating format strings when debugging is disabled,
>   - Convert from printk() to pr_*(),
>   - Add missing continuations,
>   - Drop superfluous casts.
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
>  arch/m68k/mac/iop.c | 69 ++++++++++++++++++++++-------------------------------
>  1 file changed, 29 insertions(+), 40 deletions(-)
> 
> diff --git a/arch/m68k/mac/iop.c b/arch/m68k/mac/iop.c
> index 7990b6f50105b19b..bbfeded6b270f950 100644
> --- a/arch/m68k/mac/iop.c
> +++ b/arch/m68k/mac/iop.c
> @@ -116,6 +116,13 @@
>  #include <asm/mac_iop.h>
>  
>  /*#define DEBUG_IOP*/
> +#ifdef DEBUG_IOP

We can use #if 0 here, since the macro can be avoided later...

> +#define pr_iop(fmt, ...) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_iop_cont(fmt, ...) pr_cont(fmt, ##__VA_ARGS__)
> +#else
> +#define pr_iop(fmt, ...) no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_iop_cont(fmt, ...) no_printk(fmt, ##__VA_ARGS__)
> +#endif
>  
>  /* Non-zero if the IOPs are present */
>  
> @@ -268,10 +275,10 @@ void __init iop_init(void)
>  	int i;
>  
>  	if (iop_scc_present) {
> -		printk("IOP: detected SCC IOP at %p\n", iop_base[IOP_NUM_SCC]);
> +		pr_info("IOP: detected SCC IOP at %p\n", iop_base[IOP_NUM_SCC]);
>  	}
>  	if (iop_ism_present) {
> -		printk("IOP: detected ISM IOP at %p\n", iop_base[IOP_NUM_ISM]);
> +		pr_info("IOP: detected ISM IOP at %p\n", iop_base[IOP_NUM_ISM]);
>  		iop_start(iop_base[IOP_NUM_ISM]);
>  		iop_alive(iop_base[IOP_NUM_ISM]); /* clears the alive flag */
>  	}
> @@ -310,9 +317,9 @@ void __init iop_register_interrupts(void)
>  				pr_err("Couldn't register ISM IOP interrupt\n");
>  		}
>  		if (!iop_alive(iop_base[IOP_NUM_ISM])) {
> -			printk("IOP: oh my god, they killed the ISM IOP!\n");
> +			pr_warn("IOP: oh my god, they killed the ISM IOP!\n");
>  		} else {
> -			printk("IOP: the ISM IOP seems to be alive.\n");
> +			pr_info("IOP: the ISM IOP seems to be alive.\n");
>  		}
>  	}
>  }
> @@ -349,9 +356,8 @@ void iop_complete_message(struct iop_msg *msg)
>  	int chan = msg->channel;
>  	int i,offset;
>  
> -#ifdef DEBUG_IOP
> -	printk("iop_complete(%p): iop %d chan %d\n", msg, msg->iop_num, msg->channel);
> -#endif
> +	pr_iop("iop_complete(%p): iop %d chan %d\n", msg, msg->iop_num,
> +	       msg->channel);
>  
>  	offset = IOP_ADDR_RECV_MSG + (msg->channel * IOP_MSG_LEN);
>  
> @@ -397,9 +403,7 @@ static void iop_handle_send(uint iop_num, uint chan)
>  	struct iop_msg *msg,*msg2;
>  	int i,offset;
>  
> -#ifdef DEBUG_IOP
> -	printk("iop_handle_send: iop %d channel %d\n", iop_num, chan);
> -#endif
> +	pr_iop("iop_handle_send: iop %d channel %d\n", iop_num, chan);
>  
>  	iop_writeb(iop, IOP_ADDR_SEND_STATE + chan, IOP_MSG_IDLE);
>  
> @@ -430,9 +434,7 @@ static void iop_handle_recv(uint iop_num, uint chan)
>  	int i,offset;
>  	struct iop_msg *msg;
>  
> -#ifdef DEBUG_IOP
> -	printk("iop_handle_recv: iop %d channel %d\n", iop_num, chan);
> -#endif
> +	pr_iop("iop_handle_recv: iop %d channel %d\n", iop_num, chan);
>  
>  	msg = iop_alloc_msg();
>  	msg->iop_num = iop_num;
> @@ -454,13 +456,14 @@ static void iop_handle_recv(uint iop_num, uint chan)
>  	if (msg->handler) {
>  		(*msg->handler)(msg);
>  	} else {
> +		pr_iop("iop_handle_recv: unclaimed message on iop %d channel %d\n",
> +		       iop_num, chan);
>  #ifdef DEBUG_IOP
> -		printk("iop_handle_recv: unclaimed message on iop %d channel %d\n", iop_num, chan);
> -		printk("iop_handle_recv:");
> +		pr_iop("iop_handle_recv:");
>  		for (i = 0 ; i < IOP_MSG_LEN ; i++) {
> -			printk(" %02X", (uint) msg->message[i]);
> +			pr_cont(" %02X", msg->message[i]);
>  		}
> -		printk("\n");
> +		pr_iop_cont("\n");
>  #endif
>  		iop_complete_message(msg);
>  	}

I think you can avoid the DEBUG_IOP macro entirely, by using

	pr_iop("iop_handle_recv: %*ph\n", IOP_MSG_LEN, msg->message);

Thanks.

> @@ -574,50 +577,36 @@ irqreturn_t iop_ism_irq(int irq, void *dev_id)
>  	volatile struct mac_iop *iop = iop_base[iop_num];
>  	int i,state;
>  
> -#ifdef DEBUG_IOP
> -	printk("iop_ism_irq: status = %02X\n", (uint) iop->status_ctrl);
> -#endif
> +	pr_iop("iop_ism_irq: status = %02X\n", iop->status_ctrl);
>  
>  	/* INT0 indicates a state change on an outgoing message channel */
>  
>  	if (iop->status_ctrl & IOP_INT0) {
>  		iop->status_ctrl = IOP_INT0 | IOP_RUN | IOP_AUTOINC;
> -#ifdef DEBUG_IOP
> -		printk("iop_ism_irq: new status = %02X, send states",
> -			(uint) iop->status_ctrl);
> -#endif
> +		pr_iop("iop_ism_irq: new status = %02X, send states",
> +		       iop->status_ctrl);
>  		for (i = 0 ; i < NUM_IOP_CHAN  ; i++) {
>  			state = iop_readb(iop, IOP_ADDR_SEND_STATE + i);
> -#ifdef DEBUG_IOP
> -			printk(" %02X", state);
> -#endif
> +			pr_iop_cont(" %02X", state);
>  			if (state == IOP_MSG_COMPLETE) {
>  				iop_handle_send(iop_num, i);
>  			}
>  		}
> -#ifdef DEBUG_IOP
> -		printk("\n");
> -#endif
> +		pr_iop_cont("\n");
>  	}
>  
>  	if (iop->status_ctrl & IOP_INT1) {	/* INT1 for incoming msgs */
>  		iop->status_ctrl = IOP_INT1 | IOP_RUN | IOP_AUTOINC;
> -#ifdef DEBUG_IOP
> -		printk("iop_ism_irq: new status = %02X, recv states",
> -			(uint) iop->status_ctrl);
> -#endif
> +		pr_iop("iop_ism_irq: new status = %02X, recv states",
> +		       iop->status_ctrl);
>  		for (i = 0 ; i < NUM_IOP_CHAN ; i++) {
>  			state = iop_readb(iop, IOP_ADDR_RECV_STATE + i);
> -#ifdef DEBUG_IOP
> -			printk(" %02X", state);
> -#endif
> +			pr_iop_cont(" %02X", state);
>  			if (state == IOP_MSG_NEW) {
>  				iop_handle_recv(iop_num, i);
>  			}
>  		}
> -#ifdef DEBUG_IOP
> -		printk("\n");
> -#endif
> +		pr_iop_cont("\n");
>  	}
>  	return IRQ_HANDLED;
>  }
> 

--

Patch
diff mbox series

diff --git a/arch/m68k/mac/iop.c b/arch/m68k/mac/iop.c
index 7990b6f50105b19b..bbfeded6b270f950 100644
--- a/arch/m68k/mac/iop.c
+++ b/arch/m68k/mac/iop.c
@@ -116,6 +116,13 @@ 
 #include <asm/mac_iop.h>
 
 /*#define DEBUG_IOP*/
+#ifdef DEBUG_IOP
+#define pr_iop(fmt, ...) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_iop_cont(fmt, ...) pr_cont(fmt, ##__VA_ARGS__)
+#else
+#define pr_iop(fmt, ...) no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_iop_cont(fmt, ...) no_printk(fmt, ##__VA_ARGS__)
+#endif
 
 /* Non-zero if the IOPs are present */
 
@@ -268,10 +275,10 @@  void __init iop_init(void)
 	int i;
 
 	if (iop_scc_present) {
-		printk("IOP: detected SCC IOP at %p\n", iop_base[IOP_NUM_SCC]);
+		pr_info("IOP: detected SCC IOP at %p\n", iop_base[IOP_NUM_SCC]);
 	}
 	if (iop_ism_present) {
-		printk("IOP: detected ISM IOP at %p\n", iop_base[IOP_NUM_ISM]);
+		pr_info("IOP: detected ISM IOP at %p\n", iop_base[IOP_NUM_ISM]);
 		iop_start(iop_base[IOP_NUM_ISM]);
 		iop_alive(iop_base[IOP_NUM_ISM]); /* clears the alive flag */
 	}
@@ -310,9 +317,9 @@  void __init iop_register_interrupts(void)
 				pr_err("Couldn't register ISM IOP interrupt\n");
 		}
 		if (!iop_alive(iop_base[IOP_NUM_ISM])) {
-			printk("IOP: oh my god, they killed the ISM IOP!\n");
+			pr_warn("IOP: oh my god, they killed the ISM IOP!\n");
 		} else {
-			printk("IOP: the ISM IOP seems to be alive.\n");
+			pr_info("IOP: the ISM IOP seems to be alive.\n");
 		}
 	}
 }
@@ -349,9 +356,8 @@  void iop_complete_message(struct iop_msg *msg)
 	int chan = msg->channel;
 	int i,offset;
 
-#ifdef DEBUG_IOP
-	printk("iop_complete(%p): iop %d chan %d\n", msg, msg->iop_num, msg->channel);
-#endif
+	pr_iop("iop_complete(%p): iop %d chan %d\n", msg, msg->iop_num,
+	       msg->channel);
 
 	offset = IOP_ADDR_RECV_MSG + (msg->channel * IOP_MSG_LEN);
 
@@ -397,9 +403,7 @@  static void iop_handle_send(uint iop_num, uint chan)
 	struct iop_msg *msg,*msg2;
 	int i,offset;
 
-#ifdef DEBUG_IOP
-	printk("iop_handle_send: iop %d channel %d\n", iop_num, chan);
-#endif
+	pr_iop("iop_handle_send: iop %d channel %d\n", iop_num, chan);
 
 	iop_writeb(iop, IOP_ADDR_SEND_STATE + chan, IOP_MSG_IDLE);
 
@@ -430,9 +434,7 @@  static void iop_handle_recv(uint iop_num, uint chan)
 	int i,offset;
 	struct iop_msg *msg;
 
-#ifdef DEBUG_IOP
-	printk("iop_handle_recv: iop %d channel %d\n", iop_num, chan);
-#endif
+	pr_iop("iop_handle_recv: iop %d channel %d\n", iop_num, chan);
 
 	msg = iop_alloc_msg();
 	msg->iop_num = iop_num;
@@ -454,13 +456,14 @@  static void iop_handle_recv(uint iop_num, uint chan)
 	if (msg->handler) {
 		(*msg->handler)(msg);
 	} else {
+		pr_iop("iop_handle_recv: unclaimed message on iop %d channel %d\n",
+		       iop_num, chan);
 #ifdef DEBUG_IOP
-		printk("iop_handle_recv: unclaimed message on iop %d channel %d\n", iop_num, chan);
-		printk("iop_handle_recv:");
+		pr_iop("iop_handle_recv:");
 		for (i = 0 ; i < IOP_MSG_LEN ; i++) {
-			printk(" %02X", (uint) msg->message[i]);
+			pr_cont(" %02X", msg->message[i]);
 		}
-		printk("\n");
+		pr_iop_cont("\n");
 #endif
 		iop_complete_message(msg);
 	}
@@ -574,50 +577,36 @@  irqreturn_t iop_ism_irq(int irq, void *dev_id)
 	volatile struct mac_iop *iop = iop_base[iop_num];
 	int i,state;
 
-#ifdef DEBUG_IOP
-	printk("iop_ism_irq: status = %02X\n", (uint) iop->status_ctrl);
-#endif
+	pr_iop("iop_ism_irq: status = %02X\n", iop->status_ctrl);
 
 	/* INT0 indicates a state change on an outgoing message channel */
 
 	if (iop->status_ctrl & IOP_INT0) {
 		iop->status_ctrl = IOP_INT0 | IOP_RUN | IOP_AUTOINC;
-#ifdef DEBUG_IOP
-		printk("iop_ism_irq: new status = %02X, send states",
-			(uint) iop->status_ctrl);
-#endif
+		pr_iop("iop_ism_irq: new status = %02X, send states",
+		       iop->status_ctrl);
 		for (i = 0 ; i < NUM_IOP_CHAN  ; i++) {
 			state = iop_readb(iop, IOP_ADDR_SEND_STATE + i);
-#ifdef DEBUG_IOP
-			printk(" %02X", state);
-#endif
+			pr_iop_cont(" %02X", state);
 			if (state == IOP_MSG_COMPLETE) {
 				iop_handle_send(iop_num, i);
 			}
 		}
-#ifdef DEBUG_IOP
-		printk("\n");
-#endif
+		pr_iop_cont("\n");
 	}
 
 	if (iop->status_ctrl & IOP_INT1) {	/* INT1 for incoming msgs */
 		iop->status_ctrl = IOP_INT1 | IOP_RUN | IOP_AUTOINC;
-#ifdef DEBUG_IOP
-		printk("iop_ism_irq: new status = %02X, recv states",
-			(uint) iop->status_ctrl);
-#endif
+		pr_iop("iop_ism_irq: new status = %02X, recv states",
+		       iop->status_ctrl);
 		for (i = 0 ; i < NUM_IOP_CHAN ; i++) {
 			state = iop_readb(iop, IOP_ADDR_RECV_STATE + i);
-#ifdef DEBUG_IOP
-			printk(" %02X", state);
-#endif
+			pr_iop_cont(" %02X", state);
 			if (state == IOP_MSG_NEW) {
 				iop_handle_recv(iop_num, i);
 			}
 		}
-#ifdef DEBUG_IOP
-		printk("\n");
-#endif
+		pr_iop_cont("\n");
 	}
 	return IRQ_HANDLED;
 }