[U-Boot] [PATCH v3 2/2] ppc4xx: Add PPC4xx SPI helpers to Sequoia
This patch adds helper routines needed in support of the PPC4xx SPI driver.
Signed-off-by: Steven A. Falco sfalco@harris.com ---
Changed CONFIG constant to CONFIG_PPC4xx_SPI.
board/amcc/sequoia/sequoia.c | 20 ++++++++++++++++++++ include/configs/sequoia.h | 4 ++++ 2 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/board/amcc/sequoia/sequoia.c b/board/amcc/sequoia/sequoia.c index d6668e2..080ee1e 100644 --- a/board/amcc/sequoia/sequoia.c +++ b/board/amcc/sequoia/sequoia.c @@ -26,6 +26,7 @@ #include <libfdt.h> #include <fdt_support.h> #include <ppc4xx.h> +#include <spi.h> #include <asm/gpio.h> #include <asm/processor.h> #include <asm/io.h> @@ -513,3 +514,22 @@ int post_hotkeys_pressed(void) return 0; /* No hotkeys supported */ } #endif /* CONFIG_POST */ + +#if defined(CONFIG_PPC4xx_SPI) +int spi_cs_is_valid(unsigned int bus, unsigned int cs) +{ + return bus == 0 && cs >= 0 && cs < 64; +} + +void spi_cs_activate(struct spi_slave *slave) +{ + /* Assumes chip-selects are active-low. */ + gpio_write_bit(slave->cs, 0); +} + +void spi_cs_deactivate(struct spi_slave *slave) +{ + gpio_write_bit(slave->cs, 1); +} +#endif /* CONFIG_PPC4xx_SPI */ + diff --git a/include/configs/sequoia.h b/include/configs/sequoia.h index 9321bdc..417fda1 100644 --- a/include/configs/sequoia.h +++ b/include/configs/sequoia.h @@ -236,6 +236,10 @@ #define CONFIG_SYS_DTT_LOW_TEMP -30 #define CONFIG_SYS_DTT_HYSTERESIS 3
+/* Define these if you are using the SPI port. */ +#undef CONFIG_HARD_SPI +#undef CONFIG_PPC4xx_SPI + /* * Default environment variables */
Dear "Steven A. Falco",
In message 493E9C46.8000101@harris.com you wrote:
This patch adds helper routines needed in support of the PPC4xx SPI driver.
Signed-off-by: Steven A. Falco sfalco@harris.com
Changed CONFIG constant to CONFIG_PPC4xx_SPI.
board/amcc/sequoia/sequoia.c | 20 ++++++++++++++++++++ include/configs/sequoia.h | 4 ++++ 2 files changed, 24 insertions(+), 0 deletions(-)
Stefan, same as for the SPI driver itself: I vote to put this on hold until there are any actual users of that code.
Best regards,
Wolfgang Denk
Hi Steven,
Steven A. Falco wrote:
This patch adds helper routines needed in support of the PPC4xx SPI driver.
Signed-off-by: Steven A. Falco sfalco@harris.com
Changed CONFIG constant to CONFIG_PPC4xx_SPI.
board/amcc/sequoia/sequoia.c | 20 ++++++++++++++++++++ include/configs/sequoia.h | 4 ++++ 2 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/board/amcc/sequoia/sequoia.c b/board/amcc/sequoia/sequoia.c index d6668e2..080ee1e 100644 --- a/board/amcc/sequoia/sequoia.c +++ b/board/amcc/sequoia/sequoia.c @@ -26,6 +26,7 @@ #include <libfdt.h> #include <fdt_support.h> #include <ppc4xx.h> +#include <spi.h> #include <asm/gpio.h> #include <asm/processor.h> #include <asm/io.h> @@ -513,3 +514,22 @@ int post_hotkeys_pressed(void) return 0; /* No hotkeys supported */ } #endif /* CONFIG_POST */
+#if defined(CONFIG_PPC4xx_SPI) +int spi_cs_is_valid(unsigned int bus, unsigned int cs) +{
- return bus == 0 && cs >= 0 && cs < 64;
+}
+void spi_cs_activate(struct spi_slave *slave) +{
- /* Assumes chip-selects are active-low. */
- gpio_write_bit(slave->cs, 0);
+}
+void spi_cs_deactivate(struct spi_slave *slave) +{
- gpio_write_bit(slave->cs, 1);
+} +#endif /* CONFIG_PPC4xx_SPI */
diff --git a/include/configs/sequoia.h b/include/configs/sequoia.h index 9321bdc..417fda1 100644 --- a/include/configs/sequoia.h +++ b/include/configs/sequoia.h @@ -236,6 +236,10 @@ #define CONFIG_SYS_DTT_LOW_TEMP -30 #define CONFIG_SYS_DTT_HYSTERESIS 3
+/* Define these if you are using the SPI port. */ +#undef CONFIG_HARD_SPI +#undef CONFIG_PPC4xx_SPI
Why not enable this feature on Sequoia? Wolfgang's argument for keeping the patch out then goes away. IMHO, eval boards should have as many options enabled by default as possible, and the user then has the option to opt out.
/*
- Default environment variables
*/
regards, Ben
Ben Warren wrote:
... snip ...
Why not enable this feature on Sequoia? Wolfgang's argument for keeping the patch out then goes away. IMHO, eval boards should have as many options enabled by default as possible, and the user then has the option to opt out.
I can do that if Stefan and Wolfgang find it acceptable. Just let me know.
/*
- Default environment variables
*/
regards, Ben
Dear Ben Warren,
In message 493EA876.7090008@gmail.com you wrote:
Why not enable this feature on Sequoia? Wolfgang's argument for keeping the patch out then goes away. IMHO, eval boards should have as many options enabled by default as possible, and the user then has the option to opt out.
But there is not a single SPI device on the Sequoia board, and if you attach one, you have to write driver code for it that implements the chip select handling and the specific device protocol. We would have a driver included, without any "users" (code that actually calls these functions).
In other words, this driver is a prerequisite for other SPI device drivers that might follow later, but as is, it's just a waste of memory.
It would just waste memory to enable it.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
Dear Ben Warren,
In message 493EA876.7090008@gmail.com you wrote:
Why not enable this feature on Sequoia? Wolfgang's argument for keeping the patch out then goes away. IMHO, eval boards should have as many options enabled by default as possible, and the user then has the option to opt out.
But there is not a single SPI device on the Sequoia board, and if you attach one, you have to write driver code for it that implements the chip select handling and the specific device protocol. We would have a driver included, without any "users" (code that actually calls these functions).
In other words, this driver is a prerequisite for other SPI device drivers that might follow later, but as is, it's just a waste of memory.
It would just waste memory to enable it.
Makes sense.
Best regards,
Wolfgang Denk
The problem is that if it isn't accepted now, nobody is likely to remember that it is available, and so they will have to re-write it. Also, it will suffer from bit-rot, if just left as a loose patch.
Steve
On Tuesday 09 December 2008, Steven A. Falco wrote:
The problem is that if it isn't accepted now, nobody is likely to remember that it is available, and so they will have to re-write it. Also, it will suffer from bit-rot, if just left as a loose patch.
Full ack from me on this.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
Dear Stefan,
In message 200812092012.02201.sr@denx.de you wrote:
On Tuesday 09 December 2008, Steven A. Falco wrote:
The problem is that if it isn't accepted now, nobody is likely to remember that it is available, and so they will have to re-write it. Also, it will suffer from bit-rot, if just left as a loose patch.
Full ack from me on this.
I agree - but what is a dead driver without users good for?
What was it written for? ... obviously for some specific application on some real board. I recommend to submit this driver as part of the patch series that adds this board support to U-Boot. Then we will also have at least one real user of that code, i. e. we can make sure that it at least compiles.
Best regards,
Wolfgang Denk
Dear "Steven A. Falco",
In message 493EC1F8.7080107@harris.com you wrote:
The problem is that if it isn't accepted now, nobody is likely to remember that it is available, and so they will have to re-write it. Also, it will suffer from bit-rot, if just left as a loose patch.
It will bit-rot as well if it's added - if no configuration enables it it will not even be compiled.
I recommend you keep it in your queue, and submit it again with the board that will actually use it.
Best regards,
Wolfgang Denk
Hi Wolfgang,
Wolfgang Denk wrote:
Dear Ben Warren,
In message 493EA876.7090008@gmail.com you wrote:
Why not enable this feature on Sequoia? Wolfgang's argument for keeping the patch out then goes away. IMHO, eval boards should have as many options enabled by default as possible, and the user then has the option to opt out.
But there is not a single SPI device on the Sequoia board, and if you attach one, you have to write driver code for it that implements the chip select handling and the specific device protocol. We would have a driver included, without any "users" (code that actually calls these functions).
Sure. Ignorant assumption on my part that the eval board had something like a SPI EEPROM, but it looks like there's just a header. In that case, the only advantage to including it is to ensure the driver keeps up with any API changes.
In other words, this driver is a prerequisite for other SPI device drivers that might follow later, but as is, it's just a waste of memory.
It would just waste memory to enable it.
OK, but who cares about memory on an evaluation board? Their entire raison-d'etre is to serve as a starting point for custom boards. I know if I was building a board with this CPU and planned on using SPI, it would be much nicer if the driver was included than having to search the message boards. Just my 2c.
Best regards,
Wolfgang Denk
regards, Ben
Ben Warren wrote:
Hi Wolfgang,
Wolfgang Denk wrote:
Dear Ben Warren,
In message 493EA876.7090008@gmail.com you wrote:
Why not enable this feature on Sequoia? Wolfgang's argument for keeping the patch out then goes away. IMHO, eval boards should have as many options enabled by default as possible, and the user then has the option to opt out.
But there is not a single SPI device on the Sequoia board, and if you attach one, you have to write driver code for it that implements the chip select handling and the specific device protocol. We would have a driver included, without any "users" (code that actually calls these functions).
Sure. Ignorant assumption on my part that the eval board had something like a SPI EEPROM, but it looks like there's just a header. In that case, the only advantage to including it is to ensure the driver keeps up with any API changes.
In other words, this driver is a prerequisite for other SPI device drivers that might follow later, but as is, it's just a waste of memory.
It would just waste memory to enable it.
OK, but who cares about memory on an evaluation board? Their entire raison-d'etre is to serve as a starting point for custom boards. I know if I was building a board with this CPU and planned on using SPI, it would be much nicer if the driver was included than having to search the message boards. Just my 2c.
I agree with this position. I use U-Boot for two purposes - booting a kernel, and experimenting with hardware. Given that I am currently in the design phase of a project, I am prototyping things on an evaluation board, to see what works and what doesn't. Having U-Boot support all the interfaces of a SOC like the PPC440EPx is very useful to me. Much easier and quicker to do prototyping in U-Boot than in Linux.
But again, you guys can decide the philosophical issues. I'm just the code monkey...
Steve
Best regards,
Wolfgang Denk
regards, Ben
participants (4)
-
Ben Warren -
Stefan Roese -
Steven A. Falco -
Wolfgang Denk