arm: msm: smd: use either package v3 or v4 not both

From: Daniel Walker <[email protected]>

Dima could you review this patch please?

--

This modifies SMD to use either the package v3 or package v4, but not both. The current code tries to allocate as v4 on all system which can produce a scary looking error message on boot up,

smem_find(16, 40): wrong size 16424 smd_alloc_channel() cid=02 size=08192 'SMD_RPCCALL'

With this error the code then falls back on the package v3 allocation method. This method is inefficient because it causes a slow down on some systems even when the allocation method can be determined at compile time. It also causes a kernel size increase that effects all system and is not needed.

This change corrects the allocation to use one method or the other and not both.

Signed-off-by Daniel Walker <[email protected]>

---
 arch/arm/mach-msm/Kconfig       |    4 +++ arch/arm/mach-msm/smd.c         |   44 +----------------------------- arch/arm/mach-msm/smd_private.h |   58 +++++++++++++++++++++++++++++++++++++-

 3 files changed, 61 insertions(+), 45 deletions(-)

arch/arm/mach-msm/Kconfig +0 -0

index b548e42..381e817 100644
@@ -7,6 +7,7 @@ choice

 config ARCH_MSM7X00A
       bool "MSM7x00A / MSM7x01A"
       select ARCH_MSM_ARM11
+       select MSM_SMD_PKG3
       select CPU_V6

 config ARCH_QSD8X50

arch/arm/mach-msm/Kconfig +0 -0

@@ -325,6 +326,9 @@ config MSM_SERIAL_DEBUGGER_CONSOLE

         Enables a console so that printk messages are displayed on the debugger serial port as the occur.

+config MSM_SMD_PKG3

+       bool
+
 config MSM_SMD
       default y
       bool "MSM Shared Memory Driver (SMD)"

arch/arm/mach-msm/smd.c +1 -43

index 42b59c9..1b4ecce 100644
@@ -600,48 +600,6 @@ static int smd_packet_read(smd_channel_t *ch, void *data, int len)
        return r;
  }
 
-static int smd_alloc_v2(struct smd_channel *ch)
-{
-       struct smd_shared_v2 *shared2;
-       void *buffer;
-       unsigned buffer_sz;
-
-       shared2 = smem_alloc(SMEM_SMD_BASE_ID + ch->n, sizeof(*shared2));
-       buffer = smem_item(SMEM_SMD_FIFO_BASE_ID + ch->n, &buffer_sz);
-
-       if (!buffer)
-               return -1;
-
-       /* buffer must be a power-of-two size */
-       if (buffer_sz & (buffer_sz - 1))
-               return -1;
-
-       buffer_sz /= 2;
-       ch->send = &shared2->ch0;
-       ch->recv = &shared2->ch1;
-       ch->send_data = buffer;
-       ch->recv_data = buffer + buffer_sz;
-       ch->fifo_size = buffer_sz;
-       return 0;
-}
-
-static int smd_alloc_v1(struct smd_channel *ch)
-{
-       struct smd_shared_v1 *shared1;
-       shared1 = smem_alloc(ID_SMD_CHANNELS + ch->n, sizeof(*shared1));
-       if (!shared1) {
-               pr_err("smd_alloc_channel() cid %d does not exist\n", ch->n);
-               return -1;
-       }
-       ch->send = &shared1->ch0;
-       ch->recv = &shared1->ch1;
-       ch->send_data = shared1->data0;
-       ch->recv_data = shared1->data1;
-       ch->fifo_size = SMD_BUF_SIZE;
-       return 0;
-}
-
-
  static int smd_alloc_channel(const char *name, uint32_t cid, uint32_t type)
  {
        struct smd_channel *ch;

@@ -653,7 +611,7 @@ static int smd_alloc_channel(const char *name, uint32_t cid, uint32_t type)
        }
        ch->n = cid;
 
-       if (smd_alloc_v2(ch) && smd_alloc_v1(ch)) {
+       if (_smd_alloc_channel(ch)) {
                kfree(ch);
                return -1;
        }

arch/arm/mach-msm/smd_private.h +45 -2

index 33a33f1..a403424 100644
@@ -61,7 +61,7 @@ struct smem_shared {
  #define SMSM_V1_SIZE           (sizeof(unsigned) * 8)
  #define SMSM_V2_SIZE           (sizeof(unsigned) * 4)
 
-#ifndef CONFIG_ARCH_MSM_SCORPION
+#ifdef CONFIG_MSM_SMD_PKG3
  struct smsm_interrupt_info {
        uint32_t interrupt_mask;
        uint32_t pending_interrupts;

@@ -123,7 +123,7 @@ struct msm_dem_slave_data {
  #define SMSM_WKUP_REASON_ALARM 0x00000010
  #define SMSM_WKUP_REASON_RESET 0x00000020
 
-#ifndef CONFIG_ARCH_MSM_SCORPION
+#ifdef CONFIG_ARCH_MSM7X00A
  enum smsm_state_item {
        SMSM_STATE_APPS = 1,
        SMSM_STATE_MODEM = 3,

@@ -265,6 +265,7 @@ struct smd_half_channel {
        unsigned head;
  } __attribute__(( aligned(4), packed ));
 
+/* Only used on SMD package v3 on msm7201a */
  struct smd_shared_v1 {
        struct smd_half_channel ch0;
        unsigned char data0[SMD_BUF_SIZE];

@@ -272,6 +273,7 @@ struct smd_shared_v1 {
        unsigned char data1[SMD_BUF_SIZE];
  };
 
+/* Used on SMD package v4 */
  struct smd_shared_v2 {
        struct smd_half_channel ch0;
        struct smd_half_channel ch1;

@@ -330,4 +332,56 @@ uint32_t raw_smsm_get_state(enum smsm_state_item item);
 
  extern void msm_init_last_radio_log(struct module *);
 
+#ifdef CONFIG_MSM_SMD_PKG3
+/*
+ * This allocator assumes an SMD Package v3 which only exists on
+ * MSM7x00 SoC's.
+ */
+static inline int _smd_alloc_channel(struct smd_channel *ch)
+{
+       struct smd_shared_v1 *shared1;
+
+       shared1 = smem_alloc(ID_SMD_CHANNELS + ch->n, sizeof(*shared1));
+       if (!shared1) {
+               pr_err("smd_alloc_channel() cid %d does not exist\n", ch->n);
+               return -1;
+       }
+       ch->send = &shared1->ch0;
+       ch->recv = &shared1->ch1;
+       ch->send_data = shared1->data0;
+       ch->recv_data = shared1->data1;
+       ch->fifo_size = SMD_BUF_SIZE;
+       return 0;
+}
+#else
+/*
+ * This allocator assumes an SMD Package v4, the most common
+ * and the default.
+ */
+static inline int _smd_alloc_channel(struct smd_channel *ch)
+{
+       struct smd_shared_v2 *shared2;
+       void *buffer;
+       unsigned buffer_sz;
+
+       shared2 = smem_alloc(SMEM_SMD_BASE_ID + ch->n, sizeof(*shared2));
+       buffer = smem_item(SMEM_SMD_FIFO_BASE_ID + ch->n, &buffer_sz);
+
+       if (!buffer)
+               return -1;
+
+       /* buffer must be a power-of-two size */
+       if (buffer_sz & (buffer_sz - 1))
+               return -1;

arch/arm/mach-msm/smd_private.h +1 -1

+
  #endif
--

1.6.2.3

inline.

Quoted 41 lines · Daniel Walker

From: Daniel Walker <[email protected]>

Dima could you review this patch please?

--

This modifies SMD to use either the package v3 or package v4, but not both. The current code tries to allocate as v4 on all system which can produce a scary looking error message on boot up,

smem_find(16, 40): wrong size 16424 smd_alloc_channel() cid=02 size=08192 'SMD_RPCCALL'

With this error the code then falls back on the package v3 allocation method. This method is inefficient because it causes a slow down on some systems even when the allocation method can be determined at compile time. It also causes a kernel size increase that effects all system and is not needed.

This change corrects the allocation to use one method or the other and not both.

Signed-off-by Daniel Walker <[email protected]>

---
 arch/arm/mach-msm/Kconfig       |    4 +++
 arch/arm/mach-msm/smd.c         |   44 +-----------------------------
 arch/arm/mach-msm/smd_private.h |   58 +++++++++++++++++++++++++++++++++++++-
 3 files changed, 61 insertions(+), 45 deletions(-)

arch/arm/mach-msm/Kconfig +1 -0

index b548e42..381e817 100644
@@ -7,6 +7,7 @@ choice
 config ARCH_MSM7X00A
       bool "MSM7x00A / MSM7x01A"
       select ARCH_MSM_ARM11
+       select MSM_SMD_PKG3
       select CPU_V6
 

arch/arm/mach-msm/Kconfig config MSM_SERIAL_DEBUGGER_CONSOLE · line 329

Quoted 6 lines

arch/arm/mach-msm/Kconfig +1 -0

 config ARCH_QSD8X50

@@ -325,6 +326,9 @@ config MSM_SERIAL_DEBUGGER_CONSOLE
         Enables a console so that printk messages are displayed on
         the debugger serial port as the occur.
 
+config MSM_SMD_PKG3

What is PKG3/PKG4? How does it relate to smd structure versions?

Quoted 154 lines

arch/arm/mach-msm/Kconfig +2 -0

+       bool
+
 config MSM_SMD
       default y
       bool "MSM Shared Memory Driver (SMD)"

arch/arm/mach-msm/smd.c +1 -43

index 42b59c9..1b4ecce 100644
@@ -600,48 +600,6 @@ static int smd_packet_read(smd_channel_t *ch, void *data, int len)
       return r;
 }
 
-static int smd_alloc_v2(struct smd_channel *ch)
-{
-       struct smd_shared_v2 *shared2;
-       void *buffer;
-       unsigned buffer_sz;
-
-       shared2 = smem_alloc(SMEM_SMD_BASE_ID + ch->n, sizeof(*shared2));
-       buffer = smem_item(SMEM_SMD_FIFO_BASE_ID + ch->n, &buffer_sz);
-
-       if (!buffer)
-               return -1;
-
-       /* buffer must be a power-of-two size */
-       if (buffer_sz & (buffer_sz - 1))
-               return -1;
-
-       buffer_sz /= 2;
-       ch->send = &shared2->ch0;
-       ch->recv = &shared2->ch1;
-       ch->send_data = buffer;
-       ch->recv_data = buffer + buffer_sz;
-       ch->fifo_size = buffer_sz;
-       return 0;
-}
-
-static int smd_alloc_v1(struct smd_channel *ch)
-{
-       struct smd_shared_v1 *shared1;
-       shared1 = smem_alloc(ID_SMD_CHANNELS + ch->n, sizeof(*shared1));
-       if (!shared1) {
-               pr_err("smd_alloc_channel() cid %d does not exist\n", ch->n);
-               return -1;
-       }
-       ch->send = &shared1->ch0;
-       ch->recv = &shared1->ch1;
-       ch->send_data = shared1->data0;
-       ch->recv_data = shared1->data1;
-       ch->fifo_size = SMD_BUF_SIZE;
-       return 0;
-}
-
-
 static int smd_alloc_channel(const char *name, uint32_t cid, uint32_t type)
 {
       struct smd_channel *ch;

@@ -653,7 +611,7 @@ static int smd_alloc_channel(const char *name, uint32_t cid, uint32_t type)
       }
       ch->n = cid;
 
-       if (smd_alloc_v2(ch) && smd_alloc_v1(ch)) {
+       if (_smd_alloc_channel(ch)) {
               kfree(ch);
               return -1;
       }

arch/arm/mach-msm/smd_private.h +49 -2

index 33a33f1..a403424 100644
@@ -61,7 +61,7 @@ struct smem_shared {
 #define SMSM_V1_SIZE           (sizeof(unsigned) * 8)
 #define SMSM_V2_SIZE           (sizeof(unsigned) * 4)
 
-#ifndef CONFIG_ARCH_MSM_SCORPION
+#ifdef CONFIG_MSM_SMD_PKG3
 struct smsm_interrupt_info {
       uint32_t interrupt_mask;
       uint32_t pending_interrupts;

@@ -123,7 +123,7 @@ struct msm_dem_slave_data {
 #define SMSM_WKUP_REASON_ALARM 0x00000010
 #define SMSM_WKUP_REASON_RESET 0x00000020
 
-#ifndef CONFIG_ARCH_MSM_SCORPION
+#ifdef CONFIG_ARCH_MSM7X00A
 enum smsm_state_item {
       SMSM_STATE_APPS = 1,
       SMSM_STATE_MODEM = 3,

@@ -265,6 +265,7 @@ struct smd_half_channel {
       unsigned head;
 } __attribute__(( aligned(4), packed ));
 
+/* Only used on SMD package v3 on msm7201a */
 struct smd_shared_v1 {
       struct smd_half_channel ch0;
       unsigned char data0[SMD_BUF_SIZE];

@@ -272,6 +273,7 @@ struct smd_shared_v1 {
       unsigned char data1[SMD_BUF_SIZE];
 };
 
+/* Used on SMD package v4 */
 struct smd_shared_v2 {
       struct smd_half_channel ch0;
       struct smd_half_channel ch1;

@@ -330,4 +332,56 @@ uint32_t raw_smsm_get_state(enum smsm_state_item item);
 
 extern void msm_init_last_radio_log(struct module *);
 
+#ifdef CONFIG_MSM_SMD_PKG3
+/*
+ * This allocator assumes an SMD Package v3 which only exists on
+ * MSM7x00 SoC's.
+ */
+static inline int _smd_alloc_channel(struct smd_channel *ch)
+{
+       struct smd_shared_v1 *shared1;
+
+       shared1 = smem_alloc(ID_SMD_CHANNELS + ch->n, sizeof(*shared1));
+       if (!shared1) {
+               pr_err("smd_alloc_channel() cid %d does not exist\n", ch->n);
+               return -1;
+       }
+       ch->send = &shared1->ch0;
+       ch->recv = &shared1->ch1;
+       ch->send_data = shared1->data0;
+       ch->recv_data = shared1->data1;
+       ch->fifo_size = SMD_BUF_SIZE;
+       return 0;
+}
+#else
+/*
+ * This allocator assumes an SMD Package v4, the most common
+ * and the default.
+ */
+static inline int _smd_alloc_channel(struct smd_channel *ch)
+{
+       struct smd_shared_v2 *shared2;
+       void *buffer;
+       unsigned buffer_sz;
+
+       shared2 = smem_alloc(SMEM_SMD_BASE_ID + ch->n, sizeof(*shared2));
+       buffer = smem_item(SMEM_SMD_FIFO_BASE_ID + ch->n, &buffer_sz);
+
+       if (!buffer)
+               return -1;
+
+       /* buffer must be a power-of-two size */
+       if (buffer_sz & (buffer_sz - 1))
+               return -1;
+
+       buffer_sz /= 2;
+       ch->send = &shared2->ch0;
+       ch->recv = &shared2->ch1;

arch/arm/mach-msm/smd_private.h uint32_t raw_smsm_get_state(enum smsm_state_item item); · line 385

Quoted 6 lines

arch/arm/mach-msm/smd_private.h +6 -0

+       ch->send_data = buffer;
+       ch->recv_data = buffer + buffer_sz;
+       ch->fifo_size = buffer_sz;
+       return 0;
+}
+#endif /* CONFIG_ARCH_MSM7X00A */

get rid of the comment

Also, I'm not sure I see the benefit of putting these functions the way they are as inlines into this header file. There's only a single call site to this new alloc_channel function, and moving the code out of the smd.c and putting them here doesn't make anything cleaner.