Hello.

On 12/17/2015 07:29 PM, Yoshihiro Kaneko wrote:

From: Kazuya Mizuguchi <kazuya.mizuguchi...@renesas.com>

This patch supports the following interrupts.

- One interrupt for multiple (descriptor, error, management)
- One interrupt for emac
- Four interrupts for dma queue (best effort rx/tx, network control rx/tx)


    You don't say why the current 2-interrupt scheme (implemented by Simon's
patch) isn't enpough...

Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi...@renesas.com>
Signed-off-by: Yoshihiro Kaneko <ykaneko0...@gmail.com>

[...]

diff --git a/drivers/net/ethernet/renesas/ravb.h
b/drivers/net/ethernet/renesas/ravb.h
index 9fbe92a..eada5a1 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -157,6 +157,7 @@ enum ravb_reg {
[...]
+#define RAVB_RIx2_ALL  0x8003ffff

    No prefix. And there's bit 31 in this register, according to my gen3
manual. So, your _ALL isn't really "all bits". I'd rather call it RIx2_QFx.
Or even RIE2_QFS and RID2_QFD.

I think that bit 31 is included in the value 0x8003ffff. Or I'm
missing something?

   Sorry, I misread the code.

+
+/* TIx */

    TIE/TID.

+#define RAVB_TIx_ALL   0x000fffff

    No prefix. And there's bit 31 in this register, according to my gen3
manual.

   Oops, no bit 31 in these regs.

So, your _ALL isn't really "all bits".

I think the correct value is 0x000f0f0f.

   Indeed, please fix.

[...]

diff --git a/drivers/net/ethernet/renesas/ravb_main.c
b/drivers/net/ethernet/renesas/ravb_main.c
index 120cc25..753b67d 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
[...]
@@ -654,7 +679,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(struct net_device *ndev)


    ravb_emac_interrupt_[un]locked() perhaps? Not sure which is more
correct... :-)

How about ravb_process_emac_interrupt() ?

   I've made up my mind -- I'd prefer ravb_emac_interrupt_unlocked().

[...]
+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;
         }

+       /* Management */


    Really? I thought that's gPTP Interrupt...

gPTP seems to be a part of Management related interrupts.

   ISS.CGIM is still described as gPTP interrupt mirror in my gen3 manual.

         if (iss & ISS_CGIS)
                 result = ravb_ptp_interrupt(ndev);

@@ -776,6 +843,55 @@ static irqreturn_t ravb_interrupt(int irq, void
*dev_id)
[...]

Thanks,
kaneko

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to