[PATCH v3 00/10] extend PWM framework to support PWM modes

[PATCH v3 00/10] extend PWM framework to support PWM modes

[Claudiu Beznea]

Hi all,

Please give feedback on these patches which extends the PWM framework in
order to support multiple PWM modes of operations. This series is a rework
of [1] and [2].

The current patch series add the following PWM modes:
- PWM mode normal
- PWM mode complementary
- PWM mode push-pull

Normal mode - for PWM channels with one output; output waveforms looks like
this:
             __    __    __    __
    PWM   __|  |__|  |__|  |__|  |__
            <--T-->

    Where T is the signal period

Since PWMs with more than one output per channel could be used as one
output PWM the normal mode is the default mode for all PWMs (if not
specified otherwise).

Complementary mode - for PWM channels with two outputs; output waveforms
for a PWM channel in complementary mode looks line this:
             __    __    __    __
    PWMH1 __|  |__|  |__|  |__|  |__
          __    __    __    __    __
    PWML1   |__|  |__|  |__|  |__|
            <--T-->

    Where T is the signal period.

Push-pull mode - for PWM channels with two outputs; output waveforms for a
PWM channel in push-pull mode with normal polarity looks like this:
            __          __
    PWMH __|  |________|  |________
                  __          __
    PWML ________|  |________|  |__
           <--T-->

    If polarity is inversed:
         __    ________    ________
    PWMH   |__|        |__|
         ________    ________    __
    PWML         |__|        |__|
           <--T-->

    Where T is the signal period.

The PWM working modes are per PWM channel registered as PWM's capabilities.
The driver registers itself to PWM core a get_caps() function, in
struct pwm_ops, that will be used by PWM core to retrieve PWM capabilities.
If this function is not registered in driver's probe, a default function
will be used to retrieve PWM capabilities. Currently, the default
capabilities includes only PWM normal mode.

PWM state has been updated to keep PWM mode. PWM mode could be configured
via sysfs or via DT. pwm_apply_state() will do the preliminary validation
for PWM mode to be applied.

In sysfs, user could get PWM modes by reading mode file of PWM device:
root@sama5d2-xplained:/sys/class/pwm/pwmchip0/pwm2# ls -l
total 0
-r--r--r-- 1 root root 4096 Oct  9 09:07 capture
lrwxrwxrwx 1 root root    0 Oct  9 09:07 device -> ../../pwmchip0
-rw-r--r-- 1 root root 4096 Oct  9 08:42 duty_cycle
-rw-r--r-- 1 root root 4096 Oct  9 08:44 enable
--w------- 1 root root 4096 Oct  9 09:07 export
-rw-r--r-- 1 root root 4096 Oct  9 08:43 mode
-r--r--r-- 1 root root 4096 Oct  9 09:07 npwm
-rw-r--r-- 1 root root 4096 Oct  9 08:42 period
-rw-r--r-- 1 root root 4096 Oct  9 08:44 polarity
drwxr-xr-x 2 root root    0 Oct  9 09:07 power
lrwxrwxrwx 1 root root    0 Oct  9 09:07 subsystem -> ../../../../../../../../class/pwm
-rw-r--r-- 1 root root 4096 Oct  9 08:42 uevent
--w------- 1 root root 4096 Oct  9 09:07 unexport
root@sama5d2-xplained:/sys/class/pwm/pwmchip0/pwm2# cat mode
normal complementary [push-pull]

The mode enclosed in bracket is the currently active mode.

The mode could be set, via sysfs, by writing to mode file one of the modes
displayed at read:
root@sama5d2-xplained:/sys/class/pwm/pwmchip0/pwm2# echo normal > mode
root@sama5d2-xplained:/sys/class/pwm/pwmchip0/pwm2# cat mode
[normal] complementary push-pull 

The PWM push-pull mode could be usefull in applications like half bridge
converters.

This series also add support for PWM modes on Atmel/Microchip SoCs.

Thank you,
Claudiu Beznea

[1] https://www.spinics.net/lists/arm-kernel/msg580275.html
[2] https://lkml.org/lkml/2018/1/12/359

Changes in v3:
- removed changes related to only one of_xlate function for all PWM drivers
- switch to PWM capabilities per PWM channel nor per PWM chip
- squash documentation and bindings patches as requeted by reviewer
- introduced PWM_MODE(name) macro and used a bit enum for pwm modes
- related to DT bindings, used flags cell also for PWM modes
- updated of_xlate specific functions with "state->mode = mode;"
  instructions to avoid pwm_apply_state() failures
- use available modes for PWM channel in pwm_config() by first calling
  pwm_get_caps() to get caps.modes
- use loops through available modes in mode_store()/mode_show() and also in
  of_pwm_xlate_with_flags() instead of "if else" instructions; in this way,
  the addition of a new mode is independent of this code sections
- use DTLI=1, DTHI=0 register settings to obtain push-pull mode waveforms
  for Atmel/Microchip PWM controller.

Changes in v2:
- remove of_xlate and of_pwm_n_cells and use generic functions to pharse DT
  inputs; this is done in patches 1, 2, 3, 4, 5, 6, 7 of this series; this will
  make easy the addition of PWM mode support from DT
- add PWM mode normal
- register PWM modes as capabilities of PWM chips at driver probe and, in case
  driver doesn't provide these capabilities use default ones
- change the way PWM mode is pharsed via DT by using a new input for pwms
  binding property


Claudiu Beznea (10):
  pwm: extend PWM framework with PWM modes
  pwm: clps711x: populate PWM mode in of_xlate function
  pwm: cros-ec: populate PWM mode in of_xlate function
  pwm: pxa: populate PWM mode in of_xlate function
  pwm: add PWM mode to pwm_config()
  pwm: add PWM modes
  pwm: atmel: add pwm capabilities
  pwm: add push-pull mode support
  pwm: add documentation for pwm push-pull mode
  pwm: atmel: add push-pull mode support

 Documentation/devicetree/bindings/pwm/pwm.txt |  11 ++-
 Documentation/pwm.txt                         |  42 ++++++++-
 arch/arm/mach-s3c24xx/mach-rx1950.c           |  11 ++-
 drivers/bus/ts-nbus.c                         |   2 +-
 drivers/clk/clk-pwm.c                         |   3 +-
 drivers/gpu/drm/i915/intel_panel.c            |  17 +++-
 drivers/hwmon/pwm-fan.c                       |   2 +-
 drivers/input/misc/max77693-haptic.c          |   2 +-
 drivers/input/misc/max8997_haptic.c           |   6 +-
 drivers/leds/leds-pwm.c                       |   5 +-
 drivers/media/rc/ir-rx51.c                    |   5 +-
 drivers/media/rc/pwm-ir-tx.c                  |   5 +-
 drivers/pwm/core.c                            |  98 ++++++++++++++++++++-
 drivers/pwm/pwm-atmel.c                       | 118 +++++++++++++++++++-------
 drivers/pwm/pwm-clps711x.c                    |  12 ++-
 drivers/pwm/pwm-cros-ec.c                     |   4 +
 drivers/pwm/pwm-pxa.c                         |   4 +
 drivers/pwm/sysfs.c                           |  56 ++++++++++++
 drivers/video/backlight/lm3630a_bl.c          |   4 +-
 drivers/video/backlight/lp855x_bl.c           |   4 +-
 drivers/video/backlight/lp8788_bl.c           |   5 +-
 drivers/video/backlight/pwm_bl.c              |  11 ++-
 drivers/video/fbdev/ssd1307fb.c               |   3 +-
 include/dt-bindings/pwm/pwm.h                 |   2 +
 include/linux/pwm.h                           | 100 +++++++++++++++++++++-
 25 files changed, 470 insertions(+), 62 deletions(-)

-- 
2.7.4

[PATCH v3 01/10] pwm: extend PWM framework with PWM modes

[Claudiu Beznea]

Add basic PWM modes: normal and complementary. These modes should
differentiate the single output PWM channels from two outputs PWM
channels. These modes could be set as follow:
1. PWM channels with one output per channel:
- normal mode
2. PWM channels with two outputs per channel:
- normal mode
- complementary mode
Since users could use a PWM channel with two output as one output PWM
channel, the PWM normal mode is allowed to be set for PWM channels with
two outputs; in fact PWM normal mode should be supported by all PWMs.

The PWM capabilities were implemented per PWM channel. Every PWM controller
will register a function to get PWM capabilities. If this is not explicitly
set by the driver a default function will be used to retrieve the the PWM
capabilities (in this case the PWM capabilities will contain only PWM
normal mode). This function is set in pwmchip_add_with_polarity() as a
member of "struct pwm_chip". To retrieve capabilities the pwm_get_caps()
function could be used.

Every PWM channel will have associated a mode in the PWM state. Proper
helper functions were added to get/set PWM mode. The mode could also be set
from DT via flag cells. The valid DT modes could be located in
include/dt-bindings/pwm/pwm.h. Only modes supported by PWM channel could be
set. If nothing is specified for a PWM channel, via DT, the first available
mode will be used (normally, this will be PWM normal mode).

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/pwm/core.c  | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 drivers/pwm/sysfs.c | 56 ++++++++++++++++++++++++++++++
 include/linux/pwm.h | 87 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 237 insertions(+), 4 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 1581f6ab1b1f..16a409d452c0 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -136,6 +136,8 @@ struct pwm_device *
 of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args)
 {
 	struct pwm_device *pwm;
+	struct pwm_caps caps;
+	int modebit;
 
 	/* check, whether the driver supports a third cell for flags */
 	if (pc->of_pwm_n_cells < 3)
@@ -152,11 +154,23 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args)
 	if (IS_ERR(pwm))
 		return pwm;
 
+	pwm_get_caps(pc, pwm, &caps);
+
 	pwm->args.period = args->args[1];
 	pwm->args.polarity = PWM_POLARITY_NORMAL;
+	pwm->args.mode = BIT(ffs(caps.modes) - 1);
+
+	if (args->args_count > 2) {
+		if (args->args[2] & PWM_POLARITY_INVERTED)
+			pwm->args.polarity = PWM_POLARITY_INVERSED;
 
-	if (args->args_count > 2 && args->args[2] & PWM_POLARITY_INVERTED)
-		pwm->args.polarity = PWM_POLARITY_INVERSED;
+		for (modebit = PWM_MODE_COMPLEMENTARY_BIT;
+		     modebit < PWM_MODE_CNT; modebit++)
+			if (args->args[2] & BIT(modebit)) {
+				pwm->args.mode = BIT(modebit);
+				break;
+			}
+	}
 
 	return pwm;
 }
@@ -166,6 +180,7 @@ static struct pwm_device *
 of_pwm_simple_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
 {
 	struct pwm_device *pwm;
+	struct pwm_caps caps;
 
 	/* sanity check driver support */
 	if (pc->of_pwm_n_cells < 2)
@@ -182,7 +197,9 @@ of_pwm_simple_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
 	if (IS_ERR(pwm))
 		return pwm;
 
+	pwm_get_caps(pc, pwm, &caps);
 	pwm->args.period = args->args[1];
+	pwm->args.mode = BIT(ffs(caps.modes) - 1);
 
 	return pwm;
 }
@@ -250,6 +267,39 @@ static bool pwm_ops_check(const struct pwm_ops *ops)
 }
 
 /**
+ * pwm_get_caps() - get PWM capabilities
+ * @chip: PWM chip
+ * @pwm: PWM device to get the capabilities for
+ * @caps: returned capabilities
+ *
+ * Retrievers capabilities for PWM device.
+ */
+void pwm_get_caps(struct pwm_chip *chip, struct pwm_device *pwm,
+		  struct pwm_caps *caps)
+{
+	if (!chip || !pwm || !caps)
+		return;
+
+	if (chip->ops && chip->ops->get_caps)
+		pwm->chip->ops->get_caps(chip, pwm, caps);
+	else if (chip->get_default_caps)
+		chip->get_default_caps(caps);
+}
+EXPORT_SYMBOL_GPL(pwm_get_caps);
+
+static void pwmchip_get_default_caps(struct pwm_caps *caps)
+{
+	static const struct pwm_caps default_caps = {
+		.modes = PWM_MODE(NORMAL),
+	};
+
+	if (!caps)
+		return;
+
+	*caps = default_caps;
+}
+
+/**
  * pwmchip_add_with_polarity() - register a new PWM chip
  * @chip: the PWM chip to add
  * @polarity: initial polarity of PWM channels
@@ -264,7 +314,8 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip,
 			      enum pwm_polarity polarity)
 {
 	struct pwm_device *pwm;
-	unsigned int i;
+	struct pwm_caps caps;
+	unsigned int i, j;
 	int ret;
 
 	if (!chip || !chip->dev || !chip->ops || !chip->npwm)
@@ -275,6 +326,8 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip,
 
 	mutex_lock(&pwm_lock);
 
+	chip->get_default_caps = pwmchip_get_default_caps;
+
 	ret = alloc_pwms(chip->base, chip->npwm);
 	if (ret < 0)
 		goto out;
@@ -295,6 +348,16 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip,
 		pwm->hwpwm = i;
 		pwm->state.polarity = polarity;
 
+		pwm_get_caps(chip, pwm, &caps);
+
+		/* Check if modes are supported. */
+		if (!pwm_caps_valid(caps)) {
+			ret = -EINVAL;
+			goto free;
+		}
+
+		pwm->state.mode = BIT(ffs(caps.modes) - 1);
+
 		if (chip->ops->get_state)
 			chip->ops->get_state(chip, pwm, &pwm->state);
 
@@ -316,6 +379,17 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip,
 out:
 	mutex_unlock(&pwm_lock);
 	return ret;
+
+free:
+	for (j = 0; j < i; j++) {
+		pwm = &chip->pwms[j];
+		radix_tree_delete(&pwm_tree, pwm->pwm);
+	}
+	kfree(chip->pwms);
+	chip->pwms = NULL;
+
+	mutex_unlock(&pwm_lock);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(pwmchip_add_with_polarity);
 
@@ -466,10 +540,17 @@ EXPORT_SYMBOL_GPL(pwm_free);
  */
 int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state)
 {
+	struct pwm_caps caps;
 	int err;
 
 	if (!pwm || !state || !state->period ||
-	    state->duty_cycle > state->period)
+	    state->duty_cycle > state->period ||
+	    !pwm_mode_valid(state->mode))
+		return -EINVAL;
+
+	/* Check if mode is supported by PWM. */
+	pwm_get_caps(pwm->chip, pwm, &caps);
+	if (!(caps.modes & state->mode))
 		return -EINVAL;
 
 	if (!memcmp(state, &pwm->state, sizeof(*state)))
@@ -530,6 +611,9 @@ int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state)
 
 			pwm->state.enabled = state->enabled;
 		}
+
+		/* No mode support for non-atomic PWM. */
+		pwm->state.mode = state->mode;
 	}
 
 	return 0;
@@ -579,6 +663,8 @@ int pwm_adjust_config(struct pwm_device *pwm)
 	pwm_get_args(pwm, &pargs);
 	pwm_get_state(pwm, &state);
 
