[U-Boot] [PATCH] Davinci: add a pin multiplexer configuration API

[U-Boot] [PATCH] Davinci: add a pin multiplexer configuration API

[Nick Thompson]

Davinci: add a pin multiplexer configuration API.

Creates a method allowing pin settings to be logically grouped into data
structure arrays and provids an API to configure the PINMUX settings to
enable the relevant pin functions.

Signed-off-by: Nick Thompson <nick.thompson@gefanuc.com>
---
Applies to: u-boot-ti

The PINMUXn definitions are already present in hardware.h.

The number of PINMUX fields per register and the width of the fields
needs to be set per SoC. The initial settings are appropriate for at least
DA8xx devices. These should be modified in misc.h to support other devices
as required.

 board/davinci/common/misc.c |   46 ++++++++++++++++++++++++++++++++++++++++++-
 board/davinci/common/misc.h |   12 +++++++++++
 2 files changed, 57 insertions(+), 1 deletions(-)

diff --git a/board/davinci/common/misc.c b/board/davinci/common/misc.c
index ffdc20b..d462d65 100644
--- a/board/davinci/common/misc.c
+++ b/board/davinci/common/misc.c
@@ -1,6 +1,7 @@
 /*
  * Miscelaneous DaVinci functions.
  *
+ * Copyright (C) 2009 Nick Thompson, GE Fanuc Ltd, <nick.thompson@gefanuc.com>
  * Copyright (C) 2007 Sergey Kubushyn <ksi@koi8.net>
  * Copyright (C) 2008 Lyrtech <www.lyrtech.com>
  * Copyright (C) 2004 Texas Instruments.
@@ -27,7 +28,8 @@
 #include <i2c.h>
 #include <net.h>
 #include <asm/arch/hardware.h>
-
+#include <asm/io.h>
+#include "misc.h"

 DECLARE_GLOBAL_DATA_PTR;

@@ -109,3 +111,45 @@ void dv_configure_mac_address(uint8_t *rom_enetaddr)
 }

 #endif	/* DAVINCI_EMAC */
+
+/*
+ * Change the setting of a pin multiplexer field.
+ *
+ * Takes an array of pinmux settings similar to:
+ *
+ * struct pinmux_config uart_pins[] = {
+ *	{ PINMUX8, 2, 7 },
+ *	{ PINMUX9, 2, 0 }
+ * };
+ *
+ * Stepping through the array, each PINMUXn register has the given value
+ * set in the pin mux field specified.
+ *
+ * The number of pins in the array must be passed (ARRAY_SIZE can provide
+ * this value conveniently).
+ *
+ * Returns 0 if all field numbers and values are in the correct range,
+ * else returns -1.
+ */
+int davinci_configure_pin_mux(const struct pinmux_config *pins, int n_pins)
+{
+	int i;
+
+	for (i = 0; i < n_pins; i++) {
+		int value = pins[i].value;
+		int field = pins[i].field;
+
+		if (field < PIN_MUX_NUM_FIELDS &&
+			(value & ~PIN_MUX_FIELD_MASK) == 0) {
+			int offset = field * PIN_MUX_FIELD_SIZE;
+			unsigned int mux = pins[i].mux;
+			unsigned int mask = PIN_MUX_FIELD_MASK << offset;
+			value <<= offset;
+			writel(value | (readl(mux) & (~mask)), mux);
+		} else {
+			return -1;
+		}
+	}
+
+	return 0;
+}
diff --git a/board/davinci/common/misc.h b/board/davinci/common/misc.h
index dc3cc41..f6d8b1b 100644
--- a/board/davinci/common/misc.h
+++ b/board/davinci/common/misc.h
@@ -22,8 +22,20 @@
 #ifndef __MISC_H
 #define __MISC_H

+/* pin muxer definitions */
+#define PIN_MUX_NUM_FIELDS	8	/* Per register */
+#define PIN_MUX_FIELD_SIZE	4	/* n in bits */
+#define PIN_MUX_FIELD_MASK	((1 << PIN_MUX_FIELD_SIZE) - 1)
+
+/* pin definition */
+struct pinmux_config {
+	dv_reg		*mux;		/* Address of mux register */
+	unsigned char	value;		/* Value to set in field */
+	unsigned char	field;		/* field number */
+};

 int dvevm_read_mac_address(uint8_t *buf);
 void dv_configure_mac_address(uint8_t *rom_enetaddr);
+int davinci_configure_pin_mux(const struct pinmux_config *pins, int n_pins);

 #endif /* __MISC_H */

[U-Boot] [PATCH] Davinci: add a pin multiplexer configuration API

[Kim Phillips]

On Thu, 29 Oct 2009 15:38:18 +0000
Nick Thompson <nick.thompson@gefanuc.com> wrote:

> +int davinci_configure_pin_mux(const struct pinmux_config *pins, int n_pins)
> +{
> +	int i;
> +
> +	for (i = 0; i < n_pins; i++) {
> +		int value = pins[i].value;
> +		int field = pins[i].field;
> +
> +		if (field < PIN_MUX_NUM_FIELDS &&
> +			(value & ~PIN_MUX_FIELD_MASK) == 0) {

the second line should not be indented as though it is the code
subblock; it should fall directly underneath the column where 'field
<..' starts, like this:

if (field < PIN_MUX_NUM_FIELDS &&
    (value & ~PIN_MUX_FIELD_MASK) == 0) {

> +			int offset = field * PIN_MUX_FIELD_SIZE;
> +			unsigned int mux = pins[i].mux;
> +			unsigned int mask = PIN_MUX_FIELD_MASK << offset;

also please just declare everything@the top of the function - same
for value and field declarations above.

> +			value <<= offset;
> +			writel(value | (readl(mux) & (~mask)), mux);

I guess arm doesn't have setbits32 and friends, huh.

Kim

[U-Boot] [PATCH] Davinci: add a pin multiplexer configuration API

[Wolfgang Denk]

Dear Kim Phillips,

In message <20091029182304.469c9f7f.kim.phillips@freescale.com> you wrote:
>
> > +		if (field < PIN_MUX_NUM_FIELDS &&
> > +			(value & ~PIN_MUX_FIELD_MASK) == 0) {
> 
> the second line should not be indented as though it is the code
> subblock; it should fall directly underneath the column where 'field
> <..' starts, like this:

Agreed.

> if (field < PIN_MUX_NUM_FIELDS &&
>     (value & ~PIN_MUX_FIELD_MASK) == 0) {
> 
> > +			int offset = field * PIN_MUX_FIELD_SIZE;
> > +			unsigned int mux = pins[i].mux;
> > +			unsigned int mask = PIN_MUX_FIELD_MASK << offset;
> 
> also please just declare everything at the top of the function - same
> for value and field declarations above.

No! Why should that be needed? It would be just a waste of stack
space (except that recent compilers don't care abouyt this anyway),
and keeping variables as localized as possible seems to be a good
thing to me.

> > +			value <<= offset;
> > +			writel(value | (readl(mux) & (~mask)), mux);
> 
> I guess arm doesn't have setbits32 and friends, huh.

Not yet. Patches welcome!

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Evil does seek to maintain power by suppressing the truth."
"Or by misleading the innocent."
	-- Spock and McCoy, "And The Children Shall Lead",
	   stardate 5029.5.

[U-Boot] [PATCH] Davinci: add a pin multiplexer configuration API

[Kim Phillips]

On Fri, 30 Oct 2009 09:26:12 +0100
Wolfgang Denk <wd@denx.de> wrote:

> Dear Kim Phillips,
> 
> In message <20091029182304.469c9f7f.kim.phillips@freescale.com> you wrote:
> > if (field < PIN_MUX_NUM_FIELDS &&
> >     (value & ~PIN_MUX_FIELD_MASK) == 0) {
> > 
> > > +			int offset = field * PIN_MUX_FIELD_SIZE;
> > > +			unsigned int mux = pins[i].mux;
> > > +			unsigned int mask = PIN_MUX_FIELD_MASK << offset;
> > 
> > also please just declare everything at the top of the function - same
> > for value and field declarations above.
> 
> No! Why should that be needed? It would be just a waste of stack
> space (except that recent compilers don't care abouyt this anyway),
> and keeping variables as localized as possible seems to be a good
> thing to me.

not when it sacrifices readability.  I'm looking for assignments and
finding 'unsigned'!  Either put a blank line between the declarations
and the rest of the code, or declare everything at the top of the
function, since it does nothing for the compiler (my preference is the
latter in this case).

Kim

[U-Boot] [PATCH] Davinci: add a pin multiplexer configuration API

[Thompson, Nick (GE EntSol, Intelligent Platforms)]

From: Kim Phillips [mailto:kim.phillips at freescale.com]
Sent: 30 October 2009 14:57
> On Fri, 30 Oct 2009 09:26:12 +0100
> Wolfgang Denk <wd@denx.de> wrote:
> 
> > Dear Kim Phillips,
> >
> > In message <20091029182304.469c9f7f.kim.phillips@freescale.com> you wrote:
> > > if (field < PIN_MUX_NUM_FIELDS &&
> > >     (value & ~PIN_MUX_FIELD_MASK) == 0) {
> > >
> > > > +			int offset = field * PIN_MUX_FIELD_SIZE;
> > > > +			unsigned int mux = pins[i].mux;
> > > > +			unsigned int mask = PIN_MUX_FIELD_MASK << offset;
> > >
> > > also please just declare everything at the top of the function - same
> > > for value and field declarations above.
> >
> > No! Why should that be needed? It would be just a waste of stack
> > space (except that recent compilers don't care abouyt this anyway),
> > and keeping variables as localized as possible seems to be a good
> > thing to me.
> 
> not when it sacrifices readability.  I'm looking for assignments and
> finding 'unsigned'!  Either put a blank line between the declarations
> and the rest of the code, or declare everything at the top of the
> function, since it does nothing for the compiler (my preference is the
> latter in this case).
> 
> Kim

All three of these declarations could be const, which may or may not
help the compiler, but would be technically correct and document my
intentions better.

This would not be possible if they where moved to the start of the
function or if the declaration and assignments where split up in any
other way.

Nick.

[U-Boot] [PATCH] Davinci: add a pin multiplexer configuration API

[Kim Phillips]

On Fri, 30 Oct 2009 15:17:40 +0000
"Thompson, Nick (GE EntSol, Intelligent Platforms)" <Nick.Thompson@gefanuc.com> wrote:

> 
> From: Kim Phillips [mailto:kim.phillips at freescale.com]
> Sent: 30 October 2009 14:57
> > On Fri, 30 Oct 2009 09:26:12 +0100
> > Wolfgang Denk <wd@denx.de> wrote:
> > 
> > > Dear Kim Phillips,
> > >
> > > In message <20091029182304.469c9f7f.kim.phillips@freescale.com> you wrote:
> > > > if (field < PIN_MUX_NUM_FIELDS &&
> > > >     (value & ~PIN_MUX_FIELD_MASK) == 0) {
> > > >
> > > > > +			int offset = field * PIN_MUX_FIELD_SIZE;
> > > > > +			unsigned int mux = pins[i].mux;
> > > > > +			unsigned int mask = PIN_MUX_FIELD_MASK << offset;
> > > >
> > > > also please just declare everything at the top of the function - same
> > > > for value and field declarations above.
> > >
> > > No! Why should that be needed? It would be just a waste of stack
> > > space (except that recent compilers don't care abouyt this anyway),
> > > and keeping variables as localized as possible seems to be a good
> > > thing to me.
> > 
> > not when it sacrifices readability.  I'm looking for assignments and
> > finding 'unsigned'!  Either put a blank line between the declarations
> > and the rest of the code, or declare everything at the top of the
> > function, since it does nothing for the compiler (my preference is the
> > latter in this case).
> > 
> > Kim
> 
> All three of these declarations could be const, which may or may not
> help the compiler, but would be technically correct and document my
> intentions better.
> 
> This would not be possible if they where moved to the start of the
> function or if the declaration and assignments where split up in any
> other way.

Even with a blank line?

Either comply to the level of compiler effectiveness you seek, or make
the code more readable for the rest of us.

Kim