| Submitter | Ajay Kumar Gupta |
|---|---|
| Date | 2009-10-28 15:12:38 |
| Message ID | <1256742758-32702-3-git-send-email-ajay.gupta@ti.com> |
| Download | mbox | patch |
| Permalink | /patch/56299/ |
| State | Changes Requested, archived |
| Delegated to: | Tony Lindgren |
| Headers | show |
Comments
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap- > owner@vger.kernel.org] On Behalf Of Gupta, Ajay Kumar > Sent: Wednesday, October 28, 2009 5:13 PM > [...] > diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb- > musb.c > index a80441d..529e2b1 100644 > --- a/arch/arm/mach-omap2/usb-musb.c > +++ b/arch/arm/mach-omap2/usb-musb.c > @@ -148,10 +148,14 @@ static struct platform_device musb_device = { > > void __init usb_musb_init(void) > { > - if (cpu_is_omap243x()) > + if (cpu_is_omap243x()) { > musb_resources[0].start = OMAP243X_HS_BASE; > - else > + } else { Do you need {} for a one liner? Checkpatch should have warned you I think.. > musb_resources[0].start = OMAP34XX_HSUSB_OTG_BASE; > + /* OMAP3EVM Rev >= E can source 500mA */ > + if (get_omap3_evm_rev() >= OMAP3EVM_BOARD_GEN_2) > + musb_plat.power = 250; > + } > musb_resources[0].end = musb_resources[0].start + SZ_8K - 1; > > /* > diff --git a/drivers/usb/musb/musb_regs.h b/drivers/usb/musb/musb_regs.h > index cc1d71b..a0314df 100644 > --- a/drivers/usb/musb/musb_regs.h > +++ b/drivers/usb/musb/musb_regs.h > @@ -72,6 +72,10 @@ > #define MUSB_DEVCTL_HR 0x02 > #define MUSB_DEVCTL_SESSION 0x01 > > +/* ULPI VBUSCONTROL */ > +#define ULPI_USE_EXTVBUS 0x01 > +#define ULPI_USE_EXTVBUSIND 0x02 > + > /* TESTMODE */ > #define MUSB_TEST_FORCE_HOST 0x80 > #define MUSB_TEST_FIFO_ACCESS 0x40 > @@ -246,6 +250,7 @@ > > /* REVISIT: vctrl/vstatus: optional vendor utmi+phy register at 0x68 */ > #define MUSB_HWVERS 0x6C /* 8 bit */ > +#define MUSB_ULPI_BUSCONTROL 0x70 /* 8 bit */ > > #define MUSB_EPINFO 0x78 /* 8 bit */ > #define MUSB_RAMINFO 0x79 /* 8 bit */ > diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c > index 6761d20..c5fcc30 100644 > --- a/drivers/usb/musb/omap2430.c > +++ b/drivers/usb/musb/omap2430.c > @@ -36,6 +36,7 @@ > #include <asm/mach-types.h> > #include <mach/hardware.h> > #include <plat/mux.h> > +#include <plat/board.h> > > #include "musb_core.h" > #include "omap2430.h" > @@ -203,6 +204,7 @@ int musb_platform_set_mode(struct musb *musb, u8 > musb_mode) > int __init musb_platform_init(struct musb *musb) > { > u32 l; > + u8 val; > > #if defined(CONFIG_ARCH_OMAP2430) > omap_cfg_reg(AE5_2430_USB0HS_STP); > @@ -239,6 +241,13 @@ int __init musb_platform_init(struct musb *musb) > l |= ULPI_12PIN; > omap_writel(l, OTG_INTERFSEL); > > + /* Program PHY to use external Vbus supply for OMAP3EVM Rev >= E */ > + if (get_omap3_evm_rev() >= OMAP3EVM_BOARD_GEN_2) { > + val = musb_readb(musb->mregs, MUSB_ULPI_BUSCONTROL); > + val |= ULPI_USE_EXTVBUS; > + musb_writeb(musb->mregs, MUSB_ULPI_BUSCONTROL, val); > + } Do we want to do board specific logic inside a IP logic? Wont it be better to do it some way else? Regards, Nishanth Menon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap- > > owner@vger.kernel.org] On Behalf Of Gupta, Ajay Kumar > > Sent: Wednesday, October 28, 2009 5:13 PM > > > [...] > > diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb- > > musb.c > > index a80441d..529e2b1 100644 > > --- a/arch/arm/mach-omap2/usb-musb.c > > +++ b/arch/arm/mach-omap2/usb-musb.c > > @@ -148,10 +148,14 @@ static struct platform_device musb_device = { > > > > void __init usb_musb_init(void) > > { > > - if (cpu_is_omap243x()) > > + if (cpu_is_omap243x()) { > > musb_resources[0].start = OMAP243X_HS_BASE; > > - else > > + } else { > Do you need {} for a one liner? Checkpatch should have warned you I think.. > Er, CodingStyle says you need to do this, if the else clause has brackets. > > musb_resources[0].start = OMAP34XX_HSUSB_OTG_BASE; > > + /* OMAP3EVM Rev >= E can source 500mA */ > > + if (get_omap3_evm_rev() >= OMAP3EVM_BOARD_GEN_2) > > + musb_plat.power = 250; > > + } > > musb_resources[0].end = musb_resources[0].start + SZ_8K - 1; > > > > /* > > diff --git a/drivers/usb/musb/musb_regs.h b/drivers/usb/musb/musb_regs.h > > index cc1d71b..a0314df 100644 > > --- a/drivers/usb/musb/musb_regs.h > > +++ b/drivers/usb/musb/musb_regs.h > > @@ -72,6 +72,10 @@ > > #define MUSB_DEVCTL_HR 0x02 > > #define MUSB_DEVCTL_SESSION 0x01 > > > > +/* ULPI VBUSCONTROL */ > > +#define ULPI_USE_EXTVBUS 0x01 > > +#define ULPI_USE_EXTVBUSIND 0x02 > > + > > /* TESTMODE */ > > #define MUSB_TEST_FORCE_HOST 0x80 > > #define MUSB_TEST_FIFO_ACCESS 0x40 > > @@ -246,6 +250,7 @@ > > > > /* REVISIT: vctrl/vstatus: optional vendor utmi+phy register at 0x68 */ > > #define MUSB_HWVERS 0x6C /* 8 bit */ > > +#define MUSB_ULPI_BUSCONTROL 0x70 /* 8 bit */ > > > > #define MUSB_EPINFO 0x78 /* 8 bit */ > > #define MUSB_RAMINFO 0x79 /* 8 bit */ > > diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c > > index 6761d20..c5fcc30 100644 > > --- a/drivers/usb/musb/omap2430.c > > +++ b/drivers/usb/musb/omap2430.c > > @@ -36,6 +36,7 @@ > > #include <asm/mach-types.h> > > #include <mach/hardware.h> > > #include <plat/mux.h> > > +#include <plat/board.h> > > > > #include "musb_core.h" > > #include "omap2430.h" > > @@ -203,6 +204,7 @@ int musb_platform_set_mode(struct musb *musb, u8 > > musb_mode) > > int __init musb_platform_init(struct musb *musb) > > { > > u32 l; > > + u8 val; > > > > #if defined(CONFIG_ARCH_OMAP2430) > > omap_cfg_reg(AE5_2430_USB0HS_STP); > > @@ -239,6 +241,13 @@ int __init musb_platform_init(struct musb *musb) > > l |= ULPI_12PIN; > > omap_writel(l, OTG_INTERFSEL); > > > > + /* Program PHY to use external Vbus supply for OMAP3EVM Rev >= E */ > > + if (get_omap3_evm_rev() >= OMAP3EVM_BOARD_GEN_2) { > > + val = musb_readb(musb->mregs, MUSB_ULPI_BUSCONTROL); > > + val |= ULPI_USE_EXTVBUS; > > + musb_writeb(musb->mregs, MUSB_ULPI_BUSCONTROL, val); > > + } > Do we want to do board specific logic inside a IP logic? Wont it be better to do it some way else? Maybe add a flag to pass down from the board files? I think other boards might also end up using this. > > Regards, > Nishanth Menon > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Anand, Nishanth, > -----Original Message----- > From: Gadiyar, Anand > Sent: Thursday, October 29, 2009 8:23 AM > To: Menon, Nishanth; Gupta, Ajay Kumar; linux-omap@vger.kernel.org > Cc: felipe.balbi@nokia.com; tony@atomide.com > Subject: RE: [PATCH 3/3] omap3evm: musb: Update power capability for > OMAP3EVM (Rev >= E) > > > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap- > > > owner@vger.kernel.org] On Behalf Of Gupta, Ajay Kumar > > > Sent: Wednesday, October 28, 2009 5:13 PM > > > > > [...] > > > diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb- > > > musb.c > > > index a80441d..529e2b1 100644 > > > --- a/arch/arm/mach-omap2/usb-musb.c > > > +++ b/arch/arm/mach-omap2/usb-musb.c > > > @@ -148,10 +148,14 @@ static struct platform_device musb_device = { > > > > > > void __init usb_musb_init(void) > > > { > > > - if (cpu_is_omap243x()) > > > + if (cpu_is_omap243x()) { > > > musb_resources[0].start = OMAP243X_HS_BASE; > > > - else > > > + } else { > > Do you need {} for a one liner? Checkpatch should have warned you I > think.. > > > > Er, CodingStyle says you need to do this, if the else clause > has brackets. Correct and also checkpatch didn't give any warning for this. > > > > musb_resources[0].start = OMAP34XX_HSUSB_OTG_BASE; > > > + /* OMAP3EVM Rev >= E can source 500mA */ > > > + if (get_omap3_evm_rev() >= OMAP3EVM_BOARD_GEN_2) > > > + musb_plat.power = 250; > > > + } > > > musb_resources[0].end = musb_resources[0].start + SZ_8K - 1; > > > > > > /* > > > diff --git a/drivers/usb/musb/musb_regs.h > b/drivers/usb/musb/musb_regs.h > > > index cc1d71b..a0314df 100644 > > > --- a/drivers/usb/musb/musb_regs.h > > > +++ b/drivers/usb/musb/musb_regs.h > > > @@ -72,6 +72,10 @@ > > > #define MUSB_DEVCTL_HR 0x02 > > > #define MUSB_DEVCTL_SESSION 0x01 > > > > > > +/* ULPI VBUSCONTROL */ > > > +#define ULPI_USE_EXTVBUS 0x01 > > > +#define ULPI_USE_EXTVBUSIND 0x02 > > > + > > > /* TESTMODE */ > > > #define MUSB_TEST_FORCE_HOST 0x80 > > > #define MUSB_TEST_FIFO_ACCESS 0x40 > > > @@ -246,6 +250,7 @@ > > > > > > /* REVISIT: vctrl/vstatus: optional vendor utmi+phy register at 0x68 > */ > > > #define MUSB_HWVERS 0x6C /* 8 bit */ > > > +#define MUSB_ULPI_BUSCONTROL 0x70 /* 8 bit */ > > > > > > #define MUSB_EPINFO 0x78 /* 8 bit */ > > > #define MUSB_RAMINFO 0x79 /* 8 bit */ > > > diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c > > > index 6761d20..c5fcc30 100644 > > > --- a/drivers/usb/musb/omap2430.c > > > +++ b/drivers/usb/musb/omap2430.c > > > @@ -36,6 +36,7 @@ > > > #include <asm/mach-types.h> > > > #include <mach/hardware.h> > > > #include <plat/mux.h> > > > +#include <plat/board.h> > > > > > > #include "musb_core.h" > > > #include "omap2430.h" > > > @@ -203,6 +204,7 @@ int musb_platform_set_mode(struct musb *musb, u8 > > > musb_mode) > > > int __init musb_platform_init(struct musb *musb) > > > { > > > u32 l; > > > + u8 val; > > > > > > #if defined(CONFIG_ARCH_OMAP2430) > > > omap_cfg_reg(AE5_2430_USB0HS_STP); > > > @@ -239,6 +241,13 @@ int __init musb_platform_init(struct musb *musb) > > > l |= ULPI_12PIN; > > > omap_writel(l, OTG_INTERFSEL); > > > > > > + /* Program PHY to use external Vbus supply for OMAP3EVM Rev >= E > */ > > > + if (get_omap3_evm_rev() >= OMAP3EVM_BOARD_GEN_2) { > > > + val = musb_readb(musb->mregs, MUSB_ULPI_BUSCONTROL); > > > + val |= ULPI_USE_EXTVBUS; > > > + musb_writeb(musb->mregs, MUSB_ULPI_BUSCONTROL, val); > > > + } > > Do we want to do board specific logic inside a IP logic? Wont it be > better to do it some way else? We need to access musb controller register to program this so It has to be In musb_platform_init() only but As Anand suggested we can use some flag From board file to avoid omap3_evm specific check here. > > Maybe add a flag to pass down from the board files? I think other > boards might also end up using this. Looks good. I will take this in next revision. -Ajay > > > > > Regards, > > Nishanth Menon > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> From: Gupta, Ajay Kumar > Sent: Thursday, October 29, 2009 7:10 AM > > > > > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap- > > > > owner@vger.kernel.org] On Behalf Of Gupta, Ajay Kumar > > > > Sent: Wednesday, October 28, 2009 5:13 PM > > > > > > > [...] > > > > diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach- > omap2/usb- > > > > musb.c > > > > index a80441d..529e2b1 100644 > > > > --- a/arch/arm/mach-omap2/usb-musb.c > > > > +++ b/arch/arm/mach-omap2/usb-musb.c > > > > @@ -148,10 +148,14 @@ static struct platform_device musb_device = { > > > > > > > > void __init usb_musb_init(void) > > > > { > > > > - if (cpu_is_omap243x()) > > > > + if (cpu_is_omap243x()) { > > > > musb_resources[0].start = OMAP243X_HS_BASE; > > > > - else > > > > + } else { > > > Do you need {} for a one liner? Checkpatch should have warned you I > > think.. > > > > > > > Er, CodingStyle says you need to do this, if the else clause > > has brackets. > > Correct and also checkpatch didn't give any warning for this. Thanks.. > > > > > > > musb_resources[0].start = OMAP34XX_HSUSB_OTG_BASE; > > > > + /* OMAP3EVM Rev >= E can source 500mA */ > > > > + if (get_omap3_evm_rev() >= OMAP3EVM_BOARD_GEN_2) > > > > + musb_plat.power = 250; > > > > + } > > > > musb_resources[0].end = musb_resources[0].start + SZ_8K - 1; > > > > > > > > /* > > > > diff --git a/drivers/usb/musb/musb_regs.h > > b/drivers/usb/musb/musb_regs.h > > > > index cc1d71b..a0314df 100644 > > > > --- a/drivers/usb/musb/musb_regs.h > > > > +++ b/drivers/usb/musb/musb_regs.h > > > > @@ -72,6 +72,10 @@ > > > > #define MUSB_DEVCTL_HR 0x02 > > > > #define MUSB_DEVCTL_SESSION 0x01 > > > > > > > > +/* ULPI VBUSCONTROL */ > > > > +#define ULPI_USE_EXTVBUS 0x01 > > > > +#define ULPI_USE_EXTVBUSIND 0x02 > > > > + > > > > /* TESTMODE */ > > > > #define MUSB_TEST_FORCE_HOST 0x80 > > > > #define MUSB_TEST_FIFO_ACCESS 0x40 > > > > @@ -246,6 +250,7 @@ > > > > > > > > /* REVISIT: vctrl/vstatus: optional vendor utmi+phy register at > 0x68 > > */ > > > > #define MUSB_HWVERS 0x6C /* 8 bit */ > > > > +#define MUSB_ULPI_BUSCONTROL 0x70 /* 8 bit */ > > > > > > > > #define MUSB_EPINFO 0x78 /* 8 bit */ > > > > #define MUSB_RAMINFO 0x79 /* 8 bit */ > > > > diff --git a/drivers/usb/musb/omap2430.c > b/drivers/usb/musb/omap2430.c > > > > index 6761d20..c5fcc30 100644 > > > > --- a/drivers/usb/musb/omap2430.c > > > > +++ b/drivers/usb/musb/omap2430.c > > > > @@ -36,6 +36,7 @@ > > > > #include <asm/mach-types.h> > > > > #include <mach/hardware.h> > > > > #include <plat/mux.h> > > > > +#include <plat/board.h> > > > > > > > > #include "musb_core.h" > > > > #include "omap2430.h" > > > > @@ -203,6 +204,7 @@ int musb_platform_set_mode(struct musb *musb, u8 > > > > musb_mode) > > > > int __init musb_platform_init(struct musb *musb) > > > > { > > > > u32 l; > > > > + u8 val; > > > > > > > > #if defined(CONFIG_ARCH_OMAP2430) > > > > omap_cfg_reg(AE5_2430_USB0HS_STP); > > > > @@ -239,6 +241,13 @@ int __init musb_platform_init(struct musb > *musb) > > > > l |= ULPI_12PIN; > > > > omap_writel(l, OTG_INTERFSEL); > > > > > > > > + /* Program PHY to use external Vbus supply for OMAP3EVM Rev >= > E > > */ > > > > + if (get_omap3_evm_rev() >= OMAP3EVM_BOARD_GEN_2) { > > > > + val = musb_readb(musb->mregs, MUSB_ULPI_BUSCONTROL); > > > > + val |= ULPI_USE_EXTVBUS; > > > > + musb_writeb(musb->mregs, MUSB_ULPI_BUSCONTROL, val); > > > > + } > > > Do we want to do board specific logic inside a IP logic? Wont it be > > better to do it some way else? > > We need to access musb controller register to program this so It has to be > In musb_platform_init() only but As Anand suggested we can use some flag > From board file to avoid omap3_evm specific check here. > > > > > Maybe add a flag to pass down from the board files? I think other > > boards might also end up using this. > > Looks good. I will take this in next revision. Thanks again.. that might be a good thing to have. Regards, Nishanth Menon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Felipe, > -----Original Message----- > From: Menon, Nishanth > Sent: Thursday, October 29, 2009 11:06 AM > To: Gupta, Ajay Kumar; Gadiyar, Anand; linux-omap@vger.kernel.org > Cc: felipe.balbi@nokia.com; tony@atomide.com > Subject: RE: [PATCH 3/3] omap3evm: musb: Update power capability for > OMAP3EVM (Rev >= E) > > > From: Gupta, Ajay Kumar > > Sent: Thursday, October 29, 2009 7:10 AM > > > > > > > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap- > > > > > owner@vger.kernel.org] On Behalf Of Gupta, Ajay Kumar > > > > > Sent: Wednesday, October 28, 2009 5:13 PM > > > > > > > > > [...] > > > > > diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach- > > omap2/usb- > > > > > musb.c > > > > > index a80441d..529e2b1 100644 > > > > > --- a/arch/arm/mach-omap2/usb-musb.c > > > > > +++ b/arch/arm/mach-omap2/usb-musb.c > > > > > @@ -148,10 +148,14 @@ static struct platform_device musb_device = > { > > > > > > > > > > void __init usb_musb_init(void) > > > > > { > > > > > - if (cpu_is_omap243x()) > > > > > + if (cpu_is_omap243x()) { > > > > > musb_resources[0].start = OMAP243X_HS_BASE; > > > > > - else > > > > > + } else { > > > > Do you need {} for a one liner? Checkpatch should have warned you I > > > think.. > > > > > > > > > > Er, CodingStyle says you need to do this, if the else clause > > > has brackets. > > > > Correct and also checkpatch didn't give any warning for this. > Thanks.. > > > > > > > > > > > musb_resources[0].start = OMAP34XX_HSUSB_OTG_BASE; > > > > > + /* OMAP3EVM Rev >= E can source 500mA */ > > > > > + if (get_omap3_evm_rev() >= OMAP3EVM_BOARD_GEN_2) > > > > > + musb_plat.power = 250; > > > > > + } Felipe, How about moving the platform specific data (musb_hdrc_platform_data) moved to board-x.c file from usb-musb.c (same as ehci). This would avoid using omap3evm specific check in usb-musb.c files. -Ajay > > > > > musb_resources[0].end = musb_resources[0].start + SZ_8K - 1; > > > > > > > > > > /* > > > > > diff --git a/drivers/usb/musb/musb_regs.h > > > b/drivers/usb/musb/musb_regs.h > > > > > index cc1d71b..a0314df 100644 > > > > > --- a/drivers/usb/musb/musb_regs.h > > > > > +++ b/drivers/usb/musb/musb_regs.h > > > > > @@ -72,6 +72,10 @@ > > > > > #define MUSB_DEVCTL_HR 0x02 > > > > > #define MUSB_DEVCTL_SESSION 0x01 > > > > > > > > > > +/* ULPI VBUSCONTROL */ > > > > > +#define ULPI_USE_EXTVBUS 0x01 > > > > > +#define ULPI_USE_EXTVBUSIND 0x02 > > > > > + > > > > > /* TESTMODE */ > > > > > #define MUSB_TEST_FORCE_HOST 0x80 > > > > > #define MUSB_TEST_FIFO_ACCESS 0x40 > > > > > @@ -246,6 +250,7 @@ > > > > > > > > > > /* REVISIT: vctrl/vstatus: optional vendor utmi+phy register at > > 0x68 > > > */ > > > > > #define MUSB_HWVERS 0x6C /* 8 bit */ > > > > > +#define MUSB_ULPI_BUSCONTROL 0x70 /* 8 bit */ > > > > > > > > > > #define MUSB_EPINFO 0x78 /* 8 bit */ > > > > > #define MUSB_RAMINFO 0x79 /* 8 bit */ > > > > > diff --git a/drivers/usb/musb/omap2430.c > > b/drivers/usb/musb/omap2430.c > > > > > index 6761d20..c5fcc30 100644 > > > > > --- a/drivers/usb/musb/omap2430.c > > > > > +++ b/drivers/usb/musb/omap2430.c > > > > > @@ -36,6 +36,7 @@ > > > > > #include <asm/mach-types.h> > > > > > #include <mach/hardware.h> > > > > > #include <plat/mux.h> > > > > > +#include <plat/board.h> > > > > > > > > > > #include "musb_core.h" > > > > > #include "omap2430.h" > > > > > @@ -203,6 +204,7 @@ int musb_platform_set_mode(struct musb *musb, > u8 > > > > > musb_mode) > > > > > int __init musb_platform_init(struct musb *musb) > > > > > { > > > > > u32 l; > > > > > + u8 val; > > > > > > > > > > #if defined(CONFIG_ARCH_OMAP2430) > > > > > omap_cfg_reg(AE5_2430_USB0HS_STP); > > > > > @@ -239,6 +241,13 @@ int __init musb_platform_init(struct musb > > *musb) > > > > > l |= ULPI_12PIN; > > > > > omap_writel(l, OTG_INTERFSEL); > > > > > > > > > > + /* Program PHY to use external Vbus supply for OMAP3EVM Rev > >= > > E > > > */ > > > > > + if (get_omap3_evm_rev() >= OMAP3EVM_BOARD_GEN_2) { > > > > > + val = musb_readb(musb->mregs, MUSB_ULPI_BUSCONTROL); > > > > > + val |= ULPI_USE_EXTVBUS; > > > > > + musb_writeb(musb->mregs, MUSB_ULPI_BUSCONTROL, val); > > > > > + } > > > > Do we want to do board specific logic inside a IP logic? Wont it be > > > better to do it some way else? > > > > We need to access musb controller register to program this so It has to > be > > In musb_platform_init() only but As Anand suggested we can use some flag > > From board file to avoid omap3_evm specific check here. > > > > > > > > Maybe add a flag to pass down from the board files? I think other > > > boards might also end up using this. > > > > Looks good. I will take this in next revision. > Thanks again.. that might be a good thing to have. > > Regards, > Nishanth Menon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Thu, Oct 29, 2009 at 06:56:03AM +0100, ext Gupta, Ajay Kumar wrote: > Felipe, > > -----Original Message----- > > From: Menon, Nishanth > > Sent: Thursday, October 29, 2009 11:06 AM > > To: Gupta, Ajay Kumar; Gadiyar, Anand; linux-omap@vger.kernel.org > > Cc: felipe.balbi@nokia.com; tony@atomide.com > > Subject: RE: [PATCH 3/3] omap3evm: musb: Update power capability for > > OMAP3EVM (Rev >= E) > > > > > From: Gupta, Ajay Kumar > > > Sent: Thursday, October 29, 2009 7:10 AM > > > > > > > > > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap- > > > > > > owner@vger.kernel.org] On Behalf Of Gupta, Ajay Kumar > > > > > > Sent: Wednesday, October 28, 2009 5:13 PM > > > > > > > > > > > [...] > > > > > > diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach- > > > omap2/usb- > > > > > > musb.c > > > > > > index a80441d..529e2b1 100644 > > > > > > --- a/arch/arm/mach-omap2/usb-musb.c > > > > > > +++ b/arch/arm/mach-omap2/usb-musb.c > > > > > > @@ -148,10 +148,14 @@ static struct platform_device musb_device = > > { > > > > > > > > > > > > void __init usb_musb_init(void) > > > > > > { > > > > > > - if (cpu_is_omap243x()) > > > > > > + if (cpu_is_omap243x()) { > > > > > > musb_resources[0].start = OMAP243X_HS_BASE; > > > > > > - else > > > > > > + } else { > > > > > Do you need {} for a one liner? Checkpatch should have warned you I > > > > think.. > > > > > > > > > > > > > Er, CodingStyle says you need to do this, if the else clause > > > > has brackets. > > > > > > Correct and also checkpatch didn't give any warning for this. > > Thanks.. > > > > > > > > > > > > > > > musb_resources[0].start = OMAP34XX_HSUSB_OTG_BASE; > > > > > > + /* OMAP3EVM Rev >= E can source 500mA */ > > > > > > + if (get_omap3_evm_rev() >= OMAP3EVM_BOARD_GEN_2) > > > > > > + musb_plat.power = 250; > > > > > > + } > > Felipe, > > How about moving the platform specific data (musb_hdrc_platform_data) moved to board-x.c file from usb-musb.c (same as ehci). This would avoid using omap3evm specific check in usb-musb.c files. I was thinking on adding a musb_hdrc_board_data which would group board-specific data such as this one. Musb's init phase is quite messy as of today so we would need to clean that up. Anyways, the main idea is: board will call usb_musb_init() with a musb_hdrc_board_data * as parameter. usb-musb would still hold a static struct musb_hdrc_platform_data for the (in our case) OMAP-specific init, those would be passed down to driver and init phase would be done as following: musb_init() -> musb_platform_init() -> musb_board_init() we could also have board_ops and platform_ops structures for the function pointers to be passed to musb_core.c Then all init could be done there. What do you say ???
> -----Original Message----- > From: Felipe Balbi [mailto:felipe.balbi@nokia.com] > Sent: Thursday, October 29, 2009 12:29 PM > To: Gupta, Ajay Kumar > Cc: linux-omap@vger.kernel.org; Balbi Felipe (Nokia-D/Helsinki); Menon, > Nishanth; Gadiyar, Anand > Subject: Re: [PATCH 3/3] omap3evm: musb: Update power capability for > OMAP3EVM (Rev >= E) > > Hi, > > On Thu, Oct 29, 2009 at 06:56:03AM +0100, ext Gupta, Ajay Kumar wrote: > > Felipe, > > > -----Original Message----- > > > From: Menon, Nishanth > > > Sent: Thursday, October 29, 2009 11:06 AM > > > To: Gupta, Ajay Kumar; Gadiyar, Anand; linux-omap@vger.kernel.org > > > Cc: felipe.balbi@nokia.com; tony@atomide.com > > > Subject: RE: [PATCH 3/3] omap3evm: musb: Update power capability for > > > OMAP3EVM (Rev >= E) > > > > > > > From: Gupta, Ajay Kumar > > > > Sent: Thursday, October 29, 2009 7:10 AM > > > > > > > > > > > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap- > > > > > > > owner@vger.kernel.org] On Behalf Of Gupta, Ajay Kumar > > > > > > > Sent: Wednesday, October 28, 2009 5:13 PM > > > > > > > > > > > > > [...] > > > > > > > diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach- > > > > omap2/usb- > > > > > > > musb.c > > > > > > > index a80441d..529e2b1 100644 > > > > > > > --- a/arch/arm/mach-omap2/usb-musb.c > > > > > > > +++ b/arch/arm/mach-omap2/usb-musb.c > > > > > > > @@ -148,10 +148,14 @@ static struct platform_device > musb_device = > > > { > > > > > > > > > > > > > > void __init usb_musb_init(void) > > > > > > > { > > > > > > > - if (cpu_is_omap243x()) > > > > > > > + if (cpu_is_omap243x()) { > > > > > > > musb_resources[0].start = OMAP243X_HS_BASE; > > > > > > > - else > > > > > > > + } else { > > > > > > Do you need {} for a one liner? Checkpatch should have warned > you I > > > > > think.. > > > > > > > > > > > > > > > > Er, CodingStyle says you need to do this, if the else clause > > > > > has brackets. > > > > > > > > Correct and also checkpatch didn't give any warning for this. > > > Thanks.. > > > > > > > > > > > > > > > > > > > musb_resources[0].start = > OMAP34XX_HSUSB_OTG_BASE; > > > > > > > + /* OMAP3EVM Rev >= E can source 500mA */ > > > > > > > + if (get_omap3_evm_rev() >= OMAP3EVM_BOARD_GEN_2) > > > > > > > + musb_plat.power = 250; > > > > > > > + } > > > > Felipe, > > > > How about moving the platform specific data (musb_hdrc_platform_data) > moved to board-x.c file from usb-musb.c (same as ehci). This would avoid > using omap3evm specific check in usb-musb.c files. > > I was thinking on adding a musb_hdrc_board_data which would group > board-specific data such as this one. > > Musb's init phase is quite messy as of today so we would need to clean > that up. Anyways, the main idea is: > > board will call usb_musb_init() with a musb_hdrc_board_data * as > parameter. usb-musb would still hold a static struct > musb_hdrc_platform_data for the (in our case) OMAP-specific init, those > would be passed down to driver and init phase would be done as > following: > > musb_init() > -> musb_platform_init() > -> musb_board_init() > > we could also have board_ops and platform_ops structures for the > function pointers to be passed to musb_core.c Then all init could be > done there. > > What do you say ??? This looks really good to isolate board specific settings. Do you have any ready patch on this ? -Ajay > > -- > balbi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Thu, Oct 29, 2009 at 08:03:21AM +0100, ext Gupta, Ajay Kumar wrote: > > I was thinking on adding a musb_hdrc_board_data which would group > > board-specific data such as this one. > > > > Musb's init phase is quite messy as of today so we would need to clean > > that up. Anyways, the main idea is: > > > > board will call usb_musb_init() with a musb_hdrc_board_data * as > > parameter. usb-musb would still hold a static struct > > musb_hdrc_platform_data for the (in our case) OMAP-specific init, those > > would be passed down to driver and init phase would be done as > > following: > > > > musb_init() > > -> musb_platform_init() > > -> musb_board_init() > > > > we could also have board_ops and platform_ops structures for the > > function pointers to be passed to musb_core.c Then all init could be > > done there. > > > > What do you say ??? > > This looks really good to isolate board specific settings. Do you have any > ready patch on this ? no, currently I'm working on refactoring the otg support. I could work on this after I finish otg refactoring unless you want to take this task :-)
Hi, > On Thu, Oct 29, 2009 at 08:03:21AM +0100, ext Gupta, Ajay Kumar wrote: > > > I was thinking on adding a musb_hdrc_board_data which would group > > > board-specific data such as this one. > > > > > > Musb's init phase is quite messy as of today so we would need to clean > > > that up. Anyways, the main idea is: > > > > > > board will call usb_musb_init() with a musb_hdrc_board_data * as > > > parameter. usb-musb would still hold a static struct > > > musb_hdrc_platform_data for the (in our case) OMAP-specific init, > those > > > would be passed down to driver and init phase would be done as > > > following: > > > > > > musb_init() > > > -> musb_platform_init() > > > -> musb_board_init() > > > > > > we could also have board_ops and platform_ops structures for the > > > function pointers to be passed to musb_core.c Then all init could be > > > done there. > > > > > > What do you say ??? > > > > This looks really good to isolate board specific settings. Do you have > any > > ready patch on this ? > > no, currently I'm working on refactoring the otg support. I could work > on this after I finish otg refactoring unless you want to take this task > :-) Sure, I will submit the first cut very soon. -Ajay > > -- > balbi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Oct 29, 2009 at 11:29:39AM +0100, ext Gupta, Ajay Kumar wrote: > Hi, > > On Thu, Oct 29, 2009 at 08:03:21AM +0100, ext Gupta, Ajay Kumar wrote: > > > > I was thinking on adding a musb_hdrc_board_data which would group > > > > board-specific data such as this one. > > > > > > > > Musb's init phase is quite messy as of today so we would need to clean > > > > that up. Anyways, the main idea is: > > > > > > > > board will call usb_musb_init() with a musb_hdrc_board_data * as > > > > parameter. usb-musb would still hold a static struct > > > > musb_hdrc_platform_data for the (in our case) OMAP-specific init, > > those > > > > would be passed down to driver and init phase would be done as > > > > following: > > > > > > > > musb_init() > > > > -> musb_platform_init() > > > > -> musb_board_init() > > > > > > > > we could also have board_ops and platform_ops structures for the > > > > function pointers to be passed to musb_core.c Then all init could be > > > > done there. > > > > > > > > What do you say ??? > > > > > > This looks really good to isolate board specific settings. Do you have > > any > > > ready patch on this ? > > > > no, currently I'm working on refactoring the otg support. I could work > > on this after I finish otg refactoring unless you want to take this task > > :-) > > Sure, I will submit the first cut very soon. We could discuss further off list about the design and implementation details. Just drop a mail :-)
Patch
diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c index a80441d..529e2b1 100644 --- a/arch/arm/mach-omap2/usb-musb.c +++ b/arch/arm/mach-omap2/usb-musb.c @@ -148,10 +148,14 @@ static struct platform_device musb_device = { void __init usb_musb_init(void) { - if (cpu_is_omap243x()) + if (cpu_is_omap243x()) { musb_resources[0].start = OMAP243X_HS_BASE; - else + } else { musb_resources[0].start = OMAP34XX_HSUSB_OTG_BASE; + /* OMAP3EVM Rev >= E can source 500mA */ + if (get_omap3_evm_rev() >= OMAP3EVM_BOARD_GEN_2) + musb_plat.power = 250; + } musb_resources[0].end = musb_resources[0].start + SZ_8K - 1; /* diff --git a/drivers/usb/musb/musb_regs.h b/drivers/usb/musb/musb_regs.h index cc1d71b..a0314df 100644 --- a/drivers/usb/musb/musb_regs.h +++ b/drivers/usb/musb/musb_regs.h @@ -72,6 +72,10 @@ #define MUSB_DEVCTL_HR 0x02 #define MUSB_DEVCTL_SESSION 0x01 +/* ULPI VBUSCONTROL */ +#define ULPI_USE_EXTVBUS 0x01 +#define ULPI_USE_EXTVBUSIND 0x02 + /* TESTMODE */ #define MUSB_TEST_FORCE_HOST 0x80 #define MUSB_TEST_FIFO_ACCESS 0x40 @@ -246,6 +250,7 @@ /* REVISIT: vctrl/vstatus: optional vendor utmi+phy register at 0x68 */ #define MUSB_HWVERS 0x6C /* 8 bit */ +#define MUSB_ULPI_BUSCONTROL 0x70 /* 8 bit */ #define MUSB_EPINFO 0x78 /* 8 bit */ #define MUSB_RAMINFO 0x79 /* 8 bit */ diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c index 6761d20..c5fcc30 100644 --- a/drivers/usb/musb/omap2430.c +++ b/drivers/usb/musb/omap2430.c @@ -36,6 +36,7 @@ #include <asm/mach-types.h> #include <mach/hardware.h> #include <plat/mux.h> +#include <plat/board.h> #include "musb_core.h" #include "omap2430.h" @@ -203,6 +204,7 @@ int musb_platform_set_mode(struct musb *musb, u8 musb_mode) int __init musb_platform_init(struct musb *musb) { u32 l; + u8 val; #if defined(CONFIG_ARCH_OMAP2430) omap_cfg_reg(AE5_2430_USB0HS_STP); @@ -239,6 +241,13 @@ int __init musb_platform_init(struct musb *musb) l |= ULPI_12PIN; omap_writel(l, OTG_INTERFSEL); + /* Program PHY to use external Vbus supply for OMAP3EVM Rev >= E */ + if (get_omap3_evm_rev() >= OMAP3EVM_BOARD_GEN_2) { + val = musb_readb(musb->mregs, MUSB_ULPI_BUSCONTROL); + val |= ULPI_USE_EXTVBUS; + musb_writeb(musb->mregs, MUSB_ULPI_BUSCONTROL, val); + } + pr_debug("HS USB OTG: revision 0x%x, sysconfig 0x%02x, " "sysstatus 0x%x, intrfsel 0x%x, simenable 0x%x\n", omap_readl(OTG_REVISION), omap_readl(OTG_SYSCONFIG),
Updated the MUSB power sourcing capability for OMAP3EVM (Rev >=E). MUSB interface can source 500mA on OMAP3EVM Rev >= E while Rev < E supports only 100mA.MUSB PHY is programmed to use external Vbus for this. Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com> --- This patch is dependent on below patch, [PATCH] omap3evm: Add board revision function arch/arm/mach-omap2/usb-musb.c | 8 ++++++-- drivers/usb/musb/musb_regs.h | 5 +++++ drivers/usb/musb/omap2430.c | 9 +++++++++ 3 files changed, 20 insertions(+), 2 deletions(-)