Re: [PATCH v3 2/3] doc: update kpt_restrict documentation

[Tobin C. Harding]

On Mon, Dec 18, 2017 at 03:50:06PM -0800, Randy Dunlap wrote:
> On 12/18/2017 02:29 PM, Tobin C. Harding wrote:
> > Recently the behaviour of printk specifier %pK was changed. The
> > documentation does not currently mirror this.
> > 
> > Update documentation for sysctl kpt_restrict.
> 
> Fix subject and here  .

Oh thanks. Took quite a few minutes of staring to see the mistake ;)

Will fix and re-spin.

thanks,
Tobin.
--
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

[RFC PATCH] mm: memcontrol: memory+swap accounting for cgroup-v2

[Shakeel Butt]

The memory controller in cgroup v1 provides the memory+swap (memsw)
interface to account to the combined usage of memory and swap of the
jobs. The memsw interface allows the users to limit or view the
consistent memory usage of their jobs irrespectibe of the presense of
swap on the system (consistent OOM and memory reclaim behavior). The
memory+swap accounting makes the job easier for centralized systems
doing resource usage monitoring, prediction or anomaly detection.

In cgroup v2, the 'memsw' interface was dropped and a new 'swap'
interface has been introduced which allows to limit the actual usage of
swap by the job. For the systems where swap is a limited resource,
'swap' interface can be used to fairly distribute the swap resource
between different jobs. There is no easy way to limit the swap usage
using the 'memsw' interface.

However for the systems where the swap is cheap and can be increased
dynamically (like remote swap and swap on zram), the 'memsw' interface
is much more appropriate as it makes swap transparent to the jobs and
gives consistent memory usage history to centralized monitoring systems.

This patch adds memsw interface to cgroup v2 memory controller behind a
mount option 'memsw'. The memsw interface is mutually exclusive with
the existing swap interface. When 'memsw' is enabled, reading or writing
to 'swap' interface files will return -ENOTSUPP and vice versa. Enabling
or disabling memsw through remounting cgroup v2, will only be effective
if there are no decendants of the root cgroup.

When memsw accounting is enabled then "memory.high" is comapred with
memory+swap usage. So, when the allocating job's memsw usage hits its
high mark, the job will be throttled by triggering memory reclaim.

Signed-off-by: Shakeel Butt 
---
 Documentation/cgroup-v2.txt |  69 
 include/linux/cgroup-defs.h |   5 +++
 kernel/cgroup/cgroup.c  |  12 +
 mm/memcontrol.c | 107 +---
 4 files changed, 157 insertions(+), 36 deletions(-)

diff --git a/Documentation/cgroup-v2.txt b/Documentation/cgroup-v2.txt
index 9a4f2e54a97d..1cbc51203b00 100644
--- a/Documentation/cgroup-v2.txt
+++ b/Documentation/cgroup-v2.txt
@@ -169,6 +169,12 @@ cgroup v2 currently supports the following mount options.
ignored on non-init namespace mounts.  Please refer to the
Delegation section for details.
 
+  memsw
+
+   Allows the enforcement of memory+swap limit on cgroups. This
+   option is system wide and can only be set on mount and can only
+   be modified through remount from the init namespace and if root
+   cgroup has no children.
 
 Organizing Processes and Threads
 
@@ -1020,6 +1026,10 @@ PAGE_SIZE multiple when read back.
Going over the high limit never invokes the OOM killer and
under extreme conditions the limit may be breached.
 
+   If memsw (memory+swap) enforcement is enabled then the
+   cgroup's memory+swap usage is checked against memory.high
+   instead of just memory.
+
   memory.max
A read-write single value file which exists on non-root
cgroups.  The default is "max".
@@ -1207,18 +1217,39 @@ PAGE_SIZE multiple when read back.
 
   memory.swap.current
A read-only single value file which exists on non-root
-   cgroups.
+   cgroups. If memsw is enabled then reading this file will return
+   -ENOTSUPP.
 
The total amount of swap currently being used by the cgroup
and its descendants.
 
   memory.swap.max
A read-write single value file which exists on non-root
-   cgroups.  The default is "max".
+   cgroups.  The default is "max". Accessing this file will return
+   -ENOTSUPP if memsw enforcement is enabled.
 
Swap usage hard limit.  If a cgroup's swap usage reaches this
limit, anonymous meomry of the cgroup will not be swapped out.
 
+  memory.memsw.current
+   A read-only single value file which exists on non-root
+   cgroups. -ENOTSUPP will be returned on read if memsw is not
+   enabled.
+
+   The total amount of memory+swap currently being used by the cgroup
+   and its descendants.
+
+  memory.memsw.max
+   A read-write single value file which exists on non-root
+   cgroups.  The default is "max". -ENOTSUPP will be returned on
+   access if memsw is not enabled.
+
+   Memory+swap usage hard limit. If a cgroup's memory+swap usage
+   reaches this limit and  can't be reduced, the OOM killer is
+   invoked in the cgroup. Under certain circumstances, the usage
+   may go over the limit temporarily.
+
+
 
 Usage Guidelines
 
@@ -1243,6 +1274,23 @@ memory - is necessary to determine whether a workload 
needs more
 memory; unfortunately, memory pressure monitoring mechanism isn't
 implemented yet.
 
+Please note that when memory+swap accounting is enforced then the

Re: [PATCH v3 2/3] doc: update kpt_restrict documentation

[Randy Dunlap]

On 12/18/2017 02:29 PM, Tobin C. Harding wrote:
> Recently the behaviour of printk specifier %pK was changed. The
> documentation does not currently mirror this.
> 
> Update documentation for sysctl kpt_restrict.

Fix subject and here  .

> 
> Signed-off-by: Tobin C. Harding 
> ---
>  Documentation/sysctl/kernel.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
> index 63663039acb7..412314eebda6 100644
> --- a/Documentation/sysctl/kernel.txt
> +++ b/Documentation/sysctl/kernel.txt
> @@ -391,7 +391,8 @@ kptr_restrict:
>  This toggle indicates whether restrictions are placed on
>  exposing kernel addresses via /proc and other interfaces.
>  
> -When kptr_restrict is set to (0), the default, there are no restrictions.
> +When kptr_restrict is set to 0 (the default) the address is hashed before
> +printing. (This is the equivalent to %p.)
>  
>  When kptr_restrict is set to (1), kernel pointers printed using the %pK
>  format specifier will be replaced with 0's unless the user has CAP_SYSLOG
> 


-- 
~Randy
--
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 v9 29/51] mm/mprotect, powerpc/mm/pkeys, x86/mm/pkeys: Add sysfs interface

[Ram Pai]

On Mon, Dec 18, 2017 at 02:28:14PM -0800, Dave Hansen wrote:
> On 12/18/2017 02:18 PM, Ram Pai wrote:
> > b) minimum number of keys available to the application.
> > if libraries consumes a few, they could provide a library
> > interface to the application informing the number available to
> > the application.  The library interface can leverage (b) to
> > provide the information.
> 
> OK, let's see a real user of this including a few libraries.  Then we'll
> put it in the kernel.
> 
> > c) types of disable-rights supported by keys.
> > Helps the application to determine the types of disable-features
> > available. This is helpful, otherwise the app has to 
> > make pkey_alloc() call with the corresponding parameter set
> > and see if it suceeds or fails. Painful from an application
> > point of view, in my opinion.
> 
> Again, let's see a real-world use of this.  How does it look?  How does
> an app "fall back" if it can't set a restriction the way it wants to?
> 
> Are we *sure* that such an interface makes sense?  For instance, will it
> be possible for some keys to be execute-disable while other are only
> write-disable?

Can it be on x86?

its not possible on ppc. the keys can *all* be  the-same-attributes-disable all 
the
time.

However you are right. Its conceivable that some arch could provide a
feature where it can be x-attribute-disable for key 'a' and
y-attribute-disable for key 'b'.  But than its a bit of a headache
for an application to consume such a feature.

Ben: I recall you requesting this feature.  Thoughts?

> 
> > I think on x86 you look for some hardware registers to determine
> > which hardware features are enabled by the kernel.
> 
> No, we use CPUID.  It's a part of the ISA that's designed for
> enumerating CPU and (sometimes) OS support for CPU features.
> 
> > We do not have generic support for something like that on ppc.  The
> > kernel looks at the device tree to determine what hardware features
> > are available. But does not have mechanism to tell the hardware to
> > track which of its features are currently enabled/used by the
> > kernel; atleast not for the memory-key feature.
> 
> Bummer.  You're missing out.
> 
> But, you could still do this with a syscall.  "Hey, kernel, do you
> support this feature?"

or do powerpc specific sysfs interface.
or a debugfs interface.

RP

--
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 v3 2/3] doc: update kpt_restrict documentation

[Tobin C. Harding]

Recently the behaviour of printk specifier %pK was changed. The
documentation does not currently mirror this.

Update documentation for sysctl kpt_restrict.

Signed-off-by: Tobin C. Harding 
---
 Documentation/sysctl/kernel.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index 63663039acb7..412314eebda6 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -391,7 +391,8 @@ kptr_restrict:
 This toggle indicates whether restrictions are placed on
 exposing kernel addresses via /proc and other interfaces.
 
-When kptr_restrict is set to (0), the default, there are no restrictions.
+When kptr_restrict is set to 0 (the default) the address is hashed before
+printing. (This is the equivalent to %p.)
 
 When kptr_restrict is set to (1), kernel pointers printed using the %pK
 format specifier will be replaced with 0's unless the user has CAP_SYSLOG
-- 
2.7.4

--
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 v3 1/3] doc: convert printk-formats.txt to rst

[Tobin C. Harding]

Documentation/printk-formats.txt is a candidate for conversion to
ReStructuredText format. Some effort has already been made to do this
conversion even thought the suffix is currently .txt

Changes required to complete conversion

 - Move printk-formats.txt to core-api/printk-formats.rst
 - Add entry to Documentation/core-api/index.rst
 - Remove entry from Documentation/00-INDEX
 - Fix minor grammatical errors.
 - Order heading adornments as suggested by rst docs.
 - Use 'Passed by reference' uniformly.
 - Update pointer documentation around %px specifier.
 - Fix erroneous double backticks (to commas).
 - Remove extraneous double backticks (suggested by Jonathan Corbet).
 - Simplify documentation for kobject.

Signed-off-by: Tobin C. Harding 
---
 Documentation/00-INDEX |   2 -
 Documentation/core-api/index.rst   |   1 +
 .../printk-formats.rst}| 229 +++--
 lib/vsprintf.c |   3 +-
 4 files changed, 122 insertions(+), 113 deletions(-)
 rename Documentation/{printk-formats.txt => core-api/printk-formats.rst} (63%)

diff --git a/Documentation/00-INDEX b/Documentation/00-INDEX
index 3bec49c33bbb..7023bfaec21c 100644
--- a/Documentation/00-INDEX
+++ b/Documentation/00-INDEX
@@ -346,8 +346,6 @@ prctl/
- directory with info on the priveledge control subsystem
 preempt-locking.txt
- info on locking under a preemptive kernel.
-printk-formats.txt
-   - how to get printk format specifiers right
 process/
- how to work with the mainline kernel development process.
 pps/
diff --git a/Documentation/core-api/index.rst b/Documentation/core-api/index.rst
index d4d54b05d6c5..d55ee6b006ed 100644
--- a/Documentation/core-api/index.rst
+++ b/Documentation/core-api/index.rst
@@ -22,6 +22,7 @@ Core utilities
flexible-arrays
librs
genalloc
+   printk-formats
 
 Interfaces for kernel debugging
 ===
diff --git a/Documentation/printk-formats.txt 
b/Documentation/core-api/printk-formats.rst
similarity index 63%
rename from Documentation/printk-formats.txt
rename to Documentation/core-api/printk-formats.rst
index aa0a776c817a..2c542e30b13b 100644
--- a/Documentation/printk-formats.txt
+++ b/Documentation/core-api/printk-formats.rst
@@ -5,6 +5,7 @@ How to get printk format specifiers right
 :Author: Randy Dunlap 
 :Author: Andrew Murray 
 
+
 Integer types
 =
 
@@ -25,39 +26,45 @@ Integer types
s64 %lld or %llx
u64 %llu or %llx
 
-If  is dependent on a config option for its size (e.g., ``sector_t``,
-``blkcnt_t``) or is architecture-dependent for its size (e.g., ``tcflag_t``),
-use a format specifier of its largest possible type and explicitly cast to it.
+
+If  is dependent on a config option for its size (e.g., sector_t,
+blkcnt_t) or is architecture-dependent for its size (e.g., tcflag_t), use a
+format specifier of its largest possible type and explicitly cast to it.
 
 Example::
 
printk("test: sector number/total blocks: %llu/%llu\n",
(unsigned long long)sector, (unsigned long long)blockcount);
 
-Reminder: ``sizeof()`` result is of type ``size_t``.
+Reminder: sizeof() returns type size_t.
 
-The kernel's printf does not support ``%n``. For obvious reasons, floating
-point formats (``%e, %f, %g, %a``) are also not recognized. Use of any
+The Kernel's printf does not support %n. Floating point formats (%e, %f,
+%g, %a) are also not recognized, for obvious reasons. Use of any
 unsupported specifier or length qualifier results in a WARN and early
-return from vsnprintf.
-
-Raw pointer value SHOULD be printed with %p. The kernel supports
-the following extended format specifiers for pointer types:
+return from vsnprintf().
 
-Pointer Types
+Pointer types
 =
 
-Pointers printed without a specifier extension (i.e unadorned %p) are
-hashed to give a unique identifier without leaking kernel addresses to user
-space. On 64 bit machines the first 32 bits are zeroed. If you _really_
-want the address see %px below.
+A raw pointer value may be printed with %p which will hash the address
+before printing. The Kernel also supports extended specifiers for printing
+pointers of different types.
+
+Plain Pointers
+--
 
 ::
 
%p  abcdef12 or abcdef12
 
+Pointers printed without a specifier extension (i.e unadorned %p) are
+hashed to prevent leaking information about the Kernel memory layout. This
+has the added benefit of providing a unique identifier. On 64-bit machines
+the first 32 bits are zeroed. If you *really* want the address see %px
+below.
+
 Symbols/Function Pointers
-=
+-
 
 ::
 
@@ -69,6 +76,7 @@ Symbols/Function Pointers
%ps versatile_init
%pB 

[PATCH v3 3/3] doc: add documentation on printing kernel addresses

[Tobin C. Harding]

Hashing addresses printed with printk specifier %p was implemented
recently. During development a number of issues were raised regarding
leaking kernel addresses to userspace. Other documentation was updated but
security/self-protection missed out.

Add self-protection documentation regarding printing kernel addresses.

Signed-off-by: Tobin C. Harding 
---
 Documentation/security/self-protection.rst | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/security/self-protection.rst 
b/Documentation/security/self-protection.rst
index 60c8bd8b77bf..0f53826c78b9 100644
--- a/Documentation/security/self-protection.rst
+++ b/Documentation/security/self-protection.rst
@@ -270,6 +270,21 @@ attacks, it is important to defend against exposure of 
both kernel memory
 addresses and kernel memory contents (since they may contain kernel
 addresses or other sensitive things like canary values).
 
+Kernel addresses
+
+
+Printing kernel addresses to userspace leaks sensitive information about
+the kernel memory layout. Care should be exercised when using any printk
+specifier that prints the raw address, currently %px, %p[ad], (and %p[sSb]
+in certain circumstances [*]).  Any file written to using one of these
+specifiers should be readable only by privileged processes.
+
+Kernels 4.14 and older printed the raw address using %p. As of 4.15-rc1
+addresses printed with the specifier %p are hashed before printing.
+
+[*] If KALLSYMS is enabled and symbol lookup fails, the raw address is
+printed. If KALLSYMS is not enabled the raw address is printed.
+
 Unique identifiers
 --
 
-- 
2.7.4

--
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 v3 0/3] doc: update printk documentation

[Tobin C. Harding]

This set converts printk-formats.txt -> core-api/printk-formats.rst

We also update the documentation around printing kernel addresses.

This is my first documentation conversion. Please do be hard on this
patch series. I'd like to get it _really_ correct so that future
conversions will require less review effort. Also if there are any
peculiarities to patching docs (as apposed to C files) please say so.

Thank you for the time already given to reviewing previous versions.

thanks,
Tobin.

v3:
 - Update filename .txt -> .rst in lib/vsnprintf.c comment (Joe Perches)
 - Remove unnecessary commas (as suggested by Randy Dunlap)
 - Re-apply theory 'make as few changes as possible to complete the
   conversion'

v2:
 - Remove conversion/inclusion of kernel-docs from lib/vsprintf.c
 - Add '<>' around file name (in section 'Thanks').
 - Remove a few more double back ticks. 
 - Apply theory 'make as few changes as possible to complete the
   conversion' 

Tobin C. Harding (3):
  doc: convert printk-formats.txt to rst
  doc: update kpt_restrict documentation
  doc: add documentation on printing kernel addresses

 Documentation/00-INDEX |   2 -
 Documentation/core-api/index.rst   |   1 +
 .../printk-formats.rst}| 229 +++--
 Documentation/security/self-protection.rst |  15 ++
 Documentation/sysctl/kernel.txt|   3 +-
 lib/vsprintf.c |   3 +-
 6 files changed, 139 insertions(+), 114 deletions(-)
 rename Documentation/{printk-formats.txt => core-api/printk-formats.rst} (63%)

-- 
2.7.4

--
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 v9 29/51] mm/mprotect, powerpc/mm/pkeys, x86/mm/pkeys: Add sysfs interface

[Dave Hansen]

On 12/18/2017 02:18 PM, Ram Pai wrote:
> b) minimum number of keys available to the application.
>   if libraries consumes a few, they could provide a library
>   interface to the application informing the number available to
>   the application.  The library interface can leverage (b) to
>   provide the information.

OK, let's see a real user of this including a few libraries.  Then we'll
put it in the kernel.

> c) types of disable-rights supported by keys.
>   Helps the application to determine the types of disable-features
>   available. This is helpful, otherwise the app has to 
>   make pkey_alloc() call with the corresponding parameter set
>   and see if it suceeds or fails. Painful from an application
>   point of view, in my opinion.

Again, let's see a real-world use of this.  How does it look?  How does
an app "fall back" if it can't set a restriction the way it wants to?

Are we *sure* that such an interface makes sense?  For instance, will it
be possible for some keys to be execute-disable while other are only
write-disable?

> I think on x86 you look for some hardware registers to determine which
> hardware features are enabled by the kernel.

