| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250212-reprint-underfed-fd723ebf3df3@spud>
Date: Wed, 12 Feb 2025 19:49:21 +0000
From: Conor Dooley <conor@...nel.org>
To: Svyatoslav Ryhel <clamor95@...il.com>
Cc: Lee Jones <lee@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Jonathan Cameron <jic23@...nel.org>,
Lars-Peter Clausen <lars@...afoo.de>, Pavel Machek <pavel@....cz>,
Daniel Thompson <danielt@...nel.org>,
Jingoo Han <jingoohan1@...il.com>, Helge Deller <deller@....de>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Uwe Kleine-König <u.kleine-koenig@...libre.com>,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-iio@...r.kernel.org, linux-leds@...r.kernel.org,
dri-devel@...ts.freedesktop.org, linux-fbdev@...r.kernel.org
Subject: Re: [PATCH v1 1/2] dt-bindings: mfd: Document TI LM3533 MFD
On Wed, Feb 12, 2025 at 09:58:41AM +0200, Svyatoslav Ryhel wrote:
> Add bindings for the LM3533 - a complete power source for
> backlight, keypad, and indicator LEDs in smartphone handsets.
> The high-voltage inductive boost converter provides the
> power for two series LED strings display backlight and keypad
> functions.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@...il.com>
> ---
> .../devicetree/bindings/mfd/ti,lm3533.yaml | 221 ++++++++++++++++++
> include/dt-bindings/mfd/lm3533.h | 19 ++
> 2 files changed, 240 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/ti,lm3533.yaml
> create mode 100644 include/dt-bindings/mfd/lm3533.h
>
> diff --git a/Documentation/devicetree/bindings/mfd/ti,lm3533.yaml b/Documentation/devicetree/bindings/mfd/ti,lm3533.yaml
> new file mode 100644
> index 000000000000..d0307e5894f8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/ti,lm3533.yaml
> @@ -0,0 +1,221 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/ti,lm3533.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI LM3533 Complete Lighting Power Solution
> +
> +description: |
> + The LM3533 is a complete power source for backlight,
> + keypad, and indicator LEDs in smartphone handsets. The
> + high-voltage inductive boost converter provides the
> + power for two series LED strings display backlight and
> + keypad functions.
> + https://www.ti.com/product/LM3533
> +
> +maintainers:
> + - Johan Hovold <jhovold@...il.com>
> +
> +properties:
> + compatible:
> + const: ti,lm3533
> +
> + reg:
> + maxItems: 1
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + enable-gpios:
> + description: GPIO to use to enable/disable the backlight (HWEN pin).
> + maxItems: 1
> +
> + ti,boost-ovp:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Boost OVP select (16V, 24V, 32V, 40V)
> + enum: [ 0, 1, 2, 3 ]
> + default: 0
> +
> + ti,boost-freq:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Boost frequency select (500KHz or 1MHz)
> + enum: [ 0, 1 ]
> + default: 0
Properties like these should be in units, so some standard voltage-based
one for the boost and in hertz for the second. See property-units.yaml
in dt-schema.
> +required:
> + - compatible
> + - reg
> + - '#address-cells'
> + - '#size-cells'
> + - enable-gpios
> +
> +patternProperties:
> + "^backlight@[01]$":
> + type: object
> + description:
> + Properties for a backlight device.
> +
> + properties:
> + compatible:
> + const: lm3533-backlight
Missing vendor prefix
> +
> + reg:
> + description: |
> + The control bank that is used to program the two current sinks. The
> + LM3533 has two control banks (A and B) and are represented as 0 or 1
> + in this property. The two current sinks can be controlled
> + independently with both banks, or bank A can be configured to control
> + both sinks with the led-sources property.
> + minimum: 0
> + maximum: 1
> +
> + max-current-microamp:
> + description:
> + Maximum current in µA with a 800 µA step.
> + enum: [ 5000, 5800, 6600, 7400, 8200, 9000, 9800,
> + 10600, 11400, 12200, 13000, 13800, 14600,
> + 15400, 16200, 17000, 17800, 18600, 19400,
> + 20200, 21000, 21800, 22600, 23400, 24200,
> + 25000, 25800, 26600, 27400, 28200, 29000,
> + 29800 ]
> + default: 5000
> +
> + default-brightness:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Default brightness level on boot.
> + minimum: 0
> + maximum: 255
> + default: 255
> +
> + pwm:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Default pwm level on boot.
> + minimum: 0
> + maximum: 63
> + default: 0
Why is default-brightness /and/ a default for pwm needed? If the pwm
drives the backlight, wouldn't you only need one of these two?
If it stays, I think it needs a rename to be more clear about what it is
doing. "pwm" is very close to "pwms", the property used for phandles to
pwm providers but the use is rather different.
backlight/common.yaml has standard properties that you could be using,
so I'd expect to see a ref here, rather than redefining your own
version.
> +
> + required:
> + - compatible
> + - reg
> +
> + additionalProperties: false
> +
> + "^led@[0-3]$":
> + type: object
> + description:
> + Properties for a led device.
> +
> + properties:
> + compatible:
> + const: lm3533-leds
Ditto here re: compatible.
> +
> + reg:
> + description:
> + 4 led banks
> + minimum: 0
> + maximum: 3
> +
> + max-current-microamp:
> + description:
> + Maximum current in µA with a 800 µA step.
> + enum: [ 5000, 5800, 6600, 7400, 8200, 9000, 9800,
> + 10600, 11400, 12200, 13000, 13800, 14600,
> + 15400, 16200, 17000, 17800, 18600, 19400,
> + 20200, 21000, 21800, 22600, 23400, 24200,
> + 25000, 25800, 26600, 27400, 28200, 29000,
> + 29800 ]
> + default: 5000
> +
> + default-trigger:
> + $ref: /schemas/types.yaml#/definitions/string
> + description: |
> + This parameter, if present, is a string defining
> + the trigger assigned to the LED. Check linux,default-trigger.
> +
> + pwm:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Default pwm level on boot.
> + minimum: 0
> + maximum: 63
> + default: 0
Same applies here, leds/common.yaml has some of these already.
> +
> + required:
> + - compatible
> + - reg
> +
> + additionalProperties: false
> +
> + "^light-sensor@[0]$":
Not a pattern if it can only have 1 outcome.
> + type: object
> + description:
> + Properties for an illumination sensor.
> +
> + properties:
> + compatible:
> + const: lm3533-als
> +
> + reg:
> + const: 0
> +
> + resistor-value:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + PWM resistor value a linear step in currents
> + of 10 µA per code based upon 2V/R_ALS.
> + minimum: 1
> + maximum: 127
> + default: 1
> +
I'd expect a resistor to be measured in ohms of some sort,
hard to tell what the increments actually are here, they don't appear to
be a real unit. Are they register values?
This and all other custom properties need to have a vendor prefix on
them btw.
> + pwm-mode:
> + type: boolean
> + description: Mode in which ALS is running
Are there multiple pwm modes? Or is this trying to say that, if set, the
sensor is in pwm mode? Hard to tell from the description here, sorry.
Cheers,
Conor.
> +
> + required:
> + - compatible
> + - reg
> +
> + additionalProperties: false
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/mfd/lm3533.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + led-controller@36 {
> + compatible = "ti,lm3533";
> + reg = <0x36>;
> +
> + enable-gpios = <&gpio 110 GPIO_ACTIVE_HIGH>;
> +
> + ti,boost-ovp = <LM3533_BOOST_OVP_24V>;
> + ti,boost-freq = <LM3533_BOOST_FREQ_500KHZ>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + backlight@0 {
> + compatible = "lm3533-backlight";
> + reg = <0>;
> +
> + max-current-microamp = <23400>;
> + default-brightness = <113>;
> + pwm = <0x00>;
> + };
> + };
> + };
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists