linux-kernel - Re: [PATCH 16/22] ncr5380: Fix register decoding for debugging

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <56E66C43.4030208@suse.de>
Date:	Mon, 14 Mar 2016 08:46:11 +0100
From:	Hannes Reinecke <hare@...e.de>
To:	Finn Thain <fthain@...egraphics.com.au>,
	"James E.J. Bottomley" <James.Bottomley@...senPartnership.com>,
	"Martin K. Petersen" <martin.petersen@...cle.com>,
	Michael Schmitz <schmitzmic@...il.com>,
	linux-m68k@...r.kernel.org, linux-scsi@...r.kernel.org,
	linux-kernel@...r.kernel.org
Cc:	Ondrej Zary <linux@...nbow-software.org>,
	Sam Creasey <sammy@...my.net>
Subject: Re: [PATCH 16/22] ncr5380: Fix register decoding for debugging

On 03/14/2016 05:27 AM, Finn Thain wrote:
> Decode all bits in the chip registers. They are all useful at times.
> Fix printk severity so that this output can be suppressed along with
> the other debugging output.
> 
> Signed-off-by: Finn Thain <fthain@...egraphics.com.au>
> 
> ---
>  drivers/scsi/NCR5380.c |   42 +++++++++++++++++++++++++-----------------
>  1 file changed, 25 insertions(+), 17 deletions(-)
> 
> Index: linux/drivers/scsi/NCR5380.c
> ===================================================================
> --- linux.orig/drivers/scsi/NCR5380.c	2016-03-14 15:26:48.000000000 +1100
> +++ linux/drivers/scsi/NCR5380.c	2016-03-14 15:26:50.000000000 +1100
> @@ -256,12 +256,20 @@ static struct {
>  	{0, NULL}
>  },
>  basrs[] = {
> +	{BASR_END_DMA_TRANSFER, "END OF DMA"},
> +	{BASR_DRQ, "DRQ"},
> +	{BASR_PARITY_ERROR, "PARITY ERROR"},
> +	{BASR_IRQ, "IRQ"},
> +	{BASR_PHASE_MATCH, "PHASE MATCH"},
> +	{BASR_BUSY_ERROR, "BUSY ERROR"},
>  	{BASR_ATN, "ATN"},
>  	{BASR_ACK, "ACK"},
>  	{0, NULL}
>  },
>  icrs[] = {
>  	{ICR_ASSERT_RST, "ASSERT RST"},
> +	{ICR_ARBITRATION_PROGRESS, "ARB. IN PROGRESS"},
> +	{ICR_ARBITRATION_LOST, "LOST ARB."},
>  	{ICR_ASSERT_ACK, "ASSERT ACK"},
>  	{ICR_ASSERT_BSY, "ASSERT BSY"},
>  	{ICR_ASSERT_SEL, "ASSERT SEL"},
> @@ -270,14 +278,14 @@ icrs[] = {
>  	{0, NULL}
>  },
>  mrs[] = {
> -	{MR_BLOCK_DMA_MODE, "MODE BLOCK DMA"},
> -	{MR_TARGET, "MODE TARGET"},
> -	{MR_ENABLE_PAR_CHECK, "MODE PARITY CHECK"},
> -	{MR_ENABLE_PAR_INTR, "MODE PARITY INTR"},
> -	{MR_ENABLE_EOP_INTR, "MODE EOP INTR"},
> -	{MR_MONITOR_BSY, "MODE MONITOR BSY"},
> -	{MR_DMA_MODE, "MODE DMA"},
> -	{MR_ARBITRATE, "MODE ARBITRATION"},
> +	{MR_BLOCK_DMA_MODE, "BLOCK DMA MODE"},
> +	{MR_TARGET, "TARGET"},
> +	{MR_ENABLE_PAR_CHECK, "PARITY CHECK"},
> +	{MR_ENABLE_PAR_INTR, "PARITY INTR"},
> +	{MR_ENABLE_EOP_INTR, "EOP INTR"},
> +	{MR_MONITOR_BSY, "MONITOR BSY"},
> +	{MR_DMA_MODE, "DMA MODE"},
> +	{MR_ARBITRATE, "ARBITRATE"},
>  	{0, NULL}
>  };
>  
> @@ -298,23 +306,23 @@ static void NCR5380_print(struct Scsi_Ho
>  	icr = NCR5380_read(INITIATOR_COMMAND_REG);
>  	basr = NCR5380_read(BUS_AND_STATUS_REG);
>  
> -	printk("STATUS_REG: %02x ", status);
> +	printk(KERN_DEBUG "SR =   0x%02x : ", status);
>  	for (i = 0; signals[i].mask; ++i)
>  		if (status & signals[i].mask)
> -			printk(",%s", signals[i].name);
> -	printk("\nBASR: %02x ", basr);
> +			printk(KERN_CONT "%s, ", signals[i].name);
> +	printk(KERN_CONT "\nBASR = 0x%02x : ", basr);
>  	for (i = 0; basrs[i].mask; ++i)
>  		if (basr & basrs[i].mask)
> -			printk(",%s", basrs[i].name);
> -	printk("\nICR: %02x ", icr);
> +			printk(KERN_CONT "%s, ", basrs[i].name);
> +	printk(KERN_CONT "\nICR =  0x%02x : ", icr);
>  	for (i = 0; icrs[i].mask; ++i)
>  		if (icr & icrs[i].mask)
> -			printk(",%s", icrs[i].name);
> -	printk("\nMODE: %02x ", mr);
> +			printk(KERN_CONT "%s, ", icrs[i].name);
> +	printk(KERN_CONT "\nMR =   0x%02x : ", mr);
>  	for (i = 0; mrs[i].mask; ++i)
>  		if (mr & mrs[i].mask)
> -			printk(",%s", mrs[i].name);
> -	printk("\n");
> +			printk(KERN_CONT "%s, ", mrs[i].name);
> +	printk(KERN_CONT "\n");
>  }
>  
>  static struct {
> 
> 
Well ... using individual printk() like here might end up in each
call to be broken up into individual lines.
You might want to consider using a line buffer here or, better
still, move to seq_file or debugfs output.

But as the original code did that, too:

Reviewed-by: Hannes Reinecke <hare@...e.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@...e.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)