[U-Boot] [PATCH v2] NET: add ENC28J60 driver using SPI framework

[Wolfgang Denk]

Dear Reinhard Meyer,

In message <4C87CFE8.8010203 at emk-elektronik.de> you wrote:
> Dear Mike Frysinger,
> > On Monday, September 06, 2010 10:59:16 Reinhard Meyer wrote:
> >> +int enc_miiphy_read(const char *devname, u8 phy_adr, u8 reg, u16 *value)
> >> +{
> >> +	struct eth_device *dev = eth_get_dev_by_name(devname);
> >> +	if (dev) {
> >> +		enc_dev_t *enc = to_enc(dev);
> >> +
> >> +		if (phy_adr != 0)
> >> +			return -1;
> >> +		spi_claim_bus(enc->slave);
> >> +		*value = phy_read(enc, reg);
> >> +		spi_release_bus(enc->slave);
> >> +		return 0;
> >> +	}
> >> +	return -1;
> >> +}
> >
> > seems like this (and the write func) would be nicer if it were:
> > if (!dev)
> > 	return -1;
> > ...
> 
> Umm, no. That would split enc_dev_t *enc = to_enc (dev) into two lines:
> Declaration, if, assigment

Yes? What's the problem with that?  But then you can unindent a whole
block of code one level, which means the code is much easier toi read
and to maintain.

Please do as Mike suggested.

> > check the return value
> 
> OK, for sake of completeness in every function using it..
> Hmm blows up the code :)

Yes, error handling blows up the code. But it helpos a lot to make
programs reliable and robust.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
There has been an alarming increase in the number of things you  know
nothing about.