inline.
On Fri, Apr 9, 2010 at 2:37 AM, Daniel Walker <dwalker@xxxxxxxxxxxxxx> wrote:
> From: Daniel Walker <c_dwalke@xxxxxxxxxxx>
>
>
> 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@xxxxxxxxxxx>
> ---
> 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
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
