Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751425AbdLJJAp (ORCPT <rfc822;w@1wt.eu>);
        Sun, 10 Dec 2017 04:00:45 -0500
Received: from mail-wm0-f46.google.com ([74.125.82.46]:45079 "EHLO
        mail-wm0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1751345AbdLJJAo (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Sun, 10 Dec 2017 04:00:44 -0500
X-Google-Smtp-Source: AGs4zMaqcozM4wfxu1ShgAXjAQtfy/04Xh5cryUjZqTQqYRsaKh9TAlA3P5YKyN+YXxmX3pH+zyIqjKnA81g+Zs+NMI=
MIME-Version: 1.0
In-Reply-To: <20171209224205.2b4qcqpro37fnev7@localhost.localdomain>
References: <20171209224205.2b4qcqpro37fnev7@localhost.localdomain>
From: Philippe Ombredanne <pombredanne@nexb.com>
Date: Sun, 10 Dec 2017 10:00:02 +0100
Message-ID: <CAOFm3uHd_mepHLV1kBxin=OoCYp=perRLD-tKQFawqKLmw3frA@mail.gmail.com>
Subject: Re: [alsa-devel][PATCH v3] ASoC: TSCS42xx: Add support for Tempo
 Semiconductor's TSCS42xx audio CODEC
To: Steven Eckhoff <steven.eckhoff.opensource@gmail.com>
Cc: ALSA <alsa-devel@alsa-project.org>,
        Liam Girdwood <lgirdwood@gmail.com>, Mark Brown <broonie@kernel.org>,
        Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@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: 2223
Lines: 69

Steven,

On Sat, Dec 9, 2017 at 11:42 PM, Steven Eckhoff
<steven.eckhoff.opensource@gmail.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@gmail.com>
> Cc: Steven Eckhoff <steven.eckhoff.opensource@gmail.com>
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Jaroslav Kysela <perex@perex.cz>
> Cc: Takashi Iwai <tiwai@suse.com>
> Cc: alsa-devel@alsa-project.org
> Cc: linux-kernel@vger.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@gmail.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@gmail.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@gmail.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