+	state.mode = pargs.mode;
+
 	/*
 	 * If the current period is zero it means that either the PWM driver
 	 * does not support initial state retrieval or the PWM has not yet
@@ -767,6 +853,7 @@ struct pwm_device *pwm_get(struct device *dev, const char *con_id)
 	unsigned int best = 0;
 	struct pwm_lookup *p, *chosen = NULL;
 	unsigned int match;
+	struct pwm_caps caps;
 	int err;
 
 	/* look up via DT first */
@@ -848,8 +935,10 @@ struct pwm_device *pwm_get(struct device *dev, const char *con_id)
 	if (IS_ERR(pwm))
 		return pwm;
 
+	pwm_get_caps(chip, pwm, &caps);
 	pwm->args.period = chosen->period;
 	pwm->args.polarity = chosen->polarity;
+	pwm->args.mode = BIT(ffs(caps.modes) - 1);
 
 	return pwm;
 }
@@ -999,6 +1088,7 @@ static void pwm_dbg_show(struct pwm_chip *chip, struct seq_file *s)
 		seq_printf(s, " duty: %u ns", state.duty_cycle);
 		seq_printf(s, " polarity: %s",
 			   state.polarity ? "inverse" : "normal");
+		seq_printf(s, " mode: %s", pwm_mode_desc(state.mode));
 
 		seq_puts(s, "\n");
 	}
diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index 83f2b0b15712..d9eb0eb63d23 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -223,11 +223,66 @@ static ssize_t capture_show(struct device *child,
 	return sprintf(buf, "%u %u\n", result.period, result.duty_cycle);
 }
 
+static ssize_t mode_show(struct device *child,
+			 struct device_attribute *attr,
+			 char *buf)
+{
+	struct pwm_device *pwm = child_to_pwm_device(child);
+	struct pwm_state state;
+	struct pwm_caps caps;
+	int modebit, len = 0;
+
+	pwm_get_state(pwm, &state);
+	pwm_get_caps(pwm->chip, pwm, &caps);
+
+	for (modebit = PWM_MODE_NORMAL_BIT; modebit < PWM_MODE_CNT; modebit++)
+		if (caps.modes & BIT(modebit)) {
+			if (state.mode == BIT(modebit))
+				len += scnprintf(buf + len,
+						 PAGE_SIZE - len, "[%s] ",
+						 pwm_mode_desc(BIT(modebit)));
+			else
+				len += scnprintf(buf + len,
+						 PAGE_SIZE - len, "%s ",
+						 pwm_mode_desc(BIT(modebit)));
+		}
+
+	len += scnprintf(buf + len, PAGE_SIZE - len, "\n");
+
+	return len;
+}
+
+static ssize_t mode_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 modebit, ret;
+
+	for (modebit = PWM_MODE_NORMAL_BIT; modebit < PWM_MODE_CNT; modebit++)
+		if (sysfs_streq(buf, pwm_mode_desc(BIT(modebit))))
+			break;
+
+	if (modebit == PWM_MODE_CNT)
+		return -EINVAL;
+
+	mutex_lock(&export->lock);
+	pwm_get_state(pwm, &state);
+	state.mode = BIT(modebit);
+	ret = pwm_apply_state(pwm, &state);
+	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(mode);
 
 static struct attribute *pwm_attrs[] = {
 	&dev_attr_period.attr,
@@ -235,6 +290,7 @@ static struct attribute *pwm_attrs[] = {
 	&dev_attr_enable.attr,
 	&dev_attr_polarity.attr,
 	&dev_attr_capture.attr,
+	&dev_attr_mode.attr,
 	NULL
 };
 ATTRIBUTE_GROUPS(pwm);
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 56518adc31dd..e62349f48129 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -26,9 +26,32 @@ enum pwm_polarity {
 };
 
 /**
+ * PWM modes
+ * @PWM_MODE_NORMAL_BIT: PWM has one output
+ * @PWM_MODE_COMPLEMENTARY_BIT: PWM has 2 outputs with opposite polarities
+ * @PWM_MODE_CNT: PWM modes count
+ */
+enum {
+	PWM_MODE_NORMAL_BIT,
+	PWM_MODE_COMPLEMENTARY_BIT,
+	PWM_MODE_CNT,
+};
+
+#define PWM_MODE(name)		BIT(PWM_MODE_##name##_BIT)
+
+/**
+ * struct pwm_caps - PWM capabilities
+ * @modes: PWM modes
+ */
+struct pwm_caps {
+	unsigned long modes;
+};
+
+/**
  * struct pwm_args - board-dependent PWM arguments
  * @period: reference period
  * @polarity: reference polarity
+ * @mode: reference mode
  *
  * This structure describes board-dependent arguments attached to a PWM
  * device. These arguments are usually retrieved from the PWM lookup table or
@@ -41,6 +64,7 @@ enum pwm_polarity {
 struct pwm_args {
 	unsigned int period;
 	enum pwm_polarity polarity;
+	unsigned long mode;
 };
 
 enum {
@@ -53,12 +77,14 @@ enum {
  * @period: PWM period (in nanoseconds)
  * @duty_cycle: PWM duty cycle (in nanoseconds)
  * @polarity: PWM polarity
+ * @mode: PWM mode
  * @enabled: PWM enabled status
  */
 struct pwm_state {
 	unsigned int period;
 	unsigned int duty_cycle;
 	enum pwm_polarity polarity;
+	unsigned long mode;
 	bool enabled;
 };
 
@@ -181,6 +207,7 @@ static inline void pwm_init_state(const struct pwm_device *pwm,
 	state->period = args.period;
 	state->polarity = args.polarity;
 	state->duty_cycle = 0;
+	state->mode = args.mode;
 }
 
 /**
@@ -254,6 +281,7 @@ pwm_set_relative_duty_cycle(struct pwm_state *state, unsigned int duty_cycle,
  * @get_state: get the current PWM state. This function is only
  *	       called once per PWM device when the PWM chip is
  *	       registered.
+ * @get_caps: get PWM capabilities.
  * @dbg_show: optional routine to show contents in debugfs
  * @owner: helps prevent removal of modules exporting active PWMs
  */
@@ -272,6 +300,8 @@ struct pwm_ops {
 		     struct pwm_state *state);
 	void (*get_state)(struct pwm_chip *chip, struct pwm_device *pwm,
 			  struct pwm_state *state);
+	void (*get_caps)(struct pwm_chip *chip, struct pwm_device *pwm,
+			 struct pwm_caps *caps);
 #ifdef CONFIG_DEBUG_FS
 	void (*dbg_show)(struct pwm_chip *chip, struct seq_file *s);
 #endif
@@ -287,6 +317,7 @@ struct pwm_ops {
  * @npwm: number of PWMs controlled by this chip
  * @pwms: array of PWM devices allocated by the framework
  * @of_xlate: request a PWM device given a device tree PWM specifier
+ * @get_default_caps: get default PWM capabilities
  * @of_pwm_n_cells: number of cells expected in the device tree PWM specifier
  */
 struct pwm_chip {
@@ -300,6 +331,7 @@ struct pwm_chip {
 
 	struct pwm_device * (*of_xlate)(struct pwm_chip *pc,
 					const struct of_phandle_args *args);
+	void (*get_default_caps)(struct pwm_caps *caps);
 	unsigned int of_pwm_n_cells;
 };
 
@@ -424,6 +456,37 @@ static inline void pwm_disable(struct pwm_device *pwm)
 	pwm_apply_state(pwm, &state);
 }
 
+static inline bool pwm_mode_valid(unsigned long mode)
+{
+	return (mode &&
+		hweight_long(mode) == 1 &&
+		ffs(mode) - 1 < PWM_MODE_CNT);
+}
+
+static inline bool pwm_caps_valid(struct pwm_caps caps)
+{
+	unsigned int last;
+
+	if (!caps.modes)
+		return false;
+
+	last = fls(caps.modes) - 1;
+	if (last >= PWM_MODE_CNT)
+		return false;
+
+	return true;
+}
+
+static inline const char * const pwm_mode_desc(unsigned long mode)
+{
+	static const char * const modes[] = { "normal", "complementary"	};
+
+	if (!pwm_mode_valid(mode))
+		return "invalid";
+
+	return modes[ffs(mode) - 1];
+}
+
 /* PWM provider APIs */
 int pwm_capture(struct pwm_device *pwm, struct pwm_capture *result,
 		unsigned long timeout);
@@ -438,6 +501,9 @@ struct pwm_device *pwm_request_from_chip(struct pwm_chip *chip,
 					 unsigned int index,
 					 const char *label);
 
+void pwm_get_caps(struct pwm_chip *chip, struct pwm_device *pwm,
+		  struct pwm_caps *caps);
+
 struct pwm_device *of_pwm_xlate_with_flags(struct pwm_chip *pc,
 		const struct of_phandle_args *args);
 
@@ -498,6 +564,26 @@ static inline void pwm_disable(struct pwm_device *pwm)
 {
 }
 
+static inline bool pwm_mode_valid(unsigned long mode)
+{
+	return false;
+}
+
+static inline bool pwm_caps_valid(struct pwm_caps caps)
+{
+	return false;
+}
+
+static inline const char * const pwm_mode_desc(unsigned long mode)
+{
+	return NULL;
+}
+
+static inline void pwm_get_caps(struct pwm_chip *chip, struct pwm_device *pwm,
+				struct pwm_caps *caps)
+{
+}
+
 static inline int pwm_set_chip_data(struct pwm_device *pwm, void *data)
 {
 	return -EINVAL;
@@ -592,6 +678,7 @@ static inline void pwm_apply_args(struct pwm_device *pwm)
 	state.enabled = false;
 	state.polarity = pwm->args.polarity;
 	state.period = pwm->args.period;
+	state.mode = pwm->args.mode;
 
 	pwm_apply_state(pwm, &state);
 }
-- 
2.7.4

[PATCH v3 02/10] pwm: clps711x: populate PWM mode in of_xlate function

[Claudiu Beznea]

Populate PWM mode in of_xlate function to avoid pwm_apply_state() failure.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/pwm/pwm-clps711x.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-clps711x.c b/drivers/pwm/pwm-clps711x.c
index 26ec24e457b1..2a4d31ab3af0 100644
--- a/drivers/pwm/pwm-clps711x.c
+++ b/drivers/pwm/pwm-clps711x.c
@@ -109,10 +109,20 @@ static const struct pwm_ops clps711x_pwm_ops = {
 static struct pwm_device *clps711x_pwm_xlate(struct pwm_chip *chip,
 					     const struct of_phandle_args *args)
 {
+	struct pwm_device *pwm;
+	struct pwm_caps caps;
+
 	if (args->args[0] >= chip->npwm)
 		return ERR_PTR(-EINVAL);
 
-	return pwm_request_from_chip(chip, args->args[0], NULL);
+	pwm = pwm_request_from_chip(chip, args->args[0], NULL);
+	if (IS_ERR(pwm))
+		return pwm;
+
+	pwm_get_caps(chip, pwm, &caps);
+	pwm->args.mode = BIT(ffs(caps.modes) - 1);
+
+	return pwm;
 }
 
 static int clps711x_pwm_probe(struct platform_device *pdev)
-- 
2.7.4

[PATCH v3 03/10] pwm: cros-ec: populate PWM mode in of_xlate function

[Claudiu Beznea]

Populate PWM mode in of_xlate function to avoid pwm_apply_state() failure.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/pwm/pwm-cros-ec.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pwm/pwm-cros-ec.c b/drivers/pwm/pwm-cros-ec.c
index 9c13694eaa24..e54954c13323 100644
--- a/drivers/pwm/pwm-cros-ec.c
+++ b/drivers/pwm/pwm-cros-ec.c
@@ -137,6 +137,7 @@ static struct pwm_device *
 cros_ec_pwm_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
 {
 	struct pwm_device *pwm;
+	struct pwm_caps caps;
 
 	if (args->args[0] >= pc->npwm)
 		return ERR_PTR(-EINVAL);
@@ -145,8 +146,11 @@ cros_ec_pwm_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
 	if (IS_ERR(pwm))
 		return pwm;
 
+	pwm_get_caps(pc, pwm, &caps);
+
 	/* The EC won't let us change the period */
 	pwm->args.period = EC_PWM_MAX_DUTY;
+	pwm->args.mode = BIT(ffs(caps.modes) - 1);
 
 	return pwm;
 }
-- 
2.7.4

[PATCH v3 04/10] pwm: pxa: populate PWM mode in of_xlate function

[Claudiu Beznea]

Populate PWM mode in of_xlate function to avoid pwm_apply_state() failure.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/pwm/pwm-pxa.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pwm/pwm-pxa.c b/drivers/pwm/pwm-pxa.c
index 4143a46684d2..7a035716e054 100644
--- a/drivers/pwm/pwm-pxa.c
+++ b/drivers/pwm/pwm-pxa.c
@@ -155,12 +155,16 @@ static struct pwm_device *
 pxa_pwm_of_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
 {
 	struct pwm_device *pwm;
+	struct pwm_caps caps;
 
 	pwm = pwm_request_from_chip(pc, 0, NULL);
 	if (IS_ERR(pwm))
 		return pwm;
 
+	pwm_get_caps(pc, pwm, &caps);
+
 	pwm->args.period = args->args[0];
+	pwm->args.mode = BIT(ffs(caps.modes) - 1);
 
 	return pwm;
 }
-- 
2.7.4

[PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Claudiu Beznea]

Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
were adapted to this change.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +++++++++--
 drivers/bus/ts-nbus.c                |  2 +-
 drivers/clk/clk-pwm.c                |  3 ++-
 drivers/gpu/drm/i915/intel_panel.c   | 17 ++++++++++++++---
 drivers/hwmon/pwm-fan.c              |  2 +-
 drivers/input/misc/max77693-haptic.c |  2 +-
 drivers/input/misc/max8997_haptic.c  |  6 +++++-
 drivers/leds/leds-pwm.c              |  5 ++++-
 drivers/media/rc/ir-rx51.c           |  5 ++++-
 drivers/media/rc/pwm-ir-tx.c         |  5 ++++-
 drivers/video/backlight/lm3630a_bl.c |  4 +++-
 drivers/video/backlight/lp855x_bl.c  |  4 +++-
 drivers/video/backlight/lp8788_bl.c  |  5 ++++-
 drivers/video/backlight/pwm_bl.c     | 11 +++++++++--
 drivers/video/fbdev/ssd1307fb.c      |  3 ++-
 include/linux/pwm.h                  |  6 ++++--
 16 files changed, 70 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mach-s3c24xx/mach-rx1950.c b/arch/arm/mach-s3c24xx/mach-rx1950.c
index e86ad6a68a0b..6feae73dcc73 100644
--- a/arch/arm/mach-s3c24xx/mach-rx1950.c
+++ b/arch/arm/mach-s3c24xx/mach-rx1950.c
@@ -386,8 +386,13 @@ static void rx1950_lcd_power(int enable)
 {
 	int i;
 	static int enabled;
+	struct pwm_caps caps = { };
+
 	if (enabled == enable)
 		return;
+
+	pwm_get_caps(lcd_pwm->chip, lcd_pwm, &caps);
+
 	if (!enable) {
 
 		/* GPC11-GPC15->OUTPUT */
@@ -433,14 +438,16 @@ static void rx1950_lcd_power(int enable)
 
 		/* GPB1->OUTPUT, GPB1->0 */
 		gpio_direction_output(S3C2410_GPB(1), 0);
-		pwm_config(lcd_pwm, 0, LCD_PWM_PERIOD);
+		pwm_config(lcd_pwm, 0, LCD_PWM_PERIOD,
+			   BIT(ffs(caps.modes) - 1));
 		pwm_disable(lcd_pwm);
 
 		/* GPC0->0, GPC10->0 */
 		gpio_direction_output(S3C2410_GPC(0), 0);
 		gpio_direction_output(S3C2410_GPC(10), 0);
 	} else {
-		pwm_config(lcd_pwm, LCD_PWM_DUTY, LCD_PWM_PERIOD);
+		pwm_config(lcd_pwm, LCD_PWM_DUTY, LCD_PWM_PERIOD,
+			   BIT(ffs(caps.modes) - 1));
 		pwm_enable(lcd_pwm);
 
 		gpio_direction_output(S3C2410_GPC(0), 1);
diff --git a/drivers/bus/ts-nbus.c b/drivers/bus/ts-nbus.c
index 073fd9011154..dcd2ca3bcd99 100644
--- a/drivers/bus/ts-nbus.c
+++ b/drivers/bus/ts-nbus.c
@@ -316,7 +316,7 @@ static int ts_nbus_probe(struct platform_device *pdev)
 	 * the atomic PWM API.
 	 */
 	pwm_apply_args(pwm);
-	ret = pwm_config(pwm, pargs.period, pargs.period);
+	ret = pwm_config(pwm, pargs.period, pargs.period, pargs.mode);
 	if (ret < 0)
 		return ret;
 
diff --git a/drivers/clk/clk-pwm.c b/drivers/clk/clk-pwm.c
index 8cb9d117fdbf..605a6bffe893 100644
--- a/drivers/clk/clk-pwm.c
+++ b/drivers/clk/clk-pwm.c
@@ -92,7 +92,8 @@ static int clk_pwm_probe(struct platform_device *pdev)
 	 * atomic PWM API.
 	 */
 	pwm_apply_args(pwm);
-	ret = pwm_config(pwm, (pargs.period + 1) >> 1, pargs.period);
+	ret = pwm_config(pwm, (pargs.period + 1) >> 1, pargs.period,
+			 pargs.mode);
 	if (ret < 0)
 		return ret;
 
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index adc51e452e3e..960556261787 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -633,8 +633,12 @@ static void pwm_set_backlight(const struct drm_connector_state *conn_state, u32
 {
 	struct intel_panel *panel = &to_intel_connector(conn_state->connector)->panel;
 	int duty_ns = DIV_ROUND_UP(level * CRC_PMIC_PWM_PERIOD_NS, 100);
+	struct pwm_caps caps = { };
 
-	pwm_config(panel->backlight.pwm, duty_ns, CRC_PMIC_PWM_PERIOD_NS);
+	pwm_get_caps(panel->backlight.pwm->chip, panel->backlight.pwm, &caps);
+
+	pwm_config(panel->backlight.pwm, duty_ns, CRC_PMIC_PWM_PERIOD_NS,
+		   BIT(ffs(caps.modes) - 1));
 }
 
 static void
@@ -821,9 +825,13 @@ static void pwm_disable_backlight(const struct drm_connector_state *old_conn_sta
 {
 	struct intel_connector *connector = to_intel_connector(old_conn_state->connector);
 	struct intel_panel *panel = &connector->panel;
+	struct pwm_caps caps = { };
+
+	pwm_get_caps(panel->backlight.pwm->chip, panel->backlight.pwm, &caps);
 
 	/* Disable the backlight */
-	pwm_config(panel->backlight.pwm, 0, CRC_PMIC_PWM_PERIOD_NS);
+	pwm_config(panel->backlight.pwm, 0, CRC_PMIC_PWM_PERIOD_NS,
+		   BIT(ffs(caps.modes) - 1));
 	usleep_range(2000, 3000);
 	pwm_disable(panel->backlight.pwm);
 }
@@ -1754,6 +1762,7 @@ static int pwm_setup_backlight(struct intel_connector *connector,
 {
 	struct drm_device *dev = connector->base.dev;
 	struct intel_panel *panel = &connector->panel;
+	struct pwm_caps caps = { };
 	int retval;
 
 	/* Get the PWM chip for backlight control */
@@ -1770,8 +1779,10 @@ static int pwm_setup_backlight(struct intel_connector *connector,
 	 */
 	pwm_apply_args(panel->backlight.pwm);
 
+	pwm_get_caps(panel->backlight.pwm->chip, panel->backlight.pwm, &caps);
+
 	retval = pwm_config(panel->backlight.pwm, CRC_PMIC_PWM_PERIOD_NS,
-			    CRC_PMIC_PWM_PERIOD_NS);
+			    CRC_PMIC_PWM_PERIOD_NS, BIT(ffs(caps.modes) - 1));
 	if (retval < 0) {
 		DRM_ERROR("Failed to configure the pwm chip\n");
 		pwm_put(panel->backlight.pwm);
diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
index 70cc0d134f3c..bd05cd81d3d5 100644
--- a/drivers/hwmon/pwm-fan.c
+++ b/drivers/hwmon/pwm-fan.c
@@ -308,7 +308,7 @@ static int pwm_fan_resume(struct device *dev)
 
 	pwm_get_args(ctx->pwm, &pargs);
 	duty = DIV_ROUND_UP(ctx->pwm_value * (pargs.period - 1), MAX_PWM);
-	ret = pwm_config(ctx->pwm, duty, pargs.period);
+	ret = pwm_config(ctx->pwm, duty, pargs.period, pargs.mode);
 	if (ret)
 		return ret;
 	return pwm_enable(ctx->pwm);
diff --git a/drivers/input/misc/max77693-haptic.c b/drivers/input/misc/max77693-haptic.c
index 46b0f48fbf49..5fe2ff2b408b 100644
--- a/drivers/input/misc/max77693-haptic.c
+++ b/drivers/input/misc/max77693-haptic.c
@@ -76,7 +76,7 @@ static int max77693_haptic_set_duty_cycle(struct max77693_haptic *haptic)
 
 	pwm_get_args(haptic->pwm_dev, &pargs);
 	delta = (pargs.period + haptic->pwm_duty) / 2;
-	error = pwm_config(haptic->pwm_dev, delta, pargs.period);
+	error = pwm_config(haptic->pwm_dev, delta, pargs.period, pargs.mode);
 	if (error) {
 		dev_err(haptic->dev, "failed to configure pwm: %d\n", error);
 		return error;
diff --git a/drivers/input/misc/max8997_haptic.c b/drivers/input/misc/max8997_haptic.c
index 99bc762881d5..16de524dc489 100644
--- a/drivers/input/misc/max8997_haptic.c
+++ b/drivers/input/misc/max8997_haptic.c
@@ -73,7 +73,11 @@ static int max8997_haptic_set_duty_cycle(struct max8997_haptic *chip)
 
 	if (chip->mode == MAX8997_EXTERNAL_MODE) {
 		unsigned int duty = chip->pwm_period * chip->level / 100;
-		ret = pwm_config(chip->pwm, duty, chip->pwm_period);
+		struct pwm_caps caps = { };
+
+		pwm_get_caps(chip->pwm->chip, chip->pwm, &caps);
+		ret = pwm_config(chip->pwm, duty, chip->pwm_period,
+				 BIT(ffs(caps.modes) - 1));
 	} else {
 		int i;
 		u8 duty_index = 0;
diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 8d456dc6c5bf..8ba942e96b63 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -39,8 +39,11 @@ struct led_pwm_priv {
 static void __led_pwm_set(struct led_pwm_data *led_dat)
 {
 	int new_duty = led_dat->duty;
+	struct pwm_caps caps = { };
 
-	pwm_config(led_dat->pwm, new_duty, led_dat->period);
+	pwm_get_caps(led_dat->pwm->chip, led_dat->pwm, &caps);
+	pwm_config(led_dat->pwm, new_duty, led_dat->period,
+		   BIT(ffs(caps.modes) - 1));
 
 	if (new_duty == 0)
 		pwm_disable(led_dat->pwm);
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 49265f02e772..a971b02ea021 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -55,10 +55,13 @@ static int init_timing_params(struct ir_rx51 *ir_rx51)
 {
 	struct pwm_device *pwm = ir_rx51->pwm;
 	int duty, period = DIV_ROUND_CLOSEST(NSEC_PER_SEC, ir_rx51->freq);
+	struct pwm_caps caps = { };
 
 	duty = DIV_ROUND_CLOSEST(ir_rx51->duty_cycle * period, 100);
 
-	pwm_config(pwm, duty, period);
+	pwm_get_caps(pwm->chip, pwm, &caps);
+
+	pwm_config(pwm, duty, period, BIT(ffs(caps.modes) - 1));
 
 	return 0;
 }
diff --git a/drivers/media/rc/pwm-ir-tx.c b/drivers/media/rc/pwm-ir-tx.c
index 27d0f5837a76..c630e1b450a3 100644
--- a/drivers/media/rc/pwm-ir-tx.c
+++ b/drivers/media/rc/pwm-ir-tx.c
@@ -61,6 +61,7 @@ static int pwm_ir_tx(struct rc_dev *dev, unsigned int *txbuf,
 {
 	struct pwm_ir *pwm_ir = dev->priv;
 	struct pwm_device *pwm = pwm_ir->pwm;
+	struct pwm_caps caps = { };
 	int i, duty, period;
 	ktime_t edge;
 	long delta;
@@ -68,7 +69,9 @@ static int pwm_ir_tx(struct rc_dev *dev, unsigned int *txbuf,
 	period = DIV_ROUND_CLOSEST(NSEC_PER_SEC, pwm_ir->carrier);
 	duty = DIV_ROUND_CLOSEST(pwm_ir->duty_cycle * period, 100);
 
-	pwm_config(pwm, duty, period);
+	pwm_get_caps(pwm->chip, pwm, &caps);
+
+	pwm_config(pwm, duty, period, BIT(ffs(caps.modes) - 1));
 
 	edge = ktime_get();
 
diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
index 2030a6b77a09..696fa25dafd2 100644
--- a/drivers/video/backlight/lm3630a_bl.c
+++ b/drivers/video/backlight/lm3630a_bl.c
@@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
 {
 	unsigned int period = pchip->pdata->pwm_period;
 	unsigned int duty = br * period / br_max;
+	struct pwm_caps caps = { };
 
-	pwm_config(pchip->pwmd, duty, period);
+	pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, &caps);
+	pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
 	if (duty)
 		pwm_enable(pchip->pwmd);
 	else
diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
index 939f057836e1..3d274c604862 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -240,6 +240,7 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
 	unsigned int period = lp->pdata->period_ns;
 	unsigned int duty = br * period / max_br;
 	struct pwm_device *pwm;
+	struct pwm_caps caps = { };
 
 	/* request pwm device with the consumer name */
 	if (!lp->pwm) {
@@ -256,7 +257,8 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
 		pwm_apply_args(pwm);
 	}
 
-	pwm_config(lp->pwm, duty, period);
+	pwm_get_caps(lp->pwm->chip, lp->pwm, &caps);
+	pwm_config(lp->pwm, duty, period, BIT(ffs(caps.modes) - 1));
 	if (duty)
 		pwm_enable(lp->pwm);
 	else
diff --git a/drivers/video/backlight/lp8788_bl.c b/drivers/video/backlight/lp8788_bl.c
index cf869ec90cce..06de3163650d 100644
--- a/drivers/video/backlight/lp8788_bl.c
+++ b/drivers/video/backlight/lp8788_bl.c
@@ -128,6 +128,7 @@ static void lp8788_pwm_ctrl(struct lp8788_bl *bl, int br, int max_br)
 	unsigned int duty;
 	struct device *dev;
 	struct pwm_device *pwm;
+	struct pwm_caps caps = { };
 
 	if (!bl->pdata)
 		return;
@@ -153,7 +154,9 @@ static void lp8788_pwm_ctrl(struct lp8788_bl *bl, int br, int max_br)
 		pwm_apply_args(pwm);
 	}
 
-	pwm_config(bl->pwm, duty, period);
+	pwm_get_caps(bl->pwm->chip, bl->pwm, &caps);
+
+	pwm_config(bl->pwm, duty, period, BIT(ffs(caps.modes) - 1));
 	if (duty)
 		pwm_enable(bl->pwm);
 	else
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 1c2289ddd555..706a9ab053a7 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -63,10 +63,14 @@ static void pwm_backlight_power_on(struct pwm_bl_data *pb, int brightness)
 
 static void pwm_backlight_power_off(struct pwm_bl_data *pb)
 {
+	struct pwm_caps caps = { };
+
 	if (!pb->enabled)
 		return;
 
-	pwm_config(pb->pwm, 0, pb->period);
+	pwm_get_caps(pb->pwm->chip, pb->pwm, &caps);
+
+	pwm_config(pb->pwm, 0, pb->period, BIT(ffs(caps.modes) - 1));
 	pwm_disable(pb->pwm);
 
 	if (pb->enable_gpio)
@@ -96,6 +100,7 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
 {
 	struct pwm_bl_data *pb = bl_get_data(bl);
 	int brightness = bl->props.brightness;
+	struct pwm_caps caps = { };
 	int duty_cycle;
 
 	if (bl->props.power != FB_BLANK_UNBLANK ||
@@ -108,7 +113,9 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
 
 	if (brightness > 0) {
 		duty_cycle = compute_duty_cycle(pb, brightness);
-		pwm_config(pb->pwm, duty_cycle, pb->period);
+		pwm_get_caps(pb->pwm->chip, pb->pwm, &caps);
+		pwm_config(pb->pwm, duty_cycle, pb->period,
+			   BIT(ffs(caps.modes) - 1));
 		pwm_backlight_power_on(pb, brightness);
 	} else
 		pwm_backlight_power_off(pb);
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index f599520374dd..4b57dcb5799a 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -308,7 +308,8 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
 
 		par->pwm_period = pargs.period;
 		/* Enable the PWM */
-		pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period);
+		pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period,
+			   pargs.mode);
 		pwm_enable(par->pwm);
 
 		dev_dbg(&par->client->dev, "Using PWM%d with a %dns period.\n",
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index e62349f48129..0ba416ab2772 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -357,11 +357,12 @@ int pwm_adjust_config(struct pwm_device *pwm);
  * @pwm: PWM device
  * @duty_ns: "on" time (in nanoseconds)
  * @period_ns: duration (in nanoseconds) of one cycle
+ * @mode: PWM mode
  *
  * Returns: 0 on success or a negative error code on failure.
  */
 static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
-			     int period_ns)
+			     int period_ns, unsigned long mode)
 {
 	struct pwm_state state;
 
@@ -377,6 +378,7 @@ static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
 
 	state.duty_cycle = duty_ns;
 	state.period = period_ns;
+	state.mode = mode;
 	return pwm_apply_state(pwm, &state);
 }
 
@@ -537,7 +539,7 @@ static inline int pwm_adjust_config(struct pwm_device *pwm)
 }
 
 static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
-			     int period_ns)
+			     int period_ns, unsigned long mode)
 {
 	return -EINVAL;
 }
-- 
2.7.4

[PATCH v3 06/10] pwm: add PWM modes

[Claudiu Beznea]

Add PWM normal and complementary modes.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 Documentation/devicetree/bindings/pwm/pwm.txt |  9 +++++++--
 Documentation/pwm.txt                         | 26 +++++++++++++++++++++++---
 include/dt-bindings/pwm/pwm.h                 |  1 +
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm.txt b/Documentation/devicetree/bindings/pwm/pwm.txt
index 8556263b8502..c2b4b9ba0e3f 100644
--- a/Documentation/devicetree/bindings/pwm/pwm.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm.txt
@@ -46,11 +46,16 @@ period in nanoseconds.
 Optionally, the pwm-specifier can encode a number of flags (defined in
 <dt-bindings/pwm/pwm.h>) in a third cell:
 - PWM_POLARITY_INVERTED: invert the PWM signal polarity
+- PWM_DTMODE_COMPLEMENTARY: PWM complementary working mode (for PWM
+channels two outputs); if not specified, the default for PWM channel will
+be used
 
-Example with optional PWM specifier for inverse polarity
+Example with optional PWM specifier for inverse polarity and complementary
+mode:
 
 	bl: backlight {
-		pwms = <&pwm 0 5000000 PWM_POLARITY_INVERTED>;
+		pwms = <&pwm 0 5000000
+			(PWM_DTMODE_COMPLEMENTARY | PWM_POLARITY_INVERTED)>;
 		pwm-names = "backlight";
 	};
 
diff --git a/Documentation/pwm.txt b/Documentation/pwm.txt
index 8fbf0aa3ba2d..59592f8cc381 100644
--- a/Documentation/pwm.txt
+++ b/Documentation/pwm.txt
@@ -61,9 +61,9 @@ In addition to the PWM state, the PWM API also exposes PWM arguments, which
 are the reference PWM config one should use on this PWM.
 PWM arguments are usually platform-specific and allows the PWM user to only
 care about dutycycle relatively to the full period (like, duty = 50% of the
-period). struct pwm_args contains 2 fields (period and polarity) and should
-be used to set the initial PWM config (usually done in the probe function
-of the PWM user). PWM arguments are retrieved with pwm_get_args().
+period). struct pwm_args contains 3 fields (period, polarity and mode) and
+should be used to set the initial PWM config (usually done in the probe
+function of the PWM user). PWM arguments are retrieved with pwm_get_args().
 
 Using PWMs with the sysfs interface
 -----------------------------------
@@ -110,6 +110,26 @@ channel that was exported. The following properties will then be available:
 	- 0 - disabled
 	- 1 - enabled
 
+  mode
+    Get/set PWM channel working mode.
+
+    Normal mode - for PWM channels with one output; this should be the
+        default working mode for every PWM channel; output waveforms looks
+        like this:
+             __    __    __    __
+    PWM   __|  |__|  |__|  |__|  |__
+            <--T-->
+
+    Complementary mode - for PWM channels two outputs; output waveforms
+        looks line this:
+             __    __    __    __
+    PWMH1 __|  |__|  |__|  |__|  |__
+          __    __    __    __    __
+    PWML1   |__|  |__|  |__|  |__|
+            <--T-->
+
+    Where T is the signal period.
+
 Implementing a PWM driver
 -------------------------
 
diff --git a/include/dt-bindings/pwm/pwm.h b/include/dt-bindings/pwm/pwm.h
index ab9a077e3c7d..b4d61269aced 100644
--- a/include/dt-bindings/pwm/pwm.h
+++ b/include/dt-bindings/pwm/pwm.h
@@ -11,5 +11,6 @@
 #define _DT_BINDINGS_PWM_PWM_H
 
 #define PWM_POLARITY_INVERTED			(1 << 0)
+#define PWM_DTMODE_COMPLEMENTARY		(1 << 1)
 
 #endif
-- 
2.7.4

[PATCH v3 07/10] pwm: atmel: add pwm capabilities

[Claudiu Beznea]

Add pwm capabilities for Atmel/Microchip PWM controllers.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/pwm/pwm-atmel.c | 80 ++++++++++++++++++++++++++++++++-----------------
 1 file changed, 52 insertions(+), 28 deletions(-)

diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index 530d7dc5f1b5..d2482fe28cfa 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -65,11 +65,16 @@ struct atmel_pwm_registers {
 	u8 duty_upd;
 };
 
+struct atmel_pwm_data {
+	struct atmel_pwm_registers regs;
+	struct pwm_caps caps;
+};
+
 struct atmel_pwm_chip {
 	struct pwm_chip chip;
 	struct clk *clk;
 	void __iomem *base;
-	const struct atmel_pwm_registers *regs;
+	const struct atmel_pwm_data *data;
 
 	unsigned int updated_pwms;
 	/* ISR is cleared when read, ensure only one thread does that */
@@ -150,15 +155,15 @@ static void atmel_pwm_update_cdty(struct pwm_chip *chip, struct pwm_device *pwm,
 	struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
 	u32 val;
 
-	if (atmel_pwm->regs->duty_upd ==
-	    atmel_pwm->regs->period_upd) {
+	if (atmel_pwm->data->regs.duty_upd ==
+	    atmel_pwm->data->regs.period_upd) {
 		val = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm, PWM_CMR);
 		val &= ~PWM_CMR_UPD_CDTY;
 		atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, PWM_CMR, val);
 	}
 
 	atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm,
-			    atmel_pwm->regs->duty_upd, cdty);
+			    atmel_pwm->data->regs.duty_upd, cdty);
 }
 
 static void atmel_pwm_set_cprd_cdty(struct pwm_chip *chip,
@@ -168,9 +173,9 @@ static void atmel_pwm_set_cprd_cdty(struct pwm_chip *chip,
 	struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
 
 	atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm,
-			    atmel_pwm->regs->duty, cdty);
+			    atmel_pwm->data->regs.duty, cdty);
 	atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm,
-			    atmel_pwm->regs->period, cprd);
+			    atmel_pwm->data->regs.period, cprd);
 }
 
 static void atmel_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm,
@@ -225,7 +230,7 @@ static int atmel_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 		    cstate.polarity == state->polarity &&
 		    cstate.period == state->period) {
 			cprd = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm,
-						  atmel_pwm->regs->period);
+						  atmel_pwm->data->regs.period);
 			atmel_pwm_calculate_cdty(state, cprd, &cdty);
 			atmel_pwm_update_cdty(chip, pwm, cdty);
 			return 0;
@@ -272,32 +277,51 @@ static int atmel_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	return 0;
 }
 
+static void atmel_pwm_get_caps(struct pwm_chip *chip, struct pwm_device *pwm,
+			       struct pwm_caps *caps)
+{
+	struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
+
+	*caps = atmel_pwm->data->caps;
+}
+
 static const struct pwm_ops atmel_pwm_ops = {
 	.apply = atmel_pwm_apply,
+	.get_caps = atmel_pwm_get_caps,
 	.owner = THIS_MODULE,
 };
 
-static const struct atmel_pwm_registers atmel_pwm_regs_v1 = {
-	.period		= PWMV1_CPRD,
-	.period_upd	= PWMV1_CUPD,
-	.duty		= PWMV1_CDTY,
-	.duty_upd	= PWMV1_CUPD,
+static const struct atmel_pwm_data atmel_pwm_data_v1 = {
+	.regs = {
+		.period		= PWMV1_CPRD,
+		.period_upd	= PWMV1_CUPD,
+		.duty		= PWMV1_CDTY,
+		.duty_upd	= PWMV1_CUPD,
+	},
+	.caps = {
+		.modes = PWM_MODE(NORMAL),
+	},
 };
 
-static const struct atmel_pwm_registers atmel_pwm_regs_v2 = {
-	.period		= PWMV2_CPRD,
-	.period_upd	= PWMV2_CPRDUPD,
-	.duty		= PWMV2_CDTY,
-	.duty_upd	= PWMV2_CDTYUPD,
+static const struct atmel_pwm_data atmel_pwm_data_v2 = {
+	.regs = {
+		.period		= PWMV2_CPRD,
+		.period_upd	= PWMV2_CPRDUPD,
+		.duty		= PWMV2_CDTY,
+		.duty_upd	= PWMV2_CDTYUPD,
+	},
+	.caps = {
+		.modes = PWM_MODE(NORMAL) | PWM_MODE(COMPLEMENTARY),
+	},
 };
 
 static const struct platform_device_id atmel_pwm_devtypes[] = {
 	{
 		.name = "at91sam9rl-pwm",
-		.driver_data = (kernel_ulong_t)&atmel_pwm_regs_v1,
+		.driver_data = (kernel_ulong_t)&atmel_pwm_data_v1,
 	}, {
 		.name = "sama5d3-pwm",
-		.driver_data = (kernel_ulong_t)&atmel_pwm_regs_v2,
+		.driver_data = (kernel_ulong_t)&atmel_pwm_data_v2,
 	}, {
 		/* sentinel */
 	},
@@ -307,20 +331,20 @@ MODULE_DEVICE_TABLE(platform, atmel_pwm_devtypes);
 static const struct of_device_id atmel_pwm_dt_ids[] = {
 	{
 		.compatible = "atmel,at91sam9rl-pwm",
-		.data = &atmel_pwm_regs_v1,
+		.data = &atmel_pwm_data_v1,
 	}, {
 		.compatible = "atmel,sama5d3-pwm",
-		.data = &atmel_pwm_regs_v2,
+		.data = &atmel_pwm_data_v2,
 	}, {
 		.compatible = "atmel,sama5d2-pwm",
-		.data = &atmel_pwm_regs_v2,
+		.data = &atmel_pwm_data_v2,
 	}, {
 		/* sentinel */
 	},
 };
 MODULE_DEVICE_TABLE(of, atmel_pwm_dt_ids);
 
-static inline const struct atmel_pwm_registers *
+static inline const struct atmel_pwm_data *
 atmel_pwm_get_driver_data(struct platform_device *pdev)
 {
 	const struct platform_device_id *id;
@@ -330,18 +354,18 @@ atmel_pwm_get_driver_data(struct platform_device *pdev)
 
 	id = platform_get_device_id(pdev);
 
-	return (struct atmel_pwm_registers *)id->driver_data;
+	return (struct atmel_pwm_data *)id->driver_data;
 }
 
 static int atmel_pwm_probe(struct platform_device *pdev)
 {
-	const struct atmel_pwm_registers *regs;
+	const struct atmel_pwm_data *data;
 	struct atmel_pwm_chip *atmel_pwm;
 	struct resource *res;
 	int ret;
 
-	regs = atmel_pwm_get_driver_data(pdev);
-	if (!regs)
+	data = atmel_pwm_get_driver_data(pdev);
+	if (!data)
 		return -ENODEV;
 
 	atmel_pwm = devm_kzalloc(&pdev->dev, sizeof(*atmel_pwm), GFP_KERNEL);
@@ -373,7 +397,7 @@ static int atmel_pwm_probe(struct platform_device *pdev)
 
 	atmel_pwm->chip.base = -1;
 	atmel_pwm->chip.npwm = 4;
-	atmel_pwm->regs = regs;
+	atmel_pwm->data = data;
 	atmel_pwm->updated_pwms = 0;
 	mutex_init(&atmel_pwm->isr_lock);
 
-- 
2.7.4

[PATCH v3 08/10] pwm: add push-pull mode support

[Claudiu Beznea]

Add push-pull mode support. In push-pull mode the channels' outputs have
same polarities and the edges are complementary delayed for one period.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 include/linux/pwm.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 0ba416ab2772..765d760ef82d 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -29,11 +29,14 @@ enum pwm_polarity {
  * PWM modes
  * @PWM_MODE_NORMAL_BIT: PWM has one output
  * @PWM_MODE_COMPLEMENTARY_BIT: PWM has 2 outputs with opposite polarities
+ * @PWM_MODE_PUSH_PULL_BIT: PWM has 2 outputs with same polarities and the edges
+ * are complementary delayed for one period
  * @PWM_MODE_CNT: PWM modes count
  */
 enum {
 	PWM_MODE_NORMAL_BIT,
 	PWM_MODE_COMPLEMENTARY_BIT,
+	PWM_MODE_PUSH_PULL_BIT,
 	PWM_MODE_CNT,
 };
 
@@ -481,7 +484,11 @@ static inline bool pwm_caps_valid(struct pwm_caps caps)
 
 static inline const char * const pwm_mode_desc(unsigned long mode)
 {
-	static const char * const modes[] = { "normal", "complementary"	};
+	static const char * const modes[] = {
+		"normal",
+		"complementary",
+		"push-pull",
+	};
 
 	if (!pwm_mode_valid(mode))
 		return "invalid";
-- 
2.7.4

[PATCH v3 09/10] pwm: add documentation for pwm push-pull mode

[Claudiu Beznea]

Add documentation for PWM push-pull mode.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/pwm/pwm.txt |  2 ++
 Documentation/pwm.txt                         | 16 ++++++++++++++++
 include/dt-bindings/pwm/pwm.h                 |  1 +
 3 files changed, 19 insertions(+)

diff --git a/Documentation/devicetree/bindings/pwm/pwm.txt b/Documentation/devicetree/bindings/pwm/pwm.txt
index c2b4b9ba0e3f..968dd786ad7c 100644
--- a/Documentation/devicetree/bindings/pwm/pwm.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm.txt
@@ -49,6 +49,8 @@ Optionally, the pwm-specifier can encode a number of flags (defined in
 - PWM_DTMODE_COMPLEMENTARY: PWM complementary working mode (for PWM
 channels two outputs); if not specified, the default for PWM channel will
 be used
+- PWM_DTMODE_PUSH_PULL: PWM push-pull working modes (for PWM channels with
+two outputs); if not specified the default for PWM channel will be used
 
 Example with optional PWM specifier for inverse polarity and complementary
 mode:
diff --git a/Documentation/pwm.txt b/Documentation/pwm.txt
index 59592f8cc381..174c371583b6 100644
--- a/Documentation/pwm.txt
+++ b/Documentation/pwm.txt
@@ -128,6 +128,22 @@ channel that was exported. The following properties will then be available:
     PWML1   |__|  |__|  |__|  |__|
             <--T-->
 
+    Push-pull mode - for PWM channels with two outputs; output waveforms
+        for a PWM channel with 2 outputs, in push-pull mode, with normal
+        polarity looks like this:
+            __          __
+    PWMH __|  |________|  |________
+                  __          __
+    PWML ________|  |________|  |__
+           <--T-->
+
+    If polarity is inversed:
+         __    ________    ________
+    PWMH   |__|        |__|
+         ________    ________    __
+    PWML         |__|        |__|
+           <--T-->
+
     Where T is the signal period.
 
 Implementing a PWM driver
diff --git a/include/dt-bindings/pwm/pwm.h b/include/dt-bindings/pwm/pwm.h
index b4d61269aced..674dfdd59595 100644
--- a/include/dt-bindings/pwm/pwm.h
+++ b/include/dt-bindings/pwm/pwm.h
@@ -12,5 +12,6 @@
 
 #define PWM_POLARITY_INVERTED			(1 << 0)
 #define PWM_DTMODE_COMPLEMENTARY		(1 << 1)
+#define PWM_DTMODE_PUSH_PULL			(1 << 2)
 
 #endif
-- 
2.7.4

[PATCH v3 10/10] pwm: atmel: add push-pull mode support

[Claudiu Beznea]

Add support for PWM push-pull mode. This is only supported by SAMA5D2 SoCs.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/pwm/pwm-atmel.c | 40 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index d2482fe28cfa..da4b58c1ecf2 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -33,8 +33,11 @@
 
 #define PWM_CMR			0x0
 /* Bit field in CMR */
-#define PWM_CMR_CPOL		(1 << 9)
-#define PWM_CMR_UPD_CDTY	(1 << 10)
+#define PWM_CMR_CPOL		BIT(9)
+#define PWM_CMR_UPD_CDTY	BIT(10)
+#define PWM_CMR_DTHI		BIT(17)
+#define PWM_CMR_DTLI		BIT(18)
+#define PWM_CMR_PPM		BIT(19)
 #define PWM_CMR_CPRE_MSK	0xF
 
 /* The following registers for PWM v1 */
@@ -219,16 +222,19 @@ static int atmel_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 {
 	struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
 	struct pwm_state cstate;
+	struct pwm_caps caps;
 	unsigned long cprd, cdty;
 	u32 pres, val;
 	int ret;
 
 	pwm_get_state(pwm, &cstate);
+	pwm_get_caps(chip, pwm, &caps);
 
 	if (state->enabled) {
 		if (cstate.enabled &&
 		    cstate.polarity == state->polarity &&
-		    cstate.period == state->period) {
+		    cstate.period == state->period &&
+		    cstate.mode == state->mode) {
 			cprd = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm,
 						  atmel_pwm->data->regs.period);
 			atmel_pwm_calculate_cdty(state, cprd, &cdty);
@@ -263,6 +269,18 @@ static int atmel_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 			val &= ~PWM_CMR_CPOL;
 		else
 			val |= PWM_CMR_CPOL;
+
+		/* PWM mode. */
+		if (caps.modes & PWM_MODE(PUSH_PULL)) {
+			if (state->mode == PWM_MODE(PUSH_PULL)) {
+				val |= PWM_CMR_PPM | PWM_CMR_DTLI;
+				val &= ~PWM_CMR_DTHI;
+			} else {
+				val &= ~(PWM_CMR_PPM | PWM_CMR_DTLI |
+					 PWM_CMR_DTHI);
+			}
+		}
+
 		atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, PWM_CMR, val);
 		atmel_pwm_set_cprd_cdty(chip, pwm, cprd, cdty);
 		mutex_lock(&atmel_pwm->isr_lock);
@@ -315,6 +333,20 @@ static const struct atmel_pwm_data atmel_pwm_data_v2 = {
 	},
 };
 
+static const struct atmel_pwm_data atmel_pwm_data_v3 = {
+	.regs = {
+		.period		= PWMV2_CPRD,
+		.period_upd	= PWMV2_CPRDUPD,
+		.duty		= PWMV2_CDTY,
+		.duty_upd	= PWMV2_CDTYUPD,
+	},
+	.caps = {
+		.modes = PWM_MODE(NORMAL) |
+			 PWM_MODE(COMPLEMENTARY) |
+			 PWM_MODE(PUSH_PULL),
+	},
+};
+
 static const struct platform_device_id atmel_pwm_devtypes[] = {
 	{
 		.name = "at91sam9rl-pwm",
@@ -337,7 +369,7 @@ static const struct of_device_id atmel_pwm_dt_ids[] = {
 		.data = &atmel_pwm_data_v2,
 	}, {
 		.compatible = "atmel,sama5d2-pwm",
-		.data = &atmel_pwm_data_v2,
+		.data = &atmel_pwm_data_v3,
 	}, {
 		/* sentinel */
 	},
-- 
2.7.4

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Daniel Thompson]

On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
> were adapted to this change.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +++++++++--
>  drivers/bus/ts-nbus.c                |  2 +-
>  drivers/clk/clk-pwm.c                |  3 ++-
>  drivers/gpu/drm/i915/intel_panel.c   | 17 ++++++++++++++---
>  drivers/hwmon/pwm-fan.c              |  2 +-
>  drivers/input/misc/max77693-haptic.c |  2 +-
>  drivers/input/misc/max8997_haptic.c  |  6 +++++-
>  drivers/leds/leds-pwm.c              |  5 ++++-
>  drivers/media/rc/ir-rx51.c           |  5 ++++-
>  drivers/media/rc/pwm-ir-tx.c         |  5 ++++-
>  drivers/video/backlight/lm3630a_bl.c |  4 +++-
>  drivers/video/backlight/lp855x_bl.c  |  4 +++-
>  drivers/video/backlight/lp8788_bl.c  |  5 ++++-
>  drivers/video/backlight/pwm_bl.c     | 11 +++++++++--
>  drivers/video/fbdev/ssd1307fb.c      |  3 ++-
>  include/linux/pwm.h                  |  6 ++++--
>  16 files changed, 70 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
> index 2030a6b77a09..696fa25dafd2 100644
> --- a/drivers/video/backlight/lm3630a_bl.c
> +++ b/drivers/video/backlight/lm3630a_bl.c
> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
>  {
>  	unsigned int period = pchip->pdata->pwm_period;
>  	unsigned int duty = br * period / br_max;
> +	struct pwm_caps caps = { };
>  
> -	pwm_config(pchip->pwmd, duty, period);
> +	pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, &caps);
> +	pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));

Well... I admit I've only really looked at the patches that impact 
backlight but dispersing this really odd looking bit twiddling 
throughout the kernel doesn't strike me a great API design.

IMHO callers should not be required to find the first set bit in
some specially crafted set of capability bits simply to get sane 
default behaviour.


Daniel.




>  	if (duty)
>  		pwm_enable(pchip->pwmd);
>  	else
> diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
> index 939f057836e1..3d274c604862 100644
> --- a/drivers/video/backlight/lp855x_bl.c
> +++ b/drivers/video/backlight/lp855x_bl.c
> @@ -240,6 +240,7 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
>  	unsigned int period = lp->pdata->period_ns;
>  	unsigned int duty = br * period / max_br;
>  	struct pwm_device *pwm;
> +	struct pwm_caps caps = { };
>  
>  	/* request pwm device with the consumer name */
>  	if (!lp->pwm) {
> @@ -256,7 +257,8 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
>  		pwm_apply_args(pwm);
>  	}
>  
> -	pwm_config(lp->pwm, duty, period);
> +	pwm_get_caps(lp->pwm->chip, lp->pwm, &caps);
> +	pwm_config(lp->pwm, duty, period, BIT(ffs(caps.modes) - 1));
>  	if (duty)
>  		pwm_enable(lp->pwm);
>  	else
> diff --git a/drivers/video/backlight/lp8788_bl.c b/drivers/video/backlight/lp8788_bl.c
> index cf869ec90cce..06de3163650d 100644
> --- a/drivers/video/backlight/lp8788_bl.c
> +++ b/drivers/video/backlight/lp8788_bl.c
> @@ -128,6 +128,7 @@ static void lp8788_pwm_ctrl(struct lp8788_bl *bl, int br, int max_br)
>  	unsigned int duty;
>  	struct device *dev;
>  	struct pwm_device *pwm;
> +	struct pwm_caps caps = { };
>  
>  	if (!bl->pdata)
>  		return;
> @@ -153,7 +154,9 @@ static void lp8788_pwm_ctrl(struct lp8788_bl *bl, int br, int max_br)
>  		pwm_apply_args(pwm);
>  	}
>  
> -	pwm_config(bl->pwm, duty, period);
> +	pwm_get_caps(bl->pwm->chip, bl->pwm, &caps);
> +
> +	pwm_config(bl->pwm, duty, period, BIT(ffs(caps.modes) - 1));
>  	if (duty)
>  		pwm_enable(bl->pwm);
>  	else
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index 1c2289ddd555..706a9ab053a7 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -63,10 +63,14 @@ static void pwm_backlight_power_on(struct pwm_bl_data *pb, int brightness)
>  
>  static void pwm_backlight_power_off(struct pwm_bl_data *pb)
>  {
> +	struct pwm_caps caps = { };
> +
>  	if (!pb->enabled)
>  		return;
>  
> -	pwm_config(pb->pwm, 0, pb->period);
> +	pwm_get_caps(pb->pwm->chip, pb->pwm, &caps);
> +
> +	pwm_config(pb->pwm, 0, pb->period, BIT(ffs(caps.modes) - 1));
>  	pwm_disable(pb->pwm);
>  
>  	if (pb->enable_gpio)
> @@ -96,6 +100,7 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
>  {
>  	struct pwm_bl_data *pb = bl_get_data(bl);
>  	int brightness = bl->props.brightness;
> +	struct pwm_caps caps = { };
>  	int duty_cycle;
>  
>  	if (bl->props.power != FB_BLANK_UNBLANK ||
> @@ -108,7 +113,9 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
>  
>  	if (brightness > 0) {
>  		duty_cycle = compute_duty_cycle(pb, brightness);
> -		pwm_config(pb->pwm, duty_cycle, pb->period);
> +		pwm_get_caps(pb->pwm->chip, pb->pwm, &caps);
> +		pwm_config(pb->pwm, duty_cycle, pb->period,
> +			   BIT(ffs(caps.modes) - 1));
>  		pwm_backlight_power_on(pb, brightness);
>  	} else
>  		pwm_backlight_power_off(pb);
> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> index f599520374dd..4b57dcb5799a 100644
> --- a/drivers/video/fbdev/ssd1307fb.c
> +++ b/drivers/video/fbdev/ssd1307fb.c
> @@ -308,7 +308,8 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
>  
>  		par->pwm_period = pargs.period;
>  		/* Enable the PWM */
> -		pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period);
> +		pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period,
> +			   pargs.mode);
>  		pwm_enable(par->pwm);
>  
>  		dev_dbg(&par->client->dev, "Using PWM%d with a %dns period.\n",
> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> index e62349f48129..0ba416ab2772 100644
> --- a/include/linux/pwm.h
> +++ b/include/linux/pwm.h
> @@ -357,11 +357,12 @@ int pwm_adjust_config(struct pwm_device *pwm);
>   * @pwm: PWM device
>   * @duty_ns: "on" time (in nanoseconds)
>   * @period_ns: duration (in nanoseconds) of one cycle
> + * @mode: PWM mode
>   *
>   * Returns: 0 on success or a negative error code on failure.
>   */
>  static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
> -			     int period_ns)
> +			     int period_ns, unsigned long mode)
>  {
>  	struct pwm_state state;
>  
> @@ -377,6 +378,7 @@ static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
>  
>  	state.duty_cycle = duty_ns;
>  	state.period = period_ns;
> +	state.mode = mode;
>  	return pwm_apply_state(pwm, &state);
>  }
>  
> @@ -537,7 +539,7 @@ static inline int pwm_adjust_config(struct pwm_device *pwm)
>  }
>  
>  static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
> -			     int period_ns)
> +			     int period_ns, unsigned long mode)
>  {
>  	return -EINVAL;
>  }
> -- 
> 2.7.4
> 

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Sean Young]

On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
> were adapted to this change.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---

-snip-

> diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
> index 49265f02e772..a971b02ea021 100644
> --- a/drivers/media/rc/ir-rx51.c
> +++ b/drivers/media/rc/ir-rx51.c
> @@ -55,10 +55,13 @@ static int init_timing_params(struct ir_rx51 *ir_rx51)
>  {
>  	struct pwm_device *pwm = ir_rx51->pwm;
>  	int duty, period = DIV_ROUND_CLOSEST(NSEC_PER_SEC, ir_rx51->freq);
> +	struct pwm_caps caps = { };
>  
>  	duty = DIV_ROUND_CLOSEST(ir_rx51->duty_cycle * period, 100);
>  
> -	pwm_config(pwm, duty, period);
> +	pwm_get_caps(pwm->chip, pwm, &caps);
> +
> +	pwm_config(pwm, duty, period, BIT(ffs(caps.modes) - 1));
>  
>  	return 0;
>  }
> diff --git a/drivers/media/rc/pwm-ir-tx.c b/drivers/media/rc/pwm-ir-tx.c
> index 27d0f5837a76..c630e1b450a3 100644
> --- a/drivers/media/rc/pwm-ir-tx.c
> +++ b/drivers/media/rc/pwm-ir-tx.c
> @@ -61,6 +61,7 @@ static int pwm_ir_tx(struct rc_dev *dev, unsigned int *txbuf,
>  {
>  	struct pwm_ir *pwm_ir = dev->priv;
>  	struct pwm_device *pwm = pwm_ir->pwm;
> +	struct pwm_caps caps = { };
>  	int i, duty, period;
>  	ktime_t edge;
>  	long delta;
> @@ -68,7 +69,9 @@ static int pwm_ir_tx(struct rc_dev *dev, unsigned int *txbuf,
>  	period = DIV_ROUND_CLOSEST(NSEC_PER_SEC, pwm_ir->carrier);
>  	duty = DIV_ROUND_CLOSEST(pwm_ir->duty_cycle * period, 100);
>  
> -	pwm_config(pwm, duty, period);
> +	pwm_get_caps(pwm->chip, pwm, &caps);
> +
> +	pwm_config(pwm, duty, period, BIT(ffs(caps.modes) - 1));
>  
>  	edge = ktime_get();
>  

The two PWM rc-core drivers need PWM_MODE(NORMAL), not the first available
mode that the device supports. If mode normal is not supported, then probe
should fail.


Sean

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Claudiu Beznea]

On 22.02.2018 14:33, Daniel Thompson wrote:
> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
>> were adapted to this change.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
>>  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +++++++++--
>>  drivers/bus/ts-nbus.c                |  2 +-
>>  drivers/clk/clk-pwm.c                |  3 ++-
>>  drivers/gpu/drm/i915/intel_panel.c   | 17 ++++++++++++++---
>>  drivers/hwmon/pwm-fan.c              |  2 +-
>>  drivers/input/misc/max77693-haptic.c |  2 +-
>>  drivers/input/misc/max8997_haptic.c  |  6 +++++-
>>  drivers/leds/leds-pwm.c              |  5 ++++-
>>  drivers/media/rc/ir-rx51.c           |  5 ++++-
>>  drivers/media/rc/pwm-ir-tx.c         |  5 ++++-
>>  drivers/video/backlight/lm3630a_bl.c |  4 +++-
>>  drivers/video/backlight/lp855x_bl.c  |  4 +++-
>>  drivers/video/backlight/lp8788_bl.c  |  5 ++++-
>>  drivers/video/backlight/pwm_bl.c     | 11 +++++++++--
>>  drivers/video/fbdev/ssd1307fb.c      |  3 ++-
>>  include/linux/pwm.h                  |  6 ++++--
>>  16 files changed, 70 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
>> index 2030a6b77a09..696fa25dafd2 100644
>> --- a/drivers/video/backlight/lm3630a_bl.c
>> +++ b/drivers/video/backlight/lm3630a_bl.c
>> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
>>  {
>>  	unsigned int period = pchip->pdata->pwm_period;
>>  	unsigned int duty = br * period / br_max;
>> +	struct pwm_caps caps = { };
>>  
>> -	pwm_config(pchip->pwmd, duty, period);
>> +	pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, &caps);
>> +	pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
> 
> Well... I admit I've only really looked at the patches that impact 
> backlight but dispersing this really odd looking bit twiddling 
> throughout the kernel doesn't strike me a great API design.>
> IMHO callers should not be required to find the first set bit in
> some specially crafted set of capability bits simply to get sane 
> default behaviour.
Thank you for your inputs. I will try to have a fix for this in next version.

The idea with this was to locate first available PWM mode of PWM channel. If the
driver hasn't registered any PWM capabilities related ops the default
capabilities will include only PWM normal mode. In case the PWM channel will
register different capabilities taking the first available mode from caps.modes
and passing it as argument to pwm_config() will ensure the pwm_apply_state()
will not fail.

Thank you,
Claudiu Beznea

> 
> 
> Daniel.
> 
> 
> 
> 
>>  	if (duty)
>>  		pwm_enable(pchip->pwmd);
>>  	else
>> diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
>> index 939f057836e1..3d274c604862 100644
>> --- a/drivers/video/backlight/lp855x_bl.c
>> +++ b/drivers/video/backlight/lp855x_bl.c
>> @@ -240,6 +240,7 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
>>  	unsigned int period = lp->pdata->period_ns;
>>  	unsigned int duty = br * period / max_br;
>>  	struct pwm_device *pwm;
>> +	struct pwm_caps caps = { };
>>  
>>  	/* request pwm device with the consumer name */
>>  	if (!lp->pwm) {
>> @@ -256,7 +257,8 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
>>  		pwm_apply_args(pwm);
>>  	}
>>  
>> -	pwm_config(lp->pwm, duty, period);
>> +	pwm_get_caps(lp->pwm->chip, lp->pwm, &caps);
>> +	pwm_config(lp->pwm, duty, period, BIT(ffs(caps.modes) - 1));
>>  	if (duty)
>>  		pwm_enable(lp->pwm);
>>  	else
>> diff --git a/drivers/video/backlight/lp8788_bl.c b/drivers/video/backlight/lp8788_bl.c
>> index cf869ec90cce..06de3163650d 100644
>> --- a/drivers/video/backlight/lp8788_bl.c
>> +++ b/drivers/video/backlight/lp8788_bl.c
>> @@ -128,6 +128,7 @@ static void lp8788_pwm_ctrl(struct lp8788_bl *bl, int br, int max_br)
>>  	unsigned int duty;
>>  	struct device *dev;
>>  	struct pwm_device *pwm;
>> +	struct pwm_caps caps = { };
>>  
>>  	if (!bl->pdata)
>>  		return;
>> @@ -153,7 +154,9 @@ static void lp8788_pwm_ctrl(struct lp8788_bl *bl, int br, int max_br)
>>  		pwm_apply_args(pwm);
>>  	}
>>  
>> -	pwm_config(bl->pwm, duty, period);
>> +	pwm_get_caps(bl->pwm->chip, bl->pwm, &caps);
>> +
>> +	pwm_config(bl->pwm, duty, period, BIT(ffs(caps.modes) - 1));
>>  	if (duty)
>>  		pwm_enable(bl->pwm);
>>  	else
>> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
>> index 1c2289ddd555..706a9ab053a7 100644
>> --- a/drivers/video/backlight/pwm_bl.c
>> +++ b/drivers/video/backlight/pwm_bl.c
>> @@ -63,10 +63,14 @@ static void pwm_backlight_power_on(struct pwm_bl_data *pb, int brightness)
>>  
>>  static void pwm_backlight_power_off(struct pwm_bl_data *pb)
>>  {
>> +	struct pwm_caps caps = { };
>> +
>>  	if (!pb->enabled)
>>  		return;
>>  
>> -	pwm_config(pb->pwm, 0, pb->period);
>> +	pwm_get_caps(pb->pwm->chip, pb->pwm, &caps);
>> +
>> +	pwm_config(pb->pwm, 0, pb->period, BIT(ffs(caps.modes) - 1));
>>  	pwm_disable(pb->pwm);
>>  
>>  	if (pb->enable_gpio)
>> @@ -96,6 +100,7 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
>>  {
>>  	struct pwm_bl_data *pb = bl_get_data(bl);
>>  	int brightness = bl->props.brightness;
>> +	struct pwm_caps caps = { };
>>  	int duty_cycle;
>>  
>>  	if (bl->props.power != FB_BLANK_UNBLANK ||
>> @@ -108,7 +113,9 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
>>  
>>  	if (brightness > 0) {
>>  		duty_cycle = compute_duty_cycle(pb, brightness);
>> -		pwm_config(pb->pwm, duty_cycle, pb->period);
>> +		pwm_get_caps(pb->pwm->chip, pb->pwm, &caps);
>> +		pwm_config(pb->pwm, duty_cycle, pb->period,
>> +			   BIT(ffs(caps.modes) - 1));
>>  		pwm_backlight_power_on(pb, brightness);
>>  	} else
>>  		pwm_backlight_power_off(pb);
>> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
>> index f599520374dd..4b57dcb5799a 100644
>> --- a/drivers/video/fbdev/ssd1307fb.c
>> +++ b/drivers/video/fbdev/ssd1307fb.c
>> @@ -308,7 +308,8 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
>>  
>>  		par->pwm_period = pargs.period;
>>  		/* Enable the PWM */
>> -		pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period);
>> +		pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period,
>> +			   pargs.mode);
>>  		pwm_enable(par->pwm);
>>  
>>  		dev_dbg(&par->client->dev, "Using PWM%d with a %dns period.\n",
>> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
>> index e62349f48129..0ba416ab2772 100644
>> --- a/include/linux/pwm.h
>> +++ b/include/linux/pwm.h
>> @@ -357,11 +357,12 @@ int pwm_adjust_config(struct pwm_device *pwm);
>>   * @pwm: PWM device
>>   * @duty_ns: "on" time (in nanoseconds)
>>   * @period_ns: duration (in nanoseconds) of one cycle
>> + * @mode: PWM mode
>>   *
>>   * Returns: 0 on success or a negative error code on failure.
>>   */
>>  static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
>> -			     int period_ns)
>> +			     int period_ns, unsigned long mode)
>>  {
>>  	struct pwm_state state;
>>  
>> @@ -377,6 +378,7 @@ static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
>>  
>>  	state.duty_cycle = duty_ns;
>>  	state.period = period_ns;
>> +	state.mode = mode;
>>  	return pwm_apply_state(pwm, &state);
>>  }
>>  
>> @@ -537,7 +539,7 @@ static inline int pwm_adjust_config(struct pwm_device *pwm)
>>  }
>>  
>>  static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
>> -			     int period_ns)
>> +			     int period_ns, unsigned long mode)
>>  {
>>  	return -EINVAL;
>>  }
>> -- 
>> 2.7.4
>>
> 

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Claudiu Beznea]

On 22.02.2018 15:01, Sean Young wrote:
> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
>> were adapted to this change.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
> 
> -snip-
> 
>> diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
>> index 49265f02e772..a971b02ea021 100644
>> --- a/drivers/media/rc/ir-rx51.c
>> +++ b/drivers/media/rc/ir-rx51.c
>> @@ -55,10 +55,13 @@ static int init_timing_params(struct ir_rx51 *ir_rx51)
>>  {
>>  	struct pwm_device *pwm = ir_rx51->pwm;
>>  	int duty, period = DIV_ROUND_CLOSEST(NSEC_PER_SEC, ir_rx51->freq);
>> +	struct pwm_caps caps = { };
>>  
>>  	duty = DIV_ROUND_CLOSEST(ir_rx51->duty_cycle * period, 100);
>>  
>> -	pwm_config(pwm, duty, period);
>> +	pwm_get_caps(pwm->chip, pwm, &caps);
>> +
>> +	pwm_config(pwm, duty, period, BIT(ffs(caps.modes) - 1));
>>  
>>  	return 0;
>>  }
>> diff --git a/drivers/media/rc/pwm-ir-tx.c b/drivers/media/rc/pwm-ir-tx.c
>> index 27d0f5837a76..c630e1b450a3 100644
>> --- a/drivers/media/rc/pwm-ir-tx.c
>> +++ b/drivers/media/rc/pwm-ir-tx.c
>> @@ -61,6 +61,7 @@ static int pwm_ir_tx(struct rc_dev *dev, unsigned int *txbuf,
>>  {
>>  	struct pwm_ir *pwm_ir = dev->priv;
>>  	struct pwm_device *pwm = pwm_ir->pwm;
>> +	struct pwm_caps caps = { };
>>  	int i, duty, period;
>>  	ktime_t edge;
>>  	long delta;
>> @@ -68,7 +69,9 @@ static int pwm_ir_tx(struct rc_dev *dev, unsigned int *txbuf,
>>  	period = DIV_ROUND_CLOSEST(NSEC_PER_SEC, pwm_ir->carrier);
>>  	duty = DIV_ROUND_CLOSEST(pwm_ir->duty_cycle * period, 100);
>>  
>> -	pwm_config(pwm, duty, period);
>> +	pwm_get_caps(pwm->chip, pwm, &caps);
>> +
>> +	pwm_config(pwm, duty, period, BIT(ffs(caps.modes) - 1));
>>  
>>  	edge = ktime_get();
>>  
> 
> The two PWM rc-core drivers need PWM_MODE(NORMAL), not the first available
> mode that the device supports. If mode normal is not supported, then probe
> should fail.
OK, thank you for your inputs. I will address this in next version.


Thank you,
Claudiu Beznea
> 
> 
> Sean
> 

Re: [PATCH v3 06/10] pwm: add PWM modes

[Andy Shevchenko]

On Thu, Feb 22, 2018 at 2:01 PM, Claudiu Beznea
<claudiu.beznea@microchip.com> wrote:
> Add PWM normal and complementary modes.

> +- PWM_DTMODE_COMPLEMENTARY: PWM complementary working mode (for PWM
> +channels two outputs); if not specified, the default for PWM channel will
> +be used

What DT stands for?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 06/10] pwm: add PWM modes

[Claudiu Beznea]

On 22.02.2018 19:28, Andy Shevchenko wrote:
> On Thu, Feb 22, 2018 at 2:01 PM, Claudiu Beznea
> <claudiu.beznea@microchip.com> wrote:
>> Add PWM normal and complementary modes.
> 
>> +- PWM_DTMODE_COMPLEMENTARY: PWM complementary working mode (for PWM
>> +channels two outputs); if not specified, the default for PWM channel will
>> +be used
> 
> What DT stands for?
It stands for Device Tree. It remained this way from the previous version. In
the previous version I had modes described in an enum, to be used by PWM core,
as follows:
enum pwm_mode {
	PWM_MODE_NORMAL,
	PWM_MODE_COMPLEMENTARY,
};

and, to avoid conflict b/w these defines and the one from
include/dt-bindings/pwm/pwm.h I introduced this DT in the define names from
dt-bindings. But now the DT might be removed since I've changed the way the PWM
mode is identified in PWM core. I will remove the DT in the next version, if not
requested otherwise.

Thank you,
Claudiu Benea

Re: [PATCH v3 01/10] pwm: extend PWM framework with PWM modes

[kbuild test robot]

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

Hi Claudiu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on pwm/for-next]
[also build test WARNING on v4.16-rc2 next-20180223]
[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/Claudiu-Beznea/extend-PWM-framework-to-support-PWM-modes/20180225-024011
base:   https://git.kernel.org/pub/scm/linux/kernel/git/thierry.reding/linux-pwm.git for-next
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 7.2.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
        make.cross ARCH=xtensa 

All warnings (new ones prefixed by >>):

>> drivers//pwm/pwm-sun4i.c:36:0: warning: "PWM_MODE" redefined
    #define PWM_MODE  BIT(7)
    
   In file included from drivers//pwm/pwm-sun4i.c:19:0:
   include/linux/pwm.h:40:0: note: this is the location of the previous definition
    #define PWM_MODE(name)  BIT(PWM_MODE_##name##_BIT)
    

vim +/PWM_MODE +36 drivers//pwm/pwm-sun4i.c

09853ce7 Alexandre Belloni 2014-12-17  29  
09853ce7 Alexandre Belloni 2014-12-17  30  #define PWMCH_OFFSET		15
09853ce7 Alexandre Belloni 2014-12-17  31  #define PWM_PRESCAL_MASK	GENMASK(3, 0)
09853ce7 Alexandre Belloni 2014-12-17  32  #define PWM_PRESCAL_OFF		0
09853ce7 Alexandre Belloni 2014-12-17  33  #define PWM_EN			BIT(4)
09853ce7 Alexandre Belloni 2014-12-17  34  #define PWM_ACT_STATE		BIT(5)
09853ce7 Alexandre Belloni 2014-12-17  35  #define PWM_CLK_GATING		BIT(6)
09853ce7 Alexandre Belloni 2014-12-17 @36  #define PWM_MODE		BIT(7)
09853ce7 Alexandre Belloni 2014-12-17  37  #define PWM_PULSE		BIT(8)
09853ce7 Alexandre Belloni 2014-12-17  38  #define PWM_BYPASS		BIT(9)
09853ce7 Alexandre Belloni 2014-12-17  39  

:::::: The code at line 36 was first introduced by commit
:::::: 09853ce7bc1003a490c7ee74a5705d7a7cf16b7d pwm: Add Allwinner SoC support

:::::: TO: Alexandre Belloni <alexandre.belloni@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: 52627 bytes --]

Re: [PATCH v3 01/10] pwm: extend PWM framework with PWM modes

[Claudiu Beznea]

I'll rebase it on latest for-next in next version.

Thank you,
Claudiu Beznea

On 24.02.2018 22:49, kbuild test robot wrote:
> Hi Claudiu,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on pwm/for-next]
> [also build test WARNING on v4.16-rc2 next-20180223]
> [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/Claudiu-Beznea/extend-PWM-framework-to-support-PWM-modes/20180225-024011
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/thierry.reding/linux-pwm.git for-next
> config: xtensa-allmodconfig (attached as .config)
> compiler: xtensa-linux-gcc (GCC) 7.2.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
>         make.cross ARCH=xtensa 
> 
> All warnings (new ones prefixed by >>):
> 
>>> drivers//pwm/pwm-sun4i.c:36:0: warning: "PWM_MODE" redefined
>     #define PWM_MODE  BIT(7)
>     
>    In file included from drivers//pwm/pwm-sun4i.c:19:0:
>    include/linux/pwm.h:40:0: note: this is the location of the previous definition
>     #define PWM_MODE(name)  BIT(PWM_MODE_##name##_BIT)
>     
> 
> vim +/PWM_MODE +36 drivers//pwm/pwm-sun4i.c
> 
> 09853ce7 Alexandre Belloni 2014-12-17  29  
> 09853ce7 Alexandre Belloni 2014-12-17  30  #define PWMCH_OFFSET		15
> 09853ce7 Alexandre Belloni 2014-12-17  31  #define PWM_PRESCAL_MASK	GENMASK(3, 0)
> 09853ce7 Alexandre Belloni 2014-12-17  32  #define PWM_PRESCAL_OFF		0
> 09853ce7 Alexandre Belloni 2014-12-17  33  #define PWM_EN			BIT(4)
> 09853ce7 Alexandre Belloni 2014-12-17  34  #define PWM_ACT_STATE		BIT(5)
> 09853ce7 Alexandre Belloni 2014-12-17  35  #define PWM_CLK_GATING		BIT(6)
> 09853ce7 Alexandre Belloni 2014-12-17 @36  #define PWM_MODE		BIT(7)
> 09853ce7 Alexandre Belloni 2014-12-17  37  #define PWM_PULSE		BIT(8)
> 09853ce7 Alexandre Belloni 2014-12-17  38  #define PWM_BYPASS		BIT(9)
> 09853ce7 Alexandre Belloni 2014-12-17  39  
> 
> :::::: The code at line 36 was first introduced by commit
> :::::: 09853ce7bc1003a490c7ee74a5705d7a7cf16b7d pwm: Add Allwinner SoC support
> 
> :::::: TO: Alexandre Belloni <alexandre.belloni@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
> 

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Jani Nikula]

On Thu, 22 Feb 2018, Daniel Thompson <daniel.thompson@linaro.org> wrote:
> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
>> were adapted to this change.
>> 
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
>>  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +++++++++--
>>  drivers/bus/ts-nbus.c                |  2 +-
>>  drivers/clk/clk-pwm.c                |  3 ++-
>>  drivers/gpu/drm/i915/intel_panel.c   | 17 ++++++++++++++---
>>  drivers/hwmon/pwm-fan.c              |  2 +-
>>  drivers/input/misc/max77693-haptic.c |  2 +-
>>  drivers/input/misc/max8997_haptic.c  |  6 +++++-
>>  drivers/leds/leds-pwm.c              |  5 ++++-
>>  drivers/media/rc/ir-rx51.c           |  5 ++++-
>>  drivers/media/rc/pwm-ir-tx.c         |  5 ++++-
>>  drivers/video/backlight/lm3630a_bl.c |  4 +++-
>>  drivers/video/backlight/lp855x_bl.c  |  4 +++-
>>  drivers/video/backlight/lp8788_bl.c  |  5 ++++-
>>  drivers/video/backlight/pwm_bl.c     | 11 +++++++++--
>>  drivers/video/fbdev/ssd1307fb.c      |  3 ++-
>>  include/linux/pwm.h                  |  6 ++++--
>>  16 files changed, 70 insertions(+), 21 deletions(-)
>> 
>> diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
>> index 2030a6b77a09..696fa25dafd2 100644
>> --- a/drivers/video/backlight/lm3630a_bl.c
>> +++ b/drivers/video/backlight/lm3630a_bl.c
>> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
>>  {
>>  	unsigned int period = pchip->pdata->pwm_period;
>>  	unsigned int duty = br * period / br_max;
>> +	struct pwm_caps caps = { };
>>  
>> -	pwm_config(pchip->pwmd, duty, period);
>> +	pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, &caps);
>> +	pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
>
> Well... I admit I've only really looked at the patches that impact 
> backlight but dispersing this really odd looking bit twiddling 
> throughout the kernel doesn't strike me a great API design.
>
> IMHO callers should not be required to find the first set bit in
> some specially crafted set of capability bits simply to get sane 
> default behaviour.

Agreed. IMHO the regular use case becomes rather tedious, ugly, and
error prone.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Claudiu Beznea]

On 26.02.2018 11:57, Jani Nikula wrote:
> On Thu, 22 Feb 2018, Daniel Thompson <daniel.thompson@linaro.org> wrote:
>> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
>>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
>>> were adapted to this change.
>>>
>>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>>> ---
>>>  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +++++++++--
>>>  drivers/bus/ts-nbus.c                |  2 +-
>>>  drivers/clk/clk-pwm.c                |  3 ++-
>>>  drivers/gpu/drm/i915/intel_panel.c   | 17 ++++++++++++++---
>>>  drivers/hwmon/pwm-fan.c              |  2 +-
>>>  drivers/input/misc/max77693-haptic.c |  2 +-
>>>  drivers/input/misc/max8997_haptic.c  |  6 +++++-
>>>  drivers/leds/leds-pwm.c              |  5 ++++-
>>>  drivers/media/rc/ir-rx51.c           |  5 ++++-
>>>  drivers/media/rc/pwm-ir-tx.c         |  5 ++++-
>>>  drivers/video/backlight/lm3630a_bl.c |  4 +++-
>>>  drivers/video/backlight/lp855x_bl.c  |  4 +++-
>>>  drivers/video/backlight/lp8788_bl.c  |  5 ++++-
>>>  drivers/video/backlight/pwm_bl.c     | 11 +++++++++--
>>>  drivers/video/fbdev/ssd1307fb.c      |  3 ++-
>>>  include/linux/pwm.h                  |  6 ++++--
>>>  16 files changed, 70 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
>>> index 2030a6b77a09..696fa25dafd2 100644
>>> --- a/drivers/video/backlight/lm3630a_bl.c
>>> +++ b/drivers/video/backlight/lm3630a_bl.c
>>> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
>>>  {
>>>  	unsigned int period = pchip->pdata->pwm_period;
>>>  	unsigned int duty = br * period / br_max;
>>> +	struct pwm_caps caps = { };
>>>  
>>> -	pwm_config(pchip->pwmd, duty, period);
>>> +	pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, &caps);
>>> +	pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
>>
>> Well... I admit I've only really looked at the patches that impact 
>> backlight but dispersing this really odd looking bit twiddling 
>> throughout the kernel doesn't strike me a great API design.
>>
>> IMHO callers should not be required to find the first set bit in
>> some specially crafted set of capability bits simply to get sane 
>> default behaviour.
> 
> Agreed. IMHO the regular use case becomes rather tedious, ugly, and
> error prone.

Using simply PWM_MODE(NORMAL) instead of BIT(ffs(caps.modes) - 1) would be OK
from your side?

Or, what about using a function like pwm_mode_first() to get the first supported
mode by PWM channel?

Or, would you prefer to solve this inside pwm_config() function, let's say, in
case an invalid mode is passed as argument, to let pwm_config() to choose the
first available PWM mode for PWM channel passed as argument?

Thank you,
Claudiu Beznea

> 
> BR,
> Jani.
> 
> 

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Daniel Thompson]

