2018-06-21 07:39:15

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v7 0/4] ARM: davinci: complete the conversion to using the reset framework

From: Bartosz Golaszewski <[email protected]>

These are the remaining patches that still need to be merged in order
to complete the conversion of the davinci dsp driver to using the reset
framework.

They apply on top of v4.18-rc1 with David Lechner's remaining patches
merged.

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:
- modify PATCH 1/8: drop the provider argument from the function adding
lookup entries and instead pass the provider name to the RESET_LOOKUP
macro, return -EPROBE_DEFER if we locate a correct lookup entry but
cannot get the corresponding reset controller
- modify the reset lookup entry in psc-da850
- don't manually return -EPROBE_DEFER from davinci-rproc, instead don't
emit an error message if devm_reset_control_get_exclusive() returns
this error code

v3 -> v4:
- make index the second parameter in RESET_LOOKUP() (right after the
provider name)

v4 -> v5:
- fix a bug where the dsp_reset object correctly stored in drproc struct

v5 -> v6:
- rebased on top of v4.17-rc1 and retested
- dropped patches that were applied during 4.17 merge window
- added relevant review and ack tags

v6 -> v7:
- rebased on top of v4.18-rc1 and dropped patches that were applied for
v4.17

Bartosz Golaszewski (4):
remoteproc/davinci: use the reset framework
clk: davinci: kill davinci_clk_reset_assert/deassert()
ARM: davinci: dts: make psc0 a reset provider
ARM: davinci: dts: add a reset control to the dsp node

arch/arm/boot/dts/da850.dtsi | 2 ++
arch/arm/mach-davinci/include/mach/clock.h | 21 -------------
drivers/clk/davinci/psc.c | 18 ------------
drivers/remoteproc/da8xx_remoteproc.c | 34 ++++++++++++++++++----
4 files changed, 31 insertions(+), 44 deletions(-)
delete mode 100644 arch/arm/mach-davinci/include/mach/clock.h

--
2.17.1



2018-06-21 07:38:29

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v7 3/4] ARM: davinci: dts: make psc0 a reset provider

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]>
Reviewed-by: David Lechner <[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 3fc8b8fd816e..1bc0e4bc0697 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -132,6 +132,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.17.1


2018-06-21 07:38:46

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v7 2/4] clk: davinci: kill davinci_clk_reset_assert/deassert()

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]>
Acked-by: Stephen Boyd <[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 fffbed5e263b..5b69e24a224f 100644
--- a/drivers/clk/davinci/psc.c
+++ b/drivers/clk/davinci/psc.c
@@ -303,24 +303,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.17.1


2018-06-21 07:39:01

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v7 4/4] ARM: davinci: dts: add a reset control to the dsp node

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]>
Reviewed-by: David Lechner <[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 1bc0e4bc0697..225e0cf4b7bc 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -118,6 +118,7 @@
interrupt-parent = <&intc>;
interrupts = <28>;
clocks = <&psc0 15>;
+ resets = <&psc0 15>;
status = "disabled";
};
soc@1c00000 {
--
2.17.1


2018-06-21 07:40:44

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v7 1/4] remoteproc/davinci: use the reset framework

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]>
Reviewed-by: Sekhar Nori <[email protected]>
Reviewed-by: Philipp Zabel <[email protected]>
---
drivers/remoteproc/da8xx_remoteproc.c | 34 +++++++++++++++++++++++----
1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c
index b668e32996e2..76c06b70a1c6 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) {
@@ -287,6 +310,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;

ret = da8xx_rproc_get_internal_memories(pdev, drproc);
@@ -309,7 +333,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.17.1


2018-06-21 11:06:01

by Sekhar Nori

[permalink] [raw]
Subject: Re: [PATCH v7 0/4] ARM: davinci: complete the conversion to using the reset framework

Hi Bartosz,

On Thursday 21 June 2018 01:07 PM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> These are the remaining patches that still need to be merged in order
> to complete the conversion of the davinci dsp driver to using the reset
> framework.
>
> They apply on top of v4.18-rc1 with David Lechner's remaining patches
> merged.

Series looks good to me.

To preserve bisect, shouldn't the order of applying be patch #3, #4, #1
and #2 ?

Given the dependencies and to preserve bisect its easiest if I take the
series with acks from remoteproc and clock maintainers.

Open to other suggestions as well.

Thanks,
Sekhar

2018-06-21 11:42:21

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH v7 0/4] ARM: davinci: complete the conversion to using the reset framework

