Re: [PATCH RESEND v5 1/5] usb: host: ehci-st: Add EHCI support for ST STB devices

2014-09-05 Thread Arnd Bergmann
On Friday 05 September 2014 19:16:36 Peter Griffin wrote:
> On Fri, 05 Sep 2014, Arnd Bergmann wrote:
> 
> > On Friday 05 September 2014 18:23:45 Peter Griffin wrote:
> > > +struct st_platform_priv {
> > > +   struct clk *clks[USB_MAX_CLKS];
> > > +   struct clk *clk48;
> > > +   struct reset_control *rst;
> > > +   struct reset_control *pwr;
> > > +   struct phy *phy;
> > > +};
> > 
> > Any reason why this is in a shared header file? It looks like
> > duplicating the structure under two different names would
> > actually be shorter and keep the drivers more readable as they'd
> > be self-contained, even when they have the exact same structure.
> 
> The only reason was it was a identical structure so I put it in a shared
> header file. I can unabstract it if you want?

Yes, I think that would be an improvement. Everything looks great to me
otherwise, at least after a brief look.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RESEND v5 1/5] usb: host: ehci-st: Add EHCI support for ST STB devices

2014-09-05 Thread Peter Griffin
Hi Arnd,

On Fri, 05 Sep 2014, Arnd Bergmann wrote:

> On Friday 05 September 2014 18:23:45 Peter Griffin wrote:
> > +struct st_platform_priv {
> > +   struct clk *clks[USB_MAX_CLKS];
> > +   struct clk *clk48;
> > +   struct reset_control *rst;
> > +   struct reset_control *pwr;
> > +   struct phy *phy;
> > +};
> 
> Any reason why this is in a shared header file? It looks like
> duplicating the structure under two different names would
> actually be shorter and keep the drivers more readable as they'd
> be self-contained, even when they have the exact same structure.

The only reason was it was a identical structure so I put it in a shared
header file. I can unabstract it if you want?

> 
> Do both drivers use all fields?

Yes they are. I thought the 48Mhz clock would only be used by ohci, but 
annoyingly it also
clocks the reset logic of the ehci block as well.

regards,

Peter.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RESEND v5 1/5] usb: host: ehci-st: Add EHCI support for ST STB devices

2014-09-05 Thread Arnd Bergmann
On Friday 05 September 2014 18:23:45 Peter Griffin wrote:
> +struct st_platform_priv {
> +   struct clk *clks[USB_MAX_CLKS];
> +   struct clk *clk48;
> +   struct reset_control *rst;
> +   struct reset_control *pwr;
> +   struct phy *phy;
> +};

Any reason why this is in a shared header file? It looks like
duplicating the structure under two different names would
actually be shorter and keep the drivers more readable as they'd
be self-contained, even when they have the exact same structure.

Do both drivers use all fields?

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH RESEND v5 1/5] usb: host: ehci-st: Add EHCI support for ST STB devices

2014-09-05 Thread Peter Griffin
This patch adds the glue code required to ensure the on-chip EHCI
controller works on STi consumer electronics SoC's from STMicroelectronics.

It mainly manages the setting and enabling of the relevant clocks and manages
the reset / power signals to the IP block.

Signed-off-by: Peter Griffin 
---
 drivers/usb/host/ehci-st.c   | 365 +++
 drivers/usb/host/usb-st-common.h |  31 
 2 files changed, 396 insertions(+)
 create mode 100644 drivers/usb/host/ehci-st.c
 create mode 100644 drivers/usb/host/usb-st-common.h

diff --git a/drivers/usb/host/ehci-st.c b/drivers/usb/host/ehci-st.c
new file mode 100644
index 000..c363807
--- /dev/null
+++ b/drivers/usb/host/ehci-st.c
@@ -0,0 +1,365 @@
+/*
+ * ST EHCI driver
+ *
+ * Copyright (C) 2014 STMicroelectronics – All Rights Reserved
+ *
+ * Author: Peter Griffin 
+ *
+ * Derived from ehci-platform.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ehci.h"
+#include "usb-st-common.h"
+
+#define DRIVER_DESC "EHCI STMicroelectronics driver"
+
+#define hcd_to_ehci_priv(h) ((struct st_platform_priv *)hcd_to_ehci(h)->priv)
+
+static const char hcd_name[] = "ehci-st";
+
+#define EHCI_CAPS_SIZE 0x10
+#define AHB2STBUS_INSREG01 (EHCI_CAPS_SIZE + 0x84)
+
+static int st_ehci_platform_reset(struct usb_hcd *hcd)
+{
+   struct platform_device *pdev = to_platform_device(hcd->self.controller);
+   struct usb_ehci_pdata *pdata = dev_get_platdata(&pdev->dev);
+   struct ehci_hcd *ehci = hcd_to_ehci(hcd);
+   int retval;
+   u32 threshold;
+
+   /* Set EHCI packet buffer IN/OUT threshold to 128 bytes */
+   threshold = 128 | (128 << 16);
+   writel(threshold, hcd->regs + AHB2STBUS_INSREG01);
+
+   ehci->caps = hcd->regs + pdata->caps_offset;
+   retval = ehci_setup(hcd);
+   if (retval)
+   return retval;
+
+   return 0;
+}
+
+static int st_ehci_platform_power_on(struct platform_device *dev)
+{
+   struct usb_hcd *hcd = platform_get_drvdata(dev);
+   struct st_platform_priv *priv = hcd_to_ehci_priv(hcd);
+   int clk, ret;
+
+   ret = reset_control_deassert(priv->pwr);
+   if (ret)
+   return ret;
+
+   ret = reset_control_deassert(priv->rst);
+   if (ret)
+   goto err_assert_power;
+
+   /* some SoCs don't have a dedicated 48Mhz clock, but those that do
+  need the rate to be explicitly set */
+   if (priv->clk48) {
+   ret = clk_set_rate(priv->clk48, 4800);
+   if (ret)
+   goto err_assert_reset;
+   }
+
+   for (clk = 0; clk < USB_MAX_CLKS && priv->clks[clk]; clk++) {
+   ret = clk_prepare_enable(priv->clks[clk]);
+   if (ret)
+   goto err_disable_clks;
+   }
+
+   ret = phy_init(priv->phy);
+   if (ret)
+   goto err_disable_clks;
+
+   ret = phy_power_on(priv->phy);
+   if (ret)
+   goto err_exit_phy;
+
+   return 0;
+
+err_exit_phy:
+   phy_exit(priv->phy);
+err_disable_clks:
+   while (--clk >= 0)
+   clk_disable_unprepare(priv->clks[clk]);
+err_assert_reset:
+   reset_control_assert(priv->rst);
+err_assert_power:
+   reset_control_assert(priv->pwr);
+
+   return ret;
+}
+
+static void st_ehci_platform_power_off(struct platform_device *dev)
+{
+   struct usb_hcd *hcd = platform_get_drvdata(dev);
+   struct st_platform_priv *priv = hcd_to_ehci_priv(hcd);
+   int clk;
+
+   reset_control_assert(priv->pwr);
+
+   reset_control_assert(priv->rst);
+
+   phy_power_off(priv->phy);
+
+   phy_exit(priv->phy);
+
+   for (clk = USB_MAX_CLKS - 1; clk >= 0; clk--)
+   if (priv->clks[clk])
+   clk_disable_unprepare(priv->clks[clk]);
+
+}
+
+static struct hc_driver __read_mostly ehci_platform_hc_driver;
+
+static const struct ehci_driver_overrides platform_overrides __initconst = {
+   .reset =st_ehci_platform_reset,
+   .extra_priv_size =  sizeof(struct st_platform_priv),
+};
+
+static struct usb_ehci_pdata ehci_platform_defaults = {
+   .power_on = st_ehci_platform_power_on,
+   .power_suspend =st_ehci_platform_power_off,
+   .power_off =st_ehci_platform_power_off,
+};
+
+static int st_ehci_platform_probe(struct platform_device *dev)
+{
+   struct usb_hcd *hcd;
+   struct resource *res_mem;
+   struct usb_ehci_pdata *pdata = &ehci_platform_defaults;
+   struct st_platform_priv *priv;
+   struct ehci_hcd *ehci;
+   int err, irq, clk = 0;
+
+   if (usb_disabled())
+