Hi,

On Thu, Nov 15, 2012 at 04:34:09PM +0200, Roger Quadros wrote:
> Enable the optional HSIC clocks (60MHz and 480MHz) for the ports
> that are configured in HSIC mode.
> 
> Signed-off-by: Roger Quadros <rog...@ti.com>
> ---
>  drivers/mfd/omap-usb-host.c |   56 +++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 54 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
> index 0d39bd7..e5ba193 100644
> --- a/drivers/mfd/omap-usb-host.c
> +++ b/drivers/mfd/omap-usb-host.c
> @@ -88,6 +88,8 @@
>  
>  struct usbhs_port {
>       struct clk      *utmi_clk;
> +     struct clk      *hsic60m_clk;
> +     struct clk      *hsic480m_clk;
>  };
>  
>  struct usbhs_hcd_omap {
> @@ -300,6 +302,26 @@ static int usbhs_runtime_resume(struct device *dev)
>                               }
>                       }
>               }
> +
> +             /* Enable HSIC clocks if required */
> +             if (is_ehci_hsic_mode(pdata->port_mode[i])) {
> +                     if (omap->port[i].hsic60m_clk) {
> +                             r = clk_enable(omap->port[i].hsic60m_clk);
> +                             if (r) {
> +                                     dev_err(dev,
> +                                      "%s: Can't enable port %d hsic60m clk 
> : %d\n",
> +                                      __func__, i, r);
> +                             }
> +                     }
> +                     if (omap->port[i].hsic480m_clk) {
> +                             r = clk_enable(omap->port[i].hsic480m_clk);
> +                             if (r) {
> +                                     dev_err(dev,
> +                                      "%s: Can't enable port %d hsic480m clk 
> : %d\n",
> +                                      __func__, i, r);
> +                             }
> +                     }
> +             }
>       }

with this deep indentation, it should've caught your attention that
something can definitely be re-factored.

> @@ -323,6 +345,14 @@ static int usbhs_runtime_suspend(struct device *dev)
>                       if (omap->port[i].utmi_clk)
>                               clk_disable(omap->port[i].utmi_clk);
>               }
> +
> +             if (is_ehci_hsic_mode(pdata->port_mode[i])) {
> +                     if (omap->port[i].hsic60m_clk)
> +                             clk_disable(omap->port[i].hsic60m_clk);
> +
> +                     if (omap->port[i].hsic480m_clk)
> +                             clk_disable(omap->port[i].hsic480m_clk);
> +             }
>       }
>  
>       if (omap->ehci_logic_fck && !IS_ERR(omap->ehci_logic_fck))
> @@ -575,6 +605,7 @@ static int __devinit usbhs_omap_probe(struct 
> platform_device *pdev)
>       for (i = 0; i < omap->nports; i++) {
>               struct clk *pclk;
>               char utmi_clk[] = "usb_host_hs_utmi_px_clk";
> +             char hsic_clk[] = "usb_host_hs_hsic480m_px_clk";

same comment from another patch. Was this lazyness ?

> @@ -590,6 +621,21 @@ static int __devinit usbhs_omap_probe(struct 
> platform_device *pdev)
>               else
>                       omap->port[i].utmi_clk = pclk;
>  
> +             sprintf(hsic_clk, "usb_host_hs_hsic480m_p%d_clk", i + 1);

will overflow if 'i' ever goes over 8.

> +             pclk = clk_get(dev, hsic_clk);
> +             if (IS_ERR(pclk))
> +                     dev_err(dev, "Failed to get clock : %s : %ld\n",
> +                             hsic_clk, PTR_ERR(pclk));
> +             else
> +                     omap->port[i].hsic480m_clk = pclk;
> +
> +             sprintf(hsic_clk, "usb_host_hs_hsic60m_p%d_clk", i + 1);

ditto.

-- 
balbi

Attachment: signature.asc
Description: Digital signature

Reply via email to