| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250904-aloof-cow-of-speed-ad5fe5@kuoka>
Date: Thu, 4 Sep 2025 10:17:04 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Aaron Kling <webgeek1234@...il.com>
Cc: Rob Herring <robh@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Thierry Reding <thierry.reding@...il.com>, Jonathan Hunter <jonathanh@...dia.com>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, MyungJoo Ham <myungjoo.ham@...sung.com>,
Kyungmin Park <kyungmin.park@...sung.com>, Chanwoo Choi <cw00.choi@...sung.com>,
Dmitry Osipenko <digetx@...il.com>, linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
linux-tegra@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH v2 5/8] memory: tegra210: Support interconnect framework
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@...il.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
Powered by blists - more mailing lists