| Message ID | 1250765117-22849-1-git-send-email-swami.iyer@ti.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
Swaminathan S <swami.iyer@ti.com> writes: Again, need a descriptive changelog please. > Signed-off-by: Swaminathan S <swami.iyer@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
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:khilman@deeprootsystems.com] Sent: Tuesday, August 25, 2009 5:29 PM To: Subbrathnam, Swaminathan Cc: linux-usb@vger.kernel.org; davinci-linux-open-source@linux.davincidsp.com Subject: Re: [PATCH] MUSB: Add support for DM646x USB. Swaminathan S <swami.iyer@ti.com> writes: Again, need a descriptive changelog please. > Signed-off-by: Swaminathan S <swami.iyer@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
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); + 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); } 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 #endif /* __MUSB_HDRDF_H__ */
Signed-off-by: Swaminathan S <swami.iyer@ti.com> --- drivers/usb/musb/davinci.c | 30 +++++++++++++++++++++++------- drivers/usb/musb/davinci.h | 6 ++++++ 2 files changed, 29 insertions(+), 7 deletions(-)