* [PATCH 1/2 v1] dt-bindings: backlight: Add Kinetic KTD253 bindings @ 2020-07-20 20:35 ` Linus Walleij 0 siblings, 0 replies; 16+ messages in thread From: Linus Walleij @ 2020-07-20 20:35 UTC (permalink / raw) To: Lee Jones, Daniel Thompson, Jingoo Han, dri-devel Cc: Linus Walleij, devicetree This adds device tree bindings for the Kinetic KTD253 white LED backlight driver. Cc: devicetree@vger.kernel.org Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- .../leds/backlight/kinetic,ktd253.yaml | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml diff --git a/Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml b/Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml new file mode 100644 index 000000000000..610bf9a0e270 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml @@ -0,0 +1,48 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/backlight/kinetic,ktd253.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Kinetic Technologies KTD253 one-wire backlight + +maintainers: + - Linus Walleij <linus.walleij@linaro.org> + +description: | + The Kinetic Technologies KTD253 is a white LED backlight that is + controlled by a single GPIO line. If you just turn on the backlight + it goes to maximum backlight then you can set the level of backlight + using pulses on the enable wire. + +properties: + compatible: + const: kinetic,ktd253 + + gpios: + description: GPIO to use to enable/disable and dim the backlight. + maxItems: 1 + + default-brightness: + description: Default brightness level on boot. 0 is off. + minimum: 0 + maximum: 255 + + max-brightness: + description: Maximum brightness that is allowed during runtime. + minimum: 0 + maximum: 255 + +required: + - compatible + - gpios + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + backlight { + compatible = "kinetic,ktd253"; + gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>; + default-on; + default-brightness = <160>; + }; -- 2.26.2 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 1/2 v1] dt-bindings: backlight: Add Kinetic KTD253 bindings @ 2020-07-20 20:35 ` Linus Walleij 0 siblings, 0 replies; 16+ messages in thread From: Linus Walleij @ 2020-07-20 20:35 UTC (permalink / raw) To: Lee Jones, Daniel Thompson, Jingoo Han, dri-devel; +Cc: devicetree This adds device tree bindings for the Kinetic KTD253 white LED backlight driver. Cc: devicetree@vger.kernel.org Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- .../leds/backlight/kinetic,ktd253.yaml | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml diff --git a/Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml b/Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml new file mode 100644 index 000000000000..610bf9a0e270 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml @@ -0,0 +1,48 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/backlight/kinetic,ktd253.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Kinetic Technologies KTD253 one-wire backlight + +maintainers: + - Linus Walleij <linus.walleij@linaro.org> + +description: | + The Kinetic Technologies KTD253 is a white LED backlight that is + controlled by a single GPIO line. If you just turn on the backlight + it goes to maximum backlight then you can set the level of backlight + using pulses on the enable wire. + +properties: + compatible: + const: kinetic,ktd253 + + gpios: + description: GPIO to use to enable/disable and dim the backlight. + maxItems: 1 + + default-brightness: + description: Default brightness level on boot. 0 is off. + minimum: 0 + maximum: 255 + + max-brightness: + description: Maximum brightness that is allowed during runtime. + minimum: 0 + maximum: 255 + +required: + - compatible + - gpios + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + backlight { + compatible = "kinetic,ktd253"; + gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>; + default-on; + default-brightness = <160>; + }; -- 2.26.2 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/2 v1] backlight: Add Kinetic KTD253 backlight driver 2020-07-20 20:35 ` Linus Walleij (?) @ 2020-07-20 20:35 ` Linus Walleij 2020-07-21 8:49 ` Sam Ravnborg -1 siblings, 1 reply; 16+ messages in thread From: Linus Walleij @ 2020-07-20 20:35 UTC (permalink / raw) To: Lee Jones, Daniel Thompson, Jingoo Han, dri-devel The Kinetic KTD253 backlight driver is controlled with a single GPIO line, but still supports a range of brightness settings by sending fast pulses on the line. This is based off the source code release for the Samsung GT-S7710 mobile phone. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- MAINTAINERS | 6 + drivers/video/backlight/Kconfig | 8 + drivers/video/backlight/Makefile | 1 + drivers/video/backlight/ktd253-backlight.c | 254 +++++++++++++++++++++ 4 files changed, 269 insertions(+) create mode 100644 drivers/video/backlight/ktd253-backlight.c diff --git a/MAINTAINERS b/MAINTAINERS index b4a43a9e7fbc..ea6fcc5bb79e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9610,6 +9610,12 @@ F: Documentation/admin-guide/auxdisplay/ks0108.rst F: drivers/auxdisplay/ks0108.c F: include/linux/ks0108.h +KTD253 BACKLIGHT DRIVER +M: Linus Walleij <linus.walleij@linaro.org> +S: Maintained +F: Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml +F: drivers/video/backlight/ktd253-backlight.c + L3MDEV M: David Ahern <dsahern@kernel.org> L: netdev@vger.kernel.org diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig index 7d22d7377606..6a74c60707b4 100644 --- a/drivers/video/backlight/Kconfig +++ b/drivers/video/backlight/Kconfig @@ -190,6 +190,14 @@ config BACKLIGHT_IPAQ_MICRO computers. Say yes if you have one of the h3100/h3600/h3700 machines. +config BACKLIGHT_KTD253 + tristate "Backlight Driver for Kinetic KTD253" + depends on GPIOLIB || COMPILE_TEST + help + Say y to enabled the backlight driver for the Kinetic KTD253 + which is a 1-wire GPIO-controlled backlight found in some mobile + phones. + config BACKLIGHT_LM3533 tristate "Backlight Driver for LM3533" depends on MFD_LM3533 diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile index 0c1a1524627a..d50cd12574ae 100644 --- a/drivers/video/backlight/Makefile +++ b/drivers/video/backlight/Makefile @@ -36,6 +36,7 @@ obj-$(CONFIG_BACKLIGHT_GPIO) += gpio_backlight.o obj-$(CONFIG_BACKLIGHT_HP680) += hp680_bl.o obj-$(CONFIG_BACKLIGHT_HP700) += jornada720_bl.o obj-$(CONFIG_BACKLIGHT_IPAQ_MICRO) += ipaq_micro_bl.o +obj-$(CONFIG_BACKLIGHT_KTD253) += ktd253-backlight.o obj-$(CONFIG_BACKLIGHT_LM3533) += lm3533_bl.o obj-$(CONFIG_BACKLIGHT_LM3630A) += lm3630a_bl.o obj-$(CONFIG_BACKLIGHT_LM3639) += lm3639_bl.o diff --git a/drivers/video/backlight/ktd253-backlight.c b/drivers/video/backlight/ktd253-backlight.c new file mode 100644 index 000000000000..d460d1fef329 --- /dev/null +++ b/drivers/video/backlight/ktd253-backlight.c @@ -0,0 +1,254 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Backlight driver for the Kinetic KTD253 + * Based on code and know-how from the Samsung GT-S7710 + * Gareth Phillips <gareth.phillips@samsung.com> + */ +#include <linux/backlight.h> +#include <linux/err.h> +#include <linux/fb.h> +#include <linux/gpio/consumer.h> +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/property.h> +#include <linux/slab.h> +#include <linux/delay.h> +#include <linux/limits.h> + +/* Current ratio is n/32 from 1/32 to 32/32 */ +#define KTD253_MIN_RATIO 1 +#define KTD253_MAX_RATIO 32 +#define KTD253_DEFAULT_RATIO 13 + +/* With the table we use this is 24/32 current ratio actually */ +#define KTD253_MAX_BRIGHTNESS 255 +#define KTD253_DEFAULT_BRIGHTNESS 160 + +#define KTD253_T_LOW_NS (200 + 10) /* Additional 10ns as safety factor */ +#define KTD253_T_HIGH_NS (200 + 10) /* Additional 10ns as safety factor */ +#define KTD253_T_OFF_MS 3 + +struct ktd253_backlight { + struct device *dev; + struct gpio_desc *gpiod; + u16 ratio; + unsigned int brightness; +}; + +/* + * The following table is used to convert brightness level to the LED + * Current Ratio expressed as (full current) /(n * 32). + * i.e. 1 = 1/32 full current. Zero indicates LED is powered off. + * The table is intended to allow the brightness level to be "tuned" + * to compensate for non-linearity of brightness relative to current. + */ +static const u16 ktd253_brightness_to_current_ratio[] = { + 0, /* (0/32) KTD253_BACKLIGHT_OFF */ + 39, /* (1/32) KTD253_MIN_RATIO */ + 58, /* (2/32) */ + 67, /* (3/32) */ + 76, /* (4/32) */ + 85, /* (5/32) */ + 94, /* (6/32) */ + 104, /* (7/32) */ + 113, /* (8/32) */ + 122, /* (9/32) */ + 131, /* (10/32) */ + 145, /* (11/32) */ + 159, /* (12/32) */ + 169, /* (13/32) */ + 179, /* (14/32) */ + 189, /* (15/32) */ + 196, /* (16/32) */ + 203, /* (17/32) */ + 210, /* (18/32) */ + 217, /* (19/32) */ + 224, /* (20/32) */ + 231, /* (21/32) */ + 238, /* (22/32) */ + 245, /* (23/32) */ + 255, /* (24/32) */ + 300, /* (25/32) */ + 300, /* (26/32) */ + 300, /* (27/32) */ + 300, /* (28/32) */ + 300, /* (29/32) */ + 300, /* (30/32) */ + 300, /* (31/32) */ + 300 /* (32/32) KTD253_MAX_RATIO */ + +}; + +/* Inspired by gpio_bl.c */ +static int ktd253_backlight_get_next_brightness(struct backlight_device *bl) +{ + int brightness = bl->props.brightness; + + if (bl->props.power != FB_BLANK_UNBLANK || + bl->props.fb_blank != FB_BLANK_UNBLANK || + bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK)) + brightness = 0; + + return brightness; +} + +static int ktd253_backlight_update_status(struct backlight_device *bl) +{ + struct ktd253_backlight *ktd253 = bl_get_data(bl); + int brightness = ktd253_backlight_get_next_brightness(bl); + u16 target_ratio; + u16 current_ratio = ktd253->ratio; + unsigned long flags; + + dev_dbg(ktd253->dev, "new brightness: %d\n", brightness); + + /* Look up the current ratio */ + for (target_ratio = KTD253_MAX_RATIO; target_ratio > 0; target_ratio--) { + if (brightness > ktd253_brightness_to_current_ratio[target_ratio - 1]) + break; + } + + dev_dbg(ktd253->dev, "new ratio: %d/32\n", target_ratio); + + if (target_ratio == current_ratio) + /* This is already right */ + return 0; + + if (target_ratio == 0) { + gpiod_set_value_cansleep(ktd253->gpiod, 0); + /* + * We need to keep the GPIO low for at least this long + * to actually switch the KTD253 off. + */ + msleep(KTD253_T_OFF_MS); + ktd253->ratio = 0; + return 0; + } + + if (current_ratio == 0) { + gpiod_set_value_cansleep(ktd253->gpiod, 1); + ndelay(KTD253_T_HIGH_NS); + /* We always fall back to this when we power on */ + current_ratio = KTD253_MAX_RATIO; + } + + /* + * WARNING: + * The loop to set the correct current level is performed + * with interrupts disabled as it is timing critical. + * The maximum number of cycles of the loop is 32 + * so the time taken will be (T_LOW_NS + T_HIGH_NS + loop_time) * 32, + */ + local_irq_save(flags); + while (current_ratio != target_ratio) { + /* + * These GPIO operations absolutely can NOT sleep so no + * _cansleep suffixes, and no using GPIO expanders on + * slow buses for this! + */ + gpiod_set_value(ktd253->gpiod, 0); + ndelay(KTD253_T_LOW_NS); + gpiod_set_value(ktd253->gpiod, 1); + ndelay(KTD253_T_HIGH_NS); + /* After 1/32 we loop back to 32/32 */ + if (current_ratio == KTD253_MIN_RATIO) + current_ratio = KTD253_MAX_RATIO; + else + current_ratio--; + } + local_irq_restore(flags); + ktd253->ratio = current_ratio; + + dev_dbg(ktd253->dev, "new ratio set to %d/32\n", target_ratio); + + return 0; +} + +static const struct backlight_ops ktd253_backlight_ops = { + .options = BL_CORE_SUSPENDRESUME, + .update_status = ktd253_backlight_update_status, +}; + +static int ktd253_backlight_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct backlight_device *bl; + struct ktd253_backlight *ktd253; + u32 max_brightness; + u32 brightness; + int ret; + + ktd253 = devm_kzalloc(dev, sizeof(*ktd253), GFP_KERNEL); + if (!ktd253) + return -ENOMEM; + ktd253->dev = dev; + + ret = device_property_read_u32(dev, "max-brightness", &max_brightness); + if (ret) + max_brightness = KTD253_MAX_BRIGHTNESS; + + ret = device_property_read_u32(dev, "default-brightness", &brightness); + if (ret) + brightness = KTD253_DEFAULT_BRIGHTNESS; + + if (brightness) + /* This will be the default ratio when the KTD253 is enabled */ + ktd253->ratio = KTD253_MAX_RATIO; + else + ktd253->ratio = 0; + + ktd253->gpiod = devm_gpiod_get(dev, NULL, + brightness ? GPIOD_OUT_HIGH : + GPIOD_OUT_LOW); + if (IS_ERR(ktd253->gpiod)) { + ret = PTR_ERR(ktd253->gpiod); + if (ret != -EPROBE_DEFER) + dev_err(dev, "gpio line missing or invalid.\n"); + return ret; + } + gpiod_set_consumer_name(ktd253->gpiod, dev_name(dev)); + + bl = devm_backlight_device_register(dev, dev_name(dev), dev, ktd253, + &ktd253_backlight_ops, NULL); + if (IS_ERR(bl)) { + dev_err(dev, "failed to register backlight\n"); + return PTR_ERR(bl); + } + bl->props.max_brightness = max_brightness; + /* When we just enable the GPIO line we set max brightness */ + if (brightness) { + bl->props.brightness = brightness; + bl->props.power = FB_BLANK_UNBLANK; + ktd253_backlight_update_status(bl); + } else { + bl->props.brightness = 0; + bl->props.power = FB_BLANK_POWERDOWN; + } + + platform_set_drvdata(pdev, bl); + + return 0; +} + +static const struct of_device_id ktd253_backlight_of_match[] = { + { .compatible = "kinetic,ktd253" }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, ktd253_backlight_of_match); + +static struct platform_driver ktd253_backlight_driver = { + .driver = { + .name = "ktd253-backlight", + .of_match_table = ktd253_backlight_of_match, + }, + .probe = ktd253_backlight_probe, +}; +module_platform_driver(ktd253_backlight_driver); + +MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>"); +MODULE_DESCRIPTION("Kinetic KTD253 Backlight Driver"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS("platform:ktd253-backlight"); -- 2.26.2 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2 v1] backlight: Add Kinetic KTD253 backlight driver 2020-07-20 20:35 ` [PATCH 2/2 v1] backlight: Add Kinetic KTD253 backlight driver Linus Walleij @ 2020-07-21 8:49 ` Sam Ravnborg 0 siblings, 0 replies; 16+ messages in thread From: Sam Ravnborg @ 2020-07-21 8:49 UTC (permalink / raw) To: Linus Walleij; +Cc: Jingoo Han, Daniel Thompson, Lee Jones, dri-devel Hi Linus. On Mon, Jul 20, 2020 at 10:35:06PM +0200, Linus Walleij wrote: > The Kinetic KTD253 backlight driver is controlled with a > single GPIO line, but still supports a range of brightness > settings by sending fast pulses on the line. > > This is based off the source code release for the Samsung > GT-S7710 mobile phone. > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> In -next there is a few updated to backlight stuff that this driver could benefit from. My comments in the following assumes you have the latest -next. Sam > --- > MAINTAINERS | 6 + > drivers/video/backlight/Kconfig | 8 + > drivers/video/backlight/Makefile | 1 + > drivers/video/backlight/ktd253-backlight.c | 254 +++++++++++++++++++++ > 4 files changed, 269 insertions(+) > create mode 100644 drivers/video/backlight/ktd253-backlight.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index b4a43a9e7fbc..ea6fcc5bb79e 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -9610,6 +9610,12 @@ F: Documentation/admin-guide/auxdisplay/ks0108.rst > F: drivers/auxdisplay/ks0108.c > F: include/linux/ks0108.h > > +KTD253 BACKLIGHT DRIVER > +M: Linus Walleij <linus.walleij@linaro.org> > +S: Maintained > +F: Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml > +F: drivers/video/backlight/ktd253-backlight.c > + > L3MDEV > M: David Ahern <dsahern@kernel.org> > L: netdev@vger.kernel.org > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig > index 7d22d7377606..6a74c60707b4 100644 > --- a/drivers/video/backlight/Kconfig > +++ b/drivers/video/backlight/Kconfig > @@ -190,6 +190,14 @@ config BACKLIGHT_IPAQ_MICRO > computers. Say yes if you have one of the h3100/h3600/h3700 > machines. > > +config BACKLIGHT_KTD253 > + tristate "Backlight Driver for Kinetic KTD253" > + depends on GPIOLIB || COMPILE_TEST > + help > + Say y to enabled the backlight driver for the Kinetic KTD253 > + which is a 1-wire GPIO-controlled backlight found in some mobile > + phones. > + > config BACKLIGHT_LM3533 > tristate "Backlight Driver for LM3533" > depends on MFD_LM3533 > diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile > index 0c1a1524627a..d50cd12574ae 100644 > --- a/drivers/video/backlight/Makefile > +++ b/drivers/video/backlight/Makefile > @@ -36,6 +36,7 @@ obj-$(CONFIG_BACKLIGHT_GPIO) += gpio_backlight.o > obj-$(CONFIG_BACKLIGHT_HP680) += hp680_bl.o > obj-$(CONFIG_BACKLIGHT_HP700) += jornada720_bl.o > obj-$(CONFIG_BACKLIGHT_IPAQ_MICRO) += ipaq_micro_bl.o > +obj-$(CONFIG_BACKLIGHT_KTD253) += ktd253-backlight.o > obj-$(CONFIG_BACKLIGHT_LM3533) += lm3533_bl.o > obj-$(CONFIG_BACKLIGHT_LM3630A) += lm3630a_bl.o > obj-$(CONFIG_BACKLIGHT_LM3639) += lm3639_bl.o > diff --git a/drivers/video/backlight/ktd253-backlight.c b/drivers/video/backlight/ktd253-backlight.c > new file mode 100644 > index 000000000000..d460d1fef329 > --- /dev/null > +++ b/drivers/video/backlight/ktd253-backlight.c > @@ -0,0 +1,254 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Backlight driver for the Kinetic KTD253 > + * Based on code and know-how from the Samsung GT-S7710 > + * Gareth Phillips <gareth.phillips@samsung.com> > + */ > +#include <linux/backlight.h> > +#include <linux/err.h> > +#include <linux/fb.h> > +#include <linux/gpio/consumer.h> > +#include <linux/init.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/property.h> > +#include <linux/slab.h> > +#include <linux/delay.h> > +#include <linux/limits.h> In drm land these needs to be sorted. I do not think backlight demands it. > + > +/* Current ratio is n/32 from 1/32 to 32/32 */ > +#define KTD253_MIN_RATIO 1 > +#define KTD253_MAX_RATIO 32 > +#define KTD253_DEFAULT_RATIO 13 > + > +/* With the table we use this is 24/32 current ratio actually */ > +#define KTD253_MAX_BRIGHTNESS 255 > +#define KTD253_DEFAULT_BRIGHTNESS 160 > + > +#define KTD253_T_LOW_NS (200 + 10) /* Additional 10ns as safety factor */ > +#define KTD253_T_HIGH_NS (200 + 10) /* Additional 10ns as safety factor */ > +#define KTD253_T_OFF_MS 3 > + > +struct ktd253_backlight { > + struct device *dev; > + struct gpio_desc *gpiod; > + u16 ratio; > + unsigned int brightness; brightness is not used - delete. > +}; I had expected to see a backlight pointer in the above structure. Like we do in most drivers. > + > +/* > + * The following table is used to convert brightness level to the LED > + * Current Ratio expressed as (full current) /(n * 32). > + * i.e. 1 = 1/32 full current. Zero indicates LED is powered off. > + * The table is intended to allow the brightness level to be "tuned" > + * to compensate for non-linearity of brightness relative to current. > + */ > +static const u16 ktd253_brightness_to_current_ratio[] = { > + 0, /* (0/32) KTD253_BACKLIGHT_OFF */ > + 39, /* (1/32) KTD253_MIN_RATIO */ > + 58, /* (2/32) */ > + 67, /* (3/32) */ > + 76, /* (4/32) */ > + 85, /* (5/32) */ > + 94, /* (6/32) */ > + 104, /* (7/32) */ > + 113, /* (8/32) */ > + 122, /* (9/32) */ > + 131, /* (10/32) */ > + 145, /* (11/32) */ > + 159, /* (12/32) */ > + 169, /* (13/32) */ > + 179, /* (14/32) */ > + 189, /* (15/32) */ > + 196, /* (16/32) */ > + 203, /* (17/32) */ > + 210, /* (18/32) */ > + 217, /* (19/32) */ > + 224, /* (20/32) */ > + 231, /* (21/32) */ > + 238, /* (22/32) */ > + 245, /* (23/32) */ > + 255, /* (24/32) */ > + 300, /* (25/32) */ > + 300, /* (26/32) */ > + 300, /* (27/32) */ > + 300, /* (28/32) */ > + 300, /* (29/32) */ > + 300, /* (30/32) */ > + 300, /* (31/32) */ > + 300 /* (32/32) KTD253_MAX_RATIO */ > + > +}; > + > +/* Inspired by gpio_bl.c */ > +static int ktd253_backlight_get_next_brightness(struct backlight_device *bl) > +{ > + int brightness = bl->props.brightness; > + > + if (bl->props.power != FB_BLANK_UNBLANK || > + bl->props.fb_blank != FB_BLANK_UNBLANK || > + bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK)) > + brightness = 0; > + > + return brightness; > +} > + > +static int ktd253_backlight_update_status(struct backlight_device *bl) > +{ > + struct ktd253_backlight *ktd253 = bl_get_data(bl); > + int brightness = ktd253_backlight_get_next_brightness(bl); Use backligt_get_brightness() to get the brightness. Then you can delete ktd253_backlight_get_next_brightness() > + u16 target_ratio; > + u16 current_ratio = ktd253->ratio; > + unsigned long flags; > + > + dev_dbg(ktd253->dev, "new brightness: %d\n", brightness); > + > + /* Look up the current ratio */ > + for (target_ratio = KTD253_MAX_RATIO; target_ratio > 0; target_ratio--) { > + if (brightness > ktd253_brightness_to_current_ratio[target_ratio - 1]) > + break; > + } > + > + dev_dbg(ktd253->dev, "new ratio: %d/32\n", target_ratio); Maybe only one print with both brightness and ratio? > + > + if (target_ratio == current_ratio) > + /* This is already right */ > + return 0; > + > + if (target_ratio == 0) { > + gpiod_set_value_cansleep(ktd253->gpiod, 0); > + /* > + * We need to keep the GPIO low for at least this long > + * to actually switch the KTD253 off. > + */ > + msleep(KTD253_T_OFF_MS); > + ktd253->ratio = 0; > + return 0; > + } > + > + if (current_ratio == 0) { > + gpiod_set_value_cansleep(ktd253->gpiod, 1); > + ndelay(KTD253_T_HIGH_NS); > + /* We always fall back to this when we power on */ > + current_ratio = KTD253_MAX_RATIO; > + } > + > + /* > + * WARNING: > + * The loop to set the correct current level is performed > + * with interrupts disabled as it is timing critical. > + * The maximum number of cycles of the loop is 32 > + * so the time taken will be (T_LOW_NS + T_HIGH_NS + loop_time) * 32, > + */ > + local_irq_save(flags); > + while (current_ratio != target_ratio) { > + /* > + * These GPIO operations absolutely can NOT sleep so no > + * _cansleep suffixes, and no using GPIO expanders on > + * slow buses for this! > + */ > + gpiod_set_value(ktd253->gpiod, 0); > + ndelay(KTD253_T_LOW_NS); > + gpiod_set_value(ktd253->gpiod, 1); > + ndelay(KTD253_T_HIGH_NS); > + /* After 1/32 we loop back to 32/32 */ > + if (current_ratio == KTD253_MIN_RATIO) > + current_ratio = KTD253_MAX_RATIO; > + else > + current_ratio--; > + } > + local_irq_restore(flags); > + ktd253->ratio = current_ratio; > + > + dev_dbg(ktd253->dev, "new ratio set to %d/32\n", target_ratio); > + > + return 0; > +} > + > +static const struct backlight_ops ktd253_backlight_ops = { > + .options = BL_CORE_SUSPENDRESUME, > + .update_status = ktd253_backlight_update_status, > +}; > + > +static int ktd253_backlight_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct backlight_device *bl; > + struct ktd253_backlight *ktd253; > + u32 max_brightness; > + u32 brightness; > + int ret; > + > + ktd253 = devm_kzalloc(dev, sizeof(*ktd253), GFP_KERNEL); > + if (!ktd253) > + return -ENOMEM; > + ktd253->dev = dev; > + > + ret = device_property_read_u32(dev, "max-brightness", &max_brightness); > + if (ret) > + max_brightness = KTD253_MAX_BRIGHTNESS; > + > + ret = device_property_read_u32(dev, "default-brightness", &brightness); > + if (ret) > + brightness = KTD253_DEFAULT_BRIGHTNESS; > + > + if (brightness) > + /* This will be the default ratio when the KTD253 is enabled */ > + ktd253->ratio = KTD253_MAX_RATIO; > + else > + ktd253->ratio = 0; > + > + ktd253->gpiod = devm_gpiod_get(dev, NULL, > + brightness ? GPIOD_OUT_HIGH : > + GPIOD_OUT_LOW); > + if (IS_ERR(ktd253->gpiod)) { > + ret = PTR_ERR(ktd253->gpiod); > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "gpio line missing or invalid.\n"); > + return ret; > + } > + gpiod_set_consumer_name(ktd253->gpiod, dev_name(dev)); > + > + bl = devm_backlight_device_register(dev, dev_name(dev), dev, ktd253, > + &ktd253_backlight_ops, NULL); > + if (IS_ERR(bl)) { > + dev_err(dev, "failed to register backlight\n"); > + return PTR_ERR(bl); > + } > + bl->props.max_brightness = max_brightness; > + /* When we just enable the GPIO line we set max brightness */ > + if (brightness) { > + bl->props.brightness = brightness; > + bl->props.power = FB_BLANK_UNBLANK; > + ktd253_backlight_update_status(bl); > + } else { > + bl->props.brightness = 0; > + bl->props.power = FB_BLANK_POWERDOWN; > + } Pass a backlight_properties to devm_backlight_device_register. So this is correct at init time. FB_BLANK_* are constant used by the fb_blank icotl - and should not be used here. Do not assign props.power - as there is no change in power state to report. In other words: Init backlight_properties with: - max_brightness - brightness - Type (RAW) Call devm_backlight_device_register() Then unconditionally call backlight_update_status() (Not the local variant, go via backlight core) The above is my understandig - but let the backlight people chime in too. I would love if we could make it simpler to register a backlight device... Sam > + > + platform_set_drvdata(pdev, bl); > + > + return 0; > +} > + > +static const struct of_device_id ktd253_backlight_of_match[] = { > + { .compatible = "kinetic,ktd253" }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, ktd253_backlight_of_match); > + > +static struct platform_driver ktd253_backlight_driver = { > + .driver = { > + .name = "ktd253-backlight", > + .of_match_table = ktd253_backlight_of_match, > + }, > + .probe = ktd253_backlight_probe, > +}; > +module_platform_driver(ktd253_backlight_driver); > + > +MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>"); > +MODULE_DESCRIPTION("Kinetic KTD253 Backlight Driver"); > +MODULE_LICENSE("GPL"); > +MODULE_ALIAS("platform:ktd253-backlight"); > -- > 2.26.2 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2 v1] dt-bindings: backlight: Add Kinetic KTD253 bindings 2020-07-20 20:35 ` Linus Walleij @ 2020-07-21 8:32 ` Sam Ravnborg -1 siblings, 0 replies; 16+ messages in thread From: Sam Ravnborg @ 2020-07-21 8:32 UTC (permalink / raw) To: Linus Walleij Cc: Lee Jones, Daniel Thompson, Jingoo Han, dri-devel, devicetree Hi Linus. On Mon, Jul 20, 2020 at 10:35:05PM +0200, Linus Walleij wrote: > This adds device tree bindings for the Kinetic KTD253 > white LED backlight driver. > > Cc: devicetree@vger.kernel.org > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> See a few comments in the following. Sam > --- > .../leds/backlight/kinetic,ktd253.yaml | 48 +++++++++++++++++++ > 1 file changed, 48 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml > > diff --git a/Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml b/Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml > new file mode 100644 > index 000000000000..610bf9a0e270 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml > @@ -0,0 +1,48 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/backlight/kinetic,ktd253.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Kinetic Technologies KTD253 one-wire backlight > + > +maintainers: > + - Linus Walleij <linus.walleij@linaro.org> > + > +description: | > + The Kinetic Technologies KTD253 is a white LED backlight that is > + controlled by a single GPIO line. If you just turn on the backlight > + it goes to maximum backlight then you can set the level of backlight > + using pulses on the enable wire. No $ref for common.yaml? > + > +properties: > + compatible: > + const: kinetic,ktd253 > + > + gpios: A less generic and more descriptive name would be good. > + description: GPIO to use to enable/disable and dim the backlight. > + maxItems: 1 > + > + default-brightness: > + description: Default brightness level on boot. 0 is off. > + minimum: 0 > + maximum: 255 > + > + max-brightness: > + description: Maximum brightness that is allowed during runtime. > + minimum: 0 > + maximum: 255 Both looks like candidates for common.yaml - they are used by other bindings. > + > +required: > + - compatible > + - gpios It would make senste that maximum-brighness was mandatory too. addtionalProperties: false?? > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + backlight { > + compatible = "kinetic,ktd253"; > + gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>; > + default-on; default-on is not documented - and not part of common.yaml. > + default-brightness = <160>; > + }; > -- > 2.26.2 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2 v1] dt-bindings: backlight: Add Kinetic KTD253 bindings @ 2020-07-21 8:32 ` Sam Ravnborg 0 siblings, 0 replies; 16+ messages in thread From: Sam Ravnborg @ 2020-07-21 8:32 UTC (permalink / raw) To: Linus Walleij Cc: Jingoo Han, Daniel Thompson, Lee Jones, dri-devel, devicetree Hi Linus. On Mon, Jul 20, 2020 at 10:35:05PM +0200, Linus Walleij wrote: > This adds device tree bindings for the Kinetic KTD253 > white LED backlight driver. > > Cc: devicetree@vger.kernel.org > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> See a few comments in the following. Sam > --- > .../leds/backlight/kinetic,ktd253.yaml | 48 +++++++++++++++++++ > 1 file changed, 48 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml > > diff --git a/Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml b/Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml > new file mode 100644 > index 000000000000..610bf9a0e270 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml > @@ -0,0 +1,48 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/backlight/kinetic,ktd253.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Kinetic Technologies KTD253 one-wire backlight > + > +maintainers: > + - Linus Walleij <linus.walleij@linaro.org> > + > +description: | > + The Kinetic Technologies KTD253 is a white LED backlight that is > + controlled by a single GPIO line. If you just turn on the backlight > + it goes to maximum backlight then you can set the level of backlight > + using pulses on the enable wire. No $ref for common.yaml? > + > +properties: > + compatible: > + const: kinetic,ktd253 > + > + gpios: A less generic and more descriptive name would be good. > + description: GPIO to use to enable/disable and dim the backlight. > + maxItems: 1 > + > + default-brightness: > + description: Default brightness level on boot. 0 is off. > + minimum: 0 > + maximum: 255 > + > + max-brightness: > + description: Maximum brightness that is allowed during runtime. > + minimum: 0 > + maximum: 255 Both looks like candidates for common.yaml - they are used by other bindings. > + > +required: > + - compatible > + - gpios It would make senste that maximum-brighness was mandatory too. addtionalProperties: false?? > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + backlight { > + compatible = "kinetic,ktd253"; > + gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>; > + default-on; default-on is not documented - and not part of common.yaml. > + default-brightness = <160>; > + }; > -- > 2.26.2 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2 v1] dt-bindings: backlight: Add Kinetic KTD253 bindings 2020-07-21 8:32 ` Sam Ravnborg @ 2020-08-12 6:48 ` Linus Walleij -1 siblings, 0 replies; 16+ messages in thread From: Linus Walleij @ 2020-08-12 6:48 UTC (permalink / raw) To: Sam Ravnborg Cc: Lee Jones, Daniel Thompson, Jingoo Han, open list:DRM PANEL DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS On Tue, Jul 21, 2020 at 10:32 AM Sam Ravnborg <sam@ravnborg.org> wrote: > > +description: | > > + The Kinetic Technologies KTD253 is a white LED backlight that is > > + controlled by a single GPIO line. If you just turn on the backlight > > + it goes to maximum backlight then you can set the level of backlight > > + using pulses on the enable wire. > > No $ref for common.yaml? Since this is a backlight, and we do not have common bindings for, backlight I first looked into using the LED bindings in ../common.yaml, but that has several problems, it cannot really be used for backlight. Backlight doesn't have "triggers", patterns, flash properties, the function is also pretty much evident. So I will look into creating a new common for backlight. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2 v1] dt-bindings: backlight: Add Kinetic KTD253 bindings @ 2020-08-12 6:48 ` Linus Walleij 0 siblings, 0 replies; 16+ messages in thread From: Linus Walleij @ 2020-08-12 6:48 UTC (permalink / raw) To: Sam Ravnborg Cc: Jingoo Han, Daniel Thompson, Lee Jones, open list:DRM PANEL DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS On Tue, Jul 21, 2020 at 10:32 AM Sam Ravnborg <sam@ravnborg.org> wrote: > > +description: | > > + The Kinetic Technologies KTD253 is a white LED backlight that is > > + controlled by a single GPIO line. If you just turn on the backlight > > + it goes to maximum backlight then you can set the level of backlight > > + using pulses on the enable wire. > > No $ref for common.yaml? Since this is a backlight, and we do not have common bindings for, backlight I first looked into using the LED bindings in ../common.yaml, but that has several problems, it cannot really be used for backlight. Backlight doesn't have "triggers", patterns, flash properties, the function is also pretty much evident. So I will look into creating a new common for backlight. Yours, Linus Walleij _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2 v1] dt-bindings: backlight: Add Kinetic KTD253 bindings 2020-08-12 6:48 ` Linus Walleij @ 2020-08-12 7:34 ` Sam Ravnborg -1 siblings, 0 replies; 16+ messages in thread From: Sam Ravnborg @ 2020-08-12 7:34 UTC (permalink / raw) To: Linus Walleij Cc: Lee Jones, Daniel Thompson, Jingoo Han, open list:DRM PANEL DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS Hi Linus. On Wed, Aug 12, 2020 at 08:48:42AM +0200, Linus Walleij wrote: > On Tue, Jul 21, 2020 at 10:32 AM Sam Ravnborg <sam@ravnborg.org> wrote: > > > > +description: | > > > + The Kinetic Technologies KTD253 is a white LED backlight that is > > > + controlled by a single GPIO line. If you just turn on the backlight > > > + it goes to maximum backlight then you can set the level of backlight > > > + using pulses on the enable wire. > > > > No $ref for common.yaml? > > Since this is a backlight, and we do not have common bindings for, > backlight I first looked into using the LED bindings in > ../common.yaml, but that has several problems, it cannot really > be used for backlight. Backlight doesn't have "triggers", > patterns, flash properties, the function is also pretty much > evident. > > So I will look into creating a new common for backlight. Hmm, I think I looked at leds/ when I wrote that comment about common.yaml. Please consider Rob's comment in commit: 44e1655a444fe7a1bd81994d34c6bbb5245b9e60 ("dt-bindings: backlight: Convert common backlight bindings to DT schema") Rob did not see the need for a common binding - but that may change as we add more backlight bindings. Sam ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2 v1] dt-bindings: backlight: Add Kinetic KTD253 bindings @ 2020-08-12 7:34 ` Sam Ravnborg 0 siblings, 0 replies; 16+ messages in thread From: Sam Ravnborg @ 2020-08-12 7:34 UTC (permalink / raw) To: Linus Walleij Cc: Jingoo Han, Daniel Thompson, Lee Jones, open list:DRM PANEL DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS Hi Linus. On Wed, Aug 12, 2020 at 08:48:42AM +0200, Linus Walleij wrote: > On Tue, Jul 21, 2020 at 10:32 AM Sam Ravnborg <sam@ravnborg.org> wrote: > > > > +description: | > > > + The Kinetic Technologies KTD253 is a white LED backlight that is > > > + controlled by a single GPIO line. If you just turn on the backlight > > > + it goes to maximum backlight then you can set the level of backlight > > > + using pulses on the enable wire. > > > > No $ref for common.yaml? > > Since this is a backlight, and we do not have common bindings for, > backlight I first looked into using the LED bindings in > ../common.yaml, but that has several problems, it cannot really > be used for backlight. Backlight doesn't have "triggers", > patterns, flash properties, the function is also pretty much > evident. > > So I will look into creating a new common for backlight. Hmm, I think I looked at leds/ when I wrote that comment about common.yaml. Please consider Rob's comment in commit: 44e1655a444fe7a1bd81994d34c6bbb5245b9e60 ("dt-bindings: backlight: Convert common backlight bindings to DT schema") Rob did not see the need for a common binding - but that may change as we add more backlight bindings. Sam _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2 v1] dt-bindings: backlight: Add Kinetic KTD253 bindings 2020-08-12 7:34 ` Sam Ravnborg @ 2020-08-12 9:09 ` Linus Walleij -1 siblings, 0 replies; 16+ messages in thread From: Linus Walleij @ 2020-08-12 9:09 UTC (permalink / raw) To: Sam Ravnborg Cc: Lee Jones, Daniel Thompson, Jingoo Han, open list:DRM PANEL DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS On Wed, Aug 12, 2020 at 9:34 AM Sam Ravnborg <sam@ravnborg.org> wrote: > Hmm, I think I looked at leds/ when I wrote that comment about > common.yaml. > > Please consider Rob's comment in commit: 44e1655a444fe7a1bd81994d34c6bbb5245b9e60 > ("dt-bindings: backlight: Convert common backlight bindings to DT > schema") > > Rob did not see the need for a common binding - but that may change as > we add more backlight bindings. It can't hurt. The proposal is out there, there are some drivers in backlight that can readily be converted to use it if it is favored. Thanks! Linus Walleij ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2 v1] dt-bindings: backlight: Add Kinetic KTD253 bindings @ 2020-08-12 9:09 ` Linus Walleij 0 siblings, 0 replies; 16+ messages in thread From: Linus Walleij @ 2020-08-12 9:09 UTC (permalink / raw) To: Sam Ravnborg Cc: Jingoo Han, Daniel Thompson, Lee Jones, open list:DRM PANEL DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS On Wed, Aug 12, 2020 at 9:34 AM Sam Ravnborg <sam@ravnborg.org> wrote: > Hmm, I think I looked at leds/ when I wrote that comment about > common.yaml. > > Please consider Rob's comment in commit: 44e1655a444fe7a1bd81994d34c6bbb5245b9e60 > ("dt-bindings: backlight: Convert common backlight bindings to DT > schema") > > Rob did not see the need for a common binding - but that may change as > we add more backlight bindings. It can't hurt. The proposal is out there, there are some drivers in backlight that can readily be converted to use it if it is favored. Thanks! Linus Walleij _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2 v1] dt-bindings: backlight: Add Kinetic KTD253 bindings 2020-07-20 20:35 ` Linus Walleij @ 2020-07-21 11:28 ` Daniel Thompson -1 siblings, 0 replies; 16+ messages in thread From: Daniel Thompson @ 2020-07-21 11:28 UTC (permalink / raw) To: Linus Walleij; +Cc: Lee Jones, Jingoo Han, dri-devel, devicetree On Mon, Jul 20, 2020 at 10:35:05PM +0200, Linus Walleij wrote: > This adds device tree bindings for the Kinetic KTD253 > white LED backlight driver. > > Cc: devicetree@vger.kernel.org > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > .../leds/backlight/kinetic,ktd253.yaml | 48 +++++++++++++++++++ > 1 file changed, 48 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml > > diff --git a/Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml b/Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml > new file mode 100644 > index 000000000000..610bf9a0e270 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml > @@ -0,0 +1,48 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/backlight/kinetic,ktd253.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Kinetic Technologies KTD253 one-wire backlight > + > +maintainers: > + - Linus Walleij <linus.walleij@linaro.org> > + > +description: | > + The Kinetic Technologies KTD253 is a white LED backlight that is > + controlled by a single GPIO line. If you just turn on the backlight > + it goes to maximum backlight then you can set the level of backlight > + using pulses on the enable wire. > + > +properties: > + compatible: > + const: kinetic,ktd253 > + > + gpios: > + description: GPIO to use to enable/disable and dim the backlight. > + maxItems: 1 > + > + default-brightness: > + description: Default brightness level on boot. 0 is off. > + minimum: 0 > + maximum: 255 > + > + max-brightness: > + description: Maximum brightness that is allowed during runtime. > + minimum: 0 > + maximum: 255 [I ended up dropping this into this thread... but it applies to both patches] I'm a bit sceptical of having a max-brightness in the DT and a driver defined lookup table in the driver itself. That doesn't make a whole lot of sense to me since the maximum brightness here is basically relies on knowing what scale the Linux driver has opted to implement in its tables. I think there are two options here. 1. Throw away the brightness table in the driver and expose the hardware steps directly (maybe using allowing properties such as max-brightness = 24 if the top 8 values cannot be distinguished visually). 2. Implement a brightness table in the DT if there really is a need to linearize the feel of the slider. In that case max-brightness can be inferred from the maximum value in the table. Note that #2 is absolutely *not* the same as the tables in pwm_bl.c (which are used to map a very wide linear scale on the hardware into a smaller logarithmic interface for software to use). For this driver the driver's lookup table is used to present an oversized scale to software and quantizing it in the driver (using variably sized quantums) to create a hardware value. This can be useful if the hardware's perceptual response feels *really* lumpy but often results in sliders with dead zones (because they do not "snap" to the hardware intervals). Looking at the gaps in the driver I'm suspect the table is not worth the effort (the difference in the deltas is pretty modest) but I'm happy to contradicted by someone with access to the hardware! Daniel. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2 v1] dt-bindings: backlight: Add Kinetic KTD253 bindings @ 2020-07-21 11:28 ` Daniel Thompson 0 siblings, 0 replies; 16+ messages in thread From: Daniel Thompson @ 2020-07-21 11:28 UTC (permalink / raw) To: Linus Walleij; +Cc: Jingoo Han, Lee Jones, dri-devel, devicetree On Mon, Jul 20, 2020 at 10:35:05PM +0200, Linus Walleij wrote: > This adds device tree bindings for the Kinetic KTD253 > white LED backlight driver. > > Cc: devicetree@vger.kernel.org > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > .../leds/backlight/kinetic,ktd253.yaml | 48 +++++++++++++++++++ > 1 file changed, 48 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml > > diff --git a/Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml b/Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml > new file mode 100644 > index 000000000000..610bf9a0e270 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml > @@ -0,0 +1,48 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/backlight/kinetic,ktd253.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Kinetic Technologies KTD253 one-wire backlight > + > +maintainers: > + - Linus Walleij <linus.walleij@linaro.org> > + > +description: | > + The Kinetic Technologies KTD253 is a white LED backlight that is > + controlled by a single GPIO line. If you just turn on the backlight > + it goes to maximum backlight then you can set the level of backlight > + using pulses on the enable wire. > + > +properties: > + compatible: > + const: kinetic,ktd253 > + > + gpios: > + description: GPIO to use to enable/disable and dim the backlight. > + maxItems: 1 > + > + default-brightness: > + description: Default brightness level on boot. 0 is off. > + minimum: 0 > + maximum: 255 > + > + max-brightness: > + description: Maximum brightness that is allowed during runtime. > + minimum: 0 > + maximum: 255 [I ended up dropping this into this thread... but it applies to both patches] I'm a bit sceptical of having a max-brightness in the DT and a driver defined lookup table in the driver itself. That doesn't make a whole lot of sense to me since the maximum brightness here is basically relies on knowing what scale the Linux driver has opted to implement in its tables. I think there are two options here. 1. Throw away the brightness table in the driver and expose the hardware steps directly (maybe using allowing properties such as max-brightness = 24 if the top 8 values cannot be distinguished visually). 2. Implement a brightness table in the DT if there really is a need to linearize the feel of the slider. In that case max-brightness can be inferred from the maximum value in the table. Note that #2 is absolutely *not* the same as the tables in pwm_bl.c (which are used to map a very wide linear scale on the hardware into a smaller logarithmic interface for software to use). For this driver the driver's lookup table is used to present an oversized scale to software and quantizing it in the driver (using variably sized quantums) to create a hardware value. This can be useful if the hardware's perceptual response feels *really* lumpy but often results in sliders with dead zones (because they do not "snap" to the hardware intervals). Looking at the gaps in the driver I'm suspect the table is not worth the effort (the difference in the deltas is pretty modest) but I'm happy to contradicted by someone with access to the hardware! Daniel. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2 v1] dt-bindings: backlight: Add Kinetic KTD253 bindings 2020-07-21 11:28 ` Daniel Thompson @ 2020-08-12 7:30 ` Linus Walleij -1 siblings, 0 replies; 16+ messages in thread From: Linus Walleij @ 2020-08-12 7:30 UTC (permalink / raw) To: Daniel Thompson Cc: Lee Jones, Jingoo Han, open list:DRM PANEL DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS On Tue, Jul 21, 2020 at 1:28 PM Daniel Thompson <daniel.thompson@linaro.org> wrote: > I'm a bit sceptical of having a max-brightness in the DT and a driver > defined lookup table in the driver itself. That doesn't make a whole lot > of sense to me since the maximum brightness here is basically relies on > knowing what scale the Linux driver has opted to implement in its tables. That's a good point. > I think there are two options here. > > 1. Throw away the brightness table in the driver and expose the hardware > steps directly (maybe using allowing properties such as > max-brightness = 24 if the top 8 values cannot be distinguished > visually). I think I will opt for this. It makes most sense given how we use the device tree to restrict maximum brightness, and that is definitely related to the hardware max brightness. Thanks Daniel! Linus Walleij ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2 v1] dt-bindings: backlight: Add Kinetic KTD253 bindings @ 2020-08-12 7:30 ` Linus Walleij 0 siblings, 0 replies; 16+ messages in thread From: Linus Walleij @ 2020-08-12 7:30 UTC (permalink / raw) To: Daniel Thompson Cc: Jingoo Han, Lee Jones, open list:DRM PANEL DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS On Tue, Jul 21, 2020 at 1:28 PM Daniel Thompson <daniel.thompson@linaro.org> wrote: > I'm a bit sceptical of having a max-brightness in the DT and a driver > defined lookup table in the driver itself. That doesn't make a whole lot > of sense to me since the maximum brightness here is basically relies on > knowing what scale the Linux driver has opted to implement in its tables. That's a good point. > I think there are two options here. > > 1. Throw away the brightness table in the driver and expose the hardware > steps directly (maybe using allowing properties such as > max-brightness = 24 if the top 8 values cannot be distinguished > visually). I think I will opt for this. It makes most sense given how we use the device tree to restrict maximum brightness, and that is definitely related to the hardware max brightness. Thanks Daniel! Linus Walleij _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-08-12 9:10 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-20 20:35 [PATCH 1/2 v1] dt-bindings: backlight: Add Kinetic KTD253 bindings Linus Walleij 2020-07-20 20:35 ` Linus Walleij 2020-07-20 20:35 ` [PATCH 2/2 v1] backlight: Add Kinetic KTD253 backlight driver Linus Walleij 2020-07-21 8:49 ` Sam Ravnborg 2020-07-21 8:32 ` [PATCH 1/2 v1] dt-bindings: backlight: Add Kinetic KTD253 bindings Sam Ravnborg 2020-07-21 8:32 ` Sam Ravnborg 2020-08-12 6:48 ` Linus Walleij 2020-08-12 6:48 ` Linus Walleij 2020-08-12 7:34 ` Sam Ravnborg 2020-08-12 7:34 ` Sam Ravnborg 2020-08-12 9:09 ` Linus Walleij 2020-08-12 9:09 ` Linus Walleij 2020-07-21 11:28 ` Daniel Thompson 2020-07-21 11:28 ` Daniel Thompson 2020-08-12 7:30 ` Linus Walleij 2020-08-12 7:30 ` Linus Walleij
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.