OMAP MUX framework changes

PATCH In review Dan Murphy 3 participants 6 messages Nov 17, 2010 – Dec 2, 2010

Subsystems (3)

Arm (32-bit) Arm sub-architectures OMAP2+ support

lore

Share & tools

lore

Dan Murphy

author

2010-11-17 18:07 UTC #0 lore raw

PATCH 1/1 OMAP MUX framework changes

Added APIs to get and set the mux setting via the MUX name. In doing this the omap_mux_read and omap_mux_write can be made static as these interfaces should be called indirectly.

Added a check in the omap_write_array to test the partition pointer is valid prior to dereferencing.

Tested the new interfaces with a test file.

Signed-off-by Dan Murphy <[email protected]>

---
 arch/arm/mach-omap2/mux.c |   73 +++++++++++++++++++++++++++++++++++++++++----
 arch/arm/mach-omap2/mux.h |   23 ++++++--------
 2 files changed, 77 insertions(+), 19 deletions(-)

arch/arm/mach-omap2/mux.c +67 -6

index 3d71d93..e53d6a3 100644
@@ -61,7 +61,7 @@ struct omap_mux_partition *omap_mux_get(const char *name)
 	return NULL;
 }
 
-u16 omap_mux_read(struct omap_mux_partition *partition, u16 reg)
+static u16 omap_mux_read(struct omap_mux_partition *partition, u16 reg)
 {
 	if (partition->flags & OMAP_MUX_REG_8BIT)
 		return __raw_readb(partition->base + reg);

@@ -69,7 +69,7 @@ u16 omap_mux_read(struct omap_mux_partition *partition, u16 reg)
 		return __raw_readw(partition->base + reg);
 }
 
