Hi Krzysztof,
This small patch series adds basic support for controlling self-refresh
power down on Broadcom STB memory controllers. We might be able to
contribute more features to the memory controller driver in the future
like accurate reporting of the memory type, timings, and possibly some
performance counters.
Changes in v2:
- merged the v1 first two patches
- added a sysfs document describing attributes exposed
- addressed feedback from Krzysztof regarding style and API usage
Florian Fainelli (3):
dt-bindings: memory-controller: Document Broadcom STB MEMC
Documentation: sysfs: Document Broadcom STB memc sysfs knobs
memory: Add Broadcom STB memory controller driver
.../ABI/testing/sysfs-platform-brcmstb-memc | 15 +
.../bindings/arm/bcm/brcm,brcmstb.txt | 11 +-
.../brcm,brcmstb-memc-ddr.yaml | 53 +++
drivers/memory/Kconfig | 9 +
drivers/memory/Makefile | 1 +
drivers/memory/brcmstb_memc.c | 302 ++++++++++++++++++
6 files changed, 382 insertions(+), 9 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-platform-brcmstb-memc
create mode 100644 Documentation/devicetree/bindings/memory-controllers/brcm,brcmstb-memc-ddr.yaml
create mode 100644 drivers/memory/brcmstb_memc.c
--
2.25.1
Document the Broadcom STB memory controller which is a trivial binding
for now with a set of compatible strings and single register.
Since we introduce this binding, the section in
Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt is removed
and this binding is referenced instead.
Signed-off-by: Florian Fainelli <[email protected]>
---
.../bindings/arm/bcm/brcm,brcmstb.txt | 11 +---
.../brcm,brcmstb-memc-ddr.yaml | 53 +++++++++++++++++++
2 files changed, 55 insertions(+), 9 deletions(-)
create mode 100644 Documentation/devicetree/bindings/memory-controllers/brcm,brcmstb-memc-ddr.yaml
diff --git a/Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt b/Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt
index 104cc9b41df4..e797d2f69f3b 100644
--- a/Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt
+++ b/Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt
@@ -187,15 +187,8 @@ Required properties:
Sequencer DRAM parameters and control registers. Used for Self-Refresh
Power-Down (SRPD), among other things.
-Required properties:
-- compatible : should contain one of these
- "brcm,brcmstb-memc-ddr-rev-b.2.1"
- "brcm,brcmstb-memc-ddr-rev-b.2.2"
- "brcm,brcmstb-memc-ddr-rev-b.2.3"
- "brcm,brcmstb-memc-ddr-rev-b.3.0"
- "brcm,brcmstb-memc-ddr-rev-b.3.1"
- "brcm,brcmstb-memc-ddr"
-- reg : the MEMC DDR register range
+See Documentation/devicetree/bindings/memory-controllers/brcm,memc.yaml for a
+full list of supported compatible strings and properties.
Example:
diff --git a/Documentation/devicetree/bindings/memory-controllers/brcm,brcmstb-memc-ddr.yaml b/Documentation/devicetree/bindings/memory-controllers/brcm,brcmstb-memc-ddr.yaml
new file mode 100644
index 000000000000..a1479b75c536
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/brcm,brcmstb-memc-ddr.yaml
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/memory-controllers/brcm,brcmstb-memc-ddr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Memory controller (MEMC) for Broadcom STB
+
+maintainers:
+ - Florian Fainelli <[email protected]>
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - brcm,brcmstb-memc-ddr-rev-b.1.x
+ - brcm,brcmstb-memc-ddr-rev-b.2.0
+ - brcm,brcmstb-memc-ddr-rev-b.2.1
+ - brcm,brcmstb-memc-ddr-rev-b.2.2
+ - brcm,brcmstb-memc-ddr-rev-b.2.3
+ - brcm,brcmstb-memc-ddr-rev-b.2.5
+ - brcm,brcmstb-memc-ddr-rev-b.2.6
+ - brcm,brcmstb-memc-ddr-rev-b.2.7
+ - brcm,brcmstb-memc-ddr-rev-b.2.8
+ - brcm,brcmstb-memc-ddr-rev-b.3.0
+ - brcm,brcmstb-memc-ddr-rev-b.3.1
+ - brcm,brcmstb-memc-ddr-rev-c.1.0
+ - brcm,brcmstb-memc-ddr-rev-c.1.1
+ - brcm,brcmstb-memc-ddr-rev-c.1.2
+ - brcm,brcmstb-memc-ddr-rev-c.1.3
+ - brcm,brcmstb-memc-ddr-rev-c.1.4
+ - const: brcm,brcmstb-memc-ddr
+
+ reg:
+ maxItems: 1
+
+ clock-frequency:
+ description: DDR PHY frequency in Hz
+
+required:
+ - compatible
+ - reg
+ - clock-frequency
+
+additionalProperties: false
+
+examples:
+ - |
+ memory-controller@9902000 {
+ compatible = "brcm,brcmstb-memc-ddr-rev-c.1.1", "brcm,brcmstb-memc-ddr";
+ reg = <0x9902000 0x600>;
+ clock-frequency = <2133000000>;
+ };
--
2.25.1
Document the "srpd" and "frequency" sysfs attributes exposed by
the brcmstb_memc driver.
Signed-off-by: Florian Fainelli <[email protected]>
---
.../ABI/testing/sysfs-platform-brcmstb-memc | 15 +++++++++++++++
1 file changed, 15 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-platform-brcmstb-memc
diff --git a/Documentation/ABI/testing/sysfs-platform-brcmstb-memc b/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
new file mode 100644
index 000000000000..2bf0f58e412c
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
@@ -0,0 +1,15 @@
+What: /sys/devices/platform/*/*/*/*/srpd
+Date: July 2022
+KernelVersion: 5.21
+Contact: Florian Fainelli <[email protected]>
+Description:
+ Self Refresh Power Down (SRPD) inactivity timeout counted in
+ internal DDR controller clock cycles. Possible values range
+ from 0 (disable inactivity timeout) to 65535 (0xffff).
+
+What: /sys/devices/platform/*/*/*/*/frequency
+Date: July 2022
+KernelVersion: 5.21
+Contact: Florian Fainelli <[email protected]>
+Description:
+ DDR PHY frequency in Hz.
--
2.25.1
Add support for configuring the Self Refresh Power Down (SRPD)
inactivity timeout on Broadcom STB chips. This is used to conserve power
when the DRAM activity is reduced.
Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/memory/Kconfig | 9 +
drivers/memory/Makefile | 1 +
drivers/memory/brcmstb_memc.c | 302 ++++++++++++++++++++++++++++++++++
3 files changed, 312 insertions(+)
create mode 100644 drivers/memory/brcmstb_memc.c
diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
index ac1a411648d8..fac290e48e0b 100644
--- a/drivers/memory/Kconfig
+++ b/drivers/memory/Kconfig
@@ -66,6 +66,15 @@ config BRCMSTB_DPFE
for the DRAM's temperature. Slower refresh rate means cooler RAM,
higher refresh rate means hotter RAM.
+config BRCMSTB_MEMC
+ tristate "Broadcom STB MEMC driver"
+ default ARCH_BRCMSTB
+ depends on ARCH_BRCMSTB || COMPILE_TEST
+ help
+ This driver provides a way to configure the Broadcom STB memory
+ controller and specifically control the Self Refresh Power Down
+ (SRPD) inactivity timeout.
+
config BT1_L2_CTL
bool "Baikal-T1 CM2 L2-RAM Cache Control Block"
depends on MIPS_BAIKAL_T1 || COMPILE_TEST
diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
index bc7663ed1c25..e148f636c082 100644
--- a/drivers/memory/Makefile
+++ b/drivers/memory/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_ARM_PL172_MPMC) += pl172.o
obj-$(CONFIG_ATMEL_SDRAMC) += atmel-sdramc.o
obj-$(CONFIG_ATMEL_EBI) += atmel-ebi.o
obj-$(CONFIG_BRCMSTB_DPFE) += brcmstb_dpfe.o
+obj-$(CONFIG_BRCMSTB_MEMC) += brcmstb_memc.o
obj-$(CONFIG_BT1_L2_CTL) += bt1-l2-ctl.o
obj-$(CONFIG_TI_AEMIF) += ti-aemif.o
obj-$(CONFIG_TI_EMIF) += emif.o
diff --git a/drivers/memory/brcmstb_memc.c b/drivers/memory/brcmstb_memc.c
new file mode 100644
index 000000000000..9b993343d447
--- /dev/null
+++ b/drivers/memory/brcmstb_memc.c
@@ -0,0 +1,302 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * DDR Self-Refresh Power Down (SRPD) support for Broadcom STB SoCs
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+
+#define REG_MEMC_CNTRLR_CONFIG 0x00
+#define CNTRLR_CONFIG_LPDDR4_SHIFT 5
+#define CNTRLR_CONFIG_MASK 0xf
+#define REG_MEMC_SRPD_CFG_21 0x20
+#define REG_MEMC_SRPD_CFG_20 0x34
+#define REG_MEMC_SRPD_CFG_1x 0x3c
+#define INACT_COUNT_SHIFT 0
+#define INACT_COUNT_MASK 0xffff
+#define SRPD_EN_SHIFT 16
+
+struct brcmstb_memc_data {
+ u32 srpd_offset;
+};
+
+struct brcmstb_memc {
+ struct device *dev;
+ void __iomem *ddr_ctrl;
+ unsigned int timeout_cycles;
+ 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;
+}
+
+static int brcmstb_memc_srpd_config(struct brcmstb_memc *memc,
+ unsigned int cycles)
+{
+ void __iomem *cfg = memc->ddr_ctrl + memc->srpd_offset;
+ u32 val;
+
+ /* Max timeout supported in HW */
+ if (cycles > INACT_COUNT_MASK)
+ return -EINVAL;
+
+ memc->timeout_cycles = cycles;
+
+ val = (cycles << INACT_COUNT_SHIFT) & INACT_COUNT_MASK;
+ if (cycles)
+ val |= BIT(SRPD_EN_SHIFT);
+
+ writel_relaxed(val, cfg);
+ /* Ensure the write is committed to the controller */
+ (void)readl_relaxed(cfg);
+
+ return 0;
+}
+
+static ssize_t frequency_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct brcmstb_memc *memc = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%d\n", memc->frequency);
+}
+
+static ssize_t srpd_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct brcmstb_memc *memc = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%d\n", memc->timeout_cycles);
+}
+
+static ssize_t srpd_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct brcmstb_memc *memc = dev_get_drvdata(dev);
+ unsigned int val;
+ int ret;
+
+ /*
+ * Cannot change the inactivity timeout on LPDDR4 chips because the
+ * dynamic tuning process will also get affected by the inactivity
+ * timeout, thus making it non functional.
+ */
+ if (brcmstb_memc_uses_lpddr4(memc))
+ return -EOPNOTSUPP;
+
+ ret = kstrtouint(buf, 10, &val);
+ if (ret < 0)
+ return ret;
+
+ ret = brcmstb_memc_srpd_config(memc, val);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static DEVICE_ATTR_RO(frequency);
+static DEVICE_ATTR_RW(srpd);
+
+static struct attribute *dev_attrs[] = {
+ &dev_attr_frequency.attr,
+ &dev_attr_srpd.attr,
+ NULL,
+};
+
+static struct attribute_group dev_attr_group = {
+ .attrs = dev_attrs,
+};
+
+static const struct of_device_id brcmstb_memc_of_match[];
+
+static int brcmstb_memc_probe(struct platform_device *pdev)
+{
+ const struct brcmstb_memc_data *memc_data;
+ const struct of_device_id *of_id;
+ struct device *dev = &pdev->dev;
+ struct brcmstb_memc *memc;
+ int ret;
+
+ memc = devm_kzalloc(dev, sizeof(*memc), GFP_KERNEL);
+ if (!memc)
+ return -ENOMEM;
+
+ dev_set_drvdata(dev, memc);
+
+ of_id = of_match_device(brcmstb_memc_of_match, dev);
+ memc_data = of_id->data;
+ memc->srpd_offset = memc_data->srpd_offset;
+
+ memc->ddr_ctrl = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(memc->ddr_ctrl))
+ return PTR_ERR(memc->ddr_ctrl);
+
+ of_property_read_u32(pdev->dev.of_node, "clock-frequency",
+ &memc->frequency);
+
+ ret = sysfs_create_group(&dev->kobj, &dev_attr_group);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int brcmstb_memc_remove(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+
+ sysfs_remove_group(&dev->kobj, &dev_attr_group);
+
+ return 0;
+}
+
+enum brcmstb_memc_hwtype {
+ BRCMSTB_MEMC_V21,
+ BRCMSTB_MEMC_V20,
+ BRCMSTB_MEMC_V1X,
+};
+
+static const struct brcmstb_memc_data brcmstb_memc_versions[] = {
+ { .srpd_offset = REG_MEMC_SRPD_CFG_21 },
+ { .srpd_offset = REG_MEMC_SRPD_CFG_20 },
+ { .srpd_offset = REG_MEMC_SRPD_CFG_1x },
+};
+
+static const struct of_device_id brcmstb_memc_of_match[] = {
+ {
+ .compatible = "brcm,brcmstb-memc-ddr-rev-b.1.x",
+ .data = &brcmstb_memc_versions[BRCMSTB_MEMC_V1X]
+ },
+ {
+ .compatible = "brcm,brcmstb-memc-ddr-rev-b.2.0",
+ .data = &brcmstb_memc_versions[BRCMSTB_MEMC_V20]
+ },
+ {
+ .compatible = "brcm,brcmstb-memc-ddr-rev-b.2.1",
+ .data = &brcmstb_memc_versions[BRCMSTB_MEMC_V21]
+ },
+ {
+ .compatible = "brcm,brcmstb-memc-ddr-rev-b.2.2",
+ .data = &brcmstb_memc_versions[BRCMSTB_MEMC_V21]
+ },
+ {
+ .compatible = "brcm,brcmstb-memc-ddr-rev-b.2.3",
+ .data = &brcmstb_memc_versions[BRCMSTB_MEMC_V21]
+ },
+ {
+ .compatible = "brcm,brcmstb-memc-ddr-rev-b.2.5",
+ .data = &brcmstb_memc_versions[BRCMSTB_MEMC_V21]
+ },
+ {
+ .compatible = "brcm,brcmstb-memc-ddr-rev-b.2.6",
+ .data = &brcmstb_memc_versions[BRCMSTB_MEMC_V21]
+ },
+ {
+ .compatible = "brcm,brcmstb-memc-ddr-rev-b.2.7",
+ .data = &brcmstb_memc_versions[BRCMSTB_MEMC_V21]
+ },
+ {
+ .compatible = "brcm,brcmstb-memc-ddr-rev-b.2.8",
+ .data = &brcmstb_memc_versions[BRCMSTB_MEMC_V21]
+ },
+ {
+ .compatible = "brcm,brcmstb-memc-ddr-rev-b.3.0",
+ .data = &brcmstb_memc_versions[BRCMSTB_MEMC_V21]
+ },
+ {
+ .compatible = "brcm,brcmstb-memc-ddr-rev-b.3.1",
+ .data = &brcmstb_memc_versions[BRCMSTB_MEMC_V21]
+ },
+ {
+ .compatible = "brcm,brcmstb-memc-ddr-rev-c.1.0",
+ .data = &brcmstb_memc_versions[BRCMSTB_MEMC_V21]
+ },
+ {
+ .compatible = "brcm,brcmstb-memc-ddr-rev-c.1.1",
+ .data = &brcmstb_memc_versions[BRCMSTB_MEMC_V21]
+ },
+ {
+ .compatible = "brcm,brcmstb-memc-ddr-rev-c.1.2",
+ .data = &brcmstb_memc_versions[BRCMSTB_MEMC_V21]
+ },
+ {
+ .compatible = "brcm,brcmstb-memc-ddr-rev-c.1.3",
+ .data = &brcmstb_memc_versions[BRCMSTB_MEMC_V21]
+ },
+ {
+ .compatible = "brcm,brcmstb-memc-ddr-rev-c.1.4",
+ .data = &brcmstb_memc_versions[BRCMSTB_MEMC_V21]
+ },
+ /* default to the original offset */
+ {
+ .compatible = "brcm,brcmstb-memc-ddr",
+ .data = &brcmstb_memc_versions[BRCMSTB_MEMC_V1X]
+ },
+ {}
+};
+
+static int __maybe_unused brcmstb_memc_suspend(struct device *dev)
+{
+ struct brcmstb_memc *memc = dev_get_drvdata(dev);
+ void __iomem *cfg = memc->ddr_ctrl + memc->srpd_offset;
+ u32 val;
+
+ if (memc->timeout_cycles == 0)
+ return 0;
+
+ /*
+ * Disable SRPD prior to suspending the system since that can
+ * cause issues with other memory clients managed by the ARM
+ * trusted firmware to access memory.
+ */
+ val = readl_relaxed(cfg);
+ val &= ~BIT(SRPD_EN_SHIFT);
+ writel_relaxed(val, cfg);
+ /* Ensure the write is committed to the controller */
+ (void)readl_relaxed(cfg);
+
+ return 0;
+}
+
+static int __maybe_unused brcmstb_memc_resume(struct device *dev)
+{
+ struct brcmstb_memc *memc = dev_get_drvdata(dev);
+
+ if (memc->timeout_cycles == 0)
+ return 0;
+
+ return brcmstb_memc_srpd_config(memc, memc->timeout_cycles);
+}
+
+static SIMPLE_DEV_PM_OPS(brcmstb_memc_pm_ops, brcmstb_memc_suspend,
+ brcmstb_memc_resume);
+
+static struct platform_driver brcmstb_memc_driver = {
+ .probe = brcmstb_memc_probe,
+ .remove = brcmstb_memc_remove,
+ .driver = {
+ .name = "brcmstb_memc",
+ .owner = THIS_MODULE,
+ .of_match_table = brcmstb_memc_of_match,
+ .pm = &brcmstb_memc_pm_ops,
+ },
+};
+module_platform_driver(brcmstb_memc_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Broadcom");
+MODULE_DESCRIPTION("DDR SRPD driver for Broadcom STB chips");
--
2.25.1
On Fri, 22 Jul 2022 13:10:41 -0700, Florian Fainelli wrote:
> Document the Broadcom STB memory controller which is a trivial binding
> for now with a set of compatible strings and single register.
>
> Since we introduce this binding, the section in
> Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt is removed
> and this binding is referenced instead.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> .../bindings/arm/bcm/brcm,brcmstb.txt | 11 +---
> .../brcm,brcmstb-memc-ddr.yaml | 53 +++++++++++++++++++
> 2 files changed, 55 insertions(+), 9 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/memory-controllers/brcm,brcmstb-memc-ddr.yaml
>
Running 'make dtbs_check' with the schema in this patch gives the
following warnings. Consider if they are expected or the schema is
incorrect. These may not be new warnings.
Note that it is not yet a requirement to have 0 warnings for dtbs_check.
This will change in the future.
Full log is available here: https://patchwork.ozlabs.org/patch/
memc-ddr@2000: 'clock-frequency' is a required property
arch/arm/boot/dts/bcm7445-bcm97445svmb.dtb
arch/arm/boot/dts/bcm7445-bcm97445svmb.dtb
arch/arm/boot/dts/bcm7445-bcm97445svmb.dtb
memc-ddr@2000: compatible:0: 'brcm,brcmstb-memc-ddr' is not one of ['brcm,brcmstb-memc-ddr-rev-b.1.x', 'brcm,brcmstb-memc-ddr-rev-b.2.0', 'brcm,brcmstb-memc-ddr-rev-b.2.1', 'brcm,brcmstb-memc-ddr-rev-b.2.2', 'brcm,brcmstb-memc-ddr-rev-b.2.3', 'brcm,brcmstb-memc-ddr-rev-b.2.5', 'brcm,brcmstb-memc-ddr-rev-b.2.6', 'brcm,brcmstb-memc-ddr-rev-b.2.7', 'brcm,brcmstb-memc-ddr-rev-b.2.8', 'brcm,brcmstb-memc-ddr-rev-b.3.0', 'brcm,brcmstb-memc-ddr-rev-b.3.1', 'brcm,brcmstb-memc-ddr-rev-c.1.0', 'brcm,brcmstb-memc-ddr-rev-c.1.1', 'brcm,brcmstb-memc-ddr-rev-c.1.2', 'brcm,brcmstb-memc-ddr-rev-c.1.3', 'brcm,brcmstb-memc-ddr-rev-c.1.4']
arch/arm/boot/dts/bcm7445-bcm97445svmb.dtb
arch/arm/boot/dts/bcm7445-bcm97445svmb.dtb
arch/arm/boot/dts/bcm7445-bcm97445svmb.dtb
memc-ddr@2000: compatible: ['brcm,brcmstb-memc-ddr'] is too short
arch/arm/boot/dts/bcm7445-bcm97445svmb.dtb
arch/arm/boot/dts/bcm7445-bcm97445svmb.dtb
arch/arm/boot/dts/bcm7445-bcm97445svmb.dtb
On 22/07/2022 22:10, Florian Fainelli wrote:
> Document the "srpd" and "frequency" sysfs attributes exposed by
> the brcmstb_memc driver.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> .../ABI/testing/sysfs-platform-brcmstb-memc | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-platform-brcmstb-memc
>
> diff --git a/Documentation/ABI/testing/sysfs-platform-brcmstb-memc b/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
> new file mode 100644
> index 000000000000..2bf0f58e412c
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
> @@ -0,0 +1,15 @@
> +What: /sys/devices/platform/*/*/*/*/srpd
That's a lot of */. Are you sure it is correct path? Didn't you include
here some driver-related path components? Can you paste in email full
path as an example?
> +Date: July 2022
> +KernelVersion: 5.21
> +Contact: Florian Fainelli <[email protected]>
> +Description:
> + Self Refresh Power Down (SRPD) inactivity timeout counted in
> + internal DDR controller clock cycles. Possible values range
> + from 0 (disable inactivity timeout) to 65535 (0xffff).
Using hex suggests one should write there hex? If so, skip decimal...
You describe the user interface, not hardware registers.
> +
> +What: /sys/devices/platform/*/*/*/*/frequency
> +Date: July 2022
> +KernelVersion: 5.21
> +Contact: Florian Fainelli <[email protected]>
> +Description:
> + DDR PHY frequency in Hz.
Best regards,
Krzysztof
On 7/23/22 10:59, Krzysztof Kozlowski wrote:
> On 22/07/2022 22:10, Florian Fainelli wrote:
>> Document the "srpd" and "frequency" sysfs attributes exposed by
>> the brcmstb_memc driver.
>>
>> Signed-off-by: Florian Fainelli <[email protected]>
>> ---
>> .../ABI/testing/sysfs-platform-brcmstb-memc | 15 +++++++++++++++
>> 1 file changed, 15 insertions(+)
>> create mode 100644 Documentation/ABI/testing/sysfs-platform-brcmstb-memc
>>
>> diff --git a/Documentation/ABI/testing/sysfs-platform-brcmstb-memc b/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
>> new file mode 100644
>> index 000000000000..2bf0f58e412c
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
>> @@ -0,0 +1,15 @@
>> +What: /sys/devices/platform/*/*/*/*/srpd
>
> That's a lot of */. Are you sure it is correct path? Didn't you include
> here some driver-related path components? Can you paste in email full
> path as an example?
Yes this is the correct path:
/sys/devices/platform/rdb/rdb:memory_controllers/rdb:memory_controllers:memc@0/9902000.memc-ddr/
the 'rdb' node is our top level bus node, the 'rdb:memory_controllers' is an encapsulating node that groups all of the possible memory controllers in a system (there can be between 1 and 3), the rdb:memory_controllers@0 is the first of those memory controller and finally the 9902000.memc-ddr is the sub-node that contains the register controls of interest, since the memory controller aggregates different functions (arbitration, configuration, statistics, DDR PHY SHIM layer, etc.). Maybe I should provide a more complete binding while I am it.
>
>> +Date: July 2022
>> +KernelVersion: 5.21
>> +Contact: Florian Fainelli <[email protected]>
>> +Description:
>> + Self Refresh Power Down (SRPD) inactivity timeout counted in
>> + internal DDR controller clock cycles. Possible values range
>> + from 0 (disable inactivity timeout) to 65535 (0xffff).
>
> Using hex suggests one should write there hex? If so, skip decimal...
> You describe the user interface, not hardware registers.
Fair enough.
--
Florian
On Fri, 22 Jul 2022 13:10:41 -0700, Florian Fainelli wrote:
> Document the Broadcom STB memory controller which is a trivial binding
> for now with a set of compatible strings and single register.
>
> Since we introduce this binding, the section in
> Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt is removed
> and this binding is referenced instead.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> .../bindings/arm/bcm/brcm,brcmstb.txt | 11 +---
> .../brcm,brcmstb-memc-ddr.yaml | 53 +++++++++++++++++++
> 2 files changed, 55 insertions(+), 9 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/memory-controllers/brcm,brcmstb-memc-ddr.yaml
>
Reviewed-by: Rob Herring <[email protected]>
On 25/07/2022 18:07, Florian Fainelli wrote:
> On 7/23/22 10:59, Krzysztof Kozlowski wrote:
>> On 22/07/2022 22:10, Florian Fainelli wrote:
>>> Document the "srpd" and "frequency" sysfs attributes exposed by
>>> the brcmstb_memc driver.
>>>
>>> Signed-off-by: Florian Fainelli <[email protected]>
>>> ---
>>> .../ABI/testing/sysfs-platform-brcmstb-memc | 15 +++++++++++++++
>>> 1 file changed, 15 insertions(+)
>>> create mode 100644 Documentation/ABI/testing/sysfs-platform-brcmstb-memc
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-platform-brcmstb-memc b/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
>>> new file mode 100644
>>> index 000000000000..2bf0f58e412c
>>> --- /dev/null
>>> +++ b/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
>>> @@ -0,0 +1,15 @@
>>> +What: /sys/devices/platform/*/*/*/*/srpd
>>
>> That's a lot of */. Are you sure it is correct path? Didn't you include
>> here some driver-related path components? Can you paste in email full
>> path as an example?
>
> Yes this is the correct path:
>
> /sys/devices/platform/rdb/rdb:memory_controllers/rdb:memory_controllers:memc@0/9902000.memc-ddr/
>
> the 'rdb' node is our top level bus node, the 'rdb:memory_controllers' is an encapsulating node that groups all of the possible memory controllers in a system (there can be between 1 and 3), the rdb:memory_controllers@0 is the first of those memory controller and finally the 9902000.memc-ddr is the sub-node that contains the register controls of interest, since the memory controller aggregates different functions (arbitration, configuration, statistics, DDR PHY SHIM layer, etc.). Maybe I should provide a more complete binding while I am it.
The path should be much more specific so include at least:
rdb/rdb:memory_controllers/rdb:memory_controllers:memc@*/
(or some variations of it if pieces of name change)
However looking at the driver, this is regular platform driver, thus it
will appear as:
/sys/bus/platform/9902000.memc-ddr
Best regards,
Krzysztof
On 7/26/22 02:34, Krzysztof Kozlowski wrote:
> On 25/07/2022 18:07, Florian Fainelli wrote:
>> On 7/23/22 10:59, Krzysztof Kozlowski wrote:
>>> On 22/07/2022 22:10, Florian Fainelli wrote:
>>>> Document the "srpd" and "frequency" sysfs attributes exposed by
>>>> the brcmstb_memc driver.
>>>>
>>>> Signed-off-by: Florian Fainelli <[email protected]>
>>>> ---
>>>> .../ABI/testing/sysfs-platform-brcmstb-memc | 15 +++++++++++++++
>>>> 1 file changed, 15 insertions(+)
>>>> create mode 100644 Documentation/ABI/testing/sysfs-platform-brcmstb-memc
>>>>
>>>> diff --git a/Documentation/ABI/testing/sysfs-platform-brcmstb-memc b/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
>>>> new file mode 100644
>>>> index 000000000000..2bf0f58e412c
>>>> --- /dev/null
>>>> +++ b/Documentation/ABI/testing/sysfs-platform-brcmstb-memc
>>>> @@ -0,0 +1,15 @@
>>>> +What: /sys/devices/platform/*/*/*/*/srpd
>>>
>>> That's a lot of */. Are you sure it is correct path? Didn't you include
>>> here some driver-related path components? Can you paste in email full
>>> path as an example?
>>
>> Yes this is the correct path:
>>
>> /sys/devices/platform/rdb/rdb:memory_controllers/rdb:memory_controllers:memc@0/9902000.memc-ddr/
>>
>> the 'rdb' node is our top level bus node, the 'rdb:memory_controllers' is an encapsulating node that groups all of the possible memory controllers in a system (there can be between 1 and 3), the rdb:memory_controllers@0 is the first of those memory controller and finally the 9902000.memc-ddr is the sub-node that contains the register controls of interest, since the memory controller aggregates different functions (arbitration, configuration, statistics, DDR PHY SHIM layer, etc.). Maybe I should provide a more complete binding while I am it.
>
> The path should be much more specific so include at least:
> rdb/rdb:memory_controllers/rdb:memory_controllers:memc@*/
> (or some variations of it if pieces of name change)
>
> However looking at the driver, this is regular platform driver, thus it
> will appear as:
> /sys/bus/platform/9902000.memc-ddr
There is a missing /devices in the path, but yet that will work too:
/sys/bus/platform/devices/9902000.memc-ddr
Thanks!
--
Florian