[PATCH v4 0/8] rt2800: register programing tweaks and clean ups

[PATCH v4 0/8] rt2800: register programing tweaks and clean ups

[Stanislaw Gruszka]

v3 -> v4:
- do not program addition registers for MT7620
- fix RT6362 typo

v2 -> v3:
- fix wrongly applied hunk during rebase
- add SoB

Stanislaw Gruszka (5):
  rt2800: fix registers init for MT7620
  rt2800: enable TX_PIN_CFG_LNA_PE_ bits per band
  rt2800: enable TX_PIN_CFG_RFRX_EN only for MT7620
  rt2800: remove unneeded RT6352 check
  rt2800: comment and simplify AGC init for RT6352

Tomislav Požega (3):
  rt2x00: remove unneeded check
  rt2x00: remove confusing AGC register
  rt2x00: update TX_SW_CFG2 value

 drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 150 +++++++++++++------------
 1 file changed, 78 insertions(+), 72 deletions(-)

-- 
2.7.5

[PATCH v4 1/8] rt2x00: remove unneeded check

[Stanislaw Gruszka]

From: Tomislav Požega <pozega.tomislav@gmail.com>

Remove band check from rf53xx channel config routine since all chips
using it are single band.

Signed-off-by: Tomislav Požega <pozega.tomislav@gmail.com>
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 103 ++++++++++++-------------
 1 file changed, 50 insertions(+), 53 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index 9e7b8933d30c..1a2bf6c49b82 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -2963,6 +2963,7 @@ static void rt2800_config_channel_rf53xx(struct rt2x00_dev *rt2x00dev,
 					 struct rf_channel *rf,
 					 struct channel_info *info)
 {
+	int idx = rf->channel-1;
 	u8 rfcsr;
 
 	rt2800_rfcsr_write(rt2x00dev, 8, rf->rf1);
@@ -3001,60 +3002,56 @@ static void rt2800_config_channel_rf53xx(struct rt2x00_dev *rt2x00dev,
 
 	rt2800_freq_cal_mode1(rt2x00dev);
 
-	if (rf->channel <= 14) {
-		int idx = rf->channel-1;
-
-		if (rt2x00_has_cap_bt_coexist(rt2x00dev)) {
-			if (rt2x00_rt_rev_gte(rt2x00dev, RT5390, REV_RT5390F)) {
-				/* r55/r59 value array of channel 1~14 */
-				static const char r55_bt_rev[] = {0x83, 0x83,
-					0x83, 0x73, 0x73, 0x63, 0x53, 0x53,
-					0x53, 0x43, 0x43, 0x43, 0x43, 0x43};
-				static const char r59_bt_rev[] = {0x0e, 0x0e,
-					0x0e, 0x0e, 0x0e, 0x0b, 0x0a, 0x09,
-					0x07, 0x07, 0x07, 0x07, 0x07, 0x07};
-
-				rt2800_rfcsr_write(rt2x00dev, 55,
-						   r55_bt_rev[idx]);
-				rt2800_rfcsr_write(rt2x00dev, 59,
-						   r59_bt_rev[idx]);
-			} else {
-				static const char r59_bt[] = {0x8b, 0x8b, 0x8b,
-					0x8b, 0x8b, 0x8b, 0x8b, 0x8a, 0x89,
-					0x88, 0x88, 0x86, 0x85, 0x84};
-
-				rt2800_rfcsr_write(rt2x00dev, 59, r59_bt[idx]);
-			}
+	if (rt2x00_has_cap_bt_coexist(rt2x00dev)) {
+		if (rt2x00_rt_rev_gte(rt2x00dev, RT5390, REV_RT5390F)) {
+			/* r55/r59 value array of channel 1~14 */
+			static const char r55_bt_rev[] = {0x83, 0x83,
+				0x83, 0x73, 0x73, 0x63, 0x53, 0x53,
+				0x53, 0x43, 0x43, 0x43, 0x43, 0x43};
+			static const char r59_bt_rev[] = {0x0e, 0x0e,
+				0x0e, 0x0e, 0x0e, 0x0b, 0x0a, 0x09,
+				0x07, 0x07, 0x07, 0x07, 0x07, 0x07};
+
+			rt2800_rfcsr_write(rt2x00dev, 55,
+					   r55_bt_rev[idx]);
+			rt2800_rfcsr_write(rt2x00dev, 59,
+					   r59_bt_rev[idx]);
 		} else {
-			if (rt2x00_rt_rev_gte(rt2x00dev, RT5390, REV_RT5390F)) {
-				static const char r55_nonbt_rev[] = {0x23, 0x23,
-					0x23, 0x23, 0x13, 0x13, 0x03, 0x03,
-					0x03, 0x03, 0x03, 0x03, 0x03, 0x03};
-				static const char r59_nonbt_rev[] = {0x07, 0x07,
-					0x07, 0x07, 0x07, 0x07, 0x07, 0x07,
-					0x07, 0x07, 0x06, 0x05, 0x04, 0x04};
-
-				rt2800_rfcsr_write(rt2x00dev, 55,
-						   r55_nonbt_rev[idx]);
-				rt2800_rfcsr_write(rt2x00dev, 59,
-						   r59_nonbt_rev[idx]);
-			} else if (rt2x00_rt(rt2x00dev, RT5390) ||
-				   rt2x00_rt(rt2x00dev, RT5392) ||
-				   rt2x00_rt(rt2x00dev, RT6352)) {
-				static const char r59_non_bt[] = {0x8f, 0x8f,
-					0x8f, 0x8f, 0x8f, 0x8f, 0x8f, 0x8d,
-					0x8a, 0x88, 0x88, 0x87, 0x87, 0x86};
-
-				rt2800_rfcsr_write(rt2x00dev, 59,
-						   r59_non_bt[idx]);
-			} else if (rt2x00_rt(rt2x00dev, RT5350)) {
-				static const char r59_non_bt[] = {0x0b, 0x0b,
-					0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0a,
-					0x0a, 0x09, 0x08, 0x07, 0x07, 0x06};
-
-				rt2800_rfcsr_write(rt2x00dev, 59,
-						   r59_non_bt[idx]);
-			}
+			static const char r59_bt[] = {0x8b, 0x8b, 0x8b,
+				0x8b, 0x8b, 0x8b, 0x8b, 0x8a, 0x89,
+				0x88, 0x88, 0x86, 0x85, 0x84};
+
+			rt2800_rfcsr_write(rt2x00dev, 59, r59_bt[idx]);
+		}
+	} else {
+		if (rt2x00_rt_rev_gte(rt2x00dev, RT5390, REV_RT5390F)) {
+			static const char r55_nonbt_rev[] = {0x23, 0x23,
+				0x23, 0x23, 0x13, 0x13, 0x03, 0x03,
+				0x03, 0x03, 0x03, 0x03, 0x03, 0x03};
+			static const char r59_nonbt_rev[] = {0x07, 0x07,
+				0x07, 0x07, 0x07, 0x07, 0x07, 0x07,
+				0x07, 0x07, 0x06, 0x05, 0x04, 0x04};
+
+			rt2800_rfcsr_write(rt2x00dev, 55,
+					   r55_nonbt_rev[idx]);
+			rt2800_rfcsr_write(rt2x00dev, 59,
+					   r59_nonbt_rev[idx]);
+		} else if (rt2x00_rt(rt2x00dev, RT5390) ||
+			   rt2x00_rt(rt2x00dev, RT5392) ||
+			   rt2x00_rt(rt2x00dev, RT6352)) {
+			static const char r59_non_bt[] = {0x8f, 0x8f,
+				0x8f, 0x8f, 0x8f, 0x8f, 0x8f, 0x8d,
+				0x8a, 0x88, 0x88, 0x87, 0x87, 0x86};
+
+			rt2800_rfcsr_write(rt2x00dev, 59,
+					   r59_non_bt[idx]);
+		} else if (rt2x00_rt(rt2x00dev, RT5350)) {
+			static const char r59_non_bt[] = {0x0b, 0x0b,
+				0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0a,
+				0x0a, 0x09, 0x08, 0x07, 0x07, 0x06};
+
+			rt2800_rfcsr_write(rt2x00dev, 59,
+					   r59_non_bt[idx]);
 		}
 	}
 }
-- 
2.7.5

[PATCH v4 2/8] rt2x00: remove confusing AGC register

[Stanislaw Gruszka]

From: Tomislav Požega <pozega.tomislav@gmail.com>

Register 66 was causing issues on RT6352 if set to the same value as
in MTK driver. With 1c reg value device was working fine in both HT20
and HT40 modes.

Signed-off-by: Tomislav Požega <pozega.tomislav@gmail.com>
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index 1a2bf6c49b82..3a04eaef8511 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -3981,11 +3981,7 @@ static void rt2800_config_channel(struct rt2x00_dev *rt2x00dev,
 		rt2800_bbp_write(rt2x00dev, 196, reg);
 
 		/* AGC init */
-		if (rt2x00_rt(rt2x00dev, RT6352))
-			reg = 0x04;
-		else
-			reg = rf->channel <= 14 ? 0x1c : 0x24;
-
+		reg = rf->channel <= 14 ? 0x1c : 0x24;
 		reg += 2 * rt2x00dev->lna_gain;
 		rt2800_bbp_write_with_rx_chain(rt2x00dev, 66, reg);
 
-- 
2.7.5

[PATCH v4 3/8] rt2x00: update TX_SW_CFG2 value

[Stanislaw Gruszka]

From: Tomislav Požega <pozega.tomislav@gmail.com>

Use default value of TX_SW_CFG2 register that is in charge
of LNA timings. Works for somewhat higher RX throughput.

Signed-off-by: Tomislav Požega <pozega.tomislav@gmail.com>
---
 drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index 3a04eaef8511..daf20d7424ac 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -5465,7 +5465,7 @@ static int rt2800_init_registers(struct rt2x00_dev *rt2x00dev)
 	} else if (rt2x00_rt(rt2x00dev, RT6352)) {
 		rt2800_register_write(rt2x00dev, TX_SW_CFG0, 0x00000401);
 		rt2800_register_write(rt2x00dev, TX_SW_CFG1, 0x000C0000);
-		rt2800_register_write(rt2x00dev, TX_SW_CFG2, 0x00000000);
+		rt2800_register_write(rt2x00dev, TX_SW_CFG2, 0x000C0408);
 		rt2800_register_write(rt2x00dev, MIMO_PS_CFG, 0x00000002);
 		rt2800_register_write(rt2x00dev, TX_PIN_CFG, 0x00150F0F);
 		rt2800_register_write(rt2x00dev, TX_ALC_VGA3, 0x06060606);
-- 
2.7.5

[PATCH v4 4/8] rt2800: fix registers init for MT7620

[Stanislaw Gruszka]

There is duplicated 'if (rt2x00_rt(rt2x00dev, RT6352))' entry that
causes we do not perform register initialization for RT6352 (MT7620
SOCs) in correct branch. Fix this and disable registers initialization
that is specific to particular MT7620 revision.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index daf20d7424ac..16d6d99b1d44 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -5451,8 +5451,7 @@ static int rt2800_init_registers(struct rt2x00_dev *rt2x00dev)
 					      0x00000000);
 		}
 	} else if (rt2x00_rt(rt2x00dev, RT5390) ||
-		   rt2x00_rt(rt2x00dev, RT5392) ||
-		   rt2x00_rt(rt2x00dev, RT6352)) {
+		   rt2x00_rt(rt2x00dev, RT5392)) {
 		rt2800_register_write(rt2x00dev, TX_SW_CFG0, 0x00000404);
 		rt2800_register_write(rt2x00dev, TX_SW_CFG1, 0x00080606);
 		rt2800_register_write(rt2x00dev, TX_SW_CFG2, 0x00000000);
@@ -5466,6 +5465,10 @@ static int rt2800_init_registers(struct rt2x00_dev *rt2x00dev)
 		rt2800_register_write(rt2x00dev, TX_SW_CFG0, 0x00000401);
 		rt2800_register_write(rt2x00dev, TX_SW_CFG1, 0x000C0000);
 		rt2800_register_write(rt2x00dev, TX_SW_CFG2, 0x000C0408);
+		/* TODO add chip version support and init registers
+		 * according to the version.
+		 */
+#if 0
 		rt2800_register_write(rt2x00dev, MIMO_PS_CFG, 0x00000002);
 		rt2800_register_write(rt2x00dev, TX_PIN_CFG, 0x00150F0F);
 		rt2800_register_write(rt2x00dev, TX_ALC_VGA3, 0x06060606);
@@ -5480,6 +5483,7 @@ static int rt2800_init_registers(struct rt2x00_dev *rt2x00dev)
 		reg = rt2800_register_read(rt2x00dev, TX_ALC_CFG_1);
 		rt2x00_set_field32(&reg, TX_ALC_CFG_1_ROS_BUSY_EN, 0);
 		rt2800_register_write(rt2x00dev, TX_ALC_CFG_1, reg);
+#endif
 	} else {
 		rt2800_register_write(rt2x00dev, TX_SW_CFG0, 0x00000000);
 		rt2800_register_write(rt2x00dev, TX_SW_CFG1, 0x00080606);
-- 
2.7.5

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Tom Psyborg]

chip version support exist in daniel's tree since a long time ago. so
don't disable registers initialization but try to upstream his
changes.

changing TX_SW_CFG* entries did not make any noticeable difference in
my tests either, besides small RX improvement with configured
TX_SW_CFG2.

waiting for more of your test results

On 12/10/2018, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> There is duplicated 'if (rt2x00_rt(rt2x00dev, RT6352))' entry that
> causes we do not perform register initialization for RT6352 (MT7620
> SOCs) in correct branch. Fix this and disable registers initialization
> that is specific to particular MT7620 revision.
>
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> index daf20d7424ac..16d6d99b1d44 100644
> --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> @@ -5451,8 +5451,7 @@ static int rt2800_init_registers(struct rt2x00_dev
> *rt2x00dev)
>  					      0x00000000);
>  		}
>  	} else if (rt2x00_rt(rt2x00dev, RT5390) ||
> -		   rt2x00_rt(rt2x00dev, RT5392) ||
> -		   rt2x00_rt(rt2x00dev, RT6352)) {
> +		   rt2x00_rt(rt2x00dev, RT5392)) {
>  		rt2800_register_write(rt2x00dev, TX_SW_CFG0, 0x00000404);
>  		rt2800_register_write(rt2x00dev, TX_SW_CFG1, 0x00080606);
>  		rt2800_register_write(rt2x00dev, TX_SW_CFG2, 0x00000000);
> @@ -5466,6 +5465,10 @@ static int rt2800_init_registers(struct rt2x00_dev
> *rt2x00dev)
>  		rt2800_register_write(rt2x00dev, TX_SW_CFG0, 0x00000401);
>  		rt2800_register_write(rt2x00dev, TX_SW_CFG1, 0x000C0000);
>  		rt2800_register_write(rt2x00dev, TX_SW_CFG2, 0x000C0408);
> +		/* TODO add chip version support and init registers
> +		 * according to the version.
> +		 */
> +#if 0
>  		rt2800_register_write(rt2x00dev, MIMO_PS_CFG, 0x00000002);
>  		rt2800_register_write(rt2x00dev, TX_PIN_CFG, 0x00150F0F);
>  		rt2800_register_write(rt2x00dev, TX_ALC_VGA3, 0x06060606);
> @@ -5480,6 +5483,7 @@ static int rt2800_init_registers(struct rt2x00_dev
> *rt2x00dev)
>  		reg = rt2800_register_read(rt2x00dev, TX_ALC_CFG_1);
>  		rt2x00_set_field32(&reg, TX_ALC_CFG_1_ROS_BUSY_EN, 0);
>  		rt2800_register_write(rt2x00dev, TX_ALC_CFG_1, reg);
> +#endif
>  	} else {
>  		rt2800_register_write(rt2x00dev, TX_SW_CFG0, 0x00000000);
>  		rt2800_register_write(rt2x00dev, TX_SW_CFG1, 0x00080606);
> --
> 2.7.5
>
>

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Stanislaw Gruszka]

