Re: [PATCH] docs: maintainer: discourage taking conversations off-list

[Greg KH]

On Fri, Jul 12, 2024 at 07:49:03AM -0700, Jakub Kicinski wrote:
> Multiple vendors seem to prefer taking discussions off list, and
> ask contributors to work with them privately rather than just send
> patches to the list. I'd imagine this is because it's hard to fit in
> time for random developers popping up with features to review into
> packed schedule. From what I've seen "work in private" usually means
> someone on the company side will be assigned to handle the interaction,
> possibly months later. In worst case, the person scheduled to help
> the contributor takes over and writes the code themselves.
> This is not how the community is supposed to work.
> 
> Signed-off-by: Jakub Kicinski 
> ---
> CC: workfl...@vger.kernel.org
> CC: linux-doc@vger.kernel.org
> ---
>  .../maintainer/feature-and-driver-maintainers.rst | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/maintainer/feature-and-driver-maintainers.rst 
> b/Documentation/maintainer/feature-and-driver-maintainers.rst
> index f04cc183e1de..ac7798280201 100644
> --- a/Documentation/maintainer/feature-and-driver-maintainers.rst
> +++ b/Documentation/maintainer/feature-and-driver-maintainers.rst
> @@ -83,6 +83,17 @@ bugs as well, if the report is of reasonable quality or 
> indicates a
>  problem that might be severe -- especially if they have *Supported*
>  status of the codebase in the MAINTAINERS file.
>  
> +Open development
> +
> +
> +Discussions about user reported issues, and development of new code
> +should be conducted in a manner typical for the larger subsystem.
> +It is common for development within a single company to be conducted
> +behind closed doors. However, maintainers must not redirect discussions
> +and development related to the upstream code from the upstream mailing lists
> +to closed forums or private conversations. Reasonable exceptions to this
> +guidance include discussions about security related issues.
> +
>  Selecting the maintainer
>  
>  

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH v1 2/2] Documentation: process: Recommend to put Cc: tags after cutter '---' line

[Greg KH]

On Tue, Apr 23, 2024 at 06:28:44PM +0300, Andy Shevchenko wrote:
> On Tue, Apr 23, 2024 at 08:13:37AM -0700, Greg KH wrote:
> > On Tue, Apr 23, 2024 at 04:19:38PM +0300, Andy Shevchenko wrote:
> > > The recommendation is based on the following rationale:
> > > 
> > > - it makes the commit messages much cleaner and easy to read, especially
> > >   on the screens of the mobile devices;
> > 
> > Reading commits on a mobile device is not what kernel development is
> > optimized for, sorry.
> 
> The commit messages not always for the kernel development, some people may 
> read
> them in order to understand code better or many other reasons. I.o.w. it's not
> the point.
> 
> > > - it reduces resources (memory, time, energy) to retrieve all these
> > >   headers, which are barely needed by a mere user, as for automation
> > >   they will be still available via mail archives, such as
> > >   https://lore.kernel.org, assuming the Link: or Message-ID tag is
> > >   provided.
> > > 
> > > Let's be environment friendly and save the planet!
> > > 
> > > Signed-off-by: Andy Shevchenko 
> > > ---
> > >  Documentation/process/5.Posting.rst  | 4 
> > >  Documentation/process/submitting-patches.rst | 5 +
> > >  2 files changed, 9 insertions(+)
> > 
> > This breaks my existing workflow, sorry.  I can't track cc: below the
> > --- line because obviously, git cuts that off.  So I put them above
> > where git send-email can see them,
> 
> git send-email _sees_ them very well there.

Sorry, yes, but git itself can't track them in the commit when I
rebase/squash/redo/etc.  That's the point I was trying to make before my
coffee kicked in...

thanks,

greg k-h

Re: [PATCH v1 2/2] Documentation: process: Recommend to put Cc: tags after cutter '---' line

[Greg KH]

On Tue, Apr 23, 2024 at 04:19:38PM +0300, Andy Shevchenko wrote:
> The recommendation is based on the following rationale:
> 
> - it makes the commit messages much cleaner and easy to read, especially
>   on the screens of the mobile devices;

Reading commits on a mobile device is not what kernel development is
optimized for, sorry.

> - it reduces resources (memory, time, energy) to retrieve all these
>   headers, which are barely needed by a mere user, as for automation
>   they will be still available via mail archives, such as
>   https://lore.kernel.org, assuming the Link: or Message-ID tag is
>   provided.
> 
> Let's be environment friendly and save the planet!
> 
> Signed-off-by: Andy Shevchenko 
> ---
>  Documentation/process/5.Posting.rst  | 4 
>  Documentation/process/submitting-patches.rst | 5 +
>  2 files changed, 9 insertions(+)

This breaks my existing workflow, sorry.  I can't track cc: below the
--- line because obviously, git cuts that off.  So I put them above
where git send-email can see them, AND where git can handle them when I
rebase/revise/etc.

Also, as Jon points out, the "environmental" argument seems very odd,
it's not like bits cost more to send around (they keep getting cheaper
in power and money), and processing power is pretty constant for servers
handling email, so I don't understand the power consumption argument.
Without some sort of data to back that up, I'd be loath to repeat it.

thanks,

greg k-h

Re: [PATCH] Documentation: coding-style: don't encourage WARN*()

[Greg KH]

On Mon, Apr 15, 2024 at 01:07:41AM -0700, Christoph Hellwig wrote:
> No, this advice is wronger than wrong.  If you set panic_on_warn you
> get to keep the pieces.  
> 

But don't add new WARN() calls please, just properly clean up and handle
the error.  And any WARN() that userspace can trigger ends up triggering
syzbot reports which also is a major pain, even if you don't have
panic_on_warn enabled.

And I think the "do not use panic_on_warn" recommendation has been
ignored, given the huge use of it by vendors who have enabled it (i.e.
all Samsung phones and cloud servers).

thanks,

greg k-h

Re: [PATCH] Documentation: coding-style: don't encourage WARN*()

[Greg KH]

On Mon, Apr 15, 2024 at 11:25:29AM +0300, Laurent Pinchart wrote:
> Hi Greg,
> 
> On Mon, Apr 15, 2024 at 07:21:37AM +0200, Greg KH wrote:
> > On Sun, Apr 14, 2024 at 10:48:35PM +0300, Laurent Pinchart wrote:
> > > On Sun, Apr 14, 2024 at 12:08:50PM -0500, Alex Elder wrote:
> > > > Several times recently Greg KH has admonished that variants of WARN()
> > > > should not be used, because when the panic_on_warn kernel option is set,
> > > > their use can lead to a panic. His reasoning was that the majority of
> > > > Linux instances (including Android and cloud systems) run with this 
> > > > option
> > > > enabled. And therefore a condition leading to a warning will frequently
> > > > cause an undesirable panic.
> > > > 
> > > > The "coding-style.rst" document says not to worry about this kernel
> > > > option.  Update it to provide a more nuanced explanation.
> > > > 
> > > > Signed-off-by: Alex Elder 
> > > > ---
> > > >  Documentation/process/coding-style.rst | 21 +++--
> > > >  1 file changed, 11 insertions(+), 10 deletions(-)
> > > > 
> > > > diff --git a/Documentation/process/coding-style.rst 
> > > > b/Documentation/process/coding-style.rst
> > > > index 9c7cf73473943..bce43b01721cb 100644
> > > > --- a/Documentation/process/coding-style.rst
> > > > +++ b/Documentation/process/coding-style.rst
> > > > @@ -1235,17 +1235,18 @@ example. Again: WARN*() must not be used for a 
> > > > condition that is expected
> > > >  to trigger easily, for example, by user space actions. pr_warn_once() 
> > > > is a
> > > >  possible alternative, if you need to notify the user of a problem.
> > > >  
> > > > -Do not worry about panic_on_warn users
> > > > -**
> > > > +The panic_on_warn kernel option
> > > > +
> > > >  
> > > > -A few more words about panic_on_warn: Remember that ``panic_on_warn`` 
> > > > is an
> > > > -available kernel option, and that many users set this option. This is 
> > > > why
> > > > -there is a "Do not WARN lightly" writeup, above. However, the 
> > > > existence of
> > > > -panic_on_warn users is not a valid reason to avoid the judicious use
> > > > -WARN*(). That is because, whoever enables panic_on_warn has explicitly
> > > > -asked the kernel to crash if a WARN*() fires, and such users must be
> > > > -prepared to deal with the consequences of a system that is somewhat 
> > > > more
> > > > -likely to crash.
> > > > +Note that ``panic_on_warn`` is an available kernel option. If it is 
> > > > enabled,
> > > > +a WARN*() call whose condition holds leads to a kernel panic.  Many 
> > > > users
> > > > +(including Android and many cloud providers) set this option, and this 
> > > > is
> > > > +why there is a "Do not WARN lightly" writeup, above.
> > > > +
> > > > +The existence of this option is not a valid reason to avoid the 
> > > > judicious
> > > > +use of warnings. There are other options: ``dev_warn*()`` and 
> > > > ``pr_warn*()``
> > > > +issue warnings but do **not** cause the kernel to crash. Use these if 
> > > > you
> > > > +want to prevent such panics.
> > > 
> > > Those options are not equivalent, they print a single message, which is
> > > much easier to ignore. WARN() is similar to -Werror in some sense, it
> > > pushes vendors to fix the warnings. I have used WARN() in the past to
> > > indicate usage of long-deprecated APIs that we were getting close to
> > > removing for instance. dev_warn() wouldn't have had the same effect.
> > 
> > If you want to reboot a box because someone called an "improper" api,
> 
> I don't "want" to reboot. It came as a side effect when panic_on_warn
> was added, and worsened when its adoption increased. I won't argued for
> or against panic_on_warn, but WARN() serves some use cases today that I
> consider valid. If we want to discourage its usage, we need another API
> to cover those use cases.
> 
> > then sure, use WARN(), but that feels like a really bad idea.  Just
> > remove the api and fix up all in-kernel users instead.  Why wait?
> 
> There are multiple use cases. One of them is to make sure no new user of
> the old, deprecated behaviour is introduced. This is especially
> important when driver development spans multiple kernel releases, the
> development can start before the API behaviour changes, with the driver
> merged after the API change. This is something we've done multiple times
> in V4L2.
> 
> > If you want to show a traceback, then just print that out, but I've seen
> > that totally ignored as well, removing the api is usually the only way
> > to get people to actually notice, as then their builds break.
> 
> Does your experience tell that tracebacks are routinely ignored during
> development too, not just in production ?

Yes, we have done this in the past in some driver core apis and nothing
ever changed until we actually deleted the apis.

thanks,

greg k-h

Re: [PATCH] Documentation: coding-style: don't encourage WARN*()

[Greg KH]

On Sun, Apr 14, 2024 at 12:08:50PM -0500, Alex Elder wrote:
> Several times recently Greg KH has admonished that variants of WARN()
> should not be used, because when the panic_on_warn kernel option is set,
> their use can lead to a panic. His reasoning was that the majority of
> Linux instances (including Android and cloud systems) run with this option
> enabled. And therefore a condition leading to a warning will frequently
> cause an undesirable panic.
> 
> The "coding-style.rst" document says not to worry about this kernel
> option.  Update it to provide a more nuanced explanation.
> 
> Signed-off-by: Alex Elder 

Thanks for writing this up:

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH] Documentation: coding-style: don't encourage WARN*()

[Greg KH]

On Sun, Apr 14, 2024 at 10:48:35PM +0300, Laurent Pinchart wrote:
> Hi Alex,
> 
> Thank you for the patch.
> 
> On Sun, Apr 14, 2024 at 12:08:50PM -0500, Alex Elder wrote:
> > Several times recently Greg KH has admonished that variants of WARN()
> > should not be used, because when the panic_on_warn kernel option is set,
> > their use can lead to a panic. His reasoning was that the majority of
> > Linux instances (including Android and cloud systems) run with this option
> > enabled. And therefore a condition leading to a warning will frequently
> > cause an undesirable panic.
> > 
> > The "coding-style.rst" document says not to worry about this kernel
> > option.  Update it to provide a more nuanced explanation.
> > 
> > Signed-off-by: Alex Elder 
> > ---
> >  Documentation/process/coding-style.rst | 21 +++--
> >  1 file changed, 11 insertions(+), 10 deletions(-)
> > 
> > diff --git a/Documentation/process/coding-style.rst 
> > b/Documentation/process/coding-style.rst
> > index 9c7cf73473943..bce43b01721cb 100644
> > --- a/Documentation/process/coding-style.rst
> > +++ b/Documentation/process/coding-style.rst
> > @@ -1235,17 +1235,18 @@ example. Again: WARN*() must not be used for a 
> > condition that is expected
> >  to trigger easily, for example, by user space actions. pr_warn_once() is a
> >  possible alternative, if you need to notify the user of a problem.
> >  
> > -Do not worry about panic_on_warn users
> > -**
> > +The panic_on_warn kernel option
> > +
> >  
> > -A few more words about panic_on_warn: Remember that ``panic_on_warn`` is an
> > -available kernel option, and that many users set this option. This is why
> > -there is a "Do not WARN lightly" writeup, above. However, the existence of
> > -panic_on_warn users is not a valid reason to avoid the judicious use
> > -WARN*(). That is because, whoever enables panic_on_warn has explicitly
> > -asked the kernel to crash if a WARN*() fires, and such users must be
> > -prepared to deal with the consequences of a system that is somewhat more
> > -likely to crash.
> > +Note that ``panic_on_warn`` is an available kernel option. If it is 
> > enabled,
> > +a WARN*() call whose condition holds leads to a kernel panic.  Many users
> > +(including Android and many cloud providers) set this option, and this is
> > +why there is a "Do not WARN lightly" writeup, above.
> > +
> > +The existence of this option is not a valid reason to avoid the judicious
> > +use of warnings. There are other options: ``dev_warn*()`` and 
> > ``pr_warn*()``
> > +issue warnings but do **not** cause the kernel to crash. Use these if you
> > +want to prevent such panics.
> 
> Those options are not equivalent, they print a single message, which is
> much easier to ignore. WARN() is similar to -Werror in some sense, it
> pushes vendors to fix the warnings. I have used WARN() in the past to
> indicate usage of long-deprecated APIs that we were getting close to
> removing for instance. dev_warn() wouldn't have had the same effect.

If you want to reboot a box because someone called an "improper" api,
then sure, use WARN(), but that feels like a really bad idea.  Just
remove the api and fix up all in-kernel users instead.  Why wait?

If you want to show a traceback, then just print that out, but I've seen
that totally ignored as well, removing the api is usually the only way
to get people to actually notice, as then their builds break.

thanks,

greg k-h

Re: [PATCH] Documentation: embargoed-hardware-issues.rst: Fix Trilok's email

[Greg KH]

On Fri, Feb 02, 2024 at 09:41:19AM -0700, Jeffrey Hugo wrote:
> The servers for the @codeaurora domain have long been retired and any
> messages addressed to @codeaurora will bounce.
> 
> Trilok has an entry in .mailmap, but the raw documentation files still
> list an old @codeaurora address.  Update the address in the
> documentation files for anyone reading them.
> 
> Signed-off-by: Jeffrey Hugo 
> ---
>  Documentation/process/embargoed-hardware-issues.rst | 2 +-
>  .../translations/sp_SP/process/embargoed-hardware-issues.rst| 2 +-
>  .../translations/zh_CN/process/embargoed-hardware-issues.rst| 2 +-
>  .../translations/zh_TW/process/embargoed-hardware-issues.rst| 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)

I think we need an ack from Trilok for this :)

thanks,

greg k-h

Re: [PATCH] coding-style: Add guidance to prefer dev_dbg

[Greg KH]

On Thu, Jan 25, 2024 at 04:53:11PM -0800, Abhishek Pandit-Subedi wrote:
> During review, it was suggested that drivers only emit messages when
> something is wrong or it is a debug message. Document this as a formal
> recommendation.
> 
> https://lore.kernel.org/linux-usb/2024012525-alienate-frown-916b@gregkh/
> 
> Signed-off-by: Abhishek Pandit-Subedi 

Acked-by: Greg Kroah-Hartman 

Re: [PATCH] scripts: kernel-doc: Bug fixed for erroneous warning

[Greg KH]

On Wed, Dec 20, 2023 at 11:24:46AM +0500, Muhammad Muzammil wrote:
> From: Muzammil Ashraf 
> 
> kernel-doc: fixed erroneous warning generated by '__counted_by'
> 
> Signed-off-by: Muzammil Ashraf 
> ---
>  scripts/kernel-doc | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index 1484127db104..ea9688df0e93 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -1661,6 +1661,7 @@ sub check_sections($) {
>   }
>   elsif (($decl_type eq "struct") or
>  ($decl_type eq "union")) {
> +next if (index("@_", "__counted_by") != -1);

Please fix your editor to properly use tabs, and to set them to the
correct 8 column spacing.

thanks,

greg k-h

Re: [PATCH] Sounds better

[Greg KH]

On Sun, Dec 03, 2023 at 07:00:18PM +0530, attreyee-muk wrote:
> Respected Maintainers, 
> 
> I have made a small change in the submitting-patches.rst document. I was
> reading the submitting-patches.rst file as I needed to understand
> certain things for submitting patches. That is when I found this. 
> It might not be a very sigificant change, but I think it just improves a
> bit of a readability and I feel that "Don't get discouraged, or become 
> impatient"
> sounds better.
> 
> Hence, sending this patch. Requesting the maintains to have a look at
> it. 
> 
> Thank You
> Attreyee Mukehrjee 
> 
> Signed-off-by: Attreyee Mukherjee 
> ---
>  Documentation/process/submitting-patches.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/process/submitting-patches.rst 
> b/Documentation/process/submitting-patches.rst
> index 86d346bcb8ef..f524e63fb429 100644
> --- a/Documentation/process/submitting-patches.rst
> +++ b/Documentation/process/submitting-patches.rst
> @@ -360,7 +360,7 @@ space. For more details see: 
> http://daringfireball.net/2007/07/on_top ::
>  
>  .. _resend_reminders:
>  
> -Don't get discouraged - or impatient
> +Don't get discouraged, or become impatient. 
>  
>  
>  After you have submitted your change, be patient and wait.  Reviewers are
> -- 
> 2.34.1
> 
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- Your patch contains warnings and/or errors noticed by the
  scripts/checkpatch.pl tool.

- You did not specify a description of why the patch is needed, or
  possibly, any description at all, in the email body.  Please read the
  section entitled "The canonical patch format" in the kernel file,
  Documentation/process/submitting-patches.rst for what is needed in
  order to properly describe the change.

- You did not write a descriptive Subject: for the patch, allowing Greg,
  and everyone else, to know what this patch is all about.  Please read
  the section entitled "The canonical patch format" in the kernel file,
  Documentation/process/submitting-patches.rst for what a proper
  Subject: line should look like.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: [PATCH v3] Documentation/process/coding-style.rst: space around const

[Greg KH]

On Tue, Oct 10, 2023 at 02:58:31PM +0200, Max Kellermann wrote:
> There are currently no rules on the placement of "const", but a recent
> code submission revealed that there is clearly a preference for spaces
> around it.
> 
> checkpatch.pl has no check at all for this; though it does sometimes
> complain, but only because it erroneously thinks that the "*" (on
> local variables) is an unary dereference operator, not a pointer type.
> 
> Current coding style for const pointers-to-pointers:
> 
>  "*const*": 2 occurrences
>  "* const*": 3
>  "*const *": 182
>  "* const *": 681
> 
> Just const pointers:
> 
>  "*const": 2833 occurrences
>  "* const": 16615
> 
> Link: 
> https://lore.kernel.org/r/264fa39d-aed6-4a54-a085-107997078...@roeck-us.net/
> Link: 
> https://lore.kernel.org/r/f511170fe61d7e7214a3a062661cf4103980dad6.ca...@perches.com/
> Signed-off-by: Max Kellermann 
> ---
> V1 -> V2: removed "volatile" on gregkh's request.
> V2 -> V3: moved patch changelog below the "---" line
> ---
>  Documentation/process/coding-style.rst | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/process/coding-style.rst 
> b/Documentation/process/coding-style.rst
> index 6db37a46d305..71d62d81e506 100644
> --- a/Documentation/process/coding-style.rst
> +++ b/Documentation/process/coding-style.rst
> @@ -271,6 +271,17 @@ adjacent to the type name.  Examples:
>   unsigned long long memparse(char *ptr, char **retptr);
>   char *match_strdup(substring_t *s);
>  
> +Use space around the ``const`` keyword (except when adjacent to
> +parentheses).  Example:
> +
> +.. code-block:: c
> +
> + const void *a;
> + void * const b;
> + void ** const c;
> + void * const * const d;
> + int strcmp(const char *a, const char *b);
> +
>  Use one space around (on each side of) most binary and ternary operators,
>  such as any of these::
>  
> -- 
> 2.39.2
> 

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH v2] Documentation/process/coding-style.rst: space around const

[Greg KH]

