[U-Boot-Users] [PATCH] usb: add support for R8A66597 usb controller

[Yoshihiro Shimoda <shimoda.yoshihiro@renesas.com>]

2008/07/10 4:15, Jean-Christophe PLAGNIOL-VILLARD wrote:
>  +
>> +#include <common.h>
>> +#include <usb.h>
>> +#include <asm/io.h>
>> +
>> +#include "r8a66597.h"
>> +
>> +#if defined(CONFIG_USB_R8A66597_HCD)
> please move to the Makefile

Thank you for your comment.
I wlll move to the Makefile.

>> +
>> +#ifdef R8A66597_DEBUG
>> +#define R8A66597_DPRINT		printf
>> +#else
>> +#define R8A66597_DPRINT(...)
>> +#endif
> why not using debug()

I'm sorry. I didn't know debug(). I will use it.

>> +
>> +static const char hcd_name[] = "r8a66597_hcd";
>> +static unsigned short clock = CONFIG_R8A66597_XTAL;
>> +static unsigned short vif = CONFIG_R8A66597_LDRV;
>> +static unsigned short endian = CONFIG_R8A66597_ENDIAN;
>> +static struct r8a66597 gr8a66597;
> is it not possible to have a better name for the struct and the var
> and btw for the function naming?

Because XTAL, LDRV, DEVADD are a register name or register bit name,
I think that it is a good name...
But I will change a name of the "struct r8a66957 gr8a66597" to
"struct r8a66597 r8a66597_dev".

>> +
>> +static void set_devadd_reg(struct r8a66597 *r8a66597, u8 r8a66597_address,
>> +			   u16 usbspd, u8 upphub, u8 hubport, int port)
>> +{
>> +	u16 val;
>> +	unsigned long devadd_reg = get_devadd_addr(r8a66597_address);
>> +
>> +	val = (upphub << 11) | (hubport << 8) | (usbspd << 6) | (port & 0x0001);
>> +	r8a66597_write(r8a66597, val, devadd_reg);
>> +}
>> +

Best Regards,
Yoshihiro Shimoda