On Fri, Oct 12, 2018 at 12:48:07PM +0200, Tom Psyborg wrote:
> chip version support exist in daniel's tree since a long time ago. so
> don't disable registers initialization but try to upstream his
> changes.

I do not see reason for for blocking this change because some other
changes are not unstreamed yet. When chip version support will
be unstreamed, the register initialization will be unblocked.

Regards.
Stanislaw

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Tom Psyborg]

it will cause regression on other devices

On 12/10/2018, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> On Fri, Oct 12, 2018 at 12:48:07PM +0200, Tom Psyborg wrote:
>> chip version support exist in daniel's tree since a long time ago. so
>> don't disable registers initialization but try to upstream his
>> changes.
>
> I do not see reason for for blocking this change because some other
> changes are not unstreamed yet. When chip version support will
> be unstreamed, the register initialization will be unblocked.
>
> Regards.
> Stanislaw
>

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Stanislaw Gruszka]

Please stop top-posting.

On Fri, Oct 12, 2018 at 01:51:00PM +0200, Tom Psyborg wrote:
> it will cause regression on other devices

How exactly ? On upstream tree where this patch is intended
additional registers where never programmed as proper branch 
were never used, because of additional check in RT5390 branch.

Patch does only change TX_SW_CFG* regs values for RT6352.

Thanks
Stanislaw

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Tom Psyborg]

On 12/10/2018, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> Please stop top-posting.
>
> On Fri, Oct 12, 2018 at 01:51:00PM +0200, Tom Psyborg wrote:
>> it will cause regression on other devices
>
> How exactly ?

the same way your wifi works without TX_SW_CFG entries and mine
doesn't, while both are RT6352

> On upstream tree where this patch is intended
> additional registers where never programmed as proper branch
> were never used, because of additional check in RT5390 branch.
>

on my hardware additional registers were programmed in regardless of
redundant check. that why i opened whole thread on forum since i
couldn't understand how's that happening

> Patch does only change TX_SW_CFG* regs values for RT6352.
>

i'd still prefer that we include CONFIG_RT2800SOC, and if required
move rest of the registers to that check, because at least on my
hardware driver would still recognize chip as RT5390 despite the
RT6352 defines

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Stanislaw Gruszka]

On Fri, Oct 12, 2018 at 02:20:07PM +0200, Tom Psyborg wrote:
> > On upstream tree where this patch is intended
> > additional registers where never programmed as proper branch
> > were never used, because of additional check in RT5390 branch.
> >
> 
> on my hardware additional registers were programmed in regardless of
> redundant check. that why i opened whole thread on forum since i
> couldn't understand how's that happening

I don't understand how that possible either.

> > Patch does only change TX_SW_CFG* regs values for RT6352.
> >
> 
> i'd still prefer that we include CONFIG_RT2800SOC, and if required
> move rest of the registers to that check, because at least on my
> hardware driver would still recognize chip as RT5390 despite the
> RT6352 defines

As I pointed before you should add additional printk's and provide
dmesg to make us see what is going on.

Thanks
Stanislaw

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Tom Psyborg]

On 12/10/2018, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> On Fri, Oct 12, 2018 at 02:20:07PM +0200, Tom Psyborg wrote:
>> > On upstream tree where this patch is intended
>> > additional registers where never programmed as proper branch
>> > were never used, because of additional check in RT5390 branch.
>> >
>>
>> on my hardware additional registers were programmed in regardless of
>> redundant check. that why i opened whole thread on forum since i
>> couldn't understand how's that happening
>
> I don't understand how that possible either.

i'd assume because device use external lna
>
>> > Patch does only change TX_SW_CFG* regs values for RT6352.
>> >
>>
>> i'd still prefer that we include CONFIG_RT2800SOC, and if required
>> move rest of the registers to that check, because at least on my
>> hardware driver would still recognize chip as RT5390 despite the
>> RT6352 defines
>
> As I pointed before you should add additional printk's and provide
> dmesg to make us see what is going on.

sorry,my hardware is broken, maybe somebody else could provide us with
additional printks

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Stanislaw Gruszka]

On Fri, Oct 12, 2018 at 02:41:41PM +0200, Tom Psyborg wrote:
> On 12/10/2018, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> > On Fri, Oct 12, 2018 at 02:20:07PM +0200, Tom Psyborg wrote:
> >> > On upstream tree where this patch is intended
> >> > additional registers where never programmed as proper branch
> >> > were never used, because of additional check in RT5390 branch.
> >> >
> >>
> >> on my hardware additional registers were programmed in regardless of
> >> redundant check. that why i opened whole thread on forum since i
> >> couldn't understand how's that happening
> >
> > I don't understand how that possible either.
> 
> i'd assume because device use external lna

I have no idea how this could be related. But I think I found
somewhat reasonable explenation where the problem is.
I think below code :

	if (a || b || c) {
		CODE1();
	} else if (c) {
		CODE2();
	}

can not be deterministic and can be compiled differently depending
on compiler version and used options. Sometimes it could result
in this 

	if (a || b || c) {
		CODE1();
	}

and sometimes in this:

	if (a || b) {
		CODE1();
	} else if (c) {
		CODE2();
	}

So that would explain the problems you see. And indeed patch
could cause regression on systems where second variant of
initalizing RT6352 registers was used.

Thanks
Stanislaw

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Felix Fietkau]

On 2018-10-16 10:09, Stanislaw Gruszka wrote:
> On Fri, Oct 12, 2018 at 02:41:41PM +0200, Tom Psyborg wrote:
>> On 12/10/2018, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
>> > On Fri, Oct 12, 2018 at 02:20:07PM +0200, Tom Psyborg wrote:
>> >> > On upstream tree where this patch is intended
>> >> > additional registers where never programmed as proper branch
>> >> > were never used, because of additional check in RT5390 branch.
>> >> >
>> >>
>> >> on my hardware additional registers were programmed in regardless of
>> >> redundant check. that why i opened whole thread on forum since i
>> >> couldn't understand how's that happening
>> >
>> > I don't understand how that possible either.
>> 
>> i'd assume because device use external lna
> 
> I have no idea how this could be related. But I think I found
> somewhat reasonable explenation where the problem is.
> I think below code :
> 
> 	if (a || b || c) {
> 		CODE1();
> 	} else if (c) {
> 		CODE2();
> 	}
> 
> can not be deterministic and can be compiled differently depending
> on compiler version and used options. Sometimes it could result
> in this 
> 
> 	if (a || b || c) {
> 		CODE1();
> 	}
> 
> and sometimes in this:
> 
> 	if (a || b) {
> 		CODE1();
> 	} else if (c) {
> 		CODE2();
> 	}
> 
> So that would explain the problems you see. And indeed patch
> could cause regression on systems where second variant of
> initalizing RT6352 registers was used.
I don't see how that can be non-deterministic at all. The 'else if' part
can only be hit if the first if did not match.

- Felix

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Stanislaw Gruszka]

On Tue, Oct 16, 2018 at 01:19:52PM +0200, Felix Fietkau wrote:
> > I have no idea how this could be related. But I think I found
> > somewhat reasonable explenation where the problem is.
> > I think below code :
> > 
> > 	if (a || b || c) {
> > 		CODE1();
> > 	} else if (c) {
> > 		CODE2();
> > 	}
> > 
> > can not be deterministic and can be compiled differently depending
> > on compiler version and used options. Sometimes it could result
> > in this 
> > 
> > 	if (a || b || c) {
> > 		CODE1();
> > 	}
> > 
> > and sometimes in this:
> > 
> > 	if (a || b) {
> > 		CODE1();
> > 	} else if (c) {
> > 		CODE2();
> > 	}
> > 
> > So that would explain the problems you see. And indeed patch
> > could cause regression on systems where second variant of
> > initalizing RT6352 registers was used.
> I don't see how that can be non-deterministic at all. The 'else if' part
> can only be hit if the first if did not match.

I meant non-deterministic during compilation process, when compiler
do or do not some optimizations or if compiler version differs.

Regards
Stanislaw

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Felix Fietkau]

On 2018-10-16 13:21, Stanislaw Gruszka wrote:
> On Tue, Oct 16, 2018 at 01:19:52PM +0200, Felix Fietkau wrote:
>> > I have no idea how this could be related. But I think I found
>> > somewhat reasonable explenation where the problem is.
>> > I think below code :
>> > 
>> > 	if (a || b || c) {
>> > 		CODE1();
>> > 	} else if (c) {
>> > 		CODE2();
>> > 	}
>> > 
>> > can not be deterministic and can be compiled differently depending
>> > on compiler version and used options. Sometimes it could result
>> > in this 
>> > 
>> > 	if (a || b || c) {
>> > 		CODE1();
>> > 	}
>> > 
>> > and sometimes in this:
>> > 
>> > 	if (a || b) {
>> > 		CODE1();
>> > 	} else if (c) {
>> > 		CODE2();
>> > 	}
>> > 
>> > So that would explain the problems you see. And indeed patch
>> > could cause regression on systems where second variant of
>> > initalizing RT6352 registers was used.
>> I don't see how that can be non-deterministic at all. The 'else if' part
>> can only be hit if the first if did not match.
> 
> I meant non-deterministic during compilation process, when compiler
> do or do not some optimizations or if compiler version differs.
In my opinion, this is not C undefined behavior territory. The compiler
is not allowed to change the behavior here based on optimization settings.

- Felix

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Tom Psyborg]

On 16/10/2018, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> On Fri, Oct 12, 2018 at 02:41:41PM +0200, Tom Psyborg wrote:
>> On 12/10/2018, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
>> > On Fri, Oct 12, 2018 at 02:20:07PM +0200, Tom Psyborg wrote:
>> >> > On upstream tree where this patch is intended
>> >> > additional registers where never programmed as proper branch
>> >> > were never used, because of additional check in RT5390 branch.
>> >> >
>> >>
>> >> on my hardware additional registers were programmed in regardless of
>> >> redundant check. that why i opened whole thread on forum since i
>> >> couldn't understand how's that happening
>> >
>> > I don't understand how that possible either.
>>
>> i'd assume because device use external lna
>
> I have no idea how this could be related. But I think I found
> somewhat reasonable explenation where the problem is.
> I think below code :
>
> 	if (a || b || c) {
> 		CODE1();
> 	} else if (c) {
> 		CODE2();
> 	}
>
> can not be deterministic and can be compiled differently depending
> on compiler version and used options. Sometimes it could result
> in this
>
> 	if (a || b || c) {
> 		CODE1();
> 	}
>
> and sometimes in this:
>
> 	if (a || b) {
> 		CODE1();
> 	} else if (c) {
> 		CODE2();
> 	}
>
> So that would explain the problems you see. And indeed patch
> could cause regression on systems where second variant of
> initalizing RT6352 registers was used.
>
> Thanks
> Stanislaw
>

Hi

I am sending you two builds privately so please check if there are any
differences between the two builds and report back. Thanks.

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Stanislaw Gruszka]

Hello

On Tue, Oct 16, 2018 at 01:32:18PM +0200, Tom Psyborg wrote:
> I am sending you two builds privately so please check if there are any
> differences between the two builds and report back. Thanks.

I extracted rt2800lib.ko module from provided images, did disassembly via:

./staging_dir/toolchain-mipsel_24kc_gcc-7.3.0_musl/bin/mipsel-openwrt-linux-objdump \
 -d -r --prefix-addresses ~/rt2800lib-BUILDn.ko  > ~/BUILDn.dump.txt

command and compered disassembled code. Here is difference:

$ diff -up  BUILD1.dump.txt BUILD2.dump.txt 
--- BUILD1.dump.txt	2018-10-16 16:40:34.834220838 +0200
+++ BUILD2.dump.txt	2018-10-16 16:40:40.187219211 +0200
@@ -1,5 +1,5 @@
 
-/home/stasiu/rt2800lib-BUILD1.ko:     file format elf32-tradlittlemips
+/home/stasiu/rt2800lib-BUILD2.ko:     file format elf32-tradlittlemips
 
 
 Disassembly of section .text:
@@ -9374,7 +9374,7 @@ Disassembly of section .text:
 00007f80 <rt2800_clear_beacon+0x224> jalr	v0
 00007f84 <rt2800_clear_beacon+0x228> move	a0,s0
 00007f88 <rt2800_clear_beacon+0x22c> lhu	v1,732(s0)
-00007f8c <rt2800_clear_beacon+0x230> li	v0,21392
+00007f8c <rt2800_clear_beacon+0x230> li	v0,25426
 00007f90 <rt2800_clear_beacon+0x234> bne	v1,v0,0000810c <rt2800_clear_beacon+0x3b0>
 00007f94 <rt2800_clear_beacon+0x238> li	a2,1025
 00007f98 <rt2800_clear_beacon+0x23c> lw	v0,4(s0)

There is no difference in init_registers (which is inlined in
rt2800_enable_radio). The only difference is in some number
rt2800_clear_beacon() function.

Regards
Stanislaw

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Tom Psyborg]

On 16/10/2018, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> Hello
>
> On Tue, Oct 16, 2018 at 01:32:18PM +0200, Tom Psyborg wrote:
>> I am sending you two builds privately so please check if there are any
>> differences between the two builds and report back. Thanks.
>
> I extracted rt2800lib.ko module from provided images, did disassembly via:
>
> ./staging_dir/toolchain-mipsel_24kc_gcc-7.3.0_musl/bin/mipsel-openwrt-linux-objdump
> \
>  -d -r --prefix-addresses ~/rt2800lib-BUILDn.ko  > ~/BUILDn.dump.txt
>
> command and compered disassembled code. Here is difference:
>
> $ diff -up  BUILD1.dump.txt BUILD2.dump.txt
> --- BUILD1.dump.txt	2018-10-16 16:40:34.834220838 +0200
> +++ BUILD2.dump.txt	2018-10-16 16:40:40.187219211 +0200
> @@ -1,5 +1,5 @@
>
> -/home/stasiu/rt2800lib-BUILD1.ko:     file format elf32-tradlittlemips
> +/home/stasiu/rt2800lib-BUILD2.ko:     file format elf32-tradlittlemips
>
>
>  Disassembly of section .text:
> @@ -9374,7 +9374,7 @@ Disassembly of section .text:
>  00007f80 <rt2800_clear_beacon+0x224> jalr	v0
>  00007f84 <rt2800_clear_beacon+0x228> move	a0,s0
>  00007f88 <rt2800_clear_beacon+0x22c> lhu	v1,732(s0)
> -00007f8c <rt2800_clear_beacon+0x230> li	v0,21392
> +00007f8c <rt2800_clear_beacon+0x230> li	v0,25426
>  00007f90 <rt2800_clear_beacon+0x234> bne	v1,v0,0000810c
> <rt2800_clear_beacon+0x3b0>
>  00007f94 <rt2800_clear_beacon+0x238> li	a2,1025
>  00007f98 <rt2800_clear_beacon+0x23c> lw	v0,4(s0)
>
> There is no difference in init_registers (which is inlined in
> rt2800_enable_radio). The only difference is in some number
> rt2800_clear_beacon() function.
>
> Regards
> Stanislaw
>

i meant you try it on your nexx device. and post dmesg if you can boot them

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Stanislaw Gruszka]

