Re: [PATCH] soc: qcom: smem: Document shared memory item IDs and corresponding structs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


On 9/15/2023 7:37 PM, Bjorn Andersson wrote:
On Fri, Sep 15, 2023 at 11:59:07AM -0600, Jeffrey Hugo wrote:
Shared memory items are assigned a globally unique ID and almost always
have a defined structure which is stored in the shared memory.  Document
assigned IDs and corresponding structures.

Signed-off-by: Jeffrey Hugo <quic_jhugo@xxxxxxxxxxx>

Konrad, before I get too far into this, I was hoping for some early
feedback since this documentation is a request that you made.

Please let me know if this is aligned with what you were wanting.

  include/linux/soc/qcom/smem.h | 176 ++++++++++++++++++++++++++++++++++
  1 file changed, 176 insertions(+)

diff --git a/include/linux/soc/qcom/smem.h b/include/linux/soc/qcom/smem.h
index 223db6a9c733..2f8d1f3126a4 100644
--- a/include/linux/soc/qcom/smem.h
+++ b/include/linux/soc/qcom/smem.h
@@ -4,6 +4,182 @@
#define QCOM_SMEM_HOST_ANY -1 +/* fixed items - these have a static position in shared memory */
+#define SMEM_PROC_COMM				0

Other parts of this interface are prefixed with qcom_.

+#define SMEM_HEAP_INFO				1
+#define SMEM_VERSION_INFO			3
+/* Legacy communication protocol between "Apps" and "Modem" processors */
+struct smem_proc_comm {

This is already defined in smem.c, with the same name, but slightly
different definition.

Unexpected, although I didn't look very hard.

My plan is to do this in phases. First, gather all the information I can and put it in one place. Then look at what we currently have in other committed drivers, and try to link those things up to this. In many cases, I'm thinking we define the item here and have a reference comment to the implementation that occurs elsewhere. All of that gets developed and hashed out before we commit a patch.

I think this addresses some of your further comments about overlap.

I always envisioned that we would treat this as an smem-internal
implementation detail and expose a function to invoke a proc command, if
someone cared...

Does including it here in the client api definition make sense? Is the
first entry in the smem_heap_entry list pointing to this data, even
though it's part of the header?

I think having a function to send a proc command if someone cared makes sense. I think keeping the struct in smem-internal and referencing it here for documentation purposes would be beneficial. I don't think this needs to be in the client API for code to use.

Yes, smem_heap_entry[1] does point to this, even though its in the header. Thats a quirk of the fixed memory items.

+        __le32 command;
+        __le32 status;
+        __le32 data1;
+        __le32 data2;
+/* Metadata structure for shared memory heap allocations */
+struct smem_heap_info {

This, and the next entry shouldn't be accessed outside the heap
implementation itself...

+        __le32 initialized;
+        __le32 free_offset;
+        __le32 heap_remaining;
+        __le32 reserved;
+/* SMEM_ALLOCATION_TABLE is an array of these structures.  512 elements in the array. */
+struct smem_heap_entry {
+        __le32 allocated;
+        __le32 offset;
+        __le32 size;
+        __le32 reserved; /* bits 1:0 reserved, bits 31:2 aux smem base addr */
+struct smem_version_info {
+	__le32 version[32];
+struct smem_spinlock_array {
+	volatile __le32 lock[8];
+#define FLASH_PART_MAGIC1       0x55EE73AA
+#define FLASH_PART_MAGIC2       0xE35EBDDB
+#define FLASH_PTABLE_V3         3
+#define FLASH_PTABLE_V4         4
+struct flash_partition_entry {
+        char name[FLASH_PTABLE_ENTRY_NAME_SIZE];
+        __le32 offset;     /* Offset in blocks from beginning of device */
+        __le32 length;     /* Length of the partition in blocks */
+        u8 attr;           /* Flags for this partition */
+struct flash_partition_table {
+        __le32 magic1;
+        __le32 magic2;
+        __le32 version;
+        __le32 numparts;
+        struct flash_partition_entry part_entry[FLASH_PTABLE_MAX_PARTS_V4];

This information already exist in qcomsmempart.c, but with slightly
different names.

+/* SMEM_CHANNEL_ALLOC_TBL is an array of these.  Used for SMD. */
+struct smd_alloc_elm {

This is called qcom_smd_alloc_entry, in qcom_smd.c...


+        char name[20];
+        __le32 cid;
+        __le32 type;
+        __le32 ref_count;
  int qcom_smem_alloc(unsigned host, unsigned item, size_t size);
  void *qcom_smem_get(unsigned host, unsigned item, size_t *size);

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux