From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtpout-03.galae.net (smtpout-03.galae.net [185.246.85.4]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EBCB73F1678 for ; Tue, 28 Jul 2026 10:01:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.246.85.4 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785232886; cv=none; b=lwjrhtTe48l86tGls7ADfV+70aKXzqVclp6MXSHp9zu3cGU3+3ANSrI3bHW2ljPINHreRnFOxgf4baUvD5A//wXS+qHgZFQu7y0dMCwtJkcZvJqkUDBp6eZBQjxxWFAKYDGZgCwhjWHllAH3u35mavj3y0PdBynTNKcxfrsNIpk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785232886; c=relaxed/simple; bh=OztgE34fAwdTdWSUItsXsyOWLjwHL/0oqpUJUOiF3S4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=kYlGWqv69P0WUi2yuDMzMYPQiwN6yUuJlzvHs0FwVAU7j+OQAgWDKWhpS7/A+qrtZep5BB0jImHtjvWd4omJNY8ibqRUD4VafI8QFVjmfFbCHxnVFUwL5BnGstPGSfZdUwMBlXyUdJl4kXl5j2RZrBudq3/ITXv7+YaWuTJ5n8Y= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com; spf=pass smtp.mailfrom=bootlin.com; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b=ECzlfi5a; arc=none smtp.client-ip=185.246.85.4 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="ECzlfi5a" Received: from smtpout-01.galae.net (smtpout-01.galae.net [212.83.139.233]) by smtpout-03.galae.net (Postfix) with ESMTPS id 03CBE4E40FC3; Tue, 28 Jul 2026 10:01:21 +0000 (UTC) Received: from mail.galae.net (mail.galae.net [212.83.136.155]) by smtpout-01.galae.net (Postfix) with ESMTPS id B83DB601BF; Tue, 28 Jul 2026 10:01:20 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id C278711C13F19; Tue, 28 Jul 2026 12:01:09 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=dkim; t=1785232874; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:in-reply-to:references; bh=V6Dnu/BcOn6F3D3IfNT8dfH1gBGEupvmcZmtLIc3c1g=; b=ECzlfi5aqe+hP3Nyeb2r937k6WdR2ZEh2JftXD2lfQ+nlsyYgGBkzkohiHD53/xC1wyqug IMB3sFnj1HzdHmSK4tEuIW6jsltItT7ieJQlwESeiWZyLwnweDqkkvbhav9IA+OSNMha2r kCG8fRKu93d+BQbIKSJrHZqO/J1TQR91Q/lfmuq7vyTWpJrOm1urZCcMfy0WCXENNGxWk3 MEo5OqT8M7YGpy2TiTm8rr5Yj8pEBiz+41b4ikjdk2BypkTmOIAJr6Pmq2vBfZY1ZHpjk8 kzZ7lZGZxb77Cld87fLpO+4GkD4CPmKRyP1xEl/gA5dmrGOu6HrEiMWKM4RPZA== Date: Tue, 28 Jul 2026 12:01:09 +0200 From: Alexandre Belloni To: =?utf-8?B?7LWc7Jyg7Zi4?= Cc: Daniel Lezcano , Thomas Gleixner , Nicolas Ferre , Claudiu Beznea , Boris Brezillon , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v1] clocksource/drivers/timer-atmel-pit: Fix init failure cleanup Message-ID: <20260728100109ca239fcb@mail.local> References: <20260608182016.99715-1-dbgh9129@gmail.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=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Last-TLS-Session-Version: TLSv1.3 Hello, On 02/07/2026 17:12:48-0400, 최유호 wrote: > Hi, > > Just a gentle ping on this patch. > > I would appreciate any feedback when you have a chance to review this. > > Thanks > > On Mon, 8 Jun 2026 at 14:20, Yuho Choi wrote: > > > > After clk_prepare_enable(data->mck) succeeds, at91sam926x_pit_dt_init() > > can still fail while parsing the IRQ, registering the clocksource, or > > requesting the IRQ. These paths only free the driver data, leaving the > > master clock enabled. > > > > Unwind each initialized state on failure. Stop the PIT after it has been > > started, dispose the IRQ mapping after it has been created, disable and > > put the master clock, and unmap the registers before freeing the driver > > data. > > This is a bit moot because if anything in the probe fails, then he platform will probably not boot at all. > > Fixes: 699e36e5b8e9 ("clocksource/drivers/timer-atmel-pit: Enable mck clock") > > Signed-off-by: Yuho Choi > > --- > > drivers/clocksource/timer-atmel-pit.c | 21 ++++++++++++++++----- > > 1 file changed, 16 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/clocksource/timer-atmel-pit.c b/drivers/clocksource/timer-atmel-pit.c > > index 888b06731e54..edd427ab93e6 100644 > > --- a/drivers/clocksource/timer-atmel-pit.c > > +++ b/drivers/clocksource/timer-atmel-pit.c > > @@ -12,7 +12,9 @@ > > #include > > #include > > #include > > +#include > > #include > > +#include > > #include > > #include > > #include > > @@ -185,13 +187,13 @@ static int __init at91sam926x_pit_dt_init(struct device_node *node) > > if (IS_ERR(data->mck)) { > > pr_err("Unable to get mck clk\n"); > > ret = PTR_ERR(data->mck); > > - goto exit; > > + goto exit_iounmap; > > } > > > > ret = clk_prepare_enable(data->mck); > > if (ret) { > > pr_err("Unable to enable mck\n"); > > - goto exit; > > + goto exit_clk_put; > > } Switching to devm_clk_get_enabled would avoid most of the error handling. Similarly, devm_ioremap could be used earlier. > > > > /* Get the interrupts property */ > > @@ -199,7 +201,7 @@ static int __init at91sam926x_pit_dt_init(struct device_node *node) > > if (!data->irq) { > > pr_err("Unable to get IRQ from DT\n"); > > ret = -EINVAL; > > - goto exit; > > + goto exit_clk_disable; > > } > > > > /* > > @@ -227,7 +229,7 @@ static int __init at91sam926x_pit_dt_init(struct device_node *node) > > ret = clocksource_register_hz(&data->clksrc, pit_rate); > > if (ret) { > > pr_err("Failed to register clocksource\n"); > > - goto exit; > > + goto exit_pit_disable; > > } > > > > /* Set up irq handler */ > > @@ -237,7 +239,7 @@ static int __init at91sam926x_pit_dt_init(struct device_node *node) > > if (ret) { > > pr_err("Unable to setup IRQ\n"); > > clocksource_unregister(&data->clksrc); > > - goto exit; > > + goto exit_pit_disable; > > } > > > > /* Set up and register clockevents */ > > @@ -256,6 +258,15 @@ static int __init at91sam926x_pit_dt_init(struct device_node *node) > > > > return 0; > > > > +exit_pit_disable: > > + pit_write(data->base, AT91_PIT_MR, 0); I doubt this write is useful. > > + irq_dispose_mapping(data->irq); > > +exit_clk_disable: > > + clk_disable_unprepare(data->mck); > > +exit_clk_put: > > + clk_put(data->mck); > > +exit_iounmap: > > + iounmap(data->base); > > exit: > > kfree(data); > > return ret; > > -- > > 2.43.0 > > -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com