OMAP3 SRF: Generic shared resource f/w

+       /* Function to change the level of the resource */
+       int (*change_level)(struct shared_resource *res, u32 target_level); +       /* Function to validate the requested level of the resource */ +       int (*validate_level)(struct shared_resource *res, u32 target_level);

-----Original Message-----
From: David Brownell [mailto:[email protected]]
Sent: Thursday, October 16, 2008 11:58 PM
To: Rajendra Nayak
Cc: [email protected]
Subject: Re: [PATCH 01/04] OMAP3 SRF: Generic shared resource f/w

Quoted 2 lines · Rajendra Nayak

+       /* Function to change the level of the resource */ +       int (*change_level)(struct shared_resource *res,

u32 target_level);

Quoted 1 line

+       /* Function to validate the requested level of the

resource */

Quoted 1 line

+       int (*validate_level)(struct shared_resource *res,

u32 target_level);

What is a "target_level" supposed to represent? Are they even comparable, in the senses that

(a) "42" for one resource is the same as "42" for another?
(b) "42" includes "41", "40", etc?

There's no documentation at all on what seems to be a fairly fundamental concept. Or on the rest of it either; the call syntax information is no real help. What kind of "resource" is involved, that might need a "level"?

Also, I suspect a flat namespace for all resources will be a net lose. Why isn't it scoped by devices, so that device-relative "logical names" can be used instead of requiring what have tended to be platform-specific and very mutable "physical names"? I notice that the call to request a resource is already device-scoped ...

- Dave

-----Original Message-----
From: [email protected] [mailto:linux-omap- [email protected]] On Behalf Of Nayak, Rajendra
Sent: Thursday, October 16, 2008 7:42 PM
To: [email protected]
Subject: [PATCH 01/04] OMAP3 SRF: Generic shared resource f/w

This patch defines and implements the Generic
Shared Resource Framework structures and API's

Signed-off-by Rajendra Nayak <[email protected]>

---
arch/arm/plat-omap/include/mach/resource.h | 79 +++++
arch/arm/plat-omap/resource.c | 433 +++++++++++++++++++++++++++++

2 files changed, 512 insertions(+)

Index: linux-omap-2.6/arch/arm/plat-omap/resource.c
================================================================
===

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +0 -0

18:04:05.000000000 +0530

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +14 -0

@@ -0,0 +1,433 @@
+/*
+ * linux/arch/arm/plat-omap/resource.c
+ * Shared Resource Framework API implementation
+ *
+ * Copyright (C) 2007-2008 Texas Instruments, Inc.
+ * Rajendra Nayak <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * THIS PACKAGE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
+ * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR

PURPOSE.
+ * History:
+ *
+ */

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +416 -0

+
+#include <linux/errno.h>
+#include <linux/err.h>
+#include <mach/resource.h>
+#include <linux/slab.h>
+
+/*
+ * This is for statically defining the users pool. This static pool is
+ * used early at bootup till kmalloc becomes available.
+ */
+#define MAX_USERS     10
+#define UNUSED                0x0
+#define       DYNAMIC_ALLOC   0x1
+#define STATIC_ALLOC  0x2
+
+/* res_list contains all registered struct shared_resource */
+static LIST_HEAD(res_list);
+
+/* res_lock protects res_list add and del ops */
+static DEFINE_SPINLOCK(res_lock);
+
+/* Static Pool of users for a resource used till kmalloc becomes available */
+struct  users_list usr_list[MAX_USERS];
+
+/* Private/Internal functions */
+
+/**
+ * _resource_lookup - loop up a resource by its name, return a pointer
+ * @name: The name of the resource to lookup
+ *
+ * Looks for a registered resource by its name. Returns a pointer to
+ * the struct shared_resource if found, else returns NULL.
+ * The function is not lock safe.
+ */
+static struct shared_resource *_resource_lookup(const char *name)
+{
+      struct shared_resource *res, *tmp_res;
+
+      if (!name)
+              return NULL;
+
+      res = NULL;
+
+      list_for_each_entry(tmp_res, &res_list, node) {
+              if (!strcmp(name, tmp_res->name)) {
+                      res = tmp_res;
+                      break;
+              }
+      }
+      return res;
+}
+
+/**
+ * resource_lookup - loop up a resource by its name, return a pointer
+ * @name: The name of the resource to lookup
+ *
+ * Looks for a registered resource by its name. Returns a pointer to
+ * the struct shared_resource if found, else returns NULL.
+ * The function holds spinlocks and takes care of atomicity.
+ */
+static struct shared_resource *resource_lookup(const char *name)
+{
+      struct shared_resource *res;
+      unsigned long flags;
+
+      if (!name)
+              return NULL;
+      spin_lock_irqsave(&res_lock, flags);
+      res = _resource_lookup(name);
+      spin_unlock_irqrestore(&res_lock, flags);
+
+      return res;
+}
+
+/**
+ * update_resource_level - Regenerates and updates the curr_level of the res
+ * @resp: Pointer to the resource
+ *
+ * This function looks at all the users of the given resource and the levels
+ * requested by each of them, and recomputes a target level for the resource
+ * acceptable to all its current usres. It then calls platform specific
+ * change_level to change the level of the resource.
+ * Returns 0 on success, else a non-zero value returned by the platform
+ * specific change_level function.
+ **/
+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);
+      }
+      return ret;
+}
+
+/**
+ * get_user - gets a new users_list struct from static pool or dynamically
+ *
+ * This function initally looks for availability in the static pool and
+ * tries to dynamcially allocate only once the static pool is empty.
+ * We hope that during bootup by the time we hit a case of dynamic allocation
+ * slab initialization would have happened.
+ * Returns a pointer users_list struct on success. On dynamic allocation failure
+ * returns a ERR_PTR(-ENOMEM).
+ */
+static struct users_list *get_user(void)
+{
+      int ind = 0;
+      struct users_list *user;
+
+      /* See if something available in the static pool */
+      while (ind < MAX_USERS) {
+              if (usr_list[ind].usage == UNUSED)
+                      break;
+              else
+                      ind++;
+      }
+      if (ind < MAX_USERS) {
+              /* Pick from the static pool */
+              user = &usr_list[ind];
+              user->usage = STATIC_ALLOC;
+      } else {
+              /* By this time we hope slab is initialized */
+              if (slab_is_available()) {
+                      user = kmalloc(sizeof(struct  users_list), GFP_KERNEL);
+                      if (!user) {
+                              printk(KERN_ERR "SRF:FATAL ERROR: kmalloc"
+                                                      "failed\n");
+                              return ERR_PTR(-ENOMEM);
+                      }
+                      user->usage = DYNAMIC_ALLOC;
+              } else {
+                      /* Dynamic alloc not available yet */
+                      printk(KERN_ERR "SRF: FATAL ERROR: users_list"
+                              "initial POOL EMPTY before slab init\n");
+                      return ERR_PTR(-ENOMEM);
+              }
+      }
+      return user;
+}
+
+/**
+ * free_user - frees the dynamic users_list and marks the static one unused
+ * @user: The struct users_list to be freed
+ *
+ * Looks at the usage flag and either frees the users_list if it was
+ * dynamically allocated, and if its from the static pool, marks it unused.
+ * No return value.
+ */
+void free_user(struct users_list *user)
+{
+      if (user->usage == DYNAMIC_ALLOC) {
+              kfree(user);
+      } else {
+              user->usage = UNUSED;
+              user->level = RES_DEFAULTLEVEL;
+              user->dev = NULL;
+      }
+}
+
+/**
+ * resource_init - Initializes the Shared resource framework.
+ * @resources: List of all the resources modelled
+ *
+ * Loops through the list of resources and registers all that
+ * are available for the current CPU.
+ * No return value
+ */
+void resource_init(struct shared_resource **resources)
+{
+      struct shared_resource **resp;
+      int ind;
+
+      pr_debug("Initializing Shared Resource Framework\n");
+
+      if (!cpu_is_omap34xx()) {
+              /* This CPU is not supported */
+              printk(KERN_WARNING "Shared Resource Framework does not"
+                      "support this CPU type.\n");
+              WARN_ON(1);
+      }
+
+      /* Init the users_list POOL */
+      for (ind = 0; ind < MAX_USERS; ind++) {
+              usr_list[ind].usage = UNUSED;
+              usr_list[ind].dev = NULL;
+              usr_list[ind].level = RES_DEFAULTLEVEL;
+      }
+
+      if (resources)
+              for (resp = resources; *resp; resp++)
+                      resource_register(*resp);
+}
+
+/**
+ * resource_register - registers and initializes a resource
+ * @res: struct shared_resource * to register
+ *
+ * Initializes the given resource and adds it to the resource list
+ * for the current CPU.
+ * Returns 0 on success, -EINVAL if given a NULL pointer, -EEXIST if the
+ * resource is already registered.
+ */
+int resource_register(struct shared_resource *resp)
+{
+      unsigned long flags;
+
+      if (!resp)
+              return -EINVAL;
+
+      if (!omap_chip_is(resp->omap_chip))
+              return -EINVAL;
+
+      /* Verify that the resource is not already registered */
+      if (resource_lookup(resp->name))
+              return -EEXIST;
+
+      INIT_LIST_HEAD(&resp->users_list);
+
+      spin_lock_irqsave(&res_lock, flags);
+      /* Add the resource to the resource list */
+      list_add(&resp->node, &res_list);
+
+      /* Call the resource specific init*/
+      if (resp->ops->init)
+              resp->ops->init(resp);
+
+      spin_unlock_irqrestore(&res_lock, flags);
+      pr_debug("resource: registered %s\n", resp->name);
+
+      return 0;
+}
+EXPORT_SYMBOL(resource_register);
+
+/**
+ * resource_unregister - unregister a resource
+ * @res: struct shared_resource * to unregister
+ *
+ * Removes a resource from the resource list.
+ * Returns 0 on success, -EINVAL if passed a NULL pointer.
+ */
+int resource_unregister(struct shared_resource *resp)
+{
+      unsigned long flags;
+
+      if (!resp)
+              return -EINVAL;
+
+      spin_lock_irqsave(&res_lock, flags);
+      /* delete the resource from the resource list */
+      list_del(&resp->node);
+      spin_unlock_irqrestore(&res_lock, flags);
+
+      pr_debug("resource: unregistered %s\n", resp->name);
+
+      return 0;
+}
+EXPORT_SYMBOL(resource_unregister);
+
+/**
+ * resource_request - Request for a required level of a resource
+ * @name: The name of the resource requested
+ * @dev: Uniquely identifes the caller
+ * @level: The requested level for the resource
+ *
+ * This function recomputes the target level of the resource based on
+ * the level requested by the user. The level of the resource is
+ * changed to the target level, if it is not the same as the existing level
+ * of the resource. Multiple calls to this function by the same device will
+ * replace the previous level requested
+ * Returns 0 on success, -EINVAL if the resource name passed in invalid.
+ * -ENOMEM if no static pool available or dynamic allocations fails.
+ * Else returns a non-zero error value returned by one of the failing
+ * shared_resource_ops.
+ */
+int resource_request(const char *name, struct device *dev,
+                                      unsigned long level)
+{
+      struct shared_resource *resp;
+      struct  users_list *user;
+      int     found = 0, ret = 0;
+      unsigned long flags;
+
+      spin_lock_irqsave(&res_lock, flags);
+      resp = _resource_lookup(name);
+      if (!resp) {
+              printk(KERN_ERR "resource_request: Invalid resource name\n");
+              ret = -EINVAL;
+              goto res_unlock;
+      }
+
+      /* Call the resource specific validate function */
+      if (resp->ops->validate_level) {
+              ret = resp->ops->validate_level(resp, level);
+              if (ret)
+                      goto res_unlock;
+      }
+
+      list_for_each_entry(user, &resp->users_list, node) {
+              if (user->dev == dev) {
+                      found = 1;
+                      break;
+              }
+      }
+
+      if (!found) {
+              /* First time user */
+              user = get_user();
+              if (IS_ERR(user)) {
+                      ret = -ENOMEM;
+                      goto res_unlock;
+              }
+              user->dev = dev;
+              list_add(&user->node, &resp->users_list);
+              resp->no_of_users++;
+      }
+      user->level = level;
+
+      /* Recompute and set the current level for the resource */
+      ret = update_resource_level(resp);
+res_unlock:
+      spin_unlock_irqrestore(&res_lock, flags);
+      return ret;
+}
+EXPORT_SYMBOL(resource_request);
+
+/**
+ * resource_release - Release a previously requested level of a resource
+ * @name: The name of the resource to be released
+ * @dev: Uniquely identifes the caller
+ *
+ * This function recomputes the target level of the resource after removing
+ * the level requested by the user. The level of the resource is
+ * changed to the target level, if it is not the same as the existing level
+ * of the resource.
+ * Returns 0 on success, -EINVAL if the resource name or dev structure
+ * is invalid.
+ */
+int resource_release(const char *name, struct device *dev)
+{
+      struct shared_resource *resp;
+      struct users_list *user;
+      int found = 0, ret = 0;
+      unsigned long flags;
+
+      spin_lock_irqsave(&res_lock, flags);
+      resp = _resource_lookup(name);
+      if (!resp) {
+              printk(KERN_ERR "resource_release: Invalid resource name\n");
+              ret = -EINVAL;
+              goto res_unlock;
+      }
+
+      list_for_each_entry(user, &resp->users_list, node) {
+              if (user->dev == dev) {
+                      found = 1;
+                      break;
+              }
+      }
+
+      if (!found) {
+              /* No such user exists */
+              ret = -EINVAL;
+              goto res_unlock;
+      }
+
+      resp->no_of_users--;
+      list_del(&user->node);
+      free_user(user);
+
+      /* Recompute and set the current level for the resource */
+      ret = update_resource_level(resp);
+res_unlock:
+      spin_unlock_irqrestore(&res_lock, flags);
+      return ret;
+}
+EXPORT_SYMBOL(resource_release);
+
+/**
+ * resource_get_level - Returns the current level of the resource
+ * @name: Name of the resource
+ *
+ * Returns the current level of the resource if found, else returns
+ * -EINVAL if the resource name is invalid.
+ */
+int resource_get_level(const char *name)
+{
+      struct shared_resource *resp;
+      u32 ret;
+      unsigned long flags;
+
+      spin_lock_irqsave(&res_lock, flags);
+      resp = _resource_lookup(name);
+      if (!resp) {
+              printk(KERN_ERR "resource_release: Invalid resource name\n");
+              spin_unlock_irqrestore(&res_lock, flags);
+              return -EINVAL;
+      }
+      ret = resp->curr_level;
+      spin_unlock_irqrestore(&res_lock, flags);
+      return ret;
+}
+EXPORT_SYMBOL(resource_get_level);

Index: linux-omap-2.6/arch/arm/plat-omap/include/mach/resource.h
================================================================
===

arch/arm/plat-omap/include/mach/resource.h 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +0 -0

18:01:57.000000000 +0530

arch/arm/plat-omap/include/mach/resource.h 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +14 -0

@@ -0,0 +1,79 @@
+/*
+ * linux/include/asm-arm/arch-omap/resource.h
+ * Structure definitions for Shared resource Framework
+ *
+ * Copyright (C) 2007-2008 Texas Instruments, Inc.
+ * Written by Rajendra Nayak <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * THIS PACKAGE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
+ * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR

PURPOSE.
+ *
+ * History:
+ *
+ */

arch/arm/plat-omap/include/mach/resource.h 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +61 -1

+
+#ifndef __ARCH_ARM_OMAP_RESOURCE_H
+#define __ARCH_ARM_OMAP_RESOURCE_H
+
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/device.h>
+#include <mach/cpu.h>
+
+#define RES_DEFAULTLEVEL      0x0
+
+struct shared_resource_ops; /* forward declaration */
+
+/* Used to model a Shared Multilevel Resource */
+struct shared_resource {
+      /* Resource name */
+      const char *name;
+      /* Used to represent the OMAP chip types containing this res */
+      const struct omap_chip_id omap_chip;
+      /* Total no of users at any point of this resource */
+      u8 no_of_users;
+      /* Current level of this resource */
+      u32 curr_level;
+      /* Used to store any resource specific data */
+      void  *resource_data;
+      /* List of all the current users for this resource */
+      struct list_head users_list;
+      /* Shared resource operations */
+      struct shared_resource_ops *ops;
+      struct list_head node;
+};
+
+struct shared_resource_ops {
+      /* Init function for the resource */
+      void (*init)(struct shared_resource *res);
+      /* Function to change the level of the resource */
+      int (*change_level)(struct shared_resource *res, u32 target_level);
+      /* Function to validate the requested level of the resource */
+      int (*validate_level)(struct shared_resource *res, u32 target_level);
+};
+
+/* Used to represent a user of a shared resource */
+struct users_list {
+      /* Device pointer used to uniquely identify the user */
+      struct device *dev;
+      /* Current level as requested for the resource by the user */
+      u32 level;
+      struct list_head node;
+      u8 usage;
+};
+
+/* Shared resource Framework API's */
+void resource_init(struct shared_resource **resources);
+int resource_register(struct shared_resource *res);
+int resource_unregister(struct shared_resource *res);
+int resource_request(const char *name, struct device *dev,
+                                               unsigned long level);
+int resource_release(const char *name, struct device *dev);
+int resource_get_level(const char *name);
+
+#endif /* __ARCH_ARM_OMAP_RESOURCE_H */
 
