[Linux-Xtensa] Re: Help us to open a builroot git repo

Piet Delaney pdelaney at tensilica.com
Tue Sep 28 21:35:43 PDT 2010


Piet Delaney wrote:
> truby.zhuang wrote:
>>  
>> truby.zhuang wrote:
>>  > Hi,Piet:

Hi Truby, good news, see below.

>>  >  
>>  >   >Truby, could you push your changes that you mentioned on 09/26/201010
>>  >   
>>  >and let me know what problems, if any, your experiencing or have an issue
>>  >   >with.
>>  >  
>>  > We have push the newest audio dsp driver to git!
>>  
>> Ok I'll try it out. Last time nothing eventful happen.
>> If your having a problem it would be nice to hear about it.
>>  
>> -piet
>>  
>> -----------------------------------------
>>  
>> The Problem is that the cpe still will be changed! 
> 
> Likely because you calling:
> 
> 	enable_mail_queue(void)
> 
> You should be able to just let the kernel handle it for you.
> Your not touching the queue from an interrupt handler as
> far as I can see. 

I take that back, upon closer reading of your code
I've noticed  that you ARE  using the I/O ports from
an interrupt context:

       request_irq(MAILBOX_IRQ, mailbox_int, 0, "mailbox", dsp_devp));

IRQ() {
	mailbox_int(int irq, void *devp) {
		ReceiveAck(uint32_t *chanId, uint32_t *chanFmt) {
			READ_IPQ(void) {
				read_ipq %0;
			}
	     }
    }
}

The scheduler and exception handler are maintaining
the coprocessor and I/O port state for task. The IRQ
doesn't have a task context. With the current design
it's necessary for an interrupt handler to save and
restore any coprocessor or I/O port context that it
disturbs.

In you case it looks like it requires your saving and
restoring the CPENABLE register at the entry and exit
to mailbox_int().

So instead of enable_mail_queue() at the entry of
mailbox_int() have it just save cpenable.

I just modified it and will push it for your review.

If the Tasklet accesses the I/O port you need to do
the same for it.

The idea is to leverage on the existing lazy coprocessor
saving of registers. In this model your task is free from
having to do anything. Once the kernel has given the task
access the coprocessor registers and I/O ports are available
to the task. The context is maintained by the kernel on process
switching. So if only one task is using a coprocessor or
I/O port then their is no saving of context necessary.

Here's the proposed way to do mailbox_int():
--------------------------------------------------------
430 /*
431  * mailbox irq service code
432  * deal with stream ACK mail
433  */
434 irqreturn_t mailbox_int(int irq, void *devp)
435 {
436     unsigned long flags;
437     int ret = -1;
438     int chanId;
439     int fmt;
440     int saved_cpe;
441
442     /* Save CoProcessor Enable Register and Enable I/O Port (0x80) */
443     RSR_CPENABLE(saved_cpe);
444     WSR_CPENABLE(saved_cpe | 0x80);
445
446     local_irq_save(flags);  //FIXME
447
448     DSP_LOG_ENTER();
449
450     DSP_LOG("receive ACK\n");
451     ret = ReceiveAck(&chanId, &fmt);
452     if (ret == 0) {
453         DSP_LOG("stream dec finished\n");
454     }
455     DSP_LOG("receive ACK ok\n");
456
457     tasklet_schedule(&mailbox_tasklet);
458
459     local_irq_restore(flags);
460
461     WSR_CPENABLE(saved_cpe);
462
463     DSP_LOG_LEAVE();
464     return IRQ_HANDLED;
465 }
--------------------------------------------------------------------------------

I'm pretty sure this was the problem that you were experiencing.

I tried to document this in traps.c; I suppose you missed it.
------------------------------------------------------------------
/*
 * Macros used below to setup exception handlers for
 * Coprocessor and I/O Ports. Currently coprocessors
 * are owned by tasks in user space and I/O ports are
 * owned by the kernel. Both coprocessors and I/O ports
 * share the same exception causes and bit in the CPENABLE
 * register.
 *
 * We could have a separate fast_io_port() handler. It
 * appears that the kernel exception handler assumes that
 * we always have a thread_info structure with the current
 * stack pointer so it should be safe to use the existing
 * fast_coprocessor() code.
 */
------------------------------------------------------------

-piet




