Linux Tegra architecture development
 help / color / mirror / Atom feed
* [PATCH v1 0/6] pwm: tegra: Cleanups and .get_state()
@ 2026-07-14 12:02 Uwe Kleine-König
  2026-07-14 12:02 ` [PATCH v1 1/6] pwm: tegra: Check for match_data being NULL Uwe Kleine-König
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2026-07-14 12:02 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Mikko Perttunen, Philipp Zabel
  Cc: linux-pwm, linux-tegra

Hello,

the tegra PWM driver annoyed me for some time. This patch set addresses
some of the concerns. The implementation of .get_state() is the check if
I understood the hardware correctly. If so, the next step is to fix the
rounding in .apply() and/or convert to the waveform callbacks.

The first 5 patches should be fine (unless my understanding of
pm_runtime is wrong, which is quite possible), for the 6th patch I'd
like someone to test it before it gets applied.

Best regards
Uwe

Uwe Kleine-König (6):
  pwm: tegra: Check for match_data being NULL
  pwm: tegra: Make use of dev_err_probe()
  pwm: tegra: Use devm function for pm_runtime_enable()
  pwm: tegra: Simplify using
    devm_reset_control_get_exclusive_deasserted()
  pwm: tegra: Simplify using devm_pwmchip_add()
  pwm: tegra: Implement .get_state()

 drivers/pwm/pwm-tegra.c | 123 ++++++++++++++++++++++++++++------------
 1 file changed, 86 insertions(+), 37 deletions(-)


base-commit: 82dcd68f7246eeacbc2ef614b82e284a50b2b8bb
-- 
2.55.0.11.g153666a7d9bb


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v1 1/6] pwm: tegra: Check for match_data being NULL
  2026-07-14 12:02 [PATCH v1 0/6] pwm: tegra: Cleanups and .get_state() Uwe Kleine-König
@ 2026-07-14 12:02 ` Uwe Kleine-König
  2026-07-14 12:02 ` [PATCH v1 2/6] pwm: tegra: Make use of dev_err_probe() Uwe Kleine-König
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2026-07-14 12:02 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Mikko Perttunen; +Cc: linux-pwm, linux-tegra

It's unlikely but not impossible that of_device_get_match_data() returns
NULL. Handle this case instead of triggering a NULL pointer exception.

Signed-off-by: Uwe Kleine-König <ukleinek@kernel.org>
---
 drivers/pwm/pwm-tegra.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index 5cdbe120ba2d..53743f83869a 100644
--- a/drivers/pwm/pwm-tegra.c
+++ b/drivers/pwm/pwm-tegra.c
@@ -322,6 +322,13 @@ static int tegra_pwm_probe(struct platform_device *pdev)
 	int ret;
 
 	soc = of_device_get_match_data(&pdev->dev);
+	if (!soc)
+		/*
+		 * This can only happen if pdev was matched via pdev->name
+		 * (which should not happen today) or in combination with a
+		 * driver override.
+		 */
+		return dev_err_probe(dev, -ENODEV, "Unsupported device\n");
 
 	chip = devm_pwmchip_alloc(&pdev->dev, soc->num_channels, sizeof(*pc));
 	if (IS_ERR(chip))
-- 
2.55.0.11.g153666a7d9bb


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v1 2/6] pwm: tegra: Make use of dev_err_probe()
  2026-07-14 12:02 [PATCH v1 0/6] pwm: tegra: Cleanups and .get_state() Uwe Kleine-König
  2026-07-14 12:02 ` [PATCH v1 1/6] pwm: tegra: Check for match_data being NULL Uwe Kleine-König
@ 2026-07-14 12:02 ` Uwe Kleine-König
  2026-07-14 12:02 ` [PATCH v1 3/6] pwm: tegra: Use devm function for pm_runtime_enable() Uwe Kleine-König
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2026-07-14 12:02 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Mikko Perttunen, Philipp Zabel
  Cc: linux-pwm, linux-tegra

Usage of dev_err_probe() is more compact than dev_err()'s, emits the
error code and handles -ENOMEM and -EPROBE_DEFER properly. Benefit from
these improvements.

Also add a few messages in error paths that lacked an output before.

