Re: [PATCH 3/3] pwm: lpss: Simplify using devm_pwmchip_add

From: Thierry Reding <thierry.reding@gmail.com>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Lee Jones <lee.jones@linaro.org>,
	linux-pwm@vger.kernel.org, kernel@pengutronix.de
Subject: Re: [PATCH 3/3] pwm: lpss: Simplify using devm_pwmchip_add
Date: Fri, 9 Apr 2021 15:28:07 +0200	[thread overview]
Message-ID: <YHBWZw8haF0SU1Cc@orome.fritz.box> (raw)
In-Reply-To: <20210407080155.55004-3-u.kleine-koenig@pengutronix.de>

[-- Attachment #1: Type: text/plain, Size: 1528 bytes --]

On Wed, Apr 07, 2021 at 10:01:55AM +0200, Uwe Kleine-König wrote:
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/pwm/pwm-lpss-pci.c      | 4 ----
>  drivers/pwm/pwm-lpss-platform.c | 4 +---
>  drivers/pwm/pwm-lpss.c          | 8 +-------
>  drivers/pwm/pwm-lpss.h          | 1 -
>  4 files changed, 2 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-lpss-pci.c b/drivers/pwm/pwm-lpss-pci.c
> index cf749ea0de9f..c893ec3d2fb4 100644
> --- a/drivers/pwm/pwm-lpss-pci.c
> +++ b/drivers/pwm/pwm-lpss-pci.c
> @@ -69,12 +69,8 @@ static int pwm_lpss_probe_pci(struct pci_dev *pdev,
>  
>  static void pwm_lpss_remove_pci(struct pci_dev *pdev)
>  {
> -	struct pwm_lpss_chip *lpwm = pci_get_drvdata(pdev);
> -
>  	pm_runtime_forbid(&pdev->dev);
>  	pm_runtime_get_sync(&pdev->dev);
> -
> -	pwm_lpss_remove(lpwm);
>  }

Isn't this going to defeat your quest to make all drivers release
resources only after pwmchip_remove() was called? Before this patch
you'd be able to fix that by moving pwm_lpss_remove() before the runtime
PM stuff, but after using devm_pwmchip_add() the pwmchip_remove() will
always get called after the driver's ->remove() returns.

Granted, in this case it's perhaps not the best example because this
driver is actually grabbing a runtime PM reference, so that should be
safe. However, I'm thinking in general device-managed PWM chip addition
and removal becomes less useful because of the ordering requirements.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]