linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] pwm: Add different PWM output types support
@ 2019-09-13 22:57 Guru Das Srinagesh
  2019-09-13 22:57 ` [PATCH 2/2] pwm: core: Add option to config PWM duty/period with u64 data length Guru Das Srinagesh
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Guru Das Srinagesh @ 2019-09-13 22:57 UTC (permalink / raw)
  To: linux-pwm
  Cc: Thierry Reding, kernel-team, Mark Salyzyn, Sandeep Patil,
	Subbaraman Narayanamurthy, linux-kernel, Fenglin Wu,
	Guru Das Srinagesh

From: Fenglin Wu <fenglinw@codeaurora.org>

Normally, PWM channel has fixed output until software request to change
its settings. There are some PWM devices which their outputs could be
changed autonomously according to a predefined pattern programmed in
hardware. Add pwm_output_type enum type to identify these two different
PWM types and add relevant helper functions to set and get PWM output
types and pattern.

Change-Id: Ia1f914a45ab4f4dd7be037a395eeb89d0e65a80e
Signed-off-by: Fenglin Wu <fenglinw@codeaurora.org>
Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
---
 drivers/pwm/core.c  | 26 ++++++++++++++++++++
 drivers/pwm/sysfs.c | 50 ++++++++++++++++++++++++++++++++++++++
 include/linux/pwm.h | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 146 insertions(+)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 8edfac1..960a451 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -282,6 +282,7 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip,
 		pwm->pwm = chip->base + i;
 		pwm->hwpwm = i;
 		pwm->state.polarity = polarity;
+		pwm->state.output_type = PWM_OUTPUT_FIXED;
 
 		if (chip->ops->get_state)
 			chip->ops->get_state(chip, pwm, &pwm->state);
@@ -498,6 +499,31 @@ int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state)
 			pwm->state.polarity = state->polarity;
 		}
 
+		if (state->output_type != pwm->state.output_type) {
+			if (!pwm->chip->ops->set_output_type)
+				return -ENOTSUPP;
+
+			err = pwm->chip->ops->set_output_type(pwm->chip, pwm,
+						state->output_type);
+			if (err)
+				return err;
+
+			pwm->state.output_type = state->output_type;
+		}
+
+		if (state->output_pattern != pwm->state.output_pattern &&
+				state->output_pattern != NULL) {
+			if (!pwm->chip->ops->set_output_pattern)
+				return -ENOTSUPP;
+
+			err = pwm->chip->ops->set_output_pattern(pwm->chip,
+					pwm, state->output_pattern);
+			if (err)
+				return err;
+
+			pwm->state.output_pattern = state->output_pattern;
+		}
+
 		if (state->period != pwm->state.period ||
 		    state->duty_cycle != pwm->state.duty_cycle) {
 			err = pwm->chip->ops->config(pwm->chip, pwm,
diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index 2389b86..ab703f2 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -215,11 +215,60 @@ static ssize_t capture_show(struct device *child,
 	return sprintf(buf, "%u %u\n", result.period, result.duty_cycle);
 }
 
+static ssize_t output_type_show(struct device *child,
+			     struct device_attribute *attr,
+			     char *buf)
+{
+	const struct pwm_device *pwm = child_to_pwm_device(child);
+	const char *output_type = "unknown";
+	struct pwm_state state;
+
+	pwm_get_state(pwm, &state);
+	switch (state.output_type) {
+	case PWM_OUTPUT_FIXED:
+		output_type = "fixed";
+		break;
+	case PWM_OUTPUT_MODULATED:
+		output_type = "modulated";
+		break;
+	default:
+		break;
+	}
+
+	return snprintf(buf, PAGE_SIZE, "%s\n", output_type);
+}
+
+static ssize_t output_type_store(struct device *child,
+			      struct device_attribute *attr,
+			      const char *buf, size_t size)
+{
+	struct pwm_export *export = child_to_pwm_export(child);
+	struct pwm_device *pwm = export->pwm;
+	struct pwm_state state;
+	int ret = -EINVAL;
+
+	mutex_lock(&export->lock);
+	pwm_get_state(pwm, &state);
+	if (sysfs_streq(buf, "fixed"))
+		state.output_type = PWM_OUTPUT_FIXED;
+	else if (sysfs_streq(buf, "modulated"))
+		state.output_type = PWM_OUTPUT_MODULATED;
+	else
+		goto unlock;
+
+	ret = pwm_apply_state(pwm, &state);
+unlock:
+	mutex_unlock(&export->lock);
+
+	return ret ? : size;
+}
+
 static DEVICE_ATTR_RW(period);
 static DEVICE_ATTR_RW(duty_cycle);
 static DEVICE_ATTR_RW(enable);
 static DEVICE_ATTR_RW(polarity);
 static DEVICE_ATTR_RO(capture);
+static DEVICE_ATTR_RW(output_type);
 
 static struct attribute *pwm_attrs[] = {
 	&dev_attr_period.attr,
@@ -227,6 +276,7 @@ static ssize_t capture_show(struct device *child,
 	&dev_attr_enable.attr,
 	&dev_attr_polarity.attr,
 	&dev_attr_capture.attr,
+	&dev_attr_output_type.attr,
 	NULL
 };
 ATTRIBUTE_GROUPS(pwm);
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 24632a7..416f08e 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -48,6 +48,29 @@ enum {
 	PWMF_EXPORTED = 1 << 1,
 };
 
+/**
+ * enum pwm_output_type - output type of the PWM signal
+ * @PWM_OUTPUT_FIXED: PWM output is fixed until a change request
+ * @PWM_OUTPUT_MODULATED: PWM output is modulated in hardware
+ * autonomously with a predefined pattern
+ */
+enum pwm_output_type {
+	PWM_OUTPUT_FIXED = 1 << 0,
+	PWM_OUTPUT_MODULATED = 1 << 1,
+};
+
+/**
+ * struct pwm_output_pattern - PWM duty pattern for MODULATED duty type
+ * @duty_pattern: PWM duty cycles in the pattern for duty modulation
+ * @num_entries: number of entries in the pattern
+ * @cycles_per_duty: number of PWM period cycles an entry stays at
+ */
+struct pwm_output_pattern {
+	unsigned int *duty_pattern;
+	unsigned int num_entries;
+	unsigned int cycles_per_duty;
+};
+
 /*
  * struct pwm_state - state of a PWM channel
  * @period: PWM period (in nanoseconds)
@@ -59,6 +82,8 @@ struct pwm_state {
 	unsigned int period;
 	unsigned int duty_cycle;
 	enum pwm_polarity polarity;
+	enum pwm_output_type output_type;
+	struct pwm_output_pattern *output_pattern;
 	bool enabled;
 };
 
@@ -144,6 +169,26 @@ static inline enum pwm_polarity pwm_get_polarity(const struct pwm_device *pwm)
 	return state.polarity;
 }
 
+static inline enum pwm_output_type pwm_get_output_type(
+		const struct pwm_device *pwm)
+{
+	struct pwm_state state;
+
+	pwm_get_state(pwm, &state);
+
+	return state.output_type;
+}
+
+static inline struct pwm_output_pattern *pwm_get_output_pattern(
+				struct pwm_device *pwm)
+{
+	struct pwm_state state;
+
+	pwm_get_state(pwm, &state);
+
+	return pwm->state.output_pattern ?: NULL;
+}
+
 static inline void pwm_get_args(const struct pwm_device *pwm,
 				struct pwm_args *args)
 {
@@ -250,6 +295,9 @@ static inline void pwm_init_state(const struct pwm_device *pwm,
  * @get_state: get the current PWM state. This function is only
  *	       called once per PWM device when the PWM chip is
  *	       registered.
+ * @get_output_type_supported: get the supported output type
+ * @set_output_type: set PWM output type
+ * @set_output_pattern: set the pattern for the modulated output
  * @owner: helps prevent removal of modules exporting active PWMs
  * @config: configure duty cycles and period length for this PWM
  * @set_polarity: configure the polarity of this PWM
@@ -265,6 +313,13 @@ struct pwm_ops {
 		     struct pwm_state *state);
 	void (*get_state)(struct pwm_chip *chip, struct pwm_device *pwm,
 			  struct pwm_state *state);
+	int (*get_output_type_supported)(struct pwm_chip *chip,
+			struct pwm_device *pwm);
+	int (*set_output_type)(struct pwm_chip *chip, struct pwm_device *pwm,
+			enum pwm_output_type output_type);
+	int (*set_output_pattern)(struct pwm_chip *chip,
+			struct pwm_device *pwm,
+			struct pwm_output_pattern *output_pattern);
 	struct module *owner;
 
 	/* Only used by legacy drivers */
@@ -320,6 +375,21 @@ struct pwm_capture {
 int pwm_adjust_config(struct pwm_device *pwm);
 
 /**
+ * pwm_output_type_support()
+ * @pwm: PWM device
+ *
+ * Returns:  output types supported by the PWM device
+ */
+static inline int pwm_get_output_type_supported(struct pwm_device *pwm)
+{
+	if (pwm->chip->ops->get_output_type_supported != NULL)
+		return pwm->chip->ops->get_output_type_supported(pwm->chip,
+				pwm);
+	else
+		return PWM_OUTPUT_FIXED;
+}
+
+/**
  * pwm_config() - change a PWM device configuration
  * @pwm: PWM device
  * @duty_ns: "on" time (in nanoseconds)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

* [PATCH 2/2] pwm: core: Add option to config PWM duty/period with u64 data length
  2019-09-13 22:57 [PATCH 1/2] pwm: Add different PWM output types support Guru Das Srinagesh
@ 2019-09-13 22:57 ` Guru Das Srinagesh
  2019-09-16 13:59   ` kbuild test robot
                     ` (2 more replies)
  2019-09-16 14:01 ` [PATCH 1/2] pwm: Add different PWM output types support Thierry Reding
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 12+ messages in thread
From: Guru Das Srinagesh @ 2019-09-13 22:57 UTC (permalink / raw)
  To: linux-pwm
  Cc: Thierry Reding, kernel-team, Mark Salyzyn, Sandeep Patil,
	Subbaraman Narayanamurthy, linux-kernel, Fenglin Wu,
	Guru Das Srinagesh

From: Fenglin Wu <fenglinw@codeaurora.org>

Currently, PWM core driver provides interfaces for configuring PWM
period and duty length in nanoseconds with an integer data type, so
the max period can be only set to ~2.147 seconds. Add interfaces which
can set PWM period and duty with u64 data type to remove this
limitation.

Change-Id: Ic8722088510d447fc939ab6a5014711aef1b832f
Signed-off-by: Fenglin Wu <fenglinw@codeaurora.org>
Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
---
 drivers/pwm/core.c  |  20 ++++++---
 drivers/pwm/sysfs.c |   6 +--
 include/linux/pwm.h | 115 ++++++++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 126 insertions(+), 15 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 960a451..02ad16b 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -526,9 +526,19 @@ int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state)
 
 		if (state->period != pwm->state.period ||
 		    state->duty_cycle != pwm->state.duty_cycle) {
-			err = pwm->chip->ops->config(pwm->chip, pwm,
-						     state->duty_cycle,
-						     state->period);
+			if (pwm->chip->ops->config_extend) {
+				err = pwm->chip->ops->config_extend(pwm->chip,
+						pwm, state->duty_cycle,
+						state->period);
+			} else {
+				if (state->period > UINT_MAX)
+					pr_warn("period %llu duty_cycle %llu will be truncated\n",
+							state->period,
+							state->duty_cycle);
+				err = pwm->chip->ops->config(pwm->chip, pwm,
+						state->duty_cycle,
+						state->period);
+			}
 			if (err)
 				return err;
 
@@ -1181,8 +1191,8 @@ static void pwm_dbg_show(struct pwm_chip *chip, struct seq_file *s)
 		if (state.enabled)
 			seq_puts(s, " enabled");
 
-		seq_printf(s, " period: %u ns", state.period);
-		seq_printf(s, " duty: %u ns", state.duty_cycle);
+		seq_printf(s, " period: %llu ns", state.period);
+		seq_printf(s, " duty: %llu ns", state.duty_cycle);
 		seq_printf(s, " polarity: %s",
 			   state.polarity ? "inverse" : "normal");
 
diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index ab703f2..ef78c40 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -42,7 +42,7 @@ static ssize_t period_show(struct device *child,
 
 	pwm_get_state(pwm, &state);
 
-	return sprintf(buf, "%u\n", state.period);
+	return sprintf(buf, "%llu\n", state.period);
 }
 
 static ssize_t period_store(struct device *child,
@@ -77,7 +77,7 @@ static ssize_t duty_cycle_show(struct device *child,
 
 	pwm_get_state(pwm, &state);
 
-	return sprintf(buf, "%u\n", state.duty_cycle);
+	return sprintf(buf, "%llu\n", state.duty_cycle);
 }
 
 static ssize_t duty_cycle_store(struct device *child,
@@ -212,7 +212,7 @@ static ssize_t capture_show(struct device *child,
 	if (ret)
 		return ret;
 
-	return sprintf(buf, "%u %u\n", result.period, result.duty_cycle);
+	return sprintf(buf, "%llu %llu\n", result.period, result.duty_cycle);
 }
 
 static ssize_t output_type_show(struct device *child,
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 416f08e..d714385 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -39,7 +39,7 @@ enum pwm_polarity {
  * current PWM hardware state.
  */
 struct pwm_args {
-	unsigned int period;
+	u64 period;
 	enum pwm_polarity polarity;
 };
 
@@ -66,9 +66,9 @@ enum pwm_output_type {
  * @cycles_per_duty: number of PWM period cycles an entry stays at
  */
 struct pwm_output_pattern {
-	unsigned int *duty_pattern;
+	u64 *duty_pattern;
 	unsigned int num_entries;
-	unsigned int cycles_per_duty;
+	u64 cycles_per_duty;
 };
 
 /*
@@ -79,8 +79,8 @@ struct pwm_output_pattern {
  * @enabled: PWM enabled status
  */
 struct pwm_state {
-	unsigned int period;
-	unsigned int duty_cycle;
+	u64 period;
+	u64 duty_cycle;
 	enum pwm_polarity polarity;
 	enum pwm_output_type output_type;
 	struct pwm_output_pattern *output_pattern;
@@ -136,12 +136,30 @@ static inline void pwm_set_period(struct pwm_device *pwm, unsigned int period)
 		pwm->state.period = period;
 }
 
+static inline void pwm_set_period_extend(struct pwm_device *pwm, u64 period)
+{
+	if (pwm)
+		pwm->state.period = period;
+}
+
 static inline unsigned int pwm_get_period(const struct pwm_device *pwm)
 {
 	struct pwm_state state;
 
 	pwm_get_state(pwm, &state);
 
+	if (state.period > UINT_MAX)
+		pr_warn("PWM period %llu is truncated\n", state.period);
+
+	return (unsigned int)state.period;
+}
+
+static inline u64 pwm_get_period_extend(const struct pwm_device *pwm)
+{
+	struct pwm_state state;
+
+	pwm_get_state(pwm, &state);
+
 	return state.period;
 }
 
@@ -151,12 +169,30 @@ static inline void pwm_set_duty_cycle(struct pwm_device *pwm, unsigned int duty)
 		pwm->state.duty_cycle = duty;
 }
 
+static inline void pwm_set_duty_cycle_extend(struct pwm_device *pwm, u64 duty)
+{
+	if (pwm)
+		pwm->state.duty_cycle = duty;
+}
+
 static inline unsigned int pwm_get_duty_cycle(const struct pwm_device *pwm)
 {
 	struct pwm_state state;
 
 	pwm_get_state(pwm, &state);
 
+	if (state.duty_cycle > UINT_MAX)
+		pr_warn("PWM duty cycle %llu is truncated\n", state.duty_cycle);
+
+	return (unsigned int)state.duty_cycle;
+}
+
+static inline u64 pwm_get_duty_cycle_extend(const struct pwm_device *pwm)
+{
+	struct pwm_state state;
+
+	pwm_get_state(pwm, &state);
+
 	return state.duty_cycle;
 }
 
@@ -300,6 +336,8 @@ static inline void pwm_init_state(const struct pwm_device *pwm,
  * @set_output_pattern: set the pattern for the modulated output
  * @owner: helps prevent removal of modules exporting active PWMs
  * @config: configure duty cycles and period length for this PWM
+ * @config_extend: configure duty cycles and period length for this
+ *	PWM with u64 data type
  * @set_polarity: configure the polarity of this PWM
  * @enable: enable PWM output toggling
  * @disable: disable PWM output toggling
@@ -325,6 +363,8 @@ struct pwm_ops {
 	/* Only used by legacy drivers */
 	int (*config)(struct pwm_chip *chip, struct pwm_device *pwm,
 		      int duty_ns, int period_ns);
+	int (*config_extend)(struct pwm_chip *chip, struct pwm_device *pwm,
+		      u64 duty_ns, u64 period_ns);
 	int (*set_polarity)(struct pwm_chip *chip, struct pwm_device *pwm,
 			    enum pwm_polarity polarity);
 	int (*enable)(struct pwm_chip *chip, struct pwm_device *pwm);
@@ -363,8 +403,8 @@ struct pwm_chip {
  * @duty_cycle: duty cycle of the PWM signal (in nanoseconds)
  */
 struct pwm_capture {
-	unsigned int period;
-	unsigned int duty_cycle;
+	u64 period;
+	u64 duty_cycle;
 };
 
 #if IS_ENABLED(CONFIG_PWM)
@@ -418,6 +458,67 @@ static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
 }
 
 /**
+ * pwm_config_extend() - change PWM period and duty length with u64 data type
+ * @pwm: PWM device
+ * @duty_ns: "on" time (in nanoseconds)
+ * @period_ns: duration (in nanoseconds) of one cycle
+ *
+ * Returns: 0 on success or a negative error code on failure.
+ */
+static inline int pwm_config_extend(struct pwm_device *pwm, u64 duty_ns,
+			     u64 period_ns)
+{
+	struct pwm_state state;
+
+	if (!pwm)
+		return -EINVAL;
+
+	pwm_get_state(pwm, &state);
+	if (state.duty_cycle == duty_ns && state.period == period_ns)
+		return 0;
+
+	state.duty_cycle = duty_ns;
+	state.period = period_ns;
+	return pwm_apply_state(pwm, &state);
+}
+
+/**
+ * pwm_set_polarity() - configure the polarity of a PWM signal
+ * @pwm: PWM device
+ * @polarity: new polarity of the PWM signal
+ *
+ * Note that the polarity cannot be configured while the PWM device is
+ * enabled.
+ *
+ * Returns: 0 on success or a negative error code on failure.
+ */
+static inline int pwm_set_polarity(struct pwm_device *pwm,
+				   enum pwm_polarity polarity)
+{
+	struct pwm_state state;
+
+	if (!pwm)
+		return -EINVAL;
+
+	pwm_get_state(pwm, &state);
+	if (state.polarity == polarity)
+		return 0;
+
+	/*
+	 * Changing the polarity of a running PWM without adjusting the
+	 * dutycycle/period value is a bit risky (can introduce glitches).
+	 * Return -EBUSY in this case.
+	 * Note that this is allowed when using pwm_apply_state() because
+	 * the user specifies all the parameters.
+	 */
+	if (state.enabled)
+		return -EBUSY;
+
+	state.polarity = polarity;
+	return pwm_apply_state(pwm, &state);
+}
+
+/**
  * pwm_enable() - start a PWM output toggling
  * @pwm: PWM device
  *
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

* Re: [PATCH 2/2] pwm: core: Add option to config PWM duty/period with u64 data length
  2019-09-13 22:57 ` [PATCH 2/2] pwm: core: Add option to config PWM duty/period with u64 data length Guru Das Srinagesh
