Re: [PATCH 09/18] net: mac80211.h: fix a bad comment line
On Wed, 2018-05-09 at 09:04 -0300, Mauro Carvalho Chehab wrote: > Em Mon, 07 May 2018 14:38:26 +0200 > Johannes Berg escreveu: > > > On Mon, 2018-05-07 at 15:37 +0300, Kalle Valo wrote: > > > Mauro Carvalho Chehab writes: > > > > > > > Sphinx produces a lot of errors like this: > > > > ./include/net/mac80211.h:2083: warning: bad line: > > > > > > > > > Signed-off-by: Mauro Carvalho Chehab > > > > > > Randy already submitted a similar patch: > > > > > > https://patchwork.kernel.org/patch/10367275/ > > > > > > But it seems Johannes has not applied that yet. > > > > Yeah, I've been super busy preparing for the plugfest. > > > > I'll make a pass over all the patches as soon as I can, hopefully today > > or tomorrow. > > Thanks. I'll drop it from my patchset, assuming that you'll > be applying Randy's version or mine via your tree. Right, I did, just need to send a pull request. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/18] net: mac80211.h: fix a bad comment line
On Mon, 2018-05-07 at 15:37 +0300, Kalle Valo wrote: > Mauro Carvalho Chehab writes: > > > Sphinx produces a lot of errors like this: > > ./include/net/mac80211.h:2083: warning: bad line: > > > > > Signed-off-by: Mauro Carvalho Chehab > > Randy already submitted a similar patch: > > https://patchwork.kernel.org/patch/10367275/ > > But it seems Johannes has not applied that yet. Yeah, I've been super busy preparing for the plugfest. I'll make a pass over all the patches as soon as I can, hopefully today or tomorrow. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: make xmldocs failed with error after 4.17 merge period
On Fri, 2018-04-06 at 13:38 +0200, Markus Heiser wrote: > > Sorry, not yet. Johannes started a discussion about this. Its a long > time ago and I do not have any new ideas yet :/ see: > > https://www.mail-archive.com/linux-doc@vger.kernel.org/msg07035.html I still think the tools themselves don't matter all that much, as long as there aren't totally onerous requirements. I'd argue pretty much everything available on a modern distro would be OK provided we have a reasonable fallback (e.g. to ASCII as I had even written), along with some sort of documentation of dependencies, or perhaps better, some command line option to gather the info dynamically. But then again, I've already said pretty much this a year and a half ago and most people seemed more interested in some kind of purity argument than having better documentation :-) johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: nested structs parsing
Hi, > The original patchset for nested structs was supporting it only > when not inlined. This should be fixed on this patchset: > > https://lkml.org/lkml/2018/2/19/387 > > Do you have those patches on your tree? No, looks like I don't have those yet. I'll wait for those then. > With regards to duplicated warnings, that use to happen if the same header > is included several times (with is a common pratice at the net subsystem). Yeah, doesn't really matter anyway. I think I have to, in a sense, because I'm getting lots of functions separately from the headers. > Could you please merge from docs-next and see if those problems > get solved? No, that doesn't seem to address it fully: net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.packets' not described in 'sta_info' net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.bytes' not described in 'sta_info' net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.last_rate' not described in 'sta_info' net/mac80211/sta_info.h:586: warning: Function parameter or member 'msdu' not described in 'sta_info' You can reproduce this in git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master (merging with docs-next) and running make SPHINXDIRS=driver-api/80211 htmldocs johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: nested structs parsing
On Thu, 2018-03-29 at 11:46 +0200, Johannes Berg wrote:
> Hi,
>
> For a while I haven't looked at my documentation for 802.11, and now I
> noticed I'm getting warnings due to the nested parsing.
>
> However, something seems to be wrong? I have, for example, this (in
> net/mac80211/sta_info.h)
>
> struct sta_info {
> ...
> struct {
> u64 packets[IEEE80211_NUM_ACS];
> u64 bytes[IEEE80211_NUM_ACS];
> struct ieee80211_tx_rate last_rate;
> u64 msdu[IEEE80211_NUM_TIDS + 1];
> } tx_stats;
> };
>
> but I'm getting the following warnings now, with only "@tx_stats" being
> described in the documentation:
>
> net/mac80211/sta_info.h:590: warning: Function parameter or member
> 'status_stats.last_ack' not described in 'sta_info'
> net/mac80211/sta_info.h:590: warning: Function parameter or member
> 'status_stats.last_ack_signal' not described in 'sta_info'
> net/mac80211/sta_info.h:590: warning: Function parameter or member
> 'status_stats.ack_signal_filled' not described in 'sta_info'
> net/mac80211/sta_info.h:590: warning: Function parameter or member 'msdu' not
> described in 'sta_info'
>
> I can understand the first three of those, but not the last one? Why is
> the last one not qualified?
>
> If I change it to this:
>
> struct {
> u64 packets[IEEE80211_NUM_ACS];
> u64 bytes[IEEE80211_NUM_ACS];
> /**
> * @last_rate: last TX rate
> */
> struct ieee80211_tx_rate last_rate;
> /**
> * @msdu: # of MSDUs per TID
> */
> u64 msdu[IEEE80211_NUM_TIDS + 1];
> } tx_stats;
>
> I still get a warning on "tx_stats.last_rate", but not on "msdu", which
> is sort of obvious from the warning text, but also rather unexpected.
>
> Normally I'd say that the "msdu" warning is originally wrong
>
> However, I'd also argue that if I'm using inline declarations, I
> shouldn't have to write it like this:
>
> struct {
> u64 packets[IEEE80211_NUM_ACS];
> u64 bytes[IEEE80211_NUM_ACS];
> /**
> * @tx_stats.last_rate: last TX rate
> */
> struct ieee80211_tx_rate last_rate;
> ...
> } tx_stats;
Whoops, sent a fraction of a second too early - this actually causes a
warning too (no idea why four times):
net/mac80211/sta_info.h:560: warning: Incorrect use of kernel-doc format:
* @tx_stats.last_rate: last TX rate
net/mac80211/sta_info.h:560: warning: Incorrect use of kernel-doc format:
* @tx_stats.last_rate: last TX rate
net/mac80211/sta_info.h:560: warning: Incorrect use of kernel-doc format:
* @tx_stats.last_rate: last TX rate
net/mac80211/sta_info.h:560: warning: Incorrect use of kernel-doc format:
* @tx_stats.last_rate: last TX rate
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
nested structs parsing
Hi,
For a while I haven't looked at my documentation for 802.11, and now I
noticed I'm getting warnings due to the nested parsing.
However, something seems to be wrong? I have, for example, this (in
net/mac80211/sta_info.h)
struct sta_info {
...
struct {
u64 packets[IEEE80211_NUM_ACS];
u64 bytes[IEEE80211_NUM_ACS];
struct ieee80211_tx_rate last_rate;
u64 msdu[IEEE80211_NUM_TIDS + 1];
} tx_stats;
};
but I'm getting the following warnings now, with only "@tx_stats" being
described in the documentation:
net/mac80211/sta_info.h:590: warning: Function parameter or member
'status_stats.last_ack' not described in 'sta_info'
net/mac80211/sta_info.h:590: warning: Function parameter or member
'status_stats.last_ack_signal' not described in 'sta_info'
net/mac80211/sta_info.h:590: warning: Function parameter or member
'status_stats.ack_signal_filled' not described in 'sta_info'
net/mac80211/sta_info.h:590: warning: Function parameter or member 'msdu' not
described in 'sta_info'
I can understand the first three of those, but not the last one? Why is
the last one not qualified?
If I change it to this:
struct {
u64 packets[IEEE80211_NUM_ACS];
u64 bytes[IEEE80211_NUM_ACS];
/**
* @last_rate: last TX rate
*/
struct ieee80211_tx_rate last_rate;
/**
* @msdu: # of MSDUs per TID
*/
u64 msdu[IEEE80211_NUM_TIDS + 1];
} tx_stats;
I still get a warning on "tx_stats.last_rate", but not on "msdu", which
is sort of obvious from the warning text, but also rather unexpected.
Normally I'd say that the "msdu" warning is originally wrong
However, I'd also argue that if I'm using inline declarations, I
shouldn't have to write it like this:
struct {
u64 packets[IEEE80211_NUM_ACS];
u64 bytes[IEEE80211_NUM_ACS];
/**
* @tx_stats.last_rate: last TX rate
*/
struct ieee80211_tx_rate last_rate;
...
} tx_stats;
since the comment is contained in the scope of tx_stats already, but
that seems to be what I'd have to do today?
At least fixing one of these to make it consistent would be good :-)
Thanks,
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] scripts/kernel-doc: warn on excess enum value descriptions
From: Johannes Berg
The existing message
"Excess struct/union/enum/typedef member [...]"
made it sound like this would already be done, but the
code is never invoked for enums or typedefs (and really
can't be).
Add some code to the enum dumper to handle this there
instead.
While at it, also make the above message more accurate
by simply dumping the type that was passed in, and pass
the struct/union differentiation in.
Signed-off-by: Johannes Berg
---
scripts/kernel-doc | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 9d3eafea58f0..67d051edd615 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -2168,7 +2168,7 @@ sub dump_struct($$) {
my $nested;
if ($x =~ /(struct|union)\s+(\w+)\s*{(.*)}/) {
- #my $decl_type = $1;
+ my $decl_type = $1;
$declaration_name = $2;
my $members = $3;
@@ -2194,7 +2194,7 @@ sub dump_struct($$) {
$members =~ s/DECLARE_HASHTABLE\s*\(([^,)]+), ([^,)]+)\)/unsigned long
$1\[1 << (($2) - 1)\]/gos;
create_parameterlist($members, ';', $file);
- check_sections($file, $declaration_name, "struct", $sectcheck,
$struct_actual, $nested);
+ check_sections($file, $declaration_name, $decl_type, $sectcheck,
$struct_actual, $nested);
output_declaration($declaration_name,
'struct',
@@ -2226,6 +2226,8 @@ sub dump_enum($$) {
if ($x =~ /enum\s+(\w+)\s*{(.*)}/) {
$declaration_name = $1;
my $members = $2;
+ my %_members;
+
$members =~ s/\s+$//;
foreach my $arg (split ',', $members) {
@@ -2236,9 +2238,16 @@ sub dump_enum($$) {
print STDERR "${file}:$.: warning: Enum value '$arg' ".
"not described in enum '$declaration_name'\n";
}
-
+ $_members{$arg} = 1;
}
+ while (my ($k, $v) = each %parameterdescs) {
+ if (!exists($_members{$k})) {
+print STDERR "${file}:$.: warning: Excess enum value " .
+ "'$k' description in '$declaration_name'\n";
+ }
+}
+
output_declaration($declaration_name,
'enum',
{'enum' => $declaration_name,
@@ -2506,7 +2515,7 @@ sub check_sections($$) {
} else {
if ($nested !~ m/\Q$sects[$sx]\E/) {
print STDERR "${file}:$.: warning: " .
- "Excess struct/union/enum/typedef
member " .
+ "Excess $decl_type member " .
"'$sects[$sx]' " .
"description in '$decl_name'\n";
++$warnings;
--
2.14.1
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 07/26] rfkill.txt: standardize document format
On Sat, 2017-06-17 at 12:26 -0300, Mauro Carvalho Chehab wrote: > Each text file under Documentation follows a different > format. Some doesn't even have titles! > > Change its representation to follow the adopted standard, > using ReST markups for it to be parseable by Sphinx: > > - mark titles; > - comment contents index; > - mark literal blocks; > - adjust identation. > > Signed-off-by: Mauro Carvalho Chehab Fine by me, I assume somebody else is going to merge this? Acked-by: Johannes Berg -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kernel-doc mishandles declarations split into lines
On Fri, 2017-06-16 at 17:29 +0200, Markus Heiser wrote:
> > This has the same problem as all the other versions proposed here -
> > if you have
> >
> > enum foo {
> > X,
> >
> > Y,
> > };
> >
> > (note the blank line)
> >
> > you'll get
> >
> > warning: Enum value ' ' not described in enum 'foo'
>
> Aargh, sorry .. I need glasses. The LinuxDoc parser works with the
> patch, I guess the dump_enum is different in the Perl script (seems
> not stripping leading whitespaces from the concatenated string) ...
> the following works for me:
>
> modified scripts/kernel-doc
> @@ -2223,6 +2223,7 @@ sub dump_enum($$) {
> if ($x =~ /enum\s+(\w+)\s*{(.*)}/) {
> $declaration_name = $1;
> my $members = $2;
> + $members =~ s/\s+$//;
>
> foreach my $arg (split ',', $members) {
> $arg =~ s/^\s*(\w+).*/$1/;
> @@ -2763,6 +2764,9 @@ sub process_proto_type($$) {
>
> while (1) {
> if ( $x =~ /([^{};]*)([{};])(.*)/ ) {
> +if( length $prototype ) {
> +$prototype .= " "
> +}
> $prototype .= $1 . $2;
> ($2 eq '{') && $brcount++;
> ($2 eq '}') && $brcount--;
[...]
> Can you test it with some of your constructs? / Thanks!
Yep, works for me on my iwlwifi documentation where I originally found
the problem. I hope you'll submit a patch, so here's my
Tested-by: Johannes Berg
Thanks a lot!
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kernel-doc mishandles declarations split into lines
Hi Markus,
> The parser part is same state machine as the the perl one … with same
> problems ;)
:-)
> Problem here; function process_proto_type() concatenates the striped
> lines of declaration without any whitespace. A one-liner of::
>
> struct something {
> struct foo
> bar;
> };
>
> has to be::
>
> struct something {struct foo bar;};
>
> Without the patch, the result missed the space between ‚foo' and
> 'bar'::
>
> struct something {struct foobar;};
>
> Here is my fix for the Perl script:
>
> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index a26a5f2..6aa52cc 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -2763,6 +2763,9 @@ sub process_proto_type($$) {
>
> while (1) {
> if ( $x =~ /([^{};]*)([{};])(.*)/ ) {
> +if( length $prototype ) {
> +$prototype .= " "
> +}
> $prototype .= $1 . $2;
> ($2 eq '{') && $brcount++;
> ($2 eq '}') && $brcount--;
>
> Can you test it?
This has the same problem as all the other versions proposed here - if
you have
enum foo {
X,
Y,
};
(note the blank line)
you'll get
warning: Enum value ' ' not described in enum 'foo'
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kernel-doc mishandles declarations split into lines
>
> [PATCH] be sure that multiline definitions will be properly espaced
>
> When handling comments from structs with multiple lines, like:
> /**
>* struct something
>* @very_long_member_name: abcde
>*/
> struct something {
> struct
> this_is_a_very_long_struct_name_so_need_to_break_for_the
> very_long_member_name;
> };
>
> The logic adds the continuation line without a proper space, causing
> it to be misinterpreted. Be sure to add an space to replace the
> end of line.
>
> Reported-by: Johannes Berg
> Signed-off-by: Mauro Carvalho Chehab
>
> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index a26a5f2dce39..1aa44c299f78 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -2763,17 +2763,18 @@ sub process_proto_type($$) {
>
> while (1) {
> if ( $x =~ /([^{};]*)([{};])(.*)/ ) {
> - $prototype .= $1 . $2;
> + $prototype .= $1 . $2 . " ";
> ($2 eq '{') && $brcount++;
> ($2 eq '}') && $brcount--;
> if (($2 eq ';') && ($brcount == 0)) {
> + $prototype =~ s/\s+/ /g;
> dump_declaration($prototype, $file);
> reset_state();
> last;
> }
> $x = $3;
> } else {
> - $prototype .= $x;
> + $prototype .= $x . " ";
> last;
Interesting. I just ended with almost the same patch, but didn't have
the line inside the brcount if, so it didn't work. However, your
version also introduces regressions:
/**
* enum foo - foo
* @F1: f1
* @F2: f2
*/
enum foo {
F1,
F2,
};
now generates a warning:
/tmp/test.c:20: warning: Enum value ' ' not described in enum 'foo'
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kernel-doc mishandles declarations split into lines
On Tue, 2017-06-06 at 10:59 -0300, Mauro Carvalho Chehab wrote: > > A trivial "fix" would be to use just one line for the struct field :- > ) Sure, we did this, but it makes checkpatch unhappy. We have a relatively long struct name, a relatively long member name, and then it's also an array so you have another constant that needs to fit ... (I didn't puth the array part into the example, but without that it'd actually fit on 80 cols) > The logic that handle structs is at sub dump_struct() function at > kernel-doc, with is called by dump_declaration(). > > I added some debug prints at kernel-doc... > I guess the problem you're noticing is at process_proto_type(): > [...] > Basically, that while(1) loop there seems to be misinterpreting the > line with "very_long_member_name;" Oh, that's possible - I may have been looking in the wrong place. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
kernel-doc mishandles declarations split into lines
Hi,
Apologies for the long CC list, wasn't sure who really feels like they
understand this script anymore ... Markus, I think you had a rewrite of
the script in python?
If you have something like this:
/**
* struct something
* @very_long_member_name: abcde
*/
struct something {
struct this_is_a_very_long_struct_name_so_need_to_break_for_the
very_long_member_name;
};
then kernel-doc will mishandle it:
$ ./scripts/kernel-doc -rst -internal /tmp/test.c
/tmp/test.c:9: warning: No description found for parameter
'this_is_a_very_long_struct_name_so_need_to_break_for_thevery_long_member_name'
/tmp/test.c:9: warning: Excess struct/union/enum/typedef member
'very_long_member_name' description in 'something'
.. c:type:: struct something
**Definition**
::
struct something {
};
**Members**
Clearly, the code that strips trailing and leading whitespace (in
process_proto_type) is a bit *too* eager to do so, but so far I haven't
found anything that makes it work.
Anyone have any thoughts?
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Documentation/sphinx: kerneldoc: add "unused-functions"
On Tue, 2017-04-04 at 10:26 +0300, Jani Nikula wrote: > > > Interesting, TBH I never even considered this. How would I even run > > it that way? Presumably "make htmldocs" doesn't do this? > > Try 'make SPHINXOPTS=-j8 htmldocs'. Yep, makes sense. > > Sphinx documentation (http://www.sphinx-doc.org/en/stable/extdev/) says > > this: > > > > The setup() function can return a dictionary. This is treated by > > Sphinx as metadata of the extension. Metadata keys currently > > recognized are: > > [...] > > 'parallel_read_safe': a boolean that specifies if parallel reading > > of source files can be used when the extension is loaded. It > > defaults to False, i.e. you have to explicitly specify your > > extension to be parallel-read-safe after checking that it is. > > > > We do set this right now, so I guess it'd only be guaranteed to work > > right within a single rst file, and then I should perhaps consider not > > making this state global but somehow linking it to the rst file being > > processed? > > Perhaps, but does that defeat the purpose then? Yeah, it kinda does. For my original use case in cfg80211 we only have a single file, but even in mac80211 we already use more than one. Not sure what to do then - I guess we just can't do that, unless we prevent using this with parallelization, which seems awkward. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/29] rfkill.txt: standardize document format
On Fri, 2017-05-19 at 08:11 -0300, Mauro Carvalho Chehab wrote: > > Yes, it should work. Actually, you would need to use :depth: 2 to > produce this output: > > > Contents > > . rfkill - RF kill switch support > . Introduction > . Implementation details > . Kernel API > . Userspace support Sounds good to me. > I opted to keep the contents as a comment just because, in the past, > some maintainers complained about TOC removal, arguing that it makes > harder for the ones that would be reading the file in ascii. Ok. I don't really care much either way, I guess. The file is short enough that the TOC isn't all that important to start with. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/29] rfkill.txt: standardize document format
On Thu, 2017-05-18 at 22:25 -0300, Mauro Carvalho Chehab wrote: > > +.. CONTENTS > > + 1. Introduction > + 2. Implementation details > + 3. Kernel API > + 4. Userspace support Why not let this be auto-generated? .. contents:: :depth: 1 should work, no? johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Documentation/sphinx: kerneldoc: add "unused-functions"
On Fri, 2017-03-31 at 15:54 +0300, Jani Nikula wrote: > > I'm sure the parameter name could be improved to capture what you > mean better; alas I don't have a suggestion. Yes, that's a fair point - perhaps "functions-not-linked" or something like that. > > Internally this works by collecting (per-file) those functions > > (and enums, structs, doc sections...) that are explicitly used, > > and invoking the kernel-doc script with "-nofunction" later. > > A quick thought that I don't have the time to check now, but should > be checked before merging: Is the order of directive extension > execution deterministic if the Sphinx run is parallelized (sphinx- > build -j)? Is it deterministic within an rst file? Surely it's not > deterministic when called from several rst files? The latter is, > perhaps, acceptable, but the former not. Interesting, TBH I never even considered this. How would I even run it that way? Presumably "make htmldocs" doesn't do this? Sphinx documentation (http://www.sphinx-doc.org/en/stable/extdev/) says this: The setup() function can return a dictionary. This is treated by Sphinx as metadata of the extension. Metadata keys currently recognized are: [...] 'parallel_read_safe': a boolean that specifies if parallel reading of source files can be used when the extension is loaded. It defaults to False, i.e. you have to explicitly specify your extension to be parallel-read-safe after checking that it is. We do set this right now, so I guess it'd only be guaranteed to work right within a single rst file, and then I should perhaps consider not making this state global but somehow linking it to the rst file being processed? johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Documentation/sphinx: kerneldoc: add "unused-functions"
> Do we really need such generic stuff? ... IMO explicit is better than > implicit. Why not getting an error when a function, which is referred > from a reST-document disappears in the source? Those errors help > to maintain the consistency of documentation with source-code. That's a totally different problem. > I know, there are also use-cases where generic is very helpful (e.g. > create a complete API description from the header file, with just > one line in reST). And I know, that we have already generic e.g. the > "export" option of the kernel-doc directive. Exactly. But now you can either * use "export" or "internal" to get *everything* * list every single function, and get no warning when there's a function you didn't list This serves to help get a mixture of the two, to be able to group things but also document everything that got missed as a fall-back. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] cfg80211: add remaining functions/etc. to documentation
From: Johannes Berg A lot (likely most!) enums, structs and functions aren't explicitly listed in the documentation. For now, just add everything to the documentation in a new section so at least the references to those functions can be resolved. Signed-off-by: Johannes Berg --- Documentation/driver-api/80211/cfg80211.rst | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/driver-api/80211/cfg80211.rst b/Documentation/driver-api/80211/cfg80211.rst index 8ffac57e1f5b..52caec2c9ff9 100644 --- a/Documentation/driver-api/80211/cfg80211.rst +++ b/Documentation/driver-api/80211/cfg80211.rst @@ -355,3 +355,8 @@ Test mode .. kernel-doc:: include/net/cfg80211.h :functions: cfg80211_testmode_event + +Remaining functions +=== +.. kernel-doc:: include/net/cfg80211.h + :unused-functions: -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] Documentation/sphinx: kerneldoc: add "unused-functions"
From: Johannes Berg
When adding functions one by one into documentation, in order to
order/group things properly, it's easy to miss things. Allow use
of the kernel-doc directive with "unused-functions" like this
.. kernel-doc::
:unused-functions:
to output anything previously unused from that file. This allows
grouping things but still making sure that the documentation has
all the functions.
Internally this works by collecting (per-file) those functions
(and enums, structs, doc sections...) that are explicitly used,
and invoking the kernel-doc script with "-nofunction" later.
Signed-off-by: Johannes Berg
---
Documentation/sphinx/kerneldoc.py | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/Documentation/sphinx/kerneldoc.py
b/Documentation/sphinx/kerneldoc.py
index d15e07f36881..79fc1491348a 100644
--- a/Documentation/sphinx/kerneldoc.py
+++ b/Documentation/sphinx/kerneldoc.py
@@ -41,6 +41,9 @@ from sphinx.ext.autodoc import AutodocReporter
__version__ = '1.0'
+# per-file list
+_used_fns = {}
+
class KernelDocDirective(Directive):
"""Extract kernel-doc comments from the specified file"""
required_argument = 1
@@ -50,6 +53,7 @@ class KernelDocDirective(Directive):
'functions': directives.unchanged_required,
'export': directives.unchanged,
'internal': directives.unchanged,
+'unused-functions': directives.unchanged,
}
has_content = False
@@ -60,6 +64,10 @@ class KernelDocDirective(Directive):
filename = env.config.kerneldoc_srctree + '/' + self.arguments[0]
export_file_patterns = []
+if not filename in _used_fns:
+_used_fns[filename] = []
+_used_fns_this_file = _used_fns[filename]
+
# Tell sphinx of the dependency
env.note_dependency(os.path.abspath(filename))
@@ -73,10 +81,16 @@ class KernelDocDirective(Directive):
cmd += ['-internal']
export_file_patterns = str(self.options.get('internal')).split()
elif 'doc' in self.options:
-cmd += ['-function', str(self.options.get('doc'))]
+f = str(self.options.get('doc'))
+cmd += ['-function', f]
+_used_fns_this_file.append(f)
+elif 'unused-functions' in self.options:
+for f in _used_fns_this_file:
+cmd += ['-nofunction', f]
elif 'functions' in self.options:
for f in str(self.options.get('functions')).split():
cmd += ['-function', f]
+_used_fns_this_file.append(f)
for pattern in export_file_patterns:
for f in glob.glob(env.config.kerneldoc_srctree + '/' + pattern):
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
doc build requirements
Hi, I notice that now - since commit ec868e4ee2bcebb9e4c03979d90e0ac0b79fe05a Author: Mauro Carvalho Chehab Date: Wed Nov 30 08:00:17 2016 -0200 docs-rst: media: build SVG from graphviz files was merged, it is strictly necessary to have `dot' installed to build documentation - there isn't even any fallback. Now, it's not hard to install, so I have no major objection against it, but I wanted to take the opportunity to point out that this seems a bit arbitrary - last I tried adding new functionality and *didn't* even make it a build requirement (logging only a warning if not installed) I was told I can't add such a thing. Does this mean something has changed and I can try again? :) johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mac80211: fix documentation warnings
On Fri, 2017-01-13 at 13:43 +0100, Markus Heiser wrote: > does it make live easier when we use in-line member comments: > > https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#in- > line-member-documentation-comments > > and place the entire list in a literalblock? Ah yes, I forgot about that. I should convert everything to that, but that's probably better done separately. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mac80211: fix documentation warnings
From: Johannes Berg
For a few restructured text warnings in mac80211, making the
documentation warning-free (for now). Again, this required
adding trailing whitespace to keep multiple paragraphs in a
parameter description together.
Signed-off-by: Johannes Berg
---
include/net/mac80211.h | 28 ++--
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 86967b85dfd0..228c72617916 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1771,10 +1771,12 @@ struct ieee80211_sta_rates {
* @max_amsdu_len: indicates the maximal length of an A-MSDU in bytes. This
* field is always valid for packets with a VHT preamble. For packets
* with a HT preamble, additional limits apply:
- * + If the skb is transmitted as part of a BA agreement, the
- * A-MSDU maximal size is min(max_amsdu_len, 4065) bytes.
- * + If the skb is not part of a BA aggreement, the A-MSDU maximal
- * size is min(max_amsdu_len, 7935) bytes.
+ *
+ * * If the skb is transmitted as part of a BA agreement, the
+ * A-MSDU maximal size is min(max_amsdu_len, 4065) bytes.
+ * * If the skb is not part of a BA aggreement, the A-MSDU maximal
+ * size is min(max_amsdu_len, 7935) bytes.
+ *
* Both additional HT limits must be enforced by the low level driver.
* This is defined by the spec (IEEE 802.11-2012 section 8.3.2.2 NOTE 2).
* @support_p2p_ps: indicates whether the STA supports P2P PS mechanism or not.
@@ -3212,14 +3214,20 @@ enum ieee80211_reconfig_type {
* nor send aggregates in a way that lost frames would exceed the
* buffer size. If just limiting the aggregate size, this would be
* possible with a buf_size of 8:
- * - TX: 1.7
- * - RX: 27 (lost frame #1)
- * - TX:8..1...
+ *
+ * - ``TX: 1.7``
+ * - ``RX: 27`` (lost frame #1)
+ * - ``TX:8..1...``
+ *
* which is invalid since #1 was now re-transmitted well past the
* buffer size of 8. Correct ways to retransmit #1 would be:
- * - TX: 1 or 18 or 81
- * Even "189" would be wrong since 1 could be lost again.
- *
+ *
+ * - ``TX:1 or``
+ * - ``TX:18 or``
+ * - ``TX:81``
+ *
+ * Even ``189`` would be wrong since 1 could be lost again.
+ *
* Returns a negative error code on failure.
* The callback can sleep.
*
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] cfg80211: fix a documentation warning
From: Johannes Berg
The new restructured text parser complains about the formatting,
and really this should be a definition list.
Unfortunately, due to it being inside the parameter description,
and the definition list needing a blank line, this adds trailing
whitespace to keep it in the parameter description.
Signed-off-by: Johannes Berg
---
include/net/cfg80211.h | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index b7aba6e1a586..181e2d544a1b 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -3190,10 +3190,13 @@ struct ieee80211_iface_limit {
* @radar_detect_regions: bitmap of regions supported for radar detection
* @beacon_int_min_gcd: This interface combination supports different
* beacon intervals.
- * = 0 - all beacon intervals for different interface must be same.
- * > 0 - any beacon interval for the interface part of this combination AND
- * *GCD* of all beacon intervals from beaconing interfaces of this
- * combination must be greater or equal to this value.
+ *
+ * = 0
+ * all beacon intervals for different interface must be same.
+ * > 0
+ * any beacon interval for the interface part of this combination AND
+ * GCD of all beacon intervals from beaconing interfaces of this
+ * combination must be greater or equal to this value.
*
* With this structure the driver can describe which interface
* combinations it supports concurrently.
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] cfg80211: update wireless-regdb repo url in Documentation
On Mon, 2017-01-02 at 08:28 +0100, Rafał Miłecki wrote: > From: Rafał Miłecki > > It's maintained by Seth Forshe for a long time now. > Both applied, thanks. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Ksummit-discuss] Including images on Sphinx documents
> > > You had pointed me to this plugin before > > > https://pythonhosted.org/sphinxcontrib-aafig/ > > > > > > but I don't think it can actually represent any of the pictures. > > > > No, but there are some ascii art images inside some txt/rst files > > and inside some kernel-doc comments. We could either use the above > > extension for them or to convert into some image. The ascii art > > images I saw seem to be diagrams, so Graphviz would allow replacing > > most of them, if not all. > > Please don't replace ASCII art that effectively conveys conceptual > diagrams. If you do, we'll wind up in situations where someone > hasn't built the docs and doesn't possess the tools to see a diagram > that was previously shown by every text editor (or can't be bothered > to dig out the now separate file). In the name of creating > "prettier" diagrams (and final doc), we'll have damaged capacity to > understand stuff by just reading the source if this diagram is in > kernel doc comments. I think this is a good application of "if it > ain't broke, don't fix it". Right, I agree completely! That's the selling point of aafig though, it translates to pretty diagrams, but looks fine when viewed in a normal text editor (with fixed-width font) I had a hack elsewhere that would embed the fixed-width text if the plugin isn't present, which seemed like a decent compromise, but nobody is willing to let plugins be used in general to start with, it seems :) johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Ksummit-discuss] Including images on Sphinx documents
On Sat, 2016-11-19 at 10:15 -0700, Jonathan Corbet wrote: > > I don't know what the ultimate source of these images is (Mauro, > perhaps you could shed some light there?). I'd argue that it probably no longer matters. Whether it's xfig, svg, graphviz originally etc. - the source is probably long lost. Recreating these images in any other format is probably not very difficult for most or almost all of them. > Rather than beating our heads against the wall trying to convert > between various image formats, maybe we need to take a step > back. We're trying to build better documentation, and there is > certainly a place for diagrams and such in that > documentation. Johannes was asking about it for the 802.11 docs, and > I know Paul has run into these issues with the RCU docs as > well. Might there be a tool or an extension out there that would > allow us to express these diagrams in a text-friendly, editable > form? > > With some effort, I bet we could get rid of a number of the images, > and perhaps end up with something that makes sense when read in the > .rst source files as an extra benefit. I tend to agree, and I think that having this readable in the text would be good. You had pointed me to this plugin before https://pythonhosted.org/sphinxcontrib-aafig/ but I don't think it can actually represent any of the pictures. Some surely could be represented directly by having the graphviz source inside the rst file: http://www.sphinx-doc.org/en/1.4.8/ext/graphviz.html (that's even an included plugin, no need to install anything extra) graphviz is actually quite powerful, so I suspect things like dvbstb.png can be represented there, perhaps not pixel-identically, but at least semantically equivalently. However, I don't think we'll actually find a catch-all solution, so we need to continue this discussion here for a fallback anyway - as you stated (I snipped that quote, sorry), a picture describing the video formats will likely not be representable in text. As far as my use-case for sequence diagrams is concerned, I'd really like to see this integrated with the toolchain since the source format for them is in fact a text format. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Ksummit-discuss] Including images on Sphinx documents
> So, the problem that remains is for those images whose source > is a bitmap. If we want to stick with the Sphinx supported formats, > we have only two options for bitmaps: jpg or png. We could eventually > use uuencode or base64 to make sure that the patches won't use > git binary diff extension, or, as Arnd proposed, use a portable > bitmap format, in ascii, converting via Makefile, but losing > the alpha channel with makes the background transparent. > Or just "rewrite" them in svg? None of the gif files I can see actually look like they'd have been drawn in gif format anyway. The original source may be lost, but it doesn't seem all that hard to recreate them in svg. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] fs: add userspace critical mounts event support
On Tue, 2016-11-08 at 23:47 +0100, Luis R. Rodriguez wrote: > This issue still stands. At Plumbers Johannes Berg did indicate to me > he had a simple elegant solution in mind. He suggested that since the > usermode helper was available, he had added support to be able to > differentiate async firmware request calls form sync requests, For reference: commit e9045f9178f3e3445a3a5b85206f8681b3869562 Author: Johannes Berg Date: Mon Mar 29 17:57:20 2010 +0200 firmware class: export nowait to userspace > it can determine we're initramfs. The semantics issue is the same > though, is there a generic way to determine we're initramfs ? What if > we move multiple levels? Anyway -- provided we could figure this out, > userspace would simply yield and wait until the real rootfs is met. One way or another we have to have this kind of information somewhere. I don't actually know how/where though. > Upon pivot_root() the assumption is all previous udev events pending > would be re-triggered and finally udev could finally confirm/deny if > the firmware was present. The retriggering is already the case, as far as I know, if only to load modules that weren't part of initramfs. > note Johannes was asking for *all* async firmware requests to always > rely on the kernel syfs UMH fallback -- this suggestion is against > the direction we've been taking to eventually compartamentalize the > kernel UMH code, so whatever we decide to do, lets please take a > breather and seriously address this properly *with* systemd folks. I was saying that because that's the only way you can actually rely on this functionality as a system integrator. If drivers have to opt in or can opt out then you'll always end up chasing the drivers around. My argument basically goes like this: First, given good drivers (i.e. using request_firmware_nowait()) putting firmware even for a built-in driver into initramfs or not should be a system integrator decision. If they don't need the device that early, it should be possible for them to delay it. Or, perhaps, if the firmware is too big, etc. I'm sure we can all come up with more examples of why you'd want to do it one way or another. Second, all of this can be solved in other ways by adding logic to the kernel, like the rejected proposal to add a "rootfs available" bit somewhere, that would cause async requests to behave similarly within the kernel (don't return "not found" until they time out or this bit is set, and retry loading when the bit gets set) Third, having this in place can be more friendly to users who play with kernel compilation, modules, etc. This is a fringe group in some ways, but it is (was?) actually a relatively common complaint that drivers built into the kernel wouldn't work - we'd always have to direct users to do magic steps like rebuilding initramfs with the right options etc. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] mac80211: use inline kernel-doc for struct ieee80211_hw
On Wed, 2016-10-26 at 11:36 +0300, Jani Nikula wrote: > On Wed, 26 Oct 2016, Johannes Berg wrote: > > > > On Fri, 2016-10-21 at 15:57 +0300, Jani Nikula wrote: > > > > > > It's easier to manage the kernel-doc for the fields when they > > > documentation is next to the field. > > > > Ok, actually, this doesn't apply. Perhaps I'll look into scripting > > this > > kind of conversion. > > No problem; I'll leave it up to you. I'll probably end up doing it manually though - no chance I can extend the parser to output an spatch (doing the comment placement with spatch actually works, but getting there...) -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] mac80211: use inline kernel-doc for struct ieee80211_hw
On Fri, 2016-10-21 at 15:57 +0300, Jani Nikula wrote: > It's easier to manage the kernel-doc for the fields when they > documentation is next to the field. Ok, actually, this doesn't apply. Perhaps I'll look into scripting this kind of conversion. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] mac80211: fix some sphinx warnings
Applied. I'll take this into the RCs so we don't have warnings there, and the other patch into -next. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: The downside of math::
> I'd like to refine: Do not add non-trivial hard dependencies. Do not > add dependencies the lack of which make large parts of generated > documentation useless. > > Graceful degradation on unmet dependencies is the key here. Agree. > Give a build > warn about missing dependencies. Try to do something sensible without > the dependency. For extension directives, all else failing, embed the > raw directive block contents using in the output, possibly > accompanied > by a reStructuredText admonition block [1]. For the math and diagram > directives, I think this would work just fine. All of that would be easy to achieve with something like the dummy plugin I posted the other day :) But yes, I also agree that we shouldn't add any dependencies where the input isn't suitable as a fallback, since that also means that the input is going to be difficult to read and write in the source, which should remain a reasonable way to read things as well. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: The downside of math::
On Sun, 2016-10-23 at 12:58 +0200, Markus Heiser wrote:
> If you don't want to see a warning log, we have to consider
> a solution like Johannes Berg posted here:
>
> https://www.mail-archive.com/linux-doc@vger.kernel.org/msg07071.html
>
> @johannes: since this thread and our "sequence diagrams" thread
> addressing the same questions (how should we handle dependencies from
> extensions) I propose to continue the discussion here in this thread
> with Jon.
Sure.
As I said before, regarding the annoying "while (build fails)" loop
mentioned above - I think that's actually somewhat *better* than just
(silently?) failing the build because some dependency wasn't there.
Perhaps, with my code suitably adjusted (should be easy), we could
provide some kind of command line option like "DISABLE_PLUGINS=xyz" so
you could just build as though you had no plugins installed, to avoid
all the follow-up errors. (We'd implement this by changing my dummy
code to also pass the import statement, like this:
setup = create_setup("sphinxcontrib.plantuml", ["uml"])
and moving the try/except clause into the dummy, and then that can read
the environment variable easily and avoid even trying to import when
it's set - that way, no dependencies can be needed.
> Further I think we should not generate more (and more) external
> requirements like e.g. plantuml, Java or reportlab discussed here:
I still disagree, I think we should make it easy to "opt out" for the
build, but if we're really serious about writing good documentation we
shouldn't (artificially) limit the tools available.
Sure, we probably shouldn't add seqdiag, plantuml, and others that all
the do the same thing - if only for the sake of consistency in the
generated output! - but having the ability to have such things is very
nice and helpful for documentation.
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sequence diagrams in rst documentation
On Sat, 2016-10-22 at 18:37 +0200, Markus Heiser wrote: > Yeah, I thought something similar. But is the import of the extension > a sufficient criteria? > > About ".. math::"; I guess we have to check if math extension AND > pdflatex is installed. > > What do you suppose? TBH, I only considered this briefly, but decided that somebody who went to the effort of installing the sphinx extension would likely not mind to get a subsequent error when it tries to use something that's not installed. I was, in fact, quite surprised that I even could install (on Debian) the plantuml sphinx extension without plantuml, which seems odd. In fact, I think it would be *preferable* to only check the extension ; we should print a message from the dummy plugin to let them know what extensions they need, and if they then go to the effort of installing it they probably don't just not mind getting errors on dependencies, but actually would *prefer* that, since otherwise it can be daunting to try to figure out what *else* you actually have to install. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sequence diagrams in rst documentation
On Fri, 2016-10-21 at 18:11 +0200, Markus Heiser wrote:
> Yes and No. It depends on the tools (toolchains) we want to use.
> As far as I can see from a abstract POV it should by simple for
> math:: / since we already use/need LaTeX for PDF, for other tools
> I have to look.
Not sure if we were talking past each other - but the pass-through is
actually really simple - example patch below.
This makes it output an SVG (with fallback to PNG even) into the HTML,
a PDF into the PDF, and plain pre-formatted text for both when the
plantuml sphinx plugin isn't installed.
johannes
diff --git a/Documentation/conf.py b/Documentation/conf.py
index bf6f310e5170..2c00ab6f0baf 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -34,7 +34,8 @@ from load_config import loadConfig
# Add any Sphinx extension module names here, as strings. They can be
# extensions coming with Sphinx (named 'sphinx.ext.*') or your custom
# ones.
-extensions = ['kernel-doc', 'rstFlatTable', 'kernel_include', 'cdomain']
+extensions = ['kernel-doc', 'rstFlatTable', 'kernel_include', 'cdomain',
+ 'plantuml']
# The name of the math extension changed on Sphinx 1.4
if minor > 3:
@@ -494,6 +495,9 @@ pdf_documents = [
kerneldoc_bin = '../scripts/kernel-doc'
kerneldoc_srctree = '..'
+plantuml_output_format = "svg"
+plantuml_latex_output_format = "pdf"
+
#
--
# Since loadConfig overwrites settings from the global namespace, it has to be
# the last statement in the conf.py file
diff --git a/Documentation/driver-api/index.rst
b/Documentation/driver-api/index.rst
index 8e259c5d0322..e0d4f24b6039 100644
--- a/Documentation/driver-api/index.rst
+++ b/Documentation/driver-api/index.rst
@@ -7,6 +7,12 @@ of device drivers. This document is an only somewhat
organized collection
of some of those interfaces — it will hopefully get better over time! The
available subsections can be seen below.
+.. uml::
+
+ Alice -> Bob: Hi!
+ Alice <- Bob: How are you?
+
+
.. class:: toc-title
Table of contents
diff --git a/Documentation/sphinx/dummy.py b/Documentation/sphinx/dummy.py
new file mode 100644
index ..cfac42886ebd
--- /dev/null
+++ b/Documentation/sphinx/dummy.py
@@ -0,0 +1,24 @@
+from sphinx.util.compat import Directive
+from docutils import nodes
+from docutils.parsers.rst import directives
+
+class IgnoreOptions(object):
+def __getitem__(self, key):
+return directives.unchanged
+
+def create_setup(directives):
+def setup(app):
+for d in directives:
+class TextDirective(Directive):
+has_content = True
+option_spec = IgnoreOptions()
+
+def run(self):
+text = '\n'.join(self.content)
+return [nodes.literal_block('', text,
+ classes=['code',
+ 'missing-plugin',
+ 'missing-plugin-%s' % d])]
+
+app.add_directive(d, TextDirective)
+return setup
diff --git a/Documentation/sphinx/plantuml.py b/Documentation/sphinx/plantuml.py
new file mode 100644
index ..0007bc7f24fd
--- /dev/null
+++ b/Documentation/sphinx/plantuml.py
@@ -0,0 +1,7 @@
+# -*- coding: utf-8 -*-
+from dummy import create_setup
+
+try:
+from sphinxcontrib.plantuml import *
+except:
+setup = create_setup(['uml'])
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sequence diagrams in rst documentation
> I general I think, we should not include Java into our toolchains > even if it is optional. Thats, why I won't vote for > http://plantuml.com/ Keep in mind though that such a vote may well end up being a vote not just against plantuml, but also against having sequence diagrams to start with - at least as far as I'm concerned - since we (rightfully!) also don't want to start writing our own infrastructure (again). There aren't many alternatives. Even seqdiag/blockdiag isn't nearly as fully featured as plantuml. > IMO a tool which generates SVG would be the best, I have to check > if I find some or if some of the above could used in this way. plantuml does create svg :-> Ultimately, I actually think it doesn't matter much whether we add java or python or whatever to our toolchain. As soon as we add external dependencies that have to be fulfilled outside of the kernel git tree, and such fulfilment is easy (as it is in "sudo apt-get install plantuml python3-sphinxcontrib.plantuml"), it hardly matters what exactly is behind it. So ultimately, the way I see it, we essentially have two choices: 1) we reject external dependencies entirely, shipping everything in the tree that we need to build documentation 2) we accept external dependencies, hopefully finding ways to make them optional In case of 1), which we obviously don't have today since we don't ship sphinx and everything, we're basically reduced to having very simple python-only plugins. As far as I'm concerned this will mean that I'm not adding such diagrams to the kernel documentation, life's too short to rewrite something like plantuml, and I can't find anything that's pure python (and fulfilling your - IMHO a bit exaggerated - standards). In case of 2), as long as it's easy to install on a typical Linux distro, I think it doesn't actually matter that it's java or whatever else. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] mac80211: use inline kernel-doc for struct ieee80211_hw
On Fri, 2016-10-21 at 20:59 +0200, Arend van Spriel wrote: > Me neither. I actually found it annoying to have all kernel-doc above > the type definition and indeed easy to miss fields. Now struct > ieee80211_hw is not alone and we have it all over the place in the > wireless subsystem. Is this something we want to embrace and maybe > find janitors/newbies/coccinelle-gurus to address it. Not sure if > coccinelle could deal with this. I don't think coccinelle can look into comments, though perhaps it could be used to *place* them - use kernel-doc script to extract them first, and then make that output a coccinelle script to place the comment back at the right spot? Overall though, not sure that's worth it over just doing it by hand once? Then again, that could be used to mass-convert more ... :) johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] mac80211: fix some sphinx warnings
On Fri, 2016-10-21 at 16:14 +0300, Jani Nikula wrote:
> On Fri, 21 Oct 2016, Johannes Berg wrote:
> >
> > On Fri, 2016-10-21 at 15:57 +0300, Jani Nikula wrote:
> > >
> > > Signed-off-by: Jani Nikula
> > > ---
> > > include/net/mac80211.h | 21 +
> > > 1 file changed, 13 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> > > index a810dfcb83c2..e2dba93e374f 100644
> > > --- a/include/net/mac80211.h
> > > +++ b/include/net/mac80211.h
> > > @@ -811,14 +811,18 @@ enum mac80211_rate_control_flags {
> > > * in the control information, and it will be filled by the rate
> > > * control algorithm according to what should be sent. For
> > > example,
> > > * if this array contains, in the format { , } the
> > > - * information
> > > + * information::
> > >
> > [...]
> >
> > interesting, are these not enabled by default?
> rst wants indented blocks to be separated by blank lines from the
> preceding and following blocks. For that you could just add the blank
> lines, and it'll work.
>
> I presumed this was nicer as preformatted/monospaced text, and for
> that you'll need the "::". Up to you to have that or not.
Well, I meant the warnings :)
But yeah, this is better as preformatted text. It never came out that
way with docbook either, I think, but now we can fix that.
> > I guess I'll pick up these patches, even if they probably make
> > things
> > worse in my tree, since there it's still using docbook. But once we
> > merge everything it should be good then.
>
> Oh, it's built as reStructuredText in Linus' tree. Again, up to you.
Right. No worries. I just haven't gotten my own change back from
Linus's tree into mine, but it only means that generating the doc from
mac80211-next or net-next might be weird, which isn't a problem.
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] mac80211: fix some sphinx warnings
On Fri, 2016-10-21 at 15:57 +0300, Jani Nikula wrote:
> Signed-off-by: Jani Nikula
> ---
> include/net/mac80211.h | 21 +
> 1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index a810dfcb83c2..e2dba93e374f 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -811,14 +811,18 @@ enum mac80211_rate_control_flags {
> * in the control information, and it will be filled by the rate
> * control algorithm according to what should be sent. For example,
> * if this array contains, in the format { , } the
> - * information
> + * information::
>
[...]
interesting, are these not enabled by default?
I guess I'll pick up these patches, even if they probably make things
worse in my tree, since there it's still using docbook. But once we
merge everything it should be good then.
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] mac80211: use inline kernel-doc for struct ieee80211_hw
On Fri, 2016-10-21 at 15:57 +0300, Jani Nikula wrote: > It's easier to manage the kernel-doc for the fields when they > documentation is next to the field. Hah, I wasn't even aware this was possible. Thanks! johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sequence diagrams in rst documentation
> I had the same conclusion for math:: directives pulling in latex > dependency [1]. Hopefully Markus can help here. Yeah, good one. Maybe it's actually simple? Depending on where sphinx will look for plugins first, we could just ship the plugins with a no-op implementation (pass through the text as pre-formatted text), and if it finds the plugin first in a system-wide path that version would win for the better rendering? johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sequence diagrams in rst documentation
> > https://pythonhosted.org/sphinxcontrib-aafig/ > > > > I've not actually played with it at all, but I like the idea that > > we'd have readable diagrams in the source docs as well... > > Well, maybe. I agree having it readable in the source docs as well is > nice, but for sequence diagrams in particular, I don't think > > +---+ +---+ > | Hello +>+ aafigure! | > +---+ +---+ > > really beats > > Hello -> aafigure! I found another one: https://pypi.python.org/pypi/sphinxcontrib-plantuml That one has really nice output and features, but ends up being a *java* (of all the things) tool that the thing calls out to ... Perhaps we can have a compromise and embed the raw text when the tooling isn't all installed, so you can still build useful documentation, but to get all the "prettiness" you might have to install more dependencies? johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sequence diagrams in rst documentation
On Tue, 2016-10-18 at 17:52 -0600, Jonathan Corbet wrote: > In summary, I think we can consider taking on a module if it's what > we need to do the docs right. And if somebody agrees to maintain it! > :) :) I think for the ones that we carry, they're probably specific? > I've heard others say they would like better diagramming support. Do > you think that, maybe, something like aafigure would do the trick? > > https://pythonhosted.org/sphinxcontrib-aafig/ > > I've not actually played with it at all, but I like the idea that > we'd have readable diagrams in the source docs as well... Well, maybe. I agree having it readable in the source docs as well is nice, but for sequence diagrams in particular, I don't think +---+ +---+ | Hello +>+ aafigure! | +---+ +---+ really beats Hello -> aafigure! by much. You could do some alignment even with the latter version, and the above isn't even really a sequence diagram anyway :) Some of the sequence diagrams may also be automatically generated from runtime behaviour observation (e.g. tracing, we've actually done that), and outputting the types of ascii boxes is much more difficult there. For other types of diagrams this may be nice though. Anyway, I guess we'll cross that bridge when we get to it. I'd like to have these types of documentation, perhaps with a script to auto- generate from tracing - such as script and visual display can even serve as a debugging aid :) johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sequence diagrams in rst documentation
> This could probably be argued either way... Yeah, I guess it could :) > My view has been all along that we should prefer to use existing > extensions written and maintained by others. Perhaps we (the kind of > royal "we" of which I'm personally really not part of) could take on > maintainership of some extensions in the interest of improving kernel > documentation, but I think the goal should be that the extensions are > maintained outside of the kernel tree, that the extensions are > generally usable, and have a chance of attracting attention and > contributions from outside of the kernel community. (Note that this > doesn't preclude us from shipping the extensions in the kernel tree, > as long as it's updated from the upstream, not forked.) Right. I tend to agree, though in the particular case I'm looking at we'd probably have to fork outside the kernel, forming a new upstream, and then ship that version (or perhaps rewrite it, forming a new upstream, and then ship that - doesn't matter all that much) > (This is one part of me being unhappy about making it easy to run > arbitrary scripts to produce documentation; those will never be > generic, and we'll never be able to offload their maintenance outside > of the kernel. We should not think that we have some really special > needs in the kernel.) I agree that we don't necessarily have any special needs (*), but in cases like this (**) it does seem more practical to just ship the plugin with the kernel. Whether or not a separate "upstream" is formed for it could be a secondary question, although it does seem better to do so. (*) although not wanting to ship binary files *is* kinda special :) (**) where the upstream is essentially dead (for all I can tell) and severely limited to the point where a rewrite will be a better choice. Anyway, I'll have to see if we (Intel Linux WiFi team) actually want to do things this way. Using the existing blockdiag/seqdiag is practical since it all exists already. OTOH, a simpler and better-looking solution would also be nice, so if we do go this way I'll investigate more what we can do around this. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sequence diagrams in rst documentation
On Tue, 2016-10-18 at 15:51 +0200, Markus Heiser wrote: > Here are my thoughts ... > > Every extension which is not a part of the sphinx distro brings new > external dependencies I agree. > and the development of such extensions is IMO > far of kernel development's scope. Arguably, having good documentation *is* in the scope of kernel development. Also, it could be argued that shipping a module in the kernel sources would be more reliable than having to require it being externally installed, like the GCC plugins perhaps. > ATM, there are not many use cases for diagram generators, so why not > be KISS and creating diagrams with the favorite tool only adding the > resulting (e.g.) PNG to the Kernel's source? *Only* adding the PNG would be awful, I'd have to keep track of the corresponding source elsewhere, and perhaps couldn't even GPL it because then I couldn't distribute the PNG without corresponding source... Adding the source text would really be the only practical choice, but doing so makes it easy to mismatch things, and also very easy to use proprietary services for it that may go away at any time, etc. > Before we add new dependencies / complexity, we should get rid > of the DocBook build. That argument is ... completely bogus, politely said. I'm not going to (be able to) do anything about all the docbooks that exist, and have in fact converted the one docbook that I know anything about. Holding back the development of documentation in one subsystem because another subsystem hasn't converted is a garbage argument. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
sequence diagrams in rst documentation
On Tue, 2016-10-11 at 15:53 +0200, Johannes Berg wrote: > > > > > > Related question: I have some sequence diagrams, and just found the > > seqdiag sphinx plugin. How should we manage adding extensions? Or > > would you prefer not to add any at all? > > Example here: > https://johannes.sipsolutions.net/files/80211/mac80211.html#connection-flow Coming back to this - sadly, it appears that this software (blockdiag, seqdiag) is completely unmaintained, with open pull requests dating back to 2012 and the last commit dating back to 2015-08-22. I'd want/need feature improvements in it too, but if I can't feed those back to upstream (since it appears dead), there's little point. Perhaps we can ship plugins for this as part of the kernel sources? Shouldn't be too difficult to reimplement something like this, after all. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mac80211_hwsim: suggest nl80211 instead of wext driver in documentation
On Mon, 2016-10-17 at 00:39 +0200, Linus Lüssing wrote:
> For mac80211_hwsim interfaces, suggest to use wpa_supplicant with the
> more modern, netlink based driver instead of wext.
Makes sense, applied.
> Actually, I wasn't even able to make a connection with the
> configuration
> files and information provided in
> Documentation/networking/mac80211_hwsim/{README,hostapd.conf/wpa_supp
> licant.conf}
>
This less so, we even have a few test cases we run regularly, but I
don't know. Maybe there's something special in those configuration
files that we don't test for otherwise.
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger
On Mon, 2016-06-13 at 23:21 +0200, Pavel Machek wrote: > > (Actually, "::wifi" is super confusing, I'd expect that to be a led > that blinks when wifi is active.) Agree with that, yeah, that'd be confusing. > Well... "airplane" is quite confusing. I'd we use "rfkill" for > disabling radios, and I believe we should stick with that. > > But small problem might be polarity. You may need both "::rfkill" and > "::not-rfkill". I'd argue that "airplane" matches better what you're likely to find on the chassis of the system. In either case I think the suffixes should be documented, for both future kernel and application developers. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger
On Thu, 2016-05-19 at 09:16 +0200, Pavel Machek wrote: > > In the original situation, without these patches, userspace has to > > have a list of all LEDs that are supposed to indicate airplane > > mode. > Well, that's situation for many LEDs. That doesn't make it a *good* situation though. > > With this patch only (without patch 2/3), userspace can look up the > > default trigger, but then has to change it, causing the necessary > > information to be lost immediately when you actually use it - that > > also seems like a bad idea. > We should not store "what kind of led this is" in a trigger. That's pretty much what I'm arguing though. > LED > subsystem seems to use suffix of LED name to do that. So if we > standartize, lets say "::rfkill" suffix for this, it should work and > follow existing practice. [...] > There is one -- suffix in the LED name. I don't really think that's a good way, and it doesn't seem to be used universally, but I suppose it's good enough. João, that means you should send a patch to add the ::rfkill suffix. And Pavel should send a patch to document the practice and the existing suffixes with their meaning ;-) johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger
On Wed, 2016-05-04 at 09:29 +0200, Pavel Machek wrote: > > If userspace wants to control the manually, it can do just that -- > control it manually. There should not be a need to "override the > default policy". I'm still not buying this. In the original situation, without these patches, userspace has to have a list of all LEDs that are supposed to indicate airplane mode. With this patch only (without patch 2/3), userspace can look up the default trigger, but then has to change it, causing the necessary information to be lost immediately when you actually use it - that also seems like a bad idea. With the patches, the userspace that cares can also concentrate on something it already *does* - i.e. dealing with the rfkill API - and let the rest of the situation be sorted out in itself. Now, if the LED subsystem had a really good way of specifying LED intent, and it was widely used, and rfkill didn't already concern itself with the rfkill status of all devices ... yeah maybe this wouldn't be needed. As it stands, I still think this is the best way forward. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 08/10] rfkill: Use switch to demux userspace operations
On Tue, 2016-03-01 at 00:39 +0200, Jouni Malinen wrote:
> > I agree there is a difference in the logic here,
Gah. I thought I'd reviewed the logic and made sure there's no
difference ... :)
> > thanks for taking the
> > time to point it out so clearly, and sorry for missing this. But AFAIU
> > userspace should not call RFKILL_OP_CHANGE with ev.type ==
> > RFKILL_TYPE_ALL, as RFKILL_OP_CHANGE is intended to be used to
> > block/unblock one RFKill switch, and it is not possible to create a
> > RFKill switch with type == RFKILL_TYPE_ALL (rfkill_alloc() would
> > return NULL).
> Interesting. Maybe Johannes can comment on that part since I think he
> wrote the code that interacts with kernel for the rfkill test cases.
So first of all, it seems that this argument is invalid since we can't break
the ABI/API here; although perhaps if it's only a test case ...
Oh. It took me a while, but I see now. The original intent (I think)
was that with RFKILL_OP_CHANGE, the type would be ignored entirely. It
seems that the (my) original intent wouldn't have been to force
userspace to specify *both* the index and the type, but instead do
OP_CHANGE_ALL -> use type (possibly TYPE_ALL, ignoring idx)
OP_CHANGE -> use idx (ignoring type)
The original code implemented it as follows:
if (rfkill->idx != ev.idx && ev.op != RFKILL_OP_CHANGE_ALL)
continue;
-> check the idx only for OP_CHANGE
if (rfkill->type != ev.type && ev.type != RFKILL_TYPE_ALL)
continue;
-> check the type, allowing _ALL
Now, all userspace that I found sets the ev.type field to TYPE_ALL all
the time; and it had to given these checks.
e.g. from rfkill.py:
# idx, type, op, soft, hard
_event_struct = '@I'
[...]
def block(self):
rfk = open('/dev/rfkill', 'w')
s = struct.pack(_event_struct, self.idx, TYPE_ALL, _OP_CHANGE, 1, 0)
rfk.write(s)
rfk.close()
This check, originally, probably should've been
if (rfkill->type != ev.type && ev.type != RFKILL_TYPE_ALL &&
ev.op != RFKILL_OP_CHANGE)
continue;
to ignore the type entirely.
I'm fine with Jouni's change, preserving the original behaviour of
requiring TYPE_ALL or the correct type, but I'm tempted to simply
remove the type check entirely.
Thoughts?
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: custom ioctl-based interface to control LED in networking (was Re: [PATCHv2 09/10] rfkill: Userspace control for airplane mode)
On Wed, 2016-02-24 at 13:14 +0100, Pavel Machek wrote: > > Why would it need to? It could look at default triggers for the led > if it really wanted to. And then it needs to change them; if anything goes wrong error recovery is practically impossible since the trigger information is lost forever. > > I'm sure you'd also not like to see 2**7 triggers implemented in > > rfkill > > to cover all the possibilities. > > Are all the possibilities useful? I assumed about 2 are. If you need > 2**7 of them, LED triggers can have parameters. It's not my position to decide which combinations some system integrator or userspace developer might find useful. Even when we add parameters to a trigger (I don't see a generic way to do that, but please do enlighten me), you're now ignoring the issue of the userspace controlled GSM modem... > > Really what you have here is a concept of "airplane mode", and that > > concept is specific to the rfkill subsystem. This happens to affect > > mostly an LED trigger, today, but as a concept it's something that > > *should* be managed within the rfkill subsystem. > > How is that concept used outside the LEDs? What semantics does > "airplane mode" have? You tried to explain "airplane mode" is not > well defined up in this thread... I'd say it's well-defined for any given set of system software, so if e.g. NetworkManager decides to define it one way, and connman another way, there's a definition but the kernel need not interfere with it. > > > Besides, the series really should have been Cc-ed to LED > > > people, too. > > > > That's simply unreasonable, you're essentially saying that any user > > of any kernel infrastructure should be Cc'ed to the implementer of > > that infrastructure... 9/10 patches in this series aren't even LED > > specific, > > I'm not saying that. I'm saying that LED maintainers should be Cced, > to keep the interfaces consistent. I pretty much have to read it that way, since the LED API is in no way impacted by these changes. Here's a new trigger, with some magic inner working. No impact on the LED API. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: custom ioctl-based interface to control LED in networking (was Re: [PATCHv2 09/10] rfkill: Userspace control for airplane mode)
On Wed, 2016-02-24 at 11:46 +0100, Pavel Machek wrote: > If you want different trigger, implement different trigger. If you > want to indicate all but wifi, implement all but wifi, and then > userspace can select it by writing trigger name. This is still mostly a strawman, since userspace cannot have a database of LEDs that indicate airplane mode. I'm sure you'd also not like to see 2**7 triggers implemented in rfkill to cover all the possibilities. > If you want complete > userspace control, fine, but we have standard interface and it is not > ioctl. The "standard interface" is usable if you really just want to driver a single LED and you know which one. I think you're looking at this the wrong way, focusing too much on the LED aspect. Really what you have here is a concept of "airplane mode", and that concept is specific to the rfkill subsystem. This happens to affect mostly an LED trigger, today, but as a concept it's something that *should* be managed within the rfkill subsystem. > Besides, the series really should have been Cc-ed to LED > people, too. That's simply unreasonable, you're essentially saying that any user of any kernel infrastructure should be Cc'ed to the implementer of that infrastructure... 9/10 patches in this series aren't even LED specific, only the *previous* patch, the one that tied the "airplane mode" concept to an LED trigger in a very standard way had anything to do with LED triggers at all. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 09/10] rfkill: Userspace control for airplane mode
On Tue, 2016-02-23 at 22:45 +0100, Pavel Machek wrote: > Well "the airplane mode" is well defined. It means no intentional > transmitting at radio frequencies. > > The fact that you are allowed to use WIFI on certain flights does not > change anything. Nope, not that simple. Pick up any (contemporary) smartphone and watch what happens with the airplane mode indicator (the little airplane icon) when you enable wifi after enabling airplane mode. It stays there. Clearly the same logic could then apply to an actual LED (on a system that's less size-constrained, e.g. a laptop or tablet.) Thus, the display of "in airplane mode" *does* have policy, and clearly there's precedent for not disabling the icon or LED when wifi is enabled, but the kernel shouldn't really impose that. Now, the kernel has a "safe" default which does what you thought was the "well defined" airplane mode, but at the same time it's obviously not good enough. And evidently, the Asus system that this was originally proposed for has such an LED. > I see that "airplane mode" trigger might be a tiny bit > useful... dunno, for a LED near the airplane mode switch, when vendor > replaced hardware toggle with a key. But policy should have nothing > to do with that. If you argue additional "policy daemon" is needed > for that... forget it, that's overdesigned. See above. > (Besides, finding all LEDs with given trigger is trivial > operation. Besides... there should never be more than one). That *might* actually work. But once a tool has detached the trigger the information is gone; and tools would have to do that to control the LED, making recovery from any kind of error difficult. In any case, I've applied those patches already. As far as the LED subsystem is concerned, all of this is just another trigger anyway. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 09/10] rfkill: Userspace control for airplane mode
On Tue, 2016-02-23 at 21:45 +0100, Pavel Machek wrote: > So... you add LED trigger to display the state of the airplane > mode. Ok, why not. Yes. However, consider that "the airplane mode" isn't a well-defined concept; some systems may want to light up that LED even when wifi is still enabled, since you're nowadays quite likely to be allowed to use wifi (but not enable e.g. LTE) while in-flight. > But now you add an way to override it? Why? If someone wants to > change > the led state, he can just change trigger to none, and then control > the LED manually... Yes, but now you've forced every application that wants to deal with this to know about every single LED that might be used with this trigger... that won't work for some generic userland tool that might want to implement an "airplane-mode policy". > BTW what happens when the device contains both radios controlled by > kernel (wifi, bluetooth) and radios controlled by userspace (GSM > modem)? You'd better have the userspace to control the LED :) johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/9] rfkill: Userspace control for airplane mode
Hi,
Sorry for the delay reviewing this.
On Mon, 2016-02-08 at 10:41 -0500, João Paulo Rechi Vita wrote:
> Provide an interface for the airplane-mode indicator be controlled
> from
> userspace. User has to first acquire the control through
> RFKILL_OP_AIRPLANE_MODE_ACQUIRE and keep the fd open for the whole
> time
> it wants to be in control of the indicator. Closing the fd or using
> RFKILL_OP_AIRPLANE_MODE_RELEASE restores the default policy.
I've come to the conclusion that the new ops are probably the best
thing to do here.
> +Userspace can also override the default airplane-mode indicator
> policy through
> +/dev/rfkill. Control of the airplane mode indicator has to be
> acquired first,
> +using RFKILL_OP_AIRPLANE_MODE_ACQUIRE, and is only available for one
> userspace
> +application at a time. Closing the fd or using
> RFKILL_OP_AIRPLANE_MODE_RELEASE
> +reverts the airplane-mode indicator back to the default kernel
> policy and makes
> +it available for other applications to take control. Changes to the
> +airplane-mode indicator state can be made using
> RFKILL_OP_AIRPLANE_MODE_CHANGE,
> +passing the new value in the 'soft' field of 'struct rfkill_event'.
I don't really see any value in _RELEASE, since an application can just
close the fd? I'd prefer not having the duplicate functionality
and force us to exercise the single code path every time.
> For further details consult Documentation/ABI/stable/sysfs-class-
> rfkill.
> diff --git a/include/uapi/linux/rfkill.h
> b/include/uapi/linux/rfkill.h
> index 2e00dce..9cb999b 100644
> --- a/include/uapi/linux/rfkill.h
> +++ b/include/uapi/linux/rfkill.h
> @@ -67,6 +67,9 @@ enum rfkill_operation {
> RFKILL_OP_DEL,
> RFKILL_OP_CHANGE,
> RFKILL_OP_CHANGE_ALL,
> + RFKILL_OP_AIRPLANE_MODE_ACQUIRE,
> + RFKILL_OP_AIRPLANE_MODE_RELEASE,
> + RFKILL_OP_AIRPLANE_MODE_CHANGE,
> };
> @@ -1199,7 +1202,7 @@ static ssize_t rfkill_fop_write(struct file
> *file, const char __user *buf,
> if (copy_from_user(&ev, buf, count))
> return -EFAULT;
>
> - if (ev.op != RFKILL_OP_CHANGE && ev.op !=
> RFKILL_OP_CHANGE_ALL)
> + if (ev.op < RFKILL_OP_CHANGE)
> return -EINVAL;
You need to also reject invalid high values, like 27.
> mutex_lock(&rfkill_global_mutex);
>
> + if (ev.op == RFKILL_OP_AIRPLANE_MODE_ACQUIRE) {
> + if (rfkill_apm_owned && !data->is_apm_owner) {
> + count = -EACCES;
> + } else {
> + rfkill_apm_owned = true;
> + data->is_apm_owner = true;
> + }
> + }
> +
> + if (ev.op == RFKILL_OP_AIRPLANE_MODE_RELEASE) {
It would probably be better to simply use "switch (ev.op)" and make the
default case do a reject.
> if (ev.op == RFKILL_OP_CHANGE_ALL)
> rfkill_update_global_state(ev.type, ev.soft);
Also moving the existing code inside the switch, of course.
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/9] rfkill: Create "rfkill-airplane_mode" LED trigger
On Mon, 2016-02-08 at 10:41 -0500, João Paulo Rechi Vita wrote: > This creates a new LED trigger to be used by platform drivers as a > default trigger for airplane-mode indicator LEDs. > > By default this trigger will fire when RFKILL_OP_CHANGE_ALL is called > for all types (RFKILL_TYPE_ALL), setting the LED brightness to > LED_FULL > when the changing the state to blocked, and to LED_OFF when the > changing > the state to unblocked. In the future there will be a mechanism for > userspace to override the default policy, so it can implement its > own. > > This trigger will be used by the asus-wireless x86 platform driver. Just one comment - I think you should be consistent with the _ vs - and just use "rfkill-airplane-mode" as the name. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/9] rfkill: Userspace control for airplane mode
On 2016-02-10 17:53, Dan Williams wrote: Yeah, I get that now. It's just that to me, something called "AIRPLANE_MODE_CHANGE" seems like it should actually change airplane mode on/off, which implies killing radios. I wouldn't have had the problem if it was named AIRPLANE_MODE_INDICATOR_CHANGE, which makes it clear this is simply an indicator and has no actual effect on anything other than a LED. Yeah, I agree. I'm not even sure that it's a good idea to subsume this into the regular operations in the API, although that's obviously the easiest extension. I'll need to think about that a bit more with the code at hand though. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/9] rfkill: Userspace control for airplane mode
On Mon, 2016-02-08 at 10:11 -0600, Dan Williams wrote: > I'd like to clarify a bit, so tell me if I'm correct or not. Using > RFKILL_OP_AIRPLANE_MODE_CHANGE does not actually change any device > state. It's just an indicator with no relationship to any of the > registered rfkill switches, right? Yes. I'm not sure I'm totally happy with this userspace API. > I wonder if setting RFKILL_OP_AIRPLANE_MODE_CHANGE(true) shouldn't > also > softblock all switches, otherwise you can set airplane mode all day > long with RFKILL_OP_AIRPLANE_MODE_CHANGE and it doesn't actually > enable > airplane mode at all? No, this is kinda the point that you could implement your policy in userspace now. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/9] rfkill: Userspace control for airplane mode
On 2016-02-08 17:20, Marcel Holtmann wrote: as explained in an earlier response, the concept Airplane Mode seems to be imposing policy into the kernel. Do we really want have this as a kernel exposed API. This patch is the one that *solves* that problem ... please read it more carefully? johannes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
