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. > +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. > + 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.
Attachment:
signature.asc
Description: PGP signature