| 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: <41ea5cef-e083-4a30-89a5-7aed7a6d4d6d@kernel.org>
Date: Tue, 18 Nov 2025 08:15:34 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: 429368636@...com, lee@...nel.org
Cc: pavel@...nel.org, linus.walleij@...aro.org, brgl@...ev.pl,
linux-kernel@...r.kernel.org, linux-leds@...r.kernel.org,
linux-gpio@...r.kernel.org, zhangxinyu <gavin.zhang@...ot.com>
Subject: Re: [PATCH] leds: add aw91xxx driver
On 17/11/2025 10:35, 429368636@...com wrote:
> From: zhangxinyu <gavin.zhang@...ot.com>
>
> This commit adds support for AWINIC AW91XXX 6-channel LED driver.
> The chip supports 6 PWM channels and is controlled with I2C.
>
> Signed-off-by: zhangxinyu <429368636@...com>
Mixed up SoB.
> ---
> drivers/leds/Kconfig | 11 +
...
> +
> + if (aw91xxx->matrix_key_enable) {
> + /* key init */
> + ret = aw91xxx_key_feature_init(aw91xxx);
> + if (ret) {
> + dev_err(aw91xxx->dev, "aw91xxx key feature init failed \r\n");
> + goto err_free_rst;
> + }
> + }
> +
> + if (aw91xxx->led_feature_enable) {
> + ret = aw91xxx_parse_led_cdev(aw91xxx, np);
> + if (ret < 0) {
> + dev_err(&i2c->dev, "%s error creating led class dev\n", __func__);
> + goto free_key;
> + }
> + }
> +
> + if (aw91xxx->gpio_feature_enable) {
> + /* gpio init */
> + ret = aw91xxx_gpio_feature_init(aw91xxx);
> + if (ret) {
> + dev_err(aw91xxx->dev, "aw91xxx gpio feature init failed \r\n");
> + goto free_key;
> + }
> + }
> + aw91xxx->screen_state = true;
> +
> + pr_err("%s probe completed successfully!\n", __func__);
No, drop.
> +
> + return 0;
> +
> +free_key:
> + aw91xxx_key_free_all_resource(aw91xxx);
> +err_free_rst:
> + gpio_free(aw91xxx->reset_gpio);
> +err:
> + devm_kfree(&i2c->dev, aw91xxx);
What?
> + return ret;
> +}
> +
> +static const struct of_device_id aw91xxx_dt_match[] = {
> + { .compatible = "awinic,aw91xxx_led" },
Undocumented ABI.
> + { },
> +};
> +
> +static struct i2c_driver aw91xxx_i2c_driver = {
> + .driver = {
> + .name = AW91XXX_I2C_NAME,
> + .owner = THIS_MODULE,
So you upstream us 12 year old code, with same issues, same bugs like
above and below:
> + .of_match_table = of_match_ptr(aw91xxx_dt_match),
Here with warning...
You should start by taking a recently reviewed driver as base.
> + },
> + .probe = aw91xxx_i2c_probe,
> + .remove = aw91xxx_i2c_remove,
> + .id_table = aw91xxx_i2c_id,
> +};
> +
> +static int __init aw91xxx_i2c_init(void)
> +{
> + int ret = 0;
> +
> + pr_err("aw91xxx driver version %s\n", AW91XXX_DRIVER_VERSION);
There are no versions of drivers. Don't add it.
> +
> + ret = i2c_add_driver(&aw91xxx_i2c_driver);
> + if (ret) {
> + pr_err("fail to add aw91xxx device into i2c\n");
> + return ret;
> + }
No, drop such stuff. Just module_i2c_driver.
> +
> + return 0;
> +}
> +module_init(aw91xxx_i2c_init);
> +
> +static void __exit aw91xxx_i2c_exit(void)
> +{
> + i2c_del_driver(&aw91xxx_i2c_driver);
> +}
> +module_exit(aw91xxx_i2c_exit);
> +
> +
> +MODULE_DESCRIPTION("AW91XXX LED Driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/leds/leds-aw91xxx.h b/drivers/leds/leds-aw91xxx.h
> new file mode 100644
> index 000000000000..d69c2334ffe0
> --- /dev/null
> +++ b/drivers/leds/leds-aw91xxx.h
> @@ -0,0 +1,128 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef _AW91XXX_H_
> +#define _AW91XXX_H_
> +
> +#define AWINIC_DEBUG 1
> +
> +#ifdef AWINIC_DEBUG
> +#define AW_DEBUG(fmt, args...) pr_info(fmt, ##args)
> +#else
> +#define AW_DEBUG(fmt, ...)
> +
> +#endif
> +
> +#define MAX_I2C_BUFFER_SIZE 65536
> +
> +#define AW91XXX_ID 0x23
> +#define AW91XXX_KEY_PORT_MAX (0x10) /* 16 */
> +#define AW91XXX_INT_MASK (0xFFFF)
> +
> +enum AW91XXX_FADE_TIME {
> + AW91XXX_FADE_TIME_0000MS = 0x00,
> + AW91XXX_FADE_TIME_0315MS = 0X01,
> + AW91XXX_FADE_TIME_0630MS = 0x02,
> + AW91XXX_FADE_TIME_1260MS = 0x03,
> + AW91XXX_FADE_TIME_2520MS = 0x04,
> + AW91XXX_FADE_TIME_5040MS = 0x05
> +};
> +
> +enum aw91xxx_gpio_dir {
> + AW91XXX_GPIO_INPUT = 0,
> + AW91XXX_GPIO_OUTPUT = 1,
> +};
> +
> +enum aw91xxx_gpio_val {
> + AW91XXX_GPIO_HIGH = 1,
> + AW91XXX_GPIO_LOW = 0,
> +};
> +
> +enum aw91xxx_gpio_output_mode {
> + AW91XXX_OPEN_DRAIN_OUTPUT = 0,
> + AW91XXX_PUSH_PULL_OUTPUT = 1,
> +};
> +
> +struct aw91xxx_singel_gpio {
> + unsigned int gpio_idx;
> + enum aw91xxx_gpio_dir gpio_direction;
> + enum aw91xxx_gpio_val state;
> + struct aw91xxx *priv;
> +};
> +
> +struct aw91xxx_gpio {
> + unsigned int gpio_mask;
> + unsigned int gpio_num;
> + enum aw91xxx_gpio_output_mode output_mode;
> + struct aw91xxx_singel_gpio *single_gpio_data;
> +};
> +
> +typedef struct {
> + char name[10];
> + int key_code;
> + int key_val;
> +} KEY_STATE;
> +
> +unsigned int aw91xxx_separate_key_data[AW91XXX_KEY_PORT_MAX] = {
> +/* 0 1 2 3 */
> + 1, 2, 3, 4,
> + 5, 6, 7, 8,
> + 9, 10, 11, 12,
> + 13, 14, 15, 16
> +};
> +
> +struct aw91xxx_key {
> + unsigned int key_mask;
> + unsigned int input_port_nums;
> + unsigned int output_port_nums;
> + unsigned int input_port_mask;
> + unsigned int output_port_mask;
> + unsigned int new_input_state;
> + unsigned int old_input_state;
> + unsigned int *new_output_state;
> + unsigned int *old_output_state;
> + unsigned int *def_output_state;
> + bool wake_up_enable;
> + struct input_dev *input;
> +
> + unsigned int debounce_delay;
> + struct delayed_work int_work;
> + struct hrtimer key_timer;
> + struct work_struct key_work;
> + KEY_STATE *keymap;
> + int keymap_len;
> + struct aw91xxx *priv;
> +};
> +
> +struct aw91xxx {
> + struct i2c_client *i2c;
> + struct device *dev;
> + struct led_classdev cdev;
> + struct work_struct brightness_work;
> + struct delayed_work int_work;
> +
> + int reset_gpio;
> + int irq_gpio;
> + int irq_num;
> +
> + unsigned char chipid;
> + unsigned char vendor_id;
> + unsigned char blink;
> +
> + int imax;
> + int rise_time;
> + int on_time;
> + int fall_time;
> + int off_time;
> +
> + bool led_feature_enable;
> + bool gpio_feature_enable;
> + bool matrix_key_enable;
> + bool single_key_enable;
> + bool screen_state;
> +
> + struct aw91xxx_gpio *gpio_data;
> + struct aw91xxx_key *key_data;
> +};
> +
> +
> +#endif
> +
Best regards,
Krzysztof
Powered by blists - more mailing lists