Patchwork ibm_newemac: Fix new MAL feature handling

login
register
mail settings
Submitter Josh Boyer
Date Oct. 16, 2008, 2:38 p.m.
Message ID <20081016143840.GA29805@yoda.jdub.homelinux.org>
Download mbox | patch
Permalink /patch/4708/
State Awaiting Upstream
Delegated to: Jeff Garzik
Headers show

Comments

Josh Boyer - Oct. 16, 2008, 2:38 p.m.
On Thu, Oct 16, 2008 at 03:56:50PM +1100, Benjamin Herrenschmidt wrote:
>> drivers/net/ibm_newemac/mal.c: In function 'mal_txeob':
>> drivers/net/ibm_newemac/mal.c:284: error: implicit declaration of function 'mtdcri'
>> drivers/net/ibm_newemac/mal.c:284: error: 'SDR0' undeclared (first use in this function)
>> drivers/net/ibm_newemac/mal.c:284: error: (Each undeclared identifier is reported only once
>> drivers/net/ibm_newemac/mal.c:284: error: for each function it appears in.)
>> drivers/net/ibm_newemac/mal.c:285: error: implicit declaration of function 'mfdcri'
>> drivers/net/ibm_newemac/mal.c: In function 'mal_rxeob':
>> drivers/net/ibm_newemac/mal.c:302: error: 'SDR0' undeclared (first use in this function)
>
>That's annoying, I'll have a look.

Support for new features needed by the PPC 405EZ boards
introduced some errors in the MAL and EMAC feature handling.
This broke 'allmodconfig' builds as CONFIG_PPC_DCR_NATIVE is
not set for those.

This patch fixes these errors by wrapping the code in the
appropriate #ifdefs.

Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>

---


--
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
Arnd Bergmann - Oct. 17, 2008, 12:56 p.m.
On Thursday 16 October 2008, Josh Boyer wrote:
> +#ifdef CONFIG_PPC_DCR_NATIVE
>         if (mal_has_feature(mal, MAL_FTR_CLEAR_ICINTSTAT))
>                 mtdcri(SDR0, DCRN_SDR_ICINTSTAT,
>                                 (mfdcri(SDR0, DCRN_SDR_ICINTSTAT) | ICINTSTAT_ICTX));
> +#endif
>  

So what if you have a mmio based DRC implementation and still want to use
this feature?

I guess you really should use dcr_read/dcr_write here instead of mtdcr/mfdcr.

	Arnd <><
--
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
Josh Boyer - Oct. 17, 2008, 1:09 p.m.
On Fri, 17 Oct 2008 14:56:52 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Thursday 16 October 2008, Josh Boyer wrote:
> > +#ifdef CONFIG_PPC_DCR_NATIVE
> >         if (mal_has_feature(mal, MAL_FTR_CLEAR_ICINTSTAT))
> >                 mtdcri(SDR0, DCRN_SDR_ICINTSTAT,
> >                                 (mfdcri(SDR0, DCRN_SDR_ICINTSTAT) | ICINTSTAT_ICTX));
> > +#endif
> >  
> 
> So what if you have a mmio based DRC implementation and still want to use
> this feature?

You have to have hardware that does that.  When it exists, we can
probably worry about it.  Hopefully that will never happen.  (Axon is
an abomination that should not be repeated.)

> I guess you really should use dcr_read/dcr_write here instead of mtdcr/mfdcr.

