From: Marc Zyngier <maz@kernel.org>
To: Mark-PK Tsai <mark-pk.tsai@mediatek.com>
Cc: devicetree@vger.kernel.org, alix.wu@mediatek.com,
jason@lakedaemon.net, yj.chiang@mediatek.com, daniel@0x0f.com,
linux-kernel@vger.kernel.org, robh+dt@kernel.org,
linux-mediatek@lists.infradead.org, matthias.bgg@gmail.com,
tglx@linutronix.de, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/2] irqchip: irq-mst: Add MStar interrupt controller support
Date: Wed, 19 Aug 2020 14:42:48 +0100 [thread overview]
Message-ID: <f593f5e395c8558657a3f265b7038ec3@kernel.org> (raw)
In-Reply-To: <a8ee65eb4d86963bd2b56e86eec0ab3e@kernel.org> (raw)
On 2020-08-19 14:31, Mark-PK Tsai wrote:
> From: Marc Zyngier <maz@kernel.org>
>
>> > +
>> > +static int mst_intc_domain_alloc(struct irq_domain *domain, unsigned
>> > int virq,
>> > + unsigned int nr_irqs, void *data)
>> > +{
>> > + int i;
>> > + irq_hw_number_t hwirq;
>> > + struct irq_fwspec parent_fwspec, *fwspec = data;
>> > + struct mst_intc_chip_data *cd = (struct mst_intc_chip_data
>> > *)domain->host_data;
>>
>> No cast necessary here.
>>
>> > +
>> > + /* Not GIC compliant */
>> > + if (fwspec->param_count != 3)
>> > + return -EINVAL;
>> > +
>> > + /* No PPI should point to this domain */
>> > + if (fwspec->param[0])
>> > + return -EINVAL;
>> > +
>> > + if (fwspec->param[1] >= cd->nr_irqs)
>>
>> This condition is bogus, as it doesn't take into account the nr_irqs
>> parameter.
>>
>
>
> The hwirq number need to be in the irq map range. (property:
> mstar,irqs-map-range)
> If it's not, it must be incorrect configuration.
I agree. And since you are checking whether the configuration is
correct,
it'd better be completely correct.
> So how about use the condition as following?
>
> if (hwirq >= cd->nr_irqs)
> return -EINVAL;
Again, this says nothing of the validity of (hwirq + nr_irqs - 1)...
>
>> > + return -EINVAL;
>> > +
>> > + hwirq = fwspec->param[1];
>> > + for (i = 0; i < nr_irqs; i++)
>> > + irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
>> > + &mst_intc_chip,
>> > + domain->host_data);
... which you are using here.
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-08-19 13:44 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-19 3:42 [PATCH 0/2] Add MStar interrupt controller support Mark-PK Tsai
2020-08-19 3:42 ` [PATCH 1/2] irqchip: irq-mst: " Mark-PK Tsai
2020-08-19 8:14 ` Marc Zyngier
2020-08-19 13:31 ` Mark-PK Tsai
2020-08-19 13:42 ` Marc Zyngier [this message]
2020-08-19 14:55 ` Mark-PK Tsai
2020-08-19 15:24 ` Marc Zyngier
2020-08-19 3:42 ` [PATCH 2/2] dt-bindings: interrupt-controller: Add MStar interrupt controller Mark-PK Tsai
2020-08-25 21:48 ` Rob Herring
2020-08-26 7:50 ` Mark-PK Tsai
2020-08-19 15:37 ` [PATCH v2 1/2] irqchip: irq-mst: Add MStar interrupt controller support Mark-PK Tsai
2020-08-20 12:36 ` Daniel Palmer
2020-08-20 12:47 ` Marc Zyngier
2020-09-02 6:59 ` Mark-PK Tsai
2020-08-22 4:48 ` Daniel Palmer
2020-08-24 2:36 ` Mark-PK Tsai
2020-08-24 2:48 ` [PATCH] MAINTAINERS: Add maintenance information for MStar Interrupt Controller Mark-PK Tsai
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=f593f5e395c8558657a3f265b7038ec3@kernel.org \
--to=maz@kernel.org \
--cc=alix.wu@mediatek.com \
--cc=daniel@0x0f.com \
--cc=devicetree@vger.kernel.org \
--cc=jason@lakedaemon.net \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=mark-pk.tsai@mediatek.com \
--cc=matthias.bgg@gmail.com \
--cc=robh+dt@kernel.org \
--cc=tglx@linutronix.de \
--cc=yj.chiang@mediatek.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).