diff mbox

[v2,13/14] rt2x00: rt2800lib: add support for RT3352 with 20MHz crystal

Message ID 20170116031541.GA32313@makrotopia.org (mailing list archive)
State Superseded
Delegated to: Kalle Valo
Headers show

Commit Message

Daniel Golle Jan. 16, 2017, 3:15 a.m. UTC 
Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
Signed-off-by: Mathias Kresin <dev@kresin.me>
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 50 +++++++++++++++++++++++++-
 drivers/net/wireless/ralink/rt2x00/rt2x00.h    |  2 ++
 2 files changed, 51 insertions(+), 1 deletion(-)

Comments

Stanislaw Gruszka Jan. 18, 2017, 2:30 p.m. UTC | #1 
On Mon, Jan 16, 2017 at 04:15:56AM +0100, Daniel Golle wrote:
> Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
> Signed-off-by: Mathias Kresin <dev@kresin.me>
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>  drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 50 +++++++++++++++++++++++++-
>  drivers/net/wireless/ralink/rt2x00/rt2x00.h    |  2 ++
>  2 files changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> index 93c97eade334..cb1457595f05 100644
> --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> @@ -36,6 +36,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
> +#include <linux/clk.h>
>  
>  #include "rt2x00.h"
>  #include "rt2800lib.h"
> @@ -7675,6 +7676,27 @@ static const struct rf_channel rf_vals_5592_xtal40[] = {
>  	{196, 83, 0, 12, 1},
>  };
>  
> +/*
> + * RF value list for rt3xxx with Xtal20MHz
> + * Supports: 2.4 GHz (all) (RF3322)
> + */
> +static const struct rf_channel rf_vals_xtal20mhz_3x[] = {
Please locate this values in alphabetical order (i.e. after _3x and 
before _5592 ).

>  	struct hw_mode_spec *spec = &rt2x00dev->spec;
> @@ -7764,7 +7786,10 @@ static int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
>  	case RF5390:
>  	case RF5392:
>  		spec->num_channels = 14;
> -		spec->channels = rf_vals_3x;
> +		if (spec->clk_is_20mhz)
> +			spec->channels = rf_vals_xtal20mhz_3x;
> +		else
> +			spec->channels = rf_vals_3x;
>  		break;

How does vendor drivers recognize xtal (I assume rf_vals_xtal20mhz_3x 
values were taken from vendor driver) ? It should be possible to get
clock frequency from device register like is is done on RF5592, without
adding additional clock recognition code. But if such code is needed
I prefer that low level board/platform routines do it and place clock
frequency for rt2x00 in rt2x00dev->dev->platform_data.

Stanislaw
Daniel Golle Jan. 19, 2017, 1:30 p.m. UTC | #2 
Hi Stanislaw,

On Wed, Jan 18, 2017 at 03:30:02PM +0100, Stanislaw Gruszka wrote:
> On Mon, Jan 16, 2017 at 04:15:56AM +0100, Daniel Golle wrote:
> > Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
> > Signed-off-by: Mathias Kresin <dev@kresin.me>
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> >  drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 50 +++++++++++++++++++++++++-
> >  drivers/net/wireless/ralink/rt2x00/rt2x00.h    |  2 ++
> >  2 files changed, 51 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> > index 93c97eade334..cb1457595f05 100644
> > --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> > +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> > @@ -36,6 +36,7 @@
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> >  #include <linux/slab.h>
> > +#include <linux/clk.h>
> >  
> >  #include "rt2x00.h"
> >  #include "rt2800lib.h"
> > @@ -7675,6 +7676,27 @@ static const struct rf_channel rf_vals_5592_xtal40[] = {
> >  	{196, 83, 0, 12, 1},
> >  };
> >  
> > +/*
> > + * RF value list for rt3xxx with Xtal20MHz
> > + * Supports: 2.4 GHz (all) (RF3322)
> > + */
> > +static const struct rf_channel rf_vals_xtal20mhz_3x[] = {
> Please locate this values in alphabetical order (i.e. after _3x and 
> before _5592 ).

Sure, sorry, that ended up in the wrong order when rebase the patches.

> 
> >  	struct hw_mode_spec *spec = &rt2x00dev->spec;
> > @@ -7764,7 +7786,10 @@ static int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
> >  	case RF5390:
> >  	case RF5392:
> >  		spec->num_channels = 14;
> > -		spec->channels = rf_vals_3x;
> > +		if (spec->clk_is_20mhz)
> > +			spec->channels = rf_vals_xtal20mhz_3x;
> > +		else
> > +			spec->channels = rf_vals_3x;
> >  		break;
> 
> How does vendor drivers recognize xtal (I assume rf_vals_xtal20mhz_3x 
> values were taken from vendor driver) ? It should be possible to get
> clock frequency from device register like is is done on RF5592, without
> adding additional clock recognition code. But if such code is needed
> I prefer that low level board/platform routines do it and place clock
> frequency for rt2x00 in rt2x00dev->dev->platform_data.

Recent vendor drivers probe the clock by reading a SYSCTL register:
---
// Programming channel parameters
Value = (*((volatile u32 *)(RALINK_SYSCTL_BASE + 0x10)));
if(Value & (1<<20)) { //Xtal=40M
	RT30xxWriteRFRegister(pAd, RF_R08, FreqItems3020[index].N);
	RT30xxWriteRFRegister(pAd, RF_R09, FreqItems3020[index].K);
}else {
	RT30xxWriteRFRegister(pAd, RF_R08, FreqItems3020_Xtal20M[index].N);
	RT30xxWriteRFRegister(pAd, RF_R09, FreqItems3020_Xtal20M[index].K);
}
---

From what I can see, most other drivers which need to touch SYSCTL
currently do that by defining a local precompiler macro:
---
#ifdef CONFIG_SOC_MT7621
#define RALINK_SYSCTL_BASE             0xbe000000
#else
#define RALINK_SYSCTL_BASE             0xb0000000
#endif
---

That's obviously not very elegant and probably we should define SYSCTL
in the device tree of each SoC and we should write/adapt a syscon mfd
driver which other drivers may then use to read/write stuff to/from
SYSCTL. The clock could then be provided by a clk driver sitting on top
of that and rt2x00 would use that clock.
In the meantime, why not just define a static clock in the device-tree
and already have rt2x00 consume that clock? That would already be the
way things will most likely look like from rt2x00 point of view once
syscon and clk drivers are in place.


Cheers


Daniel
Daniel Golle Jan. 19, 2017, 8:52 p.m. UTC | #3 
On Thu, Jan 19, 2017 at 02:30:14PM +0100, Daniel Golle wrote:
> Hi Stanislaw,
> 
> On Wed, Jan 18, 2017 at 03:30:02PM +0100, Stanislaw Gruszka wrote:
> > On Mon, Jan 16, 2017 at 04:15:56AM +0100, Daniel Golle wrote:
> > > Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
> > > Signed-off-by: Mathias Kresin <dev@kresin.me>
> > > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > > ---
> > >  drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 50 +++++++++++++++++++++++++-
> > >  drivers/net/wireless/ralink/rt2x00/rt2x00.h    |  2 ++
> > >  2 files changed, 51 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> > > index 93c97eade334..cb1457595f05 100644
> > > --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> > > +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> > > @@ -36,6 +36,7 @@
> > >  #include <linux/kernel.h>
> > >  #include <linux/module.h>
> > >  #include <linux/slab.h>
> > > +#include <linux/clk.h>
> > >  
> > >  #include "rt2x00.h"
> > >  #include "rt2800lib.h"
> > > @@ -7675,6 +7676,27 @@ static const struct rf_channel rf_vals_5592_xtal40[] = {
> > >  	{196, 83, 0, 12, 1},
> > >  };
> > >  
> > > +/*
> > > + * RF value list for rt3xxx with Xtal20MHz
> > > + * Supports: 2.4 GHz (all) (RF3322)
> > > + */
> > > +static const struct rf_channel rf_vals_xtal20mhz_3x[] = {
> > Please locate this values in alphabetical order (i.e. after _3x and 
> > before _5592 ).
> 
> Sure, sorry, that ended up in the wrong order when rebase the patches.
> 
> > 
> > >  	struct hw_mode_spec *spec = &rt2x00dev->spec;
> > > @@ -7764,7 +7786,10 @@ static int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
> > >  	case RF5390:
> > >  	case RF5392:
> > >  		spec->num_channels = 14;
> > > -		spec->channels = rf_vals_3x;
> > > +		if (spec->clk_is_20mhz)
> > > +			spec->channels = rf_vals_xtal20mhz_3x;
> > > +		else
> > > +			spec->channels = rf_vals_3x;
> > >  		break;
> > 
> > How does vendor drivers recognize xtal (I assume rf_vals_xtal20mhz_3x 
> > values were taken from vendor driver) ? It should be possible to get
> > clock frequency from device register like is is done on RF5592, without
> > adding additional clock recognition code. But if such code is needed
> > I prefer that low level board/platform routines do it and place clock
> > frequency for rt2x00 in rt2x00dev->dev->platform_data.

I researched and found this has already been implemented in the ramips
platform code, see

https://git.kernel.org/cgit/linux/kernel/git/kvalo/wireless-drivers-next.git/tree/arch/mips/ralink/rt305x.c#n194

The patch submitted uses this existing infrastructure which *does*
auto-probe the clock from the SoC's SYSCTRL register.
I'll re-submit a v3 with the alphabetic order above fixed, ok?


Cheers


Daniel
diff mbox

Patch

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index 93c97eade334..cb1457595f05 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -36,6 +36,7 @@ 
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/clk.h>
 
 #include "rt2x00.h"
 #include "rt2800lib.h"
@@ -7675,6 +7676,27 @@  static const struct rf_channel rf_vals_5592_xtal40[] = {
 	{196, 83, 0, 12, 1},
 };
 
+/*
+ * RF value list for rt3xxx with Xtal20MHz
+ * Supports: 2.4 GHz (all) (RF3322)
+ */
+static const struct rf_channel rf_vals_xtal20mhz_3x[] = {
+	{1,    0xE2,	 2,  0x14},
+	{2,    0xE3,	 2,  0x14},
+	{3,    0xE4,	 2,  0x14},
+	{4,    0xE5,	 2,  0x14},
+	{5,    0xE6,	 2,  0x14},
+	{6,    0xE7,	 2,  0x14},
+	{7,    0xE8,	 2,  0x14},
+	{8,    0xE9,	 2,  0x14},
+	{9,    0xEA,	 2,  0x14},
+	{10,   0xEB,	 2,  0x14},
+	{11,   0xEC,	 2,  0x14},
+	{12,   0xED,	 2,  0x14},
+	{13,   0xEE,	 2,  0x14},
+	{14,   0xF0,	 2,  0x18},
+};
+
 static int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
 {
 	struct hw_mode_spec *spec = &rt2x00dev->spec;
@@ -7764,7 +7786,10 @@  static int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
 	case RF5390:
 	case RF5392:
 		spec->num_channels = 14;
-		spec->channels = rf_vals_3x;
+		if (spec->clk_is_20mhz)
+			spec->channels = rf_vals_xtal20mhz_3x;
+		else
+			spec->channels = rf_vals_3x;
 		break;
 
 	case RF3052:
@@ -7945,6 +7970,20 @@  static int rt2800_probe_rt(struct rt2x00_dev *rt2x00dev)
 	return 0;
 }
 
+int rt2800_probe_clk(struct rt2x00_dev *rt2x00dev)
+{
+	struct hw_mode_spec *spec = &rt2x00dev->spec;
+	struct clk *clk = clk_get(rt2x00dev->dev, NULL);
+
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	if (clk_get_rate(clk) == 20000000)
+		spec->clk_is_20mhz = 1;
+
+	return 0;
+}
+
 int rt2800_probe_hw(struct rt2x00_dev *rt2x00dev)
 {
 	struct rt2800_drv_data *drv_data = rt2x00dev->drv_data;
@@ -7985,6 +8024,15 @@  int rt2800_probe_hw(struct rt2x00_dev *rt2x00dev)
 	rt2800_register_write(rt2x00dev, GPIO_CTRL, reg);
 
 	/*
+	 * Probe SoC clock.
+	 */
+	if (rt2x00_is_soc(rt2x00dev)) {
+		retval = rt2800_probe_clk(rt2x00dev);
+		if (retval)
+			return retval;
+	}
+
+	/*
 	 * Initialize hw specifications.
 	 */
 	retval = rt2800_probe_hw_mode(rt2x00dev);
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00.h b/drivers/net/wireless/ralink/rt2x00/rt2x00.h
index cfbf414c2627..b1eec49b0dac 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00.h
@@ -400,6 +400,7 @@  static inline struct rt2x00_intf* vif_to_intf(struct ieee80211_vif *vif)
  * @channels: Device/chipset specific channel values (See &struct rf_channel).
  * @channels_info: Additional information for channels (See &struct channel_info).
  * @ht: Driver HT Capabilities (See &ieee80211_sta_ht_cap).
+ * @clk_is_20mhz: External crystal of WiSoC is 20MHz instead of 40MHz
  */
 struct hw_mode_spec {
 	unsigned int supported_bands;
@@ -415,6 +416,7 @@  struct hw_mode_spec {
 	const struct channel_info *channels_info;
 
 	struct ieee80211_sta_ht_cap ht;
+	int clk_is_20mhz;
 };
 
 /*