On Tue, Oct 10, 2023 at 02:29:35PM +0200, Max Kellermann wrote:
> There are currently no rules on the placement of "const", but a recent
> code submission revealed that there is clearly a preference for spaces
> around them.
> 
> checkpatch.pl has no check at all for this; though it does sometimes
> complain, but only because it erroneously thinks that the "*" (on
> local variables) is an unary dereference operator, not a pointer type.
> 
> Current coding style for const pointers-to-pointers:
> 
>  "*const*": 2 occurrences
>  "* const*": 3
>  "*const *": 182
>  "* const *": 681
> 
> Just const pointers:
> 
>  "*const": 2833 occurrences
>  "* const": 16615
> 
> Changed in v2: removed "volatile" on gregkh's request.
> 
> Link: 
> https://lore.kernel.org/r/264fa39d-aed6-4a54-a085-107997078...@roeck-us.net/
> Link: 
> https://lore.kernel.org/r/f511170fe61d7e7214a3a062661cf4103980dad6.ca...@perches.com/
> Signed-off-by: Max Kellermann 
> ---
>  Documentation/process/coding-style.rst | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/process/coding-style.rst 
> b/Documentation/process/coding-style.rst
> index 6db37a46d305..71d62d81e506 100644
> --- a/Documentation/process/coding-style.rst
> +++ b/Documentation/process/coding-style.rst
> @@ -271,6 +271,17 @@ adjacent to the type name.  Examples:
>   unsigned long long memparse(char *ptr, char **retptr);
>   char *match_strdup(substring_t *s);
>  
> +Use space around the ``const`` keyword (except when adjacent to
> +parentheses).  Example:
> +
> +.. code-block:: c
> +
> + const void *a;
> + void * const b;
> + void ** const c;
> + void * const * const d;
> + int strcmp(const char *a, const char *b);
> +
>  Use one space around (on each side of) most binary and ternary operators,
>  such as any of these::
>  
> -- 
> 2.39.2
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/process/submitting-patches.rst for what
  needs to be done here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: [PATCH] Documentation/process/coding-style.rst: space around const/volatile

[Greg KH]

On Tue, Oct 10, 2023 at 02:03:25PM +0200, Max Kellermann wrote:
> On Tue, Oct 10, 2023 at 1:37 PM Greg KH  wrote:
> > Don't encourage the use of volatile please
> 
> I don't mean to - but I figured IF "volatile" is used (for whatever
> reason, whether correct or not), it should follow the same coding
> style as "const".
> 
> Do you want me to remove mentions of "volatile" (leaving the coding
> style unspecified), or do you want me to add some warning about using
> volatile?

I would recommend just removing the mentions of it here.

thanks,

greg k-h

Re: [PATCH] Documentation/process/coding-style.rst: space around const/volatile

[Greg KH]

On Tue, Oct 10, 2023 at 12:12:40PM +0200, Max Kellermann wrote:
> There are currently no rules on the placement of "const" and
> "volatile", but a recent code submission revealed that there is
> clearly a preference for spaces around them.
> 
> checkpatch.pl has no check at all for this; though it does sometimes
> complain, but only because it erroneously thinks that the "*" (on
> local variables) is an unary dereference operator, not a pointer type.
> 
> Current coding style for const pointers-to-pointers:
> 
>  "*const*": 2 occurrences
>  "* const*": 3
>  "*const *": 182
>  "* const *": 681
> 
> Just const pointers:
> 
>  "*const": 2833 occurrences
>  "* const": 16615
> 
> Link: 
> https://lore.kernel.org/r/264fa39d-aed6-4a54-a085-107997078...@roeck-us.net/
> Link: 
> https://lore.kernel.org/r/f511170fe61d7e7214a3a062661cf4103980dad6.ca...@perches.com/
> Signed-off-by: Max Kellermann 
> ---
>  Documentation/process/coding-style.rst | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/process/coding-style.rst 
> b/Documentation/process/coding-style.rst
> index 6db37a46d305..b40830517938 100644
> --- a/Documentation/process/coding-style.rst
> +++ b/Documentation/process/coding-style.rst
> @@ -271,6 +271,18 @@ adjacent to the type name.  Examples:
>   unsigned long long memparse(char *ptr, char **retptr);
>   char *match_strdup(substring_t *s);
>  
> +Use space around the keywords ``const`` and ``volatile`` (except when
> +adjacent to parentheses).  Example:
> +
> +.. code-block:: c
> +
> + const void *a;
> + void * const b;
> + void ** const c;
> + void * const * const d;
> + void * volatile e;
> + int strcmp(const char *a, const char *b);

Don't encourage the use of volatile please, it shouldn't be needed in
kernel code (hint, almost all uses of it in the tree is wrong except for
asm statements and some .h files that know they need it for some
hardware operations.)

thanks,

greg k-h

Re: [PATCH v7 0/5] Add initial support for S32V234-EVB

[Greg KH]

On Wed, Oct 16, 2019 at 03:48:22PM +0300, Stefan-Gabriel Mirea wrote:
> Hello,
> 
> NXP's S32V234[1] ("Treerunner") vision microprocessors are targeted for
> high-performance, computationally intensive vision and sensor fusion
> applications that require automotive safety levels. They include leading
> edge Camera Vision modules like APEX-2, ISP and GPU. The S32V234-EVB and
> S32V234-SBC boards are available for customer evaluation.
> 
> The following patch series introduces minimal enablement support for the
> NXP S32V234-EVB2[2] board, which leverages most of the SoC capabilities.
> Up to v2, this series also included the fsl_linflexuart driver, which has
> been included in Linux 5.4-rc1[3].
> 
> In the future, we aim to submit multiple drivers upstream, which can be
> found in the kernel of our Auto Linux BSP[4] ("ALB"), starting with basic
> pinmuxing, clock and uSDHC drivers.
> 
> For validation, you can use the U-Boot bootloader in the ALB[5], which we
> build and test with our patched version of the Linaro GCC 6.3.1 2017.05
> toolchain for ARM 64-bit, with sources available on [6].
> 
> Changes in v7:
> * Rebase the patch 'serial: fsl_linflexuart: Be consistent with the name'
>   on the tty-next branch in Greg's tty git tree.

I've taken patch 3 in my tty-next tree.  The others should probably go
through an arm-specific tree, right?

thanks,

greg k-h

Re: [PATCH v6 3/5] serial: fsl_linflexuart: Be consistent with the name

[Greg KH]

On Thu, Oct 10, 2019 at 07:52:26PM +0300, Stefan-Gabriel Mirea wrote:
> For consistency reasons, spell the controller name as "LINFlexD" in
> comments and documentation.
> 
> Signed-off-by: Stefan-Gabriel Mirea 
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 2 +-
>  drivers/tty/serial/Kconfig  | 8 
>  drivers/tty/serial/fsl_linflexuart.c| 4 ++--
>  include/uapi/linux/serial_core.h| 2 +-
>  4 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt 
> b/Documentation/admin-guide/kernel-parameters.txt
> index c7ac2f3ac99f..666326d74415 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1101,7 +1101,7 @@
>   mapped with the correct attributes.
>  
>   linflex,
> - Use early console provided by Freescale LinFlex UART
> + Use early console provided by Freescale LINFlexD UART
>   serial driver for NXP S32V234 SoCs. A valid base
>   address must be provided, and the serial port must
>   already be setup and configured.
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 4789b5d62f63..c8e11f62ea19 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1391,19 +1391,19 @@ config SERIAL_FSL_LPUART_CONSOLE
> you can make it the console by answering Y to this option.
>  
>  config SERIAL_FSL_LINFLEXUART
> - tristate "Freescale linflexuart serial port support"
> + tristate "Freescale LINFlexD UART serial port support"
>   depends on PRINTK
>   select SERIAL_CORE
>   help
> -   Support for the on-chip linflexuart on some Freescale SOCs.
> +   Support for the on-chip LINFlexD UART on some Freescale SOCs.
>  
>  config SERIAL_FSL_LINFLEXUART_CONSOLE
> - bool "Console on Freescale linflexuart serial port"
> + bool "Console on Freescale LINFlexD UART serial port"
>   depends on SERIAL_FSL_LINFLEXUART=y
>   select SERIAL_CORE_CONSOLE
>   select SERIAL_EARLYCON
>   help
> -   If you have enabled the linflexuart serial port on the Freescale
> +   If you have enabled the LINFlexD UART serial port on the Freescale
> SoCs, you can make it the console by answering Y to this option.
>  
>  config SERIAL_CONEXANT_DIGICOLOR
> diff --git a/drivers/tty/serial/fsl_linflexuart.c 
> b/drivers/tty/serial/fsl_linflexuart.c
> index 68d74f2b5106..2d39e13176e1 100644
> --- a/drivers/tty/serial/fsl_linflexuart.c
> +++ b/drivers/tty/serial/fsl_linflexuart.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /*
> - * Freescale linflexuart serial port driver
> + * Freescale LINFlexD UART serial port driver
>   *
>   * Copyright 2012-2016 Freescale Semiconductor, Inc.
>   * Copyright 2017-2018 NXP
> @@ -933,5 +933,5 @@ static void __exit linflex_serial_exit(void)
>  module_init(linflex_serial_init);
>  module_exit(linflex_serial_exit);
>  
> -MODULE_DESCRIPTION("Freescale linflex serial port driver");
> +MODULE_DESCRIPTION("Freescale LINFlexD serial port driver");
>  MODULE_LICENSE("GPL v2");
> diff --git a/include/uapi/linux/serial_core.h 
> b/include/uapi/linux/serial_core.h
> index 0f4f87a6fd54..49e61963e754 100644
> --- a/include/uapi/linux/serial_core.h
> +++ b/include/uapi/linux/serial_core.h
> @@ -290,7 +290,7 @@
>  /* Sunix UART */
>  #define PORT_SUNIX   121
>  
> -/* Freescale Linflex UART */
> +/* Freescale LINFlexD UART */
>  #define PORT_LINFLEXUART 121
>  
>  #endif /* _UAPILINUX_SERIAL_CORE_H */
> -- 
> 2.22.0
> 

This patch does not apply to my tree :(

Re: [PATCH 2/4] Documentation/process: describe relaxing disclosing party NDAs

[Greg KH]

On Wed, Sep 11, 2019 at 09:09:18AM -0700, Dave Hansen wrote:
> On 9/11/19 8:44 AM, Greg KH wrote:
> > Intel had months of review time for this document before this was
> > published.  Your lawyers had it and never objected to this lack of
> > inclusion at all, and explictitly said that the document as written was
> > fine with them.  So I'm sorry, but it is much too late to add something
> > like this to the document at this point in time.
> 
> Hi Greg,
> 
> I'll personally take 100% of the blame for this patch.  I intended for
> it to show our commitment to work *with* our colleagues in the
> community, not to dictate demands.  Please consider this as you would
> any other patch: a humble suggestion to address what I see as a gap.
> 
> Just to be clear: this addition came from me and only me.  It did not
> come from any Intel lawyers and does not represent any kind of objection
> to the process.  Intel's support for this process is unconditional and
> not dependent on any of these patches.

Ok, thanks for the clarification.  It looked like this came from Intel
based on the comments you made on the other patches in this series.  My
confusion, sorry.

I think that Thomas's rewording here makes more sense, and you seem to
agree, so I'll go queue that up now.

thanks,

greg k-h

Re: [PATCH 4/4] Documentation/process: add transparency promise to list subscription

[Greg KH]

On Tue, Sep 10, 2019 at 10:26:52AM -0700, Dave Hansen wrote:
> 
> From: Dave Hansen 
> 
> Transparency is good.  It it essential for everyone working under an
> embargo to know who is involved and who else is a "knower".  Being
> transparent allows everyone to always make informed decisions about
> ongoing participating in a mitigation effort.
> 
> Add a step to the subscription process which will notify existing
> subscribers when a new one is added.

As I don't run the mailing list software here, I don't know how much of
a burden adding this support to it would be, so I'll defer to Thomas.

We could just have something like "All new people need to announce
themselves" rule like many other private mailing lists have at times.
That would put the burden on the new person once, instead of the list
manager for every time a new person is added.

thanks,

greg k-h

Re: [PATCH 3/4] Documentation/process: soften language around conference talk dates

[Greg KH]

On Tue, Sep 10, 2019 at 10:26:51AM -0700, Dave Hansen wrote:
> 
> From: Dave Hansen 
> 
> Both hardware companies and the kernel community prefer coordinated
> disclosure to the alternatives.  It is also obvious that sitting on
> ready-to-go mitigations for months is not so nice for kernel
> maintainers.
> 
> I want to ensure that the patched text can not be read as "the kernel
> does not wait for conference dates".  I'm also fairly sure that, so
> far, we *have* waited for a number of conference dates.

We have been "forced" to wait for conference dates.  That is much
different from what we are saying here (i.e. we do NOT want to have to
wait for that type of thing as that causes us all real work that is a
total waste of engineering effort.)

> Change the text to make it clear that waiting for conference dates
> is possible, but keep the grumbling about it being a burden.

I don't think we want that, waiting for long periods of time like we
have been (and are currently) is a royal pain.  We are glad to take
these on a case-by-case basis, but doing delays for no other reason than
a specific conference date 6 months in the future when we have fixes now
benifits no one at all, and in fact HURTS everyone involved, including
our users the most.

> While I think this is good for everyone, this patch represents my
> personal opinion and not that of my employer.

I appreciate the disclaimer :)

I know Thomas and others are totally busy with Plumbers right now (as am
I), so I'll hold on to this and your next patch in my "to-review" queue
to give others a chance to weigh in on the tweaks to see if anyone
disagrees with my comments above.

thanks,

greg k-h

Re: [PATCH 2/4] Documentation/process: describe relaxing disclosing party NDAs

[Greg KH]

On Tue, Sep 10, 2019 at 10:26:49AM -0700, Dave Hansen wrote:
> 
> From: Dave Hansen 
> 
> Hardware companies like Intel have lots of information which they
> want to disclose to some folks but not others.  Non-disclosure
> agreements are a tool of choice for helping to ensure that the
> flow of information is controlled.
> 
> But, they have caused problems in mitigation development.  It
> can be hard for individual developers employed by companies to
> figure out how they can participate, especially if their
> employer is under an NDA.
> 
> To make this easier for developers, make it clear to disclosing
> parties that they are expected to give permission for individuals
> to participate in mitigation efforts.
> 
> Cc: Jonathan Corbet 
> Cc: Greg Kroah-Hartman 
> Cc: Sasha Levin 
> Cc: Ben Hutchings 
> Cc: Thomas Gleixner 
> Cc: Laura Abbott 
> Cc: Andrew Cooper 
> Cc: Trilok Soni 
> Cc: Kees Cook 
> Cc: Tony Luck 
> Cc: linux-doc@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Acked-by: Dan Williams 
> Signed-off-by: Dave Hansen 
> ---
> 
>  b/Documentation/process/embargoed-hardware-issues.rst |7 +++
>  1 file changed, 7 insertions(+)
> 
> diff -puN Documentation/process/embargoed-hardware-issues.rst~hw-sec-0 
> Documentation/process/embargoed-hardware-issues.rst
> --- a/Documentation/process/embargoed-hardware-issues.rst~hw-sec-0
> 2019-09-10 08:39:02.835488131 -0700
> +++ b/Documentation/process/embargoed-hardware-issues.rst 2019-09-10 
> 08:39:02.838488131 -0700
> @@ -74,6 +74,13 @@ unable to enter into any non-disclosure
>  is aware of the sensitive nature of such issues and offers a Memorandum of
>  Understanding instead.
>  
> +Disclosing parties may have shared information about an issue under a
> +non-disclosure agreement with third parties.  In order to ensure that
> +these agreements do not interfere with the mitigation development
> +process, the disclosing party must provide explicit permission to
> +participate to any response team members affected by a non-disclosure
> +agreement.  Disclosing parties must resolve requests to do so in a
> +timely manner.

I wrote a fun long rant of a response here, but deleted it so now I feel
better.  But that doesn't help anyone but myself, so here's my censored
response instead...

Intel had months of review time for this document before this was
published.  Your lawyers had it and never objected to this lack of
inclusion at all, and explictitly said that the document as written was
fine with them.  So I'm sorry, but it is much too late to add something
like this to the document at this point in time.

If your legal department has any remaining objections like this, please
bring it up in the proper legal forum where all of the other companies
that already discussed this in can review and discuss it.  As it is,
including something like this would require their buy-in anyway, and
obviously that did not happen with this proposal.

So no, I'm not going to apply this change, sorry.

Oh, and cute use of the term, "timely manner", as if we are going to
fall for that one again... :)

greg k-h

Re: [PATCH 0/4] Documentation/process: embargoed hardware issues additions

[Greg KH]

On Tue, Sep 10, 2019 at 10:26:44AM -0700, Dave Hansen wrote:
> Intel will adhere to this process for future hardware embargoed
> issues.  This series contains a patch from Tony Luck with him
> volunteering to be Intel's ambassador for this process.
> 
> These are some minor improvements here to the process document.  I've
> had the pleasure of seeing some of the problems with the various
> "processes" that led to the need for this document and I think these
> tweaks will help.  This part of the series is much more of an RFC than
> the first patch, obviously.

I've applied patch 1 to the tree, thank you for that.

The other patches will take a bit more to think about, and calm down
about, before I'll respond.  Please wait until next week for that...

thanks,

greg k-h

Re: [PATCH] Documentation/process/embargoed-hardware-issues: Microsoft ambassador

[Greg KH]

On Fri, Sep 06, 2019 at 08:57:28AM -0600, Jonathan Corbet wrote:
> On Fri,  6 Sep 2019 05:58:52 -0400
> Sasha Levin  wrote:
> 
> > Add Sasha Levin as Microsoft's process ambassador.
> > 
> > Signed-off-by: Sasha Levin 
> > Signed-off-by: Sasha Levin 
> > ---
> >  Documentation/process/embargoed-hardware-issues.rst | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/process/embargoed-hardware-issues.rst 
> > b/Documentation/process/embargoed-hardware-issues.rst
> > index d37cbc502936d..9a92ccdbce74e 100644
> > --- a/Documentation/process/embargoed-hardware-issues.rst
> > +++ b/Documentation/process/embargoed-hardware-issues.rst
> > @@ -219,7 +219,7 @@ an involved disclosed party. The current ambassadors 
> > list:
> >Intel
> >Qualcomm
> >  
> > -  Microsoft
> > +  MicrosoftSasha Levin 
> >VMware
> >XEN
> 
> This document went upstream via Greg's tree, so updates are awkward for
> me to apply without having to explain a late backmerge to Linus.  I can
> hold them until after the merge window, unless you (Greg) would like to
> take them sooner?

I already have 3 of these queued up in my tree to go to Linus in the
next few days, so no need to worry about them.

thanks,

greg k-h

Re: [PATCH v2 09/11] coresight: etm4x: docs: Update ABI doc for sysfs features added.

[Greg KH]

On Wed, Sep 04, 2019 at 10:05:51AM -0600, Mathieu Poirier wrote:
> On Tue, 3 Sep 2019 at 23:48, Greg KH  wrote:
> >
> > On Tue, Sep 03, 2019 at 04:51:40PM -0600, Mathieu Poirier wrote:
> > > On Tue, 3 Sep 2019 at 13:59, Greg KH  wrote:
> > > >
> > > > On Thu, Aug 29, 2019 at 10:33:19PM +0100, Mike Leach wrote:
> > > > > Update document to include the new sysfs features added during this
> > > > > patchset.
> > > > >
> > > > > Updated to reflect the new sysfs component nameing schema.
> > > > >
> > > > > Signed-off-by: Mike Leach 
> > > > > ---
> > > > >  .../testing/sysfs-bus-coresight-devices-etm4x | 183 
> > > > > +++---
> > > > >  1 file changed, 115 insertions(+), 68 deletions(-)
> > > > >
> > > > > diff --git 
> > > > > a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x 
> > > > > b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> > > > > index 36258bc1b473..112c50ae9986 100644
> > > > > --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> > > > > +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> > > > > @@ -1,4 +1,4 @@
> > > > > -What:
> > > > > /sys/bus/coresight/devices/.etm/enable_source
> > > > > +What:/sys/bus/coresight/devices/etm/enable_source
> > > >
> > > > You are renaming sysfs directories that have been around since:
> > > >
> > > > >  Date:April 2015
> > > >
> > > > ???
> > > >
> > > > Really?
> > > >
> > > > That's brave.
> > >
> > >
> > > When I worked on the coresight sysfs ABI a while back I specifically
> > > added it at the "testing" level as I was well aware that things could
> > > change in the future.  According to the guidelines in the
> > > documentation userspace can rely on it which was accurate since the
> > > interface didn't change for 4 years.  But the guidelines also mention
> > > that changes can occur before the interfaces are move to stables, and
> > > that programs are encouraged to manifest their interest by adding
> > > their name to the "users" field.
> > >
> > > The interface was changed in 5.2 to support coresight from ACPI and
> > > make things easier to understand for users.  It is a lot more
> > > intuitive to associate an ETM tracer with the CPU it belongs to by
> > > referring to the CPU number than the memory mapped address.  Given the
> > > "testing" status of the interface and the absence of registered users
> > > I decided to move forward with the change.  If "testing" is too strict
> > > for that I suggest to add an "experimental" category where it would be
> > > more acceptable to change things as subsystems mature.
> >
> > "testing" is not really "testing" if you have userspace tools/programs
> > assuming the location and contents of specific files in sysfs.
> >
> > You can change things in sysfs by creating new files, but to do
> > wholesale renaming like you did here can be very dangerous as you might
> > be breaking things.
> 
> Yes, something I have definitely considered.
> 
> > Usually new files are created, not existing ones
> > moved.
> 
> In this case it would have meant a new symbolic link for every
> coresight device, so twice a many entries under
> $(SYS)/bus/coresight/device/.  That would have been a lot of clutter
> and an increasing source of problems as the number of CPU and sinks
> increases.  To me, and given the permissive definition of "testing"
> found in the documentation, a clean break was a better option.

