* [PATCH net-next 0/24] bnx2x: New FW and support for 578xx
From: Vlad Zolotarov @ 2011-06-14 11:32 UTC (permalink / raw)
To: Dave Miller, Michael Chan,
Bhanu (Venkata Bhanu Prakash) Gollapudi
Cc: Eilon Greenstein, netdev@vger.kernel.org, Dmitry Kravkov,
Yaniv Rosner, David Woodhouse
In-Reply-To: <BD3F7F1EFBA6D54DB056C4FFA451400809D2C13864@SJEXCHCCR01.corp.ad.broadcom.com>
Dave, pls., consider applying this patch series.
This patch series introduce support for the new 578x0 chip series and
therefore includes new 7.0.20 FW:
1) New LINK code.
2) Absolutely new HSI that allows supporting different chips in more
consolidated way, which caused a significant changes in the driver code
including moving the HSI handling code into a spearate files (patches 11
and 12).
We also send the new FW files to David Woodhouse. It's the first time we
submit to the linux-firmware git so I hope we don't mess the things up: we
DO NOT patch firmware/WHENCE and firmware/Makefile files in our net-next
patches and we patch the WHENCE file in the linux-firmware patch. Dave,
pls., confirm if it's a correct procedure.
If it is, pls., note that the bnx2x driver will compile but won't work
until u integrate the new FW into the kernel tree.
You may alsofind these patches at our public site.
net-next series at http://linux.broadcom.com/eilong/1.70.00-0/
New FW at http://linux.broadcom.com/eilong/FW-7.0.20.0/
Thanks,
vlad
^ permalink raw reply
* RE: [PATCH net-next 2/2] drivers/net: Remove casts of void *
From: Amit Salecha @ 2011-06-14 11:31 UTC (permalink / raw)
To: Joe Perches, netdev
Cc: Arnaldo Carvalho de Melo, Jay Cliburn, Chris Snook, Jie Yang,
Sathya Perla, Subbu Seetharaman, Ajit Khaparde, Rasesh Mody,
Debashis Dutt, Sjur Braendeland, Divy Le Ray, Breno Leitao,
Ron Mercer, Linux Driver, Anirban Chakraborty, Jon Mason,
Solarflare linux maintainers, Steve Hodgson, Ben Hutchings,
Daniele Venzano, David Dillow, linux-kerne
In-Reply-To: <486a21b708a9dce0c1c0059a21a2f9b66dac9e3c.1308024069.git.joe@perches.com>
> From: Joe Perches [mailto:joe@perches.com]
>
> Unnecessary casts of void * clutter the code.
>
> These are the remainder casts after several specific
> patches to remove netdev_priv and dev_priv.
>
> Done via coccinelle script:
>
> $ cat cast_void_pointer.cocci
> @@
> type T;
> T *pt;
> void *pv;
> @@
>
> - pt = (T *)pv;
> + pt = pv;
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> drivers/net/netxen/netxen_nic_ctx.c | 16 ++++++------
> drivers/net/qlcnic/qlcnic_ctx.c | 26 ++++++++++----------
> drivers/net/qlcnic/qlcnic_ethtool.c | 2 +-
> drivers/net/qlcnic/qlcnic_hw.c | 3 +-
> drivers/net/qlcnic/qlcnic_main.c | 2 +-
netxen_nic and qlcnic looks good.
-Amit
^ permalink raw reply
* Re: [PATCH] linux-firmware: Add a new bnx2x FW 7.0.20.0
From: Vlad Zolotarov @ 2011-06-14 10:53 UTC (permalink / raw)
To: David Woodhouse
Cc: netdev@vger.kernel.org, Eilon Greenstein, Dmitry Kravkov,
Yaniv Rosner
In-Reply-To: <1308046604.3450.7.camel@i7.infradead.org>
On Tuesday 14 June 2011 13:16:43 David Woodhouse wrote:
> On Tue, 2011-06-14 at 13:10 +0300, Vlad Zolotarov wrote:
> >
> > Aha... Correct me if I got u wrong: to test it I need to manually copy
> > the new .fw files into the /lib/firmware directory, install the clean
> > net-next kernel with the driver that supports this new FW and it
> > should work. Is it what u meant?
>
> Right. Imagine that you have checked out the linux-firmware.git
> repository directly into /lib/firmware, and nothing else has been
> written there.
Got it. I checked it now and surprizingly it worked... ;)
thanks, man.
I'm resending u *binaries* ...
>
>
^ permalink raw reply
* Re: Question about LRO/GRO and TCP acknowledgements
From: Ilpo Järvinen @ 2011-06-14 10:53 UTC (permalink / raw)
To: Joris van Rantwijk; +Cc: Eric Dumazet, Netdev
In-Reply-To: <20110612213726.4d203a6e@konijn>
On Sun, 12 Jun 2011, Joris van Rantwijk wrote:
> On 2011-06-12, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Think of GRO being a receiver facility against stress/load, typically
> > in datacenter.
> >
> > Only when receiver is overloaded, GRO kicks in and can coalesce
> > several frames before being handled in TCP stack in one run.
>
> Ok, it now becomes clear to me that I have a different scenario in mind
> than GRO was designed to handle. I'm interested in LRO as a method
> to sustain 1 Gbit through a single TCP connection on a slow embedded
> computer.
>
> > If receiver is so loaded that more than 2 frames are coalesced in a
> > NAPI run, it certainly helps to not allow sender to increase its cwnd
> > more than one SMSS. We probably are right before packet drops anyway.
>
> Right. So unlike TSO, GRO is not a transparent, generally applicable
> performance improvement. It's more like a form of graceful degradation,
> helping a server to sustain overall throughput when it is already
> swamped in TCP traffic.
>
> Thanks for your clarification. This has certainly solved some confusion
> on my side.
BTW, it wouldn't be impossible to create all those "missing" ACKs on the
TCP layer relatively cheaply when receiving the GRO'ed super segment. I'm
certainly not opposed you coming up such patch which does all that minimal
work needed on TCP layer but I think it requires also some TSO/GSO related
problem solving because TSO/GSO as is won't let you create such super ACKs
we'd want to send out on that single go.
--
i.
^ permalink raw reply
* [ipv6] valid_lft and active connections
From: Stefan (metze) Metzmacher @ 2011-06-14 10:39 UTC (permalink / raw)
To: netdev@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 552 bytes --]
Hi,
I have the following problem:
If I use ipv6 addresses with valid_lft != forever, the ipv6 addresses
are removed from the interface if the valid_lft expires, even if there're
established connection which use with address.
Would it be possible keep the address until the last active connection
is closed? Otherwise the usable of the privacy extensions will make
very long living tcp connections impossible.
It would be also ok to add a new special value "used" similar to "forever",
which triggers the modified behavior.
metze
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 2/2] drivers/net: Remove casts of void *
From: Chris Snook @ 2011-06-14 10:34 UTC (permalink / raw)
To: Joe Perches
Cc: netdev, Arnaldo Carvalho de Melo, Jay Cliburn, Jie Yang,
Sathya Perla, Subbu Seetharaman, Ajit Khaparde, Rasesh Mody,
Debashis Dutt, Sjur Braendeland, Divy Le Ray, Breno Leitao,
Amit Kumar Salecha, Ron Mercer, linux-driver, Anirban Chakraborty,
Jon Mason, Solarflare linux maintainers, Steve Hodgson,
Ben Hutchings, Daniele Venzano, David Dillow
In-Reply-To: <486a21b708a9dce0c1c0059a21a2f9b66dac9e3c.1308024069.git.joe@perches.com>
On Tue, Jun 14, 2011 at 12:02 AM, Joe Perches <joe@perches.com> wrote:
> diff --git a/drivers/net/atl1e/atl1e_main.c b/drivers/net/atl1e/atl1e_main.c
> index 86a9122..c3c5db1 100644
> --- a/drivers/net/atl1e/atl1e_main.c
> +++ b/drivers/net/atl1e/atl1e_main.c
> @@ -800,8 +800,7 @@ static int atl1e_setup_ring_resources(struct atl1e_adapter *adapter)
> /* Init TPD Ring */
> tx_ring->dma = roundup(adapter->ring_dma, 8);
> offset = tx_ring->dma - adapter->ring_dma;
> - tx_ring->desc = (struct atl1e_tpd_desc *)
> - (adapter->ring_vir_addr + offset);
> + tx_ring->desc = adapter->ring_vir_addr + offset;
> size = sizeof(struct atl1e_tx_buffer) * (tx_ring->count);
> tx_ring->tx_buffer = kzalloc(size, GFP_KERNEL);
> if (tx_ring->tx_buffer == NULL) {
> @@ -827,7 +826,7 @@ static int atl1e_setup_ring_resources(struct atl1e_adapter *adapter)
>
> /* Init CMB dma address */
> tx_ring->cmb_dma = adapter->ring_dma + offset;
> - tx_ring->cmb = (u32 *)(adapter->ring_vir_addr + offset);
> + tx_ring->cmb = adapter->ring_vir_addr + offset;
> offset += sizeof(u32);
>
> for (i = 0; i < adapter->num_rx_queues; i++) {
> diff --git a/drivers/net/atlx/atl2.c b/drivers/net/atlx/atl2.c
> index 16249e9..24e1592 100644
> --- a/drivers/net/atlx/atl2.c
> +++ b/drivers/net/atlx/atl2.c
> @@ -311,8 +311,7 @@ static s32 atl2_setup_ring_resources(struct atl2_adapter *adapter)
> adapter->txd_dma = adapter->ring_dma ;
> offset = (adapter->txd_dma & 0x7) ? (8 - (adapter->txd_dma & 0x7)) : 0;
> adapter->txd_dma += offset;
> - adapter->txd_ring = (struct tx_pkt_header *) (adapter->ring_vir_addr +
> - offset);
> + adapter->txd_ring = adapter->ring_vir_addr + offset;
>
> /* Init TXS Ring */
> adapter->txs_dma = adapter->txd_dma + adapter->txd_ring_size;
atlx looks fine.
Acked-By: Chris Snook <chris.snook@gmail.com>
^ permalink raw reply
* Re: [PATCH] linux-firmware: Add a new bnx2x FW 7.0.20.0
From: David Woodhouse @ 2011-06-14 10:16 UTC (permalink / raw)
To: Vlad Zolotarov
Cc: netdev@vger.kernel.org, Eilon Greenstein, Dmitry Kravkov,
Yaniv Rosner
In-Reply-To: <201106141310.16440.vladz@broadcom.com>
On Tue, 2011-06-14 at 13:10 +0300, Vlad Zolotarov wrote:
>
> Aha... Correct me if I got u wrong: to test it I need to manually copy
> the new .fw files into the /lib/firmware directory, install the clean
> net-next kernel with the driver that supports this new FW and it
> should work. Is it what u meant?
Right. Imagine that you have checked out the linux-firmware.git
repository directly into /lib/firmware, and nothing else has been
written there.
--
dwmw2
^ permalink raw reply
* Re: [PATCH] linux-firmware: Add a new bnx2x FW 7.0.20.0
From: Vlad Zolotarov @ 2011-06-14 10:10 UTC (permalink / raw)
To: David Woodhouse
Cc: netdev@vger.kernel.org, Eilon Greenstein, Dmitry Kravkov,
Yaniv Rosner
In-Reply-To: <1308045859.3450.6.camel@i7.infradead.org>
On Tuesday 14 June 2011 13:04:18 David Woodhouse wrote:
> On Tue, 2011-06-14 at 12:57 +0300, Vlad Zolotarov wrote:
> >
> > What do u mean? I do have put the ihex files under the net-next kernel
> > tree at the regular location plus I updated the the kernel's
> > firmware/WHENCE and firmware/Makefile files and of course I ran a
> > functionality tests for the driver after all patches and with the new
> > firmware.
>
> Ah, there lies your problem.
>
> Never put new firmware into the kernel source tree. It doesn't belong
> there.
>
> In the linux-firmware tree we store the *binary* files, which go
> directly into /lib/firmware as-is.
Aha... Correct me if I got u wrong: to test it I need to manually copy the new .fw files into the /lib/firmware directory, install the clean net-next kernel with the driver that supports this new FW and it should work. Is it what u meant?
thanks,
vlad
>
>
^ permalink raw reply
* Re: [PATCH] linux-firmware: Add a new bnx2x FW 7.0.20.0
From: David Woodhouse @ 2011-06-14 10:04 UTC (permalink / raw)
To: Vlad Zolotarov
Cc: netdev@vger.kernel.org, Eilon Greenstein, Dmitry Kravkov,
Yaniv Rosner
In-Reply-To: <201106141257.40621.vladz@broadcom.com>
On Tue, 2011-06-14 at 12:57 +0300, Vlad Zolotarov wrote:
>
> What do u mean? I do have put the ihex files under the net-next kernel
> tree at the regular location plus I updated the the kernel's
> firmware/WHENCE and firmware/Makefile files and of course I ran a
> functionality tests for the driver after all patches and with the new
> firmware.
Ah, there lies your problem.
Never put new firmware into the kernel source tree. It doesn't belong
there.
In the linux-firmware tree we store the *binary* files, which go
directly into /lib/firmware as-is.
--
dwmw2
^ permalink raw reply
* Re: [PATCH] linux-firmware: Add a new bnx2x FW 7.0.20.0
From: Vlad Zolotarov @ 2011-06-14 9:57 UTC (permalink / raw)
To: David Woodhouse
Cc: netdev@vger.kernel.org, Eilon Greenstein, Dmitry Kravkov,
Yaniv Rosner
In-Reply-To: <1308044887.3450.4.camel@i7.infradead.org>
On Tuesday 14 June 2011 12:48:06 David Woodhouse wrote:
> On Tue, 2011-06-14 at 12:41 +0300, Vlad Zolotarov wrote:
> >
> > bnx2x/bnx2x-e1-7.0.20.0.fw.ihex |10075 +++++++++++++++++++++
> > bnx2x/bnx2x-e1h-7.0.20.0.fw.ihex |10538 ++++++++++++++++++++++
> > bnx2x/bnx2x-e2-7.0.20.0.fw.ihex |18190 ++++++++++++++++++++++++++++++++++++++
>
> You haven't tested this, have you?
What do u mean? I do have put the ihex files under the net-next kernel tree at the regular location plus I updated the the kernel's firmware/WHENCE and firmware/Makefile files and of course I ran a functionality tests for the driver after all patches and with the new firmware.
If u meant the different sort of testing, pls., clarify.
>
> Add the binary file, not an ihex version.
I'll resend the binary files.
thanks,
vlad
>
>
^ permalink raw reply
* Re: [PATCH] linux-firmware: Add a new bnx2x FW 7.0.20.0
From: David Woodhouse @ 2011-06-14 9:48 UTC (permalink / raw)
To: Vlad Zolotarov
Cc: netdev@vger.kernel.org, Eilon Greenstein, Dmitry Kravkov,
Yaniv Rosner
In-Reply-To: <201106141241.40764.vladz@broadcom.com>
On Tue, 2011-06-14 at 12:41 +0300, Vlad Zolotarov wrote:
>
> bnx2x/bnx2x-e1-7.0.20.0.fw.ihex |10075 +++++++++++++++++++++
> bnx2x/bnx2x-e1h-7.0.20.0.fw.ihex |10538 ++++++++++++++++++++++
> bnx2x/bnx2x-e2-7.0.20.0.fw.ihex |18190 ++++++++++++++++++++++++++++++++++++++
You haven't tested this, have you?
Add the binary file, not an ihex version.
--
dwmw2
^ permalink raw reply
* Re: Re: Re: [RFC PATCH 1/1] IPVS netns shutdown/startup dead-lock
From: Hans Schillstrom @ 2011-06-14 9:47 UTC (permalink / raw)
To: Julian Anastasov
Cc: Hans Schillstrom, horms, wensong, lvs-devel, netdev,
netfilter-devel
Hello,
>
>On Tue, 14 Jun 2011, Hans Schillstrom wrote:
>
>> >On Mon, 13 Jun 2011, Hans Schillstrom wrote:
>> >
>> >> ip_vs_mutext is used by both netns shutdown code and startup
>> >> and both implicit uses sk_lock-AF_INET mutex.
>> >>
>> >> cleanup CPU-1 startup CPU-2
>> >> ip_vs_dst_event() ip_vs_genl_set_cmd()
>> >> sk_lock-AF_INET __ip_vs_mutex
>> >> sk_lock-AF_INET
>> >> __ip_vs_mutex
>> >> * DEAD LOCK *
>> >
>> > So, sk_lock-AF_INET is locked before calling
>> >ip_vs_dst_event ? Do you have a backtrace for this case?
>>
>> Yes plenty this one is with lockdep
>>
>> Chain exists of:
>> rtnl_mutex --> __ip_vs_mutex --> sk_lock-AF_INET
>>
>> Possible unsafe locking scenario:
>>
>> CPU0 CPU1
>> ---- ----
>> lock(sk_lock-AF_INET);
>> lock(__ip_vs_mutex);
>> lock(sk_lock-AF_INET);
>> lock(rtnl_mutex);
>>
>> *** DEADLOCK ***
>>
>> 3 locks held by ipvsadm/993:
>> #0: (genl_mutex){+.+.+.}, at: [<ffffffff812edc52>] genl_lock+0x17/0x19
>> #1: (__ip_vs_mutex){+.+.+.}, at: [<ffffffff81307dcb>] ip_vs_genl_set_cmd+0xe1/0x3a3
>> #2: (sk_lock-AF_INET){+.+.+.}, at: [<ffffffff8130ffc1>] start_sync_thread+0x3ec/0x5ff
>
> I see
>
>> >> ip_vs_mutex per name-space seems to be a more future proof solution.
>> >
>> > Global mutex protects some global lists such as
>> >virtual services. If your patch works, better way to fix this problem
>> >is to use some new mutex. May be we can move the IPVS_CMD_NEW_DAEMON,
>> >IPVS_CMD_DEL_DAEMON and IP_VS_SO_GET_DAEMON code before the
>> >__ip_vs_mutex locking. This mutex should be used for start_sync_thread,
>> >stop_sync_thread, ip_vs_genl_dump_daemons and IP_VS_SO_GET_DAEMON.
>> >For example, ip_vs_sync_mutex.
>>
>> I think we should avoid global mutexes as a rule of tumb,
>> because it's realy hard to keep track of all possible cases
>> that can occur when multiple netns is alive and/or goes up and down.
>>
>> There might be more suprises while a netns exits (in terms of locks)...
>> my gut feeling is, avoid global locks as long as possible.
>
> There should not be a problem between two netns when
>using global mutexes.
as long as the locking occurs in the same order :-)
>And there are no many places in IPVS
>where other modules are accessed.
>
>> > Note that __ip_vs_sync_cleanup is missing a
>> >__ip_vs_mutex lock. We have to use the new mutex there.
>>
>> OK
>>
>> >
>> >> Which one should be used ?
>> >
>> > For now __ip_vs_mutex should be global ...
>>
>> I do agree, but in the long term I vote for mutex per netns.
>
> It will not help because the problem does not happen
>between two netspaces but between ipvs and other modules.
>The same problem would happen even if __ip_vs_mutex was
>pernet mutex.
Actually it's between userspaces that uses different netns
i.e. when starting a thread and exit a container (with different namespaces)
This bug would not have occured if a per netns mutex had been used.
>So, lets try with new mutex.
OK, I missed the reading of thread status, i.e. a sync_mutex is needed
There is no need for a lock(mutex) in ip_vs_sync_net_cleanup() before stoping threads,
because when it's called no user processes exits in that namespace.
Regards
Hans Schillstrom
^ permalink raw reply
* Re: [PATCH] net/usb: Add Samsung Kalmia driver for Samsung GT-B3730
From: Oliver Neukum @ 2011-06-14 9:46 UTC (permalink / raw)
To: Marius Kotsbak
Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Marius B. Kotsbak
In-Reply-To: <4DF72A90.1070702-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Am Dienstag, 14. Juni 2011, 11:32:00 schrieb Marius Kotsbak:
> Den 14. juni 2011 10:49, skrev Oliver Neukum:
> > Am Samstag, 11. Juni 2011, 23:55:18 schrieb Marius B. Kotsbak:
> >
> >> +static int
> >> +kalmia_init_and_get_ethernet_addr(struct usbnet *dev, u8 *ethernet_addr)
> >> +{
> >> + char init_msg_1[] =
> >> + { 0x57, 0x50, 0x04, 0x00, 0x00, 0x00, 0x00, 0x20, 0x00, 0x00,
> >> + 0x00, 0x00 };
> >> + char init_msg_2[] =
> >> + { 0x57, 0x50, 0x04, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0xf4,
> >> + 0x00, 0x00 };
> >> + char receive_buf[28];
> > You are doing DMA on the stack. This will fail on some architectures.
> >
>
> Okay, a bit strange that this API is available then. Which API functions
> should be used in this case?
The API is correct. You just need to copy the init strings into buffers
allocated with kmalloc.
> >> + if (status< 0) {
> >> + usb_set_intfdata(intf, NULL);
> >> + usb_driver_release_interface(driver_of(intf), intf);
> >> + return status;
> > Why are you doing this? What is to be undone?
>
> You mean the 3 last lines? I just did the same as done in another
> similar usbnet based driver. Is it incorrect if the init fails?
Yes, the last three lines. If all drivers do that this is an opportunity to
share common code in usbnet. I'll look at it.
> >> +static int
> >> +kalmia_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
> >> +{
> >> + /*
> >> + * Our task here is to strip off framing, leaving skb with one
> >> + * data frame for the usbnet framework code to process.
> >> + */
> >> + const u8 HEADER_END_OF_USB_PACKET[] =
> >> + { 0x57, 0x5a, 0x00, 0x00, 0x08, 0x00 };
> >> + const u8 EXPECTED_UNKNOWN_HEADER_1[] =
> >> + { 0x57, 0x43, 0x1e, 0x00, 0x15, 0x02 };
> >> + const u8 EXPECTED_UNKNOWN_HEADER_2[] =
> >> + { 0x57, 0x50, 0x0e, 0x00, 0x00, 0x00 };
> > What does the compiler do to this declaration?
>
> Not sure I understand what you mean here.
Will the compiler put those strings into the image or build them on
the stack each time the function is called? Shouldn't they be static?
> >> + u8 i = 0;
> > Why not int?
>
> I can change that. Thought int was not to be used because it is of
> unknown size.
Well, then are you sure i will never be larger than 255?
> Anyway it should not do any damage for anyone (I think even the driver
> disk is available with this included). The module could in fact also do
> the switch into modem mode to avoid the dependency on usb_modeswitch.
Please don't do that. It has been decided to put such things into usb_modeswitch
for now.
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH] net/r8169: Update the new parser for the new firmware
From: hayeswang @ 2011-06-14 9:36 UTC (permalink / raw)
To: 'Francois Romieu'; +Cc: netdev, linux-kernel
In-Reply-To: <20110613143241.GA1579@electric-eye.fr.zoreil.com>
> -----Original Message-----
> From: Francois Romieu [mailto:romieu@fr.zoreil.com]
> Sent: Monday, June 13, 2011 10:33 PM
> To: Hayeswang
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] net/r8169: Update the new parser for the
> new firmware
>
> Hayes Wang <hayeswang@realtek.com> :
> > Update the parser for the new firmware which is embedded
> some information.
> > The paser cannot be used for the old firmware. It is only
> for the new type one.
>
> >
> > Signed-off-by: Hayes Wang <hayeswang@realtek.com>
> > ---
> > drivers/net/r8169.c | 59
> ++++++++++++++++++++++++++++----------------------
> > 1 files changed, 33 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c index
> > ef1ce2e..87b684f 100644
> > --- a/drivers/net/r8169.c
> > +++ b/drivers/net/r8169.c
> > @@ -36,11 +36,11 @@
> > #define MODULENAME "r8169"
> > #define PFX MODULENAME ": "
> >
> > -#define FIRMWARE_8168D_1 "rtl_nic/rtl8168d-1.fw"
> > -#define FIRMWARE_8168D_2 "rtl_nic/rtl8168d-2.fw"
> > -#define FIRMWARE_8168E_1 "rtl_nic/rtl8168e-1.fw"
> > -#define FIRMWARE_8168E_2 "rtl_nic/rtl8168e-2.fw"
> > -#define FIRMWARE_8105E_1 "rtl_nic/rtl8105e-1.fw"
> > +#define FIRMWARE_8168D_1 "rtl_nic/rtl8168d-3.fw"
> > +#define FIRMWARE_8168D_2 "rtl_nic/rtl8168d-4.fw"
> > +#define FIRMWARE_8168E_1 "rtl_nic/rtl8168e-3.fw"
> > +#define FIRMWARE_8168E_2 "rtl_nic/rtl8168e-4.fw"
> > +#define FIRMWARE_8105E_1 "rtl_nic/rtl8105e-2.fw"
>
> Why ?
I want to keep the old firmware used by the old paser. The old paser cannot use
the new firmware and the new paser cannot use the old firmware, so I separate
them.
>
> I see it more as a calling convention to communicate with
> userspace than as a place to funnel a partial version information.
>
> > #ifdef RTL8169_DEBUG
> > #define assert(expr) \
> > @@ -594,6 +594,13 @@ struct ring_info {
> > u8 __pad[sizeof(void *) - sizeof(u32)];
> > };
> >
> > +struct fw_info {
> > + char version[32];
> > + u32 fw_start;
> > + u32 fw_len;
> > + u8 chksum;
>
> The chksum field is never used.
This field in the binary file makes sure the checksum is zero. Driver don't need
to access this address. But it needs in the binary file for checking.
>
> [...]
> > @@ -1743,16 +1750,30 @@ static void rtl_writephy_batch(struct
> > rtl8169_private *tp, static void rtl_phy_write_fw(struct
> > rtl8169_private *tp, const struct firmware *fw) {
> > - __le32 *phytable = (__le32 *)fw->data;
> > + __le32 *phytable;
> > struct net_device *dev = tp->dev;
> > - size_t index, fw_size = fw->size / sizeof(*phytable);
> > + struct fw_info *f_info;
> > + size_t index, fw_size;
>
> s/index/i/
Yes, I would replace it.
>
> > u32 predata, count;
> > + u8 checksum;
> > +
> > + f_info = (struct fw_info *)fw->data;
>
> It will not work very well on big-endian computers. start and
> len should both use le32_to_cpu.
>
I would fix these. Thanks.
> >
> > - if (fw->size % sizeof(*phytable)) {
> > - netif_err(tp, probe, dev, "odd sized firmware
> %zd\n", fw->size);
> > + if (checksum != 0) {
> > + netif_err(tp, probe, dev, "none zero
> checksum(%u)\n", checksum);
> > return;
> > }
>
> Nit: "checksum" is not used beyond this point. It should be
> possible to put a few things in distinct functions to save
> some local variables.
>
I would separate this into another function.
> > + netif_info(tp, probe, dev, "firmware: %s\n", f_info->version);
> > +
> > + phytable = (__le32 *)(fw->data + f_info->fw_start);
> > + fw_size = f_info->fw_len;
> > +
> > for (index = 0; index < fw_size; index++) {
>
> It's a bit paranoid but I would feel safer if f_info->fw_len
> was compared to fw->size beforehand.
>
You are right. I would do it.
> [...]
> > @@ -1892,14 +1913,6 @@ static void
> rtl_apply_firmware(struct rtl8169_private *tp)
> > rtl_phy_write_fw(tp, fw);
> > }
> >
> > -static void rtl_apply_firmware_cond(struct rtl8169_private *tp, u8
> > reg, u16 val) -{
> > - if (rtl_readphy(tp, reg) != val)
> > - netif_warn(tp, hw, tp->dev, "chipset not ready
> for firmware\n");
> > - else
> > - rtl_apply_firmware(tp);
> > -}
> > -
> > static void rtl8169s_hw_phy_config(struct rtl8169_private *tp) {
> > static const struct phy_reg phy_reg_init[] = { @@
> -2334,10 +2347,7
> > @@ static void rtl8168d_1_hw_phy_config(struct rtl8169_private *tp)
> > rtl_w1w0_phy(tp, 0x02, 0x0100, 0x0600);
> > rtl_w1w0_phy(tp, 0x03, 0x0000, 0xe000);
> >
> > - rtl_writephy(tp, 0x1f, 0x0005);
> > - rtl_writephy(tp, 0x05, 0x001b);
> > -
> > - rtl_apply_firmware_cond(tp, MII_EXPANSION, 0xbf00);
> > + rtl_apply_firmware(tp);
> >
> > rtl_writephy(tp, 0x1f, 0x0000);
> > }
> > @@ -2437,10 +2447,7 @@ static void
> rtl8168d_2_hw_phy_config(struct rtl8169_private *tp)
> > rtl_writephy(tp, 0x1f, 0x0002);
> > rtl_patchphy(tp, 0x0f, 0x0017);
> >
> > - rtl_writephy(tp, 0x1f, 0x0005);
> > - rtl_writephy(tp, 0x05, 0x001b);
> > -
> > - rtl_apply_firmware_cond(tp, MII_EXPANSION, 0xb300);
> > + rtl_apply_firmware(tp);
>
> Actually both the format and the content of the firmware are
> changed by the patch.
>
> Imho the new firmware format could include some specific
> string so that the driver can identify the new firmware
> format and fallback to the old format otherwise. Any fixed
> magic prefix would be enough as the new format mandates the
> version information.
>
> This way Linus's test machine won't risk of breaking
> (again...) if he builds a new kernel without updating the
> firmware at the same time.
>
I plan to let the old paser loads the old firmware and the new paser loads the
new firmware. If you build the new kernel without updating the firmware, you
just load no firmware because the new firmware doesn't exist. However, it is
more dangerous for the old paser to load the new firmware. That is why I create
the new ones, not replace the existing files.
Best Regards,
Hayes
^ permalink raw reply
* Re: [PATCH] net/usb: Add Samsung Kalmia driver for Samsung GT-B3730
From: Marius Kotsbak @ 2011-06-14 9:32 UTC (permalink / raw)
To: Oliver Neukum
Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Marius B. Kotsbak
In-Reply-To: <201106141049.39150.oneukum-l3A5Bk7waGM@public.gmane.org>
Den 14. juni 2011 10:49, skrev Oliver Neukum:
> Am Samstag, 11. Juni 2011, 23:55:18 schrieb Marius B. Kotsbak:
>
>> +static int
>> +kalmia_init_and_get_ethernet_addr(struct usbnet *dev, u8 *ethernet_addr)
>> +{
>> + char init_msg_1[] =
>> + { 0x57, 0x50, 0x04, 0x00, 0x00, 0x00, 0x00, 0x20, 0x00, 0x00,
>> + 0x00, 0x00 };
>> + char init_msg_2[] =
>> + { 0x57, 0x50, 0x04, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0xf4,
>> + 0x00, 0x00 };
>> + char receive_buf[28];
> You are doing DMA on the stack. This will fail on some architectures.
>
Okay, a bit strange that this API is available then. Which API functions
should be used in this case?
>> + int status;
>> +
>> + status = kalmia_send_init_packet(dev, init_msg_1, sizeof(init_msg_1)
>> + / sizeof(init_msg_1[0]), receive_buf, 24);
>> + if (status != 0)
>> + return status;
>> +
>> + status = kalmia_send_init_packet(dev, init_msg_2, sizeof(init_msg_2)
>> + / sizeof(init_msg_2[0]), receive_buf, 28);
>> + if (status != 0)
>> + return status;
>> +
>> + memcpy(ethernet_addr, receive_buf + 10, ETH_ALEN);
>> +
>> + return status;
>> +}
>> +
>> +static int
>> +kalmia_bind(struct usbnet *dev, struct usb_interface *intf)
>> +{
>> + u8 status;
>> + u8 ethernet_addr[ETH_ALEN];
>> +
>> + /* Don't bind to AT command interface */
>> + if (intf->cur_altsetting->desc.bInterfaceClass != USB_CLASS_VENDOR_SPEC)
>> + return -EINVAL;
>> +
>> + dev->in = usb_rcvbulkpipe(dev->udev, 0x81& USB_ENDPOINT_NUMBER_MASK);
>> + dev->out = usb_sndbulkpipe(dev->udev, 0x02& USB_ENDPOINT_NUMBER_MASK);
>> + dev->status = NULL;
>> +
>> + dev->net->hard_header_len += KALMIA_HEADER_LENGTH;
>> + dev->hard_mtu = 1400;
>> + dev->rx_urb_size = dev->hard_mtu * 10; // Found as optimal after testing
>> +
>> + status = kalmia_init_and_get_ethernet_addr(dev, ethernet_addr);
>> +
>> + if (status< 0) {
>> + usb_set_intfdata(intf, NULL);
>> + usb_driver_release_interface(driver_of(intf), intf);
>> + return status;
> Why are you doing this? What is to be undone?
You mean the 3 last lines? I just did the same as done in another
similar usbnet based driver. Is it incorrect if the init fails?
>> + }
>> +
>> + memcpy(dev->net->dev_addr, ethernet_addr, ETH_ALEN);
>> + memcpy(dev->net->perm_addr, ethernet_addr, ETH_ALEN);
>> +
>> + return status;
>> +}
>> +
>> +static struct sk_buff *
>> +kalmia_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
>> +{
>> + struct sk_buff *skb2 = NULL;
>> + u16 content_len;
>> + unsigned char *header_start;
>> + unsigned char ether_type_1, ether_type_2;
>> + u8 remainder, padlen = 0;
>> +
>> + if (!skb_cloned(skb)) {
>> + int headroom = skb_headroom(skb);
>> + int tailroom = skb_tailroom(skb);
>> +
>> + if ((tailroom>= KALMIA_ALIGN_SIZE)&& (headroom
>> + >= KALMIA_HEADER_LENGTH))
>> + goto done;
>> +
>> + if ((headroom + tailroom)> (KALMIA_HEADER_LENGTH
>> + + KALMIA_ALIGN_SIZE)) {
>> + skb->data = memmove(skb->head + KALMIA_HEADER_LENGTH,
>> + skb->data, skb->len);
>> + skb_set_tail_pointer(skb, skb->len);
>> + goto done;
>> + }
>> + }
>> +
>> + skb2 = skb_copy_expand(skb, KALMIA_HEADER_LENGTH,
>> + KALMIA_ALIGN_SIZE, flags);
>> + if (!skb2)
>> + return NULL;
>> +
>> + dev_kfree_skb_any(skb);
>> + skb = skb2;
>> +
>> + done: header_start = skb_push(skb, KALMIA_HEADER_LENGTH);
> coding style
The last line? I see.
>> + ether_type_1 = header_start[KALMIA_HEADER_LENGTH + 12];
>> + ether_type_2 = header_start[KALMIA_HEADER_LENGTH + 13];
>> +
>> + netdev_dbg(dev->net, "Sending etherType: %02x%02x", ether_type_1,
>> + ether_type_2);
>> +
>> + /* According to empiric data for data packages */
>> + header_start[0] = 0x57;
>> + header_start[1] = 0x44;
>> + content_len = skb->len - KALMIA_HEADER_LENGTH;
>> + header_start[2] = (content_len& 0xff); /* low byte */
>> + header_start[3] = (content_len>> 8); /* high byte */
> Please use an endianness macro
Wil fix. It will be very relevant to support other architectures for
this driver, as I want to run it on a ARM based NAS as LTE router.
>> + header_start[4] = ether_type_1;
>> + header_start[5] = ether_type_2;
>> +
>> + /* Align to 4 bytes by padding with zeros */
>> + remainder = skb->len % KALMIA_ALIGN_SIZE;
>> + if (remainder> 0) {
>> + padlen = KALMIA_ALIGN_SIZE - remainder;
>> + memset(skb_put(skb, padlen), 0, padlen);
>> + }
>> +
>> + netdev_dbg(
>> + dev->net,
>> + "Sending package with length %i and padding %i. Header: %02x:%02x:%02x:%02x:%02x:%02x.",
>> + content_len, padlen, header_start[0], header_start[1],
>> + header_start[2], header_start[3], header_start[4],
>> + header_start[5]);
>> +
>> + return skb;
>> +}
>> +
>> +static int
>> +kalmia_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
>> +{
>> + /*
>> + * Our task here is to strip off framing, leaving skb with one
>> + * data frame for the usbnet framework code to process.
>> + */
>> + const u8 HEADER_END_OF_USB_PACKET[] =
>> + { 0x57, 0x5a, 0x00, 0x00, 0x08, 0x00 };
>> + const u8 EXPECTED_UNKNOWN_HEADER_1[] =
>> + { 0x57, 0x43, 0x1e, 0x00, 0x15, 0x02 };
>> + const u8 EXPECTED_UNKNOWN_HEADER_2[] =
>> + { 0x57, 0x50, 0x0e, 0x00, 0x00, 0x00 };
> What does the compiler do to this declaration?
Not sure I understand what you mean here.
>> + u8 i = 0;
> Why not int?
I can change that. Thought int was not to be used because it is of
unknown size.
>> +
>> + /* incomplete header? */
>> + if (skb->len< KALMIA_HEADER_LENGTH)
>> + return 0;
>> +
>> + do {
>> + struct sk_buff *skb2 = NULL;
>> + u8 *header_start;
>> + u16 usb_packet_length, ether_packet_length;
>> + int is_last;
>> +
>> + header_start = skb->data;
>> +
>> + if (unlikely(header_start[0] != 0x57 || header_start[1] != 0x44)) {
>> + if (!memcmp(header_start, EXPECTED_UNKNOWN_HEADER_1,
>> + sizeof(EXPECTED_UNKNOWN_HEADER_1)) || !memcmp(
>> + header_start, EXPECTED_UNKNOWN_HEADER_2,
>> + sizeof(EXPECTED_UNKNOWN_HEADER_2))) {
>> + netdev_dbg(
>> + dev->net,
>> + "Received expected unknown frame header: %02x:%02x:%02x:%02x:%02x:%02x. Package length: %i\n",
>> + header_start[0], header_start[1],
>> + header_start[2], header_start[3],
>> + header_start[4], header_start[5],
>> + skb->len - KALMIA_HEADER_LENGTH);
>> + }
>> + else {
>> + netdev_err(
>> + dev->net,
>> + "Received unknown frame header: %02x:%02x:%02x:%02x:%02x:%02x. Package length: %i\n",
>> + header_start[0], header_start[1],
>> + header_start[2], header_start[3],
>> + header_start[4], header_start[5],
>> + skb->len - KALMIA_HEADER_LENGTH);
>> + return 0;
>> + }
>> + }
>> + else
>> + netdev_dbg(
>> + dev->net,
>> + "Received header: %02x:%02x:%02x:%02x:%02x:%02x. Package length: %i\n",
>> + header_start[0], header_start[1], header_start[2],
>> + header_start[3], header_start[4], header_start[5],
>> + skb->len - KALMIA_HEADER_LENGTH);
>> +
>> + /* subtract start header and end header */
>> + usb_packet_length = skb->len - (2 * KALMIA_HEADER_LENGTH);
>> + ether_packet_length = header_start[2] + (header_start[3]<< 8);
> Please use an endianness macro
Oki.
>> + skb_pull(skb, KALMIA_HEADER_LENGTH);
>> +
>> + /* Some small packets misses end marker */
>> + if (usb_packet_length< ether_packet_length) {
>> + ether_packet_length = usb_packet_length
>> + + KALMIA_HEADER_LENGTH;
>> + is_last = true;
>> + }
>> + else {
>> + netdev_dbg(dev->net, "Correct package length #%i", i
>> + + 1);
>> +
>> + is_last = (memcmp(skb->data + ether_packet_length,
>> + HEADER_END_OF_USB_PACKET,
>> + sizeof(HEADER_END_OF_USB_PACKET)) == 0);
>> + if (!is_last) {
>> + header_start = skb->data + ether_packet_length;
>> + netdev_dbg(
>> + dev->net,
>> + "End header: %02x:%02x:%02x:%02x:%02x:%02x. Package length: %i\n",
>> + header_start[0], header_start[1],
>> + header_start[2], header_start[3],
>> + header_start[4], header_start[5],
>> + skb->len - KALMIA_HEADER_LENGTH);
>> + }
>> + }
>> +
>> + if (is_last) {
>> + skb2 = skb;
>> + }
>> + else {
>> + skb2 = skb_clone(skb, GFP_ATOMIC);
>> + if (unlikely(!skb2))
>> + return 0;
>> + }
>> +
>> + skb_trim(skb2, ether_packet_length);
>> +
>> + if (is_last) {
>> + return 1;
>> + }
>> + else {
>> + usbnet_skb_return(dev, skb2);
>> + skb_pull(skb, ether_packet_length);
>> + }
>> +
>> + i++;
>> + }
>> + while (skb->len);
>> +
>> + return 1;
>> +}
>> +
>> +static const struct driver_info kalmia_info = {
>> + .description = "Samsung Kalmia LTE USB dongle",
>> + .flags = FLAG_WWAN,
>> + .bind = kalmia_bind,
>> + .rx_fixup = kalmia_rx_fixup,
>> + .tx_fixup = kalmia_tx_fixup
>> +};
>> +
>> +/*-------------------------------------------------------------------------*/
>> +
>> +static const struct usb_device_id products[] = {
>> + /* The unswitched USB ID, to get the module auto loaded: */
>> + { USB_DEVICE(0x04e8, 0x689a) },
> Why is this needed? Doesn't the switch trigger an autoload?
Please see the comment from Dan Williams. Will test his proposal.
Anyway it should not do any damage for anyone (I think even the driver
disk is available with this included). The module could in fact also do
the switch into modem mode to avoid the dependency on usb_modeswitch.
--
Marius
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next 2/2] drivers/net: Remove casts of void *
From: Geert Uytterhoeven @ 2011-06-14 9:21 UTC (permalink / raw)
To: Joe Perches
Cc: netdev, Arnaldo Carvalho de Melo, Jay Cliburn, Chris Snook,
Jie Yang, Sathya Perla, Subbu Seetharaman, Ajit Khaparde,
Rasesh Mody, Debashis Dutt, Sjur Braendeland, Divy Le Ray,
Breno Leitao, Amit Kumar Salecha, Ron Mercer, linux-driver,
Anirban Chakraborty, Jon Mason, Solarflare linux maintainers,
Steve Hodgson, Ben Hutchings, Daniele Venzano
In-Reply-To: <486a21b708a9dce0c1c0059a21a2f9b66dac9e3c.1308024069.git.joe@perches.com>
On Tue, Jun 14, 2011 at 06:02, Joe Perches <joe@perches.com> wrote:
> Unnecessary casts of void * clutter the code.
> --- a/drivers/net/a2065.c
> +++ b/drivers/net/a2065.c
> @@ -426,7 +426,7 @@ static irqreturn_t lance_interrupt (int irq, void *dev_id)
> volatile struct lance_regs *ll;
> int csr0;
>
> - dev = (struct net_device *) dev_id;
> + dev = dev_id;
I think it would look better if you merge this line with the line that
defines "dev".
>
> lp = netdev_priv(dev);
> ll = lp->ll;
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] net/usb: Add Samsung Kalmia driver for Samsung GT-B3730
From: Oliver Neukum @ 2011-06-14 8:49 UTC (permalink / raw)
To: Marius B. Kotsbak
Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Marius B. Kotsbak
In-Reply-To: <1307829318-18246-1-git-send-email-marius-iy5w9mehe2BBDgjK7y7TUQ@public.gmane.org>
Am Samstag, 11. Juni 2011, 23:55:18 schrieb Marius B. Kotsbak:
Hi,
thanks for writing a new driver. My comments are included in the quote.
Regards
Oliver
> +static int
> +kalmia_init_and_get_ethernet_addr(struct usbnet *dev, u8 *ethernet_addr)
> +{
> + char init_msg_1[] =
> + { 0x57, 0x50, 0x04, 0x00, 0x00, 0x00, 0x00, 0x20, 0x00, 0x00,
> + 0x00, 0x00 };
> + char init_msg_2[] =
> + { 0x57, 0x50, 0x04, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0xf4,
> + 0x00, 0x00 };
> + char receive_buf[28];
You are doing DMA on the stack. This will fail on some architectures.
> + int status;
> +
> + status = kalmia_send_init_packet(dev, init_msg_1, sizeof(init_msg_1)
> + / sizeof(init_msg_1[0]), receive_buf, 24);
> + if (status != 0)
> + return status;
> +
> + status = kalmia_send_init_packet(dev, init_msg_2, sizeof(init_msg_2)
> + / sizeof(init_msg_2[0]), receive_buf, 28);
> + if (status != 0)
> + return status;
> +
> + memcpy(ethernet_addr, receive_buf + 10, ETH_ALEN);
> +
> + return status;
> +}
> +
> +static int
> +kalmia_bind(struct usbnet *dev, struct usb_interface *intf)
> +{
> + u8 status;
> + u8 ethernet_addr[ETH_ALEN];
> +
> + /* Don't bind to AT command interface */
> + if (intf->cur_altsetting->desc.bInterfaceClass != USB_CLASS_VENDOR_SPEC)
> + return -EINVAL;
> +
> + dev->in = usb_rcvbulkpipe(dev->udev, 0x81 & USB_ENDPOINT_NUMBER_MASK);
> + dev->out = usb_sndbulkpipe(dev->udev, 0x02 & USB_ENDPOINT_NUMBER_MASK);
> + dev->status = NULL;
> +
> + dev->net->hard_header_len += KALMIA_HEADER_LENGTH;
> + dev->hard_mtu = 1400;
> + dev->rx_urb_size = dev->hard_mtu * 10; // Found as optimal after testing
> +
> + status = kalmia_init_and_get_ethernet_addr(dev, ethernet_addr);
> +
> + if (status < 0) {
> + usb_set_intfdata(intf, NULL);
> + usb_driver_release_interface(driver_of(intf), intf);
> + return status;
Why are you doing this? What is to be undone?
> + }
> +
> + memcpy(dev->net->dev_addr, ethernet_addr, ETH_ALEN);
> + memcpy(dev->net->perm_addr, ethernet_addr, ETH_ALEN);
> +
> + return status;
> +}
> +
> +static struct sk_buff *
> +kalmia_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
> +{
> + struct sk_buff *skb2 = NULL;
> + u16 content_len;
> + unsigned char *header_start;
> + unsigned char ether_type_1, ether_type_2;
> + u8 remainder, padlen = 0;
> +
> + if (!skb_cloned(skb)) {
> + int headroom = skb_headroom(skb);
> + int tailroom = skb_tailroom(skb);
> +
> + if ((tailroom >= KALMIA_ALIGN_SIZE) && (headroom
> + >= KALMIA_HEADER_LENGTH))
> + goto done;
> +
> + if ((headroom + tailroom) > (KALMIA_HEADER_LENGTH
> + + KALMIA_ALIGN_SIZE)) {
> + skb->data = memmove(skb->head + KALMIA_HEADER_LENGTH,
> + skb->data, skb->len);
> + skb_set_tail_pointer(skb, skb->len);
> + goto done;
> + }
> + }
> +
> + skb2 = skb_copy_expand(skb, KALMIA_HEADER_LENGTH,
> + KALMIA_ALIGN_SIZE, flags);
> + if (!skb2)
> + return NULL;
> +
> + dev_kfree_skb_any(skb);
> + skb = skb2;
> +
> + done: header_start = skb_push(skb, KALMIA_HEADER_LENGTH);
coding style
> + ether_type_1 = header_start[KALMIA_HEADER_LENGTH + 12];
> + ether_type_2 = header_start[KALMIA_HEADER_LENGTH + 13];
> +
> + netdev_dbg(dev->net, "Sending etherType: %02x%02x", ether_type_1,
> + ether_type_2);
> +
> + /* According to empiric data for data packages */
> + header_start[0] = 0x57;
> + header_start[1] = 0x44;
> + content_len = skb->len - KALMIA_HEADER_LENGTH;
> + header_start[2] = (content_len & 0xff); /* low byte */
> + header_start[3] = (content_len >> 8); /* high byte */
Please use an endianness macro
> + header_start[4] = ether_type_1;
> + header_start[5] = ether_type_2;
> +
> + /* Align to 4 bytes by padding with zeros */
> + remainder = skb->len % KALMIA_ALIGN_SIZE;
> + if (remainder > 0) {
> + padlen = KALMIA_ALIGN_SIZE - remainder;
> + memset(skb_put(skb, padlen), 0, padlen);
> + }
> +
> + netdev_dbg(
> + dev->net,
> + "Sending package with length %i and padding %i. Header: %02x:%02x:%02x:%02x:%02x:%02x.",
> + content_len, padlen, header_start[0], header_start[1],
> + header_start[2], header_start[3], header_start[4],
> + header_start[5]);
> +
> + return skb;
> +}
> +
> +static int
> +kalmia_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
> +{
> + /*
> + * Our task here is to strip off framing, leaving skb with one
> + * data frame for the usbnet framework code to process.
> + */
> + const u8 HEADER_END_OF_USB_PACKET[] =
> + { 0x57, 0x5a, 0x00, 0x00, 0x08, 0x00 };
> + const u8 EXPECTED_UNKNOWN_HEADER_1[] =
> + { 0x57, 0x43, 0x1e, 0x00, 0x15, 0x02 };
> + const u8 EXPECTED_UNKNOWN_HEADER_2[] =
> + { 0x57, 0x50, 0x0e, 0x00, 0x00, 0x00 };
What does the compiler do to this declaration?
> + u8 i = 0;
Why not int?
> +
> + /* incomplete header? */
> + if (skb->len < KALMIA_HEADER_LENGTH)
> + return 0;
> +
> + do {
> + struct sk_buff *skb2 = NULL;
> + u8 *header_start;
> + u16 usb_packet_length, ether_packet_length;
> + int is_last;
> +
> + header_start = skb->data;
> +
> + if (unlikely(header_start[0] != 0x57 || header_start[1] != 0x44)) {
> + if (!memcmp(header_start, EXPECTED_UNKNOWN_HEADER_1,
> + sizeof(EXPECTED_UNKNOWN_HEADER_1)) || !memcmp(
> + header_start, EXPECTED_UNKNOWN_HEADER_2,
> + sizeof(EXPECTED_UNKNOWN_HEADER_2))) {
> + netdev_dbg(
> + dev->net,
> + "Received expected unknown frame header: %02x:%02x:%02x:%02x:%02x:%02x. Package length: %i\n",
> + header_start[0], header_start[1],
> + header_start[2], header_start[3],
> + header_start[4], header_start[5],
> + skb->len - KALMIA_HEADER_LENGTH);
> + }
> + else {
> + netdev_err(
> + dev->net,
> + "Received unknown frame header: %02x:%02x:%02x:%02x:%02x:%02x. Package length: %i\n",
> + header_start[0], header_start[1],
> + header_start[2], header_start[3],
> + header_start[4], header_start[5],
> + skb->len - KALMIA_HEADER_LENGTH);
> + return 0;
> + }
> + }
> + else
> + netdev_dbg(
> + dev->net,
> + "Received header: %02x:%02x:%02x:%02x:%02x:%02x. Package length: %i\n",
> + header_start[0], header_start[1], header_start[2],
> + header_start[3], header_start[4], header_start[5],
> + skb->len - KALMIA_HEADER_LENGTH);
> +
> + /* subtract start header and end header */
> + usb_packet_length = skb->len - (2 * KALMIA_HEADER_LENGTH);
> + ether_packet_length = header_start[2] + (header_start[3] << 8);
Please use an endianness macro
> + skb_pull(skb, KALMIA_HEADER_LENGTH);
> +
> + /* Some small packets misses end marker */
> + if (usb_packet_length < ether_packet_length) {
> + ether_packet_length = usb_packet_length
> + + KALMIA_HEADER_LENGTH;
> + is_last = true;
> + }
> + else {
> + netdev_dbg(dev->net, "Correct package length #%i", i
> + + 1);
> +
> + is_last = (memcmp(skb->data + ether_packet_length,
> + HEADER_END_OF_USB_PACKET,
> + sizeof(HEADER_END_OF_USB_PACKET)) == 0);
> + if (!is_last) {
> + header_start = skb->data + ether_packet_length;
> + netdev_dbg(
> + dev->net,
> + "End header: %02x:%02x:%02x:%02x:%02x:%02x. Package length: %i\n",
> + header_start[0], header_start[1],
> + header_start[2], header_start[3],
> + header_start[4], header_start[5],
> + skb->len - KALMIA_HEADER_LENGTH);
> + }
> + }
> +
> + if (is_last) {
> + skb2 = skb;
> + }
> + else {
> + skb2 = skb_clone(skb, GFP_ATOMIC);
> + if (unlikely(!skb2))
> + return 0;
> + }
> +
> + skb_trim(skb2, ether_packet_length);
> +
> + if (is_last) {
> + return 1;
> + }
> + else {
> + usbnet_skb_return(dev, skb2);
> + skb_pull(skb, ether_packet_length);
> + }
> +
> + i++;
> + }
> + while (skb->len);
> +
> + return 1;
> +}
> +
> +static const struct driver_info kalmia_info = {
> + .description = "Samsung Kalmia LTE USB dongle",
> + .flags = FLAG_WWAN,
> + .bind = kalmia_bind,
> + .rx_fixup = kalmia_rx_fixup,
> + .tx_fixup = kalmia_tx_fixup
> +};
> +
> +/*-------------------------------------------------------------------------*/
> +
> +static const struct usb_device_id products[] = {
> + /* The unswitched USB ID, to get the module auto loaded: */
> + { USB_DEVICE(0x04e8, 0x689a) },
Why is this needed? Doesn't the switch trigger an autoload?
> + /* The stick swithed into modem (by e.g. usb_modeswitch): */
> + { USB_DEVICE(0x04e8, 0x6889),
> + .driver_info = (unsigned long) &kalmia_info, },
> + { /* EMPTY == end of list */} };
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH net-next 1/2] wireless: Remove casts of void *
From: Bing Zhao @ 2011-06-14 8:46 UTC (permalink / raw)
To: Joe Perches, netdev@vger.kernel.org
Cc: Luis R. Rodriguez, Jouni Malinen, Vasanthakumar Thiagarajan,
Senthil Balasubramanian, John W. Linville, Christian Lamparter,
Stanislaw Gruszka, Wey-Yi Guy, Intel Linux Wireless,
linux-wireless@vger.kernel.org, ath9k-devel@lists.ath9k.org,
linux-kernel@vger.kernel.org
In-Reply-To: <337c22b774ff7f007b90b266b25c9a33ff555c48.1308024069.git.joe@perches.com>
> - pt = (T *)pv;
> + pt = pv;
>
> Signed-off-by: Joe Perches joe@perches.com
...
> drivers/net/wireless/mwifiex/11n.c | 5 ++---
> drivers/net/wireless/mwifiex/cmdevt.c | 3 +--
> drivers/net/wireless/mwifiex/join.c | 2 +-
> drivers/net/wireless/mwifiex/scan.c | 2 +-
> drivers/net/wireless/mwifiex/sta_cmd.c | 4 ++--
> drivers/net/wireless/mwifiex/sta_cmdresp.c | 12 ++++++------
> drivers/net/wireless/mwifiex/txrx.c | 2 +-
> drivers/net/wireless/p54/eeprom.c | 4 ++--
> 22 files changed, 41 insertions(+), 47 deletions(-)
For mwifiex part, Acked-by: Bing Zhao bzhao@marvell.com
Thanks,
Bing
^ permalink raw reply
* RE: [PATCH net-next 1/2] wireless: Remove casts of void *
From: Bing Zhao @ 2011-06-14 8:45 UTC (permalink / raw)
To: Julian Calaby, Joe Perches
Cc: netdev@vger.kernel.org, Luis R. Rodriguez, Jouni Malinen,
Vasanthakumar Thiagarajan, Senthil Balasubramanian,
John W. Linville, Christian Lamparter, Stanislaw Gruszka,
Wey-Yi Guy, Intel Linux Wireless, linux-wireless@vger.kernel.org,
ath9k-devel@lists.ath9k.org, linux-kernel@vger.kernel.org
In-Reply-To: <BANLkTik4fWbCc8pRZfKCzvKD2f6sK9QCEw@mail.gmail.com>
>> diff --git a/drivers/net/wireless/mwifiex/11n.c b/drivers/net/wireless/mwifiex/11n.c
>> @@ -191,7 +191,7 @@ int mwifiex_ret_11n_cfg(struct host_cmd_ds_command *resp, void *data_buf)
>> struct host_cmd_ds_11n_cfg *htcfg = &resp->params.htcfg;
>>
>> if (data_buf) {
>> - tx_cfg = (struct mwifiex_ds_11n_tx_cfg *) data_buf;
>> + tx_cfg = data_buf;
>>
>> I think pretty useless. tx_cfg is a struct mwifiex_ds_11n_tx_cfg *.
>
> True, but IMHO, this documents, particularly in a random snippet like
> this, what's going on. Arguably though, a better fix would be to move
> the cast to the place where this function is called.
Hi Julian,
Thanks for your comment. We will work on this.
Regards,
Bing
^ permalink raw reply
* Re: [PATCH net-next 2/2] drivers/net: Remove casts of void *
From: Daniele Venzano @ 2011-06-14 8:18 UTC (permalink / raw)
To: Joe Perches
Cc: netdev, Arnaldo Carvalho de Melo, Jay Cliburn, Chris Snook,
Jie Yang, Sathya Perla, Subbu Seetharaman, Ajit Khaparde,
Rasesh Mody, Debashis Dutt, Sjur Braendeland, Divy Le Ray,
Breno Leitao, Amit Kumar Salecha, Ron Mercer, linux-driver,
Anirban Chakraborty, Jon Mason, Solarflare linux maintainers,
Steve Hodgson, Ben Hutchings, David Dillow
In-Reply-To: <486a21b708a9dce0c1c0059a21a2f9b66dac9e3c.1308024069.git.joe@perches.com>
2011/6/14 Joe Perches <joe@perches.com>:
> Unnecessary casts of void * clutter the code.
> diff --git a/drivers/net/sis900.c b/drivers/net/sis900.c
> index 484f795..658a192 100644
> --- a/drivers/net/sis900.c
> +++ b/drivers/net/sis900.c
> @@ -482,7 +482,7 @@ static int __devinit sis900_probe(struct pci_dev *pci_dev,
> ret = -ENOMEM;
> goto err_out_cleardev;
> }
> - sis_priv->tx_ring = (BufferDesc *)ring_space;
> + sis_priv->tx_ring = ring_space;
> sis_priv->tx_ring_dma = ring_dma;
>
> ring_space = pci_alloc_consistent(pci_dev, RX_TOTAL_SIZE, &ring_dma);
> @@ -490,7 +490,7 @@ static int __devinit sis900_probe(struct pci_dev *pci_dev,
> ret = -ENOMEM;
> goto err_unmap_tx;
> }
> - sis_priv->rx_ring = (BufferDesc *)ring_space;
> + sis_priv->rx_ring = ring_space;
> sis_priv->rx_ring_dma = ring_dma;
>
> /* The SiS900-specific entries in the device structure. */
sis900 looks good, too.
--
Daniele Venzano
venza@brownhat.org
http://www.brownhat.org
^ permalink raw reply
* Re: [PATCH net-next 2/2] drivers/net: Remove casts of void *
From: Debashis Dutt @ 2011-06-14 7:56 UTC (permalink / raw)
To: Joe Perches
Cc: netdev, Arnaldo Carvalho de Melo, Jay Cliburn, Chris Snook,
Jie Yang, Sathya Perla, Subbu Seetharaman, Ajit Khaparde,
Rasesh Mody, Debashis Dutt, Sjur Braendeland, Divy Le Ray,
Breno Leitao, Amit Kumar Salecha, Ron Mercer, linux-driver,
Anirban Chakraborty, Jon Mason, Solarflare linux maintainers,
Steve Hodgson, Ben Hutchings, Daniele Venzano
In-Reply-To: <486a21b708a9dce0c1c0059a21a2f9b66dac9e3c.1308024069.git.joe@perches.com>
> diff --git a/drivers/net/bna/bfa_cee.c b/drivers/net/bna/bfa_cee.c
> index f7b789a..dcfbf08 100644
> --- a/drivers/net/bna/bfa_cee.c
> +++ b/drivers/net/bna/bfa_cee.c
> @@ -236,7 +236,7 @@ static void
> bfa_cee_hbfail(void *arg)
> {
> struct bfa_cee *cee;
> - cee = (struct bfa_cee *) arg;
> + cee = arg;
>
> if (cee->get_attr_pending == true) {
> cee->get_attr_status = BFA_STATUS_FAILED;
> diff --git a/drivers/net/bna/cna.h b/drivers/net/bna/cna.h
> index bbd39dc..3c47dc5 100644
> --- a/drivers/net/bna/cna.h
> +++ b/drivers/net/bna/cna.h
> @@ -74,7 +74,7 @@ typedef struct mac { u8 mac[MAC_ADDRLEN]; } mac_t;
> bfa_q_next(_q) = bfa_q_next(*((struct list_head **) _qe)); \
> bfa_q_qe_init(*((struct list_head **) _qe)); \
> } else { \
> - *((struct list_head **) (_qe)) = (struct list_head *) NULL; \
> + *((struct list_head **)(_qe)) = NULL; \
> } \
> }
>
"bna" patches look good.
Thanks
--Debashis
^ permalink raw reply
* Re: [PATCH net-next 1/2] wireless: Remove casts of void *
From: Joe Perches @ 2011-06-14 7:19 UTC (permalink / raw)
To: Julian Calaby
Cc: netdev, Luis R. Rodriguez, Jouni Malinen,
Vasanthakumar Thiagarajan, Senthil Balasubramanian,
John W. Linville, Christian Lamparter, Stanislaw Gruszka,
Wey-Yi Guy, Intel Linux Wireless, linux-wireless, ath9k-devel,
linux-kernel
In-Reply-To: <BANLkTik4fWbCc8pRZfKCzvKD2f6sK9QCEw@mail.gmail.com>
On Tue, 2011-06-14 at 15:39 +1000, Julian Calaby wrote:
> My issue here is that you mention in the commit log that this relates
> to the removal of netdev_priv and dev_priv, but the casts removed
> (mostly) don't.
Sorry if that was unclear to you.
None of these patches remove casts of netdev_priv.
What I meant to say was that net and drivers/net have
had many patches that remove casts of void * like the
patches that removed casts of netdev_priv.
These patches remove the last casts of void * types
in net and drivers/net.
cheers, Joe
^ permalink raw reply
* Re: Re: [RFC PATCH 1/1] IPVS netns shutdown/startup dead-lock
From: Julian Anastasov @ 2011-06-14 6:36 UTC (permalink / raw)
To: Hans Schillstrom
Cc: Hans Schillstrom, horms, wensong, lvs-devel, netdev,
netfilter-devel
In-Reply-To: <oth2obo.8109e7bd386ff72c34c0bda4b6945edf@obelix.schillstrom.com>
Hello,
On Tue, 14 Jun 2011, Hans Schillstrom wrote:
> >On Mon, 13 Jun 2011, Hans Schillstrom wrote:
> >
> >> ip_vs_mutext is used by both netns shutdown code and startup
> >> and both implicit uses sk_lock-AF_INET mutex.
> >>
> >> cleanup CPU-1 startup CPU-2
> >> ip_vs_dst_event() ip_vs_genl_set_cmd()
> >> sk_lock-AF_INET __ip_vs_mutex
> >> sk_lock-AF_INET
> >> __ip_vs_mutex
> >> * DEAD LOCK *
> >
> > So, sk_lock-AF_INET is locked before calling
> >ip_vs_dst_event ? Do you have a backtrace for this case?
>
> Yes plenty this one is with lockdep
>
> Chain exists of:
> rtnl_mutex --> __ip_vs_mutex --> sk_lock-AF_INET
>
> Possible unsafe locking scenario:
>
> CPU0 CPU1
> ---- ----
> lock(sk_lock-AF_INET);
> lock(__ip_vs_mutex);
> lock(sk_lock-AF_INET);
> lock(rtnl_mutex);
>
> *** DEADLOCK ***
>
> 3 locks held by ipvsadm/993:
> #0: (genl_mutex){+.+.+.}, at: [<ffffffff812edc52>] genl_lock+0x17/0x19
> #1: (__ip_vs_mutex){+.+.+.}, at: [<ffffffff81307dcb>] ip_vs_genl_set_cmd+0xe1/0x3a3
> #2: (sk_lock-AF_INET){+.+.+.}, at: [<ffffffff8130ffc1>] start_sync_thread+0x3ec/0x5ff
I see
> >> ip_vs_mutex per name-space seems to be a more future proof solution.
> >
> > Global mutex protects some global lists such as
> >virtual services. If your patch works, better way to fix this problem
> >is to use some new mutex. May be we can move the IPVS_CMD_NEW_DAEMON,
> >IPVS_CMD_DEL_DAEMON and IP_VS_SO_GET_DAEMON code before the
> >__ip_vs_mutex locking. This mutex should be used for start_sync_thread,
> >stop_sync_thread, ip_vs_genl_dump_daemons and IP_VS_SO_GET_DAEMON.
> >For example, ip_vs_sync_mutex.
>
> I think we should avoid global mutexes as a rule of tumb,
> because it's realy hard to keep track of all possible cases
> that can occur when multiple netns is alive and/or goes up and down.
>
> There might be more suprises while a netns exits (in terms of locks)...
> my gut feeling is, avoid global locks as long as possible.
There should not be a problem between two netns when
using global mutexes. And there are no many places in IPVS
where other modules are accessed.
> > Note that __ip_vs_sync_cleanup is missing a
> >__ip_vs_mutex lock. We have to use the new mutex there.
>
> OK
>
> >
> >> Which one should be used ?
> >
> > For now __ip_vs_mutex should be global ...
>
> I do agree, but in the long term I vote for mutex per netns.
It will not help because the problem does not happen
between two netspaces but between ipvs and other modules.
The same problem would happen even if __ip_vs_mutex was
pernet mutex. So, lets try with new mutex.
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [PATCH net-next 1/2] wireless: Remove casts of void *
From: Julian Calaby @ 2011-06-14 5:39 UTC (permalink / raw)
To: Joe Perches
Cc: Intel Linux Wireless, netdev, linux-wireless, John W. Linville,
Jouni Malinen, Senthil Balasubramanian, ath9k-devel, Wey-Yi Guy,
Vasanthakumar Thiagarajan, Christian Lamparter, Stanislaw Gruszka,
linux-kernel
In-Reply-To: <1308029532.26699.68.camel@Joe-Laptop>
Joe,
On Tue, Jun 14, 2011 at 15:32, Joe Perches <joe@perches.com> wrote:
> On Tue, 2011-06-14 at 15:23 +1000, Julian Calaby wrote:
>> Joe,
>
> Hi Julian.
>
>> On Tue, Jun 14, 2011 at 14:02, Joe Perches <joe@perches.com> wrote:
>> > Unnecessary casts of void * clutter the code.
>> > These are the remainder casts after several specific
>> > patches to remove netdev_priv and dev_priv.
>> You seem to have removed a lot of casts that don't relate to these cleanups.
>> In particular, some of the casts seem to relate more to documentation
>> rather than just changing pointer types to make the compiler happy.
>
> All of the cast removals are casts of void* types.
> I think none of of the casts are useful.
> None of them are required, all are duplicative.
My issue here is that you mention in the commit log that this relates
to the removal of netdev_priv and dev_priv, but the casts removed
(mostly) don't.
>> In
>> particular, I'm referring to the casts describing the different usages
>> of data_buf in mwiflex, and around some pointer math in ath9k.
>
> Can you describe more in detail why you think these are documentary?
>
> This sort of cast:
>
> diff --git a/drivers/net/wireless/mwifiex/11n.c b/drivers/net/wireless/mwifiex/11n.c
> @@ -191,7 +191,7 @@ int mwifiex_ret_11n_cfg(struct host_cmd_ds_command *resp, void *data_buf)
> struct host_cmd_ds_11n_cfg *htcfg = &resp->params.htcfg;
>
> if (data_buf) {
> - tx_cfg = (struct mwifiex_ds_11n_tx_cfg *) data_buf;
> + tx_cfg = data_buf;
>
> I think pretty useless. tx_cfg is a struct mwifiex_ds_11n_tx_cfg *.
True, but IMHO, this documents, particularly in a random snippet like
this, what's going on. Arguably though, a better fix would be to move
the cast to the place where this function is called.
>> Whilst I'm sure that the compiler is smart enough to handle automatic
>> casts between pointer types, some of these, in particular the mwiflex
>> bits, add some documentation to the code.
>
> I think not. Opinions of course can vary.
Of course. I rarely look at full files, so the more information that
can be stuffed into a patch, the better for me. That said, I agree
that all the casts removed are superfluous.
Thanks,
--
Julian Calaby
Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/
^ permalink raw reply
* Re: [PATCH net-next 1/2] wireless: Remove casts of void *
From: Joe Perches @ 2011-06-14 5:32 UTC (permalink / raw)
To: Julian Calaby
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Luis R. Rodriguez, Jouni Malinen,
Vasanthakumar Thiagarajan, Senthil Balasubramanian,
John W. Linville, Christian Lamparter, Stanislaw Gruszka,
Wey-Yi Guy, Intel Linux Wireless,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
ath9k-devel-xDcbHBWguxHbcTqmT+pZeQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <BANLkTi=i30Rn7gpCDbn9KKJucq8CB=hssA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, 2011-06-14 at 15:23 +1000, Julian Calaby wrote:
> Joe,
Hi Julian.
> On Tue, Jun 14, 2011 at 14:02, Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> wrote:
> > Unnecessary casts of void * clutter the code.
> > These are the remainder casts after several specific
> > patches to remove netdev_priv and dev_priv.
> You seem to have removed a lot of casts that don't relate to these cleanups.
> In particular, some of the casts seem to relate more to documentation
> rather than just changing pointer types to make the compiler happy.
All of the cast removals are casts of void* types.
I think none of of the casts are useful.
None of them are required, all are duplicative.
> In
> particular, I'm referring to the casts describing the different usages
> of data_buf in mwiflex, and around some pointer math in ath9k.
Can you describe more in detail why you think these are documentary?
This sort of cast:
diff --git a/drivers/net/wireless/mwifiex/11n.c b/drivers/net/wireless/mwifiex/11n.c
@@ -191,7 +191,7 @@ int mwifiex_ret_11n_cfg(struct host_cmd_ds_command *resp, void *data_buf)
struct host_cmd_ds_11n_cfg *htcfg = &resp->params.htcfg;
if (data_buf) {
- tx_cfg = (struct mwifiex_ds_11n_tx_cfg *) data_buf;
+ tx_cfg = data_buf;
I think pretty useless. tx_cfg is a struct mwifiex_ds_11n_tx_cfg *.
> Whilst I'm sure that the compiler is smart enough to handle automatic
> casts between pointer types, some of these, in particular the mwiflex
> bits, add some documentation to the code.
I think not. Opinions of course can vary.
cheers, Joe
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox