Re: [PATCH 1/2] ASoC: Add support for CS43130 codec

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Mon, Dec 12, 2016 at 02:21:03PM -0600, li.xu@xxxxxxxxxx wrote:
> Thank you for timely feedback.
> 
> I have fixed all except following.

Please fix your mail client configuration, it is not marking quoted
material as quoted and there's no word wrapping which makes everything
very hard to read.

> ---------------------------------------------------------------------------------------------------------------------------------------------------

There's also random stuff like the above appearing in the mail.

I strongly suggest working with some of your colleagues who are more
familiar with working upstream to try to understand the processes and
tooling.

> ---------------------------------------------------------------------------------------------------------------------------------------------------
> > +     /* Enable HP detect */
> > +     regmap_update_bits(cs43130->regmap, CS43130_HP_DETECT,
> > +             CS43130_HP_DETECT_CTRL_MASK, CS43130_HP_DETECT_CTRL_MASK);

> Why enable this when the only handling is a couple of log messages?

> Placeholder for driver modification when the driver is integrated to system such as Android OS.

> I suppose I could remove it, but when the driver is integrated into actual system, it may not be clear to system integrators,
> where to add HP DET IRQ hooks

Don't just add unfinished code that does nothing useful - that just
wastes everyone's time and makes the driver harder to review.  I'm
fairly sure that if people are able to implement jack detection they
will be able to figure out the fairly trivial code that's here and of
course this is something that when it's done should be just a driver
feature accessed through normal driver APIs.

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux