Hi Rene,
On Fri, 2010-11-05 at 16:41 -0600, Sapiens, Rene wrote:
> Hi Ionut,
>
> On Fri, Nov 5, 2010 at 9:13 AM, Ionut Nicu <ionut.nicu@xxxxxxxxx> wrote:
> > Convert the pmgr module of the tidspbridge driver
> > to use struct list_head instead of struct lst_list.
>
> <snip>
>
> > + * Memory is coalesced back to the appropriate heap when a buffer is
>
> What is being fixed here?
>
It was a typo (s/coelesced/coalesced/).
> > * freed.
> > *
> > * Notes:
>
> <snip>
>
> > @@ -833,67 +768,44 @@ static void add_to_free_list(struct cmm_allocator *allocator,
> > DBC_REQUIRE(allocator != NULL);
> > dw_this_pa = pnode->dw_pa;
> > dw_next_pa = NEXT_PA(pnode);
>
> i think it would be good to return with error if !allocator or !pnode
> and remove the resulting duplicated DBC_REQUIRE.
>
Yeah I think pnode should be checked for null. Can allocator ever be
null?
> > - mnode_obj = (struct cmm_mnode *)lst_first(allocator->free_list_head);
> > - while (mnode_obj) {
> > + list_for_each_entry(mnode_obj, &allocator->free_list, link) {
> > if (dw_this_pa == NEXT_PA(mnode_obj)) {
>
> <snip>
>
> > @@ -748,18 +736,16 @@ bool dev_init(void)
> > */
> > int dev_notify_clients(struct dev_object *hdev_obj, u32 ret)
> > {
> > - int status = 0;
> > -
> > struct dev_object *dev_obj = hdev_obj;
> > - void *proc_obj;
> > + struct list_head *curr;
>
> can we add a check for !dev_obj and !dev_obj->proc_list just to be
> sure that we get always the correct pointer?
>
proc_list isn't a pointer. Can dev_obj ever be null?
> <snip>
>
> > @@ -947,15 +933,17 @@ int dev_insert_proc_object(struct dev_object *hdev_obj,
> > DBC_REQUIRE(refs > 0);
> > DBC_REQUIRE(dev_obj);
> > DBC_REQUIRE(proc_obj != 0);
> > - DBC_REQUIRE(dev_obj->proc_list != NULL);
> > DBC_REQUIRE(already_attached != NULL);
>
> can we check for !hdev_obj, !already_attached even if we have the
> DBC_REQUIRE?, maybe we can actually remove the DBC_REQUIRE that could
> be redundant after applying this.
>
Same question here.
> > - if (!LST_IS_EMPTY(dev_obj->proc_list))
> > + if (!list_empty(&dev_obj->proc_list))
> > *already_attached = true;
>
> <snip>
>
> > @@ -986,15 +974,12 @@ int dev_remove_proc_object(struct dev_object *hdev_obj, u32 proc_obj)
> >
> > DBC_REQUIRE(dev_obj);
> > DBC_REQUIRE(proc_obj != 0);
> > - DBC_REQUIRE(dev_obj->proc_list != NULL);
> > - DBC_REQUIRE(!LST_IS_EMPTY(dev_obj->proc_list));
> > + DBC_REQUIRE(!list_empty(&dev_obj->proc_list));
> >
>
> The same comment as above.
>
Regards,
Ionut.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
