Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1753148AbdLHRAK (ORCPT <rfc822;w@1wt.eu>);
        Fri, 8 Dec 2017 12:00:10 -0500
Received: from mail-wm0-f68.google.com ([74.125.82.68]:38494 "EHLO
        mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1752105AbdLHRAJ (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Fri, 8 Dec 2017 12:00:09 -0500
X-Google-Smtp-Source: AGs4zMbBqtTb6kaBES2q+rci4Oi/T0L7Un9tp5Qd2djM0DWJY3UPcJri+B0kBsXJFgkrJsq9woWNkD0USnI0qLCEip8=
MIME-Version: 1.0
In-Reply-To: <20171208162209.52bkebfvw7mirbit@excalibur.cnev.de>
References: <20171208001015.27735-1-palmer@sifive.com> <20171208162209.52bkebfvw7mirbit@excalibur.cnev.de>
From: Philippe Ombredanne <pombredanne@nexb.com>
Date: Fri, 8 Dec 2017 17:59:27 +0100
Message-ID: <CAOFm3uE+6b=-n=n9NbyPoOFP2Nmns9DM6DW4AaYn2dgf9ED1zw@mail.gmail.com>
Subject: Re: [PATCH] tty: New RISC-V SBI console driver
To: Karsten Merker <merker@debian.org>
Cc: Palmer Dabbelt <palmer@dabbelt.com>, patches@groups.riscv.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>, jslaby@suse.com,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 2558
Lines: 65

Karsten.

On Fri, Dec 8, 2017 at 5:22 PM, Karsten Merker <merker@debian.org> wrote:
> On Thu, Dec 07, 2017 at 04:10:15PM -0800, Palmer Dabbelt wrote:
>> From: Palmer Dabbelt <palmer@dabbelt.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@dabbelt.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