[PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Matthew Maurer]

Re-implements qcom-socinfo driver in Rust, using `Scoped`-based DebugFS
bindings.

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
This patch converts the QC socinfo driver to Rust, intended to be the
first Rust driver in Android that is owned by a vendor rather than the
platform.

This driver is currently quirk-compatible in DebugFS-exported values. If
the maintainers do not believe that maintaining the exact formats is a
benefit, we could simplify the code further by dropping some of the
custom formatting functions used to match the output.

I sent an earlier draft of this privately a while back that was much
rougher, but didn't get much feedback.

Now that we've got all the interfaces we need for this driver reasonably
available (DebugFS was the one that took a bit here), I wanted to bring
it up again, this time as an RFC. With the new features, the only place
it needs `unsafe` is to define an abstraction over `qcom_get_smem`.

I have tested this on a SM8650-HDK by diffing the contents of the
exported DebugFS and examining the files under /sys/bus/soc/devices/soc0

While I believe I have everything correctly exported via DebugFS, I have
not checked equivalence across a large swath of devices, only the one.
---
 drivers/soc/qcom/Kconfig             |   1 +
 drivers/soc/qcom/Makefile            |   2 +-
 drivers/soc/qcom/socinfo.c           | 931 -----------------------------------
 drivers/soc/qcom/socinfo/Makefile    |   2 +
 drivers/soc/qcom/socinfo/bindings.rs |  59 +++
 drivers/soc/qcom/socinfo/data.rs     | 424 ++++++++++++++++
 drivers/soc/qcom/socinfo/socinfo.rs  | 362 ++++++++++++++
 rust/bindgen_parameters              |   1 +
 rust/bindings/bindings_helper.h      |   6 +
 9 files changed, 856 insertions(+), 932 deletions(-)

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 2caadbbcf8307ff94f5afbdd1481e5e5e291749f..16d553f66f0cf1101d3a8c7d401d7c4d6a65dac7 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -222,6 +222,7 @@ config QCOM_SMSM
 config QCOM_SOCINFO
 	tristate "Qualcomm socinfo driver"
 	depends on QCOM_SMEM
+	depends on RUST
 	select SOC_BUS
 	help
 	 Say yes here to support the Qualcomm socinfo driver, providing
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index b7f1d2a5736748b8772c090fd24462fa91f321c6..6f6688c76a00a91ce99600f298a8e6e0fefed806 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -28,7 +28,7 @@ obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
 CFLAGS_smp2p.o := -I$(src)
 obj-$(CONFIG_QCOM_SMP2P)	+= smp2p.o
 obj-$(CONFIG_QCOM_SMSM)	+= smsm.o
-obj-$(CONFIG_QCOM_SOCINFO)	+= socinfo.o
+obj-$(CONFIG_QCOM_SOCINFO)	+= socinfo/
 obj-$(CONFIG_QCOM_SPM)		+= spm.o
 obj-$(CONFIG_QCOM_STATS)	+= qcom_stats.o
 obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o
diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
deleted file mode 100644
index 003a2304d535c2655db566c644342dbc387e24a9..0000000000000000000000000000000000000000
--- a/drivers/soc/qcom/socinfo.c
+++ /dev/null
@@ -1,931 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Copyright (c) 2009-2017, The Linux Foundation. All rights reserved.
- * Copyright (c) 2017-2019, Linaro Ltd.
- */
-
-#include <linux/debugfs.h>
-#include <linux/err.h>
-#include <linux/module.h>
-#include <linux/platform_device.h>
-#include <linux/random.h>
-#include <linux/slab.h>
-#include <linux/soc/qcom/smem.h>
-#include <linux/soc/qcom/socinfo.h>
-#include <linux/string.h>
-#include <linux/stringify.h>
-#include <linux/sys_soc.h>
-#include <linux/types.h>
-
-#include <linux/unaligned.h>
-
-#include <dt-bindings/arm/qcom,ids.h>
-
-/* Helper macros to create soc_id table */
-#define qcom_board_id(id) QCOM_ID_ ## id, __stringify(id)
-#define qcom_board_id_named(id, name) QCOM_ID_ ## id, (name)
-
-#ifdef CONFIG_DEBUG_FS
-#define SMEM_IMAGE_VERSION_BLOCKS_COUNT        32
-#define SMEM_IMAGE_VERSION_SIZE                4096
-#define SMEM_IMAGE_VERSION_NAME_SIZE           75
-#define SMEM_IMAGE_VERSION_VARIANT_SIZE        20
-#define SMEM_IMAGE_VERSION_OEM_SIZE            32
-
-/*
- * SMEM Image table indices
- */
-#define SMEM_IMAGE_TABLE_BOOT_INDEX     0
-#define SMEM_IMAGE_TABLE_TZ_INDEX       1
-#define SMEM_IMAGE_TABLE_TZSECAPP_INDEX	2
-#define SMEM_IMAGE_TABLE_RPM_INDEX      3
-#define SMEM_IMAGE_TABLE_SDI_INDEX      4
-#define SMEM_IMAGE_TABLE_HYP_INDEX      5
-#define SMEM_IMAGE_TABLE_ADSP1_INDEX	6
-#define SMEM_IMAGE_TABLE_ADSP2_INDEX	7
-#define SMEM_IMAGE_TABLE_CDSP2_INDEX	8
-#define SMEM_IMAGE_TABLE_APPSBL_INDEX	9
-#define SMEM_IMAGE_TABLE_APPS_INDEX     10
-#define SMEM_IMAGE_TABLE_MPSS_INDEX     11
-#define SMEM_IMAGE_TABLE_ADSP_INDEX     12
-#define SMEM_IMAGE_TABLE_CNSS_INDEX     13
-#define SMEM_IMAGE_TABLE_VIDEO_INDEX    14
-#define SMEM_IMAGE_TABLE_DSPS_INDEX     15
-#define SMEM_IMAGE_TABLE_CDSP_INDEX     16
-#define SMEM_IMAGE_TABLE_NPU_INDEX	17
-#define SMEM_IMAGE_TABLE_WPSS_INDEX     18
-#define SMEM_IMAGE_TABLE_CDSP1_INDEX    19
-#define SMEM_IMAGE_TABLE_GPDSP_INDEX    20
-#define SMEM_IMAGE_TABLE_GPDSP1_INDEX   21
-#define SMEM_IMAGE_TABLE_SENSORPD_INDEX	22
-#define SMEM_IMAGE_TABLE_AUDIOPD_INDEX	23
-#define SMEM_IMAGE_TABLE_OEMPD_INDEX	24
-#define SMEM_IMAGE_TABLE_CHARGERPD_INDEX	25
-#define SMEM_IMAGE_TABLE_OISPD_INDEX	26
-#define SMEM_IMAGE_TABLE_SOCCP_INDEX	27
-#define SMEM_IMAGE_TABLE_TME_INDEX	28
-#define SMEM_IMAGE_TABLE_GEARVM_INDEX	29
-#define SMEM_IMAGE_TABLE_UEFI_INDEX	30
-#define SMEM_IMAGE_TABLE_CDSP3_INDEX	31
-#define SMEM_IMAGE_TABLE_AUDIOPD_ADSP1_INDEX	32
-#define SMEM_IMAGE_TABLE_AUDIOPD_ADSP2_INDEX	33
-#define SMEM_IMAGE_TABLE_DCP_INDEX	34
-#define SMEM_IMAGE_TABLE_OOBS_INDEX	35
-#define SMEM_IMAGE_TABLE_OOBNS_INDEX	36
-#define SMEM_IMAGE_TABLE_DEVCFG_INDEX	37
-#define SMEM_IMAGE_TABLE_BTPD_INDEX	38
-#define SMEM_IMAGE_TABLE_QECP_INDEX	39
-
-#define SMEM_IMAGE_VERSION_TABLE       469
-#define SMEM_IMAGE_VERSION_TABLE_2	667
-
-/*
- * SMEM Image table names
- */
-static const char *const socinfo_image_names[] = {
-	[SMEM_IMAGE_TABLE_ADSP1_INDEX] = "adsp1",
-	[SMEM_IMAGE_TABLE_ADSP2_INDEX] = "adsp2",
-	[SMEM_IMAGE_TABLE_ADSP_INDEX] = "adsp",
-	[SMEM_IMAGE_TABLE_APPSBL_INDEX] = "appsbl",
-	[SMEM_IMAGE_TABLE_APPS_INDEX] = "apps",
-	[SMEM_IMAGE_TABLE_AUDIOPD_INDEX] = "audiopd",
-	[SMEM_IMAGE_TABLE_AUDIOPD_ADSP1_INDEX] = "audiopd_adsp1",
-	[SMEM_IMAGE_TABLE_AUDIOPD_ADSP2_INDEX] = "audiopd_adsp2",
-	[SMEM_IMAGE_TABLE_BOOT_INDEX] = "boot",
-	[SMEM_IMAGE_TABLE_BTPD_INDEX] = "btpd",
-	[SMEM_IMAGE_TABLE_CDSP1_INDEX] = "cdsp1",
-	[SMEM_IMAGE_TABLE_CDSP2_INDEX] = "cdsp2",
-	[SMEM_IMAGE_TABLE_CDSP3_INDEX] = "cdsp3",
-	[SMEM_IMAGE_TABLE_CDSP_INDEX] = "cdsp",
-	[SMEM_IMAGE_TABLE_CHARGERPD_INDEX] = "chargerpd",
-	[SMEM_IMAGE_TABLE_CNSS_INDEX] = "cnss",
-	[SMEM_IMAGE_TABLE_DCP_INDEX] = "dcp",
-	[SMEM_IMAGE_TABLE_DEVCFG_INDEX] = "devcfg",
-	[SMEM_IMAGE_TABLE_DSPS_INDEX] = "dsps",
-	[SMEM_IMAGE_TABLE_GEARVM_INDEX] = "gearvm",
-	[SMEM_IMAGE_TABLE_GPDSP1_INDEX] = "gpdsp1",
-	[SMEM_IMAGE_TABLE_GPDSP_INDEX] = "gpdsp",
-	[SMEM_IMAGE_TABLE_HYP_INDEX] = "hyp",
-	[SMEM_IMAGE_TABLE_MPSS_INDEX] = "mpss",
-	[SMEM_IMAGE_TABLE_NPU_INDEX] = "npu",
-	[SMEM_IMAGE_TABLE_OEMPD_INDEX] = "oempd",
-	[SMEM_IMAGE_TABLE_OISPD_INDEX] = "oispd",
-	[SMEM_IMAGE_TABLE_OOBNS_INDEX] = "oobns",
-	[SMEM_IMAGE_TABLE_OOBS_INDEX] = "oobs",
-	[SMEM_IMAGE_TABLE_QECP_INDEX] = "qecp",
-	[SMEM_IMAGE_TABLE_RPM_INDEX] = "rpm",
-	[SMEM_IMAGE_TABLE_SDI_INDEX] = "sdi",
-	[SMEM_IMAGE_TABLE_SENSORPD_INDEX] = "sensorpd",
-	[SMEM_IMAGE_TABLE_SOCCP_INDEX] = "soccp",
-	[SMEM_IMAGE_TABLE_TME_INDEX] = "tme",
-	[SMEM_IMAGE_TABLE_TZ_INDEX] = "tz",
-	[SMEM_IMAGE_TABLE_TZSECAPP_INDEX] = "tzsecapp",
-	[SMEM_IMAGE_TABLE_UEFI_INDEX] = "uefi",
-	[SMEM_IMAGE_TABLE_VIDEO_INDEX] = "video",
-	[SMEM_IMAGE_TABLE_WPSS_INDEX] = "wpss",
-};
-
-static const char *const pmic_models[] = {
-	[0]  = "Unknown PMIC model",
-	[1]  = "PM8941",
-	[2]  = "PM8841",
-	[3]  = "PM8019",
-	[4]  = "PM8226",
-	[5]  = "PM8110",
-	[6]  = "PMA8084",
-	[7]  = "PMI8962",
-	[8]  = "PMD9635",
-	[9]  = "PM8994",
-	[10] = "PMI8994",
-	[11] = "PM8916",
-	[12] = "PM8004",
-	[13] = "PM8909/PM8058",
-	[14] = "PM8028",
-	[15] = "PM8901",
-	[16] = "PM8950/PM8027",
-	[17] = "PMI8950/ISL9519",
-	[18] = "PMK8001/PM8921",
-	[19] = "PMI8996/PM8018",
-	[20] = "PM8998/PM8015",
-	[21] = "PMI8998/PM8014",
-	[22] = "PM8821",
-	[23] = "PM8038",
-	[24] = "PM8005/PM8922",
-	[25] = "PM8917/PM8937",
-	[26] = "PM660L",
-	[27] = "PM660",
-	[30] = "PM8150",
-	[31] = "PM8150L",
-	[32] = "PM8150B",
-	[33] = "PMK8002",
-	[36] = "PM8009",
-	[37] = "PMI632",
-	[38] = "PM8150C",
-	[40] = "PM6150",
-	[41] = "SMB2351",
-	[44] = "PM8008",
-	[45] = "PM6125",
-	[46] = "PM7250B",
-	[47] = "PMK8350",
-	[48] = "PM8350",
-	[49] = "PM8350C",
-	[50] = "PM8350B",
-	[51] = "PMR735A",
-	[52] = "PMR735B",
-	[54] = "PM6350",
-	[55] = "PM4125",
-	[58] = "PM8450",
-	[65] = "PM8010",
-	[69] = "PM8550VS",
-	[70] = "PM8550VE",
-	[71] = "PM8550B",
-	[72] = "PMR735D",
-	[73] = "PM8550",
-	[74] = "PMK8550",
-	[78] = "PMM8650AU",
-	[79] = "PMM8650AU_PSAIL",
-	[80] = "PM7550",
-	[82] = "PMC8380",
-	[83] = "SMB2360",
-	[91] = "PMIV0108",
-};
-
-struct socinfo_params {
-	u32 raw_device_family;
-	u32 hw_plat_subtype;
-	u32 accessory_chip;
-	u32 raw_device_num;
-	u32 chip_family;
-	u32 foundry_id;
-	u32 plat_ver;
-	u32 raw_ver;
-	u32 hw_plat;
-	u32 fmt;
-	u32 nproduct_id;
-	u32 num_clusters;
-	u32 ncluster_array_offset;
-	u32 num_subset_parts;
-	u32 nsubset_parts_array_offset;
-	u32 nmodem_supported;
-	u32 feature_code;
-	u32 pcode;
-	u32 oem_variant;
-	u32 num_func_clusters;
-	u32 boot_cluster;
-	u32 boot_core;
-	u32 raw_package_type;
-};
-
-struct smem_image_version {
-	char name[SMEM_IMAGE_VERSION_NAME_SIZE];
-	char variant[SMEM_IMAGE_VERSION_VARIANT_SIZE];
-	char pad;
-	char oem[SMEM_IMAGE_VERSION_OEM_SIZE];
-};
-#endif /* CONFIG_DEBUG_FS */
-
-struct qcom_socinfo {
-	struct soc_device *soc_dev;
-	struct soc_device_attribute attr;
-#ifdef CONFIG_DEBUG_FS
-	struct dentry *dbg_root;
-	struct socinfo_params info;
-#endif /* CONFIG_DEBUG_FS */
-};
-
-struct soc_id {
-	unsigned int id;
-	const char *name;
-};
-
-static const struct soc_id soc_id[] = {
-	{ qcom_board_id(MSM8260) },
-	{ qcom_board_id(MSM8660) },
-	{ qcom_board_id(APQ8060) },
-	{ qcom_board_id(MSM8960) },
-	{ qcom_board_id(APQ8064) },
-	{ qcom_board_id(MSM8930) },
-	{ qcom_board_id(MSM8630) },
-	{ qcom_board_id(MSM8230) },
-	{ qcom_board_id(APQ8030) },
-	{ qcom_board_id(MSM8627) },
-	{ qcom_board_id(MSM8227) },
-	{ qcom_board_id(MSM8660A) },
-	{ qcom_board_id(MSM8260A) },
-	{ qcom_board_id(APQ8060A) },
-	{ qcom_board_id(MSM8974) },
-	{ qcom_board_id(MSM8225) },
-	{ qcom_board_id(MSM8625) },
-	{ qcom_board_id(MPQ8064) },
-	{ qcom_board_id(MSM8960AB) },
-	{ qcom_board_id(APQ8060AB) },
-	{ qcom_board_id(MSM8260AB) },
-	{ qcom_board_id(MSM8660AB) },
-	{ qcom_board_id(MSM8930AA) },
-	{ qcom_board_id(MSM8630AA) },
-	{ qcom_board_id(MSM8230AA) },
-	{ qcom_board_id(MSM8626) },
-	{ qcom_board_id(MSM8610) },
-	{ qcom_board_id(APQ8064AB) },
-	{ qcom_board_id(MSM8930AB) },
-	{ qcom_board_id(MSM8630AB) },
-	{ qcom_board_id(MSM8230AB) },
-	{ qcom_board_id(APQ8030AB) },
-	{ qcom_board_id(MSM8226) },
-	{ qcom_board_id(MSM8526) },
-	{ qcom_board_id(APQ8030AA) },
-	{ qcom_board_id(MSM8110) },
-	{ qcom_board_id(MSM8210) },
-	{ qcom_board_id(MSM8810) },
-	{ qcom_board_id(MSM8212) },
-	{ qcom_board_id(MSM8612) },
-	{ qcom_board_id(MSM8112) },
-	{ qcom_board_id(MSM8125) },
-	{ qcom_board_id(MSM8225Q) },
-	{ qcom_board_id(MSM8625Q) },
-	{ qcom_board_id(MSM8125Q) },
-	{ qcom_board_id(APQ8064AA) },
-	{ qcom_board_id(APQ8084) },
-	{ qcom_board_id(MSM8130) },
-	{ qcom_board_id(MSM8130AA) },
-	{ qcom_board_id(MSM8130AB) },
-	{ qcom_board_id(MSM8627AA) },
-	{ qcom_board_id(MSM8227AA) },
-	{ qcom_board_id(APQ8074) },
-	{ qcom_board_id(MSM8274) },
-	{ qcom_board_id(MSM8674) },
-	{ qcom_board_id(MDM9635) },
-	{ qcom_board_id_named(MSM8974PRO_AC, "MSM8974PRO-AC") },
-	{ qcom_board_id(MSM8126) },
-	{ qcom_board_id(APQ8026) },
-	{ qcom_board_id(MSM8926) },
-	{ qcom_board_id(IPQ8062) },
-	{ qcom_board_id(IPQ8064) },
-	{ qcom_board_id(IPQ8066) },
-	{ qcom_board_id(IPQ8068) },
-	{ qcom_board_id(MSM8326) },
-	{ qcom_board_id(MSM8916) },
-	{ qcom_board_id(MSM8994) },
-	{ qcom_board_id_named(APQ8074PRO_AA, "APQ8074PRO-AA") },
-	{ qcom_board_id_named(APQ8074PRO_AB, "APQ8074PRO-AB") },
-	{ qcom_board_id_named(APQ8074PRO_AC, "APQ8074PRO-AC") },
-	{ qcom_board_id_named(MSM8274PRO_AA, "MSM8274PRO-AA") },
-	{ qcom_board_id_named(MSM8274PRO_AB, "MSM8274PRO-AB") },
-	{ qcom_board_id_named(MSM8274PRO_AC, "MSM8274PRO-AC") },
-	{ qcom_board_id_named(MSM8674PRO_AA, "MSM8674PRO-AA") },
-	{ qcom_board_id_named(MSM8674PRO_AB, "MSM8674PRO-AB") },
-	{ qcom_board_id_named(MSM8674PRO_AC, "MSM8674PRO-AC") },
-	{ qcom_board_id_named(MSM8974PRO_AA, "MSM8974PRO-AA") },
-	{ qcom_board_id_named(MSM8974PRO_AB, "MSM8974PRO-AB") },
-	{ qcom_board_id(APQ8028) },
-	{ qcom_board_id(MSM8128) },
-	{ qcom_board_id(MSM8228) },
-	{ qcom_board_id(MSM8528) },
-	{ qcom_board_id(MSM8628) },
-	{ qcom_board_id(MSM8928) },
-	{ qcom_board_id(MSM8510) },
-	{ qcom_board_id(MSM8512) },
-	{ qcom_board_id(MSM8936) },
-	{ qcom_board_id(MDM9640) },
-	{ qcom_board_id(MSM8939) },
-	{ qcom_board_id(APQ8036) },
-	{ qcom_board_id(APQ8039) },
-	{ qcom_board_id(MSM8236) },
-	{ qcom_board_id(MSM8636) },
-	{ qcom_board_id(MSM8909) },
-	{ qcom_board_id(MSM8996) },
-	{ qcom_board_id(APQ8016) },
-	{ qcom_board_id(MSM8216) },
-	{ qcom_board_id(MSM8116) },
-	{ qcom_board_id(MSM8616) },
-	{ qcom_board_id(MSM8992) },
-	{ qcom_board_id(APQ8092) },
-	{ qcom_board_id(APQ8094) },
-	{ qcom_board_id(MSM8209) },
-	{ qcom_board_id(MSM8208) },
-	{ qcom_board_id(MDM9209) },
-	{ qcom_board_id(MDM9309) },
-	{ qcom_board_id(MDM9609) },
-	{ qcom_board_id(MSM8239) },
-	{ qcom_board_id(MSM8952) },
-	{ qcom_board_id(APQ8009) },
-	{ qcom_board_id(MSM8956) },
-	{ qcom_board_id(MSM8929) },
-	{ qcom_board_id(MSM8629) },
-	{ qcom_board_id(MSM8229) },
-	{ qcom_board_id(APQ8029) },
-	{ qcom_board_id(APQ8056) },
-	{ qcom_board_id(MSM8609) },
-	{ qcom_board_id(APQ8076) },
-	{ qcom_board_id(MSM8976) },
-	{ qcom_board_id(IPQ8065) },
-	{ qcom_board_id(IPQ8069) },
-	{ qcom_board_id(MDM9650) },
-	{ qcom_board_id(MDM9655) },
-	{ qcom_board_id(MDM9250) },
-	{ qcom_board_id(MDM9255) },
-	{ qcom_board_id(MDM9350) },
-	{ qcom_board_id(APQ8052) },
-	{ qcom_board_id(MDM9607) },
-	{ qcom_board_id(APQ8096) },
-	{ qcom_board_id(MSM8998) },
-	{ qcom_board_id(MSM8953) },
-	{ qcom_board_id(MSM8937) },
-	{ qcom_board_id(APQ8037) },
-	{ qcom_board_id(MDM8207) },
-	{ qcom_board_id(MDM9207) },
-	{ qcom_board_id(MDM9307) },
-	{ qcom_board_id(MDM9628) },
-	{ qcom_board_id(MSM8909W) },
-	{ qcom_board_id(APQ8009W) },
-	{ qcom_board_id(MSM8996L) },
-	{ qcom_board_id(MSM8917) },
-	{ qcom_board_id(APQ8053) },
-	{ qcom_board_id(MSM8996SG) },
-	{ qcom_board_id(APQ8017) },
-	{ qcom_board_id(MSM8217) },
-	{ qcom_board_id(MSM8617) },
-	{ qcom_board_id(MSM8996AU) },
-	{ qcom_board_id(APQ8096AU) },
-	{ qcom_board_id(APQ8096SG) },
-	{ qcom_board_id(MSM8940) },
-	{ qcom_board_id(SDX201) },
-	{ qcom_board_id(SDM660) },
-	{ qcom_board_id(SDM630) },
-	{ qcom_board_id(APQ8098) },
-	{ qcom_board_id(MSM8920) },
-	{ qcom_board_id(SDM845) },
-	{ qcom_board_id(MDM9206) },
-	{ qcom_board_id(IPQ8074) },
-	{ qcom_board_id(SDA660) },
-	{ qcom_board_id(SDM658) },
-	{ qcom_board_id(SDA658) },
-	{ qcom_board_id(SDA630) },
-	{ qcom_board_id(MSM8905) },
-	{ qcom_board_id(SDX202) },
-	{ qcom_board_id(SDM670) },
-	{ qcom_board_id(SDM450) },
-	{ qcom_board_id(SM8150) },
-	{ qcom_board_id(SDA845) },
-	{ qcom_board_id(IPQ8072) },
-	{ qcom_board_id(IPQ8076) },
-	{ qcom_board_id(IPQ8078) },
-	{ qcom_board_id(SDM636) },
-	{ qcom_board_id(SDA636) },
-	{ qcom_board_id(SDM632) },
-	{ qcom_board_id(SDA632) },
-	{ qcom_board_id(SDA450) },
-	{ qcom_board_id(SDM439) },
-	{ qcom_board_id(SDM429) },
-	{ qcom_board_id(SM8250) },
-	{ qcom_board_id(SA8155) },
-	{ qcom_board_id(SDA439) },
-	{ qcom_board_id(SDA429) },
-	{ qcom_board_id(SM7150) },
-	{ qcom_board_id(SM7150P) },
-	{ qcom_board_id(IPQ8070) },
-	{ qcom_board_id(IPQ8071) },
-	{ qcom_board_id(QM215) },
-	{ qcom_board_id(IPQ8072A) },
-	{ qcom_board_id(IPQ8074A) },
-	{ qcom_board_id(IPQ8076A) },
-	{ qcom_board_id(IPQ8078A) },
-	{ qcom_board_id(SM6125) },
-	{ qcom_board_id(IPQ8070A) },
-	{ qcom_board_id(IPQ8071A) },
-	{ qcom_board_id(IPQ8172) },
-	{ qcom_board_id(IPQ8173) },
-	{ qcom_board_id(IPQ8174) },
-	{ qcom_board_id(IPQ6018) },
-	{ qcom_board_id(IPQ6028) },
-	{ qcom_board_id(SDM429W) },
-	{ qcom_board_id(SM4250) },
-	{ qcom_board_id(IPQ6000) },
-	{ qcom_board_id(IPQ6010) },
-	{ qcom_board_id(SC7180) },
-	{ qcom_board_id(SM6350) },
-	{ qcom_board_id(QCM2150) },
-	{ qcom_board_id(SDA429W) },
-	{ qcom_board_id(SM8350) },
-	{ qcom_board_id(QCM2290) },
-	{ qcom_board_id(SM7125) },
-	{ qcom_board_id(SM6115) },
-	{ qcom_board_id(IPQ5010) },
-	{ qcom_board_id(IPQ5018) },
-	{ qcom_board_id(IPQ5028) },
-	{ qcom_board_id(SC8280XP) },
-	{ qcom_board_id(IPQ6005) },
-	{ qcom_board_id(QRB5165) },
-	{ qcom_board_id(SM8450) },
-	{ qcom_board_id(SM7225) },
-	{ qcom_board_id(SA8295P) },
-	{ qcom_board_id(SA8540P) },
-	{ qcom_board_id(QCM4290) },
-	{ qcom_board_id(QCS4290) },
-	{ qcom_board_id(SM7325) },
-	{ qcom_board_id_named(SM8450_2, "SM8450") },
-	{ qcom_board_id_named(SM8450_3, "SM8450") },
-	{ qcom_board_id(SC7280) },
-	{ qcom_board_id(SC7180P) },
-	{ qcom_board_id(QCM6490) },
-	{ qcom_board_id(QCS6490) },
-	{ qcom_board_id(SM7325P) },
-	{ qcom_board_id(IPQ5000) },
-	{ qcom_board_id(IPQ0509) },
-	{ qcom_board_id(IPQ0518) },
-	{ qcom_board_id(SM6375) },
-	{ qcom_board_id(IPQ9514) },
-	{ qcom_board_id(IPQ9550) },
-	{ qcom_board_id(IPQ9554) },
-	{ qcom_board_id(IPQ9570) },
-	{ qcom_board_id(IPQ9574) },
-	{ qcom_board_id(SM8550) },
-	{ qcom_board_id(IPQ5016) },
-	{ qcom_board_id(IPQ9510) },
-	{ qcom_board_id(QRB4210) },
-	{ qcom_board_id(QRB2210) },
-	{ qcom_board_id(SAR2130P) },
-	{ qcom_board_id(SM8475) },
-	{ qcom_board_id(SM8475P) },
-	{ qcom_board_id(SA8255P) },
-	{ qcom_board_id(SA8775P) },
-	{ qcom_board_id(QRU1000) },
-	{ qcom_board_id(SM8475_2) },
-	{ qcom_board_id(QDU1000) },
-	{ qcom_board_id(X1E80100) },
-	{ qcom_board_id(SM8650) },
-	{ qcom_board_id(SM4450) },
-	{ qcom_board_id(SAR1130P) },
-	{ qcom_board_id(QDU1010) },
-	{ qcom_board_id(QRU1032) },
-	{ qcom_board_id(QRU1052) },
-	{ qcom_board_id(QRU1062) },
-	{ qcom_board_id(IPQ5332) },
-	{ qcom_board_id(IPQ5322) },
-	{ qcom_board_id(IPQ5312) },
-	{ qcom_board_id(IPQ5302) },
-	{ qcom_board_id(QCS8550) },
-	{ qcom_board_id(QCM8550) },
-	{ qcom_board_id(SM8750)  },
-	{ qcom_board_id(IPQ5300) },
-	{ qcom_board_id(SM7635) },
-	{ qcom_board_id(SM6650) },
-	{ qcom_board_id(SM6650P) },
-	{ qcom_board_id(IPQ5321) },
-	{ qcom_board_id(IPQ5424) },
-	{ qcom_board_id(QCM6690) },
-	{ qcom_board_id(QCS6690) },
-	{ qcom_board_id(SM8850) },
-	{ qcom_board_id(IPQ5404) },
-	{ qcom_board_id(QCS9100) },
-	{ qcom_board_id(QCS8300) },
-	{ qcom_board_id(QCS8275) },
-	{ qcom_board_id(QCS9075) },
-	{ qcom_board_id(QCS615) },
-};
-
-static const char *socinfo_machine(struct device *dev, unsigned int id)
-{
-	int idx;
-
-	for (idx = 0; idx < ARRAY_SIZE(soc_id); idx++) {
-		if (soc_id[idx].id == id)
-			return soc_id[idx].name;
-	}
-
-	return NULL;
-}
-
-#ifdef CONFIG_DEBUG_FS
-
-#define QCOM_OPEN(name, _func)						\
-static int qcom_open_##name(struct inode *inode, struct file *file)	\
-{									\
-	return single_open(file, _func, inode->i_private);		\
-}									\
-									\
-static const struct file_operations qcom_ ##name## _ops = {		\
-	.open = qcom_open_##name,					\
-	.read = seq_read,						\
-	.llseek = seq_lseek,						\
-	.release = single_release,					\
-}
-
-#define DEBUGFS_ADD(info, name)						\
-	debugfs_create_file(__stringify(name), 0444,			\
-			    qcom_socinfo->dbg_root,			\
-			    info, &qcom_ ##name## _ops)
-
-
-static int qcom_show_build_id(struct seq_file *seq, void *p)
-{
-	struct socinfo *socinfo = seq->private;
-
-	seq_printf(seq, "%s\n", socinfo->build_id);
-
-	return 0;
-}
-
-static int qcom_show_pmic_model(struct seq_file *seq, void *p)
-{
-	struct socinfo *socinfo = seq->private;
-	int model = SOCINFO_MINOR(le32_to_cpu(socinfo->pmic_model));
-
-	if (model < 0)
-		return -EINVAL;
-
-	if (model < ARRAY_SIZE(pmic_models) && pmic_models[model])
-		seq_printf(seq, "%s\n", pmic_models[model]);
-	else
-		seq_printf(seq, "unknown (%d)\n", model);
-
-	return 0;
-}
-
-static int qcom_show_pmic_model_array(struct seq_file *seq, void *p)
-{
-	struct socinfo *socinfo = seq->private;
-	unsigned int num_pmics = le32_to_cpu(socinfo->num_pmics);
-	unsigned int pmic_array_offset = le32_to_cpu(socinfo->pmic_array_offset);
-	int i;
-	void *ptr = socinfo;
-
-	ptr += pmic_array_offset;
-
-	/* No need for bounds checking, it happened at socinfo_debugfs_init */
-	for (i = 0; i < num_pmics; i++) {
-		unsigned int model = SOCINFO_MINOR(get_unaligned_le32(ptr + 2 * i * sizeof(u32)));
-		unsigned int die_rev = get_unaligned_le32(ptr + (2 * i + 1) * sizeof(u32));
-
-		if (model < ARRAY_SIZE(pmic_models) && pmic_models[model])
-			seq_printf(seq, "%s %u.%u\n", pmic_models[model],
-				   SOCINFO_MAJOR(die_rev),
-				   SOCINFO_MINOR(die_rev));
-		else
-			seq_printf(seq, "unknown (%d)\n", model);
-	}
-
-	return 0;
-}
-
-static int qcom_show_pmic_die_revision(struct seq_file *seq, void *p)
-{
-	struct socinfo *socinfo = seq->private;
-
-	seq_printf(seq, "%u.%u\n",
-		   SOCINFO_MAJOR(le32_to_cpu(socinfo->pmic_die_rev)),
-		   SOCINFO_MINOR(le32_to_cpu(socinfo->pmic_die_rev)));
-
-	return 0;
-}
-
-static int qcom_show_chip_id(struct seq_file *seq, void *p)
-{
-	struct socinfo *socinfo = seq->private;
-
-	seq_printf(seq, "%s\n", socinfo->chip_id);
-
-	return 0;
-}
-
-QCOM_OPEN(build_id, qcom_show_build_id);
-QCOM_OPEN(pmic_model, qcom_show_pmic_model);
-QCOM_OPEN(pmic_model_array, qcom_show_pmic_model_array);
-QCOM_OPEN(pmic_die_rev, qcom_show_pmic_die_revision);
-QCOM_OPEN(chip_id, qcom_show_chip_id);
-
-#define DEFINE_IMAGE_OPS(type)					\
-static int show_image_##type(struct seq_file *seq, void *p)		  \
-{								  \
-	struct smem_image_version *image_version = seq->private;  \
-	if (image_version->type[0] != '\0')			  \
-		seq_printf(seq, "%s\n", image_version->type);	  \
-	return 0;						  \
-}								  \
-static int open_image_##type(struct inode *inode, struct file *file)	  \
-{									  \
-	return single_open(file, show_image_##type, inode->i_private); \
-}									  \
-									  \
-static const struct file_operations qcom_image_##type##_ops = {	  \
-	.open = open_image_##type,					  \
-	.read = seq_read,						  \
-	.llseek = seq_lseek,						  \
-	.release = single_release,					  \
-}
-
-DEFINE_IMAGE_OPS(name);
-DEFINE_IMAGE_OPS(variant);
-DEFINE_IMAGE_OPS(oem);
-
-static void socinfo_debugfs_init(struct qcom_socinfo *qcom_socinfo,
-				 struct socinfo *info, size_t info_size)
-{
-	struct smem_image_version *versions;
-	struct dentry *dentry;
-	size_t size;
-	int i, j;
-	unsigned int num_pmics;
-	unsigned int pmic_array_offset;
-
-	qcom_socinfo->dbg_root = debugfs_create_dir("qcom_socinfo", NULL);
-
-	qcom_socinfo->info.fmt = __le32_to_cpu(info->fmt);
-
-	debugfs_create_x32("info_fmt", 0444, qcom_socinfo->dbg_root,
-			   &qcom_socinfo->info.fmt);
-
-	switch (qcom_socinfo->info.fmt) {
-	case SOCINFO_VERSION(0, 23):
-	case SOCINFO_VERSION(0, 22):
-	case SOCINFO_VERSION(0, 21):
-	case SOCINFO_VERSION(0, 20):
-		qcom_socinfo->info.raw_package_type = __le32_to_cpu(info->raw_package_type);
-		debugfs_create_u32("raw_package_type", 0444, qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.raw_package_type);
-		fallthrough;
-	case SOCINFO_VERSION(0, 19):
-		qcom_socinfo->info.num_func_clusters = __le32_to_cpu(info->num_func_clusters);
-		qcom_socinfo->info.boot_cluster = __le32_to_cpu(info->boot_cluster);
-		qcom_socinfo->info.boot_core = __le32_to_cpu(info->boot_core);
-
-		debugfs_create_u32("num_func_clusters", 0444, qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.num_func_clusters);
-		debugfs_create_u32("boot_cluster", 0444, qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.boot_cluster);
-		debugfs_create_u32("boot_core", 0444, qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.boot_core);
-		fallthrough;
-	case SOCINFO_VERSION(0, 18):
-	case SOCINFO_VERSION(0, 17):
-		qcom_socinfo->info.oem_variant = __le32_to_cpu(info->oem_variant);
-		debugfs_create_u32("oem_variant", 0444, qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.oem_variant);
-		fallthrough;
-	case SOCINFO_VERSION(0, 16):
-		qcom_socinfo->info.feature_code = __le32_to_cpu(info->feature_code);
-		qcom_socinfo->info.pcode = __le32_to_cpu(info->pcode);
-
-		debugfs_create_u32("feature_code", 0444, qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.feature_code);
-		debugfs_create_u32("pcode", 0444, qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.pcode);
-		fallthrough;
-	case SOCINFO_VERSION(0, 15):
-		qcom_socinfo->info.nmodem_supported = __le32_to_cpu(info->nmodem_supported);
-
-		debugfs_create_u32("nmodem_supported", 0444, qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.nmodem_supported);
-		fallthrough;
-	case SOCINFO_VERSION(0, 14):
-		qcom_socinfo->info.num_clusters = __le32_to_cpu(info->num_clusters);
-		qcom_socinfo->info.ncluster_array_offset = __le32_to_cpu(info->ncluster_array_offset);
-		qcom_socinfo->info.num_subset_parts = __le32_to_cpu(info->num_subset_parts);
-		qcom_socinfo->info.nsubset_parts_array_offset =
-			__le32_to_cpu(info->nsubset_parts_array_offset);
-
-		debugfs_create_u32("num_clusters", 0444, qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.num_clusters);
-		debugfs_create_u32("ncluster_array_offset", 0444, qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.ncluster_array_offset);
-		debugfs_create_u32("num_subset_parts", 0444, qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.num_subset_parts);
-		debugfs_create_u32("nsubset_parts_array_offset", 0444, qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.nsubset_parts_array_offset);
-		fallthrough;
-	case SOCINFO_VERSION(0, 13):
-		qcom_socinfo->info.nproduct_id = __le32_to_cpu(info->nproduct_id);
-
-		debugfs_create_u32("nproduct_id", 0444, qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.nproduct_id);
-		DEBUGFS_ADD(info, chip_id);
-		fallthrough;
-	case SOCINFO_VERSION(0, 12):
-		qcom_socinfo->info.chip_family =
-			__le32_to_cpu(info->chip_family);
-		qcom_socinfo->info.raw_device_family =
-			__le32_to_cpu(info->raw_device_family);
-		qcom_socinfo->info.raw_device_num =
-			__le32_to_cpu(info->raw_device_num);
-
-		debugfs_create_x32("chip_family", 0444, qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.chip_family);
-		debugfs_create_x32("raw_device_family", 0444,
-				   qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.raw_device_family);
-		debugfs_create_x32("raw_device_number", 0444,
-				   qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.raw_device_num);
-		fallthrough;
-	case SOCINFO_VERSION(0, 11):
-		num_pmics = le32_to_cpu(info->num_pmics);
-		pmic_array_offset = le32_to_cpu(info->pmic_array_offset);
-		if (pmic_array_offset + 2 * num_pmics * sizeof(u32) <= info_size)
-			DEBUGFS_ADD(info, pmic_model_array);
-		fallthrough;
-	case SOCINFO_VERSION(0, 10):
-	case SOCINFO_VERSION(0, 9):
-		qcom_socinfo->info.foundry_id = __le32_to_cpu(info->foundry_id);
-
-		debugfs_create_u32("foundry_id", 0444, qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.foundry_id);
-		fallthrough;
-	case SOCINFO_VERSION(0, 8):
-	case SOCINFO_VERSION(0, 7):
-		DEBUGFS_ADD(info, pmic_model);
-		DEBUGFS_ADD(info, pmic_die_rev);
-		fallthrough;
-	case SOCINFO_VERSION(0, 6):
-		qcom_socinfo->info.hw_plat_subtype =
-			__le32_to_cpu(info->hw_plat_subtype);
-
-		debugfs_create_u32("hardware_platform_subtype", 0444,
-				   qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.hw_plat_subtype);
-		fallthrough;
-	case SOCINFO_VERSION(0, 5):
-		qcom_socinfo->info.accessory_chip =
-			__le32_to_cpu(info->accessory_chip);
-
-		debugfs_create_u32("accessory_chip", 0444,
-				   qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.accessory_chip);
-		fallthrough;
-	case SOCINFO_VERSION(0, 4):
-		qcom_socinfo->info.plat_ver = __le32_to_cpu(info->plat_ver);
-
-		debugfs_create_u32("platform_version", 0444,
-				   qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.plat_ver);
-		fallthrough;
-	case SOCINFO_VERSION(0, 3):
-		qcom_socinfo->info.hw_plat = __le32_to_cpu(info->hw_plat);
-
-		debugfs_create_u32("hardware_platform", 0444,
-				   qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.hw_plat);
-		fallthrough;
-	case SOCINFO_VERSION(0, 2):
-		qcom_socinfo->info.raw_ver  = __le32_to_cpu(info->raw_ver);
-
-		debugfs_create_u32("raw_version", 0444, qcom_socinfo->dbg_root,
-				   &qcom_socinfo->info.raw_ver);
-		fallthrough;
-	case SOCINFO_VERSION(0, 1):
-		DEBUGFS_ADD(info, build_id);
-		break;
-	}
-
-	for (i = 0, j = 0; i < ARRAY_SIZE(socinfo_image_names); i++, j++) {
-		if (!socinfo_image_names[i])
-			continue;
-
-		if (i == 0) {
-			versions = qcom_smem_get(QCOM_SMEM_HOST_ANY,
-						 SMEM_IMAGE_VERSION_TABLE,
-						 &size);
-		} else if (i == 32) {
-			versions = qcom_smem_get(QCOM_SMEM_HOST_ANY,
-						 SMEM_IMAGE_VERSION_TABLE_2,
-						 &size);
-			if (IS_ERR(versions))
-				break;
-
-			j = 0;
-		}
-
-		dentry = debugfs_create_dir(socinfo_image_names[i],
-					    qcom_socinfo->dbg_root);
-		debugfs_create_file("name", 0444, dentry, &versions[j],
-				    &qcom_image_name_ops);
-		debugfs_create_file("variant", 0444, dentry, &versions[j],
-				    &qcom_image_variant_ops);
-		debugfs_create_file("oem", 0444, dentry, &versions[j],
-				    &qcom_image_oem_ops);
-	}
-}
-
-static void socinfo_debugfs_exit(struct qcom_socinfo *qcom_socinfo)
-{
-	debugfs_remove_recursive(qcom_socinfo->dbg_root);
-}
-#else
-static void socinfo_debugfs_init(struct qcom_socinfo *qcom_socinfo,
-				 struct socinfo *info, size_t info_size)
-{
-}
-static void socinfo_debugfs_exit(struct qcom_socinfo *qcom_socinfo) {  }
-#endif /* CONFIG_DEBUG_FS */
-
-static int qcom_socinfo_probe(struct platform_device *pdev)
-{
-	struct qcom_socinfo *qs;
-	struct socinfo *info;
-	size_t item_size;
-
-	info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID,
-			      &item_size);
-	if (IS_ERR(info)) {
-		dev_err(&pdev->dev, "Couldn't find socinfo\n");
-		return PTR_ERR(info);
-	}
-
-	qs = devm_kzalloc(&pdev->dev, sizeof(*qs), GFP_KERNEL);
-	if (!qs)
-		return -ENOMEM;
-
-	qs->attr.family = "Snapdragon";
-	qs->attr.machine = socinfo_machine(&pdev->dev,
-					   le32_to_cpu(info->id));
-	qs->attr.soc_id = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%u",
-					 le32_to_cpu(info->id));
-	qs->attr.revision = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%u.%u",
-					   SOCINFO_MAJOR(le32_to_cpu(info->ver)),
-					   SOCINFO_MINOR(le32_to_cpu(info->ver)));
-	if (!qs->attr.soc_id || !qs->attr.revision)
-		return -ENOMEM;
-
-	if (offsetofend(struct socinfo, serial_num) <= item_size) {
-		qs->attr.serial_number = devm_kasprintf(&pdev->dev, GFP_KERNEL,
-							"%u",
-							le32_to_cpu(info->serial_num));
-		if (!qs->attr.serial_number)
-			return -ENOMEM;
-	}
-
-	qs->soc_dev = soc_device_register(&qs->attr);
-	if (IS_ERR(qs->soc_dev))
-		return PTR_ERR(qs->soc_dev);
-
-	socinfo_debugfs_init(qs, info, item_size);
-
-	/* Feed the soc specific unique data into entropy pool */
-	add_device_randomness(info, item_size);
-
-	platform_set_drvdata(pdev, qs);
-
-	return 0;
-}
-
-static void qcom_socinfo_remove(struct platform_device *pdev)
-{
-	struct qcom_socinfo *qs = platform_get_drvdata(pdev);
-
-	soc_device_unregister(qs->soc_dev);
-
-	socinfo_debugfs_exit(qs);
-}
-
-static struct platform_driver qcom_socinfo_driver = {
-	.probe = qcom_socinfo_probe,
-	.remove = qcom_socinfo_remove,
-	.driver  = {
-		.name = "qcom-socinfo",
-	},
-};
-
-module_platform_driver(qcom_socinfo_driver);
-
-MODULE_DESCRIPTION("Qualcomm SoCinfo driver");
-MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("platform:qcom-socinfo");
diff --git a/drivers/soc/qcom/socinfo/Makefile b/drivers/soc/qcom/socinfo/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..8cdd77f46a22aeb97bcf1eeb90418db8f6352d3b
--- /dev/null
+++ b/drivers/soc/qcom/socinfo/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_QCOM_SOCINFO) += socinfo.o
diff --git a/drivers/soc/qcom/socinfo/bindings.rs b/drivers/soc/qcom/socinfo/bindings.rs
new file mode 100644
index 0000000000000000000000000000000000000000..edd0e0a416e6666b9e2bab2f00a41cf520ded5ce
--- /dev/null
+++ b/drivers/soc/qcom/socinfo/bindings.rs
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Google LLC.
+
+use kernel::error::from_err_ptr;
+use kernel::ffi::c_char;
+use kernel::macros::{AsBytes, FromBytes};
+use kernel::prelude::*;
+use kernel::transmute::AsBytes;
+
+pub(crate) fn qcom_smem_get(host: i32, item: u32) -> Result<&'static [u8]> {
+    let mut size = 0;
+    // SAFETY: qcom_smem_get only requires that the size pointer be a writable size_t,
+    // host and item are error checked in the qcom_smem module.
+    let err_ptr = unsafe { kernel::bindings::qcom_smem_get(host as u32, item, &mut size) };
+    let ptr = from_err_ptr(err_ptr)?;
+    // SAFETY: If qcom_smem_get does not return an error, the returned pointer points to a readable
+    // byte buffer with its size written into size. Because these buffers are derived from the
+    // static ranges in the DT, this buffer remains accessible even if the qcom_smem module is
+    // unloaded, so 'static is appropriate. The underlying buffer cannot mutate, so upgrading it
+    // to a reference is allowed.
+    Ok(unsafe { core::slice::from_raw_parts(ptr as *const u8, size) })
+}
+
+pub(crate) const SMEM_IMAGE_VERSION_TABLE: u32 = 469;
+pub(crate) const SMEM_IMAGE_VERSION_TABLE_2: u32 = 667;
+
+pub(crate) use kernel::bindings::socinfo;
+
+/// Produce `socinfo` by initializing from a byte buffer that may be too small.
+/// Code using the resulting value is expected to use the info_fmt field to determine
+/// which fields have meaningful values.
+pub(crate) fn socinfo_from_partial_bytes(soc_info_mem: &[u8]) -> socinfo {
+    let mut soc_info = socinfo::default();
+    let byte_view: &mut [u8] = soc_info.as_bytes_mut();
+    let len = core::cmp::min(soc_info_mem.len(), byte_view.len());
+    byte_view[..len].copy_from_slice(&soc_info_mem[..len]);
+    soc_info
+}
+
+#[derive(AsBytes, FromBytes)]
+#[repr(C)]
+pub(crate) struct PmicEntry {
+    pub(crate) model: u32,
+    pub(crate) die_rev: u32,
+}
+
+const SMEM_IMAGE_VERSION_NAME_SIZE: usize = 75;
+const SMEM_IMAGE_VERSION_VARIANT_SIZE: usize = 20;
+const SMEM_IMAGE_VERSION_OEM_SIZE: usize = 32;
+
+#[derive(AsBytes, FromBytes)]
+#[repr(C)]
+pub(crate) struct ImageVersion {
+    pub(crate) name: [c_char; SMEM_IMAGE_VERSION_NAME_SIZE],
+    pub(crate) variant: [c_char; SMEM_IMAGE_VERSION_VARIANT_SIZE],
+    pub(crate) pad: c_char,
+    pub(crate) oem: [c_char; SMEM_IMAGE_VERSION_OEM_SIZE],
+}
diff --git a/drivers/soc/qcom/socinfo/data.rs b/drivers/soc/qcom/socinfo/data.rs
new file mode 100644
index 0000000000000000000000000000000000000000..a3cb8698a3e0d840153d835df62c5b6c82dc6b0e
--- /dev/null
+++ b/drivers/soc/qcom/socinfo/data.rs
@@ -0,0 +1,424 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Google LLC.
+
+//! Data tables for QCom SocInfo driver
+use kernel::c_str;
+use kernel::prelude::*;
+
+macro_rules! id_entry {
+    ($id:ident) => {
+        kernel::macros::paste! {
+            SocId {
+                id: kernel::bindings::[<QCOM_ID_ $id>],
+                name: c_str!(stringify!($id)),
+            }
+        }
+    };
+    ($id:ident, $name:tt) => {
+        SocId {
+            id: kernel::macros::paste!(kernel::bindings::[<QCOM_ID_ $id>]), name: c_str!($name),
+        }
+    }
+}
+
+pub(crate) struct SocId {
+    pub(crate) id: u32,
+    pub(crate) name: &'static CStr,
+}
+
+pub(crate) static SOC_IDS: &[SocId] = &[
+    id_entry!(MSM8260),
+    id_entry!(MSM8660),
+    id_entry!(APQ8060),
+    id_entry!(MSM8960),
+    id_entry!(APQ8064),
+    id_entry!(MSM8930),
+    id_entry!(MSM8630),
+    id_entry!(MSM8230),
+    id_entry!(APQ8030),
+    id_entry!(MSM8627),
+    id_entry!(MSM8227),
+    id_entry!(MSM8660A),
+    id_entry!(MSM8260A),
+    id_entry!(APQ8060A),
+    id_entry!(MSM8974),
+    id_entry!(MSM8225),
+    id_entry!(MSM8625),
+    id_entry!(MPQ8064),
+    id_entry!(MSM8960AB),
+    id_entry!(APQ8060AB),
+    id_entry!(MSM8260AB),
+    id_entry!(MSM8660AB),
+    id_entry!(MSM8930AA),
+    id_entry!(MSM8630AA),
+    id_entry!(MSM8230AA),
+    id_entry!(MSM8626),
+    id_entry!(MSM8610),
+    id_entry!(APQ8064AB),
+    id_entry!(MSM8930AB),
+    id_entry!(MSM8630AB),
+    id_entry!(MSM8230AB),
+    id_entry!(APQ8030AB),
+    id_entry!(MSM8226),
+    id_entry!(MSM8526),
+    id_entry!(APQ8030AA),
+    id_entry!(MSM8110),
+    id_entry!(MSM8210),
+    id_entry!(MSM8810),
+    id_entry!(MSM8212),
+    id_entry!(MSM8612),
+    id_entry!(MSM8112),
+    id_entry!(MSM8125),
+    id_entry!(MSM8225Q),
+    id_entry!(MSM8625Q),
+    id_entry!(MSM8125Q),
+    id_entry!(APQ8064AA),
+    id_entry!(APQ8084),
+    id_entry!(MSM8130),
+    id_entry!(MSM8130AA),
+    id_entry!(MSM8130AB),
+    id_entry!(MSM8627AA),
+    id_entry!(MSM8227AA),
+    id_entry!(APQ8074),
+    id_entry!(MSM8274),
+    id_entry!(MSM8674),
+    id_entry!(MDM9635),
+    id_entry!(MSM8974PRO_AC, "MSM8974PRO-AC"),
+    id_entry!(MSM8126),
+    id_entry!(APQ8026),
+    id_entry!(MSM8926),
+    id_entry!(IPQ8062),
+    id_entry!(IPQ8064),
+    id_entry!(IPQ8066),
+    id_entry!(IPQ8068),
+    id_entry!(MSM8326),
+    id_entry!(MSM8916),
+    id_entry!(MSM8994),
+    id_entry!(APQ8074PRO_AA, "APQ8074PRO-AA"),
+    id_entry!(APQ8074PRO_AB, "APQ8074PRO-AB"),
+    id_entry!(APQ8074PRO_AC, "APQ8074PRO-AC"),
+    id_entry!(MSM8274PRO_AA, "MSM8274PRO-AA"),
+    id_entry!(MSM8274PRO_AB, "MSM8274PRO-AB"),
+    id_entry!(MSM8274PRO_AC, "MSM8274PRO-AC"),
+    id_entry!(MSM8674PRO_AA, "MSM8674PRO-AA"),
+    id_entry!(MSM8674PRO_AB, "MSM8674PRO-AB"),
+    id_entry!(MSM8674PRO_AC, "MSM8674PRO-AC"),
+    id_entry!(MSM8974PRO_AA, "MSM8974PRO-AA"),
+    id_entry!(MSM8974PRO_AB, "MSM8974PRO-AB"),
+    id_entry!(APQ8028),
+    id_entry!(MSM8128),
+    id_entry!(MSM8228),
+    id_entry!(MSM8528),
+    id_entry!(MSM8628),
+    id_entry!(MSM8928),
+    id_entry!(MSM8510),
+    id_entry!(MSM8512),
+    id_entry!(MSM8936),
+    id_entry!(MDM9640),
+    id_entry!(MSM8939),
+    id_entry!(APQ8036),
+    id_entry!(APQ8039),
+    id_entry!(MSM8236),
+    id_entry!(MSM8636),
+    id_entry!(MSM8909),
+    id_entry!(MSM8996),
+    id_entry!(APQ8016),
+    id_entry!(MSM8216),
+    id_entry!(MSM8116),
+    id_entry!(MSM8616),
+    id_entry!(MSM8992),
+    id_entry!(APQ8092),
+    id_entry!(APQ8094),
+    id_entry!(MSM8209),
+    id_entry!(MSM8208),
+    id_entry!(MDM9209),
+    id_entry!(MDM9309),
+    id_entry!(MDM9609),
+    id_entry!(MSM8239),
+    id_entry!(MSM8952),
+    id_entry!(APQ8009),
+    id_entry!(MSM8956),
+    id_entry!(MSM8929),
+    id_entry!(MSM8629),
+    id_entry!(MSM8229),
+    id_entry!(APQ8029),
+    id_entry!(APQ8056),
+    id_entry!(MSM8609),
+    id_entry!(APQ8076),
+    id_entry!(MSM8976),
+    id_entry!(IPQ8065),
+    id_entry!(IPQ8069),
+    id_entry!(MDM9650),
+    id_entry!(MDM9655),
+    id_entry!(MDM9250),
+    id_entry!(MDM9255),
+    id_entry!(MDM9350),
+    id_entry!(APQ8052),
+    id_entry!(MDM9607),
+    id_entry!(APQ8096),
+    id_entry!(MSM8998),
+    id_entry!(MSM8953),
+    id_entry!(MSM8937),
+    id_entry!(APQ8037),
+    id_entry!(MDM8207),
+    id_entry!(MDM9207),
+    id_entry!(MDM9307),
+    id_entry!(MDM9628),
+    id_entry!(MSM8909W),
+    id_entry!(APQ8009W),
+    id_entry!(MSM8996L),
+    id_entry!(MSM8917),
+    id_entry!(APQ8053),
+    id_entry!(MSM8996SG),
+    id_entry!(APQ8017),
+    id_entry!(MSM8217),
+    id_entry!(MSM8617),
+    id_entry!(MSM8996AU),
+    id_entry!(APQ8096AU),
+    id_entry!(APQ8096SG),
+    id_entry!(MSM8940),
+    id_entry!(SDX201),
+    id_entry!(SDM660),
+    id_entry!(SDM630),
+    id_entry!(APQ8098),
+    id_entry!(MSM8920),
+    id_entry!(SDM845),
+    id_entry!(MDM9206),
+    id_entry!(IPQ8074),
+    id_entry!(SDA660),
+    id_entry!(SDM658),
+    id_entry!(SDA658),
+    id_entry!(SDA630),
+    id_entry!(MSM8905),
+    id_entry!(SDX202),
+    id_entry!(SDM670),
+    id_entry!(SDM450),
+    id_entry!(SM8150),
+    id_entry!(SDA845),
+    id_entry!(IPQ8072),
+    id_entry!(IPQ8076),
+    id_entry!(IPQ8078),
+    id_entry!(SDM636),
+    id_entry!(SDA636),
+    id_entry!(SDM632),
+    id_entry!(SDA632),
+    id_entry!(SDA450),
+    id_entry!(SDM439),
+    id_entry!(SDM429),
+    id_entry!(SM8250),
+    id_entry!(SA8155),
+    id_entry!(SDA439),
+    id_entry!(SDA429),
+    id_entry!(SM7150),
+    id_entry!(SM7150P),
+    id_entry!(IPQ8070),
+    id_entry!(IPQ8071),
+    id_entry!(QM215),
+    id_entry!(IPQ8072A),
+    id_entry!(IPQ8074A),
+    id_entry!(IPQ8076A),
+    id_entry!(IPQ8078A),
+    id_entry!(SM6125),
+    id_entry!(IPQ8070A),
+    id_entry!(IPQ8071A),
+    id_entry!(IPQ8172),
+    id_entry!(IPQ8173),
+    id_entry!(IPQ8174),
+    id_entry!(IPQ6018),
+    id_entry!(IPQ6028),
+    id_entry!(SDM429W),
+    id_entry!(SM4250),
+    id_entry!(IPQ6000),
+    id_entry!(IPQ6010),
+    id_entry!(SC7180),
+    id_entry!(SM6350),
+    id_entry!(QCM2150),
+    id_entry!(SDA429W),
+    id_entry!(SM8350),
+    id_entry!(QCM2290),
+    id_entry!(SM7125),
+    id_entry!(SM6115),
+    id_entry!(IPQ5010),
+    id_entry!(IPQ5018),
+    id_entry!(IPQ5028),
+    id_entry!(SC8280XP),
+    id_entry!(IPQ6005),
+    id_entry!(QRB5165),
+    id_entry!(SM8450),
+    id_entry!(SM7225),
+    id_entry!(SA8295P),
+    id_entry!(SA8540P),
+    id_entry!(QCM4290),
+    id_entry!(QCS4290),
+    id_entry!(SM7325),
+    id_entry!(SM8450_2, "SM8450"),
+    id_entry!(SM8450_3, "SM8450"),
+    id_entry!(SC7280),
+    id_entry!(SC7180P),
+    id_entry!(QCM6490),
+    id_entry!(QCS6490),
+    id_entry!(SM7325P),
+    id_entry!(IPQ5000),
+    id_entry!(IPQ0509),
+    id_entry!(IPQ0518),
+    id_entry!(SM6375),
+    id_entry!(IPQ9514),
+    id_entry!(IPQ9550),
+    id_entry!(IPQ9554),
+    id_entry!(IPQ9570),
+    id_entry!(IPQ9574),
+    id_entry!(SM8550),
+    id_entry!(IPQ5016),
+    id_entry!(IPQ9510),
+    id_entry!(QRB4210),
+    id_entry!(QRB2210),
+    id_entry!(SAR2130P),
+    id_entry!(SM8475),
+    id_entry!(SM8475P),
+    id_entry!(SA8255P),
+    id_entry!(SA8775P),
+    id_entry!(QRU1000),
+    id_entry!(SM8475_2),
+    id_entry!(QDU1000),
+    id_entry!(X1E80100),
+    id_entry!(SM8650),
+    id_entry!(SM4450),
+    id_entry!(SAR1130P),
+    id_entry!(QDU1010),
+    id_entry!(QRU1032),
+    id_entry!(QRU1052),
+    id_entry!(QRU1062),
+    id_entry!(IPQ5332),
+    id_entry!(IPQ5322),
+    id_entry!(IPQ5312),
+    id_entry!(IPQ5302),
+    id_entry!(QCS8550),
+    id_entry!(QCM8550),
+    id_entry!(SM8750),
+    id_entry!(IPQ5300),
+    id_entry!(SM7635),
+    id_entry!(SM6650),
+    id_entry!(SM6650P),
+    id_entry!(IPQ5321),
+    id_entry!(IPQ5424),
+    id_entry!(QCM6690),
+    id_entry!(QCS6690),
+    id_entry!(SM8850),
+    id_entry!(IPQ5404),
+    id_entry!(QCS9100),
+    id_entry!(QCS8300),
+    id_entry!(QCS8275),
+    id_entry!(QCS9075),
+    id_entry!(QCS615),
+];
+
+pub(crate) const PMIC_MODELS: [Option<&str>; 92] = {
+    let mut models = [None; 92];
+    models[0] = Some("Unknown PMIC model");
+    models[1] = Some("PM8941");
+    models[2] = Some("PM8841");
+    models[3] = Some("PM8019");
+    models[4] = Some("PM8226");
+    models[5] = Some("PM8110");
+    models[6] = Some("PMA8084");
+    models[7] = Some("PMI8962");
+    models[8] = Some("PMD9635");
+    models[9] = Some("PM8994");
+    models[10] = Some("PMI8994");
+    models[11] = Some("PM8916");
+    models[12] = Some("PM8004");
+    models[13] = Some("PM8909/PM8058");
+    models[14] = Some("PM8028");
+    models[15] = Some("PM8901");
+    models[16] = Some("PM8950/PM8027");
+    models[17] = Some("PMI8950/ISL9519");
+    models[18] = Some("PMK8001/PM8921");
+    models[19] = Some("PMI8996/PM8018");
+    models[20] = Some("PM8998/PM8015");
+    models[21] = Some("PMI8998/PM8014");
+    models[22] = Some("PM8821");
+    models[23] = Some("PM8038");
+    models[24] = Some("PM8005/PM8922");
+    models[25] = Some("PM8917/PM8937");
+    models[26] = Some("PM660L");
+    models[27] = Some("PM660");
+    models[30] = Some("PM8150");
+    models[31] = Some("PM8150L");
+    models[32] = Some("PM8150B");
+    models[33] = Some("PMK8002");
+    models[36] = Some("PM8009");
+    models[37] = Some("PMI632");
+    models[38] = Some("PM8150C");
+    models[40] = Some("PM6150");
+    models[41] = Some("SMB2351");
+    models[44] = Some("PM8008");
+    models[45] = Some("PM6125");
+    models[46] = Some("PM7250B");
+    models[47] = Some("PMK8350");
+    models[48] = Some("PM8350");
+    models[49] = Some("PM8350C");
+    models[50] = Some("PM8350B");
+    models[51] = Some("PMR735A");
+    models[52] = Some("PMR735B");
+    models[54] = Some("PM6350");
+    models[55] = Some("PM4125");
+    models[58] = Some("PM8450");
+    models[65] = Some("PM8010");
+    models[69] = Some("PM8550VS");
+    models[70] = Some("PM8550VE");
+    models[71] = Some("PM8550B");
+    models[72] = Some("PMR735D");
+    models[73] = Some("PM8550");
+    models[74] = Some("PMK8550");
+    models[78] = Some("PMM8650AU");
+    models[79] = Some("PMM8650AU_PSAIL");
+    models[80] = Some("PM7550");
+    models[82] = Some("PMC8380");
+    models[83] = Some("SMB2360");
+    models[91] = Some("PMIV0108");
+    models
+};
+
+pub(crate) const IMAGE_NAMES: &[&CStr] = &[
+    c_str!("boot"),
+    c_str!("tz"),
+    c_str!("tzsecapp"),
+    c_str!("rpm"),
+    c_str!("sdi"),
+    c_str!("hyp"),
+    c_str!("adsp1"),
+    c_str!("adsp2"),
+    c_str!("cdsp2"),
+    c_str!("appsbl"),
+    c_str!("apps"),
+    c_str!("mpss"),
+    c_str!("adsp"),
+    c_str!("cnss"),
+    c_str!("video"),
+    c_str!("dsps"),
+    c_str!("cdsp"),
+    c_str!("npu"),
+    c_str!("wpss"),
+    c_str!("cdsp1"),
+    c_str!("gpdsp"),
+    c_str!("gpdsp1"),
+    c_str!("sensorpd"),
+    c_str!("audiopd"),
+    c_str!("oempd"),
+    c_str!("chargerpd"),
+    c_str!("oispd"),
+    c_str!("soccp"),
+    c_str!("tme"),
+    c_str!("gearvm"),
+    c_str!("uefi"),
+    c_str!("cdsp3"),
+    c_str!("audiopd"),
+    c_str!("audiopd"),
+    c_str!("dcp"),
+    c_str!("oobs"),
+    c_str!("oobns"),
+    c_str!("devcfg"),
+    c_str!("btpd"),
+    c_str!("qecp"),
+];
diff --git a/drivers/soc/qcom/socinfo/socinfo.rs b/drivers/soc/qcom/socinfo/socinfo.rs
new file mode 100644
index 0000000000000000000000000000000000000000..bff1bf8fd672e3c461352075aa85ef8fdedff953
--- /dev/null
+++ b/drivers/soc/qcom/socinfo/socinfo.rs
@@ -0,0 +1,362 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Google LLC.
+
+//! Re-implementation of Qualcomm's Socinfo driver in Rust
+use core::fmt;
+use core::fmt::Formatter;
+use kernel::debugfs::{Scope, ScopedDir};
+use kernel::device::Core;
+use kernel::module_platform_driver;
+use kernel::platform::{self, Device};
+use kernel::prelude::{fmt, pin_data, Error, PinInit, Result};
+use kernel::soc;
+use kernel::str::{CStr, CStrExt, CString};
+use kernel::transmute::FromBytes;
+use kernel::{c_str, pr_warn, try_pin_init};
+use pin_init::pin_init_scope;
+
+mod bindings;
+mod data;
+
+use bindings::*;
+use data::{IMAGE_NAMES, PMIC_MODELS, SOC_IDS};
+
+module_platform_driver! {
+    type: QcomSocInfo,
+    name: "qcom-socinfo",
+    authors: ["Matthew Maurer"],
+    description: "Qualcomm SoCinfo driver",
+    license: "GPL",
+    alias: ["platform:qcom-socinfo"],
+}
+
+#[pin_data]
+struct QcomSocInfo {
+    #[pin]
+    registration: soc::DeviceRegistration,
+    #[pin]
+    params: Scope<Params>,
+}
+
+fn fmt_pmic_model(model: &u32, f: &mut Formatter<'_>) -> fmt::Result {
+    let model = SocInfo::version_split(*model).1;
+    if let Some(Some(model)) = PMIC_MODELS.get(model as usize) {
+        writeln!(f, "{model}")
+    } else {
+        writeln!(f, "unknown ({})", model)
+    }
+}
+
+fn fmt_pmic_die_rev(rev: &u32, f: &mut Formatter<'_>) -> fmt::Result {
+    let (major, minor) = SocInfo::version_split(*rev);
+    writeln!(f, "{major}.{minor}")
+}
+
+fn fmt_pmic_model_array(array: &&'static [PmicEntry], f: &mut Formatter<'_>) -> fmt::Result {
+    for pmic_entry in *array {
+        let (die_rev_major, die_rev_minor) =
+            SocInfo::version_split(u32::from_le(pmic_entry.die_rev));
+        let model_idx = SocInfo::version_split(pmic_entry.model).1 as usize;
+        if let Some(Some(model)) = PMIC_MODELS.get(model_idx) {
+            writeln!(f, "{model} {die_rev_major}.{die_rev_minor}")?
+        } else {
+            writeln!(f, "unknown ({})", pmic_entry.model)?
+        }
+    }
+    Ok(())
+}
+
+#[derive(Default)]
+struct Params {
+    info_fmt: u32,
+    build_id: [u8; 32],
+    raw_version: Option<u32>,
+    hardware_platform: Option<u32>,
+    platform_version: Option<u32>,
+    accessory_chip: Option<u32>,
+    hardware_platform_subtype: Option<u32>,
+    pmic_model: Option<u32>,
+    pmic_die_rev: Option<u32>,
+    foundry_id: Option<u32>,
+    pmic_model_array: Option<&'static [PmicEntry]>,
+    chip_family: Option<u32>,
+    raw_device_family: Option<u32>,
+    raw_device_number: Option<u32>,
+    nproduct_id: Option<u32>,
+    chip_id: Option<[u8; 32]>,
+    num_clusters: Option<u32>,
+    ncluster_array_offset: Option<u32>,
+    num_subset_parts: Option<u32>,
+    nsubset_parts_array_offset: Option<u32>,
+    nmodem_supported: Option<u32>,
+    feature_code: Option<u32>,
+    pcode: Option<u32>,
+    oem_variant: Option<u32>,
+    boot_core: Option<u32>,
+    boot_cluster: Option<u32>,
+    num_func_clusters: Option<u32>,
+    raw_package_type: Option<u32>,
+    versions: Option<&'static [ImageVersion]>,
+    versions2: Option<&'static [ImageVersion]>,
+}
+
+#[derive(Copy, Clone)]
+struct SocInfo<'a> {
+    soc_info: socinfo,
+    soc_info_mem: &'a [u8],
+    version_mem: &'a [u8],
+    version_mem2: Option<&'a [u8]>,
+}
+
+impl<'a> SocInfo<'a> {
+    fn from_mem(
+        soc_info_mem: &'a [u8],
+        version_mem: &'a [u8],
+        version_mem2: Option<&'a [u8]>,
+    ) -> Self {
+        Self {
+            soc_info: socinfo_from_partial_bytes(soc_info_mem),
+            soc_info_mem,
+            version_mem,
+            version_mem2,
+        }
+    }
+    fn id(&self) -> u32 {
+        u32::from_le(self.soc_info.id)
+    }
+    fn version_split(ver: u32) -> (u16, u16) {
+        let major = (ver >> 16) as u16;
+        let minor = (ver & 0xFFFF) as u16;
+        (major, minor)
+    }
+    fn version_fuse(major: u16, minor: u16) -> u32 {
+        (u32::from(major) << 16) | u32::from(minor)
+    }
+    fn version(&self) -> (u16, u16) {
+        Self::version_split(self.soc_info.ver)
+    }
+    fn serial(&self) -> u32 {
+        u32::from_le(self.soc_info.id)
+    }
+    fn machine(&self) -> Result<Option<CString>> {
+        for soc in SOC_IDS {
+            if soc.id == self.id() {
+                return Ok(Some(soc.name.to_cstring()?));
+            }
+        }
+        Ok(None)
+    }
+    fn device_attribute(&self) -> Result<soc::DeviceAttribute> {
+        Ok(soc::DeviceAttribute {
+            family: Some(c_str!("Snapdragon").to_cstring()?),
+            machine: self.machine()?,
+            revision: Some(CString::try_from_fmt(fmt!(
+                "{}.{}",
+                self.version().0,
+                self.version().1
+            ))?),
+            serial_number: Some(CString::try_from_fmt(fmt!("{}", self.serial()))?),
+            soc_id: Some(CString::try_from_fmt(fmt!("{}", self.id()))?),
+        })
+    }
+}
+
+macro_rules! u32_le_versioned {
+    { $params:expr, $self:ident,
+        [ $( { $major:expr, $minor:expr, { $( $dst:ident: $src:ident ),* } } ),*  ] } => {$(
+        if $params.info_fmt >= SocInfo::version_fuse($major, $minor) {
+            $( $params.$dst = Some(u32::from_le($self.soc_info.$src).into()) );*
+        }
+    )*}
+}
+
+impl SocInfo<'static> {
+    fn build_params(&self) -> Result<Params> {
+        let mut params = Params {
+            build_id: self.soc_info.build_id,
+            info_fmt: u32::from_le(self.soc_info.fmt),
+            ..Default::default()
+        };
+        u32_le_versioned! { params, self, [
+            {0, 2, { raw_version: raw_ver }},
+            {0, 3, { hardware_platform: hw_plat }},
+            {0, 4, { platform_version: plat_ver }},
+            {0, 5, { accessory_chip: accessory_chip }},
+            {0, 6, { hardware_platform_subtype: hw_plat_subtype }},
+            {0, 7, { pmic_model: pmic_model, pmic_die_rev: pmic_die_rev }},
+            {0, 9, { foundry_id: foundry_id }},
+            {0, 12, {
+                chip_family: chip_family,
+                raw_device_family: raw_device_family,
+                raw_device_number: raw_device_num
+            }},
+            {0, 13, { nproduct_id: nproduct_id }},
+            {0, 14, {
+                num_clusters: num_clusters,
+                ncluster_array_offset: ncluster_array_offset,
+                num_subset_parts: num_subset_parts,
+                nsubset_parts_array_offset: nsubset_parts_array_offset
+            }},
+            {0, 15, { nmodem_supported: nmodem_supported }},
+            {0, 16, { feature_code: feature_code, pcode: pcode }},
+            {0, 17, { oem_variant: oem_variant }},
+            {0, 19, {
+                boot_core: boot_core,
+                boot_cluster: boot_cluster,
+                num_func_clusters: num_func_clusters
+            }},
+            {0, 20, { raw_package_type: raw_package_type }}
+        ]};
+        if params.info_fmt >= SocInfo::version_fuse(0, 11) {
+            let offset = u32::from_le(self.soc_info.pmic_array_offset) as usize;
+            let num_pmics = u32::from_le(self.soc_info.num_pmics) as usize;
+            let size = num_pmics * core::mem::size_of::<PmicEntry>();
+            params.pmic_model_array =
+                PmicEntry::from_bytes_to_slice(&self.soc_info_mem[offset..offset + size]);
+        }
+        if params.info_fmt >= SocInfo::version_fuse(0, 13) {
+            params.chip_id = Some(self.soc_info.chip_id);
+        }
+        params.versions = ImageVersion::from_bytes_to_slice(self.version_mem);
+        params.versions2 = self
+            .version_mem2
+            .and_then(ImageVersion::from_bytes_to_slice);
+        Ok(params)
+    }
+}
+
+macro_rules! value_attrs {
+    ($builder:ident, $params:ident, @) => {};
+    ($builder:ident, $params:ident, @ $s:ident $($rest:tt)*) => {
+        if let Some(v) = $params.$s.as_ref() {
+            $builder.read_only_file(c_str!(stringify!($s)), v);
+        }
+        value_attrs!($builder, $params, @ $($rest)*)
+    };
+    ($builder:ident, $params:ident, @ {$s:ident, $cb:expr} $($rest:tt)*) => {
+        if let Some(v) = $params.$s.as_ref() {
+            $builder.read_callback_file(c_str!(stringify!($s)), v, $cb);
+        }
+        value_attrs!($builder, $params, @ $($rest)*)
+    };
+    ($builder:ident, $params:ident, {$($items:tt),*}) => {
+        value_attrs!($builder, $params, @ $($items)*)
+    };
+}
+
+fn no_quirk<const SIZE: usize>(buf: &[u8; SIZE], f: &mut Formatter<'_>) -> fmt::Result {
+    if buf[0] == 0 {
+        writeln!(f)
+    } else {
+        nul_array(buf, f)
+    }
+}
+
+fn nul_array<const SIZE: usize>(buf: &[u8; SIZE], f: &mut Formatter<'_>) -> fmt::Result {
+    if let Some(end) = buf.iter().position(|x| *x == 0) {
+        if end == 0 {
+            // Match original driver quirk - empty strings don't have a trailing newline
+            return Ok(());
+        }
+        let Ok(c_str) = CStr::from_bytes_with_nul(&buf[0..=end]) else {
+            pr_warn!("Creating CStr from bytes with known first NUL failed?");
+            return Ok(());
+        };
+        let Ok(str) = c_str.to_str() else {
+            pr_warn!("Non-unicode-compatible character in string");
+            return Ok(());
+        };
+        writeln!(f, "{str}")
+    } else {
+        writeln!(f, "Missing NUL: {buf:?}")
+    }
+}
+
+impl ImageVersion {
+    fn build_debugfs<'a>(&'a self, dir: &ScopedDir<'a, '_>, image_name: &CStr) {
+        let subdir = dir.dir(image_name);
+        subdir.read_callback_file(c_str!("name"), &self.name, &nul_array);
+        subdir.read_callback_file(c_str!("variant"), &self.variant, &nul_array);
+        subdir.read_callback_file(c_str!("oem"), &self.oem, &nul_array);
+    }
+}
+
+fn hex(x: &u32, f: &mut Formatter<'_>) -> fmt::Result {
+    writeln!(f, "{x:#010x}")
+}
+
+impl Params {
+    fn build_debugfs<'data>(&'data self, dir: &ScopedDir<'data, '_>) {
+        dir.read_callback_file(c_str!("info_fmt"), &self.info_fmt, &hex);
+        dir.read_callback_file(c_str!("build_id"), &self.build_id, &no_quirk);
+        value_attrs!(dir, self, {
+            raw_version,
+            hardware_platform,
+            platform_version,
+            accessory_chip,
+            hardware_platform_subtype,
+            {raw_device_number, &hex},
+            {raw_device_family, &hex},
+            {chip_family, &hex},
+            {chip_id, &nul_array},
+            nproduct_id,
+            nsubset_parts_array_offset,
+            num_subset_parts,
+            ncluster_array_offset,
+            num_clusters,
+            nmodem_supported,
+            pcode,
+            feature_code,
+            oem_variant,
+            boot_core,
+            boot_cluster,
+            num_func_clusters,
+            foundry_id,
+            raw_package_type,
+            {pmic_model, &fmt_pmic_model},
+            {pmic_die_rev, &fmt_pmic_die_rev},
+            {pmic_model_array, &fmt_pmic_model_array}
+        });
+        let versions = self.versions.unwrap_or(&[]);
+        let versions2 = self.versions2.unwrap_or(&[]);
+        for (version, image_name) in versions
+            .iter()
+            .take(32)
+            .chain(versions2.iter())
+            .zip(IMAGE_NAMES.iter())
+        {
+            version.build_debugfs(dir, image_name);
+        }
+    }
+}
+
+impl platform::Driver for QcomSocInfo {
+    type IdInfo = ();
+    const OF_ID_TABLE: Option<kernel::of::IdTable<Self::IdInfo>> = None;
+    fn probe(_dev: &Device<Core>, _id_info: Option<&Self::IdInfo>) -> impl PinInit<Self, Error> {
+        pin_init_scope(|| {
+            let soc_info_mem = qcom_smem_get(
+                kernel::bindings::QCOM_SMEM_HOST_ANY,
+                kernel::bindings::SMEM_HW_SW_BUILD_ID,
+            )?;
+            let version_mem = qcom_smem_get(
+                kernel::bindings::QCOM_SMEM_HOST_ANY,
+                bindings::SMEM_IMAGE_VERSION_TABLE,
+            )?;
+            let version_mem2 = qcom_smem_get(
+                kernel::bindings::QCOM_SMEM_HOST_ANY,
+                bindings::SMEM_IMAGE_VERSION_TABLE_2,
+            )
+            .ok();
+            let info = SocInfo::from_mem(soc_info_mem, version_mem, version_mem2);
+            let backing = info.build_params()?;
+            kernel::rand::add_device_randomness(soc_info_mem);
+            Ok(try_pin_init!(Self {
+                registration <- soc::DeviceRegistration::register(info.device_attribute()?),
+                params <- Scope::dir(backing, c_str!("qcom_socinfo"),
+                                     Params::build_debugfs),
+            }? Error))
+        })
+    }
+}
diff --git a/rust/bindgen_parameters b/rust/bindgen_parameters
index 60910ba8731180516b4980b9e0dc5fb0c297da55..da13dc073ae1f41e9f0a835510fdc36b23ff1a4d 100644
--- a/rust/bindgen_parameters
+++ b/rust/bindgen_parameters
@@ -72,3 +72,4 @@
 # We can't auto-derive AsBytes, as we need a const-time check to see if there
 # is padding involved. Add it explicitly when you expect no padding.
 --with-derive-custom-struct cpumask=AsBytesTrait
