Question about rtl8723bs

Question about rtl8723bs

[Edmundo Carmona Antoranz]

Hi!

I was just trying to correct some compiler warnings coming up when
building this driver. The one problem that took me into kind of a
rabbit whole was about having K (in rtw_security.h) set as static. So,
every time it is included somewhere _and_ K is not being used, the
compiler is nagging about it. Ok.... easy fix, let's remove the static
keyword and we should be fine. Then I took a look at _where_ it was
being used.... well, nowhere.... so, ok, let's get rid of it.... oh,
here are these macros... are they in use? Actually _they are not_....
let's remove them also.... (I don't know if you are noticing a pattern
here) then I noticed that _a lot_ of code has been removed recently
and so I pulled the big guns and cleaned up the file and started to
add things back as they were being used in the build process.... in
the end, around 80 lines are gone (give or take) if working on top of
next, at least.

Is it a good idea to send a patch with all of these things removed or
they might be coming back later and so they should be kept?

Thanks in advance.

Re: Question about rtl8723bs

[Greg KH]

On Mon, Mar 22, 2021 at 08:40:11PM -0600, Edmundo Carmona Antoranz wrote:
> Hi!
> 
> I was just trying to correct some compiler warnings coming up when
> building this driver. The one problem that took me into kind of a
> rabbit whole was about having K (in rtw_security.h) set as static. So,
> every time it is included somewhere _and_ K is not being used, the
> compiler is nagging about it. Ok.... easy fix, let's remove the static
> keyword and we should be fine. Then I took a look at _where_ it was
> being used.... well, nowhere.... so, ok, let's get rid of it.... oh,
> here are these macros... are they in use? Actually _they are not_....
> let's remove them also.... (I don't know if you are noticing a pattern
> here) then I noticed that _a lot_ of code has been removed recently
> and so I pulled the big guns and cleaned up the file and started to
> add things back as they were being used in the build process.... in
> the end, around 80 lines are gone (give or take) if working on top of
> next, at least.
> 
> Is it a good idea to send a patch with all of these things removed or
> they might be coming back later and so they should be kept?

No need to keep anything that is not being used.

If it is needed, it can always be brought back at a later time.

thanks,

greg k-h

Re: Question about rtl8723bs

[Dan Carpenter]

On Mon, Mar 22, 2021 at 08:40:11PM -0600, Edmundo Carmona Antoranz wrote:
> Hi!
> 
> I was just trying to correct some compiler warnings coming up when
> building this driver. The one problem that took me into kind of a
> rabbit whole was about having K (in rtw_security.h) set as static. So,
> every time it is included somewhere _and_ K is not being used, the
> compiler is nagging about it. Ok.... easy fix, let's remove the static
> keyword and we should be fine. Then I took a look at _where_ it was
> being used.... well, nowhere.... so, ok, let's get rid of it.... oh,
> here are these macros... are they in use? Actually _they are not_....
> let's remove them also.... (I don't know if you are noticing a pattern
> here) then I noticed that _a lot_ of code has been removed recently
> and so I pulled the big guns and cleaned up the file and started to
> add things back as they were being used in the build process.... in
> the end, around 80 lines are gone (give or take) if working on top of
> next, at least.
> 
> Is it a good idea to send a patch with all of these things removed or
> they might be coming back later and so they should be kept?
> 
> Thanks in advance.

So in summary you found some unused code and deleted it?  Please send
that patch.

When I'm reviewing "delete unused code" patches, what I'm basically
looking at are the + lines in the commit.  If there are no plus lines
and we're deleting whole functions and macros and the code still
compiles fine then that's easy to review.

When you're sending patches please consider how to organize and explain
them so they are easy to review.

regards,
dan carpenter