[PATCH v4 0/8] ARM: davinci: complete the conversion to using the reset framework

[PATCH v4 0/8] ARM: davinci: complete the conversion to using the reset framework

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

This series converts the only user of the handcoded, mach-specific reset
routines in the davinci platform to using the reset framework.

Patch 1 modifies the way lookup entries are registered with the reset
framework.

Patches 2-4 add necessary lookups/DT-properties.

Patches 5-7 convert 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] git@github.com: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:
- 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)

Bartosz Golaszewski (8):
  reset: modify the way reset lookup works for board files
  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: da8xx: add the missing retval check for clk_enable()
  remoteproc: da8xx: prepare and unprepare the clock where needed
  remoteproc: da8xx: 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      | 42 +++++++++++++++++++++++++-----
 drivers/reset/core.c                       | 38 ++++++++++++++++++++++-----
 include/linux/reset-controller.h           | 14 +++++-----
 7 files changed, 84 insertions(+), 59 deletions(-)
 delete mode 100644 arch/arm/mach-davinci/include/mach/clock.h

-- 
2.16.1

[PATCH v4 1/8] reset: modify the way reset lookup works for board files

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Commit 7af1bb19f1d7 ("reset: add support for non-DT systems")
introduced reset control lookup mechanism for boards that still use
board files.

The routine used to register lookup entries takes the corresponding
reset_controlled_dev structure as argument.

It's been determined however that for the first user of this new
interface - davinci psc driver - it will be easier to register the
lookup entries using the reset controller device name.

This patch changes the way lookup entries are added.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/reset/core.c             | 38 +++++++++++++++++++++++++++++++-------
 include/linux/reset-controller.h | 14 ++++++++------
 2 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 06fa4907afc4..f4a29c046995 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -153,12 +153,10 @@ EXPORT_SYMBOL_GPL(devm_reset_controller_register);
 
 /**
  * reset_controller_add_lookup - register a set of lookup entries
- * @rcdev: initialized reset controller device owning the reset line
  * @lookup: array of reset lookup entries
  * @num_entries: number of entries in the lookup array
  */
