From: Claudiu Beznea <Claudiu.Beznea@microchip.com>
To: Daniel Thompson <daniel.thompson@linaro.org>
Cc: mark.rutland@arm.com, milo.kim@ti.com,
linux-fbdev@vger.kernel.org, sean@mess.org,
devicetree@vger.kernel.org, airlied@linux.ie,
mturquette@baylibre.com, joonas.lahtinen@linux.intel.com,
kamil@wypas.org, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org, thierry.reding@gmail.com,
alexandre.belloni@free-electrons.com, pavel@ucw.cz,
lee.jones@linaro.org, linux-clk@vger.kernel.org,
linux-leds@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
shc_work@mail.ru, corbet@lwn.net, linux-doc@vger.kernel.org,
linux@armlinux.org.uk, krzk@kernel.org, kgene@kernel.org,
linux-input@vger.kernel.org, linux@roeck-us.net,
linux-media@vger.kernel.org, linux-pwm@vger.kernel.org,
jdelvare@suse.com, b.zolnierkie@samsung.com,
intel-gfx@lists.freedesktop.org, jani.nikula@linux.intel.com,
robh+dt@kernel.org, jacek.anaszewski@gmail.com,
rodrigo.vivi@intel.com, mchehab@kernel.orglin
Subject: Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()
Date: Thu, 22 Feb 2018 15:21:19 +0200 [thread overview]
Message-ID: <27bab524-3c94-1149-e3de-880cd7dde352@microchip.com> (raw)
In-Reply-To: <20180222123308.mypx2r7n6o63mj5z@oak.lan>
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
>>
>
next prev parent reply other threads:[~2018-02-22 13:21 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-22 12:01 [PATCH v3 00/10] extend PWM framework to support PWM modes Claudiu Beznea
2018-02-22 12:01 ` [PATCH v3 01/10] pwm: extend PWM framework with " Claudiu Beznea
2018-02-24 20:49 ` kbuild test robot
2018-02-26 8:11 ` Claudiu Beznea
2018-02-22 12:01 ` [PATCH v3 02/10] pwm: clps711x: populate PWM mode in of_xlate function Claudiu Beznea
2018-02-22 12:01 ` [PATCH v3 03/10] pwm: cros-ec: " Claudiu Beznea
2018-02-22 12:01 ` [PATCH v3 04/10] pwm: pxa: " Claudiu Beznea
2018-02-22 12:01 ` [PATCH v3 05/10] pwm: add PWM mode to pwm_config() Claudiu Beznea
2018-02-22 12:33 ` Daniel Thompson
2018-02-22 13:21 ` Claudiu Beznea [this message]
2018-02-26 9:57 ` Jani Nikula
2018-02-26 14:24 ` Claudiu Beznea
2018-02-27 10:54 ` Daniel Thompson
2018-02-27 11:40 ` Claudiu Beznea
2018-02-27 15:38 ` Daniel Thompson
2018-02-27 16:15 ` Claudiu Beznea
2018-02-22 13:01 ` Sean Young
2018-02-22 13:23 ` Claudiu Beznea
2018-02-28 19:44 ` Thierry Reding
2018-02-28 20:04 ` Jani Nikula
2018-03-02 9:28 ` Claudiu Beznea
2018-03-02 9:19 ` Claudiu Beznea
2018-02-22 12:01 ` [PATCH v3 06/10] pwm: add PWM modes Claudiu Beznea
2018-02-22 17:28 ` Andy Shevchenko
2018-02-22 17:42 ` Claudiu Beznea
2018-02-22 12:01 ` [PATCH v3 07/10] pwm: atmel: add pwm capabilities Claudiu Beznea
2018-02-22 12:01 ` [PATCH v3 08/10] pwm: add push-pull mode support Claudiu Beznea
2018-02-22 12:01 ` [PATCH v3 09/10] pwm: add documentation for pwm push-pull mode Claudiu Beznea
2018-02-22 12:01 ` [PATCH v3 10/10] pwm: atmel: add push-pull mode support Claudiu Beznea
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=27bab524-3c94-1149-e3de-880cd7dde352@microchip.com \
--to=claudiu.beznea@microchip.com \
--cc=airlied@linux.ie \
--cc=alexandre.belloni@free-electrons.com \
--cc=b.zolnierkie@samsung.com \
--cc=corbet@lwn.net \
--cc=daniel.thompson@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jacek.anaszewski@gmail.com \
--cc=jani.nikula@linux.intel.com \
--cc=jdelvare@suse.com \
--cc=joonas.lahtinen@linux.intel.com \
--cc=kamil@wypas.org \
--cc=kgene@kernel.org \
--cc=krzk@kernel.org \
--cc=lee.jones@linaro.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-pwm@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=linux@roeck-us.net \
--cc=mark.rutland@arm.com \
--cc=mchehab@kernel.orglin \
--cc=milo.kim@ti.com \
--cc=mturquette@baylibre.com \
--cc=pavel@ucw.cz \
--cc=robh+dt@kernel.org \
--cc=rodrigo.vivi@intel.com \
--cc=sean@mess.org \
--cc=shc_work@mail.ru \
--cc=thierry.reding@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox