2020-04-11 12:28:10

by Oscar Carter

[permalink] [raw]
Subject: [PATCH v2 0/2] staging: vt6656: Refactor the vnt_get_phy_field function

This patch series makes a refactor of the vnt_get_phy_field function
through two patches.

The first one refactors the assignment of the "phy->signal" variable
using a constant array with the correct values for every rate.

The second patch removes duplicate code for the assignment of the
"phy->service" variable by putting it outside the if-else statement due
to it's the same for the two branches.

Changelog v1 -> v2:
- Remove one dimension from the constant array for the "phy->signal"
values and use an OR mask instead of the second array dimension as
Malcolm Priestley has suggested.

Oscar Carter (2):
staging: vt6656: Refactor the assignment of the phy->signal variable
staging: vt6656: Remove duplicate code for the phy->service assignment

drivers/staging/vt6656/baseband.c | 108 ++++++++----------------------
1 file changed, 27 insertions(+), 81 deletions(-)

--
2.20.1


2020-04-11 12:28:15

by Oscar Carter

[permalink] [raw]
Subject: [PATCH v2 1/2] staging: vt6656: Refactor the assignment of the phy->signal variable

Create a constant array with the values of the "phy->signal" for every
rate. Remove all "phy->signal" assignments inside the switch statement
and replace these with a single reading from the new vnt_phy_signal
array.

The constant array can be of one dimension because the OR mask with
BIT(3) or BIT(4) allow obtain a second value according to the rate,
the preamble_type and the pkt_type.

Signed-off-by: Oscar Carter <[email protected]>
---
drivers/staging/vt6656/baseband.c | 105 ++++++++----------------------
1 file changed, 26 insertions(+), 79 deletions(-)

diff --git a/drivers/staging/vt6656/baseband.c b/drivers/staging/vt6656/baseband.c
index a19a563d8bcc..05cc4797df52 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -115,6 +115,21 @@ static const u16 vnt_frame_time[MAX_RATE] = {
10, 20, 55, 110, 24, 36, 48, 72, 96, 144, 192, 216
};

+static const u8 vnt_phy_signal[] = {
+ 0x00, /* RATE_1M */
+ 0x01, /* RATE_2M */
+ 0x02, /* RATE_5M */
+ 0x03, /* RATE_11M */
+ 0x8b, /* RATE_6M */
+ 0x8f, /* RATE_9M */
+ 0x8a, /* RATE_12M */
+ 0x8e, /* RATE_18M */
+ 0x89, /* RATE_24M */
+ 0x8d, /* RATE_36M */
+ 0x88, /* RATE_48M */
+ 0x8c /* RATE_54M */
+};
+
/*
* Description: Calculate data frame transmitting time
*
@@ -183,6 +198,8 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 frame_length,
u32 count = 0;
u32 tmp;
int ext_bit;
+ int i;
+ u8 mask = 0;
u8 preamble_type = priv->preamble_type;

bit_count = frame_length * 8;
@@ -191,27 +208,12 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 frame_length,
switch (tx_rate) {
case RATE_1M:
count = bit_count;
-
- phy->signal = 0x00;
-
break;
case RATE_2M:
count = bit_count / 2;
-
- if (preamble_type == 1)
- phy->signal = 0x09;
- else
- phy->signal = 0x01;
-
break;
case RATE_5M:
count = DIV_ROUND_UP(bit_count * 10, 55);
-
- if (preamble_type == 1)
- phy->signal = 0x0a;
- else
- phy->signal = 0x02;
-
break;
case RATE_11M:
count = bit_count / 11;
@@ -224,75 +226,20 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 frame_length,
ext_bit = true;
}

- if (preamble_type == 1)
- phy->signal = 0x0b;
- else
- phy->signal = 0x03;
-
- break;
- case RATE_6M:
- if (pkt_type == PK_TYPE_11A)
- phy->signal = 0x9b;
- else
- phy->signal = 0x8b;
-
break;
- case RATE_9M:
- if (pkt_type == PK_TYPE_11A)
- phy->signal = 0x9f;
- else
- phy->signal = 0x8f;
-
- break;
- case RATE_12M:
- if (pkt_type == PK_TYPE_11A)
- phy->signal = 0x9a;
- else
- phy->signal = 0x8a;
-
- break;
- case RATE_18M:
- if (pkt_type == PK_TYPE_11A)
- phy->signal = 0x9e;
- else
- phy->signal = 0x8e;
-
- break;
- case RATE_24M:
- if (pkt_type == PK_TYPE_11A)
- phy->signal = 0x99;
- else
- phy->signal = 0x89;
-
- break;
- case RATE_36M:
- if (pkt_type == PK_TYPE_11A)
- phy->signal = 0x9d;
- else
- phy->signal = 0x8d;
-
- break;
- case RATE_48M:
- if (pkt_type == PK_TYPE_11A)
- phy->signal = 0x98;
- else
- phy->signal = 0x88;
+ }

- break;
- case RATE_54M:
+ if (tx_rate > RATE_11M) {
if (pkt_type == PK_TYPE_11A)
- phy->signal = 0x9c;
- else
- phy->signal = 0x8c;
- break;
- default:
- if (pkt_type == PK_TYPE_11A)
- phy->signal = 0x9c;
- else
- phy->signal = 0x8c;
- break;
+ mask = BIT(4);
+ } else if (tx_rate > RATE_1M) {
+ if (preamble_type == PREAMBLE_SHORT)
+ mask = BIT(3);
}

+ i = tx_rate > RATE_54M ? RATE_54M : tx_rate;
+ phy->signal = vnt_phy_signal[i] | mask;
+
if (pkt_type == PK_TYPE_11B) {
phy->service = 0x00;
if (ext_bit)
--
2.20.1

2020-04-11 12:28:24

by Oscar Carter

[permalink] [raw]
Subject: [PATCH v2 2/2] staging: vt6656: Remove duplicate code for the phy->service assignment

Take out the "phy->service" assignment from the if-else statement due to
it's the same for the two branches.

Signed-off-by: Oscar Carter <[email protected]>
---
drivers/staging/vt6656/baseband.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/vt6656/baseband.c b/drivers/staging/vt6656/baseband.c
index 05cc4797df52..c8b3cc84da6c 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -239,14 +239,13 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 frame_length,

i = tx_rate > RATE_54M ? RATE_54M : tx_rate;
phy->signal = vnt_phy_signal[i] | mask;
+ phy->service = 0x00;

if (pkt_type == PK_TYPE_11B) {
- phy->service = 0x00;
if (ext_bit)
phy->service |= 0x80;
phy->len = cpu_to_le16((u16)count);
} else {
- phy->service = 0x00;
phy->len = cpu_to_le16((u16)frame_length);
}
}
--
2.20.1

2020-04-13 15:08:32

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] staging: vt6656: Refactor the assignment of the phy->signal variable

On Sat, Apr 11, 2020 at 02:26:09PM +0200, Oscar Carter wrote:
> Create a constant array with the values of the "phy->signal" for every
> rate. Remove all "phy->signal" assignments inside the switch statement
> and replace these with a single reading from the new vnt_phy_signal
> array.
>
> The constant array can be of one dimension because the OR mask with
> BIT(3) or BIT(4) allow obtain a second value according to the rate,
> the preamble_type and the pkt_type.
>
> Signed-off-by: Oscar Carter <[email protected]>
> ---
> drivers/staging/vt6656/baseband.c | 105 ++++++++----------------------
> 1 file changed, 26 insertions(+), 79 deletions(-)

This series did not apply to my tree, please rebase and resend.

thanks,

greg k-h

2020-04-13 15:33:34

by Oscar Carter

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] staging: vt6656: Refactor the assignment of the phy->signal variable

On Mon, Apr 13, 2020 at 02:56:16PM +0200, Greg Kroah-Hartman wrote:
> On Sat, Apr 11, 2020 at 02:26:09PM +0200, Oscar Carter wrote:
> > Create a constant array with the values of the "phy->signal" for every
> > rate. Remove all "phy->signal" assignments inside the switch statement
> > and replace these with a single reading from the new vnt_phy_signal
> > array.
> >
> > The constant array can be of one dimension because the OR mask with
> > BIT(3) or BIT(4) allow obtain a second value according to the rate,
> > the preamble_type and the pkt_type.
> >
> > Signed-off-by: Oscar Carter <[email protected]>
> > ---
> > drivers/staging/vt6656/baseband.c | 105 ++++++++----------------------
> > 1 file changed, 26 insertions(+), 79 deletions(-)
>
> This series did not apply to my tree, please rebase and resend.

Rebase the patchs is a normal process in the development or am I doing something
wrong ?

>
> thanks,
>
> greg k-h

thanks,

oscar carter

2020-04-13 15:34:37

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] staging: vt6656: Refactor the assignment of the phy->signal variable

On Mon, Apr 13, 2020 at 04:25:17PM +0200, Oscar Carter wrote:
> On Mon, Apr 13, 2020 at 02:56:16PM +0200, Greg Kroah-Hartman wrote:
> > On Sat, Apr 11, 2020 at 02:26:09PM +0200, Oscar Carter wrote:
> > > Create a constant array with the values of the "phy->signal" for every
> > > rate. Remove all "phy->signal" assignments inside the switch statement
> > > and replace these with a single reading from the new vnt_phy_signal
> > > array.
> > >
> > > The constant array can be of one dimension because the OR mask with
> > > BIT(3) or BIT(4) allow obtain a second value according to the rate,
> > > the preamble_type and the pkt_type.
> > >
> > > Signed-off-by: Oscar Carter <[email protected]>
> > > ---
> > > drivers/staging/vt6656/baseband.c | 105 ++++++++----------------------
> > > 1 file changed, 26 insertions(+), 79 deletions(-)
> >
> > This series did not apply to my tree, please rebase and resend.
>
> Rebase the patchs is a normal process in the development or am I doing something
> wrong ?

It's normal when multiple people are working on the same area with lots
of patches flying around. Not a problem, it doesn't bother me at all :)

thanks,

greg k-h

2020-04-13 15:35:11

by Oscar Carter

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] staging: vt6656: Refactor the assignment of the phy->signal variable

On Mon, Apr 13, 2020 at 04:32:58PM +0200, Greg Kroah-Hartman wrote:
> On Mon, Apr 13, 2020 at 04:25:17PM +0200, Oscar Carter wrote:
> > On Mon, Apr 13, 2020 at 02:56:16PM +0200, Greg Kroah-Hartman wrote:
> > > On Sat, Apr 11, 2020 at 02:26:09PM +0200, Oscar Carter wrote:
> > > > Create a constant array with the values of the "phy->signal" for every
> > > > rate. Remove all "phy->signal" assignments inside the switch statement
> > > > and replace these with a single reading from the new vnt_phy_signal
> > > > array.
> > > >
> > > > The constant array can be of one dimension because the OR mask with
> > > > BIT(3) or BIT(4) allow obtain a second value according to the rate,
> > > > the preamble_type and the pkt_type.
> > > >
> > > > Signed-off-by: Oscar Carter <[email protected]>
> > > > ---
> > > > drivers/staging/vt6656/baseband.c | 105 ++++++++----------------------
> > > > 1 file changed, 26 insertions(+), 79 deletions(-)
> > >
> > > This series did not apply to my tree, please rebase and resend.
> >
> > Rebase the patchs is a normal process in the development or am I doing something
> > wrong ?
>
> It's normal when multiple people are working on the same area with lots
> of patches flying around. Not a problem, it doesn't bother me at all :)

Thanks for the clarification about this theme.

>
> thanks,
>
> greg k-h

thanks,

oscar carter