linux-kernel - Re: [PATCH 1/9] mfd: Add driver for Photonicat power management MCU

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: <f538ee4b-82ed-4ee1-b4e8-768a4500283e@classfun.cn>
Date: Fri, 6 Sep 2024 18:40:23 +0800
From: Junhao Xie <bigfoot@...ssfun.cn>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: devicetree@...r.kernel.org, linux-hwmon@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-leds@...r.kernel.org,
 linux-pm@...r.kernel.org, linux-rtc@...r.kernel.org,
 linux-watchdog@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-rockchip@...ts.infradead.org, Jean Delvare <jdelvare@...e.com>,
 Guenter Roeck <linux@...ck-us.net>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, "Conor Dooley,"
 <conor+dt@...nel.org>, Pavel Machek <pavel@....cz>,
 Lee Jones <lee@...nel.org>, Sebastian Reichel <sre@...nel.org>,
 Alexandre Belloni <alexandre.belloni@...tlin.com>,
 Wim Van Sebroeck <wim@...ux-watchdog.org>, Heiko Stuebner <heiko@...ech.de>,
 Chukun Pan <amadeus@....edu.cn>, Junhao Xie <bigfoot@...ssfun.cn>
Subject: Re: [PATCH 1/9] mfd: Add driver for Photonicat power management MCU

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