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

> Hi Marc:
> 
> 在 2015/9/22 2:09, Marc Zyngier 写道:
> > On Wed, 19 Aug 2015 03:55:20 +0100
> > MaJun <majun...@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@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@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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to