On Fr, 2025-02-14 at 17:13 +0000, Wilson Ding wrote:

quoted

-----Original Message-----
From: Philipp Zabel <p.zabel@pengutronix.de>
Sent: Friday, February 14, 2025 3:54 AM
To: Wilson Ding <redacted>; linux-kernel@vger.kernel.org;
devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org
Cc: andrew@lunn.ch; gregory.clement@bootlin.com;
sebastian.hesselbarth@gmail.com; robh@kernel.org; krzk+dt@kernel.org;
conor+dt@kernel.org; Sanghoon Lee [off-list ref]; Geethasowjanya
Akula [off-list ref]
Subject: [EXTERNAL] Re: [PATCH 1/4] [PATCH 1/4] reset: simple: Add syscon
device compatible

On Do, 2025-02-13 at 22:58 -0800, Wilson Ding wrote:

quoted

Introduce the new ops for updating reset line and getting status.
Thus, the reset controller can be accessed through either direct I/O
or regmap interfaces.

Please don't add a new layer of function pointer indirection, just add a new
struct reset_control_ops for the regmap variant.

If just adding a new struct reset_control_ops for the regmap variant, almost
all the functions will be duplicated for regmap variant. 
Besides reset_simple_regmap_assert/deassert(), we also need to have the
regmap version of reset_simple_update().

Yes. You could also duplicate/fold update() into assert/deassert().
It is trivial enough and the compiler will do that anyway.

Since reset_simple_reset() invokes
reset_simple_regmap_assert/deassert(), it also needs to be
duplicated.

That one could go through the data->rcdev.ops->assert/deassert function
pointers and be reused. But I wonder if that one function is worth the
added complexity.

In this case, there will be too many redundant codes in this file. I doubt if
it is worth to use the reset simple code. Maybe it's better to fork a new file
for the syscon device, such as 'reset-simple-syscon.c'. What do you say?

That sounds sensible to me.

regards
Philipp