* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
From: Arnd Bergmann @ 2012-08-08 12:39 UTC (permalink / raw)
To: Ian Molton
Cc: David Miller, linux-arm-kernel, andrew, thomas.petazzoni,
ben.dooks, netdev
In-Reply-To: <502252A6.4090409@codethink.co.uk>
On Wednesday 08 August 2012, Ian Molton wrote:
> The SMI / PHY stuff should look very similar, so I'm happy with something
> like:
>
> mdio@2000 {
> #address-cells = <1>;
> #size-cells = <1>;
> device_type = "mdio";
> compatible = "marvell,mv643xx-mdio";
> phy0: ethernet-phy@0 {
> device_type = "ethernet-phy";
> compatible = "marvell,whatever";
> interrupts = <76>;
> interrupt-parent = <&mpic>;
> reg = <0 32>; // Auto probed phy addr
> };
>
> phy1: ethernet-phy@3 {
> device_type = "ethernet-phy";
> compatible = "marvell,whatever";
> interrupts = <77>;
> interrupt-parent = <&mpic>;
> reg = <3 1>; // specified phy addr
> };
>
> ... and so on.
> }
>
> Where we can use the reg parameter to allow auto-probing, by
> specifying a size of 32 (32 phy addrs max).
I don't understand the auto-probed phy address. What is the purpose of that?
If possible, I think we should keep using #size-cells=<0>, which would
make the method you describe impossible. It might still work if you just
leave out the "reg" property for that node.
I also don't understand how the phy driver would locate ethernet-phy@0
on the bus if it does not know the address.
> The ethernet driver itself is more complicated:
>
> We have the following considerations:
>
> * we have one MDIO bus, typically, shared between all the MACs / PHYs.
> * each ethernet device can multiple ports (up to three), each with its
> own MAC/PHY.
> * MAC <-> PHY mapping can be specified, probed (ugh!) or a (gah!)
> mix of the two.
> * existing D-T users, albeit not well documented / code complete.
> * some port address ranges overlap (MIB counters, MCAST / UNICAST
> tables, etc.
>
> The existing ethernet-group idea only works because the current
> platform-device based driver doesnt really do proper resource
> management, and thus the MAC registers are actually mapped by
> the MDIO driver.
>
> I don't think that preserving this bad behaviour is a good idea, which
> leaves us with two choices:
>
> 1) My preferred solution - allow each device to specify up to three
> interrupts, MACs, and PHYs. This is clean in that it doesnt require
> multiply instantiating a driver three times over the same address
> space.
>
> ethernet@2400 {
> compatible = "marvell,mv643xx-eth";
> reg = <0x2400 0x1c00>
> interrupt_parent = <&mpic>;
> ports = <3>;
> interrupts = <4>, <5>, <6>;
> phys = <&phy0>, <&phy1>, <&phy2>;
> };
>
> ethernet@6400 {
> compatible = "marvell,mv643xx-eth";
> reg = <0x6400 0x1c00>
> interrupt_parent = <&mpic>;
> ports = <1>;
> interrupts = <4>;
> phys = <&phy3>;
> };
>
> Note that the address is 2400, not 2000 - since this driver no longer
> would share its address range with the MDIO driver.
>
> This method would require a small amount of rework in the driver to
> set up <n> ports, rather than just one.
This looks quite nice, but it is still very much incompatible with the
existing binding. Obviously we can abandon an existing binding and
introduce a second one for the same hardware, but that should not
be taken lightly.
> 2) Create some kind of pseudo-ethernet group device that manages
> all the work for some sort of lightweight ethernet device, one per
> port. This can never be done cleanly since the port address ranges
> overlap:
>
> pseudo_eth@2400 {
> #address-cells = <1>;
> #size-cells = <0>;
> compatible = "marvell,mv643xx-shared-eth"
> reg = <0x2400 0x1c00>;
>
> ethernet@0 {
> compatible = "marvell,mv643xx-port";
> interrupts = <4>;
> interrupt_parent = <&mpic>;
> phy = <&phy0>;
> };
>
> ethernet@1 {
> compatible = "marvell,mv643xx-port";
> interrupts = <5>;
> interrupt_parent = <&mpic>;
> phy = <&phy1>;
> };
>
> ethernet@2 {
> compatible = "marvell,mv643xx-port";
> interrupts = <6>;
> interrupt_parent = <&mpic>;
> phy = <&phy2>;
> };
> }
> pseudo_eth@6400 {
> #address-cells = <1>;
> #size-cells = <0>;
> compatible = "marvell,mv643xx-shared-eth"
> reg = <0x6400 0x1c00>;
>
> ethernet@0 {
> compatible = "marvell,mv643xx-port";
> interrupts = <4>;
> interrupt_parent = <&mpic>;
> phy = <&phy3>;
> };
> };
This looks almost compatible with the existing binding, which is
good. I would in fact recommend to use the actual "compatible"
strings from the binding. More generally speaking, you should not
use wildcards in those strings anyway, so always use
"marvell,mv64360-eth" instead of "marvell,mv64x60-eth" or
"marvell,mv643xx-eth". If you have multiple chips that are
completely compatible, put use the identifier for the older one.
I don't fully understand your concern with the overlapping
registers, mostly because I still don't know all the combinations
that are actually valid here. Let me try to say what I understood
so far, and you can correct me if that's wrong:
* A system can have multiple instances of an mv64360 ethernet
block, with a register area of 0x2000 bytes.
* Each such block can have three MACs and three PHYs.
* The first 0x400 bytes in the register space control the three
PHYs and the remaining registers control the MACs.
* While this is meant to be used in a way that you assign
the each of the three PHYs to one of the MACs, this is not
always done, and sometimes you use a different PHY (?), or
one from a different instance of the mv64360 ethernet block
on the same SoC?.
Arnd
^ permalink raw reply
* Re: [RFC PATCH] tun: don't zeroize sock->file on detach
From: Stanislav Kinsbursky @ 2012-08-08 12:53 UTC (permalink / raw)
To: davem@davemloft.net
Cc: netdev@vger.kernel.org, ruanzhijie@hotmail.com,
linux-kernel@vger.kernel.org, viro@zeniv.linux.org.uk
In-Reply-To: <20120711114753.24395.53193.stgit@localhost6.localdomain6>
Hi, Dave.
What about this patch?
On Wed, Jul 11, 2012 at 03:48:20PM +0400, Stanislav Kinsbursky wrote:
> This is a fix for bug, introduced in 3.4 kernel by commit
> 1ab5ecb90cb6a3df1476e052f76a6e8f6511cb3d, which, among other things, replaced
> simple sock_put() by sk_release_kernel(). Below is sequence, which leads to
> oops for non-persistent devices:
>
> tun_chr_close()
> tun_detach() <== tun->socket.file = NULL
> tun_free_netdev()
> sk_release_sock()
> sock_release(sock->file == NULL)
> iput(SOCK_INODE(sock)) <== dereference on NULL pointer
>
> This patch just removes zeroing of socket's file from __tun_detach().
> sock_release() will do this.
>
> Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
> ---
> drivers/net/tun.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 987aeef..c1639f3 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -185,7 +185,6 @@ static void __tun_detach(struct tun_struct *tun)
> netif_tx_lock_bh(tun->dev);
> netif_carrier_off(tun->dev);
> tun->tfile = NULL;
> - tun->socket.file = NULL;
> netif_tx_unlock_bh(tun->dev);
^ permalink raw reply
* [PATCH] lpc_eth: remove obsolete ifdefs
From: Roland Stigge @ 2012-08-08 13:18 UTC (permalink / raw)
To: netdev, linux-kernel, davem, edumazet, aletes.xgr, kevin.wells,
srinivas.bakki
Cc: Roland Stigge
The #ifdefs regarding CONFIG_ARCH_LPC32XX_MII_SUPPORT and
CONFIG_ARCH_LPC32XX_IRAM_FOR_NET are obsolete since the symbols have been
removed from Kconfig and replaced by devicetree based configuration.
Signed-off-by: Roland Stigge <stigge@antcom.de>
---
drivers/net/ethernet/nxp/lpc_eth.c | 13 -------------
1 file changed, 13 deletions(-)
--- linux-2.6.orig/drivers/net/ethernet/nxp/lpc_eth.c
+++ linux-2.6/drivers/net/ethernet/nxp/lpc_eth.c
@@ -346,28 +346,15 @@ static phy_interface_t lpc_phy_interface
"phy-mode", NULL);
if (mode && !strcmp(mode, "mii"))
return PHY_INTERFACE_MODE_MII;
- return PHY_INTERFACE_MODE_RMII;
}
-
- /* non-DT */
-#ifdef CONFIG_ARCH_LPC32XX_MII_SUPPORT
- return PHY_INTERFACE_MODE_MII;
-#else
return PHY_INTERFACE_MODE_RMII;
-#endif
}
static bool use_iram_for_net(struct device *dev)
{
if (dev && dev->of_node)
return of_property_read_bool(dev->of_node, "use-iram");
-
- /* non-DT */
-#ifdef CONFIG_ARCH_LPC32XX_IRAM_FOR_NET
- return true;
-#else
return false;
-#endif
}
/* Receive Status information word */
^ permalink raw reply
* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
From: Ian Molton @ 2012-08-08 13:19 UTC (permalink / raw)
To: Arnd Bergmann
Cc: David Miller, linux-arm-kernel, andrew, thomas.petazzoni,
ben.dooks, netdev
In-Reply-To: <201208081239.16778.arnd@arndb.de>
On 08/08/12 13:39, Arnd Bergmann wrote:
> On Wednesday 08 August 2012, Ian Molton wrote:
>> The SMI / PHY stuff should look very similar, so I'm happy with something
>> like:
>>
>> mdio@2000 {
>> #address-cells = <1>;
>> #size-cells = <1>;
>> device_type = "mdio";
>> compatible = "marvell,mv643xx-mdio";
>> phy0: ethernet-phy@0 {
>> device_type = "ethernet-phy";
>> compatible = "marvell,whatever";
>> interrupts = <76>;
>> interrupt-parent = <&mpic>;
>> reg = <0 32>; // Auto probed phy addr
>> };
>>
>> phy1: ethernet-phy@3 {
>> device_type = "ethernet-phy";
>> compatible = "marvell,whatever";
>> interrupts = <77>;
>> interrupt-parent = <&mpic>;
>> reg = <3 1>; // specified phy addr
>> };
>>
>> ... and so on.
>> }
>>
>> Where we can use the reg parameter to allow auto-probing, by
>> specifying a size of 32 (32 phy addrs max).
> I don't understand the auto-probed phy address. What is the purpose of that?
Personally, I think it should die - but the existing driver and a number
of its users actually scan the bus for their PHY.
I doubt the PHY really moves about or is hotplugged by any of them,
and its actually quite a slow process.
> If possible, I think we should keep using #size-cells=<0>, which would
> make the method you describe impossible. It might still work if you just
> leave out the "reg" property for that node.
I can certainly investigate that. I couldn't see any good evidence that
it was a supported mechanism when I looked.
> I also don't understand how the phy driver would locate ethernet-phy@0
> on the bus if it does not know the address.
>
>> The ethernet driver itself is more complicated:
>>
>> We have the following considerations:
>>
>> * we have one MDIO bus, typically, shared between all the MACs / PHYs.
>> * each ethernet device can multiple ports (up to three), each with its
>> own MAC/PHY.
>> * MAC <-> PHY mapping can be specified, probed (ugh!) or a (gah!)
>> mix of the two.
>> * existing D-T users, albeit not well documented / code complete.
>> * some port address ranges overlap (MIB counters, MCAST / UNICAST
>> tables, etc.
>>
>> The existing ethernet-group idea only works because the current
>> platform-device based driver doesnt really do proper resource
>> management, and thus the MAC registers are actually mapped by
>> the MDIO driver.
>>
>> I don't think that preserving this bad behaviour is a good idea, which
>> leaves us with two choices:
>>
>> 1) My preferred solution - allow each device to specify up to three
>> interrupts, MACs, and PHYs. This is clean in that it doesnt require
>> multiply instantiating a driver three times over the same address
>> space.
>>
>> ethernet@2400 {
>> compatible = "marvell,mv643xx-eth";
>> reg = <0x2400 0x1c00>
>> interrupt_parent = <&mpic>;
>> ports = <3>;
>> interrupts = <4>, <5>, <6>;
>> phys = <&phy0>, <&phy1>, <&phy2>;
>> };
>>
>> ethernet@6400 {
>> compatible = "marvell,mv643xx-eth";
>> reg = <0x6400 0x1c00>
>> interrupt_parent = <&mpic>;
>> ports = <1>;
>> interrupts = <4>;
>> phys = <&phy3>;
>> };
>>
>> Note that the address is 2400, not 2000 - since this driver no longer
>> would share its address range with the MDIO driver.
>>
>> This method would require a small amount of rework in the driver to
>> set up <n> ports, rather than just one.
> This looks quite nice, but it is still very much incompatible with the
> existing binding. Obviously we can abandon an existing binding and
> introduce a second one for the same hardware, but that should not
> be taken lightly.
Fair, however the existing users aren't anywhere near as
numerous as the new ones.
>> 2) Create some kind of pseudo-ethernet group device that manages
>> all the work for some sort of lightweight ethernet device, one per
>> port. This can never be done cleanly since the port address ranges
>> overlap:
>>
>> pseudo_eth@2400 {
>> #address-cells = <1>;
>> #size-cells = <0>;
>> compatible = "marvell,mv643xx-shared-eth"
>> reg = <0x2400 0x1c00>;
>>
>> ethernet@0 {
>> compatible = "marvell,mv643xx-port";
>> interrupts = <4>;
>> interrupt_parent = <&mpic>;
>> phy = <&phy0>;
>> };
>>
>> ethernet@1 {
>> compatible = "marvell,mv643xx-port";
>> interrupts = <5>;
>> interrupt_parent = <&mpic>;
>> phy = <&phy1>;
>> };
>>
>> ethernet@2 {
>> compatible = "marvell,mv643xx-port";
>> interrupts = <6>;
>> interrupt_parent = <&mpic>;
>> phy = <&phy2>;
>> };
>> }
>> pseudo_eth@6400 {
>> #address-cells = <1>;
>> #size-cells = <0>;
>> compatible = "marvell,mv643xx-shared-eth"
>> reg = <0x6400 0x1c00>;
>>
>> ethernet@0 {
>> compatible = "marvell,mv643xx-port";
>> interrupts = <4>;
>> interrupt_parent = <&mpic>;
>> phy = <&phy3>;
>> };
>> };
> This looks almost compatible with the existing binding, which is
> good.
Well, I'm not sure about that - if the existing bindings are really
baked into firmware, then "almost" wont be any use at all.
> I would in fact recommend to use the actual "compatible"
> strings from the binding. More generally speaking, you should not
> use wildcards in those strings anyway, so always use
> "marvell,mv64360-eth" instead of "marvell,mv64x60-eth" or
> "marvell,mv643xx-eth". If you have multiple chips that are
> completely compatible, put use the identifier for the older one.
Noted.
> I don't fully understand your concern with the overlapping
> registers, mostly because I still don't know all the combinations
> that are actually valid here. Let me try to say what I understood
> so far, and you can correct me if that's wrong:
>
> * A system can have multiple instances of an mv64360 ethernet
> block, with a register area of 0x2000 bytes.
> * Each such block can have three MACs and three PHYs.
> * The first 0x400 bytes in the register space control the three
> PHYs and the remaining registers control the MACs.
> * While this is meant to be used in a way that you assign
> the each of the three PHYs to one of the MACs, this is not
> always done, and sometimes you use a different PHY (?), or
> one from a different instance of the mv64360 ethernet block
> on the same SoC?.
Nearly - the whole block is 0x2000 in size, yes. And each one
can have 3 MACs and PHYs, as you say.
There is SMI @ 0x2000 - just one for all ports, and in many
(all?) cases, for all all the other controllers on the SoC to
share. On the armadaXP SoC, for example, each ethernet
block has its own alias of the same bas SMI reg. (there are
4 blocks)
ethernet0@ 0x2400
## regs in order: Main regs, MIB counters, Special mcast table, Mcast
table, Unicast table.
port0 has regs at +0x0000 *0x1000 +0x1400 +0x1500 +0x1600
port1 has regs at +0x0400 *0x1080 +0x1800 +0x1900 +0x1a00
port2 has regs at +0x0800 *0x1100 +0x1c00 +0x1d00 +0x1e00
ethernet1@ 0x6400
port0 has regs at +0x0000 *0x1000 +0x1400 +0x1500 +0x1600
...
As you can see, instead of putting port1 at +0x1700 or so,
marvell have overlapped the register files - in fact, doubly
so, since port1 + 0x1080 is right in the middle of
(port0 + 0x1000) -> (port0 + 0x16ff), so one cant simply map two
sets of regs like 0x0000->0x03ff and 0x1000->0x16ff for port one
either.
-Ian
^ permalink raw reply
* Re: [PATCH 3/5 (resend)] veth: Allow to create peer link with given ifindex
From: Ben Hutchings @ 2012-08-08 13:25 UTC (permalink / raw)
To: Pavel Emelyanov
Cc: David Miller, Eric Dumazet, Eric W. Biederman, Linux Netdev List
In-Reply-To: <50222AC3.8090801@parallels.com>
On Wed, 2012-08-08 at 13:00 +0400, Pavel Emelyanov wrote:
> On 08/07/2012 10:36 PM, Ben Hutchings wrote:
> > On Tue, 2012-08-07 at 15:02 +0400, Pavel Emelyanov wrote:
> >> The ifinfomsg is in there (thanks kaber@ for foreseeing this long time ago),
> >> so take the given ifidex and register netdev with it.
> >>
> >> Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
> >> ---
> >> drivers/net/veth.c | 3 +++
> >> 1 files changed, 3 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> >> index 5852361..496c026 100644
> >> --- a/drivers/net/veth.c
> >> +++ b/drivers/net/veth.c
> >> @@ -348,6 +348,9 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
> >> if (tbp[IFLA_ADDRESS] == NULL)
> >> eth_hw_addr_random(peer);
> >>
> >> + if (ifmp)
> >> + peer->ifindex = ifmp->ifi_index;
> >> +
> >> err = register_netdevice(peer);
> >> put_net(net);
> >> net = NULL;
> >
> > Is this safe, given that this code path previously ignored
> > ifmp->ifi_index? Userland could be passing in garbage and may now fail
> > occasionally because the value clashes with an existing interface.
>
> You're right, I've missed that fact :( The good news is that we still can
> use the ifmp->ifi_index for the peer index configuration. We just need to
> assume that if the caller specified the ifindex for the veth master device,
> then it's aware of this possibility and should explicitly configure (or set
> to 0) the peer's ifindex as well. Like this:
>
> if (ifmp && (dev->ifindex != 0))
> peer->ifindex = ifmp->ifi_index;
>
> Does this assumption work from you POV?
Yes, that looks like a neat way to do it. Maybe with an explanatory
comment?
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH v2] can: kvaser_usb: Add support for Kvaser CAN/USB devices
From: Olivier Sobrie @ 2012-08-08 13:30 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Marc Kleine-Budde, linux-can-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5022227F.60800-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
On Wed, Aug 08, 2012 at 10:25:35AM +0200, Wolfgang Grandegger wrote:
> Hi Oliver,
>
> On 08/08/2012 08:14 AM, Olivier Sobrie wrote:
> > Hi Wolfgang,
> >
> > On Tue, Aug 07, 2012 at 08:26:38AM +0200, Wolfgang Grandegger wrote:
> >> On 08/06/2012 07:21 AM, Olivier Sobrie wrote:
> >>> This driver provides support for several Kvaser CAN/USB devices.
> >>> Such kind of devices supports up to three can network interfaces.
> >>
> >> s/can/CAN/
> >>
> >>> It has been tested with a Kvaser USB Leaf Light (one network interface)
> >>> connected to a pch_can interface.
> >>> The firmware version of the Kvaser device was 2.5.205.
> >>>
> >>> List of Kvaser devices supported by the driver:
> >>> - Kvaser Leaf prototype (P010v2 and v3)
> >>> - Kvaser Leaf Light (P010v3)
> >>> - Kvaser Leaf Professional HS
> >>> - Kvaser Leaf SemiPro HS
> >>> - Kvaser Leaf Professional LS
> >>> - Kvaser Leaf Professional SWC
> >>> - Kvaser Leaf Professional LIN
> >>> - Kvaser Leaf SemiPro LS
> >>> - Kvaser Leaf SemiPro SWC
> >>> - Kvaser Memorator II, Prototype
> >>> - Kvaser Memorator II HS/HS
> >>> - Kvaser USBcan Professional HS/HS
> >>> - Kvaser Leaf Light GI
> >>> - Kvaser Leaf Professional HS (OBD-II connector)
> >>> - Kvaser Memorator Professional HS/LS
> >>> - Kvaser Leaf Light "China"
> >>> - Kvaser BlackBird SemiPro
> >>> - Kvaser OEM Mercury
> >>> - Kvaser OEM Leaf
> >>> - Kvaser USBcan R
> >>
> >> Impressive list! What CAN controllers are used inside the devices? SJA1000?
> >
> > I took this list from the Kvaser driver. However I only have a Kvaser
> > Leaf Light device thus I'm not sure it will work with other ones.
> > If you prefer I can only let Kvaser Leaf Light instead of the full list.
> > In my device it looks to be a Renesas M16C controller.
>
> OK. Checking the manual, if available, could help to understand how the
> firmware handles bus errors and state changes.
Ok I'll try to find it.
>
> >>> Signed-off-by: Olivier Sobrie <olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public.gmane.org>
> >>> ---
> >>> Changes since v1:
> >>> - added copyrights
> >>> - kvaser_usb.h merged into kvaser.c
> >>> - added kvaser_usb_get_endpoints to find eindpoints instead of
> >>> hardcoding their address
> >>> - some cleanup and comestic changes
> >>> - fixed issues with errors handling
> >>> - fixed restart-ms == 0 case
> >>> - removed do_get_berr_counter method since the hardware doens't return
> >>> good values for txerr and rxerr.
> >>>
> >>> If someone in the linux-usb mailing can review it, it would be nice.
> >>>
> >>> Concerning the errors, it behaves like that now:
> >>>
> >>> 1) Short-circuit CAN-H and CAN-L and restart-ms = 0
> >>>
> >>> t0: short-circuit + 'cansend can1 123#112233'
> >>>
> >>> can1 2000008C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
> >>> controller-problem{rx-error-warning,tx-error-warning}
> >>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> >>> bus-error
> >>> can1 2000008C [8] 00 30 90 00 00 00 00 00 ERRORFRAME
> >>> controller-problem{rx-error-passive,tx-error-passive}
> >>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> >>> bus-error
> >>> can1 200000C8 [8] 00 00 90 00 00 00 00 00 ERRORFRAME
> >>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> >>> bus-off
> >>> bus-error
> >>>
> >>> t1: remove short-circuit + 'ip link set can1 type can restart'
> >>>
> >>> can1 20000100 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
> >>> restarted-after-bus-off
> >>> can1 20000004 [8] 00 0C 00 00 00 00 00 00 ERRORFRAME
> >>> controller-problem{rx-error-warning,tx-error-warning}
> >>
> >> Why do we get the last error message? Maybe the firmware does it that
> >> way (going down passive->warning->active).
> >
> > It goes in that order: warning -> passive -> bus off -> warning
> > -> passive -> ...
>
> Just for curiosity? You don't see back to "error active"?
No but that's maybe because of my misunderstanding of the
M16C_STATE_BUS_ERROR flag.
What I see is:
t1: M16C_STATE_BUS_ERROR
t2: M16C_STATE_BUS_ERROR | M16C_STATE_BUS_PASSIVE
t3: M16C_STATE_BUS_ERROR | M16C_STATE_BUS_OFF
and then again
t4: M16C_STATE_BUS_ERROR
t2: M16C_STATE_BUS_ERROR | M16C_STATE_BUS_PASSIVE
t3: M16C_STATE_BUS_ERROR | M16C_STATE_BUS_OFF
Thus as you suggested below, the flag M16C_STATE_BUS_ERROR might not mean
CAN_STATE_ERROR_WARNING...
>
> >>> 2) Short-circuit CAN-H and CAN-L and restart-ms = 100
> >>>
> >>> t0: short-circuit + cansend can1 123#112233
> >>>
> >>> can1 2000008C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
> >>> controller-problem{rx-error-warning,tx-error-warning}
> >>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> >>> bus-error
> >>> can1 2000008C [8] 00 30 90 00 00 00 00 00 ERRORFRAME
> >>> controller-problem{rx-error-passive,tx-error-passive}
> >>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> >>> bus-error
> >>> can1 200000C8 [8] 00 00 90 00 00 00 00 00 ERRORFRAME
> >>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> >>> bus-off
> >>> bus-error
> >>> can1 2000018C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
> >>> controller-problem{rx-error-warning,tx-error-warning}
> >>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> >>> bus-error
> >>> restarted-after-bus-off
> >>> can1 2000008C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
> >>> controller-problem{rx-error-warning,tx-error-warning}
> >>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> >>> bus-error
> >>> can1 2000008C [8] 00 30 90 00 00 00 00 00 ERRORFRAME
> >>> controller-problem{rx-error-passive,tx-error-passive}
> >>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> >>> bus-error
> >>> can1 200000C8 [8] 00 00 90 00 00 00 00 00 ERRORFRAME
> >>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> >>> bus-off
> >>> bus-error
> >>> ...
> >>>
> >>> can1 2000018C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
> >>> controller-problem{rx-error-warning,tx-error-warning}
> >>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> >>> bus-error
> >>> restarted-after-bus-off
> >>> can1 2000008C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
> >>> controller-problem{rx-error-warning,tx-error-warning}
> >>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> >>> bus-error
> >>> can1 2000008C [8] 00 30 90 00 00 00 00 00 ERRORFRAME
> >>> controller-problem{rx-error-passive,tx-error-passive}
> >>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> >>> bus-error
> >>>
> >>> t1: remove short-circuit
> >>>
> >>> can1 123 [3] 11 22 33
> >>> can1 20000008 [8] 00 00 40 00 00 00 00 00 ERRORFRAME
> >>> protocol-violation{{back-to-error-active}{}}
> >>
> >> The order is still inverted but likely the firmware is doing it that way.
> >
> > Indeed the firmware does it that way: it sends the acknwledge of the
> > frame beofre the state change. I can avoid that behavior by checking the
> > state in the acknowledge frame and send the restart frame if the bus was
> > off.
>
> Well, if the firmware does it wrong, I would not really care. Also,
> could you use timestamping to see if they come close together.
candump can1,0:0,#FFFFFFFF -td -e:
(001.369850) can1 123 [3] 11 22 33
(004.716034) can1 20000008 [8] 00 00 40 00 00 00 00 00 ERRORFRAME
protocol-violation{{back-to-error-active}{}}
Not so close... I'll add something in the tx acknowledge.
>
> >>> 3) CAN-H and CAN-L disconnected
> >>>
> >>> t0: CAN-H and CAN-L disconnected + cansend can1 123#112233
> >>>
> >>> can1 20000004 [8] 00 30 00 00 00 00 00 00 ERRORFRAME
> >>> controller-problem{rx-error-passive,tx-error-passive}
> >>> can1 2000008C [8] 00 30 80 1B 00 00 00 00 ERRORFRAME
> >>> controller-problem{rx-error-passive,tx-error-passive}
> >>> protocol-violation{{error-on-tx}{acknowledge-delimiter}}
> >>> bus-error
> >>>
> >>> t1: CAN-H and CAN-L reconnected
> >>>
> >>> can1 123 [3] 11 22 33
> >>> can1 20000004 [8] 00 30 00 00 00 00 00 00 ERRORFRAME
> >>> controller-problem{rx-error-passive,tx-error-passive}
> >>
> >> Why do we get an error-passive message? Now I will have a closer look to
> >> the code...
> >
> > The firmware sends a CMD_CAN_ERROR_EVENT with the passive bit set...
>
> Maybe the order is again inverted. Do they come at the same time
> (visiable with candump -td).
(002.465349) can1 123 [3] 11 22 33
(004.562670) can1 20000004 [8] 00 30 00 00 00 00 00 00 ERRORFRAME
controller-problem{rx-error-passive,tx-error-passive}
>
> >>> +static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> >>> + const struct kvaser_msg *msg)
> >>> +{
> >>> + struct can_frame *cf;
> >>> + struct sk_buff *skb;
> >>> + struct net_device_stats *stats;
> >>> + struct kvaser_usb_net_priv *priv;
> >>> + unsigned int new_state;
> >>> + u8 channel, status;
> >>> +
> >>> + if (msg->id == CMD_CAN_ERROR_EVENT) {
> >>> + channel = msg->u.error_event.channel;
> >>> + status = msg->u.error_event.status;
> >>> + } else {
> >>> + channel = msg->u.chip_state_event.channel;
> >>> + status = msg->u.chip_state_event.status;
> >>> + }
> >>> +
> >>> + if (channel >= dev->nchannels) {
> >>> + dev_err(dev->udev->dev.parent,
> >>> + "Invalid channel number (%d)\n", channel);
> >>> + return;
> >>> + }
> >>> +
> >>> + priv = dev->nets[channel];
> >>> + stats = &priv->netdev->stats;
> >>> +
> >>> + if (status & M16C_STATE_BUS_RESET) {
> >>> + kvaser_usb_unlink_tx_urbs(priv);
> >>> + return;
> >>> + }
> >>> + skb = alloc_can_err_skb(priv->netdev, &cf);
> >>> + if (!skb) {
> >>> + stats->rx_dropped++;
> >>> + return;
> >>
> >> Cleanup? kvaser_usb_unlink_tx_urbs()?
> >
> > If I get the M16C_STATE_BUS_RESET I'll not receive the ack frames anymore.
> > I need to set the context->echo_index back to MAX_TX_URBS to not loose tx
> > urbs.
> > By the way I think a can_free_echo_skb() is missing...
> >
> >>
> >>> + }
> >>> +
> >>> + if (status & M16C_STATE_BUS_OFF) {
> >>> + cf->can_id |= CAN_ERR_BUSOFF;
> >>> +
> >>> + if (!priv->can.restart_ms)
> >>> + kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
> >>> +
> >>> + if (!priv->can.state != CAN_ERR_BUSOFF) {
> >>> + priv->can.can_stats.bus_off++;
> >>> + netif_carrier_off(priv->netdev);
> >>> + }
> >>> +
> >>> + new_state = CAN_STATE_BUS_OFF;
> >>> + } else if (status & M16C_STATE_BUS_PASSIVE) {
> >>> + cf->can_id |= CAN_ERR_CRTL;
> >>> + cf->data[1] = CAN_ERR_CRTL_TX_PASSIVE |
> >>> + CAN_ERR_CRTL_RX_PASSIVE;
> >>
> >> State changes should only be report when the state really changes.
> >> Therefore it should go under the if block below.
> >
> > Ok. Is it possible to get such sequence:
> > can1 20000088 [8] 00 00 90 00 00 00 00 00 ERRORFRAME
> > can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> > can1 20000088 [8] 00 00 90 00 00 00 00 00 ERRORFRAME
> > can1 20000088 [8] 00 00 90 00 00 00 00 00 ERRORFRAME
> > can1 20000088 [8] 00 00 90 00 00 00 00 00 ERRORFRAME
> >
> > 2 questions:
> > 1) If the bus is still in CAN_ERR_CRTL_RX_PASSIVE after the second frame,
> > shouldn't we let the corresponding bit set, 0x10?
>
> No, see below.
>
> > 2) Can we send multiple times same frame wih same error bits set?
>
> Yes, because that's what the hardware reports.
>
> >>
> >>> + if (priv->can.state != CAN_STATE_ERROR_PASSIVE)
> >>> + priv->can.can_stats.error_passive++;
> >>> +
> >>> + new_state = CAN_STATE_ERROR_PASSIVE;
> >>> + } else if (status & M16C_STATE_BUS_ERROR) {
> >>
> >> Hm, strange, a bus error is not a state change. You use here if...else
> >> if... Isn't it possible that more than one bit is set.
> >
> > Indeed it is possible to have multiple bits set.
> > e.g. M16C_STATE_BUS_PASSIVE + M16C_STATE_BUS_ERROR or M16C_STATE_BUS_OFF + M16C_STATE_BUS_ERROR.
>
> OK, that's the normal behaviour. Obviously they send bus errors together
> with the *actual* state. The hardware does usuallly report bus errors
> frequently while the error condition persists.
>
> > What error should I report in case of M16C_STATE_BUS_ERROR?
>
> To make that clear, I have added an (old) output from the SJA1000, which
> is the defacto reference. Bus error reporting is enabled and no cable is
> connected. Watch the TX error count increasing and how the state changes:
With my hardware I do not get txerr and rxerr... I only get
M16C_STATE_BUS_ERROR. Thus in fact I can only pass from ACTIVE to PASSIVE
without reaching the WARNING state.
People of Kvaser just told me txerr and rxerr was set with other hardwares
than Kvaser Leaf Lighti... That's the one I've... I'm unlucky :-(.
I'll adapt the code so that txerr and rxerr are reported for working
hardwares..
>
> $ ./candump -e 0xffff any
> can0 20000088 [8] 00 00 80 19 00 08 00 00 ERRORFRAME
> \ \ \-- ACK slot.
> \ \-- error occured on transmission
> \-- Bus-error | Protocol violations (data[2], data[3]).
>
> can0 20000088 [8] 00 00 80 19 00 10 00 00 ERRORFRAME
> can0 20000088 [8] 00 00 80 19 00 18 00 00 ERRORFRAME
> can0 20000088 [8] 00 00 80 19 00 20 00 00 ERRORFRAME
> can0 20000088 [8] 00 00 80 19 00 28 00 00 ERRORFRAME
> can0 20000088 [8] 00 00 80 19 00 30 00 00 ERRORFRAME
> can0 20000088 [8] 00 00 80 19 00 38 00 00 ERRORFRAME
> can0 20000088 [8] 00 00 80 19 00 40 00 00 ERRORFRAME
> can0 20000088 [8] 00 00 80 19 00 48 00 00 ERRORFRAME
> can0 20000088 [8] 00 00 80 19 00 50 00 00 ERRORFRAME
> can0 20000088 [8] 00 00 80 19 00 58 00 00 ERRORFRAME
> can0 2000008C [8] 00 08 80 19 00 60 00 00 ERRORFRAME
> \ \-- reached warning level for TX errors
> \-- | Controller problems (see data[1]).
>
> can0 20000088 [8] 00 00 80 19 00 68 00 00 ERRORFRAME
> can0 20000088 [8] 00 00 80 19 00 70 00 00 ERRORFRAME
> can0 20000088 [8] 00 00 80 19 00 78 00 00 ERRORFRAME
> can0 2000008C [8] 00 20 80 19 00 80 00 00 ERRORFRAME
> \ \-- reached passive level for TX errors
> \-- | Controller problems (see data[1]).
>
> can0 20000088 [8] 00 00 80 19 00 80 00 00 ERRORFRAME
> \ \-- RXerror count
> \-- TXerror count
>
> can0 20000088 [8] 00 00 80 19 00 80 00 00 ERRORFRAME
> ...
> can0 20000088 [8] 00 00 80 19 00 80 00 00 ERRORFRAME
Ok. I'll only set CAN_ERR_PROT_LOC_ACK and not CAN_ERR_PROT_LOC_DEL.
>
>
> >
> >>
> >>> + cf->can_id |= CAN_ERR_CRTL;
> >>> + cf->data[1] = CAN_ERR_CRTL_TX_WARNING |
> >>> + CAN_ERR_CRTL_RX_WARNING;
> >>
> >> See above.
> >>
> >>> + if (priv->can.state != CAN_STATE_ERROR_WARNING)
> >>> + priv->can.can_stats.error_warning++;
> >>> +
> >>> + new_state = CAN_STATE_ERROR_WARNING;
> >>> + } else {
> >>> + cf->can_id |= CAN_ERR_PROT;
> >>> + cf->data[2] = CAN_ERR_PROT_ACTIVE;
> >>> +
> >>> + new_state = CAN_STATE_ERROR_ACTIVE;
> >>> + }
> >>> +
> >>> + if (priv->can.restart_ms &&
> >>> + (priv->can.state >= CAN_STATE_BUS_OFF) &&
> >>> + (new_state < CAN_STATE_BUS_OFF)) {
> >>> + cf->can_id |= CAN_ERR_RESTARTED;
> >>> + priv->can.can_stats.restarts++;
> >>> + netif_carrier_on(priv->netdev);
> >>> + }
> >>> +
> >>> + if (msg->id == CMD_CAN_ERROR_EVENT) {
> >>> + u8 error_factor = msg->u.error_event.error_factor;
> >>> +
> >>> + priv->can.can_stats.bus_error++;
> >>> + stats->rx_errors++;
> >>> +
> >>> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> >>> +
> >>> + if (error_factor & M16C_EF_ACKE)
> >>> + cf->data[3] |= (CAN_ERR_PROT_LOC_ACK |
> >>> + CAN_ERR_PROT_LOC_ACK_DEL);
> >>> + if (error_factor & M16C_EF_CRCE)
> >>> + cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
> >>> + CAN_ERR_PROT_LOC_CRC_DEL);
> >>> + if (error_factor & M16C_EF_FORME)
> >>> + cf->data[2] |= CAN_ERR_PROT_FORM;
> >>> + if (error_factor & M16C_EF_STFE)
> >>> + cf->data[2] |= CAN_ERR_PROT_STUFF;
> >>> + if (error_factor & M16C_EF_BITE0)
> >>> + cf->data[2] |= CAN_ERR_PROT_BIT0;
> >>> + if (error_factor & M16C_EF_BITE1)
> >>> + cf->data[2] |= CAN_ERR_PROT_BIT1;
> >>> + if (error_factor & M16C_EF_TRE)
> >>> + cf->data[2] |= CAN_ERR_PROT_TX;
> >>> + }
> >>> +
> >>> + priv->can.state = new_state;
> >>> +
> >>> + if (!memcmp(cf, &priv->cf_err_old, sizeof(*cf))) {
> >>> + kfree_skb(skb);
> >>> + return;
> >>> + }
> >>
> >> Hm, the firmware seems not to clear error conditions? Anyway, state
> >> change and error reporting is magic on many CAN controllers. Just the
> >> SJA1000 is doing it nicely.
> >
> > I added it to prevent sending two times the same error. It happens that
> > the firmware sends multiple times the same error message.
>
> Can then be removed, I think, see above.
Ok.
>
> >>
> >>> + netif_rx(skb);
> >>> +
> >>> + priv->cf_err_old = *cf;
> >>> +
> >>> + stats->rx_packets++;
> >>> + stats->rx_bytes += cf->can_dlc;
> >>> +}
> >>> +
> >>> +static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
> >>> + const struct kvaser_msg *msg)
> >>> +{
> >>> + struct kvaser_usb_net_priv *priv;
> >>> + struct can_frame *cf;
> >>> + struct sk_buff *skb;
> >>> + struct net_device_stats *stats;
> >>> + u8 channel = msg->u.rx_can.channel;
> >>> +
> >>> + if (channel >= dev->nchannels) {
> >>> + dev_err(dev->udev->dev.parent,
> >>> + "Invalid channel number (%d)\n", channel);
> >>> + return;
> >>> + }
> >>> +
> >>> + priv = dev->nets[channel];
> >>> + stats = &priv->netdev->stats;
> >>> +
> >>> + skb = alloc_can_skb(priv->netdev, &cf);
> >>> + if (skb == NULL) {
> >>
> >> s/skb == NULL)/!skb/ ?
> >>
> >>> + stats->tx_dropped++;
> >>> + return;
> >>> + }
> >>> +
> >>> + cf->can_id = ((msg->u.rx_can.msg[0] & 0x1f) << 6) |
> >>> + (msg->u.rx_can.msg[1] & 0x3f);
> >>> + cf->can_dlc = get_can_dlc(msg->u.rx_can.msg[5]);
> >>> +
> >>> + if (msg->id == CMD_RX_EXT_MESSAGE) {
> >>> + cf->can_id <<= 18;
> >>> + cf->can_id |= ((msg->u.rx_can.msg[2] & 0x0f) << 14) |
> >>> + ((msg->u.rx_can.msg[3] & 0xff) << 6) |
> >>> + (msg->u.rx_can.msg[4] & 0x3f);
> >>> + cf->can_id |= CAN_EFF_FLAG;
> >>> + }
> >>> +
> >>> + if (msg->u.rx_can.flag & MSG_FLAG_REMOTE_FRAME) {
> >>> + cf->can_id |= CAN_RTR_FLAG;
> >>> + } else if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
> >>> + MSG_FLAG_NERR)) {
> >>> + cf->can_id |= CAN_ERR_FLAG;
> >>> + cf->can_dlc = CAN_ERR_DLC;
> >>> + cf->data[1] = CAN_ERR_CRTL_UNSPEC;
> >>
> >> What is the meaning of such errors? A comment, netdev_err() or
> >> netdev_dbg() would be nice.
> >
> > I never reached this error with the hardware I've... I don't know what's
> > the meaning of this flag... I will add a trace.
>
> Then add a netdev_err().
Ok.
Thank you,
--
Olivier
--
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: [PATCHv2 1/4] modem_shm: Add Modem Access Framework
From: Greg KH @ 2012-08-08 13:37 UTC (permalink / raw)
To: Arun MURTHY
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-doc@vger.kernel.org, Sjur BRENDELAND
In-Reply-To: <F45880696056844FA6A73F415B568C695B0E796A03@EXDCVYMBSTM006.EQ1STM.local>
On Wed, Aug 08, 2012 at 05:36:05AM +0200, Arun MURTHY wrote:
> > On Tue, Aug 07, 2012 at 12:24:28PM +0530, Arun Murthy wrote:
> > > Adds Modem Access Framework, which allows for registering platform
> > specific
> > > modem access mechanisms. The framework also exposes APIs for client
> > drivers
> > > for getting and releasing access to modem, regardless of the
> > underlying
> > > platform specific access mechanism.
> >
> > The term "modems" here has a lot of legacy connotations. First of
> > which
> > is, userspace handles this today as tty devices, why aren't you doing
> > the same here? Why does this have to be something "special"?
> >
>
> The main focus over there the modem IPC.
Over where?
What "modem IPC"?
You need to really explain what you are doing here, as I have no idea.
And I have no idea why you still can't just use a tty device, why can't
you?
> In doing so, there are some functionality like waking the modem, or
> releasing the modem etc. These will be used by the modem IPC drivers
> and also few others like sim driver and security drivers.
What's a "sim driver", and what type of "security drivers" are you
referring to?
> Since this is a shared call and hence has to be synchronized. Hence so a
> small framework like is being done to monitor the modem access related only
> operations.
Again, why can't the tty layer do this for you?
greg k-h
^ permalink raw reply
* Re: [PATCH 3/5 (resend)] veth: Allow to create peer link with given ifindex
From: Pavel Emelyanov @ 2012-08-08 13:38 UTC (permalink / raw)
To: Ben Hutchings
Cc: David Miller, Eric Dumazet, Eric W. Biederman, Linux Netdev List
In-Reply-To: <1344432330.13142.188.camel@deadeye.wl.decadent.org.uk>
>> You're right, I've missed that fact :( The good news is that we still can
>> use the ifmp->ifi_index for the peer index configuration. We just need to
>> assume that if the caller specified the ifindex for the veth master device,
>> then it's aware of this possibility and should explicitly configure (or set
>> to 0) the peer's ifindex as well. Like this:
>>
>> if (ifmp && (dev->ifindex != 0))
>> peer->ifindex = ifmp->ifi_index;
>>
>> Does this assumption work from you POV?
>
> Yes, that looks like a neat way to do it. Maybe with an explanatory
> comment?
Yes, sure. I will resend the set shortly.
Thanks for the feedback!
> Ben.
>
Thanks,
Pavel
^ permalink raw reply
* [PATCH 00/11] recently added ARM defconfig warnings
From: Arnd Bergmann @ 2012-08-08 14:47 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Takashi Iwai, Linus Walleij, Grant Likely, Laxman Dewangan,
Philipp Zabel, Giuseppe Cavallaro, Stefan Roese, Lee Jones,
Kukjin Kim, Vincent Guittot, Samuel Ortiz, arm, Alan Stern,
Grazvydas Ignotas, Paul Parsons, Russell King, Peter Zijlstra,
Arnd Bergmann, Thomas Abraham, Rafael J. Wysocki, Jaswinder Singh,
Magnus Damm, Namhyung Kim, Greg
These patches address problems that showed up in new warnings
when building all ARM defconfig files in v3.6-rc1 compared
with v3.5.
I can merge them through the arm-soc tree or have subsystem
maintainers pick them up if they prefer.
Arnd Bergmann (11):
ARM: topology: mark init_cpu_topology as __init
mfd/asic3: fix asic3_mfd_probe return value
usb/ohci-omap: remove unused variable
ARM: ux500: really kill snowball_of_platform_devs
ARM: exynos: exynos_pm_add_dev_to_genpd may be unused
gpio: em: do not discard em_gio_irq_domain_cleanup
net/stmmac: mark probe function as __devinit
mtd/omap2: fix dmaengine_slave_config error handling
regulator/twl: remove fixed resource handling
spi/s3c64xx: improve error handling
pm/drivers: fix use of SIMPLE_DEV_PM_OPS
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Grazvydas Ignotas <notasas@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jaswinder Singh <jaswinder.singh@linaro.org>
Cc: Kukjin Kim <kgene.kim@samsung.com>
Cc: Laxman Dewangan <ldewangan@nvidia.com>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Magnus Damm <damm@opensource.se>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Parsons <lost.distance@yahoo.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Philipp Zabel <philipp.zabel@gmail.com>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: Richard Zhao <richard.zhao@freescale.com>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Stefan Roese <sr@denx.de>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Thomas Abraham <thomas.abraham@linaro.org>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: netdev@vger.kernel.org
arch/arm/kernel/topology.c | 2 +-
arch/arm/mach-exynos/pm_domains.c | 2 +-
arch/arm/mach-ux500/board-mop500.c | 5 -----
drivers/char/hw_random/omap-rng.c | 2 +-
drivers/gpio/gpio-em.c | 2 +-
drivers/i2c/busses/i2c-tegra.c | 2 +-
drivers/mfd/asic3.c | 1 +
drivers/mtd/nand/omap2.c | 7 +++----
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 +-
drivers/regulator/twl-regulator.c | 21 --------------------
drivers/spi/spi-s3c64xx.c | 2 +-
drivers/usb/host/ohci-omap.c | 2 --
sound/drivers/dummy.c | 2 +-
13 files changed, 12 insertions(+), 40 deletions(-)
--
1.7.10
^ permalink raw reply
* [PATCH 07/11] net/stmmac: mark probe function as __devinit
From: Arnd Bergmann @ 2012-08-08 14:47 UTC (permalink / raw)
To: linux-arm-kernel
Cc: arm, linux-kernel, Arnd Bergmann, Stefan Roese,
Giuseppe Cavallaro, David S. Miller, netdev
In-Reply-To: <1344437248-20560-1-git-send-email-arnd@arndb.de>
Driver probe functions are generally __devinit so they will be
discarded after initialization for non-hotplug kernels.
This was found by a new warning after patch 6a228452d "stmmac: Add
device-tree support" adds a new __devinit function that is called
from stmmac_pltfr_probe.
Without this patch, building socfpga_defconfig results in:
WARNING: drivers/net/ethernet/stmicro/stmmac/stmmac.o(.text+0x5d4c): Section mismatch in reference from the function stmmac_pltfr_probe() to the function .devinit.text:stmmac_probe_config_dt()
The function stmmac_pltfr_probe() references
the function __devinit stmmac_probe_config_dt().
This is often because stmmac_pltfr_probe lacks a __devinit
annotation or the annotation of stmmac_probe_config_dt is wrong.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Stefan Roese <sr@denx.de>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
---
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index cd01ee7..b93245c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -74,7 +74,7 @@ static int __devinit stmmac_probe_config_dt(struct platform_device *pdev,
* the necessary resources and invokes the main to init
* the net device, register the mdio bus etc.
*/
-static int stmmac_pltfr_probe(struct platform_device *pdev)
+static int __devinit stmmac_pltfr_probe(struct platform_device *pdev)
{
int ret = 0;
struct resource *res;
--
1.7.10
^ permalink raw reply related
* Re: [PATCH v2] can: kvaser_usb: Add support for Kvaser CAN/USB devices
From: Wolfgang Grandegger @ 2012-08-08 15:02 UTC (permalink / raw)
To: Olivier Sobrie
Cc: Marc Kleine-Budde, linux-can-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20120808133005.GA14300@hposo>
On 08/08/2012 03:30 PM, Olivier Sobrie wrote:
> On Wed, Aug 08, 2012 at 10:25:35AM +0200, Wolfgang Grandegger wrote:
>> Hi Oliver,
>>
>> On 08/08/2012 08:14 AM, Olivier Sobrie wrote:
>>> Hi Wolfgang,
>>>
>>> On Tue, Aug 07, 2012 at 08:26:38AM +0200, Wolfgang Grandegger wrote:
>>>> On 08/06/2012 07:21 AM, Olivier Sobrie wrote:
>>>>> This driver provides support for several Kvaser CAN/USB devices.
>>>>> Such kind of devices supports up to three can network interfaces.
>>>>
>>>> s/can/CAN/
>>>>
>>>>> It has been tested with a Kvaser USB Leaf Light (one network interface)
>>>>> connected to a pch_can interface.
>>>>> The firmware version of the Kvaser device was 2.5.205.
>>>>>
>>>>> List of Kvaser devices supported by the driver:
>>>>> - Kvaser Leaf prototype (P010v2 and v3)
>>>>> - Kvaser Leaf Light (P010v3)
>>>>> - Kvaser Leaf Professional HS
>>>>> - Kvaser Leaf SemiPro HS
>>>>> - Kvaser Leaf Professional LS
>>>>> - Kvaser Leaf Professional SWC
>>>>> - Kvaser Leaf Professional LIN
>>>>> - Kvaser Leaf SemiPro LS
>>>>> - Kvaser Leaf SemiPro SWC
>>>>> - Kvaser Memorator II, Prototype
>>>>> - Kvaser Memorator II HS/HS
>>>>> - Kvaser USBcan Professional HS/HS
>>>>> - Kvaser Leaf Light GI
>>>>> - Kvaser Leaf Professional HS (OBD-II connector)
>>>>> - Kvaser Memorator Professional HS/LS
>>>>> - Kvaser Leaf Light "China"
>>>>> - Kvaser BlackBird SemiPro
>>>>> - Kvaser OEM Mercury
>>>>> - Kvaser OEM Leaf
>>>>> - Kvaser USBcan R
>>>>
>>>> Impressive list! What CAN controllers are used inside the devices? SJA1000?
>>>
>>> I took this list from the Kvaser driver. However I only have a Kvaser
>>> Leaf Light device thus I'm not sure it will work with other ones.
>>> If you prefer I can only let Kvaser Leaf Light instead of the full list.
>>> In my device it looks to be a Renesas M16C controller.
>>
>> OK. Checking the manual, if available, could help to understand how the
>> firmware handles bus errors and state changes.
>
> Ok I'll try to find it.
>
>>
>>>>> Signed-off-by: Olivier Sobrie <olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public.gmane.org>
>>>>> ---
>>>>> Changes since v1:
>>>>> - added copyrights
>>>>> - kvaser_usb.h merged into kvaser.c
>>>>> - added kvaser_usb_get_endpoints to find eindpoints instead of
>>>>> hardcoding their address
>>>>> - some cleanup and comestic changes
>>>>> - fixed issues with errors handling
>>>>> - fixed restart-ms == 0 case
>>>>> - removed do_get_berr_counter method since the hardware doens't return
>>>>> good values for txerr and rxerr.
>>>>>
>>>>> If someone in the linux-usb mailing can review it, it would be nice.
>>>>>
>>>>> Concerning the errors, it behaves like that now:
>>>>>
>>>>> 1) Short-circuit CAN-H and CAN-L and restart-ms = 0
>>>>>
>>>>> t0: short-circuit + 'cansend can1 123#112233'
>>>>>
>>>>> can1 2000008C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-warning,tx-error-warning}
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-error
>>>>> can1 2000008C [8] 00 30 90 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-passive,tx-error-passive}
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-error
>>>>> can1 200000C8 [8] 00 00 90 00 00 00 00 00 ERRORFRAME
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-off
>>>>> bus-error
>>>>>
>>>>> t1: remove short-circuit + 'ip link set can1 type can restart'
>>>>>
>>>>> can1 20000100 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
>>>>> restarted-after-bus-off
>>>>> can1 20000004 [8] 00 0C 00 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-warning,tx-error-warning}
>>>>
>>>> Why do we get the last error message? Maybe the firmware does it that
>>>> way (going down passive->warning->active).
>>>
>>> It goes in that order: warning -> passive -> bus off -> warning
>>> -> passive -> ...
>>
>> Just for curiosity? You don't see back to "error active"?
>
> No but that's maybe because of my misunderstanding of the
> M16C_STATE_BUS_ERROR flag.
> What I see is:
> t1: M16C_STATE_BUS_ERROR
> t2: M16C_STATE_BUS_ERROR | M16C_STATE_BUS_PASSIVE
> t3: M16C_STATE_BUS_ERROR | M16C_STATE_BUS_OFF
> and then again
> t4: M16C_STATE_BUS_ERROR
> t2: M16C_STATE_BUS_ERROR | M16C_STATE_BUS_PASSIVE
> t3: M16C_STATE_BUS_ERROR | M16C_STATE_BUS_OFF
>
> Thus as you suggested below, the flag M16C_STATE_BUS_ERROR might not mean
> CAN_STATE_ERROR_WARNING...
Do you see bus error bits set? If not, I could mean "error active",
otherwise "error warning". Meaning the device sends such messages
containing bus error information plus the current state.
>>>>> 2) Short-circuit CAN-H and CAN-L and restart-ms = 100
>>>>>
>>>>> t0: short-circuit + cansend can1 123#112233
>>>>>
>>>>> can1 2000008C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-warning,tx-error-warning}
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-error
>>>>> can1 2000008C [8] 00 30 90 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-passive,tx-error-passive}
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-error
>>>>> can1 200000C8 [8] 00 00 90 00 00 00 00 00 ERRORFRAME
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-off
>>>>> bus-error
>>>>> can1 2000018C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-warning,tx-error-warning}
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-error
>>>>> restarted-after-bus-off
>>>>> can1 2000008C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-warning,tx-error-warning}
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-error
>>>>> can1 2000008C [8] 00 30 90 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-passive,tx-error-passive}
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-error
>>>>> can1 200000C8 [8] 00 00 90 00 00 00 00 00 ERRORFRAME
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-off
>>>>> bus-error
>>>>> ...
>>>>>
>>>>> can1 2000018C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-warning,tx-error-warning}
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-error
>>>>> restarted-after-bus-off
>>>>> can1 2000008C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-warning,tx-error-warning}
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-error
>>>>> can1 2000008C [8] 00 30 90 00 00 00 00 00 ERRORFRAME
>>>>> controller-problem{rx-error-passive,tx-error-passive}
>>>>> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
>>>>> bus-error
>>>>>
>>>>> t1: remove short-circuit
>>>>>
>>>>> can1 123 [3] 11 22 33
>>>>> can1 20000008 [8] 00 00 40 00 00 00 00 00 ERRORFRAME
>>>>> protocol-violation{{back-to-error-active}{}}
>>>>
>>>> The order is still inverted but likely the firmware is doing it that way.
>>>
>>> Indeed the firmware does it that way: it sends the acknwledge of the
>>> frame beofre the state change. I can avoid that behavior by checking the
>>> state in the acknowledge frame and send the restart frame if the bus was
>>> off.
>>
>> Well, if the firmware does it wrong, I would not really care. Also,
>> could you use timestamping to see if they come close together.
>
> candump can1,0:0,#FFFFFFFF -td -e:
> (001.369850) can1 123 [3] 11 22 33
> (004.716034) can1 20000008 [8] 00 00 40 00 00 00 00 00 ERRORFRAME
> protocol-violation{{back-to-error-active}{}}
>
> Not so close... I'll add something in the tx acknowledge.
More than three seconds, wired. I would add netdev_dbg() to the state
change and error handling code to better understand what's going on.
....
>>> What error should I report in case of M16C_STATE_BUS_ERROR?
>>
>> To make that clear, I have added an (old) output from the SJA1000, which
>> is the defacto reference. Bus error reporting is enabled and no cable is
>> connected. Watch the TX error count increasing and how the state changes:
>
> With my hardware I do not get txerr and rxerr... I only get
> M16C_STATE_BUS_ERROR. Thus in fact I can only pass from ACTIVE to PASSIVE
> without reaching the WARNING state.
No problem, I just wanted to illustrate how the hardware does usually do
error and state change reporting.
Wolfgang.
--
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 07/11] net/stmmac: mark probe function as __devinit
From: Stefan Roese @ 2012-08-08 15:07 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, arm, linux-kernel, Giuseppe Cavallaro,
David S. Miller, netdev
In-Reply-To: <1344437248-20560-8-git-send-email-arnd@arndb.de>
On 08/08/2012 04:47 PM, Arnd Bergmann wrote:
> Driver probe functions are generally __devinit so they will be
> discarded after initialization for non-hotplug kernels.
> This was found by a new warning after patch 6a228452d "stmmac: Add
> device-tree support" adds a new __devinit function that is called
> from stmmac_pltfr_probe.
>
> Without this patch, building socfpga_defconfig results in:
>
> WARNING: drivers/net/ethernet/stmicro/stmmac/stmmac.o(.text+0x5d4c): Section mismatch in reference from the function stmmac_pltfr_probe() to the function .devinit.text:stmmac_probe_config_dt()
> The function stmmac_pltfr_probe() references
> the function __devinit stmmac_probe_config_dt().
> This is often because stmmac_pltfr_probe lacks a __devinit
> annotation or the annotation of stmmac_probe_config_dt is wrong.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
Acked-by: Stefan Roese <sr@denx.de>
Thanks,
Stefan
^ permalink raw reply
* [PATCH] can/softing: Fix potential memory leak in softing_load_fw()
From: Alexey Khoroshilov @ 2012-08-08 15:15 UTC (permalink / raw)
To: Kurt Van Dijck
Cc: Alexey Khoroshilov, Wolfgang Grandegger, Marc Kleine-Budde,
linux-can, netdev, linux-kernel, ldv-project
Do not leak memory by updating pointer with potentially NULL realloc return value.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
drivers/net/can/softing/softing_fw.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/can/softing/softing_fw.c b/drivers/net/can/softing/softing_fw.c
index 3105961..b595d34 100644
--- a/drivers/net/can/softing/softing_fw.c
+++ b/drivers/net/can/softing/softing_fw.c
@@ -150,7 +150,7 @@ int softing_load_fw(const char *file, struct softing *card,
const uint8_t *mem, *end, *dat;
uint16_t type, len;
uint32_t addr;
- uint8_t *buf = NULL;
+ uint8_t *buf = NULL, *new_buf;
int buflen = 0;
int8_t type_end = 0;
@@ -199,11 +199,12 @@ int softing_load_fw(const char *file, struct softing *card,
if (len > buflen) {
/* align buflen */
buflen = (len + (1024-1)) & ~(1024-1);
- buf = krealloc(buf, buflen, GFP_KERNEL);
- if (!buf) {
+ new_buf = krealloc(buf, buflen, GFP_KERNEL);
+ if (!new_buf) {
ret = -ENOMEM;
goto failed;
}
+ buf = new_buf;
}
/* verify record data */
memcpy_fromio(buf, &dpram[addr + offset], len);
--
1.7.9.5
^ permalink raw reply related
* [PATCH] rndis_wlan: Fix potential memory leak in update_pmkid()
From: Alexey Khoroshilov @ 2012-08-08 15:44 UTC (permalink / raw)
To: Jussi Kivilinna
Cc: Alexey Khoroshilov, John W. Linville, linux-wireless, netdev,
linux-kernel, ldv-project
Do not leak memory by updating pointer with potentially NULL realloc return value.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
drivers/net/wireless/rndis_wlan.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
index 241162e..7a4ae9e 100644
--- a/drivers/net/wireless/rndis_wlan.c
+++ b/drivers/net/wireless/rndis_wlan.c
@@ -1803,6 +1803,7 @@ static struct ndis_80211_pmkid *update_pmkid(struct usbnet *usbdev,
struct cfg80211_pmksa *pmksa,
int max_pmkids)
{
+ struct ndis_80211_pmkid *new_pmkids;
int i, err, newlen;
unsigned int count;
@@ -1833,11 +1834,12 @@ static struct ndis_80211_pmkid *update_pmkid(struct usbnet *usbdev,
/* add new pmkid */
newlen = sizeof(*pmkids) + (count + 1) * sizeof(pmkids->bssid_info[0]);
- pmkids = krealloc(pmkids, newlen, GFP_KERNEL);
- if (!pmkids) {
+ new_pmkids = krealloc(pmkids, newlen, GFP_KERNEL);
+ if (!new_pmkids) {
err = -ENOMEM;
goto error;
}
+ pmkids = new_pmkids;
pmkids->length = cpu_to_le32(newlen);
pmkids->bssid_info_count = cpu_to_le32(count + 1);
--
1.7.9.5
^ permalink raw reply related
* Re: [RFC net-next 3/4] gianfar: Separate out the Rx and Tx coalescing functions
From: Paul Gortmaker @ 2012-08-08 15:44 UTC (permalink / raw)
To: Claudiu Manoil; +Cc: netdev, David S. Miller
In-Reply-To: <1344428810-29923-4-git-send-email-claudiu.manoil@freescale.com>
[[RFC net-next 3/4] gianfar: Separate out the Rx and Tx coalescing functions] On 08/08/2012 (Wed 15:26) Claudiu Manoil wrote:
> Split the coalescing programming support by Rx and Tx h/w queues, in order to
> introduce a separate NAPI for the Tx confirmation path (next patch). This way,
> the Rx processing path will handle the coalescing settings for the Rx queues
> only, resp. the Tx confirmation processing path will handle the Tx queues.
>
> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
> ---
> drivers/net/ethernet/freescale/gianfar.c | 36 +++++++++++++++++++++++------
> 1 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
> index ddd350a..919acb3 100644
> --- a/drivers/net/ethernet/freescale/gianfar.c
> +++ b/drivers/net/ethernet/freescale/gianfar.c
> @@ -1794,8 +1794,8 @@ void gfar_start(struct net_device *dev)
> dev->trans_start = jiffies; /* prevent tx timeout */
> }
>
> -void gfar_configure_coalescing(struct gfar_private *priv,
> - unsigned long tx_mask, unsigned long rx_mask)
> +static inline void gfar_configure_tx_coalescing(struct gfar_private *priv,
I believe the preference is to not specify inline when all the chunks in
play are present in the one C file -- i.e. let gcc figure it out. Same
for the Rx instance below.
P.
--
> + unsigned long mask)
> {
> struct gfar __iomem *regs = priv->gfargrp[0].regs;
> u32 __iomem *baddr;
> @@ -1803,14 +1803,31 @@ void gfar_configure_coalescing(struct gfar_private *priv,
> if (priv->mode == MQ_MG_MODE) {
> int i;
> baddr = ®s->txic0;
> - for_each_set_bit(i, &tx_mask, priv->num_tx_queues) {
> + for_each_set_bit(i, &mask, priv->num_tx_queues) {
> gfar_write(baddr + i, 0);
> if (likely(priv->tx_queue[i]->txcoalescing))
> gfar_write(baddr + i, priv->tx_queue[i]->txic);
> }
> + } else {
> + /* Backward compatible case ---- even if we enable
> + * multiple queues, there's only single reg to program
> + */
> + gfar_write(®s->txic, 0);
> + if (likely(priv->tx_queue[0]->txcoalescing))
> + gfar_write(®s->txic, priv->tx_queue[0]->txic);
> + }
> +}
> +
> +static inline void gfar_configure_rx_coalescing(struct gfar_private *priv,
> + unsigned long mask)
> +{
> + struct gfar __iomem *regs = priv->gfargrp[0].regs;
> + u32 __iomem *baddr;
>
> + if (priv->mode == MQ_MG_MODE) {
> + int i;
> baddr = ®s->rxic0;
> - for_each_set_bit(i, &rx_mask, priv->num_rx_queues) {
> + for_each_set_bit(i, &mask, priv->num_rx_queues) {
> gfar_write(baddr + i, 0);
> if (likely(priv->rx_queue[i]->rxcoalescing))
> gfar_write(baddr + i, priv->rx_queue[i]->rxic);
> @@ -1819,16 +1836,19 @@ void gfar_configure_coalescing(struct gfar_private *priv,
> /* Backward compatible case ---- even if we enable
> * multiple queues, there's only single reg to program
> */
> - gfar_write(®s->txic, 0);
> - if (likely(priv->tx_queue[0]->txcoalescing))
> - gfar_write(®s->txic, priv->tx_queue[0]->txic);
> -
> gfar_write(®s->rxic, 0);
> if (likely(priv->rx_queue[0]->rxcoalescing))
> gfar_write(®s->rxic, priv->rx_queue[0]->rxic);
> }
> }
>
> +void gfar_configure_coalescing(struct gfar_private *priv,
> + unsigned long tx_mask, unsigned long rx_mask)
> +{
> + gfar_configure_tx_coalescing(priv, tx_mask);
> + gfar_configure_rx_coalescing(priv, rx_mask);
> +}
> +
> static int register_grp_irqs(struct gfar_priv_grp *grp)
> {
> struct gfar_private *priv = grp->priv;
> --
> 1.6.6
>
>
^ permalink raw reply
* Re: [RFC net-next 2/4] gianfar: Clear ievent from interrupt handler for [RT]x int
From: Paul Gortmaker @ 2012-08-08 16:11 UTC (permalink / raw)
To: Claudiu Manoil; +Cc: netdev, David S. Miller
In-Reply-To: <1344428810-29923-3-git-send-email-claudiu.manoil@freescale.com>
[[RFC net-next 2/4] gianfar: Clear ievent from interrupt handler for [RT]x int] On 08/08/2012 (Wed 15:26) Claudiu Manoil wrote:
> It's the interrupt handler's job to clear ievent for the Tx/Rx paths, as soon
> as the corresponding interrupt sources have been masked.
What wasn't clear to me was whether we'd ever have an instance of
gfar_poll run without RTX_MASK being cleared (in less normal conditions,
like netconsole, KGDBoE etc), since the gfar_schedule_cleanup is only
called from rx/tx IRQ threads, and neither of those are used by
gfar_poll, it seems.
Paul.
--
>
> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
> ---
> drivers/net/ethernet/freescale/gianfar.c | 16 ++++++----------
> 1 files changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
> index e9feeb9..ddd350a 100644
> --- a/drivers/net/ethernet/freescale/gianfar.c
> +++ b/drivers/net/ethernet/freescale/gianfar.c
> @@ -2568,12 +2568,13 @@ static void gfar_schedule_cleanup(struct gfar_priv_grp *gfargrp)
> if (napi_schedule_prep(&gfargrp->napi)) {
> gfar_write(&gfargrp->regs->imask, IMASK_RTX_DISABLED);
> __napi_schedule(&gfargrp->napi);
> - } else {
> - /* Clear IEVENT, so interrupts aren't called again
> - * because of the packets that have already arrived.
> - */
> - gfar_write(&gfargrp->regs->ievent, IEVENT_RTX_MASK);
> }
> +
> + /* Clear IEVENT, so interrupts aren't called again
> + * because of the packets that have already arrived.
> + */
> + gfar_write(&gfargrp->regs->ievent, IEVENT_RTX_MASK);
> +
> spin_unlock_irqrestore(&gfargrp->grplock, flags);
>
> }
> @@ -2837,11 +2838,6 @@ static int gfar_poll(struct napi_struct *napi, int budget)
> num_queues = gfargrp->num_rx_queues;
> budget_per_queue = budget/num_queues;
>
> - /* Clear IEVENT, so interrupts aren't called again
> - * because of the packets that have already arrived
> - */
> - gfar_write(®s->ievent, IEVENT_RTX_MASK);
> -
> while (num_queues && left_over_budget) {
> budget_per_queue = left_over_budget/num_queues;
> left_over_budget = 0;
> --
> 1.6.6
>
>
^ permalink raw reply
* Re: [RFC net-next 0/4] gianfar: Use separate NAPI for Tx confirmation processing
From: Paul Gortmaker @ 2012-08-08 16:24 UTC (permalink / raw)
To: Claudiu Manoil; +Cc: netdev, David S. Miller
In-Reply-To: <1344428810-29923-1-git-send-email-claudiu.manoil@freescale.com>
[[RFC net-next 0/4] gianfar: Use separate NAPI for Tx confirmation processing] On 08/08/2012 (Wed 15:26) Claudiu Manoil wrote:
> Hi all,
> This set of patches basically splits the existing napi poll routine into
> two separate napi functions, one for Rx processing (triggered by frame
> receive interrupts only) and one for the Tx confirmation path processing
> (triggerred by Tx confirmation interrupts only). The polling algorithm
> behind remains much the same.
>
> Important throughput improvements have been noted on low power boards with
> this set of changes.
> For instance, for the following netperf test:
> netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> yields a throughput gain from oscilating ~500-~700 Mbps to steady ~940 Mbps,
> (if the Rx/Tx paths are processed on different cores), w/ no increase in CPU%,
> on a p1020rdb - 2 core machine featuring etsec2.0 (Multi-Queue Multi-Group
> driver mode).
It would be interesting to know more about what was causing that large
an oscillation -- presumably you will have it reappear once one core
becomes 100% utilized. Also, any thoughts on how the change will change
performance on an older low power single core gianfar system (e.g. 83xx)?
P.
--
>
> Also, this change, which should ballance Rx and Tx processing, proves to
> be effective against Rx busy interrupt occurrences.
>
> Thanks for your review.
> Claudiu
>
>
> Claudiu Manoil (4):
> gianfar: Remove redundant programming of [rt]xic registers
> gianfar: Clear ievent from interrupt handler for [RT]x int
> gianfar: Separate out the Rx and Tx coalescing functions
> gianfar: Use separate NAPIs for Tx and Rx processing
>
> drivers/net/ethernet/freescale/gianfar.c | 220 +++++++++++++++++++++--------
> drivers/net/ethernet/freescale/gianfar.h | 16 ++-
> 2 files changed, 171 insertions(+), 65 deletions(-)
>
>
^ permalink raw reply
* Re: [RFC net-next 0/4] gianfar: Use separate NAPI for Tx confirmation processing
From: Eric Dumazet @ 2012-08-08 16:44 UTC (permalink / raw)
To: Paul Gortmaker; +Cc: Claudiu Manoil, netdev, David S. Miller
In-Reply-To: <20120808162423.GC11043@windriver.com>
On Wed, 2012-08-08 at 12:24 -0400, Paul Gortmaker wrote:
> [[RFC net-next 0/4] gianfar: Use separate NAPI for Tx confirmation processing] On 08/08/2012 (Wed 15:26) Claudiu Manoil wrote:
>
> > Hi all,
> > This set of patches basically splits the existing napi poll routine into
> > two separate napi functions, one for Rx processing (triggered by frame
> > receive interrupts only) and one for the Tx confirmation path processing
> > (triggerred by Tx confirmation interrupts only). The polling algorithm
> > behind remains much the same.
> >
> > Important throughput improvements have been noted on low power boards with
> > this set of changes.
> > For instance, for the following netperf test:
> > netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> > yields a throughput gain from oscilating ~500-~700 Mbps to steady ~940 Mbps,
> > (if the Rx/Tx paths are processed on different cores), w/ no increase in CPU%,
> > on a p1020rdb - 2 core machine featuring etsec2.0 (Multi-Queue Multi-Group
> > driver mode).
>
> It would be interesting to know more about what was causing that large
> an oscillation -- presumably you will have it reappear once one core
> becomes 100% utilized. Also, any thoughts on how the change will change
> performance on an older low power single core gianfar system (e.g. 83xx)?
I also was wondering if this low performance could be caused by BQL
Since TCP stack is driven by incoming ACKS, a NAPI run could have to
handle 10 TCP acks in a row, and resulting xmits could hit BQL and
transit on qdisc (Because NAPI handler wont handle TX completions in the
middle of RX handler)
So experiments would be nice, maybe by reducing a
bit /proc/sys/net/ipv4/tcp_limit_output_bytes
(from 131072 to 65536 or 32768)
^ permalink raw reply
* Re: [PATCH] can/softing: Fix potential memory leak in softing_load_fw()
From: Marc Kleine-Budde @ 2012-08-08 17:18 UTC (permalink / raw)
To: Alexey Khoroshilov
Cc: Kurt Van Dijck, Wolfgang Grandegger, linux-can, netdev,
linux-kernel, ldv-project
In-Reply-To: <1344438901-25005-1-git-send-email-khoroshilov@ispras.ru>
[-- Attachment #1: Type: text/plain, Size: 603 bytes --]
On 08/08/2012 05:15 PM, Alexey Khoroshilov wrote:
> Do not leak memory by updating pointer with potentially NULL realloc return value.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Thanks for the work,
applied to can/master.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply
* Re: [PATCH v6 7/7] ARM: davinci: add support for the am1808 based enbw_cmc board
From: Sekhar Nori @ 2012-08-08 17:35 UTC (permalink / raw)
To: Heiko Schocher
Cc: Kevin Hilman,
davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
Wolfgang Denk, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Wolfram Sang,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, Ben Dooks, Scott Wood,
Sylwester Nawrocki, David Woodhouse,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1341492608-20597-1-git-send-email-hs-ynQEQJNshbs@public.gmane.org>
Hi Heiko,
On 7/5/2012 6:20 PM, Heiko Schocher wrote:
> - AM1808 based board
> - 64 MiB DDR ram
> - 2 MiB Nor flash
> - 128 MiB NAND flash
> - use internal RTC
> - I2C support
> - hwmon lm75 support
> - UBI/UBIFS support
> - MMC support
> - USB OTG support
>
> Signed-off-by: Heiko Schocher <hs-ynQEQJNshbs@public.gmane.org>
> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> Cc: davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org
> Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
> Cc: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
> Cc: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Cc: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
> Cc: Kevin Hilman <khilman-l0cyMroinI0@public.gmane.org>
> Cc: Wolfgang Denk <wd-ynQEQJNshbs@public.gmane.org>
> Cc: Scott Wood <scottwood-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> Cc: Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>
> ---
> - post this board support with USB support, even though
> USB is only working with the 10 ms "workaround", posted here:
> http://comments.gmane.org/gmane.linux.usb.general/54505
> I see this issue also on the AM1808 TMDXEXP1808L evalboard.
> - MMC and USB are not using OF support yet, ideas how to port
> this are welcome. I need for USB and MMC boards board
> specific callbacks, how to solve this with OF support?
>
> - changes for v2:
> - changes in the nand node due to comments from Scott Wood:
> - add "ti,davinci-" prefix
> - Dashes are preferred to underscores
> - rename "nandflash" to "nand"
> - introduce new "ti,davinci" specific properties for setting
> up ecc_mode, ecc_bits, options and bbt options, instead
> using linux defines
> - changes for i2c due to comments from Sylwester Nawrocki:
> - use "cell-index" instead "id"
> - OF_DEV_AUXDATA in the machine code, instead pre-define
> platform device name
> - add comment from Grant Likely for i2c:
> - removed "id" resp. "cell-index" completely
> - fixed documentation
> - use of_match_ptr()
> - use devm_kzalloc() for allocating plattform data mem
> - fixed a whitespace issue
> - add net comments from Grant Likely:
> - add prefix "ti,davinci-" to davinci specific property names
> - remove version property
> - use compatible name "ti,davinci-dm6460-emac"
> - add comment from Grant Likely:
> - rename compatible node
> - do not use cell-index
> - CONFIG_OF required for this board
> TODO:
> - create a generic board support file, as I got no
> answer to my ping to grant, maybe this could be done
> in a second step?
> - changes for v3:
> - add comments from Sergei Shtylyov:
> - rename compatible" prop to "ti,cp_intc"
> - cp_intc_init now used for Interrupt controller init
> - changes for v4:
> add comment from Nori Sekhar:
> - rename davinci emac compatible property to "ti,davinci-dm6467-emac"
> - remove "pinmux-handle" property as discussed here:
> http://www.spinics.net/lists/arm-kernel/msg175701.html
> with Nori Sekhar
>
> - changes for v5:
> add comments from Grant Likely:
> - rename compatible" prop to "ti,cp-intc"
>
> - changes for v6:
> rework this patch, as patch
> ARM: davinci: cp_intc: Add OF support for TI interrupt controller
> was changed from Nori Sekhar on Jul 03, 2012; 9:16pm
> Changes therefore in this patch:
>
> Call of_irq_init() in the generic DT board file and not in the
> interrupt controller code. See arch/arm/mach-at91/board-dt.c or
> arch/arm/mach-omap2/board-generic.c for examples.
>
> At this point the question raises, if we should rename this
> board port from arch/arm/mach-davinci/enbw_cmc.c to
> arch/arm/mach-davinci/board-dt.c ? Also the defconfig to
> davinci_of_defconfig ... ? The USB and MMC callbacks are
> currently board specific, but if other boards come in,
> that could be easily adapted for their needs ...
>
> arch/arm/boot/dts/enbw_cmc.dts | 183 +++++++++++
> arch/arm/configs/enbw_cmc_defconfig | 126 ++++++++
> arch/arm/mach-davinci/Kconfig | 9 +
> arch/arm/mach-davinci/Makefile | 1 +
> arch/arm/mach-davinci/board-enbw-cmc.c | 385 +++++++++++++++++++++++
> arch/arm/mach-davinci/include/mach/uncompress.h | 1 +
> 6 files changed, 705 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/boot/dts/enbw_cmc.dts
> create mode 100644 arch/arm/configs/enbw_cmc_defconfig
> create mode 100644 arch/arm/mach-davinci/board-enbw-cmc.c
With the move to DT, we are not going to add more board specific files
and defconfigs. I am working on adding a generic dt board file for DA850
(similar to what has been done for imx). That file is not going to do
any platform device registrations. If you are OK with it, I will add the
dts file for enbw_cmc board in the patch series I am planning to post.
Things like USB and MMC/SD will need to be converted to DT before they
can be supported on new boards.
Thanks,
Sekhar
^ permalink raw reply
* Re: [RFC PATCH] tun: don't zeroize sock->file on detach
From: Yuchung Cheng @ 2012-08-08 17:48 UTC (permalink / raw)
To: Stanislav Kinsbursky
Cc: davem@davemloft.net, netdev@vger.kernel.org,
ruanzhijie@hotmail.com, linux-kernel@vger.kernel.org,
viro@zeniv.linux.org.uk
In-Reply-To: <50226147.3010309@parallels.com>
On Wed, Aug 8, 2012 at 5:53 AM, Stanislav Kinsbursky
<skinsbursky@parallels.com> wrote:
> Hi, Dave.
> What about this patch?
>
>
> On Wed, Jul 11, 2012 at 03:48:20PM +0400, Stanislav Kinsbursky wrote:
>>
>> This is a fix for bug, introduced in 3.4 kernel by commit
>> 1ab5ecb90cb6a3df1476e052f76a6e8f6511cb3d, which, among other things,
>> replaced
>> simple sock_put() by sk_release_kernel(). Below is sequence, which leads
>> to
>> oops for non-persistent devices:
>>
>> tun_chr_close()
>> tun_detach() <== tun->socket.file = NULL
>> tun_free_netdev()
>> sk_release_sock()
>> sock_release(sock->file == NULL)
>> iput(SOCK_INODE(sock)) <== dereference on NULL pointer
>>
>> This patch just removes zeroing of socket's file from __tun_detach().
>> sock_release() will do this.
>>
>> Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
I has tested this patch and it works (so my kernel stops crashing
using tun devices).
>> ---
>> drivers/net/tun.c | 1 -
>> 1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>> index 987aeef..c1639f3 100644
>> --- a/drivers/net/tun.c
>> +++ b/drivers/net/tun.c
>> @@ -185,7 +185,6 @@ static void __tun_detach(struct tun_struct *tun)
>> netif_tx_lock_bh(tun->dev);
>> netif_carrier_off(tun->dev);
>> tun->tfile = NULL;
>> - tun->socket.file = NULL;
>> netif_tx_unlock_bh(tun->dev);
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] netvm: check for page == NULL when propogating the skb->pfmemalloc flag
From: Rik van Riel @ 2012-08-08 19:14 UTC (permalink / raw)
To: Mel Gorman
Cc: David Miller, Linux-MM, LKML, Linux-Netdev, Xen-devel,
Konrad Rzeszutek Wilk, Ian Campbell, Andrew Morton
In-Reply-To: <20120807085554.GF29814@suse.de>
On 08/07/2012 04:55 AM, Mel Gorman wrote:
> Commit [c48a11c7: netvm: propagate page->pfmemalloc to skb] is responsible
> for the following bug triggered by a xen network driver
>
> [ 1.908592] BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
> [ 1.908643] IP: [<ffffffffa0037750>] xennet_poll+0x980/0xec0 [xen_netfront]
> [ 1.908703] PGD ea1df067 PUD e8ada067 PMD 0
> [ 1.908774] Oops: 0000 [#1] SMP
> [ 1.908797] Modules linked in: fbcon tileblit font radeon bitblit softcursor ttm drm_kms_helper crc32c_intel xen_blkfront xen_netfront xen_fbfront fb_sys_fops sysimgblt sysfillrect syscopyarea +xen_kbdfront xenfs xen_privcmd
> [ 1.908938] CPU 0
> [ 1.908950] Pid: 2165, comm: ip Not tainted 3.5.0upstream-08854-g444fa66 #1
> [ 1.908983] RIP: e030:[<ffffffffa0037750>] [<ffffffffa0037750>] xennet_poll+0x980/0xec0 [xen_netfront]
> [ 1.909029] RSP: e02b:ffff8800ffc03db8 EFLAGS: 00010282
> [ 1.909055] RAX: ffff8800ea010140 RBX: ffff8800f00e86c0 RCX: 000000000000009a
> [ 1.909055] RDX: 0000000000000040 RSI: 000000000000005a RDI: ffff8800fa7dee80
> [ 1.909055] RBP: ffff8800ffc03ee8 R08: ffff8800f00e86d8 R09: ffff8800ea010000
> [ 1.909055] R10: dead000000200200 R11: dead000000100100 R12: ffff8800fa7dee80
> [ 1.909055] R13: 000000000000005a R14: ffff8800fa7dee80 R15: 0000000000000200
> [ 1.909055] FS: 00007fbafc188700(0000) GS:ffff8800ffc00000(0000) knlGS:0000000000000000
> [ 1.909055] CS: e033 DS: 0000 ES: 0000 CR0: 000000008005003b
> [ 1.909055] CR2: 0000000000000010 CR3: 00000000ea108000 CR4: 0000000000002660
> [ 1.909055] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 1.909055] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [ 1.909055] Process ip (pid: 2165, threadinfo ffff8800ea0f2000, task ffff8800fa783040)
> [ 1.909055] Stack:
> [ 1.909055] ffff8800e27e5040 ffff8800ffc03e88 ffff8800ffc03e68 ffff8800ffc03e48
> [ 1.909055] 7fffffffffffffff ffff8800ffc03e00 ffff8800e27e5040 ffff8800f00e86d8
> [ 1.909055] ffff8800ffc03eb0 00000040ffffffff ffff8800f00e8000 00000000ffc03e30
> [ 1.909055] Call Trace:
> [ 1.909055] <IRQ>
> [ 1.909055] [<ffffffff81066028>] ? pvclock_clocksource_read+0x58/0xd0
> [ 1.909055] [<ffffffff81486352>] net_rx_action+0x112/0x240
> [ 1.909055] [<ffffffff8107f319>] __do_softirq+0xb9/0x190
> [ 1.909055] [<ffffffff815d8d7c>] call_softirq+0x1c/0x30
>
> The problem is that the xenfront driver is passing a NULL page to
> __skb_fill_page_desc() which was unexpected. This patch checks that
> there is a page before dereferencing.
>
> Reported-and-Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Signed-off-by: Mel Gorman <mgorman@suse.de>
Acked-by: Rik van Riel <riel@redhat.com>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] rndis_wlan: Fix potential memory leak in update_pmkid()
From: Jussi Kivilinna @ 2012-08-08 19:23 UTC (permalink / raw)
To: Alexey Khoroshilov
Cc: John W. Linville, linux-wireless, netdev, linux-kernel,
ldv-project
In-Reply-To: <1344440661-32322-1-git-send-email-khoroshilov@ispras.ru>
Quoting Alexey Khoroshilov <khoroshilov@ispras.ru>:
> Do not leak memory by updating pointer with potentially NULL realloc
> return value.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Thanks!
Acked-by: Jussi Kivilinna <jussi.kivilinna@mbnet.fi>
> ---
> drivers/net/wireless/rndis_wlan.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/rndis_wlan.c
> b/drivers/net/wireless/rndis_wlan.c
> index 241162e..7a4ae9e 100644
> --- a/drivers/net/wireless/rndis_wlan.c
> +++ b/drivers/net/wireless/rndis_wlan.c
> @@ -1803,6 +1803,7 @@ static struct ndis_80211_pmkid
> *update_pmkid(struct usbnet *usbdev,
> struct cfg80211_pmksa *pmksa,
> int max_pmkids)
> {
> + struct ndis_80211_pmkid *new_pmkids;
> int i, err, newlen;
> unsigned int count;
>
> @@ -1833,11 +1834,12 @@ static struct ndis_80211_pmkid
> *update_pmkid(struct usbnet *usbdev,
> /* add new pmkid */
> newlen = sizeof(*pmkids) + (count + 1) * sizeof(pmkids->bssid_info[0]);
>
> - pmkids = krealloc(pmkids, newlen, GFP_KERNEL);
> - if (!pmkids) {
> + new_pmkids = krealloc(pmkids, newlen, GFP_KERNEL);
> + if (!new_pmkids) {
> err = -ENOMEM;
> goto error;
> }
> + pmkids = new_pmkids;
>
> pmkids->length = cpu_to_le32(newlen);
> pmkids->bssid_info_count = cpu_to_le32(count + 1);
> --
> 1.7.9.5
>
>
>
^ permalink raw reply
* Bug with IPv6-UDP address binding
From: Jesper Dangaard Brouer @ 2012-08-08 20:37 UTC (permalink / raw)
To: netdev; +Cc: Thomas Graf
Hi NetDev
I think I have found a problem/bug with IPv6-UDP address binding.
I found this problem while playing with IPVS and IPv6-UDP, but its also
present in more basic/normal situations.
If you have two IPv6 addresses, within the same IPv6 subnet, then one
of the IPv6 addrs takes precedence over the other (for UDP only).
Meaning that, if connecting to the "secondary" IPv6 via UDP, will
result in userspace see/bind the connection as being created to the
"primary" IP, even-though tcpdump shows that the IPv6-UDP packets are
dest the "secondary".
The result is; that only the first IPv6-UDP packet is delivered to
userspace, and the next packets are denied by the kernel as the UDP
socket is "established" with the "primary" IPv6 addr.
I would appreciate some hints to where in the IPv6 code I should look
for this bug. If any one else wants to fix it, I'm also fine with
that ;-)
Its quite easy to reproduce, using netcat (nc).
Add two addresses to the "server" e.g.:
ip addr add fee0:cafe::102/64 dev eth0
ip addr add fee0:cafe::bad/64 dev eth0
Run a netcat listener on "server":
nc -6 -u -l 2000
(Notice restart the listener between runs, due to limitation in nc)
On the client add an IPv6 addr e.g.:
ip addr add fee0:cafe::101/64 dev eth0
Run a netcat UDP-IPv6 producer on "client":
nc -6 -u fee0:cafe::bad 2000
Notice that first packet, will get through, but second packets will
not (nc: Write error: Connection refused). Running a tcpdump shows
that the kernel is sending back ICMP6, destination unreachable,
unreachable port.
Its also possible to see the problem, simply running "netstat -uan" on
"server", which will show that the "established" UDP connection, is
bound to the wrong "Local Address".
(Tested on both latest net-next kernel at commit 79cda75a1, and also
on RHEL6 approx 2.6.32)
^ permalink raw reply
* Re: Bug with IPv6-UDP address binding
From: Eric Dumazet @ 2012-08-08 20:59 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: netdev, Thomas Graf
In-Reply-To: <1344458238.3069.13.camel@localhost>
On Wed, 2012-08-08 at 22:37 +0200, Jesper Dangaard Brouer wrote:
> Hi NetDev
>
> I think I have found a problem/bug with IPv6-UDP address binding.
>
> I found this problem while playing with IPVS and IPv6-UDP, but its also
> present in more basic/normal situations.
>
> If you have two IPv6 addresses, within the same IPv6 subnet, then one
> of the IPv6 addrs takes precedence over the other (for UDP only).
>
> Meaning that, if connecting to the "secondary" IPv6 via UDP, will
> result in userspace see/bind the connection as being created to the
> "primary" IP, even-though tcpdump shows that the IPv6-UDP packets are
> dest the "secondary".
>
> The result is; that only the first IPv6-UDP packet is delivered to
> userspace, and the next packets are denied by the kernel as the UDP
> socket is "established" with the "primary" IPv6 addr.
>
> I would appreciate some hints to where in the IPv6 code I should look
> for this bug. If any one else wants to fix it, I'm also fine with
> that ;-)
>
>
> Its quite easy to reproduce, using netcat (nc).
>
> Add two addresses to the "server" e.g.:
> ip addr add fee0:cafe::102/64 dev eth0
> ip addr add fee0:cafe::bad/64 dev eth0
>
> Run a netcat listener on "server":
> nc -6 -u -l 2000
> (Notice restart the listener between runs, due to limitation in nc)
>
> On the client add an IPv6 addr e.g.:
> ip addr add fee0:cafe::101/64 dev eth0
>
> Run a netcat UDP-IPv6 producer on "client":
> nc -6 -u fee0:cafe::bad 2000
>
> Notice that first packet, will get through, but second packets will
> not (nc: Write error: Connection refused). Running a tcpdump shows
> that the kernel is sending back ICMP6, destination unreachable,
> unreachable port.
>
> Its also possible to see the problem, simply running "netstat -uan" on
> "server", which will show that the "established" UDP connection, is
> bound to the wrong "Local Address".
>
> (Tested on both latest net-next kernel at commit 79cda75a1, and also
> on RHEL6 approx 2.6.32)
>
Hi Jesper
Thats because the "nc -6 -u -l 2000" on server does :
bind(3, {sa_family=AF_INET6, sin6_port=htons(2000), inet_pton(AF_INET6,
"::", &sin6_addr), sin6_flowinfo=0, sin6_scope_id=0}, 28) = 0
recvfrom(3, "\n", 1024, MSG_PEEK, {sa_family=AF_INET6,
sin6_port=htons(53696), inet_pton(AF_INET6, "fee0:cafe::101",
&sin6_addr), sin6_flowinfo=0, sin6_scope_id=0}, [28]) = 1
connect(3, {sa_family=AF_INET6, sin6_port=htons(53696),
inet_pton(AF_INET6, "fee0:cafe::101", &sin6_addr), sin6_flowinfo=0,
sin6_scope_id=0}, 28) = 0
And the kernel automatically chooses a SOURCE address (fee0:cafe::102)
that is not what you expected (fee0:cafe::bad)
So its a bug in the application.
UDP connect() is tricky : In this case, nc should learn on what IP
address the client sent the frame. (using recvmsg() and appropriate
ancillary message)
Then nc should bind a new socket on this address, then do the connect()
^ 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