[PATCH 0/3] memory: brcmstb_memc: Report more DDR information

[PATCH 0/3] memory: brcmstb_memc: Report more DDR information

[Florian Fainelli]

This patch series adds support for reporting the DDR size, type, width,
total width and single/dual rank attributes. This is useful for system
inventory and debugging.

Florian Fainelli (3):
  memory: brcmstb_memc: Cache configuration register value
  Documentation: sysfs: brcmstb-memc: Document new attributes
  memory: brcmstb_memc: Add new DDR attributes

 .../ABI/testing/sysfs-platform-brcmstb-memc   | 39 ++++++++
 drivers/memory/brcmstb_memc.c                 | 92 +++++++++++++++++--
 2 files changed, 124 insertions(+), 7 deletions(-)

-- 
2.34.1

[PATCH 0/3] memory: brcmstb_memc: Report more DDR information

[Florian Fainelli]

This patch series adds support for reporting the DDR size, type, width,
total width and single/dual rank attributes. This is useful for system
inventory and debugging.

Florian Fainelli (3):
  memory: brcmstb_memc: Cache configuration register value
  Documentation: sysfs: brcmstb-memc: Document new attributes
  memory: brcmstb_memc: Add new DDR attributes

 .../ABI/testing/sysfs-platform-brcmstb-memc   | 39 ++++++++
 drivers/memory/brcmstb_memc.c                 | 92 +++++++++++++++++--
 2 files changed, 124 insertions(+), 7 deletions(-)

-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] memory: brcmstb_memc: Cache configuration register value

[Florian Fainelli]

The configuration register does not change once the DDR controller is
configured, in preparation for providing more information about the DDR
type/width in subsequent changes, store this value so we can retrieve
it.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/memory/brcmstb_memc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/memory/brcmstb_memc.c b/drivers/memory/brcmstb_memc.c
index 233a53f5bce1..67c75e21c95e 100644
--- a/drivers/memory/brcmstb_memc.c
+++ b/drivers/memory/brcmstb_memc.c
@@ -29,18 +29,15 @@ struct brcmstb_memc {
 	struct device *dev;
 	void __iomem *ddr_ctrl;
 	unsigned int timeout_cycles;
+	u32 config_reg;
 	u32 frequency;
 	u32 srpd_offset;
 };
 
 static int brcmstb_memc_uses_lpddr4(struct brcmstb_memc *memc)
 {
-	void __iomem *config = memc->ddr_ctrl + REG_MEMC_CNTRLR_CONFIG;
-	u32 reg;
-
-	reg = readl_relaxed(config) & CNTRLR_CONFIG_MASK;
-
-	return reg == CNTRLR_CONFIG_LPDDR4_SHIFT;
+	return (memc->config_reg & CNTRLR_CONFIG_MASK) ==
+		CNTRLR_CONFIG_LPDDR4_SHIFT;
 }
 
 static int brcmstb_memc_srpd_config(struct brcmstb_memc *memc,
@@ -148,6 +145,9 @@ static int brcmstb_memc_probe(struct platform_device *pdev)
 	of_property_read_u32(pdev->dev.of_node, "clock-frequency",
 			     &memc->frequency);
 
+	memc->config_reg = readl_relaxed(memc->ddr_ctrl +
+					 REG_MEMC_CNTRLR_CONFIG);
+
 	ret = sysfs_create_group(&dev->kobj, &dev_attr_group);
 	if (ret)
 		return ret;
-- 
2.34.1

[PATCH 1/3] memory: brcmstb_memc: Cache configuration register value

[Florian Fainelli]

The configuration register does not change once the DDR controller is
configured, in preparation for providing more information about the DDR
type/width in subsequent changes, store this value so we can retrieve
it.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/memory/brcmstb_memc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/memory/brcmstb_memc.c b/drivers/memory/brcmstb_memc.c
index 233a53f5bce1..67c75e21c95e 100644
--- a/drivers/memory/brcmstb_memc.c
+++ b/drivers/memory/brcmstb_memc.c
@@ -29,18 +29,15 @@ struct brcmstb_memc {
 	struct device *dev;
 	void __iomem *ddr_ctrl;
 	unsigned int timeout_cycles;
+	u32 config_reg;
 	u32 frequency;
 	u32 srpd_offset;
 };
 
 static int brcmstb_memc_uses_lpddr4(struct brcmstb_memc *memc)
 {
-	void __iomem *config = memc->ddr_ctrl + REG_MEMC_CNTRLR_CONFIG;
-	u32 reg;
-
-	reg = readl_relaxed(config) & CNTRLR_CONFIG_MASK;
-
-	return reg == CNTRLR_CONFIG_LPDDR4_SHIFT;
+	return (memc->config_reg & CNTRLR_CONFIG_MASK) ==
+		CNTRLR_CONFIG_LPDDR4_SHIFT;
 }
 
 static int brcmstb_memc_srpd_config(struct brcmstb_memc *memc,
@@ -148,6 +145,9 @@ static int brcmstb_memc_probe(struct platform_device *pdev)
 	of_property_read_u32(pdev->dev.of_node, "clock-frequency",
 			     &memc->frequency);
 
+	memc->config_reg = readl_relaxed(memc->ddr_ctrl +
+					 REG_MEMC_CNTRLR_CONFIG);
+
 	ret = sysfs_create_group(&dev->kobj, &dev_attr_group);
 	if (ret)
 		return ret;
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] Documentation: sysfs: brcmstb-memc: Document new attributes

[Florian Fainelli]

Document the DDR rank, size, total size, width and type attributes.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 .../ABI/testing/sysfs-platform-brcmstb-memc   | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-platform-brcmstb-memc b/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
index 2f2b750ac2fd..bc969c02b85f 100644
--- a/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
+++ b/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
@@ -1,3 +1,42 @@
+What:		/sys/bus/platform/devices/*/ddr_rank
+Date:		March 2023
+KernelVersion:	6.3
+Contact:	Florian Fainelli <f.fainelli@gmail.com>
+Description:
+		Displays whether the device is single or dual rank.
+
+What:		/sys/bus/platform/devices/*/ddr_size
+Date:		March 2023
+KernelVersion:	6.3
+Contact:	Florian Fainelli <f.fainelli@gmail.com>
+Description:
+		This field specifies the size of each DRAM device in the first
+		(or only) rank
+
+What:		/sys/bus/platform/devices/*/ddr_total_width
+Date:		March 2023
+KernelVersion:	6.3
+Contact:	Florian Fainelli <f.fainelli@gmail.com>
+Description:
+		This field specifies the total data width of all DRAM devices
+		(in each rank)
+
+What:		/sys/bus/platform/devices/*/ddr_type
+Date:		March 2023
+KernelVersion:	6.3
+Contact:	Florian Fainelli <f.fainelli@gmail.com>
+Description:
+		This field specifies DRAM technology type. Possible values:
+		DDR2, DDR3, DDR4, GDDR5, GDDR5M, LPDDR4.
+
+What:		/sys/bus/platform/devices/*/ddr_width
+Date:		March 2023
+KernelVersion:	6.3
+Contact:	Florian Fainelli <f.fainelli@gmail.com>
+Description:
+		This field specifies the data width of each DRAM device.
+		Possible values are: x8, x16, x32
+
 What:		/sys/bus/platform/devices/*/srpd
 Date:		July 2022
 KernelVersion:	5.21
-- 
2.34.1

[PATCH 2/3] Documentation: sysfs: brcmstb-memc: Document new attributes

[Florian Fainelli]

Document the DDR rank, size, total size, width and type attributes.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 .../ABI/testing/sysfs-platform-brcmstb-memc   | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-platform-brcmstb-memc b/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
index 2f2b750ac2fd..bc969c02b85f 100644
--- a/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
+++ b/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
@@ -1,3 +1,42 @@
+What:		/sys/bus/platform/devices/*/ddr_rank
+Date:		March 2023
+KernelVersion:	6.3
+Contact:	Florian Fainelli <f.fainelli@gmail.com>
+Description:
+		Displays whether the device is single or dual rank.
+
+What:		/sys/bus/platform/devices/*/ddr_size
+Date:		March 2023
+KernelVersion:	6.3
+Contact:	Florian Fainelli <f.fainelli@gmail.com>
+Description:
+		This field specifies the size of each DRAM device in the first
+		(or only) rank
+
+What:		/sys/bus/platform/devices/*/ddr_total_width
+Date:		March 2023
+KernelVersion:	6.3
+Contact:	Florian Fainelli <f.fainelli@gmail.com>
+Description:
+		This field specifies the total data width of all DRAM devices
+		(in each rank)
+
+What:		/sys/bus/platform/devices/*/ddr_type
+Date:		March 2023
+KernelVersion:	6.3
+Contact:	Florian Fainelli <f.fainelli@gmail.com>
+Description:
+		This field specifies DRAM technology type. Possible values:
+		DDR2, DDR3, DDR4, GDDR5, GDDR5M, LPDDR4.
+
+What:		/sys/bus/platform/devices/*/ddr_width
+Date:		March 2023
+KernelVersion:	6.3
+Contact:	Florian Fainelli <f.fainelli@gmail.com>
+Description:
+		This field specifies the data width of each DRAM device.
+		Possible values are: x8, x16, x32
+
 What:		/sys/bus/platform/devices/*/srpd
 Date:		July 2022
 KernelVersion:	5.21
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] memory: brcmstb_memc: Add new DDR attributes

[Florian Fainelli]

Provide information about the DDR size, type, width, total width,
dual/single rank. This is useful for reporting purposes and inventory of
the system(s).

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/memory/brcmstb_memc.c | 80 ++++++++++++++++++++++++++++++++++-
 1 file changed, 79 insertions(+), 1 deletion(-)

diff --git a/drivers/memory/brcmstb_memc.c b/drivers/memory/brcmstb_memc.c
index 67c75e21c95e..032567dfd6e2 100644
--- a/drivers/memory/brcmstb_memc.c
+++ b/drivers/memory/brcmstb_memc.c
@@ -13,7 +13,14 @@
 
 #define REG_MEMC_CNTRLR_CONFIG		0x00
 #define  CNTRLR_CONFIG_LPDDR4_SHIFT	5
-#define  CNTRLR_CONFIG_MASK		0xf
+#define  CNTRLR_CONFIG_MASK		GENMASK(3, 0)
+#define  CNTRLR_CONFIG_SIZE_SHIFT	4
+#define  CNTRLR_CONFIG_SIZE_MASK	GENMASK(7, 4)
+#define  CNTRLR_CONFIG_WIDTH_SHIFT	8
+#define  CNTRLR_CONFIG_WIDTH_MASK	GENMASK(9, 8)
+#define  CNTRLR_CONFIG_TOT_WIDTH_SHIFT	10
+#define  CNTRLR_CONFIG_TOT_WIDTH_MASK	GENMASK(11, 10)
+#define  CNTRLR_CONFIG_RANK_SHIFT	16
 #define REG_MEMC_SRPD_CFG_21		0x20
 #define REG_MEMC_SRPD_CFG_20		0x34
 #define REG_MEMC_SRPD_CFG_1x		0x3c
@@ -63,6 +70,67 @@ static int brcmstb_memc_srpd_config(struct brcmstb_memc *memc,
 	return 0;
 }
 
+static ssize_t ddr_rank_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct brcmstb_memc *memc = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%s\n",
+		       memc->config_reg & CNTRLR_CONFIG_RANK_SHIFT ?
+		       "dual" : "single");
+}
+
+static ssize_t ddr_size_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct brcmstb_memc *memc = dev_get_drvdata(dev);
+	u32 val = (memc->config_reg & CNTRLR_CONFIG_SIZE_MASK) >>
+		  CNTRLR_CONFIG_SIZE_SHIFT;
+
+	return sprintf(buf, "%dMb\n", 256 << val);
+}
+
+static ssize_t ddr_total_width_show(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	struct brcmstb_memc *memc = dev_get_drvdata(dev);
+	u32 val = (memc->config_reg & CNTRLR_CONFIG_TOT_WIDTH_MASK) >>
+		   CNTRLR_CONFIG_TOT_WIDTH_SHIFT;
+
+	return sprintf(buf, "x%d\n", 8 << val);
+}
+
+static ssize_t ddr_type_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct brcmstb_memc *memc = dev_get_drvdata(dev);
+	const char *ddr_type_to_str[] = {
+		"DDR2",
+		"DDR3",
+		"DDR4",
+		"GDDR5M",
+		"GDDR5",
+		"LPDDR4",
+	};
+	u32 val = memc->config_reg & CNTRLR_CONFIG_MASK;
+	const char *type = "unknown";
+
+	if (val < ARRAY_SIZE(ddr_type_to_str))
+		type = ddr_type_to_str[val];
+
+	return sprintf(buf, "%s\n", type);
+}
+
+static ssize_t ddr_width_show(struct device *dev,
+			      struct device_attribute *attr, char *buf)
+{
+	struct brcmstb_memc *memc = dev_get_drvdata(dev);
+	u32 val = (memc->config_reg & CNTRLR_CONFIG_WIDTH_MASK) >>
+		  CNTRLR_CONFIG_WIDTH_SHIFT;
+
+	return sprintf(buf, "x%d\n", 8 << val);
+}
+
 static ssize_t frequency_show(struct device *dev,
 			      struct device_attribute *attr, char *buf)
 {
@@ -105,10 +173,20 @@ static ssize_t srpd_store(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
+static DEVICE_ATTR_RO(ddr_rank);
+static DEVICE_ATTR_RO(ddr_size);
+static DEVICE_ATTR_RO(ddr_total_width);
+static DEVICE_ATTR_RO(ddr_type);
+static DEVICE_ATTR_RO(ddr_width);
 static DEVICE_ATTR_RO(frequency);
 static DEVICE_ATTR_RW(srpd);
 
 static struct attribute *dev_attrs[] = {
+	&dev_attr_ddr_rank.attr,
+	&dev_attr_ddr_size.attr,
+	&dev_attr_ddr_total_width.attr,
+	&dev_attr_ddr_type.attr,
+	&dev_attr_ddr_width.attr,
 	&dev_attr_frequency.attr,
 	&dev_attr_srpd.attr,
 	NULL,
-- 
2.34.1

[PATCH 3/3] memory: brcmstb_memc: Add new DDR attributes

[Florian Fainelli]

Provide information about the DDR size, type, width, total width,
dual/single rank. This is useful for reporting purposes and inventory of
the system(s).

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/memory/brcmstb_memc.c | 80 ++++++++++++++++++++++++++++++++++-
 1 file changed, 79 insertions(+), 1 deletion(-)

diff --git a/drivers/memory/brcmstb_memc.c b/drivers/memory/brcmstb_memc.c
index 67c75e21c95e..032567dfd6e2 100644
--- a/drivers/memory/brcmstb_memc.c
+++ b/drivers/memory/brcmstb_memc.c
@@ -13,7 +13,14 @@
 
 #define REG_MEMC_CNTRLR_CONFIG		0x00
 #define  CNTRLR_CONFIG_LPDDR4_SHIFT	5
-#define  CNTRLR_CONFIG_MASK		0xf
+#define  CNTRLR_CONFIG_MASK		GENMASK(3, 0)
+#define  CNTRLR_CONFIG_SIZE_SHIFT	4
+#define  CNTRLR_CONFIG_SIZE_MASK	GENMASK(7, 4)
+#define  CNTRLR_CONFIG_WIDTH_SHIFT	8
+#define  CNTRLR_CONFIG_WIDTH_MASK	GENMASK(9, 8)
+#define  CNTRLR_CONFIG_TOT_WIDTH_SHIFT	10
+#define  CNTRLR_CONFIG_TOT_WIDTH_MASK	GENMASK(11, 10)
+#define  CNTRLR_CONFIG_RANK_SHIFT	16
 #define REG_MEMC_SRPD_CFG_21		0x20
 #define REG_MEMC_SRPD_CFG_20		0x34
 #define REG_MEMC_SRPD_CFG_1x		0x3c
@@ -63,6 +70,67 @@ static int brcmstb_memc_srpd_config(struct brcmstb_memc *memc,
 	return 0;
 }
 
+static ssize_t ddr_rank_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct brcmstb_memc *memc = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%s\n",
+		       memc->config_reg & CNTRLR_CONFIG_RANK_SHIFT ?
+		       "dual" : "single");
+}
+
+static ssize_t ddr_size_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct brcmstb_memc *memc = dev_get_drvdata(dev);
+	u32 val = (memc->config_reg & CNTRLR_CONFIG_SIZE_MASK) >>
+		  CNTRLR_CONFIG_SIZE_SHIFT;
+
+	return sprintf(buf, "%dMb\n", 256 << val);
+}
+
+static ssize_t ddr_total_width_show(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	struct brcmstb_memc *memc = dev_get_drvdata(dev);
+	u32 val = (memc->config_reg & CNTRLR_CONFIG_TOT_WIDTH_MASK) >>
+		   CNTRLR_CONFIG_TOT_WIDTH_SHIFT;
+
+	return sprintf(buf, "x%d\n", 8 << val);
+}
+
+static ssize_t ddr_type_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct brcmstb_memc *memc = dev_get_drvdata(dev);
+	const char *ddr_type_to_str[] = {
+		"DDR2",
+		"DDR3",
+		"DDR4",
+		"GDDR5M",
+		"GDDR5",
+		"LPDDR4",
+	};
+	u32 val = memc->config_reg & CNTRLR_CONFIG_MASK;
+	const char *type = "unknown";
+
+	if (val < ARRAY_SIZE(ddr_type_to_str))
+		type = ddr_type_to_str[val];
+
+	return sprintf(buf, "%s\n", type);
+}
+
+static ssize_t ddr_width_show(struct device *dev,
+			      struct device_attribute *attr, char *buf)
+{
+	struct brcmstb_memc *memc = dev_get_drvdata(dev);
+	u32 val = (memc->config_reg & CNTRLR_CONFIG_WIDTH_MASK) >>
+		  CNTRLR_CONFIG_WIDTH_SHIFT;
+
+	return sprintf(buf, "x%d\n", 8 << val);
+}
+
 static ssize_t frequency_show(struct device *dev,
 			      struct device_attribute *attr, char *buf)
 {
@@ -105,10 +173,20 @@ static ssize_t srpd_store(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
+static DEVICE_ATTR_RO(ddr_rank);
+static DEVICE_ATTR_RO(ddr_size);
+static DEVICE_ATTR_RO(ddr_total_width);
+static DEVICE_ATTR_RO(ddr_type);
+static DEVICE_ATTR_RO(ddr_width);
 static DEVICE_ATTR_RO(frequency);
 static DEVICE_ATTR_RW(srpd);
 
 static struct attribute *dev_attrs[] = {
+	&dev_attr_ddr_rank.attr,
+	&dev_attr_ddr_size.attr,
+	&dev_attr_ddr_total_width.attr,
+	&dev_attr_ddr_type.attr,
+	&dev_attr_ddr_width.attr,
 	&dev_attr_frequency.attr,
 	&dev_attr_srpd.attr,
 	NULL,
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] Documentation: sysfs: brcmstb-memc: Document new attributes

[Krzysztof Kozlowski]

On 24/03/2023 17:52, Florian Fainelli wrote:
> Document the DDR rank, size, total size, width and type attributes.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  .../ABI/testing/sysfs-platform-brcmstb-memc   | 39 +++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-platform-brcmstb-memc b/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
> index 2f2b750ac2fd..bc969c02b85f 100644
> --- a/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
> +++ b/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
> @@ -1,3 +1,42 @@
> +What:		/sys/bus/platform/devices/*/ddr_rank
> +Date:		March 2023
> +KernelVersion:	6.3

We are now at v6.3, so this cannot go into current cycle. Date is also
not realistic. Target v6.4 and it's date from the crystal ball.

> +Contact:	Florian Fainelli <f.fainelli@gmail.com>
> +Description:
> +		Displays whether the device is single or dual rank.
> +
> +What:		/sys/bus/platform/devices/*/ddr_size
> +Date:		March 2023
> +KernelVersion:	6.3
> +Contact:	Florian Fainelli <f.fainelli@gmail.com>
> +Description:
> +		This field specifies the size of each DRAM device in the first
> +		(or only) rank

Why only first rank? Have in mind that one physical chip can have
multiple ranks and the interface should be ready for it. Otherwise soon
you will have to break it to support different sizes in different ranks.

> +
> +What:		/sys/bus/platform/devices/*/ddr_total_width
> +Date:		March 2023
> +KernelVersion:	6.3
> +Contact:	Florian Fainelli <f.fainelli@gmail.com>
> +Description:
> +		This field specifies the total data width of all DRAM devices
> +		(in each rank)

Sorry, I do not understand. "Total" and "in each" are a bit
contradictory. Also misses units description. Is this

> +
> +What:		/sys/bus/platform/devices/*/ddr_type
> +Date:		March 2023
> +KernelVersion:	6.3
> +Contact:	Florian Fainelli <f.fainelli@gmail.com>
> +Description:
> +		This field specifies DRAM technology type. Possible values:
> +		DDR2, DDR3, DDR4, GDDR5, GDDR5M, LPDDR4.
> +
> +What:		/sys/bus/platform/devices/*/ddr_width
> +Date:		March 2023
> +KernelVersion:	6.3
> +Contact:	Florian Fainelli <f.fainelli@gmail.com>
> +Description:
> +		This field specifies the data width of each DRAM device.

Width in which units? And width of what exactly? data-bus? Then bits and
make it maybe "ddr_width_bits" and print 8/16/32. Is it IO width (number
of DQ pins)? Then it could be also up to 128 in general.


Best regards,
Krzysztof

Re: [PATCH 2/3] Documentation: sysfs: brcmstb-memc: Document new attributes

[Krzysztof Kozlowski]

On 24/03/2023 17:52, Florian Fainelli wrote:
> Document the DDR rank, size, total size, width and type attributes.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  .../ABI/testing/sysfs-platform-brcmstb-memc   | 39 +++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-platform-brcmstb-memc b/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
> index 2f2b750ac2fd..bc969c02b85f 100644
> --- a/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
> +++ b/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
> @@ -1,3 +1,42 @@
> +What:		/sys/bus/platform/devices/*/ddr_rank
> +Date:		March 2023
> +KernelVersion:	6.3

We are now at v6.3, so this cannot go into current cycle. Date is also
not realistic. Target v6.4 and it's date from the crystal ball.

> +Contact:	Florian Fainelli <f.fainelli@gmail.com>
> +Description:
> +		Displays whether the device is single or dual rank.
> +
> +What:		/sys/bus/platform/devices/*/ddr_size
> +Date:		March 2023
> +KernelVersion:	6.3
> +Contact:	Florian Fainelli <f.fainelli@gmail.com>
> +Description:
> +		This field specifies the size of each DRAM device in the first
> +		(or only) rank

Why only first rank? Have in mind that one physical chip can have
multiple ranks and the interface should be ready for it. Otherwise soon
you will have to break it to support different sizes in different ranks.

> +
> +What:		/sys/bus/platform/devices/*/ddr_total_width
> +Date:		March 2023
> +KernelVersion:	6.3
> +Contact:	Florian Fainelli <f.fainelli@gmail.com>
> +Description:
> +		This field specifies the total data width of all DRAM devices
> +		(in each rank)

Sorry, I do not understand. "Total" and "in each" are a bit
contradictory. Also misses units description. Is this

> +
> +What:		/sys/bus/platform/devices/*/ddr_type
> +Date:		March 2023
> +KernelVersion:	6.3
> +Contact:	Florian Fainelli <f.fainelli@gmail.com>
> +Description:
> +		This field specifies DRAM technology type. Possible values:
> +		DDR2, DDR3, DDR4, GDDR5, GDDR5M, LPDDR4.
> +
> +What:		/sys/bus/platform/devices/*/ddr_width
> +Date:		March 2023
> +KernelVersion:	6.3
> +Contact:	Florian Fainelli <f.fainelli@gmail.com>
> +Description:
> +		This field specifies the data width of each DRAM device.

Width in which units? And width of what exactly? data-bus? Then bits and
make it maybe "ddr_width_bits" and print 8/16/32. Is it IO width (number
of DQ pins)? Then it could be also up to 128 in general.


Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] memory: brcmstb_memc: Add new DDR attributes

[Krzysztof Kozlowski]

On 24/03/2023 17:52, Florian Fainelli wrote:
> Provide information about the DDR size, type, width, total width,
> dual/single rank. This is useful for reporting purposes and inventory of
> the system(s).
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  drivers/memory/brcmstb_memc.c | 80 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 79 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/memory/brcmstb_memc.c b/drivers/memory/brcmstb_memc.c
> index 67c75e21c95e..032567dfd6e2 100644
> --- a/drivers/memory/brcmstb_memc.c
> +++ b/drivers/memory/brcmstb_memc.c
> @@ -13,7 +13,14 @@
>  
>  #define REG_MEMC_CNTRLR_CONFIG		0x00
>  #define  CNTRLR_CONFIG_LPDDR4_SHIFT	5
> -#define  CNTRLR_CONFIG_MASK		0xf
> +#define  CNTRLR_CONFIG_MASK		GENMASK(3, 0)
> +#define  CNTRLR_CONFIG_SIZE_SHIFT	4
> +#define  CNTRLR_CONFIG_SIZE_MASK	GENMASK(7, 4)
> +#define  CNTRLR_CONFIG_WIDTH_SHIFT	8
> +#define  CNTRLR_CONFIG_WIDTH_MASK	GENMASK(9, 8)
> +#define  CNTRLR_CONFIG_TOT_WIDTH_SHIFT	10
> +#define  CNTRLR_CONFIG_TOT_WIDTH_MASK	GENMASK(11, 10)
> +#define  CNTRLR_CONFIG_RANK_SHIFT	16
>  #define REG_MEMC_SRPD_CFG_21		0x20
>  #define REG_MEMC_SRPD_CFG_20		0x34
>  #define REG_MEMC_SRPD_CFG_1x		0x3c
> @@ -63,6 +70,67 @@ static int brcmstb_memc_srpd_config(struct brcmstb_memc *memc,
>  	return 0;
>  }
>  
> +static ssize_t ddr_rank_show(struct device *dev,
> +			     struct device_attribute *attr, char *buf)
> +{
> +	struct brcmstb_memc *memc = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "%s\n",
> +		       memc->config_reg & CNTRLR_CONFIG_RANK_SHIFT ?
> +		       "dual" : "single");

Why all these are not sysfs_emit()? I think it is the preferred (safer)
interface.

Best regards,
Krzysztof

Re: [PATCH 3/3] memory: brcmstb_memc: Add new DDR attributes

[Krzysztof Kozlowski]

On 24/03/2023 17:52, Florian Fainelli wrote:
> Provide information about the DDR size, type, width, total width,
> dual/single rank. This is useful for reporting purposes and inventory of
> the system(s).
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  drivers/memory/brcmstb_memc.c | 80 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 79 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/memory/brcmstb_memc.c b/drivers/memory/brcmstb_memc.c
> index 67c75e21c95e..032567dfd6e2 100644
> --- a/drivers/memory/brcmstb_memc.c
> +++ b/drivers/memory/brcmstb_memc.c
> @@ -13,7 +13,14 @@
>  
>  #define REG_MEMC_CNTRLR_CONFIG		0x00
>  #define  CNTRLR_CONFIG_LPDDR4_SHIFT	5
> -#define  CNTRLR_CONFIG_MASK		0xf
> +#define  CNTRLR_CONFIG_MASK		GENMASK(3, 0)
> +#define  CNTRLR_CONFIG_SIZE_SHIFT	4
> +#define  CNTRLR_CONFIG_SIZE_MASK	GENMASK(7, 4)
> +#define  CNTRLR_CONFIG_WIDTH_SHIFT	8
> +#define  CNTRLR_CONFIG_WIDTH_MASK	GENMASK(9, 8)
> +#define  CNTRLR_CONFIG_TOT_WIDTH_SHIFT	10
> +#define  CNTRLR_CONFIG_TOT_WIDTH_MASK	GENMASK(11, 10)
> +#define  CNTRLR_CONFIG_RANK_SHIFT	16
>  #define REG_MEMC_SRPD_CFG_21		0x20
>  #define REG_MEMC_SRPD_CFG_20		0x34
>  #define REG_MEMC_SRPD_CFG_1x		0x3c
> @@ -63,6 +70,67 @@ static int brcmstb_memc_srpd_config(struct brcmstb_memc *memc,
>  	return 0;
>  }
>  
> +static ssize_t ddr_rank_show(struct device *dev,
> +			     struct device_attribute *attr, char *buf)
> +{
> +	struct brcmstb_memc *memc = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "%s\n",
> +		       memc->config_reg & CNTRLR_CONFIG_RANK_SHIFT ?
> +		       "dual" : "single");

Why all these are not sysfs_emit()? I think it is the preferred (safer)
interface.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel