From: Kalle Valo <kvalo@kernel.org>
To: Jeff Johnson <quic_jjohnson@quicinc.com>
Cc: <ath11k@lists.infradead.org>, <linux-wireless@vger.kernel.org>,
<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] wifi: ath11k: document HAL_RX_BUF_RBM_SW4_BM
Date: Thu, 18 Jan 2024 13:08:31 +0200 [thread overview]
Message-ID: <875xzq526o.fsf@kernel.org> (raw)
In-Reply-To: <b4f29511-e001-4964-b88d-208dabf88121@quicinc.com> (Jeff Johnson's message of "Tue, 16 Jan 2024 08:58:04 -0800")
Jeff Johnson <quic_jjohnson@quicinc.com> writes:
> On 1/14/2024 7:17 AM, Kalle Valo wrote:
>> Jeff Johnson <quic_jjohnson@quicinc.com> writes:
>>
>>> Commit 7636c9a6e7d7 ("wifi: ath11k: Add multi TX ring support for WCN6750")
>>> added HAL_RX_BUF_RBM_SW4_BM to enum hal_rx_buf_return_buf_manager. However,
>>> as flagged by the kernel-doc script, the documentation was not updated:
>>>
>>> drivers/net/wireless/ath/ath11k/hal.h:689: warning: Enum value
>>> 'HAL_RX_BUF_RBM_SW4_BM' not described in enum
>>> 'hal_rx_buf_return_buf_manager'
>>>
>>> So update the documentation. No functional changes, compile tested only.
>>>
>>> Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com>
>>
>> I'm not really a fan of kernel-doc in wireless drivers, it feels more
>> unnecessary work. Should we remove the kernel-doc markings from ath11k
>> altogether?
>
> Are you not a fan of kernel-doc format specifically, or not a fan of
> documentation at all?
I'm definitely a fan of documentation where it makes sense, but I'm not
fan of kernel-doc if there are no users or readers. For example, using
kernel-doc in cfg80211 or mac80211 makes a lot of sense, and is
important, but I'm not convinced about using kernel-doc in wireless
drivers.
> I'm personally a fan of documentation since good documentation makes the
> code more maintainable. Yes, there is a cost in creating and maintaining
> the documentation, but this is hopefully offset by cost saving when new
> developers are trying to understand and modify the code.
>
> I'm also a fan of consistency. And since kernel-doc is the standard
> format defined for the kernel, it is my personal preference to use that
> format.
I understand your points and if we had plenty of free time I would be
onboard with this. To keep my mail short few quick points:
* 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).
* 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?
* kernel-doc moves field documentation in structures away from the
actual fields which I find confusing.
* The risk of having outdated kernel-doc documentation is high, it would
need active maintenance etc.
* I'm worried about creating useless documentation, like "Count number
foo" for foo_count() just because of kernel-doc.
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.
> 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.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
next prev parent reply other threads:[~2024-01-18 11:09 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-11 19:24 [PATCH] wifi: ath11k: document HAL_RX_BUF_RBM_SW4_BM Jeff Johnson
2024-01-14 15:17 ` Kalle Valo
2024-01-16 16:58 ` Jeff Johnson
2024-01-18 11:08 ` Kalle Valo [this message]
2024-01-18 15:44 ` Jeff Johnson
2024-01-26 16:58 ` Kalle Valo
2024-01-26 17:05 ` Jeff Johnson
2024-01-29 11:14 ` Kalle Valo
2024-01-16 12:22 ` Kalle Valo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=875xzq526o.fsf@kernel.org \
--to=kvalo@kernel.org \
--cc=ath11k@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=quic_jjohnson@quicinc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.