On 10/06/2022 16:38, Phil Edworthy wrote: > Hi Krzysztof, > > Thanks for your review. > > On 08 June 2022 11:52 Krzysztof Kozlowski wrote: >> On 07/06/2022 15:56, Phil Edworthy wrote: >>> Add the documentation for the r9a09g011 SoC, but in doing so also >>> reorganise the doc to make it easier to read. >>> Additionally, make the binding require an interrupt to be specified. >>> Whilst the driver does not need an interrupt, all of the SoCs that use >>> this binding actually provide one. >>> >>> Signed-off-by: Phil Edworthy <phil.edworthy@xxxxxxxxxxx> >>> Reviewed-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> >>> --- >>> .../bindings/watchdog/renesas,wdt.yaml | 63 ++++++++++++------- >>> 1 file changed, 42 insertions(+), 21 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/watchdog/renesas,wdt.yaml >> b/Documentation/devicetree/bindings/watchdog/renesas,wdt.yaml >>> index a8d7dde5271b..6473734921e3 100644 >>> --- a/Documentation/devicetree/bindings/watchdog/renesas,wdt.yaml >>> +++ b/Documentation/devicetree/bindings/watchdog/renesas,wdt.yaml >>> @@ -31,6 +31,11 @@ properties: >>> - renesas,r9a07g054-wdt # RZ/V2L >>> - const: renesas,rzg2l-wdt >>> >>> + - items: >>> + - enum: >>> + - renesas,r9a09g011-wdt # RZ/V2M >>> + - const: renesas,rzv2m-wdt # RZ/V2M >>> + >>> - items: >>> - enum: >>> - renesas,r8a7742-wdt # RZ/G1H >>> @@ -70,13 +75,27 @@ properties: >>> reg: >>> maxItems: 1 >>> >>> - interrupts: true >>> + interrupts: >>> + minItems: 1 >>> + items: >>> + - description: Timeout >>> + - description: Parity error >>> >>> - interrupt-names: true >>> + interrupt-names: >> >> This also needs minItems > I left minItems off for interrupt-names and clock-names on the basis that > they are only needed if you have more than one interrupt or clock. True, but now you disallow them for one clock/interrupt cases in other variants. Although after looking at existing bindings - it's even messier there. For certain variants it is just ":true" which is not correct. In general, the properties in "properties:" section should have constraints - the most wide. These are narrowed for specific variants or even disallowed for some. Old bindings allowed anything for some variants, like 20 interrupt names so clearly wrong. > > After adding the lines you suggested (minItems: 1), I find that > 'make dtbs_check' passes even if there are no interrupt-names or > clock-names specified. Is this expected? These are not required, aren't they? If they are not required, they can be missing... > > minItems: 0 makes more sense to me, but it is required to be greater than > or equal 1 > > Thanks > Phil Best regards, Krzysztof
