Dear Bartosz,
Thank you for your comments,
Bartosz Golaszewski <brgl@xxxxxxxx> 於 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@xxxxxxxxxxx>");
> > +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
