Re: [PATCH 1/2] AM35x: musb: fix compilation error
On Tue, Dec 07, 2010 at 07:25:12PM +0530, Gupta, Ajay Kumar wrote: Hi, >> Is AM35x that different ? Can't you just write to MUSB_INTRRX >> MUSB_INTRTX and MUSB_INTRUSB ?? >Writing to MUSB_INTRRX/TX/USB wouldn't help. We have to clear interrupt >Bit in control register INTR_LVL_CLR. I see, thanks for the info :-) Felipe, I have recreated this patch (attached) on top of your patch set and AM35x is working. applied it. Thanks -- balbi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] AM35x: musb: fix compilation error
Hi, > >> Is AM35x that different ? Can't you just write to MUSB_INTRRX > >> MUSB_INTRTX and MUSB_INTRUSB ?? > >Writing to MUSB_INTRRX/TX/USB wouldn't help. We have to clear interrupt > >Bit in control register INTR_LVL_CLR. > > I see, thanks for the info :-) Felipe, I have recreated this patch (attached) on top of your patch set and AM35x is working. Thanks, Ajay > > -- > balbi 0001-musb-am35x-fix-compile-error-due-to-control-apis.patch Description: 0001-musb-am35x-fix-compile-error-due-to-control-apis.patch
Re: [PATCH 1/2] AM35x: musb: fix compilation error
On Mon, Dec 06, 2010 at 05:24:53PM +0530, Gupta, Ajay Kumar wrote: On Fri, Dec 03, 2010 at 07:08:53PM +0530, Gupta, Ajay Kumar wrote: >> >We already have generic APIs so I think we can pass function pointers to >> >musb driver via "struct omap_musb_board_data", >> > >> > struct omap_musb_board_data { >> >+ void(*phy_on) (void) >> >+ void(*phy_off) (void) >> >+ void (*intr_clr) (void) >> > } >> > >> >Reset part can be done in board file itself same as Ethernet driver is >> doing. >> > >> >Does this look fine? >> >> so those would be "turn phy on", "turn phy off" and "clear interrupt" >> apis ? >> >> How about: >> int (*set_phy_power)(unsigned on); >Looks good. > >> void (*clear_phy_irq)(void); >This is actually musb ip interrupt clear so I will make it like, > >void (*clear_irq) (void); Is AM35x that different ? Can't you just write to MUSB_INTRRX MUSB_INTRTX and MUSB_INTRUSB ?? Writing to MUSB_INTRRX/TX/USB wouldn't help. We have to clear interrupt Bit in control register INTR_LVL_CLR. I see, thanks for the info :-) -- balbi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] AM35x: musb: fix compilation error
> On Fri, Dec 03, 2010 at 07:08:53PM +0530, Gupta, Ajay Kumar wrote: > >> >We already have generic APIs so I think we can pass function pointers > to > >> >musb driver via "struct omap_musb_board_data", > >> > > >> > struct omap_musb_board_data { > >> >+ void(*phy_on) (void) > >> >+ void(*phy_off) (void) > >> >+ void (*intr_clr) (void) > >> > } > >> > > >> >Reset part can be done in board file itself same as Ethernet driver is > >> doing. > >> > > >> >Does this look fine? > >> > >> so those would be "turn phy on", "turn phy off" and "clear interrupt" > >> apis ? > >> > >> How about: > >> int (*set_phy_power)(unsigned on); > >Looks good. > > > >> void (*clear_phy_irq)(void); > >This is actually musb ip interrupt clear so I will make it like, > > > >void (*clear_irq) (void); > > Is AM35x that different ? Can't you just write to MUSB_INTRRX > MUSB_INTRTX and MUSB_INTRUSB ?? Writing to MUSB_INTRRX/TX/USB wouldn't help. We have to clear interrupt Bit in control register INTR_LVL_CLR. -Ajay > > -- > balbi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] AM35x: musb: fix compilation error
On Fri, Dec 03, 2010 at 07:08:53PM +0530, Gupta, Ajay Kumar wrote: >We already have generic APIs so I think we can pass function pointers to >musb driver via "struct omap_musb_board_data", > > struct omap_musb_board_data { >+ void(*phy_on) (void) >+ void(*phy_off) (void) >+ void (*intr_clr) (void) > } > >Reset part can be done in board file itself same as Ethernet driver is doing. > >Does this look fine? so those would be "turn phy on", "turn phy off" and "clear interrupt" apis ? How about: int (*set_phy_power)(unsigned on); Looks good. void (*clear_phy_irq)(void); This is actually musb ip interrupt clear so I will make it like, void (*clear_irq) (void); Is AM35x that different ? Can't you just write to MUSB_INTRRX MUSB_INTRTX and MUSB_INTRUSB ?? -- balbi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] AM35x: musb: fix compilation error
> >We already have generic APIs so I think we can pass function pointers to > >musb driver via "struct omap_musb_board_data", > > > > struct omap_musb_board_data { > >+void(*phy_on) (void) > >+void(*phy_off) (void) > >+void (*intr_clr) (void) > > } > > > >Reset part can be done in board file itself same as Ethernet driver is > doing. > > > >Does this look fine? > > so those would be "turn phy on", "turn phy off" and "clear interrupt" > apis ? > > How about: > int (*set_phy_power)(unsigned on); Looks good. > void (*clear_phy_irq)(void); This is actually musb ip interrupt clear so I will make it like, void (*clear_irq) (void); > > You'd need one less API for that. BTW, could you do it on top of my > musb-hw branch ? Then I can push together with those for merge window. Not yet, I will send once completed. Thanks, Ajay > > -- > balbi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] AM35x: musb: fix compilation error
On Thu, Dec 02, 2010 at 04:13:50PM +0530, Gupta, Ajay Kumar wrote: >> why don't you add a proper otg_transceiver driver for am35x ? >> Is it like omap4's internal phy ? A separate block ? > >AM35x PHY is built inside the ip and we need to configure it through >PHY control register. > >Additionally we also need to access IPSS reset and Intr clear register >as well which would not fit inside otg_transceiver. how about passing an extra struct resource if am35x ? Then you could ioremap that base on am35x.c and use normal musb_read/write functions. We already have generic APIs so I think we can pass function pointers to musb driver via "struct omap_musb_board_data", struct omap_musb_board_data { + void(*phy_on) (void) + void(*phy_off) (void) + void (*intr_clr) (void) } Reset part can be done in board file itself same as Ethernet driver is doing. Does this look fine? so those would be "turn phy on", "turn phy off" and "clear interrupt" apis ? How about: int (*set_phy_power)(unsigned on); void (*clear_phy_irq)(void); You'd need one less API for that. BTW, could you do it on top of my musb-hw branch ? Then I can push together with those for merge window. -- balbi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] AM35x: musb: fix compilation error
> >> why don't you add a proper otg_transceiver driver for am35x ? > >> Is it like omap4's internal phy ? A separate block ? > > > >AM35x PHY is built inside the ip and we need to configure it through > >PHY control register. > > > >Additionally we also need to access IPSS reset and Intr clear register > >as well which would not fit inside otg_transceiver. > > how about passing an extra struct resource if am35x ? Then you could > ioremap that base on am35x.c and use normal musb_read/write functions. We already have generic APIs so I think we can pass function pointers to musb driver via "struct omap_musb_board_data", struct omap_musb_board_data { + void(*phy_on) (void) + void(*phy_off) (void) + void (*intr_clr) (void) } Reset part can be done in board file itself same as Ethernet driver is doing. Does this look fine? -Ajay > > -- > balbi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] AM35x: musb: fix compilation error
On Thu, Dec 02, 2010 at 10:22:11AM +0530, Gupta, Ajay Kumar wrote: > We would need control register apis for accessing USB PHY control > , IPSS reset and interrupt clear register. This would require to > add three different function pointer and that would mostly be > custom to AM35x. Will that be acceptable from musb perspective ? why don't you add a proper otg_transceiver driver for am35x ? Is it like omap4's internal phy ? A separate block ? AM35x PHY is built inside the ip and we need to configure it through PHY control register. Additionally we also need to access IPSS reset and Intr clear register as well which would not fit inside otg_transceiver. how about passing an extra struct resource if am35x ? Then you could ioremap that base on am35x.c and use normal musb_read/write functions. -- balbi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] AM35x: musb: fix compilation error
> > We would need control register apis for accessing USB PHY control > > , IPSS reset and interrupt clear register. This would require to > > add three different function pointer and that would mostly be > > custom to AM35x. Will that be acceptable from musb perspective ? > > why don't you add a proper otg_transceiver driver for am35x ? > Is it like omap4's internal phy ? A separate block ? AM35x PHY is built inside the ip and we need to configure it through PHY control register. Additionally we also need to access IPSS reset and Intr clear register as well which would not fit inside otg_transceiver. Ajay > > -- > balbi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] AM35x: musb: fix compilation error
On Wed, 2010-12-01 at 17:38 +0530, Gupta, Ajay Kumar wrote: > > >> Tony ? Do you ack the usage of that header ? > > > > > >NAK. Drivers should not mess with the control registers directly. > > > > > >Instead, the following should be done in the platform init code: > > > > > >$ grep -r omap_ctrl_read drivers/usb > > >drivers/usb/musb/am35x.c: devconf2 = > > omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); > > >drivers/usb/musb/am35x.c: while > > (!(omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2) > > >drivers/usb/musb/am35x.c: devconf2 = > > omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); > > >drivers/usb/musb/am35x.c: lvl_intr = > > omap_ctrl_readl(AM35XX_CONTROL_LVL_INTR_CLEAR); > > >drivers/usb/musb/am35x.c: u32 devconf2 = > > omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); > > >drivers/usb/musb/am35x.c: sw_reset = > > omap_ctrl_readl(AM35XX_CONTROL_IP_SW_RESET); > > >drivers/usb/musb/am35x.c: lvl_intr = > > omap_ctrl_readl(AM35XX_CONTROL_LVL_INTR_CLEAR); > > > > > >You can pass a function pointer like board_set_power or simila in > > >platform_data. > > We would need control register apis for accessing USB PHY control > , IPSS reset and interrupt clear register. This would require to > add three different function pointer and that would mostly be > custom to AM35x. Will that be acceptable from musb perspective ? why don't you add a proper otg_transceiver driver for am35x ? Is it like omap4's internal phy ? A separate block ? -- balbi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] AM35x: musb: fix compilation error
> >> Tony ? Do you ack the usage of that header ? > > > >NAK. Drivers should not mess with the control registers directly. > > > >Instead, the following should be done in the platform init code: > > > >$ grep -r omap_ctrl_read drivers/usb > >drivers/usb/musb/am35x.c: devconf2 = > omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); > >drivers/usb/musb/am35x.c: while > (!(omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2) > >drivers/usb/musb/am35x.c: devconf2 = > omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); > >drivers/usb/musb/am35x.c: lvl_intr = > omap_ctrl_readl(AM35XX_CONTROL_LVL_INTR_CLEAR); > >drivers/usb/musb/am35x.c: u32 devconf2 = > omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); > >drivers/usb/musb/am35x.c: sw_reset = > omap_ctrl_readl(AM35XX_CONTROL_IP_SW_RESET); > >drivers/usb/musb/am35x.c: lvl_intr = > omap_ctrl_readl(AM35XX_CONTROL_LVL_INTR_CLEAR); > > > >You can pass a function pointer like board_set_power or simila in > >platform_data. We would need control register apis for accessing USB PHY control , IPSS reset and interrupt clear register. This would require to add three different function pointer and that would mostly be custom to AM35x. Will that be acceptable from musb perspective ? -Ajay > > That was my feeling too. Anand, care to update ?!? > > -- > balbi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] AM35x: musb: fix compilation error
On Tue, Nov 30, 2010 at 10:30:10AM -0800, Tony Lindgren wrote: * Felipe Balbi [101122 02:12]: Hi, On Mon, Nov 22, 2010 at 02:22:40PM +0530, Ajay Kumar Gupta wrote: >Fixes compilation error as control.h APIs are not available to >drivers/usb/musb/am35x.c file. Earlier it was getting included >from but now moved to new location by another >pacth. > >Signed-off-by: Ajay Kumar Gupta Tony ? Do you ack the usage of that header ? NAK. Drivers should not mess with the control registers directly. Instead, the following should be done in the platform init code: $ grep -r omap_ctrl_read drivers/usb drivers/usb/musb/am35x.c: devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); drivers/usb/musb/am35x.c: while (!(omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2) drivers/usb/musb/am35x.c: devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); drivers/usb/musb/am35x.c: lvl_intr = omap_ctrl_readl(AM35XX_CONTROL_LVL_INTR_CLEAR); drivers/usb/musb/am35x.c: u32 devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); drivers/usb/musb/am35x.c: sw_reset = omap_ctrl_readl(AM35XX_CONTROL_IP_SW_RESET); drivers/usb/musb/am35x.c: lvl_intr = omap_ctrl_readl(AM35XX_CONTROL_LVL_INTR_CLEAR); You can pass a function pointer like board_set_power or simila in platform_data. That was my feeling too. Anand, care to update ?!? -- balbi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] AM35x: musb: fix compilation error
* Felipe Balbi [101122 02:12]: > Hi, > > On Mon, Nov 22, 2010 at 02:22:40PM +0530, Ajay Kumar Gupta wrote: > >Fixes compilation error as control.h APIs are not available to > >drivers/usb/musb/am35x.c file. Earlier it was getting included > >from but now moved to new location by another > >pacth. > > > >Signed-off-by: Ajay Kumar Gupta > > Tony ? Do you ack the usage of that header ? NAK. Drivers should not mess with the control registers directly. Instead, the following should be done in the platform init code: $ grep -r omap_ctrl_read drivers/usb drivers/usb/musb/am35x.c: devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); drivers/usb/musb/am35x.c: while (!(omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2) drivers/usb/musb/am35x.c: devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); drivers/usb/musb/am35x.c: lvl_intr = omap_ctrl_readl(AM35XX_CONTROL_LVL_INTR_CLEAR); drivers/usb/musb/am35x.c: u32 devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); drivers/usb/musb/am35x.c: sw_reset = omap_ctrl_readl(AM35XX_CONTROL_IP_SW_RESET); drivers/usb/musb/am35x.c: lvl_intr = omap_ctrl_readl(AM35XX_CONTROL_LVL_INTR_CLEAR); You can pass a function pointer like board_set_power or simila in platform_data. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] AM35x: musb: fix compilation error
Hi, On Mon, Nov 22, 2010 at 02:22:40PM +0530, Ajay Kumar Gupta wrote: Fixes compilation error as control.h APIs are not available to drivers/usb/musb/am35x.c file. Earlier it was getting included from but now moved to new location by another pacth. Signed-off-by: Ajay Kumar Gupta Tony ? Do you ack the usage of that header ? --- Patch created against latest Linus's tree (v2.6.37-rc3) arch/arm/plat-omap/include/plat/usb.h |2 ++ drivers/usb/musb/am35x.c |1 - 2 files changed, 2 insertions(+), 1 deletions(-) diff --git a/arch/arm/plat-omap/include/plat/usb.h b/arch/arm/plat-omap/include/plat/usb.h index 59c7fe7..f1b8b40 100644 --- a/arch/arm/plat-omap/include/plat/usb.h +++ b/arch/arm/plat-omap/include/plat/usb.h @@ -6,6 +6,8 @@ #include #include +#include "../../../mach-omap2/control.h" + #define OMAP3_HS_USB_PORTS 3 enum ehci_hcd_omap_mode { EHCI_HCD_OMAP_MODE_UNKNOWN, diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c index b0aabf3..30f524a 100644 --- a/drivers/usb/musb/am35x.c +++ b/drivers/usb/musb/am35x.c @@ -30,7 +30,6 @@ #include #include -#include #include #include "musb_core.h" -- 1.6.2.4 -- balbi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] AM35x: musb: fix compilation error
Fixes compilation error as control.h APIs are not available to drivers/usb/musb/am35x.c file. Earlier it was getting included from but now moved to new location by another pacth. Signed-off-by: Ajay Kumar Gupta --- Patch created against latest Linus's tree (v2.6.37-rc3) arch/arm/plat-omap/include/plat/usb.h |2 ++ drivers/usb/musb/am35x.c |1 - 2 files changed, 2 insertions(+), 1 deletions(-) diff --git a/arch/arm/plat-omap/include/plat/usb.h b/arch/arm/plat-omap/include/plat/usb.h index 59c7fe7..f1b8b40 100644 --- a/arch/arm/plat-omap/include/plat/usb.h +++ b/arch/arm/plat-omap/include/plat/usb.h @@ -6,6 +6,8 @@ #include #include +#include "../../../mach-omap2/control.h" + #define OMAP3_HS_USB_PORTS 3 enum ehci_hcd_omap_mode { EHCI_HCD_OMAP_MODE_UNKNOWN, diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c index b0aabf3..30f524a 100644 --- a/drivers/usb/musb/am35x.c +++ b/drivers/usb/musb/am35x.c @@ -30,7 +30,6 @@ #include #include -#include #include #include "musb_core.h" -- 1.6.2.4 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html