Well, "testing" doesn't really matter in the end, if a tool/user relies
on it, we have to keep it working properly.

> > What tools use these today?  What is going to break?
> 
> Other than local shell scripts I am not aware of any tools using these
> today.  I am certainly open to discuss a better alternative but right
> now, I just don't see one.

Be aware that you might have to change this back if there is any
objections.

thanks,

greg k-h

Re: [PATCH v2 09/11] coresight: etm4x: docs: Update ABI doc for sysfs features added.

[Greg KH]

On Tue, Sep 03, 2019 at 04:51:40PM -0600, Mathieu Poirier wrote:
> On Tue, 3 Sep 2019 at 13:59, Greg KH  wrote:
> >
> > On Thu, Aug 29, 2019 at 10:33:19PM +0100, Mike Leach wrote:
> > > Update document to include the new sysfs features added during this
> > > patchset.
> > >
> > > Updated to reflect the new sysfs component nameing schema.
> > >
> > > Signed-off-by: Mike Leach 
> > > ---
> > >  .../testing/sysfs-bus-coresight-devices-etm4x | 183 +++---
> > >  1 file changed, 115 insertions(+), 68 deletions(-)
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x 
> > > b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> > > index 36258bc1b473..112c50ae9986 100644
> > > --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> > > +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> > > @@ -1,4 +1,4 @@
> > > -What:
> > > /sys/bus/coresight/devices/.etm/enable_source
> > > +What:/sys/bus/coresight/devices/etm/enable_source
> >
> > You are renaming sysfs directories that have been around since:
> >
> > >  Date:April 2015
> >
> > ???
> >
> > Really?
> >
> > That's brave.
> 
> 
> When I worked on the coresight sysfs ABI a while back I specifically
> added it at the "testing" level as I was well aware that things could
> change in the future.  According to the guidelines in the
> documentation userspace can rely on it which was accurate since the
> interface didn't change for 4 years.  But the guidelines also mention
> that changes can occur before the interfaces are move to stables, and
> that programs are encouraged to manifest their interest by adding
> their name to the "users" field.
> 
> The interface was changed in 5.2 to support coresight from ACPI and
> make things easier to understand for users.  It is a lot more
> intuitive to associate an ETM tracer with the CPU it belongs to by
> referring to the CPU number than the memory mapped address.  Given the
> "testing" status of the interface and the absence of registered users
> I decided to move forward with the change.  If "testing" is too strict
> for that I suggest to add an "experimental" category where it would be
> more acceptable to change things as subsystems mature.

"testing" is not really "testing" if you have userspace tools/programs
assuming the location and contents of specific files in sysfs.

You can change things in sysfs by creating new files, but to do
wholesale renaming like you did here can be very dangerous as you might
be breaking things.  Usually new files are created, not existing ones
moved.

What tools use these today?  What is going to break?

thanks,
greg k-h

Re: [PATCH v2 09/11] coresight: etm4x: docs: Update ABI doc for sysfs features added.

[Greg KH]

On Thu, Aug 29, 2019 at 10:33:19PM +0100, Mike Leach wrote:
> Update document to include the new sysfs features added during this
> patchset.
> 
> Updated to reflect the new sysfs component nameing schema.
> 
> Signed-off-by: Mike Leach 
> ---
>  .../testing/sysfs-bus-coresight-devices-etm4x | 183 +++---
>  1 file changed, 115 insertions(+), 68 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x 
> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> index 36258bc1b473..112c50ae9986 100644
> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> @@ -1,4 +1,4 @@
> -What:
> /sys/bus/coresight/devices/.etm/enable_source
> +What:/sys/bus/coresight/devices/etm/enable_source

You are renaming sysfs directories that have been around since:

>  Date:April 2015

???

Really?

That's brave.

What tool did you just break?

greg k-h

Re: [PATCH v5 0/9] FPGA DFL updates

[Greg KH]

On Mon, Aug 19, 2019 at 01:31:33PM +0800, Wu Hao wrote:
> On Mon, Aug 12, 2019 at 10:49:55AM +0800, Wu Hao wrote:
> > Hi Greg,
> > 
> > This is v5 patchset which adds more features to FPGA DFL. Marjor changes
> > against v4 are sysfs related code rework to address comments on v4.
> > 
> > Please help to take a look. Thanks!
> 
> Hi Greg,
> 
> Did you get a chance to take a look at this new version patchset? :)

I'm not the FPGA maintainer, what about the review from the other one
first?  :)

thanks,

greg k-h

Re: [PATCH v4 11/12] fpga: dfl: fme: add global error reporting support

[Greg KH]

