* [PATCH v4 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq
@ 2024-12-02 15:12 Christian Marangi
2024-12-02 15:12 ` [PATCH v4 2/2] cpufreq: airoha: Add EN7581 CPUFreq SMCCC driver Christian Marangi
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Christian Marangi @ 2024-12-02 15:12 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Christian Marangi, linux-pm, devicetree,
linux-kernel, upstream
Document required property for Airoha EN7581 CPUFreq .
On newer Airoha SoC, CPU Frequency is scaled indirectly with SMCCC commands
to ATF and no clocks are exposed to the OS.
The SoC have performance state described by ID for each OPP, for this a
Power Domain is used that sets the performance state ID according to the
required OPPs defined in the CPU OPP tables.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
Changes v4:
- Add this patch
.../cpufreq/airoha,en7581-cpufreq.yaml | 259 ++++++++++++++++++
1 file changed, 259 insertions(+)
create mode 100644 Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
diff --git a/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
new file mode 100644
index 000000000000..a5bdea7f34b5
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
@@ -0,0 +1,259 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/cpufreq/airoha,en7581-cpufreq.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Airoha EN7581 CPUFreq
+
+maintainers:
+ - Christian Marangi <ansuelsmth@gmail.com>
+
+description: |
+ On newer Airoha SoC, CPU Frequency is scaled indirectly with SMCCC commands
+ to ATF and no clocks are exposed to the OS.
+
+ The SoC have performance state described by ID for each OPP, for this a
+ Power Domain is used that sets the performance state ID according to the
+ required OPPs defined in the CPU OPP tables.
+
+properties:
+ compatible:
+ const: airoha,en7581-cpufreq
+
+ '#clock-cells':
+ const: 0
+
+ '#power-domain-cells':
+ const: 0
+
+ operating-points-v2: true
+
+required:
+ - compatible
+ - '#clock-cells'
+ - '#power-domain-cells'
+ - operating-points-v2
+
+additionalProperties: false
+
+examples:
+ - |
+ / {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpu0: cpu@0 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x0>;
+ operating-points-v2 = <&cpu_opp_table>;
+ enable-method = "psci";
+ clocks = <&cpufreq>;
+ clock-names = "cpu";
+ power-domains = <&cpufreq>;
+ power-domain-names = "cpu_pd";
+ next-level-cache = <&l2>;
+ #cooling-cells = <2>;
+ };
+
+ cpu1: cpu@1 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x1>;
+ operating-points-v2 = <&cpu_opp_table>;
+ enable-method = "psci";
+ clocks = <&cpufreq>;
+ clock-names = "cpu";
+ power-domains = <&cpufreq>;
+ power-domain-names = "cpu_pd";
+ next-level-cache = <&l2>;
+ #cooling-cells = <2>;
+ };
+
+ cpu2: cpu@2 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x2>;
+ operating-points-v2 = <&cpu_opp_table>;
+ enable-method = "psci";
+ clocks = <&cpufreq>;
+ clock-names = "cpu";
+ power-domains = <&cpufreq>;
+ power-domain-names = "cpu_pd";
+ next-level-cache = <&l2>;
+ #cooling-cells = <2>;
+ };
+
+ cpu3: cpu@3 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x3>;
+ operating-points-v2 = <&cpu_opp_table>;
+ enable-method = "psci";
+ clocks = <&cpufreq>;
+ clock-names = "cpu";
+ power-domains = <&cpufreq>;
+ power-domain-names = "cpu_pd";
+ next-level-cache = <&l2>;
+ #cooling-cells = <2>;
+ };
+ };
+
+ cpu_opp_table: opp-table {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-500000000 {
+ opp-hz = /bits/ 64 <500000000>;
+ required-opps = <&smcc_opp0>;
+ };
+
+ opp-550000000 {
+ opp-hz = /bits/ 64 <550000000>;
+ required-opps = <&smcc_opp1>;
+ };
+
+ opp-600000000 {
+ opp-hz = /bits/ 64 <600000000>;
+ required-opps = <&smcc_opp2>;
+ };
+
+ opp-650000000 {
+ opp-hz = /bits/ 64 <650000000>;
+ required-opps = <&smcc_opp3>;
+ };
+
+ opp-7000000000 {
+ opp-hz = /bits/ 64 <700000000>;
+ required-opps = <&smcc_opp4>;
+ };
+
+ opp-7500000000 {
+ opp-hz = /bits/ 64 <750000000>;
+ required-opps = <&smcc_opp5>;
+ };
+
+ opp-8000000000 {
+ opp-hz = /bits/ 64 <800000000>;
+ required-opps = <&smcc_opp6>;
+ };
+
+ opp-8500000000 {
+ opp-hz = /bits/ 64 <850000000>;
+ required-opps = <&smcc_opp7>;
+ };
+
+ opp-9000000000 {
+ opp-hz = /bits/ 64 <900000000>;
+ required-opps = <&smcc_opp8>;
+ };
+
+ opp-9500000000 {
+ opp-hz = /bits/ 64 <950000000>;
+ required-opps = <&smcc_opp9>;
+ };
+
+ opp-10000000000 {
+ opp-hz = /bits/ 64 <1000000000>;
+ required-opps = <&smcc_opp10>;
+ };
+
+ opp-10500000000 {
+ opp-hz = /bits/ 64 <1050000000>;
+ required-opps = <&smcc_opp11>;
+ };
+
+ opp-11000000000 {
+ opp-hz = /bits/ 64 <1100000000>;
+ required-opps = <&smcc_opp12>;
+ };
+
+ opp-11500000000 {
+ opp-hz = /bits/ 64 <1150000000>;
+ required-opps = <&smcc_opp13>;
+ };
+
+ opp-12000000000 {
+ opp-hz = /bits/ 64 <1200000000>;
+ required-opps = <&smcc_opp14>;
+ };
+ };
+
+ cpu_smcc_opp_table: opp-table-cpu-smcc {
+ compatible = "operating-points-v2";
+
+ smcc_opp0: opp0 {
+ opp-level = <0>;
+ };
+
+ smcc_opp1: opp1 {
+ opp-level = <1>;
+ };
+
+ smcc_opp2: opp2 {
+ opp-level = <2>;
+ };
+
+ smcc_opp3: opp3 {
+ opp-level = <3>;
+ };
+
+ smcc_opp4: opp4 {
+ opp-level = <4>;
+ };
+
+ smcc_opp5: opp5 {
+ opp-level = <5>;
+ };
+
+ smcc_opp6: opp6 {
+ opp-level = <6>;
+ };
+
+ smcc_opp7: opp7 {
+ opp-level = <7>;
+ };
+
+ smcc_opp8: opp8 {
+ opp-level = <8>;
+ };
+
+ smcc_opp9: opp9 {
+ opp-level = <9>;
+ };
+
+ smcc_opp10: opp10 {
+ opp-level = <10>;
+ };
+
+ smcc_opp11: opp11 {
+ opp-level = <11>;
+ };
+
+ smcc_opp12: opp12 {
+ opp-level = <12>;
+ };
+
+ smcc_opp13: opp13 {
+ opp-level = <13>;
+ };
+
+ smcc_opp14: opp14 {
+ opp-level = <14>;
+ };
+ };
+
+ cpufreq: cpufreq {
+ compatible = "airoha,en7581-cpufreq";
+
+ operating-points-v2 = <&cpu_smcc_opp_table>;
+
+ #power-domain-cells = <0>;
+ #clock-cells = <0>;
+ };
+ };
--
2.45.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4 2/2] cpufreq: airoha: Add EN7581 CPUFreq SMCCC driver
2024-12-02 15:12 [PATCH v4 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq Christian Marangi
@ 2024-12-02 15:12 ` Christian Marangi
2024-12-02 20:38 ` Uwe Kleine-König
2024-12-02 15:42 ` [PATCH v4 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq Ulf Hansson
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Christian Marangi @ 2024-12-02 15:12 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Christian Marangi, linux-pm, devicetree,
linux-kernel, upstream
Add simple CPU Freq driver for Airoha EN7581 SoC that control CPU
frequency scaling with SMC APIs and register a generic "cpufreq-dt"
device.
CPUFreq driver registers a get-only clock to get the current global CPU
frequency from SMC and a Power Domain to configure the performance state
for each OPP to apply the requested frequency from cpufreq-dt. This is
needed as SMC use index instead of raw frequency.
All CPU share the same frequency and can't be controlled independently.
Current shared CPU frequency is returned by the related SMC command.
Add SoC compatible to cpufreq-dt-plat block list as a dedicated cpufreq
driver is needed with OPP v2 nodes declared in DTS.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
Changes v4:
- Rework to clk-only + PM set_performance_state implementation
Changes v3:
- Adapt to new cpufreq-dt APIs
- Register cpufreq-dt instead of custom freq driver
Changes v2:
- Fix kernel bot error with missing slab.h and bitfield.h header
- Limit COMPILE_TEST to ARM64 due to smcc 1.2
drivers/cpufreq/Kconfig.arm | 9 ++
drivers/cpufreq/Makefile | 1 +
drivers/cpufreq/airoha-cpufreq.c | 222 +++++++++++++++++++++++++++
drivers/cpufreq/cpufreq-dt-platdev.c | 2 +
4 files changed, 234 insertions(+)
create mode 100644 drivers/cpufreq/airoha-cpufreq.c
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 5f7e13e60c80..b6f72ee41364 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -15,6 +15,15 @@ config ARM_ALLWINNER_SUN50I_CPUFREQ_NVMEM
To compile this driver as a module, choose M here: the
module will be called sun50i-cpufreq-nvmem.
+config ARM_AIROHA_SOC_CPUFREQ
+ tristate "Airoha EN7581 SoC CPUFreq support"
+ depends on ARCH_AIROHA || (COMPILE_TEST && ARM64)
+ select PM_OPP
+ select PM_GENERIC_DOMAINS
+ default ARCH_AIROHA
+ help
+ This adds the CPUFreq driver for Airoha EN7581 SoCs.
+
config ARM_APPLE_SOC_CPUFREQ
tristate "Apple Silicon SoC CPUFreq support"
depends on ARCH_APPLE || (COMPILE_TEST && 64BIT)
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 0f184031dd12..8e5a37a95d36 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_X86_AMD_FREQ_SENSITIVITY) += amd_freq_sensitivity.o
##################################################################################
# ARM SoC drivers
+obj-$(CONFIG_ARM_AIROHA_SOC_CPUFREQ) += airoha-cpufreq.o
obj-$(CONFIG_ARM_APPLE_SOC_CPUFREQ) += apple-soc-cpufreq.o
obj-$(CONFIG_ARM_ARMADA_37XX_CPUFREQ) += armada-37xx-cpufreq.o
obj-$(CONFIG_ARM_ARMADA_8K_CPUFREQ) += armada-8k-cpufreq.o
diff --git a/drivers/cpufreq/airoha-cpufreq.c b/drivers/cpufreq/airoha-cpufreq.c
new file mode 100644
index 000000000000..46f57236c4db
--- /dev/null
+++ b/drivers/cpufreq/airoha-cpufreq.c
@@ -0,0 +1,222 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/arm-smccc.h>
+#include <linux/bitfield.h>
+#include <linux/cpufreq.h>
+#include <linux/clk-provider.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+
+#include "cpufreq-dt.h"
+
+#define AIROHA_SIP_AVS_HANDLE 0x82000301
+#define AIROHA_AVS_OP_BASE 0xddddddd0
+#define AIROHA_AVS_OP_MASK GENMASK(1, 0)
+#define AIROHA_AVS_OP_FREQ_DYN_ADJ (AIROHA_AVS_OP_BASE | \
+ FIELD_PREP(AIROHA_AVS_OP_MASK, 0x1))
+#define AIROHA_AVS_OP_GET_FREQ (AIROHA_AVS_OP_BASE | \
+ FIELD_PREP(AIROHA_AVS_OP_MASK, 0x2))
+
+struct airoha_cpufreq_priv {
+ struct clk_hw hw;
+ struct generic_pm_domain pd;
+
+ int opp_token;
+ struct dev_pm_domain_list *pd_list;
+ struct platform_device *cpufreq_dt;
+};
+
+static long airoha_cpufreq_clk_round(struct clk_hw *hw, unsigned long rate,
+ unsigned long *parent_rate)
+{
+ return rate;
+}
+
+static unsigned long airoha_cpufreq_clk_get(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ const struct arm_smccc_1_2_regs args = {
+ .a0 = AIROHA_SIP_AVS_HANDLE,
+ .a1 = AIROHA_AVS_OP_GET_FREQ,
+ };
+ struct arm_smccc_1_2_regs res;
+
+ arm_smccc_1_2_smc(&args, &res);
+
+ /* SMCCC returns freq in MHz */
+ return (int)(res.a0 * 1000 * 1000);
+}
+
+/* Airoha CPU clk SMCC is always enabled */
+static int airoha_cpufreq_clk_is_enabled(struct clk_hw *hw)
+{
+ return true;
+}
+
+static const struct clk_ops airoha_cpufreq_clk_ops = {
+ .recalc_rate = airoha_cpufreq_clk_get,
+ .is_enabled = airoha_cpufreq_clk_is_enabled,
+ .round_rate = airoha_cpufreq_clk_round,
+};
+
+static const char * const airoha_cpufreq_clk_names[] = { "cpu", NULL };
+
+/* NOP function to disable OPP from setting clock */
+static int airoha_cpufreq_config_clks_nop(struct device *dev,
+ struct opp_table *opp_table,
+ struct dev_pm_opp *opp,
+ void *data, bool scaling_down)
+{
+ return 0;
+}
+
+static const char * const airoha_cpufreq_pd_names[] = { "cpu_pd" };
+
+static int airoha_cpufreq_set_performance_state(struct generic_pm_domain *domain,
+ unsigned int state)
+{
+ const struct arm_smccc_1_2_regs args = {
+ .a0 = AIROHA_SIP_AVS_HANDLE,
+ .a1 = AIROHA_AVS_OP_FREQ_DYN_ADJ,
+ .a3 = state,
+ };
+ struct arm_smccc_1_2_regs res;
+
+ arm_smccc_1_2_smc(&args, &res);
+
+ /* SMC signal correct apply by unsetting BIT 0 */
+ return res.a0 & BIT(0) ? -EINVAL : 0;
+}
+
+static int airoha_cpufreq_probe(struct platform_device *pdev)
+{
+ const struct dev_pm_domain_attach_data attach_data = {
+ .pd_names = airoha_cpufreq_pd_names,
+ .num_pd_names = ARRAY_SIZE(airoha_cpufreq_pd_names),
+ .pd_flags = PD_FLAG_DEV_LINK_ON | PD_FLAG_REQUIRED_OPP,
+ };
+ struct dev_pm_opp_config config = {
+ .clk_names = airoha_cpufreq_clk_names,
+ .config_clks = airoha_cpufreq_config_clks_nop,
+ };
+ struct platform_device *cpufreq_dt;
+ struct airoha_cpufreq_priv *priv;
+ struct device *dev = &pdev->dev;
+ const struct clk_init_data init = {
+ .name = "cpu",
+ .ops = &airoha_cpufreq_clk_ops,
+ /* Clock with no set_rate, can't cache */
+ .flags = CLK_GET_RATE_NOCACHE,
+ };
+ struct generic_pm_domain *pd;
+ struct device *cpu_dev;
+ int ret;
+
+ /* CPUs refer to the same OPP table */
+ cpu_dev = get_cpu_device(0);
+ if (!cpu_dev)
+ return -ENODEV;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ /* Init and register a get-only clk for Cpufreq */
+ priv->hw.init = &init;
+ ret = devm_clk_hw_register(dev, &priv->hw);
+ if (ret)
+ return ret;
+
+ ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
+ &priv->hw);
+ if (ret)
+ return ret;
+
+ /* Init and register a PD for Cpufreq */
+ pd = &priv->pd;
+ pd->name = "cpu_pd";
+ pd->flags = GENPD_FLAG_ALWAYS_ON;
+ pd->set_performance_state = airoha_cpufreq_set_performance_state;
+
+ ret = pm_genpd_init(pd, NULL, false);
+ if (ret)
+ return ret;
+
+ ret = of_genpd_add_provider_simple(dev->of_node, pd);
+ if (ret)
+ goto err_add_provider;
+
+ /* Set OPP table conf with NOP config_clks */
+ priv->opp_token = dev_pm_opp_set_config(cpu_dev, &config);
+ if (priv->opp_token < 0) {
+ ret = priv->opp_token;
+ dev_err(dev, "Failed to set OPP config\n");
+ goto err_set_config;
+ }
+
+ /* Attach PM for OPP */
+ ret = dev_pm_domain_attach_list(cpu_dev, &attach_data,
+ &priv->pd_list);
+ if (ret)
+ goto err_attach_pm;
+
+ cpufreq_dt = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
+ ret = PTR_ERR_OR_ZERO(cpufreq_dt);
+ if (ret) {
+ dev_err(dev, "failed to create cpufreq-dt device: %d\n", ret);
+ goto err_register_cpufreq;
+ }
+
+ priv->cpufreq_dt = cpufreq_dt;
+ platform_set_drvdata(pdev, priv);
+
+ return 0;
+
+err_register_cpufreq:
+ dev_pm_domain_detach_list(priv->pd_list);
+err_attach_pm:
+ dev_pm_opp_clear_config(priv->opp_token);
+err_set_config:
+ of_genpd_del_provider(dev->of_node);
+err_add_provider:
+ pm_genpd_remove(pd);
+
+ return ret;
+}
+
+static void airoha_cpufreq_remove(struct platform_device *pdev)
+{
+ struct airoha_cpufreq_priv *priv = platform_get_drvdata(pdev);
+
+ platform_device_unregister(priv->cpufreq_dt);
+
+ dev_pm_domain_detach_list(priv->pd_list);
+
+ dev_pm_opp_clear_config(priv->opp_token);
+
+ of_genpd_del_provider(pdev->dev.of_node);
+ pm_genpd_remove(&priv->pd);
+}
+
+static const struct of_device_id airoha_cpufreq_of_match[] = {
+ { .compatible = "airoha,en7581-cpufreq" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, airoha_cpufreq_of_match);
+
+static struct platform_driver airoha_cpufreq_driver = {
+ .probe = airoha_cpufreq_probe,
+ .remove_new = airoha_cpufreq_remove,
+ .driver = {
+ .name = "airoha-cpufreq",
+ .of_match_table = airoha_cpufreq_of_match,
+ },
+};
+module_platform_driver(airoha_cpufreq_driver);
+
+MODULE_AUTHOR("Christian Marangi <ansuelsmth@gmail.com>");
+MODULE_DESCRIPTION("CPUfreq driver for Airoha SoCs");
+MODULE_LICENSE("GPL");
diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
index 18942bfe9c95..5ecd8234bfac 100644
--- a/drivers/cpufreq/cpufreq-dt-platdev.c
+++ b/drivers/cpufreq/cpufreq-dt-platdev.c
@@ -103,6 +103,8 @@ static const struct of_device_id allowlist[] __initconst = {
* platforms using "operating-points-v2" property.
*/
static const struct of_device_id blocklist[] __initconst = {
+ { .compatible = "airoha,en7581", },
+
{ .compatible = "allwinner,sun50i-h6", },
{ .compatible = "allwinner,sun50i-h616", },
{ .compatible = "allwinner,sun50i-h618", },
--
2.45.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq
2024-12-02 15:12 [PATCH v4 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq Christian Marangi
2024-12-02 15:12 ` [PATCH v4 2/2] cpufreq: airoha: Add EN7581 CPUFreq SMCCC driver Christian Marangi
@ 2024-12-02 15:42 ` Ulf Hansson
2024-12-02 15:45 ` Christian Marangi
2024-12-02 16:16 ` Rob Herring (Arm)
2024-12-03 4:41 ` Viresh Kumar
3 siblings, 1 reply; 16+ messages in thread
From: Ulf Hansson @ 2024-12-02 15:42 UTC (permalink / raw)
To: Christian Marangi
Cc: Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-pm, devicetree, linux-kernel, upstream
On Mon, 2 Dec 2024 at 16:20, Christian Marangi <ansuelsmth@gmail.com> wrote:
>
> Document required property for Airoha EN7581 CPUFreq .
>
> On newer Airoha SoC, CPU Frequency is scaled indirectly with SMCCC commands
> to ATF and no clocks are exposed to the OS.
>
> The SoC have performance state described by ID for each OPP, for this a
> Power Domain is used that sets the performance state ID according to the
> required OPPs defined in the CPU OPP tables.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
> Changes v4:
> - Add this patch
>
> .../cpufreq/airoha,en7581-cpufreq.yaml | 259 ++++++++++++++++++
> 1 file changed, 259 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
>
> diff --git a/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> new file mode 100644
> index 000000000000..a5bdea7f34b5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
[...]
> +examples:
> + - |
> + / {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu0: cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x0>;
> + operating-points-v2 = <&cpu_opp_table>;
> + enable-method = "psci";
> + clocks = <&cpufreq>;
> + clock-names = "cpu";
> + power-domains = <&cpufreq>;
> + power-domain-names = "cpu_pd";
Nitpick: Perhaps clarify the name to be "perf" or "cpu_perf", to
indicate it's a power-domain with performance scaling support.
> + next-level-cache = <&l2>;
> + #cooling-cells = <2>;
> + };
> +
[...]
Other than the very minor thing above, feel free to add:
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq
2024-12-02 15:42 ` [PATCH v4 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq Ulf Hansson
@ 2024-12-02 15:45 ` Christian Marangi
2024-12-02 20:57 ` Rob Herring
0 siblings, 1 reply; 16+ messages in thread
From: Christian Marangi @ 2024-12-02 15:45 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-pm, devicetree, linux-kernel, upstream
On Mon, Dec 02, 2024 at 04:42:33PM +0100, Ulf Hansson wrote:
> On Mon, 2 Dec 2024 at 16:20, Christian Marangi <ansuelsmth@gmail.com> wrote:
> >
> > Document required property for Airoha EN7581 CPUFreq .
> >
> > On newer Airoha SoC, CPU Frequency is scaled indirectly with SMCCC commands
> > to ATF and no clocks are exposed to the OS.
> >
> > The SoC have performance state described by ID for each OPP, for this a
> > Power Domain is used that sets the performance state ID according to the
> > required OPPs defined in the CPU OPP tables.
> >
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> > Changes v4:
> > - Add this patch
> >
> > .../cpufreq/airoha,en7581-cpufreq.yaml | 259 ++++++++++++++++++
> > 1 file changed, 259 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> > new file mode 100644
> > index 000000000000..a5bdea7f34b5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
>
> [...]
>
> > +examples:
> > + - |
> > + / {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > +
> > + cpus {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + cpu0: cpu@0 {
> > + device_type = "cpu";
> > + compatible = "arm,cortex-a53";
> > + reg = <0x0>;
> > + operating-points-v2 = <&cpu_opp_table>;
> > + enable-method = "psci";
> > + clocks = <&cpufreq>;
> > + clock-names = "cpu";
> > + power-domains = <&cpufreq>;
> > + power-domain-names = "cpu_pd";
>
> Nitpick: Perhaps clarify the name to be "perf" or "cpu_perf", to
> indicate it's a power-domain with performance scaling support.
>
Will change to cpu_perf. Thanks a lot for the review!
> > + next-level-cache = <&l2>;
> > + #cooling-cells = <2>;
> > + };
> > +
>
> [...]
>
> Other than the very minor thing above, feel free to add:
>
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
>
> Kind regards
> Uffe
--
Ansuel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq
2024-12-02 15:12 [PATCH v4 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq Christian Marangi
2024-12-02 15:12 ` [PATCH v4 2/2] cpufreq: airoha: Add EN7581 CPUFreq SMCCC driver Christian Marangi
2024-12-02 15:42 ` [PATCH v4 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq Ulf Hansson
@ 2024-12-02 16:16 ` Rob Herring (Arm)
2024-12-03 4:41 ` Viresh Kumar
3 siblings, 0 replies; 16+ messages in thread
From: Rob Herring (Arm) @ 2024-12-02 16:16 UTC (permalink / raw)
To: Christian Marangi
Cc: linux-kernel, Viresh Kumar, upstream, linux-pm, Rafael J. Wysocki,
devicetree, Conor Dooley, Krzysztof Kozlowski
On Mon, 02 Dec 2024 16:12:03 +0100, Christian Marangi wrote:
> Document required property for Airoha EN7581 CPUFreq .
>
> On newer Airoha SoC, CPU Frequency is scaled indirectly with SMCCC commands
> to ATF and no clocks are exposed to the OS.
>
> The SoC have performance state described by ID for each OPP, for this a
> Power Domain is used that sets the performance state ID according to the
> required OPPs defined in the CPU OPP tables.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
> Changes v4:
> - Add this patch
>
> .../cpufreq/airoha,en7581-cpufreq.yaml | 259 ++++++++++++++++++
> 1 file changed, 259 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.example.dtb: /: 'compatible' is a required property
from schema $id: http://devicetree.org/schemas/root-node.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.example.dtb: /: 'model' is a required property
from schema $id: http://devicetree.org/schemas/root-node.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.example.dtb: opp-table-cpu-smcc: $nodename:0: 'opp-table-cpu-smcc' does not match '^opp-table(-[a-z0-9]+)?$'
from schema $id: http://devicetree.org/schemas/opp/opp-v2.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.example.dtb: opp-table-cpu-smcc: Unevaluated properties are not allowed ('opp0', 'opp1', 'opp10', 'opp11', 'opp12', 'opp13', 'opp14', 'opp2', 'opp3', 'opp4', 'opp5', 'opp6', 'opp7', 'opp8', 'opp9' were unexpected)
from schema $id: http://devicetree.org/schemas/opp/opp-v2.yaml#
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241202151228.32609-1-ansuelsmth@gmail.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 2/2] cpufreq: airoha: Add EN7581 CPUFreq SMCCC driver
2024-12-02 15:12 ` [PATCH v4 2/2] cpufreq: airoha: Add EN7581 CPUFreq SMCCC driver Christian Marangi
@ 2024-12-02 20:38 ` Uwe Kleine-König
2024-12-02 20:47 ` Christian Marangi
0 siblings, 1 reply; 16+ messages in thread
From: Uwe Kleine-König @ 2024-12-02 20:38 UTC (permalink / raw)
To: Christian Marangi
Cc: Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-pm, devicetree, linux-kernel, upstream
[-- Attachment #1: Type: text/plain, Size: 376 bytes --]
hello,
On Mon, Dec 02, 2024 at 04:12:04PM +0100, Christian Marangi wrote:
> +static struct platform_driver airoha_cpufreq_driver = {
> + .probe = airoha_cpufreq_probe,
> + .remove_new = airoha_cpufreq_remove,
That doesn't build on top of mainline since commit e70140ba0d2b ("Get rid of 'remove_new' relic from platform driver struct")
Please use .remove.
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 2/2] cpufreq: airoha: Add EN7581 CPUFreq SMCCC driver
2024-12-02 20:38 ` Uwe Kleine-König
@ 2024-12-02 20:47 ` Christian Marangi
2024-12-03 7:44 ` Uwe Kleine-König
0 siblings, 1 reply; 16+ messages in thread
From: Christian Marangi @ 2024-12-02 20:47 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-pm, devicetree, linux-kernel, upstream
On Mon, Dec 02, 2024 at 09:38:10PM +0100, Uwe Kleine-König wrote:
> hello,
>
> On Mon, Dec 02, 2024 at 04:12:04PM +0100, Christian Marangi wrote:
> > +static struct platform_driver airoha_cpufreq_driver = {
> > + .probe = airoha_cpufreq_probe,
> > + .remove_new = airoha_cpufreq_remove,
>
> That doesn't build on top of mainline since commit e70140ba0d2b ("Get rid of 'remove_new' relic from platform driver struct")
>
> Please use .remove.
>
Hi Uwe,
I rebased this on top of next and still compiled but yes sorry it's a
leftover from testing this also on my OpenWrt buildroot. (seems the
commit still needs to be merged)
Maybe you can point me what branch/repository I should target?
Did you notice other problem with the changes? I notice the PD
implementation changed recently.
--
Ansuel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq
2024-12-02 15:45 ` Christian Marangi
@ 2024-12-02 20:57 ` Rob Herring
2024-12-02 20:59 ` Christian Marangi
2024-12-03 10:33 ` Ulf Hansson
0 siblings, 2 replies; 16+ messages in thread
From: Rob Herring @ 2024-12-02 20:57 UTC (permalink / raw)
To: Christian Marangi
Cc: Ulf Hansson, Rafael J. Wysocki, Viresh Kumar, Krzysztof Kozlowski,
Conor Dooley, linux-pm, devicetree, linux-kernel, upstream
On Mon, Dec 02, 2024 at 04:45:17PM +0100, Christian Marangi wrote:
> On Mon, Dec 02, 2024 at 04:42:33PM +0100, Ulf Hansson wrote:
> > On Mon, 2 Dec 2024 at 16:20, Christian Marangi <ansuelsmth@gmail.com> wrote:
> > >
> > > Document required property for Airoha EN7581 CPUFreq .
> > >
> > > On newer Airoha SoC, CPU Frequency is scaled indirectly with SMCCC commands
> > > to ATF and no clocks are exposed to the OS.
> > >
> > > The SoC have performance state described by ID for each OPP, for this a
> > > Power Domain is used that sets the performance state ID according to the
> > > required OPPs defined in the CPU OPP tables.
> > >
> > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > > ---
> > > Changes v4:
> > > - Add this patch
> > >
> > > .../cpufreq/airoha,en7581-cpufreq.yaml | 259 ++++++++++++++++++
> > > 1 file changed, 259 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> > > new file mode 100644
> > > index 000000000000..a5bdea7f34b5
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> >
> > [...]
> >
> > > +examples:
> > > + - |
> > > + / {
> > > + #address-cells = <2>;
> > > + #size-cells = <2>;
> > > +
> > > + cpus {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + cpu0: cpu@0 {
> > > + device_type = "cpu";
> > > + compatible = "arm,cortex-a53";
> > > + reg = <0x0>;
> > > + operating-points-v2 = <&cpu_opp_table>;
> > > + enable-method = "psci";
> > > + clocks = <&cpufreq>;
> > > + clock-names = "cpu";
> > > + power-domains = <&cpufreq>;
> > > + power-domain-names = "cpu_pd";
> >
> > Nitpick: Perhaps clarify the name to be "perf" or "cpu_perf", to
> > indicate it's a power-domain with performance scaling support.
> >
>
> Will change to cpu_perf. Thanks a lot for the review!
Is that defined in arm/cpus.yaml? No.
The current choices are perf or psci though those aren't enforced (yet).
Or nothing which is my preference if there is only 1 power domain.
Rob
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq
2024-12-02 20:57 ` Rob Herring
@ 2024-12-02 20:59 ` Christian Marangi
2024-12-03 10:36 ` Ulf Hansson
2024-12-03 10:33 ` Ulf Hansson
1 sibling, 1 reply; 16+ messages in thread
From: Christian Marangi @ 2024-12-02 20:59 UTC (permalink / raw)
To: Rob Herring
Cc: Ulf Hansson, Rafael J. Wysocki, Viresh Kumar, Krzysztof Kozlowski,
Conor Dooley, linux-pm, devicetree, linux-kernel, upstream
On Mon, Dec 02, 2024 at 02:57:38PM -0600, Rob Herring wrote:
> On Mon, Dec 02, 2024 at 04:45:17PM +0100, Christian Marangi wrote:
> > On Mon, Dec 02, 2024 at 04:42:33PM +0100, Ulf Hansson wrote:
> > > On Mon, 2 Dec 2024 at 16:20, Christian Marangi <ansuelsmth@gmail.com> wrote:
> > > >
> > > > Document required property for Airoha EN7581 CPUFreq .
> > > >
> > > > On newer Airoha SoC, CPU Frequency is scaled indirectly with SMCCC commands
> > > > to ATF and no clocks are exposed to the OS.
> > > >
> > > > The SoC have performance state described by ID for each OPP, for this a
> > > > Power Domain is used that sets the performance state ID according to the
> > > > required OPPs defined in the CPU OPP tables.
> > > >
> > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > > > ---
> > > > Changes v4:
> > > > - Add this patch
> > > >
> > > > .../cpufreq/airoha,en7581-cpufreq.yaml | 259 ++++++++++++++++++
> > > > 1 file changed, 259 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> > > > new file mode 100644
> > > > index 000000000000..a5bdea7f34b5
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> > >
> > > [...]
> > >
> > > > +examples:
> > > > + - |
> > > > + / {
> > > > + #address-cells = <2>;
> > > > + #size-cells = <2>;
> > > > +
> > > > + cpus {
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > +
> > > > + cpu0: cpu@0 {
> > > > + device_type = "cpu";
> > > > + compatible = "arm,cortex-a53";
> > > > + reg = <0x0>;
> > > > + operating-points-v2 = <&cpu_opp_table>;
> > > > + enable-method = "psci";
> > > > + clocks = <&cpufreq>;
> > > > + clock-names = "cpu";
> > > > + power-domains = <&cpufreq>;
> > > > + power-domain-names = "cpu_pd";
> > >
> > > Nitpick: Perhaps clarify the name to be "perf" or "cpu_perf", to
> > > indicate it's a power-domain with performance scaling support.
> > >
> >
> > Will change to cpu_perf. Thanks a lot for the review!
>
> Is that defined in arm/cpus.yaml? No.
>
> The current choices are perf or psci though those aren't enforced (yet).
> Or nothing which is my preference if there is only 1 power domain.
>
I would also prefer not having to define the names property but I guess
that is mandatory for the PD APIs? Maybe Ulf can confirm.
--
Ansuel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq
2024-12-02 15:12 [PATCH v4 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq Christian Marangi
` (2 preceding siblings ...)
2024-12-02 16:16 ` Rob Herring (Arm)
@ 2024-12-03 4:41 ` Viresh Kumar
2024-12-03 10:22 ` Ulf Hansson
3 siblings, 1 reply; 16+ messages in thread
From: Viresh Kumar @ 2024-12-03 4:41 UTC (permalink / raw)
To: Christian Marangi, ulf.hansson
Cc: Rafael J. Wysocki, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-pm, devicetree, linux-kernel, upstream
+Ulf,
On 02-12-24, 16:12, Christian Marangi wrote:
> Document required property for Airoha EN7581 CPUFreq .
>
> On newer Airoha SoC, CPU Frequency is scaled indirectly with SMCCC commands
> to ATF and no clocks are exposed to the OS.
>
> The SoC have performance state described by ID for each OPP, for this a
> Power Domain is used that sets the performance state ID according to the
> required OPPs defined in the CPU OPP tables.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
> Changes v4:
> - Add this patch
>
> .../cpufreq/airoha,en7581-cpufreq.yaml | 259 ++++++++++++++++++
> 1 file changed, 259 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
>
> diff --git a/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> new file mode 100644
> index 000000000000..a5bdea7f34b5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> @@ -0,0 +1,259 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/cpufreq/airoha,en7581-cpufreq.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Airoha EN7581 CPUFreq
> +
> +maintainers:
> + - Christian Marangi <ansuelsmth@gmail.com>
> +
> +description: |
> + On newer Airoha SoC, CPU Frequency is scaled indirectly with SMCCC commands
> + to ATF and no clocks are exposed to the OS.
> +
> + The SoC have performance state described by ID for each OPP, for this a
> + Power Domain is used that sets the performance state ID according to the
> + required OPPs defined in the CPU OPP tables.
> +
> +properties:
> + compatible:
> + const: airoha,en7581-cpufreq
> +
> + '#clock-cells':
> + const: 0
> +
> + '#power-domain-cells':
> + const: 0
> +
> + operating-points-v2: true
> +
> +required:
> + - compatible
> + - '#clock-cells'
> + - '#power-domain-cells'
> + - operating-points-v2
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + / {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu0: cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x0>;
> + operating-points-v2 = <&cpu_opp_table>;
> + enable-method = "psci";
> + clocks = <&cpufreq>;
> + clock-names = "cpu";
> + power-domains = <&cpufreq>;
> + power-domain-names = "cpu_pd";
> + next-level-cache = <&l2>;
> + #cooling-cells = <2>;
> + };
> +
> + cpu1: cpu@1 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x1>;
> + operating-points-v2 = <&cpu_opp_table>;
> + enable-method = "psci";
> + clocks = <&cpufreq>;
> + clock-names = "cpu";
> + power-domains = <&cpufreq>;
> + power-domain-names = "cpu_pd";
> + next-level-cache = <&l2>;
> + #cooling-cells = <2>;
> + };
> +
> + cpu2: cpu@2 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x2>;
> + operating-points-v2 = <&cpu_opp_table>;
> + enable-method = "psci";
> + clocks = <&cpufreq>;
> + clock-names = "cpu";
> + power-domains = <&cpufreq>;
> + power-domain-names = "cpu_pd";
> + next-level-cache = <&l2>;
> + #cooling-cells = <2>;
> + };
> +
> + cpu3: cpu@3 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x3>;
> + operating-points-v2 = <&cpu_opp_table>;
> + enable-method = "psci";
> + clocks = <&cpufreq>;
> + clock-names = "cpu";
> + power-domains = <&cpufreq>;
> + power-domain-names = "cpu_pd";
> + next-level-cache = <&l2>;
> + #cooling-cells = <2>;
> + };
> + };
> +
> + cpu_opp_table: opp-table {
> + compatible = "operating-points-v2";
> + opp-shared;
> +
> + opp-500000000 {
> + opp-hz = /bits/ 64 <500000000>;
> + required-opps = <&smcc_opp0>;
In your case I think you can simply mention opp-level here itself and remove the
OPP table for the genpd. Right Ulf ?
> + };
> +
> + opp-550000000 {
> + opp-hz = /bits/ 64 <550000000>;
> + required-opps = <&smcc_opp1>;
> + };
> +
> + opp-600000000 {
> + opp-hz = /bits/ 64 <600000000>;
> + required-opps = <&smcc_opp2>;
> + };
> +
> + opp-650000000 {
> + opp-hz = /bits/ 64 <650000000>;
> + required-opps = <&smcc_opp3>;
> + };
> +
> + opp-7000000000 {
> + opp-hz = /bits/ 64 <700000000>;
> + required-opps = <&smcc_opp4>;
> + };
> +
> + opp-7500000000 {
> + opp-hz = /bits/ 64 <750000000>;
> + required-opps = <&smcc_opp5>;
> + };
> +
> + opp-8000000000 {
> + opp-hz = /bits/ 64 <800000000>;
> + required-opps = <&smcc_opp6>;
> + };
> +
> + opp-8500000000 {
> + opp-hz = /bits/ 64 <850000000>;
> + required-opps = <&smcc_opp7>;
> + };
> +
> + opp-9000000000 {
> + opp-hz = /bits/ 64 <900000000>;
> + required-opps = <&smcc_opp8>;
> + };
> +
> + opp-9500000000 {
> + opp-hz = /bits/ 64 <950000000>;
> + required-opps = <&smcc_opp9>;
> + };
> +
> + opp-10000000000 {
> + opp-hz = /bits/ 64 <1000000000>;
> + required-opps = <&smcc_opp10>;
> + };
> +
> + opp-10500000000 {
> + opp-hz = /bits/ 64 <1050000000>;
> + required-opps = <&smcc_opp11>;
> + };
> +
> + opp-11000000000 {
> + opp-hz = /bits/ 64 <1100000000>;
> + required-opps = <&smcc_opp12>;
> + };
> +
> + opp-11500000000 {
> + opp-hz = /bits/ 64 <1150000000>;
> + required-opps = <&smcc_opp13>;
> + };
> +
> + opp-12000000000 {
> + opp-hz = /bits/ 64 <1200000000>;
> + required-opps = <&smcc_opp14>;
> + };
> + };
> +
> + cpu_smcc_opp_table: opp-table-cpu-smcc {
So this won't be required I guess.
> + compatible = "operating-points-v2";
> +
> + smcc_opp0: opp0 {
> + opp-level = <0>;
> + };
> +
> + smcc_opp1: opp1 {
> + opp-level = <1>;
> + };
> +
> + smcc_opp2: opp2 {
> + opp-level = <2>;
> + };
> +
> + smcc_opp3: opp3 {
> + opp-level = <3>;
> + };
> +
> + smcc_opp4: opp4 {
> + opp-level = <4>;
> + };
> +
> + smcc_opp5: opp5 {
> + opp-level = <5>;
> + };
> +
> + smcc_opp6: opp6 {
> + opp-level = <6>;
> + };
> +
> + smcc_opp7: opp7 {
> + opp-level = <7>;
> + };
> +
> + smcc_opp8: opp8 {
> + opp-level = <8>;
> + };
> +
> + smcc_opp9: opp9 {
> + opp-level = <9>;
> + };
> +
> + smcc_opp10: opp10 {
> + opp-level = <10>;
> + };
> +
> + smcc_opp11: opp11 {
> + opp-level = <11>;
> + };
> +
> + smcc_opp12: opp12 {
> + opp-level = <12>;
> + };
> +
> + smcc_opp13: opp13 {
> + opp-level = <13>;
> + };
> +
> + smcc_opp14: opp14 {
> + opp-level = <14>;
> + };
> + };
> +
> + cpufreq: cpufreq {
And I would name it like a genpd, instead of cpufreq. But I am not sure what's
the right name is..
> + compatible = "airoha,en7581-cpufreq";
> +
> + operating-points-v2 = <&cpu_smcc_opp_table>;
> +
> + #power-domain-cells = <0>;
> + #clock-cells = <0>;
> + };
> + };
> --
> 2.45.2
--
viresh
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 2/2] cpufreq: airoha: Add EN7581 CPUFreq SMCCC driver
2024-12-02 20:47 ` Christian Marangi
@ 2024-12-03 7:44 ` Uwe Kleine-König
0 siblings, 0 replies; 16+ messages in thread
From: Uwe Kleine-König @ 2024-12-03 7:44 UTC (permalink / raw)
To: Christian Marangi
Cc: Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-pm, devicetree, linux-kernel, upstream
[-- Attachment #1: Type: text/plain, Size: 1260 bytes --]
Hello,
On Mon, Dec 02, 2024 at 09:47:58PM +0100, Christian Marangi wrote:
> On Mon, Dec 02, 2024 at 09:38:10PM +0100, Uwe Kleine-König wrote:
> > hello,
> >
> > On Mon, Dec 02, 2024 at 04:12:04PM +0100, Christian Marangi wrote:
> > > +static struct platform_driver airoha_cpufreq_driver = {
> > > + .probe = airoha_cpufreq_probe,
> > > + .remove_new = airoha_cpufreq_remove,
> >
> > That doesn't build on top of mainline since commit e70140ba0d2b ("Get rid of 'remove_new' relic from platform driver struct")
> >
> > Please use .remove.
>
> Hi Uwe,
> I rebased this on top of next and still compiled but yes sorry it's a
> leftover from testing this also on my OpenWrt buildroot. (seems the
> commit still needs to be merged)
The relevant commits are:
- Since v6.3-rc1~106^2~108 ("platform: Provide a remove callback that
returns no value") there is .remove_new()
- Since v6.11-rc1~49^2~37^2 ("platform: Make platform_driver::remove()
return void") the prototype of .remove() changed to return void
- Since e70140ba0d2b ("Get rid of 'remove_new' relic from platform
driver struct") (which is currently only in Linus Torvalds repo and
probably in next starting today) .remove_new is gone.
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq
2024-12-03 4:41 ` Viresh Kumar
@ 2024-12-03 10:22 ` Ulf Hansson
0 siblings, 0 replies; 16+ messages in thread
From: Ulf Hansson @ 2024-12-03 10:22 UTC (permalink / raw)
To: Viresh Kumar, Christian Marangi
Cc: Rafael J. Wysocki, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-pm, devicetree, linux-kernel, upstream
On Tue, 3 Dec 2024 at 05:41, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> +Ulf,
>
> On 02-12-24, 16:12, Christian Marangi wrote:
> > Document required property for Airoha EN7581 CPUFreq .
> >
> > On newer Airoha SoC, CPU Frequency is scaled indirectly with SMCCC commands
> > to ATF and no clocks are exposed to the OS.
> >
> > The SoC have performance state described by ID for each OPP, for this a
> > Power Domain is used that sets the performance state ID according to the
> > required OPPs defined in the CPU OPP tables.
> >
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> > Changes v4:
> > - Add this patch
> >
> > .../cpufreq/airoha,en7581-cpufreq.yaml | 259 ++++++++++++++++++
> > 1 file changed, 259 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> > new file mode 100644
> > index 000000000000..a5bdea7f34b5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> > @@ -0,0 +1,259 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/cpufreq/airoha,en7581-cpufreq.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Airoha EN7581 CPUFreq
> > +
> > +maintainers:
> > + - Christian Marangi <ansuelsmth@gmail.com>
> > +
> > +description: |
> > + On newer Airoha SoC, CPU Frequency is scaled indirectly with SMCCC commands
> > + to ATF and no clocks are exposed to the OS.
> > +
> > + The SoC have performance state described by ID for each OPP, for this a
> > + Power Domain is used that sets the performance state ID according to the
> > + required OPPs defined in the CPU OPP tables.
> > +
> > +properties:
> > + compatible:
> > + const: airoha,en7581-cpufreq
> > +
> > + '#clock-cells':
> > + const: 0
> > +
> > + '#power-domain-cells':
> > + const: 0
> > +
> > + operating-points-v2: true
> > +
> > +required:
> > + - compatible
> > + - '#clock-cells'
> > + - '#power-domain-cells'
> > + - operating-points-v2
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + / {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > +
> > + cpus {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + cpu0: cpu@0 {
> > + device_type = "cpu";
> > + compatible = "arm,cortex-a53";
> > + reg = <0x0>;
> > + operating-points-v2 = <&cpu_opp_table>;
> > + enable-method = "psci";
> > + clocks = <&cpufreq>;
> > + clock-names = "cpu";
> > + power-domains = <&cpufreq>;
> > + power-domain-names = "cpu_pd";
> > + next-level-cache = <&l2>;
> > + #cooling-cells = <2>;
> > + };
> > +
> > + cpu1: cpu@1 {
> > + device_type = "cpu";
> > + compatible = "arm,cortex-a53";
> > + reg = <0x1>;
> > + operating-points-v2 = <&cpu_opp_table>;
> > + enable-method = "psci";
> > + clocks = <&cpufreq>;
> > + clock-names = "cpu";
> > + power-domains = <&cpufreq>;
> > + power-domain-names = "cpu_pd";
> > + next-level-cache = <&l2>;
> > + #cooling-cells = <2>;
> > + };
> > +
> > + cpu2: cpu@2 {
> > + device_type = "cpu";
> > + compatible = "arm,cortex-a53";
> > + reg = <0x2>;
> > + operating-points-v2 = <&cpu_opp_table>;
> > + enable-method = "psci";
> > + clocks = <&cpufreq>;
> > + clock-names = "cpu";
> > + power-domains = <&cpufreq>;
> > + power-domain-names = "cpu_pd";
> > + next-level-cache = <&l2>;
> > + #cooling-cells = <2>;
> > + };
> > +
> > + cpu3: cpu@3 {
> > + device_type = "cpu";
> > + compatible = "arm,cortex-a53";
> > + reg = <0x3>;
> > + operating-points-v2 = <&cpu_opp_table>;
> > + enable-method = "psci";
> > + clocks = <&cpufreq>;
> > + clock-names = "cpu";
> > + power-domains = <&cpufreq>;
> > + power-domain-names = "cpu_pd";
> > + next-level-cache = <&l2>;
> > + #cooling-cells = <2>;
> > + };
> > + };
> > +
> > + cpu_opp_table: opp-table {
> > + compatible = "operating-points-v2";
> > + opp-shared;
> > +
> > + opp-500000000 {
> > + opp-hz = /bits/ 64 <500000000>;
> > + required-opps = <&smcc_opp0>;
>
> In your case I think you can simply mention opp-level here itself and remove the
> OPP table for the genpd. Right Ulf ?
No, that would not be the correct thing to do, I think.
The power-domain described here is modeling a performance-domain,
hence it needs an opp-table associated with it.
>
> > + };
> > +
> > + opp-550000000 {
> > + opp-hz = /bits/ 64 <550000000>;
> > + required-opps = <&smcc_opp1>;
> > + };
> > +
> > + opp-600000000 {
> > + opp-hz = /bits/ 64 <600000000>;
> > + required-opps = <&smcc_opp2>;
> > + };
> > +
> > + opp-650000000 {
> > + opp-hz = /bits/ 64 <650000000>;
> > + required-opps = <&smcc_opp3>;
> > + };
> > +
> > + opp-7000000000 {
> > + opp-hz = /bits/ 64 <700000000>;
> > + required-opps = <&smcc_opp4>;
> > + };
> > +
> > + opp-7500000000 {
> > + opp-hz = /bits/ 64 <750000000>;
> > + required-opps = <&smcc_opp5>;
> > + };
> > +
> > + opp-8000000000 {
> > + opp-hz = /bits/ 64 <800000000>;
> > + required-opps = <&smcc_opp6>;
> > + };
> > +
> > + opp-8500000000 {
> > + opp-hz = /bits/ 64 <850000000>;
> > + required-opps = <&smcc_opp7>;
> > + };
> > +
> > + opp-9000000000 {
> > + opp-hz = /bits/ 64 <900000000>;
> > + required-opps = <&smcc_opp8>;
> > + };
> > +
> > + opp-9500000000 {
> > + opp-hz = /bits/ 64 <950000000>;
> > + required-opps = <&smcc_opp9>;
> > + };
> > +
> > + opp-10000000000 {
> > + opp-hz = /bits/ 64 <1000000000>;
> > + required-opps = <&smcc_opp10>;
> > + };
> > +
> > + opp-10500000000 {
> > + opp-hz = /bits/ 64 <1050000000>;
> > + required-opps = <&smcc_opp11>;
> > + };
> > +
> > + opp-11000000000 {
> > + opp-hz = /bits/ 64 <1100000000>;
> > + required-opps = <&smcc_opp12>;
> > + };
> > +
> > + opp-11500000000 {
> > + opp-hz = /bits/ 64 <1150000000>;
> > + required-opps = <&smcc_opp13>;
> > + };
> > +
> > + opp-12000000000 {
> > + opp-hz = /bits/ 64 <1200000000>;
> > + required-opps = <&smcc_opp14>;
> > + };
> > + };
> > +
> > + cpu_smcc_opp_table: opp-table-cpu-smcc {
>
> So this won't be required I guess.
See above, I think the OPP table here is the correct way to describe it.
>
> > + compatible = "operating-points-v2";
> > +
> > + smcc_opp0: opp0 {
> > + opp-level = <0>;
> > + };
> > +
> > + smcc_opp1: opp1 {
> > + opp-level = <1>;
> > + };
> > +
> > + smcc_opp2: opp2 {
> > + opp-level = <2>;
> > + };
> > +
> > + smcc_opp3: opp3 {
> > + opp-level = <3>;
> > + };
> > +
> > + smcc_opp4: opp4 {
> > + opp-level = <4>;
> > + };
> > +
> > + smcc_opp5: opp5 {
> > + opp-level = <5>;
> > + };
> > +
> > + smcc_opp6: opp6 {
> > + opp-level = <6>;
> > + };
> > +
> > + smcc_opp7: opp7 {
> > + opp-level = <7>;
> > + };
> > +
> > + smcc_opp8: opp8 {
> > + opp-level = <8>;
> > + };
> > +
> > + smcc_opp9: opp9 {
> > + opp-level = <9>;
> > + };
> > +
> > + smcc_opp10: opp10 {
> > + opp-level = <10>;
> > + };
> > +
> > + smcc_opp11: opp11 {
> > + opp-level = <11>;
> > + };
> > +
> > + smcc_opp12: opp12 {
> > + opp-level = <12>;
> > + };
> > +
> > + smcc_opp13: opp13 {
> > + opp-level = <13>;
> > + };
> > +
> > + smcc_opp14: opp14 {
> > + opp-level = <14>;
> > + };
> > + };
> > +
> > + cpufreq: cpufreq {
>
> And I would name it like a genpd, instead of cpufreq. But I am not sure what's
> the right name is..
Good point!
The node name should follow
Documentation/devicetree/bindings/power/power-domain.yaml, which
states:
pattern: "^(power-controller|power-domain|performance-domain)([@-].*)?$"
>
> > + compatible = "airoha,en7581-cpufreq";
> > +
> > + operating-points-v2 = <&cpu_smcc_opp_table>;
> > +
> > + #power-domain-cells = <0>;
> > + #clock-cells = <0>;
> > + };
> > + };
> > --
> > 2.45.2
>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq
2024-12-02 20:57 ` Rob Herring
2024-12-02 20:59 ` Christian Marangi
@ 2024-12-03 10:33 ` Ulf Hansson
2024-12-03 10:47 ` Christian Marangi
1 sibling, 1 reply; 16+ messages in thread
From: Ulf Hansson @ 2024-12-03 10:33 UTC (permalink / raw)
To: Rob Herring
Cc: Christian Marangi, Rafael J. Wysocki, Viresh Kumar,
Krzysztof Kozlowski, Conor Dooley, linux-pm, devicetree,
linux-kernel, upstream
On Mon, 2 Dec 2024 at 21:57, Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Dec 02, 2024 at 04:45:17PM +0100, Christian Marangi wrote:
> > On Mon, Dec 02, 2024 at 04:42:33PM +0100, Ulf Hansson wrote:
> > > On Mon, 2 Dec 2024 at 16:20, Christian Marangi <ansuelsmth@gmail.com> wrote:
> > > >
> > > > Document required property for Airoha EN7581 CPUFreq .
> > > >
> > > > On newer Airoha SoC, CPU Frequency is scaled indirectly with SMCCC commands
> > > > to ATF and no clocks are exposed to the OS.
> > > >
> > > > The SoC have performance state described by ID for each OPP, for this a
> > > > Power Domain is used that sets the performance state ID according to the
> > > > required OPPs defined in the CPU OPP tables.
> > > >
> > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > > > ---
> > > > Changes v4:
> > > > - Add this patch
> > > >
> > > > .../cpufreq/airoha,en7581-cpufreq.yaml | 259 ++++++++++++++++++
> > > > 1 file changed, 259 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> > > > new file mode 100644
> > > > index 000000000000..a5bdea7f34b5
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> > >
> > > [...]
> > >
> > > > +examples:
> > > > + - |
> > > > + / {
> > > > + #address-cells = <2>;
> > > > + #size-cells = <2>;
> > > > +
> > > > + cpus {
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > +
> > > > + cpu0: cpu@0 {
> > > > + device_type = "cpu";
> > > > + compatible = "arm,cortex-a53";
> > > > + reg = <0x0>;
> > > > + operating-points-v2 = <&cpu_opp_table>;
> > > > + enable-method = "psci";
> > > > + clocks = <&cpufreq>;
> > > > + clock-names = "cpu";
> > > > + power-domains = <&cpufreq>;
> > > > + power-domain-names = "cpu_pd";
> > >
> > > Nitpick: Perhaps clarify the name to be "perf" or "cpu_perf", to
> > > indicate it's a power-domain with performance scaling support.
> > >
> >
> > Will change to cpu_perf. Thanks a lot for the review!
>
> Is that defined in arm/cpus.yaml? No.
>
> The current choices are perf or psci though those aren't enforced (yet).
> Or nothing which is my preference if there is only 1 power domain.
Right. It's not really clear in arm/cpus.yaml what name to use for a
perf domain, except for "perf" for SCMI.
If we want to move towards some alignment, perhaps we should update
the DT doc to make "perf" the common suggestion? I can send a patch if
you think it makes sense?
Even if there is only 1 power-domain at this point, we never know if
another one turns up later, for whatever reasons. That said, isn't it
better to be specific about a name, already at this point?
Kind regards
Uffe
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq
2024-12-02 20:59 ` Christian Marangi
@ 2024-12-03 10:36 ` Ulf Hansson
0 siblings, 0 replies; 16+ messages in thread
From: Ulf Hansson @ 2024-12-03 10:36 UTC (permalink / raw)
To: Christian Marangi
Cc: Rob Herring, Rafael J. Wysocki, Viresh Kumar, Krzysztof Kozlowski,
Conor Dooley, linux-pm, devicetree, linux-kernel, upstream
On Mon, 2 Dec 2024 at 21:59, Christian Marangi <ansuelsmth@gmail.com> wrote:
>
> On Mon, Dec 02, 2024 at 02:57:38PM -0600, Rob Herring wrote:
> > On Mon, Dec 02, 2024 at 04:45:17PM +0100, Christian Marangi wrote:
> > > On Mon, Dec 02, 2024 at 04:42:33PM +0100, Ulf Hansson wrote:
> > > > On Mon, 2 Dec 2024 at 16:20, Christian Marangi <ansuelsmth@gmail.com> wrote:
> > > > >
> > > > > Document required property for Airoha EN7581 CPUFreq .
> > > > >
> > > > > On newer Airoha SoC, CPU Frequency is scaled indirectly with SMCCC commands
> > > > > to ATF and no clocks are exposed to the OS.
> > > > >
> > > > > The SoC have performance state described by ID for each OPP, for this a
> > > > > Power Domain is used that sets the performance state ID according to the
> > > > > required OPPs defined in the CPU OPP tables.
> > > > >
> > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > > > > ---
> > > > > Changes v4:
> > > > > - Add this patch
> > > > >
> > > > > .../cpufreq/airoha,en7581-cpufreq.yaml | 259 ++++++++++++++++++
> > > > > 1 file changed, 259 insertions(+)
> > > > > create mode 100644 Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..a5bdea7f34b5
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> > > >
> > > > [...]
> > > >
> > > > > +examples:
> > > > > + - |
> > > > > + / {
> > > > > + #address-cells = <2>;
> > > > > + #size-cells = <2>;
> > > > > +
> > > > > + cpus {
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > +
> > > > > + cpu0: cpu@0 {
> > > > > + device_type = "cpu";
> > > > > + compatible = "arm,cortex-a53";
> > > > > + reg = <0x0>;
> > > > > + operating-points-v2 = <&cpu_opp_table>;
> > > > > + enable-method = "psci";
> > > > > + clocks = <&cpufreq>;
> > > > > + clock-names = "cpu";
> > > > > + power-domains = <&cpufreq>;
> > > > > + power-domain-names = "cpu_pd";
> > > >
> > > > Nitpick: Perhaps clarify the name to be "perf" or "cpu_perf", to
> > > > indicate it's a power-domain with performance scaling support.
> > > >
> > >
> > > Will change to cpu_perf. Thanks a lot for the review!
> >
> > Is that defined in arm/cpus.yaml? No.
> >
> > The current choices are perf or psci though those aren't enforced (yet).
> > Or nothing which is my preference if there is only 1 power domain.
> >
>
> I would also prefer not having to define the names property but I guess
> that is mandatory for the PD APIs? Maybe Ulf can confirm.
No, that's not needed.
dev_pm_domain_attach() attaches a single PM domain, without requiring a name.
But as stated in the other thread, to be future proof, I would suggest
using a name already at this point. Even if it's not required.
Kind regards
Uffe
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq
2024-12-03 10:33 ` Ulf Hansson
@ 2024-12-03 10:47 ` Christian Marangi
2024-12-03 10:50 ` Viresh Kumar
0 siblings, 1 reply; 16+ messages in thread
From: Christian Marangi @ 2024-12-03 10:47 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rob Herring, Rafael J. Wysocki, Viresh Kumar, Krzysztof Kozlowski,
Conor Dooley, linux-pm, devicetree, linux-kernel, upstream
On Tue, Dec 03, 2024 at 11:33:28AM +0100, Ulf Hansson wrote:
> On Mon, 2 Dec 2024 at 21:57, Rob Herring <robh@kernel.org> wrote:
> >
> > On Mon, Dec 02, 2024 at 04:45:17PM +0100, Christian Marangi wrote:
> > > On Mon, Dec 02, 2024 at 04:42:33PM +0100, Ulf Hansson wrote:
> > > > On Mon, 2 Dec 2024 at 16:20, Christian Marangi <ansuelsmth@gmail.com> wrote:
> > > > >
> > > > > Document required property for Airoha EN7581 CPUFreq .
> > > > >
> > > > > On newer Airoha SoC, CPU Frequency is scaled indirectly with SMCCC commands
> > > > > to ATF and no clocks are exposed to the OS.
> > > > >
> > > > > The SoC have performance state described by ID for each OPP, for this a
> > > > > Power Domain is used that sets the performance state ID according to the
> > > > > required OPPs defined in the CPU OPP tables.
> > > > >
> > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > > > > ---
> > > > > Changes v4:
> > > > > - Add this patch
> > > > >
> > > > > .../cpufreq/airoha,en7581-cpufreq.yaml | 259 ++++++++++++++++++
> > > > > 1 file changed, 259 insertions(+)
> > > > > create mode 100644 Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..a5bdea7f34b5
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> > > >
> > > > [...]
> > > >
> > > > > +examples:
> > > > > + - |
> > > > > + / {
> > > > > + #address-cells = <2>;
> > > > > + #size-cells = <2>;
> > > > > +
> > > > > + cpus {
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > +
> > > > > + cpu0: cpu@0 {
> > > > > + device_type = "cpu";
> > > > > + compatible = "arm,cortex-a53";
> > > > > + reg = <0x0>;
> > > > > + operating-points-v2 = <&cpu_opp_table>;
> > > > > + enable-method = "psci";
> > > > > + clocks = <&cpufreq>;
> > > > > + clock-names = "cpu";
> > > > > + power-domains = <&cpufreq>;
> > > > > + power-domain-names = "cpu_pd";
> > > >
> > > > Nitpick: Perhaps clarify the name to be "perf" or "cpu_perf", to
> > > > indicate it's a power-domain with performance scaling support.
> > > >
> > >
> > > Will change to cpu_perf. Thanks a lot for the review!
> >
> > Is that defined in arm/cpus.yaml? No.
> >
> > The current choices are perf or psci though those aren't enforced (yet).
> > Or nothing which is my preference if there is only 1 power domain.
>
> Right. It's not really clear in arm/cpus.yaml what name to use for a
> perf domain, except for "perf" for SCMI.
>
> If we want to move towards some alignment, perhaps we should update
> the DT doc to make "perf" the common suggestion? I can send a patch if
> you think it makes sense?
>
> Even if there is only 1 power-domain at this point, we never know if
> another one turns up later, for whatever reasons. That said, isn't it
> better to be specific about a name, already at this point?
>
Ok to summarize
- cpu node: use perf for PD
- cpufreq node change node name to power-domain
- Fix OPP format (already done)
Did I miss anything? It's strange these case weren't catch by
dt_binding_check. Sorry, wasn't aware of all these common name.
--
Ansuel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq
2024-12-03 10:47 ` Christian Marangi
@ 2024-12-03 10:50 ` Viresh Kumar
0 siblings, 0 replies; 16+ messages in thread
From: Viresh Kumar @ 2024-12-03 10:50 UTC (permalink / raw)
To: Christian Marangi
Cc: Ulf Hansson, Rob Herring, Rafael J. Wysocki, Krzysztof Kozlowski,
Conor Dooley, linux-pm, devicetree, linux-kernel, upstream
On 03-12-24, 11:47, Christian Marangi wrote:
> Ok to summarize
>
> - cpu node: use perf for PD
> - cpufreq node change node name to power-domain
> - Fix OPP format (already done)
>
> Did I miss anything? It's strange these case weren't catch by
> dt_binding_check. Sorry, wasn't aware of all these common name.
Please also run "scripts/checkpatch.pl --strict" if you haven't already, I think
I saw some inconsistencies in the second patch.
--
viresh
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2024-12-03 10:50 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-02 15:12 [PATCH v4 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq Christian Marangi
2024-12-02 15:12 ` [PATCH v4 2/2] cpufreq: airoha: Add EN7581 CPUFreq SMCCC driver Christian Marangi
2024-12-02 20:38 ` Uwe Kleine-König
2024-12-02 20:47 ` Christian Marangi
2024-12-03 7:44 ` Uwe Kleine-König
2024-12-02 15:42 ` [PATCH v4 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq Ulf Hansson
2024-12-02 15:45 ` Christian Marangi
2024-12-02 20:57 ` Rob Herring
2024-12-02 20:59 ` Christian Marangi
2024-12-03 10:36 ` Ulf Hansson
2024-12-03 10:33 ` Ulf Hansson
2024-12-03 10:47 ` Christian Marangi
2024-12-03 10:50 ` Viresh Kumar
2024-12-02 16:16 ` Rob Herring (Arm)
2024-12-03 4:41 ` Viresh Kumar
2024-12-03 10:22 ` Ulf Hansson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).