[PATCH] pwm: tiehrpwm: ensures that state.enabled is synchronized in .probe()

From: Rafael V. Volkmer
Date: Tue Feb 04 2025 - 13:56:32 EST


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@xxxxxxxxx>
---
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