Re: [PATCH v2 2/2] backlight: Add SY7758 6-channel High Efficiency LED Driver support
From: Neil Armstrong
Date: Tue May 19 2026 - 04:22:11 EST
Hi,
On 5/15/26 18:28, Daniel Thompson wrote:
On Thu, Apr 30, 2026 at 11:47:16AM +0200, Neil Armstrong wrote:
From: KancyJoe <kancy2333@xxxxxxxxxxx>
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@xxxxxxxxxx>
Signed-off-by: KancyJoe <kancy2333@xxxxxxxxxxx>
---
drivers/video/backlight/Kconfig | 8 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/sy7758.c | 311 +++++++++++++++++++++++++++++++++++++++
3 files changed, 320 insertions(+)
<snip>
diff --git a/drivers/video/backlight/sy7758.c b/drivers/video/backlight/sy7758.c
new file mode 100644
index 000000000000..9b2d3bbb4ded
--- /dev/null
+++ b/drivers/video/backlight/sy7758.c
@@ -0,0 +1,311 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Silergy SY7758 6-channel High Efficiency LED Driver
+ *
+ * Copyright (C) 2025 Kancy Joe <kancy2333@xxxxxxxxxxx>
+ * Copyright (C) 2026 Linaro Limited
+ * Author: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
I'm a bit confused by this comment. The git author and the
MODULE_AUTHOR() is Kancy Joe. What does this comment signify?
I'll fix this in v3
+ */
<snip>
+/* OTP memory */
+#define REG_OTP_CFG98 0x98
+#define REG_OTP_CFG9E 0x9E
+#define REG_OTP_CFG0 0xA0
+#define REG_OTP_CFG1 0xA1
+#define REG_OTP_CFG2 0xA2
+#define REG_OTP_CFG3 0xA3
+#define REG_OTP_CFG4 0xA4
+#define REG_OTP_CFG5 0xA5
+#define REG_OTP_CFG6 0xA6
+#define REG_OTP_CFG7 0xA7
+#define REG_OTP_CFG9 0xA9
+#define REG_OTP_CFGA 0xAA
+#define REG_OTP_CFGE 0xAE
There seems to be a lot of unused macros here, especially
combined with the unused bitfields that tell us how to interpret
the values.
Do we need them?
I'll drop those
<snip>
+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 regulators */
+ ret = devm_regulator_get_enable(dev, "vddio");
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to get regulator\n");
+
+ usleep_range(100, 200);
Any reason not to use fsleep() here?
Thanks, I'll switch to fsleep
+ /* 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 */
+ usleep_range(10000, 11000);
And here?
+
+ /* 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);
+}
Daniel.
Thanks,
Neil