Re: [PATCH] clk: actions: Use the correct style for SPDX License Identifier
From: Greg Kroah-Hartman
Date: Thu May 02 2019 - 07:31:14 EST
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-... */
> /* ...
> Some people deviate by doing
> /* SPDX-...
> * foo
> 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.