public inbox for ecos-patches@sourceware.org
 help / color / mirror / Atom feed
* SPI lpc2xxx patch
@ 2009-01-27  9:57 Sergei Gavrikov
  2009-02-17  0:54 ` Jonathan Larmour
  0 siblings, 1 reply; 6+ messages in thread
From: Sergei Gavrikov @ 2009-01-27  9:57 UTC (permalink / raw)
  To: eCos patches list

[-- Attachment #1: Type: text/plain, Size: 279 bytes --]

Hi

I never try SPI driver for lpc2xxx before, today I did and found an
annoyance copy & paste typo in spi_lpc2xxx_set_config(). Also, to be
ensure the SPI interrupts are using different, was entered a priority
selector, like that was done for the lpc2xxx serial driver.

Sergei

[-- Attachment #2: devs_spi_arm_lpc2xxx.patch --]
[-- Type: text/x-diff, Size: 6486 bytes --]

Index: lpc2xxx/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/spi/arm/lpc2xxx/current/ChangeLog,v
retrieving revision 1.1
diff -U5 -r1.1 ChangeLog
--- lpc2xxx/current/ChangeLog	12 Jul 2008 15:56:10 -0000	1.1
+++ lpc2xxx/current/ChangeLog	27 Jan 2009 09:47:01 -0000
@@ -1,5 +1,13 @@
+2009-01-27  Sergei Gavrikov  <sergei.gavrikov@gmail.com>
+
+	* cdl/spi_lpc2xxx.cdl: Ensure the SPI interrupts are using different
+	priorities: CYGNUM_IO_SPI_ARM_LPC2XXX_SPI{0,1}_INTPRIO entered.
+	* include/spi_lpc2xxx.h: cyg_spi_lpc2xxx_bus_t: spi_prio field added.
+	* src/spi_lpc2xxx.cxx: spi_lpc2xxx_set_config(): fixed copy & paste
+	typo, spi_lpc2xxx_init_bus(): added 'prio' argument to initializer.
+
 2007-07-12  Hans Rosenfeld  <rosenfeld@grumpf.hope-2000.org>
 
 	* lpc2xxx: driver for on-chip SPI units
 
 //===========================================================================
Index: lpc2xxx/current/cdl/spi_lpc2xxx.cdl
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/spi/arm/lpc2xxx/current/cdl/spi_lpc2xxx.cdl,v
retrieving revision 1.1
diff -U5 -r1.1 spi_lpc2xxx.cdl
--- lpc2xxx/current/cdl/spi_lpc2xxx.cdl	12 Jul 2008 15:56:10 -0000	1.1
+++ lpc2xxx/current/cdl/spi_lpc2xxx.cdl	27 Jan 2009 09:47:02 -0000
@@ -61,13 +61,41 @@
         default_value 1
         description   "The LPC2xxx controllers contain two SPI interfaces.
                        Enable this option to get support for SPI interface 0."
     }
 
+    cdl_option CYGNUM_IO_SPI_ARM_LPC2XXX_SPI0_INTPRIO {
+         display "Interrupt priority of the SPI bus 0 ISR"
+         flavor  data
+         legal_values 0 to 15
+         default_value 12
+         requires { is_active(CYGNUM_IO_SPI_ARM_LPC2XXX_SPI1_INTPRIO)
+           implies CYGNUM_IO_SPI_ARM_LPC2XXX_SPI0_INTPRIO !=
+                   CYGNUM_IO_SPI_ARM_LPC2XXX_SPI1_INTPRIO 
+         }
+         description "
+             This option specifies the interrupt priority of the ISR of
+             the SPI bus 0 interrupt in the VIC. Slot 0 has the highest
+             priority and slot 15 the lowest."
+    }
+
     cdl_option CYGPKG_DEVS_SPI_ARM_LPC2XXX_BUS1 {
         display       "Enable SPI interface 1"
         flavor        bool
         default_value 1
         description   "The LPC2xxx controllers contain two SPI interfaces.
                        Enable this option to get support for SPI interface 1."
     }
-}
\ No newline at end of file
+
+    cdl_option CYGNUM_IO_SPI_ARM_LPC2XXX_SPI1_INTPRIO {
+         display "Interrupt priority of the SPI bus 1 ISR"
+         flavor  data
+         legal_values 0 to 15
+         default_value 13
+         description "
+             This option specifies the interrupt priority of the ISR of
+             the SPI bus 1 interrupt in the VIC. Slot 0 has the highest
+             priority and slot 15 the lowest."
+    }
+
+}
+
Index: lpc2xxx/current/include/spi_lpc2xxx.h
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/spi/arm/lpc2xxx/current/include/spi_lpc2xxx.h,v
retrieving revision 1.1
diff -U5 -r1.1 spi_lpc2xxx.h
--- lpc2xxx/current/include/spi_lpc2xxx.h	12 Jul 2008 15:56:10 -0000	1.1
+++ lpc2xxx/current/include/spi_lpc2xxx.h	27 Jan 2009 09:47:02 -0000
@@ -71,10 +71,11 @@
   cyg_spi_bus     spi_bus;
   
   cyg_interrupt   spi_intr;
   cyg_handle_t    spi_hand;
   cyg_vector_t    spi_vect;
+  cyg_priority_t  spi_prio;
   cyg_drv_mutex_t spi_lock;
   cyg_drv_cond_t  spi_wait;
   
   struct spi_dev *spi_dev;
   
Index: lpc2xxx/current/src/spi_lpc2xxx.cxx
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/spi/arm/lpc2xxx/current/src/spi_lpc2xxx.cxx,v
retrieving revision 1.1
diff -U5 -r1.1 spi_lpc2xxx.cxx
--- lpc2xxx/current/src/spi_lpc2xxx.cxx	12 Jul 2008 15:56:10 -0000	1.1
+++ lpc2xxx/current/src/spi_lpc2xxx.cxx	27 Jan 2009 09:47:02 -0000
@@ -179,11 +179,11 @@
                        cyg_uint32 *len)
 {
   cyg_spi_lpc2xxx_dev_t *dev = (cyg_spi_lpc2xxx_dev_t *) device;
   
   switch(key) {
-    case CYG_IO_GET_CONFIG_SPI_CLOCKRATE:
+    case CYG_IO_SET_CONFIG_SPI_CLOCKRATE:
       if(*len == sizeof(cyg_uint32)) {
         dev->spi_baud = * (cyg_uint32 *) buf;
         spi_lpc2xxx_baud((cyg_spi_lpc2xxx_bus_t *) dev->spi_device.spi_bus, 
                          dev->spi_baud);
       }
@@ -303,11 +303,12 @@
  * Driver & bus initialization
  */
 static void 
 spi_lpc2xxx_init_bus(cyg_spi_lpc2xxx_bus_t *bus, 
                      cyg_addrword_t dev,
-                     cyg_vector_t vec)
+                     cyg_vector_t vec,
+                     cyg_priority_t prio)
 {
   bus->spi_bus.spi_transaction_begin    = spi_lpc2xxx_begin;
   bus->spi_bus.spi_transaction_transfer = spi_lpc2xxx_transfer;
   bus->spi_bus.spi_transaction_tick     = spi_lpc2xxx_tick;
   bus->spi_bus.spi_transaction_end      = spi_lpc2xxx_end;
@@ -318,12 +319,13 @@
   cyg_drv_mutex_init(&bus->spi_lock);
   cyg_drv_cond_init(&bus->spi_wait, &bus->spi_lock);
   
   bus->spi_dev = (struct spi_dev *) dev;
   bus->spi_vect = vec;
+  bus->spi_prio = prio;
   cyg_drv_interrupt_create(
-                           vec, 0, (cyg_addrword_t) bus,
+                           vec, prio, (cyg_addrword_t) bus,
                            &spi_lpc2xxx_isr, &spi_lpc2xxx_dsr,
                            &bus->spi_hand, &bus->spi_intr);
   cyg_drv_interrupt_attach(bus->spi_hand);
 }
 
@@ -342,21 +344,23 @@
     tmp |= 0x5500;
     HAL_WRITE_UINT32(addr, tmp);
     
     spi_lpc2xxx_init_bus(&cyg_spi_lpc2xxx_bus0,
                          CYGARC_HAL_LPC2XXX_REG_SPI0_BASE,
-                         CYGNUM_HAL_INTERRUPT_SPI0);
+                         CYGNUM_HAL_INTERRUPT_SPI0,
+                         CYGNUM_IO_SPI_ARM_LPC2XXX_SPI0_INTPRIO);
 #endif
 #ifdef CYGPKG_DEVS_SPI_ARM_LPC2XXX_BUS1
     addr = (CYGARC_HAL_LPC2XXX_REG_PIN_BASE
             + CYGARC_HAL_LPC2XXX_REG_PINSEL1);
     HAL_READ_UINT32(addr, tmp);
     tmp |= 0x2a8;
     HAL_WRITE_UINT32(addr, tmp);
     spi_lpc2xxx_init_bus(&cyg_spi_lpc2xxx_bus1,
                          CYGARC_HAL_LPC2XXX_REG_SPI1_BASE,
-                         CYGNUM_HAL_INTERRUPT_SPI1);
+                         CYGNUM_HAL_INTERRUPT_SPI1,
+                         CYGNUM_IO_SPI_ARM_LPC2XXX_SPI1_INTPRIO);
 #endif
   }
 };
 
 static cyg_spi_lpc2xxx_init_class spi_lpc2xxx_init 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: SPI lpc2xxx patch
  2009-01-27  9:57 SPI lpc2xxx patch Sergei Gavrikov
@ 2009-02-17  0:54 ` Jonathan Larmour
  2009-02-17  7:38   ` Sergei Gavrikov
  0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Larmour @ 2009-02-17  0:54 UTC (permalink / raw)
  To: Sergei Gavrikov; +Cc: eCos patches list

Sergei Gavrikov wrote:
> Hi
> 
> I never try SPI driver for lpc2xxx before, today I did and found an
> annoyance copy & paste typo in spi_lpc2xxx_set_config(). Also, to be
> ensure the SPI interrupts are using different, was entered a priority
> selector, like that was done for the lpc2xxx serial driver.

Thanks for the patch. I've checked it in with some minor mods, primarily 
placing the new options under the relevant bus options, renaming them 
consistently with that, and making the bus options components.

Jifl
-- 
*See us at Embedded World 2009, Nürnberg, Germany, 3-5 Mar, Stand 11-300*
eCosCentric Limited      http://www.eCosCentric.com/     The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
------["Si fractum non sit, noli id reficere"]------       Opinions==mine

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: SPI lpc2xxx patch
  2009-02-17  0:54 ` Jonathan Larmour
@ 2009-02-17  7:38   ` Sergei Gavrikov
  2009-02-17  9:09     ` Jonathan Larmour
  0 siblings, 1 reply; 6+ messages in thread
From: Sergei Gavrikov @ 2009-02-17  7:38 UTC (permalink / raw)
  To: Jonathan Larmour; +Cc: eCos patches list

[-- Attachment #1: Type: text/plain, Size: 846 bytes --]

On Tue, Feb 17, 2009 at 12:54:40AM +0000, Jonathan Larmour wrote:
> Sergei Gavrikov wrote:
> >Hi
> >
> >I never try SPI driver for lpc2xxx before, today I did and found an
> >annoyance copy & paste typo in spi_lpc2xxx_set_config(). Also, to be
> >ensure the SPI interrupts are using different, was entered a priority
> >selector, like that was done for the lpc2xxx serial driver.
> 
> Thanks for the patch. I've checked it in with some minor mods, primarily 
> placing the new options under the relevant bus options, renaming them 
> consistently with that, and making the bus options components.

I brief check it and got a compile error. There are two undefined CDL
names: cdl_option CYGNUM_IO_SPI_ARM_LPC2XXX_BUS{0,1}_INTPRIO instead
SPI{0,1}. It looks like a copy & paste typo. I prefer no Changelog
this. Fix it, please.

Thank you.

Sergei

[-- Attachment #2: spi_lpc2xxx.cdl.patch --]
[-- Type: text/x-diff, Size: 1570 bytes --]

Index: devs/spi/arm/lpc2xxx/current/cdl/spi_lpc2xxx.cdl
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/spi/arm/lpc2xxx/current/cdl/spi_lpc2xxx.cdl,v
retrieving revision 1.4
diff -U5 -r1.4 spi_lpc2xxx.cdl
--- devs/spi/arm/lpc2xxx/current/cdl/spi_lpc2xxx.cdl	17 Feb 2009 00:52:58 -0000	1.4
+++ devs/spi/arm/lpc2xxx/current/cdl/spi_lpc2xxx.cdl	17 Feb 2009 07:30:29 -0000
@@ -63,11 +63,11 @@
         default_value 1
         description   "The LPC2xxx controllers contain two SPI interfaces.
                        Enable this component to get support for SPI
                        interface 0."
 
-        cdl_option CYGNUM_IO_SPI_ARM_LPC2XXX_BUS0_INTPRIO {
+        cdl_option CYGNUM_IO_SPI_ARM_LPC2XXX_SPI0_INTPRIO {
             display       "Interrupt priority of the SPI bus 0 ISR"
             flavor        data
             legal_values  0 to 15
             default_value 12
             requires      { is_active(CYGNUM_IO_SPI_ARM_LPC2XXX_SPI1_INTPRIO)
@@ -87,11 +87,11 @@
         default_value 1
         description   "The LPC2xxx controllers contain two SPI interfaces.
                        Enable this component to get support for SPI
                        interface 1."
 
-        cdl_option CYGNUM_IO_SPI_ARM_LPC2XXX_BUS1_INTPRIO {
+        cdl_option CYGNUM_IO_SPI_ARM_LPC2XXX_SPI1_INTPRIO {
             display       "Interrupt priority of the SPI bus 1 ISR"
             flavor        data
             legal_values  0 to 15
             default_value 13
             description "

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: SPI lpc2xxx patch
  2009-02-17  7:38   ` Sergei Gavrikov
@ 2009-02-17  9:09     ` Jonathan Larmour
  2009-02-17 10:19       ` Sergei Gavrikov
  0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Larmour @ 2009-02-17  9:09 UTC (permalink / raw)
  To: Sergei Gavrikov; +Cc: eCos patches list

Sergei Gavrikov wrote:
> On Tue, Feb 17, 2009 at 12:54:40AM +0000, Jonathan Larmour wrote:
> 
>>Sergei Gavrikov wrote:
>>
>>>Hi
>>>
>>>I never try SPI driver for lpc2xxx before, today I did and found an
>>>annoyance copy & paste typo in spi_lpc2xxx_set_config(). Also, to be
>>>ensure the SPI interrupts are using different, was entered a priority
>>>selector, like that was done for the lpc2xxx serial driver.
>>
>>Thanks for the patch. I've checked it in with some minor mods, primarily 
>>placing the new options under the relevant bus options, renaming them 
>>consistently with that, and making the bus options components.
> 
> 
> I brief check it and got a compile error. There are two undefined CDL
> names: cdl_option CYGNUM_IO_SPI_ARM_LPC2XXX_BUS{0,1}_INTPRIO instead
> SPI{0,1}. It looks like a copy & paste typo. I prefer no Changelog
> this. Fix it, please.

It should have been the other way round to make the option named 
consistently with its parent. Fixed anyway.

Jifl
-- 
*See us at Embedded World 2009, Nürnberg, Germany, 3-5 Mar, Stand 11-300*
eCosCentric Limited      http://www.eCosCentric.com/     The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
------["Si fractum non sit, noli id reficere"]------       Opinions==mine

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: SPI lpc2xxx patch
  2009-02-17  9:09     ` Jonathan Larmour
@ 2009-02-17 10:19       ` Sergei Gavrikov
  2009-02-18 17:30         ` Jonathan Larmour
  0 siblings, 1 reply; 6+ messages in thread
From: Sergei Gavrikov @ 2009-02-17 10:19 UTC (permalink / raw)
  To: Jonathan Larmour; +Cc: Sergei Gavrikov, eCos patches list

On Tue, Feb 17, 2009 at 09:09:55AM +0000, Jonathan Larmour wrote:
> Sergei Gavrikov wrote:
> >On Tue, Feb 17, 2009 at 12:54:40AM +0000, Jonathan Larmour wrote:
> >
> >>Sergei Gavrikov wrote:
> >>
> >>>Hi
> >>>
> >>>I never try SPI driver for lpc2xxx before, today I did and found an
> >>>annoyance copy & paste typo in spi_lpc2xxx_set_config(). Also, to be
> >>>ensure the SPI interrupts are using different, was entered a priority
> >>>selector, like that was done for the lpc2xxx serial driver.
> >>
> >>Thanks for the patch. I've checked it in with some minor mods, primarily 
> >>placing the new options under the relevant bus options, renaming them 
> >>consistently with that, and making the bus options components.
> >
> >
> >I brief check it and got a compile error. There are two undefined CDL
> >names: cdl_option CYGNUM_IO_SPI_ARM_LPC2XXX_BUS{0,1}_INTPRIO instead
> >SPI{0,1}. It looks like a copy & paste typo. I prefer no Changelog
> >this. Fix it, please.
> 
> It should have been the other way round to make the option named 
> consistently with its parent. Fixed anyway.

Now compiling is okay. But, there is a mixture of {BUS,SPI}._INTPRIO in
CDL file:

requires      { is_active(CYGNUM_IO_SPI_ARM_LPC2XXX_SPI1_INTPRIO)
	     implies (CYGNUM_IO_SPI_ARM_LPC2XXX_SPI0_INTPRIO !=
	     CYGNUM_IO_SPI_ARM_LPC2XXX_SPI1_INTPRIO)
}

It seems for me the below must be applied for CDL too

s/SPI0_INTPRIO/BUS0_INTPRIO/
s/SPI1_INTPRIO/BUS1_INTPRIO/

or may be I misunderstand something. Thanks.

Sergei

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: SPI lpc2xxx patch
  2009-02-17 10:19       ` Sergei Gavrikov
@ 2009-02-18 17:30         ` Jonathan Larmour
  0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Larmour @ 2009-02-18 17:30 UTC (permalink / raw)
  To: Sergei Gavrikov; +Cc: eCos patches list

[-- Attachment #1: Type: text/plain, Size: 803 bytes --]

Sergei Gavrikov wrote:
> 
> Now compiling is okay. But, there is a mixture of {BUS,SPI}._INTPRIO in
> CDL file:
> 
> requires      { is_active(CYGNUM_IO_SPI_ARM_LPC2XXX_SPI1_INTPRIO)
> 	     implies (CYGNUM_IO_SPI_ARM_LPC2XXX_SPI0_INTPRIO !=
> 	     CYGNUM_IO_SPI_ARM_LPC2XXX_SPI1_INTPRIO)
> }
> 
> It seems for me the below must be applied for CDL too
> 
> s/SPI0_INTPRIO/BUS0_INTPRIO/
> s/SPI1_INTPRIO/BUS1_INTPRIO/
> 
> or may be I misunderstand something. Thanks.

Nope, I'm a thickie. Fixed with the attached patch.

Jifl
-- 
eCosCentric Limited      http://www.eCosCentric.com/     The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
------["The best things in life aren't things."]------      Opinions==mine

[-- Attachment #2: lpc2xxx.spi.silly.mistake.fix.patch --]
[-- Type: text/x-patch, Size: 2870 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/spi/arm/lpc2xxx/current/ChangeLog,v
retrieving revision 1.4
diff -u -5 -p -r1.4 ChangeLog
--- ChangeLog	17 Feb 2009 00:51:30 -0000	1.4
+++ ChangeLog	18 Feb 2009 17:18:14 -0000
@@ -1,5 +1,10 @@
+2009-02-18  Jonathan Larmour  <jifl@eCosCentric.com>
+
+	* cdl/spi_lpc2xxx.cdl: Fix my mistake in below change and make
+	CDL requires match option names.
+
 2009-02-17  Jonathan Larmour  <jifl@eCosCentric.com>
 2009-01-27  Sergei Gavrikov  <sergei.gavrikov@gmail.com>
 
 	* cdl/spi_lpc2xxx.cdl: Ensure the SPI interrupts are using different
 	priorities: CYGNUM_IO_SPI_ARM_LPC2XXX_SPI{0,1}_INTPRIO entered.
Index: cdl/spi_lpc2xxx.cdl
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/spi/arm/lpc2xxx/current/cdl/spi_lpc2xxx.cdl,v
retrieving revision 1.4
diff -u -5 -p -r1.4 spi_lpc2xxx.cdl
--- cdl/spi_lpc2xxx.cdl	17 Feb 2009 00:52:58 -0000	1.4
+++ cdl/spi_lpc2xxx.cdl	18 Feb 2009 17:18:14 -0000
@@ -6,11 +6,11 @@
 #
 # ====================================================================
 ## ####ECOSGPLCOPYRIGHTBEGIN####                                            
 ## -------------------------------------------                              
 ## This file is part of eCos, the Embedded Configurable Operating System.   
-## Copyright (C) 1998, 1999, 2000, 2001, 2002 Free Software Foundation, Inc.
+## Copyright (C) 1998, 1999, 2000, 2001, 2002 ,2009 Free Software Foundation, Inc.
 ##
 ## eCos 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 or (at your option) any later      
 ## version.                                                                 
@@ -68,13 +68,13 @@ cdl_package CYGPKG_DEVS_SPI_ARM_LPC2XXX 
         cdl_option CYGNUM_IO_SPI_ARM_LPC2XXX_BUS0_INTPRIO {
             display       "Interrupt priority of the SPI bus 0 ISR"
             flavor        data
             legal_values  0 to 15
             default_value 12
-            requires      { is_active(CYGNUM_IO_SPI_ARM_LPC2XXX_SPI1_INTPRIO)
-                             implies (CYGNUM_IO_SPI_ARM_LPC2XXX_SPI0_INTPRIO !=
-                             CYGNUM_IO_SPI_ARM_LPC2XXX_SPI1_INTPRIO)
+            requires      { is_active(CYGNUM_IO_SPI_ARM_LPC2XXX_BUS1_INTPRIO)
+                             implies (CYGNUM_IO_SPI_ARM_LPC2XXX_BUS0_INTPRIO !=
+                             CYGNUM_IO_SPI_ARM_LPC2XXX_BUS1_INTPRIO)
             }
             description "
                 This option specifies the interrupt priority of the ISR of
                 the SPI bus 0 interrupt in the VIC. Slot 0 has the highest
                 priority and slot 15 the lowest."

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2009-02-18 17:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-01-27  9:57 SPI lpc2xxx patch Sergei Gavrikov
2009-02-17  0:54 ` Jonathan Larmour
2009-02-17  7:38   ` Sergei Gavrikov
2009-02-17  9:09     ` Jonathan Larmour
2009-02-17 10:19       ` Sergei Gavrikov
2009-02-18 17:30         ` Jonathan Larmour

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).