@ 2019-09-16 13:59   ` kbuild test robot
  2019-09-16 14:00   ` Thierry Reding
  2019-09-16 14:07   ` kbuild test robot
  2 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2019-09-16 13:59 UTC (permalink / raw)
  To: Guru Das Srinagesh
  Cc: kbuild-all, linux-pwm, Thierry Reding, kernel-team, Mark Salyzyn,
	Sandeep Patil, Subbaraman Narayanamurthy, linux-kernel,
	Fenglin Wu, Guru Das Srinagesh

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

Hi Guru,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.3 next-20190915]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Guru-Das-Srinagesh/pwm-Add-different-PWM-output-types-support/20190916-151008
config: i386-randconfig-h004-201937 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-11) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: drivers/hwmon/pwm-fan.o: in function `pwm_fan_resume':
>> drivers/hwmon/pwm-fan.c:440: undefined reference to `__udivdi3'

vim +440 drivers/hwmon/pwm-fan.c

d82d57767c8598 Kamil Debski    2014-07-16  420  
d82d57767c8598 Kamil Debski    2014-07-16  421  static int pwm_fan_resume(struct device *dev)
d82d57767c8598 Kamil Debski    2014-07-16  422  {
d82d57767c8598 Kamil Debski    2014-07-16  423  	struct pwm_fan_ctx *ctx = dev_get_drvdata(dev);
2289711c9d4d58 Boris Brezillon 2016-04-14  424  	struct pwm_args pargs;
48b9d5b4f40825 Kamil Debski    2014-11-03  425  	unsigned long duty;
48b9d5b4f40825 Kamil Debski    2014-11-03  426  	int ret;
d82d57767c8598 Kamil Debski    2014-07-16  427  
b57e1d42939721 Stefan Wahren   2019-02-22  428  	if (ctx->reg_en) {
b57e1d42939721 Stefan Wahren   2019-02-22  429  		ret = regulator_enable(ctx->reg_en);
b57e1d42939721 Stefan Wahren   2019-02-22  430  		if (ret) {
b57e1d42939721 Stefan Wahren   2019-02-22  431  			dev_err(dev, "Failed to enable fan supply: %d\n", ret);
b57e1d42939721 Stefan Wahren   2019-02-22  432  			return ret;
b57e1d42939721 Stefan Wahren   2019-02-22  433  		}
b57e1d42939721 Stefan Wahren   2019-02-22  434  	}
b57e1d42939721 Stefan Wahren   2019-02-22  435  
48b9d5b4f40825 Kamil Debski    2014-11-03  436  	if (ctx->pwm_value == 0)
d82d57767c8598 Kamil Debski    2014-07-16  437  		return 0;
48b9d5b4f40825 Kamil Debski    2014-11-03  438  
2289711c9d4d58 Boris Brezillon 2016-04-14  439  	pwm_get_args(ctx->pwm, &pargs);
2289711c9d4d58 Boris Brezillon 2016-04-14 @440  	duty = DIV_ROUND_UP(ctx->pwm_value * (pargs.period - 1), MAX_PWM);
2289711c9d4d58 Boris Brezillon 2016-04-14  441  	ret = pwm_config(ctx->pwm, duty, pargs.period);
48b9d5b4f40825 Kamil Debski    2014-11-03  442  	if (ret)
48b9d5b4f40825 Kamil Debski    2014-11-03  443  		return ret;
48b9d5b4f40825 Kamil Debski    2014-11-03  444  	return pwm_enable(ctx->pwm);
d82d57767c8598 Kamil Debski    2014-07-16  445  }
d82d57767c8598 Kamil Debski    2014-07-16  446  #endif
d82d57767c8598 Kamil Debski    2014-07-16  447  

:::::: The code at line 440 was first introduced by commit
:::::: 2289711c9d4d588954ff86a06685f1579bf6c446 hwmon: pwm-fan: Use pwm_get_args() where appropriate

:::::: TO: Boris Brezillon <boris.brezillon@free-electrons.com>
:::::: CC: Thierry Reding <thierry.reding@gmail.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 37366 bytes --]

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

* Re: [PATCH 2/2] pwm: core: Add option to config PWM duty/period with u64 data length
  2019-09-13 22:57 ` [PATCH 2/2] pwm: core: Add option to config PWM duty/period with u64 data length Guru Das Srinagesh
  2019-09-16 13:59   ` kbuild test robot
@ 2019-09-16 14:00   ` Thierry Reding
  2019-09-16 14:07   ` kbuild test robot
  2 siblings, 0 replies; 12+ messages in thread
From: Thierry Reding @ 2019-09-16 14:00 UTC (permalink / raw)
  To: Guru Das Srinagesh
  Cc: linux-pwm, kernel-team, Mark Salyzyn, Sandeep Patil,
	Subbaraman Narayanamurthy, linux-kernel, Fenglin Wu

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

On Fri, Sep 13, 2019 at 03:57:44PM -0700, Guru Das Srinagesh wrote:
> From: Fenglin Wu <fenglinw@codeaurora.org>
> 
> Currently, PWM core driver provides interfaces for configuring PWM
> period and duty length in nanoseconds with an integer data type, so
> the max period can be only set to ~2.147 seconds. Add interfaces which
> can set PWM period and duty with u64 data type to remove this
> limitation.
> 
> Change-Id: Ic8722088510d447fc939ab6a5014711aef1b832f
> Signed-off-by: Fenglin Wu <fenglinw@codeaurora.org>
> Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
> ---
>  drivers/pwm/core.c  |  20 ++++++---
>  drivers/pwm/sysfs.c |   6 +--
>  include/linux/pwm.h | 115 ++++++++++++++++++++++++++++++++++++++++++++++++----
>  3 files changed, 126 insertions(+), 15 deletions(-)

pwm_ops->config() is considered legacy API and only remains for drivers
that haven't been converted to the atomic API yet. If you want to extend
the range for period and duty cycle, please add that to the atomic API,
which is pwm_ops->apply() and struct pwm_state.

Thierry

> 
> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> index 960a451..02ad16b 100644
> --- a/drivers/pwm/core.c
> +++ b/drivers/pwm/core.c
> @@ -526,9 +526,19 @@ int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state)
>  
>  		if (state->period != pwm->state.period ||
>  		    state->duty_cycle != pwm->state.duty_cycle) {
> -			err = pwm->chip->ops->config(pwm->chip, pwm,
> -						     state->duty_cycle,
> -						     state->period);
> +			if (pwm->chip->ops->config_extend) {
> +				err = pwm->chip->ops->config_extend(pwm->chip,
> +						pwm, state->duty_cycle,
> +						state->period);
> +			} else {
> +				if (state->period > UINT_MAX)
> +					pr_warn("period %llu duty_cycle %llu will be truncated\n",
> +							state->period,
> +							state->duty_cycle);
> +				err = pwm->chip->ops->config(pwm->chip, pwm,
> +						state->duty_cycle,
> +						state->period);
> +			}
>  			if (err)
>  				return err;
>  
> @@ -1181,8 +1191,8 @@ static void pwm_dbg_show(struct pwm_chip *chip, struct seq_file *s)
>  		if (state.enabled)
>  			seq_puts(s, " enabled");
>  
> -		seq_printf(s, " period: %u ns", state.period);
> -		seq_printf(s, " duty: %u ns", state.duty_cycle);
> +		seq_printf(s, " period: %llu ns", state.period);
> +		seq_printf(s, " duty: %llu ns", state.duty_cycle);
>  		seq_printf(s, " polarity: %s",
>  			   state.polarity ? "inverse" : "normal");
>  
> diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
> index ab703f2..ef78c40 100644
> --- a/drivers/pwm/sysfs.c
> +++ b/drivers/pwm/sysfs.c
> @@ -42,7 +42,7 @@ static ssize_t period_show(struct device *child,
>  
>  	pwm_get_state(pwm, &state);
>  
> -	return sprintf(buf, "%u\n", state.period);
> +	return sprintf(buf, "%llu\n", state.period);
>  }
>  
>  static ssize_t period_store(struct device *child,
> @@ -77,7 +77,7 @@ static ssize_t duty_cycle_show(struct device *child,
>  
>  	pwm_get_state(pwm, &state);
>  
> -	return sprintf(buf, "%u\n", state.duty_cycle);
> +	return sprintf(buf, "%llu\n", state.duty_cycle);
>  }
>  
>  static ssize_t duty_cycle_store(struct device *child,
> @@ -212,7 +212,7 @@ static ssize_t capture_show(struct device *child,
>  	if (ret)
>  		return ret;
>  
> -	return sprintf(buf, "%u %u\n", result.period, result.duty_cycle);
> +	return sprintf(buf, "%llu %llu\n", result.period, result.duty_cycle);
>  }
>  
>  static ssize_t output_type_show(struct device *child,
> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> index 416f08e..d714385 100644
> --- a/include/linux/pwm.h
> +++ b/include/linux/pwm.h
> @@ -39,7 +39,7 @@ enum pwm_polarity {
>   * current PWM hardware state.
>   */
>  struct pwm_args {
> -	unsigned int period;
> +	u64 period;
>  	enum pwm_polarity polarity;
>  };
>  
> @@ -66,9 +66,9 @@ enum pwm_output_type {
>   * @cycles_per_duty: number of PWM period cycles an entry stays at
>   */
>  struct pwm_output_pattern {
> -	unsigned int *duty_pattern;
> +	u64 *duty_pattern;
>  	unsigned int num_entries;
> -	unsigned int cycles_per_duty;
> +	u64 cycles_per_duty;
>  };
>  
>  /*
> @@ -79,8 +79,8 @@ struct pwm_output_pattern {
>   * @enabled: PWM enabled status
>   */
>  struct pwm_state {
> -	unsigned int period;
> -	unsigned int duty_cycle;
> +	u64 period;
> +	u64 duty_cycle;
>  	enum pwm_polarity polarity;
>  	enum pwm_output_type output_type;
>  	struct pwm_output_pattern *output_pattern;
> @@ -136,12 +136,30 @@ static inline void pwm_set_period(struct pwm_device *pwm, unsigned int period)
>  		pwm->state.period = period;
>  }
>  
> +static inline void pwm_set_period_extend(struct pwm_device *pwm, u64 period)
> +{
> +	if (pwm)
> +		pwm->state.period = period;
> +}
> +
>  static inline unsigned int pwm_get_period(const struct pwm_device *pwm)
>  {
>  	struct pwm_state state;
>  
>  	pwm_get_state(pwm, &state);
>  
> +	if (state.period > UINT_MAX)
> +		pr_warn("PWM period %llu is truncated\n", state.period);
> +
> +	return (unsigned int)state.period;
> +}
> +
> +static inline u64 pwm_get_period_extend(const struct pwm_device *pwm)
> +{
> +	struct pwm_state state;
> +
> +	pwm_get_state(pwm, &state);
> +
>  	return state.period;
>  }
>  
> @@ -151,12 +169,30 @@ static inline void pwm_set_duty_cycle(struct pwm_device *pwm, unsigned int duty)
>  		pwm->state.duty_cycle = duty;
>  }
>  
> +static inline void pwm_set_duty_cycle_extend(struct pwm_device *pwm, u64 duty)
> +{
> +	if (pwm)
> +		pwm->state.duty_cycle = duty;
> +}
> +
>  static inline unsigned int pwm_get_duty_cycle(const struct pwm_device *pwm)
>  {
>  	struct pwm_state state;
>  
>  	pwm_get_state(pwm, &state);
>  
> +	if (state.duty_cycle > UINT_MAX)
> +		pr_warn("PWM duty cycle %llu is truncated\n", state.duty_cycle);
> +
> +	return (unsigned int)state.duty_cycle;
> +}
> +
> +static inline u64 pwm_get_duty_cycle_extend(const struct pwm_device *pwm)
> +{
> +	struct pwm_state state;
> +
> +	pwm_get_state(pwm, &state);
> +
>  	return state.duty_cycle;
>  }
>  
> @@ -300,6 +336,8 @@ static inline void pwm_init_state(const struct pwm_device *pwm,
>   * @set_output_pattern: set the pattern for the modulated output
>   * @owner: helps prevent removal of modules exporting active PWMs
>   * @config: configure duty cycles and period length for this PWM
> + * @config_extend: configure duty cycles and period length for this
> + *	PWM with u64 data type
>   * @set_polarity: configure the polarity of this PWM
>   * @enable: enable PWM output toggling
>   * @disable: disable PWM output toggling
> @@ -325,6 +363,8 @@ struct pwm_ops {
>  	/* Only used by legacy drivers */
>  	int (*config)(struct pwm_chip *chip, struct pwm_device *pwm,
>  		      int duty_ns, int period_ns);
> +	int (*config_extend)(struct pwm_chip *chip, struct pwm_device *pwm,
> +		      u64 duty_ns, u64 period_ns);
>  	int (*set_polarity)(struct pwm_chip *chip, struct pwm_device *pwm,
>  			    enum pwm_polarity polarity);
>  	int (*enable)(struct pwm_chip *chip, struct pwm_device *pwm);
> @@ -363,8 +403,8 @@ struct pwm_chip {
>   * @duty_cycle: duty cycle of the PWM signal (in nanoseconds)
>   */
>  struct pwm_capture {
> -	unsigned int period;
> -	unsigned int duty_cycle;
> +	u64 period;
> +	u64 duty_cycle;
>  };
>  
>  #if IS_ENABLED(CONFIG_PWM)
> @@ -418,6 +458,67 @@ static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
>  }
>  
>  /**
> + * pwm_config_extend() - change PWM period and duty length with u64 data type
> + * @pwm: PWM device
> + * @duty_ns: "on" time (in nanoseconds)
> + * @period_ns: duration (in nanoseconds) of one cycle
> + *
> + * Returns: 0 on success or a negative error code on failure.
> + */
> +static inline int pwm_config_extend(struct pwm_device *pwm, u64 duty_ns,
> +			     u64 period_ns)
> +{
> +	struct pwm_state state;
> +
> +	if (!pwm)
> +		return -EINVAL;
> +
> +	pwm_get_state(pwm, &state);
> +	if (state.duty_cycle == duty_ns && state.period == period_ns)
> +		return 0;
> +
> +	state.duty_cycle = duty_ns;
> +	state.period = period_ns;
> +	return pwm_apply_state(pwm, &state);
> +}
> +
> +/**
> + * pwm_set_polarity() - configure the polarity of a PWM signal
> + * @pwm: PWM device
> + * @polarity: new polarity of the PWM signal
> + *
> + * Note that the polarity cannot be configured while the PWM device is
> + * enabled.
> + *
> + * Returns: 0 on success or a negative error code on failure.
> + */
> +static inline int pwm_set_polarity(struct pwm_device *pwm,
> +				   enum pwm_polarity polarity)
> +{
> +	struct pwm_state state;
> +
> +	if (!pwm)
> +		return -EINVAL;
> +
> +	pwm_get_state(pwm, &state);
> +	if (state.polarity == polarity)
> +		return 0;
> +
> +	/*
> +	 * Changing the polarity of a running PWM without adjusting the
> +	 * dutycycle/period value is a bit risky (can introduce glitches).
> +	 * Return -EBUSY in this case.
> +	 * Note that this is allowed when using pwm_apply_state() because
> +	 * the user specifies all the parameters.
> +	 */
> +	if (state.enabled)
> +		return -EBUSY;
> +
> +	state.polarity = polarity;
> +	return pwm_apply_state(pwm, &state);
> +}
> +
> +/**
>   * pwm_enable() - start a PWM output toggling
>   * @pwm: PWM device
>   *
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

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

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

* Re: [PATCH 1/2] pwm: Add different PWM output types support
  2019-09-13 22:57 [PATCH 1/2] pwm: Add different PWM output types support Guru Das Srinagesh
  2019-09-13 22:57 ` [PATCH 2/2] pwm: core: Add option to config PWM duty/period with u64 data length Guru Das Srinagesh
