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
