Re: [PATCH 1/4] dt-bindings: watchdog: loongson,ls1x-wdt: Add ls7a-wdt compatible

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [Medium] dt-bindings: watchdog: loongson,ls1x-wdt: clock-frequency
  replaces clocks
- [Low] dt-bindings: watchdog: loongson,ls1x-wdt: missing ls2k0300-wdt
  fallback
- [Low] dt-bindings: watchdog: loongson,ls1x-wdt: top-level if and
  single-item enum
--

commit 7aab162e32ac784b9506b7510ce703256e2590fa
Author: Rong Zhang <rongrong@oss.cipunited.com>

dt-bindings: watchdog: loongson,ls1x-wdt: Add ls7a-wdt compatible

This commit adds the "loongson,ls7a-wdt" compatible string and a
conditionally required clock-frequency property to the Loongson-1 watchdog
binding.

> diff --git a/Documentation/devicetree/bindings/watchdog/loongson,ls1x-wdt.yaml b/Documentation/devicetree/bindings/watchdog/loongson,ls1x-wdt.yaml
> index 50a9b468c4a3e..cb93e9578f82a 100644
> --- a/Documentation/devicetree/bindings/watchdog/loongson,ls1x-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/loongson,ls1x-wdt.yaml
> @@ -18,6 +18,7 @@ properties:
>        - loongson,ls1b-wdt
>        - loongson,ls1c-wdt
>        - loongson,ls2k0300-wdt
> +      - loongson,ls7a-wdt

[Severity: Low]
Should loongson,ls7a-wdt use loongson,ls2k0300-wdt as a fallback
compatible?

The commit message notes that the Loongson-7A watchdog is similar to the
Loongson 2K0300, and the driver uses the same register offsets and bit
definitions for both.  Defining a fallback with oneOf/items allows
existing drivers that match loongson,ls2k0300-wdt to bind without match
table updates.

>   reg:
>     maxItems: 1
> @@ -25,13 +26,34 @@ properties:
>    clocks:
>      maxItems: 1
>  
> +  clock-frequency:
> +    description: Frequency of clock in Hz
> +
>  required:
>    - compatible
>    - reg
> -  - clocks
>  
>  unevaluatedProperties: false
>  
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - loongson,ls7a-wdt

[Severity: Low]
Should this if/then/else block be placed under allOf: before
unevaluatedProperties: false, and use const: loongson,ls7a-wdt instead of
a single-entry enum?

> +
> +then:
> +  properties:
> +    clocks: false
> +  required:
> +    - clock-frequency
> +
> +else:
> +  properties:
> +    clock-frequency: false
> +  required:
> +    - clocks

[Severity: Medium]
Should loongson,ls7a-wdt keep the standard clocks property instead of
replacing it with clock-frequency?

Even without a software-programmable clock controller, the watchdog timer
block still has a hardware clock input that can be represented in the DTS
with a fixed-clock provider node, matching other LS7A bindings such as
loongson,ls7a-pwm.yaml.  Forbidding clocks here creates a divergent ABI
and requires separate clock-frequency handling in loongson1_wdt.c.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-mips-ls7a-wdt-v1-0-968bb85122e6@oss.cipunited.com?part=1