Re: [PATCH 16/22] ncr5380: Fix register decoding for debugging
From: Hannes Reinecke
Date: Mon Mar 14 2016 - 03:46:17 EST
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@xxxxxxxxxxxxxxxxxxx>
>
> ---
> 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@xxxxxxxx>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@xxxxxxx +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)