Re: [PATCH] memory: tegra: Deduplicate rate request management code

[Krzysztof Kozlowski <krzk@kernel.org>]

On 06/02/2026 03:54, Mikko Perttunen wrote:
> As is, the EMC drivers for each 32-bit platform contain almost
> identical duplicated code for aggregating rate requests. Move this
> code out to a shared tegra-emc-common file to reduce duplication.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
> This patch is on top of 'memory: tegra: Add Tegra114 EMC driver'

If that patch is not merged, then this should be reversed. First you
remove duplication then you add smaller new patch. Not vice versa, where
you add duplicated code just to remove it.

> ---
>  drivers/memory/tegra/Kconfig            |   7 ++
>  drivers/memory/tegra/Makefile           |   1 +
>  drivers/memory/tegra/tegra-emc-common.c |  96 ++++++++++++++++++++++++++++
>  drivers/memory/tegra/tegra-emc-common.h |  38 +++++++++++
>  drivers/memory/tegra/tegra114-emc.c     | 107 ++-----------------------------
>  drivers/memory/tegra/tegra124-emc.c     | 107 ++-----------------------------
>  drivers/memory/tegra/tegra20-emc.c      | 110 ++------------------------------
>  drivers/memory/tegra/tegra30-emc.c      | 107 ++-----------------------------
>  8 files changed, 167 insertions(+), 406 deletions(-)
> 
> diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
> index 11e7cc357d39..aeda7f104d34 100644
> --- a/drivers/memory/tegra/Kconfig
> +++ b/drivers/memory/tegra/Kconfig
> @@ -17,6 +17,7 @@ config TEGRA20_EMC
>  	select DEVFREQ_GOV_SIMPLE_ONDEMAND
>  	select PM_DEVFREQ
>  	select DDR
> +	select TEGRA_EMC_COMMON
>  	help
>  	  This driver is for the External Memory Controller (EMC) found on
>  	  Tegra20 chips. The EMC controls the external DRAM on the board.
> @@ -29,6 +30,7 @@ config TEGRA30_EMC
>  	depends on ARCH_TEGRA_3x_SOC || COMPILE_TEST
>  	select PM_OPP
>  	select DDR
> +	select TEGRA_EMC_COMMON
>  	help
>  	  This driver is for the External Memory Controller (EMC) found on
>  	  Tegra30 chips. The EMC controls the external DRAM on the board.
> @@ -41,6 +43,7 @@ config TEGRA114_EMC
>  	depends on ARCH_TEGRA_114_SOC || COMPILE_TEST
>  	select TEGRA124_CLK_EMC if ARCH_TEGRA
>  	select PM_OPP
> +	select TEGRA_EMC_COMMON
>  	help
>  	  This driver is for the External Memory Controller (EMC) found on
>  	  Tegra114 chips. The EMC controls the external DRAM on the board.
> @@ -53,6 +56,7 @@ config TEGRA124_EMC
>  	depends on ARCH_TEGRA_124_SOC || COMPILE_TEST
>  	select TEGRA124_CLK_EMC if ARCH_TEGRA
>  	select PM_OPP
> +	select TEGRA_EMC_COMMON
>  	help
>  	  This driver is for the External Memory Controller (EMC) found on
>  	  Tegra124 chips. The EMC controls the external DRAM on the board.
> @@ -73,4 +77,7 @@ config TEGRA210_EMC
>  	  This driver is required to change memory timings / clock rate for
>  	  external memory.
>  
> +config TEGRA_EMC_COMMON
> +	tristate
> +
>  endif
> diff --git a/drivers/memory/tegra/Makefile b/drivers/memory/tegra/Makefile
> index 6b9156de4b66..28f22c957a34 100644
> --- a/drivers/memory/tegra/Makefile
> +++ b/drivers/memory/tegra/Makefile
> @@ -14,6 +14,7 @@ tegra-mc-$(CONFIG_ARCH_TEGRA_264_SOC) += tegra186.o tegra264.o
>  
>  obj-$(CONFIG_TEGRA_MC) += tegra-mc.o
>  
> +obj-$(CONFIG_TEGRA_EMC_COMMON) += tegra-emc-common.o
>  obj-$(CONFIG_TEGRA20_EMC)  += tegra20-emc.o
>  obj-$(CONFIG_TEGRA30_EMC)  += tegra30-emc.o
>  obj-$(CONFIG_TEGRA114_EMC) += tegra114-emc.o
> diff --git a/drivers/memory/tegra/tegra-emc-common.c b/drivers/memory/tegra/tegra-emc-common.c
> new file mode 100644
> index 000000000000..9292472a5890
> --- /dev/null
> +++ b/drivers/memory/tegra/tegra-emc-common.c
> @@ -0,0 +1,96 @@
> +// SPDX-License-Identifier: GPL-2.0


... and that's why we ask for consistent license. Code in tegra30-emc
has difference license, so I assume here you copied the one matching
license above. Explain in the commit msg which code you copied or on
which existing code you based this.

> +
> +#include <linux/device.h>
> +#include <linux/mutex.h>
> +#include <linux/pm_opp.h>
> +
> +#include "tegra-emc-common.h"
> +

All exported functions need kerneldoc.

> +void tegra_emc_rate_requests_init(struct tegra_emc_rate_requests *reqs,
> +				  struct device *dev)
> +{
> +	unsigned int i;
> +
> +	mutex_init(&reqs->rate_lock);
> +	reqs->dev = dev;
> +
> +	for (i = 0; i < TEGRA_EMC_RATE_TYPE_MAX; i++) {
> +		reqs->requested_rate[i].min_rate = 0;
> +		reqs->requested_rate[i].max_rate = ULONG_MAX;
> +	}
> +}
> +EXPORT_SYMBOL_GPL(tegra_emc_rate_requests_init);
> +

Best regards,
Krzysztof