On Thu, Apr 30, 2026 at 11:47:16AM +0200, Neil Armstrong wrote:
> From: KancyJoe <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: KancyJoe <[email protected]>
> ---
> 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 <[email protected]>
> + * Copyright (C) 2026 Linaro Limited
> + * Author: Neil Armstrong <[email protected]>
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.
