linux-kernel - Re: [PATCH] tty: New RISC-V SBI console driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 8 Dec 2017 17:59:27 +0100
From:   Philippe Ombredanne <pombredanne@...b.com>
To:     Karsten Merker <merker@...ian.org>
Cc:     Palmer Dabbelt <palmer@...belt.com>, patches@...ups.riscv.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        jslaby@...e.com, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] tty: New RISC-V SBI console driver

Karsten.

On Fri, Dec 8, 2017 at 5:22 PM, Karsten Merker <merker@...ian.org> wrote:
> On Thu, Dec 07, 2017 at 04:10:15PM -0800, Palmer Dabbelt wrote:
>> From: Palmer Dabbelt <palmer@...belt.com>
>>
>> The RISC-V ISA defines a simple console that is availiable via SBI calls
>> on all systems.  This patch adds a driver for this console interface
>> that can act as both a target for early printk and as the system
>> console.  The core arch code already enables the early printk support
>> when CONFIG_HVC_RISCV_SBI is defined.
>>
>> There is one checkpatch.pl warning here: to check the MAINTAINERS file.
>> They're all matched by the "K: riscv" line.
>>
>> Signed-off-by: Palmer Dabbelt <palmer@...belt.com>
>
>> diff --git a/arch/riscv/include/asm/hvc_riscv_sbi.h b/arch/riscv/include/asm/hvc_riscv_sbi.h
>> new file mode 100644
>> index 000000000000..41723ed7bd97
>> --- /dev/null
>> +++ b/arch/riscv/include/asm/hvc_riscv_sbi.h
>> @@ -0,0 +1,12 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +
>> +#ifndef _ASM_RISCV_HVC_RISCV_SBI_H
>> +#define _ASM_RISCV_HVC_RISCV_SBI_H
> [...]
>> diff --git a/drivers/tty/hvc/hvc_riscv_sbi.c b/drivers/tty/hvc/hvc_riscv_sbi.c
>> new file mode 100644
>> index 000000000000..83ef0dcc24e0
>> --- /dev/null
>> +++ b/drivers/tty/hvc/hvc_riscv_sbi.c
>> @@ -0,0 +1,75 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +
>> +#include <linux/console.h>
> [...]
>
> Hello Palmer,
>
> just a non-technical nitpick: the SPDX-License-Identifier states
> the license, but not the copyright holder(s).  Could you please
> add an appropriate copyright line to each new file, i.e.
> something in the form of "copyright <year> <author>"?
>
> Although one can try to determine authorship from the git
> history, IMHO authorship should be noted explicitly in each file,
> as that information then sticks to the file whatever happens to
> it.  Files might be reused outside the kernel (e.g. in u-boot,
> which reuses quite a bit of Linux driver code), and in those
> cases the connection between the file and its original git
> history gets lost.  The same is true for example for the kernel
> releases in form of release tarballs which of course don't
> contain any git metadata. I therefore think it is good practice
> to always include the copyright/authorship information in
> each file.

FWIW, I fully agree with you and you nailed it: the combo of SPDX ids
+ a clear copyrights makes both license and author easy to discover in
and out of tree: explicit (and concise, and easy to grep) is a win!

-- 
Cordially
Philippe Ombredanne