linux-kernel - Re: [alsa-devel][PATCH v3] ASoC: TSCS42xx: Add support for Tempo Semiconductor's TSCS42xx audio CODEC

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]

Message-ID: <CAOFm3uHd_mepHLV1kBxin=OoCYp=perRLD-tKQFawqKLmw3frA@mail.gmail.com>
Date:   Sun, 10 Dec 2017 10:00:02 +0100
From:   Philippe Ombredanne <pombredanne@...b.com>
To:     Steven Eckhoff <steven.eckhoff.opensource@...il.com>
Cc:     ALSA <alsa-devel@...a-project.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [alsa-devel][PATCH v3] ASoC: TSCS42xx: Add support for Tempo
 Semiconductor's TSCS42xx audio CODEC

Steven,

On Sat, Dec 9, 2017 at 11:42 PM, Steven Eckhoff
<steven.eckhoff.opensource@...il.com> wrote:
> Currently there is no support for the TSCS42xx audio CODEC.
>
> Add support for it.
>
> Below is the link to the v2 patch in case the threading is broken. This
>  patch addressed each issue raised in the last review.
>
> https://patchwork.kernel.org/patch/10058117/
>
> Signed-off-by: Steven Eckhoff <steven.eckhoff.opensource@...il.com>
> Cc: Steven Eckhoff <steven.eckhoff.opensource@...il.com>
> Cc: Liam Girdwood <lgirdwood@...il.com>
> Cc: Mark Brown <broonie@...nel.org>
> Cc: Jaroslav Kysela <perex@...ex.cz>
> Cc: Takashi Iwai <tiwai@...e.com>
> Cc: alsa-devel@...a-project.org
> Cc: linux-kernel@...r.kernel.org
[...]
> --- /dev/null
> +++ b/sound/soc/codecs/tscs42xx.c
> @@ -0,0 +1,1571 @@
> +/*
> + * tscs42xx.c -- TSCS42xx ALSA SoC Audio driver
> + *
> + * Copyright 2017 Tempo Semiconductor, Inc.
> + *
> + * Author: Steven Eckhoff <steven.eckhoff.opensource@...il.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */

Have you considered using the new SPDX ids? This would come out as:

> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * tscs42xx.c -- TSCS42xx ALSA SoC Audio driver
> + *
> + * Copyright 2017 Tempo Semiconductor, Inc.
> + *
> + * Author: Steven Eckhoff <steven.eckhoff.opensource@...il.com>
> + */

... and is shorter and greppable: there is nothing not to like in
this, unless you love legalese of course!

Check also Thomas doc patches and Linus rationale of why he wants this
as the top line using C++-style comments.

And if you agree like me with Linus take on C++ comments, you could
even go with less boilerplate with this:

> +// SPDX-License-Identifier: GPL-2.0
> +// tscs42xx.c -- TSCS42xx ALSA SoC Audio driver
> +// Copyright 2017 Tempo Semiconductor, Inc.
> +// Author: Steven Eckhoff <steven.eckhoff.opensource@...il.com>

This would make 12 lines of comment be just 4 .... with the same
effect, but less distraction from your fine code.

Thank you for your kind consideration!
-- 
Cordially
Philippe Ombredanne