+--with-derive-custom-struct socinfo=AsBytesTrait
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 9fdf76ca630e00715503e2a3a809bedc895697fd..b076b847731d97cf345d8ef80df9c315a79c8fab 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -95,6 +95,12 @@
  */
 #include <../../drivers/base/base.h>
 
+#if IS_ENABLED(CONFIG_QCOM_SOCINFO)
+#include <dt-bindings/arm/qcom,ids.h>
+#include <linux/soc/qcom/smem.h>
+#include <linux/soc/qcom/socinfo.h>
+#endif
+
 #if defined(CONFIG_DRM_PANIC_SCREEN_QR_CODE)
 // Used by `#[export]` in `drivers/gpu/drm/drm_panic_qr.rs`.
 #include <drm/drm_panic.h>

---
base-commit: 4b31c90f86768367c84cc6e99e1deee0ec55197b
change-id: 20251029-qcom-socinfo-d8387c7fdb1c
prerequisite-change-id: 20251029-soc-bindings-9b0731bcdbed:v1
prerequisite-patch-id: e4da423ddabec93bd9a64004691266f9b740e0c5
prerequisite-patch-id: 5097ff622f8cb1d38354674cf70c1c946ac87f6c
prerequisite-change-id: 20251029-add-entropy-f57e12ebe110:v1
prerequisite-patch-id: 657de204912d2c5efff9898d3f4c51e52684d8e6
prerequisite-change-id: 20251212-transmute-8ab6076700a8:v1
prerequisite-patch-id: 4f5f7cb002d02d232083ab5c3ce6b3cb90650bd6
prerequisite-patch-id: 01a1f3d727e549c173dd142180741f5b6a3c866e
prerequisite-patch-id: ff801a7ae439876b1554b4d1132d94c825bbe74f

Best regards,
-- 
Matthew Maurer <mmaurer@google.com>

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Bryan O'Donoghue]

On 13/12/2025 00:36, Matthew Maurer wrote:

> +//! Re-implementation of Qualcomm's Socinfo driver in Rust
> +use core::fmt;
> +use core::fmt::Formatter;
> +use kernel::debugfs::{Scope, ScopedDir};
> +use kernel::device::Core;
> +use kernel::module_platform_driver;
> +use kernel::platform::{self, Device};
> +use kernel::prelude::{fmt, pin_data, Error, PinInit, Result};
> +use kernel::soc;
> +use kernel::str::{CStr, CStrExt, CString};
> +use kernel::transmute::FromBytes;
> +use kernel::{c_str, pr_warn, try_pin_init};
> +use pin_init::pin_init_scope;

Probably learning more from your patch than being in a position to 
review - thanks for that.

One thing I notice is that, in C we alphabetise the includes.

What is the normative coding standard for rust stuff I'm not sure. Is 
there any technical reason not to do the same as C includes alphabetising ?

You appear to sort these kernel:: use statements earlier on in the patch.

---
bod

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Miguel Ojeda]

On Sat, Dec 13, 2025 at 3:50 AM Bryan O'Donoghue <bod@kernel.org> wrote:
>
> One thing I notice is that, in C we alphabetise the includes.
>
> What is the normative coding standard for rust stuff I'm not sure. Is
> there any technical reason not to do the same as C includes alphabetising ?
>
> You appear to sort these kernel:: use statements earlier on in the patch.

They should be sorted, yeah (`rustfmt` takes care of that
automatically). In addition, recently we define the style for imports
(even if not all code follows it yet) -- please see:

    https://docs.kernel.org/rust/coding-guidelines.html#imports

Thanks for taking a look, and I hope that clarifies.

Cheers,
Miguel

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Dmitry Baryshkov]

On Sat, Dec 13, 2025 at 12:36:00AM +0000, Matthew Maurer wrote:
> Re-implements qcom-socinfo driver in Rust, using `Scoped`-based DebugFS
> bindings.
> 
> Signed-off-by: Matthew Maurer <mmaurer@google.com>
> ---
> This patch converts the QC socinfo driver to Rust, intended to be the
> first Rust driver in Android that is owned by a vendor rather than the
> platform.
> 
> This driver is currently quirk-compatible in DebugFS-exported values. If
> the maintainers do not believe that maintaining the exact formats is a
> benefit, we could simplify the code further by dropping some of the
> custom formatting functions used to match the output.
> 
> I sent an earlier draft of this privately a while back that was much
> rougher, but didn't get much feedback.
> 
> Now that we've got all the interfaces we need for this driver reasonably
> available (DebugFS was the one that took a bit here), I wanted to bring
> it up again, this time as an RFC. With the new features, the only place
> it needs `unsafe` is to define an abstraction over `qcom_get_smem`.
> 
> I have tested this on a SM8650-HDK by diffing the contents of the
> exported DebugFS and examining the files under /sys/bus/soc/devices/soc0
> 
> While I believe I have everything correctly exported via DebugFS, I have
> not checked equivalence across a large swath of devices, only the one.
> ---

> +
> +pub(crate) fn qcom_smem_get(host: i32, item: u32) -> Result<&'static [u8]> {
> +    let mut size = 0;
> +    // SAFETY: qcom_smem_get only requires that the size pointer be a writable size_t,
> +    // host and item are error checked in the qcom_smem module.
> +    let err_ptr = unsafe { kernel::bindings::qcom_smem_get(host as u32, item, &mut size) };
> +    let ptr = from_err_ptr(err_ptr)?;
> +    // SAFETY: If qcom_smem_get does not return an error, the returned pointer points to a readable
> +    // byte buffer with its size written into size. Because these buffers are derived from the
> +    // static ranges in the DT, this buffer remains accessible even if the qcom_smem module is
> +    // unloaded, so 'static is appropriate. The underlying buffer cannot mutate, so upgrading it
> +    // to a reference is allowed.

This is definitely not true. The smem data is ioremap'ped during
qcom_smem_probe() and will be unmapped if qcom_smem is unloaded.
Accessing such a pointer after unloading smem would result in the
exception. Current socinfo ignores that and keeps on accessing the data
at runtime (which also allows it to read DSP versions which are updated
at runtime), but the comment needs to be corrected.

> +    Ok(unsafe { core::slice::from_raw_parts(ptr as *const u8, size) })
> +}
> +

> +
> +pub(crate) static SOC_IDS: &[SocId] = &[
> +    id_entry!(MSM8260),
> +    id_entry!(MSM8660),

As we are performing a linear search over the array, would it be sensible
to sort it?

> +    id_entry!(APQ8060),
> +    id_entry!(MSM8960),

> +
> +pub(crate) const PMIC_MODELS: [Option<&str>; 92] = {
> +    let mut models = [None; 92];

I don't like the magic 92 appearing twice here just because the last
element of the array has number 91. Is there a sensible but idiomatic
way to express that (note in C we just use flex array without the size
and don't specify the size at all, so we don't have the duplication).

> +    models[0] = Some("Unknown PMIC model");
> +    models[1] = Some("PM8941");
> +    models[2] = Some("PM8841");
> +    models[3] = Some("PM8019");
> +    models[4] = Some("PM8226");
> +    models[5] = Some("PM8110");
> +    models[6] = Some("PMA8084");
> +    models[7] = Some("PMI8962");
> +    models[8] = Some("PMD9635");
> +    models[9] = Some("PM8994");
> +    models[10] = Some("PMI8994");
> +    models[11] = Some("PM8916");
> +    models[12] = Some("PM8004");
> +    models[13] = Some("PM8909/PM8058");
> +    models[14] = Some("PM8028");
> +    models[15] = Some("PM8901");
> +    models[16] = Some("PM8950/PM8027");
> +    models[17] = Some("PMI8950/ISL9519");
> +    models[18] = Some("PMK8001/PM8921");
> +    models[19] = Some("PMI8996/PM8018");
> +    models[20] = Some("PM8998/PM8015");
> +    models[21] = Some("PMI8998/PM8014");
> +    models[22] = Some("PM8821");
> +    models[23] = Some("PM8038");
> +    models[24] = Some("PM8005/PM8922");
> +    models[25] = Some("PM8917/PM8937");
> +    models[26] = Some("PM660L");
> +    models[27] = Some("PM660");
> +    models[30] = Some("PM8150");
> +    models[31] = Some("PM8150L");
> +    models[32] = Some("PM8150B");
> +    models[33] = Some("PMK8002");
> +    models[36] = Some("PM8009");
> +    models[37] = Some("PMI632");
> +    models[38] = Some("PM8150C");
> +    models[40] = Some("PM6150");
> +    models[41] = Some("SMB2351");
> +    models[44] = Some("PM8008");
> +    models[45] = Some("PM6125");
> +    models[46] = Some("PM7250B");
> +    models[47] = Some("PMK8350");
> +    models[48] = Some("PM8350");
> +    models[49] = Some("PM8350C");
> +    models[50] = Some("PM8350B");
> +    models[51] = Some("PMR735A");
> +    models[52] = Some("PMR735B");
> +    models[54] = Some("PM6350");
> +    models[55] = Some("PM4125");
> +    models[58] = Some("PM8450");
> +    models[65] = Some("PM8010");
> +    models[69] = Some("PM8550VS");
> +    models[70] = Some("PM8550VE");
> +    models[71] = Some("PM8550B");
> +    models[72] = Some("PMR735D");
> +    models[73] = Some("PM8550");
> +    models[74] = Some("PMK8550");
> +    models[78] = Some("PMM8650AU");
> +    models[79] = Some("PMM8650AU_PSAIL");
> +    models[80] = Some("PM7550");
> +    models[82] = Some("PMC8380");
> +    models[83] = Some("SMB2360");
> +    models[91] = Some("PMIV0108");
> +    models
> +};
> +
> diff --git a/drivers/soc/qcom/socinfo/socinfo.rs b/drivers/soc/qcom/socinfo/socinfo.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..bff1bf8fd672e3c461352075aa85ef8fdedff953
> --- /dev/null
> +++ b/drivers/soc/qcom/socinfo/socinfo.rs
> @@ -0,0 +1,362 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +// Copyright (C) 2025 Google LLC.
> +
> +//! Re-implementation of Qualcomm's Socinfo driver in Rust

I think this comment can be dropped. If it gets merged, there is no old
Socinfo driver.

> +use core::fmt;
> +use core::fmt::Formatter;
> +use kernel::debugfs::{Scope, ScopedDir};
> +use kernel::device::Core;
> +use kernel::module_platform_driver;
> +use kernel::platform::{self, Device};
> +use kernel::prelude::{fmt, pin_data, Error, PinInit, Result};
> +use kernel::soc;
> +use kernel::str::{CStr, CStrExt, CString};
> +use kernel::transmute::FromBytes;
> +use kernel::{c_str, pr_warn, try_pin_init};
> +use pin_init::pin_init_scope;
> +

> +        let versions = self.versions.unwrap_or(&[]);
> +        let versions2 = self.versions2.unwrap_or(&[]);
> +        for (version, image_name) in versions
> +            .iter()
> +            .take(32)
> +            .chain(versions2.iter())
> +            .zip(IMAGE_NAMES.iter())
> +        {
> +            version.build_debugfs(dir, image_name);
> +        }

I like this idiomatic code, we even get what original driver misses:
size checks for those memory regions.

> +    }
> +}
> +

-- 
With best wishes
Dmitry

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Greg KH]

On Sat, Dec 13, 2025 at 12:36:00AM +0000, Matthew Maurer wrote:
> Re-implements qcom-socinfo driver in Rust, using `Scoped`-based DebugFS
> bindings.
> 
> Signed-off-by: Matthew Maurer <mmaurer@google.com>
> ---
> This patch converts the QC socinfo driver to Rust, intended to be the
> first Rust driver in Android that is owned by a vendor rather than the
> platform.

Um, shouldn't you have a signed-off-by from the maintainers of the .c
file here?  You are deleting it, and then not adding anything to the
MAINTAINERS file to take ownership of this new file, which I thought
checkpatch would have warned about.

thanks,

greg k-h

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Greg KH]

On Sat, Dec 13, 2025 at 12:36:00AM +0000, Matthew Maurer wrote:
>  drivers/soc/qcom/Kconfig             |   1 +
>  drivers/soc/qcom/Makefile            |   2 +-
>  drivers/soc/qcom/socinfo.c           | 931 -----------------------------------
>  drivers/soc/qcom/socinfo/Makefile    |   2 +
>  drivers/soc/qcom/socinfo/bindings.rs |  59 +++
>  drivers/soc/qcom/socinfo/data.rs     | 424 ++++++++++++++++
>  drivers/soc/qcom/socinfo/socinfo.rs  | 362 ++++++++++++++

Shouldn't this all just be in 1 .rs file?  By not doing that, you have a
number of public symbols (that are very generic), that will "pollute"
the global namespace of the kernel symbol table (unless rust does this
somehow differently?)

But putting this all in 1 file should solve all of that, and remove all
global symbols.  Especially for just a small and simple driver like
this, no need to split it up at all.

thanks,

greg k-h

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Matthew Maurer]

On Sat, Dec 13, 2025 at 5:57 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> Shouldn't this all just be in 1 .rs file?  By not doing that, you have a
> number of public symbols (that are very generic), that will "pollute"
> the global namespace of the kernel symbol table (unless rust does this
> somehow differently?)

Rust produces one compilation unit per "crate". In this case,
`socinfo` is the crate, and the other files are included as "modules".
Symbols defined in them are not public unless annotated with `pub`.
`pub(crate)` makes them only exported within the scope of the crate,
i.e. inside the single produced `.o` file.

> But putting this all in 1 file should solve all of that, and remove all
> global symbols.  Especially for just a small and simple driver like
> this, no need to split it up at all.

I'm willing to put it all in one file if that's preferred for
organizational purposes, but as explained above, this shouldn't bloat
the symbol count in Rust. If it were my own codebase this would be
separate files, so up to y'all what level of granularity you'd prefer.

>
> thanks,
>
> greg k-h

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Matthew Maurer]

On Sat, Dec 13, 2025 at 5:28 AM Greg KH <greg@kroah.com> wrote:
>
> On Sat, Dec 13, 2025 at 12:36:00AM +0000, Matthew Maurer wrote:
> > Re-implements qcom-socinfo driver in Rust, using `Scoped`-based DebugFS
> > bindings.
> >
> > Signed-off-by: Matthew Maurer <mmaurer@google.com>
> > ---
> > This patch converts the QC socinfo driver to Rust, intended to be the
> > first Rust driver in Android that is owned by a vendor rather than the
> > platform.
>
> Um, shouldn't you have a signed-off-by from the maintainers of the .c
> file here?  You are deleting it, and then not adding anything to the
> MAINTAINERS file to take ownership of this new file, which I thought
> checkpatch would have warned about.

Checkpatch did not warn me about this, but I did add the maintainers
of that file on this RFC, so we can see how they feel about it on this
thread.

>
> thanks,
>
> greg k-h

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Matthew Maurer]

On Fri, Dec 12, 2025 at 7:55 PM Dmitry Baryshkov
<dmitry.baryshkov@oss.qualcomm.com> wrote:
>
> On Sat, Dec 13, 2025 at 12:36:00AM +0000, Matthew Maurer wrote:
> > Re-implements qcom-socinfo driver in Rust, using `Scoped`-based DebugFS
> > bindings.
> >
> > Signed-off-by: Matthew Maurer <mmaurer@google.com>
> > ---
> > This patch converts the QC socinfo driver to Rust, intended to be the
> > first Rust driver in Android that is owned by a vendor rather than the
> > platform.
> >
> > This driver is currently quirk-compatible in DebugFS-exported values. If
> > the maintainers do not believe that maintaining the exact formats is a
> > benefit, we could simplify the code further by dropping some of the
> > custom formatting functions used to match the output.
> >
> > I sent an earlier draft of this privately a while back that was much
> > rougher, but didn't get much feedback.
> >
> > Now that we've got all the interfaces we need for this driver reasonably
> > available (DebugFS was the one that took a bit here), I wanted to bring
> > it up again, this time as an RFC. With the new features, the only place
> > it needs `unsafe` is to define an abstraction over `qcom_get_smem`.
> >
> > I have tested this on a SM8650-HDK by diffing the contents of the
> > exported DebugFS and examining the files under /sys/bus/soc/devices/soc0
> >
> > While I believe I have everything correctly exported via DebugFS, I have
> > not checked equivalence across a large swath of devices, only the one.
> > ---
>
> > +
> > +pub(crate) fn qcom_smem_get(host: i32, item: u32) -> Result<&'static [u8]> {
> > +    let mut size = 0;
> > +    // SAFETY: qcom_smem_get only requires that the size pointer be a writable size_t,
> > +    // host and item are error checked in the qcom_smem module.
> > +    let err_ptr = unsafe { kernel::bindings::qcom_smem_get(host as u32, item, &mut size) };
> > +    let ptr = from_err_ptr(err_ptr)?;
> > +    // SAFETY: If qcom_smem_get does not return an error, the returned pointer points to a readable
> > +    // byte buffer with its size written into size. Because these buffers are derived from the
> > +    // static ranges in the DT, this buffer remains accessible even if the qcom_smem module is
> > +    // unloaded, so 'static is appropriate. The underlying buffer cannot mutate, so upgrading it
> > +    // to a reference is allowed.
>
> This is definitely not true. The smem data is ioremap'ped during
> qcom_smem_probe() and will be unmapped if qcom_smem is unloaded.
> Accessing such a pointer after unloading smem would result in the
> exception. Current socinfo ignores that and keeps on accessing the data
> at runtime (which also allows it to read DSP versions which are updated
> at runtime), but the comment needs to be corrected.

Thanks for the info. In v2 I'll look into seeing if I can make those
slices hold a reference on the smem module while they're alive.

>
> > +    Ok(unsafe { core::slice::from_raw_parts(ptr as *const u8, size) })
> > +}
> > +
>
> > +
> > +pub(crate) static SOC_IDS: &[SocId] = &[
> > +    id_entry!(MSM8260),
> > +    id_entry!(MSM8660),
>
> As we are performing a linear search over the array, would it be sensible
> to sort it?

I directly copied the ordering used in the C code. We can't easily see
that it's sorted at a source level, because the key is the soc ID,
which comes from the device tree headers. For the first four elements
at least, it *is* in sorted order already, we're just not currently
relying on it. I could potentially add a debug assert to check that
things are in sorted order and switch to assuming that if you'd like.

>
> > +    id_entry!(APQ8060),
> > +    id_entry!(MSM8960),
>
> > +
> > +pub(crate) const PMIC_MODELS: [Option<&str>; 92] = {
> > +    let mut models = [None; 92];
>
> I don't like the magic 92 appearing twice here just because the last
> element of the array has number 91. Is there a sensible but idiomatic
> way to express that (note in C we just use flex array without the size
> and don't specify the size at all, so we don't have the duplication).

The usual way to do this in Rust is to use a slice (`&[Option<&str>]`)
rather than an array (which looks like `[T; N]`, as I did above). The
reason I didn't do it here was to avoid explicitly listing any punched
holes (i.e. with slice, I'd have to write something like
`&[Some("Foo"), None, Some("Bar")]`). Rust doesn't directly have a
partial array initialization syntax because it requires that
everything be fully initialized, which is why I was doing it through
mutation. If we're going to leave everything default `None`, and
express which elements should be `Some`, we need a bound on how many
elements "everything" should be.

Some options:
1. Make holes explicit
```
pub(crate) const PMIC_MODELS: &[Option<&'str>] = &[
  Some("foo"),
  Some("bar"),
  None,
  Some("baz"),
  // ...
};
```
This is the one I'd suggest if we want to get rid of the 92. It has
the downside of some extra explicit `None` entries, but the array
isn't *that* sparse.

2. Factor out 92 into a constant.
3. Define the constant in terms of index/value pairs instead. I could
use `const`-time code to produce the array we want:
```
const PMIC_ENTRIES: &[(usize, &str)] = &[(1, "foo"), (9, "bar"), (42, "baz")];

const PMIC_MODELS_LEN: usize = {
    let mut max = 0;
    let mut i = 0;
    while i < PMIC_ENTRIES.len() {
        if PMIC_ENTRIES[i].0 > max {
            max = PMIC_ENTRIES[i].0;
        }
        i += 1;
    }
    max + 1
};

pub const PMIC_MODELS: [Option<&'static str>; PMIC_MODELS_LEN] = {
    let mut models = [None; PMIC_MODELS_LEN];
    let mut i = 0;
    while i < PMIC_ENTRIES.len() {
        let (idx, val) = PMIC_ENTRIES[i];
        models[idx] = Some(val);
        i += 1;
    }
    models
};
```
(The slightly icky looking loops are because not all features are
available in const mode.)
This seems a bit overkill for what's going on.
>
> > +    models[0] = Some("Unknown PMIC model");
> > +    models[1] = Some("PM8941");
> > +    models[2] = Some("PM8841");
> > +    models[3] = Some("PM8019");
> > +    models[4] = Some("PM8226");
> > +    models[5] = Some("PM8110");
> > +    models[6] = Some("PMA8084");
> > +    models[7] = Some("PMI8962");
> > +    models[8] = Some("PMD9635");
> > +    models[9] = Some("PM8994");
> > +    models[10] = Some("PMI8994");
> > +    models[11] = Some("PM8916");
> > +    models[12] = Some("PM8004");
> > +    models[13] = Some("PM8909/PM8058");
> > +    models[14] = Some("PM8028");
> > +    models[15] = Some("PM8901");
> > +    models[16] = Some("PM8950/PM8027");
> > +    models[17] = Some("PMI8950/ISL9519");
> > +    models[18] = Some("PMK8001/PM8921");
> > +    models[19] = Some("PMI8996/PM8018");
> > +    models[20] = Some("PM8998/PM8015");
> > +    models[21] = Some("PMI8998/PM8014");
> > +    models[22] = Some("PM8821");
> > +    models[23] = Some("PM8038");
> > +    models[24] = Some("PM8005/PM8922");
> > +    models[25] = Some("PM8917/PM8937");
> > +    models[26] = Some("PM660L");
> > +    models[27] = Some("PM660");
> > +    models[30] = Some("PM8150");
> > +    models[31] = Some("PM8150L");
> > +    models[32] = Some("PM8150B");
> > +    models[33] = Some("PMK8002");
> > +    models[36] = Some("PM8009");
> > +    models[37] = Some("PMI632");
> > +    models[38] = Some("PM8150C");
> > +    models[40] = Some("PM6150");
> > +    models[41] = Some("SMB2351");
> > +    models[44] = Some("PM8008");
> > +    models[45] = Some("PM6125");
> > +    models[46] = Some("PM7250B");
> > +    models[47] = Some("PMK8350");
> > +    models[48] = Some("PM8350");
> > +    models[49] = Some("PM8350C");
> > +    models[50] = Some("PM8350B");
> > +    models[51] = Some("PMR735A");
> > +    models[52] = Some("PMR735B");
> > +    models[54] = Some("PM6350");
> > +    models[55] = Some("PM4125");
> > +    models[58] = Some("PM8450");
> > +    models[65] = Some("PM8010");
> > +    models[69] = Some("PM8550VS");
> > +    models[70] = Some("PM8550VE");
> > +    models[71] = Some("PM8550B");
> > +    models[72] = Some("PMR735D");
> > +    models[73] = Some("PM8550");
> > +    models[74] = Some("PMK8550");
> > +    models[78] = Some("PMM8650AU");
> > +    models[79] = Some("PMM8650AU_PSAIL");
> > +    models[80] = Some("PM7550");
> > +    models[82] = Some("PMC8380");
> > +    models[83] = Some("SMB2360");
> > +    models[91] = Some("PMIV0108");
> > +    models
> > +};
> > +
> > diff --git a/drivers/soc/qcom/socinfo/socinfo.rs b/drivers/soc/qcom/socinfo/socinfo.rs
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..bff1bf8fd672e3c461352075aa85ef8fdedff953
> > --- /dev/null
> > +++ b/drivers/soc/qcom/socinfo/socinfo.rs
> > @@ -0,0 +1,362 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +// Copyright (C) 2025 Google LLC.
> > +
> > +//! Re-implementation of Qualcomm's Socinfo driver in Rust
>
> I think this comment can be dropped. If it gets merged, there is no old
> Socinfo driver.

Agreed. We do need a crate level doc comment, but it can just say
"Qualcomm SocInfo Driver" or something.

>
> > +use core::fmt;
> > +use core::fmt::Formatter;
> > +use kernel::debugfs::{Scope, ScopedDir};
> > +use kernel::device::Core;
> > +use kernel::module_platform_driver;
> > +use kernel::platform::{self, Device};
> > +use kernel::prelude::{fmt, pin_data, Error, PinInit, Result};
> > +use kernel::soc;
> > +use kernel::str::{CStr, CStrExt, CString};
> > +use kernel::transmute::FromBytes;
> > +use kernel::{c_str, pr_warn, try_pin_init};
> > +use pin_init::pin_init_scope;
> > +
>
> > +        let versions = self.versions.unwrap_or(&[]);
> > +        let versions2 = self.versions2.unwrap_or(&[]);
> > +        for (version, image_name) in versions
> > +            .iter()
> > +            .take(32)
> > +            .chain(versions2.iter())
> > +            .zip(IMAGE_NAMES.iter())
> > +        {
> > +            version.build_debugfs(dir, image_name);
> > +        }
>
> I like this idiomatic code, we even get what original driver misses:
> size checks for those memory regions.
>
> > +    }
> > +}
> > +
>
> --
> With best wishes
> Dmitry

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Matthew Maurer]

On Sat, Dec 13, 2025 at 8:24 AM Matthew Maurer <mmaurer@google.com> wrote:
>
> On Sat, Dec 13, 2025 at 5:57 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > Shouldn't this all just be in 1 .rs file?  By not doing that, you have a
> > number of public symbols (that are very generic), that will "pollute"

One other comment - unless you see `#[no_mangle]` on a Rust symbol,
don't worry about the symbol name being generic. The regular symbols
are all mangled with their path and some Rust compiler info, so for
example, `SOC_IDS` is actually a mangled variant of
`socinfo::data::SOC_IDS`.

> > the global namespace of the kernel symbol table (unless rust does this
> > somehow differently?)
>
> Rust produces one compilation unit per "crate". In this case,
> `socinfo` is the crate, and the other files are included as "modules".
> Symbols defined in them are not public unless annotated with `pub`.
> `pub(crate)` makes them only exported within the scope of the crate,
> i.e. inside the single produced `.o` file.

Slight further clarification - putting them in a separate file doesn't
affect visibility in the final `.o` file that's produced, but it is
technically possible for functions or data which are not public at the
language level to be public at the object level, usually due to
polymorphism. Additionally, an item being `pub` in a module doesn't
necessarily get exported at the language level unless the module
declaration itself is `pub` or the parent module re-exports it
explicitly through `pub use`.

>
> > But putting this all in 1 file should solve all of that, and remove all
> > global symbols.  Especially for just a small and simple driver like
> > this, no need to split it up at all.
>
> I'm willing to put it all in one file if that's preferred for
> organizational purposes, but as explained above, this shouldn't bloat
> the symbol count in Rust. If it were my own codebase this would be
> separate files, so up to y'all what level of granularity you'd prefer.
>
> >
> > thanks,
> >
> > greg k-h

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Matthew Maurer]

On Sat, Dec 13, 2025 at 8:26 AM Matthew Maurer <mmaurer@google.com> wrote:
>
> On Sat, Dec 13, 2025 at 5:28 AM Greg KH <greg@kroah.com> wrote:
> >
> > On Sat, Dec 13, 2025 at 12:36:00AM +0000, Matthew Maurer wrote:
> > > Re-implements qcom-socinfo driver in Rust, using `Scoped`-based DebugFS
> > > bindings.
> > >
> > > Signed-off-by: Matthew Maurer <mmaurer@google.com>
> > > ---
> > > This patch converts the QC socinfo driver to Rust, intended to be the
> > > first Rust driver in Android that is owned by a vendor rather than the
> > > platform.
> >
> > Um, shouldn't you have a signed-off-by from the maintainers of the .c
> > file here?  You are deleting it, and then not adding anything to the
> > MAINTAINERS file to take ownership of this new file, which I thought
> > checkpatch would have warned about.
>
> Checkpatch did not warn me about this, but I did add the maintainers
> of that file on this RFC, so we can see how they feel about it on this
> thread.
>

Further clarification - checkpatch *did* warn me that perhaps
MAINTAINERS needed updating because files were added or removed, but I
saw that the maintainers were by `drivers/soc/qcom` which should
include all the new files. It did not warn me that I needed any
additional Signed-off-by lines.

> >
> > thanks,
> >
> > greg k-h

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Gary Guo]

On Sat, 13 Dec 2025 08:58:56 -0800
Matthew Maurer <mmaurer@google.com> wrote:

> Some options:
> 1. Make holes explicit
> ```
> pub(crate) const PMIC_MODELS: &[Option<&'str>] = &[
>   Some("foo"),
>   Some("bar"),
>   None,
>   Some("baz"),
>   // ...
> };
> ```
> This is the one I'd suggest if we want to get rid of the 92. It has
> the downside of some extra explicit `None` entries, but the array
> isn't *that* sparse.
> 
> 2. Factor out 92 into a constant.
> 3. Define the constant in terms of index/value pairs instead. I could
> use `const`-time code to produce the array we want:
> ```
> const PMIC_ENTRIES: &[(usize, &str)] = &[(1, "foo"), (9, "bar"), (42, "baz")];
> 
> const PMIC_MODELS_LEN: usize = {
>     let mut max = 0;
>     let mut i = 0;
>     while i < PMIC_ENTRIES.len() {
>         if PMIC_ENTRIES[i].0 > max {
>             max = PMIC_ENTRIES[i].0;
>         }
>         i += 1;
>     }
>     max + 1
> };
> 
> pub const PMIC_MODELS: [Option<&'static str>; PMIC_MODELS_LEN] = {
>     let mut models = [None; PMIC_MODELS_LEN];
>     let mut i = 0;
>     while i < PMIC_ENTRIES.len() {
>         let (idx, val) = PMIC_ENTRIES[i];
>         models[idx] = Some(val);
>         i += 1;
>     }
>     models
> };
> ```
> (The slightly icky looking loops are because not all features are
> available in const mode.)
> This seems a bit overkill for what's going on.

How about making a macro for this and make it available from kernel crate?
Looks like this should do what you need?


/// Create a sparse array of `[Option<T>; _]`.
macro_rules! sparse_array {
    ($(
        $index:literal: $value:expr
    ),* $(,)?) => {{
        const SIZE: usize = {
            let mut size = 0;
            $(if $index >= size {
                size = $index + 1;
            })*
            size
        };

        const {
            let mut arr = [None; SIZE];
            $(arr[$index] = Some($value);)*
            arr
        }
    }}
}

fn main() {
    const EXAMPLE: &[Option<u32>] = &sparse_array! {
        0: 1,
        5: 2,
    };
    println!("{:?}", EXAMPLE);
}

Best,
Gary

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Dmitry Baryshkov]

