From: Aaron Kling <webgeek1234@gmail.com>
This makes mc and emc interconnect providers and allows for dynamic
memory clock scaling.
Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
---
drivers/memory/tegra/Kconfig | 1 +
drivers/memory/tegra/tegra210-emc-core.c | 274 ++++++++++++++++++++++++++++++-
drivers/memory/tegra/tegra210-emc.h | 23 +++
drivers/memory/tegra/tegra210.c | 81 +++++++++
4 files changed, 377 insertions(+), 2 deletions(-)
diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
index fc5a277918267ee8240f9fb9efeb80275db4790b..2d0be29afe2b9ebf9a0630ef7fb6fb43ff359499 100644
--- a/drivers/memory/tegra/Kconfig
+++ b/drivers/memory/tegra/Kconfig
@@ -55,6 +55,7 @@ config TEGRA210_EMC
tristate "NVIDIA Tegra210 External Memory Controller driver"
depends on ARCH_TEGRA_210_SOC || COMPILE_TEST
select TEGRA210_EMC_TABLE
+ select PM_OPP
help
This driver is for the External Memory Controller (EMC) found on
Tegra210 chips. The EMC controls the external DRAM on the board.
diff --git a/drivers/memory/tegra/tegra210-emc-core.c b/drivers/memory/tegra/tegra210-emc-core.c
index e96ca4157d48182574310f8caf72687bed7cc16a..f12e60b47fa87d629505cde57310d2bb68fc87f3 100644
--- a/drivers/memory/tegra/tegra210-emc-core.c
+++ b/drivers/memory/tegra/tegra210-emc-core.c
@@ -13,6 +13,7 @@
#include <linux/module.h>
#include <linux/of_reserved_mem.h>
#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
#include <linux/slab.h>
#include <linux/thermal.h>
#include <soc/tegra/fuse.h>
@@ -1569,6 +1570,79 @@ static int tegra210_emc_set_rate(struct device *dev,
return 0;
}
+static void tegra_emc_rate_requests_init(struct tegra210_emc *emc)
+{
+ unsigned int i;
+
+ for (i = 0; i < EMC_RATE_TYPE_MAX; i++) {
+ emc->requested_rate[i].min_rate = 0;
+ emc->requested_rate[i].max_rate = ULONG_MAX;
+ }
+}
+
+static int emc_request_rate(struct tegra210_emc *emc,
+ unsigned long new_min_rate,
+ unsigned long new_max_rate,
+ enum emc_rate_request_type type)
+{
+ struct emc_rate_request *req = emc->requested_rate;
+ unsigned long min_rate = 0, max_rate = ULONG_MAX;
+ unsigned int i;
+ int err;
+
+ /* select minimum and maximum rates among the requested rates */
+ for (i = 0; i < EMC_RATE_TYPE_MAX; i++, req++) {
+ if (i == type) {
+ min_rate = max(new_min_rate, min_rate);
+ max_rate = min(new_max_rate, max_rate);
+ } else {
+ min_rate = max(req->min_rate, min_rate);
+ max_rate = min(req->max_rate, max_rate);
+ }
+ }
+
+ if (min_rate > max_rate) {
+ dev_err_ratelimited(emc->dev, "%s: type %u: out of range: %lu %lu\n",
+ __func__, type, min_rate, max_rate);
+ return -ERANGE;
+ }
+
+ err = clk_set_rate(emc->clk, min_rate);
+ if (err)
+ return err;
+
+ emc->requested_rate[type].min_rate = new_min_rate;
+ emc->requested_rate[type].max_rate = new_max_rate;
+
+ return 0;
+}
+
+static int emc_set_min_rate(struct tegra210_emc *emc, unsigned long rate,
+ enum emc_rate_request_type type)
+{
+ struct emc_rate_request *req = &emc->requested_rate[type];
+ int ret;
+
+ mutex_lock(&emc->rate_lock);
+ ret = emc_request_rate(emc, rate, req->max_rate, type);
+ mutex_unlock(&emc->rate_lock);
+
+ return ret;
+}
+
+static int emc_set_max_rate(struct tegra210_emc *emc, unsigned long rate,
+ enum emc_rate_request_type type)
+{
+ struct emc_rate_request *req = &emc->requested_rate[type];
+ int ret;
+
+ mutex_lock(&emc->rate_lock);
+ ret = emc_request_rate(emc, req->min_rate, rate, type);
+ mutex_unlock(&emc->rate_lock);
+
+ return ret;
+}
+
/*
* debugfs interface
*
@@ -1641,7 +1715,7 @@ static int tegra210_emc_debug_min_rate_set(void *data, u64 rate)
if (!tegra210_emc_validate_rate(emc, rate))
return -EINVAL;
- err = clk_set_min_rate(emc->clk, rate);
+ err = emc_set_min_rate(emc, rate, EMC_RATE_DEBUG);
if (err < 0)
return err;
@@ -1671,7 +1745,7 @@ static int tegra210_emc_debug_max_rate_set(void *data, u64 rate)
if (!tegra210_emc_validate_rate(emc, rate))
return -EINVAL;
- err = clk_set_max_rate(emc->clk, rate);
+ err = emc_set_max_rate(emc, rate, EMC_RATE_DEBUG);
if (err < 0)
return err;
@@ -1758,6 +1832,193 @@ static void tegra210_emc_debugfs_init(struct tegra210_emc *emc)
&tegra210_emc_debug_temperature_fops);
}
+static inline struct tegra210_emc *
+to_tegra210_emc_provider(struct icc_provider *provider)
+{
+ return container_of(provider, struct tegra210_emc, icc_provider);
+}
+
+static struct icc_node_data *
+emc_of_icc_xlate_extended(const struct of_phandle_args *spec, void *data)
+{
+ struct icc_provider *provider = data;
+ struct icc_node_data *ndata;
+ struct icc_node *node;
+
+ /* External Memory is the only possible ICC route */
+ list_for_each_entry(node, &provider->nodes, node_list) {
+ if (node->id != TEGRA_ICC_EMEM)
+ continue;
+
+ ndata = kzalloc(sizeof(*ndata), GFP_KERNEL);
+ if (!ndata)
+ return ERR_PTR(-ENOMEM);
+
+ /*
+ * SRC and DST nodes should have matching TAG in order to have
+ * it set by default for a requested path.
+ */
+ ndata->tag = TEGRA_MC_ICC_TAG_ISO;
+ ndata->node = node;
+
+ return ndata;
+ }
+
+ return ERR_PTR(-EPROBE_DEFER);
+}
+
+static int emc_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+ struct tegra210_emc *emc = to_tegra210_emc_provider(dst->provider);
+ unsigned long long peak_bw = icc_units_to_bps(dst->peak_bw);
+ unsigned long long avg_bw = icc_units_to_bps(dst->avg_bw);
+ unsigned long long rate = max(avg_bw, peak_bw);
+ const unsigned int ddr = 2;
+ int err;
+
+ /*
+ * Tegra210 memory layout can be 1 channel at 64-bit or 2 channels
+ * at 32-bit each. Either way, the total bus width will always be
+ * 64-bit.
+ */
+ const unsigned int dram_data_bus_width_bytes = 64 / 8;
+
+ /*
+ * Tegra210 EMC runs on a clock rate of SDRAM bus. This means that
+ * EMC clock rate is twice smaller than the peak data rate because
+ * data is sampled on both EMC clock edges.
+ */
+ do_div(rate, ddr * dram_data_bus_width_bytes);
+ rate = min_t(u64, rate, U32_MAX);
+
+ err = emc_set_min_rate(emc, rate, EMC_RATE_ICC);
+ if (err)
+ return err;
+
+ return 0;
+}
+
+static int tegra_emc_icc_get_init_bw(struct icc_node *node, u32 *avg, u32 *peak)
+{
+ *avg = 0;
+ *peak = 0;
+
+ return 0;
+}
+
+static int tegra_emc_interconnect_init(struct tegra210_emc *emc)
+{
+ const struct tegra_mc_soc *soc = emc->mc->soc;
+ struct icc_node *node;
+ int err;
+
+ emc->icc_provider.dev = emc->dev;
+ emc->icc_provider.set = emc_icc_set;
+ emc->icc_provider.data = &emc->icc_provider;
+ emc->icc_provider.aggregate = soc->icc_ops->aggregate;
+ emc->icc_provider.xlate_extended = emc_of_icc_xlate_extended;
+ emc->icc_provider.get_bw = tegra_emc_icc_get_init_bw;
+
+ icc_provider_init(&emc->icc_provider);
+
+ /* create External Memory Controller node */
+ node = icc_node_create(TEGRA_ICC_EMC);
+ if (IS_ERR(node)) {
+ err = PTR_ERR(node);
+ goto err_msg;
+ }
+
+ node->name = "External Memory Controller";
+ icc_node_add(node, &emc->icc_provider);
+
+ /* link External Memory Controller to External Memory (DRAM) */
+ err = icc_link_create(node, TEGRA_ICC_EMEM);
+ if (err)
+ goto remove_nodes;
+
+ /* create External Memory node */
+ node = icc_node_create(TEGRA_ICC_EMEM);
+ if (IS_ERR(node)) {
+ err = PTR_ERR(node);
+ goto remove_nodes;
+ }
+
+ node->name = "External Memory (DRAM)";
+ icc_node_add(node, &emc->icc_provider);
+
+ err = icc_provider_register(&emc->icc_provider);
+ if (err)
+ goto remove_nodes;
+
+ return 0;
+
+remove_nodes:
+ icc_nodes_remove(&emc->icc_provider);
+err_msg:
+ dev_err(emc->dev, "failed to initialize ICC: %d\n", err);
+
+ return err;
+}
+
+static int tegra_emc_opp_table_init(struct tegra210_emc *emc)
+{
+ u32 hw_version = BIT(tegra_sku_info.soc_speedo_id);
+ struct dev_pm_opp *opp;
+ unsigned long rate;
+ int opp_token, err, max_opps, i;
+
+ err = dev_pm_opp_set_supported_hw(emc->dev, &hw_version, 1);
+ if (err < 0) {
+ dev_err(emc->dev, "failed to set OPP supported HW: %d\n", err);
+ return err;
+ }
+ opp_token = err;
+
+ err = dev_pm_opp_of_add_table(emc->dev);
+ if (err) {
+ if (err == -ENODEV)
+ dev_err(emc->dev, "OPP table not found, please update your device tree\n");
+ else
+ dev_err(emc->dev, "failed to add OPP table: %d\n", err);
+
+ goto put_hw_table;
+ }
+
+ max_opps = dev_pm_opp_get_opp_count(emc->dev);
+ if (max_opps <= 0) {
+ dev_err(emc->dev, "Failed to add OPPs\n");
+ goto remove_table;
+ }
+
+ if (emc->num_timings != max_opps) {
+ dev_err(emc->dev, "OPP table does not match emc table\n");
+ goto remove_table;
+ }
+
+ for (i = 0; i < emc->num_timings; i++) {
+ rate = emc->timings[i].rate * 1000;
+ opp = dev_pm_opp_find_freq_exact(emc->dev, rate, true);
+ if (IS_ERR(opp)) {
+ dev_err(emc->dev, "Rate %lu not found in OPP table\n", rate);
+ goto remove_table;
+ }
+
+ dev_pm_opp_put(opp);
+ }
+
+ dev_info_once(emc->dev, "OPP HW ver. 0x%x, current clock rate %lu MHz\n",
+ hw_version, clk_get_rate(emc->clk) / 1000000);
+
+ return 0;
+
+remove_table:
+ dev_pm_opp_of_remove_table(emc->dev);
+put_hw_table:
+ dev_pm_opp_put_supported_hw(opp_token);
+
+ return err;
+}
+
static void tegra210_emc_detect(struct tegra210_emc *emc)
{
u32 value;
@@ -1964,8 +2225,16 @@ static int tegra210_emc_probe(struct platform_device *pdev)
timer_setup(&emc->training, tegra210_emc_train, 0);
+ err = tegra_emc_opp_table_init(emc);
+ if (err)
+ return err;
+
+ tegra_emc_rate_requests_init(emc);
+
tegra210_emc_debugfs_init(emc);
+ tegra_emc_interconnect_init(emc);
+
cd = devm_thermal_of_cooling_device_register(emc->dev, np, "emc", emc,
&tegra210_emc_cd_ops);
if (IS_ERR(cd)) {
@@ -2050,6 +2319,7 @@ static struct platform_driver tegra210_emc_driver = {
.name = "tegra210-emc",
.of_match_table = tegra210_emc_of_match,
.pm = &tegra210_emc_pm_ops,
+ .sync_state = icc_sync_state,
},
.probe = tegra210_emc_probe,
.remove = tegra210_emc_remove,
diff --git a/drivers/memory/tegra/tegra210-emc.h b/drivers/memory/tegra/tegra210-emc.h
index 8988bcf1529072a7bdc93b185ebe0d51d82c1763..3c9142bfd5ae5c57bbc139e69e62c893b50ce40c 100644
--- a/drivers/memory/tegra/tegra210-emc.h
+++ b/drivers/memory/tegra/tegra210-emc.h
@@ -8,6 +8,7 @@
#include <linux/clk.h>
#include <linux/clk/tegra.h>
+#include <linux/interconnect-provider.h>
#include <linux/io.h>
#include <linux/platform_device.h>
@@ -784,6 +785,17 @@ enum {
#define TRIM_REGS_SIZE 138
#define BURST_REGS_SIZE 221
+enum emc_rate_request_type {
+ EMC_RATE_DEBUG,
+ EMC_RATE_ICC,
+ EMC_RATE_TYPE_MAX,
+};
+
+struct emc_rate_request {
+ unsigned long min_rate;
+ unsigned long max_rate;
+};
+
struct tegra210_emc_per_channel_regs {
u16 bank;
u16 offset;
@@ -932,6 +944,17 @@ struct tegra210_emc {
} debugfs;
struct tegra210_clk_emc_provider provider;
+
+ struct icc_provider icc_provider;
+
+ /*
+ * There are multiple sources in the EMC driver which could request
+ * a min/max clock rate, these rates are contained in this array.
+ */
+ struct emc_rate_request requested_rate[EMC_RATE_TYPE_MAX];
+
+ /* protect shared rate-change code path */
+ struct mutex rate_lock;
};
struct tegra210_emc_sequence {
diff --git a/drivers/memory/tegra/tegra210.c b/drivers/memory/tegra/tegra210.c
index 8ab6498dbe7d2f410d4eb262926c18b77edb0b3d..c5f079b60363f86b9b1382182e71bfcea9e19829 100644
--- a/drivers/memory/tegra/tegra210.c
+++ b/drivers/memory/tegra/tegra210.c
@@ -3,6 +3,9 @@
* Copyright (C) 2015 NVIDIA CORPORATION. All rights reserved.
*/
+#include <linux/of.h>
+#include <linux/device.h>
+
#include <dt-bindings/memory/tegra210-mc.h>
#include "mc.h"
@@ -1273,6 +1276,83 @@ static const struct tegra_mc_reset tegra210_mc_resets[] = {
TEGRA210_MC_RESET(TSECB, 0x970, 0x974, 13),
};
+static int tegra210_mc_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+ /* TODO: program PTSA */
+ return 0;
+}
+
+static int tegra210_mc_icc_aggregate(struct icc_node *node, u32 tag, u32 avg_bw,
+ u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
+{
+ /*
+ * ISO clients need to reserve extra bandwidth up-front because
+ * there could be high bandwidth pressure during initial filling
+ * of the client's FIFO buffers. Secondly, we need to take into
+ * account impurities of the memory subsystem.
+ */
+ if (tag & TEGRA_MC_ICC_TAG_ISO)
+ peak_bw = tegra_mc_scale_percents(peak_bw, 400);
+
+ *agg_avg += avg_bw;
+ *agg_peak = max(*agg_peak, peak_bw);
+
+ return 0;
+}
+
+static struct icc_node_data *
+tegra210_mc_of_icc_xlate_extended(const struct of_phandle_args *spec, void *data)
+{
+ struct tegra_mc *mc = icc_provider_to_tegra_mc(data);
+ const struct tegra_mc_client *client;
+ unsigned int i, idx = spec->args[0];
+ struct icc_node_data *ndata;
+ struct icc_node *node;
+
+ list_for_each_entry(node, &mc->provider.nodes, node_list) {
+ if (node->id != idx)
+ continue;
+
+ ndata = kzalloc(sizeof(*ndata), GFP_KERNEL);
+ if (!ndata)
+ return ERR_PTR(-ENOMEM);
+
+ client = &mc->soc->clients[idx];
+ ndata->node = node;
+
+ switch (client->swgroup) {
+ case TEGRA_SWGROUP_DC:
+ case TEGRA_SWGROUP_DCB:
+ case TEGRA_SWGROUP_PTC:
+ case TEGRA_SWGROUP_VI:
+ /* these clients are isochronous by default */
+ ndata->tag = TEGRA_MC_ICC_TAG_ISO;
+ break;
+
+ default:
+ ndata->tag = TEGRA_MC_ICC_TAG_DEFAULT;
+ break;
+ }
+
+ return ndata;
+ }
+
+ for (i = 0; i < mc->soc->num_clients; i++) {
+ if (mc->soc->clients[i].id == idx)
+ return ERR_PTR(-EPROBE_DEFER);
+ }
+
+ dev_err(mc->dev, "invalid ICC client ID %u\n", idx);
+
+ return ERR_PTR(-EINVAL);
+}
+
+static const struct tegra_mc_icc_ops tegra210_mc_icc_ops = {
+ .xlate_extended = tegra210_mc_of_icc_xlate_extended,
+ .aggregate = tegra210_mc_icc_aggregate,
+ .set = tegra210_mc_icc_set,
+};
+
const struct tegra_mc_soc tegra210_mc_soc = {
.clients = tegra210_mc_clients,
.num_clients = ARRAY_SIZE(tegra210_mc_clients),
@@ -1286,5 +1366,6 @@ const struct tegra_mc_soc tegra210_mc_soc = {
.reset_ops = &tegra_mc_reset_ops_common,
.resets = tegra210_mc_resets,
.num_resets = ARRAY_SIZE(tegra210_mc_resets),
+ .icc_ops = &tegra210_mc_icc_ops,
.ops = &tegra30_mc_ops,
};
--
2.50.1On Wed, Sep 03, 2025 at 02:50:11PM -0500, Aaron Kling wrote:
> This makes mc and emc interconnect providers and allows for dynamic
> memory clock scaling.
>
> Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
> ---
> drivers/memory/tegra/Kconfig | 1 +
> drivers/memory/tegra/tegra210-emc-core.c | 274 ++++++++++++++++++++++++++++++-
> drivers/memory/tegra/tegra210-emc.h | 23 +++
> drivers/memory/tegra/tegra210.c | 81 +++++++++
> 4 files changed, 377 insertions(+), 2 deletions(-)
Patch #3 was memory, patch #4 was soc, patch #5 is memory, so that
mixup pattern continues.
Please address the earlier feedback.
>
> diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
> index fc5a277918267ee8240f9fb9efeb80275db4790b..2d0be29afe2b9ebf9a0630ef7fb6fb43ff359499 100644
> --- a/drivers/memory/tegra/Kconfig
> +++ b/drivers/memory/tegra/Kconfig
> @@ -55,6 +55,7 @@ config TEGRA210_EMC
> tristate "NVIDIA Tegra210 External Memory Controller driver"
> depends on ARCH_TEGRA_210_SOC || COMPILE_TEST
> select TEGRA210_EMC_TABLE
> + select PM_OPP
> help
> This driver is for the External Memory Controller (EMC) found on
> Tegra210 chips. The EMC controls the external DRAM on the board.
> diff --git a/drivers/memory/tegra/tegra210-emc-core.c b/drivers/memory/tegra/tegra210-emc-core.c
> index e96ca4157d48182574310f8caf72687bed7cc16a..f12e60b47fa87d629505cde57310d2bb68fc87f3 100644
> --- a/drivers/memory/tegra/tegra210-emc-core.c
> +++ b/drivers/memory/tegra/tegra210-emc-core.c
> @@ -13,6 +13,7 @@
> #include <linux/module.h>
> #include <linux/of_reserved_mem.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_opp.h>
> #include <linux/slab.h>
> #include <linux/thermal.h>
> #include <soc/tegra/fuse.h>
> @@ -1569,6 +1570,79 @@ static int tegra210_emc_set_rate(struct device *dev,
> return 0;
> }
>
...
> @@ -1758,6 +1832,193 @@ static void tegra210_emc_debugfs_init(struct tegra210_emc *emc)
> &tegra210_emc_debug_temperature_fops);
> }
>
> +static inline struct tegra210_emc *
> +to_tegra210_emc_provider(struct icc_provider *provider)
> +{
> + return container_of(provider, struct tegra210_emc, icc_provider);
> +}
> +
> +static struct icc_node_data *
> +emc_of_icc_xlate_extended(const struct of_phandle_args *spec, void *data)
> +{
> + struct icc_provider *provider = data;
> + struct icc_node_data *ndata;
> + struct icc_node *node;
> +
> + /* External Memory is the only possible ICC route */
> + list_for_each_entry(node, &provider->nodes, node_list) {
> + if (node->id != TEGRA_ICC_EMEM)
> + continue;
> +
> + ndata = kzalloc(sizeof(*ndata), GFP_KERNEL);
> + if (!ndata)
> + return ERR_PTR(-ENOMEM);
> +
> + /*
> + * SRC and DST nodes should have matching TAG in order to have
> + * it set by default for a requested path.
> + */
> + ndata->tag = TEGRA_MC_ICC_TAG_ISO;
> + ndata->node = node;
> +
> + return ndata;
> + }
> +
> + return ERR_PTR(-EPROBE_DEFER);
> +}
> +
> +static int emc_icc_set(struct icc_node *src, struct icc_node *dst)
> +{
> + struct tegra210_emc *emc = to_tegra210_emc_provider(dst->provider);
> + unsigned long long peak_bw = icc_units_to_bps(dst->peak_bw);
> + unsigned long long avg_bw = icc_units_to_bps(dst->avg_bw);
> + unsigned long long rate = max(avg_bw, peak_bw);
> + const unsigned int ddr = 2;
Just use defines in top part for this.
> + int err;
> +
> + /*
> + * Tegra210 memory layout can be 1 channel at 64-bit or 2 channels
> + * at 32-bit each. Either way, the total bus width will always be
> + * 64-bit.
> + */
> + const unsigned int dram_data_bus_width_bytes = 64 / 8;
Same here.
> +
> + /*
> + * Tegra210 EMC runs on a clock rate of SDRAM bus. This means that
> + * EMC clock rate is twice smaller than the peak data rate because
> + * data is sampled on both EMC clock edges.
> + */
> + do_div(rate, ddr * dram_data_bus_width_bytes);
> + rate = min_t(u64, rate, U32_MAX);
> +
> + err = emc_set_min_rate(emc, rate, EMC_RATE_ICC);
> + if (err)
> + return err;
> +
> + return 0;
> +}
> +
> +static int tegra_emc_icc_get_init_bw(struct icc_node *node, u32 *avg, u32 *peak)
> +{
> + *avg = 0;
> + *peak = 0;
> +
> + return 0;
> +}
> +
> +static int tegra_emc_interconnect_init(struct tegra210_emc *emc)
> +{
> + const struct tegra_mc_soc *soc = emc->mc->soc;
> + struct icc_node *node;
> + int err;
> +
> + emc->icc_provider.dev = emc->dev;
> + emc->icc_provider.set = emc_icc_set;
> + emc->icc_provider.data = &emc->icc_provider;
> + emc->icc_provider.aggregate = soc->icc_ops->aggregate;
> + emc->icc_provider.xlate_extended = emc_of_icc_xlate_extended;
> + emc->icc_provider.get_bw = tegra_emc_icc_get_init_bw;
> +
> + icc_provider_init(&emc->icc_provider);
> +
> + /* create External Memory Controller node */
> + node = icc_node_create(TEGRA_ICC_EMC);
> + if (IS_ERR(node)) {
> + err = PTR_ERR(node);
> + goto err_msg;
> + }
> +
> + node->name = "External Memory Controller";
> + icc_node_add(node, &emc->icc_provider);
> +
> + /* link External Memory Controller to External Memory (DRAM) */
> + err = icc_link_create(node, TEGRA_ICC_EMEM);
> + if (err)
> + goto remove_nodes;
> +
> + /* create External Memory node */
> + node = icc_node_create(TEGRA_ICC_EMEM);
> + if (IS_ERR(node)) {
> + err = PTR_ERR(node);
> + goto remove_nodes;
> + }
> +
> + node->name = "External Memory (DRAM)";
> + icc_node_add(node, &emc->icc_provider);
> +
> + err = icc_provider_register(&emc->icc_provider);
> + if (err)
> + goto remove_nodes;
> +
> + return 0;
> +
> +remove_nodes:
> + icc_nodes_remove(&emc->icc_provider);
> +err_msg:
> + dev_err(emc->dev, "failed to initialize ICC: %d\n", err);
> +
> + return err;
> +}
> +
> +static int tegra_emc_opp_table_init(struct tegra210_emc *emc)
> +{
> + u32 hw_version = BIT(tegra_sku_info.soc_speedo_id);
> + struct dev_pm_opp *opp;
> + unsigned long rate;
> + int opp_token, err, max_opps, i;
> +
> + err = dev_pm_opp_set_supported_hw(emc->dev, &hw_version, 1);
> + if (err < 0) {
> + dev_err(emc->dev, "failed to set OPP supported HW: %d\n", err);
> + return err;
> + }
> + opp_token = err;
> +
> + err = dev_pm_opp_of_add_table(emc->dev);
> + if (err) {
> + if (err == -ENODEV)
> + dev_err(emc->dev, "OPP table not found, please update your device tree\n");
So this looks like the actual ABI break.
Best regards,
Krzysztof
On Thu, Sep 4, 2025 at 3:17 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Wed, Sep 03, 2025 at 02:50:11PM -0500, Aaron Kling wrote:
> > This makes mc and emc interconnect providers and allows for dynamic
> > memory clock scaling.
> >
> > Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
> > ---
> > drivers/memory/tegra/Kconfig | 1 +
> > drivers/memory/tegra/tegra210-emc-core.c | 274 ++++++++++++++++++++++++++++++-
> > drivers/memory/tegra/tegra210-emc.h | 23 +++
> > drivers/memory/tegra/tegra210.c | 81 +++++++++
> > 4 files changed, 377 insertions(+), 2 deletions(-)
>
> Patch #3 was memory, patch #4 was soc, patch #5 is memory, so that
> mixup pattern continues.
>
> Please address the earlier feedback.
Alright, I double check the docs and re-order this and my other series
for the next revisions. I had a misunderstanding how subsystems were
split up, which caused most but not all of the issues.
>
> >
> > diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
> > index fc5a277918267ee8240f9fb9efeb80275db4790b..2d0be29afe2b9ebf9a0630ef7fb6fb43ff359499 100644
> > --- a/drivers/memory/tegra/Kconfig
> > +++ b/drivers/memory/tegra/Kconfig
> > @@ -55,6 +55,7 @@ config TEGRA210_EMC
> > tristate "NVIDIA Tegra210 External Memory Controller driver"
> > depends on ARCH_TEGRA_210_SOC || COMPILE_TEST
> > select TEGRA210_EMC_TABLE
> > + select PM_OPP
> > help
> > This driver is for the External Memory Controller (EMC) found on
> > Tegra210 chips. The EMC controls the external DRAM on the board.
> > diff --git a/drivers/memory/tegra/tegra210-emc-core.c b/drivers/memory/tegra/tegra210-emc-core.c
> > index e96ca4157d48182574310f8caf72687bed7cc16a..f12e60b47fa87d629505cde57310d2bb68fc87f3 100644
> > --- a/drivers/memory/tegra/tegra210-emc-core.c
> > +++ b/drivers/memory/tegra/tegra210-emc-core.c
> > @@ -13,6 +13,7 @@
> > #include <linux/module.h>
> > #include <linux/of_reserved_mem.h>
> > #include <linux/platform_device.h>
> > +#include <linux/pm_opp.h>
> > #include <linux/slab.h>
> > #include <linux/thermal.h>
> > #include <soc/tegra/fuse.h>
> > @@ -1569,6 +1570,79 @@ static int tegra210_emc_set_rate(struct device *dev,
> > return 0;
> > }
> >
>
> ...
>
> > @@ -1758,6 +1832,193 @@ static void tegra210_emc_debugfs_init(struct tegra210_emc *emc)
> > &tegra210_emc_debug_temperature_fops);
> > }
> >
> > +static inline struct tegra210_emc *
> > +to_tegra210_emc_provider(struct icc_provider *provider)
> > +{
> > + return container_of(provider, struct tegra210_emc, icc_provider);
> > +}
> > +
> > +static struct icc_node_data *
> > +emc_of_icc_xlate_extended(const struct of_phandle_args *spec, void *data)
> > +{
> > + struct icc_provider *provider = data;
> > + struct icc_node_data *ndata;
> > + struct icc_node *node;
> > +
> > + /* External Memory is the only possible ICC route */
> > + list_for_each_entry(node, &provider->nodes, node_list) {
> > + if (node->id != TEGRA_ICC_EMEM)
> > + continue;
> > +
> > + ndata = kzalloc(sizeof(*ndata), GFP_KERNEL);
> > + if (!ndata)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + /*
> > + * SRC and DST nodes should have matching TAG in order to have
> > + * it set by default for a requested path.
> > + */
> > + ndata->tag = TEGRA_MC_ICC_TAG_ISO;
> > + ndata->node = node;
> > +
> > + return ndata;
> > + }
> > +
> > + return ERR_PTR(-EPROBE_DEFER);
> > +}
> > +
> > +static int emc_icc_set(struct icc_node *src, struct icc_node *dst)
> > +{
> > + struct tegra210_emc *emc = to_tegra210_emc_provider(dst->provider);
> > + unsigned long long peak_bw = icc_units_to_bps(dst->peak_bw);
> > + unsigned long long avg_bw = icc_units_to_bps(dst->avg_bw);
> > + unsigned long long rate = max(avg_bw, peak_bw);
> > + const unsigned int ddr = 2;
>
> Just use defines in top part for this.
>
> > + int err;
> > +
> > + /*
> > + * Tegra210 memory layout can be 1 channel at 64-bit or 2 channels
> > + * at 32-bit each. Either way, the total bus width will always be
> > + * 64-bit.
> > + */
> > + const unsigned int dram_data_bus_width_bytes = 64 / 8;
>
> Same here.
>
> > +
> > + /*
> > + * Tegra210 EMC runs on a clock rate of SDRAM bus. This means that
> > + * EMC clock rate is twice smaller than the peak data rate because
> > + * data is sampled on both EMC clock edges.
> > + */
> > + do_div(rate, ddr * dram_data_bus_width_bytes);
> > + rate = min_t(u64, rate, U32_MAX);
> > +
> > + err = emc_set_min_rate(emc, rate, EMC_RATE_ICC);
> > + if (err)
> > + return err;
> > +
> > + return 0;
> > +}
> > +
> > +static int tegra_emc_icc_get_init_bw(struct icc_node *node, u32 *avg, u32 *peak)
> > +{
> > + *avg = 0;
> > + *peak = 0;
> > +
> > + return 0;
> > +}
> > +
> > +static int tegra_emc_interconnect_init(struct tegra210_emc *emc)
> > +{
> > + const struct tegra_mc_soc *soc = emc->mc->soc;
> > + struct icc_node *node;
> > + int err;
> > +
> > + emc->icc_provider.dev = emc->dev;
> > + emc->icc_provider.set = emc_icc_set;
> > + emc->icc_provider.data = &emc->icc_provider;
> > + emc->icc_provider.aggregate = soc->icc_ops->aggregate;
> > + emc->icc_provider.xlate_extended = emc_of_icc_xlate_extended;
> > + emc->icc_provider.get_bw = tegra_emc_icc_get_init_bw;
> > +
> > + icc_provider_init(&emc->icc_provider);
> > +
> > + /* create External Memory Controller node */
> > + node = icc_node_create(TEGRA_ICC_EMC);
> > + if (IS_ERR(node)) {
> > + err = PTR_ERR(node);
> > + goto err_msg;
> > + }
> > +
> > + node->name = "External Memory Controller";
> > + icc_node_add(node, &emc->icc_provider);
> > +
> > + /* link External Memory Controller to External Memory (DRAM) */
> > + err = icc_link_create(node, TEGRA_ICC_EMEM);
> > + if (err)
> > + goto remove_nodes;
> > +
> > + /* create External Memory node */
> > + node = icc_node_create(TEGRA_ICC_EMEM);
> > + if (IS_ERR(node)) {
> > + err = PTR_ERR(node);
> > + goto remove_nodes;
> > + }
> > +
> > + node->name = "External Memory (DRAM)";
> > + icc_node_add(node, &emc->icc_provider);
> > +
> > + err = icc_provider_register(&emc->icc_provider);
> > + if (err)
> > + goto remove_nodes;
> > +
> > + return 0;
> > +
> > +remove_nodes:
> > + icc_nodes_remove(&emc->icc_provider);
> > +err_msg:
> > + dev_err(emc->dev, "failed to initialize ICC: %d\n", err);
> > +
> > + return err;
> > +}
> > +
> > +static int tegra_emc_opp_table_init(struct tegra210_emc *emc)
> > +{
> > + u32 hw_version = BIT(tegra_sku_info.soc_speedo_id);
> > + struct dev_pm_opp *opp;
> > + unsigned long rate;
> > + int opp_token, err, max_opps, i;
> > +
> > + err = dev_pm_opp_set_supported_hw(emc->dev, &hw_version, 1);
> > + if (err < 0) {
> > + dev_err(emc->dev, "failed to set OPP supported HW: %d\n", err);
> > + return err;
> > + }
> > + opp_token = err;
> > +
> > + err = dev_pm_opp_of_add_table(emc->dev);
> > + if (err) {
> > + if (err == -ENODEV)
> > + dev_err(emc->dev, "OPP table not found, please update your device tree\n");
>
> So this looks like the actual ABI break.
Okay, so let's discuss this. For reference, I based this patch off the
tegra124 change [0], which also caused an abi break. I know past
changes don't justify current mistakes, but this is the context. This
series adds all new required dt properties to the arch common dtsi, so
any newly compiled dtb will work. Any old dtb with a new kernel would
fail to probe, however. I think it would be safe to just skip the
interconnect init if the opp table init returns ENODEV, then let probe
succeed, but I would have to verify that. Do I need to do that and
drop the new requires from the binding?
Aaron
[0] https://github.com/torvalds/linux/commit/380def2d4cf257663de42618e57134afeded32dd#diff-3ff603a1ea7654928390eb213cea0424b6a12251bccbb5fd3b9720402a3c076aR1435
On 04/09/2025 19:28, Aaron Kling wrote:
>>> +
>>> +static int tegra_emc_opp_table_init(struct tegra210_emc *emc)
>>> +{
>>> + u32 hw_version = BIT(tegra_sku_info.soc_speedo_id);
>>> + struct dev_pm_opp *opp;
>>> + unsigned long rate;
>>> + int opp_token, err, max_opps, i;
>>> +
>>> + err = dev_pm_opp_set_supported_hw(emc->dev, &hw_version, 1);
>>> + if (err < 0) {
>>> + dev_err(emc->dev, "failed to set OPP supported HW: %d\n", err);
>>> + return err;
>>> + }
>>> + opp_token = err;
>>> +
>>> + err = dev_pm_opp_of_add_table(emc->dev);
>>> + if (err) {
>>> + if (err == -ENODEV)
>>> + dev_err(emc->dev, "OPP table not found, please update your device tree\n");
>>
>> So this looks like the actual ABI break.
>
> Okay, so let's discuss this. For reference, I based this patch off the
> tegra124 change [0], which also caused an abi break. I know past
That was almost 5 years ago and we also got stricter what we require in
the commit msg. It's also documented in writing bindings.
> changes don't justify current mistakes, but this is the context. This
> series adds all new required dt properties to the arch common dtsi, so
> any newly compiled dtb will work. Any old dtb with a new kernel would
> fail to probe, however. I think it would be safe to just skip the
That's the ABI break.
> interconnect init if the opp table init returns ENODEV, then let probe
> succeed, but I would have to verify that. Do I need to do that and
> drop the new requires from the binding?
The best would be yes, make it optional in the binding as well.
Best regards,
Krzysztof
© 2016 - 2025 Red Hat, Inc.