Patchwork [U-Boot,v2,2/7] tegra: usb: make controller init functions more self contained

login
register
mail settings
Submitter Lucas Stach
Date Jan. 25, 2013, 2:41 p.m.
Message ID <1359124871-4434-3-git-send-email-dev@lynxeye.de>
Download mbox | patch
Permalink /patch/215753/
State Superseded
Delegated to: Tom Warren
Headers show

Comments

Lucas Stach - Jan. 25, 2013, 2:41 p.m.
There is no need to pass around all those parameters. The init functions
are able to easily extract all the needed setup info on their own.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
To clarify why this is a good thing an excerpt from the first round of
review:
"The intent of this patch is not really to save up on parameters passed,
but to make it possible to later move out the controller initialization
into the ehci_hcd_init function without having to save away this global
state for later use[,thus avoid bloating the file global state]."
---
 arch/arm/cpu/armv7/tegra20/usb.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)
Simon Glass - Jan. 27, 2013, 4:31 p.m.
Hi Lucas,

On Sat, Jan 26, 2013 at 3:41 AM, Lucas Stach <dev@lynxeye.de> wrote:
> There is no need to pass around all those parameters. The init functions
> are able to easily extract all the needed setup info on their own.
>

> Signed-off-by: Lucas Stach <dev@lynxeye.de>

Acked-by: Simon Glass <sjg@chromium.org>

Minor comment: I think the commit message should include what you have
below the --- since it provides the motivation for the commit.