2018-06-21 12:52 GMT+02:00 Sekhar Nori <[email protected]>:
> Hi Bartosz,
>
> On Thursday 21 June 2018 01:07 PM, Bartosz Golaszewski wrote:
>> From: Bartosz Golaszewski <[email protected]>
>>
>> These are the remaining patches that still need to be merged in order
>> to complete the conversion of the davinci dsp driver to using the reset
>> framework.
>>
>> They apply on top of v4.18-rc1 with David Lechner's remaining patches
>> merged.
>
> Series looks good to me.
>
> To preserve bisect, shouldn't the order of applying be patch #3, #4, #1
> and #2 ?
>
> Given the dependencies and to preserve bisect its easiest if I take the
> series with acks from remoteproc and clock maintainers.
>
> Open to other suggestions as well.
>
> Thanks,
> Sekhar

Oops you're right about the order. Do you want me to resend?

Bart

2018-07-02 12:44:55

by Sekhar Nori

[permalink] [raw]
Subject: Re: [PATCH v7 3/4] ARM: davinci: dts: make psc0 a reset provider

On Thursday 21 June 2018 01:07 PM, Bartosz Golaszewski wrote:
> 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]>
> Reviewed-by: David Lechner <[email protected]>

Applied to v4.19/dt branch of my tree.

Thanks,
Sekhar

2018-07-02 12:45:11

by Sekhar Nori

[permalink] [raw]
Subject: Re: [PATCH v7 4/4] ARM: davinci: dts: add a reset control to the dsp node

