| Message ID | 66901067543efe59d87b3a805a4b19ce52a780c5.1290192590.git.linz@li-pro.net |
|---|---|
| State | Changes Requested |
| Headers | show |
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. 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 > >
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
Hi Michal, as announced here is the correction for the LL TEMAC bugfixing. I've changed all printf() to puts(). Moreover I've corrected the wrong result code (-1) if we can't found a real phy -- my mistake ;-). Please use the new patch for your tests on real hw. Best regards, Stephan Linz
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 ... "); 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"); + return -1; + } + printf("OK\n"); + } + + /* wait for link up */ + printf("Waiting for link ... "); + retries = 10; + while (retries-- && + ((xps_ll_temac_hostif_get(dev, 0, phy_addr, 1) & 0x24) != 0x24)) + ; + + if (retries < 0) { + printf("ERROR\n"); + return 0; } + printf("OK\n"); /* 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; }
* 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() Signed-off-by: Stephan Linz <linz@li-pro.net> --- drivers/net/xilinx_ll_temac.c | 44 ++++++++++++++++++++++++++++++---------- 1 files changed, 33 insertions(+), 11 deletions(-)