Re: [PATCH 3/4] net: ethernet: cpsw: split out IRQ handler

[Felipe Balbi <balbi@ti.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 --]