On Wed, Oct 17, 2018 at 03:25:58PM +0200, Tom Psyborg wrote:
> On 16/10/2018, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> > On Tue, Oct 16, 2018 at 01:32:18PM +0200, Tom Psyborg wrote:
> >> I am sending you two builds privately so please check if there are any
> >> differences between the two builds and report back. Thanks.
> >
> > I extracted rt2800lib.ko module from provided images, did disassembly via:
> >
> > ./staging_dir/toolchain-mipsel_24kc_gcc-7.3.0_musl/bin/mipsel-openwrt-linux-objdump
> > \
> >  -d -r --prefix-addresses ~/rt2800lib-BUILDn.ko  > ~/BUILDn.dump.txt
> >
> > command and compered disassembled code. Here is difference:
> >
> > $ diff -up  BUILD1.dump.txt BUILD2.dump.txt
> > --- BUILD1.dump.txt	2018-10-16 16:40:34.834220838 +0200
> > +++ BUILD2.dump.txt	2018-10-16 16:40:40.187219211 +0200
> > @@ -1,5 +1,5 @@
> >
> > -/home/stasiu/rt2800lib-BUILD1.ko:     file format elf32-tradlittlemips
> > +/home/stasiu/rt2800lib-BUILD2.ko:     file format elf32-tradlittlemips
> >
> >
> >  Disassembly of section .text:
> > @@ -9374,7 +9374,7 @@ Disassembly of section .text:
> >  00007f80 <rt2800_clear_beacon+0x224> jalr	v0
> >  00007f84 <rt2800_clear_beacon+0x228> move	a0,s0
> >  00007f88 <rt2800_clear_beacon+0x22c> lhu	v1,732(s0)
> > -00007f8c <rt2800_clear_beacon+0x230> li	v0,21392
> > +00007f8c <rt2800_clear_beacon+0x230> li	v0,25426
> >  00007f90 <rt2800_clear_beacon+0x234> bne	v1,v0,0000810c
> > <rt2800_clear_beacon+0x3b0>
> >  00007f94 <rt2800_clear_beacon+0x238> li	a2,1025
> >  00007f98 <rt2800_clear_beacon+0x23c> lw	v0,4(s0)
> >
> > There is no difference in init_registers (which is inlined in
> > rt2800_enable_radio). The only difference is in some number
> > rt2800_clear_beacon() function.
> >
> > Regards
> > Stanislaw
> >
> 
> i meant you try it on your nexx device. and post dmesg if you can boot them

I tried to do this, but somehow after update BUILD1 image into device
my configuration was wiped out :-( and I have to reconfigure the
device now. Anyway I'm going to test and provide dmesg , but this
will take some time.

Regards
Stanislaw

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Tom Psyborg]

On 19/10/2018, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
>
> I tried to do this, but somehow after update BUILD1 image into device
> my configuration was wiped out :-( and I have to reconfigure the
> device now. Anyway I'm going to test and provide dmesg , but this
> will take some time.
>
> Regards
> Stanislaw
>

that's because these builds were done on 4.4 that i had in my system
and there are config differences between these builds and current
snapshot. to save you time i need only bootlogs not wifi performance
tests.

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Stanislaw Gruszka]

