pwm: intel: Add PWM driver for a new SoC

Intel's LGM(Lightning Mountain) SoC contains a PWM fan controller which is only used to control the fan attached to the system. This PWM controller does not have any other consumer other than fan. Add DT bindings documentation for this PWM fan controller.

Signed-off-by Rahul Tanwar <rahul.tanwar-VuQAYsv1563Yd54FQh9/[email protected]>

---
 .../devicetree/bindings/pwm/intel,lgm-pwm.yaml     | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml

drivers/pwm/pwm-intel-lgm.c new file +9 -0

+static int lgm_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			 const struct pwm_state *state)
+{
+	struct lgm_pwm_chip *pc = to_lgm_pwm_chip(chip);
+	u32 duty_cycle, val;
+	int ret;
+
+	if (!state->enabled) {
+		ret = lgm_pwm_enable(chip, 0);

drivers/pwm/pwm-intel-lgm.c new file +49 -0

+	ret = regmap_update_bits(pc->regmap, LGM_PWM_FAN_CON0, LGM_PWM_FAN_DC_MSK,
+				 FIELD_PREP(LGM_PWM_FAN_DC_MSK, val));
+	if (ret)
+		return ret;
+
+	if (state->enabled)
+		ret = lgm_pwm_enable(chip, 1);
+
+	return ret;
+}
+
+static void lgm_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+			      struct pwm_state *state)
+{
+	struct lgm_pwm_chip *pc = to_lgm_pwm_chip(chip);
+	u32 duty, val;
+
+	state->enabled = regmap_test_bits(pc->regmap, LGM_PWM_FAN_CON0,
+					  LGM_PWM_FAN_EN_EN);
+	state->polarity = PWM_POLARITY_NORMAL;
+	state->period = pc->period; /* fixed period */
+
+	regmap_read(pc->regmap, LGM_PWM_FAN_CON0, &val);
+	duty = FIELD_GET(LGM_PWM_FAN_DC_MSK, val);
+	state->duty_cycle = DIV_ROUND_UP(duty * pc->period,
+					 LGM_PWM_MAX_DUTY_CYCLE);
+}
+
+static const struct pwm_ops lgm_pwm_ops = {
+	.get_state = lgm_pwm_get_state,
+	.apply = lgm_pwm_apply,
+	.owner = THIS_MODULE,
+};
+
+static void lgm_pwm_init(struct lgm_pwm_chip *pc)
+{
+	struct device *dev = pc->chip.dev;
+	struct regmap *regmap = pc->regmap;
+	u32 max_rpm, fan_wire, con0_val, con0_mask;
+
+	if (device_property_read_u32(dev, "intel,fan-wire", &fan_wire))
+		fan_wire = 2; /* default is 2 wire mode */
+
+	con0_mask = LGM_PWM_FAN_MODE_MSK;
+
+	switch (fan_wire) {
+	case 4:
+		con0_val = FIELD_PREP(LGM_PWM_FAN_MODE_MSK, LGM_PWM_FAN_MODE_4WIRE);
+		pc->period = LGM_PWM_PERIOD_4WIRE_NSECS;

Hello,

Quoted 9 lines · Rahul Tanwar

drivers/pwm/pwm-intel-lgm.c new file +9 -0

+static int lgm_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			 const struct pwm_state *state)
+{
+	struct lgm_pwm_chip *pc = to_lgm_pwm_chip(chip);
+	u32 duty_cycle, val;
+	int ret;
+
+	if (!state->enabled) {
+		ret = lgm_pwm_enable(chip, 0);

drivers/pwm/pwm-intel-lgm.c line 86

Quoted 10 lines

drivers/pwm/pwm-intel-lgm.c new file +10 -0

+		return ret;
+	}
+
+	/*
+	 * HW only supports NORMAL polarity
+	 * HW supports fixed period which can not be changed/configured by user
+	 */
+	if (state->polarity != PWM_POLARITY_NORMAL ||
+	    state->period != pc->period)
+		return -EINVAL;

At least for state->polarity you have to check before state->enabled, as the expectation on

.enabled = false
.polarity = PWM_POLARITY_INVERSED

is that the output becomes constant high. Also as confirmed at the end of v4, state->period < pc->period was the right check to do.

Hi Uwe,

Quoted 25 lines · Uwe Kleine-König

Hello,

Quoted 9 lines · Rahul Tanwar

drivers/pwm/pwm-intel-lgm.c new file +9 -0

+static int lgm_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			 const struct pwm_state *state)
+{
+	struct lgm_pwm_chip *pc = to_lgm_pwm_chip(chip);
+	u32 duty_cycle, val;
+	int ret;
+
+	if (!state->enabled) {
+		ret = lgm_pwm_enable(chip, 0);

drivers/pwm/pwm-intel-lgm.c line 86

Quoted 10 lines

drivers/pwm/pwm-intel-lgm.c new file +10 -0

+		return ret;
+	}
+
+	/*
+	 * HW only supports NORMAL polarity
+	 * HW supports fixed period which can not be changed/configured by user
+	 */
+	if (state->polarity != PWM_POLARITY_NORMAL ||
+	    state->period != pc->period)
+		return -EINVAL;

At least for state->polarity you have to check before state->enabled, as the expectation on

Quoted 5 lines

.enabled = false
.polarity = PWM_POLARITY_INVERSED

is that the output becomes constant high. Also as confirmed at the end of v4, state->period < pc->period was the right check to do.

For below case:

.enabled = false
.polarity = PWM_POLARITY_INVERSED

Since our HW does not support inversed polarity, the output for above case is expected to be constant low. And if we disable PWM before checking for polarity, the output becomes constant low. The code just does that. Sorry, i could not understand what is wrong with the code. It looks correct to me.

Given the fact that we support fixed period, if we allow state->period < pc->period case then the duty cycle will be evaluated as higher than the requested one because the state->period is lesser than the actual fixed period supported by the HW. Can you please elaborate on why you think we should allow state->period < pc->period case?

Quoted 35 lines · Rahul

Hi Uwe,

Quoted 25 lines · Uwe Kleine-König

Hello,

Quoted 9 lines · Rahul Tanwar

drivers/pwm/pwm-intel-lgm.c new file +9 -0

+static int lgm_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			 const struct pwm_state *state)
+{
+	struct lgm_pwm_chip *pc = to_lgm_pwm_chip(chip);
+	u32 duty_cycle, val;
+	int ret;
+
+	if (!state->enabled) {
+		ret = lgm_pwm_enable(chip, 0);

drivers/pwm/pwm-intel-lgm.c line 86

Quoted 10 lines

drivers/pwm/pwm-intel-lgm.c new file +10 -0

+		return ret;
+	}
+
+	/*
+	 * HW only supports NORMAL polarity
+	 * HW supports fixed period which can not be changed/configured by user
+	 */
+	if (state->polarity != PWM_POLARITY_NORMAL ||
+	    state->period != pc->period)
+		return -EINVAL;

At least for state->polarity you have to check before state->enabled, as the expectation on

Quoted 5 lines

.enabled = false
.polarity = PWM_POLARITY_INVERSED

is that the output becomes constant high. Also as confirmed at the end of v4, state->period < pc->period was the right check to do.

For below case:

Quoted 7 lines

.enabled = false
.polarity = PWM_POLARITY_INVERSED

Since our HW does not support inversed polarity, the output for above case is expected to be constant low. And if we disable PWM before checking for polarity, the output becomes constant low. The code just does that. Sorry, i could not understand what is wrong with the code. It looks correct to me.

As your hardware can only support normal polarity, the code must have:

	if (state->polarity != PWM_POLARITY_NORMAL)
		return -EINVAL;

	if (!state->enabled) {
		ret = lgm_pwm_enable(chip, 0);
		return ret;
	}

That's what I meant writing: "At least for state->polarity you have to check before state->enabled".

Quoted 5 lines

Given the fact that we support fixed period, if we allow state->period < pc->period case then the duty cycle will be evaluated as higher than the requested one because the state->period is lesser than the actual fixed period supported by the HW. Can you please elaborate on why you think we should allow state->period < pc->period case?

You should not allow it. In v4 you had:

	if (state->polarity != PWM_POLARITY_NORMAL ||
	    state->period < pc->period)
		return -EINVAL;

That's the right thing to do (even though I was unsettled at one point and wrote it was wrong). The check in v5 with state->period != pc->period is wrong.

Quoted 6 lines · Uwe Kleine-König

In v4 you had:

	if (state->polarity != PWM_POLARITY_NORMAL ||
	    state->period < pc->period)
		return -EINVAL;

Quoted 3 lines

That's the right thing to do (even though I was unsettled at one point and wrote it was wrong). The check in v5 with state->period != pc->period is wrong.

Does that mean we should allow state->period >= pc->period cases?

If the state->period is greater than HW supported pc->period and if we allow it then the duty cycle will again be evaluated to be incorrect/higher than requested duty cycle. Am i missing something else? Thanks.