From: majun258@huawei•com (majun (F))
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v4 2/2] dt-binding:Documents of the mbigen bindings
Date: Wed, 23 Sep 2015 15:24:46 +0800 [thread overview]
Message-ID: <560253BE.8030505@huawei.com> (raw)
In-Reply-To: <20150922154120.18634b41@arm.com>
? 2015/9/22 22:41, Marc Zyngier ??:
> On Tue, 22 Sep 2015 19:35:38 +0800
> "majun (F)" <majun258@huawei•com> wrote:
>
[...]
>>>> +Examples:
>>>> + smmu_dsa {
>>>> + compatible = "arm,smmu-v3";
>>>> + reg = <0x0 0xc0040000 0x0 0x20000>;
>>>> + interrupt-parent = <&mbigen_dsa>;
>>>> + interrupts = <0x40b20 6 78 1>,
>>>> + <0x40b20 6 79 1>,
>>>> + <0x40b20 6 80 1>;
>>>> + };
>>>> +
>>>
>>> I find the current split very confusing. In your example, the interrupt
>>> controller is the mbigen block, which forces you to encode the DevID as
>>> part of the interrupt specifier. This doesn't feel like an ideal
>>> design, because you end up duplicating the DevID information at both
>>> the "client" device and the mbi_device.
>>>
>>> I'd be more inclined to have the mbi_device itself be the interrupt
>>> controller for the client device. This would eliminate information
>>> duplication, and reflect the hardware (or what I understand of the
>>> hardware) a bit more.
>>
>> Do you mean make the dts likes below?
>>
>>
>> mbigen_dsa: interrupt-controller at c0080000 {
>> compatible = "hisilicon,mbigen-v2";
>> interrupt-controller;
>> #interrupt-cells = <5>;
>
> These two statements shouldn't be here...
According to you suggest that irq chip should be probed with IRQCHIP_DECLARE(),
I think the compatible string also should be moved into mbigen_clinet_device node.
And, changing mbigen driver likes below:
IRQCHIP_DECLARE(hisi_mbigen, "hisilicon,mbigen-v2", mbigen_of_init);
Thanks!
Ma Jun
>
>> #mbigen-node-cells = <3>;
>> reg = <0xc0080000 0x10000>;
>>
>> mbigen_client_device1 {
>> msi-parent = <&its 0x40b1c>;
>> nr-interrupts = <9>;
>> interrupt-controller;
>
> This is where #interrupt-cells should be.
>
>> }
>>
>> mbigen_client_device2{
>> msi-parent = <&its 0x40b1d>;
>> nr-interrupts = <3>;
>> interrupt-controller;
>> }
>> };
>>
>>
>> client_device1 {
>> compatible = "arm,smmu-v3";
>> reg = <0x0 0xc0040000 0x0 0x20000>;
>> interrupt-parent = <&mbigen_client_device1>;
>> interrupts = <pin_offset 78 1>,
>> <pin_offset 79 1>,
>> <pin_offset 80 1>;
>> };
>>
>>
>
> But otherwise, this looks better.
>
> Thanks,
>
> M.
>
prev parent reply other threads:[~2015-09-23 7:24 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-19 2:55 [PATCH v4 0/2] Support Mbigen interrupt controller MaJun
2015-08-19 2:55 ` [PATCH v4 1/2] Add the driver of mbigen " MaJun
2015-09-21 21:53 ` Marc Zyngier
2015-09-23 7:24 ` majun (F)
2015-09-24 19:30 ` Marc Zyngier
2015-09-25 11:56 ` majun (F)
2015-09-28 15:46 ` Marc Zyngier
2015-08-19 2:55 ` [PATCH v4 2/2] dt-binding:Documents of the mbigen bindings MaJun
2015-09-21 18:09 ` Marc Zyngier
[not found] ` <56013D0A.4000403@huawei.com>
2015-09-22 14:41 ` Marc Zyngier
2015-09-23 7:24 ` majun (F) [this message]
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=560253BE.8030505@huawei.com \
--to=majun258@huawei$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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