Re: [PATCH 2/4] remoteproc: Supply controller driver for ST's Remote Processors

2015-08-26 Thread Lee Jones
On Wed, 26 Aug 2015, Nathan Lynch wrote:

> On 08/26/2015 08:08 AM, Lee Jones wrote:
> > --- /dev/null
> > +++ b/drivers/remoteproc/st_remoteproc.c
> > @@ -0,0 +1,300 @@
> > +/*
> > + * ST's Remote Processor Control Driver
> > + *
> > + * Copyright (C) 2015 STMicroelectronics - All Rights Reserved
> > + *
> > + * Author: Ludovic Barre 
> 
> When submitting code you didn't write, I'd say it's better practice to
> clearly indicate its provenance in the commit message.  E.g. something
> like "Driver based on code authored by Ludovic Barre for ST".  And
> obtain signoffs etc if possible.
> 
> 
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License.
> 
> Please review the wording here.  It's unclear whether this is intended
> to be v2-only or v2 or later.
> 
> 
> > +static int st_rproc_stop(struct rproc *rproc)
> > +{
> > +   struct st_rproc *st_rproc = rproc->priv;
> > +   int err = 0;
> > +
> > +   if (st_rproc->config->sw_reset) {
> > +   err = reset_control_assert(st_rproc->sw_reset);
> > +   if (err)
> > +   dev_warn(&rproc->dev, "Failed to assert S/W Reset\n");
> > +   }
> > +
> > +   if (st_rproc->config->pwr_reset) {
> > +   err = reset_control_assert(st_rproc->pwr_reset);
> > +   if (err)
> > +   dev_warn(&rproc->dev, "Failed to assert Power Reset\n");
> > +   }
> > +
> > +   clk_disable(st_rproc->clk);
> > +
> > +   return 0;
> > +}
> 
> Seems like st_rproc_stop should propagate errors back to its caller
> instead of always returning 0.

Good points, will fix.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/4] remoteproc: Supply controller driver for ST's Remote Processors

2015-08-26 Thread Nathan Lynch
On 08/26/2015 08:08 AM, Lee Jones wrote:
> --- /dev/null
> +++ b/drivers/remoteproc/st_remoteproc.c
> @@ -0,0 +1,300 @@
> +/*
> + * ST's Remote Processor Control Driver
> + *
> + * Copyright (C) 2015 STMicroelectronics - All Rights Reserved
> + *
> + * Author: Ludovic Barre 

When submitting code you didn't write, I'd say it's better practice to
clearly indicate its provenance in the commit message.  E.g. something
like "Driver based on code authored by Ludovic Barre for ST".  And
obtain signoffs etc if possible.


> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License.

Please review the wording here.  It's unclear whether this is intended
to be v2-only or v2 or later.


> +static int st_rproc_stop(struct rproc *rproc)
> +{
> + struct st_rproc *st_rproc = rproc->priv;
> + int err = 0;
> +
> + if (st_rproc->config->sw_reset) {
> + err = reset_control_assert(st_rproc->sw_reset);
> + if (err)
> + dev_warn(&rproc->dev, "Failed to assert S/W Reset\n");
> + }
> +
> + if (st_rproc->config->pwr_reset) {
> + err = reset_control_assert(st_rproc->pwr_reset);
> + if (err)
> + dev_warn(&rproc->dev, "Failed to assert Power Reset\n");
> + }
> +
> + clk_disable(st_rproc->clk);
> +
> + return 0;
> +}

Seems like st_rproc_stop should propagate errors back to its caller
instead of always returning 0.

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


[PATCH 2/4] remoteproc: Supply controller driver for ST's Remote Processors

2015-08-26 Thread Lee Jones
Signed-off-by: Lee Jones 
---
 drivers/remoteproc/Kconfig |   9 ++
 drivers/remoteproc/Makefile|   1 +
 drivers/remoteproc/st_remoteproc.c | 300 +
 3 files changed, 310 insertions(+)
 create mode 100644 drivers/remoteproc/st_remoteproc.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 28c711f..72e97d7 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -77,4 +77,13 @@ config DA8XX_REMOTEPROC
  It's safe to say n here if you're not interested in multimedia
  offloading.
 
+config ST_REMOTEPROC
+   tristate "ST remoteproc support"
+   depends on ARCH_STI
+   select REMOTEPROC
+   help
+ Say y here to support ST's adjunct processors via the remote
+ processor framework.
+ This can be either built-in or a loadable module.
+
 endmenu
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 81b04d1..279cb2e 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_OMAP_REMOTEPROC) += omap_remoteproc.o
 obj-$(CONFIG_STE_MODEM_RPROC)  += ste_modem_rproc.o
 obj-$(CONFIG_WKUP_M3_RPROC)+= wkup_m3_rproc.o
 obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o
+obj-$(CONFIG_ST_REMOTEPROC)+= st_remoteproc.o
diff --git a/drivers/remoteproc/st_remoteproc.c 
b/drivers/remoteproc/st_remoteproc.c
new file mode 100644
index 000..7afcfaf
--- /dev/null
+++ b/drivers/remoteproc/st_remoteproc.c
@@ -0,0 +1,300 @@
+/*
+ * ST's Remote Processor Control Driver
+ *
+ * Copyright (C) 2015 STMicroelectronics - All Rights Reserved
+ *
+ * Author: Ludovic Barre 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct st_rproc_config {
+   boolsw_reset;
+   boolpwr_reset;
+   u32 bootaddr_mask;
+};
+
+struct st_rproc {
+   struct st_rproc_config  *config;
+   struct reset_control*sw_reset;
+   struct reset_control*pwr_reset;
+   struct clk  *clk;
+   u32 clk_rate;
+   struct  regmap  *boot_base;
+   u32 boot_offset;
+};
+
+static int st_rproc_stop(struct rproc *rproc)
+{
+   struct st_rproc *st_rproc = rproc->priv;
+   int err = 0;
+
+   if (st_rproc->config->sw_reset) {
+   err = reset_control_assert(st_rproc->sw_reset);
+   if (err)
+   dev_warn(&rproc->dev, "Failed to assert S/W Reset\n");
+   }
+
+   if (st_rproc->config->pwr_reset) {
+   err = reset_control_assert(st_rproc->pwr_reset);
+   if (err)
+   dev_warn(&rproc->dev, "Failed to assert Power Reset\n");
+   }
+
+   clk_disable(st_rproc->clk);
+
+   return 0;
+}
+
+static int st_rproc_start(struct rproc *rproc)
+{
+   struct st_rproc *st_rproc = rproc->priv;
+   int err;
+
+   regmap_update_bits(st_rproc->boot_base, st_rproc->boot_offset,
+  st_rproc->config->bootaddr_mask, rproc->bootaddr);
+
+   err = clk_enable(st_rproc->clk);
+   if (err) {
+   dev_err(&rproc->dev, "Failed to enable clock\n");
+   return err;
+   }
+
+   if (st_rproc->config->sw_reset) {
+   err = reset_control_deassert(st_rproc->sw_reset);
+   if (err) {
+   dev_err(&rproc->dev, "Failed to deassert S/W Reset\n");
+   return err;
+   }
+   }
+
+   if (st_rproc->config->pwr_reset) {
+   err = reset_control_deassert(st_rproc->pwr_reset);
+   if (err) {
+   dev_err(&rproc->dev, "Failed to deassert Power 
Reset\n");
+   return err;
+   }
+   }
+
+   dev_info(&rproc->dev, "Started from 0x%x\n", rproc->bootaddr);
+
+   return 0;
+}
+
+static struct rproc_ops st_rproc_ops = {
+   .start  = st_rproc_start,
+   .stop   = st_rproc_stop,
+};
+
+/*
+ * Fetch state of the processor: 0 is off, 1 is on.
+ */
+static int st_rproc_state(struct st_rproc *st_rproc)
+{
+   int reset_sw, reset_pwr;
+
+   reset_sw = st_rproc->config->sw_reset ?
+   reset_control_status(st_rproc->sw_reset) : 0;
+
+   reset_pwr = st_rproc->config->pwr_reset ?
+   reset_control_status(st_rproc->pwr_reset) : 0;
+
+   if (reset_sw < 0 || reset_pwr < 0)
+   return -EINVAL;
+
+   return !reset_sw && !reset_pwr;
+}
+
+static const struct st_rproc_config st40_rproc