From: Dan Carpenter <dan.carpenter@oracle.com>
To: navin patidar <navin.patidar@gmail.com>
Cc: devel@driverdev.osuosl.org, Greg KH <gregkh@linuxfoundation.org>,
linux-kernel@vger.kernel.org,
Larry Finger <Larry.Finger@lwfinger.net>
Subject: Re: [PATCH 05/28] staging: rtl8188eu: Remove unused function rtl8188eu_ps_func()
Date: Mon, 19 May 2014 22:04:34 +0300 [thread overview]
Message-ID: <20140519190434.GS16255@mwanda> (raw)
In-Reply-To: <CAPV97ydMiagwQqS6cAS_ck1+XOZWKH82okPG0V1fLcpsLKL8zg@mail.gmail.com>
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
next prev parent reply other threads:[~2014-05-19 19:04 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-18 15:18 [PATCH 00/28] staging: rtl8188eu: Remove unnecessary functions navin patidar
2014-05-18 15:18 ` [PATCH 01/28] staging: rtl8188eu: Remove function with empty definition _InitBeaconMaxError() navin patidar
2014-05-18 15:18 ` [PATCH 02/28] staging: rtl8188eu: Remove function with empty definition _InitOperationMode() navin patidar
2014-05-18 15:18 ` [PATCH 03/28] staging: rtl8188eu: Remove function with empty definition _ps_open_RF() navin patidar
2014-05-18 15:18 ` [PATCH 04/28] staging: rtl8188eu: Remove unused function _ps_close_RF() navin patidar
2014-05-18 15:18 ` [PATCH 05/28] staging: rtl8188eu: Remove unused function rtl8188eu_ps_func() navin patidar
2014-05-19 10:52 ` Dan Carpenter
2014-05-19 15:49 ` navin patidar
2014-05-19 19:04 ` Dan Carpenter [this message]
2014-05-20 5:09 ` navin patidar
2014-05-18 15:18 ` [PATCH 06/28] staging:rtl8188eu:Remove funtion Hal_CustomizeByCustomerID_8188EU() navin patidar
2014-05-18 15:19 ` [PATCH 07/28] staging: rtl8188eu: Remove unused function rtl8188e_PHY_ConfigRFWithParaFile() navin patidar
2014-05-18 15:19 ` [PATCH 08/28] staging: rtl8188eu: Remove unused function rtl8192c_PHY_GetHWRegOriginalValue() navin patidar
2014-05-18 15:19 ` [PATCH 09/28] staging: rtl8188eu: Remove unused function PHY_UpdateTxPowerDbm8188E() navin patidar
2014-05-18 15:19 ` [PATCH 10/28] staging:rtl8188eu: Remove unused function phy_DbmToTxPwrIdx() navin patidar
2014-05-18 15:19 ` [PATCH 11/28] staging: rtl8188eu: Remove unused funtion PHY_GetTxPowerLevel8188E() navin patidar
2014-05-18 15:19 ` [PATCH 12/28] staging: rtl8188eu: Remove unused funtion phy_TxPwrIdxToDbm() navin patidar
2014-05-18 15:19 ` [PATCH 13/28] " navin patidar
2014-05-18 15:47 ` navin patidar
2014-05-18 15:19 ` [PATCH 14/28] staging: rtl8188eu: Remove unused structure rf_shadow navin patidar
2014-05-18 15:19 ` [PATCH 15/28] staging: rtl8188eu: Remove unused function rtl8188e_RF_ChangeTxPath() navin patidar
2014-05-18 15:19 ` [PATCH 16/28] staging: rtl8188eu: Remove unused function odm_DIGbyRSSI_LPS() navin patidar
2014-05-18 15:19 ` [PATCH 17/28] staging: rtl8188eu: Remove function odm_DynamicBBPowerSaving() navin patidar
2014-05-18 15:19 ` [PATCH 18/28] staging: rtl8188eu: Remove unused function odm_DynamicBBPowerSavingInit() navin patidar
2014-05-18 15:19 ` [PATCH 19/28] staging:rtl8188eu: Remove unused function iodm_1R_CCA() navin patidar
2014-05-18 15:19 ` [PATCH 20/28] staging: rtl8188eu: Remove unused function odm_RefreshRateAdaptiveMaskMP() navin patidar
2014-05-18 15:19 ` [PATCH 21/28] staging: rtl8188eu: Remove unused function odm_RefreshRateAdaptiveMaskAPADSL() navin patidar
2014-05-18 15:19 ` [PATCH 22/28] staging: rtl8188eu: Remove unused funtion odm_DynamicTxPowerNIC() navin patidar
2014-05-18 15:19 ` [PATCH 23/28] staging: rtl8188eu: Remove unused function odm_SwAntDivChkAntSwitchCallback() navin patidar
2014-05-18 15:19 ` [PATCH 24/28] staging: rtl8188eu: Remove unused function ConvertTo_dB() navin patidar
2014-05-18 15:19 ` [PATCH 25/28] staging: rtl8188eu: Remove function with empty defination ODM_CheckPowerStatus() navin patidar
2014-05-18 15:19 ` [PATCH 26/28] staging: rtl8188eu: Remove unused function DM_DIG_LowerBound_88E() navin patidar
2014-05-18 15:19 ` [PATCH 27/28] staging: rtl8188eu: Remove unused function ODM_DynamicPrimaryCCA_DupRTS() navin patidar
2014-05-18 15:19 ` [PATCH 28/28] staging: rtl8188eu:Remove function with empty defination odm_DynamicPrimaryCCA() navin patidar
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=20140519190434.GS16255@mwanda \
--to=dan.carpenter@oracle.com \
--cc=Larry.Finger@lwfinger.net \
--cc=devel@driverdev.osuosl.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=navin.patidar@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox