2020-06-26 17:04:48

by Sumit Gupta

[permalink] [raw]
Subject: [TEGRA194_CPUFREQ PATCH v4 0/4] Add cpufreq driver for Tegra194

The patch series adds cpufreq driver for Tegra194 SOC.
Incorporated the feedback on previous version patchset.
Please consider this patch series for merging in 5.9.

v3[3] -> v4
- Open code LOOP_FOR_EACH_CPU_OF_CLUSTER macro[Viresh]
- Delete unused funciton map_freq_to_ndiv[Viresh, kernel test bot]
- Remove flush_workqueue from free_resources[Viresh]

v2[2] -> v3
- Set same policy for all cpus in a cluster[Viresh].
- Add compatible string for CPU Complex under cpus node[Thierry].
- Add reference to bpmp node under cpus node[Thierry].
- Bind cpufreq driver to CPU Complex compatible string[Thierry].
- Remove patch to get bpmp data as now using cpus node to get that[Thierry].

v1[1] -> v2:
- Remove cpufreq_lock mutex from tegra194_cpufreq_set_target [Viresh].
- Remove CPUFREQ_ASYNC_NOTIFICATION flag [Viresh].
- Remove redundant _begin|end() call from tegra194_cpufreq_set_target.
- Rename opp_table to freq_table [Viresh].

Sumit Gupta (4):
dt-bindings: arm: Add t194 ccplex compatible and bpmp property
arm64: tegra: Add t194 ccplex compatible and bpmp property
cpufreq: Add Tegra194 cpufreq driver
arm64: defconfig: Enable CONFIG_ARM_TEGRA194_CPUFREQ

Documentation/devicetree/bindings/arm/cpus.yaml | 9 +
arch/arm64/boot/dts/nvidia/tegra194.dtsi | 2 +
arch/arm64/configs/defconfig | 1 +
drivers/cpufreq/Kconfig.arm | 6 +
drivers/cpufreq/Makefile | 1 +
drivers/cpufreq/tegra194-cpufreq.c | 396 ++++++++++++++++++++++++
6 files changed, 415 insertions(+)
create mode 100644 drivers/cpufreq/tegra194-cpufreq.c

[1] https://marc.info/?t=157539452300001&r=1&w=2
[2] https://marc.info/?l=linux-tegra&m=158602857106213&w=2
[3] https://marc.info/?l=linux-pm&m=159283376010084&w=2
--
2.7.4


2020-06-26 17:06:08

by Sumit Gupta

[permalink] [raw]
Subject: [TEGRA194_CPUFREQ PATCH v4 2/4] arm64: tegra: Add t194 ccplex compatible and bpmp property

On Tegra194, data on valid operating points for the CPUs needs to be
queried from BPMP. In T194, there is no node representing CPU complex.
So, add compatible string to the 'cpus' node instead of using dummy
node to bind cpufreq driver. Also, add reference to the BPMP instance
for the CPU complex.

Signed-off-by: Sumit Gupta <[email protected]>
---
arch/arm64/boot/dts/nvidia/tegra194.dtsi | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index 7c9511a..0abf287 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -1764,6 +1764,8 @@
};

cpus {
+ compatible = "nvidia,tegra194-ccplex";
+ nvidia,bpmp = <&bpmp>;
#address-cells = <1>;
#size-cells = <0>;

--
2.7.4

2020-06-26 17:06:40

by Sumit Gupta

[permalink] [raw]
Subject: [TEGRA194_CPUFREQ PATCH v4 3/4] cpufreq: Add Tegra194 cpufreq driver

Add support for CPU frequency scaling on Tegra194. The frequency
of each core can be adjusted by writing a clock divisor value to
a MSR on the core. The range of valid divisors is queried from
the BPMP.

Signed-off-by: Mikko Perttunen <[email protected]>
Signed-off-by: Sumit Gupta <[email protected]>
---
drivers/cpufreq/Kconfig.arm | 6 +
drivers/cpufreq/Makefile | 1 +
drivers/cpufreq/tegra194-cpufreq.c | 396 +++++++++++++++++++++++++++++++++++++
3 files changed, 403 insertions(+)
create mode 100644 drivers/cpufreq/tegra194-cpufreq.c

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 15c1a12..f3d8f09 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -314,6 +314,12 @@ config ARM_TEGRA186_CPUFREQ
help
This adds the CPUFreq driver support for Tegra186 SOCs.

+config ARM_TEGRA194_CPUFREQ
+ tristate "Tegra194 CPUFreq support"
+ depends on ARCH_TEGRA && TEGRA_BPMP
+ help
+ This adds CPU frequency driver support for Tegra194 SOCs.
+
config ARM_TI_CPUFREQ
bool "Texas Instruments CPUFreq support"
depends on ARCH_OMAP2PLUS
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index f6670c4..66b5563 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -83,6 +83,7 @@ obj-$(CONFIG_ARM_TANGO_CPUFREQ) += tango-cpufreq.o
obj-$(CONFIG_ARM_TEGRA20_CPUFREQ) += tegra20-cpufreq.o
obj-$(CONFIG_ARM_TEGRA124_CPUFREQ) += tegra124-cpufreq.o
obj-$(CONFIG_ARM_TEGRA186_CPUFREQ) += tegra186-cpufreq.o
+obj-$(CONFIG_ARM_TEGRA194_CPUFREQ) += tegra194-cpufreq.o
obj-$(CONFIG_ARM_TI_CPUFREQ) += ti-cpufreq.o
obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o

diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c
new file mode 100644
index 0000000..77e422a
--- /dev/null
+++ b/drivers/cpufreq/tegra194-cpufreq.c
@@ -0,0 +1,396 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020, NVIDIA CORPORATION. All rights reserved
+ */
+
+#include <linux/cpu.h>
+#include <linux/cpufreq.h>
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include <asm/smp_plat.h>
+
+#include <soc/tegra/bpmp.h>
+#include <soc/tegra/bpmp-abi.h>
+
+#define KHZ 1000
+#define REF_CLK_MHZ 408 /* 408 MHz */
+#define US_DELAY 500
+#define US_DELAY_MIN 2
+#define CPUFREQ_TBL_STEP_HZ (50 * KHZ * KHZ)
+#define MAX_CNT ~0U
+
+/* cpufreq transisition latency */
+#define TEGRA_CPUFREQ_TRANSITION_LATENCY (300 * 1000) /* unit in nanoseconds */
+
+enum cluster {
+ CLUSTER0,
+ CLUSTER1,
+ CLUSTER2,
+ CLUSTER3,
+ MAX_CLUSTERS,
+};
+
+struct tegra194_cpufreq_data {
+ void __iomem *regs;
+ size_t num_clusters;
+ struct cpufreq_frequency_table **tables;
+};
+
+struct tegra_cpu_ctr {
+ u32 cpu;
+ u32 delay;
+ u32 coreclk_cnt, last_coreclk_cnt;
+ u32 refclk_cnt, last_refclk_cnt;
+};
+
+struct read_counters_work {
+ struct work_struct work;
+ struct tegra_cpu_ctr c;
+};
+
+static struct workqueue_struct *read_counters_wq;
+
+static enum cluster get_cpu_cluster(u8 cpu)
+{
+ return MPIDR_AFFINITY_LEVEL(cpu_logical_map(cpu), 1);
+}
+
+/*
+ * Read per-core Read-only system register NVFREQ_FEEDBACK_EL1.
+ * The register provides frequency feedback information to
+ * determine the average actual frequency a core has run at over
+ * a period of time.
+ * [31:0] PLLP counter: Counts at fixed frequency (408 MHz)
+ * [63:32] Core clock counter: counts on every core clock cycle
+ * where the core is architecturally clocking
+ */
+static u64 read_freq_feedback(void)
+{
+ u64 val = 0;
+
+ asm volatile("mrs %0, s3_0_c15_c0_5" : "=r" (val) : );
+
+ return val;
+}
+
+static inline u32 map_ndiv_to_freq(struct mrq_cpu_ndiv_limits_response
+ *nltbl, u16 ndiv)
+{
+ return nltbl->ref_clk_hz / KHZ * ndiv / (nltbl->pdiv * nltbl->mdiv);
+}
+
+static void tegra_read_counters(struct work_struct *work)
+{
+ struct read_counters_work *read_counters_work;
+ struct tegra_cpu_ctr *c;
+ u64 val;
+
+ /*
+ * ref_clk_counter(32 bit counter) runs on constant clk,
+ * pll_p(408MHz).
+ * It will take = 2 ^ 32 / 408 MHz to overflow ref clk counter
+ * = 10526880 usec = 10.527 sec to overflow
+ *
+ * Like wise core_clk_counter(32 bit counter) runs on core clock.
+ * It's synchronized to crab_clk (cpu_crab_clk) which runs at
+ * freq of cluster. Assuming max cluster clock ~2000MHz,
+ * It will take = 2 ^ 32 / 2000 MHz to overflow core clk counter
+ * = ~2.147 sec to overflow
+ */
+ read_counters_work = container_of(work, struct read_counters_work,
+ work);
+ c = &read_counters_work->c;
+
+ val = read_freq_feedback();
+ c->last_refclk_cnt = lower_32_bits(val);
+ c->last_coreclk_cnt = upper_32_bits(val);
+ udelay(c->delay);
+ val = read_freq_feedback();
+ c->refclk_cnt = lower_32_bits(val);
+ c->coreclk_cnt = upper_32_bits(val);
+}
+
+/*
+ * Return instantaneous cpu speed
+ * Instantaneous freq is calculated as -
+ * -Takes sample on every query of getting the freq.
+ * - Read core and ref clock counters;
+ * - Delay for X us
+ * - Read above cycle counters again
+ * - Calculates freq by subtracting current and previous counters
+ * divided by the delay time or eqv. of ref_clk_counter in delta time
+ * - Return Kcycles/second, freq in KHz
+ *
+ * delta time period = x sec
+ * = delta ref_clk_counter / (408 * 10^6) sec
+ * freq in Hz = cycles/sec
+ * = (delta cycles / x sec
+ * = (delta cycles * 408 * 10^6) / delta ref_clk_counter
+ * in KHz = (delta cycles * 408 * 10^3) / delta ref_clk_counter
+ *
+ * @cpu - logical cpu whose freq to be updated
+ * Returns freq in KHz on success, 0 if cpu is offline
+ */
+static unsigned int tegra194_get_speed_common(u32 cpu, u32 delay)
+{
+ struct read_counters_work read_counters_work;
+ struct tegra_cpu_ctr c;
+ u32 delta_refcnt;
+ u32 delta_ccnt;
+ u32 rate_mhz;
+
+ /*
+ * udelay() is required to reconstruct cpu frequency over an
+ * observation window. Using workqueue to call udelay() with
+ * interrupts enabled.
+ */
+ read_counters_work.c.cpu = cpu;
+ read_counters_work.c.delay = delay;
+ INIT_WORK_ONSTACK(&read_counters_work.work, tegra_read_counters);
+ queue_work_on(cpu, read_counters_wq, &read_counters_work.work);
+ flush_work(&read_counters_work.work);
+ c = read_counters_work.c;
+
+ if (c.coreclk_cnt < c.last_coreclk_cnt)
+ delta_ccnt = c.coreclk_cnt + (MAX_CNT - c.last_coreclk_cnt);
+ else
+ delta_ccnt = c.coreclk_cnt - c.last_coreclk_cnt;
+ if (!delta_ccnt)
+ return 0;
+
+ /* ref clock is 32 bits */
+ if (c.refclk_cnt < c.last_refclk_cnt)
+ delta_refcnt = c.refclk_cnt + (MAX_CNT - c.last_refclk_cnt);
+ else
+ delta_refcnt = c.refclk_cnt - c.last_refclk_cnt;
+ if (!delta_refcnt) {
+ pr_debug("cpufreq: %d is idle, delta_refcnt: 0\n", cpu);
+ return 0;
+ }
+ rate_mhz = ((unsigned long)(delta_ccnt * REF_CLK_MHZ)) / delta_refcnt;
+
+ return (rate_mhz * KHZ); /* in KHz */
+}
+
+static unsigned int tegra194_get_speed(u32 cpu)
+{
+ return tegra194_get_speed_common(cpu, US_DELAY);
+}
+
+static unsigned int tegra194_fast_get_speed(u32 cpu)
+{
+ return tegra194_get_speed_common(cpu, US_DELAY_MIN);
+}
+
+static int tegra194_cpufreq_init(struct cpufreq_policy *policy)
+{
+ struct tegra194_cpufreq_data *data = cpufreq_get_driver_data();
+ int cl = get_cpu_cluster(policy->cpu);
+ u32 cpu;
+
+ if (cl >= data->num_clusters)
+ return -EINVAL;
+
+ policy->cur = tegra194_fast_get_speed(policy->cpu); /* boot freq */
+
+ /* set same policy for all cpus in a cluster */
+ for (cpu = (cl * 2); cpu < ((cl + 1) * 2); cpu++)
+ cpumask_set_cpu(cpu, policy->cpus);
+
+ policy->freq_table = data->tables[cl];
+ policy->cpuinfo.transition_latency = TEGRA_CPUFREQ_TRANSITION_LATENCY;
+
+ return 0;
+}
+
+static void set_cpu_ndiv(void *data)
+{
+ struct cpufreq_frequency_table *tbl = data;
+ u64 ndiv_val = (u64)tbl->driver_data;
+
+ asm volatile("msr s3_0_c15_c0_4, %0" : : "r" (ndiv_val));
+}
+
+static int tegra194_cpufreq_set_target(struct cpufreq_policy *policy,
+ unsigned int index)
+{
+ struct cpufreq_frequency_table *tbl = policy->freq_table + index;
+
+ /*
+ * Each core writes frequency in per core register. Then both cores
+ * in a cluster run at same frequency which is the maximum frequency
+ * request out of the values requested by both cores in that cluster.
+ */
+ on_each_cpu_mask(policy->cpus, set_cpu_ndiv, tbl, true);
+
+ return 0;
+}
+
+static struct cpufreq_driver tegra194_cpufreq_driver = {
+ .name = "tegra194",
+ .flags = CPUFREQ_STICKY | CPUFREQ_CONST_LOOPS |
+ CPUFREQ_NEED_INITIAL_FREQ_CHECK,
+ .verify = cpufreq_generic_frequency_table_verify,
+ .target_index = tegra194_cpufreq_set_target,
+ .get = tegra194_get_speed,
+ .init = tegra194_cpufreq_init,
+ .attr = cpufreq_generic_attr,
+};
+
+static void tegra194_cpufreq_free_resources(void)
+{
+ destroy_workqueue(read_counters_wq);
+}
+
+static struct cpufreq_frequency_table *
+init_freq_table(struct platform_device *pdev, struct tegra_bpmp *bpmp,
+ unsigned int cluster_id)
+{
+ struct cpufreq_frequency_table *freq_table;
+ struct mrq_cpu_ndiv_limits_response resp;
+ unsigned int num_freqs, ndiv, delta_ndiv;
+ struct mrq_cpu_ndiv_limits_request req;
+ struct tegra_bpmp_message msg;
+ u16 freq_table_step_size;
+ int err, index;
+
+ memset(&req, 0, sizeof(req));
+ req.cluster_id = cluster_id;
+
+ memset(&msg, 0, sizeof(msg));
+ msg.mrq = MRQ_CPU_NDIV_LIMITS;
+ msg.tx.data = &req;
+ msg.tx.size = sizeof(req);
+ msg.rx.data = &resp;
+ msg.rx.size = sizeof(resp);
+
+ err = tegra_bpmp_transfer(bpmp, &msg);
+ if (err)
+ return ERR_PTR(err);
+
+ /*
+ * Make sure frequency table step is a multiple of mdiv to match
+ * vhint table granularity.
+ */
+ freq_table_step_size = resp.mdiv *
+ DIV_ROUND_UP(CPUFREQ_TBL_STEP_HZ, resp.ref_clk_hz);
+
+ dev_dbg(&pdev->dev, "cluster %d: frequency table step size: %d\n",
+ cluster_id, freq_table_step_size);
+
+ delta_ndiv = resp.ndiv_max - resp.ndiv_min;
+
+ if (unlikely(delta_ndiv == 0))
+ num_freqs = 1;
+ else
+ /* We store both ndiv_min and ndiv_max hence the +1 */
+ num_freqs = delta_ndiv / freq_table_step_size + 1;
+
+ num_freqs += (delta_ndiv % freq_table_step_size) ? 1 : 0;
+
+ freq_table = devm_kcalloc(&pdev->dev, num_freqs + 1,
+ sizeof(*freq_table), GFP_KERNEL);
+ if (!freq_table)
+ return ERR_PTR(-ENOMEM);
+
+ for (index = 0, ndiv = resp.ndiv_min;
+ ndiv < resp.ndiv_max;
+ index++, ndiv += freq_table_step_size) {
+ freq_table[index].driver_data = ndiv;
+ freq_table[index].frequency = map_ndiv_to_freq(&resp, ndiv);
+ }
+
+ freq_table[index].driver_data = resp.ndiv_max;
+ freq_table[index++].frequency = map_ndiv_to_freq(&resp, resp.ndiv_max);
+ freq_table[index].frequency = CPUFREQ_TABLE_END;
+
+ return freq_table;
+}
+
+static int tegra194_cpufreq_probe(struct platform_device *pdev)
+{
+ struct tegra194_cpufreq_data *data;
+ struct tegra_bpmp *bpmp;
+ int err, i;
+
+ data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->num_clusters = MAX_CLUSTERS;
+ data->tables = devm_kcalloc(&pdev->dev, data->num_clusters,
+ sizeof(*data->tables), GFP_KERNEL);
+ if (!data->tables)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, data);
+
+ bpmp = tegra_bpmp_get(&pdev->dev);
+ if (IS_ERR(bpmp))
+ return PTR_ERR(bpmp);
+
+ read_counters_wq = alloc_workqueue("read_counters_wq", __WQ_LEGACY, 1);
+ if (!read_counters_wq) {
+ dev_err(&pdev->dev, "fail to create_workqueue\n");
+ err = -EINVAL;
+ goto put_bpmp;
+ }
+
+ for (i = 0; i < data->num_clusters; i++) {
+ data->tables[i] = init_freq_table(pdev, bpmp, i);
+ if (IS_ERR(data->tables[i])) {
+ err = PTR_ERR(data->tables[i]);
+ goto put_bpmp;
+ }
+ }
+
+ tegra_bpmp_put(bpmp);
+
+ tegra194_cpufreq_driver.driver_data = data;
+
+ err = cpufreq_register_driver(&tegra194_cpufreq_driver);
+ if (err)
+ goto put_bpmp;
+
+ return err;
+
+put_bpmp:
+ tegra_bpmp_put(bpmp);
+ tegra194_cpufreq_free_resources();
+ return err;
+}
+
+static int tegra194_cpufreq_remove(struct platform_device *pdev)
+{
+ cpufreq_unregister_driver(&tegra194_cpufreq_driver);
+ tegra194_cpufreq_free_resources();
+
+ return 0;
+}
+
+static const struct of_device_id tegra194_cpufreq_of_match[] = {
+ { .compatible = "nvidia,tegra194-ccplex", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tegra194_cpufreq_of_match);
+
+static struct platform_driver tegra194_ccplex_driver = {
+ .driver = {
+ .name = "tegra194-cpufreq",
+ .of_match_table = tegra194_cpufreq_of_match,
+ },
+ .probe = tegra194_cpufreq_probe,
+ .remove = tegra194_cpufreq_remove,
+};
+module_platform_driver(tegra194_ccplex_driver);
+
+MODULE_AUTHOR("Mikko Perttunen <[email protected]>");
+MODULE_AUTHOR("Sumit Gupta <[email protected]>");
+MODULE_DESCRIPTION("NVIDIA Tegra194 cpufreq driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4

2020-06-26 17:07:26

by Sumit Gupta

[permalink] [raw]
Subject: [TEGRA194_CPUFREQ PATCH v4 1/4] dt-bindings: arm: Add t194 ccplex compatible and bpmp property

To do frequency scaling on all CPUs within T194 CPU Complex, we need
to query BPMP for data on valid operating points. Document a compatible
string under 'cpus' node to represent the CPU Complex for binding drivers
like cpufreq which don't have their node or CPU Complex node to bind to.
Also, document a property to point to the BPMP device that can be queried
for all CPUs.

Signed-off-by: Sumit Gupta <[email protected]>
---
Documentation/devicetree/bindings/arm/cpus.yaml | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/cpus.yaml b/Documentation/devicetree/bindings/arm/cpus.yaml
index a018147..737b55e 100644
--- a/Documentation/devicetree/bindings/arm/cpus.yaml
+++ b/Documentation/devicetree/bindings/arm/cpus.yaml
@@ -162,6 +162,7 @@ properties:
- nvidia,tegra132-denver
- nvidia,tegra186-denver
- nvidia,tegra194-carmel
+ - nvidia,tegra194-ccplex
- qcom,krait
- qcom,kryo
- qcom,kryo260
@@ -255,6 +256,14 @@ properties:

where voltage is in V, frequency is in MHz.

+ nvidia,bpmp:
+ $ref: '/schemas/types.yaml#/definitions/phandle'
+ descrption: |
+ Specifies the bpmp node that needs to be queried to get
+ operating point data for all CPUs.
+
+ Optional for NVIDIA Tegra194 Carmel CPUs
+
power-domains:
$ref: '/schemas/types.yaml#/definitions/phandle-array'
description:
--
2.7.4

2020-06-26 17:07:31

by Sumit Gupta

[permalink] [raw]
Subject: [TEGRA194_CPUFREQ PATCH v4 4/4] arm64: defconfig: Enable CONFIG_ARM_TEGRA194_CPUFREQ

Enable Tegra194 CPU frequency scaling support by default.

Signed-off-by: Sumit Gupta <[email protected]>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index f9d378d..385bd35 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -91,6 +91,7 @@ CONFIG_ARM_QCOM_CPUFREQ_NVMEM=y
CONFIG_ARM_QCOM_CPUFREQ_HW=y
CONFIG_ARM_RASPBERRYPI_CPUFREQ=m
CONFIG_ARM_TEGRA186_CPUFREQ=y
+CONFIG_ARM_TEGRA194_CPUFREQ=y
CONFIG_QORIQ_CPUFREQ=y
CONFIG_ARM_SCPI_PROTOCOL=y
CONFIG_RASPBERRYPI_FIRMWARE=y
--
2.7.4

2020-06-26 17:17:48

by Michał Mirosław

[permalink] [raw]
Subject: Re: [TEGRA194_CPUFREQ PATCH v4 1/4] dt-bindings: arm: Add t194 ccplex compatible and bpmp property

On Fri, Jun 26, 2020 at 09:13:53PM +0530, Sumit Gupta wrote:
> To do frequency scaling on all CPUs within T194 CPU Complex, we need
> to query BPMP for data on valid operating points. Document a compatible
> string under 'cpus' node to represent the CPU Complex for binding drivers
> like cpufreq which don't have their node or CPU Complex node to bind to.
> Also, document a property to point to the BPMP device that can be queried
> for all CPUs.
>
> Signed-off-by: Sumit Gupta <[email protected]>
> ---
> Documentation/devicetree/bindings/arm/cpus.yaml | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/cpus.yaml b/Documentation/devicetree/bindings/arm/cpus.yaml
> index a018147..737b55e 100644
> --- a/Documentation/devicetree/bindings/arm/cpus.yaml
> +++ b/Documentation/devicetree/bindings/arm/cpus.yaml
> @@ -162,6 +162,7 @@ properties:
> - nvidia,tegra132-denver
> - nvidia,tegra186-denver
> - nvidia,tegra194-carmel
> + - nvidia,tegra194-ccplex
> - qcom,krait
> - qcom,kryo
> - qcom,kryo260
> @@ -255,6 +256,14 @@ properties:
>
> where voltage is in V, frequency is in MHz.
>
> + nvidia,bpmp:
> + $ref: '/schemas/types.yaml#/definitions/phandle'
> + descrption: |
> + Specifies the bpmp node that needs to be queried to get
> + operating point data for all CPUs.
> +
> + Optional for NVIDIA Tegra194 Carmel CPUs

Hi,

The code (next patches) suggests, that the property is relevant for Tegra194
only. If it is required for compatible=tegra194-ccplex, then maybe you can
mark it so (there is already similar constraint described in the file, just
before the example section).

Best Regards,
Micha??Miros?aw

2020-06-29 19:16:29

by Viresh Kumar

[permalink] [raw]
Subject: Re: [TEGRA194_CPUFREQ PATCH v4 3/4] cpufreq: Add Tegra194 cpufreq driver

On 26-06-20, 21:13, Sumit Gupta wrote:
> +static int tegra194_cpufreq_probe(struct platform_device *pdev)
> +{
> + struct tegra194_cpufreq_data *data;
> + struct tegra_bpmp *bpmp;
> + int err, i;
> +
> + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->num_clusters = MAX_CLUSTERS;
> + data->tables = devm_kcalloc(&pdev->dev, data->num_clusters,
> + sizeof(*data->tables), GFP_KERNEL);
> + if (!data->tables)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, data);
> +
> + bpmp = tegra_bpmp_get(&pdev->dev);
> + if (IS_ERR(bpmp))
> + return PTR_ERR(bpmp);
> +
> + read_counters_wq = alloc_workqueue("read_counters_wq", __WQ_LEGACY, 1);
> + if (!read_counters_wq) {
> + dev_err(&pdev->dev, "fail to create_workqueue\n");
> + err = -EINVAL;
> + goto put_bpmp;

This will call destroy_workqueue() eventually and it will crash your
kernel.

Apart from this, this stuff looks okay. Don't resend the patch just
yet (and if required, send only this patch using --in-reply-to flag
for git send email). Lets wait for an Ack from Rob for the first two
patches.

> + }
> +

--
viresh

2020-06-29 20:47:15

by Viresh Kumar

[permalink] [raw]
Subject: Re: [TEGRA194_CPUFREQ PATCH v4 4/4] arm64: defconfig: Enable CONFIG_ARM_TEGRA194_CPUFREQ

On 26-06-20, 21:13, Sumit Gupta wrote:
> Enable Tegra194 CPU frequency scaling support by default.
>
> Signed-off-by: Sumit Gupta <[email protected]>
> ---
> arch/arm64/configs/defconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index f9d378d..385bd35 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -91,6 +91,7 @@ CONFIG_ARM_QCOM_CPUFREQ_NVMEM=y
> CONFIG_ARM_QCOM_CPUFREQ_HW=y
> CONFIG_ARM_RASPBERRYPI_CPUFREQ=m
> CONFIG_ARM_TEGRA186_CPUFREQ=y
> +CONFIG_ARM_TEGRA194_CPUFREQ=y
> CONFIG_QORIQ_CPUFREQ=y
> CONFIG_ARM_SCPI_PROTOCOL=y
> CONFIG_RASPBERRYPI_FIRMWARE=y

Instead of this, maybe you can rather add a default y thing in the
Kconfig itself as this is a single platform driver.

--
viresh

2020-06-29 22:10:50

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [TEGRA194_CPUFREQ PATCH v4 1/4] dt-bindings: arm: Add t194 ccplex compatible and bpmp property

On Fri, 26 Jun 2020 21:13:53 +0530, Sumit Gupta wrote:
> To do frequency scaling on all CPUs within T194 CPU Complex, we need
> to query BPMP for data on valid operating points. Document a compatible
> string under 'cpus' node to represent the CPU Complex for binding drivers
> like cpufreq which don't have their node or CPU Complex node to bind to.
> Also, document a property to point to the BPMP device that can be queried
> for all CPUs.
>
> Signed-off-by: Sumit Gupta <[email protected]>
> ---
> Documentation/devicetree/bindings/arm/cpus.yaml | 9 +++++++++
> 1 file changed, 9 insertions(+)
>


My bot found errors running 'make dt_binding_check' on your patch:

/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/arm/cpus.yaml: properties:nvidia,bpmp: Additional properties are not allowed ('descrption' was unexpected)
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/arm/cpus.yaml: properties:nvidia,bpmp: 'descrption' is not one of ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'deprecated', 'description', 'else', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'then', 'type', 'typeSize', 'unevaluatedProperties', 'uniqueItems']
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/arm/cpus.yaml: properties:nvidia,bpmp: {'$ref': '/schemas/types.yaml#/definitions/phandle', 'descrption': 'Specifies the bpmp node that needs to be queried to get\noperating point data for all CPUs.\n\nOptional for NVIDIA Tegra194 Carmel CPUs\n'} is not valid under any of the given schemas (Possible causes of the failure):
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/arm/cpus.yaml: properties:nvidia,bpmp: 'description' is a required property

Documentation/devicetree/bindings/Makefile:20: recipe for target 'Documentation/devicetree/bindings/arm/cpus.example.dts' failed
make[1]: *** [Documentation/devicetree/bindings/arm/cpus.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
Makefile:1347: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2


See https://patchwork.ozlabs.org/patch/1317775

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.

2020-07-13 14:33:09

by Sumit Gupta

[permalink] [raw]
Subject: Re: [TEGRA194_CPUFREQ PATCH v4 3/4] cpufreq: Add Tegra194 cpufreq driver



>
> On 26-06-20, 21:13, Sumit Gupta wrote:
>> +static int tegra194_cpufreq_probe(struct platform_device *pdev)
>> +{
>> + struct tegra194_cpufreq_data *data;
>> + struct tegra_bpmp *bpmp;
>> + int err, i;
>> +
>> + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
>> + if (!data)
>> + return -ENOMEM;
>> +
>> + data->num_clusters = MAX_CLUSTERS;
>> + data->tables = devm_kcalloc(&pdev->dev, data->num_clusters,
>> + sizeof(*data->tables), GFP_KERNEL);
>> + if (!data->tables)
>> + return -ENOMEM;
>> +
>> + platform_set_drvdata(pdev, data);
>> +
>> + bpmp = tegra_bpmp_get(&pdev->dev);
>> + if (IS_ERR(bpmp))
>> + return PTR_ERR(bpmp);
>> +
>> + read_counters_wq = alloc_workqueue("read_counters_wq", __WQ_LEGACY, 1);
>> + if (!read_counters_wq) {
>> + dev_err(&pdev->dev, "fail to create_workqueue\n");
>> + err = -EINVAL;
>> + goto put_bpmp;
>
> This will call destroy_workqueue() eventually and it will crash your
> kernel.
>
> Apart from this, this stuff looks okay. Don't resend the patch just
> yet (and if required, send only this patch using --in-reply-to flag
> for git send email). Lets wait for an Ack from Rob for the first two
> patches.
>
Sorry for the delayed response as i was on PTO.
Thank you for the feedback.

Have posted a v5 based on v4 patch set.

>> + }
>> +
>
> --
> viresh
>