RE: [alsa-devel] [PATCH 1/6] ASoC: Tegra: Harmony: Add headphone jack detection

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

 



Stephen Warren wrote at Friday, February 04, 2011 1:32 PM:
> Stephen Warren wrote at Thursday, February 03, 2011 4:25 PM:
> >
> > Mark Brown wrote at Thursday, February 03, 2011 3:36 PM:
> > > On Thu, Feb 03, 2011 at 01:56:13PM -0700, Stephen Warren wrote:
> > >
> > > > +static struct snd_soc_jack harmony_hp_jack;
> > > > +
> > >
> > > Since you've changed to using a platform device you should really be
> > > dynamically allocating this I guess.  But this isn't actually a
> > > practical problem so not worth caring about.
> >
> > Uggh. The code may as well be consistent. I'll fix this up and resubmit
> > if you haven't already applied it.
> 
> Hmm. Looking at this a bit more, solving it fully is kinda nasty; I'd have
> to move not only that jack definition, but also all the pins and gpios
> into struct tegra_harmony, since snd_soc_jack_add_{pins,gpios} modify all
> of those structures. But, there would still have to be static const
> "template" copies of those data structures to initialize the copies in
> struct tegra_harmony.
> 
> The same thing then applies to some of the subsequent changes, for the
> mic jacks etc.
> 
> That seems like a lot of overhead, both code-wise, and runtime space-wise,
> to solve a problem that as you mention isn't a practical concern.

Mark, I'm happy to go either way on this; the SW engineer in me doesn't like
non-const globals and inconsistency with the rest of the state data, but
equally fixing this seems like it would bloat the code and might make it
less readable.

Let me know which way you'd prefer it to go.

Thanks.

--
nvpublic

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux