* Re: changing usbnet's API to better deal with cdc-ncm
From: Ming Lei @ 2012-09-07 3:55 UTC (permalink / raw)
To: Oliver Neukum
Cc: Bjørn Mork, alexey.orishko-0IS4wlFg1OjSUeElwK9/Pw,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1676538.CEPj2RtrPe-ugxBuEnWX9yG/4A2pS7c2Q@public.gmane.org>
On Fri, Sep 7, 2012 at 1:56 AM, Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> wrote:
> On Friday 07 September 2012 00:09:13 Ming Lei wrote:
>> On Thu, Sep 6, 2012 at 4:30 PM, Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org> wrote:
>> > Ming Lei <tom.leiming-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>
>> >> Looks the introduced .tx_bundle is not necessary since .tx_fixup is OK.
>> >
>> > The minidriver does not have any information about tx in progress. The
>>
>> Inside .tx_fixup, the low level driver will get the tx progress information.
>
> That information changes after tx_fixup
You mean the low level drive can't see if the later transmission
for the aggregated frame is successful? If so, there is no any complete
notification to all low level drivers (with aggregation capability
or not) now, isn't there?
If no aggregated frame is ready for transmission, the information
can't be changed after current tx_fixup and before the next .tx_fixup.
>
>> > strategy above, which is what cdc_ncm uses today, is fine as long as you
>> > always want to wait as long as you always want to fill as many frames as
>> > possible in each packet. But what if the queue is empty and the device
>>
>> For cdc_ncm, the wait time is controlled by cdc_ncm driver, see
>> cdc_ncm_tx_timeout_start().
>
> Well, that is the mistake. Using a timer is a bad idea.
Why is a bad idea? Without a timer, how can usbnet or
low level driver aggregate the later coming transmit frames to
form a biger aggregated frame?
>
>> If we can abstract some common things about aggregation, it should be
>> meaningful. As far as I know, most of aggregation protocol is very different,
>> so almost all aggregation work is only done by low level driver, such as
>> cdc_ncm.
>>
>> If we want to implement some aggregation framework, maybe below is
>> one solution, correct me if it is wrong.
>
> It isn't so much wrong as incomplete.
>
> It seems to me we can have
>
> - can queue, buffer not full -> do nothing more
As I said above, without a timeout timer, the queued skb
might not be sent out even some long time passed if no
frame comes later.
So could you add the wait for more mechanism to your patch
for further discussion?
> - can queue, buffer full -> transmit
> - cannot queue, buffer full -> transmit and then try again to queue
This might not happen, the buffer full should have been observed
in last xmit path.
>
> and an error case
>
> - cannot queue, buffer not full
Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 2/6] net: pxaficp_ir: add irq resources
From: Eric Miao @ 2012-09-07 3:54 UTC (permalink / raw)
To: Rob Herring
Cc: linux-arm-kernel, Russell King, Arnd Bergmann, Olof Johansson,
Linus Walleij, Rob Herring, Samuel Ortiz, Haojian Zhuang, netdev
In-Reply-To: <1346960563-18689-3-git-send-email-robherring2@gmail.com>
On Fri, Sep 7, 2012 at 3:42 AM, Rob Herring <robherring2@gmail.com> wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> In order to remove dependency on mach/irqs.h, add platform device
> resources for irqs.
>
Looks good to me, we shall have this change landed long before this, thanks Rob
Acked-by: Eric Miao <eric.y.miao@gmail.com>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> Cc: Samuel Ortiz <samuel@sortiz.org>
> Cc: Eric Miao <eric.y.miao@gmail.com>
> Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
> Cc: netdev@vger.kernel.org
> ---
> arch/arm/mach-pxa/devices.c | 15 +++++++++++++++
> drivers/net/irda/pxaficp_ir.c | 28 +++++++++++++++++-----------
> 2 files changed, 32 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/mach-pxa/devices.c b/arch/arm/mach-pxa/devices.c
> index 166eee5..339eb2a 100644
> --- a/arch/arm/mach-pxa/devices.c
> +++ b/arch/arm/mach-pxa/devices.c
> @@ -384,9 +384,24 @@ struct platform_device pxa_device_asoc_platform = {
>
> static u64 pxaficp_dmamask = ~(u32)0;
>
> +static struct resource pxa_ir_resources[] = {
> + [0] = {
> + .start = IRQ_STUART,
> + .end = IRQ_STUART,
> + .flags = IORESOURCE_IRQ,
> + },
> + [1] = {
> + .start = IRQ_ICP,
> + .end = IRQ_ICP,
> + .flags = IORESOURCE_IRQ,
> + },
> +};
> +
> struct platform_device pxa_device_ficp = {
> .name = "pxa2xx-ir",
> .id = -1,
> + .num_resources = ARRAY_SIZE(pxa_ir_resources),
> + .resource = pxa_ir_resources,
> .dev = {
> .dma_mask = &pxaficp_dmamask,
> .coherent_dma_mask = 0xffffffff,
> diff --git a/drivers/net/irda/pxaficp_ir.c b/drivers/net/irda/pxaficp_ir.c
> index 8d54767..cb0a5d3 100644
> --- a/drivers/net/irda/pxaficp_ir.c
> +++ b/drivers/net/irda/pxaficp_ir.c
> @@ -29,8 +29,8 @@
>
> #include <mach/dma.h>
> #include <mach/irda.h>
> -#include <mach/regs-uart.h>
> #include <mach/regs-ost.h>
> +#include <mach/regs-uart.h>
>
> #define FICP __REG(0x40800000) /* Start of FICP area */
> #define ICCR0 __REG(0x40800000) /* ICP Control Register 0 */
> @@ -112,6 +112,9 @@ struct pxa_irda {
> int txdma;
> int rxdma;
>
> + int uart_irq;
> + int icp_irq;
> +
> struct irlap_cb *irlap;
> struct qos_info qos;
>
> @@ -672,19 +675,19 @@ static int pxa_irda_start(struct net_device *dev)
>
> si->speed = 9600;
>
> - err = request_irq(IRQ_STUART, pxa_irda_sir_irq, 0, dev->name, dev);
> + err = request_irq(si->uart_irq, pxa_irda_sir_irq, 0, dev->name, dev);
> if (err)
> goto err_irq1;
>
> - err = request_irq(IRQ_ICP, pxa_irda_fir_irq, 0, dev->name, dev);
> + err = request_irq(si->icp_irq, pxa_irda_fir_irq, 0, dev->name, dev);
> if (err)
> goto err_irq2;
>
> /*
> * The interrupt must remain disabled for now.
> */
> - disable_irq(IRQ_STUART);
> - disable_irq(IRQ_ICP);
> + disable_irq(si->uart_irq);
> + disable_irq(si->icp_irq);
>
> err = -EBUSY;
> si->rxdma = pxa_request_dma("FICP_RX",DMA_PRIO_LOW, pxa_irda_fir_dma_rx_irq, dev);
> @@ -720,8 +723,8 @@ static int pxa_irda_start(struct net_device *dev)
> /*
> * Now enable the interrupt and start the queue
> */
> - enable_irq(IRQ_STUART);
> - enable_irq(IRQ_ICP);
> + enable_irq(si->uart_irq);
> + enable_irq(si->icp_irq);
> netif_start_queue(dev);
>
> printk(KERN_DEBUG "pxa_ir: irda driver opened\n");
> @@ -738,9 +741,9 @@ err_dma_rx_buff:
> err_tx_dma:
> pxa_free_dma(si->rxdma);
> err_rx_dma:
> - free_irq(IRQ_ICP, dev);
> + free_irq(si->icp_irq, dev);
> err_irq2:
> - free_irq(IRQ_STUART, dev);
> + free_irq(si->uart_irq, dev);
> err_irq1:
>
> return err;
> @@ -760,8 +763,8 @@ static int pxa_irda_stop(struct net_device *dev)
> si->irlap = NULL;
> }
>
> - free_irq(IRQ_STUART, dev);
> - free_irq(IRQ_ICP, dev);
> + free_irq(si->uart_irq, dev);
> + free_irq(si->icp_irq, dev);
>
> pxa_free_dma(si->rxdma);
> pxa_free_dma(si->txdma);
> @@ -851,6 +854,9 @@ static int pxa_irda_probe(struct platform_device *pdev)
> si->dev = &pdev->dev;
> si->pdata = pdev->dev.platform_data;
>
> + si->uart_irq = platform_get_irq(pdev, 0);
> + si->icp_irq = platform_get_irq(pdev, 1);
> +
> si->sir_clk = clk_get(&pdev->dev, "UARTCLK");
> si->fir_clk = clk_get(&pdev->dev, "FICPCLK");
> if (IS_ERR(si->sir_clk) || IS_ERR(si->fir_clk)) {
> --
> 1.7.9.5
>
^ permalink raw reply
* [PATCH] ip autoconfig: allow specifying a list of devices rather than one or all
From: Chris Friesen @ 2012-09-07 0:31 UTC (permalink / raw)
To: netdev
In-Reply-To: <50492547.8070307@genband.com>
On 09/06/2012 04:35 PM, Chris Friesen wrote:
> We need to be able to boot off of both the gigE ports for redundancy, so
> it seems like it might be a reasonable idea to extend the "ip=" boot arg
> to allow specifying a list of devices rather than choosing between a
> single device or all of them.
>
> Thoughts? Anyone ever done this?
Just for kicks I tried coding it up. This seems to work, so I include it
as fuel for discussion. This applies to linus' current git.
Chris
Allow the user to specify up to four device names as part of IP
autoconfiguration. This allows a degree of redundancy while also
allowing the user to limit which devices are opened by the kernel
during boot (this may be useful if some devices are very high traffic
or need special configuration).
Signed-off-by: Chris Friesen <chris.friesen@genband.com>
diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index 67e8a6b..4d24ad4 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -157,7 +157,9 @@ static u8 ic_domain[64]; /* DNS (not NIS) domain name */
*/
/* Name of user-selected boot device */
-static char user_dev_name[IFNAMSIZ] __initdata = { 0, };
+#define NUM_NAMED_BOOTDEV 4
+static int user_dev_count __initdata;
+static char user_dev_name[NUM_NAMED_BOOTDEV][IFNAMSIZ] __initdata;
/* Protocols supported by available interfaces */
static int ic_proto_have_if __initdata = 0;
@@ -189,16 +191,27 @@ struct ic_device {
static struct ic_device *ic_first_dev __initdata = NULL;/* List of open device */
static struct net_device *ic_dev __initdata = NULL; /* Selected device */
+static int __init dev_in_userdevlist(struct net_device *dev)
+{
+ int i;
+ for(i=0;i<NUM_NAMED_BOOTDEV;i++) {
+ if (!strcmp(dev->name, user_dev_name[i]))
+ return 1;
+ }
+ return 0;
+}
+
static bool __init ic_is_init_dev(struct net_device *dev)
{
if (dev->flags & IFF_LOOPBACK)
return false;
- return user_dev_name[0] ? !strcmp(dev->name, user_dev_name) :
+ return user_dev_count ? dev_in_userdevlist(dev) :
(!(dev->flags & IFF_LOOPBACK) &&
(dev->flags & (IFF_POINTOPOINT|IFF_BROADCAST)) &&
strncmp(dev->name, "dummy", 5));
}
+
static int __init ic_open_devs(void)
{
struct ic_device *d, **last;
@@ -274,9 +287,8 @@ have_carrier:
*last = NULL;
if (!ic_first_dev) {
- if (user_dev_name[0])
- pr_err("IP-Config: Device `%s' not found\n",
- user_dev_name);
+ if (user_dev_count)
+ pr_err("IP-Config: Specified boot device(s) not found\n");
else
pr_err("IP-Config: No network devices available\n");
return -ENODEV;
@@ -1605,7 +1617,16 @@ static int __init ip_auto_config_setup(char *addrs)
ic_host_name_set = 1;
break;
case 5:
- strlcpy(user_dev_name, ip, sizeof(user_dev_name));
+ do {
+ dp = strchr(ip, ',');
+ if (dp)
+ *dp++ = '\0';
+ strlcpy(user_dev_name[user_dev_count], ip,
+ sizeof(user_dev_name[0]));
+ user_dev_count++;
+ if (dp)
+ ip = dp;
+ } while (dp && (user_dev_count < NUM_NAMED_BOOTDEV));
break;
case 6:
if (ic_proto_name(ip) == 0 &&
^ permalink raw reply related
* Re: [PATCHv3] virtio-spec: virtio network device multiqueue support
From: Michael S. Tsirkin @ 2012-09-07 0:26 UTC (permalink / raw)
To: Sasha Levin; +Cc: netdev, kvm, virtualization
In-Reply-To: <50493D04.1090408@gmail.com>
On Fri, Sep 07, 2012 at 02:17:08AM +0200, Sasha Levin wrote:
> Hi Michael,
>
> On 09/06/2012 02:08 PM, Michael S. Tsirkin wrote:
> > Add multiqueue support to virtio network device.
> > Add a new feature flag VIRTIO_NET_F_MULTIQUEUE for this feature, a new
> > configuration field max_virtqueue_pairs to detect supported number of
> > virtqueues as well as a new command VIRTIO_NET_CTRL_STEERING to program
> > packet steering.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> Some comments about the change:
>
> - "The following four read-only fields only exists if VIRTIO_NET_F_MULTIQUEUE
> is set." => Should be "exist" (I think).
>
> - "When rule is set to VIRTIO_NET_CTRL_STEERING_RX_FOLLOWS_TX packets are
> steered by driver to the first (param+1) multiqueue virtqueues
> transmitq1...transmitqN;" - Why param+1? I thought we ignore the default
> transmit/receive in this case.
This is so that all values are valid. E.g. param=0 -> use q1.
>
> - "As selecting a specific steering ais n optimization feature" - "is an".
>
> - It's mentioned several times that the ability to read the steering rule from
> the virtio-net config is there for debug reasons. Is it really necessary? I
> think it's the first time I see debug features go in as part of the spec.
I actually think we need to work on more debug features.
> - I'm slightly confused, why are there both receive and transmit steering? I
> can't find a difference in the way to configure the rule for transmit and
> receive. Is it a plan for the future to allow different rules for tx and rx? If
> so, shouldn't we use different ctrl commands (
> VIRTIO_NET_CTRL_TX_STEERING/VIRTIO_NET_CTRL_RX_STEERING)?
No. Receive steering is done by device. Documented for driver writer's
benefit. xmit streering by driver. documented for device benefit.
They must match for tcp to work well so I doubt we will have
commands to control them separately.
> - "When rule is set to VIRTIO_NET_CTRL_STEERING_SINGLE all packets are steered
> to the default virtqueue receveq (0);" - "receiveq (0)"
>
>
>
> Thanks,
> Sasha
Thanks for the comments will address!
^ permalink raw reply
* Re: [PATCHv3] virtio-spec: virtio network device multiqueue support
From: Sasha Levin @ 2012-09-07 0:17 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, kvm, virtualization
In-Reply-To: <20120906120828.GA1534@redhat.com>
Hi Michael,
On 09/06/2012 02:08 PM, Michael S. Tsirkin wrote:
> Add multiqueue support to virtio network device.
> Add a new feature flag VIRTIO_NET_F_MULTIQUEUE for this feature, a new
> configuration field max_virtqueue_pairs to detect supported number of
> virtqueues as well as a new command VIRTIO_NET_CTRL_STEERING to program
> packet steering.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Some comments about the change:
- "The following four read-only fields only exists if VIRTIO_NET_F_MULTIQUEUE
is set." => Should be "exist" (I think).
- "When rule is set to VIRTIO_NET_CTRL_STEERING_RX_FOLLOWS_TX packets are
steered by driver to the first (param+1) multiqueue virtqueues
transmitq1...transmitqN;" - Why param+1? I thought we ignore the default
transmit/receive in this case.
- "As selecting a specific steering ais n optimization feature" - "is an".
- It's mentioned several times that the ability to read the steering rule from
the virtio-net config is there for debug reasons. Is it really necessary? I
think it's the first time I see debug features go in as part of the spec.
- I'm slightly confused, why are there both receive and transmit steering? I
can't find a difference in the way to configure the rule for transmit and
receive. Is it a plan for the future to allow different rules for tx and rx? If
so, shouldn't we use different ctrl commands (
VIRTIO_NET_CTRL_TX_STEERING/VIRTIO_NET_CTRL_RX_STEERING)?
- "When rule is set to VIRTIO_NET_CTRL_STEERING_SINGLE all packets are steered
to the default virtqueue receveq (0);" - "receiveq (0)"
Thanks,
Sasha
^ permalink raw reply
* RE: [net-next 4/6] igb: Store the MAC address in the name in the PTP struct.
From: Keller, Jacob E @ 2012-09-06 23:40 UTC (permalink / raw)
To: Ben Hutchings
Cc: Richard Cochran, Kirsher, Jeffrey T, davem@davemloft.net,
Vick, Matthew, netdev@vger.kernel.org, gospo@redhat.com,
sassmann@redhat.com
In-Reply-To: <1346974770.2714.64.camel@bwh-desktop.uk.solarflarecom.com>
> -----Original Message-----
> From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> Sent: Thursday, September 06, 2012 4:40 PM
> To: Keller, Jacob E
> Cc: Richard Cochran; Kirsher, Jeffrey T; davem@davemloft.net; Vick,
> Matthew; netdev@vger.kernel.org; gospo@redhat.com; sassmann@redhat.com
> Subject: RE: [net-next 4/6] igb: Store the MAC address in the name in the
> PTP struct.
>
> On Thu, 2012-09-06 at 23:24 +0000, Keller, Jacob E wrote:
> > > -----Original Message-----
> > > From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> > > Sent: Thursday, September 06, 2012 4:23 PM
> > > To: Keller, Jacob E; Richard Cochran
> > > Cc: Kirsher, Jeffrey T; davem@davemloft.net; Vick, Matthew;
> > > netdev@vger.kernel.org; gospo@redhat.com; sassmann@redhat.com
> > > Subject: RE: [net-next 4/6] igb: Store the MAC address in the name
> > > in the PTP struct.
> > >
> > > On Thu, 2012-09-06 at 22:59 +0000, Keller, Jacob E wrote:
> > > > > -----Original Message-----
> > > > > From: netdev-owner@vger.kernel.org
> > > > > [mailto:netdev-owner@vger.kernel.org]
> > > > > On Behalf Of Richard Cochran
> > > > > Sent: Thursday, September 06, 2012 1:06 AM
> > > > > To: Kirsher, Jeffrey T
> > > > > Cc: davem@davemloft.net; Vick, Matthew; netdev@vger.kernel.org;
> > > > > gospo@redhat.com; sassmann@redhat.com
> > > > > Subject: Re: [net-next 4/6] igb: Store the MAC address in the
> > > > > name in the PTP struct.
> > > > >
> > > > > On Wed, Sep 05, 2012 at 04:35:04PM -0700, Jeff Kirsher wrote:
> > > > > > From: Matthew Vick <matthew.vick@intel.com>
> > > > > >
> > > > > > Change the name of the adapter in the PTP struct to enable
> > > > > > easier correlation between interface and PTP device.
> > > > >
> > > > > Still think it is better to have the driver name, not the MAC
> address.
> > > > > The correlation is clear by using the ethtool method.
> > > > >
> > > >
> > > > I found a method to correct the init code so that the device name
> > > > can appear here, and I would prefer that over either the driver
> > > > name or MAC address. It requires moving the ptp initialization
> > > > into the netdev open handler though.
> > >
> > > I don't think you should use the net device name for this, as net
> > > devices can be renamed after creation. (Though you could update the
> > > struct ptp_clock_info whenever this happens.)
> > >
> > > I'm starting to wonder what use there is for a 'clock name' distinct
> > > from the clock device name. What would be more useful is to give it a
> parent.
> > > Richard, is there any reason why ptp_clock_register() doesn't allow
> > > specifying the parent device? You can then make the clock_name
> > > attribute contain the parent device's driver name or whatever it is
> you prefer.
> > >
> > > Ben.
> >
> > Because ideally the ptp device belongs to multiple ethernet devices...
> >
> > Sadly current intel hardware doesn't support this.
>
> The PTP clock is instantiated by some driver, bound to some device,
> whether that's a PCI device or a platform device or something else. I
> think that device (not a net device) should be the parent of the clock
> device.
>
Agreed, that would make the most sense.
> Ben.
>
> --
> Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer;
> that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* RE: [net-next 4/6] igb: Store the MAC address in the name in the PTP struct.
From: Ben Hutchings @ 2012-09-06 23:39 UTC (permalink / raw)
To: Keller, Jacob E
Cc: Richard Cochran, Kirsher, Jeffrey T, davem@davemloft.net,
Vick, Matthew, netdev@vger.kernel.org, gospo@redhat.com,
sassmann@redhat.com
In-Reply-To: <02874ECE860811409154E81DA85FBB580785FA77@ORSMSX105.amr.corp.intel.com>
On Thu, 2012-09-06 at 23:24 +0000, Keller, Jacob E wrote:
> > -----Original Message-----
> > From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> > Sent: Thursday, September 06, 2012 4:23 PM
> > To: Keller, Jacob E; Richard Cochran
> > Cc: Kirsher, Jeffrey T; davem@davemloft.net; Vick, Matthew;
> > netdev@vger.kernel.org; gospo@redhat.com; sassmann@redhat.com
> > Subject: RE: [net-next 4/6] igb: Store the MAC address in the name in the
> > PTP struct.
> >
> > On Thu, 2012-09-06 at 22:59 +0000, Keller, Jacob E wrote:
> > > > -----Original Message-----
> > > > From: netdev-owner@vger.kernel.org
> > > > [mailto:netdev-owner@vger.kernel.org]
> > > > On Behalf Of Richard Cochran
> > > > Sent: Thursday, September 06, 2012 1:06 AM
> > > > To: Kirsher, Jeffrey T
> > > > Cc: davem@davemloft.net; Vick, Matthew; netdev@vger.kernel.org;
> > > > gospo@redhat.com; sassmann@redhat.com
> > > > Subject: Re: [net-next 4/6] igb: Store the MAC address in the name
> > > > in the PTP struct.
> > > >
> > > > On Wed, Sep 05, 2012 at 04:35:04PM -0700, Jeff Kirsher wrote:
> > > > > From: Matthew Vick <matthew.vick@intel.com>
> > > > >
> > > > > Change the name of the adapter in the PTP struct to enable easier
> > > > > correlation between interface and PTP device.
> > > >
> > > > Still think it is better to have the driver name, not the MAC address.
> > > > The correlation is clear by using the ethtool method.
> > > >
> > >
> > > I found a method to correct the init code so that the device name can
> > > appear here, and I would prefer that over either the driver name or
> > > MAC address. It requires moving the ptp initialization into the netdev
> > > open handler though.
> >
> > I don't think you should use the net device name for this, as net devices
> > can be renamed after creation. (Though you could update the struct
> > ptp_clock_info whenever this happens.)
> >
> > I'm starting to wonder what use there is for a 'clock name' distinct from
> > the clock device name. What would be more useful is to give it a parent.
> > Richard, is there any reason why ptp_clock_register() doesn't allow
> > specifying the parent device? You can then make the clock_name attribute
> > contain the parent device's driver name or whatever it is you prefer.
> >
> > Ben.
>
> Because ideally the ptp device belongs to multiple ethernet devices...
>
> Sadly current intel hardware doesn't support this.
The PTP clock is instantiated by some driver, bound to some device,
whether that's a PCI device or a platform device or something else. I
think that device (not a net device) should be the parent of the clock
device.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* RE: [net-next 4/6] igb: Store the MAC address in the name in the PTP struct.
From: Keller, Jacob E @ 2012-09-06 23:24 UTC (permalink / raw)
To: Ben Hutchings, Richard Cochran
Cc: Kirsher, Jeffrey T, davem@davemloft.net, Vick, Matthew,
netdev@vger.kernel.org, gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <1346973787.2714.62.camel@bwh-desktop.uk.solarflarecom.com>
> -----Original Message-----
> From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> Sent: Thursday, September 06, 2012 4:23 PM
> To: Keller, Jacob E; Richard Cochran
> Cc: Kirsher, Jeffrey T; davem@davemloft.net; Vick, Matthew;
> netdev@vger.kernel.org; gospo@redhat.com; sassmann@redhat.com
> Subject: RE: [net-next 4/6] igb: Store the MAC address in the name in the
> PTP struct.
>
> On Thu, 2012-09-06 at 22:59 +0000, Keller, Jacob E wrote:
> > > -----Original Message-----
> > > From: netdev-owner@vger.kernel.org
> > > [mailto:netdev-owner@vger.kernel.org]
> > > On Behalf Of Richard Cochran
> > > Sent: Thursday, September 06, 2012 1:06 AM
> > > To: Kirsher, Jeffrey T
> > > Cc: davem@davemloft.net; Vick, Matthew; netdev@vger.kernel.org;
> > > gospo@redhat.com; sassmann@redhat.com
> > > Subject: Re: [net-next 4/6] igb: Store the MAC address in the name
> > > in the PTP struct.
> > >
> > > On Wed, Sep 05, 2012 at 04:35:04PM -0700, Jeff Kirsher wrote:
> > > > From: Matthew Vick <matthew.vick@intel.com>
> > > >
> > > > Change the name of the adapter in the PTP struct to enable easier
> > > > correlation between interface and PTP device.
> > >
> > > Still think it is better to have the driver name, not the MAC address.
> > > The correlation is clear by using the ethtool method.
> > >
> >
> > I found a method to correct the init code so that the device name can
> > appear here, and I would prefer that over either the driver name or
> > MAC address. It requires moving the ptp initialization into the netdev
> > open handler though.
>
> I don't think you should use the net device name for this, as net devices
> can be renamed after creation. (Though you could update the struct
> ptp_clock_info whenever this happens.)
>
> I'm starting to wonder what use there is for a 'clock name' distinct from
> the clock device name. What would be more useful is to give it a parent.
> Richard, is there any reason why ptp_clock_register() doesn't allow
> specifying the parent device? You can then make the clock_name attribute
> contain the parent device's driver name or whatever it is you prefer.
>
> Ben.
Because ideally the ptp device belongs to multiple ethernet devices...
Sadly current intel hardware doesn't support this.
- Jake
>
> --
> Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer;
> that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* RE: [net-next 4/6] igb: Store the MAC address in the name in the PTP struct.
From: Ben Hutchings @ 2012-09-06 23:23 UTC (permalink / raw)
To: Keller, Jacob E, Richard Cochran
Cc: Kirsher, Jeffrey T, davem@davemloft.net, Vick, Matthew,
netdev@vger.kernel.org, gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <02874ECE860811409154E81DA85FBB580785F9F4@ORSMSX105.amr.corp.intel.com>
On Thu, 2012-09-06 at 22:59 +0000, Keller, Jacob E wrote:
> > -----Original Message-----
> > From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> > On Behalf Of Richard Cochran
> > Sent: Thursday, September 06, 2012 1:06 AM
> > To: Kirsher, Jeffrey T
> > Cc: davem@davemloft.net; Vick, Matthew; netdev@vger.kernel.org;
> > gospo@redhat.com; sassmann@redhat.com
> > Subject: Re: [net-next 4/6] igb: Store the MAC address in the name in the
> > PTP struct.
> >
> > On Wed, Sep 05, 2012 at 04:35:04PM -0700, Jeff Kirsher wrote:
> > > From: Matthew Vick <matthew.vick@intel.com>
> > >
> > > Change the name of the adapter in the PTP struct to enable easier
> > > correlation between interface and PTP device.
> >
> > Still think it is better to have the driver name, not the MAC address.
> > The correlation is clear by using the ethtool method.
> >
>
> I found a method to correct the init code so that the device name can
> appear here, and I would prefer that over either the driver name or
> MAC address. It requires moving the ptp initialization into the netdev
> open handler though.
I don't think you should use the net device name for this, as net
devices can be renamed after creation. (Though you could update the
struct ptp_clock_info whenever this happens.)
I'm starting to wonder what use there is for a 'clock name' distinct
from the clock device name. What would be more useful is to give it a
parent. Richard, is there any reason why ptp_clock_register() doesn't
allow specifying the parent device? You can then make the clock_name
attribute contain the parent device's driver name or whatever it is you
prefer.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCHv3] virtio-spec: virtio network device multiqueue support
From: Michael S. Tsirkin @ 2012-09-06 23:16 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: netdev, kvm, virtualization
In-Reply-To: <5048D2B8.4060003@redhat.com>
On Thu, Sep 06, 2012 at 06:43:36PM +0200, Paolo Bonzini wrote:
> Il 06/09/2012 14:08, Michael S. Tsirkin ha scritto:
> > +#define VIRTIO_NET_CTRL_STEERING_HOST 1
>
> You didn't change this occurrence.
Good catch, thanks!
^ permalink raw reply
* RE: [net-next 13/13] igb: Store the MAC address in the name in the PTP struct.
From: Keller, Jacob E @ 2012-09-06 23:04 UTC (permalink / raw)
To: Richard Cochran, Ben Hutchings
Cc: Kirsher, Jeffrey T, davem@davemloft.net, Vick, Matthew,
netdev@vger.kernel.org, gospo@redhat.com, sassmann@redhat.com,
Stuart Hodgson
In-Reply-To: <20120824061956.GA2212@netboy.at.omicron.at>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Richard Cochran
> Sent: Thursday, August 23, 2012 11:20 PM
> To: Ben Hutchings
> Cc: Kirsher, Jeffrey T; davem@davemloft.net; Vick, Matthew;
> netdev@vger.kernel.org; gospo@redhat.com; sassmann@redhat.com; Stuart
> Hodgson
> Subject: Re: [net-next 13/13] igb: Store the MAC address in the name in
> the PTP struct.
>
> In an ideal world, there is only one clock device per system, so the
> original concept was to use the driver name. The hardware designers have
> ignored or not understood the "one clock per system" model, and so,
> unfortunately, we have to deal with it.
>
> A clock device can and should be connected to more than one network device
> (see gianfar, dp83640), and to give it a MAC address is misleading.
>
> The ethtool solution works perfectly to go from interface->clock, which is
> all the information that a user space PTP stack needs.
>
> I think for sysfs and the ioctl it is nicer to have the driver name, since
> it is associated with the clock and its capabilities.
>
Correct. MAC address is misleading. However driver name for our parts is
misleading because each driver creates one clock per port. In either case, one
driver handling multiple cards would also have to create multiple ptp devices
and that is just as misleading.
The ethtool method is definitely preferred, and is the correct way to identify
the clock device correlation.
- Jake
> Thanks,
> Richard
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in the
> body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [net-next 4/6] igb: Store the MAC address in the name in the PTP struct.
From: Keller, Jacob E @ 2012-09-06 22:59 UTC (permalink / raw)
To: Richard Cochran, Kirsher, Jeffrey T
Cc: davem@davemloft.net, Vick, Matthew, netdev@vger.kernel.org,
gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <20120906080628.GD2550@netboy.at.omicron.at>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Richard Cochran
> Sent: Thursday, September 06, 2012 1:06 AM
> To: Kirsher, Jeffrey T
> Cc: davem@davemloft.net; Vick, Matthew; netdev@vger.kernel.org;
> gospo@redhat.com; sassmann@redhat.com
> Subject: Re: [net-next 4/6] igb: Store the MAC address in the name in the
> PTP struct.
>
> On Wed, Sep 05, 2012 at 04:35:04PM -0700, Jeff Kirsher wrote:
> > From: Matthew Vick <matthew.vick@intel.com>
> >
> > Change the name of the adapter in the PTP struct to enable easier
> > correlation between interface and PTP device.
>
> Still think it is better to have the driver name, not the MAC address.
> The correlation is clear by using the ethtool method.
>
I found a method to correct the init code so that the device name can appear here, and I would prefer that over either the driver name or MAC address. It requires moving the ptp initialization into the netdev open handler though.
- Jake
> Thanks,
> Richard
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in the
> body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [RFC] ip autoconfig: allow specifying a list of devices rather than one or all
From: Chris Friesen @ 2012-09-06 22:35 UTC (permalink / raw)
To: netdev
We have an embedded system that boots of the network. This system has a
pair of bonded gigE links used for booting and management, and a pair of
bonded 10G links used for data traffic.
What we're seeing is that when ip_auto_config() calls
ic_open_devs() to open up all the network ports, the data ports
immediately start getting *hammered* with traffic, we get "nf_conntrack:
table full, dropping packet", errors, and we sometimes lose the DHCP
responses because of this.
We need to be able to boot off of both the gigE ports for redundancy, so
it seems like it might be a reasonable idea to extend the "ip=" boot arg
to allow specifying a list of devices rather than choosing between a
single device or all of them.
Thoughts? Anyone ever done this?
Chris
--
Chris Friesen
Software Designer
3500 Carling Avenue
Ottawa, Ontario K2H 8E9
www.genband.com
^ permalink raw reply
* Re: [PATCH v2 06/10] cgroup: Assign subsystem IDs during compile time
From: Tejun Heo @ 2012-09-06 22:32 UTC (permalink / raw)
To: Daniel Wagner
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, cgroups-u79uwXL29TY76Z2rM5mHXA,
David S. Miller, Andrew Morton, Eric Dumazet, Gao feng,
Glauber Costa, Jamal Hadi Salim, John Fastabend,
Kamezawa Hiroyuki, Li Zefan, Neil Horman
In-Reply-To: <50390743.2090203-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
Hello, Daniel.
On Sat, Aug 25, 2012 at 07:11:31PM +0200, Daniel Wagner wrote:
> On 25.08.2012 01:38, Tejun Heo wrote:
> >>task_cls_classid() and task_netprioidx() (when built as
> >>module) are protected by a jump label and therefore we can
> >>simply replace the subsystem index lookup with the enum.
> >
> >Can we put these in a separate patch? ie. The first patch makes all
> >subsys IDs constant and then patches to simplify users.
>
> Wouldn't this break bisection? I merged this step so that all steps
> in this series are able to compile and run.
I don't see why it should but maybe I'm missing something. If so,
please enlighten me.
> >>+#define IS_SUBSYS_ENABLED(option) IS_MODULE(option)
> >>+#include <linux/cgroup_subsys.h>
> >>+#undef IS_SUBSYS_ENABLED
> >>+
> >
> >Why do we need to segregate in-kernel and modular ones at all? What's
> >wrong with just defining them in one go?
>
> I have done that but the result was a panic. There seems some code
> which expects this ordering. Let me dig into this and fix it.
Yes, please.
> >Hmm... patch sequence looks odd to me. If you first make all IDs
> >constant, you can first remove module specific ones and then later add
> >jump labels as separate patches. Wouldn't that be simpler?
>
> As said above, I tried to keep all steps usable so bisection would
> work. I think your steps would lead to non working versions of the
> kernel.
Hmmm... Yes, it should be bisectable but again I don't see why being
biesectable interferes with the patch ordering here.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v2 05/10] cgroup: Remove CGROUP_BUILTIN_SUBSYS_COUNT
From: Tejun Heo @ 2012-09-06 22:23 UTC (permalink / raw)
To: Daniel Wagner
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, cgroups-u79uwXL29TY76Z2rM5mHXA,
Li Zefan, David S. Miller
In-Reply-To: <5039046D.1040402-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
Hello, Daniel.
On Sat, Aug 25, 2012 at 06:59:25PM +0200, Daniel Wagner wrote:
> >Wouldn't it be better to explicitly state that a following patch would
> >reduce the SUBSYS_COUNT and stop putting builtin and module ones into
> >different sections?
>
> Sure, can do that. I just to make sure I understand you correctly,
> What do you mean with different section? Do you refer to the enum sorting?
I meant that there's no reason to have all builtin ones contiguosly
and then the module ones using two cgroup_subsys.h inclusions.
Thanks!
--
tejun
^ permalink raw reply
* Re: [PATCH net-next] ipv6: Export nd_tbl to allow modules to support IPv6
From: Thomas Graf @ 2012-09-06 21:06 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20120906.134738.425625608685741613.davem@davemloft.net>
On Thu, Sep 06, 2012 at 01:47:38PM -0400, David Miller wrote:
> Right now we're in a transition period where ipv4 is mostly refcount'less
> and ipv6 is not.
>
> This is part of the reason I don't want to expose these things, the
> calling convention and locking requirements are going to be fluid for
> any interface you might propose.
>
> arp_tbl was exported only for in-tree users, and I therefore say that
> we should only export nd_tbl for in-tree users as well, since that's
> the only way we can audit and update all the referencing callers as
> the semantics radically change.
Thanks for the explanations David. I'll pass this on to the involved
projects. We'll probably try to pick this up again as soon as the new
locking conventions have been put in place and are considered stable.
^ permalink raw reply
* Re: [ethtool 2/2] ethtool: allow setting MDI-X state
From: Jeff Kirsher @ 2012-09-06 20:57 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Jesse Brandeburg, netdev, gospo, sassmann
In-Reply-To: <1346955923.2714.26.camel@bwh-desktop.uk.solarflarecom.com>
[-- Attachment #1: Type: text/plain, Size: 3085 bytes --]
On Thu, 2012-09-06 at 19:25 +0100, Ben Hutchings wrote:
> On Tue, 2012-08-21 at 01:37 -0700, Jeff Kirsher wrote:
> > From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> >
> > A bit ago ethtool added support for reading MDI-X state, this
> > patch finishes the implementation, adding the complementary write
> > command.
> [...]
>
> Applied. I also commited the following changes:
>
> ---
> Subject: ethtool.8: Mark-up mdix arguments properly
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
> ethtool.8.in | 11 ++++++-----
> 1 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/ethtool.8.in b/ethtool.8.in
> index 3208d38..2799e25 100644
> --- a/ethtool.8.in
> +++ b/ethtool.8.in
> @@ -523,11 +523,12 @@ Sets full or half duplex mode.
> Selects device port.
> .TP
> .A3 mdix auto on off
> -Selects MDI-X mode for port. May be used to override the automatic detection
> -feature of most adapters. Auto means automatic detection of MDI status, on
> -forces MDI-X (crossover) mode, while off means MDI (straight through) mode.
> -The driver should guarantee that this command takes effect immediately, and
> -if necessary may reset the link to cause the change to take effect.
> +Selects MDI-X mode for port. May be used to override the automatic
> +detection feature of most adapters. An argument of \fBauto\fR means
> +automatic detection of MDI status, \fBon\fR forces MDI-X (crossover)
> +mode, while \fBoff\fR means MDI (straight through) mode. The driver
> +should guarantee that this command takes effect immediately, and if
> +necessary may reset the link to cause the change to take effect.
> .TP
> .A2 autoneg on off
> Specifies whether autonegotiation should be enabled. Autonegotiation
> ---
> Subject: test-cmdline: Test -s mdix keyword
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
> test-cmdline.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/test-cmdline.c b/test-cmdline.c
> index 6a60ed4..85b4ce0 100644
> --- a/test-cmdline.c
> +++ b/test-cmdline.c
> @@ -22,12 +22,14 @@ static struct test_case {
> { 1, "16_char_devname!" },
> /* Argument parsing for -s is specialised */
> { 0, "-s devname" },
> - { 0, "--change devname speed 100 duplex half" },
> + { 0, "--change devname speed 100 duplex half mdix auto" },
> { 1, "-s devname speed foo" },
> { 1, "--change devname speed" },
> { 0, "-s devname duplex half" },
> { 1, "--change devname duplex foo" },
> { 1, "-s devname duplex" },
> + { 1, "--change devname mdix foo" },
> + { 1, "-s devname mdix" },
> { 0, "--change devname port tp" },
> { 1, "-s devname port foo" },
> { 1, "--change devname port" },
> ---
>
> Please include at least basic test cases like this for any new feature.
> I did it this time because I've already kept you waiting and didn't
> think it would be fair to request changes.
>
> Ben.
>
Thanks Ben, I will make sure that we cover that with any new features
going forward.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/6] net: pxaficp_ir: add irq resources
From: David Miller @ 2012-09-06 20:46 UTC (permalink / raw)
To: robherring2
Cc: linux-arm-kernel, linux, arnd, olof, linus.walleij, rob.herring,
samuel, eric.y.miao, haojian.zhuang, netdev
In-Reply-To: <1346960563-18689-3-git-send-email-robherring2@gmail.com>
From: Rob Herring <robherring2@gmail.com>
Date: Thu, 6 Sep 2012 14:42:39 -0500
> From: Rob Herring <rob.herring@calxeda.com>
>
> In order to remove dependency on mach/irqs.h, add platform device
> resources for irqs.
>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
Feel free to put this in via the ARM tree.
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* [PATCH v2 2/6] net: pxaficp_ir: add irq resources
From: Rob Herring @ 2012-09-06 19:42 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Russell King, Arnd Bergmann, Olof Johansson, Linus Walleij,
Rob Herring, Samuel Ortiz, Eric Miao, Haojian Zhuang, netdev
In-Reply-To: <1346960563-18689-1-git-send-email-robherring2@gmail.com>
From: Rob Herring <rob.herring@calxeda.com>
In order to remove dependency on mach/irqs.h, add platform device
resources for irqs.
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
Cc: Samuel Ortiz <samuel@sortiz.org>
Cc: Eric Miao <eric.y.miao@gmail.com>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Cc: netdev@vger.kernel.org
---
arch/arm/mach-pxa/devices.c | 15 +++++++++++++++
drivers/net/irda/pxaficp_ir.c | 28 +++++++++++++++++-----------
2 files changed, 32 insertions(+), 11 deletions(-)
diff --git a/arch/arm/mach-pxa/devices.c b/arch/arm/mach-pxa/devices.c
index 166eee5..339eb2a 100644
--- a/arch/arm/mach-pxa/devices.c
+++ b/arch/arm/mach-pxa/devices.c
@@ -384,9 +384,24 @@ struct platform_device pxa_device_asoc_platform = {
static u64 pxaficp_dmamask = ~(u32)0;
+static struct resource pxa_ir_resources[] = {
+ [0] = {
+ .start = IRQ_STUART,
+ .end = IRQ_STUART,
+ .flags = IORESOURCE_IRQ,
+ },
+ [1] = {
+ .start = IRQ_ICP,
+ .end = IRQ_ICP,
+ .flags = IORESOURCE_IRQ,
+ },
+};
+
struct platform_device pxa_device_ficp = {
.name = "pxa2xx-ir",
.id = -1,
+ .num_resources = ARRAY_SIZE(pxa_ir_resources),
+ .resource = pxa_ir_resources,
.dev = {
.dma_mask = &pxaficp_dmamask,
.coherent_dma_mask = 0xffffffff,
diff --git a/drivers/net/irda/pxaficp_ir.c b/drivers/net/irda/pxaficp_ir.c
index 8d54767..cb0a5d3 100644
--- a/drivers/net/irda/pxaficp_ir.c
+++ b/drivers/net/irda/pxaficp_ir.c
@@ -29,8 +29,8 @@
#include <mach/dma.h>
#include <mach/irda.h>
-#include <mach/regs-uart.h>
#include <mach/regs-ost.h>
+#include <mach/regs-uart.h>
#define FICP __REG(0x40800000) /* Start of FICP area */
#define ICCR0 __REG(0x40800000) /* ICP Control Register 0 */
@@ -112,6 +112,9 @@ struct pxa_irda {
int txdma;
int rxdma;
+ int uart_irq;
+ int icp_irq;
+
struct irlap_cb *irlap;
struct qos_info qos;
@@ -672,19 +675,19 @@ static int pxa_irda_start(struct net_device *dev)
si->speed = 9600;
- err = request_irq(IRQ_STUART, pxa_irda_sir_irq, 0, dev->name, dev);
+ err = request_irq(si->uart_irq, pxa_irda_sir_irq, 0, dev->name, dev);
if (err)
goto err_irq1;
- err = request_irq(IRQ_ICP, pxa_irda_fir_irq, 0, dev->name, dev);
+ err = request_irq(si->icp_irq, pxa_irda_fir_irq, 0, dev->name, dev);
if (err)
goto err_irq2;
/*
* The interrupt must remain disabled for now.
*/
- disable_irq(IRQ_STUART);
- disable_irq(IRQ_ICP);
+ disable_irq(si->uart_irq);
+ disable_irq(si->icp_irq);
err = -EBUSY;
si->rxdma = pxa_request_dma("FICP_RX",DMA_PRIO_LOW, pxa_irda_fir_dma_rx_irq, dev);
@@ -720,8 +723,8 @@ static int pxa_irda_start(struct net_device *dev)
/*
* Now enable the interrupt and start the queue
*/
- enable_irq(IRQ_STUART);
- enable_irq(IRQ_ICP);
+ enable_irq(si->uart_irq);
+ enable_irq(si->icp_irq);
netif_start_queue(dev);
printk(KERN_DEBUG "pxa_ir: irda driver opened\n");
@@ -738,9 +741,9 @@ err_dma_rx_buff:
err_tx_dma:
pxa_free_dma(si->rxdma);
err_rx_dma:
- free_irq(IRQ_ICP, dev);
+ free_irq(si->icp_irq, dev);
err_irq2:
- free_irq(IRQ_STUART, dev);
+ free_irq(si->uart_irq, dev);
err_irq1:
return err;
@@ -760,8 +763,8 @@ static int pxa_irda_stop(struct net_device *dev)
si->irlap = NULL;
}
- free_irq(IRQ_STUART, dev);
- free_irq(IRQ_ICP, dev);
+ free_irq(si->uart_irq, dev);
+ free_irq(si->icp_irq, dev);
pxa_free_dma(si->rxdma);
pxa_free_dma(si->txdma);
@@ -851,6 +854,9 @@ static int pxa_irda_probe(struct platform_device *pdev)
si->dev = &pdev->dev;
si->pdata = pdev->dev.platform_data;
+ si->uart_irq = platform_get_irq(pdev, 0);
+ si->icp_irq = platform_get_irq(pdev, 1);
+
si->sir_clk = clk_get(&pdev->dev, "UARTCLK");
si->fir_clk = clk_get(&pdev->dev, "FICPCLK");
if (IS_ERR(si->sir_clk) || IS_ERR(si->fir_clk)) {
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v2] ethtool: don't overwrite useful bits in advertising bitfield
From: Ben Hutchings @ 2012-09-06 19:05 UTC (permalink / raw)
To: Johan Gunnarsson; +Cc: netdev
In-Reply-To: <1345807965-16236-1-git-send-email-johangu@axis.com>
On Fri, 2012-08-24 at 13:32 +0200, Johan Gunnarsson wrote:
> There are bits in this bitfield that we want to leave untouched (PAUSE
> and ASYM_PAUSE bits) when changing other bits (speed and duplex bits.)
> Previously, these were always overwritten to zero when running commands
> like "ethtool -s eth0 speed 10 duplex full autoneg off".
>
> Signed-off-by: Johan Gunnarsson <johangu@axis.com>
[...]
Applied, thanks. I made some minor fixes to spelling and formatting.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH 0/5] dev_<level> and dynamic_debug cleanups
From: Joe Perches @ 2012-09-06 18:43 UTC (permalink / raw)
To: Jason Baron
Cc: Andrew Morton, netdev, Greg Kroah-Hartman, David S. Miller,
Jim Cromie, Kay Sievers, linux-kernel
In-Reply-To: <20120906175158.GB9786@redhat.com>
On Thu, 2012-09-06 at 13:51 -0400, Jason Baron wrote:
> On Sun, Aug 26, 2012 at 04:25:25AM -0700, Joe Perches wrote:
> > The recent commit to fix dynamic_debug was a bit unclean.
> > Neaten the style for dynamic_debug.
> > Reduce the stack use of message logging that uses netdev_printk
> > Add utility functions dev_printk_emit and dev_vprintk_emit for /dev/kmsg.
> >
> > Joe Perches (5):
> > dev_dbg/dynamic_debug: Update to use printk_emit, optimize stack
> > netdev_printk/dynamic_netdev_dbg: Directly call printk_emit
> > netdev_printk/netif_printk: Remove a superfluous logging colon
> > dev: Add dev_vprintk_emit and dev_printk_emit
> > device and dynamic_debug: Use dev_vprintk_emit and dev_printk_emit
> >
>
> Looks Good.
>
> The one thing that is bothering me though, is that for
> __dynamic_dev_dbg(), __dynamic_netdev_dbg(), we are copying much of the core
> logic of __dev_printk(), __netdev_printk(), respectively. I would prefer
> have this in one place. Can we add a 'prefix' argument to __dev_printk(),
> and __netdev_printk() that dynamic debug can use, but is simply empty
> for dev_printk() and netdev_printk().
I don't think that's an improvement actually.
Why don't you try it.
^ permalink raw reply
* Re: [ethtool 2/2] ethtool: allow setting MDI-X state
From: Ben Hutchings @ 2012-09-06 18:25 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: Jesse Brandeburg, netdev, gospo, sassmann
In-Reply-To: <1345538236-1636-2-git-send-email-jeffrey.t.kirsher@intel.com>
On Tue, 2012-08-21 at 01:37 -0700, Jeff Kirsher wrote:
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
>
> A bit ago ethtool added support for reading MDI-X state, this
> patch finishes the implementation, adding the complementary write
> command.
[...]
Applied. I also commited the following changes:
---
Subject: ethtool.8: Mark-up mdix arguments properly
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
ethtool.8.in | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/ethtool.8.in b/ethtool.8.in
index 3208d38..2799e25 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -523,11 +523,12 @@ Sets full or half duplex mode.
Selects device port.
.TP
.A3 mdix auto on off
-Selects MDI-X mode for port. May be used to override the automatic detection
-feature of most adapters. Auto means automatic detection of MDI status, on
-forces MDI-X (crossover) mode, while off means MDI (straight through) mode.
-The driver should guarantee that this command takes effect immediately, and
-if necessary may reset the link to cause the change to take effect.
+Selects MDI-X mode for port. May be used to override the automatic
+detection feature of most adapters. An argument of \fBauto\fR means
+automatic detection of MDI status, \fBon\fR forces MDI-X (crossover)
+mode, while \fBoff\fR means MDI (straight through) mode. The driver
+should guarantee that this command takes effect immediately, and if
+necessary may reset the link to cause the change to take effect.
.TP
.A2 autoneg on off
Specifies whether autonegotiation should be enabled. Autonegotiation
---
Subject: test-cmdline: Test -s mdix keyword
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
test-cmdline.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/test-cmdline.c b/test-cmdline.c
index 6a60ed4..85b4ce0 100644
--- a/test-cmdline.c
+++ b/test-cmdline.c
@@ -22,12 +22,14 @@ static struct test_case {
{ 1, "16_char_devname!" },
/* Argument parsing for -s is specialised */
{ 0, "-s devname" },
- { 0, "--change devname speed 100 duplex half" },
+ { 0, "--change devname speed 100 duplex half mdix auto" },
{ 1, "-s devname speed foo" },
{ 1, "--change devname speed" },
{ 0, "-s devname duplex half" },
{ 1, "--change devname duplex foo" },
{ 1, "-s devname duplex" },
+ { 1, "--change devname mdix foo" },
+ { 1, "-s devname mdix" },
{ 0, "--change devname port tp" },
{ 1, "-s devname port foo" },
{ 1, "--change devname port" },
---
Please include at least basic test cases like this for any new feature.
I did it this time because I've already kept you waiting and didn't
think it would be fair to request changes.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* Re: [PATCH net-next] tcp: fix TFO regression
From: David Miller @ 2012-09-06 18:23 UTC (permalink / raw)
To: ncardwell
Cc: eric.dumazet, fengguang.wu, mkl, hkchu, ebiederm, netdev,
linux-can
In-Reply-To: <CADVnQykCJJdM3PfN=_1yyzizao2T6XQJGzC=3uwx5_oazaBicA@mail.gmail.com>
From: Neal Cardwell <ncardwell@google.com>
Date: Thu, 6 Sep 2012 14:15:38 -0400
> On Thu, Sep 6, 2012 at 2:07 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> From: Eric Dumazet <edumazet@google.com>
>>
>> Fengguang Wu reported various panics and bisected to commit
>> 8336886f786fdac (tcp: TCP Fast Open Server - support TFO listeners)
>>
>> Fix this by making sure socket is a TCP socket before accessing TFO data
>> structures.
>>
> ...
>>
>> Reported-by: Fengguang Wu <wfg@linux.intel.com>
>> Tested-by: Fengguang Wu <wfg@linux.intel.com>
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> Cc: "H.K. Jerry Chu" <hkchu@google.com>
>
> Acked-by: Neal Cardwell <ncardwell@google.com>
Applied, thanks everyone.
^ permalink raw reply
* Re: [PATCH net-next] tcp: fix TFO regression
From: Jerry Chu @ 2012-09-06 18:18 UTC (permalink / raw)
To: Eric Dumazet
Cc: Fengguang Wu, David Miller, Marc Kleine-Budde, Eric W. Biederman,
networking, linux-can
In-Reply-To: <1346954833.2484.50.camel@edumazet-glaptop>
On Thu, Sep 6, 2012 at 11:07 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Fengguang Wu reported various panics and bisected to commit
> 8336886f786fdac (tcp: TCP Fast Open Server - support TFO listeners)
>
> Fix this by making sure socket is a TCP socket before accessing TFO data
> structures.
>
> [ 233.046014] kfree_debugcheck: out of range ptr ea6000000bb8h.
> [ 233.047399] ------------[ cut here ]------------
> [ 233.048393] kernel BUG at /c/kernel-tests/src/stable/mm/slab.c:3074!
> [ 233.048393] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
> [ 233.048393] Modules linked in:
> [ 233.048393] CPU 0
> [ 233.048393] Pid: 3929, comm: trinity-watchdo Not tainted 3.6.0-rc3+
> #4192 Bochs Bochs
> [ 233.048393] RIP: 0010:[<ffffffff81169653>] [<ffffffff81169653>]
> kfree_debugcheck+0x27/0x2d
> [ 233.048393] RSP: 0018:ffff88000facbca8 EFLAGS: 00010092
> [ 233.048393] RAX: 0000000000000031 RBX: 0000ea6000000bb8 RCX:
> 00000000a189a188
> [ 233.048393] RDX: 000000000000a189 RSI: ffffffff8108ad32 RDI:
> ffffffff810d30f9
> [ 233.048393] RBP: ffff88000facbcb8 R08: 0000000000000002 R09:
> ffffffff843846f0
> [ 233.048393] R10: ffffffff810ae37c R11: 0000000000000908 R12:
> 0000000000000202
> [ 233.048393] R13: ffffffff823dbd5a R14: ffff88000ec5bea8 R15:
> ffffffff8363c780
> [ 233.048393] FS: 00007faa6899c700(0000) GS:ffff88001f200000(0000)
> knlGS:0000000000000000
> [ 233.048393] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [ 233.048393] CR2: 00007faa6841019c CR3: 0000000012c82000 CR4:
> 00000000000006f0
> [ 233.048393] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> 0000000000000000
> [ 233.048393] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
> 0000000000000400
> [ 233.048393] Process trinity-watchdo (pid: 3929, threadinfo
> ffff88000faca000, task ffff88000faec600)
> [ 233.048393] Stack:
> [ 233.048393] 0000000000000000 0000ea6000000bb8 ffff88000facbce8
> ffffffff8116ad81
> [ 233.048393] ffff88000ff588a0 ffff88000ff58850 ffff88000ff588a0
> 0000000000000000
> [ 233.048393] ffff88000facbd08 ffffffff823dbd5a ffffffff823dbcb0
> ffff88000ff58850
> [ 233.048393] Call Trace:
> [ 233.048393] [<ffffffff8116ad81>] kfree+0x5f/0xca
> [ 233.048393] [<ffffffff823dbd5a>] inet_sock_destruct+0xaa/0x13c
> [ 233.048393] [<ffffffff823dbcb0>] ? inet_sk_rebuild_header
> +0x319/0x319
> [ 233.048393] [<ffffffff8231c307>] __sk_free+0x21/0x14b
> [ 233.048393] [<ffffffff8231c4bd>] sk_free+0x26/0x2a
> [ 233.048393] [<ffffffff825372db>] sctp_close+0x215/0x224
> [ 233.048393] [<ffffffff810d6835>] ? lock_release+0x16f/0x1b9
> [ 233.048393] [<ffffffff823daf12>] inet_release+0x7e/0x85
> [ 233.048393] [<ffffffff82317d15>] sock_release+0x1f/0x77
> [ 233.048393] [<ffffffff82317d94>] sock_close+0x27/0x2b
> [ 233.048393] [<ffffffff81173bbe>] __fput+0x101/0x20a
> [ 233.048393] [<ffffffff81173cd5>] ____fput+0xe/0x10
> [ 233.048393] [<ffffffff810a3794>] task_work_run+0x5d/0x75
> [ 233.048393] [<ffffffff8108da70>] do_exit+0x290/0x7f5
> [ 233.048393] [<ffffffff82707415>] ? retint_swapgs+0x13/0x1b
> [ 233.048393] [<ffffffff8108e23f>] do_group_exit+0x7b/0xba
> [ 233.048393] [<ffffffff8108e295>] sys_exit_group+0x17/0x17
> [ 233.048393] [<ffffffff8270de10>] tracesys+0xdd/0xe2
> [ 233.048393] Code: 59 01 5d c3 55 48 89 e5 53 41 50 0f 1f 44 00 00 48
> 89 fb e8 d4 b0 f0 ff 84 c0 75 11 48 89 de 48 c7 c7 fc fa f7 82 e8 0d 0f
> 57 01 <0f> 0b 5f 5b 5d c3 55 48 89 e5 0f 1f 44 00 00 48 63 87 d8 00 00
> [ 233.048393] RIP [<ffffffff81169653>] kfree_debugcheck+0x27/0x2d
> [ 233.048393] RSP <ffff88000facbca8>
>
> Reported-by: Fengguang Wu <wfg@linux.intel.com>
> Tested-by: Fengguang Wu <wfg@linux.intel.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: "H.K. Jerry Chu" <hkchu@google.com>
> ---
> net/ipv4/af_inet.c | 7 ++-----
> net/ipv4/inet_connection_sock.c | 4 ++--
> 2 files changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 4f70ef0..845372b 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -149,11 +149,8 @@ void inet_sock_destruct(struct sock *sk)
> pr_err("Attempt to release alive inet socket %p\n", sk);
> return;
> }
> - if (sk->sk_type == SOCK_STREAM) {
> - struct fastopen_queue *fastopenq =
> - inet_csk(sk)->icsk_accept_queue.fastopenq;
> - kfree(fastopenq);
> - }
> + if (sk->sk_protocol == IPPROTO_TCP)
> + kfree(inet_csk(sk)->icsk_accept_queue.fastopenq);
>
> WARN_ON(atomic_read(&sk->sk_rmem_alloc));
> WARN_ON(atomic_read(&sk->sk_wmem_alloc));
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index 8464b79..f0c5b9c 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -314,7 +314,7 @@ struct sock *inet_csk_accept(struct sock *sk, int flags, int *err)
> newsk = req->sk;
>
> sk_acceptq_removed(sk);
> - if (sk->sk_type == SOCK_STREAM && queue->fastopenq != NULL) {
> + if (sk->sk_protocol == IPPROTO_TCP && queue->fastopenq != NULL) {
> spin_lock_bh(&queue->fastopenq->lock);
> if (tcp_rsk(req)->listener) {
> /* We are still waiting for the final ACK from 3WHS
> @@ -775,7 +775,7 @@ void inet_csk_listen_stop(struct sock *sk)
>
> percpu_counter_inc(sk->sk_prot->orphan_count);
>
> - if (sk->sk_type == SOCK_STREAM && tcp_rsk(req)->listener) {
> + if (sk->sk_protocol == IPPROTO_TCP && tcp_rsk(req)->listener) {
> BUG_ON(tcp_sk(child)->fastopen_rsk != req);
> BUG_ON(sk != tcp_rsk(req)->listener);
>
>
>
Thanks, Eric.
Acked-by: H.K. Jerry Chu <hkchu@google.com>
^ permalink raw reply
* Re: [PATCH net-next] tcp: fix TFO regression
From: Neal Cardwell @ 2012-09-06 18:15 UTC (permalink / raw)
To: Eric Dumazet
Cc: Fengguang Wu, David Miller, Marc Kleine-Budde, H.K. Jerry Chu,
Eric W. Biederman, networking, linux-can
In-Reply-To: <1346954833.2484.50.camel@edumazet-glaptop>
On Thu, Sep 6, 2012 at 2:07 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Fengguang Wu reported various panics and bisected to commit
> 8336886f786fdac (tcp: TCP Fast Open Server - support TFO listeners)
>
> Fix this by making sure socket is a TCP socket before accessing TFO data
> structures.
>
...
>
> Reported-by: Fengguang Wu <wfg@linux.intel.com>
> Tested-by: Fengguang Wu <wfg@linux.intel.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: "H.K. Jerry Chu" <hkchu@google.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
neal
^ 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