diff mbox series

[05/19] staging: wfx: fix coherency of hif_scan() prototype

Message ID 20200515083325.378539-6-Jerome.Pouiller@silabs.com
State Not Applicable
Delegated to: David Miller
Headers show
Series staging: wfx: various fixes | expand

Commit Message

Jérôme Pouiller May 15, 2020, 8:33 a.m. UTC 
From: Jérôme Pouiller <jerome.pouiller@silabs.com>

The function hif_scan() return the timeout for the completion of the
scan request. It is the only function from hif_tx.c that return another
thing than just an error code. This behavior is not coherent with the
rest of file. Worse, if value returned is positive, the caller can't
make say if it is a timeout or the value returned by the hardware.

Uniformize API with other HIF functions, only return the error code and
pass timeout with parameters.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_tx.c | 6 ++++--
 drivers/staging/wfx/hif_tx.h | 2 +-
 drivers/staging/wfx/scan.c   | 6 +++---
 3 files changed, 8 insertions(+), 6 deletions(-)

Comments

Greg Kroah-Hartman May 15, 2020, 1:53 p.m. UTC | #1 
On Fri, May 15, 2020 at 10:33:11AM +0200, Jerome Pouiller wrote:
> From: Jérôme Pouiller <jerome.pouiller@silabs.com>
> 
> The function hif_scan() return the timeout for the completion of the
> scan request. It is the only function from hif_tx.c that return another
> thing than just an error code. This behavior is not coherent with the
> rest of file. Worse, if value returned is positive, the caller can't
> make say if it is a timeout or the value returned by the hardware.
> 
> Uniformize API with other HIF functions, only return the error code and
> pass timeout with parameters.
> 
> Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> ---
>  drivers/staging/wfx/hif_tx.c | 6 ++++--
>  drivers/staging/wfx/hif_tx.h | 2 +-
>  drivers/staging/wfx/scan.c   | 6 +++---
>  3 files changed, 8 insertions(+), 6 deletions(-)

This patch fails to apply to my branch, so I've stopped here in the
patch series.

Can you rebase and resend the remaining ones?

thanks,

greg k-h
Greg Kroah-Hartman May 15, 2020, 2:01 p.m. UTC | #2 
On Fri, May 15, 2020 at 03:53:59PM +0200, Greg Kroah-Hartman wrote:
> On Fri, May 15, 2020 at 10:33:11AM +0200, Jerome Pouiller wrote:
> > From: Jérôme Pouiller <jerome.pouiller@silabs.com>
> > 
> > The function hif_scan() return the timeout for the completion of the
> > scan request. It is the only function from hif_tx.c that return another
> > thing than just an error code. This behavior is not coherent with the
> > rest of file. Worse, if value returned is positive, the caller can't
> > make say if it is a timeout or the value returned by the hardware.
> > 
> > Uniformize API with other HIF functions, only return the error code and
> > pass timeout with parameters.
> > 
> > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> > ---
> >  drivers/staging/wfx/hif_tx.c | 6 ++++--
> >  drivers/staging/wfx/hif_tx.h | 2 +-
> >  drivers/staging/wfx/scan.c   | 6 +++---
> >  3 files changed, 8 insertions(+), 6 deletions(-)
> 
> This patch fails to apply to my branch, so I've stopped here in the
> patch series.
> 
> Can you rebase and resend the remaining ones?

Ok, all others worked, just this one failed, so I've applied them.

greg k-h
Jérôme Pouiller May 15, 2020, 3:03 p.m. UTC | #3 
On Friday 15 May 2020 15:53:59 CEST Greg Kroah-Hartman wrote:
> On Fri, May 15, 2020 at 10:33:11AM +0200, Jerome Pouiller wrote:
> > From: Jérôme Pouiller <jerome.pouiller@silabs.com>
> >
> > The function hif_scan() return the timeout for the completion of the
> > scan request. It is the only function from hif_tx.c that return another
> > thing than just an error code. This behavior is not coherent with the
> > rest of file. Worse, if value returned is positive, the caller can't
> > make say if it is a timeout or the value returned by the hardware.
> >
> > Uniformize API with other HIF functions, only return the error code and
> > pass timeout with parameters.
> >
> > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> > ---
> >  drivers/staging/wfx/hif_tx.c | 6 ++++--
> >  drivers/staging/wfx/hif_tx.h | 2 +-
> >  drivers/staging/wfx/scan.c   | 6 +++---
> >  3 files changed, 8 insertions(+), 6 deletions(-)
> 
> This patch fails to apply to my branch, so I've stopped here in the
> patch series.

Hello Greg,

Did you applied the patch called "staging: wfx: unlock on error path" from
Dan?

