[PATCH 1/7] pwm: sifive: Simplify offset calculation for PWMCMP registers

[PATCH 1/7] pwm: sifive: Simplify offset calculation for PWMCMP registers

[Uwe Kleine-König]

Instead of explicitly using PWM_SIFIVE_PWMCMP0 + pwm->hwpwm *
PWM_SIFIVE_SIZE_PWMCMP for each access to one of the PWMCMP registers,
introduce a macro that takes the hwpwm id as parameter.

For the register definition using a plain 4 instead of the cpp constant
PWM_SIFIVE_SIZE_PWMCMP is easier to read, so define the offset macro
without the constant. The latter can then be dropped as there are no
users left.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-sifive.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c
index e6d05a329002..b7fc33b08d82 100644
--- a/drivers/pwm/pwm-sifive.c
+++ b/drivers/pwm/pwm-sifive.c
@@ -23,7 +23,7 @@
 #define PWM_SIFIVE_PWMCFG		0x0
 #define PWM_SIFIVE_PWMCOUNT		0x8
 #define PWM_SIFIVE_PWMS			0x10
-#define PWM_SIFIVE_PWMCMP0		0x20
+#define PWM_SIFIVE_PWMCMP(i)		(0x20 + 4 * (i))
 
 /* PWMCFG fields */
 #define PWM_SIFIVE_PWMCFG_SCALE		GENMASK(3, 0)
@@ -36,8 +36,6 @@
 #define PWM_SIFIVE_PWMCFG_GANG		BIT(24)
 #define PWM_SIFIVE_PWMCFG_IP		BIT(28)
 
-/* PWM_SIFIVE_SIZE_PWMCMP is used to calculate offset for pwmcmpX registers */
-#define PWM_SIFIVE_SIZE_PWMCMP		4
 #define PWM_SIFIVE_CMPWIDTH		16
 #define PWM_SIFIVE_DEFAULT_PERIOD	10000000
 
@@ -112,8 +110,7 @@ static void pwm_sifive_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
 	struct pwm_sifive_ddata *ddata = pwm_sifive_chip_to_ddata(chip);
 	u32 duty, val;
 
-	duty = readl(ddata->regs + PWM_SIFIVE_PWMCMP0 +
-		     pwm->hwpwm * PWM_SIFIVE_SIZE_PWMCMP);
+	duty = readl(ddata->regs + PWM_SIFIVE_PWMCMP(pwm->hwpwm));
 
 	state->enabled = duty > 0;
 
@@ -193,8 +190,7 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 		pwm_sifive_update_clock(ddata, clk_get_rate(ddata->clk));
 	}
 
-	writel(frac, ddata->regs + PWM_SIFIVE_PWMCMP0 +
-	       pwm->hwpwm * PWM_SIFIVE_SIZE_PWMCMP);
+	writel(frac, ddata->regs + PWM_SIFIVE_PWMCMP(pwm->hwpwm));
 
 	if (state->enabled != enabled)
 		pwm_sifive_enable(chip, state->enabled);

base-commit: f2906aa863381afb0015a9eb7fefad885d4e5a56
-- 
2.36.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 2/7] pwm: sifive: Fold pwm_sifive_enable() into its only caller

[Uwe Kleine-König]

There is only a single caller of pwm_sifive_enable() which only enables
or disables the clk. Put this implementation directly into
pwm_sifive_apply() which allows further simplification in the next
change.

There is no change in behaviour.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-sifive.c | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c
index b7fc33b08d82..91cf90bd4083 100644
--- a/drivers/pwm/pwm-sifive.c
+++ b/drivers/pwm/pwm-sifive.c
@@ -124,24 +124,6 @@ static void pwm_sifive_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
 	state->polarity = PWM_POLARITY_INVERSED;
 }
 
-static int pwm_sifive_enable(struct pwm_chip *chip, bool enable)
-{
-	struct pwm_sifive_ddata *ddata = pwm_sifive_chip_to_ddata(chip);
-	int ret;
-
-	if (enable) {
-		ret = clk_enable(ddata->clk);
-		if (ret) {
-			dev_err(ddata->chip.dev, "Enable clk failed\n");
-			return ret;
-		}
-	} else {
-		clk_disable(ddata->clk);
-	}
-
-	return 0;
-}
-
 static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 			    const struct pwm_state *state)
 {
@@ -192,8 +174,14 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 
 	writel(frac, ddata->regs + PWM_SIFIVE_PWMCMP(pwm->hwpwm));
 
-	if (state->enabled != enabled)
-		pwm_sifive_enable(chip, state->enabled);
+	if (state->enabled != enabled) {
+		if (state->enabled) {
+			if (clk_enable(ddata->clk))
+				dev_err(ddata->chip.dev, "Enable clk failed\n");
+		} else {
+			clk_disable(ddata->clk);
+		}
+	}
 
 exit:
 	clk_disable(ddata->clk);
-- 
2.36.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 3/7] pwm: sifive: Reduce time the controller lock is held

