[PATCH] parport: fix if-statement empty body warnings

[PATCH] parport: fix if-statement empty body warnings

[Randy Dunlap]

From: Randy Dunlap <rdunlap@infradead.org>

When debugging via DPRINTK() is not enabled, make the DPRINTK()
macro be an empty do-while block.

This fixes gcc warnings when -Wextra is set:

../drivers/parport/ieee1284.c:262:18: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
../drivers/parport/ieee1284.c:285:17: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
../drivers/parport/ieee1284.c:298:17: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
../drivers/parport/ieee1284_ops.c:576:18: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]

I have verified that there is no object code change (with gcc 7.5.0).

Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Cc: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
Cc: linux-parport@lists.infradead.org
---
 drivers/parport/ieee1284.c     |    2 +-
 drivers/parport/ieee1284_ops.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

--- linux-next-20200225.orig/drivers/parport/ieee1284.c
+++ linux-next-20200225/drivers/parport/ieee1284.c
@@ -34,7 +34,7 @@
 #ifdef DEBUG
 #define DPRINTK(stuff...) printk (stuff)
 #else
-#define DPRINTK(stuff...)
+#define DPRINTK(stuff...) do {} while (0)
 #endif
 
 /* Make parport_wait_peripheral wake up.
--- linux-next-20200225.orig/drivers/parport/ieee1284_ops.c
+++ linux-next-20200225/drivers/parport/ieee1284_ops.c
@@ -30,7 +30,7 @@
 #ifdef DEBUG
 #define DPRINTK(stuff...) printk (stuff)
 #else
-#define DPRINTK(stuff...)
+#define DPRINTK(stuff...) do {} while (0)
 #endif
 
 /***                                *

Re: [PATCH] parport: fix if-statement empty body warnings

[Joe Perches]

On Thu, 2020-02-27 at 18:08 -0800, Randy Dunlap wrote:
> From: Randy Dunlap <rdunlap@infradead.org>
> 
> When debugging via DPRINTK() is not enabled, make the DPRINTK()
> macro be an empty do-while block.
> 
> This fixes gcc warnings when -Wextra is set:
> 
> ../drivers/parport/ieee1284.c:262:18: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
> ../drivers/parport/ieee1284.c:285:17: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
> ../drivers/parport/ieee1284.c:298:17: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
> ../drivers/parport/ieee1284_ops.c:576:18: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
> 
> I have verified that there is no object code change (with gcc 7.5.0).
> 
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> Cc: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
> Cc: linux-parport@lists.infradead.org
> ---
>  drivers/parport/ieee1284.c     |    2 +-
>  drivers/parport/ieee1284_ops.c |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> --- linux-next-20200225.orig/drivers/parport/ieee1284.c
> +++ linux-next-20200225/drivers/parport/ieee1284.c
> @@ -34,7 +34,7 @@
>  #ifdef DEBUG
>  #define DPRINTK(stuff...) printk (stuff)
>  #else
> -#define DPRINTK(stuff...)
> +#define DPRINTK(stuff...) do {} while (0)

It's frequently better to use noprintk

And perhaps this should just become

#define DPRINTK pr_debug

or perhaps a conversion of DPRINTK uses
---
 drivers/parport/ieee1284_ops.c | 67 +++++++++++++++---------------------------
 1 file changed, 23 insertions(+), 44 deletions(-)

diff --git a/drivers/parport/ieee1284_ops.c b/drivers/parport/ieee1284_ops.c
index 5d41dd..b1c9f51 100644
--- a/drivers/parport/ieee1284_ops.c
+++ b/drivers/parport/ieee1284_ops.c
@@ -27,12 +27,6 @@
 #undef DEBUG /* Don't want a garbled console */
 #endif
 
-#ifdef DEBUG
-#define DPRINTK(stuff...) printk (stuff)
-#else
-#define DPRINTK(stuff...)
-#endif
-
 /***                                *
  * One-way data transfer functions. *
  *                                ***/
