Re: [PATCH 4/4] omap: musb: introduce default baord config
* Felipe Balbi ba...@ti.com [110502 07:22]: On Mon, May 02, 2011 at 07:20:52AM -0700, Tony Lindgren wrote: * Oleg Drokin gr...@linuxhacker.ru [110428 09:33]: So to me it looks like something totally in realm of musb driver itself. Nothing bad happens if you configure your MUSB as say OTG while in fact only peripheral mode was implemented, it continues to work as it did. Of course enabling HOST mode may not magically make things work, but I suspect this could be addressed from Kconfig itself instead. So based on the discussion it sounds like we still the board specific configuration for the mode in some cases. I think Felipe already has some patches to remove the various Kconfig options for musb? In any case, the musb configuration should be a runtime configuration passed in the platform data or cmdline. Yeah, I'm planning to get rid of all the ifdeferry and always compile it with HOST and Peripheral support. It's only few extra tens of bytes anyway. Still, I doubt I can get that done for this merge window, I already have pending the debugging rework. So care to ack the original patch then? It still allows overriding the default mode from board-*.c file as needed. 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 4/4] omap: musb: introduce default baord config
* Oleg Drokin gr...@linuxhacker.ru [110428 09:33]: Hello! On Apr 28, 2011, at 12:28 PM, Russell King - ARM Linux wrote: mm, it looks to me like we're ending up with two layers on top of each other, both trying to provide some kind of generic board interface. I think they should be squashed together. And that: static struct musb_hdrc_platform_data musb_plat = { #ifdef CONFIG_USB_MUSB_OTG .mode = MUSB_OTG, #elif defined(CONFIG_USB_MUSB_HDRC_HCD) .mode = MUSB_HOST, #elif defined(CONFIG_USB_GADGET_MUSB_HDRC) .mode = MUSB_PERIPHERAL, #endif in usb-musb.c needs the same treatment as I mentioned in the previous message if it really is board specific. If not, I see no reason why the above can't go into the musb driver itself. In the end it's chip dependent. And the musb can work in all three modes. Of course the board dictates if the power is supplied to the bus in host mode and such, but even that could be worked around as nokia 9x0 saga for USB host shows. So to me it looks like something totally in realm of musb driver itself. Nothing bad happens if you configure your MUSB as say OTG while in fact only peripheral mode was implemented, it continues to work as it did. Of course enabling HOST mode may not magically make things work, but I suspect this could be addressed from Kconfig itself instead. Now I totally expect musb maintainer to jump in and explain how many misconceptions I have ;) As far as I remember MUSB always relies on the OTG hardware to enable the host mode.. So I don't even know if the driver is usable with host only configuration. And for the peripheral mode, it should be possible to have OTG enabled, for cert testing some strings need to be changed in that case for peripheral only configuration. I think Felipe already has some patches to remove the various Kconfig options for musb? In any case, the musb configuration should be a runtime configuration passed in the platform data or cmdline. 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 4/4] omap: musb: introduce default baord config
On Mon, May 02, 2011 at 07:20:52AM -0700, Tony Lindgren wrote: * Oleg Drokin gr...@linuxhacker.ru [110428 09:33]: Hello! On Apr 28, 2011, at 12:28 PM, Russell King - ARM Linux wrote: mm, it looks to me like we're ending up with two layers on top of each other, both trying to provide some kind of generic board interface. I think they should be squashed together. And that: static struct musb_hdrc_platform_data musb_plat = { #ifdef CONFIG_USB_MUSB_OTG .mode = MUSB_OTG, #elif defined(CONFIG_USB_MUSB_HDRC_HCD) .mode = MUSB_HOST, #elif defined(CONFIG_USB_GADGET_MUSB_HDRC) .mode = MUSB_PERIPHERAL, #endif in usb-musb.c needs the same treatment as I mentioned in the previous message if it really is board specific. If not, I see no reason why the above can't go into the musb driver itself. In the end it's chip dependent. And the musb can work in all three modes. Of course the board dictates if the power is supplied to the bus in host mode and such, but even that could be worked around as nokia 9x0 saga for USB host shows. So to me it looks like something totally in realm of musb driver itself. Nothing bad happens if you configure your MUSB as say OTG while in fact only peripheral mode was implemented, it continues to work as it did. Of course enabling HOST mode may not magically make things work, but I suspect this could be addressed from Kconfig itself instead. Now I totally expect musb maintainer to jump in and explain how many misconceptions I have ;) As far as I remember MUSB always relies on the OTG hardware to enable the host mode.. So I don't even know if the driver is usable with host only configuration. And for the peripheral mode, it should be yes, it is :-) possible to have OTG enabled, for cert testing some strings need to be changed in that case for peripheral only configuration. I think Felipe already has some patches to remove the various Kconfig options for musb? In any case, the musb configuration should be a runtime configuration passed in the platform data or cmdline. Yeah, I'm planning to get rid of all the ifdeferry and always compile it with HOST and Peripheral support. It's only few extra tens of bytes anyway. Still, I doubt I can get that done for this merge window, I already have pending the debugging rework. -- 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 4/4] omap: musb: introduce default baord config
On Wed, Apr 27, 2011 at 12:23:18AM -0400, Oleg Drokin wrote: Hello! On Apr 24, 2011, at 6:09 PM, Mike Rapoport wrote: -void __init usb_musb_init(struct omap_musb_board_data *board_data) +static struct omap_musb_board_data musb_default_board_data = { + .interface_type = MUSB_INTERFACE_ULPI, + .mode = MUSB_OTG, In fact can you make it more generic with ifdefs like this? (since there are tons of boards taht are client only or would like to be compiled in some other way): +#ifdef CONFIG_USB_MUSB_OTG + .mode = MUSB_OTG, +#elif defined(CONFIG_USB_MUSB_HDRC_HCD) + .mode = MUSB_HOST, +#elif defined(CONFIG_USB_GADGET_MUSB_HDRC) + .mode = MUSB_PERIPHERAL, +#endif If this is something that's desired, then it should not be done via ifdefs but by platforms passing the mode argument into an initialization function. Eventually, when we switch to some DT like system, this is the kind of information that would be obtained from DT, and identifying this stuff now will help when DT stuff comes along. -- 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 4/4] omap: musb: introduce default baord config
Hello! On Apr 28, 2011, at 10:18 AM, Russell King - ARM Linux wrote: -void __init usb_musb_init(struct omap_musb_board_data *board_data) +static struct omap_musb_board_data musb_default_board_data = { + .interface_type = MUSB_INTERFACE_ULPI, + .mode = MUSB_OTG, In fact can you make it more generic with ifdefs like this? (since there are tons of boards taht are client only or would like to be compiled in some other way): +#ifdef CONFIG_USB_MUSB_OTG + .mode = MUSB_OTG, +#elif defined(CONFIG_USB_MUSB_HDRC_HCD) + .mode = MUSB_HOST, +#elif defined(CONFIG_USB_GADGET_MUSB_HDRC) + .mode = MUSB_PERIPHERAL, +#endif If this is something that's desired, then it should not be done via ifdefs but by platforms passing the mode argument into an initialization function. Eventually, when we switch to some DT like system, this is the kind of information that would be obtained from DT, and identifying this stuff now will help when DT stuff comes along. Frankly, I am not even sure why the mode needs to be supplied by the board at all. The musb code already has a very similar switch in musb_plat and so it totally looks like a duplicated specification of the same thing from the board file and from generic musb code where the original init value is then ignored. I see that in .29 the board file did not need to specify usb mode and it was all decided in musb-specific code, and I have no idea why that was later changed to the way it is now. Would it just be better if a patch removing mode field from struct omap_musb_board_data is adopted like this (warning - not a real patch)? diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c index 35559f7..c7f834f 100644 --- a/arch/arm/mach-omap2/usb-musb.c +++ b/arch/arm/mach-omap2/usb-musb.c @@ -129,7 +129,6 @@ void __init usb_musb_init(struct omap_musb_board_data *board musb_plat.clock = ick; musb_plat.board_data = board_data; musb_plat.power = board_data-power 1; - musb_plat.mode = board_data-mode; musb_plat.extvbus = board_data-extvbus; if (cpu_is_omap44xx()) Bye, Oleg-- 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 4/4] omap: musb: introduce default baord config
On Thu, Apr 28, 2011 at 12:21:47PM -0400, Oleg Drokin wrote: Frankly, I am not even sure why the mode needs to be supplied by the board at all. The musb code already has a very similar switch in musb_plat and so it totally looks like a duplicated specification of the same thing from the board file and from generic musb code where the original init value is then ignored. I see that in .29 the board file did not need to specify usb mode and it was all decided in musb-specific code, and I have no idea why that was later changed to the way it is now. Would it just be better if a patch removing mode field from struct omap_musb_board_data is adopted like this (warning - not a real patch)? Hmm, it looks to me like we're ending up with two layers on top of each other, both trying to provide some kind of generic board interface. I think they should be squashed together. And that: static struct musb_hdrc_platform_data musb_plat = { #ifdef CONFIG_USB_MUSB_OTG .mode = MUSB_OTG, #elif defined(CONFIG_USB_MUSB_HDRC_HCD) .mode = MUSB_HOST, #elif defined(CONFIG_USB_GADGET_MUSB_HDRC) .mode = MUSB_PERIPHERAL, #endif in usb-musb.c needs the same treatment as I mentioned in the previous message if it really is board specific. If not, I see no reason why the above can't go into the musb driver itself. -- 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 4/4] omap: musb: introduce default baord config
Hello! On Apr 28, 2011, at 12:28 PM, Russell King - ARM Linux wrote: mm, it looks to me like we're ending up with two layers on top of each other, both trying to provide some kind of generic board interface. I think they should be squashed together. And that: static struct musb_hdrc_platform_data musb_plat = { #ifdef CONFIG_USB_MUSB_OTG .mode = MUSB_OTG, #elif defined(CONFIG_USB_MUSB_HDRC_HCD) .mode = MUSB_HOST, #elif defined(CONFIG_USB_GADGET_MUSB_HDRC) .mode = MUSB_PERIPHERAL, #endif in usb-musb.c needs the same treatment as I mentioned in the previous message if it really is board specific. If not, I see no reason why the above can't go into the musb driver itself. In the end it's chip dependent. And the musb can work in all three modes. Of course the board dictates if the power is supplied to the bus in host mode and such, but even that could be worked around as nokia 9x0 saga for USB host shows. So to me it looks like something totally in realm of musb driver itself. Nothing bad happens if you configure your MUSB as say OTG while in fact only peripheral mode was implemented, it continues to work as it did. Of course enabling HOST mode may not magically make things work, but I suspect this could be addressed from Kconfig itself instead. Now I totally expect musb maintainer to jump in and explain how many misconceptions I have ;) Bye, Oleg -- 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 4/4] omap: musb: introduce default baord config
On 04/27/11 07:23, Oleg Drokin wrote: Hello! On Apr 24, 2011, at 6:09 PM, Mike Rapoport wrote: -void __init usb_musb_init(struct omap_musb_board_data *board_data) +static struct omap_musb_board_data musb_default_board_data = { +.interface_type = MUSB_INTERFACE_ULPI, +.mode = MUSB_OTG, In fact can you make it more generic with ifdefs like this? (since there are tons of boards taht are client only or would like to be compiled in some other way): I didn't want to change current functionality. The purpose of this patch is to reduce amount of code shared among board files. If certain board needs mode other than OTG it can still pass musb_board_data. +#ifdef CONFIG_USB_MUSB_OTG + .mode = MUSB_OTG, +#elif defined(CONFIG_USB_MUSB_HDRC_HCD) + .mode = MUSB_HOST, +#elif defined(CONFIG_USB_GADGET_MUSB_HDRC) + .mode = MUSB_PERIPHERAL, +#endif +.power = 100, +}; + Thanks! Bye, Oleg -- Sincerely yours, Mike. -- 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 4/4] omap: musb: introduce default baord config
Hello! On Apr 24, 2011, at 6:09 PM, Mike Rapoport wrote: -void __init usb_musb_init(struct omap_musb_board_data *board_data) +static struct omap_musb_board_data musb_default_board_data = { + .interface_type = MUSB_INTERFACE_ULPI, + .mode = MUSB_OTG, In fact can you make it more generic with ifdefs like this? (since there are tons of boards taht are client only or would like to be compiled in some other way): +#ifdef CONFIG_USB_MUSB_OTG + .mode = MUSB_OTG, +#elif defined(CONFIG_USB_MUSB_HDRC_HCD) + .mode = MUSB_HOST, +#elif defined(CONFIG_USB_GADGET_MUSB_HDRC) + .mode = MUSB_PERIPHERAL, +#endif + .power = 100, +}; + Thanks! Bye, Oleg -- 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 4/4] omap: musb: introduce default baord config
Most boards use exactly the same configuration for musb initialization. Create a default that can be shared amount different boards. Signed-off-by: Mike Rapoport m...@compulab.co.il --- arch/arm/mach-omap2/board-2430sdp.c |7 +-- arch/arm/mach-omap2/board-3430sdp.c |8 +--- arch/arm/mach-omap2/board-cm-t35.c |8 +--- arch/arm/mach-omap2/board-devkit8000.c |8 +--- arch/arm/mach-omap2/board-igep0020.c |8 +--- arch/arm/mach-omap2/board-igep0030.c |8 +--- arch/arm/mach-omap2/board-ldp.c |8 +--- arch/arm/mach-omap2/board-omap3beagle.c |8 +--- arch/arm/mach-omap2/board-omap3pandora.c |8 +--- arch/arm/mach-omap2/board-omap3stalker.c |8 +--- arch/arm/mach-omap2/board-omap3touchbook.c |8 +--- arch/arm/mach-omap2/board-overo.c|8 +--- arch/arm/mach-omap2/board-rm680.c|8 +--- arch/arm/mach-omap2/board-zoom-peripherals.c |8 +--- arch/arm/mach-omap2/usb-musb.c | 14 +- 15 files changed, 27 insertions(+), 98 deletions(-) diff --git a/arch/arm/mach-omap2/board-2430sdp.c b/arch/arm/mach-omap2/board-2430sdp.c index 99b3f2d..a8810f8 100644 --- a/arch/arm/mach-omap2/board-2430sdp.c +++ b/arch/arm/mach-omap2/board-2430sdp.c @@ -208,11 +208,6 @@ static struct omap2_hsmmc_info mmc[] __initdata = { {} /* Terminator */ }; -static struct omap_musb_board_data musb_board_data = { - .interface_type = MUSB_INTERFACE_ULPI, - .mode = MUSB_OTG, - .power = 100, -}; static struct omap_usb_config sdp2430_usb_config __initdata = { .otg= 1, #ifdef CONFIG_USB_GADGET_OMAP @@ -246,7 +241,7 @@ static void __init omap_2430sdp_init(void) omap2_usbfs_init(sdp2430_usb_config); omap_mux_init_signal(usb0hs_stp, OMAP_PULL_ENA | OMAP_PULL_UP); - usb_musb_init(musb_board_data); + usb_musb_init(NULL); board_smc91x_init(); diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-omap2/board-3430sdp.c index b12400e..951e585 100644 --- a/arch/arm/mach-omap2/board-3430sdp.c +++ b/arch/arm/mach-omap2/board-3430sdp.c @@ -810,12 +810,6 @@ static struct flash_partitions sdp_flash_partitions[] = { }, }; -static struct omap_musb_board_data musb_board_data = { - .interface_type = MUSB_INTERFACE_ULPI, - .mode = MUSB_OTG, - .power = 100, -}; - static void __init omap_3430sdp_init(void) { int gpio_pendown; @@ -832,7 +826,7 @@ static void __init omap_3430sdp_init(void) gpio_pendown = SDP3430_TS_GPIO_IRQ_SDPV1; omap_ads7846_init(1, gpio_pendown, 310, NULL); board_serial_init(); - usb_musb_init(musb_board_data); + usb_musb_init(NULL); board_smc91x_init(); board_flash_init(sdp_flash_partitions, chip_sel_3430, 0); sdp3430_display_init(); diff --git a/arch/arm/mach-omap2/board-cm-t35.c b/arch/arm/mach-omap2/board-cm-t35.c index 1a9e6be..286da17 100644 --- a/arch/arm/mach-omap2/board-cm-t35.c +++ b/arch/arm/mach-omap2/board-cm-t35.c @@ -651,12 +651,6 @@ static struct omap_board_mux board_mux[] __initdata = { }; #endif -static struct omap_musb_board_data musb_board_data = { - .interface_type = MUSB_INTERFACE_ULPI, - .mode = MUSB_OTG, - .power = 100, -}; - static struct omap_board_config_kernel cm_t35_config[] __initdata = { }; @@ -673,7 +667,7 @@ static void __init cm_t35_init(void) cm_t35_init_led(); cm_t35_init_display(); - usb_musb_init(musb_board_data); + usb_musb_init(NULL); usbhs_init(usbhs_bdata); } diff --git a/arch/arm/mach-omap2/board-devkit8000.c b/arch/arm/mach-omap2/board-devkit8000.c index e7dc057..405542a 100644 --- a/arch/arm/mach-omap2/board-devkit8000.c +++ b/arch/arm/mach-omap2/board-devkit8000.c @@ -509,12 +509,6 @@ static struct platform_device *devkit8000_devices[] __initdata = { omap_dm9000_dev, }; -static struct omap_musb_board_data musb_board_data = { - .interface_type = MUSB_INTERFACE_ULPI, - .mode = MUSB_OTG, - .power = 100, -}; - static const struct usbhs_omap_board_data usbhs_bdata __initconst = { .port_mode[0] = OMAP_EHCI_PORT_MODE_PHY, @@ -698,7 +692,7 @@ static void __init devkit8000_init(void) omap_ads7846_init(2, OMAP3_DEVKIT_TS_GPIO, 0, NULL); - usb_musb_init(musb_board_data); + usb_musb_init(NULL); usbhs_init(usbhs_bdata); omap_nand_flash_init(NAND_BUSWIDTH_16, devkit8000_nand_partitions, ARRAY_SIZE(devkit8000_nand_partitions)); diff --git a/arch/arm/mach-omap2/board-igep0020.c b/arch/arm/mach-omap2/board-igep0020.c index