Re: [PATCH 1/5] drm/xe/debugfs: Update xe_gt_topology_dump signature

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 30 Sep 2025, Michal Wajdeczko <michal.wajdeczko@intel.com> wrote:
> On 9/30/2025 10:45 AM, Jani Nikula wrote:
>> On Tue, 23 Sep 2025, Michal Wajdeczko <michal.wajdeczko@intel.com> wrote:
>>> +/**
>>> + * xe_gt_topology_dump() - Dump GT topology into a drm printer.
>>> + * @gt: the &xe_gt
>>> + * @p: the &drm_printer
>>> + *
>>> + * Return: always 0.
>>> + */
>>> +int xe_gt_topology_dump(struct xe_gt *gt, struct drm_printer *p)
>> 
>> What benefit do the formatted kernel-doc give us? IMO it's just
>> boilerplate with pretty much everything being obvious from the function
>> name and parameters. And the functions aren't significant enough to be
>> made part of the Sphinx build either.
>
> I'm just following the (unwritten?) rule that in Xe we should document
> all public functions, and while in some cases such kernel-doc does not
> bring anything new, also like in [1], IMO it's still better than no
> documentation at all, as sometimes, like [2], function name isn't
> telling you the whole thing

I'm not arguing against documentation. I'm arguing against excessive use
of kernel-doc formatting for functions that will never be part of the
Sphinx documentation build.

/**
 * xe_gt_topology_dump() - Dump GT topology into a drm printer.
 * @gt: the &xe_gt
 * @p: the &drm_printer
 *
 * Return: always 0.
 */

vs.

/* Dump GT topology into a drm printer. Always returns 0. */

For non-EXPORT_SYMBOL() driver internal stuff, in most cases the
parameter descriptions are self-evident, and repeating the function name
is just, well, repeating. The formatting doesn't buy us anything, it
just brings overhead and extra maintenance, because the format will be
checked.

For EXPORT_SYMBOL() and the more important functions, or generally
things you might want to include in the Sphinx build, or anything that
might benefit from the formatting for readability, sure, use
kernel-doc. Otherwise, I wouldn't bother.

Anyway, I'd solicit feedback from the xe maintainers, and go with that.


BR,
Jani.


>
> [1] https://elixir.bootlin.com/linux/v6.17-rc3/source/drivers/gpu/drm/xe/xe_bo.h#L239
> [2] https://elixir.bootlin.com/linux/v6.17-rc3/source/drivers/gpu/drm/xe/xe_gt_throttle.c#L241
>
>> 
>> BR,
>> Jani.
>> 
>> 
>

-- 
Jani Nikula, Intel