From: Greg KH <gregkh@linuxfoundation.org>
To: Phillip Potter <phil@philpotter.co.uk>
Cc: Larry.Finger@lwfinger.net, straube.linux@gmail.com,
linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: rtl8188eu: remove ASSERT and ODM_RT_ASSERT macros
Date: Tue, 25 May 2021 08:44:44 +0200 [thread overview]
Message-ID: <YKyc3AM51xODwW0Q@kroah.com> (raw)
In-Reply-To: <20210524224532.1230-1-phil@philpotter.co.uk>
On Mon, May 24, 2021 at 11:45:32PM +0100, Phillip Potter wrote:
> Remove the ASSERT and ODM_RT_ASSERT macros from include/odm_debug.h
> as they are unnecessary.
>
> ASSERT does nothing, compiling to a single empty statement.
> ODM_RT_ASSERT is used in only one place, in the ODM_RAStateCheck
> function with hal/odm.c - it seems to have been intended as an
> assertion of some kind, but given it is always called with false
> here, there is little point in it not just being a pr_info() call.
>
> Also, the lines relating to the file, function and line number are
> not needed as the pr_info() with the function name and error message
> is sufficient should anyone wish to track down this error at a source
> level, within what is currently a relatively small function.
>
> Signed-off-by: Phillip Potter <phil@philpotter.co.uk>
> ---
> drivers/staging/rtl8188eu/hal/odm.c | 2 +-
> drivers/staging/rtl8188eu/include/odm_debug.h | 13 -------------
> 2 files changed, 1 insertion(+), 14 deletions(-)
>
> diff --git a/drivers/staging/rtl8188eu/hal/odm.c b/drivers/staging/rtl8188eu/hal/odm.c
> index 4d659a812aed..b800d0c6dff5 100644
> --- a/drivers/staging/rtl8188eu/hal/odm.c
> +++ b/drivers/staging/rtl8188eu/hal/odm.c
> @@ -824,7 +824,7 @@ bool ODM_RAStateCheck(struct odm_dm_struct *pDM_Odm, s32 RSSI, bool bForceUpdate
> LowRSSIThreshForRA += GoUpGap;
> break;
> default:
> - ODM_RT_ASSERT(pDM_Odm, false, ("wrong rssi level setting %d !", *pRATRState));
> + pr_info("%s(): wrong rssi level setting %d!\n", __func__, *pRATRState);
I know you're just copying what the existing code does, but this really
should just be a dev_err() call instead. It's not "info", and as this
is a driver, dev_*() should be called instead.
So I'll take this one, but for future cleanups, consider changing the
pr_*() calls to the correct ones.
thanks,
greg k-h
next prev parent reply other threads:[~2021-05-25 6:44 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-24 22:45 [PATCH] staging: rtl8188eu: remove ASSERT and ODM_RT_ASSERT macros Phillip Potter
2021-05-25 6:44 ` Greg KH [this message]
2021-05-25 20:51 ` Phillip Potter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YKyc3AM51xODwW0Q@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=Larry.Finger@lwfinger.net \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=phil@philpotter.co.uk \
--cc=straube.linux@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.