[Uwe Kleine-König]

The lock is only to serialize access and update to user_count and
approx_period between different PWMs served by the same pwm_chip.
So the lock needs only to be taken during the check if the (chip global)
period can and/or needs to be changed.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-sifive.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c
index 91cf90bd4083..6017e311a879 100644
--- a/drivers/pwm/pwm-sifive.c
+++ b/drivers/pwm/pwm-sifive.c
@@ -41,7 +41,7 @@
 
 struct pwm_sifive_ddata {
 	struct pwm_chip	chip;
-	struct mutex lock; /* lock to protect user_count */
+	struct mutex lock; /* lock to protect user_count and approx_period */
 	struct notifier_block notifier;
 	struct clk *clk;
 	void __iomem *regs;
@@ -76,6 +76,7 @@ static void pwm_sifive_free(struct pwm_chip *chip, struct pwm_device *pwm)
 	mutex_unlock(&ddata->lock);
 }
 
+/* Called holding ddata->lock */
 static void pwm_sifive_update_clock(struct pwm_sifive_ddata *ddata,
 				    unsigned long rate)
 {
@@ -144,7 +145,6 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 		return ret;
 	}
 
-	mutex_lock(&ddata->lock);
 	cur_state = pwm->state;
 	enabled = cur_state.enabled;
 
@@ -163,14 +163,17 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	/* The hardware cannot generate a 100% duty cycle */
 	frac = min(frac, (1U << PWM_SIFIVE_CMPWIDTH) - 1);
 
+	mutex_lock(&ddata->lock);
 	if (state->period != ddata->approx_period) {
 		if (ddata->user_count != 1) {
+			mutex_unlock(&ddata->lock);
 			ret = -EBUSY;
 			goto exit;
 		}
 		ddata->approx_period = state->period;
 		pwm_sifive_update_clock(ddata, clk_get_rate(ddata->clk));
 	}
+	mutex_unlock(&ddata->lock);
 
 	writel(frac, ddata->regs + PWM_SIFIVE_PWMCMP(pwm->hwpwm));
 
@@ -185,7 +188,6 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 
 exit:
 	clk_disable(ddata->clk);
-	mutex_unlock(&ddata->lock);
 	return ret;
 }
 
-- 
2.36.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 4/7] pwm: sifive: Enable clk only after period check in .apply()

[Uwe Kleine-König]

For the period check and the initial calculations of register values there
is no hardware access needed. So delay enabling the clk a bit to simplify
the code flow a bit.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-sifive.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c
index 6017e311a879..d833536b5e7a 100644
--- a/drivers/pwm/pwm-sifive.c
+++ b/drivers/pwm/pwm-sifive.c
@@ -139,12 +139,6 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	if (state->polarity != PWM_POLARITY_INVERSED)
 		return -EINVAL;
 
-	ret = clk_enable(ddata->clk);
-	if (ret) {
-		dev_err(ddata->chip.dev, "Enable clk failed\n");
-		return ret;
-	}
-
 	cur_state = pwm->state;
 	enabled = cur_state.enabled;
 
@@ -167,14 +161,19 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	if (state->period != ddata->approx_period) {
 		if (ddata->user_count != 1) {
 			mutex_unlock(&ddata->lock);
-			ret = -EBUSY;
-			goto exit;
+			return -EBUSY;
 		}
 		ddata->approx_period = state->period;
 		pwm_sifive_update_clock(ddata, clk_get_rate(ddata->clk));
 	}
 	mutex_unlock(&ddata->lock);
 
+	ret = clk_enable(ddata->clk);
+	if (ret) {
+		dev_err(ddata->chip.dev, "Enable clk failed\n");
+		return ret;
+	}
+
 	writel(frac, ddata->regs + PWM_SIFIVE_PWMCMP(pwm->hwpwm));
 
 	if (state->enabled != enabled) {
@@ -186,9 +185,8 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 		}
 	}
 
-exit:
 	clk_disable(ddata->clk);
-	return ret;
+	return 0;
 }
 
 static const struct pwm_ops pwm_sifive_ops = {
-- 
2.36.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 5/7] pwm: sifive: Simplify clk handling

