[PATCH 1/3 v2] dt-bindings: backlight: Add some common backlight properties

[PATCH 1/3 v2] dt-bindings: backlight: Add some common backlight properties

[Linus Walleij]

Let's use a common.yaml include for the backlight like we do with
the LEDs. The LEDs are inherently incompatible so their bindings
cannot be reused for backlight.

Cc: devicetree@vger.kernel.org
Suggested-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- New patch as suggested by Sam.
---
 .../bindings/leds/backlight/common.yaml       | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/common.yaml

diff --git a/Documentation/devicetree/bindings/leds/backlight/common.yaml b/Documentation/devicetree/bindings/leds/backlight/common.yaml
new file mode 100644
index 000000000000..8ae7e3818b0d
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/common.yaml
@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/common.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Common backlight properties
+
+maintainers:
+  - Lee Jones <lee.jones@linaro.org>
+  - Daniel Thompson <daniel.thompson@linaro.org>
+  - Jingoo Han <jingoohan1@gmail.com>
+
+description: |
+  Backlight devices provide backlight for different types of graphical
+  displays. They are typically but not necessarilt implemented using a white
+  LED powered by a boost converter.
+
+properties:
+  default-on:
+    description:
+      The initial state of the backlight can be set to be on with this
+      property. This is a state applied by the operating system so that the
+      backlight is always turned on at boot.
+
+  default-brightness:
+    description:
+      The default brightness that should be applied to the LED by the operating
+      system on start-up. The brightness should not exceed the brightness the
+      LED can provide.
+    $ref: /schemas/types.yaml#definitions/uint32
+    minimum: 0
+
+  max-brightness:
+    description:
+      Normally the maximum brightness is determined by the hardware and this
+      property is not required. This property is used to put a software limit
+      on the brightness apart from what the driver says, as it could happen
+      that a LED can be made so bright that it gets damaged or causes damage
+      due to restrictions in a specific system, such as mounting conditions.
+    $ref: /schemas/types.yaml#definitions/uint32
+    minimum: 0
-- 
2.26.2

[PATCH 1/3 v2] dt-bindings: backlight: Add some common backlight properties

[Linus Walleij]

Let's use a common.yaml include for the backlight like we do with
the LEDs. The LEDs are inherently incompatible so their bindings
cannot be reused for backlight.

Cc: devicetree@vger.kernel.org
Suggested-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- New patch as suggested by Sam.
---
 .../bindings/leds/backlight/common.yaml       | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/common.yaml

diff --git a/Documentation/devicetree/bindings/leds/backlight/common.yaml b/Documentation/devicetree/bindings/leds/backlight/common.yaml
new file mode 100644
index 000000000000..8ae7e3818b0d
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/common.yaml
@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/common.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Common backlight properties
+
+maintainers:
+  - Lee Jones <lee.jones@linaro.org>
+  - Daniel Thompson <daniel.thompson@linaro.org>
+  - Jingoo Han <jingoohan1@gmail.com>
+
+description: |
+  Backlight devices provide backlight for different types of graphical
+  displays. They are typically but not necessarilt implemented using a white
+  LED powered by a boost converter.
+
+properties:
+  default-on:
+    description:
+      The initial state of the backlight can be set to be on with this
+      property. This is a state applied by the operating system so that the
+      backlight is always turned on at boot.
+
+  default-brightness:
+    description:
+      The default brightness that should be applied to the LED by the operating
+      system on start-up. The brightness should not exceed the brightness the
+      LED can provide.
+    $ref: /schemas/types.yaml#definitions/uint32
+    minimum: 0
+
+  max-brightness:
+    description:
+      Normally the maximum brightness is determined by the hardware and this
+      property is not required. This property is used to put a software limit
+      on the brightness apart from what the driver says, as it could happen
+      that a LED can be made so bright that it gets damaged or causes damage
+      due to restrictions in a specific system, such as mounting conditions.
+    $ref: /schemas/types.yaml#definitions/uint32
+    minimum: 0
-- 
2.26.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3 v2] dt-bindings: backlight: Add Kinetic KTD253 bindings

[Linus Walleij]

This adds device tree bindings for the Kinetic KTD253
white LED backlight driver.

Cc: devicetree@vger.kernel.org
Cc: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Create common.yaml for backlight as suggested by Sam and
  use that.
- Rename the GPIO line "enable-gpios"
---
 .../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..e17f45a2a6bf
--- /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. This is sometimes referred to as
+  "expresswire".
+
+allOf:
+  - $ref: common.yaml#
+
+properties:
+  compatible:
+    const: kinetic,ktd253
+
+  enable-gpios:
+    description: GPIO to use to enable/disable and dim the backlight.
+    maxItems: 1
+
+  default-on: true
+  default-brightness: true
+  max-brightness: true
+
+required:
+  - compatible
+  - enable-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    backlight {
+        compatible = "kinetic,ktd253";
+        enable-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>;
+        default-on;
+        default-brightness = <160>;
+    };
-- 
2.26.2

[PATCH 2/3 v2] dt-bindings: backlight: Add Kinetic KTD253 bindings

[Linus Walleij]

This adds device tree bindings for the Kinetic KTD253
white LED backlight driver.

Cc: devicetree@vger.kernel.org
Cc: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Create common.yaml for backlight as suggested by Sam and
  use that.
- Rename the GPIO line "enable-gpios"
---
 .../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..e17f45a2a6bf
--- /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. This is sometimes referred to as
+  "expresswire".
+
+allOf:
+  - $ref: common.yaml#
+
+properties:
+  compatible:
+    const: kinetic,ktd253
+
+  enable-gpios:
+    description: GPIO to use to enable/disable and dim the backlight.
+    maxItems: 1
+
+  default-on: true
+  default-brightness: true
+  max-brightness: true
+
+required:
+  - compatible
+  - enable-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    backlight {
+        compatible = "kinetic,ktd253";
+        enable-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

[PATCH 3/3 v2] backlight: Add Kinetic KTD253 backlight driver

[Linus Walleij]

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.

Cc: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Expose the 32 actual hardware levels of brightness directly
  instead of using an interpolated "brightness" table.
- Use the new backlight_get_brightness() helper.
- Call backlight_update_status() in probe instead of calling
  local functions to sync brightness.
- Sort includes alphabetically.
- Name the GPIO line "enable" and grab that in accordance
  with the change to the DT bindings.
---
 MAINTAINERS                                |   6 +
 drivers/video/backlight/Kconfig            |   8 +
 drivers/video/backlight/Makefile           |   1 +
 drivers/video/backlight/ktd253-backlight.c | 198 +++++++++++++++++++++
 4 files changed, 213 insertions(+)
 create mode 100644 drivers/video/backlight/ktd253-backlight.c

diff --git a/MAINTAINERS b/MAINTAINERS
index e627ed60d75a..4c93e3a7be11 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9711,6 +9711,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 87f9fc238d28..d83c87b902c1 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -182,6 +182,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 13463b99f1f9..685f3f1ca4df 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -35,6 +35,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..e3fee3f1f582
--- /dev/null
+++ b/drivers/video/backlight/ktd253-backlight.c
@@ -0,0 +1,198 @@
+// 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/delay.h>
+#include <linux/err.h>
+#include <linux/fb.h>
+#include <linux/gpio/consumer.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/limits.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/slab.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
+
+#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 backlight_device *bl;
+	struct gpio_desc *gpiod;
+	u16 ratio;
+};
+
+static int ktd253_backlight_update_status(struct backlight_device *bl)
+{
+	struct ktd253_backlight *ktd253 = bl_get_data(bl);
+	int brightness = backlight_get_brightness(bl);
+	u16 target_ratio;
+	u16 current_ratio = ktd253->ratio;
+	unsigned long flags;
+
+	dev_dbg(ktd253->dev, "new brightness/ratio: %d/32\n", brightness);
+
+	target_ratio = brightness;
+
+	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_RATIO;
+	if (max_brightness > KTD253_MAX_RATIO) {
+		/* Clamp brightness to hardware max */
+		dev_err(dev, "illegal max brightness specified\n");
+		max_brightness = KTD253_MAX_RATIO;
+	}
+
+	ret = device_property_read_u32(dev, "default-brightness", &brightness);
+	if (ret)
+		brightness = KTD253_DEFAULT_RATIO;
+	if (brightness > max_brightness) {
+		/* Clamp default brightness to max brightness */
+		dev_err(dev, "default brightness exceeds max brightness\n");
+		brightness = max_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, "enable",
+				       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;
+	} else {
+		bl->props.brightness = 0;
+		bl->props.power = FB_BLANK_POWERDOWN;
+	}
+
+	ktd253->bl = bl;
+	platform_set_drvdata(pdev, bl);
+	backlight_update_status(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

Re: [PATCH 1/3 v2] dt-bindings: backlight: Add some common backlight properties

[Rob Herring]

On Wed, Aug 12, 2020 at 2:58 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> Let's use a common.yaml include for the backlight like we do with
> the LEDs. The LEDs are inherently incompatible so their bindings
> cannot be reused for backlight.
>
> Cc: devicetree@vger.kernel.org
> Suggested-by: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - New patch as suggested by Sam.
> ---
>  .../bindings/leds/backlight/common.yaml       | 42 +++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/backlight/common.yaml

I'd expect some refactoring here with existing backlight schemas
including the ones I just added for 5.9.

Ideally, we shouldn't have any property have a type definition more
than once. (We don't have any way to detect that though it wouldn't be
hard to write a program to do so).

> diff --git a/Documentation/devicetree/bindings/leds/backlight/common.yaml b/Documentation/devicetree/bindings/leds/backlight/common.yaml
> new file mode 100644
> index 000000000000..8ae7e3818b0d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/common.yaml
> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/backlight/common.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Common backlight properties
> +
> +maintainers:
> +  - Lee Jones <lee.jones@linaro.org>
> +  - Daniel Thompson <daniel.thompson@linaro.org>
> +  - Jingoo Han <jingoohan1@gmail.com>
> +
> +description: |

You don't need '|' if there's no formatting to preserve.

> +  Backlight devices provide backlight for different types of graphical
> +  displays. They are typically but not necessarilt implemented using a white

typo

> +  LED powered by a boost converter.
> +
> +properties:
> +  default-on:
> +    description:
> +      The initial state of the backlight can be set to be on with this
> +      property. This is a state applied by the operating system so that the
> +      backlight is always turned on at boot.

Needs a type.

> +
> +  default-brightness:
> +    description:
> +      The default brightness that should be applied to the LED by the operating
> +      system on start-up. The brightness should not exceed the brightness the
> +      LED can provide.
> +    $ref: /schemas/types.yaml#definitions/uint32
> +    minimum: 0

It's an unsigned int, so the min is already 0.

> +
> +  max-brightness:
> +    description:
> +      Normally the maximum brightness is determined by the hardware and this
> +      property is not required. This property is used to put a software limit
> +      on the brightness apart from what the driver says, as it could happen
> +      that a LED can be made so bright that it gets damaged or causes damage
> +      due to restrictions in a specific system, such as mounting conditions.
> +    $ref: /schemas/types.yaml#definitions/uint32
> +    minimum: 0

Re: [PATCH 1/3 v2] dt-bindings: backlight: Add some common backlight properties

[Rob Herring]

On Wed, Aug 12, 2020 at 2:58 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> Let's use a common.yaml include for the backlight like we do with
> the LEDs. The LEDs are inherently incompatible so their bindings
> cannot be reused for backlight.
>
> Cc: devicetree@vger.kernel.org
> Suggested-by: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - New patch as suggested by Sam.
> ---
>  .../bindings/leds/backlight/common.yaml       | 42 +++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/backlight/common.yaml

I'd expect some refactoring here with existing backlight schemas
including the ones I just added for 5.9.

Ideally, we shouldn't have any property have a type definition more
than once. (We don't have any way to detect that though it wouldn't be
hard to write a program to do so).

> diff --git a/Documentation/devicetree/bindings/leds/backlight/common.yaml b/Documentation/devicetree/bindings/leds/backlight/common.yaml
> new file mode 100644
> index 000000000000..8ae7e3818b0d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/common.yaml
> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/backlight/common.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Common backlight properties
> +
> +maintainers:
> +  - Lee Jones <lee.jones@linaro.org>
> +  - Daniel Thompson <daniel.thompson@linaro.org>
> +  - Jingoo Han <jingoohan1@gmail.com>
> +
> +description: |

You don't need '|' if there's no formatting to preserve.

> +  Backlight devices provide backlight for different types of graphical
> +  displays. They are typically but not necessarilt implemented using a white

typo

> +  LED powered by a boost converter.
> +
> +properties:
> +  default-on:
> +    description:
> +      The initial state of the backlight can be set to be on with this
> +      property. This is a state applied by the operating system so that the
> +      backlight is always turned on at boot.

Needs a type.

> +
> +  default-brightness:
> +    description:
> +      The default brightness that should be applied to the LED by the operating
> +      system on start-up. The brightness should not exceed the brightness the
> +      LED can provide.
> +    $ref: /schemas/types.yaml#definitions/uint32
> +    minimum: 0

It's an unsigned int, so the min is already 0.

> +
> +  max-brightness:
> +    description:
> +      Normally the maximum brightness is determined by the hardware and this
> +      property is not required. This property is used to put a software limit
> +      on the brightness apart from what the driver says, as it could happen
> +      that a LED can be made so bright that it gets damaged or causes damage
> +      due to restrictions in a specific system, such as mounting conditions.
> +    $ref: /schemas/types.yaml#definitions/uint32
> +    minimum: 0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3 v2] backlight: Add Kinetic KTD253 backlight driver

[Daniel Thompson]

On Wed, Aug 12, 2020 at 10:58:50AM +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.
> 
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>

(although note that Sam has some patches that introduce new
APIs that this driver should, but cannot yet, use)


Daniel.

> ---
> ChangeLog v1->v2:
> - Expose the 32 actual hardware levels of brightness directly
>   instead of using an interpolated "brightness" table.
> - Use the new backlight_get_brightness() helper.
> - Call backlight_update_status() in probe instead of calling
>   local functions to sync brightness.
> - Sort includes alphabetically.
> - Name the GPIO line "enable" and grab that in accordance
>   with the change to the DT bindings.
> ---
>  MAINTAINERS                                |   6 +
>  drivers/video/backlight/Kconfig            |   8 +
>  drivers/video/backlight/Makefile           |   1 +
>  drivers/video/backlight/ktd253-backlight.c | 198 +++++++++++++++++++++
>  4 files changed, 213 insertions(+)
>  create mode 100644 drivers/video/backlight/ktd253-backlight.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e627ed60d75a..4c93e3a7be11 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9711,6 +9711,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 87f9fc238d28..d83c87b902c1 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -182,6 +182,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 13463b99f1f9..685f3f1ca4df 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -35,6 +35,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..e3fee3f1f582
> --- /dev/null
> +++ b/drivers/video/backlight/ktd253-backlight.c
> @@ -0,0 +1,198 @@
> +// 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/delay.h>
> +#include <linux/err.h>
> +#include <linux/fb.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/limits.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/slab.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
> +
> +#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 backlight_device *bl;
> +	struct gpio_desc *gpiod;
> +	u16 ratio;
> +};
> +
> +static int ktd253_backlight_update_status(struct backlight_device *bl)
> +{
> +	struct ktd253_backlight *ktd253 = bl_get_data(bl);
> +	int brightness = backlight_get_brightness(bl);
> +	u16 target_ratio;
> +	u16 current_ratio = ktd253->ratio;
> +	unsigned long flags;
> +
> +	dev_dbg(ktd253->dev, "new brightness/ratio: %d/32\n", brightness);
> +
> +	target_ratio = brightness;
> +
> +	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_RATIO;
> +	if (max_brightness > KTD253_MAX_RATIO) {
> +		/* Clamp brightness to hardware max */
> +		dev_err(dev, "illegal max brightness specified\n");
> +		max_brightness = KTD253_MAX_RATIO;
> +	}
> +
> +	ret = device_property_read_u32(dev, "default-brightness", &brightness);
> +	if (ret)
> +		brightness = KTD253_DEFAULT_RATIO;
> +	if (brightness > max_brightness) {
> +		/* Clamp default brightness to max brightness */
> +		dev_err(dev, "default brightness exceeds max brightness\n");
> +		brightness = max_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, "enable",
> +				       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;
> +	} else {
> +		bl->props.brightness = 0;
> +		bl->props.power = FB_BLANK_POWERDOWN;
> +	}
> +
> +	ktd253->bl = bl;
> +	platform_set_drvdata(pdev, bl);
> +	backlight_update_status(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

Re: [PATCH 2/3 v2] dt-bindings: backlight: Add Kinetic KTD253 bindings

[Daniel Thompson]

On Wed, Aug 12, 2020 at 10:58:49AM +0200, Linus Walleij wrote:
> This adds device tree bindings for the Kinetic KTD253
> white LED backlight driver.
> 
> Cc: devicetree@vger.kernel.org
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Create common.yaml for backlight as suggested by Sam and
>   use that.
> - Rename the GPIO line "enable-gpios"
> ---
>  .../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..e17f45a2a6bf
> --- /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. This is sometimes referred to as
> +  "expresswire".
> +
> +allOf:
> +  - $ref: common.yaml#
> +
> +properties:
> +  compatible:
> +    const: kinetic,ktd253
> +
> +  enable-gpios:
> +    description: GPIO to use to enable/disable and dim the backlight.
> +    maxItems: 1
> +
> +  default-on: true

What use is default-on here?

I'm guessing not much because there is no code in the driver to consume
it!

If there is a need to arrange a flicker-free backlight handover then
we should only use DT to handle that if the approach taken by other
drivers is not feasible (this is best demonstrated by
pwm_backlight_initial_power_state() ).

In short the approach is if not DT or no phandle link then turn the
backlight on (because noone else will) otherwise look at the current state
of the hardware and use that to inherit the power state (that is power
state, not brightness level).

To be honest flicker-free handover looks pretty hard for this driver.
It looks like we would have to flicker at some point so that we can
know exactly what brightness the hardware currently is.


Daniel.


> +  default-brightness: true
> +  max-brightness: true
> +
> +required:
> +  - compatible
> +  - enable-gpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    backlight {
> +        compatible = "kinetic,ktd253";
> +        enable-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>;
> +        default-on;
> +        default-brightness = <160>;

160 is out of range...


Daniel.

Re: [PATCH 2/3 v2] dt-bindings: backlight: Add Kinetic KTD253 bindings

[Daniel Thompson]

On Wed, Aug 12, 2020 at 10:58:49AM +0200, Linus Walleij wrote:
> This adds device tree bindings for the Kinetic KTD253
> white LED backlight driver.
> 
> Cc: devicetree@vger.kernel.org
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Create common.yaml for backlight as suggested by Sam and
>   use that.
> - Rename the GPIO line "enable-gpios"
> ---
>  .../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..e17f45a2a6bf
> --- /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. This is sometimes referred to as
> +  "expresswire".
> +
> +allOf:
> +  - $ref: common.yaml#
> +
> +properties:
> +  compatible:
> +    const: kinetic,ktd253
> +
> +  enable-gpios:
> +    description: GPIO to use to enable/disable and dim the backlight.
> +    maxItems: 1
> +
> +  default-on: true

What use is default-on here?

I'm guessing not much because there is no code in the driver to consume
it!

If there is a need to arrange a flicker-free backlight handover then
we should only use DT to handle that if the approach taken by other
drivers is not feasible (this is best demonstrated by
pwm_backlight_initial_power_state() ).

In short the approach is if not DT or no phandle link then turn the
backlight on (because noone else will) otherwise look at the current state
of the hardware and use that to inherit the power state (that is power
state, not brightness level).

To be honest flicker-free handover looks pretty hard for this driver.
It looks like we would have to flicker at some point so that we can
know exactly what brightness the hardware currently is.


Daniel.


> +  default-brightness: true
> +  max-brightness: true
> +
> +required:
> +  - compatible
> +  - enable-gpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    backlight {
> +        compatible = "kinetic,ktd253";
> +        enable-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>;
> +        default-on;
> +        default-brightness = <160>;

160 is out of range...


Daniel.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3 v2] dt-bindings: backlight: Add some common backlight properties

[Daniel Thompson]

On Wed, Aug 12, 2020 at 10:58:48AM +0200, Linus Walleij wrote:
> Let's use a common.yaml include for the backlight like we do with
> the LEDs. The LEDs are inherently incompatible so their bindings
> cannot be reused for backlight.
> 
> Cc: devicetree@vger.kernel.org
> Suggested-by: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - New patch as suggested by Sam.
> ---
>  .../bindings/leds/backlight/common.yaml       | 42 +++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/backlight/common.yaml
> 
> diff --git a/Documentation/devicetree/bindings/leds/backlight/common.yaml b/Documentation/devicetree/bindings/leds/backlight/common.yaml
> new file mode 100644
> index 000000000000..8ae7e3818b0d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/common.yaml
> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/backlight/common.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Common backlight properties
> +
> +maintainers:
> +  - Lee Jones <lee.jones@linaro.org>
> +  - Daniel Thompson <daniel.thompson@linaro.org>
> +  - Jingoo Han <jingoohan1@gmail.com>
> +
> +description: |
> +  Backlight devices provide backlight for different types of graphical
> +  displays. They are typically but not necessarilt implemented using a white
> +  LED powered by a boost converter.
> +
> +properties:
> +  default-on:
> +    description:
> +      The initial state of the backlight can be set to be on with this
> +      property. This is a state applied by the operating system so that the
> +      backlight is always turned on at boot.

Is default-on really a common property? I would describe it as legacy
that emerged when we added the gpio bindings and didn't spell
default-brightness correctly!

Currently I think this is only implemented for GPIO and it is simply
not needed for most hardware. More specifically, for hardware that is
capable of flicker-free handover (bootloader -> kernel) by examining
the hardware state at handover then we don't want a DT property. It is
duplicative and can only result in bad handovers.


Daniel.

Re: [PATCH 1/3 v2] dt-bindings: backlight: Add some common backlight properties

[Daniel Thompson]

On Wed, Aug 12, 2020 at 10:58:48AM +0200, Linus Walleij wrote:
> Let's use a common.yaml include for the backlight like we do with
> the LEDs. The LEDs are inherently incompatible so their bindings
> cannot be reused for backlight.
> 
> Cc: devicetree@vger.kernel.org
> Suggested-by: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - New patch as suggested by Sam.
> ---
>  .../bindings/leds/backlight/common.yaml       | 42 +++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/backlight/common.yaml
> 
> diff --git a/Documentation/devicetree/bindings/leds/backlight/common.yaml b/Documentation/devicetree/bindings/leds/backlight/common.yaml
> new file mode 100644
> index 000000000000..8ae7e3818b0d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/common.yaml
> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/backlight/common.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Common backlight properties
> +
> +maintainers:
> +  - Lee Jones <lee.jones@linaro.org>
> +  - Daniel Thompson <daniel.thompson@linaro.org>
> +  - Jingoo Han <jingoohan1@gmail.com>
> +
> +description: |
> +  Backlight devices provide backlight for different types of graphical
> +  displays. They are typically but not necessarilt implemented using a white
> +  LED powered by a boost converter.
> +
> +properties:
> +  default-on:
> +    description:
> +      The initial state of the backlight can be set to be on with this
> +      property. This is a state applied by the operating system so that the
> +      backlight is always turned on at boot.

Is default-on really a common property? I would describe it as legacy
that emerged when we added the gpio bindings and didn't spell
default-brightness correctly!

Currently I think this is only implemented for GPIO and it is simply
not needed for most hardware. More specifically, for hardware that is
capable of flicker-free handover (bootloader -> kernel) by examining
the hardware state at handover then we don't want a DT property. It is
duplicative and can only result in bad handovers.


Daniel.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3 v2] dt-bindings: backlight: Add some common backlight properties

[Linus Walleij]

On Wed, Aug 12, 2020 at 5:46 PM Rob Herring <robh@kernel.org> wrote:
> On Wed, Aug 12, 2020 at 2:58 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > Let's use a common.yaml include for the backlight like we do with
> > the LEDs. The LEDs are inherently incompatible so their bindings
> > cannot be reused for backlight.
> >
> > Cc: devicetree@vger.kernel.org
> > Suggested-by: Sam Ravnborg <sam@ravnborg.org>
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> > ChangeLog v1->v2:
> > - New patch as suggested by Sam.
> > ---
> >  .../bindings/leds/backlight/common.yaml       | 42 +++++++++++++++++++
> >  1 file changed, 42 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/leds/backlight/common.yaml
>
> I'd expect some refactoring here with existing backlight schemas
> including the ones I just added for 5.9.

Yeah if it takes off I can certainly make a slew of refactorings,
I would like to do that once this is applied.

> > +  LED powered by a boost converter.
> > +
> > +properties:
> > +  default-on:
> > +    description:
> > +      The initial state of the backlight can be set to be on with this
> > +      property. This is a state applied by the operating system so that the
> > +      backlight is always turned on at boot.
>
> Needs a type.

Dropping this property because the subsystem maintainer
doubts this is needed.

Fixed the rest!

Yours,
Linus Walleij

Re: [PATCH 1/3 v2] dt-bindings: backlight: Add some common backlight properties

[Linus Walleij]

On Wed, Aug 12, 2020 at 5:46 PM Rob Herring <robh@kernel.org> wrote:
> On Wed, Aug 12, 2020 at 2:58 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > Let's use a common.yaml include for the backlight like we do with
> > the LEDs. The LEDs are inherently incompatible so their bindings
> > cannot be reused for backlight.
> >
> > Cc: devicetree@vger.kernel.org
> > Suggested-by: Sam Ravnborg <sam@ravnborg.org>
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> > ChangeLog v1->v2:
> > - New patch as suggested by Sam.
> > ---
> >  .../bindings/leds/backlight/common.yaml       | 42 +++++++++++++++++++
> >  1 file changed, 42 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/leds/backlight/common.yaml
>
> I'd expect some refactoring here with existing backlight schemas
> including the ones I just added for 5.9.

Yeah if it takes off I can certainly make a slew of refactorings,
I would like to do that once this is applied.

> > +  LED powered by a boost converter.
> > +
> > +properties:
> > +  default-on:
> > +    description:
> > +      The initial state of the backlight can be set to be on with this
> > +      property. This is a state applied by the operating system so that the
> > +      backlight is always turned on at boot.
>
> Needs a type.

Dropping this property because the subsystem maintainer
doubts this is needed.

Fixed the rest!

Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel