drivers: reset: TI: SoC reset controller support.
Subsystems (2)
PATCH v3 1/6 drivers: reset: TI: SoC reset controller support.
The TI SoC reset controller support utilizes the reset controller framework to give device drivers or function drivers a common set of APIs to call to reset a module.
The reset-ti is a common interface to the reset framework.
The register data is retrieved during initialization
of the reset driver through the reset-ti-data
file. The array of data is associated with the compatible from the respective DT entry.
Once the data is available then this is derefenced within the common interface.
The device driver has the ability to assert, deassert or perform a complete reset.
This code was derived from previous work by Rajendra Nayak and Afzal Mohammed. The code was changed to adopt to the reset core and abstract away the SoC information.
---
v3 - Resolved comments from v2. To many to call out here - https://patchwork.kernel.org/patch/4116941/
drivers/reset/Kconfig | 9 ++ drivers/reset/Makefile | 1 + drivers/reset/reset-ti.c | 373 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 383 insertions(+) create mode 100644 drivers/reset/reset-ti.c
drivers/reset/Kconfig +9 -0
@@ -12,4 +12,13 @@ menuconfig RESET_CONTROLLER If unsure, say no. +config RESET_TI + depends on RESET_CONTROLLER && ARCH_OMAP || COMPILE_TEST + bool "TI reset controller" + help + Reset controller support for TI SoC's + + Reset controller found in TI's AM series of SoC's like + AM335x and AM43x and OMAP SoC's like OMAP5 and DRA7 + source "drivers/reset/sti/Kconfig"
drivers/reset/Makefile +1 -0
@@ -1,4 +1,5 @@ obj-$(CONFIG_RESET_CONTROLLER) += core.o obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o +obj-$(CONFIG_RESET_TI) += reset-ti.o obj-$(CONFIG_ARCH_STI) += sti/
drivers/reset/reset-ti.c new file +373 -0
373 lines not on this page · raw message
Signature · 1 line
1.7.9.5
PATCH v3 2/6 dt: TI: Describe the ti reset DT entries
Describe the TI reset DT entries for TI SoC's.
---
v3 - Changed Headline no other changes
.../devicetree/bindings/reset/ti,reset.txt | 103 ++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 Documentation/devicetree/bindings/reset/ti,reset.txt
Documentation/devicetree/bindings/reset/ti,reset.txt new file +103 -0
@@ -0,0 +1,103 @@ +Texas Instruments Reset Controller +====================================== +Please also refer to reset.txt in this directory for common reset +controller binding usage. + +Specifying the reset entries for the IP module +============================================== +Parent module: +This is the module node that contains the reset registers and bits. + +example: + prcm_resets: resets { + compatible = "ti,dra7-resets"; + #reset-cells = <1>; + }; + +Required parent properties: +- compatible : Should be one of, + "ti,omap4-prm" for OMAP4 PRM instances + "ti,omap5-prm" for OMAP5 PRM instances + "ti,dra7-prm" for DRA7xx PRM instances + "ti,am4-prcm" for AM43xx PRCM instances + "ti,am3-prcm" for AM33xx PRCM instances + +Required child reset property: +- compatible : Should be + "resets" for All TI SoCs + +example: + prm: prm at 4ae06000 { + compatible = "ti,omap5-prm"; + reg = <0x4ae06000 0x3000>; + + prm_resets: resets { + #address-cells = <1>; + #size-cells = <1>; + #reset-cells = <1>; + }; + }; + + +Reset node declaration +============================================== +The reset node is declared in a parent child relationship. The main parent +is the PRCM module which contains the base address. The first child within +the reset parent declares the target modules reset name. This is followed by +the control and status offsets. + +Within the first reset child node is a secondary child node which declares the +reset signal of interest. Under this node the control and status bits +are declared. These bits declare the bit mask for the target reset. + + +Required properties: +reg - This is the register offset from the PRCM parent. + This must be declared as: + + reg = <control register offset>, + <status register offset>; + +control-bit - This is the bit within the register which controls the reset + of the target module. This is declared as a bit mask for the register. +status-bit - This is the bit within the register which contains the status of + the reset of the target module. + This is declared as a bit mask for the register. + +example: +&prm_resets { + dsp_rstctrl { + reg = <0x1c00>, + <0x1c04>; + + dsp_reset: dsp_reset { + control-bit = <0x01>; + status-bit = <0x01>; + }; + }; +}; + + + +Client Node Declaration +============================================== +This is the consumer of the parent node to declare what resets this +particular module is interested in. + +example: + src: src at 55082000 { + resets = <&reset_src phandle>; + reset-names = "<reset_name>"; + }; + +Required Properties: +reset_src - This is the parent DT entry for the reset controller +phandle - This is the phandle of the specific reset to be used by the clien + driver. +reset-names - This is the reset name of module that the device driver + needs to be able to reset. This value must correspond to a value within + the reset controller array. + +example: +resets = <&prm_resets &dsp_mmu_reset>; +reset-names = "dsp_mmu_reset";
Signature · 1 line
1.7.9.5
PATCH v3 2/6 dt: TI: Describe the ti reset DT entries
Documentation/devicetree/bindings/reset/ti,reset.txt line 39
Quoted 12 lines · Dan Murphy
Documentation/devicetree/bindings/reset/ti,reset.txt new file +12
-0+ +example: + prm: prm at 4ae06000 { + compatible = "ti,omap5-prm"; + reg = <0x4ae06000 0x3000>; + + prm_resets: resets { + #address-cells = <1>; + #size-cells = <1>; + #reset-cells = <1>; + }; + };
Probably the way to go with mapping various PRCM registers to separate drivers is to provide one or more syscon entries like we already have for the SCM. You may want to coordinate this with Tero.
Regards,
Tony
PATCH v3 2/6 dt: TI: Describe the ti reset DT entries
In reply to Dan MurphyHi Dan,
Quoted 1 line · Dan
Describe the TI reset DT entries for TI SoC's.
The document definitely needs some cleanup. Here are comments from a quick glance. In any case, I think this binding will change as you address Tony's comments.
Quoted 29 lines
Signed-off-by Dan Murphy <[email protected]>---
v3 - Changed Headline no other changes
.../devicetree/bindings/reset/ti,reset.txt | 103 ++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 Documentation/devicetree/bindings/reset/ti,reset.txtDocumentation/devicetree/bindings/reset/ti,reset.txt new file +13
-0@@ -0,0 +1,103 @@ +Texas Instruments Reset Controller +====================================== +Please also refer to reset.txt in this directory for common reset +controller binding usage. + +Specifying the reset entries for the IP module +============================================== +Parent module: +This is the module node that contains the reset registers and bits. + +example: + prcm_resets: resets { + compatible = "ti,dra7-resets";
Documentation/devicetree/bindings/reset/ti,reset.txt line 23
Quoted 10 lines
Documentation/devicetree/bindings/reset/ti,reset.txt new file +10
-0+ #reset-cells = <1>; + }; + +Required parent properties: +- compatible : Should be one of, + "ti,omap4-prm" for OMAP4 PRM instances + "ti,omap5-prm" for OMAP5 PRM instances + "ti,dra7-prm" for DRA7xx PRM instances + "ti,am4-prcm" for AM43xx PRCM instances + "ti,am3-prcm" for AM33xx PRCM instances
I am not sure if this belongs to the reset driver bindings, as the PRM and CM nodes are defined at the SoC level. This document should only be describing the reset nodes bindings.
Also, please list all the required node properties first, before you can give an example.
Documentation/devicetree/bindings/reset/ti,reset.txt line 27
Quoted 4 lines
Documentation/devicetree/bindings/reset/ti,reset.txt new file +4
-0+ +Required child reset property: +- compatible : Should be + "resets" for All TI SoCs
There is no "compatible" child property. This should have been the name of the node, right? Also, please list all the properties required under this node like #address-cells, #reset-cells etc.
Documentation/devicetree/bindings/reset/ti,reset.txt line 36
Quoted 9 lines
Documentation/devicetree/bindings/reset/ti,reset.txt new file +9
-0+ +example: + prm: prm at 4ae06000 { + compatible = "ti,omap5-prm"; + reg = <0x4ae06000 0x3000>; + + prm_resets: resets { + #address-cells = <1>; + #size-cells = <1>;
This is wrong. You haven't corrected this from v2. The reg field is used to give the offsets for control register and status register, and does not use any size fields.
Documentation/devicetree/bindings/reset/ti,reset.txt line 47
Quoted 11 lines
Documentation/devicetree/bindings/reset/ti,reset.txt new file +11
-0+ #reset-cells = <1>; + }; + }; + + +Reset node declaration +============================================== +The reset node is declared in a parent child relationship. The main parent +is the PRCM module which contains the base address. The first child within +the reset parent declares the target modules reset name. This is followed by +the control and status offsets.
This is not clear. This is the case with each child node, which is describing a particular reset domain, and then the individual resets themselves are defined as child nodes of this reset domain child node.
Quoted 14 lines
Documentation/devicetree/bindings/reset/ti,reset.txt new file +14
-0+ +Within the first reset child node is a secondary child node which declares the +reset signal of interest. Under this node the control and status bits +are declared. These bits declare the bit mask for the target reset. + + +Required properties: +reg - This is the register offset from the PRCM parent. + This must be declared as: + + reg = <control register offset>, + <status register offset>; + +control-bit - This is the bit within the register which controls the reset
Documentation/devicetree/bindings/reset/ti,reset.txt line 71
Quoted 10 lines
Documentation/devicetree/bindings/reset/ti,reset.txt new file +10
-0+ of the target module. This is declared as a bit mask for the register. +status-bit - This is the bit within the register which contains the status of + the reset of the target module. + This is declared as a bit mask for the register. + +example: +&prm_resets { + dsp_rstctrl { + reg = <0x1c00>, + <0x1c04>;
I guess both these entries can be defined in one line.
Quoted 18 lines
Documentation/devicetree/bindings/reset/ti,reset.txt new file +18
-0+ + dsp_reset: dsp_reset { + control-bit = <0x01>; + status-bit = <0x01>; + }; + }; +}; + + + +Client Node Declaration +============================================== +This is the consumer of the parent node to declare what resets this +particular module is interested in. + +example: + src: src at 55082000 { + resets = <&reset_src phandle>;
Documentation/devicetree/bindings/reset/ti,reset.txt line 99
Quoted 10 lines
Documentation/devicetree/bindings/reset/ti,reset.txt new file +10
-0+ reset-names = "<reset_name>"; + }; + +Required Properties: +reset_src - This is the parent DT entry for the reset controller +phandle - This is the phandle of the specific reset to be used by the clien + driver. +reset-names - This is the reset name of module that the device driver + needs to be able to reset. This value must correspond to a value within + the reset controller array.
The reset-names is optional as per the reset.txt, so it is not clear if this is mandatory for this binding.
regards
Suman
Quoted 5 lines
Documentation/devicetree/bindings/reset/ti,reset.txt new file +4
-0+ +example: +resets = <&prm_resets &dsp_mmu_reset>; +reset-names = "dsp_mmu_reset";
PATCH v3 3/6 ARM: dts: am33xx: Add prcm_resets node
In reply to Dan MurphyAdd the prcm_resets node to the prcm parent node.
Add the am33xx_resets file to define the
am33xx reset lines that are handled by this reset
framework.
---
v3 - No changes
arch/arm/boot/dts/am33xx-resets.dtsi | 42 ++++++++++++++++++++++++++++++++++ arch/arm/boot/dts/am33xx.dtsi | 7 ++++++ 2 files changed, 49 insertions(+) create mode 100644 arch/arm/boot/dts/am33xx-resets.dtsi
arch/arm/boot/dts/am33xx-resets.dtsi new file +42 -0
@@ -0,0 +1,42 @@ +/* + * Device Tree Source for AM33XX reset data + * + * Copyright (C) 2014 Texas Instruments, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +&prcm_resets { + gfx_rstctrl { + reg = <0x1104>, + <0x1114>; + + gfx_reset: gfx_reset { + control-bit = <0x01>; + status-bit = <0x01>; + }; + }; + + per_rstctrl { + reg = <0xD00>, + <0xD0C>; + + iva_reset: iva_reset { + control-bit = <0x04>; + status-bit = <0x10>; + }; + }; + + device_rstctrl { + reg = <0xf00>, + <0xf08>; + + device_reset: device_reset { + control-bit = <0x01>; + status-bit = <0x01>; + }; + }; + +};
arch/arm/boot/dts/am33xx.dtsi +7 -0
@@ -117,6 +117,12 @@ prcm_clockdomains: clockdomains { }; + + prcm_resets: resets { + #address-cells = <1>; + #size-cells = <1>; + #reset-cells = <1>; + }; }; scrm: scrm at 44e10000 {
@@ -834,3 +840,4 @@ }; /include/ "am33xx-clocks.dtsi" +/include/ "am33xx-resets.dtsi"
Signature · 1 line
1.7.9.5
PATCH v3 3/6 ARM: dts: am33xx: Add prcm_resets node
Hi Dan,
Quoted 38 lines · Dan
Add the
prcm_resetsnode to the prcm parent node.Add the
am33xx_resetsfile to define the
am33xx reset lines that are handled by this reset
framework.Signed-off-by Dan Murphy <[email protected]>---
v3 - No changes
arch/arm/boot/dts/am33xx-resets.dtsi | 42 ++++++++++++++++++++++++++++++++++ arch/arm/boot/dts/am33xx.dtsi | 7 ++++++ 2 files changed, 49 insertions(+) create mode 100644 arch/arm/boot/dts/am33xx-resets.dtsiarch/arm/boot/dts/am33xx-resets.dtsi new file +16
-0@@ -0,0 +1,42 @@ +/* + * Device Tree Source for AM33XX reset data + * + * Copyright (C) 2014 Texas Instruments, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +&prcm_resets { + gfx_rstctrl { + reg = <0x1104>, + <0x1114>; + + gfx_reset: gfx_reset {
arch/arm/boot/dts/am33xx-resets.dtsi line 26
Quoted 10 lines
arch/arm/boot/dts/am33xx-resets.dtsi new file +10
-0+ control-bit = <0x01>; + status-bit = <0x01>; + }; + }; + + per_rstctrl { + reg = <0xD00>, + <0xD0C>; + + iva_reset: iva_reset {
There is no IVA on AM33xx, this should be corrected to reflect the PRU_ICSS.
arch/arm/boot/dts/am33xx-resets.dtsi line 30
Quoted 4 lines
arch/arm/boot/dts/am33xx-resets.dtsi new file +4
-0+ control-bit = <0x04>; + status-bit = <0x10>; + }; + };
Not defining the WkupM3 reset control?
Quoted 17 lines
arch/arm/boot/dts/am33xx-resets.dtsi new file +12
-0+ + device_rstctrl { + reg = <0xf00>, + <0xf08>; + + device_reset: device_reset { + control-bit = <0x01>; + status-bit = <0x01>; + }; + }; + +};arch/arm/boot/dts/am33xx.dtsi +0
-0
arch/arm/boot/dts/am33xx.dtsi line 123
Quoted 7 lines
arch/arm/boot/dts/am33xx.dtsi +4
-0@@ -117,6 +117,12 @@ prcm_clockdomains: clockdomains { }; + + prcm_resets: resets { + #address-cells = <1>; + #size-cells = <1>;
Should be corrected as per comment on DT bindings.
regards
Suman
Quoted 11 lines
arch/arm/boot/dts/am33xx.dtsi +3
-0+ #reset-cells = <1>; + }; }; scrm: scrm at 44e10000 {@@ -834,3 +840,4 @@ }; /include/ "am33xx-clocks.dtsi" +/include/ "am33xx-resets.dtsi"
PATCH v3 4/6 ARM: dts: am4372: Add prcm_resets node
In reply to Dan MurphyAdd the prcm_resets node to the prcm parent node.
Add the am34xx_resets file to define the
am34xx reset lines that are handled by this reset
framework.
---
v3 - No changes
arch/arm/boot/dts/am4372.dtsi | 7 +++++ arch/arm/boot/dts/am43xx-resets.dtsi | 52 ++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 arch/arm/boot/dts/am43xx-resets.dtsi
arch/arm/boot/dts/am4372.dtsi +7 -0
@@ -88,6 +88,12 @@ prcm_clockdomains: clockdomains { }; + + prcm_resets: resets { + #address-cells = <1>; + #size-cells = <1>; + #reset-cells = <1>; + }; }; scrm: scrm at 44e10000 {
@@ -892,3 +898,4 @@ }; /include/ "am43xx-clocks.dtsi" +/include/ "am43xx-resets.dtsi"
arch/arm/boot/dts/am43xx-resets.dtsi new file +52 -0
52 lines not on this page · raw message
Signature · 1 line
1.7.9.5
PATCH v3 4/6 ARM: dts: am4372: Add prcm_resets node
Hi Dan,
Quoted 21 lines · Dan
Add the
prcm_resetsnode to the prcm parent node.Add the
am34xx_resetsfile to define the
am34xx reset lines that are handled by this reset
framework.Signed-off-by Dan Murphy <[email protected]>---
v3 - No changes
arch/arm/boot/dts/am4372.dtsi | 7 +++++ arch/arm/boot/dts/am43xx-resets.dtsi | 52 ++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 arch/arm/boot/dts/am43xx-resets.dtsiarch/arm/boot/dts/am4372.dtsi +0
-0
arch/arm/boot/dts/am4372.dtsi line 94
Quoted 7 lines
arch/arm/boot/dts/am4372.dtsi +4
-0@@ -88,6 +88,12 @@ prcm_clockdomains: clockdomains { }; + + prcm_resets: resets { + #address-cells = <1>; + #size-cells = <1>;
Should be corrected as per comment on DT bindings.
Quoted 45 lines
arch/arm/boot/dts/am4372.dtsi +3
-0+ #reset-cells = <1>; + }; }; scrm: scrm at 44e10000 {@@ -892,3 +898,4 @@ }; /include/ "am43xx-clocks.dtsi" +/include/ "am43xx-resets.dtsi"arch/arm/boot/dts/am43xx-resets.dtsi new file +29
-0@@ -0,0 +1,52 @@ +/* + * Device Tree Source for AM43XX reset data + * + * Copyright (C) 2014 Texas Instruments, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +&prcm_resets { + icss_rstctrl { + reg = <0x810>, + <0x814>; + + icss_reset: icss_reset { + control-bit = <0x01>; + status-bit = <0x01>; + }; + }; + + gfx_rstctrl { + reg = <0x410>, + <0x414>; + + gfx_reset: gfx_reset { + control-bit = <0x01>; + status-bit = <0x01>; + };
arch/arm/boot/dts/am43xx-resets.dtsi line 39
Quoted 10 lines
arch/arm/boot/dts/am43xx-resets.dtsi new file +10
-0+ }; + + per_rstctrl { + reg = <0x2010>, + <0x2014>; + + iva_reset: iva_reset { + control-bit = <0x01>; + status-bit = <0x01>; + };
There's no IVA on AM4372. Looking at the offset, it looks like you were defining this for the WkupM3, in which case you got the initial node name wrong. The PER rstctrl has the reset management for PRU-ICSS, so you also need to correct the icss_rstctrl accordingly.
regards
Suman
Quoted 14 lines
arch/arm/boot/dts/am43xx-resets.dtsi new file +13
-0+ }; + + device_rstctrl { + reg = <0x4000>, + <0x4004>; + + device_reset: device_reset { + control-bit = <0x01>; + status-bit = <0x01>; + }; + }; + +};
PATCH v3 5/6 ARM: dts: dra7: Add prm_resets node
In reply to Dan MurphyAdd the prcm_resets node to the prm parent node.
Add the draxx_resets file to define the
dra7xx reset lines that are handled by this reset
framework.
---
v3 - No changes
arch/arm/boot/dts/dra7.dtsi | 7 +++ arch/arm/boot/dts/dra7xx-resets.dtsi | 82 ++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 arch/arm/boot/dts/dra7xx-resets.dtsi
arch/arm/boot/dts/dra7.dtsi +7 -0
@@ -93,6 +93,12 @@ prm_clockdomains: clockdomains { }; + + prm_resets: resets { + #address-cells = <1>; + #size-cells = <1>; + #reset-cells = <1>; + }; }; cm_core_aon: cm_core_aon at 4a005000 {
@@ -998,3 +1004,4 @@ }; /include/ "dra7xx-clocks.dtsi" +/include/ "dra7xx-resets.dtsi"
arch/arm/boot/dts/dra7xx-resets.dtsi new file +82 -0
82 lines not on this page · raw message
Signature · 1 line
1.7.9.5
PATCH v3 5/6 ARM: dts: dra7: Add prm_resets node
Hi Dan,
Quoted 21 lines · Dan
Add the
prcm_resetsnode to the prm parent node.Add the
draxx_resetsfile to define the
dra7xx reset lines that are handled by this reset
framework.Signed-off-by Dan Murphy <[email protected]>---
v3 - No changes
arch/arm/boot/dts/dra7.dtsi | 7 +++ arch/arm/boot/dts/dra7xx-resets.dtsi | 82 ++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 arch/arm/boot/dts/dra7xx-resets.dtsiarch/arm/boot/dts/dra7.dtsi +0
-0
arch/arm/boot/dts/dra7.dtsi line 99
Quoted 7 lines
arch/arm/boot/dts/dra7.dtsi +4
-0@@ -93,6 +93,12 @@ prm_clockdomains: clockdomains { }; + + prm_resets: resets { + #address-cells = <1>; + #size-cells = <1>;
Should be corrected as per comment on DT bindings.
Quoted 51 lines
arch/arm/boot/dts/dra7.dtsi +3
-0+ #reset-cells = <1>; + }; }; cm_core_aon: cm_core_aon at 4a005000 {@@ -998,3 +1004,4 @@ }; /include/ "dra7xx-clocks.dtsi" +/include/ "dra7xx-resets.dtsi"arch/arm/boot/dts/dra7xx-resets.dtsi new file +35
-0@@ -0,0 +1,82 @@ +/* + * Device Tree Source for DRA7XX reset data + * + * Copyright (C) 2014 Texas Instruments, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +&prm_resets { + dsp_rstctrl { + reg = <0x410>, + <0x414>; + + dsp_reset: dsp_reset { + control-bit = <0x01>; + status-bit = <0x01>; + }; + + dsp_mmu_reset: dsp_mmu_reset { + control-bit = <0x02>; + status-bit = <0x02>; + }; + }; + + ipu_rstctrl { + reg = <0x510>, + <0x514>; + + ipu_cpu0_reset: ipu_cpu0_reset { + control-bit = <0x01>; + status-bit = <0x01>; + }; +
arch/arm/boot/dts/dra7xx-resets.dtsi line 45
Quoted 10 lines
arch/arm/boot/dts/dra7xx-resets.dtsi new file +10
-0+ ipu_cpu1_reset: ipu_cpu1_reset { + control-bit = <0x02>; + status-bit = <0x02>; + }; + + ipu_mmu_reset: ipu_mmu_reset { + control-bit = <0x04>; + status-bit = <0x04>; + }; + };
There are two IPUs on DRA7. Which one is this node for? And please add the other reset node for the other IPU as well.
arch/arm/boot/dts/dra7xx-resets.dtsi line 54
Quoted 9 lines
arch/arm/boot/dts/dra7xx-resets.dtsi new file +9
-0+ + iva_rstctrl { + reg = <0xf10>, + <0xf14>; + + iva_reset: iva_reset { + control-bit = <0x01>; + status-bit = <0x01>; + };
IVA also has three resets, one for logic and two for the sequencers. You are describing only one of them.
Quoted 5 lines
arch/arm/boot/dts/dra7xx-resets.dtsi new file +5
-0+ }; + + pcie_rstctrl { + reg = <0x1310>, + <0x1314>;
arch/arm/boot/dts/dra7xx-resets.dtsi line 69
Quoted 10 lines
arch/arm/boot/dts/dra7xx-resets.dtsi new file +10
-0+ + pcie1_reset: pcie1_reset { + control-bit = <0x01>; + status-bit = <0x01>; + }; + + pcie2_reset: pcie2_reset { + control-bit = <0x01>; + status-bit = <0x01>; + };
Copy-paste error? Both pcie resets cannot have the same control and status bits.
regards
Suman
Quoted 14 lines
arch/arm/boot/dts/dra7xx-resets.dtsi new file +13
-0+ }; + + device_rstctrl { + reg = <0x1D00>, + <0x1D04>; + + device_reset: device_reset { + control-bit = <0x01>; + status-bit = <0x01>; + }; + }; + +};
PATCH v3 6/6 ARM: dts: omap5: Add prm_resets node
In reply to Dan MurphyAdd the prm_resets node to the prm parent node.
Add the omap54xx_resets file to define the
omap5 reset lines that are handled by this reset
framework.
---
v3 - No changes
arch/arm/boot/dts/omap5.dtsi | 7 ++++ arch/arm/boot/dts/omap54xx-resets.dtsi | 66 ++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 arch/arm/boot/dts/omap54xx-resets.dtsi
arch/arm/boot/dts/omap5.dtsi +7 -0
@@ -139,6 +139,12 @@ prm_clockdomains: clockdomains { }; + + prm_resets: resets { + #address-cells = <1>; + #size-cells = <1>; + #reset-cells = <1>; + }; }; cm_core_aon: cm_core_aon at 4a004000 {
@@ -989,3 +995,4 @@ }; /include/ "omap54xx-clocks.dtsi" +/include/ "omap54xx-resets.dtsi"
arch/arm/boot/dts/omap54xx-resets.dtsi new file +66 -0
66 lines not on this page · raw message
Signature · 1 line
1.7.9.5
PATCH v3 6/6 ARM: dts: omap5: Add prm_resets node
Hi Dan,
Quoted 21 lines · Dan
Add the
prm_resetsnode to the prm parent node.Add the
omap54xx_resetsfile to define the
omap5 reset lines that are handled by this reset
framework.Signed-off-by Dan Murphy <[email protected]>---
v3 - No changes
arch/arm/boot/dts/omap5.dtsi | 7 ++++ arch/arm/boot/dts/omap54xx-resets.dtsi | 66 ++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 arch/arm/boot/dts/omap54xx-resets.dtsiarch/arm/boot/dts/omap5.dtsi +0
-0
arch/arm/boot/dts/omap5.dtsi line 145
Quoted 7 lines
arch/arm/boot/dts/omap5.dtsi +4
-0@@ -139,6 +139,12 @@ prm_clockdomains: clockdomains { }; + + prm_resets: resets { + #address-cells = <1>; + #size-cells = <1>;
Should be corrected as per comment on DT bindings.
Quoted 51 lines
arch/arm/boot/dts/omap5.dtsi +3
-0+ #reset-cells = <1>; + }; }; cm_core_aon: cm_core_aon at 4a004000 {@@ -989,3 +995,4 @@ }; /include/ "omap54xx-clocks.dtsi" +/include/ "omap54xx-resets.dtsi"arch/arm/boot/dts/omap54xx-resets.dtsi new file +35
-0@@ -0,0 +1,66 @@ +/* + * Device Tree Source for OMAP5 reset data + * + * Copyright (C) 2014 Texas Instruments, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +&prm_resets { + dsp_rstctrl { + reg = <0x1c00>, + <0x1c04>; + + dsp_reset: dsp_reset { + control-bit = <0x01>; + status-bit = <0x01>; + }; + + dsp_mmu_reset: dsp_mmu_reset { + control-bit = <0x02>; + status-bit = <0x02>; + }; + }; + + ipu_rstctrl { + reg = <0x910>, + <0x914>; + + ipu_cpu0_reset: ipu_cpu0_reset { + control-bit = <0x01>; + status-bit = <0x01>; + }; +
arch/arm/boot/dts/omap54xx-resets.dtsi line 45
Quoted 10 lines
arch/arm/boot/dts/omap54xx-resets.dtsi new file +10
-0+ ipu_cpu1_reset: ipu_cpu1_reset { + control-bit = <0x02>; + status-bit = <0x02>; + }; + + ipu_mmu_reset: ipu_mmu_reset { + control-bit = <0x04>; + status-bit = <0x04>; + }; + };
Missing reset node for SGX/GFX?
arch/arm/boot/dts/omap54xx-resets.dtsi line 54
Quoted 9 lines
arch/arm/boot/dts/omap54xx-resets.dtsi new file +9
-0+ + iva_rstctrl { + reg = <0x1210>, + <0x1214>; + + iva_reset: iva_reset { + control-bit = <0x01>; + status-bit = <0x01>; + };
IVA also has three resets, one for logic and two for the sequencers. You are describing only one of them.
regards
Suman
Quoted 13 lines
arch/arm/boot/dts/omap54xx-resets.dtsi new file +12
-0+ }; + + device_rstctrl { + reg = <0x1c00>, + <0x1c04>; + + device_reset: device_reset { + control-bit = <0x01>; + status-bit = <0x01>; + }; + }; +};
PATCH v3 1/6 drivers: reset: TI: SoC reset controller support.
In reply to Dan MurphyHi,
Quoted 155 lines · Dan Murphy
The TI SoC reset controller support utilizes the reset controller framework to give device drivers or function drivers a common set of APIs to call to reset a module.
The reset-ti is a common interface to the reset framework.
The register data is retrieved during initialization
of the reset driver through the reset-ti-data
file. The array of data is associated with the compatible from the respective DT entry.Once the data is available then this is derefenced within the common interface.
The device driver has the ability to assert, deassert or perform a complete reset.
This code was derived from previous work by Rajendra Nayak and Afzal Mohammed. The code was changed to adopt to the reset core and abstract away the SoC information.
Signed-off-by Dan Murphy <[email protected]>---
v3 - Resolved comments from v2. To many to call out here - https://patchwork.kernel.org/patch/4116941/
drivers/reset/Kconfig | 9 ++ drivers/reset/Makefile | 1 + drivers/reset/reset-ti.c | 373 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 383 insertions(+) create mode 100644 drivers/reset/reset-ti.cdrivers/reset/Kconfig +9
-0@@ -12,4 +12,13 @@ menuconfig RESET_CONTROLLER If unsure, say no. +config RESET_TI + depends on RESET_CONTROLLER && ARCH_OMAP || COMPILE_TEST + bool "TI reset controller" + help + Reset controller support for TI SoC's + + Reset controller found in TI's AM series of SoC's like + AM335x and AM43x and OMAP SoC's like OMAP5 and DRA7 + source "drivers/reset/sti/Kconfig"drivers/reset/Makefile +1
-0@@ -1,4 +1,5 @@ obj-$(CONFIG_RESET_CONTROLLER) += core.o obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o +obj-$(CONFIG_RESET_TI) += reset-ti.o obj-$(CONFIG_ARCH_STI) += sti/drivers/reset/reset-ti.c new file +90
-0@@ -0,0 +1,373 @@ +/* +* reset-ti.c - PRCM reset driver for TI SoC's +* +* Copyright (C) 2014 Texas Instruments Incorporated - http://www.ti.com +* +* Author: Dan Murphy <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +*/ + +#include <linux/delay.h> +#include <linux/device.h> +#include <linux/err.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of_address.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/reset.h> +#include <linux/reset-controller.h> +#include <linux/slab.h> +#include <linux/spinlock.h> + +#define DRIVER_NAME "prcm_reset_ti" +#define MAX_RESET_SIGNALS 255 + +/** +* struct ti_reset_reg_data - Structure of the reset register information +* for a particular SoC. +* @rstctrl_offs: This is the reset control offset value from +* from the parent reset node. +* @rstst_offs: This is the reset status offset value from +* from the parent reset node. +* @rstctrl_bit: This is the reset control bit for the module. +* @rstst_bit: This is the reset status bit for the module. +* +* Longer description of this structure. +*/ +struct ti_reset_reg_data { +phandle handle; +u32 rstctrl_offs; +u32 rstst_offs; +u32 rstctrl_bit; +u32 rstst_bit; +}; + +/** +* struct ti_reset_data - Structure that contains the reset register data +* as well as the total number of resets for a particular SoC. +* @ti_data: Pointer to this structure to be dereferenced +* @reg_data: Pointer to the register data structure. +* @rcdev: Reset controller device instance +* @dev: Pointer to the devive structure +* @ti_reg_data: Array of register data. Only reset signal with valid +* phandles will be stored in this array. +* @reg_base: Parent register base address +* @lock: Spinlock for accessing the registers +* @nr_resets: Total number of resets for the SoC in the reset array. +* +* This structure contains a pointer to the register data and the modules +* register base. The number of resets and reset controller device data is +* stored within this structure. +* +*/ +struct ti_reset_data { +struct ti_reset_data *ti_data; +struct ti_reset_reg_data *reg_data; +struct reset_controller_dev rcdev; +struct device *dev; +struct ti_reset_reg_data ti_reg_data[MAX_RESET_SIGNALS ]; +void __iomem *reg_base; +spinlock_t lock; +int nr_resets; +}; + +static int ti_reset_wait_on_reset (struct reset_controller_dev *rcdev, +unsigned long id) +{ +struct ti_reset_data *reset_data =container_of (rcdev, + struct ti_reset_data, + rcdev); +void __iomem *status_reg = reset_data->reg_base; +u32 bit_mask =0 ; +u32 val =0 ; +unsigned long flags; +int i =1000 ; +int ret =0 ;
drivers/reset/reset-ti.c line 100
Quoted 10 lines
drivers/reset/reset-ti.c new file +10
-0+ +spin_lock_irqsave (&reset_data->lock, flags); +/* Clear the reset status bit to reflect the current status */ + status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs; + bit_mask = reset_data->ti_reg_data[id].rstst_bit; + +do { + val =readl (status_reg); + }while (--i && (val & (1 << bit_mask))); +
Is there no better way to detect timeout than counting down from some magic number? Does the reference manual give any hints about how fast the reset should be completed, so the timeout can be hinged on some real microseconds rather than a number of CPU loops?
Quoted 11 lines
drivers/reset/reset-ti.c new file +10
-0+if (i ==0 && val & (1 << bit_mask)) { +dev_err (reset_data->dev,"%s: Reset failed\n" , __func__); + ret = -EIO ; + } + +spin_unlock_irqrestore (&reset_data->lock, flags); +return ret; +} + +static int ti_reset_assert(struct reset_controller_dev *rcdev,
drivers/reset/reset-ti.c line 120
Quoted 10 lines
drivers/reset/reset-ti.c new file +10
-0+ unsigned long id) +{ +struct ti_reset_data *reset_data =container_of (rcdev, + struct ti_reset_data, + rcdev); +void __iomem *reg; +void __iomem *status_reg; +u32 status_bit =0 ; +u32 bit_mask =0 ; +u32 val =0 ;
Useless initialization!
Quoted 24 lines
drivers/reset/reset-ti.c new file +23
-0+unsigned long flags; + +spin_lock_irqsave (&reset_data->lock, flags); + +/* Clear the reset status bit to reflect the current status */ + status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs; + status_bit = reset_data->ti_reg_data[id].rstst_bit; +writel (1 << status_bit, status_reg); + + reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstctrl_offs; + bit_mask = reset_data->ti_reg_data[id].rstctrl_bit; + val =readl (reg); +if (!(val & bit_mask)) { + val |= bit_mask; +writel (val, reg); + } + +spin_unlock_irqrestore (&reset_data->lock, flags); +return 0 ; +} + +static int ti_reset_deassert (struct reset_controller_dev *rcdev, +unsigned long id)
drivers/reset/reset-ti.c line 153
Quoted 10 lines
drivers/reset/reset-ti.c new file +10
-0+{ + +struct ti_reset_data *reset_data =container_of (rcdev, + struct ti_reset_data, + rcdev); +void __iomem *reg; +void __iomem *status_reg; +u32 status_bit =0 ; +u32 bit_mask =0 ; +u32 val =0 ;
dto.
Quoted 78 lines
drivers/reset/reset-ti.c new file +77
-0+unsigned long flags; + +spin_lock_irqsave (&reset_data->lock, flags); + +/* Clear the reset status bit to reflect the current status */ + status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs; + status_bit = reset_data->ti_reg_data[id].rstst_bit; +writel (1 << status_bit, status_reg); + + reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstctrl_offs; + bit_mask = reset_data->ti_reg_data[id].rstctrl_bit; + val =readl (reg); +if (val & bit_mask) { + val &= ~bit_mask; +writel (val, reg); + } + +spin_unlock_irqrestore (&reset_data->lock, flags); + +return 0 ; +} + +static int ti_reset_reset (struct reset_controller_dev *rcdev, +unsigned long id) +{ +ti_reset_assert (rcdev, id); +ti_reset_deassert (rcdev, id); + +return ti_reset_wait_on_reset (rcdev, id); +} + +static int ti_reset_xlate (struct reset_controller_dev *rcdev, +const struct of_phandle_args *reset_spec) +{ + +struct ti_reset_data *reset_data =container_of (rcdev, + struct ti_reset_data, + rcdev); +struct device_node *dev_node; +int i; + +if (WARN_ON (reset_spec->args_count != rcdev->of_reset_n_cells)) +return -EINVAL ; + +/* Verify that the phandle exists */ + dev_node =of_find_node_by_phandle ((phandle ) reset_spec->args[0 ]); +if (!dev_node) { +dev_err (reset_data->dev, +"%s: Cannot find phandle node\n" , + __func__); +return -EINVAL ; + } + +for (i =0 ; i <= reset_data->nr_resets; i++) { +if (reset_data->ti_reg_data[i].handle == dev_node->phandle) +return i; + } + +return -EINVAL ; +} + +static struct reset_control_ops ti_reset_ops = { + .reset = ti_reset_reset, + .assert = ti_reset_assert, + .deassert = ti_reset_deassert, +}; + +static int ti_reset_populate_child (struct ti_reset_data *reset_data, +struct device_node *reset_child, +u32 ctrl_offs,u32 status_offs) +{ +struct device_node *next_reset_child =NULL ; +int i = reset_data->nr_resets; +int ret; +u32 ctrl_bit; + +while ((next_reset_child =of_get_next_child (reset_child, next_reset_child))) {
drivers/reset/reset-ti.c line 240
Quoted 10 lines
drivers/reset/reset-ti.c new file +10
-0+if (next_reset_child->phandle) { +/* Get the bits of each sub-reset */ + ret =of_property_read_u32 (next_reset_child,"control-bit" , + &ctrl_bit); +if (ret <0 ) { +dev_err (reset_data->dev, +"%s: No entry in %s for rstctrl_offs\n" , + __func__, next_reset_child->name); + +return -ENODEV ;
return the error code from of_property_read()?
drivers/reset/reset-ti.c line 250
Quoted 11 lines
drivers/reset/reset-ti.c new file +10
-0+ } + + ret =of_property_read_u32 (next_reset_child,"status-bit" , + &reset_data->ti_reg_data[i].rstst_bit); +if (ret <0 ) { +dev_err (reset_data->dev, +"%s: No entry in %s for rstst_offs\n" , + __func__, next_reset_child->name); + +return -ENODEV ;
dto.
Quoted 10 lines
drivers/reset/reset-ti.c new file +9
-0+ } + + reset_data->ti_reg_data[i].rstctrl_offs = ctrl_offs; + reset_data->ti_reg_data[i].rstst_offs = status_offs; + reset_data->ti_reg_data[i].handle = next_reset_child->phandle; + i++; + } + }; +
drivers/reset/reset-ti.c line 269
Quoted 10 lines
drivers/reset/reset-ti.c new file +10
-0+ reset_data->nr_resets = i; + +return 0 ; +} + +static int ti_reset_get_of_data (struct ti_reset_data *reset_data, +struct device_node *dev_node) +{ +struct device_node *reset_child =NULL ; +int ret = -EINVAL ;
useless initialization.
drivers/reset/reset-ti.c line 273
Quoted 5 lines
drivers/reset/reset-ti.c new file +4
-0+u32 ctrl_offs, status_offs; + +/* Loop through all the children and populate a lookup array */ +while ((reset_child =of_get_next_child (dev_node, reset_child))) {
I'd prefer:
while ((reset_child = of_get_next_child(dev_node,reset_child)) != NULL) { to make it abundantly clear that the = is not meant as a comparison operator but an assignment.
Quoted 33 lines
drivers/reset/reset-ti.c new file +32
-0+ ret =of_property_read_u32_index (reset_child,"reg" ,0 , + &ctrl_offs); +if (ret) +return ret; + + ret =of_property_read_u32_index (reset_child,"reg" ,1 , + &status_offs); +if (ret) +return ret; + + ret =ti_reset_populate_child (reset_data, reset_child, + ctrl_offs, status_offs); +if (ret) +return ret; + }; + +return 0 ; +} + +static int ti_reset_probe (struct platform_device *pdev) +{ +struct device_node *resets; +struct ti_reset_data *reset_data; +struct resource *res; +int ret; + + reset_data =devm_kzalloc (&pdev->dev,sizeof (*reset_data),GFP_KERNEL ); +if (!reset_data) +return -ENOMEM ; + + resets =of_find_node_by_name (NULL ,"resets" ); +if (!resets) {
drivers/reset/reset-ti.c line 315
Quoted 10 lines
drivers/reset/reset-ti.c new file +10
-0+dev_err (&pdev->dev,"%s: missing 'resets' child node.\n" , + __func__); +return -EINVAL ; + } + + res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); + reset_data->reg_base =devm_ioremap_resource (&pdev->dev, res); +if (IS_ERR (reset_data->reg_base)) { +dev_err (&pdev->dev,"%s: Cannot map reset parent.\n" , __func__); +return -ENODEV ;
return the error code from devm_ioremap_resource.
drivers/reset/reset-ti.c line 322
Quoted 8 lines
drivers/reset/reset-ti.c new file +7
-0+ } + +spin_lock_init (&reset_data->lock); + reset_data->dev = &pdev->dev; + ret =ti_reset_get_of_data (reset_data, resets); +if (ret) +return -EINVAL ;
return ret!
Quoted 12 lines
drivers/reset/reset-ti.c new file +11
-0+ reset_data->rcdev.owner =THIS_MODULE ; + reset_data->rcdev.of_node = resets; + reset_data->rcdev.ops = &ti_reset_ops; + + reset_data->rcdev.of_reset_n_cells =1 ; + reset_data->rcdev.of_xlate = &ti_reset_xlate; + + reset_data->ti_data = reset_data; + +platform_set_drvdata (pdev, reset_data); +
drivers/reset/reset-ti.c line 344
Quoted 10 lines
drivers/reset/reset-ti.c new file +10
-0+reset_controller_register (&reset_data->rcdev); + +return 0 ; +} + +static int ti_reset_remove (struct platform_device *pdev) +{ +struct ti_reset_data *reset_data; + + reset_data =platform_get_drvdata (pdev);
struct ti_reset_data *reset_data = platform_get_drvdata(pdev);
Quoted 4 lines
drivers/reset/reset-ti.c new file +3
-0+reset_controller_unregister (&reset_data->rcdev); + +return 0 ;
drivers/reset/reset-ti.c line 357
Quoted 10 lines
drivers/reset/reset-ti.c new file +10
-0+} + +static const struct of_device_id ti_reset_of_match[] = { + { .compatible ="ti,omap5-prm" }, + { .compatible ="ti,omap4-prm" }, + { .compatible ="ti,omap5-prm" }, + { .compatible ="ti,dra7-prm" }, + { .compatible ="ti,am4-prcm" }, + { .compatible ="ti,am3-prcm" }, + {},
useless comma after list terminator.
Without the comma an entry inadvertedly added after the terminator (e.g. due to a badly resolved merge conflict) will lead to a compile error while otherwise the new entry will silently be ignored.
Lothar Wa?mann
Signature · 9 lines
___________________________________________________________ Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Gesch?ftsf?hrer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996 www.karo-electronics.de | info at karo-electronics.de ___________________________________________________________
PATCH v3 1/6 drivers: reset: TI: SoC reset controller support.
In reply to Dan MurphyHi Dan,
Quoted 10 lines · Dan
The TI SoC reset controller support utilizes the reset controller framework to give device drivers or function drivers a common set of APIs to call to reset a module.
The reset-ti is a common interface to the reset framework.
The register data is retrieved during initialization of the reset driver through the reset-ti-data
file. The array of data is associated with the compatible from the respective DT entry.
Outdated commit description, this is no longer correct.
Quoted 7 lines
Once the data is available then this is derefenced within the common interface.
The device driver has the ability to assert, deassert or perform a complete reset.
Quoted 7 lines
This code was derived from previous work by Rajendra Nayak and Afzal Mohammed. The code was changed to adopt to the reset core and abstract away the SoC information.
Signed-off-by Dan Murphy <[email protected]>---
v3 - Resolved comments from v2. To many to call out here - https://patchwork.kernel.org/patch/4116941/
Please add a cover letter for the series next time and make sure you cc the reset driver maintainer.
Quoted 50 lines
drivers/reset/Kconfig | 9 ++ drivers/reset/Makefile | 1 + drivers/reset/reset-ti.c | 373 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 383 insertions(+) create mode 100644 drivers/reset/reset-ti.cdrivers/reset/Kconfig +9
-0@@ -12,4 +12,13 @@ menuconfig RESET_CONTROLLER If unsure, say no. +config RESET_TI + depends on RESET_CONTROLLER && ARCH_OMAP || COMPILE_TEST + bool "TI reset controller" + help + Reset controller support for TI SoC's + + Reset controller found in TI's AM series of SoC's like + AM335x and AM43x and OMAP SoC's like OMAP5 and DRA7 + source "drivers/reset/sti/Kconfig"drivers/reset/Makefile +1
-0@@ -1,4 +1,5 @@ obj-$(CONFIG_RESET_CONTROLLER) += core.o obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o +obj-$(CONFIG_RESET_TI) += reset-ti.o obj-$(CONFIG_ARCH_STI) += sti/drivers/reset/reset-ti.c new file +9
-0@@ -0,0 +1,373 @@ +/* +* reset-ti.c - PRCM reset driver for TI SoC's +* +* Copyright (C) 2014 Texas Instruments Incorporated - http://www.ti.com +* +* Author: Dan Murphy <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as
drivers/reset/reset-ti.c line 19
Quoted 10 lines
drivers/reset/reset-ti.c new file +10
-0+* published by the Free Software Foundation. +*/ + +#include <linux/delay.h> +#include <linux/device.h> +#include <linux/err.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of_address.h>
This header is no longer needed, now that you are not using of_iomap.
drivers/reset/reset-ti.c line 28
Quoted 9 lines
drivers/reset/reset-ti.c new file +9
-0+#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/reset.h> +#include <linux/reset-controller.h> +#include <linux/slab.h> +#include <linux/spinlock.h> + +#define DRIVER_NAME "prcm_reset_ti" +#define MAX_RESET_SIGNALS 255
This sounds like a lot, I think you should reduce this to not waste memory. Start with a small number, and add a trace for increasing this if you run out of the current slots.
Quoted 31 lines
drivers/reset/reset-ti.c new file +31
-0+ +/** +* struct ti_reset_reg_data - Structure of the reset register information +* for a particular SoC. +* @rstctrl_offs: This is the reset control offset value from +* from the parent reset node. +* @rstst_offs: This is the reset status offset value from +* from the parent reset node. +* @rstctrl_bit: This is the reset control bit for the module. +* @rstst_bit: This is the reset status bit for the module. +* +* Longer description of this structure. +*/ +struct ti_reset_reg_data { +phandle handle; +u32 rstctrl_offs; +u32 rstst_offs; +u32 rstctrl_bit; +u32 rstst_bit; +}; + +/** +* struct ti_reset_data - Structure that contains the reset register data +* as well as the total number of resets for a particular SoC. +* @ti_data: Pointer to this structure to be dereferenced +* @reg_data: Pointer to the register data structure. +* @rcdev: Reset controller device instance +* @dev: Pointer to the devive structure +* @ti_reg_data: Array of register data. Only reset signal with valid +* phandles will be stored in this array. +* @reg_base: Parent register base address
drivers/reset/reset-ti.c line 69
Quoted 10 lines
drivers/reset/reset-ti.c new file +10
-0+* @lock: Spinlock for accessing the registers +* @nr_resets: Total number of resets for the SoC in the reset array. +* +* This structure contains a pointer to the register data and the modules +* register base. The number of resets and reset controller device data is +* stored within this structure. +* +*/ +struct ti_reset_data { +struct ti_reset_data *ti_data;
I initially thought this was for list management to deal with multiple reset nodes, but why do we need this self-referencing pointer?
drivers/reset/reset-ti.c line 70
Quoted 1 line
drivers/reset/reset-ti.c new file +1
-0+struct ti_reset_reg_data *reg_data;
What do you need this for? I haven't seen it getting used anywhere, and ti_reg_data looks very similar.
Quoted 19 lines
drivers/reset/reset-ti.c new file +19
-0+struct reset_controller_dev rcdev; +struct device *dev; +struct ti_reset_reg_data ti_reg_data[MAX_RESET_SIGNALS ]; +void __iomem *reg_base; +spinlock_t lock; +int nr_resets; +}; + +static int ti_reset_wait_on_reset (struct reset_controller_dev *rcdev, +unsigned long id) +{ +struct ti_reset_data *reset_data =container_of (rcdev, + struct ti_reset_data, + rcdev); +void __iomem *status_reg = reset_data->reg_base; +u32 bit_mask =0 ; +u32 val =0 ; +unsigned long flags; +int i =1000 ;
drivers/reset/reset-ti.c line 99
Quoted 10 lines
drivers/reset/reset-ti.c new file +10
-0+int ret =0 ; + +spin_lock_irqsave (&reset_data->lock, flags); +/* Clear the reset status bit to reflect the current status */ + status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs; + bit_mask = reset_data->ti_reg_data[id].rstst_bit; + +do { + val =readl (status_reg); + }while (--i && (val & (1 << bit_mask)));
Others have already pointed this out, I believe you want something equivalent to omap_test_timeout here. Please check with Tero if that would be available, otherwise, you can define the same in this file.
Quoted 52 lines
drivers/reset/reset-ti.c new file +52
-0+ +if (i ==0 && val & (1 << bit_mask)) { +dev_err (reset_data->dev,"%s: Reset failed\n" , __func__); + ret = -EIO ; + } + +spin_unlock_irqrestore (&reset_data->lock, flags); +return ret; +} + +static int ti_reset_assert (struct reset_controller_dev *rcdev, +unsigned long id) +{ +struct ti_reset_data *reset_data =container_of (rcdev, + struct ti_reset_data, + rcdev); +void __iomem *reg; +void __iomem *status_reg; +u32 status_bit =0 ; +u32 bit_mask =0 ; +u32 val =0 ; +unsigned long flags; + +spin_lock_irqsave (&reset_data->lock, flags); + +/* Clear the reset status bit to reflect the current status */ + status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs; + status_bit = reset_data->ti_reg_data[id].rstst_bit; +writel (1 << status_bit, status_reg); + + reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstctrl_offs; + bit_mask = reset_data->ti_reg_data[id].rstctrl_bit; + val =readl (reg); +if (!(val & bit_mask)) { + val |= bit_mask; +writel (val, reg); + } + +spin_unlock_irqrestore (&reset_data->lock, flags); +return 0 ; +} + +static int ti_reset_deassert (struct reset_controller_dev *rcdev, +unsigned long id) +{ + +struct ti_reset_data *reset_data =container_of (rcdev, + struct ti_reset_data, + rcdev); +void __iomem *reg; +void __iomem *status_reg; +u32 status_bit =0 ;
drivers/reset/reset-ti.c line 161
Quoted 10 lines
drivers/reset/reset-ti.c new file +10
-0+u32 bit_mask =0 ; +u32 val =0 ; +unsigned long flags; + +spin_lock_irqsave (&reset_data->lock, flags); + +/* Clear the reset status bit to reflect the current status */ + status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs; + status_bit = reset_data->ti_reg_data[id].rstst_bit; +writel (1 << status_bit, status_reg);
Is there a need for clearing the reset status both in assert and deassert?
Quoted 59 lines
drivers/reset/reset-ti.c new file +59
-0+ + reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstctrl_offs; + bit_mask = reset_data->ti_reg_data[id].rstctrl_bit; + val =readl (reg); +if (val & bit_mask) { + val &= ~bit_mask; +writel (val, reg); + } + +spin_unlock_irqrestore (&reset_data->lock, flags); + +return 0 ; +} + +static int ti_reset_reset (struct reset_controller_dev *rcdev, +unsigned long id) +{ +ti_reset_assert (rcdev, id); +ti_reset_deassert (rcdev, id); + +return ti_reset_wait_on_reset (rcdev, id); +} + +static int ti_reset_xlate (struct reset_controller_dev *rcdev, +const struct of_phandle_args *reset_spec) +{ + +struct ti_reset_data *reset_data =container_of (rcdev, + struct ti_reset_data, + rcdev); +struct device_node *dev_node; +int i; + +if (WARN_ON (reset_spec->args_count != rcdev->of_reset_n_cells)) +return -EINVAL ; + +/* Verify that the phandle exists */ + dev_node =of_find_node_by_phandle ((phandle ) reset_spec->args[0 ]); +if (!dev_node) { +dev_err (reset_data->dev, +"%s: Cannot find phandle node\n" , + __func__); +return -EINVAL ; + } + +for (i =0 ; i <= reset_data->nr_resets; i++) { +if (reset_data->ti_reg_data[i].handle == dev_node->phandle) +return i; + } + +return -EINVAL ; +} + +static struct reset_control_ops ti_reset_ops = { + .reset = ti_reset_reset, + .assert = ti_reset_assert, + .deassert = ti_reset_deassert, +}; +
drivers/reset/reset-ti.c line 230
Quoted 10 lines
drivers/reset/reset-ti.c new file +10
-0+static int ti_reset_populate_child (struct ti_reset_data *reset_data, +struct device_node *reset_child, +u32 ctrl_offs,u32 status_offs) +{ +struct device_node *next_reset_child =NULL ; +int i = reset_data->nr_resets; +int ret; +u32 ctrl_bit; + +while ((next_reset_child =of_get_next_child (reset_child, next_reset_child))) {
Add a check for array bounds on the ti_reg_data array here, rather than choosing a large number to begin with.
Quoted 76 lines
drivers/reset/reset-ti.c new file +76
-0+if (next_reset_child->phandle) { +/* Get the bits of each sub-reset */ + ret =of_property_read_u32 (next_reset_child,"control-bit" , + &ctrl_bit); +if (ret <0 ) { +dev_err (reset_data->dev, +"%s: No entry in %s for rstctrl_offs\n" , + __func__, next_reset_child->name); + +return -ENODEV ; + } + + ret =of_property_read_u32 (next_reset_child,"status-bit" , + &reset_data->ti_reg_data[i].rstst_bit); +if (ret <0 ) { +dev_err (reset_data->dev, +"%s: No entry in %s for rstst_offs\n" , + __func__, next_reset_child->name); + +return -ENODEV ; + } + + reset_data->ti_reg_data[i].rstctrl_offs = ctrl_offs; + reset_data->ti_reg_data[i].rstst_offs = status_offs; + reset_data->ti_reg_data[i].handle = next_reset_child->phandle; + i++; + } + }; + + reset_data->nr_resets = i; + +return 0 ; +} + +static int ti_reset_get_of_data (struct ti_reset_data *reset_data, +struct device_node *dev_node) +{ +struct device_node *reset_child =NULL ; +int ret = -EINVAL ; +u32 ctrl_offs, status_offs; + +/* Loop through all the children and populate a lookup array */ +while ((reset_child =of_get_next_child (dev_node, reset_child))) { + ret =of_property_read_u32_index (reset_child,"reg" ,0 , + &ctrl_offs); +if (ret) +return ret; + + ret =of_property_read_u32_index (reset_child,"reg" ,1 , + &status_offs); +if (ret) +return ret; + + ret =ti_reset_populate_child (reset_data, reset_child, + ctrl_offs, status_offs); +if (ret) +return ret; + }; + +return 0 ; +} + +static int ti_reset_probe (struct platform_device *pdev) +{ +struct device_node *resets; +struct ti_reset_data *reset_data; +struct resource *res; +int ret; + + reset_data =devm_kzalloc (&pdev->dev,sizeof (*reset_data),GFP_KERNEL ); +if (!reset_data) +return -ENOMEM ; + + resets =of_find_node_by_name (NULL ,"resets" ); +if (!resets) { + dev_err(&pdev->dev, "%s: missing 'resets' child node.\n",
drivers/reset/reset-ti.c line 316
Quoted 10 lines
drivers/reset/reset-ti.c new file +10
-0+ __func__); +return -EINVAL ; + } + + res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); + reset_data->reg_base =devm_ioremap_resource (&pdev->dev, res); +if (IS_ERR (reset_data->reg_base)) { +dev_err (&pdev->dev,"%s: Cannot map reset parent.\n" , __func__); +return -ENODEV ; + }
If the PRM node address is accessed by someone else as well, then this will always fail.
Quoted 27 lines
drivers/reset/reset-ti.c new file +27
-0+ +spin_lock_init (&reset_data->lock); + reset_data->dev = &pdev->dev; + ret =ti_reset_get_of_data (reset_data, resets); +if (ret) +return -EINVAL ; + + reset_data->rcdev.owner =THIS_MODULE ; + reset_data->rcdev.of_node = resets; + reset_data->rcdev.ops = &ti_reset_ops; + + reset_data->rcdev.of_reset_n_cells =1 ; + reset_data->rcdev.of_xlate = &ti_reset_xlate; + + reset_data->ti_data = reset_data; + +platform_set_drvdata (pdev, reset_data); + +reset_controller_register (&reset_data->rcdev); + +return 0 ; +} + +static int ti_reset_remove (struct platform_device *pdev) +{ +struct ti_reset_data *reset_data; +
drivers/reset/reset-ti.c line 353
Quoted 10 lines
drivers/reset/reset-ti.c new file +10
-0+ reset_data =platform_get_drvdata (pdev); +reset_controller_unregister (&reset_data->rcdev); + +return 0 ; +} + +static const struct of_device_id ti_reset_of_match[] = { + { .compatible ="ti,omap5-prm" }, + { .compatible ="ti,omap4-prm" }, + { .compatible ="ti,omap5-prm" },
Duplicate entry
drivers/reset/reset-ti.c line 358
Quoted 5 lines
drivers/reset/reset-ti.c new file +5
-0+ { .compatible = "ti,dra7-prm" }, + { .compatible = "ti,am4-prcm" }, + { .compatible = "ti,am3-prcm" }, + {}, +};
You are using the parent SoC parent nodes as the compatibles, but this has to restricted to just the resets. I believe this driver is designed only to deal with the reset nodes under those PRM/PRCM nodes - we won't be handling all the other nodes under these PRM/PRCM nodes.
Please also see Tony's comment about working with Tero's PRM cleanup.
regards
Suman
Quoted 16 lines
drivers/reset/reset-ti.c new file +15
-0+ +static struct platform_driver ti_reset_driver = { + .probe = ti_reset_probe, + .remove = ti_reset_remove, + .driver = { + .name =DRIVER_NAME , + .owner =THIS_MODULE , + .of_match_table =of_match_ptr (ti_reset_of_match), + }, +}; +module_platform_driver (ti_reset_driver); + +MODULE_DESCRIPTION ("PRCM reset driver for TI SoCs" ); +MODULE_LICENSE ("GPL v2" ); +MODULE_ALIAS ("platform:" DRIVER_NAME );