Am Samstag, 20. November 2010, um 10:23:18 schrieb Michal Simek:

Hi Stephan,

Hi Michal, Hi Ben (see last questen to you below),

2010/11/19 Stephan Linz linz@li-pro.net

...

Adding the missing phy-id for the National 10/100/1000 MBit PHY DP83865 used on the Xilinx Spartan-3A DSP evaluation board SP3ADSP1800.

I understand that you want to add support for PHY which is on xilinx development

right.

board but here is one design problem. Is this really way how to add support for PHYs? As you can suggest the answer is NO.

Yes of course, that is my opinion too.

I was talking about with Ben some months ago and there is only one solution which can be acceptable. The solution is phy lib. Not sure if Ben did any basic tests with it but this is sensible way how to do it.

@Ben: last question to you: Did you any basic tests with the new phy library? Is there any documentation?

The main reason is that Xilinx have several development boards and every custom board can have different phy and I don't want to see that we will add support for it directly to driver.

Yes I know. This will be a problem if we do not use the phy library.

In general I agree that it is necessary to support more PHYs at least all which are on Xilinx development boards but I can't agree with adding it directly to the driver.

I accept your decision and will maintain my own patch set until we can use the new phy library.

Other question for the transitional time to use the phy lib. Can we introduce a LL TEMAC configuration for the board specific phy id? Example:

<include/configs/microblaze-generic.h> #define CONFIG_XILINX_LL_TEMAC_PHYID 0x1410cc2

<drivers/net/xilinx_ll_temac.c> #ifndef CONFIG_XILINX_LL_TEMAC_PHYID #error define phyid in configuration #endif

xps_ll_temac_phy_ctrl(): if (i == CONFIG_XILINX_LL_TEMAC_PHYID) { .... }

Best regards, Stephan

Regards, Michal

...

Signed-off-by: Stephan Linz linz@li-pro.net

drivers/net/xilinx_ll_temac.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/xilinx_ll_temac.c b/drivers/net/xilinx_ll_temac.c index a72e072..b19a74a 100644 --- a/drivers/net/xilinx_ll_temac.c +++ b/drivers/net/xilinx_ll_temac.c @@ -287,8 +287,11 @@ static int xps_ll_temac_phy_ctrl(struct eth_device *dev) return 1; }

•   /* Marwell 88e1111 id - ml50x */
  

•   if (i == 0x1410cc2) {
  

•   /*
  

•    * Marwell 88e1111 id - ml50x, ml605
  

•    * National DP83865 id - sp3adsp1800
  

•    */
  

•   if (i == 0x1410cc2 || i == 0x20005c7a) {
           result = xps_ll_temac_hostif_get(dev, 0, phy_addr, 5);
           if ((result & 0x8000) == 0x8000) {
                   xps_ll_temac_indirect_set(dev, 0, EMMC,
  

0x80000000); @@ -305,6 +308,8 @@ static int xps_ll_temac_phy_ctrl(struct eth_device *dev) } return 1; }

•   printf("Unsupported PHY\n");
   return 0;
  

}

-- 1.6.0.4

Am Samstag, 20. November 2010, um 10:27:45 schrieb Michal Simek:

2010/11/19 Stephan Linz linz@li-pro.net

...

• avoid using link variable uninitialized
• avoid using phy_addr variable with invalid value
• reorganize phy control: first looking for phy than link
• return with error (result value -1) if no phy/link was found
• fix boolean mistake in wait for link: wait as long as we got phy register 1 has no link indication (BMSR != 0x24)
• expand the 'first run' flag handling in ll_temac_init() in respect to possible error detection in xps_ll_temac_phy_ctrl()

I will test your changes on real hw.

Some comments below.

I do it so on Monday ... printf() --> puts()

Regards, Stephan

Thanks, Michal

...

Signed-off-by: Stephan Linz linz@li-pro.net

drivers/net/xilinx_ll_temac.c | 44 ++++++++++++++++++++++++++++++---------- 1 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/drivers/net/xilinx_ll_temac.c b/drivers/net/xilinx_ll_temac.c index 72a1632..a72e072 100644 --- a/drivers/net/xilinx_ll_temac.c +++ b/drivers/net/xilinx_ll_temac.c @@ -225,24 +225,21 @@ static void read_phy_reg (struct eth_device *dev, int phy_addr) #endif

static int phy_addr = -1; -static int link; +static int link = 0;