--

To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

-----Original Message-----
From: Dasgupta, Romit [mailto:[email protected]]
Sent: Friday, October 17, 2008 11:47 AM
To: Nayak, Rajendra; [email protected]
Subject: RE: [PATCH 01/04] OMAP3 SRF: Generic shared resource f/w

Rajendra,
Few bugs in the code
1) curr_level of the shared_resource is not updated in update_resource_level

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!!

Trying to review the remaining patches.

Regards,
-Romit

Quoted 7 lines

-----Original Message-----
From: [email protected] [mailto:linux-omap- [email protected]] On Behalf Of Nayak, Rajendra
Sent: Thursday, October 16, 2008 7:42 PM
To: [email protected]
Subject: [PATCH 01/04] OMAP3 SRF: Generic shared resource f/w

Quoted 7 lines

This patch defines and implements the Generic Shared Resource Framework structures and API's

Signed-off-by Rajendra Nayak <[email protected]>

---
arch/arm/plat-omap/include/mach/resource.h | 79 +++++
arch/arm/plat-omap/resource.c | 433

+++++++++++++++++++++++++++++

Quoted 9 lines

2 files changed, 512 insertions(+)

Index: linux-omap-2.6/arch/arm/plat-omap/resource.c
================================================================
===

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +0 -0

18:04:05.000000000 +0530

arch/arm/plat-omap/resource.c 2008-10-16 line 8

Quoted 8 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +8 -0

@@ -0,0 +1,433 @@
+/*
+ * linux/arch/arm/plat-omap/resource.c
+ * Shared Resource Framework API implementation
+ *
+ * Copyright (C) 2007-2008 Texas Instruments, Inc.
+ * Rajendra Nayak <[email protected]>
+ *
+ * This program is free software; you can redistribute it 

and/or modify

Quoted 10 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +6 -0

+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * THIS PACKAGE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
+ * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR

PURPOSE.
+ * History:
+ *
+ */

arch/arm/plat-omap/resource.c 2008-10-16

Quoted 8 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +8 -0

+
+#include <linux/errno.h>
+#include <linux/err.h>
+#include <mach/resource.h>
+#include <linux/slab.h>
+
+/*
+ * This is for statically defining the users pool. This 

static pool is

Quoted 4 lines

+ * used early at bootup till kmalloc becomes available.
+ */

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +2 -0

+#define MAX_USERS     10
+#define UNUSED                0x0

arch/arm/plat-omap/resource.c 2008-10-16

Quoted 10 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10 -0

