linux-kernel - RE: [PATCH v2 3/8] clk: imx: add driver for imx8ulp's sim lpav

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <PAXPR04MB84599A2457B84474ED6FD3ED88F0A@PAXPR04MB8459.eurprd04.prod.outlook.com>
Date: Thu, 23 Oct 2025 07:59:23 +0000
From: Peng Fan <peng.fan@....com>
To: "Peng Fan (OSS)" <peng.fan@....nxp.com>, Laurentiu Mihalcea
	<laurentiumihalcea111@...il.com>
CC: Abel Vesa <abelvesa@...nel.org>, Michael Turquette
	<mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>, Rob Herring
	<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
	<conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>, Fabio Estevam
	<festevam@...il.com>, Philipp Zabel <p.zabel@...gutronix.de>, Daniel Baluta
	<daniel.baluta@....com>, "S.J. Wang" <shengjiu.wang@....com>,
	"linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
	"imx@...ts.linux.dev" <imx@...ts.linux.dev>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, Pengutronix Kernel Team
	<kernel@...gutronix.de>
Subject: RE: [PATCH v2 3/8] clk: imx: add driver for imx8ulp's sim lpav

> Subject: Re: [PATCH v2 3/8] clk: imx: add driver for imx8ulp's sim lpav
> 
> Hi Laurentiu,
> 
> On Fri, Oct 17, 2025 at 04:20:20AM -0700, Laurentiu Mihalcea wrote:
> >From: Laurentiu Mihalcea <laurentiu.mihalcea@....com>
> >
> >The i.MX8ULP System Integration Module (SIM) LPAV module is a
> block
> >control module found inside the LPAV subsystem, which offers some
> clock
> >gating options and reset line assertion/de-assertion capabilities.
> >
> >Therefore, the clock gate management is supported by registering the
> >module's driver as a clock provider, while the reset capabilities are
> >managed via the auxiliary device API to allow the DT node to act as a
> >reset and clock provider.
> >
> >Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@....com>
> >---
> ....
> >+struct clk_imx8ulp_sim_lpav_data {
> >+	void __iomem *base;
> >+	struct regmap *regmap;
> >+	spinlock_t lock; /* shared by MUX, clock gate and reset */
> >+	unsigned long flags; /* for spinlock usage */
> 
> This does not need to be here, put it as function local variable should
> be fine.

Ignore this comment. It should be in the structure.

Thanks,
Peng.

> 
> >+	struct clk_hw_onecell_data clk_data; /*  keep last */ };
> >+
> >+struct clk_imx8ulp_sim_lpav_gate {
> >+	const char *name;
> >+	int id;
> >+	const struct clk_parent_data parent;
> >+	u8 bit;
> >+};
> >+
> >+static struct clk_imx8ulp_sim_lpav_gate gates[] = {
> >+	IMX8ULP_HIFI_CLK_GATE("hifi_core", CORE, "hifi_core", 17),
> >+	IMX8ULP_HIFI_CLK_GATE("hifi_pbclk", PBCLK, "lpav_bus", 18),
> >+	IMX8ULP_HIFI_CLK_GATE("hifi_plat", PLAT, "hifi_plat", 19)
> 
> For the parent name, my understanding is they should be the one from
> clk-imx8ulp.c, but I not find them, or may I miss something?
> 
> >+};
> >+
> >+#ifdef CONFIG_RESET_CONTROLLER
> >+static void clk_imx8ulp_sim_lpav_aux_reset_release(struct device
> *dev)
> >+{
> >+	struct auxiliary_device *adev = to_auxiliary_dev(dev);
> >+
> >+	kfree(adev);
> >+}
> >+
> >+static void clk_imx8ulp_sim_lpav_unregister_aux_reset(void *data) {
> >+	struct auxiliary_device *adev = data;
> >+
> >+	auxiliary_device_delete(adev);
> >+	auxiliary_device_uninit(adev);
> >+}
> >+
> >+static int clk_imx8ulp_sim_lpav_register_aux_reset(struct
> >+platform_device *pdev) {
> >+	struct auxiliary_device *adev __free(kfree) = NULL;
> >+	int ret;
> >+
> >+	adev = kzalloc(sizeof(*adev), GFP_KERNEL);
> >+	if (!adev)
> >+		return -ENOMEM;
> >+
> >+	adev->name = "reset";
> >+	adev->dev.parent = &pdev->dev;
> >+	adev->dev.release = clk_imx8ulp_sim_lpav_aux_reset_release;
> >+
> >+	ret = auxiliary_device_init(adev);
> >+	if (ret) {
> >+		dev_err(&pdev->dev, "failed to initialize aux dev\n");
> >+		return ret;
> >+	}
> >+
> >+	ret = auxiliary_device_add(adev);
> >+	if (ret) {
> >+		auxiliary_device_uninit(adev);
> >+		dev_err(&pdev->dev, "failed to add aux dev\n");
> >+		return ret;
> >+	}
> >+
> >+	return devm_add_action_or_reset(&pdev->dev,
> >+
> 	clk_imx8ulp_sim_lpav_unregister_aux_reset,
> >+					no_free_ptr(adev));
> 
> clk_imx8ulp_sim_lpav_unregister_aux_reset() clean up the resources, if
> moving this before auxiliary_device_add(), then no need
> auxiliary_device_uninit() when add fails?
> 
> >+}
> 
> Regards
> Peng