[PATCH v5 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq

[PATCH v5 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq

[Christian Marangi]

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>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
---
Changes v5:
- Add Reviewed-by tag
- Fix OPP node name error
- Rename cpufreq node name to power-domain
- Rename CPU node power domain name to perf
- Add model and compatible to example
Changes v4:
- Add this patch

 .../cpufreq/airoha,en7581-cpufreq.yaml        | 262 ++++++++++++++++++
 1 file changed, 262 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..7e36fa037e4b
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
@@ -0,0 +1,262 @@
+# 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:
+  - |
+    / {
+        model = "Airoha EN7581 Evaluation Board";
+        compatible = "airoha,en7581-evb", "airoha,en7581";
+
+        #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 = <&cpu_pd>;
+                clock-names = "cpu";
+                power-domains = <&cpu_pd>;
+                power-domain-names = "perf";
+                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 = <&cpu_pd>;
+                clock-names = "cpu";
+                power-domains = <&cpu_pd>;
+                power-domain-names = "perf";
+                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 = <&cpu_pd>;
+                clock-names = "cpu";
+                power-domains = <&cpu_pd>;
+                power-domain-names = "perf";
+                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 = <&cpu_pd>;
+                clock-names = "cpu";
+                power-domains = <&cpu_pd>;
+                power-domain-names = "perf";
+                next-level-cache = <&l2>;
+                #cooling-cells = <2>;
+            };
+        };
+
+        cpu_opp_table: opp-table-cpu {
+            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-smcc {
+            compatible = "operating-points-v2";
+
+            smcc_opp0: opp-0 {
+               opp-level = <0>;
+            };
+
+            smcc_opp1: opp-1 {
+                opp-level = <1>;
+            };
+
+            smcc_opp2: opp-2 {
+               opp-level = <2>;
+            };
+
+            smcc_opp3: opp-3 {
+               opp-level = <3>;
+            };
+
+            smcc_opp4: opp-4 {
+                opp-level = <4>;
+            };
+
+            smcc_opp5: opp-5 {
+                opp-level = <5>;
+            };
+
+            smcc_opp6: opp-6 {
+               opp-level = <6>;
+            };
+
+            smcc_opp7: opp-7 {
+               opp-level = <7>;
+            };
+
+            smcc_opp8: opp-8 {
+                opp-level = <8>;
+            };
+
+            smcc_opp9: opp-9 {
+               opp-level = <9>;
+            };
+
+            smcc_opp10: opp-10 {
+                opp-level = <10>;
+            };
+
+            smcc_opp11: opp-11 {
+                opp-level = <11>;
+            };
+
+            smcc_opp12: opp-12 {
+                opp-level = <12>;
+            };
+
+            smcc_opp13: opp-13 {
+                opp-level = <13>;
+            };
+
+            smcc_opp14: opp-14 {
+                opp-level = <14>;
+            };
+        };
+
+        cpu_pd: power-domain {
+            compatible = "airoha,en7581-cpufreq";
+
+            operating-points-v2 = <&cpu_smcc_opp_table>;
+
+            #power-domain-cells = <0>;
+            #clock-cells = <0>;
+        };
+    };
-- 
2.45.2

[PATCH v5 2/2] cpufreq: airoha: Add EN7581 CPUFreq SMCCC driver

[Christian Marangi]

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 v5:
- Rename cpu_pd to perf for power domain name
- Use remove instead of remove_new
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 d35a28dd9463..890fff99f37d 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -53,6 +53,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..363b3738adf9
--- /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[] = { "perf" };
+
+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 = 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 9c198bd4f7e9..2aa00769cf09 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-a100" },
 	{ .compatible = "allwinner,sun50i-h6", },
 	{ .compatible = "allwinner,sun50i-h616", },
-- 
2.45.2

Re: [PATCH v5 2/2] cpufreq: airoha: Add EN7581 CPUFreq SMCCC driver

[Viresh Kumar]

On 03-12-24, 17:31, Christian Marangi wrote:
> diff --git a/drivers/cpufreq/airoha-cpufreq.c b/drivers/cpufreq/airoha-cpufreq.c
> +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);