No, we use CPUID.  It's a part of the ISA that's designed for
enumerating CPU and (sometimes) OS support for CPU features.

> We do not have generic support for something like that on ppc.
> The kernel looks at the device tree to determine what hardware features
> are available. But does not have mechanism to tell the hardware to track
> which of its features are currently enabled/used by the kernel; atleast
> not for the memory-key feature.

Bummer.  You're missing out.

But, you could still do this with a syscall.  "Hey, kernel, do you
support this feature?"
--
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 v9 29/51] mm/mprotect, powerpc/mm/pkeys, x86/mm/pkeys: Add sysfs interface

[Ram Pai]

On Mon, Dec 18, 2017 at 10:54:26AM -0800, Dave Hansen wrote:
> On 11/06/2017 12:57 AM, Ram Pai wrote:
> > Expose useful information for programs using memory protection keys.
> > Provide implementation for powerpc and x86.
> > 
> > On a powerpc system with pkeys support, here is what is shown:
> > 
> > $ head /sys/kernel/mm/protection_keys/*

> > ==> /sys/kernel/mm/protection_keys/disable_access_supported <==
> > true
> 
> This is cute, but I don't think it should be part of the ABI.  Put it in

thanks :)

> debugfs if you want it for cute tests.  The stuff that this tells you
> can and should come from pkey_alloc() for the ABI.

Applications can make system calls with different parameters and on
failure determine indirectly that such a feature may not be available in
the kernel/hardware.  But from an application point of view, I think, it
is a very clumsy/difficult way to determine that.

For example, an application can keep making pkey_alloc() calls and count
till the call fails, to determine the number of keys supported by the
system. And then the application has to release those keys too.  Too
much side-effect just to determine a simple thing. Do we want the
application to endure this pain?

I think we should aim to provide sufficient API/ABI for the application
to consume the feature efficiently, and not any more.

I do not claim that the ABI exposed by this patch is sufficiently
optimal. But I do believe it is tending towards it.

currently the following ABI is  exposed.

a) total number of keys available in the system. This information may
not be useful and can possibly be dropped.

b) minimum number of keys available to the application.
if libraries consumes a few, they could provide a library
interface to the application informing the number available to
the application.  The library interface can leverage (b) to
provide the information.

c) types of disable-rights supported by keys.
Helps the application to determine the types of disable-features
available. This is helpful, otherwise the app has to 
make pkey_alloc() call with the corresponding parameter set
and see if it suceeds or fails. Painful from an application
point of view, in my opinion.

> 
> http://man7.org/linux/man-pages/man7/pkeys.7.html
> 
> >Any application wanting to use protection keys needs to be able to
> >function without them.  They might be unavailable because the
> >hardware that the application runs on does not support them, the
> >kernel code does not contain support, the kernel support has been
> >disabled, or because the keys have all been allocated, perhaps by a
> >library the application is using.  It is recommended that
> >applications wanting to use protection keys should simply call
> >pkey_alloc(2) and test whether the call succeeds, instead of
> >attempting to detect support for the feature in any other way.
> 
> Do you really not have standard way on ppc to say whether hardware
> features are supported by the kernel?  For instance, how do you know if
> a given set of registers are known to and are being context-switched by
> the kernel?

I think on x86 you look for some hardware registers to determine which
hardware features are enabled by the kernel.

We do not have generic support for something like that on ppc.
The kernel looks at the device tree to determine what hardware features
are available. But does not have mechanism to tell the hardware to track
which of its features are currently enabled/used by the kernel; atleast
not for the memory-key feature.

RP

--
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] Documentation/filesystems/vfat.txt: fix a remark that implies UCS2

[OGAWA Hirofumi]

Adam Borowski  writes:

> All non-historic operating systems support the full range of Unicode here,
> thus you can make filenames for example in Gothic (̴̼͉ͅ), the other Gothic
> (퓂ℯℴ퓌) or the third Gothic (헆햾허헐), or declare something as .
>
> Characters above U+ are encoded on four bytes.
>
> Signed-off-by: Adam Borowski 
> ---
>  Documentation/filesystems/vfat.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/filesystems/vfat.txt 
> b/Documentation/filesystems/vfat.txt
> index cf51360e3a9f..91031298beb1 100644
> --- a/Documentation/filesystems/vfat.txt
> +++ b/Documentation/filesystems/vfat.txt
> @@ -344,4 +344,4 @@ the following:
>  characters in the final slot are set to Unicode 0x.
>  
>  Finally, note that the extended name is stored in Unicode.  Each Unicode
> -character takes two bytes.
> +character takes either two or four bytes, UTF-16LE encoded.

Acked-by: OGAWA Hirofumi 

Thanks.
-- 
OGAWA Hirofumi 
--
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] Documentation/filesystems/vfat.txt: fix a remark that implies UCS2

[Adam Borowski]

All non-historic operating systems support the full range of Unicode here,
thus you can make filenames for example in Gothic (̴̼͉ͅ), the other Gothic
(퓂ℯℴ퓌) or the third Gothic (헆햾허헐), or declare something as .

Characters above U+ are encoded on four bytes.

Signed-off-by: Adam Borowski 
---
 Documentation/filesystems/vfat.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/filesystems/vfat.txt 
b/Documentation/filesystems/vfat.txt
index cf51360e3a9f..91031298beb1 100644
--- a/Documentation/filesystems/vfat.txt
+++ b/Documentation/filesystems/vfat.txt
@@ -344,4 +344,4 @@ the following:
   characters in the final slot are set to Unicode 0x.
 
 Finally, note that the extended name is stored in Unicode.  Each Unicode
-character takes two bytes.
+character takes either two or four bytes, UTF-16LE encoded.
-- 
2.15.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 15/24] media: v4l2-subdev: get rid of __V4L2_SUBDEV_MK_GET_TRY() macro

[Mauro Carvalho Chehab]

Em Mon, 9 Oct 2017 23:23:56 +0300
Sakari Ailus  escreveu:

> Hi Mauro,
> 
> On Mon, Oct 09, 2017 at 07:19:21AM -0300, Mauro Carvalho Chehab wrote:
> > The __V4L2_SUBDEV_MK_GET_TRY() macro is used to define
> > 3 functions that have the same arguments. The code of those
> > functions is simple enough to just declare them, de-obfuscating
> > the code.
> > 
> > While here, replace BUG_ON() by WARN_ON() as there's no reason
> > why to panic the Kernel if this fails.
> 
> BUG_ON() might actually be a better idea as this will lead to memory
> corruption. I presume it's not been hit often.

Well, let's then change the code to:

if (WARN_ON(pad >= sd->entity.num_pads)) 
return -EINVAL;

This way, it won't try to use an invalid value. As those are default
handlers for ioctls, userspace should be able to handle it.

> 
> That said, I, too, favour WARN_ON() in this case. In case pad exceeds the
> number of pads, then zero could be used, for instance. The only real
> problem comes if there were no pads to begin with. The callers of these
> functions also don't expect to receive NULL. Another option would be to
> define a static, dummy variable for the purpose that would be at least safe
> to access. Or we could just use the dummy entry whenever the pad isn't
> valid.
> 
> This will make the functions more complex and I might just keep the
> original macro. Even grep works on it nowadays.
> 
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> > ---
> >  include/media/v4l2-subdev.h | 37 +
> >  1 file changed, 25 insertions(+), 12 deletions(-)
> > 
> > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> > index 1f34045f07ce..35c4476c56ee 100644
> > --- a/include/media/v4l2-subdev.h
> > +++ b/include/media/v4l2-subdev.h
> > @@ -897,19 +897,32 @@ struct v4l2_subdev_fh {
> > container_of(fh, struct v4l2_subdev_fh, vfh)
> >  
> >  #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
> > -#define __V4L2_SUBDEV_MK_GET_TRY(rtype, fun_name, field_name)  
> > \
> > -   static inline struct rtype *\
> > -   fun_name(struct v4l2_subdev *sd,\
> > -struct v4l2_subdev_pad_config *cfg,\
> > -unsigned int pad)  \
> > -   {   \
> > -   BUG_ON(pad >= sd->entity.num_pads); \
> > -   return [pad].field_name;\
> > -   }
> > +static inline struct v4l2_mbus_framefmt
> > +*v4l2_subdev_get_try_format(struct v4l2_subdev *sd,
> > +   struct v4l2_subdev_pad_config *cfg,
> > +   unsigned int pad)
> > +{
> > +   WARN_ON(pad >= sd->entity.num_pads);
> > +   return [pad].try_fmt;
> > +}
> >  
> > -__V4L2_SUBDEV_MK_GET_TRY(v4l2_mbus_framefmt, v4l2_subdev_get_try_format, 
> > try_fmt)
> > -__V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, v4l2_subdev_get_try_crop, try_crop)
> > -__V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, v4l2_subdev_get_try_compose, 
> > try_compose)
> > +static inline struct v4l2_rect
> > +*v4l2_subdev_get_try_crop(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + unsigned int pad)
> > +{
> > +   WARN_ON(pad >= sd->entity.num_pads);
> > +   return [pad].try_crop;
> > +}
> > +
> > +static inline struct v4l2_rect
> > +*v4l2_subdev_get_try_compose(struct v4l2_subdev *sd,
> > +struct v4l2_subdev_pad_config *cfg,
> > +unsigned int pad)
> > +{
> > +   WARN_ON(pad >= sd->entity.num_pads);
> > +   return [pad].try_compose;
> > +}
> >  #endif
> >  
> >  extern const struct v4l2_file_operations v4l2_subdev_fops;
> 



Thanks,
Mauro
--
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 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Leon Romanovsky]

On Mon, Dec 18, 2017 at 07:39:50PM +0100, Knut Omang wrote:
> On Mon, 2017-12-18 at 17:56 +, Bart Van Assche wrote:
> > On Mon, 2017-12-18 at 10:46 -0700, Jason Gunthorpe wrote:
> > > On Sun, Dec 17, 2017 at 10:00:17PM -0800, Joe Perches wrote:
> > >
> > > > > Today when we run checkers we get so many warnings it is too hard to
> > > > > make any sense of it.
> > > >
> > > > Here is a list of the checkpatch messages for drivers/infiniband
> > > > sorted by type.
> > > >
> > > > Many of these might be corrected by using
> > > >
> > > > $ ./scripts/checkpatch.pl -f --fix-inplace --types= \
> > > >   $(git ls-files drivers/infiniband/)
> > >
> > > How many of these do you think it is worth to fix?
> > >
> > > We do get a steady trickle of changes in this topic every cycle.
> > >
> > > Is it better to just do a big number of them all at once? Do you have
> > > an idea how disruptive this kind of work is to the whole patch flow
> > > eg new patches no longer applying to for-next, backports no longer
> > > applying, merge conflicts?
> >
> > In my opinion patches that only change the coding style and do not change 
> > any
> > functionality are annoying. Before posting a patch that fixes a bug the 
> > change
> > history (git log -p) has to be cheched to figure out which patch introduced
> > the bug. Patches that only change coding style pollute the change history.
>
> I agree with you - the problem is that style issues should not have existed.
> But when they do it becomes a problem to remove them and a problem to
> keep them - for instance us who try to be compliant by having style helpers
> in our editor, we end up having to manually revert old style mistakes back in
> to avoid making unrelated whitespace changes or similar.

If the checkpatch.pl complains about coding style for the new patch in
newly added code, I'm asking from the author to prepare cleanup patch so
it will be applied before actual patch.

In case, complains are for code which patch are not touching, I'm
submitting it as is.

Thanks


signature.asc
Description: PGP signature

Re: [PATCH v2 14/17] media: v4l2-async: better describe match union at async match struct

[Mauro Carvalho Chehab]

Em Fri, 13 Oct 2017 15:49:25 +0300
Laurent Pinchart  escreveu:

> Hi Mauro,
> 
> Thank you for the patch.
> 
> On Thursday, 28 September 2017 00:46:57 EEST Mauro Carvalho Chehab wrote:
> > Now that kernel-doc handles nested unions, better document the
> > match union at struct v4l2_async_subdev.
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> > ---
> >  include/media/v4l2-async.h | 35 ---
> >  1 file changed, 32 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> > index e66a3521596f..62c2d572ec23 100644
> > --- a/include/media/v4l2-async.h
> > +++ b/include/media/v4l2-async.h
> > @@ -46,10 +46,39 @@ enum v4l2_async_match_type {
> >  /**
> >   * struct v4l2_async_subdev - sub-device descriptor, as known to a bridge
> >   *
> > - * @match_type:type of match that will be used
> > - * @match: union of per-bus type matching data sets
> > + * @match_type:
> > + * type of match that will be used
> > + * @match:
> > + * union of per-bus type matching data sets  
> 
> The lines don't exceed the 80 columnes limit, you can keep them as-is.

OK.

> 
> > + * @match.fwnode:
> > + * pointer to  fwnode_handle to be matched.
> > + * Used if @match_type is %V4L2_ASYNC_MATCH_FWNODE.
> > + * @match.device_name:
> > + * string containing the device name to be matched.
> > + * Used if @match_type is %V4L2_ASYNC_MATCH_DEVNAME.
> > + * @match.i2c:
> > + * embedded struct with I2C parameters to be matched.
> > + * Both @match.i2c.adapter_id and @match.i2c.address
> > + * should be matched.
> > + * Used if @match_type is %V4L2_ASYNC_MATCH_I2C.  
> 
> Do you really need to document this ? Isn't it enough to document 
> @match.i2c.adapter_id and @match.i2c.address ?

No. If we don't document a non-anonymous struct, kernel-doc will complain
that a field description is missed. Same applies to the custom field
below.

There's a way to get rid of it: converting it to an anonymous
struct, but I guess that will make the usage of the match rule
harder to understand.

> 
> > + * @match.i2c.adapter_id:
> > + * I2C adapter ID to be matched.
> > + * Used if @match_type is %V4L2_ASYNC_MATCH_I2C.
> > + * @match.i2c.address:
> > + * I2C address to be matched.
> > + * Used if @match_type is %V4L2_ASYNC_MATCH_I2C.
> > + * @match.custom:
> > + * Driver-specific match criteria.
> > + * Used if @match_type is %V4L2_ASYNC_MATCH_CUSTOM.  
> 
> Same here.
> 
> > + * @match.custom.match:
> > + * Driver-specific match function to be used if
> > + * %V4L2_ASYNC_MATCH_CUSTOM.
> > + * @match.custom.priv:
> > + * Driver-specific private struct with match parameters
> > + * to be used if %V4L2_ASYNC_MATCH_CUSTOM.
> >   * @list:  used to link struct v4l2_async_subdev objects, waiting to be
> > - * probed, to a notifier->waiting list
> > + * probed, to a notifier->waiting list.
> > + * Not to be used by drivers.
> >   */
> >  struct v4l2_async_subdev {
> > enum v4l2_async_match_type match_type;  
> 

Thanks,
Mauro
--
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 13/17] media: v4l2-async: simplify v4l2_async_subdev structure

[Mauro Carvalho Chehab]

Em Sat, 30 Sep 2017 01:05:24 +0300
Sakari Ailus  escreveu:

> Hi Mauro,
> 
> (Removing the non-list recipients.)
> 
> On Fri, Sep 29, 2017 at 06:27:13AM -0300, Mauro Carvalho Chehab wrote:
> > Em Thu, 28 Sep 2017 15:09:21 +0300
> > Sakari Ailus  escreveu:
> >   
> > > Hi Mauro,
> > > 
> > > On Wed, Sep 27, 2017 at 06:46:56PM -0300, Mauro Carvalho Chehab wrote:  
> > > > The V4L2_ASYNC_MATCH_FWNODE match criteria requires just one
> > > > struct to be filled (struct fwnode_handle). The V4L2_ASYNC_MATCH_DEVNAME
> > > > match criteria requires just a device name.
> > > > 
> > > > So, it doesn't make sense to enclose those into structs,
> > > > as the criteria can go directly into the union.
> > > > 
> > > > That makes easier to document it, as we don't need to document
> > > > weird senseless structs.
> > > 
> > > The idea is that in the union, there's a struct which is specific to the
> > > match_type field. I wouldn't call it senseless.  
> > 
> > Why a struct for each specific match_type is **needed**? It it is not
> > needed, then it is senseless per definition :-) 
> > 
> > In the specific case of fwnode, there's already a named struct
> > for fwnode_handle. The only thing is that it is declared outside
> > enum v4l2_async_match_type. So, I don't see any reason to do things
> > like:
> > 
> > struct {
> > struct fwnode_handle *fwnode;
> > } fwnode;
> > 
> > If you're in doubt about that, think on how would you document
> > both fwnode structs. Both fwnode structs specify the match
> > criteria if %V4L2_ASYNC_MATCH_FWNODE.
> > 
> > The same applies to this:
> > 
> > struct {
> > const char *name;
> > } device_name;
> > 
> > Both device_name and name specifies the match criteria if
> > %V4L2_ASYNC_MATCH_DEVNAME.
> >   
> > > 
> > > In the two cases there's just a single field in the containing struct. You
> > > could remove the struct in that case as you do in this patch, and just use
> > > the field. But I think the result is less clean and so I wouldn't make 
> > > this
> > > change.  
> > 
> > It is actually cleaner without the stucts.
> > 
> > Without the useless struct, if one wants to match a firmware node, it
> > should be doing:
> > 
> > pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_FWNODE;
> > pdata->asd[i]->match.fwnode = of_fwnode_handle(rem);  
> 
> This code should be and will be moved out of drivers. See:
> 
> 
> 
> So there are going to be quite a bit fewer instances of it, and none should
> remain in drivers. I frankly don't have a strong opinion on this; there are
> arguments for and against. I just don't see a reason to change it.

There are still a few occurrences on drivers. Just rebased it.
I'll post it in a few, inside a new patch series.

Simplifying the name of the match rules makes easier to understand
what's going on.


Thanks,
Mauro
--
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 v9 29/51] mm/mprotect, powerpc/mm/pkeys, x86/mm/pkeys: Add sysfs interface

[Dave Hansen]

On 11/06/2017 12:57 AM, Ram Pai wrote:
> Expose useful information for programs using memory protection keys.
> Provide implementation for powerpc and x86.
> 
> On a powerpc system with pkeys support, here is what is shown:
> 
> $ head /sys/kernel/mm/protection_keys/*
> ==> /sys/kernel/mm/protection_keys/disable_access_supported <==
> true

This is cute, but I don't think it should be part of the ABI.  Put it in
debugfs if you want it for cute tests.  The stuff that this tells you
can and should come from pkey_alloc() for the ABI.

http://man7.org/linux/man-pages/man7/pkeys.7.html

>Any application wanting to use protection keys needs to be able to
>function without them.  They might be unavailable because the
>hardware that the application runs on does not support them, the
>kernel code does not contain support, the kernel support has been
>disabled, or because the keys have all been allocated, perhaps by a
>library the application is using.  It is recommended that
>applications wanting to use protection keys should simply call
>pkey_alloc(2) and test whether the call succeeds, instead of
>attempting to detect support for the feature in any other way.

Do you really not have standard way on ppc to say whether hardware
features are supported by the kernel?  For instance, how do you know if
a given set of registers are known to and are being context-switched by
the kernel?
--
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 04/24] media: v4l2-mediabus: convert flags to enums and document them

[Mauro Carvalho Chehab]

Em Wed, 11 Oct 2017 23:26:44 +0200
Pavel Machek  escreveu:

> On Mon 2017-10-09 07:19:10, Mauro Carvalho Chehab wrote:
> > There is a mess with media bus flags: there are two sets of
> > flags, one used by parallel and ITU-R BT.656 outputs,
> > and another one for CSI2.
> > 
> > Depending on the type, the same bit has different meanings.
> >   
> 
> > @@ -86,11 +125,22 @@ enum v4l2_mbus_type {
> >  /**
> >   * struct v4l2_mbus_config - media bus configuration
> >   * @type:  in: interface type
> > - * @flags: in / out: configuration flags, depending on @type
> > + * @pb_flags:  in / out: configuration flags, if @type is
> > + * %V4L2_MBUS_PARALLEL or %V4L2_MBUS_BT656.
> > + * @csi2_flags:in / out: configuration flags, if @type is
> > + * %V4L2_MBUS_CSI2.
> > + * @flag:  access flags, no matter the @type.
> > + * Used just to avoid needing to rewrite the logic inside
> > + * soc_camera and pxa_camera drivers. Don't use on newer
> > + * drivers!
> >   */
> >  struct v4l2_mbus_config {
> > enum v4l2_mbus_type type;
> > -   unsigned int flags;
> > +   union {
> > +   enum v4l2_mbus_parallel_and_bt656_flags pb_flags;
> > +   enum v4l2_mbus_csi2_flags   csi2_flags;
> > +   unsigned intflag;
> > +   };
> >  };
> >  
> >  static inline void v4l2_fill_pix_format(struct v4l2_pix_format
> >  *pix_fmt,  
> 
> The flags->flag conversion is quite subtle, and "flag" is confusing
> because there is more than one inside. What about something like
> __legacy_flags?

Good idea.

> 
>   Pavel

Thanks,
Mauro

[PATCH] media: v4l2-mediabus: convert flags to enums and document them

There is a mess with media bus flags: there are two sets of
flags, one used by parallel and ITU-R BT.656 outputs,
and another one for CSI2.

Depending on the type, the same bit has different meanings.

That's very confusing, and counter-intuitive. So, split them
into two sets of flags, inside an enum.

This way, it becomes clearer that there are two separate sets
of flags. It also makes easier if CSI1, CSP, CSI3, etc. would
need a different set of flags.

As a side effect, enums can be documented via kernel-docs,
so there will be an improvement at flags documentation.

Unfortunately, soc_camera and pxa_camera do a mess with
the flags, using either one set of flags without proper
checks about the type. That could be fixed, but, as both drivers
are obsolete and in the process of cleanings, I opted to just
keep the behavior, using an unsigned int inside those two
drivers.

Acked-by: Hans Verkuil 
Signed-off-by: Mauro Carvalho Chehab 

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 25d24a3f10a7..4bf25a72ef4f 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -743,16 +743,16 @@ static int adv7180_g_mbus_config(struct v4l2_subdev *sd,
 
if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
cfg->type = V4L2_MBUS_CSI2;
-   cfg->flags = V4L2_MBUS_CSI2_1_LANE |
-   V4L2_MBUS_CSI2_CHANNEL_0 |
-   V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+   cfg->csi2_flags = V4L2_MBUS_CSI2_1_LANE
+ | V4L2_MBUS_CSI2_CHANNEL_0
+ | V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
} else {
/*
 * The ADV7180 sensor supports BT.601/656 output modes.
 * The BT.656 is default and not yet configurable by s/w.
 */
-   cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
-V4L2_MBUS_DATA_ACTIVE_HIGH;
+   cfg->pb_flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING
+   | V4L2_MBUS_DATA_ACTIVE_HIGH;
cfg->type = V4L2_MBUS_BT656;
}
 
diff --git a/drivers/media/i2c/ml86v7667.c b/drivers/media/i2c/ml86v7667.c
index 57ef901edb06..a25114d0c31f 100644
--- a/drivers/media/i2c/ml86v7667.c
+++ b/drivers/media/i2c/ml86v7667.c
@@ -226,8 +226,9 @@ static int ml86v7667_fill_fmt(struct v4l2_subdev *sd,
 static int ml86v7667_g_mbus_config(struct v4l2_subdev *sd,
   struct v4l2_mbus_config *cfg)
 {
-   cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
-V4L2_MBUS_DATA_ACTIVE_HIGH;
+   cfg->pb_flags = V4L2_MBUS_MASTER
+   | V4L2_MBUS_PCLK_SAMPLE_RISING
+   | V4L2_MBUS_DATA_ACTIVE_HIGH;
cfg->type = V4L2_MBUS_BT656;
 
return 0;
diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
index b1665d97e0fd..d9698b535080 100644
--- a/drivers/media/i2c/mt9m111.c
+++ b/drivers/media/i2c/mt9m111.c
@@ -857,9 +857,11 @@ static int 

Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Knut Omang]

On Mon, 2017-12-18 at 17:56 +, Bart Van Assche wrote:
> On Mon, 2017-12-18 at 10:46 -0700, Jason Gunthorpe wrote:
> > On Sun, Dec 17, 2017 at 10:00:17PM -0800, Joe Perches wrote:
> > 
> > > > Today when we run checkers we get so many warnings it is too hard to
> > > > make any sense of it.
> > > 
> > > Here is a list of the checkpatch messages for drivers/infiniband
> > > sorted by type.
> > > 
> > > Many of these might be corrected by using
> > > 
> > > $ ./scripts/checkpatch.pl -f --fix-inplace --types= \
> > >   $(git ls-files drivers/infiniband/)
> > 
> > How many of these do you think it is worth to fix?
> > 
> > We do get a steady trickle of changes in this topic every cycle.
> > 
> > Is it better to just do a big number of them all at once? Do you have
> > an idea how disruptive this kind of work is to the whole patch flow
> > eg new patches no longer applying to for-next, backports no longer
> > applying, merge conflicts?
> 
> In my opinion patches that only change the coding style and do not change any
> functionality are annoying. Before posting a patch that fixes a bug the change
> history (git log -p) has to be cheched to figure out which patch introduced
> the bug. Patches that only change coding style pollute the change history.

I agree with you - the problem is that style issues should not have existed. 
But when they do it becomes a problem to remove them and a problem to 
keep them - for instance us who try to be compliant by having style helpers 
in our editor, we end up having to manually revert old style mistakes back in
to avoid making unrelated whitespace changes or similar.

I think what we get with this patch set is that there's a way to rein in the
issues and to enable some regression testing without enforcing a lot of work 
or annoying history issues *right away*. That way this problem can be tackled 
when
appropriate for the subsystem in question, and the reason for holding back on 
some changes can be documented in the local runchecks.cfg file, focusing
willing helping hands on the issues considered most important for that 
subsystem.

Thanks,
Knut
--
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 2/7] i3c: Add core I3C infrastructure

[Randy Dunlap]

On 12/18/2017 12:37 AM, Boris Brezillon wrote:
> On Sun, 17 Dec 2017 14:32:04 -0800
> Randy Dunlap  wrote:
> 
>> On 12/14/17 07:16, Boris Brezillon wrote:
>>> Add core infrastructure to support I3C in Linux and document it.
>>>
>>> Signed-off-by: Boris Brezillon 
>>> ---
>>>  drivers/Kconfig |2 +
>>>  drivers/Makefile|2 +-
>>>  drivers/i3c/Kconfig |   24 +
>>>  drivers/i3c/Makefile|4 +
>>>  drivers/i3c/core.c  |  573 
>>>  drivers/i3c/device.c|  344 ++
>>>  drivers/i3c/internals.h |   34 +
>>>  drivers/i3c/master.c| 1433 
>>> +++
>>>  drivers/i3c/master/Kconfig  |0
>>>  drivers/i3c/master/Makefile |0
>>>  include/linux/i3c/ccc.h |  380 +++
>>>  include/linux/i3c/device.h  |  321 +
>>>  include/linux/i3c/master.h  |  564 +++
>>>  include/linux/mod_devicetable.h |   17 +
>>>  14 files changed, 3697 insertions(+), 1 deletion(-)
>>>  create mode 100644 drivers/i3c/Kconfig
>>>  create mode 100644 drivers/i3c/Makefile
>>>  create mode 100644 drivers/i3c/core.c
>>>  create mode 100644 drivers/i3c/device.c
>>>  create mode 100644 drivers/i3c/internals.h
>>>  create mode 100644 drivers/i3c/master.c
>>>  create mode 100644 drivers/i3c/master/Kconfig
>>>  create mode 100644 drivers/i3c/master/Makefile
>>>  create mode 100644 include/linux/i3c/ccc.h
>>>  create mode 100644 include/linux/i3c/device.h
>>>  create mode 100644 include/linux/i3c/master.h


>>> diff --git a/drivers/i3c/core.c b/drivers/i3c/core.c
>>> new file mode 100644
>>> index ..7eb8e84acd33
>>> --- /dev/null
>>> +++ b/drivers/i3c/core.c
>>> @@ -0,0 +1,573 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (C) 2017 Cadence Design Systems Inc.
>>> + *
>>> + * Author: Boris Brezillon 
>>> + */
>>> +
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include   
>>
>> #include 
>> #include 
>> #include 
>> #include 
>> #include 
> 
> Do you have a tool to detect those missing inclusions?

Nope, I've often wanted one, but for now it's just slow reading.



>>> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
>>> new file mode 100644
>>> index ..7ec9a4821bac
>>> --- /dev/null
>>> +++ b/include/linux/i3c/master.h
>>> @@ -0,0 +1,564 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * Copyright (C) 2017 Cadence Design Systems Inc.
>>> + *
>>> + * Author: Boris Brezillon 
>>> + */
>>> +
>>> +#ifndef I3C_MASTER_H
>>> +#define I3C_MASTER_H
>>> +
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +#define I3C_HOT_JOIN_ADDR  0x2
>>> +#define I3C_BROADCAST_ADDR 0x7e
>>> +#define I3C_MAX_ADDR   GENMASK(6, 0)
>>> +  
>>
>> Needs bitops.h, workqueue.h, rwsem.h
>>
>>
>> Needs 
> 
> Okay, that's really weird to directly include a header from the
> asm-generic directory, are you sure this is the right thing to do here?

Looks like it should be , which will grab the correct one.


-- 
~Randy
--
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 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Bart Van Assche]

On Mon, 2017-12-18 at 10:46 -0700, Jason Gunthorpe wrote:
> On Sun, Dec 17, 2017 at 10:00:17PM -0800, Joe Perches wrote:
> 
> > > Today when we run checkers we get so many warnings it is too hard to
> > > make any sense of it.
> > 
> > Here is a list of the checkpatch messages for drivers/infiniband
> > sorted by type.
> > 
> > Many of these might be corrected by using
> > 
> > $ ./scripts/checkpatch.pl -f --fix-inplace --types= \
> >   $(git ls-files drivers/infiniband/)
> 
> How many of these do you think it is worth to fix?
> 
> We do get a steady trickle of changes in this topic every cycle.
> 
> Is it better to just do a big number of them all at once? Do you have
> an idea how disruptive this kind of work is to the whole patch flow
> eg new patches no longer applying to for-next, backports no longer
> applying, merge conflicts?

In my opinion patches that only change the coding style and do not change any
functionality are annoying. Before posting a patch that fixes a bug the change
history (git log -p) has to be cheched to figure out which patch introduced
the bug. Patches that only change coding style pollute the change history.

Bart.

Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Joe Perches]

On Mon, 2017-12-18 at 10:46 -0700, Jason Gunthorpe wrote:
> On Sun, Dec 17, 2017 at 10:00:17PM -0800, Joe Perches wrote:
> 
> > > Today when we run checkers we get so many warnings it is too hard to
> > > make any sense of it.
> > 
> > Here is a list of the checkpatch messages for drivers/infiniband
> > sorted by type.
> > 
> > Many of these might be corrected by using
> > 
> > $ ./scripts/checkpatch.pl -f --fix-inplace --types= \
> >   $(git ls-files drivers/infiniband/)
> 
> How many of these do you think it is worth to fix?
> 
> We do get a steady trickle of changes in this topic every cycle.
> 
> Is it better to just do a big number of them all at once?

I think so.

> Do you have
> an idea how disruptive this kind of work is to the whole patch flow
> eg new patches no longer applying to for-next, backports no longer
> applying, merge conflicts?

Some do complain about backport patch purity.

I think that difficulty is overstated, but then
again, I don't do backports very often.

I think the best time for any rather wholesale
change is immediately after an rc-1 so overall
in-flight patch conflict volume is minimized.
--
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 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Jason Gunthorpe]

On Mon, Dec 18, 2017 at 02:05:15PM +0100, Knut Omang wrote:
 
> https://github.com/torvalds/linux/compare/master...knuto:runchecks

Several of these to rdma/core do not look so big, you should think
about sending them..

Jason
--
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 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Jason Gunthorpe]

On Sun, Dec 17, 2017 at 10:00:17PM -0800, Joe Perches wrote:

> > Today when we run checkers we get so many warnings it is too hard to
> > make any sense of it.
> 
> Here is a list of the checkpatch messages for drivers/infiniband
> sorted by type.
> 
> Many of these might be corrected by using
> 
> $ ./scripts/checkpatch.pl -f --fix-inplace --types= \
>   $(git ls-files drivers/infiniband/)

How many of these do you think it is worth to fix?

We do get a steady trickle of changes in this topic every cycle.

Is it better to just do a big number of them all at once? Do you have
an idea how disruptive this kind of work is to the whole patch flow
eg new patches no longer applying to for-next, backports no longer
applying, merge conflicts?

Jason
--
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 00/24] V4L2 kAPI cleanups and documentation improvements part 2

[Mauro Carvalho Chehab]

Em Mon,  9 Oct 2017 07:19:06 -0300
Mauro Carvalho Chehab  escreveu:

> That's the second part of my V4L2 kAPI documentation improvements.
> It is meant to reduce the gap between the kAPI media headers
> and documentation, at least with regards to kernel-doc markups.
> 
> We should likely write more things at the ReST files under Documentation/
> to better describe some of those APIs (VB2 being likely the first candidate),
> but at least let's be sure that all V4L2 bits have kernel-doc markups.

I'm also applying the patches from this series that nobody commented,
or whose comments were fully addressed.

Thanks,
Mauro
--
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 19/24] media: vb2-core: Improve kernel-doc markups

[Mauro Carvalho Chehab]

Em Tue, 10 Oct 2017 16:32:34 +0300
Sakari Ailus  escreveu:

