[Bug 1000819] New: Add support for Atmel AT91SAM9263

[Bug 1000819] New: Add support for Atmel AT91SAM9263

[bugzilla-daemon]

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

           Summary: Add support for Atmel AT91SAM9263
           Product: eCos
           Version: CVS
          Platform: Other (please specify)
        OS/Version: Other
            Status: UNCONFIRMED
          Severity: normal
          Priority: normal
         Component: Patches and contributions
        AssignedTo: jifl@ecoscentric.com
        ReportedBy: dushistov@mail.ru
         QAContact: ecos-patches@ecos.sourceware.org
                CC: ecos-patches@ecos.sourceware.org
             Class: ---


Here is patchset that introduces support for at91sam9263 cpu,
it splited to logic steps:

1)introduce arm7 package
2)fix hal_diag to handle 115200 baudrate
3)separate board's pio layout
4)move common for arm9 code for work with mmu into common header
5)hal packages for at91sam9263ek and at91sam9
6)implement support pio interrupts in the way as ordinal interrupt
7)support of norflash for at91sam9263
8)can support for at91sam9263ek


-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819





--- Comment #1 from Evgeniy <dushistov@mail.ru>  2009-09-06 18:46:13 ---
Created an attachment (id=798)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=798)
patch 1


-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819





--- Comment #2 from Evgeniy <dushistov@mail.ru>  2009-09-06 18:47:17 ---
Created an attachment (id=799)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=799)
patch 2


-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819





--- Comment #3 from Evgeniy <dushistov@mail.ru>  2009-09-06 18:48:38 ---
Created an attachment (id=800)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=800)
patch 3


-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819


Evgeniy <dushistov@mail.ru> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
    Attachment #799|application/octet-stream    |text/plain
          mime type|                            |
 Attachment #799 is|0                           |1
              patch|                            |




-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819





--- Comment #4 from Evgeniy <dushistov@mail.ru>  2009-09-06 18:49:32 ---
Created an attachment (id=801)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=801)
patch 4


-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819





--- Comment #5 from Evgeniy <dushistov@mail.ru>  2009-09-06 18:50:17 ---
Created an attachment (id=802)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=802)
patch 5


-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819





--- Comment #7 from Evgeniy <dushistov@mail.ru>  2009-09-06 18:51:31 ---
Created an attachment (id=804)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=804)
patch 7


-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819





--- Comment #6 from Evgeniy <dushistov@mail.ru>  2009-09-06 18:50:56 ---
Created an attachment (id=803)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=803)
patch 6


-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819





--- Comment #8 from Evgeniy <dushistov@mail.ru>  2009-09-06 18:52:11 ---
Created an attachment (id=805)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=805)
patch 8


-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are on the CC list for the bug.

Re: [Bug 1000819] New: Add support for Atmel AT91SAM9263

[Evgeniy Dushistov]

On Sun, Sep 06, 2009 at 06:42:24PM +0100, bugzilla-daemon@ecoscentric.com wrote:
> http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819
> 
>            Summary: Add support for Atmel AT91SAM9263
>            Product: eCos
>            Version: CVS
>           Platform: Other (please specify)
>         OS/Version: Other
>             Status: UNCONFIRMED
>           Severity: normal
>           Priority: normal
>          Component: Patches and contributions
>         AssignedTo: jifl@ecoscentric.com
>         ReportedBy: dushistov@mail.ru
>          QAContact: ecos-patches@ecos.sourceware.org
>                 CC: ecos-patches@ecos.sourceware.org
>              Class: ---
> 
> 
> Here is patchset that introduces support for at91sam9263 cpu,
> it splited to logic steps:
> 
> 1)introduce arm7 package
> 2)fix hal_diag to handle 115200 baudrate
> 3)separate board's pio layout
> 4)move common for arm9 code for work with mmu into common header
> 5)hal packages for at91sam9263ek and at91sam9
> 6)implement support pio interrupts in the way as ordinal interrupt
> 7)support of norflash for at91sam9263
> 8)can support for at91sam9263ek
> 

Finally I got reply from fsf:

>Your ECOS assignment/disclaimer process with the FSF is currently
>complete

so I ready to start discuss technical problems for merging support
for at91sam9263 into CVS.

-- 
/Evgeniy

Re: [Bug 1000819] New: Add support for Atmel AT91SAM9263

[John Dallaway]

Evgeniy, many thanks for this contribution.

> Here is patchset that introduces support for at91sam9263 cpu,
> it splited to logic steps:
> 
> 1)introduce arm7 package
> 2)fix hal_diag to handle 115200 baudrate
> 3)separate board's pio layout
> 4)move common for arm9 code for work with mmu into common header
> 5)hal packages for at91sam9263ek and at91sam9
> 6)implement support pio interrupts in the way as ordinal interrupt
> 7)support of norflash for at91sam9263
> 8)can support for at91sam9263ek

There's quite a lot to digest here. In particular, the introduction of
the ARM7 package is welcome but needs careful consideration to ensure
that the abstraction is correct.

Nick, since you have AT91SAM9 expertise, are you able to review this
contribution?

John Dallaway

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

John Dallaway <john@dallaway.org.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |john@dallaway.org.uk
         OS/Version|Other                       |ARM

--- Comment #9 from John Dallaway <john@dallaway.org.uk> 2010-02-24 11:54:41 GMT ---
Note. Current sources at: http://github.com/Dushistov/qemu_at91sam9263

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

John Dallaway <john@dallaway.org.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
         AssignedTo|backlog@bugs.ecos.sourcewar |john@dallaway.org.uk
                   |e.org                       |
     Ever Confirmed|0                           |1

--- Comment #11 from John Dallaway <john@dallaway.org.uk> 2011-03-16 12:05:52 GMT ---
Let's try to push through with review and get it checked in. I've invited all
interested parties to add themselves to the CC list and add their own comment
where necessary/appropriate.

Evgeniy, the use of multiple patches is a great help - thank you!

Let's start with the ARM7/ARM9 abstraction work (patch 1). This looks to be a
case of moving the existing HAL cache macros (which are not appropriate for
AT91SAM9) from the AT91 variant package to a new ARM7 package. I assume that
there is nothing AT91-specific in the new package so it could be used by any
other ARM7 ports in the future. Please confirm.

As it stands, this patch should only affect AT91 targets. The risk of breaking
other AT91 target support is small since there does not appear to be any
changes to the macros themselves. There is a risk of breaking compatibility
with non-contributed ports to AT91SAM7 hardware but the effort in fixing such
breakage should be limited to adding the new ARM7 package to the target
description in ecos.db. I don't have a problem with this.

Is there any other code in the existing AT91 variant HAL which might not be
appropriate for AT91SAM9? Any other comments on patch 1?

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

Jonathan Larmour <jifl@ecoscentric.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jifl@ecoscentric.com

--- Comment #12 from Jonathan Larmour <jifl@ecoscentric.com> 2011-03-16 16:44:21 GMT ---
Re patch 1: This is indeed uncontroversial.

There are a few typos, e.g. in CDL description "on a ARM7" -> "on an ARM7". 
Redundant use of "ARM ARM7" can just be "ARM7" IMHO.
No newline at end of hal_arm_arm7.cdl apparently.
No license header for the .cdl file either.
While there, no EOF line at end (to fit existing coding standards).

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

--- Comment #13 from Jonathan Larmour <jifl@ecoscentric.com> 2011-03-16 16:54:03 GMT ---
(In reply to comment #11)
> Let's try to push through with review and get it checked in. I've invited all
> interested parties to add themselves to the CC list and add their own comment
> where necessary/appropriate.
> 
> Evgeniy, the use of multiple patches is a great help - thank you!
> 
> Let's start with the ARM7/ARM9 abstraction work (patch 1). This looks to be a
> case of moving the existing HAL cache macros (which are not appropriate for
> AT91SAM9) from the AT91 variant package to a new ARM7 package. I assume that
> there is nothing AT91-specific in the new package so it could be used by any
> other ARM7 ports in the future. Please confirm.

No ARM7 has cache, so it's safe IMO.

> As it stands, this patch should only affect AT91 targets. The risk of breaking
> other AT91 target support is small since there does not appear to be any
> changes to the macros themselves. There is a risk of breaking compatibility
> with non-contributed ports to AT91SAM7 hardware but the effort in fixing such
> breakage should be limited to adding the new ARM7 package to the target
> description in ecos.db. I don't have a problem with this.

Particularly since it's obvious due to the change in the AT91 HAL for the ARM7
HAL to be its parent (which implies active_if).

> Is there any other code in the existing AT91 variant HAL which might not be
> appropriate for AT91SAM9?

That is in fact a very important issue, and one of the harder ones with this
patch. The AT91 HAL already has lots of ifdefs, making it harder to parse and
see what's going on. I think I already mentioned (way, way back) the SH HAL
having a more tractable approach by splitting out CPUs into their own files,
and that's what patch 3 appears to be about, but whether it's sufficient to
limit that to just PIO definitions I'm distinctly unsure. Maybe we'll leave
that to discussing Patch 3 in depth.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

--- Comment #14 from Grant Edwards <grant.b.edwards@gmail.com> 2011-03-16 16:59:15 GMT ---
(In reply to comment #13)
> (In reply to comment #11)

> > Let's start with the ARM7/ARM9 abstraction work (patch 1). This
> > looks to be a case of moving the existing HAL cache macros (which
> > are not appropriate for AT91SAM9) from the AT91 variant package to
> > a new ARM7 package. I assume that there is nothing AT91-specific
> > in the new package so it could be used by any other ARM7 ports in
> > the future. Please confirm.
> 
> No ARM7 has cache, so it's safe IMO.

Samsung S3C45xx ARM7 parts have cache.  Or did you mean no Atmel ARM7
parts have cache?

-- 
Grant

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

--- Comment #15 from Jonathan Larmour <jifl@ecoscentric.com> 2011-03-16 17:03:59 GMT ---
(In reply to comment #14)
> (In reply to comment #13)
> > (In reply to comment #11)
> 
> > > Let's start with the ARM7/ARM9 abstraction work (patch 1). This
> > > looks to be a case of moving the existing HAL cache macros (which
> > > are not appropriate for AT91SAM9) from the AT91 variant package to
> > > a new ARM7 package. I assume that there is nothing AT91-specific
> > > in the new package so it could be used by any other ARM7 ports in
> > > the future. Please confirm.
> > 
> > No ARM7 has cache, so it's safe IMO.
> 
> Samsung S3C45xx ARM7 parts have cache.  Or did you mean no Atmel ARM7
> parts have cache?

Ah ok, that changes things then. This is meant to be a generic ARM7 HAL. The
initial focus is on AT91, but it's definitely not meant to be an AT91 ARM7 HAL.

In that case that complicates things. We need to construct the ARM7 variant
hal_cache.h in such a way that a processor HAL (e.g. S3C45xx) can override all
the settings.

There are numerous other examples of this in other HALs. The general idea is
that the hal_cache.h #includes a <cyg/hal/proc_cache.h> or similar, before it
defines anything itself.

Jifl

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

--- Comment #16 from Grant Edwards <grant.b.edwards@gmail.com> 2011-03-16 17:19:53 GMT ---
(In reply to comment #15)
> (In reply to comment #14)
>
> > > No ARM7 has cache, so it's safe IMO.
> > 
> > Samsung S3C45xx ARM7 parts have cache.  Or did you mean no Atmel
> > ARM7 parts have cache?
> 
> Ah ok, that changes things then. This is meant to be a generic ARM7
> HAL. The initial focus is on AT91, but it's definitely not meant to
> be an AT91 ARM7 HAL.
> 
> In that case that complicates things. We need to construct the ARM7
> variant hal_cache.h in such a way that a processor HAL
> (e.g. S3C45xx) can override all the settings.

FWIW, the "snds" port is for the Samsung S3C4500/4510 ARM7 parts and
supports the CACHE macros.  I haven't looked at it in ages, so I don't
know what state it's in.  Those parts are so old that I doubt anybody
cares enough to re-write the HAL to use any sort of generic ARM7
support. (They are still selling the '4510, but the '4500 died ages
ago.)

-- 
Grant

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

--- Comment #17 from John Dallaway <john@dallaway.org.uk> 2011-03-16 17:33:50 GMT ---
(In reply to comment #15)
> (In reply to comment #14)
> > Samsung S3C45xx ARM7 parts have cache.  Or did you mean no Atmel ARM7
> > parts have cache?
> 
> Ah ok, that changes things then. This is meant to be a generic ARM7 HAL. The
> initial focus is on AT91, but it's definitely not meant to be an AT91 ARM7
> HAL.
> 
> In that case that complicates things. We need to construct the ARM7 variant
> hal_cache.h in such a way that a processor HAL (e.g. S3C45xx) can override all
> the settings.
> 
> There are numerous other examples of this in other HALs. The general idea is
> that the hal_cache.h #includes a <cyg/hal/proc_cache.h> or similar, before it
> defines anything itself.

Yes, this seems like the right approach where _most_ ARM7 processors have no
cache. Something like:

  #ifdef CYGBLD_HAL_ARM_PROC_CACHE_H
  #include <cyg/hal/proc_cache.h>
  #endif

  #ifndef HAL_DCACHE_ENABLE
  #define HAL_DCACHE_ENABLE()
  #endif

etc...

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

--- Comment #18 from Grant Edwards <grant.b.edwards@gmail.com> 2011-03-16 18:34:03 GMT ---
(In reply to comment #11)

> Let's try to push through with review and get it checked in. I've
> invited all interested parties to add themselves to the CC list and
> add their own comment where necessary/appropriate.

I'm a little concerned by Martin Laabs post on the mailing list:

   my port based on the one of Evgeniy Dushitov at the very
   beginning. However - after some weeks I discovered that it was very
   hard to support more CPUs out of the AT91SAM9 family with that
   code-base.

There are over a dozen closely related parts in that family (all
ARM926EJS CPUs with mostly the same base set of peripherals).  Do we
want to head down a road that supports only one of them?

-- 
Grant

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

--- Comment #19 from John Dallaway <john@dallaway.org.uk> 2011-03-16 19:06:44 GMT ---
(In reply to comment #18)

> (In reply to comment #11)
> 
> > Let's try to push through with review and get it checked in. I've
> > invited all interested parties to add themselves to the CC list and
> > add their own comment where necessary/appropriate.
> 
> I'm a little concerned by Martin Laabs post on the mailing list:
> 
>    my port based on the one of Evgeniy Dushitov at the very
>    beginning. However - after some weeks I discovered that it was very
>    hard to support more CPUs out of the AT91SAM9 family with that
>    code-base.
> 
> There are over a dozen closely related parts in that family (all
> ARM926EJS CPUs with mostly the same base set of peripherals).  Do we
> want to head down a road that supports only one of them?

Certainly we do not. Neither do we want to introduce significant further delay.
I still think it makes sense to use Evgeniy's contribution as the focus for
this discussion. It has been formally contributed and used as the baseline for
multiple platform ports. Everyone is welcome to comment. The review process
will determine what does and does not get checked in.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

--- Comment #20 from John Dallaway <john@dallaway.org.uk> 2011-03-22 13:23:21 GMT ---
Patch 2 looks safe enough. The comment at hal_diag.c:205 should read ".02ms
steps".

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

--- Comment #21 from Jonathan Larmour <jifl@ecoscentric.com> 2011-03-22 14:05:08 GMT ---
(In reply to comment #20)
> Patch 2 looks safe enough. The comment at hal_diag.c:205 should read ".02ms
> steps".

Yes although there has to be a ChangeLog entry for it at some point. I'll
assume there will be an additional ChangeLog patch submitted in the future for
all the changes, once approved.

Jifl

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

Jonathan Larmour <jifl@ecoscentric.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
    Attachment #798|                            |assignment+, review-
               Flag|                            |

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

Jonathan Larmour <jifl@ecoscentric.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
    Attachment #799|                            |assignment+, review+
               Flag|                            |

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

Jonathan Larmour <jifl@ecoscentric.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |Patch_or_Contribution+

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

--- Comment #22 from John Dallaway <john@dallaway.org.uk> 2011-03-22 14:13:53 GMT ---
Patch 3 is separating the PIO layout definitions for various AT91 family
processors into separate header files. Historically, these definitions have all
been placed in var_io.h within various preprocessor blocks under the control
of:

  CYGHWR_HAL_ARM_AT91_M55800A
  CYGHWR_HAL_ARM_AT91SAM7
  CYGHWR_HAL_ARM_AT91SAM7S
  CYGHWR_HAL_ARM_AT91SAM7S_at91sam7s32
  CYGHWR_HAL_ARM_AT91SAM7X
  CYGHWR_HAL_ARM_AT91SAM7SE

Patch 3 separates these out into separate header files for M55800A, SAM7S,
SAM7SE, SAM7X and other ("default") processors, all within the AT91 variant
HAL. The relevant header file is specified by CYGBLD_HAL_AT91_PIO_LAYOUT_H.

Patch 3 paves the way for the introduction of further PIO layout header files
for the AT91SAM9 processors. Evgeniy's port to AT91SAM9263 includes a PIO
layout header file dedicated to this processor (AT91SAM9263) and this is also
located within the AT91 variant HAL.

I don't think there can be any argument that the historical approach of adding
more and more preprocessor blocks to the AT91 var_io.h is not scalable. So the
issues are:

a) Is the separation of PIO layout definitions into separate header files
implemented at the correct level in this case (processor level)?

b) Does it make sense to separate the PIO layout definitions from other I/O
definitions (if any) in this way?

c) For the existing ports, would it be preferable to place the PIO layout
definitions in the processor HAL rather than in the AT91 variant HAL? This
would avoid the need to give each PIO layout header file a unique name. We need
to weigh up the risk of breaking platform ports we cannot readily test.

d) For new ports (including AT91SAM9 family), would it be preferable to place
the PIO layout definitions in the processor HAL rather than in the AT91 variant
HAL? I definitely think so.

Comments?

Any other issues relating specifically to patch 3?

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

--- Comment #23 from Jonathan Larmour <jifl@ecoscentric.com> 2011-03-22 15:08:26 GMT ---
(In reply to comment #22)
> 
> a) Is the separation of PIO layout definitions into separate header files
> implemented at the correct level in this case (processor level)?

Yes.

> b) Does it make sense to separate the PIO layout definitions from other I/O
> definitions (if any) in this way?

I think it would be good to also separate other I/O definitions. I don't know
if that's too much to ask at the moment, so starting with PIO may well be fine.
You don't _want_ e.g. all SAM7x definitions put in one single file - you want
something more modular in any case because there will be commonality with other
processors.

> c) For the existing ports, would it be preferable to place the PIO layout
> definitions in the processor HAL rather than in the AT91 variant HAL? This
> would avoid the need to give each PIO layout header file a unique name. We need
> to weigh up the risk of breaking platform ports we cannot readily test.

I think in the current patch, the new pio_sam7*.h may as well live in the
at91sam7s hal, to keep all the sam7 stuff together. Of course non-SAM7's don't
have a separate processor HAL, and I don't think it's worth changing that at
this point. As you say in (d), future processor HALs, e.g. most obviously the
SAM9 should have its pio_sam9*.h files in it. IMO.

I don't think having unique names is a big deal in itself. FAOD I think it is
important to keep it as a define, rather than /requiring/ there to be a file
with a particular name.

> d) For new ports (including AT91SAM9 family), would it be preferable to place
> the PIO layout definitions in the processor HAL rather than in the AT91 variant
> HAL? I definitely think so.

Yes. 

> Comments?
> 
> Any other issues relating specifically to patch 3?

Yes, the biggest problem is that there are (again) no copyright headers for new
files, so formally I need to reject the patch as it stands. But the above
comments will require changes anyway.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

Jonathan Larmour <jifl@ecoscentric.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
    Attachment #800|                            |assignment+, review-
               Flag|                            |

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

Mark Mumford <markthetyke@4dv.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |markthetyke@4dv.net

--- Comment #26 from Mark Mumford <markthetyke@4dv.net> 2011-05-17 23:44:03 BST ---
I have an AT91SAM9G20 Evaluation Kit that I plan on using in the near future.
If I can help with testing this patch please let me know how I can help. I am
mainly using the SPI and Serial peripherals at this point, I am willing to
create a port for this CPU/board if one does not yet exist. I have done a
couple of simple ports in the past,one for a Samsung S3C2410/FriendlyARM
development board.

cheers

Mark M

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

--- Comment #27 from Daniel Helgason <dhelgason@shaw.ca> 2011-05-18 00:10:39 BST ---
(In reply to comment #26)
> I have an AT91SAM9G20 Evaluation Kit that I plan on using in the near future.
> If I can help with testing this patch please let me know how I can help. I am
> mainly using the SPI and Serial peripherals at this point, I am willing to
> create a port for this CPU/board if one does not yet exist. I have done a
> couple of simple ports in the past,one for a Samsung S3C2410/FriendlyARM
> development board.
> ...

Mark!

Hey, always glad to see another 'G20 user.

I already have a port to the AT91SAM9G20 that runs on the 'G20-EK board and
also two other boards. I also have a port to the AT91SAM9R/RL that runs on the
'9RL-EK board. The ports include cache-aware Ethernet, SPI, and USART drivers.

The ports are in a bit of a crappy state but when the dust settles here, it
should just be a matter of moving large chunks of code around. I intend to
provide my sponsor with these ports this week, then I expect they will end up
here.

Dan H.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

--- Comment #28 from Daniel Helgason <dhelgason@shaw.ca> 2011-05-18 18:19:50 BST ---
(In reply to comment #11)
> Let's try to push through with review and get it checked in. I've invited all
> interested parties to add themselves to the CC list and add their own comment
> where necessary/appropriate.
> 
> Evgeniy, the use of multiple patches is a great help - thank you!
> 
> Let's start with the ARM7/ARM9 abstraction work (patch 1). This looks to be a
> case of moving the existing HAL cache macros (which are not appropriate for
> AT91SAM9) from the AT91 variant package to a new ARM7 package. I assume that
> there is nothing AT91-specific in the new package so it could be used by any
> other ARM7 ports in the future. Please confirm.
> ...

Is is correct to have AT91 as a variant? I see it more as a package that
defines a set of common I/O and provides macros for common AT91 stuff. Would it
make sense if things were arranged more like:

ARM (arch)
+- ARM7 (variant)
   + SAM7S (platform)
   + SAM7X (platform)
   + Other Non-AT91 chip (platform)
   + SAM7S-EK (board)
   + SAM7X-EK (board)
   ...etc.
+  ARM9 (variant)
   + SAM9263 (platform)
   + SAM9G20 (platform)
   + SAM9RL64 (platform)
   + Other Non-AT91 chip (platform)
   + SAM9G20-EK (board)
   + SAM99RL-EK (board
   ...etc.
+ AT91 (I/O support package)

Or maybe I'm just confused about the relationship between arch, var, and plf?
If so, could someone please enlighten me. Thanks!

In this scheme, the AT91-based platforms define the peripheral IDs, interrupts,
etc. that vary from one chip to another. The AT91 I/O package would have
absolutely no knowledge of any particular chip.

A real advantage to having AT91 be a stand-alone I/O support package is that we
could easily support other chips with AT91 peripherals such as Atmel's SAM3
series.

Dan H.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

Re: [Bug 1000819] Add support for Atmel AT91SAM9263

[Grant Edwards]

On 2011-05-18, bugzilla-daemon@bugs.ecos.sourceware.org <bugzilla-daemon@bugs.ecos.sourceware.org> wrote:
> Please do not reply to this email. Use the web interface provided at:
> http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819
>
> --- Comment #28 from Daniel Helgason <dhelgason@shaw.ca> 2011-05-18 18:19:50 BST ---
> (In reply to comment #11)
>> Let's try to push through with review and get it checked in. I've invited all
>> interested parties to add themselves to the CC list and add their own comment
>> where necessary/appropriate.
>> 
>> Evgeniy, the use of multiple patches is a great help - thank you!
>> 
>> Let's start with the ARM7/ARM9 abstraction work (patch 1). This looks to be a
>> case of moving the existing HAL cache macros (which are not appropriate for
>> AT91SAM9) from the AT91 variant package to a new ARM7 package. I assume that
>> there is nothing AT91-specific in the new package so it could be used by any
>> other ARM7 ports in the future. Please confirm.
>> ...
>
> Is is correct to have AT91 as a variant? I see it more as a package that
> defines a set of common I/O and provides macros for common AT91 stuff.

That makes a lot of sense to me.  In reality, "AT91" is a common set
of peripherals that are used on a variety of Atmel parts having
several different architectures, varieties, and platforms.

> Would it
> make sense if things were arranged more like:
>
> ARM (arch)
> +- ARM7 (variant)
>    + SAM7S (platform)
>    + SAM7X (platform)
>    + Other Non-AT91 chip (platform)
>    + SAM7S-EK (board)
>    + SAM7X-EK (board)
>    ...etc.
> +  ARM9 (variant)
>    + SAM9263 (platform)
>    + SAM9G20 (platform)
>    + SAM9RL64 (platform)
>    + Other Non-AT91 chip (platform)
>    + SAM9G20-EK (board)
>    + SAM99RL-EK (board
>    ...etc.
> + AT91 (I/O support package)
>
> Or maybe I'm just confused about the relationship between arch, var,
> and plf?

You wouldn't be the first. :)

-- 
Grant Edwards               grant.b.edwards        Yow! Look into my eyes and
                                  at               try to forget that you have
                              gmail.com            a Macy's charge card!

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

--- Comment #29 from Mark Mumford <markthetyke@4dv.net> 2011-05-18 22:15:19 BST ---
Hi,

I'm new here but I'm not averse to expressing my opinion and nobody else has
weighed in yet, I have also done a couple of ports to an ARM7 and an ARM9
platform so I have spent time exploring the directory structure. In my opinion
I would forget the tag AT91 this doesn't represent a variant in the ECOS sense
as it includes 3 different processor architectures the ARM7, ARM9 and ARM
Cortex-M. These are different architectures because first they have different
instruction sets, second they have different pipeline, cache and bus configs.
So the AT91SAM9 series use the variant ARM922EJ-S cpu. There are already a
number of platforms under the hal/arm/arm9 directory. As such the AT91SAM9
series should be implemented as 4 separate platforms under this structure,
although they probably be using common peripheral devices which may also be
shared with the rest of the AT91 family. The AT91SAM7 series should be
implemented under the hal/arm structure and the AT91SAM3 series under the
hal/cortexm structure. Should the arm9 have been a separate architecture
instead of a sub-architecture, who cares it's too late to change it now. The
question I see is do we need to introduce a new set of devices for the AT91SAM9
series, only if there are significant enhancements or interoperability issues
would be my humble opinion. The problem I see with supporting the SAM3 from the
same IO package is that the peripheral bus structure usually implemented on
cortex devices is not the same as the earlier models. Perhaps the IO package
should be divided by peripheral bus structure AHB,AMBA etc. Okay that's enough
from me.

Mark M

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

Ilija Kocho <ilijak@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ilijak@siva.com.mk

--- Comment #30 from Ilija Kocho <ilijak@siva.com.mk> 2011-05-19 13:48:04 BST ---
(In reply to comment #29)
[snip]
> would be my humble opinion. The problem I see with supporting the SAM3 from the
> same IO package is that the peripheral bus structure usually implemented on
> cortex devices is not the same as the earlier models. Perhaps the IO package
> should be divided by peripheral bus structure AHB,AMBA etc. Okay that's enough
> from me.

For example of grafting legacy ARM peripherals to Cortex-M3 you can look at
LPC-17xx port Bug 1001114 being combined with LPC-2xxx peripherals Bug 1001115
.

However if larger re-organization of AT91 port is considered, I would propose
moving AT91 peripherals out of ARM devs subdirectories (except for peripherals
based on ARM IP, if any). Something like this:

packages-+
         +-devs+
               +-serial+
                       +-AT91-+
                              +-current-+...

Also following scheme could be considered (although radical) if same devices
are shared among all AT91 platforms:

packages-+
         +-devs-+
                +-Atmel(AT91)-+
                              +-serial-+...
                              +-ethernet-+...
                              +-spi-+...

Ilija

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

--- Comment #31 from Grant Edwards <grant.b.edwards@gmail.com> 2011-05-19 13:53:14 BST ---
(In reply to comment #30)

> However if larger re-organization of AT91 port is considered, I
> would propose moving AT91 peripherals out of ARM devs subdirectories
> (except for peripherals based on ARM IP, if any). Something like
> this:
> 
> packages-+
>          +-devs+
>                +-serial+
>                        +-AT91-+
>                               +-current-+...

That's what makes the most sense to me, and it's the way all the other
processor families are supported -- isn't it?

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

--- Comment #32 from Ilija Kocho <ilijak@siva.com.mk> 2011-05-19 14:31:37 BST ---
(In reply to comment #31)
> (In reply to comment #30)
> 
> > However if larger re-organization of AT91 port is considered, I
> > would propose moving AT91 peripherals out of ARM devs subdirectories
> > (except for peripherals based on ARM IP, if any). Something like
> > this:
> > 
> > packages-+
> >          +-devs+
> >                +-serial+
> >                        +-AT91-+
> >                               +-current-+...
> 
> That's what makes the most sense to me, and it's the way all the other
> processor families are supported -- isn't it?

I couldn't tell. In general devs looks pretty much scattered to me. For example
there are many directories related to Freescale's controllers, I can imagine
some of them contain same devices. On the other hand are ARM directories that
contain devices that are not related to ARM (architecture or company) merely to
chips with ARM cores.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug 1000819] Add support for Atmel AT91SAM9263

[bugzilla-daemon]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819

--- Comment #33 from Daniel Helgason <dhelgason@shaw.ca> 2011-05-24 19:28:12 BST ---
(In reply to comment #29)
> Hi,
> 
> ...As such the AT91SAM9
> series should be implemented as 4 separate platforms under this structure,
> although they probably be using common peripheral devices which may also be
> shared with the rest of the AT91 family...

That's the way I see it, too.

> ...The
> question I see is do we need to introduce a new set of devices for the AT91SAM9
> series, only if there are significant enhancements or interoperability issues
> would be my humble opinion....

I don't understand. Do you mean all AT91SAM9 SoC in one AT91SAM9 package? Or
additional AT91 peripherals?

> ...The problem I see with supporting the SAM3 from the
> same IO package is that the peripheral bus structure usually implemented on
> cortex devices is not the same as the earlier models. Perhaps the IO package
> should be divided by peripheral bus structure AHB,AMBA etc. Okay that's enough
> from me.

About the only problem I had with using existing AT91 drivers with the SAM3 is
that, under Cortex arch, the interrupt number is no longer the same as the AT91
peripheral ID. Also, I had to duplicate all the AT91 peripheral definitions
into the SAM3 port. Way too much duplication. That's why I think having the
AT91 package just be a I/O support package would be a good thing.

Dan H.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.