* [PATCH v1] memory: tegra30-emc: Print additional memory info
@ 2021-12-17 23:47 Dmitry Osipenko
2021-12-20 11:03 ` Krzysztof Kozlowski
2021-12-20 15:27 ` Dmitry Osipenko
0 siblings, 2 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2021-12-17 23:47 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter, Krzysztof Kozlowski, Svyatoslav Ryhel
Cc: linux-kernel, linux-tegra
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@gmail.com> # T30 ASUS TF201 LPDDR2
Signed-off-by: Dmitry Osipenko <digetx@gmail.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);
+}
+
static int emc_setup_hw(struct tegra_emc *emc)
{
+ u32 fbio_cfg5, emc_cfg, emc_dbg, emc_adr_cfg;
u32 intmask = EMC_REFRESH_OVERFLOW_INT;
- u32 fbio_cfg5, emc_cfg, emc_dbg;
+ static bool print_sdram_info_once;
enum emc_dram_type dram_type;
+ const char *dram_type_str;
+ unsigned int emem_numdev;
fbio_cfg5 = readl_relaxed(emc->regs + EMC_FBIO_CFG5);
dram_type = fbio_cfg5 & EMC_FBIO_CFG5_DRAM_TYPE_MASK;
@@ -1076,6 +1159,34 @@ static int emc_setup_hw(struct tegra_emc *emc)
emc_dbg &= ~EMC_DBG_FORCE_UPDATE;
writel_relaxed(emc_dbg, emc->regs + EMC_DBG);
+ switch (dram_type) {
+ case DRAM_TYPE_DDR1:
+ dram_type_str = "DDR1";
+ break;
+ case DRAM_TYPE_LPDDR2:
+ dram_type_str = "LPDDR2";
+ break;
+ case DRAM_TYPE_DDR2:
+ dram_type_str = "DDR2";
+ break;
+ case DRAM_TYPE_DDR3:
+ dram_type_str = "DDR3";
+ break;
+ }
+
+ emc_adr_cfg = readl_relaxed(emc->regs + EMC_ADR_CFG);
+ emem_numdev = FIELD_GET(EMC_ADR_CFG_EMEM_NUMDEV, emc_adr_cfg) + 1;
+
+ dev_info_once(emc->dev, "%u %s %s attached\n", emem_numdev,
+ dram_type_str, emem_numdev == 2 ? "devices" : "device");
+
+ if (dram_type == DRAM_TYPE_LPDDR2 && !print_sdram_info_once) {
+ while (emem_numdev--)
+ emc_read_lpddr_sdram_info(emc, emem_numdev);
+
+ print_sdram_info_once = true;
+ }
+
return 0;
}
@@ -1538,14 +1649,6 @@ static int tegra_emc_probe(struct platform_device *pdev)
emc->clk_nb.notifier_call = emc_clk_change_notify;
emc->dev = &pdev->dev;
- np = emc_find_node_by_ram_code(&pdev->dev);
- if (np) {
- err = emc_load_timings_from_dt(emc, np);
- of_node_put(np);
- if (err)
- return err;
- }
-
emc->regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(emc->regs))
return PTR_ERR(emc->regs);
@@ -1554,6 +1657,14 @@ static int tegra_emc_probe(struct platform_device *pdev)
if (err)
return err;
+ np = emc_find_node_by_ram_code(emc);
+ if (np) {
+ err = emc_load_timings_from_dt(emc, np);
+ of_node_put(np);
+ if (err)
+ return err;
+ }
+
err = platform_get_irq(pdev, 0);
if (err < 0)
return err;
--
2.33.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1] memory: tegra30-emc: Print additional memory info
2021-12-17 23:47 [PATCH v1] memory: tegra30-emc: Print additional memory info Dmitry Osipenko
@ 2021-12-20 11:03 ` Krzysztof Kozlowski
2021-12-20 13:44 ` Dmitry Osipenko
2021-12-20 15:27 ` Dmitry Osipenko
1 sibling, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2021-12-20 11:03 UTC (permalink / raw)
To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter, Svyatoslav Ryhel
Cc: linux-kernel, linux-tegra
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@gmail.com> # T30 ASUS TF201 LPDDR2
> Signed-off-by: Dmitry Osipenko <digetx@gmail.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.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] memory: tegra30-emc: Print additional memory info
2021-12-20 11:03 ` Krzysztof Kozlowski
@ 2021-12-20 13:44 ` Dmitry Osipenko
2021-12-21 8:14 ` Krzysztof Kozlowski
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Osipenko @ 2021-12-20 13:44 UTC (permalink / raw)
To: Krzysztof Kozlowski, Thierry Reding, Jonathan Hunter, Svyatoslav Ryhel
Cc: linux-kernel, linux-tegra
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@gmail.com> # T30 ASUS TF201 LPDDR2
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] memory: tegra30-emc: Print additional memory info
2021-12-17 23:47 [PATCH v1] memory: tegra30-emc: Print additional memory info Dmitry Osipenko
2021-12-20 11:03 ` Krzysztof Kozlowski
@ 2021-12-20 15:27 ` Dmitry Osipenko
1 sibling, 0 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2021-12-20 15:27 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter, Krzysztof Kozlowski, Svyatoslav Ryhel
Cc: linux-kernel, linux-tegra
18.12.2021 02:47, Dmitry Osipenko пишет:
> +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;
I just noticed in the datasheet that memory_dev needs to be inverted,
otherwise we read dev0 instead of dev1 and vice versa. This also needs
to be corrected in the T20 EMC driver. It's not really a problem since
both memory chips usually are equal, nevertheless I'll fix it in v2
after we will decide on whether this code needs to be shared or not.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] memory: tegra30-emc: Print additional memory info
2021-12-20 13:44 ` Dmitry Osipenko
@ 2021-12-21 8:14 ` Krzysztof Kozlowski
2021-12-21 15:34 ` Dmitry Osipenko
0 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2021-12-21 8:14 UTC (permalink / raw)
To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter, Svyatoslav Ryhel
Cc: linux-kernel, linux-tegra
On 20/12/2021 14:44, Dmitry Osipenko wrote:
> 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@gmail.com> # T30 ASUS TF201 LPDDR2
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.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.
It's not about the bytes but source code lines to maintain and fix (if
there is something to fix). But if you don't plan to make a third copy
of it, it is okay.
>
> I don't see how that sharing could be done easily and nicely. Please
> tell if you see.
Since it is not about duplicated object code, but code for review, it is
pretty straightforward:
1. Create tegra-emc-common.[ch]
2. In Makefile:
+tegra20_emc-y += tegra20-emc.o tegra-emc-common.o
+obj-$(CONFIG_TEGRA20_EMC) += tegra20_emc.o
+
+tegra30_emc-y += tegra30-emc.o tegra-emc-common.o
+obj-$(CONFIG_TEGRA30_EMC) += tegra30_emc.o
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] memory: tegra30-emc: Print additional memory info
2021-12-21 8:14 ` Krzysztof Kozlowski
@ 2021-12-21 15:34 ` Dmitry Osipenko
0 siblings, 0 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2021-12-21 15:34 UTC (permalink / raw)
To: Krzysztof Kozlowski, Thierry Reding, Jonathan Hunter, Svyatoslav Ryhel
Cc: linux-kernel, linux-tegra
21.12.2021 11:14, Krzysztof Kozlowski пишет:
>>>> +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.
> It's not about the bytes but source code lines to maintain and fix (if
> there is something to fix). But if you don't plan to make a third copy
> of it, it is okay.
Only Tegra114 SoC potentially supports LPDDR2, later SoCs dropped LPDDR2
support. We don't even have memory driver for T114 at all in the today's
mainline. I also doubt that there were any consumer T114 devices sold
with LPDDR2. Hence we shouldn't have a need for the third copy anytime
soon, likely ever.
>> I don't see how that sharing could be done easily and nicely. Please
>> tell if you see.
> Since it is not about duplicated object code, but code for review, it is
> pretty straightforward:
>
> 1. Create tegra-emc-common.[ch]
> 2. In Makefile:
>
> +tegra20_emc-y += tegra20-emc.o tegra-emc-common.o
>
> +obj-$(CONFIG_TEGRA20_EMC) += tegra20_emc.o
>
> +
>
> +tegra30_emc-y += tegra30-emc.o tegra-emc-common.o
>
> +obj-$(CONFIG_TEGRA30_EMC) += tegra30_emc.o
>
The problem that struct tegra_emc isn't shareable and this common code
should introduce messiness to the Tegra EMC drivers. I'd prefer not to
share anything for now and get back to this option with sharing later
on, if will be another good reason.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-12-21 15:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-17 23:47 [PATCH v1] memory: tegra30-emc: Print additional memory info Dmitry Osipenko
2021-12-20 11:03 ` Krzysztof Kozlowski
2021-12-20 13:44 ` Dmitry Osipenko
2021-12-21 8:14 ` Krzysztof Kozlowski
2021-12-21 15:34 ` Dmitry Osipenko
2021-12-20 15:27 ` Dmitry Osipenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.