Signed-off-by: Uwe Kleine-König <ukleinek@kernel.org>
---
 drivers/pwm/pwm-tegra.c | 44 ++++++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index 53743f83869a..dba9a05675e3 100644
--- a/drivers/pwm/pwm-tegra.c
+++ b/drivers/pwm/pwm-tegra.c
@@ -316,6 +316,7 @@ static const struct pwm_ops tegra_pwm_ops = {
 
 static int tegra_pwm_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct pwm_chip *chip;
 	struct tegra_pwm_chip *pc;
 	const struct tegra_pwm_soc *soc;
@@ -330,7 +331,7 @@ static int tegra_pwm_probe(struct platform_device *pdev)
 		 */
 		return dev_err_probe(dev, -ENODEV, "Unsupported device\n");
 
-	chip = devm_pwmchip_alloc(&pdev->dev, soc->num_channels, sizeof(*pc));
+	chip = devm_pwmchip_alloc(dev, soc->num_channels, sizeof(*pc));
 	if (IS_ERR(chip))
 		return PTR_ERR(chip);
 	pc = to_tegra_pwm_chip(chip);
@@ -339,27 +340,36 @@ static int tegra_pwm_probe(struct platform_device *pdev)
 
 	pc->regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(pc->regs))
+		/*
+		 * devm_platform_ioremap_resource() already emits an error
+		 * message with CONFIG_HAS_IOMEM, so don't emit another message
+		 * here.
+		 */
 		return PTR_ERR(pc->regs);
 
 	platform_set_drvdata(pdev, chip);
 
-	pc->clk = devm_clk_get(&pdev->dev, NULL);
+	pc->clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(pc->clk))
-		return PTR_ERR(pc->clk);
+		return dev_err_probe(dev, PTR_ERR(pc->clk), "Failed to get clock\n");
 
-	ret = devm_tegra_core_dev_init_opp_table_common(&pdev->dev);
+	ret = devm_tegra_core_dev_init_opp_table_common(dev);
 	if (ret)
+		/*
+		 * devm_tegra_core_dev_init_opp_table_common() emits an error
+		 * message most of the time, so don't add another.
+		 */
 		return ret;
 
-	pm_runtime_enable(&pdev->dev);
-	ret = pm_runtime_resume_and_get(&pdev->dev);
+	pm_runtime_enable(dev);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret)
-		return ret;
+		return dev_err_probe(dev, ret, "Failed to runtime resume device\n");
 
 	/* Set maximum frequency of the IP */
-	ret = dev_pm_opp_set_rate(&pdev->dev, ULONG_MAX);
+	ret = dev_pm_opp_set_rate(dev, ULONG_MAX);
 	if (ret < 0) {
-		dev_err(&pdev->dev, "Failed to set max frequency: %d\n", ret);
+		dev_err_probe(dev, ret, "Failed to set max frequency\n");
 		goto put_pm;
 	}
 
@@ -370,8 +380,7 @@ static int tegra_pwm_probe(struct platform_device *pdev)
 	 */
 	pc->clk_rate = clk_get_rate(pc->clk);
 	if (pc->clk_rate < TEGRA_PWM_DEPTH) {
-		dev_err(&pdev->dev, "clock maximum frequency out of range\n");
-		ret = -ERANGE;
+		ret = dev_err_probe(dev, -ERANGE, "Clock maximum frequency out of range\n");
 		goto put_pm;
 	}
 
@@ -379,10 +388,9 @@ static int tegra_pwm_probe(struct platform_device *pdev)
 	pc->min_period_ns =
 	    (NSEC_PER_SEC / (pc->clk_rate / TEGRA_PWM_DEPTH)) + 1;
 
-	pc->rst = devm_reset_control_get_exclusive(&pdev->dev, "pwm");
+	pc->rst = devm_reset_control_get_exclusive(dev, "pwm");
 	if (IS_ERR(pc->rst)) {
-		ret = PTR_ERR(pc->rst);
-		dev_err(&pdev->dev, "Reset control is not found: %d\n", ret);
+		ret = dev_err_probe(dev, PTR_ERR(pc->rst), "Failed to get reset control\n");
 		goto put_pm;
 	}
 
@@ -392,17 +400,17 @@ static int tegra_pwm_probe(struct platform_device *pdev)
 
 	ret = pwmchip_add(chip);
 	if (ret < 0) {
-		dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret);
+		dev_err_probe(dev, ret, "Adding pwmchip failed\n");
 		reset_control_assert(pc->rst);
 		goto put_pm;
 	}
 
-	pm_runtime_put(&pdev->dev);
+	pm_runtime_put(dev);
 
 	return 0;
 put_pm:
-	pm_runtime_put_sync_suspend(&pdev->dev);
-	pm_runtime_force_suspend(&pdev->dev);
+	pm_runtime_put_sync_suspend(dev);
+	pm_runtime_force_suspend(dev);
 	return ret;
 }
 
-- 
2.55.0.11.g153666a7d9bb


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v1 3/6] pwm: tegra: Use devm function for pm_runtime_enable()
  2026-07-14 12:02 [PATCH v1 0/6] pwm: tegra: Cleanups and .get_state() Uwe Kleine-König
  2026-07-14 12:02 ` [PATCH v1 1/6] pwm: tegra: Check for match_data being NULL Uwe Kleine-König
  2026-07-14 12:02 ` [PATCH v1 2/6] pwm: tegra: Make use of dev_err_probe() Uwe Kleine-König
@ 2026-07-14 12:02 ` Uwe Kleine-König
  2026-07-14 12:02 ` [PATCH v1 4/6] pwm: tegra: Simplify using devm_reset_control_get_exclusive_deasserted() Uwe Kleine-König
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2026-07-14 12:02 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Mikko Perttunen; +Cc: linux-pwm, linux-tegra

This simplifies the error paths as pwm_runtime_disable() is called
automatatically by the driver core.

Note that pwm_runtime_disable() is the right function to undo
pm_runtime_enable(); pm_runtime_force_suspend() "should only be used
during system-wide PM transitions to sleep states".

Signed-off-by: Uwe Kleine-König <ukleinek@kernel.org>
---
 drivers/pwm/pwm-tegra.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index dba9a05675e3..e99e1c5b18c3 100644
--- a/drivers/pwm/pwm-tegra.c
+++ b/drivers/pwm/pwm-tegra.c
@@ -361,7 +361,10 @@ static int tegra_pwm_probe(struct platform_device *pdev)
 		 */
 		return ret;
 
-	pm_runtime_enable(dev);
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable runtime PM\n");
+
 	ret = pm_runtime_resume_and_get(dev);
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to runtime resume device\n");
@@ -410,7 +413,6 @@ static int tegra_pwm_probe(struct platform_device *pdev)
 	return 0;
 put_pm:
 	pm_runtime_put_sync_suspend(dev);
-	pm_runtime_force_suspend(dev);
 	return ret;
 }
 
@@ -422,8 +424,6 @@ static void tegra_pwm_remove(struct platform_device *pdev)
 	pwmchip_remove(chip);
 
 	reset_control_assert(pc->rst);
-
-	pm_runtime_force_suspend(&pdev->dev);
 }
 
 static int __maybe_unused tegra_pwm_runtime_suspend(struct device *dev)
-- 
2.55.0.11.g153666a7d9bb


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v1 4/6] pwm: tegra: Simplify using devm_reset_control_get_exclusive_deasserted()
  2026-07-14 12:02 [PATCH v1 0/6] pwm: tegra: Cleanups and .get_state() Uwe Kleine-König
                   ` (2 preceding siblings ...)
  2026-07-14 12:02 ` [PATCH v1 3/6] pwm: tegra: Use devm function for pm_runtime_enable() Uwe Kleine-König
@ 2026-07-14 12:02 ` Uwe Kleine-König
  2026-07-14 12:12   ` Philipp Zabel
  2026-07-14 12:02 ` [PATCH v1 5/6] pwm: tegra: Simplify using devm_pwmchip_add() Uwe Kleine-König
  2026-07-14 12:02 ` [PATCH v1 6/6] pwm: tegra: Implement .get_state() Uwe Kleine-König
  5 siblings, 1 reply; 9+ messages in thread
