[PATCH 01/04] OMAP3 SRF: Generic shared resource f/w

[PATCH 01/04] OMAP3 SRF: Generic shared resource f/w

[Rajendra Nayak]

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

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 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
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-omap-2.6/arch/arm/plat-omap/resource.c	2008-10-16 18:04:05.000000000 +0530
@@ -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 <rnayak@ti.com>
+ *
+ * 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:
+ *
+ */
+
+#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
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-omap-2.6/arch/arm/plat-omap/include/mach/resource.h	2008-10-16 18:01:57.000000000 +0530
@@ -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 <rnayak@ti.com>
+ *
+ * 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:
+ *
+ */
+
+#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 */

Re: [PATCH 01/04] OMAP3 SRF: Generic shared resource f/w

[David Brownell]

On Thursday 16 October 2008, Rajendra Nayak wrote:
> +       /* 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);

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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 01/04] OMAP3 SRF: Generic shared resource f/w

[Rajendra Nayak]

> -----Original Message-----
> From: David Brownell [mailto:david-b@pacbell.net] 
> Sent: Thursday, October 16, 2008 11:58 PM
> To: Rajendra Nayak
> Cc: linux-omap@vger.kernel.org
> Subject: Re: [PATCH 01/04] OMAP3 SRF: Generic shared resource f/w
> 
> On Thursday 16 October 2008, Rajendra Nayak wrote:
> > +       /* 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);
> 
> 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"?

I'll try and put in some more details on the design. Whats essentially 
planned is a framework to model "shared multi level" resources.
target_level and supported levels for each resource type might vary 
and are platform specific and defined in the platform specific header file.
(In this case resource34xx.h file in mach-omap2)

Shared Multi level Resources currently planned to be modeled are Power-domain-latencies 
(would have different latencies based on what state (RET/OFF) it enters 
and hence multi level) and OPP/Frequency resources (Will have mutlitple levels 
each representing a defined OPP).

So for instance, for a VDD1-OPP resource a DSP load pridictor would request for OPP2
based on its predictions and CPUFreq might request OPP3 based on the ARM load. 
The F/w then *sums* up all the requests and decides on whats the best *target_level*
for the resource which keeps everyone happy.  
  
> 
> 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
> 
> 

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

RE: [PATCH 01/04] OMAP3 SRF: Generic shared resource f/w

[Dasgupta, Romit]

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


>-----Original Message-----
>From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>owner@vger.kernel.org] On Behalf Of Nayak, Rajendra
>Sent: Thursday, October 16, 2008 7:42 PM
>To: linux-omap@vger.kernel.org
>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 <rnayak@ti.com>
>---
> 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
>================================================================
>===
>--- /dev/null  1970-01-01 00:00:00.000000000 +0000
>+++ linux-omap-2.6/arch/arm/plat-omap/resource.c       2008-10-16
>18:04:05.000000000 +0530
>@@ -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 <rnayak@ti.com>
>+ *
>+ * 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:
>+ *
>+ */
>+
>+#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
>================================================================
>===
>--- /dev/null  1970-01-01 00:00:00.000000000 +0000
>+++ linux-omap-2.6/arch/arm/plat-omap/include/mach/resource.h  2008-10-16
>18:01:57.000000000 +0530
>@@ -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 <rnayak@ti.com>
>+ *
>+ * 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:
>+ *
>+ */
>+
>+#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 majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 01/04] OMAP3 SRF: Generic shared resource f/w

[Rajendra Nayak]

> -----Original Message-----
> From: Dasgupta, Romit [mailto:romit@ti.com] 
> Sent: Friday, October 17, 2008 11:47 AM
> To: Nayak, Rajendra; linux-omap@vger.kernel.org
> 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

Yes, its not. Its done as part of the platform specific change_level call.

> 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.
 
> 
> Trying to review the remaining patches.
> 
> Regards,
> -Romit
> 
> 
> >-----Original Message-----
> >From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> >owner@vger.kernel.org] On Behalf Of Nayak, Rajendra
> >Sent: Thursday, October 16, 2008 7:42 PM
> >To: linux-omap@vger.kernel.org
> >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 <rnayak@ti.com>
> >---
> > 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
> >================================================================
> >===
> >--- /dev/null  1970-01-01 00:00:00.000000000 +0000
> >+++ linux-omap-2.6/arch/arm/plat-omap/resource.c       2008-10-16
> >18:04:05.000000000 +0530
> >@@ -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 <rnayak@ti.com>
> >+ *
> >+ * 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:
> >+ *
> >+ */
> >+
> >+#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
> >================================================================
> >===
> >--- /dev/null  1970-01-01 00:00:00.000000000 +0000
> >+++ 
> linux-omap-2.6/arch/arm/plat-omap/include/mach/resource.h  2008-10-16
> >18:01:57.000000000 +0530
> >@@ -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 <rnayak@ti.com>
> >+ *
> >+ * 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:
> >+ *
> >+ */
> >+
> >+#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 majordomo@vger.kernel.org
> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

RE: [PATCH 01/04] OMAP3 SRF: Generic shared resource f/w

[Dasgupta, Romit]

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

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

RE: [PATCH 01/04] OMAP3 SRF: Generic shared resource f/w

[Rajendra Nayak]

> -----Original Message-----
> From: Dasgupta, Romit [mailto:romit@ti.com] 
> Sent: Friday, October 17, 2008 1:06 PM
> To: Nayak, Rajendra; linux-omap@vger.kernel.org
> 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.
> 
> 

RE: [PATCH 01/04] OMAP3 SRF: Generic shared resource f/w

[Dasgupta, Romit]

Some more points can be noted

resource_init function is not prefixed by __init

>>
>> >> 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.
[Romit] I agree that the curr_level/target_level will depend on the resource type and that is why it seems to be inside struct shared_resource. Anyway in the update_resource_level function the code is trying to find the maximum target level among the users of the resource. So I am not very clear why it can't be done as I mentioned earlier. Kindly see my replies to other patches you posted. 

RE: [PATCH 01/04] OMAP3 SRF: Generic shared resource f/w

[Dasgupta, Romit]

And finally, you had proposed to move to mutex instead of spin locks. Are you sure that we would do a resource_request or request_release from an interrupt context? If unsure maybe we can keep using spin locks and move to using GFP_ATOMIC. You may decide.