On Wed, Aug 07, 2019 at 04:08:25PM +0800, Wu Hao wrote:
> On Wed, Aug 07, 2019 at 10:45:22AM +0800, Wu Hao wrote:
> > On Mon, Aug 05, 2019 at 05:56:26PM +0200, Greg KH wrote:
> > > On Sun, Aug 04, 2019 at 06:20:21PM +0800, Wu Hao wrote:
> > > > +static int fme_global_err_init(struct platform_device *pdev,
> > > > +  struct dfl_feature *feature)
> > > > +{
> > > > +   struct device *dev;
> > > > +   int ret = 0;
> > > > +
> > > > +   dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> > > > +   if (!dev)
> > > > +   return -ENOMEM;
> > > > +
> > > > +   dev->parent = &pdev->dev;
> > > > +   dev->release = err_dev_release;
> > > > +   dev_set_name(dev, "errors");
> > > > +
> > > > +   fme_error_enable(feature);
> > > > +
> > > > +   ret = device_register(dev);
> > > > +   if (ret) {
> > > > +   put_device(dev);
> > > > +   return ret;
> > > > +   }
> > > > +
> > > > +   ret = device_add_groups(dev, error_groups);
> > > 
> > > cute, but no, you do not create a whole struct device for a subdir.  Use
> > > the attribute group name like you did on earlier patches.
> > 
> > Sure, let me fix it in the next version.
> > 
> > > 
> > > And again, you raced userspace and lost :(
> > 
> > Same here, could you please give some more hints here?
> 
> Oh.. I see..
> 
> I should follow [1] as this is a platform driver. I will fix it. Thanks!
> 
> [PATCH 00/11] Platform drivers, provide a way to add sysfs groups easily
> 
> [1]https://lkml.org/lkml/2019/7/4/181

Yes, that is the correct thing to do.

thanks,

greg k-h

Re: [PATCH v4 11/12] fpga: dfl: fme: add global error reporting support

[Greg KH]

On Sun, Aug 04, 2019 at 06:20:21PM +0800, Wu Hao wrote:
> +static int fme_global_err_init(struct platform_device *pdev,
> +struct dfl_feature *feature)
> +{
> + struct device *dev;
> + int ret = 0;
> +
> + dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> + if (!dev)
> + return -ENOMEM;
> +
> + dev->parent = &pdev->dev;
> + dev->release = err_dev_release;
> + dev_set_name(dev, "errors");
> +
> + fme_error_enable(feature);
> +
> + ret = device_register(dev);
> + if (ret) {
> + put_device(dev);
> + return ret;
> + }
> +
> + ret = device_add_groups(dev, error_groups);

cute, but no, you do not create a whole struct device for a subdir.  Use
the attribute group name like you did on earlier patches.

And again, you raced userspace and lost :(

thanks,

greg k-h

Re: [PATCH v4 07/12] fpga: dfl: afu: add error reporting support.

[Greg KH]

On Sun, Aug 04, 2019 at 06:20:17PM +0800, Wu Hao wrote:
> Error reporting is one important private feature, it reports error
> detected on port and accelerated function unit (AFU). It introduces
> several sysfs interfaces to allow userspace to check and clear
> errors detected by hardware.
> 
> Signed-off-by: Xu Yilun 
> Signed-off-by: Wu Hao 
> Acked-by: Alan Tull 
> Signed-off-by: Moritz Fischer 
> ---
> v2: switch to device_add/remove_group for sysfs.
> v3: update kernel version and date in sysfs doc
> v4: remove dev_dbg in init/uinit callback function.
> ---
>  Documentation/ABI/testing/sysfs-platform-dfl-port |  39 
>  drivers/fpga/Makefile |   1 +
>  drivers/fpga/dfl-afu-error.c  | 221 
> ++
>  drivers/fpga/dfl-afu-main.c   |   4 +
>  drivers/fpga/dfl-afu.h|   4 +
>  5 files changed, 269 insertions(+)
>  create mode 100644 drivers/fpga/dfl-afu-error.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-platform-dfl-port 
> b/Documentation/ABI/testing/sysfs-platform-dfl-port
> index 5663441..3b6580b 100644
> --- a/Documentation/ABI/testing/sysfs-platform-dfl-port
> +++ b/Documentation/ABI/testing/sysfs-platform-dfl-port
> @@ -81,3 +81,42 @@ KernelVersion: 5.4
>  Contact: Wu Hao 
>  Description: Read-only. Read this file to get the status of issued command
>   to userclck_freqcntrcmd.
> +
> +What:/sys/bus/platform/devices/dfl-port.0/errors/revision
> +Date:August 2019
> +KernelVersion:   5.4
> +Contact: Wu Hao 
> +Description: Read-only. Read this file to get the revision of this error
> + reporting private feature.

Same revision question here that I had on an earlier patch.


> +
> +What:/sys/bus/platform/devices/dfl-port.0/errors/errors
> +Date:August 2019
> +KernelVersion:   5.4
> +Contact: Wu Hao 
> +Description: Read-only. Read this file to get errors detected on port and
> + Accelerated Function Unit (AFU).
> +
> +What:/sys/bus/platform/devices/dfl-port.0/errors/first_error
> +Date:August 2019
> +KernelVersion:   5.4
> +Contact: Wu Hao 
> +Description: Read-only. Read this file to get the first error detected by
> + hardware.
> +
> +What:
> /sys/bus/platform/devices/dfl-port.0/errors/first_malformed_req
> +Date:August 2019
> +KernelVersion:   5.4
> +Contact: Wu Hao 
> +Description: Read-only. Read this file to get the first malformed request
> + captured by hardware.
> +
> +What:/sys/bus/platform/devices/dfl-port.0/errors/clear
> +Date:August 2019
> +KernelVersion:   5.4
> +Contact: Wu Hao 
> +Description: Write-only. Write error code to this file to clear errors.
> + Write fails with -EINVAL if input parsing fails or input error
> + code doesn't match.
> + Write fails with -EBUSY or -ETIMEDOUT if error can't be cleared
> + as hardware is in low power state (-EBUSY) or not responding
> + (-ETIMEDOUT).
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 312b937..7255891 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -41,6 +41,7 @@ obj-$(CONFIG_FPGA_DFL_AFU)  += dfl-afu.o
>  
>  dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o
>  dfl-afu-objs := dfl-afu-main.o dfl-afu-region.o dfl-afu-dma-region.o
> +dfl-afu-objs += dfl-afu-error.o
>  
>  # Drivers for FPGAs which implement DFL
>  obj-$(CONFIG_FPGA_DFL_PCI)   += dfl-pci.o
> diff --git a/drivers/fpga/dfl-afu-error.c b/drivers/fpga/dfl-afu-error.c
> new file mode 100644
> index 000..c5e0efa
> --- /dev/null
> +++ b/drivers/fpga/dfl-afu-error.c
> @@ -0,0 +1,221 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for FPGA Accelerated Function Unit (AFU) Error Reporting
> + *
> + * Copyright 2019 Intel Corporation, Inc.
> + *
> + * Authors:
> + *   Wu Hao 
> + *   Xiao Guangrong 
> + *   Joseph Grecco 
> + *   Enno Luebbers 
> + *   Tim Whisonant 
> + *   Ananda Ravuri 
> + *   Mitchel Henry 
> + */
> +
> +#include 
> +
> +#include "dfl-afu.h"
> +
> +#define PORT_ERROR_MASK  0x8
> +#define PORT_ERROR   0x10
> +#define PORT_FIRST_ERROR 0x18
> +#define PORT_MALFORMED_REQ0  0x20
> +#define PORT_MALFORMED_REQ1  0x28
> +
> +#define ERROR_MASK   GENMASK_ULL(63, 0)
> +
> +/* mask or unmask port errors by the error mask register. */
> +static void __port_err_mask(struct device *dev, bool mask)
> +{
> + void __iomem *base;
> +
> + base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_ERROR);
> +
> + writeq(mask ? ERROR_MASK : 0, base + PORT_ERROR_MASK);
> +}
> +
> +/* clear port errors. */
> +static int __port_err_clear(struct device *dev, u64 err)
> +{
> + struct platform_device *pdev = to_platform_device(dev

Re: [PATCH v4 06/12] fpga: dfl: afu: export __port_enable/disable function.

[Greg KH]

On Sun, Aug 04, 2019 at 06:20:16PM +0800, Wu Hao wrote:
> As these two functions are used by other private features. e.g.
> in error reporting private feature, it requires to check port status
> and reset port for error clearing.
> 
> Signed-off-by: Xu Yilun 
> Signed-off-by: Wu Hao 
> Acked-by: Moritz Fischer 
> Acked-by: Alan Tull 
> Signed-off-by: Moritz Fischer 
> ---
> v2: rebased
> ---
>  drivers/fpga/dfl-afu-main.c | 25 ++---
>  drivers/fpga/dfl-afu.h  |  3 +++
>  2 files changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
> index e013afb..e312179 100644
> --- a/drivers/fpga/dfl-afu-main.c
> +++ b/drivers/fpga/dfl-afu-main.c
> @@ -22,14 +22,16 @@
>  #include "dfl-afu.h"
>  
>  /**
> - * port_enable - enable a port
> + * __port_enable - enable a port
>   * @pdev: port platform device.
>   *
>   * Enable Port by clear the port soft reset bit, which is set by default.
>   * The AFU is unable to respond to any MMIO access while in reset.
> - * port_enable function should only be used after port_disable function.
> + * __port_enable function should only be used after __port_disable function.
> + *
> + * The caller needs to hold lock for protection.
>   */
> -static void port_enable(struct platform_device *pdev)
> +void __port_enable(struct platform_device *pdev)

worst global function name ever.

Don't polute the global namespace like this for a single driver.  If you
REALLY need it, then use a prefix that shows it is your individual
dfl_special_sauce_platform_device_only type thing.

thanks,

greg k-h

Re: [PATCH v4 04/12] fpga: dfl: afu: add userclock sysfs interfaces.

[Greg KH]

On Sun, Aug 04, 2019 at 06:20:14PM +0800, Wu Hao wrote:
> This patch introduces userclock sysfs interfaces for AFU, user
> could use these interfaces for clock setting to AFU.
> 
> Please note that, this is only working for port header feature
> with revision 0, for later revisions, userclock setting is moved
> to a separated private feature, so one revision sysfs interface
> is exposed to userspace application for this purpose too.
> 
> Signed-off-by: Ananda Ravuri 
> Signed-off-by: Russ Weight 
> Signed-off-by: Xu Yilun 
> Signed-off-by: Wu Hao 
> Acked-by: Alan Tull 
> Signed-off-by: Moritz Fischer 
> ---
> v2: rebased, and switched to use device_add/remove_groups for sysfs
> v3: update kernel version and date in sysfs doc
> v4: rebased.
> ---
>  Documentation/ABI/testing/sysfs-platform-dfl-port |  35 +++
>  drivers/fpga/dfl-afu-main.c   | 114 
> +-
>  drivers/fpga/dfl.h|   9 ++
>  3 files changed, 157 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-platform-dfl-port 
> b/Documentation/ABI/testing/sysfs-platform-dfl-port
> index 1ab3e6f..5663441 100644
> --- a/Documentation/ABI/testing/sysfs-platform-dfl-port
> +++ b/Documentation/ABI/testing/sysfs-platform-dfl-port
> @@ -46,3 +46,38 @@ Contact:   Wu Hao 
>  Description: Read-write. Read or set AFU latency tolerance reporting value.
>   Set ltr to 1 if the AFU can tolerate latency >= 40us or set it
>   to 0 if it is latency sensitive.
> +
> +What:/sys/bus/platform/devices/dfl-port.0/revision
> +Date:August 2019
> +KernelVersion:   5.4
> +Contact: Wu Hao 
> +Description: Read-only. Read this file to get the revision of port header
> + feature.

What does "revision" mean?

It feels like you are creating a different set of sysfs files depending
on the revision field.  Which is fine, sysfs is one-value-per-file and
userspace needs to handle if the file is present or not.  So why not
just rely on that and not have to mess with 'revision' at all?  What is
userspace going to do with that information?

> +
> +What:/sys/bus/platform/devices/dfl-port.0/userclk_freqcmd
> +Date:August 2019
> +KernelVersion:   5.4
> +Contact: Wu Hao 
> +Description: Write-only. User writes command to this interface to set
> + userclock to AFU.
> +
> +What:/sys/bus/platform/devices/dfl-port.0/userclk_freqsts
> +Date:August 2019
> +KernelVersion:   5.4
> +Contact: Wu Hao 
> +Description: Read-only. Read this file to get the status of issued command
> + to userclck_freqcmd.
> +
> +What:/sys/bus/platform/devices/dfl-port.0/userclk_freqcntrcmd
> +Date:August 2019
> +KernelVersion:   5.4
> +Contact: Wu Hao 
> +Description: Write-only. User writes command to this interface to set
> + userclock counter.
> +
> +What:/sys/bus/platform/devices/dfl-port.0/userclk_freqcntrsts
> +Date:August 2019
> +KernelVersion:   5.4
> +Contact: Wu Hao 
> +Description: Read-only. Read this file to get the status of issued command
> + to userclck_freqcntrcmd.
> diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
> index 12175bb..407c97d 100644
> --- a/drivers/fpga/dfl-afu-main.c
> +++ b/drivers/fpga/dfl-afu-main.c
> @@ -142,6 +142,17 @@ static int port_get_id(struct platform_device *pdev)
>  static DEVICE_ATTR_RO(id);
>  
>  static ssize_t
> +revision_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + void __iomem *base;
> +
> + base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
> +
> + return sprintf(buf, "%x\n", dfl_feature_revision(base));
> +}
> +static DEVICE_ATTR_RO(revision);
> +
> +static ssize_t
>  ltr_show(struct device *dev, struct device_attribute *attr, char *buf)
>  {
>   struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
> @@ -276,6 +287,7 @@ static int port_get_id(struct platform_device *pdev)
>  
>  static struct attribute *port_hdr_attrs[] = {
>   &dev_attr_id.attr,
> + &dev_attr_revision.attr,
>   &dev_attr_ltr.attr,
>   &dev_attr_ap1_event.attr,
>   &dev_attr_ap2_event.attr,
> @@ -284,14 +296,113 @@ static int port_get_id(struct platform_device *pdev)
>  };
>  ATTRIBUTE_GROUPS(port_hdr);
>  
> +static ssize_t
> +userclk_freqcmd_store(struct device *dev, struct device_attribute *attr,
> +   const char *buf, size_t count)
> +{
> + struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
> + u64 userclk_freq_cmd;
> + void __iomem *base;
> +
> + if (kstrtou64(buf, 0, &userclk_freq_cmd))
> + return -EINVAL;
> +
> + base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
> +
> + mutex_lock(&pdata->lock);
> + writeq(userclk_freq_cmd, b

Re: [PATCH 1/1] psi: do not require setsched permission from the trigger creator

[Greg KH]

On Mon, Jul 29, 2019 at 12:42:05PM -0700, Suren Baghdasaryan wrote:
> When a process creates a new trigger by writing into /proc/pressure/*
> files, permissions to write such a file should be used to determine whether
> the process is allowed to do so or not. Current implementation would also
> require such a process to have setsched capability. Setting of psi trigger
> thread's scheduling policy is an implementation detail and should not be
> exposed to the user level. Remove the permission check by using _nocheck
> version of the function.
> 
> Suggested-by: Nick Kralevich 
> Signed-off-by: Suren Baghdasaryan 
> ---
>  kernel/sched/psi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

$ ./scripts/get_maintainer.pl --file kernel/sched/psi.c
Ingo Molnar  (maintainer:SCHEDULER)
Peter Zijlstra  (maintainer:SCHEDULER)
linux-ker...@vger.kernel.org (open list:SCHEDULER)


No where am I listed there, so why did you send this "To:" me?

please fix up and resend.

greg k-h

Re: [PATCH v3 09/12] fpga: dfl: afu: add STP (SignalTap) support

[Greg KH]

On Tue, Jul 23, 2019 at 12:51:32PM +0800, Wu Hao wrote:
> STP (SignalTap) is one of the private features under the port for
> debugging. This patch adds private feature driver support for it
> to allow userspace applications to mmap related mmio region and
> provide STP service.
> 
> Signed-off-by: Xu Yilun 
> Signed-off-by: Wu Hao 
> Acked-by: Moritz Fischer 
> Acked-by: Alan Tull 
> Signed-off-by: Moritz Fischer 
> ---
>  drivers/fpga/dfl-afu-main.c | 34 ++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
> index 15dd4cb..395f96e 100644
> --- a/drivers/fpga/dfl-afu-main.c
> +++ b/drivers/fpga/dfl-afu-main.c
> @@ -514,6 +514,36 @@ static void port_afu_uinit(struct platform_device *pdev,
>   .uinit = port_afu_uinit,
>  };
>  
> +static int port_stp_init(struct platform_device *pdev,
> +  struct dfl_feature *feature)
> +{
> + struct resource *res = &pdev->resource[feature->resource_index];
> +
> + dev_dbg(&pdev->dev, "PORT STP Init.\n");

ftrace is your friend, no need to do a lot of "look I am here!"
messages.

> +
> + return afu_mmio_region_add(dev_get_platdata(&pdev->dev),
> +DFL_PORT_REGION_INDEX_STP,
> +resource_size(res), res->start,
> +DFL_PORT_REGION_MMAP | DFL_PORT_REGION_READ |
> +DFL_PORT_REGION_WRITE);
> +}
> +
> +static void port_stp_uinit(struct platform_device *pdev,
> +struct dfl_feature *feature)
> +{
> + dev_dbg(&pdev->dev, "PORT STP UInit.\n");

Same here.

Why have this function at all if it does not do anything?


thanks,

greg k-h

Re: [PATCH v3 04/12] fpga: dfl: afu: add AFU state related sysfs interfaces

[Greg KH]

On Tue, Jul 23, 2019 at 12:51:27PM +0800, Wu Hao wrote:
> This patch introduces more sysfs interfaces for Accelerated
> Function Unit (AFU). These interfaces allow users to read
> current AFU Power State (APx), read / clear AFU Power (APx)
> events which are sticky to identify transient APx state,
> and manage AFU's LTR (latency tolerance reporting).
> 
> Signed-off-by: Ananda Ravuri 
> Signed-off-by: Xu Yilun 
> Signed-off-by: Wu Hao 
> Acked-by: Alan Tull 
> Signed-off-by: Moritz Fischer 
> ---
> v2: rebased, and remove DRV/MODULE_VERSION modifications
> v3: update kernel version and date in sysfs doc
> ---
>  Documentation/ABI/testing/sysfs-platform-dfl-port |  30 +
>  drivers/fpga/dfl-afu-main.c   | 137 
> ++
>  drivers/fpga/dfl.h|  11 ++
>  3 files changed, 178 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-platform-dfl-port 
> b/Documentation/ABI/testing/sysfs-platform-dfl-port
> index 6a92dda..5961fb2 100644
> --- a/Documentation/ABI/testing/sysfs-platform-dfl-port
> +++ b/Documentation/ABI/testing/sysfs-platform-dfl-port
> @@ -14,3 +14,33 @@ Description:   Read-only. User can program different 
> PR bitstreams to FPGA
>   Accelerator Function Unit (AFU) for different functions. It
>   returns uuid which could be used to identify which PR bitstream
>   is programmed in this AFU.
> +
> +What:/sys/bus/platform/devices/dfl-port.0/power_state
> +Date:July 2019
> +KernelVersion:   5.4
> +Contact: Wu Hao 
> +Description: Read-only. It reports the APx (AFU Power) state, different APx
> + means different throttling level. When reading this file, it
> + returns "0" - Normal / "1" - AP1 / "2" - AP2 / "6" - AP6.
> +
> +What:/sys/bus/platform/devices/dfl-port.0/ap1_event
> +Date:July 2019
> +KernelVersion:   5.4
> +Contact: Wu Hao 
> +Description: Read-write. Read or set 1 to clear AP1 (AFU Power State 1)
> + event. It's used to indicate transient AP1 state.

So reading the value changes the state of the system?  That's almost
always never a good idea.

Force userspace to write the value to change something.  Otherwise all
libraries that use sysfs will be accidentally changing the state of your
system without you ever knowing it.



> +
> +What:/sys/bus/platform/devices/dfl-port.0/ap2_event
> +Date:July 2019
> +KernelVersion:   5.4
> +Contact: Wu Hao 
> +Description: Read-write. Read or set 1 to clear AP2 (AFU Power State 2)
> + event. It's used to indicate transient AP2 state.
> +
> +What:/sys/bus/platform/devices/dfl-port.0/ltr
> +Date:July 2019
> +KernelVersion:   5.4
> +Contact: Wu Hao 
> +Description: Read-write. Read and set AFU latency tolerance reporting value.
> + Set ltr to 1 if the AFU can tolerate latency >= 40us or set it
> + to 0 if it is latency sensitive.
> diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
> index 68b4d08..cb3f73e 100644
> --- a/drivers/fpga/dfl-afu-main.c
> +++ b/drivers/fpga/dfl-afu-main.c
> @@ -141,8 +141,145 @@ static int port_get_id(struct platform_device *pdev)
>  }
>  static DEVICE_ATTR_RO(id);
>  
> +static ssize_t
> +ltr_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
> + void __iomem *base;
> + u64 v;
> +
> + base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
> +
> + mutex_lock(&pdata->lock);
> + v = readq(base + PORT_HDR_CTRL);
> + mutex_unlock(&pdata->lock);

Why do you need a lock to call readq()?  What are you protecting here?


> +
> + return sprintf(buf, "%x\n", (u8)FIELD_GET(PORT_CTRL_LATENCY, v));
> +}
> +
> +static ssize_t
> +ltr_store(struct device *dev, struct device_attribute *attr,
> +   const char *buf, size_t count)
> +{
> + struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
> + void __iomem *base;
> + u8 ltr;
> + u64 v;
> +
> + if (kstrtou8(buf, 0,  1)
> + return -EINVAL;

Are you doing anything with this value?  If not, how about just using
the sysfs boolean read function and if it is 1, then do the clearing?

Same for all other show/store functions in here.

thanks,

greg k-h

Re: [PATCH v3 03/12] fpga: dfl: pci: enable SRIOV support.

[Greg KH]

On Tue, Jul 23, 2019 at 12:51:26PM +0800, Wu Hao wrote:
> This patch enables the standard sriov support. It allows user to
> enable SRIOV (and VFs), then user could pass through accelerators
> (VFs) into virtual machine or use VFs directly in host.
> 
> Signed-off-by: Zhang Yi Z 
> Signed-off-by: Xu Yilun 
> Signed-off-by: Wu Hao 
> Acked-by: Alan Tull 
> Acked-by: Moritz Fischer 
> Signed-off-by: Moritz Fischer 
> ---
> v2: remove DRV/MODULE_VERSION modifications.
> ---
>  drivers/fpga/dfl-pci.c | 39 +++
>  drivers/fpga/dfl.c | 41 +
>  drivers/fpga/dfl.h |  1 +
>  3 files changed, 81 insertions(+)
> 
> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
> index 66b5720..0e65d81 100644
> --- a/drivers/fpga/dfl-pci.c
> +++ b/drivers/fpga/dfl-pci.c
> @@ -223,8 +223,46 @@ int cci_pci_probe(struct pci_dev *pcidev, const struct 
> pci_device_id *pcidevid)
>   return ret;
>  }
>  
> +static int cci_pci_sriov_configure(struct pci_dev *pcidev, int num_vfs)
> +{
> + struct cci_drvdata *drvdata = pci_get_drvdata(pcidev);
> + struct dfl_fpga_cdev *cdev = drvdata->cdev;
> + int ret = 0;
> +
> + mutex_lock(&cdev->lock);
> +
> + if (!num_vfs) {
> + /*
> +  * disable SRIOV and then put released ports back to default
> +  * PF access mode.
> +  */
> + pci_disable_sriov(pcidev);
> +
> + __dfl_fpga_cdev_config_port_vf(cdev, false);
> +
> + } else if (cdev->released_port_num == num_vfs) {
> + /*
> +  * only enable SRIOV if cdev has matched released ports, put
> +  * released ports into VF access mode firstly.
> +  */
> + __dfl_fpga_cdev_config_port_vf(cdev, true);
> +
> + ret = pci_enable_sriov(pcidev, num_vfs);
> + if (ret)
> + __dfl_fpga_cdev_config_port_vf(cdev, false);
> + } else {
> + ret = -EINVAL;
> + }
> +
> + mutex_unlock(&cdev->lock);
> + return ret;
> +}
> +
>  static void cci_pci_remove(struct pci_dev *pcidev)
>  {
> + if (dev_is_pf(&pcidev->dev))
> + cci_pci_sriov_configure(pcidev, 0);
> +
>   cci_remove_feature_devs(pcidev);
>   pci_disable_pcie_error_reporting(pcidev);
>  }
> @@ -234,6 +272,7 @@ static void cci_pci_remove(struct pci_dev *pcidev)
>   .id_table = cci_pcie_id_tbl,
>   .probe = cci_pci_probe,
>   .remove = cci_pci_remove,
> + .sriov_configure = cci_pci_sriov_configure,
>  };
>  
>  module_pci_driver(cci_pci_driver);
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index e04ed45..c3a8e1d 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -1112,6 +1112,47 @@ int dfl_fpga_cdev_config_port(struct dfl_fpga_cdev 
> *cdev, int port_id,
>  }
>  EXPORT_SYMBOL_GPL(dfl_fpga_cdev_config_port);
>  
> +static void config_port_vf(struct device *fme_dev, int port_id, bool is_vf)
> +{
> + void __iomem *base;
> + u64 v;
> +
> + base = dfl_get_feature_ioaddr_by_id(fme_dev, FME_FEATURE_ID_HEADER);
> +
> + v = readq(base + FME_HDR_PORT_OFST(port_id));
> +
> + v &= ~FME_PORT_OFST_ACC_CTRL;
> + v |= FIELD_PREP(FME_PORT_OFST_ACC_CTRL,
> + is_vf ? FME_PORT_OFST_ACC_VF : FME_PORT_OFST_ACC_PF);
> +
> + writeq(v, base + FME_HDR_PORT_OFST(port_id));
> +}
> +
> +/**
> + * __dfl_fpga_cdev_config_port_vf - configure port to VF access mode
> + *
> + * @cdev: parent container device.
> + * @if_vf: true for VF access mode, and false for PF access mode
> + *
> + * Return: 0 on success, negative error code otherwise.
> + *
> + * This function is needed in sriov configuration routine. It could be used 
> to
> + * configures the released ports access mode to VF or PF.
> + * The caller needs to hold lock for protection.
> + */
> +void __dfl_fpga_cdev_config_port_vf(struct dfl_fpga_cdev *cdev, bool is_vf)
> +{
> + struct dfl_feature_platform_data *pdata;
> +
> + list_for_each_entry(pdata, &cdev->port_dev_list, node) {
> + if (device_is_registered(&pdata->dev->dev))
> + continue;
> +
> + config_port_vf(cdev->fme_dev, pdata->id, is_vf);
> + }
> +}
> +EXPORT_SYMBOL_GPL(__dfl_fpga_cdev_config_port_vf);

Why are you exporting a function with a leading __?

You are expecting someone else, in who knows what code, to do locking
correctly?  If so, and the caller always has to have a local lock, then
it's not a big deal, just drop the '__', otherwise if you have to have a
specific lock for a specific device, then you have a really complex and
probably broken api here :(

thanks,

greg k-h

Re: [PATCH v3 01/12] fpga: dfl: fme: support 512bit data width PR

[Greg KH]

On Tue, Jul 23, 2019 at 12:51:24PM +0800, Wu Hao wrote:
> In early partial reconfiguration private feature, it only
> supports 32bit data width when writing data to hardware for
> PR. 512bit data width PR support is an important optimization
> for some specific solutions (e.g. XEON with FPGA integrated),
> it allows driver to use AVX512 instruction to improve the
> performance of partial reconfiguration. e.g. programming one
> 100MB bitstream image via this 512bit data width PR hardware
> only takes ~300ms, but 32bit revision requires ~3s per test
> result.
> 
> Please note now this optimization is only done on revision 2
> of this PR private feature which is only used in integrated
> solution that AVX512 is always supported. This revision 2
> hardware doesn't support 32bit PR.
> 
> Signed-off-by: Ananda Ravuri 
> Signed-off-by: Xu Yilun 
> Signed-off-by: Wu Hao 
> Acked-by: Alan Tull 
> Signed-off-by: Moritz Fischer 
> ---
> v2: remove DRV/MODULE_VERSION modifications
> ---
>  drivers/fpga/dfl-fme-mgr.c | 110 
> ++---
>  drivers/fpga/dfl-fme-pr.c  |  43 +++---
>  drivers/fpga/dfl-fme.h |   2 +
>  drivers/fpga/dfl.h |   5 +++
>  4 files changed, 129 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/fpga/dfl-fme-mgr.c b/drivers/fpga/dfl-fme-mgr.c
> index b3f7eee..46e17f0 100644
> --- a/drivers/fpga/dfl-fme-mgr.c
> +++ b/drivers/fpga/dfl-fme-mgr.c
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  
> +#include "dfl.h"
>  #include "dfl-fme-pr.h"
>  
>  /* FME Partial Reconfiguration Sub Feature Register Set */
> @@ -30,6 +31,7 @@
>  #define FME_PR_STS   0x10
>  #define FME_PR_DATA  0x18
>  #define FME_PR_ERR   0x20
> +#define FME_PR_512_DATA  0x40 /* Data Register for 512bit 
> datawidth PR */
>  #define FME_PR_INTFC_ID_L0xA8
>  #define FME_PR_INTFC_ID_H0xB0
>  
> @@ -67,8 +69,43 @@
>  #define PR_WAIT_TIMEOUT   800
>  #define PR_HOST_STATUS_IDLE  0
>  
> +#if defined(CONFIG_X86) && defined(CONFIG_AS_AVX512)
> +
> +#include 
> +#include 
> +
> +static inline int is_cpu_avx512_enabled(void)
> +{
> + return cpu_feature_enabled(X86_FEATURE_AVX512F);
> +}

That's a very arch specific function, why would a driver ever care about
this?

> +
> +static inline void copy512(const void *src, void __iomem *dst)
> +{
> + kernel_fpu_begin();
> +
> + asm volatile("vmovdqu64 (%0), %%zmm0;"
> +  "vmovntdq %%zmm0, (%1);"
> +  :
> +  : "r"(src), "r"(dst)
> +  : "memory");
> +
> + kernel_fpu_end();
> +}

Shouldn't this be an arch-specific function somewhere?  Burying this in
a random driver is not ok.  Please make this generic for all systems.

> +#else
> +static inline int is_cpu_avx512_enabled(void)
> +{
> + return 0;
> +}
> +
> +static inline void copy512(const void *src, void __iomem *dst)
> +{
> + WARN_ON_ONCE(1);

Are you trying to get reports from syzbot?  :)

Please fix this all up.

greg k-h

Re: [PATCH v3 02/12] fpga: dfl: fme: add DFL_FPGA_FME_PORT_RELEASE/ASSIGN ioctl support.

[Greg KH]

On Tue, Jul 23, 2019 at 12:51:25PM +0800, Wu Hao wrote:
> +/**
> + * dfl_fpga_cdev_config_port - configure a port feature dev
> + * @cdev: parent container device.
> + * @port_id: id of the port feature device.
> + * @release: release port or assign port back.
> + *
> + * This function allows user to release port platform device or assign it 
> back.
> + * e.g. to safely turn one port from PF into VF for PCI device SRIOV support,
> + * release port platform device is one necessary step.
> + */
> +int dfl_fpga_cdev_config_port(struct dfl_fpga_cdev *cdev, int port_id,
> +   bool release)
> +{
> + return release ? detach_port_dev(cdev, port_id) :
> +  attach_port_dev(cdev, port_id);
> +}
> +EXPORT_SYMBOL_GPL(dfl_fpga_cdev_config_port);

That's a horrible api.  Every time you see this call in code, you have
to go and look up what "bool" means here.  There's no reason for it.

Just have 2 different functions, one that attaches a port, and one that
detaches it.  That way when you read the code that calls this function,
you know what it does instantly without having to go look up some api
function somewhere else.

Write code for people to read first.  And you are saving nothing here by
trying to do two different things in the same exact function.

thanks,

greg k-h

Re: [PATCH] Documentation/security-bugs: provide more information about linux-distros

[Greg KH]

On Thu, Jul 11, 2019 at 12:36:37PM -0400, Sasha Levin wrote:
> Provide more information about how to interact with the linux-distros
> mailing list for disclosing security bugs.
> 
> First, clarify that the reporter must read and accept the linux-distros
> policies prior to sending a report.
> 
> Second, clarify that the reported must provide a tentative public
> disclosure date and time in his first contact with linux-distros.
> 
> Suggested-by: Solar Designer 
> Signed-off-by: Sasha Levin 
> ---
>  Documentation/admin-guide/security-bugs.rst | 21 +
>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/admin-guide/security-bugs.rst 
> b/Documentation/admin-guide/security-bugs.rst
> index dcd6c93c7aac..c62faced9256 100644
> --- a/Documentation/admin-guide/security-bugs.rst
> +++ b/Documentation/admin-guide/security-bugs.rst
> @@ -61,14 +61,19 @@ Coordination
>  
>  Fixes for sensitive bugs, such as those that might lead to privilege
>  escalations, may need to be coordinated with the private
> - mailing list so that distribution vendors
> -are well prepared to issue a fixed kernel upon public disclosure of the
> -upstream fix. Distros will need some time to test the proposed patch and
> -will generally request at least a few days of embargo, and vendor update
> -publication prefers to happen Tuesday through Thursday. When appropriate,
> -the security team can assist with this coordination, or the reporter can
> -include linux-distros from the start. In this case, remember to prefix
> -the email Subject line with "[vs]" as described in the linux-distros wiki:
> + mailing list so that distribution vendors are
> +well prepared to issue a fixed kernel upon public disclosure of the upstream
> +fix. As a reporter, you must read and accept the list's policy as outlined in
> +the linux-distros wiki:
> +.
> +When you report a bug, you must also provide a tentative disclosure date and
> +time in your very first message to the list. Distros will need some time to
> +test the proposed patch so please allow at least a few days of embargo, and
> +vendor update publication prefers to happen Tuesday through Thursday. When
> +appropriate, the security team can assist with this coordination, or the
> +reporter can include linux-distros from the start. In this case, remember to
> +prefix the email Subject line with "[vs]" as described in the linux-distros
> +wiki:
>  
> 

Do we really need to describe all of the information on how to use the
distro list here?  That's why we included the link, as it has all of
this well spelled out and described.  If anything, I would say we should
say less in this document about what linux-distros do, as that is
independent of the Linux security team.

thanks,

greg k-h

Re: [PATCH 06/15] fpga: dfl: fme: add DFL_FPGA_FME_PORT_RELEASE/ASSIGN ioctl support.

[Greg KH]

On Thu, Jul 04, 2019 at 04:58:55PM +0800, Wu Hao wrote:
> > > Hope things could be more clear now. :)
> > 
> > That's nice for the vfio stuff, but you are just a "normal" driver here.
> > You want an ioctl that just does one thing, no arguments, no flags, no
> > anything.  No need for a size argument then at all.  These ioctls don't
> > even need a structure for them!
> > 
> > Don't try to be fancy, it's not needed, it's not like you are running
> > out of ioctl space...
> 
> Thanks a lot for the comments and suggestions.
> 
> That's true, it's a "normal" driver, maybe I overly considered the
> extensibility of it. OK, Let me rework this patch to remove argsz from
> these two ioctls.
> 
> What about the existing ioctls for this driver, they have argsz too.
> shall I prepare another patch to remove them as well?

I am hoping you actually have users for those ioctls in userspace today?
If not, and no one is using them, then yes, please fix those too.

thanks,

greg k-h

Re: [PATCH 06/15] fpga: dfl: fme: add DFL_FPGA_FME_PORT_RELEASE/ASSIGN ioctl support.

[Greg KH]

On Thu, Jul 04, 2019 at 02:31:06PM +0800, Wu Hao wrote:
> On Thu, Jul 04, 2019 at 07:39:27AM +0200, Greg KH wrote:
> > On Thu, Jul 04, 2019 at 07:30:58AM +0800, Wu Hao wrote:
> > > On Wed, Jul 03, 2019 at 08:07:53PM +0200, Greg KH wrote:
> > > > On Thu, Jun 27, 2019 at 05:49:42PM -0700, Moritz Fischer wrote:
> > > > > From: Wu Hao 
> > > > > 
> > > > > In order to support virtualization usage via PCIe SRIOV, this patch
> > > > > adds two ioctls under FPGA Management Engine (FME) to release and
> > > > > assign back the port device. In order to safely turn Port from PF
> > > > > into VF and enable PCIe SRIOV, it requires user to invoke this
> > > > > PORT_RELEASE ioctl to release port firstly to remove userspace
> > > > > interfaces, and then configure the PF/VF access register in FME.
> > > > > After disable SRIOV, it requires user to invoke this PORT_ASSIGN
> > > > > ioctl to attach the port back to PF.
> > > > > 
> > > > >  Ioctl interfaces:
> > > > >  * DFL_FPGA_FME_PORT_RELEASE
> > > > >Release platform device of given port, it deletes port platform
> > > > >device to remove related userspace interfaces on PF, then
> > > > >configures PF/VF access mode to VF.
> > > > > 
> > > > >  * DFL_FPGA_FME_PORT_ASSIGN
> > > > >Assign platform device of given port back to PF, it configures
> > > > >PF/VF access mode to PF, then adds port platform device back to
> > > > >re-enable related userspace interfaces on PF.
> > > > 
> > > > Why are you not using the "generic" bind/unbind facility that userspace
> > > > already has for this with binding drivers to devices?  Why a special
> > > > ioctl?
> > > 
> > > Hi Greg,
> > > 
> > > Actually we think it should be safer that making the device invisble than
> > > just unbinding its driver. Looks like user can try to rebind it at any
> > > time and we don't have any method to stop them.
> > 
> > Why do you want to "stop" the user from doing something?  They asked to
> > do it, why prevent it?  If they ask to do something foolish, well, they
> > get to keep the pieces :)
> 
> Actually this is for SRIOV support, as we are moving FPGA accelerator from
> PF to VF, so we don't want users to see the FPGA accelerator from PF any
> more. We can't allow user to touch same FPGA accelerator from both PF and
> VF side (it leads to hardware erros).

Ick, ok, this needs to be documented really well then.

> > > > > --- a/include/uapi/linux/fpga-dfl.h
> > > > > +++ b/include/uapi/linux/fpga-dfl.h
> > > > > @@ -176,4 +176,36 @@ struct dfl_fpga_fme_port_pr {
> > > > >  
> > > > >  #define DFL_FPGA_FME_PORT_PR _IO(DFL_FPGA_MAGIC, DFL_FME_BASE + 0)
> > > > >  
> > > > > +/**
> > > > > + * DFL_FPGA_FME_PORT_RELEASE - _IOW(DFL_FPGA_MAGIC, DFL_FME_BASE + 1,
> > > > > + *   struct 
> > > > > dfl_fpga_fme_port_release)
> > > > > + *
> > > > > + * Driver releases the port per Port ID provided by caller.
> > > > > + * Return: 0 on success, -errno on failure.
> > > > > + */
> > > > > +struct dfl_fpga_fme_port_release {
> > > > > + /* Input */
> > > > > + __u32 argsz;/* Structure length */
> > > > > + __u32 flags;/* Zero for now */
> > > > > + __u32 port_id;
> > > > > +};
> > > > 
> > > > meta-comment, why do all of your structures for ioctls have argsz?  You
> > > > "know" the size of the structure already, it's part of the ioctl
> > > > definition.  You shouldn't need to also set it again, right?  Otherwise
> > > > ALL Linux ioctls would need something crazy like this.
> > > 
> > > Actually we followed the same method as vfio.
> > 
> > vfio is a protocol on "the wire", right?  Not an ioctl.
> > 
> > > The major purpose should be extendibility, as we really need this to
> > > be sth long term maintainable.
> > 
> > You can't change ioctl structure sizes at any time.
> > 
> > > It really helps, if we add some new members for extentions/enhancement
> > > under the same ioctl.
> > 
> > You don't do t

Re: [PATCH 05/15] Documentation: fpga: dfl: add descriptions for virtualization and new interfaces.

[Greg KH]

On Thu, Jul 04, 2019 at 02:42:08PM +0800, Wu Hao wrote:
> On Thu, Jul 04, 2019 at 07:37:19AM +0200, Greg KH wrote:
> > On Thu, Jul 04, 2019 at 07:38:22AM +0800, Wu Hao wrote:
> > > On Wed, Jul 03, 2019 at 07:59:26PM +0200, Greg KH wrote:
> > > > On Thu, Jun 27, 2019 at 05:49:41PM -0700, Moritz Fischer wrote:
> > > > > From: Wu Hao 
> > > > > 
> > > > > This patch adds virtualization support description for DFL based
> > > > > FPGA devices (based on PCIe SRIOV), and introductions to new
> > > > > interfaces added by new dfl private feature drivers.
> > > > > 
> > > > > [m...@kernel.org: Fixed up to make it work with new reStructuredText 
> > > > > docs]
> > > > > Signed-off-by: Xu Yilun 
> > > > > Signed-off-by: Wu Hao 
> > > > > Acked-by: Alan Tull 
> > > > > Signed-off-by: Moritz Fischer 
> > > > > ---
> > > > >  Documentation/fpga/dfl.rst | 100 
> > > > > +
> > > > >  1 file changed, 100 insertions(+)
> > > > 
> > > > This doesn't apply to my tree, where is this file created?
> > > 
> > > Hi Greg,
> > > 
> > > >From the cover-letter, Moritz mentioned, dfl.txt has been converted to 
> > > >.rst
> > > in linux-next. I think this patch is created on top of that by Moritz.
> > > 
> > > "Note: I've seen that Mauro touched Documentation/fpga/dfl.rst in 
> > > linux-next
> > > commit c220a1fae6c5d ("docs: fpga: convert docs to ReST and rename to 
> > > *.rst")"
> > 
> > Ok, but I can't take this patch just because the file is converted in
> > someone else's tree :(
> 
> Oh.. if that patch is merged from fpga tree, then we wont have this problem. 
> :(

You better not be basing your tree on linux-next :)

> So do you have any suggestions on what should i do now?
> wait that patch goes to offical release, and then resubmit this patch?

Wait until the change hits Linus's tree and then resend it to me.

thanks,

greg k-h

Re: [PATCH 06/15] fpga: dfl: fme: add DFL_FPGA_FME_PORT_RELEASE/ASSIGN ioctl support.

[Greg KH]

On Thu, Jul 04, 2019 at 07:30:58AM +0800, Wu Hao wrote:
> On Wed, Jul 03, 2019 at 08:07:53PM +0200, Greg KH wrote:
> > On Thu, Jun 27, 2019 at 05:49:42PM -0700, Moritz Fischer wrote:
> > > From: Wu Hao 
> > > 
> > > In order to support virtualization usage via PCIe SRIOV, this patch
> > > adds two ioctls under FPGA Management Engine (FME) to release and
> > > assign back the port device. In order to safely turn Port from PF
> > > into VF and enable PCIe SRIOV, it requires user to invoke this
> > > PORT_RELEASE ioctl to release port firstly to remove userspace
> > > interfaces, and then configure the PF/VF access register in FME.
> > > After disable SRIOV, it requires user to invoke this PORT_ASSIGN
> > > ioctl to attach the port back to PF.
> > > 
> > >  Ioctl interfaces:
> > >  * DFL_FPGA_FME_PORT_RELEASE
> > >Release platform device of given port, it deletes port platform
> > >device to remove related userspace interfaces on PF, then
> > >configures PF/VF access mode to VF.
> > > 
> > >  * DFL_FPGA_FME_PORT_ASSIGN
> > >Assign platform device of given port back to PF, it configures
> > >PF/VF access mode to PF, then adds port platform device back to
> > >re-enable related userspace interfaces on PF.
> > 
> > Why are you not using the "generic" bind/unbind facility that userspace
> > already has for this with binding drivers to devices?  Why a special
> > ioctl?
> 
> Hi Greg,
> 
> Actually we think it should be safer that making the device invisble than
> just unbinding its driver. Looks like user can try to rebind it at any
> time and we don't have any method to stop them.

Why do you want to "stop" the user from doing something?  They asked to
do it, why prevent it?  If they ask to do something foolish, well, they
get to keep the pieces :)

> > > --- a/include/uapi/linux/fpga-dfl.h
> > > +++ b/include/uapi/linux/fpga-dfl.h
> > > @@ -176,4 +176,36 @@ struct dfl_fpga_fme_port_pr {
> > >  
> > >  #define DFL_FPGA_FME_PORT_PR _IO(DFL_FPGA_MAGIC, DFL_FME_BASE + 0)
> > >  
> > > +/**
> > > + * DFL_FPGA_FME_PORT_RELEASE - _IOW(DFL_FPGA_MAGIC, DFL_FME_BASE + 1,
> > > + *   struct 
> > > dfl_fpga_fme_port_release)
> > > + *
> > > + * Driver releases the port per Port ID provided by caller.
> > > + * Return: 0 on success, -errno on failure.
> > > + */
> > > +struct dfl_fpga_fme_port_release {
> > > + /* Input */
> > > + __u32 argsz;/* Structure length */
> > > + __u32 flags;/* Zero for now */
> > > + __u32 port_id;
> > > +};
> > 
> > meta-comment, why do all of your structures for ioctls have argsz?  You
> > "know" the size of the structure already, it's part of the ioctl
> > definition.  You shouldn't need to also set it again, right?  Otherwise
> > ALL Linux ioctls would need something crazy like this.
> 
> Actually we followed the same method as vfio.

vfio is a protocol on "the wire", right?  Not an ioctl.

> The major purpose should be extendibility, as we really need this to
> be sth long term maintainable.

You can't change ioctl structure sizes at any time.

> It really helps, if we add some new members for extentions/enhancement
> under the same ioctl.

You don't do that.

> I don't think everybody needs this, but my consideration here is if
> newer generations of hardware/specs come with some extentions, I still
> hope we can resue these IOCTLs as much as we could, instead of
> creating more new ones.

You create new ones, like everyone else does, as you can not change old
code.  By trying to "version" structures like this, it's just going to
be a nightmare.

thanks,

greg k-h

Re: [PATCH 05/15] Documentation: fpga: dfl: add descriptions for virtualization and new interfaces.

[Greg KH]

On Thu, Jul 04, 2019 at 07:38:22AM +0800, Wu Hao wrote:
> On Wed, Jul 03, 2019 at 07:59:26PM +0200, Greg KH wrote:
> > On Thu, Jun 27, 2019 at 05:49:41PM -0700, Moritz Fischer wrote:
> > > From: Wu Hao 
> > > 
> > > This patch adds virtualization support description for DFL based
> > > FPGA devices (based on PCIe SRIOV), and introductions to new
> > > interfaces added by new dfl private feature drivers.
> > > 
> > > [m...@kernel.org: Fixed up to make it work with new reStructuredText docs]
> > > Signed-off-by: Xu Yilun 
> > > Signed-off-by: Wu Hao 
> > > Acked-by: Alan Tull 
> > > Signed-off-by: Moritz Fischer 
> > > ---
> > >  Documentation/fpga/dfl.rst | 100 +
> > >  1 file changed, 100 insertions(+)
> > 
> > This doesn't apply to my tree, where is this file created?
> 
> Hi Greg,
> 
> >From the cover-letter, Moritz mentioned, dfl.txt has been converted to .rst
> in linux-next. I think this patch is created on top of that by Moritz.
> 
> "Note: I've seen that Mauro touched Documentation/fpga/dfl.rst in linux-next
> commit c220a1fae6c5d ("docs: fpga: convert docs to ReST and rename to *.rst")"

Ok, but I can't take this patch just because the file is converted in
someone else's tree :(

Re: [PATCH 06/15] fpga: dfl: fme: add DFL_FPGA_FME_PORT_RELEASE/ASSIGN ioctl support.

[Greg KH]

On Thu, Jun 27, 2019 at 05:49:42PM -0700, Moritz Fischer wrote:
> From: Wu Hao 
> 
> In order to support virtualization usage via PCIe SRIOV, this patch
> adds two ioctls under FPGA Management Engine (FME) to release and
> assign back the port device. In order to safely turn Port from PF
> into VF and enable PCIe SRIOV, it requires user to invoke this
> PORT_RELEASE ioctl to release port firstly to remove userspace
> interfaces, and then configure the PF/VF access register in FME.
> After disable SRIOV, it requires user to invoke this PORT_ASSIGN
> ioctl to attach the port back to PF.
> 
>  Ioctl interfaces:
>  * DFL_FPGA_FME_PORT_RELEASE
>Release platform device of given port, it deletes port platform
>device to remove related userspace interfaces on PF, then
>configures PF/VF access mode to VF.
> 
>  * DFL_FPGA_FME_PORT_ASSIGN
>Assign platform device of given port back to PF, it configures
>PF/VF access mode to PF, then adds port platform device back to
>re-enable related userspace interfaces on PF.

Why are you not using the "generic" bind/unbind facility that userspace
already has for this with binding drivers to devices?  Why a special
ioctl?

> --- a/include/uapi/linux/fpga-dfl.h
> +++ b/include/uapi/linux/fpga-dfl.h
> @@ -176,4 +176,36 @@ struct dfl_fpga_fme_port_pr {
>  
>  #define DFL_FPGA_FME_PORT_PR _IO(DFL_FPGA_MAGIC, DFL_FME_BASE + 0)
>  
> +/**
> + * DFL_FPGA_FME_PORT_RELEASE - _IOW(DFL_FPGA_MAGIC, DFL_FME_BASE + 1,
> + *   struct dfl_fpga_fme_port_release)
> + *
> + * Driver releases the port per Port ID provided by caller.
> + * Return: 0 on success, -errno on failure.
> + */
> +struct dfl_fpga_fme_port_release {
> + /* Input */
> + __u32 argsz;/* Structure length */
> + __u32 flags;/* Zero for now */
> + __u32 port_id;
> +};

meta-comment, why do all of your structures for ioctls have argsz?  You
"know" the size of the structure already, it's part of the ioctl
definition.  You shouldn't need to also set it again, right?  Otherwise
ALL Linux ioctls would need something crazy like this.

thanks,

greg k-h

Re: [PATCH 05/15] Documentation: fpga: dfl: add descriptions for virtualization and new interfaces.

[Greg KH]

On Thu, Jun 27, 2019 at 05:49:41PM -0700, Moritz Fischer wrote:
> From: Wu Hao 
> 
> This patch adds virtualization support description for DFL based
> FPGA devices (based on PCIe SRIOV), and introductions to new
> interfaces added by new dfl private feature drivers.
> 
> [m...@kernel.org: Fixed up to make it work with new reStructuredText docs]
> Signed-off-by: Xu Yilun 
> Signed-off-by: Wu Hao 
> Acked-by: Alan Tull 
> Signed-off-by: Moritz Fischer 
> ---
>  Documentation/fpga/dfl.rst | 100 +
>  1 file changed, 100 insertions(+)

This doesn't apply to my tree, where is this file created?

thanks,

greg k-h

Re: [PATCH 04/15] fpga: dfl: fme: support 512bit data width PR

[Greg KH]

On Thu, Jun 27, 2019 at 05:49:40PM -0700, Moritz Fischer wrote:
> From: Wu Hao 
> 
> In early partial reconfiguration private feature, it only
> supports 32bit data width when writing data to hardware for
> PR. 512bit data width PR support is an important optimization
> for some specific solutions (e.g. XEON with FPGA integrated),
> it allows driver to use AVX512 instruction to improve the
> performance of partial reconfiguration. e.g. programming one
> 100MB bitstream image via this 512bit data width PR hardware
> only takes ~300ms, but 32bit revision requires ~3s per test
> result.
> 
> Please note now this optimization is only done on revision 2
> of this PR private feature which is only used in integrated
> solution that AVX512 is always supported. This revision 2
> hardware doesn't support 32bit PR.
> 
> Signed-off-by: Ananda Ravuri 
> Signed-off-by: Xu Yilun 
> Signed-off-by: Wu Hao 
> Acked-by: Alan Tull 
> Signed-off-by: Moritz Fischer 
> ---
>  drivers/fpga/dfl-fme-main.c |   3 +
>  drivers/fpga/dfl-fme-mgr.c  | 113 +++-
>  drivers/fpga/dfl-fme-pr.c   |  43 +-
>  drivers/fpga/dfl-fme.h  |   2 +
>  drivers/fpga/dfl.h  |   5 ++
>  5 files changed, 135 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c
> index 086ad2420ade..076d74f6416d 100644
> --- a/drivers/fpga/dfl-fme-main.c
> +++ b/drivers/fpga/dfl-fme-main.c
> @@ -21,6 +21,8 @@
>  #include "dfl.h"
>  #include "dfl-fme.h"
>  
> +#define DRV_VERSION  "0.8"
> +
>  static ssize_t ports_num_show(struct device *dev,
> struct device_attribute *attr, char *buf)
>  {
> @@ -277,3 +279,4 @@ MODULE_DESCRIPTION("FPGA Management Engine driver");
>  MODULE_AUTHOR("Intel Corporation");
>  MODULE_LICENSE("GPL v2");
>  MODULE_ALIAS("platform:dfl-fme");
> +MODULE_VERSION(DRV_VERSION);

No, we ripped out these useless "driver version" things all over the
place, please do not add them back in again.  They mean nothing and
confuse people to no end.

I'll not take this patch, sorry.

greg k-h

Re: [PATCH v2 00/22] Add ABI and features docs to the Kernel documentation

[Greg KH]

On Thu, Jun 20, 2019 at 02:22:52PM -0300, Mauro Carvalho Chehab wrote:
> This is a rebased version of the scripts with parse
> Documentation/ABI and Documentation/feature files
> and produce a ReST output. Those scripts are added to the
> Kernel building system, in order to output their contents
> inside the Kernel documentation.
> 
> Please notice that, as discussed, I added support at get_abi.pl
> to handle ABI files as if they're compatible with ReST. Right
> now, this feature can't be enabled for normal builds, as it will
> cause Sphinx crashes. After getting the offending ABI files fixed,
> a single line change will be enough to make it default.
> 
> a version "0" was sent back on 2017.

Ok, I added the first chunk of these patches to my tree, that add the
get_abi.pl file to the tree.  I didn't touch the others that touched the
documentation build or other scripts just yet, as I wanted to get others
to agree with them before accepting them.

Or we can just wait until after 5.3-rc1 is out and then the rest can go
through the documentation tree.

thanks,

greg k-h

Re: [Linux-kernel-mentees] [PATCH] Documentation: platform: convert x86-laptop-drivers.txt to reST

[Greg KH]

On Tue, Jun 18, 2019 at 07:17:17AM -0600, Jonathan Corbet wrote:
> On Tue, 18 Jun 2019 07:41:58 +0200
> Greg KH  wrote:
> 
> > On Tue, Jun 18, 2019 at 11:02:27AM +0530, Puranjay Mohan wrote:
> > > This converts the plain text documentation to reStructuredText format.
> > > No essential content change.
> > > 
> > > Signed-off-by: Puranjay Mohan 
> > > ---
> > >  Documentation/platform/x86-laptop-drivers.rst | 23 +++
> > >  Documentation/platform/x86-laptop-drivers.txt | 18 ---
> > >  2 files changed, 23 insertions(+), 18 deletions(-)
> > >  create mode 100644 Documentation/platform/x86-laptop-drivers.rst
> > >  delete mode 100644 Documentation/platform/x86-laptop-drivers.txt  
> > 
> > Don't you also need to hook it up to the documentation build process
> > when doing this?
> 
> Hooking it into the TOC tree is a good thing, but I think it's also good
> to think about the exercise in general.  This is a document dropped into
> place five years ago and never touched again.  It's a short list of
> seemingly ancient laptops with no explanation of what it means.  So the
> real question, IMO, is whether this document is useful to anybody and, if
> not, whether it should just be deleted instead.

I bet it should be deleted, but we should ask the platform driver
maintainers first before we do that :)

thanks,

greg k-h

Re: [Linux-kernel-mentees] [PATCH] Documentation: platform: convert x86-laptop-drivers.txt to reST

[Greg KH]

On Tue, Jun 18, 2019 at 11:02:27AM +0530, Puranjay Mohan wrote:
> This converts the plain text documentation to reStructuredText format.
> No essential content change.
> 
> Signed-off-by: Puranjay Mohan 
> ---
>  Documentation/platform/x86-laptop-drivers.rst | 23 +++
>  Documentation/platform/x86-laptop-drivers.txt | 18 ---
>  2 files changed, 23 insertions(+), 18 deletions(-)
>  create mode 100644 Documentation/platform/x86-laptop-drivers.rst
>  delete mode 100644 Documentation/platform/x86-laptop-drivers.txt

Don't you also need to hook it up to the documentation build process
when doing this?

thanks,

greg k-h

Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

[Greg KH]

On Sat, Mar 09, 2019 at 12:40:01PM +0100, Geert Uytterhoeven wrote:
> > Signing keys should be kept secure, or better yet, just deleted entirely
> > after creating and signing with them.  That's what I do for my kernels
> > and I'm pretty sure that some distros also do this.  That way there's no
> > chance that someone else can sign a module and have it loaded without
> > detection, which is what signing is supposed to prevent from happening.
> 
> If you want that kind of security, there's no point in allowing to extend the
> kernel by building more kernel modules after deployment.

That's not what these files are for (in the original user's case).  They
want these for doing tracing/ebpf stuff, which require kernel headers to
build against.

> "Raw kernel headers also cannot be copied into the filesystem like they
>  can be on other distros, due to licensing and other issues. There's no
>  linux-headers package on Android."
> 
> What's the licensing issue? What's the (legal) difference between having
> the headers on the file system, and having a kernel module including the
> headers on the file system?

There is no licensing issue, see my follow-up comment about that.

It's all in ease-of-use here.  You want to build a trace function
against a running kernel, and now you have the header files for that
specific kernel right there in the kernel itself to build against.  It
doesn't get easier than that.

thanks,

greg k-h

Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

[Greg KH]

On Fri, Mar 08, 2019 at 06:59:23PM +0100, Geert Uytterhoeven wrote:
> Hi Greg,
> 
> On Fri, Mar 8, 2019 at 6:05 PM Greg KH  wrote:
> > On Fri, Mar 08, 2019 at 05:42:32AM -0800, Joel Fernandes wrote:
> > > On Fri, Mar 8, 2019, 3:53 AM Geert Uytterhoeven  
> > > wrote:
> > > > > It is just so much easier to use tar + xz at build time, and leave the
> > > > > decompression task to the user. After decompression, the files will 
> > > > > live on
> > > > > the disk and the page-cache mechanism will free memory when/if the 
> > > > > files fall
> > > > > off the LRUs.
> > > >
> > > > I'm also considering how generic and extensible the solution is.
> > > > What if people need other build artifacts in the future (e.g. signing 
> > > > key to
> > > > load signed modules)?
> > >
> > > That sounds like it could be useful. I don't see any reason off the
> > > top why that would not be possible to add to the list of archived
> > > files in the future. The patch allows populating the list of files
> > > from Kbuild using ikh_file_list variable.
> >
> > Um, no, you don't want the signing key in the kernel itself, as that
> > totally defeats the purpose of the signing key :)
> 
> In a loadable module?
> He who has the module, can build and sign more modules.

Again, that's pretty foolish.

Signing keys should be kept secure, or better yet, just deleted entirely
after creating and signing with them.  That's what I do for my kernels
and I'm pretty sure that some distros also do this.  That way there's no
chance that someone else can sign a module and have it loaded without
detection, which is what signing is supposed to prevent from happening.

thanks,

greg k-h

Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

[Greg KH]

On Fri, Mar 08, 2019 at 02:57:24PM +0100, Enrico Weigelt, metux IT consult 
wrote:
> On 08.03.19 14:42, Joel Fernandes wrote:
> 
> Hi folks,
> 
> > That sounds like it could be useful. I don't see any reason off the
> > top why that would not be possible to add to the list of archived
> > files in the future. The patch allows populating the list of files
> > from Kbuild using ikh_file_list variable.
> 
> It seems the whole thing's going into a direction where a whole own
> subtopic is coming up: compressed built-in filesystem.
> 
> Haven't had a deeper thought about it yet, whether or not existing
> filesystems like squashfs, initramfs, etc are the right thing for that,
> or something new should be invented, but a generic mechanism for
> compiled-in compressed (ro) filesystems could also be interesting
> for very small devices, that perhaps even dont need any persistence.
> 
> Some requirements coming up in mind:
> 
> 1. it shall be possible to have any number of instances - possibly by
>separate modules.
> 2. it shall be possible to use an bootloader/firmware provided image
>(so it can serve as initrd)
> 2. data should stay compressed as long as possible, but uncompressed
>data might be cached - do decompression on-demand
> 3. only needs to be ro (write access could be done via unionfs+friends)
> 4. it shall be swappable (if swap is enabled)
> 
> In that scenario, these in-kernel headers would just one consumer,
> I can imagine lots of others.

I too want a pony :)

Until then, I think this feature should not be bikeshedded to death.  It
fits a real need, is simple (modulo the Kbuild interactions), and is
optional for those that do not wish to waste the memory.

thanks,

greg k-h

Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

[Greg KH]

On Fri, Mar 08, 2019 at 05:42:32AM -0800, Joel Fernandes wrote:
> HI Geert,
> 
> On Fri, Mar 8, 2019, 3:53 AM Geert Uytterhoeven  wrote:
> > > It is just so much easier to use tar + xz at build time, and leave the
> > > decompression task to the user. After decompression, the files will live 
> > > on
> > > the disk and the page-cache mechanism will free memory when/if the files 
> > > fall
> > > off the LRUs.
> >
> > I'm also considering how generic and extensible the solution is.
> > What if people need other build artifacts in the future (e.g. signing key to
> > load signed modules)?
> 
> That sounds like it could be useful. I don't see any reason off the
> top why that would not be possible to add to the list of archived
> files in the future. The patch allows populating the list of files
> from Kbuild using ikh_file_list variable.

Um, no, you don't want the signing key in the kernel itself, as that
totally defeats the purpose of the signing key :)

greg k-h

Re: [RFC] Provide in-kernel headers for making it easy to extend the kernel

[Greg KH]

On Thu, Mar 07, 2019 at 09:41:24PM +0100, Enrico Weigelt, metux IT consult 
wrote:
> On 07.03.19 02:49, Daniel Colascione wrote:
> 
> > Entirely FS-less operation is uncommon, granted. :-) I guess I've just
> > spent too much time debugging emulators that refuse to mount their> root 
> > filesystems. :-)
> 
> Fix these emulators ?
> 
> > There are legitimate reasons why a device's> filesystems might not be 
> > rw-mountable though, and I can imagine a
> > world where I want to attach tracing tools *very* early.
> 
> Ok, but the filesystem where the modules live is mountable, right ?
> 
> > Sure. There's some support for a ramdisk already in fastboot. But just
> > including a blob in initrd doesn't *automatically* make it available
> > to userspace in any uniform way. With Joel's approach --- which> defines 
> > both a propagation mechanism and an access interface --- we
> 
> I can define such an interface with a few words:
> 
> * the kernel headers lives in a (compressed) squashfs image in the
>   module directory for the corresponding kernel version:
>   /lib/modules/-/headers.img"
> * this image shall be mounted at a canonical mountpoint, eg:
>   /usr/src/linux-headers--
> * the kernel needs to support squashfs (may be module or built-in)
> 
> That's it. I don't need to touch a single line of kernel code for that,
> just a very simple and small convention.

