Re: [PATCH 2/3] debloat aic7xxx and aic79xx drivers

[Hannes Reinecke <hare@suse.de>]

Hi Denys,

Denys Vlasenko wrote:
> On Monday 07 April 2008 12:34, Hannes Reinecke wrote:
>>> Adds statics, #ifdefs out huge amount of unused code, adds consts
>>>
>>> Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
>>> --
>>> vda
>>>
>> NACK. We need the #defines to print out the registers.
>> And in either case the *_shipped files are in fact
>> autogenerated by aic assembler, so we need to fix that
>> one, too.
> 
> I assume you are talking about this part of a patch:
> 
> --- linux-2.6.25-rc6-aic1/drivers/scsi/aic7xxx/aic79xx_reg.h_shipped    2008-03-23 00:43:20.000000000 +0100
> +++ linux-2.6.25-rc6-aic2/drivers/scsi/aic7xxx/aic79xx_reg.h_shipped    2008-03-23 00:54:59.000000000 +0100
> @@ -11,23 +11,18 @@ typedef struct ahd_reg_parse_entry {
>         uint8_t  value;
>         uint8_t  mask;
>  } ahd_reg_parse_entry_t;
> 
> +#if 0 /* unused */
> +
>  #if AIC_DEBUG_REGISTERS
>  ahd_reg_print_t ahd_mode_ptr_print;
>  #else
>  #define ahd_mode_ptr_print(regvalue, cur_col, wrap) \
>      ahd_print_register(NULL, 0, "MODE_PTR", 0x00, regvalue, cur_col, wrap)
>  #endif
> 
> .....
> .....
> 
Correct.

> 
> Let me explain what I am doing here. I am NOT deleting ahd_intstat_print
> definition, I am moving it below the #endif which terminates big
> #if 0 /* unused */  block, moving to this place:
> 
> 
> @@ -2377,8 +2043,346 @@ ahd_reg_print_t ahd_scb_disconnected_lis
>  #define ahd_scb_disconnected_lists_print(regvalue, cur_col, wrap) \
>      ahd_print_register(NULL, 0, "SCB_DISCONNECTED_LISTS", 0x1b8, regvalue, cur_col, wrap)
>  #endif
> 
> +#endif /* unused */
> +
> +#if AIC_DEBUG_REGISTERS
> +ahd_reg_print_t ahd_intstat_print;
> +#else
> +#define ahd_intstat_print(regvalue, cur_col, wrap) \
> +    ahd_print_register(NULL, 0, "INTSTAT", 0x01, regvalue, cur_col, wrap)
> +#endif
> ...
> ...
> 
Hmm.

> 
> #if 0 / #endif block ends up containing definitions of 290 functions/macros,
> none of which is EVER used. I used this script (below) and verified that
> they are never mentioned anywhere outside of *_shipped files.
> I also did test builds with and without debug enabled and they build
> with no problems. No undefined references!
> 
Well, still not quite. The point here is that all of the functions in the
*_shipped files are in fact auto-generated by aicasm, based on the definitions
in aic79xx.seq and aic79xx.reg. So the *_reg_print.c files contains
functions for all _defined_ registers, not the actually used ones.
What we have to do here is to modify aicasm to not print out the
unused definitions, and copy those (autogenerated) files over to
the *_shipped files to have them synced properly.
Hand-patching the *_shipped files is not a good idea.

The const idea is a good one, and actually a one-liner to fix:
diff --git a/drivers/scsi/aic7xxx/aicasm/aicasm_symbol.c b/drivers/scsi/aic7xxx/aicasm/aicasm_symbol.c
index f1f448d..a7a51e4 100644
--- a/drivers/scsi/aic7xxx/aicasm/aicasm_symbol.c
+++ b/drivers/scsi/aic7xxx/aicasm/aicasm_symbol.c
@@ -370,7 +370,7 @@ aic_print_reg_dump_start(FILE *dfile, symbol_node_t *regnode)
                return;
 
        fprintf(dfile,
-"static %sreg_parse_entry_t %s_parse_table[] = {\n",
+"static const %sreg_parse_entry_t %s_parse_table[] = {\n",
                prefix,
                regnode->symbol->name);
 }


Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, 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