linux-kernel - Re: [PATCH v6 3/6] clk: spacemit: set up reset auxiliary devices

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: <dcc42499-4c6b-450e-8449-414501d6ab62@riscstar.com>
Date: Thu, 8 May 2025 15:04:46 -0500
From: Alex Elder <elder@...cstar.com>
To: Haylen Chu <heylenay@....org>, robh@...nel.org, krzk+dt@...nel.org,
 conor+dt@...nel.org, mturquette@...libre.com, sboyd@...nel.org,
 p.zabel@...gutronix.de, paul.walmsley@...ive.com, palmer@...belt.com,
 aou@...s.berkeley.edu, alex@...ti.fr, dlan@...too.org
Cc: inochiama@...look.com, guodong@...cstar.com, devicetree@...r.kernel.org,
 linux-clk@...r.kernel.org, spacemit@...ts.linux.dev,
 linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 3/6] clk: spacemit: set up reset auxiliary devices

On 5/7/25 11:46 PM, Haylen Chu wrote:
> On Tue, May 06, 2025 at 04:06:34PM -0500, Alex Elder wrote:
>> Add a new reset_name field to the spacemit_ccu_data structure.  If it is
>> non-null, the CCU implements a reset controller, and the name will be
>> used as the name for the auxiliary device that implements it.
>>
>> Define a new type to hold an auxiliary device as well as the regmap
>> pointer that will be needed by CCU reset controllers.  Set up code to
>> initialize and add an auxiliary device for any CCU that implements reset
>> functionality.
>>
>> Make it optional for a CCU to implement a clock controller.  This
>> doesn't apply to any of the existing CCUs but will for some new ones
>> that will be added soon.
>>
>> Signed-off-by: Alex Elder <elder@...cstar.com>
>> ---
>>   drivers/clk/spacemit/ccu-k1.c | 85 +++++++++++++++++++++++++++++++----
>>   include/soc/spacemit/ccu_k1.h | 12 +++++
>>   2 files changed, 89 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/clk/spacemit/ccu-k1.c b/drivers/clk/spacemit/ccu-k1.c
>> index 9545cfe60b92b..6b1845e899e5f 100644
>> --- a/drivers/clk/spacemit/ccu-k1.c
>> +++ b/drivers/clk/spacemit/ccu-k1.c
> 
> ...
> 
>> +static void spacemit_cadev_release(struct device *dev)
>> +{
>> +	struct auxiliary_device *adev = to_auxiliary_dev(dev);
>> +
>> +	kfree(to_spacemit_ccu_adev(adev));
>> +}
> 
> spacemit_ccu_adev structures are allocated with devm_kzalloc() in
> spacemit_ccu_reset_register(), which means its lifetime is bound to the
> driver and it'll be automatically released after driver removal; won't
> there be a possibility of double-free? I think the release callback
> could be simply dropped.

You are correct.  And unfortunately I didn't include the fix
for this in the patches I just posted, because somehow this
message was not included with the group in my mail program.

I'm going to send v8 after I fix this and verify it again.

					-Alex


> ...
> 
>> +static int spacemit_ccu_reset_register(struct device *dev,
>> +				       struct regmap *regmap,
>> +				       const char *reset_name)
>> +{
>> +	struct spacemit_ccu_adev *cadev;
>> +	struct auxiliary_device *adev;
>> +	static u32 next_id;
>> +	int ret;
>> +
>> +	/* Nothing to do if the CCU does not implement a reset controller */
>> +	if (!reset_name)
>> +		return 0;
>> +
>> +	cadev = devm_kzalloc(dev, sizeof(*cadev), GFP_KERNEL);
> 
> Here spacemit_ccu_adev is allocated.
> 
>> +	if (!cadev)
>> +		return -ENOMEM;
>> +	cadev->regmap = regmap;
>> +
>> +	adev = &cadev->adev;
>> +	adev->name = reset_name;
>> +	adev->dev.parent = dev;
>> +	adev->dev.release = spacemit_cadev_release;
>> +	adev->dev.of_node = dev->of_node;
>> +	adev->id = next_id++;
>> +
>> +	ret = auxiliary_device_init(adev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = auxiliary_device_add(adev);
>> +	if (ret) {
>> +		auxiliary_device_uninit(adev);
>> +		return ret;
>> +	}
>> +
>> +	return devm_add_action_or_reset(dev, spacemit_adev_unregister, adev);
>> +}
>> +
> 
> Best regards,
> Haylen Chu