Re: synch eth patch

[Jonathan Larmour <jifl at eCosCentric dot com>]

Jani Monoses wrote:
> Ok I mentioned the other driver just to point out that
> returning anything non-zero from the control function of the eth driver
> is equivalent with the curent EINVAL and my change does not go against 'the rules'.
> IMHO EINVAL should not be there at all because the low level drivers are supposed
> to be independent of upper layers. The return value of ->control is only checked
> by io/eth/net/eth_drv.c and it checks for 0 or non-zero.These glue drivers translate
> low-level error to error codes for the TCP stack.These might be standard errno.h codes
> or anything else.
I think that exposes the simplicity of eth_drv.c. Really there should be 
some sort of way for upper layers to talk to the low level driver directly 
and get more detailed info about errors.

Right now the eth ioctl return codes are a mess. Different drivers return 
different numbers for different things, the only consistency being that 
non-zero means error. Having an eth ioctl where the only return code is 
success or EINVAL isn't viable long term.

We should try and make this more consistent. Just #include <errno.h> (and 
require CYGINT_ISO_ERRNO_CODES) and stick to EINVAL. Hopefully at some 
point we can make all the drivers return sensible errors.

> > > > > > > "Jani" == Jani Monoses <jani@iv.ro> writes:
> >    Jani> I did mean to send it to the list, sorry.
> >    >> note: this should really be a public discussion.  My opinion is
> >    >> not the only one that counts :-)
> >
> >    >> My question would be - "does the synthetic ethernet driver build 
> >    >> [without changes] when using one of the other stacks?"  If so, then
> >    >> IMO it's not the driver's fault.
> >
> >    Jani> mine is "do other drivers build with lwIP?" - yes :)
> >    Jani> one of them actually defines EINVAL to be 2 before using it.
> >
> > Yes, arm/ks32c500. However #define'ing a system error code inside a
> > device driver is a very bad idea and that driver should really be
> > cleaned up. Especially since the error package #define's EINVAL as 22
> > rather than 2.
If some header file that that includes ever in turn includes <errno.h> 
(quite reasonable) you'd get redefinition problems too. Yes it should be 
cleaned up :-).

> > EINVAL should always be available - most serial drivers depend on it.
> > I believe the correct solution is to make sure that EINVAL is always
> > #define'd in the synthetic target ethernet driver, probably by an
> > explicit #include of <cyg/error/codes.h>. I'll try to look into this
> > today or tomorrow.
Okay, if you (Bart) want to look at it be my guest since I'm a bit busy 
right now.

Jifl
--
eCosCentric    http://www.eCosCentric.com/    The eCos and RedBoot experts
--[ "You can complain because roses have thorns, or you ]--
--[  can rejoice because thorns have roses." -Lincoln   ]-- Opinions==mine