ARM: davinci: complete the conversion to using the reset framework
Subsystems (8)
v3 cover ARM: davinci: complete the conversion to using the reset framework
From: Bartosz Golaszewski <[email protected]>
This series converts the only user of the handcoded, mach-specific reset routines in the davinci platform to using the reset framework.
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 at 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
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 | 12 +++++---- 7 files changed, 83 insertions(+), 58 deletions(-) delete mode 100644 arch/arm/mach-davinci/include/mach/clock.h
Signature · 1 line
2.16.1
PATCH v3 1/8 reset: modify the way reset lookup works for board files
From: Bartosz Golaszewski <[email protected]>
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.
--- drivers/reset/core.c | 38 +++++++++++++++++++++++++++++++------- include/linux/reset-controller.h | 12 +++++++----- 2 files changed, 38 insertions(+), 12 deletions(-)
drivers/reset/core.c +31 -7
@@ -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);
include/linux/reset-controller.h +7 -5
@@ -30,21 +30,22 @@ 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, _dev_id, _con_id, _index) \ { \ + .provider = _provider, \ .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
Signature · 1 line
2.16.1
PATCH v3 2/8 ARM: davinci: dts: make psc0 a reset provider
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
The psc driver registers with the reset framework as a provider. Add the #reset-cells property to the psc0 node.
--- arch/arm/boot/dts/da850.dtsi | 1 + 1 file changed, 1 insertion(+)
arch/arm/boot/dts/da850.dtsi +1 -0
@@ -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>,
Signature · 1 line
2.16.1
PATCH v3 3/8 ARM: davinci: dts: add a reset control to the dsp node
In reply to Bartosz GolaszewskiFrom: 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.
--- arch/arm/boot/dts/da850.dtsi | 1 + 1 file changed, 1 insertion(+)
arch/arm/boot/dts/da850.dtsi +1 -0
@@ -109,6 +109,7 @@ interrupt-parent = <&intc>; interrupts = <28>; clocks = <&psc0 15>; + resets = <&psc0 15>; status = "disabled"; }; soc at 1c00000 {
Signature · 1 line
2.16.1
PATCH v3 4/8 clk: davinci: add a reset lookup table for psc0
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
In order to be able to use the reset framework in legacy boot mode as well, add the reset lookup table to the psc driver for da850 variant.
--- drivers/clk/davinci/psc-da850.c | 7 +++++++ drivers/clk/davinci/psc.c | 1 + 2 files changed, 8 insertions(+)
drivers/clk/davinci/psc-da850.c +7 -0
@@ -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" ,"davinci-rproc.0" ,NULL ,15 ), +}; +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); }
drivers/clk/davinci/psc.c +1 -0
@@ -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;
Signature · 1 line
2.16.1
PATCH v3 5/8 remoteproc: da8xx: add the missing retval check for clk_enable()
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
The davinci platform is being switched to using the common clock framework, where clk_enable() can fail. Add the return value check.
--- drivers/remoteproc/da8xx_remoteproc.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/remoteproc/da8xx_remoteproc.c +7 -1
@@ -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 ;
Signature · 1 line
2.16.1
PATCH v3 6/8 remoteproc: da8xx: prepare and unprepare the clock where needed
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
We're currently switching the platform to using the common clock framework. We need to explicitly prepare and unprepare the rproc clock.
--- drivers/remoteproc/da8xx_remoteproc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/remoteproc/da8xx_remoteproc.c +3 -3
@@ -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 ; }
Signature · 1 line
2.16.1
PATCH v3 7/8 remoteproc: da8xx: use the reset framework
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
Switch to using the reset framework instead of handcoded reset routines we used so far.
--- drivers/remoteproc/da8xx_remoteproc.c | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-)
drivers/remoteproc/da8xx_remoteproc.c +27 -5
@@ -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;
Signature · 1 line
2.16.1
PATCH v3 8/8 clk: davinci: kill davinci_clk_reset_assert/deassert()
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
This code is no longer used. Remove it.
--- 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
arch/arm/mach-davinci/include/mach/clock.h deleted +0 -21
@@ -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
drivers/clk/davinci/psc.c +0 -18
@@ -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) {
Signature · 1 line
2.16.1
Philipp Zabel
maintainerRe: [PATCH v3 1/8] reset: modify the way reset lookup works for board files
In reply to Bartosz GolaszewskiHi Bartosz,
Quoted 118 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
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_devstructure 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 <[email protected]>--- drivers/reset/core.c | 38 +++++++++++++++++++++++++++++++------- include/linux/reset-controller.h | 12 +++++++----- 2 files changed, 38 insertions(+), 12 deletions(-)drivers/reset/core.c +31
-7@@ -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);include/linux/reset-controller.h +1
-1@@ -30,21 +30,22 @@ 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 {
include/linux/reset-controller.h struct of_phandle_args; · line 46
Quoted 10 lines
include/linux/reset-controller.h +2
-2struct 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, _dev_id, _con_id, _index) \
Please move the _index before _dev_id and _con_id, right after _provider. Otherwise this looks fine to me.
regards
Philipp