From patchwork Tue Jul 6 23:48:50 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tim Gardner X-Patchwork-Id: 58074 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from chlorine.canonical.com (chlorine.canonical.com [91.189.94.204]) by ozlabs.org (Postfix) with ESMTP id 73DD2B6EEC for ; Wed, 7 Jul 2010 09:49:13 +1000 (EST) Received: from localhost ([127.0.0.1] helo=chlorine.canonical.com) by chlorine.canonical.com with esmtp (Exim 4.69) (envelope-from ) id 1OWHss-00047v-7b; Wed, 07 Jul 2010 00:49:02 +0100 Received: from mail.tpi.com ([70.99.223.143]) by chlorine.canonical.com with esmtp (Exim 4.69) (envelope-from ) id 1OWHsp-00046G-EL for kernel-team@lists.ubuntu.com; Wed, 07 Jul 2010 00:49:00 +0100 Received: from [10.0.2.5] (unknown [10.0.2.5]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mail.tpi.com (Postfix) with ESMTP id 392E2252839; Tue, 6 Jul 2010 16:48:00 -0700 (PDT) Message-ID: <4C33C0E2.7070801@canonical.com> Date: Tue, 06 Jul 2010 17:48:50 -0600 From: Tim Gardner User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.10) Gecko/20100528 Thunderbird/3.0.5 MIME-Version: 1.0 To: "Vadillo, Miguel" Subject: Re: [PATCH 3/5] SYSLINK:IPU-PM: solve circular dependencies ipu_pm References: <1277960546-32336-1-git-send-email-bryan.wu@canonical.com> <1277960546-32336-4-git-send-email-bryan.wu@canonical.com> <4C2CAA77.1060307@canonical.com> <13B9B4C6EF24D648824FF11BE896716203BBBF435F@dlee02.ent.ti.com> <4C2DE6D5.4090204@canonical.com> <13B9B4C6EF24D648824FF11BE896716203BBD03858@dlee02.ent.ti.com> In-Reply-To: <13B9B4C6EF24D648824FF11BE896716203BBD03858@dlee02.ent.ti.com> Cc: "Shah, Bhavin" , "Gutierrez, Juan" , "Hunt, Paul" , "kernel-team@lists.ubuntu.com" , "Liu, Stanley" , "ogra@canonical.com" X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.9 Precedence: list Reply-To: tim.gardner@canonical.com List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kernel-team-bounces@lists.ubuntu.com Errors-To: kernel-team-bounces@lists.ubuntu.com Miguel, I think there are still memory leaks in your patch. How about the attached version. rtg On 07/06/2010 01:45 PM, Vadillo, Miguel wrote: > Tim, Bryan, > > Please find attached the patch that solves the error with the return values in the ipu_pm_setup function. > Regarding the semaphore and the static declaration both changes will be included in the next release. > > Please let us know what you think. As always your feedback is welcome. > > Regards... > Miguel Vadillo > Cel. 214-587-2910 > > >> -----Original Message----- >> From: Vadillo, Miguel >> Sent: Friday, July 02, 2010 10:39 AM >> To: 'tim.gardner@canonical.com' >> Cc: Bryan Wu; kernel-team@lists.ubuntu.com; ogra@canonical.com; Jan, >> Sebastien; Hunt, Paul; Shah, Bhavin; Liu, Stanley; Gutierrez, Juan >> Subject: RE: [PATCH 3/5] SYSLINK:IPU-PM: solve circular dependencies >> ipu_pm >> >>> -----Original Message----- >>> From: Tim Gardner [mailto:tim.gardner@canonical.com] >>> Sent: Friday, July 02, 2010 8:17 AM >>> To: Vadillo, Miguel >>> Cc: Bryan Wu; kernel-team@lists.ubuntu.com; ogra@canonical.com; Jan, >>> Sebastien; Hunt, Paul; Shah, Bhavin; Liu, Stanley; Gutierrez, Juan >>> Subject: Re: [PATCH 3/5] SYSLINK:IPU-PM: solve circular dependencies >>> ipu_pm >>> >>> On 07/01/2010 09:53 AM, Vadillo, Miguel wrote: >>> >>>>>> /* >>>>>> + Function for setup ipu_pm module >>>>>> + * >>>>>> + */ >>>>>> +int ipu_pm_setup(void *notify_driver_handle) >>>>>> +{ >>>>>> + u32 i = 0; >>>>>> + ipu_pm_notifydrv_handle = notify_driver_handle; >>>>>> + /* Get the shared RCB */ >>>>>> + rcb_table = (struct sms *) ioremap(PM_SHM_BASE_ADDR, >>>>>> + sizeof(struct sms)); >>>>>> + >>>>>> + pm_event = kzalloc(sizeof(struct pm_event) * NUMBER_PM_EVENTS, >>>>>> + GFP_KERNEL); >>>>>> + >>>>>> + /* Each event has it own sem */ >>>>>> + for (i = 0; i< NUMBER_PM_EVENTS; i++) { >>>>>> + pm_event[i].sem_handle = kzalloc(sizeof(struct >> semaphore), >>>>>> + GFP_KERNEL); >>>>>> + sema_init(pm_event[i].sem_handle, 0); >>>>>> + pm_event[i].event_type = i; >>>>>> + } >>>>>> + return 0; >>>>>> +} >>>>>> +EXPORT_SYMBOL(ipu_pm_setup); >>>>>> + >>>>> >>>>> Why bother returning a status code if its just a constant? How about >>>>> checking for failures when calling ioremap() and kzalloc() ? >>>> >>>> This part is now fixed and will be available for the next release too. >>> >We are checking the kzalloc. Also the ioremap is not done anymore >>> since we are requesting a heap from the share memory. >>> >>> So, you're saying this is all different in 2.6.35 (which I assume is the >>> 'next release') ? That could be awhile. >>> >>> As far as I can tell from looking at the repositories at >>> git://dev.omapzoom.org/pub/scm/integration you wrote these patches just >>> for the Ubuntu branch. Since we may have to live with this 2.6.34 based >>> kernel for a few weeks more, please take the time to write correct code >>> and resend this patch. At least put some BUG_ON()s in it so we catch >>> allocation failures. >>> >> >> >> Ok in that case I will rework the patches following your suggestions and >> resend them ASAP, probably next week. >> >>> rtg >>> -- >>> Tim Gardner tim.gardner@canonical.com >> >> Regards... >> Miguel Vadillo From 3f7786199d1ee79871a3fcd7540bce09feaf1454 Mon Sep 17 00:00:00 2001 From: Tim Gardner Date: Tue, 6 Jul 2010 14:56:00 -0600 Subject: [PATCH] SYSLINK:IPU-PM: solve circular dependencies ipu_pm Changes to fix circular dependencies when compiling ipu_pm in modules mode. Signed-off-by: Miguel Vadillo Signed-off-by: Tim Gardner --- arch/arm/plat-omap/clock.c | 1 + drivers/dsp/syslink/ipu_pm/ipu_pm.c | 90 ++++++++++++++++++++++++- drivers/dsp/syslink/ipu_pm/ipu_pm.h | 10 ++- drivers/dsp/syslink/multicore_ipc/platform.c | 35 ++-------- 4 files changed, 101 insertions(+), 35 deletions(-) diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c index 701e4ea..104eaec 100644 --- a/arch/arm/plat-omap/clock.c +++ b/arch/arm/plat-omap/clock.c @@ -336,6 +336,7 @@ struct clk *omap_clk_get_by_name(const char *name) return ret; } +EXPORT_SYMBOL(omap_clk_get_by_name); /* * Low level helpers diff --git a/drivers/dsp/syslink/ipu_pm/ipu_pm.c b/drivers/dsp/syslink/ipu_pm/ipu_pm.c index 3806f5a..81d078e 100644 --- a/drivers/dsp/syslink/ipu_pm/ipu_pm.c +++ b/drivers/dsp/syslink/ipu_pm/ipu_pm.c @@ -85,6 +85,9 @@ int ipu_timer_list[NUM_IPU_TIMERS] = { struct omap_dm_timer *p_gpt; struct clk *p_i2c_clk; +struct sms *rcb_table; +void *ipu_pm_notifydrv_handle; +struct pm_event *pm_event; /** ============================================================================ * Forward declarations of internal functions @@ -221,7 +224,7 @@ void ipu_pm_callback(short int procId, /* send the ACK to DUCATI*/ return_val = notify_sendevent( - platform_notifydrv_handle, + ipu_pm_notifydrv_handle, SYS_M3,/*DUCATI_PROC*/ PM_RESOURCE,/*PWR_MGMT_EVENT*/ payload, @@ -280,7 +283,7 @@ int ipu_pm_notifications(enum pm_event_type event_type) pm_msg.fields.parm = PM_SUCCESS; /* send the ACK to DUCATI*/ return_val = notify_sendevent( - platform_notifydrv_handle, + ipu_pm_notifydrv_handle, SYS_M3,/*DUCATI_PROC*/ PM_NOTIFICATION,/*PWR_MGMT_EVENT*/ (unsigned int)pm_msg.whole, @@ -301,7 +304,7 @@ int ipu_pm_notifications(enum pm_event_type event_type) pm_msg.fields.parm = PM_SUCCESS; /* send the ACK to DUCATI*/ return_val = notify_sendevent( - platform_notifydrv_handle, + ipu_pm_notifydrv_handle, SYS_M3,/*DUCATI_PROC*/ PM_NOTIFICATION,/*PWR_MGMT_EVENT*/ (unsigned int)pm_msg.whole, @@ -322,7 +325,7 @@ int ipu_pm_notifications(enum pm_event_type event_type) pm_msg.fields.parm = PM_SUCCESS; /* send the ACK to DUCATI*/ return_val = notify_sendevent( - platform_notifydrv_handle, + ipu_pm_notifydrv_handle, SYS_M3,/*DUCATI_PROC*/ PM_NOTIFICATION,/*PWR_MGMT_EVENT*/ (unsigned int)pm_msg.whole, @@ -343,6 +346,83 @@ int ipu_pm_notifications(enum pm_event_type event_type) } EXPORT_SYMBOL(ipu_pm_notifications); +static void ipu_pm_free_events(struct pm_event *pm_event) +{ + if (pm_event) { + u32 i; + for (i = 0; i < NUMBER_PM_EVENTS; i++) { + kfree(pm_event[i].sem_handle); + pm_event[i].sem_handle = NULL; + pm_event[i].event_type = 0; + } + } +} + +static int ipu_pm_setup_events(struct pm_event *pm_event) +{ + u32 i; + for (i = 0; i < NUMBER_PM_EVENTS; i++) { + pm_event[i].sem_handle = kzalloc(sizeof(struct semaphore), + GFP_KERNEL); + if (WARN_ON(!pm_event[i].sem_handle)) + goto error; + sema_init(pm_event[i].sem_handle, 0); + pm_event[i].event_type = i; + } + + return 0; +error: + ipu_pm_free_events(pm_event); + return -ENOMEM; +} + +/* + Function for setup ipu_pm module + * + */ +int ipu_pm_setup(void *notify_driver_handle) +{ + ipu_pm_notifydrv_handle = notify_driver_handle; + + /* Get the shared RCB */ + rcb_table = (struct sms *) ioremap(PM_SHM_BASE_ADDR, + sizeof(struct sms)); + if (WARN_ON(!rcb_table)) + goto error; + + pm_event = kzalloc(sizeof(struct pm_event) * NUMBER_PM_EVENTS, + GFP_KERNEL); + if (WARN_ON(!pm_event)) + goto error; + + if (WARN_ON(ipu_pm_setup_events(pm_event) < 0)) + goto error; + + return 0; + +error: + ipu_pm_finish(); + return -ENOMEM; + +} +EXPORT_SYMBOL(ipu_pm_setup); + +/* + Function for finish ipu_pm module + * + */ +void ipu_pm_finish(void) +{ + ipu_pm_free_events(pm_event); + kfree(pm_event); + pm_event = NULL; + if (rcb_table) + iounmap(rcb_table); + rcb_table = NULL; + ipu_pm_notifydrv_handle = NULL; +} +EXPORT_SYMBOL(ipu_pm_finish); + /* Function for get sdma channels from PRCM * @@ -518,3 +598,5 @@ inline void ipu_pm_rel_i2c_bus(unsigned rcb_num) pm_i2c_bus_counter--; } +MODULE_LICENSE("GPL"); + diff --git a/drivers/dsp/syslink/ipu_pm/ipu_pm.h b/drivers/dsp/syslink/ipu_pm/ipu_pm.h index e9106fb..713e269 100644 --- a/drivers/dsp/syslink/ipu_pm/ipu_pm.h +++ b/drivers/dsp/syslink/ipu_pm/ipu_pm.h @@ -185,10 +185,6 @@ struct rcb_block { }; -extern struct sms *rcb_table; -extern void *platform_notifydrv_handle; -extern struct pm_event *pm_event; - struct sms { unsigned rat; struct rcb_block rcb[RCB_MAX]; @@ -214,5 +210,11 @@ void ipu_pm_notify_callback(short int procId, /* Function for send PM Notifications */ int ipu_pm_notifications(enum pm_event_type event_type); +/* Function for setup ipu_pm module */ +int ipu_pm_setup(void *notify_driver_handle); + +/* Function for finish ipu_pm module */ +void ipu_pm_finish(void); + #endif diff --git a/drivers/dsp/syslink/multicore_ipc/platform.c b/drivers/dsp/syslink/multicore_ipc/platform.c index d1c62e7..7490f76 100644 --- a/drivers/dsp/syslink/multicore_ipc/platform.c +++ b/drivers/dsp/syslink/multicore_ipc/platform.c @@ -233,9 +233,6 @@ */ void *platform_notifydrv_handle; -struct pm_event *pm_event; -struct sms *rcb_table; - /* Handles for SysM3 */ void *platform_nsrn_gate_handle_sysm3; void *platform_nsrn_handle_sysm3; @@ -982,22 +979,9 @@ void platform_start_callback(void *arg) goto pm_register_fail; } - /* Get the shared RCB */ - rcb_table = (struct sms *) ioremap(PM_SHM_BASE_ADDR, - sizeof(struct sms)); - - pm_event = - kzalloc(sizeof(struct pm_event) - * NUMBER_PM_EVENTS, GFP_KERNEL); - - /* Each event has it own sem */ - for (i = 0; i < NUMBER_PM_EVENTS; i++) { - pm_event[i].sem_handle = - kzalloc(sizeof(struct semaphore), - GFP_KERNEL); - sema_init(pm_event[i].sem_handle, 0); - pm_event[i].event_type = i; - } + if (ipu_pm_setup(platform_notifydrv_handle) < 0) + goto pm_setup_fail; + } /* END PM */ @@ -1315,6 +1299,9 @@ proc_invalid_id: goto exit; pm_register_fail: printk(KERN_ERR "pm register events failed"); + goto exit; +pm_setup_fail: + printk(KERN_ERR "ipu_pm_setup() failed"); exit: return; } @@ -1334,7 +1321,6 @@ void platform_stop_callback(void *arg) u16 proc_id = (u32) arg; int index = 0; u32 nread = 0; - u32 i = 0; if (proc_id == multiproc_get_id("SysM3")) index = SMHEAP_SRINDEX_SYSM3; @@ -1460,13 +1446,8 @@ void platform_stop_callback(void *arg) (void *)NULL); if (status < 0) printk(KERN_INFO "ERROR UNREGISTERING PM EVENT\n"); - for (i = 0; i < NUMBER_PM_EVENTS; i++) { - kfree(pm_event[i].sem_handle); - pm_event[i].event_type = 0; - } - kfree(pm_event); - /* Release the shared RCB */ - iounmap(rcb_table); + + ipu_pm_finish(); } /* END PM */ -- 1.7.0.4