Documentation/driver-api/pm/devices.rst | 8 ++++++++ include/linux/kernel.h | 18 ++++++++++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-)
Provide some basic comments about the system_states and what they imply.
Also convert the comments to kernel-doc format.
However, kernel-doc does not support kernel-doc notation on extern
struct/union/typedef/enum/etc. So I made this a DOC: block so that
I can use (insert) it into a Documentation (.rst) file and have it
look decent.
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Acked-by: Rafael J. Wysocki <rafael@kernel.org> # v1
---
v2: add Rafael's Ack.
v3: add Andrew
v4: add DOC: so that this DOC: block can be used in Documentation/
add Greg K-H
add Jon Corbet, Mauro Chehab, & linux-doc
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Len Brown <len.brown@intel.com>
Cc: linux-pm@vger.kernel.org
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: linux-doc@vger.kernel.org
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
---
Documentation/driver-api/pm/devices.rst | 8 ++++++++
include/linux/kernel.h | 18 ++++++++++++++++--
2 files changed, 24 insertions(+), 2 deletions(-)
--- linux-next-20250819.orig/include/linux/kernel.h
+++ linux-next-20250819/include/linux/kernel.h
@@ -164,8 +164,22 @@ extern int root_mountflags;
extern bool early_boot_irqs_disabled;
-/*
- * Values used for system_state. Ordering of the states must not be changed
+/**
+ * DOC: General system_states available for drivers
+ *
+ * enum system_states - Values used for system_state.
+ *
+ * * @SYSTEM_BOOTING: %0, no init needed
+ * * @SYSTEM_SCHEDULING: system is ready for scheduling; OK to use RCU
+ * * @SYSTEM_FREEING_INITMEM: system is freeing all of initmem; almost running
+ * * @SYSTEM_RUNNING: system is up and running
+ * * @SYSTEM_HALT: system entered clean system halt state
+ * * @SYSTEM_POWER_OFF: system entered shutdown/clean power off state
+ * * @SYSTEM_RESTART: system entered emergency power off or normal restart
+ * * @SYSTEM_SUSPEND: system entered suspend or hibernate state
+ *
+ * Note:
+ * Ordering of the states must not be changed
* as code checks for <, <=, >, >= STATE.
*/
extern enum system_states {
--- linux-next-20250819.orig/Documentation/driver-api/pm/devices.rst
+++ linux-next-20250819/Documentation/driver-api/pm/devices.rst
@@ -241,6 +241,14 @@ before reactivating its class I/O queues
More power-aware drivers might prepare the devices for triggering system wakeup
events.
+System states available for drivers
+-----------------------------------
+
+These system states are available for drivers to help them determine how to
+handle state transitions.
+
+.. kernel-doc:: include/linux/kernel.h
+ :doc: General system_states available for drivers
Call Sequence Guarantees
------------------------
On Wed, 03 Sep 2025, Randy Dunlap <rdunlap@infradead.org> wrote: > Provide some basic comments about the system_states and what they imply. > Also convert the comments to kernel-doc format. > > However, kernel-doc does not support kernel-doc notation on extern > struct/union/typedef/enum/etc. So I made this a DOC: block so that > I can use (insert) it into a Documentation (.rst) file and have it > look decent. The simple workaround is to separate the enum type and the variable: /** * kernel-doc for the enum */ enum system_states { ... }; /** * kernel-doc for the variable */ extern enum system_states system_state; BR, Jani. > > Signed-off-by: Randy Dunlap <rdunlap@infradead.org> > Acked-by: Rafael J. Wysocki <rafael@kernel.org> # v1 > --- > v2: add Rafael's Ack. > v3: add Andrew > v4: add DOC: so that this DOC: block can be used in Documentation/ > add Greg K-H > add Jon Corbet, Mauro Chehab, & linux-doc > > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: "Rafael J. Wysocki" <rafael@kernel.org> > Cc: Pavel Machek <pavel@ucw.cz> > Cc: Len Brown <len.brown@intel.com> > Cc: linux-pm@vger.kernel.org > Cc: Jonathan Corbet <corbet@lwn.net> > Cc: linux-doc@vger.kernel.org > Cc: Mauro Carvalho Chehab <mchehab@kernel.org> > Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com> > --- > Documentation/driver-api/pm/devices.rst | 8 ++++++++ > include/linux/kernel.h | 18 ++++++++++++++++-- > 2 files changed, 24 insertions(+), 2 deletions(-) > > --- linux-next-20250819.orig/include/linux/kernel.h > +++ linux-next-20250819/include/linux/kernel.h > @@ -164,8 +164,22 @@ extern int root_mountflags; > > extern bool early_boot_irqs_disabled; > > -/* > - * Values used for system_state. Ordering of the states must not be changed > +/** > + * DOC: General system_states available for drivers > + * > + * enum system_states - Values used for system_state. > + * > + * * @SYSTEM_BOOTING: %0, no init needed > + * * @SYSTEM_SCHEDULING: system is ready for scheduling; OK to use RCU > + * * @SYSTEM_FREEING_INITMEM: system is freeing all of initmem; almost running > + * * @SYSTEM_RUNNING: system is up and running > + * * @SYSTEM_HALT: system entered clean system halt state > + * * @SYSTEM_POWER_OFF: system entered shutdown/clean power off state > + * * @SYSTEM_RESTART: system entered emergency power off or normal restart > + * * @SYSTEM_SUSPEND: system entered suspend or hibernate state > + * > + * Note: > + * Ordering of the states must not be changed > * as code checks for <, <=, >, >= STATE. > */ > extern enum system_states { > --- linux-next-20250819.orig/Documentation/driver-api/pm/devices.rst > +++ linux-next-20250819/Documentation/driver-api/pm/devices.rst > @@ -241,6 +241,14 @@ before reactivating its class I/O queues > More power-aware drivers might prepare the devices for triggering system wakeup > events. > > +System states available for drivers > +----------------------------------- > + > +These system states are available for drivers to help them determine how to > +handle state transitions. > + > +.. kernel-doc:: include/linux/kernel.h > + :doc: General system_states available for drivers > > Call Sequence Guarantees > ------------------------ > -- Jani Nikula, Intel
On 9/5/25 2:02 AM, Jani Nikula wrote: > On Wed, 03 Sep 2025, Randy Dunlap <rdunlap@infradead.org> wrote: >> Provide some basic comments about the system_states and what they imply. >> Also convert the comments to kernel-doc format. >> >> However, kernel-doc does not support kernel-doc notation on extern >> struct/union/typedef/enum/etc. So I made this a DOC: block so that >> I can use (insert) it into a Documentation (.rst) file and have it >> look decent. > > The simple workaround is to separate the enum type and the variable: > > /** > * kernel-doc for the enum > */ > enum system_states { > ... > }; > > /** > * kernel-doc for the variable > */ > extern enum system_states system_state; Duh. Beautiful. Thanks. @Andrew, please drop the v4 patch. I'll make another. -- ~Randy
Em Fri, 05 Sep 2025 12:02:37 +0300 Jani Nikula <jani.nikula@linux.intel.com> escreveu: > On Wed, 03 Sep 2025, Randy Dunlap <rdunlap@infradead.org> wrote: > > Provide some basic comments about the system_states and what they imply. > > Also convert the comments to kernel-doc format. > > > > However, kernel-doc does not support kernel-doc notation on extern > > struct/union/typedef/enum/etc. So I made this a DOC: block so that > > I can use (insert) it into a Documentation (.rst) file and have it > > look decent. > > The simple workaround is to separate the enum type and the variable: > > /** > * kernel-doc for the enum > */ > enum system_states { > ... > }; > > /** > * kernel-doc for the variable > */ > extern enum system_states system_state; > > BR, > Jani. > > > > > Signed-off-by: Randy Dunlap <rdunlap@infradead.org> > > Acked-by: Rafael J. Wysocki <rafael@kernel.org> # v1 > > --- > > v2: add Rafael's Ack. > > v3: add Andrew > > v4: add DOC: so that this DOC: block can be used in Documentation/ > > add Greg K-H > > add Jon Corbet, Mauro Chehab, & linux-doc > > > > Cc: Andrew Morton <akpm@linux-foundation.org> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Cc: "Rafael J. Wysocki" <rafael@kernel.org> > > Cc: Pavel Machek <pavel@ucw.cz> > > Cc: Len Brown <len.brown@intel.com> > > Cc: linux-pm@vger.kernel.org > > Cc: Jonathan Corbet <corbet@lwn.net> > > Cc: linux-doc@vger.kernel.org > > Cc: Mauro Carvalho Chehab <mchehab@kernel.org> > > Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com> > > --- > > Documentation/driver-api/pm/devices.rst | 8 ++++++++ > > include/linux/kernel.h | 18 ++++++++++++++++-- > > 2 files changed, 24 insertions(+), 2 deletions(-) > > > > --- linux-next-20250819.orig/include/linux/kernel.h > > +++ linux-next-20250819/include/linux/kernel.h > > @@ -164,8 +164,22 @@ extern int root_mountflags; > > > > extern bool early_boot_irqs_disabled; > > > > -/* > > - * Values used for system_state. Ordering of the states must not be changed > > +/** > > + * DOC: General system_states available for drivers > > + * > > + * enum system_states - Values used for system_state. > > + * > > + * * @SYSTEM_BOOTING: %0, no init needed > > + * * @SYSTEM_SCHEDULING: system is ready for scheduling; OK to use RCU > > + * * @SYSTEM_FREEING_INITMEM: system is freeing all of initmem; almost running > > + * * @SYSTEM_RUNNING: system is up and running > > + * * @SYSTEM_HALT: system entered clean system halt state > > + * * @SYSTEM_POWER_OFF: system entered shutdown/clean power off state > > + * * @SYSTEM_RESTART: system entered emergency power off or normal restart > > + * * @SYSTEM_SUSPEND: system entered suspend or hibernate state > > + * > > + * Note: > > + * Ordering of the states must not be changed > > * as code checks for <, <=, >, >= STATE. > > */ > > extern enum system_states { > > --- linux-next-20250819.orig/Documentation/driver-api/pm/devices.rst > > +++ linux-next-20250819/Documentation/driver-api/pm/devices.rst > > @@ -241,6 +241,14 @@ before reactivating its class I/O queues > > More power-aware drivers might prepare the devices for triggering system wakeup > > events. > > > > +System states available for drivers > > +----------------------------------- > > + > > +These system states are available for drivers to help them determine how to > > +handle state transitions. > > + > > +.. kernel-doc:: include/linux/kernel.h > > + :doc: General system_states available for drivers > > > > Call Sequence Guarantees > > ------------------------ > > > If the problem is with "extern" before enum, fixing kdoc be fairly trivial. If you see: def dump_function(self, ln, prototype): # Prefixes that would be removed sub_prefixes = [ ... (r"^extern +", "", 0), ... } for search, sub, flags in sub_prefixes: prototype = KernRe(search, flags).sub(sub, prototype) we need something similar to that at: def dump_enum(self, ln, proto): alternatively, we could use a simpler approach modifying adding a non-group optional match for "extern". E.g: - r = KernRe(r'enum\s+(\w*)\s*\{(.*)\}') + r = KernRe(r'(?:extern\s+)?enum\s+(\w*)\s*\{(.*)\}') (untested) Regards, Mauro Thanks, Mauro
On Fri, 05 Sep 2025, Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > Em Fri, 05 Sep 2025 12:02:37 +0300 > Jani Nikula <jani.nikula@linux.intel.com> escreveu: > >> On Wed, 03 Sep 2025, Randy Dunlap <rdunlap@infradead.org> wrote: >> > Provide some basic comments about the system_states and what they imply. >> > Also convert the comments to kernel-doc format. >> > >> > However, kernel-doc does not support kernel-doc notation on extern >> > struct/union/typedef/enum/etc. So I made this a DOC: block so that >> > I can use (insert) it into a Documentation (.rst) file and have it >> > look decent. >> >> The simple workaround is to separate the enum type and the variable: >> >> /** >> * kernel-doc for the enum >> */ >> enum system_states { >> ... >> }; >> >> /** >> * kernel-doc for the variable >> */ >> extern enum system_states system_state; >> >> BR, >> Jani. >> >> > >> > Signed-off-by: Randy Dunlap <rdunlap@infradead.org> >> > Acked-by: Rafael J. Wysocki <rafael@kernel.org> # v1 >> > --- >> > v2: add Rafael's Ack. >> > v3: add Andrew >> > v4: add DOC: so that this DOC: block can be used in Documentation/ >> > add Greg K-H >> > add Jon Corbet, Mauro Chehab, & linux-doc >> > >> > Cc: Andrew Morton <akpm@linux-foundation.org> >> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> > Cc: "Rafael J. Wysocki" <rafael@kernel.org> >> > Cc: Pavel Machek <pavel@ucw.cz> >> > Cc: Len Brown <len.brown@intel.com> >> > Cc: linux-pm@vger.kernel.org >> > Cc: Jonathan Corbet <corbet@lwn.net> >> > Cc: linux-doc@vger.kernel.org >> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org> >> > Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com> >> > --- >> > Documentation/driver-api/pm/devices.rst | 8 ++++++++ >> > include/linux/kernel.h | 18 ++++++++++++++++-- >> > 2 files changed, 24 insertions(+), 2 deletions(-) >> > >> > --- linux-next-20250819.orig/include/linux/kernel.h >> > +++ linux-next-20250819/include/linux/kernel.h >> > @@ -164,8 +164,22 @@ extern int root_mountflags; >> > >> > extern bool early_boot_irqs_disabled; >> > >> > -/* >> > - * Values used for system_state. Ordering of the states must not be changed >> > +/** >> > + * DOC: General system_states available for drivers >> > + * >> > + * enum system_states - Values used for system_state. >> > + * >> > + * * @SYSTEM_BOOTING: %0, no init needed >> > + * * @SYSTEM_SCHEDULING: system is ready for scheduling; OK to use RCU >> > + * * @SYSTEM_FREEING_INITMEM: system is freeing all of initmem; almost running >> > + * * @SYSTEM_RUNNING: system is up and running >> > + * * @SYSTEM_HALT: system entered clean system halt state >> > + * * @SYSTEM_POWER_OFF: system entered shutdown/clean power off state >> > + * * @SYSTEM_RESTART: system entered emergency power off or normal restart >> > + * * @SYSTEM_SUSPEND: system entered suspend or hibernate state >> > + * >> > + * Note: >> > + * Ordering of the states must not be changed >> > * as code checks for <, <=, >, >= STATE. >> > */ >> > extern enum system_states { >> > --- linux-next-20250819.orig/Documentation/driver-api/pm/devices.rst >> > +++ linux-next-20250819/Documentation/driver-api/pm/devices.rst >> > @@ -241,6 +241,14 @@ before reactivating its class I/O queues >> > More power-aware drivers might prepare the devices for triggering system wakeup >> > events. >> > >> > +System states available for drivers >> > +----------------------------------- >> > + >> > +These system states are available for drivers to help them determine how to >> > +handle state transitions. >> > + >> > +.. kernel-doc:: include/linux/kernel.h >> > + :doc: General system_states available for drivers >> > >> > Call Sequence Guarantees >> > ------------------------ >> > >> > > If the problem is with "extern" before enum, fixing kdoc be > fairly trivial. The non-trivial part is deciding whether you're documenting the enum type or the variable. Both are equally valid options. BR, Jani. > > If you see: > > def dump_function(self, ln, prototype): > > # Prefixes that would be removed > sub_prefixes = [ > ... > (r"^extern +", "", 0), > ... > } > > for search, sub, flags in sub_prefixes: > prototype = KernRe(search, flags).sub(sub, prototype) > > > we need something similar to that at: > def dump_enum(self, ln, proto): > > alternatively, we could use a simpler approach modifying adding a > non-group optional match for "extern". E.g: > > - r = KernRe(r'enum\s+(\w*)\s*\{(.*)\}') > + r = KernRe(r'(?:extern\s+)?enum\s+(\w*)\s*\{(.*)\}') > > (untested) > > Regards, > Mauro > > > Thanks, > Mauro -- Jani Nikula, Intel
On Fri, 2025-09-05 at 16:06 +0300, Jani Nikula wrote: > On Fri, 05 Sep 2025, Mauro Carvalho Chehab > <mchehab+huawei@kernel.org> wrote: > > Em Fri, 05 Sep 2025 12:02:37 +0300 > > Jani Nikula <jani.nikula@linux.intel.com> escreveu: > > > > > On Wed, 03 Sep 2025, Randy Dunlap <rdunlap@infradead.org> wrote: [...] > > > > +++ linux-next-20250819/Documentation/driver-api/pm/devices.rst > > > > @@ -241,6 +241,14 @@ before reactivating its class I/O queues > > > > More power-aware drivers might prepare the devices for > > > > triggering system wakeup > > > > events. > > > > > > > > +System states available for drivers > > > > +----------------------------------- > > > > + > > > > +These system states are available for drivers to help them > > > > determine how to > > > > +handle state transitions. > > > > + > > > > +.. kernel-doc:: include/linux/kernel.h > > > > + :doc: General system_states available for drivers > > > > > > > > Call Sequence Guarantees > > > > ------------------------ > > > > > > > > > > > If the problem is with "extern" before enum, fixing kdoc be > > fairly trivial. > > The non-trivial part is deciding whether you're documenting the enum > type or the variable. Both are equally valid options. If you're building a system that's easy to maintain, it shouldn't be at all non trivial: you add the documentation where someone adding a new state would find it. i.e. on the enum. If you document the variable, no-one adding a new state would likely look at it. I get that in this case they're one after the other, but think about the precedent for when they're not. Regards, James
On Sat, 06 Sep 2025, James Bottomley <James.Bottomley@HansenPartnership.com> wrote: > On Fri, 2025-09-05 at 16:06 +0300, Jani Nikula wrote: >> On Fri, 05 Sep 2025, Mauro Carvalho Chehab >> <mchehab+huawei@kernel.org> wrote: >> > Em Fri, 05 Sep 2025 12:02:37 +0300 >> > Jani Nikula <jani.nikula@linux.intel.com> escreveu: >> > >> > > On Wed, 03 Sep 2025, Randy Dunlap <rdunlap@infradead.org> wrote: > [...] >> > > > +++ linux-next-20250819/Documentation/driver-api/pm/devices.rst >> > > > @@ -241,6 +241,14 @@ before reactivating its class I/O queues >> > > > More power-aware drivers might prepare the devices for >> > > > triggering system wakeup >> > > > events. >> > > > >> > > > +System states available for drivers >> > > > +----------------------------------- >> > > > + >> > > > +These system states are available for drivers to help them >> > > > determine how to >> > > > +handle state transitions. >> > > > + >> > > > +.. kernel-doc:: include/linux/kernel.h >> > > > + :doc: General system_states available for drivers >> > > > >> > > > Call Sequence Guarantees >> > > > ------------------------ >> > > > >> > > >> > >> > If the problem is with "extern" before enum, fixing kdoc be >> > fairly trivial. >> >> The non-trivial part is deciding whether you're documenting the enum >> type or the variable. Both are equally valid options. > > If you're building a system that's easy to maintain, it shouldn't be at > all non trivial: you add the documentation where someone adding a new > state would find it. i.e. on the enum. If you document the variable, > no-one adding a new state would likely look at it. I get that in this > case they're one after the other, but think about the precedent for > when they're not. Ah, I meant deciding what the *tool* should do with the documentation when the type and the variable are bundled together like here. BR, Jani. -- Jani Nikula, Intel
On Fri, Sep 05, 2025 at 04:06:31PM +0300, Jani Nikula wrote: > On Fri, 05 Sep 2025, Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > > Em Fri, 05 Sep 2025 12:02:37 +0300 > > Jani Nikula <jani.nikula@linux.intel.com> escreveu: > > > >> On Wed, 03 Sep 2025, Randy Dunlap <rdunlap@infradead.org> wrote: > >> > Provide some basic comments about the system_states and what they imply. > >> > Also convert the comments to kernel-doc format. > >> > > >> > However, kernel-doc does not support kernel-doc notation on extern > >> > struct/union/typedef/enum/etc. So I made this a DOC: block so that > >> > I can use (insert) it into a Documentation (.rst) file and have it > >> > look decent. > >> > >> The simple workaround is to separate the enum type and the variable: > >> > >> /** > >> * kernel-doc for the enum > >> */ > >> enum system_states { > >> ... > >> }; > >> > >> /** > >> * kernel-doc for the variable > >> */ > >> extern enum system_states system_state; > >> > >> BR, > >> Jani. > >> > >> > > >> > Signed-off-by: Randy Dunlap <rdunlap@infradead.org> > >> > Acked-by: Rafael J. Wysocki <rafael@kernel.org> # v1 > >> > --- > >> > v2: add Rafael's Ack. > >> > v3: add Andrew > >> > v4: add DOC: so that this DOC: block can be used in Documentation/ > >> > add Greg K-H > >> > add Jon Corbet, Mauro Chehab, & linux-doc > >> > > >> > Cc: Andrew Morton <akpm@linux-foundation.org> > >> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > >> > Cc: "Rafael J. Wysocki" <rafael@kernel.org> > >> > Cc: Pavel Machek <pavel@ucw.cz> > >> > Cc: Len Brown <len.brown@intel.com> > >> > Cc: linux-pm@vger.kernel.org > >> > Cc: Jonathan Corbet <corbet@lwn.net> > >> > Cc: linux-doc@vger.kernel.org > >> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org> > >> > Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com> > >> > --- > >> > Documentation/driver-api/pm/devices.rst | 8 ++++++++ > >> > include/linux/kernel.h | 18 ++++++++++++++++-- > >> > 2 files changed, 24 insertions(+), 2 deletions(-) > >> > > >> > --- linux-next-20250819.orig/include/linux/kernel.h > >> > +++ linux-next-20250819/include/linux/kernel.h > >> > @@ -164,8 +164,22 @@ extern int root_mountflags; > >> > > >> > extern bool early_boot_irqs_disabled; > >> > > >> > -/* > >> > - * Values used for system_state. Ordering of the states must not be changed > >> > +/** > >> > + * DOC: General system_states available for drivers > >> > + * > >> > + * enum system_states - Values used for system_state. > >> > + * > >> > + * * @SYSTEM_BOOTING: %0, no init needed > >> > + * * @SYSTEM_SCHEDULING: system is ready for scheduling; OK to use RCU > >> > + * * @SYSTEM_FREEING_INITMEM: system is freeing all of initmem; almost running > >> > + * * @SYSTEM_RUNNING: system is up and running > >> > + * * @SYSTEM_HALT: system entered clean system halt state > >> > + * * @SYSTEM_POWER_OFF: system entered shutdown/clean power off state > >> > + * * @SYSTEM_RESTART: system entered emergency power off or normal restart > >> > + * * @SYSTEM_SUSPEND: system entered suspend or hibernate state > >> > + * > >> > + * Note: > >> > + * Ordering of the states must not be changed > >> > * as code checks for <, <=, >, >= STATE. > >> > */ > >> > extern enum system_states { > >> > --- linux-next-20250819.orig/Documentation/driver-api/pm/devices.rst > >> > +++ linux-next-20250819/Documentation/driver-api/pm/devices.rst > >> > @@ -241,6 +241,14 @@ before reactivating its class I/O queues > >> > More power-aware drivers might prepare the devices for triggering system wakeup > >> > events. > >> > > >> > +System states available for drivers > >> > +----------------------------------- > >> > + > >> > +These system states are available for drivers to help them determine how to > >> > +handle state transitions. > >> > + > >> > +.. kernel-doc:: include/linux/kernel.h > >> > + :doc: General system_states available for drivers > >> > > >> > Call Sequence Guarantees > >> > ------------------------ > >> > > >> > > > > If the problem is with "extern" before enum, fixing kdoc be > > fairly trivial. > > The non-trivial part is deciding whether you're documenting the enum > type or the variable. Both are equally valid options. True. I'd say that, if the variable is under EXPORT_SYMBOL macros, it should be documented. The same applies to the enum: if it is used by kAPI, it should also be documented. So, yeah, I suspect that on most kAPI cases, the best is to split, having documentation for both. > > BR, > Jani. > > > > > If you see: > > > > def dump_function(self, ln, prototype): > > > > # Prefixes that would be removed > > sub_prefixes = [ > > ... > > (r"^extern +", "", 0), > > ... > > } > > > > for search, sub, flags in sub_prefixes: > > prototype = KernRe(search, flags).sub(sub, prototype) > > > > > > we need something similar to that at: > > def dump_enum(self, ln, proto): > > > > alternatively, we could use a simpler approach modifying adding a > > non-group optional match for "extern". E.g: > > > > - r = KernRe(r'enum\s+(\w*)\s*\{(.*)\}') > > + r = KernRe(r'(?:extern\s+)?enum\s+(\w*)\s*\{(.*)\}') > > > > (untested) > > > > Regards, > > Mauro > > > > > > Thanks, > > Mauro > > -- > Jani Nikula, Intel -- Thanks, Mauro
Hi, On 9/5/25 6:38 AM, Mauro Carvalho Chehab wrote: > On Fri, Sep 05, 2025 at 04:06:31PM +0300, Jani Nikula wrote: >> On Fri, 05 Sep 2025, Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: >>> Em Fri, 05 Sep 2025 12:02:37 +0300 >>> Jani Nikula <jani.nikula@linux.intel.com> escreveu: >>> >>>> On Wed, 03 Sep 2025, Randy Dunlap <rdunlap@infradead.org> wrote: >>>>> Provide some basic comments about the system_states and what they imply. >>>>> Also convert the comments to kernel-doc format. >>>>> >>>>> However, kernel-doc does not support kernel-doc notation on extern >>>>> struct/union/typedef/enum/etc. So I made this a DOC: block so that >>>>> I can use (insert) it into a Documentation (.rst) file and have it >>>>> look decent. >>>> >>>> The simple workaround is to separate the enum type and the variable: >>>> >>>> /** >>>> * kernel-doc for the enum >>>> */ >>>> enum system_states { >>>> ... >>>> }; >>>> >>>> /** >>>> * kernel-doc for the variable >>>> */ >>>> extern enum system_states system_state; >>>> >>>> BR, >>>> Jani. >>>> >>>>> >>>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org> >>>>> Acked-by: Rafael J. Wysocki <rafael@kernel.org> # v1 >>>>> --- [snip] >>> If the problem is with "extern" before enum, fixing kdoc be >>> fairly trivial. >> >> The non-trivial part is deciding whether you're documenting the enum >> type or the variable. Both are equally valid options. > > True. > > I'd say that, if the variable is under EXPORT_SYMBOL macros, it > should be documented. Do you mean documented with kernel-doc? How do we do that? I was under the impression that we don't currently have a way to use kernel-doc for variables (definitions, only for declarations). > The same applies to the enum: if it is used by kAPI, it should > also be documented. > > So, yeah, I suspect that on most kAPI cases, the best is to split, > having documentation for both. > >> >> BR, >> Jani. >> >>> >>> If you see: >>> >>> def dump_function(self, ln, prototype): >>> >>> # Prefixes that would be removed >>> sub_prefixes = [ >>> ... >>> (r"^extern +", "", 0), >>> ... >>> } >>> >>> for search, sub, flags in sub_prefixes: >>> prototype = KernRe(search, flags).sub(sub, prototype) >>> >>> >>> we need something similar to that at: >>> def dump_enum(self, ln, proto): >>> >>> alternatively, we could use a simpler approach modifying adding a >>> non-group optional match for "extern". E.g: >>> >>> - r = KernRe(r'enum\s+(\w*)\s*\{(.*)\}') >>> + r = KernRe(r'(?:extern\s+)?enum\s+(\w*)\s*\{(.*)\}') >>> >>> (untested) Yes, this might work. It might also lead to documenting more than was intended. It seems safer just to do the enum declaration and variable definition split light Jani suggested. Thanks. -- ~Randy
Em Fri, 5 Sep 2025 15:07:31 -0700 Randy Dunlap <rdunlap@infradead.org> escreveu: > Hi, > > On 9/5/25 6:38 AM, Mauro Carvalho Chehab wrote: > > On Fri, Sep 05, 2025 at 04:06:31PM +0300, Jani Nikula wrote: > >> On Fri, 05 Sep 2025, Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > >>> Em Fri, 05 Sep 2025 12:02:37 +0300 > >>> Jani Nikula <jani.nikula@linux.intel.com> escreveu: > >>> > >>>> On Wed, 03 Sep 2025, Randy Dunlap <rdunlap@infradead.org> wrote: > >>>>> Provide some basic comments about the system_states and what they imply. > >>>>> Also convert the comments to kernel-doc format. > >>>>> > >>>>> However, kernel-doc does not support kernel-doc notation on extern > >>>>> struct/union/typedef/enum/etc. So I made this a DOC: block so that > >>>>> I can use (insert) it into a Documentation (.rst) file and have it > >>>>> look decent. > >>>> > >>>> The simple workaround is to separate the enum type and the variable: > >>>> > >>>> /** > >>>> * kernel-doc for the enum > >>>> */ > >>>> enum system_states { > >>>> ... > >>>> }; > >>>> > >>>> /** > >>>> * kernel-doc for the variable > >>>> */ > >>>> extern enum system_states system_state; > >>>> > >>>> BR, > >>>> Jani. > >>>> > >>>>> > >>>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org> > >>>>> Acked-by: Rafael J. Wysocki <rafael@kernel.org> # v1 > >>>>> --- > > [snip] > >>> If the problem is with "extern" before enum, fixing kdoc be > >>> fairly trivial. > >> > >> The non-trivial part is deciding whether you're documenting the enum > >> type or the variable. Both are equally valid options. > > > > True. > > > > I'd say that, if the variable is under EXPORT_SYMBOL macros, it > > should be documented. > > Do you mean documented with kernel-doc? How do we do that? > I was under the impression that we don't currently have a way to > use kernel-doc for variables (definitions, only for declarations). No, but it shouldn't be hard to add something like: /** * global system_state - stores system state * <some description> */ extern enum system_states system_state; and place a dump_global() function at kdoc parser. Ok, one might use DOC:, but IMHO the best is to have a proper handler for it that would automatically pick the type. > > The same applies to the enum: if it is used by kAPI, it should > > also be documented. > > > > So, yeah, I suspect that on most kAPI cases, the best is to split, > > having documentation for both. > > > >> > >> BR, > >> Jani. > >> > >>> > >>> If you see: > >>> > >>> def dump_function(self, ln, prototype): > >>> > >>> # Prefixes that would be removed > >>> sub_prefixes = [ > >>> ... > >>> (r"^extern +", "", 0), > >>> ... > >>> } > >>> > >>> for search, sub, flags in sub_prefixes: > >>> prototype = KernRe(search, flags).sub(sub, prototype) > >>> > >>> > >>> we need something similar to that at: > >>> def dump_enum(self, ln, proto): > >>> > >>> alternatively, we could use a simpler approach modifying adding a > >>> non-group optional match for "extern". E.g: > >>> > >>> - r = KernRe(r'enum\s+(\w*)\s*\{(.*)\}') > >>> + r = KernRe(r'(?:extern\s+)?enum\s+(\w*)\s*\{(.*)\}') > >>> > >>> (untested) > > Yes, this might work. It might also lead to documenting more than > was intended. It seems safer just to do the enum declaration and > variable definition split light Jani suggested. Yeah, the extra var at the end may be problematic, although it wouldn't be hard to drop or parse it, like: /** * enum system_states - Values used for system_state. * * @SYSTEM_BOOTING: %0, no init needed * @SYSTEM_SCHEDULING: system is ready for scheduling; OK to use RCU * @SYSTEM_FREEING_INITMEM: system is freeing all of initmem; almost running * @SYSTEM_RUNNING: system is up and running * @SYSTEM_HALT: system entered clean system halt state * @SYSTEM_POWER_OFF: system entered shutdown/clean power off state * @SYSTEM_RESTART: system entered emergency power off or normal restart * @SYSTEM_SUSPEND: system entered suspend or hibernate state * * @system_state: global var describing the current system state. * * Note: * Ordering of the states must not be changed * as code checks for <, <=, >, >= STATE. */ extern enum system_states { SYSTEM_BOOTING, SYSTEM_SCHEDULING, SYSTEM_FREEING_INITMEM, SYSTEM_RUNNING, SYSTEM_HALT, SYSTEM_POWER_OFF, SYSTEM_RESTART, SYSTEM_SUSPEND, } system_state; making it create two separate entries: one for the enum, and another one for the global var - internally calling dump_global() for the var. Thanks, Mauro
On 9/6/25 1:56 AM, Mauro Carvalho Chehab wrote: > Em Fri, 5 Sep 2025 15:07:31 -0700 > Randy Dunlap <rdunlap@infradead.org> escreveu: > >> Hi, >> >> On 9/5/25 6:38 AM, Mauro Carvalho Chehab wrote: >>> On Fri, Sep 05, 2025 at 04:06:31PM +0300, Jani Nikula wrote: >>>> On Fri, 05 Sep 2025, Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: >>>>> Em Fri, 05 Sep 2025 12:02:37 +0300 >>>>> Jani Nikula <jani.nikula@linux.intel.com> escreveu: >>>>> >>>>>> On Wed, 03 Sep 2025, Randy Dunlap <rdunlap@infradead.org> wrote: >>>>>>> Provide some basic comments about the system_states and what they imply. >>>>>>> Also convert the comments to kernel-doc format. >>>>>>> >>>>>>> However, kernel-doc does not support kernel-doc notation on extern >>>>>>> struct/union/typedef/enum/etc. So I made this a DOC: block so that >>>>>>> I can use (insert) it into a Documentation (.rst) file and have it >>>>>>> look decent. >>>>>> >>>>>> The simple workaround is to separate the enum type and the variable: >>>>>> >>>>>> /** >>>>>> * kernel-doc for the enum >>>>>> */ >>>>>> enum system_states { >>>>>> ... >>>>>> }; >>>>>> >>>>>> /** >>>>>> * kernel-doc for the variable >>>>>> */ >>>>>> extern enum system_states system_state; >>>>>> >>>>>> BR, >>>>>> Jani. >>>>>> >>>>>>> >>>>>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org> >>>>>>> Acked-by: Rafael J. Wysocki <rafael@kernel.org> # v1 >>>>>>> --- >> >> [snip] >>>>> If the problem is with "extern" before enum, fixing kdoc be >>>>> fairly trivial. >>>> >>>> The non-trivial part is deciding whether you're documenting the enum >>>> type or the variable. Both are equally valid options. >>> >>> True. >>> >>> I'd say that, if the variable is under EXPORT_SYMBOL macros, it >>> should be documented. >> >> Do you mean documented with kernel-doc? How do we do that? >> I was under the impression that we don't currently have a way to >> use kernel-doc for variables (definitions, only for declarations). > > No, but it shouldn't be hard to add something like: > > /** > * global system_state - stores system state > * <some description> > */ > extern enum system_states system_state; > > and place a dump_global() function at kdoc parser. Ok, one might use > DOC:, but IMHO the best is to have a proper handler for it that would > automatically pick the type. Nitpick, I would s/global/var/. But I won't complain about "global". More importantly, I have seen several patches where people try to document a variable, so it seems like something that we should support at some point. thanks. -- ~Randy
Em Sat, 6 Sep 2025 10:13:23 -0700 Randy Dunlap <rdunlap@infradead.org> escreveu: > On 9/6/25 1:56 AM, Mauro Carvalho Chehab wrote: > > Em Fri, 5 Sep 2025 15:07:31 -0700 > > Randy Dunlap <rdunlap@infradead.org> escreveu: > > > >> Hi, > >> > >> On 9/5/25 6:38 AM, Mauro Carvalho Chehab wrote: > >>> On Fri, Sep 05, 2025 at 04:06:31PM +0300, Jani Nikula wrote: > >>>> On Fri, 05 Sep 2025, Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > >>>>> Em Fri, 05 Sep 2025 12:02:37 +0300 > >>>>> Jani Nikula <jani.nikula@linux.intel.com> escreveu: > >>>>> > >>>>>> On Wed, 03 Sep 2025, Randy Dunlap <rdunlap@infradead.org> wrote: > >>>>>>> Provide some basic comments about the system_states and what they imply. > >>>>>>> Also convert the comments to kernel-doc format. > >>>>>>> > >>>>>>> However, kernel-doc does not support kernel-doc notation on extern > >>>>>>> struct/union/typedef/enum/etc. So I made this a DOC: block so that > >>>>>>> I can use (insert) it into a Documentation (.rst) file and have it > >>>>>>> look decent. > >>>>>> > >>>>>> The simple workaround is to separate the enum type and the variable: > >>>>>> > >>>>>> /** > >>>>>> * kernel-doc for the enum > >>>>>> */ > >>>>>> enum system_states { > >>>>>> ... > >>>>>> }; > >>>>>> > >>>>>> /** > >>>>>> * kernel-doc for the variable > >>>>>> */ > >>>>>> extern enum system_states system_state; > >>>>>> > >>>>>> BR, > >>>>>> Jani. > >>>>>> > >>>>>>> > >>>>>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org> > >>>>>>> Acked-by: Rafael J. Wysocki <rafael@kernel.org> # v1 > >>>>>>> --- > >> > >> [snip] > >>>>> If the problem is with "extern" before enum, fixing kdoc be > >>>>> fairly trivial. > >>>> > >>>> The non-trivial part is deciding whether you're documenting the enum > >>>> type or the variable. Both are equally valid options. > >>> > >>> True. > >>> > >>> I'd say that, if the variable is under EXPORT_SYMBOL macros, it > >>> should be documented. > >> > >> Do you mean documented with kernel-doc? How do we do that? > >> I was under the impression that we don't currently have a way to > >> use kernel-doc for variables (definitions, only for declarations). > > > > No, but it shouldn't be hard to add something like: > > > > /** > > * global system_state - stores system state > > * <some description> > > */ > > extern enum system_states system_state; > > > > and place a dump_global() function at kdoc parser. Ok, one might use > > DOC:, but IMHO the best is to have a proper handler for it that would > > automatically pick the type. > > Nitpick, I would s/global/var/. But I won't complain about "global". > > More importantly, I have seen several patches where people try to document a > variable, so it seems like something that we should support at some point. As we're talking about possible new kernel-doc features, there's something that IMHO we should consider implementing: define groups. For instance, media videodev2.h uapi has several ones like this (and this pattern is quite common on other places - either using BIT() or not): /* Values for 'capabilities' field */ #define V4L2_CAP_VIDEO_CAPTURE 0x00000001 /* Is a video capture device */ #define V4L2_CAP_VIDEO_OUTPUT 0x00000002 /* Is a video output device */ #define V4L2_CAP_VIDEO_OVERLAY 0x00000004 /* Can do video overlay */ #define V4L2_CAP_VBI_CAPTURE 0x00000010 /* Is a raw VBI capture device */ #define V4L2_CAP_VBI_OUTPUT 0x00000020 /* Is a raw VBI output device */ #define V4L2_CAP_SLICED_VBI_CAPTURE 0x00000040 /* Is a sliced VBI capture device */ #define V4L2_CAP_SLICED_VBI_OUTPUT 0x00000080 /* Is a sliced VBI output device */ #define V4L2_CAP_RDS_CAPTURE 0x00000100 /* RDS data capture */ #define V4L2_CAP_VIDEO_OUTPUT_OVERLAY 0x00000200 /* Can do video output overlay */ #define V4L2_CAP_HW_FREQ_SEEK 0x00000400 /* Can do hardware frequency seek */ #define V4L2_CAP_RDS_OUTPUT 0x00000800 /* Is an RDS encoder */ We could use something like this: /** * defgroup - Values for 'capabilities' field * * @V4L2_CAP_VIDEO_CAPTURE: Is a video capture device * @V4L2_CAP_VIDEO_OUTPUT: Is a video output device * @V4L2_CAP_VIDEO_OVERLAY: Can do video overlay * @V4L2_CAP_VBI_CAPTURE: Is a raw VBI capture device * @V4L2_CAP_VBI_OUTPUT: Is a raw VBI output device * @V4L2_CAP_SLICED_VBI_CAPTURE: Is a sliced VBI capture device * @V4L2_CAP_SLICED_VBI_OUTPUT: Is a sliced VBI output device * @V4L2_CAP_RDS_CAPTURE: RDS data capture * @V4L2_CAP_VIDEO_OUTPUT_OVERLAY: Can do video output overlay * @V4L2_CAP_HW_FREQ_SEEK: Can do hardware frequency seek * @V4L2_CAP_RDS_OUTPUT: Is an RDS encoder */ #define V4L2_CAP_VIDEO_CAPTURE 0x00000001 #define V4L2_CAP_VIDEO_OUTPUT 0x00000002 #define V4L2_CAP_VIDEO_OVERLAY 0x00000004 #define V4L2_CAP_VBI_CAPTURE 0x00000010 #define V4L2_CAP_VBI_OUTPUT 0x00000020 #define V4L2_CAP_SLICED_VBI_CAPTURE 0x00000040 #define V4L2_CAP_SLICED_VBI_OUTPUT 0x00000080 #define V4L2_CAP_RDS_CAPTURE 0x00000100 #define V4L2_CAP_VIDEO_OUTPUT_OVERLAY 0x00000200 #define V4L2_CAP_HW_FREQ_SEEK 0x00000400 #define V4L2_CAP_RDS_OUTPUT 0x00000800 I suspect this is easier said than done, but there are lots of similar stuff at the Kernel. On some cases, it is easier to use enums, but on others like here, it probably keeps sense to keep them a defines. Thanks, Mauro
Em Sat, 6 Sep 2025 10:13:23 -0700 Randy Dunlap <rdunlap@infradead.org> escreveu: > On 9/6/25 1:56 AM, Mauro Carvalho Chehab wrote: > > Em Fri, 5 Sep 2025 15:07:31 -0700 > > Randy Dunlap <rdunlap@infradead.org> escreveu: > > > >> Hi, > >> > >> On 9/5/25 6:38 AM, Mauro Carvalho Chehab wrote: > >>> On Fri, Sep 05, 2025 at 04:06:31PM +0300, Jani Nikula wrote: > >>>> On Fri, 05 Sep 2025, Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > >>>>> Em Fri, 05 Sep 2025 12:02:37 +0300 > >>>>> Jani Nikula <jani.nikula@linux.intel.com> escreveu: > >>>>> > >>>>>> On Wed, 03 Sep 2025, Randy Dunlap <rdunlap@infradead.org> wrote: > >>>>>>> Provide some basic comments about the system_states and what they imply. > >>>>>>> Also convert the comments to kernel-doc format. > >>>>>>> > >>>>>>> However, kernel-doc does not support kernel-doc notation on extern > >>>>>>> struct/union/typedef/enum/etc. So I made this a DOC: block so that > >>>>>>> I can use (insert) it into a Documentation (.rst) file and have it > >>>>>>> look decent. > >>>>>> > >>>>>> The simple workaround is to separate the enum type and the variable: > >>>>>> > >>>>>> /** > >>>>>> * kernel-doc for the enum > >>>>>> */ > >>>>>> enum system_states { > >>>>>> ... > >>>>>> }; > >>>>>> > >>>>>> /** > >>>>>> * kernel-doc for the variable > >>>>>> */ > >>>>>> extern enum system_states system_state; > >>>>>> > >>>>>> BR, > >>>>>> Jani. > >>>>>> > >>>>>>> > >>>>>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org> > >>>>>>> Acked-by: Rafael J. Wysocki <rafael@kernel.org> # v1 > >>>>>>> --- > >> > >> [snip] > >>>>> If the problem is with "extern" before enum, fixing kdoc be > >>>>> fairly trivial. > >>>> > >>>> The non-trivial part is deciding whether you're documenting the enum > >>>> type or the variable. Both are equally valid options. > >>> > >>> True. > >>> > >>> I'd say that, if the variable is under EXPORT_SYMBOL macros, it > >>> should be documented. > >> > >> Do you mean documented with kernel-doc? How do we do that? > >> I was under the impression that we don't currently have a way to > >> use kernel-doc for variables (definitions, only for declarations). > > > > No, but it shouldn't be hard to add something like: > > > > /** > > * global system_state - stores system state > > * <some description> > > */ > > extern enum system_states system_state; > > > > and place a dump_global() function at kdoc parser. Ok, one might use > > DOC:, but IMHO the best is to have a proper handler for it that would > > automatically pick the type. > > Nitpick, I would s/global/var/. But I won't complain about "global". Either way works for me. Yet, I would expect it to be used only for global variables, as documenting local ones using kernel-doc is probably not what we expect inside Kernel documentation. So, naming it "global" may help. > More importantly, I have seen several patches where people try to document a > variable, so it seems like something that we should support at some point. Agreed. Adding a parsing rule for the variable doesn't sound hard, as they follow, in principle, this regex(*): OPTIONAL_ATTRIBS = [" "extern" ] optional = "|".join(OPTIONAL_ATTRIBS) "^(?:extern\s+)?(\w.*)\s+([\w\_]+)(?:#.*)$" (*) eventually, some might have extra attributes, but we can start with a simpler regex, adding a more complex parser if/when needed. I added at the end a one-minute hacky prototype I wrote, completely untested and incomplete. Thanks, Mauro The following code snippet is incomplete, not tested, and requires more work, like placing global vars at the beginning. Feel free to pick it and use it to produce a GPL code to the Kernel. Note that this is just the parsing part. for it to work, you also need to modify scripts/lib/kdoc/kdoc_output.py, which contains how this will be output at rst and man formats. diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py index 574972e1f741..b3ffaa541e9d 100644 --- a/scripts/lib/kdoc/kdoc_parser.py +++ b/scripts/lib/kdoc/kdoc_parser.py @@ -886,6 +886,25 @@ class KernelDoc: self.output_declaration('enum', declaration_name, purpose=self.entry.declaration_purpose) + def dump_global(self, ln, proto): + # TODO: move this to the beginning of this file + VAR_ATTRIBS = [ + "extern", + ] + OPTIONAL_VAR_ATTR = "^(?:" + "|".join(VAR_ATTRIBS) + ")?" + + type_var = KernRe(OPTIONAL_VAR_ATTR + r"(\w.*)\s+([\w_]+)(?:#.*)?$") + + # function logic starts here: + + if type_var.match(proto): + declaration_name = r.group(2) + var_type = r.group(1) + + self.output_declaration('global', declaration_name, + var_type=var_type, + purpose=self.entry.declaration_purpose) + def dump_declaration(self, ln, prototype): """ Stores a data declaration inside self.entries array. @@ -897,6 +916,8 @@ class KernelDoc: self.dump_typedef(ln, prototype) elif self.entry.decl_type in ["union", "struct"]: self.dump_struct(ln, prototype) + elif self.entry.decl_type == "global"]: + self.dump_global(ln, prototype) else: # This would be a bug self.emit_message(ln, f'Unknown declaration type: {self.entry.decl_type}')
Em Sat, 6 Sep 2025 23:30:28 +0200 Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu: > Em Sat, 6 Sep 2025 10:13:23 -0700 > Randy Dunlap <rdunlap@infradead.org> escreveu: > > > On 9/6/25 1:56 AM, Mauro Carvalho Chehab wrote: > > > Em Fri, 5 Sep 2025 15:07:31 -0700 > > > Randy Dunlap <rdunlap@infradead.org> escreveu: > > > > > >> Hi, > > >> > > >> On 9/5/25 6:38 AM, Mauro Carvalho Chehab wrote: > > >>> On Fri, Sep 05, 2025 at 04:06:31PM +0300, Jani Nikula wrote: > > >>>> On Fri, 05 Sep 2025, Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > > >>>>> Em Fri, 05 Sep 2025 12:02:37 +0300 > > >>>>> Jani Nikula <jani.nikula@linux.intel.com> escreveu: > > >>>>> > > >>>>>> On Wed, 03 Sep 2025, Randy Dunlap <rdunlap@infradead.org> wrote: > > >>>>>>> Provide some basic comments about the system_states and what they imply. > > >>>>>>> Also convert the comments to kernel-doc format. > > >>>>>>> > > >>>>>>> However, kernel-doc does not support kernel-doc notation on extern > > >>>>>>> struct/union/typedef/enum/etc. So I made this a DOC: block so that > > >>>>>>> I can use (insert) it into a Documentation (.rst) file and have it > > >>>>>>> look decent. > > >>>>>> > > >>>>>> The simple workaround is to separate the enum type and the variable: > > >>>>>> > > >>>>>> /** > > >>>>>> * kernel-doc for the enum > > >>>>>> */ > > >>>>>> enum system_states { > > >>>>>> ... > > >>>>>> }; > > >>>>>> > > >>>>>> /** > > >>>>>> * kernel-doc for the variable > > >>>>>> */ > > >>>>>> extern enum system_states system_state; > > >>>>>> > > >>>>>> BR, > > >>>>>> Jani. > > >>>>>> > > >>>>>>> > > >>>>>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org> > > >>>>>>> Acked-by: Rafael J. Wysocki <rafael@kernel.org> # v1 > > >>>>>>> --- > > >> > > >> [snip] > > >>>>> If the problem is with "extern" before enum, fixing kdoc be > > >>>>> fairly trivial. > > >>>> > > >>>> The non-trivial part is deciding whether you're documenting the enum > > >>>> type or the variable. Both are equally valid options. > > >>> > > >>> True. > > >>> > > >>> I'd say that, if the variable is under EXPORT_SYMBOL macros, it > > >>> should be documented. > > >> > > >> Do you mean documented with kernel-doc? How do we do that? > > >> I was under the impression that we don't currently have a way to > > >> use kernel-doc for variables (definitions, only for declarations). > > > > > > No, but it shouldn't be hard to add something like: > > > > > > /** > > > * global system_state - stores system state > > > * <some description> > > > */ > > > extern enum system_states system_state; > > > > > > and place a dump_global() function at kdoc parser. Ok, one might use > > > DOC:, but IMHO the best is to have a proper handler for it that would > > > automatically pick the type. > > > > Nitpick, I would s/global/var/. But I won't complain about "global". > > Either way works for me. Yet, I would expect it to be used only for > global variables, as documenting local ones using kernel-doc is > probably not what we expect inside Kernel documentation. So, naming it > "global" may help. > > > More importantly, I have seen several patches where people try to document a > > variable, so it seems like something that we should support at some point. > > Agreed. > > Adding a parsing rule for the variable doesn't sound hard, as they follow, > in principle, this regex(*): > > OPTIONAL_ATTRIBS = [" > "extern" > ] > > optional = "|".join(OPTIONAL_ATTRIBS) > > "^(?:extern\s+)?(\w.*)\s+([\w\_]+)(?:#.*)$" > > (*) eventually, some might have extra attributes, but we can > start with a simpler regex, adding a more complex parser if/when > needed. > > I added at the end a one-minute hacky prototype I wrote, completely > untested and incomplete. Heh, it doesn't hurt spending 15 mins or so to write something that actually works and implement all functions. The example below produces: $ ./scripts/kernel-doc include/media/tuner-types.h ... .. c:var:: tuners list of tuners extern const struct tunertype tuners[]; .. c:var:: tuner_count number of known tuners $ ./scripts/kernel-doc include/media/tuner-types.h --man ... .TH "Kernel API" 9 "global tuner_count" "September 2025" "API Manual" LINUX .SH NAME extern unsigned const int tuner_count; \- number of known tuners .SH SYNOPSIS enum tuner_count { .SH "SEE ALSO" .PP Still not ready for kernel merge (plus I placed bogus descriptions for two externs from media that IMO doesn't make sense to document), but it has all needed steps for someone wanting to extend kernel-doc to see how to do it. Feel free to modify it - even renaming from "global" to "var" and submit upstream. Thanks, Mauro [PATCH] [RFC] kernel-doc: add support for handling global varaibles Add support for documenting global variables with kernel-doc. Please notice that this is mostly an example, as: 1. I'm documenting just two random variables from media, that doesn't make sense to actually be documented. I did it just to have some example and be able to test it; 2. the html output requires tweak: right now, it is just printing the entire variable prototype as-is, without any formatting, and witout making sense at the output Feel free to modify this patch to make it something actually mergeable. Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> diff --git a/include/media/tuner-types.h b/include/media/tuner-types.h index c79b773f750c..cf074beaeccc 100644 --- a/include/media/tuner-types.h +++ b/include/media/tuner-types.h @@ -199,7 +199,18 @@ struct tunertype { u8 *sleepdata; }; +/** + * global tuner - list of tuners + * + * List of all tuners defined via v4l2 API + */ extern const struct tunertype tuners[]; + +/** + * global tune_count - number of known tuners + * + * Number of tuners at @tuners list + */ extern unsigned const int tuner_count; #endif diff --git a/scripts/lib/kdoc/kdoc_output.py b/scripts/lib/kdoc/kdoc_output.py index 1eca9a918558..a58562bef35a 100644 --- a/scripts/lib/kdoc/kdoc_output.py +++ b/scripts/lib/kdoc/kdoc_output.py @@ -199,6 +199,10 @@ class OutputFormat: self.out_enum(fname, name, args) return self.data + if dtype == "global": + self.out_global(fname, name, args) + return self.data + if dtype == "typedef": self.out_typedef(fname, name, args) return self.data @@ -227,6 +231,9 @@ class OutputFormat: def out_enum(self, fname, name, args): """Outputs an enum""" + def out_global(self, fname, name, args): + """Outputs a global variable""" + def out_typedef(self, fname, name, args): """Outputs a typedef""" @@ -472,6 +479,20 @@ class RestFormat(OutputFormat): self.lineprefix = oldprefix self.out_section(args) + def out_global(self, fname, name, args): + oldprefix = self.lineprefix + ln = args.declaration_start_line + + self.data += f"\n\n.. c:var:: {name}\n\n" + + self.print_lineno(ln) + self.lineprefix = " " + self.output_highlight(args.get('purpose', '')) + self.data += "\n" + + # FIXME: better handle it + self.data += args.other_stuff["var_type"] + def out_typedef(self, fname, name, args): oldprefix = self.lineprefix @@ -772,6 +793,18 @@ class ManFormat(OutputFormat): self.data += f'.SH "{section}"' + "\n" self.output_highlight(text) + def out_global(self, fname, name, args): + out_name = self.arg_name(args, name) + prototype = args.other_stuff["var_type"] + + self.data += f'.TH "{self.modulename}" 9 "{out_name}" "{self.man_date}" "API Manual" LINUX' + "\n" + + self.data += ".SH NAME\n" + self.data += f"{prototype} \\- {args['purpose']}\n" + + self.data += ".SH SYNOPSIS\n" + self.data += f"enum {name}" + " {\n" + def out_typedef(self, fname, name, args): module = self.modulename purpose = args.get('purpose') diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py index 574972e1f741..e2a3f4574894 100644 --- a/scripts/lib/kdoc/kdoc_parser.py +++ b/scripts/lib/kdoc/kdoc_parser.py @@ -64,7 +64,7 @@ type_param = KernRe(r"@(\w*((\.\w+)|(->\w+))*(\.\.\.)?)", cache=False) # Tests for the beginning of a kerneldoc block in its various forms. # doc_block = doc_com + KernRe(r'DOC:\s*(.*)?', cache=False) -doc_begin_data = KernRe(r"^\s*\*?\s*(struct|union|enum|typedef)\b\s*(\w*)", cache = False) +doc_begin_data = KernRe(r"^\s*\*?\s*(struct|union|enum|typedef|global)\b\s*(\w*)", cache = False) doc_begin_func = KernRe(str(doc_com) + # initial " * ' r"(?:\w+\s*\*\s*)?" + # type (not captured) r'(?:define\s+)?' + # possible "define" (not captured) @@ -886,6 +886,27 @@ class KernelDoc: self.output_declaration('enum', declaration_name, purpose=self.entry.declaration_purpose) + def dump_global(self, ln, proto): + """ + Stores global variables that are part of kAPI. + """ + VAR_ATTRIBS = [ + "extern", + ] + OPTIONAL_VAR_ATTR = "^(?:" + "|".join(VAR_ATTRIBS) + ")?" + + r= KernRe(OPTIONAL_VAR_ATTR + r"(\w.*)\s+([\w_]+)[\d\]\[]*\s*;(?:#.*)?$") + if not r.match(proto): + self.emit_msg(ln,f"{proto}: can't parse variable") + return + + declaration_name = r.group(2) + var_type = r.group(0) + + self.output_declaration("global", declaration_name, + var_type=var_type, + purpose=self.entry.declaration_purpose) + def dump_declaration(self, ln, prototype): """ Stores a data declaration inside self.entries array. @@ -897,6 +918,8 @@ class KernelDoc: self.dump_typedef(ln, prototype) elif self.entry.decl_type in ["union", "struct"]: self.dump_struct(ln, prototype) + elif self.entry.decl_type == "global": + self.dump_global(ln, prototype) else: # This would be a bug self.emit_message(ln, f'Unknown declaration type: {self.entry.decl_type}')
Em Sun, 7 Sep 2025 15:35:47 +0200 Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu: > Em Sat, 6 Sep 2025 23:30:28 +0200 > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu: > > > Em Sat, 6 Sep 2025 10:13:23 -0700 > > Randy Dunlap <rdunlap@infradead.org> escreveu: > > > > > On 9/6/25 1:56 AM, Mauro Carvalho Chehab wrote: > > > > Em Fri, 5 Sep 2025 15:07:31 -0700 > > > > Randy Dunlap <rdunlap@infradead.org> escreveu: > > > > > > > >> Hi, > > > >> > > > >> On 9/5/25 6:38 AM, Mauro Carvalho Chehab wrote: > > > >>> On Fri, Sep 05, 2025 at 04:06:31PM +0300, Jani Nikula wrote: > > > >>>> On Fri, 05 Sep 2025, Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > > > >>>>> Em Fri, 05 Sep 2025 12:02:37 +0300 > > > >>>>> Jani Nikula <jani.nikula@linux.intel.com> escreveu: > > > >>>>> > > > >>>>>> On Wed, 03 Sep 2025, Randy Dunlap <rdunlap@infradead.org> wrote: > > > >>>>>>> Provide some basic comments about the system_states and what they imply. > > > >>>>>>> Also convert the comments to kernel-doc format. > > > >>>>>>> > > > >>>>>>> However, kernel-doc does not support kernel-doc notation on extern > > > >>>>>>> struct/union/typedef/enum/etc. So I made this a DOC: block so that > > > >>>>>>> I can use (insert) it into a Documentation (.rst) file and have it > > > >>>>>>> look decent. > > > >>>>>> > > > >>>>>> The simple workaround is to separate the enum type and the variable: > > > >>>>>> > > > >>>>>> /** > > > >>>>>> * kernel-doc for the enum > > > >>>>>> */ > > > >>>>>> enum system_states { > > > >>>>>> ... > > > >>>>>> }; > > > >>>>>> > > > >>>>>> /** > > > >>>>>> * kernel-doc for the variable > > > >>>>>> */ > > > >>>>>> extern enum system_states system_state; > > > >>>>>> > > > >>>>>> BR, > > > >>>>>> Jani. > > > >>>>>> > > > >>>>>>> > > > >>>>>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org> > > > >>>>>>> Acked-by: Rafael J. Wysocki <rafael@kernel.org> # v1 > > > >>>>>>> --- > > > >> > > > >> [snip] > > > >>>>> If the problem is with "extern" before enum, fixing kdoc be > > > >>>>> fairly trivial. > > > >>>> > > > >>>> The non-trivial part is deciding whether you're documenting the enum > > > >>>> type or the variable. Both are equally valid options. > > > >>> > > > >>> True. > > > >>> > > > >>> I'd say that, if the variable is under EXPORT_SYMBOL macros, it > > > >>> should be documented. > > > >> > > > >> Do you mean documented with kernel-doc? How do we do that? > > > >> I was under the impression that we don't currently have a way to > > > >> use kernel-doc for variables (definitions, only for declarations). > > > > > > > > No, but it shouldn't be hard to add something like: > > > > > > > > /** > > > > * global system_state - stores system state > > > > * <some description> > > > > */ > > > > extern enum system_states system_state; > > > > > > > > and place a dump_global() function at kdoc parser. Ok, one might use > > > > DOC:, but IMHO the best is to have a proper handler for it that would > > > > automatically pick the type. > > > > > > Nitpick, I would s/global/var/. But I won't complain about "global". > > > > Either way works for me. Yet, I would expect it to be used only for > > global variables, as documenting local ones using kernel-doc is > > probably not what we expect inside Kernel documentation. So, naming it > > "global" may help. > > > > > More importantly, I have seen several patches where people try to document a > > > variable, so it seems like something that we should support at some point. > > > > Agreed. > > > > Adding a parsing rule for the variable doesn't sound hard, as they follow, > > in principle, this regex(*): > > > > OPTIONAL_ATTRIBS = [" > > "extern" > > ] > > > > optional = "|".join(OPTIONAL_ATTRIBS) > > > > "^(?:extern\s+)?(\w.*)\s+([\w\_]+)(?:#.*)$" > > > > (*) eventually, some might have extra attributes, but we can > > start with a simpler regex, adding a more complex parser if/when > > needed. > > > > I added at the end a one-minute hacky prototype I wrote, completely > > untested and incomplete. > > Heh, it doesn't hurt spending 15 mins or so to write something that actually > works and implement all functions. > > The example below produces: > > $ ./scripts/kernel-doc include/media/tuner-types.h > ... > .. c:var:: tuners > > list of tuners > > extern const struct tunertype tuners[]; > > .. c:var:: tuner_count > > number of known tuners > > $ ./scripts/kernel-doc include/media/tuner-types.h --man > ... > .TH "Kernel API" 9 "global tuner_count" "September 2025" "API Manual" LINUX > .SH NAME > extern unsigned const int tuner_count; \- number of known tuners > .SH SYNOPSIS > enum tuner_count { > .SH "SEE ALSO" > .PP > > Still not ready for kernel merge (plus I placed bogus descriptions for > two externs from media that IMO doesn't make sense to document), but it > has all needed steps for someone wanting to extend kernel-doc to see > how to do it. > > Feel free to modify it - even renaming from "global" to "var" and > submit upstream. > > Thanks, > Mauro > > [PATCH] [RFC] kernel-doc: add support for handling global varaibles > > Add support for documenting global variables with kernel-doc. > > Please notice that this is mostly an example, as: > > 1. I'm documenting just two random variables from media, that > doesn't make sense to actually be documented. I did it just > to have some example and be able to test it; > > 2. the html output requires tweak: right now, it is just printing > the entire variable prototype as-is, without any formatting, > and witout making sense at the output > > Feel free to modify this patch to make it something actually > mergeable. > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > diff --git a/include/media/tuner-types.h b/include/media/tuner-types.h > index c79b773f750c..cf074beaeccc 100644 > --- a/include/media/tuner-types.h > +++ b/include/media/tuner-types.h > @@ -199,7 +199,18 @@ struct tunertype { > u8 *sleepdata; > }; > > +/** > + * global tuner - list of tuners > + * > + * List of all tuners defined via v4l2 API > + */ > extern const struct tunertype tuners[]; > + > +/** > + * global tune_count - number of known tuners > + * > + * Number of tuners at @tuners list > + */ > extern unsigned const int tuner_count; > > #endif > diff --git a/scripts/lib/kdoc/kdoc_output.py b/scripts/lib/kdoc/kdoc_output.py > index 1eca9a918558..a58562bef35a 100644 > --- a/scripts/lib/kdoc/kdoc_output.py > +++ b/scripts/lib/kdoc/kdoc_output.py > @@ -199,6 +199,10 @@ class OutputFormat: > self.out_enum(fname, name, args) > return self.data > > + if dtype == "global": > + self.out_global(fname, name, args) > + return self.data > + > if dtype == "typedef": > self.out_typedef(fname, name, args) > return self.data > @@ -227,6 +231,9 @@ class OutputFormat: > def out_enum(self, fname, name, args): > """Outputs an enum""" > > + def out_global(self, fname, name, args): > + """Outputs a global variable""" > + > def out_typedef(self, fname, name, args): > """Outputs a typedef""" > > @@ -472,6 +479,20 @@ class RestFormat(OutputFormat): > self.lineprefix = oldprefix > self.out_section(args) > > + def out_global(self, fname, name, args): > + oldprefix = self.lineprefix > + ln = args.declaration_start_line > + > + self.data += f"\n\n.. c:var:: {name}\n\n" > + > + self.print_lineno(ln) > + self.lineprefix = " " > + self.output_highlight(args.get('purpose', '')) > + self.data += "\n" > + > + # FIXME: better handle it > + self.data += args.other_stuff["var_type"] > + Heh, looking at Sphinx doc at: https://www.sphinx-doc.org/en/master/usage/domains/c.html: .. c:member:: declaration .. c:var:: declaration Describes a C struct member or variable. Example signature: .. c:member:: PyObject *PyTypeObject.tp_bases The difference between the two directives is only cosmetic. I guess the best is to encode it as: prototype = args.other_stuff["var_type"] self.data += f"\n\n.. c:var:: {prototype}\n\n" And let Sphinx format it for us. With such change, this is the html output for the two test variables we added at the RFC: <dl class="c var"> <dt class="sig sig-object c" id="c.tuners"> <span class="k"><span class="pre">extern</span></span><span class="w"> </span><span class="k"><span class="pre">const</span></span><span class="w"> </span><span class="k"><span class="pre">struct</span></span><span class="w"> </span><a class="reference internal" href="#c.tunertype" title="tunertype"><span class="n"><span class="pre">tunertype</span></span></a><span class="w"> </span><span class="sig-name descname"><span class="n"><span class="pre">tuners</span></span></span><span class="p"><span class="pre">[</span></span><span class="p"><span class="pre">]</span></span><span class="p"><span class="pre">;</span></span><a class="headerlink" href="#c.tuners" title="Link to this definition">¶</a><br /></dt> <dd><p>list of tuners</p> </dd></dl> <dl class="c var"> <dt class="sig sig-object c"> <span class="sig-name descname"><span class="pre">extern</span> <span class="pre">unsigned</span> <span class="pre">const</span> <span class="pre">int</span> <span class="pre">tuner_count;</span></span></dt> <dd><p>number of known tuners</p> RFC patch with the test vars enclosed. I'll submit the kernel-doc patch in separate, without V4L2 stuff, for its review. Thanks, Mauro --- [PATCH] [RFC] kernel-doc: add support for handling global varaibles Add support for documenting global variables with kernel-doc. Please notice that this is mostly an example, as: 1. I'm documenting just two random variables from media, that doesn't make sense to actually be documented. I did it just to have some example and be able to test it; 2. the html output requires tweak: right now, it is just printing the entire variable prototype as-is, without any formatting, and witout making sense at the output Feel free to modify this patch to make it something actually mergeable. Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> diff --git a/include/media/tuner-types.h b/include/media/tuner-types.h index c79b773f750c..cf074beaeccc 100644 --- a/include/media/tuner-types.h +++ b/include/media/tuner-types.h @@ -199,7 +199,18 @@ struct tunertype { u8 *sleepdata; }; +/** + * global tuner - list of tuners + * + * List of all tuners defined via v4l2 API + */ extern const struct tunertype tuners[]; + +/** + * global tune_count - number of known tuners + * + * Number of tuners at @tuners list + */ extern unsigned const int tuner_count; #endif diff --git a/scripts/lib/kdoc/kdoc_output.py b/scripts/lib/kdoc/kdoc_output.py index 1eca9a918558..405a5c407522 100644 --- a/scripts/lib/kdoc/kdoc_output.py +++ b/scripts/lib/kdoc/kdoc_output.py @@ -199,6 +199,10 @@ class OutputFormat: self.out_enum(fname, name, args) return self.data + if dtype == "global": + self.out_global(fname, name, args) + return self.data + if dtype == "typedef": self.out_typedef(fname, name, args) return self.data @@ -227,6 +231,9 @@ class OutputFormat: def out_enum(self, fname, name, args): """Outputs an enum""" + def out_global(self, fname, name, args): + """Outputs a global variable""" + def out_typedef(self, fname, name, args): """Outputs a typedef""" @@ -472,6 +479,18 @@ class RestFormat(OutputFormat): self.lineprefix = oldprefix self.out_section(args) + def out_global(self, fname, name, args): + oldprefix = self.lineprefix + ln = args.declaration_start_line + prototype = args.other_stuff["var_type"] + + self.data += f"\n\n.. c:var:: {prototype}\n\n" + + self.print_lineno(ln) + self.lineprefix = " " + self.output_highlight(args.get('purpose', '')) + self.data += "\n" + def out_typedef(self, fname, name, args): oldprefix = self.lineprefix @@ -772,6 +791,18 @@ class ManFormat(OutputFormat): self.data += f'.SH "{section}"' + "\n" self.output_highlight(text) + def out_global(self, fname, name, args): + out_name = self.arg_name(args, name) + prototype = args.other_stuff["var_type"] + + self.data += f'.TH "{self.modulename}" 9 "{out_name}" "{self.man_date}" "API Manual" LINUX' + "\n" + + self.data += ".SH NAME\n" + self.data += f"{prototype} \\- {args['purpose']}\n" + + self.data += ".SH SYNOPSIS\n" + self.data += f"enum {name}" + " {\n" + def out_typedef(self, fname, name, args): module = self.modulename purpose = args.get('purpose') diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py index 574972e1f741..e2a3f4574894 100644 --- a/scripts/lib/kdoc/kdoc_parser.py +++ b/scripts/lib/kdoc/kdoc_parser.py @@ -64,7 +64,7 @@ type_param = KernRe(r"@(\w*((\.\w+)|(->\w+))*(\.\.\.)?)", cache=False) # Tests for the beginning of a kerneldoc block in its various forms. # doc_block = doc_com + KernRe(r'DOC:\s*(.*)?', cache=False) -doc_begin_data = KernRe(r"^\s*\*?\s*(struct|union|enum|typedef)\b\s*(\w*)", cache = False) +doc_begin_data = KernRe(r"^\s*\*?\s*(struct|union|enum|typedef|global)\b\s*(\w*)", cache = False) doc_begin_func = KernRe(str(doc_com) + # initial " * ' r"(?:\w+\s*\*\s*)?" + # type (not captured) r'(?:define\s+)?' + # possible "define" (not captured) @@ -886,6 +886,27 @@ class KernelDoc: self.output_declaration('enum', declaration_name, purpose=self.entry.declaration_purpose) + def dump_global(self, ln, proto): + """ + Stores global variables that are part of kAPI. + """ + VAR_ATTRIBS = [ + "extern", + ] + OPTIONAL_VAR_ATTR = "^(?:" + "|".join(VAR_ATTRIBS) + ")?" + + r= KernRe(OPTIONAL_VAR_ATTR + r"(\w.*)\s+([\w_]+)[\d\]\[]*\s*;(?:#.*)?$") + if not r.match(proto): + self.emit_msg(ln,f"{proto}: can't parse variable") + return + + declaration_name = r.group(2) + var_type = r.group(0) + + self.output_declaration("global", declaration_name, + var_type=var_type, + purpose=self.entry.declaration_purpose) + def dump_declaration(self, ln, prototype): """ Stores a data declaration inside self.entries array. @@ -897,6 +918,8 @@ class KernelDoc: self.dump_typedef(ln, prototype) elif self.entry.decl_type in ["union", "struct"]: self.dump_struct(ln, prototype) + elif self.entry.decl_type == "global": + self.dump_global(ln, prototype) else: # This would be a bug self.emit_message(ln, f'Unknown declaration type: {self.entry.decl_type}')
On Sun, 07 Sep 2025, Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > Heh, looking at Sphinx doc at: > https://www.sphinx-doc.org/en/master/usage/domains/c.html: > > .. c:member:: declaration > .. c:var:: declaration > > Describes a C struct member or variable. Example signature: > > .. c:member:: PyObject *PyTypeObject.tp_bases > > The difference between the two directives is only cosmetic. > > I guess the best is to encode it as: > > prototype = args.other_stuff["var_type"] > self.data += f"\n\n.. c:var:: {prototype}\n\n" > > And let Sphinx format it for us. In the same vein, I believe we should let Sphinx format everything else for us as well. Function parameters should use ":param foo: desc" and struct/union members should be indented within the enclosing struct/union. I also think we're going way overboard with including e.g. struct definition in the output. I'd just chuck those away and maybe add links to kernel git source for the definition instead. BR, Jani. -- Jani Nikula, Intel
Em Mon, 08 Sep 2025 12:22:06 +0300 Jani Nikula <jani.nikula@linux.intel.com> escreveu: > On Sun, 07 Sep 2025, Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > > Heh, looking at Sphinx doc at: > > https://www.sphinx-doc.org/en/master/usage/domains/c.html: > > > > .. c:member:: declaration > > .. c:var:: declaration > > > > Describes a C struct member or variable. Example signature: > > > > .. c:member:: PyObject *PyTypeObject.tp_bases > > > > The difference between the two directives is only cosmetic. > > > > I guess the best is to encode it as: > > > > prototype = args.other_stuff["var_type"] > > self.data += f"\n\n.. c:var:: {prototype}\n\n" > > > > And let Sphinx format it for us. > > In the same vein, I believe we should let Sphinx format everything else > for us as well. Function parameters should use ":param foo: desc" and > struct/union members should be indented within the enclosing > struct/union. Good idea. We need to verify first if :param: and :returns: are available since 3.4.3. Docs imply so: https://www.sphinx-doc.org/en/master/usage/domains/c.html > I also think we're going way overboard with including e.g. struct > definition in the output. I'd just chuck those away and maybe add links > to kernel git source for the definition instead. We still need to parse all parameters, as we need them for man pages, as this is the standard practice (see "man 2 read", for instance). We may do something else for html, but: - on functions, the full prototype is required by Sphinx: .. c:function:: void media_set_bus_info (char *bus_info, size_t bus_info_size, struct device *dev) - for struct/union/enum, a data type is not supported, but the documentation has an example about how Sphinx actually expects it: .. c:struct:: Data .. c:union:: @data .. c:var:: int a .. c:var:: double b If we're willing to use the Sphinx way, tests are required. Implementing it would add more complexity, though. Not sure about the benefits if any. In summary, for html/pdf/epub, there are three possible outcomings: - keep as-is; - replace them by links; - implement Sphinx way. In any case, changing it won't cleanup the code, as we still need parameters parsing for man pages. Also, as a documentation user, when read I documentation, I do expect to see the function prototypes just before parameter descriptions. If possible, untouched. This is specially important, IMHO, where there are macros to help generating functions and structs. Thanks, Mauro
© 2016 - 2025 Red Hat, Inc.