Ick, no, no more squashfs please, let's just kill that mess once and for
all :)

Again, putting this in a simple compressed tar image allows anyone to do
whatever they need to with this.  If they want a full filesystem,
uncompress it and use it there.  If they just want it in-memory where
they can uncompress it and then discard it, that works too.

And if no one wants this, just don't select the config option and do not
load the module with this data in it.  Just like /proc/config.gz is
today.

thanks,

greg k-h

Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

[Greg KH]

On Thu, Mar 07, 2019 at 10:03:43AM -0500, Joel Fernandes wrote:
> On Thu, Mar 07, 2019 at 09:58:24AM +0100, Geert Uytterhoeven wrote:
> > Hi Joel,
> > 
> > On Fri, Mar 1, 2019 at 5:10 PM Joel Fernandes (Google)
> >  wrote:
> > > Introduce in-kernel headers and other artifacts which are made available
> > > as an archive through proc (/proc/kheaders.tar.xz file). This archive 
> > > makes
> > > it possible to build kernel modules, run eBPF programs, and other
> > > tracing programs that need to extend the kernel for tracing purposes
> > > without any dependency on the file system having headers and build
> > > artifacts.
> > >
> > > On Android and embedded systems, it is common to switch kernels but not
> > > have kernel headers available on the file system. Raw kernel headers
> > > also cannot be copied into the filesystem like they can be on other
> > > distros, due to licensing and other issues. There's no linux-headers
> > > package on Android. Further once a different kernel is booted, any
> > > headers stored on the file system will no longer be useful. By storing
> > > the headers as a compressed archive within the kernel, we can avoid these
> > > issues that have been a hindrance for a long time.
> > >
> > > The feature is also buildable as a module just in case the user desires
> > > it not being part of the kernel image. This makes it possible to load
> > > and unload the headers on demand. A tracing program, or a kernel module
> > > builder can load the module, do its operations, and then unload the
> > > module to save kernel memory. The total memory needed is 3.8MB.
> > >
> > > The code to read the headers is based on /proc/config.gz code and uses
> > > the same technique to embed the headers.
> > >
> > > To build a module, the below steps have been tested on an x86 machine:
> > > modprobe kheaders
> > > rm -rf $HOME/headers
> > > mkdir -p $HOME/headers
> > > tar -xvf /proc/kheaders.tar.xz -C $HOME/headers >/dev/null
> > > cd my-kernel-module
> > > make -C $HOME/headers M=$(pwd) modules
> > > rmmod kheaders
> > 
> > As the usage pattern will be accessing the individual files, what about
> > implementing a file system that provides read-only access to the internal
> > kheaders archive?
> > 
> > mount kheaders $HOME/headers -t kheaders
> 
> I thought about it already. This is easier said than done though. The archive
> is compressed from 40MB to 3.6MB. If we leave it uncompressed in RAM, then it
> will take up the entire 40MB of RAM and in Android we don't even use
> disk-based swap.
> 
> So we will need some kind of intra file compressed memory representation that
> a filesystem can use for the backing store. I thought of RAM-backed squashfs
> but it requires squashfs-tools to be installed at build time (which my host
> distro itself didn't have).
> 
> It is just so much easier to use tar + xz at build time, and leave the
> decompression task to the user. After decompression, the files will live on
> the disk and the page-cache mechanism will free memory when/if the files fall
> off the LRUs.
> 
> WDYT?

I think the compressed tarball is much simpler/easier overall.  If
someone really wants the filesystem, they just uncompress it into a
tmpfs mount.  It's much less moving kernel code to worry about.

thanks,

greg k-h

Re: [PATCH v3 1/2] Provide in-kernel headers for making it easy to extend the kernel

[Greg KH]

On Thu, Feb 28, 2019 at 10:37:41AM -0500, Joel Fernandes wrote:
> In any case, it is not practical to provide headers for every kernel version 
> on
> the system image and maintain them, it will take up too much space and has to
> be periodically packaged. Not to mention that there will be kernel versions
> that are booting Android that simply don't have anyone maintaining headers
> for. So the proposed solution is an easier path.

I totally agree with this, and with your proposed patches, just wanting
to clear up some potential anti-GPL-FUD before it got spread any further :)

thanks,

greg k-h

Re: [PATCH v3 1/2] Provide in-kernel headers for making it easy to extend the kernel

[Greg KH]

On Thu, Feb 28, 2019 at 10:00:54AM -0500, Joel Fernandes wrote:
> > Hmm, isn't it easier to add kernel-headers package on Android?
> 
> I have already been down that road. In the Android ecosystem, the Android
> teams only provide a "userspace system image" which goes on the system
> partition of the flash (and a couple other images are also provided but
> system is the main one). The system image cannot contain GPL source code.

Note, that last sentance is not true, it can contain whatever license
code it wants to, as long as the group doing the distribution abides by
the license of the code contained in it.

The split of system and other images has nothing to do with license
issues, but rather functional requirements.  There _might_ be GPL
licensed code in the image if it meets a requirement for the proper
functionality of the system, you never know :)

thanks,

greg "not a lawyer but I work with a lot of them all the time" k-h

Re: [PATCH] Documentation/process/howto: Update for 4.x -> 5.x versioning

[Greg KH]

On Sun, Feb 24, 2019 at 04:43:20PM +0800, Zenghui Yu wrote:
> As linux-5.0 is coming up soon, the howto.rst document can be
> updated for the new kernel version. Change all 4.x references
> to 5.x now.
> 
> Signed-off-by: Zenghui Yu 
> ---
>  Documentation/process/howto.rst | 24 
>  1 file changed, 12 insertions(+), 12 deletions(-)

I think Jon already sent a patch for this last week, you might want to
look in the archives.

thanks,

greg k-h

Re: [PATCH] Documentation/process/howto: Update for 4.x -> 5.x versioning

[Greg KH]

On Sun, Feb 24, 2019 at 11:16:56AM +0100, Federico Vaga wrote:
> hello,
> 
> I have just a general observation for the community, not related to the 
> content of this patch, but related with the idea behind.
> 
> Is it really important to specify the major release number in the documents? 
> . 
> Can't we just use a generic x.y.z, or a more generic statement?
> 
> When you open a documentation page like
> 
> https://www.kernel.org/doc/html/latest/
> 
> you will see the release number in the top left corner, which implies that 
> what you read is (should be) valid for that version. And if you read from the 
> sources you should know which version you checked out, and if you don't you
> can always verify.
> 
> I do not see the added value of having those numbers in the documents, unless 
> the purpose is to highlight some specific exceptions.
> 
> Am I missing some important reasons that justify these numbers?

Nothing really, it's just "history".  Given that the "major" number only
changes every 3-4 years, it's not all that big of a deal.

If you can think of a way to write these documents such that they do not
depend on a version number at all, I'm sure no one would object to those
patches.

thanks,

greg k-h

Re: [PATCH] doc: process: GPL -> GPL-compatible

[Greg KH]

On Tue, Jan 22, 2019 at 10:34:08AM +0100, Adam Borowski wrote:
> Drivers under MIT, BSD-17-clause, or uncle-Bob's-newest-take-on-PD are
> all fine, not just GPL.
> 
> Signed-off-by: Adam Borowski 
> ---
> Not reformatting to fill lines, it'll semi-conflict with another patch
> that's been acked but not yet pushed.
> 
>  Documentation/process/stable-api-nonsense.rst | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/process/stable-api-nonsense.rst 
> b/Documentation/process/stable-api-nonsense.rst
> index 24f5aeecee91..9b69fb08b65e 100644
> --- a/Documentation/process/stable-api-nonsense.rst
> +++ b/Documentation/process/stable-api-nonsense.rst
> @@ -170,7 +170,8 @@ nightmare, and trying to keep up with an ever changing 
> kernel interface
>  is also a rough job.
>  
>  Simple, get your kernel driver into the main kernel tree (remember we
> -are talking about GPL released drivers here, if your code doesn't fall
> +are talking about drivers released under a GPL-compatible license here,
> +if your code doesn't fall
>  under this category, good luck, you are on your own here, you leech
>  .)  If your
>  driver is in the tree, and a kernel interface changes, it will be fixed

Nice fix:

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH] doc:process: remove note from 'stable api nonsense'

[Greg KH]

On Mon, Jan 21, 2019 at 09:14:00AM +0100, Federico Vaga wrote:
> On Monday, January 21, 2019 2:43:38 AM CET Jonathan Corbet wrote:
> > On Fri, 18 Jan 2019 22:58:04 +0100
> > 
> > Federico Vaga  wrote:
> > > The link referred by the note can't be retrieved: this patch just
> > > remove that old note.
> > > 
> > > Signed-off-by: Federico Vaga 
> > > ---
> > > 
> > >  Documentation/process/stable-api-nonsense.rst | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > 
> > > diff --git a/Documentation/process/stable-api-nonsense.rst
> > > b/Documentation/process/stable-api-nonsense.rst index
> > > 24f5aeecee91..57d95a49c096 100644
> > > --- a/Documentation/process/stable-api-nonsense.rst
> > > +++ b/Documentation/process/stable-api-nonsense.rst
> > > @@ -171,8 +171,7 @@ is also a rough job.
> > > 
> > >  Simple, get your kernel driver into the main kernel tree (remember we
> > >  are talking about GPL released drivers here, if your code doesn't fall
> > > 
> > > -under this category, good luck, you are on your own here, you leech
> > > -.)  If your
> > > +under this category, good luck, you are on your own here, you leech).  If
> > > your> 
> > >  driver is in the tree, and a kernel interface changes, it will be fixed
> > >  up by the person who did the kernel change in the first place.  This
> > >  ensures that your driver is always buildable, and works over time, with
> > 
> > I've applied this.  I do wonder if the "you leech" should maybe come out
> > too, though.  I don't think that parasitic worms are a protected class
> > under the CoC, but they might still suffer emotionally from being
> > compared to the purveyors of proprietary modules...
> 
> I agree, do you want me to change the patch?

I would leave it as-is for now please.  When this was written, there was
a lot of discussion about closed source modules, and how the companies
that created them were leeches on our development community.  No one
disagreed with that statement, and a number of companies privately
agreed with us.

That still has not changed.

So I would like to see this remain.

thanks,

greg k-h

Re: [RFC] Provide in-kernel headers for making it easy to extend the kernel

[Greg KH]

On Sat, Jan 19, 2019 at 02:28:00AM -0800, Christoph Hellwig wrote:
> This seems like a pretty horrible idea and waste of kernel memory.

It's only a waste if you want it to be a waste, i.e. if you load the
kernel module.

This really isn't any different from how /proc/config.gz works.

> Just add support to kbuild to store a compressed archive in initramfs
> and unpack it in the right place.

I think the issue is that some devices do not use initramfs, or switch
away from it after init happens or something like that.  Joel has all of
the looney details that he can provide.

thanks,

greg k-h

Re: [PATCH] doc:process: remove note from 'stable api nonsense'

[Greg KH]

On Fri, Jan 18, 2019 at 10:58:04PM +0100, Federico Vaga wrote:
> The link referred by the note can't be retrieved: this patch just
> remove that old note.
> 
> Signed-off-by: Federico Vaga 
> ---
>  Documentation/process/stable-api-nonsense.rst | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Acked-by: Greg Kroah-Hartman 

Re: [RFC] Provide in-kernel headers for making it easy to extend the kernel

[Greg KH]

On Fri, Jan 18, 2019 at 05:55:43PM -0500, Joel Fernandes wrote:
> --- /dev/null
> +++ b/kernel/kheaders.c
> @@ -0,0 +1,74 @@
> +// SPDX-License-Identifier: GPL-2.0

Nice, but:



> +MODULE_LICENSE("GPL");

That means "GPL2+" (yeah, horrible, I know, we all get it wrong, look at
include/linux/module.h for details)

So you should change that line to:
MODULE_LICENSE("GPLv2");

Other than that, nice work.

greg k-h

Re: [RFC] Provide in-kernel headers for making it easy to extend the kernel

[Greg KH]

On Fri, Jan 18, 2019 at 05:55:43PM -0500, Joel Fernandes wrote:
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -549,6 +549,16 @@ config IKCONFIG_PROC
> This option enables access to the kernel configuration file
> through /proc/config.gz.
>  
> +config IKHEADERS_PROC
> + tristate "Enable kernel header artifacts through /proc/kheaders.tgz"
> + select BUILD_BIN2C
> + depends on PROC_FS
> + help
> +   This option enables access to the kernel header and other artifacts 
> that
> +  are generated during the build process. These can be used to build 
> kernel
> +  modules, and other in-kernel programs such as those generated by 
> eBPF
> +  and systemtap tools.
> +

Minor nit, say what the module name here is if you pick the option as a
module?

thanks,

greg k-h

Re: doc: stable-api-nonsense missing link

[Greg KH]

On Fri, Jan 18, 2019 at 08:20:31AM +0100, Greg KH wrote:
> On Thu, Jan 17, 2019 at 03:19:53PM -0700, Jonathan Corbet wrote:
> > On Thu, 17 Jan 2019 23:14:08 +0100
> > Federico Vaga  wrote:
> > 
> > > in this document
> > > 
> > > Documentation/process/stable-api-nonsense.rst
> > > 
> > > there is this note:
> > > 
> > > 
> > > 
> > > Is it possible to get it fixed with the proper link? I searched for it 
> > > without 
> > > success. If that comment is impossible to find I will send a patch to 
> > > remove 
> > > that note, but first I wanted to ask you if you know where to get it.
> > 
> > That sounds like a question for Greg (copied).  Given that this document
> > predates the Git era, though, I suspect that whatever reference that was
> > meant to be is long since lost.
> 
> It was referring to an email thread about this topic that happened at
> the time I wrote the document.  Let me try to dig through my archives,
> but given that it was written well over a decade ago, I don't know if I
> will be able to find it easily...

I can't find it at the moment, so taking out the text in <...> is fine
with me.

thanks,

greg k-h

Re: doc: stable-api-nonsense missing link

[Greg KH]

On Thu, Jan 17, 2019 at 03:19:53PM -0700, Jonathan Corbet wrote:
> On Thu, 17 Jan 2019 23:14:08 +0100
> Federico Vaga  wrote:
> 
> > in this document
> > 
> > Documentation/process/stable-api-nonsense.rst
> > 
> > there is this note:
> > 
> > 
> > 
> > Is it possible to get it fixed with the proper link? I searched for it 
> > without 
> > success. If that comment is impossible to find I will send a patch to 
> > remove 
> > that note, but first I wanted to ask you if you know where to get it.
> 
> That sounds like a question for Greg (copied).  Given that this document
> predates the Git era, though, I suspect that whatever reference that was
> meant to be is long since lost.

It was referring to an email thread about this topic that happened at
the time I wrote the document.  Let me try to dig through my archives,
but given that it was written well over a decade ago, I don't know if I
will be able to find it easily...

thanks,

greg k-h

Re: [PATCH v2 1/5] fs: kernfs: add poll file operation

[Greg KH]

On Thu, Jan 10, 2019 at 02:07:14PM -0800, Suren Baghdasaryan wrote:
> From: Johannes Weiner 
> 
> Kernfs has a standardized poll/notification mechanism for waking all
> pollers on all fds when a filesystem node changes. To allow polling
> for custom events, add a .poll callback that can override the default.
> 
> This is in preparation for pollable cgroup pressure files which have
> per-fd trigger configurations.
> 
> Signed-off-by: Johannes Weiner 
> Signed-off-by: Suren Baghdasaryan 

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH v3] code-of-conduct: Remove explicit list of discrimination factors

[Greg KH]