On Mon, Feb 26, 2018 at 04:24:15PM +0200, Claudiu Beznea wrote:
> On 26.02.2018 11:57, Jani Nikula wrote:
> > On Thu, 22 Feb 2018, Daniel Thompson <daniel.thompson@linaro.org> wrote:
> >> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
> >>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
> >>> were adapted to this change.
> >>>
> >>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> >>> ---
> >>>  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +++++++++--
> >>>  drivers/bus/ts-nbus.c                |  2 +-
> >>>  drivers/clk/clk-pwm.c                |  3 ++-
> >>>  drivers/gpu/drm/i915/intel_panel.c   | 17 ++++++++++++++---
> >>>  drivers/hwmon/pwm-fan.c              |  2 +-
> >>>  drivers/input/misc/max77693-haptic.c |  2 +-
> >>>  drivers/input/misc/max8997_haptic.c  |  6 +++++-
> >>>  drivers/leds/leds-pwm.c              |  5 ++++-
> >>>  drivers/media/rc/ir-rx51.c           |  5 ++++-
> >>>  drivers/media/rc/pwm-ir-tx.c         |  5 ++++-
> >>>  drivers/video/backlight/lm3630a_bl.c |  4 +++-
> >>>  drivers/video/backlight/lp855x_bl.c  |  4 +++-
> >>>  drivers/video/backlight/lp8788_bl.c  |  5 ++++-
> >>>  drivers/video/backlight/pwm_bl.c     | 11 +++++++++--
> >>>  drivers/video/fbdev/ssd1307fb.c      |  3 ++-
> >>>  include/linux/pwm.h                  |  6 ++++--
> >>>  16 files changed, 70 insertions(+), 21 deletions(-)
> >>>
> >>> diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
> >>> index 2030a6b77a09..696fa25dafd2 100644
> >>> --- a/drivers/video/backlight/lm3630a_bl.c
> >>> +++ b/drivers/video/backlight/lm3630a_bl.c
> >>> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
> >>>  {
> >>>  	unsigned int period = pchip->pdata->pwm_period;
> >>>  	unsigned int duty = br * period / br_max;
> >>> +	struct pwm_caps caps = { };
> >>>  
> >>> -	pwm_config(pchip->pwmd, duty, period);
> >>> +	pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, &caps);
> >>> +	pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
> >>
> >> Well... I admit I've only really looked at the patches that impact 
> >> backlight but dispersing this really odd looking bit twiddling 
> >> throughout the kernel doesn't strike me a great API design.
> >>
> >> IMHO callers should not be required to find the first set bit in
> >> some specially crafted set of capability bits simply to get sane 
> >> default behaviour.
> > 
> > Agreed. IMHO the regular use case becomes rather tedious, ugly, and
> > error prone.
> 
> Using simply PWM_MODE(NORMAL) instead of BIT(ffs(caps.modes) - 1) would be OK
> from your side?
>
> Or, what about using a function like pwm_mode_first() to get the first supported
> mode by PWM channel?
> 
> Or, would you prefer to solve this inside pwm_config() function, let's say, in
> case an invalid mode is passed as argument, to let pwm_config() to choose the
> first available PWM mode for PWM channel passed as argument?

What is it that actually needs solving?

If a driver requests normal mode and the PWM driver cannot support it
why not just return an error an move on.

Put another way, what is the use case for secretly adopting a mode the
caller didn't want? Under what circumstances is this a good thing?


Daniel.

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Claudiu Beznea]

On 27.02.2018 12:54, Daniel Thompson wrote:
> On Mon, Feb 26, 2018 at 04:24:15PM +0200, Claudiu Beznea wrote:
>> On 26.02.2018 11:57, Jani Nikula wrote:
>>> On Thu, 22 Feb 2018, Daniel Thompson <daniel.thompson@linaro.org> wrote:
>>>> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
>>>>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
>>>>> were adapted to this change.
>>>>>
>>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>>>>> ---
>>>>>  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +++++++++--
>>>>>  drivers/bus/ts-nbus.c                |  2 +-
>>>>>  drivers/clk/clk-pwm.c                |  3 ++-
>>>>>  drivers/gpu/drm/i915/intel_panel.c   | 17 ++++++++++++++---
>>>>>  drivers/hwmon/pwm-fan.c              |  2 +-
>>>>>  drivers/input/misc/max77693-haptic.c |  2 +-
>>>>>  drivers/input/misc/max8997_haptic.c  |  6 +++++-
>>>>>  drivers/leds/leds-pwm.c              |  5 ++++-
>>>>>  drivers/media/rc/ir-rx51.c           |  5 ++++-
>>>>>  drivers/media/rc/pwm-ir-tx.c         |  5 ++++-
>>>>>  drivers/video/backlight/lm3630a_bl.c |  4 +++-
>>>>>  drivers/video/backlight/lp855x_bl.c  |  4 +++-
>>>>>  drivers/video/backlight/lp8788_bl.c  |  5 ++++-
>>>>>  drivers/video/backlight/pwm_bl.c     | 11 +++++++++--
>>>>>  drivers/video/fbdev/ssd1307fb.c      |  3 ++-
>>>>>  include/linux/pwm.h                  |  6 ++++--
>>>>>  16 files changed, 70 insertions(+), 21 deletions(-)
>>>>>
>>>>> diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
>>>>> index 2030a6b77a09..696fa25dafd2 100644
>>>>> --- a/drivers/video/backlight/lm3630a_bl.c
>>>>> +++ b/drivers/video/backlight/lm3630a_bl.c
>>>>> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
>>>>>  {
>>>>>  	unsigned int period = pchip->pdata->pwm_period;
>>>>>  	unsigned int duty = br * period / br_max;
>>>>> +	struct pwm_caps caps = { };
>>>>>  
>>>>> -	pwm_config(pchip->pwmd, duty, period);
>>>>> +	pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, &caps);
>>>>> +	pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
>>>>
>>>> Well... I admit I've only really looked at the patches that impact 
>>>> backlight but dispersing this really odd looking bit twiddling 
>>>> throughout the kernel doesn't strike me a great API design.
>>>>
>>>> IMHO callers should not be required to find the first set bit in
>>>> some specially crafted set of capability bits simply to get sane 
>>>> default behaviour.
>>>
>>> Agreed. IMHO the regular use case becomes rather tedious, ugly, and
>>> error prone.
>>
>> Using simply PWM_MODE(NORMAL) instead of BIT(ffs(caps.modes) - 1) would be OK
>> from your side?
>>
>> Or, what about using a function like pwm_mode_first() to get the first supported
>> mode by PWM channel?
>>
>> Or, would you prefer to solve this inside pwm_config() function, let's say, in
>> case an invalid mode is passed as argument, to let pwm_config() to choose the
>> first available PWM mode for PWM channel passed as argument?
> 
> What is it that actually needs solving?
> 
> If a driver requests normal mode and the PWM driver cannot support it
> why not just return an error an move on.
Because, simply, I wasn't aware of what these PWM client drivers needs for.

> 
> Put another way, what is the use case for secretly adopting a mode the
> caller didn't want? Under what circumstances is this a good thing?
No one... But I wasn't aware of what the PWM clients needs for from their PWM
controllers. At this moment having BIT(ffs(caps.modes)) instead of
PWM_MODE(NORMAL) is mostly the same since all the driver that has not explicitly
registered PWM caps will use PWM normal mode.

I will use PWM_MODE(NORMAL) instead of this in all the cases if this is OK from
your side.

Thank you,
Claudiu Beznea
> 
> 
> Daniel.
> 

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Daniel Thompson]

On Tue, Feb 27, 2018 at 01:40:58PM +0200, Claudiu Beznea wrote:
> On 27.02.2018 12:54, Daniel Thompson wrote:
> > On Mon, Feb 26, 2018 at 04:24:15PM +0200, Claudiu Beznea wrote:
> >> On 26.02.2018 11:57, Jani Nikula wrote:
> >>> On Thu, 22 Feb 2018, Daniel Thompson <daniel.thompson@linaro.org> wrote:
> >>>> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
> >>>>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
> >>>>> were adapted to this change.
> >>>>>
> >>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> >>>>> ---
> >>>>>  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +++++++++--
> >>>>>  drivers/bus/ts-nbus.c                |  2 +-
> >>>>>  drivers/clk/clk-pwm.c                |  3 ++-
> >>>>>  drivers/gpu/drm/i915/intel_panel.c   | 17 ++++++++++++++---
> >>>>>  drivers/hwmon/pwm-fan.c              |  2 +-
> >>>>>  drivers/input/misc/max77693-haptic.c |  2 +-
> >>>>>  drivers/input/misc/max8997_haptic.c  |  6 +++++-
> >>>>>  drivers/leds/leds-pwm.c              |  5 ++++-
> >>>>>  drivers/media/rc/ir-rx51.c           |  5 ++++-
> >>>>>  drivers/media/rc/pwm-ir-tx.c         |  5 ++++-
> >>>>>  drivers/video/backlight/lm3630a_bl.c |  4 +++-
> >>>>>  drivers/video/backlight/lp855x_bl.c  |  4 +++-
> >>>>>  drivers/video/backlight/lp8788_bl.c  |  5 ++++-
> >>>>>  drivers/video/backlight/pwm_bl.c     | 11 +++++++++--
> >>>>>  drivers/video/fbdev/ssd1307fb.c      |  3 ++-
> >>>>>  include/linux/pwm.h                  |  6 ++++--
> >>>>>  16 files changed, 70 insertions(+), 21 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
> >>>>> index 2030a6b77a09..696fa25dafd2 100644
> >>>>> --- a/drivers/video/backlight/lm3630a_bl.c
> >>>>> +++ b/drivers/video/backlight/lm3630a_bl.c
> >>>>> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
> >>>>>  {
> >>>>>  	unsigned int period = pchip->pdata->pwm_period;
> >>>>>  	unsigned int duty = br * period / br_max;
> >>>>> +	struct pwm_caps caps = { };
> >>>>>  
> >>>>> -	pwm_config(pchip->pwmd, duty, period);
> >>>>> +	pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, &caps);
> >>>>> +	pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
> >>>>
> >>>> Well... I admit I've only really looked at the patches that impact 
> >>>> backlight but dispersing this really odd looking bit twiddling 
> >>>> throughout the kernel doesn't strike me a great API design.
> >>>>
> >>>> IMHO callers should not be required to find the first set bit in
> >>>> some specially crafted set of capability bits simply to get sane 
> >>>> default behaviour.
> >>>
> >>> Agreed. IMHO the regular use case becomes rather tedious, ugly, and
> >>> error prone.
> >>
> >> Using simply PWM_MODE(NORMAL) instead of BIT(ffs(caps.modes) - 1) would be OK
> >> from your side?
> >>
> >> Or, what about using a function like pwm_mode_first() to get the first supported
> >> mode by PWM channel?
> >>
> >> Or, would you prefer to solve this inside pwm_config() function, let's say, in
> >> case an invalid mode is passed as argument, to let pwm_config() to choose the
> >> first available PWM mode for PWM channel passed as argument?
> > 
> > What is it that actually needs solving?
> > 
> > If a driver requests normal mode and the PWM driver cannot support it
> > why not just return an error an move on.
> Because, simply, I wasn't aware of what these PWM client drivers needs for.