(I wrote that information in the introduction letter, but maybe I would
had include the Dan's patch in my PR?)
Greg Kroah-Hartman May 15, 2020, 3:09 p.m. UTC | #4 
On Fri, May 15, 2020 at 05:03:40PM +0200, Jérôme Pouiller wrote:
> On Friday 15 May 2020 15:53:59 CEST Greg Kroah-Hartman wrote:
> > On Fri, May 15, 2020 at 10:33:11AM +0200, Jerome Pouiller wrote:
> > > From: Jérôme Pouiller <jerome.pouiller@silabs.com>
> > >
> > > The function hif_scan() return the timeout for the completion of the
> > > scan request. It is the only function from hif_tx.c that return another
> > > thing than just an error code. This behavior is not coherent with the
> > > rest of file. Worse, if value returned is positive, the caller can't
> > > make say if it is a timeout or the value returned by the hardware.
> > >
> > > Uniformize API with other HIF functions, only return the error code and
> > > pass timeout with parameters.
> > >
> > > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> > > ---
> > >  drivers/staging/wfx/hif_tx.c | 6 ++++--
> > >  drivers/staging/wfx/hif_tx.h | 2 +-
> > >  drivers/staging/wfx/scan.c   | 6 +++---
> > >  3 files changed, 8 insertions(+), 6 deletions(-)
> > 
> > This patch fails to apply to my branch, so I've stopped here in the
> > patch series.
> 
> Hello Greg,
> 
> Did you applied the patch called "staging: wfx: unlock on error path" from
> Dan?

I have no idea :)

> (I wrote that information in the introduction letter, but maybe I would
> had include the Dan's patch in my PR?)

I think you should have, as my queue is empty now.

thanks,

greg k-h
diff mbox series

Patch

diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index 893b67f2f792..6db41587cc7a 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -240,7 +240,7 @@  int hif_write_mib(struct wfx_dev *wdev, int vif_id, u16 mib_id,
 }
 
 int hif_scan(struct wfx_vif *wvif, struct cfg80211_scan_request *req,
-	     int chan_start_idx, int chan_num)
+	     int chan_start_idx, int chan_num, int *timeout)
 {
 	int ret, i;
 	struct hif_msg *hif;
@@ -289,11 +289,13 @@  int hif_scan(struct wfx_vif *wvif, struct cfg80211_scan_request *req,
 	tmo_chan_fg = 512 * USEC_PER_TU + body->probe_delay;
 	tmo_chan_fg *= body->num_of_probe_requests;
 	tmo = chan_num * max(tmo_chan_bg, tmo_chan_fg) + 512 * USEC_PER_TU;
+	if (*timeout)
+		*timeout = usecs_to_jiffies(tmo);
 
 	wfx_fill_header(hif, wvif->id, HIF_REQ_ID_START_SCAN, buf_len);
 	ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
 	kfree(hif);
-	return ret ? ret : usecs_to_jiffies(tmo);
+	return ret;
 }
 
 int hif_stop_scan(struct wfx_vif *wvif)
diff --git a/drivers/staging/wfx/hif_tx.h b/drivers/staging/wfx/hif_tx.h
index e9eca9330178..e1da28aef706 100644
--- a/drivers/staging/wfx/hif_tx.h
+++ b/drivers/staging/wfx/hif_tx.h
@@ -42,7 +42,7 @@  int hif_read_mib(struct wfx_dev *wdev, int vif_id, u16 mib_id,
 int hif_write_mib(struct wfx_dev *wdev, int vif_id, u16 mib_id,
 		  void *buf, size_t buf_size);
 int hif_scan(struct wfx_vif *wvif, struct cfg80211_scan_request *req80211,
-	     int chan_start, int chan_num);
+	     int chan_start, int chan_num, int *timeout);
 int hif_stop_scan(struct wfx_vif *wvif);
 int hif_join(struct wfx_vif *wvif, const struct ieee80211_bss_conf *conf,
 	     struct ieee80211_channel *channel, const u8 *ssid, int ssidlen);
diff --git a/drivers/staging/wfx/scan.c b/drivers/staging/wfx/scan.c
index eff1be9fb28f..bf7ddc75c7db 100644
--- a/drivers/staging/wfx/scan.c
+++ b/drivers/staging/wfx/scan.c
@@ -56,10 +56,10 @@  static int send_scan_req(struct wfx_vif *wvif,
 	wfx_tx_lock_flush(wvif->wdev);
 	wvif->scan_abort = false;
 	reinit_completion(&wvif->scan_complete);
-	timeout = hif_scan(wvif, req, start_idx, i - start_idx);
-	if (timeout < 0) {
+	ret = hif_scan(wvif, req, start_idx, i - start_idx, &timeout);
+	if (ret) {
 		wfx_tx_unlock(wvif->wdev);
-		return timeout;
+		return -EIO;
 	}
 	ret = wait_for_completion_timeout(&wvif->scan_complete, timeout);
 	if (req->channels[start_idx]->max_power != wvif->vif->bss_conf.txpower)