* Re: likely signedness bug in drm and nvidia drivers
From: Ilia Mirkin @ 2015-07-21 2:44 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: linux-fbdev, dri-devel@lists.freedesktop.org, Tomi Valkeinen,
Ben Skeggs
In-Reply-To: <87fv4io8ug.fsf@rasmusvillemoes.dk>
I think you're right. The intent is to mask off the bits above
bits_per_pixel. So if bits_per_pixel is 24, the mask would be
0xff000000. If it's 16, then the mask would be 0xffff0000. If it's 32,
then the mask is 0.
In reality, bits_per_pixel is almost exclusively 32, which will end up
with a mask of 0 (note that the shift result is inverted at the end).
So for the majority case, there's not bug... just a useless operation.
I took a look at linux/bitops.h, and there's nothing particularly
great there. GENMASK, I guess, but it's not quite right. Just
switching to 0U should be fine there.
Now that I think about it, I believe these patches have already been
sent in the past. I also just did the grep that I did before:
$ git grep -- '~0 >>'
arch/m32r/include/asm/thread_info.h: ti->flags = (ti->flags & (~0
>> (32 - TI_FLAG_FAULT_CODE_SHIFT)))
arch/sh/include/asm/thread_info.h: ti->flags = (ti->flags & (~0
>> (32 - TI_FLAG_FAULT_CODE_SHIFT)))
drivers/gpu/drm/nouveau/nv50_fbcon.c: uint32_t mask = ~(~0 >> (32 -
info->var.bits_per_pixel));
drivers/gpu/drm/nouveau/nvc0_fbcon.c: uint32_t mask = ~(~0 >> (32 -
info->var.bits_per_pixel));
drivers/video/fbdev/nvidia/nv_accel.c: u32 fg, bg, mask = ~(~0 >> (32
- info->var.bits_per_pixel));
which shows that these are the only ones. See email with subject
"[PATCH] nvidia/noveau: Fix color mask" from June 17, 2015.
-ilia
On Mon, Jul 20, 2015 at 4:46 PM, Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
> Hi,
>
> The files
>
> drivers/gpu/drm/nouveau/nv50_fbcon.c
> drivers/gpu/drm/nouveau/nvc0_fbcon.c
> drivers/video/fbdev/nvidia/nv_accel.c
>
> all contain a right-shift of ~0 (aka -1) - just grep for '~0 >>'. gcc
> always does arithmetic right shift of signed types, which means that the
> result is always -1 again [type promotion/conversion doesn't kick in
> until after the shift subexpression has been evaluated], independent of
> the second operand. I can hardly believe that is intended (the result is
> used as a mask, which thus consists of all 1s). If these are indeed
> bugs, the patch is obvious (just make the literal an unsigned 0).
>
> Rasmus
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH] backlight: pm8941-wled: Move PM8941 WLED driver to backlight
From: Jingoo Han @ 2015-07-21 0:47 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: Bjorn Andersson, Jacek Anaszewski, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala, Bryan Wu, Richard Purdie,
Lee Jones, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Rob Clark, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-leds@vger.kernel.org,
linux-fbdev@vger.kernel.org, linux-arm-msm@vger.kernel.org,
jingoohan1
In-Reply-To: <55AD63FF.8090408@gmail.com>
On 2015. 7. 21., at AM 6:11, Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:
>> On 20.07.2015 16:15, Jingoo Han wrote:
>>> On Thursday, July 16, 2015 5:01 PM, Jacek Anaszewski wrote:
>>>> On 07/15/2015 10:02 PM, Bjorn Andersson wrote:
>>>> The Qualcomm PM8941 WLED block is used for backlight and should therefor
>>>> be in the backlight framework and not in the LED framework. This moves
>>>> the driver and adapts to the backlight api instead.
>>>>
>>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
>>>> ---
>>>> .../devicetree/bindings/leds/leds-pm8941-wled.txt | 5 +-
>>>> drivers/leds/Kconfig | 8 ---
>>>> drivers/leds/Makefile | 1 -
>>>> drivers/video/backlight/Kconfig | 8 +++
>>>> drivers/video/backlight/Makefile | 1 +
>>>> .../backlight/pm8941-wled.c} | 59 ++++++++++------------
>>>> 6 files changed, 36 insertions(+), 46 deletions(-)
>>>> rename drivers/{leds/leds-pm8941-wled.c => video/backlight/pm8941-wled.c} (90%)
>>>
>>> Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com>
>>
>> If you are not an author of this driver or a maintainer affected by this patch,
>> Just 'Reviewed-by' looks good.
>
> I am a LED subsystem maintainer.
OK, I see.
I didn't notice that you were added as the maintainer 2 months ago.
Best regards,
Jingoo Han
^ permalink raw reply
* Re: [PATCH] backlight: pm8941-wled: Move PM8941 WLED driver to backlight
From: Jacek Anaszewski @ 2015-07-20 21:11 UTC (permalink / raw)
To: Jingoo Han, 'Bjorn Andersson'
Cc: 'Jacek Anaszewski', 'Rob Herring',
'Pawel Moll', 'Mark Rutland',
'Ian Campbell', 'Kumar Gala', 'Bryan Wu',
'Richard Purdie', 'Lee Jones',
'Jean-Christophe Plagniol-Villard',
'Tomi Valkeinen', 'Rob Clark', devicetree,
linux-kernel, linux-leds, linux-fbdev, linux-arm-msm
In-Reply-To: <004401d0c2f6$9b73b820$d25b2860$@com>
On 20.07.2015 16:15, Jingoo Han wrote:
> On Thursday, July 16, 2015 5:01 PM, Jacek Anaszewski wrote:
>> On 07/15/2015 10:02 PM, Bjorn Andersson wrote:
>>> The Qualcomm PM8941 WLED block is used for backlight and should therefor
>>> be in the backlight framework and not in the LED framework. This moves
>>> the driver and adapts to the backlight api instead.
>>>
>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
>>> ---
>>> .../devicetree/bindings/leds/leds-pm8941-wled.txt | 5 +-
>>> drivers/leds/Kconfig | 8 ---
>>> drivers/leds/Makefile | 1 -
>>> drivers/video/backlight/Kconfig | 8 +++
>>> drivers/video/backlight/Makefile | 1 +
>>> .../backlight/pm8941-wled.c} | 59 ++++++++++------------
>>> 6 files changed, 36 insertions(+), 46 deletions(-)
>>> rename drivers/{leds/leds-pm8941-wled.c => video/backlight/pm8941-wled.c} (90%)
>>
>> Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com>
>
> If you are not an author of this driver or a maintainer affected by this patch,
> Just 'Reviewed-by' looks good.
I am a LED subsystem maintainer.
--
Best Regards,
Jacek Anaszewski
^ permalink raw reply
* likely signedness bug in drm and nvidia drivers
From: Rasmus Villemoes @ 2015-07-20 20:46 UTC (permalink / raw)
To: Ben Skeggs, David Airlie, Antonino Daplas, Tomi Valkeinen
Cc: linux-fbdev, dri-devel
Hi,
The files
drivers/gpu/drm/nouveau/nv50_fbcon.c
drivers/gpu/drm/nouveau/nvc0_fbcon.c
drivers/video/fbdev/nvidia/nv_accel.c
all contain a right-shift of ~0 (aka -1) - just grep for '~0 >>'. gcc
always does arithmetic right shift of signed types, which means that the
result is always -1 again [type promotion/conversion doesn't kick in
until after the shift subexpression has been evaluated], independent of
the second operand. I can hardly believe that is intended (the result is
used as a mask, which thus consists of all 1s). If these are indeed
bugs, the patch is obvious (just make the literal an unsigned 0).
Rasmus
^ permalink raw reply
* Re: [PATCH v2 00/10] pwm: add support for atomic update
From: Boris Brezillon @ 2015-07-20 20:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20150720200819.GN11162@sirena.org.uk>
On Mon, 20 Jul 2015 21:08:19 +0100
Mark Brown <broonie@kernel.org> wrote:
> On Mon, Jul 20, 2015 at 09:49:13PM +0200, Boris Brezillon wrote:
> > Mark Brown <broonie@kernel.org> wrote:
> > > On Mon, Jul 20, 2015 at 05:31:57PM +0200, Boris Brezillon wrote:
>
> > > > - dropped pwm-regulator patches (should be submitted separately)
>
> > > I think the second patch needs to go in with this series doesn't it? It
> > > was just the enable one that was good to go immediately IIRC.
>
> > I was planning on submitting those two patches after the PWM changes
> > have been merged, but you're right, maybe we should think about a proper
> > way to smoothly get all of them in the same release.
>
> I see. We should at least try to get the enable one in this time, I'd
Actually I was wrong about that patch: it depends on patch 1/15 of the
RFC ("pwm: add the pwm_is_enabled() helper").
> like to just apply it and then either do a cross tree merge for the one
> making use of the new functionality or (if the rest of the series gets
> held up for some reason) just letting it get merged via Linus' tree.
>
> > I have rebased my work on top your regulator/topic/pwm branch containing
> > Lee's work [1].
>
> > Note that patch 1 has changed a bit to take Lee's additions into
> > account.
>
> > Thierry, could you create a branch based on Mark's regulator/topic/pwm
> > branch ?
> > If everybody agrees on the solution I'll send a v3 rebasing my work on
> > top of this topic branch.
>
> I think that's fine for me, I can tag the PWM branch in my tree and
> Thierry can pull that.
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v2 00/10] pwm: add support for atomic update
From: Mark Brown @ 2015-07-20 20:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20150720214913.73c47fa6@bbrezillon>
[-- Attachment #1: Type: text/plain, Size: 1328 bytes --]
On Mon, Jul 20, 2015 at 09:49:13PM +0200, Boris Brezillon wrote:
> Mark Brown <broonie@kernel.org> wrote:
> > On Mon, Jul 20, 2015 at 05:31:57PM +0200, Boris Brezillon wrote:
> > > - dropped pwm-regulator patches (should be submitted separately)
> > I think the second patch needs to go in with this series doesn't it? It
> > was just the enable one that was good to go immediately IIRC.
> I was planning on submitting those two patches after the PWM changes
> have been merged, but you're right, maybe we should think about a proper
> way to smoothly get all of them in the same release.
I see. We should at least try to get the enable one in this time, I'd
like to just apply it and then either do a cross tree merge for the one
making use of the new functionality or (if the rest of the series gets
held up for some reason) just letting it get merged via Linus' tree.
> I have rebased my work on top your regulator/topic/pwm branch containing
> Lee's work [1].
> Note that patch 1 has changed a bit to take Lee's additions into
> account.
> Thierry, could you create a branch based on Mark's regulator/topic/pwm
> branch ?
> If everybody agrees on the solution I'll send a v3 rebasing my work on
> top of this topic branch.
I think that's fine for me, I can tag the PWM branch in my tree and
Thierry can pull that.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH v2 00/10] pwm: add support for atomic update
From: Boris Brezillon @ 2015-07-20 19:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20150720171950.GE11162@sirena.org.uk>
Hi Mark,
On Mon, 20 Jul 2015 18:19:50 +0100
Mark Brown <broonie@kernel.org> wrote:
> On Mon, Jul 20, 2015 at 05:31:57PM +0200, Boris Brezillon wrote:
>
> > - dropped pwm-regulator patches (should be submitted separately)
>
> I think the second patch needs to go in with this series doesn't it? It
> was just the enable one that was good to go immediately IIRC.
I was planning on submitting those two patches after the PWM changes
have been merged, but you're right, maybe we should think about a proper
way to smoothly get all of them in the same release.
I have rebased my work on top your regulator/topic/pwm branch containing
Lee's work [1].
Note that patch 1 has changed a bit to take Lee's additions into
account.
Thierry, could you create a branch based on Mark's regulator/topic/pwm
branch ?
If everybody agrees on the solution I'll send a v3 rebasing my work on
top of this topic branch.
Best Regards,
Boris
[1]https://github.com/bbrezillon/linux-rk/tree/atomic-pwm-v3
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH] backlight: pm8941-wled: Move PM8941 WLED driver to backlight
From: Bjorn Andersson @ 2015-07-20 17:45 UTC (permalink / raw)
To: Jingoo Han
Cc: 'Rob Herring', 'Pawel Moll',
'Mark Rutland', 'Ian Campbell',
'Kumar Gala', 'Bryan Wu',
'Richard Purdie', 'Jacek Anaszewski',
'Lee Jones', 'Jean-Christophe Plagniol-Villard',
'Tomi Valkeinen', 'Rob Clark',
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-leds-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <004501d0c2f7$7ee51f40$7caf5dc0$@com>
On Mon 20 Jul 07:22 PDT 2015, Jingoo Han wrote:
> On Thursday, July 16, 2015 5:02 AM, Bjorn Andersson wrote:
> >
> > The Qualcomm PM8941 WLED block is used for backlight and should therefor
> > be in the backlight framework and not in the LED framework. This moves
> > the driver and adapts to the backlight api instead.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> > ---
> > .../devicetree/bindings/leds/leds-pm8941-wled.txt | 5 +-
> > drivers/leds/Kconfig | 8 ---
> > drivers/leds/Makefile | 1 -
> > drivers/video/backlight/Kconfig | 8 +++
> > drivers/video/backlight/Makefile | 1 +
> > .../backlight/pm8941-wled.c} | 59 ++++++++++------------
> > 6 files changed, 36 insertions(+), 46 deletions(-)
> > rename drivers/{leds/leds-pm8941-wled.c => video/backlight/pm8941-wled.c} (90%)
> >
> > diff --git a/Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt
> > b/Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt
> > index a85a964d61f5..424f8444a6cd 100644
> > --- a/Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt
> > +++ b/Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt
>
> Please move this txt file to 'backlight' directory.
>
Of course
[..]
> > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> > index 0505b796d743..6c67c5430933 100644
> > --- a/drivers/video/backlight/Kconfig
> > +++ b/drivers/video/backlight/Kconfig
> > @@ -299,6 +299,14 @@ config BACKLIGHT_TOSA
> > If you have an Sharp SL-6000 Zaurus say Y to enable a driver
> > for its backlight
> >
> > +config BACKLIGHT_PM8941_WLED
> > + tristate "Qualcomm PM8941 WLED Driver"
> > + depends on LEDS_CLASS
>
> LEDS_CLASS?
> Please check your patch carefully.
>
Sorry about that.
> One more thing,
> Did you test this patch with the board?
> If not, I will not accept this patch, unless other people send 'Tested-by'.
I've tested the driver on the Sony Xperia Z3 and it visually behaves as
expected.
I'll send you an updated patch shortly.
Thank you,
Bjorn
^ permalink raw reply
* Re: [PATCH v2 00/10] pwm: add support for atomic update
From: Mark Brown @ 2015-07-20 17:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1437406327-6207-1-git-send-email-boris.brezillon@free-electrons.com>
[-- Attachment #1: Type: text/plain, Size: 269 bytes --]
On Mon, Jul 20, 2015 at 05:31:57PM +0200, Boris Brezillon wrote:
> - dropped pwm-regulator patches (should be submitted separately)
I think the second patch needs to go in with this series doesn't it? It
was just the enable one that was good to go immediately IIRC.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* [PATCH v2 10/10] pwm: rockchip: add support for atomic update
From: Boris Brezillon @ 2015-07-20 15:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1437406327-6207-1-git-send-email-boris.brezillon@free-electrons.com>
Implement the ->apply() function to add support for atomic update.
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
drivers/pwm/pwm-rockchip.c | 53 +++++++++++++++++++++++++---------------------
1 file changed, 29 insertions(+), 24 deletions(-)
diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index 6eab25c..48b814d 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -50,7 +50,8 @@ struct rockchip_pwm_data {
const struct pwm_ops *ops;
void (*set_enable)(struct pwm_chip *chip,
- struct pwm_device *pwm, bool enable);
+ struct pwm_device *pwm, bool enable,
+ enum pwm_polarity polarity);
void (*reset_state)(struct pwm_chip *chip, struct pwm_device *pwm);
};
@@ -60,7 +61,8 @@ static inline struct rockchip_pwm_chip *to_rockchip_pwm_chip(struct pwm_chip *c)
}
static void rockchip_pwm_set_enable_v1(struct pwm_chip *chip,
- struct pwm_device *pwm, bool enable)
+ struct pwm_device *pwm, bool enable,
+ enum pwm_polarity polarity)
{
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
u32 enable_conf = PWM_CTRL_OUTPUT_EN | PWM_CTRL_TIMER_EN;
@@ -89,14 +91,15 @@ static void rockchip_pwm_reset_state_v1(struct pwm_chip *chip,
}
static void rockchip_pwm_set_enable_v2(struct pwm_chip *chip,
- struct pwm_device *pwm, bool enable)
+ struct pwm_device *pwm, bool enable,
+ enum pwm_polarity polarity)
{
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
u32 enable_conf = PWM_OUTPUT_LEFT | PWM_LP_DISABLE | PWM_ENABLE |
PWM_CONTINUOUS;
u32 val;
- if (pwm_get_polarity(pwm) = PWM_POLARITY_INVERSED)
+ if (polarity = PWM_POLARITY_INVERSED)
enable_conf |= PWM_DUTY_NEGATIVE | PWM_INACTIVE_POSITIVE;
else
enable_conf |= PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE;
@@ -165,7 +168,6 @@ static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
unsigned long period, duty;
u64 clk_rate, div;
- int ret;
clk_rate = clk_get_rate(pc->clk);
@@ -182,15 +184,8 @@ static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
do_div(div, pc->data->prescaler * NSEC_PER_SEC);
duty = div;
- ret = clk_enable(pc->clk);
- if (ret)
- return ret;
-
writel(period, pc->base + pc->data->regs.period);
writel(duty, pc->base + pc->data->regs.duty);
- writel(0, pc->base + pc->data->regs.cntr);
-
- clk_disable(pc->clk);
return 0;
}
@@ -208,43 +203,53 @@ static int rockchip_pwm_set_polarity(struct pwm_chip *chip,
return 0;
}
-static int rockchip_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
+static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+ const struct pwm_state *state)
{
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
+ bool enabled = pwm_is_enabled(pwm);
int ret;
ret = clk_enable(pc->clk);
if (ret)
return ret;
- pc->data->set_enable(chip, pwm, true);
+ if (state->polarity != pwm_get_polarity(pwm) && enabled) {
+ pc->data->set_enable(chip, pwm, false, state->polarity);
+ enabled = false;
+ }
- return 0;
-}
+ ret = rockchip_pwm_config(chip, pwm, state->duty_cycle, state->period);
+ if (ret) {
+ if (enabled != pwm_is_enabled(pwm))
+ pc->data->set_enable(chip, pwm, !enabled,
+ state->polarity);
-static void rockchip_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
-{
- struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
+ goto out;
+ }
- pc->data->set_enable(chip, pwm, false);
+ if (state->enabled != enabled)
+ pc->data->set_enable(chip, pwm, state->enabled,
+ state->polarity);
+out:
clk_disable(pc->clk);
+
+ return ret;
}
static const struct pwm_ops rockchip_pwm_ops_v1 = {
.reset_state = rockchip_pwm_reset_state,
.config = rockchip_pwm_config,
- .enable = rockchip_pwm_enable,
- .disable = rockchip_pwm_disable,
+ .apply = rockchip_pwm_apply,
.owner = THIS_MODULE,
};
static const struct pwm_ops rockchip_pwm_ops_v2 = {
.reset_state = rockchip_pwm_reset_state,
.config = rockchip_pwm_config,
+ .apply = rockchip_pwm_apply,
.set_polarity = rockchip_pwm_set_polarity,
- .enable = rockchip_pwm_enable,
- .disable = rockchip_pwm_disable,
.owner = THIS_MODULE,
};
--
1.9.1
^ permalink raw reply related
* [PATCH v2 09/10] pwm: rockchip: add initial state retrieval
From: Boris Brezillon @ 2015-07-20 15:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1437406327-6207-1-git-send-email-boris.brezillon@free-electrons.com>
Implement the ->reset_state() function to expose initial state.
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
drivers/pwm/pwm-rockchip.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index 7d9cc90..6eab25c 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -51,6 +51,7 @@ struct rockchip_pwm_data {
void (*set_enable)(struct pwm_chip *chip,
struct pwm_device *pwm, bool enable);
+ void (*reset_state)(struct pwm_chip *chip, struct pwm_device *pwm);
};
static inline struct rockchip_pwm_chip *to_rockchip_pwm_chip(struct pwm_chip *c)
@@ -75,6 +76,18 @@ static void rockchip_pwm_set_enable_v1(struct pwm_chip *chip,
writel_relaxed(val, pc->base + pc->data->regs.ctrl);
}
+static void rockchip_pwm_reset_state_v1(struct pwm_chip *chip,
+ struct pwm_device *pwm)
+{
+ struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
+ u32 enable_conf = PWM_CTRL_OUTPUT_EN | PWM_CTRL_TIMER_EN;
+ u32 val;
+
+ val = readl(pc->base + pc->data->regs.ctrl);
+ if ((val & enable_conf) = enable_conf)
+ pwm->state.enabled = true;
+}
+
static void rockchip_pwm_set_enable_v2(struct pwm_chip *chip,
struct pwm_device *pwm, bool enable)
{
@@ -98,6 +111,54 @@ static void rockchip_pwm_set_enable_v2(struct pwm_chip *chip,
writel_relaxed(val, pc->base + pc->data->regs.ctrl);
}
+static void rockchip_pwm_reset_state_v2(struct pwm_chip *chip,
+ struct pwm_device *pwm)
+{
+ struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
+ u32 enable_conf = PWM_OUTPUT_LEFT | PWM_LP_DISABLE | PWM_ENABLE |
+ PWM_CONTINUOUS;
+ u32 val;
+
+ val = readl(pc->base + pc->data->regs.ctrl);
+ if ((val & enable_conf) != enable_conf)
+ return;
+
+ pwm->state.enabled = true;
+
+ if (!(val & PWM_DUTY_POSITIVE))
+ pwm->state.polarity = PWM_POLARITY_INVERSED;
+}
+
+static void rockchip_pwm_reset_state(struct pwm_chip *chip,
+ struct pwm_device *pwm)
+{
+ struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
+ unsigned long clk_rate;
+ u64 tmp;
+ int ret;
+
+ ret = clk_enable(pc->clk);
+ if (ret)
+ return;
+
+ clk_rate = clk_get_rate(pc->clk);
+
+ tmp = readl(pc->base + pc->data->regs.period);
+ tmp *= pc->data->prescaler * NSEC_PER_SEC;
+ do_div(tmp, clk_rate);
+ pwm->state.period = tmp;
+
+ tmp = readl(pc->base + pc->data->regs.duty);
+ tmp *= pc->data->prescaler * NSEC_PER_SEC;
+ do_div(tmp, clk_rate);
+ pwm->state.duty_cycle = tmp;
+
+ pc->data->reset_state(chip, chip->pwms);
+
+ if (!pwm_is_enabled(pwm))
+ clk_disable(pc->clk);
+}
+
static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
int duty_ns, int period_ns)
{
@@ -171,6 +232,7 @@ static void rockchip_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
}
static const struct pwm_ops rockchip_pwm_ops_v1 = {
+ .reset_state = rockchip_pwm_reset_state,
.config = rockchip_pwm_config,
.enable = rockchip_pwm_enable,
.disable = rockchip_pwm_disable,
@@ -178,6 +240,7 @@ static const struct pwm_ops rockchip_pwm_ops_v1 = {
};
static const struct pwm_ops rockchip_pwm_ops_v2 = {
+ .reset_state = rockchip_pwm_reset_state,
.config = rockchip_pwm_config,
.set_polarity = rockchip_pwm_set_polarity,
.enable = rockchip_pwm_enable,
@@ -195,6 +258,7 @@ static const struct rockchip_pwm_data pwm_data_v1 = {
.prescaler = 2,
.ops = &rockchip_pwm_ops_v1,
.set_enable = rockchip_pwm_set_enable_v1,
+ .reset_state = rockchip_pwm_reset_state_v1,
};
static const struct rockchip_pwm_data pwm_data_v2 = {
@@ -207,6 +271,7 @@ static const struct rockchip_pwm_data pwm_data_v2 = {
.prescaler = 1,
.ops = &rockchip_pwm_ops_v2,
.set_enable = rockchip_pwm_set_enable_v2,
+ .reset_state = rockchip_pwm_reset_state_v2,
};
static const struct rockchip_pwm_data pwm_data_vop = {
@@ -219,6 +284,7 @@ static const struct rockchip_pwm_data pwm_data_vop = {
.prescaler = 1,
.ops = &rockchip_pwm_ops_v2,
.set_enable = rockchip_pwm_set_enable_v2,
+ .reset_state = rockchip_pwm_reset_state_v2,
};
static const struct of_device_id rockchip_pwm_dt_ids[] = {
--
1.9.1
^ permalink raw reply related
* [PATCH v2 08/10] pwm: add information about polarity, duty cycle and period to debugfs
From: Boris Brezillon @ 2015-07-20 15:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1437406327-6207-1-git-send-email-boris.brezillon@free-electrons.com>
From: Heiko Stübner <heiko@sntech.de>
The pwm-states make it possible to also output the polarity, duty cycle
and period information in the debugfs pwm summary-outout.
This makes it easier to gather overview information about pwms without
needing to walk through the sysfs attributes of every pwm.
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
drivers/pwm/core.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 4fe4703..9fc8338 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -986,6 +986,11 @@ static void pwm_dbg_show(struct pwm_chip *chip, struct seq_file *s)
if (pwm_is_enabled(pwm))
seq_puts(s, " enabled");
+ seq_printf(s, " period:%uns", pwm_get_period(pwm));
+ seq_printf(s, " duty:%uns", pwm_get_duty_cycle(pwm));
+ seq_printf(s, " polarity:%s", pwm_get_polarity(pwm) ? "inverse"
+ : "normal");
+
seq_puts(s, "\n");
}
}
--
1.9.1
^ permalink raw reply related
* [PATCH v2 07/10] pwm: add the core infrastructure to allow atomic update
From: Boris Brezillon @ 2015-07-20 15:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1437406327-6207-1-git-send-email-boris.brezillon@free-electrons.com>
Add an ->apply() method to the pwm_ops struct to allow PWM drivers to
implement atomic update.
This method will be preferred over the ->enable(), ->disable() and
->config() methods if available.
Add the pwm_get_state(), pwm_get_default_state() and pwm_apply_state()
functions for PWM users to be able to use the atomic update feature.
Note that the pwm_apply_state() does not guarantee the atomicity of the
update operation, it all depends on the availability and implementation
of the ->apply() method.
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
drivers/pwm/core.c | 142 +++++++++++++++++++++++++++++++++++++++++++++++-----
include/linux/pwm.h | 28 +++++++++++
2 files changed, 158 insertions(+), 12 deletions(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 236a8cf..4fe4703 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -46,6 +46,12 @@ static struct pwm_device *pwm_to_device(unsigned int pwm)
return radix_tree_lookup(&pwm_tree, pwm);
}
+static void pwm_set_state(struct pwm_device *pwm,
+ const struct pwm_state *state)
+{
+ pwm->state = *state;
+}
+
static int alloc_pwms(int pwm, unsigned int count)
{
unsigned int from = 0;
@@ -222,6 +228,19 @@ void *pwm_get_chip_data(struct pwm_device *pwm)
}
EXPORT_SYMBOL_GPL(pwm_get_chip_data);
+static bool pwm_ops_check(const struct pwm_ops *ops)
+{
+ /* driver supports legacy, non-atomic operation */
+ if (ops->config && ops->enable && ops->disable)
+ return true;
+
+ /* driver supports atomic operation */
+ if (ops->apply)
+ return true;
+
+ return false;
+}
+
/**
* pwmchip_add_with_polarity() - register a new PWM chip
* @chip: the PWM chip to add
@@ -238,8 +257,10 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip,
unsigned int i;
int ret;
- if (!chip || !chip->dev || !chip->ops || !chip->ops->config ||
- !chip->ops->enable || !chip->ops->disable || !chip->npwm)
+ if (!chip || !chip->dev || !chip->ops || !chip->npwm)
+ return -EINVAL;
+
+ if (!pwm_ops_check(chip->ops))
return -EINVAL;
mutex_lock(&pwm_lock);
@@ -430,7 +451,19 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
if (!pwm || duty_ns < 0 || period_ns <= 0 || duty_ns > period_ns)
return -EINVAL;
- err = pwm->chip->ops->config(pwm->chip, pwm, duty_ns, period_ns);
+ if (pwm->chip->ops->apply) {
+ struct pwm_state state;
+
+ pwm_get_state(pwm, &state);
+ state.period = period_ns;
+ state.duty_cycle = duty_ns;
+
+ err = pwm->chip->ops->apply(pwm->chip, pwm, &state);
+ } else {
+ err = pwm->chip->ops->config(pwm->chip, pwm, duty_ns,
+ period_ns);
+ }
+
if (err)
return err;
@@ -455,6 +488,18 @@ int pwm_set_polarity(struct pwm_device *pwm, enum pwm_polarity polarity)
if (!pwm || !pwm->chip->ops)
return -EINVAL;
+ if (pwm->chip->ops->apply) {
+ struct pwm_state state;
+
+ pwm_get_state(pwm, &state);
+ state.polarity = polarity;
+ err = pwm->chip->ops->apply(pwm->chip, pwm, &state);
+ if (!err)
+ pwm->state.polarity = polarity;
+
+ return err;
+ }
+
if (!pwm->chip->ops->set_polarity)
return -ENOSYS;
@@ -477,17 +522,29 @@ EXPORT_SYMBOL_GPL(pwm_set_polarity);
*/
int pwm_enable(struct pwm_device *pwm)
{
- if (pwm && !pwm_is_enabled(pwm)) {
- int err;
+ int err;
- err = pwm->chip->ops->enable(pwm->chip, pwm);
- if (!err)
- pwm->state.enabled = true;
+ if (!pwm)
+ return -EINVAL;
- return err;
+ if (pwm_is_enabled(pwm))
+ return 0;
+
+ if (pwm->chip->ops->apply) {
+ struct pwm_state state;
+
+ pwm_get_state(pwm, &state);
+ state.enabled = true;
+
+ err = pwm->chip->ops->apply(pwm->chip, pwm, &state);
+ } else {
+ err = pwm->chip->ops->enable(pwm->chip, pwm);
}
- return pwm ? 0 : -EINVAL;
+ if (!err)
+ pwm->state.enabled = true;
+
+ return err;
}
EXPORT_SYMBOL_GPL(pwm_enable);
@@ -497,13 +554,74 @@ EXPORT_SYMBOL_GPL(pwm_enable);
*/
void pwm_disable(struct pwm_device *pwm)
{
- if (pwm && pwm_is_enabled(pwm)) {
+ if (!pwm || !pwm_is_enabled(pwm))
+ return;
+
+ if (pwm->chip->ops->apply) {
+ struct pwm_state state;
+
+ pwm_get_state(pwm, &state);
+ state.enabled = false;
+
+ pwm->chip->ops->apply(pwm->chip, pwm, &state);
+ } else {
pwm->chip->ops->disable(pwm->chip, pwm);
- pwm->state.enabled = false;
}
+
+ pwm->state.enabled = false;
}
EXPORT_SYMBOL_GPL(pwm_disable);
+/**
+ * pwm_apply_state() - atomically apply a new state to a PWM device
+ * @pwm: PWM device
+ * @state: new state to apply
+ */
+int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
+{
+ int err = 0;
+
+ if (!pwm)
+ return -EINVAL;
+
+ if (!memcmp(state, &pwm->state, sizeof(*state)))
+ return 0;
+
+ if (pwm->chip->ops->apply) {
+ err = pwm->chip->ops->apply(pwm->chip, pwm, state);
+ if (!err)
+ pwm_set_state(pwm, state);
+ } else {
+ /*
+ * FIXME: restore the initial state in case of error.
+ */
+ if (state->polarity != pwm->state.polarity) {
+ pwm_disable(pwm);
+ err = pwm_set_polarity(pwm, state->polarity);
+ if (err)
+ goto out;
+ }
+
+ if (state->period != pwm->state.period ||
+ state->duty_cycle != pwm->state.duty_cycle) {
+ err = pwm_config(pwm, state->period, state->duty_cycle);
+ if (err)
+ goto out;
+ }
+
+ if (state->enabled != pwm->state.enabled) {
+ if (state->enabled)
+ err = pwm_enable(pwm);
+ else
+ pwm_disable(pwm);
+ }
+ }
+
+out:
+ return err;
+}
+EXPORT_SYMBOL_GPL(pwm_apply_state);
+
static struct pwm_chip *of_node_to_pwmchip(struct device_node *np)
{
struct pwm_chip *chip;
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index df78211..25f7bd1 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -160,6 +160,30 @@ static inline enum pwm_polarity pwm_get_polarity(const struct pwm_device *pwm)
return pwm ? pwm->state.polarity : PWM_POLARITY_NORMAL;
}
+int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state);
+
+/**
+ * pwm_get_state() - retrieve the current PWM state
+ * @pwm: PWM device
+ * @state: state to fill with the current PWM state
+ */
+static inline void pwm_get_state(struct pwm_device *pwm,
+ struct pwm_state *state)
+{
+ *state = pwm->state;
+}
+
+/**
+ * pwm_get_default_state() - retrieve the default PWM state
+ * @pwm: PWM device
+ * @state: state to fill with the default PWM state
+ */
+static inline void pwm_get_default_state(struct pwm_device *pwm,
+ struct pwm_state *state)
+{
+ *state = pwm->default_state;
+}
+
/**
* struct pwm_ops - PWM controller operations
* @request: optional hook for requesting a PWM
@@ -168,6 +192,7 @@ static inline enum pwm_polarity pwm_get_polarity(const struct pwm_device *pwm)
* @set_polarity: configure the polarity of this PWM
* @enable: enable PWM output toggling
* @disable: disable PWM output toggling
+ * @apply: atomically apply a new PWM config
* @reset_state: reset the current PWM state (pwm->state) to the actual
* hardware state. This function is only called once per
* PWM device when the PWM chip is registered.
@@ -189,6 +214,9 @@ struct pwm_ops {
struct pwm_device *pwm);
void (*disable)(struct pwm_chip *chip,
struct pwm_device *pwm);
+ int (*apply)(struct pwm_chip *chip,
+ struct pwm_device *pwm,
+ const struct pwm_state *state);
void (*reset_state)(struct pwm_chip *chip,
struct pwm_device *pwm);
#ifdef CONFIG_DEBUG_FS
--
1.9.1
^ permalink raw reply related
* [PATCH v2 06/10] pwm: add the PWM initial state retrieval infra
From: Boris Brezillon @ 2015-07-20 15:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1437406327-6207-1-git-send-email-boris.brezillon@free-electrons.com>
Add a ->reset_state() function to the pwm_ops struct to let PWM drivers
initialize the PWM state attached to a PWM device.
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
drivers/pwm/core.c | 3 +++
include/linux/pwm.h | 5 +++++
2 files changed, 8 insertions(+)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 3e830ce..236a8cf 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -264,6 +264,9 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip,
pwm->hwpwm = i;
pwm_set_default_polarity(pwm, polarity);
+ if (chip->ops->reset_state)
+ chip->ops->reset_state(chip, pwm);
+
radix_tree_insert(&pwm_tree, pwm->pwm, pwm);
}
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 1e6e9d1..df78211 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -168,6 +168,9 @@ static inline enum pwm_polarity pwm_get_polarity(const struct pwm_device *pwm)
* @set_polarity: configure the polarity of this PWM
* @enable: enable PWM output toggling
* @disable: disable PWM output toggling
+ * @reset_state: reset the current PWM state (pwm->state) to the actual
+ * hardware state. This function is only called once per
+ * PWM device when the PWM chip is registered.
* @dbg_show: optional routine to show contents in debugfs
* @owner: helps prevent removal of modules exporting active PWMs
*/
@@ -186,6 +189,8 @@ struct pwm_ops {
struct pwm_device *pwm);
void (*disable)(struct pwm_chip *chip,
struct pwm_device *pwm);
+ void (*reset_state)(struct pwm_chip *chip,
+ struct pwm_device *pwm);
#ifdef CONFIG_DEBUG_FS
void (*dbg_show)(struct pwm_chip *chip,
struct seq_file *s);
--
1.9.1
^ permalink raw reply related
* [PATCH v2 05/10] pwm: declare a default PWM state
From: Boris Brezillon @ 2015-07-20 15:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1437406327-6207-1-git-send-email-boris.brezillon@free-electrons.com>
Prepare the addition of the PWM initial state retrieval by adding a default
state where all the parameters retrieved from DT, platform data or
statically forced by the hardware will be stored.
Once done we will be able to store the initial state in the ->state field
without risking to loose the default parameters.
Update the pwm_set/get_default_xxx helpers accordingly.
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
include/linux/pwm.h | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index f4bc034..1e6e9d1 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -101,6 +101,7 @@ struct pwm_device {
void *chip_data;
struct pwm_state state;
+ struct pwm_state default_state;
};
static inline bool pwm_is_enabled(const struct pwm_device *pwm)
@@ -117,7 +118,8 @@ static inline void pwm_set_period(struct pwm_device *pwm, unsigned int period)
static inline void pwm_set_default_period(struct pwm_device *pwm,
unsigned int period)
{
- pwm_set_period(pwm, period);
+ if (pwm)
+ pwm->default_state.period = period;
}
static inline unsigned int pwm_get_period(const struct pwm_device *pwm)
@@ -127,7 +129,7 @@ static inline unsigned int pwm_get_period(const struct pwm_device *pwm)
static inline unsigned int pwm_get_default_period(const struct pwm_device *pwm)
{
- return pwm_get_period(pwm);
+ return pwm ? pwm->default_state.period : 0;
}
static inline void pwm_set_duty_cycle(struct pwm_device *pwm, unsigned int duty)
@@ -149,7 +151,8 @@ int pwm_set_polarity(struct pwm_device *pwm, enum pwm_polarity polarity);
static inline void pwm_set_default_polarity(struct pwm_device *pwm,
enum pwm_polarity polarity)
{
- pwm_set_polarity(pwm, polarity);
+ if (pwm)
+ pwm->default_state.polarity = polarity;
}
static inline enum pwm_polarity pwm_get_polarity(const struct pwm_device *pwm)
--
1.9.1
^ permalink raw reply related
* [PATCH v2 04/10] backlight: pwm_bl: remove useless call to pwm_set_period
From: Boris Brezillon @ 2015-07-20 15:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1437406327-6207-1-git-send-email-boris.brezillon@free-electrons.com>
The PWM period will be set when calling pwm_config. Remove this useless
call to pwm_set_period, which might mess up with the initial PWM state
once we have added proper support for PWM init state retrieval.
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
drivers/video/backlight/pwm_bl.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index ae498c1..71944f8 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -293,12 +293,14 @@ static int pwm_backlight_probe(struct platform_device *pdev)
* period, parsed from the DT, in the PWM device. For the non-DT case,
* set the period from platform data if it has not already been set
* via the PWM lookup table.
+ * FIXME: This assignment should be dropped as soon as all the boards
+ * have moved to the PWM lookup table approach. The same goes for the
+ * pb->period field which should be replaced by
+ * pwm_get_default_period() calls.
*/
pb->period = pwm_get_default_period(pb->pwm);
- if (!pb->period && (data->pwm_period_ns > 0)) {
+ if (!pb->period && (data->pwm_period_ns > 0))
pb->period = data->pwm_period_ns;
- pwm_set_period(pb->pwm, data->pwm_period_ns);
- }
pb->lth_brightness = data->lth_brightness * (pb->period / pb->scale);
--
1.9.1
^ permalink raw reply related
* [PATCH v2 03/10] pwm: move the enabled/disabled info to pwm_state struct
From: Boris Brezillon @ 2015-07-20 15:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1437406327-6207-1-git-send-email-boris.brezillon@free-electrons.com>
Prepare the transition to PWM atomic update by moving the enabled/disabled
state into the pwm_state struct. This way we can easily update the whole
PWM state by copying the new state in the ->state field.
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
drivers/pwm/core.c | 15 ++++++++++++---
include/linux/pwm.h | 7 ++++---
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index a6bc8e6..3e830ce 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -474,8 +474,15 @@ EXPORT_SYMBOL_GPL(pwm_set_polarity);
*/
int pwm_enable(struct pwm_device *pwm)
{
- if (pwm && !test_and_set_bit(PWMF_ENABLED, &pwm->flags))
- return pwm->chip->ops->enable(pwm->chip, pwm);
+ if (pwm && !pwm_is_enabled(pwm)) {
+ int err;
+
+ err = pwm->chip->ops->enable(pwm->chip, pwm);
+ if (!err)
+ pwm->state.enabled = true;
+
+ return err;
+ }
return pwm ? 0 : -EINVAL;
}
@@ -487,8 +494,10 @@ EXPORT_SYMBOL_GPL(pwm_enable);
*/
void pwm_disable(struct pwm_device *pwm)
{
- if (pwm && test_and_clear_bit(PWMF_ENABLED, &pwm->flags))
+ if (pwm && pwm_is_enabled(pwm)) {
pwm->chip->ops->disable(pwm->chip, pwm);
+ pwm->state.enabled = false;
+ }
}
EXPORT_SYMBOL_GPL(pwm_disable);
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index fafed26..f4bc034 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -75,8 +75,7 @@ enum pwm_polarity {
enum {
PWMF_REQUESTED = 1 << 0,
- PWMF_ENABLED = 1 << 1,
- PWMF_EXPORTED = 1 << 2,
+ PWMF_EXPORTED = 1 << 1,
};
/**
@@ -84,11 +83,13 @@ enum {
* @period: PWM period (in nanoseconds)
* @duty_cycle: PWM duty cycle (in nanoseconds)
* @polarity: PWM polarity
+ * @enabled: PWM enabled status
*/
struct pwm_state {
unsigned int period;
unsigned int duty_cycle;
enum pwm_polarity polarity;
+ bool enabled;
};
struct pwm_device {
@@ -104,7 +105,7 @@ struct pwm_device {
static inline bool pwm_is_enabled(const struct pwm_device *pwm)
{
- return test_bit(PWMF_ENABLED, &pwm->flags);
+ return pwm->state.enabled;
}
static inline void pwm_set_period(struct pwm_device *pwm, unsigned int period)
--
1.9.1
^ permalink raw reply related
* [PATCH v2 02/10] pwm: define a new pwm_state struct
From: Boris Brezillon @ 2015-07-20 15:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1437406327-6207-1-git-send-email-boris.brezillon@free-electrons.com>
The PWM state, represented by its period, duty_cycle and polarity,
is currently directly stored in the PWM device.
Declare a pwm_state structure embedding those field so that we can later
use this struct to atomically update all the PWM parameters at once.
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
drivers/pwm/core.c | 6 +++---
include/linux/pwm.h | 26 ++++++++++++++++++--------
2 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 7ffad2b..a6bc8e6 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -431,8 +431,8 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
if (err)
return err;
- pwm->duty_cycle = duty_ns;
- pwm->period = period_ns;
+ pwm->state.duty_cycle = duty_ns;
+ pwm->state.period = period_ns;
return 0;
}
@@ -462,7 +462,7 @@ int pwm_set_polarity(struct pwm_device *pwm, enum pwm_polarity polarity)
if (err)
return err;
- pwm->polarity = polarity;
+ pwm->state.polarity = polarity;
return 0;
}
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index ba4b7ed..fafed26 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -79,6 +79,18 @@ enum {
PWMF_EXPORTED = 1 << 2,
};
+/**
+ * struct pwm_state - state of a PWM channel
+ * @period: PWM period (in nanoseconds)
+ * @duty_cycle: PWM duty cycle (in nanoseconds)
+ * @polarity: PWM polarity
+ */
+struct pwm_state {
+ unsigned int period;
+ unsigned int duty_cycle;
+ enum pwm_polarity polarity;
+};
+
struct pwm_device {
const char *label;
unsigned long flags;
@@ -87,9 +99,7 @@ struct pwm_device {
struct pwm_chip *chip;
void *chip_data;
- unsigned int period; /* in nanoseconds */
- unsigned int duty_cycle; /* in nanoseconds */
- enum pwm_polarity polarity;
+ struct pwm_state state;
};
static inline bool pwm_is_enabled(const struct pwm_device *pwm)
@@ -100,7 +110,7 @@ static inline bool pwm_is_enabled(const struct pwm_device *pwm)
static inline void pwm_set_period(struct pwm_device *pwm, unsigned int period)
{
if (pwm)
- pwm->period = period;
+ pwm->state.period = period;
}
static inline void pwm_set_default_period(struct pwm_device *pwm,
@@ -111,7 +121,7 @@ static inline void pwm_set_default_period(struct pwm_device *pwm,
static inline unsigned int pwm_get_period(const struct pwm_device *pwm)
{
- return pwm ? pwm->period : 0;
+ return pwm ? pwm->state.period : 0;
}
static inline unsigned int pwm_get_default_period(const struct pwm_device *pwm)
@@ -122,12 +132,12 @@ static inline unsigned int pwm_get_default_period(const struct pwm_device *pwm)
static inline void pwm_set_duty_cycle(struct pwm_device *pwm, unsigned int duty)
{
if (pwm)
- pwm->duty_cycle = duty;
+ pwm->state.duty_cycle = duty;
}
static inline unsigned int pwm_get_duty_cycle(const struct pwm_device *pwm)
{
- return pwm ? pwm->duty_cycle : 0;
+ return pwm ? pwm->state.duty_cycle : 0;
}
/*
@@ -143,7 +153,7 @@ static inline void pwm_set_default_polarity(struct pwm_device *pwm,
static inline enum pwm_polarity pwm_get_polarity(const struct pwm_device *pwm)
{
- return pwm ? pwm->polarity : PWM_POLARITY_NORMAL;
+ return pwm ? pwm->state.polarity : PWM_POLARITY_NORMAL;
}
/**
--
1.9.1
^ permalink raw reply related
* [PATCH v2 01/10] pwm: introduce default period and polarity concepts
From: Boris Brezillon @ 2015-07-20 15:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1437406327-6207-1-git-send-email-boris.brezillon@free-electrons.com>
When requested by a user, the PWM is assigned a default period and polarity
extracted from the DT, the platform data or statically set by the driver.
Those default values are currently stored in the period and polarity
fields of the pwm_device struct, but they will be stored somewhere else
once we have introduced the architecture allowing for hardware state
retrieval.
The pwm_set_default_polarity and pwm_set_default_period should only be
used by PWM drivers or the PWM core infrastructure to specify the
default period and polarity values.
PWM users might call the pwm_get_default_period to query the default
period value. There is currently no helper to query the default
polarity, but it might be added later on if there is a need for it.
This patch also modifies all the places where the default helpers should
be used in place of the standard ones.
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
drivers/leds/leds-pwm.c | 2 +-
drivers/pwm/core.c | 14 +++++++-------
drivers/pwm/pwm-pxa.c | 2 +-
drivers/pwm/pwm-sun4i.c | 3 ++-
drivers/regulator/pwm-regulator.c | 2 +-
drivers/video/backlight/lm3630a_bl.c | 4 ++--
drivers/video/backlight/pwm_bl.c | 2 +-
drivers/video/fbdev/ssd1307fb.c | 2 +-
include/linux/pwm.h | 17 +++++++++++++++++
9 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 1d07e3e..2c564d1 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -125,7 +125,7 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
if (led_data->can_sleep)
INIT_WORK(&led_data->work, led_pwm_work);
- led_data->period = pwm_get_period(led_data->pwm);
+ led_data->period = pwm_get_default_period(led_data->pwm);
if (!led_data->period && (led->pwm_period_ns > 0))
led_data->period = led->pwm_period_ns;
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index f7c11d2..7ffad2b 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -146,12 +146,12 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args)
if (IS_ERR(pwm))
return pwm;
- pwm_set_period(pwm, args->args[1]);
+ pwm_set_default_period(pwm, args->args[1]);
if (args->args[2] & PWM_POLARITY_INVERTED)
- pwm_set_polarity(pwm, PWM_POLARITY_INVERSED);
+ pwm_set_default_polarity(pwm, PWM_POLARITY_INVERSED);
else
- pwm_set_polarity(pwm, PWM_POLARITY_NORMAL);
+ pwm_set_default_polarity(pwm, PWM_POLARITY_NORMAL);
return pwm;
}
@@ -172,7 +172,7 @@ of_pwm_simple_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
if (IS_ERR(pwm))
return pwm;
- pwm_set_period(pwm, args->args[1]);
+ pwm_set_default_period(pwm, args->args[1]);
return pwm;
}
@@ -262,7 +262,7 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip,
pwm->chip = chip;
pwm->pwm = chip->base + i;
pwm->hwpwm = i;
- pwm->polarity = polarity;
+ pwm_set_default_polarity(pwm, polarity);
radix_tree_insert(&pwm_tree, pwm->pwm, pwm);
}
@@ -704,8 +704,8 @@ struct pwm_device *pwm_get(struct device *dev, const char *con_id)
if (IS_ERR(pwm))
goto out;
- pwm_set_period(pwm, chosen->period);
- pwm_set_polarity(pwm, chosen->polarity);
+ pwm_set_default_period(pwm, chosen->period);
+ pwm_set_default_polarity(pwm, chosen->polarity);
out:
mutex_unlock(&pwm_lookup_lock);
diff --git a/drivers/pwm/pwm-pxa.c b/drivers/pwm/pwm-pxa.c
index cb2f702..65b80aa 100644
--- a/drivers/pwm/pwm-pxa.c
+++ b/drivers/pwm/pwm-pxa.c
@@ -160,7 +160,7 @@ pxa_pwm_of_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
if (IS_ERR(pwm))
return pwm;
- pwm_set_period(pwm, args->args[0]);
+ pwm_set_default_period(pwm, args->args[0]);
return pwm;
}
diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index cd9dde5..a364fb7 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -333,7 +333,8 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
val = sun4i_pwm_readl(pwm, PWM_CTRL_REG);
for (i = 0; i < pwm->chip.npwm; i++)
if (!(val & BIT_CH(PWM_ACT_STATE, i)))
- pwm->chip.pwms[i].polarity = PWM_POLARITY_INVERSED;
+ pwm_set_default_polarity(&pwm->chip.pwms[i],
+ PWM_POLARITY_INVERSED);
clk_disable_unprepare(pwm->clk);
return 0;
diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index ffa9612..12b4d9d 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -46,7 +46,7 @@ static int pwm_regulator_set_voltage_sel(struct regulator_dev *rdev,
int dutycycle;
int ret;
- pwm_reg_period = pwm_get_period(drvdata->pwm);
+ pwm_reg_period = pwm_get_default_period(drvdata->pwm);
dutycycle = (pwm_reg_period *
drvdata->duty_cycle_table[selector].dutycycle) / 100;
diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
index 35fe482..449ebc3 100644
--- a/drivers/video/backlight/lm3630a_bl.c
+++ b/drivers/video/backlight/lm3630a_bl.c
@@ -162,7 +162,7 @@ static int lm3630a_intr_config(struct lm3630a_chip *pchip)
static void lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
{
- unsigned int period = pwm_get_period(pchip->pwmd);
+ unsigned int period = pwm_get_default_period(pchip->pwmd);
unsigned int duty = br * period / br_max;
pwm_config(pchip->pwmd, duty, period);
@@ -425,7 +425,7 @@ static int lm3630a_probe(struct i2c_client *client,
return PTR_ERR(pchip->pwmd);
}
}
- pchip->pwmd->period = pdata->pwm_period;
+ pwm_set_default_period(pchip->pwmd, pdata->pwm_period);
/* interrupt enable : irq 0 is not allowed */
pchip->irq = client->irq;
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index eff379b..ae498c1 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -294,7 +294,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
* set the period from platform data if it has not already been set
* via the PWM lookup table.
*/
- pb->period = pwm_get_period(pb->pwm);
+ pb->period = pwm_get_default_period(pb->pwm);
if (!pb->period && (data->pwm_period_ns > 0)) {
pb->period = data->pwm_period_ns;
pwm_set_period(pb->pwm, data->pwm_period_ns);
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 3e153c0..6949626 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -294,7 +294,7 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
return PTR_ERR(par->pwm);
}
- par->pwm_period = pwm_get_period(par->pwm);
+ par->pwm_period = pwm_get_default_period(par->pwm);
/* Enable the PWM */
pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period);
pwm_enable(par->pwm);
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 6f286df..ba4b7ed 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -103,11 +103,22 @@ static inline void pwm_set_period(struct pwm_device *pwm, unsigned int period)
pwm->period = period;
}
+static inline void pwm_set_default_period(struct pwm_device *pwm,
+ unsigned int period)
+{
+ pwm_set_period(pwm, period);
+}
+
static inline unsigned int pwm_get_period(const struct pwm_device *pwm)
{
return pwm ? pwm->period : 0;
}
+static inline unsigned int pwm_get_default_period(const struct pwm_device *pwm)
+{
+ return pwm_get_period(pwm);
+}
+
static inline void pwm_set_duty_cycle(struct pwm_device *pwm, unsigned int duty)
{
if (pwm)
@@ -124,6 +135,12 @@ static inline unsigned int pwm_get_duty_cycle(const struct pwm_device *pwm)
*/
int pwm_set_polarity(struct pwm_device *pwm, enum pwm_polarity polarity);
+static inline void pwm_set_default_polarity(struct pwm_device *pwm,
+ enum pwm_polarity polarity)
+{
+ pwm_set_polarity(pwm, polarity);
+}
+
static inline enum pwm_polarity pwm_get_polarity(const struct pwm_device *pwm)
{
return pwm ? pwm->polarity : PWM_POLARITY_NORMAL;
--
1.9.1
^ permalink raw reply related
* [PATCH v2 00/10] pwm: add support for atomic update
From: Boris Brezillon @ 2015-07-20 15:31 UTC (permalink / raw)
To: linux-arm-kernel
Hello Thierry,
This series adds support for atomic PWM update, or IOW, the capability
to update all the parameters of a PWM device (enabled/disabled, period,
duty and polarity) in one go.
Best Regards,
Boris
Changes since v1:
- dropped applied patches
- squashed Heiko's fixes into the rockchip driver changes
- made a few cosmetic changes
- added kerneldoc comments
- added Heiko's patch to display more information in debugfs
- dropped pwm-regulator patches (should be submitted separately)
Boris Brezillon (9):
pwm: introduce default period and polarity concepts
pwm: define a new pwm_state struct
pwm: move the enabled/disabled info to pwm_state struct
backlight: pwm_bl: remove useless call to pwm_set_period
pwm: declare a default PWM state
pwm: add the PWM initial state retrieval infra
pwm: add the core infrastructure to allow atomic update
pwm: rockchip: add initial state retrieval
pwm: rockchip: add support for atomic update
Heiko Stübner (1):
pwm: add information about polarity, duty cycle and period to debugfs
drivers/leds/leds-pwm.c | 2 +-
drivers/pwm/core.c | 169 +++++++++++++++++++++++++++++++----
drivers/pwm/pwm-pxa.c | 2 +-
drivers/pwm/pwm-rockchip.c | 119 +++++++++++++++++++-----
drivers/pwm/pwm-sun4i.c | 3 +-
drivers/regulator/pwm-regulator.c | 2 +-
drivers/video/backlight/lm3630a_bl.c | 4 +-
drivers/video/backlight/pwm_bl.c | 10 ++-
drivers/video/fbdev/ssd1307fb.c | 2 +-
include/linux/pwm.h | 86 +++++++++++++++---
10 files changed, 336 insertions(+), 63 deletions(-)
--
1.9.1
^ permalink raw reply
* Re: [PATCH] backlight: pm8941-wled: Move PM8941 WLED driver to backlight
From: Jingoo Han @ 2015-07-20 14:22 UTC (permalink / raw)
To: 'Bjorn Andersson'
Cc: 'Rob Herring', 'Pawel Moll',
'Mark Rutland', 'Ian Campbell',
'Kumar Gala', 'Bryan Wu',
'Richard Purdie', 'Jacek Anaszewski',
'Lee Jones', 'Jean-Christophe Plagniol-Villard',
'Tomi Valkeinen', 'Rob Clark', devicetree,
linux-kernel, linux-leds, linux-fbdev, linux-arm-msm,
'Jingoo Han'
In-Reply-To: <1436990540-23354-1-git-send-email-bjorn.andersson@sonymobile.com>
On Thursday, July 16, 2015 5:02 AM, Bjorn Andersson wrote:
>
> The Qualcomm PM8941 WLED block is used for backlight and should therefor
> be in the backlight framework and not in the LED framework. This moves
> the driver and adapts to the backlight api instead.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> ---
> .../devicetree/bindings/leds/leds-pm8941-wled.txt | 5 +-
> drivers/leds/Kconfig | 8 ---
> drivers/leds/Makefile | 1 -
> drivers/video/backlight/Kconfig | 8 +++
> drivers/video/backlight/Makefile | 1 +
> .../backlight/pm8941-wled.c} | 59 ++++++++++------------
> 6 files changed, 36 insertions(+), 46 deletions(-)
> rename drivers/{leds/leds-pm8941-wled.c => video/backlight/pm8941-wled.c} (90%)
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt
> b/Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt
> index a85a964d61f5..424f8444a6cd 100644
> --- a/Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt
> +++ b/Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt
Please move this txt file to 'backlight' directory.
> @@ -5,10 +5,7 @@ Required properties:
> - reg: slave address
>
> Optional properties:
> -- label: The label for this led
> - See Documentation/devicetree/bindings/leds/common.txt
> -- linux,default-trigger: Default trigger assigned to the LED
> - See Documentation/devicetree/bindings/leds/common.txt
> +- label: The name of the backlight device
> - qcom,cs-out: bool; enable current sink output
> - qcom,cabc: bool; enable content adaptive backlight control
> - qcom,ext-gen: bool; use externally generated modulator signal to dim
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 9ad35f72ab4c..b8d4b965ca2a 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -578,14 +578,6 @@ config LEDS_VERSATILE
> This option enabled support for the LEDs on the ARM Versatile
> and RealView boards. Say Y to enabled these.
>
> -config LEDS_PM8941_WLED
> - tristate "LED support for the Qualcomm PM8941 WLED block"
> - depends on LEDS_CLASS
> - select REGMAP
> - help
> - This option enables support for the 'White' LED block
> - on Qualcomm PM8941 PMICs.
> -
> comment "LED Triggers"
> source "drivers/leds/trigger/Kconfig"
>
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 8d6a24a2f513..abe96d960ebe 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -63,7 +63,6 @@ obj-$(CONFIG_LEDS_BLINKM) += leds-blinkm.o
> obj-$(CONFIG_LEDS_SYSCON) += leds-syscon.o
> obj-$(CONFIG_LEDS_VERSATILE) += leds-versatile.o
> obj-$(CONFIG_LEDS_MENF21BMC) += leds-menf21bmc.o
> -obj-$(CONFIG_LEDS_PM8941_WLED) += leds-pm8941-wled.o
> obj-$(CONFIG_LEDS_KTD2692) += leds-ktd2692.o
>
> # LED SPI Drivers
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 0505b796d743..6c67c5430933 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -299,6 +299,14 @@ config BACKLIGHT_TOSA
> If you have an Sharp SL-6000 Zaurus say Y to enable a driver
> for its backlight
>
> +config BACKLIGHT_PM8941_WLED
> + tristate "Qualcomm PM8941 WLED Driver"
> + depends on LEDS_CLASS
LEDS_CLASS?
Please check your patch carefully.
One more thing,
Did you test this patch with the board?
If not, I will not accept this patch, unless other people send 'Tested-by'.
Best regards,
Jingoo Han
> + select REGMAP
> + help
> + If you have the Qualcomm PM8941, say Y to enable a driver for the
> + WLED block.
> +
> config BACKLIGHT_SAHARA
> tristate "Tabletkiosk Sahara Touch-iT Backlight Driver"
> depends on X86
> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> index d67073f9d421..16ec534cff30 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_BACKLIGHT_OMAP1) += omap1_bl.o
> obj-$(CONFIG_BACKLIGHT_OT200) += ot200_bl.o
> obj-$(CONFIG_BACKLIGHT_PANDORA) += pandora_bl.o
> obj-$(CONFIG_BACKLIGHT_PCF50633) += pcf50633-backlight.o
> +obj-$(CONFIG_BACKLIGHT_PM8941_WLED) += pm8941-wled.o
> obj-$(CONFIG_BACKLIGHT_PWM) += pwm_bl.o
> obj-$(CONFIG_BACKLIGHT_SAHARA) += kb3886_bl.o
> obj-$(CONFIG_BACKLIGHT_SKY81452) += sky81452-backlight.o
> diff --git a/drivers/leds/leds-pm8941-wled.c b/drivers/video/backlight/pm8941-wled.c
> similarity index 90%
> rename from drivers/leds/leds-pm8941-wled.c
> rename to drivers/video/backlight/pm8941-wled.c
> index bf64a593fbf1..4f5ae95331a1 100644
> --- a/drivers/leds/leds-pm8941-wled.c
> +++ b/drivers/video/backlight/pm8941-wled.c
> @@ -11,7 +11,7 @@
> */
>
> #include <linux/kernel.h>
> -#include <linux/leds.h>
> +#include <linux/backlight.h>
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
> @@ -76,30 +76,29 @@ struct pm8941_wled_config {
> };
>
> struct pm8941_wled {
> + const char *name;
> struct regmap *regmap;
> u16 addr;
>
> - struct led_classdev cdev;
> -
> struct pm8941_wled_config cfg;
> };
>
> -static int pm8941_wled_set(struct led_classdev *cdev,
> - enum led_brightness value)
> +static int pm8941_wled_update_status(struct backlight_device *bl)
> {
> - struct pm8941_wled *wled;
> + struct pm8941_wled *wled = bl_get_data(bl);
> + u16 val = bl->props.brightness;
> u8 ctrl = 0;
> - u16 val;
> int rc;
> int i;
>
> - wled = container_of(cdev, struct pm8941_wled, cdev);
> + if (bl->props.power != FB_BLANK_UNBLANK ||
> + bl->props.fb_blank != FB_BLANK_UNBLANK ||
> + bl->props.state & BL_CORE_FBBLANK)
> + val = 0;
>
> - if (value != 0)
> + if (val != 0)
> ctrl = PM8941_WLED_REG_MOD_EN_BIT;
>
> - val = value * PM8941_WLED_REG_VAL_MAX / LED_FULL;
> -
> rc = regmap_update_bits(wled->regmap,
> wled->addr + PM8941_WLED_REG_MOD_EN,
> PM8941_WLED_REG_MOD_EN_MASK, ctrl);
> @@ -128,16 +127,6 @@ static int pm8941_wled_set(struct led_classdev *cdev,
> return rc;
> }
>
> -static void pm8941_wled_set_brightness(struct led_classdev *cdev,
> - enum led_brightness value)
> -{
> - if (pm8941_wled_set(cdev, value)) {
> - dev_err(cdev->dev, "Unable to set brightness\n");
> - return;
> - }
> - cdev->brightness = value;
> -}
> -
> static int pm8941_wled_setup(struct pm8941_wled *wled)
> {
> int rc;
> @@ -336,12 +325,9 @@ static int pm8941_wled_configure(struct pm8941_wled *wled, struct device *dev)
> }
> wled->addr = val;
>
> - rc = of_property_read_string(dev->of_node, "label", &wled->cdev.name);
> + rc = of_property_read_string(dev->of_node, "label", &wled->name);
> if (rc)
> - wled->cdev.name = dev->of_node->name;
> -
> - wled->cdev.default_trigger = of_get_property(dev->of_node,
> - "linux,default-trigger", NULL);
> + wled->name = dev->of_node->name;
>
> *cfg = pm8941_wled_config_defaults;
> for (i = 0; i < ARRAY_SIZE(u32_opts); ++i) {
> @@ -377,8 +363,14 @@ static int pm8941_wled_configure(struct pm8941_wled *wled, struct device *dev)
> return 0;
> }
>
> +static const struct backlight_ops pm8941_wled_ops = {
> + .update_status = pm8941_wled_update_status,
> +};
> +
> static int pm8941_wled_probe(struct platform_device *pdev)
> {
> + struct backlight_properties props;
> + struct backlight_device *bl;
> struct pm8941_wled *wled;
> struct regmap *regmap;
> int rc;
> @@ -403,13 +395,14 @@ static int pm8941_wled_probe(struct platform_device *pdev)
> if (rc)
> return rc;
>
> - wled->cdev.brightness_set = pm8941_wled_set_brightness;
> -
> - rc = devm_led_classdev_register(&pdev->dev, &wled->cdev);
> - if (rc)
> - return rc;
> -
> - platform_set_drvdata(pdev, wled);
> + memset(&props, 0, sizeof(struct backlight_properties));
> + props.type = BACKLIGHT_RAW;
> + props.max_brightness = PM8941_WLED_REG_VAL_MAX;
> + bl = devm_backlight_device_register(&pdev->dev, wled->name,
> + &pdev->dev, wled,
> + &pm8941_wled_ops, &props);
> + if (IS_ERR(bl))
> + return PTR_ERR(bl);
>
> return 0;
> };
> --
> 1.8.2.2
^ permalink raw reply
* Re: [PATCH] backlight: pm8941-wled: Move PM8941 WLED driver to backlight
From: Jingoo Han @ 2015-07-20 14:15 UTC (permalink / raw)
To: 'Jacek Anaszewski', 'Bjorn Andersson'
Cc: 'Rob Herring', 'Pawel Moll',
'Mark Rutland', 'Ian Campbell',
'Kumar Gala', 'Bryan Wu',
'Richard Purdie', 'Lee Jones',
'Jean-Christophe Plagniol-Villard',
'Tomi Valkeinen', 'Rob Clark', devicetree,
linux-kernel, linux-leds, linux-fbdev, linux-arm-msm,
'Jingoo Han'
In-Reply-To: <55A764C5.5060205@samsung.com>
On Thursday, July 16, 2015 5:01 PM, Jacek Anaszewski wrote:
> On 07/15/2015 10:02 PM, Bjorn Andersson wrote:
> > The Qualcomm PM8941 WLED block is used for backlight and should therefor
> > be in the backlight framework and not in the LED framework. This moves
> > the driver and adapts to the backlight api instead.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> > ---
> > .../devicetree/bindings/leds/leds-pm8941-wled.txt | 5 +-
> > drivers/leds/Kconfig | 8 ---
> > drivers/leds/Makefile | 1 -
> > drivers/video/backlight/Kconfig | 8 +++
> > drivers/video/backlight/Makefile | 1 +
> > .../backlight/pm8941-wled.c} | 59 ++++++++++------------
> > 6 files changed, 36 insertions(+), 46 deletions(-)
> > rename drivers/{leds/leds-pm8941-wled.c => video/backlight/pm8941-wled.c} (90%)
>
> Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com>
If you are not an author of this driver or a maintainer affected by this patch,
Just 'Reviewed-by' looks good.
Best regards,
Jingoo Han
>
> --
> Best Regards,
> Jacek Anaszewski
^ permalink raw reply
* Re: [RFC PATCH 06/15] pwm: define a new pwm_state struct
From: Boris Brezillon @ 2015-07-20 10:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20150720100925.GX29614@ulmo>
On Mon, 20 Jul 2015 12:09:26 +0200
Thierry Reding <thierry.reding@gmail.com> wrote:
> On Mon, Jul 20, 2015 at 12:01:16PM +0200, Boris Brezillon wrote:
> > On Mon, 20 Jul 2015 10:04:59 +0200
> > Thierry Reding <thierry.reding@gmail.com> wrote:
> >
> > > On Wed, Jul 01, 2015 at 10:21:52AM +0200, Boris Brezillon wrote:
> > > [...]
> > > > diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> > > [...]
> > > > +struct pwm_state {
> > > > + unsigned int period; /* in nanoseconds */
> > > > + unsigned int duty_cycle; /* in nanoseconds */
> > > > + enum pwm_polarity polarity;
> > > > +};
> > >
> > > No need for the extra padding here.
> >
> > What do you mean by "extra padding" ?
> > I just reused the indentation used in the pwm_device struct.
>
> Yeah, I have a local patch to fix that up. I find it useless to pad
> things like this, and it has the downside that it will become totally
> inconsistent (or cause a lot of churn by reformatting) if ever you add a
> field that extends beyond the padding. Single spaces don't have any such
> drawbacks and, in my opinion, look just as good.
I prefer the single space approach too, so I won't complain ;-).
>
> > Would you prefer something like that ?
> >
> > struct pwm_state {
> > unsigned int period; /* in nanoseconds */
> > unsigned int duty_cycle; /* in nanoseconds */
> > enum pwm_polarity polarity;
> > };
>
> Yeah. I'd say even the comments would be more suited in a kerneldoc-
> style comment:
>
> /**
> * struct pwm_state - state of a PWM channel
> * @period: PWM period (in nanoseconds)
> * @duty_cycle: PWM duty cycle (in nanoseconds)
> * @polarity: PWM polarity
> */
> struct pwm_state {
> unsigned int period;
> unsigned int duty_cycle;
> enum pwm_polarity polarity;
> };
>
> This is something that users will need to deal with, so eventually
> somebody might look at this via some DocBook generated HTML or PDF.
I agree.
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [RFC PATCH 06/15] pwm: define a new pwm_state struct
From: Thierry Reding @ 2015-07-20 10:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20150720120116.2358b829@bbrezillon>
[-- Attachment #1: Type: text/plain, Size: 1729 bytes --]
On Mon, Jul 20, 2015 at 12:01:16PM +0200, Boris Brezillon wrote:
> On Mon, 20 Jul 2015 10:04:59 +0200
> Thierry Reding <thierry.reding@gmail.com> wrote:
>
> > On Wed, Jul 01, 2015 at 10:21:52AM +0200, Boris Brezillon wrote:
> > [...]
> > > diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> > [...]
> > > +struct pwm_state {
> > > + unsigned int period; /* in nanoseconds */
> > > + unsigned int duty_cycle; /* in nanoseconds */
> > > + enum pwm_polarity polarity;
> > > +};
> >
> > No need for the extra padding here.
>
> What do you mean by "extra padding" ?
> I just reused the indentation used in the pwm_device struct.
Yeah, I have a local patch to fix that up. I find it useless to pad
things like this, and it has the downside that it will become totally
inconsistent (or cause a lot of churn by reformatting) if ever you add a
field that extends beyond the padding. Single spaces don't have any such
drawbacks and, in my opinion, look just as good.
> Would you prefer something like that ?
>
> struct pwm_state {
> unsigned int period; /* in nanoseconds */
> unsigned int duty_cycle; /* in nanoseconds */
> enum pwm_polarity polarity;
> };
Yeah. I'd say even the comments would be more suited in a kerneldoc-
style comment:
/**
* struct pwm_state - state of a PWM channel
* @period: PWM period (in nanoseconds)
* @duty_cycle: PWM duty cycle (in nanoseconds)
* @polarity: PWM polarity
*/
struct pwm_state {
unsigned int period;
unsigned int duty_cycle;
enum pwm_polarity polarity;
};
This is something that users will need to deal with, so eventually
somebody might look at this via some DocBook generated HTML or PDF.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 11/15] pwm: add the core infrastructure to allow atomic update
From: Thierry Reding @ 2015-07-20 10:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20150720114827.2e5d52a5@bbrezillon>
[-- Attachment #1: Type: text/plain, Size: 2001 bytes --]
On Mon, Jul 20, 2015 at 11:48:27AM +0200, Boris Brezillon wrote:
> On Mon, 20 Jul 2015 10:59:40 +0200 Thierry Reding <thierry.reding@gmail.com> wrote:
> > On Wed, Jul 01, 2015 at 10:21:57AM +0200, Boris Brezillon wrote:
[...]
> > > +int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
> > > +{
> > > + int err = 0;
> > > +
> > > + if (!pwm)
> > > + return -EINVAL;
> > > +
> > > + if (!memcmp(state, &pwm->state, sizeof(*state)))
> > > + return 0;
> > > +
> > > + if (pwm->chip->ops->apply) {
> > > + err = pwm->chip->ops->apply(pwm->chip, pwm, state);
> > > + if (!err)
> > > + pwm->state = *state;
> >
> > Maybe we want pwm_set_state() for this?
>
> I'm not opposed to the addition of the pwm_set_state() function as long
> as it's a private one: I don't want to let PMW drivers or users mess up
> with the current PWM state.
Yeah, it could be a static function in core.c. What I want to avoid is
having to change a bunch of code if ever state assignment becomes
something other than merely copying a structure.
[...]
> > > diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> > > index b47244a..7e99679 100644
> > > --- a/include/linux/pwm.h
> > > +++ b/include/linux/pwm.h
> > > @@ -151,6 +151,29 @@ static inline enum pwm_polarity pwm_get_polarity(const struct pwm_device *pwm)
> > > return pwm ? pwm->state.polarity : PWM_POLARITY_NORMAL;
> > > }
> > >
> > > +/*
> > > + * pwm_apply_state - apply a new state to the PWM device
> > > + */
> > > +int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state);
> >
> > If you add kerneldoc, please add it properly. It should start with /**
> > and you need to list at least the parameters and return value.
>
> Yes, I'll fix that.
> BTW, I remember that you were expecting another name for this function
> (pwm_update IIRC).
I don't mind the pwm_apply_state() name very much. It's pretty accurate
with regards to what it does.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox