linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Larry Finger <Larry.Finger@lwfinger.net>
To: Johannes Berg <johannes@sipsolutions.net>,
	Gaston Gonzalez <gascoar@gmail.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>,
	devel@driverdev.osuosl.org, arnd@arndb.de,
	gregkh@linuxfoundation.org, linux-wireless@vger.kernel.org,
	joe@perches.com, navyasri.tech@gmail.com,
	julian.calaby@gmail.com
Subject: Re: [PATCH] staging: rtl8192u: ieee80211: Silence sparse warning
Date: Thu, 14 May 2015 15:01:42 -0500	[thread overview]
Message-ID: <5554FF26.1040304@lwfinger.net> (raw)
In-Reply-To: <1431632112.2237.5.camel@sipsolutions.net>

On 05/14/2015 02:35 PM, Johannes Berg wrote:
> On Wed, 2015-05-13 at 22:04 -0300, Gaston Gonzalez wrote:
>
>>   .../rtl8192u/ieee80211/ieee80211_crypt_tkip.c      | 53
>> ++--------------------
>>   include/net/mac80211.h                             |  3 ++
>>   net/mac80211/tkip.c                                |  9 ++--
>>   3 files changed, 10 insertions(+), 55 deletions(-)
>
> That doesn't seem right to me. Clearly, that's a staging driver that
> ships its own 802.11 subsystem, and bears no existing relation to
> mac80211. Exporting a mac80211 internal function to make such a driver
> happy makes me very suspicious. That function might not be very mac80211
> specific, but exporting it from mac80211 doesn't seem right, that
> introduces a massive and mostly artificial dependency into the driver.
>
> Now arguably that driver is in staging and that doesn't matter, but then
> why even bother cleaning this up? It seems likely that a well-written
> driver for this would use mac80211, and not have to bother with this at
> all since it could then use ieee80211_get_tkip_rx_p1k() or
> ieee80211_get_tkip_p2k() or the update_tkip_key() method.
>
> So I'm not fond of this patch and without a *very very* good reason and
> explanation I'm not going to merge the mac80211 part. It certainly
> bothers me much less that a crappy staging driver has a few lines of
> duplicated code than it would to export mac80211 internals that real
> mac80211 drivers don't need.

I agree. No staging driver should ever require changes in any part of mac80211!

If you want to get credit for kernel patches, you are welcome to fuss with the 
code, but none of your changes should touch any part of the kernel other than 
the driver you are working on. That is an absolute requirement.

One other comment is that I have never seen this hardware. I wonder if anyone 
has it anymore.

Larry



  parent reply	other threads:[~2015-05-14 20:01 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1429476231-6197-1-git-send-email-gascoar@gmail.com>
     [not found] ` <20150420082441.GU10964@mwanda>
     [not found]   ` <553C18B6.807@gmail.com>
     [not found]     ` <20150427101242.GR14154@mwanda>
     [not found]       ` <554AD76E.7060605@gmail.com>
2015-05-08 11:03         ` [PATCH] staging: rtl8192u: ieee80211: Silence sparse warning Dan Carpenter
2015-05-12 22:23           ` Gaston Gonzalez
2015-05-12 23:11             ` Julian Calaby
2015-05-13  8:36             ` Dan Carpenter
2015-05-14  1:04               ` Gaston Gonzalez
2015-05-14  8:30                 ` Dan Carpenter
2015-05-14 19:35                 ` Johannes Berg
2015-05-14 19:51                   ` Dan Carpenter
2015-05-14 20:01                   ` Larry Finger [this message]
2015-05-14 22:03                   ` Gaston Gonzalez
2015-05-15  7:26                     ` Johannes Berg
2015-05-15 20:47                       ` Gaston Gonzalez

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=5554FF26.1040304@lwfinger.net \
    --to=larry.finger@lwfinger.net \
    --cc=arnd@arndb.de \
    --cc=dan.carpenter@oracle.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gascoar@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=joe@perches.com \
    --cc=johannes@sipsolutions.net \
    --cc=julian.calaby@gmail.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=navyasri.tech@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).