/* setting ll_temac and phy to proper setting */ static int xps_ll_temac_phy_ctrl(struct eth_device *dev) {

•   int i;
  

•   int i, retries;
   unsigned int result;
  

•   unsigned retries = 10;
  
  

•    /* link is setup */
   if (link == 1)
  

•           return 1; /* link is setup */
  

•   /* wait for link up */
  

•   while (retries-- &&
  

•           ((xps_ll_temac_hostif_get(dev, 0, phy_addr, 1) & 0x24) ==
  

0x24))

•           ;
  

•           return 1;
  
  

•   /* try out if have ever found the right phy? */
   if (phy_addr == -1) {
  

•           printf("Looking for phy ... ");
  

use puts instead.

...

           for (i = 31; i >= 0; i--) {
                   result = xps_ll_temac_hostif_get(dev, 0, i, 1);
                   if ((result & 0x0ffff) != 0x0ffff) {

@@ -251,7 +248,27 @@ static int xps_ll_temac_phy_ctrl(struct eth_device *dev) break; } }

•           /* no success? -- wery bad */
  

•           if (phy_addr == -1) {
  

•                   printf("ERROR\n");
  

same here

...

•                   return -1;
  

•           }
  

•           printf("OK\n");
  

•   }
  

•   /* wait for link up */
  

•   printf("Waiting for link ... ");
  

and here.

...

•   retries = 10;
  

•   while (retries-- &&
  

•           ((xps_ll_temac_hostif_get(dev, 0, phy_addr, 1) & 0x24) !=
  

0x24))

•           ;
  

•   if (retries < 0) {
  

•           printf("ERROR\n");
  

and here

...

•           return 0;
   }
  

•   printf("OK\n");
  

and here.

...

   /* get PHY id */
   i = (xps_ll_temac_hostif_get(dev, 0, phy_addr, 2) << 16) | \

@@ -491,7 +508,6 @@ static int ll_temac_init(struct eth_device *dev, bd_t *bis) #endif if (!first) return 0;

•   first = 0;
  
   xps_ll_temac_init(dev, bis);
  
  

@@ -502,7 +518,13 @@ static int ll_temac_init(struct eth_device *dev, bd_t *bis) for (i = 0; i < 32; i++) read_phy_reg(dev, i); #endif

•   xps_ll_temac_phy_ctrl(dev);
  

•   if (xps_ll_temac_phy_ctrl(dev) == 0) {
  

•           xps_ll_temac_halt(dev);
  

•           return -1;
  

•   }
  

•   first = 0;
   return 1;
  

}

-- 1.6.0.4

* avoid using link variable uninitialized * avoid using phy_addr variable with invalid value * reorganize phy control: first looking for phy than link * return with error (result value 0) if no phy/link was found * fix boolean mistake in wait for link: wait as long as we got phy register 1 has no link indication (BMSR != 0x24) * expand the 'first run' flag handling in ll_temac_init() in respect to possible error detection in xps_ll_temac_phy_ctrl()

Signed-off-by: Stephan Linz linz@li-pro.net --- drivers/net/xilinx_ll_temac.c | 52 ++++++++++++++++++++++++++++++----------- 1 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/drivers/net/xilinx_ll_temac.c b/drivers/net/xilinx_ll_temac.c index 72a1632..ff93c1f 100644 --- a/drivers/net/xilinx_ll_temac.c +++ b/drivers/net/xilinx_ll_temac.c @@ -225,24 +225,21 @@ static void read_phy_reg (struct eth_device *dev, int phy_addr) #endif

static int phy_addr = -1; -static int link; +static int link = 0;

/* setting ll_temac and phy to proper setting */ static int xps_ll_temac_phy_ctrl(struct eth_device *dev) { - int i; + int i, retries; unsigned int result; - unsigned retries = 10;

+ /* link is setup */ if (link == 1) - return 1; /* link is setup */ - - /* wait for link up */ - while (retries-- && - ((xps_ll_temac_hostif_get(dev, 0, phy_addr, 1) & 0x24) == 0x24)) - ; + return 1;

+ /* try out if have ever found the right phy? */ if (phy_addr == -1) { + puts("Looking for phy ... "); for (i = 31; i >= 0; i--) { result = xps_ll_temac_hostif_get(dev, 0, i, 1); if ((result & 0x0ffff) != 0x0ffff) { @@ -251,7 +248,27 @@ static int xps_ll_temac_phy_ctrl(struct eth_device *dev) break; } } + + /* no success? -- wery bad */ + if (phy_addr == -1) { + puts("ERROR\n"); + return 0; + } + puts("OK\n"); + } + + /* wait for link up */ + puts("Waiting for link ... "); + retries = 10; + while (retries-- && + ((xps_ll_temac_hostif_get(dev, 0, phy_addr, 1) & 0x24) != 0x24)) + ; + + if (retries < 0) { + puts("ERROR\n"); + return 0; } + puts("OK\n");

/* get PHY id */ i = (xps_ll_temac_hostif_get(dev, 0, phy_addr, 2) << 16) | \ @@ -275,19 +292,21 @@ static int xps_ll_temac_phy_ctrl(struct eth_device *dev) result = xps_ll_temac_hostif_get(dev, 0, phy_addr, 5); if ((result & 0x8000) == 0x8000) { xps_ll_temac_indirect_set(dev, 0, EMMC, 0x80000000); - printf("1000BASE-T/FD\n"); + puts("1000BASE-T/FD\n"); link = 1; } else if ((result & 0x4000) == 0x4000) { xps_ll_temac_indirect_set(dev, 0, EMMC, 0x40000000); - printf("100BASE-T/FD\n"); + puts("100BASE-T/FD\n"); link = 1; } else { - printf("Unsupported mode\n"); + puts("Unsupported mode\n"); link = 0; return 0; } return 1; } + + puts("Unsupported PHY\n"); return 0; }

@@ -491,7 +510,6 @@ static int ll_temac_init(struct eth_device *dev, bd_t *bis) #endif if (!first) return 0; - first = 0;

xps_ll_temac_init(dev, bis);

@@ -502,7 +520,13 @@ static int ll_temac_init(struct eth_device *dev, bd_t *bis) for (i = 0; i < 32; i++) read_phy_reg(dev, i); #endif - xps_ll_temac_phy_ctrl(dev); + + if (xps_ll_temac_phy_ctrl(dev) == 0) { + xps_ll_temac_halt(dev); + return -1; + } + + first = 0; return 1; }