Message-ID: <87ttn0xc9y.fsf@kernel.org>
Date: Fri, 26 Jan 2024 18:58:01 +0200
From: Kalle Valo <kvalo@...nel.org>
To: Jeff Johnson <quic_jjohnson@...cinc.com>
Cc: <ath11k@...ts.infradead.org>,  <linux-wireless@...r.kernel.org>,
  <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] wifi: ath11k: document HAL_RX_BUF_RBM_SW4_BM

Jeff Johnson <quic_jjohnson@...cinc.com> writes:

> On 1/18/2024 3:08 AM, Kalle Valo wrote:
>> * To make sure there are no kernel-doc warnings we would have to add
>>   checks to ath11k-check, which would slow down it considerably and it
>>   would again slow down our workflow (I run it several times a day).
>
> I currently run the following on every upstream patch series I review:
>
> scripts/kernel-doc -Werror -none \
> 	$(find drivers/net/wireless/ath/ath1*k -name '*.[ch]')
>
> It takes a trivial amount of time.

Hehe, indeed. It takes 0.2 seconds on my workstation, that's fast enough
for me :)

Thanks for the idea, I added this to ath12-check:

https://github.com/qca/qca-swiss-army-knife/commit/ef11ea4c7a866247f23f3d0825f9b08bd29c4989

So from now on I will always run kernel-doc for ath12k.

>> * To use kernel-doc formatting alone doesn't really make sense so we
>>   would have to start creating a kernel-doc book or something. But who
>>   would read it?
>
> Due to the sparseness, nobody would read the actual rendered
> documentation; only the documentation as it exists in the code would be
> read. However note that Linux cross-reference tool also links to the
> documentation, which I often find useful when looking at core kernel
> code, and would find it useful when looking at driver code.

It's just that in my experience it's SO hard to get people reading
documentation, even something really small. I'm not really optimistic
that using kernel-doc in drivers is any different and people ignore that
as well, like any other documentation.

>> * kernel-doc moves field documentation in structures away from the
>>   actual fields which I find confusing.
>
> kernel-doc does support in-line commenting as well:
> <https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#in-line-member-documentation-comments>
>
> Although I don't see that used much.

That's good to know. But in a way I understand why that style is not
used, at least the example in the link is hard to read.

>> * The risk of having outdated kernel-doc documentation is high, it would
>>   need active maintenance etc.
>
> Agree, but that is true of any documentation.

But the existing style is to provide documentation only when it's
necessary. With kernel-doc we have to provide documentation when it's
not even needed, no? For example, if only one enum needs documentation
and all others obvious,

>>> I'm curious what others think of the ath10/11/12k level and style of
>>> documentation.
>> 
>> IIRC iwlwifi uses kernel-doc to document the firmware interface, not
>> sure how much it's used elsewhere in the driver.
>
> They have the same problem I'm trying to fix ;)
> % scripts/kernel-doc -Werror -none \
> 	$(find drivers/net/wireless/intel/iwlwifi -name '*.[ch]')
> ...
> 322 warnings as Errors
> %

Ouch. But I'm surprised why nobody has reported these before? Are the
kernel-doc warnings ignored by everyone?

> I'm not looking to change the status quo.

I have no problems changing the status quo but having extra work without
a clear benefit is my concern here.

> Let me get the last of the ath10k cleanups in place, and I'll continue
> to run kernel-doc to make sure the existing ath1*k documentation is
> kept up to date.

Thanks. If you have time, maybe you could update ath11k-check and
ath10k-check? Every developer should use those scripts so having them
notice the warnings earlier is easier for everyone.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Powered by blists - more mailing lists