From mboxrd@z Thu Jan 1 00:00:00 1970 From: thomas.petazzoni@bootlin.com (Thomas Petazzoni) Date: Wed, 2 May 2018 10:13:00 +0200 Subject: [PATCH 09/17] irqchip/irq-mvebu-icu: support ICU subnodes In-Reply-To: <20180421135537.24716-10-miquel.raynal@bootlin.com> References: <20180421135537.24716-1-miquel.raynal@bootlin.com> <20180421135537.24716-10-miquel.raynal@bootlin.com> Message-ID: <20180502101300.49cc3623@windsurf.home> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hello Miqu?l, On Sat, 21 Apr 2018 15:55:29 +0200, Miquel Raynal wrote: > Introduce new bindings for the ICU. Perhaps this should explain *why* we need new bindings. > Each DT subnode of the ICU represents a type of interrupt that should > be handled separately. Add the possibility for the ICU to have subnodes > and probe each of them automatically with devm_platform_populate(). If > the node as no child, the probe function for NSRs will still be called > 'manually'. ... in order to preserve backward compatibility with Device Trees using the old binding. > +static struct mvebu_icu *mvebu_dev_get_drvdata(struct platform_device *pdev) The function should be prefixed by mvebu_icu_, not just mvebu_. > +{ > + struct mvebu_icu *icu; > + > + icu = dev_get_drvdata(&pdev->dev); > + if (icu) { > + /* Legacy bindings: get the device data */ I find this comment weird, because it doesn't document what the test just below is doing. > + if (!icu->legacy_bindings) > + return ERR_PTR(-EINVAL); > + } else { > + /* New bindings: get the parent device (ICU) data */ > + icu = dev_get_drvdata(pdev->dev.parent); > + if (!icu) > + return ERR_PTR(-ENODEV); > + if (icu->legacy_bindings) > + return ERR_PTR(-EINVAL); > + } > @@ -144,7 +170,10 @@ mvebu_icu_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, > goto free_irqd; > } > > - icu_irqd->icu_group = fwspec->param[0]; > + if (icu->legacy_bindings) > + icu_irqd->icu_group = fwspec->param[0]; > + else > + icu_irqd->icu_group = ICU_GRP_NSR; In practice here fwspec->param[0] is always going to be equal to ICU_GRP_NSR, but OK, the test makes sense as in a future commit, the "else" case will be changed to support SEIs. > +static const struct of_device_id mvebu_icu_nsr_of_match[] = { > + { .compatible = "marvell,cp110-icu-nsr", }, > + {}, > +}; > + > +static struct platform_driver mvebu_icu_nsr_driver = { > + .probe = mvebu_icu_nsr_probe, > + .driver = { > + .name = "mvebu-icu-nsr", > + .of_match_table = mvebu_icu_nsr_of_match, > + }, > +}; > +builtin_platform_driver(mvebu_icu_nsr_driver); I'm not sure why you call this icu_nsr here, and change it later to icu_subset. Wouldn't it make sense to call it right away with the final name ? Note that this is not a very strong request to change this aspect, I'm fine with how it's done today, it's just that I would have done it differently. Other than that, looks good to me. Thomas -- Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Petazzoni Subject: Re: [PATCH 09/17] irqchip/irq-mvebu-icu: support ICU subnodes Date: Wed, 2 May 2018 10:13:00 +0200 Message-ID: <20180502101300.49cc3623@windsurf.home> References: <20180421135537.24716-1-miquel.raynal@bootlin.com> <20180421135537.24716-10-miquel.raynal@bootlin.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20180421135537.24716-10-miquel.raynal@bootlin.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: Miquel Raynal Cc: Mark Rutland , Andrew Lunn , Jason Cooper , devicetree@vger.kernel.org, Marc Zyngier , Catalin Marinas , Gregory Clement , Haim Boot , Will Deacon , Maxime Chevallier , Nadav Haklai , Antoine Tenart , Rob Herring , Thomas Gleixner , Hanna Hawa , linux-arm-kernel@lists.infradead.org, Sebastian Hesselbarth List-Id: devicetree@vger.kernel.org SGVsbG8gTWlxdcOobCwKCk9uIFNhdCwgMjEgQXByIDIwMTggMTU6NTU6MjkgKzAyMDAsIE1pcXVl bCBSYXluYWwgd3JvdGU6Cj4gSW50cm9kdWNlIG5ldyBiaW5kaW5ncyBmb3IgdGhlIElDVS4KClBl cmhhcHMgdGhpcyBzaG91bGQgZXhwbGFpbiAqd2h5KiB3ZSBuZWVkIG5ldyBiaW5kaW5ncy4KCj4g RWFjaCBEVCBzdWJub2RlIG9mIHRoZSBJQ1UgcmVwcmVzZW50cyBhIHR5cGUgb2YgaW50ZXJydXB0 IHRoYXQgc2hvdWxkCj4gYmUgaGFuZGxlZCBzZXBhcmF0ZWx5LiBBZGQgdGhlIHBvc3NpYmlsaXR5 IGZvciB0aGUgSUNVIHRvIGhhdmUgc3Vibm9kZXMKPiBhbmQgcHJvYmUgZWFjaCBvZiB0aGVtIGF1 dG9tYXRpY2FsbHkgd2l0aCBkZXZtX3BsYXRmb3JtX3BvcHVsYXRlKCkuIElmCj4gdGhlIG5vZGUg YXMgbm8gY2hpbGQsIHRoZSBwcm9iZSBmdW5jdGlvbiBmb3IgTlNScyB3aWxsIHN0aWxsIGJlIGNh bGxlZAo+ICdtYW51YWxseScuCgogLi4uIGluIG9yZGVyIHRvIHByZXNlcnZlIGJhY2t3YXJkIGNv bXBhdGliaWxpdHkgd2l0aCBEZXZpY2UgVHJlZXMKIHVzaW5nIHRoZSBvbGQgYmluZGluZy4KCj4g K3N0YXRpYyBzdHJ1Y3QgbXZlYnVfaWN1ICptdmVidV9kZXZfZ2V0X2RydmRhdGEoc3RydWN0IHBs YXRmb3JtX2RldmljZSAqcGRldikKClRoZSBmdW5jdGlvbiBzaG91bGQgYmUgcHJlZml4ZWQgYnkg bXZlYnVfaWN1Xywgbm90IGp1c3QgbXZlYnVfLgoKPiArewo+ICsJc3RydWN0IG12ZWJ1X2ljdSAq aWN1Owo+ICsKPiArCWljdSA9IGRldl9nZXRfZHJ2ZGF0YSgmcGRldi0+ZGV2KTsKPiArCWlmIChp Y3UpIHsKPiArCQkvKiBMZWdhY3kgYmluZGluZ3M6IGdldCB0aGUgZGV2aWNlIGRhdGEgKi8KCkkg ZmluZCB0aGlzIGNvbW1lbnQgd2VpcmQsIGJlY2F1c2UgaXQgZG9lc24ndCBkb2N1bWVudCB3aGF0 IHRoZSB0ZXN0Cmp1c3QgYmVsb3cgaXMgZG9pbmcuCgo+ICsJCWlmICghaWN1LT5sZWdhY3lfYmlu ZGluZ3MpCj4gKwkJCXJldHVybiBFUlJfUFRSKC1FSU5WQUwpOwo+ICsJfSBlbHNlIHsKPiArCQkv KiBOZXcgYmluZGluZ3M6IGdldCB0aGUgcGFyZW50IGRldmljZSAoSUNVKSBkYXRhICovCj4gKwkJ aWN1ID0gZGV2X2dldF9kcnZkYXRhKHBkZXYtPmRldi5wYXJlbnQpOwo+ICsJCWlmICghaWN1KQo+ ICsJCQlyZXR1cm4gRVJSX1BUUigtRU5PREVWKTsKPiArCQlpZiAoaWN1LT5sZWdhY3lfYmluZGlu Z3MpCj4gKwkJCXJldHVybiBFUlJfUFRSKC1FSU5WQUwpOwo+ICsJfQoKCj4gQEAgLTE0NCw3ICsx NzAsMTAgQEAgbXZlYnVfaWN1X2lycV9kb21haW5fYWxsb2Moc3RydWN0IGlycV9kb21haW4gKmRv bWFpbiwgdW5zaWduZWQgaW50IHZpcnEsCj4gIAkJZ290byBmcmVlX2lycWQ7Cj4gIAl9Cj4gIAo+ IC0JaWN1X2lycWQtPmljdV9ncm91cCA9IGZ3c3BlYy0+cGFyYW1bMF07Cj4gKwlpZiAoaWN1LT5s ZWdhY3lfYmluZGluZ3MpCj4gKwkJaWN1X2lycWQtPmljdV9ncm91cCA9IGZ3c3BlYy0+cGFyYW1b MF07Cj4gKwllbHNlCj4gKwkJaWN1X2lycWQtPmljdV9ncm91cCA9IElDVV9HUlBfTlNSOwoKSW4g cHJhY3RpY2UgaGVyZSBmd3NwZWMtPnBhcmFtWzBdIGlzIGFsd2F5cyBnb2luZyB0byBiZSBlcXVh bCB0bwpJQ1VfR1JQX05TUiwgYnV0IE9LLCB0aGUgdGVzdCBtYWtlcyBzZW5zZSBhcyBpbiBhIGZ1 dHVyZSBjb21taXQsIHRoZQoiZWxzZSIgY2FzZSB3aWxsIGJlIGNoYW5nZWQgdG8gc3VwcG9ydCBT RUlzLgoKPiArc3RhdGljIGNvbnN0IHN0cnVjdCBvZl9kZXZpY2VfaWQgbXZlYnVfaWN1X25zcl9v Zl9tYXRjaFtdID0gewo+ICsJeyAuY29tcGF0aWJsZSA9ICJtYXJ2ZWxsLGNwMTEwLWljdS1uc3Ii LCB9LAo+ICsJe30sCj4gK307Cj4gKwo+ICtzdGF0aWMgc3RydWN0IHBsYXRmb3JtX2RyaXZlciBt dmVidV9pY3VfbnNyX2RyaXZlciA9IHsKPiArCS5wcm9iZSAgPSBtdmVidV9pY3VfbnNyX3Byb2Jl LAo+ICsJLmRyaXZlciA9IHsKPiArCQkubmFtZSA9ICJtdmVidS1pY3UtbnNyIiwKPiArCQkub2Zf bWF0Y2hfdGFibGUgPSBtdmVidV9pY3VfbnNyX29mX21hdGNoLAo+ICsJfSwKPiArfTsKPiArYnVp bHRpbl9wbGF0Zm9ybV9kcml2ZXIobXZlYnVfaWN1X25zcl9kcml2ZXIpOwoKSSdtIG5vdCBzdXJl IHdoeSB5b3UgY2FsbCB0aGlzIGljdV9uc3IgaGVyZSwgYW5kIGNoYW5nZSBpdCBsYXRlciB0bwpp Y3Vfc3Vic2V0LiBXb3VsZG4ndCBpdCBtYWtlIHNlbnNlIHRvIGNhbGwgaXQgcmlnaHQgYXdheSB3 aXRoIHRoZSBmaW5hbApuYW1lID8gTm90ZSB0aGF0IHRoaXMgaXMgbm90IGEgdmVyeSBzdHJvbmcg cmVxdWVzdCB0byBjaGFuZ2UgdGhpcwphc3BlY3QsIEknbSBmaW5lIHdpdGggaG93IGl0J3MgZG9u ZSB0b2RheSwgaXQncyBqdXN0IHRoYXQgSSB3b3VsZCBoYXZlCmRvbmUgaXQgZGlmZmVyZW50bHku CgpPdGhlciB0aGFuIHRoYXQsIGxvb2tzIGdvb2QgdG8gbWUuCgpUaG9tYXMKLS0gClRob21hcyBQ ZXRhenpvbmksIENUTywgQm9vdGxpbiAoZm9ybWVybHkgRnJlZSBFbGVjdHJvbnMpCkVtYmVkZGVk IExpbnV4IGFuZCBLZXJuZWwgZW5naW5lZXJpbmcKaHR0cHM6Ly9ib290bGluLmNvbQoKX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KbGludXgtYXJtLWtlcm5l bCBtYWlsaW5nIGxpc3QKbGludXgtYXJtLWtlcm5lbEBsaXN0cy5pbmZyYWRlYWQub3JnCmh0dHA6 Ly9saXN0cy5pbmZyYWRlYWQub3JnL21haWxtYW4vbGlzdGluZm8vbGludXgtYXJtLWtlcm5lbAo=