| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <d7f6edd4-d797-4a6f-8df5-d25bc557c9bb@kernel.org>
Date: Thu, 5 Oct 2023 22:40:41 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Duje Mihanović <duje.mihanovic@...le.hr>,
Lee Jones <lee@...nel.org>,
Daniel Thompson <daniel.thompson@...aro.org>,
Jingoo Han <jingoohan1@...il.com>, Pavel Machek <pavel@....cz>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Helge Deller <deller@....de>
Cc: Karel Balej <balejk@...fyz.cz>, dri-devel@...ts.freedesktop.org,
linux-leds@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-fbdev@...r.kernel.org,
~postmarketos/upstreaming@...ts.sr.ht
Subject: Re: [PATCH 2/2] backlight: Add Kinetic KTD2801 driver
On 05/10/2023 20:49, Duje Mihanović wrote:
> Add driver for the Kinetic KTD2801 backlight driver.
>
> Signed-off-by: Duje Mihanović <duje.mihanovic@...le.hr>
> ---
> MAINTAINERS | 6 ++
> drivers/video/backlight/Kconfig | 7 ++
> drivers/video/backlight/Makefile | 1 +
> drivers/video/backlight/ktd2801-backlight.c | 151 ++++++++++++++++++++++++++++
> 4 files changed, 165 insertions(+)
...
> +
> +#define EW_DELAY 150
> +#define EW_DET 270
> +#define LOW_BIT_HIGH 5
> +#define LOW_BIT_LOW (4 * HIGH_BIT_LOW)
> +#define HIGH_BIT_LOW 5
> +#define HIGH_BIT_HIGH (4 * HIGH_BIT_LOW)
> +#define DS 5
> +#define EOD_L 10
> +#define EOD_H 350
> +#define PWR_DOWN_DELAY 2600
> +
> +#define KTD2801_DEFAULT_BRIGHTNESS 100
> +#define KTD2801_MAX_BRIGHTNESS 255
> +
> +struct ktd2801_backlight {
> + struct device *dev;
> + struct backlight_device *bd;
> + struct gpio_desc *desc;
s/desc/enable_gpio/ or something similar. desc is totally not related.
> + bool was_on;
> +};
> +
> +static int ktd2801_update_status(struct backlight_device *bd)
> +{
> + struct ktd2801_backlight *ktd2801 = bl_get_data(bd);
> + u8 brightness = (u8) backlight_get_brightness(bd);
> +
> + if (backlight_is_blank(bd)) {
> + gpiod_set_value(ktd2801->desc, 1);
> + udelay(PWR_DOWN_DELAY);
> + ktd2801->was_on = false;
> + return 0;
> + }
> +
> + if (!ktd2801->was_on) {
> + gpiod_set_value(ktd2801->desc, 0);
> + udelay(EW_DELAY);
> + gpiod_set_value(ktd2801->desc, 1);
> + udelay(EW_DET);
> + gpiod_set_value(ktd2801->desc, 0);
> + ktd2801->was_on = true;
> + }
> +
> + gpiod_set_value(ktd2801->desc, 0);
> + udelay(DS);
> +
> + for (int i = 0; i < 8; i++) {
> + u8 next_bit = (brightness & 0x80) >> 7;
> +
> + if (!next_bit) {
> + gpiod_set_value(ktd2801->desc, 1);
> + udelay(LOW_BIT_LOW);
> + gpiod_set_value(ktd2801->desc, 0);
> + udelay(LOW_BIT_HIGH);
> + } else {
> + gpiod_set_value(ktd2801->desc, 1);
> + udelay(HIGH_BIT_LOW);
> + gpiod_set_value(ktd2801->desc, 0);
> + udelay(HIGH_BIT_HIGH);
> + }
> + brightness <<= 1;
> + }
> + gpiod_set_value(ktd2801->desc, 1);
> + udelay(EOD_L);
> + gpiod_set_value(ktd2801->desc, 0);
> + udelay(EOD_H);
Hm, why device is kept off after this? Setting 0 means enable GPIO is
logical 0.
> + return 0;
> +}
> +
> +static const struct backlight_ops ktd2801_backlight_ops = {
> + .update_status = ktd2801_update_status,
> +};
> +
> +static int ktd2801_backlight_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct backlight_device *bd;
> + struct ktd2801_backlight *ktd2801;
> + u32 brightness, max_brightness;
> + int ret;
> +
> + ktd2801 = devm_kzalloc(dev, sizeof(*ktd2801), GFP_KERNEL);
> + if (!ktd2801)
> + return -ENOMEM;
> + ktd2801->dev = dev;
> + ktd2801->was_on = true;
> +
> + ret = device_property_read_u32(dev, "max-brightness", &max_brightness);
> + if (ret)
> + max_brightness = KTD2801_MAX_BRIGHTNESS;
> + if (max_brightness > KTD2801_MAX_BRIGHTNESS) {
> + dev_err(dev, "illegal max brightness specified\n");
> + max_brightness = KTD2801_MAX_BRIGHTNESS;
> + }
> +
> + ret = device_property_read_u32(dev, "default-brightness", &brightness);
> + if (ret)
> + brightness = KTD2801_DEFAULT_BRIGHTNESS;
> + if (brightness > max_brightness) {
> + dev_err(dev, "default brightness exceeds max\n");
> + brightness = max_brightness;
> + }
> +
> + ktd2801->desc = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW);
OUT_LOW is keep it disabled, so is this intentional?
> + if (IS_ERR(ktd2801->desc))
> + return dev_err_probe(dev, PTR_ERR(ktd2801->desc),
> + "failed to get backlight GPIO");
> + gpiod_set_consumer_name(ktd2801->desc, dev_name(dev));
> +
> + bd = devm_backlight_device_register(dev, dev_name(dev), dev, ktd2801,
> + &ktd2801_backlight_ops, NULL);
> + if (IS_ERR(bd))
> + return dev_err_probe(dev, PTR_ERR(bd),
> + "failed to register backlight");
> +
> + bd->props.max_brightness = max_brightness;
> + bd->props.brightness = brightness;
> +
> + ktd2801->bd = bd;
> + platform_set_drvdata(pdev, bd);
> + backlight_update_status(bd);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ktd2801_of_match[] = {
> + { .compatible = "kinetic,ktd2801" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ktd2801_of_match);
> +
> +static struct platform_driver ktd2801_backlight_driver = {
> + .driver = {
> + .name = "ktd2801-backlight",
> + .of_match_table = ktd2801_of_match,
> + },
> + .probe = ktd2801_backlight_probe,
> +};
> +module_platform_driver(ktd2801_backlight_driver);
> +
> +MODULE_AUTHOR("Duje Mihanović <duje.mihanovic@...le.hr>");
> +MODULE_DESCRIPTION("Kinetic KTD2801 Backlight Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:ktd2801-backlight");
You should not need MODULE_ALIAS() in normal cases. If you need it,
usually it means your device ID table is wrong.
>
Best regards,
Krzysztof
Powered by blists - more mailing lists