| Message ID | 20241212-dt-bcm2712-fixes-v3-0-44a7f3390331@raspberrypi.com |
|---|---|
| Headers | show |
| Series | drm/vc4: Fixup DT and DT binding issues from recent patchset | expand |
On 12/12/24 10:36, Dave Stevenson wrote: > gpio-line-names is a generic property that can be supported by any > GPIO controller, so permit it through the binding. > > It is permitted to have a variable number of GPIOs per node based > on brcm,gpio-bank-widths, so define an arbitrary maximum number of > items based on current users. > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> Acked-by: Florian Fainelli <florian.fainelli@broadcom.com>
On Thu, Dec 12, 2024 at 06:36:28PM +0000, Dave Stevenson wrote: > Commit 62948c62abca ("dt-bindings: display: Add BCM2712 HDMI bindings") > added the compatible strings for BCM2712, but missed out that the > number of interrupts changed. > > Update the schema to include the interrupt requirements. > > Fixes: 62948c62abca ("dt-bindings: display: Add BCM2712 HDMI bindings") > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > --- > .../bindings/display/brcm,bcm2711-hdmi.yaml | 107 ++++++++++++++++++--- > 1 file changed, 93 insertions(+), 14 deletions(-) > > diff --git a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml > index 6d11f5955b51..dd7dea60183b 100644 > --- a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml > +++ b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml > @@ -56,22 +56,38 @@ properties: > - const: cec > > interrupts: > - items: > - - description: CEC TX interrupt > - - description: CEC RX interrupt > - - description: CEC stuck at low interrupt > - - description: Wake-up interrupt > - - description: Hotplug connected interrupt > - - description: Hotplug removed interrupt > + oneOf: > + - items: > + - description: CEC TX interrupt > + - description: CEC RX interrupt > + - description: CEC stuck at low interrupt > + - description: Wake-up interrupt > + - description: Hotplug connected interrupt > + - description: Hotplug removed interrupt > + > + - items: > + - description: CEC TX interrupt > + - description: CEC RX interrupt > + - description: CEC stuck at low interrupt > + - description: Hotplug connected interrupt > + - description: Hotplug removed interrupt You have chosen unusual syntax. There are no bindings doing this way, so I really do not get which file you used as template. Expected here are minItems and maxItems. These are the widest constraints. Otherwise you are repeating the same in allOf:if:then. And then the allOf:if:then: defines the items. You can do the opposite - define the items here then just choose constraints in if:then:. Less popular if you have list without common part (so no minItems here), but sure, if you insist... yet you chosen some third way of duplicating it in both places. Look: https://elixir.bootlin.com/linux/v6.11-rc6/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L127 https://elixir.bootlin.com/linux/v6.11-rc6/source/Documentation/devicetree/bindings/ufs/samsung,exynos-ufs.yaml#L39 https://elixir.bootlin.com/linux/v5.19/source/Documentation/devicetree/bindings/example-schema.yaml#L91 There is nowhere syntax like here, with duplicating everything twice. BTW, drop the full stop from your subjects in some other patches. Subject never ends with full stop. Best regards, Krzysztof
On Thu, Dec 12, 2024 at 06:36:30PM +0000, Dave Stevenson wrote: > gpio-line-names is a generic property that can be supported by any > GPIO controller, so permit it through the binding. > > It is permitted to have a variable number of GPIOs per node based > on brcm,gpio-bank-widths, so define an arbitrary maximum number of > items based on current users. > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > --- > Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.yaml | 4 ++++ > 1 file changed, 4 insertions(+) Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> On Thu, 12 Dec 2024 18:36:27 +0000, Dave Stevenson wrote: > I missed the DT errors from the recent patchset[1] (DT patches > in linux-next via Florian, DRM bindings patches on dri-misc-next) > as Rob's bot report got spam filtered, so this is a fixup set. > > Largely it was changes to number of interrupts or clocks in the > bindings, so those are now covered. > > [...] Applied, thanks! [3/7] dt-bindings: gpio: brcmstb: permit gpio-line-names property commit: 83a9752729c455a6bd9b7cf62198506180691931 Best regards,
I missed the DT errors from the recent patchset[1] (DT patches in linux-next via Florian, DRM bindings patches on dri-misc-next) as Rob's bot report got spam filtered, so this is a fixup set. Largely it was changes to number of interrupts or clocks in the bindings, so those are now covered. I've fixed up the missing "interrupt-controller" flags for 2711 and 2712 whilst here. I can't get my head around what is meant to happen with ranges: "soc@107c000000: firmware: 'ranges' is a required property" The meaning seems obvious. However if I add it then I get: "firmware: '#address-cells', '#size-cells', 'dma-ranges', 'ranges' do not match any of the regexes: 'pinctrl-[0-9]+' from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml# There's obviously some other flag I need to set in the bindings, but I can't work it out. We have similar errors for all the Pi platforms for one or more nodes. Please advise and I'll happily fix them all. Thanks Dave [1] https://lore.kernel.org/linux-arm-kernel/20241025-drm-vc4-2712-support-v2-0-35efa83c8fc0@raspberrypi.com/ Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> --- Changes in v3: - Fixed up indentation on 1/7. (I fixed it once, but obviously reworked things and lost it). - Link to v2: https://lore.kernel.org/r/20241212-dt-bcm2712-fixes-v2-0-35986e04d0f4@raspberrypi.com Thanks to Stefan and Krzysztof for their reviews. Hopefully I've addressed all points raised in the correct manner. Changes in v2: - Commits have now be merged from drm-misc-next to linux-next, so all commit hashes are valid on linux-next. - 1/7 Removed references to "previous commit". Fixed up indentation. Added maxItems - 2/7 Defined widest constraints - 3/7 Added maxItems and removed reference to Linux - 4/7 Described the errors. Split into two for fix of node name vs addr being wrong. - Added new patch removing "required" for interrupt-controller and interrupt-cells for bcm2836-l1-intc - 5/7 (now 7/7) Removed the intc node for 2712 - it's irrelevant on 64bit systems - 6/7 dropped as updating the binding is the correct answer - 7/7 dropped. simple-bus claims ranges is required, but adding it creates other errors. I'm unclear as to the right solution. - Link to v1: https://lore.kernel.org/r/20241202-dt-bcm2712-fixes-v1-0-fac67cc2f98a@raspberrypi.com --- Dave Stevenson (7): dt-bindings: display: bcm2711-hdmi: Add interrupt details for BCM2712 dt-bindings: display: Fix BCM2835 HVS bindings for BCM2712 dt-bindings: gpio: brcmstb: permit gpio-line-names property dt-bindings: interrupt-controller: brcm,bcm2836-l1-intc: Drop interrupt-controller requirement arm64: dts: broadcom: Rename bcm2712 interrupt controllers arm64: dts: broadcom: Correct hdmi device node names arm64: dts: broadcom: Remove intc controller on BCM2712. .../bindings/display/brcm,bcm2711-hdmi.yaml | 107 ++++++++++++++++++--- .../bindings/display/brcm,bcm2835-hvs.yaml | 83 +++++++++++++--- .../bindings/gpio/brcm,brcmstb-gpio.yaml | 4 + .../interrupt-controller/brcm,bcm2836-l1-intc.yaml | 2 - arch/arm64/boot/dts/broadcom/bcm2712.dtsi | 13 +-- 5 files changed, 170 insertions(+), 39 deletions(-) --- base-commit: 3a6b7ba51f16c093420959ab2bd3476d180547fa change-id: 20241128-dt-bcm2712-fixes-afb0e8a0a476 Best regards,