2016-03-03 3:50 GMT+09:00 Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>: > On 03/02/2016 09:16 PM, Yoshihiro Kaneko wrote: > >>>> From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@xxxxxxxxxxx> >>>> >>>> This patch supports the following interrupts. >>>> >>>> - One interrupt for multiple (timestamp, 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@xxxxxxxxx: define bit names of registers] >>>> [ykaneko0929@xxxxxxxxx: add comment for gen3 only registers] >>>> [ykaneko0929@xxxxxxxxx: fix coding style] >>>> [ykaneko0929@xxxxxxxxx: update changelog] >>>> [ykaneko0929@xxxxxxxxx: gen3: fix initialization of interrupts] >>>> [ykaneko0929@xxxxxxxxx: gen3: fix clearing interrupts] >>>> [ykaneko0929@xxxxxxxxx: gen3: add helper function for request_irq()] >>>> [ykaneko0929@xxxxxxxxx: gen3: remove IRQF_SHARED flag for request_irq()] >>>> [ykaneko0929@xxxxxxxxx: revert ravb_close() and ravb_ptp_stop()] >>>> [ykaneko0929@xxxxxxxxx: avoid calling free_irq() to non-hooked >>>> interrupts] >>>> [ykaneko0929@xxxxxxxxx: make NC/BE interrupt handler a function] >>>> [ykaneko0929@xxxxxxxxx: make timestamp interrupt handler a function] >>>> [ykaneko0929@xxxxxxxxx: timestamp interrupt is handled in multiple >>>> interrupt handler instead of dma queue interrupt handler] >>>> Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@xxxxxxxxxxx> >>>> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@xxxxxxxxx> >>> >>> >>> OK, you are very close now! Just a few comments... >>> > [...] >>>> >>>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c >>>> b/drivers/net/ethernet/renesas/ravb_main.c >>>> index c936682..22ef65d 100644 >>>> --- a/drivers/net/ethernet/renesas/ravb_main.c >>>> +++ b/drivers/net/ethernet/renesas/ravb_main.c > > [...] >>>> >>>> >>>> +static irqreturn_t ravb_rx_tx_interrupt(int irq, void *dev_id, int >>>> ravb_queue) >>> >>> >>> Please, please shorten this 'ravb_queue'... >> >> >> I will fix it. >> >>> Also, would make sense to rename it to ravb_dma_interrupt()... >> >> >> I have renamed it from ravb_dmaq_interrupt() in this version as you >> suggested in the previous review. Did you not mean it? > > > Yes, I meant that, though perhaps got somewhat muddled up. Another > variant is to call the current ravb_queue_interrupt() > ravb_dma_interrupt_unlocked() (after adding the register reads there) > calling this one ravb_dma_interrupt() but I don't insist on the former, > ravb_queue_interrupt() is good enough as is; just rename this function > please. Thanks for the clarification. I understand. I will rename this function to ravb_dma_interrupt(). > >>> [...] >>> >>> Unfortunately, I still can't do a full gen2 regression testing as >>> both >>> Alt and Porter boards don't work with the recent kernel due to AVB_MDIO >>> stuck at 1... But perhaps such testing isn't even necessary. >> >> >> Thanks, >> kaneko > > > MBR, Sergei > Thanks, kaneko
