On Mon, Apr 18, 2011 at 11:51 AM, Wolfgang Denk <w...@denx.de> wrote:
> Dear Simon Glass,
>
>> +/* Rx status word */
>> +#define RX_STS_FL_ (0x3FFF0000) /* Frame Length */
>> +#define RX_STS_ES_ (0x00008000) /* Error Summary */
> ...
>
> Please drop the parens around all these constants. Please fix
> globally.
Done
>> + ret = smsc95xx_write_reg(dev, HW_CFG, read_buf);
>> + if (ret < 0) {
>> + debug("Failed to write HW_CFG_BIR_ bit in HW_CFG "
>> + "register, ret = %d\n", ret);
>> + return ret;
>> + }
>
> You repeat these sequences of smsc95xx_read_reg() resp.
> smsc95xx_write_reg() followed by the "if (ret < 0)" part about 20
> times.
>
> Please move the respective debug() message for the failure case into
> the functions, and consider using a loop here instead.
I have moved the debug messages into the routines. Not sure what you
mean by a loop.
>
>> + read_buf = DEFAULT_BULK_IN_DELAY;
>> + ret = smsc95xx_write_reg(dev, BULK_IN_DLY, read_buf);
>> + if (ret < 0) {
>> + debug("ret = %d", ret);
>> + return ret;
>> + }
>
> Is it intentional to use the read_buf with a smsc95xx_write_reg()
> call?
It doesn't need to - I have changed it to use the value directly.
>> + ret = smsc95xx_write_reg(dev, HW_CFG, read_buf);
>> + if (ret < 0) {
>> + debug("Failed to write HW_CFG register, ret=%d\n", ret);
>> + return ret;
>> + }
>
> Ditto here?
This is updating a register. It reads the value, modifies it and writes it back.
>
>> + ret = smsc95xx_write_reg(dev, AFC_CFG, read_buf);
>> + if (ret < 0) {
>> + debug("Failed to write AFC_CFG: %d\n", ret);
>> + return ret;
>> + }
>
> And here?
Same as above.
>
>> + ret = smsc95xx_read_reg(dev, INT_EP_CTL, &read_buf);
>> + if (ret < 0) {
>> + debug("Failed to read INT_EP_CTL: %d", ret);
>> + return ret;
>> + }
>
> And here we have &read_buf while everywhere else we have read_buf
> only?
That's because it is reading the value here. The read routine needs an
address of a variable to put the value into.
>
>
>
> Best regards,
>
> Wolfgang Denk
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