[Uwe Kleine-König]

The clk is necessary for both register access and (enabled) operation of
the PWM. Instead of

	clk_enable()
	update_hw()
	if pwm_got_enabled():
		clk_enable()
	elif pwm_got_disabled():
		clk_disable()
	clk_disable()

which is some cases only calls clk_enable() to immediately afterwards
call clk_disable again, do:

	if (!prev_state.enabled)
		clk_enable()

	# clk enabled exactly once

	update_hw()

	if (!next_state.enabled)
		clk_disable()

which is much easier.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-sifive.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c
index d833536b5e7a..34d23d56fa25 100644
--- a/drivers/pwm/pwm-sifive.c
+++ b/drivers/pwm/pwm-sifive.c
@@ -168,24 +168,24 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	}
 	mutex_unlock(&ddata->lock);
 
-	ret = clk_enable(ddata->clk);
-	if (ret) {
-		dev_err(ddata->chip.dev, "Enable clk failed\n");
-		return ret;
+	/*
+	 * If the PWM is enabled the clk is already on. So only enable it
+	 * conditionally to have it on exactly once afterwards independent of
+	 * the PWM state.
+	 */
+	if (!enabled) {
+		ret = clk_enable(ddata->clk);
+		if (ret) {
+			dev_err(ddata->chip.dev, "Enable clk failed\n");
+			return ret;
+		}
 	}
 
 	writel(frac, ddata->regs + PWM_SIFIVE_PWMCMP(pwm->hwpwm));
 
-	if (state->enabled != enabled) {
-		if (state->enabled) {
-			if (clk_enable(ddata->clk))
-				dev_err(ddata->chip.dev, "Enable clk failed\n");
-		} else {
-			clk_disable(ddata->clk);
-		}
-	}
+	if (!state->enabled)
+		clk_disable(ddata->clk);
 
-	clk_disable(ddata->clk);
 	return 0;
 }
 
-- 
2.36.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 6/7] pwm: sifive: Ensure the clk is enabled exactly one per running PWM

[Uwe Kleine-König]

.apply() assumes the clk to be for a given PWM iff the PWM is enabled.
So make sure this is the case when .probe() completes. And in .remove()
disable the according number of times.

This fixes a clk enable/disable imbalance, if some PWMs are already running
at probe time.

Fixes: 9e37a53eb051 (pwm: sifive: Add a driver for SiFive SoC PWM)
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-sifive.c | 46 ++++++++++++++++++++++++++++++++--------
 1 file changed, 37 insertions(+), 9 deletions(-)

diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c
index 34d23d56fa25..da40ade0ebdf 100644
--- a/drivers/pwm/pwm-sifive.c
+++ b/drivers/pwm/pwm-sifive.c
@@ -216,6 +216,8 @@ static int pwm_sifive_probe(struct platform_device *pdev)
 	struct pwm_sifive_ddata *ddata;
 	struct pwm_chip *chip;
 	int ret;
+	u32 val;
+	unsigned int enabled_pwms = 0, enabled_clks = 1;
 
 	ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
 	if (!ddata)
@@ -242,6 +244,33 @@ static int pwm_sifive_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	val = readl(ddata->regs + PWM_SIFIVE_PWMCFG);
+	if (val & PWM_SIFIVE_PWMCFG_EN_ALWAYS) {
+		unsigned int i;
+
+		for (i = 0; i < chip->npwm; ++i) {
+			val = readl(ddata->regs + PWM_SIFIVE_PWMCMP(i));
+			if (val > 0)
+				++enabled_pwms;
+		}
+	}
+
+	/* The clk should be on once for each running PWM. */
+	if (enabled_pwms) {
+		while (enabled_clks < enabled_pwms) {
+			/* This is not expected to fail as the clk is already on */
+			ret = clk_enable(ddata->clk);
+			if (unlikely(ret)) {
+				dev_err_probe(dev, ret, "Failed to enable clk\n");
+				goto disable_clk;
+			}
+			++enabled_clks;
+		}
+	} else {
+		clk_disable(ddata->clk);
+		enabled_clks = 0;
+	}
+
 	/* Watch for changes to underlying clock frequency */
 	ddata->notifier.notifier_call = pwm_sifive_clock_notifier;
 	ret = clk_notifier_register(ddata->clk, &ddata->notifier);
@@ -264,7 +293,11 @@ static int pwm_sifive_probe(struct platform_device *pdev)
 unregister_clk:
 	clk_notifier_unregister(ddata->clk, &ddata->notifier);
 disable_clk:
