[PATCH v8 01/18] remoteproc: st_slim_rproc: add a slimcore rproc driver
On Tue, 30 Aug 2016, Peter Griffin wrote: > On Tue, 30 Aug 2016, Lee Jones wrote: > > On Fri, 26 Aug 2016, Peter Griffin wrote: > > > > > slim core is used as a basis for many IPs in the STi > > > chipsets such as fdma and demux. To avoid duplicating > > > the elf loading code in each device driver a slim > > > rproc driver has been created. > > > > > > This driver is designed to be used by other device drivers > > > such as fdma, or demux whose IP is based around a slim core. > > > The device driver can call slim_rproc_alloc() to allocate > > > a slim rproc and slim_rproc_put() when finished. > > > > > > This driver takes care of ioremapping the slim > > > registers (dmem, imem, slimcore, peripherals), whose offsets > > > and sizes can change between IP's. It also obtains and enables > > > any clocks used by the device. This approach avoids having > > > a double mapping of the registers as slim_rproc does not register > > > its own platform device. It also maps well to device tree > > > abstraction as it allows us to have one dt node for the whole > > > device. > > > > > > All of the generic rproc elf loading code can be reused, and > > > we provide start() stop() hooks to start and stop the slim > > > core once the firmware has been loaded. This has been tested > > > successfully with fdma driver. > > > > Nit. It would be good to use a constant line-wrap. > > > > 'M-x post-mode' will help with this. > > Can you provide the magic which makes this happen for GIT commit messages? I tend to do it manually. However a 3 second Google search produced [0], which looks like it could be fun/useful. [0] https://www.emacswiki.org/emacs/Git [...] > > > + * License terms: GNU General Public License (GPL), version 2 > > > > Are you sure ST are okay with the shortened version of the GPL? > > Do you mean the banner should be like this? > > * 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, or > * (at your option) any later version. Yes, exactly. [...] > > > +/* slimcore registers */ > > > > What's it called? slimcore, slim core, ST Slim? > > It is usually referred to as SLIM core, or SLIM CPU in the various functional > specifications. > > > > > Please be consistent. Use the name from the datasheet. > > OK. The datasheet isn't consistent either, so we will settle on SLIM core and > SLIM CPU. Perfect. -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org â Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
[PATCH v8 01/18] remoteproc: st_slim_rproc: add a slimcore rproc driver
Hi Bjorn, On Tue, 30 Aug 2016, Bjorn Andersson wrote: > On Tue 30 Aug 05:34 PDT 2016, Lee Jones wrote: > > Thanks for your review Lee. > > > On Fri, 26 Aug 2016, Peter Griffin wrote: > [..] > > > diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig > > > index 1a8bf76a..06765e0 100644 > > > --- a/drivers/remoteproc/Kconfig > > > +++ b/drivers/remoteproc/Kconfig > > > @@ -100,4 +100,12 @@ config ST_REMOTEPROC > > > processor framework. > > > This can be either built-in or a loadable module. > > > > > > +config ST_SLIM_REMOTEPROC > > > + tristate "ST Slim remoteproc support" > > > + select REMOTEPROC > > > + help > > > + Say y here to support firmware loading on IP based around > > > + the Slim core. > > > + If unsure say N. > > Saw one more thing when browsing through... > > As this piece of code doesn't do anything on its own and is going to be > selected by the "function driver" I don't think this should be > user-selectable. Applogies, I believe you pointed this out in a previous review, but it seems to have slipped through the net. Will fix in the next version. Regards, Peter.
[PATCH v8 01/18] remoteproc: st_slim_rproc: add a slimcore rproc driver
Hi Lee, Thanks for reviewing. On Tue, 30 Aug 2016, Lee Jones wrote: > On Fri, 26 Aug 2016, Peter Griffin wrote: > > > slim core is used as a basis for many IPs in the STi > > chipsets such as fdma and demux. To avoid duplicating > > the elf loading code in each device driver a slim > > rproc driver has been created. > > > > This driver is designed to be used by other device drivers > > such as fdma, or demux whose IP is based around a slim core. > > The device driver can call slim_rproc_alloc() to allocate > > a slim rproc and slim_rproc_put() when finished. > > > > This driver takes care of ioremapping the slim > > registers (dmem, imem, slimcore, peripherals), whose offsets > > and sizes can change between IP's. It also obtains and enables > > any clocks used by the device. This approach avoids having > > a double mapping of the registers as slim_rproc does not register > > its own platform device. It also maps well to device tree > > abstraction as it allows us to have one dt node for the whole > > device. > > > > All of the generic rproc elf loading code can be reused, and > > we provide start() stop() hooks to start and stop the slim > > core once the firmware has been loaded. This has been tested > > successfully with fdma driver. > > Nit. It would be good to use a constant line-wrap. > > 'M-x post-mode' will help with this. Can you provide the magic which makes this happen for GIT commit messages? > > > Signed-off-by: Peter Griffin > > --- > > drivers/remoteproc/Kconfig | 8 + > > drivers/remoteproc/Makefile | 1 + > > drivers/remoteproc/st_slim_rproc.c | 364 > > +++ > > include/linux/remoteproc/st_slim_rproc.h | 53 + > > 4 files changed, 426 insertions(+) > > create mode 100644 drivers/remoteproc/st_slim_rproc.c > > create mode 100644 include/linux/remoteproc/st_slim_rproc.h > > > > diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig > > index 1a8bf76a..06765e0 100644 > > --- a/drivers/remoteproc/Kconfig > > +++ b/drivers/remoteproc/Kconfig > > @@ -100,4 +100,12 @@ config ST_REMOTEPROC > > processor framework. > > This can be either built-in or a loadable module. > > > > +config ST_SLIM_REMOTEPROC > > + tristate "ST Slim remoteproc support" > > + select REMOTEPROC > > + help > > + Say y here to support firmware loading on IP based around > > + the Slim core. > > + If unsure say N. > > + > > endmenu > > diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile > > index 92d3758..db1dae7 100644 > > --- a/drivers/remoteproc/Makefile > > +++ b/drivers/remoteproc/Makefile > > @@ -14,3 +14,4 @@ obj-$(CONFIG_DA8XX_REMOTEPROC)+= > > da8xx_remoteproc.o > > obj-$(CONFIG_QCOM_MDT_LOADER) += qcom_mdt_loader.o > > obj-$(CONFIG_QCOM_Q6V5_PIL)+= qcom_q6v5_pil.o > > obj-$(CONFIG_ST_REMOTEPROC)+= st_remoteproc.o > > +obj-$(CONFIG_ST_SLIM_REMOTEPROC) += st_slim_rproc.o > > diff --git a/drivers/remoteproc/st_slim_rproc.c > > b/drivers/remoteproc/st_slim_rproc.c > > new file mode 100644 > > index 000..f4bf2d7 > > --- /dev/null > > +++ b/drivers/remoteproc/st_slim_rproc.c > > @@ -0,0 +1,364 @@ > > +/* > > + * st_slim_rproc.c > > These serve no purpose and have a habit of becoming out-of-date. > Please remove it and replace with a nice succinct description > instead. OK will fix. > > > + * Copyright (C) 2016 STMicroelectronics > > Nit: '\n' here. Will fix. > > > + * Author: Peter Griffin > > Nit: '\n' here. Will fix. > > > + * License terms: GNU General Public License (GPL), version 2 > > Are you sure ST are okay with the shortened version of the GPL? Do you mean the banner should be like this? * 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, or * (at your option) any later version. As I'm not aware of a shortened version of the GPV v2 license. > > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include "remoteproc_internal.h" > > + > > +/* slimcore registers */ > > What's it called? slimcore, slim core, ST Slim? It is usually referred to as SLIM core, or SLIM CPU in the various functional specifications. > > Please be consistent. Use the name from the datasheet. OK. The datasheet isn't consistent either, so we will settle on SLIM core and SLIM CPU. > > > +#define SLIM_ID_OFST 0x0 > > +#define SLIM_VER_OFST 0x4 > > + > > +#define SLIM_EN_OFST 0x8 > > +#define SLIM_EN_RUNBIT(0) > > + > > +#define SLIM_CLK_GATE_OFST 0xC > > +#define SLIM_CLK_GATE_DIS BIT(0) > > +#define SLIM_CLK_GATE_RESETBIT(2) > > + > > +#define
[PATCH v8 01/18] remoteproc: st_slim_rproc: add a slimcore rproc driver
On Fri, 26 Aug 2016, Peter Griffin wrote:
> slim core is used as a basis for many IPs in the STi
> chipsets such as fdma and demux. To avoid duplicating
> the elf loading code in each device driver a slim
> rproc driver has been created.
>
> This driver is designed to be used by other device drivers
> such as fdma, or demux whose IP is based around a slim core.
> The device driver can call slim_rproc_alloc() to allocate
> a slim rproc and slim_rproc_put() when finished.
>
> This driver takes care of ioremapping the slim
> registers (dmem, imem, slimcore, peripherals), whose offsets
> and sizes can change between IP's. It also obtains and enables
> any clocks used by the device. This approach avoids having
> a double mapping of the registers as slim_rproc does not register
> its own platform device. It also maps well to device tree
> abstraction as it allows us to have one dt node for the whole
> device.
>
> All of the generic rproc elf loading code can be reused, and
> we provide start() stop() hooks to start and stop the slim
> core once the firmware has been loaded. This has been tested
> successfully with fdma driver.
Nit. It would be good to use a constant line-wrap.
'M-x post-mode' will help with this.
> Signed-off-by: Peter Griffin
> ---
> drivers/remoteproc/Kconfig | 8 +
> drivers/remoteproc/Makefile | 1 +
> drivers/remoteproc/st_slim_rproc.c | 364
> +++
> include/linux/remoteproc/st_slim_rproc.h | 53 +
> 4 files changed, 426 insertions(+)
> create mode 100644 drivers/remoteproc/st_slim_rproc.c
> create mode 100644 include/linux/remoteproc/st_slim_rproc.h
>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 1a8bf76a..06765e0 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -100,4 +100,12 @@ config ST_REMOTEPROC
> processor framework.
> This can be either built-in or a loadable module.
>
> +config ST_SLIM_REMOTEPROC
> + tristate "ST Slim remoteproc support"
> + select REMOTEPROC
> + help
> + Say y here to support firmware loading on IP based around
> + the Slim core.
> + If unsure say N.
> +
> endmenu
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index 92d3758..db1dae7 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -14,3 +14,4 @@ obj-$(CONFIG_DA8XX_REMOTEPROC) +=
> da8xx_remoteproc.o
> obj-$(CONFIG_QCOM_MDT_LOADER)+= qcom_mdt_loader.o
> obj-$(CONFIG_QCOM_Q6V5_PIL) += qcom_q6v5_pil.o
> obj-$(CONFIG_ST_REMOTEPROC) += st_remoteproc.o
> +obj-$(CONFIG_ST_SLIM_REMOTEPROC) += st_slim_rproc.o
> diff --git a/drivers/remoteproc/st_slim_rproc.c
> b/drivers/remoteproc/st_slim_rproc.c
> new file mode 100644
> index 000..f4bf2d7
> --- /dev/null
> +++ b/drivers/remoteproc/st_slim_rproc.c
> @@ -0,0 +1,364 @@
> +/*
> + * st_slim_rproc.c
These serve no purpose and have a habit of becoming out-of-date.
Please remove it and replace with a nice succinct description
instead.
> + * Copyright (C) 2016 STMicroelectronics
Nit: '\n' here.
> + * Author: Peter Griffin
Nit: '\n' here.
> + * License terms: GNU General Public License (GPL), version 2
Are you sure ST are okay with the shortened version of the GPL?
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include "remoteproc_internal.h"
> +
> +/* slimcore registers */
What's it called? slimcore, slim core, ST Slim?
Please be consistent. Use the name from the datasheet.
> +#define SLIM_ID_OFST 0x0
> +#define SLIM_VER_OFST0x4
> +
> +#define SLIM_EN_OFST 0x8
> +#define SLIM_EN_RUN BIT(0)
> +
> +#define SLIM_CLK_GATE_OFST 0xC
> +#define SLIM_CLK_GATE_DISBIT(0)
> +#define SLIM_CLK_GATE_RESET BIT(2)
> +
> +#define SLIM_SLIM_PC_OFST0x20
> +
> +/* dmem registers */
> +#define SLIM_REV_ID_OFST 0x0
> +#define SLIM_REV_ID_MIN_MASK GENMASK(15, 8)
> +#define SLIM_REV_ID_MIN(id) ((id & SLIM_REV_ID_MIN_MASK) >> 8)
> +#define SLIM_REV_ID_MAJ_MASK GENMASK(23, 16)
> +#define SLIM_REV_ID_MAJ(id) ((id & SLIM_REV_ID_MAJ_MASK) >> 16)
> +
> +
> +/* peripherals registers */
> +#define SLIM_STBUS_SYNC_OFST 0xF88
> +#define SLIM_STBUS_SYNC_DIS BIT(0)
> +
> +#define SLIM_INT_SET_OFST0xFD4
> +#define SLIM_INT_CLR_OFST0xFD8
> +#define SLIM_INT_MASK_OFST 0xFDC
> +
> +#define SLIM_CMD_CLR_OFST0xFC8
> +#define SLIM_CMD_MASK_OFST 0xFCC
> +
> +static const char *mem_names[ST_SLIM_MEM_MAX] = {
> + [ST_SLIM_DMEM] = "dmem",
> + [ST_SLIM_IMEM] = "imem",
> +};
> +
> +static int slim_clk_get(struct st_slim_rproc *slim_rproc, struct device *dev)
> +{
> + int clk, err;
> +
> + for (clk = 0; clk < ST_SLIM_MAX_CLK; clk++) {
> +
[PATCH v8 01/18] remoteproc: st_slim_rproc: add a slimcore rproc driver
On Tue 30 Aug 05:34 PDT 2016, Lee Jones wrote: Thanks for your review Lee. > On Fri, 26 Aug 2016, Peter Griffin wrote: [..] > > diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig > > index 1a8bf76a..06765e0 100644 > > --- a/drivers/remoteproc/Kconfig > > +++ b/drivers/remoteproc/Kconfig > > @@ -100,4 +100,12 @@ config ST_REMOTEPROC > > processor framework. > > This can be either built-in or a loadable module. > > > > +config ST_SLIM_REMOTEPROC > > + tristate "ST Slim remoteproc support" > > + select REMOTEPROC > > + help > > + Say y here to support firmware loading on IP based around > > + the Slim core. > > + If unsure say N. Saw one more thing when browsing through... As this piece of code doesn't do anything on its own and is going to be selected by the "function driver" I don't think this should be user-selectable. Regards, Bjorn
[PATCH v8 01/18] remoteproc: st_slim_rproc: add a slimcore rproc driver
slim core is used as a basis for many IPs in the STi
chipsets such as fdma and demux. To avoid duplicating
the elf loading code in each device driver a slim
rproc driver has been created.
This driver is designed to be used by other device drivers
such as fdma, or demux whose IP is based around a slim core.
The device driver can call slim_rproc_alloc() to allocate
a slim rproc and slim_rproc_put() when finished.
This driver takes care of ioremapping the slim
registers (dmem, imem, slimcore, peripherals), whose offsets
and sizes can change between IP's. It also obtains and enables
any clocks used by the device. This approach avoids having
a double mapping of the registers as slim_rproc does not register
its own platform device. It also maps well to device tree
abstraction as it allows us to have one dt node for the whole
device.
All of the generic rproc elf loading code can be reused, and
we provide start() stop() hooks to start and stop the slim
core once the firmware has been loaded. This has been tested
successfully with fdma driver.
Signed-off-by: Peter Griffin
---
drivers/remoteproc/Kconfig | 8 +
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/st_slim_rproc.c | 364 +++
include/linux/remoteproc/st_slim_rproc.h | 53 +
4 files changed, 426 insertions(+)
create mode 100644 drivers/remoteproc/st_slim_rproc.c
create mode 100644 include/linux/remoteproc/st_slim_rproc.h
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 1a8bf76a..06765e0 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -100,4 +100,12 @@ config ST_REMOTEPROC
processor framework.
This can be either built-in or a loadable module.
+config ST_SLIM_REMOTEPROC
+ tristate "ST Slim remoteproc support"
+ select REMOTEPROC
+ help
+ Say y here to support firmware loading on IP based around
+ the Slim core.
+ If unsure say N.
+
endmenu
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 92d3758..db1dae7 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -14,3 +14,4 @@ obj-$(CONFIG_DA8XX_REMOTEPROC)+=
da8xx_remoteproc.o
obj-$(CONFIG_QCOM_MDT_LOADER) += qcom_mdt_loader.o
obj-$(CONFIG_QCOM_Q6V5_PIL)+= qcom_q6v5_pil.o
obj-$(CONFIG_ST_REMOTEPROC)+= st_remoteproc.o
+obj-$(CONFIG_ST_SLIM_REMOTEPROC) += st_slim_rproc.o
diff --git a/drivers/remoteproc/st_slim_rproc.c
b/drivers/remoteproc/st_slim_rproc.c
new file mode 100644
index 000..f4bf2d7
--- /dev/null
+++ b/drivers/remoteproc/st_slim_rproc.c
@@ -0,0 +1,364 @@
+/*
+ * st_slim_rproc.c
+ *
+ * Copyright (C) 2016 STMicroelectronics
+ * Author: Peter Griffin
+ * License terms: GNU General Public License (GPL), version 2
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "remoteproc_internal.h"
+
+/* slimcore registers */
+#define SLIM_ID_OFST 0x0
+#define SLIM_VER_OFST 0x4
+
+#define SLIM_EN_OFST 0x8
+#define SLIM_EN_RUNBIT(0)
+
+#define SLIM_CLK_GATE_OFST 0xC
+#define SLIM_CLK_GATE_DIS BIT(0)
+#define SLIM_CLK_GATE_RESETBIT(2)
+
+#define SLIM_SLIM_PC_OFST 0x20
+
+/* dmem registers */
+#define SLIM_REV_ID_OFST 0x0
+#define SLIM_REV_ID_MIN_MASK GENMASK(15, 8)
+#define SLIM_REV_ID_MIN(id)((id & SLIM_REV_ID_MIN_MASK) >> 8)
+#define SLIM_REV_ID_MAJ_MASK GENMASK(23, 16)
+#define SLIM_REV_ID_MAJ(id)((id & SLIM_REV_ID_MAJ_MASK) >> 16)
+
+
+/* peripherals registers */
+#define SLIM_STBUS_SYNC_OFST 0xF88
+#define SLIM_STBUS_SYNC_DISBIT(0)
+
+#define SLIM_INT_SET_OFST 0xFD4
+#define SLIM_INT_CLR_OFST 0xFD8
+#define SLIM_INT_MASK_OFST 0xFDC
+
+#define SLIM_CMD_CLR_OFST 0xFC8
+#define SLIM_CMD_MASK_OFST 0xFCC
+
+static const char *mem_names[ST_SLIM_MEM_MAX] = {
+ [ST_SLIM_DMEM] = "dmem",
+ [ST_SLIM_IMEM] = "imem",
+};
+
+static int slim_clk_get(struct st_slim_rproc *slim_rproc, struct device *dev)
+{
+ int clk, err;
+
+ for (clk = 0; clk < ST_SLIM_MAX_CLK; clk++) {
+ slim_rproc->clks[clk] = of_clk_get(dev->of_node, clk);
+ if (IS_ERR(slim_rproc->clks[clk])) {
+ err = PTR_ERR(slim_rproc->clks[clk]);
+ if (err == -EPROBE_DEFER)
+ goto err_put_clks;
+ slim_rproc->clks[clk] = NULL;
+ break;
+ }
+ }
+
+ return 0;
+
+err_put_clks:
+ while (--clk >= 0)
+ clk_put(slim_rproc->clks[clk]);
+
+ return err;
+}
+
+static void slim_clk_disable(struct st_slim_rproc *slim_rproc)
+{
+ int clk;
+
+ for (clk = 0; clk < ST_SLIM_MAX_CLK && slim_rproc->clks[clk]; clk++)
+
