From: sashiko-bot@kernel.org
To: "Yu-Chun Lin" <eleanor.lin@realtek.com>
Cc: devicetree@vger.kernel.org, robh@kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH v10 02/11] reset: Add Realtek basic reset support
Date: Mon, 13 Jul 2026 16:08:19 +0000 [thread overview]
Message-ID: <20260713160820.0594B1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260713155630.3054636-3-eleanor.lin@realtek.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The Kconfig help text and commit message incorrectly claim the driver provides a registration helper function to extract the regmap and device tree node.
- [Low] Unnecessary inclusion of `<linux/device.h>` and `<linux/of.h>`.
--
--- Patch [2]: [PATCH v10 02/11] reset: Add Realtek basic reset support ---
commit 231fee7f5cf687d120e12d9c11d120d53d5aa54d
Author: Cheng-Yu Lee <cylee12@realtek.com>
reset: Add Realtek basic reset support
This commit defines reset operations backed by a regmap-based register
interface for Realtek SoCs. It exports the reset control operations to allow
callers to manually assign their own reset controller data structures.
> reset: Add Realtek basic reset support
>
> Define the reset operations backed by a regmap-based register interface
> and prepare the reset controller to be registered through the reset
> framework.
>
> Since the reset controllers on Realtek SoCs often share the same register
> space with the clock controllers, this common framework is designed to
> extract the regmap and device tree node from the parent device
> (e.g., an auxiliary device parent).
[Severity: Medium]
Is this description still accurate?
The changelog notes that in v10, the wrapper function was removed to allow
callers to assign the regmap, ops, dev, and of_node individually. It seems the
common framework no longer extracts the regmap and device tree node from the
parent device.
[ ... ]
> diff --git a/drivers/reset/realtek/Kconfig b/drivers/reset/realtek/Kconfig
> new file mode 100644
> index 0000000000000..bb6dd856a64a5
> --- /dev/null
> +++ b/drivers/reset/realtek/Kconfig
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +config RESET_RTK_COMMON
> + tristate "Realtek common reset driver" if COMPILE_TEST
> + help
> + This option enables the common reset controller library for
> + Realtek SoCs. It provides shared reset control operations
> + (assert, deassert, status) and a registration helper function
> + that other Realtek-specific reset drivers can use.
[Severity: Medium]
Should the Kconfig help text be updated to reflect the v10 changes?
It explicitly claims a registration helper function is provided, which appears
to no longer be true since rtk_reset_ops is exported directly instead.
[ ... ]
> diff --git a/drivers/reset/realtek/reset-rtk-common.c b/drivers/reset/realtek/reset-rtk-common.c
> new file mode 100644
> index 0000000000000..4d5ec2b0416d1
> --- /dev/null
> +++ b/drivers/reset/realtek/reset-rtk-common.c
> @@ -0,0 +1,75 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2019-2026 Realtek Semiconductor Corporation
> + */
> +
> +#include <linux/device.h>
> +#include <linux/export.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
[Severity: Low]
Since the registration helper function was removed, are linux/device.h and
linux/of.h still necessary?
It looks like these headers were used to interact with struct device and
of_node, and could now be safely dropped.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260713155630.3054636-1-eleanor.lin@realtek.com?part=2
next prev parent reply other threads:[~2026-07-13 16:08 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-13 15:56 [PATCH v10 00/11] clk / reset: realtek: Add RTD1625 clock and reset support Yu-Chun Lin
2026-07-13 15:56 ` [PATCH v10 01/11] dt-bindings: clock: Add Realtek RTD1625 Clock & Reset Controller Yu-Chun Lin
2026-07-13 15:56 ` [PATCH v10 02/11] reset: Add Realtek basic reset support Yu-Chun Lin
2026-07-13 16:08 ` sashiko-bot [this message]
2026-07-13 15:56 ` [PATCH v10 03/11] reset: realtek: Add RTD1625 reset controller driver Yu-Chun Lin
2026-07-13 15:56 ` [PATCH v10 04/11] clk: realtek: Introduce a common probe() Yu-Chun Lin
2026-07-13 16:15 ` sashiko-bot
2026-07-13 15:56 ` [PATCH v10 05/11] clk: realtek: Add support for phase locked loops (PLLs) Yu-Chun Lin
2026-07-13 16:09 ` sashiko-bot
2026-07-13 15:56 ` [PATCH v10 06/11] clk: realtek: Add support for gate clock Yu-Chun Lin
2026-07-13 15:56 ` [PATCH v10 07/11] clk: realtek: Add support for mux clock Yu-Chun Lin
2026-07-13 15:56 ` [PATCH v10 08/11] clk: realtek: Add support for MMC-tuned PLL clocks Yu-Chun Lin
2026-07-13 16:30 ` sashiko-bot
2026-07-13 15:56 ` [PATCH v10 09/11] clk: realtek: Add RTD1625-CRT clock controller driver Yu-Chun Lin
2026-07-13 15:56 ` [PATCH v10 10/11] clk: realtek: Add RTD1625-ISO " Yu-Chun Lin
2026-07-13 16:25 ` sashiko-bot
2026-07-13 15:56 ` [PATCH v10 11/11] arm64: dts: realtek: Add clock support for RTD1625 Yu-Chun Lin
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=20260713160820.0594B1F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=eleanor.lin@realtek.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.