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 <[email protected]> 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.