-	clk_disable_unprepare(ddata->clk);
+	while (enabled_clks) {
+		clk_disable(ddata->clk);
+		--enabled_clks;
+	}
+	clk_unprepare(ddata->clk);
 
 	return ret;
 }
@@ -272,21 +305,16 @@ static int pwm_sifive_probe(struct platform_device *pdev)
 static int pwm_sifive_remove(struct platform_device *dev)
 {
 	struct pwm_sifive_ddata *ddata = platform_get_drvdata(dev);
-	bool is_enabled = false;
 	struct pwm_device *pwm;
 	int ch;
 
 	for (ch = 0; ch < ddata->chip.npwm; ch++) {
 		pwm = &ddata->chip.pwms[ch];
-		if (pwm->state.enabled) {
-			is_enabled = true;
-			break;
-		}
+		if (pwm->state.enabled)
+			clk_disable(ddata->clk);
 	}
-	if (is_enabled)
-		clk_disable(ddata->clk);
 
-	clk_disable_unprepare(ddata->clk);
+	clk_unprepare(ddata->clk);
 	pwmchip_remove(&ddata->chip);
 	clk_notifier_unregister(ddata->clk, &ddata->notifier);
 
-- 
2.36.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 6/7] pwm: sifive: Ensure the clk is enabled exactly one per running PWM

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 474 bytes --]

Hello,

[dropped Yash Shah from Cc, their email address bounces]

While browsing the list of open patches in patchwork, I noticed a typo:

$Subject ~= s/one/once/

@Thierry: Assuming you are otherwise happy with this patch, should I
resend for this typo, or do you fixup while applying it?

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 6/7] pwm: sifive: Ensure the clk is enabled exactly one per running PWM

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 445 bytes --]

On Thu, Jul 21, 2022 at 10:45:22PM +0200, Uwe Kleine-König wrote:
> Hello,
> 
> [dropped Yash Shah from Cc, their email address bounces]
> 
> While browsing the list of open patches in patchwork, I noticed a typo:
> 
> $Subject ~= s/one/once/
> 
> @Thierry: Assuming you are otherwise happy with this patch, should I
> resend for this typo, or do you fixup while applying it?

I've fixed this up while applying, thanks.

Thierry

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

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 7/7] pwm: sifive: Shut down hardware only after pwmchip_remove() completed

[Uwe Kleine-König]

The PWMs are expected to be functional until pwmchip_remove() is called.
So disable the clks only afterwards.

Fixes: 9e37a53eb051 ("pwm: sifive: Add a driver for SiFive SoC PWM")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-sifive.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c
index da40ade0ebdf..2d4fa5e5fdd4 100644
--- a/drivers/pwm/pwm-sifive.c
+++ b/drivers/pwm/pwm-sifive.c
@@ -308,6 +308,9 @@ static int pwm_sifive_remove(struct platform_device *dev)
 	struct pwm_device *pwm;
 	int ch;
 
+	pwmchip_remove(&ddata->chip);
+	clk_notifier_unregister(ddata->clk, &ddata->notifier);
+
 	for (ch = 0; ch < ddata->chip.npwm; ch++) {
 		pwm = &ddata->chip.pwms[ch];
 		if (pwm->state.enabled)
@@ -315,8 +318,6 @@ static int pwm_sifive_remove(struct platform_device *dev)
 	}
 
 	clk_unprepare(ddata->clk);
-	pwmchip_remove(&ddata->chip);
-	clk_notifier_unregister(ddata->clk, &ddata->notifier);
 
 	return 0;
 }
-- 
2.36.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 7/7] pwm: sifive: Shut down hardware only after pwmchip_remove() completed

[Emil Renner Berthing]

On Thu, 21 Jul 2022 at 12:32, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> The PWMs are expected to be functional until pwmchip_remove() is called.
> So disable the clks only afterwards.
>
> Fixes: 9e37a53eb051 ("pwm: sifive: Add a driver for SiFive SoC PWM")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Hi Uwe,

You didn't send a cover letter so unsure which mail to respond to, but
I tested this series with

https://lore.kernel.org/linux-riscv/20220705210143.315151-1-emil.renner.berthing@canonical.com/

..and everything keeps working, so

Tested-by: Emil Renner Berhing <emil.renner.berthing@canonical.com>

