RE: [PATCH] [UPDATED] tsec: Allow Ten Bit Interface to be configurable

2007-08-14 Thread Joe Hamman
Hi Kumar,

Or do you think it should be added to gianfar_mdio_data?  Easier, but does
it belong there?

Thanks,
Joe


 -Original Message-
 From: linuxppc-embedded-
 [EMAIL PROTECTED] [mailto:linuxppc-
 [EMAIL PROTECTED] On Behalf
 Of Joe Hamman
 Sent: Monday, August 13, 2007 10:30 PM
 To: 'Kumar Gala'
 Cc: linuxppc-embedded@ozlabs.org
 Subject: RE: [PATCH] [UPDATED] tsec: Allow Ten Bit Interface to be
 configurable
 
 
  -Original Message-
  From: Kumar Gala [mailto:[EMAIL PROTECTED]
  Sent: Monday, August 13, 2007 10:10 PM
  To: [EMAIL PROTECTED]
  Cc: linuxppc-embedded@ozlabs.org
  Subject: Re: [PATCH] [UPDATED] tsec: Allow Ten Bit Interface to be
  configurable
 
 
  On Aug 13, 2007, at 5:37 PM, Joe Hamman wrote:
 
   Allow the address of the Ten Bit Interface (TBI) to be changed in the
   event of a conflict with another device.
  
   Signed-off by: Joe Hamman [EMAIL PROTECTED]
   ---
  
   Please ignore the last patch - I missed a cut  paste error on the
   range
   that my testing didn't catch.
 
  I think we'd rather this came from the device tree.
 
 Duh, that makes sense ;-)
 
 Has there been any discussion yet?  Maybe something like this:
 
   [EMAIL PROTECTED] {
   #address-cells = 1;
   #size-cells = 0;
   device_type = mdio;
   compatible = gianfar;
   reg = 24520 20;
   phy0: [EMAIL PROTECTED] {
   interrupt-parent = mpic;
   interrupts = a 1;
   reg = 0;
   device_type = ethernet-phy;
   };
   phy1: [EMAIL PROTECTED] {
   interrupt-parent = mpic;
   interrupts = a 1;
   reg = 1;
   device_type = ethernet-phy;
   };
   phy2: [EMAIL PROTECTED] {
   interrupt-parent = mpic;
   interrupts = a 1;
   reg = 2;
   device_type = ethernet-phy;
   };
   phy3: [EMAIL PROTECTED] {
   interrupt-parent = mpic;
   interrupts = a 1;
   reg = 3;
   device_type = ethernet-phy;
   };
   tbi:  [EMAIL PROTECTED] {
   reg = 1f;
   device_type = ethernet-tbi;
   };
   };
 
 Thanks,
 Joe
 
 
 ___
 Linuxppc-embedded mailing list
 Linuxppc-embedded@ozlabs.org
 https://ozlabs.org/mailman/listinfo/linuxppc-embedded

___
Linuxppc-embedded mailing list
Linuxppc-embedded@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-embedded


Re: [PATCH] [UPDATED] tsec: Allow Ten Bit Interface to be configurable

2007-08-14 Thread Andy Fleming

On Aug 13, 2007, at 22:29, Joe Hamman wrote:



 On Aug 13, 2007, at 5:37 PM, Joe Hamman wrote:

 Allow the address of the Ten Bit Interface (TBI) to be changed in  
 the
 event of a conflict with another device.

 Signed-off by: Joe Hamman [EMAIL PROTECTED]
 ---

 Please ignore the last patch - I missed a cut  paste error on the
 range
 that my testing didn't catch.

 I think we'd rather this came from the device tree.

 Duh, that makes sense ;-)

 Has there been any discussion yet?  Maybe something like this:

   [EMAIL PROTECTED] {


...

   phy3: [EMAIL PROTECTED] {
   interrupt-parent = mpic;
   interrupts = a 1;
   reg = 3;
   device_type = ethernet-phy;
   };
   tbi:  [EMAIL PROTECTED] {
   reg = 1f;
   device_type = ethernet-tbi;
   };

It's actually a per-tsec property.  There's not one tbi, there's one  
per TSEC.  The one on TSEC 0 is special in that it can interfere with  
PHYs on the MDIO bus.

So I would suggest making it a property of the ethernet node:

[EMAIL PROTECTED] {
...
tbipa = 1f;
...
}

etc

Andy


___
Linuxppc-embedded mailing list
Linuxppc-embedded@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-embedded


Re: [PATCH] [UPDATED] tsec: Allow Ten Bit Interface to be configurable

2007-08-14 Thread Scott Wood
Andy Fleming wrote:
 It's actually a per-tsec property.  There's not one tbi, there's one  
 per TSEC.  The one on TSEC 0 is special in that it can interfere with  
 PHYs on the MDIO bus.
 
 So I would suggest making it a property of the ethernet node:
 
 [EMAIL PROTECTED] {
 ...
 tbipa = 1f;
 ...
 }

tbipa isn't likely to pass the Segher test. :-)

If the TBI address is in PHY-space, then it should go in the MDIO bus. 
For the second TSEC, create a second MDIO bus node.

-Scott
___
Linuxppc-embedded mailing list
Linuxppc-embedded@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-embedded


RE: [PATCH] [UPDATED] tsec: Allow Ten Bit Interface to be configurable

2007-08-14 Thread Joe Hamman
 -Original Message-
 From: Scott Wood [mailto:[EMAIL PROTECTED]
 Sent: Tuesday, August 14, 2007 11:16 AM
 To: Andy Fleming
 Cc: [EMAIL PROTECTED]; linuxppc-embedded@ozlabs.org
 Subject: Re: [PATCH] [UPDATED] tsec: Allow Ten Bit Interface to be
 configurable
 
 Andy Fleming wrote:
  It's actually a per-tsec property.  There's not one tbi, there's one
  per TSEC.  The one on TSEC 0 is special in that it can interfere with
  PHYs on the MDIO bus.
 
  So I would suggest making it a property of the ethernet node:
 
  [EMAIL PROTECTED] {
  ...
  tbipa = 1f;
  ...
  }
 
 tbipa isn't likely to pass the Segher test. :-)
 
 If the TBI address is in PHY-space, then it should go in the MDIO bus.
 For the second TSEC, create a second MDIO bus node.
 

How about something like this?

[EMAIL PROTECTED] {
#address-cells = 1;
#size-cells = 0;
device_type = mdio;
compatible = gianfar;
reg = 24520 20;
phy1f: [EMAIL PROTECTED] {
reg = 1f;
device_type = ethernet-phy;
};
phy0: [EMAIL PROTECTED] {
reg = 0;
device_type = ethernet-phy;
};
phy1: [EMAIL PROTECTED] {
reg = 1;
device_type = ethernet-phy;
};
phy2: [EMAIL PROTECTED] {
reg = 2;
device_type = ethernet-phy;
};
tbi1e: [EMAIL PROTECTED] {
reg = 1e;
device_type = ethernet-tbi;
};
};

[EMAIL PROTECTED] {
#address-cells = 1;
#size-cells = 0;
device_type = network;
model = eTSEC;
compatible = gianfar;
reg = 24000 1000;
mac-address = [ 00 E0 0C 00 73 00 ];
interrupts = 1d 2 1e 2 22 2;
interrupt-parent = mpic;
phy-handle = phy1f;
tbi-handle = tbi1e;
};


Joe


___
Linuxppc-embedded mailing list
Linuxppc-embedded@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-embedded


Re: [PATCH] [UPDATED] tsec: Allow Ten Bit Interface to be configurable

2007-08-14 Thread Scott Wood
Joe Hamman wrote:
   [EMAIL PROTECTED] {
   #address-cells = 1;
   #size-cells = 0;
   device_type = network;
   model = eTSEC;
   compatible = gianfar;
   reg = 24000 1000;
   mac-address = [ 00 E0 0C 00 73 00 ];
   interrupts = 1d 2 1e 2 22 2;
   interrupt-parent = mpic;
   phy-handle = phy1f;
   tbi-handle = tbi1e;
   };

Is any given board going to have at runtime (i.e. not jumper selectable) 
  both a phy and a tbi (I'm not very familiar with the latter, so I 
apologize if this is a dumb question).  If not, I'd stick with 
phy-handle and have something in the phy node to indicate that it's tbi.

-Scott
___
Linuxppc-embedded mailing list
Linuxppc-embedded@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-embedded


Re: [PATCH] [UPDATED] tsec: Allow Ten Bit Interface to be configurable

2007-08-14 Thread Andy Fleming

On Aug 14, 2007, at 13:00, Scott Wood wrote:

 Joe Hamman wrote:
  [EMAIL PROTECTED] {
  #address-cells = 1;
  #size-cells = 0;
  device_type = network;
  model = eTSEC;
  compatible = gianfar;
  reg = 24000 1000;
  mac-address = [ 00 E0 0C 00 73 00 ];
  interrupts = 1d 2 1e 2 22 2;
  interrupt-parent = mpic;
  phy-handle = phy1f;
  tbi-handle = tbi1e;
  };

 Is any given board going to have at runtime (i.e. not jumper  
 selectable)  both a phy and a tbi (I'm not very familiar with the  
 latter, so I apologize if this is a dumb question).  If not, I'd  
 stick with phy-handle and have something in the phy node to  
 indicate that it's tbi.


Yes.  That will be the more common case.  The TBI PHYs are connected  
to the MDIO pins of each TSEC.  The TBIPA register defines what  
address it sits on.  It is used to configure non MII-style data  
connections, usually to another PHY.  For instance, to configure a  
TSEC for SGMII, you first configure the attached TBI to communicate  
with the on-chip SERDES.

While the TBI devices technically all sit on MDIO busses, only the  
TBI PHY connected to the first TSEC will interfere with MDIO  
transactions.  I don't think we need to create nodes for each of the  
TBIs.  They exist as a secondary part of the ethernet controller, and  
their address is only really important to that controller.

I still think it should just be a property of the ethernet node.  We  
aren't describing the TBI, we're describing a setting for the  
ethernet controller's register.

Andy
___
Linuxppc-embedded mailing list
Linuxppc-embedded@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-embedded


RE: [PATCH] [UPDATED] tsec: Allow Ten Bit Interface to be configurable

2007-08-14 Thread Joe Hamman
Hi Andy,

 
  Is any given board going to have at runtime (i.e. not jumper
  selectable)  both a phy and a tbi (I'm not very familiar with the
  latter, so I apologize if this is a dumb question).  If not, I'd
  stick with phy-handle and have something in the phy node to
  indicate that it's tbi.
 
 
 Yes.  That will be the more common case.  The TBI PHYs are connected
 to the MDIO pins of each TSEC.  The TBIPA register defines what
 address it sits on.  It is used to configure non MII-style data
 connections, usually to another PHY.  For instance, to configure a
 TSEC for SGMII, you first configure the attached TBI to communicate
 with the on-chip SERDES.
 
 While the TBI devices technically all sit on MDIO busses, only the
 TBI PHY connected to the first TSEC will interfere with MDIO
 transactions.  I don't think we need to create nodes for each of the
 TBIs.  They exist as a secondary part of the ethernet controller, and
 their address is only really important to that controller.
 
 I still think it should just be a property of the ethernet node.  We
 aren't describing the TBI, we're describing a setting for the
 ethernet controller's register.
 

Like this?  Do we need to have one for each [EMAIL PROTECTED]

[EMAIL PROTECTED] {
#address-cells = 1;
#size-cells = 0;
device_type = network;
model = eTSEC;
compatible = gianfar;
reg = 24000 1000;
mac-address = [ 00 E0 0C 00 73 00 ];
interrupts = 1d 2 1e 2 22 2;
interrupt-parent = mpic;
phy-handle = phy1f;
tbi-address = 0x1e;
};

Joe


___
Linuxppc-embedded mailing list
Linuxppc-embedded@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-embedded


Re: [PATCH] [UPDATED] tsec: Allow Ten Bit Interface to be configurable

2007-08-14 Thread Andy Fleming

On Aug 14, 2007, at 16:01, Joe Hamman wrote:

 Hi Andy,

 I still think it should just be a property of the ethernet node.  We
 aren't describing the TBI, we're describing a setting for the
 ethernet controller's register.


 Like this?  Do we need to have one for each [EMAIL PROTECTED]


Scott and I have been talking, and right now it seems like a better  
idea might be to have the TBI address get assigned dynamically, based  
on where PHYs aren't.

It's non-trivial to do (though not difficult), but this way we won't  
have to regret creating a bad interface.  You just need to look at  
the mdio bus, and find out which addresses responded.  Then you  
create a variable in the gfar_private structure to hold what it gets  
set to, and make sure that gfar_configure_serdes uses that value  
instead of the TBIPA constant.

Andy
___
Linuxppc-embedded mailing list
Linuxppc-embedded@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-embedded


RE: [PATCH] [UPDATED] tsec: Allow Ten Bit Interface to be configurable

2007-08-14 Thread Joe Hamman


 -Original Message-
 From: Andy Fleming [mailto:[EMAIL PROTECTED]
 Sent: Tuesday, August 14, 2007 4:30 PM
 To: [EMAIL PROTECTED]
 Cc: 'Scott Wood'; linuxppc-embedded@ozlabs.org
 Subject: Re: [PATCH] [UPDATED] tsec: Allow Ten Bit Interface to be
 configurable
 
 
 On Aug 14, 2007, at 16:01, Joe Hamman wrote:
 
  Hi Andy,
 
  I still think it should just be a property of the ethernet node.  We
  aren't describing the TBI, we're describing a setting for the
  ethernet controller's register.
 
 
  Like this?  Do we need to have one for each [EMAIL PROTECTED]
 
 
 Scott and I have been talking, and right now it seems like a better
 idea might be to have the TBI address get assigned dynamically, based
 on where PHYs aren't.
 
 It's non-trivial to do (though not difficult), but this way we won't
 have to regret creating a bad interface.  You just need to look at
 the mdio bus, and find out which addresses responded.  Then you
 create a variable in the gfar_private structure to hold what it gets
 set to, and make sure that gfar_configure_serdes uses that value
 instead of the TBIPA constant.

Sounds like a good plan.  I'll try to put something together and post for
comments.

Thanks,
Joe


___
Linuxppc-embedded mailing list
Linuxppc-embedded@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-embedded


[PATCH] [UPDATED] tsec: Allow Ten Bit Interface to be configurable

2007-08-13 Thread Joe Hamman
Allow the address of the Ten Bit Interface (TBI) to be changed in the
event of a conflict with another device.

Signed-off by: Joe Hamman [EMAIL PROTECTED]
---

Please ignore the last patch - I missed a cut  paste error on the range
that my testing didn't catch.
---

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 81ef81c..b4813d9 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -2276,6 +2276,14 @@ config GFAR_NAPI
bool NAPI Support
depends on GIANFAR
 
+config GFAR_TBIPA_VALUE
+   hex Ten Bit Interface Port Address Value
+   depends on GIANFAR
+   range 0 0x1f
+   default 0x1f
+   help
+ Select an address that does not conflict with other addresses on the 
board.
+
 config UCC_GETH
tristate Freescale QE Gigabit Ethernet
depends on QUICC_ENGINE
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index f926905..91ae0d3 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -490,15 +490,15 @@ static void gfar_configure_serdes(struct net_device *dev)
/* Initialise TBI i/f to communicate with serdes (lynx phy) */
 
/* Single clk mode, mii mode off(for aerdes communication) */
-   gfar_local_mdio_write(regs, TBIPA_VALUE, MII_TBICON, TBICON_CLK_SELECT);
+   gfar_local_mdio_write(regs, CONFIG_GFAR_TBIPA_VALUE, MII_TBICON, 
TBICON_CLK_SELECT);
 
/* Supported pause and full-duplex, no half-duplex */
-   gfar_local_mdio_write(regs, TBIPA_VALUE, MII_ADVERTISE,
+   gfar_local_mdio_write(regs, CONFIG_GFAR_TBIPA_VALUE, MII_ADVERTISE,
ADVERTISE_1000XFULL | ADVERTISE_1000XPAUSE |
ADVERTISE_1000XPSE_ASYM);
 
/* ANEG enable, restart ANEG, full duplex mode, speed[1] set */
-   gfar_local_mdio_write(regs, TBIPA_VALUE, MII_BMCR, BMCR_ANENABLE |
+   gfar_local_mdio_write(regs, CONFIG_GFAR_TBIPA_VALUE, MII_BMCR, 
BMCR_ANENABLE |
BMCR_ANRESTART | BMCR_FULLDPLX | BMCR_SPEED1000);
 }
 
@@ -547,7 +547,7 @@ static void init_registers(struct net_device *dev)
gfar_write(priv-regs-minflr, MINFLR_INIT_SETTINGS);
 
/* Assign the TBI an address which won't conflict with the PHYs */
-   gfar_write(priv-regs-tbipa, TBIPA_VALUE);
+   gfar_write(priv-regs-tbipa, CONFIG_GFAR_TBIPA_VALUE);
 }
 
 
diff --git a/drivers/net/gianfar.h b/drivers/net/gianfar.h
index d8e779c..0fd1c02 100644
--- a/drivers/net/gianfar.h
+++ b/drivers/net/gianfar.h
@@ -131,7 +131,6 @@ extern const char gfar_driver_version[];
 #define DEFAULT_RXCOUNT16
 #define DEFAULT_RXTIME 4
 
-#define TBIPA_VALUE0x1f
 #define MIIMCFG_INIT_VALUE 0x0007
 #define MIIMCFG_RESET   0x8000
 #define MIIMIND_BUSY0x0001


___
Linuxppc-embedded mailing list
Linuxppc-embedded@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-embedded


Re: [PATCH] [UPDATED] tsec: Allow Ten Bit Interface to be configurable

2007-08-13 Thread Kumar Gala

On Aug 13, 2007, at 5:37 PM, Joe Hamman wrote:

 Allow the address of the Ten Bit Interface (TBI) to be changed in the
 event of a conflict with another device.

 Signed-off by: Joe Hamman [EMAIL PROTECTED]
 ---

 Please ignore the last patch - I missed a cut  paste error on the  
 range
 that my testing didn't catch.

I think we'd rather this came from the device tree.

- k



___
Linuxppc-embedded mailing list
Linuxppc-embedded@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-embedded


RE: [PATCH] [UPDATED] tsec: Allow Ten Bit Interface to be configurable

2007-08-13 Thread Joe Hamman

 -Original Message-
 From: Kumar Gala [mailto:[EMAIL PROTECTED]
 Sent: Monday, August 13, 2007 10:10 PM
 To: [EMAIL PROTECTED]
 Cc: linuxppc-embedded@ozlabs.org
 Subject: Re: [PATCH] [UPDATED] tsec: Allow Ten Bit Interface to be
 configurable
 
 
 On Aug 13, 2007, at 5:37 PM, Joe Hamman wrote:
 
  Allow the address of the Ten Bit Interface (TBI) to be changed in the
  event of a conflict with another device.
 
  Signed-off by: Joe Hamman [EMAIL PROTECTED]
  ---
 
  Please ignore the last patch - I missed a cut  paste error on the
  range
  that my testing didn't catch.
 
 I think we'd rather this came from the device tree.
 
Duh, that makes sense ;-)

Has there been any discussion yet?  Maybe something like this:

[EMAIL PROTECTED] {
#address-cells = 1;
#size-cells = 0;
device_type = mdio;
compatible = gianfar;
reg = 24520 20;
phy0: [EMAIL PROTECTED] {
interrupt-parent = mpic;
interrupts = a 1;
reg = 0;
device_type = ethernet-phy;
};
phy1: [EMAIL PROTECTED] {
interrupt-parent = mpic;
interrupts = a 1;
reg = 1;
device_type = ethernet-phy;
};
phy2: [EMAIL PROTECTED] {
interrupt-parent = mpic;
interrupts = a 1;
reg = 2;
device_type = ethernet-phy;
};
phy3: [EMAIL PROTECTED] {
interrupt-parent = mpic;
interrupts = a 1;
reg = 3;
device_type = ethernet-phy;
};
tbi:  [EMAIL PROTECTED] {
reg = 1f;
device_type = ethernet-tbi;
};
};

Thanks,
Joe


___
Linuxppc-embedded mailing list
Linuxppc-embedded@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-embedded