diff mbox series

[1/2] dt-bindings: backlight: Add Texas Instruments LM3509 bindings

Message ID 20240302212757.1871164-1-paroga@paroga.com
State Changes Requested
Headers show
Series [1/2] dt-bindings: backlight: Add Texas Instruments LM3509 bindings | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Patrick Gansterer March 2, 2024, 9:27 p.m. UTC
Add Device Tree bindings for Texas Instruments LM3509 - a
High Efficiency Boost for White LED's and/or OLED Displays

Signed-off-by: Patrick Gansterer <paroga@paroga.com>
---
 .../bindings/leds/backlight/ti,lm3509.yaml    | 81 +++++++++++++++++++
 1 file changed, 81 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/ti,lm3509.yaml

Comments

Krzysztof Kozlowski March 4, 2024, 8:19 a.m. UTC | #1
On 02/03/2024 22:27, Patrick Gansterer wrote:
> Add Device Tree bindings for Texas Instruments LM3509 - a
> High Efficiency Boost for White LED's and/or OLED Displays
> 
> Signed-off-by: Patrick Gansterer <paroga@paroga.com>

A nit, subject: drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

> ---
>  .../bindings/leds/backlight/ti,lm3509.yaml    | 81 +++++++++++++++++++
>  1 file changed, 81 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/backlight/ti,lm3509.yaml
> 
> diff --git a/Documentation/devicetree/bindings/leds/backlight/ti,lm3509.yaml b/Documentation/devicetree/bindings/leds/backlight/ti,lm3509.yaml
> new file mode 100644
> index 000000000000..8fbb83934e30
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/ti,lm3509.yaml
> @@ -0,0 +1,81 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/backlight/ti,lm3509.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI LM3509 High Efficiency Boost for White LED's and/or OLED Displays
> +
> +maintainers:
> +  - Patrick Gansterer <paroga@paroga.com>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  The LM3509 current mode boost converter offers two separate outputs.
> +  https://www.ti.com/product/LM3509
> +

Missing allOf with ref to common.yaml

> +properties:
> +  compatible:
> +    const: ti,lm3509
> +
> +  reg:
> +    maxItems: 1
> +
> +  reset-gpios:
> +    maxItems: 1
> +
> +  default-brightness:
> +    minimum: 0
> +    maximum: 15
> +
> +  max-brightness:
> +    minimum: 0
> +    maximum: 15
> +
> +  ti,brightness-rate-of-change-us:
> +    description: Brightness Rate of Change in microseconds.
> +    enum: [51, 13000, 26000, 52000]
> +
> +  ti,oled-mode:
> +    description: Enable OLED mode.
> +    type: boolean
> +
> +  ti,unison-mode:
> +    description: |

Do not need '|' unless you need to preserve formatting.

> +      Enable unison mode. If disabled, then it will provide two
> +      independent controllable LED currents for BMAIN and BSUB.
> +    type: boolean
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false

unevaluatedProperties instead (open existing bindings and look how they
do it).

> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        backlight@36 {
> +            compatible = "ti,lm3509";
> +            reg = <0x36>;
> +
> +            reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
> +
> +            ti,unison-mode;
> +        };
> +    };
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        backlight@36 {
> +            compatible = "ti,lm3509";
> +            reg = <0x36>;
> +
> +            ti,brightness-rate-of-change-us = <52000>;

Just combine these examples.

> +        };
> +    };

Best regards,
Krzysztof
Krzysztof Kozlowski March 4, 2024, 8:23 a.m. UTC | #2
On 02/03/2024 22:27, Patrick Gansterer wrote:
> This is a general driver for LM3509 backlight chip of TI.
> LM3509 is High Efficiency Boost for White LEDs and/or OLED Displays with
> Dual Current Sinks. This driver supports OLED/White LED select, brightness
> control and sub/main control.
> The datasheet can be found at http://www.ti.com/product/lm3509.
> 
> Signed-off-by: Patrick Gansterer <paroga@paroga.com>
> ---


...

> +
> +static int lm3509_probe(struct i2c_client *client)
> +{
> +	struct lm3509_bl *data;
> +	struct device *dev = &client->dev;
> +	int ret;
> +	bool unison_mode = false;
> +	bool oled_mode = false;
> +	unsigned int reg_gp_val = 0;
> +	u32 rate_of_change = 0;
> +	u32 brightness = LM3509_DEF_BRIGHTNESS;
> +	u32 max_brightness = LM3509_MAX_BRIGHTNESS;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> +		dev_err(dev, "fail : i2c functionality check\n");

Drop the "fail : " everywhere and instead write something useful.


> +		return -EOPNOTSUPP;
> +	}
> +
> +	data = devm_kzalloc(dev, sizeof(struct lm3509_bl), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->regmap = devm_regmap_init_i2c(client, &lm3509_regmap);
> +	if (IS_ERR(data->regmap)) {
> +		dev_err(dev, "fail : allocate register map\n");

This message can be dropped entirely.

> +		return PTR_ERR(data->regmap);
> +	}
> +	i2c_set_clientdata(client, data);
> +
> +	data->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(data->reset_gpio)) {
> +		ret = PTR_ERR(data->reset_gpio);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "fail : get reset GPIO: %d\n", ret);

No, don't upstream old vendor code directly but instead choose existing
mainline driver and customize it. The syntax is `return dev_err_probe()`.

> +		return ret;
> +	}
> +
> +	lm3509_reset(data);
> +
> +	of_property_read_u32(dev->of_node, "default-brightness", &brightness);
> +	of_property_read_u32(dev->of_node, "max-brightness", &max_brightness);
> +	unison_mode = of_property_read_bool(dev->of_node, "ti,unison-mode");
> +	oled_mode = of_property_read_bool(dev->of_node, "ti,oled-mode");
> +

Best regards,
Krzysztof
Krzysztof Kozlowski March 4, 2024, 8:30 a.m. UTC | #3
On 04/03/2024 09:23, Krzysztof Kozlowski wrote:
> 
>> +		return PTR_ERR(data->regmap);
>> +	}
>> +	i2c_set_clientdata(client, data);
>> +
>> +	data->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
>> +	if (IS_ERR(data->reset_gpio)) {
>> +		ret = PTR_ERR(data->reset_gpio);
>> +		if (ret != -EPROBE_DEFER)
>> +			dev_err(dev, "fail : get reset GPIO: %d\n", ret);
> 
> No, don't upstream old vendor code directly but instead choose existing
> mainline driver and customize it. The syntax is `return dev_err_probe()

Hm, I found such pattern in gpio backlight. I'll fix it.

Best regards,
Krzysztof
Daniel Thompson March 4, 2024, 11:29 a.m. UTC | #4
On Sat, Mar 02, 2024 at 10:27:56PM +0100, Patrick Gansterer wrote:
> Add Device Tree bindings for Texas Instruments LM3509 - a
> High Efficiency Boost for White LED's and/or OLED Displays
>
> Signed-off-by: Patrick Gansterer <paroga@paroga.com>
> ---
> <snip>
> +  ti,unison-mode:
> +    description: |
> +      Enable unison mode. If disabled, then it will provide two
> +      independent controllable LED currents for BMAIN and BSUB.
> +    type: boolean

How does not-unison mode interact with the backlight property in
panel-common.yaml ?

If this mode intended to provide two strings that can be controlled by
different panels then a phandle link will no longer be sufficient to
describe the connectivity.


Daniel.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/leds/backlight/ti,lm3509.yaml b/Documentation/devicetree/bindings/leds/backlight/ti,lm3509.yaml
new file mode 100644
index 000000000000..8fbb83934e30
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/ti,lm3509.yaml
@@ -0,0 +1,81 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/ti,lm3509.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI LM3509 High Efficiency Boost for White LED's and/or OLED Displays
+
+maintainers:
+  - Patrick Gansterer <paroga@paroga.com>
+
+description: |
+  The LM3509 current mode boost converter offers two separate outputs.
+  https://www.ti.com/product/LM3509
+
+properties:
+  compatible:
+    const: ti,lm3509
+
+  reg:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  default-brightness:
+    minimum: 0
+    maximum: 15
+
+  max-brightness:
+    minimum: 0
+    maximum: 15
+
+  ti,brightness-rate-of-change-us:
+    description: Brightness Rate of Change in microseconds.
+    enum: [51, 13000, 26000, 52000]
+
+  ti,oled-mode:
+    description: Enable OLED mode.
+    type: boolean
+
+  ti,unison-mode:
+    description: |
+      Enable unison mode. If disabled, then it will provide two
+      independent controllable LED currents for BMAIN and BSUB.
+    type: boolean
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        backlight@36 {
+            compatible = "ti,lm3509";
+            reg = <0x36>;
+
+            reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
+
+            ti,unison-mode;
+        };
+    };
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        backlight@36 {
+            compatible = "ti,lm3509";
+            reg = <0x36>;
+
+            ti,brightness-rate-of-change-us = <52000>;
+        };
+    };