[PATCH 3/5] SYSLINK:IPU-PM: solve circular dependencies ipu_pm

[Tim Gardner]

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 at canonical.com'
>> Cc: Bryan Wu; kernel-team at lists.ubuntu.com; ogra at 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 at canonical.com]
>>> Sent: Friday, July 02, 2010 8:17 AM
>>> To: Vadillo, Miguel
>>> Cc: Bryan Wu; kernel-team at lists.ubuntu.com; ogra at 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 at canonical.com
>>
>> Regards...
>> Miguel Vadillo


-- 
Tim Gardner tim.gardner at canonical.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-SYSLINK-IPU-PM-solve-circular-dependencies-ipu_pm.patch
Type: text/x-patch
Size: 7383 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20100706/8abc1898/attachment.bin>