From: Bartosz Golaszewski <[email protected]>
This series converts the only user of the handcoded, mach-specific reset
routines in the davinci platform to using the reset framework.
Patches 1-3 add necessary lookups/DT-properties.
Patches 4-6 fix issues found in the remoteproc davinci driver.
Patch 7 converts the davinci-rproc driver to the reset framework.
Patch 8 removes now dead code.
Philipp: it turned out that it's indeed better to use the reset
controller's device name for the entry lookup.
Tested both in DT and legacy modes by booting the examples from
ti-ipc-rtos recipe in meta-ti.
This series applies on top of David Lechner's common-clk-v9 branch[1]
with Philipp Zabel's reset/next branch[2] pulled in.
It can be found in my github tree as well[3].
[1] git://github.com/dlech/ev3dev-kernel.git common-clk-v9
[2] git://git.pengutronix.de/git/pza/linux reset/next
[3] [email protected]:brgl/linux.git topic/davinci-reset
v1 -> v2:
- fixed the device tree patches the descriptions of which were mixed up
- return -EPROBE_DEFER from davinci-rproc's probe() if we can't get the
reset provider, since it's possible that the lookup table was not yet
registered
- made the local variable naming consistent in the davinci-rproc driver
- fixed a typo in PATCH 5/8
v2 -> v3:
- reworded the commit messages (s/remoteproc: da8xx/remoteproc\/davinci/)
- call clk_disable_unprepare() if reset_control_deassert() fails in
da8xx_rproc_start()
- added a patch fixing the S_IRUGO checkpatch warning, since we're
already modifying this driver anyway
- added a temp variable for code brevity in da8xx_rproc_stop()
- removed patch 1/8 (already applied to reset/next)
Bartosz Golaszewski (8):
ARM: davinci: dts: make psc0 a reset provider
ARM: davinci: dts: add a reset control to the dsp node
clk: davinci: add a reset lookup table for psc0
remoteproc/davinci: add the missing retval check for clk_enable()
remoteproc/davinci: prepare and unprepare the clock where needed
remoteproc/davinci: use octal permissions for module_param()
remoteproc/davinci: use the reset framework
clk: davinci: kill davinci_clk_reset_assert/deassert()
arch/arm/boot/dts/da850.dtsi | 2 ++
arch/arm/mach-davinci/include/mach/clock.h | 21 --------------
drivers/clk/davinci/psc-da850.c | 7 +++++
drivers/clk/davinci/psc.c | 19 +------------
drivers/remoteproc/da8xx_remoteproc.c | 45 ++++++++++++++++++++++++------
5 files changed, 47 insertions(+), 47 deletions(-)
delete mode 100644 arch/arm/mach-davinci/include/mach/clock.h
--
2.16.1
From: Bartosz Golaszewski <[email protected]>
In order to be able to use the reset framework in legacy boot mode as
well, add the reset lookup table to the psc driver for da850 variant.
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
drivers/clk/davinci/psc-da850.c | 7 +++++++
drivers/clk/davinci/psc.c | 1 +
2 files changed, 8 insertions(+)
diff --git a/drivers/clk/davinci/psc-da850.c b/drivers/clk/davinci/psc-da850.c
index ccc7eb17bf3a..d196dcbed560 100644
--- a/drivers/clk/davinci/psc-da850.c
+++ b/drivers/clk/davinci/psc-da850.c
@@ -6,6 +6,7 @@
*/
#include <linux/clk-provider.h>
+#include <linux/reset-controller.h>
#include <linux/clk.h>
#include <linux/clkdev.h>
#include <linux/init.h>
@@ -66,8 +67,14 @@ LPSC_CLKDEV3(ecap_clkdev, "fck", "ecap.0",
"fck", "ecap.1",
"fck", "ecap.2");
+static struct reset_control_lookup da850_psc0_reset_lookup_table[] = {
+ RESET_LOOKUP("da850-psc0", 15, "davinci-rproc.0", NULL),
+};
+
static int da850_psc0_init(struct device *dev, void __iomem *base)
{
+ reset_controller_add_lookup(da850_psc0_reset_lookup_table,
+ ARRAY_SIZE(da850_psc0_reset_lookup_table));
return davinci_psc_register_clocks(dev, da850_psc0_info, 16, base);
}
diff --git a/drivers/clk/davinci/psc.c b/drivers/clk/davinci/psc.c
index 3b0e59dfbdd7..063df62381ea 100644
--- a/drivers/clk/davinci/psc.c
+++ b/drivers/clk/davinci/psc.c
@@ -425,6 +425,7 @@ __davinci_psc_register_clocks(struct device *dev,
psc->rcdev.ops = &davinci_psc_reset_ops;
psc->rcdev.owner = THIS_MODULE;
+ psc->rcdev.dev = dev;
psc->rcdev.of_node = dev->of_node;
psc->rcdev.of_reset_n_cells = 1;
psc->rcdev.of_xlate = davinci_psc_reset_of_xlate;
--
2.16.1
From: Bartosz Golaszewski <[email protected]>
Switch to using the reset framework instead of handcoded reset routines
we used so far.
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
drivers/remoteproc/da8xx_remoteproc.c | 33 ++++++++++++++++++++++++++++-----
1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c
index b668e32996e2..788f59809c02 100644
--- a/drivers/remoteproc/da8xx_remoteproc.c
+++ b/drivers/remoteproc/da8xx_remoteproc.c
@@ -10,6 +10,7 @@
#include <linux/bitops.h>
#include <linux/clk.h>
+#include <linux/reset.h>
#include <linux/err.h>
#include <linux/interrupt.h>
#include <linux/io.h>
@@ -20,8 +21,6 @@
#include <linux/platform_device.h>
#include <linux/remoteproc.h>
-#include <mach/clock.h> /* for davinci_clk_reset_assert/deassert() */
-
#include "remoteproc_internal.h"
static char *da8xx_fw_name;
@@ -72,6 +71,7 @@ struct da8xx_rproc {
struct da8xx_rproc_mem *mem;
int num_mems;
struct clk *dsp_clk;
+ struct reset_control *dsp_reset;
void (*ack_fxn)(struct irq_data *data);
struct irq_data *irq_data;
void __iomem *chipsig;
@@ -138,6 +138,7 @@ static int da8xx_rproc_start(struct rproc *rproc)
struct device *dev = rproc->dev.parent;
struct da8xx_rproc *drproc = (struct da8xx_rproc *)rproc->priv;
struct clk *dsp_clk = drproc->dsp_clk;
+ struct reset_control *dsp_reset = drproc->dsp_reset;
int ret;
/* hw requires the start (boot) address be on 1KB boundary */
@@ -155,7 +156,12 @@ static int da8xx_rproc_start(struct rproc *rproc)
return ret;
}
- davinci_clk_reset_deassert(dsp_clk);
+ ret = reset_control_deassert(dsp_reset);
+ if (ret) {
+ dev_err(dev, "reset_control_deassert() failed: %d\n", ret);
+ clk_disable_unprepare(dsp_clk);
+ return ret;
+ }
return 0;
}
@@ -163,8 +169,15 @@ static int da8xx_rproc_start(struct rproc *rproc)
static int da8xx_rproc_stop(struct rproc *rproc)
{
struct da8xx_rproc *drproc = rproc->priv;
+ struct device *dev = rproc->dev.parent;
+ int ret;
+
+ ret = reset_control_assert(drproc->dsp_reset);
+ if (ret) {
+ dev_err(dev, "reset_control_assert() failed: %d\n", ret);
+ return ret;
+ }
- davinci_clk_reset_assert(drproc->dsp_clk);
clk_disable_unprepare(drproc->dsp_clk);
return 0;
@@ -232,6 +245,7 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
struct resource *bootreg_res;
struct resource *chipsig_res;
struct clk *dsp_clk;
+ struct reset_control *dsp_reset;
void __iomem *chipsig;
void __iomem *bootreg;
int irq;
@@ -268,6 +282,15 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
return PTR_ERR(dsp_clk);
}
+ dsp_reset = devm_reset_control_get_exclusive(dev, NULL);
+ if (IS_ERR(dsp_reset)) {
+ if (PTR_ERR(dsp_reset) != -EPROBE_DEFER)
+ dev_err(dev, "unable to get reset control: %ld\n",
+ PTR_ERR(dsp_reset));
+
+ return PTR_ERR(dsp_reset);
+ }
+
if (dev->of_node) {
ret = of_reserved_mem_device_init(dev);
if (ret) {
@@ -309,7 +332,7 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
* *not* in reset, but da8xx_rproc_start() needs the DSP to be
* held in reset at the time it is called.
*/
- ret = davinci_clk_reset_assert(drproc->dsp_clk);
+ ret = reset_control_assert(dsp_reset);
if (ret)
goto free_rproc;
--
2.16.1
From: Bartosz Golaszewski <[email protected]>
This code is no longer used. Remove it.
Signed-off-by: Bartosz Golaszewski <[email protected]>
Reviewed-by: David Lechner <[email protected]>
---
arch/arm/mach-davinci/include/mach/clock.h | 21 ---------------------
drivers/clk/davinci/psc.c | 18 ------------------
2 files changed, 39 deletions(-)
delete mode 100644 arch/arm/mach-davinci/include/mach/clock.h
diff --git a/arch/arm/mach-davinci/include/mach/clock.h b/arch/arm/mach-davinci/include/mach/clock.h
deleted file mode 100644
index 42ed4f2f5ce4..000000000000
--- a/arch/arm/mach-davinci/include/mach/clock.h
+++ /dev/null
@@ -1,21 +0,0 @@
-/*
- * arch/arm/mach-davinci/include/mach/clock.h
- *
- * Clock control driver for DaVinci - header file
- *
- * Authors: Vladimir Barinov <[email protected]>
- *
- * 2007 (c) MontaVista Software, Inc. This file is licensed under
- * the terms of the GNU General Public License version 2. This program
- * is licensed "as is" without any warranty of any kind, whether express
- * or implied.
- */
-#ifndef __ASM_ARCH_DAVINCI_CLOCK_H
-#define __ASM_ARCH_DAVINCI_CLOCK_H
-
-struct clk;
-
-int davinci_clk_reset_assert(struct clk *c);
-int davinci_clk_reset_deassert(struct clk *c);
-
-#endif
diff --git a/drivers/clk/davinci/psc.c b/drivers/clk/davinci/psc.c
index 063df62381ea..75c4bc6f23e3 100644
--- a/drivers/clk/davinci/psc.c
+++ b/drivers/clk/davinci/psc.c
@@ -295,24 +295,6 @@ static int davinci_lpsc_clk_reset(struct clk *clk, bool reset)
return 0;
}
-/*
- * REVISIT: These exported functions can be removed after a non-DT lookup is
- * added to the reset controller framework and the davinci-rproc driver is
- * updated to use the generic reset controller framework.
- */
-
-int davinci_clk_reset_assert(struct clk *clk)
-{
- return davinci_lpsc_clk_reset(clk, true);
-}
-EXPORT_SYMBOL(davinci_clk_reset_assert);
-
-int davinci_clk_reset_deassert(struct clk *clk)
-{
- return davinci_lpsc_clk_reset(clk, false);
-}
-EXPORT_SYMBOL(davinci_clk_reset_deassert);
-
static int davinci_psc_reset_assert(struct reset_controller_dev *rcdev,
unsigned long id)
{
--
2.16.1
From: Bartosz Golaszewski <[email protected]>
The psc driver registers with the reset framework as a provider. Add
the #reset-cells property to the psc0 node.
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
arch/arm/boot/dts/da850.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index a3b9a99b43ca..dad64dbf1f23 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -123,6 +123,7 @@
compatible = "ti,da850-psc0";
reg = <0x10000 0x1000>;
#clock-cells = <1>;
+ #reset-cells = <1>;
#power-domain-cells = <1>;
clocks = <&pll0_sysclk 1>, <&pll0_sysclk 2>,
<&pll0_sysclk 4>, <&pll0_sysclk 6>,
--
2.16.1
From: Bartosz Golaszewski <[email protected]>
We're currently switching the platform to using the common clock
framework. We need to explicitly prepare and unprepare the rproc
clock.
Signed-off-by: Bartosz Golaszewski <[email protected]>
Acked-by: Suman Anna <[email protected]>
---
drivers/remoteproc/da8xx_remoteproc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c
index 2b24291337b7..f134192922e0 100644
--- a/drivers/remoteproc/da8xx_remoteproc.c
+++ b/drivers/remoteproc/da8xx_remoteproc.c
@@ -149,9 +149,9 @@ static int da8xx_rproc_start(struct rproc *rproc)
writel(rproc->bootaddr, drproc->bootreg);
- ret = clk_enable(dsp_clk);
+ ret = clk_prepare_enable(dsp_clk);
if (ret) {
- dev_err(dev, "clk_enable() failed: %d\n", ret);
+ dev_err(dev, "clk_prepare_enable() failed: %d\n", ret);
return ret;
}
@@ -165,7 +165,7 @@ static int da8xx_rproc_stop(struct rproc *rproc)
struct da8xx_rproc *drproc = rproc->priv;
davinci_clk_reset_assert(drproc->dsp_clk);
- clk_disable(drproc->dsp_clk);
+ clk_disable_unprepare(drproc->dsp_clk);
return 0;
}
--
2.16.1
From: Bartosz Golaszewski <[email protected]>
The davinci platform is being switched to using the common clock
framework, where clk_enable() can fail. Add the return value check.
Signed-off-by: Bartosz Golaszewski <[email protected]>
Acked-by: Suman Anna <[email protected]>
---
drivers/remoteproc/da8xx_remoteproc.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c
index bf3b9034c319..2b24291337b7 100644
--- a/drivers/remoteproc/da8xx_remoteproc.c
+++ b/drivers/remoteproc/da8xx_remoteproc.c
@@ -138,6 +138,7 @@ static int da8xx_rproc_start(struct rproc *rproc)
struct device *dev = rproc->dev.parent;
struct da8xx_rproc *drproc = (struct da8xx_rproc *)rproc->priv;
struct clk *dsp_clk = drproc->dsp_clk;
+ int ret;
/* hw requires the start (boot) address be on 1KB boundary */
if (rproc->bootaddr & 0x3ff) {
@@ -148,7 +149,12 @@ static int da8xx_rproc_start(struct rproc *rproc)
writel(rproc->bootaddr, drproc->bootreg);
- clk_enable(dsp_clk);
+ ret = clk_enable(dsp_clk);
+ if (ret) {
+ dev_err(dev, "clk_enable() failed: %d\n", ret);
+ return ret;
+ }
+
davinci_clk_reset_deassert(dsp_clk);
return 0;
--
2.16.1
From: Bartosz Golaszewski <[email protected]>
Checkpatch recommends to use octal perms instead of S_IRUGO.
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
drivers/remoteproc/da8xx_remoteproc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c
index f134192922e0..b668e32996e2 100644
--- a/drivers/remoteproc/da8xx_remoteproc.c
+++ b/drivers/remoteproc/da8xx_remoteproc.c
@@ -25,7 +25,7 @@
#include "remoteproc_internal.h"
static char *da8xx_fw_name;
-module_param(da8xx_fw_name, charp, S_IRUGO);
+module_param(da8xx_fw_name, charp, 0444);
MODULE_PARM_DESC(da8xx_fw_name,
"Name of DSP firmware file in /lib/firmware (if not specified defaults to 'rproc-dsp-fw')");
--
2.16.1
From: Bartosz Golaszewski <[email protected]>
The davinci-rproc driver will soon use the reset framework. Add the
resets property to the dsp node in da850.dtsi.
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
arch/arm/boot/dts/da850.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index dad64dbf1f23..5b485e44b83e 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -109,6 +109,7 @@
interrupt-parent = <&intc>;
interrupts = <28>;
clocks = <&psc0 15>;
+ resets = <&psc0 15>;
status = "disabled";
};
soc@1c00000 {
--
2.16.1
On Tue, 2018-03-27 at 11:20 +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> This series converts the only user of the handcoded, mach-specific reset
> routines in the davinci platform to using the reset framework.
>
> Patches 1-3 add necessary lookups/DT-properties.
>
> Patches 4-6 fix issues found in the remoteproc davinci driver.
>
> Patch 7 converts the davinci-rproc driver to the reset framework.
>
> Patch 8 removes now dead code.
>
> Philipp: it turned out that it's indeed better to use the reset
> controller's device name for the entry lookup.
>
> Tested both in DT and legacy modes by booting the examples from
> ti-ipc-rtos recipe in meta-ti.
>
> This series applies on top of David Lechner's common-clk-v9 branch[1]
> with Philipp Zabel's reset/next branch[2] pulled in.
>
> It can be found in my github tree as well[3].
>
> [1] git://github.com/dlech/ev3dev-kernel.git common-clk-v9
> [2] git://git.pengutronix.de/git/pza/linux reset/next
I have moved the relevant patches onto a separate, immutable branch,
which is merged into reset/next:
git://git.pengutronix.de/git/pza/linux reset/lookup
regards
Philipp
Wasn't there a v4 already? Is this really v5 instead of v3?
On 03/27/2018 04:20 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> This series converts the only user of the handcoded, mach-specific reset
> routines in the davinci platform to using the reset framework.
>
> Patches 1-3 add necessary lookups/DT-properties.
Should I just squash the DT changes into my common-clock-v9 branch
since those patches haven't been picked up yet anyway? That will be
2 fewer patches to keep track of.
On 03/27/2018 04:20 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> Checkpatch recommends to use octal perms instead of S_IRUGO.
>
> Signed-off-by: Bartosz Golaszewski <[email protected]>
Acked-by: Suman Anna <[email protected]>
> ---
> drivers/remoteproc/da8xx_remoteproc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c
> index f134192922e0..b668e32996e2 100644
> --- a/drivers/remoteproc/da8xx_remoteproc.c
> +++ b/drivers/remoteproc/da8xx_remoteproc.c
> @@ -25,7 +25,7 @@
> #include "remoteproc_internal.h"
>
> static char *da8xx_fw_name;
> -module_param(da8xx_fw_name, charp, S_IRUGO);
> +module_param(da8xx_fw_name, charp, 0444);
> MODULE_PARM_DESC(da8xx_fw_name,
> "Name of DSP firmware file in /lib/firmware (if not specified defaults to 'rproc-dsp-fw')");
>
>
Hi Bjorn,
On 03/27/2018 04:20 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> This series converts the only user of the handcoded, mach-specific reset
> routines in the davinci platform to using the reset framework.
>
> Patches 1-3 add necessary lookups/DT-properties.
>
> Patches 4-6 fix issues found in the remoteproc davinci driver.
Can you pick up patches 4 through 6 for 4.17 itself through the
remoteproc tree? They can be picked up independent of the CCF
conversion. Anyway, the actual CCF conversion and the reset framework
adaptation patch would need the dts patches to be picked up first to
maintain functionality, and I do not think Sekhar is picking up the dts
changes for 4.17 this late.
regards
Suman
>
> Patch 7 converts the davinci-rproc driver to the reset framework.
>
> Patch 8 removes now dead code.
>
> Philipp: it turned out that it's indeed better to use the reset
> controller's device name for the entry lookup.
>
> Tested both in DT and legacy modes by booting the examples from
> ti-ipc-rtos recipe in meta-ti.
>
> This series applies on top of David Lechner's common-clk-v9 branch[1]
> with Philipp Zabel's reset/next branch[2] pulled in.
>
> It can be found in my github tree as well[3].
>
> [1] git://github.com/dlech/ev3dev-kernel.git common-clk-v9
> [2] git://git.pengutronix.de/git/pza/linux reset/next
> [3] [email protected]:brgl/linux.git topic/davinci-reset
>
> v1 -> v2:
> - fixed the device tree patches the descriptions of which were mixed up
> - return -EPROBE_DEFER from davinci-rproc's probe() if we can't get the
> reset provider, since it's possible that the lookup table was not yet
> registered
> - made the local variable naming consistent in the davinci-rproc driver
> - fixed a typo in PATCH 5/8
>
> v2 -> v3:
> - reworded the commit messages (s/remoteproc: da8xx/remoteproc\/davinci/)
> - call clk_disable_unprepare() if reset_control_deassert() fails in
> da8xx_rproc_start()
> - added a patch fixing the S_IRUGO checkpatch warning, since we're
> already modifying this driver anyway
> - added a temp variable for code brevity in da8xx_rproc_stop()
> - removed patch 1/8 (already applied to reset/next)
>
> Bartosz Golaszewski (8):
> ARM: davinci: dts: make psc0 a reset provider
> ARM: davinci: dts: add a reset control to the dsp node
> clk: davinci: add a reset lookup table for psc0
> remoteproc/davinci: add the missing retval check for clk_enable()
> remoteproc/davinci: prepare and unprepare the clock where needed
> remoteproc/davinci: use octal permissions for module_param()
> remoteproc/davinci: use the reset framework
> clk: davinci: kill davinci_clk_reset_assert/deassert()
>
> arch/arm/boot/dts/da850.dtsi | 2 ++
> arch/arm/mach-davinci/include/mach/clock.h | 21 --------------
> drivers/clk/davinci/psc-da850.c | 7 +++++
> drivers/clk/davinci/psc.c | 19 +------------
> drivers/remoteproc/da8xx_remoteproc.c | 45 ++++++++++++++++++++++++------
> 5 files changed, 47 insertions(+), 47 deletions(-)
> delete mode 100644 arch/arm/mach-davinci/include/mach/clock.h
>
Hi Bart,
On 03/27/2018 04:20 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> Switch to using the reset framework instead of handcoded reset routines
> we used so far.
>
> Signed-off-by: Bartosz Golaszewski <[email protected]>
> ---
> drivers/remoteproc/da8xx_remoteproc.c | 33 ++++++++++++++++++++++++++++-----
> 1 file changed, 28 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c
> index b668e32996e2..788f59809c02 100644
> --- a/drivers/remoteproc/da8xx_remoteproc.c
> +++ b/drivers/remoteproc/da8xx_remoteproc.c
> @@ -10,6 +10,7 @@
>
> #include <linux/bitops.h>
> #include <linux/clk.h>
> +#include <linux/reset.h>
> #include <linux/err.h>
> #include <linux/interrupt.h>
> #include <linux/io.h>
> @@ -20,8 +21,6 @@
> #include <linux/platform_device.h>
> #include <linux/remoteproc.h>
>
> -#include <mach/clock.h> /* for davinci_clk_reset_assert/deassert() */
> -
> #include "remoteproc_internal.h"
>
> static char *da8xx_fw_name;
> @@ -72,6 +71,7 @@ struct da8xx_rproc {
> struct da8xx_rproc_mem *mem;
> int num_mems;
> struct clk *dsp_clk;
> + struct reset_control *dsp_reset;
> void (*ack_fxn)(struct irq_data *data);
> struct irq_data *irq_data;
> void __iomem *chipsig;
> @@ -138,6 +138,7 @@ static int da8xx_rproc_start(struct rproc *rproc)
> struct device *dev = rproc->dev.parent;
> struct da8xx_rproc *drproc = (struct da8xx_rproc *)rproc->priv;
> struct clk *dsp_clk = drproc->dsp_clk;
> + struct reset_control *dsp_reset = drproc->dsp_reset;
> int ret;
>
> /* hw requires the start (boot) address be on 1KB boundary */
> @@ -155,7 +156,12 @@ static int da8xx_rproc_start(struct rproc *rproc)
> return ret;
> }
>
> - davinci_clk_reset_deassert(dsp_clk);
> + ret = reset_control_deassert(dsp_reset);
> + if (ret) {
> + dev_err(dev, "reset_control_deassert() failed: %d\n", ret);
> + clk_disable_unprepare(dsp_clk);
> + return ret;
> + }
>
> return 0;
> }
> @@ -163,8 +169,15 @@ static int da8xx_rproc_start(struct rproc *rproc)
> static int da8xx_rproc_stop(struct rproc *rproc)
> {
> struct da8xx_rproc *drproc = rproc->priv;
> + struct device *dev = rproc->dev.parent;
> + int ret;
> +
> + ret = reset_control_assert(drproc->dsp_reset);
> + if (ret) {
> + dev_err(dev, "reset_control_assert() failed: %d\n", ret);
> + return ret;
> + }
>
> - davinci_clk_reset_assert(drproc->dsp_clk);
> clk_disable_unprepare(drproc->dsp_clk);
>
> return 0;
> @@ -232,6 +245,7 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
> struct resource *bootreg_res;
> struct resource *chipsig_res;
> struct clk *dsp_clk;
> + struct reset_control *dsp_reset;
> void __iomem *chipsig;
> void __iomem *bootreg;
> int irq;
> @@ -268,6 +282,15 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
> return PTR_ERR(dsp_clk);
> }
>
> + dsp_reset = devm_reset_control_get_exclusive(dev, NULL);
> + if (IS_ERR(dsp_reset)) {
> + if (PTR_ERR(dsp_reset) != -EPROBE_DEFER)
> + dev_err(dev, "unable to get reset control: %ld\n",
> + PTR_ERR(dsp_reset));
> +
> + return PTR_ERR(dsp_reset);
> + }
> +
> if (dev->of_node) {
> ret = of_reserved_mem_device_init(dev);
> if (ret) {
> @@ -309,7 +332,7 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
> * *not* in reset, but da8xx_rproc_start() needs the DSP to be
> * held in reset at the time it is called.
> */
> - ret = davinci_clk_reset_assert(drproc->dsp_clk);
> + ret = reset_control_assert(dsp_reset);
> if (ret)
> goto free_rproc;
>
Tested your previous branch, need one more change in this patch to have
the remoteproc boot be actually executing some code. The acquired
dsp_reset is not stored in the drproc, so the start and stop were not
effective. The issue was masked because reset_control_assert() and
deassert() return 0 when a NULL pointer is passed in.
--- a/drivers/remoteproc/da8xx_remoteproc.c
+++ b/drivers/remoteproc/da8xx_remoteproc.c
@@ -311,6 +311,7 @@ static int da8xx_rproc_probe(struct platform_device
*pdev)
drproc = rproc->priv;
drproc->rproc = rproc;
drproc->dsp_clk = dsp_clk;
+ drproc->dsp_reset = dsp_reset;
rproc->has_iommu = false;
regards
Suman
2018-03-29 0:30 GMT+02:00 Suman Anna <[email protected]>:
> Hi Bart,
>
> On 03/27/2018 04:20 AM, Bartosz Golaszewski wrote:
>> From: Bartosz Golaszewski <[email protected]>
>>
>> Switch to using the reset framework instead of handcoded reset routines
>> we used so far.
>>
>> Signed-off-by: Bartosz Golaszewski <[email protected]>
>> ---
>> drivers/remoteproc/da8xx_remoteproc.c | 33 ++++++++++++++++++++++++++++-----
>> 1 file changed, 28 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c
>> index b668e32996e2..788f59809c02 100644
>> --- a/drivers/remoteproc/da8xx_remoteproc.c
>> +++ b/drivers/remoteproc/da8xx_remoteproc.c
>> @@ -10,6 +10,7 @@
>>
>> #include <linux/bitops.h>
>> #include <linux/clk.h>
>> +#include <linux/reset.h>
>> #include <linux/err.h>
>> #include <linux/interrupt.h>
>> #include <linux/io.h>
>> @@ -20,8 +21,6 @@
>> #include <linux/platform_device.h>
>> #include <linux/remoteproc.h>
>>
>> -#include <mach/clock.h> /* for davinci_clk_reset_assert/deassert() */
>> -
>> #include "remoteproc_internal.h"
>>
>> static char *da8xx_fw_name;
>> @@ -72,6 +71,7 @@ struct da8xx_rproc {
>> struct da8xx_rproc_mem *mem;
>> int num_mems;
>> struct clk *dsp_clk;
>> + struct reset_control *dsp_reset;
>> void (*ack_fxn)(struct irq_data *data);
>> struct irq_data *irq_data;
>> void __iomem *chipsig;
>> @@ -138,6 +138,7 @@ static int da8xx_rproc_start(struct rproc *rproc)
>> struct device *dev = rproc->dev.parent;
>> struct da8xx_rproc *drproc = (struct da8xx_rproc *)rproc->priv;
>> struct clk *dsp_clk = drproc->dsp_clk;
>> + struct reset_control *dsp_reset = drproc->dsp_reset;
>> int ret;
>>
>> /* hw requires the start (boot) address be on 1KB boundary */
>> @@ -155,7 +156,12 @@ static int da8xx_rproc_start(struct rproc *rproc)
>> return ret;
>> }
>>
>> - davinci_clk_reset_deassert(dsp_clk);
>> + ret = reset_control_deassert(dsp_reset);
>> + if (ret) {
>> + dev_err(dev, "reset_control_deassert() failed: %d\n", ret);
>> + clk_disable_unprepare(dsp_clk);
>> + return ret;
>> + }
>>
>> return 0;
>> }
>> @@ -163,8 +169,15 @@ static int da8xx_rproc_start(struct rproc *rproc)
>> static int da8xx_rproc_stop(struct rproc *rproc)
>> {
>> struct da8xx_rproc *drproc = rproc->priv;
>> + struct device *dev = rproc->dev.parent;
>> + int ret;
>> +
>> + ret = reset_control_assert(drproc->dsp_reset);
>> + if (ret) {
>> + dev_err(dev, "reset_control_assert() failed: %d\n", ret);
>> + return ret;
>> + }
>>
>> - davinci_clk_reset_assert(drproc->dsp_clk);
>> clk_disable_unprepare(drproc->dsp_clk);
>>
>> return 0;
>> @@ -232,6 +245,7 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
>> struct resource *bootreg_res;
>> struct resource *chipsig_res;
>> struct clk *dsp_clk;
>> + struct reset_control *dsp_reset;
>> void __iomem *chipsig;
>> void __iomem *bootreg;
>> int irq;
>> @@ -268,6 +282,15 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
>> return PTR_ERR(dsp_clk);
>> }
>>
>> + dsp_reset = devm_reset_control_get_exclusive(dev, NULL);
>> + if (IS_ERR(dsp_reset)) {
>> + if (PTR_ERR(dsp_reset) != -EPROBE_DEFER)
>> + dev_err(dev, "unable to get reset control: %ld\n",
>> + PTR_ERR(dsp_reset));
>> +
>> + return PTR_ERR(dsp_reset);
>> + }
>> +
>> if (dev->of_node) {
>> ret = of_reserved_mem_device_init(dev);
>> if (ret) {
>> @@ -309,7 +332,7 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
>> * *not* in reset, but da8xx_rproc_start() needs the DSP to be
>> * held in reset at the time it is called.
>> */
>> - ret = davinci_clk_reset_assert(drproc->dsp_clk);
>> + ret = reset_control_assert(dsp_reset);
>> if (ret)
>> goto free_rproc;
>>
>
> Tested your previous branch, need one more change in this patch to have
> the remoteproc boot be actually executing some code. The acquired
> dsp_reset is not stored in the drproc, so the start and stop were not
> effective. The issue was masked because reset_control_assert() and
> deassert() return 0 when a NULL pointer is passed in.
>
> --- a/drivers/remoteproc/da8xx_remoteproc.c
> +++ b/drivers/remoteproc/da8xx_remoteproc.c
> @@ -311,6 +311,7 @@ static int da8xx_rproc_probe(struct platform_device
> *pdev)
> drproc = rproc->priv;
> drproc->rproc = rproc;
> drproc->dsp_clk = dsp_clk;
> + drproc->dsp_reset = dsp_reset;
> rproc->has_iommu = false;
>
> regards
> Suman
>
Oh snap, thanks for spotting it.
I'll just resend last two patches, since the previous six will be
picked up at different places.
Thanks,
Bartosz
2018-03-27 18:48 GMT+02:00 David Lechner <[email protected]>:
> Wasn't there a v4 already? Is this really v5 instead of v3?
>
Yes there was and Philipp applied the v4 alright.
>
> On 03/27/2018 04:20 AM, Bartosz Golaszewski wrote:
>>
>> From: Bartosz Golaszewski <[email protected]>
>>
>> This series converts the only user of the handcoded, mach-specific reset
>> routines in the davinci platform to using the reset framework.
>>
>> Patches 1-3 add necessary lookups/DT-properties.
>
>
> Should I just squash the DT changes into my common-clock-v9 branch
> since those patches haven't been picked up yet anyway? That will be
> 2 fewer patches to keep track of.
I would personally prefer to keep the changesets thematically
separate. The clock series is big anyway already.
Thanks,
Bartosz