On Sat, Dec 13, 2025 at 08:58:56AM -0800, Matthew Maurer wrote:
> On Fri, Dec 12, 2025 at 7:55 PM Dmitry Baryshkov
> <dmitry.baryshkov@oss.qualcomm.com> wrote:
> >
> > On Sat, Dec 13, 2025 at 12:36:00AM +0000, Matthew Maurer wrote:
> > > Re-implements qcom-socinfo driver in Rust, using `Scoped`-based DebugFS
> > > bindings.
> > >
> > > Signed-off-by: Matthew Maurer <mmaurer@google.com>
> > > ---
> > > This patch converts the QC socinfo driver to Rust, intended to be the
> > > first Rust driver in Android that is owned by a vendor rather than the
> > > platform.
> > >
> > > This driver is currently quirk-compatible in DebugFS-exported values. If
> > > the maintainers do not believe that maintaining the exact formats is a
> > > benefit, we could simplify the code further by dropping some of the
> > > custom formatting functions used to match the output.
> > >
> > > I sent an earlier draft of this privately a while back that was much
> > > rougher, but didn't get much feedback.
> > >
> > > Now that we've got all the interfaces we need for this driver reasonably
> > > available (DebugFS was the one that took a bit here), I wanted to bring
> > > it up again, this time as an RFC. With the new features, the only place
> > > it needs `unsafe` is to define an abstraction over `qcom_get_smem`.
> > >
> > > I have tested this on a SM8650-HDK by diffing the contents of the
> > > exported DebugFS and examining the files under /sys/bus/soc/devices/soc0
> > >
> > > While I believe I have everything correctly exported via DebugFS, I have
> > > not checked equivalence across a large swath of devices, only the one.
> > > ---
> >
> > > +
> > > +pub(crate) fn qcom_smem_get(host: i32, item: u32) -> Result<&'static [u8]> {
> > > +    let mut size = 0;
> > > +    // SAFETY: qcom_smem_get only requires that the size pointer be a writable size_t,
> > > +    // host and item are error checked in the qcom_smem module.
> > > +    let err_ptr = unsafe { kernel::bindings::qcom_smem_get(host as u32, item, &mut size) };
> > > +    let ptr = from_err_ptr(err_ptr)?;
> > > +    // SAFETY: If qcom_smem_get does not return an error, the returned pointer points to a readable
> > > +    // byte buffer with its size written into size. Because these buffers are derived from the
> > > +    // static ranges in the DT, this buffer remains accessible even if the qcom_smem module is
> > > +    // unloaded, so 'static is appropriate. The underlying buffer cannot mutate, so upgrading it
> > > +    // to a reference is allowed.
> >
> > This is definitely not true. The smem data is ioremap'ped during
> > qcom_smem_probe() and will be unmapped if qcom_smem is unloaded.
> > Accessing such a pointer after unloading smem would result in the
> > exception. Current socinfo ignores that and keeps on accessing the data
> > at runtime (which also allows it to read DSP versions which are updated
> > at runtime), but the comment needs to be corrected.
> 
> Thanks for the info. In v2 I'll look into seeing if I can make those
> slices hold a reference on the smem module while they're alive.

It should be a reference to the _bound_ device. I don't think we have
a way to do it. I suggest just fixing the comment instead.

> 
> >
> > > +    Ok(unsafe { core::slice::from_raw_parts(ptr as *const u8, size) })
> > > +}
> > > +
> >
> > > +
> > > +pub(crate) static SOC_IDS: &[SocId] = &[
> > > +    id_entry!(MSM8260),
> > > +    id_entry!(MSM8660),
> >
> > As we are performing a linear search over the array, would it be sensible
> > to sort it?
> 
> I directly copied the ordering used in the C code. We can't easily see
> that it's sorted at a source level, because the key is the soc ID,
> which comes from the device tree headers. For the first four elements
> at least, it *is* in sorted order already, we're just not currently
> relying on it. I could potentially add a debug assert to check that
> things are in sorted order and switch to assuming that if you'd like.

Sorry, I should be more explicit: sort by the name, not by the value. It
would make it easier to extend and reduce possible conflicts.

> 
> >
> > > +    id_entry!(APQ8060),
> > > +    id_entry!(MSM8960),
> >
> > > +
> > > +pub(crate) const PMIC_MODELS: [Option<&str>; 92] = {
> > > +    let mut models = [None; 92];
> >
> > I don't like the magic 92 appearing twice here just because the last
> > element of the array has number 91. Is there a sensible but idiomatic
> > way to express that (note in C we just use flex array without the size
> > and don't specify the size at all, so we don't have the duplication).
> 
> The usual way to do this in Rust is to use a slice (`&[Option<&str>]`)
> rather than an array (which looks like `[T; N]`, as I did above). The
> reason I didn't do it here was to avoid explicitly listing any punched
> holes (i.e. with slice, I'd have to write something like
> `&[Some("Foo"), None, Some("Bar")]`). Rust doesn't directly have a
> partial array initialization syntax because it requires that
> everything be fully initialized, which is why I was doing it through
> mutation. If we're going to leave everything default `None`, and
> express which elements should be `Some`, we need a bound on how many
> elements "everything" should be.
> 
> Some options:
> 1. Make holes explicit
> ```
> pub(crate) const PMIC_MODELS: &[Option<&'str>] = &[
>   Some("foo"),
>   Some("bar"),
>   None,
>   Some("baz"),
>   // ...
> };
> ```
> This is the one I'd suggest if we want to get rid of the 92. It has
> the downside of some extra explicit `None` entries, but the array
> isn't *that* sparse.
> 
> 2. Factor out 92 into a constant.
> 3. Define the constant in terms of index/value pairs instead. I could
> use `const`-time code to produce the array we want:
> ```
> const PMIC_ENTRIES: &[(usize, &str)] = &[(1, "foo"), (9, "bar"), (42, "baz")];
> 
> const PMIC_MODELS_LEN: usize = {
>     let mut max = 0;
>     let mut i = 0;
>     while i < PMIC_ENTRIES.len() {
>         if PMIC_ENTRIES[i].0 > max {
>             max = PMIC_ENTRIES[i].0;
>         }
>         i += 1;
>     }
>     max + 1
> };
> 
> pub const PMIC_MODELS: [Option<&'static str>; PMIC_MODELS_LEN] = {
>     let mut models = [None; PMIC_MODELS_LEN];
>     let mut i = 0;
>     while i < PMIC_ENTRIES.len() {
>         let (idx, val) = PMIC_ENTRIES[i];
>         models[idx] = Some(val);
>         i += 1;
>     }
>     models
> };
> ```
> (The slightly icky looking loops are because not all features are
> available in const mode.)
> This seems a bit overkill for what's going on.

I'd say, the easiet way would be to define PMIC_ENTRIES and then search
in it. It's the most idiomatic from my point of view. Other options
require extra knowledge or extra checks.

> >
> > > +    models[0] = Some("Unknown PMIC model");
> > > +    models[1] = Some("PM8941");
> > > +    models[2] = Some("PM8841");
> > > +    models[3] = Some("PM8019");
> > > +    models[4] = Some("PM8226");
> > > +    models[5] = Some("PM8110");
> > > +    models[6] = Some("PMA8084");
> > > +    models[7] = Some("PMI8962");
> > > +    models[8] = Some("PMD9635");
> > > +    models[9] = Some("PM8994");
> > > +    models[10] = Some("PMI8994");
> > > +    models[11] = Some("PM8916");
> > > +    models[12] = Some("PM8004");
> > > +    models[13] = Some("PM8909/PM8058");
> > > +    models[14] = Some("PM8028");
> > > +    models[15] = Some("PM8901");
> > > +    models[16] = Some("PM8950/PM8027");
> > > +    models[17] = Some("PMI8950/ISL9519");
> > > +    models[18] = Some("PMK8001/PM8921");
> > > +    models[19] = Some("PMI8996/PM8018");
> > > +    models[20] = Some("PM8998/PM8015");
> > > +    models[21] = Some("PMI8998/PM8014");
> > > +    models[22] = Some("PM8821");
> > > +    models[23] = Some("PM8038");
> > > +    models[24] = Some("PM8005/PM8922");
> > > +    models[25] = Some("PM8917/PM8937");
> > > +    models[26] = Some("PM660L");
> > > +    models[27] = Some("PM660");
> > > +    models[30] = Some("PM8150");
> > > +    models[31] = Some("PM8150L");
> > > +    models[32] = Some("PM8150B");
> > > +    models[33] = Some("PMK8002");
> > > +    models[36] = Some("PM8009");
> > > +    models[37] = Some("PMI632");
> > > +    models[38] = Some("PM8150C");
> > > +    models[40] = Some("PM6150");
> > > +    models[41] = Some("SMB2351");
> > > +    models[44] = Some("PM8008");
> > > +    models[45] = Some("PM6125");
> > > +    models[46] = Some("PM7250B");
> > > +    models[47] = Some("PMK8350");
> > > +    models[48] = Some("PM8350");
> > > +    models[49] = Some("PM8350C");
> > > +    models[50] = Some("PM8350B");
> > > +    models[51] = Some("PMR735A");
> > > +    models[52] = Some("PMR735B");
> > > +    models[54] = Some("PM6350");
> > > +    models[55] = Some("PM4125");
> > > +    models[58] = Some("PM8450");
> > > +    models[65] = Some("PM8010");
> > > +    models[69] = Some("PM8550VS");
> > > +    models[70] = Some("PM8550VE");
> > > +    models[71] = Some("PM8550B");
> > > +    models[72] = Some("PMR735D");
> > > +    models[73] = Some("PM8550");
> > > +    models[74] = Some("PMK8550");
> > > +    models[78] = Some("PMM8650AU");
> > > +    models[79] = Some("PMM8650AU_PSAIL");
> > > +    models[80] = Some("PM7550");
> > > +    models[82] = Some("PMC8380");
> > > +    models[83] = Some("SMB2360");
> > > +    models[91] = Some("PMIV0108");
> > > +    models
> > > +};
> > > +
> > > diff --git a/drivers/soc/qcom/socinfo/socinfo.rs b/drivers/soc/qcom/socinfo/socinfo.rs
> > > new file mode 100644
> > > index 0000000000000000000000000000000000000000..bff1bf8fd672e3c461352075aa85ef8fdedff953
> > > --- /dev/null
> > > +++ b/drivers/soc/qcom/socinfo/socinfo.rs
> > > @@ -0,0 +1,362 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +
> > > +// Copyright (C) 2025 Google LLC.
> > > +
> > > +//! Re-implementation of Qualcomm's Socinfo driver in Rust
> >
> > I think this comment can be dropped. If it gets merged, there is no old
> > Socinfo driver.
> 
> Agreed. We do need a crate level doc comment, but it can just say
> "Qualcomm SocInfo Driver" or something.

Just drop it here, it will go to the commit message.

> 
> >
> > > +use core::fmt;
> > > +use core::fmt::Formatter;
> > > +use kernel::debugfs::{Scope, ScopedDir};
> > > +use kernel::device::Core;
> > > +use kernel::module_platform_driver;
> > > +use kernel::platform::{self, Device};
> > > +use kernel::prelude::{fmt, pin_data, Error, PinInit, Result};
> > > +use kernel::soc;
> > > +use kernel::str::{CStr, CStrExt, CString};
> > > +use kernel::transmute::FromBytes;
> > > +use kernel::{c_str, pr_warn, try_pin_init};
> > > +use pin_init::pin_init_scope;
> > > +
> >
> > > +        let versions = self.versions.unwrap_or(&[]);
> > > +        let versions2 = self.versions2.unwrap_or(&[]);
> > > +        for (version, image_name) in versions
> > > +            .iter()
> > > +            .take(32)
> > > +            .chain(versions2.iter())
> > > +            .zip(IMAGE_NAMES.iter())
> > > +        {
> > > +            version.build_debugfs(dir, image_name);
> > > +        }
> >
> > I like this idiomatic code, we even get what original driver misses:
> > size checks for those memory regions.
> >
> > > +    }
> > > +}
> > > +
> >
> > --
> > With best wishes
> > Dmitry

-- 
With best wishes
Dmitry

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Matthew Maurer]

I'll get to your other comments in a v2 soon, but wanted to focus on
the lifetime of `qcom_smem_get` buffers for a moment.

> > > > ---
> > >
> > > > +
> > > > +pub(crate) fn qcom_smem_get(host: i32, item: u32) -> Result<&'static [u8]> {
> > > > +    let mut size = 0;
> > > > +    // SAFETY: qcom_smem_get only requires that the size pointer be a writable size_t,
> > > > +    // host and item are error checked in the qcom_smem module.
> > > > +    let err_ptr = unsafe { kernel::bindings::qcom_smem_get(host as u32, item, &mut size) };
> > > > +    let ptr = from_err_ptr(err_ptr)?;
> > > > +    // SAFETY: If qcom_smem_get does not return an error, the returned pointer points to a readable
> > > > +    // byte buffer with its size written into size. Because these buffers are derived from the
> > > > +    // static ranges in the DT, this buffer remains accessible even if the qcom_smem module is
> > > > +    // unloaded, so 'static is appropriate. The underlying buffer cannot mutate, so upgrading it
> > > > +    // to a reference is allowed.
> > >
> > > This is definitely not true. The smem data is ioremap'ped during
> > > qcom_smem_probe() and will be unmapped if qcom_smem is unloaded.
> > > Accessing such a pointer after unloading smem would result in the
> > > exception. Current socinfo ignores that and keeps on accessing the data
> > > at runtime (which also allows it to read DSP versions which are updated
> > > at runtime), but the comment needs to be corrected.
> >
> > Thanks for the info. In v2 I'll look into seeing if I can make those
> > slices hold a reference on the smem module while they're alive.
>
> It should be a reference to the _bound_ device. I don't think we have
> a way to do it. I suggest just fixing the comment instead.

So, unfortunately fixing the comment doesn't really address this issue
- a `&'static [u8]` is a promise that this buffer is immutable and
will remain valid to read indefinitely, and it sounds like neither of
these are actually true (the former due to the DSP live updates, and
the latter due to the potential for the buffer to be unmapped).

I can deal with "this buffer may mutate" by changing to another
abstraction, but it'd help a lot if I understood the intended way
(even in C, we can ignore Rust for a moment) to know that the buffer
you got from `qcom_smem_get` is still valid. My best guess right now
is that because the smem driver is a *parent* driver to at least this
one, we know based on where we are instantiated that it will clean us
up before we go away. Is that the intended mechanism?

If that is the case, then question for the Rust maintainers, since
`probe` is not an `unsafe` function, could allow the equivalent of
safety requirements on probe methods? Since the match tables are
empty, it will only be probed if explicitly requested, so there's an
argument to be made for "this driver may only be probed by a device
descended from a qcom smem device".

>
> >
> > >
> > > > +    Ok(unsafe { core::slice::from_raw_parts(ptr as *const u8, size) })
> > > > +}
> > > > +

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Danilo Krummrich]

On Tue Dec 16, 2025 at 1:55 AM CET, Matthew Maurer wrote:
> I'll get to your other comments in a v2 soon, but wanted to focus on
> the lifetime of `qcom_smem_get` buffers for a moment.
>
>> > > > ---
>> > >
>> > > > +
>> > > > +pub(crate) fn qcom_smem_get(host: i32, item: u32) -> Result<&'static [u8]> {
>> > > > +    let mut size = 0;
>> > > > +    // SAFETY: qcom_smem_get only requires that the size pointer be a writable size_t,
>> > > > +    // host and item are error checked in the qcom_smem module.
>> > > > +    let err_ptr = unsafe { kernel::bindings::qcom_smem_get(host as u32, item, &mut size) };
>> > > > +    let ptr = from_err_ptr(err_ptr)?;
>> > > > +    // SAFETY: If qcom_smem_get does not return an error, the returned pointer points to a readable
>> > > > +    // byte buffer with its size written into size. Because these buffers are derived from the
>> > > > +    // static ranges in the DT, this buffer remains accessible even if the qcom_smem module is
>> > > > +    // unloaded, so 'static is appropriate. The underlying buffer cannot mutate, so upgrading it
>> > > > +    // to a reference is allowed.
>> > >
>> > > This is definitely not true. The smem data is ioremap'ped during
>> > > qcom_smem_probe() and will be unmapped if qcom_smem is unloaded.
>> > > Accessing such a pointer after unloading smem would result in the
>> > > exception. Current socinfo ignores that and keeps on accessing the data
>> > > at runtime (which also allows it to read DSP versions which are updated
>> > > at runtime), but the comment needs to be corrected.
>> >
>> > Thanks for the info. In v2 I'll look into seeing if I can make those
>> > slices hold a reference on the smem module while they're alive.
>>
>> It should be a reference to the _bound_ device. I don't think we have
>> a way to do it. I suggest just fixing the comment instead.
>
> So, unfortunately fixing the comment doesn't really address this issue
> - a `&'static [u8]` is a promise that this buffer is immutable and
> will remain valid to read indefinitely, and it sounds like neither of
> these are actually true (the former due to the DSP live updates, and
> the latter due to the potential for the buffer to be unmapped).
>
> I can deal with "this buffer may mutate" by changing to another
> abstraction, but it'd help a lot if I understood the intended way
> (even in C, we can ignore Rust for a moment) to know that the buffer
> you got from `qcom_smem_get` is still valid. My best guess right now
> is that because the smem driver is a *parent* driver to at least this
> one, we know based on where we are instantiated that it will clean us
> up before we go away. Is that the intended mechanism?
>
> If that is the case, then question for the Rust maintainers, since
> `probe` is not an `unsafe` function, could allow the equivalent of
> safety requirements on probe methods? Since the match tables are
> empty, it will only be probed if explicitly requested, so there's an
> argument to be made for "this driver may only be probed by a device
> descended from a qcom smem device".

This is a driver lifecycle problem that arises from the fact that in this case
you deal with two (technically independent) platform drivers: the QCOM SMEM one
and this one, the SoC info one.

At the same time though there are dependencies between the two, i.e. the SoC
info driver tries to access device resources of the SMEM driver, which requires
a lifetime relationship that mandates that the SoC info driver is unbound once
the SMEM driver is unbound.

So, rather than having two independent platform drivers the correct way to
handle this lifetime relationship through the driver-core would be to let the
SMEM driver create an auxiliary device and register the SoC info driver as
auxiliary driver rather than a platform driver.

With this you get the guarantee that throughout the whole lifetime of the SoC
info driver also the parent driver (QCOM SMEM) is bound and the corresponding
device resources are valid to access.

Or in other words, for "this driver may only be probed by a device descended
from a qcom smem device" the auxiliary bus models exactly this relationship and
is the way to go.

In order to make the precense of the SoC info driver still controllable by an OF
node, the QCOM SMEM driver can parse the OF node and register the auxiliary
device conditionally.

With this, your qcom_smem_get() method should take an additional &Device<Bound>
argument, which is the parent device of the auxiliary::Device<Bound> you get in
probe() in the SoC info driver. I.e. the parent() method of an
auxiliary::Device<Bound> provides you a &Device<Bound> of the parent as the
parent is guaranteed to be bound when the auxiliary device is bound, see also
[1].

Additionally, you should wrap the resulting &'static [u8] into a Devres [2]
container, which will revoke the slice when the parent device, i.e. the owner of
the resource behind the slice is unbound.

Given that we have all the required mechanisms in place (in Rust) to model such
lifetime relationships safely, I'd love to see them being used instead of a
comment mandating to do the right thing. :)

I hope this helps!

- Danilo

[1] https://rust.docs.kernel.org/kernel/auxiliary/struct.Device.html#method.parent
[2] https://rust.docs.kernel.org/kernel/devres/struct.Devres.html

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Matthew Maurer]

On Tue, Dec 16, 2025 at 12:49 PM Bjorn Andersson <andersson@kernel.org> wrote:
>
> On Sat, Dec 13, 2025 at 12:36:00AM +0000, Matthew Maurer wrote:
> > Re-implements qcom-socinfo driver in Rust, using `Scoped`-based DebugFS
> > bindings.
>
> Okay, but why?
>
> https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
>
> [..]

I'll move more of the text from the cover letter into the commit
message, but the short version is that it's intended to be a low-risk
driver to pilot vendor drivers in Android.

>
> > ---
> > base-commit: 4b31c90f86768367c84cc6e99e1deee0ec55197b
> > change-id: 20251029-qcom-socinfo-d8387c7fdb1c
> > prerequisite-change-id: 20251029-soc-bindings-9b0731bcdbed:v1
> > prerequisite-patch-id: e4da423ddabec93bd9a64004691266f9b740e0c5
> > prerequisite-patch-id: 5097ff622f8cb1d38354674cf70c1c946ac87f6c
> > prerequisite-change-id: 20251029-add-entropy-f57e12ebe110:v1
> > prerequisite-patch-id: 657de204912d2c5efff9898d3f4c51e52684d8e6
> > prerequisite-change-id: 20251212-transmute-8ab6076700a8:v1
> > prerequisite-patch-id: 4f5f7cb002d02d232083ab5c3ce6b3cb90650bd6
> > prerequisite-patch-id: 01a1f3d727e549c173dd142180741f5b6a3c866e
> > prerequisite-patch-id: ff801a7ae439876b1554b4d1132d94c825bbe74f
>
> So, it doesn't work on a clean v6.19-rc1 tree?

Correct. Those other patchsets are patchsets I'm landing which add
bindings used in this one.

>
> Regards,
> Bjorn
>
> >
> > Best regards,
> > --
> > Matthew Maurer <mmaurer@google.com>
> >

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Matthew Maurer]

On Tue, Dec 16, 2025 at 12:52 PM Bjorn Andersson <andersson@kernel.org> wrote:
>
> On Sat, Dec 13, 2025 at 08:24:55AM -0800, Matthew Maurer wrote:
> > On Sat, Dec 13, 2025 at 5:57 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > Shouldn't this all just be in 1 .rs file?  By not doing that, you have a
> > > number of public symbols (that are very generic), that will "pollute"
> > > the global namespace of the kernel symbol table (unless rust does this
> > > somehow differently?)
> >
> > Rust produces one compilation unit per "crate". In this case,
> > `socinfo` is the crate, and the other files are included as "modules".
> > Symbols defined in them are not public unless annotated with `pub`.
> > `pub(crate)` makes them only exported within the scope of the crate,
> > i.e. inside the single produced `.o` file.
> >
>
> Does this imply that a fully converted drivers/soc/qcom would have ~50
> subdirectories in it then?

Not necessarily. Directories are not mandatory for a Rust module,
whether to do things this way is a code structure/stylistic decision.
I did it in this case primarily because I wanted to separate out the
large in-code data table (e.g. SOC_IDS) and the logic for speaking to
the smem driver from the rest of the code. If you have a preference
for a single file, I can very easily put it all in a single file.

>
> Regards,
> Bjorn
>
> > > But putting this all in 1 file should solve all of that, and remove all
> > > global symbols.  Especially for just a small and simple driver like
> > > this, no need to split it up at all.
> >
> > I'm willing to put it all in one file if that's preferred for
> > organizational purposes, but as explained above, this shouldn't bloat
> > the symbol count in Rust. If it were my own codebase this would be
> > separate files, so up to y'all what level of granularity you'd prefer.
> >
> > >
> > > thanks,
> > >
> > > greg k-h

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Bjorn Andersson]

