From: Greg KH <gregkh@linuxfoundation.org>
To: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Dan Carpenter <dan.carpenter@oracle.com>,
devel@driverdev.osuosl.org, netdev@vger.kernel.org
Subject: Re: [PATCH 02/11] staging: r8188eu: Remove wrapper around spin_lock_bh
Date: Fri, 20 Dec 2013 08:57:07 -0800 [thread overview]
Message-ID: <20131220165707.GA32581@kroah.com> (raw)
In-Reply-To: <52B471F9.9080400@lwfinger.net>
On Fri, Dec 20, 2013 at 10:36:09AM -0600, Larry Finger wrote:
> On 12/20/2013 01:14 AM, Dan Carpenter wrote:
> > On Thu, Dec 19, 2013 at 10:38:34PM -0600, Larry Finger wrote:
> >> Some comment lines that mentioned spin_lock_bh() are also removed.
> >>
> >> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> >> @@ -1509,10 +1509,6 @@ _func_enter_;
> >>
> >> rtw_p2p_set_state(pwdinfo, P2P_STATE_FIND_PHASE_SEARCH);
> >>
> >> - _enter_critical_bh(&pmlmepriv->lock, &irqL);
> >> - _exit_critical_bh(&pmlmepriv->lock, &irqL);
> >> -
> >> -
> >> _func_exit_;
> >> }
> >
> > This is a functionality change that slipped in. This is like
> > spin_unlock_wait() where you want to wait until the lock is released.
> >
> > In this code it's probably unintended? But don't put these things into
> > a patch without mentioning it.
>
> Dan,
>
> Yes, I should have mentioned that I was removing a pointless lock/unlock
> sequence. Upon further checking, I see that the original driver I received from
> Realtek did have a reason for locking there, but a line was dropped from the
> code. This section should contain the following:
>
> diff --git a/drivers/staging/rtl8188eu/core/rtw_p2p.c
> b/drivers/staging/rtl8188eu/core/rtw_p2p.c
> index 402fd21..b376d09 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_p2p.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_p2p.c
> @@ -1505,6 +1505,9 @@ _func_enter_;
>
> rtw_p2p_set_state(pwdinfo, P2P_STATE_FIND_PHASE_SEARCH);
>
> + spin_lock_bh(&pmlmepriv->lock);
> + rtw_sitesurvey_cmd(struct adapter padapter, &ssid, 1, NULL, 0);
> + spin_unlock_bh(&pmlmepriv->lock);
> _func_exit_;
> }
>
>
> Thanks for reading the patches.
>
> @Greg: Is it OK if I leave the previous patch alone and submit the above as a
> separate change?
Yes, just send a follow-on change would be fine, I'll take this as-is.
thanks,
greg k-h
next prev parent reply other threads:[~2013-12-20 16:56 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-20 4:38 [PATCH 00/11] staging: r8188eu: Clean up a number of pointless wrappers Larry Finger
2013-12-20 4:38 ` [PATCH 01/11] staging: r8188eu: Remove pointless thread_exit macro Larry Finger
2013-12-20 16:58 ` Greg KH
2013-12-20 17:22 ` Larry Finger
2013-12-20 19:18 ` Greg KH
2013-12-20 20:30 ` Larry Finger
2013-12-20 20:34 ` Greg KH
2013-12-20 20:43 ` Larry Finger
2013-12-20 4:38 ` [PATCH 02/11] staging: r8188eu: Remove wrapper around spin_lock_bh Larry Finger
2013-12-20 7:14 ` Dan Carpenter
2013-12-20 16:36 ` Larry Finger
2013-12-20 16:57 ` Greg KH [this message]
2013-12-20 4:38 ` [PATCH 03/11] staging: r8188eu: Remove wrapper around spin_unlock_bh Larry Finger
2013-12-20 4:38 ` [PATCH 04/11] staging: r8188eu: Remove wrappers for spin_lock_irqsave Larry Finger
2013-12-20 4:38 ` [PATCH 05/11] staging: r8188eu: Remove wrappers around spin_unlock_irqrestore Larry Finger
2013-12-20 4:38 ` [PATCH 06/11] staging: r8188eu: Remove wrapper around spin_lock_init Larry Finger
2013-12-20 4:38 ` [PATCH 07/11] staging: r8188eu: Remove calls to _rtw_spinlock_free Larry Finger
2013-12-20 4:38 ` [PATCH 08/11] staging: r8188eu: Replace rtw_get_current_time() with jiffies Larry Finger
2013-12-20 4:38 ` [PATCH 09/11] staging: r8188eu: Replace wrappers ODM_sleep_ms() and rtw_msleep_os() with a simple msleep Larry Finger
2013-12-20 4:38 ` [PATCH 10/11] staging: r8188eu: Replace wrappers ODM_delay_ms() and rtw_mdelay_os() with a simple mdelay Larry Finger
2013-12-20 4:38 ` [PATCH 11/11] staging: r8188eu: Replace wrappers ODM_StallExecution, ODM_delay_us, and rtw_udelay_os Larry Finger
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=20131220165707.GA32581@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=Larry.Finger@lwfinger.net \
--cc=dan.carpenter@oracle.com \
--cc=devel@driverdev.osuosl.org \
--cc=netdev@vger.kernel.org \
/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.