| Message ID | 20220707210238.917477-2-Frank.Li@nxp.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/3] irqchip: imx mu worked as msi controller | expand |
On Thu, 07 Jul 2022 16:02:37 -0500, 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@nxp.com> > --- > .../interrupt-controller/fsl,mu-msi.yaml | 94 +++++++++++++++++++ > 1 file changed, 94 insertions(+) > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/fsl,mu-msi.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: Error: Documentation/devicetree/bindings/interrupt-controller/fsl,mu-msi.example.dts:31.41-42 syntax error FATAL ERROR: Unable to parse input tree make[1]: *** [scripts/Makefile.lib:383: Documentation/devicetree/bindings/interrupt-controller/fsl,mu-msi.example.dtb] Error 1 make[1]: *** Waiting for unfinished jobs.... make: *** [Makefile:1404: dt_binding_check] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/ This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
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@nxp.com> > --- > .../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: http://devicetree.org/schemas/interrupt-controller/fsl,mu-msi.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: NXP i.MX Messaging Unit (MU) > + > +maintainers: > + - Frank Li <Frank.Li@nxp.com> > + > +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 they are not allowed by your own bindings. Please perform some internal reviews... > + }; Best regards, Krzysztof
> -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: Tuesday, July 12, 2022 5:26 AM > To: Frank Li <frank.li@nxp.com>; tglx@linutronix.de; maz@kernel.org; > robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; > shawnguo@kernel.org; s.hauer@pengutronix.de; kw@linux.com; > bhelgaas@google.com; linux-kernel@vger.kernel.org; > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux- > pci@vger.kernel.org; Peng Fan <peng.fan@nxp.com>; Aisheng Dong > <aisheng.dong@nxp.com>; jdmason@kudzu.us > Cc: kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx <linux- > imx@nxp.com>; kishon@ti.com; lorenzo.pieralisi@arm.com; > ntb@lists.linux.dev > 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@nxp.com> > > --- > > .../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@nxp.com> > > + > > +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
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: http://devicetree.org/schemas/interrupt-controller/fsl,mu-msi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NXP i.MX Messaging Unit (MU) + +maintainers: + - Frank Li <Frank.Li@nxp.com> + +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 + - items: + - const: fsl,imx8ulp-mu-msi + - 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 + - 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 + + interrupt-names: + minItems: 1 + items: + - const: tx + - const: rx + + clocks: + maxItems: 1 + + power-domains: + maxItems: 1 + +required: + - compatible + - reg + - interrupts + - 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; + 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>; + power-domain-names = "a", "b"; + };
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@nxp.com> --- .../interrupt-controller/fsl,mu-msi.yaml | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/fsl,mu-msi.yaml