[alsa-devel] [PATCH 1/2] ASoC: Add support for CS43130 codec

[Mark Brown]

On Mon, Dec 12, 2016 at 02:21:03PM -0600, li.xu at cirrus.com 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.
-------------- 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/20161213/d7ecf931/attachment.sig>