[PATCH 3/5] SYSLINK:IPU-PM: solve circular dependencies ipu_pm
Tim Gardner
tim.gardner at canonical.com
Thu Jul 1 14:47:19 UTC 2010
On 06/30/2010 11:02 PM, Bryan Wu wrote:
> From: Miguel Vadillo<vadillo at ti.com>
>
> Changes to fix circular dependencies when compiling ipu_pm in
> modules mode.
>
> Signed-off-by: Miguel Vadillo<vadillo at ti.com>
> ---
> arch/arm/plat-omap/clock.c | 1 +
> drivers/dsp/syslink/ipu_pm/ipu_pm.c | 60 ++++++++++++++++++++++++--
> drivers/dsp/syslink/ipu_pm/ipu_pm.h | 10 +++--
> drivers/dsp/syslink/multicore_ipc/platform.c | 31 ++------------
> 4 files changed, 67 insertions(+), 35 deletions(-)
> mode change 100644 => 100755 arch/arm/plat-omap/clock.c
> mode change 100644 => 100755 drivers/dsp/syslink/ipu_pm/ipu_pm.c
> mode change 100644 => 100755 drivers/dsp/syslink/ipu_pm/ipu_pm.h
> mode change 100644 => 100755 drivers/dsp/syslink/multicore_ipc/platform.c
>
> diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
> old mode 100644
> new mode 100755
> index 701e4ea..104eaec
> --- 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
> old mode 100644
> new mode 100755
> index 3806f5a..f27512f
> --- 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;
>
Is there any reason these aren't static? Their scope appears to be
purely local to this file.
> /** ============================================================================
> * 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,
> @@ -344,6 +347,53 @@ int ipu_pm_notifications(enum pm_event_type event_type)
> EXPORT_SYMBOL(ipu_pm_notifications);
>
> /*
> + 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() ?
> +/*
> + Function for finish ipu_pm module
> + *
> + */
> +int ipu_pm_finish()
> +{
> + u32 i = 0;
> + /* Release the shared RCB */
> + for (i = 0; i< NUMBER_PM_EVENTS; i++) {
> + kfree(pm_event[i].sem_handle);
> + pm_event[i].event_type = 0;
> + }
> + kfree(pm_event);
> + pm_event = NULL;
> + iounmap(rcb_table);
> + rcb_table = NULL;
> + ipu_pm_notifydrv_handle = NULL;
> + return 0;
> +}
> +EXPORT_SYMBOL(ipu_pm_finish);
> +
> +/*
> Function for get sdma channels from PRCM
> *
> */
> @@ -518,3 +568,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
> old mode 100644
> new mode 100755
> index e9106fb..4481c6d
> --- 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 */
> +int ipu_pm_finish(void);
> +
> #endif
>
> diff --git a/drivers/dsp/syslink/multicore_ipc/platform.c b/drivers/dsp/syslink/multicore_ipc/platform.c
> old mode 100644
> new mode 100755
> index d1c62e7..1cdd686
> --- 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,8 @@ 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;
> - }
> + ipu_pm_setup(platform_notifydrv_handle);
> +
> }
> /* END PM */
>
> @@ -1334,7 +1317,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 +1442,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 */
>
I realize this is outside the scope of this patch, but why are the
semaphore events handles (pm_event[i].sem_handle) allocated? In other
words, why isn't the structure definition:
struct pm_event {
enum pm_event_type event_type;
struct semaphore sem;
};
Then the initialization wouldn't have to allocate memory and could just
call sema_init(&pm_event[i].sem, 0), which makes init and de-init a bit
simpler.
rtg
--
Tim Gardner tim.gardner at canonical.com
More information about the kernel-team
mailing list