| Message ID | 1455478769-16084-1-git-send-email-ykaneko0929@gmail.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Simon Horman |
| Headers | show |
On 02/14/2016 10:39 PM, Yoshihiro Kaneko wrote: > From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com> > > This patch supports the following interrupts. > > - One interrupt for multiple (error, gPTP) > - One interrupt for emac > - Four interrupts for dma queue (best effort rx/tx, network control rx/tx) > > This patch improve efficiency of the interrupt handler by adding the > interrupt handler corresponding to each interrupt source described > above. Additionally, it reduces the number of times of the access to > EthernetAVB IF. > Also this patch prevent this driver depends on the whim of a boot loader. > > [ykaneko0929@gmail.com: define bit names of registers] > [ykaneko0929@gmail.com: add comment for gen3 only registers] > [ykaneko0929@gmail.com: fix coding style] > [ykaneko0929@gmail.com: update changelog] > [ykaneko0929@gmail.com: gen3: fix initialization of interrupts] > [ykaneko0929@gmail.com: gen3: fix clearing interrupts] > [ykaneko0929@gmail.com: gen3: add helper function for request_irq()] > [ykaneko0929@gmail.com: revert ravb_close() and ravb_ptp_stop()] > [ykaneko0929@gmail.com: avoid calling free_irq() to non-hooked interrupts] > [ykaneko0929@gmail.com: make NC/BE interrupt handler a function] > Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com> > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com> [...] > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c > index c936682..1bec71e 100644 > --- a/drivers/net/ethernet/renesas/ravb_main.c > +++ b/drivers/net/ethernet/renesas/ravb_main.c [...] > @@ -697,6 +726,39 @@ static void ravb_error_interrupt(struct net_device *ndev) > } > } > > +static int ravb_nc_be_interrupt(struct net_device *ndev, int ravb_queue, I'd call this function e.g. ravb_queue_interrupt(). And make it return 'bool' or even 'irqreturn_t' directly. And I'd suggest a shorter name for the 'ravb_queue' parameter, like 'queue' or even 'q'... > + u32 ris0, u32 *ric0, u32 tis, u32 *tic) You don't seem to need 'ric0' and 'tic' past the call sites, so no real need to pass them by reference. [...] > @@ -725,31 +787,15 @@ static irqreturn_t ravb_interrupt(int irq, void *dev_id) > > /* Network control and best effort queue RX/TX */ > for (q = RAVB_NC; q >= RAVB_BE; q--) { > - if (((ris0 & ric0) & BIT(q)) || > - ((tis & tic) & BIT(q))) { > - if (napi_schedule_prep(&priv->napi[q])) { > - /* Mask RX and TX interrupts */ > - ric0 &= ~BIT(q); > - tic &= ~BIT(q); > - ravb_write(ndev, ric0, RIC0); > - ravb_write(ndev, tic, TIC); > - __napi_schedule(&priv->napi[q]); > - } else { > - netdev_warn(ndev, > - "ignoring interrupt, rx status 0x%08x, rx mask 0x%08x,\n", > - ris0, ric0); > - netdev_warn(ndev, > - " tx status 0x%08x, tx mask 0x%08x.\n", > - tis, tic); > - } > + if (ravb_nc_be_interrupt(ndev, q, ris0, &ric0, tis, > + &tic)) > result = IRQ_HANDLED; > - } > } Unroll this *for* loop please... > } [...] > @@ -767,6 +813,73 @@ static irqreturn_t ravb_interrupt(int irq, void *dev_id) > return result; > } > > +/* Descriptor IRQ/Error/Management interrupt handler */ You don't handle the descriptor interrupt, do you? > +static irqreturn_t ravb_multi_interrupt(int irq, void *dev_id) [...] > +static irqreturn_t ravb_dmaq_interrupt(int irq, void *dev_id, int ravb_queue) Perhaps, ravb_rx_tx_interrupt()? > +{ > + struct net_device *ndev = dev_id; > + struct ravb_private *priv = netdev_priv(ndev); > + irqreturn_t result = IRQ_NONE; > + u32 ris0, ric0, tis, tic; > + > + spin_lock(&priv->lock); > + > + ris0 = ravb_read(ndev, RIS0); > + ric0 = ravb_read(ndev, RIC0); > + tis = ravb_read(ndev, TIS); > + tic = ravb_read(ndev, TIC); > + > + /* Timestamp updated */ > + if (tis & TIS_TFUF) { > + ravb_write(ndev, ~TIS_TFUF, TIS); > + ravb_get_tx_tstamp(ndev); > + result = IRQ_HANDLED; > + } Hm, why this interrupt is handled here? It has no relation to the TX queues; the manual says it belongs to group 2 (management related interrupts), and should probably be routed to ch22. Sorry for not noticing this before... [...] > @@ -1208,29 +1326,66 @@ static const struct ethtool_ops ravb_ethtool_ops = { [...] > @@ -1257,8 +1412,17 @@ out_ptp_stop: > if (priv->chip_id == RCAR_GEN2) > ravb_ptp_stop(ndev); > out_free_irq2: Rename to 'out_free_irq_nc_tx' please. > - if (priv->chip_id == RCAR_GEN3) > - free_irq(priv->emac_irq, ndev); > + if (priv->chip_id == RCAR_GEN2) > + goto out_free_irq; > + free_irq(priv->tx_irqs[RAVB_NC], ndev); > +out_free_irq_nc_rx: > + free_irq(priv->rx_irqs[RAVB_NC], ndev); > +out_free_irq_be_tx: > + free_irq(priv->tx_irqs[RAVB_BE], ndev); > +out_free_irq_be_rx: > + free_irq(priv->rx_irqs[RAVB_BE], ndev); > +out_free_irq_emac: > + free_irq(priv->emac_irq, ndev); > out_free_irq: > free_irq(ndev->irq, ndev); > out_napi_off: Seems correct now, thanks. [...] > diff --git a/drivers/net/ethernet/renesas/ravb_ptp.c b/drivers/net/ethernet/renesas/ravb_ptp.c > index 57992cc..7cb1cc4 100644 > --- a/drivers/net/ethernet/renesas/ravb_ptp.c > +++ b/drivers/net/ethernet/renesas/ravb_ptp.c > @@ -194,7 +194,14 @@ static int ravb_ptp_extts(struct ptp_clock_info *ptp, > priv->ptp.extts[req->index] = on; > > spin_lock_irqsave(&priv->lock, flags); > - ravb_modify(ndev, GIC, GIC_PTCE, on ? GIC_PTCE : 0); > + if (priv->chip_id == RCAR_GEN2) { > + ravb_modify(ndev, GIC, GIC_PTCE, on ? GIC_PTCE : 0); > + } else { > + if (on) Could be folded into one line with one less tab for the indentation below and no {} whatsoever... > + ravb_write(ndev, GIE_PTCS, GIE); > + else > + ravb_write(ndev, GID_PTCD, GID); > + } > mmiowb(); > spin_unlock_irqrestore(&priv->lock, flags); > [...] MBR, Sergei
2016-02-22 4:05 GMT+09:00 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>: > On 02/14/2016 10:39 PM, Yoshihiro Kaneko wrote: > >> From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com> >> >> This patch supports the following interrupts. >> >> - One interrupt for multiple (error, gPTP) >> - One interrupt for emac >> - Four interrupts for dma queue (best effort rx/tx, network control rx/tx) >> >> This patch improve efficiency of the interrupt handler by adding the >> interrupt handler corresponding to each interrupt source described >> above. Additionally, it reduces the number of times of the access to >> EthernetAVB IF. >> Also this patch prevent this driver depends on the whim of a boot loader. >> >> [ykaneko0929@gmail.com: define bit names of registers] >> [ykaneko0929@gmail.com: add comment for gen3 only registers] >> [ykaneko0929@gmail.com: fix coding style] >> [ykaneko0929@gmail.com: update changelog] >> [ykaneko0929@gmail.com: gen3: fix initialization of interrupts] >> [ykaneko0929@gmail.com: gen3: fix clearing interrupts] >> [ykaneko0929@gmail.com: gen3: add helper function for request_irq()] >> [ykaneko0929@gmail.com: revert ravb_close() and ravb_ptp_stop()] >> [ykaneko0929@gmail.com: avoid calling free_irq() to non-hooked interrupts] >> [ykaneko0929@gmail.com: make NC/BE interrupt handler a function] >> Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com> >> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com> > > > [...] > >> diff --git a/drivers/net/ethernet/renesas/ravb_main.c >> b/drivers/net/ethernet/renesas/ravb_main.c >> index c936682..1bec71e 100644 >> --- a/drivers/net/ethernet/renesas/ravb_main.c >> +++ b/drivers/net/ethernet/renesas/ravb_main.c > > [...] >> >> @@ -697,6 +726,39 @@ static void ravb_error_interrupt(struct net_device >> *ndev) >> } >> } >> >> +static int ravb_nc_be_interrupt(struct net_device *ndev, int ravb_queue, > > > I'd call this function e.g. ravb_queue_interrupt(). And make it return > 'bool' or even 'irqreturn_t' directly. And I'd suggest a shorter name for > the 'ravb_queue' parameter, like 'queue' or even 'q'... Agreed. > >> + u32 ris0, u32 *ric0, u32 tis, u32 *tic) > > > You don't seem to need 'ric0' and 'tic' past the call sites, so no real > need to pass them by reference. When Rx/Tx interrupt for NC and BE is issued at the same time, this function is called twice (for NC, BE) from ravb_interrupt. The interrupt mask of NC set in the first call will be reset in the next call for BE. So it is necessary to keep the modified value of "ric0" and "tic". > > [...] >> >> @@ -725,31 +787,15 @@ static irqreturn_t ravb_interrupt(int irq, void >> *dev_id) >> >> /* Network control and best effort queue RX/TX */ >> for (q = RAVB_NC; q >= RAVB_BE; q--) { >> - if (((ris0 & ric0) & BIT(q)) || >> - ((tis & tic) & BIT(q))) { >> - if (napi_schedule_prep(&priv->napi[q])) { >> - /* Mask RX and TX interrupts */ >> - ric0 &= ~BIT(q); >> - tic &= ~BIT(q); >> - ravb_write(ndev, ric0, RIC0); >> - ravb_write(ndev, tic, TIC); >> - __napi_schedule(&priv->napi[q]); >> - } else { >> - netdev_warn(ndev, >> - "ignoring interrupt, >> rx status 0x%08x, rx mask 0x%08x,\n", >> - ris0, ric0); >> - netdev_warn(ndev, >> - " >> tx status 0x%08x, tx mask 0x%08x.\n", >> - tis, tic); >> - } >> + if (ravb_nc_be_interrupt(ndev, q, ris0, &ric0, >> tis, >> + &tic)) >> result = IRQ_HANDLED; >> - } >> } > > > Unroll this *for* loop please... OK. > >> } > > [...] >> >> @@ -767,6 +813,73 @@ static irqreturn_t ravb_interrupt(int irq, void >> *dev_id) >> return result; >> } >> >> +/* Descriptor IRQ/Error/Management interrupt handler */ > > > You don't handle the descriptor interrupt, do you? Oops. I will fix this. > >> +static irqreturn_t ravb_multi_interrupt(int irq, void *dev_id) > > [...] >> >> +static irqreturn_t ravb_dmaq_interrupt(int irq, void *dev_id, int >> ravb_queue) > > > Perhaps, ravb_rx_tx_interrupt()? Agreed. > >> +{ >> + struct net_device *ndev = dev_id; >> + struct ravb_private *priv = netdev_priv(ndev); >> + irqreturn_t result = IRQ_NONE; >> + u32 ris0, ric0, tis, tic; >> + >> + spin_lock(&priv->lock); >> + >> + ris0 = ravb_read(ndev, RIS0); >> + ric0 = ravb_read(ndev, RIC0); >> + tis = ravb_read(ndev, TIS); >> + tic = ravb_read(ndev, TIC); >> + >> + /* Timestamp updated */ >> + if (tis & TIS_TFUF) { >> + ravb_write(ndev, ~TIS_TFUF, TIS); >> + ravb_get_tx_tstamp(ndev); >> + result = IRQ_HANDLED; >> + } > > > Hm, why this interrupt is handled here? It has no relation to the TX > queues; the manual says it belongs to group 2 (management related > interrupts), and should probably be routed to ch22. Sorry for not noticing > this before... Nice catch. I will move this handler into ravb_multi_interrupt(). And I will make the handler a function. > > [...] >> >> @@ -1208,29 +1326,66 @@ static const struct ethtool_ops ravb_ethtool_ops = >> { > > [...] >> >> @@ -1257,8 +1412,17 @@ out_ptp_stop: >> if (priv->chip_id == RCAR_GEN2) >> ravb_ptp_stop(ndev); >> out_free_irq2: > > > Rename to 'out_free_irq_nc_tx' please. Sure, will do. > >> - if (priv->chip_id == RCAR_GEN3) >> - free_irq(priv->emac_irq, ndev); >> + if (priv->chip_id == RCAR_GEN2) >> + goto out_free_irq; > > > > >> + free_irq(priv->tx_irqs[RAVB_NC], ndev); >> +out_free_irq_nc_rx: >> + free_irq(priv->rx_irqs[RAVB_NC], ndev); >> +out_free_irq_be_tx: >> + free_irq(priv->tx_irqs[RAVB_BE], ndev); >> +out_free_irq_be_rx: >> + free_irq(priv->rx_irqs[RAVB_BE], ndev); >> +out_free_irq_emac: >> + free_irq(priv->emac_irq, ndev); >> out_free_irq: >> free_irq(ndev->irq, ndev); >> out_napi_off: > > > Seems correct now, thanks. > > [...] >> >> diff --git a/drivers/net/ethernet/renesas/ravb_ptp.c >> b/drivers/net/ethernet/renesas/ravb_ptp.c >> index 57992cc..7cb1cc4 100644 >> --- a/drivers/net/ethernet/renesas/ravb_ptp.c >> +++ b/drivers/net/ethernet/renesas/ravb_ptp.c >> @@ -194,7 +194,14 @@ static int ravb_ptp_extts(struct ptp_clock_info *ptp, >> priv->ptp.extts[req->index] = on; >> >> spin_lock_irqsave(&priv->lock, flags); >> - ravb_modify(ndev, GIC, GIC_PTCE, on ? GIC_PTCE : 0); >> + if (priv->chip_id == RCAR_GEN2) { >> + ravb_modify(ndev, GIC, GIC_PTCE, on ? GIC_PTCE : 0); >> + } else { >> + if (on) > > > Could be folded into one line with one less tab for the indentation below > and no {} whatsoever... OK. > >> + ravb_write(ndev, GIE_PTCS, GIE); >> + else >> + ravb_write(ndev, GID_PTCD, GID); >> + } >> mmiowb(); >> spin_unlock_irqrestore(&priv->lock, flags); >> > [...] > > MBR, Sergei > Thanks, kaneko
On 02/28/2016 05:13 PM, Yoshihiro Kaneko wrote: >>> From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com> >>> >>> This patch supports the following interrupts. >>> >>> - One interrupt for multiple (error, gPTP) >>> - One interrupt for emac >>> - Four interrupts for dma queue (best effort rx/tx, network control rx/tx) >>> >>> This patch improve efficiency of the interrupt handler by adding the >>> interrupt handler corresponding to each interrupt source described >>> above. Additionally, it reduces the number of times of the access to >>> EthernetAVB IF. >>> Also this patch prevent this driver depends on the whim of a boot loader. >>> >>> [ykaneko0929@gmail.com: define bit names of registers] >>> [ykaneko0929@gmail.com: add comment for gen3 only registers] >>> [ykaneko0929@gmail.com: fix coding style] >>> [ykaneko0929@gmail.com: update changelog] >>> [ykaneko0929@gmail.com: gen3: fix initialization of interrupts] >>> [ykaneko0929@gmail.com: gen3: fix clearing interrupts] >>> [ykaneko0929@gmail.com: gen3: add helper function for request_irq()] >>> [ykaneko0929@gmail.com: revert ravb_close() and ravb_ptp_stop()] >>> [ykaneko0929@gmail.com: avoid calling free_irq() to non-hooked interrupts] >>> [ykaneko0929@gmail.com: make NC/BE interrupt handler a function] >>> Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com> >>> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com> >> >> >> [...] >> >>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c >>> b/drivers/net/ethernet/renesas/ravb_main.c >>> index c936682..1bec71e 100644 >>> --- a/drivers/net/ethernet/renesas/ravb_main.c >>> +++ b/drivers/net/ethernet/renesas/ravb_main.c >> >> [...] >>> >>> @@ -697,6 +726,39 @@ static void ravb_error_interrupt(struct net_device >>> *ndev) >>> } >>> } >>> >>> +static int ravb_nc_be_interrupt(struct net_device *ndev, int ravb_queue, >> >> >> I'd call this function e.g. ravb_queue_interrupt(). And make it return >> 'bool' or even 'irqreturn_t' directly. And I'd suggest a shorter name for >> the 'ravb_queue' parameter, like 'queue' or even 'q'... > > Agreed. > >> >>> + u32 ris0, u32 *ric0, u32 tis, u32 *tic) >> >> >> You don't seem to need 'ric0' and 'tic' past the call sites, so no real >> need to pass them by reference. > > When Rx/Tx interrupt for NC and BE is issued at the same time, > this function is called twice (for NC, BE) from ravb_interrupt. > The interrupt mask of NC set in the first call will be reset in the next > call for BE. So it is necessary to keep the modified value of "ric0" and > "tic". OK, but we still can simplify this by reading these registers right in ravb_queue_interrupt()... [...] >>> @@ -725,31 +787,15 @@ static irqreturn_t ravb_interrupt(int irq, void >>> *dev_id) >>> >>> /* Network control and best effort queue RX/TX */ >>> for (q = RAVB_NC; q >= RAVB_BE; q--) { >>> - if (((ris0 & ric0) & BIT(q)) || >>> - ((tis & tic) & BIT(q))) { >>> - if (napi_schedule_prep(&priv->napi[q])) { >>> - /* Mask RX and TX interrupts */ >>> - ric0 &= ~BIT(q); >>> - tic &= ~BIT(q); >>> - ravb_write(ndev, ric0, RIC0); >>> - ravb_write(ndev, tic, TIC); >>> - __napi_schedule(&priv->napi[q]); >>> - } else { >>> - netdev_warn(ndev, >>> - "ignoring interrupt, >>> rx status 0x%08x, rx mask 0x%08x,\n", >>> - ris0, ric0); >>> - netdev_warn(ndev, >>> - " >>> tx status 0x%08x, tx mask 0x%08x.\n", >>> - tis, tic); >>> - } >>> + if (ravb_nc_be_interrupt(ndev, q, ris0, &ric0, >>> tis, >>> + &tic)) >>> result = IRQ_HANDLED; >>> - } >>> } >> >> >> Unroll this *for* loop please... > OK. It was a bad idea actually, sorry... [...] >>> @@ -767,6 +813,73 @@ static irqreturn_t ravb_interrupt(int irq, void [...] >>> +static irqreturn_t ravb_dmaq_interrupt(int irq, void *dev_id, int >>> ravb_queue) >> >> >> Perhaps, ravb_rx_tx_interrupt()? > > Agreed. And we still have ravb_dma_interrupt() unused, right? [...] > Thanks, > kaneko MBR, Sergei
2016-03-01 5:55 GMT+09:00 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>: > On 02/28/2016 05:13 PM, Yoshihiro Kaneko wrote: > >>>> From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com> >>>> >>>> This patch supports the following interrupts. >>>> >>>> - One interrupt for multiple (error, gPTP) >>>> - One interrupt for emac >>>> - Four interrupts for dma queue (best effort rx/tx, network control >>>> rx/tx) >>>> >>>> This patch improve efficiency of the interrupt handler by adding the >>>> interrupt handler corresponding to each interrupt source described >>>> above. Additionally, it reduces the number of times of the access to >>>> EthernetAVB IF. >>>> Also this patch prevent this driver depends on the whim of a boot >>>> loader. >>>> >>>> [ykaneko0929@gmail.com: define bit names of registers] >>>> [ykaneko0929@gmail.com: add comment for gen3 only registers] >>>> [ykaneko0929@gmail.com: fix coding style] >>>> [ykaneko0929@gmail.com: update changelog] >>>> [ykaneko0929@gmail.com: gen3: fix initialization of interrupts] >>>> [ykaneko0929@gmail.com: gen3: fix clearing interrupts] >>>> [ykaneko0929@gmail.com: gen3: add helper function for request_irq()] >>>> [ykaneko0929@gmail.com: revert ravb_close() and ravb_ptp_stop()] >>>> [ykaneko0929@gmail.com: avoid calling free_irq() to non-hooked >>>> interrupts] >>>> [ykaneko0929@gmail.com: make NC/BE interrupt handler a function] >>>> Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com> >>>> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com> >>> >>> >>> >>> [...] >>> >>>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c >>>> b/drivers/net/ethernet/renesas/ravb_main.c >>>> index c936682..1bec71e 100644 >>>> --- a/drivers/net/ethernet/renesas/ravb_main.c >>>> +++ b/drivers/net/ethernet/renesas/ravb_main.c >>> >>> >>> [...] >>>> >>>> >>>> @@ -697,6 +726,39 @@ static void ravb_error_interrupt(struct net_device >>>> *ndev) >>>> } >>>> } >>>> >>>> +static int ravb_nc_be_interrupt(struct net_device *ndev, int >>>> ravb_queue, >>> >>> >>> >>> I'd call this function e.g. ravb_queue_interrupt(). And make it >>> return >>> 'bool' or even 'irqreturn_t' directly. And I'd suggest a shorter name for >>> the 'ravb_queue' parameter, like 'queue' or even 'q'... >> >> >> Agreed. >> >>> >>>> + u32 ris0, u32 *ric0, u32 tis, u32 *tic) >>> >>> >>> >>> You don't seem to need 'ric0' and 'tic' past the call sites, so no >>> real >>> need to pass them by reference. >> >> >> When Rx/Tx interrupt for NC and BE is issued at the same time, >> this function is called twice (for NC, BE) from ravb_interrupt. >> The interrupt mask of NC set in the first call will be reset in the next >> call for BE. So it is necessary to keep the modified value of "ric0" and >> "tic". > > > OK, but we still can simplify this by reading these registers right in > ravb_queue_interrupt()... OK. > > > [...] >>>> >>>> @@ -725,31 +787,15 @@ static irqreturn_t ravb_interrupt(int irq, void >>>> *dev_id) >>>> >>>> /* Network control and best effort queue RX/TX */ >>>> for (q = RAVB_NC; q >= RAVB_BE; q--) { >>>> - if (((ris0 & ric0) & BIT(q)) || >>>> - ((tis & tic) & BIT(q))) { >>>> - if (napi_schedule_prep(&priv->napi[q])) >>>> { >>>> - /* Mask RX and TX interrupts */ >>>> - ric0 &= ~BIT(q); >>>> - tic &= ~BIT(q); >>>> - ravb_write(ndev, ric0, RIC0); >>>> - ravb_write(ndev, tic, TIC); >>>> - __napi_schedule(&priv->napi[q]); >>>> - } else { >>>> - netdev_warn(ndev, >>>> - "ignoring interrupt, >>>> rx status 0x%08x, rx mask 0x%08x,\n", >>>> - ris0, ric0); >>>> - netdev_warn(ndev, >>>> - " >>>> tx status 0x%08x, tx mask 0x%08x.\n", >>>> - tis, tic); >>>> - } >>>> + if (ravb_nc_be_interrupt(ndev, q, ris0, &ric0, >>>> tis, >>>> + &tic)) >>>> result = IRQ_HANDLED; >>>> - } >>>> } >>> >>> >>> >>> Unroll this *for* loop please... > > >> OK. > > > It was a bad idea actually, sorry... OK, I will revert this part. (But I think it is not that bad...) > > [...] >>>> >>>> @@ -767,6 +813,73 @@ static irqreturn_t ravb_interrupt(int irq, void > > [...] >>>> >>>> +static irqreturn_t ravb_dmaq_interrupt(int irq, void *dev_id, int >>>> ravb_queue) >>> >>> >>> >>> Perhaps, ravb_rx_tx_interrupt()? >> >> >> Agreed. > > > And we still have ravb_dma_interrupt() unused, right? Do you mean that there is an unused "ravb_dma_interrupt()" in this version? I probably be misunderstanding something. > > [...] > >> Thanks, >> kaneko > > > MBR, Sergei > Thanks, kaneko
Hello. On 03/02/2016 09:32 PM, Yoshihiro Kaneko wrote: >>>>> From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com> >>>>> >>>>> This patch supports the following interrupts. >>>>> >>>>> - One interrupt for multiple (error, gPTP) >>>>> - One interrupt for emac >>>>> - Four interrupts for dma queue (best effort rx/tx, network control >>>>> rx/tx) >>>>> >>>>> This patch improve efficiency of the interrupt handler by adding the >>>>> interrupt handler corresponding to each interrupt source described >>>>> above. Additionally, it reduces the number of times of the access to >>>>> EthernetAVB IF. >>>>> Also this patch prevent this driver depends on the whim of a boot >>>>> loader. >>>>> >>>>> [ykaneko0929@gmail.com: define bit names of registers] >>>>> [ykaneko0929@gmail.com: add comment for gen3 only registers] >>>>> [ykaneko0929@gmail.com: fix coding style] >>>>> [ykaneko0929@gmail.com: update changelog] >>>>> [ykaneko0929@gmail.com: gen3: fix initialization of interrupts] >>>>> [ykaneko0929@gmail.com: gen3: fix clearing interrupts] >>>>> [ykaneko0929@gmail.com: gen3: add helper function for request_irq()] >>>>> [ykaneko0929@gmail.com: revert ravb_close() and ravb_ptp_stop()] >>>>> [ykaneko0929@gmail.com: avoid calling free_irq() to non-hooked >>>>> interrupts] >>>>> [ykaneko0929@gmail.com: make NC/BE interrupt handler a function] >>>>> Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com> >>>>> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com> [...] >>>>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c >>>>> b/drivers/net/ethernet/renesas/ravb_main.c >>>>> index c936682..1bec71e 100644 >>>>> --- a/drivers/net/ethernet/renesas/ravb_main.c >>>>> +++ b/drivers/net/ethernet/renesas/ravb_main.c [...] >>>>> @@ -725,31 +787,15 @@ static irqreturn_t ravb_interrupt(int irq, void >>>>> *dev_id) >>>>> >>>>> /* Network control and best effort queue RX/TX */ >>>>> for (q = RAVB_NC; q >= RAVB_BE; q--) { >>>>> - if (((ris0 & ric0) & BIT(q)) || >>>>> - ((tis & tic) & BIT(q))) { >>>>> - if (napi_schedule_prep(&priv->napi[q])) >>>>> { >>>>> - /* Mask RX and TX interrupts */ >>>>> - ric0 &= ~BIT(q); >>>>> - tic &= ~BIT(q); >>>>> - ravb_write(ndev, ric0, RIC0); >>>>> - ravb_write(ndev, tic, TIC); >>>>> - __napi_schedule(&priv->napi[q]); >>>>> - } else { >>>>> - netdev_warn(ndev, >>>>> - "ignoring interrupt, >>>>> rx status 0x%08x, rx mask 0x%08x,\n", >>>>> - ris0, ric0); >>>>> - netdev_warn(ndev, >>>>> - " >>>>> tx status 0x%08x, tx mask 0x%08x.\n", >>>>> - tis, tic); >>>>> - } >>>>> + if (ravb_nc_be_interrupt(ndev, q, ris0, &ric0, >>>>> tis, >>>>> + &tic)) >>>>> result = IRQ_HANDLED; >>>>> - } >>>>> } >>>> >>>> Unroll this *for* loop please... >>> >>> OK. >> >> It was a bad idea actually, sorry... > > OK, I will revert this part. > (But I think it is not that bad...) I think the loop will scale better if we ever support the AVB queues... [...] >>>>> @@ -767,6 +813,73 @@ static irqreturn_t ravb_interrupt(int irq, void [...] >>>>> +static irqreturn_t ravb_dmaq_interrupt(int irq, void *dev_id, int >>>>> ravb_queue) >>>> >>>> Perhaps, ravb_rx_tx_interrupt()? >>> >>> Agreed. >> >> And we still have ravb_dma_interrupt() unused, right? > > Do you mean that there is an unused "ravb_dma_interrupt()" in this version? > I probably be misunderstanding something. No, I meant that this name isn't used yet. >> [...] > Thanks, > kaneko MBR, Sergei
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h index b2160d1..5c16241 100644 --- a/drivers/net/ethernet/renesas/ravb.h +++ b/drivers/net/ethernet/renesas/ravb.h @@ -157,6 +157,7 @@ enum ravb_reg { TIC = 0x0378, TIS = 0x037C, ISS = 0x0380, + CIE = 0x0384, /* R-Car Gen3 only */ GCCR = 0x0390, GMTT = 0x0394, GPTC = 0x0398, @@ -170,6 +171,15 @@ enum ravb_reg { GCT0 = 0x03B8, GCT1 = 0x03BC, GCT2 = 0x03C0, + GIE = 0x03CC, /* R-Car Gen3 only */ + GID = 0x03D0, /* R-Car Gen3 only */ + DIL = 0x0440, /* R-Car Gen3 only */ + RIE0 = 0x0460, /* R-Car Gen3 only */ + RID0 = 0x0464, /* R-Car Gen3 only */ + RIE2 = 0x0470, /* R-Car Gen3 only */ + RID2 = 0x0474, /* R-Car Gen3 only */ + TIE = 0x0478, /* R-Car Gen3 only */ + TID = 0x047c, /* R-Car Gen3 only */ /* E-MAC registers */ ECMR = 0x0500, @@ -556,6 +566,16 @@ enum ISS_BIT { ISS_DPS15 = 0x80000000, }; +/* CIE (R-Car Gen3 only) */ +enum CIE_BIT { + CIE_CRIE = 0x00000001, + CIE_CTIE = 0x00000100, + CIE_RQFM = 0x00010000, + CIE_CL0M = 0x00020000, + CIE_RFWL = 0x00040000, + CIE_RFFL = 0x00080000, +}; + /* GCCR */ enum GCCR_BIT { GCCR_TCR = 0x00000003, @@ -592,6 +612,188 @@ enum GIS_BIT { GIS_PTMF = 0x00000004, }; +/* GIE (R-Car Gen3 only) */ +enum GIE_BIT { + GIE_PTCS = 0x00000001, + GIE_PTOS = 0x00000002, + GIE_PTMS0 = 0x00000004, + GIE_PTMS1 = 0x00000008, + GIE_PTMS2 = 0x00000010, + GIE_PTMS3 = 0x00000020, + GIE_PTMS4 = 0x00000040, + GIE_PTMS5 = 0x00000080, + GIE_PTMS6 = 0x00000100, + GIE_PTMS7 = 0x00000200, + GIE_ATCS0 = 0x00010000, + GIE_ATCS1 = 0x00020000, + GIE_ATCS2 = 0x00040000, + GIE_ATCS3 = 0x00080000, + GIE_ATCS4 = 0x00100000, + GIE_ATCS5 = 0x00200000, + GIE_ATCS6 = 0x00400000, + GIE_ATCS7 = 0x00800000, + GIE_ATCS8 = 0x01000000, + GIE_ATCS9 = 0x02000000, + GIE_ATCS10 = 0x04000000, + GIE_ATCS11 = 0x08000000, + GIE_ATCS12 = 0x10000000, + GIE_ATCS13 = 0x20000000, + GIE_ATCS14 = 0x40000000, + GIE_ATCS15 = 0x80000000, +}; + +/* GID (R-Car Gen3 only) */ +enum GID_BIT { + GID_PTCD = 0x00000001, + GID_PTOD = 0x00000002, + GID_PTMD0 = 0x00000004, + GID_PTMD1 = 0x00000008, + GID_PTMD2 = 0x00000010, + GID_PTMD3 = 0x00000020, + GID_PTMD4 = 0x00000040, + GID_PTMD5 = 0x00000080, + GID_PTMD6 = 0x00000100, + GID_PTMD7 = 0x00000200, + GID_ATCD0 = 0x00010000, + GID_ATCD1 = 0x00020000, + GID_ATCD2 = 0x00040000, + GID_ATCD3 = 0x00080000, + GID_ATCD4 = 0x00100000, + GID_ATCD5 = 0x00200000, + GID_ATCD6 = 0x00400000, + GID_ATCD7 = 0x00800000, + GID_ATCD8 = 0x01000000, + GID_ATCD9 = 0x02000000, + GID_ATCD10 = 0x04000000, + GID_ATCD11 = 0x08000000, + GID_ATCD12 = 0x10000000, + GID_ATCD13 = 0x20000000, + GID_ATCD14 = 0x40000000, + GID_ATCD15 = 0x80000000, +}; + +/* RIE0 (R-Car Gen3 only) */ +enum RIE0_BIT { + RIE0_FRS0 = 0x00000001, + RIE0_FRS1 = 0x00000002, + RIE0_FRS2 = 0x00000004, + RIE0_FRS3 = 0x00000008, + RIE0_FRS4 = 0x00000010, + RIE0_FRS5 = 0x00000020, + RIE0_FRS6 = 0x00000040, + RIE0_FRS7 = 0x00000080, + RIE0_FRS8 = 0x00000100, + RIE0_FRS9 = 0x00000200, + RIE0_FRS10 = 0x00000400, + RIE0_FRS11 = 0x00000800, + RIE0_FRS12 = 0x00001000, + RIE0_FRS13 = 0x00002000, + RIE0_FRS14 = 0x00004000, + RIE0_FRS15 = 0x00008000, + RIE0_FRS16 = 0x00010000, + RIE0_FRS17 = 0x00020000, +}; + +/* RID0 (R-Car Gen3 only) */ +enum RID0_BIT { + RID0_FRD0 = 0x00000001, + RID0_FRD1 = 0x00000002, + RID0_FRD2 = 0x00000004, + RID0_FRD3 = 0x00000008, + RID0_FRD4 = 0x00000010, + RID0_FRD5 = 0x00000020, + RID0_FRD6 = 0x00000040, + RID0_FRD7 = 0x00000080, + RID0_FRD8 = 0x00000100, + RID0_FRD9 = 0x00000200, + RID0_FRD10 = 0x00000400, + RID0_FRD11 = 0x00000800, + RID0_FRD12 = 0x00001000, + RID0_FRD13 = 0x00002000, + RID0_FRD14 = 0x00004000, + RID0_FRD15 = 0x00008000, + RID0_FRD16 = 0x00010000, + RID0_FRD17 = 0x00020000, +}; + +/* RIE2 (R-Car Gen3 only) */ +enum RIE2_BIT { + RIE2_QFS0 = 0x00000001, + RIE2_QFS1 = 0x00000002, + RIE2_QFS2 = 0x00000004, + RIE2_QFS3 = 0x00000008, + RIE2_QFS4 = 0x00000010, + RIE2_QFS5 = 0x00000020, + RIE2_QFS6 = 0x00000040, + RIE2_QFS7 = 0x00000080, + RIE2_QFS8 = 0x00000100, + RIE2_QFS9 = 0x00000200, + RIE2_QFS10 = 0x00000400, + RIE2_QFS11 = 0x00000800, + RIE2_QFS12 = 0x00001000, + RIE2_QFS13 = 0x00002000, + RIE2_QFS14 = 0x00004000, + RIE2_QFS15 = 0x00008000, + RIE2_QFS16 = 0x00010000, + RIE2_QFS17 = 0x00020000, + RIE2_RFFS = 0x80000000, +}; + +/* RID2 (R-Car Gen3 only) */ +enum RID2_BIT { + RID2_QFD0 = 0x00000001, + RID2_QFD1 = 0x00000002, + RID2_QFD2 = 0x00000004, + RID2_QFD3 = 0x00000008, + RID2_QFD4 = 0x00000010, + RID2_QFD5 = 0x00000020, + RID2_QFD6 = 0x00000040, + RID2_QFD7 = 0x00000080, + RID2_QFD8 = 0x00000100, + RID2_QFD9 = 0x00000200, + RID2_QFD10 = 0x00000400, + RID2_QFD11 = 0x00000800, + RID2_QFD12 = 0x00001000, + RID2_QFD13 = 0x00002000, + RID2_QFD14 = 0x00004000, + RID2_QFD15 = 0x00008000, + RID2_QFD16 = 0x00010000, + RID2_QFD17 = 0x00020000, + RID2_RFFD = 0x80000000, +}; + +/* TIE (R-Car Gen3 only) */ +enum TIE_BIT { + TIE_FTS0 = 0x00000001, + TIE_FTS1 = 0x00000002, + TIE_FTS2 = 0x00000004, + TIE_FTS3 = 0x00000008, + TIE_TFUS = 0x00000100, + TIE_TFWS = 0x00000200, + TIE_MFUS = 0x00000400, + TIE_MFWS = 0x00000800, + TIE_TDPS0 = 0x00010000, + TIE_TDPS1 = 0x00020000, + TIE_TDPS2 = 0x00040000, + TIE_TDPS3 = 0x00080000, +}; + +/* TID (R-Car Gen3 only) */ +enum TID_BIT { + TID_FTD0 = 0x00000001, + TID_FTD1 = 0x00000002, + TID_FTD2 = 0x00000004, + TID_FTD3 = 0x00000008, + TID_TFUD = 0x00000100, + TID_TFWD = 0x00000200, + TID_MFUD = 0x00000400, + TID_MFWD = 0x00000800, + TID_TDPD0 = 0x00010000, + TID_TDPD1 = 0x00020000, + TID_TDPD2 = 0x00040000, + TID_TDPD3 = 0x00080000, +}; + /* ECMR */ enum ECMR_BIT { ECMR_PRM = 0x00000001, @@ -817,6 +1019,8 @@ struct ravb_private { int duplex; int emac_irq; enum ravb_chip_id chip_id; + int rx_irqs[NUM_RX_QUEUE]; + int tx_irqs[NUM_TX_QUEUE]; unsigned no_avb_link:1; unsigned avb_link_active_low:1; diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c index c936682..1bec71e 100644 --- a/drivers/net/ethernet/renesas/ravb_main.c +++ b/drivers/net/ethernet/renesas/ravb_main.c @@ -42,6 +42,16 @@ NETIF_MSG_RX_ERR | \ NETIF_MSG_TX_ERR) +static const char *ravb_rx_irqs[NUM_RX_QUEUE] = { + "ch0", /* RAVB_BE */ + "ch1", /* RAVB_NC */ +}; + +static const char *ravb_tx_irqs[NUM_TX_QUEUE] = { + "ch18", /* RAVB_BE */ + "ch19", /* RAVB_NC */ +}; + void ravb_modify(struct net_device *ndev, enum ravb_reg reg, u32 clear, u32 set) { @@ -367,6 +377,7 @@ static void ravb_emac_init(struct net_device *ndev) /* Device init function for Ethernet AVB */ static int ravb_dmac_init(struct net_device *ndev) { + struct ravb_private *priv = netdev_priv(ndev); int error; /* Set CONFIG mode */ @@ -403,6 +414,12 @@ static int ravb_dmac_init(struct net_device *ndev) ravb_write(ndev, TCCR_TFEN, TCCR); /* Interrupt init: */ + if (priv->chip_id == RCAR_GEN3) { + /* Clear DIL.DPLx */ + ravb_write(ndev, 0, DIL); + /* Set queue specific interrupt */ + ravb_write(ndev, CIE_CRIE | CIE_CTIE | CIE_CL0M, CIE); + } /* Frame receive */ ravb_write(ndev, RIC0_FRE0 | RIC0_FRE1, RIC0); /* Disable FIFO full warning */ @@ -645,7 +662,7 @@ static int ravb_stop_dma(struct net_device *ndev) } /* E-MAC interrupt handler */ -static void ravb_emac_interrupt(struct net_device *ndev) +static void ravb_emac_interrupt_unlocked(struct net_device *ndev) { struct ravb_private *priv = netdev_priv(ndev); u32 ecsr, psr; @@ -671,6 +688,18 @@ static void ravb_emac_interrupt(struct net_device *ndev) } } +static irqreturn_t ravb_emac_interrupt(int irq, void *dev_id) +{ + struct net_device *ndev = dev_id; + struct ravb_private *priv = netdev_priv(ndev); + + spin_lock(&priv->lock); + ravb_emac_interrupt_unlocked(ndev); + mmiowb(); + spin_unlock(&priv->lock); + return IRQ_HANDLED; +} + /* Error interrupt handler */ static void ravb_error_interrupt(struct net_device *ndev) { @@ -697,6 +726,39 @@ static void ravb_error_interrupt(struct net_device *ndev) } } +static int ravb_nc_be_interrupt(struct net_device *ndev, int ravb_queue, + u32 ris0, u32 *ric0, u32 tis, u32 *tic) +{ + struct ravb_private *priv = netdev_priv(ndev); + + if (((ris0 & *ric0) & BIT(ravb_queue)) || + ((tis & *tic) & BIT(ravb_queue))) { + if (napi_schedule_prep(&priv->napi[ravb_queue])) { + /* Mask RX and TX interrupts */ + if (priv->chip_id == RCAR_GEN2) { + *ric0 &= ~BIT(ravb_queue); + *tic &= ~BIT(ravb_queue); + ravb_write(ndev, *ric0, RIC0); + ravb_write(ndev, *tic, TIC); + } else { + ravb_write(ndev, BIT(ravb_queue), RID0); + ravb_write(ndev, BIT(ravb_queue), TID); + } + __napi_schedule(&priv->napi[ravb_queue]); + } else { + netdev_warn(ndev, + "ignoring interrupt, rx status 0x%08x, rx mask 0x%08x,\n", + ris0, *ric0); + netdev_warn(ndev, + " tx status 0x%08x, tx mask 0x%08x.\n", + tis, *tic); + } + return 1; + } + + return 0; +} + static irqreturn_t ravb_interrupt(int irq, void *dev_id) { struct net_device *ndev = dev_id; @@ -725,31 +787,15 @@ static irqreturn_t ravb_interrupt(int irq, void *dev_id) /* Network control and best effort queue RX/TX */ for (q = RAVB_NC; q >= RAVB_BE; q--) { - if (((ris0 & ric0) & BIT(q)) || - ((tis & tic) & BIT(q))) { - if (napi_schedule_prep(&priv->napi[q])) { - /* Mask RX and TX interrupts */ - ric0 &= ~BIT(q); - tic &= ~BIT(q); - ravb_write(ndev, ric0, RIC0); - ravb_write(ndev, tic, TIC); - __napi_schedule(&priv->napi[q]); - } else { - netdev_warn(ndev, - "ignoring interrupt, rx status 0x%08x, rx mask 0x%08x,\n", - ris0, ric0); - netdev_warn(ndev, - " tx status 0x%08x, tx mask 0x%08x.\n", - tis, tic); - } + if (ravb_nc_be_interrupt(ndev, q, ris0, &ric0, tis, + &tic)) result = IRQ_HANDLED; - } } } /* E-MAC status summary */ if (iss & ISS_MS) { - ravb_emac_interrupt(ndev); + ravb_emac_interrupt_unlocked(ndev); result = IRQ_HANDLED; } @@ -767,6 +813,73 @@ static irqreturn_t ravb_interrupt(int irq, void *dev_id) return result; } +/* Descriptor IRQ/Error/Management interrupt handler */ +static irqreturn_t ravb_multi_interrupt(int irq, void *dev_id) +{ + struct net_device *ndev = dev_id; + struct ravb_private *priv = netdev_priv(ndev); + irqreturn_t result = IRQ_NONE; + u32 iss; + + spin_lock(&priv->lock); + /* Get interrupt status */ + iss = ravb_read(ndev, ISS); + + /* Error status summary */ + if (iss & ISS_ES) { + ravb_error_interrupt(ndev); + result = IRQ_HANDLED; + } + + /* gPTP interrupt status summary */ + if (iss & ISS_CGIS) + result = ravb_ptp_interrupt(ndev); + + mmiowb(); + spin_unlock(&priv->lock); + return result; +} + +static irqreturn_t ravb_dmaq_interrupt(int irq, void *dev_id, int ravb_queue) +{ + struct net_device *ndev = dev_id; + struct ravb_private *priv = netdev_priv(ndev); + irqreturn_t result = IRQ_NONE; + u32 ris0, ric0, tis, tic; + + spin_lock(&priv->lock); + + ris0 = ravb_read(ndev, RIS0); + ric0 = ravb_read(ndev, RIC0); + tis = ravb_read(ndev, TIS); + tic = ravb_read(ndev, TIC); + + /* Timestamp updated */ + if (tis & TIS_TFUF) { + ravb_write(ndev, ~TIS_TFUF, TIS); + ravb_get_tx_tstamp(ndev); + result = IRQ_HANDLED; + } + + /* Network control/Best effort queue RX/TX */ + if (ravb_nc_be_interrupt(ndev, ravb_queue, ris0, &ric0, tis, &tic)) + result = IRQ_HANDLED; + + mmiowb(); + spin_unlock(&priv->lock); + return result; +} + +static irqreturn_t ravb_be_interrupt(int irq, void *dev_id) +{ + return ravb_dmaq_interrupt(irq, dev_id, RAVB_BE); +} + +static irqreturn_t ravb_nc_interrupt(int irq, void *dev_id) +{ + return ravb_dmaq_interrupt(irq, dev_id, RAVB_NC); +} + static int ravb_poll(struct napi_struct *napi, int budget) { struct net_device *ndev = napi->dev; @@ -806,8 +919,13 @@ static int ravb_poll(struct napi_struct *napi, int budget) /* Re-enable RX/TX interrupts */ spin_lock_irqsave(&priv->lock, flags); - ravb_modify(ndev, RIC0, mask, mask); - ravb_modify(ndev, TIC, mask, mask); + if (priv->chip_id == RCAR_GEN2) { + ravb_modify(ndev, RIC0, mask, mask); + ravb_modify(ndev, TIC, mask, mask); + } else { + ravb_write(ndev, mask, RIE0); + ravb_write(ndev, mask, TIE); + } mmiowb(); spin_unlock_irqrestore(&priv->lock, flags); @@ -1208,29 +1326,66 @@ static const struct ethtool_ops ravb_ethtool_ops = { .get_ts_info = ravb_get_ts_info, }; +static inline int ravb_hook_irq(unsigned int irq, irq_handler_t handler, + struct net_device *ndev, struct device *dev, + const char *ch) +{ + char *name; + int error; + + name = devm_kasprintf(dev, GFP_KERNEL, "%s:%s", ndev->name, ch); + if (!name) + return -ENOMEM; + error = request_irq(irq, handler, IRQF_SHARED, name, ndev); + if (error) + netdev_err(ndev, "cannot request IRQ %s\n", name); + + return error; +} + /* Network device open function for Ethernet AVB */ static int ravb_open(struct net_device *ndev) { struct ravb_private *priv = netdev_priv(ndev); + struct platform_device *pdev = priv->pdev; + struct device *dev = &pdev->dev; int error; napi_enable(&priv->napi[RAVB_BE]); napi_enable(&priv->napi[RAVB_NC]); - error = request_irq(ndev->irq, ravb_interrupt, IRQF_SHARED, ndev->name, - ndev); - if (error) { - netdev_err(ndev, "cannot request IRQ\n"); - goto out_napi_off; - } - - if (priv->chip_id == RCAR_GEN3) { - error = request_irq(priv->emac_irq, ravb_interrupt, - IRQF_SHARED, ndev->name, ndev); + if (priv->chip_id == RCAR_GEN2) { + error = request_irq(ndev->irq, ravb_interrupt, IRQF_SHARED, + ndev->name, ndev); if (error) { netdev_err(ndev, "cannot request IRQ\n"); - goto out_free_irq; + goto out_napi_off; } + } else { + error = ravb_hook_irq(ndev->irq, ravb_multi_interrupt, ndev, + dev, "ch22:multi"); + if (error) + goto out_napi_off; + error = ravb_hook_irq(priv->emac_irq, ravb_emac_interrupt, ndev, + dev, "ch24:emac"); + if (error) + goto out_free_irq; + error = ravb_hook_irq(priv->rx_irqs[RAVB_BE], ravb_be_interrupt, + ndev, dev, "ch0:rx_be"); + if (error) + goto out_free_irq_emac; + error = ravb_hook_irq(priv->tx_irqs[RAVB_BE], ravb_be_interrupt, + ndev, dev, "ch18:tx_be"); + if (error) + goto out_free_irq_be_rx; + error = ravb_hook_irq(priv->rx_irqs[RAVB_NC], ravb_nc_interrupt, + ndev, dev, "ch1:rx_nc"); + if (error) + goto out_free_irq_be_tx; + error = ravb_hook_irq(priv->tx_irqs[RAVB_NC], ravb_nc_interrupt, + ndev, dev, "ch19:tx_nc"); + if (error) + goto out_free_irq_nc_rx; } /* Device init */ @@ -1257,8 +1412,17 @@ out_ptp_stop: if (priv->chip_id == RCAR_GEN2) ravb_ptp_stop(ndev); out_free_irq2: - if (priv->chip_id == RCAR_GEN3) - free_irq(priv->emac_irq, ndev); + if (priv->chip_id == RCAR_GEN2) + goto out_free_irq; + free_irq(priv->tx_irqs[RAVB_NC], ndev); +out_free_irq_nc_rx: + free_irq(priv->rx_irqs[RAVB_NC], ndev); +out_free_irq_be_tx: + free_irq(priv->tx_irqs[RAVB_BE], ndev); +out_free_irq_be_rx: + free_irq(priv->rx_irqs[RAVB_BE], ndev); +out_free_irq_emac: + free_irq(priv->emac_irq, ndev); out_free_irq: free_irq(ndev->irq, ndev); out_napi_off: @@ -1712,6 +1876,7 @@ static int ravb_probe(struct platform_device *pdev) struct net_device *ndev; int error, irq, q; struct resource *res; + int i; if (!np) { dev_err(&pdev->dev, @@ -1782,6 +1947,22 @@ static int ravb_probe(struct platform_device *pdev) goto out_release; } priv->emac_irq = irq; + for (i = 0; i < NUM_RX_QUEUE; i++) { + irq = platform_get_irq_byname(pdev, ravb_rx_irqs[i]); + if (irq < 0) { + error = irq; + goto out_release; + } + priv->rx_irqs[i] = irq; + } + for (i = 0; i < NUM_TX_QUEUE; i++) { + irq = platform_get_irq_byname(pdev, ravb_tx_irqs[i]); + if (irq < 0) { + error = irq; + goto out_release; + } + priv->tx_irqs[i] = irq; + } } priv->chip_id = chip_id; diff --git a/drivers/net/ethernet/renesas/ravb_ptp.c b/drivers/net/ethernet/renesas/ravb_ptp.c index 57992cc..7cb1cc4 100644 --- a/drivers/net/ethernet/renesas/ravb_ptp.c +++ b/drivers/net/ethernet/renesas/ravb_ptp.c @@ -194,7 +194,14 @@ static int ravb_ptp_extts(struct ptp_clock_info *ptp, priv->ptp.extts[req->index] = on; spin_lock_irqsave(&priv->lock, flags); - ravb_modify(ndev, GIC, GIC_PTCE, on ? GIC_PTCE : 0); + if (priv->chip_id == RCAR_GEN2) { + ravb_modify(ndev, GIC, GIC_PTCE, on ? GIC_PTCE : 0); + } else { + if (on) + ravb_write(ndev, GIE_PTCS, GIE); + else + ravb_write(ndev, GID_PTCD, GID); + } mmiowb(); spin_unlock_irqrestore(&priv->lock, flags); @@ -241,7 +248,10 @@ static int ravb_ptp_perout(struct ptp_clock_info *ptp, error = ravb_ptp_update_compare(priv, (u32)start_ns); if (!error) { /* Unmask interrupt */ - ravb_modify(ndev, GIC, GIC_PTME, GIC_PTME); + if (priv->chip_id == RCAR_GEN2) + ravb_modify(ndev, GIC, GIC_PTME, GIC_PTME); + else + ravb_write(ndev, GIE_PTMS0, GIE); } } else { spin_lock_irqsave(&priv->lock, flags); @@ -250,7 +260,10 @@ static int ravb_ptp_perout(struct ptp_clock_info *ptp, perout->period = 0; /* Mask interrupt */ - ravb_modify(ndev, GIC, GIC_PTME, 0); + if (priv->chip_id == RCAR_GEN2) + ravb_modify(ndev, GIC, GIC_PTME, 0); + else + ravb_write(ndev, GID_PTMD0, GID); } mmiowb(); spin_unlock_irqrestore(&priv->lock, flags);