linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Larry Finger <Larry.Finger@lwfinger.net>
To: Hin-Tak Leung <hintak.leung@gmail.com>
Cc: antti@kaijanmaki.net, Hin-Tak Leung <htl10@users.sourceforge.net>,
	Herton Ronaldo Krzesinski <herton@mandriva.com.br>,
	linux-wireless@vger.kernel.org
Subject: Re: [RFC/RFT] rtl8187: Modify rfkill for new models
Date: Sat, 05 Dec 2009 14:17:39 -0600	[thread overview]
Message-ID: <4B1ABFE3.5090804@lwfinger.net> (raw)
In-Reply-To: <3ace41890912051155q722901ffx340ee8613ca465f8@mail.gmail.com>

On 12/05/2009 01:55 PM, Hin-Tak Leung wrote:

> The patch looks fine - I'll have a go later. Stylistically, I don't
> think you need to copy the vendor driver though. Since you are storing
> rfkill_mask in the device structure, there is no need to have the
> variable tmp (and it is an awkful name - although, I know, it is
> temp!), because you can simply set the rfkill_mask at the beginning,
> then override it for newer models. e.g.
> I suggest you change the logic to something like this:
> 
> priv->rfkill_mask = 0x02;
> if (newer product/gpio detection code) {
>      priv->rfkill_mask = 0x04;
> }
> 
> and while you are at it, called
> 0x02 RFKILL_MASK_FOR_MODEL_8187_89_97 and 0x04
> RFKILL_MASK_FOR_MODEL_8198 for clarity. Does it sound reasonable?

I got rid of the variable tmp; however due to the way that
eeprom_93cx6() returns its value, it is not possible to get all the
logic in one if statement. That section now looks like this:

   /* Handle the differing GPIO bit in different models */
   priv->rfkill_mask = RFKILL_MASK_8187_89_97;
   if (product_id == 0x8197 || product_id == 0x8198) {
       eeprom_93cx6_read(&eeprom, RTL8187_EEPROM_SELECT_GPIO, &reg);
       if (reg & 0xFF00)
           priv->rfkill_mask = RFKILL_MASK_8198;
   }
   printk(KERN_INFO "rtl8187: rfkill mask set to %d for Product ID 0x%x",
          priv->rfkill_mask, product_id);

Thanks for the suggestions.

Larry

      reply	other threads:[~2009-12-05 20:17 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-05 19:06 [RFC/RFT] rtl8187: Modify rfkill for new models Larry Finger
2009-12-05 19:55 ` Hin-Tak Leung
2009-12-05 20:17   ` Larry Finger [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=4B1ABFE3.5090804@lwfinger.net \
    --to=larry.finger@lwfinger.net \
    --cc=antti@kaijanmaki.net \
    --cc=herton@mandriva.com.br \
    --cc=hintak.leung@gmail.com \
    --cc=htl10@users.sourceforge.net \
    --cc=linux-wireless@vger.kernel.org \
    /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).