On Thursday 21 June 2018 01:07 PM, Bartosz Golaszewski wrote:
> 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]>
> Reviewed-by: David Lechner <[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 1bc0e4bc0697..225e0cf4b7bc 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -118,6 +118,7 @@
> interrupt-parent = <&intc>;
> interrupts = <28>;
> clocks = <&psc0 15>;
> + resets = <&psc0 15>;

Applied to v4.19/dt

Thanks,
Sekhar

2018-07-02 12:45:23

by Sekhar Nori

[permalink] [raw]
Subject: Re: [PATCH v7 0/4] ARM: davinci: complete the conversion to using the reset framework

Hi Bjorn,

On Thursday 21 June 2018 05:11 PM, Bartosz Golaszewski wrote:
> 2018-06-21 12:52 GMT+02:00 Sekhar Nori <[email protected]>:
>> Hi Bartosz,
>>
>> On Thursday 21 June 2018 01:07 PM, Bartosz Golaszewski wrote:
>>> From: Bartosz Golaszewski <[email protected]>
>>>
>>> These are the remaining patches that still need to be merged in order
>>> to complete the conversion of the davinci dsp driver to using the reset
>>> framework.
>>>
>>> They apply on top of v4.18-rc1 with David Lechner's remaining patches
>>> merged.
>>
>> Series looks good to me.
>>
>> To preserve bisect, shouldn't the order of applying be patch #3, #4, #1
>> and #2 ?
>>
>> Given the dependencies and to preserve bisect its easiest if I take the
>> series with acks from remoteproc and clock maintainers.
>>
>> Open to other suggestions as well.
>>
>> Thanks,
>> Sekhar
>
> Oops you're right about the order. Do you want me to resend?

With your ack, I can queue 1/4 for v4.19 and provide an immutable commit
to you (on top of v4.18-rc1) for you to merge any further changes you
want to queue from your tree.

Bartosz, given the number of moving pieces, I think its better to keep
2/4 for v4.20 release - once all other other dependencies have been merged.

Thanks,
Sekhar

2018-07-02 15:29:07

by David Lechner

[permalink] [raw]
Subject: Re: [PATCH v7 0/4] ARM: davinci: complete the conversion to using the reset framework

On 07/02/2018 07:08 AM, Sekhar Nori wrote:
> Hi Bjorn,
>
> On Thursday 21 June 2018 05:11 PM, Bartosz Golaszewski wrote:
>> 2018-06-21 12:52 GMT+02:00 Sekhar Nori <[email protected]>:
>>> Hi Bartosz,
>>>
>>> On Thursday 21 June 2018 01:07 PM, Bartosz Golaszewski wrote:
>>>> From: Bartosz Golaszewski <[email protected]>
>>>>
>>>> These are the remaining patches that still need to be merged in order
>>>> to complete the conversion of the davinci dsp driver to using the reset
>>>> framework.
>>>>
>>>> They apply on top of v4.18-rc1 with David Lechner's remaining patches
>>>> merged.
>>>
>>> Series looks good to me.
>>>
>>> To preserve bisect, shouldn't the order of applying be patch #3, #4, #1
>>> and #2 ?
>>>
>>> Given the dependencies and to preserve bisect its easiest if I take the
>>> series with acks from remoteproc and clock maintainers.
>>>
>>> Open to other suggestions as well.
>>>
>>> Thanks,
>>> Sekhar
>>
>> Oops you're right about the order. Do you want me to resend?
>
> With your ack, I can queue 1/4 for v4.19 and provide an immutable commit
> to you (on top of v4.18-rc1) for you to merge any further changes you
> want to queue from your tree.
>
> Bartosz, given the number of moving pieces, I think its better to keep
> 2/4 for v4.20 release - once all other other dependencies have been merged.

I was thinking the same thing as well. I will pick it up in a
clk-davinci-4.20 branch if that sounds OK.


2018-07-03 04:45:55

by Sekhar Nori

[permalink] [raw]
Subject: Re: [PATCH v7 0/4] ARM: davinci: complete the conversion to using the reset framework

On Monday 02 July 2018 08:57 PM, David Lechner wrote:
> On 07/02/2018 07:08 AM, Sekhar Nori wrote:
>> Hi Bjorn,
>>
>> On Thursday 21 June 2018 05:11 PM, Bartosz Golaszewski wrote:
>>> 2018-06-21 12:52 GMT+02:00 Sekhar Nori <[email protected]>:
>>>> Hi Bartosz,
>>>>
>>>> On Thursday 21 June 2018 01:07 PM, Bartosz Golaszewski wrote:
>>>>> From: Bartosz Golaszewski <[email protected]>
>>>>>
>>>>> These are the remaining patches that still need to be merged in order
>>>>> to complete the conversion of the davinci dsp driver to using the
>>>>> reset
>>>>> framework.
>>>>>
>>>>> They apply on top of v4.18-rc1 with David Lechner's remaining patches
>>>>> merged.
>>>>
>>>> Series looks good to me.
>>>>
>>>> To preserve bisect, shouldn't the order of applying be patch #3, #4, #1
>>>> and #2 ?
>>>>
>>>> Given the dependencies and to preserve bisect its easiest if I take the
>>>> series with acks from remoteproc and clock maintainers.
>>>>
>>>> Open to other suggestions as well.
>>>>
>>>> Thanks,
>>>> Sekhar
>>>
>>> Oops you're right about the order. Do you want me to resend?
>>
>> With your ack, I can queue 1/4 for v4.19 and provide an immutable commit
>> to you (on top of v4.18-rc1) for you to merge any further changes you
>> want to queue from your tree.
>>
>> Bartosz, given the number of moving pieces, I think its better to keep
>> 2/4 for v4.20 release - once all other other dependencies have been
>> merged.
>
> I was thinking the same thing as well. I will pick it up in a
> clk-davinci-4.20 branch if that sounds OK.

Sounds good. Just check that there really are no users of that API
before queuing :)

Regards,
Sekhar

2018-07-06 17:55:40

by David Lechner

[permalink] [raw]
Subject: Re: [PATCH v7 2/4] clk: davinci: kill davinci_clk_reset_assert/deassert()

On 06/21/2018 02:37 AM, Bartosz Golaszewski wrote:
> 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]>
> Acked-by: Stephen Boyd <[email protected]>
> ---

I've picked this up in clk-davinci-4.20, so it will just hang out
there for a while until the rest of this series gets merged.



