[PATCH] ASoC: nau8821: Add driver for Nuvoton codec NAU88L21

[Mark Brown]

On Tue, Aug 24, 2021 at 12:16:47PM +0800, Seven Lee wrote:

> @@ -0,0 +1,1804 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * nau8821.c -- Nuvoton NAU88L21 audio codec driver
> + *

Please make the entire comment a C++ one so things look more
intentional.

> +/**
> + * nau8821_sema_acquire - acquire the semaphore of nau8821
> + * @nau8821:  component to register the codec private data with
> + * @timeout: how long in jiffies to wait before failure or zero to wait
> + * until release
> + *
> + * Attempts to acquire the semaphore with number of jiffies. If no more
> + * tasks are allowed to acquire the semaphore, calling this function will
> + * put the task to sleep. If the semaphore is not released within the
> + * specified number of jiffies, this function returns.
> + * If the semaphore is not released within the specified number of jiffies,
> + * this function returns -ETIME. If the sleep is interrupted by a signal,
> + * this function will return -EINTR. It returns 0 if the semaphore was
> + * acquired successfully.
> + *
> + * Acquires the semaphore without jiffies. Try to acquire the semaphore
> + * atomically. Returns 0 if the semaphore has been acquired successfully
> + * or 1 if it cannot be acquired.
> + */

As I said in reply to Pierre's mail and as I have said on
previous verisons of this patch can you please explain what is
going on with the semaphore - why it's being used, how it's
supposed to work and so on.  The above comment just documents
what a semaphore is which isn't the compliated or unusual part
here, what's complicated and unusual is the fact that there's a
semaphore in use at all.

As I have also suggested in reply to previous versions of this
patch I strongly recommend splitting the semaphore related
functionality out into separate patches so that it doesn't block
the rest of the driver.

Please don't ignore review comments, people are generally making them
for a reason and are likely to have the same concerns if issues remain
unaddressed.  Having to repeat the same comments can get repetitive and
make people question the value of time spent reviewing.  If you disagree
with the review comments that's fine but you need to reply and discuss
your concerns so that the reviewer can understand your decisions.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20210826/9e57467e/attachment.sig>