Re: [PATCH 05/28] staging: rtl8188eu: Remove unused function rtl8188eu_ps_func()

[Dan Carpenter]

On Mon, May 19, 2014 at 09:19:07PM +0530, navin patidar wrote:
> > You have changed it to return _FAIL instead of true.  Perhaps that is ok
> > but you need to explain it in the changelog.
> >
> oops, I'll resend this patch with proper changelog.
> I have one question, do i need to send v2 of whole patch series or
> just this patch.

Greg, none of the other patches rely on this one.  Could you apply them
and drop this one?  This one is also fine except for the changelog but
Navin can send a new version in a new thread.

[ General rules on redoing patches threads ]

When you are sending a patch series and someone asks you to update one
changelog, then you can resend just the patch but you must set the
In-Reply-To email header.  You didn't do that for [PATCH 13/28] so now
the v2 of that patch is in its own thread.  That's fine because it can
be applied in any order and it doesn't matter.

If you are sending a patch series and updating one in the middle means
that the other patches won't apply then you should redo the whole thing
or ask Greg to apply the first 7 patches and redo the last ones.

If you are sending a series and you have to update a lot of patches then
resend the whole thing because otherwise it becomes confusing.

[ This patch in particular ]

Ok.  So I looked at this and the function which calls
rtw_hal_intf_ps_func() is never called, so you're right that this
function isn't used.  Probably this is what you were going to put in
your changelog and I would check and that's fine.

But when I am reviewing these kinds of "delete unused code" patches then
I just "Ok.  If there are still users the compile will break."  So I
don't have to look outside the email client.  So I would prefer if you
did it in this order:

[patch 1/4] staging: rtl8188eu: Remove unused function rtw_interface_ps_func()
[patch 2/4] staging: rtl8188eu: Remove unused function rtw_hal_intf_ps_func()
[patch 3/4] staging: rtl8188eu: Remove unused function pointer ->interface_ps_func
[patch 4/4] staging: rtl8188eu: staging: rtl8188eu: Remove unused function rtl8188eu_ps_func()

You can fold them together if you want, that's not my point.  What I'm
saying is that when you remove the ->interface_ps_func pointer first,
it's obvious that rtl8188eu_ps_func() is unused because otherwise the
compiler will complain.

But on the other hand, I also don't really care about this patch.  I
know it is ok now because I have verified it.  Resend it however you
want and I will Ack it.  No worries.  I hope the long explanation is
helpfull and thanks for doing this cleanup work.

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/