[U-Boot] [RFC 01/10] New board-specific USB initialization interface

[U-Boot] [RFC 01/10] New board-specific USB initialization interface

[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: Marek Vasut <marex@denx.de>
---
 common/cmd_dfu.c              |  5 ++---
 common/cmd_usb_mass_storage.c |  3 ++-
 common/usb.c                  |  5 +++++
 drivers/usb/host/ehci-omap.c  |  8 +-------
 drivers/usb/host/ehci-tegra.c |  2 +-
 drivers/usb/host/ohci-hcd.c   |  4 ++--
 drivers/usb/host/ohci.h       | 12 +++++-------
 include/g_dnl.h               |  2 --
 include/usb.h                 | 17 ++++++++++++++++-
 include/usb_mass_storage.h    | 12 +++++-------
 10 files changed, 39 insertions(+), 31 deletions(-)

diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c
index db066ac..c701ebc 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_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..86135b4 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_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 f740e5e..793fb35 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -982,4 +982,9 @@ int usb_new_device(struct usb_device *dev)
 	return 0;
 }
 
+__attribute__((weak))
+int board_usb_init(enum board_usb_init_type what_to_init)
+{
+	return 0;
+}
 /* EOF */
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index a47e078..61b7c8d 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -86,12 +86,6 @@ static void omap_ehci_soft_phy_reset(int port)
 	ulpi_reset(&ulpi_vp);
 }
 
-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)
 /* controls PHY(s) reset signal(s) */
@@ -150,7 +144,7 @@ int omap_ehci_hcd_init(struct omap_usbhs_board_data *usbhs_pdata,
 
 	debug("Initializing OMAP EHCI\n");
 
-	ret = board_usb_init();
+	ret = board_usb_init(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..e6a5623 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(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..29f67bf 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -165,10 +165,25 @@ 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, device or both
+ * capabilities by passing this enum as an argument to
+ * board_usb_init().
+ */
+enum board_usb_init_type {
+	USB_INIT_ALL,
+	USB_INIT_HOST,
+	USB_INIT_DEVICE
+};
+
+/* board-specific hardware initialization, called by
+ * usb drivers and u-boot commands
+ */
+int board_usb_init(enum board_usb_init_type what_to_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..de31b0e 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,
+int fsg_init(struct ums_board_info *);
+void fsg_cleanup(void);
+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 usb_gadget_handle_interrupts(void);
+int fsg_main_thread(void *);
 
 #endif /* __USB_MASS_STORAGE_H__ */
-- 
1.8.2.1

[U-Boot] [RFC 01/10] New board-specific USB initialization interface

[Stephen Warren]

On 08/06/2013 04:50 AM, Mateusz Zalega wrote:
> This commit unifies board-specific USB initialization implementations
> under one symbol (usb_board_init), declaration of which is available in
> usb.h.

> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c

> -int board_usb_init(const void *blob)
> +int usb_process_devicetree(const void *blob)
>  {
>  	int node_list[USB_PORTS_MAX];
>  	int count, err = 0;

With just this patch applied, nothing calls that function. This breaks
"git bisect".

[U-Boot] [RFC 01/10] New board-specific USB initialization interface

[Mateusz Zalega]

On 08/07/13 18:23, Stephen Warren wrote:
>> This commit unifies board-specific USB initialization implementations
>> under one symbol (usb_board_init), declaration of which is available in
>> usb.h.
> 
>> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
> 
>> -int board_usb_init(const void *blob)
>> +int usb_process_devicetree(const void *blob)
>>  {
>>  	int node_list[USB_PORTS_MAX];
>>  	int count, err = 0;
> 
> With just this patch applied, nothing calls that function. This breaks
> "git bisect".
> 

Yup, I suppose I should squash this RFC PATCH. It's not so large anyway.

Thanks,

-- 
Mateusz Zalega
Samsung R&D Institute Poland (SRPOL) | Kernel and System Framework group

[U-Boot] [RFC 01/10] New board-specific USB initialization interface

[Marek Vasut]

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: Marek Vasut <marex@denx.de>
> ---
>  common/cmd_dfu.c              |  5 ++---
>  common/cmd_usb_mass_storage.c |  3 ++-
>  common/usb.c                  |  5 +++++
>  drivers/usb/host/ehci-omap.c  |  8 +-------
>  drivers/usb/host/ehci-tegra.c |  2 +-
>  drivers/usb/host/ohci-hcd.c   |  4 ++--
>  drivers/usb/host/ohci.h       | 12 +++++-------
>  include/g_dnl.h               |  2 --
>  include/usb.h                 | 17 ++++++++++++++++-
>  include/usb_mass_storage.h    | 12 +++++-------
>  10 files changed, 39 insertions(+), 31 deletions(-)

[...]

> --- a/include/usb.h
> +++ b/include/usb.h
> @@ -165,10 +165,25 @@ 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, device or both
> + * capabilities by passing this enum as an argument to
> + * board_usb_init().
> + */

The comment style is wrong, please fix. Did the patchset pass checkpatch ?

/*
 * multi
 * line
 * comment
 */

> +enum board_usb_init_type {
> +	USB_INIT_ALL,
> +	USB_INIT_HOST,
> +	USB_INIT_DEVICE
> +};
> +
> +/* board-specific hardware initialization, called by
> + * usb drivers and u-boot commands
> + */
> +int board_usb_init(enum board_usb_init_type what_to_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..de31b0e 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,
> +int fsg_init(struct ums_board_info *);
> +void fsg_cleanup(void);
> +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 usb_gadget_handle_interrupts(void);
> +int fsg_main_thread(void *);
> 
>  #endif /* __USB_MASS_STORAGE_H__ */

Best regards,
Marek Vasut

[U-Boot] [RFC 01/10] New board-specific USB initialization interface

[Mateusz Zalega]

On 08/11/13 20:04, Marek Vasut wrote:
>> +/* You can initialize platform's USB host, device or both
>> + * capabilities by passing this enum as an argument to
>> + * board_usb_init().
>> + */
> 
> The comment style is wrong, please fix. Did the patchset pass checkpatch ?
> 
> /*
>  * multi
>  * line
>  * comment
>  */

Yes, it did:

---8<---
$ ./tools/checkpatch.pl
0001-New-board-specific-USB-initialization-interface.patch
total: 0 errors, 0 warnings, 0 checks, 154 lines checked

NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX
MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE
USLEEP_RANGE

0001-New-board-specific-USB-initialization-interface.patch has no
obvious style problems and is ready for submission.
--->8---

NETWORKING_BLOCK_COMMENT_STYLE - ...and, in our case, this option is the
culprit. Will fix.

Thanks,

-- 
Mateusz Zalega
Samsung R&D Institute Poland (SRPOL) | Kernel and System Framework group