@@ -115,7 +109,7 @@ size_t parport_ieee1284_write_compat (struct parport *port,
 		if (signal_pending (current))
 			break;
 
-		DPRINTK (KERN_DEBUG "%s: Timed out\n", port->name);
+		pr_debug("%s: Timed out\n", port->name);
 		break;
 
 	ready:
@@ -178,9 +172,8 @@ size_t parport_ieee1284_read_nibble (struct parport *port,
 		if (parport_wait_peripheral (port,
 					     PARPORT_STATUS_ACK, 0)) {
 			/* Timeout -- no more data? */
-			DPRINTK (KERN_DEBUG
-				 "%s: Nibble timeout at event 9 (%d bytes)\n",
-				 port->name, i/2);
+			pr_debug("%s: Nibble timeout at event 9 (%d bytes)\n",
+				 port->name, i / 2);
 			parport_frob_control (port, PARPORT_CONTROL_AUTOFD, 0);
 			break;
 		}
@@ -201,8 +194,7 @@ size_t parport_ieee1284_read_nibble (struct parport *port,
 					     PARPORT_STATUS_ACK,
 					     PARPORT_STATUS_ACK)) {
 			/* Timeout -- no more data? */
-			DPRINTK (KERN_DEBUG
-				 "%s: Nibble timeout at event 11\n",
+			pr_debug("%s: Nibble timeout at event 11\n",
 				 port->name);
 			break;
 		}
@@ -219,9 +211,8 @@ size_t parport_ieee1284_read_nibble (struct parport *port,
 		/* Read the last nibble without checking data avail. */
 		if (parport_read_status (port) & PARPORT_STATUS_ERROR) {
 		end_of_data:
-			DPRINTK (KERN_DEBUG
-				"%s: No more nibble data (%d bytes)\n",
-				port->name, i/2);
+			pr_debug("%s: No more nibble data (%d bytes)\n",
+				 port->name, i / 2);
 
 			/* Go to reverse idle phase. */
 			parport_frob_control (port,
@@ -272,8 +263,7 @@ size_t parport_ieee1284_read_byte (struct parport *port,
 			/* Timeout -- no more data? */
 			parport_frob_control (port, PARPORT_CONTROL_AUTOFD,
 						 0);
-			DPRINTK (KERN_DEBUG "%s: Byte timeout at event 9\n",
-				 port->name);
+			pr_debug("%s: Byte timeout at event 9\n", port->name);
 			break;
 		}
 
@@ -288,8 +278,7 @@ size_t parport_ieee1284_read_byte (struct parport *port,
 					     PARPORT_STATUS_ACK,
 					     PARPORT_STATUS_ACK)) {
 			/* Timeout -- no more data? */
-			DPRINTK (KERN_DEBUG "%s: Byte timeout at event 11\n",
-				 port->name);
+			pr_debug("%s: Byte timeout at event 11\n", port->name);
 			break;
 		}
 
@@ -307,8 +296,7 @@ size_t parport_ieee1284_read_byte (struct parport *port,
 		/* Read the last byte without checking data avail. */
 		if (parport_read_status (port) & PARPORT_STATUS_ERROR) {
 		end_of_data:
-			DPRINTK (KERN_DEBUG
-				 "%s: No more byte data (%zd bytes)\n",
+			pr_debug("%s: No more byte data (%zd bytes)\n",
 				 port->name, count);
 
 			/* Go to reverse idle phase. */
@@ -353,12 +341,10 @@ int ecp_forward_to_reverse (struct parport *port)
 					  PARPORT_STATUS_PAPEROUT, 0);
 
 	if (!retval) {
-		DPRINTK (KERN_DEBUG "%s: ECP direction: reverse\n",
-			 port->name);
+		pr_debug("%s: ECP direction: reverse\n", port->name);
 		port->ieee1284.phase = IEEE1284_PH_REV_IDLE;
 	} else {
-		DPRINTK (KERN_DEBUG "%s: ECP direction: failed to reverse\n",
-			 port->name);
+		pr_debug("%s: ECP direction: failed to reverse\n", port->name);
 		port->ieee1284.phase = IEEE1284_PH_ECP_DIR_UNKNOWN;
 	}
 
@@ -384,12 +370,10 @@ int ecp_reverse_to_forward (struct parport *port)
 
 	if (!retval) {
 		parport_data_forward (port);
-		DPRINTK (KERN_DEBUG "%s: ECP direction: forward\n",
-			 port->name);
+		pr_debug("%s: ECP direction: forward\n", port->name);
 		port->ieee1284.phase = IEEE1284_PH_FWD_IDLE;
 	} else {
-		DPRINTK (KERN_DEBUG
-			 "%s: ECP direction: failed to switch forward\n",
+		pr_debug("%s: ECP direction: failed to switch forward\n",
 			 port->name);
 		port->ieee1284.phase = IEEE1284_PH_ECP_DIR_UNKNOWN;
 	}
@@ -450,7 +434,7 @@ size_t parport_ieee1284_ecp_write_data (struct parport *port,
 		}
 
 		/* Time for Host Transfer Recovery (page 41 of IEEE1284) */
-		DPRINTK (KERN_DEBUG "%s: ECP transfer stalled!\n", port->name);
+		pr_debug("%s: ECP transfer stalled!\n", port->name);
 
 		parport_frob_control (port, PARPORT_CONTROL_INIT,
 				      PARPORT_CONTROL_INIT);
@@ -466,8 +450,7 @@ size_t parport_ieee1284_ecp_write_data (struct parport *port,
 		if (!(parport_read_status (port) & PARPORT_STATUS_PAPEROUT))
 			break;
 
-		DPRINTK (KERN_DEBUG "%s: Host transfer recovered\n",
-			 port->name);
+		pr_debug("%s: Host transfer recovered\n", port->name);
 
 		if (time_after_eq (jiffies, expire)) break;
 		goto try_again;
@@ -565,23 +548,20 @@ size_t parport_ieee1284_ecp_read_data (struct parport *port,
                    command or a normal data byte, don't accept it. */
 		if (command) {
 			if (byte & 0x80) {
-				DPRINTK (KERN_DEBUG "%s: stopping short at "
-					 "channel command (%02x)\n",
+				pr_debug("%s: stopping short at channel command (%02x)\n",
 					 port->name, byte);
 				goto out;
 			}
 			else if (port->ieee1284.mode != IEEE1284_MODE_ECPRLE)
-				DPRINTK (KERN_DEBUG "%s: device illegally "
-					 "using RLE; accepting anyway\n",
+				pr_debug("%s: device illegally using RLE; accepting anyway\n",
 					 port->name);
 
 			rle_count = byte + 1;
 
 			/* Are we allowed to read that many bytes? */
 			if (rle_count > (len - count)) {
-				DPRINTK (KERN_DEBUG "%s: leaving %d RLE bytes "
-					 "for next time\n", port->name,
-					 rle_count);
+				pr_debug("%s: leaving %d RLE bytes for next time\n",
+					 port->name, rle_count);
 				break;
 			}
 
@@ -596,7 +576,7 @@ size_t parport_ieee1284_ecp_read_data (struct parport *port,
 					     PARPORT_STATUS_ACK)) {
 			/* It's gone wrong.  Return what data we have
                            to the caller. */
-			DPRINTK (KERN_DEBUG "ECP read timed out at 45\n");
+			pr_debug("ECP read timed out at 45\n");
 
 			if (command)
 				printk (KERN_WARNING
@@ -620,7 +600,7 @@ size_t parport_ieee1284_ecp_read_data (struct parport *port,
 			memset (buf, byte, rle_count);
 			buf += rle_count;
 			count += rle_count;
-			DPRINTK (KERN_DEBUG "%s: decompressed to %d bytes\n",
+			pr_debug("%s: decompressed to %d bytes\n",
 				 port->name, rle_count);
 		} else {
 			/* Normal data byte. */
@@ -686,7 +666,7 @@ size_t parport_ieee1284_ecp_write_addr (struct parport *port,
 		}
 
 		/* Time for Host Transfer Recovery (page 41 of IEEE1284) */
-		DPRINTK (KERN_DEBUG "%s: ECP transfer stalled!\n", port->name);
+		pr_debug("%s: ECP transfer stalled!\n", port->name);
 
 		parport_frob_control (port, PARPORT_CONTROL_INIT,
 				      PARPORT_CONTROL_INIT);
@@ -702,8 +682,7 @@ size_t parport_ieee1284_ecp_write_addr (struct parport *port,
 		if (!(parport_read_status (port) & PARPORT_STATUS_PAPEROUT))
 			break;
 
-		DPRINTK (KERN_DEBUG "%s: Host transfer recovered\n",
-			 port->name);
+		pr_debug("%s: Host transfer recovered\n", port->name);
 
 		if (time_after_eq (jiffies, expire)) break;
 		goto try_again;

Re: [PATCH] parport: fix if-statement empty body warnings

[Randy Dunlap]

On 2/27/20 6:32 PM, Joe Perches wrote:
> On Thu, 2020-02-27 at 18:08 -0800, Randy Dunlap wrote:
>> From: Randy Dunlap <rdunlap@infradead.org>
>>
>> When debugging via DPRINTK() is not enabled, make the DPRINTK()
>> macro be an empty do-while block.
>>
>> This fixes gcc warnings when -Wextra is set:
>>
>> ../drivers/parport/ieee1284.c:262:18: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
>> ../drivers/parport/ieee1284.c:285:17: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
>> ../drivers/parport/ieee1284.c:298:17: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
>> ../drivers/parport/ieee1284_ops.c:576:18: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
>>
>> I have verified that there is no object code change (with gcc 7.5.0).
>>
>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>> Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
>> Cc: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
>> Cc: linux-parport@lists.infradead.org
>> ---
>>  drivers/parport/ieee1284.c     |    2 +-
>>  drivers/parport/ieee1284_ops.c |    2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> --- linux-next-20200225.orig/drivers/parport/ieee1284.c
>> +++ linux-next-20200225/drivers/parport/ieee1284.c
>> @@ -34,7 +34,7 @@
>>  #ifdef DEBUG
>>  #define DPRINTK(stuff...) printk (stuff)
>>  #else
>> -#define DPRINTK(stuff...)
>> +#define DPRINTK(stuff...) do {} while (0)
> 
> It's frequently better to use noprintk

Hi Joe,

I can't find that one. Oh, it's no_printk.


> And perhaps this should just become
> 
> #define DPRINTK pr_debug
> 
> or perhaps a conversion of DPRINTK uses


I'm OK with any of those.  If you would also fix ieee1284.c:

  CC [M]  drivers/parport/ieee1284.o
../drivers/parport/ieee1284.c: In function ‘parport_ieee1284_terminate’:
../drivers/parport/ieee1284.c:262:18: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
       port->name);
                  ^
../drivers/parport/ieee1284.c:285:17: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
      port->name);
                 ^
../drivers/parport/ieee1284.c:298:17: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
      port->name);
                 ^

you can add my ack to the patch:
Acked-by: Randy Dunlap <rdunlap@infradead.org>

Thanks.

> ---
>  drivers/parport/ieee1284_ops.c | 67 +++++++++++++++---------------------------
>  1 file changed, 23 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/parport/ieee1284_ops.c b/drivers/parport/ieee1284_ops.c
> index 5d41dd..b1c9f51 100644
> --- a/drivers/parport/ieee1284_ops.c
> +++ b/drivers/parport/ieee1284_ops.c
> @@ -27,12 +27,6 @@
>  #undef DEBUG /* Don't want a garbled console */
>  #endif
>  
> -#ifdef DEBUG
> -#define DPRINTK(stuff...) printk (stuff)
> -#else
> -#define DPRINTK(stuff...)
> -#endif
> -
>  /***                                *
>   * One-way data transfer functions. *
>   *                                ***/
> @@ -115,7 +109,7 @@ size_t parport_ieee1284_write_compat (struct parport *port,
>  		if (signal_pending (current))
>  			break;
>  
> -		DPRINTK (KERN_DEBUG "%s: Timed out\n", port->name);
> +		pr_debug("%s: Timed out\n", port->name);
>  		break;
>  
>  	ready:
> @@ -178,9 +172,8 @@ size_t parport_ieee1284_read_nibble (struct parport *port,
>  		if (parport_wait_peripheral (port,
>  					     PARPORT_STATUS_ACK, 0)) {
>  			/* Timeout -- no more data? */
> -			DPRINTK (KERN_DEBUG
> -				 "%s: Nibble timeout at event 9 (%d bytes)\n",
> -				 port->name, i/2);
> +			pr_debug("%s: Nibble timeout at event 9 (%d bytes)\n",
> +				 port->name, i / 2);
>  			parport_frob_control (port, PARPORT_CONTROL_AUTOFD, 0);
>  			break;
>  		}
> @@ -201,8 +194,7 @@ size_t parport_ieee1284_read_nibble (struct parport *port,
>  					     PARPORT_STATUS_ACK,
>  					     PARPORT_STATUS_ACK)) {
>  			/* Timeout -- no more data? */
> -			DPRINTK (KERN_DEBUG
> -				 "%s: Nibble timeout at event 11\n",
> +			pr_debug("%s: Nibble timeout at event 11\n",
>  				 port->name);
>  			break;
>  		}
> @@ -219,9 +211,8 @@ size_t parport_ieee1284_read_nibble (struct parport *port,
>  		/* Read the last nibble without checking data avail. */
>  		if (parport_read_status (port) & PARPORT_STATUS_ERROR) {
>  		end_of_data:
> -			DPRINTK (KERN_DEBUG
> -				"%s: No more nibble data (%d bytes)\n",
> -				port->name, i/2);
> +			pr_debug("%s: No more nibble data (%d bytes)\n",
> +				 port->name, i / 2);
>  
>  			/* Go to reverse idle phase. */
>  			parport_frob_control (port,
> @@ -272,8 +263,7 @@ size_t parport_ieee1284_read_byte (struct parport *port,
>  			/* Timeout -- no more data? */
>  			parport_frob_control (port, PARPORT_CONTROL_AUTOFD,
>  						 0);
> -			DPRINTK (KERN_DEBUG "%s: Byte timeout at event 9\n",
> -				 port->name);
> +			pr_debug("%s: Byte timeout at event 9\n", port->name);
>  			break;
>  		}
>  
> @@ -288,8 +278,7 @@ size_t parport_ieee1284_read_byte (struct parport *port,
>  					     PARPORT_STATUS_ACK,
>  					     PARPORT_STATUS_ACK)) {
>  			/* Timeout -- no more data? */
> -			DPRINTK (KERN_DEBUG "%s: Byte timeout at event 11\n",
> -				 port->name);
> +			pr_debug("%s: Byte timeout at event 11\n", port->name);
>  			break;
>  		}
>  
> @@ -307,8 +296,7 @@ size_t parport_ieee1284_read_byte (struct parport *port,
>  		/* Read the last byte without checking data avail. */
>  		if (parport_read_status (port) & PARPORT_STATUS_ERROR) {
>  		end_of_data:
> -			DPRINTK (KERN_DEBUG
> -				 "%s: No more byte data (%zd bytes)\n",
> +			pr_debug("%s: No more byte data (%zd bytes)\n",
>  				 port->name, count);
>  
>  			/* Go to reverse idle phase. */
> @@ -353,12 +341,10 @@ int ecp_forward_to_reverse (struct parport *port)
>  					  PARPORT_STATUS_PAPEROUT, 0);
>  
>  	if (!retval) {
> -		DPRINTK (KERN_DEBUG "%s: ECP direction: reverse\n",
> -			 port->name);
> +		pr_debug("%s: ECP direction: reverse\n", port->name);
>  		port->ieee1284.phase = IEEE1284_PH_REV_IDLE;
>  	} else {
> -		DPRINTK (KERN_DEBUG "%s: ECP direction: failed to reverse\n",
> -			 port->name);
> +		pr_debug("%s: ECP direction: failed to reverse\n", port->name);
>  		port->ieee1284.phase = IEEE1284_PH_ECP_DIR_UNKNOWN;
>  	}
>  
> @@ -384,12 +370,10 @@ int ecp_reverse_to_forward (struct parport *port)
>  
>  	if (!retval) {
>  		parport_data_forward (port);
> -		DPRINTK (KERN_DEBUG "%s: ECP direction: forward\n",
> -			 port->name);
> +		pr_debug("%s: ECP direction: forward\n", port->name);
>  		port->ieee1284.phase = IEEE1284_PH_FWD_IDLE;
>  	} else {
> -		DPRINTK (KERN_DEBUG
> -			 "%s: ECP direction: failed to switch forward\n",
> +		pr_debug("%s: ECP direction: failed to switch forward\n",
>  			 port->name);
>  		port->ieee1284.phase = IEEE1284_PH_ECP_DIR_UNKNOWN;
>  	}
> @@ -450,7 +434,7 @@ size_t parport_ieee1284_ecp_write_data (struct parport *port,
>  		}
>  
>  		/* Time for Host Transfer Recovery (page 41 of IEEE1284) */
> -		DPRINTK (KERN_DEBUG "%s: ECP transfer stalled!\n", port->name);
> +		pr_debug("%s: ECP transfer stalled!\n", port->name);
>  
>  		parport_frob_control (port, PARPORT_CONTROL_INIT,
>  				      PARPORT_CONTROL_INIT);
> @@ -466,8 +450,7 @@ size_t parport_ieee1284_ecp_write_data (struct parport *port,
>  		if (!(parport_read_status (port) & PARPORT_STATUS_PAPEROUT))
>  			break;
>  
> -		DPRINTK (KERN_DEBUG "%s: Host transfer recovered\n",
> -			 port->name);
> +		pr_debug("%s: Host transfer recovered\n", port->name);
>  
>  		if (time_after_eq (jiffies, expire)) break;
>  		goto try_again;
> @@ -565,23 +548,20 @@ size_t parport_ieee1284_ecp_read_data (struct parport *port,
>                     command or a normal data byte, don't accept it. */
>  		if (command) {
>  			if (byte & 0x80) {
> -				DPRINTK (KERN_DEBUG "%s: stopping short at "
> -					 "channel command (%02x)\n",
> +				pr_debug("%s: stopping short at channel command (%02x)\n",
>  					 port->name, byte);
>  				goto out;
>  			}
>  			else if (port->ieee1284.mode != IEEE1284_MODE_ECPRLE)
> -				DPRINTK (KERN_DEBUG "%s: device illegally "
> -					 "using RLE; accepting anyway\n",
> +				pr_debug("%s: device illegally using RLE; accepting anyway\n",
>  					 port->name);
>  
>  			rle_count = byte + 1;
>  
>  			/* Are we allowed to read that many bytes? */
>  			if (rle_count > (len - count)) {
> -				DPRINTK (KERN_DEBUG "%s: leaving %d RLE bytes "
> -					 "for next time\n", port->name,
> -					 rle_count);
> +				pr_debug("%s: leaving %d RLE bytes for next time\n",
> +					 port->name, rle_count);
>  				break;
>  			}
>  
> @@ -596,7 +576,7 @@ size_t parport_ieee1284_ecp_read_data (struct parport *port,
>  					     PARPORT_STATUS_ACK)) {
>  			/* It's gone wrong.  Return what data we have
>                             to the caller. */
> -			DPRINTK (KERN_DEBUG "ECP read timed out at 45\n");
> +			pr_debug("ECP read timed out at 45\n");
>  
>  			if (command)
>  				printk (KERN_WARNING
> @@ -620,7 +600,7 @@ size_t parport_ieee1284_ecp_read_data (struct parport *port,
>  			memset (buf, byte, rle_count);
>  			buf += rle_count;
>  			count += rle_count;
> -			DPRINTK (KERN_DEBUG "%s: decompressed to %d bytes\n",
> +			pr_debug("%s: decompressed to %d bytes\n",
>  				 port->name, rle_count);
>  		} else {
>  			/* Normal data byte. */
> @@ -686,7 +666,7 @@ size_t parport_ieee1284_ecp_write_addr (struct parport *port,
>  		}
>  
>  		/* Time for Host Transfer Recovery (page 41 of IEEE1284) */
> -		DPRINTK (KERN_DEBUG "%s: ECP transfer stalled!\n", port->name);
> +		pr_debug("%s: ECP transfer stalled!\n", port->name);
>  
>  		parport_frob_control (port, PARPORT_CONTROL_INIT,
>  				      PARPORT_CONTROL_INIT);
> @@ -702,8 +682,7 @@ size_t parport_ieee1284_ecp_write_addr (struct parport *port,
>  		if (!(parport_read_status (port) & PARPORT_STATUS_PAPEROUT))
>  			break;
>  
> -		DPRINTK (KERN_DEBUG "%s: Host transfer recovered\n",
> -			 port->name);
> +		pr_debug("%s: Host transfer recovered\n", port->name);
>  
>  		if (time_after_eq (jiffies, expire)) break;
>  		goto try_again;
> 
> 


-- 
~Randy
Reported-by: Randy Dunlap <rdunlap@infradead.org>

Re: [PATCH] parport: fix if-statement empty body warnings

[Joe Perches]

Eliminate warnings by using pr_debug which is the more typical
kernel debugging style and also enable dynamic_debug on these
outputs.

Miscellaneous:

o A few messages were logged at KERN_INFO when enabled, now KERN_DEBUG
o Convert %d/%d to %zd/%zu to avoid compilation warnings

Original-patch-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Joe Perches <joe@perches.com>
Acked-by: Randy Dunlap <rdunlap@infradead.org>
---
 drivers/parport/ieee1284.c     | 90 ++++++++++++++++--------------------------
 drivers/parport/ieee1284_ops.c | 67 +++++++++++--------------------
 2 files changed, 57 insertions(+), 100 deletions(-)

diff --git a/drivers/parport/ieee1284.c b/drivers/parport/ieee1284.c
index 90fb735..ba562bc 100644
--- a/drivers/parport/ieee1284.c
+++ b/drivers/parport/ieee1284.c
@@ -31,12 +31,6 @@
 #undef DEBUG /* Don't want a garbled console */
 #endif
 
-#ifdef DEBUG
-#define DPRINTK(stuff...) printk (stuff)
-#else
-#define DPRINTK(stuff...)
-#endif
-
 /* Make parport_wait_peripheral wake up.
  * It will be useful to call this from an interrupt handler. */
 static void parport_ieee1284_wakeup (struct parport *port)
@@ -258,12 +252,11 @@ static void parport_ieee1284_terminate (struct parport *port)
 						     PARPORT_STATUS_PAPEROUT,
 						     PARPORT_STATUS_PAPEROUT);
 			if (r)
-				DPRINTK (KERN_INFO "%s: Timeout at event 49\n",
+				pr_debug("%s: Timeout at event 49\n",
 					 port->name);
 
 			parport_data_forward (port);
-			DPRINTK (KERN_DEBUG "%s: ECP direction: forward\n",
-				 port->name);
+			pr_debug("%s: ECP direction: forward\n", port->name);
 			port->ieee1284.phase = IEEE1284_PH_FWD_IDLE;
 		}
 
@@ -281,8 +274,7 @@ static void parport_ieee1284_terminate (struct parport *port)
 		/* Event 24: nAck goes low */
 		r = parport_wait_peripheral (port, PARPORT_STATUS_ACK, 0);
 		if (r)
-			DPRINTK (KERN_INFO "%s: Timeout at event 24\n",
-				 port->name);
+			pr_debug("%s: Timeout at event 24\n", port->name);
 
 		/* Event 25: Set nAutoFd low */
 		parport_frob_control (port,
@@ -294,8 +286,7 @@ static void parport_ieee1284_terminate (struct parport *port)
 					     PARPORT_STATUS_ACK, 
 					     PARPORT_STATUS_ACK);
 		if (r)
-			DPRINTK (KERN_INFO "%s: Timeout at event 27\n",
-				 port->name);
+			pr_debug("%s: Timeout at event 27\n", port->name);
 
 		/* Event 29: Set nAutoFd high */
 		parport_frob_control (port, PARPORT_CONTROL_AUTOFD, 0);
@@ -304,8 +295,7 @@ static void parport_ieee1284_terminate (struct parport *port)
 	port->ieee1284.mode = IEEE1284_MODE_COMPAT;
 	port->ieee1284.phase = IEEE1284_PH_FWD_IDLE;
 
-	DPRINTK (KERN_DEBUG "%s: In compatibility (forward idle) mode\n",
-		 port->name);
+	pr_debug("%s: In compatibility (forward idle) mode\n", port->name);
 }		
 #endif /* IEEE1284 support */
 
@@ -406,8 +396,7 @@ int parport_negotiate (struct parport *port, int mode)
 				      PARPORT_CONTROL_SELECT
 				      | PARPORT_CONTROL_AUTOFD,
 				      PARPORT_CONTROL_SELECT);
-		DPRINTK (KERN_DEBUG
-			 "%s: Peripheral not IEEE1284 compliant (0x%02X)\n",
+		pr_debug("%s: Peripheral not IEEE1284 compliant (0x%02X)\n",
 			 port->name, parport_read_status (port));
 		port->ieee1284.phase = IEEE1284_PH_FWD_IDLE;
 		return -1; /* Not IEEE1284 compliant */
@@ -430,8 +419,7 @@ int parport_negotiate (struct parport *port, int mode)
 				     PARPORT_STATUS_ACK,
 				     PARPORT_STATUS_ACK)) {
 		/* This shouldn't really happen with a compliant device. */
-		DPRINTK (KERN_DEBUG
-			 "%s: Mode 0x%02x not supported? (0x%02x)\n",
+		pr_debug("%s: Mode 0x%02x not supported? (0x%02x)\n",
 			 port->name, mode, port->ops->read_status (port));
 		parport_ieee1284_terminate (port);
 		return 1;
@@ -442,7 +430,7 @@ int parport_negotiate (struct parport *port, int mode)
 	/* xflag should be high for all modes other than nibble (0). */
 	if (mode && !xflag) {
 		/* Mode not supported. */
-		DPRINTK (KERN_DEBUG "%s: Mode 0x%02x rejected by peripheral\n",
+		pr_debug("%s: Mode 0x%02x rejected by peripheral\n",
 			 port->name, mode);
 		parport_ieee1284_terminate (port);
 		return 1;
@@ -463,9 +451,7 @@ int parport_negotiate (struct parport *port, int mode)
 		/* Event 52: nAck goes low */
 		if (parport_wait_peripheral (port, PARPORT_STATUS_ACK, 0)) {
 			/* This peripheral is _very_ slow. */
-			DPRINTK (KERN_DEBUG
-				 "%s: Event 52 didn't happen\n",
-				 port->name);
+			pr_debug("%s: Event 52 didn't happen\n", port->name);
 			parport_ieee1284_terminate (port);
 			return 1;
 		}
@@ -481,10 +467,9 @@ int parport_negotiate (struct parport *port, int mode)
 					     PARPORT_STATUS_ACK)) {
 			/* This shouldn't really happen with a compliant
 			 * device. */
-			DPRINTK (KERN_DEBUG
-				 "%s: Mode 0x%02x not supported? (0x%02x)\n",
+			pr_debug("%s: Mode 0x%02x not supported? (0x%02x)\n",
 				 port->name, mode,
-				 port->ops->read_status (port));
+				 port->ops->read_status(port));
 			parport_ieee1284_terminate (port);
 			return 1;
 		}
@@ -495,8 +480,8 @@ int parport_negotiate (struct parport *port, int mode)
 		/* xflag should be high. */
 		if (!xflag) {
 			/* Extended mode not supported. */
-			DPRINTK (KERN_DEBUG "%s: Extended mode 0x%02x not "
-				 "supported\n", port->name, mode);
+			pr_debug("%s: Extended mode 0x%02x not supported\n",
+				 port->name, mode);
 			parport_ieee1284_terminate (port);
 			return 1;
 		}
@@ -505,7 +490,7 @@ int parport_negotiate (struct parport *port, int mode)
 	}
 
 	/* Mode is supported */
-	DPRINTK (KERN_DEBUG "%s: In mode 0x%02x\n", port->name, mode);
+	pr_debug("%s: In mode 0x%02x\n", port->name, mode);
 	port->ieee1284.mode = mode;
 
 	/* But ECP is special */
@@ -522,13 +507,11 @@ int parport_negotiate (struct parport *port, int mode)
 					     PARPORT_STATUS_PAPEROUT,
 					     PARPORT_STATUS_PAPEROUT);
 		if (r) {
-			DPRINTK (KERN_INFO "%s: Timeout at event 31\n",
-				port->name);
+			pr_debug("%s: Timeout at event 31\n", port->name);
 		}
 
 		port->ieee1284.phase = IEEE1284_PH_FWD_IDLE;
-		DPRINTK (KERN_DEBUG "%s: ECP direction: forward\n",
-			 port->name);
+		pr_debug("%s: ECP direction: forward\n", port->name);
 	} else switch (mode) {
 	case IEEE1284_MODE_NIBBLE:
 	case IEEE1284_MODE_BYTE:
@@ -573,7 +556,7 @@ void parport_ieee1284_interrupt (void *handle)
 	if (port->ieee1284.phase == IEEE1284_PH_REV_IDLE) {
 		/* An interrupt in this phase means that data
 		 * is now available. */
-		DPRINTK (KERN_DEBUG "%s: Data available\n", port->name);
+		pr_debug("%s: Data available\n", port->name);
 		parport_ieee1284_ack_data_avail (port);
 	}
 #endif /* IEEE1284 support */
@@ -617,13 +600,12 @@ ssize_t parport_write (struct parport *port, const void *buffer, size_t len)
 		parport_negotiate (port, IEEE1284_MODE_COMPAT);
 		/* fall through */
 	case IEEE1284_MODE_COMPAT:
-		DPRINTK (KERN_DEBUG "%s: Using compatibility mode\n",
-			 port->name);
+		pr_debug("%s: Using compatibility mode\n", port->name);
 		fn = port->ops->compat_write_data;
 		break;
 
 	case IEEE1284_MODE_EPP:
-		DPRINTK (KERN_DEBUG "%s: Using EPP mode\n", port->name);
+		pr_debug("%s: Using EPP mode\n", port->name);
 		if (addr) {
 			fn = port->ops->epp_write_addr;
 		} else {
@@ -631,8 +613,7 @@ ssize_t parport_write (struct parport *port, const void *buffer, size_t len)
 		}
 		break;
 	case IEEE1284_MODE_EPPSWE:
-		DPRINTK (KERN_DEBUG "%s: Using software-emulated EPP mode\n",
-			port->name);
+		pr_debug("%s: Using software-emulated EPP mode\n", port->name);
 		if (addr) {
 			fn = parport_ieee1284_epp_write_addr;
 		} else {
@@ -641,7 +622,7 @@ ssize_t parport_write (struct parport *port, const void *buffer, size_t len)
 		break;
 	case IEEE1284_MODE_ECP:
 	case IEEE1284_MODE_ECPRLE:
-		DPRINTK (KERN_DEBUG "%s: Using ECP mode\n", port->name);
+		pr_debug("%s: Using ECP mode\n", port->name);
 		if (addr) {
 			fn = port->ops->ecp_write_addr;
 		} else {
@@ -650,8 +631,7 @@ ssize_t parport_write (struct parport *port, const void *buffer, size_t len)
 		break;
 
 	case IEEE1284_MODE_ECPSWE:
-		DPRINTK (KERN_DEBUG "%s: Using software-emulated ECP mode\n",
-			 port->name);
+		pr_debug("%s: Using software-emulated ECP mode\n", port->name);
 		/* The caller has specified that it must be emulated,
 		 * even if we have ECP hardware! */
 		if (addr) {
@@ -662,13 +642,13 @@ ssize_t parport_write (struct parport *port, const void *buffer, size_t len)
 		break;
 
 	default:
-		DPRINTK (KERN_DEBUG "%s: Unknown mode 0x%02x\n", port->name,
-			port->ieee1284.mode);
+		pr_debug("%s: Unknown mode 0x%02x\n",
+			 port->name, port->ieee1284.mode);
 		return -ENOSYS;
 	}
 
 	retval = (*fn) (port, buffer, len, 0);
-	DPRINTK (KERN_DEBUG "%s: wrote %d/%d bytes\n", port->name, retval, len);
+	pr_debug("%s: wrote %zd/%zu bytes\n", port->name, retval, len);
 	return retval;
 #endif /* IEEE1284 support */
 }
@@ -715,7 +695,7 @@ ssize_t parport_read (struct parport *port, void *buffer, size_t len)
 		if ((port->physport->modes & PARPORT_MODE_TRISTATE) &&
 		    !parport_negotiate (port, IEEE1284_MODE_BYTE)) {
 			/* got into BYTE mode OK */
-			DPRINTK (KERN_DEBUG "%s: Using byte mode\n", port->name);
+			pr_debug("%s: Using byte mode\n", port->name);
 			fn = port->ops->byte_read_data;
 			break;
 		}
@@ -724,17 +704,17 @@ ssize_t parport_read (struct parport *port, void *buffer, size_t len)
 		}
 		/* fall through - to NIBBLE */
 	case IEEE1284_MODE_NIBBLE:
-		DPRINTK (KERN_DEBUG "%s: Using nibble mode\n", port->name);
+		pr_debug("%s: Using nibble mode\n", port->name);
 		fn = port->ops->nibble_read_data;
 		break;
 
 	case IEEE1284_MODE_BYTE:
-		DPRINTK (KERN_DEBUG "%s: Using byte mode\n", port->name);
+		pr_debug("%s: Using byte mode\n", port->name);
 		fn = port->ops->byte_read_data;
 		break;
 
 	case IEEE1284_MODE_EPP:
-		DPRINTK (KERN_DEBUG "%s: Using EPP mode\n", port->name);
+		pr_debug("%s: Using EPP mode\n", port->name);
 		if (addr) {
 			fn = port->ops->epp_read_addr;
 		} else {
@@ -742,8 +722,7 @@ ssize_t parport_read (struct parport *port, void *buffer, size_t len)
 		}
 		break;
 	case IEEE1284_MODE_EPPSWE:
-		DPRINTK (KERN_DEBUG "%s: Using software-emulated EPP mode\n",
-			port->name);
+		pr_debug("%s: Using software-emulated EPP mode\n", port->name);
 		if (addr) {
 			fn = parport_ieee1284_epp_read_addr;
 		} else {
@@ -752,19 +731,18 @@ ssize_t parport_read (struct parport *port, void *buffer, size_t len)
 		break;
 	case IEEE1284_MODE_ECP:
 	case IEEE1284_MODE_ECPRLE:
-		DPRINTK (KERN_DEBUG "%s: Using ECP mode\n", port->name);
+		pr_debug("%s: Using ECP mode\n", port->name);
 		fn = port->ops->ecp_read_data;
 		break;
 
 	case IEEE1284_MODE_ECPSWE:
-		DPRINTK (KERN_DEBUG "%s: Using software-emulated ECP mode\n",
-			 port->name);
+		pr_debug("%s: Using software-emulated ECP mode\n", port->name);
 		fn = parport_ieee1284_ecp_read_data;
 		break;
 
 	default:
-		DPRINTK (KERN_DEBUG "%s: Unknown mode 0x%02x\n", port->name,
-			 port->physport->ieee1284.mode);
+		pr_debug("%s: Unknown mode 0x%02x\n",
+			 port->name, port->physport->ieee1284.mode);
 		return -ENOSYS;
 	}
 
diff --git a/drivers/parport/ieee1284_ops.c b/drivers/parport/ieee1284_ops.c
index 5d41dd..b1c9f51 100644
--- a/drivers/parport/ieee1284_ops.c
+++ b/drivers/parport/ieee1284_ops.c
@@ -27,12 +27,6 @@
 #undef DEBUG /* Don't want a garbled console */
 #endif
 
-#ifdef DEBUG
-#define DPRINTK(stuff...) printk (stuff)
-#else
-#define DPRINTK(stuff...)
-#endif
-
 /***                                *
  * One-way data transfer functions. *
  *                                ***/
@@ -115,7 +109,7 @@ size_t parport_ieee1284_write_compat (struct parport *port,
 		if (signal_pending (current))
 			break;
 
-		DPRINTK (KERN_DEBUG "%s: Timed out\n", port->name);
+		pr_debug("%s: Timed out\n", port->name);
 		break;
 
 	ready:
@@ -178,9 +172,8 @@ size_t parport_ieee1284_read_nibble (struct parport *port,
 		if (parport_wait_peripheral (port,
 					     PARPORT_STATUS_ACK, 0)) {
 			/* Timeout -- no more data? */
-			DPRINTK (KERN_DEBUG
-				 "%s: Nibble timeout at event 9 (%d bytes)\n",
-				 port->name, i/2);
+			pr_debug("%s: Nibble timeout at event 9 (%d bytes)\n",
+				 port->name, i / 2);
 			parport_frob_control (port, PARPORT_CONTROL_AUTOFD, 0);
 			break;
 		}
@@ -201,8 +194,7 @@ size_t parport_ieee1284_read_nibble (struct parport *port,
 					     PARPORT_STATUS_ACK,
 					     PARPORT_STATUS_ACK)) {
 			/* Timeout -- no more data? */
-			DPRINTK (KERN_DEBUG
-				 "%s: Nibble timeout at event 11\n",
+			pr_debug("%s: Nibble timeout at event 11\n",
 				 port->name);
 			break;
 		}
@@ -219,9 +211,8 @@ size_t parport_ieee1284_read_nibble (struct parport *port,
 		/* Read the last nibble without checking data avail. */
 		if (parport_read_status (port) & PARPORT_STATUS_ERROR) {
 		end_of_data:
-			DPRINTK (KERN_DEBUG
-				"%s: No more nibble data (%d bytes)\n",
-				port->name, i/2);
+			pr_debug("%s: No more nibble data (%d bytes)\n",
+				 port->name, i / 2);
 
 			/* Go to reverse idle phase. */
 			parport_frob_control (port,
@@ -272,8 +263,7 @@ size_t parport_ieee1284_read_byte (struct parport *port,
 			/* Timeout -- no more data? */
 			parport_frob_control (port, PARPORT_CONTROL_AUTOFD,
 						 0);
-			DPRINTK (KERN_DEBUG "%s: Byte timeout at event 9\n",
-				 port->name);
+			pr_debug("%s: Byte timeout at event 9\n", port->name);
 			break;
 		}
 
@@ -288,8 +278,7 @@ size_t parport_ieee1284_read_byte (struct parport *port,
 					     PARPORT_STATUS_ACK,
 					     PARPORT_STATUS_ACK)) {
 			/* Timeout -- no more data? */
-			DPRINTK (KERN_DEBUG "%s: Byte timeout at event 11\n",
-				 port->name);
+			pr_debug("%s: Byte timeout at event 11\n", port->name);
 			break;
 		}
 
@@ -307,8 +296,7 @@ size_t parport_ieee1284_read_byte (struct parport *port,
 		/* Read the last byte without checking data avail. */
 		if (parport_read_status (port) & PARPORT_STATUS_ERROR) {
 		end_of_data:
-			DPRINTK (KERN_DEBUG
-				 "%s: No more byte data (%zd bytes)\n",
+			pr_debug("%s: No more byte data (%zd bytes)\n",
 				 port->name, count);
 
 			/* Go to reverse idle phase. */
@@ -353,12 +341,10 @@ int ecp_forward_to_reverse (struct parport *port)
 					  PARPORT_STATUS_PAPEROUT, 0);
 
 	if (!retval) {
-		DPRINTK (KERN_DEBUG "%s: ECP direction: reverse\n",
-			 port->name);
+		pr_debug("%s: ECP direction: reverse\n", port->name);
 		port->ieee1284.phase = IEEE1284_PH_REV_IDLE;
 	} else {
-		DPRINTK (KERN_DEBUG "%s: ECP direction: failed to reverse\n",
-			 port->name);
+		pr_debug("%s: ECP direction: failed to reverse\n", port->name);
 		port->ieee1284.phase = IEEE1284_PH_ECP_DIR_UNKNOWN;
 	}
 
@@ -384,12 +370,10 @@ int ecp_reverse_to_forward (struct parport *port)
 
 	if (!retval) {
 		parport_data_forward (port);
-		DPRINTK (KERN_DEBUG "%s: ECP direction: forward\n",
-			 port->name);
+		pr_debug("%s: ECP direction: forward\n", port->name);
 		port->ieee1284.phase = IEEE1284_PH_FWD_IDLE;
 	} else {
-		DPRINTK (KERN_DEBUG
-			 "%s: ECP direction: failed to switch forward\n",
+		pr_debug("%s: ECP direction: failed to switch forward\n",
 			 port->name);
 		port->ieee1284.phase = IEEE1284_PH_ECP_DIR_UNKNOWN;
 	}
@@ -450,7 +434,7 @@ size_t parport_ieee1284_ecp_write_data (struct parport *port,
 		}
 
 		/* Time for Host Transfer Recovery (page 41 of IEEE1284) */
-		DPRINTK (KERN_DEBUG "%s: ECP transfer stalled!\n", port->name);
+		pr_debug("%s: ECP transfer stalled!\n", port->name);
 
 		parport_frob_control (port, PARPORT_CONTROL_INIT,
 				      PARPORT_CONTROL_INIT);
@@ -466,8 +450,7 @@ size_t parport_ieee1284_ecp_write_data (struct parport *port,
 		if (!(parport_read_status (port) & PARPORT_STATUS_PAPEROUT))
 			break;
 
-		DPRINTK (KERN_DEBUG "%s: Host transfer recovered\n",
-			 port->name);
+		pr_debug("%s: Host transfer recovered\n", port->name);
 
 		if (time_after_eq (jiffies, expire)) break;
 		goto try_again;
@@ -565,23 +548,20 @@ size_t parport_ieee1284_ecp_read_data (struct parport *port,
                    command or a normal data byte, don't accept it. */
 		if (command) {
 			if (byte & 0x80) {
-				DPRINTK (KERN_DEBUG "%s: stopping short at "
-					 "channel command (%02x)\n",
+				pr_debug("%s: stopping short at channel command (%02x)\n",
 					 port->name, byte);
 				goto out;
 			}
 			else if (port->ieee1284.mode != IEEE1284_MODE_ECPRLE)
-				DPRINTK (KERN_DEBUG "%s: device illegally "
-					 "using RLE; accepting anyway\n",
+				pr_debug("%s: device illegally using RLE; accepting anyway\n",
 					 port->name);
 
 			rle_count = byte + 1;
 
 			/* Are we allowed to read that many bytes? */
 			if (rle_count > (len - count)) {
-				DPRINTK (KERN_DEBUG "%s: leaving %d RLE bytes "
-					 "for next time\n", port->name,
-					 rle_count);
+				pr_debug("%s: leaving %d RLE bytes for next time\n",
+					 port->name, rle_count);
 				break;
 			}
 
@@ -596,7 +576,7 @@ size_t parport_ieee1284_ecp_read_data (struct parport *port,
 					     PARPORT_STATUS_ACK)) {
 			/* It's gone wrong.  Return what data we have
                            to the caller. */
-			DPRINTK (KERN_DEBUG "ECP read timed out at 45\n");
+			pr_debug("ECP read timed out at 45\n");
 
 			if (command)
 				printk (KERN_WARNING
@@ -620,7 +600,7 @@ size_t parport_ieee1284_ecp_read_data (struct parport *port,
 			memset (buf, byte, rle_count);
 			buf += rle_count;
 			count += rle_count;
-			DPRINTK (KERN_DEBUG "%s: decompressed to %d bytes\n",
+			pr_debug("%s: decompressed to %d bytes\n",
 				 port->name, rle_count);
 		} else {
 			/* Normal data byte. */
@@ -686,7 +666,7 @@ size_t parport_ieee1284_ecp_write_addr (struct parport *port,
 		}
 
 		/* Time for Host Transfer Recovery (page 41 of IEEE1284) */
-		DPRINTK (KERN_DEBUG "%s: ECP transfer stalled!\n", port->name);
+		pr_debug("%s: ECP transfer stalled!\n", port->name);
 
 		parport_frob_control (port, PARPORT_CONTROL_INIT,
 				      PARPORT_CONTROL_INIT);
@@ -702,8 +682,7 @@ size_t parport_ieee1284_ecp_write_addr (struct parport *port,
 		if (!(parport_read_status (port) & PARPORT_STATUS_PAPEROUT))
 			break;
 
-		DPRINTK (KERN_DEBUG "%s: Host transfer recovered\n",
-			 port->name);
+		pr_debug("%s: Host transfer recovered\n", port->name);
 
 		if (time_after_eq (jiffies, expire)) break;
 		goto try_again;

Re: [PATCH] parport: fix if-statement empty body warnings

[Randy Dunlap]

On 2/27/20 7:19 PM, Joe Perches wrote:
> Eliminate warnings by using pr_debug which is the more typical
> kernel debugging style and also enable dynamic_debug on these
> outputs.
> 
> Miscellaneous:
> 
> o A few messages were logged at KERN_INFO when enabled, now KERN_DEBUG
> o Convert %d/%d to %zd/%zu to avoid compilation warnings
> 
> Original-patch-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Joe Perches <joe@perches.com>
> Acked-by: Randy Dunlap <rdunlap@infradead.org>
> ---
>  drivers/parport/ieee1284.c     | 90 ++++++++++++++++--------------------------
>  drivers/parport/ieee1284_ops.c | 67 +++++++++++--------------------
>  2 files changed, 57 insertions(+), 100 deletions(-)

Thanks, Joe.

-- 
~Randy

[PATCH 0/7] parport: Use generic kernel logging styles

[Joe Perches]

Well, if the parport logging is getting some generic fixing,
here's some more generic logging fixing...

Joe Perches (7):
  parport: Convert printk(KERN_<LEVEL> to pr_<level>(
  parport: Use more comon logging styles
  parport: daisy: Convert DPRINTK to pr_debug
  parport_amiga: Convert DPRINTK to pr_debug
  parport_mfc3: Convert DPRINTK to pr_debug
  parport_pc: Convert DPRINTK to pr_debug
  parport: Standardize use of printmode

 drivers/parport/daisy.c          |  29 +---
 drivers/parport/ieee1284.c       |   4 +-
 drivers/parport/ieee1284_ops.c   |   3 +-
 drivers/parport/parport_amiga.c  |  22 +--
 drivers/parport/parport_atari.c  |   2 +-
 drivers/parport/parport_cs.c     |   6 +-
 drivers/parport/parport_gsc.c    |  25 +--
 drivers/parport/parport_gsc.h    |  21 ++-
 drivers/parport/parport_ip32.c   | 117 +++++++-------
 drivers/parport/parport_mfc3.c   |  21 +--
 drivers/parport/parport_pc.c     | 263 +++++++++++++------------------
 drivers/parport/parport_sunbpp.c |   2 +-
 drivers/parport/probe.c          |  34 ++--
 drivers/parport/procfs.c         |   6 +-
 drivers/parport/share.c          |  37 +++--
 15 files changed, 261 insertions(+), 331 deletions(-)

-- 
2.24.0

[PATCH 1/7] parport: Convert printk(KERN_<LEVEL> to pr_<level>(

[Joe Perches]

Use the more common kernel style.

Miscellanea:

o Coalesce formats
o Realign arguments

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/parport/daisy.c          |   6 +-
 drivers/parport/ieee1284.c       |   4 +-
 drivers/parport/ieee1284_ops.c   |   3 +-
 drivers/parport/parport_amiga.c  |   2 +-
 drivers/parport/parport_atari.c  |   2 +-
 drivers/parport/parport_cs.c     |   6 +-
 drivers/parport/parport_gsc.c    |   7 +-
 drivers/parport/parport_ip32.c   |  25 ++---
 drivers/parport/parport_mfc3.c   |   2 +-
 drivers/parport/parport_pc.c     | 166 +++++++++++++------------------
 drivers/parport/parport_sunbpp.c |   2 +-
 drivers/parport/probe.c          |   7 +-
 drivers/parport/share.c          |  24 ++---
 13 files changed, 110 insertions(+), 146 deletions(-)

diff --git a/drivers/parport/daisy.c b/drivers/parport/daisy.c
index 3b00e2c..f87cc80 100644
--- a/drivers/parport/daisy.c
+++ b/drivers/parport/daisy.c
@@ -145,8 +145,7 @@ int parport_daisy_init(struct parport *port)
 	    ((num_ports = num_mux_ports(port)) == 2 || num_ports == 4)) {
 		/* Leave original as port zero. */
 		port->muxport = 0;
-		printk(KERN_INFO
-			"%s: 1st (default) port of %d-way multiplexor\n",
+		pr_info("%s: 1st (default) port of %d-way multiplexor\n",
 			port->name, num_ports);
 		for (i = 1; i < num_ports; i++) {
 			/* Clone the port. */
@@ -159,8 +158,7 @@ int parport_daisy_init(struct parport *port)
 				continue;
 			}
 
-			printk(KERN_INFO
-				"%s: %d%s port of %d-way multiplexor on %s\n",
+			pr_info("%s: %d%s port of %d-way multiplexor on %s\n",
 				extra->name, i + 1, th[i + 1], num_ports,
 				port->name);
 
diff --git a/drivers/parport/ieee1284.c b/drivers/parport/ieee1284.c
index ba562bc..f28d6a3 100644
--- a/drivers/parport/ieee1284.c
+++ b/drivers/parport/ieee1284.c
@@ -319,7 +319,7 @@ int parport_negotiate (struct parport *port, int mode)
 #ifndef CONFIG_PARPORT_1284
 	if (mode == IEEE1284_MODE_COMPAT)
 		return 0;
-	printk (KERN_ERR "parport: IEEE1284 not supported in this kernel\n");
+	pr_err("parport: IEEE1284 not supported in this kernel\n");
 	return -1;
 #else
 	int m = mode & ~IEEE1284_ADDR;
@@ -674,7 +674,7 @@ ssize_t parport_write (struct parport *port, const void *buffer, size_t len)
 ssize_t parport_read (struct parport *port, void *buffer, size_t len)
 {
 #ifndef CONFIG_PARPORT_1284
-	printk (KERN_ERR "parport: IEEE1284 not supported in this kernel\n");
+	pr_err("parport: IEEE1284 not supported in this kernel\n");
 	return -ENODEV;
 #else
 	int mode = port->physport->ieee1284.mode;
diff --git a/drivers/parport/ieee1284_ops.c b/drivers/parport/ieee1284_ops.c
index b1c9f51..2c11bd 100644
--- a/drivers/parport/ieee1284_ops.c
+++ b/drivers/parport/ieee1284_ops.c
@@ -579,8 +579,7 @@ size_t parport_ieee1284_ecp_read_data (struct parport *port,
 			pr_debug("ECP read timed out at 45\n");
 
 			if (command)
-				printk (KERN_WARNING
-					"%s: command ignored (%02x)\n",
+				pr_warn("%s: command ignored (%02x)\n",
 					port->name, byte);
 
 			break;
diff --git a/drivers/parport/parport_amiga.c b/drivers/parport/parport_amiga.c
index 3301861f..8c7a598a 100644
--- a/drivers/parport/parport_amiga.c
+++ b/drivers/parport/parport_amiga.c
@@ -212,7 +212,7 @@ static int __init amiga_parallel_probe(struct platform_device *pdev)
 	if (err)
 		goto out_irq;
 
-	printk(KERN_INFO "%s: Amiga built-in port using irq\n", p->name);
+	pr_info("%s: Amiga built-in port using irq\n", p->name);
 	/* XXX: set operating mode */
 	parport_announce_port(p);
 
diff --git a/drivers/parport/parport_atari.c b/drivers/parport/parport_atari.c
index f8dd368b..2ff0fe0 100644
--- a/drivers/parport/parport_atari.c
+++ b/drivers/parport/parport_atari.c
@@ -200,7 +200,7 @@ static int __init parport_atari_init(void)
 		}
 
 		this_port = p;
-		printk(KERN_INFO "%s: Atari built-in port using irq\n", p->name);
+		pr_info("%s: Atari built-in port using irq\n", p->name);
 		parport_announce_port (p);
 
 		return 0;
diff --git a/drivers/parport/parport_cs.c b/drivers/parport/parport_cs.c
index e77044c2..8e7e3ac 100644
--- a/drivers/parport/parport_cs.c
+++ b/drivers/parport/parport_cs.c
@@ -142,10 +142,8 @@ static int parport_config(struct pcmcia_device *link)
 			      link->irq, PARPORT_DMA_NONE,
 			      &link->dev, IRQF_SHARED);
     if (p == NULL) {
-	printk(KERN_NOTICE "parport_cs: parport_pc_probe_port() at "
-	       "0x%3x, irq %u failed\n",
-	       (unsigned int) link->resource[0]->start,
-	       link->irq);
+	    pr_notice("parport_cs: parport_pc_probe_port() at 0x%3x, irq %u failed\n",
+		      (unsigned int)link->resource[0]->start, link->irq);
 	goto failed;
     }
 
diff --git a/drivers/parport/parport_gsc.c b/drivers/parport/parport_gsc.c
index 922535a..81082d 100644
--- a/drivers/parport/parport_gsc.c
+++ b/drivers/parport/parport_gsc.c
@@ -282,7 +282,7 @@ struct parport *parport_gsc_probe_port(unsigned long base,
 	p->size = (p->modes & PARPORT_MODE_EPP)?8:3;
 	p->private_data = priv;
 
-	printk(KERN_INFO "%s: PC-style at 0x%lx", p->name, p->base);
+	pr_info("%s: PC-style at 0x%lx", p->name, p->base);
 	p->irq = irq;
 	if (p->irq == PARPORT_IRQ_AUTO) {
 		p->irq = PARPORT_IRQ_NONE;
@@ -315,8 +315,7 @@ struct parport *parport_gsc_probe_port(unsigned long base,
 	if (p->irq != PARPORT_IRQ_NONE) {
 		if (request_irq (p->irq, parport_irq_handler,
 				 0, p->name, p)) {
-			printk (KERN_WARNING "%s: irq %d in use, "
-				"resorting to polled operation\n",
+			pr_warn("%s: irq %d in use, resorting to polled operation\n",
 				p->name, p->irq);
 			p->irq = PARPORT_IRQ_NONE;
 			p->dma = PARPORT_DMA_NONE;
@@ -347,7 +346,7 @@ static int __init parport_init_chip(struct parisc_device *dev)
 	unsigned long port;
 
 	if (!dev->irq) {
-		printk(KERN_WARNING "IRQ not found for parallel device at 0x%llx\n",
+		pr_warn("IRQ not found for parallel device at 0x%llx\n",
 			(unsigned long long)dev->hpa.start);
 		return -ENODEV;
 	}
diff --git a/drivers/parport/parport_ip32.c b/drivers/parport/parport_ip32.c
index ab215b..4f76e4 100644
--- a/drivers/parport/parport_ip32.c
+++ b/drivers/parport/parport_ip32.c
@@ -1337,9 +1337,8 @@ static unsigned int parport_ip32_fwp_wait_interrupt(struct parport *p)
 			ecr = parport_ip32_read_econtrol(p);
 			if ((ecr & ECR_F_EMPTY) && !(ecr & ECR_SERVINTR)
 			    && !lost_interrupt) {
-				printk(KERN_WARNING PPIP32
-				       "%s: lost interrupt in %s\n",
-				       p->name, __func__);
+				pr_warn(PPIP32 "%s: lost interrupt in %s\n",
+					p->name, __func__);
 				lost_interrupt = 1;
 			}
 		}
@@ -1643,8 +1642,8 @@ static size_t parport_ip32_compat_write_data(struct parport *p,
 				       DSR_nBUSY | DSR_nFAULT)) {
 		/* Avoid to flood the logs */
 		if (ready_before)
-			printk(KERN_INFO PPIP32 "%s: not ready in %s\n",
-			       p->name, __func__);
+			pr_info(PPIP32 "%s: not ready in %s\n",
+				p->name, __func__);
 		ready_before = 0;
 		goto stop;
 	}
@@ -1724,8 +1723,8 @@ static size_t parport_ip32_ecp_write_data(struct parport *p,
 				       DSR_nBUSY | DSR_nFAULT)) {
 		/* Avoid to flood the logs */
 		if (ready_before)
-			printk(KERN_INFO PPIP32 "%s: not ready in %s\n",
-			       p->name, __func__);
+			pr_info(PPIP32 "%s: not ready in %s\n",
+				p->name, __func__);
 		ready_before = 0;
 		goto stop;
 	}
@@ -2064,8 +2063,7 @@ static __init struct parport *parport_ip32_probe_port(void)
 	p->modes |= PARPORT_MODE_TRISTATE;
 
 	if (!parport_ip32_fifo_supported(p)) {
-		printk(KERN_WARNING PPIP32
-		       "%s: error: FIFO disabled\n", p->name);
+		pr_warn(PPIP32 "%s: error: FIFO disabled\n", p->name);
 		/* Disable hardware modes depending on a working FIFO. */
 		features &= ~PARPORT_IP32_ENABLE_SPP;
 		features &= ~PARPORT_IP32_ENABLE_ECP;
@@ -2077,8 +2075,7 @@ static __init struct parport *parport_ip32_probe_port(void)
 	if (features & PARPORT_IP32_ENABLE_IRQ) {
 		int irq = MACEISA_PARALLEL_IRQ;
 		if (request_irq(irq, parport_ip32_interrupt, 0, p->name, p)) {
-			printk(KERN_WARNING PPIP32
-			       "%s: error: IRQ disabled\n", p->name);
+			pr_warn(PPIP32 "%s: error: IRQ disabled\n", p->name);
 			/* DMA cannot work without interrupts. */
 			features &= ~PARPORT_IP32_ENABLE_DMA;
 		} else {
@@ -2091,8 +2088,7 @@ static __init struct parport *parport_ip32_probe_port(void)
 	/* Allocate DMA resources */
 	if (features & PARPORT_IP32_ENABLE_DMA) {
 		if (parport_ip32_dma_register())
-			printk(KERN_WARNING PPIP32
-			       "%s: error: DMA disabled\n", p->name);
+			pr_warn(PPIP32 "%s: error: DMA disabled\n", p->name);
 		else {
 			pr_probe(p, "DMA support enabled\n");
 			p->dma = 0; /* arbitrary value != PARPORT_DMA_NONE */
@@ -2134,8 +2130,7 @@ static __init struct parport *parport_ip32_probe_port(void)
 	parport_ip32_dump_state(p, "end init", 0);
 
 	/* Print out what we found */
-	printk(KERN_INFO "%s: SGI IP32 at 0x%lx (0x%lx)",
-	       p->name, p->base, p->base_hi);
+	pr_info("%s: SGI IP32 at 0x%lx (0x%lx)", p->name, p->base, p->base_hi);
 	if (p->irq != PARPORT_IRQ_NONE)
 		printk(", irq %d", p->irq);
 	printk(" [");
diff --git a/drivers/parport/parport_mfc3.c b/drivers/parport/parport_mfc3.c
index 9f87faf..3190ef0 100644
--- a/drivers/parport/parport_mfc3.c
+++ b/drivers/parport/parport_mfc3.c
@@ -325,7 +325,7 @@ static int __init parport_mfc3_init(void)
 		p->dev = &z->dev;
 
 		this_port[pias++] = p;
-		printk(KERN_INFO "%s: Multiface III port using irq\n", p->name);
+		pr_info("%s: Multiface III port using irq\n", p->name);
 		/* XXX: set operating mode */
 
 		p->private_data = (void *)piabase;
diff --git a/drivers/parport/parport_pc.c b/drivers/parport/parport_pc.c
index 1f17a3..2872c67 100644
--- a/drivers/parport/parport_pc.c
+++ b/drivers/parport/parport_pc.c
@@ -982,28 +982,24 @@ static void show_parconfig_smsc37c669(int io, int key)
 	outb(0xaa, io);
 
 	if (verbose_probing) {
-		printk(KERN_INFO
-			"SMSC 37c669 LPT Config: cr_1=0x%02x, 4=0x%02x, "
-			"A=0x%2x, 23=0x%02x, 26=0x%02x, 27=0x%02x\n",
+		pr_info("SMSC 37c669 LPT Config: cr_1=0x%02x, 4=0x%02x, A=0x%2x, 23=0x%02x, 26=0x%02x, 27=0x%02x\n",
 			cr1, cr4, cra, cr23, cr26, cr27);
 
 		/* The documentation calls DMA and IRQ-Lines by letters, so
 		   the board maker can/will wire them
 		   appropriately/randomly...  G=reserved H=IDE-irq, */
-		printk(KERN_INFO
-	"SMSC LPT Config: io=0x%04x, irq=%c, dma=%c, fifo threshold=%d\n",
-				cr23 * 4,
-				(cr27 & 0x0f) ? 'A' - 1 + (cr27 & 0x0f) : '-',
-				(cr26 & 0x0f) ? 'A' - 1 + (cr26 & 0x0f) : '-',
-				cra & 0x0f);
-		printk(KERN_INFO "SMSC LPT Config: enabled=%s power=%s\n",
-		       (cr23 * 4 >= 0x100) ? "yes" : "no",
-		       (cr1 & 4) ? "yes" : "no");
-		printk(KERN_INFO
-			"SMSC LPT Config: Port mode=%s, EPP version =%s\n",
-				(cr1 & 0x08) ? "Standard mode only (SPP)"
-					      : modes[cr4 & 0x03],
-				(cr4 & 0x40) ? "1.7" : "1.9");
+		pr_info("SMSC LPT Config: io=0x%04x, irq=%c, dma=%c, fifo threshold=%d\n",
+			cr23 * 4,
+			(cr27 & 0x0f) ? 'A' - 1 + (cr27 & 0x0f) : '-',
+			(cr26 & 0x0f) ? 'A' - 1 + (cr26 & 0x0f) : '-',
+			cra & 0x0f);
+		pr_info("SMSC LPT Config: enabled=%s power=%s\n",
+			(cr23 * 4 >= 0x100) ? "yes" : "no",
+			(cr1 & 4) ? "yes" : "no");
+		pr_info("SMSC LPT Config: Port mode=%s, EPP version =%s\n",
+			(cr1 & 0x08) ? "Standard mode only (SPP)"
+			: modes[cr4 & 0x03],
+			(cr4 & 0x40) ? "1.7" : "1.9");
 	}
 
 	/* Heuristics !  BIOS setup for this mainboard device limits
@@ -1013,7 +1009,7 @@ static void show_parconfig_smsc37c669(int io, int key)
 	if (cr23 * 4 >= 0x100) { /* if active */
 		s = find_free_superio();
 		if (s == NULL)
-			printk(KERN_INFO "Super-IO: too many chips!\n");
+			pr_info("Super-IO: too many chips!\n");
 		else {
 			int d;
 			switch (cr23 * 4) {
@@ -1078,26 +1074,24 @@ static void show_parconfig_winbond(int io, int key)
 	outb(0xaa, io);
 
 	if (verbose_probing) {
-		printk(KERN_INFO
-    "Winbond LPT Config: cr_30=%02x 60,61=%02x%02x 70=%02x 74=%02x, f0=%02x\n",
-					cr30, cr60, cr61, cr70, cr74, crf0);
-		printk(KERN_INFO "Winbond LPT Config: active=%s, io=0x%02x%02x irq=%d, ",
-		       (cr30 & 0x01) ? "yes" : "no", cr60, cr61, cr70 & 0x0f);
+		pr_info("Winbond LPT Config: cr_30=%02x 60,61=%02x%02x 70=%02x 74=%02x, f0=%02x\n",
+			cr30, cr60, cr61, cr70, cr74, crf0);
+		pr_info("Winbond LPT Config: active=%s, io=0x%02x%02x irq=%d, ",
+			(cr30 & 0x01) ? "yes" : "no", cr60, cr61, cr70 & 0x0f);
 		if ((cr74 & 0x07) > 3)
 			pr_cont("dma=none\n");
 		else
 			pr_cont("dma=%d\n", cr74 & 0x07);
-		printk(KERN_INFO
-		    "Winbond LPT Config: irqtype=%s, ECP fifo threshold=%d\n",
-					irqtypes[crf0>>7], (crf0>>3)&0x0f);
-		printk(KERN_INFO "Winbond LPT Config: Port mode=%s\n",
-					modes[crf0 & 0x07]);
+		pr_info("Winbond LPT Config: irqtype=%s, ECP fifo threshold=%d\n",
+			irqtypes[crf0 >> 7], (crf0 >> 3) & 0x0f);
+		pr_info("Winbond LPT Config: Port mode=%s\n",
+			modes[crf0 & 0x07]);
 	}
 
 	if (cr30 & 0x01) { /* the settings can be interrogated later ... */
 		s = find_free_superio();
 		if (s == NULL)
-			printk(KERN_INFO "Super-IO: too many chips!\n");
+			pr_info("Super-IO: too many chips!\n");
 		else {
 			s->io = (cr60 << 8) | cr61;
 			s->irq = cr70 & 0x0f;
@@ -1151,9 +1145,8 @@ static void decode_winbond(int efer, int key, int devid, int devrev, int oldid)
 		progif = 0;
 
 	if (verbose_probing)
-		printk(KERN_INFO "Winbond chip at EFER=0x%x key=0x%02x "
-		       "devid=%02x devrev=%02x oldid=%02x type=%s\n",
-		       efer, key, devid, devrev, oldid, type);
+		pr_info("Winbond chip at EFER=0x%x key=0x%02x devid=%02x devrev=%02x oldid=%02x type=%s\n",
+			efer, key, devid, devrev, oldid, type);
 
 	if (progif == 2)
 		show_parconfig_winbond(efer, key);
@@ -1184,9 +1177,8 @@ static void decode_smsc(int efer, int key, int devid, int devrev)
 		type = "37c666GT";
 
 	if (verbose_probing)
-		printk(KERN_INFO "SMSC chip at EFER=0x%x "
-		       "key=0x%02x devid=%02x devrev=%02x type=%s\n",
-		       efer, key, devid, devrev, type);
+		pr_info("SMSC chip at EFER=0x%x key=0x%02x devid=%02x devrev=%02x type=%s\n",
+			efer, key, devid, devrev, type);
 
 	if (func)
 		func(efer, key);
@@ -1358,7 +1350,7 @@ static void detect_and_report_it87(void)
 	dev |= inb(0x2f);
 	if (dev == 0x8712 || dev == 0x8705 || dev == 0x8715 ||
 	    dev == 0x8716 || dev == 0x8718 || dev == 0x8726) {
-		printk(KERN_INFO "IT%04X SuperIO detected.\n", dev);
+		pr_info("IT%04X SuperIO detected\n", dev);
 		outb(0x07, 0x2E);	/* Parallel Port */
 		outb(0x03, 0x2F);
 		outb(0xF0, 0x2E);	/* BOOT 0x80 off */
@@ -1445,8 +1437,8 @@ static int parport_SPP_supported(struct parport *pb)
 	if (user_specified)
 		/* That didn't work, but the user thinks there's a
 		 * port here. */
-		printk(KERN_INFO "parport 0x%lx (WARNING): CTR: "
-			"wrote 0x%02x, read 0x%02x\n", pb->base, w, r);
+		pr_info("parport 0x%lx (WARNING): CTR: wrote 0x%02x, read 0x%02x\n",
+			pb->base, w, r);
 
 	/* Try the data register.  The data lines aren't tri-stated at
 	 * this stage, so we expect back what we wrote. */
@@ -1464,10 +1456,9 @@ static int parport_SPP_supported(struct parport *pb)
 	if (user_specified) {
 		/* Didn't work, but the user is convinced this is the
 		 * place. */
-		printk(KERN_INFO "parport 0x%lx (WARNING): DATA: "
-			"wrote 0x%02x, read 0x%02x\n", pb->base, w, r);
-		printk(KERN_INFO "parport 0x%lx: You gave this address, "
-			"but there is probably no parallel port there!\n",
+		pr_info("parport 0x%lx (WARNING): DATA: wrote 0x%02x, read 0x%02x\n",
+			pb->base, w, r);
+		pr_info("parport 0x%lx: You gave this address, but there is probably no parallel port there!\n",
 			pb->base);
 	}
 
@@ -1642,7 +1633,7 @@ static int parport_ECP_supported(struct parport *pb)
 
 	if (i <= priv->fifo_depth) {
 		if (verbose_probing)
-			printk(KERN_INFO "0x%lx: readIntrThreshold is %d\n",
+			pr_info("0x%lx: readIntrThreshold is %d\n",
 				pb->base, i);
 	} else
 		/* Number of bytes we can read if we get an interrupt. */
@@ -1657,17 +1648,14 @@ static int parport_ECP_supported(struct parport *pb)
 	switch (pword) {
 	case 0:
 		pword = 2;
-		printk(KERN_WARNING "0x%lx: Unsupported pword size!\n",
-			pb->base);
+		pr_warn("0x%lx: Unsupported pword size!\n", pb->base);
 		break;
 	case 2:
 		pword = 4;
-		printk(KERN_WARNING "0x%lx: Unsupported pword size!\n",
-			pb->base);
+		pr_warn("0x%lx: Unsupported pword size!\n", pb->base);
 		break;
 	default:
-		printk(KERN_WARNING "0x%lx: Unknown implementation ID\n",
-			pb->base);
+		pr_warn("0x%lx: Unknown implementation ID\n", pb->base);
 		/* Fall through - Assume 1 */
 	case 1:
 		pword = 1;
@@ -2107,9 +2095,9 @@ struct parport *parport_pc_probe_port(unsigned long int base,
 
 	p->size = (p->modes & PARPORT_MODE_EPP) ? 8 : 3;
 
-	printk(KERN_INFO "%s: PC-style at 0x%lx", p->name, p->base);
+	pr_info("%s: PC-style at 0x%lx", p->name, p->base);
 	if (p->base_hi && priv->ecr)
-		printk(KERN_CONT " (0x%lx)", p->base_hi);
+		pr_cont(" (0x%lx)", p->base_hi);
 	if (p->irq == PARPORT_IRQ_AUTO) {
 		p->irq = PARPORT_IRQ_NONE;
 		parport_irq_probe(p);
@@ -2120,7 +2108,7 @@ struct parport *parport_pc_probe_port(unsigned long int base,
 		p->irq = PARPORT_IRQ_NONE;
 	}
 	if (p->irq != PARPORT_IRQ_NONE) {
-		printk(KERN_CONT ", irq %d", p->irq);
+		pr_cont(", irq %d", p->irq);
 		priv->ctr_writable |= 0x10;
 
 		if (p->dma == PARPORT_DMA_AUTO) {
@@ -2144,21 +2132,21 @@ struct parport *parport_pc_probe_port(unsigned long int base,
 		/* p->ops->ecp_read_data = parport_pc_ecp_read_block_pio; */
 #endif /* IEEE 1284 support */
 		if (p->dma != PARPORT_DMA_NONE) {
-			printk(KERN_CONT ", dma %d", p->dma);
+			pr_cont(", dma %d", p->dma);
 			p->modes |= PARPORT_MODE_DMA;
 		} else
-			printk(KERN_CONT ", using FIFO");
+			pr_cont(", using FIFO");
 	} else
 		/* We can't use the DMA channel after all. */
 		p->dma = PARPORT_DMA_NONE;
 #endif /* Allowed to use FIFO/DMA */
 
-	printk(KERN_CONT " [");
+	pr_cont(" [");
 
 #define printmode(x) \
 	{\
 		if (p->modes & PARPORT_MODE_##x) {\
-			printk(KERN_CONT "%s%s", f ? "," : "", #x);\
+			pr_cont("%s%s", f ? "," : "", #x);	\
 			f++;\
 		} \
 	}
@@ -2174,11 +2162,11 @@ struct parport *parport_pc_probe_port(unsigned long int base,
 	}
 #undef printmode
 #ifndef CONFIG_PARPORT_1284
-	printk(KERN_CONT "(,...)");
+	pr_cont("(,...)");
 #endif /* CONFIG_PARPORT_1284 */
-	printk(KERN_CONT "]\n");
+	pr_cont("]\n");
 	if (probedirq != PARPORT_IRQ_NONE)
-		printk(KERN_INFO "%s: irq %d detected\n", p->name, probedirq);
+		pr_info("%s: irq %d detected\n", p->name, probedirq);
 
 	/* If No ECP release the ports grabbed above. */
 	if (ECR_res && (p->modes & PARPORT_MODE_ECP) == 0) {
@@ -2193,8 +2181,7 @@ struct parport *parport_pc_probe_port(unsigned long int base,
 	if (p->irq != PARPORT_IRQ_NONE) {
 		if (request_irq(p->irq, parport_irq_handler,
 				 irqflags, p->name, p)) {
-			printk(KERN_WARNING "%s: irq %d in use, "
-				"resorting to polled operation\n",
+			pr_warn("%s: irq %d in use, resorting to polled operation\n",
 				p->name, p->irq);
 			p->irq = PARPORT_IRQ_NONE;
 			p->dma = PARPORT_DMA_NONE;
@@ -2204,8 +2191,7 @@ struct parport *parport_pc_probe_port(unsigned long int base,
 #ifdef HAS_DMA
 		if (p->dma != PARPORT_DMA_NONE) {
 			if (request_dma(p->dma, p->name)) {
-				printk(KERN_WARNING "%s: dma %d in use, "
-					"resorting to PIO operation\n",
+				pr_warn("%s: dma %d in use, resorting to PIO operation\n",
 					p->name, p->dma);
 				p->dma = PARPORT_DMA_NONE;
 			} else {
@@ -2215,9 +2201,7 @@ struct parport *parport_pc_probe_port(unsigned long int base,
 						       &priv->dma_handle,
 						       GFP_KERNEL);
 				if (!priv->dma_buf) {
-					printk(KERN_WARNING "%s: "
-						"cannot get buffer for DMA, "
-						"resorting to PIO operation\n",
+					pr_warn("%s: cannot get buffer for DMA, resorting to PIO operation\n",
 						p->name);
 					free_dma(p->dma);
 					p->dma = PARPORT_DMA_NONE;
@@ -2330,7 +2314,7 @@ static int sio_ite_8872_probe(struct pci_dev *pdev, int autoirq, int autodma,
 		}
 	}
 	if (i >= 5) {
-		printk(KERN_INFO "parport_pc: cannot find ITE8872 INTA\n");
+		pr_info("parport_pc: cannot find ITE8872 INTA\n");
 		return 0;
 	}
 
@@ -2339,29 +2323,28 @@ static int sio_ite_8872_probe(struct pci_dev *pdev, int autoirq, int autodma,
 
 	switch (type) {
 	case 0x2:
-		printk(KERN_INFO "parport_pc: ITE8871 found (1P)\n");
+		pr_info("parport_pc: ITE8871 found (1P)\n");
 		ite8872set = 0x64200000;
 		break;
 	case 0xa:
-		printk(KERN_INFO "parport_pc: ITE8875 found (1P)\n");
+		pr_info("parport_pc: ITE8875 found (1P)\n");
 		ite8872set = 0x64200000;
 		break;
 	case 0xe:
-		printk(KERN_INFO "parport_pc: ITE8872 found (2S1P)\n");
+		pr_info("parport_pc: ITE8872 found (2S1P)\n");
 		ite8872set = 0x64e00000;
 		break;
 	case 0x6:
-		printk(KERN_INFO "parport_pc: ITE8873 found (1S)\n");
+		pr_info("parport_pc: ITE8873 found (1S)\n");
 		release_region(inta_addr[i], 32);
 		return 0;
 	case 0x8:
-		printk(KERN_INFO "parport_pc: ITE8874 found (2S)\n");
+		pr_info("parport_pc: ITE8874 found (2S)\n");
 		release_region(inta_addr[i], 32);
 		return 0;
 	default:
-		printk(KERN_INFO "parport_pc: unknown ITE887x\n");
-		printk(KERN_INFO "parport_pc: please mail 'lspci -nvv' "
-			"output to Rich.Liu@ite.com.tw\n");
+		pr_info("parport_pc: unknown ITE887x\n");
+		pr_info("parport_pc: please mail 'lspci -nvv' output to Rich.Liu@ite.com.tw\n");
 		release_region(inta_addr[i], 32);
 		return 0;
 	}
@@ -2396,9 +2379,8 @@ static int sio_ite_8872_probe(struct pci_dev *pdev, int autoirq, int autodma,
 	release_region(inta_addr[i], 32);
 	if (parport_pc_probe_port(ite8872_lpt, ite8872_lpthi,
 				   irq, PARPORT_DMA_NONE, &pdev->dev, 0)) {
-		printk(KERN_INFO
-			"parport_pc: ITE 8872 parallel port: io=0x%X",
-								ite8872_lpt);
+		pr_info("parport_pc: ITE 8872 parallel port: io=0x%X",
+			ite8872_lpt);
 		if (irq != PARPORT_IRQ_NONE)
 			pr_cont(", irq=%d", irq);
 		pr_cont("\n");
@@ -2525,7 +2507,7 @@ static int sio_via_probe(struct pci_dev *pdev, int autoirq, int autodma,
 	pci_write_config_byte(pdev, via->via_pci_superio_config_reg, tmp);
 
 	if (siofunc == VIA_FUNCTION_PARPORT_DISABLE) {
-		printk(KERN_INFO "parport_pc: VIA parallel port disabled in BIOS\n");
+		pr_info("parport_pc: VIA parallel port disabled in BIOS\n");
 		return 0;
 	}
 
@@ -2558,9 +2540,8 @@ static int sio_via_probe(struct pci_dev *pdev, int autoirq, int autodma,
 	case 0x278:
 		port2 = 0x678; break;
 	default:
-		printk(KERN_INFO
-			"parport_pc: Weird VIA parport base 0x%X, ignoring\n",
-									port1);
+		pr_info("parport_pc: Weird VIA parport base 0x%X, ignoring\n",
+			port1);
 		return 0;
 	}
 
@@ -2579,8 +2560,7 @@ static int sio_via_probe(struct pci_dev *pdev, int autoirq, int autodma,
 
 	/* finally, do the probe with values obtained */
 	if (parport_pc_probe_port(port1, port2, irq, dma, &pdev->dev, 0)) {
-		printk(KERN_INFO
-			"parport_pc: VIA parallel port: io=0x%X", port1);
+		pr_info("parport_pc: VIA parallel port: io=0x%X", port1);
 		if (irq != PARPORT_IRQ_NONE)
 			pr_cont(", irq=%d", irq);
 		if (dma != PARPORT_DMA_NONE)
@@ -2589,7 +2569,7 @@ static int sio_via_probe(struct pci_dev *pdev, int autoirq, int autodma,
 		return 1;
 	}
 
-	printk(KERN_WARNING "parport_pc: Strange, can't probe VIA parallel port: io=0x%X, irq=%d, dma=%d\n",
+	pr_warn("parport_pc: Strange, can't probe VIA parallel port: io=0x%X, irq=%d, dma=%d\n",
 		port1, irq, dma);
 	return 0;
 }
@@ -3111,7 +3091,7 @@ static int __init parport_parse_param(const char *s, int *val,
 		if (ep != s)
 			*val = r;
 		else {
-			printk(KERN_ERR "parport: bad specifier `%s'\n", s);
+			pr_err("parport: bad specifier `%s'\n", s);
 			return -1;
 		}
 	}
@@ -3201,10 +3181,7 @@ static int __init parse_parport_params(void)
 				irqval[0] = val;
 				break;
 			default:
-				printk(KERN_WARNING
-					"parport_pc: irq specified "
-					"without base address.  Use 'io=' "
-					"to specify one\n");
+				pr_warn("parport_pc: irq specified without base address.  Use 'io=' to specify one\n");
 			}
 
 		if (dma[0] && !parport_parse_dma(dma[0], &val))
@@ -3214,10 +3191,7 @@ static int __init parse_parport_params(void)
 				dmaval[0] = val;
 				break;
 			default:
-				printk(KERN_WARNING
-					"parport_pc: dma specified "
-					"without base address.  Use 'io=' "
-					"to specify one\n");
+				pr_warn("parport_pc: dma specified without base address.  Use 'io=' to specify one\n");
 			}
 	}
 	return 0;
@@ -3256,12 +3230,12 @@ static int __init parport_setup(char *str)
 
 	val = simple_strtoul(str, &endptr, 0);
 	if (endptr == str) {
-		printk(KERN_WARNING "parport=%s not understood\n", str);
+		pr_warn("parport=%s not understood\n", str);
 		return 1;
 	}
 
 	if (parport_setup_ptr == PARPORT_PC_MAX_PORTS) {
-		printk(KERN_ERR "parport=%s ignored, too many ports\n", str);
+		pr_err("parport=%s ignored, too many ports\n", str);
 		return 1;
 	}
 
diff --git a/drivers/parport/parport_sunbpp.c b/drivers/parport/parport_sunbpp.c
index d5a669..e840c1b 100644
--- a/drivers/parport/parport_sunbpp.c
+++ b/drivers/parport/parport_sunbpp.c
@@ -314,7 +314,7 @@ static int bpp_probe(struct platform_device *op)
 	value_tcr &= ~P_TCR_DIR;
 	sbus_writeb(value_tcr, &regs->p_tcr);
 
-	printk(KERN_INFO "%s: sunbpp at 0x%lx\n", p->name, p->base);
+	pr_info("%s: sunbpp at 0x%lx\n", p->name, p->base);
 
 	dev_set_drvdata(&op->dev, p);
 
diff --git a/drivers/parport/probe.c b/drivers/parport/probe.c
index e5e6a46..10ed0d1 100644
--- a/drivers/parport/probe.c
+++ b/drivers/parport/probe.c
@@ -38,7 +38,7 @@ static void pretty_print(struct parport *port, int device)
 {
 	struct parport_device_info *info = &port->probe_info[device + 1];
 
-	printk(KERN_INFO "%s", port->name);
+	pr_info("%s", port->name);
 
 	if (device >= 0)
 		printk (" (addr %d)", device);
@@ -58,7 +58,7 @@ static void parse_data(struct parport *port, int device, char *str)
 	struct parport_device_info *info = &port->probe_info[device + 1];
 
 	if (!txt) {
-		printk(KERN_WARNING "%s probe: memory squeeze\n", port->name);
+		pr_warn("%s probe: memory squeeze\n", port->name);
 		return;
 	}
 	strcpy(txt, str);
@@ -98,7 +98,8 @@ static void parse_data(struct parport *port, int device, char *str)
 						goto rock_on;
 					}
 				}
-				printk(KERN_WARNING "%s probe: warning, class '%s' not understood.\n", port->name, sep);
+				pr_warn("%s probe: warning, class '%s' not understood\n",
+					port->name, sep);
 				info->class = PARPORT_CLASS_OTHER;
 			} else if (!strcmp(p, "CMD") ||
 				   !strcmp(p, "COMMAND SET")) {
diff --git a/drivers/parport/share.c b/drivers/parport/share.c
index d6920eb..b0f451 100644
--- a/drivers/parport/share.c
+++ b/drivers/parport/share.c
@@ -554,8 +554,8 @@ void parport_announce_port(struct parport *port)
 #endif
 
 	if (!port->dev)
-		printk(KERN_WARNING "%s: fix this legacy no-device port driver!\n",
-		       port->name);
+		pr_warn("%s: fix this legacy no-device port driver!\n",
+			port->name);
 
 	parport_proc_register(port);
 	mutex_lock(&registration_lock);
@@ -727,7 +727,8 @@ parport_register_device(struct parport *port, const char *name,
 
 	if (flags & PARPORT_DEV_LURK) {
 		if (!pf || !kf) {
-			printk(KERN_INFO "%s: refused to register lurking device (%s) without callbacks\n", port->name, name);
+			pr_info("%s: refused to register lurking device (%s) without callbacks\n",
+				port->name, name);
 			return NULL;
 		}
 	}
@@ -996,7 +997,7 @@ void parport_unregister_device(struct pardevice *dev)
 
 #ifdef PARPORT_PARANOID
 	if (!dev) {
-		printk(KERN_ERR "parport_unregister_device: passed NULL\n");
+		pr_err("%s: passed NULL\n", __func__);
 		return;
 	}
 #endif
@@ -1137,8 +1138,7 @@ int parport_claim(struct pardevice *dev)
 	unsigned long flags;
 
 	if (port->cad == dev) {
-		printk(KERN_INFO "%s: %s already owner\n",
-		       dev->port->name,dev->name);
+		pr_info("%s: %s already owner\n", dev->port->name, dev->name);
 		return 0;
 	}
 
@@ -1158,9 +1158,8 @@ int parport_claim(struct pardevice *dev)
 			 * I think we'll actually deadlock rather than
 			 * get here, but just in case..
 			 */
-			printk(KERN_WARNING
-			       "%s: %s released port when preempted!\n",
-			       port->name, oldcad->name);
+			pr_warn("%s: %s released port when preempted!\n",
+				port->name, oldcad->name);
 			if (port->cad)
 				goto blocked;
 		}
@@ -1320,8 +1319,8 @@ void parport_release(struct pardevice *dev)
 	write_lock_irqsave(&port->cad_lock, flags);
 	if (port->cad != dev) {
 		write_unlock_irqrestore(&port->cad_lock, flags);
-		printk(KERN_WARNING "%s: %s tried to release parport when not owner\n",
-		       port->name, dev->name);
+		pr_warn("%s: %s tried to release parport when not owner\n",
+			port->name, dev->name);
 		return;
 	}
 
@@ -1361,7 +1360,8 @@ void parport_release(struct pardevice *dev)
 			if (dev->port->cad) /* racy but no matter */
 				return;
 		} else {
-			printk(KERN_ERR "%s: don't know how to wake %s\n", port->name, pd->name);
+			pr_err("%s: don't know how to wake %s\n",
+			       port->name, pd->name);
 		}
 	}
 
-- 
2.24.0

[PATCH 2/7] parport: Use more comon logging styles

[Joe Perches]

Some of the logging can be poorly formatted because of unexpected
line breaks given printks without KERN_CONT that should be pr_cont.

Miscellanea:

o Remove unnecessary spaces between function name and open parenthesis
o Convert bare printks to pr_<level> where appropriate
o Convert embedded function names to use %s, __func__
o Coalesce formats
o Realign arguments
o Use do {} while (0) in a macro and not a bare if

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/parport/parport_gsc.c  | 10 ++--
 drivers/parport/parport_gsc.h  | 21 ++++----
 drivers/parport/parport_ip32.c | 92 ++++++++++++++++++----------------
 drivers/parport/parport_pc.c   | 62 +++++++++++------------
 drivers/parport/probe.c        | 27 ++++------
 drivers/parport/share.c        | 13 +++--
 6 files changed, 108 insertions(+), 117 deletions(-)

diff --git a/drivers/parport/parport_gsc.c b/drivers/parport/parport_gsc.c
index 81082d..cb6a08 100644
--- a/drivers/parport/parport_gsc.c
+++ b/drivers/parport/parport_gsc.c
@@ -238,14 +238,14 @@ struct parport *parport_gsc_probe_port(unsigned long base,
 
 	priv = kzalloc (sizeof (struct parport_gsc_private), GFP_KERNEL);
 	if (!priv) {
-		printk (KERN_DEBUG "parport (0x%lx): no memory!\n", base);
+		printk(KERN_DEBUG "parport (0x%lx): no memory!\n", base);
 		return NULL;
 	}
 	ops = kmemdup(&parport_gsc_ops, sizeof(struct parport_operations),
 		      GFP_KERNEL);
 	if (!ops) {
-		printk (KERN_DEBUG "parport (0x%lx): no memory for ops!\n",
-			base);
+		printk(KERN_DEBUG "parport (0x%lx): no memory for ops!\n",
+		       base);
 		kfree (priv);
 		return NULL;
 	}
@@ -359,11 +359,11 @@ static int __init parport_init_chip(struct parisc_device *dev)
 	if (boot_cpu_data.cpu_type > pcxt && !pdc_add_valid(port+4)) {
 
 		/* Initialize bidirectional-mode (0x10) & data-tranfer-mode #1 (0x20) */
-		printk("%s: initialize bidirectional-mode.\n", __func__);
+		pr_info("%s: initialize bidirectional-mode\n", __func__);
 		parport_writeb ( (0x10 + 0x20), port + 4);
 
 	} else {
-		printk("%s: enhanced parport-modes not supported.\n", __func__);
+		pr_info("%s: enhanced parport-modes not supported\n", __func__);
 	}
 	
 	p = parport_gsc_probe_port(port, 0, dev->irq,
diff --git a/drivers/parport/parport_gsc.h b/drivers/parport/parport_gsc.h
index 4c4d3c..9301217 100644
--- a/drivers/parport/parport_gsc.h
+++ b/drivers/parport/parport_gsc.h
@@ -71,7 +71,7 @@ struct parport_gsc_private {
 static inline void parport_gsc_write_data(struct parport *p, unsigned char d)
 {
 #ifdef DEBUG_PARPORT
-	printk (KERN_DEBUG "parport_gsc_write_data(%p,0x%02x)\n", p, d);
+	printk(KERN_DEBUG "%s(%p,0x%02x)\n", __func__, p, d);
 #endif
 	parport_writeb(d, DATA(p));
 }
@@ -80,8 +80,7 @@ static inline unsigned char parport_gsc_read_data(struct parport *p)
 {
 	unsigned char val = parport_readb (DATA (p));
 #ifdef DEBUG_PARPORT
-	printk (KERN_DEBUG "parport_gsc_read_data(%p) = 0x%02x\n",
-		p, val);
+	printk(KERN_DEBUG "%s(%p) = 0x%02x\n", __func__, p, val);
 #endif
 	return val;
 }
@@ -95,9 +94,9 @@ static inline unsigned char __parport_gsc_frob_control(struct parport *p,
 	struct parport_gsc_private *priv = p->physport->private_data;
 	unsigned char ctr = priv->ctr;
 #ifdef DEBUG_PARPORT
-	printk (KERN_DEBUG
-		"__parport_gsc_frob_control(%02x,%02x): %02x -> %02x\n",
-		mask, val, ctr, ((ctr & ~mask) ^ val) & priv->ctr_writable);
+	printk(KERN_DEBUG "%s(%02x,%02x): %02x -> %02x\n",
+	       __func__, mask, val,
+	       ctr, ((ctr & ~mask) ^ val) & priv->ctr_writable);
 #endif
 	ctr = (ctr & ~mask) ^ val;
 	ctr &= priv->ctr_writable; /* only write writable bits. */
@@ -126,8 +125,8 @@ static inline void parport_gsc_write_control(struct parport *p,
 
 	/* Take this out when drivers have adapted to newer interface. */
 	if (d & 0x20) {
-		printk (KERN_DEBUG "%s (%s): use data_reverse for this!\n",
-			p->name, p->cad->name);
+		printk(KERN_DEBUG "%s (%s): use data_reverse for this!\n",
+		       p->name, p->cad->name);
 		parport_gsc_data_reverse (p);
 	}
 
@@ -155,9 +154,9 @@ static inline unsigned char parport_gsc_frob_control(struct parport *p,
 
 	/* Take this out when drivers have adapted to newer interface. */
 	if (mask & 0x20) {
-		printk (KERN_DEBUG "%s (%s): use data_%s for this!\n",
-			p->name, p->cad->name,
-			(val & 0x20) ? "reverse" : "forward");
+		printk(KERN_DEBUG "%s (%s): use data_%s for this!\n",
+		       p->name, p->cad->name,
+		       (val & 0x20) ? "reverse" : "forward");
 		if (val & 0x20)
 			parport_gsc_data_reverse (p);
 		else
diff --git a/drivers/parport/parport_ip32.c b/drivers/parport/parport_ip32.c
index 4f76e4..48b084 100644
--- a/drivers/parport/parport_ip32.c
+++ b/drivers/parport/parport_ip32.c
@@ -328,19 +328,19 @@ static void parport_ip32_dump_state(struct parport *p, char *str,
 						     "TST", "CFG"};
 		unsigned int ecr = readb(priv->regs.ecr);
 		printk(KERN_DEBUG PPIP32 "    ecr=0x%02x", ecr);
-		printk(" %s",
-		       ecr_modes[(ecr & ECR_MODE_MASK) >> ECR_MODE_SHIFT]);
+		pr_cont(" %s",
+			ecr_modes[(ecr & ECR_MODE_MASK) >> ECR_MODE_SHIFT]);
 		if (ecr & ECR_nERRINTR)
-			printk(",nErrIntrEn");
+			pr_cont(",nErrIntrEn");
 		if (ecr & ECR_DMAEN)
-			printk(",dmaEn");
+			pr_cont(",dmaEn");
 		if (ecr & ECR_SERVINTR)
-			printk(",serviceIntr");
+			pr_cont(",serviceIntr");
 		if (ecr & ECR_F_FULL)
-			printk(",f_full");
+			pr_cont(",f_full");
 		if (ecr & ECR_F_EMPTY)
-			printk(",f_empty");
-		printk("\n");
+			pr_cont(",f_empty");
+		pr_cont("\n");
 	}
 	if (show_ecp_config) {
 		unsigned int oecr, cnfgA, cnfgB;
@@ -352,52 +352,53 @@ static void parport_ip32_dump_state(struct parport *p, char *str,
 		writeb(ECR_MODE_PS2, priv->regs.ecr);
 		writeb(oecr, priv->regs.ecr);
 		printk(KERN_DEBUG PPIP32 "    cnfgA=0x%02x", cnfgA);
-		printk(" ISA-%s", (cnfgA & CNFGA_IRQ) ? "Level" : "Pulses");
+		pr_cont(" ISA-%s", (cnfgA & CNFGA_IRQ) ? "Level" : "Pulses");
 		switch (cnfgA & CNFGA_ID_MASK) {
 		case CNFGA_ID_8:
-			printk(",8 bits");
+			pr_cont(",8 bits");
 			break;
 		case CNFGA_ID_16:
-			printk(",16 bits");
+			pr_cont(",16 bits");
 			break;
 		case CNFGA_ID_32:
-			printk(",32 bits");
+			pr_cont(",32 bits");
 			break;
 		default:
-			printk(",unknown ID");
+			pr_cont(",unknown ID");
 			break;
 		}
 		if (!(cnfgA & CNFGA_nBYTEINTRANS))
-			printk(",ByteInTrans");
+			pr_cont(",ByteInTrans");
 		if ((cnfgA & CNFGA_ID_MASK) != CNFGA_ID_8)
-			printk(",%d byte%s left", cnfgA & CNFGA_PWORDLEFT,
-			       ((cnfgA & CNFGA_PWORDLEFT) > 1) ? "s" : "");
-		printk("\n");
+			pr_cont(",%d byte%s left",
+				cnfgA & CNFGA_PWORDLEFT,
+				((cnfgA & CNFGA_PWORDLEFT) > 1) ? "s" : "");
+		pr_cont("\n");
 		printk(KERN_DEBUG PPIP32 "    cnfgB=0x%02x", cnfgB);
-		printk(" irq=%u,dma=%u",
-		       (cnfgB & CNFGB_IRQ_MASK) >> CNFGB_IRQ_SHIFT,
-		       (cnfgB & CNFGB_DMA_MASK) >> CNFGB_DMA_SHIFT);
-		printk(",intrValue=%d", !!(cnfgB & CNFGB_INTRVAL));
+		pr_cont(" irq=%u,dma=%u",
+			(cnfgB & CNFGB_IRQ_MASK) >> CNFGB_IRQ_SHIFT,
+			(cnfgB & CNFGB_DMA_MASK) >> CNFGB_DMA_SHIFT);
+		pr_cont(",intrValue=%d", !!(cnfgB & CNFGB_INTRVAL));
 		if (cnfgB & CNFGB_COMPRESS)
-			printk(",compress");
-		printk("\n");
+			pr_cont(",compress");
+		pr_cont("\n");
 	}
 	for (i = 0; i < 2; i++) {
 		unsigned int dcr = i ? priv->dcr_cache : readb(priv->regs.dcr);
 		printk(KERN_DEBUG PPIP32 "    dcr(%s)=0x%02x",
 		       i ? "soft" : "hard", dcr);
-		printk(" %s", (dcr & DCR_DIR) ? "rev" : "fwd");
+		pr_cont(" %s", (dcr & DCR_DIR) ? "rev" : "fwd");
 		if (dcr & DCR_IRQ)
-			printk(",ackIntEn");
+			pr_cont(",ackIntEn");
 		if (!(dcr & DCR_SELECT))
-			printk(",nSelectIn");
+			pr_cont(",nSelectIn");
 		if (dcr & DCR_nINIT)
-			printk(",nInit");
+			pr_cont(",nInit");
 		if (!(dcr & DCR_AUTOFD))
-			printk(",nAutoFD");
+			pr_cont(",nAutoFD");
 		if (!(dcr & DCR_STROBE))
-			printk(",nStrobe");
-		printk("\n");
+			pr_cont(",nStrobe");
+		pr_cont("\n");
 	}
 #define sep (f++ ? ',' : ' ')
 	{
@@ -405,20 +406,20 @@ static void parport_ip32_dump_state(struct parport *p, char *str,
 		unsigned int dsr = readb(priv->regs.dsr);
 		printk(KERN_DEBUG PPIP32 "    dsr=0x%02x", dsr);
 		if (!(dsr & DSR_nBUSY))
-			printk("%cBusy", sep);
+			pr_cont("%cBusy", sep);
 		if (dsr & DSR_nACK)
-			printk("%cnAck", sep);
+			pr_cont("%cnAck", sep);
 		if (dsr & DSR_PERROR)
-			printk("%cPError", sep);
+			pr_cont("%cPError", sep);
 		if (dsr & DSR_SELECT)
-			printk("%cSelect", sep);
+			pr_cont("%cSelect", sep);
 		if (dsr & DSR_nFAULT)
-			printk("%cnFault", sep);
+			pr_cont("%cnFault", sep);
 		if (!(dsr & DSR_nPRINT))
-			printk("%c(Print)", sep);
+			pr_cont("%c(Print)", sep);
 		if (dsr & DSR_TIMEOUT)
-			printk("%cTimeout", sep);
-		printk("\n");
+			pr_cont("%cTimeout", sep);
+		pr_cont("\n");
 	}
 #undef sep
 }
@@ -1703,7 +1704,7 @@ static size_t parport_ip32_ecp_write_data(struct parport *p,
 
 		/* Event 49: PError goes high. */
 		if (parport_wait_peripheral(p, DSR_PERROR, DSR_PERROR)) {
-			printk(KERN_DEBUG PPIP32 "%s: PError timeout in %s",
+			printk(KERN_DEBUG PPIP32 "%s: PError timeout in %s\n",
 			       p->name, __func__);
 			physport->ieee1284.phase = IEEE1284_PH_ECP_DIR_UNKNOWN;
 			return 0;
@@ -2132,10 +2133,13 @@ static __init struct parport *parport_ip32_probe_port(void)
 	/* Print out what we found */
 	pr_info("%s: SGI IP32 at 0x%lx (0x%lx)", p->name, p->base, p->base_hi);
 	if (p->irq != PARPORT_IRQ_NONE)
-		printk(", irq %d", p->irq);
-	printk(" [");
-#define printmode(x)	if (p->modes & PARPORT_MODE_##x)		\
-				printk("%s%s", f++ ? "," : "", #x)
+		pr_cont(", irq %d", p->irq);
+	pr_cont(" [");
+#define printmode(x)							\
+do {									\
+	if (p->modes & PARPORT_MODE_##x)				\
+		pr_cont("%s%s", f++ ? "," : "", #x);			\
+} while (0)
 	{
 		unsigned int f = 0;
 		printmode(PCSPP);
@@ -2146,7 +2150,7 @@ static __init struct parport *parport_ip32_probe_port(void)
 		printmode(DMA);
 	}
 #undef printmode
-	printk("]\n");
+	pr_cont("]\n");
 
 	parport_announce_port(p);
 	return p;
diff --git a/drivers/parport/parport_pc.c b/drivers/parport/parport_pc.c
index 2872c67..ab3b04f 100644
--- a/drivers/parport/parport_pc.c
+++ b/drivers/parport/parport_pc.c
@@ -298,8 +298,8 @@ static size_t parport_pc_epp_read_data(struct parport *port, void *buf,
 			status = inb(STATUS(port));
 			if (status & 0x01) {
 				/* EPP timeout should never occur... */
-				printk(KERN_DEBUG
-"%s: EPP timeout occurred while talking to w91284pic (should not have done)\n", port->name);
+				printk(KERN_DEBUG "%s: EPP timeout occurred while talking to w91284pic (should not have done)\n",
+				       port->name);
 				clear_epp_timeout(port);
 			}
 		}
@@ -727,7 +727,7 @@ static size_t parport_pc_compat_write_block_pio(struct parport *port,
 	r = change_mode(port, ECR_PPF); /* Parallel port FIFO */
 	if (r)
 		printk(KERN_DEBUG "%s: Warning change_mode ECR_PPF failed\n",
-								port->name);
+		       port->name);
 
 	port->physport->ieee1284.phase = IEEE1284_PH_FWD_DATA;
 
@@ -770,9 +770,8 @@ static size_t parport_pc_compat_write_block_pio(struct parport *port,
 				     PARPORT_STATUS_BUSY,
 				     PARPORT_STATUS_BUSY);
 	if (r)
-		printk(KERN_DEBUG
-			"%s: BUSY timeout (%d) in compat_write_block_pio\n",
-			port->name, r);
+		printk(KERN_DEBUG "%s: BUSY timeout (%d) in compat_write_block_pio\n",
+		       port->name, r);
 
 	port->physport->ieee1284.phase = IEEE1284_PH_FWD_IDLE;
 
@@ -810,8 +809,8 @@ static size_t parport_pc_ecp_write_block_pio(struct parport *port,
 					     PARPORT_STATUS_PAPEROUT,
 					     PARPORT_STATUS_PAPEROUT);
 		if (r) {
-			printk(KERN_DEBUG "%s: PError timeout (%d) "
-				"in ecp_write_block_pio\n", port->name, r);
+			printk(KERN_DEBUG "%s: PError timeout (%d) in ecp_write_block_pio\n",
+			       port->name, r);
 		}
 	}
 
@@ -824,7 +823,7 @@ static size_t parport_pc_ecp_write_block_pio(struct parport *port,
 	r = change_mode(port, ECR_ECP); /* ECP FIFO */
 	if (r)
 		printk(KERN_DEBUG "%s: Warning change_mode ECR_ECP failed\n",
-								port->name);
+		       port->name);
 	port->physport->ieee1284.phase = IEEE1284_PH_FWD_DATA;
 
 	/* Write the data to the FIFO. */
@@ -867,8 +866,8 @@ static size_t parport_pc_ecp_write_block_pio(struct parport *port,
 		parport_frob_control(port, PARPORT_CONTROL_INIT, 0);
 		r = parport_wait_peripheral(port, PARPORT_STATUS_PAPEROUT, 0);
 		if (r)
-			printk(KERN_DEBUG "%s: PE,1 timeout (%d) "
-				"in ecp_write_block_pio\n", port->name, r);
+			printk(KERN_DEBUG "%s: PE,1 timeout (%d) in ecp_write_block_pio\n",
+			       port->name, r);
 
 		parport_frob_control(port,
 				      PARPORT_CONTROL_INIT,
@@ -877,17 +876,16 @@ static size_t parport_pc_ecp_write_block_pio(struct parport *port,
 					     PARPORT_STATUS_PAPEROUT,
 					     PARPORT_STATUS_PAPEROUT);
 		if (r)
-			printk(KERN_DEBUG "%s: PE,2 timeout (%d) "
-				"in ecp_write_block_pio\n", port->name, r);
+			printk(KERN_DEBUG "%s: PE,2 timeout (%d) in ecp_write_block_pio\n",
+			       port->name, r);
 	}
 
 	r = parport_wait_peripheral(port,
 				     PARPORT_STATUS_BUSY,
 				     PARPORT_STATUS_BUSY);
 	if (r)
-		printk(KERN_DEBUG
-			"%s: BUSY timeout (%d) in ecp_write_block_pio\n",
-			port->name, r);
+		printk(KERN_DEBUG "%s: BUSY timeout (%d) in ecp_write_block_pio\n",
+		       port->name, r);
 
 	port->physport->ieee1284.phase = IEEE1284_PH_FWD_IDLE;
 
@@ -1611,7 +1609,7 @@ static int parport_ECP_supported(struct parport *pb)
 	if (i <= priv->fifo_depth) {
 		if (verbose_probing)
 			printk(KERN_DEBUG "0x%lx: writeIntrThreshold is %d\n",
-				pb->base, i);
+			       pb->base, i);
 	} else
 		/* Number of bytes we know we can write if we get an
 		   interrupt. */
@@ -1664,14 +1662,14 @@ static int parport_ECP_supported(struct parport *pb)
 
 	if (verbose_probing) {
 		printk(KERN_DEBUG "0x%lx: PWord is %d bits\n",
-			pb->base, 8 * pword);
+		       pb->base, 8 * pword);
 
-		printk(KERN_DEBUG "0x%lx: Interrupts are ISA-%s\n", pb->base,
-			config & 0x80 ? "Level" : "Pulses");
+		printk(KERN_DEBUG "0x%lx: Interrupts are ISA-%s\n",
+		       pb->base, config & 0x80 ? "Level" : "Pulses");
 
 		configb = inb(CONFIGB(pb));
 		printk(KERN_DEBUG "0x%lx: ECP port cfgA=0x%02x cfgB=0x%02x\n",
-			pb->base, config, configb);
+		       pb->base, config, configb);
 		printk(KERN_DEBUG "0x%lx: ECP settings irq=", pb->base);
 		if ((configb >> 3) & 0x07)
 			pr_cont("%d", intrline[(configb >> 3) & 0x07]);
@@ -2453,8 +2451,7 @@ static int sio_via_probe(struct pci_dev *pdev, int autoirq, int autodma,
 		have_epp = 1;
 		break;
 	default:
-		printk(KERN_DEBUG
-			"parport_pc: probing current configuration\n");
+		printk(KERN_DEBUG "parport_pc: probing current configuration\n");
 		siofunc = VIA_FUNCTION_PROBE;
 		break;
 	}
@@ -2490,12 +2487,11 @@ static int sio_via_probe(struct pci_dev *pdev, int autoirq, int autodma,
 	port1 = inb(VIA_CONFIG_DATA) << 2;
 
 	printk(KERN_DEBUG "parport_pc: Current parallel port base: 0x%X\n",
-									port1);
+	       port1);
 	if (port1 == 0x3BC && have_epp) {
 		outb(via->viacfg_parport_base, VIA_CONFIG_INDEX);
 		outb((0x378 >> 2), VIA_CONFIG_DATA);
-		printk(KERN_DEBUG
-			"parport_pc: Parallel port base changed to 0x378\n");
+		printk(KERN_DEBUG "parport_pc: Parallel port base changed to 0x378\n");
 		port1 = 0x378;
 	}
 
@@ -2834,14 +2830,12 @@ static int parport_pc_pci_probe(struct pci_dev *dev,
 		/* TODO: test if sharing interrupts works */
 		irq = dev->irq;
 		if (irq == IRQ_NONE) {
-			printk(KERN_DEBUG
-	"PCI parallel port detected: %04x:%04x, I/O at %#lx(%#lx)\n",
-				id->vendor, id->device, io_lo, io_hi);
+			printk(KERN_DEBUG "PCI parallel port detected: %04x:%04x, I/O at %#lx(%#lx)\n",
+			       id->vendor, id->device, io_lo, io_hi);
 			irq = PARPORT_IRQ_NONE;
 		} else {
-			printk(KERN_DEBUG
-	"PCI parallel port detected: %04x:%04x, I/O at %#lx(%#lx), IRQ %d\n",
-				id->vendor, id->device, io_lo, io_hi, irq);
+			printk(KERN_DEBUG "PCI parallel port detected: %04x:%04x, I/O at %#lx(%#lx), IRQ %d\n",
+			       id->vendor, id->device, io_lo, io_hi, irq);
 		}
 		data->ports[count] =
 			parport_pc_probe_port(io_lo, io_hi, irq,
@@ -3113,8 +3107,8 @@ static int __init parport_parse_dma(const char *dmastr, int *val)
 #ifdef CONFIG_PCI
 static int __init parport_init_mode_setup(char *str)
 {
-	printk(KERN_DEBUG
-	     "parport_pc.c: Specified parameter parport_init_mode=%s\n", str);
+	printk(KERN_DEBUG "parport_pc.c: Specified parameter parport_init_mode=%s\n",
+	       str);
 
 	if (!strcmp(str, "spp"))
 		parport_init_mode = 1;
diff --git a/drivers/parport/probe.c b/drivers/parport/probe.c
index 10ed0d1..7e6d71 100644
--- a/drivers/parport/probe.c
+++ b/drivers/parport/probe.c
@@ -41,13 +41,13 @@ static void pretty_print(struct parport *port, int device)
 	pr_info("%s", port->name);
 
 	if (device >= 0)
-		printk (" (addr %d)", device);
+		pr_cont(" (addr %d)", device);
 
-	printk (": %s", classes[info->class].descr);
+	pr_cont(": %s", classes[info->class].descr);
 	if (info->class)
-		printk(", %s %s", info->mfr, info->model);
+		pr_cont(", %s %s", info->mfr, info->model);
 
-	printk("\n");
+	pr_cont("\n");
 }
 
 static void parse_data(struct parport *port, int device, char *str)
@@ -178,9 +178,8 @@ static ssize_t parport_read_device_id (struct parport *port, char *buffer,
 		 * just return constant nibble forever. This catches
 		 * also those cases. */
 		if (idlens[0] == 0 || idlens[0] > 0xFFF) {
-			printk (KERN_DEBUG "%s: reported broken Device ID"
-				" length of %#zX bytes\n",
-				port->name, idlens[0]);
+			printk(KERN_DEBUG "%s: reported broken Device ID length of %#zX bytes\n",
+			       port->name, idlens[0]);
 			return -EIO;
 		}
 		numidlens = 2;
@@ -202,10 +201,8 @@ static ssize_t parport_read_device_id (struct parport *port, char *buffer,
 
 		if (port->physport->ieee1284.phase != IEEE1284_PH_HBUSY_DAVAIL) {
 			if (belen != len) {
-				printk (KERN_DEBUG "%s: Device ID was %zd bytes"
-					" while device told it would be %d"
-					" bytes\n",
-					port->name, len, belen);
+				printk(KERN_DEBUG "%s: Device ID was %zd bytes while device told it would be %d bytes\n",
+				       port->name, len, belen);
 			}
 			goto done;
 		}
@@ -215,11 +212,9 @@ static ssize_t parport_read_device_id (struct parport *port, char *buffer,
 		 * the first 256 bytes or so that we must have read so
 		 * far. */
 		if (buffer[len-1] == ';') {
- 			printk (KERN_DEBUG "%s: Device ID reading stopped"
-				" before device told data not available. "
-				"Current idlen %u of %u, len bytes %02X %02X\n",
-				port->name, current_idlen, numidlens,
-				length[0], length[1]);
+			printk(KERN_DEBUG "%s: Device ID reading stopped before device told data not available. Current idlen %u of %u, len bytes %02X %02X\n",
+			       port->name, current_idlen, numidlens,
+			       length[0], length[1]);
 			goto done;
 		}
 	}
diff --git a/drivers/parport/share.c b/drivers/parport/share.c
index b0f451..3169fee 100644
--- a/drivers/parport/share.c
+++ b/drivers/parport/share.c
@@ -720,8 +720,7 @@ parport_register_device(struct parport *port, const char *name,
 
 	if (port->physport->flags & PARPORT_FLAG_EXCL) {
 		/* An exclusive device is registered. */
-		printk(KERN_DEBUG "%s: no more devices allowed\n",
-			port->name);
+		printk(KERN_DEBUG "%s: no more devices allowed\n", port->name);
 		return NULL;
 	}
 
@@ -789,9 +788,8 @@ parport_register_device(struct parport *port, const char *name,
 	if (flags & PARPORT_DEV_EXCL) {
 		if (port->physport->devices) {
 			spin_unlock(&port->physport->pardevice_lock);
-			printk(KERN_DEBUG
-				"%s: cannot grant exclusive access for device %s\n",
-				port->name, name);
+			printk(KERN_DEBUG "%s: cannot grant exclusive access for device %s\n",
+			       port->name, name);
 			goto out_free_all;
 		}
 		port->flags |= PARPORT_FLAG_EXCL;
@@ -1259,7 +1257,8 @@ int parport_claim_or_block(struct pardevice *dev)
 	r = parport_claim(dev);
 	if (r == -EAGAIN) {
 #ifdef PARPORT_DEBUG_SHARING
-		printk(KERN_DEBUG "%s: parport_claim() returned -EAGAIN\n", dev->name);
+		printk(KERN_DEBUG "%s: parport_claim() returned -EAGAIN\n",
+		       dev->name);
 #endif
 		/*
 		 * FIXME!!! Use the proper locking for dev->waiting,
@@ -1292,7 +1291,7 @@ int parport_claim_or_block(struct pardevice *dev)
 		if (dev->port->physport->cad != dev)
 			printk(KERN_DEBUG "%s: exiting parport_claim_or_block but %s owns port!\n",
 			       dev->name, dev->port->physport->cad ?
-			       dev->port->physport->cad->name:"nobody");
+			       dev->port->physport->cad->name : "nobody");
 #endif
 	}
 	dev->waiting = 0;
-- 
2.24.0

Re: [PATCH 2/7] parport: Use more comon logging styles

[Helge Deller]

On 28.02.20 09:32, Joe Perches wrote:
> Some of the logging can be poorly formatted because of unexpected
> line breaks given printks without KERN_CONT that should be pr_cont.
>
> Miscellanea:
>
> o Remove unnecessary spaces between function name and open parenthesis
> o Convert bare printks to pr_<level> where appropriate
> o Convert embedded function names to use %s, __func__
> o Coalesce formats
> o Realign arguments
> o Use do {} while (0) in a macro and not a bare if
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/parport/parport_gsc.c  | 10 ++--
>  drivers/parport/parport_gsc.h  | 21 ++++----
>  drivers/parport/parport_ip32.c | 92 ++++++++++++++++++----------------
>  drivers/parport/parport_pc.c   | 62 +++++++++++------------
>  drivers/parport/probe.c        | 27 ++++------
>  drivers/parport/share.c        | 13 +++--
>  6 files changed, 108 insertions(+), 117 deletions(-)
>
> diff --git a/drivers/parport/parport_gsc.c b/drivers/parport/parport_gsc.c
> index 81082d..cb6a08 100644
> --- a/drivers/parport/parport_gsc.c
> +++ b/drivers/parport/parport_gsc.c
> @@ -238,14 +238,14 @@ struct parport *parport_gsc_probe_port(unsigned long base,
>
>  	priv = kzalloc (sizeof (struct parport_gsc_private), GFP_KERNEL);
>  	if (!priv) {
> -		printk (KERN_DEBUG "parport (0x%lx): no memory!\n", base);
> +		printk(KERN_DEBUG "parport (0x%lx): no memory!\n", base);

pr_warn() instead?

>  		return NULL;
>  	}
>  	ops = kmemdup(&parport_gsc_ops, sizeof(struct parport_operations),
>  		      GFP_KERNEL);
>  	if (!ops) {
> -		printk (KERN_DEBUG "parport (0x%lx): no memory for ops!\n",
> -			base);
> +		printk(KERN_DEBUG "parport (0x%lx): no memory for ops!\n",
> +		       base);

pr_warn() instead too ?

>  		kfree (priv);
>  		return NULL;
>  	}
> @@ -359,11 +359,11 @@ static int __init parport_init_chip(struct parisc_device *dev)
>  	if (boot_cpu_data.cpu_type > pcxt && !pdc_add_valid(port+4)) {
>
>  		/* Initialize bidirectional-mode (0x10) & data-tranfer-mode #1 (0x20) */
> -		printk("%s: initialize bidirectional-mode.\n", __func__);
> +		pr_info("%s: initialize bidirectional-mode\n", __func__);
>  		parport_writeb ( (0x10 + 0x20), port + 4);
>
>  	} else {
> -		printk("%s: enhanced parport-modes not supported.\n", __func__);
> +		pr_info("%s: enhanced parport-modes not supported\n", __func__);
>  	}
>
>  	p = parport_gsc_probe_port(port, 0, dev->irq,
> diff --git a/drivers/parport/parport_gsc.h b/drivers/parport/parport_gsc.h
> index 4c4d3c..9301217 100644
> --- a/drivers/parport/parport_gsc.h
> +++ b/drivers/parport/parport_gsc.h
> @@ -71,7 +71,7 @@ struct parport_gsc_private {
>  static inline void parport_gsc_write_data(struct parport *p, unsigned char d)
>  {
>  #ifdef DEBUG_PARPORT
> -	printk (KERN_DEBUG "parport_gsc_write_data(%p,0x%02x)\n", p, d);
> +	printk(KERN_DEBUG "%s(%p,0x%02x)\n", __func__, p, d);

pr_debug() ?

>  #endif
>  	parport_writeb(d, DATA(p));
>  }
> @@ -80,8 +80,7 @@ static inline unsigned char parport_gsc_read_data(struct parport *p)
>  {
>  	unsigned char val = parport_readb (DATA (p));
>  #ifdef DEBUG_PARPORT
> -	printk (KERN_DEBUG "parport_gsc_read_data(%p) = 0x%02x\n",
> -		p, val);
> +	printk(KERN_DEBUG "%s(%p) = 0x%02x\n", __func__, p, val);

pr_debug() ?

>  #endif
>  	return val;
>  }
> @@ -95,9 +94,9 @@ static inline unsigned char __parport_gsc_frob_control(struct parport *p,
>  	struct parport_gsc_private *priv = p->physport->private_data;
>  	unsigned char ctr = priv->ctr;
>  #ifdef DEBUG_PARPORT
> -	printk (KERN_DEBUG
> -		"__parport_gsc_frob_control(%02x,%02x): %02x -> %02x\n",
> -		mask, val, ctr, ((ctr & ~mask) ^ val) & priv->ctr_writable);
> +	printk(KERN_DEBUG "%s(%02x,%02x): %02x -> %02x\n",

pr_debug() ?

> +	       __func__, mask, val,
> +	       ctr, ((ctr & ~mask) ^ val) & priv->ctr_writable);
>  #endif
>  	ctr = (ctr & ~mask) ^ val;
>  	ctr &= priv->ctr_writable; /* only write writable bits. */
> @@ -126,8 +125,8 @@ static inline void parport_gsc_write_control(struct parport *p,
>
>  	/* Take this out when drivers have adapted to newer interface. */
>  	if (d & 0x20) {
> -		printk (KERN_DEBUG "%s (%s): use data_reverse for this!\n",
> -			p->name, p->cad->name);
> +		printk(KERN_DEBUG "%s (%s): use data_reverse for this!\n",
> +		       p->name, p->cad->name);

here too?

>  		parport_gsc_data_reverse (p);
>  	}
>
> @@ -155,9 +154,9 @@ static inline unsigned char parport_gsc_frob_control(struct parport *p,
>
>  	/* Take this out when drivers have adapted to newer interface. */
>  	if (mask & 0x20) {
> -		printk (KERN_DEBUG "%s (%s): use data_%s for this!\n",
> -			p->name, p->cad->name,
> -			(val & 0x20) ? "reverse" : "forward");
> +		printk(KERN_DEBUG "%s (%s): use data_%s for this!\n",

and here?

> +		       p->name, p->cad->name,
> +		       (val & 0x20) ? "reverse" : "forward");
>  		if (val & 0x20)
>  			parport_gsc_data_reverse (p);
>  		else
> diff --git a/drivers/parport/parport_ip32.c b/drivers/parport/parport_ip32.c
> index 4f76e4..48b084 100644
> --- a/drivers/parport/parport_ip32.c
> +++ b/drivers/parport/parport_ip32.c
> @@ -328,19 +328,19 @@ static void parport_ip32_dump_state(struct parport *p, char *str,
>  						     "TST", "CFG"};
>  		unsigned int ecr = readb(priv->regs.ecr);
>  		printk(KERN_DEBUG PPIP32 "    ecr=0x%02x", ecr);
> -		printk(" %s",
> -		       ecr_modes[(ecr & ECR_MODE_MASK) >> ECR_MODE_SHIFT]);
> +		pr_cont(" %s",
> +			ecr_modes[(ecr & ECR_MODE_MASK) >> ECR_MODE_SHIFT]);
>  		if (ecr & ECR_nERRINTR)
> -			printk(",nErrIntrEn");
> +			pr_cont(",nErrIntrEn");
>  		if (ecr & ECR_DMAEN)
> -			printk(",dmaEn");
> +			pr_cont(",dmaEn");
>  		if (ecr & ECR_SERVINTR)
> -			printk(",serviceIntr");
> +			pr_cont(",serviceIntr");
>  		if (ecr & ECR_F_FULL)
> -			printk(",f_full");
> +			pr_cont(",f_full");
>  		if (ecr & ECR_F_EMPTY)
> -			printk(",f_empty");
> -		printk("\n");
> +			pr_cont(",f_empty");
> +		pr_cont("\n");
>  	}
>  	if (show_ecp_config) {
>  		unsigned int oecr, cnfgA, cnfgB;
> @@ -352,52 +352,53 @@ static void parport_ip32_dump_state(struct parport *p, char *str,
>  		writeb(ECR_MODE_PS2, priv->regs.ecr);
>  		writeb(oecr, priv->regs.ecr);
>  		printk(KERN_DEBUG PPIP32 "    cnfgA=0x%02x", cnfgA);
> -		printk(" ISA-%s", (cnfgA & CNFGA_IRQ) ? "Level" : "Pulses");
> +		pr_cont(" ISA-%s", (cnfgA & CNFGA_IRQ) ? "Level" : "Pulses");
>  		switch (cnfgA & CNFGA_ID_MASK) {
>  		case CNFGA_ID_8:
> -			printk(",8 bits");
> +			pr_cont(",8 bits");
>  			break;
>  		case CNFGA_ID_16:
> -			printk(",16 bits");
> +			pr_cont(",16 bits");
>  			break;
>  		case CNFGA_ID_32:
> -			printk(",32 bits");
> +			pr_cont(",32 bits");
>  			break;
>  		default:
> -			printk(",unknown ID");
> +			pr_cont(",unknown ID");
>  			break;
>  		}
>  		if (!(cnfgA & CNFGA_nBYTEINTRANS))
> -			printk(",ByteInTrans");
> +			pr_cont(",ByteInTrans");
>  		if ((cnfgA & CNFGA_ID_MASK) != CNFGA_ID_8)
> -			printk(",%d byte%s left", cnfgA & CNFGA_PWORDLEFT,
> -			       ((cnfgA & CNFGA_PWORDLEFT) > 1) ? "s" : "");
> -		printk("\n");
> +			pr_cont(",%d byte%s left",
> +				cnfgA & CNFGA_PWORDLEFT,
> +				((cnfgA & CNFGA_PWORDLEFT) > 1) ? "s" : "");
> +		pr_cont("\n");
>  		printk(KERN_DEBUG PPIP32 "    cnfgB=0x%02x", cnfgB);
> -		printk(" irq=%u,dma=%u",
> -		       (cnfgB & CNFGB_IRQ_MASK) >> CNFGB_IRQ_SHIFT,
> -		       (cnfgB & CNFGB_DMA_MASK) >> CNFGB_DMA_SHIFT);
> -		printk(",intrValue=%d", !!(cnfgB & CNFGB_INTRVAL));
> +		pr_cont(" irq=%u,dma=%u",
> +			(cnfgB & CNFGB_IRQ_MASK) >> CNFGB_IRQ_SHIFT,
> +			(cnfgB & CNFGB_DMA_MASK) >> CNFGB_DMA_SHIFT);
> +		pr_cont(",intrValue=%d", !!(cnfgB & CNFGB_INTRVAL));
>  		if (cnfgB & CNFGB_COMPRESS)
> -			printk(",compress");
> -		printk("\n");
> +			pr_cont(",compress");
> +		pr_cont("\n");
>  	}
>  	for (i = 0; i < 2; i++) {
>  		unsigned int dcr = i ? priv->dcr_cache : readb(priv->regs.dcr);
>  		printk(KERN_DEBUG PPIP32 "    dcr(%s)=0x%02x",
>  		       i ? "soft" : "hard", dcr);
> -		printk(" %s", (dcr & DCR_DIR) ? "rev" : "fwd");
> +		pr_cont(" %s", (dcr & DCR_DIR) ? "rev" : "fwd");
>  		if (dcr & DCR_IRQ)
> -			printk(",ackIntEn");
> +			pr_cont(",ackIntEn");
>  		if (!(dcr & DCR_SELECT))
> -			printk(",nSelectIn");
> +			pr_cont(",nSelectIn");
>  		if (dcr & DCR_nINIT)
> -			printk(",nInit");
> +			pr_cont(",nInit");
>  		if (!(dcr & DCR_AUTOFD))
> -			printk(",nAutoFD");
> +			pr_cont(",nAutoFD");
>  		if (!(dcr & DCR_STROBE))
> -			printk(",nStrobe");
> -		printk("\n");
> +			pr_cont(",nStrobe");
> +		pr_cont("\n");
>  	}
>  #define sep (f++ ? ',' : ' ')
>  	{
> @@ -405,20 +406,20 @@ static void parport_ip32_dump_state(struct parport *p, char *str,
>  		unsigned int dsr = readb(priv->regs.dsr);
>  		printk(KERN_DEBUG PPIP32 "    dsr=0x%02x", dsr);
>  		if (!(dsr & DSR_nBUSY))
> -			printk("%cBusy", sep);
> +			pr_cont("%cBusy", sep);
>  		if (dsr & DSR_nACK)
> -			printk("%cnAck", sep);
> +			pr_cont("%cnAck", sep);
>  		if (dsr & DSR_PERROR)
> -			printk("%cPError", sep);
> +			pr_cont("%cPError", sep);
>  		if (dsr & DSR_SELECT)
> -			printk("%cSelect", sep);
> +			pr_cont("%cSelect", sep);
>  		if (dsr & DSR_nFAULT)
> -			printk("%cnFault", sep);
> +			pr_cont("%cnFault", sep);
>  		if (!(dsr & DSR_nPRINT))
> -			printk("%c(Print)", sep);
> +			pr_cont("%c(Print)", sep);
>  		if (dsr & DSR_TIMEOUT)
> -			printk("%cTimeout", sep);
> -		printk("\n");
> +			pr_cont("%cTimeout", sep);
> +		pr_cont("\n");
>  	}
>  #undef sep
>  }
> @@ -1703,7 +1704,7 @@ static size_t parport_ip32_ecp_write_data(struct parport *p,
>
>  		/* Event 49: PError goes high. */
>  		if (parport_wait_peripheral(p, DSR_PERROR, DSR_PERROR)) {
> -			printk(KERN_DEBUG PPIP32 "%s: PError timeout in %s",
> +			printk(KERN_DEBUG PPIP32 "%s: PError timeout in %s\n",

and here?

>  			       p->name, __func__);
>  			physport->ieee1284.phase = IEEE1284_PH_ECP_DIR_UNKNOWN;
>  			return 0;
> @@ -2132,10 +2133,13 @@ static __init struct parport *parport_ip32_probe_port(void)
>  	/* Print out what we found */
>  	pr_info("%s: SGI IP32 at 0x%lx (0x%lx)", p->name, p->base, p->base_hi);
>  	if (p->irq != PARPORT_IRQ_NONE)
> -		printk(", irq %d", p->irq);
> -	printk(" [");
> -#define printmode(x)	if (p->modes & PARPORT_MODE_##x)		\
> -				printk("%s%s", f++ ? "," : "", #x)
> +		pr_cont(", irq %d", p->irq);
> +	pr_cont(" [");
> +#define printmode(x)							\
> +do {									\
> +	if (p->modes & PARPORT_MODE_##x)				\
> +		pr_cont("%s%s", f++ ? "," : "", #x);			\
> +} while (0)
>  	{
>  		unsigned int f = 0;
>  		printmode(PCSPP);
> @@ -2146,7 +2150,7 @@ static __init struct parport *parport_ip32_probe_port(void)
>  		printmode(DMA);
>  	}
>  #undef printmode
> -	printk("]\n");
> +	pr_cont("]\n");
>
>  	parport_announce_port(p);
>  	return p;
> diff --git a/drivers/parport/parport_pc.c b/drivers/parport/parport_pc.c
> index 2872c67..ab3b04f 100644
> --- a/drivers/parport/parport_pc.c
> +++ b/drivers/parport/parport_pc.c
> @@ -298,8 +298,8 @@ static size_t parport_pc_epp_read_data(struct parport *port, void *buf,
>  			status = inb(STATUS(port));
>  			if (status & 0x01) {
>  				/* EPP timeout should never occur... */
> -				printk(KERN_DEBUG
> -"%s: EPP timeout occurred while talking to w91284pic (should not have done)\n", port->name);
> +				printk(KERN_DEBUG "%s: EPP timeout occurred while talking to w91284pic (should not have done)\n",


here.

> +				       port->name);
>  				clear_epp_timeout(port);
>  			}
>  		}
> @@ -727,7 +727,7 @@ static size_t parport_pc_compat_write_block_pio(struct parport *port,
>  	r = change_mode(port, ECR_PPF); /* Parallel port FIFO */
>  	if (r)
>  		printk(KERN_DEBUG "%s: Warning change_mode ECR_PPF failed\n",

and here?


There are more such below...

Helge



> -								port->name);
> +		       port->name);
>
>  	port->physport->ieee1284.phase = IEEE1284_PH_FWD_DATA;
>
> @@ -770,9 +770,8 @@ static size_t parport_pc_compat_write_block_pio(struct parport *port,
>  				     PARPORT_STATUS_BUSY,
>  				     PARPORT_STATUS_BUSY);
>  	if (r)
> -		printk(KERN_DEBUG
> -			"%s: BUSY timeout (%d) in compat_write_block_pio\n",
> -			port->name, r);
> +		printk(KERN_DEBUG "%s: BUSY timeout (%d) in compat_write_block_pio\n",
> +		       port->name, r);
>
>  	port->physport->ieee1284.phase = IEEE1284_PH_FWD_IDLE;
>
> @@ -810,8 +809,8 @@ static size_t parport_pc_ecp_write_block_pio(struct parport *port,
>  					     PARPORT_STATUS_PAPEROUT,
>  					     PARPORT_STATUS_PAPEROUT);
>  		if (r) {
> -			printk(KERN_DEBUG "%s: PError timeout (%d) "
> -				"in ecp_write_block_pio\n", port->name, r);
> +			printk(KERN_DEBUG "%s: PError timeout (%d) in ecp_write_block_pio\n",
> +			       port->name, r);
>  		}
>  	}
>
> @@ -824,7 +823,7 @@ static size_t parport_pc_ecp_write_block_pio(struct parport *port,
>  	r = change_mode(port, ECR_ECP); /* ECP FIFO */
>  	if (r)
>  		printk(KERN_DEBUG "%s: Warning change_mode ECR_ECP failed\n",
> -								port->name);
> +		       port->name);
>  	port->physport->ieee1284.phase = IEEE1284_PH_FWD_DATA;
>
>  	/* Write the data to the FIFO. */
> @@ -867,8 +866,8 @@ static size_t parport_pc_ecp_write_block_pio(struct parport *port,
>  		parport_frob_control(port, PARPORT_CONTROL_INIT, 0);
>  		r = parport_wait_peripheral(port, PARPORT_STATUS_PAPEROUT, 0);
>  		if (r)
> -			printk(KERN_DEBUG "%s: PE,1 timeout (%d) "
> -				"in ecp_write_block_pio\n", port->name, r);
> +			printk(KERN_DEBUG "%s: PE,1 timeout (%d) in ecp_write_block_pio\n",
> +			       port->name, r);
>
>  		parport_frob_control(port,
>  				      PARPORT_CONTROL_INIT,
> @@ -877,17 +876,16 @@ static size_t parport_pc_ecp_write_block_pio(struct parport *port,
>  					     PARPORT_STATUS_PAPEROUT,
>  					     PARPORT_STATUS_PAPEROUT);
>  		if (r)
> -			printk(KERN_DEBUG "%s: PE,2 timeout (%d) "
> -				"in ecp_write_block_pio\n", port->name, r);
> +			printk(KERN_DEBUG "%s: PE,2 timeout (%d) in ecp_write_block_pio\n",
> +			       port->name, r);
>  	}
>
>  	r = parport_wait_peripheral(port,
>  				     PARPORT_STATUS_BUSY,
>  				     PARPORT_STATUS_BUSY);
>  	if (r)
> -		printk(KERN_DEBUG
> -			"%s: BUSY timeout (%d) in ecp_write_block_pio\n",
> -			port->name, r);
> +		printk(KERN_DEBUG "%s: BUSY timeout (%d) in ecp_write_block_pio\n",
> +		       port->name, r);
>
>  	port->physport->ieee1284.phase = IEEE1284_PH_FWD_IDLE;
>
> @@ -1611,7 +1609,7 @@ static int parport_ECP_supported(struct parport *pb)
>  	if (i <= priv->fifo_depth) {
>  		if (verbose_probing)
>  			printk(KERN_DEBUG "0x%lx: writeIntrThreshold is %d\n",
> -				pb->base, i);
> +			       pb->base, i);
>  	} else
>  		/* Number of bytes we know we can write if we get an
>  		   interrupt. */
> @@ -1664,14 +1662,14 @@ static int parport_ECP_supported(struct parport *pb)
>
>  	if (verbose_probing) {
>  		printk(KERN_DEBUG "0x%lx: PWord is %d bits\n",
> -			pb->base, 8 * pword);
> +		       pb->base, 8 * pword);
>
> -		printk(KERN_DEBUG "0x%lx: Interrupts are ISA-%s\n", pb->base,
> -			config & 0x80 ? "Level" : "Pulses");
> +		printk(KERN_DEBUG "0x%lx: Interrupts are ISA-%s\n",
> +		       pb->base, config & 0x80 ? "Level" : "Pulses");
>
>  		configb = inb(CONFIGB(pb));
>  		printk(KERN_DEBUG "0x%lx: ECP port cfgA=0x%02x cfgB=0x%02x\n",
> -			pb->base, config, configb);
> +		       pb->base, config, configb);
>  		printk(KERN_DEBUG "0x%lx: ECP settings irq=", pb->base);
>  		if ((configb >> 3) & 0x07)
>  			pr_cont("%d", intrline[(configb >> 3) & 0x07]);
> @@ -2453,8 +2451,7 @@ static int sio_via_probe(struct pci_dev *pdev, int autoirq, int autodma,
>  		have_epp = 1;
>  		break;
>  	default:
> -		printk(KERN_DEBUG
> -			"parport_pc: probing current configuration\n");
> +		printk(KERN_DEBUG "parport_pc: probing current configuration\n");
>  		siofunc = VIA_FUNCTION_PROBE;
>  		break;
>  	}
> @@ -2490,12 +2487,11 @@ static int sio_via_probe(struct pci_dev *pdev, int autoirq, int autodma,
>  	port1 = inb(VIA_CONFIG_DATA) << 2;
>
>  	printk(KERN_DEBUG "parport_pc: Current parallel port base: 0x%X\n",
> -									port1);
> +	       port1);
>  	if (port1 == 0x3BC && have_epp) {
>  		outb(via->viacfg_parport_base, VIA_CONFIG_INDEX);
>  		outb((0x378 >> 2), VIA_CONFIG_DATA);
> -		printk(KERN_DEBUG
> -			"parport_pc: Parallel port base changed to 0x378\n");
> +		printk(KERN_DEBUG "parport_pc: Parallel port base changed to 0x378\n");
>  		port1 = 0x378;
>  	}
>
> @@ -2834,14 +2830,12 @@ static int parport_pc_pci_probe(struct pci_dev *dev,
>  		/* TODO: test if sharing interrupts works */
>  		irq = dev->irq;
>  		if (irq == IRQ_NONE) {
> -			printk(KERN_DEBUG
> -	"PCI parallel port detected: %04x:%04x, I/O at %#lx(%#lx)\n",
> -				id->vendor, id->device, io_lo, io_hi);
> +			printk(KERN_DEBUG "PCI parallel port detected: %04x:%04x, I/O at %#lx(%#lx)\n",
> +			       id->vendor, id->device, io_lo, io_hi);
>  			irq = PARPORT_IRQ_NONE;
>  		} else {
> -			printk(KERN_DEBUG
> -	"PCI parallel port detected: %04x:%04x, I/O at %#lx(%#lx), IRQ %d\n",
> -				id->vendor, id->device, io_lo, io_hi, irq);
> +			printk(KERN_DEBUG "PCI parallel port detected: %04x:%04x, I/O at %#lx(%#lx), IRQ %d\n",
> +			       id->vendor, id->device, io_lo, io_hi, irq);
>  		}
>  		data->ports[count] =
>  			parport_pc_probe_port(io_lo, io_hi, irq,
> @@ -3113,8 +3107,8 @@ static int __init parport_parse_dma(const char *dmastr, int *val)
>  #ifdef CONFIG_PCI
>  static int __init parport_init_mode_setup(char *str)
>  {
> -	printk(KERN_DEBUG
> -	     "parport_pc.c: Specified parameter parport_init_mode=%s\n", str);
> +	printk(KERN_DEBUG "parport_pc.c: Specified parameter parport_init_mode=%s\n",
> +	       str);
>
>  	if (!strcmp(str, "spp"))
>  		parport_init_mode = 1;
> diff --git a/drivers/parport/probe.c b/drivers/parport/probe.c
> index 10ed0d1..7e6d71 100644
> --- a/drivers/parport/probe.c
> +++ b/drivers/parport/probe.c
> @@ -41,13 +41,13 @@ static void pretty_print(struct parport *port, int device)
>  	pr_info("%s", port->name);
>
>  	if (device >= 0)
> -		printk (" (addr %d)", device);
> +		pr_cont(" (addr %d)", device);
>
> -	printk (": %s", classes[info->class].descr);
> +	pr_cont(": %s", classes[info->class].descr);
>  	if (info->class)
> -		printk(", %s %s", info->mfr, info->model);
> +		pr_cont(", %s %s", info->mfr, info->model);
>
> -	printk("\n");
> +	pr_cont("\n");
>  }
>
>  static void parse_data(struct parport *port, int device, char *str)
> @@ -178,9 +178,8 @@ static ssize_t parport_read_device_id (struct parport *port, char *buffer,
>  		 * just return constant nibble forever. This catches
>  		 * also those cases. */
>  		if (idlens[0] == 0 || idlens[0] > 0xFFF) {
> -			printk (KERN_DEBUG "%s: reported broken Device ID"
> -				" length of %#zX bytes\n",
> -				port->name, idlens[0]);
> +			printk(KERN_DEBUG "%s: reported broken Device ID length of %#zX bytes\n",
> +			       port->name, idlens[0]);
>  			return -EIO;
>  		}
>  		numidlens = 2;
> @@ -202,10 +201,8 @@ static ssize_t parport_read_device_id (struct parport *port, char *buffer,
>
>  		if (port->physport->ieee1284.phase != IEEE1284_PH_HBUSY_DAVAIL) {
>  			if (belen != len) {
> -				printk (KERN_DEBUG "%s: Device ID was %zd bytes"
> -					" while device told it would be %d"
> -					" bytes\n",
> -					port->name, len, belen);
> +				printk(KERN_DEBUG "%s: Device ID was %zd bytes while device told it would be %d bytes\n",
> +				       port->name, len, belen);
>  			}
>  			goto done;
>  		}
> @@ -215,11 +212,9 @@ static ssize_t parport_read_device_id (struct parport *port, char *buffer,
>  		 * the first 256 bytes or so that we must have read so
>  		 * far. */
>  		if (buffer[len-1] == ';') {
> - 			printk (KERN_DEBUG "%s: Device ID reading stopped"
> -				" before device told data not available. "
> -				"Current idlen %u of %u, len bytes %02X %02X\n",
> -				port->name, current_idlen, numidlens,
> -				length[0], length[1]);
> +			printk(KERN_DEBUG "%s: Device ID reading stopped before device told data not available. Current idlen %u of %u, len bytes %02X %02X\n",
> +			       port->name, current_idlen, numidlens,
> +			       length[0], length[1]);
>  			goto done;
>  		}
>  	}
> diff --git a/drivers/parport/share.c b/drivers/parport/share.c
> index b0f451..3169fee 100644
> --- a/drivers/parport/share.c
> +++ b/drivers/parport/share.c
> @@ -720,8 +720,7 @@ parport_register_device(struct parport *port, const char *name,
>
>  	if (port->physport->flags & PARPORT_FLAG_EXCL) {
>  		/* An exclusive device is registered. */
> -		printk(KERN_DEBUG "%s: no more devices allowed\n",
> -			port->name);
> +		printk(KERN_DEBUG "%s: no more devices allowed\n", port->name);
>  		return NULL;
>  	}
>
> @@ -789,9 +788,8 @@ parport_register_device(struct parport *port, const char *name,
>  	if (flags & PARPORT_DEV_EXCL) {
>  		if (port->physport->devices) {
>  			spin_unlock(&port->physport->pardevice_lock);
> -			printk(KERN_DEBUG
> -				"%s: cannot grant exclusive access for device %s\n",
> -				port->name, name);
> +			printk(KERN_DEBUG "%s: cannot grant exclusive access for device %s\n",
> +			       port->name, name);
>  			goto out_free_all;
>  		}
>  		port->flags |= PARPORT_FLAG_EXCL;
> @@ -1259,7 +1257,8 @@ int parport_claim_or_block(struct pardevice *dev)
>  	r = parport_claim(dev);
>  	if (r == -EAGAIN) {
>  #ifdef PARPORT_DEBUG_SHARING
> -		printk(KERN_DEBUG "%s: parport_claim() returned -EAGAIN\n", dev->name);
> +		printk(KERN_DEBUG "%s: parport_claim() returned -EAGAIN\n",
> +		       dev->name);
>  #endif
>  		/*
>  		 * FIXME!!! Use the proper locking for dev->waiting,
> @@ -1292,7 +1291,7 @@ int parport_claim_or_block(struct pardevice *dev)
>  		if (dev->port->physport->cad != dev)
>  			printk(KERN_DEBUG "%s: exiting parport_claim_or_block but %s owns port!\n",
>  			       dev->name, dev->port->physport->cad ?
> -			       dev->port->physport->cad->name:"nobody");
> +			       dev->port->physport->cad->name : "nobody");
>  #endif
>  	}
>  	dev->waiting = 0;
>

Re: [PATCH 2/7] parport: Use more comon logging styles

[Joe Perches]

On Mon, 2020-03-02 at 20:29 +0100, Helge Deller wrote:
> On 28.02.20 09:32, Joe Perches wrote:
> > Some of the logging can be poorly formatted because of unexpected
> > line breaks given printks without KERN_CONT that should be pr_cont.
[]
> > diff --git a/drivers/parport/parport_gsc.c b/drivers/parport/parport_gsc.c
[]
> > @@ -238,14 +238,14 @@ struct parport *parport_gsc_probe_port(unsigned long base,
> > 
> >  	priv = kzalloc (sizeof (struct parport_gsc_private), GFP_KERNEL);
> >  	if (!priv) {
> > -		printk (KERN_DEBUG "parport (0x%lx): no memory!\n", base);
> > +		printk(KERN_DEBUG "parport (0x%lx): no memory!\n", base);
> 
> pr_warn() instead?

For all of your remarks, the intent here is to keep the
same output.

Logging level changes and printk(KERN_DEBUG -> pr_debug(
conversions cause the dmesg output to be different.

	printk(KERN_DEBUG...)

is always emitted when the console level allows but

	pr_debug(...)

is not normally compiled in at all.

So it's possible for all printk(KERN_DEBUG to be pr_debug
but that causes no logging at all to be emitted when
DEBUG is not defined or CONFIG_DYNAMIC_DEBUG not enabled.

Re: [PATCH 2/7] parport: Use more comon logging styles

[Helge Deller]

On 02.03.20 20:39, Joe Perches wrote:
> On Mon, 2020-03-02 at 20:29 +0100, Helge Deller wrote:
>> On 28.02.20 09:32, Joe Perches wrote:
>>> Some of the logging can be poorly formatted because of unexpected
>>> line breaks given printks without KERN_CONT that should be pr_cont.
> []
>>> diff --git a/drivers/parport/parport_gsc.c b/drivers/parport/parport_gsc.c
> []
>>> @@ -238,14 +238,14 @@ struct parport *parport_gsc_probe_port(unsigned long base,
>>>
>>>  	priv = kzalloc (sizeof (struct parport_gsc_private), GFP_KERNEL);
>>>  	if (!priv) {
>>> -		printk (KERN_DEBUG "parport (0x%lx): no memory!\n", base);
>>> +		printk(KERN_DEBUG "parport (0x%lx): no memory!\n", base);
>>
>> pr_warn() instead?
>
> For all of your remarks, the intent here is to keep the
> same output.

Ok.

> Logging level changes and printk(KERN_DEBUG -> pr_debug(
> conversions cause the dmesg output to be different.
>
> 	printk(KERN_DEBUG...)
>
> is always emitted when the console level allows but
>
> 	pr_debug(...)
>
> is not normally compiled in at all.
>
> So it's possible for all printk(KERN_DEBUG to be pr_debug
> but that causes no logging at all to be emitted when
> DEBUG is not defined or CONFIG_DYNAMIC_DEBUG not enabled.

Ok, then keep it as you had.

Thanks!
Helge

[PATCH 3/7] parport: daisy: Convert DPRINTK to pr_debug

[Joe Perches]

Use a more common logging style.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/parport/daisy.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/drivers/parport/daisy.c b/drivers/parport/daisy.c
index f87cc80..6d78ec 100644
--- a/drivers/parport/daisy.c
+++ b/drivers/parport/daisy.c
@@ -30,12 +30,6 @@
 
 #undef DEBUG
 
-#ifdef DEBUG
-#define DPRINTK(stuff...) printk(stuff)
-#else
-#define DPRINTK(stuff...)
-#endif
-
 static struct daisydev {
 	struct daisydev *next;
 	struct parport *port;
@@ -321,8 +315,7 @@ static int cpp_daisy(struct parport *port, int cmd)
 		  | PARPORT_STATUS_PAPEROUT
 		  | PARPORT_STATUS_SELECT
 		  | PARPORT_STATUS_ERROR)) {
-		DPRINTK(KERN_DEBUG "%s: cpp_daisy: aa5500ff(%02x)\n",
-			 port->name, s);
+		pr_debug("%s: cpp_daisy: aa5500ff(%02x)\n", port->name, s);
 		return -ENXIO;
 	}
 
@@ -332,8 +325,7 @@ static int cpp_daisy(struct parport *port, int cmd)
 					  | PARPORT_STATUS_SELECT
 					  | PARPORT_STATUS_ERROR);
 	if (s != (PARPORT_STATUS_SELECT | PARPORT_STATUS_ERROR)) {
-		DPRINTK(KERN_DEBUG "%s: cpp_daisy: aa5500ff87(%02x)\n",
-			 port->name, s);
+		pr_debug("%s: cpp_daisy: aa5500ff87(%02x)\n", port->name, s);
 		return -ENXIO;
 	}
 
@@ -368,7 +360,7 @@ static int cpp_mux(struct parport *port, int cmd)
 
 	s = parport_read_status(port);
 	if (!(s & PARPORT_STATUS_ACK)) {
-		DPRINTK(KERN_DEBUG "%s: cpp_mux: aa55f00f52ad%02x(%02x)\n",
+		pr_debug("%s: cpp_mux: aa55f00f52ad%02x(%02x)\n",
 			 port->name, cmd, s);
 		return -EIO;
 	}
@@ -454,8 +446,7 @@ static int assign_addrs(struct parport *port)
 		  | PARPORT_STATUS_PAPEROUT
 		  | PARPORT_STATUS_SELECT
 		  | PARPORT_STATUS_ERROR)) {
-		DPRINTK(KERN_DEBUG "%s: assign_addrs: aa5500ff(%02x)\n",
-			 port->name, s);
+		pr_debug("%s: assign_addrs: aa5500ff(%02x)\n", port->name, s);
 		return 0;
 	}
 
@@ -465,8 +456,7 @@ static int assign_addrs(struct parport *port)
 					  | PARPORT_STATUS_SELECT
 					  | PARPORT_STATUS_ERROR);
 	if (s != (PARPORT_STATUS_SELECT | PARPORT_STATUS_ERROR)) {
-		DPRINTK(KERN_DEBUG "%s: assign_addrs: aa5500ff87(%02x)\n",
-			 port->name, s);
+		pr_debug("%s: assign_addrs: aa5500ff87(%02x)\n", port->name, s);
 		return 0;
 	}
 
@@ -503,8 +493,7 @@ static int assign_addrs(struct parport *port)
 
 	parport_write_data(port, 0xff); udelay(2);
 	detected = numdevs - thisdev;
-	DPRINTK(KERN_DEBUG "%s: Found %d daisy-chained devices\n", port->name,
-		 detected);
+	pr_debug("%s: Found %d daisy-chained devices\n", port->name, detected);
 
 	/* Ask the new devices to introduce themselves. */
 	deviceid = kmalloc(1024, GFP_KERNEL);
-- 
2.24.0

[PATCH 4/7] parport_amiga: Convert DPRINTK to pr_debug

[Joe Perches]

Use a more common logging style.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/parport/parport_amiga.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/parport/parport_amiga.c b/drivers/parport/parport_amiga.c
index 8c7a598a..1e88bc 100644
--- a/drivers/parport/parport_amiga.c
+++ b/drivers/parport/parport_amiga.c
@@ -28,16 +28,10 @@
 #include <asm/amigaints.h>
 
 #undef DEBUG
-#ifdef DEBUG
-#define DPRINTK printk
-#else
-#define DPRINTK(x...)	do { } while (0)
-#endif
-
 
 static void amiga_write_data(struct parport *p, unsigned char data)
 {
-	DPRINTK(KERN_DEBUG "write_data %c\n",data);
+	pr_debug("write_data %c\n", data);
 	/* Triggers also /STROBE. This behavior cannot be changed */
 	ciaa.prb = data;
 	mb();
@@ -59,13 +53,13 @@ static unsigned char control_amiga_to_pc(unsigned char control)
 
 static void amiga_write_control(struct parport *p, unsigned char control)
 {
-	DPRINTK(KERN_DEBUG "write_control %02x\n",control);
+	pr_debug("write_control %02x\n", control);
 	/* No implementation possible */
 }
 	
 static unsigned char amiga_read_control( struct parport *p)
 {
-	DPRINTK(KERN_DEBUG "read_control \n");
+	pr_debug("read_control\n");
 	return control_amiga_to_pc(0);
 }
 
@@ -73,7 +67,7 @@ static unsigned char amiga_frob_control( struct parport *p, unsigned char mask,
 {
 	unsigned char old;
 
-	DPRINTK(KERN_DEBUG "frob_control mask %02x, value %02x\n",mask,val);
+	pr_debug("frob_control mask %02x, value %02x\n", mask, val);
 	old = amiga_read_control(p);
 	amiga_write_control(p, (old & ~mask) ^ val);
 	return old;
@@ -99,7 +93,7 @@ static unsigned char amiga_read_status(struct parport *p)
 	unsigned char status;
 
 	status = status_amiga_to_pc(ciab.pra & 7);
-	DPRINTK(KERN_DEBUG "read_status %02x\n", status);
+	pr_debug("read_status %02x\n", status);
 	return status;
 }
 
@@ -115,14 +109,14 @@ static void amiga_disable_irq(struct parport *p)
 
 static void amiga_data_forward(struct parport *p)
 {
-	DPRINTK(KERN_DEBUG "forward\n");
+	pr_debug("forward\n");
 	ciaa.ddrb = 0xff; /* all pins output */
 	mb();
 }
 
 static void amiga_data_reverse(struct parport *p)
 {
-	DPRINTK(KERN_DEBUG "reverse\n");
+	pr_debug("reverse\n");
 	ciaa.ddrb = 0; /* all pins input */
 	mb();
 }
-- 
2.24.0

[PATCH 5/7] parport_mfc3: Convert DPRINTK to pr_debug

[Joe Perches]

Use a more common logging style.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/parport/parport_mfc3.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/parport/parport_mfc3.c b/drivers/parport/parport_mfc3.c
index 3190ef0..d6bbe8 100644
--- a/drivers/parport/parport_mfc3.c
+++ b/drivers/parport/parport_mfc3.c
@@ -70,11 +70,6 @@
 #define MAX_MFC 5
 
 #undef DEBUG
-#ifdef DEBUG
-#define DPRINTK printk
-#else
-static inline int DPRINTK(void *nothing, ...) {return 0;}
-#endif
 
 static struct parport *this_port[MAX_MFC] = {NULL, };
 static volatile int dummy; /* for trigger readds */
@@ -84,7 +79,7 @@ static struct parport_operations pp_mfc3_ops;
 
 static void mfc3_write_data(struct parport *p, unsigned char data)
 {
-DPRINTK(KERN_DEBUG "write_data %c\n",data);
+	pr_debug("write_data %c\n", data);
 
 	dummy = pia(p)->pprb; /* clears irq bit */
 	/* Triggers also /STROBE.*/
@@ -128,13 +123,13 @@ static unsigned char control_mfc3_to_pc(unsigned char control)
 
 static void mfc3_write_control(struct parport *p, unsigned char control)
 {
-DPRINTK(KERN_DEBUG "write_control %02x\n",control);
+	pr_debug("write_control %02x\n", control);
 	pia(p)->ppra = (pia(p)->ppra & 0x1f) | control_pc_to_mfc3(control);
 }
 	
 static unsigned char mfc3_read_control( struct parport *p)
 {
-DPRINTK(KERN_DEBUG "read_control \n");
+	pr_debug("read_control\n");
 	return control_mfc3_to_pc(pia(p)->ppra & 0xe0);
 }
 
@@ -142,7 +137,7 @@ static unsigned char mfc3_frob_control( struct parport *p, unsigned char mask, u
 {
 	unsigned char old;
 
-DPRINTK(KERN_DEBUG "frob_control mask %02x, value %02x\n",mask,val);
+	pr_debug("frob_control mask %02x, value %02x\n", mask, val);
 	old = mfc3_read_control(p);
 	mfc3_write_control(p, (old & ~mask) ^ val);
 	return old;
@@ -171,7 +166,7 @@ static unsigned char mfc3_read_status(struct parport *p)
 	unsigned char status;
 
 	status = status_mfc3_to_pc(pia(p)->ppra & 0x1f);
-DPRINTK(KERN_DEBUG "read_status %02x\n", status);
+	pr_debug("read_status %02x\n", status);
 	return status;
 }
 
@@ -202,7 +197,7 @@ static void mfc3_disable_irq(struct parport *p)
 
 static void mfc3_data_forward(struct parport *p)
 {
-	DPRINTK(KERN_DEBUG "forward\n");
+	pr_debug("forward\n");
 	pia(p)->crb &= ~PIA_DDR; /* make data direction register visible */
 	pia(p)->pddrb = 255; /* all pins output */
 	pia(p)->crb |= PIA_DDR; /* make data register visible - default */
@@ -210,7 +205,7 @@ static void mfc3_data_forward(struct parport *p)
 
 static void mfc3_data_reverse(struct parport *p)
 {
-	DPRINTK(KERN_DEBUG "reverse\n");
+	pr_debug("reverse\n");
 	pia(p)->crb &= ~PIA_DDR; /* make data direction register visible */
 	pia(p)->pddrb = 0; /* all pins input */
 	pia(p)->crb |= PIA_DDR; /* make data register visible - default */
-- 
2.24.0

[PATCH 6/7] parport_pc: Convert DPRINTK to pr_debug

[Joe Perches]

Use a more common logging style.

Miscellanea:

o One message converted from KERN_INFO to KERN_DEBUG

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/parport/parport_pc.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/parport/parport_pc.c b/drivers/parport/parport_pc.c
index ab3b04f..aae03b 100644
--- a/drivers/parport/parport_pc.c
+++ b/drivers/parport/parport_pc.c
@@ -87,13 +87,6 @@
 
 #undef DEBUG
 
-#ifdef DEBUG
-#define DPRINTK  printk
-#else
-#define DPRINTK(stuff...)
-#endif
-
-
 #define NR_SUPERIOS 3
 static struct superio_struct {	/* For Super-IO chips autodetection */
 	int io;
@@ -118,8 +111,8 @@ static void frob_econtrol(struct parport *pb, unsigned char m,
 	if (m != 0xff)
 		ectr = inb(ECONTROL(pb));
 
-	DPRINTK(KERN_DEBUG "frob_econtrol(%02x,%02x): %02x -> %02x\n",
-		m, v, ectr, (ectr & ~m) ^ v);
+	pr_debug("frob_econtrol(%02x,%02x): %02x -> %02x\n",
+		 m, v, ectr, (ectr & ~m) ^ v);
 
 	outb((ectr & ~m) ^ v, ECONTROL(pb));
 }
@@ -142,7 +135,7 @@ static int change_mode(struct parport *p, int m)
 	unsigned char oecr;
 	int mode;
 
-	DPRINTK(KERN_INFO "parport change_mode ECP-ISA to mode 0x%02x\n", m);
+	pr_debug("parport change_mode ECP-ISA to mode 0x%02x\n", m);
 
 	if (!priv->ecr) {
 		printk(KERN_DEBUG "change_mode: but there's no ECR!\n");
@@ -2295,7 +2288,7 @@ static int sio_ite_8872_probe(struct pci_dev *pdev, int autoirq, int autodma,
 	int irq;
 	int i;
 
-	DPRINTK(KERN_DEBUG "sio_ite_8872_probe()\n");
+	pr_debug("sio_ite_8872_probe()\n");
 
 	/* make sure which one chip */
 	for (i = 0; i < 5; i++) {
@@ -2360,11 +2353,9 @@ static int sio_ite_8872_probe(struct pci_dev *pdev, int autoirq, int autodma,
 	pci_write_config_dword(pdev, 0x9c,
 				ite8872set | (ite8872_irq * 0x11111));
 
-	DPRINTK(KERN_DEBUG "ITE887x: The IRQ is %d.\n", ite8872_irq);
-	DPRINTK(KERN_DEBUG "ITE887x: The PARALLEL I/O port is 0x%x.\n",
-		 ite8872_lpt);
-	DPRINTK(KERN_DEBUG "ITE887x: The PARALLEL I/O porthi is 0x%x.\n",
-		 ite8872_lpthi);
+	pr_debug("ITE887x: The IRQ is %d\n", ite8872_irq);
+	pr_debug("ITE887x: The PARALLEL I/O port is 0x%x\n", ite8872_lpt);
+	pr_debug("ITE887x: The PARALLEL I/O porthi is 0x%x\n", ite8872_lpthi);
 
 	/* Let the user (or defaults) steer us away from interrupts */
 	irq = ite8872_irq;
-- 
2.24.0

[PATCH 7/7] parport: Standardize use of printmode

[Joe Perches]

Standardize the define and the uses of printmode.

Miscellanea:

o Add missing statement termination ; where necessary

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/parport/parport_gsc.c |  8 ++++++--
 drivers/parport/parport_pc.c  | 14 ++++++--------
 drivers/parport/procfs.c      |  6 +++++-
 3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/parport/parport_gsc.c b/drivers/parport/parport_gsc.c
index cb6a08..9228e8 100644
--- a/drivers/parport/parport_gsc.c
+++ b/drivers/parport/parport_gsc.c
@@ -299,12 +299,16 @@ struct parport *parport_gsc_probe_port(unsigned long base,
 		p->dma = PARPORT_DMA_NONE;
 
 	pr_cont(" [");
-#define printmode(x) {if(p->modes&PARPORT_MODE_##x){pr_cont("%s%s",f?",":"",#x);f++;}}
+#define printmode(x)							\
+do {									\
+	if (p->modes & PARPORT_MODE_##x)				\
+		pr_cont("%s%s", f++ ? "," : "", #x);			\
+} while (0)
 	{
 		int f = 0;
 		printmode(PCSPP);
 		printmode(TRISTATE);
-		printmode(COMPAT)
+		printmode(COMPAT);
 		printmode(EPP);
 //		printmode(ECP);
 //		printmode(DMA);
diff --git a/drivers/parport/parport_pc.c b/drivers/parport/parport_pc.c
index aae03b..77e37e3 100644
--- a/drivers/parport/parport_pc.c
+++ b/drivers/parport/parport_pc.c
@@ -2134,19 +2134,17 @@ struct parport *parport_pc_probe_port(unsigned long int base,
 
 	pr_cont(" [");
 
-#define printmode(x) \
-	{\
-		if (p->modes & PARPORT_MODE_##x) {\
-			pr_cont("%s%s", f ? "," : "", #x);	\
-			f++;\
-		} \
-	}
+#define printmode(x)							\
+do {									\
+	if (p->modes & PARPORT_MODE_##x)				\
+		pr_cont("%s%s", f++ ? "," : "", #x);			\
+} while (0)
 
 	{
 		int f = 0;
 		printmode(PCSPP);
 		printmode(TRISTATE);
-		printmode(COMPAT)
+		printmode(COMPAT);
 		printmode(EPP);
 		printmode(ECP);
 		printmode(DMA);
diff --git a/drivers/parport/procfs.c b/drivers/parport/procfs.c
index 488040..e957be 100644
--- a/drivers/parport/procfs.c
+++ b/drivers/parport/procfs.c
@@ -213,7 +213,11 @@ static int do_hardware_modes(struct ctl_table *table, int write,
 		return -EACCES;
 
 	{
-#define printmode(x) {if(port->modes&PARPORT_MODE_##x){len+=sprintf(buffer+len,"%s%s",f?",":"",#x);f++;}}
+#define printmode(x)							\
+do {									\
+	if (port->modes & PARPORT_MODE_##x)				\
+		len += sprintf(buffer + len, "%s%s", f++ ? "," : "", #x); \
+} while (0)
 		int f = 0;
 		printmode(PCSPP);
 		printmode(TRISTATE);
-- 
2.24.0

Re: [PATCH 0/7] parport: Use generic kernel logging styles

[Randy Dunlap]

On 2/28/20 12:32 AM, Joe Perches wrote:
> Well, if the parport logging is getting some generic fixing,
> here's some more generic logging fixing...
> 
> Joe Perches (7):
>   parport: Convert printk(KERN_<LEVEL> to pr_<level>(
>   parport: Use more comon logging styles
>   parport: daisy: Convert DPRINTK to pr_debug
>   parport_amiga: Convert DPRINTK to pr_debug
>   parport_mfc3: Convert DPRINTK to pr_debug
>   parport_pc: Convert DPRINTK to pr_debug
>   parport: Standardize use of printmode

Reviewed-by: Randy Dunlap <rdunlap@infradead.org>

Thanks, Joe.

>  drivers/parport/daisy.c          |  29 +---
>  drivers/parport/ieee1284.c       |   4 +-
>  drivers/parport/ieee1284_ops.c   |   3 +-
>  drivers/parport/parport_amiga.c  |  22 +--
>  drivers/parport/parport_atari.c  |   2 +-
>  drivers/parport/parport_cs.c     |   6 +-
>  drivers/parport/parport_gsc.c    |  25 +--
>  drivers/parport/parport_gsc.h    |  21 ++-
>  drivers/parport/parport_ip32.c   | 117 +++++++-------
>  drivers/parport/parport_mfc3.c   |  21 +--
>  drivers/parport/parport_pc.c     | 263 +++++++++++++------------------
>  drivers/parport/parport_sunbpp.c |   2 +-
>  drivers/parport/probe.c          |  34 ++--
>  drivers/parport/procfs.c         |   6 +-
>  drivers/parport/share.c          |  37 +++--
>  15 files changed, 261 insertions(+), 331 deletions(-)
> 


-- 
~Randy

Re: [PATCH 0/7] parport: Use generic kernel logging styles

[Joe Perches]

On Sat, 2020-02-29 at 08:40 -0800, Randy Dunlap wrote:
> On 2/28/20 12:32 AM, Joe Perches wrote:
> > Well, if the parport logging is getting some generic fixing,
> > here's some more generic logging fixing...
> > 
> > Joe Perches (7):
> >   parport: Convert printk(KERN_<LEVEL> to pr_<level>(
> >   parport: Use more comon logging styles
> >   parport: daisy: Convert DPRINTK to pr_debug
> >   parport_amiga: Convert DPRINTK to pr_debug
> >   parport_mfc3: Convert DPRINTK to pr_debug
> >   parport_pc: Convert DPRINTK to pr_debug
> >   parport: Standardize use of printmode
> 
> Reviewed-by: Randy Dunlap <rdunlap@infradead.org>

btw: Sudip

After this recent daisy change:
---------------------------------------------------------------------

commit 60f8a59ddcdc7fb7c17180ba10d9c49bc91156c7
Author: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Date:   Wed Oct 16 15:45:40 2019 +0100

    parport: daisy: use new parport device model
    
    Modify parport daisy driver to use the new parallel port device model.
    
    Last attempt was '1aec4211204d ("parport: daisy: use new parport device
    model")' which failed as daisy was also trying to load the low level
    driver and that resulted in a deadlock.

---------------------------------------------------------------------

parport_register_device is no longer in use and
parport_register_dev_model is used instead.

Documentation/driver-api/parport-lowlevel.rst
should be updated and the various comments can
be updated or deleted.

$ git grep -w parport_register_device
Documentation/driver-api/parport-lowlevel.rst:  parport_register_device
Documentation/driver-api/parport-lowlevel.rst:          dev[count++] = parport_register_device (...);
Documentation/driver-api/parport-lowlevel.rst:parport_unregister_driver, parport_register_device, parport_enumerate
Documentation/driver-api/parport-lowlevel.rst:parport_register_device - register to use a port
Documentation/driver-api/parport-lowlevel.rst:  struct pardevice *parport_register_device(struct parport *port,
Documentation/driver-api/parport-lowlevel.rst:          private->dev = parport_register_device (port, "toaster", preempt,
Documentation/driver-api/parport-lowlevel.rst:This function is the opposite of parport_register_device.  After using
Documentation/driver-api/parport-lowlevel.rst:This is like parport_register_device but takes a device number instead
Documentation/driver-api/parport-lowlevel.rst:See parport_register_device.  If no device is associated with devnum,
Documentation/driver-api/parport-lowlevel.rst:parport_register_device
drivers/parport/daisy.c: *      This function is similar to parport_register_device(), except
drivers/parport/daisy.c: *      parport_register_device().  The return value is the same as
drivers/parport/daisy.c: *      for parport_register_device().
drivers/parport/daisy.c: *      parport_register_device().
drivers/parport/share.c: *      parport_register_device() on that port will do this for you.
drivers/parport/share.c: *      parport_register_device - register a device on a parallel port
drivers/parport/share.c:parport_register_device(struct parport *port, const char *name,
drivers/parport/share.c:EXPORT_SYMBOL(parport_register_device);
drivers/parport/share.c: *      This undoes the effect of parport_register_device().
include/linux/parport.h:/* parport_register_device declares that a device is connected to a
include/linux/parport.h:struct pardevice *parport_register_device(struct parport *port, 
scripts/kernel-doc:    # - parport_register_device (function pointer parameters)

Maybe something like this is necessary:
(it will need to be edited, I just deleted the references)
---
 Documentation/driver-api/parport-lowlevel.rst |  72 ++--------
 drivers/auxdisplay/panel.c                    |   2 +-
 drivers/parport/daisy.c                       |  10 +-
 drivers/parport/share.c                       | 197 +-------------------------
 include/linux/parport.h                       |  12 --
 scripts/kernel-doc                            |   1 -
 6 files changed, 17 insertions(+), 277 deletions(-)

diff --git a/Documentation/driver-api/parport-lowlevel.rst b/Documentation/driver-api/parport-lowlevel.rst
index 0633d70..42b1c87 100644
--- a/Documentation/driver-api/parport-lowlevel.rst
+++ b/Documentation/driver-api/parport-lowlevel.rst
@@ -10,7 +10,7 @@ Global functions::
   parport_register_driver
   parport_unregister_driver
   parport_enumerate
-  parport_register_device
+  parport_register_dev_model
   parport_unregister_device
   parport_claim
   parport_claim_or_block
@@ -205,7 +205,7 @@ EXAMPLE
 	{
 		...
 		private = kmalloc (...);
-		dev[count++] = parport_register_device (...);
+		dev[count++] = parport_register_dev_model (...);
 		...
 	}
 
@@ -235,7 +235,7 @@ EXAMPLE
 SEE ALSO
 ^^^^^^^^
 
-parport_unregister_driver, parport_register_device, parport_enumerate
+parport_unregister_driver, parport_enumerate
 
 
 
@@ -354,7 +354,7 @@ parport_register_driver, parport_unregister_driver
 
 
 
-parport_register_device - register to use a port
+parport_register_dev_model - register to use a port
 ------------------------------------------------
 
 SYNOPSIS
@@ -368,60 +368,21 @@ SYNOPSIS
 	typedef void (*wakeup_func) (void *handle);
 	typedef int (*irq_func) (int irq, void *handle, struct pt_regs *);
 
-	struct pardevice *parport_register_device(struct parport *port,
-						  const char *name,
-						  preempt_func preempt,
-						  wakeup_func wakeup,
-						  irq_func irq,
-						  int flags,
-						  void *handle);
+	struct pardevice *parport_register_dev_model(struct parport *port,
+						     const char *name,
+						     const struct pardev_cb *par_dev_cb,
+						     int cnt);
 
 DESCRIPTION
 ^^^^^^^^^^^
 
 Use this function to register your device driver on a parallel port
-(``port``).  Once you have done that, you will be able to use
-parport_claim and parport_release in order to use the port.
+(``port``).
 
 The (``name``) argument is the name of the device that appears in /proc
 filesystem. The string must be valid for the whole lifetime of the
 device (until parport_unregister_device is called).
 
-This function will register three callbacks into your driver:
-``preempt``, ``wakeup`` and ``irq``.  Each of these may be NULL in order to
-indicate that you do not want a callback.
-
-When the ``preempt`` function is called, it is because another driver
-wishes to use the parallel port.  The ``preempt`` function should return
-non-zero if the parallel port cannot be released yet -- if zero is
-returned, the port is lost to another driver and the port must be
-re-claimed before use.
-
-The ``wakeup`` function is called once another driver has released the
-port and no other driver has yet claimed it.  You can claim the
-parallel port from within the ``wakeup`` function (in which case the
-claim is guaranteed to succeed), or choose not to if you don't need it
-now.
-
-If an interrupt occurs on the parallel port your driver has claimed,
-the ``irq`` function will be called. (Write something about shared
-interrupts here.)
-
-The ``handle`` is a pointer to driver-specific data, and is passed to
-the callback functions.
-
-``flags`` may be a bitwise combination of the following flags:
-
-  ===================== =================================================
-        Flag            Meaning
-  ===================== =================================================
-  PARPORT_DEV_EXCL	The device cannot share the parallel port at all.
-			Use this only when absolutely necessary.
-  ===================== =================================================
-
-The typedefs are not actually defined -- they are only shown in order
-to make the function prototype more readable.
-
 The visible parts of the returned ``struct pardevice`` are::
 
 	struct pardevice {
@@ -468,9 +429,7 @@ EXAMPLE
 
 	static int toaster_detect (struct toaster *private, struct parport *port)
 	{
-		private->dev = parport_register_device (port, "toaster", preempt,
-							wakeup, NULL, 0,
-							private);
+		private->dev = parport_register_dev_model(port, "toaster", ...);
 		if (!private->dev)
 			/* Couldn't register with parport. */
 			return -EIO;
@@ -511,7 +470,7 @@ SYNPOPSIS
 DESCRIPTION
 ^^^^^^^^^^^
 
-This function is the opposite of parport_register_device.  After using
+This function is the opposite of parport_register_dev_model.  After using
 parport_unregister_device, ``dev`` is no longer a valid device handle.
 
 You should not unregister a device that is currently claimed, although
@@ -879,20 +838,15 @@ SYNOPSIS
 DESCRIPTION
 ^^^^^^^^^^^
 
-This is like parport_register_device but takes a device number instead
+This is like parport_register_dev_model but takes a device number instead
 of a pointer to a struct parport.
 
 RETURN VALUE
 ^^^^^^^^^^^^
 
-See parport_register_device.  If no device is associated with devnum,
+See parport_register_dev_model.  If no device is associated with devnum,
 NULL is returned.
 
-SEE ALSO
-^^^^^^^^
-
-parport_register_device
-
 
 
 parport_close - unregister device for particular device number
diff --git a/drivers/auxdisplay/panel.c b/drivers/auxdisplay/panel.c
index 85965953..cf4b12 100644
--- a/drivers/auxdisplay/panel.c
+++ b/drivers/auxdisplay/panel.c
@@ -1589,7 +1589,7 @@ static void panel_attach(struct parport *port)
 
 	pprt = parport_register_dev_model(port, "panel", &panel_cb, 0);
 	if (!pprt) {
-		pr_err("%s: port->number=%d parport=%d, parport_register_device() failed\n",
+		pr_err("%s: port->number=%d parport=%d, parport_register_dev_model() failed\n",
 		       __func__, port->number, parport);
 		return;
 	}
diff --git a/drivers/parport/daisy.c b/drivers/parport/daisy.c
index 6d78ec..7dc641 100644
--- a/drivers/parport/daisy.c
+++ b/drivers/parport/daisy.c
@@ -228,14 +228,6 @@ void parport_daisy_fini(struct parport *port)
  *	parport_open - find a device by canonical device number
  *	@devnum: canonical device number
  *	@name: name to associate with the device
- *
- *	This function is similar to parport_register_device(), except
- *	that it locates a device by its number rather than by the port
- *	it is attached to.
- *
- *	All parameters except for @devnum are the same as for
- *	parport_register_device().  The return value is the same as
- *	for parport_register_device().
  **/
 
 struct pardevice *parport_open(int devnum, const char *name)
@@ -289,7 +281,7 @@ struct pardevice *parport_open(int devnum, const char *name)
  *	@dev: device to close
  *
  *	This is to parport_open() as parport_unregister_device() is to
- *	parport_register_device().
+ *	parport_register_dev_model().
  **/
 
 void parport_close(struct pardevice *dev)
diff --git a/drivers/parport/share.c b/drivers/parport/share.c
index 3169fee..484723 100644
--- a/drivers/parport/share.c
+++ b/drivers/parport/share.c
@@ -262,8 +262,7 @@ static int port_detect(struct device *dev, void *dev_drv)
  *	The driver's attach() function may block.  The port that
  *	attach() is given will be valid for the duration of the
  *	callback, but if the driver wants to take a copy of the
- *	pointer it must call parport_get_port() to do so.  Calling
- *	parport_register_device() on that port will do this for you.
+ *	pointer it must call parport_get_port() to do so.
  *
  *	The driver's detach() function may block.  The port that
  *	detach() is given will be valid for the duration of the
@@ -641,198 +640,6 @@ void parport_remove_port(struct parport *port)
 }
 EXPORT_SYMBOL(parport_remove_port);
 
-/**
- *	parport_register_device - register a device on a parallel port
- *	@port: port to which the device is attached
- *	@name: a name to refer to the device
- *	@pf: preemption callback
- *	@kf: kick callback (wake-up)
- *	@irq_func: interrupt handler
- *	@flags: registration flags
- *	@handle: data for callback functions
- *
- *	This function, called by parallel port device drivers,
- *	declares that a device is connected to a port, and tells the
- *	system all it needs to know.
- *
- *	The @name is allocated by the caller and must not be
- *	deallocated until the caller calls @parport_unregister_device
- *	for that device.
- *
- *	The preemption callback function, @pf, is called when this
- *	device driver has claimed access to the port but another
- *	device driver wants to use it.  It is given @handle as its
- *	parameter, and should return zero if it is willing for the
- *	system to release the port to another driver on its behalf.
- *	If it wants to keep control of the port it should return
- *	non-zero, and no action will be taken.  It is good manners for
- *	the driver to try to release the port at the earliest
- *	opportunity after its preemption callback rejects a preemption
- *	attempt.  Note that if a preemption callback is happy for
- *	preemption to go ahead, there is no need to release the port;
- *	it is done automatically.  This function may not block, as it
- *	may be called from interrupt context.  If the device driver
- *	does not support preemption, @pf can be %NULL.
- *
- *	The wake-up ("kick") callback function, @kf, is called when
- *	the port is available to be claimed for exclusive access; that
- *	is, parport_claim() is guaranteed to succeed when called from
- *	inside the wake-up callback function.  If the driver wants to
- *	claim the port it should do so; otherwise, it need not take
- *	any action.  This function may not block, as it may be called
- *	from interrupt context.  If the device driver does not want to
- *	be explicitly invited to claim the port in this way, @kf can
- *	be %NULL.
- *
- *	The interrupt handler, @irq_func, is called when an interrupt
- *	arrives from the parallel port.  Note that if a device driver
- *	wants to use interrupts it should use parport_enable_irq(),
- *	and can also check the irq member of the parport structure
- *	representing the port.
- *
- *	The parallel port (lowlevel) driver is the one that has called
- *	request_irq() and whose interrupt handler is called first.
- *	This handler does whatever needs to be done to the hardware to
- *	acknowledge the interrupt (for PC-style ports there is nothing
- *	special to be done).  It then tells the IEEE 1284 code about
- *	the interrupt, which may involve reacting to an IEEE 1284
- *	event depending on the current IEEE 1284 phase.  After this,
- *	it calls @irq_func.  Needless to say, @irq_func will be called
- *	from interrupt context, and may not block.
- *
- *	The %PARPORT_DEV_EXCL flag is for preventing port sharing, and
- *	so should only be used when sharing the port with other device
- *	drivers is impossible and would lead to incorrect behaviour.
- *	Use it sparingly!  Normally, @flags will be zero.
- *
- *	This function returns a pointer to a structure that represents
- *	the device on the port, or %NULL if there is not enough memory
- *	to allocate space for that structure.
- **/
-
-struct pardevice *
-parport_register_device(struct parport *port, const char *name,
-			int (*pf)(void *), void (*kf)(void *),
-			void (*irq_func)(void *),
-			int flags, void *handle)
-{
-	struct pardevice *tmp;
-
-	if (port->physport->flags & PARPORT_FLAG_EXCL) {
-		/* An exclusive device is registered. */
-		printk(KERN_DEBUG "%s: no more devices allowed\n", port->name);
-		return NULL;
-	}
-
-	if (flags & PARPORT_DEV_LURK) {
-		if (!pf || !kf) {
-			pr_info("%s: refused to register lurking device (%s) without callbacks\n",
-				port->name, name);
-			return NULL;
-		}
-	}
-
-	if (flags & PARPORT_DEV_EXCL) {
-		if (port->physport->devices) {
-			/*
-			 * If a device is already registered and this new
-			 * device wants exclusive access, then no need to
-			 * continue as we can not grant exclusive access to
-			 * this device.
-			 */
-			pr_err("%s: cannot grant exclusive access for device %s\n",
-			       port->name, name);
-			return NULL;
-		}
-	}
-
-	/*
-	 * We up our own module reference count, and that of the port
-	 * on which a device is to be registered, to ensure that
-	 * neither of us gets unloaded while we sleep in (e.g.)
-	 * kmalloc.
-	 */
-	if (!try_module_get(port->ops->owner))
-		return NULL;
-
-	parport_get_port(port);
-
-	tmp = kmalloc(sizeof(struct pardevice), GFP_KERNEL);
-	if (!tmp)
-		goto out;
-
-	tmp->state = kmalloc(sizeof(struct parport_state), GFP_KERNEL);
-	if (!tmp->state)
-		goto out_free_pardevice;
-
-	tmp->name = name;
-	tmp->port = port;
-	tmp->daisy = -1;
-	tmp->preempt = pf;
-	tmp->wakeup = kf;
-	tmp->private = handle;
-	tmp->flags = flags;
-	tmp->irq_func = irq_func;
-	tmp->waiting = 0;
-	tmp->timeout = 5 * HZ;
-	tmp->devmodel = false;
-
-	/* Chain this onto the list */
-	tmp->prev = NULL;
-	/*
-	 * This function must not run from an irq handler so we don' t need
-	 * to clear irq on the local CPU. -arca
-	 */
-	spin_lock(&port->physport->pardevice_lock);
-
-	if (flags & PARPORT_DEV_EXCL) {
-		if (port->physport->devices) {
-			spin_unlock(&port->physport->pardevice_lock);
-			printk(KERN_DEBUG "%s: cannot grant exclusive access for device %s\n",
-			       port->name, name);
-			goto out_free_all;
-		}
-		port->flags |= PARPORT_FLAG_EXCL;
-	}
-
-	tmp->next = port->physport->devices;
-	wmb(); /*
-		* Make sure that tmp->next is written before it's
-		* added to the list; see comments marked 'no locking
-		* required'
-		*/
-	if (port->physport->devices)
-		port->physport->devices->prev = tmp;
-	port->physport->devices = tmp;
-	spin_unlock(&port->physport->pardevice_lock);
-
-	init_waitqueue_head(&tmp->wait_q);
-	tmp->timeslice = parport_default_timeslice;
-	tmp->waitnext = tmp->waitprev = NULL;
-
-	/*
-	 * This has to be run as last thing since init_state may need other
-	 * pardevice fields. -arca
-	 */
-	port->ops->init_state(tmp, tmp->state);
-	if (!test_and_set_bit(PARPORT_DEVPROC_REGISTERED, &port->devflags)) {
-		port->proc_device = tmp;
-		parport_device_proc_register(tmp);
-	}
-	return tmp;
-
- out_free_all:
-	kfree(tmp->state);
- out_free_pardevice:
-	kfree(tmp);
- out:
-	parport_put_port(port);
-	module_put(port->ops->owner);
-
-	return NULL;
-}
-EXPORT_SYMBOL(parport_register_device);
-
 static void free_pardevice(struct device *dev)
 {
 	struct pardevice *par_dev = to_pardevice(dev);
@@ -986,7 +793,7 @@ EXPORT_SYMBOL(parport_register_dev_model);
  *	parport_unregister_device - deregister a device on a parallel port
  *	@dev: pointer to structure representing device
  *
- *	This undoes the effect of parport_register_device().
+ *	This undoes the effect of parport_register_dev_model().
  **/
 
 void parport_unregister_device(struct pardevice *dev)
diff --git a/include/linux/parport.h b/include/linux/parport.h
index 13932ce..b7a5d06 100644
--- a/include/linux/parport.h
+++ b/include/linux/parport.h
@@ -325,18 +325,6 @@ struct pardev_cb {
 	unsigned int flags;
 };
 
-/* parport_register_device declares that a device is connected to a
-   port, and tells the kernel all it needs to know.
-   - pf is the preemption function (may be NULL for no callback)
-   - kf is the wake-up function (may be NULL for no callback)
-   - irq_func is the interrupt handler (may be NULL for no interrupts)
-   - handle is a user pointer that gets handed to callback functions.  */
-struct pardevice *parport_register_device(struct parport *port, 
-			  const char *name,
-			  int (*pf)(void *), void (*kf)(void *),
-			  void (*irq_func)(void *), 
-			  int flags, void *handle);
-
 struct pardevice *
 parport_register_dev_model(struct parport *port, const char *name,
 			   const struct pardev_cb *par_dev_cb, int cnt);
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index f2d73f..acd0e3 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1607,7 +1607,6 @@ sub dump_function($$) {
     #
     # If you mess with these regexps, it's a good idea to check that
     # the following functions' documentation still comes out right:
-    # - parport_register_device (function pointer parameters)
     # - atomic_set (macro)
     # - pci_match_device, __copy_to_user (long return type)
 

Re: [PATCH 0/7] parport: Use generic kernel logging styles

[Sudip Mukherjee]

On Sat, Feb 29, 2020 at 7:35 PM Joe Perches <joe@perches.com> wrote:
>
> On Sat, 2020-02-29 at 08:40 -0800, Randy Dunlap wrote:
> > On 2/28/20 12:32 AM, Joe Perches wrote:
> > > Well, if the parport logging is getting some generic fixing,
> > > here's some more generic logging fixing...
> > >
> > > Joe Perches (7):
> > >   parport: Convert printk(KERN_<LEVEL> to pr_<level>(
> > >   parport: Use more comon logging styles
> > >   parport: daisy: Convert DPRINTK to pr_debug
> > >   parport_amiga: Convert DPRINTK to pr_debug
> > >   parport_mfc3: Convert DPRINTK to pr_debug
> > >   parport_pc: Convert DPRINTK to pr_debug
> > >   parport: Standardize use of printmode
> >
> > Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
>
> btw: Sudip
>
> After this recent daisy change:
> ---------------------------------------------------------------------
>
> commit 60f8a59ddcdc7fb7c17180ba10d9c49bc91156c7
> Author: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> Date:   Wed Oct 16 15:45:40 2019 +0100
>
>     parport: daisy: use new parport device model
>
>     Modify parport daisy driver to use the new parallel port device model.
>
>     Last attempt was '1aec4211204d ("parport: daisy: use new parport device
>     model")' which failed as daisy was also trying to load the low level
>     driver and that resulted in a deadlock.
>
> ---------------------------------------------------------------------
>
> parport_register_device is no longer in use and
> parport_register_dev_model is used instead.

Yes, I will do the cleanup, if you have not done that already. ;-)
This last change was done after a failed attempt where the previous
change had to be reverted by Linus for a regression.
So, just thought it to be safe to wait for a cycle for any regression
report before I remove the last bits.


-- 
Regards
Sudip