From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f51.google.com (mail-wr1-f51.google.com [209.85.221.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4DD4E433D4 for ; Wed, 1 May 2024 13:32:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.51 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714570372; cv=none; b=hau5MGOGKw4D4UycjtHI9+s2160+xDuKoomtLwvYOEHhulZ6WtP0kxNmqs5Eff+MtN/681+Ak5dnYUtYugo5dBhdhWfFFYWPaILxOlITS6uRtzIFseL8BFLCXUqXntlOL4jqjap27GfWo+2XLFRwVOXmygiUb+E+MDH5J2qd0eA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714570372; c=relaxed/simple; bh=ZqcDVgO1hmm+00KuoWmbr7W1NZLQvg1QhUiwr8SEKP0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=KFnhbbm+h9i6E3+E7U2YpEcqIjGrri3xnFq8sm9BJpwY0Fs7VY3wYNUPehpQN4eUku/KVS0hHC8yjsOxa7esoEcuVcBkanVF4FCaqqjI25zRqAJKj9uT4SxXgreB53hGzFrbJ0F8QBHekZAuSk2CkSivNr7GKsJebHYOJVjfClU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=F6vq4feq; arc=none smtp.client-ip=209.85.221.51 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="F6vq4feq" Received: by mail-wr1-f51.google.com with SMTP id ffacd0b85a97d-34d7b0dac54so459764f8f.0 for ; Wed, 01 May 2024 06:32:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1714570370; x=1715175170; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=ior2gPFVlEOyeV8FJQcXfdx4VXM0R/609sKh6Of53VM=; b=F6vq4feq2N5K6oTj556OwcPXVHZ7iw0HaZm1/3fK7K35ZYPLVA0aEuiko5H4MqYkQh qSe5VEjBpSX5FByvdImvMsfRPr6G+ZrVMz0YoI/HbJ3QeVzdcsCZ6ZekbH6GTvtKEgj1 WBCFyOatej8cYNzphRSYOmgr3QN/8q7hkrGHzYwN2POMuPVHWKe9DGe0pKFAEUO1l1t6 mWgb4K4+leSysSgczOEMCz+aChc6vudaAWSlV36rkLHpFSd8ZfJb/sGN0wvi8bxq04eA vcRNjkl/2HVD/Q82Qhv9vnhuCGOS9syNHALjM4eJ72PUF/+Nuxe0rEm7F19s8DlUBlhg xPaQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714570370; x=1715175170; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=ior2gPFVlEOyeV8FJQcXfdx4VXM0R/609sKh6Of53VM=; b=rGe0mYOUFsZtZVoO1nwA0a71iG2t6MlgN5YhPDGzH2mKSJ3naeDGFRLAqiFxFsqMPf NVIhhbAknr3jMTCU3Z3mvxizkgZnLIYz0w8uowlgGft/zLADnMYFDoIunbC3c6aF/Bni u7NMniDgvcwLOC6fu1thm3JBlKcYa5P280KKS3ApRbzt531OatHg9FxBeiA3G2xM08QX ePuLseG30bWogbx0v2/JxxwminsJldxvg5RJNyS+ltv/YZa4fDOcIOr4x1rSfR2bKgrs W7IPQPN2lLwW8VO2SFdFIC4qb3TQnBzQn9+eZbpPiRKJLCCTyoSsJyaH2Dv4emktatvD 8Qdg== X-Forwarded-Encrypted: i=1; AJvYcCWUYT8mzSP1GE8DQA6UOrbBy3G+BNEAQgsy2F0djhlYq2BNxTuSkmue324x6RdnLvflFp6jju0wuOJVjZtaDJNbBf3/LfzbsmRHxUw= X-Gm-Message-State: AOJu0YwQQwFH1mRIjWJLPm5QHDzduOnSITOry1CQL1otl1DJcF6Pzk/P bFwi1eusTv2BY8Bg8gjk6OCMwsZ1Cd/hPgR4CqrjgeXx6Gy+c826GM2FoOqaJWo= X-Google-Smtp-Source: AGHT+IHuNu3c35RsAkvpJAfWqKVWQhv6Z4Ac0yjTjDmpBKt12bv5Zna3drXEkGYfHrENBoZl/SwOjw== X-Received: by 2002:a05:6000:24f:b0:34c:bf22:73f9 with SMTP id m15-20020a056000024f00b0034cbf2273f9mr2265612wrz.28.1714570369370; Wed, 01 May 2024 06:32:49 -0700 (PDT) Received: from localhost ([102.222.70.76]) by smtp.gmail.com with ESMTPSA id d4-20020adfe2c4000000b0034a3a0a753asm32876234wrj.100.2024.05.01.06.32.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 May 2024 06:32:49 -0700 (PDT) Date: Wed, 1 May 2024 16:32:40 +0300 From: Dan Carpenter To: "Peng Fan (OSS)" Cc: Linus Walleij , Thierry Reding , Jonathan Hunter , Dvorkin Dmitry , Wells Lu , Maxime Coquelin , Alexandre Torgue , Emil Renner Berthing , Jianlong Huang , Hal Feng , Orson Zhai , Baolin Wang , Chunyan Zhang , Viresh Kumar , Shiraz Hashim , soc@kernel.org, Krzysztof Kozlowski , Sylwester Nawrocki , Alim Akhtar , Geert Uytterhoeven , Patrice Chotard , Heiko Stuebner , Damien Le Moal , Ludovic Desroches , Nicolas Ferre , Alexandre Belloni , Claudiu Beznea , Dong Aisheng , Fabio Estevam , Shawn Guo , Jacky Bai , Pengutronix Kernel Team , Chester Lin , Matthias Brugger , Ghennadi Procopciuc , Sean Wang , Matthias Brugger , AngeloGioacchino Del Regno , Sascha Hauer , Andrew Jeffery , Joel Stanley , linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org, linux-tegra@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-stm32@st-md-mailman.stormreply.com, linux-samsung-soc@vger.kernel.org, linux-renesas-soc@vger.kernel.org, linux-rockchip@lists.infradead.org, linux-riscv@lists.infradead.org, linux-mediatek@lists.infradead.org, imx@lists.linux.dev, linux-aspeed@lists.ozlabs.org, openbmc@lists.ozlabs.org, Peng Fan Subject: Re: [PATCH 01/21] pinctrl: ti: iodelay: Use scope based of_node_put() cleanups Message-ID: References: <20240501-pinctrl-cleanup-v1-0-797ceca46e5c@nxp.com> <20240501-pinctrl-cleanup-v1-1-797ceca46e5c@nxp.com> Precedence: bulk X-Mailing-List: linux-tegra@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: <20240501-pinctrl-cleanup-v1-1-797ceca46e5c@nxp.com> On Wed, May 01, 2024 at 08:55:59PM +0800, Peng Fan (OSS) wrote: > @@ -879,16 +874,12 @@ static int ti_iodelay_probe(struct platform_device *pdev) > ret = pinctrl_register_and_init(&iod->desc, dev, iod, &iod->pctl); > if (ret) { > dev_err(dev, "Failed to register pinctrl\n"); > - goto exit_out; > + return ret; > } > > platform_set_drvdata(pdev, iod); > > return pinctrl_enable(iod->pctl); > - > -exit_out: > - of_node_put(np); > - return ret; > } This will call of_node_put() on the success path so it's a behavior change. The original code is buggy, it's supposed to call of_node_put() on the success path here or in ti_iodelay_remove(). If it's supposed to call of_node_put() here, then fine, this is bugfix but if it's supposed to call it in ti_iodelay_remove() then we need to save the pointer somewhere using no_free_ptr(). Probably saving ->np is the safest choice? The original code is already a little bit buggy because it doesn't check for pinctrl_enable() errors and cleanup. diff --git a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c index 040f2c46a868..f40a1476e4ff 100644 --- a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c +++ b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c @@ -156,6 +156,7 @@ struct ti_iodelay_device { const struct ti_iodelay_reg_data *reg_data; struct ti_iodelay_reg_values reg_init_conf_values; + struct device_node *np; }; /** @@ -884,7 +885,12 @@ static int ti_iodelay_probe(struct platform_device *pdev) platform_set_drvdata(pdev, iod); - return pinctrl_enable(iod->pctl); + ret = pinctrl_enable(iod->pctl); + if (ret) + goto exit_out; + + iod->np = no_free_ptr(np); + return 0; exit_out: of_node_put(np); @@ -903,6 +909,7 @@ static void ti_iodelay_remove(struct platform_device *pdev) pinctrl_unregister(iod->pctl); ti_iodelay_pinconf_deinit_dev(iod); + of_node_put(iod->np); /* Expect other allocations to be freed by devm */ }