2015-07-15 20:02:30

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH] backlight: pm8941-wled: Move PM8941 WLED driver to backlight

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 <[email protected]>
---
.../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
@@ -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
+ 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


2015-07-16 08:01:17

by Jacek Anaszewski

[permalink] [raw]
Subject: Re: [PATCH] backlight: pm8941-wled: Move PM8941 WLED driver to backlight

Hi Bjorn,

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 <[email protected]>
> ---
> .../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 <[email protected]>

--
Best Regards,
Jacek Anaszewski

2015-07-20 14:16:06

by Jingoo Han

[permalink] [raw]
Subject: Re: [PATCH] backlight: pm8941-wled: Move PM8941 WLED driver to backlight

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 <[email protected]>
> > ---
> > .../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 <[email protected]>

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

2015-07-20 14:22:29

by Jingoo Han

[permalink] [raw]
Subject: Re: [PATCH] backlight: pm8941-wled: Move PM8941 WLED driver to backlight

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 <[email protected]>
> ---
> .../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

2015-07-20 17:45:46

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH] backlight: pm8941-wled: Move PM8941 WLED driver to backlight

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 <[email protected]>
> > ---
> > .../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

2015-07-20 21:13:03

by Jacek Anaszewski

[permalink] [raw]
Subject: Re: [PATCH] backlight: pm8941-wled: Move PM8941 WLED driver to backlight



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 <[email protected]>
>>> ---
>>> .../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 <[email protected]>
>
> 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

2015-07-21 00:47:50

by Jingoo Han

[permalink] [raw]
Subject: Re: [PATCH] backlight: pm8941-wled: Move PM8941 WLED driver to backlight


On 2015. 7. 21., at AM 6:11, Jacek Anaszewski <[email protected]> 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 <[email protected]>
>>>> ---
>>>> .../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 <[email protected]>
>>
>> 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-

2015-07-21 08:50:45

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH] backlight: pm8941-wled: Move PM8941 WLED driver to backlight

On Tue, 21 Jul 2015, Jingoo Han wrote:

>
> On 2015. 7. 21., at AM 6:11, Jacek Anaszewski <[email protected]> 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 <[email protected]>
> >>>> ---
> >>>> .../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 <[email protected]>
> >>
> >> 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.

Please read: Sections 12 and 13 of Documentation/SubmittingPatches.

Granted, Acked-bys are used by Maintainers when reviewing changes in
subsystems they are responsible for. However, that's not the sole use
of them. Anyone can provide an Ack for code they have reviewed, but
are not an expert in. Reviewed-bys have much stronger meaning and
should only be used if the supplier knows the code and the subsystem
well. Please see SubmittingPatches for a full meaning of the
Reviewed-by tag.

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2015-07-21 18:22:11

by Rob Clark

[permalink] [raw]
Subject: Re: [PATCH] backlight: pm8941-wled: Move PM8941 WLED driver to backlight

On Wed, Jul 15, 2015 at 4:02 PM, Bjorn Andersson
<[email protected]> 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 <[email protected]>

Tested-by: Rob Clark <[email protected]>

(also on a z3, with work-in-progress panel driver for the AUO panel + drm/msm)

BR,
-R

> ---
> .../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
> @@ -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
> + 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
>

2015-07-22 00:45:04

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH v2] backlight: pm8941-wled: Move PM8941 WLED driver to backlight

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.

Acked-by: Jacek Anaszewski <[email protected]>
Tested-by: Rob Clark <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
---
.../backlight/pm8941-wled.txt} | 5 +-
drivers/leds/Kconfig | 8 ---
drivers/leds/Makefile | 1 -
drivers/video/backlight/Kconfig | 7 +++
drivers/video/backlight/Makefile | 1 +
.../backlight/pm8941-wled.c} | 60 ++++++++++------------
6 files changed, 35 insertions(+), 47 deletions(-)
rename Documentation/devicetree/bindings/{leds/leds-pm8941-wled.txt => video/backlight/pm8941-wled.txt} (84%)
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/video/backlight/pm8941-wled.txt
similarity index 84%
rename from Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt
rename to Documentation/devicetree/bindings/video/backlight/pm8941-wled.txt
index a85a964d61f5..424f8444a6cd 100644
--- a/Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt
+++ b/Documentation/devicetree/bindings/video/backlight/pm8941-wled.txt
@@ -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..5ffa4b4e26c0 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -299,6 +299,13 @@ 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"
+ 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..c704c3236034 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;
};
@@ -432,4 +425,3 @@ module_platform_driver(pm8941_wled_driver);

MODULE_DESCRIPTION("pm8941 wled driver");
MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("platform:pm8941-wled");
--
1.8.2.2

2015-07-22 00:49:50

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v2] backlight: pm8941-wled: Move PM8941 WLED driver to backlight

On Tue 21 Jul 17:44 PDT 2015, 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.
>
> Acked-by: Jacek Anaszewski <[email protected]>
> Tested-by: Rob Clark <[email protected]>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---

Sorry, forgot the changelog.

Changed since v1:
- Moved dt binding document to backlight
- Dropped LED_CLASS as dependency

Regards,
Bjorn

2015-08-06 15:37:50

by Jingoo Han

[permalink] [raw]
Subject: Re: [PATCH v2] backlight: pm8941-wled: Move PM8941 WLED driver to backlight

On Wednesday, July 22, 2015 9:45 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.
>
> Acked-by: Jacek Anaszewski <[email protected]>
> Tested-by: Rob Clark <[email protected]>
> Signed-off-by: Bjorn Andersson <[email protected]>

Acked-by: Jingoo Han <[email protected]>

Best regards,
Jingoo Han

> ---
> .../backlight/pm8941-wled.txt} | 5 +-
> drivers/leds/Kconfig | 8 ---
> drivers/leds/Makefile | 1 -
> drivers/video/backlight/Kconfig | 7 +++
> drivers/video/backlight/Makefile | 1 +
> .../backlight/pm8941-wled.c} | 60 ++++++++++------------
> 6 files changed, 35 insertions(+), 47 deletions(-)
> rename Documentation/devicetree/bindings/{leds/leds-pm8941-wled.txt => video/backlight/pm8941-
> wled.txt} (84%)
> 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/video/backlight/pm8941-wled.txt
> similarity index 84%
> rename from Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt
> rename to Documentation/devicetree/bindings/video/backlight/pm8941-wled.txt
> index a85a964d61f5..424f8444a6cd 100644
> --- a/Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt
> +++ b/Documentation/devicetree/bindings/video/backlight/pm8941-wled.txt
> @@ -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..5ffa4b4e26c0 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -299,6 +299,13 @@ 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"
> + 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..c704c3236034 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;
> };
> @@ -432,4 +425,3 @@ module_platform_driver(pm8941_wled_driver);
>
> MODULE_DESCRIPTION("pm8941 wled driver");
> MODULE_LICENSE("GPL v2");
> -MODULE_ALIAS("platform:pm8941-wled");
> --
> 1.8.2.2

2015-08-10 15:53:37

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v2] backlight: pm8941-wled: Move PM8941 WLED driver to backlight

On Tue, 21 Jul 2015, 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.
>
> Acked-by: Jacek Anaszewski <[email protected]>
> Tested-by: Rob Clark <[email protected]>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
> .../backlight/pm8941-wled.txt} | 5 +-
> drivers/leds/Kconfig | 8 ---
> drivers/leds/Makefile | 1 -
> drivers/video/backlight/Kconfig | 7 +++
> drivers/video/backlight/Makefile | 1 +
> .../backlight/pm8941-wled.c} | 60 ++++++++++------------
> 6 files changed, 35 insertions(+), 47 deletions(-)
> rename Documentation/devicetree/bindings/{leds/leds-pm8941-wled.txt => video/backlight/pm8941-wled.txt} (84%)
> rename drivers/{leds/leds-pm8941-wled.c => video/backlight/pm8941-wled.c} (90%)

Applied with Jingoo's Ack, thanks.

> diff --git a/Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt b/Documentation/devicetree/bindings/video/backlight/pm8941-wled.txt
> similarity index 84%
> rename from Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt
> rename to Documentation/devicetree/bindings/video/backlight/pm8941-wled.txt
> index a85a964d61f5..424f8444a6cd 100644
> --- a/Documentation/devicetree/bindings/leds/leds-pm8941-wled.txt
> +++ b/Documentation/devicetree/bindings/video/backlight/pm8941-wled.txt
> @@ -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..5ffa4b4e26c0 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -299,6 +299,13 @@ 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"
> + 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..c704c3236034 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;
> };
> @@ -432,4 +425,3 @@ module_platform_driver(pm8941_wled_driver);
>
> MODULE_DESCRIPTION("pm8941 wled driver");
> MODULE_LICENSE("GPL v2");
> -MODULE_ALIAS("platform:pm8941-wled");

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog