[PATCH v5 0/4] Apple SoC cpufreq driver

[PATCH v5 0/4] Apple SoC cpufreq driver

[Hector Martin]

Hi folks,

Here's v5 of the cpufreq driver for Apple SoCs. v5 just incorporates
minor review feedback changes from v3, and no functional changes. v4
had a DT schema SNAFU; this supersedes it.

Once reviewed, please merge #3 via the cpufreq tree, and we'll take
care of #1,#2,#4 via the asahi-soc tree. This lets us merge the DT
changes in the same cycle without blocking on the binding coming in
via the cpufreq tree first.

This version takes a page from both v1 and v2, keeping the dedicated
cpufreq style (instead of pretending to be a clock controller) but using
dedicated DT nodes for each cluster, which accurately represents the
hardware. In particular, this makes supporting t6002 (M1 Ultra) a lot
more reasonable on the DT side.

This version also switches to the standard performance-domains binding,
so we don't need any more vendor-specific properties. In order to
support this, I had to make the performance-domains parsing code more
generic. This required a minor change to the only consumer
(mediatek-cpufreq-hw).

The Linux driver probes based on platform compatible, and then attempts
to locate the cluster nodes by following the performance-domains links
from CPU nodes (this will then fail for any incompatible nodes, e.g. if
a future SoC needs a new compatible and can't fall back). This approach
was suggested by robh as the right way to handle the impedance mismatch
between the hardware, which has separate controllers per cluster, and
the Linux model where there can only be one CPUFreq driver instance.

Functionality-wise, there are no significant changes from v2. The only
notable difference is support for t8112 (M2). This works largely the
same as the other SoCs, but they ran out of bits in the current PState
register, so that needs a SoC-specific quirk. Since that register is
not used by macOS (it was discovered experimentally) and is not critical
for functionality (it just allows accurately reporting the current
frequency to userspace, given boost clock limitations), I've decided to
only use it when a SoC-specific compatible is present. The default
fallback code will simply report the requested frequency as actual.
I expect this will work for future SoCs.

Hector Martin (4):
  MAINTAINERS: Add entries for Apple SoC cpufreq driver
  dt-bindings: cpufreq: apple,soc-cpufreq: Add binding for Apple SoC
    cpufreq
  cpufreq: apple-soc: Add new driver to control Apple SoC CPU P-states
  arm64: dts: apple: Add CPU topology & cpufreq nodes for t8103

 .../cpufreq/apple,cluster-cpufreq.yaml        | 117 ++++++
 MAINTAINERS                                   |   2 +
 arch/arm64/boot/dts/apple/t8103.dtsi          | 204 +++++++++-
 drivers/cpufreq/Kconfig.arm                   |   9 +
 drivers/cpufreq/Makefile                      |   1 +
 drivers/cpufreq/apple-soc-cpufreq.c           | 352 ++++++++++++++++++
 drivers/cpufreq/cpufreq-dt-platdev.c          |   2 +
 7 files changed, 677 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/apple,cluster-cpufreq.yaml
 create mode 100644 drivers/cpufreq/apple-soc-cpufreq.c

-- 
2.35.1

[PATCH v5 1/4] MAINTAINERS: Add entries for Apple SoC cpufreq driver

[Hector Martin]

This MAINTAINERS update is split, as usual, to facilitate merges via the
SoC tree and avoid conflicts.

Acked-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Hector Martin <marcan@marcan.st>
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 379945f82a64..52df511ad87c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1897,6 +1897,7 @@ T:	git https://github.com/AsahiLinux/linux.git
 F:	Documentation/devicetree/bindings/arm/apple.yaml
 F:	Documentation/devicetree/bindings/arm/apple/*
 F:	Documentation/devicetree/bindings/clock/apple,nco.yaml
+F:	Documentation/devicetree/bindings/cpufreq/apple,cluster-cpufreq.yaml
 F:	Documentation/devicetree/bindings/dma/apple,admac.yaml
 F:	Documentation/devicetree/bindings/i2c/apple,i2c.yaml
 F:	Documentation/devicetree/bindings/interrupt-controller/apple,*
@@ -1911,6 +1912,7 @@ F:	Documentation/devicetree/bindings/power/apple*
 F:	Documentation/devicetree/bindings/watchdog/apple,wdt.yaml
 F:	arch/arm64/boot/dts/apple/
 F:	drivers/clk/clk-apple-nco.c
+F:	drivers/cpufreq/apple-soc-cpufreq.c
 F:	drivers/dma/apple-admac.c
 F:	drivers/i2c/busses/i2c-pasemi-core.c
 F:	drivers/i2c/busses/i2c-pasemi-platform.c
-- 
2.35.1

[PATCH v5 2/4] dt-bindings: cpufreq: apple,soc-cpufreq: Add binding for Apple SoC cpufreq

[Hector Martin]

This binding represents the cpufreq/DVFS hardware present in Apple SoCs.
The hardware has an independent controller per CPU cluster, and we
represent them as unique nodes in order to accurately describe the
hardware. The driver is responsible for binding them as a single cpufreq
device (in the Linux cpufreq model).

Acked-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Hector Martin <marcan@marcan.st>
---
 .../cpufreq/apple,cluster-cpufreq.yaml        | 117 ++++++++++++++++++
 1 file changed, 117 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/apple,cluster-cpufreq.yaml

diff --git a/Documentation/devicetree/bindings/cpufreq/apple,cluster-cpufreq.yaml b/Documentation/devicetree/bindings/cpufreq/apple,cluster-cpufreq.yaml
new file mode 100644
index 000000000000..76cb9726660e
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/apple,cluster-cpufreq.yaml
@@ -0,0 +1,117 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/cpufreq/apple,cluster-cpufreq.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Apple SoC cluster cpufreq device
+
+maintainers:
+  - Hector Martin <marcan@marcan.st>
+
+description: |
+  Apple SoCs (e.g. M1) have a per-cpu-cluster DVFS controller that is part of
+  the cluster management register block. This binding uses the standard
+  operating-points-v2 table to define the CPU performance states, with the
+  opp-level property specifying the hardware p-state index for that level.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - apple,t8103-cluster-cpufreq
+              - apple,t8112-cluster-cpufreq
+          - const: apple,cluster-cpufreq
+      - items:
+          - const: apple,t6000-cluster-cpufreq
+          - const: apple,t8103-cluster-cpufreq
+          - const: apple,cluster-cpufreq
+
+  reg:
+    maxItems: 1
+
+  '#performance-domain-cells':
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - '#performance-domain-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    // This example shows a single CPU per domain and 2 domains,
+    // with two p-states per domain.
+    // Shipping hardware has 2-4 CPUs per domain and 2-6 domains.
+    cpus {
+      #address-cells = <2>;
+      #size-cells = <0>;
+
+      cpu@0 {
+        compatible = "apple,icestorm";
+        device_type = "cpu";
+        reg = <0x0 0x0>;
+        operating-points-v2 = <&ecluster_opp>;
+        performance-domains = <&cpufreq_e>;
+      };
+
+      cpu@10100 {
+        compatible = "apple,firestorm";
+        device_type = "cpu";
+        reg = <0x0 0x10100>;
+        operating-points-v2 = <&pcluster_opp>;
+        performance-domains = <&cpufreq_p>;
+      };
+    };
+
+    ecluster_opp: opp-table-0 {
+      compatible = "operating-points-v2";
+      opp-shared;
+
+      opp01 {
+        opp-hz = /bits/ 64 <600000000>;
+        opp-level = <1>;
+        clock-latency-ns = <7500>;
+      };
+      opp02 {
+        opp-hz = /bits/ 64 <972000000>;
+        opp-level = <2>;
+        clock-latency-ns = <22000>;
+      };
+    };
+
+    pcluster_opp: opp-table-1 {
+      compatible = "operating-points-v2";
+      opp-shared;
+
+      opp01 {
+        opp-hz = /bits/ 64 <600000000>;
+        opp-level = <1>;
+        clock-latency-ns = <8000>;
+      };
+      opp02 {
+        opp-hz = /bits/ 64 <828000000>;
+        opp-level = <2>;
+        clock-latency-ns = <19000>;
+      };
+    };
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      cpufreq_e: performance-controller@210e20000 {
+        compatible = "apple,t8103-cluster-cpufreq", "apple,cluster-cpufreq";
+        reg = <0x2 0x10e20000 0 0x1000>;
+        #performance-domain-cells = <0>;
+      };
+
+      cpufreq_p: performance-controller@211e20000 {
+        compatible = "apple,t8103-cluster-cpufreq", "apple,cluster-cpufreq";
+        reg = <0x2 0x11e20000 0 0x1000>;
+        #performance-domain-cells = <0>;
+      };
+    };
-- 
2.35.1

[PATCH v5 3/4] cpufreq: apple-soc: Add new driver to control Apple SoC CPU P-states

[Hector Martin]

This driver implements CPU frequency scaling for Apple Silicon SoCs,
including M1 (t8103), M1 Max/Pro/Ultra (t600x), and M2 (t8112).

Each CPU cluster has its own register set, and frequency management is
fully automated by the hardware; the driver only has to write one
register. There is boost frequency support, but the hardware will only
allow their use if only a subset of cores in a cluster are in
non-deep-idle. Since we don't support deep idle yet, these frequencies
are not achievable, but the driver supports them. They will remain
disabled in the device tree until deep idle is implemented, to avoid
confusing users.

This driver does not yet implement the memory controller performance
state tuning that usually accompanies higher CPU p-states. This will be
done in a future patch.

Acked-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Hector Martin <marcan@marcan.st>
---
 drivers/cpufreq/Kconfig.arm          |   9 +
 drivers/cpufreq/Makefile             |   1 +
 drivers/cpufreq/apple-soc-cpufreq.c  | 352 +++++++++++++++++++++++++++
 drivers/cpufreq/cpufreq-dt-platdev.c |   2 +
 4 files changed, 364 insertions(+)
 create mode 100644 drivers/cpufreq/apple-soc-cpufreq.c

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index be590f498e6a..0a0352d8fa45 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -41,6 +41,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_APPLE_SOC_CPUFREQ
+	tristate "Apple Silicon SoC CPUFreq support"
+	depends on ARCH_APPLE || (COMPILE_TEST && 64BIT)
+	select PM_OPP
+	default ARCH_APPLE
+	help
+	  This adds the CPUFreq driver for Apple Silicon machines
+	  (e.g. Apple M1).
+
 config ARM_ARMADA_37XX_CPUFREQ
 	tristate "Armada 37xx CPUFreq support"
 	depends on ARCH_MVEBU && CPUFREQ_DT
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 49b98c62c5af..32a7029e25ed 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_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
 obj-$(CONFIG_ARM_BRCMSTB_AVS_CPUFREQ)	+= brcmstb-avs-cpufreq.o
diff --git a/drivers/cpufreq/apple-soc-cpufreq.c b/drivers/cpufreq/apple-soc-cpufreq.c
new file mode 100644
index 000000000000..d1801281cdd9
--- /dev/null
+++ b/drivers/cpufreq/apple-soc-cpufreq.c
@@ -0,0 +1,352 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Apple SoC CPU cluster performance state driver
+ *
+ * Copyright The Asahi Linux Contributors
+ *
+ * Based on scpi-cpufreq.c
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/cpu.h>
+#include <linux/cpufreq.h>
+#include <linux/cpumask.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/pm_opp.h>
+#include <linux/slab.h>
+
+#define APPLE_DVFS_CMD			0x20
+#define APPLE_DVFS_CMD_BUSY		BIT(31)
+#define APPLE_DVFS_CMD_SET		BIT(25)
+#define APPLE_DVFS_CMD_PS2		GENMASK(16, 12)
+#define APPLE_DVFS_CMD_PS1		GENMASK(4, 0)
+
+/* Same timebase as CPU counter (24MHz) */
+#define APPLE_DVFS_LAST_CHG_TIME	0x38
+
+/*
+ * Apple ran out of bits and had to shift this in T8112...
+ */
+#define APPLE_DVFS_STATUS			0x50
+#define APPLE_DVFS_STATUS_CUR_PS_T8103		GENMASK(7, 4)
+#define APPLE_DVFS_STATUS_CUR_PS_SHIFT_T8103	4
+#define APPLE_DVFS_STATUS_TGT_PS_T8103		GENMASK(3, 0)
+#define APPLE_DVFS_STATUS_CUR_PS_T8112		GENMASK(9, 5)
+#define APPLE_DVFS_STATUS_CUR_PS_SHIFT_T8112	5
+#define APPLE_DVFS_STATUS_TGT_PS_T8112		GENMASK(4, 0)
+
+/*
+ * Div is +1, base clock is 12MHz on existing SoCs.
+ * For documentation purposes. We use the OPP table to
+ * get the frequency.
+ */
+#define APPLE_DVFS_PLL_STATUS		0xc0
+#define APPLE_DVFS_PLL_FACTOR		0xc8
+#define APPLE_DVFS_PLL_FACTOR_MULT	GENMASK(31, 16)
+#define APPLE_DVFS_PLL_FACTOR_DIV	GENMASK(15, 0)
+
+#define APPLE_DVFS_TRANSITION_TIMEOUT 100
+
+struct apple_soc_cpufreq_info {
+	u64 max_pstate;
+	u64 cur_pstate_mask;
+	u64 cur_pstate_shift;
+};
+
+struct apple_cpu_priv {
+	struct device *cpu_dev;
+	void __iomem *reg_base;
+	const struct apple_soc_cpufreq_info *info;
+};
+
+static struct cpufreq_driver apple_soc_cpufreq_driver;
+
+static const struct apple_soc_cpufreq_info soc_t8103_info = {
+	.max_pstate = 15,
+	.cur_pstate_mask = APPLE_DVFS_STATUS_CUR_PS_T8103,
+	.cur_pstate_shift = APPLE_DVFS_STATUS_CUR_PS_SHIFT_T8103,
+};
+
+static const struct apple_soc_cpufreq_info soc_t8112_info = {
+	.max_pstate = 31,
+	.cur_pstate_mask = APPLE_DVFS_STATUS_CUR_PS_T8112,
+	.cur_pstate_shift = APPLE_DVFS_STATUS_CUR_PS_SHIFT_T8112,
+};
+
+static const struct apple_soc_cpufreq_info soc_default_info = {
+	.max_pstate = 15,
+	.cur_pstate_mask = 0, /* fallback */
+};
+
+static const struct of_device_id apple_soc_cpufreq_of_match[] = {
+	{
+		.compatible = "apple,t8103-cluster-cpufreq",
+		.data = &soc_t8103_info,
+	},
+	{
+		.compatible = "apple,t8112-cluster-cpufreq",
+		.data = &soc_t8112_info,
+	},
+	{
+		.compatible = "apple,cluster-cpufreq",
+		.data = &soc_default_info,
+	},
+	{}
+};
+
+static unsigned int apple_soc_cpufreq_get_rate(unsigned int cpu)
+{
+	struct cpufreq_policy *policy = cpufreq_cpu_get_raw(cpu);
+	struct apple_cpu_priv *priv = policy->driver_data;
+	struct cpufreq_frequency_table *p;
+	unsigned int pstate;
+
+	if (priv->info->cur_pstate_mask) {
+		u64 reg = readq_relaxed(priv->reg_base + APPLE_DVFS_STATUS);
+
+		pstate = (reg & priv->info->cur_pstate_mask) >>  priv->info->cur_pstate_shift;
+	} else {
+		/*
+		 * For the fallback case we might not know the layout of DVFS_STATUS,
+		 * so just use the command register value (which ignores boost limitations).
+		 */
+		u64 reg = readq_relaxed(priv->reg_base + APPLE_DVFS_CMD);
+
+		pstate = FIELD_GET(APPLE_DVFS_CMD_PS1, reg);
+	}
+
+	cpufreq_for_each_valid_entry(p, policy->freq_table)
+		if (p->driver_data == pstate)
+			return p->frequency;
+
+	dev_err(priv->cpu_dev, "could not find frequency for pstate %d\n",
+		pstate);
+	return 0;
+}
+
+static int apple_soc_cpufreq_set_target(struct cpufreq_policy *policy,
+					unsigned int index)
+{
+	struct apple_cpu_priv *priv = policy->driver_data;
+	unsigned int pstate = policy->freq_table[index].driver_data;
+	u64 reg;
+
+	/* Fallback for newer SoCs */
+	if (index > priv->info->max_pstate)
+		index = priv->info->max_pstate;
+
+	if (readq_poll_timeout_atomic(priv->reg_base + APPLE_DVFS_CMD, reg,
+				      !(reg & APPLE_DVFS_CMD_BUSY), 2,
+				      APPLE_DVFS_TRANSITION_TIMEOUT)) {
+		return -EIO;
+	}
+
+	reg &= ~(APPLE_DVFS_CMD_PS1 | APPLE_DVFS_CMD_PS2);
+	reg |= FIELD_PREP(APPLE_DVFS_CMD_PS1, pstate);
+	reg |= FIELD_PREP(APPLE_DVFS_CMD_PS2, pstate);
+	reg |= APPLE_DVFS_CMD_SET;
+
+	writeq_relaxed(reg, priv->reg_base + APPLE_DVFS_CMD);
+
+	return 0;
+}
+
+static unsigned int apple_soc_cpufreq_fast_switch(struct cpufreq_policy *policy,
+						  unsigned int target_freq)
+{
+	if (apple_soc_cpufreq_set_target(policy, policy->cached_resolved_idx) < 0)
+		return 0;
+
+	return policy->freq_table[policy->cached_resolved_idx].frequency;
+}
+
+static int apple_soc_cpufreq_find_cluster(struct cpufreq_policy *policy,
+					  void __iomem **reg_base,
+					  const struct apple_soc_cpufreq_info **info)
+{
+	struct of_phandle_args args;
+	const struct of_device_id *match;
+	int ret = 0;
+
+	ret = of_perf_domain_get_sharing_cpumask(policy->cpu, "performance-domains",
+						 "#performance-domain-cells",
+						 policy->cpus, &args);
+	if (ret < 0)
+		return ret;
+
+	match = of_match_node(apple_soc_cpufreq_of_match, args.np);
+	of_node_put(args.np);
+	if (!match)
+		return -ENODEV;
+
+	*info = match->data;
+
+	*reg_base = of_iomap(args.np, 0);
+	if (IS_ERR(*reg_base))
+		return PTR_ERR(*reg_base);
+
+	return 0;
+}
+
+static struct freq_attr *apple_soc_cpufreq_hw_attr[] = {
+	&cpufreq_freq_attr_scaling_available_freqs,
+	NULL, /* Filled in below if boost is enabled */
+	NULL,
+};
+
+static int apple_soc_cpufreq_init(struct cpufreq_policy *policy)
+{
+	int ret, i;
+	unsigned int transition_latency;
+	void __iomem *reg_base;
+	struct device *cpu_dev;
+	struct apple_cpu_priv *priv;
+	const struct apple_soc_cpufreq_info *info;
+	struct cpufreq_frequency_table *freq_table;
+
+	cpu_dev = get_cpu_device(policy->cpu);
+	if (!cpu_dev) {
+		pr_err("failed to get cpu%d device\n", policy->cpu);
+		return -ENODEV;
+	}
+
+	ret = dev_pm_opp_of_add_table(cpu_dev);
+	if (ret < 0) {
+		dev_err(cpu_dev, "%s: failed to add OPP table: %d\n", __func__, ret);
+		return ret;
+	}
+
+	ret = apple_soc_cpufreq_find_cluster(policy, &reg_base, &info);
+	if (ret) {
+		dev_err(cpu_dev, "%s: failed to get cluster info: %d\n", __func__, ret);
+		return ret;
+	}
+
+	ret = dev_pm_opp_set_sharing_cpus(cpu_dev, policy->cpus);
+	if (ret) {
+		dev_err(cpu_dev, "%s: failed to mark OPPs as shared: %d\n", __func__, ret);
+		goto out_iounmap;
+	}
+
+	ret = dev_pm_opp_get_opp_count(cpu_dev);
+	if (ret <= 0) {
+		dev_dbg(cpu_dev, "OPP table is not ready, deferring probe\n");
+		ret = -EPROBE_DEFER;
+		goto out_free_opp;
+	}
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		ret = -ENOMEM;
+		goto out_free_opp;
+	}
+
+	ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table);
+	if (ret) {
+		dev_err(cpu_dev, "failed to init cpufreq table: %d\n", ret);
+		goto out_free_priv;
+	}
+
+	/* Get OPP levels (p-state indexes) and stash them in driver_data */
+	for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++) {
+		unsigned long rate = freq_table[i].frequency * 1000 + 999;
+		struct dev_pm_opp *opp = dev_pm_opp_find_freq_floor(cpu_dev, &rate);
+
+		if (IS_ERR(opp)) {
+			ret = PTR_ERR(opp);
+			goto out_free_cpufreq_table;
+		}
+		freq_table[i].driver_data = dev_pm_opp_get_level(opp);
+		dev_pm_opp_put(opp);
+	}
+
+	priv->cpu_dev = cpu_dev;
+	priv->reg_base = reg_base;
+	priv->info = info;
+	policy->driver_data = priv;
+	policy->freq_table = freq_table;
+
+	transition_latency = dev_pm_opp_get_max_transition_latency(cpu_dev);
+	if (!transition_latency)
+		transition_latency = CPUFREQ_ETERNAL;
+
+	policy->cpuinfo.transition_latency = transition_latency;
+	policy->dvfs_possible_from_any_cpu = true;
+	policy->fast_switch_possible = true;
+
+	if (policy_has_boost_freq(policy)) {
+		ret = cpufreq_enable_boost_support();
+		if (ret) {
+			dev_warn(cpu_dev, "failed to enable boost: %d\n", ret);
+		} else {
+			apple_soc_cpufreq_hw_attr[1] = &cpufreq_freq_attr_scaling_boost_freqs;
+			apple_soc_cpufreq_driver.boost_enabled = true;
+		}
+	}
+
+	return 0;
+
+out_free_cpufreq_table:
+	dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table);
+out_free_priv:
+	kfree(priv);
+out_free_opp:
+	dev_pm_opp_remove_all_dynamic(cpu_dev);
+out_iounmap:
+	iounmap(reg_base);
+	return ret;
+}
+
+static int apple_soc_cpufreq_exit(struct cpufreq_policy *policy)
+{
+	struct apple_cpu_priv *priv = policy->driver_data;
+
+	dev_pm_opp_free_cpufreq_table(priv->cpu_dev, &policy->freq_table);
+	dev_pm_opp_remove_all_dynamic(priv->cpu_dev);
+	iounmap(priv->reg_base);
+	kfree(priv);
+
+	return 0;
+}
+
+static struct cpufreq_driver apple_soc_cpufreq_driver = {
+	.name		= "apple-cpufreq",
+	.flags		= CPUFREQ_HAVE_GOVERNOR_PER_POLICY |
+			  CPUFREQ_NEED_INITIAL_FREQ_CHECK | CPUFREQ_IS_COOLING_DEV,
+	.verify		= cpufreq_generic_frequency_table_verify,
+	.attr		= cpufreq_generic_attr,
+	.get		= apple_soc_cpufreq_get_rate,
+	.init		= apple_soc_cpufreq_init,
+	.exit		= apple_soc_cpufreq_exit,
+	.target_index	= apple_soc_cpufreq_set_target,
+	.fast_switch	= apple_soc_cpufreq_fast_switch,
+	.register_em	= cpufreq_register_em_with_opp,
+	.attr		= apple_soc_cpufreq_hw_attr,
+};
+
+static int __init apple_soc_cpufreq_module_init(void)
+{
+	if (!of_machine_is_compatible("apple,arm-platform"))
+		return -ENODEV;
+
+	return cpufreq_register_driver(&apple_soc_cpufreq_driver);
+}
+module_init(apple_soc_cpufreq_module_init);
+
+static void __exit apple_soc_cpufreq_module_exit(void)
+{
+	cpufreq_unregister_driver(&apple_soc_cpufreq_driver);
+}
+module_exit(apple_soc_cpufreq_module_exit);
+
+MODULE_DEVICE_TABLE(of, apple_soc_cpufreq_of_match);
+MODULE_AUTHOR("Hector Martin <marcan@marcan.st>");
+MODULE_DESCRIPTION("Apple SoC CPU cluster DVFS driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
index 85ee11f79840..8ab672883043 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 = {
 static const struct of_device_id blocklist[] __initconst = {
 	{ .compatible = "allwinner,sun50i-h6", },
 
+	{ .compatible = "apple,arm-platform", },
+
 	{ .compatible = "arm,vexpress", },
 
 	{ .compatible = "calxeda,highbank", },
-- 
2.35.1

[PATCH v5 4/4] arm64: dts: apple: Add CPU topology & cpufreq nodes for t8103

[Hector Martin]

Add the missing CPU topology/capacity information and the cpufreq nodes,
so we can have CPU frequency scaling and the scheduler has the
information it needs to make the correct decisions.

Boost states are commented out, as they are not yet available (that
requires CPU deep sleep support, to be eventually done via PSCI).
The driver supports them fine; the hardware will just refuse to ever
go into them at this time, so don't expose them to users until that's
done.

Acked-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Hector Martin <marcan@marcan.st>
---
 arch/arm64/boot/dts/apple/t8103.dtsi | 204 +++++++++++++++++++++++++--
 1 file changed, 194 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/boot/dts/apple/t8103.dtsi b/arch/arm64/boot/dts/apple/t8103.dtsi
index 51a63b29d404..d56708038d05 100644
--- a/arch/arm64/boot/dts/apple/t8103.dtsi
+++ b/arch/arm64/boot/dts/apple/t8103.dtsi
@@ -22,71 +22,243 @@ cpus {
 		#address-cells = <2>;
 		#size-cells = <0>;
 
-		cpu0: cpu@0 {
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&cpu_e0>;
+				};
+				core1 {
+					cpu = <&cpu_e1>;
+				};
+				core2 {
+					cpu = <&cpu_e2>;
+				};
+				core3 {
+					cpu = <&cpu_e3>;
+				};
+			};
+
+			cluster1 {
+				core0 {
+					cpu = <&cpu_p0>;
+				};
+				core1 {
+					cpu = <&cpu_p1>;
+				};
+				core2 {
+					cpu = <&cpu_p2>;
+				};
+				core3 {
+					cpu = <&cpu_p3>;
+				};
+			};
+		};
+
+		cpu_e0: cpu@0 {
 			compatible = "apple,icestorm";
 			device_type = "cpu";
 			reg = <0x0 0x0>;
 			enable-method = "spin-table";
 			cpu-release-addr = <0 0>; /* To be filled by loader */
+			operating-points-v2 = <&ecluster_opp>;
+			capacity-dmips-mhz = <714>;
+			performance-domains = <&cpufreq_e>;
 		};
 
-		cpu1: cpu@1 {
+		cpu_e1: cpu@1 {
 			compatible = "apple,icestorm";
 			device_type = "cpu";
 			reg = <0x0 0x1>;
 			enable-method = "spin-table";
 			cpu-release-addr = <0 0>; /* To be filled by loader */
+			operating-points-v2 = <&ecluster_opp>;
+			capacity-dmips-mhz = <714>;
+			performance-domains = <&cpufreq_e>;
 		};
 
-		cpu2: cpu@2 {
+		cpu_e2: cpu@2 {
 			compatible = "apple,icestorm";
 			device_type = "cpu";
 			reg = <0x0 0x2>;
 			enable-method = "spin-table";
 			cpu-release-addr = <0 0>; /* To be filled by loader */
+			operating-points-v2 = <&ecluster_opp>;
+			capacity-dmips-mhz = <714>;
+			performance-domains = <&cpufreq_e>;
 		};
 
-		cpu3: cpu@3 {
+		cpu_e3: cpu@3 {
 			compatible = "apple,icestorm";
 			device_type = "cpu";
 			reg = <0x0 0x3>;
 			enable-method = "spin-table";
 			cpu-release-addr = <0 0>; /* To be filled by loader */
+			operating-points-v2 = <&ecluster_opp>;
+			capacity-dmips-mhz = <714>;
+			performance-domains = <&cpufreq_e>;
 		};
 
-		cpu4: cpu@10100 {
+		cpu_p0: cpu@10100 {
 			compatible = "apple,firestorm";
 			device_type = "cpu";
 			reg = <0x0 0x10100>;
 			enable-method = "spin-table";
 			cpu-release-addr = <0 0>; /* To be filled by loader */
+			operating-points-v2 = <&pcluster_opp>;
+			capacity-dmips-mhz = <1024>;
+			performance-domains = <&cpufreq_p>;
 		};
 
-		cpu5: cpu@10101 {
+		cpu_p1: cpu@10101 {
 			compatible = "apple,firestorm";
 			device_type = "cpu";
 			reg = <0x0 0x10101>;
 			enable-method = "spin-table";
 			cpu-release-addr = <0 0>; /* To be filled by loader */
+			operating-points-v2 = <&pcluster_opp>;
+			capacity-dmips-mhz = <1024>;
+			performance-domains = <&cpufreq_p>;
 		};
 
-		cpu6: cpu@10102 {
+		cpu_p2: cpu@10102 {
 			compatible = "apple,firestorm";
 			device_type = "cpu";
 			reg = <0x0 0x10102>;
 			enable-method = "spin-table";
 			cpu-release-addr = <0 0>; /* To be filled by loader */
+			operating-points-v2 = <&pcluster_opp>;
+			capacity-dmips-mhz = <1024>;
+			performance-domains = <&cpufreq_p>;
 		};
 
-		cpu7: cpu@10103 {
+		cpu_p3: cpu@10103 {
 			compatible = "apple,firestorm";
 			device_type = "cpu";
 			reg = <0x0 0x10103>;
 			enable-method = "spin-table";
 			cpu-release-addr = <0 0>; /* To be filled by loader */
+			operating-points-v2 = <&pcluster_opp>;
+			capacity-dmips-mhz = <1024>;
+			performance-domains = <&cpufreq_p>;
 		};
 	};
 
+	ecluster_opp: opp-table-0 {
+		compatible = "operating-points-v2";
+
+		opp01 {
+			opp-hz = /bits/ 64 <600000000>;
+			opp-level = <1>;
+			clock-latency-ns = <7500>;
+		};
+		opp02 {
+			opp-hz = /bits/ 64 <972000000>;
+			opp-level = <2>;
+			clock-latency-ns = <22000>;
+		};
+		opp03 {
+			opp-hz = /bits/ 64 <1332000000>;
+			opp-level = <3>;
+			clock-latency-ns = <27000>;
+		};
+		opp04 {
+			opp-hz = /bits/ 64 <1704000000>;
+			opp-level = <4>;
+			clock-latency-ns = <33000>;
+		};
+		opp05 {
+			opp-hz = /bits/ 64 <2064000000>;
+			opp-level = <5>;
+			clock-latency-ns = <50000>;
+		};
+	};
+
+	pcluster_opp: opp-table-1 {
+		compatible = "operating-points-v2";
+
+		opp01 {
+			opp-hz = /bits/ 64 <600000000>;
+			opp-level = <1>;
+			clock-latency-ns = <8000>;
+		};
+		opp02 {
+			opp-hz = /bits/ 64 <828000000>;
+			opp-level = <2>;
+			clock-latency-ns = <19000>;
+		};
+		opp03 {
+			opp-hz = /bits/ 64 <1056000000>;
+			opp-level = <3>;
+			clock-latency-ns = <21000>;
+		};
+		opp04 {
+			opp-hz = /bits/ 64 <1284000000>;
+			opp-level = <4>;
+			clock-latency-ns = <23000>;
+		};
+		opp05 {
+			opp-hz = /bits/ 64 <1500000000>;
+			opp-level = <5>;
+			clock-latency-ns = <24000>;
+		};
+		opp06 {
+			opp-hz = /bits/ 64 <1728000000>;
+			opp-level = <6>;
+			clock-latency-ns = <29000>;
+		};
+		opp07 {
+			opp-hz = /bits/ 64 <1956000000>;
+			opp-level = <7>;
+			clock-latency-ns = <31000>;
+		};
+		opp08 {
+			opp-hz = /bits/ 64 <2184000000>;
+			opp-level = <8>;
+			clock-latency-ns = <34000>;
+		};
+		opp09 {
+			opp-hz = /bits/ 64 <2388000000>;
+			opp-level = <9>;
+			clock-latency-ns = <36000>;
+		};
+		opp10 {
+			opp-hz = /bits/ 64 <2592000000>;
+			opp-level = <10>;
+			clock-latency-ns = <51000>;
+		};
+		opp11 {
+			opp-hz = /bits/ 64 <2772000000>;
+			opp-level = <11>;
+			clock-latency-ns = <54000>;
+		};
+		opp12 {
+			opp-hz = /bits/ 64 <2988000000>;
+			opp-level = <12>;
+			clock-latency-ns = <55000>;
+		};
+#if 0
+		/* Not available until CPU deep sleep is implemented */
+		opp13 {
+			opp-hz = /bits/ 64 <3096000000>;
+			opp-level = <13>;
+			clock-latency-ns = <55000>;
+			turbo-mode;
+		};
+		opp14 {
+			opp-hz = /bits/ 64 <3144000000>;
+			opp-level = <14>;
+			clock-latency-ns = <56000>;
+			turbo-mode;
+		};
+		opp15 {
+			opp-hz = /bits/ 64 <3204000000>;
+			opp-level = <15>;
+			clock-latency-ns = <56000>;
+			turbo-mode;
+		};
+#endif
+	};
+
 	timer {
 		compatible = "arm,armv8-timer";
 		interrupt-parent = <&aic>;
@@ -124,6 +296,18 @@ soc {
 		ranges;
 		nonposted-mmio;
 
+		cpufreq_e: performance-controller@210e20000 {
+			compatible = "apple,t8103-cluster-cpufreq", "apple,cluster-cpufreq";
+			reg = <0x2 0x10e20000 0 0x1000>;
+			#performance-domain-cells = <0>;
+		};
+
+		cpufreq_p: performance-controller@211e20000 {
+			compatible = "apple,t8103-cluster-cpufreq", "apple,cluster-cpufreq";
+			reg = <0x2 0x11e20000 0 0x1000>;
+			#performance-domain-cells = <0>;
+		};
+
 		i2c0: i2c@235010000 {
 			compatible = "apple,t8103-i2c", "apple,i2c";
 			reg = <0x2 0x35010000 0x0 0x4000>;
@@ -229,12 +413,12 @@ aic: interrupt-controller@23b100000 {
 			affinities {
 				e-core-pmu-affinity {
 					apple,fiq-index = <AIC_CPU_PMU_E>;
-					cpus = <&cpu0 &cpu1 &cpu2 &cpu3>;
+					cpus = <&cpu_e0 &cpu_e1 &cpu_e2 &cpu_e3>;
 				};
 
 				p-core-pmu-affinity {
 					apple,fiq-index = <AIC_CPU_PMU_P>;
-					cpus = <&cpu4 &cpu5 &cpu6 &cpu7>;
+					cpus = <&cpu_p0 &cpu_p1 &cpu_p2 &cpu_p3>;
 				};
 			};
 		};
-- 
2.35.1

Re: [PATCH v5 2/4] dt-bindings: cpufreq: apple,soc-cpufreq: Add binding for Apple SoC cpufreq

[Krzysztof Kozlowski]

On 28/11/2022 15:29, Hector Martin wrote:
> This binding represents the cpufreq/DVFS hardware present in Apple SoCs.
> The hardware has an independent controller per CPU cluster, and we
> represent them as unique nodes in order to accurately describe the
> hardware. The driver is responsible for binding them as a single cpufreq
> device (in the Linux cpufreq model).
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v5 2/4] dt-bindings: cpufreq: apple,soc-cpufreq: Add binding for Apple SoC cpufreq

[Ulf Hansson]

On Mon, 28 Nov 2022 at 15:29, Hector Martin <marcan@marcan.st> wrote:
>
> This binding represents the cpufreq/DVFS hardware present in Apple SoCs.
> The hardware has an independent controller per CPU cluster, and we
> represent them as unique nodes in order to accurately describe the
> hardware. The driver is responsible for binding them as a single cpufreq
> device (in the Linux cpufreq model).
>
> Acked-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>  .../cpufreq/apple,cluster-cpufreq.yaml        | 117 ++++++++++++++++++
>  1 file changed, 117 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/cpufreq/apple,cluster-cpufreq.yaml
>
> diff --git a/Documentation/devicetree/bindings/cpufreq/apple,cluster-cpufreq.yaml b/Documentation/devicetree/bindings/cpufreq/apple,cluster-cpufreq.yaml
> new file mode 100644
> index 000000000000..76cb9726660e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/apple,cluster-cpufreq.yaml
> @@ -0,0 +1,117 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/cpufreq/apple,cluster-cpufreq.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Apple SoC cluster cpufreq device
> +
> +maintainers:
> +  - Hector Martin <marcan@marcan.st>
> +
> +description: |
> +  Apple SoCs (e.g. M1) have a per-cpu-cluster DVFS controller that is part of
> +  the cluster management register block. This binding uses the standard
> +  operating-points-v2 table to define the CPU performance states, with the
> +  opp-level property specifying the hardware p-state index for that level.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - apple,t8103-cluster-cpufreq
> +              - apple,t8112-cluster-cpufreq
> +          - const: apple,cluster-cpufreq
> +      - items:
> +          - const: apple,t6000-cluster-cpufreq
> +          - const: apple,t8103-cluster-cpufreq
> +          - const: apple,cluster-cpufreq
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#performance-domain-cells':
> +    const: 0
> +
> +required:
> +  - compatible
> +  - reg
> +  - '#performance-domain-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    // This example shows a single CPU per domain and 2 domains,
> +    // with two p-states per domain.
> +    // Shipping hardware has 2-4 CPUs per domain and 2-6 domains.
> +    cpus {
> +      #address-cells = <2>;
> +      #size-cells = <0>;
> +
> +      cpu@0 {
> +        compatible = "apple,icestorm";
> +        device_type = "cpu";
> +        reg = <0x0 0x0>;
> +        operating-points-v2 = <&ecluster_opp>;

To me, it looks like the operating-points-v2 phandle better belongs in
the performance-domains provider node. I mean, isn't the OPPs really a
description of the performance-domain provider?

That said, I suggest we try to extend the generic performance-domain
binding [1] with an "operating-points-v2". In that way, we should
instead be able to reference it from this binding.

In fact, that would be very similar to what already exists for the
generic power-domain binding [2]. I think it would be rather nice to
follow a similar pattern for the performance-domain binding.

> +        performance-domains = <&cpufreq_e>;
> +      };
> +
> +      cpu@10100 {
> +        compatible = "apple,firestorm";
> +        device_type = "cpu";
> +        reg = <0x0 0x10100>;
> +        operating-points-v2 = <&pcluster_opp>;
> +        performance-domains = <&cpufreq_p>;
> +      };
> +    };
> +
> +    ecluster_opp: opp-table-0 {
> +      compatible = "operating-points-v2";
> +      opp-shared;
> +
> +      opp01 {
> +        opp-hz = /bits/ 64 <600000000>;
> +        opp-level = <1>;
> +        clock-latency-ns = <7500>;
> +      };
> +      opp02 {
> +        opp-hz = /bits/ 64 <972000000>;
> +        opp-level = <2>;
> +        clock-latency-ns = <22000>;
> +      };
> +    };
> +
> +    pcluster_opp: opp-table-1 {
> +      compatible = "operating-points-v2";
> +      opp-shared;
> +
> +      opp01 {
> +        opp-hz = /bits/ 64 <600000000>;
> +        opp-level = <1>;
> +        clock-latency-ns = <8000>;
> +      };
> +      opp02 {
> +        opp-hz = /bits/ 64 <828000000>;
> +        opp-level = <2>;
> +        clock-latency-ns = <19000>;
> +      };
> +    };
> +
> +    soc {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      cpufreq_e: performance-controller@210e20000 {
> +        compatible = "apple,t8103-cluster-cpufreq", "apple,cluster-cpufreq";
> +        reg = <0x2 0x10e20000 0 0x1000>;
> +        #performance-domain-cells = <0>;
> +      };
> +
> +      cpufreq_p: performance-controller@211e20000 {
> +        compatible = "apple,t8103-cluster-cpufreq", "apple,cluster-cpufreq";
> +        reg = <0x2 0x11e20000 0 0x1000>;
> +        #performance-domain-cells = <0>;
> +      };
> +    };

Kind regards
Uffe

[1]
Documentation/devicetree/bindings/dvfs/performance-domain.yaml
[2]
Documentation/devicetree/bindings/power/power-domain.yaml

Re: [PATCH v5 2/4] dt-bindings: cpufreq: apple,soc-cpufreq: Add binding for Apple SoC cpufreq

[Hector Martin]

On 29/11/2022 20.36, Ulf Hansson wrote:
> On Mon, 28 Nov 2022 at 15:29, Hector Martin <marcan@marcan.st> wrote:
>> +examples:
>> +  - |
>> +    // This example shows a single CPU per domain and 2 domains,
>> +    // with two p-states per domain.
>> +    // Shipping hardware has 2-4 CPUs per domain and 2-6 domains.
>> +    cpus {
>> +      #address-cells = <2>;
>> +      #size-cells = <0>;
>> +
>> +      cpu@0 {
>> +        compatible = "apple,icestorm";
>> +        device_type = "cpu";
>> +        reg = <0x0 0x0>;
>> +        operating-points-v2 = <&ecluster_opp>;
> 
> To me, it looks like the operating-points-v2 phandle better belongs in
> the performance-domains provider node. I mean, isn't the OPPs really a
> description of the performance-domain provider?
> 
> That said, I suggest we try to extend the generic performance-domain
> binding [1] with an "operating-points-v2". In that way, we should
> instead be able to reference it from this binding.
> 
> In fact, that would be very similar to what already exists for the
> generic power-domain binding [2]. I think it would be rather nice to
> follow a similar pattern for the performance-domain binding.

While I agree with the technical rationale and the proposed approach
being better in principle...

We're at v5 of bikeshedding this trivial driver's DT binding, and the
comment could've been made at v3. To quote IRC just now:

> this way the machines will be obsolete before things are fully upstreamed

I think it's long overdue for the kernel community to take a deep look
at itself and its development and review process, because it is quite
honestly insane how pathologically inefficient it is compared to,
basically, every other large and healthy open source project of similar
or even greater impact and scope.

Cc Linus, because this is for your Mac and I assume you care. We're at
v5 here for this silly driver. Meanwhile, rmk recently threw the towel
on upstreaming macsmc for us. We're trying, and I'll keep trying because
I actually get paid (by very generous donors) to do this, but if I
weren't I'd have given up a long time ago. And while I won't give up, I
can't deny this situation affects my morale and willingness to keep
pushing on upstreaming on a regular basis.

Meanwhile, OpenBSD has been *shipping* full M1 support for a while now
in official release images (and since Linux is the source of truth for
DT bindings, every time we re-bikeshed it we break their users because
they, quite reasonably, aren't interested in waiting for us Linux
slowpokes to figure it out first).

Please, let's introspect about this for a moment. Something is deeply
broken if people with 25+ years being an arch maintainer can't get a
700-line mfd driver upstreamed before giving up. I don't know how we
expect to ever get a Rust GPU driver merged if it takes 6+ versions to
upstream the world's easiest cpufreq hardware.

- Hector

Re: [PATCH v5 2/4] dt-bindings: cpufreq: apple,soc-cpufreq: Add binding for Apple SoC cpufreq

[Marc Zyngier]

On 2022-11-29 11:36, Ulf Hansson wrote:
> On Mon, 28 Nov 2022 at 15:29, Hector Martin <marcan@marcan.st> wrote:
>> 
>> +examples:
>> +  - |
>> +    // This example shows a single CPU per domain and 2 domains,
>> +    // with two p-states per domain.
>> +    // Shipping hardware has 2-4 CPUs per domain and 2-6 domains.
>> +    cpus {
>> +      #address-cells = <2>;
>> +      #size-cells = <0>;
>> +
>> +      cpu@0 {
>> +        compatible = "apple,icestorm";
>> +        device_type = "cpu";
>> +        reg = <0x0 0x0>;
>> +        operating-points-v2 = <&ecluster_opp>;
> 
> To me, it looks like the operating-points-v2 phandle better belongs in
> the performance-domains provider node. I mean, isn't the OPPs really a
> description of the performance-domain provider?
> 
> That said, I suggest we try to extend the generic performance-domain
> binding [1] with an "operating-points-v2". In that way, we should
> instead be able to reference it from this binding.
> 
> In fact, that would be very similar to what already exists for the
> generic power-domain binding [2]. I think it would be rather nice to
> follow a similar pattern for the performance-domain binding.

I'm not going to rabbit-hole into whether this is a good
or a bad binding. As far as I'm concerned, and as a user
of the HW it describes, it does the job in a satisfactory
manner.

What I'm concerned about is that this constant, last minute
bikeshedding that actively prevents upstreaming of support
for HW that people are actively using.

If anything, this is only causing people to stop trying to
contribute to the upstream kernel because of this constant
DT bikeshed. Honestly, writing code to support this HW is
a lot less effort than trying to get 3 different people to
agree on a binding.

It also affects other OSs that rely on the the same bindings,
and will eventually only result in developers not submitting
bindings anymore. After all, nothing says that bindings and
device trees have to exist in the Linux kernel tree.

         M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v5 2/4] dt-bindings: cpufreq: apple,soc-cpufreq: Add binding for Apple SoC cpufreq

[Krzysztof Kozlowski]

On 29/11/2022 15:00, Hector Martin wrote:
> On 29/11/2022 20.36, Ulf Hansson wrote:
>> On Mon, 28 Nov 2022 at 15:29, Hector Martin <marcan@marcan.st> wrote:
>>> +examples:
>>> +  - |
>>> +    // This example shows a single CPU per domain and 2 domains,
>>> +    // with two p-states per domain.
>>> +    // Shipping hardware has 2-4 CPUs per domain and 2-6 domains.
>>> +    cpus {
>>> +      #address-cells = <2>;
>>> +      #size-cells = <0>;
>>> +
>>> +      cpu@0 {
>>> +        compatible = "apple,icestorm";
>>> +        device_type = "cpu";
>>> +        reg = <0x0 0x0>;
>>> +        operating-points-v2 = <&ecluster_opp>;
>>
>> To me, it looks like the operating-points-v2 phandle better belongs in
>> the performance-domains provider node. I mean, isn't the OPPs really a
>> description of the performance-domain provider?
>>
>> That said, I suggest we try to extend the generic performance-domain
>> binding [1] with an "operating-points-v2". In that way, we should
>> instead be able to reference it from this binding.
>>
>> In fact, that would be very similar to what already exists for the
>> generic power-domain binding [2]. I think it would be rather nice to
>> follow a similar pattern for the performance-domain binding.
> 
> While I agree with the technical rationale and the proposed approach
> being better in principle...
> 
> We're at v5 of bikeshedding this trivial driver's DT binding, and the
> comment could've been made at v3. To quote IRC just now:
> 
>> this way the machines will be obsolete before things are fully upstreamed
> 
> I think it's long overdue for the kernel community to take a deep look
> at itself and its development and review process, because it is quite
> honestly insane how pathologically inefficient it is compared to,
> basically, every other large and healthy open source project of similar
> or even greater impact and scope.
> 
> Cc Linus, because this is for your Mac and I assume you care. We're at
> v5 here for this silly driver. Meanwhile, rmk recently threw the towel
> on upstreaming macsmc for us. We're trying, and I'll keep trying because
> I actually get paid (by very generous donors) to do this, but if I
> weren't I'd have given up a long time ago. And while I won't give up, I
> can't deny this situation affects my morale and willingness to keep
> pushing on upstreaming on a regular basis.
> 
> Meanwhile, OpenBSD has been *shipping* full M1 support for a while now
> in official release images (and since Linux is the source of truth for
> DT bindings, every time we re-bikeshed it we break their users because
> they, quite reasonably, aren't interested in waiting for us Linux
> slowpokes to figure it out first).
> 
> Please, let's introspect about this for a moment. Something is deeply
> broken if people with 25+ years being an arch maintainer can't get a

If arch maintainer sends patches which does not build (make
dt_binding_check), then what do you exactly expect? Accept them just
because it is 25+ years of experience or a maintainer? So we have
difference processes - for beginners code should compile. For
experienced people, it does not have to build because otherwise they
will get discouraged?

> 700-line mfd driver upstreamed before giving up. I don't know how we
> expect to ever get a Rust GPU driver merged if it takes 6+ versions to
> upstream the world's easiest cpufreq hardware.

While I understand your point about bikeschedding, but I think your
previous bindings got pretty nice and fast reviews, so using examples of
non-building case is poor choice.

Best regards,
Krzysztof

Re: [PATCH v5 2/4] dt-bindings: cpufreq: apple,soc-cpufreq: Add binding for Apple SoC cpufreq

[Hector Martin]

On 29/11/2022 23.34, Krzysztof Kozlowski wrote:
> On 29/11/2022 15:00, Hector Martin wrote:
>> On 29/11/2022 20.36, Ulf Hansson wrote:
>> Please, let's introspect about this for a moment. Something is deeply
>> broken if people with 25+ years being an arch maintainer can't get a
> 
> If arch maintainer sends patches which does not build (make
> dt_binding_check), then what do you exactly expect? Accept them just
> because it is 25+ years of experience or a maintainer? So we have
> difference processes - for beginners code should compile. For
> experienced people, it does not have to build because otherwise they
> will get discouraged?

I expect the process to not be so confusing and frustrating that a
maintainer with 25+ years of experience gives up. That the bindings
didn't pass the checker is besides the point. People say the Linux
kernel community is hostile to newbies. This issue proves it's not just
newbies, the process is failing even experienced folks.

On that specific issue, any other functional open source project would
have the binding checks be a CI bot, with a friendly message telling you
what to do to fix it, and it would re-run when you push to the PR again,
which is a *much* lower friction action than sending a whole new patch
series out for review via email (if you don't agree with this, then
you're not the average contributor - the Linux kernel is by far the
scariest major open source project to contribute to, and I think most
people would agree with me on that).

I know Rob has a DT checker bot, but its error output is practically
line noise, and the error email doesn't even mention the
DT_SCHEMA_FILES= make option (which is the only way to make the check
not take *forever* to run). Absolutely nobody is going to look at those
emails without already knowing the intricacies of DT bindings and the
checker and not find them incredibly frustrating.

But it's not just the DT checker. That came after an argument where the
MFD maintainer complained about the driver while offering no solution
nor proposed path forward. I had to have an IRC conversation with him to
work it out, after which he accepted one of the options I'd already
proposed over email. If you have to change communication mediums to
resolve an issue, that means your initial medium failed at its job.

Not to mention the random drive-by reviews, nitpicks, disagreements
between maintainers about how to do things, or just plain cases of
maintainers stubbornly being *wrong* and refusing to listen while
everyone around them is telling them they're wrong (until someone above
them in the maintainer tree tells them they're wrong - then they finally
get it. If it takes someone in a position of authority telling you
you're wrong for you to accept it, you're doing a poor job at your own
position.)

And then there's the coding style. The rest of the world has
standardized on formatting tools. Here, every subsystem maintainer has
their own pet style you have to learn. "Please put your variables in
reverse christmas tree order". I once got a review comment that
complained that I re-aligned an existing #define when I added another
one (to keep them visually aligned, as the new one increased the
required tab stop). Because "cleanup patches should be separate" (I
didn't submit a cleanup patch after that, I just left the defines
misaligned). So now I need to maintain a massive mental map of exactly
what style conventions and patch breakup conventions every subsystem
maintainer wants me to use.

I'm so glad `make rustfmt` is a thing now. Maybe 50 years down the line,
most of the kernel will have been rewritten in Rust and we'll finally
fix just one of these problems /s

Some of these things are, to some extent, a natural result of working
with other humans. But the kernel community has both a number of humans
harder to work with than is reasonable in their position, and an overall
process that multiplies the resulting pain by an order of magnitude for
everyone involved.

> While I understand your point about bikeschedding, but I think your
> previous bindings got pretty nice and fast reviews, so using examples of
> non-building case is poor choice.

Yeah, after a while, because I learned how to do DT bindings the hard
way after having to submit a bunch and getting everything wrong (and
even then I still get the simplest things wrong, see: v4 here). Which is
why I'll pick up after rmk's attempt and try again with macsmc at some
point (probably after 6.1). But I'm not holding my breath that I won't
need another half dozen rounds of bikeshedding.

When it takes 10 times more man-hours to upstream a driver than to
reverse engineer and write it with zero documentation, never mind the
calendar time it takes, something is very wrong. I actively dread
submitting new drivers to new subsystems or some existing ones now. How
much pain will the next one be? Will I be asked to move files around 3
times? Spend 4 rounds bikeshedding the DT schema? Think it's finally
ready only for someone to show up and ask to change a major part of the
design at the last minute?

And this all when we're actually submitting code of decent quality (I
think I have enough experience not to submit crap most of the time). Now
imagine how much worse this all is for a newbie who submits a
well-intentioned but definitely not up to standard patch. There's a huge
chance they'll give up before ever getting the submission through.

Again, I'll keep pushing and sending out patches, because this is
important. But this is the reality. This is how bad it is. The process
is broken, and everyone outside the kernel community can see that and
has been saying that for ages. Just because some of us are willing to
put up with the pain anyway doesn't mean it's working as intended.

- Hector

Re: [PATCH v5 2/4] dt-bindings: cpufreq: apple,soc-cpufreq: Add binding for Apple SoC cpufreq

[Krzysztof Kozlowski]

On 29/11/2022 16:17, Hector Martin wrote:
> On 29/11/2022 23.34, Krzysztof Kozlowski wrote:
>> On 29/11/2022 15:00, Hector Martin wrote:
>>> On 29/11/2022 20.36, Ulf Hansson wrote:
>>> Please, let's introspect about this for a moment. Something is deeply
>>> broken if people with 25+ years being an arch maintainer can't get a
>>
>> If arch maintainer sends patches which does not build (make
>> dt_binding_check), then what do you exactly expect? Accept them just
>> because it is 25+ years of experience or a maintainer? So we have
>> difference processes - for beginners code should compile. For
>> experienced people, it does not have to build because otherwise they
>> will get discouraged?
> 
> I expect the process to not be so confusing and frustrating that a
> maintainer with 25+ years of experience gives up. That the bindings
> didn't pass the checker is besides the point. People say the Linux
> kernel community is hostile to newbies. This issue proves it's not just
> newbies, the process is failing even experienced folks.
> 
> On that specific issue, any other functional open source project would
> have the binding checks be a CI bot, with a friendly message telling you
> what to do to fix it, and it would re-run when you push to the PR again,
> which is a *much* lower friction action than sending a whole new patch
> series out for review via email (if you don't agree with this, then
> you're not the average contributor - the Linux kernel is by far the
> scariest major open source project to contribute to, and I think most
> people would agree with me on that).

I agree with this entirely. Not only DT checks, but also for driver code
with sparse/smatch/coccinelle/W=1/clang builds.

> I know Rob has a DT checker bot, but its error output is practically
> line noise, and the error email doesn't even mention the
> DT_SCHEMA_FILES= make option (which is the only way to make the check
> not take *forever* to run). Absolutely nobody is going to look at those
> emails without already knowing the intricacies of DT bindings and the
> checker and not find them incredibly frustrating.

Ack

> 
> But it's not just the DT checker. That came after an argument where the
> MFD maintainer complained about the driver while offering no solution
> nor proposed path forward. I had to have an IRC conversation with him to
> work it out, after which he accepted one of the options I'd already
> proposed over email. If you have to change communication mediums to
> resolve an issue, that means your initial medium failed at its job.

Ack

> 
> Not to mention the random drive-by reviews, nitpicks, disagreements
> between maintainers about how to do things, or just plain cases of
> maintainers stubbornly being *wrong* and refusing to listen while
> everyone around them is telling them they're wrong (until someone above
> them in the maintainer tree tells them they're wrong - then they finally
> get it. If it takes someone in a position of authority telling you
> you're wrong for you to accept it, you're doing a poor job at your own
> position.)

Ack

> 
> And then there's the coding style. The rest of the world has
> standardized on formatting tools. Here, every subsystem maintainer has
> their own pet style you have to learn. "Please put your variables in
> reverse christmas tree order".

Ack here as well, and by above Acks I meant - I agree, you are right.  I
don't think we should discuss people's differences, I mean, different
people can give you different review, sometimes less sometimes more
thorough. However at least policies/processes and coding style should be
heavily unified.

> I once got a review comment that
> complained that I re-aligned an existing #define when I added another
> one (to keep them visually aligned, as the new one increased the
> required tab stop). Because "cleanup patches should be separate" (I
> didn't submit a cleanup patch after that, I just left the defines
> misaligned). So now I need to maintain a massive mental map of exactly
> what style conventions and patch breakup conventions every subsystem
> maintainer wants me to use.

Yep. I just have a doc, instead of mental map. :)

> 
> I'm so glad `make rustfmt` is a thing now. Maybe 50 years down the line,
> most of the kernel will have been rewritten in Rust and we'll finally
> fix just one of these problems /s

+1

> 
> Some of these things are, to some extent, a natural result of working
> with other humans. But the kernel community has both a number of humans
> harder to work with than is reasonable in their position, and an overall
> process that multiplies the resulting pain by an order of magnitude for
> everyone involved.
> 
>> While I understand your point about bikeschedding, but I think your
>> previous bindings got pretty nice and fast reviews, so using examples of
>> non-building case is poor choice.
> 
> Yeah, after a while, because I learned how to do DT bindings the hard
> way after having to submit a bunch and getting everything wrong (and
> even then I still get the simplest things wrong, see: v4 here). Which is
> why I'll pick up after rmk's attempt and try again with macsmc at some
> point (probably after 6.1). But I'm not holding my breath that I won't
> need another half dozen rounds of bikeshedding.
> 
> When it takes 10 times more man-hours to upstream a driver than to
> reverse engineer and write it with zero documentation, never mind the
> calendar time it takes, something is very wrong. 

Yes, that's a point. Yet we (community) still have another goal here
than the pure goal of submitter. The submitter wants its stuff upstream.
Therefore this upstreaming effort is for submitter a bit wasted (as I
agree that reverse-engineering should be the biggest task).

However the community (expressed maybe mostly as maintainers) wants
something which will be maintainable and usable also for others, not
only for patch submitters.

Indeed maybe 10x difference between writing code and upstreaming is
something to improve, but please do not forget that the concept is here
to be able to manage this big codebase.

> I actively dread
> submitting new drivers to new subsystems or some existing ones now. How
> much pain will the next one be? Will I be asked to move files around 3
> times? Spend 4 rounds bikeshedding the DT schema? Think it's finally
> ready only for someone to show up and ask to change a major part of the
> design at the last minute?
> 
> And this all when we're actually submitting code of decent quality (I
> think I have enough experience not to submit crap most of the time). Now
> imagine how much worse this all is for a newbie who submits a
> well-intentioned but definitely not up to standard patch. There's a huge
> chance they'll give up before ever getting the submission through.

You say this like any newbie should be able to send a patch and get it
accepted right away, regardless of actually what is in that patch (not
up to standard). It's not newbie's right. No one said it's easy and fast
process... If you want easy and fast, do JavaScript... If you want to
make it easier and faster in the kernel, then we need more reviewers and
maintainers. Somehow many, many people want to send patches, but not
that many want to review them.

> 
> Again, I'll keep pushing and sending out patches, because this is
> important. But this is the reality. This is how bad it is. The process
> is broken, and everyone outside the kernel community can see that and
> has been saying that for ages. Just because some of us are willing to
> put up with the pain anyway doesn't mean it's working as intended.
Best regards,
Krzysztof

Re: [PATCH v5 2/4] dt-bindings: cpufreq: apple,soc-cpufreq: Add binding for Apple SoC cpufreq

[Hector Martin]

On 30/11/2022 00.46, Krzysztof Kozlowski wrote:
> On 29/11/2022 16:17, Hector Martin wrote:
>> When it takes 10 times more man-hours to upstream a driver than to
>> reverse engineer and write it with zero documentation, never mind the
>> calendar time it takes, something is very wrong. 
> 
> Yes, that's a point. Yet we (community) still have another goal here
> than the pure goal of submitter. The submitter wants its stuff upstream.
> Therefore this upstreaming effort is for submitter a bit wasted (as I
> agree that reverse-engineering should be the biggest task).
> 
> However the community (expressed maybe mostly as maintainers) wants
> something which will be maintainable and usable also for others, not
> only for patch submitters.
> 
> Indeed maybe 10x difference between writing code and upstreaming is
> something to improve, but please do not forget that the concept is here
> to be able to manage this big codebase.

You're misunderstanding me here. The problem isn't the goal of keeping
things maintainable. The problem is that the process involved makes the
entire experience take much, much longer than it should. A more
efficient process could maintain the same (actual) code quality with a
lot less time wasted for everyone, both in process issues and in
bikeshedding details that don't actually increase code quality.

>> I actively dread
>> submitting new drivers to new subsystems or some existing ones now. How
>> much pain will the next one be? Will I be asked to move files around 3
>> times? Spend 4 rounds bikeshedding the DT schema? Think it's finally
>> ready only for someone to show up and ask to change a major part of the
>> design at the last minute?
>>
>> And this all when we're actually submitting code of decent quality (I
>> think I have enough experience not to submit crap most of the time). Now
>> imagine how much worse this all is for a newbie who submits a
>> well-intentioned but definitely not up to standard patch. There's a huge
>> chance they'll give up before ever getting the submission through.
> 
> You say this like any newbie should be able to send a patch and get it
> accepted right away, regardless of actually what is in that patch (not
> up to standard). It's not newbie's right. No one said it's easy and fast
> process... If you want easy and fast, do JavaScript... If you want to
> make it easier and faster in the kernel, then we need more reviewers and
> maintainers. Somehow many, many people want to send patches, but not
> that many want to review them.

Again, I'm not saying the bad code should go in. I'm saying the
*process* is so frustrating that most newbies will give up before the
code has a chance to evolve from bad to good and go in. The solution
isn't to let more bad code in, it's to make the process less painful.

And yes, some of it does mean either relaxing style rules or just
adopting a formatter, because let's face it: complaints that a line is
81 characters instead of 80 don't *actually* increase code quality to
any measurable extent, they just waste everyone's time, and if that
level of style pickiness is desired, that's what formatters are for.

I have personal experience with this with the Asahi Linux bootloader
(m1n1). We use a formatter for C; the CI tells people when they have to
run `make format` to fix their style and that solves all our C
formatting issues (incidentally, we also have a CI bot to check for
S-o-b lines - very handy). We don't use a formatter for Python, and as a
result, I don't impose overly strict rules on things like exactly how
many lines of whitespace should exist between functions or exactly how
expression continuations should be indented or what the maximum line
length is (within reason), because code doesn't have to look like it was
written by a homogeneous robot to still be completely understandable and
readable to humans using modern editing tools, and trying to teach every
contributor my own personal style (which isn't even 100% consistent, I'm
not a robot either!) would be a waste of my and their time. Most
contributors are capable of defaulting to a readable coding style
similar to the rest of the code around it, and it's rare I have to tell
someone to fix the formatting because something is clearly wrong (like
blatantly inconsistent block indentation).

- Hector

Re: [PATCH v5 2/4] dt-bindings: cpufreq: apple,soc-cpufreq: Add binding for Apple SoC cpufreq

[Ulf Hansson]

On Tue, 29 Nov 2022 at 15:00, Hector Martin <marcan@marcan.st> wrote:
>
> On 29/11/2022 20.36, Ulf Hansson wrote:
> > On Mon, 28 Nov 2022 at 15:29, Hector Martin <marcan@marcan.st> wrote:
> >> +examples:
> >> +  - |
> >> +    // This example shows a single CPU per domain and 2 domains,
> >> +    // with two p-states per domain.
> >> +    // Shipping hardware has 2-4 CPUs per domain and 2-6 domains.
> >> +    cpus {
> >> +      #address-cells = <2>;
> >> +      #size-cells = <0>;
> >> +
> >> +      cpu@0 {
> >> +        compatible = "apple,icestorm";
> >> +        device_type = "cpu";
> >> +        reg = <0x0 0x0>;
> >> +        operating-points-v2 = <&ecluster_opp>;
> >
> > To me, it looks like the operating-points-v2 phandle better belongs in
> > the performance-domains provider node. I mean, isn't the OPPs really a
> > description of the performance-domain provider?
> >
> > That said, I suggest we try to extend the generic performance-domain
> > binding [1] with an "operating-points-v2". In that way, we should
> > instead be able to reference it from this binding.
> >
> > In fact, that would be very similar to what already exists for the
> > generic power-domain binding [2]. I think it would be rather nice to
> > follow a similar pattern for the performance-domain binding.
>
> While I agree with the technical rationale and the proposed approach
> being better in principle...
>
> We're at v5 of bikeshedding this trivial driver's DT binding, and the
> comment could've been made at v3. To quote IRC just now:

It could and I certainly apologize for that.

It's simply been a busy period for me, so I haven't been able to look
closer at the DT bindings, until now.

>
> > this way the machines will be obsolete before things are fully upstreamed
>
> I think it's long overdue for the kernel community to take a deep look
> at itself and its development and review process, because it is quite
> honestly insane how pathologically inefficient it is compared to,
> basically, every other large and healthy open source project of similar
> or even greater impact and scope.
>
> Cc Linus, because this is for your Mac and I assume you care. We're at
> v5 here for this silly driver. Meanwhile, rmk recently threw the towel
> on upstreaming macsmc for us. We're trying, and I'll keep trying because
> I actually get paid (by very generous donors) to do this, but if I
> weren't I'd have given up a long time ago. And while I won't give up, I
> can't deny this situation affects my morale and willingness to keep
> pushing on upstreaming on a regular basis.
>
> Meanwhile, OpenBSD has been *shipping* full M1 support for a while now
> in official release images (and since Linux is the source of truth for
> DT bindings, every time we re-bikeshed it we break their users because
> they, quite reasonably, aren't interested in waiting for us Linux
> slowpokes to figure it out first).
>
> Please, let's introspect about this for a moment. Something is deeply
> broken if people with 25+ years being an arch maintainer can't get a
> 700-line mfd driver upstreamed before giving up. I don't know how we
> expect to ever get a Rust GPU driver merged if it takes 6+ versions to
> upstream the world's easiest cpufreq hardware.
>
> - Hector

I didn't intend to bikesheed this, while I do understand your valid
concerns from the above statements.

Instead, my intent was to help, by reviewing. Simply, because I care
about this too.

If you think incorporating the changes I proposed is a too big deal at
this point, let me not stand in the way of applying this. In the end,
it's the DT maintainers' decision.

Kind regards
Uffe

Re: [PATCH v5 2/4] dt-bindings: cpufreq: apple,soc-cpufreq: Add binding for Apple SoC cpufreq

[Rob Herring]

On Wed, Nov 30, 2022 at 12:17:08AM +0900, Hector Martin wrote:
> On 29/11/2022 23.34, Krzysztof Kozlowski wrote:
> > On 29/11/2022 15:00, Hector Martin wrote:
> >> On 29/11/2022 20.36, Ulf Hansson wrote:
> >> Please, let's introspect about this for a moment. Something is deeply
> >> broken if people with 25+ years being an arch maintainer can't get a
> > 
> > If arch maintainer sends patches which does not build (make
> > dt_binding_check), then what do you exactly expect? Accept them just
> > because it is 25+ years of experience or a maintainer? So we have
> > difference processes - for beginners code should compile. For
> > experienced people, it does not have to build because otherwise they
> > will get discouraged?
> 
> I expect the process to not be so confusing and frustrating that a
> maintainer with 25+ years of experience gives up. That the bindings
> didn't pass the checker is besides the point. People say the Linux
> kernel community is hostile to newbies. This issue proves it's not just
> newbies, the process is failing even experienced folks.

IME, a lack of response is a bigger issue and more frustrating.

> On that specific issue, any other functional open source project would
> have the binding checks be a CI bot, with a friendly message telling you
> what to do to fix it, and it would re-run when you push to the PR again,
> which is a *much* lower friction action than sending a whole new patch
> series out for review via email (if you don't agree with this, then
> you're not the average contributor - the Linux kernel is by far the
> scariest major open source project to contribute to, and I think most
> people would agree with me on that).

We could probably add a $ci_provider job description to do that. In 
fact, I did try that once[1]. The challenge would be what to run if 
there's multiple maintainers doing something. Otherwise, it's a 
maintainer creating their own thing which we have too much of already.

> I know Rob has a DT checker bot, but its error output is practically
> line noise,

I'm not sure what to do there beyond the 'hint' lines I've added. It's 
kind of how json-schema functions unfortunately. I think it stems from 
each schema keyword being evaluated independently.

>  and the error email doesn't even mention the
> DT_SCHEMA_FILES= make option (which is the only way to make the check
> not take *forever* to run).

That's easy enough to add and a specific suggestion I can act on.

However, note that the full thing still has to be run because any 
schema change can affect any other example (which is a large part of 
why it's slow).

>  Absolutely nobody is going to look at those
> emails without already knowing the intricacies of DT bindings and the
> checker and not find them incredibly frustrating.

I don't know how else to enable someone not understanding DT bindings 
nor json-schema to write DT bindings. I get that json-schema is not 
something kernel developers typically know already. Trust me, the 
alternatives proposed for a schema over the years would have been 
much worse.

Rob

[1] https://lore.kernel.org/all/20181003222715.28667-1-robh@kernel.org/

Re: [PATCH v5 2/4] dt-bindings: cpufreq: apple,soc-cpufreq: Add binding for Apple SoC cpufreq

[Rob Herring]

On Mon, 28 Nov 2022 23:29:10 +0900, Hector Martin wrote:
> This binding represents the cpufreq/DVFS hardware present in Apple SoCs.
> The hardware has an independent controller per CPU cluster, and we
> represent them as unique nodes in order to accurately describe the
> hardware. The driver is responsible for binding them as a single cpufreq
> device (in the Linux cpufreq model).
> 
> Acked-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>  .../cpufreq/apple,cluster-cpufreq.yaml        | 117 ++++++++++++++++++
>  1 file changed, 117 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/cpufreq/apple,cluster-cpufreq.yaml

In case there's any doubt:

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v5 3/4] cpufreq: apple-soc: Add new driver to control Apple SoC CPU P-states

[Viresh Kumar]

On 28-11-22, 23:29, Hector Martin wrote:
> This driver implements CPU frequency scaling for Apple Silicon SoCs,
> including M1 (t8103), M1 Max/Pro/Ultra (t600x), and M2 (t8112).
> 
> Each CPU cluster has its own register set, and frequency management is
> fully automated by the hardware; the driver only has to write one
> register. There is boost frequency support, but the hardware will only
> allow their use if only a subset of cores in a cluster are in
> non-deep-idle. Since we don't support deep idle yet, these frequencies
> are not achievable, but the driver supports them. They will remain
> disabled in the device tree until deep idle is implemented, to avoid
> confusing users.
> 
> This driver does not yet implement the memory controller performance
> state tuning that usually accompanies higher CPU p-states. This will be
> done in a future patch.
> 
> Acked-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>  drivers/cpufreq/Kconfig.arm          |   9 +
>  drivers/cpufreq/Makefile             |   1 +
>  drivers/cpufreq/apple-soc-cpufreq.c  | 352 +++++++++++++++++++++++++++
>  drivers/cpufreq/cpufreq-dt-platdev.c |   2 +
>  4 files changed, 364 insertions(+)
>  create mode 100644 drivers/cpufreq/apple-soc-cpufreq.c

Applied. Thanks.

-- 
viresh

Re: [PATCH v5 0/4] Apple SoC cpufreq driver

[Viresh Kumar]

On 28-11-22, 23:29, Hector Martin wrote:
> Hi folks,
> 
> Here's v5 of the cpufreq driver for Apple SoCs. v5 just incorporates
> minor review feedback changes from v3, and no functional changes. v4
> had a DT schema SNAFU; this supersedes it.
> 
> Once reviewed, please merge #3 via the cpufreq tree, and we'll take
> care of #1,#2,#4 via the asahi-soc tree. This lets us merge the DT
> changes in the same cycle without blocking on the binding coming in
> via the cpufreq tree first.

For patches 1/2/4:

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH v5 2/4] dt-bindings: cpufreq: apple,soc-cpufreq: Add binding for Apple SoC cpufreq

[Rob Herring]

On Tue, Nov 29, 2022 at 5:28 PM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Nov 30, 2022 at 12:17:08AM +0900, Hector Martin wrote:
> > On 29/11/2022 23.34, Krzysztof Kozlowski wrote:
> > > On 29/11/2022 15:00, Hector Martin wrote:
> > >> On 29/11/2022 20.36, Ulf Hansson wrote:
> > >> Please, let's introspect about this for a moment. Something is deeply
> > >> broken if people with 25+ years being an arch maintainer can't get a
> > >
> > > If arch maintainer sends patches which does not build (make
> > > dt_binding_check), then what do you exactly expect? Accept them just
> > > because it is 25+ years of experience or a maintainer? So we have
> > > difference processes - for beginners code should compile. For
> > > experienced people, it does not have to build because otherwise they
> > > will get discouraged?
> >
> > I expect the process to not be so confusing and frustrating that a
> > maintainer with 25+ years of experience gives up. That the bindings
> > didn't pass the checker is besides the point. People say the Linux
> > kernel community is hostile to newbies. This issue proves it's not just
> > newbies, the process is failing even experienced folks.
>
> IME, a lack of response is a bigger issue and more frustrating.
>
> > On that specific issue, any other functional open source project would
> > have the binding checks be a CI bot, with a friendly message telling you
> > what to do to fix it, and it would re-run when you push to the PR again,
> > which is a *much* lower friction action than sending a whole new patch
> > series out for review via email (if you don't agree with this, then
> > you're not the average contributor - the Linux kernel is by far the
> > scariest major open source project to contribute to, and I think most
> > people would agree with me on that).
>
> We could probably add a $ci_provider job description to do that. In
> fact, I did try that once[1]. The challenge would be what to run if
> there's multiple maintainers doing something. Otherwise, it's a
> maintainer creating their own thing which we have too much of already.

Actually, turns out this pretty much already exists with my CI. I just
had to turn on merge requests on the project. If anyone actually uses
it, I'll have to tweak it to not do 'make dtbs_check' because that is
really slow. And this all runs on my machines, so that is another
issue. It already is just running it for patches on the list (which is
a different CI job).

Just create a MR here:

https://gitlab.com/robherring/linux-dt/-/merge_requests

Rob