> ---
> To clarify why this is a good thing an excerpt from the first round of
> review:
> "The intent of this patch is not really to save up on parameters passed,
> but to make it possible to later move out the controller initialization
> into the ehci_hcd_init function without having to save away this global
> state for later use[,thus avoid bloating the file global state]."
> ---
>  arch/arm/cpu/armv7/tegra20/usb.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/tegra20/usb.c b/arch/arm/cpu/armv7/tegra20/usb.c
> index f151fb2..07c1ade 100644
> --- a/arch/arm/cpu/armv7/tegra20/usb.c
> +++ b/arch/arm/cpu/armv7/tegra20/usb.c
> @@ -198,11 +198,12 @@ void usbf_reset_controller(struct fdt_usb *config, struct usb_ctlr *usbctlr)
>  }
>
>  /* set up the UTMI USB controller with the parameters provided */
> -static int init_utmi_usb_controller(struct fdt_usb *config,
> -                               struct usb_ctlr *usbctlr, const u32 timing[])
> +static int init_utmi_usb_controller(struct fdt_usb *config)
>  {
>         u32 val;
>         int loop_count;
> +       const unsigned *timing;
> +       struct usb_ctlr *usbctlr = config->reg;
>
>         clock_enable(config->periph_id);
>
> @@ -229,6 +230,8 @@ static int init_utmi_usb_controller(struct fdt_usb *config,
>          * PLL Delay CONFIGURATION settings. The following parameters control
>          * the bring up of the plls.
>          */
> +       timing = usb_pll[clock_get_osc_freq()];
> +
>         val = readl(&usbctlr->utmip_misc_cfg1);
>         clrsetbits_le32(&val, UTMIP_PLLU_STABLE_COUNT_MASK,
>                 timing[PARAM_STABLE_COUNT] << UTMIP_PLLU_STABLE_COUNT_SHIFT);
> @@ -331,12 +334,12 @@ static int init_utmi_usb_controller(struct fdt_usb *config,
>  #endif
>
>  /* set up the ULPI USB controller with the parameters provided */
> -static int init_ulpi_usb_controller(struct fdt_usb *config,
> -                               struct usb_ctlr *usbctlr)
> +static int init_ulpi_usb_controller(struct fdt_usb *config)
>  {
>         u32 val;
>         int loop_count;
>         struct ulpi_viewport ulpi_vp;
> +       struct usb_ctlr *usbctlr = config->reg;
>
>         /* set up ULPI reference clock on pllp_out4 */
>         clock_enable(PERIPH_ID_DEV2_OUT);
> @@ -408,8 +411,7 @@ static int init_ulpi_usb_controller(struct fdt_usb *config,
>         return 0;
>  }
>  #else
> -static int init_ulpi_usb_controller(struct fdt_usb *config,
> -                               struct usb_ctlr *usbctlr)
> +static int init_ulpi_usb_controller(struct fdt_usb *config)
>  {
>         printf("No code to set up ULPI controller, please enable"
>                         "CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT");
> @@ -430,22 +432,20 @@ static void config_clock(const u32 timing[])
>   * @param config       USB port configuration
>   * @return 0 if ok, -1 if error (too many ports)
>   */
> -static int add_port(struct fdt_usb *config, const u32 timing[])
> +static int add_port(struct fdt_usb *config)
>  {
> -       struct usb_ctlr *usbctlr = config->reg;
> -
>         if (port_count == USB_PORTS_MAX) {
>                 printf("tegrausb: Cannot register more than %d ports\n",
>                       USB_PORTS_MAX);
>                 return -1;
>         }
>
> -       if (config->utmi && init_utmi_usb_controller(config, usbctlr, timing)) {
> +       if (config->utmi && init_utmi_usb_controller(config)) {
>                 printf("tegrausb: Cannot init port\n");
>                 return -1;
>         }
>
> -       if (config->ulpi && init_ulpi_usb_controller(config, usbctlr)) {
> +       if (config->ulpi && init_ulpi_usb_controller(config)) {
>                 printf("tegrausb: Cannot init port\n");
>                 return -1;
>         }
> @@ -558,7 +558,7 @@ int board_usb_init(const void *blob)
>                         return -1;
>                 }
>
> -               if (add_port(&config, usb_pll[freq]))
> +               if (add_port(&config))
>                         return -1;
>                 set_host_mode(&config);
>         }
> --
> 1.8.0.2
>

Patch

diff --git a/arch/arm/cpu/armv7/tegra20/usb.c b/arch/arm/cpu/armv7/tegra20/usb.c
index f151fb2..07c1ade 100644
--- a/arch/arm/cpu/armv7/tegra20/usb.c
+++ b/arch/arm/cpu/armv7/tegra20/usb.c
@@ -198,11 +198,12 @@  void usbf_reset_controller(struct fdt_usb *config, struct usb_ctlr *usbctlr)
 }
 
 /* set up the UTMI USB controller with the parameters provided */
-static int init_utmi_usb_controller(struct fdt_usb *config,
-				struct usb_ctlr *usbctlr, const u32 timing[])
+static int init_utmi_usb_controller(struct fdt_usb *config)
 {
 	u32 val;
 	int loop_count;
+	const unsigned *timing;
+	struct usb_ctlr *usbctlr = config->reg;
 
 	clock_enable(config->periph_id);
 
@@ -229,6 +230,8 @@  static int init_utmi_usb_controller(struct fdt_usb *config,
 	 * PLL Delay CONFIGURATION settings. The following parameters control
 	 * the bring up of the plls.
 	 */
+	timing = usb_pll[clock_get_osc_freq()];
+
 	val = readl(&usbctlr->utmip_misc_cfg1);
 	clrsetbits_le32(&val, UTMIP_PLLU_STABLE_COUNT_MASK,
 		timing[PARAM_STABLE_COUNT] << UTMIP_PLLU_STABLE_COUNT_SHIFT);
@@ -331,12 +334,12 @@  static int init_utmi_usb_controller(struct fdt_usb *config,
 #endif
 
 /* set up the ULPI USB controller with the parameters provided */
-static int init_ulpi_usb_controller(struct fdt_usb *config,
-				struct usb_ctlr *usbctlr)
+static int init_ulpi_usb_controller(struct fdt_usb *config)
 {
 	u32 val;
 	int loop_count;
 	struct ulpi_viewport ulpi_vp;
+	struct usb_ctlr *usbctlr = config->reg;
 
 	/* set up ULPI reference clock on pllp_out4 */
 	clock_enable(PERIPH_ID_DEV2_OUT);
@@ -408,8 +411,7 @@  static int init_ulpi_usb_controller(struct fdt_usb *config,
 	return 0;
 }
 #else
-static int init_ulpi_usb_controller(struct fdt_usb *config,
-				struct usb_ctlr *usbctlr)
+static int init_ulpi_usb_controller(struct fdt_usb *config)
 {
 	printf("No code to set up ULPI controller, please enable"
 			"CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT");
@@ -430,22 +432,20 @@  static void config_clock(const u32 timing[])
  * @param config	USB port configuration
  * @return 0 if ok, -1 if error (too many ports)
  */
-static int add_port(struct fdt_usb *config, const u32 timing[])
+static int add_port(struct fdt_usb *config)
 {
-	struct usb_ctlr *usbctlr = config->reg;
-
 	if (port_count == USB_PORTS_MAX) {
 		printf("tegrausb: Cannot register more than %d ports\n",
 		      USB_PORTS_MAX);
 		return -1;
 	}
 
-	if (config->utmi && init_utmi_usb_controller(config, usbctlr, timing)) {
+	if (config->utmi && init_utmi_usb_controller(config)) {
 		printf("tegrausb: Cannot init port\n");
 		return -1;
 	}
 
-	if (config->ulpi && init_ulpi_usb_controller(config, usbctlr)) {
+	if (config->ulpi && init_ulpi_usb_controller(config)) {
 		printf("tegrausb: Cannot init port\n");
 		return -1;
 	}
@@ -558,7 +558,7 @@  int board_usb_init(const void *blob)
 			return -1;
 		}
 
-		if (add_port(&config, usb_pll[freq]))
+		if (add_port(&config))
 			return -1;
 		set_host_mode(&config);
 	}