linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <moorray@wp.pl>
To: Gertjan van Wingerde <gwingerde@gmail.com>
Cc: Xose Vazquez Perez <xose.vazquez@gmail.com>,
	"users@rt2x00.serialmonkey.com" <users@rt2x00.serialmonkey.com>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Subject: Re: [rt2x00-users] [PATCH 3.10-rc] wireless: rt2x00: rt2800: fix hardware antenna diversity for RT5370G
Date: Sun, 2 Jun 2013 17:38:10 +0200	[thread overview]
Message-ID: <20130602173810.1d058f12@north> (raw)
In-Reply-To: <86F5D1A2-7E2C-4EE5-9D29-C66306833C35@gmail.com>

Hi!

Any progress on this patch?

  -- Kuba

On Wed, 8 May 2013 00:30:22 +0200, Gertjan van Wingerde wrote:
> Hi Xose,
> 
> Sent from my iPad
> 
> On 7 mei 2013, at 22:03, Xose Vazquez Perez <xose.vazquez@gmail.com> wrote:
> 
> > RT5370G has hardware RX antenna diversity like RT5390R.
> > 
> > based on 2012_03_22_RT5572_Linux_STA_v2.6.0.0_DPO
> 
> I'll check this patch tomorrow, when I get back home.
> 
> In the meantime some style related comments below.
> 
> > 
> > Cc: Ivo van Doorn <IvDoorn@gmail.com>
> > Cc: Gertjan van Wingerde <gwingerde@gmail.com>
> > Cc: Helmut Schaa <helmut.schaa@googlemail.com>
> > Cc: John W. Linville <linville@tuxdriver.com>
> > Cc: users@rt2x00.serialmonkey.com
> > Cc: linux-wireless@vger.kernel.org
> > Tested-by: wnewbie72@gmail.com
> > Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
> > ---
> > drivers/net/wireless/rt2x00/rt2800.h    | 3 ++-
> > drivers/net/wireless/rt2x00/rt2800lib.c | 8 +++++---
> > 2 files changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/rt2x00/rt2800.h b/drivers/net/wireless/rt2x00/rt2800.h
> > index a7630d5..6e84eee 100644
> > --- a/drivers/net/wireless/rt2x00/rt2800.h
> > +++ b/drivers/net/wireless/rt2x00/rt2800.h
> > @@ -89,7 +89,8 @@
> > #define REV_RT3090E            0x0211
> > #define REV_RT3390E            0x0211
> > #define REV_RT5390F            0x0502
> > -#define REV_RT5390R            0x1502
> > +#define REV_RT5370G            0x0503 /* hardware RX antenna diversity */
> > +#define REV_RT5390R            0x1502 /* hardware RX antenna diversity */
> > #define REV_RT5592C            0x0221
> 
> Please don't the comments to the chipset revision definitions. These macros are usually used all over the place; there is no need to specify this here.
> 
> > 
> > #define DEFAULT_RSSI_OFFSET        120
> > diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
> > index b52d70c..e202ec7 100644
> > --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> > +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> > @@ -4311,8 +4311,9 @@ static int rt2800_init_bbp(struct rt2x00_dev *rt2x00dev)
> >            rt2800_register_write(rt2x00dev, GPIO_CTRL, reg);
> >        }
> > 
> > -        /* This chip has hardware antenna diversity*/
> > -        if (rt2x00_rt_rev_gte(rt2x00dev, RT5390, REV_RT5390R)) {
> > +        /* These chips have hardware RX antenna diversity */
> > +        if (rt2x00_rt_rev_gte(rt2x00dev, RT5390, REV_RT5390R) ||
> > +            rt2x00_rt_rev_gte(rt2x00dev, RT5390, REV_RT5370G)) {
> >            rt2800_bbp_write(rt2x00dev, 150, 0); /* Disable Antenna Software OFDM */
> >            rt2800_bbp_write(rt2x00dev, 151, 0); /* Disable Antenna Software CCK */
> >            rt2800_bbp_write(rt2x00dev, 154, 0); /* Clear previously selected antenna */
> 
> With respect to the comment change: are you sure this is only about RX antenna diversity?
> 
> Also, I'm wondering if the change to the if condition brings us everything that is wanted. Checking the same chipset for different minimal revisions doesn't make a whole lot of sense to me.
> Sure, you are fixing later RT5370 revisions here, but may be botching up early RT5390 revisions here at the same time
> 
> > @@ -5554,7 +5555,8 @@ static int rt2800_init_eeprom(struct rt2x00_dev *rt2x00dev)
> >        rt2x00dev->default_ant.rx = ANTENNA_A;
> >    }
> > 
> > -    if (rt2x00_rt_rev_gte(rt2x00dev, RT5390, REV_RT5390R)) {
> > +    if (rt2x00_rt_rev_gte(rt2x00dev, RT5390, REV_RT5390R) ||
> > +        rt2x00_rt_rev_gte(rt2x00dev, RT5390, REV_RT5370G)) {
> >        rt2x00dev->default_ant.tx = ANTENNA_HW_DIVERSITY; /* Unused */
> >        rt2x00dev->default_ant.rx = ANTENNA_HW_DIVERSITY; /* Unused */
> >      }
> 
> The same comment on the if conditions apply here.
> 
> ---
> Gertjan
> _______________________________________________
> users mailing list
> users@rt2x00.serialmonkey.com
> http://rt2x00.serialmonkey.com/mailman/listinfo/users_rt2x00.serialmonkey.com
> 


      reply	other threads:[~2013-06-02 15:38 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-07 20:03 [PATCH 3.10-rc] wireless: rt2x00: rt2800: fix hardware antenna diversity for RT5370G Xose Vazquez Perez
2013-05-07 21:00 ` [rt2x00-users] " Jakub Kicinski
2013-05-07 22:30 ` Gertjan van Wingerde
2013-06-02 15:38   ` Jakub Kicinski [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130602173810.1d058f12@north \
    --to=moorray@wp.pl \
    --cc=gwingerde@gmail.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=users@rt2x00.serialmonkey.com \
    --cc=xose.vazquez@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).