Skip to contentSkip to conversation

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

PATCH In review Daniel Walker 3 participants 5 messages Apr 19, 2010 – Apr 20, 2010

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 2/5 arm: msm: smd: use either package v3 or v4 not both

From: Daniel Walker <[email protected]>

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.

---
 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 +4 -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
@@ -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

56 lines not on this page · raw message

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

79 lines not on this page · raw message

Signature · 1 line
1.6.2.3
#1 lore raw

Daniel,

Thanks for the cleanup. As mentioned in my previous comment, please do not move the alloc_channel functions out of smd.c. Making them conditional at build is fine, but they do not belong in the header file.

--Dima

Quoted 200 lines · Daniel Walker

From: Daniel Walker <[email protected]>

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.

---
 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.

arch/arm/mach-msm/Kconfig +3 -0
+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..6237a6c 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_for_package_version(ch)) {
                kfree(ch);
                return -1;
        }
arch/arm/mach-msm/smd_private.h +56 -3
index 33a33f1..eb9c824 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;
@@ -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 *);
 
-#endif
+#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_for_package_version(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_for_package_version(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;
+}
+#endif /* CONFIG_MSM_SMD_PKG3 */
+
+#endif /* CONFIG_ARCH_MSM7X00A */
--

1.6.2.3

#2 lore raw

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

From: Daniel Walker <[email protected]>

I moved the code back, even tho I certain don't agree with it. I'll resubmit there into your gerrit system.

Daniel

--

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.

Change-Id I677c17a2d92005df7fb1b55bb196762d63ea4f3d
---
 arch/arm/mach-msm/Kconfig       |    4 +++
 arch/arm/mach-msm/smd.c         |   48 +++++++++++++++++++++++---------------
 arch/arm/mach-msm/smd_private.h |    6 +++-
 3 files changed, 37 insertions(+), 21 deletions(-)
arch/arm/mach-msm/Kconfig +4 -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
@@ -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 +29 -19

66 lines not on this page · raw message

arch/arm/mach-msm/smd_private.h +4 -2
index 33a33f1..fd299e8 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;
@@ -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,4 @@ uint32_t raw_smsm_get_state(enum smsm_state_item item);
 
 extern void msm_init_last_radio_log(struct module *);
 
-#endif
+#endif /* CONFIG_ARCH_MSM7X00A */
Signature · 1 line
1.6.2.3
#3 lore raw
In reply to Daniel Walker
Quoted 6 lines · Daniel Walker

From: Daniel Walker <[email protected]>

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,

Quoted 8 lines

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.

Well... it adds about 100 bytes to kernel text and produces unmeasurable slowdown.

Quoted 11 lines
arch/arm/mach-msm/Kconfig +1 -0
@@ -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 config MSM_SERIAL_DEBUGGER_CONSOLE · line 331

Quoted 6 lines
arch/arm/mach-msm/Kconfig +3 -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
+

...but I guess that as long as kconfig system gets it right automatically... its probably ok.

....well, will that prevent us from producing 'generic' kernel in future?
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#4 lore raw
Quoted 10 lines · Pavel Machek
Quoted 9 lines · Daniel Walker

From: Daniel Walker <[email protected]>

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'

Quoted 8 lines
Quoted 5 lines

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.

Well... it adds about 100 bytes to kernel text and produces unmeasurable slowdown.

I would think it would actually be very easily measurable, as you can measure the amount of time spent doing a channel allocation and failing. In any case, it is a good change, we don't need to try and fail. That code hasn't been necessary for a long time.

Quoted 22 lines
Quoted 15 lines
arch/arm/mach-msm/Kconfig +0 -0
@@ -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.

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

Quoted 3 lines
arch/arm/mach-msm/Kconfig +3 -0
+config MSM_SMD_PKG3
+     bool
+

...but I guess that as long as kconfig system gets it right automatically... its probably ok.

Quoted 2 lines

....well, will that prevent us from producing 'generic' kernel in future?

You really can't generate a generic kernel for msm7k and qsd8k anyway. Too many architectural differences.

--Dima

Quoted 5 lines

                                                                       Pavel --
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html