2023-08-01 16:05:59

by Petr Tesarik

[permalink] [raw]
Subject: [PATCH v1 1/3] wifi: brcm80211: drop struct brcmf_p2p_scan_le

From: Petr Tesarik <[email protected]>

The 'S' scan method is not implemented. The type member is always written
as 'E' and never read. Consequently, struct brcmf_p2p_scan_le can be
replaced with struct brcmf_escan_params_le.

Signed-off-by: Petr Tesarik <[email protected]>
---
.../broadcom/brcm80211/brcmfmac/p2p.c | 38 ++++---------------
1 file changed, 8 insertions(+), 30 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
index d4492d02e4ea..915f95760470 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -123,23 +123,6 @@ enum brcmf_p2p_disc_state {
WL_P2P_DISC_ST_SEARCH
};

-/**
- * struct brcmf_p2p_scan_le - P2P specific scan request.
- *
- * @type: type of scan method requested (values: 'E' or 'S').
- * @reserved: reserved (ignored).
- * @eparams: parameters used for type 'E'.
- * @sparams: parameters used for type 'S'.
- */
-struct brcmf_p2p_scan_le {
- u8 type;
- u8 reserved[3];
- union {
- struct brcmf_escan_params_le eparams;
- struct brcmf_scan_params_le sparams;
- };
-};
-
/**
* struct brcmf_p2p_pub_act_frame - WiFi P2P Public Action Frame
*
@@ -646,14 +629,14 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
{
struct brcmf_pub *drvr = p2p->cfg->pub;
s32 ret = 0;
- s32 memsize = offsetof(struct brcmf_p2p_scan_le,
- eparams.params_le.channel_list);
+ s32 memsize = offsetof(struct brcmf_escan_params_le,
+ params_le.channel_list);
s32 nprobes;
s32 active;
u32 i;
u8 *memblk;
struct brcmf_cfg80211_vif *vif;
- struct brcmf_p2p_scan_le *p2p_params;
+ struct brcmf_escan_params_le *eparams;
struct brcmf_scan_params_le *sparams;

memsize += num_chans * sizeof(__le16);
@@ -667,8 +650,8 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
ret = -EINVAL;
goto exit;
}
- p2p_params = (struct brcmf_p2p_scan_le *)memblk;
- sparams = &p2p_params->eparams.params_le;
+ eparams = (struct brcmf_escan_params_le *)memblk;
+ sparams = &eparams->params_le;

switch (search_state) {
case WL_P2P_DISC_ST_SEARCH:
@@ -698,11 +681,6 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,

brcmf_p2p_set_discover_state(vif->ifp, search_state, 0, 0);

- /*
- * set p2p scan parameters.
- */
- p2p_params->type = 'E';
-
/* determine the scan engine parameters */
sparams->bss_type = DOT11_BSSTYPE_ANY;
sparams->scan_type = BRCMF_SCANTYPE_ACTIVE;
@@ -747,9 +725,9 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
sparams->channel_list[i] = cpu_to_le16(chanspecs[i]);

/* set the escan specific parameters */
- p2p_params->eparams.version = cpu_to_le32(BRCMF_ESCAN_REQ_VERSION);
- p2p_params->eparams.action = cpu_to_le16(WL_ESCAN_ACTION_START);
- p2p_params->eparams.sync_id = cpu_to_le16(0x1234);
+ eparams->version = cpu_to_le32(BRCMF_ESCAN_REQ_VERSION);
+ eparams->action = cpu_to_le16(WL_ESCAN_ACTION_START);
+ eparams->sync_id = cpu_to_le16(0x1234);
/* perform p2p scan on primary device */
ret = brcmf_fil_bsscfg_data_set(vif->ifp, "p2p_scan", memblk, memsize);
if (!ret)
--
2.25.1



2023-08-01 18:15:55

by Petr Tesarik

[permalink] [raw]
Subject: Re: [PATCH v1 1/3] wifi: brcm80211: drop struct brcmf_p2p_scan_le

On 8/1/2023 5:36 PM, Petr Tesarik wrote:
> From: Petr Tesarik <[email protected]>
>
> The 'S' scan method is not implemented. The type member is always written
> as 'E' and never read. Consequently, struct brcmf_p2p_scan_le can be
> replaced with struct brcmf_escan_params_le.

Seeing that Hans has already fixed the actual issue with the flex array,
this whole series is not needed.

But maybe this simplification has some value on its own?

Petr T

