Re: [PATCH 10/22] scsi: consolidate scsi_print_status()

[Hannes Reinecke <hare@suse.de>]

On 09/01/2014 12:14 AM, Christoph Hellwig wrote:
>>  #if defined(AHA152X_DEBUG)
>> -	if (HOSTDATA(shpnt)->debug & debug_status) {
>> -		printk(DEBUG_LEAD "inbound status %02x ", CMDINFO(CURRENT_SC), CURRENT_SC->SCp.Status);
>> -		scsi_print_status(CURRENT_SC->SCp.Status);
>> -		printk("\n");
>> -	}
>> +	if (HOSTDATA(shpnt)->debug & debug_status)
>> +		printk(DEBUG_LEAD "inbound status %02x\n", CMDINFO(CURRENT_SC), CURRENT_SC->SCp.Status);
> 
> This one just removes the call.  While this is fine with me it needs to
> be mentioned in the changelog.
> 
> Also if you change the line above it please make sure it fits into an
> 80 character line.
> 
>> +const char *
>>  scsi_print_status(unsigned char scsi_status) {
>> -#ifdef CONFIG_SCSI_CONSTANTS
> 
> Please explain in the changelog why you're removing this ifdef.
> 
>> +	case SAM_STAT_BUSY:
>> +		ccp = "Busy"; break;
> 
> Please put the break statements on separate lines per normal Linux
> style.
> 
>>  #define scsi_prot_op_name(result)	{ result, #result }
>>  #define show_prot_op_name(val)					\
>>  	__print_symbolic(val,					\
>> @@ -331,7 +316,7 @@ DECLARE_EVENT_CLASS(scsi_cmd_done_timeout_template,
>>  		  show_driverbyte_name(((__entry->result) >> 24) & 0xff),
>>  		  show_hostbyte_name(((__entry->result) >> 16) & 0xff),
>>  		  show_msgbyte_name(((__entry->result) >> 8) & 0xff),
>> -		  show_statusbyte_name(__entry->result & 0xff))
>> +		  scsi_print_status(__entry->result & 0xff))
> 
> 
> Not using __print_symbolic for trace events breaks all tracing tools
> that parse the binary trace buffers, please drop the tracing changes
> (which weren't mentioned in the changelog anyway and should have been a
> separate patch).
> 
I've now taken another approach; I've updated the logging functions
aha152x.c and removed all debugging code. And with that we can
remove scsi_print_status() altogether.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html