-void omap_mux_write(struct omap_mux_partition *partition, u16 val,
+static void omap_mux_write(struct omap_mux_partition *partition, u16 val,
 			   u16 reg)
 {
 	if (partition->flags & OMAP_MUX_REG_8BIT)

@@ -81,10 +81,14 @@ void omap_mux_write(struct omap_mux_partition *partition, u16 val,
 void omap_mux_write_array(struct omap_mux_partition *partition,
 				 struct omap_board_mux *board_mux)
 {
-	while (board_mux->reg_offset != OMAP_MUX_TERMINATOR) {
-		omap_mux_write(partition, board_mux->value,
-			       board_mux->reg_offset);
-		board_mux++;
+	if (partition) {
+		while (board_mux->reg_offset != OMAP_MUX_TERMINATOR) {
+			omap_mux_write(partition, board_mux->value,
+				       board_mux->reg_offset);
+			board_mux++;
+		}
+	} else {
+		pr_err("%s: Partition was NULL\n", __func__);
 	}
 }

@@ -745,6 +749,63 @@ void omap_mux_set_gpio(u16 val, int gpio)
 		pr_err("%s: Could not set gpio%i\n", __func__, gpio);
 }
 
+static struct omap_mux *omap_mux_get_by_mux(struct omap_mux_partition *partition,
+					char *name)
+{
+	struct omap_mux_entry *e;
+	int i = 0;
+
+	list_for_each_entry(e, &partition->muxmodes, node) {
+		struct omap_mux *m = &e->mux;
+		for (i = 0; i < OMAP_MUX_NR_MODES; i++) {
+			if (m->muxnames[i] == NULL)
+				break;
+			else if (!strcmp(name, m->muxnames[i]))
+				return m;
+		}
+	}
+
+	return NULL;
+}
+
+/* Needed for dynamic muxing of pins for off-idle */
+u16 omap_mux_get_mux(char *mux_name)
+{
+	struct omap_mux_partition *partition;
+	struct omap_mux *m;
+
+	list_for_each_entry(partition, &mux_partitions, node) {
+		m = omap_mux_get_by_mux(partition, mux_name);
+		if (m)
+			return omap_mux_read(partition, m->reg_offset);
+	}
+
+	if (!m || m->reg_offset == OMAP_MUX_TERMINATOR)
+		pr_err("%s: Could not get mux %s\n",
+			__func__, mux_name);
+
+	return OMAP_MUX_TERMINATOR;
+}
+
+/* Needed for dynamic muxing pins for off-idle */
+void omap_mux_set_mux(u16 val, char *mux_name)
+{
+	struct omap_mux_partition *partition;
+	struct omap_mux *m = NULL;
+
+	list_for_each_entry(partition, &mux_partitions, node) {
+		m = omap_mux_get_by_mux(partition, mux_name);
+		if (m) {
+			omap_mux_write(partition, val, m->reg_offset);
+			return;
+		}
+	}
+
+	if (!m || m->reg_offset == OMAP_MUX_TERMINATOR)
+		pr_err("%s: Could not set mux %s\n",
+			__func__, mux_name);
+}
+
 static struct omap_mux * __init omap_mux_list_add(
 					struct omap_mux_partition *partition,
 					struct omap_mux *src)

arch/arm/mach-omap2/mux.h +10 ~~-13~~

index 79076d6..27ce55a 100644
@@ -190,29 +190,26 @@ u16 omap_mux_get_gpio(int gpio);
 void omap_mux_set_gpio(u16 val, int gpio);
 
 /**
- * omap_mux_get() - get a mux partition by name
- * @name:		Name of the mux partition
+ * omap_mux_get_mux() - set mux register value based on mux name
+ * @mux_name:		Mux name
  *
  */
-struct omap_mux_partition *omap_mux_get(const char *name);
+u16 omap_mux_get_mux(char *mux_name);
 
 /**
- * omap_mux_read() - read mux register
- * @partition:		Mux partition
- * @mux_offset:		Offset of the mux register
+ * omap_mux_set_mux() - set mux register value based on mux name
+ * @val:		New mux register value
+ * @mux_name:		Mux name
  *
  */
-u16 omap_mux_read(struct omap_mux_partition *p, u16 mux_offset);
+void omap_mux_set_mux(u16 val, char *mux_name);
 
 /**
- * omap_mux_write() - write mux register
- * @partition:		Mux partition
- * @val:		New mux register value
- * @mux_offset:		Offset of the mux register
+ * omap_mux_get() - get a mux partition by name
+ * @name:		Name of the mux partition
  *
- * This should be only needed for dynamic remuxing of non-gpio signals.
  */
-void omap_mux_write(struct omap_mux_partition *p, u16 val, u16 mux_offset);
+struct omap_mux_partition *omap_mux_get(const char *name);
 
 /**
  * omap_mux_write_array() - write an array of mux registers

Signature · 1 line

1.7.0.4

Tony Lindgren

maintainer

2010-11-18 17:57 UTC #1 lore raw

Hi Dan,

Quoted 9 lines · Dan Murphy

arch/arm/mach-omap2/mux.c +1 -4

@@ -81,10 +81,14 @@ void omap_mux_write(struct omap_mux_partition *partition, u16 val,
 void omap_mux_write_array(struct omap_mux_partition *partition,
 				 struct omap_board_mux *board_mux)
 {
-	while (board_mux->reg_offset != OMAP_MUX_TERMINATOR) {
-		omap_mux_write(partition, board_mux->value,
-			       board_mux->reg_offset);
-		board_mux++;
+	if (partition) {

arch/arm/mach-omap2/mux.c void omap_mux_write(struct omap_mux_partition *partition, u16 val, · line 94

Quoted 10 lines

arch/arm/mach-omap2/mux.c +7 -0

+		while (board_mux->reg_offset != OMAP_MUX_TERMINATOR) {
+			omap_mux_write(partition, board_mux->value,
+				       board_mux->reg_offset);
+			board_mux++;
+		}
+	} else {
+		pr_err("%s: Partition was NULL\n", __func__);
 	}
 }

Can you please make this into a separate patch. And instead of indenting the code more, just do something like:

	if (!partition)
		return -EINVAL;

Regards,

Tony

Cousson, Benoit

2010-11-18 21:01 UTC #2 lore raw

Hi Tony,

Quoted 1 line · Tony Lindgren

Hi Dan,

[...]

Quoted 5 lines

Can you please make this into a separate patch. And instead of indenting the code more, just do something like:
	if (!partition)
		return -EINVAL;

Do you want me to merge that in my current OMAP4 series branch?

Regards,
Benoit

Tony Lindgren

maintainer

2010-11-18 21:29 UTC #3 lore raw

Quoted 7 lines · Cousson, Benoit

Hi Tony,
Quoted 1 line · Tony Lindgren
Hi Dan,
[...]

Quoted 7 lines

Quoted 5 lines
Can you please make this into a separate patch. And instead of indenting the code more, just do something like:
	if (!partition)
		return -EINVAL;
Do you want me to merge that in my current OMAP4 series branch?

I got that already pulled, so I'd prefer not to touch that branch any longer unless we have to. Sounds like this can be a separate patch.

Regards,

Tony

Tony Lindgren

maintainer

2010-11-19 17:47 UTC #4 lore raw

In reply to Dan Murphy

Quoted 11 lines · Dan Murphy

arch/arm/mach-omap2/mux.c +8 -0

 
+static struct omap_mux *omap_mux_get_by_mux(struct omap_mux_partition *partition,
+					char *name)
+{
+	struct omap_mux_entry *e;
+	int i = 0;
+
+	list_for_each_entry(e, &partition->muxmodes, node) {
+		struct omap_mux *m = &e->mux;

arch/arm/mach-omap2/mux.c

Quoted 10 lines

arch/arm/mach-omap2/mux.c +10 -0

+		for (i = 0; i < OMAP_MUX_NR_MODES; i++) {
+			if (m->muxnames[i] == NULL)
+				break;
+			else if (!strcmp(name, m->muxnames[i]))
+				return m;
+		}
+	}
+
+	return NULL;
+}

Hmm turns out we almost have this already in _omap_mux_init_signal. Also we need to know the mux mode value to make use of this, so how about the patch below instead?

Regards,

Tony

From: Tony Lindgren <[email protected]>
Date: Thu, 18 Nov 2010 18:55:53 -0800
Subject: [PATCH] omap: mux: Add omap_mux_get_by_name

Do this by splitting _omap_mux_init_signal as it already has most of the necessary features.

Based on an earlier patch by Dan Murphy <[email protected]>.

Signed-off-by Tony Lindgren <[email protected]>

arch/arm/mach-omap2/mux.c +31 ~~-24~~

@@ -151,12 +151,14 @@ int __init omap_mux_init_gpio(int gpio, int val)
 	return -ENODEV;
 }
 
-static int __init _omap_mux_init_signal(struct omap_mux_partition *partition,
-					const char *muxname, int val)
+static int __init omap_mux_get_by_name(struct omap_mux_partition *partition,
+					const char *muxname,
+					struct omap_mux **found_mux)
 {
+	struct omap_mux *mux = NULL;
 	struct omap_mux_entry *e;
 	const char *mode_name;
-	int found = 0, mode0_len = 0;
+	int found = 0, mode, mode0_len = 0;
 	struct list_head *muxmodes = &partition->muxmodes;
 
 	mode_name = strchr(muxname, '.');

@@ -168,40 +170,34 @@ static int __init _omap_mux_init_signal(struct omap_mux_partition *partition,
 	}
 
 	list_for_each_entry(e, muxmodes, node) {
-		struct omap_mux *m = &e->mux;
-		char *m0_entry = m->muxnames[0];
+		char *m0_entry;
 		int i;
 
+		mux = &e->mux;
+		m0_entry = mux->muxnames[0];
+
 		/* First check for full name in mode0.muxmode format */
 		if (mode0_len && strncmp(muxname, m0_entry, mode0_len))
 			continue;
 
 		/* Then check for muxmode only */
 		for (i = 0; i < OMAP_MUX_NR_MODES; i++) {
-			char *mode_cur = m->muxnames[i];
+			char *mode_cur = mux->muxnames[i];
 
 			if (!mode_cur)
 				continue;
 
 			if (!strcmp(mode_name, mode_cur)) {
-				u16 old_mode;
-				u16 mux_mode;
-
-				old_mode = omap_mux_read(partition,
-							 m->reg_offset);
-				mux_mode = val | i;
-				pr_debug("%s: Setting signal "
-					 "%s.%s 0x%04x -> 0x%04x\n", __func__,
-					 m0_entry, muxname, old_mode, mux_mode);
-				omap_mux_write(partition, mux_mode,
-					       m->reg_offset);
 				found++;
+				mode = i;
 			}
 		}
 	}
 
-	if (found == 1)
-		return 0;
+	if (found == 1) {
+		*found_mux = mux;
+		return mode;
+	}
 
 	if (found > 1) {
 		pr_err("%s: Multiple signal paths (%i) for %s\n", __func__,

@@ -209,7 +205,7 @@ static int __init _omap_mux_init_signal(struct omap_mux_partition *partition,
 		return -EINVAL;
 	}
 
-	pr_err("%s: Could not set signal %s\n", __func__, muxname);
+	pr_err("%s: Could not find signal %s\n", __func__, muxname);
 
 	return -ENODEV;
 }

@@ -217,12 +213,23 @@ static int __init _omap_mux_init_signal(struct omap_mux_partition *partition,
 int __init omap_mux_init_signal(const char *muxname, int val)
 {
 	struct omap_mux_partition *partition;
-	int ret;
 
 	list_for_each_entry(partition, &mux_partitions, node) {
-		ret = _omap_mux_init_signal(partition, muxname, val);
-		if (!ret)
-			return ret;
+		struct omap_mux *mux = NULL;
+		u16 old_mode;
+		u16 mux_mode;
+
+		mux_mode = omap_mux_get_by_name(partition, muxname, &mux);
+		if (mux_mode < 0)
+			continue;
+
+		old_mode = omap_mux_read(partition, mux->reg_offset);
+		mux_mode |= val;
+		pr_debug("%s: Setting signal %s 0x%04x -> 0x%04x\n",
+			 __func__, muxname, old_mode, mux_mode);
+		omap_mux_write(partition, mux_mode, mux->reg_offset);
+
+		return 0;
 	}
 
 	return -ENODEV;

Tony Lindgren

maintainer

2010-12-02 22:02 UTC #5 lore raw

Quoted 18 lines · Tony Lindgren

Quoted 17 lines · Dan Murphy

arch/arm/mach-omap2/mux.c +14 -0

 
+static struct omap_mux *omap_mux_get_by_mux(struct omap_mux_partition *partition,
+					char *name)
+{
+	struct omap_mux_entry *e;
+	int i = 0;
+
+	list_for_each_entry(e, &partition->muxmodes, node) {
+		struct omap_mux *m = &e->mux;
+		for (i = 0; i < OMAP_MUX_NR_MODES; i++) {
+			if (m->muxnames[i] == NULL)
+				break;
+			else if (!strcmp(name, m->muxnames[i]))
+				return m;
+		}

Quoted 8 lines

arch/arm/mach-omap2/mux.c static int __init _omap_mux_init_signal(struct omap_mux_partition *partition, · line 200
Quoted 4 lines
arch/arm/mach-omap2/mux.c +4 -0
+	}
+
+	return NULL;
+}
Hmm turns out we almost have this already in _omap_mux_init_signal. Also we need to know the mux mode value to make use of this, so how about the patch below instead?

Will post a better version soon.

Tony