* [PATCHv7][ 1/2] backlight: gpio_backlight: Use a default state enum. @ 2013-12-05 17:55 Denis Carikli 2013-12-05 17:55 ` [PATCHv7][ 2/2] video: backlight: gpio-backlight: Add DT support Denis Carikli 0 siblings, 1 reply; 6+ messages in thread From: Denis Carikli @ 2013-12-05 17:55 UTC (permalink / raw) To: linux-arm-kernel That enum adds a "keep" state which permits to tell the driver trough its platform data not to touch the hardware during the probe. Cc: Richard Purdie <rpurdie@rpsys.net> Cc: Jingoo Han <jg1.han@samsung.com> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Rob Herring <rob.herring@calxeda.com> Cc: Pawel Moll <pawel.moll@arm.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Stephen Warren <swarren@wwwdotorg.org> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk> Cc: devicetree at vger.kernel.org Cc: Sascha Hauer <kernel@pengutronix.de> Cc: linux-arm-kernel at lists.infradead.org Cc: Lothar Wa?mann <LW@KARO-electronics.de> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com> Cc: Thierry Reding <thierry.reding@gmail.com> Cc: Eric B?nard <eric@eukrea.com> Signed-off-by: Denis Carikli <denis@eukrea.com> --- ChangeLog v5->v6: - New patch. --- drivers/video/backlight/gpio_backlight.c | 7 +++++-- include/linux/platform_data/gpio_backlight.h | 6 ++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index 81fb127..6ddeba9 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -23,6 +23,7 @@ struct gpio_backlight { int gpio; int active; + enum gpio_backlight_default_state def_value; }; static int gpio_backlight_update_status(struct backlight_device *bl) @@ -103,8 +104,10 @@ static int gpio_backlight_probe(struct platform_device *pdev) return PTR_ERR(bl); } - bl->props.brightness = pdata->def_value; - backlight_update_status(bl); + if (pdata->def_value != BACKLIGHT_GPIO_DEFSTATE_KEEP) { + bl->props.brightness = pdata->def_value; + backlight_update_status(bl); + } platform_set_drvdata(pdev, bl); return 0; diff --git a/include/linux/platform_data/gpio_backlight.h b/include/linux/platform_data/gpio_backlight.h index 5ae0d9c..3b437b3 100644 --- a/include/linux/platform_data/gpio_backlight.h +++ b/include/linux/platform_data/gpio_backlight.h @@ -10,6 +10,12 @@ struct device; +enum gpio_backlight_default_state { + BACKLIGHT_GPIO_DEFSTATE_OFF, + BACKLIGHT_GPIO_DEFSTATE_ON, + BACKLIGHT_GPIO_DEFSTATE_KEEP, +}; + struct gpio_backlight_platform_data { struct device *fbdev; int gpio; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCHv7][ 2/2] video: backlight: gpio-backlight: Add DT support. 2013-12-05 17:55 [PATCHv7][ 1/2] backlight: gpio_backlight: Use a default state enum Denis Carikli @ 2013-12-05 17:55 ` Denis Carikli 2013-12-06 13:00 ` Thierry Reding 0 siblings, 1 reply; 6+ messages in thread From: Denis Carikli @ 2013-12-05 17:55 UTC (permalink / raw) To: linux-arm-kernel Cc: Richard Purdie <rpurdie@rpsys.net> Cc: Jingoo Han <jg1.han@samsung.com> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Rob Herring <rob.herring@calxeda.com> Cc: Pawel Moll <pawel.moll@arm.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Stephen Warren <swarren@wwwdotorg.org> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk> Cc: devicetree at vger.kernel.org Cc: Sascha Hauer <kernel@pengutronix.de> Cc: linux-arm-kernel at lists.infradead.org Cc: Lothar Wa?mann <LW@KARO-electronics.de> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com> Cc: Thierry Reding <thierry.reding@gmail.com> Cc: Eric B?nard <eric@eukrea.com> Signed-off-by: Denis Carikli <denis@eukrea.com> --- ChangeLog v6->v7: - removed a compilation warning with the removal of the useless ret declaration. ChangeLog v5->v6: - The default state handling was reworked: - it's now called default-state, and looks like the gpio-leds default-state. - it now has a "keep" option, like for the gpio-leds. - that "keep" option is the default when the default-state property is not set. - The documentation was updated accordingly. ChangeLog v4->v5: - The default-brightness property now defaults to 0 in the driver. - def_value int becomes a bool. - The check for the gpio validity has been reworked. --- .../bindings/video/backlight/gpio-backlight.txt | 23 +++++++ drivers/video/backlight/gpio_backlight.c | 71 +++++++++++++++++--- 2 files changed, 86 insertions(+), 8 deletions(-) create mode 100644 Documentation/devicetree/bindings/video/backlight/gpio-backlight.txt diff --git a/Documentation/devicetree/bindings/video/backlight/gpio-backlight.txt b/Documentation/devicetree/bindings/video/backlight/gpio-backlight.txt new file mode 100644 index 0000000..f36f6e3 --- /dev/null +++ b/Documentation/devicetree/bindings/video/backlight/gpio-backlight.txt @@ -0,0 +1,23 @@ +gpio-backlight bindings + +Required properties: + - compatible: "gpio-backlight" + - gpios: describes the gpio that is used for enabling/disabling the backlight + (see GPIO binding[0] for more details). + +Optional properties: + - default-state: The initial state of the backlight. + Valid values are "on", "off", and "keep". + The "keep" setting will keep the backlight at whatever its current + state is, without producing a glitch. The default is keep if this + property is not present. + +[0]: Documentation/devicetree/bindings/gpio/gpio.txt + +Example: + + backlight { + compatible = "gpio-backlight"; + gpios = <&gpio3 4 0>; + default-state = "on"; + }; diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index 6ddeba9..2e20e32 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -13,6 +13,8 @@ #include <linux/init.h> #include <linux/kernel.h> #include <linux/module.h> +#include <linux/of.h> +#include <linux/of_gpio.h> #include <linux/platform_data/gpio_backlight.h> #include <linux/platform_device.h> #include <linux/slab.h> @@ -61,6 +63,42 @@ static const struct backlight_ops gpio_backlight_ops = { .check_fb = gpio_backlight_check_fb, }; +static int gpio_backlight_probe_dt(struct platform_device *pdev, + struct gpio_backlight *gbl) +{ + struct device_node *np = pdev->dev.of_node; + enum of_gpio_flags gpio_flags; + const char *state; + + gbl->fbdev = NULL; + gbl->gpio = of_get_gpio_flags(np, 0, &gpio_flags); + + if (!gpio_is_valid(gbl->gpio)) { + if (gbl->gpio != -EPROBE_DEFER) { + dev_err(&pdev->dev, + "Error: The gpios parameter is missing or invalid.\n"); + } + return gbl->gpio; + } + + gbl->active = (gpio_flags & OF_GPIO_ACTIVE_LOW) ? 0 : 1; + + state = of_get_property(np, "default-state", NULL); + if (state) { + if (!strcmp(state, "keep")) + gbl->def_value = BACKLIGHT_GPIO_DEFSTATE_KEEP; + else if (!strcmp(state, "on")) + gbl->def_value = BACKLIGHT_GPIO_DEFSTATE_ON; + else + gbl->def_value = BACKLIGHT_GPIO_DEFSTATE_OFF; + } else { + /* we don't want to touch the hardware if we're not told to */ + gbl->def_value = BACKLIGHT_GPIO_DEFSTATE_KEEP; + } + + return 0; +} + static int gpio_backlight_probe(struct platform_device *pdev) { struct gpio_backlight_platform_data *pdata = @@ -68,10 +106,12 @@ static int gpio_backlight_probe(struct platform_device *pdev) struct backlight_properties props; struct backlight_device *bl; struct gpio_backlight *gbl; + struct device_node *np = pdev->dev.of_node; int ret; - if (!pdata) { - dev_err(&pdev->dev, "failed to find platform data\n"); + if (!pdata && !np) { + dev_err(&pdev->dev, + "failed to find platform data or device tree node.\n"); return -ENODEV; } @@ -80,14 +120,23 @@ static int gpio_backlight_probe(struct platform_device *pdev) return -ENOMEM; gbl->dev = &pdev->dev; - gbl->fbdev = pdata->fbdev; - gbl->gpio = pdata->gpio; - gbl->active = pdata->active_low ? 0 : 1; + + if (np) { + ret = gpio_backlight_probe_dt(pdev, gbl); + if (ret) + return ret; + } else { + gbl->fbdev = pdata->fbdev; + gbl->gpio = pdata->gpio; + gbl->active = pdata->active_low ? 0 : 1; + if (pdata->def_value != BACKLIGHT_GPIO_DEFSTATE_KEEP) + gbl->def_value = pdata->def_value; + } ret = devm_gpio_request_one(gbl->dev, gbl->gpio, GPIOF_DIR_OUT | (gbl->active ? GPIOF_INIT_LOW : GPIOF_INIT_HIGH), - pdata->name); + pdata ? pdata->name : "backlight"); if (ret < 0) { dev_err(&pdev->dev, "unable to request GPIO\n"); return ret; @@ -104,8 +153,8 @@ static int gpio_backlight_probe(struct platform_device *pdev) return PTR_ERR(bl); } - if (pdata->def_value != BACKLIGHT_GPIO_DEFSTATE_KEEP) { - bl->props.brightness = pdata->def_value; + if (gbl->def_value != BACKLIGHT_GPIO_DEFSTATE_KEEP) { + bl->props.brightness = gbl->def_value; backlight_update_status(bl); } @@ -113,10 +162,16 @@ static int gpio_backlight_probe(struct platform_device *pdev) return 0; } +static struct of_device_id gpio_backlight_of_match[] = { + { .compatible = "gpio-backlight" }, + { /* sentinel */ } +}; + static struct platform_driver gpio_backlight_driver = { .driver = { .name = "gpio-backlight", .owner = THIS_MODULE, + .of_match_table = of_match_ptr(gpio_backlight_of_match), }, .probe = gpio_backlight_probe, }; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCHv7][ 2/2] video: backlight: gpio-backlight: Add DT support. 2013-12-05 17:55 ` [PATCHv7][ 2/2] video: backlight: gpio-backlight: Add DT support Denis Carikli @ 2013-12-06 13:00 ` Thierry Reding 2013-12-06 13:08 ` Alexander Shiyan 0 siblings, 1 reply; 6+ messages in thread From: Thierry Reding @ 2013-12-06 13:00 UTC (permalink / raw) To: linux-arm-kernel On Thu, Dec 05, 2013 at 06:55:09PM +0100, Denis Carikli wrote: [...] > +Optional properties: > + - default-state: The initial state of the backlight. > + Valid values are "on", "off", and "keep". > + The "keep" setting will keep the backlight at whatever its current > + state is, without producing a glitch. The default is keep if this > + property is not present. I'm not sure if "on", "off" and "keep" are a good choice for this binding. Having strings for these tristate values seems suboptimal. Other bindings have chosen a representation that, transposed to this use-case, would read something like this: - default-state: The initial state of the backlight. Valid values: - 0: off - 1: on If the "default-state" property is not present, the default will be to keep the current backlight state. Which is in fact the exact behaviour that your binding describes, but it's much more intuitive in my opinion. Thierry -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131206/fd9e72af/attachment.sig> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv7][ 2/2] video: backlight: gpio-backlight: Add DT support. 2013-12-06 13:00 ` Thierry Reding @ 2013-12-06 13:08 ` Alexander Shiyan 2013-12-06 14:12 ` Thierry Reding 0 siblings, 1 reply; 6+ messages in thread From: Alexander Shiyan @ 2013-12-06 13:08 UTC (permalink / raw) To: linux-arm-kernel > On Thu, Dec 05, 2013 at 06:55:09PM +0100, Denis Carikli wrote: > [...] > > +Optional properties: > > + - default-state: The initial state of the backlight. > > + Valid values are "on", "off", and "keep". > > + The "keep" setting will keep the backlight at whatever its current > > + state is, without producing a glitch. The default is keep if this > > + property is not present. > > I'm not sure if "on", "off" and "keep" are a good choice for this > binding. Having strings for these tristate values seems suboptimal. > Other bindings have chosen a representation that, transposed to this > use-case, would read something like this: > > - default-state: The initial state of the backlight. Valid > values: > - 0: off > - 1: on > > If the "default-state" property is not present, the default > will be to keep the current backlight state. > > Which is in fact the exact behaviour that your binding describes, but > it's much more intuitive in my opinion. Why we cannot use GPIO bindings for active level here? What a reason for "keep" state? Can this be an additional property? --- ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCHv7][ 2/2] video: backlight: gpio-backlight: Add DT support. 2013-12-06 13:08 ` Alexander Shiyan @ 2013-12-06 14:12 ` Thierry Reding 2013-12-06 14:48 ` Alexander Shiyan 0 siblings, 1 reply; 6+ messages in thread From: Thierry Reding @ 2013-12-06 14:12 UTC (permalink / raw) To: linux-arm-kernel On Fri, Dec 06, 2013 at 05:08:38PM +0400, Alexander Shiyan wrote: > > On Thu, Dec 05, 2013 at 06:55:09PM +0100, Denis Carikli wrote: > > [...] > > > +Optional properties: > > > + - default-state: The initial state of the backlight. > > > + Valid values are "on", "off", and "keep". > > > + The "keep" setting will keep the backlight at whatever its current > > > + state is, without producing a glitch. The default is keep if this > > > + property is not present. > > > > I'm not sure if "on", "off" and "keep" are a good choice for this > > binding. Having strings for these tristate values seems suboptimal. > > Other bindings have chosen a representation that, transposed to this > > use-case, would read something like this: > > > > - default-state: The initial state of the backlight. Valid > > values: > > - 0: off > > - 1: on > > > > If the "default-state" property is not present, the default > > will be to keep the current backlight state. > > > > Which is in fact the exact behaviour that your binding describes, but > > it's much more intuitive in my opinion. > > Why we cannot use GPIO bindings for active level here? > What a reason for "keep" state? Can this be an additional property? Default state and active level are two different things. Thierry -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131206/6b4b0460/attachment.sig> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv7][ 2/2] video: backlight: gpio-backlight: Add DT support. 2013-12-06 14:12 ` Thierry Reding @ 2013-12-06 14:48 ` Alexander Shiyan 0 siblings, 0 replies; 6+ messages in thread From: Alexander Shiyan @ 2013-12-06 14:48 UTC (permalink / raw) To: linux-arm-kernel > On Fri, Dec 06, 2013 at 05:08:38PM +0400, Alexander Shiyan wrote: > > > On Thu, Dec 05, 2013 at 06:55:09PM +0100, Denis Carikli wrote: > > > [...] > > > > +Optional properties: > > > > + - default-state: The initial state of the backlight. > > > > + Valid values are "on", "off", and "keep". > > > > + The "keep" setting will keep the backlight at whatever its current > > > > + state is, without producing a glitch. The default is keep if this > > > > + property is not present. > > > > > > I'm not sure if "on", "off" and "keep" are a good choice for this > > > binding. Having strings for these tristate values seems suboptimal. > > > Other bindings have chosen a representation that, transposed to this > > > use-case, would read something like this: > > > > > > - default-state: The initial state of the backlight. Valid > > > values: > > > - 0: off > > > - 1: on > > > > > > If the "default-state" property is not present, the default > > > will be to keep the current backlight state. > > > > > > Which is in fact the exact behaviour that your binding describes, but > > > it's much more intuitive in my opinion. > > > > Why we cannot use GPIO bindings for active level here? > > What a reason for "keep" state? Can this be an additional property? > > Default state and active level are two different things. OK. And what is "default" state? State before "unblank"? --- ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-12-06 14:48 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-12-05 17:55 [PATCHv7][ 1/2] backlight: gpio_backlight: Use a default state enum Denis Carikli 2013-12-05 17:55 ` [PATCHv7][ 2/2] video: backlight: gpio-backlight: Add DT support Denis Carikli 2013-12-06 13:00 ` Thierry Reding 2013-12-06 13:08 ` Alexander Shiyan 2013-12-06 14:12 ` Thierry Reding 2013-12-06 14:48 ` Alexander Shiyan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).