+#define       DYNAMIC_ALLOC   0x1
+#define STATIC_ALLOC  0x2
+
+/* res_list contains all registered struct shared_resource */
+static LIST_HEAD(res_list);
+
+/* res_lock protects res_list add and del ops */
+static DEFINE_SPINLOCK(res_lock);
+
+/* Static Pool of users for a resource used till kmalloc 

becomes available */

arch/arm/plat-omap/resource.c 2008-10-16

Quoted 6 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +6 -0

+struct  users_list usr_list[MAX_USERS];
+
+/* Private/Internal functions */
+
+/**
+ * _resource_lookup - loop up a resource by its name, 

return a pointer

Quoted 16 lines

+ * @name: The name of the resource to lookup
+ *
+ * Looks for a registered resource by its name. Returns a pointer to
+ * the struct shared_resource if found, else returns NULL.
+ * The function is not lock safe.
+ */

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10 -0

+static struct shared_resource *_resource_lookup(const char *name)
+{
+      struct shared_resource *res, *tmp_res;
+
+      if (!name)
+              return NULL;
+
+      res = NULL;
+
+      list_for_each_entry(tmp_res, &res_list, node) {

arch/arm/plat-omap/resource.c 2008-10-16

Quoted 10 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10 -0

+              if (!strcmp(name, tmp_res->name)) {
+                      res = tmp_res;
+                      break;
+              }
+      }
+      return res;
+}
+
+/**
+ * resource_lookup - loop up a resource by its name, return 

a pointer

Quoted 12 lines

+ * @name: The name of the resource to lookup
+ *
+ * Looks for a registered resource by its name. Returns a pointer to
+ * the struct shared_resource if found, else returns NULL.
+ * The function holds spinlocks and takes care of atomicity.
+ */

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +6 -0

+static struct shared_resource *resource_lookup(const char *name)
+{
+      struct shared_resource *res;
+      unsigned long flags;
+
+      if (!name)

arch/arm/plat-omap/resource.c 2008-10-16

Quoted 10 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10 -0

+              return NULL;
+      spin_lock_irqsave(&res_lock, flags);
+      res = _resource_lookup(name);
+      spin_unlock_irqrestore(&res_lock, flags);
+
+      return res;
+}
+
+/**
+ * update_resource_level - Regenerates and updates the 

curr_level of the res

Quoted 3 lines

+ * @resp: Pointer to the resource
+ *
+ * This function looks at all the users of the given

resource and the levels

Quoted 1 line

+ * requested by each of them, and recomputes a target level

for the resource

Quoted 1 line

+ * acceptable to all its current usres. It then calls

platform specific

Quoted 2 lines

+ * change_level to change the level of the resource.
+ * Returns 0 on success, else a non-zero value returned by

the platform

Quoted 10 lines

+ * specific change_level function.
+ **/

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +8 -0

+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;

arch/arm/plat-omap/resource.c 2008-10-16

Quoted 10 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10 -0

+      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",

arch/arm/plat-omap/resource.c 2008-10-16

Quoted 7 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +7 -0

+              resp->name, target_level);
+      }
+      return ret;
+}
+
+/**
+ * get_user - gets a new users_list struct from static pool 

or dynamically

Quoted 2 lines

+ *
+ * This function initally looks for availability in the

static pool and

Quoted 2 lines

+ * tries to dynamcially allocate only once the static pool is empty.
+ * We hope that during bootup by the time we hit a case of

dynamic allocation

Quoted 2 lines

+ * slab initialization would have happened.
+ * Returns a pointer users_list struct on success. On

dynamic allocation failure

Quoted 12 lines

+ * returns a ERR_PTR(-ENOMEM).
+ */

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10 -0

