diff mbox

[PATCHv2,0/9] macb: add support for Cadence GEM

Message ID 20110322175523.GE10058@pulham.picochip.com
State Not Applicable, archived
Delegated to: David Miller
Headers show

Commit Message

Jamie Iles March 22, 2011, 5:55 p.m. UTC
Hi Jean-Christophe,

On Tue, Mar 22, 2011 at 04:39:17PM +0000, Jamie Iles wrote:
> Hi Jean-Christophe,
> 
> On Tue, Mar 22, 2011 at 05:18:11PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 11:18 Mon 21 Mar     , Jamie Iles wrote:
> > > I have an existing tree for this at
> > > 
> > > 	git://github.com/jamieiles/linux-2.6-ji.git macb-gem
> > > 
> > > based off of 2.6.38 (with your ACK's now added) and I'd be happy with 
> > > either route.
> > but we must detect the gem via the version register and ditto for macb for
> > avr32 and at91
> > 
> > so please rebase it over my patch
> > 
> > and you get my sob too
> 
> Would you mind respinning your patch without the changes to the clk 
> stuff?  Otherwise we're changing it from compile time to version based, 
> only to completely remove it in subsequent patches.

Actually, this patch doesn't work anyway as the version register is 
being read before the clks are enabled so the device isn't accessible 
(and the registers haven't yet been ioremap()'d).

> Also, can you confirm that the module ID's that you are using to 
> differentiate between AT91 and AVR32 won't clash with MACB uses in 
> other, non-at91/avr32 chips, or that it doesn't matter?

If this does work, then it would be nice if we made the else path of the 
RMII AT91 tests also test for avr32 too so we aren't driving the USRIO 
pins on platforms that aren't at91 but also aren't avr32.  So something 
the patch below instead.

Jamie

8<---

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

Comments

Jean-Christophe PLAGNIOL-VILLARD March 24, 2011, 4:25 p.m. UTC | #1
On 17:55 Tue 22 Mar     , Jamie Iles wrote:
> Hi Jean-Christophe,
> 
> On Tue, Mar 22, 2011 at 04:39:17PM +0000, Jamie Iles wrote:
> > Hi Jean-Christophe,
> > 
> > On Tue, Mar 22, 2011 at 05:18:11PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > On 11:18 Mon 21 Mar     , Jamie Iles wrote:
> > > > I have an existing tree for this at
> > > > 
> > > > 	git://github.com/jamieiles/linux-2.6-ji.git macb-gem
> > > > 
> > > > based off of 2.6.38 (with your ACK's now added) and I'd be happy with 
> > > > either route.
> > > but we must detect the gem via the version register and ditto for macb for
> > > avr32 and at91
> > > 
> > > so please rebase it over my patch
> > > 
> > > and you get my sob too
> > 
> > Would you mind respinning your patch without the changes to the clk 
> > stuff?  Otherwise we're changing it from compile time to version based, 
> > only to completely remove it in subsequent patches.
> 
> Actually, this patch doesn't work anyway as the version register is 
> being read before the clks are enabled so the device isn't accessible 
> (and the registers haven't yet been ioremap()'d).
yeah I found it also I forget to put he RFC in the patch title as I want just to
give the way to detect it the way to detect it for avr32 and at91
so we can remove the ifdef fully
> 
> > Also, can you confirm that the module ID's that you are using to 
> > differentiate between AT91 and AVR32 won't clash with MACB uses in 
> > other, non-at91/avr32 chips, or that it doesn't matter?
> 
> If this does work, then it would be nice if we made the else path of the 
> RMII AT91 tests also test for avr32 too so we aren't driving the USRIO 
> pins on platforms that aren't at91 but also aren't avr32.  So something 
> the patch below instead.
I'm currently traveling so I can not test it yet will test it next week
but looks good

Best Regards,
J.
--
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
Jamie Iles March 31, 2011, 9:40 a.m. UTC | #2
Hi Jean-Christophe,

On Thu, Mar 24, 2011 at 05:25:17PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 17:55 Tue 22 Mar     , Jamie Iles wrote:
> > Hi Jean-Christophe,
> > > Also, can you confirm that the module ID's that you are using to 
> > > differentiate between AT91 and AVR32 won't clash with MACB uses in 
> > > other, non-at91/avr32 chips, or that it doesn't matter?
> > 
> > If this does work, then it would be nice if we made the else path of the 
> > RMII AT91 tests also test for avr32 too so we aren't driving the USRIO 
> > pins on platforms that aren't at91 but also aren't avr32.  So something 
> > the patch below instead.
> I'm currently traveling so I can not test it yet will test it next week
> but looks good

Just wondering if you'd had chance to test.  If so, would you mind 
reposting and I'll rebase my patches on top of it.

I also have patches for checksum offloading, receive packet timestamping 
and wake-on-lan support but I'd like to get the basic GEM ones into next 
first.

Jamie
--
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
Jean-Christophe PLAGNIOL-VILLARD April 5, 2011, 10:28 a.m. UTC | #3
On 10:40 Thu 31 Mar     , Jamie Iles wrote:
> Hi Jean-Christophe,
> 
> On Thu, Mar 24, 2011 at 05:25:17PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 17:55 Tue 22 Mar     , Jamie Iles wrote:
> > > Hi Jean-Christophe,
> > > > Also, can you confirm that the module ID's that you are using to 
> > > > differentiate between AT91 and AVR32 won't clash with MACB uses in 
> > > > other, non-at91/avr32 chips, or that it doesn't matter?
> > > 
> > > If this does work, then it would be nice if we made the else path of the 
> > > RMII AT91 tests also test for avr32 too so we aren't driving the USRIO 
> > > pins on platforms that aren't at91 but also aren't avr32.  So something 
> > > the patch below instead.
> > I'm currently traveling so I can not test it yet will test it next week
> > but looks good
> 
> Just wondering if you'd had chance to test.  If so, would you mind 
> reposting and I'll rebase my patches on top of it.
> 
> I also have patches for checksum offloading, receive packet timestamping 
> and wake-on-lan support but I'd like to get the basic GEM ones into next 
> first.
work fine on 9263ek except the IP version detection.

the at91 macb ip version is supposed to be at 0x0601010C but it's not.
At least on 9263 it's 0x0001010C. So we can not detect the arch at runtime
but we can detect that it's a macb.

So could keep the ifdef for 2 archs but use the ip version on arm

Best Regards,
J.
--
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
Jamie Iles April 5, 2011, 10:49 a.m. UTC | #4
On Tue, Apr 05, 2011 at 12:28:42PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> work fine on 9263ek except the IP version detection.
> 
> the at91 macb ip version is supposed to be at 0x0601010C but it's not.
> At least on 9263 it's 0x0001010C. So we can not detect the arch at runtime
> but we can detect that it's a macb.
> 
> So could keep the ifdef for 2 archs but use the ip version on arm

OK, well I think my patches are already doing that so should be OK as 
they are.

Russell, are you able to take these through your tree (I think they 
count as consolidation work) or should I ask Stephen for a tree in 
linux-next for a while first?

Jamie
--
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
Peter Korsgaard April 5, 2011, 11:12 a.m. UTC | #5
>>>>> "Jean-Christophe" == Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> writes:

Hi,

 JC> work fine on 9263ek except the IP version detection.

 JC> the at91 macb ip version is supposed to be at 0x0601010C but it's not.
 JC> At least on 9263 it's 0x0001010C. So we can not detect the arch at runtime
 JC> but we can detect that it's a macb.

Same here on a 9g45.
Jean-Christophe PLAGNIOL-VILLARD April 5, 2011, 11:21 a.m. UTC | #6
On 11:49 Tue 05 Apr     , Jamie Iles wrote:
> On Tue, Apr 05, 2011 at 12:28:42PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > work fine on 9263ek except the IP version detection.
> > 
> > the at91 macb ip version is supposed to be at 0x0601010C but it's not.
> > At least on 9263 it's 0x0001010C. So we can not detect the arch at runtime
> > but we can detect that it's a macb.
> > 
> > So could keep the ifdef for 2 archs but use the ip version on arm
> 
> OK, well I think my patches are already doing that so should be OK as 
> they are.
> 
> Russell, are you able to take these through your tree (I think they 
> count as consolidation work) or should I ask Stephen for a tree in 
> linux-next for a while first?
no please do not us the is_gem but the same way as I did in the ip detection
keep the version register and then check it.

as this ip can be used on other arch we do not want to see thousands of is_xxx

Best Regards,
J.
--
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
Jamie Iles April 5, 2011, 11:47 a.m. UTC | #7
On Tue, Apr 05, 2011 at 01:21:02PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 11:49 Tue 05 Apr     , Jamie Iles wrote:
> > On Tue, Apr 05, 2011 at 12:28:42PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > work fine on 9263ek except the IP version detection.
> > > 
> > > the at91 macb ip version is supposed to be at 0x0601010C but it's not.
> > > At least on 9263 it's 0x0001010C. So we can not detect the arch at runtime
> > > but we can detect that it's a macb.
> > > 
> > > So could keep the ifdef for 2 archs but use the ip version on arm
> > 
> > OK, well I think my patches are already doing that so should be OK as 
> > they are.
> > 
> > Russell, are you able to take these through your tree (I think they 
> > count as consolidation work) or should I ask Stephen for a tree in 
> > linux-next for a while first?
> no please do not us the is_gem but the same way as I did in the ip detection
> keep the version register and then check it.
> 
> as this ip can be used on other arch we do not want to see thousands 
> of is_xxx

But GEM isn't an architecture/machine type, it's a new Cadence Ethernet 
controller that follows on from MACB, not some arch specific tweaks so 
we really only have two options - MACB or GEM.

Still, if it's important to you then I'll make the change.

Jamie
--
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
Jean-Christophe PLAGNIOL-VILLARD April 5, 2011, 11:57 a.m. UTC | #8
On 12:47 Tue 05 Apr     , Jamie Iles wrote:
> On Tue, Apr 05, 2011 at 01:21:02PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 11:49 Tue 05 Apr     , Jamie Iles wrote:
> > > On Tue, Apr 05, 2011 at 12:28:42PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > > work fine on 9263ek except the IP version detection.
> > > > 
> > > > the at91 macb ip version is supposed to be at 0x0601010C but it's not.
> > > > At least on 9263 it's 0x0001010C. So we can not detect the arch at runtime
> > > > but we can detect that it's a macb.
> > > > 
> > > > So could keep the ifdef for 2 archs but use the ip version on arm
> > > 
> > > OK, well I think my patches are already doing that so should be OK as 
> > > they are.
> > > 
> > > Russell, are you able to take these through your tree (I think they 
> > > count as consolidation work) or should I ask Stephen for a tree in 
> > > linux-next for a while first?
> > no please do not us the is_gem but the same way as I did in the ip detection
> > keep the version register and then check it.
> > 
> > as this ip can be used on other arch we do not want to see thousands 
> > of is_xxx
> 
> But GEM isn't an architecture/machine type, it's a new Cadence Ethernet 
> controller that follows on from MACB, not some arch specific tweaks so 
> we really only have two options - MACB or GEM.
I agree but you can have this ip in other soc so we need to try to make it
as generic as possible
and socs can have specific tweaks
So use the version register and mask it is better in my mind and more flexible
It's a shame that we can not detect the diff between avr32 and at91 via ip
version.

I hope that ip variation could be detected via version register.
Next time.
> 
> Still, if it's important to you then I'll make the change.
If you don't ming please

Best Regards,
J.
--
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
diff mbox

Patch

diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index 79ccb54..55b81f5 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -22,7 +22,6 @@ 
 #include <linux/phy.h>
 
 #include <mach/board.h>
-#include <mach/cpu.h>
 
 #include "macb.h"
 
@@ -1169,6 +1168,7 @@  static int __init macb_probe(struct platform_device *pdev)
 		err = -ENOMEM;
 		goto err_out_disable_clocks;
 	}
+	bp->version = macb_readl(bp, VERSION);
 
 	dev->irq = platform_get_irq(pdev, 0);
 	err = request_irq(dev->irq, macb_interrupt, IRQF_SAMPLE_RANDOM,
@@ -1201,18 +1201,18 @@  static int __init macb_probe(struct platform_device *pdev)
 	macb_get_hwaddr(bp);
 	pdata = pdev->dev.platform_data;
 
-	if (pdata && pdata->is_rmii)
-#if defined(CONFIG_ARCH_AT91)
-		macb_writel(bp, USRIO, (MACB_BIT(RMII) | MACB_BIT(CLKEN)) );
-#else
-		macb_writel(bp, USRIO, 0);
-#endif
-	else
-#if defined(CONFIG_ARCH_AT91)
-		macb_writel(bp, USRIO, MACB_BIT(CLKEN));
-#else
-		macb_writel(bp, USRIO, MACB_BIT(MII));
-#endif
+	if (pdata && pdata->is_rmii) {
+		if (macb_is_at91(bp))
+			macb_writel(bp, USRIO,
+				    (MACB_BIT(RMII) | MACB_BIT(CLKEN)));
+		else if (macb_is_avr32(bp))
+			macb_writel(bp, USRIO, 0);
+	} else {
+		if (macb_is_at91(bp))
+			macb_writel(bp, USRIO, MACB_BIT(CLKEN));
+		else if (macb_is_avr32(bp))
+			macb_writel(bp, USRIO, MACB_BIT(MII));
+	}
 
 	bp->tx_pending = DEF_TX_RING_PENDING;
 
diff --git a/drivers/net/macb.h b/drivers/net/macb.h
index d3212f6..56a4fcb 100644
--- a/drivers/net/macb.h
+++ b/drivers/net/macb.h
@@ -59,6 +59,7 @@ 
 #define MACB_TPQ				0x00bc
 #define MACB_USRIO				0x00c0
 #define MACB_WOL				0x00c4
+#define MACB_VERSION				0x00fc
 
 /* Bitfields in NCR */
 #define MACB_LB_OFFSET				0
@@ -389,6 +390,14 @@  struct macb {
 	unsigned int 		link;
 	unsigned int 		speed;
 	unsigned int 		duplex;
+
+	uint32_t		version;
 };
 
+#define MACB_VERSION_MASK	0xffff0000
+#define macb_is_at91(bp)	\
+	(((bp)->version & MACB_VERSION_MASK) == 0x06010000)
+#define macb_is_avr32(bp)	\
+	(((bp)->version & MACB_VERSION_MASK) == 0x00010000)
+
 #endif /* _MACB_H */