From mboxrd@z Thu Jan 1 00:00:00 1970 From: miquel.raynal@bootlin.com (Miquel Raynal) Date: Fri, 28 Sep 2018 18:38:06 +0200 Subject: [PATCH v5 07/14] irqchip/irq-mvebu-sei: add new driver for Marvell SEI In-Reply-To: <9a0de5ac-8cef-0950-e218-0368167ecff8@arm.com> References: <20180830073535.10710-1-miquel.raynal@bootlin.com> <20180830073535.10710-8-miquel.raynal@bootlin.com> <86pnx7x5pz.wl-marc.zyngier@arm.com> <20180924180133.2ea93762@xps13> <9a0de5ac-8cef-0950-e218-0368167ecff8@arm.com> Message-ID: <20180928183806.176ca084@xps13> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Marc, Marc Zyngier wrote on Fri, 28 Sep 2018 11:25:32 +0100: > On 24/09/18 17:01, Miquel Raynal wrote: > > Hi Miquel, > > [...] > > > The difference is that at this stage, the irq_data->chip pointer of the > > SEI controller _parent_ (ie. the GIC's chip pointer) is not valid. I > > digged a lot in this direction during your vacations to find out what I > > missed, and I ended up calling back irq_alloc_irqs_parent(). > > > > If you have an idea of how to handle this properly, I am all ears! > > The most glaring problem is that you create a hierarchy that encompasses > the GIC, which is just wrong. The hierarchy cannot point to the GIC, > because it end-up as a multiplexer. > > This code sequence in the probe function is the root of all evil: > > /* Get a reference to the parent domain */ > parent = of_irq_find_parent(node); > if (!parent) { > dev_err(sei->dev, "Failed to find parent IRQ node\n"); > ret = -ENODEV; > goto dispose_irq; > } > > This is a GIC interrupt, which is the output line for the SEI block. > > parent_domain = irq_find_host(parent); > if (!parent_domain) { > dev_err(sei->dev, "Failed to find parent IRQ domain\n"); > ret = -ENODEV; > goto dispose_irq; > } > > That's the GIC domain. > > /* Create the 'wired' domain */ > sei->ap_domain = irq_domain_create_hierarchy(parent_domain, 0, > sei->caps->ap_range.size, > of_node_to_fwnode(node), > &mvebu_sei_ap_domain_ops, > sei); > if (!sei->ap_domain) { > dev_err(sei->dev, "Failed to create AP IRQ domain\n"); > ret = -ENOMEM; > goto dispose_irq; > } > > And here, you're saying "each and every AP SEI interrupt is directly > linked to a unique GIC interrupt". Nothing could be further from the > truth, since all SEI interrupts are funnelled through a *single* > GIC interrupt. So you cannot create it as a hierarchy parented at > the GIC. > > /* Create the 'MSI' domain */ > sei->cp_domain = irq_domain_create_hierarchy(parent_domain, 0, > sei->caps->cp_range.size, > of_node_to_fwnode(node), > &mvebu_sei_cp_domain_ops, > sei); > > > Same thing here. > > The issue here is that you're using the GIC domain as the way to root > the two distinct SEI domains, while they should be rooted at an internal, > SEI-specific domain. I'd suggest a topology like this: > > AP-SEI ---> S > E > Plat-MSI ---> CP-SEI ---> I > > CP-SEI and AP-SEI use SEI as a parent. SEI does not have a parent, but is > a chained irqchip. Thanks you very much for this detailed explanation. The above is what I intended to do, but maybe what I achieved is more something like: AP-SEI ---> G I Plat-MSI ---> CP-SEI ---> C And now I understand better what is bothering you since the beginning. > > I'm happy to help you reworking this piece of code if you tell me how to > plug a driver that can use it on an mcbin system. Next week I'll have another look at the driver, but it could be great if you could show me the big lines of how you imagine the rework. I prepared a branch based on 4.19-rc1 with: * The ICU/SEI series * The series adding support for thermal interrupts in the armada_thermal.c driver (it triggers a wired SEI when the AP is too hot or an MSI SEI when it is a CP). * The needed DT changes. https://github.com/miquelraynal/linux/tree/marvell/4.19-rc1/icu-sei Thanks, Miqu?l From mboxrd@z Thu Jan 1 00:00:00 1970 From: Miquel Raynal Subject: Re: [PATCH v5 07/14] irqchip/irq-mvebu-sei: add new driver for Marvell SEI Date: Fri, 28 Sep 2018 18:38:06 +0200 Message-ID: <20180928183806.176ca084@xps13> References: <20180830073535.10710-1-miquel.raynal@bootlin.com> <20180830073535.10710-8-miquel.raynal@bootlin.com> <86pnx7x5pz.wl-marc.zyngier@arm.com> <20180924180133.2ea93762@xps13> <9a0de5ac-8cef-0950-e218-0368167ecff8@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <9a0de5ac-8cef-0950-e218-0368167ecff8@arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Marc Zyngier Cc: Mark Rutland , Andrew Lunn , Jason Cooper , devicetree@vger.kernel.org, Antoine Tenart , Catalin Marinas , Gregory Clement , Haim Boot , Will Deacon , Maxime Chevallier , Nadav Haklai , Rob Herring , Thomas Petazzoni , Thomas Gleixner , Hanna Hawa , linux-arm-kernel@lists.infradead.org, Sebastian Hesselbarth List-Id: devicetree@vger.kernel.org SGkgTWFyYywKCk1hcmMgWnluZ2llciA8bWFyYy56eW5naWVyQGFybS5jb20+IHdyb3RlIG9uIEZy aSwgMjggU2VwIDIwMTggMTE6MjU6MzIKKzAxMDA6Cgo+IE9uIDI0LzA5LzE4IDE3OjAxLCBNaXF1 ZWwgUmF5bmFsIHdyb3RlOgo+IAo+IEhpIE1pcXVlbCwKPiAKPiBbLi4uXQo+IAo+ID4gVGhlIGRp ZmZlcmVuY2UgaXMgdGhhdCBhdCB0aGlzIHN0YWdlLCB0aGUgaXJxX2RhdGEtPmNoaXAgcG9pbnRl ciBvZiB0aGUKPiA+IFNFSSBjb250cm9sbGVyIF9wYXJlbnRfIChpZS4gdGhlIEdJQydzIGNoaXAg cG9pbnRlcikgaXMgbm90IHZhbGlkLiBJCj4gPiBkaWdnZWQgYSBsb3QgaW4gdGhpcyBkaXJlY3Rp b24gZHVyaW5nIHlvdXIgdmFjYXRpb25zIHRvIGZpbmQgb3V0IHdoYXQgSQo+ID4gbWlzc2VkLCBh bmQgSSBlbmRlZCB1cCBjYWxsaW5nIGJhY2sgaXJxX2FsbG9jX2lycXNfcGFyZW50KCkuCj4gPiAK PiA+IElmIHlvdSBoYXZlIGFuIGlkZWEgb2YgaG93IHRvIGhhbmRsZSB0aGlzIHByb3Blcmx5LCBJ IGFtIGFsbCBlYXJzISAgCj4gCj4gVGhlIG1vc3QgZ2xhcmluZyBwcm9ibGVtIGlzIHRoYXQgeW91 IGNyZWF0ZSBhIGhpZXJhcmNoeSB0aGF0IGVuY29tcGFzc2VzCj4gdGhlIEdJQywgd2hpY2ggaXMg anVzdCB3cm9uZy4gVGhlIGhpZXJhcmNoeSBjYW5ub3QgcG9pbnQgdG8gdGhlIEdJQywKPiBiZWNh dXNlIGl0IGVuZC11cCBhcyBhIG11bHRpcGxleGVyLgo+IAo+IFRoaXMgY29kZSBzZXF1ZW5jZSBp biB0aGUgcHJvYmUgZnVuY3Rpb24gaXMgdGhlIHJvb3Qgb2YgYWxsIGV2aWw6Cj4gCj4gCS8qIEdl dCBhIHJlZmVyZW5jZSB0byB0aGUgcGFyZW50IGRvbWFpbiAqLwo+IAlwYXJlbnQgPSBvZl9pcnFf ZmluZF9wYXJlbnQobm9kZSk7Cj4gCWlmICghcGFyZW50KSB7Cj4gCQlkZXZfZXJyKHNlaS0+ZGV2 LCAiRmFpbGVkIHRvIGZpbmQgcGFyZW50IElSUSBub2RlXG4iKTsKPiAJCXJldCA9IC1FTk9ERVY7 Cj4gCQlnb3RvIGRpc3Bvc2VfaXJxOwo+IAl9Cj4gCj4gVGhpcyBpcyBhIEdJQyBpbnRlcnJ1cHQs IHdoaWNoIGlzIHRoZSBvdXRwdXQgbGluZSBmb3IgdGhlIFNFSSBibG9jay4KPiAKPiAJcGFyZW50 X2RvbWFpbiA9IGlycV9maW5kX2hvc3QocGFyZW50KTsKPiAJaWYgKCFwYXJlbnRfZG9tYWluKSB7 Cj4gCQlkZXZfZXJyKHNlaS0+ZGV2LCAiRmFpbGVkIHRvIGZpbmQgcGFyZW50IElSUSBkb21haW5c biIpOwo+IAkJcmV0ID0gLUVOT0RFVjsKPiAJCWdvdG8gZGlzcG9zZV9pcnE7Cj4gCX0KPiAKPiBU aGF0J3MgdGhlIEdJQyBkb21haW4uCj4gCj4gCS8qIENyZWF0ZSB0aGUgJ3dpcmVkJyBkb21haW4g Ki8KPiAJc2VpLT5hcF9kb21haW4gPSBpcnFfZG9tYWluX2NyZWF0ZV9oaWVyYXJjaHkocGFyZW50 X2RvbWFpbiwgMCwKPiAJCQkJCQkgICAgIHNlaS0+Y2Fwcy0+YXBfcmFuZ2Uuc2l6ZSwKPiAJCQkJ CQkgICAgIG9mX25vZGVfdG9fZndub2RlKG5vZGUpLAo+IAkJCQkJCSAgICAgJm12ZWJ1X3NlaV9h cF9kb21haW5fb3BzLAo+IAkJCQkJCSAgICAgc2VpKTsKPiAJaWYgKCFzZWktPmFwX2RvbWFpbikg ewo+IAkJZGV2X2VycihzZWktPmRldiwgIkZhaWxlZCB0byBjcmVhdGUgQVAgSVJRIGRvbWFpblxu Iik7Cj4gCQlyZXQgPSAtRU5PTUVNOwo+IAkJZ290byBkaXNwb3NlX2lycTsKPiAJfQo+IAo+IEFu ZCBoZXJlLCB5b3UncmUgc2F5aW5nICJlYWNoIGFuZCBldmVyeSBBUCBTRUkgaW50ZXJydXB0IGlz IGRpcmVjdGx5Cj4gbGlua2VkIHRvIGEgdW5pcXVlIEdJQyBpbnRlcnJ1cHQiLiBOb3RoaW5nIGNv dWxkIGJlIGZ1cnRoZXIgZnJvbSB0aGUKPiB0cnV0aCwgc2luY2UgYWxsIFNFSSBpbnRlcnJ1cHRz IGFyZSBmdW5uZWxsZWQgdGhyb3VnaCBhICpzaW5nbGUqCj4gR0lDIGludGVycnVwdC4gU28geW91 IGNhbm5vdCBjcmVhdGUgaXQgYXMgYSBoaWVyYXJjaHkgcGFyZW50ZWQgYXQKPiB0aGUgR0lDLgo+ IAo+IAkvKiBDcmVhdGUgdGhlICdNU0knIGRvbWFpbiAqLwo+IAlzZWktPmNwX2RvbWFpbiA9IGly cV9kb21haW5fY3JlYXRlX2hpZXJhcmNoeShwYXJlbnRfZG9tYWluLCAwLAo+IAkJCQkJCSAgICAg c2VpLT5jYXBzLT5jcF9yYW5nZS5zaXplLAo+IAkJCQkJCSAgICAgb2Zfbm9kZV90b19md25vZGUo bm9kZSksCj4gCQkJCQkJICAgICAmbXZlYnVfc2VpX2NwX2RvbWFpbl9vcHMsCj4gCQkJCQkJICAg ICBzZWkpOwo+IAo+IAo+IFNhbWUgdGhpbmcgaGVyZS4KPiAKPiBUaGUgaXNzdWUgaGVyZSBpcyB0 aGF0IHlvdSdyZSB1c2luZyB0aGUgR0lDIGRvbWFpbiBhcyB0aGUgd2F5IHRvIHJvb3QKPiB0aGUg dHdvIGRpc3RpbmN0IFNFSSBkb21haW5zLCB3aGlsZSB0aGV5IHNob3VsZCBiZSByb290ZWQgYXQg YW4gaW50ZXJuYWwsCj4gU0VJLXNwZWNpZmljIGRvbWFpbi4gSSdkIHN1Z2dlc3QgYSB0b3BvbG9n eSBsaWtlIHRoaXM6Cj4gCj4gICAgICAgICAgICAgICAgICAgQVAtU0VJIC0tLT4gUwo+ICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgIEUKPiAgICAgUGxhdC1NU0kgLS0tPiBDUC1TRUkgLS0t PiBJCj4gCj4gQ1AtU0VJIGFuZCBBUC1TRUkgdXNlIFNFSSBhcyBhIHBhcmVudC4gU0VJIGRvZXMg bm90IGhhdmUgYSBwYXJlbnQsIGJ1dCBpcwo+IGEgY2hhaW5lZCBpcnFjaGlwLgoKVGhhbmtzIHlv dSB2ZXJ5IG11Y2ggZm9yIHRoaXMgZGV0YWlsZWQgZXhwbGFuYXRpb24uIFRoZSBhYm92ZSBpcyB3 aGF0IEkKaW50ZW5kZWQgdG8gZG8sIGJ1dCBtYXliZSB3aGF0IEkgYWNoaWV2ZWQgaXMgbW9yZSBz b21ldGhpbmcgbGlrZToKCiAgICAgICAgICAgICAgICAgICBBUC1TRUkgLS0tPiBHCiAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICBJCiAgICAgUGxhdC1NU0kgLS0tPiBDUC1TRUkgLS0tPiBD CgpBbmQgbm93IEkgdW5kZXJzdGFuZCBiZXR0ZXIgd2hhdCBpcyBib3RoZXJpbmcgeW91IHNpbmNl IHRoZSBiZWdpbm5pbmcuCgo+IAo+IEknbSBoYXBweSB0byBoZWxwIHlvdSByZXdvcmtpbmcgdGhp cyBwaWVjZSBvZiBjb2RlIGlmIHlvdSB0ZWxsIG1lIGhvdyB0bwo+IHBsdWcgYSBkcml2ZXIgdGhh dCBjYW4gdXNlIGl0IG9uIGFuIG1jYmluIHN5c3RlbS4KCk5leHQgd2VlayBJJ2xsIGhhdmUgYW5v dGhlciBsb29rIGF0IHRoZSBkcml2ZXIsIGJ1dCBpdCBjb3VsZCBiZSBncmVhdAppZiB5b3UgY291 bGQgc2hvdyBtZSB0aGUgYmlnIGxpbmVzIG9mIGhvdyB5b3UgaW1hZ2luZSB0aGUgcmV3b3JrLiBJ CnByZXBhcmVkIGEgYnJhbmNoIGJhc2VkIG9uIDQuMTktcmMxIHdpdGg6CiogVGhlIElDVS9TRUkg c2VyaWVzCiogVGhlIHNlcmllcyBhZGRpbmcgc3VwcG9ydCBmb3IgdGhlcm1hbCBpbnRlcnJ1cHRz IGluIHRoZQogIGFybWFkYV90aGVybWFsLmMgZHJpdmVyIChpdCB0cmlnZ2VycyBhIHdpcmVkIFNF SSB3aGVuIHRoZSBBUCBpcyB0b28KICBob3Qgb3IgYW4gTVNJIFNFSSB3aGVuIGl0IGlzIGEgQ1Ap LgoqIFRoZSBuZWVkZWQgRFQgY2hhbmdlcy4KCmh0dHBzOi8vZ2l0aHViLmNvbS9taXF1ZWxyYXlu YWwvbGludXgvdHJlZS9tYXJ2ZWxsLzQuMTktcmMxL2ljdS1zZWkKCgpUaGFua3MsCk1pcXXDqGwK Cl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmxpbnV4LWFy bS1rZXJuZWwgbWFpbGluZyBsaXN0CmxpbnV4LWFybS1rZXJuZWxAbGlzdHMuaW5mcmFkZWFkLm9y ZwpodHRwOi8vbGlzdHMuaW5mcmFkZWFkLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2xpbnV4LWFybS1r ZXJuZWwK