Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Artem Novak" <artem122novak@gmail.com>
Cc: devicetree@vger.kernel.org, robh@kernel.org, conor+dt@kernel.org
Subject: Re: [RFC PATCH 1/2] sh: dts: add Casio fx-9750GIII board
Date: Thu, 22 Oct 2026 06:50:45 +0000	[thread overview]
Message-ID: <sashiko-outbox-181106@kernel.org> (raw)
In-Reply-To: <20261022063936.472520-2-artem122novak@gmail.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] sh: dts: missing sh7305.dtsi include breaks fx9750giii.dtb build
- [Medium] sh: dts: fx9750giii uses unselectable Kconfig and missing sh7305.dtsi
- [Low] sh: dts: undocumented compatible strings and vendor prefix in fx9750giii.dts
--

commit 82402ca3074df3c76845795376be9cd0891cff1d
Author: Artem Novak <artem122novak@gmail.com>

sh: dts: add Casio fx-9750GIII board

This commit adds the device tree source file fx9750giii.dts for the Casio
fx-9750GIII graphing calculator and registers fx9750giii.dtb in
arch/sh/boot/dts/Makefile.

> diff --git a/arch/sh/boot/dts/Makefile b/arch/sh/boot/dts/Makefile
> index d109978a5eb9c..4887fb16dbf23 100644
> --- a/arch/sh/boot/dts/Makefile
> +++ b/arch/sh/boot/dts/Makefile
> @@ -1,2 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0-only
> +dtb-$(CONFIG_SH_FX9750GIII) += fx9750giii.dtb

[Severity: Medium]
Can CONFIG_SH_FX9750GIII ever be enabled to build fx9750giii.dtb?

In arch/sh/boot/dts/Makefile, dtb-$(CONFIG_SH_FX9750GIII) references
CONFIG_SH_FX9750GIII, which is not defined in this patch. Although the
follow-up commit ("sh: fx9750giii: add board support") adds config
SH_FX9750GIII in arch/sh/boards/Kconfig, it specifies:

    depends on CPU_SUBTYPE_SH7305

which is not defined anywhere in arch/sh/Kconfig or the kernel tree.

Because CONFIG_SH_FX9750GIII remains unselectable, fx9750giii.dtb is only
placed in $(dtb-), where scripts/Makefile.dtbs promotes it into $(dtb-y) when
CONFIG_OF_ALL_DTBS=y is enabled:

    dtb-$(CONFIG_OF_ALL_DTBS) += $(dtb-)

At that point, cmd_dtc in scripts/Makefile.dtbs attempts to preprocess
arch/sh/boot/dts/fx9750giii.dts and fails because sh7305.dtsi is missing.

>  obj-$(CONFIG_BUILTIN_DTB) += $(addsuffix .dtb.o, $(CONFIG_BUILTIN_DTB_NAME))
> diff --git a/arch/sh/boot/dts/fx9750giii.dts b/arch/sh/boot/dts/fx9750giii.dts
> new file mode 100644
> index 0000000000000..bccae22e48994
> --- /dev/null
> +++ b/arch/sh/boot/dts/fx9750giii.dts
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +// Copyright (C) 2026 Artem Novak
> +/dts-v1/;
> +
> +#include "sh7305.dtsi"

[Severity: High]
Where is sh7305.dtsi defined in the kernel tree?

The commit message states "The description uses existing SH7305 SoC include",
and arch/sh/boot/dts/fx9750giii.dts includes sh7305.dtsi, but no sh7305.dtsi
file exists in arch/sh/boot/dts/, scripts/dtc/include-prefixes/, or anywhere
else in the repository (nor is it added in the follow-up commit
"sh: fx9750giii: add board support"). Without it, the board device tree also
lacks the SoC definitions (cpu nodes, interrupt controller, timers, clocks,
and root #address-cells and #size-cells).

When building fx9750giii.dtb directly, setting
CONFIG_BUILTIN_DTB_NAME="fx9750giii", or building under CONFIG_OF_ALL_DTBS=y
(where scripts/Makefile.dtbs promotes $(dtb-) into $(dtb-y)), cmd_dtc in
scripts/Makefile.dtbs runs:

    $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \

Doesn't this cause a build regression when the preprocessor fails with
"sh7305.dtsi: No such file or directory"?

> +
> +/ {
> +	model = "Casio fx-9750GIII";
> +	compatible = "casio,fx-9750giii", "renesas,sh7305";

[Severity: Low]
Should the vendor prefix and compatible strings in the root node "/" of
arch/sh/boot/dts/fx9750giii.dts be documented in the device tree bindings
first?

The vendor prefix "casio" is not listed in
Documentation/devicetree/bindings/vendor-prefixes.yaml, and neither
"casio,fx-9750giii" nor "renesas,sh7305" is documented under
Documentation/devicetree/bindings/ (such as
Documentation/devicetree/bindings/soc/renesas/renesas.yaml), which causes
scripts/checkpatch.pl and dtbs_check warnings when validating fx9750giii.dts.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022063936.472520-1-artem122novak@gmail.com?part=1

  reply	other threads:[~2026-10-22  6:50 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-22  6:39 [RFC PATCH 0/2] sh: Add Casio fx-9750GIII board support Artem Novak
2026-10-22  6:39 ` [RFC PATCH 1/2] sh: dts: add Casio fx-9750GIII board Artem Novak
2026-10-22  6:50   ` sashiko-bot [this message]
2026-10-22  6:39 ` [RFC PATCH 2/2] sh: fx9750giii: add board support Artem Novak
2026-10-22  6:52   ` sashiko-bot

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=sashiko-outbox-181106@kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=artem122novak@gmail.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox