| Message ID | 1378204870-25884-1-git-send-email-m.zalega@samsung.com |
|---|---|
| State | Superseded |
| Delegated to: | Marek Vasut |
| Headers | show |
Dear Mateusz Zalega, > This commit unifies board-specific USB initialization implementations > under one symbol (usb_board_init), declaration of which is available in > usb.h. > > Signed-off-by: Mateusz Zalega <m.zalega@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > Reviewed-by: Lukasz Majewski <l.majewski@samsung.com> > Cc: Minkyu Kang <mk7.kang@samsung.com> > Cc: Marek Vasut <marex@denx.de> > --- > Changes since RFC (v1): > - NVIDIA Tegra doesn't postpone its USB init anymore > - board_usb_init()'s sole argument name was shortened > - networking code comment style (/* blurb...) dropped > - squashed RFC changes so that patch won't break bisect > > v2 changes: > - commit message fixup > > v3 changes: > - added 'index' argument to perform selective port initialization OK, a few general ideas again: Why not wrap board_usb_init() and board_usb_init_fail() into single call. You now pass some flags to board_usb_init() already, so just add another for the fail case. How does it sound to you? Moreover, the 'int index' should likely be unsigned int and the special value to init all controllers at once should probably then be 0xffffffff Best regards, Marek Vasut
On 09/05/13 17:50, Marek Vasut wrote: >> v3 changes: >> - added 'index' argument to perform selective port initialization > > OK, a few general ideas again: > > Why not wrap board_usb_init() and board_usb_init_fail() into single call. You > now pass some flags to board_usb_init() already, so just add another for the > fail case. How does it sound to you? Like overengineering. It would lead to "board_usb_init(USB_INIT_ALL, USB_INIT_DEVICE, USB_CLEANUP)" calls, which are not very readable. > Moreover, the 'int index' should likely be unsigned int and the special value to > init all controllers at once should probably then be 0xffffffff Despite our greatest ambitions, I don't think we're likely to use more than 2^31-1 USB controllers at a time. Besides, negative values look better both in code and debugger session. Best Regards,
Dear Mateusz Zalega, > On 09/05/13 17:50, Marek Vasut wrote: > >> v3 changes: > >> - added 'index' argument to perform selective port initialization > > > > OK, a few general ideas again: > > > > Why not wrap board_usb_init() and board_usb_init_fail() into single call. > > You now pass some flags to board_usb_init() already, so just add another > > for the fail case. How does it sound to you? > > Like overengineering. It would lead to "board_usb_init(USB_INIT_ALL, > USB_INIT_DEVICE, USB_CLEANUP)" calls, which are not very readable. This is not what I mean, see this: int board_usb_init(int index, enum board_usb_init_type init) Add a new "init" type (or maybe change the init field to be flags) that will say "OK, do a fail init" ? > > Moreover, the 'int index' should likely be unsigned int and the special > > value to init all controllers at once should probably then be 0xffffffff > > Despite our greatest ambitions, I don't think we're likely to use more > than 2^31-1 USB controllers at a time. Besides, negative values look > better both in code and debugger session. Thinking of it further, instead of using negative value here, like I mentioned above, why not make the "board_usb_init_type" into a field of flags , then add flag to init all controllers at once ? Best regards, Marek Vasut
On 09/05/13 19:51, Marek Vasut wrote: >>> Why not wrap board_usb_init() and board_usb_init_fail() into single call. >>> You now pass some flags to board_usb_init() already, so just add another >>> for the fail case. How does it sound to you? >> >> Like overengineering. It would lead to "board_usb_init(USB_INIT_ALL, >> USB_INIT_DEVICE, USB_CLEANUP)" calls, which are not very readable. > > This is not what I mean, see this: > > int board_usb_init(int index, enum board_usb_init_type init) > > Add a new "init" type (or maybe change the init field to be flags) that will say > "OK, do a fail init" ? As for providing a way to do a selective cleanup if anything gone wrong, it's a good idea, but adding such functionality to board_usb_init() would be confusing, especially for newcomers. Why not do this in board_usb_init_fail(int index, enum board_usb_init_type)? ...and maybe rename board_usb_init_fail to board_usb_cleanup. >>> Moreover, the 'int index' should likely be unsigned int and the special >>> value to init all controllers at once should probably then be 0xffffffff >> >> Despite our greatest ambitions, I don't think we're likely to use more >> than 2^31-1 USB controllers at a time. Besides, negative values look >> better both in code and debugger session. > > Thinking of it further, instead of using negative value here, like I mentioned > above, why not make the "board_usb_init_type" into a field of flags , then add > flag to init all controllers at once ? That's unnecessary. It wouldn't lead to any practical advantage over existing interface. Best Regards,
Dear Mateusz Zalega, > On 09/05/13 19:51, Marek Vasut wrote: > >>> Why not wrap board_usb_init() and board_usb_init_fail() into single > >>> call. You now pass some flags to board_usb_init() already, so just add > >>> another for the fail case. How does it sound to you? > >> > >> Like overengineering. It would lead to "board_usb_init(USB_INIT_ALL, > >> USB_INIT_DEVICE, USB_CLEANUP)" calls, which are not very readable. > > > > This is not what I mean, see this: > > > > int board_usb_init(int index, enum board_usb_init_type init) > > > > Add a new "init" type (or maybe change the init field to be flags) that > > will say "OK, do a fail init" ? > > As for providing a way to do a selective cleanup if anything gone wrong, > it's a good idea, but adding such functionality to board_usb_init() > would be confusing, especially for newcomers. Why not do this in > board_usb_init_fail(int index, enum board_usb_init_type)? > > ...and maybe rename board_usb_init_fail to board_usb_cleanup. Cleanup is nice name, yeah. > >>> Moreover, the 'int index' should likely be unsigned int and the special > >>> value to init all controllers at once should probably then be > >>> 0xffffffff > >> > >> Despite our greatest ambitions, I don't think we're likely to use more > >> than 2^31-1 USB controllers at a time. Besides, negative values look > >> better both in code and debugger session. > > > > Thinking of it further, instead of using negative value here, like I > > mentioned above, why not make the "board_usb_init_type" into a field of > > flags , then add flag to init all controllers at once ? > > That's unnecessary. It wouldn't lead to any practical advantage over > existing interface. The advantage would be you won't be mixing two things (value AND value with special meaning) into the "index" parameter. Best regards, Marek Vasut
On 09/06/13 13:24, Marek Vasut wrote: >>>>> Moreover, the 'int index' should likely be unsigned int and the special >>>>> value to init all controllers at once should probably then be >>>>> 0xffffffff >>>> >>>> Despite our greatest ambitions, I don't think we're likely to use more >>>> than 2^31-1 USB controllers at a time. Besides, negative values look >>>> better both in code and debugger session. >>> >>> Thinking of it further, instead of using negative value here, like I >>> mentioned above, why not make the "board_usb_init_type" into a field of >>> flags , then add flag to init all controllers at once ? >> >> That's unnecessary. It wouldn't lead to any practical advantage over >> existing interface. > > The advantage would be you won't be mixing two things (value AND value with > special meaning) into the "index" parameter. Alright, provide a use-case. The only 'special' value we have now doesn't interfere with controller index. Why write code or interfaces that won't ever be used? Best regards,
Dear Mateusz Zalega, > On 09/06/13 13:24, Marek Vasut wrote: > >>>>> Moreover, the 'int index' should likely be unsigned int and the > >>>>> special value to init all controllers at once should probably then > >>>>> be 0xffffffff > >>>> > >>>> Despite our greatest ambitions, I don't think we're likely to use more > >>>> than 2^31-1 USB controllers at a time. Besides, negative values look > >>>> better both in code and debugger session. > >>> > >>> Thinking of it further, instead of using negative value here, like I > >>> mentioned above, why not make the "board_usb_init_type" into a field of > >>> flags , then add flag to init all controllers at once ? > >> > >> That's unnecessary. It wouldn't lead to any practical advantage over > >> existing interface. > > > > The advantage would be you won't be mixing two things (value AND value > > with special meaning) into the "index" parameter. > > Alright, provide a use-case. The only 'special' value we have now > doesn't interfere with controller index. Why write code or interfaces > that won't ever be used? Look, abusing the index field with a special value is moronic, especially if you _do_ have another field that can very well be turned into a block of flags instead of enum right next to it. function(int foo , enum bar) | ^ | | `---------' flags go here Then int foo can be turned into unsigned int foo _and_ be used for it's one singular purpose. Likewise, enum bar will now be "unsigned int flags" . Do you see the separation now ? Best regards, Marek Vasut
On 09/06/13 13:40, Marek Vasut wrote: >>>>>>> Moreover, the 'int index' should likely be unsigned int and the >>>>>>> special value to init all controllers at once should probably then >>>>>>> be 0xffffffff >>>>>> >>>>>> Despite our greatest ambitions, I don't think we're likely to use more >>>>>> than 2^31-1 USB controllers at a time. Besides, negative values look >>>>>> better both in code and debugger session. >>>>> >>>>> Thinking of it further, instead of using negative value here, like I >>>>> mentioned above, why not make the "board_usb_init_type" into a field of >>>>> flags , then add flag to init all controllers at once ? >>>> >>>> That's unnecessary. It wouldn't lead to any practical advantage over >>>> existing interface. >>> >>> The advantage would be you won't be mixing two things (value AND value >>> with special meaning) into the "index" parameter. >> >> Alright, provide a use-case. The only 'special' value we have now >> doesn't interfere with controller index. Why write code or interfaces >> that won't ever be used? > > Look, abusing the index field with a special value is moronic, especially if you I wouldn't call a de-facto standard abusive or moronic. On the other hand, contributing to unnecessary code bloat would be. > _do_ have another field that can very well be turned into a block of flags Have you provided another use-case for that, as I asked? > instead of enum right next to it. > > function(int foo , enum bar) > | ^ > | | > `---------' > flags go > here > > Then int foo can be turned into unsigned int foo _and_ be used for it's one > singular purpose. Likewise, enum bar will now be "unsigned int flags" . Do you > see the separation now ? Read your mail carefully - at this point I don't have any problems understanding what you wish to do, but I see your API changes as unjustified. Can you justify them without appealing to your authority and maybe inserting a couple of ad-hominems here and there?
Dear Mateusz Zalega, > On 09/06/13 13:40, Marek Vasut wrote: > >>>>>>> Moreover, the 'int index' should likely be unsigned int and the > >>>>>>> special value to init all controllers at once should probably then > >>>>>>> be 0xffffffff > >>>>>> > >>>>>> Despite our greatest ambitions, I don't think we're likely to use > >>>>>> more than 2^31-1 USB controllers at a time. Besides, negative > >>>>>> values look better both in code and debugger session. > >>>>> > >>>>> Thinking of it further, instead of using negative value here, like I > >>>>> mentioned above, why not make the "board_usb_init_type" into a field > >>>>> of flags , then add flag to init all controllers at once ? > >>>> > >>>> That's unnecessary. It wouldn't lead to any practical advantage over > >>>> existing interface. > >>> > >>> The advantage would be you won't be mixing two things (value AND value > >>> with special meaning) into the "index" parameter. > >> > >> Alright, provide a use-case. The only 'special' value we have now > >> doesn't interfere with controller index. Why write code or interfaces > >> that won't ever be used? > > > > Look, abusing the index field with a special value is moronic, especially > > if you > > I wouldn't call a de-facto standard abusive or moronic. > On the other hand, contributing to unnecessary code bloat would be. > > > _do_ have another field that can very well be turned into a block of > > flags > > Have you provided another use-case for that, as I asked? > > > instead of enum right next to it. > > > > function(int foo , enum bar) > > > > | ^ > > > > `---------' > > > > flags go > > > > here > > > > Then int foo can be turned into unsigned int foo _and_ be used for it's > > one singular purpose. Likewise, enum bar will now be "unsigned int > > flags" . Do you see the separation now ? > > Read your mail carefully - at this point I don't have any problems > understanding what you wish to do, but I see your API changes as > unjustified. Can you justify them without appealing to your authority > and maybe inserting a couple of ad-hominems here and there? I suppose this thread can be concluded by droping the INIT_ALL stuff entirely. Afterall, we do not want to init _ALL_ ports at once, but we want to init them selectively. Best regards, Marek Vasut
On 09/15/13 16:21, Marek Vasut wrote: > I suppose this thread can be concluded by droping the INIT_ALL stuff entirely. > Afterall, we do not want to init _ALL_ ports at once, but we want to init them > selectively. > > Best regards, > Marek Vasut +1
diff --git a/arch/arm/include/asm/arch-tegra/usb.h b/arch/arm/include/asm/arch-tegra/usb.h index f66257c..a1efd07 100644 --- a/arch/arm/include/asm/arch-tegra/usb.h +++ b/arch/arm/include/asm/arch-tegra/usb.h @@ -131,8 +131,7 @@ /* USB3_IF_USB_PHY_VBUS_SENSORS_0 */ #define VBUS_VLD_STS (1 << 26) - /* Setup USB on the board */ -int board_usb_init(const void *blob); +int usb_process_devicetree(const void *blob); #endif /* _TEGRA_USB_H_ */ diff --git a/arch/arm/include/asm/ehci-omap.h b/arch/arm/include/asm/ehci-omap.h index ac83a53..c7bca05 100644 --- a/arch/arm/include/asm/ehci-omap.h +++ b/arch/arm/include/asm/ehci-omap.h @@ -145,8 +145,8 @@ struct omap_ehci { struct ehci_hccr; struct ehci_hcor; -int omap_ehci_hcd_init(struct omap_usbhs_board_data *usbhs_pdata, - struct ehci_hccr **hccr, struct ehci_hcor **hcor); +int omap_ehci_hcd_init(int index, struct omap_usbhs_board_data *usbhs_pdata, + struct ehci_hccr **hccr, struct ehci_hcor **hcor); int omap_ehci_hcd_stop(void); #endif /* _OMAP_COMMON_EHCI_H_ */ diff --git a/board/amcc/canyonlands/canyonlands.c b/board/amcc/canyonlands/canyonlands.c index cc36f45..8e85bee 100644 --- a/board/amcc/canyonlands/canyonlands.c +++ b/board/amcc/canyonlands/canyonlands.c @@ -16,6 +16,7 @@ #include <asm/4xx_pcie.h> #include <asm/ppc4xx-gpio.h> #include <asm/errno.h> +#include <usb.h> extern flash_info_t flash_info[CONFIG_SYS_MAX_FLASH_BANKS]; /* info for FLASH chips */ @@ -188,7 +189,7 @@ int board_early_init_f(void) } #if defined(CONFIG_USB_OHCI_NEW) && defined(CONFIG_SYS_USB_OHCI_BOARD_INIT) -int usb_board_init(void) +int board_usb_init(int index, enum board_usb_init_type init) { struct board_bcsr *bcsr_data = (struct board_bcsr *)CONFIG_SYS_BCSR_BASE; @@ -229,7 +230,7 @@ int usb_board_stop(void) return 0; } -int usb_board_init_fail(void) +int board_usb_init_fail(void) { return usb_board_stop(); } diff --git a/board/balloon3/balloon3.c b/board/balloon3/balloon3.c index ecbac16..891e48a 100644 --- a/board/balloon3/balloon3.c +++ b/board/balloon3/balloon3.c @@ -13,6 +13,7 @@ #include <asm/io.h> #include <spartan3.h> #include <command.h> +#include <usb.h> DECLARE_GLOBAL_DATA_PTR; @@ -59,7 +60,7 @@ void dram_init_banksize(void) } #ifdef CONFIG_CMD_USB -int usb_board_init(void) +int board_usb_init(int index, enum board_usb_init_type init) { writel((readl(UHCHR) | UHCHR_PCPL | UHCHR_PSPL) & ~(UHCHR_SSEP0 | UHCHR_SSEP1 | UHCHR_SSEP2 | UHCHR_SSE), @@ -90,7 +91,7 @@ int usb_board_init(void) return 0; } -void usb_board_init_fail(void) +void board_usb_init_fail(void) { return; } diff --git a/board/compulab/cm_t35/cm_t35.c b/board/compulab/cm_t35/cm_t35.c index 3caa5be..7626abc 100644 --- a/board/compulab/cm_t35/cm_t35.c +++ b/board/compulab/cm_t35/cm_t35.c @@ -591,7 +591,7 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor) twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, offset, 0xC0); udelay(1); - return omap_ehci_hcd_init(&usbhs_bdata, hccr, hcor); + return omap_ehci_hcd_init(index, &usbhs_bdata, hccr, hcor); } int ehci_hcd_stop(void) diff --git a/board/esd/apc405/apc405.c b/board/esd/apc405/apc405.c index f13f088..7592b61 100644 --- a/board/esd/apc405/apc405.c +++ b/board/esd/apc405/apc405.c @@ -17,6 +17,7 @@ #include <mtd/cfi_flash.h> #include <asm/4xx_pci.h> #include <pci.h> +#include <usb.h> DECLARE_GLOBAL_DATA_PTR; @@ -428,7 +429,7 @@ void reset_phy(void) } #if defined(CONFIG_USB_OHCI_NEW) && defined(CONFIG_SYS_USB_OHCI_BOARD_INIT) -int usb_board_init(void) +int board_usb_init(int index, enum board_usb_init_type init) { return 0; } @@ -453,7 +454,7 @@ int usb_board_stop(void) return 0; } -int usb_board_init_fail(void) +int board_usb_init_fail(void) { usb_board_stop(); return 0; diff --git a/board/esd/pmc440/pmc440.c b/board/esd/pmc440/pmc440.c index 549b3b7..b81da1e 100644 --- a/board/esd/pmc440/pmc440.c +++ b/board/esd/pmc440/pmc440.c @@ -27,6 +27,7 @@ #endif #include <serial.h> #include <asm/4xx_pci.h> +#include <usb.h> #include "fpga.h" #include "pmc440.h" @@ -821,7 +822,7 @@ int bootstrap_eeprom_read (unsigned dev_addr, unsigned offset, } #if defined(CONFIG_USB_OHCI_NEW) && defined(CONFIG_SYS_USB_OHCI_BOARD_INIT) -int usb_board_init(void) +int board_usb_init(int index, enum board_usb_init_type init) { char *act = getenv("usbact"); int i; @@ -845,7 +846,7 @@ int usb_board_stop(void) return 0; } -int usb_board_init_fail(void) +int board_usb_init_fail(void) { usb_board_stop(); return 0; diff --git a/board/htkw/mcx/mcx.c b/board/htkw/mcx/mcx.c index 653d7ea..6f85b47 100644 --- a/board/htkw/mcx/mcx.c +++ b/board/htkw/mcx/mcx.c @@ -42,7 +42,7 @@ static struct omap_usbhs_board_data usbhs_bdata = { int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor) { - return omap_ehci_hcd_init(&usbhs_bdata, hccr, hcor); + return omap_ehci_hcd_init(index, &usbhs_bdata, hccr, hcor); } int ehci_hcd_stop(int index) diff --git a/board/icpdas/lp8x4x/lp8x4x.c b/board/icpdas/lp8x4x/lp8x4x.c index 1b68ef3..73e0266 100644 --- a/board/icpdas/lp8x4x/lp8x4x.c +++ b/board/icpdas/lp8x4x/lp8x4x.c @@ -15,6 +15,7 @@ #include <netdev.h> #include <serial.h> #include <asm/io.h> +#include <usb.h> DECLARE_GLOBAL_DATA_PTR; @@ -58,7 +59,7 @@ int board_mmc_init(bd_t *bis) #endif #ifdef CONFIG_CMD_USB -int usb_board_init(void) +int board_usb_init(int index, enum board_usb_init_type init) { writel((UHCHR | UHCHR_PCPL | UHCHR_PSPL) & ~(UHCHR_SSEP0 | UHCHR_SSEP1 | UHCHR_SSEP2 | UHCHR_SSE), @@ -89,7 +90,7 @@ int usb_board_init(void) return 0; } -void usb_board_init_fail(void) +void board_usb_init_fail(void) { return; } diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c index 126e56e..1972527 100644 --- a/board/nvidia/common/board.c +++ b/board/nvidia/common/board.c @@ -32,6 +32,7 @@ #ifdef CONFIG_USB_EHCI_TEGRA #include <asm/arch-tegra/usb.h> #include <asm/arch/usb.h> +#include <usb.h> #endif #ifdef CONFIG_TEGRA_MMC #include <asm/arch-tegra/tegra_mmc.h> @@ -153,8 +154,9 @@ int board_init(void) #ifdef CONFIG_USB_EHCI_TEGRA pin_mux_usb(); - board_usb_init(gd->fdt_blob); + usb_process_devicetree(gd->fdt_blob); #endif + #ifdef CONFIG_LCD tegra_lcd_check_next_stage(gd->fdt_blob, 0); #endif diff --git a/board/samsung/trats/trats.c b/board/samsung/trats/trats.c index 7f61d17..58d925f 100644 --- a/board/samsung/trats/trats.c +++ b/board/samsung/trats/trats.c @@ -26,6 +26,7 @@ #include <power/max8997_muic.h> #include <power/battery.h> #include <power/max17042_fg.h> +#include <usb.h> #include <usb_mass_storage.h> #include "setup.h" @@ -495,10 +496,10 @@ struct s3c_plat_otg_data s5pc210_otg_data = { .usb_flags = PHY0_SLEEP, }; -void board_usb_init(void) +int board_usb_init(int index, enum board_usb_init_type init) { debug("USB_udc_probe\n"); - s3c_udc_probe(&s5pc210_otg_data); + return s3c_udc_probe(&s5pc210_otg_data); } #endif diff --git a/board/technexion/twister/twister.c b/board/technexion/twister/twister.c index cd91d8f..6f2ff55 100644 --- a/board/technexion/twister/twister.c +++ b/board/technexion/twister/twister.c @@ -53,7 +53,7 @@ static struct omap_usbhs_board_data usbhs_bdata = { int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor) { - return omap_ehci_hcd_init(&usbhs_bdata, hccr, hcor); + return omap_ehci_hcd_init(index, &usbhs_bdata, hccr, hcor); } int ehci_hcd_stop(int index) diff --git a/board/teejet/mt_ventoux/mt_ventoux.c b/board/teejet/mt_ventoux/mt_ventoux.c index b4e01d1..df873f5 100644 --- a/board/teejet/mt_ventoux/mt_ventoux.c +++ b/board/teejet/mt_ventoux/mt_ventoux.c @@ -104,7 +104,7 @@ static struct omap_usbhs_board_data usbhs_bdata = { int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor) { - return omap_ehci_hcd_init(&usbhs_bdata, hccr, hcor); + return omap_ehci_hcd_init(index, &usbhs_bdata, hccr, hcor); } int ehci_hcd_stop(int index) diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c index 62e9bea..41fed54 100644 --- a/board/ti/beagle/beagle.c +++ b/board/ti/beagle/beagle.c @@ -523,7 +523,7 @@ static struct omap_usbhs_board_data usbhs_bdata = { int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor) { - return omap_ehci_hcd_init(&usbhs_bdata, hccr, hcor); + return omap_ehci_hcd_init(index, &usbhs_bdata, hccr, hcor); } int ehci_hcd_stop(int index) diff --git a/board/ti/panda/panda.c b/board/ti/panda/panda.c index e838ffd..fe7a437 100644 --- a/board/ti/panda/panda.c +++ b/board/ti/panda/panda.c @@ -263,7 +263,7 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor) utmi_clk |= HSUSBHOST_CLKCTRL_CLKSEL_UTMI_P1_MASK; sr32((void *)CM_L3INIT_HSUSBHOST_CLKCTRL, 0, 32, utmi_clk); - ret = omap_ehci_hcd_init(&usbhs_bdata, hccr, hcor); + ret = omap_ehci_hcd_init(index, &usbhs_bdata, hccr, hcor); if (ret < 0) return ret; diff --git a/board/toradex/colibri_pxa270/colibri_pxa270.c b/board/toradex/colibri_pxa270/colibri_pxa270.c index c1e2562..2e0ee0d 100644 --- a/board/toradex/colibri_pxa270/colibri_pxa270.c +++ b/board/toradex/colibri_pxa270/colibri_pxa270.c @@ -13,6 +13,7 @@ #include <netdev.h> #include <asm/io.h> #include <serial.h> +#include <usb.h> DECLARE_GLOBAL_DATA_PTR; @@ -39,7 +40,7 @@ int dram_init(void) } #ifdef CONFIG_CMD_USB -int usb_board_init(void) +int board_usb_init(int index, enum board_usb_init_type init) { writel((readl(UHCHR) | UHCHR_PCPL | UHCHR_PSPL) & ~(UHCHR_SSEP0 | UHCHR_SSEP1 | UHCHR_SSEP2 | UHCHR_SSE), @@ -70,7 +71,7 @@ int usb_board_init(void) return 0; } -void usb_board_init_fail(void) +void board_usb_init_fail(void) { return; } diff --git a/board/trizepsiv/conxs.c b/board/trizepsiv/conxs.c index c0c318f..2063288 100644 --- a/board/trizepsiv/conxs.c +++ b/board/trizepsiv/conxs.c @@ -21,6 +21,7 @@ #include <asm/arch/regs-mmc.h> #include <netdev.h> #include <asm/io.h> +#include <usb.h> DECLARE_GLOBAL_DATA_PTR; @@ -42,7 +43,7 @@ extern struct serial_device serial_stuart_device; * Miscelaneous platform dependent initialisations */ -int usb_board_init(void) +int board_usb_init(int index, enum board_usb_init_type init) { writel((readl(UHCHR) | UHCHR_PCPL | UHCHR_PSPL) & ~(UHCHR_SSEP0 | UHCHR_SSEP1 | UHCHR_SSEP2 | UHCHR_SSE), @@ -69,7 +70,7 @@ int usb_board_init(void) return 0; } -void usb_board_init_fail(void) +void board_usb_init_fail(void) { return; } diff --git a/board/vpac270/vpac270.c b/board/vpac270/vpac270.c index 616736f..39d3b4b 100644 --- a/board/vpac270/vpac270.c +++ b/board/vpac270/vpac270.c @@ -13,6 +13,7 @@ #include <netdev.h> #include <serial.h> #include <asm/io.h> +#include <usb.h> DECLARE_GLOBAL_DATA_PTR; @@ -66,7 +67,7 @@ int board_mmc_init(bd_t *bis) #endif #ifdef CONFIG_CMD_USB -int usb_board_init(void) +int board_usb_init(int index, enum board_usb_init_type init) { writel((UHCHR | UHCHR_PCPL | UHCHR_PSPL) & ~(UHCHR_SSEP0 | UHCHR_SSEP1 | UHCHR_SSEP2 | UHCHR_SSE), @@ -97,7 +98,7 @@ int usb_board_init(void) return 0; } -void usb_board_init_fail(void) +void board_usb_init_fail(void) { return; } diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c index 793c422..933d0d6 100644 --- a/common/cmd_dfu.c +++ b/common/cmd_dfu.c @@ -14,6 +14,7 @@ #include <dfu.h> #include <asm/errno.h> #include <g_dnl.h> +#include <usb.h> static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { @@ -43,9 +44,7 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) goto done; } -#ifdef CONFIG_TRATS - board_usb_init(); -#endif + board_usb_init(USB_INIT_ALL, USB_INIT_DEVICE); g_dnl_register(s); while (1) { diff --git a/common/cmd_usb_mass_storage.c b/common/cmd_usb_mass_storage.c index 33a4715..42efd5c 100644 --- a/common/cmd_usb_mass_storage.c +++ b/common/cmd_usb_mass_storage.c @@ -9,6 +9,7 @@ #include <common.h> #include <command.h> #include <g_dnl.h> +#include <usb.h> #include <usb_mass_storage.h> int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag, @@ -33,7 +34,7 @@ int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag, goto fail; } - board_usb_init(); + board_usb_init(USB_INIT_ALL, USB_INIT_DEVICE); ums_info = board_ums_init(dev_num, offset, part_size); if (!ums_info) { diff --git a/common/usb.c b/common/usb.c index c97f522..bdcdd63 100644 --- a/common/usb.c +++ b/common/usb.c @@ -1037,4 +1037,9 @@ int usb_new_device(struct usb_device *dev) return 0; } +__attribute__((weak)) +int board_usb_init(int index, enum board_usb_init_type init) +{ + return 0; +} /* EOF */ diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c index 3c58f9e..c4ce487 100644 --- a/drivers/usb/host/ehci-omap.c +++ b/drivers/usb/host/ehci-omap.c @@ -96,12 +96,6 @@ static void omap_ehci_soft_phy_reset(int port) } #endif -inline int __board_usb_init(void) -{ - return 0; -} -int board_usb_init(void) __attribute__((weak, alias("__board_usb_init"))); - #if defined(CONFIG_OMAP_EHCI_PHY1_RESET_GPIO) || \ defined(CONFIG_OMAP_EHCI_PHY2_RESET_GPIO) || \ defined(CONFIG_OMAP_EHCI_PHY3_RESET_GPIO) @@ -157,15 +151,15 @@ int omap_ehci_hcd_stop(void) * Based on "drivers/usb/host/ehci-omap.c" from Linux 3.1 * See there for additional Copyrights. */ -int omap_ehci_hcd_init(struct omap_usbhs_board_data *usbhs_pdata, - struct ehci_hccr **hccr, struct ehci_hcor **hcor) +int omap_ehci_hcd_init(int index, struct omap_usbhs_board_data *usbhs_pdata, + struct ehci_hccr **hccr, struct ehci_hcor **hcor) { int ret; unsigned int i, reg = 0, rev = 0; debug("Initializing OMAP EHCI\n"); - ret = board_usb_init(); + ret = board_usb_init(index, USB_INIT_HOST); if (ret < 0) return ret; diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c index c6da449..cc23133 100644 --- a/drivers/usb/host/ehci-tegra.c +++ b/drivers/usb/host/ehci-tegra.c @@ -699,7 +699,7 @@ static int process_usb_nodes(const void *blob, int node_list[], int count) return 0; } -int board_usb_init(const void *blob) +int usb_process_devicetree(const void *blob) { int node_list[USB_PORTS_MAX]; int count, err = 0; diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c index c33c487..004f9dc 100644 --- a/drivers/usb/host/ohci-hcd.c +++ b/drivers/usb/host/ohci-hcd.c @@ -1861,7 +1861,7 @@ int usb_lowlevel_init(int index, void **controller) #ifdef CONFIG_SYS_USB_OHCI_BOARD_INIT /* board dependant init */ - if (usb_board_init()) + if (board_usb_init(index, USB_INIT_HOST)) return -1; #endif memset(&gohci, 0, sizeof(ohci_t)); @@ -1918,7 +1918,7 @@ int usb_lowlevel_init(int index, void **controller) err ("can't reset usb-%s", gohci.slot_name); #ifdef CONFIG_SYS_USB_OHCI_BOARD_INIT /* board dependant cleanup */ - usb_board_init_fail(); + board_usb_init_fail(); #endif #ifdef CONFIG_SYS_USB_OHCI_CPU_INIT diff --git a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h index d977e8f..9f7f961 100644 --- a/drivers/usb/host/ohci.h +++ b/drivers/usb/host/ohci.h @@ -19,14 +19,12 @@ #endif /* CONFIG_SYS_OHCI_SWAP_REG_ACCESS */ /* functions for doing board or CPU specific setup/cleanup */ -extern int usb_board_init(void); -extern int usb_board_stop(void); -extern int usb_board_init_fail(void); - -extern int usb_cpu_init(void); -extern int usb_cpu_stop(void); -extern int usb_cpu_init_fail(void); +int usb_board_stop(void); +int board_usb_init_fail(void); +int usb_cpu_init(void); +int usb_cpu_stop(void); +int usb_cpu_init_fail(void); static int cc_to_error[16] = { diff --git a/include/g_dnl.h b/include/g_dnl.h index 2b2f11a..b6c4dd4 100644 --- a/include/g_dnl.h +++ b/include/g_dnl.h @@ -14,6 +14,4 @@ int g_dnl_bind_fixup(struct usb_device_descriptor *); int g_dnl_register(const char *s); void g_dnl_unregister(void); -/* USB initialization declaration - board specific */ -void board_usb_init(void); #endif /* __G_DOWNLOAD_H_ */ diff --git a/include/usb.h b/include/usb.h index 60db897..5be90ff 100644 --- a/include/usb.h +++ b/include/usb.h @@ -165,10 +165,36 @@ int submit_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, extern void udc_disconnect(void); -#else +#elif !defined(CONFIG_USB_GADGET) #error USB Lowlevel not defined #endif +/* + * You can initialize platform's USB host or device + * ports by passing this enum as an argument to + * board_usb_init(). + */ +enum board_usb_init_type { + USB_INIT_HOST, + USB_INIT_DEVICE +}; + +/* + * Pass USB_INIT_ALL as board_usb_init's 'index' argument + * to initialize every available USB port of type specified + * by board_usb_init_type argument. + */ +#define USB_INIT_ALL -1 + +/* + * board-specific hardware initialization, called by + * usb drivers and u-boot commands + * + * @param index USB port number + * @param init initializes port as USB host or device + */ +int board_usb_init(int index, enum board_usb_init_type init); + #ifdef CONFIG_USB_STORAGE #define USB_MAX_STOR_DEV 5 diff --git a/include/usb_mass_storage.h b/include/usb_mass_storage.h index 35cdcc3..ed27aeb 100644 --- a/include/usb_mass_storage.h +++ b/include/usb_mass_storage.h @@ -30,13 +30,11 @@ struct ums_board_info { struct ums_device ums_dev; }; -extern void board_usb_init(void); - -extern int fsg_init(struct ums_board_info *); -extern void fsg_cleanup(void); -extern struct ums_board_info *board_ums_init(unsigned int, - unsigned int, unsigned int); -extern int usb_gadget_handle_interrupts(void); -extern int fsg_main_thread(void *); +int fsg_init(struct ums_board_info *); +void fsg_cleanup(void); +struct ums_board_info *board_ums_init(unsigned int, unsigned int, + unsigned int); +int usb_gadget_handle_interrupts(void); +int fsg_main_thread(void *); #endif /* __USB_MASS_STORAGE_H__ */