From: "Rafael V. Volkmer" <rafael.v.volkmer@gmail.com>
To: "Uwe Kleine-König" <ukleinek@kernel.org>
Cc: linux-pwm@vger.kernel.org, linux-kernel@vger.kernel.org,
"Rafael V. Volkmer" <rafael.v.volkmer@gmail.com>
Subject: [PATCH] pwm: tiehrpwm: ensures that state.enabled is synchronized in .probe()
Date: Tue, 4 Feb 2025 15:55:40 -0300 [thread overview]
Message-ID: <20250204185540.52233-1-rafael.v.volkmer@gmail.com> (raw)
Fixes potential desynchronization of state.enabled in the .probe()
method by suggesting proper handling of hardware state initialization.
Adds considerations for implementing .get_hw_state() to check the
current state of the module by checking physical registers.
Signed-off-by: Rafael V. Volkmer <rafael.v.volkmer@gmail.com>
---
drivers/pwm/pwm-tiehrpwm.c | 151 ++++++++++++++++++++++++++++++++++++-
1 file changed, 150 insertions(+), 1 deletion(-)
diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index 0125e73b98df..5de213bc3ef5 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -91,6 +91,20 @@
#define AQCSFRC_CSFA_FRCHIGH BIT(1)
#define AQCSFRC_CSFA_DISSWFRC (BIT(1) | BIT(0))
+#define AQCTLA_CAU_MASK (BIT(5) | BIT(4))
+#define AQCTLA_CAU_SHIFT 4
+#define AQCTLA_CAD_MASK (BIT(9) | BIT(8))
+#define AQCTLA_CAD_SHIFT 8
+
+/* The ePWM hardware encodes compare actions with two bits each:
+ * 00 = Do nothing
+ * 01 = Clear
+ * 10 = Set
+ * 11 = Toggle
+ */
+#define AQ_CLEAR 1
+#define AQ_SET 2
+
#define NUM_PWM_CHANNEL 2 /* EHRPWM channels */
struct ehrpwm_context {
@@ -353,6 +367,118 @@ static int ehrpwm_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
return 0;
}
+static bool ehrpwm_is_enabled(struct pwm_chip *chip)
+{
+ struct ehrpwm_pwm_chip *pc = to_ehrpwm_pwm_chip(chip);
+ bool ret;
+ u16 aqcsfrc_reg;
+ u8 csfa_bits;
+ u16 aqctla_reg;
+
+ if(chip == NULL) {
+ return -EINVAL;
+ }
+
+ aqcsfrc_reg = readw(pc->mmio_base + AQCSFRC);
+ csfa_bits = (u8)(aqcsfrc_reg & AQCSFRC_CSFA_MASK);
+
+ aqctla_reg = readw(pc->mmio_base + AQCTLA);
+
+ ret = (csfa_bits != 0u) ? false :
+ (aqctla_reg == 0u) ? false : true;
+
+ return ret;
+}
+
+static u64 ehrpwm_read_period(struct pwm_chip *chip)
+{
+ struct ehrpwm_pwm_chip *pc = to_ehrpwm_pwm_chip(chip);
+ u64 ret;
+ unsigned long tbclk_rate;
+ u16 tbprd_reg;
+ u64 period_cycles;
+ u64 period_ns;
+
+ if(chip == NULL) {
+ return -EINVAL;
+ }
+
+ tbprd_reg = readw(pc->mmio_base + TBPRD);
+ tbclk_rate = clk_get_rate(pc->tbclk);
+ period_cycles = tbprd_reg + 1u;
+
+ /* period_ns = (period_cycles * 1e9) / tblck_rate */
+ period_ns = DIV_ROUND_UP_ULL(period_cycles * NSEC_PER_SEC, tbclk_rate);
+
+ ret = period_ns;
+ return ret;
+}
+
+static u64 ehrpwm_read_duty_cycle(struct pwm_chip *chip)
+{
+ struct ehrpwm_pwm_chip *pc = to_ehrpwm_pwm_chip(chip);
+ u64 ret;
+ u16 cmpa_reg;
+ u64 duty_cycles;
+ u64 duty_ns;
+ unsigned long tbclk_rate;
+
+ if(chip == NULL) {
+ return -EINVAL;
+ }
+
+ cmpa_reg = readw(pc->mmio_base + CMPA);
+ tbclk_rate = clk_get_rate(pc->tbclk);
+ duty_cycles = cmpa_reg;
+ duty_ns = DIV_ROUND_UP_ULL(duty_cycles * NSEC_PER_SEC, tbclk_rate);
+ ret = duty_ns;
+
+ return ret;
+}
+
+static enum pwm_polarity ehrpwm_read_polarity(struct pwm_chip *chip)
+{
+ struct ehrpwm_pwm_chip *pc = to_ehrpwm_pwm_chip(chip);
+ enum pwm_polarity ret;
+ u16 aqctla_reg;
+ u8 cau_action;
+ u8 cad_action;
+
+ if(chip == NULL) {
+ return -EINVAL;
+ }
+
+ aqctla_reg = readw(pc->mmio_base + AQCTLA);
+ cau_action = (aqctla_reg & AQCTLA_CAU_MASK) >> AQCTLA_CAU_SHIFT;
+ cad_action = (aqctla_reg & AQCTLA_CAD_MASK) >> AQCTLA_CAD_SHIFT;
+
+ if (cau_action == AQ_SET && cad_action == AQ_CLEAR) {
+ ret = PWM_POLARITY_NORMAL;
+ }
+ else if (cau_action == AQ_CLEAR && cad_action == AQ_SET) {
+ ret = PWM_POLARITY_INVERSED;
+ }
+
+ return ret;
+}
+
+static int ehrpwm_get_hw_state(struct pwm_chip *chip, struct pwm_device *pwm,
+ struct pwm_state *state)
+{
+ int ret;
+
+ if(chip == NULL || pwm == NULL || state == NULL){
+ return -EINVAL;
+ }
+
+ state->enabled = ehrpwm_is_enabled(chip);
+ state->period = ehrpwm_read_period(chip);
+ state->duty_cycle = ehrpwm_read_duty_cycle(chip);
+ state->polarity = ehrpwm_read_polarity(chip);
+
+ return ret;
+}
+
static void ehrpwm_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
{
struct ehrpwm_pwm_chip *pc = to_ehrpwm_pwm_chip(chip);
@@ -449,8 +575,10 @@ static int ehrpwm_pwm_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
struct ehrpwm_pwm_chip *pc;
+ struct pwm_state state;
struct pwm_chip *chip;
struct clk *clk;
+ bool tbclk_enabled;
int ret;
chip = devm_pwmchip_alloc(&pdev->dev, NUM_PWM_CHANNEL, sizeof(*pc));
@@ -501,10 +629,31 @@ static int ehrpwm_pwm_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, chip);
pm_runtime_enable(&pdev->dev);
+ ehrpwm_get_hw_state(chip, &chip->pwms[0], &state);
+ if(state.enabled == true) {
+ ret = clk_prepare_enable(pc->tbclk);
+ if (ret) {
+ dev_err(&pdev->dev, "clk_prepare_enable() failed: %d\n", ret);
+ goto err_pwmchip_remove;
+ }
+
+ tbclk_enabled = true;
+ ret = pm_runtime_get_sync(&pdev->dev);
+ if(ret < 0) {
+ dev_err(&pdev->dev, "pm_runtime_get_sync() failed: %d\n", ret);
+ clk_disable_unprepare(pc->tbclk);
+ goto err_pwmchip_remove;
+ }
+ }
+
return 0;
+err_pwmchip_remove:
+ pwmchip_remove(chip);
+
err_clk_unprepare:
- clk_unprepare(pc->tbclk);
+ if(tbclk_enabled)
+ clk_unprepare(pc->tbclk);
return ret;
}
--
2.25.1
next reply other threads:[~2025-02-04 18:56 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-04 18:55 Rafael V. Volkmer [this message]
2025-02-05 11:12 ` Uwe Kleine-König
2025-02-06 3:13 ` [PATCH v2 0/1] pwm: tiehrpwm: ensures that state.enabled is Rafael V. Volkmer
2025-02-06 3:18 ` [PATCH v2 1/1] pwm: tiehrpwm: ensures that state.enabled is synchronized in .probe() Rafael V. Volkmer
2025-02-07 8:49 ` Uwe Kleine-König
2025-02-07 21:29 ` [PATCH v3 1/3] pwm: tiehrpwm: replace manual bit definitions with bitfield.h macros Rafael V. Volkmer
2025-02-07 21:32 ` [PATCH v3 2/3] pwm: ehrpwm: add get_state function to retrieve PWM channel state Rafael V. Volkmer
2025-02-07 21:34 ` [PATCH v3 3/3] pwm: ehrpwm: ensure clock and runtime PM are enabled if hardware is active Rafael V. Volkmer
2025-03-26 11:45 ` Uwe Kleine-König
2025-04-19 19:48 ` [PATCH v4 1/4] pwm: tiehrpwm: replace manual bit definitions with bitfield.h macros Rafael V. Volkmer
2025-04-19 19:55 ` [PATCH v4 2/4] pwm: tiehrpwm: add get_state function to retrieve PWM channel state Rafael V. Volkmer
2025-04-19 19:58 ` [PATCH v4 3/4] pwm: tiehrpwm: ensure clock and runtime PM are enabled if hardware is active Rafael V. Volkmer
2025-04-19 20:01 ` [PATCH v4 4/4] pwm: tiehrpwm: drop unnecessary parentheses and fix spacing Rafael V. Volkmer
2025-05-13 9:36 ` Uwe Kleine-König
2025-05-13 9:33 ` [PATCH v4 3/4] pwm: tiehrpwm: ensure clock and runtime PM are enabled if hardware is active Uwe Kleine-König
2025-05-13 9:27 ` [PATCH v4 2/4] pwm: tiehrpwm: add get_state function to retrieve PWM channel state Uwe Kleine-König
2025-05-13 9:18 ` [PATCH v4 1/4] pwm: tiehrpwm: replace manual bit definitions with bitfield.h macros Uwe Kleine-König
2025-03-26 11:32 ` [PATCH v3 2/3] pwm: ehrpwm: add get_state function to retrieve PWM channel state Uwe Kleine-König
2025-04-20 2:21 ` [PATCH v2 1/1] pwm: tiehrpwm: ensures that state.enabled is synchronized in .probe() kernel test robot
2025-04-20 5:17 ` kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250204185540.52233-1-rafael.v.volkmer@gmail.com \
--to=rafael.v.volkmer@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pwm@vger.kernel.org \
--cc=ukleinek@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®