[U-Boot-Users] [PATCH 3/3] QE IO - Add pario command
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" +); +
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
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.
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
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.
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"?
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
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
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.
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
participants (3)
-
David Saada -
Timur Tabi -
Wolfgang Denk