All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Stefan Dösinger" <stefandoesinger@gmail.com>
Cc: vkoul@kernel.org, linux-phy@lists.infradead.org, robh@kernel.org,
	olteanv@gmail.com, lee@kernel.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org, neil.armstrong@linaro.org,
	mfd@lists.linux.dev
Subject: Re: [PATCH v9 04/12] mfd: zx297520v3: Add a clock and reset MFD driver
Date: Sun, 02 Aug 2026 20:45:16 +0000	[thread overview]
Message-ID: <20260802204517.5126D1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260802-zx29clk-v9-4-d05530d85d28@gmail.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] Direct cast from `const void *` to an `enum` triggers a `-Wpointer-to-int-cast` compiler warning on 64-bit architectures.
- [Low] The Kconfig symbol `MFD_ZTE_ZX297520V3_CRM` lacks an architecture-specific dependency, unconditionally exposing a highly SoC-specific driver to all architectures.
- [Low] The enum `zx297520v3_mfd_type` includes the string 'mfd', violating the MFD subsystem naming conventions.
--

--- Patch [4]: [PATCH v9 04/12] mfd: zx297520v3: Add a clock and reset MFD driver ---
commit 39c5ccee8d6ebff3fee809871f3a8191107ab75b
Author: Stefan Dösinger <stefandoesinger@gmail.com>

mfd: zx297520v3: Add a clock and reset MFD driver

This driver registers child devices for the zx297520v3 clock and reset
controllers. The clk-zx297520v3 and reset-zte-zx297520v3 submitted in
the next patches will drive the respective functionalities.

> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 35f6e9b76d056..c0c7e83487f40 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
[ ... ]
> @@ -2572,5 +2572,16 @@ config MFD_MAX7360
>  	  additional drivers must be enabled in order to use the functionality
>  	  of the device.
>  
> +config MFD_ZTE_ZX297520V3_CRM
> +	tristate "ZTE zx297520v3 Clock and Reset Manager"
> +	select MFD_CORE
> +	default SOC_ZX297520V3

[Severity: Low]
Should this include an architecture dependency, such as
depends on ARCH_ZTE || COMPILE_TEST? Without it, this highly SoC-specific
driver will be presented in the configuration options for all architectures.

> +	help
> +	  Say yes here to enable the driver for the ZTE zx297520v3 clock and
> +	  reset manager MFD driver. This driver provides the host device for
> +	  the clock and reset drivers and is required to boot the SoC. You
> +	  will also need to enable CLK_ZTE_ZX297520V3 and RESET_ZTE_ZX297520V3
> +	  to build the actual clock and reset child drivers.
> +
>  endmenu
>  endif
> diff --git a/drivers/mfd/zte-zx297520v3-crm.c b/drivers/mfd/zte-zx297520v3-crm.c
> new file mode 100644
> index 0000000000000..37cd0c14680eb
> --- /dev/null
> +++ b/drivers/mfd/zte-zx297520v3-crm.c
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2026 Stefan Dösinger
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/mfd/core.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +
> +enum zx297520v3_mfd_type {
> +	ZX297520V3_MFD_INVALID = 0,
> +	ZX297520V3_MFD_TOPCRM,
> +	ZX297520V3_MFD_MATRIXCRM,
> +	ZX297520V3_MFD_LSPCRM,
> +};

[Severity: Low]
This isn't a bug, but does including the string mfd in the enum name violate
the MFD subsystem naming conventions? The guidelines suggest avoiding
hard-coding implementation details like mfd in driver, struct, or device
names.

[ ... ]

> +static int zx297520v3_crm_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	enum zx297520v3_mfd_type type;
> +	const struct mfd_cell *cells;
> +	struct reset_control *rst;
> +	unsigned int num_cells;
> +	struct clk *pclk;
> +
> +	type = (enum zx297520v3_mfd_type)of_device_get_match_data(dev);

[Severity: Low]
Could this direct cast from a const void * pointer to an enum trigger a
-Wpointer-to-int-cast compiler warning on 64-bit architectures? Often an
intermediate cast to uintptr_t or kernel_ulong_t is used to safely preserve
the size before casting to the enum type.

> +	switch (type) {
> +	case ZX297520V3_MFD_TOPCRM:
> +		cells = zx297520v3_topcrm_cells;
> +		num_cells = ARRAY_SIZE(zx297520v3_topcrm_cells);
> +		break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260802-zx29clk-v9-0-d05530d85d28@gmail.com?part=4

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

WARNING: multiple messages have this Message-ID (diff)
From: sashiko-bot@kernel.org
To: "Stefan Dösinger" <stefandoesinger@gmail.com>
Cc: vkoul@kernel.org, linux-phy@lists.infradead.org, robh@kernel.org,
	olteanv@gmail.com, lee@kernel.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org, neil.armstrong@linaro.org,
	mfd@lists.linux.dev
Subject: Re: [PATCH v9 04/12] mfd: zx297520v3: Add a clock and reset MFD driver
Date: Sun, 02 Aug 2026 20:45:16 +0000	[thread overview]
Message-ID: <20260802204517.5126D1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260802-zx29clk-v9-4-d05530d85d28@gmail.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] Direct cast from `const void *` to an `enum` triggers a `-Wpointer-to-int-cast` compiler warning on 64-bit architectures.
- [Low] The Kconfig symbol `MFD_ZTE_ZX297520V3_CRM` lacks an architecture-specific dependency, unconditionally exposing a highly SoC-specific driver to all architectures.
- [Low] The enum `zx297520v3_mfd_type` includes the string 'mfd', violating the MFD subsystem naming conventions.
--

--- Patch [4]: [PATCH v9 04/12] mfd: zx297520v3: Add a clock and reset MFD driver ---
commit 39c5ccee8d6ebff3fee809871f3a8191107ab75b
Author: Stefan Dösinger <stefandoesinger@gmail.com>

mfd: zx297520v3: Add a clock and reset MFD driver

This driver registers child devices for the zx297520v3 clock and reset
controllers. The clk-zx297520v3 and reset-zte-zx297520v3 submitted in
the next patches will drive the respective functionalities.

> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 35f6e9b76d056..c0c7e83487f40 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
[ ... ]
> @@ -2572,5 +2572,16 @@ config MFD_MAX7360
>  	  additional drivers must be enabled in order to use the functionality
>  	  of the device.
>  
> +config MFD_ZTE_ZX297520V3_CRM
> +	tristate "ZTE zx297520v3 Clock and Reset Manager"
> +	select MFD_CORE
> +	default SOC_ZX297520V3

[Severity: Low]
Should this include an architecture dependency, such as
depends on ARCH_ZTE || COMPILE_TEST? Without it, this highly SoC-specific
driver will be presented in the configuration options for all architectures.

> +	help
> +	  Say yes here to enable the driver for the ZTE zx297520v3 clock and
> +	  reset manager MFD driver. This driver provides the host device for
> +	  the clock and reset drivers and is required to boot the SoC. You
> +	  will also need to enable CLK_ZTE_ZX297520V3 and RESET_ZTE_ZX297520V3
> +	  to build the actual clock and reset child drivers.
> +
>  endmenu
>  endif
> diff --git a/drivers/mfd/zte-zx297520v3-crm.c b/drivers/mfd/zte-zx297520v3-crm.c
> new file mode 100644
> index 0000000000000..37cd0c14680eb
> --- /dev/null
> +++ b/drivers/mfd/zte-zx297520v3-crm.c
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2026 Stefan Dösinger
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/mfd/core.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +
> +enum zx297520v3_mfd_type {
> +	ZX297520V3_MFD_INVALID = 0,
> +	ZX297520V3_MFD_TOPCRM,
> +	ZX297520V3_MFD_MATRIXCRM,
> +	ZX297520V3_MFD_LSPCRM,
> +};

[Severity: Low]
This isn't a bug, but does including the string mfd in the enum name violate
the MFD subsystem naming conventions? The guidelines suggest avoiding
hard-coding implementation details like mfd in driver, struct, or device
names.

[ ... ]

> +static int zx297520v3_crm_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	enum zx297520v3_mfd_type type;
> +	const struct mfd_cell *cells;
> +	struct reset_control *rst;
> +	unsigned int num_cells;
> +	struct clk *pclk;
> +
> +	type = (enum zx297520v3_mfd_type)of_device_get_match_data(dev);

[Severity: Low]
Could this direct cast from a const void * pointer to an enum trigger a
-Wpointer-to-int-cast compiler warning on 64-bit architectures? Often an
intermediate cast to uintptr_t or kernel_ulong_t is used to safely preserve
the size before casting to the enum type.