I'm afraid you have confused me here.

Didn't you just *add* the whole concept of PWM caps with your patches?
How could any existing call site expect anything except normal mode.
Until now there has been no possiblity to request anything else.


> > Put another way, what is the use case for secretly adopting a mode the
> > caller didn't want? Under what circumstances is this a good thing?
> No one... But I wasn't aware of what the PWM clients needs for from their PWM
> controllers. At this moment having BIT(ffs(caps.modes)) instead of
> PWM_MODE(NORMAL) is mostly the same since all the driver that has not explicitly
> registered PWM caps will use PWM normal mode.
> 
> I will use PWM_MODE(NORMAL) instead of this in all the cases if this is OK from
> your side.
> 
> Thank you,
> Claudiu Beznea
> > 
> > 
> > Daniel.
> > 

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Claudiu Beznea]

On 27.02.2018 17:38, Daniel Thompson wrote:
> On Tue, Feb 27, 2018 at 01:40:58PM +0200, Claudiu Beznea wrote:
>> On 27.02.2018 12:54, Daniel Thompson wrote:
>>> On Mon, Feb 26, 2018 at 04:24:15PM +0200, Claudiu Beznea wrote:
>>>> On 26.02.2018 11:57, Jani Nikula wrote:
>>>>> On Thu, 22 Feb 2018, Daniel Thompson <daniel.thompson@linaro.org> wrote:
>>>>>> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
>>>>>>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
>>>>>>> were adapted to this change.
>>>>>>>
>>>>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>>>>>>> ---
>>>>>>>  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +++++++++--
>>>>>>>  drivers/bus/ts-nbus.c                |  2 +-
>>>>>>>  drivers/clk/clk-pwm.c                |  3 ++-
>>>>>>>  drivers/gpu/drm/i915/intel_panel.c   | 17 ++++++++++++++---
>>>>>>>  drivers/hwmon/pwm-fan.c              |  2 +-
>>>>>>>  drivers/input/misc/max77693-haptic.c |  2 +-
>>>>>>>  drivers/input/misc/max8997_haptic.c  |  6 +++++-
>>>>>>>  drivers/leds/leds-pwm.c              |  5 ++++-
>>>>>>>  drivers/media/rc/ir-rx51.c           |  5 ++++-
>>>>>>>  drivers/media/rc/pwm-ir-tx.c         |  5 ++++-
>>>>>>>  drivers/video/backlight/lm3630a_bl.c |  4 +++-
>>>>>>>  drivers/video/backlight/lp855x_bl.c  |  4 +++-
>>>>>>>  drivers/video/backlight/lp8788_bl.c  |  5 ++++-
>>>>>>>  drivers/video/backlight/pwm_bl.c     | 11 +++++++++--
>>>>>>>  drivers/video/fbdev/ssd1307fb.c      |  3 ++-
>>>>>>>  include/linux/pwm.h                  |  6 ++++--
>>>>>>>  16 files changed, 70 insertions(+), 21 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
>>>>>>> index 2030a6b77a09..696fa25dafd2 100644
>>>>>>> --- a/drivers/video/backlight/lm3630a_bl.c
>>>>>>> +++ b/drivers/video/backlight/lm3630a_bl.c
>>>>>>> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
>>>>>>>  {
>>>>>>>  	unsigned int period = pchip->pdata->pwm_period;
>>>>>>>  	unsigned int duty = br * period / br_max;
>>>>>>> +	struct pwm_caps caps = { };
>>>>>>>  
>>>>>>> -	pwm_config(pchip->pwmd, duty, period);
>>>>>>> +	pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, &caps);
>>>>>>> +	pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
>>>>>>
>>>>>> Well... I admit I've only really looked at the patches that impact 
>>>>>> backlight but dispersing this really odd looking bit twiddling 
>>>>>> throughout the kernel doesn't strike me a great API design.
>>>>>>
>>>>>> IMHO callers should not be required to find the first set bit in
>>>>>> some specially crafted set of capability bits simply to get sane 
>>>>>> default behaviour.
>>>>>
>>>>> Agreed. IMHO the regular use case becomes rather tedious, ugly, and
>>>>> error prone.
>>>>
>>>> Using simply PWM_MODE(NORMAL) instead of BIT(ffs(caps.modes) - 1) would be OK
>>>> from your side?
>>>>
>>>> Or, what about using a function like pwm_mode_first() to get the first supported
>>>> mode by PWM channel?
>>>>
>>>> Or, would you prefer to solve this inside pwm_config() function, let's say, in
>>>> case an invalid mode is passed as argument, to let pwm_config() to choose the
>>>> first available PWM mode for PWM channel passed as argument?
>>>
>>> What is it that actually needs solving?
>>>
>>> If a driver requests normal mode and the PWM driver cannot support it
>>> why not just return an error an move on.
>> Because, simply, I wasn't aware of what these PWM client drivers needs for.
> 
> I'm afraid you have confused me here.
> 
> Didn't you just *add* the whole concept of PWM caps with your patches?
> How could any existing call site expect anything except normal mode.
> Until now there has been no possiblity to request anything else.
Agree. And agree I was confusing in previous email, sorry about that. And
agree that there was nothing before and everything should work with PWM
normal mode.

When I choose to have BIT(ffs(caps.modes)) instead of PWM_MODE(NORMAL) I
was thinking at having these pwm_config() calls working all the time having
in mind that in future the PWM controllers that these drivers use, might
change in terms of PWM supported modes.

Thank you,
Claudiu Beznea

> 
> 
>>> Put another way, what is the use case for secretly adopting a mode the
>>> caller didn't want? Under what circumstances is this a good thing?
>> No one... But I wasn't aware of what the PWM clients needs for from their PWM
>> controllers. At this moment having BIT(ffs(caps.modes)) instead of
>> PWM_MODE(NORMAL) is mostly the same since all the driver that has not explicitly
>> registered PWM caps will use PWM normal mode.
>>
>> I will use PWM_MODE(NORMAL) instead of this in all the cases if this is OK from
>> your side.
>>
>> Thank you,
>> Claudiu Beznea
>>>
>>>
>>> Daniel.
>>>
> 

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Thierry Reding]

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

On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
> were adapted to this change.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +++++++++--
>  drivers/bus/ts-nbus.c                |  2 +-
>  drivers/clk/clk-pwm.c                |  3 ++-
>  drivers/gpu/drm/i915/intel_panel.c   | 17 ++++++++++++++---
>  drivers/hwmon/pwm-fan.c              |  2 +-
>  drivers/input/misc/max77693-haptic.c |  2 +-
>  drivers/input/misc/max8997_haptic.c  |  6 +++++-
>  drivers/leds/leds-pwm.c              |  5 ++++-
>  drivers/media/rc/ir-rx51.c           |  5 ++++-
>  drivers/media/rc/pwm-ir-tx.c         |  5 ++++-
>  drivers/video/backlight/lm3630a_bl.c |  4 +++-
>  drivers/video/backlight/lp855x_bl.c  |  4 +++-
>  drivers/video/backlight/lp8788_bl.c  |  5 ++++-
>  drivers/video/backlight/pwm_bl.c     | 11 +++++++++--
>  drivers/video/fbdev/ssd1307fb.c      |  3 ++-
>  include/linux/pwm.h                  |  6 ++++--
>  16 files changed, 70 insertions(+), 21 deletions(-)

I don't think it makes sense to leak mode support into the legacy API.
The pwm_config() function is considered legacy and should eventually go
away. As such it doesn't make sense to integrate a new feature such as
PWM modes into it. All users of pwm_config() assume normal mode, and
that's what pwm_config() should provide.

Anyone that needs something other than normal mode should use the new
atomic PWM API.

Thierry

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

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Jani Nikula]

On Wed, 28 Feb 2018, Thierry Reding <thierry.reding@gmail.com> wrote:
> Anyone that needs something other than normal mode should use the new
> atomic PWM API.

At the risk of revealing my true ignorance, what is the new atomic PWM
API? Where? Examples of how one would convert old code over to the new
API?

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Claudiu Beznea]

On 28.02.2018 21:44, Thierry Reding wrote:
> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
>> were adapted to this change.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
>>  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +++++++++--
>>  drivers/bus/ts-nbus.c                |  2 +-
>>  drivers/clk/clk-pwm.c                |  3 ++-
>>  drivers/gpu/drm/i915/intel_panel.c   | 17 ++++++++++++++---
>>  drivers/hwmon/pwm-fan.c              |  2 +-
>>  drivers/input/misc/max77693-haptic.c |  2 +-
>>  drivers/input/misc/max8997_haptic.c  |  6 +++++-
>>  drivers/leds/leds-pwm.c              |  5 ++++-
>>  drivers/media/rc/ir-rx51.c           |  5 ++++-
>>  drivers/media/rc/pwm-ir-tx.c         |  5 ++++-
>>  drivers/video/backlight/lm3630a_bl.c |  4 +++-
>>  drivers/video/backlight/lp855x_bl.c  |  4 +++-
>>  drivers/video/backlight/lp8788_bl.c  |  5 ++++-
>>  drivers/video/backlight/pwm_bl.c     | 11 +++++++++--
>>  drivers/video/fbdev/ssd1307fb.c      |  3 ++-
>>  include/linux/pwm.h                  |  6 ++++--
>>  16 files changed, 70 insertions(+), 21 deletions(-)
> 
> I don't think it makes sense to leak mode support into the legacy API.
> The pwm_config() function is considered legacy
I missed this aspect.

 and should eventually go
> away. As such it doesn't make sense to integrate a new feature such as
> PWM modes into it. 
Agree.

All users of pwm_config() assume normal mode, and
> that's what pwm_config() should provide.
Agree.

> 
> Anyone that needs something other than normal mode should use the new
> atomic PWM API.
Agree.

> 
> Thierry
> 

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Claudiu Beznea]

On 28.02.2018 22:04, Jani Nikula wrote:
> On Wed, 28 Feb 2018, Thierry Reding <thierry.reding@gmail.com> wrote:
>> Anyone that needs something other than normal mode should use the new
>> atomic PWM API.
> 
> At the risk of revealing my true ignorance, what is the new atomic PWM
> API? Where? Examples of how one would convert old code over to the new
> API?
As far as I know, the old PWM core code uses config(), set_polarity(),
enable(), disable() methods of driver, registered as pwm_ops:
struct pwm_ops {

        int (*request)(struct pwm_chip *chip, struct pwm_device *pwm);

        void (*free)(struct pwm_chip *chip, struct pwm_device *pwm);

        int (*config)(struct pwm_chip *chip, struct pwm_device *pwm,

                      int duty_ns, int period_ns);

        int (*set_polarity)(struct pwm_chip *chip, struct pwm_device *pwm,

                            enum pwm_polarity polarity);

        int (*capture)(struct pwm_chip *chip, struct pwm_device *pwm,

                       struct pwm_capture *result, unsigned long timeout);

        int (*enable)(struct pwm_chip *chip, struct pwm_device *pwm);

        void (*disable)(struct pwm_chip *chip, struct pwm_device *pwm);

        int (*apply)(struct pwm_chip *chip, struct pwm_device *pwm,

                     struct pwm_state *state);

        void (*get_state)(struct pwm_chip *chip, struct pwm_device *pwm,

                          struct pwm_state *state);

#ifdef CONFIG_DEBUG_FS

        void (*dbg_show)(struct pwm_chip *chip, struct seq_file *s);

#endif

        struct module *owner;

};


to do settings on hardware. In order to so settings on a PWM the users
should have been follow the below steps:
->config()
->set_polarity()
->enable()
Moreover, if the PWM was previously enabled it should have been first
disable and then to follow the above steps in order to apply a new settings
on hardware.
The driver should have been provide, at probe, all the above function:
->config(), ->set_polarity(), ->disable(), ->enable(), function that were
used by PWM core.

Now, having atomic PWM, the driver should provide one function to PWM core,
which is ->apply() function. Every PWM has a state associated, which keeps
the period, duty cycle, polarity and enable/disable status. The driver's
->apply() function takes as argument the state that should be applied and
it takes care of applying this new state directly without asking user to
call ->disable(), then ->config()/->set_polarity(), then ->enable() to
apply new hardware settings.

The PWM consumer could set a new state for PWM it uses, using
pwm_apply_state(pwm, new_state);

Regarding the models to switch on atomic PWM, on the controller side you
can check for drivers that registers apply function at probe time.
Regarding the PWM users, you can look for pwm_apply_state()
(drivers/hwmon/pwm-fan.c or drivers/input/misc/pwm-beeper.c are some examples).

Thierry, please correct me if I'm wrong.

Thank you,
Claudiu Beznea

> 
> BR,
> Jani.
>