* Claude review: dt-bindings: leds: backlight: document the SY7758 6-channel High Efficiency LED Driver
2026-04-30 9:47 ` [PATCH v2 1/2] dt-bindings: leds: backlight: document the SY7758 6-channel High Efficiency LED Driver Neil Armstrong
@ 2026-05-05 0:44 ` Claude Code Review Bot
0 siblings, 0 replies; 8+ messages in thread
From: Claude Code Review Bot @ 2026-05-05 0:44 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Overall**: Clean binding. Already has a Reviewed-by from Krzysztof Kozlowski. Minor suggestions only.
**Good:**
- Correctly references `common.yaml#` via `allOf`, inheriting `default-brightness` and `max-brightness` properties
- Uses `unevaluatedProperties: false` properly
- Example is well-formed
**Minor note:** The binding inherits `default-brightness` and `max-brightness` from `common.yaml`, but the driver (patch 2) doesn't read these properties. This means they'll be accepted in DT but silently ignored by the driver, which could be confusing.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 8+ messages in thread
* Claude review: dt-bindings: leds: backlight: document the SY7758 6-channel High Efficiency LED Driver
2026-05-21 8:08 ` [PATCH v4 1/2] dt-bindings: leds: backlight: document the SY7758 6-channel High Efficiency LED Driver Neil Armstrong
@ 2026-05-25 10:43 ` Claude Code Review Bot
0 siblings, 0 replies; 8+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 10:43 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
This patch adds the YAML devicetree binding for the SY7758. It's straightforward and already carries a `Reviewed-by` from Krzysztof Kozlowski.
The binding correctly references `common.yaml#`, defines the required properties (`compatible`, `reg`, `vddio-supply`, `enable-gpios`), and uses `unevaluatedProperties: false`. The example is well-formed and includes the necessary `#include` for `GPIO_ACTIVE_HIGH`.
**No issues found.** This patch is clean.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v5 0/2] backlight: Add SY7758 6-channel High Efficiency LED Driver support
@ 2026-05-29 19:23 Neil Armstrong
2026-05-29 19:23 ` [PATCH v5 1/2] dt-bindings: leds: backlight: document the SY7758 6-channel High Efficiency LED Driver Neil Armstrong
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Neil Armstrong @ 2026-05-29 19:23 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev,
KancyJoe, Neil Armstrong, Krzysztof Kozlowski
Implement support for the Silergy SY7758 6-channel High Efficiency LED Driver
used for backlight brightness control in the Ayaneo Pocket S2 dual-DSI panel.
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
Changes in v5:
- Rename vddio to vdd and make it optional in bindings
- Kept the bindings review since the change is trivial
- Link to v4: https://patch.msgid.link/20260521-topic-sm8650-ayaneo-pocket-s2-sy7758-v4-0-73c732615e4a@linaro.org
Changes in v4:
- Fixed Kconfig typo
- Remove again unused macros
- Added delay.h include
- Link to v3: https://patch.msgid.link/20260519-topic-sm8650-ayaneo-pocket-s2-sy7758-v3-0-ec8194bbc885@linaro.org
Changes in v3:
- Dropped unused macros
- Added second autho entry to match header and commit message
- Move my signof at the end
- Switched to flseep()
- Link to v2: https://patch.msgid.link/20260430-topic-sm8650-ayaneo-pocket-s2-sy7758-v2-0-308140640de9@linaro.org
Changes in v2:
- Fixed bindings subject and removed "|"
- Added review tag
- Added higher delay before reading ID from HW (100us was too short)
- Removed probe defer if i2c read fails
- Link to v1: https://patch.msgid.link/20260428-topic-sm8650-ayaneo-pocket-s2-sy7758-v1-0-0caade5fdb32@linaro.org
---
KancyJoe (1):
backlight: Add SY7758 6-channel High Efficiency LED Driver support
Neil Armstrong (1):
dt-bindings: leds: backlight: document the SY7758 6-channel High Efficiency LED Driver
.../bindings/leds/backlight/silergy,sy7758.yaml | 52 +++++
drivers/video/backlight/Kconfig | 8 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/sy7758.c | 259 +++++++++++++++++++++
4 files changed, 320 insertions(+)
---
base-commit: 39704f00f747aba3144289870b5fd8ac230a9aaf
change-id: 20260428-topic-sm8650-ayaneo-pocket-s2-sy7758-3081ee7f1e25
Best regards,
--
Neil Armstrong <neil.armstrong@linaro.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v5 1/2] dt-bindings: leds: backlight: document the SY7758 6-channel High Efficiency LED Driver
2026-05-29 19:23 [PATCH v5 0/2] backlight: Add SY7758 6-channel High Efficiency LED Driver support Neil Armstrong
@ 2026-05-29 19:23 ` Neil Armstrong
2026-06-04 6:03 ` Claude review: " Claude Code Review Bot
2026-05-29 19:23 ` [PATCH v5 2/2] backlight: Add SY7758 6-channel High Efficiency LED Driver support Neil Armstrong
2026-06-04 6:03 ` Claude Code Review Bot
2 siblings, 1 reply; 8+ messages in thread
From: Neil Armstrong @ 2026-05-29 19:23 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev,
KancyJoe, Neil Armstrong, Krzysztof Kozlowski
Document the Silergy SY7758 6-channel High Efficiency LED Driver
used for backlight brightness control.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
.../bindings/leds/backlight/silergy,sy7758.yaml | 52 ++++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/Documentation/devicetree/bindings/leds/backlight/silergy,sy7758.yaml b/Documentation/devicetree/bindings/leds/backlight/silergy,sy7758.yaml
new file mode 100644
index 000000000000..64af6b34641a
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/silergy,sy7758.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/silergy,sy7758.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Silergy SY7758 6-channel High Efficiency LED Driver
+
+maintainers:
+ - Neil Armstrong <neil.armstrong@linaro.org>
+
+description:
+ Silergy SY7758 is a high efficiency 6-channels LED backlight
+ driver with I2C brightness control.
+
+allOf:
+ - $ref: common.yaml#
+
+properties:
+ compatible:
+ const: silergy,sy7758
+
+ reg:
+ maxItems: 1
+
+ vdd-supply: true
+
+ enable-gpios:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - enable-gpios
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ backlight@11 {
+ compatible = "silergy,sy7758";
+ reg = <0x11>;
+ vdd-supply = <&bl_vdd>;
+ enable-gpios = <&gpio 16 GPIO_ACTIVE_HIGH>;
+ };
+ };
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v5 2/2] backlight: Add SY7758 6-channel High Efficiency LED Driver support
2026-05-29 19:23 [PATCH v5 0/2] backlight: Add SY7758 6-channel High Efficiency LED Driver support Neil Armstrong
2026-05-29 19:23 ` [PATCH v5 1/2] dt-bindings: leds: backlight: document the SY7758 6-channel High Efficiency LED Driver Neil Armstrong
@ 2026-05-29 19:23 ` Neil Armstrong
2026-06-04 6:03 ` Claude review: " Claude Code Review Bot
2026-06-04 6:03 ` Claude Code Review Bot
2 siblings, 1 reply; 8+ messages in thread
From: Neil Armstrong @ 2026-05-29 19:23 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev,
KancyJoe, Neil Armstrong
From: KancyJoe <kancy2333@outlook.com>
Implement support for the Silergy SY7758 6-channel High Efficiency LED
Driver used for backlight brightness control in the Ayaneo Pocket S2
dual-DSI panel.
Signed-off-by: KancyJoe <kancy2333@outlook.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/video/backlight/Kconfig | 8 ++
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/sy7758.c | 259 +++++++++++++++++++++++++++++++++++++++
3 files changed, 268 insertions(+)
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index a7a3fbaf7c29..a1f70a2bae99 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -207,6 +207,14 @@ config BACKLIGHT_KTZ8866
Say Y to enable the backlight driver for the Kinetic KTZ8866
found in Xiaomi Mi Pad 5 series.
+config BACKLIGHT_SY7758
+ tristate "Backlight Driver for Silergy SY7758"
+ depends on I2C
+ select REGMAP_I2C
+ help
+ Say Y to enable the backlight driver for the Silergy SY7758
+ backlight controller found in Ayaneo Pocket S2.
+
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 794820a98ed4..39ef588b1cf2 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -56,6 +56,7 @@ obj-$(CONFIG_BACKLIGHT_PWM) += pwm_bl.o
obj-$(CONFIG_BACKLIGHT_QCOM_WLED) += qcom-wled.o
obj-$(CONFIG_BACKLIGHT_RT4831) += rt4831-backlight.o
obj-$(CONFIG_BACKLIGHT_SAHARA) += kb3886_bl.o
+obj-$(CONFIG_BACKLIGHT_SY7758) += sy7758.o
obj-$(CONFIG_BACKLIGHT_SKY81452) += sky81452-backlight.o
obj-$(CONFIG_BACKLIGHT_TPS65217) += tps65217_bl.o
obj-$(CONFIG_BACKLIGHT_WM831X) += wm831x_bl.o
diff --git a/drivers/video/backlight/sy7758.c b/drivers/video/backlight/sy7758.c
new file mode 100644
index 000000000000..786589cb8df9
--- /dev/null
+++ b/drivers/video/backlight/sy7758.c
@@ -0,0 +1,259 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Silergy SY7758 6-channel High Efficiency LED Driver
+ *
+ * Copyright (C) 2025 Kancy Joe <kancy2333@outlook.com>
+ * Copyright (C) 2026 Linaro Limited
+ * Author: Neil Armstrong <neil.armstrong@linaro.org>
+ */
+#include <linux/backlight.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/of.h>
+#include <linux/err.h>
+#include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/regmap.h>
+#include <linux/bitfield.h>
+#include <linux/gpio/consumer.h>
+#include <linux/regulator/consumer.h>
+
+#define DEFAULT_BRIGHTNESS 1024
+#define MAX_BRIGHTNESS 4080
+#define REG_MAX 0xAE
+
+/* Registers */
+#define REG_DEV_CTL 0x01
+#define REG_DEV_ID 0x03
+#define REG_BRT_12BIT_L 0x10
+#define REG_BRT_12BIT_H 0x11
+
+/* OTP memory */
+#define REG_OTP_CFG0 0xA0
+#define REG_OTP_CFG1 0xA1
+#define REG_OTP_CFG2 0xA2
+#define REG_OTP_CFG5 0xA5
+#define REG_OTP_CFG9 0xA9
+
+/* Fields */
+#define BIT_DEV_CTL_FAST BIT(7)
+#define MSK_DEV_CTL_BRT_MODE GENMASK(2, 1)
+#define BIT_DEV_CTL_BL_CTLB BIT(0)
+
+#define MSK_BRT_12BIT_L GENMASK(7, 0)
+#define MSK_BRT_12BIT_H GENMASK(3, 0)
+
+#define MSK_CFG0_CURRENT_LOW GENMASK(7, 0)
+
+#define BIT_CFG1_PDET_STDBY BIT(7)
+#define MSK_CFG1_CURRENT_MAX GENMASK(6, 4)
+#define MSK_CFG1_CURRENT_HIGH GENMASK(3, 0)
+
+#define BIT_CFG2_UVLO_EN BIT(5)
+#define BIT_CFG2_UVLO_TH BIT(4)
+#define BIT_CFG2_BL_ON BIT(3)
+#define BIT_CFG2_ISET_EN BIT(2)
+#define BIT_CFG2_BST_ESET_EN BIT(1)
+
+#define BIT_CFG5_PWM_DIRECT BIT(7)
+#define MSK_CFG5_PS_MODE GENMASK(6, 4)
+#define MSK_CFG5_PWM_FREQ GENMASK(3, 0)
+
+#define MSK_CFG9_VBST_MAX GENMASK(7, 5)
+#define BIT_CFG9_JUMP_EN BIT(4)
+#define MSK_CFG9_JUMP_TH GENMASK(3, 2)
+#define MSK_CFG9_JUMP_VOLTAGE GENMASK(1, 0)
+
+struct sy7758 {
+ struct i2c_client *client;
+ struct regmap *regmap;
+ struct gpio_desc *gpio;
+ struct backlight_device *bl;
+};
+
+static const struct regmap_config sy7758_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = REG_MAX,
+};
+
+static int sy7758_backlight_update_status(struct backlight_device *backlight_dev)
+{
+ struct sy7758 *sydev = bl_get_data(backlight_dev);
+ unsigned int brightness = backlight_get_brightness(backlight_dev);
+ int ret;
+
+ ret = regmap_write(sydev->regmap, REG_BRT_12BIT_L,
+ FIELD_PREP(MSK_BRT_12BIT_L,
+ brightness & 0xff));
+ if (ret)
+ return ret;
+
+ ret = regmap_write(sydev->regmap, REG_BRT_12BIT_H,
+ FIELD_PREP(MSK_BRT_12BIT_H,
+ (brightness >> 8) & 0xf));
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static const struct backlight_ops sy7758_backlight_ops = {
+ .options = BL_CORE_SUSPENDRESUME,
+ .update_status = sy7758_backlight_update_status,
+};
+
+static int sy7758_init(struct sy7758 *sydev)
+{
+ int ret = 0;
+
+ ret = regmap_write(sydev->regmap, REG_DEV_CTL,
+ BIT_DEV_CTL_FAST | BIT_DEV_CTL_BL_CTLB |
+ FIELD_PREP(MSK_DEV_CTL_BRT_MODE, 2));
+ if (ret)
+ return ret;
+
+ ret = regmap_write(sydev->regmap, REG_BRT_12BIT_L,
+ FIELD_PREP(MSK_BRT_12BIT_L,
+ DEFAULT_BRIGHTNESS & 0xff));
+ if (ret)
+ return ret;
+
+ ret = regmap_write(sydev->regmap, REG_BRT_12BIT_H,
+ FIELD_PREP(MSK_BRT_12BIT_H,
+ (DEFAULT_BRIGHTNESS >> 8)));
+ if (ret)
+ return ret;
+
+ ret = regmap_write(sydev->regmap, REG_OTP_CFG5,
+ FIELD_PREP(MSK_CFG5_PS_MODE, 6) |
+ FIELD_PREP(MSK_CFG5_PWM_FREQ, 4));
+ if (ret)
+ return ret;
+
+ ret = regmap_write(sydev->regmap, REG_OTP_CFG0,
+ FIELD_PREP(MSK_CFG0_CURRENT_LOW, 85));
+ if (ret)
+ return ret;
+
+ ret = regmap_write(sydev->regmap, REG_OTP_CFG1,
+ BIT_CFG1_PDET_STDBY |
+ FIELD_PREP(MSK_CFG1_CURRENT_MAX, 1) |
+ FIELD_PREP(MSK_CFG1_CURRENT_HIGH, 10));
+ if (ret)
+ return ret;
+
+ ret = regmap_write(sydev->regmap, REG_OTP_CFG9,
+ FIELD_PREP(MSK_CFG9_VBST_MAX, 4));
+ if (ret)
+ return ret;
+
+ ret = regmap_write(sydev->regmap, REG_OTP_CFG2,
+ BIT_CFG2_BL_ON | BIT_CFG2_UVLO_EN);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int sy7758_probe(struct i2c_client *client)
+{
+ struct backlight_properties props = { };
+ struct device *dev = &client->dev;
+ struct sy7758 *sydev;
+ unsigned int dev_id;
+ int ret;
+
+ sydev = devm_kzalloc(dev, sizeof(*sydev), GFP_KERNEL);
+ if (!sydev)
+ return -ENOMEM;
+
+ i2c_set_clientdata(client, sydev);
+
+ /* Initialize regmap */
+ sydev->client = client;
+ sydev->regmap = devm_regmap_init_i2c(client, &sy7758_regmap_config);
+ if (IS_ERR(sydev->regmap))
+ return dev_err_probe(dev, PTR_ERR(sydev->regmap),
+ "failed to init regmap\n");
+
+ /* Get and enable regulator */
+ ret = devm_regulator_get_enable(dev, "vdd");
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to get regulator\n");
+
+ fsleep(100);
+
+ /* Get enable GPIO and set to high */
+ sydev->gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_HIGH);
+ if (IS_ERR(sydev->gpio))
+ return dev_err_probe(dev, PTR_ERR(sydev->gpio),
+ "failed to get enable GPIO\n");
+
+ /* Let some time for HW to settle */
+ fsleep(10000);
+
+ /* try read and check device id */
+ ret = regmap_read(sydev->regmap, REG_DEV_ID, &dev_id);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "failed to read device id\n");
+ if (dev_id != 0x63) {
+ dev_err(dev, "unexpected device id: 0x%02x\n", dev_id);
+ return -ENODEV;
+ }
+
+ /* Initialize and set default brightness */
+ ret = sy7758_init(sydev);
+ if (ret)
+ return ret;
+
+ props.type = BACKLIGHT_RAW;
+ props.max_brightness = MAX_BRIGHTNESS;
+ props.brightness = DEFAULT_BRIGHTNESS;
+ props.scale = BACKLIGHT_SCALE_LINEAR;
+
+ sydev->bl = devm_backlight_device_register(dev, "sy7758-backlight",
+ dev, sydev, &sy7758_backlight_ops,
+ &props);
+ if (IS_ERR(sydev->bl))
+ return dev_err_probe(dev, PTR_ERR(sydev->bl),
+ "failed to register backlight device\n");
+
+ return backlight_update_status(sydev->bl);
+}
+
+static void sy7758_remove(struct i2c_client *client)
+{
+ struct sy7758 *sydev = i2c_get_clientdata(client);
+
+ backlight_disable(sydev->bl);
+}
+
+static const struct i2c_device_id sy7758_ids[] = {
+ { "sy7758" },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, sy7758_ids);
+
+static const struct of_device_id sy7758_match_table[] = {
+ { .compatible = "silergy,sy7758", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, sy7758_match_table);
+
+static struct i2c_driver sy7758_driver = {
+ .driver = {
+ .name = "sy7758",
+ .of_match_table = sy7758_match_table,
+ },
+ .probe = sy7758_probe,
+ .remove = sy7758_remove,
+ .id_table = sy7758_ids,
+};
+
+module_i2c_driver(sy7758_driver);
+
+MODULE_DESCRIPTION("Silergy SY7758 Backlight Driver");
+MODULE_AUTHOR("Kancy Joe <kancy2333@outlook.com>");
+MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
+MODULE_LICENSE("GPL");
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Claude review: backlight: Add SY7758 6-channel High Efficiency LED Driver support
2026-05-29 19:23 [PATCH v5 0/2] backlight: Add SY7758 6-channel High Efficiency LED Driver support Neil Armstrong
2026-05-29 19:23 ` [PATCH v5 1/2] dt-bindings: leds: backlight: document the SY7758 6-channel High Efficiency LED Driver Neil Armstrong
2026-05-29 19:23 ` [PATCH v5 2/2] backlight: Add SY7758 6-channel High Efficiency LED Driver support Neil Armstrong
@ 2026-06-04 6:03 ` Claude Code Review Bot
2 siblings, 0 replies; 8+ messages in thread
From: Claude Code Review Bot @ 2026-06-04 6:03 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: backlight: Add SY7758 6-channel High Efficiency LED Driver support
Author: Neil Armstrong <neil.armstrong@linaro.org>
Patches: 3
Reviewed: 2026-06-04T16:03:08.474195
---
This is a clean, well-structured 2-patch series adding support for the Silergy SY7758 6-channel LED backlight driver, used in the Ayaneo Pocket S2 handheld. The DT binding is minimal and follows existing patterns. The driver is compact and follows the established backlight driver idioms (ktz8866, aw99706). This is v5 with several rounds of review already incorporated.
There are a few concerns worth discussing:
1. **All hardware configuration is hardcoded** in `sy7758_init()`, making the driver specific to the Ayaneo Pocket S2 board rather than a generic SY7758 driver. A different board using this chip would need different current, PWM, and boost settings, but there's no DT property to configure them.
2. **Brightness=0 doesn't disable the LED channels** — the driver only writes zero to the brightness registers but never clears `BIT_CFG2_BL_ON`, so the chip stays powered even at minimum brightness.
3. Minor style nits (redundant masking, unused initialization).
None of these are blockers for a first version targeting a specific board, but items 1 and 2 are worth discussing.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 8+ messages in thread
* Claude review: dt-bindings: leds: backlight: document the SY7758 6-channel High Efficiency LED Driver
2026-05-29 19:23 ` [PATCH v5 1/2] dt-bindings: leds: backlight: document the SY7758 6-channel High Efficiency LED Driver Neil Armstrong
@ 2026-06-04 6:03 ` Claude Code Review Bot
0 siblings, 0 replies; 8+ messages in thread
From: Claude Code Review Bot @ 2026-06-04 6:03 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
This binding is clean and minimal. Already has a Reviewed-by from Krzysztof Kozlowski.
**Minor observation — no description for `vdd-supply`:**
The binding uses shorthand:
```yaml
vdd-supply: true
```
Other bindings in the same directory (e.g., `kinetic,ktz8866.yaml`) add a one-line description for their supply properties:
```yaml
vddpos-supply:
description: positive boost supply regulator.
```
Not required, but adding a brief description like "main power supply" would be consistent with the other bindings and helpful for board integrators.
**Otherwise**: The binding correctly references `common.yaml`, uses `unevaluatedProperties: false`, and the example is valid. No issues.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 8+ messages in thread
* Claude review: backlight: Add SY7758 6-channel High Efficiency LED Driver support
2026-05-29 19:23 ` [PATCH v5 2/2] backlight: Add SY7758 6-channel High Efficiency LED Driver support Neil Armstrong
@ 2026-06-04 6:03 ` Claude Code Review Bot
0 siblings, 0 replies; 8+ messages in thread
From: Claude Code Review Bot @ 2026-06-04 6:03 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Issue 1 (medium) — No backlight off control when brightness is 0:**
`sy7758_backlight_update_status()` only writes the brightness registers:
```c
static int sy7758_backlight_update_status(struct backlight_device *backlight_dev)
{
struct sy7758 *sydev = bl_get_data(backlight_dev);
unsigned int brightness = backlight_get_brightness(backlight_dev);
int ret;
ret = regmap_write(sydev->regmap, REG_BRT_12BIT_L,
FIELD_PREP(MSK_BRT_12BIT_L,
brightness & 0xff));
...
ret = regmap_write(sydev->regmap, REG_BRT_12BIT_H,
FIELD_PREP(MSK_BRT_12BIT_H,
(brightness >> 8) & 0xf));
...
}
```
When brightness reaches 0, the LED channels remain powered (`BIT_CFG2_BL_ON` stays set). Compare with ktz8866 which toggles `BL_EN_BIT` on brightness 0, and aw99706 which toggles `BACKLIGHT_EN_MASK`. This matters for power consumption during suspend (the backlight core sets brightness=0 on suspend due to `BL_CORE_SUSPENDRESUME`). The `update_status` callback should also clear/set `BIT_CFG2_BL_ON` based on whether brightness is zero.
Similarly, `sy7758_remove()` calls `backlight_disable()` but that only writes brightness=0 — without toggling `BIT_CFG2_BL_ON`, the chip may still be driving the LEDs at minimum current.
**Issue 2 (minor) — Hardcoded hardware configuration limits reusability:**
`sy7758_init()` writes board-specific values directly:
```c
ret = regmap_write(sydev->regmap, REG_OTP_CFG0,
FIELD_PREP(MSK_CFG0_CURRENT_LOW, 85));
...
ret = regmap_write(sydev->regmap, REG_OTP_CFG1,
BIT_CFG1_PDET_STDBY |
FIELD_PREP(MSK_CFG1_CURRENT_MAX, 1) |
FIELD_PREP(MSK_CFG1_CURRENT_HIGH, 10));
```
The LED current (85 + 10<<8 = 2645 in whatever unit), PWM frequency (mode 4), boost voltage max (mode 4), UVLO enable, etc. are all hardcoded for the Ayaneo Pocket S2. A second board using SY7758 would need to fork the init or add DT properties. This is acceptable for a v1 driver targeting a single board, but worth a comment acknowledging the limitation, or better yet, reading `default-brightness` and `max-brightness` from the DT as `common.yaml` provides them.
**Issue 3 (nit) — Redundant masking with FIELD_PREP:**
```c
ret = regmap_write(sydev->regmap, REG_BRT_12BIT_L,
FIELD_PREP(MSK_BRT_12BIT_L,
brightness & 0xff));
```
`MSK_BRT_12BIT_L` is `GENMASK(7, 0)` = 0xFF, starting at bit 0. `FIELD_PREP` already masks the value to fit the field, so the explicit `& 0xff` is redundant. Same for `(brightness >> 8) & 0xf` with `MSK_BRT_12BIT_H` = `GENMASK(3, 0)`. This appears in both `update_status` and `sy7758_init`. Not a bug, just noise.
**Issue 4 (nit) — Unnecessary initialization of `ret`:**
```c
static int sy7758_init(struct sy7758 *sydev)
{
int ret = 0;
ret = regmap_write(sydev->regmap, REG_DEV_CTL, ...);
```
`ret` is initialized to 0 then immediately overwritten. Should be just `int ret;`.
**Issue 5 (nit) — Last return in `sy7758_init` could be simplified:**
```c
ret = regmap_write(sydev->regmap, REG_OTP_CFG2,
BIT_CFG2_BL_ON | BIT_CFG2_UVLO_EN);
if (ret)
return ret;
return 0;
```
This could be simply:
```c
return regmap_write(sydev->regmap, REG_OTP_CFG2,
BIT_CFG2_BL_ON | BIT_CFG2_UVLO_EN);
```
**Positive notes:**
- Good use of `devm_regulator_get_enable()` and `devm_gpiod_get()` for resource management.
- Proper `dev_err_probe()` usage throughout.
- Device ID check is solid with a clear error message.
- The `fsleep(10000)` (10ms) after asserting the enable GPIO before attempting I2C communication is reasonable and was already iterated upon in earlier versions.
- Kconfig correctly uses `select REGMAP_I2C` and `depends on I2C`.
- Makefile entry maintains alphabetical order.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-06-04 6:03 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-29 19:23 [PATCH v5 0/2] backlight: Add SY7758 6-channel High Efficiency LED Driver support Neil Armstrong
2026-05-29 19:23 ` [PATCH v5 1/2] dt-bindings: leds: backlight: document the SY7758 6-channel High Efficiency LED Driver Neil Armstrong
2026-06-04 6:03 ` Claude review: " Claude Code Review Bot
2026-05-29 19:23 ` [PATCH v5 2/2] backlight: Add SY7758 6-channel High Efficiency LED Driver support Neil Armstrong
2026-06-04 6:03 ` Claude review: " Claude Code Review Bot
2026-06-04 6:03 ` Claude Code Review Bot
-- strict thread matches above, loose matches on Subject: below --
2026-05-21 8:07 [PATCH v4 0/2] " Neil Armstrong
2026-05-21 8:08 ` [PATCH v4 1/2] dt-bindings: leds: backlight: document the SY7758 6-channel High Efficiency LED Driver Neil Armstrong
2026-05-25 10:43 ` Claude review: " Claude Code Review Bot
2026-04-30 9:47 [PATCH v2 0/2] backlight: Add SY7758 6-channel High Efficiency LED Driver support Neil Armstrong
2026-04-30 9:47 ` [PATCH v2 1/2] dt-bindings: leds: backlight: document the SY7758 6-channel High Efficiency LED Driver Neil Armstrong
2026-05-05 0:44 ` Claude review: " Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox