public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: marc.zyngier@arm•com (Marc Zyngier)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v4 2/2] dt-binding:Documents of the mbigen bindings
Date: Tue, 22 Sep 2015 15:41:20 +0100	[thread overview]
Message-ID: <20150922154120.18634b41@arm.com> (raw)
In-Reply-To: <56013D0A.4000403@huawei.com>

On Tue, 22 Sep 2015 19:35:38 +0800
"majun (F)" <majun258@huawei•com> wrote:

> Hi Marc:
> 
> ? 2015/9/22 2:09, Marc Zyngier ??:
> > On Wed, 19 Aug 2015 03:55:20 +0100
> > MaJun <majun258@huawei•com> wrote:
> > 
> [...]
> >> +These nodes must have the following properties:
> >> +- msi-parent: This property has two cells.
> >> +	The 1st cell specifies the ITS this device connected.
> >> +	The 2nd cell specifies the device id.
> >> +- nr-interrupts:Specifies the total number of interrupt this device has.
> >> +- mbigen_node: Specifies the information of mbigen nodes this device
> >> +  connected.Some devices with many interrupts maybe connects with several
> >> +  mbigen nodes.
> >> +
> >> +Examples:
> >> +
> >> +		mbigen_dsa: interrupt-controller at c0080000 {
> >> +			compatible = "hisilicon,mbigen-v2";
> >> +			interrupt-controller;
> >> +			#interrupt-cells = <5>;
> >> +			#mbigen-node-cells = <3>;
> >> +			reg = <0xc0080000 0x10000>;
> >> +
> >> +			mbigen_device_01 {
> >> +				msi-parent = <&its 0x40b1c>;
> >> +				nr-interrupts = <9>;
> >> +				mbigen_node = <1 2 0>,
> >> +							<3 2 4>,
> >> +							<4 5 0>;
> >> +			}
> >> +
> >> +			mbigen_device_02 {
> >> +				msi-parent = <&its 0x40b1d>;
> >> +				nr-interrupts = <3>;
> >> +				mbigen_node = <6 3 0>;
> >> +				interrupt-controller;
> >> +			}
> >> +		};
> >> +
> >> +Device connect to mbigen required properties:
> >> +----------------------------------------------------
> >> +-interrupt-parent: Specifies the mbigen node which device connected.
> >> +-interrupts:specifies the interrupt source.The first cell is hwirq num, the
> >> +  second number is trigger type.
> >> +
> >> +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...

> 			#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.
-- 
Jazz is not dead. It just smells funny.

  parent reply	other threads:[~2015-09-22 14:41 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 [this message]
2015-09-23  7:24         ` majun (F)

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=20150922154120.18634b41@arm.com \
    --to=marc.zyngier@arm$(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