* Re: [PATCH v2 2/2] backlight: Add SY7758 6-channel High Efficiency LED Driver support
[not found] ` <20260430-topic-sm8650-ayaneo-pocket-s2-sy7758-v2-2-308140640de9@linaro.org>
@ 2026-05-15 16:28 ` Daniel Thompson
0 siblings, 0 replies; 2+ messages in thread
From: Daniel Thompson @ 2026-05-15 16:28 UTC (permalink / raw)
To: Neil Armstrong
Cc: Lee Jones, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, dri-devel,
linux-leds, devicetree, linux-kernel, linux-fbdev, KancyJoe
On Thu, Apr 30, 2026 at 11:47:16AM +0200, Neil Armstrong wrote:
> 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: Neil Armstrong <neil.armstrong@linaro.org>
> Signed-off-by: KancyJoe <kancy2333@outlook.com>
> ---
> 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@outlook.com>
> + * Copyright (C) 2026 Linaro Limited
> + * Author: Neil Armstrong <neil.armstrong@linaro.org>
I'm a bit confused by this comment. The git author and the
MODULE_AUTHOR() is Kancy Joe. What does this comment signify?
> + */
> <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?
> <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?
> + /* 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.
^ permalink raw reply [flat|nested] 2+ messages in thread