netdev - Re: [PATCH v2 2/7] gpio: Add Nuvoton NCT6694 GPIO support

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOoeyxVzVF0Jiiv1MeY6b=2XR5HFuGx+4q8Kvw3kFrgC+_LnBw@mail.gmail.com>
Date: Fri, 22 Nov 2024 15:20:06 +0800
From: Ming Yu <a0282524688@...il.com>
To: Bartosz Golaszewski <brgl@...ev.pl>
Cc: tmyu0@...oton.com, lee@...nel.org, linus.walleij@...aro.org, 
	andi.shyti@...nel.org, mkl@...gutronix.de, mailhol.vincent@...adoo.fr, 
	andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com, 
	kuba@...nel.org, pabeni@...hat.com, wim@...ux-watchdog.org, 
	linux@...ck-us.net, jdelvare@...e.com, alexandre.belloni@...tlin.com, 
	linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org, 
	linux-i2c@...r.kernel.org, linux-can@...r.kernel.org, netdev@...r.kernel.org, 
	linux-watchdog@...r.kernel.org, linux-hwmon@...r.kernel.org, 
	linux-rtc@...r.kernel.org
Subject: Re: [PATCH v2 2/7] gpio: Add Nuvoton NCT6694 GPIO support

Dear Bartosz,

Thank you for your comments,

Bartosz Golaszewski <brgl@...ev.pl> 於 2024年11月21日 週四 下午4:28寫道：
>
> > +struct nct6694_gpio_data {
> > +       struct nct6694 *nct6694;
> > +       struct gpio_chip gpio;
> > +       struct mutex lock;
> > +       /* Protect irq operation */
> > +       struct mutex irq_lock;
> > +
> > +       unsigned char xmit_buf;
> > +       unsigned char irq_trig_falling;
> > +       unsigned char irq_trig_rising;
> > +
> > +       /* Current gpio group */
> > +       unsigned char group;
> > +
> > +       /* GPIO line names */
> > +       char **names;
>
> You only use this in probe() and after assigning it to gc->names, you
> never reference it again. You don't need this field here, it can be a
> local variable in probe().
>

Understood. I will modify it in the next patch.

> > +};
...
> > +
> > +       data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> > +       if (!data)
> > +               return -ENOMEM;
> > +
> > +       data->names = devm_kzalloc(dev, sizeof(char *) * NCT6694_NR_GPIO,
>
> devm_kcalloc()?
>

Okay, fix it in v3.

> > +                                  GFP_KERNEL);
...
> > +       mutex_init(&data->irq_lock);
> > +
> > +       platform_set_drvdata(pdev, data);
>
> There is no corresponding platform_get_drvdata() so you don't need this.
>

Okay, I'll drop it.

> > +
...
> > +module_platform_driver(nct6694_gpio_driver);
> > +
> > +MODULE_DESCRIPTION("USB-GPIO controller driver for NCT6694");
> > +MODULE_AUTHOR("Ming Yu <tmyu0@...oton.com>");
> > +MODULE_LICENSE("GPL");
>
> It's an MFD device, don't you need a MODULE_ALIAS() for this module to load?
>

I will add MODULE_ALIAS() for each child driver.


Best regards,
Ming