From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-dl1-f53.google.com (mail-dl1-f53.google.com [74.125.82.53]) (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 4EBDA36606C for ; Fri, 6 Mar 2026 16:13:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.125.82.53 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772813586; cv=none; b=anXNgJFHuUF5m4CJp5cXaJSW2y5kK5pji3R3/Aw09BtwFLdS54EJASi5I5tsG7Xx9Z/eDDN5bSUCeb4e7uk+83/mlQM4msHhVZLT1eCJ7r+OzEZRZsV3tTCQZbG+5tR4guX6SutMZxeeYyJqw7D6aToctL7b2BEFTX7ovKD8jsg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772813586; c=relaxed/simple; bh=GbHZvMrlDjk98tFLySxmu7d7hM4MTcgCQPURO46KJPQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=V9Eb2yRbk+PQvIXpxn718fsRKJUl6IRis24yULjcRyWiRNJTXnGi/ajDwA4FPnKBKNtGcy9oElW6noA/3W5pnGQxfVW0yY7+fV7wFNHxNxHmJKQa8cVt2osVBDm278aVvp0rXpS/0hpRK1B6w3YZUapPo1s4+2CbjsOZ34XYUt8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=roeck-us.net; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=WcJ4ELEf; arc=none smtp.client-ip=74.125.82.53 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=roeck-us.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="WcJ4ELEf" Received: by mail-dl1-f53.google.com with SMTP id a92af1059eb24-1271195d2a7so499088c88.0 for ; Fri, 06 Mar 2026 08:13:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1772813584; x=1773418384; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:sender:from:to:cc:subject:date:message-id :reply-to; bh=2ExyNVSWsZEiPhZCTeAKx/MrxZrOY+AoCmCT2YcRTRo=; b=WcJ4ELEfu5j/mE0YjraLcHB+98yCpJoKbx+g8pRRZKw6ifhjvGWDZwVhUwAHnCq6Pp T6VMjZCl9AFwb7ZqHd7jFYhz+Uj7z60SzlzyuKxv+JTUXpcGmGK9WMXmha3l4fg2Hnmg R4RN99bLTJ6uR2dEclau1N7hk74ULzrVbWQd1qTm67Kizm28UpiUnUh+RG+BazEyI0zP frqfrzrBqYt/Ey9OEB6ELMw+AMOIn08U5A5BrJ2tIXMe6HXijO7UdqNCSusnkzjBQMKh q6uk3H+y0xV5n7gaqnAJUnxgBluACxN7nIgBep/ZKWEHW1XcGyTL4f9bklRqZzuZ8nEG 3ikQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1772813584; x=1773418384; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:sender:x-gm-gg:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=2ExyNVSWsZEiPhZCTeAKx/MrxZrOY+AoCmCT2YcRTRo=; b=kxUIGgPlaHzG6qHN/zk/S/SNKSkl7o3vVTjhUc5w1D/wJVLaTa5S2+RmkF5t8sFqws KA6LPfT6icmCORg5EXq2YiKE4MLeuwrExiYgsw6ZC6ezRAcq4puBRSQI/IS5kQpOWd46 QqINOHAYSH+vzNp7dqZQ768r4ZhKFn7WZ3/rWmnIZv6H98DTJ37sPJdK7jgoRtiKnzyH YVcpLEBs99J1LG08LB5I2L67JYE75k7H/zFmJ2LHLVka/xShnFqJiSq/Nq8wtLMxAUSP Hv/iXzEBx1x5/crIRlZE6KCB80ZTF2HlMH7imueHwoyDhlD+zn7K1HaOn9M7mY+yPnV2 gUSw== X-Forwarded-Encrypted: i=1; AJvYcCU1oSrnz3YypeJX78hif7l26LP+pwDzds6UfQlN7FrrhVfKDk+EGek8zRIUom7x0zuynn6zDGzaEXBVTyw=@vger.kernel.org X-Gm-Message-State: AOJu0Yz6IrSaBrZEJ3kgj8t749pyFa35WYaR7vk1XMArsmJEYCRe0tZ7 9YY1/a8dA9l7rNM6Sl670e4CGS1FI52IrgPOgKUbVLOvYr5LRxh3Xm9d X-Gm-Gg: ATEYQzw/3700TP3bS44r0Dy8VQo0dRWU62y403gks87e6HQanxYia6AuHmSmGUaew4f 6OSH7kGQeZ6iAmGLLlpEMFrbGdoXl9bzkgH/iXayO2L4WU//lXaoIsnaSmDiw6EDi45UeRShD14 oA9cL8ZIs9OpW2kEyNIU25raKKV4O3ckM+HIwadPaB967PmfLG9KmcjmOz1rS9SsNOi+4WH24fp JLVnTFNtsP0nxskSAs6rQ3Z6pAkj68FaFUln2Djy679gpdJfCpFo+1N4FuAV/Z7RIQcadK7Wz2v snWvodmzDhZISAfIDGTNZA0AezXH3jKilrD1xLwIhOk2v5www36/i5EsrX6caIueCniDRj/Q7Ds gRZ/9OP4IpvA+d4zQRg2kL4A0Na9uVxcB11fAbNZ5BxR+4Ne+zeuWjm8XjXJPqoKKyb3WSOXr6Y /UbNGgMx4B5GwoXf6SQ/WPH9wMf2/NtkR+Bqee X-Received: by 2002:a05:7300:320a:b0:2b7:fdb6:ccf2 with SMTP id 5a478bee46e88-2be4de99574mr1150793eec.13.1772813584166; Fri, 06 Mar 2026 08:13:04 -0800 (PST) Received: from server.roeck-us.net ([2600:1700:e321:62f0:da43:aeff:fecc:bfd5]) by smtp.gmail.com with ESMTPSA id 5a478bee46e88-2be4f94823esm1578036eec.20.2026.03.06.08.13.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 06 Mar 2026 08:13:03 -0800 (PST) Sender: Guenter Roeck Date: Fri, 6 Mar 2026 08:13:02 -0800 From: Guenter Roeck To: Billy Tsai Cc: Joel Stanley , Andrew Jeffery , linux-hwmon@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] hwmon: (aspeed-g6-pwm-tach): fix driver data handling in probe/remove Message-ID: References: <20260306-pwm_fixes-v1-1-e8ede6fbaf47@aspeedtech.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: <20260306-pwm_fixes-v1-1-e8ede6fbaf47@aspeedtech.com> On Fri, Mar 06, 2026 at 02:44:31PM +0800, Billy Tsai wrote: > Ensure proper association of driver data by setting and retrieving > the platform device's driver data during probe and remove. > > Fixes: 7e1449cd15d1 ("hwmon: (aspeed-g6-pwm-tacho): Support for ASPEED g6 PWM/Fan tach") > Signed-off-by: Billy Tsai > --- > drivers/hwmon/aspeed-g6-pwm-tach.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/hwmon/aspeed-g6-pwm-tach.c b/drivers/hwmon/aspeed-g6-pwm-tach.c > index 44e1ecba205d..0c03d272279a 100644 > --- a/drivers/hwmon/aspeed-g6-pwm-tach.c > +++ b/drivers/hwmon/aspeed-g6-pwm-tach.c > @@ -490,6 +490,7 @@ static int aspeed_pwm_tach_probe(struct platform_device *pdev) > if (IS_ERR(chip)) > return PTR_ERR(chip); > > + platform_set_drvdata(pdev, chip); > pwmchip_set_drvdata(chip, priv); > chip->ops = &aspeed_pwm_ops; > > @@ -519,7 +520,8 @@ static int aspeed_pwm_tach_probe(struct platform_device *pdev) > > static void aspeed_pwm_tach_remove(struct platform_device *pdev) > { > - struct aspeed_pwm_tach_data *priv = platform_get_drvdata(pdev); > + struct pwm_chip *chip = platform_get_drvdata(pdev); > + struct aspeed_pwm_tach_data *priv = aspeed_pwm_chip_to_data(chip); > > reset_control_assert(priv->reset); AI review has the same (and more) concerns. Here is what it has to say: Does asserting the reset here place the hardware into reset while the sysfs interfaces are still active? Since probe uses devm_pwmchip_add() and devm_hwmon_device_register_with_info(), those devices are unregistered during devres cleanup, which happens after this remove function returns. If userspace accesses the devices before devres cleanup finishes, it could access an IP block that is in reset and cause a system hang. Also, since probe already registers a devres action (devm_add_action_or_reset()) to call aspeed_pwm_tach_reset_assert(), this explicit assert appears to be redundant. Can the remove function be dropped entirely? This isn't a bug, but probe calls of_platform_populate() without a corresponding of_platform_depopulate(). Using devm_of_platform_populate() in probe would fix the leak and allow deleting this remove function. I think all those points are valid. Thanks, Guenter