/Emil
> ---
>  drivers/pwm/pwm-sifive.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c
> index da40ade0ebdf..2d4fa5e5fdd4 100644
> --- a/drivers/pwm/pwm-sifive.c
> +++ b/drivers/pwm/pwm-sifive.c
> @@ -308,6 +308,9 @@ static int pwm_sifive_remove(struct platform_device *dev)
>         struct pwm_device *pwm;
>         int ch;
>
> +       pwmchip_remove(&ddata->chip);
> +       clk_notifier_unregister(ddata->clk, &ddata->notifier);
> +
>         for (ch = 0; ch < ddata->chip.npwm; ch++) {
>                 pwm = &ddata->chip.pwms[ch];
>                 if (pwm->state.enabled)
> @@ -315,8 +318,6 @@ static int pwm_sifive_remove(struct platform_device *dev)
>         }
>
>         clk_unprepare(ddata->clk);
> -       pwmchip_remove(&ddata->chip);
> -       clk_notifier_unregister(ddata->clk, &ddata->notifier);
>
>         return 0;
>  }
> --
> 2.36.1
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 7/7] pwm: sifive: Shut down hardware only after pwmchip_remove() completed

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1170 bytes --]

On Fri, Jul 22, 2022 at 07:45:32PM +0200, Emil Renner Berthing wrote:
> On Thu, 21 Jul 2022 at 12:32, Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> > The PWMs are expected to be functional until pwmchip_remove() is called.
> > So disable the clks only afterwards.
> >
> > Fixes: 9e37a53eb051 ("pwm: sifive: Add a driver for SiFive SoC PWM")
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> Hi Uwe,
> 
> You didn't send a cover letter so unsure which mail to respond to, but
> I tested this series with
> 
> https://lore.kernel.org/linux-riscv/20220705210143.315151-1-emil.renner.berthing@canonical.com/
> 
> ..and everything keeps working, so
> 
> Tested-by: Emil Renner Berhing <emil.renner.berthing@canonical.com>

This is fine, I've applied the tag to the whole series since you said
that you had tested the whole series. I'm not sure, but I don't think
patchwork automatically adds tags to all patches if they are given to
the cover letter, so in those cases a bit of manual intervention can
be necessary. Perhaps b4 can do this automatically. I should probably
test that at some point.

Thierry

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

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 7/7] pwm: sifive: Shut down hardware only after pwmchip_remove() completed

[Conor.Dooley]

On 28/07/2022 18:12, Thierry Reding wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Fri, Jul 22, 2022 at 07:45:32PM +0200, Emil Renner Berthing wrote:
>> On Thu, 21 Jul 2022 at 12:32, Uwe Kleine-König
>> <u.kleine-koenig@pengutronix.de> wrote:
>>> The PWMs are expected to be functional until pwmchip_remove() is called.
>>> So disable the clks only afterwards.
>>>
>>> Fixes: 9e37a53eb051 ("pwm: sifive: Add a driver for SiFive SoC PWM")
>>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>>
>> Hi Uwe,
>>
>> You didn't send a cover letter so unsure which mail to respond to, but
>> I tested this series with
>>
>> https://lore.kernel.org/linux-riscv/20220705210143.315151-1-emil.renner.berthing@canonical.com/
>>
>> ..and everything keeps working, so
>>
>> Tested-by: Emil Renner Berhing <emil.renner.berthing@canonical.com>
                             ^
Pretty minor I guess, but that should be "Berthing"

> 
> This is fine, I've applied the tag to the whole series since you said
> that you had tested the whole series. I'm not sure, but I don't think
> patchwork automatically adds tags to all patches if they are given to
> the cover letter, so in those cases a bit of manual intervention can
> be necessary. Perhaps b4 can do this automatically. I should probably
> test that at some point.

The -t flag for b4 am will do it automatically for tags applied to the
cove. In fact it prompts you if you don't pass -t and the cover has
tags.

Thanks,
Conor.
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/7] pwm: sifive: Simplify offset calculation for PWMCMP registers

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 738 bytes --]

On Thu, Jul 21, 2022 at 12:31:23PM +0200, Uwe Kleine-König wrote:
> Instead of explicitly using PWM_SIFIVE_PWMCMP0 + pwm->hwpwm *
> PWM_SIFIVE_SIZE_PWMCMP for each access to one of the PWMCMP registers,
> introduce a macro that takes the hwpwm id as parameter.
> 
> For the register definition using a plain 4 instead of the cpp constant
> PWM_SIFIVE_SIZE_PWMCMP is easier to read, so define the offset macro
> without the constant. The latter can then be dropped as there are no
> users left.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/pwm/pwm-sifive.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)

I've applied the whole series now, thanks.

Thierry

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

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv