On 07/12/2023 18:03, Biju Das wrote: Trim the quote from irrelevant context, especially if your email client malforms replies... (because it does) >>> @@ -35,6 +42,19 @@ properties: >>> "#interrupt-cells": >>> const: 2 >>> >>> + gpio: >> >> Old binding did not have such node and nothing in commit msg explained >> changes from pure conversion. > > OK will update the commit message. Check patch complained about undocumented compatible. > >> >>> + type: object >>> + $ref: /schemas/gpio/gpio.yaml# >> >> ?!? Why? First: It's always selected. Second, so you have two gpio >> controllers? And original binding had 0? Why this is not explained at all? >> Open the binding and look at its properties. > > > I have referred[1] and [2] to add gpio controller property. > > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/nxp/imx/imx6qdl-phytec-phycore-som.dtsi?h=next-20231207#n95 > > [2] > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/mfd/da9062.txt?h=next-20231207#n38 But does it make sense? Don't just blindly copy things, but actually investigate whether this is correct DTS. > >> >> >>> + unevaluatedProperties: false >>> + properties: >>> + compatible: >>> + const: dlg,da9062-gpio >>> + >>> + gpio-controller: true >> >> And here is the second gpio-controller... > > So you mean it is redundant as $ref: /schemas/gpio/gpio.yaml > has already defined gpio-controller for this object?? I meant this would mean you have two GPIO controllers. Why one device would have two GPIO controllers? Please answer to this in commit msg, so there will be no questions/concerns. You have entire commit msg to explain all weird and unexpected things with this binding. ... >>> + #include <dt-bindings/interrupt-controller/irq.h> >>> + #include <dt-bindings/regulator/dlg,da9063-regulator.h> >>> + i2c { >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + pmic@58 { >>> + compatible = "dlg,da9062"; >>> + reg = <0x58>; >>> + #interrupt-cells = <2>; >>> + interrupt-parent = <&gpio1>; >>> + interrupts = <2 IRQ_TYPE_LEVEL_LOW>; >>> + interrupt-controller; >>> + >>> + gpio { >>> + compatible = "dlg,da9062-gpio"; >>> + status = "disabled"; >> >> Why? Why disabling? Drop all statuses from all your binding examples. >> Where are gpio-controller and cells? For this node and for parent? Why >> this example is incomplete? > > I have used a real example [1] here. > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/nxp/imx/imx6qdl-phytec-phycore-som.dtsi?h=next-20231207#n95 > > Currently I don't see any driver is using this compatible other than MFD. Open the MFD so you will see it... Best regards, Krzysztof
