Kevin,
        I will re-organize the vbus control as suggested as a separate patch.
        
        It will have to be tested across DaVinci, OMAP platforms (probably 
Blackfin) as at that point it will be a generic way of controlling the Vbus 
line.

Regards
swami

-----Original Message-----
From: Kevin Hilman [mailto:khil...@deeprootsystems.com] 
Sent: Tuesday, August 25, 2009 5:29 PM
To: Subbrathnam, Swaminathan
Cc: linux-...@vger.kernel.org; davinci-linux-open-source@linux.davincidsp.com
Subject: Re: [PATCH] MUSB: Add support for DM646x USB.

Swaminathan S <swami.i...@ti.com> writes:

Again, need a descriptive changelog please.

>  Signed-off-by: Swaminathan S <swami.i...@ti.com>
>
> ---
>  drivers/usb/musb/davinci.c |   30 +++++++++++++++++++++++-------
>  drivers/usb/musb/davinci.h |    6 ++++++
>  2 files changed, 29 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c
> index 6691381..2b8345a 100644
> --- a/drivers/usb/musb/davinci.c
> +++ b/drivers/usb/musb/davinci.c
> @@ -65,6 +65,13 @@ static inline void phy_on(void)
>       /* power everything up; start the on-chip PHY and its PLL */
>       phy_ctrl &= ~(USBPHY_OSCPDWN | USBPHY_OTGPDWN | USBPHY_PHYPDWN);
>       phy_ctrl |= USBPHY_SESNDEN | USBPHY_VBDTCTEN | USBPHY_PHYPLLON;
> +     if (cpu_is_davinci_dm646x()) {
> +             phy_ctrl |= USBPHY_NDATAPOL | USBPHY_SESSION_VBUS;
> +             phy_ctrl |= is_peripheral_enabled() ? USBPHY_PERI_USBID :
> +                                             phy_ctrl;
> +             phy_ctrl &= ~USBPHY_VBDTCTEN;
> +     }
> +
>       __raw_writel(phy_ctrl, USB_PHY_CTRL);
>  
>       /* wait for PLL to lock before proceeding */
> @@ -152,7 +159,7 @@ void musb_platform_disable(struct musb *musb)
>   * when J10 is out, and TI documents it as handling OTG.
>   */
>  
> -#ifdef CONFIG_MACH_DAVINCI_EVM
> +#if defined(CONFIG_MACH_DAVINCI_EVM) || 
> defined(CONFIG_MACH_DAVINCI_DM6467_EVM)
>  
>  static int vbus_state = -1;
>  
> @@ -162,7 +169,12 @@ static int vbus_state = -1;
>   */
>  static void evm_deferred_drvvbus(struct work_struct *ignored)
>  {
> -     gpio_set_value_cansleep(GPIO_nVBUS_DRV, vbus_state);
> +     if (machine_is_davinci_evm())
> +             gpio_set_value_cansleep(GPIO_nVBUS_DRV, vbus_state);
> +
> +     if (machine_is_davinci_dm6467_evm())
> +             usb_vbus_control(vbus_state);
> +

OK, this I don't like.  The current hack of board-specific code in
this file was bad enought, but it's time to fix this instead of
extending the hack.

What we need is a board-specific vbus control function, defined in the
board files and passed in platform_data.

That will allow us to get rid of board-specific code from this file.

>       vbus_state = !vbus_state;
>  }
>  
> @@ -170,7 +182,7 @@ static void evm_deferred_drvvbus(struct work_struct 
> *ignored)
>  
>  static void davinci_source_power(struct musb *musb, int is_on, int immediate)
>  {
> -#ifdef CONFIG_MACH_DAVINCI_EVM
> +#if defined(CONFIG_MACH_DAVINCI_EVM) || 
> defined(CONFIG_MACH_DAVINCI_DM6467_EVM)
>       if (is_on)
>               is_on = 1;
>  
> @@ -178,12 +190,16 @@ static void davinci_source_power(struct musb *musb, int 
> is_on, int immediate)
>               return;
>       vbus_state = !is_on;            /* 0/1 vs "-1 == unknown/init" */
>  
> -     if (machine_is_davinci_evm()) {
> +     if (machine_is_davinci_evm() || machine_is_davinci_dm6467_evm()) {
>               static DECLARE_WORK(evm_vbus_work, evm_deferred_drvvbus);
>  
> -             if (immediate)
> -                     gpio_set_value_cansleep(GPIO_nVBUS_DRV, vbus_state);
> -             else
> +             if (immediate) {
> +                     if (machine_is_davinci_evm())
> +                             gpio_set_value_cansleep(GPIO_nVBUS_DRV,
> +                                                     vbus_state);
> +                     if (machine_is_davinci_dm6467_evm())
> +                             usb_vbus_control(vbus_state);
> +             } else
>                       schedule_work(&evm_vbus_work);

ditto... we shouldn't have any machine_is_* code in this driver.

>       }
>       if (immediate)
> diff --git a/drivers/usb/musb/davinci.h b/drivers/usb/musb/davinci.h
> index 046c844..b802b83 100644
> --- a/drivers/usb/musb/davinci.h
> +++ b/drivers/usb/musb/davinci.h
> @@ -16,6 +16,9 @@
>  
>  /* Integrated highspeed/otg PHY */
>  #define USBPHY_CTL_PADDR     (DAVINCI_SYSTEM_MODULE_BASE + 0x34)
> +#define USBPHY_NDATAPOL              BIT(18)
> +#define USBPHY_SESSION_VBUS  BIT(17)
> +#define USBPHY_PERI_USBID    BIT(16)
>  #define USBPHY_DATAPOL               BIT(11) /* (dm355) switch D+/D- */
>  #define USBPHY_PHYCLKGD              BIT(8)
>  #define USBPHY_SESNDEN               BIT(7)  /* v(sess_end) comparator */
> @@ -104,4 +107,7 @@
>  
>  #define DAVINCI_BASE_OFFSET          0x400
>  
> +#ifdef CONFIG_MACH_DAVINCI_DM6467_EVM
> +extern void usb_vbus_control(u8 on);
> +#endif

then this can disappear as well.

>  #endif       /* __MUSB_HDRDF_H__ */

Kevin

_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to