[U-Boot] [PATCH] fix lockup in mcfmii/mii_discover_phy() in case communication fails

[w.wegner at astro-kom.de]

Hi Ben,

On 4 Apr 2010 at 22:44, Ben Warren wrote:

> Hi Wolfgang,
> 
> On 3/30/2010 10:19 AM, Wolfgang Wegner wrote:
> > Signed-off-by: Wolfgang Wegner<w.wegner at astro-kom.de>
> > ---
> >   drivers/net/mcfmii.c |    6 +++++-
> >   1 files changed, 5 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/net/mcfmii.c b/drivers/net/mcfmii.c
> > index 4acc29e..83c0873 100644
> > --- a/drivers/net/mcfmii.c
> > +++ b/drivers/net/mcfmii.c
> > @@ -185,7 +185,11 @@ int mii_discover_phy(struct eth_device *dev)
> >   				printf("PHY @ 0x%x pass %d\n", phyno, pass);
> >   #endif
> >
> > -				for (i = 0; i<  (sizeof(phyinfo) / sizeof(phy_info_t)); i++) {
> > +				for (i = 0;
> > +					(i<  (sizeof(phyinfo)
> > +						/ sizeof(phy_info_t)))
> > +					&&  (phyinfo[i].phyid != 0);
> > +					i++) {
> >   					if (phyinfo[i].phyid == phytype) {
> >   #ifdef ET_DEBUG
> >   						printf("phyid %x - %s\n",
> >    
> This is brutal.  Using 8-space tabs really does a good job of 
> highlighting deep nesting of conditionals.  If you're unable to make the 
> driver easier to read (and I understand if that's the case), my 
> preference would be to keep the sizeof()s on one line and combine the 
> second && term and the i++ on another.  This makes lines > 80 chars, but 
> it was already that way.

I agree, but was worried if the > 80 char line would be a reason to reject the
patch and wanted to have it as non-intrusive as possible.

If there are no objections, I am happy to supply a second version with
formatting as you suggested.

Regards,
Wolfgang