Hi Mark,
On Tue, 13 Nov 2018 at 23:51, Mark Brown broonie@kernel.org wrote:
On Thu, Nov 08, 2018 at 01:49:33PM +0100, Clément Péron wrote:
This looks mostly good, a few small things below but nothing too major:
--- /dev/null +++ b/sound/soc/codecs/ak4118.c @@ -0,0 +1,449 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- ak4118.c -- Asahi Kasei ALSA Soc Audio driver
Please make the entire comment a C++ one so this looks more intentional.
I check from other files and also this article says : "For normal C source files, the string will be a comment using the "//" syntax; header files, instead, use traditional (/* */) comments for reasons related to tooling" https://lwn.net/Articles/739183/
+static const char * const ak4118_iec958_fs_texts[] = {
"44100","Reserved",If you use a _VALUE_ENUM_SINGLE you can hide the reserved/invalid values from userspace which is easier to use.
Ok will change to SOC_VALUE_ENUM_SINGLE_DECL
ret = request_threaded_irq(gpiod_to_irq(ak4118->irq), NULL,ak4118_irq_handler, IRQF_TRIGGER_RISING |IRQF_ONESHOT, "ak4118-irq", ak4118);if (ret < 0) {dev_err(component->dev, "Fail to request_irq: %d\n", ret);return ret;}You should request resources in the device level probe, there's no point in requesting and releasing the interrupt like this.
Ok, will change to devm_request_threaded_irq and remove the irq_free
Thanks for the review, Regards, Clément