From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sergei Shtylyov Date: Wed, 02 Mar 2016 18:42:13 +0000 Subject: Re: [PATCH/RFC v5 net-next] ravb: Add dma queue interrupt support Message-Id: <56D73405.2060806@cogentembedded.com> List-Id: References: <1455478769-16084-1-git-send-email-ykaneko0929@gmail.com> <56CA0A68.2020709@cogentembedded.com> <56D4B02F.7030803@cogentembedded.com> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Yoshihiro Kaneko Cc: netdev@vger.kernel.org, "David S. Miller" , Simon Horman , Magnus Damm , linux-renesas-soc@vger.kernel.org, Linux-sh list Hello. On 03/02/2016 09:32 PM, Yoshihiro Kaneko wrote: >>>>> From: Kazuya Mizuguchi >>>>> >>>>> 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 >>>>> Signed-off-by: Yoshihiro Kaneko [...] >>>>> 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