* Re: [PATCH v4 09/13] net: ethernet: ti: cpts: rework initialization/deinitialization
From: Richard Cochran @ 2016-12-06 18:04 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev, Mugunthan V N, Sekhar Nori, linux-kernel,
linux-omap, devicetree, Murali Karicheri, Wingman Kwok,
Thomas Gleixner
In-Reply-To: <c0d6d670-cf44-c113-3443-3b5209e68ee2@ti.com>
On Tue, Dec 06, 2016 at 11:49:14AM -0600, Grygorii Strashko wrote:
> But we do reset whole cpsw :( and that's required to support PM use cases as
> suspend/resume.
The code is resetting the clock unconditionally on ifup/down. That
sucks. If you reset the clock *only* after resume, that would be ok.
> There are also PM requirement to shutdown cpsw in case all interfaces are down.
Well, those requirements are not too smart. As an end user, I expect
that ifdown/up does not change the time. There isn't any reason to
reset the clock in this case.
> More over, there are requirement to minimize cpsw power consumption in case all links are
> disconnected (and cpts is special case here).
>
> So, at least resetting of the timecounter still required.
Only if you follow that poorly conceived PM plan. Anyhow, I agree
that it isn't the task of your present series to fix that.
> Ok. I'll try to optimize it following your directions.
What I would like to see is: initialize the cyclecounter fields
exactly once.
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH 4/6] net: ethernet: ti: cpts: add ptp pps support
From: Richard Cochran @ 2016-12-06 18:08 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev-u79uwXL29TY76Z2rM5mHXA, Mugunthan V N,
Sekhar Nori, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA, Murali Karicheri, Wingman Kwok
In-Reply-To: <20161130100519.GD28680-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
On Wed, Nov 30, 2016 at 11:05:19AM +0100, Richard Cochran wrote:
> Can you adjust the frequency of the keystone devices in hardware? If
> so, then please implement it, and just disable PPS for the CPSW.
>
> The only reason I used the timecounter for frequency adjustment was
> because the am335x HW is broken. But this shouldn't hold back other
> newer HW without the same silicon flaws.
I am talking here about the ADPLLLJ units. Are they usable on the
keystone?
If so, please implement the frequency adjustment with them.
Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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: [RESEND][PATCH v4] cgroup: Use CAP_SYS_RESOURCE to allow a process to migrate other tasks between cgroups
From: Tejun Heo @ 2016-12-06 18:12 UTC (permalink / raw)
To: Andy Lutomirski
Cc: John Stultz, Alexei Starovoitov, Andy Lutomirski,
Mickaël Salaün, Daniel Mack, David S. Miller,
kafai-b10kYP2dOMg, Florian Westphal, Harald Hoyer,
Network Development, Sargun Dhillon, Pablo Neira Ayuso, lkml,
Li Zefan, Jonathan Corbet, open list:CONTROL GROUP (CGROUP),
Android Kernel Team, Rom Lemarchand, Colin Cross
In-Reply-To: <CALCETrV3Yrq1hRcoGCTU_z-T-6hmq-gY-HytR2HGkvnRK-W1SQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hello,
On Tue, Dec 06, 2016 at 09:01:17AM -0800, Andy Lutomirski wrote:
> How would one be granted the right to move processes around in one's
> own subtree?
Through expicit delegation - chowning of the directory and
cgroup.procs file.
> Are you imagining that, if you're in /a/b and you want to move a
> process that's currently in /a/b/c to /a/b/d then you're allowed to
> because the target process is in your tree? If so, I doubt this has
> the security properties you want -- namely, if you can cooperate with
> anyone in /, even if they're unprivileged, you can break it.
Delegation is an explicit operation and reflected in the ownership of
the subdirectories and cgroup interface files in them. The
subhierarchy containment is achieved by requiring the user who's
trying to migrate a process to have write perm on cgroup.procs on the
common ancestor of the source and target in addition to the target.
So, a user who has a subhierarchy delegated to it can move processes
around inside but not out of or into it. Also, if there are multiple
delegated disjoint subhierarchies, processes can't be moved across
them by the delegatee either.
Thanks.
--
tejun
^ permalink raw reply
* Re: [RESEND][PATCH v4] cgroup: Use CAP_SYS_RESOURCE to allow a process to migrate other tasks between cgroups
From: Andy Lutomirski @ 2016-12-06 18:13 UTC (permalink / raw)
To: Tejun Heo
Cc: John Stultz, Alexei Starovoitov, Andy Lutomirski,
Mickaël Salaün, Daniel Mack, David S. Miller, kafai,
Florian Westphal, Harald Hoyer, Network Development,
Sargun Dhillon, Pablo Neira Ayuso, lkml, Li Zefan,
Jonathan Corbet, open list:CONTROL GROUP (CGROUP),
Android Kernel Team, Rom Lemarchand, Colin Cross
In-Reply-To: <20161206181221.GA2625@mtj.duckdns.org>
On Tue, Dec 6, 2016 at 10:12 AM, Tejun Heo <tj@kernel.org> wrote:
> Hello,
>
> On Tue, Dec 06, 2016 at 09:01:17AM -0800, Andy Lutomirski wrote:
>> How would one be granted the right to move processes around in one's
>> own subtree?
>
> Through expicit delegation - chowning of the directory and
> cgroup.procs file.
>
>> Are you imagining that, if you're in /a/b and you want to move a
>> process that's currently in /a/b/c to /a/b/d then you're allowed to
>> because the target process is in your tree? If so, I doubt this has
>> the security properties you want -- namely, if you can cooperate with
>> anyone in /, even if they're unprivileged, you can break it.
>
> Delegation is an explicit operation and reflected in the ownership of
> the subdirectories and cgroup interface files in them. The
> subhierarchy containment is achieved by requiring the user who's
> trying to migrate a process to have write perm on cgroup.procs on the
> common ancestor of the source and target in addition to the target.
OK, I see what you're doing. That's interesting.
^ permalink raw reply
* [PATCH v3 0/7] irda: w83977af_ir: Neatening
From: Joe Perches @ 2016-12-06 18:15 UTC (permalink / raw)
To: netdev; +Cc: Arnd Bergmann, Sergei Shtylyov, Samuel Ortiz, linux-kernel
Originally on top of Arnd's overly long udelay patches because I
noticed a misindented block. That's now already fixed along with some
other whitespace problems. These patches are the remainder style
issues from my original series.
Even though I haven't turned on the netwinder in a box in the
garage in who knows how long, if this device is still used somewhere,
might as well neaten the code too.
Joe Perches (7):
irda: w83977af_ir: Whitespace neatening
irda: w83977af_ir: More whitespace neatening
irda: w83977af_ir: Remove and add blank lines
irda: w83977af_ir: Neaten pointer comparisons
irda: w83977af_ir: Use the common brace style
irda: w83977af_ir: Parenthesis alignment
irda: w83977af_ir: Neaten logging
drivers/net/irda/w83977af_ir.c | 323 ++++++++++++++++++++---------------------
1 file changed, 160 insertions(+), 163 deletions(-)
--
2.10.0.rc2.1.g053435c
^ permalink raw reply
* [PATCH v3 1/7] irda: w83977af_ir: Whitespace neatening
From: Joe Perches @ 2016-12-06 18:16 UTC (permalink / raw)
To: Samuel Ortiz; +Cc: Arnd Bergmann, Sergei Shtylyov, netdev, linux-kernel
In-Reply-To: <cover.1481047792.git.joe@perches.com>
Remove leading and trailing whitespace.
git diff -w shows no differences.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/irda/w83977af_ir.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/irda/w83977af_ir.c b/drivers/net/irda/w83977af_ir.c
index 96745888a4fc..d4b1d74d3081 100644
--- a/drivers/net/irda/w83977af_ir.c
+++ b/drivers/net/irda/w83977af_ir.c
@@ -263,7 +263,7 @@ static int w83977af_close(struct w83977af_ir *self)
{
int iobase;
- iobase = self->io.fir_base;
+ iobase = self->io.fir_base;
#ifdef CONFIG_USE_W977_PNP
/* enter PnP configuration mode */
--
2.10.0.rc2.1.g053435c
^ permalink raw reply related
* [PATCH v3 2/7] irda: w83977af_ir: More whitespace neatening
From: Joe Perches @ 2016-12-06 18:16 UTC (permalink / raw)
To: Samuel Ortiz; +Cc: Arnd Bergmann, Sergei Shtylyov, netdev, linux-kernel
In-Reply-To: <cover.1481047792.git.joe@perches.com>
Add spaces around operators.
git diff -w shows no differences.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/irda/w83977af_ir.c | 230 ++++++++++++++++++++---------------------
1 file changed, 115 insertions(+), 115 deletions(-)
diff --git a/drivers/net/irda/w83977af_ir.c b/drivers/net/irda/w83977af_ir.c
index d4b1d74d3081..166dd4e524fc 100644
--- a/drivers/net/irda/w83977af_ir.c
+++ b/drivers/net/irda/w83977af_ir.c
@@ -110,7 +110,7 @@ static int __init w83977af_init(void)
{
int i;
- for (i=0; i < ARRAY_SIZE(dev_self) && io[i] < 2000; i++) {
+ for (i = 0; i < ARRAY_SIZE(dev_self) && io[i] < 2000; i++) {
if (w83977af_open(i, io[i], irq[i], dma[i]) == 0)
return 0;
}
@@ -127,7 +127,7 @@ static void __exit w83977af_cleanup(void)
{
int i;
- for (i=0; i < ARRAY_SIZE(dev_self); i++) {
+ for (i = 0; i < ARRAY_SIZE(dev_self); i++) {
if (dev_self[i])
w83977af_close(dev_self[i]);
}
@@ -156,7 +156,7 @@ static int w83977af_open(int i, unsigned int iobase, unsigned int irq,
/* Lock the port that we need */
if (!request_region(iobase, CHIP_IO_EXTENT, driver_name)) {
pr_debug("%s(), can't get iobase of 0x%03x\n",
- __func__ , iobase);
+ __func__, iobase);
return -ENODEV;
}
@@ -169,7 +169,7 @@ static int w83977af_open(int i, unsigned int iobase, unsigned int irq,
*/
dev = alloc_irdadev(sizeof(struct w83977af_ir));
if (dev == NULL) {
- printk( KERN_ERR "IrDA: Can't allocate memory for "
+ printk(KERN_ERR "IrDA: Can't allocate memory for "
"IrDA control block!\n");
err = -ENOMEM;
goto err_out;
@@ -192,8 +192,8 @@ static int w83977af_open(int i, unsigned int iobase, unsigned int irq,
/* The only value we must override it the baudrate */
/* FIXME: The HP HDLS-1100 does not support 1152000! */
- self->qos.baud_rate.bits = IR_9600|IR_19200|IR_38400|IR_57600|
- IR_115200|IR_576000|IR_1152000|(IR_4000000 << 8);
+ self->qos.baud_rate.bits = IR_9600 | IR_19200 | IR_38400 | IR_57600 |
+ IR_115200 | IR_576000 | IR_1152000 | (IR_4000000 << 8);
/* The HP HDLS-1100 needs 1 ms according to the specs */
self->qos.min_turn_time.bits = qos_mtt_bits;
@@ -282,7 +282,7 @@ static int w83977af_close(struct w83977af_ir *self)
/* Release the PORT that this driver is using */
pr_debug("%s(), Releasing Region %03x\n",
- __func__ , self->io.fir_base);
+ __func__, self->io.fir_base);
release_region(self->io.fir_base, self->io.fir_ext);
if (self->tx_buff.head)
@@ -303,7 +303,7 @@ static int w83977af_probe(int iobase, int irq, int dma)
int version;
int i;
- for (i=0; i < 2; i++) {
+ for (i = 0; i < 2; i++) {
#ifdef CONFIG_USE_W977_PNP
/* Enter PnP configuration mode */
w977_efm_enter(efbase[i]);
@@ -317,7 +317,7 @@ static int w83977af_probe(int iobase, int irq, int dma)
w977_write_reg(0x70, irq, efbase[i]);
#ifdef CONFIG_ARCH_NETWINDER
/* Netwinder uses 1 higher than Linux */
- w977_write_reg(0x74, dma+1, efbase[i]);
+ w977_write_reg(0x74, dma + 1, efbase[i]);
#else
w977_write_reg(0x74, dma, efbase[i]);
#endif /* CONFIG_ARCH_NETWINDER */
@@ -333,23 +333,23 @@ static int w83977af_probe(int iobase, int irq, int dma)
#endif /* CONFIG_USE_W977_PNP */
/* Disable Advanced mode */
switch_bank(iobase, SET2);
- outb(iobase+2, 0x00);
+ outb(iobase + 2, 0x00);
/* Turn on UART (global) interrupts */
switch_bank(iobase, SET0);
- outb(HCR_EN_IRQ, iobase+HCR);
+ outb(HCR_EN_IRQ, iobase + HCR);
/* Switch to advanced mode */
switch_bank(iobase, SET2);
- outb(inb(iobase+ADCR1) | ADCR1_ADV_SL, iobase+ADCR1);
+ outb(inb(iobase + ADCR1) | ADCR1_ADV_SL, iobase + ADCR1);
/* Set default IR-mode */
switch_bank(iobase, SET0);
- outb(HCR_SIR, iobase+HCR);
+ outb(HCR_SIR, iobase + HCR);
/* Read the Advanced IR ID */
switch_bank(iobase, SET3);
- version = inb(iobase+AUID);
+ version = inb(iobase + AUID);
/* Should be 0x1? */
if (0x10 == (version & 0xf0)) {
@@ -357,17 +357,17 @@ static int w83977af_probe(int iobase, int irq, int dma)
/* Set FIFO size to 32 */
switch_bank(iobase, SET2);
- outb(ADCR2_RXFS32|ADCR2_TXFS32, iobase+ADCR2);
+ outb(ADCR2_RXFS32 | ADCR2_TXFS32, iobase + ADCR2);
/* Set FIFO threshold to TX17, RX16 */
switch_bank(iobase, SET0);
- outb(UFR_RXTL|UFR_TXTL|UFR_TXF_RST|UFR_RXF_RST|
- UFR_EN_FIFO,iobase+UFR);
+ outb(UFR_RXTL | UFR_TXTL | UFR_TXF_RST | UFR_RXF_RST |
+ UFR_EN_FIFO, iobase + UFR);
/* Receiver frame length */
switch_bank(iobase, SET4);
- outb(2048 & 0xff, iobase+6);
- outb((2048 >> 8) & 0x1f, iobase+7);
+ outb(2048 & 0xff, iobase + 6);
+ outb((2048 >> 8) & 0x1f, iobase + 7);
/*
* Init HP HSDL-1100 transceiver.
@@ -382,7 +382,7 @@ static int w83977af_probe(int iobase, int irq, int dma)
* CIRRX pin 40 connected to pin 37
*/
switch_bank(iobase, SET7);
- outb(0x40, iobase+7);
+ outb(0x40, iobase + 7);
net_info_ratelimited("W83977AF (IR) driver loaded. Version: 0x%02x\n",
version);
@@ -408,22 +408,22 @@ static void w83977af_change_speed(struct w83977af_ir *self, __u32 speed)
self->io.speed = speed;
/* Save current bank */
- set = inb(iobase+SSR);
+ set = inb(iobase + SSR);
/* Disable interrupts */
switch_bank(iobase, SET0);
- outb(0, iobase+ICR);
+ outb(0, iobase + ICR);
/* Select Set 2 */
switch_bank(iobase, SET2);
- outb(0x00, iobase+ABHL);
+ outb(0x00, iobase + ABHL);
switch (speed) {
- case 9600: outb(0x0c, iobase+ABLL); break;
- case 19200: outb(0x06, iobase+ABLL); break;
- case 38400: outb(0x03, iobase+ABLL); break;
- case 57600: outb(0x02, iobase+ABLL); break;
- case 115200: outb(0x01, iobase+ABLL); break;
+ case 9600: outb(0x0c, iobase + ABLL); break;
+ case 19200: outb(0x06, iobase + ABLL); break;
+ case 38400: outb(0x03, iobase + ABLL); break;
+ case 57600: outb(0x02, iobase + ABLL); break;
+ case 115200: outb(0x01, iobase + ABLL); break;
case 576000:
ir_mode = HCR_MIR_576;
pr_debug("%s(), handling baud of 576000\n", __func__);
@@ -438,36 +438,36 @@ static void w83977af_change_speed(struct w83977af_ir *self, __u32 speed)
break;
default:
ir_mode = HCR_FIR;
- pr_debug("%s(), unknown baud rate of %d\n", __func__ , speed);
+ pr_debug("%s(), unknown baud rate of %d\n", __func__, speed);
break;
}
/* Set speed mode */
switch_bank(iobase, SET0);
- outb(ir_mode, iobase+HCR);
+ outb(ir_mode, iobase + HCR);
/* set FIFO size to 32 */
switch_bank(iobase, SET2);
- outb(ADCR2_RXFS32|ADCR2_TXFS32, iobase+ADCR2);
+ outb(ADCR2_RXFS32 | ADCR2_TXFS32, iobase + ADCR2);
/* set FIFO threshold to TX17, RX16 */
switch_bank(iobase, SET0);
- outb(0x00, iobase+UFR); /* Reset */
- outb(UFR_EN_FIFO, iobase+UFR); /* First we must enable FIFO */
- outb(0xa7, iobase+UFR);
+ outb(0x00, iobase + UFR); /* Reset */
+ outb(UFR_EN_FIFO, iobase + UFR); /* First we must enable FIFO */
+ outb(0xa7, iobase + UFR);
netif_wake_queue(self->netdev);
/* Enable some interrupts so we can receive frames */
switch_bank(iobase, SET0);
if (speed > PIO_MAX_SPEED) {
- outb(ICR_EFSFI, iobase+ICR);
+ outb(ICR_EFSFI, iobase + ICR);
w83977af_dma_receive(self);
} else
- outb(ICR_ERBRI, iobase+ICR);
+ outb(ICR_ERBRI, iobase + ICR);
/* Restore SSR */
- outb(set, iobase+SSR);
+ outb(set, iobase + SSR);
}
/*
@@ -489,7 +489,7 @@ static netdev_tx_t w83977af_hard_xmit(struct sk_buff *skb,
iobase = self->io.fir_base;
- pr_debug("%s(%ld), skb->len=%d\n", __func__ , jiffies,
+ pr_debug("%s(%ld), skb->len=%d\n", __func__, jiffies,
(int)skb->len);
/* Lock transmit buffer */
@@ -508,7 +508,7 @@ static netdev_tx_t w83977af_hard_xmit(struct sk_buff *skb,
}
/* Save current set */
- set = inb(iobase+SSR);
+ set = inb(iobase + SSR);
/* Decide if we should use PIO or DMA transfer */
if (self->io.speed > PIO_MAX_SPEED) {
@@ -517,15 +517,15 @@ static netdev_tx_t w83977af_hard_xmit(struct sk_buff *skb,
self->tx_buff.len = skb->len;
mtt = irda_get_mtt(skb);
- pr_debug("%s(%ld), mtt=%d\n", __func__ , jiffies, mtt);
+ pr_debug("%s(%ld), mtt=%d\n", __func__, jiffies, mtt);
if (mtt > 1000)
- mdelay(mtt/1000);
+ mdelay(mtt / 1000);
else if (mtt)
udelay(mtt);
/* Enable DMA interrupt */
switch_bank(iobase, SET0);
- outb(ICR_EDMAI, iobase+ICR);
+ outb(ICR_EDMAI, iobase + ICR);
w83977af_dma_write(self, iobase);
} else {
self->tx_buff.data = self->tx_buff.head;
@@ -534,12 +534,12 @@ static netdev_tx_t w83977af_hard_xmit(struct sk_buff *skb,
/* Add interrupt on tx low level (will fire immediately) */
switch_bank(iobase, SET0);
- outb(ICR_ETXTHI, iobase+ICR);
+ outb(ICR_ETXTHI, iobase + ICR);
}
dev_kfree_skb(skb);
/* Restore set register */
- outb(set, iobase+SSR);
+ outb(set, iobase + SSR);
return NETDEV_TX_OK;
}
@@ -553,28 +553,28 @@ static netdev_tx_t w83977af_hard_xmit(struct sk_buff *skb,
static void w83977af_dma_write(struct w83977af_ir *self, int iobase)
{
__u8 set;
- pr_debug("%s(), len=%d\n", __func__ , self->tx_buff.len);
+ pr_debug("%s(), len=%d\n", __func__, self->tx_buff.len);
/* Save current set */
- set = inb(iobase+SSR);
+ set = inb(iobase + SSR);
/* Disable DMA */
switch_bank(iobase, SET0);
- outb(inb(iobase+HCR) & ~HCR_EN_DMA, iobase+HCR);
+ outb(inb(iobase + HCR) & ~HCR_EN_DMA, iobase + HCR);
/* Choose transmit DMA channel */
switch_bank(iobase, SET2);
- outb(ADCR1_D_CHSW|/*ADCR1_DMA_F|*/ADCR1_ADV_SL, iobase+ADCR1);
+ outb(ADCR1_D_CHSW | /*ADCR1_DMA_F|*/ADCR1_ADV_SL, iobase + ADCR1);
irda_setup_dma(self->io.dma, self->tx_buff_dma, self->tx_buff.len,
DMA_MODE_WRITE);
self->io.direction = IO_XMIT;
/* Enable DMA */
switch_bank(iobase, SET0);
- outb(inb(iobase+HCR) | HCR_EN_DMA | HCR_TX_WT, iobase+HCR);
+ outb(inb(iobase + HCR) | HCR_EN_DMA | HCR_TX_WT, iobase + HCR);
/* Restore set register */
- outb(set, iobase+SSR);
+ outb(set, iobase + SSR);
}
/*
@@ -589,28 +589,28 @@ static int w83977af_pio_write(int iobase, __u8 *buf, int len, int fifo_size)
__u8 set;
/* Save current bank */
- set = inb(iobase+SSR);
+ set = inb(iobase + SSR);
switch_bank(iobase, SET0);
- if (!(inb_p(iobase+USR) & USR_TSRE)) {
+ if (!(inb_p(iobase + USR) & USR_TSRE)) {
pr_debug("%s(), warning, FIFO not empty yet!\n", __func__);
fifo_size -= 17;
pr_debug("%s(), %d bytes left in tx fifo\n",
- __func__ , fifo_size);
+ __func__, fifo_size);
}
/* Fill FIFO with current frame */
while ((fifo_size-- > 0) && (actual < len)) {
/* Transmit next byte */
- outb(buf[actual++], iobase+TBR);
+ outb(buf[actual++], iobase + TBR);
}
pr_debug("%s(), fifo_size %d ; %d sent of %d\n",
- __func__ , fifo_size, actual, len);
+ __func__, fifo_size, actual, len);
/* Restore bank */
- outb(set, iobase+SSR);
+ outb(set, iobase + SSR);
return actual;
}
@@ -627,28 +627,28 @@ static void w83977af_dma_xmit_complete(struct w83977af_ir *self)
int iobase;
__u8 set;
- pr_debug("%s(%ld)\n", __func__ , jiffies);
+ pr_debug("%s(%ld)\n", __func__, jiffies);
IRDA_ASSERT(self != NULL, return;);
iobase = self->io.fir_base;
/* Save current set */
- set = inb(iobase+SSR);
+ set = inb(iobase + SSR);
/* Disable DMA */
switch_bank(iobase, SET0);
- outb(inb(iobase+HCR) & ~HCR_EN_DMA, iobase+HCR);
+ outb(inb(iobase + HCR) & ~HCR_EN_DMA, iobase + HCR);
/* Check for underrun! */
- if (inb(iobase+AUDR) & AUDR_UNDR) {
+ if (inb(iobase + AUDR) & AUDR_UNDR) {
pr_debug("%s(), Transmit underrun!\n", __func__);
self->netdev->stats.tx_errors++;
self->netdev->stats.tx_fifo_errors++;
/* Clear bit, by writing 1 to it */
- outb(AUDR_UNDR, iobase+AUDR);
+ outb(AUDR_UNDR, iobase + AUDR);
} else
self->netdev->stats.tx_packets++;
@@ -663,7 +663,7 @@ static void w83977af_dma_xmit_complete(struct w83977af_ir *self)
netif_wake_queue(self->netdev);
/* Restore set */
- outb(set, iobase+SSR);
+ outb(set, iobase + SSR);
}
/*
@@ -685,19 +685,19 @@ static int w83977af_dma_receive(struct w83977af_ir *self)
pr_debug("%s\n", __func__);
- iobase= self->io.fir_base;
+ iobase = self->io.fir_base;
/* Save current set */
- set = inb(iobase+SSR);
+ set = inb(iobase + SSR);
/* Disable DMA */
switch_bank(iobase, SET0);
- outb(inb(iobase+HCR) & ~HCR_EN_DMA, iobase+HCR);
+ outb(inb(iobase + HCR) & ~HCR_EN_DMA, iobase + HCR);
/* Choose DMA Rx, DMA Fairness, and Advanced mode */
switch_bank(iobase, SET2);
- outb((inb(iobase+ADCR1) & ~ADCR1_D_CHSW)/*|ADCR1_DMA_F*/|ADCR1_ADV_SL,
- iobase+ADCR1);
+ outb((inb(iobase + ADCR1) & ~ADCR1_D_CHSW)/*|ADCR1_DMA_F*/ | ADCR1_ADV_SL,
+ iobase + ADCR1);
self->io.direction = IO_RECV;
self->rx_buff.data = self->rx_buff.head;
@@ -720,21 +720,21 @@ static int w83977af_dma_receive(struct w83977af_ir *self)
* be finished transmitting yet
*/
switch_bank(iobase, SET0);
- outb(UFR_RXTL|UFR_TXTL|UFR_RXF_RST|UFR_EN_FIFO, iobase+UFR);
+ outb(UFR_RXTL | UFR_TXTL | UFR_RXF_RST | UFR_EN_FIFO, iobase + UFR);
self->st_fifo.len = self->st_fifo.tail = self->st_fifo.head = 0;
/* Enable DMA */
switch_bank(iobase, SET0);
#ifdef CONFIG_ARCH_NETWINDER
- hcr = inb(iobase+HCR);
- outb(hcr | HCR_EN_DMA, iobase+HCR);
+ hcr = inb(iobase + HCR);
+ outb(hcr | HCR_EN_DMA, iobase + HCR);
enable_dma(self->io.dma);
spin_unlock_irqrestore(&self->lock, flags);
#else
- outb(inb(iobase+HCR) | HCR_EN_DMA, iobase+HCR);
+ outb(inb(iobase + HCR) | HCR_EN_DMA, iobase + HCR);
#endif
/* Restore set */
- outb(set, iobase+SSR);
+ outb(set, iobase + SSR);
return 0;
}
@@ -761,17 +761,17 @@ static int w83977af_dma_receive_complete(struct w83977af_ir *self)
iobase = self->io.fir_base;
/* Save current set */
- set = inb(iobase+SSR);
+ set = inb(iobase + SSR);
iobase = self->io.fir_base;
/* Read status FIFO */
switch_bank(iobase, SET5);
- while ((status = inb(iobase+FS_FO)) & FS_FO_FSFDR) {
+ while ((status = inb(iobase + FS_FO)) & FS_FO_FSFDR) {
st_fifo->entries[st_fifo->tail].status = status;
- st_fifo->entries[st_fifo->tail].len = inb(iobase+RFLFL);
- st_fifo->entries[st_fifo->tail].len |= inb(iobase+RFLFH) << 8;
+ st_fifo->entries[st_fifo->tail].len = inb(iobase + RFLFL);
+ st_fifo->entries[st_fifo->tail].len |= inb(iobase + RFLFH) << 8;
st_fifo->tail++;
st_fifo->len++;
@@ -814,16 +814,16 @@ static int w83977af_dma_receive_complete(struct w83977af_ir *self)
} else {
/* Check if we have transferred all data to memory */
switch_bank(iobase, SET0);
- if (inb(iobase+USR) & USR_RDR) {
+ if (inb(iobase + USR) & USR_RDR) {
udelay(80); /* Should be enough!? */
}
- skb = dev_alloc_skb(len+1);
+ skb = dev_alloc_skb(len + 1);
if (skb == NULL) {
printk(KERN_INFO
"%s(), memory squeeze, dropping frame.\n", __func__);
/* Restore set register */
- outb(set, iobase+SSR);
+ outb(set, iobase + SSR);
return FALSE;
}
@@ -833,12 +833,12 @@ static int w83977af_dma_receive_complete(struct w83977af_ir *self)
/* Copy frame without CRC */
if (self->io.speed < 4000000) {
- skb_put(skb, len-2);
+ skb_put(skb, len - 2);
skb_copy_to_linear_data(skb,
self->rx_buff.data,
len - 2);
} else {
- skb_put(skb, len-4);
+ skb_put(skb, len - 4);
skb_copy_to_linear_data(skb,
self->rx_buff.data,
len - 4);
@@ -855,7 +855,7 @@ static int w83977af_dma_receive_complete(struct w83977af_ir *self)
}
}
/* Restore set register */
- outb(set, iobase+SSR);
+ outb(set, iobase + SSR);
return TRUE;
}
@@ -877,10 +877,10 @@ static void w83977af_pio_receive(struct w83977af_ir *self)
/* Receive all characters in Rx FIFO */
do {
- byte = inb(iobase+RBR);
+ byte = inb(iobase + RBR);
async_unwrap_char(self->netdev, &self->netdev->stats, &self->rx_buff,
byte);
- } while (inb(iobase+USR) & USR_RDR); /* Data available */
+ } while (inb(iobase + USR) & USR_RDR); /* Data available */
}
/*
@@ -896,7 +896,7 @@ static __u8 w83977af_sir_interrupt(struct w83977af_ir *self, int isr)
__u8 set;
int iobase;
- pr_debug("%s(), isr=%#x\n", __func__ , isr);
+ pr_debug("%s(), isr=%#x\n", __func__, isr);
iobase = self->io.fir_base;
/* Transmit FIFO low on data */
@@ -916,10 +916,10 @@ static __u8 w83977af_sir_interrupt(struct w83977af_ir *self, int isr)
if (self->tx_buff.len > 0) {
new_icr |= ICR_ETXTHI;
} else {
- set = inb(iobase+SSR);
+ set = inb(iobase + SSR);
switch_bank(iobase, SET0);
- outb(AUDR_SFEND, iobase+AUDR);
- outb(set, iobase+SSR);
+ outb(AUDR_SFEND, iobase + AUDR);
+ outb(set, iobase + SSR);
self->netdev->stats.tx_packets++;
@@ -965,10 +965,10 @@ static __u8 w83977af_fir_interrupt(struct w83977af_ir *self, int isr)
int iobase;
iobase = self->io.fir_base;
- set = inb(iobase+SSR);
+ set = inb(iobase + SSR);
/* End of frame detected in FIFO */
- if (isr & (ISR_FEND_I|ISR_FSF_I)) {
+ if (isr & (ISR_FEND_I | ISR_FSF_I)) {
if (w83977af_dma_receive_complete(self)) {
/* Wait for next status FIFO interrupt */
@@ -978,11 +978,11 @@ static __u8 w83977af_fir_interrupt(struct w83977af_ir *self, int isr)
/* Set timer value, resolution 1 ms */
switch_bank(iobase, SET4);
- outb(0x01, iobase+TMRL); /* 1 ms */
- outb(0x00, iobase+TMRH);
+ outb(0x01, iobase + TMRL); /* 1 ms */
+ outb(0x00, iobase + TMRH);
/* Start timer */
- outb(IR_MSL_EN_TMR, iobase+IR_MSL);
+ outb(IR_MSL_EN_TMR, iobase + IR_MSL);
new_icr |= ICR_ETMRI;
}
@@ -991,7 +991,7 @@ static __u8 w83977af_fir_interrupt(struct w83977af_ir *self, int isr)
if (isr & ISR_TMR_I) {
/* Disable timer */
switch_bank(iobase, SET4);
- outb(0, iobase+IR_MSL);
+ outb(0, iobase + IR_MSL);
/* Clear timer event */
/* switch_bank(iobase, SET0); */
@@ -1026,7 +1026,7 @@ static __u8 w83977af_fir_interrupt(struct w83977af_ir *self, int isr)
}
/* Restore set */
- outb(set, iobase+SSR);
+ outb(set, iobase + SSR);
return new_icr;
}
@@ -1049,24 +1049,24 @@ static irqreturn_t w83977af_interrupt(int irq, void *dev_id)
iobase = self->io.fir_base;
/* Save current bank */
- set = inb(iobase+SSR);
+ set = inb(iobase + SSR);
switch_bank(iobase, SET0);
- icr = inb(iobase+ICR);
- isr = inb(iobase+ISR) & icr; /* Mask out the interesting ones */
+ icr = inb(iobase + ICR);
+ isr = inb(iobase + ISR) & icr; /* Mask out the interesting ones */
- outb(0, iobase+ICR); /* Disable interrupts */
+ outb(0, iobase + ICR); /* Disable interrupts */
if (isr) {
/* Dispatch interrupt handler for the current speed */
- if (self->io.speed > PIO_MAX_SPEED )
+ if (self->io.speed > PIO_MAX_SPEED)
icr = w83977af_fir_interrupt(self, isr);
else
icr = w83977af_sir_interrupt(self, isr);
}
- outb(icr, iobase+ICR); /* Restore (new) interrupts */
- outb(set, iobase+SSR); /* Restore bank register */
+ outb(icr, iobase + ICR); /* Restore (new) interrupts */
+ outb(set, iobase + SSR); /* Restore bank register */
return IRQ_RETVAL(isr);
}
@@ -1088,13 +1088,13 @@ static int w83977af_is_receiving(struct w83977af_ir *self)
iobase = self->io.fir_base;
/* Check if rx FIFO is not empty */
- set = inb(iobase+SSR);
+ set = inb(iobase + SSR);
switch_bank(iobase, SET2);
- if ((inb(iobase+RXFDTH) & 0x3f) != 0) {
+ if ((inb(iobase + RXFDTH) & 0x3f) != 0) {
/* We are receiving something */
status = TRUE;
}
- outb(set, iobase+SSR);
+ outb(set, iobase + SSR);
} else
status = (self->rx_buff.state != OUTSIDE_FRAME);
@@ -1123,7 +1123,7 @@ static int w83977af_net_open(struct net_device *dev)
iobase = self->io.fir_base;
if (request_irq(self->io.irq, w83977af_interrupt, 0, dev->name,
- (void *) dev)) {
+ (void *)dev)) {
return -EAGAIN;
}
/*
@@ -1136,18 +1136,18 @@ static int w83977af_net_open(struct net_device *dev)
}
/* Save current set */
- set = inb(iobase+SSR);
+ set = inb(iobase + SSR);
/* Enable some interrupts so we can receive frames again */
switch_bank(iobase, SET0);
if (self->io.speed > 115200) {
- outb(ICR_EFSFI, iobase+ICR);
+ outb(ICR_EFSFI, iobase + ICR);
w83977af_dma_receive(self);
} else
- outb(ICR_ERBRI, iobase+ICR);
+ outb(ICR_ERBRI, iobase + ICR);
/* Restore bank register */
- outb(set, iobase+SSR);
+ outb(set, iobase + SSR);
/* Ready to play! */
netif_start_queue(dev);
@@ -1195,17 +1195,17 @@ static int w83977af_net_close(struct net_device *dev)
disable_dma(self->io.dma);
/* Save current set */
- set = inb(iobase+SSR);
+ set = inb(iobase + SSR);
/* Disable interrupts */
switch_bank(iobase, SET0);
- outb(0, iobase+ICR);
+ outb(0, iobase + ICR);
free_irq(self->io.irq, dev);
free_dma(self->io.dma);
/* Restore bank register */
- outb(set, iobase+SSR);
+ outb(set, iobase + SSR);
return 0;
}
@@ -1218,7 +1218,7 @@ static int w83977af_net_close(struct net_device *dev)
*/
static int w83977af_net_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
{
- struct if_irda_req *irq = (struct if_irda_req *) rq;
+ struct if_irda_req *irq = (struct if_irda_req *)rq;
struct w83977af_ir *self;
unsigned long flags;
int ret = 0;
@@ -1229,7 +1229,7 @@ static int w83977af_net_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
IRDA_ASSERT(self != NULL, return -1;);
- pr_debug("%s(), %s, (cmd=0x%X)\n", __func__ , dev->name, cmd);
+ pr_debug("%s(), %s, (cmd=0x%X)\n", __func__, dev->name, cmd);
spin_lock_irqsave(&self->lock, flags);
--
2.10.0.rc2.1.g053435c
^ permalink raw reply related
* [PATCH v3 3/7] irda: w83977af_ir: Remove and add blank lines
From: Joe Perches @ 2016-12-06 18:16 UTC (permalink / raw)
To: Samuel Ortiz; +Cc: Arnd Bergmann, Sergei Shtylyov, netdev, linux-kernel
In-Reply-To: <cover.1481047792.git.joe@perches.com>
Use a more typical vertical spacing style.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/irda/w83977af_ir.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/net/irda/w83977af_ir.c b/drivers/net/irda/w83977af_ir.c
index 166dd4e524fc..e30cbf005320 100644
--- a/drivers/net/irda/w83977af_ir.c
+++ b/drivers/net/irda/w83977af_ir.c
@@ -178,7 +178,6 @@ static int w83977af_open(int i, unsigned int iobase, unsigned int irq,
self = netdev_priv(dev);
spin_lock_init(&self->lock);
-
/* Initialize IO */
self->io.fir_base = iobase;
self->io.irq = irq;
@@ -553,6 +552,7 @@ static netdev_tx_t w83977af_hard_xmit(struct sk_buff *skb,
static void w83977af_dma_write(struct w83977af_ir *self, int iobase)
{
__u8 set;
+
pr_debug("%s(), len=%d\n", __func__, self->tx_buff.len);
/* Save current set */
@@ -652,7 +652,6 @@ static void w83977af_dma_xmit_complete(struct w83977af_ir *self)
} else
self->netdev->stats.tx_packets++;
-
if (self->new_speed) {
w83977af_change_speed(self, self->new_speed);
self->new_speed = 0;
@@ -1114,7 +1113,6 @@ static int w83977af_net_open(struct net_device *dev)
char hwname[32];
__u8 set;
-
IRDA_ASSERT(dev != NULL, return -1;);
self = netdev_priv(dev);
@@ -1263,7 +1261,6 @@ MODULE_AUTHOR("Dag Brattli <dagb@cs.uit.no>");
MODULE_DESCRIPTION("Winbond W83977AF IrDA Device Driver");
MODULE_LICENSE("GPL");
-
module_param(qos_mtt_bits, int, 0);
MODULE_PARM_DESC(qos_mtt_bits, "Mimimum Turn Time");
module_param_array(io, int, NULL, 0);
--
2.10.0.rc2.1.g053435c
^ permalink raw reply related
* [PATCH v3 4/7] irda: w83977af_ir: Neaten pointer comparisons
From: Joe Perches @ 2016-12-06 18:16 UTC (permalink / raw)
To: Samuel Ortiz; +Cc: Arnd Bergmann, Sergei Shtylyov, netdev, linux-kernel
In-Reply-To: <cover.1481047792.git.joe@perches.com>
Convert pointer comparisons to NULL.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/irda/w83977af_ir.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/net/irda/w83977af_ir.c b/drivers/net/irda/w83977af_ir.c
index e30cbf005320..92f1525b0570 100644
--- a/drivers/net/irda/w83977af_ir.c
+++ b/drivers/net/irda/w83977af_ir.c
@@ -168,7 +168,7 @@ static int w83977af_open(int i, unsigned int iobase, unsigned int irq,
* Allocate new instance of the driver
*/
dev = alloc_irdadev(sizeof(struct w83977af_ir));
- if (dev == NULL) {
+ if (!dev) {
printk(KERN_ERR "IrDA: Can't allocate memory for "
"IrDA control block!\n");
err = -ENOMEM;
@@ -206,7 +206,7 @@ static int w83977af_open(int i, unsigned int iobase, unsigned int irq,
self->rx_buff.head =
dma_zalloc_coherent(NULL, self->rx_buff.truesize,
&self->rx_buff_dma, GFP_KERNEL);
- if (self->rx_buff.head == NULL) {
+ if (!self->rx_buff.head) {
err = -ENOMEM;
goto err_out1;
}
@@ -214,7 +214,7 @@ static int w83977af_open(int i, unsigned int iobase, unsigned int irq,
self->tx_buff.head =
dma_zalloc_coherent(NULL, self->tx_buff.truesize,
&self->tx_buff_dma, GFP_KERNEL);
- if (self->tx_buff.head == NULL) {
+ if (!self->tx_buff.head) {
err = -ENOMEM;
goto err_out2;
}
@@ -629,7 +629,7 @@ static void w83977af_dma_xmit_complete(struct w83977af_ir *self)
pr_debug("%s(%ld)\n", __func__, jiffies);
- IRDA_ASSERT(self != NULL, return;);
+ IRDA_ASSERT(self, return;);
iobase = self->io.fir_base;
@@ -680,7 +680,7 @@ static int w83977af_dma_receive(struct w83977af_ir *self)
unsigned long flags;
__u8 hcr;
#endif
- IRDA_ASSERT(self != NULL, return -1;);
+ IRDA_ASSERT(self, return -1;);
pr_debug("%s\n", __func__);
@@ -818,7 +818,7 @@ static int w83977af_dma_receive_complete(struct w83977af_ir *self)
}
skb = dev_alloc_skb(len + 1);
- if (skb == NULL) {
+ if (!skb) {
printk(KERN_INFO
"%s(), memory squeeze, dropping frame.\n", __func__);
/* Restore set register */
@@ -870,7 +870,7 @@ static void w83977af_pio_receive(struct w83977af_ir *self)
__u8 byte = 0x00;
int iobase;
- IRDA_ASSERT(self != NULL, return;);
+ IRDA_ASSERT(self, return;);
iobase = self->io.fir_base;
@@ -1081,7 +1081,7 @@ static int w83977af_is_receiving(struct w83977af_ir *self)
int iobase;
__u8 set;
- IRDA_ASSERT(self != NULL, return FALSE;);
+ IRDA_ASSERT(self, return FALSE;);
if (self->io.speed > 115200) {
iobase = self->io.fir_base;
@@ -1113,10 +1113,10 @@ static int w83977af_net_open(struct net_device *dev)
char hwname[32];
__u8 set;
- IRDA_ASSERT(dev != NULL, return -1;);
+ IRDA_ASSERT(dev, return -1;);
self = netdev_priv(dev);
- IRDA_ASSERT(self != NULL, return 0;);
+ IRDA_ASSERT(self, return 0;);
iobase = self->io.fir_base;
@@ -1174,11 +1174,11 @@ static int w83977af_net_close(struct net_device *dev)
int iobase;
__u8 set;
- IRDA_ASSERT(dev != NULL, return -1;);
+ IRDA_ASSERT(dev, return -1;);
self = netdev_priv(dev);
- IRDA_ASSERT(self != NULL, return 0;);
+ IRDA_ASSERT(self, return 0;);
iobase = self->io.fir_base;
@@ -1221,11 +1221,11 @@ static int w83977af_net_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
unsigned long flags;
int ret = 0;
- IRDA_ASSERT(dev != NULL, return -1;);
+ IRDA_ASSERT(dev, return -1;);
self = netdev_priv(dev);
- IRDA_ASSERT(self != NULL, return -1;);
+ IRDA_ASSERT(self, return -1;);
pr_debug("%s(), %s, (cmd=0x%X)\n", __func__, dev->name, cmd);
--
2.10.0.rc2.1.g053435c
^ permalink raw reply related
* [PATCH v3 6/7] irda: w83977af_ir: Parenthesis alignment
From: Joe Perches @ 2016-12-06 18:16 UTC (permalink / raw)
To: Samuel Ortiz; +Cc: Arnd Bergmann, Sergei Shtylyov, netdev, linux-kernel
In-Reply-To: <cover.1481047792.git.joe@perches.com>
Neaten function declaration and definition arguments.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/irda/w83977af_ir.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/irda/w83977af_ir.c b/drivers/net/irda/w83977af_ir.c
index 5d5ec8938e2b..2f95ee26ed49 100644
--- a/drivers/net/irda/w83977af_ir.c
+++ b/drivers/net/irda/w83977af_ir.c
@@ -84,13 +84,13 @@ static struct w83977af_ir *dev_self[] = { NULL, NULL, NULL, NULL};
/* Some prototypes */
static int w83977af_open(int i, unsigned int iobase, unsigned int irq,
- unsigned int dma);
+ unsigned int dma);
static int w83977af_close(struct w83977af_ir *self);
static int w83977af_probe(int iobase, int irq, int dma);
static int w83977af_dma_receive(struct w83977af_ir *self);
static int w83977af_dma_receive_complete(struct w83977af_ir *self);
static netdev_tx_t w83977af_hard_xmit(struct sk_buff *skb,
- struct net_device *dev);
+ struct net_device *dev);
static int w83977af_pio_write(int iobase, __u8 *buf, int len, int fifo_size);
static void w83977af_dma_write(struct w83977af_ir *self, int iobase);
static void w83977af_change_speed(struct w83977af_ir *self, __u32 speed);
@@ -477,7 +477,7 @@ static void w83977af_change_speed(struct w83977af_ir *self, __u32 speed)
*
*/
static netdev_tx_t w83977af_hard_xmit(struct sk_buff *skb,
- struct net_device *dev)
+ struct net_device *dev)
{
struct w83977af_ir *self;
__s32 speed;
--
2.10.0.rc2.1.g053435c
^ permalink raw reply related
* [PATCH v3 7/7] irda: w83977af_ir: Neaten logging
From: Joe Perches @ 2016-12-06 18:16 UTC (permalink / raw)
To: Samuel Ortiz; +Cc: Arnd Bergmann, Sergei Shtylyov, netdev, linux-kernel
In-Reply-To: <cover.1481047792.git.joe@perches.com>
Use more common logging style, standardize function output logging use.
Miscellanea:
o Add and use pr_fmt
o Convert printks to pr_<level>
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/irda/w83977af_ir.c | 56 ++++++++++++++++++++----------------------
1 file changed, 27 insertions(+), 29 deletions(-)
diff --git a/drivers/net/irda/w83977af_ir.c b/drivers/net/irda/w83977af_ir.c
index 2f95ee26ed49..f293d33fb28f 100644
--- a/drivers/net/irda/w83977af_ir.c
+++ b/drivers/net/irda/w83977af_ir.c
@@ -39,6 +39,8 @@
*
********************************************************************/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/types.h>
@@ -155,7 +157,7 @@ static int w83977af_open(int i, unsigned int iobase, unsigned int irq,
/* Lock the port that we need */
if (!request_region(iobase, CHIP_IO_EXTENT, driver_name)) {
- pr_debug("%s(), can't get iobase of 0x%03x\n",
+ pr_debug("%s: can't get iobase of 0x%03x\n",
__func__, iobase);
return -ENODEV;
}
@@ -169,8 +171,7 @@ static int w83977af_open(int i, unsigned int iobase, unsigned int irq,
*/
dev = alloc_irdadev(sizeof(struct w83977af_ir));
if (!dev) {
- printk(KERN_ERR "IrDA: Can't allocate memory for "
- "IrDA control block!\n");
+ pr_err("IrDA: Can't allocate memory for IrDA control block!\n");
err = -ENOMEM;
goto err_out;
}
@@ -229,7 +230,7 @@ static int w83977af_open(int i, unsigned int iobase, unsigned int irq,
err = register_netdev(dev);
if (err) {
- net_err_ratelimited("%s(), register_netdevice() failed!\n",
+ net_err_ratelimited("%s:, register_netdevice() failed!\n",
__func__);
goto err_out3;
}
@@ -280,8 +281,7 @@ static int w83977af_close(struct w83977af_ir *self)
unregister_netdev(self->netdev);
/* Release the PORT that this driver is using */
- pr_debug("%s(), Releasing Region %03x\n",
- __func__, self->io.fir_base);
+ pr_debug("%s: Releasing Region %03x\n", __func__, self->io.fir_base);
release_region(self->io.fir_base, self->io.fir_ext);
if (self->tx_buff.head)
@@ -389,7 +389,7 @@ static int w83977af_probe(int iobase, int irq, int dma)
return 0;
} else {
/* Try next extented function register address */
- pr_debug("%s(), Wrong chip version", __func__);
+ pr_debug("%s: Wrong chip version\n", __func__);
}
}
return -1;
@@ -425,19 +425,19 @@ static void w83977af_change_speed(struct w83977af_ir *self, __u32 speed)
case 115200: outb(0x01, iobase + ABLL); break;
case 576000:
ir_mode = HCR_MIR_576;
- pr_debug("%s(), handling baud of 576000\n", __func__);
+ pr_debug("%s: handling baud of 576000\n", __func__);
break;
case 1152000:
ir_mode = HCR_MIR_1152;
- pr_debug("%s(), handling baud of 1152000\n", __func__);
+ pr_debug("%s: handling baud of 1152000\n", __func__);
break;
case 4000000:
ir_mode = HCR_FIR;
- pr_debug("%s(), handling baud of 4000000\n", __func__);
+ pr_debug("%s: handling baud of 4000000\n", __func__);
break;
default:
ir_mode = HCR_FIR;
- pr_debug("%s(), unknown baud rate of %d\n", __func__, speed);
+ pr_debug("%s: unknown baud rate of %d\n", __func__, speed);
break;
}
@@ -489,8 +489,7 @@ static netdev_tx_t w83977af_hard_xmit(struct sk_buff *skb,
iobase = self->io.fir_base;
- pr_debug("%s(%ld), skb->len=%d\n", __func__, jiffies,
- (int)skb->len);
+ pr_debug("%s: %ld, skb->len=%d\n", __func__, jiffies, (int)skb->len);
/* Lock transmit buffer */
netif_stop_queue(dev);
@@ -517,10 +516,10 @@ static netdev_tx_t w83977af_hard_xmit(struct sk_buff *skb,
self->tx_buff.len = skb->len;
mtt = irda_get_mtt(skb);
- pr_debug("%s(%ld), mtt=%d\n", __func__, jiffies, mtt);
- if (mtt > 1000)
- mdelay(mtt / 1000);
- else if (mtt)
+ pr_debug("%s: %ld, mtt=%d\n", __func__, jiffies, mtt);
+ if (mtt > 1000)
+ mdelay(mtt / 1000);
+ else if (mtt)
udelay(mtt);
/* Enable DMA interrupt */
@@ -554,7 +553,7 @@ static void w83977af_dma_write(struct w83977af_ir *self, int iobase)
{
__u8 set;
- pr_debug("%s(), len=%d\n", __func__, self->tx_buff.len);
+ pr_debug("%s: len=%d\n", __func__, self->tx_buff.len);
/* Save current set */
set = inb(iobase + SSR);
@@ -594,11 +593,10 @@ static int w83977af_pio_write(int iobase, __u8 *buf, int len, int fifo_size)
switch_bank(iobase, SET0);
if (!(inb_p(iobase + USR) & USR_TSRE)) {
- pr_debug("%s(), warning, FIFO not empty yet!\n", __func__);
+ pr_debug("%s: warning, FIFO not empty yet!\n", __func__);
fifo_size -= 17;
- pr_debug("%s(), %d bytes left in tx fifo\n",
- __func__, fifo_size);
+ pr_debug("%s: %d bytes left in tx fifo\n", __func__, fifo_size);
}
/* Fill FIFO with current frame */
@@ -607,7 +605,7 @@ static int w83977af_pio_write(int iobase, __u8 *buf, int len, int fifo_size)
outb(buf[actual++], iobase + TBR);
}
- pr_debug("%s(), fifo_size %d ; %d sent of %d\n",
+ pr_debug("%s: fifo_size %d ; %d sent of %d\n",
__func__, fifo_size, actual, len);
/* Restore bank */
@@ -628,7 +626,7 @@ static void w83977af_dma_xmit_complete(struct w83977af_ir *self)
int iobase;
__u8 set;
- pr_debug("%s(%ld)\n", __func__, jiffies);
+ pr_debug("%s: %ld\n", __func__, jiffies);
IRDA_ASSERT(self, return;);
@@ -643,7 +641,7 @@ static void w83977af_dma_xmit_complete(struct w83977af_ir *self)
/* Check for underrun! */
if (inb(iobase + AUDR) & AUDR_UNDR) {
- pr_debug("%s(), Transmit underrun!\n", __func__);
+ pr_debug("%s: Transmit underrun!\n", __func__);
self->netdev->stats.tx_errors++;
self->netdev->stats.tx_fifo_errors++;
@@ -820,8 +818,8 @@ static int w83977af_dma_receive_complete(struct w83977af_ir *self)
skb = dev_alloc_skb(len + 1);
if (!skb) {
- printk(KERN_INFO
- "%s(), memory squeeze, dropping frame.\n", __func__);
+ pr_info("%s: memory squeeze, dropping frame\n",
+ __func__);
/* Restore set register */
outb(set, iobase + SSR);
@@ -896,7 +894,7 @@ static __u8 w83977af_sir_interrupt(struct w83977af_ir *self, int isr)
__u8 set;
int iobase;
- pr_debug("%s(), isr=%#x\n", __func__, isr);
+ pr_debug("%s: isr=%#x\n", __func__, isr);
iobase = self->io.fir_base;
/* Transmit FIFO low on data */
@@ -932,7 +930,7 @@ static __u8 w83977af_sir_interrupt(struct w83977af_ir *self, int isr)
if (isr & ISR_TXEMP_I) {
/* Check if we need to change the speed? */
if (self->new_speed) {
- pr_debug("%s(), Changing speed!\n", __func__);
+ pr_debug("%s: Changing speed!\n", __func__);
w83977af_change_speed(self, self->new_speed);
self->new_speed = 0;
}
@@ -1229,7 +1227,7 @@ static int w83977af_net_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
IRDA_ASSERT(self, return -1;);
- pr_debug("%s(), %s, (cmd=0x%X)\n", __func__, dev->name, cmd);
+ pr_debug("%s: %s, (cmd=0x%X)\n", __func__, dev->name, cmd);
spin_lock_irqsave(&self->lock, flags);
--
2.10.0.rc2.1.g053435c
^ permalink raw reply related
* [PATCH v3 5/7] irda: w83977af_ir: Use the common brace style
From: Joe Perches @ 2016-12-06 18:16 UTC (permalink / raw)
To: Samuel Ortiz; +Cc: Arnd Bergmann, Sergei Shtylyov, netdev, linux-kernel
In-Reply-To: <cover.1481047792.git.joe@perches.com>
Add braces where appropriate and remove an unnecessary else.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/irda/w83977af_ir.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/net/irda/w83977af_ir.c b/drivers/net/irda/w83977af_ir.c
index 92f1525b0570..5d5ec8938e2b 100644
--- a/drivers/net/irda/w83977af_ir.c
+++ b/drivers/net/irda/w83977af_ir.c
@@ -462,8 +462,9 @@ static void w83977af_change_speed(struct w83977af_ir *self, __u32 speed)
if (speed > PIO_MAX_SPEED) {
outb(ICR_EFSFI, iobase + ICR);
w83977af_dma_receive(self);
- } else
+ } else {
outb(ICR_ERBRI, iobase + ICR);
+ }
/* Restore SSR */
outb(set, iobase + SSR);
@@ -502,8 +503,8 @@ static netdev_tx_t w83977af_hard_xmit(struct sk_buff *skb,
w83977af_change_speed(self, speed);
dev_kfree_skb(skb);
return NETDEV_TX_OK;
- } else
- self->new_speed = speed;
+ }
+ self->new_speed = speed;
}
/* Save current set */
@@ -649,8 +650,9 @@ static void w83977af_dma_xmit_complete(struct w83977af_ir *self)
/* Clear bit, by writing 1 to it */
outb(AUDR_UNDR, iobase + AUDR);
- } else
+ } else {
self->netdev->stats.tx_packets++;
+ }
if (self->new_speed) {
w83977af_change_speed(self, self->new_speed);
@@ -813,9 +815,8 @@ static int w83977af_dma_receive_complete(struct w83977af_ir *self)
} else {
/* Check if we have transferred all data to memory */
switch_bank(iobase, SET0);
- if (inb(iobase + USR) & USR_RDR) {
+ if (inb(iobase + USR) & USR_RDR)
udelay(80); /* Should be enough!? */
- }
skb = dev_alloc_skb(len + 1);
if (!skb) {
@@ -969,7 +970,6 @@ static __u8 w83977af_fir_interrupt(struct w83977af_ir *self, int isr)
/* End of frame detected in FIFO */
if (isr & (ISR_FEND_I | ISR_FSF_I)) {
if (w83977af_dma_receive_complete(self)) {
-
/* Wait for next status FIFO interrupt */
new_icr |= ICR_EFSFI;
} else {
@@ -1094,8 +1094,9 @@ static int w83977af_is_receiving(struct w83977af_ir *self)
status = TRUE;
}
outb(set, iobase + SSR);
- } else
+ } else {
status = (self->rx_buff.state != OUTSIDE_FRAME);
+ }
return status;
}
@@ -1141,8 +1142,9 @@ static int w83977af_net_open(struct net_device *dev)
if (self->io.speed > 115200) {
outb(ICR_EFSFI, iobase + ICR);
w83977af_dma_receive(self);
- } else
+ } else {
outb(ICR_ERBRI, iobase + ICR);
+ }
/* Restore bank register */
outb(set, iobase + SSR);
--
2.10.0.rc2.1.g053435c
^ permalink raw reply related
* Re: [RESEND][PATCH v4] cgroup: Use CAP_SYS_RESOURCE to allow a process to migrate other tasks between cgroups
From: Tejun Heo @ 2016-12-06 18:23 UTC (permalink / raw)
To: Andy Lutomirski
Cc: John Stultz, Alexei Starovoitov, Andy Lutomirski,
Mickaël Salaün, Daniel Mack, David S. Miller, kafai,
Florian Westphal, Harald Hoyer, Network Development,
Sargun Dhillon, Pablo Neira Ayuso, lkml, Li Zefan,
Jonathan Corbet, open list:CONTROL GROUP (CGROUP),
Android Kernel Team, Rom Lemarchand, Colin Cross
In-Reply-To: <CALCETrUmwPaWbN_U2XAsg6Z0UGBJ=Ou7BoD09-GZFdRVT5Y-EQ@mail.gmail.com>
Hello,
On Tue, Dec 06, 2016 at 10:13:53AM -0800, Andy Lutomirski wrote:
> > Delegation is an explicit operation and reflected in the ownership of
> > the subdirectories and cgroup interface files in them. The
> > subhierarchy containment is achieved by requiring the user who's
> > trying to migrate a process to have write perm on cgroup.procs on the
> > common ancestor of the source and target in addition to the target.
>
> OK, I see what you're doing. That's interesting.
It's something born out of usages of cgroup v1. People used it that
way (chowning files and directories) and combined with the uid checksn
it yielded something which is useful sometimes, but it always had
issues with hierarchical behaviors, which files to chmod and the weird
combination of uid checks. cgroup v2 has a clear delegation model but
the uid checks are still left in as not changing was the default.
It's not necessary and I'm thinking about queueing something like the
following in the next cycle.
As for the android CAP discussion, I think it'd be nice to share an
existing CAP but if we can't find a good one to share, let's create a
new one.
Thanks.
diff --git a/Documentation/cgroup-v2.txt b/Documentation/cgroup-v2.txt
index 4cc07ce..34b4b44 100644
--- a/Documentation/cgroup-v2.txt
+++ b/Documentation/cgroup-v2.txt
@@ -328,14 +328,12 @@ a process with a non-root euid to migrate a target process into a
cgroup by writing its PID to the "cgroup.procs" file, the following
conditions must be met.
-- The writer's euid must match either uid or suid of the target process.
-
- The writer must have write access to the "cgroup.procs" file.
- The writer must have write access to the "cgroup.procs" file of the
common ancestor of the source and destination cgroups.
-The above three constraints ensure that while a delegatee may migrate
+The above two constraints ensure that while a delegatee may migrate
processes around freely in the delegated sub-hierarchy it can't pull
in from or push out to outside the sub-hierarchy.
@@ -350,10 +348,10 @@ all processes under C0 and C1 belong to U0.
Let's also say U0 wants to write the PID of a process which is
currently in C10 into "C00/cgroup.procs". U0 has write access to the
-file and uid match on the process; however, the common ancestor of the
-source cgroup C10 and the destination cgroup C00 is above the points
-of delegation and U0 would not have write access to its "cgroup.procs"
-files and thus the write will be denied with -EACCES.
+file; however, the common ancestor of the source cgroup C10 and the
+destination cgroup C00 is above the points of delegation and U0 would
+not have write access to its "cgroup.procs" files and thus the write
+will be denied with -EACCES.
2-6. Guidelines
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 85bc9be..49384ff 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2854,12 +2854,18 @@ static int cgroup_procs_write_permission(struct task_struct *task,
* even if we're attaching all tasks in the thread group, we only
* need to check permissions on one of them.
*/
- if (!uid_eq(cred->euid, GLOBAL_ROOT_UID) &&
- !uid_eq(cred->euid, tcred->uid) &&
- !uid_eq(cred->euid, tcred->suid))
- ret = -EACCES;
- if (!ret && cgroup_on_dfl(dst_cgrp)) {
+ /* root is allowed to do anything */
+ if (uid_eq(cred->euid, GLOBAL_ROOT_UID))
+ goto out;
+
+ /*
+ * On v2, follow the delegation model. Inside a delegated subtree,
+ * the delegatee can move around the processes however it sees fit.
+ *
+ * On v1, a process should match one of the target's uids.
+ */
+ if (cgroup_on_dfl(dst_cgrp)) {
struct super_block *sb = of->file->f_path.dentry->d_sb;
struct cgroup *cgrp;
struct inode *inode;
@@ -2877,8 +2883,11 @@ static int cgroup_procs_write_permission(struct task_struct *task,
ret = inode_permission(inode, MAY_WRITE);
iput(inode);
}
+ } else if (!uid_eq(cred->euid, tcred->uid) &&
+ !uid_eq(cred->euid, tcred->suid)) {
+ ret = -EACCES;
}
-
+out:
put_cred(tcred);
return ret;
}
^ permalink raw reply related
* Re: [PATCH] net/udp: do not touch skb->peeked unless really needed
From: Paolo Abeni @ 2016-12-06 18:31 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Willem de Bruijn
In-Reply-To: <1481046434.18162.599.camel@edumazet-glaptop3.roam.corp.google.com>
On Tue, 2016-12-06 at 09:47 -0800, Eric Dumazet wrote:
> On Tue, 2016-12-06 at 18:08 +0100, Paolo Abeni wrote:
> > On Tue, 2016-12-06 at 11:34 +0100, Paolo Abeni wrote:
> > > On Mon, 2016-12-05 at 09:57 -0800, Eric Dumazet wrote:
> > > > From: Eric Dumazet <edumazet@google.com>
> > > >
> > > > In UDP recvmsg() path we currently access 3 cache lines from an skb
> > > > while holding receive queue lock, plus another one if packet is
> > > > dequeued, since we need to change skb->next->prev
> > > >
> > > > 1st cache line (contains ->next/prev pointers, offsets 0x00 and 0x08)
> > > > 2nd cache line (skb->len & skb->peeked, offsets 0x80 and 0x8e)
> > > > 3rd cache line (skb->truesize/users, offsets 0xe0 and 0xe4)
> > > >
> > > > skb->peeked is only needed to make sure 0-length packets are properly
> > > > handled while MSG_PEEK is operated.
> > > >
> > > > I had first the intent to remove skb->peeked but the "MSG_PEEK at
> > > > non-zero offset" support added by Sam Kumar makes this not possible.
> > > >
> > > > This patch avoids one cache line miss during the locked section, when
> > > > skb->len and skb->peeked do not have to be read.
> > > >
> > > > It also avoids the skb_set_peeked() cost for non empty UDP datagrams.
> > > >
> > > > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > >
> > > Thank you for all the good work.
> > >
> > > After all your improvement, I see the cacheline miss in inet_recvmsg()
> > > as a major perf offender for the user space process in the udp flood
> > > scenario due to skc_rxhash sharing the same sk_drops cacheline.
> > >
> > > Using an udp-specific drop counter (and an sk_drops accessor to wrap
> > > sk_drops access where needed), we could avoid such cache miss. With that
> > > - patch for udp.h only below - I get 3% improvement on top of all the
> > > pending udp patches, and the gain should be more relevant after the 2
> > > queues rework. What do you think ?
> >
> > Here follow what I'm experimenting.
>
> Well, new socket layout makes this kind of patches not really needed ?
>
> /* --- cacheline 2 boundary (128 bytes) was 8 bytes ago --- */
> socket_lock_t sk_lock; /* 0x88 0x20 */
> atomic_t sk_drops; /* 0xa8 0x4 */
> int sk_rcvlowat; /* 0xac 0x4 */
> struct sk_buff_head sk_error_queue; /* 0xb0 0x18 */
> /* --- cacheline 3 boundary (192 bytes) was 8 bytes ago --- */
cacheline 2 boundary (128 bytes) is 8 bytes before sk_lock: cacheline 2
includes also skc_refcnt and skc_rxhash from __sk_common (I use 'pahole
-E ...' to get the full blown output). skc_rxhash is read for each
packet in inet_recvmsg()/sock_rps_record_flow() if CONFIG_RPS is set. I
get a cache miss per packet there and inet_recvmsg() in my test takes
about 8% of the whole u/s processing time.
> I mentioned at some point that we can very easily instruct
> sock_skb_set_dropcount() to not read sk_drops if application
> does not care about getting sk_drops ;)
>
> https://patchwork.kernel.org/patch/9405677/
>
> Now sk_drops was moved, the plan is to submit this patch in an official way.
Looking forward to that!
Thank you,
Paolo
^ permalink raw reply
* Re: "af_unix: conditionally use freezable blocking calls in read" is wrong
From: Colin Cross @ 2016-12-06 18:37 UTC (permalink / raw)
To: Cong Wang; +Cc: Al Viro, David Miller, Linux Kernel Network Developers
In-Reply-To: <CAM_iQpXn7ssNpxviX+sLf0k-88GoWubcBeoMW=JkZTh_q8iKdg@mail.gmail.com>
On Mon, Dec 5, 2016 at 8:24 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Sun, Dec 4, 2016 at 7:52 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>> On Sun, Dec 04, 2016 at 09:42:14PM -0500, David Miller wrote:
>>> > I've run into that converting AF_UNIX to generic_file_splice_read();
>>> > I can kludge around that ("freezable unless ->msg_iter is ITER_PIPE"), but
>>> > that only delays trouble.
>>> >
>>> > Note that the only other user of freezable_schedule_timeout() is
>>> > a very different story - it's a kernel thread, which *does* have a guaranteed
>>> > locking environment. Making such assumptions in unix_stream_recvmsg(),
>>> > OTOH, is insane...
>>>
>>> We have to otherwise Android phones drain their batteries in 10
>>> minutes.
>>>
>>> I'm not going to revert this and be responsible for that.
This is an optimization for going in and out of suspend without
context switching through blocked processes, reverting it will not
cause batteries to drain in 10 minutes. On my phone, it would cause
~83 context switches on each transition in and out of suspend, which
sometimes happens every 1-5 seconds on noisy networks, but more
normally happens on the order of minutes.
>>>
>>> So you have to find a way to make the freezable calls legitimate.
>>
>> Oh, well... As I said, I can kludge around that - call from
>> generic_file_splice_read() can be distinguished by looking at the
>> ->msg_iter->type; it still means unpleasantness for kernel_recvmsg()
>> users - in effect, it can only be called with locks held if you know that
>> the socket is not an AF_UNIX one.
>>
>> BTW, how do they deal with plain pipes?
>
> I suppose this question is for Colin. ;)
The original patch set is at https://lkml.org/lkml/2013/4/29/495. It
was targeted to the sites on which many threads were blocked on an
Android device, pipe_wait didn't show up high on the list (there is
only 1 thread blocked on pipe_wait on my phone right now), so I didn't
look at it.
^ permalink raw reply
* Re: [PATCH v2 net-next 3/4] mlx4: xdp: Reserve headroom for receiving packet when XDP prog is active
From: Martin KaFai Lau @ 2016-12-06 18:27 UTC (permalink / raw)
To: Saeed Mahameed
Cc: Linux Netdev List, Alexei Starovoitov, Brenden Blanco,
Daniel Borkmann, David Miller, Jesper Dangaard Brouer,
Saeed Mahameed, Tariq Toukan, Kernel Team
In-Reply-To: <CALzJLG8-dZAvio7035CM4bFtzGanaLNWA=pExZfBJ6n9-K9ntQ@mail.gmail.com>
On Tue, Dec 06, 2016 at 06:50:47PM +0200, Saeed Mahameed wrote:
> On Mon, Dec 5, 2016 at 9:55 PM, Martin KaFai Lau <kafai@fb.com> wrote:
> > On Mon, Dec 05, 2016 at 02:54:06AM +0200, Saeed Mahameed wrote:
> >> On Sun, Dec 4, 2016 at 5:17 AM, Martin KaFai Lau <kafai@fb.com> wrote:
> >> > Reserve XDP_PACKET_HEADROOM and honor bpf_xdp_adjust_head()
> >> > when XDP prog is active. This patch only affects the code
> >> > path when XDP is active.
> >> >
> >> > Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> >> > ---
> >>
> >> Hi Martin, Sorry for the late review, i have some comments below
> >>
> >> > drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 17 +++++++++++++++--
> >> > drivers/net/ethernet/mellanox/mlx4/en_rx.c | 23 +++++++++++++++++------
> >> > drivers/net/ethernet/mellanox/mlx4/en_tx.c | 9 +++++----
> >> > drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 3 ++-
> >> > 4 files changed, 39 insertions(+), 13 deletions(-)
> >> >
> >> > diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> >> > index 311c14153b8b..094a13b52cf6 100644
> >> > --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> >> > +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> >> > @@ -51,7 +51,8 @@
> >> > #include "mlx4_en.h"
> >> > #include "en_port.h"
> >> >
> >> > -#define MLX4_EN_MAX_XDP_MTU ((int)(PAGE_SIZE - ETH_HLEN - (2 * VLAN_HLEN)))
> >> > +#define MLX4_EN_MAX_XDP_MTU ((int)(PAGE_SIZE - ETH_HLEN - (2 * VLAN_HLEN) - \
> >> > + XDP_PACKET_HEADROOM))
> >> >
> >> > int mlx4_en_setup_tc(struct net_device *dev, u8 up)
> >> > {
> >> > @@ -1551,6 +1552,7 @@ int mlx4_en_start_port(struct net_device *dev)
> >> > struct mlx4_en_tx_ring *tx_ring;
> >> > int rx_index = 0;
> >> > int err = 0;
> >> > + int mtu;
> >> > int i, t;
> >> > int j;
> >> > u8 mc_list[16] = {0};
> >> > @@ -1684,8 +1686,12 @@ int mlx4_en_start_port(struct net_device *dev)
> >> > }
> >> >
> >> > /* Configure port */
> >> > + mtu = priv->rx_skb_size + ETH_FCS_LEN;
> >> > + if (priv->tx_ring_num[TX_XDP])
> >> > + mtu += XDP_PACKET_HEADROOM;
> >> > +
> >>
> >> Why would the physical MTU care for the headroom you preserve for XDP prog?
> >> This is the wire MTU, it shouldn't be changed, please keep it as
> >> before, any preservation you make in packets buffers are needed only
> >> for FWD case or modify case (HW or wire should not care about them).
> >
> > Thanks for your feedback!
>
> Just doing my job :))
>
> >
> > FWD:
> > packet received from a port
> > => process by a XDP prog
> > => XDP_TX out to the same port.
> >
> > For example, if the received packet has 1500 payload and the XDP prog
> > encapsulates it in an IPv6 header (+40 bytes). After testing, it cannot
> > be sent out due to the HW/wire MTU is 1500.
> >
> > Even the wire MTU info was passed to the XDP prog, there is not much a
> > XDP prog could do here other than dropping it.
> >
> > Hence, this patch gives guarantee to the XDP prog such that
> > it can always send out what it has received + XDP_PACKET_HEADROOM.
> >
>
> Still i am not convinced ! this is against common sense,
> this means that the XDP prog can send packets larger than the MTU
> seen on netdev!
>
> anyway if a packet with the size (MTU + XDP_PACKET_HEADROOM) was sent
> from XDP ring and HW allowed it to exit somehow (with the code you
> provided :)), most likely it will be dropped
> at the other end.
The MTU of our receiver side is larger than 1500.
If the otherside could not handle >1500, we could lower the box running
XDP prog to 1460.
Just ensure we are on the same page. The rx MTU stays the same (1500)
because the rx_desc's byte_count is not raised by XDP_PACKET_HEADROOM.
>
> I still think XDP prog should not be allowed to FW packets larger than
> the MTU seen on the netdev and you shouldn't modify the wire MTU just
> for this case.
>
> >>
> >> > err = mlx4_SET_PORT_general(mdev->dev, priv->port,
> >> > - priv->rx_skb_size + ETH_FCS_LEN,
> >> > + mtu,
> >> > priv->prof->tx_pause,
> >> > priv->prof->tx_ppp,
> >> > priv->prof->rx_pause,
> >> > @@ -2255,6 +2261,13 @@ static bool mlx4_en_check_xdp_mtu(struct net_device *dev, int mtu)
> >> > {
> >> > struct mlx4_en_priv *priv = netdev_priv(dev);
> >> >
> >> > + if (mtu + XDP_PACKET_HEADROOM > priv->max_mtu) {
> >> > + en_err(priv,
> >> > + "Device max mtu:%d does not allow %d bytes reserved headroom for XDP prog\n",
> >> > + priv->max_mtu, XDP_PACKET_HEADROOM);
> >> > + return false;
> >> > + }
> >> > +
> >> > if (mtu > MLX4_EN_MAX_XDP_MTU) {
> >> > en_err(priv, "mtu:%d > max:%d when XDP prog is attached\n",
> >> > mtu, MLX4_EN_MAX_XDP_MTU);
> >> > diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> >> > index 23e9d04d1ef4..324771ac929e 100644
> >> > --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> >> > +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> >> > @@ -96,7 +96,6 @@ static int mlx4_en_alloc_frags(struct mlx4_en_priv *priv,
> >> > struct mlx4_en_rx_alloc page_alloc[MLX4_EN_MAX_RX_FRAGS];
> >> > const struct mlx4_en_frag_info *frag_info;
> >> > struct page *page;
> >> > - dma_addr_t dma;
> >> > int i;
> >> >
> >> > for (i = 0; i < priv->num_frags; i++) {
> >> > @@ -115,9 +114,10 @@ static int mlx4_en_alloc_frags(struct mlx4_en_priv *priv,
> >> >
> >> > for (i = 0; i < priv->num_frags; i++) {
> >> > frags[i] = ring_alloc[i];
> >> > - dma = ring_alloc[i].dma + ring_alloc[i].page_offset;
> >> > + frags[i].page_offset += priv->frag_info[i].rx_headroom;
> >>
> >> I don't see any need for headroom on frag_info other that frag0 (which
> >> where the packet starts).
> >> What is the meaning of a headroom of a frag in a middle of a packet ?
> >>
> >> if you agree with me then, you can use XDP_PACKET_HEADROOM as is where
> >> needed (i.e frag0 page offset) and remove
> >> "priv->frag_info[i].rx_headroom"
> >>
> >> ...
> >>
> >> After going through the code a little bit i see that this code is
> >> shared between XDP and common path, and you didn't want to add boolean
> >> conditions.
> >>
> >> Ok i see what you did here.
> >>
> >> Maybe we can pass headroom as a function parameter and split frag0
> >> handling from the rest ?
> >> If it is too much then i am ok with the code as it is,
> > Right, this patch does the boolean check (XDP active or not) early on
> > in mlx4_en_calc_rx_buf() (i.e. out of the fast path) and store
> > the result in priv->frag_info[0].rx_headroom.
> >
> > Just want to ensure I understand your comment correctly.
> > You prefer not to store the boolean test result in frag_info[0].rx_headroom
> > since it is redundant to !!priv->tx_ring_num[TX_XDP] and rx_headroom is also
> > confusing for frag[1-3].
> >
> > Instead, do the XDP [in]active test before calling mlx4_en_alloc_frags()
> > and then only adjust frags[0].page_offset by +XDP_PACKET_HEADROOM if is needed.
> > It could be done either by passing an extra argument to mlx4_en_alloc_frags()
> > or completely separate mlx4_en_alloc_frags(). I am fine with this also.
> >
>
> Correct, but if this change will add extra checks to the data path
> then I am ok with the current code.
Right, the check has to be done somewhere in the data path.
Lets stay with the current approach then.
>
> >
> >>
> >> > + rx_desc->data[i].addr = cpu_to_be64(frags[i].dma +
> >> > + frags[i].page_offset);
> >> > ring_alloc[i] = page_alloc[i];
> >> > - rx_desc->data[i].addr = cpu_to_be64(dma);
> >> > }
> >> >
> >> > return 0;
> >> > @@ -250,7 +250,8 @@ static int mlx4_en_prepare_rx_desc(struct mlx4_en_priv *priv,
> >> >
> >> > if (ring->page_cache.index > 0) {
> >> > frags[0] = ring->page_cache.buf[--ring->page_cache.index];
> >> > - rx_desc->data[0].addr = cpu_to_be64(frags[0].dma);
> >> > + rx_desc->data[0].addr = cpu_to_be64(frags[0].dma +
> >> > + frags[0].page_offset);
> >> > return 0;
> >> > }
> >> >
> >> > @@ -889,6 +890,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
> >> > if (xdp_prog) {
> >> > struct xdp_buff xdp;
> >> > dma_addr_t dma;
> >> > + void *pg_addr, *orig_data;
> >> > u32 act;
> >> >
> >> > dma = be64_to_cpu(rx_desc->data[0].addr);
> >> > @@ -896,11 +898,18 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
> >> > priv->frag_info[0].frag_size,
> >> > DMA_FROM_DEVICE);
> >> >
> >> > - xdp.data = page_address(frags[0].page) +
> >> > - frags[0].page_offset;
> >> > + pg_addr = page_address(frags[0].page);
> >> > + orig_data = pg_addr + frags[0].page_offset;
> >> > + xdp.data = orig_data;
> >> > xdp.data_end = xdp.data + length;
> >> >
> >> > act = bpf_prog_run_xdp(xdp_prog, &xdp);
> >> > +
> >> > + if (xdp.data != orig_data) {
> >> > + length = xdp.data_end - xdp.data;
> >> > + frags[0].page_offset = xdp.data - pg_addr;
> >> > + }
> >> > +
> >> >
> >>
> >> is this needed only for XDP FWD case ?
> > No. It is also for PASS.
> >
>
> I see.
>
> >> is this the only way to detect that the user modified the packet
> >> headers (comparing pointers, before and after) ?
> > Yes
> >
> >>
> >> if the answer is yes, it should be faster to unconditionally reset
> >> packet offset and lenght on XDP_FWD :
> >> case XDP_FWD:
> >> length = xdp.data_end - xdp.data;
> >> frags[0].page_offset = xdp.data - pg_addr;
> >>
> >>
> >> > switch (act) {
> >> > case XDP_PASS:
> >> > break;
> >> > @@ -1180,6 +1189,7 @@ void mlx4_en_calc_rx_buf(struct net_device *dev)
> >> > */
> >> > priv->frag_info[0].frag_stride = PAGE_SIZE;
> >> > priv->frag_info[0].dma_dir = PCI_DMA_BIDIRECTIONAL;
> >> > + priv->frag_info[0].rx_headroom = XDP_PACKET_HEADROOM;
> >> > i = 1;
> >> > } else {
> >> > int buf_size = 0;
> >> > @@ -1194,6 +1204,7 @@ void mlx4_en_calc_rx_buf(struct net_device *dev)
> >> > ALIGN(priv->frag_info[i].frag_size,
> >> > SMP_CACHE_BYTES);
> >> > priv->frag_info[i].dma_dir = PCI_DMA_FROMDEVICE;
> >> > + priv->frag_info[i].rx_headroom = 0;
> >>
> >> IMHO, redundant. as you see here frag0 and other frags handling are
> >> separated, maybe we can do the same in mlx4_en_alloc_frags.
> >>
> >> > buf_size += priv->frag_info[i].frag_size;
> >> > i++;
> >> > }
> >> > diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> >> > index 4b597dca5c52..9e5f38cefe5f 100644
> >> > --- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> >> > +++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> >> > @@ -354,7 +354,7 @@ u32 mlx4_en_recycle_tx_desc(struct mlx4_en_priv *priv,
> >> > struct mlx4_en_rx_alloc frame = {
> >> > .page = tx_info->page,
> >> > .dma = tx_info->map0_dma,
> >> > - .page_offset = 0,
> >> > + .page_offset = XDP_PACKET_HEADROOM,
> >> > .page_size = PAGE_SIZE,
> >> > };
> >> >
> >> > @@ -1132,7 +1132,7 @@ netdev_tx_t mlx4_en_xmit_frame(struct mlx4_en_rx_ring *rx_ring,
> >> > tx_info->page = frame->page;
> >> > frame->page = NULL;
> >> > tx_info->map0_dma = dma;
> >> > - tx_info->map0_byte_count = length;
> >> > + tx_info->map0_byte_count = length + frame->page_offset;
> >>
> >> Didn't you already take care of lenght by the following code:
> >> if (xdp.data != orig_data) {
> >> length = xdp.data_end - xdp.data;
> >> frags[0].page_offset = xdp.data - pg_addr;
> >> }
> >>
> > Before this patch, length always assumes the data starts at the beginning
> > of the page and dma is the start of the page. Hence, adding
> > framg->page_offset back to the length here.
> >
> > However, if I read the codes correctly, I think the map0_byte_count (before or
> > after this patch) does not matter since it is only used in dma_unmap_page() and
> > PAGE_SIZE is always used in dma_unmap_page() for this code patch. Hence, I think
> > we can just set map0_byte_count to PAGE_SIZE here.
> >
>
> Right, in mlx4_alloc_pages we always map with PAGE_SIZE << order
> dma = dma_map_page(priv->ddev, page, 0, PAGE_SIZE << order,
> frag_info->dma_dir);
> for XDP order is always 0, so you can safely set it to PAGE_SIZE.
>
> >> and here frame->page_offset is not really page offset, it can only be
> >> XDP_PACKET_HEADROOM.
> > Note that the XDP prog can call bpf_xdp_adjust_head() to add a header.
> > The XDP prog can extend up to XDP_PACKET_HEADROOM (256) bytes but it
> > can also (and usually) only add 40 bytes IPv6 header and then XDP_TX it out.
> >
>
> I see.
>
> >>
> >> > tx_info->nr_txbb = nr_txbb;
> >> > tx_info->nr_bytes = max_t(unsigned int, length, ETH_ZLEN);
> >> > tx_info->data_offset = (void *)data - (void *)tx_desc;
> >> > @@ -1141,9 +1141,10 @@ netdev_tx_t mlx4_en_xmit_frame(struct mlx4_en_rx_ring *rx_ring,
> >> > tx_info->linear = 1;
> >> > tx_info->inl = 0;
> >> >
> >> > - dma_sync_single_for_device(priv->ddev, dma, length, PCI_DMA_TODEVICE);
> >> > + dma_sync_single_range_for_device(priv->ddev, dma, frame->page_offset,
> >> > + length, PCI_DMA_TODEVICE);
> >> >
> >> > - data->addr = cpu_to_be64(dma);
> >> > + data->addr = cpu_to_be64(dma + frame->page_offset);
> >> > data->lkey = ring->mr_key;
> >> > dma_wmb();
> >> > data->byte_count = cpu_to_be32(length);
> >> > diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
> >> > index 20a936428f4a..ba1c6cd0cc79 100644
> >> > --- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
> >> > +++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
> >> > @@ -475,7 +475,8 @@ struct mlx4_en_frag_info {
> >> > u16 frag_prefix_size;
> >> > u32 frag_stride;
> >> > enum dma_data_direction dma_dir;
> >> > - int order;
> >> > + u16 order;
> >> > + u16 rx_headroom;
> >> > };
> >> >
> >> > #ifdef CONFIG_MLX4_EN_DCB
> >> > --
> >> > 2.5.1
> >> >
^ permalink raw reply
* Re: [PATCH] drivers: net: cpsw-phy-sel: Clear RGMII_IDMODE on "rgmii" links
From: Alex @ 2016-12-06 18:56 UTC (permalink / raw)
To: David Miller
Cc: mugunthanvnm, grygorii.strashko, linux-omap, netdev, linux-kernel,
gokhan
In-Reply-To: <20161206.113630.1454582039498444818.davem@davemloft.net>
On 12/06/2016 08:36 AM, David Miller wrote:
> From: Alexandru Gagniuc <alex.g@adaptrum.com>
> Date: Mon, 5 Dec 2016 17:33:53 -0800
>
>> Support for setting the RGMII_IDMODE bit was added in commit:
>> "drivers: net: cpsw-phy-sel: add support to configure rgmii internal delay"
>> However, that commit did not add the symmetrical clearing of the bit
>> by way of setting it in "mask". Add it here.
>>
>> Note that the documentation marks clearing this bit as "reserved",
>> however, according to TI, support for delaying the clock does exist in
>> the MAC, although it is not officially supported.
>> We tested this on a board with an RGMII to RGMII link that will not
>> work unless this bit is cleared.
>>
>> Signed-off-by: Alexandru Gagniuc <alex.g@adaptrum.com>
>
> Commits must be referenced by both short-form SHA1-ID as well as
> the commit header text.
>
> And since this change is fixing that commit, you should also provide
> a proper "Fixes: " tag on the line right before your signoff.
Thank you very much for the feedback. I will update accordingly.
Alex
> Thanks.
>
^ permalink raw reply
* [PATCH v2] drivers: net: cpsw-phy-sel: Clear RGMII_IDMODE on "rgmii" links
From: Alexandru Gagniuc @ 2016-12-06 18:56 UTC (permalink / raw)
To: mugunthanvnm
Cc: davem, grygorii.strashko, linux-omap, netdev, linux-kernel,
gokhan, Alexandru Gagniuc
In-Reply-To: <20161206.113630.1454582039498444818.davem@davemloft.net>
Support for setting the RGMII_IDMODE bit was added in the commit
referenced below. However, that commit did not add the symmetrical
clearing of the bit by way of setting it in "mask". Add it here.
Note that the documentation marks clearing this bit as "reserved",
however, according to TI, support for delaying the clock does exist in
the MAC, although it is not officially supported.
We tested this on a board with an RGMII to RGMII link that will not
work unless this bit is cleared.
Fixes: 0fb26c3063ea ("drivers: net: cpsw-phy-sel: add support to configure rgmii internal delay")
Signed-off-by: Alexandru Gagniuc <alex.g@adaptrum.com>
---
drivers/net/ethernet/ti/cpsw-phy-sel.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/ti/cpsw-phy-sel.c b/drivers/net/ethernet/ti/cpsw-phy-sel.c
index ba1e45f..1801364 100644
--- a/drivers/net/ethernet/ti/cpsw-phy-sel.c
+++ b/drivers/net/ethernet/ti/cpsw-phy-sel.c
@@ -81,6 +81,7 @@ static void cpsw_gmii_sel_am3352(struct cpsw_phy_sel_priv *priv,
};
mask = GMII_SEL_MODE_MASK << (slave * 2) | BIT(slave + 6);
+ mask |= BIT(slave + 4);
mode <<= slave * 2;
if (priv->rmii_clock_external) {
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] net/udp: do not touch skb->peeked unless really needed
From: Eric Dumazet @ 2016-12-06 18:58 UTC (permalink / raw)
To: Paolo Abeni; +Cc: David Miller, netdev, Willem de Bruijn
In-Reply-To: <1481049061.7129.18.camel@redhat.com>
On Tue, 2016-12-06 at 19:31 +0100, Paolo Abeni wrote:
> cacheline 2 boundary (128 bytes) is 8 bytes before sk_lock: cacheline 2
> includes also skc_refcnt and skc_rxhash from __sk_common (I use 'pahole
> -E ...' to get the full blown output). skc_rxhash is read for each
> packet in inet_recvmsg()/sock_rps_record_flow() if CONFIG_RPS is set. I
> get a cache miss per packet there and inet_recvmsg() in my test takes
> about 8% of the whole u/s processing time.
Wait a minute, this sk->sk_rxhash should only be read on connected
socket. Relying on it being 0 was okay only if we did not care
of false sharing. And UDP sockets used to grab socket refcount, so we
had false sharing a _lot_ in the past.
We must fix this if not already done properly.
Can you take care of this problem ?
Thanks !
^ permalink raw reply
* Re: [PATCH v2 net-next 1/4] bpf: xdp: Allow head adjustment in XDP prog
From: Martin KaFai Lau @ 2016-12-06 18:52 UTC (permalink / raw)
To: John Fastabend
Cc: netdev, Alexei Starovoitov, Brenden Blanco, Daniel Borkmann,
David Miller, Jesper Dangaard Brouer, Saeed Mahameed,
Tariq Toukan, Kernel Team
In-Reply-To: <5846F6E3.2040808@gmail.com>
On Tue, Dec 06, 2016 at 09:35:31AM -0800, John Fastabend wrote:
> On 16-12-03 07:17 PM, Martin KaFai Lau wrote:
> > This patch allows XDP prog to extend/remove the packet
> > data at the head (like adding or removing header). It is
> > done by adding a new XDP helper bpf_xdp_adjust_head().
> >
> > It also renames bpf_helper_changes_skb_data() to
> > bpf_helper_changes_pkt_data() to better reflect
> > that XDP prog does not work on skb.
> >
> > Acked-by: Alexei Starovoitov <ast@kernel.org>
> > Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> > ---
>
>
> [...]
>
> >
> > -bool bpf_helper_changes_skb_data(void *func)
> > +BPF_CALL_2(bpf_xdp_adjust_head, struct xdp_buff *, xdp, int, offset)
> > +{
> > + /* Both mlx4 and mlx5 driver align each packet to PAGE_SIZE when
> > + * XDP prog is set.
> > + * If the above is not true for the other drivers to support
> > + * bpf_xdp_adjust_head, struct xdp_buff can be extended.
> > + */
> > + unsigned long addr = (unsigned long)xdp->data & PAGE_MASK;
> > + void *data_hard_start = (void *)addr;
> > + void *data = xdp->data + offset;
> > +
> > + if (unlikely(data < data_hard_start || data > xdp->data_end - ETH_HLEN))
> > + return -EINVAL;
> > +
> > + xdp->data = data;
> > +
> > + return 0;
> > +}
> > +
>
> Sorry for the delay but I don't like the assumptions being made here
> with regards to page alignment and free space.
>
> Instead of taking the offset from PAGE_MASK how about adding a pointer
> to xdp_buff so that we get,
>
> struct xdp_buff {
> void *data;
> void *data_end;
> void *data_start;
> };
>
> This gives the headroom explicitly in the data structure. This way we
> can handle non-paged aligned usages and also some of the drivers are
> putting metadata (descriptors) at the front of the page. With this
> patch we could stomp on that metadata with the above we avoid that
> problem altogether.
Extending the xdp_buff like this was my first local attempt. And
then I realized the assumption that mlx4/5 is making. Since
there is no immediate need for this patch series and the xdp_buff
can always be extended later if needed without breaking
the xdp prog, I dropped the xdp_buff addition for now in this patch.
Sure, it will be done at the next spin.
^ permalink raw reply
* Re: Avoid deadlock situation due to use of xmit_lock
From: Lino Sanfilippo @ 2016-12-06 19:10 UTC (permalink / raw)
To: David Miller
Cc: bh74.an, ks.giri, vipul.pandya, peppe.cavallaro, alexandre.torgue,
linux-kernel, netdev
In-Reply-To: <20161206.100607.919832811727709338.davem@davemloft.net>
Hi,
On 06.12.2016 16:06, David Miller wrote:
> From: Lino Sanfilippo <LinoSanfilippo@gmx.de>
> Date: Sat, 3 Dec 2016 00:06:04 +0100
>
>> after stumbling over a potential deadlock situation in the altera driver
>> (see http://marc.info/?l=linux-netdev&m=148054615230447&w=2), I checked
>> all other ethernet drivers for the same issue and actually found it in 2
>> more, namely stmmac, and sxgbe. Please see the commit messages for a
>> description of the problem.
>> These 2 patches fix the concerning drivers.
>
> First of all, I don't want to apply these patches without proper testing
> and ACKs from the individual driver maintainers.
>
> For both of these drivers, this situation only exists because the TX
> path uses the unnecessary ->tx_lock. This private lock should be
> removed completely and the driver should use the lock the mid-layer
> already holds in the transmit path and take it in the TX reclaim path
> instead of the private ->tx_lock.
>
Ok, I will prepare a new set of patches to remove those private locks entirely.
Regards,
Lino
^ permalink raw reply
* Re: Gigabit ethernet driver for Alacritechs SLIC devices (v4)
From: Lino Sanfilippo @ 2016-12-06 19:11 UTC (permalink / raw)
To: David Miller
Cc: charrer, liodot, gregkh, andrew, roszenrami, markus.boehme,
f.fainelli, devel, linux-kernel, netdev
In-Reply-To: <20161206.113004.1297109434722530511.davem@davemloft.net>
On 06.12.2016 17:30, David Miller wrote:
> From: Lino Sanfilippo <LinoSanfilippo@gmx.de>
> Date: Mon, 5 Dec 2016 23:07:15 +0100
>
>> this is the forth version of the slicoss gigabit ethernet driver (which is a
>> rework of the driver from Alacritech which can currently be found under
>> drivers/staging/slicoss). The driver is supposed to support Mojave, Oasis and
>> Kalahari cards, for both copper and fiber.
>>
>> If this code is accepted the staging version can be removed.
>>
>> The driver has been tested on a SEN2104ET adapter (4 Port PCIe copper).
>
> I've applied this series, nice work.
>
Great, thanks!
Regards,
Lino
^ permalink raw reply
* Re: [PATCH] net/udp: do not touch skb->peeked unless really needed
From: Paolo Abeni @ 2016-12-06 19:16 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Willem de Bruijn
In-Reply-To: <1481050715.18162.604.camel@edumazet-glaptop3.roam.corp.google.com>
On Tue, 2016-12-06 at 10:58 -0800, Eric Dumazet wrote:
> On Tue, 2016-12-06 at 19:31 +0100, Paolo Abeni wrote:
>
> > cacheline 2 boundary (128 bytes) is 8 bytes before sk_lock: cacheline 2
> > includes also skc_refcnt and skc_rxhash from __sk_common (I use 'pahole
> > -E ...' to get the full blown output). skc_rxhash is read for each
> > packet in inet_recvmsg()/sock_rps_record_flow() if CONFIG_RPS is set. I
> > get a cache miss per packet there and inet_recvmsg() in my test takes
> > about 8% of the whole u/s processing time.
>
> Wait a minute, this sk->sk_rxhash should only be read on connected
> socket. Relying on it being 0 was okay only if we did not care
> of false sharing. And UDP sockets used to grab socket refcount, so we
> had false sharing a _lot_ in the past.
Thank you for the pointer.
> We must fix this if not already done properly.
>
> Can you take care of this problem ?
I'll try, but it can be very soon: I'll have limited time and bad
internet connection up to next week.
Cheers,
Paolo
^ permalink raw reply
* Re: [PATCH] net/udp: do not touch skb->peeked unless really needed
From: Eric Dumazet @ 2016-12-06 19:35 UTC (permalink / raw)
To: Paolo Abeni; +Cc: David Miller, netdev, Willem de Bruijn
In-Reply-To: <1481051786.7129.22.camel@redhat.com>
On Tue, 2016-12-06 at 20:16 +0100, Paolo Abeni wrote:
> On Tue, 2016-12-06 at 10:58 -0800, Eric Dumazet wrote:
> > On Tue, 2016-12-06 at 19:31 +0100, Paolo Abeni wrote:
> >
> > > cacheline 2 boundary (128 bytes) is 8 bytes before sk_lock: cacheline 2
> > > includes also skc_refcnt and skc_rxhash from __sk_common (I use 'pahole
> > > -E ...' to get the full blown output). skc_rxhash is read for each
> > > packet in inet_recvmsg()/sock_rps_record_flow() if CONFIG_RPS is set. I
> > > get a cache miss per packet there and inet_recvmsg() in my test takes
> > > about 8% of the whole u/s processing time.
> >
> > Wait a minute, this sk->sk_rxhash should only be read on connected
> > socket. Relying on it being 0 was okay only if we did not care
> > of false sharing. And UDP sockets used to grab socket refcount, so we
> > had false sharing a _lot_ in the past.
>
> Thank you for the pointer.
>
> > We must fix this if not already done properly.
> >
> > Can you take care of this problem ?
>
> I'll try, but it can be very soon: I'll have limited time and bad
> internet connection up to next week.
Do not worry, I had a better idea anyway. I am testing it ;)
^ permalink raw reply
* Re: [PATCH 2/6] net: ethernet: ti: cpts: add support for ext rftclk selection
From: Grygorii Strashko @ 2016-12-06 19:39 UTC (permalink / raw)
To: Richard Cochran, Murali Karicheri
Cc: David S. Miller, netdev, Mugunthan V N, Sekhar Nori, linux-kernel,
linux-omap, Rob Herring, devicetree, Wingman Kwok, linux-clk
In-Reply-To: <bf207ac4-0f9f-d4a7-14ac-704d93fb7764@ti.com>
On 11/30/2016 11:35 AM, Grygorii Strashko wrote:
>
>
> On 11/30/2016 03:56 AM, Richard Cochran wrote:
>> On Mon, Nov 28, 2016 at 05:04:24PM -0600, Grygorii Strashko wrote:
>>> Some CPTS instances, which can be found on KeyStone 2 1/10G Ethernet
>>> Switch Subsystems, can control an external multiplexer that selects
>>> one of up to 32 clocks for time sync reference (RFTCLK). This feature
>>> can be configured through CPTS_RFTCLK_SEL register (offset: x08).
>>>
>>> Hence, introduce optional DT cpts_rftclk_sel poperty wich, if present,
>>> will specify CPTS reference clock. The cpts_rftclk_sel should be
>>> omitted in DT if HW doesn't support this feature. The external fixed
>>> rate clocks can be defined in board files as "fixed-clock".
>>
>> Can't you implement this using the clock tree, rather than an ad-hoc
>> DT property?
>>
>
> I've thought about this, but decided to move forward with this impl
> which is pretty simple. I will try.
>
>
I come with below RFC patch. if no objection I'll move forward with it.
According to Keystone 2 66AK2e DM there are 7 possible ref clocks:
0000 = SYSCLK2
0001 = SYSCLK3
0010 = TIMI0
0011 = TIMI1
0100 = TSIPCLKA
1000 = TSREFCLK
1100 = TSIPCLKB
Others = Reserved
2 from above clocks are internal SYSCLK2 and SYSCLK3 - other external
(board specific). So default definition of cpts_mux will have only two parents.
If ext clock is going to be use as cpts rftclk then it should be
defined in board file and cpts_refclk_mux definition updated to support
this ext clock:
timi1clk: timi1clk {
#clock-cells = <0>;
compatible = "fixed-clock";
clock-frequency = <xxxxxxxxxx>;
clock-output-names = "timi1";
};
&cpts_mux {
clocks = <&chipclk12>, <&chipclk13>, <timi1clk>;
cpts-mux-tbl = <0>, <1>, <3>;
assigned-clocks = <&cpts_mux>;
assigned-clock-parents = <&timi1clk>;
};
>From ec5c7bed0e021c2ca7e9392173bf67bb9a45d0f4 Mon Sep 17 00:00:00 2001
From: Grygorii Strashko <grygorii.strashko@ti.com>
Date: Mon, 5 Dec 2016 12:34:45 -0600
Subject: [PATCH] cpts refclk sel
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
arch/arm/boot/dts/keystone-k2e-netcp.dtsi | 10 +++++-
drivers/net/ethernet/ti/cpts.c | 52 ++++++++++++++++++++++++++++++-
2 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/keystone-k2e-netcp.dtsi b/arch/arm/boot/dts/keystone-k2e-netcp.dtsi
index 919e655..b27aa22 100644
--- a/arch/arm/boot/dts/keystone-k2e-netcp.dtsi
+++ b/arch/arm/boot/dts/keystone-k2e-netcp.dtsi
@@ -138,7 +138,7 @@ netcp: netcp@24000000 {
/* NetCP address range */
ranges = <0 0x24000000 0x1000000>;
- clocks = <&clkpa>, <&clkcpgmac>, <&chipclk12>;
+ clocks = <&clkpa>, <&clkcpgmac>, <&cpts_mux>;
clock-names = "pa_clk", "ethss_clk", "cpts";
dma-coherent;
@@ -162,6 +162,14 @@ netcp: netcp@24000000 {
cpts-ext-ts-inputs = <6>;
cpts-ts-comp-length;
+ cpts_mux: cpts_refclk_mux {
+ #clock-cells = <0>;
+ clocks = <&chipclk12>, <&chipclk13>;
+ cpts-mux-tbl = <0>, <1>;
+ assigned-clocks = <&cpts_mux>;
+ assigned-clock-parents = <&chipclk12>;
+ };
+
interfaces {
gbe0: interface-0 {
slave-port = <0>;
diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
index 938de22..ef94316 100644
--- a/drivers/net/ethernet/ti/cpts.c
+++ b/drivers/net/ethernet/ti/cpts.c
@@ -17,6 +17,7 @@
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/
+#include <linux/clk-provider.h>
#include <linux/err.h>
#include <linux/if.h>
#include <linux/hrtimer.h>
@@ -672,6 +673,7 @@ int cpts_register(struct cpts *cpts)
cpts->phc_index = ptp_clock_index(cpts->clock);
schedule_delayed_work(&cpts->overflow_work, cpts->ov_check_period);
+
return 0;
err_ptp:
@@ -741,6 +743,54 @@ static void cpts_calc_mult_shift(struct cpts *cpts)
freq, cpts->cc_mult, cpts->cc.shift, (ns - NSEC_PER_SEC));
}
+static int cpts_of_mux_clk_setup(struct cpts *cpts, struct device_node *node)
+{
+ unsigned int num_parents;
+ const char **parent_names;
+ struct device_node *refclk_np;
+ void __iomem *reg;
+ struct clk *clk;
+ u32 *mux_table;
+ int ret;
+
+ refclk_np = of_get_child_by_name(node, "cpts_refclk_mux");
+ if (!refclk_np)
+ return -EINVAL;
+
+ num_parents = of_clk_get_parent_count(refclk_np);
+ if (num_parents < 1) {
+ dev_err(cpts->dev, "mux-clock %s must have parents\n",
+ refclk_np->name);
+ return -EINVAL;
+ }
+ parent_names = devm_kzalloc(cpts->dev, (sizeof(char *) * num_parents),
+ GFP_KERNEL);
+ if (!parent_names)
+ return -ENOMEM;
+
+ of_clk_parent_fill(refclk_np, parent_names, num_parents);
+
+ mux_table = devm_kzalloc(cpts->dev, sizeof(*mux_table) * (32 + 1),
+ GFP_KERNEL);
+ if (!mux_table)
+ return -ENOMEM;
+
+ ret = of_property_read_variable_u32_array(refclk_np, "cpts-mux-tbl",
+ mux_table, 1, 32);
+ if (ret < 0)
+ return ret;
+
+ reg = &cpts->reg->rftclk_sel;
+
+ clk = clk_register_mux_table(cpts->dev, refclk_np->name,
+ parent_names, num_parents,
+ 0, reg, 0, 0x1F, 0, mux_table, NULL);
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ return of_clk_add_provider(refclk_np, of_clk_src_simple_get, clk);
+}
+
static int cpts_of_parse(struct cpts *cpts, struct device_node *node)
{
int ret = -EINVAL;
@@ -787,7 +837,7 @@ static int cpts_of_parse(struct cpts *cpts, struct device_node *node)
if (!of_property_read_u32(node, "cpts-ext-ts-inputs", &prop))
cpts->ext_ts_inputs = prop;
- return 0;
+ return cpts_of_mux_clk_setup(cpts, node);
of_error:
dev_err(cpts->dev, "CPTS: Missing property in the DT.\n");
--
2.10.1
--
regards,
-grygorii
^ permalink raw reply related
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