[PATCH v5 0/3] Retrieve information about DDR from SMEM

[PATCH v5 0/3] Retrieve information about DDR from SMEM

[Konrad Dybcio]

SMEM allows the OS to retrieve information about the DDR memory.
Among that information, is a semi-magic value called 'HBB', or Highest
Bank address Bit, which multimedia drivers (for hardware like Adreno
and MDSS) must retrieve in order to program the IP blocks correctly.

This series introduces an API to retrieve that value, uses it in the
aforementioned programming sequences and exposes available DDR
frequencies in debugfs (to e.g. pass to aoss_qmp debugfs). More
information can be exposed in the future, as needed.

Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
Changes in v5:
- Parse DRAM data before publishing __smem
- Add trivial patch 1 converting 'unsigned' to 'unsigned int' to make
  checkpatch happy
- Link to v4: https://patch.msgid.link/20260721-topic-smem_dramc-v4-0-748f992278a3@oss.qualcomm.com

Changes in v4:
- Rebase on top of Dmitry's cleanups to ubwc_config
- Mark and consume the shared data as little-endian
- Fix compiler optimizing out what it thinks is impossible (odd
  v3_14freq case)
- Rework qcom_ubwc_config_get_data() per discussion to not alter
  hardcoded data
- Remove some unnecessary comments and assignments
- Return -ENODATA instead of -EINVAL for out-of-sensible-range HBB
- Cosmetic/line wrapping fixups
- Drop applied drm/msm patch
- Link to v3: https://lore.kernel.org/r/20260108-topic-smem_dramc-v3-0-6b64df58a017@oss.qualcomm.com

Changes in v3:
- Support v6 and v7 DDRInfo (v7 is used on e.g. Hamoa)
- Handle rare cases of DDRInfo v5 with additional trailing data
- Rebase/adjust to SSoT UBWC
- Expose hbb value in debugfs
- cosmetic changes
- Link to v2: https://lore.kernel.org/r/20250410-topic-smem_dramc-v2-0-dead15264714@oss.qualcomm.com

Changes in v2:
- Avoid checking for < 0 on unsigned types
- Overwrite Adreno UBWC data to keep the data shared with userspace
  coherent with what's programmed into the hardware
- Call get_hbb() in msm_mdss_enable() instead of all UBWC setup
  branches separately
- Pick up Bjorn's rb on patch 1
- Link to v1: https://lore.kernel.org/r/20250409-topic-smem_dramc-v1-0-94d505cd5593@oss.qualcomm.com

To: Bjorn Andersson <andersson@kernel.org>
To: Konrad Dybcio <konradybcio@kernel.org>
To: Kees Cook <kees@kernel.org>
To: "Gustavo A. R. Silva" <gustavoars@kernel.org>
To: Alim Akhtar <alim.akhtar@samsung.com>
To: Avri Altman <avri.altman@sandisk.com>
To: Bart Van Assche <bvanassche@acm.org>
To: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
To: "Martin K. Petersen" <martin.petersen@oracle.com>
To: Matthias Brugger <matthias.bgg@gmail.com>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
To: Suzuki K Poulose <suzuki.poulose@arm.com>
To: Mike Leach <mike.leach@arm.com>
To: James Clark <james.clark@linaro.org>
To: Leo Yan <leo.yan@arm.com>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>
To: Mao Jinlong <jinlong.mao@oss.qualcomm.com>
To: Jie Gan <jie.gan@oss.qualcomm.com>
To: Paul Walmsley <pjw@kernel.org>
To: Palmer Dabbelt <palmer@dabbelt.com>
To: Albert Ou <aou@eecs.berkeley.edu>
To: Alexandre Ghiti <alex@ghiti.fr>
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-hardening@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-mediatek@lists.infradead.org
Cc: coresight@lists.linaro.org
Cc: devicetree@vger.kernel.org
Cc: linux-riscv@lists.infradead.org

---
Konrad Dybcio (3):
      soc: qcom: smem: Use 'unsigned int' instead of 'unsigned'
      soc: qcom: smem: Expose DDR data from SMEM
      soc: qcom: ubwc: Get HBB from SMEM

 drivers/soc/qcom/Makefile      |   3 +-
 drivers/soc/qcom/smem.c        |  80 +++++---
 drivers/soc/qcom/smem.h        |  12 ++
 drivers/soc/qcom/smem_dramc.c  | 430 +++++++++++++++++++++++++++++++++++++++++
 drivers/soc/qcom/ubwc_config.c |  40 +++-
 include/linux/soc/qcom/smem.h  |   2 +
 6 files changed, 533 insertions(+), 34 deletions(-)
---
base-commit: c5e32e86ca02b003f86e095d379b38148999293d
change-id: 20250409-topic-smem_dramc-6467187ac865

Best regards,
--  
Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

[PATCH v5 1/3] soc: qcom: smem: Use 'unsigned int' instead of 'unsigned'

[Konrad Dybcio]

From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

checkpatch.pl reports:

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'

Convert the SMEM driver to use the less ambiguous type instead.

Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 drivers/soc/qcom/smem.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index afb21a778fe7..43b66120fc64 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -281,7 +281,7 @@ struct qcom_smem {
 	struct smem_partition global_partition;
 	struct xarray partitions;
 
-	unsigned num_regions;
+	unsigned int num_regions;
 	struct smem_region regions[] __counted_by(num_regions);
 };
 
