From: axel.rasmussen1@gmail.com
To: linux-kernel@vger.kernel.org
Subject: Re: [PATCH 7/9] drivers: staging: rtl8187se: fixed checkpatch.pl errors
Date: Sun, 02 Mar 2014 13:19:40 -0700 [thread overview]
Message-ID: <2481284.M04WIiAyls@celine> (raw)
In-Reply-To: <1393738591.31402.23.camel@joe-AO722>
On Saturday 01 March 2014 9:36:31 PM Joe Perches wrote:
> On Sat, 2014-03-01 at 22:22 -0700, Axel Rasmussen wrote:
> > The definition of the driver's ChannelPlan array produced a large number
> > of checkpatch.pl errors. This patch fixes all of them by adding spaces
> > and wrapping the resulting overly-long lines.
>
> I think the current code is better.
>
> > diff --git a/drivers/staging/rtl8187se/r8180_core.c
> > b/drivers/staging/rtl8187se/r8180_core.c
> []
>
> > @@ -2242,17 +2242,44 @@ static void watch_dog_adaptive(unsigned long data)
>
> [for instance]
>
> > static struct rtl8187se_channel_list channel_plan_list[] = {
> >
> > - {{1,2,3,4,5,6,7,8,9,10,11,36,40,44,48,52,56,60,64},19}, /* FCC */
>
> []
>
> > + /* FCC */
> > + {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 36, 40,
> > + 44, 48, 52, 56, 60, 64}, 19},
>
> If you really do this, it may be better to
> remove the len variable and test for != 0
> instead of channel_plan_list[].len
>
> So instead of:
>
> drivers/staging/rtl8187se/r8180_core.c:
> for (i = 0; i < channel_plan_list[channel_plan].len; i++) {
> Maybe:
> for (i = 0; channel_plan_list[channel_plan].channel[i]; i++) {
True, removing the length and using a 0 terminating element would simplify the channel plan list, allowing it to just be a list of arrays, rather than a list of structs. The one thing I'd be curious about is, judging by the driver's TODO file, it seems that its existing private ieee80211 implementation is going to be replaced with one that already exists elsewhere in the kernel, so the entire channel plan list may disappear at that point? If true, I'm not sure it's worth doing anything very complicated to the existing implementation.
Maybe someone can confirm that this channel plan list will go away once the driver is integrated with drivers/net/wireless/rtl818x? Otherwise, I can alter this patch to replace struct rtl8187se_channel_list with a simple array of channels.
next prev parent reply other threads:[~2014-03-02 20:19 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-03-02 5:22 [PATCH 0/9] drivers: staging: rtl8187se: various code cleanups Axel Rasmussen
2014-03-02 5:22 ` [PATCH 1/9] drivers: staging: rtl8187se: use netdev_* instead of prink Axel Rasmussen
2014-03-02 5:22 ` [PATCH 2/9] drivers: staging: rtl8187se: refactor/clean signal smoothing Axel Rasmussen
2014-03-02 5:22 ` [PATCH 3/9] drivers: staging: rtl8187se: wrap excessively long lines Axel Rasmussen
2014-03-03 8:58 ` Dan Carpenter
2014-03-02 5:22 ` [PATCH 4/9] " Axel Rasmussen
2014-03-03 9:02 ` Dan Carpenter
2014-03-02 5:22 ` [PATCH 5/9] " Axel Rasmussen
2014-03-02 5:22 ` [PATCH 6/9] drivers: staging: rtl8187se: fixed broken indentation Axel Rasmussen
2014-03-02 5:22 ` [PATCH 7/9] drivers: staging: rtl8187se: fixed checkpatch.pl errors Axel Rasmussen
2014-03-02 5:36 ` Joe Perches
2014-03-02 20:19 ` axel.rasmussen1 [this message]
2014-03-02 5:22 ` [PATCH 8/9] drivers: staging: rtl8187se: wrap excessively long lines Axel Rasmussen
2014-03-02 5:22 ` [PATCH 9/9] drivers: staging: rtl8187se: refactor wmm_param_update Axel Rasmussen
2014-03-03 9:15 ` Dan Carpenter
2014-03-04 7:12 ` [PATCH v2 0/9] drivers: staging: rtl8187se: various code cleanups Axel Rasmussen
2014-03-04 7:12 ` [PATCH v2 1/9] drivers: staging: rtl8187se: use netdev_* instead of prink Axel Rasmussen
2014-03-04 7:12 ` [PATCH v2 2/9] drivers: staging: rtl8187se: refactor/clean signal smoothing Axel Rasmussen
2014-03-04 7:12 ` [PATCH v2 3/9] drivers: staging: rtl8187se: wrap excessively long lines Axel Rasmussen
2014-03-04 7:12 ` [PATCH v2 4/9] " Axel Rasmussen
2014-03-04 7:12 ` [PATCH v2 5/9] " Axel Rasmussen
2014-03-04 7:12 ` [PATCH v2 6/9] drivers: staging: rtl8187se: fixed broken indentation Axel Rasmussen
2014-03-04 7:12 ` [PATCH v2 7/9] drivers: staging: rtl8187se: fixed checkpatch.pl errors Axel Rasmussen
2014-03-04 7:12 ` [PATCH v2 8/9] drivers: staging: rtl8187se: wrap excessively long lines Axel Rasmussen
2014-03-04 7:12 ` [PATCH v2 9/9] drivers: staging: rtl8187se: refactor wmm_param_update Axel Rasmussen
2014-03-04 9:51 ` [PATCH v2 0/9] drivers: staging: rtl8187se: various code cleanups Dan Carpenter
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=2481284.M04WIiAyls@celine \
--to=axel.rasmussen1@gmail.com \
--cc=linux-kernel@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