We can look at that.  It's not going to be high on my TODO list at the
moment though.  (hint, there are other places that do the same thing
here and they should all be cleaned up if we're going to bother.)

josh
--
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
Benjamin Herrenschmidt - Oct. 20, 2008, 11:43 p.m.
On Thu, 2008-10-16 at 10:38 -0400, Josh Boyer wrote:
> On Thu, Oct 16, 2008 at 03:56:50PM +1100, Benjamin Herrenschmidt wrote:
> >> drivers/net/ibm_newemac/mal.c: In function 'mal_txeob':
> >> drivers/net/ibm_newemac/mal.c:284: error: implicit declaration of function 'mtdcri'
> >> drivers/net/ibm_newemac/mal.c:284: error: 'SDR0' undeclared (first use in this function)
> >> drivers/net/ibm_newemac/mal.c:284: error: (Each undeclared identifier is reported only once
> >> drivers/net/ibm_newemac/mal.c:284: error: for each function it appears in.)
> >> drivers/net/ibm_newemac/mal.c:285: error: implicit declaration of function 'mfdcri'
> >> drivers/net/ibm_newemac/mal.c: In function 'mal_rxeob':
> >> drivers/net/ibm_newemac/mal.c:302: error: 'SDR0' undeclared (first use in this function)
> >
> >That's annoying, I'll have a look.
> 
> Support for new features needed by the PPC 405EZ boards
> introduced some errors in the MAL and EMAC feature handling.
> This broke 'allmodconfig' builds as CONFIG_PPC_DCR_NATIVE is
> not set for those.
> 
> This patch fixes these errors by wrapping the code in the
> appropriate #ifdefs.
> 
> Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

Jeff, this breaks build, I'll put it in powerpc.git for a merge today,

Cheers,
Ben.
Jeff Garzik - Oct. 21, 2008, 5:17 a.m.
Benjamin Herrenschmidt wrote:
> On Thu, 2008-10-16 at 10:38 -0400, Josh Boyer wrote:
>> On Thu, Oct 16, 2008 at 03:56:50PM +1100, Benjamin Herrenschmidt wrote:
>>>> drivers/net/ibm_newemac/mal.c: In function 'mal_txeob':
>>>> drivers/net/ibm_newemac/mal.c:284: error: implicit declaration of function 'mtdcri'
>>>> drivers/net/ibm_newemac/mal.c:284: error: 'SDR0' undeclared (first use in this function)
>>>> drivers/net/ibm_newemac/mal.c:284: error: (Each undeclared identifier is reported only once
>>>> drivers/net/ibm_newemac/mal.c:284: error: for each function it appears in.)
>>>> drivers/net/ibm_newemac/mal.c:285: error: implicit declaration of function 'mfdcri'
>>>> drivers/net/ibm_newemac/mal.c: In function 'mal_rxeob':
>>>> drivers/net/ibm_newemac/mal.c:302: error: 'SDR0' undeclared (first use in this function)
>>> That's annoying, I'll have a look.
>> Support for new features needed by the PPC 405EZ boards
>> introduced some errors in the MAL and EMAC feature handling.
>> This broke 'allmodconfig' builds as CONFIG_PPC_DCR_NATIVE is
>> not set for those.
>>
>> This patch fixes these errors by wrapping the code in the
>> appropriate #ifdefs.
>>
>> Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>
> 
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> 
> Jeff, this breaks build, I'll put it in powerpc.git for a merge today,

ACK


--
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

Patch

diff --git a/drivers/net/ibm_newemac/core.c b/drivers/net/ibm_newemac/core.c
index efcf21c..2ee2622 100644
--- a/drivers/net/ibm_newemac/core.c
+++ b/drivers/net/ibm_newemac/core.c
@@ -2604,8 +2604,16 @@  static int __devinit emac_init_config(struct emac_instance *dev)
 		if (of_device_is_compatible(np, "ibm,emac-440ep") ||
 		    of_device_is_compatible(np, "ibm,emac-440gr"))
 			dev->features |= EMAC_FTR_440EP_PHY_CLK_FIX;
-		if (of_device_is_compatible(np, "ibm,emac-405ez"))
+		if (of_device_is_compatible(np, "ibm,emac-405ez")) {
+#ifdef CONFIG_IBM_NEW_EMAC_NO_FLOW_CONTROL
 			dev->features |= EMAC_FTR_NO_FLOW_CONTROL_40x;
+#else
+			printk(KERN_ERR "%s: Flow control not disabled!\n",
+					np->full_name);
+			return -ENXIO;
+#endif
+		}
+
 	}
 
 	/* Fixup some feature bits based on the device tree */
diff --git a/drivers/net/ibm_newemac/mal.c b/drivers/net/ibm_newemac/mal.c
index 1839d3f..ecf9798 100644
--- a/drivers/net/ibm_newemac/mal.c
+++ b/drivers/net/ibm_newemac/mal.c
@@ -280,9 +280,11 @@  static irqreturn_t mal_txeob(int irq, void *dev_instance)
 	mal_schedule_poll(mal);
 	set_mal_dcrn(mal, MAL_TXEOBISR, r);
 
+#ifdef CONFIG_PPC_DCR_NATIVE
 	if (mal_has_feature(mal, MAL_FTR_CLEAR_ICINTSTAT))
 		mtdcri(SDR0, DCRN_SDR_ICINTSTAT,
 				(mfdcri(SDR0, DCRN_SDR_ICINTSTAT) | ICINTSTAT_ICTX));
+#endif
 
 	return IRQ_HANDLED;
 }
@@ -298,9 +300,11 @@  static irqreturn_t mal_rxeob(int irq, void *dev_instance)
 	mal_schedule_poll(mal);
 	set_mal_dcrn(mal, MAL_RXEOBISR, r);
 
+#ifdef CONFIG_PPC_DCR_NATIVE
 	if (mal_has_feature(mal, MAL_FTR_CLEAR_ICINTSTAT))
 		mtdcri(SDR0, DCRN_SDR_ICINTSTAT,
 				(mfdcri(SDR0, DCRN_SDR_ICINTSTAT) | ICINTSTAT_ICRX));
+#endif
 
 	return IRQ_HANDLED;
 }
@@ -572,9 +576,18 @@  static int __devinit mal_probe(struct of_device *ofdev,
 		goto fail;
 	}
 
-	if (of_device_is_compatible(ofdev->node, "ibm,mcmal-405ez"))
+	if (of_device_is_compatible(ofdev->node, "ibm,mcmal-405ez")) {
+#if defined(CONFIG_IBM_NEW_EMAC_MAL_CLR_ICINTSTAT) && \
+		defined(CONFIG_IBM_NEW_EMAC_MAL_COMMON_ERR)
 		mal->features |= (MAL_FTR_CLEAR_ICINTSTAT |
 				MAL_FTR_COMMON_ERR_INT);
+#else
+		printk(KERN_ERR "%s: Support for 405EZ not enabled!\n",
+				ofdev->node->full_name);
+		err = -ENODEV;
+		goto fail;
+#endif
+	}
 
 	mal->txeob_irq = irq_of_parse_and_map(ofdev->node, 0);
 	mal->rxeob_irq = irq_of_parse_and_map(ofdev->node, 1);