* [PATCH] arm: msm: smd: use either package v3 or v4 not both
@ 2010-04-09 9:37 Daniel Walker
2010-04-12 8:23 ` Dima Zavin
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Walker @ 2010-04-09 9:37 UTC (permalink / raw)
To: linux-arm-msm; +Cc: dima, Daniel Walker
From: Daniel Walker <c_dwalke@quicinc.com>
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 <c_dwalke@quicinc.com>
---
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(-)
diff --git a/arch/arm/mach-msm/Kconfig b/arch/arm/mach-msm/Kconfig
index b548e42..381e817 100644
--- a/arch/arm/mach-msm/Kconfig
+++ b/arch/arm/mach-msm/Kconfig
@@ -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)"
diff --git a/arch/arm/mach-msm/smd.c b/arch/arm/mach-msm/smd.c
index 42b59c9..1b4ecce 100644
--- a/arch/arm/mach-msm/smd.c
+++ b/arch/arm/mach-msm/smd.c
@@ -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;
}
diff --git a/arch/arm/mach-msm/smd_private.h b/arch/arm/mach-msm/smd_private.h
index 33a33f1..a403424 100644
--- a/arch/arm/mach-msm/smd_private.h
+++ b/arch/arm/mach-msm/smd_private.h
@@ -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;
+ ch->send_data = buffer;
+ ch->recv_data = buffer + buffer_sz;
+ ch->fifo_size = buffer_sz;
+ return 0;
+}
+#endif /* CONFIG_ARCH_MSM7X00A */
+
#endif
--
1.6.2.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] arm: msm: smd: use either package v3 or v4 not both
2010-04-09 9:37 [PATCH] arm: msm: smd: use either package v3 or v4 not both Daniel Walker
@ 2010-04-12 8:23 ` Dima Zavin
2010-04-12 14:43 ` Daniel Walker
0 siblings, 1 reply; 4+ messages in thread
From: Dima Zavin @ 2010-04-12 8:23 UTC (permalink / raw)
To: Daniel Walker; +Cc: linux-arm-msm, Daniel Walker
inline.
On Fri, Apr 9, 2010 at 2:37 AM, Daniel Walker <dwalker@codeaurora.org> wrote:
> From: Daniel Walker <c_dwalke@quicinc.com>
>
>
> 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 <c_dwalke@quicinc.com>
> ---
> 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(-)
>
> diff --git a/arch/arm/mach-msm/Kconfig b/arch/arm/mach-msm/Kconfig
> index b548e42..381e817 100644
> --- a/arch/arm/mach-msm/Kconfig
> +++ b/arch/arm/mach-msm/Kconfig
> @@ -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
What is PKG3/PKG4? How does it relate to smd structure versions?
> + bool
> +
> config MSM_SMD
> default y
> bool "MSM Shared Memory Driver (SMD)"
> diff --git a/arch/arm/mach-msm/smd.c b/arch/arm/mach-msm/smd.c
> index 42b59c9..1b4ecce 100644
> --- a/arch/arm/mach-msm/smd.c
> +++ b/arch/arm/mach-msm/smd.c
> @@ -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;
> }
> diff --git a/arch/arm/mach-msm/smd_private.h b/arch/arm/mach-msm/smd_private.h
> index 33a33f1..a403424 100644
> --- a/arch/arm/mach-msm/smd_private.h
> +++ b/arch/arm/mach-msm/smd_private.h
> @@ -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;
> + 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.
--Dima
> +
> #endif
> --
> 1.6.2.3
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] arm: msm: smd: use either package v3 or v4 not both
2010-04-12 8:23 ` Dima Zavin
@ 2010-04-12 14:43 ` Daniel Walker
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Walker @ 2010-04-12 14:43 UTC (permalink / raw)
To: Dima Zavin; +Cc: linux-arm-msm
On Mon, 2010-04-12 at 01:23 -0700, Dima Zavin wrote:
> inline.
>
> On Fri, Apr 9, 2010 at 2:37 AM, Daniel Walker <dwalker@codeaurora.org> wrote:
> > From: Daniel Walker <c_dwalke@quicinc.com>
> >
> >
> > 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 <c_dwalke@quicinc.com>
> > ---
> > 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(-)
> >
> > diff --git a/arch/arm/mach-msm/Kconfig b/arch/arm/mach-msm/Kconfig
> > index b548e42..381e817 100644
> > --- a/arch/arm/mach-msm/Kconfig
> > +++ b/arch/arm/mach-msm/Kconfig
> > @@ -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
>
> What is PKG3/PKG4? How does it relate to smd structure versions?
It represents the different structures used. We called it "package" with
a version. It only loosely connected with the SoC type, but I don't
expect we will see any more package v3's.
> > + bool
> > +
> > config MSM_SMD
> > default y
> > bool "MSM Shared Memory Driver (SMD)"
> > diff --git a/arch/arm/mach-msm/smd.c b/arch/arm/mach-msm/smd.c
> > index 42b59c9..1b4ecce 100644
> > --- a/arch/arm/mach-msm/smd.c
> > +++ b/arch/arm/mach-msm/smd.c
> > @@ -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;
> > }
> > diff --git a/arch/arm/mach-msm/smd_private.h b/arch/arm/mach-msm/smd_private.h
> > index 33a33f1..a403424 100644
> > --- a/arch/arm/mach-msm/smd_private.h
> > +++ b/arch/arm/mach-msm/smd_private.h
> > @@ -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;
> > + 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
the comment needs to be corrected, but traditionally in Linux we put
those comments onto "endif" lines. It's easier to see which is which
like in this situation when you have two of them together.
>
> 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.
It's another traditional Linux clean up. You usually move ifdef'd
inlines into header files. In fact most static inlines are in header
files. From my perspective the clean up comes from having a clean line
of development in the C file, vs. having ifdefs in the c file so all the
cases have to examined as you down read the source.
Daniel
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] arm: msm: smd: use either package v3 or v4 not both
2010-04-19 18:29 [PATCH 2/5] " Dima Zavin
@ 2010-04-19 23:22 ` Daniel Walker
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Walker @ 2010-04-19 23:22 UTC (permalink / raw)
To: linux-arm-msm; +Cc: dima, Daniel Walker
From: Daniel Walker <c_dwalke@quicinc.com>
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
Signed-off-by: Daniel Walker <c_dwalke@quicinc.com>
---
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(-)
diff --git a/arch/arm/mach-msm/Kconfig b/arch/arm/mach-msm/Kconfig
index b548e42..381e817 100644
--- a/arch/arm/mach-msm/Kconfig
+++ b/arch/arm/mach-msm/Kconfig
@@ -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)"
diff --git a/arch/arm/mach-msm/smd.c b/arch/arm/mach-msm/smd.c
index 42b59c9..7c0a22c 100644
--- a/arch/arm/mach-msm/smd.c
+++ b/arch/arm/mach-msm/smd.c
@@ -600,7 +600,33 @@ static int smd_packet_read(smd_channel_t *ch, void *data, int len)
return r;
}
-static int smd_alloc_v2(struct smd_channel *ch)
+#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;
@@ -624,23 +650,7 @@ static int smd_alloc_v2(struct smd_channel *ch)
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;
-}
-
+#endif /* CONFIG_MSM_SMD_PKG3 */
static int smd_alloc_channel(const char *name, uint32_t cid, uint32_t type)
{
@@ -653,7 +663,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;
}
diff --git a/arch/arm/mach-msm/smd_private.h b/arch/arm/mach-msm/smd_private.h
index 33a33f1..fd299e8 100644
--- a/arch/arm/mach-msm/smd_private.h
+++ b/arch/arm/mach-msm/smd_private.h
@@ -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 */
--
1.6.2.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-04-19 23:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-04-09 9:37 [PATCH] arm: msm: smd: use either package v3 or v4 not both Daniel Walker
2010-04-12 8:23 ` Dima Zavin
2010-04-12 14:43 ` Daniel Walker
-- strict thread matches above, loose matches on Subject: below --
2010-04-19 18:29 [PATCH 2/5] " Dima Zavin
2010-04-19 23:22 ` [PATCH] " Daniel Walker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).