From: Uwe Kleine-König @ 2026-07-14 12:02 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Mikko Perttunen, Philipp Zabel
  Cc: linux-pwm, linux-tegra

This function ensures the reset is already deasserted at probe time and
asserted at unbind. So the remove function and the error paths in the
probe function can be simplified accordingly.

Signed-off-by: Uwe Kleine-König <ukleinek@kernel.org>
---
 drivers/pwm/pwm-tegra.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index e99e1c5b18c3..d7f4baa4cd9b 100644
--- a/drivers/pwm/pwm-tegra.c
+++ b/drivers/pwm/pwm-tegra.c
@@ -108,7 +108,6 @@ struct tegra_pwm_soc {
 
 struct tegra_pwm_chip {
 	struct clk *clk;
-	struct reset_control*rst;
 
 	unsigned long clk_rate;
 	unsigned long min_period_ns;
@@ -319,6 +318,7 @@ static int tegra_pwm_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct pwm_chip *chip;
 	struct tegra_pwm_chip *pc;
+	struct reset_control*rst;
 	const struct tegra_pwm_soc *soc;
 	int ret;
 
@@ -391,20 +391,17 @@ static int tegra_pwm_probe(struct platform_device *pdev)
 	pc->min_period_ns =
 	    (NSEC_PER_SEC / (pc->clk_rate / TEGRA_PWM_DEPTH)) + 1;
 
-	pc->rst = devm_reset_control_get_exclusive(dev, "pwm");
-	if (IS_ERR(pc->rst)) {
-		ret = dev_err_probe(dev, PTR_ERR(pc->rst), "Failed to get reset control\n");
+	rst = devm_reset_control_get_exclusive_deasserted(dev, "pwm");
+	if (IS_ERR(rst)) {
+		ret = dev_err_probe(dev, PTR_ERR(rst), "Failed to get reset control\n");
 		goto put_pm;
 	}
 
-	reset_control_deassert(pc->rst);
-
 	chip->ops = &tegra_pwm_ops;
 
 	ret = pwmchip_add(chip);
 	if (ret < 0) {
 		dev_err_probe(dev, ret, "Adding pwmchip failed\n");
-		reset_control_assert(pc->rst);
 		goto put_pm;
 	}
 
@@ -419,11 +416,8 @@ static int tegra_pwm_probe(struct platform_device *pdev)
 static void tegra_pwm_remove(struct platform_device *pdev)
 {
 	struct pwm_chip *chip = platform_get_drvdata(pdev);
-	struct tegra_pwm_chip *pc = to_tegra_pwm_chip(chip);
 
 	pwmchip_remove(chip);
-
-	reset_control_assert(pc->rst);
 }
 
 static int __maybe_unused tegra_pwm_runtime_suspend(struct device *dev)
-- 
2.55.0.11.g153666a7d9bb


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v1 5/6] pwm: tegra: Simplify using devm_pwmchip_add()
  2026-07-14 12:02 [PATCH v1 0/6] pwm: tegra: Cleanups and .get_state() Uwe Kleine-König
                   ` (3 preceding siblings ...)
  2026-07-14 12:02 ` [PATCH v1 4/6] pwm: tegra: Simplify using devm_reset_control_get_exclusive_deasserted() Uwe Kleine-König
@ 2026-07-14 12:02 ` Uwe Kleine-König
  2026-07-14 12:02 ` [PATCH v1 6/6] pwm: tegra: Implement .get_state() Uwe Kleine-König
  5 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2026-07-14 12:02 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Mikko Perttunen; +Cc: linux-pwm, linux-tegra

devm_pwmchip_add() makes the device core cope for calling
pwmchip_remove() at unbind time. This way the remove callback becomes
empty and can be dropped.

Signed-off-by: Uwe Kleine-König <ukleinek@kernel.org>
---
 drivers/pwm/pwm-tegra.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index d7f4baa4cd9b..8e5e7e37f4ff 100644
--- a/drivers/pwm/pwm-tegra.c
+++ b/drivers/pwm/pwm-tegra.c
@@ -399,7 +399,7 @@ static int tegra_pwm_probe(struct platform_device *pdev)
 
 	chip->ops = &tegra_pwm_ops;
 
-	ret = pwmchip_add(chip);
+	ret = devm_pwmchip_add(dev, chip);
 	if (ret < 0) {
 		dev_err_probe(dev, ret, "Adding pwmchip failed\n");
 		goto put_pm;
@@ -413,13 +413,6 @@ static int tegra_pwm_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static void tegra_pwm_remove(struct platform_device *pdev)
-{
-	struct pwm_chip *chip = platform_get_drvdata(pdev);
-
-	pwmchip_remove(chip);
-}
-
 static int __maybe_unused tegra_pwm_runtime_suspend(struct device *dev)
 {
 	struct pwm_chip *chip = dev_get_drvdata(dev);
@@ -497,7 +490,6 @@ static struct platform_driver tegra_pwm_driver = {
 		.pm = &tegra_pwm_pm_ops,
 	},
 	.probe = tegra_pwm_probe,
-	.remove = tegra_pwm_remove,
 };
 
 module_platform_driver(tegra_pwm_driver);
-- 
2.55.0.11.g153666a7d9bb


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v1 6/6] pwm: tegra: Implement .get_state()
  2026-07-14 12:02 [PATCH v1 0/6] pwm: tegra: Cleanups and .get_state() Uwe Kleine-König
                   ` (4 preceding siblings ...)
  2026-07-14 12:02 ` [PATCH v1 5/6] pwm: tegra: Simplify using devm_pwmchip_add() Uwe Kleine-König
@ 2026-07-14 12:02 ` Uwe Kleine-König
  5 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2026-07-14 12:02 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Mikko Perttunen; +Cc: linux-pwm, linux-tegra

The registers of the PWM IP are readable. Use that to implement the
.get_state() callback.

Signed-off-by: Uwe Kleine-König <ukleinek@kernel.org>
---
 drivers/pwm/pwm-tegra.c | 48 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index 8e5e7e37f4ff..79bfc7589db8 100644
--- a/drivers/pwm/pwm-tegra.c
+++ b/drivers/pwm/pwm-tegra.c
@@ -309,8 +309,56 @@ static int tegra_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	return err;
 }
 
+static int tegra_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+			       struct pwm_state *state)
+{
+	struct tegra_pwm_chip *pc = to_tegra_pwm_chip(chip);
+	int rc;
+	u32 val;
+
+	rc = pm_runtime_resume_and_get(pwmchip_parent(chip));
+	if (rc)
+		return rc;
+
+	val = tegra_pwm_readl(pwm, pc->soc->enable_reg);
+	if (val & TEGRA_PWM_ENABLE) {
+		u32 scale, pwm0;
+
+		if (pc->soc->enable_reg != TEGRA_PWM_CSR_0)
+			val = tegra_pwm_readl(pwm, TEGRA_PWM_CSR_0);
+
+		scale = (val >> TEGRA_PWM_SCALE_SHIFT) & (((1 << pc->soc->scale_width) - 1));
+		pwm0 = (val >> TEGRA_PWM_DUTY_SHIFT) & (2 * TEGRA_PWM_DEPTH - 1);
+
+		if (pwm0 > TEGRA_PWM_DEPTH)
+			pwm0 = TEGRA_PWM_DEPTH;
+
+		/*
+		 * scale + 1 is at most 1 << 17, TEGRA_PWM_DEPTH is 256, so the
+		 * multiplication for .period doesn't overflow a u64. With
+		 * pwm0 ≤ TEGRA_PWM_DEPTH, .duty_cycle is also fine.
+		 */
+		*state = (struct pwm_state){
+			.period = DIV64_U64_ROUND_UP((u64)(scale + 1) * TEGRA_PWM_DEPTH * NSEC_PER_SEC, pc->clk_rate),
+			.duty_cycle = DIV64_U64_ROUND_UP((u64)(scale + 1) * pwm0 * NSEC_PER_SEC, pc->clk_rate),
+			.polarity = PWM_POLARITY_NORMAL,
+			.enabled = true,
+		};
+
+	} else {
+		*state = (struct pwm_state){
+			.enabled = false,
+		};
+	}
+
+	pm_runtime_put(pwmchip_parent(chip));
+
+	return 0;
+}
+
 static const struct pwm_ops tegra_pwm_ops = {
 	.apply = tegra_pwm_apply,
+	.get_state = tegra_pwm_get_state,
 };
 
 static int tegra_pwm_probe(struct platform_device *pdev)
-- 
2.55.0.11.g153666a7d9bb


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 4/6] pwm: tegra: Simplify using devm_reset_control_get_exclusive_deasserted()
  2026-07-14 12:02 ` [PATCH v1 4/6] pwm: tegra: Simplify using devm_reset_control_get_exclusive_deasserted() Uwe Kleine-König
@ 2026-07-14 12:12   ` Philipp Zabel
  2026-07-14 14:07     ` Uwe Kleine-König
  0 siblings, 1 reply; 9+ messages in thread
From: Philipp Zabel @ 2026-07-14 12:12 UTC (permalink / raw)
  To: Uwe Kleine-König, Thierry Reding, Jonathan Hunter,
	Mikko Perttunen
  Cc: linux-pwm, linux-tegra

On Di, 2026-07-14 at 14:02 +0200, Uwe Kleine-König wrote:
> This function ensures the reset is already deasserted at probe time and
> asserted at unbind. So the remove function and the error paths in the
> probe function can be simplified accordingly.
> 
> Signed-off-by: Uwe Kleine-König <ukleinek@kernel.org>
> ---
>  drivers/pwm/pwm-tegra.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> index e99e1c5b18c3..d7f4baa4cd9b 100644
> --- a/drivers/pwm/pwm-tegra.c
> +++ b/drivers/pwm/pwm-tegra.c
> @@ -108,7 +108,6 @@ struct tegra_pwm_soc {
>  
>  struct tegra_pwm_chip {
>  	struct clk *clk;
> -	struct reset_control*rst;
>  
>  	unsigned long clk_rate;
>  	unsigned long min_period_ns;
> @@ -319,6 +318,7 @@ static int tegra_pwm_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct pwm_chip *chip;
>  	struct tegra_pwm_chip *pc;
> +	struct reset_control*rst;

You could use this opportunity to add a space between reset_control and
*rst.

>  	const struct tegra_pwm_soc *soc;
>  	int ret;
>  
> @@ -391,20 +391,17 @@ static int tegra_pwm_probe(struct platform_device *pdev)
>  	pc->min_period_ns =
>  	    (NSEC_PER_SEC / (pc->clk_rate / TEGRA_PWM_DEPTH)) + 1;
>  
> -	pc->rst = devm_reset_control_get_exclusive(dev, "pwm");
> -	if (IS_ERR(pc->rst)) {
> -		ret = dev_err_probe(dev, PTR_ERR(pc->rst), "Failed to get reset control\n");
> +	rst = devm_reset_control_get_exclusive_deasserted(dev, "pwm");
> +	if (IS_ERR(rst)) {
> +		ret = dev_err_probe(dev, PTR_ERR(rst), "Failed to get reset control\n");
>  		goto put_pm;
>  	}
>  
> -	reset_control_deassert(pc->rst);
> -
>  	chip->ops = &tegra_pwm_ops;
>  
>  	ret = pwmchip_add(chip);
>  	if (ret < 0) {
>  		dev_err_probe(dev, ret, "Adding pwmchip failed\n");
> -		reset_control_assert(pc->rst);

With this change, pm_runtime_put_sync_suspend() and
pm_runtime_force_suspend() are called before the reset control is
asserted again in the error case. Is this safe?

regards
Philipp

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 4/6] pwm: tegra: Simplify using devm_reset_control_get_exclusive_deasserted()
  2026-07-14 12:12   ` Philipp Zabel
@ 2026-07-14 14:07     ` Uwe Kleine-König
  0 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2026-07-14 14:07 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Thierry Reding, Jonathan Hunter, Mikko Perttunen, linux-pwm,
	linux-tegra

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

On Tue, Jul 14, 2026 at 02:12:38PM +0200, Philipp Zabel wrote:
> On Di, 2026-07-14 at 14:02 +0200, Uwe Kleine-König wrote:
> > This function ensures the reset is already deasserted at probe time and
> > asserted at unbind. So the remove function and the error paths in the
> > probe function can be simplified accordingly.
> > 
> > Signed-off-by: Uwe Kleine-König <ukleinek@kernel.org>
> > ---
> >  drivers/pwm/pwm-tegra.c | 14 ++++----------
> >  1 file changed, 4 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> > index e99e1c5b18c3..d7f4baa4cd9b 100644
> > --- a/drivers/pwm/pwm-tegra.c
> > +++ b/drivers/pwm/pwm-tegra.c
> > @@ -108,7 +108,6 @@ struct tegra_pwm_soc {
> >  
> >  struct tegra_pwm_chip {
> >  	struct clk *clk;
> > -	struct reset_control*rst;
> >  
> >  	unsigned long clk_rate;
> >  	unsigned long min_period_ns;
> > @@ -319,6 +318,7 @@ static int tegra_pwm_probe(struct platform_device *pdev)
> >  	struct device *dev = &pdev->dev;
> >  	struct pwm_chip *chip;
> >  	struct tegra_pwm_chip *pc;
> > +	struct reset_control*rst;
> 
> You could use this opportunity to add a space between reset_control and
> *rst.

Oh indeed. I thought I called checkpatch, but there are two more
warnings in this series that I'm not aware of, so it seems I didn't
check before sending :-o

> >  	const struct tegra_pwm_soc *soc;
> >  	int ret;
> >  
> > @@ -391,20 +391,17 @@ static int tegra_pwm_probe(struct platform_device *pdev)
> >  	pc->min_period_ns =
> >  	    (NSEC_PER_SEC / (pc->clk_rate / TEGRA_PWM_DEPTH)) + 1;
> >  
> > -	pc->rst = devm_reset_control_get_exclusive(dev, "pwm");
> > -	if (IS_ERR(pc->rst)) {
> > -		ret = dev_err_probe(dev, PTR_ERR(pc->rst), "Failed to get reset control\n");
> > +	rst = devm_reset_control_get_exclusive_deasserted(dev, "pwm");
> > +	if (IS_ERR(rst)) {
> > +		ret = dev_err_probe(dev, PTR_ERR(rst), "Failed to get reset control\n");
> >  		goto put_pm;
> >  	}
> >  
> > -	reset_control_deassert(pc->rst);
> > -
> >  	chip->ops = &tegra_pwm_ops;
> >  
> >  	ret = pwmchip_add(chip);
> >  	if (ret < 0) {
> >  		dev_err_probe(dev, ret, "Adding pwmchip failed\n");
> > -		reset_control_assert(pc->rst);
> 
> With this change, pm_runtime_put_sync_suspend() and
> pm_runtime_force_suspend() are called before the reset control is
> asserted again in the error case. Is this safe?

Ah, this is true for pm_runtime_put_sync_suspend(). Too bad this isn't
easily fixable :-\ I would expect this not to be a problem, but given
that I don't have the hardware, I guess being conservative here is
needed.

Thanks for catching this.

Uwe

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-07-14 14:07 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-14 12:02 [PATCH v1 0/6] pwm: tegra: Cleanups and .get_state() Uwe Kleine-König
2026-07-14 12:02 ` [PATCH v1 1/6] pwm: tegra: Check for match_data being NULL Uwe Kleine-König
2026-07-14 12:02 ` [PATCH v1 2/6] pwm: tegra: Make use of dev_err_probe() Uwe Kleine-König
2026-07-14 12:02 ` [PATCH v1 3/6] pwm: tegra: Use devm function for pm_runtime_enable() Uwe Kleine-König
2026-07-14 12:02 ` [PATCH v1 4/6] pwm: tegra: Simplify using devm_reset_control_get_exclusive_deasserted() Uwe Kleine-König
2026-07-14 12:12   ` Philipp Zabel
2026-07-14 14:07     ` Uwe Kleine-König
2026-07-14 12:02 ` [PATCH v1 5/6] pwm: tegra: Simplify using devm_pwmchip_add() Uwe Kleine-König
2026-07-14 12:02 ` [PATCH v1 6/6] pwm: tegra: Implement .get_state() Uwe Kleine-König

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox