On 2024/9/6 17:43, Krzysztof Kozlowski wrote: > On 06/09/2024 11:36, Junhao Xie wrote: >> Add a driver for Photonicat power management MCU, which [...]>> +void *pcat_data_get_data(struct pcat_data *data) >> +{ >> + if (!data) >> + return NULL; >> + return data->data; >> +} >> +EXPORT_SYMBOL_GPL(pcat_data_get_data); > > You need kerneldoc... or just drop it. Looks a bit useless as an > export... Is it because you want to hide from your own driver pcat_data? > What for? It's your driver... Now struct pcat_data is in mfd-photonicat.c, I will move it to photonicat-pmu.h and remove pcat_data_get_data. > [...] >> +void pcat_pmu_unregister_notify(struct pcat_pmu *pmu, struct notifier_block *nb) > > You need kerneldoc. I will add missing kernel documentation for all exported functions. > [...] >> + >> +static const struct of_device_id pcat_pmu_dt_ids[] = { >> + { .compatible = "ariaboard,photonicat-pmu", }, > > Undocumented compatible. > > Remember about correct order of patches. ABI documentation is before users. > I will adjust order of patches. > [...] >> + >> +MODULE_ALIAS("platform:photonicat-pmu"); > > You should not need MODULE_ALIAS() in normal cases. If you need it, > usually it means your device ID table is wrong (e.g. misses either > entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute > for incomplete ID table. > > And it is not even correct. This is not a platform driver! > Yes, I will remove MODULE_ALIAS line > > Best regards, > Krzysztof > Thanks for your review, I will fix all problems in next version! Best regards, Junhao