Re: [PATCH 15/20] dt-bindings: clock: add Synaptics SL261X clock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] dt-bindings: clock: schema erroneously matches generic syscon globally
- [Medium] dt-bindings: clock: missing clock-cells in required properties
- [Medium] dt-bindings: clock: missing clock IDs for AIO clock controller
- [Low] dt-bindings: clock: missing include guard in header file
--

commit e7f8a3b9148879b63e354fd7d1d51ae0ec2df9d0
Author: Jisheng Zhang <jszhang@kernel.org>

dt-bindings: clock: add Synaptics SL261X clock

Add support for clocks, plls in Synaptics SL261X SoCs.

> diff --git a/Documentation/devicetree/bindings/clock/syna,sl261x-clk.yaml b/Documentation/devicetree/bindings/clock/syna,sl261x-clk.yaml
> new file mode 100755
> index 0000000000000..3fd380c1dd39d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/syna,sl261x-clk.yaml
> @@ -0,0 +1,92 @@

[ ... ]

> +properties:
> +  compatible:
> +    oneOf:
> +      - enum:
> +          - syna,sl261x-soc-clk
> +          - syna,sl261x-cpu-clk
> +          - syna,sl261x-aio-clk
> +          - syna,sl261x-system-clk
> +      - items:
> +          - const: simple-mfd
> +          - const: syscon

[Severity: Medium]
Will defining these generic strings as valid matches cause the schema
toolchain to improperly select and apply this highly restrictive clock
schema to all syscon nodes system-wide?

> +
> +  reg:
> +    maxItems: 1

[ ... ]

> +  '#clock-cells':
> +    const: 1
> +
> +  clocks:
> +    maxItems: 1
> +    description: Reference clock used as the input to the PLLs.
> +
> +required:
> +  - compatible

[Severity: Medium]
Should '#clock-cells' be explicitly included in this required list to
enforce that these nodes act as valid clock providers?

> +
> +allOf:

[ ... ]

> +    else:
> +      properties:
> +        syna,gbl-syscon: false
> +
> +additionalProperties: false

[Severity: Medium]
When applied to a generic simple-mfd or syscon node system-wide, will this
strict constraint incorrectly reject valid properties and child nodes?

[ ... ]

> diff --git a/include/dt-bindings/clock/syna,sl261x.h b/include/dt-bindings/clock/syna,sl261x.h
> new file mode 100644
> index 0000000000000..4649b233f08a5
> --- /dev/null
> +++ b/include/dt-bindings/clock/syna,sl261x.h
> @@ -0,0 +1,131 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> +/*
> + * Synaptics SL261X clock tree IDs
> + *
> + * Copyright (C) 2025 Synaptics Incorporated
> + *
> + * Author: Jisheng Zhang <jszhang@kernel.org>
> + */

[Severity: Low]
Does this file need a standard include guard (e.g.,
#ifndef _DT_BINDINGS_CLOCK_SYNA_SL261X_H) to prevent macro redefinition
warnings and compilation errors if included multiple times?

> +
> +/* SoC domain */
> +
> +/* GBL common clks */
> +#define CLK_CPUFASTREF		0

[Severity: Medium]
The header explicitly maps domains like SoC, CPU, and sysmgr, but lacks a
macro section defining IDs for the AIO clock controller.

Since the schema declares the AIO clock controller (compatible =
"syna,sl261x-aio-clk") and requires '#clock-cells = <1>', device tree
authors need valid IDs to consume its clocks.

Are there missing clock IDs for the AIO clock controller?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929061417.20733-1-jszhang@kernel.org?part=15

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c