[U-Boot] [PATCH v3 2/7] ehci-omap: Clean up added ehci-omap.c

[Igor Grinberg <grinberg@compulab.co.il>]

On 02/03/12 15:38, Govindraj.R wrote:
> From: "Govindraj.R" <govindraj.raja@ti.com>
> 
> Clean up added ehci-omap.c and make it generic for re-use across
> omap-soc having same ehci ip block. Also pass the modes to be configured
> from board file and configure the ports accordingly. All usb layers
> are not cache aligned, till then keep cache off for usb ops as ehci will use
> internally dma for all usb ops.
> 
> * Add a generic common header ehci-omap.h having common ip block
>   data and reg shifts.
> * Rename and modify ehci-omap3 to ehci.h retain only conflicting
>   sysc reg shifts remove others and move to common header file.
> 
> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>

Some final neats below, otherwise:

Acked-by: Igor Grinberg <grinberg@compulab.co.il>

> ---
>  arch/arm/include/asm/arch-omap3/ehci.h       |   55 +++++++
>  arch/arm/include/asm/arch-omap3/ehci_omap3.h |   58 -------
>  arch/arm/include/asm/arch-omap4/ehci.h       |   49 ++++++
>  arch/arm/include/asm/ehci-omap.h             |  148 ++++++++++++++++++
>  drivers/usb/host/ehci-omap.c                 |  212 +++++++++++++++++++-------
>  5 files changed, 408 insertions(+), 114 deletions(-)
>  create mode 100644 arch/arm/include/asm/arch-omap3/ehci.h
>  delete mode 100644 arch/arm/include/asm/arch-omap3/ehci_omap3.h
>  create mode 100644 arch/arm/include/asm/arch-omap4/ehci.h
>  create mode 100644 arch/arm/include/asm/ehci-omap.h

[...]

> diff --git a/arch/arm/include/asm/ehci-omap.h b/arch/arm/include/asm/ehci-omap.h
> new file mode 100644
> index 0000000..c99ac20
> --- /dev/null
> +++ b/arch/arm/include/asm/ehci-omap.h

[...]

> +
> +/* ULPI */
> +#define ULPI_SET(a)					(a + 1)
> +#define ULPI_CLR(a)					(a + 2)
> +#define ULPI_FUNC_CTRL					0x04
> +#define ULPI_FUNC_CTRL_RESET				(1 << 5)

The above should be removed as it is not used anymore.

> +
> +struct omap_usbhs_board_data {
> +	enum usbhs_omap_port_mode port_mode[OMAP_HS_USB_PORTS];
> +};
> +
> +struct omap_usbtll {
> +	u32 rev;		/* 0x00 */
> +	u32 hwinfo;		/* 0x04 */
> +	u8 reserved1[0x8];

Are you sure you want this to be an array of bytes instead of
register (u32) wide fields?
IMO,
u32 reserved1[2];

looks much better than:
u8 reserved1[0x8];

and does not have alignment issues, but if you think u8 is better,
I will not object.


> +	u32 sysc;		/* 0x10 */
> +	u32 syss;		/* 0x14 */
> +	u32 irqst;		/* 0x18 */
> +	u32 irqen;		/* 0x1c */
> +	u8 reserved2[0x10];
> +	u32 shared_conf;	/* 0x30 */
> +	u8 reserved3[0xc];
> +	u32 channel_conf;	/* 0x40 */
> +};
> +
> +struct omap_uhh {
> +	u32 rev;	/* 0x00 */
> +	u32 hwinfo;	/* 0x04 */
> +	u8 reserved1[0x8];
> +	u32 sysc;	/* 0x10 */
> +	u32 syss;	/* 0x14 */
> +	u8 reserved2[0x28];
> +	u32 hostconfig;	/* 0x40 */
> +	u32 debugcsr;	/* 0x44 */
> +};
> +
> +struct omap_ehci {
> +	u32 hccapbase;		/* 0x00 */
> +	u32 hcsparams;		/* 0x04 */
> +	u32 hccparams;		/* 0x08 */
> +	u8 reserved1[0x04];
> +	u32 usbcmd;		/* 0x10 */
> +	u32 usbsts;		/* 0x14 */
> +	u32 usbintr;		/* 0x18 */
> +	u32 frindex;		/* 0x1c */
> +	u32 ctrldssegment;	/* 0x20 */
> +	u32 periodiclistbase;	/* 0x24 */
> +	u32 asysnclistaddr;	/* 0x28 */
> +	u8 reserved2[0x24];
> +	u32 configflag;		/* 0x50 */
> +	u32 portsc_i;		/* 0x54 */
> +	u8 reserved3[0x38];
> +	u32 insreg00;		/* 0x90 */
> +	u32 insreg01;		/* 0x94 */
> +	u32 insreg02;		/* 0x98 */
> +	u32 insreg03;		/* 0x9c */
> +	u32 insreg04;		/* 0xa0 */
> +	u32 insreg05_utmi_ulpi;	/* 0xa4 */
> +	u32 insreg06;		/* 0xa8 */
> +	u32 insreg07;		/* 0xac */
> +	u32 insreg08;		/* 0xb0 */
> +};
> +
> +int omap_ehci_hcd_init(struct omap_usbhs_board_data *usbhs_pdata);
> +int omap_ehci_hcd_stop(void);
> +
> +#endif /* _OMAP_COMMON_EHCI_H_ */

[...]

> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> index 93d3bb7..89c3c3a 100644
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c

[...]

> +static void omap_ehci_soft_phy_reset(int port)
> +{
> +	struct ulpi_viewport ulpi_vp;
> +
> +	ulpi_vp.viewport_addr = (u32)&ehci->insreg05_utmi_ulpi;
> +	ulpi_vp.port_num = port;
> +
> +	ulpi_reset(&ulpi_vp);
> +

no need for the empty line here

> +}
> +

[...]

>  /*
> - * Initialize the OMAP3 EHCI controller and PHY.
> - * Based on "drivers/usb/host/ehci-omap.c" from Linux 2.6.37.
> + * Initialize the OMAP EHCI controller and PHY.
> + * Based on "drivers/usb/host/ehci-omap.c" from Linux 3.1
>   * See there for additional Copyrights.
>   */
> -int ehci_hcd_init(void)
> +int omap_ehci_hcd_init(struct omap_usbhs_board_data *usbhs_pdata)
>  {
> -	int ret;
> +	int ret = 0;
> +	unsigned int i, reg = 0, rev = 0;

no need to initialize ret here, it is done below.

>  
> -	debug("Initializing OMAP3 EHCI\n");
> +	debug("Initializing OMAP EHCI\n");
>  
>  	ret = board_usb_init();
>  	if (ret < 0)

[...]


-- 
Regards,
Igor.