diff mbox series

[1/3,v3] Documentation:leds: Add leds-st1202.rst

Message ID 20241106061812.6819-1-vicentiu.galanopulo@remote-tech.co.uk
State Superseded
Headers show
Series [1/3,v3] Documentation:leds: Add leds-st1202.rst | expand

Commit Message

Vicentiu Galanopulo Nov. 6, 2024, 6:18 a.m. UTC
Add usage for sysfs hw_pattern entry for leds-st1202

Signed-off-by: Vicentiu Galanopulo <vicentiu.galanopulo@remote-tech.co.uk>
---
 - Changes in v3: Add leds-st1202 to index.rst
 - Changes in v2: Implement review comments
---
 Documentation/leds/index.rst       |  1 +
 Documentation/leds/leds-st1202.rst | 36 ++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)
 create mode 100644 Documentation/leds/leds-st1202.rst

Comments

Krzysztof Kozlowski Nov. 6, 2024, 1:59 p.m. UTC | #1
On Wed, Nov 06, 2024 at 06:18:08AM +0000, Vicentiu Galanopulo wrote:
> The LED1202 is a 12-channel low quiescent current LED driver with:
>   * Supply range from 2.6 V to 5 V
>   * 20 mA current capability per channel
>   * 1.8 V compatible I2C control interface
>   * 8-bit analog dimming individual control
>   * 12-bit local PWM resolution
>   * 8 programmable patterns
> 
> If the led node is present in the controller then the channel is
> set to active.
> 
> v1: https://lore.kernel.org/lkml/ZnCnnQfwuRueCIQ0@admins-Air/T/
> v2: https://lore.kernel.org/all/ZniNdGgKyUMV-hjq@admins-Air/T/
> v3: https://lore.kernel.org/all/ZniNdGgKyUMV-hjq@admins-Air/T/

This goes after ---.

Please run scripts/checkpatch.pl and fix reported warnings. Then please
run 'scripts/checkpatch.pl --strict' and (probably) fix more warnings.
Some warnings can be ignored, especially from --strict run, but the code
here looks like it needs a fix. Feel free to get in touch if the warning
is not clear.

This ws correct in v3 so I do not understand what is happening with this
patch.

> 
> Signed-off-by: Vicentiu Galanopulo <vicentiu.galanopulo@remote-tech.co.uk>
> 
> ---
> Changes in v4:
>   - remove label property, use devm_led_classdev_register_ext instead

Where? This is a binding.

>   - use as base patch the v3

What does it mean? How can you no use v3, but something else?

> Changes in v3:
>   - remove active property
> Changes in v2:
>   - renamed label to remove color from it
>   - add color property for each node
>   - add function and function-enumerator property for each node
> ---

You have also whitespace errors in your patch. Again:
Please run scripts/checkpatch.pl and fix reported warnings. Then please
run 'scripts/checkpatch.pl --strict' and (probably) fix more warnings.
Some warnings can be ignored, especially from --strict run, but the code
here looks like it needs a fix. Feel free to get in touch if the warning
is not clear.

>  .../devicetree/bindings/leds/st,led1202.yaml  | 132 ++++++++++++++++++
>  1 file changed, 132 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/st,led1202.yaml

Best regards,
Krzysztof
Krzysztof Kozlowski Nov. 6, 2024, 2:01 p.m. UTC | #2
On Wed, Nov 06, 2024 at 06:18:07AM +0000, Vicentiu Galanopulo wrote:
> Add usage for sysfs hw_pattern entry for leds-st1202
> 
> Signed-off-by: Vicentiu Galanopulo <vicentiu.galanopulo@remote-tech.co.uk>
> ---
>  - Changes in v3: Add leds-st1202 to index.rst
>  - Changes in v2: Implement review comments
> ---
>  Documentation/leds/index.rst       |  1 +
>  Documentation/leds/leds-st1202.rst | 36 ++++++++++++++++++++++++++++++
>  2 files changed, 37 insertions(+)
>  create mode 100644 Documentation/leds/leds-st1202.rst
> 

I don't understand your patch threading and formatting. Why binding is
v4 but this is v3?

Please mark your patches appropriately. Use b4 or git format-patch -v4.
If you use other commands, you can screw things up, so then it is up to
you to create proper sets.

Best regards,
Krzysztof
Krzysztof Kozlowski Nov. 6, 2024, 2:02 p.m. UTC | #3
On Wed, Nov 06, 2024 at 06:18:08AM +0000, Vicentiu Galanopulo wrote:
> The LED1202 is a 12-channel low quiescent current LED driver with:
>   * Supply range from 2.6 V to 5 V
>   * 20 mA current capability per channel
>   * 1.8 V compatible I2C control interface
>   * 8-bit analog dimming individual control
>   * 12-bit local PWM resolution
>   * 8 programmable patterns
> 
> If the led node is present in the controller then the channel is
> set to active.
> 
> v1: https://lore.kernel.org/lkml/ZnCnnQfwuRueCIQ0@admins-Air/T/
> v2: https://lore.kernel.org/all/ZniNdGgKyUMV-hjq@admins-Air/T/
> v3: https://lore.kernel.org/all/ZniNdGgKyUMV-hjq@admins-Air/T/
> 
> Signed-off-by: Vicentiu Galanopulo <vicentiu.galanopulo@remote-tech.co.uk>

Really, try by yourself:

b4 diff 20241106061812.6819-2-vicentiu.galanopulo@remote-tech.co.uk
Works? No. Should work? Yes and tools make it working, don't re-invent
the process.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/leds/index.rst b/Documentation/leds/index.rst
index 3ade16c18328..0ab0a2128a11 100644
--- a/Documentation/leds/index.rst
+++ b/Documentation/leds/index.rst
@@ -28,4 +28,5 @@  LEDs
    leds-mlxcpld
    leds-mt6370-rgb
    leds-sc27xx
+   leds-st1202.rst
    leds-qcom-lpg
diff --git a/Documentation/leds/leds-st1202.rst b/Documentation/leds/leds-st1202.rst
new file mode 100644
index 000000000000..e647966e496c
--- /dev/null
+++ b/Documentation/leds/leds-st1202.rst
@@ -0,0 +1,36 @@ 
+.. SPDX-License-Identifier: GPL-2.0
+
+============================================
+Kernel driver for STMicroelectronics LED1202
+============================================
+
+/sys/class/leds/<led>/hw_pattern
+--------------------------------
+
+Specify a hardware pattern for the ST1202 LED. The LED
+controller implements 12 low-side current generators
+with independent dimming control. Internal volatile memory
+allows the user to store up to 8 different patterns.
+Each pattern is a particular output configuration in terms
+of PWM duty-cycle and duration (ms).
+
+To be compatible with the hardware pattern
+format, maximum 8 tuples of brightness (PWM) and duration must
+be written to hw_pattern.
+
+- Min pattern duration: 22 ms
+- Max pattern duration: 5660 ms
+
+The format of the hardware pattern values should be:
+"brightness duration brightness duration ..."
+
+/sys/class/leds/<led>/repeat
+----------------------------
+
+Specify a pattern repeat number, which is common for all channels.
+Default is 1; negative numbers and 0 are invalid.
+
+This file will always return the originally written repeat number.
+
+When the 255 value is written to it, all patterns will repeat
+indefinitely.