linux-kernel - Re: [PATCH] staging: r8188eu: make dump_chip

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] [day] [month] [year] [list]

Message-ID: <YuDbjzkSJk3gRNpN@kroah.com>
Date:   Wed, 27 Jul 2022 08:30:39 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Michael Straube <straube.linux@...il.com>
Cc:     Larry.Finger@...inger.net, phil@...lpotter.co.uk,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: r8188eu: make dump_chip_info() static

On Sun, Jul 24, 2022 at 08:25:20PM +0200, Michael Straube wrote:
> The function dump_chip_info() is only used in rtl8188e_hal_init.c.
> Make it static to reduce the driver object file size by 281 bytes.
> 
> before:
>   text    data     bss     dec     hex filename
> 530606   43897    7072  581575   8dfc7 drivers/staging/r8188eu/r8188eu.o
> 
> after:
>   text    data     bss     dec     hex filename
> 530405   43817    7072  581294   8deae drivers/staging/r8188eu/r8188eu.o
> 
> Signed-off-by: Michael Straube <straube.linux@...il.com>
> ---
>  drivers/staging/r8188eu/hal/hal_com.c         | 39 -------------------
>  .../staging/r8188eu/hal/rtl8188e_hal_init.c   | 39 +++++++++++++++++++
>  drivers/staging/r8188eu/include/hal_com.h     |  3 --
>  3 files changed, 39 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/hal/hal_com.c b/drivers/staging/r8188eu/hal/hal_com.c
> index e9a32dd84a8e..6a1cdc67335b 100644
> --- a/drivers/staging/r8188eu/hal/hal_com.c
> +++ b/drivers/staging/r8188eu/hal/hal_com.c
> @@ -10,45 +10,6 @@
>  
>  #define _HAL_INIT_C_
>  
> -void dump_chip_info(struct HAL_VERSION	chip_vers)
> -{
> -	uint cnt = 0;
> -	char buf[128];
> -
> -	cnt += sprintf((buf + cnt), "Chip Version Info: CHIP_8188E_");
> -	cnt += sprintf((buf + cnt), "%s_", IS_NORMAL_CHIP(chip_vers) ?
> -		       "Normal_Chip" : "Test_Chip");
> -	cnt += sprintf((buf + cnt), "%s_", IS_CHIP_VENDOR_TSMC(chip_vers) ?
> -		       "TSMC" : "UMC");
> -
> -	switch (chip_vers.CUTVersion) {
> -	case A_CUT_VERSION:
> -		cnt += sprintf((buf + cnt), "A_CUT_");
> -		break;
> -	case B_CUT_VERSION:
> -		cnt += sprintf((buf + cnt), "B_CUT_");
> -		break;
> -	case C_CUT_VERSION:
> -		cnt += sprintf((buf + cnt), "C_CUT_");
> -		break;
> -	case D_CUT_VERSION:
> -		cnt += sprintf((buf + cnt), "D_CUT_");
> -		break;
> -	case E_CUT_VERSION:
> -		cnt += sprintf((buf + cnt), "E_CUT_");
> -		break;
> -	default:
> -		cnt += sprintf((buf + cnt), "UNKNOWN_CUT(%d)_", chip_vers.CUTVersion);
> -		break;
> -	}
> -
> -	cnt += sprintf((buf + cnt), "1T1R_");
> -
> -	cnt += sprintf((buf + cnt), "RomVer(%d)\n", 0);
> -
> -	pr_info("%s", buf);
> -}
> -
>  #define	CHAN_PLAN_HW	0x80
>  
>  u8 /* return the final channel plan decision */
> diff --git a/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c b/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c
> index fe477438899e..5b8f1a912bbb 100644
> --- a/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c
> +++ b/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c
> @@ -526,6 +526,45 @@ void rtl8188e_ReadEFuse(struct adapter *Adapter, u16 _size_byte, u8 *pbuf)
>  	Hal_EfuseReadEFuse88E(Adapter, 0, _size_byte, pbuf);
>  }
>  
> +static void dump_chip_info(struct HAL_VERSION chip_vers)
> +{
> +	uint cnt = 0;
> +	char buf[128];
> +
> +	cnt += sprintf((buf + cnt), "Chip Version Info: CHIP_8188E_");
> +	cnt += sprintf((buf + cnt), "%s_", IS_NORMAL_CHIP(chip_vers) ?
> +		       "Normal_Chip" : "Test_Chip");
> +	cnt += sprintf((buf + cnt), "%s_", IS_CHIP_VENDOR_TSMC(chip_vers) ?
> +		       "TSMC" : "UMC");
> +
> +	switch (chip_vers.CUTVersion) {
> +	case A_CUT_VERSION:
> +		cnt += sprintf((buf + cnt), "A_CUT_");
> +		break;
> +	case B_CUT_VERSION:
> +		cnt += sprintf((buf + cnt), "B_CUT_");
> +		break;
> +	case C_CUT_VERSION:
> +		cnt += sprintf((buf + cnt), "C_CUT_");
> +		break;
> +	case D_CUT_VERSION:
> +		cnt += sprintf((buf + cnt), "D_CUT_");
> +		break;
> +	case E_CUT_VERSION:
> +		cnt += sprintf((buf + cnt), "E_CUT_");
> +		break;
> +	default:
> +		cnt += sprintf((buf + cnt), "UNKNOWN_CUT(%d)_", chip_vers.CUTVersion);
> +		break;
> +	}
> +
> +	cnt += sprintf((buf + cnt), "1T1R_");
> +
> +	cnt += sprintf((buf + cnt), "RomVer(%d)\n", 0);
> +
> +	pr_info("%s", buf);

Nit, none of this should be sent to the kernel log if all is good.  When
drivers are working properly, they are quiet.  This is spamming the
kernel log for when things are working properly, which just slows
booting down.

Also no driver should be calling pr_*() calls when they have a real
device at hand.

So this should be moved to be a dev_dbg() so that developers can still
enable it when they want to see this information, but it doesn't spam
the log for normal users.  But that's all for a future change, this one
is fine as-is, I'll take it now, thanks.

greg k-h