On Mon, Dec 03, 2018 at 12:53:00PM +0100, Geert Uytterhoeven wrote:
> Hi Greg,
> 
> On Mon, Dec 3, 2018 at 12:05 PM Greg Kroah-Hartman
>  wrote:
> > On Sun, Dec 02, 2018 at 10:32:57AM +0100, Geert Uytterhoeven wrote:
> > > Providing an explicit list of discrimination factors may give the false
> > > impression that discrimination based on other unlisted factors would be
> > > allowed.
> > >
> > > Furthermore, this list is already overly long, polarizing,
> > > politically-laden, and reinstating the concept of human races.
> > > None of these is related to the goals of the Linux kernel project.
> > >
> > > Avoid any ambiguity or political undertone by removing the list, to
> > > ensure "a harassment-free experience for everyone", period.
> >
> > I understand the reason you and others are proposing this change,
> > however for now, let us stick with the text that we have.  As Linus and
> > I said just over a month ago, let's sit with the text we have until
> > something comes up that requires a change to happen.
> >
> > Also, I recommend you work with the upstream developers of this text to
> > see if they agree with your changes here.  If they do, and update their
> > version, I will be glad to revisit this text at that time.
> 
> I did, cfr. 
> https://github.com/ContributorCovenant/contributor_covenant/issues/610
> 
> The official response was:
> 
>"I'm not going to make this change to the Contributor Covenant itself,
> since I believe that explicitly listing examples of protected classes is
> important. However, any adopting project is free to modify the document
> according to the license."

I figured.  As I said, some people feel that the list is good to have,
if not essential.  So let's stick with it for now.

thanks,

greg k-h

Re: [PATCH v1 1/2]: Documentation/admin-guide: update admin-guide index.rst

[Greg KH]

On Mon, Nov 19, 2018 at 08:41:31AM +0300, Alexey Budankov wrote:
> 
> Extend index.rst index file at admin-guide root directory with 
> the reference to perf-security.rst file being introduced.
> 
> Signed-off-by: Alexey Budankov 
> ---
>  Documentation/admin-guide/index.rst | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/admin-guide/index.rst 
> b/Documentation/admin-guide/index.rst
> index 0873685bab0f..885cc0de9114 100644
> --- a/Documentation/admin-guide/index.rst
> +++ b/Documentation/admin-guide/index.rst
> @@ -75,6 +75,7 @@ configure specific aspects of kernel behavior to your 
> liking.
> thunderbolt
> LSM/index
> mm/index
> +   perf-security

You just broke the build with this patch.  They need to be ordered the
other way around :(

Re: [PATCH v3 0/3] Add driver for Synopsys DesignWare I3C master IP

[Greg KH]

On Thu, Nov 08, 2018 at 05:14:08PM +, Vitor soares wrote:
> This patch series is a proposal for the I3C master driver for Synopsys IP.
> This patch is to be applied on top of I3C subsystem RFC V10 submitted by
> Boris Brezillon.

I'd like to get Boris's reviewed/signed-off on these before I take them.

thanks,

greg k-h

Re: [PATCH] docs/uio: fix a grammar nitpick

[Greg KH]

On Sat, Oct 13, 2018 at 09:47:46PM +, Will Korteland wrote:
> This patch fixes a minor, incorrect piece of grammar in the UIO howto.
> 
> Signed-off-by: Will Korteland 
> Acked-by: Randy Dunlap 
> ---
> This is my first attempt at a kernel patch, so please let me know if
> I've done something silly.

You sent this in html format :(

Also, the non-html format did not apply to the tree, can you redo this
against linux-next and resend it?

thanks,

greg k-h

Re: [PATCH v2] Documentation: typec.rst: Use literal-block element with ascii art

[Greg KH]

On Wed, Apr 25, 2018 at 04:47:01PM +0300, Jani Nikula wrote:
> On Fri, 06 Apr 2018, Heikki Krogerus  wrote:
> > Using reStructuredText literal-block element with ascii-art.
> > That prevents the ascii art from being processed as
> > reStructuredText.
> >
> > Reported-by: Masanari Iida 
> > Fixes: bdecb33af34f ("usb: typec: API for controlling USB Type-C 
> > Multiplexers")
> > Signed-off-by: Heikki Krogerus 
> 
> Jon, this fixes a documentation build failure in v4.17-rc1. It's not
> just a warning, it's a complete fail. Our docs builder at 01.org is
> failing, apparently the same at kernel.org. Please pick it up soon.

This is in my tree, will go to Linus in a day or so.

thanks,

greg k-h
--
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 v1 1/1] misc: IBM Virtual Management Channel Driver

[Greg KH]

On Mon, Apr 23, 2018 at 02:17:28PM -0700, Randy Dunlap wrote:
> On 04/23/18 12:53, Greg KH wrote:
> > On Mon, Apr 23, 2018 at 11:38:18AM -0700, Randy Dunlap wrote:
> >> On 04/23/18 07:46, Bryant G. Ly wrote:
> >>> This driver is a logical device which provides an
> >>> interface between the hypervisor and a management
> >>> partition.
> >>>
> >>> This driver is to be used for the POWER Virtual
> >>> Management Channel Virtual Adapter on the PowerVM
> >>> platform. It provides both request/response and
> >>> async message support through the /dev/ibmvmc node.
> >>>
> >>> Signed-off-by: Bryant G. Ly 
> >>> Reviewed-by: Steven Royer 
> >>> Reviewed-by: Adam Reznechek 
> >>> Tested-by: Taylor Jakobson 
> >>> Tested-by: Brad Warrum 
> >>> Cc: Greg Kroah-Hartman 
> >>> Cc: Arnd Bergmann 
> >>> Cc: Benjamin Herrenschmidt 
> >>> Cc: Michael Ellerman 
> >>> ---
> >>>  Documentation/ioctl/ioctl-number.txt  |1 +
> >>>  Documentation/misc-devices/ibmvmc.txt |  161 +++
> >>>  MAINTAINERS   |6 +
> >>>  arch/powerpc/include/asm/hvcall.h |1 +
> >>>  drivers/misc/Kconfig  |   14 +
> >>>  drivers/misc/Makefile |1 +
> >>>  drivers/misc/ibmvmc.c | 2415 
> >>> +
> >>>  drivers/misc/ibmvmc.h |  209 +++
> >>>  8 files changed, 2808 insertions(+)
> >>>  create mode 100644 Documentation/misc-devices/ibmvmc.txt
> >>>  create mode 100644 drivers/misc/ibmvmc.c
> >>>  create mode 100644 drivers/misc/ibmvmc.h
> >>
> >>> diff --git a/Documentation/misc-devices/ibmvmc.txt 
> >>> b/Documentation/misc-devices/ibmvmc.txt
> >>> new file mode 100644
> >>> index 000..bae1064
> >>> --- /dev/null
> >>> +++ b/Documentation/misc-devices/ibmvmc.txt
> > 
> > Aren't we doing new documentation in .rst format instead of .txt?
> 
> I am not aware that .rst format is a *requirement* for new documentation.
> 
> ??

Why wouldn't you create new documentation in that format?  Just saves
the step of having to change it again in the future.

thanks,

greg k-h
--
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 v1 1/1] misc: IBM Virtual Management Channel Driver

[Greg KH]

On Mon, Apr 23, 2018 at 11:38:18AM -0700, Randy Dunlap wrote:
> On 04/23/18 07:46, Bryant G. Ly wrote:
> > This driver is a logical device which provides an
> > interface between the hypervisor and a management
> > partition.
> > 
> > This driver is to be used for the POWER Virtual
> > Management Channel Virtual Adapter on the PowerVM
> > platform. It provides both request/response and
> > async message support through the /dev/ibmvmc node.
> > 
> > Signed-off-by: Bryant G. Ly 
> > Reviewed-by: Steven Royer 
> > Reviewed-by: Adam Reznechek 
> > Tested-by: Taylor Jakobson 
> > Tested-by: Brad Warrum 
> > Cc: Greg Kroah-Hartman 
> > Cc: Arnd Bergmann 
> > Cc: Benjamin Herrenschmidt 
> > Cc: Michael Ellerman 
> > ---
> >  Documentation/ioctl/ioctl-number.txt  |1 +
> >  Documentation/misc-devices/ibmvmc.txt |  161 +++
> >  MAINTAINERS   |6 +
> >  arch/powerpc/include/asm/hvcall.h |1 +
> >  drivers/misc/Kconfig  |   14 +
> >  drivers/misc/Makefile |1 +
> >  drivers/misc/ibmvmc.c | 2415 
> > +
> >  drivers/misc/ibmvmc.h |  209 +++
> >  8 files changed, 2808 insertions(+)
> >  create mode 100644 Documentation/misc-devices/ibmvmc.txt
> >  create mode 100644 drivers/misc/ibmvmc.c
> >  create mode 100644 drivers/misc/ibmvmc.h
> 
> > diff --git a/Documentation/misc-devices/ibmvmc.txt 
> > b/Documentation/misc-devices/ibmvmc.txt
> > new file mode 100644
> > index 000..bae1064
> > --- /dev/null
> > +++ b/Documentation/misc-devices/ibmvmc.txt

Aren't we doing new documentation in .rst format instead of .txt?

thanks,

greg k-h
--
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 03/10] drivers/peci: Add support for PECI bus driver core

[Greg KH]

On Tue, Apr 10, 2018 at 11:32:05AM -0700, Jae Hyun Yoo wrote:
> +static void peci_adapter_dev_release(struct device *dev)
> +{
> + /* do nothing */
> +}

As per the in-kernel documentation, I am now allowed to make fun of you.

You are trying to "out smart" the kernel by getting rid of a warning
message that was explicitly put there for you to do something.  To think
that by just providing an "empty" function you are somehow fulfilling
the API requirement is quite bold, don't you think?

This has to be fixed.  I didn't put that warning in there for no good
reason.  Please go read the documentation again...

greg k-h
--
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: fix reST markup error in driver-api/usb/typec.rst

[Greg KH]

On Sun, Apr 08, 2018 at 10:47:12AM +0800, changbin...@intel.com wrote:
> From: Changbin Du 
> 
> There is an format error in driver-api/usb/typec.rst that breaks sphinx
> docs building.
> 
> reST markup error:
> /home/changbin/work/linux/Documentation/driver-api/usb/typec.rst:215: 
> (SEVERE/4) Unexpected section title or transition.
> 
> 
> Documentation/Makefile:68: recipe for target 'htmldocs' failed
> make[1]: *** [htmldocs] Error 1
> Makefile:1527: recipe for target 'htmldocs' failed
> make: *** [htmldocs] Error 2
> 
> Signed-off-by: Changbin Du 
> ---
>  Documentation/driver-api/usb/typec.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks, someone else already sent this, sorry.  I'll be sending it
onward after 4.17-rc1 is out.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: make xmldocs failed with error after 4.17 merge period

[Greg KH]

On Fri, Apr 06, 2018 at 11:15:55AM +0300, Heikki Krogerus wrote:
> On Fri, Apr 06, 2018 at 09:57:34AM +0200, Greg KH wrote:
> > On Fri, Apr 06, 2018 at 10:51:09AM +0300, Heikki Krogerus wrote:
> > > On Fri, Apr 06, 2018 at 12:38:42PM +0900, Masanari Iida wrote:
> > > > After merge following patch during 4.17 merger period,
> > > > make xmldocs start to fail with error.
> > > > 
> > > >  [bdecb33af34f79cbfbb656661210f77c8b8b5b5f]
> > > > usb: typec: API for controlling USB Type-C Multiplexers
> > > > 
> > > > Error messages.
> > > > reST markup error:
> > > > /home/iida/Repo/linux-2.6/Documentation/driver-api/usb/typec.rst:215:
> > > > (SEVERE/4) Unexpected section title or transition.
> > > > 
> > > > 
> > > > Documentation/Makefile:93: recipe for target 'xmldocs' failed
> > > > make[1]: *** [xmldocs] Error 1
> > > > Makefile:1527: recipe for target 'xmldocs' failed
> > > > make: *** [xmldocs] Error 2
> > > > 
> > > > $
> > > > 
> > > > An ascii graphic in typec.rst cause the error.
> > > 
> > > Thanks for the report. I'm going to propose that we fix this by
> > > marking the ascii art as comment:
> > > 
> > > diff --git a/Documentation/driver-api/usb/typec.rst 
> > > b/Documentation/driver-api/usb/typec.rst
> > > index feb31946490b..972c11bf4141 100644
> > > --- a/Documentation/driver-api/usb/typec.rst
> > > +++ b/Documentation/driver-api/usb/typec.rst
> > > @@ -212,7 +212,7 @@ port drivers can use USB Role Class API with those.
> > > 
> > >  Illustration of the muxes behind a connector that supports an alternate 
> > > mode:
> > > 
> > > - 
> > > +..   
> > >   |   Connector  |
> > >   
> > >  | |
> > > 
> > > I hope that works.
> > 
> > Try it and see!  :)
> 
> It will fix this issue. I was just wondering if use of ascii art is
> acceptable in general with the .rst files? But then again, why
> wouldn't it be.

There are ways to do this, look at how the v4l2 and I think the drm
subsystems handle ascii art such that "real" drawings end up being
produced.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: make xmldocs failed with error after 4.17 merge period

[Greg KH]

On Fri, Apr 06, 2018 at 10:51:09AM +0300, Heikki Krogerus wrote:
> On Fri, Apr 06, 2018 at 12:38:42PM +0900, Masanari Iida wrote:
> > After merge following patch during 4.17 merger period,
> > make xmldocs start to fail with error.
> > 
> >  [bdecb33af34f79cbfbb656661210f77c8b8b5b5f]
> > usb: typec: API for controlling USB Type-C Multiplexers
> > 
> > Error messages.
> > reST markup error:
> > /home/iida/Repo/linux-2.6/Documentation/driver-api/usb/typec.rst:215:
> > (SEVERE/4) Unexpected section title or transition.
> > 
> > 
> > Documentation/Makefile:93: recipe for target 'xmldocs' failed
> > make[1]: *** [xmldocs] Error 1
> > Makefile:1527: recipe for target 'xmldocs' failed
> > make: *** [xmldocs] Error 2
> > 
> > $
> > 
> > An ascii graphic in typec.rst cause the error.
> 
> Thanks for the report. I'm going to propose that we fix this by
> marking the ascii art as comment:
> 
> diff --git a/Documentation/driver-api/usb/typec.rst 
> b/Documentation/driver-api/usb/typec.rst
> index feb31946490b..972c11bf4141 100644
> --- a/Documentation/driver-api/usb/typec.rst
> +++ b/Documentation/driver-api/usb/typec.rst
> @@ -212,7 +212,7 @@ port drivers can use USB Role Class API with those.
> 
>  Illustration of the muxes behind a connector that supports an alternate mode:
> 
> - 
> +..   
>   |   Connector  |
>   
>  | |
> 
> I hope that works.

Try it and see!  :)
--
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] docs: clarify security-bugs disclosure policy

[Greg KH]

On Wed, Mar 07, 2018 at 01:46:24PM -0800, Dave Hansen wrote:
> 
> From: Dave Hansen 
> 
> I think we need to soften the language a bit.  It might scare folks
> off, especially the:
> 
>We prefer to fully disclose the bug as soon as possible.
> 
> which is not really the case.  Linus says:
> 
>   It's not full disclosure, it's not coordinated disclosure,
>   and it's not "no disclosure".  It's more like just "timely
>   open fixes".
> 
> I changed a bit of the wording in here, but mostly to remove the word
> "disclosure" since it seems to mean very specific things to people
> that we do not mean here.
> 
> Signed-off-by: Dave Hansen 
> Reviewed-by: Dan Williams 
> Cc: Thomas Gleixner 
> Cc: Greg Kroah-Hartman 
> Cc: Linus Torvalds 
> Cc: Alan Cox 
> Cc: Andrea Arcangeli 
> Cc: Andy Lutomirski 
> Cc: Kees Cook 
> Cc: Tim Chen 
> Cc: Alexander Viro 
> Cc: Andrew Morton 
> Cc: linux-doc@vger.kernel.org
> Cc: Jonathan Corbet 
> Cc: Mark Rutland 
> ---
> 

Reviewed-by: Greg Kroah-Hartman 
--
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 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core

[Greg KH]

On Wed, Feb 21, 2018 at 12:42:30PM -0800, Jae Hyun Yoo wrote:
> On 2/21/2018 9:58 AM, Greg KH wrote:
> > On Wed, Feb 21, 2018 at 08:15:59AM -0800, Jae Hyun Yoo wrote:
> > > This commit adds driver implementation for PECI bus into linux
> > > driver framework.
> > > 
> > > Signed-off-by: Jae Hyun Yoo 
> > > ---
> > 
> > Why is there no other Intel developers willing to review and sign off on
> > this patch?  Please get their review first before asking us to do their
> > work for them :)
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> Hi Greg,
> 
> This patch set got our internal review process. Sorry if it's code quality
> is under your expectation but it's the reason why I'm asking you to review
> the code. Could you please share your time to review it?

Nope.  If no other Intel developer thinks it is good enough to put their
name on it as part of their review process, why should I?

Again, please use the resources you have, to fix the obvious problems in
your code, BEFORE asking the community to do that work for you.

greg k-h
--
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 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core

[Greg KH]

On Wed, Feb 21, 2018 at 08:15:59AM -0800, Jae Hyun Yoo wrote:
> This commit adds driver implementation for PECI bus into linux
> driver framework.
> 
> Signed-off-by: Jae Hyun Yoo 
> ---

Why is there no other Intel developers willing to review and sign off on
this patch?  Please get their review first before asking us to do their
work for them :)

thanks,

greg k-h
--
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] Staging: netlogic: platform_net: Fixed '(' at the EOL

[Greg KH]

On Tue, Jan 16, 2018 at 07:33:03PM +0530, Naveen Panwar wrote:
> Hi Guys,
> 
> I submitted a new patch with the suggestions from Al Viro, did you guys
> check it?

I do not see any patch from my in my queue :(

--
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 linux dev-4.10 0/6] Add support PECI and PECI hwmon drivers

[Greg KH]

On Thu, Jan 11, 2018 at 07:56:51PM +1100, Benjamin Herrenschmidt wrote:
> On Thu, 2018-01-11 at 08:30 +0100, Greg KH wrote:
> > 4.13?  Why that kernel?  It too is obsolete and insecure and
> > unsupported.
> 
> Haha, it's n-1. come on :-)

And, if you use it in a device, it's still totally unsupported and
insecure.  Seriously, does no one actually pay attention to the patches
I merge in the stable trees anymore?

Anyway, your other comments are good, glad to see work is progressing
well, and yes it's better than a 2.6.y based kernel, but really, that's
a low bar...

thanks,

greg k-h
--
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 linux dev-4.10 0/6] Add support PECI and PECI hwmon drivers

[Greg KH]

On Thu, Jan 11, 2018 at 12:28:48AM -0800, Joel Stanley wrote:
> On Wed, Jan 10, 2018 at 11:30 PM, Greg KH  wrote:
> > On Wed, Jan 10, 2018 at 01:46:34PM -0800, Jae Hyun Yoo wrote:
> >> Thanks for your pointing it out and I totally agree with you. Actually, we
> >> are preparing 4.13 update for now and an another update will be followed 
> >> up.
> >> As I answered above, I'll rebase this patch set onto the latest kernel.org
> >> mainline. Sorry for my misunderstanding of upstream process.
> >
> > 4.13?  Why that kernel?  It too is obsolete and insecure and
> > unsupported.
> 
> It contains support for our hardware that I have integrated from work
> in progress patches and upstream commits.
> 
> The OpenBMC project, with myself as the kernel maintainer, have
> intentions to regularly move to upstream releases. This takes time and
> effort. This time and effort is balanced with submitting our drivers
> upstream.

Of course, but please do not have your "users" use a kernel that is
known to have bugs and can not be supported.  That would not be good at
all, don't you think?

> > What keeps you all from just always tracking the latest tree from Linus?
> 
> Linus' tree does not contain all of the drivers required to boot
> systems. Many of them are still under review on lkml, and others still
> require rewrite from the vendor tree.

Merging vendor trees into your tree has got to be a complicated mess.
Why try to keep it all together in one place?

And who is responsible for getting the vendor code upstream?  The
individual drivers?  Individual driver submissions should be quite easy,
what is preventing them from getting merged?

> > What is in your tree that is not upstream that requires you to have a
> > kernel tree at all?
> 
> We have PECI, video compression, crypto, USB CDC, DRM (graphics),
> serial GPIO, LPC mailbox for the ASPEED SoC.

What "USB CDC" do you have that is not upstream?  I'll pick on this one
specifically as I don't think I've seen any patches recently submitted
for that driver at all.  Am I just missing them?

The other ones should also all be easy to get merged, with maybe the
exception of the drm stuff due to the speed that subsystem moves at.
But even there, the community is very helpful in getting stuff upstream,
have you asked for help?

> Another silicon vendor has recently joined the project and that brings
> an entire SoC that is not upstream. We have patches on the ARM that
> are under review for this SoC, with more drivers undergoing cleanup in
> order to submit them to the relevant maintainers.

Why are you merging all SoC trees together into one place?  That seems
like a nightmare to manage, especially with git.

> > And if you do have out-of-tree code, why not use a process that makes it
> > trivial to update the base kernel version so that you can keep up to
> > date very easily?  (hint, just using 'git' is not a good way to do
> > this...)
> 
> We have a process that we've been developing under for the past few
> years. I find git to be a great tool for managing Linux kernel trees.
> 
> What would you recommend for managing kernel trees?

