From: Robin Murphy <robin.murphy@arm.com> To: linucherian@gmail.com, catalin.marinas@arm.com, will.deacon@arm.com, lorenzo.pieralisi@arm.com, hanjun.guo@linaro.org, sudeep.holla@arm.com Cc: rjw@rjwysocki.net, lenb@kernel.org, joro@8bytes.org, robert.moore@intel.com, lv.zheng@intel.com, linux-arm-kernel@lists.infradead.org, linux-acpi@vger.kernel.org, iommu@lists.linux-foundation.org, devel@acpica.org, Sunil.Goutham@cavium.com, Geethasowjanya.Akula@cavium.com, robert.richter@cavium.com, linu.cherian@cavium.com, Geetha <gakula@cavium.com> Subject: Re: [RFC PATCH 3/7] iommu/arm-smmu-v3: Introduce smmu option USE_SHARED_IRQS for Silicon errata Date: Tue, 11 Apr 2017 16:54:26 +0100 [thread overview] Message-ID: <b84e2383-1a65-3c5a-8e2c-28eafdb20adc@arm.com> (raw) In-Reply-To: <1491921765-29475-4-git-send-email-linucherian@gmail.com> On 11/04/17 15:42, linucherian@gmail.com wrote: > From: Geetha <gakula@cavium.com> > > Cavium 99xx SMMU implementation doesn't not support unique irq lines for > gerror, eventq and cmdq-sync. USE_SHARED_IRQS option enables to use single > irq line for all three interrupts. AFAICS, there's nothing actually wrong with using shared wired IRQs - the architecture spec doesn't appear to say anything about it. I think it might suffice to simply add IRQF_SHARED if we can see the SMMU doesn't support MSIs anyway - it doesn't really seem like something we need to treat as a specific quirk. > Signed-off-by: Geetha Sowjanya <gakula@cavium.com> > --- > drivers/iommu/arm-smmu-v3.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c > index b326195..1475ad8 100644 > --- a/drivers/iommu/arm-smmu-v3.c > +++ b/drivers/iommu/arm-smmu-v3.c > @@ -415,6 +415,9 @@ > #define ARM_SMMU_PAGE0_REGS_ONLY(s) \ > ((s)->options & ARM_SMMU_OPT_PAGE0_REGS_ONLY) > > +#define ARM_SMMU_USE_SHARED_IRQS(s) \ > + ((s)->options & ARM_SMMU_OPT_USE_SHARED_IRQS) > + > static bool disable_bypass; > module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO); > MODULE_PARM_DESC(disable_bypass, > @@ -601,6 +604,7 @@ struct arm_smmu_device { > > #define ARM_SMMU_OPT_SKIP_PREFETCH (1 << 0) > #define ARM_SMMU_OPT_PAGE0_REGS_ONLY (1 << 1) > +#define ARM_SMMU_OPT_USE_SHARED_IRQS (1 << 2) > u32 options; > > struct arm_smmu_cmdq cmdq; > @@ -668,6 +672,7 @@ struct arm_smmu_option_prop { > static struct arm_smmu_option_prop arm_smmu_options[] = { > { ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" }, > { ARM_SMMU_OPT_PAGE0_REGS_ONLY, "cavium-cn99xx,broken-page1-regspace"}, > + { ARM_SMMU_OPT_USE_SHARED_IRQS, "cavium-cn99xx,broken-unique-irqlines"}, > { 0, NULL}, > }; > > @@ -2237,6 +2242,7 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu) > { > int ret, irq; > u32 irqen_flags = IRQ_CTRL_EVTQ_IRQEN | IRQ_CTRL_GERROR_IRQEN; > + u32 irqflags = IRQF_ONESHOT | IRQF_SHARED; Either way, this is a really ugly way to go about it - I'd much rather initialise the common base value: u32 irqflags = (definitely not an MSI) ? IRQF_SHARED : 0; > > /* Disable IRQs first */ > ret = arm_smmu_write_reg_sync(smmu, 0, ARM_SMMU_IRQ_CTRL, > @@ -2251,9 +2257,11 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu) > /* Request interrupt lines */ > irq = smmu->evtq.q.irq; > if (irq) { > + if (!ARM_SMMU_USE_SHARED_IRQS(smmu)) > + irqflags = IRQF_ONESHOT; > ret = devm_request_threaded_irq(smmu->dev, irq, NULL, > arm_smmu_evtq_thread, > - IRQF_ONESHOT, ...and just pass irqflags | IRQF_ONESHOT here (and irqflags elsewhere), without all the horrible copy-paste conditions. Robin. > + irqflags, > "arm-smmu-v3-evtq", smmu); > if (ret < 0) > dev_warn(smmu->dev, "failed to enable evtq irq\n"); > @@ -2261,8 +2269,10 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu) > > irq = smmu->cmdq.q.irq; > if (irq) { > + if (!ARM_SMMU_USE_SHARED_IRQS(smmu)) > + irqflags = 0; > ret = devm_request_irq(smmu->dev, irq, > - arm_smmu_cmdq_sync_handler, 0, > + arm_smmu_cmdq_sync_handler, irqflags, > "arm-smmu-v3-cmdq-sync", smmu); > if (ret < 0) > dev_warn(smmu->dev, "failed to enable cmdq-sync irq\n"); > @@ -2270,8 +2280,10 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu) > > irq = smmu->gerr_irq; > if (irq) { > + if (!ARM_SMMU_USE_SHARED_IRQS(smmu)) > + irqflags = 0; > ret = devm_request_irq(smmu->dev, irq, arm_smmu_gerror_handler, > - 0, "arm-smmu-v3-gerror", smmu); > + irqflags, "arm-smmu-v3-gerror", smmu); > if (ret < 0) > dev_warn(smmu->dev, "failed to enable gerror irq\n"); > } >
WARNING: multiple messages have this Message-ID (diff)
From: robin.murphy@arm.com (Robin Murphy) To: linux-arm-kernel@lists.infradead.org Subject: [RFC PATCH 3/7] iommu/arm-smmu-v3: Introduce smmu option USE_SHARED_IRQS for Silicon errata Date: Tue, 11 Apr 2017 16:54:26 +0100 [thread overview] Message-ID: <b84e2383-1a65-3c5a-8e2c-28eafdb20adc@arm.com> (raw) In-Reply-To: <1491921765-29475-4-git-send-email-linucherian@gmail.com> On 11/04/17 15:42, linucherian at gmail.com wrote: > From: Geetha <gakula@cavium.com> > > Cavium 99xx SMMU implementation doesn't not support unique irq lines for > gerror, eventq and cmdq-sync. USE_SHARED_IRQS option enables to use single > irq line for all three interrupts. AFAICS, there's nothing actually wrong with using shared wired IRQs - the architecture spec doesn't appear to say anything about it. I think it might suffice to simply add IRQF_SHARED if we can see the SMMU doesn't support MSIs anyway - it doesn't really seem like something we need to treat as a specific quirk. > Signed-off-by: Geetha Sowjanya <gakula@cavium.com> > --- > drivers/iommu/arm-smmu-v3.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c > index b326195..1475ad8 100644 > --- a/drivers/iommu/arm-smmu-v3.c > +++ b/drivers/iommu/arm-smmu-v3.c > @@ -415,6 +415,9 @@ > #define ARM_SMMU_PAGE0_REGS_ONLY(s) \ > ((s)->options & ARM_SMMU_OPT_PAGE0_REGS_ONLY) > > +#define ARM_SMMU_USE_SHARED_IRQS(s) \ > + ((s)->options & ARM_SMMU_OPT_USE_SHARED_IRQS) > + > static bool disable_bypass; > module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO); > MODULE_PARM_DESC(disable_bypass, > @@ -601,6 +604,7 @@ struct arm_smmu_device { > > #define ARM_SMMU_OPT_SKIP_PREFETCH (1 << 0) > #define ARM_SMMU_OPT_PAGE0_REGS_ONLY (1 << 1) > +#define ARM_SMMU_OPT_USE_SHARED_IRQS (1 << 2) > u32 options; > > struct arm_smmu_cmdq cmdq; > @@ -668,6 +672,7 @@ struct arm_smmu_option_prop { > static struct arm_smmu_option_prop arm_smmu_options[] = { > { ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" }, > { ARM_SMMU_OPT_PAGE0_REGS_ONLY, "cavium-cn99xx,broken-page1-regspace"}, > + { ARM_SMMU_OPT_USE_SHARED_IRQS, "cavium-cn99xx,broken-unique-irqlines"}, > { 0, NULL}, > }; > > @@ -2237,6 +2242,7 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu) > { > int ret, irq; > u32 irqen_flags = IRQ_CTRL_EVTQ_IRQEN | IRQ_CTRL_GERROR_IRQEN; > + u32 irqflags = IRQF_ONESHOT | IRQF_SHARED; Either way, this is a really ugly way to go about it - I'd much rather initialise the common base value: u32 irqflags = (definitely not an MSI) ? IRQF_SHARED : 0; > > /* Disable IRQs first */ > ret = arm_smmu_write_reg_sync(smmu, 0, ARM_SMMU_IRQ_CTRL, > @@ -2251,9 +2257,11 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu) > /* Request interrupt lines */ > irq = smmu->evtq.q.irq; > if (irq) { > + if (!ARM_SMMU_USE_SHARED_IRQS(smmu)) > + irqflags = IRQF_ONESHOT; > ret = devm_request_threaded_irq(smmu->dev, irq, NULL, > arm_smmu_evtq_thread, > - IRQF_ONESHOT, ...and just pass irqflags | IRQF_ONESHOT here (and irqflags elsewhere), without all the horrible copy-paste conditions. Robin. > + irqflags, > "arm-smmu-v3-evtq", smmu); > if (ret < 0) > dev_warn(smmu->dev, "failed to enable evtq irq\n"); > @@ -2261,8 +2269,10 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu) > > irq = smmu->cmdq.q.irq; > if (irq) { > + if (!ARM_SMMU_USE_SHARED_IRQS(smmu)) > + irqflags = 0; > ret = devm_request_irq(smmu->dev, irq, > - arm_smmu_cmdq_sync_handler, 0, > + arm_smmu_cmdq_sync_handler, irqflags, > "arm-smmu-v3-cmdq-sync", smmu); > if (ret < 0) > dev_warn(smmu->dev, "failed to enable cmdq-sync irq\n"); > @@ -2270,8 +2280,10 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu) > > irq = smmu->gerr_irq; > if (irq) { > + if (!ARM_SMMU_USE_SHARED_IRQS(smmu)) > + irqflags = 0; > ret = devm_request_irq(smmu->dev, irq, arm_smmu_gerror_handler, > - 0, "arm-smmu-v3-gerror", smmu); > + irqflags, "arm-smmu-v3-gerror", smmu); > if (ret < 0) > dev_warn(smmu->dev, "failed to enable gerror irq\n"); > } >
next prev parent reply other threads:[~2017-04-11 15:54 UTC|newest] Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-04-11 14:42 [RFC PATCH 0/7] Cavium CN99xx SMMUv3 Errata workarounds linucherian-Re5JQEeQqe8AvxtiuMwx3w 2017-04-11 14:42 ` linucherian at gmail.com 2017-04-11 14:42 ` [RFC PATCH 1/7] iommu/arm-smmu-v3: Introduce smmu option PAGE0_REGS_ONLY for Silicon errata linucherian 2017-04-11 14:42 ` linucherian at gmail.com 2017-04-11 15:42 ` Robin Murphy 2017-04-11 15:42 ` Robin Murphy 2017-04-12 5:05 ` Linu Cherian 2017-04-12 5:05 ` Linu Cherian 2017-04-11 14:42 ` [RFC PATCH 2/7] iommu/arm-smmu-v3: Do resource size checks based on smmu option PAGE0_REGS_ONLY linucherian 2017-04-11 14:42 ` linucherian at gmail.com 2017-04-11 15:43 ` Robin Murphy 2017-04-11 15:43 ` Robin Murphy 2017-04-11 16:39 ` Sunil Kovvuri 2017-04-11 16:39 ` Sunil Kovvuri 2017-04-11 14:42 ` [RFC PATCH 3/7] iommu/arm-smmu-v3: Introduce smmu option USE_SHARED_IRQS for Silicon errata linucherian 2017-04-11 14:42 ` linucherian at gmail.com 2017-04-11 15:54 ` Robin Murphy [this message] 2017-04-11 15:54 ` Robin Murphy 2017-04-11 16:21 ` Will Deacon 2017-04-11 16:21 ` Will Deacon [not found] ` <20170411162123.GF17109-5wv7dgnIgG8@public.gmane.org> 2017-04-11 16:34 ` Sunil Kovvuri 2017-04-11 16:34 ` Sunil Kovvuri 2017-04-11 16:38 ` Robin Murphy 2017-04-11 16:38 ` Robin Murphy [not found] ` <a971af83-10f1-5696-f0c6-0600c04705c3-5wv7dgnIgG8@public.gmane.org> 2017-04-11 16:41 ` Will Deacon 2017-04-11 16:41 ` Will Deacon 2017-04-11 14:42 ` [RFC PATCH 4/7] ACPICA: IORT: Add SMMuV3 model definitions linucherian 2017-04-11 14:42 ` linucherian at gmail.com 2017-04-11 15:59 ` Robin Murphy 2017-04-11 15:59 ` Robin Murphy 2017-04-11 16:57 ` Sunil Kovvuri 2017-04-11 16:57 ` Sunil Kovvuri 2017-04-12 2:33 ` Hanjun Guo 2017-04-12 2:33 ` [Devel] " Hanjun Guo 2017-04-12 2:33 ` Hanjun Guo 2017-04-12 15:21 ` Lorenzo Pieralisi 2017-04-12 15:21 ` Lorenzo Pieralisi 2017-04-11 14:42 ` [RFC PATCH 5/7] iommu/arm-smmu-v3: For ACPI based device probing, set relevant options for different SMMUv3 implementations linucherian 2017-04-11 14:42 ` linucherian at gmail.com 2017-04-12 8:43 ` Robert Richter 2017-04-12 8:43 ` Robert Richter 2017-04-12 10:32 ` Linu Cherian 2017-04-12 10:32 ` Linu Cherian 2017-04-11 14:42 ` [RFC PATCH 6/7] ACPI/IORT: Fixup SMMUv3 resource size for Cavium 99xx SMMUv3 model linucherian 2017-04-11 14:42 ` linucherian at gmail.com 2017-04-11 14:42 ` [RFC PATCH 7/7] arm64: Documentation: Add Cavium ThunderX2 SMMUv3 erratas linucherian 2017-04-11 14:42 ` linucherian at gmail.com 2017-04-11 16:30 ` [RFC PATCH 0/7] Cavium CN99xx SMMUv3 Errata workarounds Will Deacon 2017-04-11 16:30 ` Will Deacon
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=b84e2383-1a65-3c5a-8e2c-28eafdb20adc@arm.com \ --to=robin.murphy@arm.com \ --cc=Geethasowjanya.Akula@cavium.com \ --cc=Sunil.Goutham@cavium.com \ --cc=catalin.marinas@arm.com \ --cc=devel@acpica.org \ --cc=gakula@cavium.com \ --cc=hanjun.guo@linaro.org \ --cc=iommu@lists.linux-foundation.org \ --cc=joro@8bytes.org \ --cc=lenb@kernel.org \ --cc=linu.cherian@cavium.com \ --cc=linucherian@gmail.com \ --cc=linux-acpi@vger.kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=lorenzo.pieralisi@arm.com \ --cc=lv.zheng@intel.com \ --cc=rjw@rjwysocki.net \ --cc=robert.moore@intel.com \ --cc=robert.richter@cavium.com \ --cc=sudeep.holla@arm.com \ --cc=will.deacon@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.