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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <03a09ff5-fe2d-3ce2-a93b-4e44fd030ffb@gmail.com>
Date:   Mon, 20 Dec 2021 16:44:42 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
        Thierry Reding <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Svyatoslav Ryhel <clamor95@...il.com>
Cc:     linux-kernel@...r.kernel.org, linux-tegra@...r.kernel.org
Subject: Re: [PATCH v1] memory: tegra30-emc: Print additional memory info

20.12.2021 14:03, Krzysztof Kozlowski пишет:
> On 18/12/2021 00:47, Dmitry Osipenko wrote:
>> Print out memory type and LPDDR2 configuration on Tegra30, making it
>> similar to the memory info printed by the Tegra20 memory driver. This
>> info is useful for debugging purposes.
>>
>> Tested-by: Svyatoslav Ryhel <clamor95@...il.com> # T30 ASUS TF201 LPDDR2
>> Signed-off-by: Dmitry Osipenko <digetx@...il.com>
>> ---
>>  drivers/memory/tegra/Kconfig       |   1 +
>>  drivers/memory/tegra/tegra30-emc.c | 131 ++++++++++++++++++++++++++---
>>  2 files changed, 122 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
>> index 7951764b4efe..3fe83d7c2bf8 100644
>> --- a/drivers/memory/tegra/Kconfig
>> +++ b/drivers/memory/tegra/Kconfig
>> @@ -28,6 +28,7 @@ config TEGRA30_EMC
>>  	default y
>>  	depends on ARCH_TEGRA_3x_SOC || COMPILE_TEST
>>  	select PM_OPP
>> +	select DDR
>>  	help
>>  	  This driver is for the External Memory Controller (EMC) found on
>>  	  Tegra30 chips. The EMC controls the external DRAM on the board.
>> diff --git a/drivers/memory/tegra/tegra30-emc.c b/drivers/memory/tegra/tegra30-emc.c
>> index 80f98d717e13..4c0432704f46 100644
>> --- a/drivers/memory/tegra/tegra30-emc.c
>> +++ b/drivers/memory/tegra/tegra30-emc.c
>> @@ -9,6 +9,7 @@
>>   * Copyright (C) 2019 GRATE-DRIVER project
>>   */
>>  
>> +#include <linux/bitfield.h>
>>  #include <linux/clk.h>
>>  #include <linux/clk/tegra.h>
>>  #include <linux/debugfs.h>
>> @@ -31,11 +32,15 @@
>>  #include <soc/tegra/common.h>
>>  #include <soc/tegra/fuse.h>
>>  
>> +#include "../jedec_ddr.h"
>> +#include "../of_memory.h"
>> +
>>  #include "mc.h"
>>  
>>  #define EMC_INTSTATUS				0x000
>>  #define EMC_INTMASK				0x004
>>  #define EMC_DBG					0x008
>> +#define EMC_ADR_CFG				0x010
>>  #define EMC_CFG					0x00c
>>  #define EMC_REFCTRL				0x020
>>  #define EMC_TIMING_CONTROL			0x028
>> @@ -81,6 +86,7 @@
>>  #define EMC_EMRS				0x0d0
>>  #define EMC_SELF_REF				0x0e0
>>  #define EMC_MRW					0x0e8
>> +#define EMC_MRR					0x0ec
>>  #define EMC_XM2DQSPADCTRL3			0x0f8
>>  #define EMC_FBIO_SPARE				0x100
>>  #define EMC_FBIO_CFG5				0x104
>> @@ -208,6 +214,13 @@
>>  
>>  #define EMC_REFRESH_OVERFLOW_INT		BIT(3)
>>  #define EMC_CLKCHANGE_COMPLETE_INT		BIT(4)
>> +#define EMC_MRR_DIVLD_INT			BIT(5)
>> +
>> +#define EMC_MRR_DEV_SELECTN			GENMASK(31, 30)
>> +#define EMC_MRR_MRR_MA				GENMASK(23, 16)
>> +#define EMC_MRR_MRR_DATA			GENMASK(15, 0)
>> +
>> +#define EMC_ADR_CFG_EMEM_NUMDEV			BIT(0)
>>  
>>  enum emc_dram_type {
>>  	DRAM_TYPE_DDR3,
>> @@ -378,6 +391,8 @@ struct tegra_emc {
>>  
>>  	/* protect shared rate-change code path */
>>  	struct mutex rate_lock;
>> +
>> +	bool mrr_error;
>>  };
>>  
>>  static int emc_seq_update_timing(struct tegra_emc *emc)
>> @@ -1008,12 +1023,18 @@ static int emc_load_timings_from_dt(struct tegra_emc *emc,
>>  	return 0;
>>  }
>>  
>> -static struct device_node *emc_find_node_by_ram_code(struct device *dev)
>> +static struct device_node *emc_find_node_by_ram_code(struct tegra_emc *emc)
>>  {
>> +	struct device *dev = emc->dev;
>>  	struct device_node *np;
>>  	u32 value, ram_code;
>>  	int err;
>>  
>> +	if (emc->mrr_error) {
>> +		dev_warn(dev, "memory timings skipped due to MRR error\n");
>> +		return NULL;
>> +	}
>> +
>>  	if (of_get_child_count(dev->of_node) == 0) {
>>  		dev_info_once(dev, "device-tree doesn't have memory timings\n");
>>  		return NULL;
>> @@ -1035,11 +1056,73 @@ static struct device_node *emc_find_node_by_ram_code(struct device *dev)
>>  	return NULL;
>>  }
>>  
>> +static int emc_read_lpddr_mode_register(struct tegra_emc *emc,
>> +					unsigned int emem_dev,
>> +					unsigned int register_addr,
>> +					unsigned int *register_data)
>> +{
>> +	u32 memory_dev = emem_dev + 1;
>> +	u32 val, mr_mask = 0xff;
>> +	int err;
>> +
>> +	/* clear data-valid interrupt status */
>> +	writel_relaxed(EMC_MRR_DIVLD_INT, emc->regs + EMC_INTSTATUS);
>> +
>> +	/* issue mode register read request */
>> +	val  = FIELD_PREP(EMC_MRR_DEV_SELECTN, memory_dev);
>> +	val |= FIELD_PREP(EMC_MRR_MRR_MA, register_addr);
>> +
>> +	writel_relaxed(val, emc->regs + EMC_MRR);
>> +
>> +	/* wait for the LPDDR2 data-valid interrupt */
>> +	err = readl_relaxed_poll_timeout_atomic(emc->regs + EMC_INTSTATUS, val,
>> +						val & EMC_MRR_DIVLD_INT,
>> +						1, 100);
>> +	if (err) {
>> +		dev_err(emc->dev, "mode register %u read failed: %d\n",
>> +			register_addr, err);
>> +		emc->mrr_error = true;
>> +		return err;
>> +	}
>> +
>> +	/* read out mode register data */
>> +	val = readl_relaxed(emc->regs + EMC_MRR);
>> +	*register_data = FIELD_GET(EMC_MRR_MRR_DATA, val) & mr_mask;
>> +
>> +	return 0;
>> +}
>> +
>> +static void emc_read_lpddr_sdram_info(struct tegra_emc *emc,
>> +				      unsigned int emem_dev)
>> +{
>> +	union lpddr2_basic_config4 basic_conf4;
>> +	unsigned int manufacturer_id;
>> +	unsigned int revision_id1;
>> +	unsigned int revision_id2;
>> +
>> +	/* these registers are standard for all LPDDR JEDEC memory chips */
>> +	emc_read_lpddr_mode_register(emc, emem_dev, 5, &manufacturer_id);
>> +	emc_read_lpddr_mode_register(emc, emem_dev, 6, &revision_id1);
>> +	emc_read_lpddr_mode_register(emc, emem_dev, 7, &revision_id2);
>> +	emc_read_lpddr_mode_register(emc, emem_dev, 8, &basic_conf4.value);
>> +
>> +	dev_info(emc->dev, "SDRAM[dev%u]: manufacturer: 0x%x (%s) rev1: 0x%x rev2: 0x%x prefetch: S%u density: %uMbit iowidth: %ubit\n",
>> +		 emem_dev, manufacturer_id,
>> +		 lpddr2_jedec_manufacturer(manufacturer_id),
>> +		 revision_id1, revision_id2,
>> +		 4 >> basic_conf4.arch_type,
>> +		 64 << basic_conf4.density,
>> +		 32 >> basic_conf4.io_width);
>> +}
>> +
> 
> Quickly looking, these two functions are exactly the same as ones in
> tegra20-emc.c
> . Later you might come up with another set for other SoCs, so it looks
> it is worth to share these.

Should be too much trouble for not much gain, IMO. How many bytes will
be shared in the end? There is no much code here, we may lose more than
win. All these Tegra EMC drivers can be compiled as a loadable modules,
that's what distro kernels usually do. There are no plans for other SoCs
for today.

I don't see how that sharing could be done easily and nicely. Please
tell if you see.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