[U-Boot-Users] [PATCH 3/3] QE IO - Add pario command

[U-Boot-Users] [PATCH 3/3] QE IO - Add pario command

[David Saada]

This patch fragment includes commands for reading and writing parallel
I/O ports (pario command). 

Signed-off-by: David Saada <david.saada@ecitele.com>

common/cmd_pario.c |   85
+++++++++++++++++++++++++++++++++++++++++++++++++++
common/Makefile      |    1 
2 files changed, 86 insertions(+)
create mode 100644 common/cmd_pario.c

--- a/common/Makefile	2008-03-28 01:49:12.000000000 +0200
+++ b/common/Makefile	2008-03-30 15:59:57.944754000 +0300
@@ -81,6 +81,7 @@ COBJS-$(CONFIG_CMD_NET) += cmd_net.o
 COBJS-y += cmd_nvedit.o
 COBJS-y += cmd_onenand.o
 COBJS-$(CONFIG_CMD_OTP) += cmd_otp.o
+COBJS-$(CONFIG_CMD_PARIO) += cmd_pario.o
 ifdef CONFIG_PCI
 COBJS-$(CONFIG_CMD_PCI) += cmd_pci.o
 endif
0a1,85
--- /dev/null	2008-03-30 10:13:32.378222985 +0300
+++ b/common/cmd_pario.c	2008-03-30 16:00:43.124433000 +0300
@@ -0,0 +1,85 @@
+/*
+ * Copyright 2008 ECI Telecommunication.
+ *
+ * (C) Copyright 2008 David Saada <david.saada@ecitele.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <command.h>
+#include <ioports.h>
+
+int do_pario (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
+{
+	char op;
+	int	port, pin, data;
+	static char	last_op;
+	static int last_port, last_pin, last_data;
+
+	/*
+	 * We use the last specified parameters, unless new ones are
+	 * entered.
+	 */
+	op = last_op;
+	port = last_port;
+	pin = last_pin;
+	data = last_data;
+
+	if ((flag & CMD_FLAG_REPEAT) == 0) {
+		op = argv[1][0];
+
+		if (argc >= 3)
+			port = simple_strtoul (argv[2], NULL, 10);
+		if (argc >= 4)
+			pin = simple_strtoul (argv[3], NULL, 10);
+		if (argc >= 5)
+			data = simple_strtoul (argv[4], NULL, 10);
+	}
+
+	if (op == 'r') {
+		qe_read_iopin(port ,pin ,&data);
+		printf("%d\n", data);
+	} else if (op == 'w') {
+		qe_write_iopin(port ,pin ,data);
+	} else {
+		printf("Usage:\n%s\n", cmdtp->usage);
+		return 1;
+	}
+
+	/*
+	 * Save the parameters for repeats.
+	 */
+	last_op = op;
+	last_port = port;
+	last_pin = pin;
+	last_data = data;
+
+	return 0;
+}
+
+/***************************************************/
+
+U_BOOT_CMD(
+	pario,	5,	1,	do_pario,
+	"pario     - Parallel I/O utility commands\n",
+	"read   <port> <pin>        - read from port <port> pin <pin>\n"
+	"pario write  <port> <pin> <data> - write to port  <port> pin
<pin>\n"
+);
+

[U-Boot-Users] [PATCH 3/3] QE IO - Add pario command

[Wolfgang Denk]

In message <B27D27F93BC429468DBC3B0DA043AA4401FF257D@ILPTEX02.ecitele.com> you wrote:
> This patch fragment includes commands for reading and writing parallel
> I/O ports (pario command). 

This sounds line a very general feature, but having a close look it
seems that it is highly specialized on QE only.

As such, it does not qualify for such a generic command name.

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
Our OS who art in CPU, UNIX be thy name.
Thy programs run, thy syscalls done,
In kernel as it is in user!

[U-Boot-Users] [PATCH 3/3] QE IO - Add pario command

[David Saada]

> This sounds line a very general feature, but having a close look it
> seems that it is highly specialized on QE only.

> As such, it does not qualify for such a generic command name.


Wolfgang,
It is indeed a general feature, whose first implementation is for the
QE. It can (and should) be extended for other CPU types. I wanted to add
the CPM2 I/O setup, but I don't have a board to check this.
If desired I can wrap the QE related calls with #ifdef QE...

David.

[U-Boot-Users] [PATCH 3/3] QE IO - Add pario command

[Wolfgang Denk]

In message <B27D27F93BC429468DBC3B0DA043AA4401FF264C@ILPTEX02.ecitele.com> you wrote:
> 
> It is indeed a general feature, whose first implementation is for the
> QE. It can (and should) be extended for other CPU types. I wanted to add
> the CPM2 I/O setup, but I don't have a board to check this.
> If desired I can wrap the QE related calls with #ifdef QE...

I don't think is a good idea. The code as presented is  actually  not
suited  for such an extension. And I definitly don't want to plan for
another #ifdef maze.

Are you aware of the "reginfo" command? Few people seem to know it.

Maybe you should add such code there.

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
When choosing between two evils, I always like to take the  one  I've
never tried before.                     -- Mae West, "Klondike Annie"

[U-Boot-Users] [PATCH 3/3] QE IO - Add pario command

[David Saada]

> I don't think is a good idea. The code as presented is  actually  not
> suited  for such an extension. And I definitly don't want to plan for
> another #ifdef maze.

> Are you aware of the "reginfo" command? Few people seem to know it.

> Maybe you should add such code there.

As far as I managed to understand, reginfo just shows a dump of CPU
based register values, while pario allows you to set and get the value
of a GPIO port - two different things. We found this very useful for
debugging our board, and I think it can be useful for many other
processor families as well. 
David.

[U-Boot-Users] [PATCH 3/3] QE IO - Add pario command

[Timur Tabi]

David Saada wrote:

> As far as I managed to understand, reginfo just shows a dump of CPU
> based register values, while pario allows you to set and get the value
> of a GPIO port - two different things. We found this very useful for
> debugging our board, and I think it can be useful for many other
> processor families as well. 

There is also a "qe" command that takes sub-commands as a parameter.  Currently,
the only one is "qe fw" for uploading firmware.  Perhaps you could do "qe pario"?

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot-Users] [PATCH 3/3] QE IO - Add pario command

[Wolfgang Denk]

In message <B27D27F93BC429468DBC3B0DA043AA4401FF2660@ILPTEX02.ecitele.com> you wrote:
> 
> > I don't think is a good idea. The code as presented is  actually  not
> > suited  for such an extension. And I definitly don't want to plan for
> > another #ifdef maze.
> 
> > Are you aware of the "reginfo" command? Few people seem to know it.
> 
> > Maybe you should add such code there.
> 
> As far as I managed to understand, reginfo just shows a dump of CPU
> based register values, while pario allows you to set and get the value
> of a GPIO port - two different things. We found this very useful for
> debugging our board, and I think it can be useful for many other
> processor families as well. 

I'm aware of this. Changing register contents seems a useful extension
to me, too. That's why I wrote "add such code". I think something like

Print all (or a predefined set of) registers:
	=> reg

Print a specific register:
	=> reg name

Set a specific register to "value":
	=> reg name value

would be generally useful, not only for parallel I/O registers.

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
"If it ain't broke, don't fix it."                       - Bert Lantz

[U-Boot-Users] [PATCH 3/3] QE IO - Add pario command

[Wolfgang Denk]

In message <47F11124.8070705@freescale.com> you wrote:
> 
> > As far as I managed to understand, reginfo just shows a dump of CPU
> > based register values, while pario allows you to set and get the value
> > of a GPIO port - two different things. We found this very useful for
> > debugging our board, and I think it can be useful for many other
> > processor families as well. 
> 
> There is also a "qe" command that takes sub-commands as a parameter.  Currently,
> the only one is "qe fw" for uploading firmware.  Perhaps you could do "qe pario"?

Please don;t.

Instead of adding such functionality several times in several  places
distributed all over the whole source tree, let's do it just once and
right.

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
Even historians fail to learn from history -- they repeat the same
mistakes.
	-- John Gill, "Patterns of Force", stardate 2534.7

[U-Boot-Users] [PATCH 3/3] QE IO - Add pario command

[David Saada]

> I'm aware of this. Changing register contents seems a useful extension
> to me, too. That's why I wrote "add such code". I think something like

> Print all (or a predefined set of) registers:

 <	=> reg

> Print a specific register:
< 	=> reg name

>Set a specific register to "value":
>	=> reg name value

> would be generally useful, not only for parallel I/O registers.

Specifying parallel I/O ports has whole registers is really
uncomfortable: If for example one would like to write a value to a
parallel I/O port, then he'd need to read the data register first, then
mask off all irrelevant bits, and then write the shifted value to this
register. The pario command does this job in a much more comfortable
way.
Again - we can keep this command in our boards' common code, but I think
it would be a pity, as I think this functionality can be useful for many
other CPU types as well.
David.

[U-Boot-Users] [PATCH 3/3] QE IO - Add pario command

[Wolfgang Denk]

In message <B27D27F93BC429468DBC3B0DA043AA4401FF270D@ILPTEX02.ecitele.com> you wrote:
> > I'm aware of this. Changing register contents seems a useful extension
> > to me, too. That's why I wrote "add such code". I think something like
> 
> > Print all (or a predefined set of) registers:
> 
>  <	=> reg
> 
> > Print a specific register:
> < 	=> reg name
> 
> >Set a specific register to "value":
> >	=> reg name value
> 
> > would be generally useful, not only for parallel I/O registers.
> 
> Specifying parallel I/O ports has whole registers is really
> uncomfortable: If for example one would like to write a value to a
> parallel I/O port, then he'd need to read the data register first, then
> mask off all irrelevant bits, and then write the shifted value to this
> register. The pario command does this job in a much more comfortable
> way.

Nothing prevents you to write your code such that  "name"  is  not  a
register  name  but  a  parallel  port  name,  and that the code that
handles it does exactly what you describe above transparently for the
user.

> Again - we can keep this command in our boards' common code, but I think
> it would be a pity, as I think this functionality can be useful for many
> other CPU types as well.

Agreed.

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
The most exciting phrase to hear in science, the one that heralds new
discoveries, is not "Eureka!" (I found it!) but "That's funny ..."
                                                      -- Isaac Asimov