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

This is a discussion on Re: [PATCH 2/3] debloat aic7xxx and aic79xx drivers - Kernel ; Hi Denys, Denys Vlasenko wrote: > Adds statics, #ifdefs out huge amount of unused code, adds consts > > Signed-off-by: Denys Vlasenko > -- > vda > NACK. We need the #defines to print out the registers. And in either ...

+ Reply to Thread
Results 1 to 2 of 2

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

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

    Hi Denys,

    Denys Vlasenko wrote:
    > Adds statics, #ifdefs out huge amount of unused code, adds consts
    >
    > Signed-off-by: Denys Vlasenko
    > --
    > 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.

    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-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

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

    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
    > >

    > 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

    #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
    -
    -#if AIC_DEBUG_REGISTERS
    ahd_reg_print_t ahd_seqintcode_print;
    #else
    #define ahd_seqintcode_print(regvalue, cur_col, wrap) \
    ahd_print_register(NULL, 0, "SEQINTCODE", 0x02, regvalue, cur_col, wrap)
    @@ -75,22 +70,8 @@ ahd_reg_print_t ahd_hescb_qoff_print;
    ahd_print_register(NULL, 0, "HESCB_QOFF", 0x08, regvalue, cur_col, wrap)
    #endif

    #if AIC_DEBUG_REGISTERS
    -ahd_reg_print_t ahd_hs_mailbox_print;
    -#else
    -#define ahd_hs_mailbox_print(regvalue, cur_col, wrap) \
    - ahd_print_register(NULL, 0, "HS_MAILBOX", 0x0b, regvalue, cur_col, wrap)
    -#endif
    -
    -#if AIC_DEBUG_REGISTERS
    -ahd_reg_print_t ahd_seqintstat_print;
    -#else
    -#define ahd_seqintstat_print(regvalue, cur_col, wrap) \
    - ahd_print_register(NULL, 0, "SEQINTSTAT", 0x0c, regvalue, cur_col, wrap)
    -#endif
    -
    -#if AIC_DEBUG_REGISTERS
    ......
    ......


    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
    ....
    ....


    #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!

    cd $KERNEL_TREE/drivers/scsi/aic7xxx/ || exit 1

    echo -n "\
    ahd_mode_ptr_print
    ahd_seqintcode_print
    ahd_clrint_print
    ahd_error_print
    ahd_clrerr_print
    ahd_hcntrl_print
    ahd_hnscb_qoff_print
    ahd_hescb_qoff_print
    ahd_clrseqintstat_print
    ahd_swtimer_print
    ahd_snscb_qoff_print
    ahd_sescb_qoff_print
    ahd_sdscb_qoff_print
    ahd_qoff_ctlsta_print
    ahd_dscommand0_print
    ahd_arbctl_print
    ahd_sg_cache_pre_print
    ahd_lqin_print
    ahd_typeptr_print
    ahd_tagptr_print
    ahd_lunptr_print
    ahd_datalenptr_print
    ahd_statlenptr_print
    ahd_cmdlenptr_print
    ahd_attrptr_print
    ahd_flagptr_print
    ahd_cmdptr_print
    ahd_qnextptr_print
    ahd_idptr_print
    ahd_abrtbyteptr_print
    ahd_abrtbitptr_print
    ahd_maxcmdbytes_print
    ahd_maxcmd2rcv_print
    ahd_shortthresh_print
    ahd_lunlen_print
    ahd_cdblimit_print
    ahd_maxcmd_print
    ahd_maxcmdcnt_print
    ahd_lqrsvd01_print
    ahd_lqrsvd16_print
    ahd_lqrsvd17_print
    ahd_cmdrsvd0_print
    ahd_lqctl0_print
    ahd_lqctl1_print
    ahd_scsbist0_print
    ahd_lqctl2_print
    ahd_scsbist1_print
    ahd_sxfrctl0_print
    ahd_dlcount_print
    ahd_businitid_print
    ahd_sxfrctl1_print
    ahd_bustargid_print
    ahd_sxfrctl2_print
    ahd_scsisigo_print
    ahd_multargid_print
    ahd_scsidat0_img_print
    ahd_scsidat_print
    ahd_targidin_print
    ahd_optionmode_print
    ahd_sblkctl_print
    ahd_clrsint0_print
    ahd_clrsint1_print
    ahd_simode2_print
    ahd_clrsint2_print
    ahd_lqistate_print
    ahd_lqostate_print
    ahd_clrlqiint0_print
    ahd_lqimode0_print
    ahd_lqimode1_print
    ahd_clrlqiint1_print
    ahd_simode3_print
    ahd_clrsint3_print
    ahd_clrlqoint0_print
    ahd_lqomode0_print
    ahd_lqomode1_print
    ahd_clrlqoint1_print
    ahd_os_space_cnt_print
    ahd_gsfifo_print
    ahd_lqoscsctl_print
    ahd_nextscb_print
    ahd_clrseqintsrc_print
    ahd_currscb_print
    ahd_crccontrol_print
    ahd_dfftag_print
    ahd_lastscb_print
    ahd_scsitest_print
    ahd_iopdnctl_print
    ahd_shaddr_print
    ahd_negoaddr_print
    ahd_dgrpcrci_print
    ahd_negperiod_print
    ahd_packcrci_print
    ahd_negoffset_print
    ahd_negppropts_print
    ahd_negconopts_print
    ahd_annexcol_print
    ahd_annexdat_print
    ahd_scschkn_print
    ahd_iownid_print
    ahd_pll960ctl0_print
    ahd_shcnt_print
    ahd_townid_print
    ahd_pll960ctl1_print
    ahd_pll960cnt0_print
    ahd_xsig_print
    ahd_pll400ctl0_print
    ahd_fairness_print
    ahd_pll400ctl1_print
    ahd_unfairness_print
    ahd_pll400cnt0_print
    ahd_haddr_print
    ahd_plldelay_print
    ahd_hodmaadr_print
    ahd_hodmacnt_print
    ahd_hcnt_print
    ahd_hodmaen_print
    ahd_scbhaddr_print
    ahd_sghaddr_print
    ahd_scbhcnt_print
    ahd_sghcnt_print
    ahd_dff_thrsh_print
    ahd_romaddr_print
    ahd_romcntrl_print
    ahd_romdata_print
    ahd_cmcrxmsg0_print
    ahd_roenable_print
    ahd_ovlyrxmsg0_print
    ahd_dchrxmsg0_print
    ahd_ovlyrxmsg1_print
    ahd_nsenable_print
    ahd_cmcrxmsg1_print
    ahd_dchrxmsg1_print
    ahd_dchrxmsg2_print
    ahd_cmcrxmsg2_print
    ahd_ost_print
    ahd_ovlyrxmsg2_print
    ahd_dchrxmsg3_print
    ahd_ovlyrxmsg3_print
    ahd_cmcrxmsg3_print
    ahd_pcixctl_print
    ahd_ovlyseqbcnt_print
    ahd_dchseqbcnt_print
    ahd_cmcseqbcnt_print
    ahd_cmcspltstat0_print
    ahd_dchspltstat0_print
    ahd_ovlyspltstat0_print
    ahd_cmcspltstat1_print
    ahd_ovlyspltstat1_print
    ahd_dchspltstat1_print
    ahd_sgrxmsg0_print
    ahd_slvspltoutadr0_print
    ahd_sgrxmsg1_print
    ahd_slvspltoutadr1_print
    ahd_sgrxmsg2_print
    ahd_slvspltoutadr2_print
    ahd_sgrxmsg3_print
    ahd_slvspltoutadr3_print
    ahd_sgseqbcnt_print
    ahd_slvspltoutattr0_print
    ahd_slvspltoutattr1_print
    ahd_slvspltoutattr2_print
    ahd_sgspltstat0_print
    ahd_sgspltstat1_print
    ahd_sfunct_print
    ahd_df0pcistat_print
    ahd_reg0_print
    ahd_df1pcistat_print
    ahd_sgpcistat_print
    ahd_reg1_print
    ahd_cmcpcistat_print
    ahd_ovlypcistat_print
    ahd_reg_isr_print
    ahd_msipcistat_print
    ahd_targpcistat_print
    ahd_data_count_odd_print
    ahd_scbptr_print
    ahd_ccscbacnt_print
    ahd_scbautoptr_print
    ahd_ccsgaddr_print
    ahd_ccscbadr_bk_print
    ahd_ccscbaddr_print
    ahd_cmc_rambist_print
    ahd_ccsgram_print
    ahd_flexadr_print
    ahd_ccscbram_print
    ahd_flexcnt_print
    ahd_flexdmastat_print
    ahd_flexdata_print
    ahd_brddat_print
    ahd_brdctl_print
    ahd_seeadr_print
    ahd_seedat_print
    ahd_seectl_print
    ahd_seestat_print
    ahd_scbcnt_print
    ahd_dfwaddr_print
    ahd_dspfltrctl_print
    ahd_dspdatactl_print
    ahd_dfraddr_print
    ahd_dspreqctl_print
    ahd_dspackctl_print
    ahd_dfdat_print
    ahd_dspselect_print
    ahd_wrtbiasctl_print
    ahd_rcvrbiosctl_print
    ahd_wrtbiascalc_print
    ahd_rcvrbiascalc_print
    ahd_dfptrs_print
    ahd_skewcalc_print
    ahd_dfbkptr_print
    ahd_dfdbctl_print
    ahd_dfscnt_print
    ahd_dfbcnt_print
    ahd_ovlyaddr_print
    ahd_seqctl1_print
    ahd_flags_print
    ahd_seqram_print
    ahd_prgmcnt_print
    ahd_accum_print
    ahd_sindex_print
    ahd_dindex_print
    ahd_brkaddr0_print
    ahd_brkaddr1_print
    ahd_allones_print
    ahd_allzeros_print
    ahd_none_print
    ahd_sindir_print
    ahd_dindir_print
    ahd_function1_print
    ahd_stack_print
    ahd_intvec1_addr_print
    ahd_curaddr_print
    ahd_lastaddr_print
    ahd_intvec2_addr_print
    ahd_longjmp_addr_print
    ahd_accum_save_print
    ahd_waiting_scb_tails_print
    ahd_ahd_pci_config_base_print
    ahd_sram_base_print
    ahd_waiting_tid_head_print
    ahd_waiting_tid_tail_print
    ahd_next_queued_scb_addr_print
    ahd_complete_scb_head_print
    ahd_complete_scb_dmainprog_head_print
    ahd_complete_dma_scb_head_print
    ahd_complete_dma_scb_tail_print
    ahd_complete_on_qfreeze_head_print
    ahd_msg_out_print
    ahd_dmaparams_print
    ahd_saved_scsiid_print
    ahd_saved_lun_print
    ahd_qoutfifo_entry_valid_tag_print
    ahd_kernel_tqinpos_print
    ahd_tqinpos_print
    ahd_shared_data_addr_print
    ahd_qoutfifo_next_addr_print
    ahd_arg_1_print
    ahd_arg_2_print
    ahd_last_msg_print
    ahd_scsiseq_template_print
    ahd_initiator_tag_print
    ahd_allocfifo_scbptr_print
    ahd_int_coalescing_timer_print
    ahd_int_coalescing_maxcmds_print
    ahd_int_coalescing_mincmds_print
    ahd_cmds_pending_print
    ahd_int_coalescing_cmdcount_print
    ahd_local_hs_mailbox_print
    ahd_cmdsize_table_print
    ahd_scb_base_print
    ahd_scb_residual_datacnt_print
    ahd_scb_residual_sgptr_print
    ahd_scb_scsi_status_print
    ahd_scb_target_phases_print
    ahd_scb_target_data_dir_print
    ahd_scb_target_itag_print
    ahd_scb_sense_busaddr_print
    ahd_scb_tag_print
    ahd_scb_lun_print
    ahd_scb_task_attribute_print
    ahd_scb_cdb_len_print
    ahd_scb_task_management_print
    ahd_scb_dataptr_print
    ahd_scb_datacnt_print
    ahd_scb_sgptr_print
    ahd_scb_busaddr_print
    ahd_scb_next_print
    ahd_scb_next2_print
    ahd_scb_spare_print
    ahd_scb_disconnected_lists_print
    " | while read name; do
    grep "$name" -rc .
    done | grep -vF ':0'

    I got the following output:
    ../aic79xx_reg.h_shipped:2
    ../aic79xx_reg_print.c_shipped:1
    ../aic79xx_reg.h_shipped:2
    ../aic79xx_reg_print.c_shipped:1
    ....
    ....

    which means that these functions are defined but never referenced.

    (Yes, patch similarly ifdefs out function definitions in aic79xx_reg_print.c_shipped)


    > And in either case the *_shipped files are in fact
    > autogenerated by aic assembler, so we need to fix that
    > one, too.


    I tried this approach. But running aic assembler doesn't seem
    to regenerate _shipped files.

    I built kernel with the following options and none of the *_shipped
    files change (in fact, no source file changes):

    # CONFIG_PREVENT_FIRMWARE_BUILD is not set
    CONFIG_SCSI_AIC7XXX=y
    CONFIG_AIC7XXX_CMDS_PER_DEVICE=8
    CONFIG_AIC7XXX_RESET_DELAY_MS=15000
    CONFIG_AIC7XXX_BUILD_FIRMWARE=y <============
    CONFIG_AIC7XXX_DEBUG_ENABLE=y
    CONFIG_AIC7XXX_DEBUG_MASK=0
    CONFIG_AIC7XXX_REG_PRETTY_PRINT=y
    # CONFIG_SCSI_AIC7XXX_OLD is not set
    CONFIG_SCSI_AIC79XX=y
    CONFIG_AIC79XX_CMDS_PER_DEVICE=8
    CONFIG_AIC79XX_RESET_DELAY_MS=15000
    CONFIG_AIC79XX_BUILD_FIRMWARE=y <============
    CONFIG_AIC79XX_DEBUG_ENABLE=y
    CONFIG_AIC79XX_DEBUG_MASK=0
    CONFIG_AIC79XX_REG_PRETTY_PRINT=y
    CONFIG_SCSI_AIC94XX=m
    CONFIG_AIC94XX_DEBUG=y

    (full .config is attached)

    I also grepped through the directory and found no rules in Makefiles
    which regenerate _shipped files.

    If you know how to regenerate them, please let me know.

    Any other objections to the patch?
    --
    vda


+ Reply to Thread