Re: [PATCH v4 2/4] dt-bindings: power: rockchip: Convert to json-schema
Hi,Heiko:
在 2021/3/24 下午9:31, Heiko Stübner 写道:
Am Mittwoch, 24. März 2021, 11:32:42 CET schrieb Enric Balletbo i Serra:
On 24/3/21 11:25, Enric Balletbo i Serra wrote:
Hi Elaine,
On 24/3/21 11:18, elaine.zhang wrote:
Hi, Enric
在 2021/3/24 下午5:56, Enric Balletbo i Serra 写道:
Hi Elaine,
This is not the exact version I sent, and you reintroduced a "problem" that were
already solved/discussed on previous versions. See below:
On 24/3/21 8:16, Elaine Zhang wrote:
Convert the soc/rockchip/power_domain.txt binding document to
json-schema and move to the power bindings directory.
Signed-off-by: Enric Balletbo i Serra
If you do significant is a good practice shortly describe them within [] here.
Signed-off-by: Elaine Zhang
Note that my last version already had the
Reviewed-by: Rob Herring
Which should be fine for merging (with probably only minor changes) and you
could maintain if you don't do significant changes, but that's not the case, as
I said, you are reintroducing one problem. Please review the comments already
received on this patchset or similar patchsets to avoid this.
---
.../power/rockchip,power-controller.yaml | 284 ++
.../bindings/soc/rockchip/power_domain.txt| 136 -
2 files changed, 284 insertions(+), 136 deletions(-)
create mode 100644
Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
delete mode 100644
Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
diff --git
a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
new file mode 100644
index ..a220322c5139
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
@@ -0,0 +1,284 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/rockchip,power-controller.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip Power Domains
+
+maintainers:
+ - Elaine Zhang
+ - Rob Herring
Up to Rob, but I don't think Rob would like to be the maintainer. I think you
can only include yourself and Heiko.
+ - Heiko Stuebner
+
+description: |
+ Rockchip processors include support for multiple power domains which can be
+ powered up/down by software based on different application scenarios to
save power.
+
+ Power domains contained within power-controller node are generic power domain
+ providers documented in
Documentation/devicetree/bindings/power/power-domain.yaml.
+
+ IP cores belonging to a power domain should contain a "power-domains"
+ property that is a phandle for the power domain node representing the domain.
+
+properties:
+ $nodename:
+const: power-controller
+
+ compatible:
+enum:
+ - rockchip,px30-power-controller
+ - rockchip,rk3036-power-controller
+ - rockchip,rk3066-power-controller
+ - rockchip,rk3128-power-controller
+ - rockchip,rk3188-power-controller
+ - rockchip,rk3228-power-controller
+ - rockchip,rk3288-power-controller
+ - rockchip,rk3328-power-controller
+ - rockchip,rk3366-power-controller
+ - rockchip,rk3368-power-controller
+ - rockchip,rk3399-power-controller
+
+ "#power-domain-cells":
+const: 1
+
+ "#address-cells":
+const: 1
+
+ "#size-cells":
+const: 0
+
+patternProperties:
+ "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
+type: object
+description: |
+ Represents the power domains within the power controller node as
documented
+ in Documentation/devicetree/bindings/power/power-domain.yaml.
+
The node names must be generic, as this is power-domain must be in the form:
+patternProperties:
+ "^power-domain@[0-9a-f]+$":
In this way, dtbs_check cannot be passed, and all the usage methods in dts of
Rockchip need to be corrected, which I think is a bigger change.
Well, the problem is in the Rockchip dtbs, so needs to be fixed there. The
bindings must describe hardware in a generic way, not describe the actual dtbs
to not report errors.
FWIW I remember I did something regarding this but never sent to upstream, feel
free to pick if you find useful.
*
https://gitlab.collabora.com/eballetbo/linux/-/commit/12499f223e3d33602449b9102404fe573fb804f5
*
https://gitlab.collabora.com/eballetbo/linux/-/commit/12499f223e3d33602449b9102404fe573fb804f5
*
https://gitlab.collabora.com/eballetbo/linux/-/commit/492bf2213c341152a1c2423242c5634b9e53ff27
looks good that way. I did look at the power-domain driver and
we're (of course) not doing anything with the name in front of the @ :-) .
So I'd be happy to get these.
Heiko
I still don't want to modify this, I think the PD summary will become
less intuitive.
Now:
domain status slaves
/device runtime status
Re: [PATCH v4 2/4] dt-bindings: power: rockchip: Convert to json-schema
Hi, Enric
在 2021/3/24 下午5:56, Enric Balletbo i Serra 写道:
Hi Elaine,
This is not the exact version I sent, and you reintroduced a "problem" that were
already solved/discussed on previous versions. See below:
On 24/3/21 8:16, Elaine Zhang wrote:
Convert the soc/rockchip/power_domain.txt binding document to
json-schema and move to the power bindings directory.
Signed-off-by: Enric Balletbo i Serra
If you do significant is a good practice shortly describe them within [] here.
Signed-off-by: Elaine Zhang
Note that my last version already had the
Reviewed-by: Rob Herring
Which should be fine for merging (with probably only minor changes) and you
could maintain if you don't do significant changes, but that's not the case, as
I said, you are reintroducing one problem. Please review the comments already
received on this patchset or similar patchsets to avoid this.
---
.../power/rockchip,power-controller.yaml | 284 ++
.../bindings/soc/rockchip/power_domain.txt| 136 -
2 files changed, 284 insertions(+), 136 deletions(-)
create mode 100644
Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
delete mode 100644
Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
diff --git
a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
new file mode 100644
index ..a220322c5139
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
@@ -0,0 +1,284 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/rockchip,power-controller.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip Power Domains
+
+maintainers:
+ - Elaine Zhang
+ - Rob Herring
Up to Rob, but I don't think Rob would like to be the maintainer. I think you
can only include yourself and Heiko.
+ - Heiko Stuebner
+
+description: |
+ Rockchip processors include support for multiple power domains which can be
+ powered up/down by software based on different application scenarios to save
power.
+
+ Power domains contained within power-controller node are generic power domain
+ providers documented in
Documentation/devicetree/bindings/power/power-domain.yaml.
+
+ IP cores belonging to a power domain should contain a "power-domains"
+ property that is a phandle for the power domain node representing the domain.
+
+properties:
+ $nodename:
+const: power-controller
+
+ compatible:
+enum:
+ - rockchip,px30-power-controller
+ - rockchip,rk3036-power-controller
+ - rockchip,rk3066-power-controller
+ - rockchip,rk3128-power-controller
+ - rockchip,rk3188-power-controller
+ - rockchip,rk3228-power-controller
+ - rockchip,rk3288-power-controller
+ - rockchip,rk3328-power-controller
+ - rockchip,rk3366-power-controller
+ - rockchip,rk3368-power-controller
+ - rockchip,rk3399-power-controller
+
+ "#power-domain-cells":
+const: 1
+
+ "#address-cells":
+const: 1
+
+ "#size-cells":
+const: 0
+
+patternProperties:
+ "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
+type: object
+description: |
+ Represents the power domains within the power controller node as
documented
+ in Documentation/devicetree/bindings/power/power-domain.yaml.
+
The node names must be generic, as this is power-domain must be in the form:
+patternProperties:
+ "^power-domain@[0-9a-f]+$":
In this way, dtbs_check cannot be passed, and all the usage methods in
dts of Rockchip need to be corrected, which I think is a bigger change.
+properties:
+
+ "#power-domain-cells":
+description:
+ Must be 0 for nodes representing a single PM domain and 1 for nodes
+ providing multiple PM domains.
+
+ "#address-cells":
+const: 1
+
+ "#size-cells":
+const: 0
+
+ reg:
+maxItems: 1
+description: |
+ Power domain index. Valid values are defined in
+ "include/dt-bindings/power/px30-power.h"
+ "include/dt-bindings/power/rk3036-power.h"
+ "include/dt-bindings/power/rk3066-power.h"
+ "include/dt-bindings/power/rk3128-power.h"
+ "include/dt-bindings/power/rk3188-power.h"
+ "include/dt-bindings/power/rk3228-power.h"
+ "include/dt-bindings/power/rk3288-power.h"
+ "include/dt-bindings/power/rk3328-power.h"
+ "include/dt-bindings/power/rk3366-power.h"
+ "include/dt-bindings/power/rk3368-power.h"
+ "include/dt-bindings/power/rk3399-power.h"
+
+ clocks:
+description: |
+ A number of phandles to clocks that need to be enabled while power
domain
+ switches state.
+
+ pm_qos:
+description: |
+ A number of phandles to qos blocks which need to be saved and
restored
+ while power domain
Re: [PATCH v4 2/4] dt-bindings: power: rockchip: Convert to json-schema
Hi, Johan:
在 2021/3/24 下午5:17, Johan Jonker 写道:
Hi Elaine,
>From Rob's build log it turns out that 2 more properties must be added.
Add these new properties in separate patch.
Retest with commands below.
See rk3288.dtsi
assigned-clocks = < SCLK_EDP_24M>;
assigned-clock-parents = <>;
This should not be in the power node.
It should be on the CRU node, or on the EDP's own node.
I could have added it just to solve dtbs_check .
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20210324071609.7531-3-zhangq...@rock-chips.com/
make ARCH=arm dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
On 3/24/21 8:16 AM, Elaine Zhang wrote:
Convert the soc/rockchip/power_domain.txt binding document to
json-schema and move to the power bindings directory.
Signed-off-by: Enric Balletbo i Serra
Signed-off-by: Elaine Zhang
---
.../power/rockchip,power-controller.yaml | 284 ++
.../bindings/soc/rockchip/power_domain.txt| 136 -
2 files changed, 284 insertions(+), 136 deletions(-)
create mode 100644
Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
delete mode 100644
Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
diff --git
a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
new file mode 100644
index ..a220322c5139
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
@@ -0,0 +1,284 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/rockchip,power-controller.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip Power Domains
+
+maintainers:
+ - Elaine Zhang
+ - Rob Herring
+ - Heiko Stuebner
+
+description: |
+ Rockchip processors include support for multiple power domains which can be
+ powered up/down by software based on different application scenarios to save
power.
+
+ Power domains contained within power-controller node are generic power domain
+ providers documented in
Documentation/devicetree/bindings/power/power-domain.yaml.
+
+ IP cores belonging to a power domain should contain a "power-domains"
+ property that is a phandle for the power domain node representing the domain.
+
+properties:
+ $nodename:
+const: power-controller
+
+ compatible:
+enum:
+ - rockchip,px30-power-controller
+ - rockchip,rk3036-power-controller
+ - rockchip,rk3066-power-controller
+ - rockchip,rk3128-power-controller
+ - rockchip,rk3188-power-controller
+ - rockchip,rk3228-power-controller
+ - rockchip,rk3288-power-controller
+ - rockchip,rk3328-power-controller
+ - rockchip,rk3366-power-controller
+ - rockchip,rk3368-power-controller
+ - rockchip,rk3399-power-controller
+
+ "#power-domain-cells":
+const: 1
+
+ "#address-cells":
+const: 1
+
+ "#size-cells":
+const: 0
assigned-clocks:
maxItems: 1
assigned-clock-parents:
maxItems: 1
+
+patternProperties:
+ "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
+type: object
+description: |
+ Represents the power domains within the power controller node as
documented
+ in Documentation/devicetree/bindings/power/power-domain.yaml.
+
+properties:
+
+ "#power-domain-cells":
+description:
+ Must be 0 for nodes representing a single PM domain and 1 for nodes
+ providing multiple PM domains.
+
+ "#address-cells":
+const: 1
+
+ "#size-cells":
+const: 0
+
+ reg:
+maxItems: 1
+description: |
+ Power domain index. Valid values are defined in
+ "include/dt-bindings/power/px30-power.h"
+ "include/dt-bindings/power/rk3036-power.h"
+ "include/dt-bindings/power/rk3066-power.h"
+ "include/dt-bindings/power/rk3128-power.h"
+ "include/dt-bindings/power/rk3188-power.h"
+ "include/dt-bindings/power/rk3228-power.h"
+ "include/dt-bindings/power/rk3288-power.h"
+ "include/dt-bindings/power/rk3328-power.h"
+ "include/dt-bindings/power/rk3366-power.h"
+ "include/dt-bindings/power/rk3368-power.h"
+ "include/dt-bindings/power/rk3399-power.h"
+
+ clocks:
+description: |
+ A number of phandles to clocks that need to be enabled while power
domain
+ switches state.
+
+ pm_qos:
+description: |
+ A number of phandles to qos blocks which need to be saved and
restored
+ while power domain switches state.
+
+patternProperties:
+ "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
+type: object
+description: |
+ Represents a
Re: [PATCH v2 2/3] dt-bindings: Convert the rockchip power_domain to YAML and extend
Hi, Enric 在 2021/3/24 上午4:58, Enric Balletbo Serra 写道: Hi Elaine, Missatge de Johan Jonker del dia dt., 23 de març 2021 a les 12:06: Hi Elaine, Some comments. Have a look if it's useful or that you disagree with...(part 1) == There is currently already a patch proposal that does the same. Could you read that review history and port the good things to your own patch serie? Re: [PATCH] dt-bindings: power: rockchip: Convert to json-schema https://lore.kernel.org/linux-rockchip/20201007151159.GA221754@bogus/ Re: [PATCH v3] dt-bindings: power: rockchip: Convert to json-schema https://lore.kernel.org/linux-rockchip/20201007151159.GA221754@bogus/ In fact, the latest version is v6 which can be found here: https://patchwork.kernel.org/project/linux-rockchip/patch/20210225102643.653095-1-enric.balle...@collabora.com/ Feel free to integrate and/or improve that version in your series. Thank you for your submission. I will revise the submission on this basis.
Re: [PATCH v2 2/3] dt-bindings: Convert the rockchip power_domain to YAML and extend【请注意，邮件由robherri...@gmail.com代发】
Hi, Rob Herring 在 2021/3/24 上午4:16, Rob Herring 写道: On Tue, 23 Mar 2021 16:24:09 +0800, Elaine Zhang wrote: This converts the rockchip power domain family bindings to YAML schema, and add binding documentation for the power domains found on Rockchip RK3568 SoCs. Signed-off-by: Elaine Zhang --- .../bindings/soc/rockchip/power_domain.txt| 136 - .../rockchip/rockchip,power-controller.yaml | 259 ++ 2 files changed, 259 insertions(+), 136 deletions(-) delete mode 100644 Documentation/devicetree/bindings/soc/rockchip/power_domain.txt create mode 100644 Documentation/devicetree/bindings/soc/rockchip/rockchip,power-controller.yaml My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: Documentation/devicetree/bindings/soc/rockchip/rockchip,power-controller.example.dts:19:18: fatal error: dt-bindings/clock/rk3568-cru.h: No such file or directory 19 | #include | ^~~~ compilation terminated. make[1]: *** [scripts/Makefile.lib:349: Documentation/devicetree/bindings/soc/rockchip/rockchip,power-controller.example.dt.yaml] Error 1 make: *** [Makefile:1380: dt_binding_check] Error 2 #include This file has been merged, can be seen on the Master branch of Linux-Next. I will rearrange the submission based on this: https://patchwork.kernel.org/project/linux-rockchip/patch/20210225102643.653095-1-enric.balle...@collabora.com/ See https://patchwork.ozlabs.org/patch/1457096 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
Re: [PATCH v2] PM: runtime: Update device status before letting suppliers suspend
Hi, Rafael:
The patch V2 test works well.
Tested-by: Elaine Zhang
在 2021/2/26 上午2:23, Rafael J. Wysocki 写道:
From: Rafael J. Wysocki
Because the PM-runtime status of the device is not updated in
__rpm_callback(), attempts to suspend the suppliers of the given
device triggered by rpm_put_suppliers() called by it may fail.
Fix this by making __rpm_callback() update the device's status to
RPM_SUSPENDED before calling rpm_put_suppliers() if the current
status of the device is RPM_SUSPENDING and the callback just invoked
by it has returned 0 (success).
While at it, modify the code in __rpm_callback() to always check
the device's PM-runtime status under its PM lock.
Link:
https://lore.kernel.org/linux-pm/capdykfqm06kdw_p8wxsm4dijdbho4bb6t4k50uqqvr1_cos...@mail.gmail.com/
Fixes: 21d5c57b3726 ("PM / runtime: Use device links")
Reported-by: elaine.zhang
Diagnosed-by: Ulf Hansson
Signed-off-by: Rafael J. Wysocki
---
v1 -> v2:
* Initialize the "get" variable to avoid a false-positive warning from
the compiler.
---
drivers/base/power/runtime.c | 62
+--
1 file changed, 37 insertions(+), 25 deletions(-)
Index: linux-pm/drivers/base/power/runtime.c
===
--- linux-pm.orig/drivers/base/power/runtime.c
+++ linux-pm/drivers/base/power/runtime.c
@@ -325,22 +325,22 @@ static void rpm_put_suppliers(struct dev
static int __rpm_callback(int (*cb)(struct device *), struct device *dev)
__releases(>power.lock) __acquires(>power.lock)
{
- int retval, idx;
bool use_links = dev->power.links_count > 0;
+ bool get = false;
+ int retval, idx;
+ bool put;
if (dev->power.irq_safe) {
spin_unlock(>power.lock);
+ } else if (!use_links) {
+ spin_unlock_irq(>power.lock);
} else {
+ get = dev->power.runtime_status == RPM_RESUMING;
+
spin_unlock_irq(>power.lock);
- /*
-* Resume suppliers if necessary.
-*
-* The device's runtime PM status cannot change until this
-* routine returns, so it is safe to read the status outside of
-* the lock.
-*/
- if (use_links && dev->power.runtime_status == RPM_RESUMING) {
+ /* Resume suppliers if necessary. */
+ if (get) {
idx = device_links_read_lock();
retval = rpm_get_suppliers(dev);
@@ -355,24 +355,36 @@ static int __rpm_callback(int (*cb)(stru
if (dev->power.irq_safe) {
spin_lock(>power.lock);
- } else {
- /*
-* If the device is suspending and the callback has returned
-* success, drop the usage counters of the suppliers that have
-* been reference counted on its resume.
-*
-* Do that if resume fails too.
-*/
- if (use_links
- && ((dev->power.runtime_status == RPM_SUSPENDING && !retval)
- || (dev->power.runtime_status == RPM_RESUMING && retval))) {
- idx = device_links_read_lock();
+ return retval;
+ }
- fail:
- rpm_put_suppliers(dev);
+ spin_lock_irq(>power.lock);
- device_links_read_unlock(idx);
- }
+ if (!use_links)
+ return retval;
+
+ /*
+* If the device is suspending and the callback has returned success,
+* drop the usage counters of the suppliers that have been reference
+* counted on its resume.
+*
+* Do that if the resume fails too.
+*/
+ put = dev->power.runtime_status == RPM_SUSPENDING && !retval;
+ if (put)
+ __update_runtime_status(dev, RPM_SUSPENDED);
+ else
+ put = get && retval;
+
+ if (put) {
+ spin_unlock_irq(>power.lock);
+
+ idx = device_links_read_lock();
+
+fail:
+ rpm_put_suppliers(dev);
+
+ device_links_read_unlock(idx);
spin_lock_irq(>power.lock);
}
Re: [PATCH v1 3/4] clk: rockchip: support more core div setting
Hi,Heiko:
在 2021/2/23 下午6:22, Heiko Stübner 写道:
Hi Elaine,
Am Dienstag, 23. Februar 2021, 10:53:51 CET schrieb Elaine Zhang:
A55 supports each core to work at different frequencies, and each core
has an independent divider control.
Signed-off-by: Elaine Zhang
---
drivers/clk/rockchip/clk-cpu.c | 25 +
drivers/clk/rockchip/clk.h | 17 -
2 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/rockchip/clk-cpu.c b/drivers/clk/rockchip/clk-cpu.c
index fa9027fb1920..cac06f4f7573 100644
--- a/drivers/clk/rockchip/clk-cpu.c
+++ b/drivers/clk/rockchip/clk-cpu.c
@@ -164,6 +164,18 @@ static int rockchip_cpuclk_pre_rate_change(struct
rockchip_cpuclk *cpuclk,
reg_data->mux_core_mask,
reg_data->mux_core_shift),
cpuclk->reg_base + reg_data->core_reg);
+ if (reg_data->core1_reg)
+ writel(HIWORD_UPDATE(alt_div, reg_data->div_core1_mask,
+reg_data->div_core1_shift),
+ cpuclk->reg_base + reg_data->core1_reg);
+ if (reg_data->core2_reg)
+ writel(HIWORD_UPDATE(alt_div, reg_data->div_core2_mask,
+reg_data->div_core2_shift),
+ cpuclk->reg_base + reg_data->core2_reg);
+ if (reg_data->core3_reg)
+ writel(HIWORD_UPDATE(alt_div, reg_data->div_core3_mask,
+reg_data->div_core3_shift),
+ cpuclk->reg_base + reg_data->core3_reg);
for (i = 0; i < reg_data->num_cores; i++)
writel(...)
} else {
/* select alternate parent */
writel(HIWORD_UPDATE(reg_data->mux_core_alt,
@@ -209,6 +221,19 @@ static int rockchip_cpuclk_post_rate_change(struct
rockchip_cpuclk *cpuclk,
reg_data->mux_core_shift),
cpuclk->reg_base + reg_data->core_reg);
+ if (reg_data->core1_reg)
+ writel(HIWORD_UPDATE(0, reg_data->div_core1_mask,
+reg_data->div_core1_shift),
+ cpuclk->reg_base + reg_data->core1_reg);
+ if (reg_data->core2_reg)
+ writel(HIWORD_UPDATE(0, reg_data->div_core2_mask,
+reg_data->div_core2_shift),
+ cpuclk->reg_base + reg_data->core2_reg);
+ if (reg_data->core3_reg)
+ writel(HIWORD_UPDATE(0, reg_data->div_core3_mask,
+reg_data->div_core3_shift),
+ cpuclk->reg_base + reg_data->core3_reg);
+
for (i = 0; i < reg_data->num_cores; i++)
writel(...)
if (ndata->old_rate > ndata->new_rate)
rockchip_cpuclk_set_dividers(cpuclk, rate);
diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
index 2271a84124b0..b46c93fd0cb5 100644
--- a/drivers/clk/rockchip/clk.h
+++ b/drivers/clk/rockchip/clk.h
@@ -322,7 +322,7 @@ struct rockchip_cpuclk_clksel {
u32 val;
};
-#define ROCKCHIP_CPUCLK_NUM_DIVIDERS 2
+#define ROCKCHIP_CPUCLK_NUM_DIVIDERS 5
please move this into a separate patch, as yes the rk3568 needs more
dividers but that isn't related to adding separate core divider controls.
[...]
add
#define ROCKCHIP_CPUCLK_MAX_CORES 4
struct rockchip_cpuclk_rate_table {
unsigned long prate;
struct rockchip_cpuclk_clksel divs[ROCKCHIP_CPUCLK_NUM_DIVIDERS];
@@ -333,6 +333,12 @@ struct rockchip_cpuclk_rate_table {
* @core_reg: register offset of the core settings register
* @div_core_shift: core divider offset used to divide the pll value
* @div_core_mask:core divider mask
+ * @div_core1_shift: core1 divider offset used to divide the pll value
+ * @div_core1_mask:core1 divider mask
+ * @div_core2_shift: core2 divider offset used to divide the pll value
+ * @div_core2_mask:core2 divider mask
+ * @div_core3_shift: core3 divider offset used to divide the pll value
+ * @div_core3_mask:core3 divider mask
* @mux_core_alt: mux value to select alternate parent
* @mux_core_main:mux value to select main parent of core
* @mux_core_shift: offset of the core multiplexer
@@ -342,6 +348,15 @@ struct rockchip_cpuclk_reg_data {
int core_reg;
u8 div_core_shift;
u32 div_core_mask;
+ int core1_reg;
+ u8 div_core1_shift;
+ u32 div_core1_mask;
+ int core2_reg;
+ u8 div_core2_shift;
+ u32 div_core2_mask;
+ int core3_reg;
+ u8 div_core3_shift;
+ u32 div_core3_mask;
please make this instead like:
int
Re: [PATCH v1] PM: runtime: Update device status before letting suppliers suspend
Hi, Rafael:
在 2021/2/25 上午1:53, Rafael J. Wysocki 写道:
From: Rafael J. Wysocki
Because the PM-runtime status of the device is not updated in
__rpm_callback(), attempts to suspend the suppliers of the given
device triggered by rpm_put_suppliers() called by it may fail.
Fix this by making __rpm_callback() update the device's status to
RPM_SUSPENDED before calling rpm_put_suppliers() if the current
status of the device is RPM_SUSPENDING and the callback just invoked
by it has returned 0 (success).
While at it, modify the code in __rpm_callback() to always check
the device's PM-runtime status under its PM lock.
Link:
https://lore.kernel.org/linux-pm/capdykfqm06kdw_p8wxsm4dijdbho4bb6t4k50uqqvr1_cos...@mail.gmail.com/
Fixes: 21d5c57b3726 ("PM / runtime: Use device links")
Reported-by: elaine.zhang
Diagnosed-by: Ulf Hansson
Signed-off-by: Rafael J. Wysocki
---
This is different from the previously posted tentative patch, please retest.
---
drivers/base/power/runtime.c | 61
+--
1 file changed, 36 insertions(+), 25 deletions(-)
Index: linux-pm/drivers/base/power/runtime.c
===
--- linux-pm.orig/drivers/base/power/runtime.c
+++ linux-pm/drivers/base/power/runtime.c
@@ -325,22 +325,21 @@ static void rpm_put_suppliers(struct dev
static int __rpm_callback(int (*cb)(struct device *), struct device *dev)
__releases(>power.lock) __acquires(>power.lock)
{
- int retval, idx;
bool use_links = dev->power.links_count > 0;
+ int retval, idx;
+ bool get, put;
if (dev->power.irq_safe) {
spin_unlock(>power.lock);
+ } else if (!use_links) {
+ spin_unlock_irq(>power.lock);
} else {
+ get = dev->power.runtime_status == RPM_RESUMING;
+
spin_unlock_irq(>power.lock);
- /*
-* Resume suppliers if necessary.
-*
-* The device's runtime PM status cannot change until this
-* routine returns, so it is safe to read the status outside of
-* the lock.
-*/
- if (use_links && dev->power.runtime_status == RPM_RESUMING) {
+ /* Resume suppliers if necessary. */
+ if (get) {
idx = device_links_read_lock();
retval = rpm_get_suppliers(dev);
@@ -355,24 +354,36 @@ static int __rpm_callback(int (*cb)(stru
if (dev->power.irq_safe) {
spin_lock(>power.lock);
- } else {
- /*
-* If the device is suspending and the callback has returned
-* success, drop the usage counters of the suppliers that have
-* been reference counted on its resume.
-*
-* Do that if resume fails too.
-*/
- if (use_links
- && ((dev->power.runtime_status == RPM_SUSPENDING && !retval)
- || (dev->power.runtime_status == RPM_RESUMING && retval))) {
- idx = device_links_read_lock();
+ return retval;
+ }
- fail:
- rpm_put_suppliers(dev);
+ spin_lock_irq(>power.lock);
- device_links_read_unlock(idx);
- }
+ if (!use_links)
+ return retval;
+
+ /*
+* If the device is suspending and the callback has returned success,
+* drop the usage counters of the suppliers that have been reference
+* counted on its resume.
+*
+* Do that if the resume fails too.
+*/
+ put = dev->power.runtime_status == RPM_SUSPENDING && !retval;
+ if (put)
+ __update_runtime_status(dev, RPM_SUSPENDED);
+ else
+ put = get && retval;
+
+ if (put) {
+ spin_unlock_irq(>power.lock);
+
+ idx = device_links_read_lock();
+
+fail:
+ rpm_put_suppliers(dev);
+
+ device_links_read_unlock(idx);
spin_lock_irq(>power.lock);
}
drivers/base/power/runtime.c: In function '__rpm_callback':
drivers/base/power/runtime.c:355:13: warning: 'get' may be used
uninitialized in this function [-Wmaybe-uninitialized]
error, forbidden warning:runtime.c:355
put = get && retval;
There is a compilation error. I change it as:
put = dev->power.runtime_status == RPM_SUSPENDING && retval;
And test works well.Please check it.
Re: [PATCH v1 2/4] clk: rockchip: add dt-binding header for rk3568
Hi, Heiko: 在 2021/2/23 下午6:45, Heiko Stübner 写道: Hi Elaine, Am Dienstag, 23. Februar 2021, 10:53:50 CET schrieb Elaine Zhang: Add the dt-bindings header for the rk3568, that gets shared between the clock controller and the clock references in the dts. Add softreset ID for rk3568. Signed-off-by: Elaine Zhang --- include/dt-bindings/clock/rk3568-cru.h | 926 + 1 file changed, 926 insertions(+) create mode 100644 include/dt-bindings/clock/rk3568-cru.h diff --git a/include/dt-bindings/clock/rk3568-cru.h b/include/dt-bindings/clock/rk3568-cru.h new file mode 100644 index ..22b0b8739b5d --- /dev/null +++ b/include/dt-bindings/clock/rk3568-cru.h @@ -0,0 +1,926 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (c) 2021 Rockchip Electronics Co. Ltd. + * Author: Elaine Zhang + */ + +#ifndef _DT_BINDINGS_CLK_ROCKCHIP_RK3568_H +#define _DT_BINDINGS_CLK_ROCKCHIP_RK3568_H + +/* pmucru-clocks indices */ + +/* pmucru plls */ +#define PLL_PPLL 1 +#define PLL_HPLL 2 + +/* pmucru clocks */ +#define XIN_OSC0_DIV 4 +#define CLK_RTC_32K5 +#define CLK_PMU6 +#define CLK_I2C0 7 can we change the prefix of CLK_* ids to SCLK_* (for special clock), like on previous socs. Especially as some of them already have that SCLK_prefix already anyway. Having that 4-letter prefix makes reading these IDs easier as well ;-) SCLK is for special clock, CLK is for common clock. rk3568-cru.h is automatically generated from TRM using tools. Can we minimize the work of manual modification? Because of the increasing number of clocks, writing by hand often makes mistakes.We use tools to generate rk3568-cru.h(100% use tools) and generate descriptions of registers in clk-rk3568.c(50% use tools) Thanks Heiko +#define CLK_RTC32K_FRAC8 +#define CLK_UART0_DIV 9 +#define CLK_UART0_FRAC 10 +#define SCLK_UART0 11 +#define DBCLK_GPIO012 +#define CLK_PWM0 13 +#define CLK_CAPTURE_PWM0_NDFT 14 +#define CLK_PMUPVTM15 +#define CLK_CORE_PMUPVTM 16 +#define CLK_REF24M 17 +#define XIN_OSC0_USBPHY0_G 18 +#define CLK_USBPHY0_REF19 +#define XIN_OSC0_USBPHY1_G 20 +#define CLK_USBPHY1_REF21 +#define XIN_OSC0_MIPIDSIPHY0_G 22 +#define CLK_MIPIDSIPHY0_REF23 +#define XIN_OSC0_MIPIDSIPHY1_G 24 +#define CLK_MIPIDSIPHY1_REF25 +#define CLK_WIFI_DIV 26 +#define CLK_WIFI_OSC0 27 +#define CLK_WIFI 28 +#define CLK_PCIEPHY0_DIV 29 +#define CLK_PCIEPHY0_OSC0 30 +#define CLK_PCIEPHY0_REF 31 +#define CLK_PCIEPHY1_DIV 32 +#define CLK_PCIEPHY1_OSC0 33 +#define CLK_PCIEPHY1_REF 34 +#define CLK_PCIEPHY2_DIV 35 +#define CLK_PCIEPHY2_OSC0 36 +#define CLK_PCIEPHY2_REF 37 +#define CLK_PCIE30PHY_REF_M38 +#define CLK_PCIE30PHY_REF_N39 +#define CLK_HDMI_REF 40 +#define XIN_OSC0_EDPPHY_G 41 +#define PCLK_PDPMU 42 +#define PCLK_PMU 43 +#define PCLK_UART0 44 +#define PCLK_I2C0 45 +#define PCLK_GPIO0 46 +#define PCLK_PMUPVTM 47 +#define PCLK_PWM0 48 +#define CLK_PDPMU 49 +#define SCLK_32K_IOE 50 + +#define CLKPMU_NR_CLKS (SCLK_32K_IOE + 1) + +/* cru-clocks indices */ + +/* cru plls */ +#define PLL_APLL 1 +#define PLL_DPLL 2 +#define PLL_CPLL 3 +#define PLL_GPLL 4 +#define PLL_VPLL 5 +#define PLL_NPLL 6 + +/* cru clocks */ +#define CPLL_333M 9 +#define ARMCLK 10 +#define USB480M11 +#define ACLK_CORE_NIU2BUS 18 +#define CLK_CORE_PVTM 19 +#define CLK_CORE_PVTM_CORE 20 +#define CLK_CORE_PVTPLL21 +#define CLK_GPU_SRC22 +#define CLK_GPU_PRE_NDFT 23 +#define CLK_GPU_PRE_MUX24 +#define ACLK_GPU_PRE 25 +#define PCLK_GPU_PRE 26 +#define CLK_GPU27 +#define CLK_GPU_NP528 +#define PCLK_GPU_PVTM 29 +#define CLK_GPU_PVTM 30 +#define CLK_GPU_PVTM_CORE 31 +#define CLK_GPU_PVTPLL 32 +#define CLK_NPU_SRC33 +#define CLK_NPU_PRE_NDFT 34 +#define CLK_NPU35 +#define CLK_NPU_NP536 +#define HCLK_NPU_PRE 37 +#define PCLK_NPU_PRE 38 +#define ACLK_NPU_PRE 39 +#define ACLK_NPU 40 +#define HCLK_NPU 41 +#define PCLK_NPU_PVTM 42 +#define CLK_NPU_PVTM 43 +#define CLK_NPU_PVTM_CORE 44 +#define CLK_NPU_PVTPLL 45 +#define CLK_DDRPHY1X_SRC 46 +#define CLK_DDRPHY1X_HWFFC_SRC 47 +#define CLK_DDR1X 48 +#define CLK_MSCH 49 +#define CLK24_DDRMON
Re: [PATCH] clk: rockchip: Fix overflow rate during fractional approximation
hi, We have two submissions which I hope will be helpful to you. https://patchwork.kernel.org/patch/11272465/ https://patchwork.kernel.org/patch/11272471/ A few more notes: 1. DCLK does not recommend the use of fractional frequency divider. Generally, DCLK will monopolize a PLL, and the relationship between DCLK frequency and PLL frequency is 1:1. 2, half-div, not all SOC support, detailed need to see TRM. 在 2020/9/15 上午9:20, Finley Xiao 写道: 在 2020/9/1 上午12:14, Jagan Teki 写道: The current rockchip fractional approximation overflow the desired rate if parent rate is lower than the (rate * 20) for few clocks like dclk_vopb_frac. The overflow condition has observed in px30 for dclk_vopb_frac clock with an input rate of 71.1MHz and parent rate of 24MHz is, [ 2.543280] rockchip-drm display-subsystem: bound ff46.vop (ops vop_component_ops) [ 2.557313] rockchip-drm display-subsystem: bound ff47.vop (ops vop_component_ops) [ 2.566356] rockchip-drm display-subsystem: bound ff14.syscon:lvds (ops rockchip_lvds_component_ops) [ 2.576999] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). [ 2.592177] Unexpected kernel BRK exception at EL1 [ 2.597551] Internal error: ptrace BRK handler: f20003e8 [#1] PREEMPT SMP [ 2.605143] Modules linked in: [ 2.608566] CPU: 1 PID: 31 Comm: kworker/1:1 Tainted: G U 5.8.0-rc1-15632-g97edd822b844 #30 [ 2.619363] Hardware name: Engicam PX30.Core C.TOUCH 2.0 10.1" Open Frame (DT) [ 2.627460] Workqueue: events deferred_probe_work_func [ 2.633209] pstate: 2005 (nzCv daif -PAN -UAO BTYPE=--) [ 2.639445] pc : rational_best_approximation+0xc4/0xd0 [ 2.645194] lr : rockchip_fractional_approximation+0xa8/0xe0 [ 2.651520] sp : 800011ea31c0 [ 2.655222] x29: 800011ea31c0 x28: 7a4ecd50 [ 2.661162] x27: 7d042600 x26: 0439fca3 [ 2.667102] x25: x24: 800011ac9948 [ 2.673033] x23: 800011ea3308 x22: 7d042418 [ 2.678973] x21: 800011ea3240 x20: 800011ea3238 [ 2.684904] x19: ea47 x18: [ 2.690836] x17: 0500 x16: 0001 [ 2.696775] x15: x14: [ 2.702707] x13: x12: 003c [ 2.708647] x11: 0030 x10: 0101010101010101 [ 2.714586] x9 : 03200320 x8 : 7f7f7f7f7f7f7f7f [ 2.720517] x7 : 00a3c59050d3 x6 : 0030 [ 2.726457] x5 : 800011ea3240 x4 : 800011ea3238 [ 2.732397] x3 : x2 : [ 2.738329] x1 : 016e3600 x0 : 01497e00 [ 2.744269] Call trace: [ 2.747005] rational_best_approximation+0xc4/0xd0 [ 2.752365] clk_fd_round_rate+0x8c/0x110 [ 2.756846] clk_composite_round_rate+0x30/0x40 [ 2.761917] clk_core_determine_round_nolock.part.30+0x44/0x80 [ 2.768442] clk_core_round_rate_nolock+0x78/0x80 [ 2.773701] clk_mux_determine_rate_flags+0xd8/0x200 [ 2.779253] clk_mux_determine_rate+0x10/0x20 [ 2.784124] clk_core_determine_round_nolock.part.30+0x1c/0x80 [ 2.790639] clk_core_round_rate_nolock+0x78/0x80 [ 2.795900] clk_core_round_rate_nolock+0x5c/0x80 [ 2.801159] clk_round_rate+0x64/0xf0 [ 2.805254] vop_crtc_mode_fixup+0x2c/0x60 [ 2.809828] drm_atomic_helper_check_modeset+0x95c/0xae0 [ 2.815767] drm_atomic_helper_check+0x1c/0xa0 [ 2.820738] drm_atomic_check_only+0x43c/0x760 [ 2.825705] drm_atomic_commit+0x18/0x60 [ 2.830095] drm_client_modeset_commit_atomic.isra.16+0x17c/0x250 [ 2.836911] drm_client_modeset_commit_locked+0x58/0x1a0 [ 2.842851] drm_client_modeset_commit+0x2c/0x50 [ 2.848014] drm_fb_helper_restore_fbdev_mode_unlocked+0x70/0xd0 [ 2.854730] drm_fb_helper_set_par+0x2c/0x60 [ 2.859497] fbcon_init+0x3c0/0x540 [ 2.863400] visual_init+0xac/0x100 [ 2.867298] do_bind_con_driver+0x1e4/0x3a0 [ 2.871973] do_take_over_console+0x140/0x200 [ 2.876843] do_fbcon_takeover+0x6c/0xe0 [ 2.881228] fbcon_fb_registered+0x10c/0x120 [ 2.886005] register_framebuffer+0x1f0/0x340 [ 2.890878] __drm_fb_helper_initial_config_and_unlock+0x318/0x4a0 [ 2.897790] drm_fb_helper_initial_config+0x3c/0x50 [ 2.903244] rockchip_drm_fbdev_init+0x5c/0xf0 [ 2.908202] rockchip_drm_bind+0x194/0x1e0 [ 2.912785] try_to_bring_up_master+0x164/0x1d0 [ 2.917851] component_master_add_with_match+0xac/0xf0 [ 2.923597] rockchip_drm_platform_probe+0x238/0x2e0 [ 2.929150] platform_drv_probe+0x50/0xa0 [ 2.933631] really_probe+0xd4/0x330 [ 2.937628] driver_probe_device+0x54/0xb0 [ 2.942207] __device_attach_driver+0x80/0xc0 [ 2.947078] bus_for_each_drv+0x78/0xd0 [ 2.951365] __device_attach+0xd4/0x130 [ 2.955652] device_initial_probe+0x10/0x20 [ 2.960328] bus_probe_device+0x90/0xa0 [ 2.964616] deferred_probe_work_func+0x6c/0xa0 [ 2.969685] process_one_work+0x1e4/0x360 [ 2.974166]
Re: [PATCH v3 6/6] clk: rockchip: rk3399: Support module build
hi,
在 2020/9/7 上午6:49, Heiko Stübner 写道:
Am Freitag, 4. September 2020, 09:45:05 CEST schrieb Elaine Zhang:
support CLK_OF_DECLARE and builtin_platform_driver_probe
double clk init method.
add module author, description and license to support building
Soc Rk3399 clock driver as module.
Signed-off-by: Elaine Zhang
Reviewed-by: Kever Yang
---
drivers/clk/rockchip/clk-rk3399.c | 55 +++
1 file changed, 55 insertions(+)
diff --git a/drivers/clk/rockchip/clk-rk3399.c
b/drivers/clk/rockchip/clk-rk3399.c
index ce1d2446f142..40ff17aee5b6 100644
--- a/drivers/clk/rockchip/clk-rk3399.c
+++ b/drivers/clk/rockchip/clk-rk3399.c
@@ -5,9 +5,11 @@
*/
#include
+#include
#include
#include
#include
+#include
#include
#include
#include
@@ -1600,3 +1602,56 @@ static void __init rk3399_pmu_clk_init(struct
device_node *np)
rockchip_clk_of_add_provider(np, ctx);
}
CLK_OF_DECLARE(rk3399_cru_pmu, "rockchip,rk3399-pmucru", rk3399_pmu_clk_init);
+
+struct clk_rk3399_inits {
+ void (*inits)(struct device_node *np);
+};
+
+static const struct clk_rk3399_inits clk_rk3399_pmucru_init = {
+ .inits = rk3399_pmu_clk_init,
+};
+
+static const struct clk_rk3399_inits clk_rk3399_cru_init = {
+ .inits = rk3399_clk_init,
+};
+
+static const struct of_device_id clk_rk3399_match_table[] = {
+ {
+ .compatible = "rockchip,rk3399-cru",
+ .data = _rk3399_cru_init,
+ }, {
+ .compatible = "rockchip,rk3399-pmucru",
+ .data = _rk3399_pmucru_init,
+ },
+ { }
+};
+MODULE_DEVICE_TABLE(of, clk_rk3399_match_table);
+
+static int __init clk_rk3399_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ const struct of_device_id *match;
+ const struct clk_rk3399_inits *init_data;
+
+ match = of_match_device(clk_rk3399_match_table, >dev);
+ if (!match || !match->data)
+ return -EINVAL;
+
+ init_data = match->data;
+ if (init_data->inits)
+ init_data->inits(np);
+
+ return 0;
+}
+
+static struct platform_driver clk_rk3399_driver = {
+ .driver = {
+ .name = "clk-rk3399",
+ .of_match_table = clk_rk3399_match_table,
I guess we probably want
.suppress_bind_attrs = true,
OK, I will add it in the next version.
here, because there is no unloading.
Also what happens when you try to rmmod the module?
console:/ # lsmod | grep clk
clk_rk808 16384 0
clk_rk3399 49152 0 [permanent]
clk_rockchip 57344 32 rockchip_dmc,rockchip_opp_select,clk_rk3399
rockchip_sip 24576 6
rk_vcodec,rockchip_pwm_remotectl,rockchip_bus,nvmem_rockchip_efuse,rockchip_pm_config,clk_rockchip
console:/ # rmmod clk_rk3399
rmmod: failed to unload clk_rk3399: Device or resource busy
console:/ # rmmod -f clk_rk3399
rmmod: failed to unload clk_rk3399: Device or resource busy
The builtin_platform_driver_probe() without the __exit parts.
Heiko
Re: [PATCH v1] clk: Export __clk_lookup()
在 2020/7/23 上午2:26, Heiko Stuebner 写道: Hi Elaine, Am Mittwoch, 22. Juli 2020, 04:32:30 CEST schrieb Elaine Zhang: Export __clk_lookup() to support user built as module. ERROR: drivers/clk/rockchip/clk.ko: In function `rockchip_clk_protect_critical': drivers/clk/rockchip/clk.c:741: undefined reference to `__clk_lookup' can you elaborate a bit more on why this would be needed? Because right now the Rockchip clocks are of course built into the main kernel image (especially due to them being needed during early boot) and __clk_lookup actually is a pretty deep part of the clock- framework itself, as probably also denoted by the "__" in the function name. Rockchip clocks are of course support to built as module(to support GKI),These changes will be pushed soon. In drivers/clk/rockchip/clk.c and drivers/clk/rockchip/clk-cpu.c use the __clk_lookup. Heiko Signed-off-by: Elaine Zhang --- drivers/clk/clk.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 3f588ed06ce3..600284fbb257 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -618,6 +618,7 @@ struct clk *__clk_lookup(const char *name) return !core ? NULL : core->hw->clk; } +EXPORT_SYMBOL_GPL(__clk_lookup); static void clk_core_get_boundaries(struct clk_core *core, unsigned long *min_rate,
Re: [PATCH v1] clk: Export __clk_lookup()
在 2020/7/23 上午9:42, elaine.zhang 写道:
在 2020/7/23 上午8:51, Stephen Boyd 写道:
Quoting Heiko Stuebner (2020-07-22 11:26:50)
Hi Elaine,
Am Mittwoch, 22. Juli 2020, 04:32:30 CEST schrieb Elaine Zhang:
Export __clk_lookup() to support user built as module.
ERROR:
drivers/clk/rockchip/clk.ko: In function
`rockchip_clk_protect_critical':
drivers/clk/rockchip/clk.c:741:
undefined reference to `__clk_lookup'
can you elaborate a bit more on why this would be needed?
Because right now the Rockchip clocks are of course built into
the main kernel image (especially due to them being needed during early
boot) and __clk_lookup actually is a pretty deep part of the clock-
framework itself, as probably also denoted by the "__" in the function
name.
Can you stop using __clk_lookup()? The plan is to remove it.
Rk use __clk_lookup() as:
drivers/clk/rockchip/clk.c
void __init rockchip_clk_protect_critical(const char *const clocks[],
int nclocks)
{
int i;
/* Protect the clocks that needs to stay on */
for (i = 0; i < nclocks; i++) {
struct clk *clk = __clk_lookup(clocks[i]);
if (clk)
clk_prepare_enable(clk);
}
}
e.g:
drivers/clk/rockchip/clk-rk3328.c
static const char *const rk3328_critical_clocks[] __initconst = {
"aclk_bus",
"aclk_bus_niu",
"pclk_bus",
"pclk_bus_niu",
"hclk_bus",
"hclk_bus_niu",
"aclk_peri",
};
If have plan to remove the __clk_lookup, I need to replace the
rockchip_clk_protect_critical, and use the flag CLK_IS_CRITICAL.(but
use flag CLK_IS_CRITICAL, the enable count is always "0")
Use the CLK_IS_CRITICAL, there is no guarantee that the parent clk is
enabled, So the flag CLK_IS_CRITICAL need to be added to all special
clks according to the clk tree.
Re: [PATCH v1] clk: Export __clk_lookup()
在 2020/7/23 上午8:51, Stephen Boyd 写道:
Quoting Heiko Stuebner (2020-07-22 11:26:50)
Hi Elaine,
Am Mittwoch, 22. Juli 2020, 04:32:30 CEST schrieb Elaine Zhang:
Export __clk_lookup() to support user built as module.
ERROR:
drivers/clk/rockchip/clk.ko: In function
`rockchip_clk_protect_critical':
drivers/clk/rockchip/clk.c:741:
undefined reference to `__clk_lookup'
can you elaborate a bit more on why this would be needed?
Because right now the Rockchip clocks are of course built into
the main kernel image (especially due to them being needed during early
boot) and __clk_lookup actually is a pretty deep part of the clock-
framework itself, as probably also denoted by the "__" in the function
name.
Can you stop using __clk_lookup()? The plan is to remove it.
Rk use __clk_lookup() as:
drivers/clk/rockchip/clk.c
void __init rockchip_clk_protect_critical(const char *const clocks[],
int nclocks)
{
int i;
/* Protect the clocks that needs to stay on */
for (i = 0; i < nclocks; i++) {
struct clk *clk = __clk_lookup(clocks[i]);
if (clk)
clk_prepare_enable(clk);
}
}
e.g:
drivers/clk/rockchip/clk-rk3328.c
static const char *const rk3328_critical_clocks[] __initconst = {
"aclk_bus",
"aclk_bus_niu",
"pclk_bus",
"pclk_bus_niu",
"hclk_bus",
"hclk_bus_niu",
"aclk_peri",
};
If have plan to remove the __clk_lookup, I need to replace the
rockchip_clk_protect_critical, and use the flag CLK_IS_CRITICAL.(but use
flag CLK_IS_CRITICAL, the enable count is always "0")
Re: [PATCH] clk: rockchip: mark pclk_uart2 as critical on rk3328
swapper/0-1 [003] d..1 2.208605: clk_disable: sclk_uart2
swapper/0-1 [003] d..1 2.208646: clk_enable: sclk_uart2
On 7/9/20 3:32 AM, elaine.zhang wrote:
在 2020/7/8 下午10:45, Johan Jonker 写道:
The rk3328 uart2 port is used as boot console and to debug.
During the boot pclk_uart2 is disabled by a clk_disable_unused
initcall. Fix the uart2 function by marking pclk_uart2
as critical on rk3328. Also add sclk_uart2 as that is needed
for the same DT node.
Signed-off-by: Johan Jonker
---
drivers/clk/rockchip/clk-rk3328.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/clk/rockchip/clk-rk3328.c
b/drivers/clk/rockchip/clk-rk3328.c
index c186a1985..cb7749cb7 100644
--- a/drivers/clk/rockchip/clk-rk3328.c
+++ b/drivers/clk/rockchip/clk-rk3328.c
@@ -875,6 +875,8 @@ static const char *const rk3328_critical_clocks[]
__initconst = {
"aclk_gmac_niu",
"pclk_gmac_niu",
"pclk_phy_niu",
+ "pclk_uart2",
+ "sclk_uart2",
};
Not need to mark the uart2 as critical clocks, the uart clk will enabled
by uart driver probe(dw8250_probe()).
For your question, Please check the uart2 dts node "status = okay".
Or You can send me the complete log, I check the status of uart2.
static void __init rk3328_clk_init(struct device_node *np)
Re: [PATCH] clk: rockchip: mark pclk_uart2 as critical on rk3328
在 2020/7/8 下午10:45, Johan Jonker 写道:
The rk3328 uart2 port is used as boot console and to debug.
During the boot pclk_uart2 is disabled by a clk_disable_unused
initcall. Fix the uart2 function by marking pclk_uart2
as critical on rk3328. Also add sclk_uart2 as that is needed
for the same DT node.
Signed-off-by: Johan Jonker
---
drivers/clk/rockchip/clk-rk3328.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/clk/rockchip/clk-rk3328.c
b/drivers/clk/rockchip/clk-rk3328.c
index c186a1985..cb7749cb7 100644
--- a/drivers/clk/rockchip/clk-rk3328.c
+++ b/drivers/clk/rockchip/clk-rk3328.c
@@ -875,6 +875,8 @@ static const char *const rk3328_critical_clocks[]
__initconst = {
"aclk_gmac_niu",
"pclk_gmac_niu",
"pclk_phy_niu",
+ "pclk_uart2",
+ "sclk_uart2",
};
Not need to mark the uart2 as critical clocks, the uart clk will enabled
by uart driver probe(dw8250_probe()).
For your question, Please check the uart2 dts node "status = okay".
Or You can send me the complete log, I check the status of uart2.
static void __init rk3328_clk_init(struct device_node *np)
Re: [PATCH v3 1/3] thermal: rockchip: fix up the tsadc pinctrl setting error
hi, Heiko & Enric:
在 2019/5/22 下午8:27, Heiko Stuebner 写道:
Hi Enric,
Am Montag, 20. Mai 2019, 15:38:32 CEST schrieb Enric Balletbo Serra:
Hi all,
As pointed by [1] and [2] this commit, that now is upstream, breaks
veyron (rk3288) and kevin (rk3399) boards. The problem is especially
critical for veyron boards because they don't boot anymore.
I didn't look deep at the problem but I have some concerns about this
patch, see below.
[1] https://www.spinics.net/lists/linux-rockchip/msg24657.html
[2] https://www.spinics.net/lists/linux-rockchip/msg24735.html
Missatge de Daniel Lezcano del dia dt., 30
d’abr. 2019 a les 15:39:
On 30/04/2019 12:09, Elaine Zhang wrote:
Explicitly use the pinctrl to set/unset the right mode
instead of relying on the pinctrl init mode.
And it requires setting the tshut polarity before select pinctrl.
When the temperature sensor mode is set to 0, it will automatically
reset the board via the Clock-Reset-Unit (CRU) if the over temperature
threshold is reached. However, when the pinctrl initializes, it does a
transition to "otp_out" which may lead the SoC restart all the time.
"otp_out" IO may be connected to the RESET circuit on the hardware.
If the IO is in the wrong state, it will trigger RESET.
(similar to the effect of pressing the RESET button)
which will cause the soc to restart all the time.
Signed-off-by: Elaine Zhang
Reviewed-by: Daniel Lezcano
---
drivers/thermal/rockchip_thermal.c | 36 +---
1 file changed, 33 insertions(+), 3 deletions(-)
diff --git a/drivers/thermal/rockchip_thermal.c
b/drivers/thermal/rockchip_thermal.c
index 9c7643d62ed7..6dc7fc516abf 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -172,6 +172,9 @@ struct rockchip_thermal_data {
int tshut_temp;
enum tshut_mode tshut_mode;
enum tshut_polarity tshut_polarity;
+ struct pinctrl *pinctrl;
+ struct pinctrl_state *gpio_state;
+ struct pinctrl_state *otp_state;
};
/**
@@ -1242,6 +1245,8 @@ static int rockchip_thermal_probe(struct platform_device
*pdev)
return error;
}
+ thermal->chip->control(thermal->regs, false);
+
That's the line that causes the hang. Commenting this makes the veyron
boot again. Probably this needs to go after chip->initialize?
It needs to go after the clk_enable calls.
At this point the tsadc may still be unclocked.
The clk is enable by default.
The reason for this modification:
The otp Pin polarity setting for tsadc must be set when tsadc is turned off.
The order:
Close the tsadc->Set the otp pin polarity ->Set the pinctrl->initialize
the tsadc->Open the tsadc
As for the problem you mentioned, I guess: The default polarity of otp
does not match the default state, that is, the otp is triggered by
default, and then the reset circuit of the hardware takes effect and is
restarted all the time.
Modification:
1. For this hardware, otp pin default state is modified.
2. The mode of using CRU is rockchip,hw-tshut-mode = <0> in DTS;
/* tshut mode 0:CRU 1:GPIO */
Recommended use method 2. You can try it.
error = clk_prepare_enable(thermal->clk);
if (error) {
dev_err(>dev, "failed to enable converter clock: %d\n",
@@ -1267,6 +1272,30 @@ static int rockchip_thermal_probe(struct platform_device
*pdev)
thermal->chip->initialize(thermal->grf, thermal->regs,
thermal->tshut_polarity);
+ if (thermal->tshut_mode == TSHUT_MODE_GPIO) {
+ thermal->pinctrl = devm_pinctrl_get(>dev);
+ if (IS_ERR(thermal->pinctrl)) {
+ dev_err(>dev, "failed to find thermal pinctrl\n");
+ return PTR_ERR(thermal->pinctrl);
+ }
+
+ thermal->gpio_state = pinctrl_lookup_state(thermal->pinctrl,
+"gpio");
Shouldn't this mode be documented properly in the binding first?
More importantly, it should be _backwards-compatible_, aka work with
old devicetrees without that property and not break thermal handling for
them entirely.
If need _backwards-compatible_, It's can't return
PTR_ERR(thermal->pinctrl) when get
devm_pinctrl_get failed.
The binding [3] talks about init, default and sleep states but *not*
gpio and otpout. The patch series looks incomplete to me or not using
the proper names.
[3]
https://elixir.bootlin.com/linux/v5.2-rc1/source/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
+ if (IS_ERR_OR_NULL(thermal->gpio_state)) {
+ dev_err(>dev, "failed to find thermal gpio
state\n");
+ return -EINVAL;
+ }
+
+ thermal->otp_state = pinctrl_lookup_state(thermal->pinctrl,
+ "otpout");
+ if (IS_ERR_OR_NULL(thermal->otp_state)) {
+ dev_err(>dev, "failed to
Re: [PATCH v2 1/3] thermal: rockchip: fix up the tsadc pinctrl setting error
hi,
在 2019/4/28 下午8:45, Daniel Lezcano 写道:
On 25/04/2019 12:12, Elaine Zhang wrote:
Explicitly use the pinctrl to set/unset the right mode
instead of relying on the pinctrl init mode.
And it requires setting the tshut polarity before select pinctrl.
When the temperature sensor mode is set to 0, it will automatically
reset the board via the Clock-Reset-Unit (CRU) if the over temperature
threshold is reached. However, when the pinctrl initializes, it does a
transition to "otp_out" which may lead the SoC restart all the time.
"otp_out" IO may be connected to the RESET circuit on the hardware.
If the IO is in the wrong state, it will trigger RESET.
(similar to the effect of pressing the RESET button)
which will cause the soc to restart all the time.
Signed-off-by: Elaine Zhang
---
drivers/thermal/rockchip_thermal.c | 37 ++---
1 file changed, 34 insertions(+), 3 deletions(-)
diff --git a/drivers/thermal/rockchip_thermal.c
b/drivers/thermal/rockchip_thermal.c
index 9c7643d62ed7..03ff494f2864 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -172,6 +172,9 @@ struct rockchip_thermal_data {
int tshut_temp;
enum tshut_mode tshut_mode;
enum tshut_polarity tshut_polarity;
+ struct pinctrl *pinctrl;
+ struct pinctrl_state *gpio_state;
+ struct pinctrl_state *otp_state;
};
/**
@@ -1242,6 +1245,8 @@ static int rockchip_thermal_probe(struct platform_device
*pdev)
return error;
}
+ thermal->chip->control(thermal->regs, false);
+
error = clk_prepare_enable(thermal->clk);
if (error) {
dev_err(>dev, "failed to enable converter clock: %d\n",
@@ -1267,6 +1272,31 @@ static int rockchip_thermal_probe(struct platform_device
*pdev)
thermal->chip->initialize(thermal->grf, thermal->regs,
thermal->tshut_polarity);
+ if (thermal->tshut_mode == TSHUT_MODE_GPIO) {
+ thermal->pinctrl = devm_pinctrl_get(>dev);
+ if (IS_ERR(thermal->pinctrl)) {
+ dev_err(>dev, "failed to find thermal pinctrl\n");
+ panic("panic_on_find thermal pinctrl...\n");
I realize my comment was confusing. I was not saying to add a panic()
call here but that the code was accessing a NULL pointer. Please remove
the panic.
OK, I'll fixed it.
+ return PTR_ERR(thermal->pinctrl);
+ }
+
+ thermal->gpio_state = pinctrl_lookup_state(thermal->pinctrl,
+ "gpio");
+ if (IS_ERR_OR_NULL(thermal->gpio_state)) {
+ dev_err(>dev, "failed to find thermal gpio
state\n");
+ return -EINVAL;
+ }
+
+ thermal->otp_state = pinctrl_lookup_state(thermal->pinctrl,
+ "otpout");
+ if (IS_ERR_OR_NULL(thermal->otp_state)) {
+ dev_err(>dev, "failed to find thermal otpout
state\n");
+ return -EINVAL;
+ }
+
+ pinctrl_select_state(thermal->pinctrl, thermal->otp_state);
I don't understand this portion of code. The test above says tshut_mode
is TSHUT_MODE_GPIO. Why acting on thermal->otp_state then ?
In my previous comment, I was suggesting the following:
Two more fields instead of three:
struct rockchip_thermal_data {
int tshut_temp;
enum tshut_mode tshut_mode;
enum tshut_polarity tshut_polarity;
struct pinctrl *pinctrl;
struct pinctrl_state *pinctrl_state;
};
[ ... ]
thermal->pinctrl = devm_pinctrl_get(>dev);
if (IS_ERR(thermal->pinctrl)) {
dev_err("...");
return PTR_ERR(thermal->pinctrl);
}
thermal->pinctrl_state = pinctrl_lookup_state(thermal->pinctrl,
thermal->tshut_mode == TSHUT_MODE_GPIO ? "gpio" :
"otpout";
if (IS_ERR_OR_NULL(thermal->pinctrl_state) {
dev_err("...");
return PTR_ERR(thermal->pinctrl_state);
}
pinctrl_select_state(thermal->pinctrl, thermal->pinctrl_state);
[ ... ]
Is it wrong ?
+ }
+
for (i = 0; i < thermal->chip->chn_num; i++) {
error = rockchip_thermal_register_sensor(pdev, thermal,
>sensors[i],
@@ -1337,8 +1367,8 @@ static int __maybe_unused rockchip_thermal_suspend(struct
device *dev)
clk_disable(thermal->pclk);
clk_disable(thermal->clk);
-
- pinctrl_pm_select_sleep_state(dev);
+ if (thermal->tshut_mode == TSHUT_MODE_GPIO)
+ pinctrl_select_state(thermal->pinctrl, thermal->gpio_state);
And then:
Re: [PATCH v2] clk: rockchip: undo several noc and special clocks as critical on rk3288
hi,
在 2019/4/22 下午11:23, Doug Anderson 写道:
Elaine,
On Fri, Apr 12, 2019 at 9:18 AM Douglas Anderson wrote:
This is mostly a revert of commit 55bb6a633c33 ("clk: rockchip: mark
noc and some special clk as critical on rk3288") except that we're
keeping "pmu_hclk_otg0" as critical still.
NOTE: turning these clocks off doesn't seem to do a whole lot in terms
of power savings (checking the power on the logic rail). It appears
to save maybe 1-2mW. ...but still it seems like we should turn the
clocks off if they aren't needed.
About "pmu_hclk_otg0" (the one clock from the original commit we're
still keeping critical) from an email thread:
pmu ahb clock
Function: Clock to pmu module when hibernation and/or ADP is
enabled. Must be greater than or equal to 30 MHz.
If the SOC design does not support hibernation/ADP function, only have
hclk_otg, this clk can be switched according to the usage of otg.
If the SOC design support hibernation/ADP, has two clocks, hclk_otg and
pmu_hclk_otg0.
Hclk_otg belongs to the closed part of otg logic, which can be switched
according to the use of otg.
pmu_hclk_otg0 belongs to the always on part.
As for whether pmu_hclk_otg0 can be turned off when otg is not in use,
we have not tested. IC suggest make pmu_hclk_otg0 always on.
For the rest of the clocks:
atclk: No documentation about this clock other than that it goes to
the CPU. CPU functions fine without it on. Maybe needed for JTAG?
jtag: Presumably this clock is only needed if you're debugging with
JTAG. It doesn't seem like it makes sense to waste power for every
rk3288 user. In any case to do JTAG you'd need private patches to
adjust the pinctrl the mux the JTAG out anyway.
pclk_dbg, pclk_core_niu: On veyron Chromebooks we turn these two
clocks on only during kernel panics in order to access some coresight
registers. Since nothing in the upstream kernel does this we should
be able to leave them off safely. Maybe also needed for JTAG?
hsicphy12m_xin12m: There is no indication of why this clock would need
to be turned on for boards that don't use HSIC.
pclk_ddrupctl[0-1], pclk_publ0[0-1]: On veyron Chromebooks we turn
these 4 clocks on only when doing DDR transitions and they are off
otherwise. I see no reason why they'd need to be on in the upstream
kernel which doesn't support DDRFreq.
Signed-off-by: Douglas Anderson
---
Changes in v2:
- Now keep pmu_hclk_otg0 as critical.
- Updated description since this isn't a clean revert.
- PWM patches have landed, so just one patch in the series now.
drivers/clk/rockchip/clk-rk3288.c | 13 -
1 file changed, 4 insertions(+), 9 deletions(-)
>From previous discussions I think you're all happy with this patch
now, right? Care to give it an official Reviewed-by tag?
Yes.
Reviewed-by: Elaine Zhang
-Doug
Re: [PATCH v1 1/3] thermal: rockchip: add pinctrl control
hi,
在 2019/4/16 下午6:12, Daniel Lezcano 写道:
Hi Elaine,
On 11/04/2019 09:46, elaine.zhang wrote:
hi,
在 2019/4/4 上午11:03, Daniel Lezcano 写道:
On 01/04/2019 08:43, Elaine Zhang wrote:
Based on the TSADC Tshut mode to select pinctrl,
instead of setting pinctrl based on architecture
(Not depends on pinctrl setting by "init" or "default").
And it requires setting the tshut polarity before select pinctrl.
I'm not sure to fully read the description. Can you rephrase/elaborate
the changelog?
Example:
tsadc: tsadc@ff25 {
compatible = "rockchip,rk3328-tsadc";
.
pinctrl-names = "init", "default", "sleep";
pinctrl-0 = <_gpio>;
pinctrl-1 = <_out>;
pinctrl-2 = <_gpio>;
status = "disabled";
.
};
{
rockchip,hw-tshut-mode = <0>; /* tshut mode 0:CRU 1:GPIO */
status = "okay";
};
Application on the product, hope tsadc overtemperature reset cru to
restart.
But when pinctrl is initialized, it will setting pinctrl by "init"
and "default".
So the pinctrl will set iomux to "otp_out", which may be make system
crash.
why?
This "otp_out" IO may be connected to the RESET circuit on the hardware.
If the IO is in the wrong state, it will trigger RESET (similar to the
effect of pressing the RESET button), which will cause the system to
restart all the time.
tsadc gpio iomux:
"otp_gpio" : just normal gpio, keep default level.
"otp_out" : tsadc shut down io, when overtemperature,this io may be
trigger high
level or low level(depend on the tsadc polarity).
After correction:
if rockchip,hw-tshut-mode = <0>; (tsadc overtemperature reset cru to
restart)
select pinctrl to otp_gpio
if rockchip,hw-tshut-mode = <1>;(tsadc overtemperature IO triggers
output at high or low levels)
select pinctrl to otp_out.
Actively select iomux based on rockchip,hw-tshut-mode,
rather than relying on the pinctrl framework to select iomux.
Let me rephrase it and tell me if I understood correctly:
"When the temperature sensor mode is set to 0, it will automatically
reset the board via the Clock-Reset-Unit (CRU) if the over temperature
threshold is reached. However, when the pinctrl initializes, it does a
transition to "otp_out" which may lead the SoC to crash (why?)
Explicitly use the pinctrl to set/unset the right mode instead of
relying on the pinctrl init mode"
This explanation is correct.
So this patch is a fix and it must contain the Fixes: ... tag.
OK, I'll fix that in the v2.
Signed-off-by: Elaine Zhang
---
drivers/thermal/rockchip_thermal.c | 61
+++---
1 file changed, 50 insertions(+), 11 deletions(-)
diff --git a/drivers/thermal/rockchip_thermal.c
b/drivers/thermal/rockchip_thermal.c
index 9c7643d62ed7..faa6c7792155 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -34,7 +34,7 @@
*/
enum tshut_mode {
TSHUT_MODE_CRU = 0,
- TSHUT_MODE_GPIO,
+ TSHUT_MODE_OTP,
Why do you change the enum name? The impact on the patch is much higher,
no ?
I just want to make it a little bit more intuitive to understand the
definition of mode.
TSHUT_MODE_CRU: pinctrl select iomux to otp_gpio,the io is normal io.
TSHUT_MODE_OTP: pinctrl select iomux to otp_out, the io is tsadc shut io.
I understand, but at the end the changes for this patch are counter
intuitive, so it is preferable to keep it as is so we can review the
important part.
[ ... ]
OK, keep the enum name. I'll fix it in the V2.
+static void thermal_pinctrl_select_otp(struct
rockchip_thermal_data *thermal)
+{
+ if (!IS_ERR(thermal->pinctrl) &&
!IS_ERR_OR_NULL(thermal->otp_state))
+ pinctrl_select_state(thermal->pinctrl,
+ thermal->otp_state);
+}
+
+static void thermal_pinctrl_select_gpio(struct rockchip_thermal_data
*thermal)
+{
+ if (!IS_ERR(thermal->pinctrl) &&
!IS_ERR_OR_NULL(thermal->gpio_state))
+ pinctrl_select_state(thermal->pinctrl,
+ thermal->gpio_state);
+}
You should not have to create a couple of specific functions just to
check the pinctrl pointers are set. The caller should do that.
Because there are several places where the call is made,create a couple
of specific functions reduce a lot of duplicated code.
No, that does not reduce a lot of duplicated code, it hides the fact
there is no control from the caller. See the comments below.
[ ... ]
OK, I'll fix it in the V2.
static int rockchip_configure_from_dt(struct device *dev,
Re: [PATCH 1/5] clk: rockchip: Turn on "aclk_dmac1" for suspend
hi,
在 2019/4/12 上午7:21, Douglas Anderson 写道:
Experimentally it can be seen that going into deep sleep (specifically
setting PMU_CLR_DMA and PMU_CLR_BUS in RK3288_PMU_PWRMODE_CON1)
appears to fail unless "aclk_dmac1" is on. The failure is that the
system never signals that it made it into suspend on the GLOBAL_PWROFF
pin and it just hangs.
NOTE that it's confirmed that it's the actual suspend that fails, not
one of the earlier calls to read/write registers. Specifically if you
comment out the "PMU_GLOBAL_INT_DISABLE" setting in
rk3288_slp_mode_set() and then comment out the "cpu_do_idle()" call in
rockchip_lpmode_enter() then you can exercise the whole suspend path
without any crashing.
This is currently not a problem with suspend upstream because there is
no current way to exercise the deep suspend code. However, anyone
trying to make it work will run into this issue.
This was not a problem on shipping rk3288-based Chromebooks because
those devices all ran on an old kernel based on 3.14. On that kernel
"aclk_dmac1" appears to be left on all the time.
There are several ways to skin this problem.
A) We could add "aclk_dmac1" to the list of critical clocks and that
apperas to work, but presumably that wastes power.
B) We could keep a list of "struct clk" objects to enable at suspend
time in clk-rk3288.c and use the standard clock APIs.
C) We could make the rk3288-pmu driver keep a list of clocks to enable
at suspend time. Presumably this would require a dts and bindings
change.
D) We could just whack the clock on in the existing syscore suspend
function where we whack a bunch of other clocks. This is particularly
easy because we know for sure that the clock's only parent
("aclk_cpu") is a critical clock so we don't need to do anything more
than ungate it.
In this case I have chosen D) because it seemed like the least work,
but any of the other options would presumably also work fine.
Signed-off-by: Douglas Anderson
---
drivers/clk/rockchip/clk-rk3288.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/clk/rockchip/clk-rk3288.c
b/drivers/clk/rockchip/clk-rk3288.c
index 5a67b7869960..b245367393cd 100644
--- a/drivers/clk/rockchip/clk-rk3288.c
+++ b/drivers/clk/rockchip/clk-rk3288.c
@@ -859,6 +859,9 @@ static const int rk3288_saved_cru_reg_ids[] = {
RK3288_CLKSEL_CON(10),
RK3288_CLKSEL_CON(33),
RK3288_CLKSEL_CON(37),
+
+ /* We turn aclk_dmac1 on for suspend; this will restore it */
+ RK3288_CLKGATE_CON(10),
};
static u32 rk3288_saved_cru_regs[ARRAY_SIZE(rk3288_saved_cru_reg_ids)];
@@ -874,6 +877,14 @@ static int rk3288_clk_suspend(void)
readl_relaxed(rk3288_cru_base + reg_id);
}
+ /*
+* Going into deep sleep (specifically setting PMU_CLR_DMA in
+* RK3288_PMU_PWRMODE_CON1) appears to fail unless
+* "aclk_dmac1" is on.
+*/
+ writel_relaxed(1 << (12 + 16),
+ rk3288_cru_base + RK3288_CLKGATE_CON(10));
+
/*
* Switch PLLs other than DPLL (for SDRAM) to slow mode to
* avoid crashes on resume. The Mask ROM on the system will
Thank you for your correction.
Reviewed-by: Elaine Zhang
Re: [PATCH 5/5] ARM: dts: rockchip: vdd_gpu off in suspend for rk3288-veyron
hi,
在 2019/4/12 上午7:21, Douglas Anderson 写道:
At some point long long ago the downstream GPU driver would crash if
we turned the GPU off during suspend. For some context you can see:
https://chromium-review.googlesource.com/#/c/215780/5..6/arch/arm/boot/dts/rk3288-pinky-rev2.dts
At some point in time not too long after that got fixed.
It's unclear why the GPU is left enabled during suspend on the
mainline kernel. Everything seems fine if I turn this off, so let's
do it.
Signed-off-by: Douglas Anderson
---
arch/arm/boot/dts/rk3288-veyron.dtsi | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/rk3288-veyron.dtsi
b/arch/arm/boot/dts/rk3288-veyron.dtsi
index 279d7f4ecce0..1252522392c7 100644
--- a/arch/arm/boot/dts/rk3288-veyron.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron.dtsi
@@ -217,8 +217,7 @@
regulator-max-microvolt = <125>;
regulator-ramp-delay = <6001>;
regulator-state-mem {
- regulator-on-in-suspend;
- regulator-suspend-microvolt = <100>;
+ regulator-off-in-suspend;
};
};
Reviewed-by: Elaine Zhang
Re: [PATCH 4/5] ARM: dts: rockchip: vcc33_ccd off in suspend for rk3288-veyron-chromebook
hi,
在 2019/4/12 上午7:21, Douglas Anderson 写道:
As per my comments when the device tree for rk3288-veyron-chromebook
first landed:
Technically I think vcc33_ccd can be off since we have
'needs-reset-on-resume' down in the EHCI port (this regulator is for
the USB webcam that's connected to the EHCI port).
...but leaving it on for now seems fine until we get suspend/resume
more solid.
It's probably about time to do it right.
[1]
https://lore.kernel.org/linux-arm-kernel/CAD=FV=u37yx8mqk75_x05zxonvdc3qrmhqp8tytdpwghqsu...@mail.gmail.com/
Signed-off-by: Douglas Anderson
---
arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
b/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
index b9cc90f0f25c..fbef34578100 100644
--- a/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
@@ -176,8 +176,7 @@
regulator-min-microvolt = <330>;
regulator-max-microvolt = <330>;
regulator-state-mem {
- regulator-on-in-suspend;
- regulator-suspend-microvolt = <330>;
+ regulator-off-in-suspend;
};
};
};
Reviewed-by: Elaine Zhang
Re: [PATCH 1/3] Revert "clk: rockchip: mark noc and some special clk as critical on rk3288"
hi,
在 2019/4/12 上午6:05, Heiko Stübner 写道:
Hi,
Am Donnerstag, 11. April 2019, 17:26:41 CEST schrieb Doug Anderson:
On Wed, Apr 10, 2019 at 8:27 PM elaine.zhang wrote:
在 2019/4/10 下午11:34, Doug Anderson 写道:
On Tue, Apr 9, 2019 at 11:23 PM elaine.zhang wrote:
在 2019/4/10 上午4:47, Douglas Anderson 写道:
This reverts commit 55bb6a633c33caf68ab470907ecf945289cb733d.
The clocks that were enabled by that patch are pretty questionable.
Specifically looking at what has been shipping on rk3288-veyron
Chromebooks almost all of these clocks are safely turned off and cause
no apparent problems. If some boards need these clocks turned on for
some reason then it seems like we should figure out how to do that at
a board level.
NOTE: turning these clocks off doesn't seem to do a whole lot in terms
of power savings (checking the power on the logic rail). It appears
to save maybe 1-2mW. ...but still it seems like we should turn the
clocks off if they aren't needed.
Digging into the clocks here to describe why they shouldn't need to be
left on:
atclk: No documentation about this clock other than that it goes to
the CPU. CPU functions fine without it on.
jtag: Presumably this clock is only needed if you're debugging with
JTAG. It doesn't seem like it makes sense to waste power for every
rk3288 user. Perhaps this could be turned on with a CONFIG option?
pclk_dbg, pclk_core_niu: On veyron Chromebooks we turn these two
clocks on only during kernel panics in order to access some coresight
registers. Since nothing in the upstream kernel does this we should
be able to leave them off safely.
hsicphy12m_xin12m: There is no indication of why this clock would need
to be turned on for boards that don't use HSIC.
pclk_ddrupctl[0-1], pclk_publ0[0-1]: On veyron Chromebooks we turn
these 4 clocks on only when doing DDR transitions and they are off
otherwise. I see no reason why they'd need to be on in the upstream
kernel which doesn't support DDRFreq.
pmu_hclk_otg0: A "chip design defect" is mentioned in the original
patch but no details. This clock has always been gated in shipping
veyron Chromebooks so presumably this chip defect doesn't affect all
boards.
Signed-off-by: Douglas Anderson
---
drivers/clk/rockchip/clk-rk3288.c | 14 --
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/clk/rockchip/clk-rk3288.c
b/drivers/clk/rockchip/clk-rk3288.c
index 5a67b7869960..06287810474e 100644
--- a/drivers/clk/rockchip/clk-rk3288.c
+++ b/drivers/clk/rockchip/clk-rk3288.c
@@ -313,13 +313,13 @@ static struct rockchip_clk_branch rk3288_clk_branches[]
__initdata = {
COMPOSITE_NOMUX(0, "aclk_core_mp", "armclk", CLK_IGNORE_UNUSED,
RK3288_CLKSEL_CON(0), 4, 4, DFLAGS |
CLK_DIVIDER_READ_ONLY,
RK3288_CLKGATE_CON(12), 6, GFLAGS),
- COMPOSITE_NOMUX(0, "atclk", "armclk", CLK_IGNORE_UNUSED,
+ COMPOSITE_NOMUX(0, "atclk", "armclk", 0,
RK3288_CLKSEL_CON(37), 4, 5, DFLAGS |
CLK_DIVIDER_READ_ONLY,
RK3288_CLKGATE_CON(12), 7, GFLAGS),
COMPOSITE_NOMUX(0, "pclk_dbg_pre", "armclk", CLK_IGNORE_UNUSED,
RK3288_CLKSEL_CON(37), 9, 5, DFLAGS |
CLK_DIVIDER_READ_ONLY,
RK3288_CLKGATE_CON(12), 8, GFLAGS),
- GATE(0, "pclk_dbg", "pclk_dbg_pre", CLK_IGNORE_UNUSED,
+ GATE(0, "pclk_dbg", "pclk_dbg_pre", 0,
RK3288_CLKGATE_CON(12), 9, GFLAGS),
GATE(0, "cs_dbg", "pclk_dbg_pre", CLK_IGNORE_UNUSED,
RK3288_CLKGATE_CON(12), 10, GFLAGS),
@@ -647,7 +647,7 @@ static struct rockchip_clk_branch rk3288_clk_branches[]
__initdata = {
INVERTER(SCLK_HSADC, "sclk_hsadc", "sclk_hsadc_out",
RK3288_CLKSEL_CON(22), 7, IFLAGS),
- GATE(0, "jtag", "ext_jtag", CLK_IGNORE_UNUSED,
+ GATE(0, "jtag", "ext_jtag", 0,
RK3288_CLKGATE_CON(4), 14, GFLAGS),
CLK_IGNORE_UNUSED:
Whether to close the unused clk after clk init complete. not affect
there own enable/disable.
JTAG is not have device node, when need jtag to debug, may be the system
is crashed, there is no way to turn on this clk.
As I mentioned in the commit message this seems like the kind of thing
that should be controlled by a CONFIG_ option. It's a debug option
that's fine to have on all the time but consumes resources so some
people might want to turn it off.
Currently, CONFIG_ option is not implemented. We will refer to this suggestion.
For debug, I don't hope to remove the flag CLK_IGNORE_UNUSED.(The clk static
power is very small)
I'll leave it up to Heiko for what to do here. I agree that it's not
tons of power (this whole patch saved 1-2 mW on the INT rail) but I'd
still prefer for clocks
Re: [PATCH v1 1/3] thermal: rockchip: add pinctrl control
hi,
在 2019/4/4 上午11:03, Daniel Lezcano 写道:
On 01/04/2019 08:43, Elaine Zhang wrote:
Based on the TSADC Tshut mode to select pinctrl,
instead of setting pinctrl based on architecture
(Not depends on pinctrl setting by "init" or "default").
And it requires setting the tshut polarity before select pinctrl.
I'm not sure to fully read the description. Can you rephrase/elaborate
the changelog?
Example:
tsadc: tsadc@ff25 {
compatible = "rockchip,rk3328-tsadc";
.
pinctrl-names = "init", "default", "sleep";
pinctrl-0 = <_gpio>;
pinctrl-1 = <_out>;
pinctrl-2 = <_gpio>;
status = "disabled";
.
};
{
rockchip,hw-tshut-mode = <0>; /* tshut mode 0:CRU 1:GPIO */
status = "okay";
};
Application on the product, hope tsadc overtemperature reset cru to
restart.
But when pinctrl is initialized, it will setting pinctrl by "init"
and "default".
So the pinctrl will set iomux to "otp_out", which may be make
system crash.
tsadc gpio iomux:
"otp_gpio" : just normal gpio, keep default level.
"otp_out" : tsadc shut down io, when overtemperature,this io may be
trigger high
level or low level(depend on the tsadc polarity).
After correction:
if rockchip,hw-tshut-mode = <0>; (tsadc overtemperature reset cru
to restart)
select pinctrl to otp_gpio
if rockchip,hw-tshut-mode = <1>;(tsadc overtemperature IO triggers
output at high or low levels)
select pinctrl to otp_out.
Actively select iomux based on rockchip,hw-tshut-mode,
rather than relying on the pinctrl framework to select iomux.
Signed-off-by: Elaine Zhang
---
drivers/thermal/rockchip_thermal.c | 61 +++---
1 file changed, 50 insertions(+), 11 deletions(-)
diff --git a/drivers/thermal/rockchip_thermal.c
b/drivers/thermal/rockchip_thermal.c
index 9c7643d62ed7..faa6c7792155 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -34,7 +34,7 @@
*/
enum tshut_mode {
TSHUT_MODE_CRU = 0,
- TSHUT_MODE_GPIO,
+ TSHUT_MODE_OTP,
Why do you change the enum name? The impact on the patch is much higher,
no ?
I just want to make it a little bit more intuitive to understand the
definition of mode.
TSHUT_MODE_CRU: pinctrl select iomux to otp_gpio,the io is normal io.
TSHUT_MODE_OTP: pinctrl select iomux to otp_out, the io is tsadc shut io.
};
/**
@@ -172,6 +172,9 @@ struct rockchip_thermal_data {
int tshut_temp;
enum tshut_mode tshut_mode;
enum tshut_polarity tshut_polarity;
+ struct pinctrl *pinctrl;
+ struct pinctrl_state *gpio_state;
+ struct pinctrl_state *otp_state;
};
/**
@@ -807,7 +810,7 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem
*regs,
u32 val;
val = readl_relaxed(regs + TSADCV2_INT_EN);
- if (mode == TSHUT_MODE_GPIO) {
+ if (mode == TSHUT_MODE_OTP) {
val &= ~TSADCV2_SHUT_2CRU_SRC_EN(chn);
val |= TSADCV2_SHUT_2GPIO_SRC_EN(chn);
} else {
@@ -822,7 +825,7 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem
*regs,
.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
.chn_num = 1, /* one channel for tsadc */
- .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
+ .tshut_mode = TSHUT_MODE_OTP, /* default TSHUT via GPIO give PMIC */
.tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
.tshut_temp = 95000,
@@ -846,7 +849,7 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem *regs,
.chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
.chn_num = 1, /* one channel for tsadc */
- .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
+ .tshut_mode = TSHUT_MODE_OTP, /* default TSHUT via GPIO give PMIC */
.tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
.tshut_temp = 95000,
@@ -871,7 +874,7 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem *regs,
.chn_id[SENSOR_GPU] = 2, /* gpu sensor is channel 2 */
.chn_num = 2, /* two channels for tsadc */
- .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
+ .tshut_mode = TSHUT_MODE_OTP, /* default TSHUT via GPIO give PMIC */
.tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
.tshut_temp = 95000,
@@ -919,7 +922,7 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem *regs,
.chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
.chn_num = 2, /* two channels for tsadc */
- .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
+ .tshut_mode = TSHUT_MODE_OTP, /* default TSHUT via GPIO give PMIC */
Re: [PATCH 2/3] clk: rockchip: Make rkpwm a critical clock on rk3288
hi,
在 2019/4/10 下午11:25, Doug Anderson 写道:
Hi,
On Tue, Apr 9, 2019 at 11:42 PM elaine.zhang wrote:
hi,
在 2019/4/10 上午4:47, Douglas Anderson 写道:
Most rk3288-based boards are derived from the EVB and thus use a PWM
regulator for the logic rail. However, most rk3288-based boards don't
specify the PWM regulator in their device tree. We'll deal with that
by making it critical.
NOTE: it's important to make it critical and not just IGNORE_UNUSED
because all PWMs in the system share the same clock. We don't want
another PWM user to turn the clock on and off and kill the logic rail.
This change is in preparation for actually having the PWMs in the
rk3288 device tree actually point to the proper PWM clock. Up until
now they've all pointed to the clock for the old IP block and they've
all worked due to the fact that rkpwm was IGNORE_UNUSED and that the
clock rates for both clocks were the same.
Signed-off-by: Douglas Anderson
---
drivers/clk/rockchip/clk-rk3288.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/rockchip/clk-rk3288.c
b/drivers/clk/rockchip/clk-rk3288.c
index 06287810474e..c3321eade23e 100644
--- a/drivers/clk/rockchip/clk-rk3288.c
+++ b/drivers/clk/rockchip/clk-rk3288.c
@@ -697,7 +697,7 @@ static struct rockchip_clk_branch rk3288_clk_branches[]
__initdata = {
GATE(PCLK_TZPC, "pclk_tzpc", "pclk_cpu", 0, RK3288_CLKGATE_CON(11), 3,
GFLAGS),
GATE(PCLK_UART2, "pclk_uart2", "pclk_cpu", 0, RK3288_CLKGATE_CON(11), 9,
GFLAGS),
GATE(PCLK_EFUSE256, "pclk_efuse_256", "pclk_cpu", 0,
RK3288_CLKGATE_CON(11), 10, GFLAGS),
- GATE(PCLK_RKPWM, "pclk_rkpwm", "pclk_cpu", CLK_IGNORE_UNUSED,
RK3288_CLKGATE_CON(11), 11, GFLAGS),
+ GATE(PCLK_RKPWM, "pclk_rkpwm", "pclk_cpu", 0, RK3288_CLKGATE_CON(11), 11,
GFLAGS),
/* ddrctrl [DDR Controller PHY clock] gates */
GATE(0, "nclk_ddrupctl0", "ddrphy", CLK_IGNORE_UNUSED,
RK3288_CLKGATE_CON(11), 4, GFLAGS),
@@ -837,6 +837,7 @@ static const char *const rk3288_critical_clocks[]
__initconst = {
"pclk_alive_niu",
"pclk_pd_pmu",
"pclk_pmu_niu",
+ "pclk_rkpwm",
pwm have device node, can enable and disable it in the pwm drivers.
pwm regulator use pwm node as:
pwms = < 0 25000 1>
when set Logic voltage:
pwm_regulator_set_voltage()
--> pwm_apply_state()
-->clk_enable()
-->pwm_enable()
-->pwm_config()
-->pinctrl_select()
--
For mark pclk_rkpwm as critical,do you have any questions, or provides
some log or more information.
Right, if we actually specify the PWM used for the PWM regulator in
the device tree then there is no need to mark it as a critical clock.
In fact rk3288-veyron devices boot absolutely fine without marking
this clock as critical. Actually, it seems like the way the PWM
framework works (IIRC it was designed this way specifically to support
PWM regulators) is that even just specifying that pwm1 is "okay" is
enough to keep the clock on even if the PWM regulator isn't specified.
...however...
Take a look at, for instance, the rk3288-evb device tree file.
Nowhere in there does it specify that the PWM used for the PWM
regulator should be on. Presumably that means that if we don't mark
the clock as critical then rk3288-evb will fail to boot. That's easy
for me to fix since I have the rk3288-evb schematics, but what about
other rk3288 boards? We could make educated guesses about each of
them and/or fix things are we hear about breakages.
...but...
All the above would only be worth doing if we thought someone would
get some benefit out of it. I'd bet that pretty much all rk3288-based
boards use a PWM regulator. Thus, in reality, everyone will want the
rkpwm clock on all the time anyway. In that case going through all
that extra work / potentially breaking other boards doesn't seem worth
it. Just mark the clock as critical.
I have no problem with changing it like this, but I think it is better
to modify dts:
vdd_log: vdd-log {
compatible = "pwm-regulator";
rockchip,pwm_id = <2>; //for rk uboot
rockchip,pwm_voltage = <90>; // for rk uboot
pwms = < 0 25000 1>;
regulator-name = "vdd_log";
regulator-min-microvolt = <80>;//hw logic min voltage
regulator-max-microvolt = <140>;//hw logic max voltage
regulator-always-on;
regulator-boot-on;
};
Maybe we did not push the modification of this part in rk3288-evb, I
will push to deal with this.(rk3229-evb.dts and rk3399 has been already
pushed)
-Doug
Re: [PATCH] clk: rockchip: Fix video codec clocks on rk3288
hi,
在 2019/4/11 上午7:37, Doug Anderson 写道:
Hi,
On Wed, Apr 10, 2019 at 11:38 AM Jonas Karlman wrote:
On 2019-04-10 17:45, Doug Anderson wrote:
Hi,
On Fri, Mar 29, 2019 at 2:55 PM Douglas Anderson wrote:
It appears that there is a typo in the rk3288 TRM. For
GRF_SOC_CON0[7] it says that 0 means "vepu" and 1 means "vdpu". It's
the other way around.
How do I know? Here's my evidence:
1. Prior to commit 4d3e84f99628 ("clk: rockchip: describe aclk_vcodec
using the new muxgrf type on rk3288") we always pretended that we
were using "aclk_vdpu" and the comment in the code said that this
matched the default setting in the system. In fact the default
setting is 0 according to the TRM and according to reading memory
at bootup. In addition rk3288-based Chromebooks ran like this and
the video codecs worked.
2. With the existing clock code if you boot up and try to enable the
new VIDEO_ROCKCHIP_VPU as a module (and without "clk_ignore_unused"
on the command line), you get errors like "failed to get ack on
domain 'pd_video', val=0x80208". After flipping vepu/vdpu things
init OK.
3. If I export and add both the vepu and vdpu to the list of clocks
for RK3288_PD_VIDEO I can get past the power domain errors, but now
I freeze when the vpu_mmu gets initted.
4. If I just mark the "vdpu" as IGNORE_UNUSED then everything boots up
and probes OK showing that somehow the "vdpu" was important to keep
enabled. This is because we were actually using it as a parent.
5. After this change I can hack "aclk_vcodec_pre" to parent from
"aclk_vepu" using assigned-clocks and the video codec still probes
OK.
Fixes: 4d3e84f99628 ("clk: rockchip: describe aclk_vcodec using the new muxgrf type
on rk3288")
Signed-off-by: Douglas Anderson
---
I currently have no way to test the JPEG mem2mem driver, so hopefully
others can test this and make sure it's happy for them. I'm just
happy not to get strange errors at boot anymore.
drivers/clk/rockchip/clk-rk3288.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Any thoughts about this patch? I'm 99.9% certain it's correct and
it'd be nice to get it landed. Heiko: I assume you're still
collecting Rockchip clock patches and would be the one to apply it and
(at some point) send a pull request to the clock tree?
-Doug
This clk fix is needed to make MPEG-2 decoding work on my RK3288 Tinker Board
using the
rockchip vpu patchset and a patch to add RK3288 specific MPEG-2 code [1].
Also note that the same change was suggested in a previous patch [2] from by
ayaka.
If possible please also add the CLK_SET_RATE_PARENT change from [3] in a
possible v2,
that fixes assigning the aclk_vcodec clk to 400Mhz in the rockchip vpu driver.
[1]
https://github.com/Kwiboo/linux-rockchip/commit/1f78093e05c7360515a185f48b7c5cb8ba1e3e15
[2] https://patchwork.kernel.org/patch/9725553/
[3]
https://github.com/Kwiboo/linux-rockchip/commit/9216da3f1521a0be5889235abe7fa093a4894160
Thanks for the pointers! Now I'm 99.% certain that my patch is
correct instead of just 99.9%. ;-)
IMO the CLK_SET_RATE_PARENT should probably be a separate patch but it
does seem like we need it. ...and actually the patch adding it should
have a Fixes tag of the same commit. Prior to that commit the parent
of "aclk_vcodec" was "aclk_vdpu". As a gate clock it would
automatically have CLK_SET_RATE_PARENT so you could easily set the
rate. ...and it looks as if the downstream video codec driver in
Chrome OS relies on this too.
I'm happy to add that in a v2 if Heiko is happy with it.
I also agree with this modification, which is completely correct. Thank
you for your modification.
(We didn't push for this change because the GRF_SOC_CON0[7] is setting
in vcodec driver(RK mainline branch))
-Doug
Re: [PATCH 2/3] clk: rockchip: Make rkpwm a critical clock on rk3288
hi,
在 2019/4/10 上午4:47, Douglas Anderson 写道:
Most rk3288-based boards are derived from the EVB and thus use a PWM
regulator for the logic rail. However, most rk3288-based boards don't
specify the PWM regulator in their device tree. We'll deal with that
by making it critical.
NOTE: it's important to make it critical and not just IGNORE_UNUSED
because all PWMs in the system share the same clock. We don't want
another PWM user to turn the clock on and off and kill the logic rail.
This change is in preparation for actually having the PWMs in the
rk3288 device tree actually point to the proper PWM clock. Up until
now they've all pointed to the clock for the old IP block and they've
all worked due to the fact that rkpwm was IGNORE_UNUSED and that the
clock rates for both clocks were the same.
Signed-off-by: Douglas Anderson
---
drivers/clk/rockchip/clk-rk3288.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/rockchip/clk-rk3288.c
b/drivers/clk/rockchip/clk-rk3288.c
index 06287810474e..c3321eade23e 100644
--- a/drivers/clk/rockchip/clk-rk3288.c
+++ b/drivers/clk/rockchip/clk-rk3288.c
@@ -697,7 +697,7 @@ static struct rockchip_clk_branch rk3288_clk_branches[]
__initdata = {
GATE(PCLK_TZPC, "pclk_tzpc", "pclk_cpu", 0, RK3288_CLKGATE_CON(11), 3,
GFLAGS),
GATE(PCLK_UART2, "pclk_uart2", "pclk_cpu", 0, RK3288_CLKGATE_CON(11),
9, GFLAGS),
GATE(PCLK_EFUSE256, "pclk_efuse_256", "pclk_cpu", 0,
RK3288_CLKGATE_CON(11), 10, GFLAGS),
- GATE(PCLK_RKPWM, "pclk_rkpwm", "pclk_cpu", CLK_IGNORE_UNUSED,
RK3288_CLKGATE_CON(11), 11, GFLAGS),
+ GATE(PCLK_RKPWM, "pclk_rkpwm", "pclk_cpu", 0, RK3288_CLKGATE_CON(11),
11, GFLAGS),
/* ddrctrl [DDR Controller PHY clock] gates */
GATE(0, "nclk_ddrupctl0", "ddrphy", CLK_IGNORE_UNUSED,
RK3288_CLKGATE_CON(11), 4, GFLAGS),
@@ -837,6 +837,7 @@ static const char *const rk3288_critical_clocks[]
__initconst = {
"pclk_alive_niu",
"pclk_pd_pmu",
"pclk_pmu_niu",
+ "pclk_rkpwm",
pwm have device node, can enable and disable it in the pwm drivers.
pwm regulator use pwm node as:
pwms = < 0 25000 1>
when set Logic voltage:
pwm_regulator_set_voltage()
--> pwm_apply_state()
-->clk_enable()
-->pwm_enable()
-->pwm_config()
-->pinctrl_select()
--
For mark pclk_rkpwm as critical,do you have any questions, or provides
some log or more information.
};
static void __iomem *rk3288_cru_base;
Re: [PATCH 1/3] Revert "clk: rockchip: mark noc and some special clk as critical on rk3288"
hi,
在 2019/4/10 上午4:47, Douglas Anderson 写道:
This reverts commit 55bb6a633c33caf68ab470907ecf945289cb733d.
The clocks that were enabled by that patch are pretty questionable.
Specifically looking at what has been shipping on rk3288-veyron
Chromebooks almost all of these clocks are safely turned off and cause
no apparent problems. If some boards need these clocks turned on for
some reason then it seems like we should figure out how to do that at
a board level.
NOTE: turning these clocks off doesn't seem to do a whole lot in terms
of power savings (checking the power on the logic rail). It appears
to save maybe 1-2mW. ...but still it seems like we should turn the
clocks off if they aren't needed.
Digging into the clocks here to describe why they shouldn't need to be
left on:
atclk: No documentation about this clock other than that it goes to
the CPU. CPU functions fine without it on.
jtag: Presumably this clock is only needed if you're debugging with
JTAG. It doesn't seem like it makes sense to waste power for every
rk3288 user. Perhaps this could be turned on with a CONFIG option?
pclk_dbg, pclk_core_niu: On veyron Chromebooks we turn these two
clocks on only during kernel panics in order to access some coresight
registers. Since nothing in the upstream kernel does this we should
be able to leave them off safely.
hsicphy12m_xin12m: There is no indication of why this clock would need
to be turned on for boards that don't use HSIC.
pclk_ddrupctl[0-1], pclk_publ0[0-1]: On veyron Chromebooks we turn
these 4 clocks on only when doing DDR transitions and they are off
otherwise. I see no reason why they'd need to be on in the upstream
kernel which doesn't support DDRFreq.
pmu_hclk_otg0: A "chip design defect" is mentioned in the original
patch but no details. This clock has always been gated in shipping
veyron Chromebooks so presumably this chip defect doesn't affect all
boards.
Signed-off-by: Douglas Anderson
---
drivers/clk/rockchip/clk-rk3288.c | 14 --
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/clk/rockchip/clk-rk3288.c
b/drivers/clk/rockchip/clk-rk3288.c
index 5a67b7869960..06287810474e 100644
--- a/drivers/clk/rockchip/clk-rk3288.c
+++ b/drivers/clk/rockchip/clk-rk3288.c
@@ -313,13 +313,13 @@ static struct rockchip_clk_branch rk3288_clk_branches[]
__initdata = {
COMPOSITE_NOMUX(0, "aclk_core_mp", "armclk", CLK_IGNORE_UNUSED,
RK3288_CLKSEL_CON(0), 4, 4, DFLAGS |
CLK_DIVIDER_READ_ONLY,
RK3288_CLKGATE_CON(12), 6, GFLAGS),
- COMPOSITE_NOMUX(0, "atclk", "armclk", CLK_IGNORE_UNUSED,
+ COMPOSITE_NOMUX(0, "atclk", "armclk", 0,
RK3288_CLKSEL_CON(37), 4, 5, DFLAGS |
CLK_DIVIDER_READ_ONLY,
RK3288_CLKGATE_CON(12), 7, GFLAGS),
COMPOSITE_NOMUX(0, "pclk_dbg_pre", "armclk", CLK_IGNORE_UNUSED,
RK3288_CLKSEL_CON(37), 9, 5, DFLAGS |
CLK_DIVIDER_READ_ONLY,
RK3288_CLKGATE_CON(12), 8, GFLAGS),
- GATE(0, "pclk_dbg", "pclk_dbg_pre", CLK_IGNORE_UNUSED,
+ GATE(0, "pclk_dbg", "pclk_dbg_pre", 0,
RK3288_CLKGATE_CON(12), 9, GFLAGS),
GATE(0, "cs_dbg", "pclk_dbg_pre", CLK_IGNORE_UNUSED,
RK3288_CLKGATE_CON(12), 10, GFLAGS),
@@ -647,7 +647,7 @@ static struct rockchip_clk_branch rk3288_clk_branches[]
__initdata = {
INVERTER(SCLK_HSADC, "sclk_hsadc", "sclk_hsadc_out",
RK3288_CLKSEL_CON(22), 7, IFLAGS),
- GATE(0, "jtag", "ext_jtag", CLK_IGNORE_UNUSED,
+ GATE(0, "jtag", "ext_jtag", 0,
RK3288_CLKGATE_CON(4), 14, GFLAGS),
CLK_IGNORE_UNUSED:
Whether to close the unused clk after clk init complete. not affect
there own enable/disable.
JTAG is not have device node, when need jtag to debug, may be the system
is crashed, there is no way to turn on this clk.
COMPOSITE_NODIV(SCLK_USBPHY480M_SRC, "usbphy480m_src", mux_usbphy480m_p, 0,
@@ -656,7 +656,7 @@ static struct rockchip_clk_branch rk3288_clk_branches[]
__initdata = {
COMPOSITE_NODIV(SCLK_HSICPHY480M, "sclk_hsicphy480m",
mux_hsicphy480m_p, 0,
RK3288_CLKSEL_CON(29), 0, 2, MFLAGS,
RK3288_CLKGATE_CON(3), 6, GFLAGS),
- GATE(0, "hsicphy12m_xin12m", "xin12m", CLK_IGNORE_UNUSED,
+ GATE(0, "hsicphy12m_xin12m", "xin12m", 0,
RK3288_CLKGATE_CON(13), 9, GFLAGS),
DIV(0, "hsicphy12m_usbphy", "sclk_hsicphy480m", 0,
RK3288_CLKSEL_CON(11), 8, 6, DFLAGS),
@@ -837,12 +837,6 @@ static const char *const rk3288_critical_clocks[]
__initconst = {
"pclk_alive_niu",
"pclk_pd_pmu",
"pclk_pmu_niu",
- "pclk_core_niu",
- "pclk_ddrupctl0",
- "pclk_publ0",
- "pclk_ddrupctl1",
- "pclk_publ1",
These clks needed enable, device node not use this clk, so we mark it as