[-- Attachment #1: Type: text/plain, Size: 755 bytes --]

On Fri, Oct 19, 2018 at 04:21:09PM +0200, Tom Psyborg wrote:
> On 19/10/2018, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> >
> > I tried to do this, but somehow after update BUILD1 image into device
> > my configuration was wiped out :-( and I have to reconfigure the
> > device now. Anyway I'm going to test and provide dmesg , but this
> > will take some time.
> >
> > Regards
> > Stanislaw
> >
> 
> that's because these builds were done on 4.4 that i had in my system
> and there are config differences between these builds and current
> snapshot. to save you time i need only bootlogs not wifi performance
> tests.

So, there is no diffrence in dmesg and device is recognized as RT6352.
I atteched both dmesg's for the record.

Regards
Stanislaw


[-- Attachment #2: BUILD1.dmesg.txt --]
[-- Type: text/plain, Size: 10797 bytes --]

[    0.000000] Linux version 4.9.73 (ubuntu@ubuntu) (gcc version 5.5.0 (OpenWrt GCC 5.5.0 r5682-b153dbf) ) #0 Fri Jan 5 10:59:59 2018
[    0.000000] Board has DDR1
[    0.000000] Analog PMU set to hw control
[    0.000000] Digital PMU set to hw control
[    0.000000] SoC Type: MediaTek MT7620N ver:2 eco:6
[    0.000000] bootconsole [early0] enabled
[    0.000000] CPU0 revision is: 00019650 (MIPS 24KEc)
[    0.000000] MIPS: machine is Nexx WT3020 (8M)
[    0.000000] Determined physical RAM map:
[    0.000000]  memory: 04000000 @ 00000000 (usable)
[    0.000000] Initrd not found or empty - disabling initrd
[    0.000000] Primary instruction cache 64kB, VIPT, 4-way, linesize 32 bytes.
[    0.000000] Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 bytes
[    0.000000] Zone ranges:
[    0.000000]   Normal   [mem 0x0000000000000000-0x0000000003ffffff]
[    0.000000] Movable zone start for each node
[    0.000000] Early memory node ranges
[    0.000000]   node   0: [mem 0x0000000000000000-0x0000000003ffffff]
[    0.000000] Initmem setup node 0 [mem 0x0000000000000000-0x0000000003ffffff]
[    0.000000] On node 0 totalpages: 16384
[    0.000000] free_area_init_node: node 0, pgdat 803af724, node_mem_map 81000020
[    0.000000]   Normal zone: 128 pages used for memmap
[    0.000000]   Normal zone: 0 pages reserved
[    0.000000]   Normal zone: 16384 pages, LIFO batch:3
[    0.000000] pcpu-alloc: s0 r0 d32768 u32768 alloc=1*32768
[    0.000000] pcpu-alloc: [0] 0 
[    0.000000] Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 16256
[    0.000000] Kernel command line: console=ttyS0,57600 rootfstype=squashfs,jffs2
[    0.000000] PID hash table entries: 256 (order: -2, 1024 bytes)
[    0.000000] Dentry cache hash table entries: 8192 (order: 3, 32768 bytes)
[    0.000000] Inode-cache hash table entries: 4096 (order: 2, 16384 bytes)
[    0.000000] Writing ErrCtl register=0003f290
[    0.000000] Readback ErrCtl register=0003f290
[    0.000000] Memory: 60708K/65536K available (2912K kernel code, 144K rwdata, 720K rodata, 184K init, 210K bss, 4828K reserved, 0K cma-reserved)
[    0.000000] SLUB: HWalign=32, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
[    0.000000] NR_IRQS:256
[    0.000000] CPU Clock: 580MHz
[    0.000000] clocksource: systick: mask: 0xffff max_cycles: 0xffff, max_idle_ns: 583261500 ns
[    0.000000] systick: enable autosleep mode
[    0.000000] systick: running - mult: 214748, shift: 32
[    0.000000] clocksource: MIPS: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 6590553264 ns
[    0.000011] sched_clock: 32 bits at 290MHz, resolution 3ns, wraps every 7405115902ns
[    0.015451] Calibrating delay loop... 385.84 BogoMIPS (lpj=1929216)
[    0.080614] pid_max: default: 32768 minimum: 301
[    0.089921] Mount-cache hash table entries: 1024 (order: 0, 4096 bytes)
[    0.102950] Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes)
[    0.122146] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns
[    0.141623] futex hash table entries: 256 (order: -1, 3072 bytes)
[    0.153770] pinctrl core: initialized pinctrl subsystem
[    0.164734] NET: Registered protocol family 16
[    0.193594] rt2880_gpio 10000600.gpio: registering 24 gpios
[    0.204618] rt2880_gpio 10000600.gpio: registering 24 irq handlers
[    0.217201] rt2880_gpio 10000660.gpio: registering 32 gpios
[    0.228189] rt2880_gpio 10000660.gpio: registering 32 irq handlers
[    0.240725] rt2880_gpio 10000688.gpio: registering 1 gpios
[    0.251533] rt2880_gpio 10000688.gpio: registering 1 irq handlers
[    0.267228] clocksource: Switched to clocksource systick
[    0.278917] NET: Registered protocol family 2
[    0.288380] TCP established hash table entries: 1024 (order: 0, 4096 bytes)
[    0.302151] TCP bind hash table entries: 1024 (order: 0, 4096 bytes)
[    0.314747] TCP: Hash tables configured (established 1024 bind 1024)
[    0.327466] UDP hash table entries: 256 (order: 0, 4096 bytes)
[    0.339003] UDP-Lite hash table entries: 256 (order: 0, 4096 bytes)
[    0.351666] NET: Registered protocol family 1
[    0.360231] PCI: CLS 0 bytes, default 32
[    0.362405] rt-timer 10000100.timer: maximum frequency is 1220Hz
[    0.375038] Crashlog allocated RAM at address 0x3f00000
[    0.386437] workingset: timestamp_bits=30 max_order=14 bucket_order=0
[    0.407190] squashfs: version 4.0 (2009/01/31) Phillip Lougher
[    0.418689] jffs2: version 2.2 (NAND) (SUMMARY) (LZMA) (RTIME) (CMODE_PRIORITY) (c) 2001-2006 Red Hat, Inc.
[    0.446316] io scheduler noop registered
[    0.454010] io scheduler deadline registered (default)
[    0.464396] ralink-usb-phy usbphy: invalid resource
[    0.474527] Serial: 8250/16550 driver, 2 ports, IRQ sharing disabled
[    0.488166] console [ttyS0] disabled
[    0.495153] 10000c00.uartlite: ttyS0 at MMIO 0x10000c00 (irq = 20, base_baud = 2500000) is a Palmchip BK-3103
[    0.514923] console [ttyS0] enabled
[    0.528706] bootconsole [early0] disabled
[    0.548747] spi spi0.0: force spi mode3
[    0.557103] m25p80 spi0.0: s25fl064k (8192 Kbytes)
[    0.566737] 4 ofpart partitions found on MTD device spi0.0
[    0.577668] Creating 4 MTD partitions on "spi0.0":
[    0.587225] 0x000000000000-0x000000030000 : "u-boot"
[    0.598728] 0x000000030000-0x000000040000 : "u-boot-env"
[    0.611161] 0x000000040000-0x000000050000 : "factory"
[    0.623060] 0x000000050000-0x000000800000 : "firmware"
[    0.679040] 2 uimage-fw partitions found on MTD device firmware
[    0.690883] 0x000000050000-0x000000185c89 : "kernel"
[    0.702349] 0x000000185c89-0x000000800000 : "rootfs"
[    0.714076] mtd: device 5 (rootfs) set to be root filesystem
[    0.725948] 1 squashfs-split partitions found on MTD device rootfs
[    0.738322] 0x000000500000-0x000000800000 : "rootfs_data"
[    0.751334] libphy: Fixed MDIO Bus: probed
[    0.763164] mtk_soc_eth 10100000.ethernet: loaded mt7620 driver
[    0.775813] mtk_soc_eth 10100000.ethernet eth0: mediatek frame engine at 0xb0100000, irq 5
[    0.792810] rt2880_wdt 10000120.watchdog: Initialized
[    0.803452] NET: Registered protocol family 17
[    0.812412] 8021q: 802.1Q VLAN Support v1.8
[    0.835948] VFS: Mounted root (squashfs filesystem) readonly on device 31:5.
[    0.851051] Freeing unused kernel memory: 184K
[    0.859933] This architecture does not have kernel memory protection.
[    2.552839] init: Console is alive
[    2.559905] init: - watchdog -
[    2.637187] random: fast init done
[    4.753882] kmodloader: loading kernel modules from /etc/modules-boot.d/*
[    5.026940] usbcore: registered new interface driver usbfs
[    5.038040] usbcore: registered new interface driver hub
[    5.048758] usbcore: registered new device driver usb
[    5.064401] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
[    5.079109] ehci-platform: EHCI generic platform driver
[    5.099868] phy phy-usbphy.0: remote usb device wakeup disabled
[    5.111672] phy phy-usbphy.0: UTMI 16bit 30MHz
[    5.120559] ehci-platform 101c0000.ehci: EHCI Host Controller
[    5.132052] ehci-platform 101c0000.ehci: new USB bus registered, assigned bus number 1
[    5.147963] ehci-platform 101c0000.ehci: irq 26, io mem 0x101c0000
[    5.169582] ehci-platform 101c0000.ehci: USB 2.0 started, EHCI 1.00
[    5.183115] hub 1-0:1.0: USB hub found
[    5.190973] hub 1-0:1.0: 1 port detected
[    5.201814] ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
[    5.215587] ohci-platform: OHCI generic platform driver
[    5.226319] ohci-platform 101c1000.ohci: Generic Platform OHCI controller
[    5.239919] ohci-platform 101c1000.ohci: new USB bus registered, assigned bus number 2
[    5.255809] ohci-platform 101c1000.ohci: irq 26, io mem 0x101c1000
[    5.294056] hub 2-0:1.0: USB hub found
[    5.301954] hub 2-0:1.0: 1 port detected
[    5.312495] kmodloader: done loading kernel modules from /etc/modules-boot.d/*
[    5.331112] init: - preinit -
[    7.291794] 8021q: adding VLAN 0 to HW filter on device eth0
[    8.713650] jffs2_scan_eraseblock(): End of filesystem marker found at 0x10000
[    8.728085] jffs2_build_filesystem(): unlocking the mtd device... 
[    8.740062] done.
[    8.743886] jffs2_build_filesystem(): erasing all blocks after the end marker... 
[   21.742699] done.
[   21.746576] jffs2: notice: (326) jffs2_build_xattr_subsystem: complete building xattr subsystem, 0 of xdatum (0 unchecked, 0 orphan) and 0 of xref (0 dead, 0 orphan) found.
[   21.779256] mount_root: overlay filesystem has not been fully initialized yet
[   21.813658] mount_root: switching to jffs2 overlay
[   22.273659] urandom-seed: Seed file not found (/etc/urandom.seed)
[   22.406324] procd: - early -
[   22.412943] procd: - watchdog -
[   22.687776] procd: - watchdog -
[   22.694401] procd: - ubus -
[   22.891447] procd: - init -
[   23.338422] kmodloader: loading kernel modules from /etc/modules.d/*
[   23.357273] Loading modules backported from Linux version wt-2017-11-01-0-gfe248fc2c180
[   23.373288] Backport generated by backports.git v4.14-rc2-1-31-g86cf0e5d
[   23.394531] nf_conntrack version 0.5.0 (1024 buckets, 4096 max)
[   23.463679] xt_time: kernel timezone is -0000
[   23.492833] ip_tables: (C) 2000-2006 Netfilter Core Team
[   23.553033] PPP generic driver version 2.4.2
[   23.564396] NET: Registered protocol family 24
[   23.631494] rt2800_wmac 10180000.wmac: loaded eeprom from mtd device "factory"
[   23.645960] ieee80211 phy0: rt2x00_set_rt: Info - RT chipset 6352, rev 0500 detected
[   23.661418] ieee80211 phy0: rt2x00_set_rf: Info - RF chipset 7620 detected
[   23.675814] ieee80211 phy0: Selected rate control algorithm 'minstrel_ht'
[   23.699557] usbcore: registered new interface driver rt2800usb
[   23.818150] usbcore: registered new interface driver ath9k_htc
[   23.831039] kmodloader: done loading kernel modules from /etc/modules.d/*
[   34.462869] 8021q: adding VLAN 0 to HW filter on device eth0
[   34.508019] br-lan: port 1(eth0.1) entered blocking state
[   34.518854] br-lan: port 1(eth0.1) entered disabled state
[   34.530017] device eth0.1 entered promiscuous mode
[   34.539600] device eth0 entered promiscuous mode
[   34.654634] br-lan: port 1(eth0.1) entered blocking state
[   34.665458] br-lan: port 1(eth0.1) entered forwarding state
[   40.437728] br-lan: port 1(eth0.1) entered disabled state
[   40.474772] br-lan: port 2(wlan0) entered blocking state
[   40.485438] br-lan: port 2(wlan0) entered disabled state
[   40.496446] device wlan0 entered promiscuous mode
[   41.113018] br-lan: port 2(wlan0) entered blocking state
[   41.123641] br-lan: port 2(wlan0) entered forwarding state
[   41.820846] br-lan: port 1(eth0.1) entered blocking state
[   41.831666] br-lan: port 1(eth0.1) entered forwarding state

[-- Attachment #3: BUILD2.dmesg.txt --]
[-- Type: text/plain, Size: 10797 bytes --]

[    0.000000] Linux version 4.9.73 (ubuntu@ubuntu) (gcc version 5.5.0 (OpenWrt GCC 5.5.0 r5682-b153dbf) ) #0 Fri Jan 5 10:59:59 2018
[    0.000000] Board has DDR1
[    0.000000] Analog PMU set to hw control
[    0.000000] Digital PMU set to hw control
[    0.000000] SoC Type: MediaTek MT7620N ver:2 eco:6
[    0.000000] bootconsole [early0] enabled
[    0.000000] CPU0 revision is: 00019650 (MIPS 24KEc)
[    0.000000] MIPS: machine is Nexx WT3020 (8M)
[    0.000000] Determined physical RAM map:
[    0.000000]  memory: 04000000 @ 00000000 (usable)
[    0.000000] Initrd not found or empty - disabling initrd
[    0.000000] Primary instruction cache 64kB, VIPT, 4-way, linesize 32 bytes.
[    0.000000] Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 bytes
[    0.000000] Zone ranges:
[    0.000000]   Normal   [mem 0x0000000000000000-0x0000000003ffffff]
[    0.000000] Movable zone start for each node
[    0.000000] Early memory node ranges
[    0.000000]   node   0: [mem 0x0000000000000000-0x0000000003ffffff]
[    0.000000] Initmem setup node 0 [mem 0x0000000000000000-0x0000000003ffffff]
[    0.000000] On node 0 totalpages: 16384
[    0.000000] free_area_init_node: node 0, pgdat 803af724, node_mem_map 81000020
[    0.000000]   Normal zone: 128 pages used for memmap
[    0.000000]   Normal zone: 0 pages reserved
[    0.000000]   Normal zone: 16384 pages, LIFO batch:3
[    0.000000] pcpu-alloc: s0 r0 d32768 u32768 alloc=1*32768
[    0.000000] pcpu-alloc: [0] 0 
[    0.000000] Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 16256
[    0.000000] Kernel command line: console=ttyS0,57600 rootfstype=squashfs,jffs2
[    0.000000] PID hash table entries: 256 (order: -2, 1024 bytes)
[    0.000000] Dentry cache hash table entries: 8192 (order: 3, 32768 bytes)
[    0.000000] Inode-cache hash table entries: 4096 (order: 2, 16384 bytes)
[    0.000000] Writing ErrCtl register=0003f290
[    0.000000] Readback ErrCtl register=0003f290
[    0.000000] Memory: 60708K/65536K available (2912K kernel code, 144K rwdata, 720K rodata, 184K init, 210K bss, 4828K reserved, 0K cma-reserved)
[    0.000000] SLUB: HWalign=32, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
[    0.000000] NR_IRQS:256
[    0.000000] CPU Clock: 580MHz
[    0.000000] clocksource: systick: mask: 0xffff max_cycles: 0xffff, max_idle_ns: 583261500 ns
[    0.000000] systick: enable autosleep mode
[    0.000000] systick: running - mult: 214748, shift: 32
[    0.000000] clocksource: MIPS: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 6590553264 ns
[    0.000011] sched_clock: 32 bits at 290MHz, resolution 3ns, wraps every 7405115902ns
[    0.015451] Calibrating delay loop... 385.84 BogoMIPS (lpj=1929216)
[    0.080607] pid_max: default: 32768 minimum: 301
[    0.089913] Mount-cache hash table entries: 1024 (order: 0, 4096 bytes)
[    0.102943] Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes)
[    0.122139] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns
[    0.141618] futex hash table entries: 256 (order: -1, 3072 bytes)
[    0.153763] pinctrl core: initialized pinctrl subsystem
[    0.164727] NET: Registered protocol family 16
[    0.193588] rt2880_gpio 10000600.gpio: registering 24 gpios
[    0.204611] rt2880_gpio 10000600.gpio: registering 24 irq handlers
[    0.217196] rt2880_gpio 10000660.gpio: registering 32 gpios
[    0.228184] rt2880_gpio 10000660.gpio: registering 32 irq handlers
[    0.240720] rt2880_gpio 10000688.gpio: registering 1 gpios
[    0.251528] rt2880_gpio 10000688.gpio: registering 1 irq handlers
[    0.267223] clocksource: Switched to clocksource systick
[    0.278913] NET: Registered protocol family 2
[    0.288376] TCP established hash table entries: 1024 (order: 0, 4096 bytes)
[    0.302147] TCP bind hash table entries: 1024 (order: 0, 4096 bytes)
[    0.314743] TCP: Hash tables configured (established 1024 bind 1024)
[    0.327462] UDP hash table entries: 256 (order: 0, 4096 bytes)
[    0.338999] UDP-Lite hash table entries: 256 (order: 0, 4096 bytes)
[    0.351662] NET: Registered protocol family 1
[    0.360227] PCI: CLS 0 bytes, default 32
[    0.362401] rt-timer 10000100.timer: maximum frequency is 1220Hz
[    0.375034] Crashlog allocated RAM at address 0x3f00000
[    0.386433] workingset: timestamp_bits=30 max_order=14 bucket_order=0
[    0.407187] squashfs: version 4.0 (2009/01/31) Phillip Lougher
[    0.418686] jffs2: version 2.2 (NAND) (SUMMARY) (LZMA) (RTIME) (CMODE_PRIORITY) (c) 2001-2006 Red Hat, Inc.
[    0.446313] io scheduler noop registered
[    0.454007] io scheduler deadline registered (default)
[    0.464392] ralink-usb-phy usbphy: invalid resource
[    0.474523] Serial: 8250/16550 driver, 2 ports, IRQ sharing disabled
[    0.488163] console [ttyS0] disabled
[    0.495150] 10000c00.uartlite: ttyS0 at MMIO 0x10000c00 (irq = 20, base_baud = 2500000) is a Palmchip BK-3103
[    0.514920] console [ttyS0] enabled
[    0.528703] bootconsole [early0] disabled
[    0.548743] spi spi0.0: force spi mode3
[    0.557098] m25p80 spi0.0: s25fl064k (8192 Kbytes)
[    0.566731] 4 ofpart partitions found on MTD device spi0.0
[    0.577662] Creating 4 MTD partitions on "spi0.0":
[    0.587219] 0x000000000000-0x000000030000 : "u-boot"
[    0.598721] 0x000000030000-0x000000040000 : "u-boot-env"
[    0.611154] 0x000000040000-0x000000050000 : "factory"
[    0.623053] 0x000000050000-0x000000800000 : "firmware"
[    0.679029] 2 uimage-fw partitions found on MTD device firmware
[    0.690873] 0x000000050000-0x000000185c89 : "kernel"
[    0.702340] 0x000000185c89-0x000000800000 : "rootfs"
[    0.714067] mtd: device 5 (rootfs) set to be root filesystem
[    0.725939] 1 squashfs-split partitions found on MTD device rootfs
[    0.738312] 0x000000500000-0x000000800000 : "rootfs_data"
[    0.751325] libphy: Fixed MDIO Bus: probed
[    0.763154] mtk_soc_eth 10100000.ethernet: loaded mt7620 driver
[    0.775803] mtk_soc_eth 10100000.ethernet eth0: mediatek frame engine at 0xb0100000, irq 5
[    0.792802] rt2880_wdt 10000120.watchdog: Initialized
[    0.803443] NET: Registered protocol family 17
[    0.812405] 8021q: 802.1Q VLAN Support v1.8
[    0.835941] VFS: Mounted root (squashfs filesystem) readonly on device 31:5.
[    0.851044] Freeing unused kernel memory: 184K
[    0.859926] This architecture does not have kernel memory protection.
[    2.552869] init: Console is alive
[    2.559936] init: - watchdog -
[    2.637179] random: fast init done
[    4.754118] kmodloader: loading kernel modules from /etc/modules-boot.d/*
[    5.027260] usbcore: registered new interface driver usbfs
[    5.038317] usbcore: registered new interface driver hub
[    5.049033] usbcore: registered new device driver usb
[    5.064638] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
[    5.079350] ehci-platform: EHCI generic platform driver
[    5.100108] phy phy-usbphy.0: remote usb device wakeup disabled
[    5.111911] phy phy-usbphy.0: UTMI 16bit 30MHz
[    5.120796] ehci-platform 101c0000.ehci: EHCI Host Controller
[    5.132289] ehci-platform 101c0000.ehci: new USB bus registered, assigned bus number 1
[    5.148201] ehci-platform 101c0000.ehci: irq 26, io mem 0x101c0000
[    5.169736] ehci-platform 101c0000.ehci: USB 2.0 started, EHCI 1.00
[    5.183270] hub 1-0:1.0: USB hub found
[    5.191126] hub 1-0:1.0: 1 port detected
[    5.201966] ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
[    5.215744] ohci-platform: OHCI generic platform driver
[    5.226473] ohci-platform 101c1000.ohci: Generic Platform OHCI controller
[    5.240074] ohci-platform 101c1000.ohci: new USB bus registered, assigned bus number 2
[    5.255964] ohci-platform 101c1000.ohci: irq 26, io mem 0x101c1000
[    5.294208] hub 2-0:1.0: USB hub found
[    5.302108] hub 2-0:1.0: 1 port detected
[    5.312644] kmodloader: done loading kernel modules from /etc/modules-boot.d/*
[    5.331276] init: - preinit -
[    7.294190] 8021q: adding VLAN 0 to HW filter on device eth0
[    8.720519] jffs2_scan_eraseblock(): End of filesystem marker found at 0x10000
[    8.734948] jffs2_build_filesystem(): unlocking the mtd device... 
[    8.746920] done.
[    8.750763] jffs2_build_filesystem(): erasing all blocks after the end marker... 
[   21.560944] done.
[   21.564823] jffs2: notice: (327) jffs2_build_xattr_subsystem: complete building xattr subsystem, 0 of xdatum (0 unchecked, 0 orphan) and 0 of xref (0 dead, 0 orphan) found.
[   21.597479] mount_root: overlay filesystem has not been fully initialized yet
[   21.631804] mount_root: switching to jffs2 overlay
[   22.092048] urandom-seed: Seed file not found (/etc/urandom.seed)
[   22.225140] procd: - early -
[   22.231057] procd: - watchdog -
[   22.506591] procd: - watchdog -
[   22.513116] procd: - ubus -
[   22.710100] procd: - init -
[   23.156989] kmodloader: loading kernel modules from /etc/modules.d/*
[   23.175863] Loading modules backported from Linux version wt-2017-11-01-0-gfe248fc2c180
[   23.191876] Backport generated by backports.git v4.14-rc2-1-31-g86cf0e5d
[   23.213051] nf_conntrack version 0.5.0 (1024 buckets, 4096 max)
[   23.284767] xt_time: kernel timezone is -0000
[   23.313913] ip_tables: (C) 2000-2006 Netfilter Core Team
[   23.374477] PPP generic driver version 2.4.2
[   23.385852] NET: Registered protocol family 24
[   23.452696] rt2800_wmac 10180000.wmac: loaded eeprom from mtd device "factory"
[   23.467162] ieee80211 phy0: rt2x00_set_rt: Info - RT chipset 6352, rev 0500 detected
[   23.482621] ieee80211 phy0: rt2x00_set_rf: Info - RF chipset 7620 detected
[   23.497016] ieee80211 phy0: Selected rate control algorithm 'minstrel_ht'
[   23.520822] usbcore: registered new interface driver rt2800usb
[   23.639371] usbcore: registered new interface driver ath9k_htc
[   23.652266] kmodloader: done loading kernel modules from /etc/modules.d/*
[   34.235987] 8021q: adding VLAN 0 to HW filter on device eth0
[   34.279728] br-lan: port 1(eth0.1) entered blocking state
[   34.290564] br-lan: port 1(eth0.1) entered disabled state
[   34.301732] device eth0.1 entered promiscuous mode
[   34.311321] device eth0 entered promiscuous mode
[   34.425063] br-lan: port 1(eth0.1) entered blocking state
[   34.435885] br-lan: port 1(eth0.1) entered forwarding state
[   40.235344] br-lan: port 1(eth0.1) entered disabled state
[   40.266543] br-lan: port 2(wlan0) entered blocking state
[   40.277215] br-lan: port 2(wlan0) entered disabled state
[   40.288224] device wlan0 entered promiscuous mode
[   40.910514] br-lan: port 2(wlan0) entered blocking state
[   40.921138] br-lan: port 2(wlan0) entered forwarding state
[   41.780483] br-lan: port 1(eth0.1) entered blocking state
[   41.791304] br-lan: port 1(eth0.1) entered forwarding state

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Tom Psyborg]

On 16/10/2018, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> Hello
>
> On Tue, Oct 16, 2018 at 01:32:18PM +0200, Tom Psyborg wrote:
>> I am sending you two builds privately so please check if there are any
>> differences between the two builds and report back. Thanks.
>
> I extracted rt2800lib.ko module from provided images, did disassembly via:
>
> ./staging_dir/toolchain-mipsel_24kc_gcc-7.3.0_musl/bin/mipsel-openwrt-linux-objdump
> \
>  -d -r --prefix-addresses ~/rt2800lib-BUILDn.ko  > ~/BUILDn.dump.txt
>
> command and compered disassembled code. Here is difference:
>
> $ diff -up  BUILD1.dump.txt BUILD2.dump.txt
> --- BUILD1.dump.txt	2018-10-16 16:40:34.834220838 +0200
> +++ BUILD2.dump.txt	2018-10-16 16:40:40.187219211 +0200
> @@ -1,5 +1,5 @@
>
> -/home/stasiu/rt2800lib-BUILD1.ko:     file format elf32-tradlittlemips
> +/home/stasiu/rt2800lib-BUILD2.ko:     file format elf32-tradlittlemips
>
>
>  Disassembly of section .text:
> @@ -9374,7 +9374,7 @@ Disassembly of section .text:
>  00007f80 <rt2800_clear_beacon+0x224> jalr	v0
>  00007f84 <rt2800_clear_beacon+0x228> move	a0,s0
>  00007f88 <rt2800_clear_beacon+0x22c> lhu	v1,732(s0)
> -00007f8c <rt2800_clear_beacon+0x230> li	v0,21392
> +00007f8c <rt2800_clear_beacon+0x230> li	v0,25426
>  00007f90 <rt2800_clear_beacon+0x234> bne	v1,v0,0000810c
> <rt2800_clear_beacon+0x3b0>
>  00007f94 <rt2800_clear_beacon+0x238> li	a2,1025
>  00007f98 <rt2800_clear_beacon+0x23c> lw	v0,4(s0)
>
> There is no difference in init_registers (which is inlined in
> rt2800_enable_radio). The only difference is in some number
> rt2800_clear_beacon() function.
>
> Regards
> Stanislaw
>

hi

i rechecked this and your debug procedure seems to be unreliable.

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Stanislaw Gruszka]

On Fri, Oct 12, 2018 at 12:48:07PM +0200, Tom Psyborg wrote:
> chip version support exist in daniel's tree since a long time ago. so
> don't disable registers initialization but try to upstream his
> changes.

Where is this patch ? I can not find it.

Thanks
Stanislaw

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Daniel Golle]

On Tue, Oct 16, 2018 at 10:11:16AM +0200, Stanislaw Gruszka wrote:
> On Fri, Oct 12, 2018 at 12:48:07PM +0200, Tom Psyborg wrote:
> > chip version support exist in daniel's tree since a long time ago. so
> > don't disable registers initialization but try to upstream his
> > changes.
> 
> Where is this patch ? I can not find it.

So this requires to make the chip version and package available to
drivers like rt2x00. First of all, this is a patch for linux-mips:

https://git.openwrt.org/?p=openwrt/staging/dangole.git;a=blob;f=target/linux/ramips/patches-4.4/300-mt7620-export-chip-version-and-pkg.patch;h=f6aca6c90516f9c534b3c51e9f99dff6a3f41b75;hb=709fe05dfea58728d6accb9fe56c7056d9d0715b

It belongs to this (very outdated) tree:
https://git.openwrt.org/?p=openwrt/staging/dangole.git;a=shortlog;h=refs/heads/differentiate-pkg-ver-eco

I'm not sure whether this is the right way to do this, but it worked.


Cheers


Daniel



> 
> Thanks
> Stanislaw

[PATCH] mips: ralink: add accessors for MT7620 chipver and pkg

[Daniel Golle]

The RT6352 wireless core included in all MT7620 chips is implemented
differently in MT7620A (TFBGA) and MT7620N (DR-QFN).
Hence provide accessor functions similar to the already existing
mt7620_get_eco() function which allow the rt2x00 wireless driver to
figure out which WiSoC it is being run on.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 arch/mips/include/asm/mach-ralink/mt7620.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/mips/include/asm/mach-ralink/mt7620.h b/arch/mips/include/asm/mach-ralink/mt7620.h
index 66af4ccb5c6c..d0310a92a63f 100644
--- a/arch/mips/include/asm/mach-ralink/mt7620.h
+++ b/arch/mips/include/asm/mach-ralink/mt7620.h
@@ -137,4 +137,16 @@ static inline int mt7620_get_eco(void)
 	return rt_sysc_r32(SYSC_REG_CHIP_REV) & CHIP_REV_ECO_MASK;
 }
 
+static inline int mt7620_get_chipver(void)
+{
+	return (rt_sysc_r32(SYSC_REG_CHIP_REV) >> CHIP_REV_VER_SHIFT) &
+		CHIP_REV_VER_MASK;
+}
+
+static inline int mt7620_get_pkg(void)
+{
+	return (rt_sysc_r32(SYSC_REG_CHIP_REV) >> CHIP_REV_PKG_SHIFT) &
+		CHIP_REV_PKG_MASK;
+}
+
 #endif
-- 
2.19.1

Re: [PATCH] mips: ralink: add accessors for MT7620 chipver and pkg

[Paul Burton]

Hi Daniel,

On Fri, Nov 02, 2018 at 03:07:19AM +0100, Daniel Golle wrote:
> The RT6352 wireless core included in all MT7620 chips is implemented
> differently in MT7620A (TFBGA) and MT7620N (DR-QFN).
> Hence provide accessor functions similar to the already existing
> mt7620_get_eco() function which allow the rt2x00 wireless driver to
> figure out which WiSoC it is being run on.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>  arch/mips/include/asm/mach-ralink/mt7620.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/mips/include/asm/mach-ralink/mt7620.h b/arch/mips/include/asm/mach-ralink/mt7620.h
> index 66af4ccb5c6c..d0310a92a63f 100644
> --- a/arch/mips/include/asm/mach-ralink/mt7620.h
> +++ b/arch/mips/include/asm/mach-ralink/mt7620.h
> @@ -137,4 +137,16 @@ static inline int mt7620_get_eco(void)
>  	return rt_sysc_r32(SYSC_REG_CHIP_REV) & CHIP_REV_ECO_MASK;
>  }
>  
> +static inline int mt7620_get_chipver(void)
> +{
> +	return (rt_sysc_r32(SYSC_REG_CHIP_REV) >> CHIP_REV_VER_SHIFT) &
> +		CHIP_REV_VER_MASK;
> +}
> +
> +static inline int mt7620_get_pkg(void)
> +{
> +	return (rt_sysc_r32(SYSC_REG_CHIP_REV) >> CHIP_REV_PKG_SHIFT) &
> +		CHIP_REV_PKG_MASK;
> +}
> +
>  #endif

Is there an in-tree user for these?

Looking at it I don't see any in-tree code which uses the existing
mt7620_get_eco() function. I'm not fond of adding code which isn't used
at all in-tree, I'd much rather we either:

 1) Get the driver that needs these upstreamed, and these functions
    could be added at the same time.

or

 2) Keep functions only used by out-of-tree code out-of-tree.

Thanks,
    Paul

Re: [PATCH] mips: ralink: add accessors for MT7620 chipver and pkg

[Daniel Golle]

Hi Paul,

thank you for the review!

On Mon, Nov 05, 2018 at 06:36:16PM +0000, Paul Burton wrote:
> Hi Daniel,
> 
> On Fri, Nov 02, 2018 at 03:07:19AM +0100, Daniel Golle wrote:
> > The RT6352 wireless core included in all MT7620 chips is implemented
> > differently in MT7620A (TFBGA) and MT7620N (DR-QFN).
> > Hence provide accessor functions similar to the already existing
> > mt7620_get_eco() function which allow the rt2x00 wireless driver to
> > figure out which WiSoC it is being run on.
> > 
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> >  arch/mips/include/asm/mach-ralink/mt7620.h | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/arch/mips/include/asm/mach-ralink/mt7620.h b/arch/mips/include/asm/mach-ralink/mt7620.h
> > index 66af4ccb5c6c..d0310a92a63f 100644
> > --- a/arch/mips/include/asm/mach-ralink/mt7620.h
> > +++ b/arch/mips/include/asm/mach-ralink/mt7620.h
> > @@ -137,4 +137,16 @@ static inline int mt7620_get_eco(void)
> >  	return rt_sysc_r32(SYSC_REG_CHIP_REV) & CHIP_REV_ECO_MASK;
> >  }
> >  
> > +static inline int mt7620_get_chipver(void)
> > +{
> > +	return (rt_sysc_r32(SYSC_REG_CHIP_REV) >> CHIP_REV_VER_SHIFT) &
> > +		CHIP_REV_VER_MASK;
> > +}
> > +
> > +static inline int mt7620_get_pkg(void)
> > +{
> > +	return (rt_sysc_r32(SYSC_REG_CHIP_REV) >> CHIP_REV_PKG_SHIFT) &
> > +		CHIP_REV_PKG_MASK;
> > +}
> > +
> >  #endif
> 
> Is there an in-tree user for these?

Not yet, OpenWrt's out-of-tree Ethernet driver needs the already
existing int mt7620_get_eco(void) and is going to be upstreamed once
MT7530 DSA for has been completed to work with MT7621. See the driver
in [1].

The two newly introduced accessors are going to be used by the in-tree
rt2x00 driver which gained support for the RT6352 wireless core
included in that SoC recently. In order to be able to carry out tuning
in the same way the vendor driver does, rt2x00 will need to access the
pkg and chipver fields. See [2] for example.

> 
> Looking at it I don't see any in-tree code which uses the existing
> mt7620_get_eco() function. I'm not fond of adding code which isn't used
> at all in-tree, I'd much rather we either:
> 
>  1) Get the driver that needs these upstreamed, and these functions
>     could be added at the same time.
> 
> or
> 
>  2) Keep functions only used by out-of-tree code out-of-tree.

I understand your concerns with regard to the mt7620_get_eco(void)
function which is currently only used by the out-of-tree Ethernet
driver. However, the to-be-introduced functions mt7620_get_pkg(void)
and mt7620_get_chipver(void) are to be used in-tree by
drivers/net/wireless/ralink/rt2x00 in the very near future. I just
wanted to consult whether the introductions of such accessors is
generally acceptable before implementing the changes in rt2x00.


Best regards


Daniel


[1]: git://git.openwrt.org/openwrt.git:/target/linux/ramips/files-4.14/drivers/net/ethernet/mediatek/soc_mt7620.c

[2]: https://github.com/i80s/mtk-sources/blob/master/mt7620/src/chips/rt6352.c#L1019

> 
> Thanks,
>     Paul

Re: [PATCH] mips: ralink: add accessors for MT7620 chipver and pkg

[Paul Burton]

Hi Daniel,

On Mon, Nov 05, 2018 at 09:19:35PM +0100, Daniel Golle wrote:
> Hi Paul,
> 
> thank you for the review!

You're very welcome :)

> On Mon, Nov 05, 2018 at 06:36:16PM +0000, Paul Burton wrote:
> > Is there an in-tree user for these?
> 
> Not yet, OpenWrt's out-of-tree Ethernet driver needs the already
> existing int mt7620_get_eco(void) and is going to be upstreamed once
> MT7530 DSA for has been completed to work with MT7621. See the driver
> in [1].
> 
> The two newly introduced accessors are going to be used by the in-tree
> rt2x00 driver which gained support for the RT6352 wireless core
> included in that SoC recently. In order to be able to carry out tuning
> in the same way the vendor driver does, rt2x00 will need to access the
> pkg and chipver fields. See [2] for example.
> 
> > 
> > Looking at it I don't see any in-tree code which uses the existing
> > mt7620_get_eco() function. I'm not fond of adding code which isn't used
> > at all in-tree, I'd much rather we either:
> > 
> >  1) Get the driver that needs these upstreamed, and these functions
> >     could be added at the same time.
> > 
> > or
> > 
> >  2) Keep functions only used by out-of-tree code out-of-tree.
> 
> I understand your concerns with regard to the mt7620_get_eco(void)
> function which is currently only used by the out-of-tree Ethernet
> driver. However, the to-be-introduced functions mt7620_get_pkg(void)
> and mt7620_get_chipver(void) are to be used in-tree by
> drivers/net/wireless/ralink/rt2x00 in the very near future. I just
> wanted to consult whether the introductions of such accessors is
> generally acceptable before implementing the changes in rt2x00.

OK, then my suggestion is that you include this patch in the rt2x00
series you create & feel free to add:

    Acked-by: Paul Burton <paul.burton@mips.com>

Presuming that the functions are used by something else in the patch
series.

Does that sound OK to you?

Thanks,
    Paul

Re: [PATCH] mips: ralink: add accessors for MT7620 chipver and pkg

[Tom Psyborg]

Hi

ACK from me on this one. Tested and working.

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Kalle Valo]

Stanislaw Gruszka <sgruszka@redhat.com> writes:

> There is duplicated 'if (rt2x00_rt(rt2x00dev, RT6352))' entry that
> causes we do not perform register initialization for RT6352 (MT7620
> SOCs) in correct branch. Fix this and disable registers initialization
> that is specific to particular MT7620 revision.
>
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>

[...]

> @@ -5466,6 +5465,10 @@ static int rt2800_init_registers(struct rt2x00_dev *rt2x00dev)
>  		rt2800_register_write(rt2x00dev, TX_SW_CFG0, 0x00000401);
>  		rt2800_register_write(rt2x00dev, TX_SW_CFG1, 0x000C0000);
>  		rt2800_register_write(rt2x00dev, TX_SW_CFG2, 0x000C0408);
> +		/* TODO add chip version support and init registers
> +		 * according to the version.
> +		 */
> +#if 0
>  		rt2800_register_write(rt2x00dev, MIMO_PS_CFG, 0x00000002);
>  		rt2800_register_write(rt2x00dev, TX_PIN_CFG, 0x00150F0F);
>  		rt2800_register_write(rt2x00dev, TX_ALC_VGA3, 0x06060606);
> @@ -5480,6 +5483,7 @@ static int rt2800_init_registers(struct rt2x00_dev *rt2x00dev)
>  		reg = rt2800_register_read(rt2x00dev, TX_ALC_CFG_1);
>  		rt2x00_set_field32(&reg, TX_ALC_CFG_1_ROS_BUSY_EN, 0);
>  		rt2800_register_write(rt2x00dev, TX_ALC_CFG_1, reg);
> +#endif

No '#if 0', please. If the code is not needed you can remove it, it's
available from git history anyway if it's needed later.

-- 
Kalle Valo

Re: [PATCH v4 4/8] rt2800: fix registers init for MT7620

[Stanislaw Gruszka]

On Sat, Oct 13, 2018 at 12:46:54PM +0300, Kalle Valo wrote:
> No '#if 0', please. If the code is not needed you can remove it, it's
> available from git history anyway if it's needed later.

Plase drop this patch, other patches from the set can be applied
without it.

Thanks
Stanislaw

[PATCH v4 5/8] rt2800: enable TX_PIN_CFG_LNA_PE_ bits per band

[Stanislaw Gruszka]

Do not enable TX_PIN_CFG_LNA_PE_A* bits for 2.4GHz band and
vice versa TX_PIN_CFG_LNA_PE_G* bits for 5GHz.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index 16d6d99b1d44..bf0d12c5b2db 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -3891,18 +3891,24 @@ static void rt2800_config_channel(struct rt2x00_dev *rt2x00dev,
 	switch (rt2x00dev->default_ant.rx_chain_num) {
 	case 3:
 		/* Turn on tertiary LNAs */
-		rt2x00_set_field32(&tx_pin, TX_PIN_CFG_LNA_PE_A2_EN, 1);
-		rt2x00_set_field32(&tx_pin, TX_PIN_CFG_LNA_PE_G2_EN, 1);
+		rt2x00_set_field32(&tx_pin, TX_PIN_CFG_LNA_PE_A2_EN,
+				   rf->channel > 14);
+		rt2x00_set_field32(&tx_pin, TX_PIN_CFG_LNA_PE_G2_EN,
+				   rf->channel <= 14);
 		/* fall-through */
 	case 2:
 		/* Turn on secondary LNAs */
-		rt2x00_set_field32(&tx_pin, TX_PIN_CFG_LNA_PE_A1_EN, 1);
-		rt2x00_set_field32(&tx_pin, TX_PIN_CFG_LNA_PE_G1_EN, 1);
+		rt2x00_set_field32(&tx_pin, TX_PIN_CFG_LNA_PE_A1_EN,
+				   rf->channel > 14);
+		rt2x00_set_field32(&tx_pin, TX_PIN_CFG_LNA_PE_G1_EN,
+				   rf->channel <= 14);
 		/* fall-through */
 	case 1:
 		/* Turn on primary LNAs */
-		rt2x00_set_field32(&tx_pin, TX_PIN_CFG_LNA_PE_A0_EN, 1);
-		rt2x00_set_field32(&tx_pin, TX_PIN_CFG_LNA_PE_G0_EN, 1);
+		rt2x00_set_field32(&tx_pin, TX_PIN_CFG_LNA_PE_A0_EN,
+				   rf->channel > 14);
+		rt2x00_set_field32(&tx_pin, TX_PIN_CFG_LNA_PE_G0_EN,
+				   rf->channel <= 14);
 		break;
 	}
 
-- 
2.7.5

[PATCH v4 6/8] rt2800: enable TX_PIN_CFG_RFRX_EN only for MT7620

[Stanislaw Gruszka]

The TX_PIN_CFG_RFRX_EN bit was not set on other devices than MT7620,
restore old behavaviour since setting this bit maight not be
correct for older devices.

Fixes: 41977e86c984 ("rt2x00: add support for MT7620")
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index bf0d12c5b2db..d0af0d9d2550 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -3856,10 +3856,12 @@ static void rt2800_config_channel(struct rt2x00_dev *rt2x00dev,
 	if (rt2x00_rt(rt2x00dev, RT3572))
 		rt2800_rfcsr_write(rt2x00dev, 8, 0);
 
-	if (rt2x00_rt(rt2x00dev, RT6352))
+	if (rt2x00_rt(rt2x00dev, RT6352)) {
 		tx_pin = rt2800_register_read(rt2x00dev, TX_PIN_CFG);
-	else
+		rt2x00_set_field32(&tx_pin, TX_PIN_CFG_RFRX_EN, 1);
+	} else {
 		tx_pin = 0;
+	}
 
 	switch (rt2x00dev->default_ant.tx_chain_num) {
 	case 3:
@@ -3914,7 +3916,6 @@ static void rt2800_config_channel(struct rt2x00_dev *rt2x00dev,
 
 	rt2x00_set_field32(&tx_pin, TX_PIN_CFG_RFTR_EN, 1);
 	rt2x00_set_field32(&tx_pin, TX_PIN_CFG_TRSW_EN, 1);
-	rt2x00_set_field32(&tx_pin, TX_PIN_CFG_RFRX_EN, 1); /* mt7620 */
 
 	rt2800_register_write(rt2x00dev, TX_PIN_CFG, tx_pin);
 
-- 
2.7.5

Re: [PATCH v4 6/8] rt2800: enable TX_PIN_CFG_RFRX_EN only for MT7620

[Tom Psyborg]

is there some specific reason to read TX_PIN_CFG register on RT6352,
rather than just null it before programming in tx values like in other
chips?

On 12/10/2018, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> The TX_PIN_CFG_RFRX_EN bit was not set on other devices than MT7620,
> restore old behavaviour since setting this bit maight not be
> correct for older devices.
>
> Fixes: 41977e86c984 ("rt2x00: add support for MT7620")
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> index bf0d12c5b2db..d0af0d9d2550 100644
> --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> @@ -3856,10 +3856,12 @@ static void rt2800_config_channel(struct rt2x00_dev
> *rt2x00dev,
>  	if (rt2x00_rt(rt2x00dev, RT3572))
>  		rt2800_rfcsr_write(rt2x00dev, 8, 0);
>
> -	if (rt2x00_rt(rt2x00dev, RT6352))
> +	if (rt2x00_rt(rt2x00dev, RT6352)) {
>  		tx_pin = rt2800_register_read(rt2x00dev, TX_PIN_CFG);
> -	else
> +		rt2x00_set_field32(&tx_pin, TX_PIN_CFG_RFRX_EN, 1);
> +	} else {
>  		tx_pin = 0;
> +	}
>
>  	switch (rt2x00dev->default_ant.tx_chain_num) {
>  	case 3:
> @@ -3914,7 +3916,6 @@ static void rt2800_config_channel(struct rt2x00_dev
> *rt2x00dev,
>
>  	rt2x00_set_field32(&tx_pin, TX_PIN_CFG_RFTR_EN, 1);
>  	rt2x00_set_field32(&tx_pin, TX_PIN_CFG_TRSW_EN, 1);
> -	rt2x00_set_field32(&tx_pin, TX_PIN_CFG_RFRX_EN, 1); /* mt7620 */
>
>  	rt2800_register_write(rt2x00dev, TX_PIN_CFG, tx_pin);
>
> --
> 2.7.5
>
>

Re: [PATCH v4 6/8] rt2800: enable TX_PIN_CFG_RFRX_EN only for MT7620

[Stanislaw Gruszka]

On Fri, Oct 12, 2018 at 12:52:13PM +0200, Tom Psyborg wrote:
> is there some specific reason to read TX_PIN_CFG register on RT6352,
> rather than just null it before programming in tx values like in other
> chips?

I don't remember the details, but Daniel explained that tere are 
some bits in the TX_PIN_CFG register that have to be preserved
for RT6352.

Regards
Stanislaw

[PATCH v4 7/8] rt2800: remove unneeded RT6352 check

[Stanislaw Gruszka]

Remove rt2x00_rt(rt2x00dev, RT6352)) check from
rt2800_config_channel_rf53xx() which is not called for RT6352 devices.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index d0af0d9d2550..ba7470897f3c 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -3037,8 +3037,7 @@ static void rt2800_config_channel_rf53xx(struct rt2x00_dev *rt2x00dev,
 			rt2800_rfcsr_write(rt2x00dev, 59,
 					   r59_nonbt_rev[idx]);
 		} else if (rt2x00_rt(rt2x00dev, RT5390) ||
-			   rt2x00_rt(rt2x00dev, RT5392) ||
-			   rt2x00_rt(rt2x00dev, RT6352)) {
+			   rt2x00_rt(rt2x00dev, RT5392)) {
 			static const char r59_non_bt[] = {0x8f, 0x8f,
 				0x8f, 0x8f, 0x8f, 0x8f, 0x8f, 0x8d,
 				0x8a, 0x88, 0x88, 0x87, 0x87, 0x86};
-- 
2.7.5

[PATCH v4 8/8] rt2800: comment and simplify AGC init for RT6352

[Stanislaw Gruszka]

We do not need separate lines for calculating register values.
Also add comment that value is different than in vendor driver.

Suggested-by: Daniel Golle <daniel@makrotopia.org>
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index ba7470897f3c..29b08d63e95b 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -3986,9 +3986,12 @@ static void rt2800_config_channel(struct rt2x00_dev *rt2x00dev,
 		rt2800_bbp_write(rt2x00dev, 195, 141);
 		rt2800_bbp_write(rt2x00dev, 196, reg);
 
-		/* AGC init */
-		reg = rf->channel <= 14 ? 0x1c : 0x24;
-		reg += 2 * rt2x00dev->lna_gain;
+		/* AGC init.
+		 * Despite the vendor driver using different values here for
+		 * RT6352 chip, we use 0x1c for now. This may have to be changed
+		 * once TSSI got implemented.
+		 */
+		reg = (rf->channel <= 14 ? 0x1c : 0x24) + 2*rt2x00dev->lna_gain;
 		rt2800_bbp_write_with_rx_chain(rt2x00dev, 66, reg);
 
 		rt2800_iq_calibrate(rt2x00dev, rf->channel);
-- 
2.7.5