@ 2019-09-16 14:01 ` Thierry Reding
  2019-09-24  5:43   ` Guru Das Srinagesh
  2019-09-16 18:25 ` Uwe Kleine-König
  2021-09-18 11:18 ` Greg KH
  3 siblings, 1 reply; 12+ messages in thread
From: Thierry Reding @ 2019-09-16 14:01 UTC (permalink / raw)
  To: Guru Das Srinagesh
  Cc: linux-pwm, kernel-team, Mark Salyzyn, Sandeep Patil,
	Subbaraman Narayanamurthy, linux-kernel, Fenglin Wu

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

On Fri, Sep 13, 2019 at 03:57:43PM -0700, Guru Das Srinagesh wrote:
> From: Fenglin Wu <fenglinw@codeaurora.org>
> 
> Normally, PWM channel has fixed output until software request to change
> its settings. There are some PWM devices which their outputs could be
> changed autonomously according to a predefined pattern programmed in
> hardware. Add pwm_output_type enum type to identify these two different
> PWM types and add relevant helper functions to set and get PWM output
> types and pattern.
> 
> Change-Id: Ia1f914a45ab4f4dd7be037a395eeb89d0e65a80e
> Signed-off-by: Fenglin Wu <fenglinw@codeaurora.org>
> Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
> ---
>  drivers/pwm/core.c  | 26 ++++++++++++++++++++
>  drivers/pwm/sysfs.c | 50 ++++++++++++++++++++++++++++++++++++++
>  include/linux/pwm.h | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 146 insertions(+)

This doesn't seem right to me. Are you describing a PWM pin that's
actually driven in GPIO mode? We usually configure that using pinctrl.

Thierry

> 
> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> index 8edfac1..960a451 100644
> --- a/drivers/pwm/core.c
> +++ b/drivers/pwm/core.c
> @@ -282,6 +282,7 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip,
>  		pwm->pwm = chip->base + i;
>  		pwm->hwpwm = i;
>  		pwm->state.polarity = polarity;
> +		pwm->state.output_type = PWM_OUTPUT_FIXED;
>  
>  		if (chip->ops->get_state)
>  			chip->ops->get_state(chip, pwm, &pwm->state);
> @@ -498,6 +499,31 @@ int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state)
>  			pwm->state.polarity = state->polarity;
>  		}
>  
> +		if (state->output_type != pwm->state.output_type) {
> +			if (!pwm->chip->ops->set_output_type)
> +				return -ENOTSUPP;
> +
> +			err = pwm->chip->ops->set_output_type(pwm->chip, pwm,
> +						state->output_type);
> +			if (err)
> +				return err;
> +
> +			pwm->state.output_type = state->output_type;
> +		}
> +
> +		if (state->output_pattern != pwm->state.output_pattern &&
> +				state->output_pattern != NULL) {
> +			if (!pwm->chip->ops->set_output_pattern)
> +				return -ENOTSUPP;
> +
> +			err = pwm->chip->ops->set_output_pattern(pwm->chip,
> +					pwm, state->output_pattern);
> +			if (err)
> +				return err;
> +
> +			pwm->state.output_pattern = state->output_pattern;
> +		}
> +
>  		if (state->period != pwm->state.period ||
>  		    state->duty_cycle != pwm->state.duty_cycle) {
>  			err = pwm->chip->ops->config(pwm->chip, pwm,
> diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
> index 2389b86..ab703f2 100644
> --- a/drivers/pwm/sysfs.c
> +++ b/drivers/pwm/sysfs.c
> @@ -215,11 +215,60 @@ static ssize_t capture_show(struct device *child,
>  	return sprintf(buf, "%u %u\n", result.period, result.duty_cycle);
>  }
>  
> +static ssize_t output_type_show(struct device *child,
> +			     struct device_attribute *attr,
> +			     char *buf)
> +{
> +	const struct pwm_device *pwm = child_to_pwm_device(child);
> +	const char *output_type = "unknown";
> +	struct pwm_state state;
> +
> +	pwm_get_state(pwm, &state);
> +	switch (state.output_type) {
> +	case PWM_OUTPUT_FIXED:
> +		output_type = "fixed";
> +		break;
> +	case PWM_OUTPUT_MODULATED:
> +		output_type = "modulated";
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return snprintf(buf, PAGE_SIZE, "%s\n", output_type);
> +}
> +
> +static ssize_t output_type_store(struct device *child,
> +			      struct device_attribute *attr,
> +			      const char *buf, size_t size)
> +{
> +	struct pwm_export *export = child_to_pwm_export(child);
> +	struct pwm_device *pwm = export->pwm;
> +	struct pwm_state state;
> +	int ret = -EINVAL;
> +
> +	mutex_lock(&export->lock);
> +	pwm_get_state(pwm, &state);
> +	if (sysfs_streq(buf, "fixed"))
> +		state.output_type = PWM_OUTPUT_FIXED;
> +	else if (sysfs_streq(buf, "modulated"))
> +		state.output_type = PWM_OUTPUT_MODULATED;
> +	else
> +		goto unlock;
> +
> +	ret = pwm_apply_state(pwm, &state);
> +unlock:
> +	mutex_unlock(&export->lock);
> +
> +	return ret ? : size;
> +}
> +
>  static DEVICE_ATTR_RW(period);
>  static DEVICE_ATTR_RW(duty_cycle);
>  static DEVICE_ATTR_RW(enable);
>  static DEVICE_ATTR_RW(polarity);
>  static DEVICE_ATTR_RO(capture);
> +static DEVICE_ATTR_RW(output_type);
>  
>  static struct attribute *pwm_attrs[] = {
>  	&dev_attr_period.attr,
> @@ -227,6 +276,7 @@ static ssize_t capture_show(struct device *child,
>  	&dev_attr_enable.attr,
>  	&dev_attr_polarity.attr,
>  	&dev_attr_capture.attr,
> +	&dev_attr_output_type.attr,
>  	NULL
>  };
>  ATTRIBUTE_GROUPS(pwm);
> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> index 24632a7..416f08e 100644
> --- a/include/linux/pwm.h
> +++ b/include/linux/pwm.h
> @@ -48,6 +48,29 @@ enum {
>  	PWMF_EXPORTED = 1 << 1,
>  };
>  
> +/**
> + * enum pwm_output_type - output type of the PWM signal
> + * @PWM_OUTPUT_FIXED: PWM output is fixed until a change request
> + * @PWM_OUTPUT_MODULATED: PWM output is modulated in hardware
> + * autonomously with a predefined pattern
> + */
> +enum pwm_output_type {
> +	PWM_OUTPUT_FIXED = 1 << 0,
> +	PWM_OUTPUT_MODULATED = 1 << 1,
> +};
> +
> +/**
> + * struct pwm_output_pattern - PWM duty pattern for MODULATED duty type
> + * @duty_pattern: PWM duty cycles in the pattern for duty modulation
> + * @num_entries: number of entries in the pattern
> + * @cycles_per_duty: number of PWM period cycles an entry stays at
> + */
> +struct pwm_output_pattern {
> +	unsigned int *duty_pattern;
> +	unsigned int num_entries;
> +	unsigned int cycles_per_duty;
> +};
> +
>  /*
>   * struct pwm_state - state of a PWM channel
>   * @period: PWM period (in nanoseconds)
> @@ -59,6 +82,8 @@ struct pwm_state {
>  	unsigned int period;
>  	unsigned int duty_cycle;
>  	enum pwm_polarity polarity;
> +	enum pwm_output_type output_type;
> +	struct pwm_output_pattern *output_pattern;
>  	bool enabled;
>  };
>  
> @@ -144,6 +169,26 @@ static inline enum pwm_polarity pwm_get_polarity(const struct pwm_device *pwm)
>  	return state.polarity;
>  }
>  
> +static inline enum pwm_output_type pwm_get_output_type(
> +		const struct pwm_device *pwm)
> +{
> +	struct pwm_state state;
> +
> +	pwm_get_state(pwm, &state);
> +
> +	return state.output_type;
> +}
> +
> +static inline struct pwm_output_pattern *pwm_get_output_pattern(
> +				struct pwm_device *pwm)
> +{
> +	struct pwm_state state;
> +
> +	pwm_get_state(pwm, &state);
> +
> +	return pwm->state.output_pattern ?: NULL;
> +}
> +
>  static inline void pwm_get_args(const struct pwm_device *pwm,
>  				struct pwm_args *args)
>  {
> @@ -250,6 +295,9 @@ static inline void pwm_init_state(const struct pwm_device *pwm,
>   * @get_state: get the current PWM state. This function is only
>   *	       called once per PWM device when the PWM chip is
>   *	       registered.
> + * @get_output_type_supported: get the supported output type
> + * @set_output_type: set PWM output type
> + * @set_output_pattern: set the pattern for the modulated output
>   * @owner: helps prevent removal of modules exporting active PWMs
>   * @config: configure duty cycles and period length for this PWM
>   * @set_polarity: configure the polarity of this PWM
> @@ -265,6 +313,13 @@ struct pwm_ops {
>  		     struct pwm_state *state);
>  	void (*get_state)(struct pwm_chip *chip, struct pwm_device *pwm,
>  			  struct pwm_state *state);
> +	int (*get_output_type_supported)(struct pwm_chip *chip,
> +			struct pwm_device *pwm);
> +	int (*set_output_type)(struct pwm_chip *chip, struct pwm_device *pwm,
> +			enum pwm_output_type output_type);
> +	int (*set_output_pattern)(struct pwm_chip *chip,
> +			struct pwm_device *pwm,
> +			struct pwm_output_pattern *output_pattern);
>  	struct module *owner;
>  
>  	/* Only used by legacy drivers */
> @@ -320,6 +375,21 @@ struct pwm_capture {
>  int pwm_adjust_config(struct pwm_device *pwm);
>  
>  /**
> + * pwm_output_type_support()
> + * @pwm: PWM device
> + *
> + * Returns:  output types supported by the PWM device
> + */
> +static inline int pwm_get_output_type_supported(struct pwm_device *pwm)
> +{
> +	if (pwm->chip->ops->get_output_type_supported != NULL)
> +		return pwm->chip->ops->get_output_type_supported(pwm->chip,
> +				pwm);
> +	else
> +		return PWM_OUTPUT_FIXED;
> +}
> +
> +/**
>   * pwm_config() - change a PWM device configuration
>   * @pwm: PWM device
>   * @duty_ns: "on" time (in nanoseconds)
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

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

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

* Re: [PATCH 2/2] pwm: core: Add option to config PWM duty/period with u64 data length
  2019-09-13 22:57 ` [PATCH 2/2] pwm: core: Add option to config PWM duty/period with u64 data length Guru Das Srinagesh
  2019-09-16 13:59   ` kbuild test robot
  2019-09-16 14:00   ` Thierry Reding
@ 2019-09-16 14:07   ` kbuild test robot
  2 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2019-09-16 14:07 UTC (permalink / raw)
  To: Guru Das Srinagesh
  Cc: kbuild-all, linux-pwm, Thierry Reding, kernel-team, Mark Salyzyn,
	Sandeep Patil, Subbaraman Narayanamurthy, linux-kernel,
	Fenglin Wu, Guru Das Srinagesh

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

Hi Guru,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.3 next-20190915]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Guru-Das-Srinagesh/pwm-Add-different-PWM-output-types-support/20190916-151008
config: arm-multi_v4t_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/pwm/pwm-clps711x.o: In function `clps711x_pwm_config':
>> pwm-clps711x.c:(.text+0x64): undefined reference to `__aeabi_uldivmod'
   drivers/pwm/pwm-clps711x.o: In function `clps711x_pwm_enable':
   pwm-clps711x.c:(.text+0x168): undefined reference to `__aeabi_uldivmod'
   drivers/video/backlight/pwm_bl.o: In function `pwm_backlight_probe':
>> pwm_bl.c:(.text+0x5fc): undefined reference to `__aeabi_uldivmod'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 13245 bytes --]

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

* Re: [PATCH 1/2] pwm: Add different PWM output types support
  2019-09-13 22:57 [PATCH 1/2] pwm: Add different PWM output types support Guru Das Srinagesh
  2019-09-13 22:57 ` [PATCH 2/2] pwm: core: Add option to config PWM duty/period with u64 data length Guru Das Srinagesh
  2019-09-16 14:01 ` [PATCH 1/2] pwm: Add different PWM output types support Thierry Reding
@ 2019-09-16 18:25 ` Uwe Kleine-König
  2019-09-24  6:01   ` Guru Das Srinagesh
  2021-09-18 11:18 ` Greg KH
  3 siblings, 1 reply; 12+ messages in thread
From: Uwe Kleine-König @ 2019-09-16 18:25 UTC (permalink / raw)
  To: Guru Das Srinagesh
  Cc: linux-pwm, Thierry Reding, kernel-team, Mark Salyzyn,
	Sandeep Patil, Subbaraman Narayanamurthy, linux-kernel,
	Fenglin Wu

Hello,

On Fri, Sep 13, 2019 at 03:57:43PM -0700, Guru Das Srinagesh wrote:
> From: Fenglin Wu <fenglinw@codeaurora.org>
> 
> Normally, PWM channel has fixed output until software request to change
> its settings. There are some PWM devices which their outputs could be
> changed autonomously according to a predefined pattern programmed in
> hardware. Add pwm_output_type enum type to identify these two different
> PWM types and add relevant helper functions to set and get PWM output
> types and pattern.

I have problems to understand what your modulated mode does even after
reading your commit log and the patch.

Also you should note here what is the intended usage and add support for
it for at least one (preferably more) drivers to make this actually
usable.

> Change-Id: Ia1f914a45ab4f4dd7be037a395eeb89d0e65a80e

In Linux we don't use these. You're making it easier to apply your patch
if you drop the change-id lines.

> diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
> index 2389b86..ab703f2 100644
> --- a/drivers/pwm/sysfs.c
> +++ b/drivers/pwm/sysfs.c
> @@ -215,11 +215,60 @@ static ssize_t capture_show(struct device *child,
>  	return sprintf(buf, "%u %u\n", result.period, result.duty_cycle);
>  }
>  
> +static ssize_t output_type_show(struct device *child,
> +			     struct device_attribute *attr,
> +			     char *buf)
> +{
> +	const struct pwm_device *pwm = child_to_pwm_device(child);
> +	const char *output_type = "unknown";
> +	struct pwm_state state;
> +
> +	pwm_get_state(pwm, &state);
> +	switch (state.output_type) {
> +	case PWM_OUTPUT_FIXED:
> +		output_type = "fixed";
> +		break;
> +	case PWM_OUTPUT_MODULATED:
> +		output_type = "modulated";
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return snprintf(buf, PAGE_SIZE, "%s\n", output_type);
> +}
> +
> +static ssize_t output_type_store(struct device *child,
> +			      struct device_attribute *attr,
> +			      const char *buf, size_t size)
> +{
> +	struct pwm_export *export = child_to_pwm_export(child);
> +	struct pwm_device *pwm = export->pwm;
> +	struct pwm_state state;
> +	int ret = -EINVAL;
> +
> +	mutex_lock(&export->lock);
> +	pwm_get_state(pwm, &state);
> +	if (sysfs_streq(buf, "fixed"))
> +		state.output_type = PWM_OUTPUT_FIXED;
> +	else if (sysfs_streq(buf, "modulated"))
> +		state.output_type = PWM_OUTPUT_MODULATED;
> +	else
> +		goto unlock;
> +
> +	ret = pwm_apply_state(pwm, &state);
> +unlock:
> +	mutex_unlock(&export->lock);
> +
> +	return ret ? : size;
> +}

So in sysfs you cannot set a pattern. Doesn't that mean this makes using
modulated mode hardly useful?

Best regards
Uwe

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

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

* Re: [PATCH 1/2] pwm: Add different PWM output types support
  2019-09-16 14:01 ` [PATCH 1/2] pwm: Add different PWM output types support Thierry Reding
@ 2019-09-24  5:43   ` Guru Das Srinagesh
  2019-09-24  6:39     ` Uwe Kleine-König
  0 siblings, 1 reply; 12+ messages in thread
From: Guru Das Srinagesh @ 2019-09-24  5:43 UTC (permalink / raw)
  To: Thierry Reding
  Cc: linux-pwm, kernel-team, Mark Salyzyn, Sandeep Patil,
	Subbaraman Narayanamurthy, linux-kernel, Fenglin Wu

On Mon, Sep 16, 2019 at 04:01:46PM +0200, Thierry Reding wrote:
> On Fri, Sep 13, 2019 at 03:57:43PM -0700, Guru Das Srinagesh wrote:
> > From: Fenglin Wu <fenglinw@codeaurora.org>
> > 
> > Normally, PWM channel has fixed output until software request to change
> > its settings. There are some PWM devices which their outputs could be
> > changed autonomously according to a predefined pattern programmed in
> > hardware. Add pwm_output_type enum type to identify these two different
> > PWM types and add relevant helper functions to set and get PWM output
> > types and pattern.
> > 
> > Change-Id: Ia1f914a45ab4f4dd7be037a395eeb89d0e65a80e
> > Signed-off-by: Fenglin Wu <fenglinw@codeaurora.org>
> > Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
> > ---
> >  drivers/pwm/core.c  | 26 ++++++++++++++++++++
> >  drivers/pwm/sysfs.c | 50 ++++++++++++++++++++++++++++++++++++++
> >  include/linux/pwm.h | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 146 insertions(+)
> 
> This doesn't seem right to me. Are you describing a PWM pin that's
> actually driven in GPIO mode? We usually configure that using pinctrl.
> 
> Thierry

Sorry, let me clarify.

Some Qualcomm PMICs have a PWM block called the Light Pulse Generator (LPG).
This block allows for the generation of a HW-controlled PWM "pattern", i.e. a
sequential altering of duty cycle, in addition to the normal PWM "fixed" duty
cycle operation, which is what the framework does currently. This pattern is
user-configurable in the form of a look-up table in the devicetree. The LPG's
registers have to be configured with the data in the look up table in order to
start the generation of the pattern. An example of a pattern is the "breath"
pattern, which simply ramps up the duty cycle and then ramps it down.

This "pattern" mode is what has been defined as PWM_OUTPUT_MODULATED in this
patch. I see that the use of the term "modulated" is misleading - a more
accurate term would be PWM_OUTPUT_PATTERN perhaps.

This patch merely adds framework support to differentiate between the "fixed"
and "pattern" modes of operation. Actions such as configuring the LPG with the
devicetree pattern and setting it up for generating the pattern are performed
in the driver only if the output type is read as "pattern" and not otherwise.

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

* Re: [PATCH 1/2] pwm: Add different PWM output types support
  2019-09-16 18:25 ` Uwe Kleine-König
@ 2019-09-24  6:01   ` Guru Das Srinagesh
  0 siblings, 0 replies; 12+ messages in thread
From: Guru Das Srinagesh @ 2019-09-24  6:01 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: linux-pwm, Thierry Reding, kernel-team, Mark Salyzyn,
	Sandeep Patil, Subbaraman Narayanamurthy, linux-kernel,
	Fenglin Wu

On Mon, Sep 16, 2019 at 08:25:24PM +0200, Uwe Kleine-König wrote:
> Hello,
> 
> On Fri, Sep 13, 2019 at 03:57:43PM -0700, Guru Das Srinagesh wrote:
> > From: Fenglin Wu <fenglinw@codeaurora.org>
> > 
> > Normally, PWM channel has fixed output until software request to change
> > its settings. There are some PWM devices which their outputs could be
> > changed autonomously according to a predefined pattern programmed in
> > hardware. Add pwm_output_type enum type to identify these two different
> > PWM types and add relevant helper functions to set and get PWM output
> > types and pattern.
> 
> I have problems to understand what your modulated mode does even after
> reading your commit log and the patch.

Hi Uwe,

I have posted a reply to Thierry's email in which I provide some
background and details about this patch for your review. Hopefully that
clarifies things - I can provide some more clarifications if necessary.

> 
> Also you should note here what is the intended usage and add support for
> it for at least one (preferably more) drivers to make this actually
> usable.

The PWM_OUTPUT_MODULATED mode is useful for PWM peripherals like
Qualcomm's Light Pulse Generator (LPG) that can output a duty-cycle
pattern in hardware as well as output a fixed duty-cycle signal. All
this mode really does is to provide a way for drivers to carry out some
actions for the "pattern" mode of operation that are not required (or
relevant) for the "fixed" mode of operation.

> 
> > Change-Id: Ia1f914a45ab4f4dd7be037a395eeb89d0e65a80e
> 
> In Linux we don't use these. You're making it easier to apply your patch
> if you drop the change-id lines.

Sorry, forgot to strip these before sending out the patch.

> 
> > diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
> > index 2389b86..ab703f2 100644
> > --- a/drivers/pwm/sysfs.c
> > +++ b/drivers/pwm/sysfs.c
> > @@ -215,11 +215,60 @@ static ssize_t capture_show(struct device *child,
> >  	return sprintf(buf, "%u %u\n", result.period, result.duty_cycle);
> >  }
> >  
> > +static ssize_t output_type_show(struct device *child,
> > +			     struct device_attribute *attr,
> > +			     char *buf)
> > +{
> > +	const struct pwm_device *pwm = child_to_pwm_device(child);
> > +	const char *output_type = "unknown";
> > +	struct pwm_state state;
> > +
> > +	pwm_get_state(pwm, &state);
> > +	switch (state.output_type) {
> > +	case PWM_OUTPUT_FIXED:
> > +		output_type = "fixed";
> > +		break;
> > +	case PWM_OUTPUT_MODULATED:
> > +		output_type = "modulated";
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +
> > +	return snprintf(buf, PAGE_SIZE, "%s\n", output_type);
> > +}
> > +
> > +static ssize_t output_type_store(struct device *child,
> > +			      struct device_attribute *attr,
> > +			      const char *buf, size_t size)
> > +{
> > +	struct pwm_export *export = child_to_pwm_export(child);
> > +	struct pwm_device *pwm = export->pwm;
> > +	struct pwm_state state;
> > +	int ret = -EINVAL;
> > +
> > +	mutex_lock(&export->lock);
> > +	pwm_get_state(pwm, &state);
> > +	if (sysfs_streq(buf, "fixed"))
> > +		state.output_type = PWM_OUTPUT_FIXED;
> > +	else if (sysfs_streq(buf, "modulated"))
> > +		state.output_type = PWM_OUTPUT_MODULATED;
> > +	else
> > +		goto unlock;
> > +
> > +	ret = pwm_apply_state(pwm, &state);
> > +unlock:
> > +	mutex_unlock(&export->lock);
> > +
> > +	return ret ? : size;
> > +}
> 
> So in sysfs you cannot set a pattern. Doesn't that mean this makes using
> modulated mode hardly useful?

The pattern has to be set through the devicetree and not sysfs. Will
change the output type sysfs parameter to read-only.

> 
> Best regards
> Uwe
> 
> -- 
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

* Re: [PATCH 1/2] pwm: Add different PWM output types support
  2019-09-24  5:43   ` Guru Das Srinagesh
@ 2019-09-24  6:39     ` Uwe Kleine-König
  2019-09-24 10:46       ` Thierry Reding
  0 siblings, 1 reply; 12+ messages in thread
From: Uwe Kleine-König @ 2019-09-24  6:39 UTC (permalink / raw)
  To: Guru Das Srinagesh
  Cc: Thierry Reding, linux-pwm, kernel-team, Mark Salyzyn,
	Sandeep Patil, Subbaraman Narayanamurthy, linux-kernel,
	Fenglin Wu

On Mon, Sep 23, 2019 at 10:43:43PM -0700, Guru Das Srinagesh wrote:
> On Mon, Sep 16, 2019 at 04:01:46PM +0200, Thierry Reding wrote:
> > On Fri, Sep 13, 2019 at 03:57:43PM -0700, Guru Das Srinagesh wrote:
> > > From: Fenglin Wu <fenglinw@codeaurora.org>
> > > 
> > > Normally, PWM channel has fixed output until software request to change
> > > its settings. There are some PWM devices which their outputs could be
> > > changed autonomously according to a predefined pattern programmed in
> > > hardware. Add pwm_output_type enum type to identify these two different
> > > PWM types and add relevant helper functions to set and get PWM output
> > > types and pattern.
> > > 
> > > Change-Id: Ia1f914a45ab4f4dd7be037a395eeb89d0e65a80e
> > > Signed-off-by: Fenglin Wu <fenglinw@codeaurora.org>
> > > Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
> > > ---
> > >  drivers/pwm/core.c  | 26 ++++++++++++++++++++
> > >  drivers/pwm/sysfs.c | 50 ++++++++++++++++++++++++++++++++++++++
> > >  include/linux/pwm.h | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 146 insertions(+)
> > 
> > This doesn't seem right to me. Are you describing a PWM pin that's
> > actually driven in GPIO mode? We usually configure that using pinctrl.
> > 
> > Thierry
> 
> Sorry, let me clarify.
> 
> Some Qualcomm PMICs have a PWM block called the Light Pulse Generator (LPG).
> This block allows for the generation of a HW-controlled PWM "pattern", i.e. a
> sequential altering of duty cycle, in addition to the normal PWM "fixed" duty
> cycle operation, which is what the framework does currently. This pattern is
> user-configurable in the form of a look-up table in the devicetree. The LPG's
> registers have to be configured with the data in the look up table in order to
> start the generation of the pattern. An example of a pattern is the "breath"
> pattern, which simply ramps up the duty cycle and then ramps it down.

I'll try to describe it in my words to check if I got it right: So the
mode you want to add needs a sequence of PWM states and the hardware is
expected to apply them in turn, each for a configurable count of
periods. If I understand this right, this is expected to be cyclic?

> This "pattern" mode is what has been defined as PWM_OUTPUT_MODULATED in this
> patch. I see that the use of the term "modulated" is misleading - a more
> accurate term would be PWM_OUTPUT_PATTERN perhaps.

Not sure "pattern" is better. 

The PWM on the newer imx SoCs (using the imx27 driver) has a FIFO with
length 4 that allows to program changing settings. Only the duty cycle
can be modified and as repeat count only 1, 2, 4 and 8 are available. I
assume the FIFO can be fed by the dma engine.

Note I only know this feature from reading the reference manual and
never used it.

> This patch merely adds framework support to differentiate between the "fixed"
> and "pattern" modes of operation. Actions such as configuring the LPG with the
> devicetree pattern and setting it up for generating the pattern are performed
> in the driver only if the output type is read as "pattern" and not otherwise.

Up to now I'm not convinced that this extension is a good one that can
be supported by several PWM implementations. I'd say we should collect
first some details about different implementations and what these could
implement to get a picture what kind of API is sensible.

Best regards
Uwe

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

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

* Re: [PATCH 1/2] pwm: Add different PWM output types support
  2019-09-24  6:39     ` Uwe Kleine-König
@ 2019-09-24 10:46       ` Thierry Reding
  0 siblings, 0 replies; 12+ messages in thread
From: Thierry Reding @ 2019-09-24 10:46 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Guru Das Srinagesh, linux-pwm, kernel-team, Mark Salyzyn,
	Sandeep Patil, Subbaraman Narayanamurthy, linux-kernel,
	Fenglin Wu

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

On Tue, Sep 24, 2019 at 08:39:26AM +0200, Uwe Kleine-König wrote:
> On Mon, Sep 23, 2019 at 10:43:43PM -0700, Guru Das Srinagesh wrote:
> > On Mon, Sep 16, 2019 at 04:01:46PM +0200, Thierry Reding wrote:
> > > On Fri, Sep 13, 2019 at 03:57:43PM -0700, Guru Das Srinagesh wrote:
> > > > From: Fenglin Wu <fenglinw@codeaurora.org>
> > > > 
> > > > Normally, PWM channel has fixed output until software request to change
> > > > its settings. There are some PWM devices which their outputs could be
> > > > changed autonomously according to a predefined pattern programmed in
> > > > hardware. Add pwm_output_type enum type to identify these two different
> > > > PWM types and add relevant helper functions to set and get PWM output
> > > > types and pattern.
> > > > 
> > > > Change-Id: Ia1f914a45ab4f4dd7be037a395eeb89d0e65a80e
> > > > Signed-off-by: Fenglin Wu <fenglinw@codeaurora.org>
> > > > Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
> > > > ---
> > > >  drivers/pwm/core.c  | 26 ++++++++++++++++++++
> > > >  drivers/pwm/sysfs.c | 50 ++++++++++++++++++++++++++++++++++++++
> > > >  include/linux/pwm.h | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  3 files changed, 146 insertions(+)
> > > 
> > > This doesn't seem right to me. Are you describing a PWM pin that's
> > > actually driven in GPIO mode? We usually configure that using pinctrl.
> > > 
> > > Thierry
> > 
> > Sorry, let me clarify.
> > 
> > Some Qualcomm PMICs have a PWM block called the Light Pulse Generator (LPG).
> > This block allows for the generation of a HW-controlled PWM "pattern", i.e. a
> > sequential altering of duty cycle, in addition to the normal PWM "fixed" duty
> > cycle operation, which is what the framework does currently. This pattern is
> > user-configurable in the form of a look-up table in the devicetree. The LPG's
> > registers have to be configured with the data in the look up table in order to
> > start the generation of the pattern. An example of a pattern is the "breath"
> > pattern, which simply ramps up the duty cycle and then ramps it down.
> 
> I'll try to describe it in my words to check if I got it right: So the
> mode you want to add needs a sequence of PWM states and the hardware is
> expected to apply them in turn, each for a configurable count of
> periods. If I understand this right, this is expected to be cyclic?
> 
> > This "pattern" mode is what has been defined as PWM_OUTPUT_MODULATED in this
> > patch. I see that the use of the term "modulated" is misleading - a more
> > accurate term would be PWM_OUTPUT_PATTERN perhaps.
> 
> Not sure "pattern" is better. 
> 
> The PWM on the newer imx SoCs (using the imx27 driver) has a FIFO with
> length 4 that allows to program changing settings. Only the duty cycle
> can be modified and as repeat count only 1, 2, 4 and 8 are available. I
> assume the FIFO can be fed by the dma engine.
> 
> Note I only know this feature from reading the reference manual and
> never used it.
> 
> > This patch merely adds framework support to differentiate between the "fixed"
> > and "pattern" modes of operation. Actions such as configuring the LPG with the
> > devicetree pattern and setting it up for generating the pattern are performed
> > in the driver only if the output type is read as "pattern" and not otherwise.
> 
> Up to now I'm not convinced that this extension is a good one that can
> be supported by several PWM implementations. I'd say we should collect
> first some details about different implementations and what these could
> implement to get a picture what kind of API is sensible.

I agree. This sounds to me like it's stretching the concept of a PWM a
bit too much. Sounds like drivers/leds might be a better fit for this.

Thierry

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

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

* Re: [PATCH 1/2] pwm: Add different PWM output types support
  2019-09-13 22:57 [PATCH 1/2] pwm: Add different PWM output types support Guru Das Srinagesh
                   ` (2 preceding siblings ...)
  2019-09-16 18:25 ` Uwe Kleine-König
@ 2021-09-18 11:18 ` Greg KH
  3 siblings, 0 replies; 12+ messages in thread
From: Greg KH @ 2021-09-18 11:18 UTC (permalink / raw)
  To: Guru Das Srinagesh
  Cc: linux-pwm, Thierry Reding, kernel-team, Mark Salyzyn,
	Sandeep Patil, Subbaraman Narayanamurthy, linux-kernel,
	Fenglin Wu

On Fri, Sep 13, 2019 at 03:57:43PM -0700, Guru Das Srinagesh wrote:
> From: Fenglin Wu <fenglinw@codeaurora.org>
> 
> Normally, PWM channel has fixed output until software request to change
> its settings. There are some PWM devices which their outputs could be
> changed autonomously according to a predefined pattern programmed in
> hardware. Add pwm_output_type enum type to identify these two different
> PWM types and add relevant helper functions to set and get PWM output
> types and pattern.
> 
> Change-Id: Ia1f914a45ab4f4dd7be037a395eeb89d0e65a80e

This is not needed an a upstream patch, checkpatch should have told you
:(

> Signed-off-by: Fenglin Wu <fenglinw@codeaurora.org>
> Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
> ---
>  drivers/pwm/core.c  | 26 ++++++++++++++++++++
>  drivers/pwm/sysfs.c | 50 ++++++++++++++++++++++++++++++++++++++

You forgot a Documentation/ABI/ update for your new sysfs file :(

thanks,

greg k-h

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

end of thread, other threads:[~2021-09-18 11:18 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-13 22:57 [PATCH 1/2] pwm: Add different PWM output types support Guru Das Srinagesh
2019-09-13 22:57 ` [PATCH 2/2] pwm: core: Add option to config PWM duty/period with u64 data length Guru Das Srinagesh
2019-09-16 13:59   ` kbuild test robot
2019-09-16 14:00   ` Thierry Reding
2019-09-16 14:07   ` kbuild test robot
2019-09-16 14:01 ` [PATCH 1/2] pwm: Add different PWM output types support Thierry Reding
2019-09-24  5:43   ` Guru Das Srinagesh
2019-09-24  6:39     ` Uwe Kleine-König
2019-09-24 10:46       ` Thierry Reding
2019-09-16 18:25 ` Uwe Kleine-König
2019-09-24  6:01   ` Guru Das Srinagesh
2021-09-18 11:18 ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).