Re: [PATCH v2] Legacy DVB API: completion of documentation

From: Stefan Herdler <herdler@nurfuerspam.de>
To: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>,
	linux-media@vger.kernel.org,
	Manu Abraham <abraham.manu@gmail.com>,
	Tomasz Maciej Nowak <tmn505@gmail.com>,
	Corinna Vinschen <vinschen@redhat.com>,
	Soeren Moch <smoch@web.de>
Subject: Re: [PATCH v2] Legacy DVB API: completion of documentation
Date: Mon, 3 Apr 2023 00:25:00 +0200	[thread overview]
Message-ID: <5b5a1dc0-96c4-c3c4-a24b-1917b39f8292@nurfuerspam.de> (raw)
In-Reply-To: <20230327192826.65ae299d@sal.lan>

Hi Mauro,

I just want to say thank you, for your very detailed review. It helps me a lot.
Now I know for sure which kind details have been missed.
Reading and writing such a documentation is definitely a different thing.

On 27/03/23 20:28, Mauro Carvalho Chehab wrote:
[...]
>
>>> At least everything in the header-files is in the documentation now. I hope, I have done it sufficiently.
>
> Good! It took me quite a while to read everything... At the end, as was
> a little tired, so I probably missed things.
>
> Next time, please split this on a patch series, in order to make
> easier for the poor reviewers to look into it ;-)

I have to apologies. I just realized, that it got that long,
while reading your answer.
I should have tried to scroll down to the end, before sending ;-).

[...]

>
>>> I haven't found any useful hint how to get rid of them.
>>> Should I switch to "code-block:: c" instead?
>>> But there are a lot of this warnings from other files too. It seems I'm mot the only one with this problem.
>> I switched to "code-block".
>> Now there are no warnings anymore.
>
> For things like ioctl, code-blocks are preferred. For other things,
> better to keep the warnings and not use code-block where not needed. The
> warnings are part of the Sphinx cross-reference system, which tries to
> create references to the C domain functions and data types.
>
> Unfortunately, Sphinx has a known bug that it is hit when the same symbol
> name is used with different meanings e.g., on C declarations like those:
>
> 	struct foo;
> 	enum foo;
> 	typedef foo;
> 	foo() {}
>
> Each one has a different type. So, `foo` cross-references depend on
> the context. Right now, Sphinx will just consider them to be attempts
> to re-define an existing name. There's already patches fixing it
> Sphinx upstream, but (last time I checked) not applied yet as those
> would cause regressions on intersphinx.
>
>> This functions shouldn't be referenced from outside this document anyway.
>
> No, we link the header files with the documentation, exactly to
> generate cross-references and detect gaps at the docsO.k., I thought this applies only to the inline-documentation generated from
source-code.
But it makes sense now.

Thanks to your tips I was able to find the solution for this problem in
minutes.
One of the versions I tried was already almost right. But I haven't seen it.

I already noticed, that not everything of the API is in use by the AV7110
driver, but I forgot it in the remarks.
I will provide a list next time.

I had searched for the declarations from the header-files in the projects
including them.
Almost everything was found in the AV7110 driver, only 2 or 3 items were
missing. And this rest seems to be used elsewhere.
My first target was to make the documentation consistent to the headers.
So I kept them, at least for now.

I'll mail again when I'm finished, but it may take a while.

Regards
Stefan