[U-Boot] [PATCH v3 2/2] ppc4xx: Add PPC4xx SPI helpers to Sequoia

[U-Boot] [PATCH v3 2/2] ppc4xx: Add PPC4xx SPI helpers to Sequoia

[Steven A. Falco]

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
  */
-- 
1.6.0.2

[U-Boot] [PATCH v3 2/2] ppc4xx: Add PPC4xx SPI helpers to Sequoia

[Wolfgang Denk]

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

-- 
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 ignorance is bliss, why aren't there more happy people?

[U-Boot] [PATCH v3 2/2] ppc4xx: Add PPC4xx SPI helpers to Sequoia

[Ben Warren]

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

[U-Boot] [PATCH v3 2/2] ppc4xx: Add PPC4xx SPI helpers to Sequoia

[Steven A. Falco]

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
> 

[U-Boot] [PATCH v3 2/2] ppc4xx: Add PPC4xx SPI helpers to Sequoia

[Wolfgang Denk]

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

-- 
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
Women are more easily and more deeply terrified ...  generating  more
sheer horror than the male of the species.
	-- Spock, "Wolf in the Fold", stardate 3615.4

[U-Boot] [PATCH v3 2/2] ppc4xx: Add PPC4xx SPI helpers to Sequoia

[Steven A. Falco]

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

[U-Boot] [PATCH v3 2/2] ppc4xx: Add PPC4xx SPI helpers to Sequoia

[Ben Warren]

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

[U-Boot] [PATCH v3 2/2] ppc4xx: Add PPC4xx SPI helpers to Sequoia

[Stefan Roese]

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 at denx.de
=====================================================================

[U-Boot] [PATCH v3 2/2] ppc4xx: Add PPC4xx SPI helpers to Sequoia

[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

-- 
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
Every program has at least one bug and can be shortened by  at  least
one instruction - from which, by induction, one can deduce that every
program can be reduced to one instruction which doesn't work.

[U-Boot] [PATCH v3 2/2] ppc4xx: Add PPC4xx SPI helpers to Sequoia

[Wolfgang Denk]

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

-- 
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 value of marriage is not that adults produce children, but  that
children produce adults."                            - Peter De Vries

[U-Boot] [PATCH v3 2/2] ppc4xx: Add PPC4xx SPI helpers to Sequoia

[Steven A. Falco]

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
>