This is the mail archive of the ecos-patches@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: FW: Miscellaneous innovator patches


I'll go back in and scrub things up.  With this patch (or the scrubbed
version of it), I was able to run all* of the tests built with the default
template.  Since the time that I submitted it (and once I started working on
the USB support), I have discovered a number of bugs in the interrupt
handling -- it worked well enough to support the timer, but not well enough
for other devices.  Oops.  Anyway, I'll have patches for that once this one
gets committed and once I get the signoff on the copyright assignment form
here at DTC (my new company)

Speaking of which -- I think I'll do the cleanups to the last Delphi (my old
company) patch at home on my own time, so you don't need to wait for DTC to
sign paperwork, but it means that it may take a little while before I get to
it :-(


(*) IIRC, the floating point test did not run.  Probably I didn't have the
patience to allow it to run to completion -- it just seemed to be taking a
long time, so I skipped that one.  Since the test seemed to be related to
testing the hardware FPU, and since the OMAP doesn't have a hardware FPU, it
seemed just as plausable to me that a) it would take an inordinate amount of
time to run; as b) there is a bug in the software emulation of the FPU (on
the OMAP).  I didn't worry too much about it.
--wpd


> -----Original Message-----
> From: Jonathan Larmour [mailto:jifl@eCosCentric.com] 
> Sent: Monday, June 16, 2003 12:13 AM
> To: Doyle, Patrick
> Cc: 'ecos-patches@sources.redhat.com'
> Subject: Re: FW: Miscellaneous innovator patches
> 
> 
> Doyle, Patrick wrote:
> > Would now be a good time to ask about the status of these 
> patches again?
> 
> Yep.
> 
> >  I
> > am waiting for these to be committed, and for management 
> here to sign off on
> > the copyright assignment form, to submit new OMAP patches.  
> I have fixed
> > some interrupt related stuff and have started reorganizing 
> the tree to pull
> > all of the OMAP specific stuff into a single package, which 
> targets such as
> > the Innovator and Minno can use.
> 
> Sounds good.
> 
> With this patch, is the eCos support complete for this port now?
> 
> > -        //#define PLATFORM_SETUP_FROM_CCS_GEL_SCRIPT
> > -#ifdef PLATFORM_SETUP_FROM_CCS_GEL_SCRIPT	
> > -        // This is the version of _platform_setup adapted 
> from the contents
> > -        // of the GEL script shipped with Code Composer Studio
> 
> I missed this before. I'm glad it's going though as we can't 
> have stuff 
> that we don't own unless we have permission to use it (unless 
> we already 
> discussed this and it does come with permission :)).
> 
> > -	// This is all stolen from the ipaq setup
> 
> iPAQ for eCos, right?
> 
> > -        // The rest of this is stolen from "init.c" in the 
> sloader program.
> 
> Ditto as above... if this is just being removed anyway, 
> that's okay. If 
> stuff remains and it isn't an implicit requirement with the 
> hardware, we 
> need to know about it.
> 
> > +// -- stolen from proc-arm925.S (in Linux source)
> > +	mov	r0, #0
> > +// There should be a CDL option to decide wheter we want 
> the streaming
> > +// option turned on or not 
> (CONFIG_CPU_ARM925_NON_STREAMING_ON).  Since
> > +// Linux seems to work with the streaming mode disabled, 
> we will disable
> > +// it here.
> > +        orr     r0,r0,#0x80
> > +#if defined(CONFIG_CPU_ARM925_TRANSPARENT_ON)
> > +// We need to add CDL to decide whether or not to turn transparent
> > +// mode on.  For now, since it is disabled in Linux, we 
> don't enable it
> > +// here.
> > +        orr     r0,r0,#0x2
> > +#endif
> > +	mcr	p15, 0, r0, c15, c1, 0		@ write TI 
> config register
> > +// -- end of stuff stolen from proc-arm925.S (in Linux source)
> 
> I don't think you should say "stolen". We have to be careful about 
> copyright. If you mean "inspired by" then say so because 
> that's okay :).
> 
> You can't copy stuff directly, that's a no-no. If you copied 
> ideas about 
> the hardware setup that are sorta obvious (as in, that's the 
> only real way 
> to do it because the hardware has to be set up that way) then 
> that's okay. 
> We don't want to fall foul of copyright issues. But note: never copy 
> anything literally.
> 
> >  void
> >  hal_interrupt_mask(int vector)
> >  {
> > -#ifdef LATER
> > +    volatile cyg_uint32 *mir;
> > +    cyg_uint32  old_mir;
> > +
> >      CYG_ASSERT(vector <= CYGNUM_HAL_ISR_MAX &&
> >                 vector >= CYGNUM_HAL_ISR_MIN , "Invalid vector");
> >  
> > -    HAL_WRITE_UINT32(INNOVATOR_INT_MASK_CLEAR, 1<<vector);
> > -#endif
> > +    // Handle a specific startup case where we are asked 
> to mask interrupt
> > +    // vector number 0.  Since 'CYGNUM_HAL_ISR_MIN' is 2, 
> if asserts
> > +    // are not enabled, we will end up masking an 
> interrupt vector that,
> > +    // perhaps, we didn't want to.
> > +    if (vector == 0) {
> > +      return;
> > +    }
> 
> I'm not sure what the intention is here. Why is anything 
> masking 0? But 
> then why is it then appropriate to hit asserts in a 
> development build? 
> Ditto unmask.
> 
> 
> > +    // WARNING WARNING WARNING DANGER WILL ROBINSON 
> WARNING WARNING WARNING
> > +    // This will clear a pending edge triggered interrupt 
> that occured
> > +    // since the time that we received the previous edge triggered
> > +    // interrupt.  (On the OMAP, simply determining that 
> we received an
> > +    // edge triggered interrupt clears the fact that we 
> received an edge
> > +    // triggered interrrupt -- if we receive another one, 
> it will be recorded
> > +    // in the ITR and this will clear that).  This feels 
> like a dangerous
> > +    // thing to do here, but as long as all edge triggered 
> interrupt
> > +    // handlers first acknowledge the receipt of the 
> interrupt, and then
> > +    // determine the cause and/or the number of things to 
> do, it should be
> > +    // ok.
> 
> This should generally be okay because a driver should in 
> general always 
> loop round to determine whether there's stuff to do, rather 
> than assume 
> that 1 interrupt => exactly 1 event to process.
> 
> > +    // Later, rinse, repeat for interrupt handler 1 if 
> this was an interrupt
> 
> Lather :-).
> 
> > + * Finally, the instruction stream was extracted into the 
> following array:
> > + */
> > +static char idle_loop[] = {
> 
> const?
> 
> > +  /* Copy the idle loop insruction stream to address 
> 0x10000 in the DSP */
> > +  memcpy((void *)MPUI_PORT_RAM + 0x10000, idle_loop, 
> sizeof(idle_loop));
> 
> If it's important this is done byte at a time and in the 
> right order, then 
> be careful of using memcpy.
> 
> > +2003-04-15  Patrick Doyle  <wpd@dtccom.com>
> > +
> > +	* include/hal_cache.h: Added
> > +	'CYGHWR_HAL_ARM_ARM9_ALT_CLEAN_DCACHE' (stolen from the 
> Linux code
> > +	for the innovator) because the 'CYGHWR_HAL_ARM_ARM9_CLEAN_CACHE'
> > +	feature did not work correctly on the innovator.
> 
> Nope, not the s word please! But then, I'm afraid what you've done is 
> fairly obviously literally copied. Unfortunately that can't be used 
> without it being fully GPL'd. Can I instead you suggest you 
> look at the 
> linux kernel code for _inspiration_ (if you see the 
> distinction I mean) 
> but then write the code from what you've now understood. 
> Sorry if this 
> seems picky, but we have to keep our noses clean.
> 
> The rest of the patch is absolutely fine in as much as I can 
> tell! If you 
> can fix/answer the above then I should have no problems 
> checking it in.
> 
> BTW, It would be good if you could write something for the RedBoot 
> documentation, just so people have an idea of how to get 
> stuff onto the 
> board. The file to look at is 
> ecos/packages/redboot/current/doc/redboot_installing.sgml, 
> and you can 
> just copy an appropriate other target as a starting point if 
> you like. If 
> you have access to jade that would be a great help. If not, 
> then I can 
> probably munge around the tags and fix it up if you can make 
> a stab at it. 
> Just send this as a separate patch.
> 
> Jifl
> -- 
> eCosCentric    http://www.eCosCentric.com/    The eCos and 
> RedBoot experts
> --[ "You can complain because roses have thorns, or you ]--
> --[  can rejoice because thorns have roses." -Lincoln   ]-- 
> Opinions==mine
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]