-void reset_controller_add_lookup(struct reset_controller_dev *rcdev,
-				 struct reset_control_lookup *lookup,
+void reset_controller_add_lookup(struct reset_control_lookup *lookup,
 				 unsigned int num_entries)
 {
 	struct reset_control_lookup *entry;
@@ -168,13 +166,12 @@ void reset_controller_add_lookup(struct reset_controller_dev *rcdev,
 	for (i = 0; i < num_entries; i++) {
 		entry = &lookup[i];
 
-		if (!entry->dev_id) {
-			pr_warn("%s(): reset lookup entry has no dev_id, skipping\n",
+		if (!entry->dev_id || !entry->provider) {
+			pr_warn("%s(): reset lookup entry badly specified, skipping\n",
 				__func__);
 			continue;
 		}
 
-		entry->rcdev = rcdev;
 		list_add_tail(&entry->list, &reset_lookup_list);
 	}
 	mutex_unlock(&reset_lookup_mutex);
@@ -526,11 +523,30 @@ struct reset_control *__of_reset_control_get(struct device_node *node,
 }
 EXPORT_SYMBOL_GPL(__of_reset_control_get);
 
+static struct reset_controller_dev *
+__reset_controller_by_name(const char *name)
+{
+	struct reset_controller_dev *rcdev;
+
+	lockdep_assert_held(&reset_list_mutex);
+
+	list_for_each_entry(rcdev, &reset_controller_list, list) {
+		if (!rcdev->dev)
+			continue;
+
+		if (!strcmp(name, dev_name(rcdev->dev)))
+			return rcdev;
+	}
+
+	return NULL;
+}
+
 static struct reset_control *
 __reset_control_get_from_lookup(struct device *dev, const char *con_id,
 				bool shared, bool optional)
 {
 	const struct reset_control_lookup *lookup;
+	struct reset_controller_dev *rcdev;
 	const char *dev_id = dev_name(dev);
 	struct reset_control *rstc = NULL;
 
@@ -547,7 +563,15 @@ __reset_control_get_from_lookup(struct device *dev, const char *con_id,
 		    ((con_id && lookup->con_id) &&
 		     !strcmp(con_id, lookup->con_id))) {
 			mutex_lock(&reset_list_mutex);
-			rstc = __reset_control_get_internal(lookup->rcdev,
+			rcdev = __reset_controller_by_name(lookup->provider);
+			if (!rcdev) {
+				mutex_unlock(&reset_list_mutex);
+				mutex_unlock(&reset_lookup_mutex);
+				/* Reset provider may not be ready yet. */
+				return -EPROBE_DEFER;
+			}
+
+			rstc = __reset_control_get_internal(rcdev,
 							    lookup->index,
 							    shared);
 			mutex_unlock(&reset_list_mutex);
diff --git a/include/linux/reset-controller.h b/include/linux/reset-controller.h
index 25698f6c1fae..9326d671b6e6 100644
--- a/include/linux/reset-controller.h
+++ b/include/linux/reset-controller.h
@@ -30,24 +30,25 @@ struct of_phandle_args;
  * struct reset_control_lookup - represents a single lookup entry
  *
  * @list: internal list of all reset lookup entries
- * @rcdev: reset controller device controlling this reset line
+ * @provider: name of the reset controller device controlling this reset line
  * @index: ID of the reset controller in the reset controller device
  * @dev_id: name of the device associated with this reset line
  * @con_id name of the reset line (can be NULL)
  */
 struct reset_control_lookup {
 	struct list_head list;
-	struct reset_controller_dev *rcdev;
+	const char *provider;
 	unsigned int index;
 	const char *dev_id;
 	const char *con_id;
 };
 
-#define RESET_LOOKUP(_dev_id, _con_id, _index)				\
+#define RESET_LOOKUP(_provider, _index, _dev_id, _con_id)		\
 	{								\
+		.provider = _provider,					\
+		.index = _index,					\
 		.dev_id = _dev_id,					\
 		.con_id = _con_id,					\
-		.index = _index,					\
 	}
 
 /**
@@ -57,6 +58,7 @@ struct reset_control_lookup {
  * @owner: kernel module of the reset controller driver
  * @list: internal list of reset controller devices
  * @reset_control_head: head of internal list of requested reset controls
+ * @dev: corresponding driver model device struct
  * @of_node: corresponding device tree node as phandle target
  * @of_reset_n_cells: number of cells in reset line specifiers
  * @of_xlate: translation function to translate from specifier as found in the
@@ -68,6 +70,7 @@ struct reset_controller_dev {
 	struct module *owner;
 	struct list_head list;
 	struct list_head reset_control_head;
+	struct device *dev;
 	struct device_node *of_node;
 	int of_reset_n_cells;
 	int (*of_xlate)(struct reset_controller_dev *rcdev,
@@ -82,8 +85,7 @@ struct device;
 int devm_reset_controller_register(struct device *dev,
 				   struct reset_controller_dev *rcdev);
 
-void reset_controller_add_lookup(struct reset_controller_dev *rcdev,
-				 struct reset_control_lookup *lookup,
+void reset_controller_add_lookup(struct reset_control_lookup *lookup,
 				 unsigned int num_entries);
 
 #endif
-- 
2.16.1

Re: [PATCH v4 1/8] reset: modify the way reset lookup works for board files

[Philipp Zabel]

On Fri, 2018-03-23 at 14:04 +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Commit 7af1bb19f1d7 ("reset: add support for non-DT systems")
> introduced reset control lookup mechanism for boards that still use
> board files.
> 
> The routine used to register lookup entries takes the corresponding
> reset_controlled_dev structure as argument.
> 
> It's been determined however that for the first user of this new
> interface - davinci psc driver - it will be easier to register the
> lookup entries using the reset controller device name.
> 
> This patch changes the way lookup entries are added.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  drivers/reset/core.c             | 38 +++++++++++++++++++++++++++++++-------
>  include/linux/reset-controller.h | 14 ++++++++------
>  2 files changed, 39 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index 06fa4907afc4..f4a29c046995 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -153,12 +153,10 @@ EXPORT_SYMBOL_GPL(devm_reset_controller_register);
>  
>  /**
>   * reset_controller_add_lookup - register a set of lookup entries
> - * @rcdev: initialized reset controller device owning the reset line
>   * @lookup: array of reset lookup entries
>   * @num_entries: number of entries in the lookup array
>   */
> -void reset_controller_add_lookup(struct reset_controller_dev *rcdev,
> -				 struct reset_control_lookup *lookup,
> +void reset_controller_add_lookup(struct reset_control_lookup *lookup,
>  				 unsigned int num_entries)
>  {
>  	struct reset_control_lookup *entry;
> @@ -168,13 +166,12 @@ void reset_controller_add_lookup(struct reset_controller_dev *rcdev,
>  	for (i = 0; i < num_entries; i++) {
>  		entry = &lookup[i];
>  
> -		if (!entry->dev_id) {
> -			pr_warn("%s(): reset lookup entry has no dev_id, skipping\n",
> +		if (!entry->dev_id || !entry->provider) {
> +			pr_warn("%s(): reset lookup entry badly specified, skipping\n",
>  				__func__);
>  			continue;
>  		}
>  
> -		entry->rcdev = rcdev;
>  		list_add_tail(&entry->list, &reset_lookup_list);
>  	}
>  	mutex_unlock(&reset_lookup_mutex);
> @@ -526,11 +523,30 @@ struct reset_control *__of_reset_control_get(struct device_node *node,
>  }
>  EXPORT_SYMBOL_GPL(__of_reset_control_get);
>  
> +static struct reset_controller_dev *
> +__reset_controller_by_name(const char *name)
> +{
> +	struct reset_controller_dev *rcdev;
> +
> +	lockdep_assert_held(&reset_list_mutex);
> +
> +	list_for_each_entry(rcdev, &reset_controller_list, list) {
> +		if (!rcdev->dev)
> +			continue;
> +
> +		if (!strcmp(name, dev_name(rcdev->dev)))
> +			return rcdev;
> +	}
> +
> +	return NULL;
> +}
> +
>  static struct reset_control *
>  __reset_control_get_from_lookup(struct device *dev, const char *con_id,
>  				bool shared, bool optional)
>  {
>  	const struct reset_control_lookup *lookup;
> +	struct reset_controller_dev *rcdev;
>  	const char *dev_id = dev_name(dev);
>  	struct reset_control *rstc = NULL;
>  
> @@ -547,7 +563,15 @@ __reset_control_get_from_lookup(struct device *dev, const char *con_id,
>  		    ((con_id && lookup->con_id) &&
>  		     !strcmp(con_id, lookup->con_id))) {
>  			mutex_lock(&reset_list_mutex);
> -			rstc = __reset_control_get_internal(lookup->rcdev,
> +			rcdev = __reset_controller_by_name(lookup->provider);
> +			if (!rcdev) {
> +				mutex_unlock(&reset_list_mutex);
> +				mutex_unlock(&reset_lookup_mutex);
> +				/* Reset provider may not be ready yet. */
> +				return -EPROBE_DEFER;

Thanks, I've applied this patch to reset/next with the following change:

-                               return -EPROBE_DEFER;
+                               return ERR_PTR(-EPROBE_DEFER);

regards
Philipp

Re: [PATCH v4 1/8] reset: modify the way reset lookup works for board files

[Bartosz Golaszewski]

2018-03-23 14:43 GMT+01:00 Philipp Zabel <p.zabel@pengutronix.de>:
> On Fri, 2018-03-23 at 14:04 +0100, Bartosz Golaszewski wrote:
>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>
>> Commit 7af1bb19f1d7 ("reset: add support for non-DT systems")
>> introduced reset control lookup mechanism for boards that still use
>> board files.
>>
>> The routine used to register lookup entries takes the corresponding
>> reset_controlled_dev structure as argument.
>>
>> It's been determined however that for the first user of this new
>> interface - davinci psc driver - it will be easier to register the
>> lookup entries using the reset controller device name.
>>
>> This patch changes the way lookup entries are added.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> ---
>>  drivers/reset/core.c             | 38 +++++++++++++++++++++++++++++++-------
>>  include/linux/reset-controller.h | 14 ++++++++------
>>  2 files changed, 39 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
>> index 06fa4907afc4..f4a29c046995 100644
>> --- a/drivers/reset/core.c
>> +++ b/drivers/reset/core.c
>> @@ -153,12 +153,10 @@ EXPORT_SYMBOL_GPL(devm_reset_controller_register);
>>
>>  /**
>>   * reset_controller_add_lookup - register a set of lookup entries
>> - * @rcdev: initialized reset controller device owning the reset line
>>   * @lookup: array of reset lookup entries
>>   * @num_entries: number of entries in the lookup array
>>   */
>> -void reset_controller_add_lookup(struct reset_controller_dev *rcdev,
>> -                              struct reset_control_lookup *lookup,
>> +void reset_controller_add_lookup(struct reset_control_lookup *lookup,
>>                                unsigned int num_entries)
>>  {
>>       struct reset_control_lookup *entry;
>> @@ -168,13 +166,12 @@ void reset_controller_add_lookup(struct reset_controller_dev *rcdev,
>>       for (i = 0; i < num_entries; i++) {
>>               entry = &lookup[i];
>>
>> -             if (!entry->dev_id) {
>> -                     pr_warn("%s(): reset lookup entry has no dev_id, skipping\n",
>> +             if (!entry->dev_id || !entry->provider) {
>> +                     pr_warn("%s(): reset lookup entry badly specified, skipping\n",
>>                               __func__);
>>                       continue;
>>               }
>>
>> -             entry->rcdev = rcdev;
>>               list_add_tail(&entry->list, &reset_lookup_list);
>>       }
>>       mutex_unlock(&reset_lookup_mutex);
>> @@ -526,11 +523,30 @@ struct reset_control *__of_reset_control_get(struct device_node *node,
>>  }
>>  EXPORT_SYMBOL_GPL(__of_reset_control_get);
>>
>> +static struct reset_controller_dev *
>> +__reset_controller_by_name(const char *name)
>> +{
>> +     struct reset_controller_dev *rcdev;
>> +
>> +     lockdep_assert_held(&reset_list_mutex);
>> +
>> +     list_for_each_entry(rcdev, &reset_controller_list, list) {
>> +             if (!rcdev->dev)
>> +                     continue;
>> +
>> +             if (!strcmp(name, dev_name(rcdev->dev)))
>> +                     return rcdev;
>> +     }
>> +
>> +     return NULL;
>> +}
>> +
>>  static struct reset_control *
>>  __reset_control_get_from_lookup(struct device *dev, const char *con_id,
>>                               bool shared, bool optional)
>>  {
>>       const struct reset_control_lookup *lookup;
>> +     struct reset_controller_dev *rcdev;
>>       const char *dev_id = dev_name(dev);
>>       struct reset_control *rstc = NULL;
>>
>> @@ -547,7 +563,15 @@ __reset_control_get_from_lookup(struct device *dev, const char *con_id,
>>                   ((con_id && lookup->con_id) &&
>>                    !strcmp(con_id, lookup->con_id))) {
>>                       mutex_lock(&reset_list_mutex);
>> -                     rstc = __reset_control_get_internal(lookup->rcdev,
>> +                     rcdev = __reset_controller_by_name(lookup->provider);
>> +                     if (!rcdev) {
>> +                             mutex_unlock(&reset_list_mutex);
>> +                             mutex_unlock(&reset_lookup_mutex);
>> +                             /* Reset provider may not be ready yet. */
>> +                             return -EPROBE_DEFER;
>
> Thanks, I've applied this patch to reset/next with the following change:
>
> -                               return -EPROBE_DEFER;
> +                               return ERR_PTR(-EPROBE_DEFER);
>
> regards
> Philipp

Oops, thanks for spotting that.

Thanks,
Bart

[PATCH v4 2/8] ARM: davinci: dts: make psc0 a reset provider

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

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 <bgolaszewski@baylibre.com>
---
 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

[PATCH v4 3/8] ARM: davinci: dts: add a reset control to the dsp node

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

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 <bgolaszewski@baylibre.com>
---
 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

[PATCH v4 4/8] clk: davinci: add a reset lookup table for psc0

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

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 <bgolaszewski@baylibre.com>
---
 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

[PATCH v4 5/8] remoteproc: da8xx: add the missing retval check for clk_enable()

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

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 <bgolaszewski@baylibre.com>
---
 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

Re: [PATCH v4 5/8] remoteproc: da8xx: add the missing retval check for clk_enable()

[Suman Anna]

On 03/23/2018 08:04 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> 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 <bgolaszewski@baylibre.com>

This patch can be applied agnostic of the CCF change as well.
I had been using "remoteproc/davinci: ..." on the patch subject for all
davinci remoteproc patches. So prefer to maintain the same.

With that,
Acked-by: Suman Anna <s-anna@ti.com>

regards
Suman

> ---
>  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;
> 

[PATCH v4 6/8] remoteproc: da8xx: prepare and unprepare the clock where needed

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

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 <bgolaszewski@baylibre.com>
---
 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

Re: [PATCH v4 6/8] remoteproc: da8xx: prepare and unprepare the clock where needed

[Suman Anna]

On 03/23/2018 08:04 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> 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 <bgolaszewski@baylibre.com>

FWIW, I have been able to use this patch even without the CCF adaptation
as clk_prepare()/clk_unprepare() are stubs returning 0, and the
effective code would be same as before. Prefer "remoteproc/davinci: ..."
on the patch subject as was being done previously.

With that,
Acked-by: Suman Anna <s-anna@ti.com>

regards
Suman

> ---
>  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;
>  }
> 

[PATCH v4 7/8] remoteproc: da8xx: use the reset framework

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Switch to using the reset framework instead of handcoded reset routines
we used so far.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/remoteproc/da8xx_remoteproc.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c
index f134192922e0..3689473f8b49 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,11 @@ 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);
+		return ret;
+	}
 
 	return 0;
 }
@@ -163,8 +168,15 @@ static int da8xx_rproc_start(struct rproc *rproc)
 static int da8xx_rproc_stop(struct rproc *rproc)
 {
 	struct da8xx_rproc *drproc = rproc->priv;
+	int ret;
+
+	ret = reset_control_assert(drproc->dsp_reset);
+	if (ret) {
+		dev_err(rproc->dev.parent,
+			"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 +244,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 +281,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 +331,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

Re: [PATCH v4 7/8] remoteproc: da8xx: use the reset framework

[Suman Anna]

Hi Bart,

On 03/23/2018 08:04 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Switch to using the reset framework instead of handcoded reset routines
> we used so far.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  drivers/remoteproc/da8xx_remoteproc.c | 32 +++++++++++++++++++++++++++-----
>  1 file changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c
> index f134192922e0..3689473f8b49 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,11 @@ 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);
> +		return ret;

need to unwind the clk_prepare_enable() here.

> +	}
>  
>  	return 0;
>  }
> @@ -163,8 +168,15 @@ static int da8xx_rproc_start(struct rproc *rproc)
>  static int da8xx_rproc_stop(struct rproc *rproc)
>  {
>  	struct da8xx_rproc *drproc = rproc->priv;
> +	int ret;
> +
> +	ret = reset_control_assert(drproc->dsp_reset);
> +	if (ret) {
> +		dev_err(rproc->dev.parent,
> +			"reset_control_assert() failed: %d\n", ret);

prefer the trace statement to be on the same line as dev_err, you can
just keep the ret on the next line. I am fine with defining a local dev
variable as well (mirrors da8xx_rproc_start() to have the trace in a
single line).

> +		return ret;
> +	}
>  
> -	davinci_clk_reset_assert(drproc->dsp_clk);
>  	clk_disable_unprepare(drproc->dsp_clk);
>  
>  	return 0;
> @@ -232,6 +244,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 +281,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 +331,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;
>  

The reset API usage changes themselves look good otherwise.

regards
Suman

[PATCH v4 8/8] clk: davinci: kill davinci_clk_reset_assert/deassert()

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

This code is no longer used. Remove it.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: David Lechner <david@lechnology.com>
---
 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 <source@mvista.com>
- *
- * 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

Re: [PATCH v4 0/8] ARM: davinci: complete the conversion to using the reset framework

[Stephen Boyd]

Quoting Bartosz Golaszewski (2018-03-23 06:04:47)
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> This series converts the only user of the handcoded, mach-specific reset
> routines in the davinci platform to using the reset framework.
> 
> Patch 1 modifies the way lookup entries are registered with the reset
> framework.
> 
> Patches 2-4 add necessary lookups/DT-properties.
> 
> Patches 5-7 convert 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].
> 

What's the merge strategy for the rest of the patches? I should apply
the clk ones after the next -rc1?

Re: [PATCH v4 0/8] ARM: davinci: complete the conversion to using the reset framework

[Bartosz Golaszewski]

2018-03-23 17:49 GMT+01:00 Stephen Boyd <sboyd@kernel.org>:
> Quoting Bartosz Golaszewski (2018-03-23 06:04:47)
>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>
>> This series converts the only user of the handcoded, mach-specific reset
>> routines in the davinci platform to using the reset framework.
>>
>> Patch 1 modifies the way lookup entries are registered with the reset
>> framework.
>>
>> Patches 2-4 add necessary lookups/DT-properties.
>>
>> Patches 5-7 convert 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].
>>
>
> What's the merge strategy for the rest of the patches? I should apply
> the clk ones after the next -rc1?

Or maybe Philipp can provide us with an immutable branch with the reset patches?

The you could apply the driver patches and let Sekhar take all the
platform code?

Best regards,
Bartosz Golaszewski

Re: [PATCH v4 0/8] ARM: davinci: complete the conversion to using the reset framework

[Stephen Boyd]

Quoting Bartosz Golaszewski (2018-03-23 09:55:47)
> 2018-03-23 17:49 GMT+01:00 Stephen Boyd <sboyd@kernel.org>:
> > Quoting Bartosz Golaszewski (2018-03-23 06:04:47)
> >> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >>
> >> This series converts the only user of the handcoded, mach-specific reset
> >> routines in the davinci platform to using the reset framework.
> >>
> >> Patch 1 modifies the way lookup entries are registered with the reset
> >> framework.
> >>
> >> Patches 2-4 add necessary lookups/DT-properties.
> >>
> >> Patches 5-7 convert 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].
> >>
> >
> > What's the merge strategy for the rest of the patches? I should apply
> > the clk ones after the next -rc1?
> 
> Or maybe Philipp can provide us with an immutable branch with the reset patches?
> 
> The you could apply the driver patches and let Sekhar take all the
> platform code?
> 

Ok that could work too.

Re: [PATCH v4 0/8] ARM: davinci: complete the conversion to using the reset framework

[Bartosz Golaszewski]

2018-03-23 18:08 GMT+01:00 Stephen Boyd <sboyd@kernel.org>:
> Quoting Bartosz Golaszewski (2018-03-23 09:55:47)
>> 2018-03-23 17:49 GMT+01:00 Stephen Boyd <sboyd@kernel.org>:
>> > Quoting Bartosz Golaszewski (2018-03-23 06:04:47)
>> >> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> >>
>> >> This series converts the only user of the handcoded, mach-specific reset
>> >> routines in the davinci platform to using the reset framework.
>> >>
>> >> Patch 1 modifies the way lookup entries are registered with the reset
>> >> framework.
>> >>
>> >> Patches 2-4 add necessary lookups/DT-properties.
>> >>
>> >> Patches 5-7 convert 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].
>> >>
>> >
>> > What's the merge strategy for the rest of the patches? I should apply
>> > the clk ones after the next -rc1?
>>
>> Or maybe Philipp can provide us with an immutable branch with the reset patches?
>>
>> The you could apply the driver patches and let Sekhar take all the
>> platform code?
>>
>
> Ok that could work too.

Ohad, Bjorn can you ack the remoteproc patches? Are you OK with
Stephen taking them through the clock tree? Otherwise it would get
complicated since they depend on the first clk patch and the last clk
patch depends on them.

Thanks,
Bart

Re: [PATCH v4 0/8] ARM: davinci: complete the conversion to using the reset framework

[Suman Anna]

Hi Bart,

On 03/23/2018 12:16 PM, Bartosz Golaszewski wrote:
> 2018-03-23 18:08 GMT+01:00 Stephen Boyd <sboyd@kernel.org>:
>> Quoting Bartosz Golaszewski (2018-03-23 09:55:47)
>>> 2018-03-23 17:49 GMT+01:00 Stephen Boyd <sboyd@kernel.org>:
>>>> Quoting Bartosz Golaszewski (2018-03-23 06:04:47)
>>>>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>>>
>>>>> This series converts the only user of the handcoded, mach-specific reset
>>>>> routines in the davinci platform to using the reset framework.
>>>>>
>>>>> Patch 1 modifies the way lookup entries are registered with the reset
>>>>> framework.
>>>>>
>>>>> Patches 2-4 add necessary lookups/DT-properties.
>>>>>
>>>>> Patches 5-7 convert 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].
>>>>>
>>>>
>>>> What's the merge strategy for the rest of the patches? I should apply
>>>> the clk ones after the next -rc1?
>>>
>>> Or maybe Philipp can provide us with an immutable branch with the reset patches?
>>>
>>> The you could apply the driver patches and let Sekhar take all the
>>> platform code?
>>>
>>
>> Ok that could work too.
> 
> Ohad, Bjorn can you ack the remoteproc patches? Are you OK with
> Stephen taking them through the clock tree? Otherwise it would get
> complicated since they depend on the first clk patch and the last clk
> patch depends on them.

I will take a closer look and test on Mon. A quick glance of the
remoteproc changes seem to be fine. I will let Bjorn comment on the
patch flow.

The only reason I had to use the clock in the driver was for the reset
before, and hopefully this will allow me to actually switch to using
pm_runtime API like I do with the rest of the TI remoteproc drivers
(Keystone remoteproc driver also uses PSC for clock and reset but then
it goes through different set of drivers).

So, I see a mix of driver and dts patches in the series, are all the dts
patches coming through Sekhar?

regards
Suman

> 
> Thanks,
> Bart
> --
> To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v4 0/8] ARM: davinci: complete the conversion to using the reset framework

[Suman Anna]

Hi Bart,

On 03/23/2018 07:30 PM, Suman Anna wrote:
> Hi Bart,
> 
> On 03/23/2018 12:16 PM, Bartosz Golaszewski wrote:
>> 2018-03-23 18:08 GMT+01:00 Stephen Boyd <sboyd@kernel.org>:
>>> Quoting Bartosz Golaszewski (2018-03-23 09:55:47)
>>>> 2018-03-23 17:49 GMT+01:00 Stephen Boyd <sboyd@kernel.org>:
>>>>> Quoting Bartosz Golaszewski (2018-03-23 06:04:47)
>>>>>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>>>>
>>>>>> This series converts the only user of the handcoded, mach-specific reset
>>>>>> routines in the davinci platform to using the reset framework.
>>>>>>
>>>>>> Patch 1 modifies the way lookup entries are registered with the reset
>>>>>> framework.
>>>>>>
>>>>>> Patches 2-4 add necessary lookups/DT-properties.
>>>>>>
>>>>>> Patches 5-7 convert 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].
>>>>>>
>>>>>
>>>>> What's the merge strategy for the rest of the patches? I should apply
>>>>> the clk ones after the next -rc1?
>>>>
>>>> Or maybe Philipp can provide us with an immutable branch with the reset patches?
>>>>
>>>> The you could apply the driver patches and let Sekhar take all the
>>>> platform code?
>>>>
>>>
>>> Ok that could work too.
>>
>> Ohad, Bjorn can you ack the remoteproc patches? Are you OK with
>> Stephen taking them through the clock tree? Otherwise it would get
>> complicated since they depend on the first clk patch and the last clk
>> patch depends on them.
> 
> I will take a closer look and test on Mon. A quick glance of the
> remoteproc changes seem to be fine. I will let Bjorn comment on the
> patch flow.
> 
> The only reason I had to use the clock in the driver was for the reset
> before, and hopefully this will allow me to actually switch to using
> pm_runtime API like I do with the rest of the TI remoteproc drivers
> (Keystone remoteproc driver also uses PSC for clock and reset but then
> it goes through different set of drivers).
> 
> So, I see a mix of driver and dts patches in the series, are all the dts
> patches coming through Sekhar?

I tried booting your tree [3] on the OMAPL138-LCDK board. I am using
NFS, and I am unable to get to the console. I don't have any issues
booting latest -next branch (next-20180326) which only has the reset and
clock driver related changes.

Here's the log towards the end of the boot..
---
pinctrl-single 1c14120.pinmux: 160 pins, size 80
Serial: 8250/16550 driver, 3 ports, IRQ sharing disabled
console [ttyS2] disabled
1d0d000.serial: ttyS2 at MMIO 0x1d0d000 (irq = 61, base_baud = 8250000)
is a TI DA8xx/66AK2x
console [ttyS2] enabled
brd: module loaded
libphy: Fixed MDIO Bus: probed
davinci_mdio 1e24000.mdio: davinci mdio revision 1.5, bus freq 2200000
davinci_mdio 1e24000.mdio: no live phy, scanning all
davinci_mdio: probe of 1e24000.mdio failed with error -5
i2c /dev entries driver
<nothing after this>

regards
Suman

> 
> regards
> Suman
> 
>>
>> Thanks,
>> Bart
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v4 0/8] ARM: davinci: complete the conversion to using the reset framework

[Sekhar Nori]

Hi Suman,

On Tuesday 27 March 2018 06:12 AM, Suman Anna wrote:
> I tried booting your tree [3] on the OMAPL138-LCDK board. I am using
> NFS, and I am unable to get to the console. I don't have any issues
> booting latest -next branch (next-20180326) which only has the reset and
> clock driver related changes.
> 
> Here's the log towards the end of the boot..
> ---
> pinctrl-single 1c14120.pinmux: 160 pins, size 80
> Serial: 8250/16550 driver, 3 ports, IRQ sharing disabled
> console [ttyS2] disabled
> 1d0d000.serial: ttyS2 at MMIO 0x1d0d000 (irq = 61, base_baud = 8250000)
> is a TI DA8xx/66AK2x
> console [ttyS2] enabled
> brd: module loaded
> libphy: Fixed MDIO Bus: probed
> davinci_mdio 1e24000.mdio: davinci mdio revision 1.5, bus freq 2200000
> davinci_mdio 1e24000.mdio: no live phy, scanning all
> davinci_mdio: probe of 1e24000.mdio failed with error -5
> i2c /dev entries driver
> <nothing after this>

This is because of a bug in bootloader PLL configuration which got
uncovered only with CCF conversion. The bootloader that comes
pre-flashed with the board or the latest U-Boot master should work fine.

Here is the log with latest U-Boot master:
https://pastebin.ubuntu.com/p/HkwY9g4YDc/

Thanks,
Sekhar

Re: [PATCH v4 0/8] ARM: davinci: complete the conversion to using the reset framework

[Suman Anna]

On 03/27/2018 12:24 AM, Sekhar Nori wrote:
> Hi Suman,
> 
> On Tuesday 27 March 2018 06:12 AM, Suman Anna wrote:
>> I tried booting your tree [3] on the OMAPL138-LCDK board. I am using
>> NFS, and I am unable to get to the console. I don't have any issues
>> booting latest -next branch (next-20180326) which only has the reset and
>> clock driver related changes.
>>
>> Here's the log towards the end of the boot..
>> ---
>> pinctrl-single 1c14120.pinmux: 160 pins, size 80
>> Serial: 8250/16550 driver, 3 ports, IRQ sharing disabled
>> console [ttyS2] disabled
>> 1d0d000.serial: ttyS2 at MMIO 0x1d0d000 (irq = 61, base_baud = 8250000)
>> is a TI DA8xx/66AK2x
>> console [ttyS2] enabled
>> brd: module loaded
>> libphy: Fixed MDIO Bus: probed
>> davinci_mdio 1e24000.mdio: davinci mdio revision 1.5, bus freq 2200000
>> davinci_mdio 1e24000.mdio: no live phy, scanning all
>> davinci_mdio: probe of 1e24000.mdio failed with error -5
>> i2c /dev entries driver
>> <nothing after this>
> 
> This is because of a bug in bootloader PLL configuration which got
> uncovered only with CCF conversion. The bootloader that comes
> pre-flashed with the board or the latest U-Boot master should work fine.
> 
> Here is the log with latest U-Boot master:
> https://pastebin.ubuntu.com/p/HkwY9g4YDc/

Thanks Sekhar, able to boot and test after using the latest u-boot.

regards
Suman

Re: [PATCH v4 0/8] ARM: davinci: complete the conversion to using the reset framework

[Sekhar Nori]

Hi Bart,

On Friday 23 March 2018 10:46 PM, Bartosz Golaszewski wrote:
> 2018-03-23 18:08 GMT+01:00 Stephen Boyd <sboyd@kernel.org>:
>> Quoting Bartosz Golaszewski (2018-03-23 09:55:47)
>>> 2018-03-23 17:49 GMT+01:00 Stephen Boyd <sboyd@kernel.org>:
>>>> Quoting Bartosz Golaszewski (2018-03-23 06:04:47)
>>>>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>>>
>>>>> This series converts the only user of the handcoded, mach-specific reset
>>>>> routines in the davinci platform to using the reset framework.
>>>>>
>>>>> Patch 1 modifies the way lookup entries are registered with the reset
>>>>> framework.
>>>>>
>>>>> Patches 2-4 add necessary lookups/DT-properties.
>>>>>
>>>>> Patches 5-7 convert 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].
>>>>>
>>>>
>>>> What's the merge strategy for the rest of the patches? I should apply
>>>> the clk ones after the next -rc1?
>>>
>>> Or maybe Philipp can provide us with an immutable branch with the reset patches?
>>>
>>> The you could apply the driver patches and let Sekhar take all the
>>> platform code?
>>>
>>
>> Ok that could work too.
> 
> Ohad, Bjorn can you ack the remoteproc patches? Are you OK with
> Stephen taking them through the clock tree? Otherwise it would get
> complicated since they depend on the first clk patch and the last clk
> patch depends on them.

I will not be queuing the DTS patches for v4.17. They depend on clock
framework DTS patches which itself I plan to queue for v4.18. Given
this, I think the best bet is:

Ohad/Bjorn queue remoteproc patches already acked by Suman and having no
dependencies. That is, 5/8 and 6/8.

If Philipp can provide an immutable branch with reset changes, it will
be great and Stephen can queue 4/8. If not, its best to resend that
patch to Stephen once v4.17-rc1 is out.

The remaining patches need to wait till v4.18 (or even v4.19 if
dependencies don't pan out well).

Thanks,
Sekhar

Re: [PATCH v4 0/8] ARM: davinci: complete the conversion to using the reset framework

[Philipp Zabel]

On Tue, 2018-03-27 at 11:39 +0530, Sekhar Nori wrote:
> Hi Bart,
> 
> On Friday 23 March 2018 10:46 PM, Bartosz Golaszewski wrote:
> > 2018-03-23 18:08 GMT+01:00 Stephen Boyd <sboyd@kernel.org>:
> > > Quoting Bartosz Golaszewski (2018-03-23 09:55:47)
> > > > 2018-03-23 17:49 GMT+01:00 Stephen Boyd <sboyd@kernel.org>:
> > > > > Quoting Bartosz Golaszewski (2018-03-23 06:04:47)
> > > > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > > > > > 
> > > > > > This series converts the only user of the handcoded, mach-specific reset
> > > > > > routines in the davinci platform to using the reset framework.
> > > > > > 
> > > > > > Patch 1 modifies the way lookup entries are registered with the reset
> > > > > > framework.
> > > > > > 
> > > > > > Patches 2-4 add necessary lookups/DT-properties.
> > > > > > 
> > > > > > Patches 5-7 convert 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].
> > > > > > 
> > > > > 
> > > > > What's the merge strategy for the rest of the patches? I should apply
> > > > > the clk ones after the next -rc1?
> > > > 
> > > > Or maybe Philipp can provide us with an immutable branch with the reset patches?
> > > > 
> > > > The you could apply the driver patches and let Sekhar take all the
> > > > platform code?
> > > > 
> > > 
> > > Ok that could work too.
> > 
> > Ohad, Bjorn can you ack the remoteproc patches? Are you OK with
> > Stephen taking them through the clock tree? Otherwise it would get
> > complicated since they depend on the first clk patch and the last clk
> > patch depends on them.
> 
> I will not be queuing the DTS patches for v4.17. They depend on clock
> framework DTS patches which itself I plan to queue for v4.18. Given
> this, I think the best bet is:
> 
> Ohad/Bjorn queue remoteproc patches already acked by Suman and having no
> dependencies. That is, 5/8 and 6/8.
> 
> If Philipp can provide an immutable branch with reset changes, it will
> be great and Stephen can queue 4/8. If not, its best to resend that
> patch to Stephen once v4.17-rc1 is out.

I have shuffled reset/next a bit and moved Bartosz' patches onto a
separate, immutable branch:

  git://git.pengutronix.de/git/pza/linux.git reset/lookup

regards
Philipp

Re: [PATCH v4 0/8] ARM: davinci: complete the conversion to using the reset framework

[Bartosz Golaszewski]

2018-03-27 8:09 GMT+02:00 Sekhar Nori <nsekhar@ti.com>:
> Hi Bart,
>
> On Friday 23 March 2018 10:46 PM, Bartosz Golaszewski wrote:
>> 2018-03-23 18:08 GMT+01:00 Stephen Boyd <sboyd@kernel.org>:
>>> Quoting Bartosz Golaszewski (2018-03-23 09:55:47)
>>>> 2018-03-23 17:49 GMT+01:00 Stephen Boyd <sboyd@kernel.org>:
>>>>> Quoting Bartosz Golaszewski (2018-03-23 06:04:47)
>>>>>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>>>>
>>>>>> This series converts the only user of the handcoded, mach-specific reset
>>>>>> routines in the davinci platform to using the reset framework.
>>>>>>
>>>>>> Patch 1 modifies the way lookup entries are registered with the reset
>>>>>> framework.
>>>>>>
>>>>>> Patches 2-4 add necessary lookups/DT-properties.
>>>>>>
>>>>>> Patches 5-7 convert 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].
>>>>>>
>>>>>
>>>>> What's the merge strategy for the rest of the patches? I should apply
>>>>> the clk ones after the next -rc1?
>>>>
>>>> Or maybe Philipp can provide us with an immutable branch with the reset patches?
>>>>
>>>> The you could apply the driver patches and let Sekhar take all the
>>>> platform code?
>>>>
>>>
>>> Ok that could work too.
>>
>> Ohad, Bjorn can you ack the remoteproc patches? Are you OK with
>> Stephen taking them through the clock tree? Otherwise it would get
>> complicated since they depend on the first clk patch and the last clk
>> patch depends on them.
>
> I will not be queuing the DTS patches for v4.17. They depend on clock
> framework DTS patches which itself I plan to queue for v4.18. Given
> this, I think the best bet is:
>
> Ohad/Bjorn queue remoteproc patches already acked by Suman and having no
> dependencies. That is, 5/8 and 6/8.
>
> If Philipp can provide an immutable branch with reset changes, it will
> be great and Stephen can queue 4/8. If not, its best to resend that
> patch to Stephen once v4.17-rc1 is out.
>
> The remaining patches need to wait till v4.18 (or even v4.19 if
> dependencies don't pan out well).
>
> Thanks,
> Sekhar

Very good, thanks.

I've just sent out a v3 with three non-reset remoteproc patches that
can already be applied by Suman.

Thanks,
Bartosz