* Re: [RFC PATCH] et1011c: Replaced PHY driver by a small dm646x board fixup
From: Andy Fleming @ 2011-03-24 0:24 UTC (permalink / raw)
To: Kyle Moffett
Cc: netdev, Kevin Hilman, Russell King, David S. Miller,
Uwe Kleine-König, David J. Choi, Giuseppe Cavallaro,
Randy Dunlap, linux-arm-kernel, linux-kernel
In-Reply-To: <1300919616-6780-1-git-send-email-Kyle.D.Moffett@boeing.com>
On Wed, Mar 23, 2011 at 5:33 PM, Kyle Moffett <Kyle.D.Moffett@boeing.com> wrote:
> -
> -static struct phy_driver et1011c_driver = {
> - .phy_id = 0x0282f014,
> - .name = "ET1011C",
> - .phy_id_mask = 0xfffffff0,
> - .features = (PHY_BASIC_FEATURES | SUPPORTED_1000baseT_Full),
> - .flags = PHY_POLL,
> - .config_aneg = et1011c_config_aneg,
> - .read_status = et1011c_read_status,
> - .driver = { .owner = THIS_MODULE,},
> -};
Might I suggest that you not eliminate the whole driver? If you leave
just this part (and the init stuff below it), and convert the
config_aneg and read_status pointers to use the genphy versions, the
kernel will, at least, be able to report what type of PHY it is. And
maybe one day, someone who is familiar with the inner workings of this
PHY will fill in a more correct driver.
Andy
^ permalink raw reply
* Re: [PATCH 2/2] virtio_net: remove send completion interrupts and avoid TX queue overrun through packet drop
From: Rusty Russell @ 2011-03-24 0:16 UTC (permalink / raw)
To: Michael S. Tsirkin, Shirley Ma; +Cc: Herbert Xu, davem, kvm, netdev
In-Reply-To: <20110322113649.GA17071@redhat.com>
On Tue, 22 Mar 2011 13:36:50 +0200, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Mon, Mar 21, 2011 at 11:03:07AM -0700, Shirley Ma wrote:
> > On Fri, 2011-03-18 at 18:41 -0700, Shirley Ma wrote:
> > > > > + /* Drop packet instead of stop queue for better
> > > performance
> > > > */
> > > >
> > > > I would like to see some justification as to why this is the right
> > > > way to go and not just papering over the real problem.
> > >
> > > Fair. KVM guest virtio_net TX queue stop/restart is pretty expensive,
> > > which involves:
> > >
> > > 1. Guest enable callback: one memory barrier, interrupt flag set
> >
> > Missed this cost: for history reason, it also involves a guest exit from
> > I/O write (PCI_QUEUE_NOTIFY).
>
> OK, after some research, it looks like the reason was the tx timer that
> qemu used to use. So the hack of avoiding the add_buf call will
> avoid this kick and so break these hosts.
> I guess we can add a feature bit to detect a new host
> and so avoid the kick. We are running low on feature bits
> unfortunately, but just fo testing, could you quantify the difference
> that this makes using the following patch:
Performance would suffer for those ancient qemus if we didn't do this,
but it wouldn't be fatal to them.
I think we should just remove it; the standard certainly doesn't mention
it.
Cheers,
Rusty.
^ permalink raw reply
* Re: [RFC] usbnet: use eth%d name for known ethernet devices
From: Ben Hutchings @ 2011-03-24 0:01 UTC (permalink / raw)
To: Steve Calfee
Cc: Michal Nazarewicz, Randy Dunlap, Arnd Bergmann, broonie, lkml,
Nicolas Pitre, Greg KH, David Brownell, Alan Cox, grant.likely,
Linux USB list, andy.green, netdev, Benjamin Herrenschmidt,
roger.quadros, Jaswinder Singh
In-Reply-To: <4D8A8471.9060904@gmail.com>
On Wed, 2011-03-23 at 16:38 -0700, Steve Calfee wrote:
> On 03/23/11 16:17, Michal Nazarewicz wrote:
> >>>>>>> @@ -97,6 +97,8 @@ struct driver_info {
> >>>>>>>
> >>>>>>> #define FLAG_LINK_INTR 0x0800 /* updates link (carrier) status */
> >>>>>>>
> >>>>>>> +#define FLAG_PTP 0x1000 /* maybe use "usb%d" names */
> >
> >>>>> On Wednesday 23 March 2011 19:46:50 Greg KH wrote:
> >>>>>> "PTP"? What does that stand for?
> >
> >>>> On Mar 23, 2011 8:36 PM, "Arnd Bergmann"<arnd@arndb.de> wrote:
> >>>>> point-to-point, I'll improve the comment to spell it out when
> >>>>> I send the fixed version.
> >
> >>> On Wednesday 23 March 2011 20:53:13 Michał Nazarewicz wrote:
> >>>> I think P2P could be better.
> >
> >> On Wed, 23 Mar 2011 20:57:18 +0100 Arnd Bergmann wrote:
> >>> Yes, good idea.
> >
> > Randy Dunlap<rdunlap@xenotime.net> writes:
> >> that's peer-to-peer.
> >>
> >> OTOH, I knew that PTP was point-to-point.
> >
> > It can be any of that, depending on context. For me PTP is more like
> > Picture Transport Protocol, whereas "2" between two letters is usually
> > "to".
> >
> Well, my 2 cents, picture transport protocol is so obviously different
> than flags for network interfaces it does not cause a mental collision.
PTP is also Precision Time Protocol, which *is* used on network
interfaces (maybe not USB-connected interfaces though).
Anyway, these USB cable interfaces can presumably be bridged to standard
Ethernet, so they are really no more point-to-point than any other
Ethernet link has been since the demise of the hub.
> However P2P is about to become a huge wifi issue, which definitely is in
> the network space. So don't confuse things in the near future, don't use
> P2P.
>
> Maybe a completely different, not so overused flag name would be better.
How about FLAG_NON_IEEE, meaning that the physical layer is not based on
an IEEE 802.3, 802.11 or other standard physical layer.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [RFC] usbnet: use eth%d name for known ethernet devices
From: Steve Calfee @ 2011-03-23 23:38 UTC (permalink / raw)
To: Michal Nazarewicz
Cc: Randy Dunlap, Arnd Bergmann, broonie, lkml, Nicolas Pitre,
Greg KH, David Brownell, Alan Cox, grant.likely, Linux USB list,
andy.green, netdev, Benjamin Herrenschmidt, roger.quadros,
Jaswinder Singh
In-Reply-To: <8762r9l94h.fsf@erwin.mina86.com>
On 03/23/11 16:17, Michal Nazarewicz wrote:
>>>>>>> @@ -97,6 +97,8 @@ struct driver_info {
>>>>>>>
>>>>>>> #define FLAG_LINK_INTR 0x0800 /* updates link (carrier) status */
>>>>>>>
>>>>>>> +#define FLAG_PTP 0x1000 /* maybe use "usb%d" names */
>
>>>>> On Wednesday 23 March 2011 19:46:50 Greg KH wrote:
>>>>>> "PTP"? What does that stand for?
>
>>>> On Mar 23, 2011 8:36 PM, "Arnd Bergmann"<arnd@arndb.de> wrote:
>>>>> point-to-point, I'll improve the comment to spell it out when
>>>>> I send the fixed version.
>
>>> On Wednesday 23 March 2011 20:53:13 Michał Nazarewicz wrote:
>>>> I think P2P could be better.
>
>> On Wed, 23 Mar 2011 20:57:18 +0100 Arnd Bergmann wrote:
>>> Yes, good idea.
>
> Randy Dunlap<rdunlap@xenotime.net> writes:
>> that's peer-to-peer.
>>
>> OTOH, I knew that PTP was point-to-point.
>
> It can be any of that, depending on context. For me PTP is more like
> Picture Transport Protocol, whereas "2" between two letters is usually
> "to".
>
Well, my 2 cents, picture transport protocol is so obviously different
than flags for network interfaces it does not cause a mental collision.
However P2P is about to become a huge wifi issue, which definitely is in
the network space. So don't confuse things in the near future, don't use
P2P.
Maybe a completely different, not so overused flag name would be better.
Regards, Steve
^ permalink raw reply
* Re: [PATCH 02/36] scsi,rcu: convert call_rcu(fc_rport_free_rcu) to kfree_rcu()
From: Matthew Wilcox @ 2011-03-23 23:19 UTC (permalink / raw)
To: James Bottomley
Cc: paulmck, Robert Love, Lai Jiangshan, Ingo Molnar, Jens Axboe,
Neil Horman, David S. Miller, Alexey Kuznetsov,
Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, Eric Dumazet, Stephen Hemminger, Tejun Heo,
Jarek Poplawski, linux-kernel@vger.kernel.org,
devel@open-fcoe.org, linux-scsi@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <1300889151.15899.4.camel@mulgrave.site>
On Wed, Mar 23, 2011 at 09:05:51AM -0500, James Bottomley wrote:
> > #define kfree_rcu(ptr, rcu_head) \
> > __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
>
> Isn't this one of those cases where the obvious use of the interface is
> definitely wrong?
But it's a compile time breakage if you use it wrong, not runtime.
> It's also another nasty pseudo C prototype. I know we do this sort of
> thing for container_of et al, but I don't really think we want to extend
> it.
We do it for list_entry, list_for_each_entry, etc. And those are very
widespread within the kernel.
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
^ permalink raw reply
* Re: [RFC] usbnet: use eth%d name for known ethernet devices
From: Randy Dunlap @ 2011-03-23 23:19 UTC (permalink / raw)
To: Michal Nazarewicz
Cc: Arnd Bergmann, broonie, lkml, Nicolas Pitre, Greg KH,
David Brownell, Alan Cox, grant.likely, Linux USB list,
andy.green, netdev, Benjamin Herrenschmidt, roger.quadros,
Jaswinder Singh
In-Reply-To: <8762r9l94h.fsf@erwin.mina86.com>
On Thu, 24 Mar 2011 00:17:34 +0100 Michal Nazarewicz wrote:
> >>>>>> @@ -97,6 +97,8 @@ struct driver_info {
> >>>>>>
> >>>>>> #define FLAG_LINK_INTR 0x0800 /* updates link (carrier) status */
> >>>>>>
> >>>>>> +#define FLAG_PTP 0x1000 /* maybe use "usb%d" names */
>
> >>>> On Wednesday 23 March 2011 19:46:50 Greg KH wrote:
> >>>>> "PTP"? What does that stand for?
>
> >>> On Mar 23, 2011 8:36 PM, "Arnd Bergmann" <arnd@arndb.de> wrote:
> >>>> point-to-point, I'll improve the comment to spell it out when
> >>>> I send the fixed version.
>
> >> On Wednesday 23 March 2011 20:53:13 Michał Nazarewicz wrote:
> >>> I think P2P could be better.
>
> > On Wed, 23 Mar 2011 20:57:18 +0100 Arnd Bergmann wrote:
> >> Yes, good idea.
>
> Randy Dunlap <rdunlap@xenotime.net> writes:
> > that's peer-to-peer.
> >
> > OTOH, I knew that PTP was point-to-point.
>
> It can be any of that, depending on context. For me PTP is more like
> Picture Transport Protocol, whereas "2" between two letters is usually
> "to".
>
> Just my two cents though, no strong feelings or anything.
Agreed.
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply
* Re: [RFC] usbnet: use eth%d name for known ethernet devices
From: Michal Nazarewicz @ 2011-03-23 23:17 UTC (permalink / raw)
To: Randy Dunlap
Cc: Arnd Bergmann, Michał Nazarewicz, broonie, lkml,
Nicolas Pitre, Greg KH, David Brownell, Alan Cox, grant.likely,
Linux USB list, andy.green, netdev, Benjamin Herrenschmidt,
roger.quadros, Jaswinder Singh
In-Reply-To: <20110323125958.76285c0f.rdunlap@xenotime.net>
[-- Attachment #1: Type: text/plain, Size: 1284 bytes --]
>>>>>> @@ -97,6 +97,8 @@ struct driver_info {
>>>>>>
>>>>>> #define FLAG_LINK_INTR 0x0800 /* updates link (carrier) status */
>>>>>>
>>>>>> +#define FLAG_PTP 0x1000 /* maybe use "usb%d" names */
>>>> On Wednesday 23 March 2011 19:46:50 Greg KH wrote:
>>>>> "PTP"? What does that stand for?
>>> On Mar 23, 2011 8:36 PM, "Arnd Bergmann" <arnd@arndb.de> wrote:
>>>> point-to-point, I'll improve the comment to spell it out when
>>>> I send the fixed version.
>> On Wednesday 23 March 2011 20:53:13 Michał Nazarewicz wrote:
>>> I think P2P could be better.
> On Wed, 23 Mar 2011 20:57:18 +0100 Arnd Bergmann wrote:
>> Yes, good idea.
Randy Dunlap <rdunlap@xenotime.net> writes:
> that's peer-to-peer.
>
> OTOH, I knew that PTP was point-to-point.
It can be any of that, depending on context. For me PTP is more like
Picture Transport Protocol, whereas "2" between two letters is usually
"to".
Just my two cents though, no strong feelings or anything.
--
Best regards, _ _
.o. | Liege of Serenly Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michal "mina86" Nazarewicz (o o)
ooo +-<mina86-mina86.com>-<jid:mina86-jabber.org>--ooO--(_)--Ooo--
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: pull request: sfc-2.6 2011-03-23
From: David Miller @ 2011-03-23 22:56 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1300914873.2638.19.camel@bwh-desktop>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 23 Mar 2011 21:14:33 +0000
> The following changes since commit 736561a01f11114146b1b7f82d486fa9c95828ef:
>
> IPVS: Use global mutex in ip_vs_app.c (2011-03-21 20:39:24 -0700)
>
> are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-2.6.git sfc-2.6.39
>
> Bug fix for the latest optimisation.
Pulled, thanks a lot Ben.
^ permalink raw reply
* Re: [PATCH 02/36] scsi,rcu: convert call_rcu(fc_rport_free_rcu) to kfree_rcu()
From: James Bottomley @ 2011-03-23 22:45 UTC (permalink / raw)
To: paulmck
Cc: Robert Love, Lai Jiangshan, Ingo Molnar, Jens Axboe, Neil Horman,
David S. Miller, Alexey Kuznetsov, Pekka Savola (ipv6),
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, Eric Dumazet,
Stephen Hemminger, Tejun Heo, Jarek Poplawski,
linux-kernel@vger.kernel.org, devel@open-fcoe.org,
linux-scsi@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20110323222456.GC2322@linux.vnet.ibm.com>
On Wed, 2011-03-23 at 15:24 -0700, Paul E. McKenney wrote:
> On Wed, Mar 23, 2011 at 09:05:51AM -0500, James Bottomley wrote:
> > On Tue, 2011-03-22 at 23:50 -0700, Paul E. McKenney wrote:
> > > The kfree_rcu() definition is as
> > > follows:
> > >
> > > #define kfree_rcu(ptr, rcu_head) \
> > > __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
> >
> > Isn't this one of those cases where the obvious use of the interface is
> > definitely wrong?
> >
> > It's also another nasty pseudo C prototype. I know we do this sort of
> > thing for container_of et al, but I don't really think we want to extend
> > it.
> >
> > Why not make the interface take a pointer to the embedding structure and
> > one to the rcu_head ... that way all pointer mathematics can be
> > contained inside the RCU routines.
>
> Hello, James,
>
> If you pass in a pair of pointers, then it is difficult for RCU to detect
> bugs where the two pointers are unrelated. Yes, you can do some sanity
> checks, but these get cumbersome and have corner cases where they can
> be fooled. In contrast, Lai's interface allows the compiler to do the
> needed type checking -- unless the second argument is a field of type
> struct rcu_head in the structure pointed to by the first argument, the
> compiler will complain.
>
> Either way, the pointer mathematics are buried in the RCU API.
>
> Or am I missing something here?
No ... I like the utility ... I just dislike the inelegance of having to
name a structure element in what looks like a C prototype.
I can see this proliferating everywhere since most of our reference
counting release callbacks basically free the enclosing object ...
James
^ permalink raw reply
* [RFC PATCH] et1011c: Replaced PHY driver by a small dm646x board fixup
From: Kyle Moffett @ 2011-03-23 22:33 UTC (permalink / raw)
To: netdev
Cc: Kyle Moffett, Kevin Hilman, Russell King, David S. Miller,
Uwe Kleine-König, David J. Choi, Giuseppe Cavallaro,
Randy Dunlap, linux-arm-kernel, linux-kernel
In-Reply-To: <091F7EC1-5D33-4C90-AFC1-D6E24EF3EEC6@boeing.com>
The et1011c PHY driver has several noticeable code smells:
(1) It uses a "static int speed" variable to see if the speed changed
between calls to et1011c_read_status(). This obviously breaks if
more than one PHY is present in a system.
(2) The "GMII_INTERFACE" and "SYS_CLK_EN" bits should be properly set
at reset-time by hardwired pins on the ET1011C chip, as they are
specific to the wiring on the board. They should NOT be set by a
generic PHY driver, and at best belong in a board-specific fixup.
(3) The FIFO bits are "changed" to the default reset value specified
in the datasheet.
(4) The driver does not appear to contain code anywhere which undoes
any of the above changes if the interface drops from 1000BaseT to
100BaseTX without a chip reset. Instead it appears to perform an
extraneous BMCR_RESET in its ->config_aneg() method, which would
wipe out any settings applied by phy_register_fixup() and friends.
This PHY should be handled entirely by the genphy driver with only a
minimal board-specific "phy_register_fixup()" in the DM646x code.
Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
---
Completely untested. Needs somebody with hardware to try it out.
My attempts to get ahold of "Chaithrika U S <chaithrika@ti.com>" (the
author of this driver) have failed, I assume he's no longer at TI?
arch/arm/mach-davinci/dm646x.c | 23 ++++++++
drivers/net/phy/Kconfig | 5 --
drivers/net/phy/Makefile | 1 -
drivers/net/phy/et1011c.c | 119 ----------------------------------------
4 files changed, 23 insertions(+), 125 deletions(-)
delete mode 100644 drivers/net/phy/et1011c.c
diff --git a/arch/arm/mach-davinci/dm646x.c b/arch/arm/mach-davinci/dm646x.c
index 1e0f809..e32e384 100644
--- a/arch/arm/mach-davinci/dm646x.c
+++ b/arch/arm/mach-davinci/dm646x.c
@@ -905,6 +905,27 @@ void __init dm646x_init(void)
davinci_common_init(&davinci_soc_info_dm646x);
}
+/* Apparently the PHY bootstrap pin wiring on the board is wrong */
+#define ET1011C_CONFIG_REG (0x16)
+#define ET1011C_TX_FIFO_MASK (0x3000)
+#define ET1011C_TX_FIFO_DEPTH_16 (0x1000)
+#define ET1011C_SYS_CLK_EN (0x0010)
+#define ET1011C_INTERFACE_MASK (0x0007)
+#define ET1011C_INTERFACE_GMII_GTX_CLK (0x0002)
+static int dm646x_et1011c_phy_fixup(struct phy_device *phydev)
+{
+ int val = phy_read(phydev, ET1011C_CONFIG_REG);
+ if (val < 0)
+ return val;
+
+ val &= ~ET1011C_TX_FIFO_MASK;
+ val |= ET1011C_TX_FIFO_DEPTH_16;
+ val |= ET1011C_SYS_CLK_EN;
+ val &= ~ET1011C_INTERFACE_MASK
+ val |= ET1011C_INTERFACE_GMII_GTX_CLK;
+ return phy_write(phydev, ET1011C_CONFIG_REG, val);
+}
+
static int __init dm646x_init_devices(void)
{
if (!cpu_is_davinci_dm646x())
@@ -914,6 +935,8 @@ static int __init dm646x_init_devices(void)
platform_device_register(&dm646x_emac_device);
clk_add_alias(NULL, dev_name(&dm646x_mdio_device.dev),
NULL, &dm646x_emac_device.dev);
+ phy_register_fixup_for_uid(0x0282f014, 0xfffffff0,
+ &dm646x_et1011c_phy_fixup);
return 0;
}
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 35fda5a..758cd94 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -82,11 +82,6 @@ config STE10XP
---help---
This is the driver for the STe100p and STe101p PHYs.
-config LSI_ET1011C_PHY
- tristate "Driver for LSI ET1011C PHY"
- ---help---
- Supports the LSI ET1011C PHY.
-
config MICREL_PHY
tristate "Driver for Micrel PHYs"
---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 13bebab..4ee2cb3 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -14,7 +14,6 @@ obj-$(CONFIG_BROADCOM_PHY) += broadcom.o
obj-$(CONFIG_BCM63XX_PHY) += bcm63xx.o
obj-$(CONFIG_ICPLUS_PHY) += icplus.o
obj-$(CONFIG_REALTEK_PHY) += realtek.o
-obj-$(CONFIG_LSI_ET1011C_PHY) += et1011c.o
obj-$(CONFIG_FIXED_PHY) += fixed.o
obj-$(CONFIG_MDIO_BITBANG) += mdio-bitbang.o
obj-$(CONFIG_MDIO_GPIO) += mdio-gpio.o
diff --git a/drivers/net/phy/et1011c.c b/drivers/net/phy/et1011c.c
deleted file mode 100644
index a8eb19e..0000000
--- a/drivers/net/phy/et1011c.c
+++ /dev/null
@@ -1,119 +0,0 @@
-/*
- * drivers/net/phy/et1011c.c
- *
- * Driver for LSI ET1011C PHYs
- *
- * Author: Chaithrika U S
- *
- * Copyright (c) 2008 Texas Instruments
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License as published by the
- * Free Software Foundation; either version 2 of the License, or (at your
- * option) any later version.
- *
- */
-#include <linux/kernel.h>
-#include <linux/string.h>
-#include <linux/errno.h>
-#include <linux/unistd.h>
-#include <linux/interrupt.h>
-#include <linux/init.h>
-#include <linux/delay.h>
-#include <linux/netdevice.h>
-#include <linux/etherdevice.h>
-#include <linux/skbuff.h>
-#include <linux/spinlock.h>
-#include <linux/mm.h>
-#include <linux/module.h>
-#include <linux/mii.h>
-#include <linux/ethtool.h>
-#include <linux/phy.h>
-#include <linux/io.h>
-#include <linux/uaccess.h>
-#include <asm/irq.h>
-
-#define ET1011C_STATUS_REG (0x1A)
-#define ET1011C_CONFIG_REG (0x16)
-#define ET1011C_SPEED_MASK (0x0300)
-#define ET1011C_GIGABIT_SPEED (0x0200)
-#define ET1011C_TX_FIFO_MASK (0x3000)
-#define ET1011C_TX_FIFO_DEPTH_8 (0x0000)
-#define ET1011C_TX_FIFO_DEPTH_16 (0x1000)
-#define ET1011C_INTERFACE_MASK (0x0007)
-#define ET1011C_GMII_INTERFACE (0x0002)
-#define ET1011C_SYS_CLK_EN (0x01 << 4)
-
-
-MODULE_DESCRIPTION("LSI ET1011C PHY driver");
-MODULE_AUTHOR("Chaithrika U S");
-MODULE_LICENSE("GPL");
-
-static int et1011c_config_aneg(struct phy_device *phydev)
-{
- int ctl = 0;
- ctl = phy_read(phydev, MII_BMCR);
- if (ctl < 0)
- return ctl;
- ctl &= ~(BMCR_FULLDPLX | BMCR_SPEED100 | BMCR_SPEED1000 |
- BMCR_ANENABLE);
- /* First clear the PHY */
- phy_write(phydev, MII_BMCR, ctl | BMCR_RESET);
-
- return genphy_config_aneg(phydev);
-}
-
-static int et1011c_read_status(struct phy_device *phydev)
-{
- int ret;
- u32 val;
- static int speed;
- ret = genphy_read_status(phydev);
-
- if (speed != phydev->speed) {
- speed = phydev->speed;
- val = phy_read(phydev, ET1011C_STATUS_REG);
- if ((val & ET1011C_SPEED_MASK) ==
- ET1011C_GIGABIT_SPEED) {
- val = phy_read(phydev, ET1011C_CONFIG_REG);
- val &= ~ET1011C_TX_FIFO_MASK;
- phy_write(phydev, ET1011C_CONFIG_REG, val\
- | ET1011C_GMII_INTERFACE\
- | ET1011C_SYS_CLK_EN\
- | ET1011C_TX_FIFO_DEPTH_16);
-
- }
- }
- return ret;
-}
-
-static struct phy_driver et1011c_driver = {
- .phy_id = 0x0282f014,
- .name = "ET1011C",
- .phy_id_mask = 0xfffffff0,
- .features = (PHY_BASIC_FEATURES | SUPPORTED_1000baseT_Full),
- .flags = PHY_POLL,
- .config_aneg = et1011c_config_aneg,
- .read_status = et1011c_read_status,
- .driver = { .owner = THIS_MODULE,},
-};
-
-static int __init et1011c_init(void)
-{
- return phy_driver_register(&et1011c_driver);
-}
-
-static void __exit et1011c_exit(void)
-{
- phy_driver_unregister(&et1011c_driver);
-}
-
-module_init(et1011c_init);
-module_exit(et1011c_exit);
-
-static struct mdio_device_id __maybe_unused et1011c_tbl[] = {
- { 0x0282f014, 0xfffffff0 },
- { }
-};
-
-MODULE_DEVICE_TABLE(mdio, et1011c_tbl);
--
1.7.2.5
^ permalink raw reply related
* Re: [PATCH 02/36] scsi,rcu: convert call_rcu(fc_rport_free_rcu) to kfree_rcu()
From: Paul E. McKenney @ 2011-03-23 22:24 UTC (permalink / raw)
To: James Bottomley
Cc: Robert Love, Lai Jiangshan, Ingo Molnar, Jens Axboe, Neil Horman,
David S. Miller, Alexey Kuznetsov, Pekka Savola (ipv6),
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, Eric Dumazet,
Stephen Hemminger, Tejun Heo, Jarek Poplawski,
linux-kernel@vger.kernel.org, devel@open-fcoe.org,
linux-scsi@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <1300889151.15899.4.camel@mulgrave.site>
On Wed, Mar 23, 2011 at 09:05:51AM -0500, James Bottomley wrote:
> On Tue, 2011-03-22 at 23:50 -0700, Paul E. McKenney wrote:
> > The kfree_rcu() definition is as
> > follows:
> >
> > #define kfree_rcu(ptr, rcu_head) \
> > __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
>
> Isn't this one of those cases where the obvious use of the interface is
> definitely wrong?
>
> It's also another nasty pseudo C prototype. I know we do this sort of
> thing for container_of et al, but I don't really think we want to extend
> it.
>
> Why not make the interface take a pointer to the embedding structure and
> one to the rcu_head ... that way all pointer mathematics can be
> contained inside the RCU routines.
Hello, James,
If you pass in a pair of pointers, then it is difficult for RCU to detect
bugs where the two pointers are unrelated. Yes, you can do some sanity
checks, but these get cumbersome and have corner cases where they can
be fooled. In contrast, Lai's interface allows the compiler to do the
needed type checking -- unless the second argument is a field of type
struct rcu_head in the structure pointed to by the first argument, the
compiler will complain.
Either way, the pointer mathematics are buried in the RCU API.
Or am I missing something here?
Thanx, Paul
^ permalink raw reply
* Re: [PATCH v2] net/unix: Add secdata to unix_stream msgs
From: Paul Moore @ 2011-03-23 22:16 UTC (permalink / raw)
To: Pat Kane
Cc: Casey Schaufler, Eric Paris, David Miller, linux-kernel, netdev,
cxzhang, sds, jmorris, eparis, LSM
In-Reply-To: <AANLkTik55CAKfU5v4PE87h26vwAMy8vV7rhMmu9EA19B@mail.gmail.com>
On Wednesday, March 23, 2011 4:23:35 PM Pat Kane wrote:
> >> On Tue, 2011-03-22 at 19:32 -0700, David Miller wrote:
> >>> The SELINUX hook implementations even have "_dgram()" in their names.
>
> The LSM hook that I am having problems with, and that the patch fixes
> is "secid_to_secctx()" not "socket_getpeersec_dgram()".
Can you explain the problem you are having? I'm specifically interested in
who is calling secid_to_secctx() on a AF_UNIX stream packet.
Adding the secid token to a AF_UNIX stream packet isn't likely to be the right
solution, although until we hear what the core problem is, I'm not sure we can
offer any fixes.
--
paul moore
linux @ hp
^ permalink raw reply
* Re: [PATCH V12 4/4] ptp: Added a clock driver for the National Semiconductor PHYTER.
From: John Stultz @ 2011-03-23 21:46 UTC (permalink / raw)
To: Richard Cochran
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, Alan Cox, Arnd Bergmann,
Christoph Lameter, David Miller, Krzysztof Halasa, Peter Zijlstra,
Rodolfo Giometti, Thomas Gleixner, Benjamin Herrenschmidt,
Mike Frysinger, Paul Mackerras, Russell King
In-Reply-To: <f8af215ebe32a8c0f4b3a82b8ee7e8008b4feb1e.1298878618.git.richard.cochran-3mrvs1K0uXizZXS1Dc/lvw@public.gmane.org>
On Mon, 2011-02-28 at 08:58 +0100, Richard Cochran wrote:
> This patch adds support for the PTP clock found on the DP83640.
> The basic clock operations and one external time stamp have
> been implemented.
Just locking rule comment nits here.
> +static int tdr_write(int bc, struct phy_device *dev,
> + const struct timespec *ts, u16 cmd)
> +{
> + ext_write(bc, dev, PAGE4, PTP_TDR, ts->tv_nsec & 0xffff);/* ns[15:0] */
> + ext_write(bc, dev, PAGE4, PTP_TDR, ts->tv_nsec >> 16); /* ns[31:16] */
> + ext_write(bc, dev, PAGE4, PTP_TDR, ts->tv_sec & 0xffff); /* sec[15:0] */
> + ext_write(bc, dev, PAGE4, PTP_TDR, ts->tv_sec >> 16); /* sec[31:16]*/
> +
> + ext_write(bc, dev, PAGE4, PTP_CTL, cmd);
> +
> + return 0;
> +}
The above needs to hold the extreg_lock, and should be commented as
such.
And again, the function names are sort of generic, and could use a
dp83640_ prefix or something.
thanks
-john
^ permalink raw reply
* Understanding the et1011c PHY driver
From: Moffett, Kyle D @ 2011-03-23 21:14 UTC (permalink / raw)
To: netdev@vger.kernel.org; +Cc: Chaithrika U S
Hello,
I'm currently working on some PHY layer refactoring I'd like to propose to
support some of our custom hardware, and I'm trying to understand what the
heck the ET1011C PHY driver is doing so I can fix it up to match.
Specifically, the et1011c_read_status() code does this:
int ret;
u32 val;
static int speed;
ret = genphy_read_status(phydev);
if (speed != phydev->speed) {
speed = phydev->speed;
val = phy_read(phydev, ET1011C_STATUS_REG);
if ((val & ET1011C_SPEED_MASK) == ET1011C_GIGABIT_SPEED) {
val = phy_read(phydev, ET1011C_CONFIG_REG);
val &= ~ET1011C_TX_FIFO_MASK;
phy_write(phydev, ET1011C_CONFIG_REG, val\
| ET1011C_GMII_INTERFACE\
| ET1011C_SYS_CLK_EN\
| ET1011C_TX_FIFO_DEPTH_16);
}
}
return ret;
There are so many things wrong with this code that it's not even funny.
There's a static variable promising terrible terrible things should you
have more than 1 such PHY in a system. The "GMII_INTERFACE" and
"SYS_CLK_EN" bits should only ever be modified as a board-specific PHY
fixup, because the PHY driver has no hope of getting it right in general.
The FIFO depth bits are "changed" to the default value after a reset,
and they are not touched anywhere else in the code.
On top of that, I don't see anywhere that the code would ever undo any
of these changes if the interface dropped from 1000BaseT down to 100BaseT
without a reset. Looking at the datasheet, it does not even appear that
any of those changes *should* be different between gigabit/non-gigabit.
Then the et1011c_config_aneg() function is customized to do an extra PHY
reset before calling genphy_config_aneg(), which I suspect is only
necessary because of the other abominations above.
As far as I can tell, this PHY *SHOULD* be handled entirely by the genphy
driver with just a small board-specific phy_register_fixup() that gets
run after every reset of the PHY.
I'm looking forward to hearing your opinions on this driver.
Cheers,
Kyle Moffett
^ permalink raw reply
* Re: [PATCH V12 3/4] ptp: Added a clock driver for the IXP46x.
From: John Stultz @ 2011-03-23 21:43 UTC (permalink / raw)
To: Richard Cochran
Cc: linux-kernel, linux-api, netdev, devicetree-discuss,
linux-arm-kernel, linuxppc-dev, Alan Cox, Arnd Bergmann,
Christoph Lameter, David Miller, Krzysztof Halasa, Peter Zijlstra,
Rodolfo Giometti, Thomas Gleixner, Benjamin Herrenschmidt,
Mike Frysinger, Paul Mackerras, Russell King
In-Reply-To: <cd6ddff0dcca60ac88b35660505d7bf54ad32405.1298878618.git.richard.cochran@omicron.at>
On Mon, 2011-02-28 at 08:58 +0100, Richard Cochran wrote:
> This patch adds a driver for the hardware time stamping unit found on the
> IXP465. The basic clock operations and an external trigger are implemented.
>
> Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
> Acked-by: John Stultz <johnstul@us.ibm.com>
My ack is still fine, but found a few more nits to consider on another
pass...
> +static int match(struct sk_buff *skb, u16 uid_hi, u32 uid_lo, u16 seq)
[snip]
> +static void do_rx_timestamp(struct port *port, struct sk_buff *skb)
[snip]
> +static u64 sys_time_read(struct ixp46x_ts_regs *regs)
[snip]
> +static void sys_time_write(struct ixp46x_ts_regs *regs, u64 ns)
I know these are static, but these function names are sort of generic
names, and make grep/cscoping for similar sounding functions a little
noisy.
Maybe add a ipx_ prefix just to be more obvious?
thanks
-john
^ permalink raw reply
* Re: Understanding the et1011c PHY driver
From: Moffett, Kyle D @ 2011-03-23 21:16 UTC (permalink / raw)
To: netdev@vger.kernel.org
In-Reply-To: <091F7EC1-5D33-4C90-AFC1-D6E24EF3EEC6@boeing.com>
On Mar 23, 2011, at 17:14, Moffett, Kyle D wrote:
> I'm currently working on some PHY layer refactoring I'd like to propose to
> support some of our custom hardware, and I'm trying to understand what the
> heck the ET1011C PHY driver is doing so I can fix it up to match.
[...snip...]
Apparently the original author's email address is no longer valid. Anyone
have an updated address for "Chaithrika U S <chaithrika@ti.com>"?
Cheers,
Kyle Moffett
^ permalink raw reply
* Re: [PATCH V12 2/4] ptp: Added a clock that uses the eTSEC found on the MPC85xx.
From: John Stultz @ 2011-03-23 21:30 UTC (permalink / raw)
To: Richard Cochran
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, Alan Cox, Arnd Bergmann,
Christoph Lameter, David Miller, Krzysztof Halasa, Peter Zijlstra,
Rodolfo Giometti, Thomas Gleixner, Benjamin Herrenschmidt,
Mike Frysinger, Paul Mackerras, Russell King
In-Reply-To: <aa97e94c82ebd8953d83b53f64cbfef66c81b7be.1298878618.git.richard.cochran-3mrvs1K0uXizZXS1Dc/lvw@public.gmane.org>
On Mon, 2011-02-28 at 08:57 +0100, Richard Cochran wrote:
> The eTSEC includes a PTP clock with quite a few features. This patch adds
> support for the basic clock adjustment functions, plus two external time
> stamps, one alarm, and the PPS callback.
Just a minor question on the locking, but otherwise looks ok.
> --- /dev/null
> +++ b/drivers/net/gianfar_ptp.c
> @@ -0,0 +1,579 @@
[snip]
> +/*
> + * Register access functions
> + */
So what are the locking rules on the functions below? I assume the
etsects->lock needs to be held prior to calling, so that should be made
explicit in a comment.
> +static u64 tmr_cnt_read(struct etsects *etsects)
> +{
> + u64 ns;
> + u32 lo, hi;
> +
> + lo = gfar_read(&etsects->regs->tmr_cnt_l);
> + hi = gfar_read(&etsects->regs->tmr_cnt_h);
> + ns = ((u64) hi) << 32;
> + ns |= lo;
> + return ns;
> +}
> +
> +static void tmr_cnt_write(struct etsects *etsects, u64 ns)
> +{
> + u32 hi = ns >> 32;
> + u32 lo = ns & 0xffffffff;
> +
> + gfar_write(&etsects->regs->tmr_cnt_l, lo);
> + gfar_write(&etsects->regs->tmr_cnt_h, hi);
> +}
> +
> +static void set_alarm(struct etsects *etsects)
> +{
> + u64 ns;
> + u32 lo, hi;
> +
> + ns = tmr_cnt_read(etsects) + 1500000000ULL;
> + ns = div_u64(ns, 1000000000UL) * 1000000000ULL;
> + ns -= etsects->tclk_period;
> + hi = ns >> 32;
> + lo = ns & 0xffffffff;
> + gfar_write(&etsects->regs->tmr_alarm1_l, lo);
> + gfar_write(&etsects->regs->tmr_alarm1_h, hi);
> +}
> +
> +static void set_fipers(struct etsects *etsects)
> +{
> + u32 tmr_ctrl = gfar_read(&etsects->regs->tmr_ctrl);
> +
> + gfar_write(&etsects->regs->tmr_ctrl, tmr_ctrl & (~TE));
> + gfar_write(&etsects->regs->tmr_prsc, etsects->tmr_prsc);
> + gfar_write(&etsects->regs->tmr_fiper1, etsects->tmr_fiper1);
> + gfar_write(&etsects->regs->tmr_fiper2, etsects->tmr_fiper2);
> + set_alarm(etsects);
> + gfar_write(&etsects->regs->tmr_ctrl, tmr_ctrl|TE);
> +}
> +
[snip]
> +static int gianfar_ptp_probe(struct platform_device *dev)
> +{
> + struct device_node *node = dev->dev.of_node;
> + struct etsects *etsects;
> + struct timespec now;
> + int err = -ENOMEM;
> + u32 tmr_ctrl;
> +
> + etsects = kzalloc(sizeof(*etsects), GFP_KERNEL);
> + if (!etsects)
> + goto no_memory;
> +
> + err = -ENODEV;
> +
> + etsects->caps = ptp_gianfar_caps;
> + etsects->cksel = DEFAULT_CKSEL;
> +
> + if (get_of_u32(node, "fsl,tclk-period", &etsects->tclk_period) ||
> + get_of_u32(node, "fsl,tmr-prsc", &etsects->tmr_prsc) ||
> + get_of_u32(node, "fsl,tmr-add", &etsects->tmr_add) ||
> + get_of_u32(node, "fsl,tmr-fiper1", &etsects->tmr_fiper1) ||
> + get_of_u32(node, "fsl,tmr-fiper2", &etsects->tmr_fiper2) ||
> + get_of_u32(node, "fsl,max-adj", &etsects->caps.max_adj)) {
> + pr_err("device tree node missing required elements\n");
> + goto no_node;
> + }
> +
> + etsects->irq = platform_get_irq(dev, 0);
> +
> + if (etsects->irq == NO_IRQ) {
> + pr_err("irq not in device tree\n");
> + goto no_node;
> + }
> + if (request_irq(etsects->irq, isr, 0, DRIVER, etsects)) {
> + pr_err("request_irq failed\n");
> + goto no_node;
> + }
> +
> + etsects->rsrc = platform_get_resource(dev, IORESOURCE_MEM, 0);
> + if (!etsects->rsrc) {
> + pr_err("no resource\n");
> + goto no_resource;
> + }
> + if (request_resource(&ioport_resource, etsects->rsrc)) {
> + pr_err("resource busy\n");
> + goto no_resource;
> + }
> +
> + spin_lock_init(&etsects->lock);
> +
> + etsects->regs = ioremap(etsects->rsrc->start,
> + 1 + etsects->rsrc->end - etsects->rsrc->start);
> + if (!etsects->regs) {
> + pr_err("ioremap ptp registers failed\n");
> + goto no_ioremap;
> + }
> + getnstimeofday(&now);
> + ptp_gianfar_settime(&etsects->caps, &now);
> +
> + tmr_ctrl =
> + (etsects->tclk_period & TCLK_PERIOD_MASK) << TCLK_PERIOD_SHIFT |
> + (etsects->cksel & CKSEL_MASK) << CKSEL_SHIFT;
> +
> + gfar_write(&etsects->regs->tmr_ctrl, tmr_ctrl);
> + gfar_write(&etsects->regs->tmr_add, etsects->tmr_add);
> + gfar_write(&etsects->regs->tmr_prsc, etsects->tmr_prsc);
> + gfar_write(&etsects->regs->tmr_fiper1, etsects->tmr_fiper1);
> + gfar_write(&etsects->regs->tmr_fiper2, etsects->tmr_fiper2);
> + set_alarm(etsects);
> + gfar_write(&etsects->regs->tmr_ctrl, tmr_ctrl|FS|RTPE|TE);
Does any of the above need a lock should an irq land in the middle of
the writes?
thanks
-john
^ permalink raw reply
* Re: [PATCH V12 1/4] ptp: Added a brand new class driver for ptp clocks.
From: John Stultz @ 2011-03-23 21:19 UTC (permalink / raw)
To: Richard Cochran
Cc: linux-kernel, linux-api, netdev, devicetree-discuss,
linux-arm-kernel, linuxppc-dev, Alan Cox, Arnd Bergmann,
Christoph Lameter, David Miller, Krzysztof Halasa, Peter Zijlstra,
Rodolfo Giometti, Thomas Gleixner, Benjamin Herrenschmidt,
Mike Frysinger, Paul Mackerras, Russell King
In-Reply-To: <bf964489f4c6ee51bd8a2ab818cd6a5e2f544627.1298878618.git.richard.cochran@omicron.at>
On Mon, 2011-02-28 at 08:57 +0100, Richard Cochran wrote:
> This patch adds an infrastructure for hardware clocks that implement
> IEEE 1588, the Precision Time Protocol (PTP). A class driver offers a
> registration method to particular hardware clock drivers. Each clock is
> presented as a standard POSIX clock.
>
> The ancillary clock features are exposed in two different ways, via
> the sysfs and by a character device.
>
> Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
I think this looks mostly ok. I've got one concern on the locking and
there are a few minor nits that might need extra commenting below.
thanks
-john
> --- /dev/null
> +++ b/Documentation/ptp/testptp.c
[snip]
> + if (0x7fffffff != adjfreq) {
> + memset(&tx, 0, sizeof(tx));
> + tx.modes = ADJ_FREQUENCY;
> + tx.freq = (long) (adjfreq * 65.536);
So, 65.536... I understand you're converting ppb to ppm << 16 (which is
what adjtime takes), but I'm not sure if anyone else will. I'd either
wrap that in a commented conversion function/macro or just do the
conversion explicitly (freq = (adjfreq << 16)/1000; although this has
potential overflow risks with large ppb values) with a comment about
what is going on.
> --- /dev/null
> +++ b/drivers/ptp/ptp_clock.c
> @@ -0,0 +1,320 @@
[snip]
> +static void enqueue_external_timestamp(struct timestamp_event_queue *queue,
> + struct ptp_clock_event *src)
> +{
> + struct ptp_extts_event *dst;
> + unsigned long flags;
> + u32 remainder;
> +
> + dst = &queue->buf[queue->tail];
Doesn't the lock need to happen before you access the
queue->buf[queue->tail] ?
For example: What happens if two cpus enter the function at the same
time, both get the same tail index, one overwrite the other's data, then
both take turns bumping up the tail pointer?
> + dst->index = src->index;
> + dst->t.sec = div_u64_rem(src->timestamp, 1000000000, &remainder);
> + dst->t.nsec = remainder;
> +
> + spin_lock_irqsave(&queue->lock, flags);
> +
> + if (!queue_free(queue))
> + queue->head = (queue->head + 1) % PTP_MAX_TIMESTAMPS;
> +
> + queue->tail = (queue->tail + 1) % PTP_MAX_TIMESTAMPS;
> +
> + spin_unlock_irqrestore(&queue->lock, flags);
> +}
[snip]
> +static int ptp_clock_adjtime(struct posix_clock *pc, struct timex *tx)
> +{
> + struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
> + struct ptp_clock_info *ops;
> + int err = -EOPNOTSUPP;
> +
> + ops = ptp->info;
> +
> + if (tx->modes & ADJ_SETOFFSET) {
> + struct timespec ts;
> + ktime_t kt;
> + s64 delta;
> +
> + ts.tv_sec = tx->time.tv_sec;
> + ts.tv_nsec = tx->time.tv_usec;
> +
> + if (!(tx->modes & ADJ_NANO))
> + ts.tv_nsec *= 1000;
> +
> + if ((unsigned long) ts.tv_nsec >= NSEC_PER_SEC)
> + return -EINVAL;
> +
> + kt = timespec_to_ktime(ts);
> + delta = ktime_to_ns(kt);
> + err = ops->adjtime(ops, delta);
> +
> + } else if (tx->modes & ADJ_FREQUENCY) {
> + s64 ppb = 1 + tx->freq;
> + ppb *= 125;
> + ppb >>= 13;
((1 + freq) *125)>>13 == magic :)
Needs a clear comment.
> + err = ops->adjfreq(ops, (s32)ppb);
> + }
> +
> + return err;
> +}
[snip]
> diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
> new file mode 100644
> index 0000000..2f76266
> --- /dev/null
> +++ b/drivers/ptp/ptp_private.h
> @@ -0,0 +1,86 @@
> +/*
> + * PTP 1588 clock support - private declarations for the core module.
> + *
> + * Copyright (C) 2010 OMICRON electronics GmbH
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +#ifndef _PTP_PRIVATE_H_
> +#define _PTP_PRIVATE_H_
> +
> +#include <linux/cdev.h>
> +#include <linux/device.h>
> +#include <linux/mutex.h>
> +#include <linux/posix-clock.h>
> +#include <linux/ptp_clock.h>
> +#include <linux/ptp_clock_kernel.h>
> +#include <linux/time.h>
> +
> +#define PTP_MAX_TIMESTAMPS 128
> +#define PTP_BUF_TIMESTAMPS 30
> +
> +struct timestamp_event_queue {
> + struct ptp_extts_event buf[PTP_MAX_TIMESTAMPS];
> + int head;
> + int tail;
> + spinlock_t lock;
> +};
> +
> +struct ptp_clock {
> + struct posix_clock clock;
> + struct device *dev;
> + struct ptp_clock_info *info;
> + dev_t devid;
> + int index; /* index into clocks.map */
> + struct pps_device *pps_source;
> + struct timestamp_event_queue tsevq; /* simple fifo for time stamps */
> + struct mutex tsevq_mux; /* one process at a time reading the fifo */
> + wait_queue_head_t tsev_wq;
> +};
> +
> +static inline int queue_cnt(struct timestamp_event_queue *q)
> +{
> + int cnt = q->tail - q->head;
> + return cnt < 0 ? PTP_MAX_TIMESTAMPS + cnt : cnt;
> +}
q->tail and head access probably need to happen only when locked.
So probably need a comment that queue_cnt must be called only when
holding the proper lock.
thanks
-john
^ permalink raw reply
* [PATCH net-2.6] sfc: Siena: Disable write-combining when SR-IOV is enabled
From: Ben Hutchings @ 2011-03-23 21:16 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1300914873.2638.19.camel@bwh-desktop>
From: Steve Hodgson <shodgson@solarflare.com>
If SR-IOV is enabled by firmware, even if it is not enabled in the PCI
capability, TX pushes using write-combining may be corrupted.
We want to know whether it is enabled before mapping the NIC
registers, and even if PCI extended capabilities are not accessible.
Therefore, we look for the MSI capability, which is removed if SR-IOV
is enabled.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/sfc/efx.c | 18 ++++++++++++++++--
drivers/net/sfc/workarounds.h | 2 ++
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c
index b8bd936..d890679 100644
--- a/drivers/net/sfc/efx.c
+++ b/drivers/net/sfc/efx.c
@@ -1054,6 +1054,7 @@ static int efx_init_io(struct efx_nic *efx)
{
struct pci_dev *pci_dev = efx->pci_dev;
dma_addr_t dma_mask = efx->type->max_dma_mask;
+ bool use_wc;
int rc;
netif_dbg(efx, probe, efx->net_dev, "initialising I/O\n");
@@ -1104,8 +1105,21 @@ static int efx_init_io(struct efx_nic *efx)
rc = -EIO;
goto fail3;
}
- efx->membase = ioremap_wc(efx->membase_phys,
- efx->type->mem_map_size);
+
+ /* bug22643: If SR-IOV is enabled then tx push over a write combined
+ * mapping is unsafe. We need to disable write combining in this case.
+ * MSI is unsupported when SR-IOV is enabled, and the firmware will
+ * have removed the MSI capability. So write combining is safe if
+ * there is an MSI capability.
+ */
+ use_wc = (!EFX_WORKAROUND_22643(efx) ||
+ pci_find_capability(pci_dev, PCI_CAP_ID_MSI));
+ if (use_wc)
+ efx->membase = ioremap_wc(efx->membase_phys,
+ efx->type->mem_map_size);
+ else
+ efx->membase = ioremap_nocache(efx->membase_phys,
+ efx->type->mem_map_size);
if (!efx->membase) {
netif_err(efx, probe, efx->net_dev,
"could not map memory BAR at %llx+%x\n",
diff --git a/drivers/net/sfc/workarounds.h b/drivers/net/sfc/workarounds.h
index e4dd3a7..99ff114 100644
--- a/drivers/net/sfc/workarounds.h
+++ b/drivers/net/sfc/workarounds.h
@@ -38,6 +38,8 @@
#define EFX_WORKAROUND_15783 EFX_WORKAROUND_ALWAYS
/* Legacy interrupt storm when interrupt fifo fills */
#define EFX_WORKAROUND_17213 EFX_WORKAROUND_SIENA
+/* Write combining and sriov=enabled are incompatible */
+#define EFX_WORKAROUND_22643 EFX_WORKAROUND_SIENA
/* Spurious parity errors in TSORT buffers */
#define EFX_WORKAROUND_5129 EFX_WORKAROUND_FALCON_A
--
1.7.4
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* Re: [patch net-2.6] bonding: fix rx_handler locking
From: Jiri Pirko @ 2011-03-23 21:16 UTC (permalink / raw)
To: Nicolas de Pesloüan; +Cc: netdev, davem, andy, fubar
In-Reply-To: <4D8A4931.10903@gmail.com>
Wed, Mar 23, 2011 at 08:25:37PM CET, nicolas.2p.debian@gmail.com wrote:
>Le 22/03/2011 13:38, Jiri Pirko a écrit :
>>This prevents possible race between bond_enslave and bond_handle_frame
>>as reported by Nicolas by moving rx_handler register/unregister.
>>slave->bond is added to hold pointer to master bonding sructure. That
>>way dev->master is no longer used in bond_handler_frame.
>>Also, this removes "BUG: scheduling while atomic" message
>>
>>Reported-by: Nicolas de Pesloüan<nicolas.2p.debian@gmail.com>
>>Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>
>Thanks Jiri, it works.
>
>Tested-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>
>
>Regarding the code review, can you explain the reasons why you
>apparently duplicated the fields related to the slave/master
>relationship?
>
>Do you plan to totally remove dev->master usage in bonding in a follow-up patch?
dev->master could be possibly retired. Not sure yet. It's on my todo
list.
>
> Nicolas.
^ permalink raw reply
* pull request: sfc-2.6 2011-03-23
From: Ben Hutchings @ 2011-03-23 21:14 UTC (permalink / raw)
To: David Miller; +Cc: netdev, sf-linux-drivers
The following changes since commit 736561a01f11114146b1b7f82d486fa9c95828ef:
IPVS: Use global mutex in ip_vs_app.c (2011-03-21 20:39:24 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-2.6.git sfc-2.6.39
Bug fix for the latest optimisation.
Ben.
Steve Hodgson (1):
sfc: Siena: Disable write-combining when SR-IOV is enabled
drivers/net/sfc/efx.c | 18 ++++++++++++++++--
drivers/net/sfc/workarounds.h | 2 ++
2 files changed, 18 insertions(+), 2 deletions(-)
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH 1/3] drivers/net/ariadne.c: Convert release_resource to release_region/release_mem_region
From: David Miller @ 2011-03-23 21:10 UTC (permalink / raw)
To: julia
Cc: kernel-janitors, eric.dumazet, tj, crindy646, geert, netdev,
linux-kernel
In-Reply-To: <1300814123-25314-1-git-send-email-julia@diku.dk>
From: Julia Lawall <julia@diku.dk>
Date: Tue, 22 Mar 2011 18:15:21 +0100
> Request_mem_region should be used with release_mem_region, not
> release_resource.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
...
> Signed-off-by: Julia Lawall <julia@diku.dk>
Applied.
^ permalink raw reply
* Re: [PATCH 3/3] drivers/net/a2065.c: Convert release_resource to release_region/release_mem_region
From: David Miller @ 2011-03-23 21:10 UTC (permalink / raw)
To: julia
Cc: kernel-janitors, eric.dumazet, tj, jpirko, geert, netdev,
linux-kernel
In-Reply-To: <1300814123-25314-3-git-send-email-julia@diku.dk>
From: Julia Lawall <julia@diku.dk>
Date: Tue, 22 Mar 2011 18:15:23 +0100
> Request_mem_region should be used with release_mem_region, not
> release_resource.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
...
> Signed-off-by: Julia Lawall <julia@diku.dk>
Applied.
^ permalink raw reply
* Re: [PATCH] vlan: Fix duplicate delivery of vlan 0 packets to ETH_P_ALL packet sockets
From: Jesse Gross @ 2011-03-23 20:59 UTC (permalink / raw)
To: Eric W. Biederman
Cc: David Miller, netdev, Michał Mirosław, Ben Hutchings,
Eric Dumazet, John Fastabend
In-Reply-To: <m1sjugi2d0.fsf@fess.ebiederm.org>
On Mon, Mar 21, 2011 at 2:35 PM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
>
> For vlan data coming in from nics without vlan hardware accelleration we
> get two copies of vlan packets with vlan id 0 on pf_packet sockets, causing
> userspace to break. This is caused by delivering the same packet to the same
> networking device more than once.
I agree that this is a problem and the code consolidation is very nice
but I'm concerned that there is extra complexity for the rest of the
system to counterbalance what is saved here.
> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> index ce8e3ab..a0849b9 100644
> --- a/net/8021q/vlan_core.c
> +++ b/net/8021q/vlan_core.c
> +void emulate_vlan_hwaccel(struct sk_buff *skb)
> +{
> + struct vlan_hdr *vhdr = (struct vlan_hdr *)skb->data;
> + __be16 proto;
> +
> + if (!pskb_may_pull(skb, VLAN_HLEN))
> + return;
> +
> + __vlan_hwaccel_put_tag(skb, vhdr->h_vlan_TCI);
> + skb_pull_rcsum(skb, VLAN_HLEN);
Doesn't this break things which push the header back on? Bridging
pushes ETH_HLEN before forwarding but here it will be a garbage value
due to the extra vlan header. AF_PACKET pushes the mac header back
on, which in this case includes the original vlan header. However,
since we've also put the tag in skb->vlan_tci, won't it appear to be
double tagged?
More generally, even though we pull the tag off the skb it's pretty
common on the receive path to look backwards into previous headers.
Given that this can happen, I think it's somewhat confusing/fragile to
have packet data which effectively should not be there. It also adds
a third case to any generic vlan handling code: tag in packet (can
still happen, such as on transmit), received on vlan accelerated NIC -
tag in skb but not in packet, receive on non-vlan accelerated NIC -
tag in both skb and packet.
If we actually removed the tag in the emulated case that would avoid
these concerns but would, of course, add extra overhead in some
situations.
^ permalink raw reply
* Re: [PATCH v2] net/unix: Add secdata to unix_stream msgs
From: Pat Kane @ 2011-03-23 20:23 UTC (permalink / raw)
To: Casey Schaufler
Cc: Eric Paris, David Miller, linux-kernel, netdev, cxzhang, sds,
jmorris, eparis, paul.moore, LSM
In-Reply-To: <4D8A1BB4.4030709@schaufler-ca.com>
>> On Tue, 2011-03-22 at 19:32 -0700, David Miller wrote:
>>> The SELINUX hook implementations even have "_dgram()" in their names.
The LSM hook that I am having problems with, and that the patch fixes
is "secid_to_secctx()" not "socket_getpeersec_dgram()".
Pat
---
^ 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