2018-07-23 08:04:33

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH v7 1/4] remoteproc/davinci: use the reset framework

2018-06-21 9:37 GMT+02:00 Bartosz Golaszewski <[email protected]>:
> 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]>
> Reviewed-by: Sekhar Nori <[email protected]>
> Reviewed-by: Philipp Zabel <[email protected]>
> ---
> drivers/remoteproc/da8xx_remoteproc.c | 34 +++++++++++++++++++++++----
> 1 file changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c
> index b668e32996e2..76c06b70a1c6 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) {
> @@ -287,6 +310,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;
>
> ret = da8xx_rproc_get_internal_memories(pdev, drproc);
> @@ -309,7 +333,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.17.1
>

Hi Bjorn, Sekhar,

I'm not seeing this patch in next, did you agree on how to pick it up for 4.19?

Thanks in advance,
Bartosz

2018-07-31 04:26:00

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v7 1/4] remoteproc/davinci: use the reset framework

On Thu 21 Jun 00:37 PDT 2018, 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]>
> Reviewed-by: Sekhar Nori <[email protected]>
> Reviewed-by: Philipp Zabel <[email protected]>

Acked-by: Bjorn Andersson <[email protected]>

Regards,
Bjorn

> ---
> drivers/remoteproc/da8xx_remoteproc.c | 34 +++++++++++++++++++++++----
> 1 file changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c
> index b668e32996e2..76c06b70a1c6 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) {
> @@ -287,6 +310,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;
>
> ret = da8xx_rproc_get_internal_memories(pdev, drproc);
> @@ -309,7 +333,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.17.1
>

2018-07-31 04:34:24

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v7 0/4] ARM: davinci: complete the conversion to using the reset framework

On Mon 02 Jul 05:08 PDT 2018, Sekhar Nori wrote:

> Hi Bjorn,
>
> On Thursday 21 June 2018 05:11 PM, Bartosz Golaszewski wrote:
> > 2018-06-21 12:52 GMT+02:00 Sekhar Nori <[email protected]>:
> >> Hi Bartosz,
> >>
> >> On Thursday 21 June 2018 01:07 PM, Bartosz Golaszewski wrote:
> >>> From: Bartosz Golaszewski <[email protected]>
> >>>
> >>> These are the remaining patches that still need to be merged in order
> >>> to complete the conversion of the davinci dsp driver to using the reset
> >>> framework.
> >>>
> >>> They apply on top of v4.18-rc1 with David Lechner's remaining patches
> >>> merged.
> >>
> >> Series looks good to me.
> >>
> >> To preserve bisect, shouldn't the order of applying be patch #3, #4, #1
> >> and #2 ?
> >>
> >> Given the dependencies and to preserve bisect its easiest if I take the
> >> series with acks from remoteproc and clock maintainers.
> >>
> >> Open to other suggestions as well.
> >>
> >> Thanks,
> >> Sekhar
> >
> > Oops you're right about the order. Do you want me to resend?
>
> With your ack, I can queue 1/4 for v4.19 and provide an immutable commit
> to you (on top of v4.18-rc1) for you to merge any further changes you
> want to queue from your tree.
>

I'm not sure why I didn't see your request earlier, sorry about that.

I seem to have one other davinci patch from Suman, which should be
possible to merge without any conflicts. So there's no need for an
immutable branch at this time.

Regards,
Bjorn

2018-07-31 07:57:04

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH v7 1/4] remoteproc/davinci: use the reset framework

2018-07-31 6:25 GMT+02:00 Bjorn Andersson <[email protected]>:
> On Thu 21 Jun 00:37 PDT 2018, 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]>
>> Reviewed-by: Sekhar Nori <[email protected]>
>> Reviewed-by: Philipp Zabel <[email protected]>
>
> Acked-by: Bjorn Andersson <[email protected]>
>

Sekhar,

can you take this through your tree or is it already too late?

Bart