Why casting to "int" when we can return ulong ?

> +}
> +
> +/* 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;
> +}

I wonder whats better here. Provide this helper or provide a dummy clk-set-rate
at the provider itself ?

-- 
viresh

Re: [PATCH v5 2/2] cpufreq: airoha: Add EN7581 CPUFreq SMCCC driver

[Christian Marangi]

On Wed, Dec 04, 2024 at 11:02:11AM +0530, Viresh Kumar wrote:
> On 03-12-24, 17:31, Christian Marangi wrote:
> > diff --git a/drivers/cpufreq/airoha-cpufreq.c b/drivers/cpufreq/airoha-cpufreq.c
> > +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);
> 
> Why casting to "int" when we can return ulong ?
>

Leftover from old. Yes will drop. Coincidentally arm_smccc_1_2_regs
entry are already ulong.

> > +}
> > +
> > +/* 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;
> > +}
> 
> I wonder whats better here. Provide this helper or provide a dummy clk-set-rate
> at the provider itself ?
>

The idea I prefer this is to save a few CPU cycle and also to prevent
bad usage of the CLK if anyone starts to use it. Returning 0 from a set_rate
would provide bad information. Or your idea was to declare a set_rate
and always return an error like -EINVAL? 

-- 
	Ansuel

Re: [PATCH v5 2/2] cpufreq: airoha: Add EN7581 CPUFreq SMCCC driver

[Viresh Kumar]

On 04-12-24, 08:04, Christian Marangi wrote:
> The idea I prefer this is to save a few CPU cycle and also to prevent
> bad usage of the CLK if anyone starts to use it. Returning 0 from a set_rate
> would provide bad information. Or your idea was to declare a set_rate
> and always return an error like -EINVAL? 

Returning error is not okay, as it will make opp-set fail eventually.
I think we are doing the right thing right now.

-- 
viresh

Re: [PATCH v5 2/2] cpufreq: airoha: Add EN7581 CPUFreq SMCCC driver

[kernel test robot]

Hi Christian,

kernel test robot noticed the following build errors:

[auto build test ERROR on rafael-pm/linux-next]
[also build test ERROR on rafael-pm/bleeding-edge robh/for-next linus/master v6.13-rc1 next-20241203]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Christian-Marangi/cpufreq-airoha-Add-EN7581-CPUFreq-SMCCC-driver/20241204-113105
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20241203163158.580-2-ansuelsmth%40gmail.com
patch subject: [PATCH v5 2/2] cpufreq: airoha: Add EN7581 CPUFreq SMCCC driver
config: arm-randconfig-003 (https://download.01.org/0day-ci/archive/20241204/202412041929.8aCqrGnO-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 592c0fe55f6d9a811028b5f3507be91458ab2713)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241204/202412041929.8aCqrGnO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412041929.8aCqrGnO-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/cpufreq/airoha-cpufreq.c:41:34: error: variable has incomplete type 'const struct arm_smccc_1_2_regs'
      41 |         const struct arm_smccc_1_2_regs args = {
         |                                         ^
   drivers/cpufreq/airoha-cpufreq.c:41:15: note: forward declaration of 'struct arm_smccc_1_2_regs'
      41 |         const struct arm_smccc_1_2_regs args = {
         |                      ^
>> drivers/cpufreq/airoha-cpufreq.c:45:28: error: variable has incomplete type 'struct arm_smccc_1_2_regs'
      45 |         struct arm_smccc_1_2_regs res;
         |                                   ^
   drivers/cpufreq/airoha-cpufreq.c:41:15: note: forward declaration of 'struct arm_smccc_1_2_regs'
      41 |         const struct arm_smccc_1_2_regs args = {
         |                      ^
>> drivers/cpufreq/airoha-cpufreq.c:47:2: error: call to undeclared function 'arm_smccc_1_2_smc'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
      47 |         arm_smccc_1_2_smc(&args, &res);
         |         ^
   drivers/cpufreq/airoha-cpufreq.c:81:34: error: variable has incomplete type 'const struct arm_smccc_1_2_regs'
      81 |         const struct arm_smccc_1_2_regs args = {
         |                                         ^
   drivers/cpufreq/airoha-cpufreq.c:81:15: note: forward declaration of 'struct arm_smccc_1_2_regs'
      81 |         const struct arm_smccc_1_2_regs args = {
         |                      ^
   drivers/cpufreq/airoha-cpufreq.c:86:28: error: variable has incomplete type 'struct arm_smccc_1_2_regs'
      86 |         struct arm_smccc_1_2_regs res;
         |                                   ^
   drivers/cpufreq/airoha-cpufreq.c:81:15: note: forward declaration of 'struct arm_smccc_1_2_regs'
      81 |         const struct arm_smccc_1_2_regs args = {
         |                      ^
   drivers/cpufreq/airoha-cpufreq.c:88:2: error: call to undeclared function 'arm_smccc_1_2_smc'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
      88 |         arm_smccc_1_2_smc(&args, &res);
         |         ^
   6 errors generated.


vim +41 drivers/cpufreq/airoha-cpufreq.c

    37	
    38	static unsigned long airoha_cpufreq_clk_get(struct clk_hw *hw,
    39						    unsigned long parent_rate)
    40	{
  > 41		const struct arm_smccc_1_2_regs args = {
    42			.a0 = AIROHA_SIP_AVS_HANDLE,
    43			.a1 = AIROHA_AVS_OP_GET_FREQ,
    44		};
  > 45		struct arm_smccc_1_2_regs res;
    46	
  > 47		arm_smccc_1_2_smc(&args, &res);
    48	
    49		/* SMCCC returns freq in MHz */
    50		return (int)(res.a0 * 1000 * 1000);
    51	}
    52	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v5 2/2] cpufreq: airoha: Add EN7581 CPUFreq SMCCC driver

[kernel test robot]

Hi Christian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on rafael-pm/bleeding-edge robh/for-next linus/master v6.13-rc1 next-20241203]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Christian-Marangi/cpufreq-airoha-Add-EN7581-CPUFreq-SMCCC-driver/20241204-113105
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20241203163158.580-2-ansuelsmth%40gmail.com
patch subject: [PATCH v5 2/2] cpufreq: airoha: Add EN7581 CPUFreq SMCCC driver
config: arm-kismet-CONFIG_PM_GENERIC_DOMAINS-CONFIG_ARM_AIROHA_SOC_CPUFREQ-0-0 (https://download.01.org/0day-ci/archive/20241204/202412042218.ldyfHhMs-lkp@intel.com/config)
reproduce: (https://download.01.org/0day-ci/archive/20241204/202412042218.ldyfHhMs-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412042218.ldyfHhMs-lkp@intel.com/

kismet warnings: (new ones prefixed by >>)
>> kismet: WARNING: unmet direct dependencies detected for PM_GENERIC_DOMAINS when selected by ARM_AIROHA_SOC_CPUFREQ
   WARNING: unmet direct dependencies detected for PM_GENERIC_DOMAINS
     Depends on [n]: PM [=n]
     Selected by [y]:
     - ARM_AIROHA_SOC_CPUFREQ [=y] && CPU_FREQ [=y] && (ARCH_AIROHA [=y] || COMPILE_TEST [=n] && ARM64)

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v5 2/2] cpufreq: airoha: Add EN7581 CPUFreq SMCCC driver

[Christian Marangi]

On Wed, Dec 04, 2024 at 07:22:41PM +0800, kernel test robot wrote:
> Hi Christian,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on rafael-pm/linux-next]
> [also build test ERROR on rafael-pm/bleeding-edge robh/for-next linus/master v6.13-rc1 next-20241203]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Christian-Marangi/cpufreq-airoha-Add-EN7581-CPUFreq-SMCCC-driver/20241204-113105
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
> patch link:    https://lore.kernel.org/r/20241203163158.580-2-ansuelsmth%40gmail.com
> patch subject: [PATCH v5 2/2] cpufreq: airoha: Add EN7581 CPUFreq SMCCC driver
> config: arm-randconfig-003 (https://download.01.org/0day-ci/archive/20241204/202412041929.8aCqrGnO-lkp@intel.com/config)
> compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 592c0fe55f6d9a811028b5f3507be91458ab2713)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241204/202412041929.8aCqrGnO-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202412041929.8aCqrGnO-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
> >> drivers/cpufreq/airoha-cpufreq.c:41:34: error: variable has incomplete type 'const struct arm_smccc_1_2_regs'
>       41 |         const struct arm_smccc_1_2_regs args = {
>          |                                         ^
>    drivers/cpufreq/airoha-cpufreq.c:41:15: note: forward declaration of 'struct arm_smccc_1_2_regs'
>       41 |         const struct arm_smccc_1_2_regs args = {
>          |                      ^
> >> drivers/cpufreq/airoha-cpufreq.c:45:28: error: variable has incomplete type 'struct arm_smccc_1_2_regs'
>       45 |         struct arm_smccc_1_2_regs res;
>          |                                   ^
>    drivers/cpufreq/airoha-cpufreq.c:41:15: note: forward declaration of 'struct arm_smccc_1_2_regs'
>       41 |         const struct arm_smccc_1_2_regs args = {
>          |                      ^
> >> drivers/cpufreq/airoha-cpufreq.c:47:2: error: call to undeclared function 'arm_smccc_1_2_smc'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
>       47 |         arm_smccc_1_2_smc(&args, &res);
>          |         ^
>    drivers/cpufreq/airoha-cpufreq.c:81:34: error: variable has incomplete type 'const struct arm_smccc_1_2_regs'
>       81 |         const struct arm_smccc_1_2_regs args = {
>          |                                         ^
>    drivers/cpufreq/airoha-cpufreq.c:81:15: note: forward declaration of 'struct arm_smccc_1_2_regs'
>       81 |         const struct arm_smccc_1_2_regs args = {
>          |                      ^
>    drivers/cpufreq/airoha-cpufreq.c:86:28: error: variable has incomplete type 'struct arm_smccc_1_2_regs'
>       86 |         struct arm_smccc_1_2_regs res;
>          |                                   ^
>    drivers/cpufreq/airoha-cpufreq.c:81:15: note: forward declaration of 'struct arm_smccc_1_2_regs'
>       81 |         const struct arm_smccc_1_2_regs args = {
>          |                      ^
>    drivers/cpufreq/airoha-cpufreq.c:88:2: error: call to undeclared function 'arm_smccc_1_2_smc'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
>       88 |         arm_smccc_1_2_smc(&args, &res);
>          |         ^
>    6 errors generated.
> 
> 
> vim +41 drivers/cpufreq/airoha-cpufreq.c
> 
>     37	
>     38	static unsigned long airoha_cpufreq_clk_get(struct clk_hw *hw,
>     39						    unsigned long parent_rate)
>     40	{
>   > 41		const struct arm_smccc_1_2_regs args = {
>     42			.a0 = AIROHA_SIP_AVS_HANDLE,
>     43			.a1 = AIROHA_AVS_OP_GET_FREQ,
>     44		};
>   > 45		struct arm_smccc_1_2_regs res;
>     46	
>   > 47		arm_smccc_1_2_smc(&args, &res);
>     48	
>     49		/* SMCCC returns freq in MHz */
>     50		return (int)(res.a0 * 1000 * 1000);
>     51	}
>     52	
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki

False positive or better say my error in the kconfig depends logic

This driver REQUIRE ARM64 bit for smccc and the target SoC is 64bit
only. The randconfig catch a situation with ARCH_AIROHA and 32bit.

-- 
	Ansuel

Re: [PATCH v5 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq

[Rob Herring]

On Tue, Dec 03, 2024 at 05:31:49PM +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>
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> Changes v5:
> - Add Reviewed-by tag
> - Fix OPP node name error
> - Rename cpufreq node name to power-domain
> - Rename CPU node power domain name to perf
> - Add model and compatible to example
> Changes v4:
> - Add this patch
> 
>  .../cpufreq/airoha,en7581-cpufreq.yaml        | 262 ++++++++++++++++++
>  1 file changed, 262 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..7e36fa037e4b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> @@ -0,0 +1,262 @@
> +# 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

You just said no clocks are exposed to the OS.

> +
> +  '#power-domain-cells':
> +    const: 0
> +
> +  operating-points-v2: true
> +
> +required:
> +  - compatible
> +  - '#clock-cells'
> +  - '#power-domain-cells'
> +  - operating-points-v2
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    / {
> +        model = "Airoha EN7581 Evaluation Board";
> +        compatible = "airoha,en7581-evb", "airoha,en7581";
> +
> +        #address-cells = <2>;
> +      	#size-cells = <2>;

mixed tab and spaces.

Can't I just go read the actual .dts files if I want to see 
*everything*? Examples should generally be just what the schema covers.

> +
> +        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 = <&cpu_pd>;
> +                clock-names = "cpu";
> +                power-domains = <&cpu_pd>;
> +                power-domain-names = "perf";
> +                next-level-cache = <&l2>;
> +                #cooling-cells = <2>;

I don't understand why you have clocks, power-domains and OPP? 
Certainly that's conceivable, but not with how you're abusing 
power-domains for performance points and you said clocks are not exposed 
to the OS.

Rob

Re: [PATCH v5 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq

[Christian Marangi]

On Wed, Dec 04, 2024 at 12:42:53PM -0600, Rob Herring wrote:
> On Tue, Dec 03, 2024 at 05:31:49PM +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>
> > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> > ---
> > Changes v5:
> > - Add Reviewed-by tag
> > - Fix OPP node name error
> > - Rename cpufreq node name to power-domain
> > - Rename CPU node power domain name to perf
> > - Add model and compatible to example
> > Changes v4:
> > - Add this patch
> > 
> >  .../cpufreq/airoha,en7581-cpufreq.yaml        | 262 ++++++++++++++++++
> >  1 file changed, 262 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..7e36fa037e4b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> > @@ -0,0 +1,262 @@
> > +# 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
> 
> You just said no clocks are exposed to the OS.
>

Well we now simulate one due to request from cpufreq reviewers.

Everything is still handled by SMC that only report the current
frequency of the CPU.

> > +
> > +  '#power-domain-cells':
> > +    const: 0
> > +
> > +  operating-points-v2: true
> > +
> > +required:
> > +  - compatible
> > +  - '#clock-cells'
> > +  - '#power-domain-cells'
> > +  - operating-points-v2
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    / {
> > +        model = "Airoha EN7581 Evaluation Board";
> > +        compatible = "airoha,en7581-evb", "airoha,en7581";
> > +
> > +        #address-cells = <2>;
> > +      	#size-cells = <2>;
> 
> mixed tab and spaces.
> 
> Can't I just go read the actual .dts files if I want to see 
> *everything*? Examples should generally be just what the schema covers.
>

Idea here is to give example as both clock and power-domain property are
needed in the CPU nodes for the CPUFreq driver to correctly work.

Should I drop and just define the CPUFreq node?

> > +
> > +        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 = <&cpu_pd>;
> > +                clock-names = "cpu";
> > +                power-domains = <&cpu_pd>;
> > +                power-domain-names = "perf";
> > +                next-level-cache = <&l2>;
> > +                #cooling-cells = <2>;
> 
> I don't understand why you have clocks, power-domains and OPP? 
> Certainly that's conceivable, but not with how you're abusing 
> power-domains for performance points and you said clocks are not exposed 
> to the OS.
>

SMC scale based on index values not frequency. That really resembles a
power-domain. SMC provide frequency in MHz tho so we model that as a
get-only clock.

At times with no clocks are exposed I intend that they SoC doesn't
provide any raw control on them in the normal way with a register, bits
to change and logic to apply for mux and divisor, this thing is very
special and works only with 2 command and nothing else so I'm trying my
best to model this in the most descriptive and complete way possible.

-- 
	Ansuel

Re: [PATCH v5 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq

[Rob Herring]

On Wed, Dec 4, 2024 at 12:51 PM Christian Marangi <ansuelsmth@gmail.com> wrote:
>
> On Wed, Dec 04, 2024 at 12:42:53PM -0600, Rob Herring wrote:
> > On Tue, Dec 03, 2024 at 05:31:49PM +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>
> > > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> > > ---
> > > Changes v5:
> > > - Add Reviewed-by tag
> > > - Fix OPP node name error
> > > - Rename cpufreq node name to power-domain
> > > - Rename CPU node power domain name to perf
> > > - Add model and compatible to example
> > > Changes v4:
> > > - Add this patch
> > >
> > >  .../cpufreq/airoha,en7581-cpufreq.yaml        | 262 ++++++++++++++++++
> > >  1 file changed, 262 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..7e36fa037e4b
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> > > @@ -0,0 +1,262 @@
> > > +# 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
> >
> > You just said no clocks are exposed to the OS.
> >
>
> Well we now simulate one due to request from cpufreq reviewers.
>
> Everything is still handled by SMC that only report the current
> frequency of the CPU.
>
> > > +
> > > +  '#power-domain-cells':
> > > +    const: 0
> > > +
> > > +  operating-points-v2: true
> > > +
> > > +required:
> > > +  - compatible
> > > +  - '#clock-cells'
> > > +  - '#power-domain-cells'
> > > +  - operating-points-v2
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    / {
> > > +        model = "Airoha EN7581 Evaluation Board";
> > > +        compatible = "airoha,en7581-evb", "airoha,en7581";
> > > +
> > > +        #address-cells = <2>;
> > > +           #size-cells = <2>;
> >
> > mixed tab and spaces.
> >
> > Can't I just go read the actual .dts files if I want to see
> > *everything*? Examples should generally be just what the schema covers.
> >
>
> Idea here is to give example as both clock and power-domain property are
> needed in the CPU nodes for the CPUFreq driver to correctly work.

If we want to do that, then we really should have a schema defining
that. But since there's only 1 for cpus that doesn't really work.

> Should I drop and just define the CPUFreq node?

Yes.

> > > +
> > > +        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 = <&cpu_pd>;
> > > +                clock-names = "cpu";
> > > +                power-domains = <&cpu_pd>;
> > > +                power-domain-names = "perf";
> > > +                next-level-cache = <&l2>;
> > > +                #cooling-cells = <2>;
> >
> > I don't understand why you have clocks, power-domains and OPP?
> > Certainly that's conceivable, but not with how you're abusing
> > power-domains for performance points and you said clocks are not exposed
> > to the OS.
> >
>
> SMC scale based on index values not frequency. That really resembles a
> power-domain.

So what is the point of the OPP table with frequency? You can set an
OPP and read the frequency, right? So a table of frequencies is
redundant.

> SMC provide frequency in MHz tho so we model that as a
> get-only clock.
>
> At times with no clocks are exposed I intend that they SoC doesn't
> provide any raw control on them in the normal way with a register, bits
> to change and logic to apply for mux and divisor, this thing is very
> special and works only with 2 command and nothing else so I'm trying my
> best to model this in the most descriptive and complete way possible.

Fair enough for the clock. Please clarify the description with what
clock is provided. Just to make sure, all CPUs run at the same
frequency?

Rob

Re: [PATCH v5 1/2] dt-bindings: cpufreq: Document support for Airoha EN7581 CPUFreq

[Christian Marangi]

On Wed, Dec 04, 2024 at 02:30:17PM -0600, Rob Herring wrote:
> On Wed, Dec 4, 2024 at 12:51 PM Christian Marangi <ansuelsmth@gmail.com> wrote:
> >
> > On Wed, Dec 04, 2024 at 12:42:53PM -0600, Rob Herring wrote:
> > > On Tue, Dec 03, 2024 at 05:31:49PM +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>
> > > > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> > > > ---
> > > > Changes v5:
> > > > - Add Reviewed-by tag
> > > > - Fix OPP node name error
> > > > - Rename cpufreq node name to power-domain
> > > > - Rename CPU node power domain name to perf
> > > > - Add model and compatible to example
> > > > Changes v4:
> > > > - Add this patch
> > > >
> > > >  .../cpufreq/airoha,en7581-cpufreq.yaml        | 262 ++++++++++++++++++
> > > >  1 file changed, 262 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..7e36fa037e4b
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/cpufreq/airoha,en7581-cpufreq.yaml
> > > > @@ -0,0 +1,262 @@
> > > > +# 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
> > >
> > > You just said no clocks are exposed to the OS.
> > >
> >
> > Well we now simulate one due to request from cpufreq reviewers.
> >
> > Everything is still handled by SMC that only report the current
> > frequency of the CPU.
> >
> > > > +
> > > > +  '#power-domain-cells':
> > > > +    const: 0
> > > > +
> > > > +  operating-points-v2: true
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - '#clock-cells'
> > > > +  - '#power-domain-cells'
> > > > +  - operating-points-v2
> > > > +
> > > > +additionalProperties: false
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    / {
> > > > +        model = "Airoha EN7581 Evaluation Board";
> > > > +        compatible = "airoha,en7581-evb", "airoha,en7581";
> > > > +
> > > > +        #address-cells = <2>;
> > > > +           #size-cells = <2>;
> > >
> > > mixed tab and spaces.
> > >
> > > Can't I just go read the actual .dts files if I want to see
> > > *everything*? Examples should generally be just what the schema covers.
> > >
> >
> > Idea here is to give example as both clock and power-domain property are
> > needed in the CPU nodes for the CPUFreq driver to correctly work.
> 
> If we want to do that, then we really should have a schema defining
> that. But since there's only 1 for cpus that doesn't really work.
> 
> > Should I drop and just define the CPUFreq node?
> 
> Yes.
> 
> > > > +
> > > > +        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 = <&cpu_pd>;
> > > > +                clock-names = "cpu";
> > > > +                power-domains = <&cpu_pd>;
> > > > +                power-domain-names = "perf";
> > > > +                next-level-cache = <&l2>;
> > > > +                #cooling-cells = <2>;
> > >
> > > I don't understand why you have clocks, power-domains and OPP?
> > > Certainly that's conceivable, but not with how you're abusing
> > > power-domains for performance points and you said clocks are not exposed
> > > to the OS.
> > >
> >
> > SMC scale based on index values not frequency. That really resembles a
> > power-domain.
> 
> So what is the point of the OPP table with frequency? You can set an
> OPP and read the frequency, right? So a table of frequencies is
> redundant.
>

The OPP for CPU node is to describe the supported frequency and then
each OPP have a required-opp property to describe the level to configure
the power-domain. It's really to make a connection between the 2. I need
to check but from my test the separate OPP table for the power domain is
needed or it does refuse to probe.

This is a common pattern also used by Qcom and Mediatek. Example qcs404 [0]

As you notice the very same pattern is used here.

> > SMC provide frequency in MHz tho so we model that as a
> > get-only clock.
> >
> > At times with no clocks are exposed I intend that they SoC doesn't
> > provide any raw control on them in the normal way with a register, bits
> > to change and logic to apply for mux and divisor, this thing is very
> > special and works only with 2 command and nothing else so I'm trying my
> > best to model this in the most descriptive and complete way possible.
> 
> Fair enough for the clock. Please clarify the description with what
> clock is provided. Just to make sure, all CPUs run at the same
> frequency?
>

Ok, yes it's all global also signaled by the opp-shared property.

[0] https://elixir.bootlin.com/linux/v6.12.1/source/arch/arm64/boot/dts/qcom/qcs404.dtsi

-- 
	Ansuel