> -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> > Sent: Tuesday, July 12, 2022 5:26 AM > To: Frank Li <frank.li@xxxxxxx>; tglx@xxxxxxxxxxxxx; maz@xxxxxxxxxx; > robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx; > shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; kw@xxxxxxxxx; > bhelgaas@xxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux- > pci@xxxxxxxxxxxxxxx; Peng Fan <peng.fan@xxxxxxx>; Aisheng Dong > <aisheng.dong@xxxxxxx>; jdmason@xxxxxxxx > Cc: kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; dl-linux-imx <linux- > imx@xxxxxxx>; kishon@xxxxxx; lorenzo.pieralisi@xxxxxxx; > ntb@xxxxxxxxxxxxxxx > Subject: [EXT] Re: [PATCH 2/3] dt-bindings: irqchip: imx mu work as msi > controller > > Caution: EXT Email > > On 07/07/2022 23:02, Frank Li wrote: > > imx mu support generate irq by write a register. > > provide msi controller support so other driver > > can use it by standard msi interface. > > > > Signed-off-by: Frank Li <Frank.Li@xxxxxxx> > > --- > > .../interrupt-controller/fsl,mu-msi.yaml | 94 +++++++++++++++++++ > > 1 file changed, 94 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/interrupt- > controller/fsl,mu-msi.yaml > > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,mu- > msi.yaml b/Documentation/devicetree/bindings/interrupt-controller/fsl,mu- > msi.yaml > > new file mode 100644 > > index 0000000000000..b4ac583f60227 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,mu- > msi.yaml > > @@ -0,0 +1,94 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicet > ree.org%2Fschemas%2Finterrupt-controller%2Ffsl%2Cmu- > msi.yaml%23&data=05%7C01%7CFrank.Li%40nxp.com%7C1f7d0921308 > 74d4a52d808da63f0e9db%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C > 0%7C637932183637319092%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wL > jAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C% > 7C%7C&sdata=eA4wrkb39C4NFpLcpvMdFfqEBxcikyTOGaBthf61tjI%3D& > amp;reserved=0 > > +$schema: > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicet > ree.org%2Fmeta- > schemas%2Fcore.yaml%23&data=05%7C01%7CFrank.Li%40nxp.com%7 > C1f7d092130874d4a52d808da63f0e9db%7C686ea1d3bc2b4c6fa92cd99c5c30 > 1635%7C0%7C0%7C637932183637319092%7CUnknown%7CTWFpbGZsb3d8e > yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D > %7C3000%7C%7C%7C&sdata=ke6K1nzmgNXh%2FkyEh6UP%2F0VZ4C17 > GuhnqZTX4WeB4kY%3D&reserved=0 > > + > > +title: NXP i.MX Messaging Unit (MU) > > + > > +maintainers: > > + - Frank Li <Frank.Li@xxxxxxx> > > + > > +description: | > > + The Messaging Unit module enables two processors within the SoC to > > + communicate and coordinate by passing messages (e.g. data, status > > + and control) through the MU interface. The MU also provides the ability > > + for one processor to signal the other processor using interrupts. > > + > > + Because the MU manages the messaging between processors, the MU > uses > > + different clocks (from each side of the different peripheral buses). > > + Therefore, the MU must synchronize the accesses from one side to the > > + other. The MU accomplishes synchronization using two sets of matching > > + registers (Processor A-facing, Processor B-facing). > > + > > + MU can work as msi interrupt controller to do doorbell > > + > > +properties: > > + compatible: > > + oneOf: > > + - const: fsl,imx6sx-mu-msi > > + - const: fsl,imx7ulp-mu-msi > > + - const: fsl,imx8ulp-mu-msi > > + - const: fsl,imx8-mu-msi > > + - const: fsl,imx8ulp-mu-msi-s4 > > Use enum > > > + - items: > > + - const: fsl,imx8ulp-mu-msi > > Single item... why? > > > + - items: > > + - enum: > > + - fsl,imx7s-mu-msi > > + - fsl,imx8mq-mu-msi > > + - fsl,imx8mm-mu-msi > > + - fsl,imx8mn-mu-msi > > + - fsl,imx8mp-mu-msi > > + - fsl,imx8qm-mu-msi > > Why qm is here not compatible with qxp? It's already mentioned in > section below. > > > + - fsl,imx8qxp-mu-msi > > + - const: fsl,imx6sx-mu-msi > > + - description: MU work as msi controller > > + items: > > + - enum: > > + - fsl,imx8qm-mu-msi > > + - fsl,imx8qxp-mu-msi > > + - const: fsl,imx6sx-mu-msi > > + reg: > > + maxItems: 2 > > + > > + interrupts: > > + minItems: 1 > > + maxItems: 2 > > Instead describe the items. > > > + > > + interrupt-names: > > + minItems: 1 > > + items: > > + - const: tx > > + - const: rx > > + > > + clocks: > > + maxItems: 1 > > + > > + power-domains: > > + maxItems: 1 > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + - msi-controller > > How this end up here? > > Aren't you missing allOf with a reference to msi-controller? > > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + > > + lsio_mu12: msi@5d270000 { > > + compatible = "fsl,imx6sx-mu-msi-db"; > > ??? > > > + msi-controller; > > + interrupt-controller; > > ??? How this appeared here > > Also fix your indentation like in example-schema. > > > + reg = <0x5d270000 0x10000>, /* A side */ > > + <0x5d300000 0x10000>; /* B side */ > > + reg-names = "a", "b"; > > + interrupts = <GIC_SPI 191 IRQ_TYPE_LEVEL_HIGH>; > > + power-domains = <&pd IMX_SC_R_MU_12A>, > > + <&pd IMX_SC_R_MU_12B>; > > Please do not send untested bindings. It's a waste of our time. > > Really two items here? You just said only one is allowed. > > > + power-domain-names = "a", "b"; > > Sorry, this patch looks really poor. a/b is not a descriptive name and [Frank Li] MU spec said it is A-side and B-side. So I think "a" "b" is good Enough here. I fixed most problem at v2. PCI EP using MSI as door bell is quite new. I want check if the overall idea is good Enough to go further. > they are not allowed by your own bindings. Please perform some internal > reviews... > > > + }; > > > Best regards, > Krzysztof