* Re: [PATCH] net_sched: prio: use qdisc_dequeue_peeked
From: Eric Dumazet @ 2011-08-09 12:56 UTC (permalink / raw)
To: Florian Westphal; +Cc: netdev
In-Reply-To: <1312891483-30034-1-git-send-email-fw@strlen.de>
Le mardi 09 août 2011 à 14:04 +0200, Florian Westphal a écrit :
> commit 07bd8df5df4369487812bf85a237322ff3569b77
> (sch_sfq: fix peek() implementation) changed sfq to use generic
> peek helper.
>
> This makes HFSC complain about a non-work-conserving child qdisc, if
> prio with sfq child is used within hfsc:
>
> hfsc peeks into prio qdisc, which will then peek into sfq.
> returned skb is stashed in sch->gso_skb.
>
> Next, hfsc tries to dequeue from prio, but prio will call sfq dequeue
> directly, which may return NULL instead of previously peeked-at skb.
>
> Have prio call qdisc_dequeue_peeked, so sfq->dequeue() is
> not called in this case.
>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> Eric, does this look correct to you?
> I am not sure if sfq needs fixing instead of this patch here.
>
> diff --git a/net/sched/sch_prio.c b/net/sched/sch_prio.c
> index 2a318f2..b5d56a2 100644
> --- a/net/sched/sch_prio.c
> +++ b/net/sched/sch_prio.c
> @@ -112,7 +112,7 @@ static struct sk_buff *prio_dequeue(struct Qdisc *sch)
>
> for (prio = 0; prio < q->bands; prio++) {
> struct Qdisc *qdisc = q->queues[prio];
> - struct sk_buff *skb = qdisc->dequeue(qdisc);
> + struct sk_buff *skb = qdisc_dequeue_peeked(qdisc);
> if (skb) {
> qdisc_bstats_update(sch, skb);
> sch->q.qlen--;
Hi Florian
Are you sure this patch is still needed, after commit
e1738bd9cecc5c867b0e2996470c1ff20f66ba79
(sch_sfq: fix sfq_enqueue())
I am asking before fully reviewing the code, I dont know who
should/shouldnt call qdisc_dequeue_peeked() instead of qdisc->dequeue()
Thanks !
^ permalink raw reply
* Re: [PATCH 4/4] [powerpc] Add flexcan device support for p1010rdb.
From: Wolfgang Grandegger @ 2011-08-09 13:00 UTC (permalink / raw)
To: Robin Holt
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w, U Bhaskar-B22300,
Marc Kleine-Budde, PPC list, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1312892907-20419-5-git-send-email-holt-sJ/iWh9BUns@public.gmane.org>
Hi Robin,
On 08/09/2011 02:28 PM, Robin Holt wrote:
> I added a simple clock source for the p1010rdb so the flexcan driver
> could determine a clock frequency. The p1010 can device only has an
> oscillator of system bus frequency divided by 2.
>
> Signed-off-by: Robin Holt <holt-sJ/iWh9BUns@public.gmane.org>
> To: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
> To: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>,
> To: U Bhaskar-B22300 <B22300-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
> Cc: PPC list <linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org>
> ---
> arch/powerpc/platforms/85xx/Kconfig | 2 +
> arch/powerpc/platforms/85xx/Makefile | 2 +
> arch/powerpc/platforms/85xx/clock.c | 42 ++++++++++++++++++++++++++++++++
> arch/powerpc/platforms/85xx/p1010rdb.c | 8 ++++++
> 4 files changed, 54 insertions(+), 0 deletions(-)
> create mode 100644 arch/powerpc/platforms/85xx/clock.c
>
> diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig
> index 498534c..c4304ae 100644
> --- a/arch/powerpc/platforms/85xx/Kconfig
> +++ b/arch/powerpc/platforms/85xx/Kconfig
> @@ -70,6 +70,8 @@ config MPC85xx_RDB
> config P1010_RDB
> bool "Freescale P1010RDB"
> select DEFAULT_UIMAGE
> + select HAVE_CAN_FLEXCAN if NET && CAN
> + select PPC_CLOCK if CAN_FLEXCAN
> help
> This option enables support for the MPC85xx RDB (P1010 RDB) board
>
> diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
> index a971b32..cc7f381 100644
> --- a/arch/powerpc/platforms/85xx/Makefile
> +++ b/arch/powerpc/platforms/85xx/Makefile
> @@ -3,6 +3,8 @@
> #
> obj-$(CONFIG_SMP) += smp.o
>
> +obj-$(CONFIG_PPC_CLOCK) += clock.o
> +
> obj-$(CONFIG_MPC8540_ADS) += mpc85xx_ads.o
> obj-$(CONFIG_MPC8560_ADS) += mpc85xx_ads.o
> obj-$(CONFIG_MPC85xx_CDS) += mpc85xx_cds.o
> diff --git a/arch/powerpc/platforms/85xx/clock.c b/arch/powerpc/platforms/85xx/clock.c
> new file mode 100644
> index 0000000..a6fd2c8
> --- /dev/null
> +++ b/arch/powerpc/platforms/85xx/clock.c
> @@ -0,0 +1,42 @@
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +
> +#include <asm/clk_interface.h>
> +
> +#include <sysdev/fsl_soc.h>
> +
> +/*
> + * p1010 needs to provide a clock source for the flexcan driver. The
> + * oscillator for the p1010 processor is only ever the system clock / 2.
> + */
> +
> +static struct clk *mpc85xx_clk_get(struct device *dev, const char *id)
> +{
> + if (!dev)
> + return ERR_PTR(-ENOENT);
> +
Ah, I think you removed too much code here. I obviously did not
understand what the device node check is good for, sorry. The clock is
only implemented for the Flexcan and therefore we should add a check here:
if (!dev->of_node ||
!of_device_is_compatible(dev->of_node, "fsl,flexcan"))
return ERR_PTR(-ENOENT);
Something like that should work. For the next version you can then add
my "Acked-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org" to all patches.
Wolfgang.
^ permalink raw reply
* Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
From: Wolfgang Grandegger @ 2011-08-09 13:03 UTC (permalink / raw)
To: Robin Holt
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
U Bhaskar-B22300, Marc Kleine-Budde
In-Reply-To: <20110809124919.GS4926-sJ/iWh9BUns@public.gmane.org>
On 08/09/2011 02:49 PM, Robin Holt wrote:
> On Tue, Aug 09, 2011 at 12:41:39PM +0000, U Bhaskar-B22300 wrote:
>>
>>
>>> -----Original Message-----
>>> From: Wolfgang Grandegger [mailto:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
>>> Sent: Tuesday, August 09, 2011 4:19 PM
>>> To: U Bhaskar-B22300
>>> Cc: Marc Kleine-Budde; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
>>> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
>>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
>>>
>>> On 08/09/2011 11:27 AM, U Bhaskar-B22300 wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Wolfgang Grandegger [mailto:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
>>>>> Sent: Tuesday, August 09, 2011 2:03 PM
>>>>> To: U Bhaskar-B22300
>>>>> Cc: Marc Kleine-Budde; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
>>>>> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
>>>>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
>>>>>
>>>>> Hi Bhaskar,
>>>>>
>>>>> On 08/09/2011 09:57 AM, U Bhaskar-B22300 wrote:
>>>>>>
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Marc Kleine-Budde [mailto:mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org]
>>>>>>> Sent: Tuesday, August 09, 2011 12:23 AM
>>>>>>> To: Wolfgang Grandegger
>>>>>>> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; U
>>>>>>> Bhaskar- B22300
>>>>>>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
>>>>>>>
>>>>>>> On 08/08/2011 05:33 PM, Wolfgang Grandegger wrote:
>>>>>>>>> ACK - The device tree bindings as in mainline's Documentation is
>>>>>>>>> a
>>>>>>> mess.
>>>>>>>>> If the powerpc guys are happy with a clock interfaces based
>>>>>>>>> approach somewhere in arch/ppc, I'm more than happy to remove:
>>>>>>>>> - fsl,flexcan-clock-source (not implemented, even in the fsl
>>>>>>>>> driver)
>>>>>> [Bhaskar]I have pushed the FlexCAN series of patches, It contains
>>>>>> the usage of all the fields posted in the FlexCAN bindings at
>>>>>> http://git.kernel.org/?p=linux/kernel/git/stable/linux-3.0.y.git;a=b
>>>>>> lo
>>>>>> b;f=Documentation/devicetree/bindings/net/can/fsl-flexcan.txt;h=1a72
>>>>>> 9f
>>>>>> 089866259ef82d0db5893ff7a8c54d5ccf;hb=94ed5b4788a7cdbe68bc7cb8516972
>>>>>> cb
>>>>>> ebdc8274
>>>>>
>>>>> As Marc already pointed out, Robin already has a much more advanced
>>>>> patch stack in preparation. Especially your patches do not care about
>>>>> the already existing Flexcan core on the Freescale's ARM socks.
>>>> [Bhaskar] No, the patches are taking care of the existing ARM
>>> functionality.
>>>> I have not tested on the ARM based board, but the patches are made
>>> in a
>>>> Manner that it should not break the ARM based functionality.
>>>>>
>>>>>>>>>
>>>>>>>>> - fsl,flexcan-clock-divider \__ replace with code in arch/ppc, or
>>>>>>>>> - clock-frequency / a single clock-frequency attribute
>>>>>>>>
>>>>>>>> In the "net-next-2.6" tree there is also:
>>>>>>>>
>>>>>>>> $ grep flexcan arch/powerpc/boots/dts/*.dts
>>>>>>>> p1010rdb.dts: fsl,flexcan-clock-source =
>>>>> "platform";
>>>>>>>> p1010rdb.dts: fsl,flexcan-clock-source =
>>>>> "platform";
>>>>>>>> p1010si.dtsi: compatible = "fsl,flexcan-v1.0";
>>>>>>>> p1010si.dtsi: fsl,flexcan-clock-divider = <2>;
>>>>>>>> p1010si.dtsi: compatible = "fsl,flexcan-v1.0";
>>>>>>>> p1010si.dtsi: fsl,flexcan-clock-divider = <2>;
>>>>>>>>
>>>>>>>> Especially the fsl,flexcan-clock-divider = <2>; might make people
>>>>>>>> think, that they could set something else.
>>>>>>>
>>>>>> [Bhaskar] As it is mentioned in the Flexcan bindings, the need of
>>>>> fsl,flexcan-clock-divider = <2>;
>>>>>> But I kept it as "2" because FlexCan clock source is the
>>>>> platform clock and it is CCB/2
>>>>>> If the "2" is misleading, the bindings can be changed or some
>>>>> text can be written to make the meaning of "2"
>>>>>> Understandable , Please suggest ..
>>>>>
>>>>> The clock source and frequency is fixed. Why do we need an extra
>>>>> properties for that. We have panned to remove these bogus bindings
>>>>> from the Linux kernel, which sneaked in *without* any review on the
>>>>> relevant mailing lists (at least I have not realized any posting). We
>>>>> do not think they are really needed. They just confuse the user. We
>>>>> also prefer to use the compatibility string "fsl,flexcan" instead
>>>>> "fsl,flexcan-v1.0". It's unusual to add a version number, which is
>>>>> for the Flexcan on the PowerPC cores only, I assume, but there will
>>>>> be device tree for ARM soon. A proper compatibility string would be
>>>>> "fsl,p1010-flexcan" if we really need to distinguish.
>>>>>
>>>> [Bhaskar] About clock source.. There can be two sources of clock for
>>> the CAN.
>>>> Oscillator or the platform clock, but at present only platform
>>> clock is supported
>>>> in P1010.If we remove the fsl,flexcan-clock-source property, we
>>> will lost the flexibility
>>>> of changing the clock source ..
>>>>
>>>> About clock-frequency... it is also not fixed. It depends on
>>> the platform clock which in turns
>>>> Depends on the CCB clock. So it will be better to keep clock-
>>> frequency property which is getting fixed via u-boot.
>>>
>>> The frequency is fixed to CCB-frequency / 2. Will that ever change? What
>>> can we expect from future Flexcan hardware? Will it support further clock
>>> sources?
>> [Bhaskar] Yes the frequency will always be CCB-frequency/2.Even if the CCB gets changed that will be taken care by the u-boot fixup code for
>> clock-frequency. clock-frequency is not filled by somebody in the dts file. It will be done by u-boot.
>> For clock source,I can't say right now, that's why I have kept a property for this in the can node. So that in future, we need to fill it
>> appropriately
>
> Speaking of the dts file, I have left the p1010si.dtsi file with
> the fsl,flexcan-v1.0 .compatible definition. The flexcan folks
> (IIRC Wolfgang) objected to that as it does not follow the standard
> which should be just fsl,flexcan.
>
> How would you like to change that? Should I add it as part of this patch,
> add another patch to the series, or let you take care of it?
>
> Also, I assume the uboot project will need to be changed as well to
> reflect the corrected name.
I think you should provide patches within this series to cleanup the
obsolete stuff, dts and binding doc.
Wolfgang.
^ permalink raw reply
* Re: gianfar.c null pointer deref in gfar_start_xmit().
From: Robin Holt @ 2011-08-09 13:06 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Robin Holt, Sandeep Gopalpet, David S. Miller, netdev
In-Reply-To: <1312873813.2531.54.camel@edumazet-laptop>
On Tue, Aug 09, 2011 at 09:10:13AM +0200, Eric Dumazet wrote:
> Le mardi 09 août 2011 à 01:54 -0500, Robin Holt a écrit :
> > On Tue, Aug 02, 2011 at 09:44:38PM -0500, Robin Holt wrote:
> > >
> > > While using the v3.0 kernel on a Freescale P1010RDB with 3 minor patches
> > > (None which affect gianfar.c), I get a NULL pointer deref at:
> > >
> > > static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
> > > {
> > > ...
> > > regs = tx_queue->grp->regs;
> > >
> > > I put a BUG_ON(tx_queue->grp) just before this line and it did trip.
> > > I have not looked at this any more than that.
> > >
> > > Any suggestions would be welcome. To reproduce, all I need to do is
> > > a few sequences of pings.
> >
> > I was able to reproduce this with the net-next-2.6 kernel as well.
> >
>
> This driver incorrectly assumes a non dense txqueue array is possible
> for a netdev, but its not true.
>
> In the meantime, you could force it to use one tx_queue only.
>
> tx_queues = (u32 *)of_get_property(np, "fsl,num_tx_queues", NULL);
> num_tx_qs = tx_queues ? *tx_queues : 1;
I fixed up the .dts file and now it works. I have not tested the RGMII
interface yet so I do not know if that supports multiple queues. I assume
I could find it in the documentation. What might I be looking for?
Thanks,
Robin
^ permalink raw reply
* Re: [RFC 4/4] [powerpc] Add flexcan device support for p1010rdb.
From: Marc Kleine-Budde @ 2011-08-09 13:09 UTC (permalink / raw)
To: Robin Holt
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA, U Bhaskar-B22300, PPC list,
Wolfgang Grandegger
In-Reply-To: <20110809124042.GR4926-sJ/iWh9BUns@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1366 bytes --]
On 08/09/2011 02:40 PM, Robin Holt wrote:
> On Tue, Aug 09, 2011 at 02:33:31PM +0200, Marc Kleine-Budde wrote:
>> On 08/09/2011 07:55 AM, Robin Holt wrote:
>>> I added a clock source for the p1010rdb so the flexcan driver
>>> could find its clock frequency.
>>>
>>> Signed-off-by: Robin Holt <holt-sJ/iWh9BUns@public.gmane.org>
>>> To: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
>>> To: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>,
>>> To: U Bhaskar-B22300 <B22300-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>>> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
>>> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
>>> Cc: PPC list <linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org>
>>
>> After fixing Wolfgangs remarks add by Acked-by, for what it's worth in
>> the ppc-world :)
>
> Does that go for the other two patches in the series you have not
> commented on in many revs as well?
They look pretty good so far! I'll compile the next round on arm and
hopefully find time to test on HW.
cheers, 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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
[-- Attachment #2: Type: text/plain, Size: 188 bytes --]
_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core
^ permalink raw reply
* Re: [forcedeth bug] Re: [GIT] Networking
From: Jiri Pirko @ 2011-08-09 13:13 UTC (permalink / raw)
To: Ingo Molnar; +Cc: David Miller, torvalds, akpm, netdev, linux-kernel
In-Reply-To: <20110805143732.GH1928@minipsycho.orion>
Fri, Aug 05, 2011 at 04:37:33PM CEST, jpirko@redhat.com wrote:
>Fri, Aug 05, 2011 at 02:31:37PM CEST, jpirko@redhat.com wrote:
>>Fri, Aug 05, 2011 at 02:18:55PM CEST, mingo@elte.hu wrote:
>>>
>>>* Jiri Pirko <jpirko@redhat.com> wrote:
>>>
>>>> >> Is DEV_HAS_VLAN set in id->driver_data (L5344) ?
>>>> >
>>>> >How do i tell that without hacking the driver?
>>>>
>>>> look in dmesg for line like:
>>>> "forcedeth 0000:00:08.0: highdma csum vlan pwrctl mgmt gbit lnktim msi
>>>> desc-v3"
>>>>
>>>> if "vlan" is there, DEV_HAS_VLAN is set
>>>
>>>[ 3.534489] forcedeth 0000:00:0a.0: highdma csum gbit lnktim desc-v3
>>>
>>>Note, this is a pretty old system with an old nvidia chipset and
>>>on-board ethernet:
>>>
>>>00:0a.0 Bridge: nVidia Corporation CK804 Ethernet Controller (rev a3)
>>> Subsystem: ASUSTeK Computer Inc. K8N4-E Mainboard
>>> Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
>>> Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR-
>>> Latency: 0 (250ns min, 5000ns max)
>>> Interrupt: pin A routed to IRQ 11
>>> Region 0: Memory at da100000 (32-bit, non-prefetchable) [size=4K]
>>> Region 1: I/O ports at d000 [size=8]
>>> Capabilities: [44] Power Management version 2
>>> Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0+,D1+,D2+,D3hot+,D3cold+)
>>> Status: D0 PME-Enable+ DSel=0 DScale=0 PME-
>>
>>Please do lspci -nn
>>
>>There are two CK804 chips:
>>0x10DE, 0x0056
>>0x10DE, 0x0057
>>
>>I have only the second one handy - Getting the machine as we speak.
>
>I'm unable to see problems you are referring to on my machine.
>
>Would you please try following patch if it fixes your issue? It's
>in fact returning everything back to the original state (for your card).
Ingo, any news with this?
Thanks.
Jirka
>
>diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
>index e55df30..d7d43d4 100644
>--- a/drivers/net/forcedeth.c
>+++ b/drivers/net/forcedeth.c
>@@ -2763,18 +2763,18 @@ static int nv_rx_process_optimized(struct net_device *dev, int limit)
> skb->protocol = eth_type_trans(skb, dev);
> prefetch(skb->data);
>
>- vlanflags = le32_to_cpu(np->get_rx.ex->buflow);
>-
> /*
> * There's need to check for NETIF_F_HW_VLAN_RX here.
> * Even if vlan rx accel is disabled,
> * NV_RX3_VLAN_TAG_PRESENT is pseudo randomly set.
> */
>- if (dev->features & NETIF_F_HW_VLAN_RX &&
>- vlanflags & NV_RX3_VLAN_TAG_PRESENT) {
>- u16 vid = vlanflags & NV_RX3_VLAN_TAG_MASK;
>+ if (dev->features & NETIF_F_HW_VLAN_RX) {
>+ vlanflags = le32_to_cpu(np->get_rx.ex->buflow);
>+ if (vlanflags & NV_RX3_VLAN_TAG_PRESENT) {
>+ u16 vid = vlanflags & NV_RX3_VLAN_TAG_MASK;
>
>- __vlan_hwaccel_put_tag(skb, vid);
>+ __vlan_hwaccel_put_tag(skb, vid);
>+ }
> }
> napi_gro_receive(&np->napi, skb);
>
>@@ -5615,7 +5615,7 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i
> goto out_error;
> }
>
>- nv_vlan_mode(dev, dev->features);
>+ //nv_vlan_mode(dev, dev->features);
>
> netif_carrier_off(dev);
>
^ permalink raw reply
* [PATCH net 1/5] bnx2x: init FCOE FP only once
From: Dmitry Kravkov @ 2011-08-09 13:08 UTC (permalink / raw)
To: davem, netdev@vger.kernel.org; +Cc: Eilon Greenstein, Vladislav Zolotarov
From: Vladislav Zolotarov <vladz@broadcom.com>
Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/bnx2x/bnx2x_cmn.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bnx2x/bnx2x_cmn.c b/drivers/net/bnx2x/bnx2x_cmn.c
index d724a18..64df0ef 100644
--- a/drivers/net/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/bnx2x/bnx2x_cmn.c
@@ -63,8 +63,9 @@ static inline void bnx2x_bz_fp(struct bnx2x *bp, int index)
fp->disable_tpa = ((bp->flags & TPA_ENABLE_FLAG) == 0);
#ifdef BCM_CNIC
- /* We don't want TPA on FCoE, FWD and OOO L2 rings */
- bnx2x_fcoe(bp, disable_tpa) = 1;
+ /* We don't want TPA on an FCoE L2 ring */
+ if (IS_FCOE_FP(fp))
+ fp->disable_tpa = 1;
#endif
}
--
1.7.2.2
^ permalink raw reply related
* [PATCH net 2/5] bnx2x: fix select_queue when FCoE is disabled
From: Dmitry Kravkov @ 2011-08-09 13:08 UTC (permalink / raw)
To: davem, netdev@vger.kernel.org; +Cc: Vladislav Zolotarov, Eilon Greenstein
From: Vladislav Zolotarov <vladz@broadcom.com>
Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/bnx2x/bnx2x_cmn.c | 30 +++++++++++++++++++++++++-----
1 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/net/bnx2x/bnx2x_cmn.c b/drivers/net/bnx2x/bnx2x_cmn.c
index 64df0ef..37e5790 100644
--- a/drivers/net/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/bnx2x/bnx2x_cmn.c
@@ -1405,10 +1405,9 @@ void bnx2x_netif_stop(struct bnx2x *bp, int disable_hw)
u16 bnx2x_select_queue(struct net_device *dev, struct sk_buff *skb)
{
struct bnx2x *bp = netdev_priv(dev);
+
#ifdef BCM_CNIC
- if (NO_FCOE(bp))
- return skb_tx_hash(dev, skb);
- else {
+ if (!NO_FCOE(bp)) {
struct ethhdr *hdr = (struct ethhdr *)skb->data;
u16 ether_type = ntohs(hdr->h_proto);
@@ -1425,8 +1424,7 @@ u16 bnx2x_select_queue(struct net_device *dev, struct sk_buff *skb)
return bnx2x_fcoe_tx(bp, txq_index);
}
#endif
- /* Select a none-FCoE queue: if FCoE is enabled, exclude FCoE L2 ring
- */
+ /* select a non-FCoE queue */
return __skb_tx_hash(dev, skb, BNX2X_NUM_ETH_QUEUES(bp));
}
@@ -1449,6 +1447,28 @@ void bnx2x_set_num_queues(struct bnx2x *bp)
bp->num_queues += NON_ETH_CONTEXT_USE;
}
+/**
+ * bnx2x_set_real_num_queues - configure netdev->real_num_[tx,rx]_queues
+ *
+ * @bp: Driver handle
+ *
+ * We currently support for at most 16 Tx queues for each CoS thus we will
+ * allocate a multiple of 16 for ETH L2 rings according to the value of the
+ * bp->max_cos.
+ *
+ * If there is an FCoE L2 queue the appropriate Tx queue will have the next
+ * index after all ETH L2 indices.
+ *
+ * If the actual number of Tx queues (for each CoS) is less than 16 then there
+ * will be the holes at the end of each group of 16 ETh L2 indices (0..15,
+ * 16..31,...) with indicies that are not coupled with any real Tx queue.
+ *
+ * The proper configuration of skb->queue_mapping is handled by
+ * bnx2x_select_queue() and __skb_tx_hash().
+ *
+ * bnx2x_setup_tc() takes care of the proper TC mappings so that __skb_tx_hash()
+ * will return a proper Tx index if TC is enabled (netdev->num_tc > 0).
+ */
static inline int bnx2x_set_real_num_queues(struct bnx2x *bp)
{
int rc, tx, rx;
--
1.7.2.2
^ permalink raw reply related
* [PATCH net 3/5] bnx2x: prevent race between undi_unload and load flows
From: Dmitry Kravkov @ 2011-08-09 13:09 UTC (permalink / raw)
To: davem, netdev; +Cc: Vladislav Zolotarov, Eilon Greenstein
Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/bnx2x/bnx2x_main.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c
index 1507091..1f5467f 100644
--- a/drivers/net/bnx2x/bnx2x_main.c
+++ b/drivers/net/bnx2x/bnx2x_main.c
@@ -5798,6 +5798,12 @@ static int bnx2x_init_hw_common(struct bnx2x *bp)
DP(BNX2X_MSG_MCP, "starting common init func %d\n", BP_ABS_FUNC(bp));
+ /*
+ * take the UNDI lock to protect undi_unload flow from accessing
+ * registers while we're resetting the chip
+ */
+ bnx2x_acquire_hw_lock(bp, HW_LOCK_RESOURCE_UNDI);
+
bnx2x_reset_common(bp);
REG_WR(bp, GRCBASE_MISC + MISC_REGISTERS_RESET_REG_1_SET, 0xffffffff);
@@ -5808,6 +5814,8 @@ static int bnx2x_init_hw_common(struct bnx2x *bp)
}
REG_WR(bp, GRCBASE_MISC + MISC_REGISTERS_RESET_REG_2_SET, val);
+ bnx2x_release_hw_lock(bp, HW_LOCK_RESOURCE_UNDI);
+
bnx2x_init_block(bp, BLOCK_MISC, PHASE_COMMON);
if (!CHIP_IS_E1x(bp)) {
--
1.7.2.2
^ permalink raw reply related
* [PATCH net 4/5] bnx2x: properly clean indirect addresses
From: Dmitry Kravkov @ 2011-08-09 13:10 UTC (permalink / raw)
To: davem, netdev; +Cc: Vladislav Zolotarov, Eilon Greenstein
Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/bnx2x/bnx2x_main.c | 15 +++++++++++----
drivers/net/bnx2x/bnx2x_reg.h | 26 +++++++++++++++++++++-----
2 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c
index 1f5467f..f74582a 100644
--- a/drivers/net/bnx2x/bnx2x_main.c
+++ b/drivers/net/bnx2x/bnx2x_main.c
@@ -10259,10 +10259,17 @@ static int __devinit bnx2x_init_dev(struct pci_dev *pdev,
/* clean indirect addresses */
pci_write_config_dword(bp->pdev, PCICFG_GRC_ADDRESS,
PCICFG_VENDOR_ID_OFFSET);
- REG_WR(bp, PXP2_REG_PGL_ADDR_88_F0 + BP_PORT(bp)*16, 0);
- REG_WR(bp, PXP2_REG_PGL_ADDR_8C_F0 + BP_PORT(bp)*16, 0);
- REG_WR(bp, PXP2_REG_PGL_ADDR_90_F0 + BP_PORT(bp)*16, 0);
- REG_WR(bp, PXP2_REG_PGL_ADDR_94_F0 + BP_PORT(bp)*16, 0);
+ /* Clean the following indirect addresses for all functions since it
+ * is not used by the driver.
+ */
+ REG_WR(bp, PXP2_REG_PGL_ADDR_88_F0, 0);
+ REG_WR(bp, PXP2_REG_PGL_ADDR_8C_F0, 0);
+ REG_WR(bp, PXP2_REG_PGL_ADDR_90_F0, 0);
+ REG_WR(bp, PXP2_REG_PGL_ADDR_94_F0, 0);
+ REG_WR(bp, PXP2_REG_PGL_ADDR_88_F1, 0);
+ REG_WR(bp, PXP2_REG_PGL_ADDR_8C_F1, 0);
+ REG_WR(bp, PXP2_REG_PGL_ADDR_90_F1, 0);
+ REG_WR(bp, PXP2_REG_PGL_ADDR_94_F1, 0);
/*
* Enable internal target-read (in case we are probed after PF FLR).
diff --git a/drivers/net/bnx2x/bnx2x_reg.h b/drivers/net/bnx2x/bnx2x_reg.h
index 27b5ecb..40266c1 100644
--- a/drivers/net/bnx2x/bnx2x_reg.h
+++ b/drivers/net/bnx2x/bnx2x_reg.h
@@ -3007,11 +3007,27 @@
/* [R 6] Debug only: Number of used entries in the data FIFO */
#define PXP2_REG_HST_DATA_FIFO_STATUS 0x12047c
/* [R 7] Debug only: Number of used entries in the header FIFO */
-#define PXP2_REG_HST_HEADER_FIFO_STATUS 0x120478
-#define PXP2_REG_PGL_ADDR_88_F0 0x120534
-#define PXP2_REG_PGL_ADDR_8C_F0 0x120538
-#define PXP2_REG_PGL_ADDR_90_F0 0x12053c
-#define PXP2_REG_PGL_ADDR_94_F0 0x120540
+#define PXP2_REG_HST_HEADER_FIFO_STATUS 0x120478
+#define PXP2_REG_PGL_ADDR_88_F0 0x120534
+/* [R 32] GRC address for configuration access to PCIE config address 0x88.
+ * any write to this PCIE address will cause a GRC write access to the
+ * address that's in t this register */
+#define PXP2_REG_PGL_ADDR_88_F1 0x120544
+#define PXP2_REG_PGL_ADDR_8C_F0 0x120538
+/* [R 32] GRC address for configuration access to PCIE config address 0x8c.
+ * any write to this PCIE address will cause a GRC write access to the
+ * address that's in t this register */
+#define PXP2_REG_PGL_ADDR_8C_F1 0x120548
+#define PXP2_REG_PGL_ADDR_90_F0 0x12053c
+/* [R 32] GRC address for configuration access to PCIE config address 0x90.
+ * any write to this PCIE address will cause a GRC write access to the
+ * address that's in t this register */
+#define PXP2_REG_PGL_ADDR_90_F1 0x12054c
+#define PXP2_REG_PGL_ADDR_94_F0 0x120540
+/* [R 32] GRC address for configuration access to PCIE config address 0x94.
+ * any write to this PCIE address will cause a GRC write access to the
+ * address that's in t this register */
+#define PXP2_REG_PGL_ADDR_94_F1 0x120550
#define PXP2_REG_PGL_CONTROL0 0x120490
#define PXP2_REG_PGL_CONTROL1 0x120514
#define PXP2_REG_PGL_DEBUG 0x120520
--
1.7.2.2
^ permalink raw reply related
* [PATCH net 5/5] bnx2x: disable dcb on 578xx since not supported yet
From: Dmitry Kravkov @ 2011-08-09 13:11 UTC (permalink / raw)
To: davem, netdev@vger.kernel.org; +Cc: Vladislav Zolotarov, Eilon Greenstein
Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/bnx2x/bnx2x_dcb.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/bnx2x/bnx2x_dcb.c b/drivers/net/bnx2x/bnx2x_dcb.c
index a4ea35f..a1e004a 100644
--- a/drivers/net/bnx2x/bnx2x_dcb.c
+++ b/drivers/net/bnx2x/bnx2x_dcb.c
@@ -920,7 +920,7 @@ static void bnx2x_dcbx_admin_mib_updated_params(struct bnx2x *bp,
void bnx2x_dcbx_set_state(struct bnx2x *bp, bool dcb_on, u32 dcbx_enabled)
{
- if (!CHIP_IS_E1x(bp)) {
+ if (!CHIP_IS_E1x(bp) && !CHIP_IS_E3(bp)) {
bp->dcb_state = dcb_on;
bp->dcbx_enabled = dcbx_enabled;
} else {
--
1.7.2.2
^ permalink raw reply related
* [PATCH net 0/5] bnx2x fixes
From: Dmitry Kravkov @ 2011-08-09 13:13 UTC (permalink / raw)
To: davem; +Cc: Eilon Greenstein, Vladislav Zolotarov, netdev@vger.kernel.org
Hello Dave,
Please, consider applying the short bug-fixes series to the 'net'.
Thanks,
Dmitry
^ permalink raw reply
* Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
From: Robin Holt @ 2011-08-09 13:17 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Robin Holt, U Bhaskar-B22300, socketcan-core@lists.berlios.de,
netdev@vger.kernel.org, Devicetree-discuss@lists.ozlabs.org,
Marc Kleine-Budde
In-Reply-To: <4E413036.5080207@grandegger.com>
On Tue, Aug 09, 2011 at 03:03:50PM +0200, Wolfgang Grandegger wrote:
> On 08/09/2011 02:49 PM, Robin Holt wrote:
> > On Tue, Aug 09, 2011 at 12:41:39PM +0000, U Bhaskar-B22300 wrote:
> >>
> >>
> >>> -----Original Message-----
> >>> From: Wolfgang Grandegger [mailto:wg@grandegger.com]
> >>> Sent: Tuesday, August 09, 2011 4:19 PM
> >>> To: U Bhaskar-B22300
> >>> Cc: Marc Kleine-Budde; socketcan-core@lists.berlios.de;
> >>> netdev@vger.kernel.org; Devicetree-discuss@lists.ozlabs.org
> >>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
> >>>
> >>> On 08/09/2011 11:27 AM, U Bhaskar-B22300 wrote:
> >>>>
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Wolfgang Grandegger [mailto:wg@grandegger.com]
> >>>>> Sent: Tuesday, August 09, 2011 2:03 PM
> >>>>> To: U Bhaskar-B22300
> >>>>> Cc: Marc Kleine-Budde; socketcan-core@lists.berlios.de;
> >>>>> netdev@vger.kernel.org; Devicetree-discuss@lists.ozlabs.org
> >>>>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
> >>>>>
> >>>>> Hi Bhaskar,
> >>>>>
> >>>>> On 08/09/2011 09:57 AM, U Bhaskar-B22300 wrote:
> >>>>>>
> >>>>>>
> >>>>>>> -----Original Message-----
> >>>>>>> From: Marc Kleine-Budde [mailto:mkl@pengutronix.de]
> >>>>>>> Sent: Tuesday, August 09, 2011 12:23 AM
> >>>>>>> To: Wolfgang Grandegger
> >>>>>>> Cc: socketcan-core@lists.berlios.de; netdev@vger.kernel.org; U
> >>>>>>> Bhaskar- B22300
> >>>>>>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
> >>>>>>>
> >>>>>>> On 08/08/2011 05:33 PM, Wolfgang Grandegger wrote:
> >>>>>>>>> ACK - The device tree bindings as in mainline's Documentation is
> >>>>>>>>> a
> >>>>>>> mess.
> >>>>>>>>> If the powerpc guys are happy with a clock interfaces based
> >>>>>>>>> approach somewhere in arch/ppc, I'm more than happy to remove:
> >>>>>>>>> - fsl,flexcan-clock-source (not implemented, even in the fsl
> >>>>>>>>> driver)
> >>>>>> [Bhaskar]I have pushed the FlexCAN series of patches, It contains
> >>>>>> the usage of all the fields posted in the FlexCAN bindings at
> >>>>>> http://git.kernel.org/?p=linux/kernel/git/stable/linux-3.0.y.git;a=b
> >>>>>> lo
> >>>>>> b;f=Documentation/devicetree/bindings/net/can/fsl-flexcan.txt;h=1a72
> >>>>>> 9f
> >>>>>> 089866259ef82d0db5893ff7a8c54d5ccf;hb=94ed5b4788a7cdbe68bc7cb8516972
> >>>>>> cb
> >>>>>> ebdc8274
> >>>>>
> >>>>> As Marc already pointed out, Robin already has a much more advanced
> >>>>> patch stack in preparation. Especially your patches do not care about
> >>>>> the already existing Flexcan core on the Freescale's ARM socks.
> >>>> [Bhaskar] No, the patches are taking care of the existing ARM
> >>> functionality.
> >>>> I have not tested on the ARM based board, but the patches are made
> >>> in a
> >>>> Manner that it should not break the ARM based functionality.
> >>>>>
> >>>>>>>>>
> >>>>>>>>> - fsl,flexcan-clock-divider \__ replace with code in arch/ppc, or
> >>>>>>>>> - clock-frequency / a single clock-frequency attribute
> >>>>>>>>
> >>>>>>>> In the "net-next-2.6" tree there is also:
> >>>>>>>>
> >>>>>>>> $ grep flexcan arch/powerpc/boots/dts/*.dts
> >>>>>>>> p1010rdb.dts: fsl,flexcan-clock-source =
> >>>>> "platform";
> >>>>>>>> p1010rdb.dts: fsl,flexcan-clock-source =
> >>>>> "platform";
> >>>>>>>> p1010si.dtsi: compatible = "fsl,flexcan-v1.0";
> >>>>>>>> p1010si.dtsi: fsl,flexcan-clock-divider = <2>;
> >>>>>>>> p1010si.dtsi: compatible = "fsl,flexcan-v1.0";
> >>>>>>>> p1010si.dtsi: fsl,flexcan-clock-divider = <2>;
> >>>>>>>>
> >>>>>>>> Especially the fsl,flexcan-clock-divider = <2>; might make people
> >>>>>>>> think, that they could set something else.
> >>>>>>>
> >>>>>> [Bhaskar] As it is mentioned in the Flexcan bindings, the need of
> >>>>> fsl,flexcan-clock-divider = <2>;
> >>>>>> But I kept it as "2" because FlexCan clock source is the
> >>>>> platform clock and it is CCB/2
> >>>>>> If the "2" is misleading, the bindings can be changed or some
> >>>>> text can be written to make the meaning of "2"
> >>>>>> Understandable , Please suggest ..
> >>>>>
> >>>>> The clock source and frequency is fixed. Why do we need an extra
> >>>>> properties for that. We have panned to remove these bogus bindings
> >>>>> from the Linux kernel, which sneaked in *without* any review on the
> >>>>> relevant mailing lists (at least I have not realized any posting). We
> >>>>> do not think they are really needed. They just confuse the user. We
> >>>>> also prefer to use the compatibility string "fsl,flexcan" instead
> >>>>> "fsl,flexcan-v1.0". It's unusual to add a version number, which is
> >>>>> for the Flexcan on the PowerPC cores only, I assume, but there will
> >>>>> be device tree for ARM soon. A proper compatibility string would be
> >>>>> "fsl,p1010-flexcan" if we really need to distinguish.
> >>>>>
> >>>> [Bhaskar] About clock source.. There can be two sources of clock for
> >>> the CAN.
> >>>> Oscillator or the platform clock, but at present only platform
> >>> clock is supported
> >>>> in P1010.If we remove the fsl,flexcan-clock-source property, we
> >>> will lost the flexibility
> >>>> of changing the clock source ..
> >>>>
> >>>> About clock-frequency... it is also not fixed. It depends on
> >>> the platform clock which in turns
> >>>> Depends on the CCB clock. So it will be better to keep clock-
> >>> frequency property which is getting fixed via u-boot.
> >>>
> >>> The frequency is fixed to CCB-frequency / 2. Will that ever change? What
> >>> can we expect from future Flexcan hardware? Will it support further clock
> >>> sources?
> >> [Bhaskar] Yes the frequency will always be CCB-frequency/2.Even if the CCB gets changed that will be taken care by the u-boot fixup code for
> >> clock-frequency. clock-frequency is not filled by somebody in the dts file. It will be done by u-boot.
> >> For clock source,I can't say right now, that's why I have kept a property for this in the can node. So that in future, we need to fill it
> >> appropriately
> >
> > Speaking of the dts file, I have left the p1010si.dtsi file with
> > the fsl,flexcan-v1.0 .compatible definition. The flexcan folks
> > (IIRC Wolfgang) objected to that as it does not follow the standard
> > which should be just fsl,flexcan.
> >
> > How would you like to change that? Should I add it as part of this patch,
> > add another patch to the series, or let you take care of it?
> >
> > Also, I assume the uboot project will need to be changed as well to
> > reflect the corrected name.
>
> I think you should provide patches within this series to cleanup the
> obsolete stuff, dts and binding doc.
Will do.
Robin
^ permalink raw reply
* Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
From: Wolfgang Grandegger @ 2011-08-09 13:19 UTC (permalink / raw)
To: U Bhaskar-B22300
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
Marc Kleine-Budde
In-Reply-To: <9C64B7751C3BCA41B64A68E23005A7BE1C3552-TcFNo7jSaXPiTqIcKZ1S2K4g8xLGJsHaLnY5E4hWTkheoWH0uzbU5w@public.gmane.org>
On 08/09/2011 02:41 PM, U Bhaskar-B22300 wrote:
>
>
>> -----Original Message-----
>> From: Wolfgang Grandegger [mailto:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
...
>> The frequency is fixed to CCB-frequency / 2. Will that ever change? What
>> can we expect from future Flexcan hardware? Will it support further clock
>> sources?
> [Bhaskar] Yes the frequency will always be CCB-frequency/2.Even if the CCB gets changed that will be taken care by the u-boot fixup code for
> clock-frequency. clock-frequency is not filled by somebody in the dts file. It will be done by u-boot.
U-Boot fills in "clock_freq", anyway...
> For clock source,I can't say right now, that's why I have kept a property for this in the can node. So that in future, we need to fill it
> appropriately
This just confirms that we currently can live without clock-frequency,
clock-source and clock-divider properties. If it's really required
sometime in the future, we can add it, I think.
Wolfgang.
^ permalink raw reply
* Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
From: Robin Holt @ 2011-08-09 13:35 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
U Bhaskar-B22300,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
Marc Kleine-Budde
In-Reply-To: <4E413036.5080207-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
On Tue, Aug 09, 2011 at 03:03:50PM +0200, Wolfgang Grandegger wrote:
> On 08/09/2011 02:49 PM, Robin Holt wrote:
> > On Tue, Aug 09, 2011 at 12:41:39PM +0000, U Bhaskar-B22300 wrote:
> >>
> >>
> >>> -----Original Message-----
> >>> From: Wolfgang Grandegger [mailto:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
> >>> Sent: Tuesday, August 09, 2011 4:19 PM
> >>> To: U Bhaskar-B22300
> >>> Cc: Marc Kleine-Budde; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
> >>> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> >>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
> >>>
> >>> On 08/09/2011 11:27 AM, U Bhaskar-B22300 wrote:
> >>>>
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Wolfgang Grandegger [mailto:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
> >>>>> Sent: Tuesday, August 09, 2011 2:03 PM
> >>>>> To: U Bhaskar-B22300
> >>>>> Cc: Marc Kleine-Budde; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
> >>>>> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> >>>>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
> >>>>>
> >>>>> Hi Bhaskar,
> >>>>>
> >>>>> On 08/09/2011 09:57 AM, U Bhaskar-B22300 wrote:
> >>>>>>
> >>>>>>
> >>>>>>> -----Original Message-----
> >>>>>>> From: Marc Kleine-Budde [mailto:mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org]
> >>>>>>> Sent: Tuesday, August 09, 2011 12:23 AM
> >>>>>>> To: Wolfgang Grandegger
> >>>>>>> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; U
> >>>>>>> Bhaskar- B22300
> >>>>>>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
> >>>>>>>
> >>>>>>> On 08/08/2011 05:33 PM, Wolfgang Grandegger wrote:
> >>>>>>>>> ACK - The device tree bindings as in mainline's Documentation is
> >>>>>>>>> a
> >>>>>>> mess.
> >>>>>>>>> If the powerpc guys are happy with a clock interfaces based
> >>>>>>>>> approach somewhere in arch/ppc, I'm more than happy to remove:
> >>>>>>>>> - fsl,flexcan-clock-source (not implemented, even in the fsl
> >>>>>>>>> driver)
> >>>>>> [Bhaskar]I have pushed the FlexCAN series of patches, It contains
> >>>>>> the usage of all the fields posted in the FlexCAN bindings at
> >>>>>> http://git.kernel.org/?p=linux/kernel/git/stable/linux-3.0.y.git;a=b
> >>>>>> lo
> >>>>>> b;f=Documentation/devicetree/bindings/net/can/fsl-flexcan.txt;h=1a72
> >>>>>> 9f
> >>>>>> 089866259ef82d0db5893ff7a8c54d5ccf;hb=94ed5b4788a7cdbe68bc7cb8516972
> >>>>>> cb
> >>>>>> ebdc8274
> >>>>>
> >>>>> As Marc already pointed out, Robin already has a much more advanced
> >>>>> patch stack in preparation. Especially your patches do not care about
> >>>>> the already existing Flexcan core on the Freescale's ARM socks.
> >>>> [Bhaskar] No, the patches are taking care of the existing ARM
> >>> functionality.
> >>>> I have not tested on the ARM based board, but the patches are made
> >>> in a
> >>>> Manner that it should not break the ARM based functionality.
> >>>>>
> >>>>>>>>>
> >>>>>>>>> - fsl,flexcan-clock-divider \__ replace with code in arch/ppc, or
> >>>>>>>>> - clock-frequency / a single clock-frequency attribute
> >>>>>>>>
> >>>>>>>> In the "net-next-2.6" tree there is also:
> >>>>>>>>
> >>>>>>>> $ grep flexcan arch/powerpc/boots/dts/*.dts
> >>>>>>>> p1010rdb.dts: fsl,flexcan-clock-source =
> >>>>> "platform";
> >>>>>>>> p1010rdb.dts: fsl,flexcan-clock-source =
> >>>>> "platform";
> >>>>>>>> p1010si.dtsi: compatible = "fsl,flexcan-v1.0";
> >>>>>>>> p1010si.dtsi: fsl,flexcan-clock-divider = <2>;
> >>>>>>>> p1010si.dtsi: compatible = "fsl,flexcan-v1.0";
> >>>>>>>> p1010si.dtsi: fsl,flexcan-clock-divider = <2>;
> >>>>>>>>
> >>>>>>>> Especially the fsl,flexcan-clock-divider = <2>; might make people
> >>>>>>>> think, that they could set something else.
> >>>>>>>
> >>>>>> [Bhaskar] As it is mentioned in the Flexcan bindings, the need of
> >>>>> fsl,flexcan-clock-divider = <2>;
> >>>>>> But I kept it as "2" because FlexCan clock source is the
> >>>>> platform clock and it is CCB/2
> >>>>>> If the "2" is misleading, the bindings can be changed or some
> >>>>> text can be written to make the meaning of "2"
> >>>>>> Understandable , Please suggest ..
> >>>>>
> >>>>> The clock source and frequency is fixed. Why do we need an extra
> >>>>> properties for that. We have panned to remove these bogus bindings
> >>>>> from the Linux kernel, which sneaked in *without* any review on the
> >>>>> relevant mailing lists (at least I have not realized any posting). We
> >>>>> do not think they are really needed. They just confuse the user. We
> >>>>> also prefer to use the compatibility string "fsl,flexcan" instead
> >>>>> "fsl,flexcan-v1.0". It's unusual to add a version number, which is
> >>>>> for the Flexcan on the PowerPC cores only, I assume, but there will
> >>>>> be device tree for ARM soon. A proper compatibility string would be
> >>>>> "fsl,p1010-flexcan" if we really need to distinguish.
> >>>>>
> >>>> [Bhaskar] About clock source.. There can be two sources of clock for
> >>> the CAN.
> >>>> Oscillator or the platform clock, but at present only platform
> >>> clock is supported
> >>>> in P1010.If we remove the fsl,flexcan-clock-source property, we
> >>> will lost the flexibility
> >>>> of changing the clock source ..
> >>>>
> >>>> About clock-frequency... it is also not fixed. It depends on
> >>> the platform clock which in turns
> >>>> Depends on the CCB clock. So it will be better to keep clock-
> >>> frequency property which is getting fixed via u-boot.
> >>>
> >>> The frequency is fixed to CCB-frequency / 2. Will that ever change? What
> >>> can we expect from future Flexcan hardware? Will it support further clock
> >>> sources?
> >> [Bhaskar] Yes the frequency will always be CCB-frequency/2.Even if the CCB gets changed that will be taken care by the u-boot fixup code for
> >> clock-frequency. clock-frequency is not filled by somebody in the dts file. It will be done by u-boot.
> >> For clock source,I can't say right now, that's why I have kept a property for this in the can node. So that in future, we need to fill it
> >> appropriately
> >
> > Speaking of the dts file, I have left the p1010si.dtsi file with
> > the fsl,flexcan-v1.0 .compatible definition. The flexcan folks
> > (IIRC Wolfgang) objected to that as it does not follow the standard
> > which should be just fsl,flexcan.
> >
> > How would you like to change that? Should I add it as part of this patch,
> > add another patch to the series, or let you take care of it?
> >
> > Also, I assume the uboot project will need to be changed as well to
> > reflect the corrected name.
>
> I think you should provide patches within this series to cleanup the
> obsolete stuff, dts and binding doc.
It reads to me that the binding doc now reduces just the required
properties. Should I remove the file entirely?
Robin
^ permalink raw reply
* RE: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
From: U Bhaskar-B22300 @ 2011-08-09 13:44 UTC (permalink / raw)
To: Robin Holt, Wolfgang Grandegger
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
Marc Kleine-Budde
In-Reply-To: <20110809133531.GV4926-sJ/iWh9BUns@public.gmane.org>
> -----Original Message-----
> From: Robin Holt [mailto:holt-sJ/iWh9BUns@public.gmane.org]
> Sent: Tuesday, August 09, 2011 7:06 PM
> To: Wolfgang Grandegger
> Cc: Robin Holt; U Bhaskar-B22300; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org; Marc Kleine-
> Budde
> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
>
> On Tue, Aug 09, 2011 at 03:03:50PM +0200, Wolfgang Grandegger wrote:
> > On 08/09/2011 02:49 PM, Robin Holt wrote:
> > > On Tue, Aug 09, 2011 at 12:41:39PM +0000, U Bhaskar-B22300 wrote:
> > >>
> > >>
> > >>> -----Original Message-----
> > >>> From: Wolfgang Grandegger [mailto:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
> > >>> Sent: Tuesday, August 09, 2011 4:19 PM
> > >>> To: U Bhaskar-B22300
> > >>> Cc: Marc Kleine-Budde; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
> > >>> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> > >>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
> > >>>
> > >>> On 08/09/2011 11:27 AM, U Bhaskar-B22300 wrote:
> > >>>>
> > >>>>
> > >>>>> -----Original Message-----
> > >>>>> From: Wolfgang Grandegger [mailto:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
> > >>>>> Sent: Tuesday, August 09, 2011 2:03 PM
> > >>>>> To: U Bhaskar-B22300
> > >>>>> Cc: Marc Kleine-Budde; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
> > >>>>> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> > >>>>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock
> source.
> > >>>>>
> > >>>>> Hi Bhaskar,
> > >>>>>
> > >>>>> On 08/09/2011 09:57 AM, U Bhaskar-B22300 wrote:
> > >>>>>>
> > >>>>>>
> > >>>>>>> -----Original Message-----
> > >>>>>>> From: Marc Kleine-Budde [mailto:mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org]
> > >>>>>>> Sent: Tuesday, August 09, 2011 12:23 AM
> > >>>>>>> To: Wolfgang Grandegger
> > >>>>>>> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; U
> > >>>>>>> Bhaskar- B22300
> > >>>>>>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock
> source.
> > >>>>>>>
> > >>>>>>> On 08/08/2011 05:33 PM, Wolfgang Grandegger wrote:
> > >>>>>>>>> ACK - The device tree bindings as in mainline's
> > >>>>>>>>> Documentation is a
> > >>>>>>> mess.
> > >>>>>>>>> If the powerpc guys are happy with a clock interfaces based
> > >>>>>>>>> approach somewhere in arch/ppc, I'm more than happy to
> remove:
> > >>>>>>>>> - fsl,flexcan-clock-source (not implemented, even in the fsl
> > >>>>>>>>> driver)
> > >>>>>> [Bhaskar]I have pushed the FlexCAN series of patches, It
> > >>>>>> contains the usage of all the fields posted in the FlexCAN
> > >>>>>> bindings at
> > >>>>>> http://git.kernel.org/?p=linux/kernel/git/stable/linux-3.0.y.gi
> > >>>>>> t;a=b
> > >>>>>> lo
> > >>>>>> b;f=Documentation/devicetree/bindings/net/can/fsl-flexcan.txt;h
> > >>>>>> =1a72
> > >>>>>> 9f
> > >>>>>> 089866259ef82d0db5893ff7a8c54d5ccf;hb=94ed5b4788a7cdbe68bc7cb85
> > >>>>>> 16972
> > >>>>>> cb
> > >>>>>> ebdc8274
> > >>>>>
> > >>>>> As Marc already pointed out, Robin already has a much more
> > >>>>> advanced patch stack in preparation. Especially your patches do
> > >>>>> not care about the already existing Flexcan core on the
> Freescale's ARM socks.
> > >>>> [Bhaskar] No, the patches are taking care of the existing ARM
> > >>> functionality.
> > >>>> I have not tested on the ARM based board, but the patches are
> > >>>> made
> > >>> in a
> > >>>> Manner that it should not break the ARM based functionality.
> > >>>>>
> > >>>>>>>>>
> > >>>>>>>>> - fsl,flexcan-clock-divider \__ replace with code in
> arch/ppc, or
> > >>>>>>>>> - clock-frequency / a single clock-frequency
> attribute
> > >>>>>>>>
> > >>>>>>>> In the "net-next-2.6" tree there is also:
> > >>>>>>>>
> > >>>>>>>> $ grep flexcan arch/powerpc/boots/dts/*.dts
> > >>>>>>>> p1010rdb.dts: fsl,flexcan-clock-source =
> > >>>>> "platform";
> > >>>>>>>> p1010rdb.dts: fsl,flexcan-clock-source =
> > >>>>> "platform";
> > >>>>>>>> p1010si.dtsi: compatible = "fsl,flexcan-
> v1.0";
> > >>>>>>>> p1010si.dtsi: fsl,flexcan-clock-divider =
> <2>;
> > >>>>>>>> p1010si.dtsi: compatible = "fsl,flexcan-
> v1.0";
> > >>>>>>>> p1010si.dtsi: fsl,flexcan-clock-divider =
> <2>;
> > >>>>>>>>
> > >>>>>>>> Especially the fsl,flexcan-clock-divider = <2>; might make
> > >>>>>>>> people think, that they could set something else.
> > >>>>>>>
> > >>>>>> [Bhaskar] As it is mentioned in the Flexcan bindings, the need
> > >>>>>> of
> > >>>>> fsl,flexcan-clock-divider = <2>;
> > >>>>>> But I kept it as "2" because FlexCan clock source is the
> > >>>>> platform clock and it is CCB/2
> > >>>>>> If the "2" is misleading, the bindings can be changed or
> > >>>>>> some
> > >>>>> text can be written to make the meaning of "2"
> > >>>>>> Understandable , Please suggest ..
> > >>>>>
> > >>>>> The clock source and frequency is fixed. Why do we need an extra
> > >>>>> properties for that. We have panned to remove these bogus
> > >>>>> bindings from the Linux kernel, which sneaked in *without* any
> > >>>>> review on the relevant mailing lists (at least I have not
> > >>>>> realized any posting). We do not think they are really needed.
> > >>>>> They just confuse the user. We also prefer to use the
> > >>>>> compatibility string "fsl,flexcan" instead "fsl,flexcan-v1.0".
> > >>>>> It's unusual to add a version number, which is for the Flexcan
> > >>>>> on the PowerPC cores only, I assume, but there will be device
> > >>>>> tree for ARM soon. A proper compatibility string would be
> "fsl,p1010-flexcan" if we really need to distinguish.
> > >>>>>
> > >>>> [Bhaskar] About clock source.. There can be two sources of clock
> > >>>> for
> > >>> the CAN.
> > >>>> Oscillator or the platform clock, but at present only
> platform
> > >>> clock is supported
> > >>>> in P1010.If we remove the fsl,flexcan-clock-source property,
> we
> > >>> will lost the flexibility
> > >>>> of changing the clock source ..
> > >>>>
> > >>>> About clock-frequency... it is also not fixed. It
> > >>>> depends on
> > >>> the platform clock which in turns
> > >>>> Depends on the CCB clock. So it will be better to keep
> > >>>> clock-
> > >>> frequency property which is getting fixed via u-boot.
> > >>>
> > >>> The frequency is fixed to CCB-frequency / 2. Will that ever
> > >>> change? What can we expect from future Flexcan hardware? Will it
> > >>> support further clock sources?
> > >> [Bhaskar] Yes the frequency will always be CCB-frequency/2.Even if
> the CCB gets changed that will be taken care by the u-boot fixup code for
> > >> clock-frequency. clock-frequency is not filled by somebody in the
> dts file. It will be done by u-boot.
> > >> For clock source,I can't say right now, that's why I have kept a
> property for this in the can node. So that in future, we need to fill it
> > >> appropriately
> > >
> > > Speaking of the dts file, I have left the p1010si.dtsi file with the
> > > fsl,flexcan-v1.0 .compatible definition. The flexcan folks (IIRC
> > > Wolfgang) objected to that as it does not follow the standard which
> > > should be just fsl,flexcan.
> > >
> > > How would you like to change that? Should I add it as part of this
> > > patch, add another patch to the series, or let you take care of it?
> > >
> > > Also, I assume the uboot project will need to be changed as well to
> > > reflect the corrected name.
> >
> > I think you should provide patches within this series to cleanup the
> > obsolete stuff, dts and binding doc.
>
> It reads to me that the binding doc now reduces just the required
> properties. Should I remove the file entirely?
[Bhaskar] I think the binding doc should atleast be present with the required properties to give the clarity
about the CAN functionality
can0@1c000 {
compatible = "fsl,flexcan";
reg = <0x1c000 0x1000>;
interrupts = <48 0x2>;
interrupt-parent = <&mpic>;
clock-frequency = <fixed by u-boot>;
};
>
> Robin
^ permalink raw reply
* Re: [PATCH] net_sched: prio: use qdisc_dequeue_peeked
From: Florian Westphal @ 2011-08-09 13:45 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Florian Westphal, netdev
In-Reply-To: <1312894585.2371.24.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Eric, does this look correct to you?
> > I am not sure if sfq needs fixing instead of this patch here.
> >
> > diff --git a/net/sched/sch_prio.c b/net/sched/sch_prio.c
> > index 2a318f2..b5d56a2 100644
> > --- a/net/sched/sch_prio.c
> > +++ b/net/sched/sch_prio.c
> > @@ -112,7 +112,7 @@ static struct sk_buff *prio_dequeue(struct Qdisc *sch)
> >
> > for (prio = 0; prio < q->bands; prio++) {
> > struct Qdisc *qdisc = q->queues[prio];
> > - struct sk_buff *skb = qdisc->dequeue(qdisc);
> > + struct sk_buff *skb = qdisc_dequeue_peeked(qdisc);
> > if (skb) {
> > qdisc_bstats_update(sch, skb);
> > sch->q.qlen--;
>
>
> Hi Florian
>
> Are you sure this patch is still needed, after commit
> e1738bd9cecc5c867b0e2996470c1ff20f66ba79
> (sch_sfq: fix sfq_enqueue())
Yes, I double checked. This patch is applied, also
I see HFSC complaints even without that code path touched by
"sch_sfq: fix sfq_enqueue" being hit.
^ permalink raw reply
* Re: return of ip_rt_bug()
From: Julian Anastasov @ 2011-08-09 13:51 UTC (permalink / raw)
To: David Miller; +Cc: selinux, davej, netdev
In-Reply-To: <20110807.222042.1034530719366562334.davem@davemloft.net>
Hello,
On Sun, 7 Aug 2011, David Miller wrote:
> commit 1b86a58f9d7ce4fe2377687f378fbfb53bdc9b6c
> Author: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
> Date: Thu Apr 7 14:04:08 2011 -0700
>
> ipv4: Fix "Set rt->rt_iif more sanely on output routes."
I now checked these changes back to 2.6.38.
As rt_iif is used to provide input device even for loopback packets
that come with output route, may be we can optimize further
the code to save some CPU cycles. In fact, it restores
some route.c functions to 2.6.38 semantics. The conversion was:
fl.iif -> rt_route_iif
rt_iif -> preserved
There are other places that used fl.iif (0 for output
routes) but are now using rt_iif instead of rt_route_iif,
not sure if this change is fatal for them because:
- net/sched/cls_route.c, route4_classify() gets optional
iif, so it can be 0, may be to match output route? And
later route4_classify does exact match for rt_iif. Does
it mean that now we can not match output packets without
providing "fromif OUTDEV" ?
- net/sched/em_meta.c: now int_rtiif (being rt_iif) is
always != 0, may be not good to match output routes?
In short, the fl.iif -> rt_iif conversion is risky
at some places.
For now posting patch for route.c in another thread...
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
From: Robin Holt @ 2011-08-09 13:50 UTC (permalink / raw)
To: U Bhaskar-B22300
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
Marc Kleine-Budde, Wolfgang Grandegger
In-Reply-To: <9C64B7751C3BCA41B64A68E23005A7BE1C4746-TcFNo7jSaXPiTqIcKZ1S2K4g8xLGJsHaLnY5E4hWTkheoWH0uzbU5w@public.gmane.org>
On Tue, Aug 09, 2011 at 01:44:16PM +0000, U Bhaskar-B22300 wrote:
>
>
> > -----Original Message-----
> > From: Robin Holt [mailto:holt-sJ/iWh9BUns@public.gmane.org]
> > Sent: Tuesday, August 09, 2011 7:06 PM
> > To: Wolfgang Grandegger
> > Cc: Robin Holt; U Bhaskar-B22300; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
> > netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org; Marc Kleine-
> > Budde
> > Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
> >
> > On Tue, Aug 09, 2011 at 03:03:50PM +0200, Wolfgang Grandegger wrote:
> > > On 08/09/2011 02:49 PM, Robin Holt wrote:
...
> > > > How would you like to change that? Should I add it as part of this
> > > > patch, add another patch to the series, or let you take care of it?
> > > >
> > > > Also, I assume the uboot project will need to be changed as well to
> > > > reflect the corrected name.
> > >
> > > I think you should provide patches within this series to cleanup the
> > > obsolete stuff, dts and binding doc.
> >
> > It reads to me that the binding doc now reduces just the required
> > properties. Should I remove the file entirely?
> [Bhaskar] I think the binding doc should atleast be present with the required properties to give the clarity
> about the CAN functionality
> can0@1c000 {
> compatible = "fsl,flexcan";
> reg = <0x1c000 0x1000>;
> interrupts = <48 0x2>;
> interrupt-parent = <&mpic>;
> clock-frequency = <fixed by u-boot>;
> };
I am not sure what clarity we get for it. Here it is as a work in progress:
CAN Device Tree Bindings
------------------------
2011 Freescale Semiconductor, Inc.
fsl,flexcan nodes
-----------------------
Only the required compatible-, reg- and interrupt-properties are supported.
Examples:
can0@1c000 {
compatible = "fsl,flexcan";
reg = <0x1c000 0x1000>;
interrupts = <48 0x2>;
interrupt-parent = <&mpic>;
};
^ permalink raw reply
* [PATCH] ipv4: some rt_iif -> rt_route_iif conversions
From: Julian Anastasov @ 2011-08-09 14:01 UTC (permalink / raw)
To: David Miller; +Cc: netdev
As rt_iif represents input device even for packets
coming from loopback with output route, it is not an unique
key specific to input routes. Now rt_route_iif has such role,
it was fl.iif in 2.6.38, so better to change the checks at
some places to save CPU cycles and to restore 2.6.38 semantics.
compare_keys:
- input routes: only rt_route_iif matters, rt_iif is same
- output routes: only rt_oif matters, rt_iif is not
used for matching in __ip_route_output_key
- now we are back to 2.6.38 state
ip_route_input_common:
- matching rt_route_iif implies input route
- compared to 2.6.38 we eliminated one rth->fl.oif check
because it was not needed even for 2.6.38
compare_hash_inputs:
Only the change here is not an optimization, it has
effect only for output routes. I assume I'm restoring
the original intention to ignore oif, it was using fl.iif
- now we are back to 2.6.38 state
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
diff -urp linux/net/ipv4/route.c linux/net/ipv4/route.c
--- linux/net/ipv4/route.c 2011-08-09 12:40:43.000000000 +0300
+++ linux/net/ipv4/route.c 2011-08-09 12:46:16.880358197 +0300
@@ -716,7 +716,7 @@ static inline bool compare_hash_inputs(c
{
return ((((__force u32)rt1->rt_key_dst ^ (__force u32)rt2->rt_key_dst) |
((__force u32)rt1->rt_key_src ^ (__force u32)rt2->rt_key_src) |
- (rt1->rt_iif ^ rt2->rt_iif)) == 0);
+ (rt1->rt_route_iif ^ rt2->rt_route_iif)) == 0);
}
static inline int compare_keys(struct rtable *rt1, struct rtable *rt2)
@@ -726,8 +726,7 @@ static inline int compare_keys(struct rt
(rt1->rt_mark ^ rt2->rt_mark) |
(rt1->rt_key_tos ^ rt2->rt_key_tos) |
(rt1->rt_route_iif ^ rt2->rt_route_iif) |
- (rt1->rt_oif ^ rt2->rt_oif) |
- (rt1->rt_iif ^ rt2->rt_iif)) == 0;
+ (rt1->rt_oif ^ rt2->rt_oif)) == 0;
}
static inline int compare_netns(struct rtable *rt1, struct rtable *rt2)
@@ -2281,9 +2280,8 @@ int ip_route_input_common(struct sk_buff
rth = rcu_dereference(rth->dst.rt_next)) {
if ((((__force u32)rth->rt_key_dst ^ (__force u32)daddr) |
((__force u32)rth->rt_key_src ^ (__force u32)saddr) |
- (rth->rt_iif ^ iif) |
+ (rth->rt_route_iif ^ iif) |
(rth->rt_key_tos ^ tos)) == 0 &&
- rt_is_input_route(rth) &&
rth->rt_mark == skb->mark &&
net_eq(dev_net(rth->dst.dev), net) &&
!rt_is_expired(rth)) {
^ permalink raw reply
* Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
From: Wolfgang Grandegger @ 2011-08-09 14:03 UTC (permalink / raw)
To: U Bhaskar-B22300
Cc: Robin Holt, socketcan-core@lists.berlios.de,
netdev@vger.kernel.org, Devicetree-discuss@lists.ozlabs.org,
Marc Kleine-Budde
In-Reply-To: <9C64B7751C3BCA41B64A68E23005A7BE1C4746@039-SN1MPN1-002.039d.mgd.msft.net>
On 08/09/2011 03:44 PM, U Bhaskar-B22300 wrote:
>
>
>> -----Original Message-----
>> From: Robin Holt [mailto:holt@sgi.com]
>> Sent: Tuesday, August 09, 2011 7:06 PM
>> To: Wolfgang Grandegger
>> Cc: Robin Holt; U Bhaskar-B22300; socketcan-core@lists.berlios.de;
>> netdev@vger.kernel.org; Devicetree-discuss@lists.ozlabs.org; Marc Kleine-
>> Budde
>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
>>
>> On Tue, Aug 09, 2011 at 03:03:50PM +0200, Wolfgang Grandegger wrote:
>>> On 08/09/2011 02:49 PM, Robin Holt wrote:
>>>> On Tue, Aug 09, 2011 at 12:41:39PM +0000, U Bhaskar-B22300 wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Wolfgang Grandegger [mailto:wg@grandegger.com]
>>>>>> Sent: Tuesday, August 09, 2011 4:19 PM
>>>>>> To: U Bhaskar-B22300
>>>>>> Cc: Marc Kleine-Budde; socketcan-core@lists.berlios.de;
>>>>>> netdev@vger.kernel.org; Devicetree-discuss@lists.ozlabs.org
>>>>>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
>>>>>>
>>>>>> On 08/09/2011 11:27 AM, U Bhaskar-B22300 wrote:
>>>>>>>
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Wolfgang Grandegger [mailto:wg@grandegger.com]
>>>>>>>> Sent: Tuesday, August 09, 2011 2:03 PM
>>>>>>>> To: U Bhaskar-B22300
>>>>>>>> Cc: Marc Kleine-Budde; socketcan-core@lists.berlios.de;
>>>>>>>> netdev@vger.kernel.org; Devicetree-discuss@lists.ozlabs.org
>>>>>>>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock
>> source.
>>>>>>>>
>>>>>>>> Hi Bhaskar,
>>>>>>>>
>>>>>>>> On 08/09/2011 09:57 AM, U Bhaskar-B22300 wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: Marc Kleine-Budde [mailto:mkl@pengutronix.de]
>>>>>>>>>> Sent: Tuesday, August 09, 2011 12:23 AM
>>>>>>>>>> To: Wolfgang Grandegger
>>>>>>>>>> Cc: socketcan-core@lists.berlios.de; netdev@vger.kernel.org; U
>>>>>>>>>> Bhaskar- B22300
>>>>>>>>>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock
>> source.
>>>>>>>>>>
>>>>>>>>>> On 08/08/2011 05:33 PM, Wolfgang Grandegger wrote:
>>>>>>>>>>>> ACK - The device tree bindings as in mainline's
>>>>>>>>>>>> Documentation is a
>>>>>>>>>> mess.
>>>>>>>>>>>> If the powerpc guys are happy with a clock interfaces based
>>>>>>>>>>>> approach somewhere in arch/ppc, I'm more than happy to
>> remove:
>>>>>>>>>>>> - fsl,flexcan-clock-source (not implemented, even in the fsl
>>>>>>>>>>>> driver)
>>>>>>>>> [Bhaskar]I have pushed the FlexCAN series of patches, It
>>>>>>>>> contains the usage of all the fields posted in the FlexCAN
>>>>>>>>> bindings at
>>>>>>>>> http://git.kernel.org/?p=linux/kernel/git/stable/linux-3.0.y.gi
>>>>>>>>> t;a=b
>>>>>>>>> lo
>>>>>>>>> b;f=Documentation/devicetree/bindings/net/can/fsl-flexcan.txt;h
>>>>>>>>> =1a72
>>>>>>>>> 9f
>>>>>>>>> 089866259ef82d0db5893ff7a8c54d5ccf;hb=94ed5b4788a7cdbe68bc7cb85
>>>>>>>>> 16972
>>>>>>>>> cb
>>>>>>>>> ebdc8274
>>>>>>>>
>>>>>>>> As Marc already pointed out, Robin already has a much more
>>>>>>>> advanced patch stack in preparation. Especially your patches do
>>>>>>>> not care about the already existing Flexcan core on the
>> Freescale's ARM socks.
>>>>>>> [Bhaskar] No, the patches are taking care of the existing ARM
>>>>>> functionality.
>>>>>>> I have not tested on the ARM based board, but the patches are
>>>>>>> made
>>>>>> in a
>>>>>>> Manner that it should not break the ARM based functionality.
>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> - fsl,flexcan-clock-divider \__ replace with code in
>> arch/ppc, or
>>>>>>>>>>>> - clock-frequency / a single clock-frequency
>> attribute
>>>>>>>>>>>
>>>>>>>>>>> In the "net-next-2.6" tree there is also:
>>>>>>>>>>>
>>>>>>>>>>> $ grep flexcan arch/powerpc/boots/dts/*.dts
>>>>>>>>>>> p1010rdb.dts: fsl,flexcan-clock-source =
>>>>>>>> "platform";
>>>>>>>>>>> p1010rdb.dts: fsl,flexcan-clock-source =
>>>>>>>> "platform";
>>>>>>>>>>> p1010si.dtsi: compatible = "fsl,flexcan-
>> v1.0";
>>>>>>>>>>> p1010si.dtsi: fsl,flexcan-clock-divider =
>> <2>;
>>>>>>>>>>> p1010si.dtsi: compatible = "fsl,flexcan-
>> v1.0";
>>>>>>>>>>> p1010si.dtsi: fsl,flexcan-clock-divider =
>> <2>;
>>>>>>>>>>>
>>>>>>>>>>> Especially the fsl,flexcan-clock-divider = <2>; might make
>>>>>>>>>>> people think, that they could set something else.
>>>>>>>>>>
>>>>>>>>> [Bhaskar] As it is mentioned in the Flexcan bindings, the need
>>>>>>>>> of
>>>>>>>> fsl,flexcan-clock-divider = <2>;
>>>>>>>>> But I kept it as "2" because FlexCan clock source is the
>>>>>>>> platform clock and it is CCB/2
>>>>>>>>> If the "2" is misleading, the bindings can be changed or
>>>>>>>>> some
>>>>>>>> text can be written to make the meaning of "2"
>>>>>>>>> Understandable , Please suggest ..
>>>>>>>>
>>>>>>>> The clock source and frequency is fixed. Why do we need an extra
>>>>>>>> properties for that. We have panned to remove these bogus
>>>>>>>> bindings from the Linux kernel, which sneaked in *without* any
>>>>>>>> review on the relevant mailing lists (at least I have not
>>>>>>>> realized any posting). We do not think they are really needed.
>>>>>>>> They just confuse the user. We also prefer to use the
>>>>>>>> compatibility string "fsl,flexcan" instead "fsl,flexcan-v1.0".
>>>>>>>> It's unusual to add a version number, which is for the Flexcan
>>>>>>>> on the PowerPC cores only, I assume, but there will be device
>>>>>>>> tree for ARM soon. A proper compatibility string would be
>> "fsl,p1010-flexcan" if we really need to distinguish.
>>>>>>>>
>>>>>>> [Bhaskar] About clock source.. There can be two sources of clock
>>>>>>> for
>>>>>> the CAN.
>>>>>>> Oscillator or the platform clock, but at present only
>> platform
>>>>>> clock is supported
>>>>>>> in P1010.If we remove the fsl,flexcan-clock-source property,
>> we
>>>>>> will lost the flexibility
>>>>>>> of changing the clock source ..
>>>>>>>
>>>>>>> About clock-frequency... it is also not fixed. It
>>>>>>> depends on
>>>>>> the platform clock which in turns
>>>>>>> Depends on the CCB clock. So it will be better to keep
>>>>>>> clock-
>>>>>> frequency property which is getting fixed via u-boot.
>>>>>>
>>>>>> The frequency is fixed to CCB-frequency / 2. Will that ever
>>>>>> change? What can we expect from future Flexcan hardware? Will it
>>>>>> support further clock sources?
>>>>> [Bhaskar] Yes the frequency will always be CCB-frequency/2.Even if
>> the CCB gets changed that will be taken care by the u-boot fixup code for
>>>>> clock-frequency. clock-frequency is not filled by somebody in the
>> dts file. It will be done by u-boot.
>>>>> For clock source,I can't say right now, that's why I have kept a
>> property for this in the can node. So that in future, we need to fill it
>>>>> appropriately
>>>>
>>>> Speaking of the dts file, I have left the p1010si.dtsi file with the
>>>> fsl,flexcan-v1.0 .compatible definition. The flexcan folks (IIRC
>>>> Wolfgang) objected to that as it does not follow the standard which
>>>> should be just fsl,flexcan.
>>>>
>>>> How would you like to change that? Should I add it as part of this
>>>> patch, add another patch to the series, or let you take care of it?
>>>>
>>>> Also, I assume the uboot project will need to be changed as well to
>>>> reflect the corrected name.
>>>
>>> I think you should provide patches within this series to cleanup the
>>> obsolete stuff, dts and binding doc.
>>
>> It reads to me that the binding doc now reduces just the required
>> properties. Should I remove the file entirely?
> [Bhaskar] I think the binding doc should atleast be present with the required properties to give the clarity
> about the CAN functionality
> can0@1c000 {
> compatible = "fsl,flexcan";
> reg = <0x1c000 0x1000>;
> interrupts = <48 0x2>;
> interrupt-parent = <&mpic>;
> clock-frequency = <fixed by u-boot>;
> };
Yes, I also find the introduction is quite useful, with some related
correction.
Wolfgang.
^ permalink raw reply
* Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
From: Robin Holt @ 2011-08-09 14:09 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: U Bhaskar-B22300, Robin Holt, socketcan-core@lists.berlios.de,
netdev@vger.kernel.org, Devicetree-discuss@lists.ozlabs.org,
Marc Kleine-Budde
In-Reply-To: <4E413E3A.7050502@grandegger.com>
On Tue, Aug 09, 2011 at 04:03:38PM +0200, Wolfgang Grandegger wrote:
> On 08/09/2011 03:44 PM, U Bhaskar-B22300 wrote:
> >
> >
> >> -----Original Message-----
> >> From: Robin Holt [mailto:holt@sgi.com]
> >> Sent: Tuesday, August 09, 2011 7:06 PM
> >> To: Wolfgang Grandegger
> >> Cc: Robin Holt; U Bhaskar-B22300; socketcan-core@lists.berlios.de;
> >> netdev@vger.kernel.org; Devicetree-discuss@lists.ozlabs.org; Marc Kleine-
> >> Budde
> >> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
> >>
> >> On Tue, Aug 09, 2011 at 03:03:50PM +0200, Wolfgang Grandegger wrote:
> >>> On 08/09/2011 02:49 PM, Robin Holt wrote:
> >>>> On Tue, Aug 09, 2011 at 12:41:39PM +0000, U Bhaskar-B22300 wrote:
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Wolfgang Grandegger [mailto:wg@grandegger.com]
> >>>>>> Sent: Tuesday, August 09, 2011 4:19 PM
> >>>>>> To: U Bhaskar-B22300
> >>>>>> Cc: Marc Kleine-Budde; socketcan-core@lists.berlios.de;
> >>>>>> netdev@vger.kernel.org; Devicetree-discuss@lists.ozlabs.org
> >>>>>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
> >>>>>>
> >>>>>> On 08/09/2011 11:27 AM, U Bhaskar-B22300 wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>>> -----Original Message-----
> >>>>>>>> From: Wolfgang Grandegger [mailto:wg@grandegger.com]
> >>>>>>>> Sent: Tuesday, August 09, 2011 2:03 PM
> >>>>>>>> To: U Bhaskar-B22300
> >>>>>>>> Cc: Marc Kleine-Budde; socketcan-core@lists.berlios.de;
> >>>>>>>> netdev@vger.kernel.org; Devicetree-discuss@lists.ozlabs.org
> >>>>>>>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock
> >> source.
> >>>>>>>>
> >>>>>>>> Hi Bhaskar,
> >>>>>>>>
> >>>>>>>> On 08/09/2011 09:57 AM, U Bhaskar-B22300 wrote:
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>> -----Original Message-----
> >>>>>>>>>> From: Marc Kleine-Budde [mailto:mkl@pengutronix.de]
> >>>>>>>>>> Sent: Tuesday, August 09, 2011 12:23 AM
> >>>>>>>>>> To: Wolfgang Grandegger
> >>>>>>>>>> Cc: socketcan-core@lists.berlios.de; netdev@vger.kernel.org; U
> >>>>>>>>>> Bhaskar- B22300
> >>>>>>>>>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock
> >> source.
> >>>>>>>>>>
> >>>>>>>>>> On 08/08/2011 05:33 PM, Wolfgang Grandegger wrote:
> >>>>>>>>>>>> ACK - The device tree bindings as in mainline's
> >>>>>>>>>>>> Documentation is a
> >>>>>>>>>> mess.
> >>>>>>>>>>>> If the powerpc guys are happy with a clock interfaces based
> >>>>>>>>>>>> approach somewhere in arch/ppc, I'm more than happy to
> >> remove:
> >>>>>>>>>>>> - fsl,flexcan-clock-source (not implemented, even in the fsl
> >>>>>>>>>>>> driver)
> >>>>>>>>> [Bhaskar]I have pushed the FlexCAN series of patches, It
> >>>>>>>>> contains the usage of all the fields posted in the FlexCAN
> >>>>>>>>> bindings at
> >>>>>>>>> http://git.kernel.org/?p=linux/kernel/git/stable/linux-3.0.y.gi
> >>>>>>>>> t;a=b
> >>>>>>>>> lo
> >>>>>>>>> b;f=Documentation/devicetree/bindings/net/can/fsl-flexcan.txt;h
> >>>>>>>>> =1a72
> >>>>>>>>> 9f
> >>>>>>>>> 089866259ef82d0db5893ff7a8c54d5ccf;hb=94ed5b4788a7cdbe68bc7cb85
> >>>>>>>>> 16972
> >>>>>>>>> cb
> >>>>>>>>> ebdc8274
> >>>>>>>>
> >>>>>>>> As Marc already pointed out, Robin already has a much more
> >>>>>>>> advanced patch stack in preparation. Especially your patches do
> >>>>>>>> not care about the already existing Flexcan core on the
> >> Freescale's ARM socks.
> >>>>>>> [Bhaskar] No, the patches are taking care of the existing ARM
> >>>>>> functionality.
> >>>>>>> I have not tested on the ARM based board, but the patches are
> >>>>>>> made
> >>>>>> in a
> >>>>>>> Manner that it should not break the ARM based functionality.
> >>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> - fsl,flexcan-clock-divider \__ replace with code in
> >> arch/ppc, or
> >>>>>>>>>>>> - clock-frequency / a single clock-frequency
> >> attribute
> >>>>>>>>>>>
> >>>>>>>>>>> In the "net-next-2.6" tree there is also:
> >>>>>>>>>>>
> >>>>>>>>>>> $ grep flexcan arch/powerpc/boots/dts/*.dts
> >>>>>>>>>>> p1010rdb.dts: fsl,flexcan-clock-source =
> >>>>>>>> "platform";
> >>>>>>>>>>> p1010rdb.dts: fsl,flexcan-clock-source =
> >>>>>>>> "platform";
> >>>>>>>>>>> p1010si.dtsi: compatible = "fsl,flexcan-
> >> v1.0";
> >>>>>>>>>>> p1010si.dtsi: fsl,flexcan-clock-divider =
> >> <2>;
> >>>>>>>>>>> p1010si.dtsi: compatible = "fsl,flexcan-
> >> v1.0";
> >>>>>>>>>>> p1010si.dtsi: fsl,flexcan-clock-divider =
> >> <2>;
> >>>>>>>>>>>
> >>>>>>>>>>> Especially the fsl,flexcan-clock-divider = <2>; might make
> >>>>>>>>>>> people think, that they could set something else.
> >>>>>>>>>>
> >>>>>>>>> [Bhaskar] As it is mentioned in the Flexcan bindings, the need
> >>>>>>>>> of
> >>>>>>>> fsl,flexcan-clock-divider = <2>;
> >>>>>>>>> But I kept it as "2" because FlexCan clock source is the
> >>>>>>>> platform clock and it is CCB/2
> >>>>>>>>> If the "2" is misleading, the bindings can be changed or
> >>>>>>>>> some
> >>>>>>>> text can be written to make the meaning of "2"
> >>>>>>>>> Understandable , Please suggest ..
> >>>>>>>>
> >>>>>>>> The clock source and frequency is fixed. Why do we need an extra
> >>>>>>>> properties for that. We have panned to remove these bogus
> >>>>>>>> bindings from the Linux kernel, which sneaked in *without* any
> >>>>>>>> review on the relevant mailing lists (at least I have not
> >>>>>>>> realized any posting). We do not think they are really needed.
> >>>>>>>> They just confuse the user. We also prefer to use the
> >>>>>>>> compatibility string "fsl,flexcan" instead "fsl,flexcan-v1.0".
> >>>>>>>> It's unusual to add a version number, which is for the Flexcan
> >>>>>>>> on the PowerPC cores only, I assume, but there will be device
> >>>>>>>> tree for ARM soon. A proper compatibility string would be
> >> "fsl,p1010-flexcan" if we really need to distinguish.
> >>>>>>>>
> >>>>>>> [Bhaskar] About clock source.. There can be two sources of clock
> >>>>>>> for
> >>>>>> the CAN.
> >>>>>>> Oscillator or the platform clock, but at present only
> >> platform
> >>>>>> clock is supported
> >>>>>>> in P1010.If we remove the fsl,flexcan-clock-source property,
> >> we
> >>>>>> will lost the flexibility
> >>>>>>> of changing the clock source ..
> >>>>>>>
> >>>>>>> About clock-frequency... it is also not fixed. It
> >>>>>>> depends on
> >>>>>> the platform clock which in turns
> >>>>>>> Depends on the CCB clock. So it will be better to keep
> >>>>>>> clock-
> >>>>>> frequency property which is getting fixed via u-boot.
> >>>>>>
> >>>>>> The frequency is fixed to CCB-frequency / 2. Will that ever
> >>>>>> change? What can we expect from future Flexcan hardware? Will it
> >>>>>> support further clock sources?
> >>>>> [Bhaskar] Yes the frequency will always be CCB-frequency/2.Even if
> >> the CCB gets changed that will be taken care by the u-boot fixup code for
> >>>>> clock-frequency. clock-frequency is not filled by somebody in the
> >> dts file. It will be done by u-boot.
> >>>>> For clock source,I can't say right now, that's why I have kept a
> >> property for this in the can node. So that in future, we need to fill it
> >>>>> appropriately
> >>>>
> >>>> Speaking of the dts file, I have left the p1010si.dtsi file with the
> >>>> fsl,flexcan-v1.0 .compatible definition. The flexcan folks (IIRC
> >>>> Wolfgang) objected to that as it does not follow the standard which
> >>>> should be just fsl,flexcan.
> >>>>
> >>>> How would you like to change that? Should I add it as part of this
> >>>> patch, add another patch to the series, or let you take care of it?
> >>>>
> >>>> Also, I assume the uboot project will need to be changed as well to
> >>>> reflect the corrected name.
> >>>
> >>> I think you should provide patches within this series to cleanup the
> >>> obsolete stuff, dts and binding doc.
> >>
> >> It reads to me that the binding doc now reduces just the required
> >> properties. Should I remove the file entirely?
> > [Bhaskar] I think the binding doc should atleast be present with the required properties to give the clarity
> > about the CAN functionality
> > can0@1c000 {
> > compatible = "fsl,flexcan";
> > reg = <0x1c000 0x1000>;
> > interrupts = <48 0x2>;
> > interrupt-parent = <&mpic>;
> > clock-frequency = <fixed by u-boot>;
> > };
>
> Yes, I also find the introduction is quite useful, with some related
> correction.
I am not sure what is useful. The clock source bits are all wrong.
When that is removed, you end up with a discussion about the prescaler
which is actually related to the flexcan.c file and has nothing
to do with the device node. Maybe I am just going back into my
not-communicating-well mode. Could you follow up with what you think
belongs in the introduction of the binding file?
Thanks,
Robin
^ permalink raw reply
* Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
From: Wolfgang Grandegger @ 2011-08-09 14:14 UTC (permalink / raw)
To: Robin Holt
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
U Bhaskar-B22300, Marc Kleine-Budde
In-Reply-To: <20110809140901.GX4926-sJ/iWh9BUns@public.gmane.org>
On 08/09/2011 04:09 PM, Robin Holt wrote:
> On Tue, Aug 09, 2011 at 04:03:38PM +0200, Wolfgang Grandegger wrote:
>> On 08/09/2011 03:44 PM, U Bhaskar-B22300 wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Robin Holt [mailto:holt-sJ/iWh9BUns@public.gmane.org]
>>>> Sent: Tuesday, August 09, 2011 7:06 PM
>>>> To: Wolfgang Grandegger
>>>> Cc: Robin Holt; U Bhaskar-B22300; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
>>>> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org; Marc Kleine-
>>>> Budde
>>>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
>>>>
>>>> On Tue, Aug 09, 2011 at 03:03:50PM +0200, Wolfgang Grandegger wrote:
>>>>> On 08/09/2011 02:49 PM, Robin Holt wrote:
>>>>>> On Tue, Aug 09, 2011 at 12:41:39PM +0000, U Bhaskar-B22300 wrote:
>>>>>>>
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Wolfgang Grandegger [mailto:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
>>>>>>>> Sent: Tuesday, August 09, 2011 4:19 PM
>>>>>>>> To: U Bhaskar-B22300
>>>>>>>> Cc: Marc Kleine-Budde; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
>>>>>>>> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
>>>>>>>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
>>>>>>>>
>>>>>>>> On 08/09/2011 11:27 AM, U Bhaskar-B22300 wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: Wolfgang Grandegger [mailto:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
>>>>>>>>>> Sent: Tuesday, August 09, 2011 2:03 PM
>>>>>>>>>> To: U Bhaskar-B22300
>>>>>>>>>> Cc: Marc Kleine-Budde; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
>>>>>>>>>> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
>>>>>>>>>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock
>>>> source.
>>>>>>>>>>
>>>>>>>>>> Hi Bhaskar,
>>>>>>>>>>
>>>>>>>>>> On 08/09/2011 09:57 AM, U Bhaskar-B22300 wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>> From: Marc Kleine-Budde [mailto:mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org]
>>>>>>>>>>>> Sent: Tuesday, August 09, 2011 12:23 AM
>>>>>>>>>>>> To: Wolfgang Grandegger
>>>>>>>>>>>> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; U
>>>>>>>>>>>> Bhaskar- B22300
>>>>>>>>>>>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock
>>>> source.
>>>>>>>>>>>>
>>>>>>>>>>>> On 08/08/2011 05:33 PM, Wolfgang Grandegger wrote:
>>>>>>>>>>>>>> ACK - The device tree bindings as in mainline's
>>>>>>>>>>>>>> Documentation is a
>>>>>>>>>>>> mess.
>>>>>>>>>>>>>> If the powerpc guys are happy with a clock interfaces based
>>>>>>>>>>>>>> approach somewhere in arch/ppc, I'm more than happy to
>>>> remove:
>>>>>>>>>>>>>> - fsl,flexcan-clock-source (not implemented, even in the fsl
>>>>>>>>>>>>>> driver)
>>>>>>>>>>> [Bhaskar]I have pushed the FlexCAN series of patches, It
>>>>>>>>>>> contains the usage of all the fields posted in the FlexCAN
>>>>>>>>>>> bindings at
>>>>>>>>>>> http://git.kernel.org/?p=linux/kernel/git/stable/linux-3.0.y.gi
>>>>>>>>>>> t;a=b
>>>>>>>>>>> lo
>>>>>>>>>>> b;f=Documentation/devicetree/bindings/net/can/fsl-flexcan.txt;h
>>>>>>>>>>> =1a72
>>>>>>>>>>> 9f
>>>>>>>>>>> 089866259ef82d0db5893ff7a8c54d5ccf;hb=94ed5b4788a7cdbe68bc7cb85
>>>>>>>>>>> 16972
>>>>>>>>>>> cb
>>>>>>>>>>> ebdc8274
>>>>>>>>>>
>>>>>>>>>> As Marc already pointed out, Robin already has a much more
>>>>>>>>>> advanced patch stack in preparation. Especially your patches do
>>>>>>>>>> not care about the already existing Flexcan core on the
>>>> Freescale's ARM socks.
>>>>>>>>> [Bhaskar] No, the patches are taking care of the existing ARM
>>>>>>>> functionality.
>>>>>>>>> I have not tested on the ARM based board, but the patches are
>>>>>>>>> made
>>>>>>>> in a
>>>>>>>>> Manner that it should not break the ARM based functionality.
>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> - fsl,flexcan-clock-divider \__ replace with code in
>>>> arch/ppc, or
>>>>>>>>>>>>>> - clock-frequency / a single clock-frequency
>>>> attribute
>>>>>>>>>>>>>
>>>>>>>>>>>>> In the "net-next-2.6" tree there is also:
>>>>>>>>>>>>>
>>>>>>>>>>>>> $ grep flexcan arch/powerpc/boots/dts/*.dts
>>>>>>>>>>>>> p1010rdb.dts: fsl,flexcan-clock-source =
>>>>>>>>>> "platform";
>>>>>>>>>>>>> p1010rdb.dts: fsl,flexcan-clock-source =
>>>>>>>>>> "platform";
>>>>>>>>>>>>> p1010si.dtsi: compatible = "fsl,flexcan-
>>>> v1.0";
>>>>>>>>>>>>> p1010si.dtsi: fsl,flexcan-clock-divider =
>>>> <2>;
>>>>>>>>>>>>> p1010si.dtsi: compatible = "fsl,flexcan-
>>>> v1.0";
>>>>>>>>>>>>> p1010si.dtsi: fsl,flexcan-clock-divider =
>>>> <2>;
>>>>>>>>>>>>>
>>>>>>>>>>>>> Especially the fsl,flexcan-clock-divider = <2>; might make
>>>>>>>>>>>>> people think, that they could set something else.
>>>>>>>>>>>>
>>>>>>>>>>> [Bhaskar] As it is mentioned in the Flexcan bindings, the need
>>>>>>>>>>> of
>>>>>>>>>> fsl,flexcan-clock-divider = <2>;
>>>>>>>>>>> But I kept it as "2" because FlexCan clock source is the
>>>>>>>>>> platform clock and it is CCB/2
>>>>>>>>>>> If the "2" is misleading, the bindings can be changed or
>>>>>>>>>>> some
>>>>>>>>>> text can be written to make the meaning of "2"
>>>>>>>>>>> Understandable , Please suggest ..
>>>>>>>>>>
>>>>>>>>>> The clock source and frequency is fixed. Why do we need an extra
>>>>>>>>>> properties for that. We have panned to remove these bogus
>>>>>>>>>> bindings from the Linux kernel, which sneaked in *without* any
>>>>>>>>>> review on the relevant mailing lists (at least I have not
>>>>>>>>>> realized any posting). We do not think they are really needed.
>>>>>>>>>> They just confuse the user. We also prefer to use the
>>>>>>>>>> compatibility string "fsl,flexcan" instead "fsl,flexcan-v1.0".
>>>>>>>>>> It's unusual to add a version number, which is for the Flexcan
>>>>>>>>>> on the PowerPC cores only, I assume, but there will be device
>>>>>>>>>> tree for ARM soon. A proper compatibility string would be
>>>> "fsl,p1010-flexcan" if we really need to distinguish.
>>>>>>>>>>
>>>>>>>>> [Bhaskar] About clock source.. There can be two sources of clock
>>>>>>>>> for
>>>>>>>> the CAN.
>>>>>>>>> Oscillator or the platform clock, but at present only
>>>> platform
>>>>>>>> clock is supported
>>>>>>>>> in P1010.If we remove the fsl,flexcan-clock-source property,
>>>> we
>>>>>>>> will lost the flexibility
>>>>>>>>> of changing the clock source ..
>>>>>>>>>
>>>>>>>>> About clock-frequency... it is also not fixed. It
>>>>>>>>> depends on
>>>>>>>> the platform clock which in turns
>>>>>>>>> Depends on the CCB clock. So it will be better to keep
>>>>>>>>> clock-
>>>>>>>> frequency property which is getting fixed via u-boot.
>>>>>>>>
>>>>>>>> The frequency is fixed to CCB-frequency / 2. Will that ever
>>>>>>>> change? What can we expect from future Flexcan hardware? Will it
>>>>>>>> support further clock sources?
>>>>>>> [Bhaskar] Yes the frequency will always be CCB-frequency/2.Even if
>>>> the CCB gets changed that will be taken care by the u-boot fixup code for
>>>>>>> clock-frequency. clock-frequency is not filled by somebody in the
>>>> dts file. It will be done by u-boot.
>>>>>>> For clock source,I can't say right now, that's why I have kept a
>>>> property for this in the can node. So that in future, we need to fill it
>>>>>>> appropriately
>>>>>>
>>>>>> Speaking of the dts file, I have left the p1010si.dtsi file with the
>>>>>> fsl,flexcan-v1.0 .compatible definition. The flexcan folks (IIRC
>>>>>> Wolfgang) objected to that as it does not follow the standard which
>>>>>> should be just fsl,flexcan.
>>>>>>
>>>>>> How would you like to change that? Should I add it as part of this
>>>>>> patch, add another patch to the series, or let you take care of it?
>>>>>>
>>>>>> Also, I assume the uboot project will need to be changed as well to
>>>>>> reflect the corrected name.
>>>>>
>>>>> I think you should provide patches within this series to cleanup the
>>>>> obsolete stuff, dts and binding doc.
>>>>
>>>> It reads to me that the binding doc now reduces just the required
>>>> properties. Should I remove the file entirely?
>>> [Bhaskar] I think the binding doc should atleast be present with the required properties to give the clarity
>>> about the CAN functionality
>>> can0@1c000 {
>>> compatible = "fsl,flexcan";
>>> reg = <0x1c000 0x1000>;
>>> interrupts = <48 0x2>;
>>> interrupt-parent = <&mpic>;
>>> clock-frequency = <fixed by u-boot>;
>>> };
>>
>> Yes, I also find the introduction is quite useful, with some related
>> correction.
>
> I am not sure what is useful. The clock source bits are all wrong.
> When that is removed, you end up with a discussion about the prescaler
> which is actually related to the flexcan.c file and has nothing
> to do with the device node. Maybe I am just going back into my
> not-communicating-well mode. Could you follow up with what you think
> belongs in the introduction of the binding file?
Yep, you are right. Keep it simple...
Wolfgang,
^ permalink raw reply
* [PATCH v2] net: add Documentation/networking/scaling.txt
From: Willem de Bruijn @ 2011-08-09 14:20 UTC (permalink / raw)
To: rdunlap, linux-doc, davem, netdev, therbert
Describes RSS, RPS, RFS, accelerated RFS, and XPS.
This version incorporates comments by Randy Dunlap and Rick Jones.
Besides text cleanup, it adds an explicit "Suggested Configuration"
heading to each section.
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
Documentation/networking/scaling.txt | 372 ++++++++++++++++++++++++++++++++++
1 files changed, 372 insertions(+), 0 deletions(-)
create mode 100644 Documentation/networking/scaling.txt
diff --git a/Documentation/networking/scaling.txt b/Documentation/networking/scaling.txt
new file mode 100644
index 0000000..3da03c3
--- /dev/null
+++ b/Documentation/networking/scaling.txt
@@ -0,0 +1,372 @@
+Scaling in the Linux Networking Stack
+
+
+Introduction
+============
+
+This document describes a set of complementary techniques in the Linux
+networking stack to increase parallelism and improve performance for
+multi-processor systems.
+
+The following technologies are described:
+
+ RSS: Receive Side Scaling
+ RPS: Receive Packet Steering
+ RFS: Receive Flow Steering
+ Accelerated Receive Flow Steering
+ XPS: Transmit Packet Steering
+
+
+RSS: Receive Side Scaling
+=========================
+
+Contemporary NICs support multiple receive and transmit descriptor queues
+(multi-queue). On reception, a NIC can send different packets to different
+queues to distribute processing among CPUs. The NIC distributes packets by
+applying a filter to each packet that assigns it to one of a small number
+of logical flows. Packets for each flow are steered to a separate receive
+queue, which in turn can be processed by separate CPUs. This mechanism is
+generally known as “Receive-side Scaling” (RSS). The goal of RSS and
+the other scaling techniques to increase performance uniformly.
+Multi-queue distribution can also be used for traffic prioritization, but
+that is not the focus of these techniques.
+
+The filter used in RSS is typically a hash function over the network
+and/or transport layer headers-- for example, a 4-tuple hash over
+IP addresses and TCP ports of a packet. The most common hardware
+implementation of RSS uses a 128-entry indirection table where each entry
+stores a queue number. The receive queue for a packet is determined
+by masking out the low order seven bits of the computed hash for the
+packet (usually a Toeplitz hash), taking this number as a key into the
+indirection table and reading the corresponding value.
+
+Some advanced NICs allow steering packets to queues based on
+programmable filters. For example, webserver bound TCP port 80 packets
+can be directed to their own receive queue. Such “n-tuple” filters can
+be configured from ethtool (--config-ntuple).
+
+==== RSS Configuration
+
+The driver for a multi-queue capable NIC typically provides a kernel
+module parameter for specifying the number of hardware queues to
+configure. In the bnx2x driver, for instance, this parameter is called
+num_queues. A typical RSS configuration would be to have one receive queue
+for each CPU if the device supports enough queues, or otherwise at least
+one for each cache domain at a particular cache level (L1, L2, etc.).
+
+The indirection table of an RSS device, which resolves a queue by masked
+hash, is usually programmed by the driver at initialization. The
+default mapping is to distribute the queues evenly in the table, but the
+indirection table can be retrieved and modified at runtime using ethtool
+commands (--show-rxfh-indir and --set-rxfh-indir). Modifying the
+indirection table could be done to give different queues different
+relative weights.
+
+== RSS IRQ Configuration
+
+Each receive queue has a separate IRQ associated with it. The NIC triggers
+this to notify a CPU when new packets arrive on the given queue. The
+signaling path for PCIe devices uses message signaled interrupts (MSI-X),
+that can route each interrupt to a particular CPU. The active mapping
+of queues to IRQs can be determined from /proc/interrupts. By default,
+an IRQ may be handled on any CPU. Because a non-negligible part of packet
+processing takes place in receive interrupt handling, it is advantageous
+to spread receive interrupts between CPUs. To manually adjust the IRQ
+affinity of each interrupt see Documentation/IRQ-affinity. Some systems
+will be running irqbalance, a daemon that dynamically optimizes IRQ
+assignments and as a result may override any manual settings.
+
+== Suggested Configuration
+
+RSS should be enabled when latency is a concern or whenever receive
+interrupt processing forms a bottleneck. Spreading load between CPUs
+decreases queue length. For low latency networking, the optimal setting
+is to allocate as many queues as there are CPUs in the system (or the
+NIC maximum, if lower). Because the aggregate number of interrupts grows
+with each additional queue, the most efficient high-rate configuration
+is likely the one with the smallest number of receive queues where no
+CPU that processes receive interrupts reaches 100% utilization. Per-cpu
+load can be observed using the mpstat utility.
+
+
+RPS: Receive Packet Steering
+============================
+
+Receive Packet Steering (RPS) is logically a software implementation of
+RSS. Being in software, it is necessarily called later in the datapath.
+Whereas RSS selects the queue and hence CPU that will run the hardware
+interrupt handler, RPS selects the CPU to perform protocol processing
+above the interrupt handler. This is accomplished by placing the packet
+on the desired CPU’s backlog queue and waking up the CPU for processing.
+RPS has some advantages over RSS: 1) it can be used with any NIC,
+2) software filters can easily be added to hash over new protocols,
+3) it does not increase hardware device interrupt rate (although it does
+introduce inter-processor interrupts (IPIs)).
+
+RPS is called during bottom half of the receive interrupt handler, when
+a driver sends a packet up the network stack with netif_rx() or
+netif_receive_skb(). These call the get_rps_cpu() function, which
+selects the queue that should process a packet.
+
+The first step in determining the target CPU for RPS is to calculate a
+flow hash over the packet’s addresses or ports (2-tuple or 4-tuple hash
+depending on the protocol). This serves as a consistent hash of the
+associated flow of the packet. The hash is either provided by hardware
+or will be computed in the stack. Capable hardware can pass the hash in
+the receive descriptor for the packet; this would usually be the same
+hash used for RSS (e.g. computed Toeplitz hash). The hash is saved in
+skb->rx_hash and can be used elsewhere in the stack as a hash of the
+packet’s flow.
+
+Each receive hardware queue has an associated list of CPUs to which
+RPS may enqueue packets for processing. For each received packet,
+an index into the list is computed from the flow hash modulo the size
+of the list. The indexed CPU is the target for processing the packet,
+and the packet is queued to the tail of that CPU’s backlog queue. At
+the end of the bottom half routine, IPIs are sent to any CPUs for which
+packets have been queued to their backlog queue. The IPI wakes backlog
+processing on the remote CPU, and any queued packets are then processed
+up the networking stack.
+
+==== RPS Configuration
+
+RPS requires a kernel compiled with the CONFIG_RPS kconfig symbol (on
+by default for SMP). Even when compiled in, RPS remains disabled until
+explicitly configured. The list of CPUs to which RPS may forward traffic
+can be configured for each receive queue using a sysfs file entry:
+
+ /sys/class/net/<dev>/queues/rx-<n>/rps_cpus
+
+This file implements a bitmap of CPUs. RPS is disabled when it is zero
+(the default), in which case packets are processed on the interrupting
+CPU. Documentation/IRQ-affinity.txt explains how CPUs are assigned to
+the bitmap.
+
+== Suggested Configuration
+
+For a single queue device, a typical RPS configuration would be to set
+the rps_cpus to the CPUs in the same cache domain of the interrupting
+CPU. If NUMA locality is not an issue, this could also be all CPUs in
+the system. At high interrupt rate, it might be wise to exclude the
+interrupting CPU from the map since that already performs much work.
+
+For a multi-queue system, if RSS is configured so that a hardware
+receive queue is mapped to each CPU, then RPS is probably redundant
+and unnecessary. If there are fewer hardware queues than CPUs, then
+RPS might be beneficial if the rps_cpus for each queue are the ones that
+share the same cache domain as the interrupting CPU for that queue.
+
+
+RFS: Receive Flow Steering
+==========================
+
+While RPS steers packets solely based on hash, and thus generally
+provides good load distribution, it does not take into account
+application locality. This is accomplished by Receive Flow Steering
+(RFS). The goal of RFS is to increase datacache hitrate by steering
+kernel processing of packets to the CPU where the application thread
+consuming the packet is running. RFS relies on the same RPS mechanisms
+to enqueue packets onto the backlog of another CPU and to wake up that
+CPU.
+
+In RFS, packets are not forwarded directly by the value of their hash,
+but the hash is used as index into a flow lookup table. This table maps
+flows to the CPUs where those flows are being processed. The flow hash
+(see RPS section above) is used to calculate the index into this table.
+The CPU recorded in each entry is the one which last processed the flow.
+If an entry does not hold a valid CPU, then packets mapped to that entry
+are steered using plain RPS. Multiple table entries may point to the
+same CPU. Indeed, with many flows and few CPUs, it is very likely that
+a single application thread handles flows with many different flow hashes.
+
+rps_sock_table is a global flow table that contains the *desired* CPU for
+flows: the CPU that is currently processing the flow in userspace. Each
+table value is a CPU index that is updated during calls to recvmsg and
+sendmsg (specifically, inet_recvmsg(), inet_sendmsg(), inet_sendpage()
+and tcp_splice_read()).
+
+When the scheduler moves a thread to a new CPU while it has outstanding
+receive packets on the old CPU, packets may arrive out of order. To
+avoid this, RFS uses a second flow table to track outstanding packets
+for each flow: rps_dev_flow_table is a table specific to each hardware
+receive queue of each device. Each table value stores a CPU index and a
+counter. The CPU index represents the *current* CPU onto which packets
+for this flow are enqueued for further kernel processing. Ideally, kernel
+and userspace processing occur on the same CPU, and hence the CPU index
+in both tables is identical. This is likely false if the scheduler has
+recently migrated a userspace thread while the kernel still has packets
+enqueued for kernel processing on the old CPU.
+
+The counter in rps_dev_flow_table values records the length of the current
+CPU's backlog when a packet in this flow was last enqueued. Each backlog
+queue has a head counter that is incremented on dequeue. A tail counter
+is computed as head counter + queue length. In other words, the counter
+in rps_dev_flow_table[i] records the last element in flow i that has
+been enqueued onto the currently designated CPU for flow i (of course,
+entry i is actually selected by hash and multiple flows may hash to the
+same entry i).
+
+And now the trick for avoiding out of order packets: when selecting the
+CPU for packet processing (from get_rps_cpu()) the rps_sock_flow table
+and the rps_dev_flow table of the queue that the packet was received on
+are compared. If the desired CPU for the flow (found in the
+rps_sock_flow table) matches the current CPU (found in the rps_dev_flow
+table), the packet is enqueued onto that CPU’s backlog. If they differ,
+the current CPU is updated to match the desired CPU if one of the
+following is true:
+
+- The current CPU's queue head counter >= the recorded tail counter
+ value in rps_dev_flow[i]
+- The current CPU is unset (equal to NR_CPUS)
+- The current CPU is offline
+
+After this check, the packet is sent to the (possibly updated) current
+CPU. These rules aim to ensure that a flow only moves to a new CPU when
+there are no packets outstanding on the old CPU, as the outstanding
+packets could arrive later than those about to be processed on the new
+CPU.
+
+==== RFS Configuration
+
+RFS is only available if the kconfig symbol CONFIG_RFS is enabled (on
+by default for SMP). The functionality remains disabled until explicitly
+configured. The number of entries in the global flow table is set through:
+
+ /proc/sys/net/core/rps_sock_flow_entries
+
+The number of entries in the per-queue flow table are set through:
+
+ /sys/class/net/<dev>/queues/tx-<n>/rps_flow_cnt
+
+== Suggested Configuration
+
+Both of these need to be set before RFS is enabled for a receive queue.
+Values for both are rounded up to the nearest power of two. The
+suggested flow count depends on the expected number of active connections
+at any given time, which may be significantly less than the number of open
+connections. We have found that a value of 32768 for rps_sock_flow_entries
+works fairly well on a moderately loaded server.
+
+For a single queue device, the rps_flow_cnt value for the single queue
+would normally be configured to the same value as rps_sock_flow_entries.
+For a multi-queue device, the rps_flow_cnt for each queue might be
+configured as rps_sock_flow_entries / N, where N is the number of
+queues. So for instance, if rps_flow_entries is set to 32768 and there
+are 16 configured receive queues, rps_flow_cnt for each queue might be
+configured as 2048.
+
+
+Accelerated RFS
+===============
+
+Accelerated RFS is to RFS what RSS is to RPS: a hardware-accelerated load
+balancing mechanism that uses soft state to steer flows based on where
+the application thread consuming the packets of each flow is running.
+Accelerated RFS should perform better than RFS since packets are sent
+directly to a CPU local to the thread consuming the data. The target CPU
+will either be the same CPU where the application runs, or at least a CPU
+which is local to the application thread’s CPU in the cache hierarchy.
+
+To enable accelerated RFS, the networking stack calls the
+ndo_rx_flow_steer driver function to communicate the desired hardware
+queue for packets matching a particular flow. The network stack
+automatically calls this function every time a flow entry in
+rps_dev_flow_table is updated. The driver in turn uses a device specific
+method to program the NIC to steer the packets.
+
+The hardware queue for a flow is derived from the CPU recorded in
+rps_dev_flow_table. The stack consults a CPU to hardware queue map which
+is maintained by the NIC driver. This is an auto-generated reverse map of
+the IRQ affinity table shown by /proc/interrupts. Drivers can use
+functions in the cpu_rmap (“CPU affinity reverse map”) kernel library
+to populate the map. For each CPU, the corresponding queue in the map is
+set to be one whose processing CPU is closest in cache locality.
+
+==== Accelerated RFS Configuration
+
+Accelerated RFS is only available if the kernel is compiled with
+CONFIG_RFS_ACCEL and support is provided by the NIC device and driver.
+It also requires that ntuple filtering is enabled via ethtool. The map
+of CPU to queues is automatically deduced from the IRQ affinities
+configured for each receive queue by the driver, so no additional
+configuration should be necessary.
+
+== Suggested Configuration
+
+This technique should be enabled whenever one wants to use RFS and the
+NIC supports hardware acceleration.
+
+XPS: Transmit Packet Steering
+=============================
+
+Transmit Packet Steering is a mechanism for intelligently selecting
+which transmit queue to use when transmitting a packet on a multi-queue
+device. To accomplish this, a mapping from CPU to hardware queue(s) is
+recorded. The goal of this mapping is usually to assign queues
+exclusively to a subset of CPUs, where the transmit completions for
+these queues are processed on a CPU within this set. This choice
+provides two benefits. First, contention on the device queue lock is
+significantly reduced since fewer CPUs contend for the same queue
+(contention can be eliminated completely if each CPU has its own
+transmit queue). Secondly, cache miss rate on transmit completion is
+reduced, in particular for data cache lines that hold the sk_buff
+structures.
+
+XPS is configured per transmit queue by setting a bitmap of CPUs that
+may use that queue to transmit. The reverse mapping, from CPUs to
+transmit queues, is computed and maintained for each network device.
+When transmitting the first packet in a flow, the function
+get_xps_queue() is called to select a queue. This function uses the ID
+of the running CPU as a key into the CPU-to-queue lookup table. If the
+ID matches a single queue, that is used for transmission. If multiple
+queues match, one is selected by using the flow hash to compute an index
+into the set.
+
+The queue chosen for transmitting a particular flow is saved in the
+corresponding socket structure for the flow (e.g. a TCP connection).
+This transmit queue is used for subsequent packets sent on the flow to
+prevent out of order (ooo) packets. The choice also amortizes the cost
+of calling get_xps_queues() over all packets in the connection. To avoid
+ooo packets, the queue for a flow can subsequently only be changed if
+skb->ooo_okay is set for a packet in the flow. This flag indicates that
+there are no outstanding packets in the flow, so the transmit queue can
+change without the risk of generating out of order packets. The
+transport layer is responsible for setting ooo_okay appropriately. TCP,
+for instance, sets the flag when all data for a connection has been
+acknowledged.
+
+==== XPS Configuration
+
+XPS is only available if the kconfig symbol CONFIG_XPS is enabled (on by
+default for SMP). The functionality remains disabled until explicitly
+configured. To enable XPS, the bitmap of CPUs that may use a transmit
+queue is configured using the sysfs file entry:
+
+/sys/class/net/<dev>/queues/tx-<n>/xps_cpus
+
+== Suggested Configuration
+
+For a network device with a single transmission queue, XPS configuration
+has no effect, since there is no choice in this case. In a multi-queue
+system, XPS is preferably configured so that each CPU maps onto one queue.
+If there are as many queues as there are CPUs in the system, then each
+queue can also map onto one CPU, resulting in exclusive pairings that
+experience no contention. If there are fewer queues than CPUs, then the
+best CPUs to share a given queue are probably those that share the cache
+with the CPU that processes transmit completions for that queue
+(transmit interrupts).
+
+
+Further Information
+===================
+RPS and RFS were introduced in kernel 2.6.35. XPS was incorporated into
+2.6.38. Original patches were submitted by Tom Herbert
+(therbert@google.com)
+
+Accelerated RFS was introduced in 2.6.35. Original patches were
+submitted by Ben Hutchings (bhutchings@solarflare.com)
+
+Authors:
+Tom Herbert (therbert@google.com)
+Willem de Bruijn (willemb@google.com)
+
--
1.7.3.1
^ permalink raw reply related
* Re: [PATCH net-next 2/2] bnx2x: Coalesce pr_cont uses and fix DP typos
From: Eilon Greenstein @ 2011-08-09 14:41 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <98d09e1b3d7a73bf5f2744f0ca238321573e4579.1312818707.git.joe@perches.com>
On Mon, 2011-08-08 at 08:53 -0700, Joe Perches wrote:
> Uses of pr_cont should be avoided where reasonably possible
> because they can be interleaved by other threads and processes.
>
> Coalesce pr_cont uses.
>
> Fix typos, duplicated words and spacing in DP uses caused
> by split multi-line formats. Coalesce some of these
> split formats. Add missing terminating newlines to DP uses.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
....
> @@ -12254,7 +12261,7 @@ void bnx2x_period_func(struct link_params *params, struct link_vars *vars)
> {
> struct bnx2x *bp = params->bp;
> if (!params) {
> - DP(NETIF_MSG_LINK, "Ininitliazed params !\n");
> + DP(NETIF_MSG_LINK, "Uninitialized params !\n");
> return;
> }
> /* DP(NETIF_MSG_LINK, "Periodic called vars->phy_flags 0x%x speed 0x%x
This does not apply on the current net-next. Other than that - it looks
good.
Please re-submit this patch series once net-next is opened for driver
changes (see: http://marc.info/?l=linux-netdev&m=131278578427946&w=2 )
and I will ACK it.
Thanks,
Eilon
^ 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