From mboxrd@z Thu Jan 1 00:00:00 1970 Date: Sun, 15 Nov 2015 19:43:59 +0100 From: Gilles Chanteperdrix Message-ID: <20151115184359.GW1820@hermes.click-hack.org> References: <20151114123953.GN1820@hermes.click-hack.org> <20151114182603.GS1820@hermes.click-hack.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Xenomai] [PATCH] IPIPE patch for BCM2709/RPI2. List-Id: Discussions about the Xenomai project List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mathieu Rondonneau Cc: xenomai@xenomai.org On Sat, Nov 14, 2015 at 04:14:04PM -0800, Mathieu Rondonneau wrote: > On 15-11-14 10:26 AM, Gilles Chanteperdrix wrote: > > On Sat, Nov 14, 2015 at 08:11:52AM -0800, Mathieu Rondonneau wrote: > >> On 15-11-14 04:39 AM, Gilles Chanteperdrix wrote: > >>> On Fri, Nov 13, 2015 at 09:31:23PM -0800, Mathieu Rondonneau wrote: > >>>> #ifdef CONFIG_OF > >>>> @@ -333,6 +349,10 @@ static struct irq_chip armctrl_chip = { > >>>> .irq_mask = armctrl_mask_irq, > >>>> .irq_unmask = armctrl_unmask_irq, > >>>> .irq_set_wake = armctrl_set_wake, > >>>> +#ifdef CONFIG_IPIPE > >>>> + .irq_hold = armctrl_mask_irq, > >>>> + .irq_release = armctrl_unmask_irq, > >>>> +#endif > >>> > >>> Also, as explained during your lengthy discussion, you do not need > >>> to define irq_hold and irq_release here. > >>> > >> Kernel does not boot if I remove them (also explained in our lengthy > >> discussion). I think that's the part we did not understood each other. > >> I am happy to remove them if there is another workaround to get the > >> kernel booting. > > > > I can not find the mail where you explain what happens when you > > remove it. Could you explain it again, please? > > > > Regards. > > > > the doc refers to handle_fasteoi_irq > (http://xenomai.org/2014/09/porting-xenomai-dual-kernel-to-a-new-arm-soc/#flow_handler) > but I think there are more corner cases (else if cases). There seems to > be a case also for the handle_percpu_devid_irq for fasteoi behavior (to > call mask/unmask). The code allows that only when irq_hold is defined. > > See the code from kernel/chip.c > > } else if (handle == handle_percpu_irq || > handle == handle_percpu_devid_irq) { > if (irq_desc_get_chip(desc) && > irq_desc_get_chip(desc)->irq_hold) { > desc->ipipe_ack = __ipipe_ack_fasteoi_irq; > desc->ipipe_end = __ipipe_end_fasteoi_irq; > } else { > desc->ipipe_ack = __ipipe_ack_percpu_irq; > desc->ipipe_end = __ipipe_nop_irq; > } > } > > In our earlier conversation you said this hack was for A9 only. > But it seems that this might be also needed for non-A9 (bcm2709) because > the mask and un-mask seems to be the one that are needed to be called. > this is the only reason I defined a irq_hold. > > Does irq_ack and irq_eoi are required for handle_percpu_devid_irq? > the doc does not cover that part and current platform does not use > those. Removing the irq_hold then fails because irq not getting handled. > > Currently handle_level_irq and handle_percpu_devid_irq share the same > irqchip, but that can not be the case if I define a irq_ack (irq_mask > will be called, then irq_ack, and then irq_unmask). > > So far defining irq_hold seems to be what is required for this platform > in order to call the mask/unmask without having two irqchip. > > I agree with you, this might look more like a hack. > > Is there another way to do this? >>From what I understand: - on A9, the irqchip is shared between fasteoi irq and percpu irq - on other platforms (where handling percpu irq was first introduced), the irqchip is shared between percpu irq and another flow handler, probably not level irqs - on your platform, percpu irq shares the irqchip with level irqs, __ipipe_ack_percpu_irq does not work because it does not mask the irq, and this can not work with level irqs. So, if implementing irq_hold works for this case, then let us keep it that way. The porting guide simply need to be updated for this case. -- Gilles. https://click-hack.org