> Signed-off-by: Petr Tesarik <[email protected]>
> ---
> .../broadcom/brcm80211/brcmfmac/p2p.c | 38 ++++---------------
> 1 file changed, 8 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> index d4492d02e4ea..915f95760470 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> @@ -123,23 +123,6 @@ enum brcmf_p2p_disc_state {
> WL_P2P_DISC_ST_SEARCH
> };
>
> -/**
> - * struct brcmf_p2p_scan_le - P2P specific scan request.
> - *
> - * @type: type of scan method requested (values: 'E' or 'S').
> - * @reserved: reserved (ignored).
> - * @eparams: parameters used for type 'E'.
> - * @sparams: parameters used for type 'S'.
> - */
> -struct brcmf_p2p_scan_le {
> - u8 type;
> - u8 reserved[3];
> - union {
> - struct brcmf_escan_params_le eparams;
> - struct brcmf_scan_params_le sparams;
> - };
> -};
> -
> /**
> * struct brcmf_p2p_pub_act_frame - WiFi P2P Public Action Frame
> *
> @@ -646,14 +629,14 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
> {
> struct brcmf_pub *drvr = p2p->cfg->pub;
> s32 ret = 0;
> - s32 memsize = offsetof(struct brcmf_p2p_scan_le,
> - eparams.params_le.channel_list);
> + s32 memsize = offsetof(struct brcmf_escan_params_le,
> + params_le.channel_list);
> s32 nprobes;
> s32 active;
> u32 i;
> u8 *memblk;
> struct brcmf_cfg80211_vif *vif;
> - struct brcmf_p2p_scan_le *p2p_params;
> + struct brcmf_escan_params_le *eparams;
> struct brcmf_scan_params_le *sparams;
>
> memsize += num_chans * sizeof(__le16);
> @@ -667,8 +650,8 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
> ret = -EINVAL;
> goto exit;
> }
> - p2p_params = (struct brcmf_p2p_scan_le *)memblk;
> - sparams = &p2p_params->eparams.params_le;
> + eparams = (struct brcmf_escan_params_le *)memblk;
> + sparams = &eparams->params_le;
>
> switch (search_state) {
> case WL_P2P_DISC_ST_SEARCH:
> @@ -698,11 +681,6 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
>
> brcmf_p2p_set_discover_state(vif->ifp, search_state, 0, 0);
>
> - /*
> - * set p2p scan parameters.
> - */
> - p2p_params->type = 'E';
> -
> /* determine the scan engine parameters */
> sparams->bss_type = DOT11_BSSTYPE_ANY;
> sparams->scan_type = BRCMF_SCANTYPE_ACTIVE;
> @@ -747,9 +725,9 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
> sparams->channel_list[i] = cpu_to_le16(chanspecs[i]);
>
> /* set the escan specific parameters */
> - p2p_params->eparams.version = cpu_to_le32(BRCMF_ESCAN_REQ_VERSION);
> - p2p_params->eparams.action = cpu_to_le16(WL_ESCAN_ACTION_START);
> - p2p_params->eparams.sync_id = cpu_to_le16(0x1234);
> + eparams->version = cpu_to_le32(BRCMF_ESCAN_REQ_VERSION);
> + eparams->action = cpu_to_le16(WL_ESCAN_ACTION_START);
> + eparams->sync_id = cpu_to_le16(0x1234);
> /* perform p2p scan on primary device */
> ret = brcmf_fil_bsscfg_data_set(vif->ifp, "p2p_scan", memblk, memsize);
> if (!ret)


2023-08-02 01:08:07

by Franky Lin

[permalink] [raw]
Subject: Re: [PATCH v1 1/3] wifi: brcm80211: drop struct brcmf_p2p_scan_le

On Tue, Aug 1, 2023 at 10:57 AM Petr Tesarik
<[email protected]> wrote:
>
> On 8/1/2023 5:36 PM, Petr Tesarik wrote:
> > From: Petr Tesarik <[email protected]>
> >
> > The 'S' scan method is not implemented. The type member is always written
> > as 'E' and never read. Consequently, struct brcmf_p2p_scan_le can be
> > replaced with struct brcmf_escan_params_le.
>
> Seeing that Hans has already fixed the actual issue with the flex array,
> this whole series is not needed.
>
> But maybe this simplification has some value on its own?

I don't think this change would work.