On Sat, Dec 13, 2025 at 12:36:00AM +0000, Matthew Maurer wrote:
> Re-implements qcom-socinfo driver in Rust, using `Scoped`-based DebugFS
> bindings.

Okay, but why?

https://docs.kernel.org/process/submitting-patches.html#describe-your-changes

[..]

> ---
> base-commit: 4b31c90f86768367c84cc6e99e1deee0ec55197b
> change-id: 20251029-qcom-socinfo-d8387c7fdb1c
> prerequisite-change-id: 20251029-soc-bindings-9b0731bcdbed:v1
> prerequisite-patch-id: e4da423ddabec93bd9a64004691266f9b740e0c5
> prerequisite-patch-id: 5097ff622f8cb1d38354674cf70c1c946ac87f6c
> prerequisite-change-id: 20251029-add-entropy-f57e12ebe110:v1
> prerequisite-patch-id: 657de204912d2c5efff9898d3f4c51e52684d8e6
> prerequisite-change-id: 20251212-transmute-8ab6076700a8:v1
> prerequisite-patch-id: 4f5f7cb002d02d232083ab5c3ce6b3cb90650bd6
> prerequisite-patch-id: 01a1f3d727e549c173dd142180741f5b6a3c866e
> prerequisite-patch-id: ff801a7ae439876b1554b4d1132d94c825bbe74f

So, it doesn't work on a clean v6.19-rc1 tree?

Regards,
Bjorn

> 
> Best regards,
> -- 
> Matthew Maurer <mmaurer@google.com>
> 

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Bjorn Andersson]

On Sat, Dec 13, 2025 at 08:24:55AM -0800, Matthew Maurer wrote:
> On Sat, Dec 13, 2025 at 5:57 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > Shouldn't this all just be in 1 .rs file?  By not doing that, you have a
> > number of public symbols (that are very generic), that will "pollute"
> > the global namespace of the kernel symbol table (unless rust does this
> > somehow differently?)
> 
> Rust produces one compilation unit per "crate". In this case,
> `socinfo` is the crate, and the other files are included as "modules".
> Symbols defined in them are not public unless annotated with `pub`.
> `pub(crate)` makes them only exported within the scope of the crate,
> i.e. inside the single produced `.o` file.
> 

Does this imply that a fully converted drivers/soc/qcom would have ~50
subdirectories in it then?

Regards,
Bjorn

> > But putting this all in 1 file should solve all of that, and remove all
> > global symbols.  Especially for just a small and simple driver like
> > this, no need to split it up at all.
> 
> I'm willing to put it all in one file if that's preferred for
> organizational purposes, but as explained above, this shouldn't bloat
> the symbol count in Rust. If it were my own codebase this would be
> separate files, so up to y'all what level of granularity you'd prefer.
> 
> >
> > thanks,
> >
> > greg k-h

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Bjorn Andersson]

On Tue, Dec 16, 2025 at 12:53:28PM -0800, Matthew Maurer wrote:
> On Tue, Dec 16, 2025 at 12:49 PM Bjorn Andersson <andersson@kernel.org> wrote:
> >
> > On Sat, Dec 13, 2025 at 12:36:00AM +0000, Matthew Maurer wrote:
> > > Re-implements qcom-socinfo driver in Rust, using `Scoped`-based DebugFS
> > > bindings.
> >
> > Okay, but why?
> >
> > https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> >
> > [..]
> 
> I'll move more of the text from the cover letter into the commit
> message, but the short version is that it's intended to be a low-risk
> driver to pilot vendor drivers in Android.
> 

The answers I'm looking for isn't in your cover letter either. I want to
know what benefits Rust provides in this particular case.

> >
> > > ---
> > > base-commit: 4b31c90f86768367c84cc6e99e1deee0ec55197b
> > > change-id: 20251029-qcom-socinfo-d8387c7fdb1c
> > > prerequisite-change-id: 20251029-soc-bindings-9b0731bcdbed:v1
> > > prerequisite-patch-id: e4da423ddabec93bd9a64004691266f9b740e0c5
> > > prerequisite-patch-id: 5097ff622f8cb1d38354674cf70c1c946ac87f6c
> > > prerequisite-change-id: 20251029-add-entropy-f57e12ebe110:v1
> > > prerequisite-patch-id: 657de204912d2c5efff9898d3f4c51e52684d8e6
> > > prerequisite-change-id: 20251212-transmute-8ab6076700a8:v1
> > > prerequisite-patch-id: 4f5f7cb002d02d232083ab5c3ce6b3cb90650bd6
> > > prerequisite-patch-id: 01a1f3d727e549c173dd142180741f5b6a3c866e
> > > prerequisite-patch-id: ff801a7ae439876b1554b4d1132d94c825bbe74f
> >
> > So, it doesn't work on a clean v6.19-rc1 tree?
> 
> Correct. Those other patchsets are patchsets I'm landing which add
> bindings used in this one.
> 

Okay, please make that explicit when submitting patches, so that the
maintainers know that your patch(es) can't be merged.

Regards,
Bjorn

> >
> > Regards,
> > Bjorn
> >
> > >
> > > Best regards,
> > > --
> > > Matthew Maurer <mmaurer@google.com>
> > >

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Matthew Maurer]

On Tue, Dec 16, 2025 at 1:53 PM Bjorn Andersson <andersson@kernel.org> wrote:
>
> On Tue, Dec 16, 2025 at 12:53:28PM -0800, Matthew Maurer wrote:
> > On Tue, Dec 16, 2025 at 12:49 PM Bjorn Andersson <andersson@kernel.org> wrote:
> > >
> > > On Sat, Dec 13, 2025 at 12:36:00AM +0000, Matthew Maurer wrote:
> > > > Re-implements qcom-socinfo driver in Rust, using `Scoped`-based DebugFS
> > > > bindings.
> > >
> > > Okay, but why?
> > >
> > > https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> > >
> > > [..]
> >
> > I'll move more of the text from the cover letter into the commit
> > message, but the short version is that it's intended to be a low-risk
> > driver to pilot vendor drivers in Android.
> >
>
> The answers I'm looking for isn't in your cover letter either. I want to
> know what benefits Rust provides in this particular case.
>

Rust likely does not provide any significant benefits for the socinfo
driver itself, aside from having caught the slight disconnect
(mentioned on the other thread) where it should probably be an
auxiliary device because it assumes that the `qcom-smem` device will
remain active as long as it does.

The primary intention of converting this driver is to be a low risk
trial for GKI's ABI stability for vendor modules and a first Rust
vendor module (we have platform modules already for binder and ashmem)
on Android. This was discussed informally with Trilok (a long time
ago) to act as an example for other vendor drivers and encourage usage
of Rust for new drivers or those where memory safety has been a
problem in the past, but nothing was ever formally committed to.
Trying to apply Rust to more significant existing modules (where it
would provide a greater benefit) raises concern about whether they
will operate identically. Socinfo in particular can be more thoroughly
tested for equivalency, as aside from the runtime DSP information
modification that was mentioned in this discussion, the output of the
socinfo driver is static and can just be checked. This means that a
socinfo driver swap will essentially have the language change be the
*only* significant thing about the change, keeping the risk to a
minimum.

Part of the reason for the RFC prefix on this patch is to solicit your
feedback on whether you are open to this. If you have somewhere else
you'd prefer we try this, I'd be open to that as well.

> > >
> > > > ---
> > > > base-commit: 4b31c90f86768367c84cc6e99e1deee0ec55197b
> > > > change-id: 20251029-qcom-socinfo-d8387c7fdb1c
> > > > prerequisite-change-id: 20251029-soc-bindings-9b0731bcdbed:v1
> > > > prerequisite-patch-id: e4da423ddabec93bd9a64004691266f9b740e0c5
> > > > prerequisite-patch-id: 5097ff622f8cb1d38354674cf70c1c946ac87f6c
> > > > prerequisite-change-id: 20251029-add-entropy-f57e12ebe110:v1
> > > > prerequisite-patch-id: 657de204912d2c5efff9898d3f4c51e52684d8e6
> > > > prerequisite-change-id: 20251212-transmute-8ab6076700a8:v1
> > > > prerequisite-patch-id: 4f5f7cb002d02d232083ab5c3ce6b3cb90650bd6
> > > > prerequisite-patch-id: 01a1f3d727e549c173dd142180741f5b6a3c866e
> > > > prerequisite-patch-id: ff801a7ae439876b1554b4d1132d94c825bbe74f
> > >
> > > So, it doesn't work on a clean v6.19-rc1 tree?
> >
> > Correct. Those other patchsets are patchsets I'm landing which add
> > bindings used in this one.
> >
>
> Okay, please make that explicit when submitting patches, so that the
> maintainers know that your patch(es) can't be merged.

I thought that was what I had done by using the combination of "RFC"
and putting the prerequisite change IDs from b4, but I will include a
statement to that effect in my cover letter in the future.

>
> Regards,
> Bjorn
>
> > >
> > > Regards,
> > > Bjorn
> > >
> > > >
> > > > Best regards,
> > > > --
> > > > Matthew Maurer <mmaurer@google.com>
> > > >

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Bjorn Andersson]

On Tue, Dec 16, 2025 at 12:55:11PM -0800, Matthew Maurer wrote:
> On Tue, Dec 16, 2025 at 12:52 PM Bjorn Andersson <andersson@kernel.org> wrote:
> >
> > On Sat, Dec 13, 2025 at 08:24:55AM -0800, Matthew Maurer wrote:
> > > On Sat, Dec 13, 2025 at 5:57 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > Shouldn't this all just be in 1 .rs file?  By not doing that, you have a
> > > > number of public symbols (that are very generic), that will "pollute"
> > > > the global namespace of the kernel symbol table (unless rust does this
> > > > somehow differently?)
> > >
> > > Rust produces one compilation unit per "crate". In this case,
> > > `socinfo` is the crate, and the other files are included as "modules".
> > > Symbols defined in them are not public unless annotated with `pub`.
> > > `pub(crate)` makes them only exported within the scope of the crate,
> > > i.e. inside the single produced `.o` file.
> > >
> >
> > Does this imply that a fully converted drivers/soc/qcom would have ~50
> > subdirectories in it then?
> 
> Not necessarily. Directories are not mandatory for a Rust module,
> whether to do things this way is a code structure/stylistic decision.
> I did it in this case primarily because I wanted to separate out the
> large in-code data table (e.g. SOC_IDS) and the logic for speaking to
> the smem driver from the rest of the code.

Separating out the logic for interfacing the smem driver sounds like a
quite reasonable proposition, to the point that I'd expect this to be a
module of its own?

> If you have a preference
> for a single file, I can very easily put it all in a single file.

My preference is that this follows whatever is the idiomatic model for
in-kernel Rust. Whichever way that pulls us would be the answer to my
question above...

Regards,
Bjorn

> 
> >
> > Regards,
> > Bjorn
> >
> > > > But putting this all in 1 file should solve all of that, and remove all
> > > > global symbols.  Especially for just a small and simple driver like
> > > > this, no need to split it up at all.
> > >
> > > I'm willing to put it all in one file if that's preferred for
> > > organizational purposes, but as explained above, this shouldn't bloat
> > > the symbol count in Rust. If it were my own codebase this would be
> > > separate files, so up to y'all what level of granularity you'd prefer.
> > >
> > > >
> > > > thanks,
> > > >
> > > > greg k-h

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Matthew Maurer]

On Tue, Dec 16, 2025 at 2:06 PM Bjorn Andersson <andersson@kernel.org> wrote:
>
> On Tue, Dec 16, 2025 at 12:55:11PM -0800, Matthew Maurer wrote:
> > On Tue, Dec 16, 2025 at 12:52 PM Bjorn Andersson <andersson@kernel.org> wrote:
> > >
> > > On Sat, Dec 13, 2025 at 08:24:55AM -0800, Matthew Maurer wrote:
> > > > On Sat, Dec 13, 2025 at 5:57 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > >
> > > > > Shouldn't this all just be in 1 .rs file?  By not doing that, you have a
> > > > > number of public symbols (that are very generic), that will "pollute"
> > > > > the global namespace of the kernel symbol table (unless rust does this
> > > > > somehow differently?)
> > > >
> > > > Rust produces one compilation unit per "crate". In this case,
> > > > `socinfo` is the crate, and the other files are included as "modules".
> > > > Symbols defined in them are not public unless annotated with `pub`.
> > > > `pub(crate)` makes them only exported within the scope of the crate,
> > > > i.e. inside the single produced `.o` file.
> > > >
> > >
> > > Does this imply that a fully converted drivers/soc/qcom would have ~50
> > > subdirectories in it then?
> >
> > Not necessarily. Directories are not mandatory for a Rust module,
> > whether to do things this way is a code structure/stylistic decision.
> > I did it in this case primarily because I wanted to separate out the
> > large in-code data table (e.g. SOC_IDS) and the logic for speaking to
> > the smem driver from the rest of the code.
>
> Separating out the logic for interfacing the smem driver sounds like a
> quite reasonable proposition, to the point that I'd expect this to be a
> module of its own?

Currently the socinfo driver is the only user of that code, so
splitting it into a full kernel module would just add overhead to no
benefit. If we had multiple Rust drivers, we could consider between
either referencing the same file multiple places (would cause minor
code bloat, but avoid module overhead) or making it a module.

>
> > If you have a preference
> > for a single file, I can very easily put it all in a single file.
>
> My preference is that this follows whatever is the idiomatic model for
> in-kernel Rust. Whichever way that pulls us would be the answer to my
> question above...

drivers/gpu/{drm/nova,nova-core} use multi-file layout (with a single
kernel module, and multiple Rust files), so I would say that it is
idiomatic to use multiple files to define a single kernel module.

As far as a directory per kernel module, it generally appears that
multi-file Rust modules happen to have an exclusive directory with the
exception of the binder driver. However, that may be coincidence, as
most of those are defining a single high level kernel module, not a
cluster of related kernel modules.

Danilo or Miguel might have an opinion here on when directories should
be used? The Rust Coding Guidelines [1] don't seem to have any
opinions on directory structure, and I was unable to find any generic
guidance for general kernel development.

Were it my project, I would use directories for any Rust module that
was multi-file, and just use a regular file for Rust modules which
don't have a natural division into multiple pieces.

[1]: https://docs.kernel.org/rust/coding-guidelines.html

>
> Regards,
> Bjorn
>
> >
> > >
> > > Regards,
> > > Bjorn
> > >
> > > > > But putting this all in 1 file should solve all of that, and remove all
> > > > > global symbols.  Especially for just a small and simple driver like
> > > > > this, no need to split it up at all.
> > > >
> > > > I'm willing to put it all in one file if that's preferred for
> > > > organizational purposes, but as explained above, this shouldn't bloat
> > > > the symbol count in Rust. If it were my own codebase this would be
> > > > separate files, so up to y'all what level of granularity you'd prefer.
> > > >
> > > > >
> > > > > thanks,
> > > > >
> > > > > greg k-h

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Bjorn Andersson]

On Tue, Dec 16, 2025 at 02:13:08PM -0800, Matthew Maurer wrote:
> On Tue, Dec 16, 2025 at 1:53 PM Bjorn Andersson <andersson@kernel.org> wrote:
> >
> > On Tue, Dec 16, 2025 at 12:53:28PM -0800, Matthew Maurer wrote:
> > > On Tue, Dec 16, 2025 at 12:49 PM Bjorn Andersson <andersson@kernel.org> wrote:
> > > >
> > > > On Sat, Dec 13, 2025 at 12:36:00AM +0000, Matthew Maurer wrote:
> > > > > Re-implements qcom-socinfo driver in Rust, using `Scoped`-based DebugFS
> > > > > bindings.
> > > >
> > > > Okay, but why?
> > > >
> > > > https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> > > >
> > > > [..]
> > >
> > > I'll move more of the text from the cover letter into the commit
> > > message, but the short version is that it's intended to be a low-risk
> > > driver to pilot vendor drivers in Android.
> > >
> >
> > The answers I'm looking for isn't in your cover letter either. I want to
> > know what benefits Rust provides in this particular case.
> >
> 
> Rust likely does not provide any significant benefits for the socinfo
> driver itself, aside from having caught the slight disconnect
> (mentioned on the other thread) where it should probably be an
> auxiliary device because it assumes that the `qcom-smem` device will
> remain active as long as it does.
> 

Yes, this isn't unique to Rust. The smem API would benefit from an
overhaul in general... At the time I wrote it, you couldn't really boot
the system without SMEM, so I added the suppress_bind_attrs, these days
you can exercise a fair amount of use cases without it.

> The primary intention of converting this driver is to be a low risk
> trial for GKI's ABI stability for vendor modules and a first Rust
> vendor module (we have platform modules already for binder and ashmem)
> on Android. This was discussed informally with Trilok (a long time
> ago) to act as an example for other vendor drivers and encourage usage
> of Rust for new drivers or those where memory safety has been a
> problem in the past, but nothing was ever formally committed to.

But if the test bed for GKI ABI stability is the goal, don't you need
something with a more interesting interface?

Also, how would this manifest itself in the upstream kernel? Do you have
a test bed where you build GKI + vendor "kernel" out of the upstream
kernel tree today?

> Trying to apply Rust to more significant existing modules (where it
> would provide a greater benefit) raises concern about whether they
> will operate identically. Socinfo in particular can be more thoroughly
> tested for equivalency, as aside from the runtime DSP information
> modification that was mentioned in this discussion, the output of the
> socinfo driver is static and can just be checked. This means that a
> socinfo driver swap will essentially have the language change be the
> *only* significant thing about the change, keeping the risk to a
> minimum.
> 
> Part of the reason for the RFC prefix on this patch is to solicit your
> feedback on whether you are open to this. If you have somewhere else
> you'd prefer we try this, I'd be open to that as well.
> 

Rust is a new dependency and it's a foreign language to the vast
majority of the contributors in the upstream Qualcomm community.

So, IMHO sufficient value needs to come with such rewrite, and this
value needs to be properly documented in the git history. Rewriting
things for the sake of rewriting things, or the facilitate a downstream
experiment, is hard for me to force onto our community.

Regards,
Bjorn

> > > >
> > > > > ---
> > > > > base-commit: 4b31c90f86768367c84cc6e99e1deee0ec55197b
> > > > > change-id: 20251029-qcom-socinfo-d8387c7fdb1c
> > > > > prerequisite-change-id: 20251029-soc-bindings-9b0731bcdbed:v1
> > > > > prerequisite-patch-id: e4da423ddabec93bd9a64004691266f9b740e0c5
> > > > > prerequisite-patch-id: 5097ff622f8cb1d38354674cf70c1c946ac87f6c
> > > > > prerequisite-change-id: 20251029-add-entropy-f57e12ebe110:v1
> > > > > prerequisite-patch-id: 657de204912d2c5efff9898d3f4c51e52684d8e6
> > > > > prerequisite-change-id: 20251212-transmute-8ab6076700a8:v1
> > > > > prerequisite-patch-id: 4f5f7cb002d02d232083ab5c3ce6b3cb90650bd6
> > > > > prerequisite-patch-id: 01a1f3d727e549c173dd142180741f5b6a3c866e
> > > > > prerequisite-patch-id: ff801a7ae439876b1554b4d1132d94c825bbe74f
> > > >
> > > > So, it doesn't work on a clean v6.19-rc1 tree?
> > >
> > > Correct. Those other patchsets are patchsets I'm landing which add
> > > bindings used in this one.
> > >
> >
> > Okay, please make that explicit when submitting patches, so that the
> > maintainers know that your patch(es) can't be merged.
> 
> I thought that was what I had done by using the combination of "RFC"
> and putting the prerequisite change IDs from b4, but I will include a
> statement to that effect in my cover letter in the future.
> 
> >
> > Regards,
> > Bjorn
> >
> > > >
> > > > Regards,
> > > > Bjorn
> > > >
> > > > >
> > > > > Best regards,
> > > > > --
> > > > > Matthew Maurer <mmaurer@google.com>
> > > > >

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Matthew Maurer]

On Tue, Dec 16, 2025 at 3:05 PM Bjorn Andersson <andersson@kernel.org> wrote:
>
> On Tue, Dec 16, 2025 at 02:13:08PM -0800, Matthew Maurer wrote:
> > On Tue, Dec 16, 2025 at 1:53 PM Bjorn Andersson <andersson@kernel.org> wrote:
> > >
> > > On Tue, Dec 16, 2025 at 12:53:28PM -0800, Matthew Maurer wrote:
> > > > On Tue, Dec 16, 2025 at 12:49 PM Bjorn Andersson <andersson@kernel.org> wrote:
> > > > >
> > > > > On Sat, Dec 13, 2025 at 12:36:00AM +0000, Matthew Maurer wrote:
> > > > > > Re-implements qcom-socinfo driver in Rust, using `Scoped`-based DebugFS
> > > > > > bindings.
> > > > >
> > > > > Okay, but why?
> > > > >
> > > > > https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> > > > >
> > > > > [..]
> > > >
> > > > I'll move more of the text from the cover letter into the commit
> > > > message, but the short version is that it's intended to be a low-risk
> > > > driver to pilot vendor drivers in Android.
> > > >
> > >
> > > The answers I'm looking for isn't in your cover letter either. I want to
> > > know what benefits Rust provides in this particular case.
> > >
> >
> > Rust likely does not provide any significant benefits for the socinfo
> > driver itself, aside from having caught the slight disconnect
> > (mentioned on the other thread) where it should probably be an
> > auxiliary device because it assumes that the `qcom-smem` device will
> > remain active as long as it does.
> >
>
> Yes, this isn't unique to Rust. The smem API would benefit from an
> overhaul in general... At the time I wrote it, you couldn't really boot
> the system without SMEM, so I added the suppress_bind_attrs, these days
> you can exercise a fair amount of use cases without it.
>
> > The primary intention of converting this driver is to be a low risk
> > trial for GKI's ABI stability for vendor modules and a first Rust
> > vendor module (we have platform modules already for binder and ashmem)
> > on Android. This was discussed informally with Trilok (a long time
> > ago) to act as an example for other vendor drivers and encourage usage
> > of Rust for new drivers or those where memory safety has been a
> > problem in the past, but nothing was ever formally committed to.
>
> But if the test bed for GKI ABI stability is the goal, don't you need
> something with a more interesting interface?

