netdev - Re: [PATCH net-next v5 2/2] net: ethernet: socionext: add AVE ethernet 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:   Mon, 11 Dec 2017 22:34:17 +0900
From:   Masami Hiramatsu <masami.hiramatsu@...aro.org>
To:     Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>
Cc:     Philippe Ombredanne <pombredanne@...b.com>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        Linux kernel mailing list <linux-kernel@...r.kernel.org>,
        devicetree@...r.kernel.org,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Jassi Brar <jaswinder.singh@...aro.org>
Subject: Re: [PATCH net-next v5 2/2] net: ethernet: socionext: add AVE
 ethernet driver

Hello,

2017-12-11 21:04 GMT+09:00 Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>:
> On Mon, 11 Dec 2017 10:19:15 +0100 Phlippe Ombredanne <pombredanne@...b.com> wrote:
>
>> Dear Kunihiko-san,
>>
>> On Mon, Dec 11, 2017 at 8:57 AM, Kunihiko Hayashi
>> <hayashi.kunihiko@...ionext.com> wrote:
>> > The UniPhier platform from Socionext provides the AVE ethernet
>> > controller that includes MAC and MDIO bus supporting RGMII/RMII
>> > modes. The controller is named AVE.
>> >
>> > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>
>> > Signed-off-by: Jassi Brar <jaswinder.singh@...aro.org>
>> > Reviewed-by: Andrew Lunn <andrew@...n.ch>
>> [...]
>> > --- /dev/null
>> > +++ b/drivers/net/ethernet/socionext/Makefile
>> > @@ -0,0 +1,5 @@
>> > +# SPDX-License-Identifier: GPL-2.0
>>
>> You are correctly using SPDX ids here....
>>
>> > +obj-$(CONFIG_SNI_AVE) += sni_ave.o
>> > diff --git a/drivers/net/ethernet/socionext/sni_ave.c b/drivers/net/ethernet/socionext/sni_ave.c
>> > new file mode 100644
>> > index 0000000..7b293c2
>> > --- /dev/null
>> > +++ b/drivers/net/ethernet/socionext/sni_ave.c
>> > @@ -0,0 +1,1744 @@
>> > +/**
>> > + * sni_ave.c - Socionext UniPhier AVE ethernet driver
>> > + *
>> > + * Copyright 2014 Panasonic Corporation
>> > + * Copyright 2015-2017 Socionext Inc.
>> > + *
>> > + * This program is free software: you can redistribute it and/or modify
>> > + * it under the terms of the GNU General Public License version 2  of
>> > + * the License as published by the Free Software Foundation.
>> > + *
>> > + * This program is distributed in the hope that it will be useful,
>> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> > + * GNU General Public License for more details.
>> > + */
>>
>> ... then I guess you could also use them here, replacing at least 7
>> lines of boilerplate by a single id line?
>>
>> > +// SDPX-License-Identifier: GPL-2.0
>
> Thank you for your kindly comment.
>
> It seems there are few ethernet drivers applying SPDX-License-Identifier
> to C source, then it has been difficult for me to decide whether to replace
> boilerplate.
>
> If it's no problem to apply SPDX to sources of ethernet drivers,
> I'll replace it with the single line.
>
>> And if you go C++ style all the way, this could be even more compact:
>>
>> > +// SDPX-License-Identifier: GPL-2.0
>> > +// sni_ave.c - Socionext UniPhier AVE ethernet driver
>> > +// Copyright 2014 Panasonic Corporation
>> > +// Copyright 2015-2017 Socionext Inc.
>
> It's much simpler.
> If it's reasonable to apply this style here, I can replace it, too.

IMHO, even if we use SPDX license identifier, I recommend to use
C-style comments as many other files do, since it is C code.
If SPDX identifier requires C++ style, that is SPDX parser's issue
and should be fixed to get it from C-style comment.

https://spdx.org/sites/cpstandard/files/pages/files/using_spdx_license_list_short_identifiers.pdf

Anyway, SPDX.org itself shows C-style (short header) example. See page 6.

Thank you,

>
> Thank you,
>
> ---
> Best Regards,
> Kunihiko Hayashi
>
>



-- 
Masami Hiramatsu