> Petr T
>
> > Signed-off-by: Petr Tesarik <[email protected]>
> > ---
> > .../broadcom/brcm80211/brcmfmac/p2p.c | 38 ++++---------------
> > 1 file changed, 8 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> > index d4492d02e4ea..915f95760470 100644
> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> > @@ -123,23 +123,6 @@ enum brcmf_p2p_disc_state {
> > WL_P2P_DISC_ST_SEARCH
> > };
> >
> > -/**
> > - * struct brcmf_p2p_scan_le - P2P specific scan request.
> > - *
> > - * @type: type of scan method requested (values: 'E' or 'S').
> > - * @reserved: reserved (ignored).
> > - * @eparams: parameters used for type 'E'.
> > - * @sparams: parameters used for type 'S'.
> > - */
> > -struct brcmf_p2p_scan_le {
> > - u8 type;
> > - u8 reserved[3];
> > - union {
> > - struct brcmf_escan_params_le eparams;
> > - struct brcmf_scan_params_le sparams;
> > - };
> > -};
> > -
> > /**
> > * struct brcmf_p2p_pub_act_frame - WiFi P2P Public Action Frame
> > *
> > @@ -646,14 +629,14 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
> > {
> > struct brcmf_pub *drvr = p2p->cfg->pub;
> > s32 ret = 0;
> > - s32 memsize = offsetof(struct brcmf_p2p_scan_le,
> > - eparams.params_le.channel_list);
> > + s32 memsize = offsetof(struct brcmf_escan_params_le,
> > + params_le.channel_list);
> > s32 nprobes;
> > s32 active;
> > u32 i;
> > u8 *memblk;
> > struct brcmf_cfg80211_vif *vif;
> > - struct brcmf_p2p_scan_le *p2p_params;
> > + struct brcmf_escan_params_le *eparams;
> > struct brcmf_scan_params_le *sparams;
> >
> > memsize += num_chans * sizeof(__le16);
> > @@ -667,8 +650,8 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
> > ret = -EINVAL;
> > goto exit;
> > }
> > - p2p_params = (struct brcmf_p2p_scan_le *)memblk;
> > - sparams = &p2p_params->eparams.params_le;
> > + eparams = (struct brcmf_escan_params_le *)memblk;
> > + sparams = &eparams->params_le;
> >
> > switch (search_state) {
> > case WL_P2P_DISC_ST_SEARCH:
> > @@ -698,11 +681,6 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
> >
> > brcmf_p2p_set_discover_state(vif->ifp, search_state, 0, 0);
> >
> > - /*
> > - * set p2p scan parameters.
> > - */
> > - p2p_params->type = 'E';
> > -
> > /* determine the scan engine parameters */
> > sparams->bss_type = DOT11_BSSTYPE_ANY;
> > sparams->scan_type = BRCMF_SCANTYPE_ACTIVE;
> > @@ -747,9 +725,9 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
> > sparams->channel_list[i] = cpu_to_le16(chanspecs[i]);
> >
> > /* set the escan specific parameters */
> > - p2p_params->eparams.version = cpu_to_le32(BRCMF_ESCAN_REQ_VERSION);
> > - p2p_params->eparams.action = cpu_to_le16(WL_ESCAN_ACTION_START);
> > - p2p_params->eparams.sync_id = cpu_to_le16(0x1234);
> > + eparams->version = cpu_to_le32(BRCMF_ESCAN_REQ_VERSION);
> > + eparams->action = cpu_to_le16(WL_ESCAN_ACTION_START);
> > + eparams->sync_id = cpu_to_le16(0x1234);
> > /* perform p2p scan on primary device */
> > ret = brcmf_fil_bsscfg_data_set(vif->ifp, "p2p_scan", memblk, memsize);

Firmware is expecting struct brcmf_p2p_scan_le as the parameter for
"p2p_scan" iovar.

Regards,
- Franky


Attachments:
smime.p7s (4.10 kB)
S/MIME Cryptographic Signature

2023-08-02 06:43:55

by Petr Tesařík

[permalink] [raw]
Subject: Re: [PATCH v1 1/3] wifi: brcm80211: drop struct brcmf_p2p_scan_le

On Tue, 1 Aug 2023 17:08:55 -0700
Franky Lin <[email protected]> wrote:

> On Tue, Aug 1, 2023 at 10:57 AM Petr Tesarik
> <[email protected]> wrote:
> >
> > On 8/1/2023 5:36 PM, Petr Tesarik wrote:
> > > From: Petr Tesarik <[email protected]>
> > >
> > > The 'S' scan method is not implemented. The type member is always written
> > > as 'E' and never read. Consequently, struct brcmf_p2p_scan_le can be
> > > replaced with struct brcmf_escan_params_le.
> >
> > Seeing that Hans has already fixed the actual issue with the flex array,
> > this whole series is not needed.
> >
> > But maybe this simplification has some value on its own?
>
> I don't think this change would work.
>
> > Petr T
> >
> > > Signed-off-by: Petr Tesarik <[email protected]>
> > > ---
> > > .../broadcom/brcm80211/brcmfmac/p2p.c | 38 ++++---------------
> > > 1 file changed, 8 insertions(+), 30 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> > > index d4492d02e4ea..915f95760470 100644
> > > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> > > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> > > @@ -123,23 +123,6 @@ enum brcmf_p2p_disc_state {
> > > WL_P2P_DISC_ST_SEARCH
> > > };
> > >
> > > -/**
> > > - * struct brcmf_p2p_scan_le - P2P specific scan request.
> > > - *
> > > - * @type: type of scan method requested (values: 'E' or 'S').
> > > - * @reserved: reserved (ignored).
> > > - * @eparams: parameters used for type 'E'.
> > > - * @sparams: parameters used for type 'S'.
> > > - */
> > > -struct brcmf_p2p_scan_le {
> > > - u8 type;
> > > - u8 reserved[3];
> > > - union {
> > > - struct brcmf_escan_params_le eparams;
> > > - struct brcmf_scan_params_le sparams;
> > > - };
> > > -};
> > > -
> > > /**
> > > * struct brcmf_p2p_pub_act_frame - WiFi P2P Public Action Frame
> > > *
> > > @@ -646,14 +629,14 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
> > > {
> > > struct brcmf_pub *drvr = p2p->cfg->pub;
> > > s32 ret = 0;
> > > - s32 memsize = offsetof(struct brcmf_p2p_scan_le,
> > > - eparams.params_le.channel_list);
> > > + s32 memsize = offsetof(struct brcmf_escan_params_le,
> > > + params_le.channel_list);
> > > s32 nprobes;
> > > s32 active;
> > > u32 i;
> > > u8 *memblk;
> > > struct brcmf_cfg80211_vif *vif;
> > > - struct brcmf_p2p_scan_le *p2p_params;
> > > + struct brcmf_escan_params_le *eparams;
> > > struct brcmf_scan_params_le *sparams;
> > >
> > > memsize += num_chans * sizeof(__le16);
> > > @@ -667,8 +650,8 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
> > > ret = -EINVAL;
> > > goto exit;
> > > }
> > > - p2p_params = (struct brcmf_p2p_scan_le *)memblk;
> > > - sparams = &p2p_params->eparams.params_le;
> > > + eparams = (struct brcmf_escan_params_le *)memblk;
> > > + sparams = &eparams->params_le;
> > >
> > > switch (search_state) {
> > > case WL_P2P_DISC_ST_SEARCH:
> > > @@ -698,11 +681,6 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
> > >
> > > brcmf_p2p_set_discover_state(vif->ifp, search_state, 0, 0);
> > >
> > > - /*
> > > - * set p2p scan parameters.
> > > - */
> > > - p2p_params->type = 'E';
> > > -
> > > /* determine the scan engine parameters */
> > > sparams->bss_type = DOT11_BSSTYPE_ANY;
> > > sparams->scan_type = BRCMF_SCANTYPE_ACTIVE;
> > > @@ -747,9 +725,9 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
> > > sparams->channel_list[i] = cpu_to_le16(chanspecs[i]);
> > >
> > > /* set the escan specific parameters */
> > > - p2p_params->eparams.version = cpu_to_le32(BRCMF_ESCAN_REQ_VERSION);
> > > - p2p_params->eparams.action = cpu_to_le16(WL_ESCAN_ACTION_START);
> > > - p2p_params->eparams.sync_id = cpu_to_le16(0x1234);
> > > + eparams->version = cpu_to_le32(BRCMF_ESCAN_REQ_VERSION);
> > > + eparams->action = cpu_to_le16(WL_ESCAN_ACTION_START);
> > > + eparams->sync_id = cpu_to_le16(0x1234);
> > > /* perform p2p scan on primary device */
> > > ret = brcmf_fil_bsscfg_data_set(vif->ifp, "p2p_scan", memblk, memsize);
>
> Firmware is expecting struct brcmf_p2p_scan_le as the parameter for
> "p2p_scan" iovar.

Ah, you're right, the layout is defined by the firmware protocol, and I
missed that part.

Sorry for the fuss,
Petr T