> Regards,
> Bjorn
>
>> ---
>> drivers/remoteproc/da8xx_remoteproc.c | 34 +++++++++++++++++++++++----
>> 1 file changed, 29 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c
>> index b668e32996e2..76c06b70a1c6 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) {
>> @@ -287,6 +310,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;
>>
>> ret = da8xx_rproc_get_internal_memories(pdev, drproc);
>> @@ -309,7 +333,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.17.1
>>

2018-07-31 10:06:59

by Sekhar Nori

[permalink] [raw]
Subject: Re: [PATCH v7 1/4] remoteproc/davinci: use the reset framework

Bjorn,

On Tuesday 31 July 2018 01:25 PM, Bartosz Golaszewski wrote:
> 2018-07-31 6:25 GMT+02:00 Bjorn Andersson <[email protected]>:
>> On Thu 21 Jun 00:37 PDT 2018, 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]>
>>> Reviewed-by: Sekhar Nori <[email protected]>
>>> Reviewed-by: Philipp Zabel <[email protected]>
>>
>> Acked-by: Bjorn Andersson <[email protected]>
>>
>
> Sekhar,
>
> can you take this through your tree or is it already too late?

The last -rc is already tagged and I am not sure ARM-SoC will be open to
new stuff now (there is also a bit of lag after I send my pull request).

Is this something you can take in your tree along with Suman's patch?

There are no build dependencies with my tree, so it will be safe to
queue from your tree. It will be great if this can be merged late in the
cycle though to avoid breaking remoteproc even for short while during
the merge window.

Or if you okay with it, it will be safest to send it soon after
v4.19-rc1 is tagged.

Let me know.

Thanks,
Sekhar

2018-10-02 15:09:22

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH v7 2/4] clk: davinci: kill davinci_clk_reset_assert/deassert()

pt., 6 lip 2018 o 19:54 David Lechner <[email protected]> napisał(a):
>
> On 06/21/2018 02:37 AM, Bartosz Golaszewski wrote:
> > 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]>
> > Acked-by: Stephen Boyd <[email protected]>
> > ---
>
> I've picked this up in clk-davinci-4.20, so it will just hang out
> there for a while until the rest of this series gets merged.
>

Hi David,

I don't see this patch in next yet. Is it queued for 4.20?

Thanks in advance,
Bartosz

2018-10-02 15:29:21

by David Lechner

[permalink] [raw]
Subject: Re: [PATCH v7 2/4] clk: davinci: kill davinci_clk_reset_assert/deassert()

On 10/02/2018 10:08 AM, Bartosz Golaszewski wrote:
> pt., 6 lip 2018 o 19:54 David Lechner <[email protected]> napisał(a):
>>
>> On 06/21/2018 02:37 AM, Bartosz Golaszewski wrote:
>>> 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]>
>>> Acked-by: Stephen Boyd <[email protected]>
>>> ---
>>
>> I've picked this up in clk-davinci-4.20, so it will just hang out
>> there for a while until the rest of this series gets merged.
>>
>
> Hi David,
>
> I don't see this patch in next yet. Is it queued for 4.20?
>
> Thanks in advance,
> Bartosz
>

This is currently the only clk-davinici patch I have queued, so it
might be easier to just resend it and let Stephen/Mike pick it up
directly instead of me sending a pull request for one patch.


2018-10-02 16:44:54

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v7 2/4] clk: davinci: kill davinci_clk_reset_assert/deassert()

Quoting David Lechner (2018-10-02 08:28:44)
> On 10/02/2018 10:08 AM, Bartosz Golaszewski wrote:
> > pt., 6 lip 2018 o 19:54 David Lechner <[email protected]> napisał(a):
> >>
> >> On 06/21/2018 02:37 AM, Bartosz Golaszewski wrote:
> >>> 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]>
> >>> Acked-by: Stephen Boyd <[email protected]>
> >>> ---
> >>
> >> I've picked this up in clk-davinci-4.20, so it will just hang out
> >> there for a while until the rest of this series gets merged.
> >>
> >
> > Hi David,
> >
> > I don't see this patch in next yet. Is it queued for 4.20?
> >
> > Thanks in advance,
> > Bartosz
> >
>
> This is currently the only clk-davinici patch I have queued, so it
> might be easier to just resend it and let Stephen/Mike pick it up
> directly instead of me sending a pull request for one patch.
>

Ok I applied it directly to clk-next.