[U-Boot] [PATCH] net: Add Marvell Kirkwood gigabit ethernet driver

[Wolfgang Denk]

Dear Prafulla Wadaskar,

In message <1243626217-9232-6-git-send-email-prafulla at marvell.com> you wrote:
> This patch adds a egiga driver for the Marvell Kirkwood SoC's.
...
> +	/* check parameters */
> +	if ((phy_adr << KWGBE_PHY_SMI_DEV_ADDR_OFFS) &
> +		~KWGBE_PHY_SMI_DEV_ADDR_MASK) {
> +		printf("Illegal phy address\n");
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

> +		return -EINVAL;
> +	}
> +	if ((reg_ofs << KWGBE_SMI_REG_ADDR_OFFS) &
> +				~KWGBE_SMI_REG_ADDR_MASK) {
> +		printf("Illegal register offset\n");
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

If you priont an error message, please print a helpful one, i. e.
provide the information what failed the test, and why.


> +static int kwgbe_send(struct eth_device *dev, volatile void *dataptr, int datasize)
> +{
> +	struct kwgbe_device *dkwgbe = to_dkwgbe(dev);
> +	struct kwgbe_registers *regs = dkwgbe->regs;
> +	struct kwgbe_txdesc *p_txdesc = dkwgbe->p_txdesc;
> +
> +	if ((u32)dataptr &0x07) {
> +		printf("Err..xmit dataptr not 64bit aligned\n");

Ditto. The user does not even know which driver this is coming from.

Also, please write "dataptr & 0x07".

> +	while (p_txdesc->cmd_sts & KWGBE_BUFFER_OWNED_BY_DMA) {
> +		/* return fail if error is detected */
> +		if (p_txdesc->cmd_sts & (KWGBE_UR_ERROR | KWGBE_RL_ERROR)) {
> +			printf("Err..in xmit packet\n");

Ditto.

> +static int kwgbe_recv(struct eth_device *dev)
> +{
> +	volatile struct kwgbe_device *dkwgbe = to_dkwgbe(dev);
> +	volatile struct kwgbe_rxdesc *p_rxdesc_curr= dkwgbe->p_rxdesc_curr;
> +
> +	/* Sanity check */
> +	if (p_rxdesc_curr == NULL) {
> +		printf("Err.. null ptr \n");

Please do not print trailing white space.

Also, this error message could be more helpful if you printed where it
happened, and which pointer was unexpectedly NULL.

And so on - please check rest of this file, and all other files for
_helpful_ error messages.



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
Inside every old person is a young person wondering what happened.
                              - Terry Pratchett, _Moving Pictures_