Re: [PATCH 3/3] ibm_newemac: MAL support for PowerPC 405EZ
> > > + 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. > > I don't. If Ben complains I'll change it. I wouldn't do a tatrum over it but if you're going to respin the patch you may as well put the second constant on the next line. Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 3/3] ibm_newemac: MAL support for PowerPC 405EZ
On Fri, 5 Sep 2008 12:10:37 +1000
Simon Horman <[EMAIL PROTECTED]> wrote:
> > +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.
I hate checkpatch, but that's easy enough to fix. Though I don't see
what other places I add with that mistake.
> > + 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.
I don't. If Ben complains I'll change it.
> > +
> > + 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);
> }
Indeed they could. Good catch.
> > +
> > 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_i
Re: [PATCH 3/3] ibm_newemac: MAL support for PowerPC 405EZ
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 <[EMAIL PROTECTED]>
> ---
> 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
>
> #include "core.h"
> +#include
>
> 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) | 0x6000));
> +
> 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) | 0x8000));
> +
> 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(struc
[PATCH 3/3] ibm_newemac: MAL support for PowerPC 405EZ
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 <[EMAIL PROTECTED]>
---
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
#include "core.h"
+#include
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) | 0x6000));
+
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) | 0x8000));
+
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));
+ 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);
+
+ 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);
+ }
+
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)
+
