| Message ID | 20221117060704.367945-2-jernej.skrabec@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: allwinner: h6: Add second IOMMU reference to Cedrus | expand |
On 17/11/2022 07:07, Jernej Skrabec wrote: > Cedrus (video engine) on Allwinner H6 actually uses two IOMMU channel, > not just one. However, Cedrus on SoCs like D1 only uses one channel. > > Allow up to 2 IOMMU channels. > > Fixes: 62a8ccf3a248 ("arm64: dts: allwinner: h6: Fix Cedrus IOMMU usage") > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com> > --- > .../bindings/media/allwinner,sun4i-a10-video-engine.yaml | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine.yaml b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine.yaml > index 541325f900a1..6446004d59d9 100644 > --- a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine.yaml > +++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine.yaml > @@ -55,7 +55,8 @@ properties: > description: Phandle to the device SRAM > > iommus: > - maxItems: 1 > + minItems: 1 > + maxItems: 2 You have several compatibles in the file, so usually this is further constrained per each variant in allOf:if:then:. Best regards, Krzysztof
Dne četrtek, 17. november 2022 ob 14:13:00 CET je Krzysztof Kozlowski napisal(a): > On 17/11/2022 07:07, Jernej Skrabec wrote: > > Cedrus (video engine) on Allwinner H6 actually uses two IOMMU channel, > > not just one. However, Cedrus on SoCs like D1 only uses one channel. > > > > Allow up to 2 IOMMU channels. > > > > Fixes: 62a8ccf3a248 ("arm64: dts: allwinner: h6: Fix Cedrus IOMMU usage") > > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com> > > --- > > > > .../bindings/media/allwinner,sun4i-a10-video-engine.yaml | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git > > a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engin > > e.yaml > > b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engin > > e.yaml index 541325f900a1..6446004d59d9 100644 > > --- > > a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engin > > e.yaml +++ > > b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engin > > e.yaml> > > @@ -55,7 +55,8 @@ properties: > > description: Phandle to the device SRAM > > > > iommus: > > - maxItems: 1 > > + minItems: 1 > > + maxItems: 2 > > You have several compatibles in the file, so usually this is further > constrained per each variant in allOf:if:then:. Usually, yes. But this whole binding would need update. It has a few optional properties and none of them is tied to any compatible. Additionally, if I do it as you suggest, then Robs automatic test will report the issue, because existing H6 based boards won't match this binding anymore. I would much rather send follow up patch which clears up all optional properties. Best regards, Jernej
On 17/11/2022 21:31, Jernej Škrabec wrote: > Dne četrtek, 17. november 2022 ob 14:13:00 CET je Krzysztof Kozlowski > napisal(a): >> On 17/11/2022 07:07, Jernej Skrabec wrote: >>> Cedrus (video engine) on Allwinner H6 actually uses two IOMMU channel, >>> not just one. However, Cedrus on SoCs like D1 only uses one channel. >>> >>> Allow up to 2 IOMMU channels. >>> >>> Fixes: 62a8ccf3a248 ("arm64: dts: allwinner: h6: Fix Cedrus IOMMU usage") >>> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com> >>> --- >>> >>> .../bindings/media/allwinner,sun4i-a10-video-engine.yaml | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engin >>> e.yaml >>> b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engin >>> e.yaml index 541325f900a1..6446004d59d9 100644 >>> --- >>> a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engin >>> e.yaml +++ >>> b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engin >>> e.yaml> >>> @@ -55,7 +55,8 @@ properties: >>> description: Phandle to the device SRAM >>> >>> iommus: >>> - maxItems: 1 >>> + minItems: 1 >>> + maxItems: 2 >> >> You have several compatibles in the file, so usually this is further >> constrained per each variant in allOf:if:then:. > > Usually, yes. But this whole binding would need update. It has a few optional > properties and none of them is tied to any compatible. Additionally, if I do > it as you suggest, then Robs automatic test will report the issue, because > existing H6 based boards won't match this binding anymore. I would much rather > send follow up patch which clears up all optional properties. I don't understand last argument. It's basically like saying - I keep bindings not really correct, because otherwise I would see warnings and I would need to fix them. If this can be constrained per variant, constrain it and then fix the boards. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine.yaml b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine.yaml index 541325f900a1..6446004d59d9 100644 --- a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine.yaml +++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine.yaml @@ -55,7 +55,8 @@ properties: description: Phandle to the device SRAM iommus: - maxItems: 1 + minItems: 1 + maxItems: 2 memory-region: maxItems: 1
Cedrus (video engine) on Allwinner H6 actually uses two IOMMU channel, not just one. However, Cedrus on SoCs like D1 only uses one channel. Allow up to 2 IOMMU channels. Fixes: 62a8ccf3a248 ("arm64: dts: allwinner: h6: Fix Cedrus IOMMU usage") Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com> --- .../bindings/media/allwinner,sun4i-a10-video-engine.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)