Re: [PATCH v2 3/5] ASoC: Intel: Add Geminilake Dialog Maxim machine driver support

From: Mark Brown <broonie@kernel.org>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: vinod.koul@intel.com, Naveen Manohar <naveen.m@intel.com>,
	alsa-devel@alsa-project.org, harshapriya.n@intel.com,
	pierre-louis.bossart@intel.com
Subject: Re: [PATCH v2 3/5] ASoC: Intel: Add Geminilake Dialog Maxim machine driver support
Date: Wed, 11 Apr 2018 17:34:14 +0100	[thread overview]
Message-ID: <20180411163414.GC12166@sirena.org.uk> (raw)
In-Reply-To: <8fefaa36-2682-a604-4af9-6d4a2575c4f9@linux.intel.com>

[-- Attachment #1.1: Type: text/plain, Size: 689 bytes --]

On Wed, Apr 11, 2018 at 11:20:10AM -0500, Pierre-Louis Bossart wrote:

> > +static char glk_spk_dai_name[10];
> > +static char glk_hs_dai_name[10];

> Off-by-one? "SSPx-Codec" would be 11 chars if you include null termination.

Or avoid needing global statics entirely...

> >   static int broxton_audio_probe(struct platform_device *pdev)
> >   {
> >   	struct bxt_card_private *ctx;
> > +	int dai_index = 8;

> maybe better to do an explicit search that hard-code values?
> This will actually not work if you have an additional FE for headset on GLK,
> or you are assuming a dependency on a patch added later in the series - not
> good.

Definitely, this is just a recipe for fragility.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]