RTEMS-CVS is broken

[Joel Sherrill]

Ralf Corsepius wrote:

>On Thu, 2006-07-13 at 10:24 -0500, Joel Sherrill wrote:
>  
>
>>Ralf Corsepius wrote:
>>
>>    
>>
>>>Thomas,
>>>
>>>your last week's monster check in into RTEMS cvs screwed up RTEMS badly:
>>>
>>>Since your patch, all BSPs, I've tried so far, fail with an error
>>>similar to this:
>>>..
>>>./../../../../armulator/lib/include/rtems/confdefs.h:385: error:
>>>'ATA_DRIVER_TASK_DEFAULT_PRIORITY' undeclared here (not in a function)
>>>gmake[5]: *** [init.o] Error 1
>>>..
>>>
>>> 
>>>
>>>      
>>>
>>What configure command are you using? 
>>    
>>
>
>In the case above:
>
><dir>/rtems/configure  --target=arm-rtems4.7 --enable-maintainer-mode
>--enable-posix
>
>  
>
Hmmm.. that built for me.  I don't have anything different from CVS 
right now
that would be in an arm build.

>>>Furthermore, checking what you checked in to confdefs.h, makes me
>>>believe you broke cpukit:
>>>
>>>E.g. this
>>>+#ifdef CONFIGURE_APPLICATION_NEEDS_ATA_DRIVER
>>>+#include <libchip/ata.h>
>>>+#endif
>>>
>>>accesses libchip/ata.h (not part of cpukit).
>>>
>>>
>>> 
>>>
>>>      
>>>
>>This may not be so bad since it should only be turned on by code which knows
>>the libchip ATA driver is supported on that BSP. 
>>    
>>
>Sorry I consider the above to be truely unacceptably bad design:
>
>You are turning the hierarchies upside down:
>1. libchip drivers are "plugins", a BSP has to "glue them together".
>cpukit should not need to know anything about a driver.
>  
>

RTEMS CPUKit can define the standard public interface to a device driver.
The top 1/2 of ata.h looks to be the generic driver interface similar to the
*drv.h files in libcsupport.  The bottom looks implementation specific 
to me.

>2. Conditionally including files outside of cpukit from inside of
>cpukit, renders successful building a random accident.
>
>  
>
>> It would be more 
>>correct long
>>term to move the ata.h file to cpukit if it is a "standard RTEMS driver 
>>interface" and (hopefully) doesn't suck in a lot more.
>>    
>>
>The ata stuff is i386 specific.
>
>  
>
Other embedded targets have ata interfaces.  Maybe this implementation 
is i386 specific
but the interface is not.

>>>Furthermore, you missed to add changelog entries.
>>>
>>> 
>>>
>>>      
>>>
>>That's not good. :(
>>    
>>
>Yes, it renders reverting these changes a pain.
>
>Ralf
>
>
>  
>