> Hi Mauro,
> 
> On Mon, Oct 09, 2017 at 07:19:25AM -0300, Mauro Carvalho Chehab wrote:
> > There are several issues on the current markups:
> > - lack of cross-references;
> > - wrong cross-references;
> > - lack of a period of the end of several phrases;
> > - Some descriptions can be enhanced.
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> > ---
> >  include/media/videobuf2-core.h | 376 
> > ++---
> >  1 file changed, 204 insertions(+), 172 deletions(-)
> > 
> > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> > index 0308d8439049..e145f1475ffe 100644
> > --- a/include/media/videobuf2-core.h
> > +++ b/include/media/videobuf2-core.h  
> 
> ...
> 
> >  /**
> >   * vb2_core_queue_init() - initialize a videobuf2 queue
> > - * @q: videobuf2 queue; this structure should be allocated in 
> > driver
> > + * @q: pointer to  vb2_queue with videobuf2 queue.
> > + * This structure should be allocated in driver
> >   *
> >   * The _queue structure should be allocated by the driver. The driver 
> > is
> >   * responsible of clearing it's content and setting initial values for some
> >   * required entries before calling this function.
> > - * q->ops, q->mem_ops, q->type and q->io_modes are mandatory. Please refer
> > - * to the struct vb2_queue description in include/media/videobuf2-core.h
> > - * for more information.
> > + *
> > + * .. note::
> > + *
> > + *The following fields at @q should be set before calling this 
> > function:  
> 
> should -> shall
> 
> I bet there's a lot of that in the documentation elsewhere, including this
> patch.

Yes, there are, and not only on this file (where it uses should
everywhere).

I prefer to do such change globally.

For now, I'll apply this patch as-is.

Thanks,
Mauro
--
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 05/24] media: v4l2-dev: convert VFL_TYPE_* into an enum

[Mauro Carvalho Chehab]

Em Tue, 10 Oct 2017 21:47:04 +0100
Andrey Utkin  escreveu:

> On Mon, Oct 09, 2017 at 07:19:11AM -0300, Mauro Carvalho Chehab wrote:
> > Using enums makes easier to document, as it can use kernel-doc
> > markups. It also allows cross-referencing, with increases the
> > kAPI readability.
> >   
> 
> 
> All changes look legit.
> 
> But I'd expect cx88_querycap() return type change and such to be in
> separate commit.

It should be together, as the switch() now would generate a warning,
because some enum values aren't listed. On such case, it has to
return an error.

I added an explanation at the commit message.

> 
> > diff --git a/drivers/media/pci/cx88/cx88-blackbird.c 
> > b/drivers/media/pci/cx88/cx88-blackbird.c
> > index e3101f04941c..0e0952e60795 100644
> > --- a/drivers/media/pci/cx88/cx88-blackbird.c
> > +++ b/drivers/media/pci/cx88/cx88-blackbird.c
> > @@ -805,8 +805,7 @@ static int vidioc_querycap(struct file *file, void  
> > *priv,
> >  
> > strcpy(cap->driver, "cx88_blackbird");
> > sprintf(cap->bus_info, "PCI:%s", pci_name(dev->pci));
> > -   cx88_querycap(file, core, cap);
> > -   return 0;
> > +   return cx88_querycap(file, core, cap);
> >  }
> >  
> >  static int vidioc_enum_fmt_vid_cap(struct file *file, void  *priv,
> > diff --git a/drivers/media/pci/cx88/cx88-video.c 
> > b/drivers/media/pci/cx88/cx88-video.c
> > index 7d25ecd4404b..9be682cdb644 100644
> > --- a/drivers/media/pci/cx88/cx88-video.c
> > +++ b/drivers/media/pci/cx88/cx88-video.c
> > @@ -806,8 +806,8 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void 
> > *priv,
> > return 0;
> >  }
> >  
> > -void cx88_querycap(struct file *file, struct cx88_core *core,
> > -  struct v4l2_capability *cap)
> > +int cx88_querycap(struct file *file, struct cx88_core *core,
> > + struct v4l2_capability *cap)
> >  {
> > struct video_device *vdev = video_devdata(file);
> >



Thanks,
Mauro
--
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 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Knut Omang]

On Mon, 2017-12-18 at 07:30 -0800, Joe Perches wrote:
> On Mon, 2017-12-18 at 14:05 +0100, Knut Omang wrote:
> > > Here is a list of the checkpatch messages for drivers/infiniband
> > > sorted by type.
> > > 
> > > Many of these might be corrected by using
> > > 
> > > $ ./scripts/checkpatch.pl -f --fix-inplace --types= \
> > >   $(git ls-files drivers/infiniband/)
> > 
> > Yes, and I already did that work piece by piece for individual types,
> > just to test the runchecks tool, and want to post that set once the 
> > runchecks script and Makefile changes itself are in,
> 
> I think those are independent of any runcheck tool changes and
> could be posted now.  In general, don't keep patches in a local
> tree waiting on some other unrelated patch.

It becomes related in that the runchecks.cfg file is updated 
in all the patches to keep 'make C=2' run with 0 errors while 
enabling more checks. I think they serve well as examples of 
how a workflow with runchecks could be.

> Just fyi:
> 
> There is a script that helps automate checkpatch "by-type" conversions
> with compilation, .o difference checking, and git commit editing.
> 
> https://lkml.org/lkml/2014/7/11/794

oh - good to know - seems it would have been a good help
during my little exercise..

Thanks,
Knut
--
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 08/17] media: v4l2-ioctl.h: convert debug macros into enum and document

[Laurent Pinchart]

Hi Mauro,

On Monday, 18 December 2017 17:13:26 EET Mauro Carvalho Chehab wrote:
> Em Fri, 13 Oct 2017 15:38:11 +0300 Laurent Pinchart escreveu:
> > On Thursday, 28 September 2017 00:46:51 EEST Mauro Carvalho Chehab wrote:
> >> Currently, there's no way to document #define foo 
> >> with kernel-doc. So, convert it to an enum, and document.
> > 
> > The documentation seems fine to me (except for one comment below).
> > However, converting macros to an enum just to work around a defect of the
> > documentation system doesn't seem like a good idea to me. I'd rather find
> > a way to document macros.
> 
> I agree that this limitation should be fixed.
> 
> Yet, in this specific case where we have an "array" of defines, all
> associated to the same field (even being a bitmask), and assuming that
> we would add a way for kernel-doc to parse this kind of defines
> (not sure how easy/doable would be), then, in order to respect the
> way kernel-doc markup is, the documentation for those macros would be:
> 
> 
> /**
>  * define: Just log the ioctl name + error code
>  */
> #define V4L2_DEV_DEBUG_IOCTL  0x01
> /**
>  * define: Log the ioctl name arguments + error code
>  */
> #define V4L2_DEV_DEBUG_IOCTL_ARG  0x02
> /**
>  * define: Log the file operations open, release, mmap and get_unmapped_area
> */
> #define V4L2_DEV_DEBUG_FOP0x04
> /**
>  * define: Log the read and write file operations and the VIDIOC_(D)QBUF
> ioctls */
> #define V4L2_DEV_DEBUG_STREAMING  0x08
> 
> IMHO, this is a way easier to read/understand by humans, and a way more
> coincise:
> 
> /**
>  * enum v4l2_debug_flags - Device debug flags to be used with the video
>  *device debug attribute
>  *
>  * @V4L2_DEV_DEBUG_IOCTL: Just log the ioctl name + error code.
>  * @V4L2_DEV_DEBUG_IOCTL_ARG: Log the ioctl name arguments + error code.
>  * @V4L2_DEV_DEBUG_FOP:   Log the file operations and open, 
> release,
>  *mmap and get_unmapped_area syscalls.
>  * @V4L2_DEV_DEBUG_STREAMING: Log the read and write syscalls and
>  *:c:ref:`VIDIOC_[Q|DQ]BUFF ` ioctls.
>  */
> 
> It also underlines the aspect that those names are grouped altogether.
> 
> So, IMHO, the main reason to place them inside an enum and document
> as such is that it looks a way better for humans to read.

As we're talking about extending kerneldoc to document macros, we're free to 
decide on a format that would make it easy and clear. Based on your above 
example, we could write it

/**
 * define v4l2_debug_flags - Device debug flags to be used with the video
 *  device debug attribute
 *
 * @V4L2_DEV_DEBUG_IOCTL:   Just log the ioctl name + error code.
 * @V4L2_DEV_DEBUG_IOCTL_ARG:   Log the ioctl name arguments + error code.
 * @V4L2_DEV_DEBUG_FOP: Log the file operations and open, release,
 *  mmap and get_unmapped_area syscalls.
 * @V4L2_DEV_DEBUG_STREAMING:   Log the read and write syscalls and
 *  :c:ref:`VIDIOC_[Q|DQ]BUFF ` ioctls.
 */

That would be simple, clear, and wouldn't require a code change to workaround 
a limitation of the documentation system.

-- 
Regards,

Laurent Pinchart

--
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 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Joe Perches]

On Mon, 2017-12-18 at 14:05 +0100, Knut Omang wrote:
> > Here is a list of the checkpatch messages for drivers/infiniband
> > sorted by type.
> > 
> > Many of these might be corrected by using
> > 
> > $ ./scripts/checkpatch.pl -f --fix-inplace --types= \
> >   $(git ls-files drivers/infiniband/)
> 
> Yes, and I already did that work piece by piece for individual types,
> just to test the runchecks tool, and want to post that set once the 
> runchecks script and Makefile changes itself are in,

I think those are independent of any runcheck tool changes and
could be posted now.  In general, don't keep patches in a local
tree waiting on some other unrelated patch.

Just fyi:

There is a script that helps automate checkpatch "by-type" conversions
with compilation, .o difference checking, and git commit editing.

https://lkml.org/lkml/2014/7/11/794

--
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 v3 00/17] kernel-doc: add supported to document nested structs/

[Mauro Carvalho Chehab]

Em Wed,  4 Oct 2017 08:48:38 -0300
Mauro Carvalho Chehab  escreveu:

> Right now, it is not possible to document nested struct and nested unions.
> kernel-doc simply ignore them.
> 
> Add support to document them.
> 
> Patches 1 to 6 improve kernel-doc documentation to reflect what
> kernel-doc currently supports and import some stuff from the
> old kernel-doc-nano-HOWTO.txt.
> 
> Patch 7 gets rid of the old documentation (kernel-doc-nano-HOWTO.txt).
> 
> 
> Patch 8 gets rid of the now unused output formats for kernel-doc: since 
> we got rid of all DocBook stuff, we should not need them anymore. The
> reason for dropping it (despite cleaning up), is that it doesn't make 
> sense to invest time on adding new features for formats that aren't
> used anymore.
> 
> Patch 9 improves argument handling, printing script usage if an
> invalid argument is passed and accepting both -cmd and --cmd
> forms.
> 
> Patch 10 changes the default output format to ReST, as this is a way
> more used than man output nowadays.
> 
> Patch 11 solves an issue after ReST conversion, where tabs were not
> properly handled on kernel-doc tags.
> 
> Patch 12 adds support for parsing kernel-doc nested structures and 
> unions.
> 
> Patch 13 does a cleanup, removing $nexted parameter at the
> routines that handle structs.
> 
> Patch 14 Improves warning output by printing the identifier where
> the warning occurred.
> 
> Patch 15 complements patch 12, by adding support for function
> definitions inside nested structures. It is needed by some media
> docs with use such kind of things.
> 
> Patch 16 improves nested struct handling even further, allowing it
> to handle cases where a nested named struct/enum with multiple
> identifiers added (e. g. struct { ... } foo, bar;).
> 
> Patch 17 adds documentation for nested union/struct inside w1_netlink.
> 
> The entire patch series are at my development tree, at:
> https://git.linuxtv.org/mchehab/experimental.git/log/?h=nested-fix-v4b

I'm applying all patches from this series that either didn't have
any comments of whose editorial comments I fully embraced.

The patches that weren't applied from this series are:
0008-media-v4l2-device.h-document-ancillary-macros.patch
0010-media-v4l2-ioctl.h-convert-debug-macros-into-enum-an.patch
0014-media-v4l2-async-simplify-v4l2_async_subdev-structur.patch
0015-media-v4l2-async-better-describe-match-union-at-asyn.patch

I'll send later a patch series with those - probably together with
other documentation patches from the other two series I sent - that
need further reviews.

Thanks,
Mauro
--
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 08/17] media: v4l2-ioctl.h: convert debug macros into enum and document

[Mauro Carvalho Chehab]

Em Fri, 13 Oct 2017 15:38:11 +0300
Laurent Pinchart  escreveu:

> Hi Mauro,
> 
> Thank you for the patch.
> 
> On Thursday, 28 September 2017 00:46:51 EEST Mauro Carvalho Chehab wrote:
> > Currently, there's no way to document #define foo 
> > with kernel-doc. So, convert it to an enum, and document.  
> 
> The documentation seems fine to me (except for one comment below). However, 
> converting macros to an enum just to work around a defect of the 
> documentation 
> system doesn't seem like a good idea to me. I'd rather find a way to document 
> macros.

I agree that this limitation should be fixed.

Yet, in this specific case where we have an "array" of defines, all
associated to the same field (even being a bitmask), and assuming that
we would add a way for kernel-doc to parse this kind of defines 
(not sure how easy/doable would be), then, in order to respect the
way kernel-doc markup is, the documentation for those macros would be:


/**
 * define: Just log the ioctl name + error code 
 */
#define V4L2_DEV_DEBUG_IOCTL0x01
/**
 * define: Log the ioctl name arguments + error code 
 */
#define V4L2_DEV_DEBUG_IOCTL_ARG0x02
/**
 * define: Log the file operations open, release, mmap and get_unmapped_area 
 */
#define V4L2_DEV_DEBUG_FOP  0x04
/**
 * define: Log the read and write file operations and the VIDIOC_(D)QBUF ioctls 
 */
#define V4L2_DEV_DEBUG_STREAMING0x08

IMHO, this is a way easier to read/understand by humans, and a way more
coincise:

/**
 * enum v4l2_debug_flags - Device debug flags to be used with the video
 *  device debug attribute
 *
 * @V4L2_DEV_DEBUG_IOCTL:   Just log the ioctl name + error code.
 * @V4L2_DEV_DEBUG_IOCTL_ARG:   Log the ioctl name arguments + error code.
 * @V4L2_DEV_DEBUG_FOP: Log the file operations and open, release,
 *  mmap and get_unmapped_area syscalls.
 * @V4L2_DEV_DEBUG_STREAMING:   Log the read and write syscalls and
 *  :c:ref:`VIDIOC_[Q|DQ]BUFF ` ioctls.
 */

It also underlines the aspect that those names are grouped altogether.

So, IMHO, the main reason to place them inside an enum and document
as such is that it looks a way better for humans to read.

Thanks,
Mauro
--
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 05/17] media: v4l2-device.h: document ancillary macros

[Mauro Carvalho Chehab]

Em Fri, 13 Oct 2017 15:33:01 +0300
Laurent Pinchart  escreveu:

> Hi Mauro,
> 
> Thank you for the patch.
> 
> On Thursday, 28 September 2017 00:46:48 EEST Mauro Carvalho Chehab wrote:
> > There are several widely macros that aren't documented using kernel-docs  
> 
> What's a widely macro ? :-)
> 
> > markups.
> > 
> > Add it.  
> 
> Did you mean "Add documentation." ? "Document them." is also an option.
> 
> > 
> > Signed-off-by: Mauro Carvalho Chehab 

I attended most of the points you pointed. I'll comment just the
ones that I have a different opinion, or require further discussions.

> > -/* Call the specified callback for all subdevs matching the condition.
> > -   Ignore any errors. Note that you cannot add or delete a subdev
> > -   while walking the subdevs list. */
> > +/**
> > + * __v4l2_device_call_subdevs_p - Calls the specified callback for  
> 
> All the __v4l2_device_* macros are internal, I don't think there's a need to 
> document them just for the sake of it.

Yeah, they're meant to be used only internally, but there are lot
of tricks on those macros. So, I prefer to keep them documented.

> 
> > + * all subdevs matching a device-specific group ID.  
> 
> How exactly is the group ID device-specific ?

This "group" is really device-specific: each device may define its own
private set of device groups. I guess only one or two drivers actually
use it, like this one:

drivers/media/platform/davinci/vpif_display.c

See calls for v4l2_device_call_until_err() there.

The group ID is filled on this logic, inside vpif_probe():

if (vpif_obj.sd[i])
vpif_obj.sd[i]->grp_id = 1 << i;

Maybe it could say, instead:

"all subdevs matching the _subdev.grp_id, as
assigned by the bridge driver."

 
> > + * @v4l2_dev: pointer to  v4l2_device
> > + * @grpid:  v4l2_subdev->grp_id group ID to match.
> > + *Use 0 to match them all.
> > + * @o: name of the element at  v4l2_subdev_ops that contains @f.
> > + * Each element there groups a set of callbacks functions.
> > + * @f: callback function that will be called if @cond matches.
> > + * The callback functions are defined in groups, according to
> > + * each element at  v4l2_subdev_ops.  
> 
> Using the word "group" here makes it very confusing. You could use "operation 
> class" instead.

It isn't an operation class. It is a group of sub-devices, defined by
the bridge driver.

> Another option would be to document @o.@f of Sphinx doesn't 
> complain/

It will complain.

I'm enclosing a new version of this patch with the requested changes.


Thanks,
Mauro

[PATCH v3 05/17] media: v4l2-device.h: document helper macros

There are several macros that aren't documented using kernel-docs
markups.

Document them.

While here, add cross-references to structs on this file.

Acked-by: Sakari Ailus 
Signed-off-by: Mauro Carvalho Chehab 
---
 include/media/v4l2-device.h |  246 +---
 1 file changed, 211 insertions(+), 35 deletions(-)

--- patchwork.orig/include/media/v4l2-device.h
+++ patchwork/include/media/v4l2-device.h
@@ -38,7 +38,7 @@ struct v4l2_ctrl_handler;
  * @lock: lock this struct; can be used by the driver as well
  * if this struct is embedded into a larger struct.
  * @name: unique device name, by default the driver name + bus ID
- * @notify: notify callback called by some sub-devices.
+ * @notify: notify operation called by some sub-devices.
  * @ctrl_handler: The control handler. May be %NULL.
  * @prio: Device's priority state
  * @ref: Keep track of the references to this struct.
@@ -56,7 +56,6 @@ struct v4l2_ctrl_handler;
  *#) @dev->driver_data points to this struct.
  *#) @dev might be %NULL if there is no parent device
  */