quilt is best for a tree that you can not rebase (i.e. a public git
tree).  Otherwise you end up getting patches all mushed together and
hard to extract in any simple way.

Take a clue from the distros that have been managing kernels for decades
and deal with an updated kernel all the time easily.

Good luck, it sounds like you will need it :)

thanks,

greg k-h
--
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 linux dev-4.10 0/6] Add support PECI and PECI hwmon drivers

[Greg KH]

On Wed, Jan 10, 2018 at 01:46:34PM -0800, Jae Hyun Yoo wrote:
> On 1/10/2018 12:27 PM, Greg KH wrote:
> > On Wed, Jan 10, 2018 at 11:30:05AM -0800, Jae Hyun Yoo wrote:
> > > On 1/10/2018 11:17 AM, Greg KH wrote:
> > > > On Wed, Jan 10, 2018 at 11:14:34AM -0800, Jae Hyun Yoo wrote:
> > > > > On 1/10/2018 2:17 AM, Greg KH wrote:
> > > > > > On Tue, Jan 09, 2018 at 02:31:20PM -0800, Jae Hyun Yoo wrote:
> > > > > > > From: Jae Hyun Yoo 
> > > > > > > 
> > > > > > > Hello,
> > > > > > > 
> > > > > > > This patch set provides support for PECI of AST2400/2500 which 
> > > > > > > can give us PECI
> > > > > > > functionalities such as temperature monitoring, platform 
> > > > > > > manageability,
> > > > > > > processor diagnostics and failure analysis. Also provides generic 
> > > > > > > peci.h and
> > > > > > > peci_ioctl.h headers to provide compatibility to peci drivers 
> > > > > > > that can be
> > > > > > > implemented later e.g. Nuvoton's BMC SoC family.
> > > > > > 
> > > > > > What is the "dev-4.10" in the subject for?  4.10 is really old and
> > > > > > obsolete :(
> > > > > > 
> > > > > > thanks,
> > > > > > 
> > > > > > greg k-h
> > > > > > 
> > > > > 
> > > > > I made this patch set on top of the v4.10 which OpenBmc project is 
> > > > > currently
> > > > > using. I'll rebase this patch set onto the current kernel.org 
> > > > > mainline.
> > > > 
> > > > What is "OpenBmc", and why are they using an obsolete and insecure
> > > > kernel for their project?  That seems like a very foolish thing to do...
> > > > 
> > > > thanks,
> > > > 
> > > > greg k-h
> > > > 
> > > 
> > > OpenBmc is an open source project to create a highly extensible framework
> > > for BMC (Board Management Controller) software for data-center computer
> > > systems:
> > > https://github.com/openbmc
> > > 
> > > Its current mainline is v4.10 but it is being kept upgrading so it will be
> > > upgraded to the latest stable or long-term version soon.
> > 
> > Why hasn't it been updated in the year since 4.10 was released?  That's
> > a _very_ long time to be running on a totally insecure kernel, and no
> > new development should ever be done on old kernels, that's even crazier
> > (as we can't go back in time and accept patches for new features to old
> > releases...)
> > 
> 
> Thanks for your pointing it out and I totally agree with you. Actually, we
> are preparing 4.13 update for now and an another update will be followed up.
> As I answered above, I'll rebase this patch set onto the latest kernel.org
> mainline. Sorry for my misunderstanding of upstream process.

4.13?  Why that kernel?  It too is obsolete and insecure and
unsupported.

What keeps you all from just always tracking the latest tree from Linus?
What is in your tree that is not upstream that requires you to have a
kernel tree at all?

And if you do have out-of-tree code, why not use a process that makes it
trivial to update the base kernel version so that you can keep up to
date very easily?  (hint, just using 'git' is not a good way to do
this...)

thanks,

greg k-h
--
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 linux dev-4.10 0/6] Add support PECI and PECI hwmon drivers

[Greg KH]

On Wed, Jan 10, 2018 at 11:30:05AM -0800, Jae Hyun Yoo wrote:
> On 1/10/2018 11:17 AM, Greg KH wrote:
> > On Wed, Jan 10, 2018 at 11:14:34AM -0800, Jae Hyun Yoo wrote:
> > > On 1/10/2018 2:17 AM, Greg KH wrote:
> > > > On Tue, Jan 09, 2018 at 02:31:20PM -0800, Jae Hyun Yoo wrote:
> > > > > From: Jae Hyun Yoo 
> > > > > 
> > > > > Hello,
> > > > > 
> > > > > This patch set provides support for PECI of AST2400/2500 which can 
> > > > > give us PECI
> > > > > functionalities such as temperature monitoring, platform 
> > > > > manageability,
> > > > > processor diagnostics and failure analysis. Also provides generic 
> > > > > peci.h and
> > > > > peci_ioctl.h headers to provide compatibility to peci drivers that 
> > > > > can be
> > > > > implemented later e.g. Nuvoton's BMC SoC family.
> > > > 
> > > > What is the "dev-4.10" in the subject for?  4.10 is really old and
> > > > obsolete :(
> > > > 
> > > > thanks,
> > > > 
> > > > greg k-h
> > > > 
> > > 
> > > I made this patch set on top of the v4.10 which OpenBmc project is 
> > > currently
> > > using. I'll rebase this patch set onto the current kernel.org mainline.
> > 
> > What is "OpenBmc", and why are they using an obsolete and insecure
> > kernel for their project?  That seems like a very foolish thing to do...
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> OpenBmc is an open source project to create a highly extensible framework
> for BMC (Board Management Controller) software for data-center computer
> systems:
> https://github.com/openbmc
> 
> Its current mainline is v4.10 but it is being kept upgrading so it will be
> upgraded to the latest stable or long-term version soon.

Why hasn't it been updated in the year since 4.10 was released?  That's
a _very_ long time to be running on a totally insecure kernel, and no
new development should ever be done on old kernels, that's even crazier
(as we can't go back in time and accept patches for new features to old
releases...)

It sounds like the openbmc project needs to learn how to manage their
kernels a whole lot better, who do I need to go poke about this?

thanks,

greg k-h
--
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 linux dev-4.10 0/6] Add support PECI and PECI hwmon drivers

[Greg KH]

On Wed, Jan 10, 2018 at 11:14:34AM -0800, Jae Hyun Yoo wrote:
> On 1/10/2018 2:17 AM, Greg KH wrote:
> > On Tue, Jan 09, 2018 at 02:31:20PM -0800, Jae Hyun Yoo wrote:
> > > From: Jae Hyun Yoo 
> > > 
> > > Hello,
> > > 
> > > This patch set provides support for PECI of AST2400/2500 which can give 
> > > us PECI
> > > functionalities such as temperature monitoring, platform manageability,
> > > processor diagnostics and failure analysis. Also provides generic peci.h 
> > > and
> > > peci_ioctl.h headers to provide compatibility to peci drivers that can be
> > > implemented later e.g. Nuvoton's BMC SoC family.
> > 
> > What is the "dev-4.10" in the subject for?  4.10 is really old and
> > obsolete :(
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> I made this patch set on top of the v4.10 which OpenBmc project is currently
> using. I'll rebase this patch set onto the current kernel.org mainline.

What is "OpenBmc", and why are they using an obsolete and insecure
kernel for their project?  That seems like a very foolish thing to do...

thanks,

greg k-h
--
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 linux dev-4.10 3/6] drivers/misc: Add driver for Aspeed PECI and generic PECI headers

[Greg KH]

On Tue, Jan 09, 2018 at 02:31:23PM -0800, Jae Hyun Yoo wrote:
> This commit adds driver implementation for Aspeed PECI. Also adds
> generic peci.h and peci_ioctl.h files to provide compatibility
> to peci drivers that can be implemented later e.g. Nuvoton's BMC
> SoC family.

We don't add code that could be used "sometime in the future".  Only
include stuff that we use now.

Please fix up this series based on that and resubmit.  There should not
be any need for any uapi file then, right?

thanks,

greg k-h
--
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 linux dev-4.10 3/6] drivers/misc: Add driver for Aspeed PECI and generic PECI headers

[Greg KH]

On Tue, Jan 09, 2018 at 02:31:23PM -0800, Jae Hyun Yoo wrote:
> +#pragma pack(push, 1)
> +struct peci_xfer_msg {
> + unsigned char client_addr;
> + unsigned char tx_len;
> + unsigned char rx_len;
> + unsigned char tx_buf[MAX_BUFFER_SIZE];
> + unsigned char rx_buf[MAX_BUFFER_SIZE];
> +};
> +#pragma pack(pop)

For any structure that crosses the user/kernel boundry, you _HAVE_ to
use the "__" variant.  So for here you would use __u8 instead of
"unsigned char" in order for things to work properly.

I'm guessing you didn't test this all out on a mixed 32/64 bit system?

Please fix up and test to ensure that it all works properly before
resubmitting.

thanks,

greg k-h
--
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 linux dev-4.10 0/6] Add support PECI and PECI hwmon drivers

[Greg KH]

On Tue, Jan 09, 2018 at 02:31:20PM -0800, Jae Hyun Yoo wrote:
> From: Jae Hyun Yoo 
> 
> Hello,
> 
> This patch set provides support for PECI of AST2400/2500 which can give us 
> PECI
> functionalities such as temperature monitoring, platform manageability,
> processor diagnostics and failure analysis. Also provides generic peci.h and
> peci_ioctl.h headers to provide compatibility to peci drivers that can be
> implemented later e.g. Nuvoton's BMC SoC family.

What is the "dev-4.10" in the subject for?  4.10 is really old and
obsolete :(

thanks,

greg k-h
--
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] arm64: v8.4: Support for new floating point multiplication instructions

[Greg KH]

On Fri, Jan 05, 2018 at 09:22:54AM +0800, gengdongjiu wrote:
> Hi will/catalin
> 
> On 2017/12/13 18:09, Suzuki K Poulose wrote:
> > On 13/12/17 10:13, Dongjiu Geng wrote:
> >> ARM v8.4 extensions add new neon instructions for performing a
> >> multiplication of each FP16 element of one vector with the corresponding
> >> FP16 element of a second vector, and to add or subtract this without an
> >> intermediate rounding to the corresponding FP32 element in a third vector.
> >>
> >> This patch detects this feature and let the userspace know about it via a
> >> HWCAP bit and MRS emulation.
> >>
> >> Cc: Dave Martin 
> >> Cc: Suzuki K Poulose 
> >> Signed-off-by: Dongjiu Geng 
> >> Reviewed-by: Dave Martin 
> > 
> > Looks good to me.
> > 
> > Reviewed-by: Suzuki K Poulose 
> 
>  sorry to disturb you. Reminder, hope this patch can be applied to Linux 
> 4.15-rc7.

New features should not be going into 4.15-rc, that should be a 4.16-rc1
thing, right?

thanks,

greg k-h
--
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: device attribute documentation

[Greg KH]

On Thu, Dec 14, 2017 at 10:09:44AM -0700, Jonathan Corbet wrote:
> On Thu, 14 Dec 2017 15:30:13 +0100 (CET)
> Julia Lawall  wrote:
> 
> [CC += Greg in case he disagrees]
> 
> > My intern, Aishwarya Pant, is looking into how to improve the
> > documentation of device attributes.  She collected a list of attirbutes
> > that are not represented anywhere under Documentation/ABI, but soeof these
> > seem to be represented elsewhere, for examples:
> > 
> > Documentation/hwmon/sht3x:
> > 
> > sysfs-Interface
> > ---
> > 
> > temp1_input:temperature input
> > humidity1_input:humidity input
> > temp1_max:  temperature max value
> > ...
> > 
> > This is not in the ABI subdirectory and is not in the format used there.
> > Would it be useful to move the information there and fill in the missing
> > parts, or is it good enough as is?
> 
> This seems like a useful exercise and yes, that does seem to me like
> information that should be kept in the ABI directory with the rest.

No objection from me, this would be great to have!

thanks,

greg k-h
--
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] usb: core: Add "quirks" parameter for usbcore

[Greg KH]

On Wed, Dec 06, 2017 at 06:26:21PM +0800, Kai-Heng Feng wrote:
> Trying quirks in usbcore needs to rebuild the driver or the entire
> kernel if it's builtin. It can save a lot of time if usbcore has similar
> ability like "usbhid.quirks=" and "usb-storage.quirks=".
> 
> Rename the original quirk detection function to "static" as we introduce
> this new "dynamic" function.
> 
> Now users can use "usbcore.quirks=" as short term workaround before the
> next kernel release.
> 
> This is inspired by usbhid and usb-storage.
> 
> Signed-off-by: Kai-Heng Feng 
> ---
> v2: use in-kernel tolower() function.
> 
>  Documentation/admin-guide/kernel-parameters.txt |  55 +
>  drivers/usb/core/quirks.c   | 100 
> ++--
>  include/linux/usb/quirks.h  |   2 +
>  3 files changed, 151 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt 
> b/Documentation/admin-guide/kernel-parameters.txt
> index 6571fbfdb2a1..d42fb278320b 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4302,6 +4302,61 @@
>  
>   usbcore.nousb   [USB] Disable the USB subsystem
>  
> + usbcore.quirks=
> + [USB] A list of quirks entries to supplement or
> + override the built-in usb core quirk list.  List
> + entries are separated by commas.  Each entry has
> + the form VID:PID:Flags where VID and PID are Vendor
> + and Product ID values (4-digit hex numbers) and
> + Flags is a set of characters, each corresponding
> + to a common usb core quirk flag as follows:
> + a = USB_QUIRK_STRING_FETCH_255 (string
> + descriptors must not be fetched using
> + a 255-byte read);
> + b = USB_QUIRK_RESET_RESUME (device can't resume
> + correctly so reset it instead);
> + c = USB_QUIRK_NO_SET_INTF (device can't handle
> + Set-Interface requests);
> + d = USB_QUIRK_CONFIG_INTF_STRINGS (device can't
> + handle its Configuration or Interface
> + strings);
> + e = USB_QUIRK_RESET (device can't be reset
> + (e.g morph devices), don't use reset);
> + f = USB_QUIRK_HONOR_BNUMINTERFACES (device has
> + more interface descriptions than the
> + bNumInterfaces count, and can't handle
> + talking to these interfaces);
> + g = USB_QUIRK_DELAY_INIT (device needs a pause
> + during initialization, after we read
> + the device descriptor);
> + h = USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL (For
> + high speed and super speed interrupt
> + endpoints, the USB 2.0 and USB 3.0 spec
> + require the interval in microframes (1
> + microframe = 125 microseconds) to be
> + calculated as interval = 2 ^
> + (bInterval-1).
> + Devices with this quirk report their
> + bInterval as the result of this
> + calculation instead of the exponent
> + variable used in the calculation);
> + i = USB_QUIRK_DEVICE_QUALIFIER (device can't
> + handle device_qualifier descriptor
> + requests);
> + j = USB_QUIRK_IGNORE_REMOTE_WAKEUP (device
> + generates spurious wakeup, ignore
> + remote wakeup capability);
> + k = USB_QUIRK_NO_LPM (device can't handle Link
> + Power Management);
> + l = USB_QUIRK_LINEAR_FRAME_INTR_BINTERVAL
> + (Device reports its bInterval as linear
> + frames instead of the USB 2.0
> + calculation);
> + m = USB_QUIRK_DISCONNECT_SUSPEND (Device needs
> + to be disconnected before suspend to
> +

Re: [PATCH] usb: core: Add "quirks" parameter for usbcore

[Greg KH]

On Wed, Dec 06, 2017 at 05:09:32PM +0800, Kai-Heng Feng wrote:
> +/* Works only for digits and letters, but small and fast */
> +#define TOLOWER(x) ((x) | 0x20)

What is wrong with the in-kernel version of tolower()?

thanks,

greg k-h
--
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] mod_devicetable.h: Fix docs build warnings

[Greg KH]

On Mon, Jul 24, 2017 at 01:54:17PM -0600, Jonathan Corbet wrote:
> Commit 0afef45654ae908536278ecb143ded5bbc713391 (staging: fsl-mc: add
> support for device table matching) added kerneldoc comments for two
> nonexistent structure fields, leading to these warnings in the docs build:
> 
>   ./include/linux/mod_devicetable.h:687: warning: Excess 
> struct/union/enum/typedef member 'ver_major' description in 'fsl_mc_device_id'
>   ./include/linux/mod_devicetable.h:687: warning: Excess 
> struct/union/enum/typedef member 'ver_minor' description in 'fsl_mc_device_id'
> 
> Remove the offending lines to make the docs build a bit quieter.
> 
> CC: Stuart Yoder 
> Signed-off-by: Jonathan Corbet 
> ---
> Greg, you seem as likely a person as any to take this one...?

Yes, I'll take it, someone else just sent this to me before you did so
I'll add your signed-off-by to it and use it.

thanks,

greg k-h
--
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 6/8] usb: gadget: f_acm: add an ioctl to get the current line coding

[Greg KH]

On Tue, Jun 13, 2017 at 09:52:12AM +0300, Tal Shorer wrote:
> The user can issue USB_F_GET_LINE_CODING to get the current line coding
> as set by the host (or the default if unset yet).
> 
> Signed-off-by: Tal Shorer 
> ---
>  Documentation/ioctl/ioctl-number.txt |  1 +
>  drivers/usb/gadget/function/f_acm.c  | 19 +++
>  include/uapi/linux/usb/f_acm.h   | 12 

Where is this ioctl being called?  On the tty device?  If so, which one?
The gadget driver's tty device node?  Or somewhere else?

confused at the different levels here, sorry.

greg k-h
--
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 0/3] move visorbus out of staging to drivers/virt/visorbus

[Greg KH]

On Tue, Jun 06, 2017 at 06:20:17PM +, Kershner, David A wrote:
> 
> 
> > -Original Message-
> > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > Sent: Tuesday, June 6, 2017 11:06 AM
> > To: Kershner, David A 
> > Cc: cor...@lwn.net; t...@linutronix.de; mi...@kernel.org; akpm@linux-
> > foundation.org; jes.soren...@gmail.com; linux-ker...@vger.kernel.org;
> > linux-doc@vger.kernel.org; driverdev-de...@linuxdriverproject.org; *S-Par-
> > Maintainer 
> > Subject: Re: [PATCH 0/3] move visorbus out of staging to
> > drivers/virt/visorbus
> > 
> > On Tue, Jun 06, 2017 at 04:54:30PM +0200, Greg KH wrote:
> > > On Tue, Jun 06, 2017 at 04:53:22PM +0200, Greg KH wrote:
> > > > On Tue, Jun 06, 2017 at 04:49:09PM +0200, Greg KH wrote:
> > > > > On Mon, Jun 05, 2017 at 04:07:29PM -0400, David Kershner wrote:
> > > > > > This patchset moves drivers/staging/unisys/include to
> > > > > > include/linux/visorbus, and moves drivers/staging/unisys/visorbus to
> > > > > > drivers/virt/visorbus.
> > > > >
> > > > > Um, are you thinking it is ready to be moved?  Have you asked for
> > > > > another review?
> > > > >
> 
> Thank you for taking a quick look at our patch series. Part of the motivation
> behind this submission was, in fact, to initiate another code review. What is
> the formal procedure for initiating a code review?

Send an email that says, "Hey Greg, we think the code is ready to be
moved out of staging, can you review it to see if we have missed
anything?"

Of course, do it _AFTER_ you have fixed up the checkpatch.pl issues.
That's not even done yet...

thanks,

greg k-h
--
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 0/3] move visorbus out of staging to drivers/virt/visorbus

[Greg KH]

On Tue, Jun 06, 2017 at 08:52:27AM -0700, Joe Perches wrote:
> On Tue, 2017-06-06 at 17:39 +0200, Greg KH wrote:
> > On Tue, Jun 06, 2017 at 08:33:49AM -0700, Joe Perches wrote:
> > > On Tue, 2017-06-06 at 16:53 +0200, Greg KH wrote:
> > > > On Tue, Jun 06, 2017 at 04:49:09PM +0200, Greg KH wrote:
> > > > > I noticed that in drivers/staging/unisys/visorbus/visorbus_main.c, you
> > > > > have 2 tabs for your 'struct attribute' variables, which is really 
> > > > > odd.
> > > 
> > > []
> > > > Also, many of the attribute callbacks in that file seem to all have
> > > > their leading '{' in the wrong place.  Odd that checkpatch.pl doesn't
> > > > catch that...
> []
> > the following code in that file should be caught, right:
> > 
> > static ssize_t partition_handle_show(struct device *dev,
> >  struct device_attribute *attr,
> >  char *buf) {
> > struct visor_device *vdev = to_visor_device(dev);
> > u64 handle = visorchannel_get_clientpartition(vdev->visorchannel);
> > 
> > return sprintf(buf, "0x%llx\n", handle);
> > }
> > static DEVICE_ATTR_RO(partition_handle);
> 
> Not really.
> 
> > The initial { is in the wrong place...
> 
> True.
> 
> Please understand that checkpatch looks at patches one line
> at a time.  It's not very smart about function definitions
> or context.
> 
> checkpatch's function definition code is pretty limited.
> It can miss a lot of style misuses.
> 
> Single line function definitions brace tests work well.
> Multiple line function definitions do not.

Ok, that makes sense why this is missed.  No big deal, a simple visual
inspection shows stuff like this up really easily, which obviously no
one did yet on this file :)

thanks,

greg k-h
--
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