Re: [PATCH v7 1/2] dt-bindings: Pinefeat cef168 lens control board

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The Device Tree schema describes software rather than the physical hardware, violating Device Tree design principles. The title explicitly calls the binding a 'driver' instead of the hardware device, and the commit message intentionally omits power supply definitions because 'the driver does not manage any regulator', which couples the hardware description to current software limitations.
--

commit 5752dd4b724a501f53ddad43b87fa9c0328d6fb5
Author: Aliaksandr Smirnou <asmirnou@pinefeat.co.uk>

dt-bindings: Pinefeat cef168 lens control board

This commit adds the Device Tree schema and examples for the Pinefeat
cef168 lens control board. It supports controlling Canon EF and EF-S
lenses via V4L2.

> Power supply is derived from fixed supplies via connector or GPIO
> header. Therefore, the driver does not manage any regulator, so
> representing any supply in the binding is redundant.

[Severity: Low]
Device tree is intended to describe the physical hardware independent of
the operating system or current driver features. Shouldn't the physical
power rails be described in the binding, even if the current driver does
not actively manage them?

> diff --git a/Documentation/devicetree/bindings/media/i2c/pinefeat,cef168.yaml b/Documentation/devicetree/bindings/media/i2c/pinefeat,cef168.yaml
> new file mode 100644
> index 0000000000000..1295b1f4edebc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/pinefeat,cef168.yaml
> @@ -0,0 +1,47 @@
[ ... ]
> +$id: http://devicetree.org/schemas/media/i2c/pinefeat,cef168.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Pinefeat cef168 lens driver

[Severity: Low]
Does this title reflect a conceptual mismatch by referring to a software
driver rather than the hardware device? Would it be better to title this
"Pinefeat cef168 lens control board" to match standard device tree naming
practices?

[ ... ]
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false

[Severity: Low]
Because physical power supplies are omitted from the schema, does setting
additionalProperties to false prevent creating a complete, hardware-accurate
device tree? If someone describes the board's actual power rails in their
device tree, it will fail schema validation. Should the physical supply
properties be added here to allow accurate hardware descriptions?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601204814.19148-1-asmirnou@pinefeat.co.uk?part=1