We've been watching things manually for the 6.12 release and manually
backtested on the 6.6 release (by looking at our ABI dumps) and
believe that it works. The only way we can really progress further is
by deploying a vendor driver in the field. If we pick something more
interesting, there was concern around it being caught up on concerns
related to the specific driver. The hope was that this driver would
simultaneously be real enough to count as a driver in the eyes of
other vendors watching for someone to take the lead, and simple enough
to be low risk enough to be possible.

We do have more interesting drivers in process, e.g. the tyr graphics
driver, but those are longer term efforts.

>
> Also, how would this manifest itself in the upstream kernel? Do you have
> a test bed where you build GKI + vendor "kernel" out of the upstream
> kernel tree today?

In Android, kernel modules are split into two categories - "GKI
modules", which get shipped with the primary kernel to every system,
and vendor modules, which are shipped and loaded by particular
devices. Neither current builtin modules [1] nor current GKI modules
[2] contain the qcom socinfo, so it is a vendor module. This
categorization is not an "in-tree" / "out of tree" categorization, but
a "every Android device" / "some Android devices" one.

[1] https://ci.android.com/builds/submitted/14027866/kernel_aarch64/latest/modules.builtin
[2] https://android.googlesource.com/kernel/common/+/refs/heads/android16-6.12/modules.bzl
>
> > Trying to apply Rust to more significant existing modules (where it
> > would provide a greater benefit) raises concern about whether they
> > will operate identically. Socinfo in particular can be more thoroughly
> > tested for equivalency, as aside from the runtime DSP information
> > modification that was mentioned in this discussion, the output of the
> > socinfo driver is static and can just be checked. This means that a
> > socinfo driver swap will essentially have the language change be the
> > *only* significant thing about the change, keeping the risk to a
> > minimum.
> >
> > Part of the reason for the RFC prefix on this patch is to solicit your
> > feedback on whether you are open to this. If you have somewhere else
> > you'd prefer we try this, I'd be open to that as well.
> >
>
> Rust is a new dependency and it's a foreign language to the vast
> majority of the contributors in the upstream Qualcomm community.
>
> So, IMHO sufficient value needs to come with such rewrite, and this
> value needs to be properly documented in the git history. Rewriting
> things for the sake of rewriting things, or the facilitate a downstream
> experiment, is hard for me to force onto our community.

Since you brought it up, I took a brief look and found CVE-2024-58007
[3] , which would have been prevented through use of Rust, as the
access would have been bounds-checked. I understand that such
vulnerabilities in socinfo.c are likely to be uncommon, but even for a
driver this simple memory safety issues do happen, and have even
happened recently.

[3]: https://nvd.nist.gov/vuln/detail/CVE-2024-58007
>
> Regards,
> Bjorn
>
> > > > >
> > > > > > ---
> > > > > > base-commit: 4b31c90f86768367c84cc6e99e1deee0ec55197b
> > > > > > change-id: 20251029-qcom-socinfo-d8387c7fdb1c
> > > > > > prerequisite-change-id: 20251029-soc-bindings-9b0731bcdbed:v1
> > > > > > prerequisite-patch-id: e4da423ddabec93bd9a64004691266f9b740e0c5
> > > > > > prerequisite-patch-id: 5097ff622f8cb1d38354674cf70c1c946ac87f6c
> > > > > > prerequisite-change-id: 20251029-add-entropy-f57e12ebe110:v1
> > > > > > prerequisite-patch-id: 657de204912d2c5efff9898d3f4c51e52684d8e6
> > > > > > prerequisite-change-id: 20251212-transmute-8ab6076700a8:v1
> > > > > > prerequisite-patch-id: 4f5f7cb002d02d232083ab5c3ce6b3cb90650bd6
> > > > > > prerequisite-patch-id: 01a1f3d727e549c173dd142180741f5b6a3c866e
> > > > > > prerequisite-patch-id: ff801a7ae439876b1554b4d1132d94c825bbe74f
> > > > >
> > > > > So, it doesn't work on a clean v6.19-rc1 tree?
> > > >
> > > > Correct. Those other patchsets are patchsets I'm landing which add
> > > > bindings used in this one.
> > > >
> > >
> > > Okay, please make that explicit when submitting patches, so that the
> > > maintainers know that your patch(es) can't be merged.
> >
> > I thought that was what I had done by using the combination of "RFC"
> > and putting the prerequisite change IDs from b4, but I will include a
> > statement to that effect in my cover letter in the future.
> >
> > >
> > > Regards,
> > > Bjorn
> > >
> > > > >
> > > > > Regards,
> > > > > Bjorn
> > > > >
> > > > > >
> > > > > > Best regards,
> > > > > > --
> > > > > > Matthew Maurer <mmaurer@google.com>
> > > > > >

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Bjorn Andersson]

On Tue, Dec 16, 2025 at 03:32:53PM -0800, Matthew Maurer wrote:
> On Tue, Dec 16, 2025 at 3:05 PM Bjorn Andersson <andersson@kernel.org> wrote:
> >
> > On Tue, Dec 16, 2025 at 02:13:08PM -0800, Matthew Maurer wrote:
> > > On Tue, Dec 16, 2025 at 1:53 PM Bjorn Andersson <andersson@kernel.org> wrote:
> > > >
> > > > On Tue, Dec 16, 2025 at 12:53:28PM -0800, Matthew Maurer wrote:
> > > > > On Tue, Dec 16, 2025 at 12:49 PM Bjorn Andersson <andersson@kernel.org> wrote:
> > > > > >
> > > > > > On Sat, Dec 13, 2025 at 12:36:00AM +0000, Matthew Maurer wrote:
> > > > > > > Re-implements qcom-socinfo driver in Rust, using `Scoped`-based DebugFS
> > > > > > > bindings.
> > > > > >
> > > > > > Okay, but why?
> > > > > >
> > > > > > https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> > > > > >
> > > > > > [..]
> > > > >
> > > > > I'll move more of the text from the cover letter into the commit
> > > > > message, but the short version is that it's intended to be a low-risk
> > > > > driver to pilot vendor drivers in Android.
> > > > >
> > > >
> > > > The answers I'm looking for isn't in your cover letter either. I want to
> > > > know what benefits Rust provides in this particular case.
> > > >
> > >
> > > Rust likely does not provide any significant benefits for the socinfo
> > > driver itself, aside from having caught the slight disconnect
> > > (mentioned on the other thread) where it should probably be an
> > > auxiliary device because it assumes that the `qcom-smem` device will
> > > remain active as long as it does.
> > >
> >
> > Yes, this isn't unique to Rust. The smem API would benefit from an
> > overhaul in general... At the time I wrote it, you couldn't really boot
> > the system without SMEM, so I added the suppress_bind_attrs, these days
> > you can exercise a fair amount of use cases without it.
> >
> > > The primary intention of converting this driver is to be a low risk
> > > trial for GKI's ABI stability for vendor modules and a first Rust
> > > vendor module (we have platform modules already for binder and ashmem)
> > > on Android. This was discussed informally with Trilok (a long time
> > > ago) to act as an example for other vendor drivers and encourage usage
> > > of Rust for new drivers or those where memory safety has been a
> > > problem in the past, but nothing was ever formally committed to.
> >
> > But if the test bed for GKI ABI stability is the goal, don't you need
> > something with a more interesting interface?
> 
> We've been watching things manually for the 6.12 release and manually
> backtested on the 6.6 release (by looking at our ABI dumps) and
> believe that it works. The only way we can really progress further is
> by deploying a vendor driver in the field.

But this is the upstream kernel, you're not contributing to 6.6, 6.12,
or even 6.18. If we merge this rewrite for v6.20 it will be deploying
into the field in 2027.

Perhaps I'm misunderstanding what you're saying here though?

> If we pick something more
> interesting, there was concern around it being caught up on concerns
> related to the specific driver. The hope was that this driver would
> simultaneously be real enough to count as a driver in the eyes of
> other vendors watching for someone to take the lead, and simple enough
> to be low risk enough to be possible.
> 

Are you talking about upstream drivers, or downstream vendor drivers?

> We do have more interesting drivers in process, e.g. the tyr graphics
> driver, but those are longer term efforts.
> 
> >
> > Also, how would this manifest itself in the upstream kernel? Do you have
> > a test bed where you build GKI + vendor "kernel" out of the upstream
> > kernel tree today?
> 
> In Android, kernel modules are split into two categories - "GKI
> modules", which get shipped with the primary kernel to every system,
> and vendor modules, which are shipped and loaded by particular
> devices. Neither current builtin modules [1] nor current GKI modules
> [2] contain the qcom socinfo, so it is a vendor module. This
> categorization is not an "in-tree" / "out of tree" categorization, but
> a "every Android device" / "some Android devices" one.
> 

Forgive me, but I'm reading this as "Android downstream" + "Qualcomm
downstream", my question was how this relates to the upstream kernel.

> [1] https://ci.android.com/builds/submitted/14027866/kernel_aarch64/latest/modules.builtin
> [2] https://android.googlesource.com/kernel/common/+/refs/heads/android16-6.12/modules.bzl
> >
> > > Trying to apply Rust to more significant existing modules (where it
> > > would provide a greater benefit) raises concern about whether they
> > > will operate identically. Socinfo in particular can be more thoroughly
> > > tested for equivalency, as aside from the runtime DSP information
> > > modification that was mentioned in this discussion, the output of the
> > > socinfo driver is static and can just be checked. This means that a
> > > socinfo driver swap will essentially have the language change be the
> > > *only* significant thing about the change, keeping the risk to a
> > > minimum.
> > >
> > > Part of the reason for the RFC prefix on this patch is to solicit your
> > > feedback on whether you are open to this. If you have somewhere else
> > > you'd prefer we try this, I'd be open to that as well.
> > >
> >
> > Rust is a new dependency and it's a foreign language to the vast
> > majority of the contributors in the upstream Qualcomm community.
> >
> > So, IMHO sufficient value needs to come with such rewrite, and this
> > value needs to be properly documented in the git history. Rewriting
> > things for the sake of rewriting things, or the facilitate a downstream
> > experiment, is hard for me to force onto our community.
> 
> Since you brought it up, I took a brief look and found CVE-2024-58007
> [3] , which would have been prevented through use of Rust, as the
> access would have been bounds-checked. I understand that such
> vulnerabilities in socinfo.c are likely to be uncommon, but even for a
> driver this simple memory safety issues do happen, and have even
> happened recently.
> 

Yes, this is exactly the kind of argumentation I would like to see.

This is something we can weigh against the added cost of the Rust
implementation.

Regards,
Bjorn

> [3]: https://nvd.nist.gov/vuln/detail/CVE-2024-58007
> >
> > Regards,
> > Bjorn
> >
> > > > > >
> > > > > > > ---
> > > > > > > base-commit: 4b31c90f86768367c84cc6e99e1deee0ec55197b
> > > > > > > change-id: 20251029-qcom-socinfo-d8387c7fdb1c
> > > > > > > prerequisite-change-id: 20251029-soc-bindings-9b0731bcdbed:v1
> > > > > > > prerequisite-patch-id: e4da423ddabec93bd9a64004691266f9b740e0c5
> > > > > > > prerequisite-patch-id: 5097ff622f8cb1d38354674cf70c1c946ac87f6c
> > > > > > > prerequisite-change-id: 20251029-add-entropy-f57e12ebe110:v1
> > > > > > > prerequisite-patch-id: 657de204912d2c5efff9898d3f4c51e52684d8e6
> > > > > > > prerequisite-change-id: 20251212-transmute-8ab6076700a8:v1
> > > > > > > prerequisite-patch-id: 4f5f7cb002d02d232083ab5c3ce6b3cb90650bd6
> > > > > > > prerequisite-patch-id: 01a1f3d727e549c173dd142180741f5b6a3c866e
> > > > > > > prerequisite-patch-id: ff801a7ae439876b1554b4d1132d94c825bbe74f
> > > > > >
> > > > > > So, it doesn't work on a clean v6.19-rc1 tree?
> > > > >
> > > > > Correct. Those other patchsets are patchsets I'm landing which add
> > > > > bindings used in this one.
> > > > >
> > > >
> > > > Okay, please make that explicit when submitting patches, so that the
> > > > maintainers know that your patch(es) can't be merged.
> > >
> > > I thought that was what I had done by using the combination of "RFC"
> > > and putting the prerequisite change IDs from b4, but I will include a
> > > statement to that effect in my cover letter in the future.
> > >
> > > >
> > > > Regards,
> > > > Bjorn
> > > >
> > > > > >
> > > > > > Regards,
> > > > > > Bjorn
> > > > > >
> > > > > > >
> > > > > > > Best regards,
> > > > > > > --
> > > > > > > Matthew Maurer <mmaurer@google.com>
> > > > > > >

Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

[Matthew Maurer]

On Tue, Dec 16, 2025 at 4:15 PM Bjorn Andersson <andersson@kernel.org> wrote:
>
> On Tue, Dec 16, 2025 at 03:32:53PM -0800, Matthew Maurer wrote:
> > On Tue, Dec 16, 2025 at 3:05 PM Bjorn Andersson <andersson@kernel.org> wrote:
> > >
> > > On Tue, Dec 16, 2025 at 02:13:08PM -0800, Matthew Maurer wrote:
> > > > On Tue, Dec 16, 2025 at 1:53 PM Bjorn Andersson <andersson@kernel.org> wrote:
> > > > >
> > > > > On Tue, Dec 16, 2025 at 12:53:28PM -0800, Matthew Maurer wrote:
> > > > > > On Tue, Dec 16, 2025 at 12:49 PM Bjorn Andersson <andersson@kernel.org> wrote:
> > > > > > >
> > > > > > > On Sat, Dec 13, 2025 at 12:36:00AM +0000, Matthew Maurer wrote:
> > > > > > > > Re-implements qcom-socinfo driver in Rust, using `Scoped`-based DebugFS
> > > > > > > > bindings.
> > > > > > >
> > > > > > > Okay, but why?
> > > > > > >
> > > > > > > https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> > > > > > >
> > > > > > > [..]
> > > > > >
> > > > > > I'll move more of the text from the cover letter into the commit
> > > > > > message, but the short version is that it's intended to be a low-risk
> > > > > > driver to pilot vendor drivers in Android.
> > > > > >
> > > > >
> > > > > The answers I'm looking for isn't in your cover letter either. I want to
> > > > > know what benefits Rust provides in this particular case.
> > > > >
> > > >
> > > > Rust likely does not provide any significant benefits for the socinfo
> > > > driver itself, aside from having caught the slight disconnect
> > > > (mentioned on the other thread) where it should probably be an
> > > > auxiliary device because it assumes that the `qcom-smem` device will
> > > > remain active as long as it does.
> > > >
> > >
> > > Yes, this isn't unique to Rust. The smem API would benefit from an
> > > overhaul in general... At the time I wrote it, you couldn't really boot
> > > the system without SMEM, so I added the suppress_bind_attrs, these days
> > > you can exercise a fair amount of use cases without it.
> > >
> > > > The primary intention of converting this driver is to be a low risk
> > > > trial for GKI's ABI stability for vendor modules and a first Rust
> > > > vendor module (we have platform modules already for binder and ashmem)
> > > > on Android. This was discussed informally with Trilok (a long time
> > > > ago) to act as an example for other vendor drivers and encourage usage
> > > > of Rust for new drivers or those where memory safety has been a
> > > > problem in the past, but nothing was ever formally committed to.
> > >
> > > But if the test bed for GKI ABI stability is the goal, don't you need
> > > something with a more interesting interface?
> >
> > We've been watching things manually for the 6.12 release and manually
> > backtested on the 6.6 release (by looking at our ABI dumps) and
> > believe that it works. The only way we can really progress further is
> > by deploying a vendor driver in the field.
>
> But this is the upstream kernel, you're not contributing to 6.6, 6.12,
> or even 6.18. If we merge this rewrite for v6.20 it will be deploying
> into the field in 2027.

Android's 6.18 can take backports, and we prefer to do everything
upstream-first unless there's a reason not to. If this lands upstream,
we would backport it onto the Android 6.18 branch.

>
> Perhaps I'm misunderstanding what you're saying here though?
>
> > If we pick something more
> > interesting, there was concern around it being caught up on concerns
> > related to the specific driver. The hope was that this driver would
> > simultaneously be real enough to count as a driver in the eyes of
> > other vendors watching for someone to take the lead, and simple enough
> > to be low risk enough to be possible.
> >
>
> Are you talking about upstream drivers, or downstream vendor drivers?

Both - some vendors have upstream drivers and some are downstream
only, and we'd ideally like both of them to consider using Rust when
producing drivers that end up in Android.

>
> > We do have more interesting drivers in process, e.g. the tyr graphics
> > driver, but those are longer term efforts.
> >
> > >
> > > Also, how would this manifest itself in the upstream kernel? Do you have
> > > a test bed where you build GKI + vendor "kernel" out of the upstream
> > > kernel tree today?
> >
> > In Android, kernel modules are split into two categories - "GKI
> > modules", which get shipped with the primary kernel to every system,
> > and vendor modules, which are shipped and loaded by particular
> > devices. Neither current builtin modules [1] nor current GKI modules
> > [2] contain the qcom socinfo, so it is a vendor module. This
> > categorization is not an "in-tree" / "out of tree" categorization, but
> > a "every Android device" / "some Android devices" one.
> >
>
> Forgive me, but I'm reading this as "Android downstream" + "Qualcomm
> downstream", my question was how this relates to the upstream kernel.

I'm starting to think that this part of the discussion may be
irrelevant to your final position, but I'll still try to clarify.
Android devices ship three things on a device:
1. The base image (GKI) - a kernel image built from the Android tree,
downstream of a particular LTS. This is ideally the same on every
phone, but there are some exceptions.
2. Modules that are shipped to every device (GKI modules) - these are
produced in the same build as the kernel image.
3. Modules that are needed for a particular device (Vendor modules) -
these are built against one of the GKI builds, though possibly not the
newest. They may be in or out of tree. We endeavor to make it so that
vendor modules built it against older GKI revisions from the same
kernel LTS can be used with newer ones without recompilation. For
example, a module built against android16-6.12 when it was first
released can be used alongside android16-6.12 that was released a year
later, without recompilation.

We believe that #3 will work correctly with Rust based on our
monitoring and backtesting, but this would serve as the first "real
world" instance of #3 + Rust.

The only way this relates to the upstream kernel is that we try to
have a minimum of changes in our downstream kernel trees - we try to
work upstream first where possible. While I want to be transparent
about my motivation for looking at this module (which is expressed
above), a better rationale for Linux may be the CVE we mentioned
below, and we can ignore all this.

>
> > [1] https://ci.android.com/builds/submitted/14027866/kernel_aarch64/latest/modules.builtin
> > [2] https://android.googlesource.com/kernel/common/+/refs/heads/android16-6.12/modules.bzl
> > >
> > > > Trying to apply Rust to more significant existing modules (where it
> > > > would provide a greater benefit) raises concern about whether they
> > > > will operate identically. Socinfo in particular can be more thoroughly
> > > > tested for equivalency, as aside from the runtime DSP information
> > > > modification that was mentioned in this discussion, the output of the
> > > > socinfo driver is static and can just be checked. This means that a
> > > > socinfo driver swap will essentially have the language change be the
> > > > *only* significant thing about the change, keeping the risk to a
> > > > minimum.
> > > >
> > > > Part of the reason for the RFC prefix on this patch is to solicit your
> > > > feedback on whether you are open to this. If you have somewhere else
> > > > you'd prefer we try this, I'd be open to that as well.
> > > >
> > >
> > > Rust is a new dependency and it's a foreign language to the vast
> > > majority of the contributors in the upstream Qualcomm community.
> > >
> > > So, IMHO sufficient value needs to come with such rewrite, and this
> > > value needs to be properly documented in the git history. Rewriting
> > > things for the sake of rewriting things, or the facilitate a downstream
> > > experiment, is hard for me to force onto our community.
> >
> > Since you brought it up, I took a brief look and found CVE-2024-58007
> > [3] , which would have been prevented through use of Rust, as the
> > access would have been bounds-checked. I understand that such
> > vulnerabilities in socinfo.c are likely to be uncommon, but even for a
> > driver this simple memory safety issues do happen, and have even
> > happened recently.
> >
>
> Yes, this is exactly the kind of argumentation I would like to see.
>
> This is something we can weigh against the added cost of the Rust
> implementation.
>
> Regards,
> Bjorn
>
> > [3]: https://nvd.nist.gov/vuln/detail/CVE-2024-58007
> > >
> > > Regards,
> > > Bjorn
> > >
> > > > > > >
> > > > > > > > ---
> > > > > > > > base-commit: 4b31c90f86768367c84cc6e99e1deee0ec55197b
> > > > > > > > change-id: 20251029-qcom-socinfo-d8387c7fdb1c
> > > > > > > > prerequisite-change-id: 20251029-soc-bindings-9b0731bcdbed:v1
> > > > > > > > prerequisite-patch-id: e4da423ddabec93bd9a64004691266f9b740e0c5
> > > > > > > > prerequisite-patch-id: 5097ff622f8cb1d38354674cf70c1c946ac87f6c
> > > > > > > > prerequisite-change-id: 20251029-add-entropy-f57e12ebe110:v1
> > > > > > > > prerequisite-patch-id: 657de204912d2c5efff9898d3f4c51e52684d8e6
> > > > > > > > prerequisite-change-id: 20251212-transmute-8ab6076700a8:v1
> > > > > > > > prerequisite-patch-id: 4f5f7cb002d02d232083ab5c3ce6b3cb90650bd6
> > > > > > > > prerequisite-patch-id: 01a1f3d727e549c173dd142180741f5b6a3c866e
> > > > > > > > prerequisite-patch-id: ff801a7ae439876b1554b4d1132d94c825bbe74f
> > > > > > >
> > > > > > > So, it doesn't work on a clean v6.19-rc1 tree?
> > > > > >
> > > > > > Correct. Those other patchsets are patchsets I'm landing which add
> > > > > > bindings used in this one.
> > > > > >
> > > > >
> > > > > Okay, please make that explicit when submitting patches, so that the
> > > > > maintainers know that your patch(es) can't be merged.
> > > >
> > > > I thought that was what I had done by using the combination of "RFC"
> > > > and putting the prerequisite change IDs from b4, but I will include a
> > > > statement to that effect in my cover letter in the future.
> > > >
> > > > >
> > > > > Regards,
> > > > > Bjorn
> > > > >
> > > > > > >
> > > > > > > Regards,
> > > > > > > Bjorn
> > > > > > >
> > > > > > > >
> > > > > > > > Best regards,
> > > > > > > > --
> > > > > > > > Matthew Maurer <mmaurer@google.com>
> > > > > > > >