Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 2 May 2019 13:28:53 +0200
From:   Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To:     Andreas =?iso-8859-1?Q?F=E4rber?= <afaerber@suse.de>
Cc:     Nishad Kamdar <nishadkamdar@gmail.com>,
        Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>, Joe Perches <joe@perches.com>,
        Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= 
        <u.kleine-koenig@pengutronix.de>,
        linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-actions@lists.infradead.org
Subject: Re: [PATCH] clk: actions: Use the correct style for SPDX License
 Identifier
Message-ID: <20190502112853.GB7358@kroah.com>
References: <20190501070707.GA5619@nishad>
 <057d9b37-7475-1902-bce7-6d519c2e0fdf@suse.de>
 <20190502070746.GA16247@kroah.com>
 <315de620-b638-aea4-d8d2-e00f5a493625@suse.de>
 <20190502103848.GA17256@kroah.com>
 <f52484fc-b35b-f92a-9c7b-ce53fd731ab5@suse.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <f52484fc-b35b-f92a-9c7b-ce53fd731ab5@suse.de>
User-Agent: Mutt/1.11.4 (2019-03-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, May 02, 2019 at 12:45:05PM +0200, Andreas F?rber wrote:
> Am 02.05.19 um 12:38 schrieb Greg Kroah-Hartman:
> > On Thu, May 02, 2019 at 12:25:36PM +0200, Andreas F?rber wrote:
> >> Am 02.05.19 um 09:07 schrieb Greg Kroah-Hartman:
> >>> On Wed, May 01, 2019 at 10:20:44PM +0200, Andreas F?rber wrote:
> >>>> + linux-actions
> >>>>
> >>>> Am 01.05.19 um 09:07 schrieb Nishad Kamdar:
> >>>>> This patch corrects the SPDX License Identifier style
> >>>>> in header files related to Clock Drivers for Actions Semi Socs.
> >>>>> For C header files Documentation/process/license-rules.rst
> >>>>> mandates C-like comments (opposed to C source files where
> >>>>> C++ style should be used)
> >>>> [...]
> >>>>>  drivers/clk/actions/owl-common.h       | 2 +-
> >>>>>  drivers/clk/actions/owl-composite.h    | 2 +-
> >>>>>  drivers/clk/actions/owl-divider.h      | 2 +-
> >>>>>  drivers/clk/actions/owl-factor.h       | 2 +-
> >>>>>  drivers/clk/actions/owl-fixed-factor.h | 2 +-
> >>>>>  drivers/clk/actions/owl-gate.h         | 2 +-
> >>>>>  drivers/clk/actions/owl-mux.h          | 2 +-
> >>>>>  drivers/clk/actions/owl-pll.h          | 2 +-
> >>>>>  drivers/clk/actions/owl-reset.h        | 2 +-
> >>>>>  9 files changed, 9 insertions(+), 9 deletions(-)
> >>>>
> >>>> Where's the practical benefit of this patch? These are all private
> >>>> headers used from C files, so they can handle C++ comments just fine,
> >>>> otherwise we would've seen build failures.
> >>>
> >>> Please read Documentation/process/license-rules.rst, the section
> >>> entitled "Style", for what the documented formats are for SPDX lines,
> >>> depending on the file type.
> >>
> >> That does in no way answer my question! You conveniently dropped my
> >> paragraph indicating that I understand why we would do that for public
> >> headers in include/, but none of these private headers here are included
> >> in .lds files. So there really seems to be no benefit of switching from
> >> one style to another for in-tree code.
> > 
> > It should answer the question, it was "decreed" that all header files
> > use /* */, and all C files use // for their SPDX lines, so we documented
> > it that way.
> > 
> > Yes, maybe it doesn't make "sense" in that this really is only needed
> > for headers that get included into asm files, which is why we had to do
> > it this way, but it's better to be consistant than to have random
> > breakages at times.
> > 
> > It's not an issue of public headers at all, sorry.
> > 
> > Consistency is good, as we can have automatic tools check these types of
> > things, which is the only way to reliably handle the format of something
> > that needs to be in every file in a project with 63,100+ different
> > files.
> 
> Okay, if it's about consistency then there will be more cases to fix.

Agreed, hopefully checkpatch is up to date enough to catch these.

> What about this one:
> 
> My interpretation of the documentation has been that I should end the
> comment after the identifiers:
> 
> /* SPDX-... */
> /* ...
>  */

Correct.

> Some people deviate by doing
> 
> /* SPDX-...
>  * foo
>  */

Not correct.

> So the documentation may need to be extended to clarify that for full
> consistency, as well as clarify the previous scenario:
>   "If a specific tool cannot handle the standard comment style, then the
>    appropriate comment mechanism which the tool accepts shall be used."
> To me that reads very different from what you just said above.

Documentation can always be updated, a patch to make it clearer is
always appreciated.  But look at what we have today in the document, I
think it should be pretty obvious that:
	/* SPDX... */
is the thing to use for C header files.

If you disagree, that's fine, please send a patch to make it clearer and
we can all review it.

thanks,

greg k-h