From: sashiko-bot@kernel.org
To: "Eduard Bostina" <egbostina@gmail.com>
Cc: robh@kernel.org, conor+dt@kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v2 4/5] dt-bindings: soc: ti: omap-iva: Make ti,hwmods optional
Date: Mon, 06 Jul 2026 09:26:26 +0000 [thread overview]
Message-ID: <20260706092627.55C381F00A3D@smtp.kernel.org> (raw)
In-Reply-To: <20260706085917.835875-5-egbostina@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Globally removing 'ti,hwmods' from the 'required' list silently breaks schema validation for legacy OMAP generations, and the commit message justifies this using a fundamental misunderstanding of the driver architecture.
--
commit b2b7440e95e8ab91078767b3f9e9a47ec5186538
Author: Eduard Bostina <egbostina@gmail.com>
dt-bindings: soc: ti: omap-iva: Make ti,hwmods optional
This commit removes "ti,hwmods" from the required list in the ti,iva
binding schema. The commit message indicates this is safe because the
ti-sysc driver does not strictly require it for probing.
> A review of the ti-sysc driver (drivers/bus/ti-sysc.c) shows that the
> property is not strictly required for probing. The driver only reads
> it to populate the 'ddata->legacy_mode' flag. If the property is absent,
> the probe sequence continues normally without error.
[Severity: Medium]
Is this justification accurate?
Looking at sysc_check_one_child() in drivers/bus/ti-sysc.c, the driver
actually warns if it finds this property on a child node like ti,iva:
drivers/bus/ti-sysc.c:sysc_check_one_child() {
...
name = of_get_property(np, "ti,hwmods", NULL);
if (name && !of_device_is_compatible(np, "ti,sysc"))
dev_warn(ddata->dev, "really a child ti,hwmods property?");
...
}
> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,iva.yaml b/Documentation/devicetree/bindings/soc/ti/ti,iva.yaml
> index 5724b8367a158..799461a20b049 100644
> --- a/Documentation/devicetree/bindings/soc/ti/ti,iva.yaml
> +++ b/Documentation/devicetree/bindings/soc/ti/ti,iva.yaml
> @@ -24,7 +24,6 @@ properties:
>
> required:
> - compatible
> - - ti,hwmods
[Severity: Medium]
Does removing this requirement globally break schema validation for older OMAP
generations?
Legacy platforms (like OMAP2/OMAP3) rely on arch/arm/mach-omap2/omap_device.c,
which strictly requires this property to initialize the device:
arch/arm/mach-omap2/omap_device.c:omap_device_build_from_dt() {
...
oh_cnt = of_property_count_strings(node, "ti,hwmods");
if (oh_cnt <= 0) {
dev_dbg(&pdev->dev, "No 'hwmods' to build omap_device\n");
return -ENODEV;
}
...
}
If "ti,hwmods" is globally optional, won't invalid device trees for these
legacy platforms silently pass dtbs_check, only to fail at runtime with
-ENODEV?
> additionalProperties: false
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260706085917.835875-1-egbostina@gmail.com?part=4
next prev parent reply other threads:[~2026-07-06 9:26 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-06 8:59 [PATCH v2 0/5] dt-bindings: soc: ti: Convert OMAP DSP and IVA to DT schema Eduard Bostina
2026-07-06 8:59 ` [PATCH v2 1/5] dt-bindings: soc: ti: Convert DSP " Eduard Bostina
2026-07-08 6:58 ` Krzysztof Kozlowski
2026-07-06 8:59 ` [PATCH v2 2/5] dt-bindings: soc: ti: omap-dsp: Make ti,hwmods optional Eduard Bostina
2026-07-08 6:59 ` Krzysztof Kozlowski
2026-07-06 8:59 ` [PATCH v2 3/5] dt-bindings: soc: ti: Convert IVA to DT schema Eduard Bostina
2026-07-06 8:59 ` [PATCH v2 4/5] dt-bindings: soc: ti: omap-iva: Make ti,hwmods optional Eduard Bostina
2026-07-06 9:26 ` sashiko-bot [this message]
2026-07-06 8:59 ` [PATCH v2 5/5] dt-bindings: soc: ti: omap-iva: Document DSP child node Eduard Bostina
2026-07-08 7:00 ` Krzysztof Kozlowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260706092627.55C381F00A3D@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=egbostina@gmail.com \
--cc=robh@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.