@@ -399,7 +399,7 @@ EXPORT_SYMBOL_GPL(qcom_smem_is_available);
 
 static int qcom_smem_alloc_private(struct qcom_smem *smem,
 				   struct smem_partition *part,
-				   unsigned item,
+				   unsigned int item,
 				   size_t size)
 {
 	struct smem_private_entry *hdr, *end;
@@ -460,7 +460,7 @@ static int qcom_smem_alloc_private(struct qcom_smem *smem,
 }
 
 static int qcom_smem_alloc_global(struct qcom_smem *smem,
-				  unsigned item,
+				  unsigned int item,
 				  size_t size)
 {
 	struct smem_global_entry *entry;
@@ -503,7 +503,7 @@ static int qcom_smem_alloc_global(struct qcom_smem *smem,
  *
  * Return: 0 on success, negative errno on failure.
  */
-int qcom_smem_alloc(unsigned host, unsigned item, size_t size)
+int qcom_smem_alloc(unsigned int host, unsigned int item, size_t size)
 {
 	struct smem_partition *part;
 	unsigned long flags;
@@ -544,7 +544,7 @@ int qcom_smem_alloc(unsigned host, unsigned item, size_t size)
 EXPORT_SYMBOL_GPL(qcom_smem_alloc);
 
 static void *qcom_smem_get_global(struct qcom_smem *smem,
-				  unsigned item,
+				  unsigned int item,
 				  size_t *size)
 {
 	struct smem_header *header;
@@ -553,7 +553,6 @@ static void *qcom_smem_get_global(struct qcom_smem *smem,
 	u64 entry_offset;
 	u32 e_size;
 	u32 aux_base;
-	unsigned i;
 
 	header = smem->regions[0].virt_base;
 	entry = &header->toc[item];
@@ -562,7 +561,7 @@ static void *qcom_smem_get_global(struct qcom_smem *smem,
 
 	aux_base = le32_to_cpu(entry->aux_base) & AUX_BASE_MASK;
 
-	for (i = 0; i < smem->num_regions; i++) {
+	for (unsigned int i = 0; i < smem->num_regions; i++) {
 		region = &smem->regions[i];
 
 		if ((u32)region->aux_base == aux_base || !aux_base) {
@@ -584,7 +583,7 @@ static void *qcom_smem_get_global(struct qcom_smem *smem,
 
 static void *qcom_smem_get_private(struct qcom_smem *smem,
 				   struct smem_partition *part,
-				   unsigned item,
+				   unsigned int item,
 				   size_t *size)
 {
 	struct smem_private_entry *e, *end;
@@ -683,7 +682,7 @@ static void *qcom_smem_get_private(struct qcom_smem *smem,
  *
  * Return: a pointer to an SMEM item on success, ERR_PTR() on failure.
  */
-void *qcom_smem_get(unsigned host, unsigned item, size_t *size)
+void *qcom_smem_get(unsigned int host, unsigned int item, size_t *size)
 {
 	struct smem_partition *part;
 	void *ptr;
@@ -717,12 +716,12 @@ EXPORT_SYMBOL_GPL(qcom_smem_get);
  *
  * Return: number of available bytes on success, negative errno on failure.
  */
-int qcom_smem_get_free_space(unsigned host)
+int qcom_smem_get_free_space(unsigned int host)
 {
 	struct smem_partition *part;
 	struct smem_partition_header *phdr;
 	struct smem_header *header;
-	unsigned ret;
+	unsigned int ret;
 
 	if (IS_ERR(__smem))
 		return PTR_ERR(__smem);

-- 
2.55.0

[PATCH v5 2/3] soc: qcom: smem: Expose DDR data from SMEM

[Konrad Dybcio]

From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Most modern Qualcomm platforms (>= SM8150) expose information about the
DDR memory present on the system via SMEM.

Details from this information is used in various scenarios, such as
multimedia drivers configuring the hardware based on the "Highest Bank
address Bit" (hbb), or the list of valid frequencies in validation
scenarios...

Add support for parsing v3-v7 version of the structs. Unforunately,
they are not versioned, so some elbow grease is necessary to determine
which one is present. See for reference:

ver 3: https://git.codelinaro.org/clo/la/abl/tianocore/edk2/-/commit/1d11897d2cfcc7b85f28ff74c445018dbbecac7a
ver 4: https://git.codelinaro.org/clo/la/abl/tianocore/edk2/-/commit/f6e9aa549260bbc0bdcb156c2b05f48dc5963203
ver 5: https://git.codelinaro.org/clo/la/abl/tianocore/edk2/-/commit/617d3297abe8b1b8dd3de3d1dd69c3961e6f343f
ver 5 with 6regions: https://git.codelinaro.org/clo/la/abl/tianocore/edk2/-/commit/d770e009f9bae58d56d926f7490bbfb45af8341f
ver 6: https://git.codelinaro.org/clo/la/abl/tianocore/edk2/-/commit/62659b557fdb1551b20fae8073d1d701dfa8a62e
ver 7: https://git.codelinaro.org/clo/la/abl/tianocore/edk2/-/commit/734d95599c5ebb1ca0d4e1639142e65c590532b7

Reviewed-by: Bjorn Andersson <andersson@kernel.org>
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 drivers/soc/qcom/Makefile     |   3 +-
 drivers/soc/qcom/smem.c       |  59 +++---
 drivers/soc/qcom/smem.h       |  12 ++
 drivers/soc/qcom/smem_dramc.c | 430 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/soc/qcom/smem.h |   2 +
 5 files changed, 484 insertions(+), 22 deletions(-)

diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index b7f1d2a57367..798643be3590 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -23,7 +23,8 @@ obj-$(CONFIG_QCOM_RPMH)		+= qcom_rpmh.o
 qcom_rpmh-y			+= rpmh-rsc.o
 qcom_rpmh-y			+= rpmh.o
 obj-$(CONFIG_QCOM_SMD_RPM)	+= rpm-proc.o smd-rpm.o
-obj-$(CONFIG_QCOM_SMEM) +=	smem.o
+qcom_smem-y			+= smem.o smem_dramc.o
+obj-$(CONFIG_QCOM_SMEM) +=	qcom_smem.o
 obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
 CFLAGS_smp2p.o := -I$(src)
 obj-$(CONFIG_QCOM_SMP2P)	+= smp2p.o
diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index 43b66120fc64..4c689fc17011 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -4,6 +4,7 @@
  * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
  */
 
+#include <linux/debugfs.h>
 #include <linux/hwspinlock.h>
 #include <linux/io.h>
 #include <linux/module.h>
@@ -16,6 +17,8 @@
 #include <linux/soc/qcom/smem.h>
 #include <linux/soc/qcom/socinfo.h>
 
+#include "smem.h"
+
 /*
  * The Qualcomm shared memory system is a allocate only heap structure that
  * consists of one of more memory areas that can be accessed by the processors
@@ -281,6 +284,8 @@ struct qcom_smem {
 	struct smem_partition global_partition;
 	struct xarray partitions;
 
+	struct dentry *debugfs_dir;
+
 	unsigned int num_regions;
 	struct smem_region regions[] __counted_by(num_regions);
 };
@@ -671,6 +676,30 @@ static void *qcom_smem_get_private(struct qcom_smem *smem,
 	return ERR_PTR(-EINVAL);
 }
 
+void *__qcom_smem_get(struct qcom_smem *smem, unsigned int host, unsigned int item, size_t *size)
+{
+	struct smem_partition *part;
+	void *ptr;
+
+	if (IS_ERR(smem))
+		return smem;
+
+	if (item >= smem->item_count)
+		return ERR_PTR(-EINVAL);
+
+	part = xa_load(&smem->partitions, host);
+	if (part) {
+		ptr = qcom_smem_get_private(smem, part, item, size);
+	} else if (smem->global_partition.virt_base) {
+		part = &smem->global_partition;
+		ptr = qcom_smem_get_private(smem, part, item, size);
+	} else {
+		ptr = qcom_smem_get_global(smem, item, size);
+	}
+
+	return ptr;
+}
+
 /**
  * qcom_smem_get() - resolve ptr of size of a smem item
  * @host:	the remote processor, or -1
@@ -684,26 +713,7 @@ static void *qcom_smem_get_private(struct qcom_smem *smem,
  */
 void *qcom_smem_get(unsigned int host, unsigned int item, size_t *size)
 {
-	struct smem_partition *part;
-	void *ptr;
-
-	if (IS_ERR(__smem))
-		return __smem;
-
-	if (item >= __smem->item_count)
-		return ERR_PTR(-EINVAL);
-
-	part = xa_load(&__smem->partitions, host);
-	if (part) {
-		ptr = qcom_smem_get_private(__smem, part, item, size);
-	} else if (__smem->global_partition.virt_base) {
-		part = &__smem->global_partition;
-		ptr = qcom_smem_get_private(__smem, part, item, size);
-	} else {
-		ptr = qcom_smem_get_global(__smem, item, size);
-	}
-
-	return ptr;
+	return __qcom_smem_get(__smem, host, item, size);
 }
 EXPORT_SYMBOL_GPL(qcom_smem_get);
 
@@ -1236,19 +1246,26 @@ static int qcom_smem_probe(struct platform_device *pdev)
 	if (ret < 0 && ret != -ENOENT)
 		return ret;
 
+	smem->debugfs_dir = smem_dram_parse(smem, smem->dev);
+
 	__smem = smem;
 
 	smem->socinfo = platform_device_register_data(&pdev->dev, "qcom-socinfo",
 						      PLATFORM_DEVID_NONE, NULL,
 						      0);
-	if (IS_ERR(smem->socinfo))
+	if (IS_ERR(smem->socinfo)) {
+		debugfs_remove_recursive(smem->debugfs_dir);
+
 		dev_dbg(&pdev->dev, "failed to register socinfo device\n");
+	}
 
 	return 0;
 }
 
 static void qcom_smem_remove(struct platform_device *pdev)
 {
+	debugfs_remove_recursive(__smem->debugfs_dir);
+
 	platform_device_unregister(__smem->socinfo);
 
 	xa_destroy(&__smem->partitions);
diff --git a/drivers/soc/qcom/smem.h b/drivers/soc/qcom/smem.h
new file mode 100644
index 000000000000..40523eb87231
--- /dev/null
+++ b/drivers/soc/qcom/smem.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __QCOM_SMEM_INTERNAL__
+#define __QCOM_SMEM_INTERNAL__
+
+#include <linux/device.h>
+
+struct qcom_smem;
+
+struct dentry *smem_dram_parse(struct qcom_smem *smem, struct device *dev);
+void *__qcom_smem_get(struct qcom_smem *smem, unsigned int host, unsigned int item, size_t *size);
+
+#endif
diff --git a/drivers/soc/qcom/smem_dramc.c b/drivers/soc/qcom/smem_dramc.c
new file mode 100644
index 000000000000..63ebe7f6db59
--- /dev/null
+++ b/drivers/soc/qcom/smem_dramc.c
@@ -0,0 +1,430 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#include <linux/debugfs.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/soc/qcom/smem.h>
+#include <linux/units.h>
+
+#include "smem.h"
+
+#define SMEM_DDR_INFO_ID		603
+
+#define MAX_DDR_FREQ_NUM_V3		13
+#define MAX_DDR_FREQ_NUM_V5		14
+
+#define MAX_CHAN_NUM			8
+#define MAX_RANK_NUM			2
+
+#define DDR_HBB_MIN			13
+#define DDR_HBB_MAX			19
+
+#define MAX_SHUB_ENTRIES		8
+
+static struct smem_dram *__dram;
+
+enum ddr_info_version {
+	INFO_UNKNOWN,
+	INFO_V3,
+	INFO_V3_WITH_14_FREQS,
+	INFO_V4,
+	INFO_V5,
+	INFO_V5_WITH_6_REGIONS,
+	INFO_V6, /* INFO_V6 seems to only have shipped with 6 DDR regions, unlike V7 */
+	INFO_V7,
+	INFO_V7_WITH_6_REGIONS,
+};
+
+struct smem_dram {
+	unsigned long frequencies[MAX_DDR_FREQ_NUM_V5];
+	u32 num_frequencies;
+	u8 hbb;
+};
+
+enum ddr_type {
+	DDR_TYPE_NODDR = 0,
+	DDR_TYPE_LPDDR1 = 1,
+	DDR_TYPE_LPDDR2 = 2,
+	DDR_TYPE_PCDDR2 = 3,
+	DDR_TYPE_PCDDR3 = 4,
+	DDR_TYPE_LPDDR3 = 5,
+	DDR_TYPE_LPDDR4 = 6,
+	DDR_TYPE_LPDDR4X = 7,
+	DDR_TYPE_LPDDR5 = 8,
+	DDR_TYPE_LPDDR5X = 9,
+};
+
+/* The data structures below are NOT __packed on purpose! */
+
+/* Structs used across multiple versions */
+struct ddr_part_details {
+	__le16 revision_id1;
+	__le16 revision_id2;
+	__le16 width;
+	__le16 density;
+};
+
+struct ddr_freq_table {
+	__le32 freq_khz;
+	u8 enabled;
+};
+
+/* V3 */
+struct ddr_freq_plan_v3 {
+	struct ddr_freq_table ddr_freq[MAX_DDR_FREQ_NUM_V3];
+	u8 num_ddr_freqs;
+	phys_addr_t clk_period_address;
+};
+
+struct ddr_details_v3 {
+	u8 manufacturer_id;
+	u8 device_type;
+	struct ddr_part_details ddr_params[MAX_CHAN_NUM];
+	struct ddr_freq_plan_v3 ddr_freq_tbl;
+	u8 num_channels;
+};
+
+/* Some V3 structs have an additional frequency level */
+struct ddr_freq_plan_v3_14freqs {
+	struct ddr_freq_table ddr_freq[MAX_DDR_FREQ_NUM_V3 + 1];
+	u8 num_ddr_freqs;
+	phys_addr_t clk_period_address;
+};
+
+struct ddr_details_v3_14freqs {
+	u8 manufacturer_id;
+	u8 device_type;
+	struct ddr_part_details ddr_params[MAX_CHAN_NUM];
+	struct ddr_freq_plan_v3_14freqs ddr_freq_tbl;
+	u8 num_channels;
+};
+
+/* V4 */
+struct ddr_details_v4 {
+	u8 manufacturer_id;
+	u8 device_type;
+	struct ddr_part_details ddr_params[MAX_CHAN_NUM];
+	struct ddr_freq_plan_v3 ddr_freq_tbl;
+	u8 num_channels;
+	u8 num_ranks[MAX_CHAN_NUM];
+	u8 highest_bank_addr_bit[MAX_CHAN_NUM][MAX_RANK_NUM];
+};
+
+/* V5 */
+struct shub_freq_table {
+	u8 enable;
+	__le32 freq_khz;
+};
+
+struct shub_freq_plan_entry {
+	u8 num_shub_freqs;
+	struct shub_freq_table shub_freq[MAX_SHUB_ENTRIES];
+};
+
+struct ddr_xbl2quantum_smem_data {
+	phys_addr_t ssr_cookie_addr;
+	__le32 reserved[10];
+};
+
+struct ddr_freq_plan_v5 {
+	struct ddr_freq_table ddr_freq[MAX_DDR_FREQ_NUM_V5];
+	u8 num_ddr_freqs;
+	phys_addr_t clk_period_address;
+	__le32 max_nom_ddr_freq;
+};
+
+struct ddr_region_v5 {
+	__le64 start_address;
+	__le64 size;
+	__le64 mem_controller_address;
+	__le32 granule_size; /* MiB */
+	u8  ddr_rank;
+#define DDR_RANK_0	BIT(0)
+#define DDR_RANK_1	BIT(1)
+	u8  segments_start_index;
+	__le64 segments_start_offset;
+};
+
+struct ddr_regions_v5 {
+	__le32 ddr_region_num; /* We expect this to always be 4 or 6 */
+	__le64 ddr_rank0_size;
+	__le64 ddr_rank1_size;
+	__le64 ddr_cs0_start_addr;
+	__le64 ddr_cs1_start_addr;
+	__le32 highest_bank_addr_bit;
+	struct ddr_region_v5 ddr_region[] __counted_by_le(ddr_region_num);
+};
+
+struct ddr_details_v5 {
+	u8 manufacturer_id;
+	u8 device_type;
+	struct ddr_part_details ddr_params[MAX_CHAN_NUM];
+	struct ddr_freq_plan_v5 ddr_freq_tbl;
+	u8 num_channels;
+	u8 _padding;
+	struct ddr_regions_v5 ddr_regions;
+};
+
+/* V6 */
+struct ddr_misc_info_v6 {
+	__le32 dsf_version;
+	__le32 reserved[10];
+};
+
+/* V7 */
+struct ddr_details_v7 {
+	u8 manufacturer_id;
+	u8 device_type;
+	struct ddr_part_details ddr_params[MAX_CHAN_NUM];
+	struct ddr_freq_plan_v5 ddr_freq_tbl;
+	u8 num_channels;
+	u8 sct_config;
+	struct ddr_regions_v5 ddr_regions;
+};
+
+/**
+ * qcom_smem_dram_get_hbb(): Get the Highest bank address bit
+ *
+ * Context: Check qcom_smem_is_available() before calling this function.
+ * Because __dram * is initialized by smem_dram_parse(), which is in turn
+ * called from * qcom_smem_probe(), __dram will only be NULL if the data
+ * couldn't have been found/interpreted correctly.
+ *
+ * Return: highest bank bit on success, -ENODATA on failure.
+ */
+int qcom_smem_dram_get_hbb(void)
+{
+	if (!__dram || !__dram->hbb)
+		return -ENODATA;
+
+	if (__dram->hbb < DDR_HBB_MIN || __dram->hbb > DDR_HBB_MAX)
+		return -ENODATA;
+
+	return __dram->hbb;
+}
+EXPORT_SYMBOL_GPL(qcom_smem_dram_get_hbb);
+
+static void smem_dram_parse_v3_data(struct smem_dram *dram, void *data)
+{
+	struct ddr_details_v3 *details = data;
+
+	for (int i = 0; i < MAX_DDR_FREQ_NUM_V3; i++) {
+		struct ddr_freq_table *freq_entry = &details->ddr_freq_tbl.ddr_freq[i];
+
+		if (freq_entry->freq_khz && freq_entry->enabled) {
+			u32 freq_khz = le32_to_cpu(freq_entry->freq_khz);
+			dram->frequencies[dram->num_frequencies++] = 1000 * freq_khz;
+		}
+	}
+}
+
+static void smem_dram_parse_v3_14freqs_data(struct smem_dram *dram, void *data)
+{
+	struct ddr_details_v3_14freqs *details = data;
+
+	for (int i = 0; i < MAX_DDR_FREQ_NUM_V3 + 1; i++) {
+		struct ddr_freq_table *freq_entry = &details->ddr_freq_tbl.ddr_freq[i];
+
+		if (freq_entry->freq_khz && freq_entry->enabled)
+			dram->frequencies[dram->num_frequencies++] = 1000 * freq_entry->freq_khz;
+	}
+}
+
+static void smem_dram_parse_v4_data(struct smem_dram *dram, void *data)
+{
+	struct ddr_details_v4 *details = data;
+
+	/* Rank 0 channel 0 entry holds the correct value */
+	dram->hbb = details->highest_bank_addr_bit[0][0];
+
+	for (int i = 0; i < MAX_DDR_FREQ_NUM_V3; i++) {
+		struct ddr_freq_table *freq_entry = &details->ddr_freq_tbl.ddr_freq[i];
+
+		if (freq_entry->freq_khz && freq_entry->enabled) {
+			u32 freq_khz = le32_to_cpu(freq_entry->freq_khz);
+			dram->frequencies[dram->num_frequencies++] = 1000 * freq_khz;
+		}
+	}
+}
+
+static void smem_dram_parse_v5_data(struct smem_dram *dram, void *data)
+{
+	struct ddr_details_v5 *details = data;
+	struct ddr_regions_v5 *region = &details->ddr_regions;
+
+	dram->hbb = le32_to_cpu(region[0].highest_bank_addr_bit);
+
+	for (int i = 0; i < MAX_DDR_FREQ_NUM_V5; i++) {
+		struct ddr_freq_table *freq_entry = &details->ddr_freq_tbl.ddr_freq[i];
+
+		if (freq_entry->freq_khz && freq_entry->enabled) {
+			u32 freq_khz = le32_to_cpu(freq_entry->freq_khz);
+			dram->frequencies[dram->num_frequencies++] = 1000 * freq_khz;
+		}
+	}
+}
+
+static void smem_dram_parse_v7_data(struct smem_dram *dram, void *data)
+{
+	struct ddr_details_v7 *details = data;
+	struct ddr_regions_v5 *region = &details->ddr_regions;
+
+	dram->hbb = le32_to_cpu(region[0].highest_bank_addr_bit);
+
+	for (int i = 0; i < MAX_DDR_FREQ_NUM_V5; i++) {
+		struct ddr_freq_table *freq_entry = &details->ddr_freq_tbl.ddr_freq[i];
+
+		if (freq_entry->freq_khz && freq_entry->enabled) {
+			u32 freq_khz = le32_to_cpu(freq_entry->freq_khz);
+			dram->frequencies[dram->num_frequencies++] = 1000 * freq_khz;
+		}
+	}
+}
+
+/* The structure contains no version field, so we have to perform some guesswork.. */
+static int smem_dram_infer_struct_version(size_t size)
+{
+	/* Some early versions provided less bytes of less useful data */
+	if (size < sizeof(struct ddr_details_v3))
+		return -EINVAL;
+
+	if (size == sizeof(struct ddr_details_v3))
+		return INFO_V3;
+
+	if (size == sizeof(struct ddr_details_v3_14freqs))
+		return INFO_V3_WITH_14_FREQS;
+
+	if (size == sizeof(struct ddr_details_v4))
+		return INFO_V4;
+
+	if (size == sizeof(struct ddr_details_v5) +
+		    4 * sizeof(struct ddr_region_v5))
+		return INFO_V5;
+
+	if (size == sizeof(struct ddr_details_v5) +
+		    4 * sizeof(struct ddr_region_v5) +
+		    sizeof(struct ddr_xbl2quantum_smem_data) +
+		    sizeof(struct shub_freq_plan_entry))
+		return INFO_V5;
+
+	if (size == sizeof(struct ddr_details_v5) +
+		    6 * sizeof(struct ddr_region_v5))
+		return INFO_V5_WITH_6_REGIONS;
+
+	if (size == sizeof(struct ddr_details_v5) +
+		    6 * sizeof(struct ddr_region_v5) +
+		    sizeof(struct ddr_xbl2quantum_smem_data) +
+		    sizeof(struct shub_freq_plan_entry))
+		return INFO_V5_WITH_6_REGIONS;
+
+	if (size == sizeof(struct ddr_details_v5) +
+		    6 * sizeof(struct ddr_region_v5) +
+		    sizeof(struct ddr_misc_info_v6) +
+		    sizeof(struct shub_freq_plan_entry))
+		return INFO_V6;
+
+	if (size == sizeof(struct ddr_details_v7) +
+		    4 * sizeof(struct ddr_region_v5) +
+		    sizeof(struct ddr_misc_info_v6) +
+		    sizeof(struct shub_freq_plan_entry))
+		return INFO_V7;
+
+	if (size == sizeof(struct ddr_details_v7) +
+		    6 * sizeof(struct ddr_region_v5) +
+		    sizeof(struct ddr_misc_info_v6) +
+		    sizeof(struct shub_freq_plan_entry))
+		return INFO_V7_WITH_6_REGIONS;
+
+	return INFO_UNKNOWN;
+}
+
+static int smem_dram_frequencies_show(struct seq_file *s, void *unused)
+{
+	struct smem_dram *dram = s->private;
+
+	for (int i = 0; i < dram->num_frequencies; i++)
+		seq_printf(s, "%lu\n", dram->frequencies[i]);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(smem_dram_frequencies);
+
+static int smem_hbb_show(struct seq_file *s, void *unused)
+{
+	struct smem_dram *dram = s->private;
+
+	if (!dram->hbb)
+		return -EINVAL;
+
+	seq_printf(s, "%d\n", dram->hbb);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(smem_hbb);
+
+struct dentry *smem_dram_parse(struct qcom_smem *smem, struct device *dev)
+{
+	struct dentry *debugfs_dir;
+	enum ddr_info_version ver;
+	struct smem_dram *dram;
+	size_t actual_size;
+	void *data;
+
+	/* No need to check qcom_smem_is_available(), this func is called by the SMEM driver */
+	data = __qcom_smem_get(smem, QCOM_SMEM_HOST_ANY, SMEM_DDR_INFO_ID, &actual_size);
+	if (IS_ERR_OR_NULL(data))
+		return ERR_PTR(-ENODATA);
+
+	ver = smem_dram_infer_struct_version(actual_size);
+	if (ver < 0) {
+		/* Some SoCs don't provide data that's useful for us */
+		return ERR_PTR(-ENODATA);
+	} else if (ver == INFO_UNKNOWN) {
+		/* In other cases, we may not have added support for a newer struct revision */
+		dev_err(dev, "Found an unknown type of DRAM info struct (size = %zu)\n",
+			actual_size);
+		return ERR_PTR(-EINVAL);
+	}
+
+	dram = devm_kzalloc(dev, sizeof(*dram), GFP_KERNEL);
+	if (!dram)
+		return ERR_PTR(-ENOMEM);
+
+	switch (ver) {
+	case INFO_V3:
+		smem_dram_parse_v3_data(dram, data);
+		break;
+	case INFO_V3_WITH_14_FREQS:
+		smem_dram_parse_v3_14freqs_data(dram, data);
+		break;
+	case INFO_V4:
+		smem_dram_parse_v4_data(dram, data);
+		break;
+	case INFO_V5:
+	case INFO_V5_WITH_6_REGIONS:
+	case INFO_V6:
+		smem_dram_parse_v5_data(dram, data);
+		break;
+	case INFO_V7:
+	case INFO_V7_WITH_6_REGIONS:
+		smem_dram_parse_v7_data(dram, data);
+		break;
+	default:
+		return ERR_PTR(-EINVAL);
+	}
+
+	debugfs_dir = debugfs_create_dir("qcom_smem", NULL);
+	debugfs_create_file("dram_frequencies", 0444, debugfs_dir, dram,
+			    &smem_dram_frequencies_fops);
+	debugfs_create_file("hbb", 0444, debugfs_dir, dram, &smem_hbb_fops);
+
+	__dram = dram;
+
+	return debugfs_dir;
+}
diff --git a/include/linux/soc/qcom/smem.h b/include/linux/soc/qcom/smem.h
index f946e3beca21..3249cbd2016b 100644
--- a/include/linux/soc/qcom/smem.h
+++ b/include/linux/soc/qcom/smem.h
@@ -17,4 +17,6 @@ int qcom_smem_get_feature_code(u32 *code);
 
 int qcom_smem_bust_hwspin_lock_by_host(unsigned int host);
 
+int qcom_smem_dram_get_hbb(void);
+
 #endif

-- 
2.55.0

[PATCH v5 3/3] soc: qcom: ubwc: Get HBB from SMEM

[Konrad Dybcio]

From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

To make sure the correct settings for a given DRAM configuration get
applied, attempt to retrieve that data from SMEM (which happens to be
what the BSP kernel does, albeit with through convoluted means of the
bootloader altering the DT with this data).

Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 drivers/soc/qcom/ubwc_config.c | 40 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/ubwc_config.c b/drivers/soc/qcom/ubwc_config.c
index 0209a02c390f..23901a4c51f7 100644
--- a/drivers/soc/qcom/ubwc_config.c
+++ b/drivers/soc/qcom/ubwc_config.c
@@ -11,6 +11,7 @@
 #include <linux/platform_device.h>
 
 #include <linux/soc/qcom/ubwc.h>
+#include <linux/soc/qcom/smem.h>
 
 static const struct qcom_ubwc_cfg_data no_ubwc_data = {
 	/* no UBWC, no HBB */
@@ -168,9 +169,25 @@ static const struct of_device_id qcom_ubwc_configs[] __maybe_unused = {
 	{ }
 };
 
+static struct qcom_ubwc_cfg_data *cfg;
+static DEFINE_MUTEX(cfg_mutex);
+/**
+ * qcom_ubwc_config_get_data() - Retrieve UBWC data for the platform.
+ *
+ * Return: Pointer to valid struct qcom_ubwc_cfg_data on success, negative
+ * errno on failure. Note that this may return EPROBE_DEFER.
+ */
 const struct qcom_ubwc_cfg_data *qcom_ubwc_config_get_data(void)
 {
 	const struct qcom_ubwc_cfg_data *data;
+	int hbb;
+
+	guard(mutex)(&cfg_mutex);
+	if (cfg)
+		return cfg;
+
+	if (!qcom_smem_is_available())
+		return ERR_PTR(-EPROBE_DEFER);
 
 	data = of_machine_get_match_data(qcom_ubwc_configs);
 	if (!data) {
@@ -178,9 +195,30 @@ const struct qcom_ubwc_cfg_data *qcom_ubwc_config_get_data(void)
 		return ERR_PTR(-EINVAL);
 	}
 
-	return data;
+	hbb = qcom_smem_dram_get_hbb();
+	if (hbb == -ENODATA) {
+		/* Lack of HBB data is OK - it was only introduced later */
+		return data;
+	} else if (hbb < 0) {
+		pr_err("Couldn't get HBB data from SMEM: %d\n", hbb);
+		return ERR_PTR(hbb);
+	}
+
+	cfg = kmemdup(data, sizeof(*data), GFP_KERNEL);
+	if (!cfg)
+		return ERR_PTR(-ENOMEM);
+
+	cfg->highest_bank_bit = hbb;
+
+	return cfg;
 }
 EXPORT_SYMBOL_GPL(qcom_ubwc_config_get_data);
 
+static void __exit ubwc_config_exit(void)
+{
+	kfree(cfg);
+}
+module_exit(ubwc_config_exit);
+
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("UBWC config database for QTI SoCs");

-- 
2.55.0

Re: [PATCH v5 1/3] soc: qcom: smem: Use 'unsigned int' instead of 'unsigned'

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Incomplete conversion from 'unsigned' to 'unsigned int' leaves header and source file inconsistent.
--

--- Patch [1]: [PATCH v5 1/3] soc: qcom: smem: Use 'unsigned int' instead of 'unsigned' ---
commit 936d065428d0ec1f3bef2d47e4d7642e78104f77
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

soc: qcom: smem: Use 'unsigned int' instead of 'unsigned'

This commit converts the SMEM driver to use the 'unsigned int' type instead
of bare 'unsigned'. This is intended to resolve a checkpatch warning about
preferring less ambiguous types.

> diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
> index afb21a778fe7b..43b66120fc644 100644
> --- a/drivers/soc/qcom/smem.c
> +++ b/drivers/soc/qcom/smem.c
[ ... ]
> @@ -503,7 +503,7 @@ static int qcom_smem_alloc_global(struct qcom_smem *smem,
>   *
>   * Return: 0 on success, negative errno on failure.
>   */
> -int qcom_smem_alloc(unsigned host, unsigned item, size_t size)
> +int qcom_smem_alloc(unsigned int host, unsigned int item, size_t size)

[Severity: Low]
Should the function declarations in include/linux/soc/qcom/smem.h also be
updated to match these new definitions?

The public header still declares qcom_smem_alloc(), qcom_smem_get(), and 
qcom_smem_get_free_space() using bare unsigned types. Updating the header
as well would keep the API consistent and fully resolve the checkpatch
warnings for users of the header.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727-topic-smem_dramc-v5-0-66188b3e338d@oss.qualcomm.com?part=1

Re: [PATCH v5 2/3] soc: qcom: smem: Expose DDR data from SMEM

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

New issues:
- [High] Missing endianness conversion when reading `freq_khz` causes data corruption on big-endian architectures.
- [High] Global pointer `__dram` is left dangling when `qcom_smem` unbinds, leading to a Use-After-Free.
- [High] Double free / Use-After-Free of `smem->debugfs_dir` dentry if `socinfo` registration fails.
- [High] Hardware/firmware interface structures incorrectly use the variable-sized kernel type `phys_addr_t`, breaking strict ABI layout.
- [High] The DDR frequency calculation `1000 * freq_khz` overflows 32-bit unsigned arithmetic for high frequencies.

Pre-existing issues:
- [High] `qcom_smem_remove()` unconditionally passes `__smem->socinfo` to `platform_device_unregister()`, leading to a kernel panic if it is an error pointer.
--

--- Patch [2]: [PATCH v5 2/3] soc: qcom: smem: Expose DDR data from SMEM ---
commit 31a4d704c3d015e6e35b14ee7b507b87476be924
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

soc: qcom: smem: Expose DDR data from SMEM

Most modern Qualcomm platforms (>= SM8150) expose information about the
DDR memory present on the system via SMEM.

Link: https://git.codelinaro.org/clo/la/abl/tianocore/edk2/-/commit/1d11897d2cfcc7b85f28ff74c445018dbbecac7a

> diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
> index 43b66120fc644..4c689fc17011b 100644
> --- a/drivers/soc/qcom/smem.c
> +++ b/drivers/soc/qcom/smem.c
[ ... ]
> @@ -1236,19 +1246,26 @@ static int qcom_smem_probe(struct platform_device *pdev)
>  	if (ret < 0 && ret != -ENOENT)
>  		return ret;
>  
> +	smem->debugfs_dir = smem_dram_parse(smem, smem->dev);
> +
>  	__smem = smem;
>  
>  	smem->socinfo = platform_device_register_data(&pdev->dev, "qcom-socinfo",
>  						      PLATFORM_DEVID_NONE, NULL,
>  						      0);
> -	if (IS_ERR(smem->socinfo))
> +	if (IS_ERR(smem->socinfo)) {
> +		debugfs_remove_recursive(smem->debugfs_dir);
> +
>  		dev_dbg(&pdev->dev, "failed to register socinfo device\n");
> +	}
>  
>  	return 0;
>  }

[Severity: High]
When socinfo registration fails, debugfs_remove_recursive() is called, but
smem->debugfs_dir is not cleared to NULL. Since the probe function still
succeeds and returns 0, could this lead to a double-free later?

When the driver is eventually unbound, qcom_smem_remove() will call
debugfs_remove_recursive(__smem->debugfs_dir) again on the same stale pointer.

>  static void qcom_smem_remove(struct platform_device *pdev)
>  {
> +	debugfs_remove_recursive(__smem->debugfs_dir);
> +
>  	platform_device_unregister(__smem->socinfo);

[Severity: High]
This is a pre-existing issue, but does this code unconditionally pass an
ERR_PTR to platform_device_unregister() if the probe function failed to
register socinfo?

In qcom_smem_probe(), if platform_device_register_data() fails, it
stores an ERR_PTR in smem->socinfo and continues. Since
platform_device_unregister() only checks for NULL, deregistering an ERR_PTR
would cause a kernel panic.

> diff --git a/drivers/soc/qcom/smem_dramc.c b/drivers/soc/qcom/smem_dramc.c
> new file mode 100644
> index 0000000000000..63ebe7f6db590
> --- /dev/null
> +++ b/drivers/soc/qcom/smem_dramc.c
[ ... ]
> +/* V3 */
> +struct ddr_freq_plan_v3 {
> +	struct ddr_freq_table ddr_freq[MAX_DDR_FREQ_NUM_V3];
> +	u8 num_ddr_freqs;
> +	phys_addr_t clk_period_address;
> +};

[Severity: High]
Are these structures representing a fixed firmware ABI?

Using the kernel-dependent type phys_addr_t can cause the structure layout to
vary between 32-bit and 64-bit kernel builds. If the firmware layout is fixed,
would this cause the driver to read misaligned data when the kernel config
doesn't match the bootloader's ABI size? 

This also applies to struct ddr_freq_plan_v5 and 
struct ddr_xbl2quantum_smem_data further below.

[ ... ]
> +/**
> + * qcom_smem_dram_get_hbb(): Get the Highest bank address bit
> + *
> + * Context: Check qcom_smem_is_available() before calling this function.
> + * Because __dram * is initialized by smem_dram_parse(), which is in turn
> + * called from * qcom_smem_probe(), __dram will only be NULL if the data
> + * couldn't have been found/interpreted correctly.
> + *
> + * Return: highest bank bit on success, -ENODATA on failure.
> + */
> +int qcom_smem_dram_get_hbb(void)
> +{
> +	if (!__dram || !__dram->hbb)
> +		return -ENODATA;

[Severity: High]
Is there a risk of a use-after-free here if the qcom_smem driver is unbound?

The __dram pointer is allocated with devm_kzalloc() during probe. When the
device unbinds, devm automatically frees this memory. However, __dram is
never cleared to NULL in qcom_smem_remove().

If this exported API is called by another driver after qcom_smem unbinds,
it will unconditionally dereference the freed __dram pointer.

[ ... ]
> +static void smem_dram_parse_v3_14freqs_data(struct smem_dram *dram, void *data)
> +{
> +	struct ddr_details_v3_14freqs *details = data;
> +
> +	for (int i = 0; i < MAX_DDR_FREQ_NUM_V3 + 1; i++) {
> +		struct ddr_freq_table *freq_entry = &details->ddr_freq_tbl.ddr_freq[i];
> +
> +		if (freq_entry->freq_khz && freq_entry->enabled)
> +			dram->frequencies[dram->num_frequencies++] = 1000 * freq_entry->freq_khz;
> +	}
> +}

[Severity: High]
Does freq_entry->freq_khz need a le32_to_cpu() conversion here?

Other parsing functions in this file convert this __le32 field before use,
but this assignment reads it directly. On big-endian architectures, this might
result in byte-swapped values being used for DDR frequencies.

[ ... ]
> +static void smem_dram_parse_v5_data(struct smem_dram *dram, void *data)
> +{
> +	struct ddr_details_v5 *details = data;
> +	struct ddr_regions_v5 *region = &details->ddr_regions;
> +
> +	dram->hbb = le32_to_cpu(region[0].highest_bank_addr_bit);
> +
> +	for (int i = 0; i < MAX_DDR_FREQ_NUM_V5; i++) {
> +		struct ddr_freq_table *freq_entry = &details->ddr_freq_tbl.ddr_freq[i];
> +
> +		if (freq_entry->freq_khz && freq_entry->enabled) {
> +			u32 freq_khz = le32_to_cpu(freq_entry->freq_khz);
> +			dram->frequencies[dram->num_frequencies++] = 1000 * freq_khz;
> +		}
> +	}
> +}

[Severity: High]
Could this frequency calculation overflow 32-bit arithmetic?

The calculation 1000 * freq_khz multiplies a signed 32-bit integer by an
unsigned 32-bit integer. For high frequencies like modern LPDDR5X (e.g.,
4,800,000 kHz), the result will exceed the 32-bit maximum and wrap around
before being zero-extended and assigned to the 64-bit array element.

This identical calculation appears in the v3, v4, v5, and v7 parsing
functions. Should this cast to unsigned long before the multiplication?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727-topic-smem_dramc-v5-0-66188b3e338d@oss.qualcomm.com?part=2

Re: [PATCH v5 3/3] soc: qcom: ubwc: Get HBB from SMEM

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The Kconfig option `QCOM_UBWC_CONFIG` lacks a dependency on `QCOM_SMEM`, leading to linker errors when `QCOM_SMEM` is disabled or built as a module.
- [High] Unconditional probe deferral if SMEM is absent or fails to probe, breaking device initialization (such as display and GPU).
--

--- Patch [3]: [PATCH v5 3/3] soc: qcom: ubwc: Get HBB from SMEM ---
commit 20edaba770542f67d21a0b4400573bdcc45e195f
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

soc: qcom: ubwc: Get HBB from SMEM

This patch attempts to retrieve Highest Bank Bit (HBB) data from SMEM to
ensure correct DRAM configuration settings are applied. It updates the
UBWC configuration retrieval logic to first check for SMEM availability
and dynamically populate the HBB value if present.

> diff --git a/drivers/soc/qcom/ubwc_config.c b/drivers/soc/qcom/ubwc_config.c
> index 0209a02c390f0..23901a4c51f73 100644
> --- a/drivers/soc/qcom/ubwc_config.c
> +++ b/drivers/soc/qcom/ubwc_config.c
[ ... ]
> @@ -168,9 +169,25 @@ static const struct of_device_id qcom_ubwc_configs[] __maybe_unused = {
>  	{ }
>  };
>  
> +static struct qcom_ubwc_cfg_data *cfg;
> +static DEFINE_MUTEX(cfg_mutex);
> +/**
> + * qcom_ubwc_config_get_data() - Retrieve UBWC data for the platform.
> + *
> + * Return: Pointer to valid struct qcom_ubwc_cfg_data on success, negative
> + * errno on failure. Note that this may return EPROBE_DEFER.
> + */
>  const struct qcom_ubwc_cfg_data *qcom_ubwc_config_get_data(void)
>  {
>  	const struct qcom_ubwc_cfg_data *data;
> +	int hbb;
> +
> +	guard(mutex)(&cfg_mutex);
> +	if (cfg)
> +		return cfg;
> +
> +	if (!qcom_smem_is_available())
> +		return ERR_PTR(-EPROBE_DEFER);

[Severity: High]
If SMEM is absent on a platform or fails to probe (for example, if the
bootloader didn't initialize it and sets __smem to ERR_PTR(-ENODEV)),
qcom_smem_is_available() will always return false.

Will this cause dependent drivers to endlessly defer probe, completely
bypassing the valid static fallback data in qcom_ubwc_configs?

[Severity: High]
Since this patch introduces calls to qcom_smem_is_available() and
qcom_smem_dram_get_hbb(), which lack static inline stubs for the
!CONFIG_QCOM_SMEM case, could this cause a build failure?

For example, if a kernel is built with CONFIG_DRM_MSM=y (which selects
QCOM_UBWC_CONFIG) and CONFIG_QCOM_SMEM=m (or =n), will this result in
undefined reference errors during the vmlinux linking phase because
QCOM_UBWC_CONFIG lacks a dependency on QCOM_SMEM in drivers/soc/qcom/Kconfig?

>  
>  	data = of_machine_get_match_data(qcom_ubwc_configs);
>  	if (!data) {
> @@ -178,9 +195,30 @@ const struct qcom_ubwc_cfg_data *qcom_ubwc_config_get_data(void)
>  		return ERR_PTR(-EINVAL);
>  	}
>  
> -	return data;
> +	hbb = qcom_smem_dram_get_hbb();
> +	if (hbb == -ENODATA) {
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727-topic-smem_dramc-v5-0-66188b3e338d@oss.qualcomm.com?part=3

Re: [PATCH v5 1/3] soc: qcom: smem: Use 'unsigned int' instead of 'unsigned'

[Mukesh Ojha]

On Mon, Jul 27, 2026 at 11:59:27AM +0200, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> 
> checkpatch.pl reports:
> 
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> 
> Convert the SMEM driver to use the less ambiguous type instead.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Reviewed-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

-- 
-Mukesh Ojha