> Your strictly in the process context
> doing an ioctl() from what I see. I tested this with
> the DC233 and it works fine for I/O ports.
> 
> Your function prints out nasty messages if the I/O
> port isn't enabled:
> 
> +static inline void enable_mail_queue(void)
> +{
> +    int cpe = -1;
> +    RSR_CPENABLE(cpe);
> +    barrier();
> +    if ((cpe & 0x80) == 0) {
> +        printk(KERN_INFO "cpe:%#x\n", cpe);  <--------------     I'm NOT SEEING T
> 
> 
> +        cpe |= 0x80;
> +        barrier();
> +        WSR_CPENABLE(cpe);
> +    }
> 
> Since I'm not seeing them it looks like I'm not replicating your problem.
> Perhaps due to a difference between the DC233 and the CB570.
> 
> 
>> Although the audio dsp driver since to work as I excepted now,
>> so, I double that how to make sure the coprocessor management codes work ok?
> 
> You might change enable_mail_queue() to just  check the CPENABLE AFTER
> doing I/O to the port. You shouldn't get an excpetion crashing your application,
> it you do I'd like to see the backtrace. After using the I/O port the 1st time
> the CPENABLE should stay set for you task.
> 
> I wouldn't actually describe the User Task Region as going all of the way
> up to D000,0000. Shared memory and de-aliasing code uses part of the C000,0000
> region.
> 
> Your map is only showing one core, the CB570 I assume. Nothing about
> the map for the DC330. The CB570 reset vector goes to the SDRAM.
> 
> Have you looked into using kexec() to load the CB570?
> 
> 
>>  
>> By the way, as mentioned by benn,
>>  
>>  > 170 void __init zones_init(void)
>>  > 171 {
>>  > 172         unsigned long zones_size[MAX_NR_ZONES];
>>  > 173         int i;
>>  > 174
>>  > 175         for (i = 0; i  < MAX_NR_ZONES; i++)
>>  > 176                 zones_size[i] = 0;
>>  > 177
>>  > 178
>>  > 179         /* All pages are DMA-able, so we put them all in the DMA 
>> zone. */
>>  > 180         zones_size[ZONE_DMA] = max_low_pfn - ARCH_PFN_OFFSET;
>>  
>> Our mem static mapping is showed as follow, would you help us to double 
>> check it. thx
>>  
>>  
>> What is the value of max_low_pfn and ARCH_PFN_OFFSET?
> 
> The guys from Emlix checked in a diagram of this into the
> git log but I thought it was worthy of being in the code
> so folks like your self would see it without having to
> dig around. I put the diagram just before the beginning
> of bootmem_init() in arch/xtensa/mm/init.c at lines
> 101 thru      112.
> 
> ascii art doens't cut-n-past with my mail tool .
> 
> #define ARCH_PFN_OFFSET         (PLATFORM_DEFAULT_MEM_START >> PAGE_SHIFT)
> 
> You should be defining PLATFORM_DEFAULT_MEM_START to zero.
> That's the physical address of the lowest memory that's
> available for Linux to map.
> 
> max_low_pfn seems to be maximum Page Frame Number for low memory.
> 
> I'm not sure about this bit:
> 
>     135         max_low_pfn = max_pfn < MAX_MEM_PFN >> PAGE_SHIFT ? ?
>     136                 max_pfn : MAX_MEM_PFN >> PAGE_SHIFT;
> 
> I'd have used explicit parens and I don't see the shift a PFN by PAGE_SHIFT.
> 
> 	max_low_pfn = max_pfn < MAX_MEM_PFN >> PAGE_SHIFT  ?  max_pfn : MAX_MEM_PFN >> PAGE_SHIFT;;
> 
> Looks like MAX_MEM_PFN isn't a Page Frame Number at all, its a Phyical  Address Range,
> that explains the bizzare shift. Should be more like:
> 
> 	max_low_pfn = max_pfn  <  (MAX_MEM_PHYS_PAGE_RANGE >> PAGE_SHIFT)  ?  max_pfn : MAX_MEM_PHYS_PAGE_RANGE>> PAGE_SHIFT;;
> 	
> I wonder why we don't just shfit it on page.h, from:
> 	#define MAX_MEM_PFN             XCHAL_KSEG_SIZE
> to
> 	#define MAX_MEM_PFN             (XCHAL_KSEG_SIZE >> PAGE_SHIFT)
> 	
> In your case I expect your  XCHAL_KSEG_SIZE  is 0x08000000,
> and  since your using a MMU that MAX_MEM_PFN is 0x08000000.
> 
> You could have a hole in the memory map for the memory that the
> CB330 is going to use but I suspect that doing this with kexec()
> would likely be better. In this case you give all of the SDRAM
> to linux and let kexec() borrow some memory.  We would need to
> check if kexec() is flexible enough to do this.
> 
> It's 2:00am, time to head home.
> 
> -piet
> 
> -
> 
>>  
>>  
>>  
>> -------------------------------------------------------
>> With Regards,
>> Truby.Zong 
>>  ׯØȱß
>> -------------------------------------------------------
>> <mailto:truby.zhuang at chipsbank.com> 
>> ------------------------------------------------------------------------
>> *·¢¼þÈË£º* Piet Delaney
>> *·¢ËÍʱ¼ä£º* 2010-09-28  15:18:12
>> *ÊÕ¼þÈË£º* truby.zhuang
>> *³­ËÍ£º* Piet Delaney; benn.huang; leeking.liÀî »ÝÇä\\""; dengkexi; Tong 
>> Wang; mark li; Marc Gauthier; pietdelaney
>> *Ö÷Ì⣺* Re: Help us to open a builroot git repo
>> truby.zhuang wrote:
>>  > Hi,Piet:
>>  >  
>>  >   >Truby, could you push your changes that you mentioned on 09/26/201010
>>  >   
>>  >and let me know what problems, if any, your experiencing or have an issue
>>  >   >with.
>>  >  
>>  > We have push the newest audio dsp driver to git!
>>  
>> Ok I'll try it out. Last time nothing eventful happen.
>> If your having a problem it would be nice to hear about it.
>>  
>> -piet
>>  
>>  >  
>>  >  
>>  > -------------------------------------------------------
>>  > With Regards,
>>  > Truby.Zong 
>>  >  ׯØȱß
>>  > -------------------------------------------------------
>>  >  <mailto:truby.zhuang at chipsbank.com > 
>>  > ------------------------------------------------------------------------
>>  > *·¢¼þÈË£º* Piet Delaney
>>  > *·¢ËÍʱ¼ä£º* 2010-09-28  10:07:09
>>  > *ÊÕ¼þÈË£º* benn.huang
>>  > *³­ËÍ£º* Piet Delaney; leeking.liÀî »ÝÇä""; dengkexi; Tong Wang; mark 
>>  > li; truby.zhuang; Marc Gauthier; pietdelaney
>>  > *Ö÷Ì⣺* Re: Help us to open a builroot git repo
>>  > benn.huang wrote:
>>  >   > Hi Piet:
>>  >   >  
>>  >   > Could you please help us to copy a buildroot git repo from 
>>  >   > 'buildroot/buildroot-xtensa.git' repo at git.linux-kernel.org site?
>>  >  
>>  > Hi Benn:
>>  >  
>>  > A couple weeks ago I included you guys in the 'git' group  so you should be able
>>  > to just create a branch  and check it into an existing buildroot.
>>  > We already have quite a few buildroot git repo and suspect that  it's
>>  > not a good idea to add yet another.
>>  >  
>>  > How about using:
>>  >  
>>  > http://git.linux-xtensa.org/cgi-bin/git.cgi?p=buildroot/buildroot-xtensa-HiFi2-Snapshot.git;a=summary
>>  >  
>>  > You can see that Prasanna, over at Transwitch, just created a Ext2-Debug
>>  > branch for our working together on a ramfs problem they are having.
>>  >  
>>  >  
>>  >   > Just like 'kernel/xtensa-2.6.29-smp-chipsbank.git'¡£ As Tong Wang's 
>>  >   
>>  > suggestion, we will integrate the Qt(modified by chipsbank) library to 
>>  >   > the buildroot and push to the git repo. Thanks!
>>  >  
>>  > Would be easier to just create a 'qt' branch and include a README with
>>  > a description on the existing problems.
>>  >  
>>  > I'd suggest you make a branch off of snapshot_2+SMP which is what I've
>>  > been using recently. Eventually we need to merge with a more recent
>>  > buildroot snapshot. snapshot_2 was the second snapshot of buildroot.
>>  > The master branch is derived from a 3ed snapshot and has problems supporting
>>  > gcc and X11. Christian  and I have been trying to get a new snapshot working
>>  > that is more in sync with the folks at uclibc.org.
>>  >  
>>  > Once you check in the code and have it building up to the gcc build failure
>>  > post the description of the problem(s) to Marc and I, including a Cc: to
>>  > the related mailing lists 'linux-xtensa.org' and 'buildroot-xtensa-commits'.
>>  >  
>>  > This will provide an efficient platform for resolving you buildroot issues
>>  > in general, and with Qt and gcc in particular right now. For your other problems
>>  > like supporting the loading of the 330  HiFi core with the 570+MMU core I suggest
>>  > we use the same approach. Last week I suggested your considering using the kernel
>>  > kexec() system call. It's provided to load a kernel into memory from a ELF file and
>>  > not touching that memory. Seems like just what your looking for. For your XCC problem
>>  > I suggest we start with it being discussed here until we have enough information
>>  > to get XCC developers involved. Perhaps you need to get your Xtensa Tools upgrades
>>  > license updated to get the fixes once we sort out the problem. XCC seems like an
>>  > important tool to support with Xtensa-Linux, perhaps we should even have a separate
>>  > mailing list for discussing it. Anyway for now  how about we just use the xtensa-linux
>>  > and/or buildroot-xtensa mailing list(s).
>>  >  
>>  > Truby, could you push your changes that you mentioned on 09/26/201010
>>  > and let me know what problems, if any, your experiencing or have an issue
>>  > with.
>>  >  
>>  > On the module loading problems we have some changes on a branch of 2.6.29-smp
>>  > that seem mostly fine. We have a problem with some buildroots, likely with
>>  > the new binutils, with modules not initializing correctly.
>>  >  
>>  > -piet
>>  >  
>>  >   >  
>>  >   >  
>>  >   > Best regards,
>>  >   >  
>>  >   > 2010-09-26
>>  >   
>>  > ------------------------------------------------------------------------
>>  >   > Benn
>>  >  
>>  >  
>>  
>>  
> 
> 
> 



More information about the linux-xtensa mailing list