> -----Original Message----- > From: Dasgupta, Romit [mailto:romit@xxxxxx] > Sent: Friday, October 17, 2008 1:06 PM > To: Nayak, Rajendra; linux-omap@xxxxxxxxxxxxxxx > Subject: RE: [PATCH 01/04] OMAP3 SRF: Generic shared resource f/w > > >> 1) curr_level of the shared_resource is not updated in > >> update_resource_level > > > >Yes, its not. Its done as part of the platform specific > change_level call. > > [Romit] Kindly see below. I was thinking that it is better > that we do not access any fields of struct shared_resource > from outside this file. Would the line below solve the problem? The actual *target_level* of the resource will depend on the resource type. Hence for say a "per_pwrdm_latency" resource if some user request's for a latency of no more than 130ms, it inturn translates to a PWRDM_RET level depending on the specific latency to go to RET/OFF. So what is currently done is the current_level of the resource in the above case is set to PWRDM_RET and if done the way you are suggesting will be set to 130ms. > > >> >+static int update_resource_level(struct shared_resource *resp) > >> >+{ > >> >+ struct users_list *user; > >> >+ unsigned long target_level; > >> >+ int ret; > >> >+ > >> >+ /* Regenerate the target_value for the resource */ > >> >+ target_level = RES_DEFAULTLEVEL; > >> >+ list_for_each_entry(user, &resp->users_list, node) > >> >+ if (user->level > target_level) > >> >+ target_level = user->level; > >> >+ > >> >+ pr_debug("SRF: Changing Level for resource %s to %ld\n", > >> >+ resp->name, target_level); > >> >+ ret = resp->ops->change_level(resp, target_level); > >> >+ if (ret) { > >> >+ printk(KERN_ERR "Unable to Change" > >> >+ "level for resource > >> %s to %ld\n", > >> >+ resp->name, target_level); > >> >+ } else > resp->curr_level = target_level; /* [Romit] > Should be done here.*/ > >> >+ return ret; > >> >+} > > > >> 2) resource_request invokes spin_lock_irqsave and then if it > >> is a request for a new device it invokes get_user(). > >> get_user() calls kmalloc with GFP_KERNEL. So it can sleep. > >> Hence you will sleep with spinlocks held!! > > > >Right, so I'll probably have to add a GFP_ATOMIC flag to that. > >I am now thinking If I really need spinlocks, think I can do > with mutex's instead. > >The spinlocks were put in place to take care of the omap-pm > hooks from > >clock f/w which no longer seem to be needed. > [Romit] Trying to review the rest of the patches in the > patchset to get a better picture. > > -- 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
