Received-SPF: pass (google.com: domain of linux-wireless+bounces-2192-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <fc6ca019-83e9-4571-bfce-2b4e5c233aef@quicinc.com>
Date: Thu, 18 Jan 2024 07:44:10 -0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] wifi: ath11k: document HAL_RX_BUF_RBM_SW4_BM
Content-Language: en-US
To: Kalle Valo <kvalo@kernel.org>
CC: <ath11k@lists.infradead.org>, <linux-wireless@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
References: <20240111-document-hal_rx_buf_rbm_sw4_bm-v1-1-ad277e8ab3cc@quicinc.com>
 <874jfg7xm4.fsf@kernel.org>
 <b4f29511-e001-4964-b88d-208dabf88121@quicinc.com>
 <875xzq526o.fsf@kernel.org>
From: Jeff Johnson <quic_jjohnson@quicinc.com>
In-Reply-To: <875xzq526o.fsf@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit

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.

> * 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.

> * 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.

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

Agree, but that is true of any documentation. The nice thing about
kernel-doc is that it can actively tell you when the documentation
doesn't match the code.

> * I'm worried about creating useless documentation, like "Count number
>   foo" for foo_count() just because of kernel-doc.

Valid concern

> This is why I consider return on investment is low here :) My preference
> is to make the code understandable (good symbol names etc) and document
> the special cases, which are not obvious from the code, with a normal
> code comment.

Which I'm also fine with.

>> 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
%

I'm not looking to change the status quo. 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.

/jeff