> +	switch (type) {
> +	case ZX297520V3_MFD_TOPCRM:
> +		cells = zx297520v3_topcrm_cells;
> +		num_cells = ARRAY_SIZE(zx297520v3_topcrm_cells);
> +		break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260802-zx29clk-v9-0-d05530d85d28@gmail.com?part=4

  reply	other threads:[~2026-08-02 20:45 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-02 20:33 [PATCH v9 00/12] ZTE zx297520v3 clock bindings and driver Stefan Dösinger
2026-08-02 20:33 ` Stefan Dösinger
2026-08-02 20:33 ` [PATCH v9 01/12] dt-bindings: clk: zte: Add zx297520v3 top clock and reset controller Stefan Dösinger
2026-08-02 20:33   ` Stefan Dösinger
2026-08-02 20:42   ` sashiko-bot
2026-08-02 20:42     ` sashiko-bot
2026-08-04  6:37   ` Krzysztof Kozlowski
2026-08-04  6:37     ` Krzysztof Kozlowski
2026-08-02 20:33 ` [PATCH v9 02/12] dt-bindings: clk: zte: Add zx297520v3 matrix " Stefan Dösinger
2026-08-02 20:33   ` Stefan Dösinger
2026-08-02 20:42   ` sashiko-bot
2026-08-02 20:42     ` sashiko-bot
2026-08-02 20:33 ` [PATCH v9 03/12] dt-bindings: clk: zte: Add zx297520v3 LSP " Stefan Dösinger
2026-08-02 20:33   ` Stefan Dösinger
2026-08-02 20:40   ` sashiko-bot
2026-08-02 20:40     ` sashiko-bot
2026-08-02 20:33 ` [PATCH v9 04/12] mfd: zx297520v3: Add a clock and reset MFD driver Stefan Dösinger
2026-08-02 20:33   ` Stefan Dösinger
2026-08-02 20:45   ` sashiko-bot [this message]
2026-08-02 20:45     ` sashiko-bot
2026-08-02 20:33 ` [PATCH v9 05/12] clk: zte: Add Clock registration infrastructure Stefan Dösinger
2026-08-02 20:33   ` Stefan Dösinger
2026-08-02 20:49   ` sashiko-bot
2026-08-02 20:49     ` sashiko-bot
2026-08-03 16:03   ` Brian Masney
2026-08-03 16:03     ` Brian Masney
2026-08-03 17:48     ` Stefan Dösinger
2026-08-03 17:48       ` Stefan Dösinger
2026-08-05 23:15       ` Brian Masney
2026-08-05 23:15         ` Brian Masney
2026-08-03 16:05   ` Brian Masney
2026-08-03 16:05     ` Brian Masney
2026-08-02 20:33 ` [PATCH v9 06/12] clk: zte: Add regmap-based clocks Stefan Dösinger
2026-08-02 20:33   ` Stefan Dösinger
2026-08-02 20:55   ` sashiko-bot
2026-08-02 20:55     ` sashiko-bot
2026-08-03 16:08   ` Brian Masney
2026-08-03 16:08     ` Brian Masney
2026-08-02 20:33 ` [PATCH v9 07/12] clk: zte: Add zx PLL support infrastructure Stefan Dösinger
2026-08-02 20:33   ` Stefan Dösinger
2026-08-02 20:52   ` sashiko-bot
2026-08-02 20:52     ` sashiko-bot
2026-08-03 16:14   ` Brian Masney
2026-08-03 16:14     ` Brian Masney
2026-08-02 20:33 ` [PATCH v9 08/12] clk: zte: Introduce a driver for zx297520v3 top clocks Stefan Dösinger
2026-08-02 20:33   ` Stefan Dösinger
2026-08-02 20:59   ` sashiko-bot
2026-08-02 20:59     ` sashiko-bot
2026-08-02 20:33 ` [PATCH v9 09/12] clk: zte: Introduce a driver for zx297520v3 matrix clocks Stefan Dösinger
2026-08-02 20:33   ` Stefan Dösinger
2026-08-02 21:26   ` sashiko-bot
2026-08-02 21:26     ` sashiko-bot
2026-08-02 20:33 ` [PATCH v9 10/12] clk: zte: Introduce a driver for zx297520v3 LSP clocks Stefan Dösinger
2026-08-02 20:33   ` Stefan Dösinger
2026-08-02 20:59   ` sashiko-bot
2026-08-02 20:59     ` sashiko-bot
2026-08-02 20:33 ` [PATCH v9 11/12] reset: zte: Add a zx297520v3 reset driver Stefan Dösinger
2026-08-02 20:33   ` Stefan Dösinger
2026-08-02 21:04   ` sashiko-bot
2026-08-02 21:04     ` sashiko-bot
2026-08-02 20:33 ` [PATCH v9 12/12] ARM: dts: zte: Declare zx297520v3 CRM device nodes Stefan Dösinger
2026-08-02 20:33   ` Stefan Dösinger
2026-08-02 21:03   ` sashiko-bot
2026-08-02 21:03     ` 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=20260802204517.5126D1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=lee@kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=mfd@lists.linux.dev \
    --cc=neil.armstrong@linaro.org \
    --cc=olteanv@gmail.com \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=stefandoesinger@gmail.com \
    --cc=vkoul@kernel.org \
    /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.