+static struct users_list *get_user(void)
+{
+      int ind = 0;
+      struct users_list *user;
+
+      /* See if something available in the static pool */
+      while (ind < MAX_USERS) {
+              if (usr_list[ind].usage == UNUSED)
+                      break;
+              else

arch/arm/plat-omap/resource.c 2008-10-16

Quoted 10 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10 -0

+                      ind++;
+      }
+      if (ind < MAX_USERS) {
+              /* Pick from the static pool */
+              user = &usr_list[ind];
+              user->usage = STATIC_ALLOC;
+      } else {
+              /* By this time we hope slab is initialized */
+              if (slab_is_available()) {
+                      user = kmalloc(sizeof(struct  

users_list), GFP_KERNEL);

arch/arm/plat-omap/resource.c 2008-10-16

Quoted 2 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +2 -0

+                      if (!user) {
+                              printk(KERN_ERR "SRF:FATAL 

ERROR: kmalloc"

arch/arm/plat-omap/resource.c 2008-10-16

Quoted 8 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +8 -0

+                                                      "failed\n");
+                              return ERR_PTR(-ENOMEM);
+                      }
+                      user->usage = DYNAMIC_ALLOC;
+              } else {
+                      /* Dynamic alloc not available yet */
+                      printk(KERN_ERR "SRF: FATAL ERROR: users_list"
+                              "initial POOL EMPTY before 

slab init\n");

arch/arm/plat-omap/resource.c 2008-10-16

Quoted 8 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +8 -0

+                      return ERR_PTR(-ENOMEM);
+              }
+      }
+      return user;
+}
+
+/**
+ * free_user - frees the dynamic users_list and marks the 

static one unused

Quoted 4 lines

+ * @user: The struct users_list to be freed
+ *
+ * Looks at the usage flag and either frees the users_list if it was
+ * dynamically allocated, and if its from the static pool,

marks it unused.

Quoted 21 lines

+ * No return value.
+ */

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +19 -0

+void free_user(struct users_list *user)
+{
+      if (user->usage == DYNAMIC_ALLOC) {
+              kfree(user);
+      } else {
+              user->usage = UNUSED;
+              user->level = RES_DEFAULTLEVEL;
+              user->dev = NULL;
+      }
+}
+
+/**
+ * resource_init - Initializes the Shared resource framework.
+ * @resources: List of all the resources modelled
+ *
+ * Loops through the list of resources and registers all that
+ * are available for the current CPU.
+ * No return value
+ */

arch/arm/plat-omap/resource.c 2008-10-16

Quoted 10 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10 -0

+void resource_init(struct shared_resource **resources)
+{
+      struct shared_resource **resp;
+      int ind;
+
+      pr_debug("Initializing Shared Resource Framework\n");
+
+      if (!cpu_is_omap34xx()) {
+              /* This CPU is not supported */
+              printk(KERN_WARNING "Shared Resource 

Framework does not"

Quoted 13 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +13 -0

+                      "support this CPU type.\n");
+              WARN_ON(1);
+      }
+
+      /* Init the users_list POOL */
+      for (ind = 0; ind < MAX_USERS; ind++) {
+              usr_list[ind].usage = UNUSED;
+              usr_list[ind].dev = NULL;
+              usr_list[ind].level = RES_DEFAULTLEVEL;
+      }
+
+      if (resources)
+              for (resp = resources; *resp; resp++)

arch/arm/plat-omap/resource.c 2008-10-16

Quoted 10 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10 -0

+                      resource_register(*resp);
+}
+
+/**
+ * resource_register - registers and initializes a resource
+ * @res: struct shared_resource * to register
+ *
+ * Initializes the given resource and adds it to the resource list
+ * for the current CPU.
+ * Returns 0 on success, -EINVAL if given a NULL pointer, 

-EEXIST if the

Quoted 55 lines

+ * resource is already registered.
+ */

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +53 -0

+int resource_register(struct shared_resource *resp)
+{
+      unsigned long flags;
+
+      if (!resp)
+              return -EINVAL;
+
+      if (!omap_chip_is(resp->omap_chip))
+              return -EINVAL;
+
+      /* Verify that the resource is not already registered */
+      if (resource_lookup(resp->name))
+              return -EEXIST;
+
+      INIT_LIST_HEAD(&resp->users_list);
+
+      spin_lock_irqsave(&res_lock, flags);
+      /* Add the resource to the resource list */
+      list_add(&resp->node, &res_list);
+
+      /* Call the resource specific init*/
+      if (resp->ops->init)
+              resp->ops->init(resp);
+
+      spin_unlock_irqrestore(&res_lock, flags);
+      pr_debug("resource: registered %s\n", resp->name);
+
+      return 0;
+}
+EXPORT_SYMBOL(resource_register);
+
+/**
+ * resource_unregister - unregister a resource
+ * @res: struct shared_resource * to unregister
+ *
+ * Removes a resource from the resource list.
+ * Returns 0 on success, -EINVAL if passed a NULL pointer.
+ */
+int resource_unregister(struct shared_resource *resp)
+{
+      unsigned long flags;
+
+      if (!resp)
+              return -EINVAL;
+
+      spin_lock_irqsave(&res_lock, flags);
+      /* delete the resource from the resource list */
+      list_del(&resp->node);
+      spin_unlock_irqrestore(&res_lock, flags);
+
+      pr_debug("resource: unregistered %s\n", resp->name);
+
+      return 0;

arch/arm/plat-omap/resource.c 2008-10-16

Quoted 10 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10 -0

+}
+EXPORT_SYMBOL(resource_unregister);
+
+/**
+ * resource_request - Request for a required level of a resource
+ * @name: The name of the resource requested
+ * @dev: Uniquely identifes the caller
+ * @level: The requested level for the resource
+ *
+ * This function recomputes the target level of the 

resource based on

Quoted 2 lines

+ * the level requested by the user. The level of the resource is
+ * changed to the target level, if it is not the same as

the existing level

Quoted 1 line

+ * of the resource. Multiple calls to this function by the

same device will

Quoted 2 lines

+ * replace the previous level requested
+ * Returns 0 on success, -EINVAL if the resource name

passed in invalid.

Quoted 2 lines

+ * -ENOMEM if no static pool available or dynamic allocations fails.
+ * Else returns a non-zero error value returned by one of

the failing

Quoted 4 lines

+ * shared_resource_ops.
+ */

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +2 -0

+int resource_request(const char *name, struct device *dev,
+                                      unsigned long level)

arch/arm/plat-omap/resource.c 2008-10-16

Quoted 10 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10 -0

+{
+      struct shared_resource *resp;
+      struct  users_list *user;
+      int     found = 0, ret = 0;
+      unsigned long flags;
+
+      spin_lock_irqsave(&res_lock, flags);
+      resp = _resource_lookup(name);
+      if (!resp) {
+              printk(KERN_ERR "resource_request: Invalid 

resource name\n");

Quoted 31 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +31 -0

+              ret = -EINVAL;
+              goto res_unlock;
+      }
+
+      /* Call the resource specific validate function */
+      if (resp->ops->validate_level) {
+              ret = resp->ops->validate_level(resp, level);
+              if (ret)
+                      goto res_unlock;
+      }
+
+      list_for_each_entry(user, &resp->users_list, node) {
+              if (user->dev == dev) {
+                      found = 1;
+                      break;
+              }
+      }
+
+      if (!found) {
+              /* First time user */
+              user = get_user();
+              if (IS_ERR(user)) {
+                      ret = -ENOMEM;
+                      goto res_unlock;
+              }
+              user->dev = dev;
+              list_add(&user->node, &resp->users_list);
+              resp->no_of_users++;
+      }
+      user->level = level;
+

arch/arm/plat-omap/resource.c 2008-10-16

Quoted 10 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10 -0

+      /* Recompute and set the current level for the resource */
+      ret = update_resource_level(resp);
+res_unlock:
+      spin_unlock_irqrestore(&res_lock, flags);
+      return ret;
+}
+EXPORT_SYMBOL(resource_request);
+
+/**
+ * resource_release - Release a previously requested level 

of a resource

Quoted 4 lines

+ * @name: The name of the resource to be released
+ * @dev: Uniquely identifes the caller
+ *
+ * This function recomputes the target level of the

resource after removing

Quoted 2 lines

+ * the level requested by the user. The level of the resource is
+ * changed to the target level, if it is not the same as

the existing level

Quoted 2 lines

+ * of the resource.
+ * Returns 0 on success, -EINVAL if the resource name or

dev structure

arch/arm/plat-omap/resource.c 2008-10-16

Quoted 13 lines

+ * is invalid.
+ */

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +11 -0

+int resource_release(const char *name, struct device *dev)
+{
+      struct shared_resource *resp;
+      struct users_list *user;
+      int found = 0, ret = 0;
+      unsigned long flags;
+
+      spin_lock_irqsave(&res_lock, flags);
+      resp = _resource_lookup(name);
+      if (!resp) {
+              printk(KERN_ERR "resource_release: Invalid 

resource name\n");

Quoted 36 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +36 -0

+              ret = -EINVAL;
+              goto res_unlock;
+      }
+
+      list_for_each_entry(user, &resp->users_list, node) {
+              if (user->dev == dev) {
+                      found = 1;
+                      break;
+              }
+      }
+
+      if (!found) {
+              /* No such user exists */
+              ret = -EINVAL;
+              goto res_unlock;
+      }
+
+      resp->no_of_users--;
+      list_del(&user->node);
+      free_user(user);
+
+      /* Recompute and set the current level for the resource */
+      ret = update_resource_level(resp);
+res_unlock:
+      spin_unlock_irqrestore(&res_lock, flags);
+      return ret;
+}
+EXPORT_SYMBOL(resource_release);
+
+/**
+ * resource_get_level - Returns the current level of the resource
+ * @name: Name of the resource
+ *
+ * Returns the current level of the resource if found, else returns
+ * -EINVAL if the resource name is invalid.
+ */

arch/arm/plat-omap/resource.c 2008-10-16

Quoted 10 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10 -0

+int resource_get_level(const char *name)
+{
+      struct shared_resource *resp;
+      u32 ret;
+      unsigned long flags;
+
+      spin_lock_irqsave(&res_lock, flags);
+      resp = _resource_lookup(name);
+      if (!resp) {
+              printk(KERN_ERR "resource_release: Invalid 

resource name\n");

dev/null 1970-01-01 00:00:00.000000000 +0000

Quoted 13 lines

arch/arm/plat-omap/resource.c 2008-10-16 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +8 -0

+              spin_unlock_irqrestore(&res_lock, flags);
+              return -EINVAL;
+      }
+      ret = resp->curr_level;
+      spin_unlock_irqrestore(&res_lock, flags);
+      return ret;
+}
+EXPORT_SYMBOL(resource_get_level);

Index: linux-omap-2.6/arch/arm/plat-omap/include/mach/resource.h
================================================================
===

dev/null 1970-01-01 00:00:00.000000000 +0000 deleted +0 -0

linux-omap-2.6/arch/arm/plat-omap/include/mach/resource.h 2008-10-16

dev/null 1970-01-01 00:00:00.000000000 +0000 line 8

Quoted 10 lines

18:01:57.000000000 +0530

dev/null 1970-01-01 00:00:00.000000000 +0000 deleted +8 -0

@@ -0,0 +1,79 @@
+/*
+ * linux/include/asm-arm/arch-omap/resource.h
+ * Structure definitions for Shared resource Framework
+ *
+ * Copyright (C) 2007-2008 Texas Instruments, Inc.
+ * Written by Rajendra Nayak <[email protected]>
+ *
+ * This program is free software; you can redistribute it 

and/or modify

Quoted 19 lines

dev/null 1970-01-01 00:00:00.000000000 +0000 deleted +6 -0

+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * THIS PACKAGE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
+ * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR

PURPOSE.
+ *
+ * History:
+ *
+ */

dev/null 1970-01-01 00:00:00.000000000 +0000 deleted +8 -0

+
+#ifndef __ARCH_ARM_OMAP_RESOURCE_H
+#define __ARCH_ARM_OMAP_RESOURCE_H
+
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/device.h>
+#include <mach/cpu.h>

dev/null 1970-01-01 00:00:00.000000000 +0000

Quoted 10 lines

dev/null 1970-01-01 00:00:00.000000000 +0000 deleted +10 -0

+
+#define RES_DEFAULTLEVEL      0x0
+
+struct shared_resource_ops; /* forward declaration */
+
+/* Used to model a Shared Multilevel Resource */
+struct shared_resource {
+      /* Resource name */
+      const char *name;
+      /* Used to represent the OMAP chip types containing 

this res */

Quoted 9 lines

dev/null 1970-01-01 00:00:00.000000000 +0000 deleted +9 -0

+      const struct omap_chip_id omap_chip;
+      /* Total no of users at any point of this resource */
+      u8 no_of_users;
+      /* Current level of this resource */
+      u32 curr_level;
+      /* Used to store any resource specific data */
+      void  *resource_data;
+      /* List of all the current users for this resource */
+      struct list_head users_list;

dev/null 1970-01-01 00:00:00.000000000 +0000

Quoted 10 lines

dev/null 1970-01-01 00:00:00.000000000 +0000 deleted +10 -0

+      /* Shared resource operations */
+      struct shared_resource_ops *ops;
+      struct list_head node;
+};
+
+struct shared_resource_ops {
+      /* Init function for the resource */
+      void (*init)(struct shared_resource *res);
+      /* Function to change the level of the resource */
+      int (*change_level)(struct shared_resource *res, u32 

target_level);

dev/null 1970-01-01 00:00:00.000000000 +0000

Quoted 2 lines

dev/null 1970-01-01 00:00:00.000000000 +0000 deleted +2 -0

+      /* Function to validate the requested level of the resource */
+      int (*validate_level)(struct shared_resource *res, 

u32 target_level);

Quoted 24 lines

dev/null 1970-01-01 00:00:00.000000000 +0000 deleted +22 -1

+};
+
+/* Used to represent a user of a shared resource */
+struct users_list {
+      /* Device pointer used to uniquely identify the user */
+      struct device *dev;
+      /* Current level as requested for the resource by the user */
+      u32 level;
+      struct list_head node;
+      u8 usage;
+};
+
+/* Shared resource Framework API's */
+void resource_init(struct shared_resource **resources);
+int resource_register(struct shared_resource *res);
+int resource_unregister(struct shared_resource *res);
+int resource_request(const char *name, struct device *dev,
+                                               unsigned long level);
+int resource_release(const char *name, struct device *dev);
+int resource_get_level(const char *name);
+
+#endif /* __ARCH_ARM_OMAP_RESOURCE_H */
 
--

Quoted 1 line

To unsubscribe from this list: send the line "unsubscribe

linux-omap" in

Quoted 2 lines

the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 2 lines

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.

Quoted 19 lines

Quoted 8 lines

arch/arm/plat-omap/resource.c new file +8 -0

+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;

arch/arm/plat-omap/resource.c

Quoted 10 lines

arch/arm/plat-omap/resource.c new file +10 -0

+      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",

Quoted 2 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+              resp->name, target_level);
+      } else 

Quoted 7 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+      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.

-----Original Message-----
From: Dasgupta, Romit [mailto:[email protected]]
Sent: Friday, October 17, 2008 1:06 PM
To: Nayak, Rajendra; [email protected]
Subject: RE: [PATCH 01/04] OMAP3 SRF: Generic shared resource f/w

Quoted 3 lines

Quoted 2 lines

1) curr_level of the shared_resource is not updated in update_resource_level

Quoted 1 line

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?

Quoted 20 lines

Quoted 20 lines

Quoted 9 lines

arch/arm/plat-omap/resource.c new file +8 -0

+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;

arch/arm/plat-omap/resource.c

Quoted 10 lines

arch/arm/plat-omap/resource.c new file +10 -0

+      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",

Quoted 2 lines

Quoted 2 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+              resp->name, target_level);
+      } else 

	          resp->curr_level = target_level; /* [Romit]  

Should be done here.*/

Quoted 10 lines

Quoted 7 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+      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.

Quoted 1 line

The spinlocks were put in place to take care of the omap-pm

hooks from

Quoted 1 line

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.

Quoted 10 lines

Quoted 5 lines

Quoted 3 lines

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.