On 26.02.2018 11:57, Jani Nikula wrote:

On Thu, 22 Feb 2018, Daniel Thompson [off-list ref] wrote:

quoted

On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:

quoted

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

Signed-off-by: Claudiu Beznea <redacted>
---
 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.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx