> -----Original Message----- > From: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> > Sent: Friday, February 14, 2025 10:03 AM > To: Wilson Ding <dingwei@xxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > Cc: andrew@xxxxxxx; gregory.clement@xxxxxxxxxxx; > sebastian.hesselbarth@xxxxxxxxx; robh@xxxxxxxxxx; krzk+dt@xxxxxxxxxx; > conor+dt@xxxxxxxxxx; Sanghoon Lee <salee@xxxxxxxxxxx>; Geethasowjanya > Akula <gakula@xxxxxxxxxxx> > Subject: [EXTERNAL] Re: [PATCH 1/4] [PATCH 1/4] reset: simple: Add syscon > device compatible > > On Fr, 2025-02-14 at 17: 13 +0000, Wilson Ding wrote: > > > -----Original > Message----- > > From: Philipp Zabel <p. zabel@ pengutronix. de> > > Sent: > Friday, February 14, 2025 3: 54 AM > > To: Wilson Ding > <dingwei@ marvell. com>; > On Fr, 2025-02-14 at 17:13 +0000, Wilson Ding wrote: > > > > > -----Original Message----- > > > From: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> > > > Sent: Friday, February 14, 2025 3:54 AM > > > To: Wilson Ding <dingwei@xxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; > > > devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > > > Cc: andrew@xxxxxxx; gregory.clement@xxxxxxxxxxx; > > > sebastian.hesselbarth@xxxxxxxxx; robh@xxxxxxxxxx; > > > krzk+dt@xxxxxxxxxx; > > > conor+dt@xxxxxxxxxx; Sanghoon Lee <salee@xxxxxxxxxxx>; > > > conor+Geethasowjanya > > > Akula <gakula@xxxxxxxxxxx> > > > 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: > > > > 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. > Well. I will go with the approach of a new driver, which avoids all the unnecessary complexity and redundance. Thank! > regards > Philipp