From: Felipe Balbi <balbi@ti.com>
To: Dave Taht <dave.taht@gmail.com>
Cc: Felipe Balbi <balbi@ti.com>,
netdev@vger.kernel.org,
Linux OMAP Mailing List <linux-omap@vger.kernel.org>
Subject: Re: [PATCH 3/4] net: ethernet: cpsw: split out IRQ handler
Date: Fri, 2 Jan 2015 13:03:50 -0600 [thread overview]
Message-ID: <20150102190350.GC4920@saruman> (raw)
In-Reply-To: <CAA93jw7=aFM3yQLO+vtN4uHW2gMnfNaY=BdbA+b0WYq-0+gSYQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 6806 bytes --]
Hi,
(please use reply-all to keep mailing lists in Cc, also avoid
top-posting)
On Fri, Jan 02, 2015 at 10:58:29AM -0800, Dave Taht wrote:
> The beaglebone only has a 100mbit phy, so you aren't going to get more
> than that.
very true :-) Still, with AM437x SK which is definitely GigE, I'm
getting 201Mbits/sec.
> (so do a lot of IoT devices).
>
> So you have the two patches that went by on BQL and on NAPI for the beagle?
no, got any pointers ?
> On Fri, Jan 2, 2015 at 10:55 AM, Felipe Balbi <balbi@ti.com> wrote:
> > Hi,
> >
> > On Fri, Jan 02, 2015 at 10:49:49AM -0800, Dave Taht wrote:
> >> +1.
> >>
> >> We'd had a thread on netdev (can't find it now) where we discussed
> >> adding BQL support and also something saner for the NAPI handling to
> >> this driver.
> >
> > yeah, currently is completely borked. I'm on a gigabit network and I'm
> > getting 94Mbits/sec, total crap.
> >
> >> Initial results for the beaglebone black were pretty spectacular, and
> >> it does look like this is way cleaner infrastructure underneat th deal
> >> with. Are you testing
> >
> > cool, if I new more about networking I'd certainly help, but I can help
> > testing for sure, just keep me in Cc ;-)
> >
> >> on the beaglebone black.? do you remember that convo?
> >
> > yeah, testing on beagleboneblack and AM437x SK.
> >
> > cheers
> >
> >> On Fri, Jan 2, 2015 at 10:10 AM, Felipe Balbi <balbi@ti.com> wrote:
> >> > Now we can introduce dedicated IRQ handlers
> >> > for each of the IRQ events. This helps with
> >> > cleaning up a little bit of the clutter in
> >> > cpsw_interrupt() while also making sure that
> >> > TX IRQs will try to handle TX buffers while
> >> > RX IRQs will try to handle RX buffers.
> >> >
> >> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> >> > ---
> >> > drivers/net/ethernet/ti/cpsw.c | 41 ++++++++++++++++++++++++++++++-----------
> >> > 1 file changed, 30 insertions(+), 11 deletions(-)
> >> >
> >> > diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> >> > index 6e04128..c9081bd 100644
> >> > --- a/drivers/net/ethernet/ti/cpsw.c
> >> > +++ b/drivers/net/ethernet/ti/cpsw.c
> >> > @@ -754,18 +754,36 @@ requeue:
> >> > dev_kfree_skb_any(new_skb);
> >> > }
> >> >
> >> > -static irqreturn_t cpsw_interrupt(int irq, void *dev_id)
> >> > +static irqreturn_t cpsw_dummy_interrupt(int irq, void *dev_id)
> >> > {
> >> > struct cpsw_priv *priv = dev_id;
> >> > int value = irq - priv->irqs_table[0];
> >> >
> >> > - /* NOTICE: Ending IRQ here. The trick with the 'value' variable above
> >> > - * is to make sure we will always write the correct value to the EOI
> >> > - * register. Namely 0 for RX_THRESH Interrupt, 1 for RX Interrupt, 2
> >> > - * for TX Interrupt and 3 for MISC Interrupt.
> >> > - */
> >> > cpdma_ctlr_eoi(priv->dma, value);
> >> >
> >> > + return IRQ_HANDLED;
> >> > +}
> >> > +
> >> > +static irqreturn_t cpsw_tx_interrupt(int irq, void *dev_id)
> >> > +{
> >> > + struct cpsw_priv *priv = dev_id;
> >> > +
> >> > + cpdma_ctlr_eoi(priv->dma, CPDMA_EOI_TX);
> >> > + cpdma_chan_process(priv->txch, 128);
> >> > +
> >> > + priv = cpsw_get_slave_priv(priv, 1);
> >> > + if (priv)
> >> > + cpdma_chan_process(priv->txch, 128);
> >> > +
> >> > + return IRQ_HANDLED;
> >> > +}
> >> > +
> >> > +static irqreturn_t cpsw_rx_interrupt(int irq, void *dev_id)
> >> > +{
> >> > + struct cpsw_priv *priv = dev_id;
> >> > +
> >> > + cpdma_ctlr_eoi(priv->dma, CPDMA_EOI_RX);
> >> > +
> >> > cpsw_intr_disable(priv);
> >> > if (priv->irq_enabled == true) {
> >> > cpsw_disable_irq(priv);
> >> > @@ -1617,7 +1635,8 @@ static void cpsw_ndo_poll_controller(struct net_device *ndev)
> >> >
> >> > cpsw_intr_disable(priv);
> >> > cpdma_ctlr_int_ctrl(priv->dma, false);
> >> > - cpsw_interrupt(ndev->irq, priv);
> >> > + cpsw_rx_interrupt(priv->irq[1], priv);
> >> > + cpsw_tx_interrupt(priv->irq[2], priv);
> >> > cpdma_ctlr_int_ctrl(priv->dma, true);
> >> > cpsw_intr_enable(priv);
> >> > }
> >> > @@ -2351,7 +2370,7 @@ static int cpsw_probe(struct platform_device *pdev)
> >> > goto clean_ale_ret;
> >> >
> >> > priv->irqs_table[0] = irq;
> >> > - ret = devm_request_irq(&pdev->dev, irq, cpsw_interrupt,
> >> > + ret = devm_request_irq(&pdev->dev, irq, cpsw_dummy_interrupt,
> >> > 0, dev_name(&pdev->dev), priv);
> >> > if (ret < 0) {
> >> > dev_err(priv->dev, "error attaching irq (%d)\n", ret);
> >> > @@ -2363,7 +2382,7 @@ static int cpsw_probe(struct platform_device *pdev)
> >> > goto clean_ale_ret;
> >> >
> >> > priv->irqs_table[1] = irq;
> >> > - ret = devm_request_irq(&pdev->dev, irq, cpsw_interrupt,
> >> > + ret = devm_request_irq(&pdev->dev, irq, cpsw_rx_interrupt,
> >> > 0, dev_name(&pdev->dev), priv);
> >> > if (ret < 0) {
> >> > dev_err(priv->dev, "error attaching irq (%d)\n", ret);
> >> > @@ -2375,7 +2394,7 @@ static int cpsw_probe(struct platform_device *pdev)
> >> > goto clean_ale_ret;
> >> >
> >> > priv->irqs_table[2] = irq;
> >> > - ret = devm_request_irq(&pdev->dev, irq, cpsw_interrupt,
> >> > + ret = devm_request_irq(&pdev->dev, irq, cpsw_tx_interrupt,
> >> > 0, dev_name(&pdev->dev), priv);
> >> > if (ret < 0) {
> >> > dev_err(priv->dev, "error attaching irq (%d)\n", ret);
> >> > @@ -2387,7 +2406,7 @@ static int cpsw_probe(struct platform_device *pdev)
> >> > goto clean_ale_ret;
> >> >
> >> > priv->irqs_table[3] = irq;
> >> > - ret = devm_request_irq(&pdev->dev, irq, cpsw_interrupt,
> >> > + ret = devm_request_irq(&pdev->dev, irq, cpsw_dummy_interrupt,
> >> > 0, dev_name(&pdev->dev), priv);
> >> > if (ret < 0) {
> >> > dev_err(priv->dev, "error attaching irq (%d)\n", ret);
> >> > --
> >> > 2.2.0
> >> >
> >> > --
> >> > 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
> >>
> >>
> >>
> >> --
> >> Dave Täht
> >>
> >> thttp://www.bufferbloat.net/projects/bloat/wiki/Upcoming_Talks
> >
> > --
> > balbi
>
>
>
> --
> Dave Täht
>
> thttp://www.bufferbloat.net/projects/bloat/wiki/Upcoming_Talks
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Felipe Balbi <balbi@ti.com>
To: Dave Taht <dave.taht@gmail.com>
Cc: Felipe Balbi <balbi@ti.com>, <netdev@vger.kernel.org>,
Linux OMAP Mailing List <linux-omap@vger.kernel.org>
Subject: Re: [PATCH 3/4] net: ethernet: cpsw: split out IRQ handler
Date: Fri, 2 Jan 2015 13:03:50 -0600 [thread overview]
Message-ID: <20150102190350.GC4920@saruman> (raw)
In-Reply-To: <CAA93jw7=aFM3yQLO+vtN4uHW2gMnfNaY=BdbA+b0WYq-0+gSYQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 6806 bytes --]
Hi,
(please use reply-all to keep mailing lists in Cc, also avoid
top-posting)
On Fri, Jan 02, 2015 at 10:58:29AM -0800, Dave Taht wrote:
> The beaglebone only has a 100mbit phy, so you aren't going to get more
> than that.
very true :-) Still, with AM437x SK which is definitely GigE, I'm
getting 201Mbits/sec.
> (so do a lot of IoT devices).
>
> So you have the two patches that went by on BQL and on NAPI for the beagle?
no, got any pointers ?
> On Fri, Jan 2, 2015 at 10:55 AM, Felipe Balbi <balbi@ti.com> wrote:
> > Hi,
> >
> > On Fri, Jan 02, 2015 at 10:49:49AM -0800, Dave Taht wrote:
> >> +1.
> >>
> >> We'd had a thread on netdev (can't find it now) where we discussed
> >> adding BQL support and also something saner for the NAPI handling to
> >> this driver.
> >
> > yeah, currently is completely borked. I'm on a gigabit network and I'm
> > getting 94Mbits/sec, total crap.
> >
> >> Initial results for the beaglebone black were pretty spectacular, and
> >> it does look like this is way cleaner infrastructure underneat th deal
> >> with. Are you testing
> >
> > cool, if I new more about networking I'd certainly help, but I can help
> > testing for sure, just keep me in Cc ;-)
> >
> >> on the beaglebone black.? do you remember that convo?
> >
> > yeah, testing on beagleboneblack and AM437x SK.
> >
> > cheers
> >
> >> On Fri, Jan 2, 2015 at 10:10 AM, Felipe Balbi <balbi@ti.com> wrote:
> >> > Now we can introduce dedicated IRQ handlers
> >> > for each of the IRQ events. This helps with
> >> > cleaning up a little bit of the clutter in
> >> > cpsw_interrupt() while also making sure that
> >> > TX IRQs will try to handle TX buffers while
> >> > RX IRQs will try to handle RX buffers.
> >> >
> >> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> >> > ---
> >> > drivers/net/ethernet/ti/cpsw.c | 41 ++++++++++++++++++++++++++++++-----------
> >> > 1 file changed, 30 insertions(+), 11 deletions(-)
> >> >
> >> > diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> >> > index 6e04128..c9081bd 100644
> >> > --- a/drivers/net/ethernet/ti/cpsw.c
> >> > +++ b/drivers/net/ethernet/ti/cpsw.c
> >> > @@ -754,18 +754,36 @@ requeue:
> >> > dev_kfree_skb_any(new_skb);
> >> > }
> >> >
> >> > -static irqreturn_t cpsw_interrupt(int irq, void *dev_id)
> >> > +static irqreturn_t cpsw_dummy_interrupt(int irq, void *dev_id)
> >> > {
> >> > struct cpsw_priv *priv = dev_id;
> >> > int value = irq - priv->irqs_table[0];
> >> >
> >> > - /* NOTICE: Ending IRQ here. The trick with the 'value' variable above
> >> > - * is to make sure we will always write the correct value to the EOI
> >> > - * register. Namely 0 for RX_THRESH Interrupt, 1 for RX Interrupt, 2
> >> > - * for TX Interrupt and 3 for MISC Interrupt.
> >> > - */
> >> > cpdma_ctlr_eoi(priv->dma, value);
> >> >
> >> > + return IRQ_HANDLED;
> >> > +}
> >> > +
> >> > +static irqreturn_t cpsw_tx_interrupt(int irq, void *dev_id)
> >> > +{
> >> > + struct cpsw_priv *priv = dev_id;
> >> > +
> >> > + cpdma_ctlr_eoi(priv->dma, CPDMA_EOI_TX);
> >> > + cpdma_chan_process(priv->txch, 128);
> >> > +
> >> > + priv = cpsw_get_slave_priv(priv, 1);
> >> > + if (priv)
> >> > + cpdma_chan_process(priv->txch, 128);
> >> > +
> >> > + return IRQ_HANDLED;
> >> > +}
> >> > +
> >> > +static irqreturn_t cpsw_rx_interrupt(int irq, void *dev_id)
> >> > +{
> >> > + struct cpsw_priv *priv = dev_id;
> >> > +
> >> > + cpdma_ctlr_eoi(priv->dma, CPDMA_EOI_RX);
> >> > +
> >> > cpsw_intr_disable(priv);
> >> > if (priv->irq_enabled == true) {
> >> > cpsw_disable_irq(priv);
> >> > @@ -1617,7 +1635,8 @@ static void cpsw_ndo_poll_controller(struct net_device *ndev)
> >> >
> >> > cpsw_intr_disable(priv);
> >> > cpdma_ctlr_int_ctrl(priv->dma, false);
> >> > - cpsw_interrupt(ndev->irq, priv);
> >> > + cpsw_rx_interrupt(priv->irq[1], priv);
> >> > + cpsw_tx_interrupt(priv->irq[2], priv);
> >> > cpdma_ctlr_int_ctrl(priv->dma, true);
> >> > cpsw_intr_enable(priv);
> >> > }
> >> > @@ -2351,7 +2370,7 @@ static int cpsw_probe(struct platform_device *pdev)
> >> > goto clean_ale_ret;
> >> >
> >> > priv->irqs_table[0] = irq;
> >> > - ret = devm_request_irq(&pdev->dev, irq, cpsw_interrupt,
> >> > + ret = devm_request_irq(&pdev->dev, irq, cpsw_dummy_interrupt,
> >> > 0, dev_name(&pdev->dev), priv);
> >> > if (ret < 0) {
> >> > dev_err(priv->dev, "error attaching irq (%d)\n", ret);
> >> > @@ -2363,7 +2382,7 @@ static int cpsw_probe(struct platform_device *pdev)
> >> > goto clean_ale_ret;
> >> >
> >> > priv->irqs_table[1] = irq;
> >> > - ret = devm_request_irq(&pdev->dev, irq, cpsw_interrupt,
> >> > + ret = devm_request_irq(&pdev->dev, irq, cpsw_rx_interrupt,
> >> > 0, dev_name(&pdev->dev), priv);
> >> > if (ret < 0) {
> >> > dev_err(priv->dev, "error attaching irq (%d)\n", ret);
> >> > @@ -2375,7 +2394,7 @@ static int cpsw_probe(struct platform_device *pdev)
> >> > goto clean_ale_ret;
> >> >
> >> > priv->irqs_table[2] = irq;
> >> > - ret = devm_request_irq(&pdev->dev, irq, cpsw_interrupt,
> >> > + ret = devm_request_irq(&pdev->dev, irq, cpsw_tx_interrupt,
> >> > 0, dev_name(&pdev->dev), priv);
> >> > if (ret < 0) {
> >> > dev_err(priv->dev, "error attaching irq (%d)\n", ret);
> >> > @@ -2387,7 +2406,7 @@ static int cpsw_probe(struct platform_device *pdev)
> >> > goto clean_ale_ret;
> >> >
> >> > priv->irqs_table[3] = irq;
> >> > - ret = devm_request_irq(&pdev->dev, irq, cpsw_interrupt,
> >> > + ret = devm_request_irq(&pdev->dev, irq, cpsw_dummy_interrupt,
> >> > 0, dev_name(&pdev->dev), priv);
> >> > if (ret < 0) {
> >> > dev_err(priv->dev, "error attaching irq (%d)\n", ret);
> >> > --
> >> > 2.2.0
> >> >
> >> > --
> >> > 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
> >>
> >>
> >>
> >> --
> >> Dave Täht
> >>
> >> thttp://www.bufferbloat.net/projects/bloat/wiki/Upcoming_Talks
> >
> > --
> > balbi
>
>
>
> --
> Dave Täht
>
> thttp://www.bufferbloat.net/projects/bloat/wiki/Upcoming_Talks
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
next prev parent reply other threads:[~2015-01-02 19:03 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-02 18:10 [PATCH 0/4] net: cpsw: fix hangs and improve IRQ handling Felipe Balbi
2015-01-02 18:10 ` Felipe Balbi
2015-01-02 18:10 ` [PATCH 1/4] net: ethernet: cpsw: fix hangs with interrupts Felipe Balbi
2015-01-02 18:10 ` Felipe Balbi
2015-01-02 18:10 ` [PATCH 2/4] net: ethernet: cpsw: unroll IRQ request loop Felipe Balbi
2015-01-02 18:10 ` Felipe Balbi
2015-01-02 18:10 ` [PATCH 3/4] net: ethernet: cpsw: split out IRQ handler Felipe Balbi
2015-01-02 18:10 ` Felipe Balbi
[not found] ` <CAA93jw7qyZjdHGKXjiBhiYp4BWBFrUFM6FF-Lzc0i7eOnM6cNg@mail.gmail.com>
2015-01-02 18:55 ` Felipe Balbi
2015-01-02 18:55 ` Felipe Balbi
[not found] ` <CAA93jw7=aFM3yQLO+vtN4uHW2gMnfNaY=BdbA+b0WYq-0+gSYQ@mail.gmail.com>
2015-01-02 19:03 ` Felipe Balbi [this message]
2015-01-02 19:03 ` Felipe Balbi
2015-01-02 22:56 ` Dave Taht
2015-01-03 3:02 ` Felipe Balbi
2015-01-02 18:10 ` [PATCH 4/4] net: ethernet: cpsw: don't requests IRQs we don't use Felipe Balbi
2015-01-02 18:10 ` Felipe Balbi
2015-01-02 21:45 ` [PATCH 0/4] net: cpsw: fix hangs and improve IRQ handling David Miller
2015-01-02 21:53 ` Felipe Balbi
2015-01-02 21:53 ` Felipe Balbi
2015-01-02 22:04 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150102190350.GC4920@saruman \
--to=balbi@ti.com \
--cc=dave.taht@gmail.com \
--cc=linux-omap@vger.kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.