Hello.
On 12/20/2015 12:15 PM, Yoshihiro Kaneko wrote:
From: Kazuya Mizuguchi <[email protected]>
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 still don't say why it's better than the current scheme...
Signed-off-by: Kazuya Mizuguchi <[email protected]>
Signed-off-by: Yoshihiro Kaneko <[email protected]>
---
This patch is based on the master branch of David Miller's next networking
tree.
v2 [Yoshihiro Kaneko]
* compile tested only
* As suggested by Sergei Shtylyov
- add comment to CIE
- remove comments from CIE bits
- fix value of TIx_ALL
- define each bits for CIE, GIE, GID, RIE0, RID0, RIE2, RID2, TIE, TID
- reversed Christmas tree declaration ordered
- rename _ravb_emac_interrupt() to ravb_emac_interrupt_unlocked()
- remove unnecessary clearing of CIE
- use a bit name corresponding to the target register, RIE0, RIE2, TIE,
TID, RID2, GID, GIE
drivers/net/ethernet/renesas/ravb.h | 213 ++++++++++++++++++++++++++
drivers/net/ethernet/renesas/ravb_main.c | 247 +++++++++++++++++++++++++++----
drivers/net/ethernet/renesas/ravb_ptp.c | 45 ++++--
3 files changed, 464 insertions(+), 41 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h
b/drivers/net/ethernet/renesas/ravb.h
index 9fbe92a..71badd6d 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,
+ GID = 0x03D0,
+ DIL = 0x0440,
+ RIE0 = 0x0460,
+ RID0 = 0x0464,
+ RIE2 = 0x0470,
+ RID2 = 0x0474,
+ TIE = 0x0478,
+ TID = 0x047c,
So you only commented on CIE and considered it done? :-)
[...]
@@ -411,14 +422,27 @@ static int ravb_dmac_init(struct net_device *ndev)
ravb_write(ndev, TCCR_TFEN, TCCR);
/* Interrupt init: */
- /* Frame receive */
- ravb_write(ndev, RIC0_FRE0 | RIC0_FRE1, RIC0);
- /* Disable FIFO full warning */
- ravb_write(ndev, 0, RIC1);
- /* Receive FIFO full error, descriptor empty */
- ravb_write(ndev, RIC2_QFE0 | RIC2_QFE1 | RIC2_RFFE, RIC2);
- /* Frame transmitted, timestamp FIFO updated */
- ravb_write(ndev, TIC_FTE0 | TIC_FTE1 | TIC_TFUE, TIC);
+ if (priv->chip_id == RCAR_GEN2) {
+ /* Frame receive */
+ ravb_write(ndev, RIC0_FRE0 | RIC0_FRE1, RIC0);
+ /* Disable FIFO full warning */
+ ravb_write(ndev, 0, RIC1);
+ /* Receive FIFO full error, descriptor empty */
+ ravb_write(ndev, RIC2_QFE0 | RIC2_QFE1 | RIC2_RFFE, RIC2);
+ /* Frame transmitted, timestamp FIFO updated */
+ ravb_write(ndev, TIC_FTE0 | TIC_FTE1 | TIC_TFUE, TIC);
+ } else {
+ /* 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, RIE0_FRS0 | RIE0_FRS1, RIE0);
+ /* Receive FIFO full error, descriptor empty */
+ ravb_write(ndev, RIE2_QFS0 | RIE2_QFS1 | RIE2_RFFS, RIE2);
+ /* Frame transmitted, timestamp FIFO updated */
+ ravb_write(ndev, TIE_FTS0 | TIE_FTS1 | TIE_TFUS, TIE);
+ }
So in this case for gen3 we enable interrupts we need in addition to already
enabled (by a boot loader perhaps)? I don't think you actually want it...
[...]
@@ -690,7 +726,10 @@ static void ravb_error_interrupt(struct net_device *ndev)
ravb_write(ndev, ~EIS_QFS, EIS);
if (eis & EIS_QFS) {
ris2 = ravb_read(ndev, RIS2);
- ravb_write(ndev, ~(RIS2_QFF0 | RIS2_RFFF), RIS2);
+ if (priv->chip_id == RCAR_GEN2)
+ ravb_write(ndev, ~(RIS2_QFF0 | RIS2_RFFF), RIS2);
+ else
+ ravb_write(ndev, RID2_QFD0 | RID2_RFFD, RID2);
Err, aren't you doing 2 different things for gen2 and gen3 here. For gen2
you're clearing the QFF0/RFFF interrupts, for gen3 you're disabling them, no?
[...]
@@ -758,16 +797,43 @@ static irqreturn_t ravb_interrupt(int irq, void *dev_id)
[...]
+/* 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;
}
+ /* Management */
I still doubt this comment is valid...
if (iss & ISS_CGIS)
result = ravb_ptp_interrupt(ndev);
[...]
@@ -1223,23 +1343,68 @@ static const struct ethtool_ops ravb_ethtool_ops = {
static int ravb_open(struct net_device *ndev)
{
struct ravb_private *priv = netdev_priv(ndev);
- int error;
+ struct platform_device *pdev = priv->pdev;
+ struct device *dev = &pdev->dev;
+ int error, i;
+ char *name;
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_napi_off;
+ }
+ } else {
+ name = devm_kasprintf(dev, GFP_KERNEL, "%s:ch22:multi",
+ ndev->name);
+ error = request_irq(ndev->irq, ravb_multi_interrupt,
+ IRQF_SHARED, name, ndev);
+ if (error) {
+ netdev_err(ndev, "cannot request IRQ %s\n", name);
+ goto out_napi_off;
+ }
+ name = devm_kasprintf(dev, GFP_KERNEL, "%s:ch24:emac",
+ ndev->name);
+ error = request_irq(priv->emac_irq, ravb_emac_interrupt,
+ IRQF_SHARED, name, ndev);
+ if (error) {
+ netdev_err(ndev, "cannot request IRQ %s\n", name);
+ goto out_free_irq;
+ }
+ name = devm_kasprintf(dev, GFP_KERNEL, "%s:ch0:rx_be",
+ ndev->name);
+ error = request_irq(priv->rx_irqs[RAVB_BE], ravb_be_interrupt,
+ IRQF_SHARED, name, ndev);
+ if (error) {
+ netdev_err(ndev, "cannot request IRQ %s\n", name);
+ goto out_free_irq;
+ }
+ name = devm_kasprintf(dev, GFP_KERNEL, "%s:ch18:tx_be",
+ ndev->name);
+ error = request_irq(priv->tx_irqs[RAVB_BE], ravb_be_interrupt,
+ IRQF_SHARED, name, ndev);
+ if (error) {
+ netdev_err(ndev, "cannot request IRQ %s\n", name);
+ goto out_free_irq;
+ }
+ name = devm_kasprintf(dev, GFP_KERNEL, "%s:ch1:rx_nc",
+ ndev->name);
+ error = request_irq(priv->rx_irqs[RAVB_NC], ravb_nc_interrupt,
+ IRQF_SHARED, name, ndev);
+ if (error) {
+ netdev_err(ndev, "cannot request IRQ %s\n", name);
+ goto out_free_irq;
+ }
+ name = devm_kasprintf(dev, GFP_KERNEL, "%s:ch19:tx_nc",
+ ndev->name);
+ error = request_irq(priv->tx_irqs[RAVB_NC], ravb_nc_interrupt,
+ IRQF_SHARED, name, ndev);
+ if (error) {
+ netdev_err(ndev, "cannot request IRQ %s\n", name);
goto out_free_irq;
}
This error case is repetitive, couldn't you consolidate it somehow?
[...]
@@ -1494,10 +1663,15 @@ static int ravb_close(struct net_device *ndev)
netif_tx_stop_all_queues(ndev);
/* Disable interrupts by clearing the interrupt masks. */
- ravb_write(ndev, 0, RIC0);
- ravb_write(ndev, 0, RIC2);
- ravb_write(ndev, 0, TIC);
-
+ if (priv->chip_id == RCAR_GEN2) {
+ ravb_write(ndev, 0, RIC0);
+ ravb_write(ndev, 0, RIC2);
+ ravb_write(ndev, 0, TIC);
+ } else {
+ ravb_write(ndev, RID0_ALL, RID0);
+ ravb_write(ndev, RID2_ALL, RID2);
+ ravb_write(ndev, TID_ALL, TID);
+ }
Don't see why this is better than the old code. We don't care about
atomicity here AFAIU.
[...]
@@ -1798,6 +1973,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;
+ }
Perhaps it would better to use sprintf() here, in both loops...
[...]
diff --git a/drivers/net/ethernet/renesas/ravb_ptp.c
b/drivers/net/ethernet/renesas/ravb_ptp.c
index 7a8ce92..870d7b7 100644
--- a/drivers/net/ethernet/renesas/ravb_ptp.c
+++ b/drivers/net/ethernet/renesas/ravb_ptp.c
[...]
@@ -352,7 +367,11 @@ void ravb_ptp_stop(struct net_device *ndev)
{
struct ravb_private *priv = netdev_priv(ndev);
- ravb_write(ndev, 0, GIC);
+ if (priv->chip_id == RCAR_GEN2)
+ ravb_write(ndev, 0, GIC);
+ else
+ ravb_write(ndev, GID_ALL, GID);
Again, don't see why it's better than the old code.
[...]
MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
