From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id DA59225D216 for ; Mon, 9 Feb 2026 11:28:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770636503; cv=none; b=JkdIvu4pqo/vkvxtuqBMq6N6iFNYheCOS7I8mwG0eK7tHzCyi8zZK9XmFU7HAtEl/q62h4rKWj2KTgLzjHOLuNaQVbxtshon8vm2vRiFHZdQNub2eDyZ8cz26djzmchASlriexz+P5Gar57Vri8u4wwIgkbVgI97V+flpGyPfOA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770636503; c=relaxed/simple; bh=EA9uS4x0TBDmYFFLswYFQdz5aniFiogNv1tTt6XU+74=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Vc8rrCQxUZq+38vwv5qFeUjzBA5O66jJIMW5TYinooyQBLRuEjrN9U/+wZgs4QMBG4+N66B59VhPC4KbDXt3gPAgb0v1UyG5NT7BN6z3yJjlKsFhNQmAvAotIgxg/fBdEvHv6tStH+MRMs/JVqiKLEz6YPOw2mlz9Ku4ZPQYIAM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D1913339; Mon, 9 Feb 2026 03:28:15 -0800 (PST) Received: from localhost (e132581.arm.com [10.1.196.87]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id BC92E3F740; Mon, 9 Feb 2026 03:28:21 -0800 (PST) Date: Mon, 9 Feb 2026 11:28:19 +0000 From: Leo Yan To: James Clark Cc: coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Suzuki K Poulose , Mike Leach , Alexander Shishkin , Greg Kroah-Hartman , Mathieu Poirier , Mao Jinlong Subject: Re: [PATCH v3 8/8] coresight: Unify error handling in coresight_register() Message-ID: <20260209112819.GL3529712@e132581.arm.com> References: <20260202-arm_coresight_refactor_dev_register-v3-0-03292470c48c@arm.com> <20260202-arm_coresight_refactor_dev_register-v3-8-03292470c48c@arm.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Tue, Feb 03, 2026 at 11:15:55AM +0000, James Clark wrote: [...] > > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c > > index 955af43010446803030973c72f07315492b2fcf3..65cf975493c86de42515845147d90497aa20c595 100644 > > --- a/drivers/hwtracing/coresight/coresight-core.c > > +++ b/drivers/hwtracing/coresight/coresight-core.c > > @@ -1326,7 +1326,6 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) > > { > > int ret; > > struct coresight_device *csdev; > > - bool registered = false; > > csdev = kzalloc(sizeof(*csdev), GFP_KERNEL); > > if (!csdev) { > > @@ -1380,7 +1379,8 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) > > * All resources are free'd explicitly via > > * coresight_device_release(), triggered from put_device(). > > */ > > - goto out_unlock; > > + mutex_unlock(&coresight_mutex); > > + goto err_out; > > I'm not sure if replacing the "registered" system with extra calls to unlock > is necessarily better. I think the whole point of out_unlock was to have a > single call to unlock so it couldn't be forgotten or didn't need to be > duplicated. The motivation for this patch is to use out_unlock as a central point for releasing resources via coresight_unregister(). The tricky case is a device_register() failure. Since the device is not successfully registered, there is no need to call coresight_unregister() to release bus resources. However, the mutex and platform data still need to be released. The code here unlocks and jumps to err_out to release the platform data. > Probably a better way to clean this up would be to pull out a function for > all the stuff that needs to be locked and use guard(). Then do the stuff > that doesn't need to be locked after that function. Either way it doesn't > look wrong. If so, although locking is not a concern, the device_register() failures still need to release platform data particularly. That means we still need extra flag (or returned error) to indicate if it is a device_register() failure. I understand we don't want multiple places for mutex release. It is not bad to keep the "registered" flag and drop this patch. Thanks, Leo