From: Simon Horman <horms@verge.net.au>
To: Josh Boyer <jwboyer@linux.vnet.ibm.com>
Cc: benh@kernel.crashing.org, netdev@vger.kernel.org,
linuxppc-dev@ozlabs.org
Subject: Re: [PATCH 3/3] ibm_newemac: MAL support for PowerPC 405EZ
Date: Fri, 5 Sep 2008 12:10:37 +1000 [thread overview]
Message-ID: <20080905021037.GG14128@verge.net.au> (raw)
In-Reply-To: <20080904150216.GD2479@yoda.jdub.homelinux.org>
On Thu, Sep 04, 2008 at 11:02:16AM -0400, Josh Boyer wrote:
> The PowerPC 405EZ SoC has some differences in the interrupt layout and
> handling for the MAL. The SERR, TXDE, and RXDE interrupts are OR'd into
> a single interrupt. Also, due to the possibility for interrupt coalescing,
> the TXEOB and RXEOB interrupts require an interrupt bit to be cleared in
> the ICINTSTAT SDR.
>
> This sets the proper MAL feature bits for 405EZ boards, and adds a common
> shared handler for SERR, TXDE, and RXDE. This has been adapted from code
> originally written by Stefan Roese.
>
> Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>
> ---
> drivers/net/ibm_newemac/mal.c | 98 ++++++++++++++++++++++++++++++++--------
> 1 files changed, 78 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/ibm_newemac/mal.c b/drivers/net/ibm_newemac/mal.c
> index 10c267b..3cef534 100644
> --- a/drivers/net/ibm_newemac/mal.c
> +++ b/drivers/net/ibm_newemac/mal.c
> @@ -28,6 +28,7 @@
> #include <linux/delay.h>
>
> #include "core.h"
> +#include <asm/dcr-regs.h>
>
> static int mal_count;
>
> @@ -279,6 +280,9 @@ static irqreturn_t mal_txeob(int irq, void *dev_instance)
> mal_schedule_poll(mal);
> set_mal_dcrn(mal, MAL_TXEOBISR, r);
>
> + if (mal_has_feature(mal, MAL_FTR_CLEAR_ICINTSTAT))
> + mtdcri(SDR0, 0x4510, (mfdcri(SDR0, 0x4510) | 0x60000000));
> +
> return IRQ_HANDLED;
> }
>
> @@ -293,6 +297,9 @@ static irqreturn_t mal_rxeob(int irq, void *dev_instance)
> mal_schedule_poll(mal);
> set_mal_dcrn(mal, MAL_RXEOBISR, r);
>
> + if (mal_has_feature(mal, MAL_FTR_CLEAR_ICINTSTAT))
> + mtdcri(SDR0, 0x4510, (mfdcri(SDR0, 0x4510) | 0x80000000));
> +
> return IRQ_HANDLED;
> }
>
> @@ -336,6 +343,25 @@ static irqreturn_t mal_rxde(int irq, void *dev_instance)
> return IRQ_HANDLED;
> }
>
> +static irqreturn_t mal_int(int irq, void *dev_instance)
> +{
> + struct mal_instance *mal = dev_instance;
> + u32 esr = get_mal_dcrn(mal, MAL_ESR);
> +
> + if (esr & MAL_ESR_EVB) {
> + /* descriptor error */
> + if (esr & MAL_ESR_DE) {
> + if (esr & MAL_ESR_CIDT)
> + return (mal_rxde(irq, dev_instance));
Return statements shouldn't be enlosed in brackets according to
checkpatch.pl. Also in a few other places.
> + else
> + return (mal_txde(irq, dev_instance));
> + } else { /* SERR */
> + return (mal_serr(irq, dev_instance));
> + }
> + }
> + return IRQ_HANDLED;
> +}
> +
> void mal_poll_disable(struct mal_instance *mal, struct mal_commac *commac)
> {
> /* Spinlock-type semantics: only one caller disable poll at a time */
> @@ -542,11 +568,22 @@ static int __devinit mal_probe(struct of_device *ofdev,
> goto fail;
> }
>
> - mal->txeob_irq = irq_of_parse_and_map(ofdev->node, 0);
> - mal->rxeob_irq = irq_of_parse_and_map(ofdev->node, 1);
> - mal->serr_irq = irq_of_parse_and_map(ofdev->node, 2);
> - mal->txde_irq = irq_of_parse_and_map(ofdev->node, 3);
> - mal->rxde_irq = irq_of_parse_and_map(ofdev->node, 4);
> + if (of_device_is_compatible(ofdev->node, "ibm,mcmal-405ez"))
> + mal->features |= (MAL_FTR_CLEAR_ICINTSTAT | MAL_FTR_COMMON_ERR_INT);
The above like is >80 characters wide.
But I'm not sure that anyone cares.
> +
> + if (mal_has_feature(mal, MAL_FTR_COMMON_ERR_INT)) {
> + mal->txeob_irq = irq_of_parse_and_map(ofdev->node, 0);
> + mal->rxeob_irq = irq_of_parse_and_map(ofdev->node, 1);
> + mal->serr_irq = irq_of_parse_and_map(ofdev->node, 2);
> + mal->txde_irq = mal->rxde_irq = mal->serr_irq;
> + } else {
> + mal->txeob_irq = irq_of_parse_and_map(ofdev->node, 0);
> + mal->rxeob_irq = irq_of_parse_and_map(ofdev->node, 1);
> + mal->serr_irq = irq_of_parse_and_map(ofdev->node, 2);
> + mal->txde_irq = irq_of_parse_and_map(ofdev->node, 3);
> + mal->rxde_irq = irq_of_parse_and_map(ofdev->node, 4);
> + }
It seems that that first three calls to irq_of_parse_and_map() could
be moved outside of the if/else clause.
mal->txeob_irq = irq_of_parse_and_map(ofdev->node, 0);
mal->rxeob_irq = irq_of_parse_and_map(ofdev->node, 1);
mal->serr_irq = irq_of_parse_and_map(ofdev->node, 2);
if (mal_has_feature(mal, MAL_FTR_COMMON_ERR_INT)) {
mal->txde_irq = mal->rxde_irq = mal->serr_irq;
} else {
mal->txde_irq = irq_of_parse_and_map(ofdev->node, 3);
mal->rxde_irq = irq_of_parse_and_map(ofdev->node, 4);
}
> +
> if (mal->txeob_irq == NO_IRQ || mal->rxeob_irq == NO_IRQ ||
> mal->serr_irq == NO_IRQ || mal->txde_irq == NO_IRQ ||
> mal->rxde_irq == NO_IRQ) {
> @@ -608,21 +645,42 @@ static int __devinit mal_probe(struct of_device *ofdev,
> sizeof(struct mal_descriptor) *
> mal_rx_bd_offset(mal, i));
>
> - err = request_irq(mal->serr_irq, mal_serr, 0, "MAL SERR", mal);
> - if (err)
> - goto fail2;
> - err = request_irq(mal->txde_irq, mal_txde, 0, "MAL TX DE", mal);
> - if (err)
> - goto fail3;
> - err = request_irq(mal->txeob_irq, mal_txeob, 0, "MAL TX EOB", mal);
> - if (err)
> - goto fail4;
> - err = request_irq(mal->rxde_irq, mal_rxde, 0, "MAL RX DE", mal);
> - if (err)
> - goto fail5;
> - err = request_irq(mal->rxeob_irq, mal_rxeob, 0, "MAL RX EOB", mal);
> - if (err)
> - goto fail6;
> + if (mal_has_feature(mal, MAL_FTR_COMMON_ERR_INT)) {
> + err = request_irq(mal->serr_irq, mal_int, IRQF_SHARED,
> + "MAL SERR", mal);
> + if (err)
> + goto fail2;
> + err = request_irq(mal->txde_irq, mal_int, IRQF_SHARED,
> + "MAL TX DE", mal);
> + if (err)
> + goto fail3;
> + err = request_irq(mal->txeob_irq, mal_txeob, 0, "MAL TX EOB", mal);
> + if (err)
> + goto fail4;
> + err = request_irq(mal->rxde_irq, mal_int, IRQF_SHARED,
> + "MAL RX DE", mal);
> + if (err)
> + goto fail5;
> + err = request_irq(mal->rxeob_irq, mal_rxeob, 0, "MAL RX EOB", mal);
> + if (err)
> + goto fail6;
> + } else {
> + err = request_irq(mal->serr_irq, mal_serr, 0, "MAL SERR", mal);
> + if (err)
> + goto fail2;
> + err = request_irq(mal->txde_irq, mal_txde, 0, "MAL TX DE", mal);
> + if (err)
> + goto fail3;
> + err = request_irq(mal->txeob_irq, mal_txeob, 0, "MAL TX EOB", mal);
> + if (err)
> + goto fail4;
> + err = request_irq(mal->rxde_irq, mal_rxde, 0, "MAL RX DE", mal);
> + if (err)
> + goto fail5;
> + err = request_irq(mal->rxeob_irq, mal_rxeob, 0, "MAL RX EOB", mal);
> + if (err)
> + goto fail6;
> + }
There seems to be a lot of repention in the above if/else clauses.
I wonder if something like this might be nicer.
if (mal_has_feature(mal, MAL_FTR_COMMON_ERR_INT)) {
irqflags = IRQF_SHARED;
hdlr_serr = hdlr_txde = hdlr_rxde = mal_int;
} else {
irqflags = 0;
hdlr_serr = mal_serr;
hdlr_txde = mal_txde;
hdlr_rxde = mal_rxde;
}
err = request_irq(mal->serr_irq, hdlr_serr, irqflags, "MAL SERR", mal);
if (err)
goto fail2;
err = request_irq(mal->txde_irq, hdlr_txde, irqflags, "MAL TX DE", mal);
if (err)
goto fail3;
err = request_irq(mal->txeob_irq, mal_txeob, 0, "MAL TX EOB", mal);
if (err)
goto fail4;
err = request_irq(mal->rxde_irq, hdlr_rxde, irqflags, "MAL RX DE", mal);
if (err)
goto fail5;
err = request_irq(mal->rxeob_irq, mal_rxeob, 0, "MAL RX EOB", mal);
if (err)
goto fail6;
>
> /* Enable all MAL SERR interrupt sources */
> if (mal->version == 2)
> --
> 1.5.5.1
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2008-09-05 2:10 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <cover.1220540078.git.jwboyer@linux.vnet.ibm.com>
2008-09-04 15:01 ` [PATCH 1/3] ibm_newemac: Allow the "no flow control" EMAC feature to work Josh Boyer
2008-09-04 15:02 ` [PATCH 2/3] ibm_newemac: Introduce mal_has_feature Josh Boyer
2008-09-05 3:19 ` Benjamin Herrenschmidt
2008-09-05 3:37 ` Josh Boyer
2008-09-04 15:02 ` [PATCH 3/3] ibm_newemac: MAL support for PowerPC 405EZ Josh Boyer
2008-09-05 2:10 ` Simon Horman [this message]
2008-09-05 3:41 ` Josh Boyer
2008-09-05 9:34 ` Benjamin Herrenschmidt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20080905021037.GG14128@verge.net.au \
--to=horms@verge.net.au \
--cc=benh@kernel.crashing.org \
--cc=jwboyer@linux.vnet.ibm.com \
--cc=linuxppc-dev@ozlabs.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).