-
 struct v4l2_device {
struct device *dev;
 #if defined(CONFIG_MEDIA_CONTROLLER)
@@ -166,7 +165,7 @@ void v4l2_device_unregister(struct v4l2_
  * v4l2_device_register_subdev - Registers a subdev with a v4l2 device.
  *
  * @v4l2_dev: pointer to struct _device
- * @sd: pointer to struct _subdev
+ * @sd: pointer to  v4l2_subdev
  *
  * While registered, the subdev module is marked as in-use.
  *
@@ -179,7 +178,7 @@ int __must_check v4l2_device_register_su
 /**
  * v4l2_device_unregister_subdev - Unregisters a subdev with a v4l2 device.
  *
- * @sd: pointer to struct _subdev
+ * @sd: pointer to  v4l2_subdev
  *
  * .. note ::
  *
@@ -201,7 +200,7 @@ v4l2_device_register_subdev_nodes(struct
 /**
  * v4l2_subdev_notify - Sends a notification to v4l2_device.
  *
- * @sd: pointer to struct _subdev
+ * @sd: pointer to  v4l2_subdev
  * @notification: type of notification. Please notice that the notification
  * type is driver-specific.
  * @arg: arguments for the notification. Those are specific to each
@@ -214,13 +213,43 @@ static inline void v4l2_subdev_notify(st
 

Re: [PATCH v2 02/17] media: v4l2-common: get rid of v4l2_routing dead struct

[Sean Young]

On Mon, Dec 18, 2017 at 12:11:13PM -0200, Mauro Carvalho Chehab wrote:
> Em Fri, 13 Oct 2017 15:24:34 +0200
> Hans Verkuil  escreveu:
> 
> > > ---
> > >  include/media/v4l2-common.h | 14 +-
> > >  1 file changed, 5 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> > > @@ -238,11 +239,6 @@ struct v4l2_priv_tun_config {
> > >  
> > >  #define VIDIOC_INT_RESET _IOW ('d', 102, u32)  
> 
> > 
> > Regarding this one: I *think* (long time ago) that the main reason for this
> > was to reset a locked up IR blaster. I wonder if this is still needed after
> > Sean's rework of this. Once that's all done and merged this would probably
> > merit another look to see if it can be removed.
> 
> Sean,
> 
> Could you please double-check if this is still required on RC?

The ioctl can also reset the digitizer in ivtv, I don't know anything
about that.

As far the IR receiver/blaster is concerned:

This ioctl does exactly as it says and it works. There are a few ways
that the zilog microcontroller can be crashed, but I don't know of any
that exist in the current code base.

I seem to recall that the microcontroller could get stuck due particular 
i2c states, which was fixed with patches to ivtv-i2c.c. That does not mean
there aren't any others though. 

What would be nicer if the ir driver got no response, it would reset it
automatically. I'm not sure how to wire up ir-kbd-i2c with the ir reset
function of ivtv and cx18 though.

Then again with no known use cases it seems superfluous. How about removing
the ioctl and then hooking up the IR driver to the reset, should it be
necessary?


Sean
--
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 02/17] media: v4l2-common: get rid of v4l2_routing dead struct

[Mauro Carvalho Chehab]

Em Fri, 13 Oct 2017 15:24:34 +0200
Hans Verkuil  escreveu:

> > ---
> >  include/media/v4l2-common.h | 14 +-
> >  1 file changed, 5 insertions(+), 9 deletions(-)
> > 
> > diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> > @@ -238,11 +239,6 @@ struct v4l2_priv_tun_config {
> >  
> >  #define VIDIOC_INT_RESET   _IOW ('d', 102, u32)  

> 
> Regarding this one: I *think* (long time ago) that the main reason for this
> was to reset a locked up IR blaster. I wonder if this is still needed after
> Sean's rework of this. Once that's all done and merged this would probably
> merit another look to see if it can be removed.

Sean,

Could you please double-check if this is still required on RC?


Thanks,
Mauro
--
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 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Knut Omang]

On Sun, 2017-12-17 at 22:00 -0700, Jason Gunthorpe wrote:
> On Sun, Dec 17, 2017 at 03:14:10AM +0100, Knut Omang wrote:
> 
> > > I like the ability to add more checkers and keep then in the main
> > > upstream tree. But adding overrides for specific subsystems goes against
> > > the policy that all subsystems should be treated equally.
> > 
> > This is a tool to enable automated testing for as many checks as
> > possible, as soon as possible. Like any tool, it can be misused, but
> > that's IMHO an orthogonal problem that I think the maintainers will
> > be more than capable of preventing.
> > 
> > Think of this as a tightening screw: We eliminate errors class by
> > class or file by file, and in the same commit narrows in the list of
> > exceptions. That way we can fix issues piece by piece while avoiding
> > a lot of regressions in already clean parts.
> 
> Since you used drivers/infiniband as an example for this script..
> 
> I will say I agree with this idea.

> It is not that we *want* infiniband to be different from the rest of
> the kernel, it is that we have this historical situation where we
> don't have a code base that already passes the various static checker
> things.

I poked around trying make M= in different parts of the kernel to get some 
"mileage" and to get as much examples as possible, and it appears most of 
the kernel has issues of sorts. Also Joe and others keep adding more checks 
as well, and we'd want that process to coexist with the need for automatic 
regression testing in this area.

> I would like it very much if I could run 'make static checker' and see
> no warnings. 

which is just what is the result with 'make C=2 M=drivers/infiniband/core' 
and the patches #1 and #5 in this set in case not everyone got the point,

> This helps me know that I when I accept patches I am not
> introducing new problems to code that has already been cleaned up.
> 
> Today when we run checkers we get so many warnings it is too hard to
> make any sense of it.
> 
> Being able to say File X is now clean for check XYZ seems very useful
> and may motivate people to clean up the files they actualy care
> about...
> 
> > > There was discussion at Kernel Summit about how the different
> > > subsystems already have different rules. This appears to be a way
> > > to make that worse.
> > 
> > IMHO this is a tool that should help maintainers implement the
> > policies they desire.  But the tool itself does not dictate any
> > such.
> 
> Yes, again, in infiniband we like to see checkpatch be good for new
> submission, even if that clashes with surrounding code. For instance
> we have a mixture of sizeof foo and sizeof(foo) styles in the same
> file/function now.

That's one of the fixes that the excellent --fix-inplace handles so 
one of my patches here

https://github.com/torvalds/linux/compare/master...knuto:runchecks

fixes those in drivers/infiniband/core.

> I certainly don't want to tell people they need to follow some
> different style from 10 years ago when they send patches.
> 

Thanks,
Knut

> Jason
--
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 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Knut Omang]

On Sun, 2017-12-17 at 22:00 -0800, Joe Perches wrote:
> On Sun, 2017-12-17 at 22:00 -0700, Jason Gunthorpe wrote:
> > On Sun, Dec 17, 2017 at 03:14:10AM +0100, Knut Omang wrote:
> > 
> > > > I like the ability to add more checkers and keep then in the main
> > > > upstream tree. But adding overrides for specific subsystems goes against
> > > > the policy that all subsystems should be treated equally.
> > > 
> > > This is a tool to enable automated testing for as many checks as
> > > possible, as soon as possible. Like any tool, it can be misused, but
> > > that's IMHO an orthogonal problem that I think the maintainers will
> > > be more than capable of preventing.
> > > 
> > > Think of this as a tightening screw: We eliminate errors class by
> > > class or file by file, and in the same commit narrows in the list of
> > > exceptions. That way we can fix issues piece by piece while avoiding
> > > a lot of regressions in already clean parts.
> > 
> > Since you used drivers/infiniband as an example for this script..
> > 
> > I will say I agree with this idea.
> > 
> > It is not that we *want* infiniband to be different from the rest of
> > the kernel, it is that we have this historical situation where we
> > don't have a code base that already passes the various static checker
> > things.
> > 
> > I would like it very much if I could run 'make static checker' and see
> > no warnings. This helps me know that I when I accept patches I am not
> > introducing new problems to code that has already been cleaned up.
> > 
> > Today when we run checkers we get so many warnings it is too hard to
> > make any sense of it.
> 
> Here is a list of the checkpatch messages for drivers/infiniband
> sorted by type.
> 
> Many of these might be corrected by using
> 
> $ ./scripts/checkpatch.pl -f --fix-inplace --types= \
>   $(git ls-files drivers/infiniband/)

Yes, and I already did that work piece by piece for individual types,
just to test the runchecks tool, and want to post that set once the 
runchecks script and Makefile changes itself are in,

see:

https://github.com/torvalds/linux/compare/master...knuto:runchecks

What I personally like about this approach is that with the runchecks.cfg file
one can focus easily on one or two check types at a time, create a commit out 
of that
and at the same time "tighten" runchecks.cfg - changes in that file then serves 
as 
documentation for what got changed and use maintainers can use comments to 
indicate 
why the remaining exceptions are still there - and there's a one-stop for 
anyone 
wanting to contribute to checkpatch/sparse/doc fixes.

And it will be possible to run bisect with make C=2 and always have 0 errors.

Thanks,
Knut
> 
>5243 CHECK:CAMELCASE
>4487 WARNING:LONG_LINE
>1755 CHECK:PARENTHESIS_ALIGNMENT
>1664 CHECK:SPACING
> 910 WARNING:FUNCTION_ARGUMENTS
> 742 CHECK:OPEN_ENDED_LINE
> 685 CHECK:BRACES
> 643 CHECK:UNNECESSARY_PARENTHESES
> 478 WARNING:SIZEOF_PARENTHESIS
> 361 WARNING:UNSPECIFIED_INT
> 342 WARNING:LONG_LINE_COMMENT
> 338 ERROR:SPACING
> 338 CHECK:LINE_SPACING
> 306 WARNING:SPLIT_STRING
> 278 WARNING:SPACING
> 242 WARNING:SYMBOLIC_PERMS
> 194 WARNING:BLOCK_COMMENT_STYLE
> 175 CHECK:BIT_MACRO
> 158 WARNING:SPACE_BEFORE_TAB
> 154 WARNING:LINE_SPACING
> 139 CHECK:MACRO_ARG_REUSE
> 133 CHECK:UNCOMMENTED_DEFINITION
> 122 CHECK:AVOID_BUG
> 103 CHECK:COMPARISON_TO_NULL
> 101 WARNING:ENOSYS
>  89 WARNING:BRACES
>  78 WARNING:PREFER_PR_LEVEL
>  74 WARNING:MULTILINE_DEREFERENCE
>  59 CHECK:TYPO_SPELLING
>  52 WARNING:EMBEDDED_FUNCTION_NAME
>  52 CHECK:MULTIPLE_ASSIGNMENTS
>  50 CHECK:PREFER_KERNEL_TYPES
>  45 WARNING:RETURN_VOID
>  39 WARNING:UNNECESSARY_ELSE
>  38 ERROR:POINTER_LOCATION
>  37 WARNING:ALLOC_WITH_MULTIPLY
>  36 CHECK:ALLOC_SIZEOF_STRUCT
>  35 CHECK:AVOID_EXTERNS
>  34 WARNING:PRINTK_WITHOUT_KERN_LEVEL
>  33 ERROR:CODE_INDENT
>  32 WARNING:PREFER_PACKED
>  32 CHECK:LOGICAL_CONTINUATIONS
>  29 WARNING:MEMORY_BARRIER
>  29 WARNING:LEADING_SPACE
>  28 WARNING:DEEP_INDENTATION
>  27 CHECK:USLEEP_RANGE
>  23 WARNING:SUSPECT_CODE_INDENT
>  23 ERROR:TRAILING_STATEMENTS
>  21 WARNING:LONG_LINE_STRING
>  20 WARNING:CONSIDER_KSTRTO
>  18 WARNING:CONSTANT_COMPARISON
>  18 ERROR:OPEN_BRACE
>  15 WARNING:QUOTED_WHITESPACE_BEFORE_NEWLINE
>  14 WARNING:VOLATILE
>  14 ERROR:SWITCH_CASE_INDENT_LEVEL
>  11 WARNING:OOM_MESSAGE
>  11 WARNING:INCLUDE_LINUX
>  10 WARNING:SSCANF_TO_KSTRTO
>  10 WARNING:INDENTED_LABEL
>   9 ERROR:GLOBAL_INITIALISERS
>   9 ERROR:COMPLEX_MACRO
>   9 ERROR:ASSIGN_IN_IF
>   8 WARNING:UNNECESSARY_BREAK
>   6 WARNING:PRINTF_L
>   6 WARNING:MISORDERED_TYPE
>   6 ERROR:INITIALISED_STATIC
>   5 WARNING:TABSTOP
>   5 WARNING:SINGLE_STATEMENT_DO_WHILE_MACRO
>   5 WARNING:NAKED_SSCANF

[PATCH v4 06/18] docs: kernel-doc.rst: add documentation about man pages

[Mauro Carvalho Chehab]

kernel-doc-nano-HOWTO.txt has a chapter about man pages
production. While we don't have a working  "make manpages"
target, add it.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/doc-guide/kernel-doc.rst | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/Documentation/doc-guide/kernel-doc.rst 
b/Documentation/doc-guide/kernel-doc.rst
index b178857866f8..14c226e8154f 100644
--- a/Documentation/doc-guide/kernel-doc.rst
+++ b/Documentation/doc-guide/kernel-doc.rst
@@ -452,3 +452,37 @@ file.
 
 Data structures visible in kernel include files should also be documented using
 kernel-doc formatted comments.
+
+How to use kernel-doc to generate man pages
+---
+
+If you just want to use kernel-doc to generate man pages you can do this
+from the Kernel git tree::
+
+  $ scripts/kernel-doc -man $(git grep -l '/\*\*' |grep -v Documentation/) | 
./split-man.pl /tmp/man
+
+Using the small ``split-man.pl`` script below::
+
+
+  #!/usr/bin/perl
+
+  if ($#ARGV < 0) {
+ die "where do I put the results?\n";
+  }
+
+  mkdir $ARGV[0],0777;
+  $state = 0;
+  while () {
+  if (/^\.TH \"[^\"]*\" 9 \"([^\"]*)\"/) {
+   if ($state == 1) { close OUT }
+   $state = 1;
+   $fn = "$ARGV[0]/$1.9";
+   print STDERR "Creating $fn\n";
+   open OUT, ">$fn" or die "can't open $fn: $!\n";
+   print OUT $_;
+  } elsif ($state != 0) {
+   print OUT $_;
+  }
+  }
+
+  close OUT;
-- 
2.14.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 v4 04/18] docs: kernel-doc.rst: improve structs chapter

[Mauro Carvalho Chehab]

There is a mess on this chapter: it suggests that even
enums and unions should be documented with "struct". That's
not the way it should be ;-)

Fix it and move it to happen earlier.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/doc-guide/kernel-doc.rst | 48 +-
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/Documentation/doc-guide/kernel-doc.rst 
b/Documentation/doc-guide/kernel-doc.rst
index 3aac228fc346..e3e82f8f4de5 100644
--- a/Documentation/doc-guide/kernel-doc.rst
+++ b/Documentation/doc-guide/kernel-doc.rst
@@ -258,6 +258,30 @@ named ``Return``.
  as a new section heading, with probably won't produce the desired
  effect.
 
+Structure, union, and enumeration documentation
+---
+
+The general format of a struct, union, and enum kernel-doc comment is::
+
+  /**
+   * struct struct_name - Brief description.
+   * @argument: Description of member member_name.
+   *
+   * Description of the structure.
+   */
+
+On the above, ``struct`` is used to mean structs. You can also use ``union``
+and ``enum``  to describe unions and enums. ``argument`` is used
+to mean struct and union member names as well as enumerations in an enum.
+
+The brief description following the structure name may span multiple lines, and
+ends with a member description, a blank comment line, or the end of the
+comment block.
+
+The kernel-doc data structure comments describe each member of the structure,
+in order, with the member descriptions.
+
+
 
 Highlights and cross-references
 ---
@@ -331,30 +355,6 @@ cross-references.
 For further details, please refer to the `Sphinx C Domain`_ documentation.
 
 
-Structure, union, and enumeration documentation

-
-The general format of a struct, union, and enum kernel-doc comment is::
-
-  /**
-   * struct struct_name - Brief description.
-   * @member_name: Description of member member_name.
-   *
-   * Description of the structure.
-   */
-
-Below, "struct" is used to mean structs, unions and enums, and "member" is used
-to mean struct and union members as well as enumerations in an enum.
-
-The brief description following the structure name may span multiple lines, and
-ends with a ``@member:`` description, a blank comment line, or the end of the
-comment block.
-
-The kernel-doc data structure comments describe each member of the structure, 
in
-order, with the ``@member:`` descriptions. The ``@member:`` descriptions must
-begin on the very next line following the opening brief function description
-line, with no intervening blank comment lines. The ``@member:`` descriptions 
may
-span multiple lines. The continuation lines may contain indentation.
 
 In-line member documentation comments
 ~
-- 
2.14.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 v4 10/18] scripts: kernel-doc: change default to ReST format

[Mauro Carvalho Chehab]

Right now, if kernel-doc is called without arguments, it
defaults to man pages. IMO, it makes more sense to
default to ReST, as this is the output that it is most
used nowadays, and it easier to check if everything got
parsed fine on an enriched text mode format.

Signed-off-by: Mauro Carvalho Chehab 
---
 scripts/kernel-doc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 11aec7469776..e417d93575b9 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -268,12 +268,12 @@ my $kernelversion;
 my $dohighlight = "";
 
 my $verbose = 0;
-my $output_mode = "man";
+my $output_mode = "rst";
 my $output_preformatted = 0;
 my $no_doc_sections = 0;
 my $enable_lineno = 0;
-my @highlights = @highlights_man;
-my $blankline = $blankline_man;
+my @highlights = @highlights_rst;
+my $blankline = $blankline_rst;
 my $modulename = "Kernel API";
 
 use constant {
-- 
2.14.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 v4 13/18] scripts: kernel-doc: get rid of $nested parameter

[Mauro Carvalho Chehab]

The check_sections() function has a $nested parameter, meant
to identify when a nested struct is present. As we now have
a logic that handles it, get rid of such parameter.

Suggested-by: Markus Heiser 
Signed-off-by: Mauro Carvalho Chehab 
---
 scripts/kernel-doc | 19 ---
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index b0eb1cd6afbe..fadb832733d9 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1003,7 +1003,6 @@ sub dump_union($$) {
 sub dump_struct($$) {
 my $x = shift;
 my $file = shift;
-my $nested;
 
 if ($x =~ /(struct|union)\s+(\w+)\s*{(.*)}/) {
my $decl_type = $1;
@@ -1063,11 +1062,9 @@ sub dump_struct($$) {
 
# Ignore other nested elements, like enums
$members =~ s/({[^\{\}]*})//g;
-   $nested = $decl_type;
-   $nested =~ s/\/\*.*?\*\///gos;
 
create_parameterlist($members, ';', $file);
-   check_sections($file, $declaration_name, $decl_type, $sectcheck, 
$struct_actual, $nested);
+   check_sections($file, $declaration_name, $decl_type, $sectcheck, 
$struct_actual);
 
# Adjust declaration for better display
$declaration =~ s/([{;])/$1\n/g;
@@ -1372,8 +1369,8 @@ sub push_parameter($$$) {
$parametertypes{$param} = $type;
 }
 
-sub check_sections($$) {
-   my ($file, $decl_name, $decl_type, $sectcheck, $prmscheck, $nested) = 
@_;
+sub check_sections($) {
+   my ($file, $decl_name, $decl_type, $sectcheck, $prmscheck) = @_;
my @sects = split ' ', $sectcheck;
my @prms = split ' ', $prmscheck;
my $err;
@@ -1407,14 +1404,6 @@ sub check_sections($$) {
"'$sects[$sx]' " .
"description in '$decl_name'\n";
++$warnings;
-   } else {
-   if ($nested !~ m/\Q$sects[$sx]\E/) {
-   print STDERR "${file}:$.: warning: " .
-   "Excess $decl_type member " .
-   "'$sects[$sx]' " .
-   "description in '$decl_name'\n";
-   ++$warnings;
-   }
}
}
}
@@ -1525,7 +1514,7 @@ sub dump_function($$) {
 }
 
my $prms = join " ", @parameterlist;
-   check_sections($file, $declaration_name, "function", $sectcheck, $prms, 
"");
+   check_sections($file, $declaration_name, "function", $sectcheck, $prms);
 
 # This check emits a lot of warnings at the moment, because many
 # functions don't have a 'Return' doc section. So until the number
-- 
2.14.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 v4 16/18] scripts: kernel-doc: improve nested logic to handle multiple identifiers

[Mauro Carvalho Chehab]

It is possible to use nested structs like:

struct {
struct {
void *arg1;
} st1, st2, *st3, st4;
};

Handling it requires to split each parameter. Change the logic
to allow such definitions.

In order to test the new nested logic, the following file
was used to test


struct foo { int a; }; /* Just to avoid errors if compiled */

/**
 * struct my_struct - a struct with nested unions and structs
 * @arg1: first argument of anonymous union/anonymous struct
 * @arg2: second argument of anonymous union/anonymous struct
 * @arg1b: first argument of anonymous union/anonymous struct
 * @arg2b: second argument of anonymous union/anonymous struct
 * @arg3: third argument of anonymous union/anonymous struct
 * @arg4: fourth argument of anonymous union/anonymous struct
 * @bar.st1.arg1: first argument of struct st1 on union bar
 * @bar.st1.arg2: second argument of struct st1 on union bar
 * @bar.st1.bar1: bar1 at st1
 * @bar.st1.bar2: bar2 at st1
 * @bar.st2.arg1: first argument of struct st2 on union bar
 * @bar.st2.arg2: second argument of struct st2 on union bar
 * @bar.st3.arg2: second argument of struct st3 on union bar
 * @f1: nested function on anonimous union/struct
 * @bar.st2.f2: nested function on named union/struct
 */
struct my_struct {
   /* Anonymous union/struct*/
   union {
struct {
char arg1 : 1;
char arg2 : 3;
};
   struct {
   int arg1b;
   int arg2b;
   };
   struct {
   void *arg3;
   int arg4;
   int (*f1)(char foo, int bar);
   };
   };
   union {
   struct {
   int arg1;
   int arg2;
   struct foo bar1, *bar2;
   } st1;   /* bar.st1 is undocumented, cause a warning */
   struct {
   void *arg1;  /* bar.st3.arg1 is undocumented, cause a warning */
int arg2;
  int (*f2)(char foo, int bar); /* bar.st3.fn2 is undocumented, cause a 
warning */
   } st2, st3, *st4;
   int (*f3)(char foo, int bar); /* f3 is undocumented, cause a warning */
   } bar;   /* bar is undocumented, cause a warning */

   /* private: */
   int undoc_privat;/* is undocumented but private, no warning */

   /* public: */
   int undoc_public;/* is undocumented, cause a warning */
};


It produces the following warnings, as expected:

test2.h:57: warning: Function parameter or member 'bar' not described in 
'my_struct'
test2.h:57: warning: Function parameter or member 'bar.st1' not described in 
'my_struct'
test2.h:57: warning: Function parameter or member 'bar.st2' not described in 
'my_struct'
test2.h:57: warning: Function parameter or member 'bar.st3' not described in 
'my_struct'
test2.h:57: warning: Function parameter or member 'bar.st3.arg1' not described 
in 'my_struct'
test2.h:57: warning: Function parameter or member 'bar.st3.f2' not described in 
'my_struct'
test2.h:57: warning: Function parameter or member 'bar.st4' not described in 
'my_struct'
test2.h:57: warning: Function parameter or member 'bar.st4.arg1' not described 
in 'my_struct'
test2.h:57: warning: Function parameter or member 'bar.st4.arg2' not described 
in 'my_struct'
test2.h:57: warning: Function parameter or member 'bar.st4.f2' not described in 
'my_struct'
test2.h:57: warning: Function parameter or member 'bar.f3' not described in 
'my_struct'
test2.h:57: warning: Function parameter or member 'undoc_public' not described 
in 'my_struct'

Suggested-by: Markus Heiser 
Signed-off-by: Mauro Carvalho Chehab 
---
 scripts/kernel-doc | 69 ++
 1 file changed, 44 insertions(+), 25 deletions(-)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 5d03c9086323..0bda21d9d3f2 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1030,15 +1030,16 @@ sub dump_struct($$) {
my $declaration = $members;
 
# Split nested struct/union elements as newer ones
-   my $cont = 1;
-   while ($cont) {
-   $cont = 0;
-   while ($members =~ 
m/(struct|union)([^{};]+){([^{}]*)}([^{}\;]*)\;/) {
-   my $newmember = "$1 $4;";
-   my $id = $4;
-   my $content = $3;
+   while ($members =~ 
m/(struct|union)([^\{\};]+)\{([^\{\}]*)\}([^\{\}\;]*)\;/) {
+   my $newmember;
+   my $maintype = $1;
+   my $ids = $4;
+   my $content = $3;
+   foreach my $id(split /,/, $ids) {
+   $newmember .= "$maintype $id; ";
+
$id =~ s/[:\[].*//;
-   $id =~ s/^\*+//;
+   $id =~ s/^\s*\**(\S+)\s*/$1/;
foreach my $arg (split /;/, $content) {
next if ($arg =~ m/^\s*$/);
if ($arg =~ 
m/^([^\(]+\(\*?\s*)([\w\.]*)(\s*\).*)/) {
@@ 

[PATCH v4 03/18] docs: kernel-doc.rst: improve function documentation section

[Mauro Carvalho Chehab]

Move its contents to happen earlier and improve the description
of return values, adding a subsection to it. Most of the contents
there came from kernel-doc-nano-HOWTO.txt.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/doc-guide/kernel-doc.rst | 100 -
 1 file changed, 61 insertions(+), 39 deletions(-)

diff --git a/Documentation/doc-guide/kernel-doc.rst 
b/Documentation/doc-guide/kernel-doc.rst
index 7cf58c3489de..3aac228fc346 100644
--- a/Documentation/doc-guide/kernel-doc.rst
+++ b/Documentation/doc-guide/kernel-doc.rst
@@ -197,6 +197,67 @@ Example::
   int d;
   };
 
+Function documentation
+--
+
+The general format of a function and function-like macro kernel-doc comment 
is::
+
+  /**
+   * function_name() - Brief description of function.
+   * @arg1: Describe the first argument.
+   * @arg2: Describe the second argument.
+   *One can provide multiple line descriptions
+   *for arguments.
+   *
+   * A longer description, with more discussion of the function function_name()
+   * that might be useful to those using or modifying it. Begins with an
+   * empty comment line, and may include additional embedded empty
+   * comment lines.
+   *
+   * The longer description may have multiple paragraphs.
+   *
+   * Return: Describe the return value of foobar.
+   *
+   * The return value description can also have multiple paragraphs, and should
+   * be placed at the end of the comment block.
+   */
+
+The brief description following the function name may span multiple lines, and
+ends with an argument description, a blank comment line, or the end of the
+comment block.
+
+Return values
+~
+
+The return value, if any, should be described in a dedicated section
+named ``Return``.
+
+.. note::
+
+  #) The multi-line descriptive text you provide does *not* recognize
+ line breaks, so if you try to format some text nicely, as in::
+
+   * Return:
+   * 0 - OK
+   * -EINVAL - invalid argument
+   * -ENOMEM - out of memory
+
+ this will all run together and produce::
+
+   Return: 0 - OK -EINVAL - invalid argument -ENOMEM - out of memory
+
+ So, in order to produce the desired line breaks, you need to use a
+ ReST list, e. g.::
+
+  * Return:
+  * * 0- OK to runtime suspend the device
+  * * -EBUSY   - Device should not be runtime suspended
+
+  #) If the descriptive text you provide has lines that begin with
+ some phrase followed by a colon, each of those phrases will be taken
+ as a new section heading, with probably won't produce the desired
+ effect.
+
 
 Highlights and cross-references
 ---
@@ -269,45 +330,6 @@ cross-references.
 
 For further details, please refer to the `Sphinx C Domain`_ documentation.
 
-Function documentation
---
-
-The general format of a function and function-like macro kernel-doc comment 
is::
-
-  /**
-   * function_name() - Brief description of function.
-   * @arg1: Describe the first argument.
-   * @arg2: Describe the second argument.
-   *One can provide multiple line descriptions
-   *for arguments.
-   *
-   * A longer description, with more discussion of the function function_name()
-   * that might be useful to those using or modifying it. Begins with an
-   * empty comment line, and may include additional embedded empty
-   * comment lines.
-   *
-   * The longer description may have multiple paragraphs.
-   *
-   * Return: Describe the return value of foobar.
-   *
-   * The return value description can also have multiple paragraphs, and should
-   * be placed at the end of the comment block.
-   */
-
-The brief description following the function name may span multiple lines, and
-ends with an ``@argument:`` description, a blank comment line, or the end of 
the
-comment block.
-
-The kernel-doc function comments describe each parameter to the function, in
-order, with the ``@argument:`` descriptions. The ``@argument:`` descriptions
-must begin on the very next line following the opening brief function
-description line, with no intervening blank comment lines. The ``@argument:``
-descriptions may span multiple lines. The continuation lines may contain
-indentation. If a function parameter is ``...`` (varargs), it should be listed
-in kernel-doc notation as: ``@...:``.
-
-The return value, if any, should be described in a dedicated section at the end
-of the comment starting with "Return:".
 
 Structure, union, and enumeration documentation
 ---
-- 
2.14.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 v4 18/18] w1_netlink.h: add support for nested structs

[Mauro Carvalho Chehab]

Now that kernel-doc can hanle nested structs/unions, describe
such fields at w1_netlink_message_types.

Acked-by: Evgeniy Polyakov 
Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/w1/w1_netlink.h | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/w1/w1_netlink.h b/drivers/w1/w1_netlink.h
index a36661cd1f05..f876772c0fb4 100644
--- a/drivers/w1/w1_netlink.h
+++ b/drivers/w1/w1_netlink.h
@@ -59,7 +59,11 @@ enum w1_netlink_message_types {
  * @type: one of enum w1_netlink_message_types
  * @status: kernel feedback for success 0 or errno failure value
  * @len: length of data following w1_netlink_msg
- * @id: union holding master bus id (msg.id) and slave device id (id[8]).
+ * @id: union holding bus master id (msg.id) and slave device id (id[8]).
+ * @id.id: Slave ID (8 bytes)
+ * @id.mst: bus master identification
+ * @id.mst.id: bus master ID
+ * @id.mst.res: bus master reserved
  * @data: start address of any following data
  *
  * The base message structure for w1 messages over netlink.
-- 
2.14.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 v4 09/18] scripts: kernel-doc: improve argument handling

[Mauro Carvalho Chehab]

Right now, if one uses "--rst" instead of "-rst", it just
ignore the argument and produces a man page. Change the
logic to accept both "-cmd" and "--cmd". Also, if
"cmd" doesn't exist, print the usage information and exit.

Signed-off-by: Mauro Carvalho Chehab 
---
 scripts/kernel-doc | 36 
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index c8ad05eb3e5e..11aec7469776 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -391,47 +391,51 @@ my $undescribed = "-- undescribed --";
 
 reset_state();
 
-while ($ARGV[0] =~ m/^-(.*)/) {
-my $cmd = shift @ARGV;
-if ($cmd eq "-man") {
+while ($ARGV[0] =~ m/^--?(.*)/) {
+my $cmd = $1;
+shift @ARGV;
+if ($cmd eq "man") {
$output_mode = "man";
@highlights = @highlights_man;
$blankline = $blankline_man;
-} elsif ($cmd eq "-rst") {
+} elsif ($cmd eq "rst") {
$output_mode = "rst";
@highlights = @highlights_rst;
$blankline = $blankline_rst;
-} elsif ($cmd eq "-none") {
+} elsif ($cmd eq "none") {
$output_mode = "none";
-} elsif ($cmd eq "-module") { # not needed for XML, inherits from calling 
document
+} elsif ($cmd eq "module") { # not needed for XML, inherits from calling 
document
$modulename = shift @ARGV;
-} elsif ($cmd eq "-function") { # to only output specific functions
+} elsif ($cmd eq "function") { # to only output specific functions
$output_selection = OUTPUT_INCLUDE;
$function = shift @ARGV;
$function_table{$function} = 1;
-} elsif ($cmd eq "-nofunction") { # output all except specific functions
+} elsif ($cmd eq "nofunction") { # output all except specific functions
$output_selection = OUTPUT_EXCLUDE;
$function = shift @ARGV;
$function_table{$function} = 1;
-} elsif ($cmd eq "-export") { # only exported symbols
+} elsif ($cmd eq "export") { # only exported symbols
$output_selection = OUTPUT_EXPORTED;
%function_table = ();
-} elsif ($cmd eq "-internal") { # only non-exported symbols
+} elsif ($cmd eq "internal") { # only non-exported symbols
$output_selection = OUTPUT_INTERNAL;
%function_table = ();
-} elsif ($cmd eq "-export-file") {
+} elsif ($cmd eq "export-file") {
my $file = shift @ARGV;
push(@export_file_list, $file);
-} elsif ($cmd eq "-v") {
+} elsif ($cmd eq "v") {
$verbose = 1;
-} elsif (($cmd eq "-h") || ($cmd eq "--help")) {
+} elsif (($cmd eq "h") || ($cmd eq "help")) {
usage();
-} elsif ($cmd eq '-no-doc-sections') {
+} elsif ($cmd eq 'no-doc-sections') {
$no_doc_sections = 1;
-} elsif ($cmd eq '-enable-lineno') {
+} elsif ($cmd eq 'enable-lineno') {
$enable_lineno = 1;
-} elsif ($cmd eq '-show-not-found') {
+} elsif ($cmd eq 'show-not-found') {
$show_not_found = 1;
+} else {
+   # Unknown argument
+usage();
 }
 }
 
-- 
2.14.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 v4 12/18] scripts: kernel-doc: parse next structs/unions

[Mauro Carvalho Chehab]

There are several places within the Kernel tree with nested
structs/unions, like this one:

  struct ingenic_cgu_clk_info {
const char *name;
enum {
  CGU_CLK_NONE = 0,
  CGU_CLK_EXT = BIT(0),
  CGU_CLK_PLL = BIT(1),
  CGU_CLK_GATE = BIT(2),
  CGU_CLK_MUX = BIT(3),
  CGU_CLK_MUX_GLITCHFREE = BIT(4),
  CGU_CLK_DIV = BIT(5),
  CGU_CLK_FIXDIV = BIT(6),
  CGU_CLK_CUSTOM = BIT(7),
} type;
int parents[4];
union {
  struct ingenic_cgu_pll_info pll;
  struct {
struct ingenic_cgu_gate_info gate;
struct ingenic_cgu_mux_info mux;
struct ingenic_cgu_div_info div;
struct ingenic_cgu_fixdiv_info fixdiv;
  };
  struct ingenic_cgu_custom_info custom;
};
  };

Currently, such struct is documented as:

**Definition**

::
struct ingenic_cgu_clk_info {
const char * name;
};

**Members**

``name``
  name of the clock

With is obvioulsy wrong. It also generates an error:
drivers/clk/ingenic/cgu.h:169: warning: No description found for 
parameter 'enum'

However, there's nothing wrong with this kernel-doc markup: everything
is documented there.

It makes sense to document all fields there. So, add a
way for the core to parse those structs.

With this patch, all documented fields will properly generate
documentation.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/doc-guide/kernel-doc.rst |  46 +
 scripts/kernel-doc | 121 ++---
 2 files changed, 114 insertions(+), 53 deletions(-)

diff --git a/Documentation/doc-guide/kernel-doc.rst 
b/Documentation/doc-guide/kernel-doc.rst
index 14c226e8154f..722d4525f7cf 100644
--- a/Documentation/doc-guide/kernel-doc.rst
+++ b/Documentation/doc-guide/kernel-doc.rst
@@ -281,6 +281,52 @@ comment block.
 The kernel-doc data structure comments describe each member of the structure,
 in order, with the member descriptions.
 
+Nested structs/unions
+~
+
+It is possible to document nested structs unions, like::
+
+  /**
+   * struct nested_foobar - a struct with nested unions and structs
+   * @arg1: - first argument of anonymous union/anonymous struct
+   * @arg2: - second argument of anonymous union/anonymous struct
+   * @arg3: - third argument of anonymous union/anonymous struct
+   * @arg4: - fourth argument of anonymous union/anonymous struct
+   * @bar.st1.arg1 - first argument of struct st1 on union bar
+   * @bar.st1.arg2 - second argument of struct st1 on union bar
+   * @bar.st2.arg1 - first argument of struct st2 on union bar
+   * @bar.st2.arg2 - second argument of struct st2 on union bar
+  struct nested_foobar {
+/* Anonymous union/struct*/
+union {
+  struct {
+int arg1;
+int arg2;
+ }
+  struct {
+void *arg3;
+int arg4;
+ }
+   }
+   union {
+  struct {
+int arg1;
+int arg2;
+ } st1;
+  struct {
+void *arg1;
+int arg2;
+ } st2;
+   } bar;
+  };
+
+.. note::
+
+   #) When documenting nested structs or unions, if the struct/union ``foo``
+  is named, the argument ``bar`` inside it should be documented as
+  ``@foo.bar:``
+   #) When the nested struct/union is anonymous, the argument ``bar`` on it
+  should be documented as ``@bar:``
 
 Typedef documentation
 -
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 05aadac0612a..b0eb1cd6afbe 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -211,7 +211,7 @@ my $anon_struct_union = 0;
 my $type_constant = '\b``([^\`]+)``\b';
 my $type_constant2 = '\%([-_\w]+)';
 my $type_func = '(\w+)\(\)';
-my $type_param = '\@(\w+(\.\.\.)?)';
+my $type_param = '\@(\w*(\.\w+)*(\.\.\.)?)';
 my $type_fp_param = '\@(\w+)\(\)';  # Special RST handling for func ptr params
 my $type_env = '(\$\w+)';
 my $type_enum = '\&(enum\s*([_\w]+))';
@@ -670,32 +670,12 @@ sub output_struct_man(%) {
 print ".SH NAME\n";
 print $args{'type'} . " " . $args{'struct'} . " \\- " . $args{'purpose'} . 
"\n";
 
+my $declaration = $args{'definition'};
+$declaration =~ s/\t/  /g;
+$declaration =~ s/\n/"\n.br\n.BI \"/g;
 print ".SH SYNOPSIS\n";
 print $args{'type'} . " " . $args{'struct'} . " {\n.br\n";
-
-foreach my $parameter (@{$args{'parameterlist'}}) {
-   if ($parameter =~ /^#/) {
-   print ".BI \"$parameter\"\n.br\n";
-   next;
-   }
-   my $parameter_name = $parameter;
-   $parameter_name =~ s/\[.*//;
-
-   ($args{'parameterdescs'}{$parameter_name} ne $undescribed) || next;
-   $type = $args{'parametertypes'}{$parameter};
-   if ($type =~ m/([^\(]*\(\*)\s*\)\s*\(([^\)]*)\)/) {
-   # pointer-to-function
-   print 

[PATCH v4 08/18] scripts: kernel-doc: get rid of unused output formats

[Mauro Carvalho Chehab]

Since there isn't any docbook code anymore upstream,
we can get rid of several output formats:

- docbook/xml, html, html5 and list formats were used by
  the old build system;
- As ReST is text, there's not much sense on outputting
  on a different text format.

After this patch, only man and rst output formats are
supported.

Signed-off-by: Mauro Carvalho Chehab 
Acked-by: Jani Nikula 
---
 scripts/kernel-doc | 1183 +---
 1 file changed, 4 insertions(+), 1179 deletions(-)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index bed74e9bc141..c8ad05eb3e5e 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -51,13 +51,8 @@ The documentation comments are identified by "/**" opening 
comment mark. See
 Documentation/doc-guide/kernel-doc.rst for the documentation comment syntax.
 
 Output format selection (mutually exclusive):
-  -docbook Output DocBook format.
-  -htmlOutput HTML format.
-  -html5   Output HTML5 format.
-  -listOutput symbol list format. This is for use by 
docproc.
   -man Output troff manual page format. This is the default.
   -rst Output reStructuredText format.
-  -textOutput plain text format.
   -noneDo not output documentation, only warnings.
 
 Output selection (mutually exclusive):
@@ -225,84 +220,11 @@ my $type_typedef = '\&(typedef\s*([_\w]+))';
 my $type_union = '\&(union\s*([_\w]+))';
 my $type_member = '\&([_\w]+)(\.|->)([_\w]+)';
 my $type_fallback = '\&([_\w]+)';
-my $type_enum_xml = '\(enum\s*([_\w]+))';
-my $type_struct_xml = '\(struct\s*([_\w]+))';
-my $type_typedef_xml = '\(typedef\s*([_\w]+))';
-my $type_union_xml = '\(union\s*([_\w]+))';
-my $type_member_xml = '\([_\w]+)(\.|-\)([_\w]+)';
-my $type_fallback_xml = '\([_\w]+)';
 my $type_member_func = $type_member . '\(\)';
 
 # Output conversion substitutions.
 #  One for each output format
 
-# these work fairly well
-my @highlights_html = (
-   [$type_constant, "\$1"],
-   [$type_constant2, "\$1"],
-   [$type_func, "\$1"],
-   [$type_enum_xml, "\$1"],
-   [$type_struct_xml, "\$1"],
-   [$type_typedef_xml, "\$1"],
-   [$type_union_xml, "\$1"],
-   [$type_env, "\$1"],
-   [$type_param, "\$1"],
-   [$type_member_xml, "\$1\$2\$3"],
-   [$type_fallback_xml, "\$1"]
-  );
-my $local_lt = "lt:";
-my $local_gt = "gt:";
-my $blankline_html = $local_lt . "p" . $local_gt;  # was ""
-
-# html version 5
-my @highlights_html5 = (
-[$type_constant, "\$1"],
-[$type_constant2, "\$1"],
-[$type_func, "\$1"],
-[$type_enum_xml, "\$1"],
-[$type_struct_xml, "\$1"],
-[$type_typedef_xml, "\$1"],
-[$type_union_xml, "\$1"],
-[$type_env, "\$1"],
-[$type_param, "\$1]"],
-[$type_member_xml, "\$1\$2\$3"],
-[$type_fallback_xml, "\$1"]
-  );
-my $blankline_html5 = $local_lt . "br /" . $local_gt;
-
-# XML, docbook format
-my @highlights_xml = (
-  ["([^=])\\\"([^\\\"<]+)\\\"", "\$1\$2"],
-  [$type_constant, "\$1"],
-  [$type_constant2, "\$1"],
-  [$type_enum_xml, "\$1"],
-  [$type_struct_xml, "\$1"],
-  [$type_typedef_xml, "\$1"],
-  [$type_union_xml, "\$1"],
-  [$type_param, "\$1"],
-  [$type_func, "\$1"],
-  [$type_env, "\$1"],
-  [$type_member_xml, 
"\$1\$2\$3"],
-  [$type_fallback_xml, "\$1"]
-);
-my $blankline_xml = $local_lt . "/para" . $local_gt . $local_lt . "para" . 
$local_gt . "\n";
-
-# gnome, docbook format
-my @highlights_gnome = (
-[$type_constant, "\$1"],
-[$type_constant2, "\$1"],
-[$type_func, "\$1"],
-[$type_enum, "\$1"],
-[$type_struct, "\$1"],
-[$type_typedef, "\$1"],
-[$type_union, "\$1"],
-[$type_env, "\$1"],
-[$type_param, "\$1" ],
-[$type_member, 
"\$1\$2\$3"],
-[$type_fallback, "\$1"]
-  );
-my $blankline_gnome = "\n";
-
 # these are pretty rough
 my @highlights_man = (
   

[PATCH v4 00/18] kernel-doc: add supported to document nested structs

[Mauro Carvalho Chehab]

Hi Jon,

This is a rebased version of my patch series that add support for
nested structs on kernel-doc. With this version, it won't produce anymore
hundreds of identical warnings, as patch 17 removes the warning
duplication.

Excluding warnings about duplicated Note: section at hash.h, before
this series, it reports 166 kernel-doc warnings. After this patch series,
it reports 123 kernel-doc warnings, being 51 from DVB. I have already a patch
series that will cleanup those new DVB warnings due to nested structs.

So, the net result is that the number of warnings is reduced with
this version.

-

Right now, it is not possible to document nested struct and nested unions.
kernel-doc simply ignore them.

Add support to document them.

Patches 1 to 6 improve kernel-doc documentation to reflect what
kernel-doc currently supports and import some stuff from the
old kernel-doc-nano-HOWTO.txt.

Patch 7 gets rid of the old documentation (kernel-doc-nano-HOWTO.txt).

Patch 8 gets rid of the now unused output formats for kernel-doc: since 
we got rid of all DocBook stuff, we should not need them anymore. The
reason for dropping it (despite cleaning up), is that it doesn't make 
sense to invest time on adding new features for formats that aren't
used anymore.

Patch 9 improves argument handling, printing script usage if an
invalid argument is passed and accepting both -cmd and --cmd
forms.

Patch 10 changes the default output format to ReST, as this is a way
more used than man output nowadays.

Patch 11 solves an issue after ReST conversion, where tabs were not
properly handled on kernel-doc tags.

Patch 12 adds support for parsing kernel-doc nested structures and 
unions.

Patch 13 does a cleanup, removing $nexted parameter at the
routines that handle structs.

Patch 14 Improves warning output by printing the identifier where
the warning occurred.

Patch 15 complements patch 12, by adding support for function
definitions inside nested structures. It is needed by some media
docs with use such kind of things.

Patch 16 improves nested struct handling even further, allowing it
to handle cases where a nested named struct/enum with multiple
identifiers added (e. g. struct { ... } foo, bar;).

Patch 17 avoid warnings when -function or -nofunction is used and
the symbol to be warned is not listed.

Patch 18 adds documentation for nested union/struct inside w1_netlink.

The entire patch series are at my development tree, at:
https://git.linuxtv.org/mchehab/experimental.git/log/?h=nested-fix-v4b

--

v4:
- Rebased on the top of Dec, 18 docs-next branch;
- Don't print multiple times the same error, when a single file is
  parsed multiple times with -function NAME or -nofunction NAME.

v3:
- rebased on the top of docs-next branch at docs git tree;
- patches reordered to a more logical sequence;
- Change script to produce ReST format by default;
- Improve argument handling;
- Accept structs with multiple identifiers.


v2:
  - solved some issues that Randy pointed;
  - added patch 10 as suggested by Markus;
  - Fixed some bugs on patch 9, after parsing nested structs
   on media subsystem;
  - added patch 11 with a warning improvement fixup;
  - added patch 12 in order to handle function parameters
   on nested structures, due to DVB demux kAPI.

v1:
  - original version.


Mauro Carvalho Chehab (18):
  docs: kernel-doc.rst: better describe kernel-doc arguments
  docs: kernel-doc.rst: improve private members description
  docs: kernel-doc.rst: improve function documentation section
  docs: kernel-doc.rst: improve structs chapter
  docs: kernel-doc.rst: improve typedef documentation
  docs: kernel-doc.rst: add documentation about man pages
  docs: get rid of kernel-doc-nano-HOWTO.txt
  scripts: kernel-doc: get rid of unused output formats
  scripts: kernel-doc: improve argument handling
  scripts: kernel-doc: change default to ReST format
  scripts: kernel-doc: replace tabs by spaces
  scripts: kernel-doc: parse next structs/unions
  scripts: kernel-doc: get rid of $nested parameter
  scripts: kernel-doc: print the declaration name on warnings
  scripts: kernel-doc: handle nested struct function arguments
  scripts: kernel-doc: improve nested logic to handle multiple
identifiers
  scripts: kernel-doc: apply filtering rules to warnings
  w1_netlink.h: add support for nested structs

 Documentation/00-INDEX  |2 -
 Documentation/doc-guide/kernel-doc.rst  |  360 +---
 Documentation/kernel-doc-nano-HOWTO.txt |  322 ---
 drivers/w1/w1_netlink.h |6 +-
 scripts/kernel-doc  | 1468 ---
 5 files changed, 435 insertions(+), 1723 deletions(-)
 delete mode 100644 Documentation/kernel-doc-nano-HOWTO.txt

-- 
2.14.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 v4 05/18] docs: kernel-doc.rst: improve typedef documentation

[Mauro Carvalho Chehab]

Add documentation about typedefs for function prototypes and
move it to happen earlier.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/doc-guide/kernel-doc.rst | 32 ++--
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/Documentation/doc-guide/kernel-doc.rst 
b/Documentation/doc-guide/kernel-doc.rst
index e3e82f8f4de5..b178857866f8 100644
--- a/Documentation/doc-guide/kernel-doc.rst
+++ b/Documentation/doc-guide/kernel-doc.rst
@@ -282,6 +282,28 @@ The kernel-doc data structure comments describe each 
member of the structure,
 in order, with the member descriptions.
 
 
+Typedef documentation
+-
+
+The general format of a typedef kernel-doc comment is::
+
+  /**
+   * typedef type_name - Brief description.
+   *
+   * Description of the type.
+   */
+
+Typedefs with function prototypes can also be documented::
+
+  /**
+   * typedef type_name - Brief description.
+   * @arg1: description of arg1
+   * @arg2: description of arg2
+   *
+   * Description of the type.
+   */
+   typedef void (*type_name)(struct v4l2_ctrl *arg1, void *arg2);
+
 
 Highlights and cross-references
 ---
@@ -384,16 +406,6 @@ on a line of their own, like all other kernel-doc 
comments::
 int foobar;
   }
 
-Typedef documentation
--
-
-The general format of a typedef kernel-doc comment is::
-
-  /**
-   * typedef type_name - Brief description.
-   *
-   * Description of the type.
-   */
 
 Overview documentation comments
 ---
-- 
2.14.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 v4 01/18] docs: kernel-doc.rst: better describe kernel-doc arguments

[Mauro Carvalho Chehab]

Add a new section to describe kernel-doc arguments,
adding examples about how identation should happen, as failing
to do that causes Sphinx to do the wrong thing.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/doc-guide/kernel-doc.rst | 44 +++---
 1 file changed, 41 insertions(+), 3 deletions(-)

diff --git a/Documentation/doc-guide/kernel-doc.rst 
b/Documentation/doc-guide/kernel-doc.rst
index 0268335414ce..f1c03c16f03b 100644
--- a/Documentation/doc-guide/kernel-doc.rst
+++ b/Documentation/doc-guide/kernel-doc.rst
@@ -112,16 +112,17 @@ Example kernel-doc function comment::
 
   /**
* foobar() - Brief description of foobar.
-   * @arg: Description of argument of foobar.
+   * @argument1: Description of parameter argument1 of foobar.
+   * @argument2: Description of parameter argument2 of foobar.
*
* Longer description of foobar.
*
* Return: Description of return value of foobar.
*/
-  int foobar(int arg)
+  int foobar(int argument1, char *argument2)
 
 The format is similar for documentation for structures, enums, paragraphs,
-etc. See the sections below for details.
+etc. See the sections below for specific details of each type.
 
 The kernel-doc structure is extracted from the comments, and proper `Sphinx C
 Domain`_ function and type descriptions with anchors are generated for them. 
The
@@ -130,6 +131,43 @@ cross-references. See below for details.
 
 .. _Sphinx C Domain: http://www.sphinx-doc.org/en/stable/domains.html
 
+
+Parameters and member arguments
+---
+
+The kernel-doc function comments describe each parameter to the function and
+function typedefs or each member of struct/union, in order, with the
+``@argument:`` descriptions. For each non-private member argument, one
+``@argument`` definition is needed.
+
+The ``@argument:`` descriptions begin on the very next line following
+the opening brief function description line, with no intervening blank
+comment lines.
+
+The ``@argument:`` descriptions may span multiple lines.
+
+.. note::
+
+   If the ``@argument`` description has multiple lines, the continuation
+   of the description should be starting exactly at the same column as
+   the previous line, e. g.::
+
+  * @argument: some long description
+  *   that continues on next lines
+
+   or::
+
+  * @argument:
+  *some long description
+  *that continues on next lines
+
+If a function or typedef parameter argument is ``...`` (e. g. a variable
+number of arguments), its description should be listed in kernel-doc
+notation as::
+
+  * @...: description
+
+
 Highlights and cross-references
 ---
 
-- 
2.14.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 v4 07/18] docs: get rid of kernel-doc-nano-HOWTO.txt

[Mauro Carvalho Chehab]

Everything there is already described at
Documentation/doc-guide/kernel-doc.rst. So, there's no reason why
to keep it anymore.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/00-INDEX  |   2 -
 Documentation/kernel-doc-nano-HOWTO.txt | 322 
 scripts/kernel-doc  |   2 +-
 3 files changed, 1 insertion(+), 325 deletions(-)
 delete mode 100644 Documentation/kernel-doc-nano-HOWTO.txt

diff --git a/Documentation/00-INDEX b/Documentation/00-INDEX
index 3bec49c33bbb..aca4f00ec69b 100644
--- a/Documentation/00-INDEX
+++ b/Documentation/00-INDEX
@@ -228,8 +228,6 @@ isdn/
- directory with info on the Linux ISDN support, and supported cards.
 kbuild/
- directory with info about the kernel build process.
-kernel-doc-nano-HOWTO.txt
-   - outdated info about kernel-doc documentation.
 kdump/
- directory with mini HowTo on getting the crash dump code to work.
 doc-guide/
diff --git a/Documentation/kernel-doc-nano-HOWTO.txt 
b/Documentation/kernel-doc-nano-HOWTO.txt
deleted file mode 100644
index c23e2c5ab80d..
--- a/Documentation/kernel-doc-nano-HOWTO.txt
+++ /dev/null
@@ -1,322 +0,0 @@
-NOTE: this document is outdated and will eventually be removed.  See
-Documentation/doc-guide/ for current information.
-
-kernel-doc nano-HOWTO
-=
-
-How to format kernel-doc comments
--
-
-In order to provide embedded, 'C' friendly, easy to maintain,
-but consistent and extractable documentation of the functions and
-data structures in the Linux kernel, the Linux kernel has adopted
-a consistent style for documenting functions and their parameters,
-and structures and their members.
-
-The format for this documentation is called the kernel-doc format.
-It is documented in this Documentation/kernel-doc-nano-HOWTO.txt file.
-
-This style embeds the documentation within the source files, using
-a few simple conventions.  The scripts/kernel-doc perl script, the
-Documentation/sphinx/kerneldoc.py Sphinx extension and other tools understand
-these conventions, and are used to extract this embedded documentation
-into various documents.
-
-In order to provide good documentation of kernel functions and data
-structures, please use the following conventions to format your
-kernel-doc comments in Linux kernel source.
-
-We definitely need kernel-doc formatted documentation for functions
-that are exported to loadable modules using EXPORT_SYMBOL.
-
-We also look to provide kernel-doc formatted documentation for
-functions externally visible to other kernel files (not marked
-"static").
-
-We also recommend providing kernel-doc formatted documentation
-for private (file "static") routines, for consistency of kernel
-source code layout.  But this is lower priority and at the
-discretion of the MAINTAINER of that kernel source file.
-
-Data structures visible in kernel include files should also be
-documented using kernel-doc formatted comments.
-
-The opening comment mark "/**" is reserved for kernel-doc comments.
-Only comments so marked will be considered by the kernel-doc scripts,
-and any comment so marked must be in kernel-doc format.  Do not use
-"/**" to be begin a comment block unless the comment block contains
-kernel-doc formatted comments.  The closing comment marker for
-kernel-doc comments can be either "*/" or "**/", but "*/" is
-preferred in the Linux kernel tree.
-
-Kernel-doc comments should be placed just before the function
-or data structure being described.
-
-Example kernel-doc function comment:
-
-/**
- * foobar() - short function description of foobar
- * @arg1:  Describe the first argument to foobar.
- * @arg2:  Describe the second argument to foobar.
- * One can provide multiple line descriptions
- * for arguments.
- *
- * A longer description, with more discussion of the function foobar()
- * that might be useful to those using or modifying it.  Begins with
- * empty comment line, and may include additional embedded empty
- * comment lines.
- *
- * The longer description can have multiple paragraphs.
- *
- * Return: Describe the return value of foobar.
- */
-
-The short description following the subject can span multiple lines
-and ends with an @argument description, an empty line or the end of
-the comment block.
-
-The @argument descriptions must begin on the very next line following
-this opening short function description line, with no intervening
-empty comment lines.
-
-If a function parameter is "..." (varargs), it should be listed in
-kernel-doc notation as:
- * @...: description
-
-The return value, if any, should be described in a dedicated section
-named "Return".
-
-Example kernel-doc data structure comment.
-
-/**
- * struct blah - the basic blah structure
- * @mem1:  describe the first member of struct blah
- * @mem2:  describe the second member of struct blah,
- * 

[PATCH v4 15/18] scripts: kernel-doc: handle nested struct function arguments

[Mauro Carvalho Chehab]

Function arguments are different than usual ones. So, an
special logic is needed in order to handle such arguments
on nested structs.

Signed-off-by: Mauro Carvalho Chehab 
---
 scripts/kernel-doc | 38 ++
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index c97b89f47795..5d03c9086323 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1041,18 +1041,32 @@ sub dump_struct($$) {
$id =~ s/^\*+//;
foreach my $arg (split /;/, $content) {
next if ($arg =~ m/^\s*$/);
-   my $type = $arg;
-   my $name = $arg;
-   $type =~ s/\s\S+$//;
-   $name =~ s/.*\s//;
-   $name =~ s/[:\[].*//;
-   $name =~ s/^\*+//;
-   next if (($name =~ m/^\s*$/));
-   if ($id =~ m/^\s*$/) {
-   # anonymous struct/union
-   $newmember .= "$type $name;";
+   if ($arg =~ 
m/^([^\(]+\(\*?\s*)([\w\.]*)(\s*\).*)/) {
+   # pointer-to-function
+   my $type = $1;
+   my $name = $2;
+   my $extra = $3;
+   next if (!$name);
+   if ($id =~ m/^\s*$/) {
+   # anonymous struct/union
+   $newmember .= 
"$type$name$extra;";
+   } else {
+   $newmember .= 
"$type$id.$name$extra;";
+   }
} else {
-   $newmember .= "$type $id.$name;";
+   my $type = $arg;
+   my $name = $arg;
+   $type =~ s/\s\S+$//;
+   $name =~ s/.*\s+//;
+   $name =~ s/[:\[].*//;
+   $name =~ s/^\*+//;
+   next if (($name =~ m/^\s*$/));
+   if ($id =~ m/^\s*$/) {
+   # anonymous struct/union
+   $newmember .= "$type $name;";
+   } else {
+   $newmember .= "$type 
$id.$name;";
+   }
}
}
$members =~ 
s/(struct|union)([^{};]+){([^{}]*)}([^{}\;]*)\;/$newmember/;
@@ -1250,7 +1264,7 @@ sub create_parameterlist() {
} elsif ($arg =~ m/\(.+\)\s*\(/) {
# pointer-to-function
$arg =~ tr/#/,/;
-   $arg =~ m/[^\(]+\(\*?\s*(\w*)\s*\)/;
+   $arg =~ m/[^\(]+\(\*?\s*([\w\.]*)\s*\)/;
$param = $1;
$type = $arg;
$type =~ s/([^\(]+\(\*?)\s*$param/$1/;
-- 
2.14.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 v4 11/18] scripts: kernel-doc: replace tabs by spaces

[Mauro Carvalho Chehab]

Sphinx has a hard time dealing with tabs, causing it to
misinterpret paragraph continuation.

As we're now mainly focused on supporting ReST output,
replace tabs by spaces, in order to avoid troubles when
the output is parsed by Sphinx.

Signed-off-by: Mauro Carvalho Chehab 
---
 scripts/kernel-doc | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index e417d93575b9..05aadac0612a 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1579,7 +1579,7 @@ sub tracepoint_munge($) {
 sub syscall_munge() {
my $void = 0;
 
-   $prototype =~ s@[\r\n\t]+@ @gos; # strip newlines/CR's/tabs
+   $prototype =~ s@[\r\n]+@ @gos; # strip newlines/CR's
 ## if ($prototype =~ m/SYSCALL_DEFINE0\s*\(\s*(a-zA-Z0-9_)*\s*\)/) {
if ($prototype =~ m/SYSCALL_DEFINE0/) {
$void = 1;
@@ -1778,6 +1778,8 @@ sub process_file($) {
while (s/\\\s*$//) {
$_ .= ;
}
+   # Replace tabs by spaces
+while ($_ =~ s/\t+/' ' x (length($&) * 8 - length($`) % 8)/e) {};
if ($state == STATE_NORMAL) {
if (/$doc_start/o) {
$state = STATE_NAME;# next line is always the function name
@@ -1877,8 +1879,7 @@ sub process_file($) {
$in_purpose = 0;
$contents = $newcontents;
 $new_start_line = $.;
-   while ((substr($contents, 0, 1) eq " ") ||
-  substr($contents, 0, 1) eq "\t") {
+   while (substr($contents, 0, 1) eq " ") {
$contents = substr($contents, 1);
}
if ($contents ne "") {
@@ -1947,8 +1948,7 @@ sub process_file($) {
$contents = $2;
 $new_start_line = $.;
if ($contents ne "") {
-   while ((substr($contents, 0, 1) eq " ") ||
-  substr($contents, 0, 1) eq "\t") {
+   while (substr($contents, 0, 1) eq " ") {
$contents = substr($contents, 1);
}
$contents .= "\n";
-- 
2.14.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 v4 02/18] docs: kernel-doc.rst: improve private members description

[Mauro Carvalho Chehab]

The private members section can now be moved to be together
with the arguments section. Move it there and add an example
about the usage of public:

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/doc-guide/kernel-doc.rst | 56 ++
 1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/Documentation/doc-guide/kernel-doc.rst 
b/Documentation/doc-guide/kernel-doc.rst
index f1c03c16f03b..7cf58c3489de 100644
--- a/Documentation/doc-guide/kernel-doc.rst
+++ b/Documentation/doc-guide/kernel-doc.rst
@@ -167,6 +167,36 @@ notation as::
 
   * @...: description
 
+Private members
+~~~
+
+Inside a struct or union description, you can use the ``private:`` and
+``public:`` comment tags. Structure fields that are inside a ``private:``
+area are not listed in the generated output documentation.
+
+The ``private:`` and ``public:`` tags must begin immediately following a
+``/*`` comment marker.  They may optionally include comments between the
+``:`` and the ending ``*/`` marker.
+
+Example::
+
+  /**
+   * struct my_struct - short description
+   * @a: first member
+   * @b: second member
+   * @d: fourth member
+   *
+   * Longer description
+   */
+  struct my_struct {
+  int a;
+  int b;
+  /* private: internal use only */
+  int c;
+  /* public: the next one is public */
+  int d;
+  };
+
 
 Highlights and cross-references
 ---
@@ -332,32 +362,6 @@ on a line of their own, like all other kernel-doc 
comments::
 int foobar;
   }
 
-Private members
-~~~
-
-Inside a struct description, you can use the "private:" and "public:" comment
-tags. Structure fields that are inside a "private:" area are not listed in the
-generated output documentation.  The "private:" and "public:" tags must begin
-immediately following a ``/*`` comment marker.  They may optionally include
-comments between the ``:`` and the ending ``*/`` marker.
-
-Example::
-
-  /**
-   * struct my_struct - short description
-   * @a: first member
-   * @b: second member
-   *
-   * Longer description
-   */
-  struct my_struct {
-  int a;
-  int b;
-  /* private: internal use only */
-  int c;
-  };
-
-
 Typedef documentation
 -
 
-- 
2.14.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 v4 14/18] scripts: kernel-doc: print the declaration name on warnings

[Mauro Carvalho Chehab]

The logic at create_parameterlist()'s ancillary push_parameter()
function has already a way to output the declaration name, with
would help to discover what declaration is missing.

However, currently, the logic is utterly broken, as it uses
the var $type with a wrong meaning. With the current code,
it will never print anything. I suspect that originally
it was using the second argument of output_declaration().

I opted to not rely on a globally defined $declaration_name,
but, instead, to pass it explicitly as a parameter.

While here, I removed a unaligned check for !$anon_struct_union.
This is not needed, as, if $anon_struct_union is not zero,
$parameterdescs{$param} will be defined.

Signed-off-by: Mauro Carvalho Chehab 
---
 scripts/kernel-doc | 38 --
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index fadb832733d9..c97b89f47795 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1063,7 +1063,7 @@ sub dump_struct($$) {
# Ignore other nested elements, like enums
$members =~ s/({[^\{\}]*})//g;
 
-   create_parameterlist($members, ';', $file);
+   create_parameterlist($members, ';', $file, $declaration_name);
check_sections($file, $declaration_name, $decl_type, $sectcheck, 
$struct_actual);
 
# Adjust declaration for better display
@@ -1172,7 +1172,7 @@ sub dump_typedef($$) {
$declaration_name = $2;
my $args = $3;
 
-   create_parameterlist($args, ',', $file);
+   create_parameterlist($args, ',', $file, $declaration_name);
 
output_declaration($declaration_name,
   'function',
@@ -1221,10 +1221,11 @@ sub save_struct_actual($) {
 $struct_actual = $struct_actual . $actual . " ";
 }
 
-sub create_parameterlist($$$) {
+sub create_parameterlist() {
 my $args = shift;
 my $splitter = shift;
 my $file = shift;
+my $declaration_name = shift;
 my $type;
 my $param;
 
@@ -1254,7 +1255,7 @@ sub create_parameterlist($$$) {
$type = $arg;
$type =~ s/([^\(]+\(\*?)\s*$param/$1/;
save_struct_actual($param);
-   push_parameter($param, $type, $file);
+   push_parameter($param, $type, $file, $declaration_name);
} elsif ($arg) {
$arg =~ s/\s*:\s*/:/g;
$arg =~ s/\s*\[/\[/g;
@@ -1279,27 +1280,28 @@ sub create_parameterlist($$$) {
foreach $param (@args) {
if ($param =~ m/^(\*+)\s*(.*)/) {
save_struct_actual($2);
-   push_parameter($2, "$type $1", $file);
+   push_parameter($2, "$type $1", $file, $declaration_name);
}
elsif ($param =~ m/(.*?):(\d+)/) {
if ($type ne "") { # skip unnamed bit-fields
save_struct_actual($1);
-   push_parameter($1, "$type:$2", $file)
+   push_parameter($1, "$type:$2", $file, $declaration_name)
}
}
else {
save_struct_actual($param);
-   push_parameter($param, $type, $file);
+   push_parameter($param, $type, $file, $declaration_name);
}
}
}
 }
 }
 
-sub push_parameter($$$) {
+sub push_parameter() {
my $param = shift;
my $type = shift;
my $file = shift;
+   my $declaration_name = shift;
 
if (($anon_struct_union == 1) && ($type eq "") &&
($param eq "}")) {
@@ -1336,21 +1338,13 @@ sub push_parameter($$$) {
# warn if parameter has no description
# (but ignore ones starting with # as these are not parameters
# but inline preprocessor statements);
-   # also ignore unnamed structs/unions;
-   if (!$anon_struct_union) {
+   # Note: It will also ignore void params and unnamed structs/unions
if (!defined $parameterdescs{$param} && $param !~ /^#/) {
+   $parameterdescs{$param} = $undescribed;
 
-   $parameterdescs{$param} = $undescribed;
-
-   if (($type eq 'function') || ($type eq 'enum')) {
-   print STDERR "${file}:$.: warning: Function parameter ".
-   "or member '$param' not " .
-   "described in '$declaration_name'\n";
-   }
-   print STDERR "${file}:$.: warning:" .
-" No description found for parameter '$param'\n";
-   ++$warnings;
-   }
+   print STDERR
+ "${file}:$.: warning: Function parameter or member 
'$param' not described in '$declaration_name'\n";
+   ++$warnings;
}
 
$param = xml_escape($param);
@@ -1507,7 +1501,7 @@ sub dump_function($$) {
$declaration_name = $2;
my $args = $3;
 
-   create_parameterlist($args, ',', $file);
+   

[PATCH v4 17/18] scripts: kernel-doc: apply filtering rules to warnings

[Mauro Carvalho Chehab]

When kernel-doc is called with output selection filters,
it will be called lots of time for a single file. If
there is a warning present there, it means that it may
print hundreds of identical warnings.

Worse than that, the -function NAME actually filters only
functions. So, it makes no sense at all to print warnings
for structs or enums.

Signed-off-by: Mauro Carvalho Chehab 
---
 scripts/kernel-doc | 30 +++---
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 0bda21d9d3f2..1e2b35ce1c9d 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1160,16 +1160,26 @@ sub dump_enum($$) {
push @parameterlist, $arg;
if (!$parameterdescs{$arg}) {
$parameterdescs{$arg} = $undescribed;
-   print STDERR "${file}:$.: warning: Enum value '$arg' ".
-   "not described in enum '$declaration_name'\n";
+   if (($output_selection == OUTPUT_ALL) ||
+   ($output_selection == OUTPUT_INCLUDE &&
+defined($function_table{$declaration_name})) ||
+   ($output_selection == OUTPUT_EXCLUDE &&
+!defined($function_table{$declaration_name}))) {
+   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";
+   if (($output_selection == OUTPUT_ALL) ||
+   ($output_selection == OUTPUT_INCLUDE &&
+defined($function_table{$declaration_name})) ||
+   ($output_selection == OUTPUT_EXCLUDE &&
+!defined($function_table{$declaration_name}))) {
+print STDERR "${file}:$.: warning: Excess enum value '$k' 
description in '$declaration_name'\n";
+   }
}
 }
 
@@ -1375,9 +1385,15 @@ sub push_parameter() {
if (!defined $parameterdescs{$param} && $param !~ /^#/) {
$parameterdescs{$param} = $undescribed;
 
-   print STDERR
- "${file}:$.: warning: Function parameter or member 
'$param' not described in '$declaration_name'\n";
-   ++$warnings;
+   if (($output_selection == OUTPUT_ALL) ||
+   ($output_selection == OUTPUT_INCLUDE &&
+defined($function_table{$declaration_name})) ||
+   ($output_selection == OUTPUT_EXCLUDE &&
+!defined($function_table{$declaration_name}))) {
+   print STDERR
+ "${file}:$.: warning: Function parameter or 
member '$param' not described in '$declaration_name'\n";
+   ++$warnings;
+   }
}
 
$param = xml_escape($param);
-- 
2.14.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: Documentation license

[Jani Nikula]

On Thu, 14 Dec 2017, Matthew Wilcox  wrote:
> 1. How does one add an SPDX tag to an rst file?

Do we want the SPDX tags visible in the generated documentation or not?

If not, you have to put it in an rst comment:

.. SPDX-License-Identifier: GPL-2.0

If yes, I'd go for using bibliographic data field lists [1]. They can be
and are already used for authorship and copyright, for example
Documentation/driver-api/device-io.rst which is rendered like [2]. There
are some alternatives for the field names, though none of the
"registered field names" seem to apply. For example:

:License: SPDX-License-Identifier: GPL-2.0
:SPDX: SPDX-License-Identifier: GPL-2.0
:SPDX-License-Identifier: GPL-2.0


BR,
Jani.


[1] 
http://docutils.sourceforge.net/docs/ref/rst/restructuredtext.html#field-lists
[2] https://www.kernel.org/doc/html/latest/driver-api/device-io.html


-- 
Jani Nikula, Intel Open Source Technology Center
--
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 2/7] i3c: Add core I3C infrastructure

[Boris Brezillon]

On Sun, 17 Dec 2017 14:32:04 -0800
Randy Dunlap  wrote:

> On 12/14/17 07:16, Boris Brezillon wrote:
> > Add core infrastructure to support I3C in Linux and document it.
> > 
> > Signed-off-by: Boris Brezillon 
> > ---
> >  drivers/Kconfig |2 +
> >  drivers/Makefile|2 +-
> >  drivers/i3c/Kconfig |   24 +
> >  drivers/i3c/Makefile|4 +
> >  drivers/i3c/core.c  |  573 
> >  drivers/i3c/device.c|  344 ++
> >  drivers/i3c/internals.h |   34 +
> >  drivers/i3c/master.c| 1433 
> > +++
> >  drivers/i3c/master/Kconfig  |0
> >  drivers/i3c/master/Makefile |0
> >  include/linux/i3c/ccc.h |  380 +++
> >  include/linux/i3c/device.h  |  321 +
> >  include/linux/i3c/master.h  |  564 +++
> >  include/linux/mod_devicetable.h |   17 +
> >  14 files changed, 3697 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/i3c/Kconfig
> >  create mode 100644 drivers/i3c/Makefile
> >  create mode 100644 drivers/i3c/core.c
> >  create mode 100644 drivers/i3c/device.c
> >  create mode 100644 drivers/i3c/internals.h
> >  create mode 100644 drivers/i3c/master.c
> >  create mode 100644 drivers/i3c/master/Kconfig
> >  create mode 100644 drivers/i3c/master/Makefile
> >  create mode 100644 include/linux/i3c/ccc.h
> >  create mode 100644 include/linux/i3c/device.h
> >  create mode 100644 include/linux/i3c/master.h
> > 
> > diff --git a/drivers/i3c/Kconfig b/drivers/i3c/Kconfig
> > new file mode 100644
> > index ..cf3752412ae9
> > --- /dev/null
> > +++ b/drivers/i3c/Kconfig
> > @@ -0,0 +1,24 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +menuconfig I3C
> > +   tristate "I3C support"
> > +   select I2C
> > +   help
> > + I3C is a serial protocol standardized by the MIPI alliance.
> > +
> > + It's supposed to be backward compatible with I2C while providing
> > + support for high speed transfers and native interrupt support
> > + without the need for extra pins.
> > +
> > + The I3C protocol also standardizes the slave device types and is
> > + mainly design to communicate with sensors.
> > +
> > + If you want I3C support, you should say Y here and also to the
> > + specific driver for your bus adapter(s) below.
> > +
> > + This I3C support can also be built as a module.  If so, the module
> > + will be called i3c.
> > +
> > +if I3C
> > +source "drivers/i3c/master/Kconfig"
> > +endif # I3C  
> 
> > diff --git a/drivers/i3c/core.c b/drivers/i3c/core.c
> > new file mode 100644
> > index ..7eb8e84acd33
> > --- /dev/null
> > +++ b/drivers/i3c/core.c
> > @@ -0,0 +1,573 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2017 Cadence Design Systems Inc.
> > + *
> > + * Author: Boris Brezillon 
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include   
> 
> #include 
> #include 
> #include 
> #include 
> #include 

Do you have a tool to detect those missing inclusions?

> 
> 
> > +#include "internals.h"
> > +
> > +static DEFINE_IDR(i3c_bus_idr);
> > +static DEFINE_MUTEX(i3c_core_lock);
> > +  
> 
> > +/**
> > + * i3c_bus_maintenance_lock - Release the bus lock after a maintenance  
> 
>   unlock
> 

Will fix that.

> > + *   operation
> > + * @bus: I3C bus to release the lock on
> > + *
> > + * Should be called when the bus maintenance operation is done. See
> > + * i3c_bus_maintenance_lock() for more details on what these maintenance
> > + * operations are.
> > + */
> > +void i3c_bus_maintenance_unlock(struct i3c_bus *bus)
> > +{
> > +   up_write(>lock);
> > +}
> > +EXPORT_SYMBOL_GPL(i3c_bus_maintenance_unlock);
> > +  
> 
> > +/**
> > + * i3c_bus_normaluse_lock - Release the bus lock after a normal operation  
> 
> unlock

Ditto.

> 
> > + * @bus: I3C bus to release the lock on
> > + *
> > + * Should be called when a normal operation is done. See
> > + * i3c_bus_normaluse_lock() for more details on what these normal 
> > operations
> > + * are.
> > + */
> > +void i3c_bus_normaluse_unlock(struct i3c_bus *bus)
> > +{
> > +   up_read(>lock);
> > +}
> > +EXPORT_SYMBOL_GPL(i3c_bus_normaluse_unlock);  
> 
> 
> 
> > +static int i3c_device_match(struct device *dev, struct device_driver *drv) 
> >  
> 
> bool?
> 
> > +{
> > +   struct i3c_device *i3cdev;
> > +   struct i3c_driver *i3cdrv;
> > +
> > +   if (dev->type != _device_type)
> > +   return 0;
> > +
> > +   i3cdev = dev_to_i3cdev(dev);
> > +   i3cdrv = drv_to_i3cdrv(drv);
> > +   if (i3c_device_match_id(i3cdev, i3cdrv->id_table))
> > +   return 1;
> > +
> > +   return 0;
> > +}  
> 
> 
> > diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
> > new file mode 100644
> > index